Skip to content

Commit b9f6ca2

Browse files
filanovfilanov
committed
fix: replace arping with ping
arping command won't update the ARP table as it is not initiated by the kernel. Using ping instead. For this reason we are switching to ping and checking the ip neighbor to avoid parsing any output for ping or arping commands directly Signed-off-by: Michael Filanov <[email protected]>
1 parent 8ea2be3 commit b9f6ca2

File tree

3 files changed

+30
-20
lines changed

3 files changed

+30
-20
lines changed

‎Dockerfile‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ FROM nvcr.io/nvidia/doca/doca:3.1.0-full-rt-host
4444

4545
RUN apt update && apt install -y \
4646
iproute2 \
47-
iputils-arping
47+
iputils-ping
4848

4949
WORKDIR /
5050
COPY --from=builder /workspace/build/flowcontroller .

‎internal/controller/flows.go‎

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -161,21 +161,18 @@ func (f *Flows) IsBridgeManagedByRailCNI(bridge, podID string) (bool, error) {
161161
}
162162

163163
func (f *Flows) getTorMac(torIP string) (string, error) {
164-
// nsenter --target 1 --net -- arping 2.0.0.3 -c 1
165-
// TODO: check why it always return an error
166-
_, _ = f.Exec.ExecutePrivileged(fmt.Sprintf(`arping %s -c 1 &> /dev/null`, torIP))
167-
// if err != nil {
168-
// logr.Error(err, fmt.Sprintf("failed to exec: arping %s -c 1", rail.Tor))
169-
// return "", err
170-
// }
171-
172-
reply, err := f.Exec.ExecutePrivileged(fmt.Sprintf(`ip neighbor | grep %s | awk '{print $5}'`, torIP))
164+
reply, err := f.Exec.ExecutePrivileged(fmt.Sprintf(`ping %s -c 1`, torIP))
165+
if err != nil {
166+
return "", fmt.Errorf("failed to exec: ping %s -c 1: reply: %s, err: %v", torIP, reply, err)
167+
}
168+
169+
reply, err = f.Exec.ExecutePrivileged(fmt.Sprintf(`ip neighbor | grep %s | awk '{print $5}'`, torIP))
173170
if err != nil {
174171
return "", fmt.Errorf("failed to get tor mac for bridge %s: reply: %s, err: %v", torIP, reply, err)
175172
}
176173

177174
if reply == "" {
178-
return "", fmt.Errorf("arp reply not found from arping %s", torIP)
175+
return "", fmt.Errorf("TOR %s mac not found", torIP)
179176
}
180177

181178
return reply, nil

‎internal/controller/flows_test.go‎

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ 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("arping 192.168.1.1 -c 1 &> /dev/null").Return("", nil)
98+
execMock.EXPECT().ExecutePrivileged("ping 192.168.1.1 -c 1").Return("", nil)
9999

100100
// check
101101
execMock.EXPECT().ExecutePrivileged("ip neighbor | grep 192.168.1.1 | awk '{print $5}'").Return("00:11:22:33:44:55", nil)
@@ -122,7 +122,7 @@ var _ = Describe("Flows", func() {
122122
execMock.EXPECT().Execute(matchSubstring("ovs-ofctl add-flow br-rail1")).Return("", nil)
123123
netlinkMock.EXPECT().LinkByName("br-rail1").Return(mockLink, nil)
124124
execMock.EXPECT().Execute("ovs-vsctl br-get-external-id br-rail1 rail_peer_ip").Return("192.168.1.1", nil)
125-
execMock.EXPECT().ExecutePrivileged("arping 192.168.1.1 -c 1 &> /dev/null").Return("", nil)
125+
execMock.EXPECT().ExecutePrivileged("ping 192.168.1.1 -c 1").Return("", nil)
126126
execMock.EXPECT().ExecutePrivileged("ip neighbor | grep 192.168.1.1 | awk '{print $5}'").Return("00:11:22:33:44:55", nil)
127127
execMock.EXPECT().Execute("ovs-vsctl br-get-external-id br-rail1 rail_uplink").Return("p0", nil)
128128
// Second IP flow fails
@@ -138,7 +138,7 @@ var _ = Describe("Flows", func() {
138138
execMock.EXPECT().Execute(matchSubstring("ovs-ofctl add-flow br-rail1")).Return("", nil)
139139
netlinkMock.EXPECT().LinkByName("br-rail1").Return(mockLink, nil)
140140
execMock.EXPECT().Execute("ovs-vsctl br-get-external-id br-rail1 rail_peer_ip").Return("192.168.1.1", nil)
141-
execMock.EXPECT().ExecutePrivileged("arping 192.168.1.1 -c 1 &> /dev/null").Return("", nil)
141+
execMock.EXPECT().ExecutePrivileged("ping 192.168.1.1 -c 1").Return("", nil)
142142
execMock.EXPECT().ExecutePrivileged("ip neighbor | grep 192.168.1.1 | awk '{print $5}'").Return("00:11:22:33:44:55", nil)
143143
execMock.EXPECT().Execute("ovs-vsctl br-get-external-id br-rail1 rail_uplink").Return("p0", nil)
144144
// Second IP flow succeeds
@@ -189,7 +189,7 @@ var _ = Describe("Flows", func() {
189189
// Then it checks for TOR IP
190190
execMock.EXPECT().Execute("ovs-vsctl br-get-external-id br-rail1 rail_peer_ip").Return("192.168.1.1", nil)
191191
// Then it gets TOR MAC
192-
execMock.EXPECT().ExecutePrivileged("arping 192.168.1.1 -c 1 &> /dev/null").Return("", nil)
192+
execMock.EXPECT().ExecutePrivileged("ping 192.168.1.1 -c 1").Return("", nil)
193193
execMock.EXPECT().ExecutePrivileged("ip neighbor | grep 192.168.1.1 | awk '{print $5}'").Return("00:11:22:33:44:55", nil)
194194
// Finally it checks for uplink
195195
execMock.EXPECT().Execute("ovs-vsctl br-get-external-id br-rail1 rail_uplink").Return("", nil)
@@ -199,27 +199,40 @@ var _ = Describe("Flows", func() {
199199
Expect(err.Error()).Should(ContainSubstring("uplink is empty"))
200200
})
201201

202+
It("should return error if fails to ping", func() {
203+
// First ARP flow succeeds
204+
execMock.EXPECT().Execute(matchSubstring("ovs-ofctl add-flow br-rail1")).Return("", nil)
205+
netlinkMock.EXPECT().LinkByName("br-rail1").Return(mockLink, nil)
206+
execMock.EXPECT().Execute("ovs-vsctl br-get-external-id br-rail1 rail_peer_ip").Return("192.168.1.1", nil)
207+
// TOR MAC retrieval fails
208+
execMock.EXPECT().ExecutePrivileged("ping 192.168.1.1 -c 1").Return("", fmt.Errorf("failed to get TOR MAC"))
209+
210+
err := flows.AddPodRailFlows(0x5, "vf0", "br-rail1", "10.0.0.1", "00:11:22:33:44:66")
211+
Expect(err).Should(HaveOccurred())
212+
Expect(err.Error()).Should(ContainSubstring("failed to get tor mac for bridge"))
213+
})
214+
202215
It("should return error if fails to get TOR MAC", func() {
203216
// First ARP flow succeeds
204217
execMock.EXPECT().Execute(matchSubstring("ovs-ofctl add-flow br-rail1")).Return("", nil)
205218
netlinkMock.EXPECT().LinkByName("br-rail1").Return(mockLink, nil)
206219
execMock.EXPECT().Execute("ovs-vsctl br-get-external-id br-rail1 rail_peer_ip").Return("192.168.1.1", nil)
207220
// TOR MAC retrieval fails
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"))
221+
execMock.EXPECT().ExecutePrivileged("ping 192.168.1.1 -c 1").Return("", nil)
222+
execMock.EXPECT().ExecutePrivileged("ip neighbor | grep 192.168.1.1 | awk '{print $5}'").Return("", fmt.Errorf("error"))
210223

211224
err := flows.AddPodRailFlows(0x5, "vf0", "br-rail1", "10.0.0.1", "00:11:22:33:44:66")
212225
Expect(err).Should(HaveOccurred())
213226
Expect(err.Error()).Should(ContainSubstring("failed to get tor mac for bridge"))
214227
})
215228

216-
It("should return error if fails to get TOR MAC from arping", func() {
229+
It("should return error if fails to get TOR MAC from ping", func() {
217230
// First ARP flow succeeds
218231
execMock.EXPECT().Execute(matchSubstring("ovs-ofctl add-flow br-rail1")).Return("", nil)
219232
netlinkMock.EXPECT().LinkByName("br-rail1").Return(mockLink, nil)
220233
execMock.EXPECT().Execute("ovs-vsctl br-get-external-id br-rail1 rail_peer_ip").Return("192.168.1.1", nil)
221234
// 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"))
235+
execMock.EXPECT().ExecutePrivileged("ping 192.168.1.1 -c 1").Return("", nil)
223236
execMock.EXPECT().ExecutePrivileged("ip neighbor | grep 192.168.1.1 | awk '{print $5}'").Return("", nil)
224237

225238
err := flows.AddPodRailFlows(0x5, "vf0", "br-rail1", "10.0.0.1", "00:11:22:33:44:66")
@@ -233,7 +246,7 @@ var _ = Describe("Flows", func() {
233246
netlinkMock.EXPECT().LinkByName("br-rail1").Return(mockLink, nil)
234247
execMock.EXPECT().Execute("ovs-vsctl br-get-external-id br-rail1 rail_peer_ip").Return("192.168.1.1", nil)
235248
// TOR MAC retrieval succeeds
236-
execMock.EXPECT().ExecutePrivileged("arping 192.168.1.1 -c 1 &> /dev/null").Return("", nil)
249+
execMock.EXPECT().ExecutePrivileged("ping 192.168.1.1 -c 1").Return("", nil)
237250
execMock.EXPECT().ExecutePrivileged(gomock.Any()).Return("00:11:22:33:44:55", nil)
238251
// Uplink retrieval fails
239252
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)