Merge pull request #848 from Mirantis/ivan4th/fix-mtu

Fix tap MTU setting

pkg/nettools/calico_test.go

@@ -124,7 +124,7 @@ func TestCalicoDetection(t *testing.T) {
  },
  } {
  t.Run(tc.name, func(t *testing.T) {
- withFakeCNIVeth(t, func(hostNS, contNS ns.NetNS, origHostVeth, origContVeth netlink.Link) {
+ withFakeCNIVeth(t, defaultMTU, func(hostNS, contNS ns.NetNS, origHostVeth, origContVeth netlink.Link) {
  for _, r := range tc.routes {
  if r.Scope == SCOPE_LINK {
  r.LinkIndex = origContVeth.Attrs().Index
@@ -148,7 +148,7 @@ func TestCalicoDetection(t *testing.T) {
 
 func TestFixCalicoNetworking(t *testing.T) {
  withDummyNetworkNamespace(t, func(dummyNS ns.NetNS, dummyInfo *cnicurrent.Result) {
- withFakeCNIVeth(t, func(hostNS, contNS ns.NetNS, origHostVeth, origContVeth netlink.Link) {
+ withFakeCNIVeth(t, defaultMTU, func(hostNS, contNS ns.NetNS, origHostVeth, origContVeth netlink.Link) {
  addCalicoRoutes(t, origContVeth)
  info, err := ExtractLinkInfo(origContVeth, contNS.Path())
  if err != nil {

pkg/nettools/nettools.go

@@ -55,6 +55,7 @@ import (
 )
 
 const (
+ defaultMTU = 1500
  tapInterfaceNameTemplate = "tap%d"
  containerBridgeNameTemplate = "br%d"
  loopbackInterfaceName = "lo"

pkg/nettools/nettools_test.go

@@ -259,9 +259,9 @@ func setupLink(hwAddrAsText string, link netlink.Link) netlink.Link {
  return link
 }
 
-func withFakeCNIVeth(t *testing.T, toRun func(hostNS, contNS ns.NetNS, origHostVeth, origContVeth netlink.Link)) {
+func withFakeCNIVeth(t *testing.T, mtu int, toRun func(hostNS, contNS ns.NetNS, origHostVeth, origContVeth netlink.Link)) {
  withHostAndContNS(t, func(hostNS, contNS ns.NetNS) {
- origHostVeth, origContVeth, err := CreateEscapeVethPair(contNS, "eth0", 1500)
+ origHostVeth, origContVeth, err := CreateEscapeVethPair(contNS, "eth0", mtu)
  if err != nil {
  log.Panicf("failed to create veth pair: %v", err)
  }
@@ -282,8 +282,8 @@ func withFakeCNIVeth(t *testing.T, toRun func(hostNS, contNS ns.NetNS, origHostV
  })
 }
 
-func withFakeCNIVethAndGateway(t *testing.T, toRun func(hostNS, contNS ns.NetNS, origHostVeth, origContVeth netlink.Link)) {
- withFakeCNIVeth(t, func(hostNS, contNS ns.NetNS, origHostVeth, origContVeth netlink.Link) {
+func withFakeCNIVethAndGateway(t *testing.T, mtu int, toRun func(hostNS, contNS ns.NetNS, origHostVeth, origContVeth netlink.Link)) {
+ withFakeCNIVeth(t, mtu, func(hostNS, contNS ns.NetNS, origHostVeth, origContVeth netlink.Link) {
  addTestRoute(t, &netlink.Route{
  Gw: parseAddr("10.1.90.1/24").IPNet.IP,
  Scope: SCOPE_UNIVERSE,
@@ -308,7 +308,7 @@ func verifyNoAddressAndRoutes(t *testing.T, link netlink.Link) {
 }
 
 func TestFindVeth(t *testing.T) {
- withFakeCNIVethAndGateway(t, func(hostNS, contNS ns.NetNS, origHostVeth, origContVeth netlink.Link) {
+ withFakeCNIVethAndGateway(t, defaultMTU, func(hostNS, contNS ns.NetNS, origHostVeth, origContVeth netlink.Link) {
  allLinks, err := netlink.LinkList()
  if err != nil {
  log.Panicf("LinkList() failed: %v", err)
@@ -325,7 +325,7 @@ func TestFindVeth(t *testing.T) {
 }
 
 func TestStripLink(t *testing.T) {
- withFakeCNIVethAndGateway(t, func(hostNS, contNS ns.NetNS, origHostVeth, origContVeth netlink.Link) {
+ withFakeCNIVethAndGateway(t, defaultMTU, func(hostNS, contNS ns.NetNS, origHostVeth, origContVeth netlink.Link) {
  if err := StripLink(origContVeth); err != nil {
  log.Panicf("StripLink() failed: %v", err)
  }
@@ -334,7 +334,7 @@ func TestStripLink(t *testing.T) {
 }
 
 func TestExtractLinkInfo(t *testing.T) {
- withFakeCNIVethAndGateway(t, func(hostNS, contNS ns.NetNS, origHostVeth, origContVeth netlink.Link) {
+ withFakeCNIVethAndGateway(t, defaultMTU, func(hostNS, contNS ns.NetNS, origHostVeth, origContVeth netlink.Link) {
  info, err := ExtractLinkInfo(origContVeth, contNS.Path())
  if err != nil {
  log.Panicf("failed to grab interface info: %v", err)
@@ -347,7 +347,7 @@ func TestExtractLinkInfo(t *testing.T) {
  })
 }
 
-func verifyContainerSideNetwork(t *testing.T, origContVeth netlink.Link, contNsPath string, hostNS ns.NetNS) {
+func verifyContainerSideNetwork(t *testing.T, origContVeth netlink.Link, contNsPath string, hostNS ns.NetNS, mtu int) {
  allLinks, err := netlink.LinkList()
  if err != nil {
  log.Panicf("error listing links: %v", err)
@@ -375,6 +375,9 @@ func verifyContainerSideNetwork(t *testing.T, origContVeth netlink.Link, contNsP
  if reflect.DeepEqual(origContVeth.Attrs().HardwareAddr, origHwAddr) {
  t.Errorf("cni veth hardware address didn't change")
  }
+ if origContVeth.Attrs().MTU != mtu {
+ t.Errorf("bad veth MTU: %d instead of %d", origContVeth.Attrs().MTU, mtu)
+ }
 
  verifyNoAddressAndRoutes(t, origContVeth)
 
@@ -384,6 +387,9 @@ func verifyContainerSideNetwork(t *testing.T, origContVeth netlink.Link, contNsP
  if tap.Type() != "tun" {
  t.Errorf("tap0 interface must have type tun, but has %q instead", tap.Type())
  }
+ if tap.Attrs().MTU != mtu {
+ t.Errorf("bad tap MTU: %d instead of %d", tap.Attrs().MTU, mtu)
+ }
 
  addrs, err := netlink.AddrList(bridge, FAMILY_V4)
  if err != nil {
@@ -395,20 +401,33 @@ func verifyContainerSideNetwork(t *testing.T, origContVeth netlink.Link, contNsP
  } else if addrs[0].String() != expectedAddr {
  t.Errorf("bad br0 address %q (expected %q)", addrs[0].String(), expectedAddr)
  }
+
+ if bridge.Attrs().MTU != mtu {
+ t.Errorf("bad bridge MTU: %d instead of %d", bridge.Attrs().MTU, mtu)
+ }
 }
 
 func TestSetUpContainerSideNetworkWithInfo(t *testing.T) {
- withFakeCNIVethAndGateway(t, func(hostNS, contNS ns.NetNS, origHostVeth, origContVeth netlink.Link) {
+ withFakeCNIVethAndGateway(t, defaultMTU, func(hostNS, contNS ns.NetNS, origHostVeth, origContVeth netlink.Link) {
+ if err := StripLink(origContVeth); err != nil {
+ log.Panicf("StripLink() failed: %v", err)
+ }
+ verifyContainerSideNetwork(t, origContVeth, contNS.Path(), hostNS, defaultMTU)
+ })
+}
+
+func TestSetUpContainerSideNetworkMTU(t *testing.T) {
+ withFakeCNIVethAndGateway(t, 9000, func(hostNS, contNS ns.NetNS, origHostVeth, origContVeth netlink.Link) {
  if err := StripLink(origContVeth); err != nil {
  log.Panicf("StripLink() failed: %v", err)
  }
- verifyContainerSideNetwork(t, origContVeth, contNS.Path(), hostNS)
+ verifyContainerSideNetwork(t, origContVeth, contNS.Path(), hostNS, 9000)
  })
 }
 
 func TestLoopbackInterface(t *testing.T) {
- withFakeCNIVethAndGateway(t, func(hostNS, contNS ns.NetNS, origHostVeth, origContVeth netlink.Link) {
- verifyContainerSideNetwork(t, origContVeth, contNS.Path(), hostNS)
+ withFakeCNIVethAndGateway(t, defaultMTU, func(hostNS, contNS ns.NetNS, origHostVeth, origContVeth netlink.Link) {
+ verifyContainerSideNetwork(t, origContVeth, contNS.Path(), hostNS, defaultMTU)
  if out, err := exec.Command("ping", "-c", "1", "127.0.0.1").CombinedOutput(); err != nil {
  log.Panicf("ping 127.0.0.1 failed:\n%s", out)
  }
@@ -483,7 +502,7 @@ func verifyVethHaveConfiguration(t *testing.T, info *cnicurrent.Result) {
 }
 
 func TestTeardownContainerSideNetwork(t *testing.T) {
- withFakeCNIVethAndGateway(t, func(hostNS, contNS ns.NetNS, origHostVeth, origContVeth netlink.Link) {
+ withFakeCNIVethAndGateway(t, defaultMTU, func(hostNS, contNS ns.NetNS, origHostVeth, origContVeth netlink.Link) {
  if err := StripLink(origContVeth); err != nil {
  log.Panicf("StripLink() failed: %v", err)
  }
@@ -516,7 +535,7 @@ func TestTeardownContainerSideNetwork(t *testing.T) {
 }
 
 func TestFindingLinkByAddress(t *testing.T) {
- withFakeCNIVeth(t, func(hostNS, contNS ns.NetNS, origHostVeth, origContVeth netlink.Link) {
+ withFakeCNIVeth(t, defaultMTU, func(hostNS, contNS ns.NetNS, origHostVeth, origContVeth netlink.Link) {
  expectedInfo := expectedExtractedLinkInfo(contNS.Path())
  allLinks, err := netlink.LinkList()
  if err != nil {

pkg/nettools/tap_linux.go

@@ -83,5 +83,10 @@ func CreateTAP(devName string, mtu int) (netlink.Link, error) {
  return nil, fmt.Errorf("failed to set %q up: %v", devName, err)
  }
 
+ // NOTE: link mtu in LinkAttrs above is actually ignored
+ if err := netlink.LinkSetMTU(tap, mtu); err != nil {
+ return nil, fmt.Errorf("LinkSetMTU(): %v", err)
+ }
+
  return tap, nil
 }

tests/network/fake_cni_test.go

@@ -31,6 +31,10 @@ import (
  "github.com/Mirantis/virtlet/pkg/utils"
 )
 
+const (
+ defaultMTU = 1500
+)
+
 // FakeCNIVethPair represents a veth pair created by the fake CNI
 type FakeCNIVethPair struct {
  HostSide netlink.Link
@@ -46,6 +50,7 @@ type fakeCNIEntry struct {
  added bool
  removed bool
  useBadResult bool
+ mtu int
 }
 
 func (e *fakeCNIEntry) addSandboxToNetwork(ifaceIndex int) error {
@@ -61,7 +66,7 @@ func (e *fakeCNIEntry) addSandboxToNetwork(ifaceIndex int) error {
  var vp FakeCNIVethPair
  if err := e.hostNS.Do(func(ns.NetNS) error {
  var err error
- vp.HostSide, vp.ContSide, err = nettools.CreateEscapeVethPair(e.contNS, iface.Name, 1500)
+ vp.HostSide, vp.ContSide, err = nettools.CreateEscapeVethPair(e.contNS, iface.Name, e.mtu)
  return err
  }); err != nil {
  return fmt.Errorf("failed to create escape veth pair: %v", err)
@@ -161,19 +166,23 @@ func NewFakeCNIClient() *FakeCNIClient {
  }
 }
 
-func (c *FakeCNIClient) ExpectPod(podId, podName, podNS string, info *cnicurrent.Result, hostNS ns.NetNS, extraRoutes map[int][]netlink.Route) {
+func (c *FakeCNIClient) ExpectPod(podId, podName, podNS string, info *cnicurrent.Result, hostNS ns.NetNS, extraRoutes map[int][]netlink.Route, mtu int) {
+ if mtu == 0 {
+ mtu = defaultMTU
+ }
  c.entries[podKey(podId, podName, podNS)] = &fakeCNIEntry{
  podId: podId,
  podName: podName,
  podNS: podNS,
  info: info,
  hostNS: hostNS,
  extraRoutes: extraRoutes,
+ mtu: mtu,
  }
 }
 
 func (c *FakeCNIClient) ExpectDummyPod(info *cnicurrent.Result, hostNS ns.NetNS, extraRoutes map[int][]netlink.Route) {
- c.ExpectPod(c.DummyPodId, "", "", info, hostNS, extraRoutes)
+ c.ExpectPod(c.DummyPodId, "", "", info, hostNS, extraRoutes, 0)
 }
 
 func (c *FakeCNIClient) GetDummyNetwork() (*cnicurrent.Result, string, error) {

tests/network/utils_test.go

@@ -503,6 +503,60 @@ func addAddress(t *testing.T, netNS ns.NetNS, link netlink.Link, addr string) {
  }
 }
 
+func getBridgeFromMember(t *testing.T, netNS ns.NetNS, link netlink.Link) netlink.Link {
+ var bridge netlink.Link
+ if err := netNS.Do(func(ns.NetNS) (err error) {
+ link, err = netlink.LinkByName(link.Attrs().Name)
+ if err != nil {
+ return fmt.Errorf("LinkByName(): %v", err)
+ }
+ masterIndex := link.Attrs().MasterIndex
+ if masterIndex == 0 {
+ return fmt.Errorf("link %q doesn't have master", link.Attrs().Name)
+ }
+ bridge, err = netlink.LinkByIndex(masterIndex)
+ if err != nil {
+ return fmt.Errorf("LinkByIndex(): %v", err)
+ }
+ return nil
+ }); err != nil {
+ t.Fatalf("getBridgeFromMember(): %v", err)
+ }
+ return bridge
+}
+
+func verifyMTU(t *testing.T, netNS ns.NetNS, link netlink.Link, expectedMTU int) {
+ if link.Type() != "bridge" {
+ link = getBridgeFromMember(t, netNS, link)
+ }
+ if err := netNS.Do(func(ns.NetNS) (err error) {
+ link, err = netlink.LinkByName(link.Attrs().Name)
+ if err != nil {
+ return fmt.Errorf("LinkByName(): %v", err)
+ }
+ if link.Type() != "bridge" {
+ return fmt.Errorf("doVerifyMTU(): %q: bridge expected", link.Attrs().Name)
+ }
+ index := link.Attrs().Index
+ links, err := netlink.LinkList()
+ if err != nil {
+ return fmt.Errorf("LinkList(): %v", err)
+ }
+ for _, link := range links {
+ attrs := link.Attrs()
+ if attrs.MasterIndex != index {
+ continue
+ }
+ if attrs.MTU != expectedMTU {
+ t.Errorf("bad MTU for link %q: %d instead of %d", attrs.Name, attrs.MTU, expectedMTU)
+ }
+ }
+ return nil
+ }); err != nil {
+ t.Fatalf("verifying mtu failed: %v", err)
+ }
+}
+
 // tapConnector copies frames between tap interfaces. It returns
 // a channel that should be closed to stop copying and close
 // the tap devices

tests/network/vm_network_test.go

@@ -104,12 +104,12 @@ func sampleCNIResult() *cnicurrent.Result {
 }
 
 type vmNetworkTester struct {
- t  *testing.T
- linkCount  int
- hostNS, contNS, clientNS ns.NetNS
- clientTapLinks  []netlink.Link
- dhcpClientTaps  []*os.File
- g  *NetTestGroup
+ t *testing.T
+ linkCount int
+ hostNS, clientNS ns.NetNS
+ clientTapLinks []netlink.Link
+ dhcpClientTaps []*os.File
+ g *NetTestGroup
 }
 
 func newVMNetworkTester(t *testing.T, linkCount int) *vmNetworkTester {
@@ -291,9 +291,9 @@ type tapFDSourceTester struct {
  c *tapmanager.FDClient
 }
 
-func newTapFDSourceTester(t *testing.T, podId string, info *cnicurrent.Result, hostNS ns.NetNS, extraRoutes map[int][]netlink.Route) *tapFDSourceTester {
+func newTapFDSourceTester(t *testing.T, podId string, info *cnicurrent.Result, hostNS ns.NetNS, extraRoutes map[int][]netlink.Route, mtu int) *tapFDSourceTester {
  cniClient := NewFakeCNIClient()
- cniClient.ExpectPod(podId, samplePodName, samplePodNS, info, hostNS, extraRoutes)
+ cniClient.ExpectPod(podId, samplePodName, samplePodNS, info, hostNS, extraRoutes, mtu)
 
  tmpDir, err := ioutil.TempDir("", "pass-fd-test")
  if err != nil {
@@ -415,6 +415,8 @@ func TestTapFDSource(t *testing.T) {
  outerAddrs []string
  // clientAddrs specifies per-interface VM IPs to ping
  clientAddrs []string
+ // mtu specifies MTU for the host interface
+ mtu int
  }{
  {
  name: "single cni",
@@ -428,6 +430,7 @@ func TestTapFDSource(t *testing.T) {
  "new_network_number='10.1.90.0'",
  "new_subnet_mask='255.255.255.0'",
  "tap0: offered 10.1.90.5 from 169.254.254.2",
+ "new_interface_mtu='1500'",
  },
  },
  interfaceDesc: []tapmanager.InterfaceDescription{
@@ -441,6 +444,33 @@ func TestTapFDSource(t *testing.T) {
  outerAddrs: outerAddrs,
  clientAddrs: clientAddrs,
  },
+ {
+ name: "single cni with MTU",
+ interfaceCount: 1,
+ info: sampleCNIResult(),
+ tcpdumpStopOn: "10.1.90.1.4243 > 10.1.90.5.4242: UDP",
+ dhcpExpectedSubstrings: [][]string{
+ {
+ "new_classless_static_routes='10.10.42.0/24 10.1.90.90 0.0.0.0/0 10.1.90.1'",
+ "new_ip_address='10.1.90.5'",
+ "new_network_number='10.1.90.0'",
+ "new_subnet_mask='255.255.255.0'",
+ "tap0: offered 10.1.90.5 from 169.254.254.2",
+ "new_interface_mtu='9000'",
+ },
+ },
+ interfaceDesc: []tapmanager.InterfaceDescription{
+ {
+ Type: network.InterfaceTypeTap,
+ HardwareAddr: mustParseMAC(clientMacAddrs[0]),
+ FdIndex: 0,
+ PCIAddress: "",
+ },
+ },
+ outerAddrs: outerAddrs,
+ clientAddrs: clientAddrs,
+ mtu: 9000,
+ },
  {
  name: "multiple cnis",
  interfaceCount: 2,
@@ -949,7 +979,7 @@ func TestTapFDSource(t *testing.T) {
  vnt := newVMNetworkTester(t, tc.interfaceCount)
  defer vnt.teardown()
 
- tst := newTapFDSourceTester(t, podId, tc.info, vnt.hostNS, tc.extraRoutes)
+ tst := newTapFDSourceTester(t, podId, tc.info, vnt.hostNS, tc.extraRoutes, tc.mtu)
  defer tst.teardown()
  c := tst.setupServerAndConnectToFDServer()
  if tc.dummyInfo != nil {
@@ -1048,7 +1078,15 @@ func TestTapFDSource(t *testing.T) {
  verifyNoDiff(t, "interfaceDesc", tc.interfaceDesc, interfaceDesc)
  }
 
+ contNS, err := ns.GetNS(csn.NsPath)
+ if err != nil {
+ t.Fatalf("GetNS(): %v", err)
+ }
+
  for n, veth := range veths {
+ if tc.mtu != 0 {
+ verifyMTU(t, contNS, veth.ContSide, tc.mtu)
+ }
  addAddress(t, vnt.hostNS, veth.HostSide, tc.outerAddrs[n])
  }
 
@@ -1079,3 +1117,4 @@ func TestTapFDSource(t *testing.T) {
 
 // TODO: test DNS handling
 // TODO: test SR-IOV (by making a fake sysfs dir)
+// TODO: mtu option over DHCP