netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: Michal Kubecek <mkubecek@suse.cz>
Cc: "Mogilappagari, Sudheer" <sudheer.mogilappagari@intel.com>,
	Francois Romieu <romieu@fr.zoreil.com>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"andrew@lunn.ch" <andrew@lunn.ch>,
	"corbet@lwn.net" <corbet@lwn.net>,
	"Samudrala, Sridhar" <sridhar.samudrala@intel.com>,
	"Nguyen, Anthony L" <anthony.l.nguyen@intel.com>
Subject: Re: [PATCH net-next v3] ethtool: add netlink based get rss support
Date: Mon, 21 Nov 2022 10:58:41 -0800	[thread overview]
Message-ID: <20221121105841.214ce8e2@kernel.org> (raw)
In-Reply-To: <20221120210217.zcdmr47r6ck33cf4@lion.mk-sys.cz>

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

  reply	other threads:[~2022-11-21 18:58 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-16 23:25 [PATCH net-next v3] ethtool: add netlink based get rss support Sudheer Mogilappagari
2022-11-17  3:12 ` Michal Kubecek
2022-11-18  4:08 ` Jakub Kicinski
2022-11-18 10:38 ` Francois Romieu
2022-11-20  8:40   ` Mogilappagari, Sudheer
2022-11-20 15:37     ` Francois Romieu
2022-11-20 21:02     ` Michal Kubecek
2022-11-21 18:58       ` Jakub Kicinski [this message]
2022-11-23 19:05         ` Mogilappagari, Sudheer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20221121105841.214ce8e2@kernel.org \
    --to=kuba@kernel.org \
    --cc=andrew@lunn.ch \
    --cc=anthony.l.nguyen@intel.com \
    --cc=corbet@lwn.net \
    --cc=mkubecek@suse.cz \
    --cc=netdev@vger.kernel.org \
    --cc=romieu@fr.zoreil.com \
    --cc=sridhar.samudrala@intel.com \
    --cc=sudheer.mogilappagari@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).