netdev - RE: [PATCH net-next v3] ethtool: add netlink based get rss 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:   Wed, 23 Nov 2022 19:05:31 +0000
From:   "Mogilappagari, Sudheer" <sudheer.mogilappagari@...el.com>
To:     Jakub Kicinski <kuba@...nel.org>, Michal Kubecek <mkubecek@...e.cz>
CC:     Francois Romieu <romieu@...zoreil.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "andrew@...n.ch" <andrew@...n.ch>,
        "corbet@....net" <corbet@....net>,
        "Samudrala, Sridhar" <sridhar.samudrala@...el.com>,
        "Nguyen, Anthony L" <anthony.l.nguyen@...el.com>
Subject: RE: [PATCH net-next v3] ethtool: add netlink based get rss support



> -----Original Message-----
> From: Jakub Kicinski <kuba@...nel.org>
> Subject: Re: [PATCH net-next v3] ethtool: add netlink based get rss
> support
> 
> On Sun, 20 Nov 2022 22:02:17 +0100 Michal Kubecek wrote:
> > That would leave us with two questions:
> 
> Here's my 2 cents:
> 
> > 1. What to do with ETHTOOL_GRXRING? Can we use ETHTOOL_MSG_RINGS_GET
> > as it is? (I.e. should the count be always equal to rx + combined?)
> If
> > not, should we extend it or put the count into ETHTOOL_MSG_RSS_GET?
> 
> That'd be great.. but there are drivers out there for which rx +
> combined is incorrect.
> 
> Maybe we need to add an attr to ETHTOOL_MSG_RINGS_GET which core will
> fill in by default to rx + combined but broken drivers can correct it
> to whatever is right for them?
> 
> We can either create that attr already or wait for someone to complain?
> The same info is needed to size AF_XDP tables, which was a bit of a
> unifying force to do the right thing (i.e. make rx + combined correct).
> 
> I'm torn, because I'm happy for the driver authors who got this wrong
> to suffer and get complaints. But that implies that users also suffer
> which is not cool :(

Based on this discussion, I added ring count attribute to ETHTOOL_MSG_RSS_GET
in v5 to return queue count info also to user space. This simplifies user space
code by avoiding another netlink call to just get ring count. If there are  
concerns, we can pull out rings attribute and send v6.  
 
-Sudheer
 

> 
> > 2. What would be the best way to handle creation and deletion of RSS
> > contexts? I guess either a separate message type or combining the
> > functionality into ETHTOOL_MSG_RSS_SET somehow (but surely not via
> > some magic values like it's done in ioctl).
> 
> Explicit RSS_CTX_ADD / RSS_CTX_DEL seems reasonable. And we should have
> the core keep an explicit list of the contexts while at it :/