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

Fix pkt mark conflict between HostLocalSourceMark and SNATIPMark #3430

Merged
merged 1 commit into from
Mar 11, 2022

Conversation

tnqn
Copy link
Member

@tnqn tnqn commented Mar 10, 2022

HostLocalSourceMark is used to mark locally generated packets from Node
net namespace, but it conflicted with bits used by SNATIPMark, leading
to these packets being SNATed mistakenly when there happens to be an
Egress using same mark.

Fixes #3431

Signed-off-by: Quan Tian qtian@vmware.com

@tnqn
Copy link
Member Author

tnqn commented Mar 10, 2022

/test-all

@codecov-commenter
Copy link

codecov-commenter commented Mar 10, 2022

Codecov Report

Merging #3430 (a57a660) into main (a51003a) will increase coverage by 17.44%.
The diff coverage is n/a.

❗ Current head a57a660 differs from pull request most recent head 7c13f85. Consider uploading reports for the commit 7c13f85 to get more accurate results

Impacted file tree graph

@@             Coverage Diff             @@
##             main    #3430       +/-   ##
===========================================
+ Coverage   42.43%   59.88%   +17.44%     
===========================================
  Files         200      268       +68     
  Lines       24295    26870     +2575     
===========================================
+ Hits        10310    16090     +5780     
+ Misses      12921     9018     -3903     
- Partials     1064     1762      +698     
Flag Coverage Δ
kind-e2e-tests 45.81% <ø> (?)
unit-tests 42.41% <ø> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pkg/apiserver/handlers/webhook/convert_crd.go 2.56% <0.00%> (ø)
pkg/apiserver/handlers/webhook/mutation_labels.go 24.71% <0.00%> (ø)
pkg/version/version.go 46.15% <0.00%> (ø)
pkg/features/antrea_features.go 16.66% <0.00%> (ø)
pkg/agent/apiserver/handlers/errors.go 0.00% <0.00%> (ø)
pkg/agent/interfacestore/interface_cache.go 77.27% <0.00%> (ø)
pkg/agent/proxy/types/groupcounter.go 93.61% <0.00%> (ø)
pkg/agent/secondarynetwork/ipam/ipam_delegator.go 0.00% <0.00%> (ø)
pkg/signals/signals.go 100.00% <0.00%> (ø)
...gregator/apiserver/handlers/flowrecords/handler.go 0.00% <0.00%> (ø)
... and 177 more

HostLocalSourceMark is used to mark locally generated packets from Node
net namespace, but it conflicted with bits used by SNATIPMark, leading
to these packets being SNATed mistakenly when there happens to be an
Egress using same mark.

Fixes antrea-io#3431

Signed-off-by: Quan Tian <qtian@vmware.com>
@tnqn tnqn requested a review from antoninbas March 10, 2022 13:58
@tnqn tnqn added this to the Antrea v1.6 release milestone Mar 10, 2022
@tnqn tnqn added action/backport Indicates a PR that requires backports. action/release-note Indicates a PR that should be included in release notes. labels Mar 10, 2022
@tnqn
Copy link
Member Author

tnqn commented Mar 10, 2022

/test-all

@tnqn
Copy link
Member Author

tnqn commented Mar 10, 2022

/test-ipv6-all
/test-ipv6-only-all

@antoninbas
Copy link
Contributor

/test-ipv6-only-e2e
/test-ipv6-only-conformance

@antoninbas
Copy link
Contributor

/test-ipv6-e2e

@antoninbas
Copy link
Contributor

@tnqn this test failed twice in a row for jenkins-ipv6-only-e2e
Do you think it could be related?
I checked the job's history in Jenkins and the job seems healthy.

=== RUN   TestWireGuard/testServiceConnectivity
    service_test.go:183: Created service Pod IPs [fd74:ca9b:172:16:1::1545]
    wireguard_test.go:150: 
        	Error Trace:	wireguard_test.go:150
        	            				wireguard_test.go:75
        	Error:      	Received unexpected error:
        	            	nc stdout: <>, stderr: <nc: fd74:ca9b:172:17::e92d ([fd74:ca9b:172:17::e92d]:80): Connection timed out
        	            	nc: fd74:ca9b:172:17::e92d ([fd74:ca9b:172:17::e92d]:80): Connection timed out
        	            	nc: fd74:ca9b:172:17::e92d ([fd74:ca9b:172:17::e92d]:80): Connection timed out
        	            	nc: fd74:ca9b:172:17::e92d ([fd74:ca9b:172:17::e92d]:80): Connection timed out
        	            	nc: fd74:ca9b:172:17::e92d ([fd74:ca9b:172:17::e92d]:80): Connection timed out
        	            	>, err: <command terminated with exit code 1>
        	Test:       	TestWireGuard/testServiceConnectivity
        	Messages:   	Pod hostnetwork-pod should be able to connect the service's ClusterIP [fd74:ca9b:172:17::e92d]:80, but was not able to connect
=== CONT  TestWireGuard
    fixtures.go:269: Exporting test logs to '/var/lib/jenkins/workspace/antrea-ipv6-only-e2e-for-pull-request/antrea-test-logs/TestWireGuard/beforeTeardown.Mar10-20-48-30'
    fixtures.go:375: Error when exporting kubelet logs: error when running journalctl on Node 'antrea-ipv6-2-0', is it available? Error: <nil>
    fixtures.go:396: Deleting 'antrea-test' K8s Namespace
I0310 20:48:45.848635    6521 framework.go:630] Deleting Namespace antrea-test took 6.005761736s
--- FAIL: TestWireGuard (137.90s)
    --- PASS: TestWireGuard/testPodConnectivity (15.19s)
    --- FAIL: TestWireGuard/testServiceConnectivity (58.18s)

@tnqn
Copy link
Member Author

tnqn commented Mar 11, 2022

Checked with @antoninbas, the test failure was introduced by #3336. We will fix it separately. Merging this one to unblock other PRs.

@tnqn tnqn merged commit efca1bb into antrea-io:main Mar 11, 2022
@tnqn tnqn deleted the egress-mark-conflict branch March 11, 2022 01:40
WenzelZ pushed a commit to WenzelZ/antrea that referenced this pull request Jun 20, 2022
…rea-io#3430)

HostLocalSourceMark is used to mark locally generated packets from Node
net namespace, but it conflicted with bits used by SNATIPMark, leading
to these packets being SNATed mistakenly when there happens to be an
Egress using same mark.

Fixes antrea-io#3431

Signed-off-by: Quan Tian <qtian@vmware.com>

(cherry picked from commit 045f342686ad042b2ffae4af3445e6e7d1dd7da1)

(cherry picked from commit c71eb1302d6b4d7c55aaa5331586db3654369897)
WenzelZ pushed a commit to WenzelZ/antrea that referenced this pull request Jun 20, 2022
Fix pkt mark conflict between HostLocalSourceMark and SNATIPMark (antrea-io#3430)

See merge request tos/opensource/antrea!33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action/backport Indicates a PR that requires backports. action/release-note Indicates a PR that should be included in release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Locally generated packets from Node net namespace is SNATed mistakenly
3 participants