Skip to content

Commit

Permalink
Make disabling TX checksum for Antrea gateway
Browse files Browse the repository at this point in the history
This is a supplement to PR antrea-io#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 <lhongliang@vmware.com>
  • Loading branch information
hongliangl committed Jun 30, 2022
1 parent 0aad945 commit 4ac634f
Show file tree
Hide file tree
Showing 5 changed files with 87 additions and 39 deletions.
3 changes: 2 additions & 1 deletion cmd/antrea-agent/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
80 changes: 47 additions & 33 deletions pkg/agent/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand All @@ -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,
}
}

Expand Down Expand Up @@ -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{}
Expand Down
2 changes: 1 addition & 1 deletion pkg/agent/cniserver/interface_configuration_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand Down
14 changes: 10 additions & 4 deletions pkg/agent/util/ethtool/ethtool_linux.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
//go:build linux
// +build linux

// Copyright 2019 Antrea Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
Expand All @@ -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)
Expand All @@ -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)
}
Expand All @@ -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,
Expand Down
27 changes: 27 additions & 0 deletions pkg/agent/util/ethtool/ethtool_windows.go
Original file line number Diff line number Diff line change
@@ -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
}

0 comments on commit 4ac634f

Please sign in to comment.