Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make disabling TX checksum offload configurable #3832

Merged
merged 1 commit into from
May 27, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions build/charts/antrea/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ Kubernetes: `>= 1.16.0-0`
| controller.selfSignedCert | bool | `true` | Indicates whether to use auto-generated self-signed TLS certificates. If false, a Secret named "antrea-controller-tls" must be provided with the following keys: ca.crt, tls.crt, tls.key. |
| controller.tolerations | list | `[{"key":"CriticalAddonsOnly","operator":"Exists"},{"effect":"NoSchedule","key":"node-role.kubernetes.io/master"},{"effect":"NoSchedule","key":"node-role.kubernetes.io/control-plane"}]` | Tolerations for the antrea-controller Pod. |
| defaultMTU | int | `0` | Default MTU to use for the host gateway interface and the network interface of each Pod. By default, antrea-agent will discover the MTU of the Node's primary interface and adjust it to accommodate for tunnel encapsulation overhead if applicable. |
| disableTXChecksumOffload | bool | `false` | Disable TX checksum offloading for container network interfaces. It's supposed to be set to true when the datapath doesn't support TX checksum offloading, which causes packets to be dropped due to bad checksum. It affects Pods running on Linux Nodes only. |
| egress.exceptCIDRs | list | `[]` | CIDR ranges to which outbound Pod traffic will not be SNAT'd by Egresses. |
| enableBridgingMode | bool | `false` | Enable bridging mode of Pod network on Nodes, in which the Node's transport interface is connected to the OVS bridge. |
| featureGates | object | `{}` | To explicitly enable or disable a FeatureGate and bypass the Antrea defaults, add an entry to the dictionary with the FeatureGate's name as the key and a boolean as the value. |
Expand Down
5 changes: 5 additions & 0 deletions build/charts/antrea/conf/antrea-agent.conf
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,11 @@ trafficEncryptionMode: {{ .Values.trafficEncryptionMode | quote }}
# `trafficEncapMode` is `noEncap`, and `noSNAT` is true.
enableBridgingMode: {{ .Values.enableBridgingMode }}

# Disable TX checksum offloading for container network interfaces. It's supposed to be set to true when the
# datapath doesn't support TX checksum offloading, which causes packets to be dropped due to bad checksum.
# It affects Pods running on Linux Nodes only.
disableTXChecksumOffload: {{ .Values.disableTXChecksumOffload }}

# Default MTU to use for the host gateway interface and the network interface of each Pod.
# If omitted, antrea-agent will discover the MTU of the Node's primary interface and
# also adjust MTU to accommodate for tunnel encapsulation overhead (if applicable).
Expand Down
5 changes: 5 additions & 0 deletions build/charts/antrea/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@ trafficEncryptionMode: "none"
# -- Enable bridging mode of Pod network on Nodes, in which the Node's transport
# interface is connected to the OVS bridge.
enableBridgingMode: false
# -- Disable TX checksum offloading for container network interfaces. It's
# supposed to be set to true when the datapath doesn't support TX checksum
# offloading, which causes packets to be dropped due to bad checksum. It affects
# Pods running on Linux Nodes only.
disableTXChecksumOffload: false
# -- Whether or not to SNAT (using the Node IP) the egress traffic from a Pod to
# the external network.
noSNAT: false
Expand Down
9 changes: 7 additions & 2 deletions build/yamls/antrea-aks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,11 @@ data:
# `trafficEncapMode` is `noEncap`, and `noSNAT` is true.
enableBridgingMode: false

# Disable TX checksum offloading for container network interfaces. It's supposed to be set to true when the
# datapath doesn't support TX checksum offloading, which causes packets to be dropped due to bad checksum.
# It affects Pods running on Linux Nodes only.
disableTXChecksumOffload: false

