Subject: Re: [PATCH v2] net: mvneta: add basic XDP_DROP support

From: Jesper Dangaard Brouer <[email-masked]>
Date: Sat, 22 Dec 2018 11:52:54 +0100

On Fri, 21 Dec 2018 14:22:23 +0100
Domagoj Pintaric <[email-masked]> wrote:

> Add initial mvneta XDP support for hardware buffer management enabled
> devices only.

Hi Domagoj,

I would really appreciate if we could coordinate our work on the mvneta
driver.  Ilias (Cc'ed) and I are also working on adding XDP support for
this driver, although this is the software-buffer side of the driver we
have functioning now.

You can directly follow our progress here: [1][2]
 [1] https://github.com/xdp-project/xdp-project/blob/master/areas/arm64/board_espressobin04_bench_xdp.org
 [2] https://github.com/apalos/bpf-next/commits/mvneta_04_page_pool_recycle_xdp

You XDP-setup function is actually more correct that ours[3], as you
handle BPF per queue (which we were planning to fix before upstreaming).

That said, adding XDP_DROP is easy, but I want to see more of the XDP
features/actions added, as those require a lot more work.  I always
worry that a driver will stop at just XDP_DROP, so what are your plans
for adding the harder features? 

Even-thought XDP_DROP looks easy in this patch, then you are actually
doing some wrong, as XDP can also modify frames before doing XDP_PASS,
and (1) you have non-standard-head room (MVNETA_MH_SIZE + NET_SKB_PAD),
(2) and you don't handle if XDP changed the xdp.data header pointer,
and (3) you backing memory either comes from page-fragments or kmalloc
which also goes against the XDP memory requirements.

[3] https://github.com/apalos/bpf-next/commit/4b567e74552d3cdf55
-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer