ovs: Handle port range parsing for tp_dst and tp_src keys (#46)

* Handle port range parsing for tp_dst and tp_src keys

Parsing of a flow with tp_dst and tp_src in port/mask format does not work.
Currently parseIntMatch() assumes only a single port in tp_dst and tp_src and not a port-range. As a result parsing a flow with port range fails.
This fix adds logic to parse tp_dst and tp_src in port-range format in a flow.

tp_dst=0xea60/0xffe0

* Simplied parsePort and added new testcase

Author: ssrikant20

AUTHORS

@@ -11,4 +11,5 @@ Contributors
 Michael Ben-Ami <[email protected]>
 Tejas Kokje <[email protected]>
 Kei Nohguchi <[email protected]>
-Neal Shrader <[email protected]>
+Neal Shrader <[email protected]>
+Sangeetha Srikanth <[email protected]>

ovs/flow_test.go

@@ -278,6 +278,19 @@ func TestFlowMarshalText(t *testing.T) {
 			},
 			s: "priority=400,ip,nw_dst=192.0.2.1,table=45,idle_timeout=0,actions=conjunction(123,1/2)",
 		},
+		{
+			desc: "TP Port Range",
+			f: &Flow{
+				InPort: 72,
+				Matches: []Match{
+					TransportSourceMaskedPort(0xea60, 0xffe0),
+					TransportDestinationMaskedPort(60000, 0xffe0),
+				},
+				Table:   55,
+				Actions: []Action{Drop()},
+			},
+			s: "priority=0,in_port=72,tp_src=0xea60/0xffe0,tp_dst=0xea60/0xffe0,table=55,idle_timeout=0,actions=drop",
+		},
 	}
 
 	for _, tt := range tests {
@@ -737,6 +750,22 @@ func TestFlowUnmarshalText(t *testing.T) {
 				},
 			},
 		},
+		{
+			desc: "TP Port Range",
+			s:    "priority=3000,tcp,in_port=72,tp_src=0xea60/0xffe0,tp_dst=0xea60/0xffe0,table=0,idle_timeout=0,actions=drop",
+			f: &Flow{
+				Priority: 3000,
+				Protocol: ProtocolTCPv4,
+				InPort:   72,
+				Matches: []Match{
+					TransportSourceMaskedPort(60000, 0xffe0),
+					TransportDestinationMaskedPort(0xea60, 0xffe0),
+				},
+				Table:       0,
+				IdleTimeout: 0,
+				Actions:     []Action{Drop()},
+			},
+		},
 	}
 
 	for _, tt := range tests {

ovs/matchflow_test.go

@@ -203,6 +203,19 @@ func TestMatchFlowMarshalText(t *testing.T) {
 			},
 			s: "tcp,tcp_flags=+syn+ack,nw_dst=192.0.2.1,tp_dst=22,table=45",
 		},
