Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bridge: Add mac field to specify container iface mac #636

Merged
merged 1 commit into from
Jun 30, 2021
Merged

bridge: Add mac field to specify container iface mac #636

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 14 additions & 7 deletions pkg/ip/link_linux.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ var (
ErrLinkNotFound = errors.New("link not found")
)

func makeVethPair(name, peer string, mtu int) (netlink.Link, error) {
func makeVethPair(name, peer string, mtu int, mac string) (netlink.Link, error) {
veth := &netlink.Veth{
LinkAttrs: netlink.LinkAttrs{
Name: name,
Expand All @@ -42,6 +42,13 @@ func makeVethPair(name, peer string, mtu int) (netlink.Link, error) {
},
PeerName: peer,
}
if mac != "" {
m, err := net.ParseMAC(mac)
if err != nil {
return nil, err
}
veth.LinkAttrs.HardwareAddr = m
}
if err := netlink.LinkAdd(veth); err != nil {
return nil, err
}
Expand All @@ -62,7 +69,7 @@ func peerExists(name string) bool {
return true
}

func makeVeth(name, vethPeerName string, mtu int) (peerName string, veth netlink.Link, err error) {
func makeVeth(name, vethPeerName string, mtu int, mac string) (peerName string, veth netlink.Link, err error) {
for i := 0; i < 10; i++ {
if vethPeerName != "" {
peerName = vethPeerName
Expand All @@ -73,7 +80,7 @@ func makeVeth(name, vethPeerName string, mtu int) (peerName string, veth netlink
}
}

veth, err = makeVethPair(name, peerName, mtu)
veth, err = makeVethPair(name, peerName, mtu, mac)
switch {
case err == nil:
return
Expand Down Expand Up @@ -132,8 +139,8 @@ func ifaceFromNetlinkLink(l netlink.Link) net.Interface {
// devices and move the host-side veth into the provided hostNS namespace.
// hostVethName: If hostVethName is not specified, the host-side veth name will use a random string.
// On success, SetupVethWithName returns (hostVeth, containerVeth, nil)
func SetupVethWithName(contVethName, hostVethName string, mtu int, hostNS ns.NetNS) (net.Interface, net.Interface, error) {
hostVethName, contVeth, err := makeVeth(contVethName, hostVethName, mtu)
func SetupVethWithName(contVethName, hostVethName string, mtu int, contVethMac string, hostNS ns.NetNS) (net.Interface, net.Interface, error) {
hostVethName, contVeth, err := makeVeth(contVethName, hostVethName, mtu, contVethMac)
if err != nil {
return net.Interface{}, net.Interface{}, err
}
Expand Down Expand Up @@ -175,8 +182,8 @@ func SetupVethWithName(contVethName, hostVethName string, mtu int, hostNS ns.Net
// Call SetupVeth from inside the container netns. It will create both veth
// devices and move the host-side veth into the provided hostNS namespace.
// On success, SetupVeth returns (hostVeth, containerVeth, nil)
func SetupVeth(contVethName string, mtu int, hostNS ns.NetNS) (net.Interface, net.Interface, error) {
return SetupVethWithName(contVethName, "", mtu, hostNS)
func SetupVeth(contVethName string, mtu int, contVethMac string, hostNS ns.NetNS) (net.Interface, net.Interface, error) {
return SetupVethWithName(contVethName, "", mtu, contVethMac, hostNS)
}

// DelLinkByName removes an interface link.
Expand Down
34 changes: 30 additions & 4 deletions pkg/ip/link_linux_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ var _ = Describe("Link", func() {
_ = containerNetNS.Do(func(ns.NetNS) error {
defer GinkgoRecover()

hostVeth, containerVeth, err = ip.SetupVeth(fmt.Sprintf(ifaceFormatString, ifaceCounter), mtu, hostNetNS)
hostVeth, containerVeth, err = ip.SetupVeth(fmt.Sprintf(ifaceFormatString, ifaceCounter), mtu, "", hostNetNS)
if err != nil {
return err
}
Expand Down Expand Up @@ -159,7 +159,7 @@ var _ = Describe("Link", func() {
_ = containerNetNS.Do(func(ns.NetNS) error {
defer GinkgoRecover()

_, _, err := ip.SetupVeth(containerVethName, mtu, hostNetNS)
_, _, err := ip.SetupVeth(containerVethName, mtu, "", hostNetNS)
Expect(err.Error()).To(Equal(fmt.Sprintf("container veth name provided (%s) already exists", containerVethName)))

return nil
Expand Down Expand Up @@ -189,7 +189,7 @@ var _ = Describe("Link", func() {
It("returns useful error", func() {
_ = containerNetNS.Do(func(ns.NetNS) error {
defer GinkgoRecover()
_, _, err := ip.SetupVeth(containerVethName, mtu, hostNetNS)
_, _, err := ip.SetupVeth(containerVethName, mtu, "", hostNetNS)
Expect(err.Error()).To(HavePrefix("failed to move veth to host netns: "))

return nil
Expand All @@ -207,7 +207,7 @@ var _ = Describe("Link", func() {
_ = containerNetNS.Do(func(ns.NetNS) error {
defer GinkgoRecover()

hostVeth, _, err := ip.SetupVeth(containerVethName, mtu, hostNetNS)
hostVeth, _, err := ip.SetupVeth(containerVethName, mtu, "", hostNetNS)
Expect(err).NotTo(HaveOccurred())
hostVethName = hostVeth.Name
return nil
Expand All @@ -233,6 +233,32 @@ var _ = Describe("Link", func() {
})
})

It("successfully creates a veth pair with an explicit mac", func() {
const mac = "02:00:00:00:01:23"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why this test case is separated into three steps? and I don't see the necessity of the third step

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I followed the format of the previous test.
I agree that the first two steps can be merged, they execute on the same netns so there is no reason to split them.
Perhaps the original reason for the split was to emphasize that the first step is the setup while the second is the assertion.
Anyway, I merged them.

The 3rd step is validating that the mac address specified is not assigned to the host side of the veth peer. It seemed a reasonable check to me.

_ = containerNetNS.Do(func(ns.NetNS) error {
defer GinkgoRecover()

hostVeth, _, err := ip.SetupVeth(containerVethName, mtu, mac, hostNetNS)
Expect(err).NotTo(HaveOccurred())
hostVethName = hostVeth.Name

link, err := netlink.LinkByName(containerVethName)
Expect(err).NotTo(HaveOccurred())
Expect(link.Attrs().HardwareAddr.String()).To(Equal(mac))

return nil
})

_ = hostNetNS.Do(func(ns.NetNS) error {
defer GinkgoRecover()

link, err := netlink.LinkByName(hostVethName)
Expect(err).NotTo(HaveOccurred())
Expect(link.Attrs().HardwareAddr.String()).NotTo(Equal(mac))

return nil
})
})
})

It("DelLinkByName must delete the veth endpoints", func() {
Expand Down
55 changes: 47 additions & 8 deletions plugins/main/bridge/bridge.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,25 @@ type NetConf struct {
HairpinMode bool `json:"hairpinMode"`
PromiscMode bool `json:"promiscMode"`
Vlan int `json:"vlan"`

Args struct {
Cni BridgeArgs `json:"cni,omitempty"`
} `json:"args,omitempty"`
RuntimeConfig struct {
Mac string `json:"mac,omitempty"`
} `json:"runtimeConfig,omitempty"`

mac string
}

type BridgeArgs struct {
Mac string `json:"mac,omitempty"`
}

// MacEnvArgs represents CNI_ARGS
type MacEnvArgs struct {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't the name be something more abstract, because it might contain more kinds of args in the future?
I mean, maybe it is better to drop Mac from the name?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, this is the type boilerplate to automatically parse CNI_ARGS "MAC=FOO" strings.

types.CommonArgs
MAC types.UnmarshallableString `json:"mac,omitempty"`
}

type gwInfo struct {
Expand All @@ -70,7 +89,7 @@ func init() {
runtime.LockOSThread()
}

func loadNetConf(bytes []byte) (*NetConf, string, error) {
func loadNetConf(bytes []byte, envArgs string) (*NetConf, string, error) {
n := &NetConf{
BrName: defaultBrName,
}
Expand All @@ -80,6 +99,26 @@ func loadNetConf(bytes []byte) (*NetConf, string, error) {
if n.Vlan < 0 || n.Vlan > 4094 {
return nil, "", fmt.Errorf("invalid VLAN ID %d (must be between 0 and 4094)", n.Vlan)
}

if envArgs != "" {
e := MacEnvArgs{}
if err := types.LoadArgs(envArgs, &e); err != nil {
return nil, "", err
}

if e.MAC != "" {
dcbw marked this conversation as resolved.
Show resolved Hide resolved
n.mac = string(e.MAC)
}
}

if mac := n.Args.Cni.Mac; mac != "" {
n.mac = mac
}

if mac := n.RuntimeConfig.Mac; mac != "" {
n.mac = mac
}

return n, n.CNIVersion, nil
}

Expand Down Expand Up @@ -273,7 +312,7 @@ func ensureVlanInterface(br *netlink.Bridge, vlanId int) (netlink.Link, error) {
return nil, fmt.Errorf("faild to find host namespace: %v", err)
}

_, brGatewayIface, err := setupVeth(hostNS, br, name, br.MTU, false, vlanId)
_, brGatewayIface, err := setupVeth(hostNS, br, name, br.MTU, false, vlanId, "")
if err != nil {
return nil, fmt.Errorf("faild to create vlan gateway %q: %v", name, err)
}
Expand All @@ -287,13 +326,13 @@ func ensureVlanInterface(br *netlink.Bridge, vlanId int) (netlink.Link, error) {
return brGatewayVeth, nil
}

func setupVeth(netns ns.NetNS, br *netlink.Bridge, ifName string, mtu int, hairpinMode bool, vlanID int) (*current.Interface, *current.Interface, error) {
func setupVeth(netns ns.NetNS, br *netlink.Bridge, ifName string, mtu int, hairpinMode bool, vlanID int, mac string) (*current.Interface, *current.Interface, error) {
contIface := &current.Interface{}
hostIface := &current.Interface{}

err := netns.Do(func(hostNS ns.NetNS) error {
// create the veth pair in the container and move host end into host netns
hostVeth, containerVeth, err := ip.SetupVeth(ifName, mtu, hostNS)
hostVeth, containerVeth, err := ip.SetupVeth(ifName, mtu, mac, hostNS)
if err != nil {
return err
}
Expand Down Expand Up @@ -380,7 +419,7 @@ func enableIPForward(family int) error {
func cmdAdd(args *skel.CmdArgs) error {
var success bool = false

n, cniVersion, err := loadNetConf(args.StdinData)
n, cniVersion, err := loadNetConf(args.StdinData, args.Args)
if err != nil {
return err
}
Expand All @@ -406,7 +445,7 @@ func cmdAdd(args *skel.CmdArgs) error {
}
defer netns.Close()

hostInterface, containerInterface, err := setupVeth(netns, br, args.IfName, n.MTU, n.HairpinMode, n.Vlan)
hostInterface, containerInterface, err := setupVeth(netns, br, args.IfName, n.MTU, n.HairpinMode, n.Vlan, n.mac)
if err != nil {
return err
}
Expand Down Expand Up @@ -585,7 +624,7 @@ func cmdAdd(args *skel.CmdArgs) error {
}

func cmdDel(args *skel.CmdArgs) error {
n, _, err := loadNetConf(args.StdinData)
n, _, err := loadNetConf(args.StdinData, args.Args)
if err != nil {
return err
}
Expand Down Expand Up @@ -776,7 +815,7 @@ func validateCniContainerInterface(intf current.Interface) (cniBridgeIf, error)

func cmdCheck(args *skel.CmdArgs) error {

n, _, err := loadNetConf(args.StdinData)
n, _, err := loadNetConf(args.StdinData, args.Args)
if err != nil {
return err
}
Expand Down
Loading