From bee8d6cf302d0f962fa2bc6274eb154139de6926 Mon Sep 17 00:00:00 2001 From: Bruce Ma Date: Fri, 25 Oct 2019 19:59:33 +0800 Subject: [PATCH 1/2] vlan: add MTU validation in loadNetConf Signed-off-by: Bruce Ma --- plugins/main/vlan/vlan.go | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/plugins/main/vlan/vlan.go b/plugins/main/vlan/vlan.go index 355d261f3..8f4240697 100644 --- a/plugins/main/vlan/vlan.go +++ b/plugins/main/vlan/vlan.go @@ -58,9 +58,27 @@ func loadConf(bytes []byte) (*NetConf, string, error) { if n.VlanId < 0 || n.VlanId > 4094 { return nil, "", fmt.Errorf("invalid VLAN ID %d (must be between 0 and 4095 inclusive)", n.VlanId) } + + // check existing and MTU of master interface + masterMTU, err := getMTUByName(n.Master) + if err != nil { + return nil, "", err + } + if n.MTU < 0 || n.MTU > masterMTU { + return nil, "", fmt.Errorf("invalid MTU %d, must be [0, master MTU(%d)]", n.MTU, masterMTU) + } + return n, n.CNIVersion, nil } +func getMTUByName(ifName string) (int, error) { + link, err := netlink.LinkByName(ifName) + if err != nil { + return 0, err + } + return link.Attrs().MTU, nil +} + func createVlan(conf *NetConf, ifName string, netns ns.NetNS) (*current.Interface, error) { vlan := ¤t.Interface{} @@ -76,10 +94,6 @@ func createVlan(conf *NetConf, ifName string, netns ns.NetNS) (*current.Interfac return nil, err } - if conf.MTU <= 0 { - conf.MTU = m.Attrs().MTU - } - v := &netlink.Vlan{ LinkAttrs: netlink.LinkAttrs{ MTU: conf.MTU, From 1a30688da02733aea171eeec4d2fab57b3b8a4ec Mon Sep 17 00:00:00 2001 From: Bruce Ma Date: Fri, 25 Oct 2019 20:00:23 +0800 Subject: [PATCH 2/2] add some testcases about invalid MTUs Signed-off-by: Bruce Ma --- plugins/main/vlan/vlan_test.go | 71 ++++++++++++++++++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/plugins/main/vlan/vlan_test.go b/plugins/main/vlan/vlan_test.go index cdff984f0..17e4f39df 100644 --- a/plugins/main/vlan/vlan_test.go +++ b/plugins/main/vlan/vlan_test.go @@ -408,4 +408,75 @@ var _ = Describe("vlan Operations", func() { }) Expect(err).NotTo(HaveOccurred()) }) + + Describe("fails to create vlan link with invalid MTU", func() { + conf := `{ + "cniVersion": "0.3.1", + "name": "mynet", + "type": "vlan", + "master": "%s", + "mtu": %d, + "ipam": { + "type": "host-local", + "subnet": "10.1.2.0/24" + } +}` + BeforeEach(func() { + var err error + err = originalNS.Do(func(ns.NetNS) error { + defer GinkgoRecover() + + // set master link's MTU to 1500 + link, err := netlink.LinkByName(MASTER_NAME) + Expect(err).NotTo(HaveOccurred()) + err = netlink.LinkSetMTU(link, 1500) + Expect(err).NotTo(HaveOccurred()) + + return nil + }) + Expect(err).NotTo(HaveOccurred()) + }) + + It("fails to create vlan link with greater MTU than master interface", func() { + var err error + + args := &skel.CmdArgs{ + ContainerID: "dummy", + Netns: "/var/run/netns/test", + IfName: "eth0", + StdinData: []byte(fmt.Sprintf(conf, MASTER_NAME, 1600)), + } + + _ = originalNS.Do(func(netNS ns.NetNS) error { + defer GinkgoRecover() + + _, _, err = testutils.CmdAddWithArgs(args, func() error { + return cmdAdd(args) + }) + Expect(err).To(Equal(fmt.Errorf("invalid MTU 1600, must be [0, master MTU(1500)]"))) + return nil + }) + }) + + It("fails to create vlan link with negative MTU", func() { + var err error + + args := &skel.CmdArgs{ + ContainerID: "dummy", + Netns: "/var/run/netns/test", + IfName: "eth0", + StdinData: []byte(fmt.Sprintf(conf, MASTER_NAME, -100)), + } + + _ = originalNS.Do(func(netNS ns.NetNS) error { + defer GinkgoRecover() + + _, _, err = testutils.CmdAddWithArgs(args, func() error { + return cmdAdd(args) + }) + Expect(err).To(Equal(fmt.Errorf("invalid MTU -100, must be [0, master MTU(1500)]"))) + return nil + }) + }) + }) })