Skip to content

Commit 154d603

Browse files
yrkurapatiyrkurapati
committed
Fixes:
- Refactored PortMapping to use nested PortAttr structure - Moveed ofPort and macAddress into PortAttr struct, keep ifName at top level
1 parent 9ce9349 commit 154d603

File tree

2 files changed

+93
-53
lines changed

2 files changed

+93
-53
lines changed

‎ovs/openflow.go‎

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -72,15 +72,21 @@ type flowDirective struct {
7272
flow string
7373
}
7474

75-
// PortMapping contains the ofport number and MAC address for an interface.
76-
type PortMapping struct {
75+
// PortAttr contains the ofport number and MAC address for an interface.
76+
type PortAttr struct {
7777
// ofPort specifies the OpenFlow port number.
7878
ofPort int
7979

8080
// macAddress specifies the MAC address of the interface.
8181
macAddress string
8282
}
8383

84+
// PortMapping contains the interface name, ofport number and MAC address for an interface.
85+
type PortMapping struct {
86+
portAttr PortAttr
87+
ifName string
88+
}
89+
8490
// UnmarshalText unmarshals a PortMapping from textual form as output by
8591
// 'ovs-ofctl show':
8692
//
@@ -102,6 +108,9 @@ func (p *PortMapping) UnmarshalText(b []byte) error {
102108
return fmt.Errorf("invalid port mapping format")
103109
}
104110

111+
// Extract interface name (between parentheses)
112+
ifName := s[openParen+1 : closeParen]
113+
105114
// Extract ofport number (before opening parenthesis)
106115
ofportStr := strings.TrimSpace(s[:openParen])
107116
ofport, err := strconv.Atoi(ofportStr)
@@ -130,9 +139,9 @@ func (p *PortMapping) UnmarshalText(b []byte) error {
130139
return fmt.Errorf("invalid MAC address %q: %w", macAddress, err)
131140
}
132141

133-
p.ofPort = ofport
134-
// Note: interface name is not stored in PortMapping, it's used as map key
135-
p.macAddress = macAddress
142+
p.portAttr.ofPort = ofport
143+
p.portAttr.macAddress = macAddress
144+
p.ifName = ifName
136145

137146
return nil
138147
}
@@ -421,20 +430,15 @@ func (o *OpenFlowService) DumpPortMappings(bridge string) (map[string]*PortMappi
421430

422431
mappings := make(map[string]*PortMapping)
423432
err = parseEachPort(out, showPrefix, func(line []byte) error {
424-
// Parse the PortMapping - UnmarshalText will validate format
433+
// Parse the PortMapping - UnmarshalText validates format and extracts all fields
425434
pm := new(PortMapping)
426435
if err := pm.UnmarshalText(line); err != nil {
427436
// Skip malformed lines
428437
return nil
429438
}
430439

431-
// Extract interface name from the validated line for map key
432-
s := string(line)
433-
openParen := strings.IndexByte(s, '(')
434-
closeParen := strings.IndexByte(s, ')')
435-
interfaceName := s[openParen+1 : closeParen]
436-
437-
mappings[interfaceName] = pm
440+
// Use interface name from PortMapping as map key
441+
mappings[pm.ifName] = pm
438442
return nil
439443
})
440444

‎ovs/openflow_test.go‎

Lines changed: 76 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1419,28 +1419,46 @@ func mustVerifyFlowBundle(t *testing.T, stdin io.Reader, flows []*Flow, matchFlo
14191419
func TestClientOpenFlowDumpPortMappingsOK(t *testing.T) {
14201420
want := map[string]*PortMapping{
14211421
"interface1": {
1422-
ofPort: 7,
1423-
macAddress: "fe:4f:76:09:88:2b",
1422+
portAttr: PortAttr{
1423+
ofPort: 7,
1424+
macAddress: "fe:4f:76:09:88:2b",
1425+
},
1426+
ifName: "interface1",
14241427
},
14251428
"interface2": {
1426-
ofPort: 8,
1427-
macAddress: "fe:be:7b:0d:53:d8",
1429+
portAttr: PortAttr{
1430+
ofPort: 8,
1431+
macAddress: "fe:be:7b:0d:53:d8",
1432+
},
1433+
ifName: "interface2",
14281434
},
14291435
"interface3": {
1430-
ofPort: 9,
1431-
macAddress: "fe:b6:4c:d5:40:79",
1436+
portAttr: PortAttr{
1437+
ofPort: 9,
1438+
macAddress: "fe:b6:4c:d5:40:79",
1439+
},
1440+
ifName: "interface3",
14321441
},
14331442
"interface4": {
1434-
ofPort: 20,
1435-
macAddress: "fe:cf:a6:90:30:29",
1443+
portAttr: PortAttr{
1444+
ofPort: 20,
1445+
macAddress: "fe:cf:a6:90:30:29",
1446+
},
1447+
ifName: "interface4",
14361448
},
14371449
"LOCAL": {
1438-
ofPort: 65534,
1439-
macAddress: "fe:74:0f:80:cf:9a",
1450+
portAttr: PortAttr{
1451+
ofPort: 65534,
1452+
macAddress: "fe:74:0f:80:cf:9a",
1453+
},
1454+
ifName: "LOCAL",
14401455
},
14411456
"eth0": {
1442-
ofPort: 1,
1443-
macAddress: "aa:bb:cc:dd:ee:ff",
1457+
portAttr: PortAttr{
1458+
ofPort: 1,
1459+
macAddress: "aa:bb:cc:dd:ee:ff",
1460+
},
1461+
ifName: "eth0",
14441462
},
14451463
}
14461464

@@ -1511,14 +1529,14 @@ actions: output enqueue set_vlan_vid set_vlan_pcp strip_vlan mod_dl_src mod_dl_d
15111529
t.Fatalf("missing mapping for interface %q", name)
15121530
}
15131531

1514-
if wantMapping.ofPort != gotMapping.ofPort {
1532+
if wantMapping.portAttr.ofPort != gotMapping.portAttr.ofPort {
15151533
t.Fatalf("unexpected ofPort for %q:\n- want: %d\n- got: %d",
1516-
name, wantMapping.ofPort, gotMapping.ofPort)
1534+
name, wantMapping.portAttr.ofPort, gotMapping.portAttr.ofPort)
15171535
}
15181536

1519-
if wantMapping.macAddress != gotMapping.macAddress {
1537+
if wantMapping.portAttr.macAddress != gotMapping.portAttr.macAddress {
15201538
t.Fatalf("unexpected macAddress for %q:\n- want: %q\n- got: %q",
1521-
name, wantMapping.macAddress, gotMapping.macAddress)
1539+
name, wantMapping.portAttr.macAddress, gotMapping.portAttr.macAddress)
15221540
}
15231541
}
15241542

@@ -1548,12 +1566,18 @@ func TestClientOpenFlowDumpPortMappingsEmptyOutput(t *testing.T) {
15481566
func TestClientOpenFlowDumpPortMappingsAllInterfaces(t *testing.T) {
15491567
want := map[string]*PortMapping{
15501568
"eth0": {
1551-
ofPort: 1,
1552-
macAddress: "aa:bb:cc:dd:ee:ff",
1569+
portAttr: PortAttr{
1570+
ofPort: 1,
1571+
macAddress: "aa:bb:cc:dd:ee:ff",
1572+
},
1573+
ifName: "eth0",
15531574
},
15541575
"eth1": {
1555-
ofPort: 2,
1556-
macAddress: "11:22:33:44:55:66",
1576+
portAttr: PortAttr{
1577+
ofPort: 2,
1578+
macAddress: "11:22:33:44:55:66",
1579+
},
1580+
ifName: "eth1",
15571581
},
15581582
}
15591583

@@ -1582,14 +1606,14 @@ OFPT_FEATURES_REPLY (xid=0x2): dpid:0000000000000001
15821606
t.Fatalf("missing mapping for interface %q", name)
15831607
}
15841608

1585-
if wantMapping.ofPort != gotMapping.ofPort {
1609+
if wantMapping.portAttr.ofPort != gotMapping.portAttr.ofPort {
15861610
t.Fatalf("unexpected ofPort for %q:\n- want: %d\n- got: %d",
1587-
name, wantMapping.ofPort, gotMapping.ofPort)
1611+
name, wantMapping.portAttr.ofPort, gotMapping.portAttr.ofPort)
15881612
}
15891613

1590-
if wantMapping.macAddress != gotMapping.macAddress {
1614+
if wantMapping.portAttr.macAddress != gotMapping.portAttr.macAddress {
15911615
t.Fatalf("unexpected macAddress for %q:\n- want: %q\n- got: %q",
1592-
name, wantMapping.macAddress, gotMapping.macAddress)
1616+
name, wantMapping.portAttr.macAddress, gotMapping.portAttr.macAddress)
15931617
}
15941618
}
15951619
}
@@ -1605,8 +1629,11 @@ func TestClientOpenFlowDumpPortMappingsForInterface(t *testing.T) {
16051629
name: "LOCAL interface",
16061630
interfaceName: "LOCAL",
16071631
want: &PortMapping{
1608-
ofPort: 65534,
1609-
macAddress: "fe:74:0f:80:cf:9a",
1632+
portAttr: PortAttr{
1633+
ofPort: 65534,
1634+
macAddress: "fe:74:0f:80:cf:9a",
1635+
},
1636+
ifName: "LOCAL",
16101637
},
16111638
output: `
16121639
OFPT_FEATURES_REPLY (xid=0x2): dpid:0000000000000001
@@ -1617,8 +1644,11 @@ OFPT_FEATURES_REPLY (xid=0x2): dpid:0000000000000001
16171644
name: "interface1",
16181645
interfaceName: "interface1",
16191646
want: &PortMapping{
1620-
ofPort: 7,
1621-
macAddress: "fe:4f:76:09:88:2b",
1647+
portAttr: PortAttr{
1648+
ofPort: 7,
1649+
macAddress: "fe:4f:76:09:88:2b",
1650+
},
1651+
ifName: "interface1",
16221652
},
16231653
output: `
16241654
OFPT_FEATURES_REPLY (xid=0x2): dpid:0000000000000001
@@ -1645,14 +1675,14 @@ OFPT_FEATURES_REPLY (xid=0x2): dpid:0000000000000001
16451675
t.Fatalf("missing mapping for interface %q", tt.interfaceName)
16461676
}
16471677

1648-
if tt.want.ofPort != gotMapping.ofPort {
1678+
if tt.want.portAttr.ofPort != gotMapping.portAttr.ofPort {
16491679
t.Fatalf("unexpected ofPort for %q:\n- want: %d\n- got: %d",
1650-
tt.interfaceName, tt.want.ofPort, gotMapping.ofPort)
1680+
tt.interfaceName, tt.want.portAttr.ofPort, gotMapping.portAttr.ofPort)
16511681
}
16521682

1653-
if tt.want.macAddress != gotMapping.macAddress {
1683+
if tt.want.portAttr.macAddress != gotMapping.portAttr.macAddress {
16541684
t.Fatalf("unexpected macAddress for %q:\n- want: %q\n- got: %q",
1655-
tt.interfaceName, tt.want.macAddress, gotMapping.macAddress)
1685+
tt.interfaceName, tt.want.portAttr.macAddress, gotMapping.portAttr.macAddress)
16561686
}
16571687
})
16581688
}
@@ -1689,8 +1719,11 @@ func TestClientOpenFlowDumpPortMapping(t *testing.T) {
16891719
name: "successful retrieval",
16901720
interfaceName: "interface1",
16911721
want: &PortMapping{
1692-
ofPort: 7,
1693-
macAddress: "fe:4f:76:09:88:2b",
1722+
portAttr: PortAttr{
1723+
ofPort: 7,
1724+
macAddress: "fe:4f:76:09:88:2b",
1725+
},
1726+
ifName: "interface1",
16941727
},
16951728
output: `
16961729
OFPT_FEATURES_REPLY (xid=0x2): dpid:0000000000000001
@@ -1713,8 +1746,11 @@ OFPT_FEATURES_REPLY (xid=0x2): dpid:0000000000000001
17131746
name: "LOCAL interface",
17141747
interfaceName: "LOCAL",
17151748
want: &PortMapping{
1716-
ofPort: 65534,
1717-
macAddress: "fe:74:0f:80:cf:9a",
1749+
portAttr: PortAttr{
1750+
ofPort: 65534,
1751+
macAddress: "fe:74:0f:80:cf:9a",
1752+
},
1753+
ifName: "LOCAL",
17181754
},
17191755
output: `
17201756
OFPT_FEATURES_REPLY (xid=0x2): dpid:0000000000000001
@@ -1747,14 +1783,14 @@ OFPT_FEATURES_REPLY (xid=0x2): dpid:0000000000000001
17471783
t.Fatalf("unexpected error: %v", err)
17481784
}
17491785

1750-
if tt.want.ofPort != got.ofPort {
1786+
if tt.want.portAttr.ofPort != got.portAttr.ofPort {
17511787
t.Fatalf("unexpected ofPort:\n- want: %d\n- got: %d",
1752-
tt.want.ofPort, got.ofPort)
1788+
tt.want.portAttr.ofPort, got.portAttr.ofPort)
17531789
}
17541790

1755-
if tt.want.macAddress != got.macAddress {
1791+
if tt.want.portAttr.macAddress != got.portAttr.macAddress {
17561792
t.Fatalf("unexpected macAddress:\n- want: %q\n- got: %q",
1757-
tt.want.macAddress, got.macAddress)
1793+
tt.want.portAttr.macAddress, got.portAttr.macAddress)
17581794
}
17591795
})
17601796
}

0 commit comments

Comments
 (0)