net/af_xdp: make compatible with libbpf v0.8.0
Checks
Commit Message
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(-)
Comments
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?
>
> 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
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.
>
> 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.
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.
Ciara
>
> Andrew.
On 6/27/22 18:24, Loftus, Ciara wrote:
>>
>> 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.
>
> 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.
> >>>>
> >>>> 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.
> >
> > 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
>
> > >>>>
> > >>>> 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.
> > >
> > > 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
On 24/06/2022 11: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>
Hi Andrew/Qi (assuming Ciara is still out of office),
I am seeing a similar issue [1] on 21.11 branch with Fedora 37
(libbpf-0.8.0-2.fc37.x86_64 and libxdp-1.2.6-1.fc37.x86_64).
This patch alone won't apply as there are other dependencies. Looking at
the commits in main branch, it seems like I could take all these [2] to
resolve the issue. With these cherry-picked the build warnings on Fedora
37 are removed.
It's a bit late to take these for DPDK 21.11.3 as I intend to release
later today/tomorrow, so it can be resolved for DPDK 21.11.4.
Do the commits below look ok for backport? Main branch might be able to
demand user uses new libbpf/libxdp versions etc, but with stable we
never want to break the users existing setup when they upgrade from
2X.11.n to 2X.11.n+1.
Let me know what you think?
thanks,
Kevin.
[1] https://paste.centos.org/view/e4eec764
[2]
1eb1846b1a net/af_xdp: make compatible with libbpf 0.8.0
5ff3dbe6ce net/af_xdp: add log on XDP program removal failures
0ed0bc3834 net/af_xdp: avoid version-based check for program load
e024c7e838 net/af_xdp: avoid version-based check for shared UMEM
f76dc44ded net/af_xdp: make clear which libxdp version is required
50b855fc47 net/af_xdp: move XDP library presence flag setting
Hi Kevin,
On 12/20/22 17:05, Kevin Traynor wrote:
> On 24/06/2022 11: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>
>
> Hi Andrew/Qi (assuming Ciara is still out of office),
>
> I am seeing a similar issue [1] on 21.11 branch with Fedora 37
> (libbpf-0.8.0-2.fc37.x86_64 and libxdp-1.2.6-1.fc37.x86_64).
>
> This patch alone won't apply as there are other dependencies. Looking at
> the commits in main branch, it seems like I could take all these [2] to
> resolve the issue. With these cherry-picked the build warnings on Fedora
> 37 are removed.
>
> It's a bit late to take these for DPDK 21.11.3 as I intend to release
> later today/tomorrow, so it can be resolved for DPDK 21.11.4.
>
> Do the commits below look ok for backport? Main branch might be able to
> demand user uses new libbpf/libxdp versions etc, but with stable we
> never want to break the users existing setup when they upgrade from
> 2X.11.n to 2X.11.n+1.
>
> Let me know what you think?
IMO these patches are to to be backported to stable branch.
However, af_xdp maintainers opinion is more important here.
>
> thanks,
> Kevin.
>
> [1] https://paste.centos.org/view/e4eec764
> [2]
> 1eb1846b1a net/af_xdp: make compatible with libbpf 0.8.0
> 5ff3dbe6ce net/af_xdp: add log on XDP program removal failures
> 0ed0bc3834 net/af_xdp: avoid version-based check for program load
> e024c7e838 net/af_xdp: avoid version-based check for shared UMEM
> f76dc44ded net/af_xdp: make clear which libxdp version is required
> 50b855fc47 net/af_xdp: move XDP library presence flag setting
>
On 21/12/2022 06:09, Andrew Rybchenko wrote:
> Hi Kevin,
>
> On 12/20/22 17:05, Kevin Traynor wrote:
>> On 24/06/2022 11: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>
>>
>> Hi Andrew/Qi (assuming Ciara is still out of office),
>>
>> I am seeing a similar issue [1] on 21.11 branch with Fedora 37
>> (libbpf-0.8.0-2.fc37.x86_64 and libxdp-1.2.6-1.fc37.x86_64).
>>
>> This patch alone won't apply as there are other dependencies. Looking at
>> the commits in main branch, it seems like I could take all these [2] to
>> resolve the issue. With these cherry-picked the build warnings on Fedora
>> 37 are removed.
>>
>> It's a bit late to take these for DPDK 21.11.3 as I intend to release
>> later today/tomorrow, so it can be resolved for DPDK 21.11.4.
>>
>> Do the commits below look ok for backport? Main branch might be able to
>> demand user uses new libbpf/libxdp versions etc, but with stable we
>> never want to break the users existing setup when they upgrade from
>> 2X.11.n to 2X.11.n+1.
>>
>> Let me know what you think?
>
> IMO these patches are to to be backported to stable branch.
Thanks Andrew.
> However, af_xdp maintainers opinion is more important here.
>
Qi, what do you think?
>>
>> thanks,
>> Kevin.
>>
>> [1] https://paste.centos.org/view/e4eec764
>> [2]
>> 1eb1846b1a net/af_xdp: make compatible with libbpf 0.8.0
>> 5ff3dbe6ce net/af_xdp: add log on XDP program removal failures
>> 0ed0bc3834 net/af_xdp: avoid version-based check for program load
>> e024c7e838 net/af_xdp: avoid version-based check for shared UMEM
>> f76dc44ded net/af_xdp: make clear which libxdp version is required
>> 50b855fc47 net/af_xdp: move XDP library presence flag setting
>>
>
On 21/12/2022 09:28, Kevin Traynor wrote:
> On 21/12/2022 06:09, Andrew Rybchenko wrote:
>> Hi Kevin,
>>
>> On 12/20/22 17:05, Kevin Traynor wrote:
>>> On 24/06/2022 11: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>
>>>
>>> Hi Andrew/Qi (assuming Ciara is still out of office),
>>>
>>> I am seeing a similar issue [1] on 21.11 branch with Fedora 37
>>> (libbpf-0.8.0-2.fc37.x86_64 and libxdp-1.2.6-1.fc37.x86_64).
>>>
>>> This patch alone won't apply as there are other dependencies. Looking at
>>> the commits in main branch, it seems like I could take all these [2] to
>>> resolve the issue. With these cherry-picked the build warnings on Fedora
>>> 37 are removed.
>>>
>>> It's a bit late to take these for DPDK 21.11.3 as I intend to release
>>> later today/tomorrow, so it can be resolved for DPDK 21.11.4.
>>>
>>> Do the commits below look ok for backport? Main branch might be able to
>>> demand user uses new libbpf/libxdp versions etc, but with stable we
>>> never want to break the users existing setup when they upgrade from
>>> 2X.11.n to 2X.11.n+1.
N.B. ^^^^
>>>
>>> Let me know what you think?
>>
>> IMO these patches are to to be backported to stable branch.
>
> Thanks Andrew.
>
>> However, af_xdp maintainers opinion is more important here.
>>
>
> Qi, what do you think?
>
Hi Qi/Ciara, this issue is still present approaching 21.11.4.
What is your opinion on backporting these patches? Please especially
note the paragraph above wrt users not being required to upgrade libbpf.
thanks,
Kevin.
>>>
>>> thanks,
>>> Kevin.
>>>
>>> [1] https://paste.centos.org/view/e4eec764
>>> [2]
>>> 1eb1846b1a net/af_xdp: make compatible with libbpf 0.8.0
>>> 5ff3dbe6ce net/af_xdp: add log on XDP program removal failures
>>> 0ed0bc3834 net/af_xdp: avoid version-based check for program load
>>> e024c7e838 net/af_xdp: avoid version-based check for shared UMEM
>>> f76dc44ded net/af_xdp: make clear which libxdp version is required
>>> 50b855fc47 net/af_xdp: move XDP library presence flag setting
>>>
>>
>
On 15/03/2023 11:47, Kevin Traynor wrote:
> On 21/12/2022 09:28, Kevin Traynor wrote:
>> On 21/12/2022 06:09, Andrew Rybchenko wrote:
>>> Hi Kevin,
>>>
>>> On 12/20/22 17:05, Kevin Traynor wrote:
>>>> On 24/06/2022 11: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>
>>>>
>>>> Hi Andrew/Qi (assuming Ciara is still out of office),
>>>>
>>>> I am seeing a similar issue [1] on 21.11 branch with Fedora 37
>>>> (libbpf-0.8.0-2.fc37.x86_64 and libxdp-1.2.6-1.fc37.x86_64).
>>>>
>>>> This patch alone won't apply as there are other dependencies. Looking at
>>>> the commits in main branch, it seems like I could take all these [2] to
>>>> resolve the issue. With these cherry-picked the build warnings on Fedora
>>>> 37 are removed.
>>>>
>>>> It's a bit late to take these for DPDK 21.11.3 as I intend to release
>>>> later today/tomorrow, so it can be resolved for DPDK 21.11.4.
>>>>
>>>> Do the commits below look ok for backport? Main branch might be able to
>>>> demand user uses new libbpf/libxdp versions etc, but with stable we
>>>> never want to break the users existing setup when they upgrade from
>>>> 2X.11.n to 2X.11.n+1.
>
> N.B. ^^^^
>
>>>>
>>>> Let me know what you think?
>>>
>>> IMO these patches are to to be backported to stable branch.
>>
>> Thanks Andrew.
>>
>>> However, af_xdp maintainers opinion is more important here.
>>>
>>
>> Qi, what do you think?
>>
>
> Hi Qi/Ciara, this issue is still present approaching 21.11.4.
>
> What is your opinion on backporting these patches? Please especially
> note the paragraph above wrt users not being required to upgrade libbpf.
>
+Shibin, following discussion in DPDK release meeting.
> thanks,
> Kevin.
>
>>>>
>>>> thanks,
>>>> Kevin.
>>>>
>>>> [1] https://paste.centos.org/view/e4eec764
>>>> [2]
>>>> 1eb1846b1a net/af_xdp: make compatible with libbpf 0.8.0
>>>> 5ff3dbe6ce net/af_xdp: add log on XDP program removal failures
>>>> 0ed0bc3834 net/af_xdp: avoid version-based check for program load
>>>> e024c7e838 net/af_xdp: avoid version-based check for shared UMEM
>>>> f76dc44ded net/af_xdp: make clear which libxdp version is required
>>>> 50b855fc47 net/af_xdp: move XDP library presence flag setting
>>>>
>>>
>>
>
+cc John
For better context, this part of the discussion starts here:
http://inbox.dpdk.org/dev/d718d0fe-09a2-8840-e8a4-dd41b732b391@redhat.com/
On 16/03/2023 13:31, Kevin Traynor wrote:
> On 15/03/2023 11:47, Kevin Traynor wrote:
>> On 21/12/2022 09:28, Kevin Traynor wrote:
>>> On 21/12/2022 06:09, Andrew Rybchenko wrote:
>>>> Hi Kevin,
>>>>
>>>> On 12/20/22 17:05, Kevin Traynor wrote:
>>>>> On 24/06/2022 11: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>
>>>>>
>>>>> Hi Andrew/Qi (assuming Ciara is still out of office),
>>>>>
>>>>> I am seeing a similar issue [1] on 21.11 branch with Fedora 37
>>>>> (libbpf-0.8.0-2.fc37.x86_64 and libxdp-1.2.6-1.fc37.x86_64).
>>>>>
>>>>> This patch alone won't apply as there are other dependencies. Looking at
>>>>> the commits in main branch, it seems like I could take all these [2] to
>>>>> resolve the issue. With these cherry-picked the build warnings on Fedora
>>>>> 37 are removed.
>>>>>
>>>>> It's a bit late to take these for DPDK 21.11.3 as I intend to release
>>>>> later today/tomorrow, so it can be resolved for DPDK 21.11.4.
>>>>>
>>>>> Do the commits below look ok for backport? Main branch might be able to
>>>>> demand user uses new libbpf/libxdp versions etc, but with stable we
>>>>> never want to break the users existing setup when they upgrade from
>>>>> 2X.11.n to 2X.11.n+1.
>>
>> N.B. ^^^^
>>
>>>>>
>>>>> Let me know what you think?
>>>>
>>>> IMO these patches are to to be backported to stable branch.
>>>
>>> Thanks Andrew.
>>>
>>>> However, af_xdp maintainers opinion is more important here.
>>>>
>>>
>>> Qi, what do you think?
>>>
>>
>> Hi Qi/Ciara, this issue is still present approaching 21.11.4.
>>
>> What is your opinion on backporting these patches? Please especially
>> note the paragraph above wrt users not being required to upgrade libbpf.
>>
>
> +Shibin, following discussion in DPDK release meeting.
>
>> thanks,
>> Kevin.
>>
>>>>>
>>>>> thanks,
>>>>> Kevin.
>>>>>
>>>>> [1] https://paste.centos.org/view/e4eec764
>>>>> [2]
>>>>> 1eb1846b1a net/af_xdp: make compatible with libbpf 0.8.0
>>>>> 5ff3dbe6ce net/af_xdp: add log on XDP program removal failures
>>>>> 0ed0bc3834 net/af_xdp: avoid version-based check for program load
>>>>> e024c7e838 net/af_xdp: avoid version-based check for shared UMEM
>>>>> f76dc44ded net/af_xdp: make clear which libxdp version is required
>>>>> 50b855fc47 net/af_xdp: move XDP library presence flag setting
>>>>>
>>>>
>>>
>>
>
On 21/12/2022 09:28, Kevin Traynor wrote:
> On 21/12/2022 06:09, Andrew Rybchenko wrote:
>> Hi Kevin,
>>
>> On 12/20/22 17:05, Kevin Traynor wrote:
>>> On 24/06/2022 11: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>
>>>
>>> Hi Andrew/Qi (assuming Ciara is still out of office),
>>>
>>> I am seeing a similar issue [1] on 21.11 branch with Fedora 37
>>> (libbpf-0.8.0-2.fc37.x86_64 and libxdp-1.2.6-1.fc37.x86_64).
>>>
>>> This patch alone won't apply as there are other dependencies. Looking at
>>> the commits in main branch, it seems like I could take all these [2] to
>>> resolve the issue. With these cherry-picked the build warnings on Fedora
>>> 37 are removed.
>>>
>>> It's a bit late to take these for DPDK 21.11.3 as I intend to release
>>> later today/tomorrow, so it can be resolved for DPDK 21.11.4.
>>>
>>> Do the commits below look ok for backport? Main branch might be able to
>>> demand user uses new libbpf/libxdp versions etc, but with stable we
>>> never want to break the users existing setup when they upgrade from
>>> 2X.11.n to 2X.11.n+1.
>>>
>>> Let me know what you think?
>>
>> IMO these patches are to to be backported to stable branch.
>
> Thanks Andrew.
>
>> However, af_xdp maintainers opinion is more important here.
>>
>
> Qi, what do you think?
>
As F37 will be the oldest supported Fedora release from mid-May and
there is no available af_xdp maintainers to comment on backports, I'm
going to just squash the warnings [0] for 21.11.4.
If a better fix is available for 21.11.5+ please send patches to stable ML.
thanks,
Kevin.
[0]
http://inbox.dpdk.org/stable/20230404153501.123038-1-ktraynor@redhat.com/
>>>
>>> thanks,
>>> Kevin.
>>>
>>> [1] https://paste.centos.org/view/e4eec764
>>> [2]
>>> 1eb1846b1a net/af_xdp: make compatible with libbpf 0.8.0
>>> 5ff3dbe6ce net/af_xdp: add log on XDP program removal failures
>>> 0ed0bc3834 net/af_xdp: avoid version-based check for program load
>>> e024c7e838 net/af_xdp: avoid version-based check for shared UMEM
>>> f76dc44ded net/af_xdp: make clear which libxdp version is required
>>> 50b855fc47 net/af_xdp: move XDP library presence flag setting
>>>
>>
>
@@ -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.
@@ -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
static int load_program(const char *prog_path, struct bpf_object **obj)
{
struct bpf_program *prog;
@@ -85,6 +85,23 @@ static int load_program(const char *prog_path, struct bpf_object **obj)
bpf_object__close(*obj);
return -1;
}
+
+static int
+remove_xdp_program(int ifindex)
+{
+ uint32_t curr_prog_id = 0;
+
+ if (bpf_xdp_query_id(ifindex, XDP_FLAGS_UPDATE_IF_NOEXIST,
+ &curr_prog_id))
+ return -1;
+
+ return bpf_xdp_detach(ifindex, XDP_FLAGS_UPDATE_IF_NOEXIST, NULL);
+}
+
+static int link_xdp_prog_with_dev(int ifindex, int fd, __u32 flags)
+{
+ return bpf_xdp_attach(ifindex, fd, flags, NULL);
+}
#else
static int load_program(const char *prog_path, struct bpf_object **obj)
{
@@ -96,4 +113,21 @@ static int load_program(const char *prog_path, struct bpf_object **obj)
return prog_fd;
}
+
+static int
+remove_xdp_program(int ifindex)
+{
+ uint32_t curr_prog_id = 0;
+
+ if (bpf_get_link_xdp_id(ifindex, &curr_prog_id,
+ XDP_FLAGS_UPDATE_IF_NOEXIST))
+ return -1;
+
+ return bpf_set_link_xdp_fd(ifindex, -1, XDP_FLAGS_UPDATE_IF_NOEXIST);
+}
+
+static int link_xdp_prog_with_dev(int ifindex, int fd, __u32 flags)
+{
+ return bpf_set_link_xdp_fd(ifindex, fd, flags);
+}
#endif
@@ -10,10 +10,7 @@ endif
sources = files('rte_eth_af_xdp.c')
xdp_dep = dependency('libxdp', version : '>=1.2.2', required: false, method: 'pkg-config')
-bpf_dep = dependency('libbpf', required: false, method: 'pkg-config')
-if not bpf_dep.found()
- bpf_dep = cc.find_library('bpf', required: false)
-endif
+bpf_dep = dependency('libbpf', version : '<=0.8.0', required: false, method: 'pkg-config')
if cc.has_header('linux/if_xdp.h')
if xdp_dep.found() and cc.has_header('xdp/xsk.h')
@@ -25,7 +22,7 @@ if cc.has_header('linux/if_xdp.h')
bpf_ver_dep = dependency('libbpf', version : '>=0.7.0',
required: false, method: 'pkg-config')
if bpf_ver_dep.found()
- cflags += ['-DRTE_NET_AF_XDP_LIBBPF_OBJ_OPEN']
+ cflags += ['-DRTE_NET_AF_XDP_LIBBPF_V070']
endif
else
build = false
@@ -863,20 +863,6 @@ eth_stats_reset(struct rte_eth_dev *dev)
return 0;
}
-static void
-remove_xdp_program(struct pmd_internals *internals)
-{
- uint32_t curr_prog_id = 0;
-
- if (bpf_get_link_xdp_id(internals->if_index, &curr_prog_id,
- XDP_FLAGS_UPDATE_IF_NOEXIST)) {
- AF_XDP_LOG(ERR, "bpf_get_link_xdp_id failed\n");
- return;
- }
- bpf_set_link_xdp_fd(internals->if_index, -1,
- XDP_FLAGS_UPDATE_IF_NOEXIST);
-}
-
static void
xdp_umem_destroy(struct xsk_umem_info *umem)
{
@@ -929,7 +915,8 @@ eth_dev_close(struct rte_eth_dev *dev)
*/
dev->data->mac_addrs = NULL;
- remove_xdp_program(internals);
+ if (remove_xdp_program(internals->if_index))
+ AF_XDP_LOG(ERR, "Error while removing XDP program.\n");
if (internals->shared_umem) {
struct internal_list *list;
@@ -1195,7 +1182,7 @@ load_custom_xdp_prog(const char *prog_path, int if_index, struct bpf_map **map)
}
/* Link the program with the given network device */
- ret = bpf_set_link_xdp_fd(if_index, prog_fd,
+ ret = link_xdp_prog_with_dev(if_index, prog_fd,
XDP_FLAGS_UPDATE_IF_NOEXIST);
if (ret) {
AF_XDP_LOG(ERR, "Failed to set prog fd %d on interface\n",