Skip to content

Commit b2bbd8d

Browse files
committed
Avoid handling mac duplication
The bond-cni should not attempt to modify the mac addresses of slaves during both, creation and deletion of the bond interface. During deletion, the bond driver takes care of reverting any modifications. Only balance-tlb and balance-alb do not support slaves with same mac addresses. The plugin now returns an error if this is the case for these modes. Signed-off-by: Marcelo Guerrero <[email protected]>
1 parent d996cc5 commit b2bbd8d

File tree

3 files changed

+4
-65
lines changed

3 files changed

+4
-65
lines changed

bond/bond.go

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,10 @@ import (
2929
"github.com/containernetworking/cni/pkg/version"
3030
"github.com/containernetworking/plugins/pkg/ipam"
3131
"github.com/containernetworking/plugins/pkg/ns"
32-
"github.com/intel/bond-cni/bond/util"
3332
"github.com/vishvananda/netlink"
3433
"github.com/vishvananda/netns"
34+
35+
"github.com/intel/bond-cni/bond/util"
3536
)
3637

3738
type bondingConfig struct {
@@ -283,6 +284,7 @@ func createBond(bondName string, bondConf *bondingConfig, nspath string, ns ns.N
283284
if bondConf.FailOverMac < 0 || bondConf.FailOverMac > 2 {
284285
return nil, fmt.Errorf("FailOverMac mode should be 0, 1 or 2 actual: %+v", bondConf.FailOverMac)
285286
}
287+
286288
bondLinkObj, err := createBondedLink(bondName, bondConf, netNsHandle)
287289
if err != nil {
288290
return nil, fmt.Errorf("failed to create bonded link (%+v), error: %+v", bondName, err)
@@ -433,19 +435,6 @@ func cmdDel(args *skel.CmdArgs) error {
433435
return fmt.Errorf("failed to deattached links from bond, error: %+v", err)
434436
}
435437

436-
// Fetch slave links again to have the latest state
437-
// For instance, Active-backup mode with fail_over_mac=0 reverts the mac address of backup slaves
438-
for i := range linkObjectsToDeattach {
439-
linkObjectsToDeattach[i], err = netNsHandle.LinkByName(linkObjectsToDeattach[i].Attrs().Name)
440-
if err != nil {
441-
return fmt.Errorf("failed to find link (%+v), error: %+v", linkObjectsToDeattach[i].Attrs().Name, err)
442-
}
443-
}
444-
445-
if err = util.HandleMacDuplicates(linkObjectsToDeattach, netNsHandle); err != nil {
446-
return fmt.Errorf("failed to validate deattached links macs, error: %+v", err)
447-
}
448-
449438
err = netNsHandle.LinkDel(linkObjToDel)
450439
if err != nil {
451440
return fmt.Errorf("failed to delete bonded link (%+v), error: %+v", linkObjToDel.Attrs().Name, err)

bond/bond_test.go

Lines changed: 0 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -228,57 +228,6 @@ var _ = Describe("bond plugin", func() {
228228
Expect(err).NotTo(HaveOccurred())
229229
})
230230

231-
It("verifies the plugin handles duplicated macs on delete", func() {
232-
var slave1, slave2 netlink.Link
233-
var err error
234-
235-
err = podNS.Do(func(ns.NetNS) error {
236-
defer GinkgoRecover()
237-
slave1, err = netlink.LinkByName(Slave1)
238-
Expect(err).NotTo(HaveOccurred())
239-
240-
slave2, err = netlink.LinkByName(Slave2)
241-
Expect(err).NotTo(HaveOccurred())
242-
243-
err = netlink.LinkSetHardwareAddr(slave2, slave1.Attrs().HardwareAddr)
244-
Expect(err).NotTo(HaveOccurred())
245-
return nil
246-
})
247-
248-
By("creating the plugin")
249-
r, _, err := testutils.CmdAddWithArgs(args, func() error {
250-
return cmdAdd(args)
251-
})
252-
Expect(err).NotTo(HaveOccurred())
253-
254-
By("checking the bond was created")
255-
checkAddReturnResult(&r, IfName)
256-
257-
err = podNS.Do(func(ns.NetNS) error {
258-
defer GinkgoRecover()
259-
By("duplicating the macs on the slaves")
260-
err = netlink.LinkSetHardwareAddr(slave2, slave1.Attrs().HardwareAddr)
261-
Expect(err).NotTo(HaveOccurred())
262-
return nil
263-
})
264-
By("deleting the plugin")
265-
err = testutils.CmdDel(podNS.Path(),
266-
args.ContainerID, "", func() error { return cmdDel(args) })
267-
Expect(err).NotTo(HaveOccurred())
268-
269-
err = podNS.Do(func(ns.NetNS) error {
270-
defer GinkgoRecover()
271-
By("validating the macs are not duplicated")
272-
slave1, err = netlink.LinkByName(Slave1)
273-
Expect(err).NotTo(HaveOccurred())
274-
slave2, err = netlink.LinkByName(Slave2)
275-
Expect(err).NotTo(HaveOccurred())
276-
Expect(slave1.Attrs().HardwareAddr.String()).NotTo(Equal(slave2.Attrs().HardwareAddr.String()))
277-
return nil
278-
})
279-
Expect(err).NotTo(HaveOccurred())
280-
})
281-
282231
It("verifies that mac addresses are restored correctly in active-backup with fail_over_mac 0", func() {
283232
var bond netlink.Link
284233
var slave1 netlink.Link

bond/util/validation.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"bytes"
55
"crypto/rand"
66
"fmt"
7+
78
"github.com/vishvananda/netlink"
89
)
910

0 commit comments

Comments
 (0)