# Default MTU to use for the host gateway interface and the network interface of each Pod.
# If omitted, antrea-agent will discover the MTU of the Node's primary interface and
# also adjust MTU to accommodate for tunnel encapsulation overhead (if applicable).
Expand Down Expand Up @@ -3491,7 +3496,7 @@ spec:
kubectl.kubernetes.io/default-container: antrea-agent
# Automatically restart Pods with a RollingUpdate if the ConfigMap changes
# See https://helm.sh/docs/howto/charts_tips_and_tricks/#automatically-roll-deployments
checksum/config: 0cc20edc3fc882f0ea9bd3450fbab504858feeff47e1d3f09d8f6ebacd741dbe
checksum/config: fd449f30e949fff2d22ed79bca0a040535429c5b605b7b93dfdbfd3b359115ae
labels:
app: antrea
component: antrea-agent
Expand Down Expand Up @@ -3731,7 +3736,7 @@ spec:
annotations:
# Automatically restart Pod if the ConfigMap changes
# See https://helm.sh/docs/howto/charts_tips_and_tricks/#automatically-roll-deployments
checksum/config: 0cc20edc3fc882f0ea9bd3450fbab504858feeff47e1d3f09d8f6ebacd741dbe
checksum/config: fd449f30e949fff2d22ed79bca0a040535429c5b605b7b93dfdbfd3b359115ae
labels:
app: antrea
component: antrea-controller
Expand Down
9 changes: 7 additions & 2 deletions build/yamls/antrea-eks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,11 @@ data:
# `trafficEncapMode` is `noEncap`, and `noSNAT` is true.
enableBridgingMode: false

# Disable TX checksum offloading for container network interfaces. It's supposed to be set to true when the
# datapath doesn't support TX checksum offloading, which causes packets to be dropped due to bad checksum.
# It affects Pods running on Linux Nodes only.
disableTXChecksumOffload: false

# Default MTU to use for the host gateway interface and the network interface of each Pod.
# If omitted, antrea-agent will discover the MTU of the Node's primary interface and
# also adjust MTU to accommodate for tunnel encapsulation overhead (if applicable).
Expand Down Expand Up @@ -3491,7 +3496,7 @@ spec:
kubectl.kubernetes.io/default-container: antrea-agent
# Automatically restart Pods with a RollingUpdate if the ConfigMap changes
# See https://helm.sh/docs/howto/charts_tips_and_tricks/#automatically-roll-deployments
checksum/config: 0cc20edc3fc882f0ea9bd3450fbab504858feeff47e1d3f09d8f6ebacd741dbe
checksum/config: fd449f30e949fff2d22ed79bca0a040535429c5b605b7b93dfdbfd3b359115ae
labels:
app: antrea
component: antrea-agent
Expand Down Expand Up @@ -3733,7 +3738,7 @@ spec:
annotations:
# Automatically restart Pod if the ConfigMap changes
# See https://helm.sh/docs/howto/charts_tips_and_tricks/#automatically-roll-deployments
checksum/config: 0cc20edc3fc882f0ea9bd3450fbab504858feeff47e1d3f09d8f6ebacd741dbe
checksum/config: fd449f30e949fff2d22ed79bca0a040535429c5b605b7b93dfdbfd3b359115ae
labels:
app: antrea
component: antrea-controller
Expand Down
9 changes: 7 additions & 2 deletions build/yamls/antrea-gke.yml
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,11 @@ data:
# `trafficEncapMode` is `noEncap`, and `noSNAT` is true.
enableBridgingMode: false

# Disable TX checksum offloading for container network interfaces. It's supposed to be set to true when the
# datapath doesn't support TX checksum offloading, which causes packets to be dropped due to bad checksum.
# It affects Pods running on Linux Nodes only.
disableTXChecksumOffload: false

# Default MTU to use for the host gateway interface and the network interface of each Pod.
# If omitted, antrea-agent will discover the MTU of the Node's primary interface and
# also adjust MTU to accommodate for tunnel encapsulation overhead (if applicable).
Expand Down Expand Up @@ -3491,7 +3496,7 @@ spec:
kubectl.kubernetes.io/default-container: antrea-agent
# Automatically restart Pods with a RollingUpdate if the ConfigMap changes
# See https://helm.sh/docs/howto/charts_tips_and_tricks/#automatically-roll-deployments
checksum/config: 6b6be76fd37d8fdac7783fcd026b6f34e993630c12c339b1dafa99ba5b36cf00
checksum/config: 7e0d8c70d728f9f981756d8238d9b19a9c2321206b09a814a1cdb4ac604b190c
labels:
app: antrea
component: antrea-agent
Expand Down Expand Up @@ -3731,7 +3736,7 @@ spec:
annotations:
# Automatically restart Pod if the ConfigMap changes
# See https://helm.sh/docs/howto/charts_tips_and_tricks/#automatically-roll-deployments
checksum/config: 6b6be76fd37d8fdac7783fcd026b6f34e993630c12c339b1dafa99ba5b36cf00
checksum/config: 7e0d8c70d728f9f981756d8238d9b19a9c2321206b09a814a1cdb4ac604b190c
labels:
app: antrea
component: antrea-controller
Expand Down
9 changes: 7 additions & 2 deletions build/yamls/antrea-ipsec.yml
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,11 @@ data:
# `trafficEncapMode` is `noEncap`, and `noSNAT` is true.
enableBridgingMode: false

