Merge pull request #36 from digitalocean/nshrader/resubmit-port

ovs: Introduce separate action for ResubmitPort

Author: icosahedral

ovs/action.go

@@ -41,6 +41,10 @@ var (
 	// errLoadSetFieldZero is returned when Load or SetField is called with value and/or
 	// field set to empty strings.
 	errLoadSetFieldZero = errors.New("value and/or field for action load or set_field are empty")
+
+	// errResubmitPortInvalid is returned when ResubmitPort is given a port number that is
+	// invalid per the openflow spec.
+	errResubmitPortInvalid = errors.New("resubmit port must be between 0 and 65279 inclusive")
 )
 
 // Action strings in lower case, as those are compared to the lower case letters
@@ -385,6 +389,36 @@ type resubmitAction struct {
 	table int
 }
 
+// ResubmitPort resubmits a packet into the current table with its context modified
+// to look like it originated from the specified openflow port ID.
+func ResubmitPort(port int) Action {
+	return &resubmitPortAction{
+		port: port,
+	}
+}
+
+// A resubmitPortAction is an Action which is used by ConneectionTracking.
+type resubmitPortAction struct {
+	port int
+}
+
+// MarshalText implements Action.
+func (a *resubmitPortAction) MarshalText() ([]byte, error) {
+	// Largest valid port ID is 0xfeff per openflow spec.
+	if a.port < 0 || a.port > 0xfeff {
+		return nil, errResubmitPortInvalid
+	}
+
+	p := strconv.Itoa(a.port)
+
+	return bprintf(patResubmitPort, p), nil
+}
+
+// GoString implements Action.
+func (a *resubmitPortAction) GoString() string {
+	return fmt.Sprintf("ovs.ResubmitPort(%d)", a.port)
+}
+
 // MarshalText implements Action.
 func (a *resubmitAction) MarshalText() ([]byte, error) {
 	if a.port == 0 && a.table == 0 {
@@ -399,10 +433,9 @@ func (a *resubmitAction) MarshalText() ([]byte, error) {
 	t := ""
 	if a.table != 0 {
 		t = strconv.Itoa(a.table)
-		return bprintf(patResubmitPortTable, p, t), nil
 	}
 
-	return bprintf(patResubmitPort, p), nil
+	return bprintf(patResubmitPortTable, p, t), nil
 }
 
 // GoString implements Action.

ovs/action_test.go

@@ -344,7 +344,7 @@ func TestActionResubmit(t *testing.T) {
 		{
 			desc:   "table zero",
 			port:   1,
-			action: "resubmit:1",
+			action: "resubmit(1,)",
 		},
 		{
 			desc:   "both port and table non-zero",
@@ -374,6 +374,60 @@ func TestActionResubmit(t *testing.T) {
 	}
 }
 
+func TestActionResubmitPort(t *testing.T) {
+	var tests = []struct {
+		desc   string
+		port   int
+		action string
+		err    error
+	}{
+		{
+			desc: "invalid port",
+			port: -1,
+			err:  errResubmitPortInvalid,
+		},
+		{
+			desc:   "port zero",
+			port:   0,
+			action: "resubmit:0",
+		},
+		{
+			desc:   "port 1",
+			port:   1,
+			action: "resubmit:1",
+		},
+		{
+			desc:   "max port (0xfeff)",
+			port:   0xfeff,
+			action: "resubmit:65279",
+		},
+		{
+			desc: "max port+1 (0xfeff)",
+			port: 0xff00,
+			err:  errResubmitPortInvalid,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.desc, func(t *testing.T) {
+			action, err := ResubmitPort(tt.port).MarshalText()
+
+			if want, got := tt.err, err; want != got {
+				t.Fatalf("unexpected error:\n- want: %v\n-  got: %v",
+					want, got)
+			}
+			if err != nil {
+				return
+			}
+
+			if want, got := tt.action, string(action); want != got {
+				t.Fatalf("unexpected Action:\n- want: %q\n-  got: %q",
+					want, got)
+			}
+		})
+	}
+}
+
 func TestActionLoadSetField(t *testing.T) {
 	var tests = []struct {
 		desc   string

ovs/actionparser.go

@@ -145,8 +145,12 @@ func (s *stack) pop() {
 
 var (
 	// resubmitRe is the regex used to match the resubmit action
-	// with one or more of its parameters.
-	resubmitRe = regexp.MustCompile(`resubmit(?::(\d+)|\((\d*),(\d*)\))`)
+	// with port and table specified
+	resubmitRe = regexp.MustCompile(`resubmit\((\d*),(\d*)\)`)
+
+	// resubmitPortRe is the regex used to match the resubmit action
+	// when only a port is specified
+	resubmitPortRe = regexp.MustCompile(`resubmit:(\d+)`)
 
 	// ctRe is the regex used to match the ct action with its
 	// parameter list.
@@ -305,8 +309,8 @@ func parseAction(s string) (Action, error) {
 		}
 	}
 
-	// ActionResubmit, with one or both of port number and table number
-	if ss := resubmitRe.FindAllStringSubmatch(s, 1); len(ss) > 0 && len(ss[0]) == 4 {
+	// ActionResubmit, with both port number and table number
+	if ss := resubmitRe.FindAllStringSubmatch(s, 1); len(ss) > 0 && len(ss[0]) == 3 {
 		var (
 			port  int
 			table int
@@ -316,7 +320,6 @@ func parseAction(s string) (Action, error) {
 
 		// Results are:
 		//  - full string
-		//  - single port
 		//  - port in parenthesis
 		//  - table in parenthesis
 
@@ -325,16 +328,8 @@ func parseAction(s string) (Action, error) {
 			if err != nil {
 				return nil, err
 			}
-			return Resubmit(port, 0), nil
 		}
-
 		if s := ss[0][2]; s != "" {
-			port, err = strconv.Atoi(s)
-			if err != nil {
-				return nil, err
-			}
-		}
-		if s := ss[0][3]; s != "" {
 			table, err = strconv.Atoi(s)
 			if err != nil {
 				return nil, err
@@ -344,6 +339,16 @@ func parseAction(s string) (Action, error) {
 		return Resubmit(port, table), nil
 	}
 
+	// ActionResubmitPort, with only a port number
+	if ss := resubmitPortRe.FindAllStringSubmatch(s, 1); len(ss) > 0 && len(ss[0]) == 2 {
+		port, err := strconv.Atoi(ss[0][1])
+		if err != nil {
+			return nil, err
+		}
+
+		return ResubmitPort(port), nil
+	}
+
 	if ss := loadRe.FindAllStringSubmatch(s, 2); len(ss) > 0 && len(ss[0]) == 3 {
 		// Results are:
 		//  - full string

ovs/actionparser_test.go

@@ -239,12 +239,11 @@ func Test_parseAction(t *testing.T) {
 		},
 		{
 			s: "resubmit:4",
-			a: Resubmit(4, 0),
+			a: ResubmitPort(4),
 		},
 		{
-			s:     "resubmit(1,)",
-			final: "resubmit:1",
-			a:     Resubmit(1, 0),
+			s: "resubmit(1,)",
+			a: Resubmit(1, 0),
 		},
 		{
 			s: "resubmit(,2)",