crap: introduce cilium raw acquisition of packets #3
Conversation
Co-developed-by: Tomasz Mielech <[email protected]> Co-developed-by: Jarrod Baumann <[email protected]> Signed-off-by: Jarrod Baumann <[email protected]> Signed-off-by: Stanislav Zaikin <[email protected]>
with additional annotation service.cilium.io/raw now it's possible to push raw packets for ExternalIPs w/o NAT. therefore, a custom load-balancer or service can process packets and see all the headers including src ip, dst ip and so on. Signed-off-by: Stanislav Zaikin <[email protected]>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| } | ||
|
|
||
| if endpoint.Networking.NodeIP == nodeIP { | ||
| if ep := cm.endpointManager.LookupIP(addr); ep != nil { |
There was a problem hiding this comment.
Endpoint lookup uses uninitialized address variable
High Severity
The LookupIP(addr) call uses the variable addr before it's assigned a value. The variable addr is declared at line 111 with its zero value, and the parsed IP a is only assigned to addr at line 143, after the lookup. This means the endpoint manager lookup always searches for the zero IP address instead of the actual parsed endpoint IP. As a result, ifindex will never be correctly set for local endpoints, causing raw packet delivery to potentially fail.
| Use: "update", | ||
| Short: "Update CRAP entries", | ||
| Aliases: []string{"add"}, | ||
| Long: vtepPolUpdateUsage, |
There was a problem hiding this comment.
Wrong variable used for command help text
Low Severity
The Long field uses vtepPolUpdateUsage (which contains "Create/Update vtep entry") instead of the locally defined crapUpdateUsage constant. This is a copy-paste error from the VTEP policy update command that causes incorrect help text to be displayed for the CRAP update command.
| if _, ok := annotation.Get(svc, annotation.ServiceRaw); !ok { | ||
| log.DebugContext(ctx, "Skip services w/o RAW annotation") | ||
| return nil | ||
| } |
There was a problem hiding this comment.
CRAP rules not removed when annotation is deleted
Medium Severity
When a service with the ServiceRaw annotation is updated to remove that annotation, addService returns early without adding anything to the diff. This means the service remains in svcDataStore and the CRAP BPF rules persist even though the annotation no longer exists. The service should be explicitly removed from the diff (by setting diff.serviceDiff[svc.UID] = nil) when the annotation is absent on a service that may have been previously tracked.
| return ipv4_local_delivery(ctx, l3_off, SECLABEL_IPV4, MARK_MAGIC_IDENTITY, ip4, ep, | ||
| METRIC_INGRESS, true, false, 0); | ||
| } | ||
| } |
There was a problem hiding this comment.
Missing remote endpoint fallback in overlay CRAP handling
Medium Severity
In bpf_host.c, when a CRAP entry exists but the local endpoint lookup fails, the code falls back to lookup_ip4_remote_endpoint and uses encap_and_redirect_with_nodeid to forward traffic to the correct node. However, bpf_overlay.c lacks this fallback—if the CRAP entry exists but the pod isn't local, traffic silently falls through to normal processing. This inconsistency means CRAP won't work correctly for overlay traffic when the target pod is on a different node.
Additional Locations (1)
| continue | ||
| } | ||
| addrs = append(addrs, addr) | ||
| } |
There was a problem hiding this comment.
Missing IPv4 check causes panic on IPv6 addresses
High Severity
getServiceMetadata parses all ExternalIPs without filtering for IPv4. These addresses are later passed to crap.NewKey() in buildRules, which calls dstIP.As4(). According to Go's netip documentation, As4() panics if the address is IPv6. If a service with the raw annotation has an IPv6 ExternalIP, this will crash the daemon. The code needs to check addr.Is4() before adding addresses to the slice.
with additional annotation service.cilium.io/raw now it's possible to push raw packets for ExternalIPs w/o NAT. therefore, a custom load-balancer or service can process packets and see all the headers including src ip, dst ip and so on.
Please ensure your pull request adheres to the following guidelines:
description and a
Fixes: #XXXline if the commit addresses a particularGitHub issue.
Fixes: <commit-id>tag, thenplease add the commit author[s] as reviewer[s] to this issue.
Fixes: #issue-number
Note
Introduces direct raw routing for ExternalIPs without NAT.
cilium_crap_map(lib/crap.h) and consults it inbpf_host.c,bpf_overlay.c, andbpf_lxc.cto deliver IPv4 packets directly to local endpoints or encap to remote nodes based ondst_ip -> pod_ipmappingpkg/rawmanager watches Services annotated withservice.cilium.io/rawand CiliumEndpoints, computes matches via selectors, and reconciles the BPF map; wired into agent viaraw.Cellannotation.ServiceRawand logs that LB reflector ignores these servicescilium-dbg bpf crap {list,update,delete}for map inspection and managementdaemon/cmd/cells.goto register the new cellWritten by Cursor Bugbot for commit 30d5808. This will update automatically on new commits. Configure here.