From 2122c2beb050399c79a290f880c3fac7295a18ea Mon Sep 17 00:00:00 2001 From: Alona Kaplan Date: Thu, 11 Feb 2021 16:02:03 +0200 Subject: [PATCH] virt-launcher: make nftables as the default Currently the default is iptables, and only if iptables are not working we try to use nft. Since iptables is deprecated in fedora 32 we need to change the default to be nft. Signed-off-by: Alona Kaplan --- .../virtwrap/network/BUILD.bazel | 1 + pkg/virt-launcher/virtwrap/network/common.go | 41 +++++++++++----- .../virtwrap/network/common_test.go | 17 +++++++ .../virtwrap/network/generated_mock_common.go | 22 +++------ .../virtwrap/network/podinterface.go | 48 +++++++------------ .../virtwrap/network/podinterface_test.go | 15 +++--- 6 files changed, 77 insertions(+), 67 deletions(-) diff --git a/pkg/virt-launcher/virtwrap/network/BUILD.bazel b/pkg/virt-launcher/virtwrap/network/BUILD.bazel index b65e7c87c419..3eb01478ca4f 100644 --- a/pkg/virt-launcher/virtwrap/network/BUILD.bazel +++ b/pkg/virt-launcher/virtwrap/network/BUILD.bazel @@ -50,6 +50,7 @@ go_test( "//vendor/github.com/coreos/go-iptables/iptables:go_default_library", "//vendor/github.com/golang/mock/gomock:go_default_library", "//vendor/github.com/onsi/ginkgo:go_default_library", + "//vendor/github.com/onsi/ginkgo/extensions/table:go_default_library", "//vendor/github.com/onsi/gomega:go_default_library", "//vendor/github.com/vishvananda/netlink:go_default_library", "//vendor/k8s.io/apimachinery/pkg/types:go_default_library", diff --git a/pkg/virt-launcher/virtwrap/network/common.go b/pkg/virt-launcher/virtwrap/network/common.go index 3353800ac87c..5aa3eb5762b6 100644 --- a/pkg/virt-launcher/virtwrap/network/common.go +++ b/pkg/virt-launcher/virtwrap/network/common.go @@ -49,6 +49,7 @@ const ( randomMacGenerationAttempts = 10 tapOwnerUID = "0" tapOwnerGID = "0" + allowForwarding = 1 ) type VIF struct { @@ -105,14 +106,13 @@ type NetworkHandler interface { HasNatIptables(proto iptables.Protocol) bool IsIpv6Enabled(interfaceName string) (bool, error) IsIpv4Primary() (bool, error) - ConfigureIpv6Forwarding() error - ConfigureIpv4Forwarding() error + ConfigureIpForwarding(proto iptables.Protocol) error ConfigureIpv4ArpIgnore() error IptablesNewChain(proto iptables.Protocol, table, chain string) error IptablesAppendRule(proto iptables.Protocol, table, chain string, rulespec ...string) error NftablesNewChain(proto iptables.Protocol, table, chain string) error NftablesAppendRule(proto iptables.Protocol, table, chain string, rulespec ...string) error - NftablesLoad(fnName string) error + NftablesLoad(proto iptables.Protocol) error GetNFTIPString(proto iptables.Protocol) string CreateTapDevice(tapName string, queueNumber uint32, launcherPID int, mtu int) error BindTapDeviceToBridge(tapName string, bridgeName string) error @@ -183,13 +183,15 @@ func (h *NetworkUtilsHandler) ConfigureIpv4ArpIgnore() error { return err } -func (h *NetworkUtilsHandler) ConfigureIpv6Forwarding() error { - err := sysctl.New().SetSysctl(sysctl.NetIPv6Forwarding, 1) - return err -} +func (h *NetworkUtilsHandler) ConfigureIpForwarding(proto iptables.Protocol) error { + var forwarding string + if proto == iptables.ProtocolIPv6 { + forwarding = sysctl.NetIPv6Forwarding + } else { + forwarding = sysctl.NetIPv4Forwarding + } -func (h *NetworkUtilsHandler) ConfigureIpv4Forwarding() error { - err := sysctl.New().SetSysctl(sysctl.NetIPv4Forwarding, 1) + err := sysctl.New().SetSysctl(forwarding, allowForwarding) return err } @@ -266,9 +268,13 @@ func (h *NetworkUtilsHandler) GetNFTIPString(proto iptables.Protocol) string { return "ip" } -func (h *NetworkUtilsHandler) NftablesLoad(fnName string) error { - // #nosec g204 no risk to use Sprintf as argument as it uses two static strings (fname limited to ipv4-nat or ipv6-nat) - output, err := exec.Command("nft", "-f", fmt.Sprintf("/etc/nftables/%s.nft", fnName)).CombinedOutput() +func (h *NetworkUtilsHandler) NftablesLoad(proto iptables.Protocol) error { + ipVersion := "4" + if proto == iptables.ProtocolIPv6 { + ipVersion = "6" + } + fnName := fmt.Sprintf("ipv%s-nat", ipVersion) + output, err := composeNftablesLoad(proto).CombinedOutput() if err != nil { log.Log.V(5).Reason(err).Infof("failed to load nftable %s", fnName) return fmt.Errorf("failed to load nftable %s error %s", fnName, string(output)) @@ -276,6 +282,17 @@ func (h *NetworkUtilsHandler) NftablesLoad(fnName string) error { return nil } + +func composeNftablesLoad(proto iptables.Protocol) *exec.Cmd { + ipVersion := "4" + if proto == iptables.ProtocolIPv6 { + ipVersion = "6" + } + fnName := fmt.Sprintf("ipv%s-nat", ipVersion) + // #nosec g204 no risk to use Sprintf as argument as it uses two static strings (fname limited to ipv4-nat or ipv6-nat) + return exec.Command("nft", "-f", fmt.Sprintf("/etc/nftables/%s.nft", fnName)) +} + func (h *NetworkUtilsHandler) GetHostAndGwAddressesFromCIDR(s string) (string, string, error) { ip, ipnet, err := net.ParseCIDR(s) if err != nil { diff --git a/pkg/virt-launcher/virtwrap/network/common_test.go b/pkg/virt-launcher/virtwrap/network/common_test.go index b8cad2ca04d4..25a8d8cad920 100644 --- a/pkg/virt-launcher/virtwrap/network/common_test.go +++ b/pkg/virt-launcher/virtwrap/network/common_test.go @@ -26,6 +26,9 @@ import ( "os" "strings" + "github.com/coreos/go-iptables/iptables" + "github.com/onsi/ginkgo/extensions/table" + . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" "github.com/vishvananda/netlink" @@ -110,6 +113,20 @@ var _ = Describe("Common Methods", func() { Expect(strings.HasPrefix(mac.String(), "02:00:00")).To(BeTrue()) }) }) + Context("composeNftablesLoad function", func() { + table.DescribeTable("should compose the correct command", + func(protocol iptables.Protocol, protocolVersionNum string) { + cmd := composeNftablesLoad(protocol) + Expect(cmd.Path).To(Equal("nft")) + Expect(cmd.Args).To(Equal([]string{ + "nft", + "-f", + fmt.Sprintf("/etc/nftables/ipv%s-nat.nft", protocolVersionNum)})) + }, + table.Entry("ipv4", iptables.ProtocolIPv4, "4"), + table.Entry("ipv6", iptables.ProtocolIPv6, "6"), + ) + }) }) var _ = Describe("VIF", func() { diff --git a/pkg/virt-launcher/virtwrap/network/generated_mock_common.go b/pkg/virt-launcher/virtwrap/network/generated_mock_common.go index e0816d473da6..a4cd03f76706 100644 --- a/pkg/virt-launcher/virtwrap/network/generated_mock_common.go +++ b/pkg/virt-launcher/virtwrap/network/generated_mock_common.go @@ -255,24 +255,14 @@ func (_mr *_MockNetworkHandlerRecorder) IsIpv4Primary() *gomock.Call { return _mr.mock.ctrl.RecordCall(_mr.mock, "IsIpv4Primary") } -func (_m *MockNetworkHandler) ConfigureIpv6Forwarding() error { - ret := _m.ctrl.Call(_m, "ConfigureIpv6Forwarding") +func (_m *MockNetworkHandler) ConfigureIpForwarding(proto iptables.Protocol) error { + ret := _m.ctrl.Call(_m, "ConfigureIpForwarding", proto) ret0, _ := ret[0].(error) return ret0 } -func (_mr *_MockNetworkHandlerRecorder) ConfigureIpv6Forwarding() *gomock.Call { - return _mr.mock.ctrl.RecordCall(_mr.mock, "ConfigureIpv6Forwarding") -} - -func (_m *MockNetworkHandler) ConfigureIpv4Forwarding() error { - ret := _m.ctrl.Call(_m, "ConfigureIpv4Forwarding") - ret0, _ := ret[0].(error) - return ret0 -} - -func (_mr *_MockNetworkHandlerRecorder) ConfigureIpv4Forwarding() *gomock.Call { - return _mr.mock.ctrl.RecordCall(_mr.mock, "ConfigureIpv4Forwarding") +func (_mr *_MockNetworkHandlerRecorder) ConfigureIpForwarding(arg0 interface{}) *gomock.Call { + return _mr.mock.ctrl.RecordCall(_mr.mock, "ConfigureIpForwarding", arg0) } func (_m *MockNetworkHandler) ConfigureIpv4ArpIgnore() error { @@ -335,8 +325,8 @@ func (_mr *_MockNetworkHandlerRecorder) NftablesAppendRule(arg0, arg1, arg2 inte return _mr.mock.ctrl.RecordCall(_mr.mock, "NftablesAppendRule", _s...) } -func (_m *MockNetworkHandler) NftablesLoad(fnName string) error { - ret := _m.ctrl.Call(_m, "NftablesLoad", fnName) +func (_m *MockNetworkHandler) NftablesLoad(proto iptables.Protocol) error { + ret := _m.ctrl.Call(_m, "NftablesLoad", proto) ret0, _ := ret[0].(error) return ret0 } diff --git a/pkg/virt-launcher/virtwrap/network/podinterface.go b/pkg/virt-launcher/virtwrap/network/podinterface.go index ce439ed282f6..d30ff193f620 100644 --- a/pkg/virt-launcher/virtwrap/network/podinterface.go +++ b/pkg/virt-launcher/virtwrap/network/podinterface.go @@ -818,20 +818,10 @@ func (b *MasqueradeBindMechanism) preparePodNetworkInterfaces(queueNumber uint32 return err } - if Handler.HasNatIptables(iptables.ProtocolIPv4) || Handler.NftablesLoad("ipv4-nat") == nil { - err = Handler.ConfigureIpv4Forwarding() - if err != nil { - log.Log.Reason(err).Errorf("failed to configure ipv4 forwarding") - return err - } - - err = b.createNatRules(iptables.ProtocolIPv4) - if err != nil { - log.Log.Reason(err).Errorf("failed to create ipv4 nat rules for vm error: %v", err) - return err - } - } else { - return fmt.Errorf("Couldn't configure ipv4 nat rules") + err = b.createNatRules(iptables.ProtocolIPv4) + if err != nil { + log.Log.Reason(err).Errorf("failed to create ipv4 nat rules for vm error: %v", err) + return err } ipv6Enabled, err := Handler.IsIpv6Enabled(b.podInterfaceName) @@ -840,20 +830,10 @@ func (b *MasqueradeBindMechanism) preparePodNetworkInterfaces(queueNumber uint32 return err } if ipv6Enabled { - if Handler.HasNatIptables(iptables.ProtocolIPv6) || Handler.NftablesLoad("ipv6-nat") == nil { - err = Handler.ConfigureIpv6Forwarding() - if err != nil { - log.Log.Reason(err).Errorf("failed to configure ipv6 forwarding") - return err - } - - err = b.createNatRules(iptables.ProtocolIPv6) - if err != nil { - log.Log.Reason(err).Errorf("failed to create ipv6 nat rules for vm error: %v", err) - return err - } - } else { - return fmt.Errorf("Couldn't configure ipv6 nat rules") + err = b.createNatRules(iptables.ProtocolIPv6) + if err != nil { + log.Log.Reason(err).Errorf("failed to create ipv6 nat rules for vm error: %v", err) + return err } } @@ -985,10 +965,18 @@ func (b *MasqueradeBindMechanism) createBridge() error { } func (b *MasqueradeBindMechanism) createNatRules(protocol iptables.Protocol) error { - if Handler.HasNatIptables(protocol) { + err := Handler.ConfigureIpForwarding(protocol) + if err != nil { + log.Log.Reason(err).Errorf("failed to configure ip forwarding") + return err + } + + if Handler.NftablesLoad(protocol) == nil { + return b.createNatRulesUsingNftables(protocol) + } else if Handler.HasNatIptables(protocol) { return b.createNatRulesUsingIptables(protocol) } - return b.createNatRulesUsingNftables(protocol) + return fmt.Errorf("Couldn't configure ip nat rules") } func (b *MasqueradeBindMechanism) createNatRulesUsingIptables(protocol iptables.Protocol) error { diff --git a/pkg/virt-launcher/virtwrap/network/podinterface_test.go b/pkg/virt-launcher/virtwrap/network/podinterface_test.go index 892ec7e70490..2885fbf8cb2d 100644 --- a/pkg/virt-launcher/virtwrap/network/podinterface_test.go +++ b/pkg/virt-launcher/virtwrap/network/podinterface_test.go @@ -232,8 +232,8 @@ var _ = Describe("Pod Network", func() { mockNetwork.EXPECT().CreateTapDevice(tapDeviceName, queueNumber, pid, mtu).Return(nil) mockNetwork.EXPECT().DisableTXOffloadChecksum(bridgeTest.Name).Return(nil) // Global nat rules using iptables - mockNetwork.EXPECT().ConfigureIpv4Forwarding().Return(nil) - mockNetwork.EXPECT().ConfigureIpv6Forwarding().Return(nil) + mockNetwork.EXPECT().ConfigureIpForwarding(iptables.ProtocolIPv4).Return(nil) + mockNetwork.EXPECT().ConfigureIpForwarding(iptables.ProtocolIPv6).Return(nil) mockNetwork.EXPECT().GetNFTIPString(iptables.ProtocolIPv4).Return("ip").AnyTimes() mockNetwork.EXPECT().GetNFTIPString(iptables.ProtocolIPv6).Return("ip6").AnyTimes() for _, proto := range ipProtocols() { @@ -263,11 +263,6 @@ var _ = Describe("Pod Network", func() { "--to-destination", GetMasqueradeVmIp(proto)).Return(nil) //Global net rules using nftable - ipVersionNum := "4" - if proto == iptables.ProtocolIPv6 { - ipVersionNum = "6" - } - mockNetwork.EXPECT().NftablesLoad(fmt.Sprintf("ipv%s-nat", ipVersionNum)).Return(nil) mockNetwork.EXPECT().NftablesNewChain(proto, "nat", "KUBEVIRT_PREINBOUND").Return(nil) mockNetwork.EXPECT().NftablesNewChain(proto, "nat", "KUBEVIRT_POSTINBOUND").Return(nil) mockNetwork.EXPECT().NftablesAppendRule(proto, "nat", "postrouting", GetNFTIPString(proto), "saddr", GetMasqueradeVmIp(proto), "counter", "masquerade").Return(nil) @@ -431,6 +426,7 @@ var _ = Describe("Pod Network", func() { // forward all the traffic for _, proto := range ipProtocols() { + mockNetwork.EXPECT().NftablesLoad(proto).Return(fmt.Errorf("no nft")) mockNetwork.EXPECT().HasNatIptables(proto).Return(true).Times(2) } mockNetwork.EXPECT().IsIpv6Enabled(primaryPodInterfaceName).Return(true, nil).Times(3) @@ -448,6 +444,7 @@ var _ = Describe("Pod Network", func() { mockNetwork.EXPECT().IsIpv4Primary().Return(true, nil).Times(1) for _, proto := range ipProtocols() { + mockNetwork.EXPECT().NftablesLoad(proto).Return(fmt.Errorf("no nft")) mockNetwork.EXPECT().HasNatIptables(proto).Return(true).Times(2) mockNetwork.EXPECT().IptablesAppendRule(proto, "nat", "KUBEVIRT_POSTINBOUND", @@ -482,7 +479,7 @@ var _ = Describe("Pod Network", func() { It("should define a new VIF bind to a bridge and create a default nat rule using nftables", func() { // forward all the traffic for _, proto := range ipProtocols() { - mockNetwork.EXPECT().HasNatIptables(proto).Return(true).Times(2) + mockNetwork.EXPECT().NftablesLoad(proto).Return(nil) } mockNetwork.EXPECT().IsIpv6Enabled(primaryPodInterfaceName).Return(true, nil).Times(3) mockNetwork.EXPECT().IsIpv4Primary().Return(true, nil).Times(1) @@ -499,7 +496,7 @@ var _ = Describe("Pod Network", func() { mockNetwork.EXPECT().IsIpv4Primary().Return(true, nil).Times(1) for _, proto := range ipProtocols() { - mockNetwork.EXPECT().HasNatIptables(proto).Return(false).Times(2) + mockNetwork.EXPECT().NftablesLoad(proto).Return(nil) mockNetwork.EXPECT().NftablesAppendRule(proto, "nat", "KUBEVIRT_POSTINBOUND",