From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-f68.google.com (mail-wr1-f68.google.com [209.85.221.68]) by dpdk.org (Postfix) with ESMTP id 57E531B39E for ; Wed, 3 Apr 2019 15:29:09 +0200 (CEST) Received: by mail-wr1-f68.google.com with SMTP id k17so21335615wrx.10 for ; Wed, 03 Apr 2019 06:29:09 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:message-id:subject:from:to:cc:date:in-reply-to :references:content-transfer-encoding:user-agent:mime-version; bh=U2wibpU4y9iut86Uj9D1okSi9zxjqjS1n7QJEUoApcU=; b=VHSmVgbcEeBcMywm/hdNBMgVct0lZXWTPTYa39PYsOuhQEwoDtoNV1WYLCOVufCDvK wVOpKmGmfJN26DhmMUaEy6A9vgOveIha9WKG15Qrd/u1VB7sBrHeiR4zFM8NvVptNMit kWsNWDRV1LEQGs8Dp2ocd+i+qSPo/k4ePrW1GtQHoUmJc2Ae3WZSCHdT0MusedcYKRCy Vgd5Ynr2mbNI24JRhYvCJheMbWFKpeLEyOGxaPNOWTAU+CZkblUz66CXS6gvSb4e9AkY /bmMFj0lfGitvctmoTD9RNkOJk0hQ0xrkpm07XxvK8AmUFcvX8IjTkvYHAaCqUAmcInq TWMQ== X-Gm-Message-State: APjAAAWnz6OcfA57ukuGzNPzKkE/ldr639TyRSnJY4yxiqT2gZErLRZ6 EOr2ZJSRVglthbu+8NhlQhENkaa8 X-Google-Smtp-Source: APXvYqxLPWGii+ymHTfp6lUpRZVFaD0KNFpkg4WM7tz9GczOj28bEKBMImZXO5IEQBzssyUjFyJvDA== X-Received: by 2002:a5d:428f:: with SMTP id k15mr53126344wrq.113.1554298148926; Wed, 03 Apr 2019 06:29:08 -0700 (PDT) Received: from localhost ([2a01:4b00:f419:6f00:250:b6ff:feb7:bd60]) by smtp.gmail.com with ESMTPSA id t24sm20906481wmi.10.2019.04.03.06.29.07 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Wed, 03 Apr 2019 06:29:08 -0700 (PDT) Message-ID: <46d92b70a40581462f5ee3ba301c793c4cf0c2df.camel@debian.org> From: Luca Boccassi To: Ferruh Yigit , Ye Xiaolong Cc: dev@dpdk.org, Karlsson Magnus , Topel Bjorn Date: Wed, 03 Apr 2019 14:29:07 +0100 In-Reply-To: <5bc49c51-04f4-6f73-889d-d3c0ff749784@intel.com> References: <20190301080947.91086-1-xiaolong.ye@intel.com> <20190402154653.711-1-xiaolong.ye@intel.com> <20190402154653.711-2-xiaolong.ye@intel.com> <20190403095939.GA32340@intel.com> <56ce5855b02d47a085a8d36451561c400f0b039c.camel@debian.org> <0dde8c20e9992047f29d39ad45dcf511244a5297.camel@debian.org> <80c81c0c-cf64-59f8-a592-26cd865fbd89@intel.com> <37073834d0b9a9f5a6e9f39bac3adc5eb29779ab.camel@debian.org> <5bc49c51-04f4-6f73-889d-d3c0ff749784@intel.com> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable User-Agent: Evolution 3.30.5-1 MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v9 1/1] net/af_xdp: introduce AF XDP PMD driver X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 03 Apr 2019 13:29:09 -0000 On Wed, 2019-04-03 at 14:09 +0100, Ferruh Yigit wrote: > On 4/3/2019 12:35 PM, Luca Boccassi wrote: > > On Wed, 2019-04-03 at 12:18 +0100, Ferruh Yigit wrote: > > > On 4/3/2019 11:42 AM, Luca Boccassi wrote: > > > > On Wed, 2019-04-03 at 11:36 +0100, Luca Boccassi wrote: > > > > > On Wed, 2019-04-03 at 17:59 +0800, Ye Xiaolong wrote: > > > > > > Hi, Luca > > > > > >=20 > > > > > > On 04/02, Luca Boccassi wrote: > > > > > > > On Tue, 2019-04-02 at 23:46 +0800, Xiaolong Ye wrote: > > > > > > > > diff --git a/drivers/net/af_xdp/Makefile > > > > > > > > b/drivers/net/af_xdp/Makefile > > > > > > > > new file mode 100644 > > > > > > > > index 000000000..8343e3016 > > > > > > > > --- /dev/null > > > > > > > > +++ b/drivers/net/af_xdp/Makefile > > > > > > > > @@ -0,0 +1,32 @@ > > > > > > > > +# SPDX-License-Identifier: BSD-3-Clause > > > > > > > > +# Copyright(c) 2019 Intel Corporation > > > > > > > > + > > > > > > > > +include $(RTE_SDK)/mk/rte.vars.mk > > > > > > > > + > > > > > > > > +# > > > > > > > > +# library name > > > > > > > > +# > > > > > > > > +LIB =3D librte_pmd_af_xdp.a > > > > > > > > + > > > > > > > > +EXPORT_MAP :=3D rte_pmd_af_xdp_version.map > > > > > > > > + > > > > > > > > +LIBABIVER :=3D 1 > > > > > > > > + > > > > > > > > +CFLAGS +=3D -O3 > > > > > > > > + > > > > > > > > +# require kernel version >=3D v5.1-rc1 > > > > > > > > +CFLAGS +=3D -I$(RTE_KERNELDIR)/tools/include > > > > > > > > +CFLAGS +=3D -I$(RTE_KERNELDIR)/tools/lib/bpf > > > > > > >=20 > > > > > > > Sorry for not noticing this before, but doesn't this > > > > > > > require > > > > > > > the > > > > > > > full > > > > > > > kernel tree rather than just the typical headers package? > > > > > > > Requiring > > > > > > > the > > > > > > > full kernel tree to be available at build time will make > > > > > > > this > > > > > > > unbuildable on distros that still use makefiles, like > > > > > > > RHEL > > > > > > > and > > > > > > > SUSE. At > > > > > > > least on Debian and Ubuntu, the kernel headers packages > > > > > > > distributed > > > > > > > do > > > > > > > not include the full kernel tree, only the headers, so > > > > > > > there's no > > > > > > > tools/lib or tools/include. > > > > > >=20 > > > > > > Currently we do have dependencies on the kernel src tree, > > > > > > as > > > > > > xsk.h > > > > > > and > > > > > > asm/barrier wouldn't be installed by libbpf, so before > > > > > > libbpf > > > > > > handles > > > > > > these > > > > > > properly, can we keep the current RTE_KERNELDIR in Makefile > > > > > > for > > > > > > now, > > > > > > and mention > > > > > > the dependencies in document, also suggest users to config > > > > > > RTE_KERNELDIR to correct > > > > > > kernel src tree if they want to use af_xdp pmd? > > > > > >=20 > > > > > > Something like: > > > > > >=20 > > > > > > dependencies: > > > > > > - kernel source code (>=3D v5.1-rc1) > > > > > > - build libbfp and install > > > > > >=20 > > > > > > Thanks, > > > > > > Xiaolong > > > > >=20 > > > > > asm/barrier.h is installed by the kernel headers packages so > > > > > it > > > > > would > > > > > be fine (although not ideal) and not need the full source > > > > > tree. > > > > > xsk.h is a bit more worrying, as it looks like an internal > > > > > header > > > > > from > > > > > here. > > > > >=20 > > > > > Is it really necessary for external applications to use an > > > > > internal- > > > > > only header and a kernel header to be able to use libbpf? > > > >=20 > > > > Actually, xsk.h is now installed by the library makefile: > > > >=20 > > > > https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git/commit/= ?id=3D379e2014c95b > > > >=20 > > > >=20 > > >=20 > > > Good to have this one. But again it is in BPF tree and it won't > > > be in > > > 5.1. > >=20 > > It looks like a small and required bug fix to me, and 5.1 is still > > in > > RC state, so perhaps there's still time. > >=20 > > Bjorn and Magnus, any chance the above makefile install fix could > > be > > sent for inclusion in 5.1-rc4? > >=20 > > > I suggested changing code as following for now, it would help to > > > keep > > > changes > > > small when above patch merged into kernel: > > > CFLAGS +=3D -I$(RTE_KERNELDIR)/tools/lib [in makefile] > > > #include [in .c file] > > >=20 > > > > So the full kernel source tree is no longer required. > > > >=20 > > > > Is asm/barrier.h really required? Isn't there an userspace > > > > alternative? > > >=20 > > > The 'asm/barrier.h' in the kernel headers and the > > > 'tools/include/asm/barrier.h' > > > looks different, the one in the kernel source has dependency to > > > other > > > kernel > > > headers. > > >=20 > > > I wonder same thing, what is used from > > > 'tools/include/asm/barrier.h' > > > and if it > > > can be avoided. >=20 > It seems, 'tools/include/asm/barrier.h' is required for 'smp_wmb()' & > 'smp_rmb()' in 'xsk.h'. > We have equivalents of these in DPDK [1], and perhaps it can be > possible to use > them and not include this header at all. >=20 > in 'rte_eth_af_xdp.c', before including 'xsk.h', we can include an > local > compatibility header which does following should work: > #define smp_rmb() rte_rmb() > #define smp_wmb() rte_wmb() >=20 > @Xiaolong, what do you think? >=20 > [1] > https://git.dpdk.org/dpdk/tree/lib/librte_eal/common/include/arch/x86/rte= _atomic.h?h=3Dv19.02#n30 Perfect, that looks like a great solution for the PMD. For the broader picture, now that xsk.h is a public userspace header, it should at some point in the future be fixed to avoid depending on an internal kernel definition for the barriers, and either ship their own or depend on another public header that provides them. Otherwise every application that wants to use bpf with xdp needs to provide their own implementation - but this is not relanted to this patchset and we can live without for the moment in DPDK. > > The one in tools/include also is GPL-2.0 only so it cannot be > > included > > from the PMD, which is BSD-3-clause only (and it recursively > > includes > > the other arch-specific kernel headers) > >=20 > > > Anyway, as Xiaolong mentioned, following is working, can it work > > > from > > > a distro > > > point of view: > > > - get kernel source code (>=3D v5.1-rc1) > > > - build libbfp and install > > > - set 'RTE_KERNELDIR' to point kernel source path > > > - build dpdk with af_xdp enabled > >=20 > > As long as the full kernel tree is required, we cannot enable it in > > Debian and Ubuntu - we can't have it at build time on the build > > workers, and also there's the licensing problem. >=20 > Got it. >=20 > In above steps, 'libbpf' also build from kernel source tree, will it > be problem > in you builds to not have it build from source? >=20 > If not, taking into account that xsk.h also will be fixed, only > 'tools/include/asm/barrier.h' remains the problem, and it looks like > it can be > solved, please check above. libbpf is already packaged separately in Debian and I think other distros will follow soon, so it's all good for me once the barrier issue is solved. https://packages.debian.org/buster/libbpf-dev >>From the makefile's perspective it should not matter where it comes from - the headers should be expected to be in /usr/include and the library in /usr/lib* - and pkg-config can help with that if available. And if a user wants to use a custom path, then it's no different than any of the other dependencies on other external libraries --=20 Kind regards, Luca Boccassi From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by dpdk.space (Postfix) with ESMTP id 7585AA0679 for ; Wed, 3 Apr 2019 15:29:10 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 1FD431B3B9; Wed, 3 Apr 2019 15:29:10 +0200 (CEST) Received: from mail-wr1-f68.google.com (mail-wr1-f68.google.com [209.85.221.68]) by dpdk.org (Postfix) with ESMTP id 57E531B39E for ; Wed, 3 Apr 2019 15:29:09 +0200 (CEST) Received: by mail-wr1-f68.google.com with SMTP id k17so21335615wrx.10 for ; Wed, 03 Apr 2019 06:29:09 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:message-id:subject:from:to:cc:date:in-reply-to :references:content-transfer-encoding:user-agent:mime-version; bh=U2wibpU4y9iut86Uj9D1okSi9zxjqjS1n7QJEUoApcU=; b=VHSmVgbcEeBcMywm/hdNBMgVct0lZXWTPTYa39PYsOuhQEwoDtoNV1WYLCOVufCDvK wVOpKmGmfJN26DhmMUaEy6A9vgOveIha9WKG15Qrd/u1VB7sBrHeiR4zFM8NvVptNMit kWsNWDRV1LEQGs8Dp2ocd+i+qSPo/k4ePrW1GtQHoUmJc2Ae3WZSCHdT0MusedcYKRCy Vgd5Ynr2mbNI24JRhYvCJheMbWFKpeLEyOGxaPNOWTAU+CZkblUz66CXS6gvSb4e9AkY /bmMFj0lfGitvctmoTD9RNkOJk0hQ0xrkpm07XxvK8AmUFcvX8IjTkvYHAaCqUAmcInq TWMQ== X-Gm-Message-State: APjAAAWnz6OcfA57ukuGzNPzKkE/ldr639TyRSnJY4yxiqT2gZErLRZ6 EOr2ZJSRVglthbu+8NhlQhENkaa8 X-Google-Smtp-Source: APXvYqxLPWGii+ymHTfp6lUpRZVFaD0KNFpkg4WM7tz9GczOj28bEKBMImZXO5IEQBzssyUjFyJvDA== X-Received: by 2002:a5d:428f:: with SMTP id k15mr53126344wrq.113.1554298148926; Wed, 03 Apr 2019 06:29:08 -0700 (PDT) Received: from localhost ([2a01:4b00:f419:6f00:250:b6ff:feb7:bd60]) by smtp.gmail.com with ESMTPSA id t24sm20906481wmi.10.2019.04.03.06.29.07 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Wed, 03 Apr 2019 06:29:08 -0700 (PDT) Message-ID: <46d92b70a40581462f5ee3ba301c793c4cf0c2df.camel@debian.org> From: Luca Boccassi To: Ferruh Yigit , Ye Xiaolong Cc: dev@dpdk.org, Karlsson Magnus , Topel Bjorn Date: Wed, 03 Apr 2019 14:29:07 +0100 In-Reply-To: <5bc49c51-04f4-6f73-889d-d3c0ff749784@intel.com> References: <20190301080947.91086-1-xiaolong.ye@intel.com> <20190402154653.711-1-xiaolong.ye@intel.com> <20190402154653.711-2-xiaolong.ye@intel.com> <20190403095939.GA32340@intel.com> <56ce5855b02d47a085a8d36451561c400f0b039c.camel@debian.org> <0dde8c20e9992047f29d39ad45dcf511244a5297.camel@debian.org> <80c81c0c-cf64-59f8-a592-26cd865fbd89@intel.com> <37073834d0b9a9f5a6e9f39bac3adc5eb29779ab.camel@debian.org> <5bc49c51-04f4-6f73-889d-d3c0ff749784@intel.com> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable User-Agent: Evolution 3.30.5-1 MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v9 1/1] net/af_xdp: introduce AF XDP PMD driver X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Message-ID: <20190403132907.7jyTvfvmhwy5SYg2YK1BoVyKL9INVxI5uewJVd-HNXs@z> On Wed, 2019-04-03 at 14:09 +0100, Ferruh Yigit wrote: > On 4/3/2019 12:35 PM, Luca Boccassi wrote: > > On Wed, 2019-04-03 at 12:18 +0100, Ferruh Yigit wrote: > > > On 4/3/2019 11:42 AM, Luca Boccassi wrote: > > > > On Wed, 2019-04-03 at 11:36 +0100, Luca Boccassi wrote: > > > > > On Wed, 2019-04-03 at 17:59 +0800, Ye Xiaolong wrote: > > > > > > Hi, Luca > > > > > >=20 > > > > > > On 04/02, Luca Boccassi wrote: > > > > > > > On Tue, 2019-04-02 at 23:46 +0800, Xiaolong Ye wrote: > > > > > > > > diff --git a/drivers/net/af_xdp/Makefile > > > > > > > > b/drivers/net/af_xdp/Makefile > > > > > > > > new file mode 100644 > > > > > > > > index 000000000..8343e3016 > > > > > > > > --- /dev/null > > > > > > > > +++ b/drivers/net/af_xdp/Makefile > > > > > > > > @@ -0,0 +1,32 @@ > > > > > > > > +# SPDX-License-Identifier: BSD-3-Clause > > > > > > > > +# Copyright(c) 2019 Intel Corporation > > > > > > > > + > > > > > > > > +include $(RTE_SDK)/mk/rte.vars.mk > > > > > > > > + > > > > > > > > +# > > > > > > > > +# library name > > > > > > > > +# > > > > > > > > +LIB =3D librte_pmd_af_xdp.a > > > > > > > > + > > > > > > > > +EXPORT_MAP :=3D rte_pmd_af_xdp_version.map > > > > > > > > + > > > > > > > > +LIBABIVER :=3D 1 > > > > > > > > + > > > > > > > > +CFLAGS +=3D -O3 > > > > > > > > + > > > > > > > > +# require kernel version >=3D v5.1-rc1 > > > > > > > > +CFLAGS +=3D -I$(RTE_KERNELDIR)/tools/include > > > > > > > > +CFLAGS +=3D -I$(RTE_KERNELDIR)/tools/lib/bpf > > > > > > >=20 > > > > > > > Sorry for not noticing this before, but doesn't this > > > > > > > require > > > > > > > the > > > > > > > full > > > > > > > kernel tree rather than just the typical headers package? > > > > > > > Requiring > > > > > > > the > > > > > > > full kernel tree to be available at build time will make > > > > > > > this > > > > > > > unbuildable on distros that still use makefiles, like > > > > > > > RHEL > > > > > > > and > > > > > > > SUSE. At > > > > > > > least on Debian and Ubuntu, the kernel headers packages > > > > > > > distributed > > > > > > > do > > > > > > > not include the full kernel tree, only the headers, so > > > > > > > there's no > > > > > > > tools/lib or tools/include. > > > > > >=20 > > > > > > Currently we do have dependencies on the kernel src tree, > > > > > > as > > > > > > xsk.h > > > > > > and > > > > > > asm/barrier wouldn't be installed by libbpf, so before > > > > > > libbpf > > > > > > handles > > > > > > these > > > > > > properly, can we keep the current RTE_KERNELDIR in Makefile > > > > > > for > > > > > > now, > > > > > > and mention > > > > > > the dependencies in document, also suggest users to config > > > > > > RTE_KERNELDIR to correct > > > > > > kernel src tree if they want to use af_xdp pmd? > > > > > >=20 > > > > > > Something like: > > > > > >=20 > > > > > > dependencies: > > > > > > - kernel source code (>=3D v5.1-rc1) > > > > > > - build libbfp and install > > > > > >=20 > > > > > > Thanks, > > > > > > Xiaolong > > > > >=20 > > > > > asm/barrier.h is installed by the kernel headers packages so > > > > > it > > > > > would > > > > > be fine (although not ideal) and not need the full source > > > > > tree. > > > > > xsk.h is a bit more worrying, as it looks like an internal > > > > > header > > > > > from > > > > > here. > > > > >=20 > > > > > Is it really necessary for external applications to use an > > > > > internal- > > > > > only header and a kernel header to be able to use libbpf? > > > >=20 > > > > Actually, xsk.h is now installed by the library makefile: > > > >=20 > > > > https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git/commit/= ?id=3D379e2014c95b > > > >=20 > > > >=20 > > >=20 > > > Good to have this one. But again it is in BPF tree and it won't > > > be in > > > 5.1. > >=20 > > It looks like a small and required bug fix to me, and 5.1 is still > > in > > RC state, so perhaps there's still time. > >=20 > > Bjorn and Magnus, any chance the above makefile install fix could > > be > > sent for inclusion in 5.1-rc4? > >=20 > > > I suggested changing code as following for now, it would help to > > > keep > > > changes > > > small when above patch merged into kernel: > > > CFLAGS +=3D -I$(RTE_KERNELDIR)/tools/lib [in makefile] > > > #include [in .c file] > > >=20 > > > > So the full kernel source tree is no longer required. > > > >=20 > > > > Is asm/barrier.h really required? Isn't there an userspace > > > > alternative? > > >=20 > > > The 'asm/barrier.h' in the kernel headers and the > > > 'tools/include/asm/barrier.h' > > > looks different, the one in the kernel source has dependency to > > > other > > > kernel > > > headers. > > >=20 > > > I wonder same thing, what is used from > > > 'tools/include/asm/barrier.h' > > > and if it > > > can be avoided. >=20 > It seems, 'tools/include/asm/barrier.h' is required for 'smp_wmb()' & > 'smp_rmb()' in 'xsk.h'. > We have equivalents of these in DPDK [1], and perhaps it can be > possible to use > them and not include this header at all. >=20 > in 'rte_eth_af_xdp.c', before including 'xsk.h', we can include an > local > compatibility header which does following should work: > #define smp_rmb() rte_rmb() > #define smp_wmb() rte_wmb() >=20 > @Xiaolong, what do you think? >=20 > [1] > https://git.dpdk.org/dpdk/tree/lib/librte_eal/common/include/arch/x86/rte= _atomic.h?h=3Dv19.02#n30 Perfect, that looks like a great solution for the PMD. For the broader picture, now that xsk.h is a public userspace header, it should at some point in the future be fixed to avoid depending on an internal kernel definition for the barriers, and either ship their own or depend on another public header that provides them. Otherwise every application that wants to use bpf with xdp needs to provide their own implementation - but this is not relanted to this patchset and we can live without for the moment in DPDK. > > The one in tools/include also is GPL-2.0 only so it cannot be > > included > > from the PMD, which is BSD-3-clause only (and it recursively > > includes > > the other arch-specific kernel headers) > >=20 > > > Anyway, as Xiaolong mentioned, following is working, can it work > > > from > > > a distro > > > point of view: > > > - get kernel source code (>=3D v5.1-rc1) > > > - build libbfp and install > > > - set 'RTE_KERNELDIR' to point kernel source path > > > - build dpdk with af_xdp enabled > >=20 > > As long as the full kernel tree is required, we cannot enable it in > > Debian and Ubuntu - we can't have it at build time on the build > > workers, and also there's the licensing problem. >=20 > Got it. >=20 > In above steps, 'libbpf' also build from kernel source tree, will it > be problem > in you builds to not have it build from source? >=20 > If not, taking into account that xsk.h also will be fixed, only > 'tools/include/asm/barrier.h' remains the problem, and it looks like > it can be > solved, please check above. libbpf is already packaged separately in Debian and I think other distros will follow soon, so it's all good for me once the barrier issue is solved. https://packages.debian.org/buster/libbpf-dev >From the makefile's perspective it should not matter where it comes from - the headers should be expected to be in /usr/include and the library in /usr/lib* - and pkg-config can help with that if available. And if a user wants to use a custom path, then it's no different than any of the other dependencies on other external libraries --=20 Kind regards, Luca Boccassi