From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jesper Dangaard Brouer Subject: Re: [PATCH v2] net: mvneta: add basic XDP_DROP support Date: Sat, 22 Dec 2018 11:52:54 +0100 Message-ID: <20181222115254.2001201b@redhat.com> References: <20181221132223.14012-1-domagoj.pintaric@sartura.hr> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: brouer@redhat.com, netdev@vger.kernel.org, Luka Perkov , Thomas Petazzoni , "David S . Miller" , Ilias Apalodimas , Marcin Wojtas To: Domagoj Pintaric Return-path: Received: from mx1.redhat.com ([209.132.183.28]:58532 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1731562AbeLVQuG (ORCPT ); Sat, 22 Dec 2018 11:50:06 -0500 In-Reply-To: <20181221132223.14012-1-domagoj.pintaric@sartura.hr> Sender: netdev-owner@vger.kernel.org List-ID: On Fri, 21 Dec 2018 14:22:23 +0100 Domagoj Pintaric 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