cilium, socklb: Add states filter for the termination iteration#1
cilium, socklb: Add states filter for the termination iteration#1MitchLewis930 wants to merge 1 commit intopr_041_beforefrom
Conversation
Add a state filter to the iterator and skip TCP sockets which are in closing or time wait state. There is no need to spend time to iterate these. Technically, there is no harm since when the client app closes the socket and it goes into time wait state, then upon close the socket LB removes the socket from the revnat map in cil_sock_release.. but then again, no need to iterate through these. Suggested-by: Yusuke Suzuki <yusuke.suzuki@isovalent.com> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
There was a problem hiding this comment.
Pull request overview
This PR adds state filtering for socket termination operations to improve efficiency by only iterating over relevant socket states. The changes introduce TCP and UDP state filters that exclude unnecessary states (like TCP_CLOSE and TCP_TIME_WAIT) from termination operations.
Changes:
- Introduced
StateFilterTCPandStateFilterUDPconstants to define which socket states should be considered for termination - Updated
SocketFilterstruct to include aStatesfield for state filtering - Modified termination and socket destruction logic to use the new state filters instead of the hardcoded
0xffffmask
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| pkg/datapath/sockets/sockets.go | Added state filter constants and helper function; updated SocketFilter struct and Destroy function to use state filtering |
| pkg/loadbalancer/reconciler/termination.go | Added states variable assignment based on protocol type and passed it to socket destroyer |
| pkg/datapath/sockets/sockets_test.go | Updated test to include StateFilterUDP in socket filter configuration |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| netlink.TCP_NEW_SYN_REC, | ||
| // Sockets in TCP_LISTEN are moved to closing state | ||
| // (net/ipv4/tcp.c#L4908) | ||
| netlink.TCP_CLOSE, |
There was a problem hiding this comment.
The comment at lines 46-48 states that TCP_CLOSE is excluded from the filter because destroying sockets in this state results in ENOENT. However, TCP_CLOSE is included in the StateFilterTCP mask at line 69. This contradicts the documentation. Either remove TCP_CLOSE from the mask or update the comment to accurately reflect which states are included.
2 participants
PR_041