Fixes:

- Added JSON tags (ofPort, macAddress) to PortMapping struct for consistency
- Refactored to use parseEachPort() semantics instead of regex, following parseEach() pattern
- Removed unused regexp import and portMappingPattern variable
- Updated test to be generic table-driven instead of LOCAL-specific
- Added DumpPortMapping (singular) function for single interface lookup
- Fixed some error handling consistency, add MAC/ofport validation, and eliminate code duplication

Author: yrkurapati

ovs/openflow.go

@@ -21,8 +21,9 @@ import (
 	"errors"
 	"fmt"
 	"io"
-	"regexp"
+	"net"
 	"strconv"
+	"strings"
 )
 
 var (
@@ -74,10 +75,67 @@ 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
+	OfPort int `json:"ofPort"`
 
 	// MACAddress specifies the MAC address of the interface.
-	MACAddress string
+	MACAddress string `json:"macAddress"`
+}
+
+// UnmarshalText unmarshals a PortMapping from textual form as output by
+// 'ovs-ofctl show':
+//
+//	7(interface1): addr:fe:4f:76:09:88:2b
+func (p *PortMapping) UnmarshalText(b []byte) error {
+	// Make a copy per documentation for encoding.TextUnmarshaler.
+	s := strings.TrimSpace(string(b))
+
+	// Expected format: " 7(interface1): addr:fe:4f:76:09:88:2b"
+	// Find the opening parenthesis
+	openParen := strings.IndexByte(s, '(')
+	if openParen == -1 {
+		return fmt.Errorf("invalid port mapping format: missing opening parenthesis")
+	}
+
+	// Find the closing parenthesis
+	closeParen := strings.IndexByte(s[openParen:], ')')
+	if closeParen == -1 {
+		return fmt.Errorf("invalid port mapping format: missing closing parenthesis")
+	}
+	closeParen += openParen
+
+	// Extract ofport number (before opening parenthesis)
+	ofportStr := strings.TrimSpace(s[:openParen])
+	ofport, err := strconv.Atoi(ofportStr)
+	if err != nil {
+		return fmt.Errorf("invalid ofport number: %w", err)
+	}
+
+	// Validate ofport is in valid OpenFlow port range [0, 65535]
+	if ofport < 0 || ofport > 65535 {
+		return fmt.Errorf("ofport %d out of valid range [0, 65535]", ofport)
+	}
+
+	// Find "addr:" after the closing parenthesis
+	addrPrefix := ": addr:"
+	addrIdx := strings.Index(s[closeParen:], addrPrefix)
+	if addrIdx == -1 {
+		return fmt.Errorf("invalid port mapping format: missing addr prefix")
+	}
+	addrIdx += closeParen + len(addrPrefix)
+
+	// Extract MAC address (after "addr:")
+	macAddress := strings.TrimSpace(s[addrIdx:])
+
+	// Validate MAC address format
+	if _, err := net.ParseMAC(macAddress); err != nil {
+		return fmt.Errorf("invalid MAC address %q: %w", macAddress, err)
+	}
+
+	p.OfPort = ofport
+	// Note: interface name is not stored in PortMapping, it's used as map key
+	p.MACAddress = macAddress
+
+	return nil
 }
 
 // Possible flowDirective directive values.
@@ -335,6 +393,22 @@ func (o *OpenFlowService) DumpAggregate(bridge string, flow *MatchFlow) (*FlowSt
 	return stats, nil
 }
 
+// DumpPortMapping retrieves port mapping (ofport and MAC address) for a
+// specific interface from the specified bridge.
+func (o *OpenFlowService) DumpPortMapping(bridge string, interfaceName string) (*PortMapping, error) {
+	mappings, err := o.DumpPortMappings(bridge)
+	if err != nil {
+		return nil, err
+	}
+
+	mapping, ok := mappings[interfaceName]
+	if !ok {
+		return nil, fmt.Errorf("interface %q not found", interfaceName)
+	}
+
+	return mapping, nil
+}
+
 // DumpPortMappings retrieves port mappings (ofport and MAC address) for all
 // interfaces from the specified bridge. The output is parsed from 'ovs-ofctl show' command.
 func (o *OpenFlowService) DumpPortMappings(bridge string) (map[string]*PortMapping, error) {
@@ -347,30 +421,37 @@ func (o *OpenFlowService) DumpPortMappings(bridge string) (map[string]*PortMappi
 	}
 
 	mappings := make(map[string]*PortMapping)
-	scanner := bufio.NewScanner(bytes.NewReader(out))
-
-	for scanner.Scan() {
-		line := scanner.Bytes()
-		matches := portMappingPattern.FindSubmatch(line)
-		if len(matches) != 4 {
-			// Skip lines that don't match the pattern
-			continue
+	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
 		}
 
-		interfaceName := string(matches[2])
-		ofport, err := strconv.Atoi(string(matches[1]))
-		if err != nil {
-			// Skip malformed ofport numbers
-			continue
+		// 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])
 
-		mappings[interfaceName] = &PortMapping{
-			OfPort:     ofport,
-			MACAddress: string(matches[3]),
+		// Parse the PortMapping
+		pm := new(PortMapping)
+		if err := pm.UnmarshalText(line); err != nil {
+			// Skip malformed lines
+			return nil
 		}
-	}
 
-	if err := scanner.Err(); err != nil {
+		mappings[interfaceName] = pm
+		return nil
+	})
+
+	if err != nil {
 		return nil, fmt.Errorf("failed to parse port mappings: %w", err)
 	}
 
@@ -397,10 +478,9 @@ var (
 	// the output from "ovs-ofctl dump-aggregate"
 	//dumpAggregatePrefix = []byte("NXST_AGGREGATE reply")
 
-	// portMappingPattern matches port lines from 'ovs-ofctl show' output.
-	// Pattern matches: " 7(interface1): addr:fe:4f:76:09:88:2b"
-	// or " 65534(LOCAL): addr:fe:4f:76:09:88:2b"
-	portMappingPattern = regexp.MustCompile(`^\s*(\d+)\(([^)]+)\):\s*addr:([a-fA-F0-9:]+)`)
+	// showPrefix is a sentinel value returned at the beginning of
+	// the output from 'ovs-ofctl show'.
+	showPrefix = []byte("OFPT_FEATURES_REPLY")
 )
 
 // dumpPorts calls 'ovs-ofctl dump-ports' with the specified arguments and
@@ -555,6 +635,39 @@ func parseEach(in []byte, prefix []byte, fn func(b []byte) error) error {
 	return scanner.Err()
 }
 
+// parseEachPort parses ovs-ofctl show output from the input buffer, ensuring it has the
+// specified prefix, and invoking the input function on each line scanned.
+func parseEachPort(in []byte, prefix []byte, fn func(b []byte) error) error {
+	// Handle empty input - return nil to allow empty mappings (matches test expectations)
+	if len(in) == 0 {
+		return nil
+	}
+
+	// First line must not be empty
+	scanner := bufio.NewScanner(bytes.NewReader(in))
+	scanner.Split(bufio.ScanLines)
+	if !scanner.Scan() {
+		return io.ErrUnexpectedEOF
+	}
+
+	// First line must contain the prefix returned by OVS
+	if !bytes.Contains(scanner.Bytes(), prefix) {
+		return io.ErrUnexpectedEOF
+	}
+
+	// Scan every line to retrieve information needed to unmarshal
+	// a single PortMapping struct.
+	for scanner.Scan() {
+		b := make([]byte, len(scanner.Bytes()))
+		copy(b, scanner.Bytes())
+		if err := fn(b); err != nil {
+			return err
+		}
+	}
+
+	return scanner.Err()
+}
+
 // exec executes an ExecFunc using 'ovs-ofctl'.
 func (o *OpenFlowService) exec(args ...string) ([]byte, error) {
 	return o.c.exec("ovs-ofctl", args...)