# Disable TX checksum offloading for container network interfaces. It's supposed to be set to true when the
# datapath doesn't support TX checksum offloading, which causes packets to be dropped due to bad checksum.
# It affects Pods running on Linux Nodes only.
disableTXChecksumOffload: false

# Default MTU to use for the host gateway interface and the network interface of each Pod.
# If omitted, antrea-agent will discover the MTU of the Node's primary interface and
# also adjust MTU to accommodate for tunnel encapsulation overhead (if applicable).
Expand Down Expand Up @@ -3504,7 +3509,7 @@ spec:
kubectl.kubernetes.io/default-container: antrea-agent
# Automatically restart Pods with a RollingUpdate if the ConfigMap changes
# See https://helm.sh/docs/howto/charts_tips_and_tricks/#automatically-roll-deployments
checksum/config: d289c621cfdc7aee9e8320c0398e76f302591b0adc12156d470320ee9839c073
checksum/config: d5038309e5a226d5d860b167b95e0d8ed55af1914526f52e5ef8600e527695e5
checksum/ipsec-secret: d0eb9c52d0cd4311b6d252a951126bf9bea27ec05590bed8a394f0f792dcb2a4
labels:
app: antrea
Expand Down Expand Up @@ -3780,7 +3785,7 @@ spec:
annotations:
# Automatically restart Pod if the ConfigMap changes
# See https://helm.sh/docs/howto/charts_tips_and_tricks/#automatically-roll-deployments
checksum/config: d289c621cfdc7aee9e8320c0398e76f302591b0adc12156d470320ee9839c073
checksum/config: d5038309e5a226d5d860b167b95e0d8ed55af1914526f52e5ef8600e527695e5
labels:
app: antrea
component: antrea-controller
Expand Down
9 changes: 7 additions & 2 deletions build/yamls/antrea.yml
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,11 @@ data:
# `trafficEncapMode` is `noEncap`, and `noSNAT` is true.
enableBridgingMode: false

# Disable TX checksum offloading for container network interfaces. It's supposed to be set to true when the
# datapath doesn't support TX checksum offloading, which causes packets to be dropped due to bad checksum.
# It affects Pods running on Linux Nodes only.
disableTXChecksumOffload: false
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should say it is on Linux only?


# Default MTU to use for the host gateway interface and the network interface of each Pod.
# If omitted, antrea-agent will discover the MTU of the Node's primary interface and
# also adjust MTU to accommodate for tunnel encapsulation overhead (if applicable).
Expand Down Expand Up @@ -3491,7 +3496,7 @@ spec:
kubectl.kubernetes.io/default-container: antrea-agent
# Automatically restart Pods with a RollingUpdate if the ConfigMap changes
# See https://helm.sh/docs/howto/charts_tips_and_tricks/#automatically-roll-deployments
checksum/config: 976e8c918d8c411df17238dd333a51f9adfdfafe2d6d480d7652f16be02fff3c
checksum/config: 03ee0481c65f3ec8ff9e879ff10c3cf65991a347a7aec1c9bc866b019ec470f1
labels:
app: antrea
component: antrea-agent
Expand Down Expand Up @@ -3731,7 +3736,7 @@ spec:
annotations:
# Automatically restart Pod if the ConfigMap changes
# See https://helm.sh/docs/howto/charts_tips_and_tricks/#automatically-roll-deployments
checksum/config: 976e8c918d8c411df17238dd333a51f9adfdfafe2d6d480d7652f16be02fff3c
checksum/config: 03ee0481c65f3ec8ff9e879ff10c3cf65991a347a7aec1c9bc866b019ec470f1
labels:
app: antrea
component: antrea-controller
Expand Down
1 change: 1 addition & 0 deletions cmd/antrea-agent/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -374,6 +374,7 @@ func run(o *Options) error {
isChaining,
enableBridgingMode,
enableAntreaIPAM,
o.config.DisableTXChecksumOffload,
networkReadyCh)

