netdev - RE: [EXT] Re: [PATCH v2 1/1] net: fec: add initial 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] [day] [month] [year] [list]

Date:   Mon, 31 Oct 2022 21:17:19 +0000
From:   Shenwei Wang <shenwei.wang@....com>
To:     Andrew Lunn <andrew@...n.ch>
CC:     "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Jesper Dangaard Brouer <hawk@...nel.org>,
        John Fastabend <john.fastabend@...il.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "imx@...ts.linux.dev" <imx@...ts.linux.dev>,
        kernel test robot <lkp@...el.com>
Subject: RE: [EXT] Re: [PATCH v2 1/1] net: fec: add initial XDP support



> -----Original Message-----
> From: Andrew Lunn <andrew@...n.ch>
> Sent: Monday, October 31, 2022 3:17 PM
> > > to disable the receiver, wait for it to indicate it really has
> > > stopped, and only then make these modifications.
> > >
> >
> > Sounds reasonable. How about moving the codes of updating ring size to
> > the place right after the enet reset inside the fec_restart? This
> > should clear those risky corner cases.
> 
> That sounds reasonable. But please add some comments. The driver has
> RX_RING_SIZE elements allocated, but you are only using a subset. This needs to
> be clear for when somebody implements the ethtool --rings option.
> 

The latest performance data regarding the native XDP has got improved a lot. The ring 
size change doesn't seem necessary any more. The v3 patch has removed that part
of codes, and that logic can be added back in future if necessary as well as the ethtool -rings option.

Thanks,
Shenwei

> And i still think it would be good to implement that code. As your numbers show,
> the ring size does affect performance, and it is hard to know if your hard coded
> XDP_RX_RING_SIZE is the right value, depending on what the eBPF program is
> doing. If the ethtool option was provided, it allows users to benchmark different
> ring sizes for their workload.
> 
>    Andrew