Skip to content

Commit e189b8a

Browse files
filanovrollandf
filanov
authored and
rollandf
committed
fix: change arping call
different versions of arping will get different reply format to avoid grep errors we will switch to two different calls arping to store the mac address in the host ip neighbor to find the mac Signed-off-by: Michael Filanov <[email protected]>
1 parent 88e6f31 commit e189b8a

File tree

2 files changed

+35
-10
lines changed

2 files changed

+35
-10
lines changed

‎internal/controller/flows.go‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ func (f *Flows) AddPodRailFlows(cookie uint64, vf, bridge, podIP, podMAC string)
7676

7777
torMAC, err := f.getTorMac(torIP)
7878
if err != nil {
79-
return fmt.Errorf("failed to get tor mac for bridge [%s]: %v", bridge, err)
79+
return fmt.Errorf("failed to get tor mac for bridge [%s] reply: %s err: %v", bridge, torMAC, err)
8080
}
8181

8282
uplink, err := f.Exec.Execute(fmt.Sprintf("ovs-vsctl br-get-external-id %s %s", bridge, railUplink))
@@ -162,17 +162,20 @@ func (f *Flows) IsBridgeManagedByRailCNI(bridge, podID string) (bool, error) {
162162

163163
func (f *Flows) getTorMac(torIP string) (string, error) {
164164
// nsenter --target 1 --net -- arping 2.0.0.3 -c 1
165-
// nsenter --target 1 --net -- ip neighbor | grep 2.0.0.3 | awk '{print $5}'
166165
// TODO: check why it always return an error
167-
reply, _ := f.Exec.ExecutePrivileged(fmt.Sprintf(`arping %s -c 1 | grep "reply from" | awk '{print $5}' | tr -d '[]'`,
168-
torIP))
166+
_, _ = f.Exec.ExecutePrivileged(fmt.Sprintf(`arping %s -c 1 &> /dev/null`, torIP))
169167
// if err != nil {
170168
// logr.Error(err, fmt.Sprintf("failed to exec: arping %s -c 1", rail.Tor))
171169
// return "", err
172170
// }
173171

172+
reply, err := f.Exec.ExecutePrivileged(fmt.Sprintf(`ip neighbor | grep %s | awk '{print $5}'`, torIP))
173+
if err != nil {
174+
return "", fmt.Errorf("failed to get tor mac for bridge %s: reply: %s, err: %v", torIP, reply, err)
175+
}
176+
174177
if reply == "" {
175-
return "", fmt.Errorf("no reply from arping %s", torIP)
178+
return "", fmt.Errorf("arp reply not found from arping %s", torIP)
176179
}
177180

178181
return reply, nil

‎internal/controller/flows_test.go‎

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,10 @@ var _ = Describe("Flows", func() {
9595
execMock.EXPECT().Execute("ovs-vsctl br-get-external-id br-rail1 rail_peer_ip").Return("192.168.1.1", nil)
9696

9797
// Mock getting TOR MAC
98-
execMock.EXPECT().ExecutePrivileged(gomock.Any()).Return("00:11:22:33:44:55", nil)
98+
execMock.EXPECT().ExecutePrivileged("arping 192.168.1.1 -c 1 &> /dev/null").Return("", nil)
99+
100+
// check
101+
execMock.EXPECT().ExecutePrivileged("ip neighbor | grep 192.168.1.1 | awk '{print $5}'").Return("00:11:22:33:44:55", nil)
99102

100103
// Mock getting uplink
101104
execMock.EXPECT().Execute("ovs-vsctl br-get-external-id br-rail1 rail_uplink").Return("p0", nil)
@@ -119,7 +122,8 @@ var _ = Describe("Flows", func() {
119122
execMock.EXPECT().Execute(matchSubstring("ovs-ofctl add-flow br-rail1")).Return("", nil)
120123
netlinkMock.EXPECT().LinkByName("br-rail1").Return(mockLink, nil)
121124
execMock.EXPECT().Execute("ovs-vsctl br-get-external-id br-rail1 rail_peer_ip").Return("192.168.1.1", nil)
122-
execMock.EXPECT().ExecutePrivileged(gomock.Any()).Return("00:11:22:33:44:55", nil)
125+
execMock.EXPECT().ExecutePrivileged("arping 192.168.1.1 -c 1 &> /dev/null").Return("", nil)
126+
execMock.EXPECT().ExecutePrivileged("ip neighbor | grep 192.168.1.1 | awk '{print $5}'").Return("00:11:22:33:44:55", nil)
123127
execMock.EXPECT().Execute("ovs-vsctl br-get-external-id br-rail1 rail_uplink").Return("p0", nil)
124128
// Second IP flow fails
125129
execMock.EXPECT().Execute(matchSubstring("ovs-ofctl add-flow br-rail1")).Return("", fmt.Errorf("failed to add ip flow"))
@@ -134,7 +138,8 @@ var _ = Describe("Flows", func() {
134138
execMock.EXPECT().Execute(matchSubstring("ovs-ofctl add-flow br-rail1")).Return("", nil)
135139
netlinkMock.EXPECT().LinkByName("br-rail1").Return(mockLink, nil)
136140
execMock.EXPECT().Execute("ovs-vsctl br-get-external-id br-rail1 rail_peer_ip").Return("192.168.1.1", nil)
137-
execMock.EXPECT().ExecutePrivileged(gomock.Any()).Return("00:11:22:33:44:55", nil)
141+
execMock.EXPECT().ExecutePrivileged("arping 192.168.1.1 -c 1 &> /dev/null").Return("", nil)
142+
execMock.EXPECT().ExecutePrivileged("ip neighbor | grep 192.168.1.1 | awk '{print $5}'").Return("00:11:22:33:44:55", nil)
138143
execMock.EXPECT().Execute("ovs-vsctl br-get-external-id br-rail1 rail_uplink").Return("p0", nil)
139144
// Second IP flow succeeds
140145
execMock.EXPECT().Execute(matchSubstring("ovs-ofctl add-flow br-rail1")).Return("", nil)
@@ -184,7 +189,8 @@ var _ = Describe("Flows", func() {
184189
// Then it checks for TOR IP
185190
execMock.EXPECT().Execute("ovs-vsctl br-get-external-id br-rail1 rail_peer_ip").Return("192.168.1.1", nil)
186191
// Then it gets TOR MAC
187-
execMock.EXPECT().ExecutePrivileged(gomock.Any()).Return("00:11:22:33:44:55", nil)
192+
execMock.EXPECT().ExecutePrivileged("arping 192.168.1.1 -c 1 &> /dev/null").Return("", nil)
193+
execMock.EXPECT().ExecutePrivileged("ip neighbor | grep 192.168.1.1 | awk '{print $5}'").Return("00:11:22:33:44:55", nil)
188194
// Finally it checks for uplink
189195
execMock.EXPECT().Execute("ovs-vsctl br-get-external-id br-rail1 rail_uplink").Return("", nil)
190196

@@ -199,7 +205,22 @@ var _ = Describe("Flows", func() {
199205
netlinkMock.EXPECT().LinkByName("br-rail1").Return(mockLink, nil)
200206
execMock.EXPECT().Execute("ovs-vsctl br-get-external-id br-rail1 rail_peer_ip").Return("192.168.1.1", nil)
201207
// TOR MAC retrieval fails
202-
execMock.EXPECT().ExecutePrivileged(gomock.Any()).Return("", fmt.Errorf("failed to get TOR MAC"))
208+
execMock.EXPECT().ExecutePrivileged("arping 192.168.1.1 -c 1 &> /dev/null").Return("", fmt.Errorf("failed to get TOR MAC"))
209+
execMock.EXPECT().ExecutePrivileged("ip neighbor | grep 192.168.1.1 | awk '{print $5}'").Return("", fmt.Errorf("failed to get TOR MAC"))
210+
211+
err := flows.AddPodRailFlows(0x5, "vf0", "br-rail1", "10.0.0.1", "00:11:22:33:44:66")
212+
Expect(err).Should(HaveOccurred())
213+
Expect(err.Error()).Should(ContainSubstring("failed to get tor mac for bridge"))
214+
})
215+
216+
It("should return error if fails to get TOR MAC from arping", func() {
217+
// First ARP flow succeeds
218+
execMock.EXPECT().Execute(matchSubstring("ovs-ofctl add-flow br-rail1")).Return("", nil)
219+
netlinkMock.EXPECT().LinkByName("br-rail1").Return(mockLink, nil)
220+
execMock.EXPECT().Execute("ovs-vsctl br-get-external-id br-rail1 rail_peer_ip").Return("192.168.1.1", nil)
221+
// TOR MAC retrieval fails
222+
execMock.EXPECT().ExecutePrivileged("arping 192.168.1.1 -c 1 &> /dev/null").Return("", fmt.Errorf("failed to get TOR MAC"))
223+
execMock.EXPECT().ExecutePrivileged("ip neighbor | grep 192.168.1.1 | awk '{print $5}'").Return("", nil)
203224

204225
err := flows.AddPodRailFlows(0x5, "vf0", "br-rail1", "10.0.0.1", "00:11:22:33:44:66")
205226
Expect(err).Should(HaveOccurred())
@@ -212,6 +233,7 @@ var _ = Describe("Flows", func() {
212233
netlinkMock.EXPECT().LinkByName("br-rail1").Return(mockLink, nil)
213234
execMock.EXPECT().Execute("ovs-vsctl br-get-external-id br-rail1 rail_peer_ip").Return("192.168.1.1", nil)
214235
// TOR MAC retrieval succeeds
236+
execMock.EXPECT().ExecutePrivileged("arping 192.168.1.1 -c 1 &> /dev/null").Return("", nil)
215237
execMock.EXPECT().ExecutePrivileged(gomock.Any()).Return("00:11:22:33:44:55", nil)
216238
// Uplink retrieval fails
217239
execMock.EXPECT().Execute("ovs-vsctl br-get-external-id br-rail1 rail_uplink").Return("", fmt.Errorf("failed to get uplink"))

0 commit comments

Comments
 (0)