Re: [PATCH] net/af_xdp: make compatible with libbpf v0.8.0

[Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>]

On 6/27/22 17:17, Loftus, Ciara wrote:
>>
>> 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.loftus@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.

Andrew.