[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20220314131114.635d5acb@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date: Mon, 14 Mar 2022 13:11:14 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: "Samudrala, Sridhar" <sridhar.samudrala@...el.com>
Cc: Tony Nguyen <anthony.l.nguyen@...el.com>, <davem@...emloft.net>,
<netdev@...r.kernel.org>, <sudheer.mogilappagari@...el.com>,
<amritha.nambiar@...el.com>, <jiri@...dia.com>,
<leonro@...dia.com>,
"Bharathi Sreenivas" <bharathi.sreenivas@...el.com>
Subject: Re: [PATCH net-next 2/2] ice: Add inline flow director support for
channels
On Sun, 13 Mar 2022 17:11:42 -0500 Samudrala, Sridhar wrote:
> > ethtool has RSS context which is what you should use.
> > I presume you used TCs because it's a quick shortcut for getting
> > rate control?
>
> When tc mqprio hw offload is used to split tx/rx queues into queue groups, we
> create rss contexts per queue group automatically.
> Today we are not exposing the rss indirection table for queue groups via ethtool.
> We will add that support by enabling get_rxfh_context() ethtool_ops.
👍
> IIUC, you are suggesting using the rsvd fields in struct ethtool_rxfh to enable
> additional per-rss_context parameters.
Poosssibllyy... I'm not sure if it's initialized by user space, always?
You'll need to double check that, since we haven't been enforcing that
it's set to zero in the kernel.
If not we can limit the feature to ethtool netlink. Either way I think
we can keep the driver facing API intact.
> ethtool -X enp175s0f0 context 1 inline_fd on/off
>
> Will this be acceptable?
Slightly risky for me to agree because of the lack of documentation -
I can't be sure what the knob does, exactly.
If I'm reading the cover letter right the feature flips from hash-based
RSS to assigning flows in a round-robin fashion. Are there flows which
remain hashed or everything goes to round robin? We should document
what the eviction policy is as well.
I think we may need to polish the exact definition of what on/off means
a little more but your example command seems quite plausible at this
point.
Powered by blists - more mailing lists