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

datapath: bigtcp: Fix the IPv4 BIG TCP may not work #26336

Merged
merged 1 commit into from Jun 20, 2023
Merged

datapath: bigtcp: Fix the IPv4 BIG TCP may not work #26336

merged 1 commit into from Jun 20, 2023

Conversation

haiyuewa
Copy link
Contributor

@haiyuewa haiyuewa commented Jun 17, 2023

The kernel will also update the IPv4 GRO/GSO setting if the new value of
"gso/gro_max_size" isn't greater than 65536:

  1. ip link set dev ens787np0 gso_max_size 130000
  2. ip link set dev ens787np0 gso_ipv4_max_size 128000
  3. ip -d -j link show dev ens787np0 | jq -c '.[0].gso_ipv4_max_size'
    128000
  4. ip link set dev ens787np0 gso_max_size 65536
  5. ip -d -j link show dev ens787np0 | jq -c '.[0].gso_ipv4_max_size'
    65536

Cilium will call IPv4 GRO/GSO setup firstly, then call IPv6. To handle the
above issue, setup the IPv6 GRO/GSO firstly, then IPv4 GRO/GSO.

@haiyuewa haiyuewa requested review from a team as code owners June 17, 2023 19:25
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Jun 17, 2023
@github-actions github-actions bot added the kind/community-contribution This was a contribution made by a community member. label Jun 17, 2023
@haiyuewa haiyuewa changed the title BIG TCP datapath: bigtcp: Fix the IPv4 BIG TCP may not work Jun 19, 2023
@ti-mo ti-mo requested a review from borkmann June 20, 2023 13:07
@ti-mo ti-mo added kind/bug This is a bug in the Cilium logic. release-note/bug This PR fixes an issue in a previous release of Cilium. labels Jun 20, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot removed dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Jun 20, 2023
test/bigtcp/test.sh Outdated Show resolved Hide resolved
The kernel will also update the IPv4 GRO/GSO setting if the new value of
"gso/gro_max_size" isn't greater than 65536:

1. Enable IPv4 and IPv6 BIG TCP firstly:
  a). Dump the agent's log:
	$ kubectl -n kube-system logs cilium-tfhz5  2>&1 | grep "big-tcp"
	level=info msg="  --enable-ipv4-big-tcp='true'" subsys=daemon
	level=info msg="  --enable-ipv6-big-tcp='true'" subsys=daemon
	level=info msg="Setting up BIG TCP" subsys=big-tcp
	level=info msg="Setting IPv4 gso_max_size to 131072 and gro_max_size to 131072" device=enp0 subsys=big-tcp
	level=info msg="Setting IPv6 gso_max_size to 131072 and gro_max_size to 131072" device=enp0 subsys=big-tcp

  b). Check the GSO value on the host's net device:
	$ ip -d -j link show dev enp0 | jq -c '.[0].gso_max_size'
	131072

	$ ip -d -j link show dev enp0 | jq -c '.[0].gso_ipv4_max_size'
	131072

2. Then re-install the cilium by enabling IPv4 BIG TCP only:
  a). Dump the agent's log:
	$ kubectl -n kube-system logs cilium-zwpg6  2>&1 | grep "big-tcp"
	level=info msg="  --enable-ipv4-big-tcp='true'" subsys=daemon
	level=info msg="  --enable-ipv6-big-tcp='false'" subsys=daemon
	level=info msg="Setting up BIG TCP" subsys=big-tcp
	level=info msg="Setting IPv4 gso_max_size to 131072 and gro_max_size to 131072" device=enp0 subsys=big-tcp
	level=info msg="Setting IPv6 gso_max_size to 65536 and gro_max_size to 65536" device=enp0 subsys=big-tcp

  b). Check the GSO value on the host's net device:
	$ ip -d -j link show dev enp0 | jq -c '.[0].gso_max_size'
	65536

	$ip -d -j link show dev enp0 | jq -c '.[0].gso_ipv4_max_size'
	65536

No errors about the BIG TCP setting in cilium agent's log, but the value
of net device's '{gso,gro}_ipv4_max_size' is wrong.

So it needs to handle the IPv6 BIG TCP setting firstly, then IPv4.

Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
Link: https://lore.kernel.org/netdev/7e1f733cc96c7f7658fbf3276a90281b2f37acd1.1674921359.git.lucien.xin@gmail.com/
Copy link
Member

@borkmann borkmann left a comment

Choose a reason for hiding this comment

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

Awesome, looks good to me! Thanks!

@borkmann
Copy link
Member

/test

@borkmann borkmann added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jun 20, 2023
@borkmann borkmann merged commit 66f3359 into cilium:main Jun 20, 2023
59 of 60 checks passed
@haiyuewa haiyuewa deleted the big-tcp branch June 20, 2023 15:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug This is a bug in the Cilium logic. kind/community-contribution This was a contribution made by a community member. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/bug This PR fixes an issue in a previous release of Cilium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants