WIP: Add interfaces discovery to bpf agent process #408
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #408 +/- ##
==========================================
- Coverage 31.69% 31.16% -0.53%
==========================================
Files 66 66
Lines 7794 7919 +125
==========================================
- Hits 2470 2468 -2
- Misses 5097 5215 +118
- Partials 227 236 +9
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
msherif1234
force-pushed
the
intf-discovery
branch
3 times, most recently
from
88ce99a to
3e82264
Compare
| @@ -80,53 +84,63 @@ type ClBpfApplicationProgram struct { | |||
|
|
|||
| // xdp defines the desired state of the application's XdpPrograms. | |||
| // +unionMember | |||
| // +kubebuilder:validation:MinItems:=1 | |||
There was a problem hiding this comment.
Does this work? They're all optional, and only one can be set, so they can't all have a minimum of 1 item at the same time.
There was a problem hiding this comment.
I think the existing validation takes care of it. Type is required, and then based on the type, one of the programInfo members needs to be set.
| // automatically attaching eBPF hooks to newly discovered interfaces in both directions. | ||
| //+kubebuilder:default:=false | ||
| // +optional | ||
| AutoDiscovery bool `json:"autoDiscovery"` |
There was a problem hiding this comment.
This should probably be called something like "InterfaceAutoDiscovery" to make it clear that it applies to interfaces. Also, will you need some additional configuration interfaces to ignore, or interface name patterns?
There was a problem hiding this comment.
yes we need interface list to ignore at min we can get fancy with regex matching later but I think for GA we need to enable it and make sure the interface is not in the ignore list
| // +optional | ||
| XDPInfo *ClXdpProgramInfo `json:"xdpInfo,omitempty"` | ||
| XDPInfo []ClXdpProgramInfo `json:"xdpInfo,omitempty"` |
There was a problem hiding this comment.
Why are you changing these to slices? As it is currently, the ClBpfApplicationProgram struct represents a single program of one of the types, and then []Programs in ClBpfApplicationSpec contains the list of programs in the application.
There was a problem hiding this comment.
yeah scratch that no need to list here we have interface lists in each type will revert that
There was a problem hiding this comment.
That could be done, but it would be a redesign of the existing model and logic. Currently, each program has a list of links that define where it will be attached. The interface-related programs have an Interface selector. I was thinking that we would extend the interface selector with an "auto-discover" option. Alternatively, it could be a separate entry in Cl*AttachInfo. Then, rather having to spell out each interface in the selector, it would trigger the auto-discovery process to figure it out. Then, the auto-discovery process would be responsible for populating some entries in Links []Cl*AttachInfoState. We'd need to identify which of these Link entries came from the standard Interface selector, and which came from the auto-discovery because they'd need to be processed a little differently. Alternatively, we could have another slice called AutoLinks []Cl*AttachInfoState for the links managed by auto discovery.
There was a problem hiding this comment.
One way or another, I think the auto-discovery process will need to be able to populate a list of links for xdp, tc, and tcx programs based on the interfaces discovered. For each interface discovered, we will need a single link for each program that needs to be attached to the given interface.
msherif1234
force-pushed
the
intf-discovery
branch
5 times, most recently
from
cbc0cd8 to
ea9ef1e
Compare
|
This looks good to me with the modifications. As a reminder, we'll need to implement #411 before you can attach the programs. |
msherif1234
force-pushed
the
intf-discovery
branch
6 times, most recently
from
2df77e6 to
8bc95d1
Compare
Signed-off-by: Mohamed Mahmoud <mmahmoud@redhat.com>
msherif1234
force-pushed
the
intf-discovery
branch
from
8bc95d1 to
7a50c29
Compare
No description provided.