netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: "Nambiar, Amritha" <amritha.nambiar@intel.com>,
	"Nguyen, Anthony L" <anthony.l.nguyen@intel.com>,
	"davem@davemloft.net" <davem@davemloft.net>,
	"pabeni@redhat.com" <pabeni@redhat.com>,
	"edumazet@google.com" <edumazet@google.com>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"Mogilappagari, Sudheer" <sudheer.mogilappagari@intel.com>,
	"Samudrala, Sridhar" <sridhar.samudrala@intel.com>,
	"Sreenivas, Bharathi" <bharathi.sreenivas@intel.com>
Subject: Re: [PATCH net-next 01/11] ice: Add support for classid based queue selection
Date: Tue, 3 May 2022 08:47:32 -0700	[thread overview]
Message-ID: <20220503084732.363b89cc@kernel.org> (raw)
In-Reply-To: <d3935370-b12c-e9db-1e59-52c8cceacf9a@mojatatu.com>

On Tue, 3 May 2022 06:32:01 -0400 Jamal Hadi Salim wrote:
> I am on the fence of "six of one, half a dozen of the other" ;->
> 
> TC classids have *always been used to identify queues* (or hierarchy of
> but always leading to a single queue). Essentially a classid identity
> is equivalent to a built-in action which says which queue to pick.
> The data structure is very much engrained in the tc psyche.
> 
> When TX side HW queues(IIRC, mqprio) showed up there was ambiguity to
> distinguish between a s/w queue vs a h/w queue hence queue_mapping
> which allows us to override the *HW TX queue* selection - or at least
> that was the intended goal.
> Note: There are other ways to tell the h/w what queues to use on TX
> (eg skb->priority) i.e there's no monopoly by queue_mapping.
> 
> Given lack of s/w queues on RX (hence lack of ambiguity) it seemed
> natural that the classid could be used to describe the RX queue
> identity for offload, it's just there.
> I thought it was brilliant when Amritha first posted.

Is it just me or is TC generally considered highly confusing?
IMO using a qdisc cls construct in clsact is only going to add 
to that.

Assigning classid can still be meaningful on ingress in case 
of a switch where there are actual qdiscs offloaded.

> I think we should pick the simpler of "half-dozen or six".
> The posted patch seems driver-only change i.e no core code
> changes required (which other vendors could follow).
> But i am not sure if that defines "simpler".

No core changes is not an argument we should take seriously upstream.

> BTW:
> Didnt follow the skb_record_rx_queue() thought - isnt that
> always set by the driver based on which h/w queue the packet
> arrived at? Whereas the semantics of this is to tell the h/w
> what queue to use.

Overriding the queue mapping in the SW could still be useful 
if TC wants to override the actual queue ID assigned by the NIC.

This way whether the action gets offloaded or not the resulting
skb will have the same metadata (in case of offload because it 
came on the right queue and the driver set the mapping, in case
of sw because we overwrote the mapping).

  reply	other threads:[~2022-05-03 15:47 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-28 17:24 [PATCH net-next 00/11][pull request] 100GbE Intel Wired LAN Driver Updates 2022-04-28 Tony Nguyen
2022-04-28 17:24 ` [PATCH net-next 01/11] ice: Add support for classid based queue selection Tony Nguyen
2022-04-28 23:04   ` Jakub Kicinski
2022-04-29 23:43     ` Nambiar, Amritha
2022-04-30  0:17       ` Jakub Kicinski
2022-04-30  2:00         ` Nambiar, Amritha
2022-04-30  2:42           ` Jakub Kicinski
2022-05-03 10:32             ` Jamal Hadi Salim
2022-05-03 15:47               ` Jakub Kicinski [this message]
2022-05-03 21:44                 ` Jamal Hadi Salim
2022-04-28 17:24 ` [PATCH net-next 02/11] ice: use min_t() to make code cleaner in ice_gnss Tony Nguyen
2022-04-28 17:24 ` [PATCH net-next 03/11] ice: introduce common helper for retrieving VSI by vsi_num Tony Nguyen
2022-04-28 17:24 ` [PATCH net-next 04/11] ice: return ENOSPC when exceeding ICE_MAX_CHAIN_WORDS Tony Nguyen
2022-04-28 17:24 ` [PATCH net-next 05/11] ice: get switch id on switchdev devices Tony Nguyen
2022-04-28 17:24 ` [PATCH net-next 06/11] ice: add newline to dev_dbg in ice_vf_fdir_dump_info Tony Nguyen
2022-04-28 17:24 ` [PATCH net-next 07/11] ice: always check VF VSI pointer values Tony Nguyen
2022-04-28 17:24 ` [PATCH net-next 08/11] ice: remove return value comment for ice_reset_all_vfs Tony Nguyen
2022-04-28 17:24 ` [PATCH net-next 09/11] ice: fix wording in comment for ice_reset_vf Tony Nguyen
2022-04-28 17:24 ` [PATCH net-next 10/11] ice: add a function comment for ice_cfg_mac_antispoof Tony Nguyen
2022-04-28 17:24 ` [PATCH net-next 11/11] ice: remove period on argument description in ice_for_each_vf Tony Nguyen

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=20220503084732.363b89cc@kernel.org \
    --to=kuba@kernel.org \
    --cc=amritha.nambiar@intel.com \
    --cc=anthony.l.nguyen@intel.com \
    --cc=bharathi.sreenivas@intel.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=jhs@mojatatu.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.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).