> > > >>>> > > >>>> 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.
I've submitted an RFC for this feature: http://patches.dpdk.org/project/dpdk/list/?series=24043 I'm starting maternity leave next week so am not in a position to rework it in the near future, but if it is functionality that a community member finds useful perhaps they can pick it up in my absence. Thanks, Ciara > > Thanks, > Ciara
