ovsdb: don't create sized arrays for OVS Sets

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 and enforce the size when marshalling the set.

Signed-off-by: Jaime Caamaño Ruiz <[email protected]>
Co-authored-by: Dan Williams [email protected]

cache/cache.go

@@ -1336,7 +1336,10 @@ func (t *TableCache) ApplyModifications(tableName string, base model.Model, upda
 		nv := reflect.ValueOf(value)
 
 		switch reflect.ValueOf(current).Kind() {
-		case reflect.Slice, reflect.Array:
+		case reflect.Array:
+			// Not supposed to happen as all sets (max > 1) map to slices
+			panic(fmt.Sprintf("Array native value not supported, column: %s", k))
+		case reflect.Slice:
 			// The difference between two sets are all elements that only belong to one of the sets.
 			// If a value in the update set exists in the set, it will be removed from the base set.
 			// If a value in the update set does not exist in the set, it will be added to the base set.

client/client_test.go

@@ -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"`

mapper/mapper_test.go

@@ -32,7 +32,7 @@ var (
 	}
 	aFloat = 42.00
 
-	aFloatSet = [10]float64{
+	aFloatSet = []float64{
 		3.0,
 		2.0,
 		42.0,
@@ -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"`
@@ -308,7 +308,7 @@ func TestMapperNewRow(t *testing.T) {
 	}, {
 		name: "aFloatSet",
 		objInput: &struct {
-			MyFloatSet [10]float64 `ovsdb:"aFloatSet"`
+			MyFloatSet []float64 `ovsdb:"aFloatSet"`
 		}{
 			MyFloatSet: aFloatSet,
 		},

modelgen/table.go

@@ -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))

modelgen/table_test.go

@@ -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,

ovsdb/bindings.go

@@ -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, 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
 func NativeType(column *ColumnSchema) reflect.Type {
 	switch column.Type {
 	case TypeInteger, TypeReal, TypeBoolean, TypeUUID, TypeString:
@@ -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))
@@ -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)

ovsdb/encoding_test.go

@@ -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},

ovsdb/set.go

@@ -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())
 		}