Fixes:

- Removeed JSON tags and use unexported fields (ofPort, macAddress) following L62-66 pattern
- Removed redundant format checks in callback (UnmarshalText handles all validation)
- Fixed string slicing to search full string for closeParen and addrIdx
- Consolidated error messages to generic 'invalid port mapping format'

Author: yrkurapati

ovs/openflow.go

@@ -74,11 +74,11 @@ type flowDirective struct {
 
 // PortMapping contains the ofport number and MAC address for an interface.
 type PortMapping struct {
-	// OfPort specifies the OpenFlow port number.
-	OfPort int `json:"ofPort"`
+	// ofPort specifies the OpenFlow port number.
+	ofPort int
 
-	// MACAddress specifies the MAC address of the interface.
-	MACAddress string `json:"macAddress"`
+	// macAddress specifies the MAC address of the interface.
+	macAddress string
 }
 
 // UnmarshalText unmarshals a PortMapping from textual form as output by
@@ -93,15 +93,14 @@ func (p *PortMapping) UnmarshalText(b []byte) error {
 	// Find the opening parenthesis
 	openParen := strings.IndexByte(s, '(')
 	if openParen == -1 {
-		return fmt.Errorf("invalid port mapping format: missing opening parenthesis")
+		return fmt.Errorf("invalid port mapping format")
 	}
 
-	// Find the closing parenthesis
-	closeParen := strings.IndexByte(s[openParen:], ')')
-	if closeParen == -1 {
-		return fmt.Errorf("invalid port mapping format: missing closing parenthesis")
+	// Find the closing parenthesis in the full string
+	closeParen := strings.IndexByte(s, ')')
+	if closeParen == -1 || closeParen <= openParen {
+		return fmt.Errorf("invalid port mapping format")
 	}
-	closeParen += openParen
 
 	// Extract ofport number (before opening parenthesis)
 	ofportStr := strings.TrimSpace(s[:openParen])
@@ -115,13 +114,13 @@ func (p *PortMapping) UnmarshalText(b []byte) error {
 		return fmt.Errorf("ofport %d out of valid range [0, 65535]", ofport)
 	}
 
-	// Find "addr:" after the closing parenthesis
+	// Find "addr:" after the closing parenthesis in the full string
 	addrPrefix := ": addr:"
-	addrIdx := strings.Index(s[closeParen:], addrPrefix)
-	if addrIdx == -1 {
-		return fmt.Errorf("invalid port mapping format: missing addr prefix")
+	addrIdx := strings.Index(s, addrPrefix)
+	if addrIdx == -1 || addrIdx <= closeParen {
+		return fmt.Errorf("invalid port mapping format")
 	}
-	addrIdx += closeParen + len(addrPrefix)
+	addrIdx += len(addrPrefix)
 
 	// Extract MAC address (after "addr:")
 	macAddress := strings.TrimSpace(s[addrIdx:])
@@ -131,9 +130,9 @@ func (p *PortMapping) UnmarshalText(b []byte) error {
 		return fmt.Errorf("invalid MAC address %q: %w", macAddress, err)
 	}
 
-	p.OfPort = ofport
+	p.ofPort = ofport
 	// Note: interface name is not stored in PortMapping, it's used as map key
-	p.MACAddress = macAddress
+	p.macAddress = macAddress
 
 	return nil
 }
@@ -422,31 +421,19 @@ func (o *OpenFlowService) DumpPortMappings(bridge string) (map[string]*PortMappi
 
 	mappings := make(map[string]*PortMapping)
 	err = parseEachPort(out, showPrefix, func(line []byte) error {
-		// Check if line matches port format: " 7(interface1): addr:..."
-		if !bytes.Contains(line, []byte("(")) || !bytes.Contains(line, []byte("): addr:")) {
-			// Skip lines that don't match the port format
-			return nil
-		}
-
-		// Extract interface name from the line for map key (do this once)
-		openParen := bytes.IndexByte(line, '(')
-		if openParen == -1 {
-			return nil
-		}
-		closeParen := bytes.IndexByte(line[openParen:], ')')
-		if closeParen == -1 {
-			return nil
-		}
-		closeParen += openParen
-		interfaceName := string(line[openParen+1 : closeParen])
-
-		// Parse the PortMapping
+		// Parse the PortMapping - UnmarshalText will validate format
 		pm := new(PortMapping)
 		if err := pm.UnmarshalText(line); err != nil {
 			// Skip malformed lines
 			return nil
 		}
 
+		// Extract interface name from the validated line for map key
+		s := string(line)
+		openParen := strings.IndexByte(s, '(')
+		closeParen := strings.IndexByte(s, ')')
+		interfaceName := s[openParen+1 : closeParen]
+
 		mappings[interfaceName] = pm
 		return nil
 	})

ovs/openflow_test.go

@@ -1419,28 +1419,28 @@ func mustVerifyFlowBundle(t *testing.T, stdin io.Reader, flows []*Flow, matchFlo
 func TestClientOpenFlowDumpPortMappingsOK(t *testing.T) {
 	want := map[string]*PortMapping{
 		"interface1": {
-			OfPort:     7,
-			MACAddress: "fe:4f:76:09:88:2b",
+			ofPort:     7,
+			macAddress: "fe:4f:76:09:88:2b",
 		},
 		"interface2": {
-			OfPort:     8,
-			MACAddress: "fe:be:7b:0d:53:d8",
+			ofPort:     8,
+			macAddress: "fe:be:7b:0d:53:d8",
 		},
 		"interface3": {
-			OfPort:     9,
-			MACAddress: "fe:b6:4c:d5:40:79",
+			ofPort:     9,
+			macAddress: "fe:b6:4c:d5:40:79",
 		},
 		"interface4": {
-			OfPort:     20,
-			MACAddress: "fe:cf:a6:90:30:29",
+			ofPort:     20,
+			macAddress: "fe:cf:a6:90:30:29",
 		},
 		"LOCAL": {
-			OfPort:     65534,
-			MACAddress: "fe:74:0f:80:cf:9a",
+			ofPort:     65534,
+			macAddress: "fe:74:0f:80:cf:9a",
 		},
 		"eth0": {
-			OfPort:     1,
-			MACAddress: "aa:bb:cc:dd:ee:ff",
+			ofPort:     1,
+			macAddress: "aa:bb:cc:dd:ee:ff",
 		},
 	}
 
@@ -1511,14 +1511,14 @@ actions: output enqueue set_vlan_vid set_vlan_pcp strip_vlan mod_dl_src mod_dl_d
 			t.Fatalf("missing mapping for interface %q", name)
 		}
 
-		if wantMapping.OfPort != gotMapping.OfPort {
-			t.Fatalf("unexpected OfPort for %q:\n- want: %d\n-  got: %d",
-				name, wantMapping.OfPort, gotMapping.OfPort)
+		if wantMapping.ofPort != gotMapping.ofPort {
+			t.Fatalf("unexpected ofPort for %q:\n- want: %d\n-  got: %d",
+				name, wantMapping.ofPort, gotMapping.ofPort)
 		}
 
-		if wantMapping.MACAddress != gotMapping.MACAddress {
-			t.Fatalf("unexpected MACAddress for %q:\n- want: %q\n-  got: %q",
-				name, wantMapping.MACAddress, gotMapping.MACAddress)
+		if wantMapping.macAddress != gotMapping.macAddress {
+			t.Fatalf("unexpected macAddress for %q:\n- want: %q\n-  got: %q",
+				name, wantMapping.macAddress, gotMapping.macAddress)
 		}
 	}
 
@@ -1548,12 +1548,12 @@ func TestClientOpenFlowDumpPortMappingsEmptyOutput(t *testing.T) {
 func TestClientOpenFlowDumpPortMappingsAllInterfaces(t *testing.T) {
 	want := map[string]*PortMapping{
 		"eth0": {
-			OfPort:     1,
-			MACAddress: "aa:bb:cc:dd:ee:ff",
+			ofPort:     1,
+			macAddress: "aa:bb:cc:dd:ee:ff",
 		},
 		"eth1": {
-			OfPort:     2,
-			MACAddress: "11:22:33:44:55:66",
+			ofPort:     2,
+			macAddress: "11:22:33:44:55:66",
 		},
 	}
 
@@ -1582,14 +1582,14 @@ OFPT_FEATURES_REPLY (xid=0x2): dpid:0000000000000001
 			t.Fatalf("missing mapping for interface %q", name)
 		}
 
-		if wantMapping.OfPort != gotMapping.OfPort {
-			t.Fatalf("unexpected OfPort for %q:\n- want: %d\n-  got: %d",
-				name, wantMapping.OfPort, gotMapping.OfPort)
+		if wantMapping.ofPort != gotMapping.ofPort {
+			t.Fatalf("unexpected ofPort for %q:\n- want: %d\n-  got: %d",
+				name, wantMapping.ofPort, gotMapping.ofPort)
 		}
 
-		if wantMapping.MACAddress != gotMapping.MACAddress {
-			t.Fatalf("unexpected MACAddress for %q:\n- want: %q\n-  got: %q",
-				name, wantMapping.MACAddress, gotMapping.MACAddress)
+		if wantMapping.macAddress != gotMapping.macAddress {
+			t.Fatalf("unexpected macAddress for %q:\n- want: %q\n-  got: %q",
+				name, wantMapping.macAddress, gotMapping.macAddress)
 		}
 	}
 }
@@ -1605,8 +1605,8 @@ func TestClientOpenFlowDumpPortMappingsForInterface(t *testing.T) {
 			name:          "LOCAL interface",
 			interfaceName: "LOCAL",
 			want: &PortMapping{
-				OfPort:     65534,
-				MACAddress: "fe:74:0f:80:cf:9a",
+				ofPort:     65534,
+				macAddress: "fe:74:0f:80:cf:9a",
 			},
 			output: `
 OFPT_FEATURES_REPLY (xid=0x2): dpid:0000000000000001
@@ -1617,8 +1617,8 @@ OFPT_FEATURES_REPLY (xid=0x2): dpid:0000000000000001
 			name:          "interface1",
 			interfaceName: "interface1",
 			want: &PortMapping{
-				OfPort:     7,
-				MACAddress: "fe:4f:76:09:88:2b",
+				ofPort:     7,
+				macAddress: "fe:4f:76:09:88:2b",
 			},
 			output: `
 OFPT_FEATURES_REPLY (xid=0x2): dpid:0000000000000001
@@ -1645,14 +1645,14 @@ OFPT_FEATURES_REPLY (xid=0x2): dpid:0000000000000001
 				t.Fatalf("missing mapping for interface %q", tt.interfaceName)
 			}
 
-			if tt.want.OfPort != gotMapping.OfPort {
-				t.Fatalf("unexpected OfPort for %q:\n- want: %d\n-  got: %d",
-					tt.interfaceName, tt.want.OfPort, gotMapping.OfPort)
+			if tt.want.ofPort != gotMapping.ofPort {
+				t.Fatalf("unexpected ofPort for %q:\n- want: %d\n-  got: %d",
+					tt.interfaceName, tt.want.ofPort, gotMapping.ofPort)
 			}
 
-			if tt.want.MACAddress != gotMapping.MACAddress {
-				t.Fatalf("unexpected MACAddress for %q:\n- want: %q\n-  got: %q",
-					tt.interfaceName, tt.want.MACAddress, gotMapping.MACAddress)
+			if tt.want.macAddress != gotMapping.macAddress {
+				t.Fatalf("unexpected macAddress for %q:\n- want: %q\n-  got: %q",
+					tt.interfaceName, tt.want.macAddress, gotMapping.macAddress)
 			}
 		})
 	}
@@ -1689,8 +1689,8 @@ func TestClientOpenFlowDumpPortMapping(t *testing.T) {
 			name:          "successful retrieval",
 			interfaceName: "interface1",
 			want: &PortMapping{
-				OfPort:     7,
-				MACAddress: "fe:4f:76:09:88:2b",
+				ofPort:     7,
+				macAddress: "fe:4f:76:09:88:2b",
 			},
 			output: `
 OFPT_FEATURES_REPLY (xid=0x2): dpid:0000000000000001
@@ -1713,8 +1713,8 @@ OFPT_FEATURES_REPLY (xid=0x2): dpid:0000000000000001
 			name:          "LOCAL interface",
 			interfaceName: "LOCAL",
 			want: &PortMapping{
-				OfPort:     65534,
-				MACAddress: "fe:74:0f:80:cf:9a",
+				ofPort:     65534,
+				macAddress: "fe:74:0f:80:cf:9a",
 			},
 			output: `
 OFPT_FEATURES_REPLY (xid=0x2): dpid:0000000000000001
@@ -1747,14 +1747,14 @@ OFPT_FEATURES_REPLY (xid=0x2): dpid:0000000000000001
 				t.Fatalf("unexpected error: %v", err)
 			}
 
-			if tt.want.OfPort != got.OfPort {
-				t.Fatalf("unexpected OfPort:\n- want: %d\n-  got: %d",
-					tt.want.OfPort, got.OfPort)
+			if tt.want.ofPort != got.ofPort {
+				t.Fatalf("unexpected ofPort:\n- want: %d\n-  got: %d",
+					tt.want.ofPort, got.ofPort)
 			}
 
-			if tt.want.MACAddress != got.MACAddress {
-				t.Fatalf("unexpected MACAddress:\n- want: %q\n-  got: %q",
-					tt.want.MACAddress, got.MACAddress)
+			if tt.want.macAddress != got.macAddress {
+				t.Fatalf("unexpected macAddress:\n- want: %q\n-  got: %q",
+					tt.want.macAddress, got.macAddress)
 			}
 		})
 	}