Re: [PATCH v9 1/1] net/af_xdp: introduce AF XDP PMD driver

[Ferruh Yigit <ferruh.yigit@intel.com>]

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
>>>
>>> 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 = librte_pmd_af_xdp.a
>>>>> +
>>>>> +EXPORT_MAP := rte_pmd_af_xdp_version.map
>>>>> +
>>>>> +LIBABIVER := 1
>>>>> +
>>>>> +CFLAGS += -O3
>>>>> +
>>>>> +# require kernel version >= v5.1-rc1
>>>>> +CFLAGS += -I$(RTE_KERNELDIR)/tools/include
>>>>> +CFLAGS += -I$(RTE_KERNELDIR)/tools/lib/bpf
>>>>
>>>> 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.
>>>
>>> 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?
>>>
>>> Something like:
>>>
>>> dependencies:
>>> - kernel source code (>= v5.1-rc1)
>>> - build libbfp and install
>>>
>>> Thanks,
>>> Xiaolong
>>
>> 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.
>>
>> Is it really necessary for external applications to use an internal-
>> only header and a kernel header to be able to use libbpf?
> 
> Actually, xsk.h is now installed by the library makefile:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git/commit/?id=379e2014c95b

Good to have this one. But again it is in BPF tree and it won't be in 5.1.

I suggested changing code as following for now, it would help to keep changes
small when above patch merged into kernel:
 CFLAGS += -I$(RTE_KERNELDIR)/tools/lib [in makefile]
 #include <bpf/xsk.h>                   [in .c file]

> 
> So the full kernel source tree is no longer required.
> 
> Is asm/barrier.h really required? Isn't there an userspace alternative?

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.

I wonder same thing, what is used from 'tools/include/asm/barrier.h' and if it
can be avoided.

Anyway, as Xiaolong mentioned, following is working, can it work from a distro
point of view:
- get kernel source code (>= v5.1-rc1)
- build libbfp and install
- set 'RTE_KERNELDIR' to point kernel source path
- build dpdk with af_xdp enabled

> 
> Also, the license in asm/barrier.h is GPL-2.0 only. It is not a
> userspace header so it is not covered by the userspace exception, which
> means at the very least the af_xdp PMD shared object is also licensed
> under GPL-2.0 only, isn't it?
>