Re: [bpf-next V2 PATCH 02/15] xdp: introduce xdp_return_frame API and use in cpumap

[Jason Wang <jasowang@redhat.com>]

On 2018年03月10日 00:05, Jesper Dangaard Brouer wrote:
> On Fri, 9 Mar 2018 21:04:23 +0800
> Jason Wang<jasowang@redhat.com>  wrote:
>
>> On 2018年03月09日 17:35, Jesper Dangaard Brouer wrote:
>>> On Fri, 9 Mar 2018 15:24:10 +0800
>>> Jason Wang<jasowang@redhat.com>  wrote:
>>>   
>>>> On 2018年03月08日 21:07, Jesper Dangaard Brouer wrote:
>>>>> Introduce an xdp_return_frame API, and convert over cpumap as
>>>>> the first user, given it have queued XDP frame structure to leverage.
>>>>>
>>>>> Signed-off-by: Jesper Dangaard Brouer<brouer@redhat.com>
>>>>> ---
>>>>>     include/net/xdp.h   |   32 +++++++++++++++++++++++++++
>>>>>     kernel/bpf/cpumap.c |   60 +++++++++++++++++++++++++++++++--------------------
>>>>>     net/core/xdp.c      |   18 +++++++++++++++
>>>>>     3 files changed, 86 insertions(+), 24 deletions(-)
>>>>>
>>>>> diff --git a/include/net/xdp.h b/include/net/xdp.h
>>>>> index b2362ddfa694..3cb726a6dc5b 100644
>>>>> --- a/include/net/xdp.h
>>>>> +++ b/include/net/xdp.h
>>>>> @@ -33,16 +33,48 @@
>>>>>      * also mandatory during RX-ring setup.
>>>>>      */
>>>>>     
>>>>> +enum mem_type {
>>>>> +	MEM_TYPE_PAGE_SHARED = 0, /* Split-page refcnt based model */
>>>>> +	MEM_TYPE_PAGE_ORDER0,     /* Orig XDP full page model */
>>>>> +	// Possible new ideas for types:
>>>>> +	// MEM_TYPE_PAGE_POOL,    /* Will be added later  */
>>>>> +	// MEM_TYPE_AF_XDP,
>>>>> +	// MEM_TYPE_DEVICE_OWNED -- invoking an dev->ndo?
>>>>> +	MEM_TYPE_MAX,
>>>>> +};
>>>> So if we plan to support dev->ndo, it looks to me two types AF_XDP and
>>>> DEVICE_OWNED are sufficient? Driver can do what it wants (e.g page pool
>>>> or ordinary page allocator) in ndo or what ever other callbacks.
>>> So, the design is open to go both ways, we can figure out later.
>>>
>>> The reason I'm not calling page_pool from a driver level dev->ndo, is
>>> that I'm trying to avoid invoking code in a module, as the driver
>>> module code can be (in the process of being) unloaded from the kernel,
>>> while another driver is calling xdp_return_frame.  The current design
>>> in patch 10, uses the RCU period to guarantee that the allocator
>>> pointer is valid as long as the ID lookup succeeded.
>>>
>>> To do what you propose, we also need to guarantee that the net_device
>>> cannot disappear so it is safe to invoke dev->ndo.  To do so, the
>>> driver likely have to add a rcu_barrier before unloading.  I'm also
>>> thinking that for MEM_TYPE_DEVICE_OWNED the allocator pointer ("under
>>> protection") could be the net_device pointer, and we could take a
>>> refcnt on dev (dev_hold/dev_put).
>> Then it looks like a device (e.g tun) can lock another device for
>> indefinite time.
>>
>>> Thus, I think it is doable, but lets
>>> figure this out later.
>>>
>>> Another important design consideration, is that the xdp core need to
>>> know how to release memory in case the ID lookup failed.
>> Can we simply use put_page() in this case?
> For now, yes. The (future) use case is when pages are kept DMA mapped,
> then the page also need to be DMA unmapped.

This probably needs careful thought. E.g current virtio-net driver does 
not do mapping by itself, instead it depends on the virtio core to do 
map and unmap.

> To DMA unmap we need to
> know the struct device (not net_device).  (And then the discussion
> start around how to know struct device is still valid).

So still need a safe fallback if I understand this correctly.

Thanks