netdev - Re: [net-next, PATCH 2/2, v2] net: socionext: 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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20180913163206.4feed505@redhat.com>
Date:   Thu, 13 Sep 2018 16:32:06 +0200
From:   Jesper Dangaard Brouer <brouer@...hat.com>
To:     Ilias Apalodimas <ilias.apalodimas@...aro.org>
Cc:     netdev@...r.kernel.org, jaswinder.singh@...aro.org,
        ard.biesheuvel@...aro.org, masami.hiramatsu@...aro.org,
        arnd@...db.de, mykyta.iziumtsev@...aro.org, bjorn.topel@...el.com,
        magnus.karlsson@...el.com, daniel@...earbox.net, ast@...nel.org,
        brouer@...hat.com
Subject: Re: [net-next, PATCH 2/2, v2] net: socionext: add XDP support

On Wed, 12 Sep 2018 12:29:15 +0300
Ilias Apalodimas <ilias.apalodimas@...aro.org> wrote:

> On Wed, Sep 12, 2018 at 11:25:24AM +0200, Jesper Dangaard Brouer wrote:
> > On Wed, 12 Sep 2018 12:02:38 +0300
> > Ilias Apalodimas <ilias.apalodimas@...aro.org> wrote:
> >   
> > >  static const struct net_device_ops netsec_netdev_ops = {
> > >  	.ndo_init		= netsec_netdev_init,
> > >  	.ndo_uninit		= netsec_netdev_uninit,
> > > @@ -1430,6 +1627,7 @@ static const struct net_device_ops netsec_netdev_ops = {
> > >  	.ndo_set_mac_address    = eth_mac_addr,
> > >  	.ndo_validate_addr	= eth_validate_addr,
> > >  	.ndo_do_ioctl		= netsec_netdev_ioctl,
> > > +	.ndo_bpf		= netsec_xdp,
> > >  };
> > >    
> > 
> > You have not implemented ndo_xdp_xmit.
> > 
> > Thus, you have "only" implemented the RX side of XDP_REDIRECT.  Which
> > allows you to do, cpumap and AF_XDP redirects, but not allowing other
> > drivers to XDP send out this device.  
>
> Correct, that was the planning, is ndo_xdp_xmit() needed for the patch or
> is the patch message just misleading and i should change that ?

Yes, I think you should ALSO implement ndo_xdp_xmit, maybe as a separate
patch, but in the same series. (Our experience is that if we don't
require this, people forget to complete this part of the XDP support).

Also you XDP_TX is not optimal, as it (looks like) you flush TX on
every send.

BTW, do you have any performance numbers?

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer