From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id B9790A0551; Tue, 28 Jun 2022 11:15:08 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 9EA4F40691; Tue, 28 Jun 2022 11:15:08 +0200 (CEST) Received: from shelob.oktetlabs.ru (shelob.oktetlabs.ru [91.220.146.113]) by mails.dpdk.org (Postfix) with ESMTP id 78905400D7 for ; Tue, 28 Jun 2022 11:15:07 +0200 (CEST) Received: from [192.168.1.39] (unknown [188.170.75.69]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by shelob.oktetlabs.ru (Postfix) with ESMTPSA id BC0EAAA; Tue, 28 Jun 2022 12:15:06 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 shelob.oktetlabs.ru BC0EAAA DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=oktetlabs.ru; s=default; t=1656407707; bh=LIt0J5P4ZYCLCkq2/yNlFlKLRezKFBy/gXY9Vb8ndmQ=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=dRTJz53A/D7RyubTuDzkB5cNYvt3CiwFm7HDwx0fL59Kz7xJYW4tVCveGBSGffLsC jZ7JqlspVGwaC+dHgq0qspttNBEuVYNNqXrZVjJ8EvkpbTfT49VSOYzbAXn4KDeO4j jgbt4WVkfyQRZDsEHjtgI0WU/0w8S0VAhURwwnDU= Message-ID: <245830c5-0507-3b77-ee29-49376f01a5a0@oktetlabs.ru> Date: Tue, 28 Jun 2022 12:15:05 +0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.10.0 Subject: Re: [PATCH] net/af_xdp: make compatible with libbpf v0.8.0 Content-Language: en-US To: "Loftus, Ciara" , "dev@dpdk.org" Cc: "thomas@monjalon.net" , "ferruh.yigit@xilinx.com" References: <20220624102354.1516606-1-ciara.loftus@intel.com> From: Andrew Rybchenko In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org 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 >>>>> --- >>>>> 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.