> >>>> > >>>> On 6/24/22 13:23, Ciara Loftus wrote: > >>>>> libbpf v0.8.0 deprecates the bpf_get_link_xdp_id and > >> bpf_set_link_xdp_fd > >>>>> functions. Use meson to detect if libbpf >= v0.7.0 is linked and if so, > use > >>>>> the recommended replacement functions bpf_xdp_query_id, > >>>> bpf_xdp_attach > >>>>> and bpf_xdp_detach which are available to use since libbpf v0.7.0. > >>>>> > >>>>> Also prevent linking with libbpf versions > v0.8.0. > >>>>> > >>>>> Signed-off-by: Ciara Loftus <ciara.lof...@intel.com> > >>>>> --- > >>>>> doc/guides/nics/af_xdp.rst | 3 ++- > >>>>> drivers/net/af_xdp/compat.h | 36 > >>>> ++++++++++++++++++++++++++++- > >>>>> drivers/net/af_xdp/meson.build | 7 ++---- > >>>>> drivers/net/af_xdp/rte_eth_af_xdp.c | 19 +++------------ > >>>>> 4 files changed, 42 insertions(+), 23 deletions(-) > >>>> > >>>> Don't we need to mention these changes in release notes? > >>>> > >>>>> > >>>>> diff --git a/doc/guides/nics/af_xdp.rst b/doc/guides/nics/af_xdp.rst > >>>>> index 56681c8365..9edb48df67 100644 > >>>>> --- a/doc/guides/nics/af_xdp.rst > >>>>> +++ b/doc/guides/nics/af_xdp.rst > >>>>> @@ -43,7 +43,8 @@ Prerequisites > >>>>> This is a Linux-specific PMD, thus the following prerequisites > >>>>> apply: > >>>>> > >>>>> * A Linux Kernel (version > v4.18) with XDP sockets configuration > >> enabled; > >>>>> -* Both libxdp >=v1.2.2 and libbpf libraries installed, or, libbpf > <=v0.6.0 > >>>>> +* Both libxdp >=v1.2.2 and libbpf <=v0.8.0 libraries installed, or, > libbpf > >>>>> + <=v0.6.0. > >>>>> * If using libxdp, it requires an environment variable called > >>>>> LIBXDP_OBJECT_PATH to be set to the location of where libxdp > >> placed its > >>>> bpf > >>>>> object files. This is usually in /usr/local/lib/bpf or > /usr/local/lib64/bpf. > >>>>> diff --git a/drivers/net/af_xdp/compat.h > >> b/drivers/net/af_xdp/compat.h > >>>>> index 28ea64aeaa..8f4ac8b5ea 100644 > >>>>> --- a/drivers/net/af_xdp/compat.h > >>>>> +++ b/drivers/net/af_xdp/compat.h > >>>>> @@ -60,7 +60,7 @@ tx_syscall_needed(struct xsk_ring_prod *q > >>>> __rte_unused) > >>>>> } > >>>>> #endif > >>>>> > >>>>> -#ifdef RTE_NET_AF_XDP_LIBBPF_OBJ_OPEN > >>>>> +#ifdef RTE_NET_AF_XDP_LIBBPF_V070 > >>>> > >>>> Typically version-based checks are considered as bad. Isn't it > >>>> better use feature-based checks/defines? > >>> > >>> Hi Andrew, > >>> > >>> Thank you for the feedback. Is the feature-based checking something > that > >> we can push to the next release? > >>> > >>> We are already using the pkg-config version-check method for other > >> libraries/features in the meson.build file: > >>> * libxdp >= v1.2.2 # earliest compatible libxdp release > >>> * libbpf >= v0.7.0 # bpf_object__* functions > >>> * libbpf >= v0.2.0 # shared umem feature > >>> > >>> If we change to your suggested method I think we should change them > all > >> in one patch. IMO it's probably too close to the release to change them all > >> right now. What do you think? > >>> > >>> Thanks, > >>> Ciara > >> > >> Hi Ciara, > >> > >> yes, ideally we should avoid usage of version-based check everywhere, > >> but I don't think that it is critical to switch at once. We can use it > >> for new checks right now and rewrite old/existing checks a bit later in > >> the next release. > >> > >> Please, note that my notes are related to review notes from Thomas who > >> asked by file_library() method is removed. Yes, it is confusing and it > >> is better to avoid it. Usage of feature-based checks would allow to > >> preserve find_library() as well. > > > > Thank you for the explanation. > > In this case we want to check that the libbpf library is <=v0.8.0. At this > moment in time v0.8.0 is the latest version of libbpf so we cannot check for a > symbol that tells us the library is > v0.8.0. Can you think of a way to > approach > this without using the pkg-config version check method? > > > > I've introduced this check to future-proof the PMD and ensure we only > ever link with versions of libbpf that we've validated to be compatible with > the PMD. When say v0.9.0 is released we can patch the PMD allowing for > libbpf <= v0.9.0 and make any necessary API changes as part of that patch. > This should hopefully help avoid the scenario Thomas encountered. > > Personally I'd consider such checks which limit version as a drawback. > I think checks on build should not be used to reject future versions. > Otherwise, introduction of any further even minor version would require > a patch to allow it. Documentation is the place for information about > validated versions. Build should not enforce it.
Got it. I'll submit a v2 which removes the version-limiting and reinstates the cc.find_library() method. I'll update the documentation to indicate only versions up to v0.8.0 are supported and add a note to the release notes. Although if it's too late in the release cycle we can postpone this patch until after, and simply patch the docs stating that only libbpf <=v0.7.0 is supported for now? Next release we can move away from the pkg-config version-checking method which already exists for other features, and replace with the symbol checking method. Thanks, Ciara
