Skip to content

Commit

Permalink
virt-launcher: make nftables as the default
Browse files Browse the repository at this point in the history 
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 <[email protected]>
  • Loading branch information
@AlonaKaplan
AlonaKaplan committed Feb 16, 2021
1 parent c57dd62 commit 2122c2b
Show file tree
Hide file tree
Showing 6 changed files with 77 additions and 67 deletions.
1 change: 1 addition & 0 deletions pkg/virt-launcher/virtwrap/network/BUILD.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
41 changes: 29 additions & 12 deletions pkg/virt-launcher/virtwrap/network/common.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ const (
randomMacGenerationAttempts = 10
tapOwnerUID = "0"
tapOwnerGID = "0"
allowForwarding = 1
)

type VIF struct {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -266,16 +268,31 @@ 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))
}

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 {
Expand Down
17 changes: 17 additions & 0 deletions pkg/virt-launcher/virtwrap/network/common_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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() {
Expand Down
22 changes: 6 additions & 16 deletions pkg/virt-launcher/virtwrap/network/generated_mock_common.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
Expand Down
48 changes: 18 additions & 30 deletions pkg/virt-launcher/virtwrap/network/podinterface.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
}
}

Expand Down Expand Up @@ -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 {
Expand Down
15 changes: 6 additions & 9 deletions pkg/virt-launcher/virtwrap/network/podinterface_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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",
Expand Down Expand Up @@ -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)
Expand All @@ -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",
Expand Down

0 comments on commit 2122c2b

Please sign in to comment.