var cniPodInfoStore cnipodcache.CNIPodInfoStore
Expand Down
8 changes: 5 additions & 3 deletions pkg/agent/cniserver/interface_configuration_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,11 @@ const (
type ifConfigurator struct {
ovsDatapathType ovsconfig.OVSDatapathType
isOvsHardwareOffloadEnabled bool
disableTXChecksumOffload bool
}

func newInterfaceConfigurator(ovsDatapathType ovsconfig.OVSDatapathType, isOvsHardwareOffloadEnabled bool) (*ifConfigurator, error) {
return &ifConfigurator{ovsDatapathType: ovsDatapathType, isOvsHardwareOffloadEnabled: isOvsHardwareOffloadEnabled}, nil
func newInterfaceConfigurator(ovsDatapathType ovsconfig.OVSDatapathType, isOvsHardwareOffloadEnabled bool, disableTXChecksumOffload bool) (*ifConfigurator, error) {
return &ifConfigurator{ovsDatapathType: ovsDatapathType, isOvsHardwareOffloadEnabled: isOvsHardwareOffloadEnabled, disableTXChecksumOffload: disableTXChecksumOffload}, nil
}

func renameLink(curName, newName string) error {
Expand Down Expand Up @@ -260,9 +261,10 @@ func (ic *ifConfigurator) configureContainerLinkVeth(
}
containerIface.Mac = containerVeth.HardwareAddr.String()
hostIface.Mac = hostVeth.HardwareAddr.String()
// Disable TX checksum offloading when it's configured explicitly or the datapath is netdev.
// 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.ovsDatapathType == ovsconfig.OVSDatapathNetdev {
if ic.disableTXChecksumOffload || ic.ovsDatapathType == ovsconfig.OVSDatapathNetdev {
if err := ethtool.EthtoolTXHWCsumOff(containerVeth.Name); err != nil {
return fmt.Errorf("error when disabling TX checksum offload on container veth: %v", err)
}
Expand Down
3 changes: 2 additions & 1 deletion pkg/agent/cniserver/interface_configuration_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ type ifConfigurator struct {
epCache *sync.Map
}

func newInterfaceConfigurator(ovsDatapathType ovsconfig.OVSDatapathType, isOvsHardwareOffloadEnabled bool) (*ifConfigurator, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a comment saying disableTXChecksumOffload is ignored on Windows?

// disableTXChecksumOffload is ignored on Windows.
func newInterfaceConfigurator(ovsDatapathType ovsconfig.OVSDatapathType, isOvsHardwareOffloadEnabled bool, disableTXChecksumOffload bool) (*ifConfigurator, error) {
hnsNetwork, err := hcsshim.GetHNSNetworkByName(util.LocalHNSNetwork)
if err != nil {
return nil, err
Expand Down
3 changes: 2 additions & 1 deletion pkg/agent/cniserver/pod_configuration.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,9 @@ func newPodConfigurator(
isOvsHardwareOffloadEnabled bool,
podUpdateNotifier channel.Notifier,
podInfoStore cnipodcache.CNIPodInfoStore,
disableTXChecksumOffload bool,
) (*podConfigurator, error) {
ifConfigurator, err := newInterfaceConfigurator(ovsDatapathType, isOvsHardwareOffloadEnabled)
ifConfigurator, err := newInterfaceConfigurator(ovsDatapathType, isOvsHardwareOffloadEnabled, disableTXChecksumOffload)
if err != nil {
return nil, err
}
Expand Down
6 changes: 4 additions & 2 deletions pkg/agent/cniserver/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ type CNIServer struct {
enableBridgingMode bool
// Enable AntreaIPAM for secondary networks implementd by other CNIs.
enableSecondaryNetworkIPAM bool
disableTXChecksumOffload bool
secondaryNetworkEnabled bool
// networkReadyCh notifies that the network is ready so new Pods can be created. Therefore, CmdAdd waits for it.
networkReadyCh <-chan struct{}
Expand Down Expand Up @@ -623,7 +624,7 @@ func New(
nodeConfig *config.NodeConfig,
kubeClient clientset.Interface,
routeClient route.Interface,
isChaining, enableBridgingMode, enableSecondaryNetworkIPAM bool,
isChaining, enableBridgingMode, enableSecondaryNetworkIPAM, disableTXChecksumOffload bool,
networkReadyCh <-chan struct{},
) *CNIServer {
return &CNIServer{
Expand All @@ -637,6 +638,7 @@ func New(
routeClient: routeClient,
isChaining: isChaining,
enableBridgingMode: enableBridgingMode,
disableTXChecksumOffload: disableTXChecksumOffload,
enableSecondaryNetworkIPAM: enableSecondaryNetworkIPAM,
networkReadyCh: networkReadyCh,
}
Expand All @@ -660,7 +662,7 @@ func (s *CNIServer) Initialize(
s.podConfigurator, err = newPodConfigurator(
ovsBridgeClient, ofClient, s.routeClient, ifaceStore, s.nodeConfig.GatewayConfig.MAC,
ovsBridgeClient.GetOVSDatapathType(), ovsBridgeClient.IsHardwareOffloadEnabled(), podUpdateNotifier,
podInfoStore,
podInfoStore, s.disableTXChecksumOffload,
)
if err != nil {
return fmt.Errorf("error during initialize podConfigurator: %v", err)
Expand Down
6 changes: 3 additions & 3 deletions pkg/agent/cniserver/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -495,7 +495,7 @@ func TestValidatePrevResult(t *testing.T) {
cniConfig.Netns = "invalid_netns"
sriovVFDeviceID := ""
prevResult.Interfaces = []*current.Interface{hostIface, containerIface}
cniServer.podConfigurator, _ = newPodConfigurator(nil, nil, nil, nil, nil, "", false, channel.NewSubscribableChannel("PodUpdate", 100), nil)
cniServer.podConfigurator, _ = newPodConfigurator(nil, nil, nil, nil, nil, "", false, channel.NewSubscribableChannel("PodUpdate", 100), nil, false)
response := cniServer.validatePrevResult(cniConfig.CniCmdArgs, prevResult, sriovVFDeviceID)
checkErrorResponse(t, response, cnipb.ErrorCode_CHECK_INTERFACE_FAILURE, "")
})
Expand All @@ -506,7 +506,7 @@ func TestValidatePrevResult(t *testing.T) {
cniConfig.Netns = "invalid_netns"
sriovVFDeviceID := "0000:03:00.6"
prevResult.Interfaces = []*current.Interface{hostIface, containerIface}
cniServer.podConfigurator, _ = newPodConfigurator(nil, nil, nil, nil, nil, "", true, channel.NewSubscribableChannel("PodUpdate", 100), nil)
cniServer.podConfigurator, _ = newPodConfigurator(nil, nil, nil, nil, nil, "", true, channel.NewSubscribableChannel("PodUpdate", 100), nil, false)
response := cniServer.validatePrevResult(cniConfig.CniCmdArgs, prevResult, sriovVFDeviceID)
checkErrorResponse(t, response, cnipb.ErrorCode_CHECK_INTERFACE_FAILURE, "")
})
Expand Down Expand Up @@ -626,7 +626,7 @@ func TestRemoveInterface(t *testing.T) {
ifaceStore := interfacestore.NewInterfaceStore()
routeMock := routetest.NewMockInterface(controller)
gwMAC, _ := net.ParseMAC("00:00:11:11:11:11")
podConfigurator, err := newPodConfigurator(mockOVSBridgeClient, mockOFClient, routeMock, ifaceStore, gwMAC, "system", false, channel.NewSubscribableChannel("PodUpdate", 100), nil)
podConfigurator, err := newPodConfigurator(mockOVSBridgeClient, mockOFClient, routeMock, ifaceStore, gwMAC, "system", false, channel.NewSubscribableChannel("PodUpdate", 100), nil, false)
require.Nil(t, err, "No error expected in podConfigurator constructor")

containerMAC, _ := net.ParseMAC("aa:bb:cc:dd:ee:ff")
Expand Down
4 changes: 4 additions & 0 deletions pkg/config/agent/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,10 @@ type AgentConfig struct {
// IPv4 and Linux Nodes, and can be enabled only when `ovsDatapathType` is `system`,
// `trafficEncapMode` is `noEncap`, and `noSNAT` is true.
EnableBridgingMode bool `yaml:"enableBridgingMode,omitempty"`
// Disable TX checksum offloading for container network interfaces. It's supposed to be set to true when the
// datapath doesn't support TX checksum offloading, which causes packets to be dropped due to bad checksum.
// It affects Pods running on Linux Nodes only.
DisableTXChecksumOffload bool `yaml:"disableTXChecksumOffload,omitempty"`
// APIPort is the port for the antrea-agent APIServer to serve on.
// Defaults to 10350.
APIPort int `yaml:"apiPort,omitempty"`
Expand Down
4 changes: 2 additions & 2 deletions test/integration/agent/cniserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -574,7 +574,7 @@ func newTester() *cmdAddDelTester {
testNodeConfig,
k8sFake.NewSimpleClientset(),
routeMock,
false, false, false,
false, false, false, false,
tester.networkReadyCh)
tester.server.Initialize(ovsServiceMock, ofServiceMock, ifaceStore, channel.NewSubscribableChannel("PodUpdate", 100), nil)
ctx := context.Background()
Expand Down Expand Up @@ -738,7 +738,7 @@ func setupChainTest(
testNodeConfig,
k8sFake.NewSimpleClientset(),
routeMock,
true, false, false,
true, false, false, false,
networkReadyCh)
} else {
server = inServer
Expand Down