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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190630165119.GE12704@khorivan>
Date:   Sun, 30 Jun 2019 19:51:20 +0300
From:   Ivan Khoronzhuk <ivan.khoronzhuk@...aro.org>
To:     Ilias Apalodimas <ilias.apalodimas@...aro.org>
Cc:     netdev@...r.kernel.org, jaswinder.singh@...aro.org,
        ard.biesheuvel@...aro.org, bjorn.topel@...el.com,
        magnus.karlsson@...el.com, brouer@...hat.com, daniel@...earbox.net,
        ast@...nel.org, makita.toshiaki@....ntt.co.jp,
        jakub.kicinski@...ronome.com, john.fastabend@...il.com,
        davem@...emloft.net, maciejromanfijalkowski@...il.com
Subject: Re: [net-next, PATCH 3/3, v2] net: netsec: add XDP support

On Sun, Jun 30, 2019 at 07:47:45PM +0300, Ilias Apalodimas wrote:
>On Sun, Jun 30, 2019 at 07:41:28PM +0300, Ivan Khoronzhuk wrote:
>> 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@...aro.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...
>It's doable but it means i'd have to change the buffer allocation again. I'd
>also prefer mapping FOR_DEVICE only if XDP is not loaded. Most of the drivers do
>restart so i'll stick with this for the current version.
>Most of the drivers do restart now so i'll stick to that for now.
I have nothing against it to be merged. Just pay attention on
XDP_FLAGS_UPDATE_IF_NOEXIST or update it layter or now...ups to your.

>
>Thanks
>/Ilias

-- 
Regards,
Ivan Khoronzhuk