From 4ac634f5b74e6c3ade2b9a850d714f33f0e4e45a Mon Sep 17 00:00:00 2001 From: Hongliang Liu Date: Thu, 30 Jun 2022 19:44:54 +0800 Subject: [PATCH] Make disabling TX checksum for Antrea gateway This is a supplement to PR #3832. When `disableTXChecksumOffload` is true, TX checksum offload should be also disabled, otherwise for the cases in which the datapath doesn't support TX checksum offloading, packets received on Antrea gateway could be dropped due to bad checksum. Signed-off-by: Hongliang Liu --- cmd/antrea-agent/agent.go | 3 +- pkg/agent/agent.go | 80 +++++++++++-------- .../interface_configuration_linux.go | 2 +- pkg/agent/util/ethtool/ethtool_linux.go | 14 +++- pkg/agent/util/ethtool/ethtool_windows.go | 27 +++++++ 5 files changed, 87 insertions(+), 39 deletions(-) create mode 100644 pkg/agent/util/ethtool/ethtool_windows.go diff --git a/cmd/antrea-agent/agent.go b/cmd/antrea-agent/agent.go index 901d48cae7f..aeda5a9ddd4 100644 --- a/cmd/antrea-agent/agent.go +++ b/cmd/antrea-agent/agent.go @@ -236,7 +236,8 @@ func run(o *Options) error { stopCh, features.DefaultFeatureGate.Enabled(features.AntreaProxy), o.config.AntreaProxy.ProxyAll, - connectUplinkToBridge) + connectUplinkToBridge, + o.config.DisableTXChecksumOffload) err = agentInitializer.Initialize() if err != nil { return fmt.Errorf("error initializing agent: %v", err) diff --git a/pkg/agent/agent.go b/pkg/agent/agent.go index 08ea15bac75..7ab2154a578 100644 --- a/pkg/agent/agent.go +++ b/pkg/agent/agent.go @@ -44,6 +44,7 @@ import ( "antrea.io/antrea/pkg/agent/route" "antrea.io/antrea/pkg/agent/types" "antrea.io/antrea/pkg/agent/util" + "antrea.io/antrea/pkg/agent/util/ethtool" "antrea.io/antrea/pkg/agent/wireguard" "antrea.io/antrea/pkg/features" "antrea.io/antrea/pkg/ovs/ovsconfig" @@ -81,22 +82,23 @@ var otherConfigKeysForIPsecCertificates = []string{"certificate", "private_key", // Initializer knows how to setup host networking, OpenVSwitch, and Openflow. type Initializer struct { - client clientset.Interface - ovsBridgeClient ovsconfig.OVSBridgeClient - ofClient openflow.Client - routeClient route.Interface - wireGuardClient wireguard.Interface - ifaceStore interfacestore.InterfaceStore - ovsBridge string - hostGateway string // name of gateway port on the OVS bridge - mtu int - networkConfig *config.NetworkConfig - nodeConfig *config.NodeConfig - wireGuardConfig *config.WireGuardConfig - egressConfig *config.EgressConfig - serviceConfig *config.ServiceConfig - enableProxy bool - connectUplinkToBridge bool + client clientset.Interface + ovsBridgeClient ovsconfig.OVSBridgeClient + ofClient openflow.Client + routeClient route.Interface + wireGuardClient wireguard.Interface + ifaceStore interfacestore.InterfaceStore + ovsBridge string + hostGateway string // name of gateway port on the OVS bridge + mtu int + networkConfig *config.NetworkConfig + nodeConfig *config.NodeConfig + wireGuardConfig *config.WireGuardConfig + egressConfig *config.EgressConfig + serviceConfig *config.ServiceConfig + enableProxy bool + connectUplinkToBridge bool + disableTXChecksumOffload bool // networkReadyCh should be closed once the Node's network is ready. // The CNI server will wait for it before handling any CNI Add requests. proxyAll bool @@ -122,25 +124,27 @@ func NewInitializer( enableProxy bool, proxyAll bool, connectUplinkToBridge bool, + disableTXChecksumOffload bool, ) *Initializer { return &Initializer{ - ovsBridgeClient: ovsBridgeClient, - client: k8sClient, - ifaceStore: ifaceStore, - ofClient: ofClient, - routeClient: routeClient, - ovsBridge: ovsBridge, - hostGateway: hostGateway, - mtu: mtu, - networkConfig: networkConfig, - wireGuardConfig: wireGuardConfig, - egressConfig: egressConfig, - serviceConfig: serviceConfig, - networkReadyCh: networkReadyCh, - stopCh: stopCh, - enableProxy: enableProxy, - proxyAll: proxyAll, - connectUplinkToBridge: connectUplinkToBridge, + ovsBridgeClient: ovsBridgeClient, + client: k8sClient, + ifaceStore: ifaceStore, + ofClient: ofClient, + routeClient: routeClient, + ovsBridge: ovsBridge, + hostGateway: hostGateway, + mtu: mtu, + networkConfig: networkConfig, + wireGuardConfig: wireGuardConfig, + egressConfig: egressConfig, + serviceConfig: serviceConfig, + networkReadyCh: networkReadyCh, + stopCh: stopCh, + enableProxy: enableProxy, + proxyAll: proxyAll, + connectUplinkToBridge: connectUplinkToBridge, + disableTXChecksumOffload: disableTXChecksumOffload, } } @@ -663,6 +667,16 @@ func (i *Initializer) configureGatewayInterface(gatewayIface *interfacestore.Int return err } + if i.disableTXChecksumOffload { + if err := ethtool.EthtoolTXHWCsumSwitch(i.hostGateway, ethtool.TXCSUM_OFF); err != nil { + return fmt.Errorf("error when disabling TX checksum offload on %s: %v", i.hostGateway, err) + } + } else { + if err := ethtool.EthtoolTXHWCsumSwitch(i.hostGateway, ethtool.TXCSUM_ON); err != nil { + return fmt.Errorf("error when enabling TX checksum offload on %s: %v", i.hostGateway, err) + } + } + i.nodeConfig.GatewayConfig = &config.GatewayConfig{Name: i.hostGateway, MAC: gwMAC, OFPort: uint32(gatewayIface.OFPort)} gatewayIface.MAC = gwMAC gatewayIface.IPs = []net.IP{} diff --git a/pkg/agent/cniserver/interface_configuration_linux.go b/pkg/agent/cniserver/interface_configuration_linux.go index d165876bbb9..d5c24732ef6 100644 --- a/pkg/agent/cniserver/interface_configuration_linux.go +++ b/pkg/agent/cniserver/interface_configuration_linux.go @@ -265,7 +265,7 @@ func (ic *ifConfigurator) configureContainerLinkVeth( // OVS netdev datapath doesn't support TX checksum offloading, i.e. if packet // arrives with bad/no checksum it will be sent to the output port with same bad/no checksum. if ic.disableTXChecksumOffload || ic.ovsDatapathType == ovsconfig.OVSDatapathNetdev { - if err := ethtool.EthtoolTXHWCsumOff(containerVeth.Name); err != nil { + if err := ethtool.EthtoolTXHWCsumSwitch(containerVeth.Name, ethtool.TXCSUM_OFF); err != nil { return fmt.Errorf("error when disabling TX checksum offload on container veth: %v", err) } } diff --git a/pkg/agent/util/ethtool/ethtool_linux.go b/pkg/agent/util/ethtool/ethtool_linux.go index f97bfed048e..051f99fa98b 100644 --- a/pkg/agent/util/ethtool/ethtool_linux.go +++ b/pkg/agent/util/ethtool/ethtool_linux.go @@ -1,3 +1,6 @@ +//go:build linux +// +build linux + // Copyright 2019 Antrea Authors // // Licensed under the Apache License, Version 2.0 (the "License"); @@ -24,6 +27,9 @@ const ( IFNAMSIZ = 16 // defined in linux/if.h SIOCETHTOOL = 0x8946 // ethtool interface, defined in linux/sockios.h ETHTOOL_STXCSUM = 0x00000017 // set TX hw csum enable, defined in linux/ethtool.h + + TXCSUM_ON = uint32(1) + TXCSUM_OFF = uint32(0) ) // defined in linux/if.h (struct ifreq) @@ -38,8 +44,8 @@ type ethtoolValue struct { Data uint32 } -// EthtoolTXHWCsumOff disables TX checksum offload on the specified interface. -func EthtoolTXHWCsumOff(name string) error { +// EthtoolTXHWCsumSwitch enables or disables TX checksum offload on the specified interface. +func EthtoolTXHWCsumSwitch(name string, op uint32) error { if len(name)+1 > IFNAMSIZ { return fmt.Errorf("name '%s' exceeds IFNAMSIZ (%d)", name, IFNAMSIZ) } @@ -52,14 +58,14 @@ func EthtoolTXHWCsumOff(name string) error { value := ethtoolValue{ Cmd: ETHTOOL_STXCSUM, - Data: 0, + Data: op, } request := ifReq{ Data: uintptr(unsafe.Pointer(&value)), } copy(request.Name[:], []byte(name)) - // We perform the call unconditionally: if TX checksum offload is already disabled the call + // We perform the call unconditionally: if TX checksum offload is already as expected the call // will be a no-op and there will be no error. if _, _, errno := syscall.RawSyscall( syscall.SYS_IOCTL, diff --git a/pkg/agent/util/ethtool/ethtool_windows.go b/pkg/agent/util/ethtool/ethtool_windows.go new file mode 100644 index 00000000000..3101e6bf391 --- /dev/null +++ b/pkg/agent/util/ethtool/ethtool_windows.go @@ -0,0 +1,27 @@ +//go:build windows +// +build windows + +// Copyright 2022 Antrea Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package ethtool + +var ( + TXCSUM_ON = uint32(1) + TXCSUM_OFF = uint32(0) +) + +func EthtoolTXHWCsumSwitch(name string, op uint32) error { + return nil +}