Re: [net-next, PATCH 3/3, v2] net: netsec: add XDP support

From: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
To: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Cc: netdev@vger.kernel.org, jaswinder.singh@linaro.org,
	ard.biesheuvel@linaro.org, bjorn.topel@intel.com,
	magnus.karlsson@intel.com, brouer@redhat.com,
	daniel@iogearbox.net, ast@kernel.org,
	makita.toshiaki@lab.ntt.co.jp, jakub.kicinski@netronome.com,
	john.fastabend@gmail.com, davem@davemloft.net,
	maciejromanfijalkowski@gmail.com
Subject: Re: [net-next, PATCH 3/3, v2] net: netsec: add XDP support
Date: Sun, 30 Jun 2019 19:41:28 +0300	[thread overview]
Message-ID: <20190630164127.GC12704@khorivan> (raw)
In-Reply-To: <20190630163214.GA10484@apalos>

On Sun, Jun 30, 2019 at 07:32:14PM +0300, Ilias Apalodimas wrote:
>On Sun, Jun 30, 2019 at 07:25:53PM +0300, Ivan Khoronzhuk wrote:
>> On Sat, Jun 29, 2019 at 08:23:25AM +0300, Ilias Apalodimas wrote:
>> >The interface only supports 1 Tx queue so locking is introduced on
>> >the Tx queue if XDP is enabled to make sure .ndo_start_xmit and
>> >.ndo_xdp_xmit won't corrupt Tx ring
>> >
>> >- Performance (SMMU off)
>> >
>> >Benchmark   XDP_SKB     XDP_DRV
>> >xdp1        291kpps     344kpps
>> >rxdrop      282kpps     342kpps
>> >
>> >- Performance (SMMU on)
>> >Benchmark   XDP_SKB     XDP_DRV
>> >xdp1        167kpps     324kpps
>> >rxdrop      164kpps     323kpps
>> >
>> >Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
>> >---
>> >drivers/net/ethernet/socionext/netsec.c | 361 ++++++++++++++++++++++--
>> >1 file changed, 334 insertions(+), 27 deletions(-)
>> >
>>
>> [...]
>>
>> >+
>> >+static int netsec_xdp_setup(struct netsec_priv *priv, struct bpf_prog *prog,
>> >+			    struct netlink_ext_ack *extack)
>> >+{
>> >+	struct net_device *dev = priv->ndev;
>> >+	struct bpf_prog *old_prog;
>> >+
>> >+	/* For now just support only the usual MTU sized frames */
>> >+	if (prog && dev->mtu > 1500) {
>> >+		NL_SET_ERR_MSG_MOD(extack, "Jumbo frames not supported on XDP");
>> >+		return -EOPNOTSUPP;
>> >+	}
>> >+
>> >+	if (netif_running(dev))
>> >+		netsec_netdev_stop(dev);
>> And why to stop the interface. XDP allows to update prog in runtime.
>>
>Adding the support is not limited to  adding a prog only in this driver.
>It also rebuilts the queues which changes the dma mapping of buffers.
>Since i don't want to map BIDIRECTIONAL buffers if XDP is not in place,
>i am resetting the device and forcing the buffer re-allocation
>
>Thanks
>/Ilias
I don't know the internals, probably it has some dependencies, but here you
just update the prog and can at least do it when exchange is happening.
I mean not in case of prog is attached/removed first time.
In case of prog -> prog it seems doable...

It ups to you ofc, but I can run smth like:
ip -force link set dev eth0 xdp obj xdp-example-pass.o sec .text
and expect it's updated w/o interface reset I mean on new prog.

I'm not sure, but maintainers can help, conceptually it's supposed to be in
runtime the prog be update uder rcu as a part of API usage...

-- 
Regards,
Ivan Khoronzhuk