Skip to content

bpf: correct comments in cil_from_netdev function#43864

Merged
joestringer merged 1 commit intocilium:mainfrom
liyihuang:pr/liyih/update_from-netdev_comment
Feb 6, 2026
Merged

bpf: correct comments in cil_from_netdev function#43864
joestringer merged 1 commit intocilium:mainfrom
liyihuang:pr/liyih/update_from-netdev_comment

Conversation

@liyihuang
Copy link
Copy Markdown
Contributor

Removing conditions from the comment block describing the cil_from_netdev function since the logic has been changed.

with the current codebase, we dont have the condition to attach the cil_from_netdev for certain features.

In my test environment I dont have BPF Nodeport, Host firewall nor L2 announcements.

KubeProxyReplacement Details:
  Status:               False
  Socket LB:            Disabled
  Socket LB Tracing:    Disabled
  Socket LB Coverage:   Full
  Session Affinity:     Enabled
  NAT46/64 Support:     Disabled
  Services:
  - ClusterIP:      Enabled                                                                                                                                                                  - NodePort:       Disabled
  - LoadBalancer:   Disabled
  - externalIPs:    Disabled
  - HostPort:       Disabled

Host firewall:          Disabled


kn logs cilium-9ch6s | grep l2
time=2026-01-18T19:15:00.128901398Z level=info msg="  --enable-l2-announcements='false'"
time=2026-01-18T19:15:00.128904315Z level=info msg="  --enable-l2-neigh-discovery='false'"
time=2026-01-18T19:15:00.128907898Z level=info msg="  --enable-l2-pod-announcements='false'"

but I still have

root@kind-worker3:/home/cilium# bpftool net list
xdp:

tc:
eth0(13) clsact/ingress cil_from_netdev-eth0 id 1266

when I check the loader, I can see it's just going through the devices without condition like others

https://github.com/liyihuang/cilium/blob/db2182352aae62a0178a1e3c1f858e570c847bb8/pkg/datapath/loader/host.go#L254-L257

@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 Jan 18, 2026
Removed conditions from the comment block describing the cil_from_netdev function since the logic has been changed

Signed-off-by: Liyi Huang <liyi.huang@isovalent.com>
@liyihuang liyihuang force-pushed the pr/liyih/update_from-netdev_comment branch from db21823 to f4a38bf Compare January 18, 2026 22:35
@liyihuang liyihuang 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 Jan 18, 2026
@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 Jan 18, 2026
@liyihuang
Copy link
Copy Markdown
Contributor Author

/test

@liyihuang liyihuang marked this pull request as ready for review January 19, 2026 03:23
@liyihuang liyihuang requested a review from a team as a code owner January 19, 2026 03:23
@liyihuang liyihuang requested a review from ldelossa January 19, 2026 03:23
Copy link
Copy Markdown
Member

@joestringer joestringer left a comment

Choose a reason for hiding this comment

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

I agree. This is not the place to document when the program is attached, exactly because it can get out-of-date like this. At best this comment should instead refer to the structure or function that determines when the program should be attached.

@joestringer joestringer added this pull request to the merge queue Feb 6, 2026
Merged via the queue into cilium:main with commit 74c333d Feb 6, 2026
80 checks passed
@liyihuang liyihuang deleted the pr/liyih/update_from-netdev_comment branch February 6, 2026 19:55
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. 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