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

cni-server: clear iptables mark before doing masquerade #2919

Merged
merged 2 commits into from
Jun 8, 2023

Conversation

zhangzujian
Copy link
Member

@zhangzujian zhangzujian commented Jun 8, 2023

What type of this PR

  • Bug fixes

Which issue(s) this PR fixes:

Fixes bad checksum while accessing pods running on other nodes via service:

15:17:04.808484 IP (tos 0x0, ttl 64, id 36415, offset 0, flags [DF], proto UDP (17), length 170)
    192.168.129.203.18647 > 192.168.129.205.6081: [bad udp cksum 0x82d8 -> 0xa635!] Geneve, Flags [C], vni 0x3, options [class Open Virtual Networking (OVN) (0x102) type 0x80(C) len 8 data 00010021]
    IP6 (hlim 63, next-header UDP (17) payload length: 72) fd00:100:64::2.41375 > fd00:10:16::24.1053: [udp sum ok] UDP, length 64

WHAT

🤖 Generated by Copilot at bb1b85e

Add a new iptables chain for node port service traffic with local external policy. Refactor some iptables functions and rules in gateway_linux.go.

🤖 Generated by Copilot at bb1b85e

To handle node port traffic well
This pull request adds OvnNodePort shell
It also refactors rules
And some iptables tools
To make them more readable and swell

HOW

🤖 Generated by Copilot at bb1b85e

  • Add a new iptables chain OvnNodePort to handle node port service traffic with external traffic policy set to Local (link, link, link, link, link, link, link, link)
  • Extract a new function createIptablesChain from updateIptablesChain to simplify the logic of ensuring the iptables chain exists (link)
  • Modify updateIptablesChain to use createIptablesChain for both the chain and its parent chain, and return early if the chain already exists (link)
  • Use a short variable declaration for the error variable err in updateIptablesChain (link)
  • Replace the iptables rules that nat packets marked by kube-proxy or kube-ovn with rules that return those packets, since they will be SNATed later by kube-proxy (link, link)
  • Add the --random-fully option to the MASQUERADE rules if the iptables version supports it, which improves the performance and reduces the risk of port exhaustion (link, link)

@zhangzujian zhangzujian added bug Something isn't working need backport labels Jun 8, 2023
@zhangzujian zhangzujian force-pushed the fix-checksum branch 2 times, most recently from f227928 to 1d18e81 Compare June 8, 2023 09:28
@zhangzujian zhangzujian marked this pull request as ready for review June 8, 2023 10:59
@zhangzujian zhangzujian requested a review from oilbeater June 8, 2023 10:59
@zhangzujian zhangzujian merged commit dff950b into kubeovn:master Jun 8, 2023
@zhangzujian zhangzujian deleted the fix-checksum branch June 8, 2023 11:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working need backport
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants