Skip to content

Conversation

borkmann
Copy link
Member

(see individual commit msgs)

borkmann added 2 commits May 30, 2025 09:00
This is just forward-porting commit d3c80fa ("SocketLB: Terminate
connections for services with mixed protocols") given it has been missing
in the new control plane.

Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
filterAndDestroyUDPSockets has family as an argument, but iterateNetlinkSockets
kept hard-coding syscall.AF_INET as input. Pass the right family along instead
of the latter. It seems this was an oversight from refactoring in commit
c7760bd ("sockets: use general purpose iterateNetlinkSockets function.")

Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
@borkmann borkmann requested review from joamaki and tommyp1ckles May 30, 2025 09:29
@borkmann borkmann requested a review from a team as a code owner May 30, 2025 09:29
@borkmann borkmann added area/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. release-note/misc This PR makes changes that have no direct user impact. labels May 30, 2025
@borkmann
Copy link
Member Author

/test

@@ -184,7 +184,7 @@ func terminateUDPConnectionsToBackend(p socketTerminationParams, l3n4Addr lb.L3n
l4Addr := l3n4Addr.L4Addr

switch l3n4Addr.Protocol {
case lb.UDP:
case lb.UDP, lb.ANY:
Copy link
Contributor

Choose a reason for hiding this comment

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

lb.ANY should be impossible with the new control-plane as at least currently you can't turn off "protocol differentiation". But good to add this still as we might still end up adding the ability to turn it off...

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah ok, but what happens when people upgrade and they still have old ANY entries in the maps where then the backend goes away?

Copy link
Contributor

Choose a reason for hiding this comment

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

Those are removed by the reconciler once the maps have been populated with the new entries.

Copy link
Contributor

Choose a reason for hiding this comment

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

Those are removed by the reconciler once the maps have been populated with the new entries.

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jun 2, 2025
@borkmann borkmann merged commit 4c873e1 into main Jun 3, 2025
363 of 366 checks passed
@borkmann borkmann deleted the pr/termination-fixes branch June 3, 2025 07:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants