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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20221121105841.214ce8e2@kernel.org>
Date:   Mon, 21 Nov 2022 10:58:41 -0800
From:   Jakub Kicinski <kuba@...nel.org>
To:     Michal Kubecek <mkubecek@...e.cz>
Cc:     "Mogilappagari, Sudheer" <sudheer.mogilappagari@...el.com>,
        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

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 :(

> 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 :/