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

bpf: update dsr flag properly #18041

Merged
merged 1 commit into from
Dec 14, 2021
Merged

Conversation

Inode1
Copy link

@Inode1 Inode1 commented Nov 29, 2021

I reproduced this issue and after this patch new tcp sessions with dsr flag update the already created entries in the conntrack table.

Fix TCP connectivity issues in the DSR mode when conntrack entries with missing DSR flag are reused.

Fixes: #17759

@Inode1 Inode1 requested review from a team and joamaki November 29, 2021 13:20
@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 Nov 29, 2021
@pchaigno pchaigno requested a review from brb November 30, 2021 08:34
@pchaigno pchaigno 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 Nov 30, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Nov 30, 2021
@pchaigno
Copy link
Member

pchaigno commented Nov 30, 2021

/test

Job 'Cilium-PR-K8s-1.22-kernel-4.19' failed and has not been observed before, so may be related to your PR:

Click to show.

Test Name

K8sServicesTest Checks service across nodes Tests NodePort BPF Tests with direct routing Tests NodePort

Failure Output

FAIL: Request from k8s1 to service http://[fd04::11]:32538 failed

If it is a flake, comment /mlh new-flake Cilium-PR-K8s-1.22-kernel-4.19 so I can create a new GitHub issue to track it.

Job 'Cilium-PR-K8s-1.21-kernel-5.4' hit: #17353 (89.67% similarity)

@Inode1
Copy link
Author

Inode1 commented Nov 30, 2021

/test

@Inode1
Copy link
Author

Inode1 commented Nov 30, 2021

test-1.22-4.19

@Inode1
Copy link
Author

Inode1 commented Dec 1, 2021

/test-1.22-4.19

@Inode1
Copy link
Author

Inode1 commented Dec 1, 2021

@pchaigno Hello, could you recheck this commit please. IMO, based on the logs of failed tests, I don’t think my PR was the cause of these problems.

Copy link
Member

@brb brb left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

A few comments. Also, could you improve the commit msg. In particular, it should describe the problem your commit tries to solve, and then briefly discuss the solution.

I've checked the CI failures - all of them are flakes and unrelated to your changes.

bpf/bpf_lxc.c Outdated Show resolved Hide resolved
bpf/bpf_lxc.c Outdated Show resolved Hide resolved
@Inode1
Copy link
Author

Inode1 commented Dec 3, 2021

@brb, Hello. Here you are, I update the commit msg and describe the problem and the solution.

@brb
Copy link
Member

brb commented Dec 3, 2021

@Inode1 Thanks for the updated commit msg.

@brb
Copy link
Member

brb commented Dec 3, 2021

/test

Job 'Cilium-PR-K8s-1.21-kernel-5.4' failed and has not been observed before, so may be related to your PR:

Click to show.

Test Name

K8sDatapathConfig AutoDirectNodeRoutes Check connectivity with sockops and direct routing

Failure Output

FAIL: Error creating resource /home/jenkins/workspace/Cilium-PR-K8s-1.21-kernel-5.4/src/github.com/cilium/cilium/test/k8sT/manifests/l3-policy-demo.yaml: Cannot retrieve cilium pod cilium-r2hv9 policy revision: cannot get revision from json output 'Defaulted container "cilium-agent" out of: cilium-agent, mount-cgroup (init), clean-cilium-state (init)

If it is a flake, comment /mlh new-flake Cilium-PR-K8s-1.21-kernel-5.4 so I can create a new GitHub issue to track it.

@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.11.0 Dec 3, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.10.6 Dec 3, 2021
@brb brb added sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. sig/loadbalancing labels Dec 3, 2021
bpf/lib/conntrack.h Outdated Show resolved Hide resolved
bpf/lib/conntrack.h Outdated Show resolved Hide resolved
@brb
Copy link
Member

brb commented Dec 8, 2021

/test

@Inode1 Inode1 requested a review from brb December 9, 2021 13:41
@joestringer joestringer added this to Needs backport from master in 1.10.7 Dec 10, 2021
@joestringer joestringer removed this from Needs backport from master in 1.10.6 Dec 10, 2021
@brb
Copy link
Member

brb commented Dec 10, 2021

test-gke

Job 'Cilium-PR-K8s-GKE' hit: #18218 (92.43% similarity)

@Inode1
Copy link
Author

Inode1 commented Dec 13, 2021

@brb, Hello. There are several failed tests, is it somehow connected with my PR? Do you need any help from me?

@brb
Copy link
Member

brb commented Dec 14, 2021

The test failures are unrelated (#18218). Marking as ready to merge.

@brb brb added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Dec 14, 2021
@nbusseneau
Copy link
Member

@Inode1 Thanks for your contribution :) Merging the PR.

@nbusseneau nbusseneau merged commit f6c7825 into cilium:master Dec 14, 2021
@Inode1
Copy link
Author

Inode1 commented Dec 14, 2021

Thanks, good news.

@tklauser tklauser added backport-done/1.11 The backport for Cilium 1.11.x for this PR is done. and removed backport-pending/1.11 labels Dec 17, 2021
@tklauser tklauser moved this from Needs backport from master to Backport pending to v1.10 in 1.10.7 Dec 20, 2021
@tklauser tklauser moved this from Backport pending to v1.10 to Backport done to v1.10 in 1.10.7 Dec 22, 2021
@tklauser tklauser moved this from Needs backport from master to Backport done to v1.11 in 1.11.1 Dec 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-done/1.11 The backport for Cilium 1.11.x for this PR is done. kind/bug This is a bug in the Cilium logic. 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. sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages.
Projects
No open projects
1.10.7
Backport done to v1.10
1.11.1
Backport done to v1.11
Development

Successfully merging this pull request may close these issues.

Some tcp sessions in DSR mode are not SNATed properly
7 participants