From dffc988d9210ffc74b2dd8666dc53e9e88e2799c Mon Sep 17 00:00:00 2001 From: Manohar Castelino Date: Tue, 5 Feb 2019 11:19:08 -0800 Subject: [PATCH] virtcontainers: Eliminate legacy networking models Prior to the addition of tcMirroring support kata-runtime had compatibility issues with some CNI plugins some of which were addressed by the bridged model. With the addition of tc mode there are no gaps in networking that can be filled by the bridged mode or enlightened mode (which was never implemented). Eliminate both of these options to simplify the setup. Fixes: #1213 Signed-off-by: Manohar Castelino --- cli/config/configuration-fc.toml.in | 8 +- cli/config/configuration-qemu.toml.in | 8 +- virtcontainers/documentation/api/1.0/api.md | 12 -- virtcontainers/network.go | 198 -------------------- virtcontainers/network_test.go | 38 +--- virtcontainers/qemu_arch_base.go | 7 - 6 files changed, 7 insertions(+), 264 deletions(-) diff --git a/cli/config/configuration-fc.toml.in b/cli/config/configuration-fc.toml.in index 520642d914..446db958b0 100644 --- a/cli/config/configuration-fc.toml.in +++ b/cli/config/configuration-fc.toml.in @@ -289,12 +289,6 @@ path = "@NETMONPATH@" # the container network interface # Options: # -# - bridged (Deprecated) -# Uses a linux bridge to interconnect the container interface to -# the VM. Works for most cases except macvlan and ipvlan. -# ***NOTE: This feature has been deprecated with plans to remove this -# feature in the future. Please use other network models listed below. -# # - macvtap # Used when the Container network interface can be bridged using # macvtap. @@ -323,7 +317,7 @@ disable_guest_seccomp=@DEFDISABLEGUESTSECCOMP@ # If enabled, the runtime will not create a network namespace for shim and hypervisor processes. # This option may have some potential impacts to your host. It should only be used when you know what you're doing. # `disable_new_netns` conflicts with `enable_netmon` -# `disable_new_netns` conflicts with `internetworking_model=bridged` and `internetworking_model=macvtap`. It works only +# `disable_new_netns` conflicts with `internetworking_model=tcfilter` and `internetworking_model=macvtap`. It works only # with `internetworking_model=none`. The tap device will be in the host network namespace and can connect to a bridge # (like OVS) directly. # If you are using docker, `disable_new_netns` only works with `docker run --net=none` diff --git a/cli/config/configuration-qemu.toml.in b/cli/config/configuration-qemu.toml.in index 3fbbf37910..404eb4754a 100644 --- a/cli/config/configuration-qemu.toml.in +++ b/cli/config/configuration-qemu.toml.in @@ -379,12 +379,6 @@ path = "@NETMONPATH@" # the container network interface # Options: # -# - bridged (Deprecated) -# Uses a linux bridge to interconnect the container interface to -# the VM. Works for most cases except macvlan and ipvlan. -# ***NOTE: This feature has been deprecated with plans to remove this -# feature in the future. Please use other network models listed below. -# # - macvtap # Used when the Container network interface can be bridged using # macvtap. @@ -413,7 +407,7 @@ disable_guest_seccomp=@DEFDISABLEGUESTSECCOMP@ # If enabled, the runtime will not create a network namespace for shim and hypervisor processes. # This option may have some potential impacts to your host. It should only be used when you know what you're doing. # `disable_new_netns` conflicts with `enable_netmon` -# `disable_new_netns` conflicts with `internetworking_model=bridged` and `internetworking_model=macvtap`. It works only +# `disable_new_netns` conflicts with `internetworking_model=tcfilter` and `internetworking_model=macvtap`. It works only # with `internetworking_model=none`. The tap device will be in the host network namespace and can connect to a bridge # (like OVS) directly. # If you are using docker, `disable_new_netns` only works with `docker run --net=none` diff --git a/virtcontainers/documentation/api/1.0/api.md b/virtcontainers/documentation/api/1.0/api.md index 4753f5c9d1..f5d3a7347c 100644 --- a/virtcontainers/documentation/api/1.0/api.md +++ b/virtcontainers/documentation/api/1.0/api.md @@ -278,22 +278,10 @@ const ( // NetXConnectDefaultModel Ask to use DefaultNetInterworkingModel NetXConnectDefaultModel NetInterworkingModel = iota - // NetXConnectBridgedModel uses a linux bridge to interconnect - // the container interface to the VM. This is the - // safe default that works for most cases except - // macvlan and ipvlan - NetXConnectBridgedModel - // NetXConnectMacVtapModel can be used when the Container network // interface can be bridged using macvtap NetXConnectMacVtapModel - // NetXConnectEnlightenedModel can be used when the Network plugins - // are enlightened to create VM native interfaces - // when requested by the runtime - // This will be used for vethtap, macvtap, ipvtap - NetXConnectEnlightenedModel - // NetXConnectInvalidModel is the last item to check valid values by IsValid() NetXConnectInvalidModel ) diff --git a/virtcontainers/network.go b/virtcontainers/network.go index 83766a8590..55bb773319 100644 --- a/virtcontainers/network.go +++ b/virtcontainers/network.go @@ -38,22 +38,10 @@ const ( // NetXConnectDefaultModel Ask to use DefaultNetInterworkingModel NetXConnectDefaultModel NetInterworkingModel = iota - // NetXConnectBridgedModel uses a linux bridge to interconnect - // the container interface to the VM. This is the - // safe default that works for most cases except - // macvlan and ipvlan - NetXConnectBridgedModel - // NetXConnectMacVtapModel can be used when the Container network // interface can be bridged using macvtap NetXConnectMacVtapModel - // NetXConnectEnlightenedModel can be used when the Network plugins - // are enlightened to create VM native interfaces - // when requested by the runtime - // This will be used for vethtap, macvtap, ipvtap - NetXConnectEnlightenedModel - // NetXConnectTCFilterModel redirects traffic from the network interface // provided by the network plugin to a tap interface. // This works for ipvlan and macvlan as well. @@ -74,12 +62,8 @@ func (n NetInterworkingModel) IsValid() bool { const ( defaultNetModelStr = "default" - bridgedNetModelStr = "bridged" - macvtapNetModelStr = "macvtap" - enlightenedNetModelStr = "enlightened" - tcFilterNetModelStr = "tcfilter" noneNetModelStr = "none" @@ -91,15 +75,9 @@ func (n *NetInterworkingModel) SetModel(modelName string) error { case defaultNetModelStr: *n = DefaultNetInterworkingModel return nil - case bridgedNetModelStr: - *n = NetXConnectBridgedModel - return nil case macvtapNetModelStr: *n = NetXConnectMacVtapModel return nil - case enlightenedNetModelStr: - *n = NetXConnectEnlightenedModel - return nil case tcFilterNetModelStr: *n = NetXConnectTCFilterModel return nil @@ -325,11 +303,6 @@ func createLink(netHandle *netlink.Handle, name string, expectedLink netlink.Lin var fds []*os.File switch expectedLink.Type() { - case (&netlink.Bridge{}).Type(): - newLink = &netlink.Bridge{ - LinkAttrs: netlink.LinkAttrs{Name: name}, - MulticastSnooping: expectedLink.(*netlink.Bridge).MulticastSnooping, - } case (&netlink.Tuntap{}).Type(): flags := netlink.TUNTAP_VNET_HDR if queues > 0 { @@ -400,10 +373,6 @@ func getLinkByName(netHandle *netlink.Handle, name string, expectedLink netlink. } switch expectedLink.Type() { - case (&netlink.Bridge{}).Type(): - if l, ok := link.(*netlink.Bridge); ok { - return l, nil - } case (&netlink.Tuntap{}).Type(): if l, ok := link.(*netlink.Tuntap); ok { return l, nil @@ -448,14 +417,10 @@ func xConnectVMNetwork(endpoint Endpoint, h hypervisor) error { } switch netPair.NetInterworkingModel { - case NetXConnectBridgedModel: - return bridgeNetworkPair(endpoint, queues, disableVhostNet) case NetXConnectMacVtapModel: return tapNetworkPair(endpoint, queues, disableVhostNet) case NetXConnectTCFilterModel: return setupTCFiltering(endpoint, queues, disableVhostNet) - case NetXConnectEnlightenedModel: - return fmt.Errorf("Unsupported networking model") default: return fmt.Errorf("Invalid internetworking model") } @@ -470,14 +435,10 @@ func xDisconnectVMNetwork(endpoint Endpoint) error { } switch netPair.NetInterworkingModel { - case NetXConnectBridgedModel: - return unBridgeNetworkPair(endpoint) case NetXConnectMacVtapModel: return untapNetworkPair(endpoint) case NetXConnectTCFilterModel: return removeTCFiltering(endpoint) - case NetXConnectEnlightenedModel: - return fmt.Errorf("Unsupported networking model") default: return fmt.Errorf("Invalid internetworking model") } @@ -656,100 +617,6 @@ func tapNetworkPair(endpoint Endpoint, queues int, disableVhostNet bool) error { return nil } -func bridgeNetworkPair(endpoint Endpoint, queues int, disableVhostNet bool) error { - netHandle, err := netlink.NewHandle() - if err != nil { - return err - } - defer netHandle.Delete() - - netPair := endpoint.NetworkPair() - - tapLink, fds, err := createLink(netHandle, netPair.TAPIface.Name, &netlink.Tuntap{}, queues) - if err != nil { - return fmt.Errorf("Could not create TAP interface: %s", err) - } - netPair.VMFds = fds - - if !disableVhostNet { - vhostFds, err := createVhostFds(queues) - if err != nil { - return fmt.Errorf("Could not setup vhost fds %s : %s", netPair.VirtIface.Name, err) - } - netPair.VhostFds = vhostFds - } - - var attrs *netlink.LinkAttrs - var link netlink.Link - - link, err = getLinkForEndpoint(endpoint, netHandle) - if err != nil { - return err - } - - attrs = link.Attrs() - - // Save the veth MAC address to the TAP so that it can later be used - // to build the hypervisor command line. This MAC address has to be - // the one inside the VM in order to avoid any firewall issues. The - // bridge created by the network plugin on the host actually expects - // to see traffic from this MAC address and not another one. - netPair.TAPIface.HardAddr = attrs.HardwareAddr.String() - - if err := netHandle.LinkSetMTU(tapLink, attrs.MTU); err != nil { - return fmt.Errorf("Could not set TAP MTU %d: %s", attrs.MTU, err) - } - - hardAddr, err := net.ParseMAC(netPair.VirtIface.HardAddr) - if err != nil { - return err - } - if err := netHandle.LinkSetHardwareAddr(link, hardAddr); err != nil { - return fmt.Errorf("Could not set MAC address %s for veth interface %s: %s", - netPair.VirtIface.HardAddr, netPair.VirtIface.Name, err) - } - - mcastSnoop := false - bridgeLink, _, err := createLink(netHandle, netPair.Name, &netlink.Bridge{MulticastSnooping: &mcastSnoop}, queues) - if err != nil { - return fmt.Errorf("Could not create bridge: %s", err) - } - - if err := netHandle.LinkSetMaster(tapLink, bridgeLink.(*netlink.Bridge)); err != nil { - return fmt.Errorf("Could not attach TAP %s to the bridge %s: %s", - netPair.TAPIface.Name, netPair.Name, err) - } - - if err := netHandle.LinkSetUp(tapLink); err != nil { - return fmt.Errorf("Could not enable TAP %s: %s", netPair.TAPIface.Name, err) - } - - if err := netHandle.LinkSetMaster(link, bridgeLink.(*netlink.Bridge)); err != nil { - return fmt.Errorf("Could not attach veth %s to the bridge %s: %s", - netPair.VirtIface.Name, netPair.Name, err) - } - - // Clear the IP addresses from the veth interface to prevent ARP conflict - netPair.VirtIface.Addrs, err = netlink.AddrList(link, netlink.FAMILY_V4) - if err != nil { - return fmt.Errorf("Unable to obtain veth IP addresses: %s", err) - } - - if err := clearIPs(link, netPair.VirtIface.Addrs); err != nil { - return fmt.Errorf("Unable to clear veth IP addresses: %s", err) - } - - if err := netHandle.LinkSetUp(link); err != nil { - return fmt.Errorf("Could not enable veth %s: %s", netPair.VirtIface.Name, err) - } - - if err := netHandle.LinkSetUp(bridgeLink); err != nil { - return fmt.Errorf("Could not enable bridge %s: %s", netPair.Name, err) - } - - return nil -} - func setupTCFiltering(endpoint Endpoint, queues int, disableVhostNet bool) error { netHandle, err := netlink.NewHandle() if err != nil { @@ -963,71 +830,6 @@ func untapNetworkPair(endpoint Endpoint) error { return err } -func unBridgeNetworkPair(endpoint Endpoint) error { - netHandle, err := netlink.NewHandle() - if err != nil { - return err - } - defer netHandle.Delete() - - netPair := endpoint.NetworkPair() - - tapLink, err := getLinkByName(netHandle, netPair.TAPIface.Name, &netlink.Tuntap{}) - if err != nil { - return fmt.Errorf("Could not get TAP interface: %s", err) - } - - bridgeLink, err := getLinkByName(netHandle, netPair.Name, &netlink.Bridge{}) - if err != nil { - return fmt.Errorf("Could not get bridge interface: %s", err) - } - - if err := netHandle.LinkSetDown(bridgeLink); err != nil { - return fmt.Errorf("Could not disable bridge %s: %s", netPair.Name, err) - } - - if err := netHandle.LinkSetDown(tapLink); err != nil { - return fmt.Errorf("Could not disable TAP %s: %s", netPair.TAPIface.Name, err) - } - - if err := netHandle.LinkSetNoMaster(tapLink); err != nil { - return fmt.Errorf("Could not detach TAP %s: %s", netPair.TAPIface.Name, err) - } - - if err := netHandle.LinkDel(bridgeLink); err != nil { - return fmt.Errorf("Could not remove bridge %s: %s", netPair.Name, err) - } - - if err := netHandle.LinkDel(tapLink); err != nil { - return fmt.Errorf("Could not remove TAP %s: %s", netPair.TAPIface.Name, err) - } - - link, err := getLinkForEndpoint(endpoint, netHandle) - if err != nil { - return err - } - - hardAddr, err := net.ParseMAC(netPair.TAPIface.HardAddr) - if err != nil { - return err - } - if err := netHandle.LinkSetHardwareAddr(link, hardAddr); err != nil { - return fmt.Errorf("Could not set MAC address %s for veth interface %s: %s", - netPair.VirtIface.HardAddr, netPair.VirtIface.Name, err) - } - - if err := netHandle.LinkSetDown(link); err != nil { - return fmt.Errorf("Could not disable veth %s: %s", netPair.VirtIface.Name, err) - } - - if err := netHandle.LinkSetNoMaster(link); err != nil { - return fmt.Errorf("Could not detach veth %s: %s", netPair.VirtIface.Name, err) - } - - // Restore the IPs that were cleared - return setIPs(link, netPair.VirtIface.Addrs) -} - func removeTCFiltering(endpoint Endpoint) error { netHandle, err := netlink.NewHandle() if err != nil { diff --git a/virtcontainers/network_test.go b/virtcontainers/network_test.go index 83cec20079..c484b0866c 100644 --- a/virtcontainers/network_test.go +++ b/virtcontainers/network_test.go @@ -110,10 +110,8 @@ func TestNetInterworkingModelIsValid(t *testing.T) { }{ {"Invalid Model", NetXConnectInvalidModel, false}, {"Default Model", NetXConnectDefaultModel, true}, - {"Bridged Model", NetXConnectBridgedModel, true}, {"TC Filter Model", NetXConnectTCFilterModel, true}, {"Macvtap Model", NetXConnectMacVtapModel, true}, - {"Enlightened Model", NetXConnectEnlightenedModel, true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -133,9 +131,7 @@ func TestNetInterworkingModelSetModel(t *testing.T) { }{ {"Invalid Model", "Invalid", true}, {"default Model", defaultNetModelStr, false}, - {"bridged Model", bridgedNetModelStr, false}, {"macvtap Model", macvtapNetModelStr, false}, - {"enlightened Model", enlightenedNetModelStr, false}, {"tcfilter Model", tcFilterNetModelStr, false}, {"none Model", noneNetModelStr, false}, } @@ -167,30 +163,6 @@ func TestGenerateRandomPrivateMacAdd(t *testing.T) { assert.NotEqual(addr1, addr2) } -func TestCreateGetBridgeLink(t *testing.T) { - if tc.NotValid(ktu.NeedRoot()) { - t.Skip(testDisabledAsNonRoot) - } - - assert := assert.New(t) - - netHandle, err := netlink.NewHandle() - defer netHandle.Delete() - - assert.NoError(err) - - brName := "testbr0" - brLink, _, err := createLink(netHandle, brName, &netlink.Bridge{}, 1) - assert.NoError(err) - assert.NotNil(brLink) - - brLink, err = getLinkByName(netHandle, brName, &netlink.Bridge{}) - assert.NoError(err) - - err = netHandle.LinkDel(brLink) - assert.NoError(err) -} - func TestCreateGetTunTapLink(t *testing.T) { if tc.NotValid(ktu.NeedRoot()) { t.Skip(testDisabledAsNonRoot) @@ -228,11 +200,11 @@ func TestCreateMacVtap(t *testing.T) { assert.NoError(err) - brName := "testbr0" - brLink, _, err := createLink(netHandle, brName, &netlink.Bridge{}, 1) + tapName := "testtap0" + tapLink, _, err := createLink(netHandle, tapName, &netlink.Tuntap{}, 1) assert.NoError(err) - attrs := brLink.Attrs() + attrs := tapLink.Attrs() mcLink := &netlink.Macvtap{ Macvlan: netlink.Macvlan{ @@ -253,10 +225,10 @@ func TestCreateMacVtap(t *testing.T) { err = netHandle.LinkDel(macvtapLink) assert.NoError(err) - brLink, err = getLinkByName(netHandle, brName, &netlink.Bridge{}) + tapLink, err = getLinkByName(netHandle, tapName, &netlink.Tuntap{}) assert.NoError(err) - err = netHandle.LinkDel(brLink) + err = netHandle.LinkDel(tapLink) assert.NoError(err) } diff --git a/virtcontainers/qemu_arch_base.go b/virtcontainers/qemu_arch_base.go index 9d6d68cf33..41e7018c6e 100644 --- a/virtcontainers/qemu_arch_base.go +++ b/virtcontainers/qemu_arch_base.go @@ -467,15 +467,8 @@ func (q *qemuArchBase) appendVSock(devices []govmmQemu.Device, vsock types.VSock func networkModelToQemuType(model NetInterworkingModel) govmmQemu.NetDeviceType { switch model { - case NetXConnectBridgedModel: - return govmmQemu.MACVTAP //TODO: We should rename MACVTAP to .NET_FD case NetXConnectMacVtapModel: return govmmQemu.MACVTAP - //case ModelEnlightened: - // Here the Network plugin will create a VM native interface - // which could be MacVtap, IpVtap, SRIOV, veth-tap, vhost-user - // In these cases we will determine the interface type here - // and pass in the native interface through default: //TAP should work for most other cases return govmmQemu.TAP