Add support for program loading flags #3657
Conversation
Alan-Jowett
requested review from
dthaler,
poornagmsft,
saxena-anurag,
shankarseal,
dv-msft,
gtrevi,
shpalani,
matthewige,
mtfriesen and
rectified95
as code owners
|
Marking as draft until failure is root caused. |
Alan-Jowett
force-pushed
the
issue3576
branch
2 times, most recently
from
3d2c7af
to
a433c52
Compare
|
@dthaler Would you mind taking another look at this? There were some fairly significant changes required to make this work with older extensions (XDP and some internal ones) that weren't correctly checking the version fields. The updated model requires the attach provider to declare that they support the new version of the data structures to prevent breaking older versions that didn't. |
Signed-off-by: Alan Jowett <[email protected]>
|
Tentatively approving though I have two outstanding questions that might warrant changes |
Signed-off-by: Alan Jowett <[email protected]>
Signed-off-by: Alan Jowett (from Dev Box) <[email protected]>
Co-authored-by: Dave Thaler <[email protected]>
| @@ -755,3 +755,19 @@ bpf_prog_test_run_opts(int prog_fd, struct bpf_test_run_opts* opts) | |||
|
|
|||
| return 0; | |||
| } | |||
|
|
|||
| __u32 | |||
There was a problem hiding this comment.
Should this be __u64? and also below in set_flags?
| structure size. | ||
| 2. A field for capabilities was added. Currently only prog_attach_flags is used to represent if the prog_attach_flags field is set. | ||
| 2. Field data_size was added to contain the size of the client data. | ||
| 3. An extension defined prog_attach_flags field was added. |
There was a problem hiding this comment.
Should we add some details about what does prog_attach_flags mean.
| @@ -1174,6 +1174,7 @@ net_ebpf_ext_sock_addr_register_providers() | |||
| _net_ebpf_sock_addr_hook_provider_data[i].bpf_attach_type = | |||
| (bpf_attach_type_t)_net_ebpf_extension_sock_addr_bpf_attach_types[i]; | |||
| _net_ebpf_sock_addr_hook_provider_data[i].link_type = BPF_LINK_TYPE_CGROUP; | |||
| _net_ebpf_sock_addr_hook_provider_data[i].capabilities.support_extension_data_v1 = true; | |||
There was a problem hiding this comment.
question: how do we ensure forward compatibility in such a case? for example, if the extension claims is supports extension_data_v1, but it is deployed on a system with an older eBPF runtime, how do we prevent any crash, etc.
|
Closing in favor of #3763 |
Pull request was closed
Resolves: #3576
Description
This pull request primarily focuses on enhancing the eBPF extension data structures and related functionalities. The changes include the introduction of new fields, versioning of extension data structures, and modifications to improve the handling of program flags.
Key changes:
Changes to eBPF extension data structures:
docs/eBpfExtensions.md: Addedcapabilitiesfield to the struct used for specifying the attach type supported by the extension. Also, introduced versioning for the extension data structure and recommended support for version 1 as version 0 will be deprecated eventually. [1] [2] [3]include/ebpf_extension.h: Introducedebpf_extension_data_v0_tandebpf_extension_data_v1_tto represent version 0 and version 1 of the extension data structure respectively. The version 1 structure includes additional fields for capabilities, data size, and program attach flags. [1] [2]include/ebpf_windows.h: Added macros for version 1 of the eBPF extension data structures and warned about the deprecation of version 1 in a future release.Changes related to program flags:
include/bpf/libbpf.h,libs/api/api_internal.h,libs/api/ebpf_api.cpp,libs/api/libbpf_program.cpp,libs/execution_context/ebpf_core.c,libs/execution_context/ebpf_link.c: Added methods to query and set BPF program flags. Also, modified theebpf_program_tstruct to include aflagsfield. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13] [14] [15]Changes in the
ebpfapi/Source.deffile:bpf_program__flagsandbpf_program__set_flagsto the list of exports. [1] [2]Testing
CI/CD
Documentation
Document new EBPF_ATTACH_CLIENT_DATA_VERSION_V1 structure.
Installation
No.