+		{
+			desc: "TP port range flow",
+			f: &MatchFlow{
+				Protocol: ProtocolUDPv4,
+				InPort:   33,
+				Matches: []Match{
+					NetworkDestination("192.0.2.1"),
+					TransportDestinationMaskedPort(0xea60, 0xffe0),
+				},
+				Table: 55,
+			},
+			s: "udp,in_port=33,nw_dst=192.0.2.1,tp_dst=0xea60/0xffe0,table=55",
+		},
 	}
 
 	for _, tt := range tests {

ovs/matchparser.go

@@ -31,8 +31,10 @@ func parseMatch(key string, value string) (Match, error) {
 		return parseMACMatch(key, value)
 	case icmpType, nwProto:
 		return parseIntMatch(key, value, math.MaxUint8)
-	case tpSRC, tpDST, ctZone:
+	case ctZone:
 		return parseIntMatch(key, value, math.MaxUint16)
+	case tpSRC, tpDST:
+		return parsePort(key, value, math.MaxUint16)
 	case conjID:
 		return parseIntMatch(key, value, math.MaxUint32)
 	case arpSPA:
@@ -104,10 +106,6 @@ func parseIntMatch(key string, value string, max int) (Match, error) {
 		return ICMPType(uint8(t)), nil
 	case nwProto:
 		return NetworkProtocol(uint8(t)), nil
-	case tpSRC:
-		return TransportSourcePort(uint16(t)), nil
-	case tpDST:
-		return TransportDestinationPort(uint16(t)), nil
 	case ctZone:
 		return ConnectionTrackingZone(uint16(t)), nil
 	case conjID:
@@ -117,6 +115,51 @@ func parseIntMatch(key string, value string, max int) (Match, error) {
 	return nil, fmt.Errorf("no action matched for %s=%s", key, value)
 }
 
+// parsePort parses a port or port/mask Match value from the input key and value,
+// with a maximum possible value of max.
+func parsePort(key string, value string, max int) (Match, error) {
+
+	var values []uint64
+	//Split the string
+	ss := strings.Split(value, "/")
+
+	//If input is just port
+	switch len(ss) {
+	case 1:
+		val, err := parseClampInt(value, max)
+		if err != nil {
+			return nil, err
+		}
+		values = append(values, uint64(val))
+		values = append(values, 0)
+		// If input is port/mask
+	case 2:
+		for _, s := range ss {
+			val, err := parseHexUint64(s)
+			if err != nil {
+				return nil, err
+			}
+			// Return error if val > 65536 (uint16)
+			if val > uint64(max) {
+				return nil, fmt.Errorf("integer %d too large; %d > %d", val, val, max)
+			}
+
+			values = append(values, val)
+		}
+	default:
+		return nil, fmt.Errorf("invalid value, no action matched for %s=%s", key, value)
+	}
+
+	switch key {
+	case tpSRC:
+		return TransportSourceMaskedPort(uint16(values[0]), uint16(values[1])), nil
+	case tpDST:
+		return TransportDestinationMaskedPort(uint16(values[0]), uint16(values[1])), nil
+	}
+	// Return error if input is invalid
+	return nil, fmt.Errorf("no action matched for %s=%s", key, value)
+}
+
 // parseMACMatch parses a MAC address Match value from the input key and value.
 func parseMACMatch(key string, value string) (Match, error) {
 	mac, err := net.ParseMAC(value)

ovs/matchparser_test.go

@@ -295,6 +295,56 @@ func Test_parseMatch(t *testing.T) {
 			s:       "conj_id=nope",
 			invalid: true,
 		},
+		{
+			desc:    "tp_dst out of range 65536/0xffe0",
+			s:       "tp_dst=65536/0xffe0",
+			invalid: true,
+		},
+		{
+			desc:    "tp_dst out of range 0x10000/0xffe0",
+			s:       "tp_dst=0x10000/0xffe0",
+			invalid: true,
+		},
+		{
+			desc:    "tp_dst out of range 0xea60/0x10000",
+			s:       "tp_dst=0xea60/0x10000",
+			invalid: true,
+		},
+		{
+			desc: "tp_dst 0xea60/0xffe0",
+			s:    "tp_dst=0xea60/0xffe0",
+			m:    TransportDestinationMaskedPort(0xea60, 0xffe0),
+		},
+		{
+			desc:    "tp_dst 0xea60/0xffe0/0xdddd",
+			s:       "tp_dst=0xea60/0xffe0/0xdddd",
+			invalid: true,
+		},
+		{
+			desc:    "tp_src out of range 65536/0xffe0",
+			s:       "tp_src=65536/0xffe0",
+			invalid: true,
+		},
+		{
+			desc:    "tp_src out of range 0x10000/0xffe0",
+			s:       "tp_src=0x10000/0xffe0",
+			invalid: true,
+		},
+		{
+			desc:    "tp_src out of range 0xea60/0x10000",
+			s:       "tp_src=0xea60/0x10000",
+			invalid: true,
+		},
+		{
+			desc: "tp_src 0xea60/0xffe0",
+			s:    "tp_src=0xea60/0xffe0",
+			m:    TransportSourceMaskedPort(0xea60, 0xffe0),
+		},
+		{
+			desc:    "tp_src 0xea60/0xffe0/0xdddd",
+			s:       "tp_src=0xea60/0xffe0/0xdddd",
+			invalid: true,
+		},
 	}
 
 	for _, tt := range tests {