Skip to content

Commit

Permalink
ovsdb: don't create sized arrays for OVS Sets
Browse files Browse the repository at this point in the history
OVS Sets must be unique, but modelgen created sized arrays
to represent OVS Sets, which when marshalled are filled with
duplicate default values.

For a column schema like:

   "cvlans": {
     "type": {"key": {"type": "integer",
                      "minInteger": 0,
                      "maxInteger": 4095},
              "min": 0, "max": 4096}},

modelgen would create:

   CVLANs [4096]int

which when marshalled and sent to ovsdb-server becomes:

cvlans:{GoSet:[0 0 0 0 0 0 0 <repeat many times>]}

and is rejected by ovsdb-server with:

{Count:0 Error:ovsdb error Details:set contains duplicate UUID:{GoUUID:} Rows:[]}] and errors [ovsdb error: set contains duplicate]: 1 ovsdb operations failed

Instead, generate these fields as slices instead of sized
arrays.

Signed-off-by: Jaime Caamaño Ruiz <[email protected]>
Co-authored-by: Dan Williams [email protected]
  • Loading branch information
jcaamano committed May 16, 2023
1 parent 564b257 commit 02ba76f
Show file tree
Hide file tree
Showing 7 changed files with 14 additions and 53 deletions.
2 changes: 1 addition & 1 deletion client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ type Bridge struct {
DatapathVersion string `ovsdb:"datapath_version"`
ExternalIDs map[string]string `ovsdb:"external_ids"`
FailMode *BridgeFailMode `ovsdb:"fail_mode"`
FloodVLANs [4096]int `ovsdb:"flood_vlans"`
FloodVLANs []int `ovsdb:"flood_vlans"`
FlowTables map[int]string `ovsdb:"flow_tables"`
IPFIX *string `ovsdb:"ipfix"`
McastSnoopingEnable bool `ovsdb:"mcast_snooping_enable"`
Expand Down
6 changes: 3 additions & 3 deletions mapper/mapper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ var (
}
aFloat = 42.00

aFloatSet = [10]float64{
aFloatSet = []float64{
3.0,
2.0,
42.0,
Expand Down Expand Up @@ -192,7 +192,7 @@ func TestMapperGetData(t *testing.T) {
AUUID string `ovsdb:"aUUID"`
AIntSet []int `ovsdb:"aIntSet"`
AFloat float64 `ovsdb:"aFloat"`
AFloatSet [10]float64 `ovsdb:"aFloatSet"`
AFloatSet []float64 `ovsdb:"aFloatSet"`
YetAnotherStringSet []string `ovsdb:"aEmptySet"`
AEnum string `ovsdb:"aEnum"`
AMap map[string]string `ovsdb:"aMap"`
Expand Down Expand Up @@ -308,7 +308,7 @@ func TestMapperNewRow(t *testing.T) {
}, {
name: "aFloatSet",
objInput: &struct {
MyFloatSet [10]float64 `ovsdb:"aFloatSet"`
MyFloatSet []float64 `ovsdb:"aFloatSet"`
}{
MyFloatSet: aFloatSet,
},
Expand Down
7 changes: 0 additions & 7 deletions modelgen/table.go
Original file line number Diff line number Diff line change
Expand Up @@ -356,13 +356,6 @@ func fieldType(tableName, columnName string, column *ovsdb.ColumnSchema, enumTyp
}
return AtomicType(column.TypeObj.Key.Type)
}
// use array for columns with max > 1
if column.TypeObj.Max() > 1 {
if enumTypes && FieldEnum(tableName, columnName, column) != nil {
return fmt.Sprintf("[%d]%s", column.TypeObj.Max(), enumName(tableName, columnName))
}
return fmt.Sprintf("[%d]%s", column.TypeObj.Max(), AtomicType(column.TypeObj.Key.Type))
}
// use a slice
if enumTypes && FieldEnum(tableName, columnName, column) != nil {
return fmt.Sprintf("[]%s", enumName(tableName, columnName))
Expand Down
2 changes: 1 addition & 1 deletion modelgen/table_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -850,7 +850,7 @@ func buildTestBridge() *vswitchd.Bridge {
DatapathVersion: *buildRandStr(),
ExternalIDs: map[string]string{*buildRandStr(): *buildRandStr(), *buildRandStr(): *buildRandStr()},
FailMode: &vswitchd.BridgeFailModeSecure,
FloodVLANs: [4096]int{0, 1, 2, 3, 4, 5, 6, 7, 8, 9},
FloodVLANs: []int{0, 1, 2, 3, 4, 5, 6, 7, 8, 9},
FlowTables: map[int]string{1: *buildRandStr(), 2: *buildRandStr()},
IPFIX: buildRandStr(),
McastSnoopingEnable: false,
Expand Down
38 changes: 8 additions & 30 deletions ovsdb/bindings.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,13 @@ func NativeTypeFromAtomic(basicType string) reflect.Type {
}
}

//NativeType returns the reflect.Type that can hold the value of a column
//OVS Type to Native Type convertions:
// OVS sets -> go slices, arrays or a go native type depending on the key
// OVS uuid -> go strings
// OVS map -> go map
// OVS enum -> go native type depending on the type of the enum key
// NativeType returns the reflect.Type that can hold the value of a column
// OVS Type to Native Type convertions:
//
// OVS sets -> go slices or a go native type depending on the key
// OVS uuid -> go strings
// OVS map -> go map
// OVS enum -> go native type depending on the type of the enum key
func NativeType(column *ColumnSchema) reflect.Type {
switch column.Type {
case TypeInteger, TypeReal, TypeBoolean, TypeUUID, TypeString:
Expand All @@ -78,10 +79,6 @@ func NativeType(column *ColumnSchema) reflect.Type {
if column.TypeObj.Min() == 1 && column.TypeObj.Max() == 1 {
return keyType
}
// max is > 1, use an array
if column.TypeObj.Max() > 1 {
return reflect.ArrayOf(column.TypeObj.Max(), keyType)
}
return reflect.SliceOf(keyType)
default:
panic(fmt.Errorf("unknown extended type %s", column.Type))
Expand Down Expand Up @@ -178,29 +175,10 @@ func OvsToNative(column *ColumnSchema, ovsElem interface{}) (interface{}, error)
pv.Elem().Set(reflect.ValueOf(native))
return pv.Interface(), nil
}
case reflect.Array:
array := reflect.New(reflect.ArrayOf(column.TypeObj.Max(), naType.Elem())).Elem()
switch ovsSet := ovsElem.(type) {
case OvsSet:
for i, v := range ovsSet.GoSet {
nv, err := OvsToNativeAtomic(column.TypeObj.Key.Type, v)
if err != nil {
return nil, err
}
array.Index(i).Set(reflect.ValueOf(nv))
}
default:
nv, err := OvsToNativeAtomic(column.TypeObj.Key.Type, ovsElem)
if err != nil {
return nil, err
}
array.Index(0).Set(reflect.ValueOf(nv))
}
return array.Interface(), nil
case reflect.Slice:
return OvsToNativeSlice(column.TypeObj.Key.Type, ovsElem)
default:
return nil, fmt.Errorf("native type was not slice, array or pointer. got %d", naType.Kind())
return nil, fmt.Errorf("native type was not slice or pointer. got %d", naType.Kind())
}
case TypeMap:
naType := NativeType(column)
Expand Down
10 changes: 0 additions & 10 deletions ovsdb/encoding_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,21 +96,11 @@ func TestSet(t *testing.T) {
[]string{`aa`},
`"aa"`,
},
{
"string array",
[1]string{`aa`},
`"aa"`,
},
{
"string slice with multiple elements",
[]string{`aa`, `bb`},
`["set",["aa","bb"]]`,
},
{
"string array multiple elements",
[2]string{`aa`, `bb`},
`["set",["aa","bb"]]`,
},
{
"float slice",
[]float64{10.2, 15.4},
Expand Down
2 changes: 1 addition & 1 deletion ovsdb/set.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func NewOvsSet(obj interface{}) (OvsSet, error) {
}

switch v.Kind() {
case reflect.Slice, reflect.Array:
case reflect.Slice:
for i := 0; i < v.Len(); i++ {
ovsSet = append(ovsSet, v.Index(i).Interface())
}
Expand Down

0 comments on commit 02ba76f

Please sign in to comment.