Re: [PATCH net-next 01/11] ice: Add support for classid based queue selection

From: Jakub Kicinski <kuba@kernel.org>
To: "Nambiar, Amritha" <amritha.nambiar@intel.com>
Cc: "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>,
	Jamal Hadi Salim <jhs@mojatatu.com>
Subject: Re: [PATCH net-next 01/11] ice: Add support for classid based queue selection
Date: Fri, 29 Apr 2022 17:17:17 -0700	[thread overview]
Message-ID: <20220429171717.5b0b2a81@kernel.org> (raw)
In-Reply-To: <MWHPR11MB1293C17C30E689270E0C39AAF1FC9@MWHPR11MB1293.namprd11.prod.outlook.com>

On Fri, 29 Apr 2022 23:43:06 +0000 Nambiar, Amritha wrote:
> > > HW.
> > > Example:
> > > $ tc filter add dev ens4f0 protocol ip ingress flower\
> > >   dst_ip 192.168.1.12 ip_proto tcp dst_port 5001\
> > >   skip_sw classid ffff:0x5
> > >
> > > The above command adds an ingress filter, the accepted packets
> > > will be directed to queue 4. The major number represents the ingress  
> > 
> > ..and "directed" here. TC is used for so many different things you
> > really need to explain what your use case is.
> >   
> 
> Sorry about using the terms "forward" and "direct" interchangeably in this
> context. I should have been more consistent with the terminology. 
> 
> The use case is to accept incoming packets into a queue via TC ingress filter.
> TC filters are offloaded to a hardware table called the "switch" table. This
> table supports two types of actions in hardware termed as "forward to queue" and 
> "forward to a VSI aka queue-group". Accepting packets into a queue using
> ethtool filter is also supported, but this type of filter is added into a 
> different hardware table called the "flow director" table. The flow director
> table has certain restrictions that it can only have filters with the same packet
> type. The switch table does not have this restriction.
> 
> > > qdisc. The general rule is "classID's minor number - 1" upto max
> > > queues supported. The queue number is in hex format.  
> > 
> > The "general rule" you speak of is a rule you'd like to establish,
> > or an existing rule?  
> 
> This is an existing rule already being used in the TX qdiscs. We are using
> this in the ingress qdisc and offloading RX filters following the explanation
> from Netdev 0x13 session presented by Jamal. Section 4.1 from
> https://legacy.netdevconf.info/0x13/session.html?talk-tc-u-classifier
> "There is one interesting tidbit on the above rule exposed
> via the "classid 1:1" construct: the accepted packets will be
> queued to DMA ring 0. If classid 1:2 was used then they
> would be queued to DMA ring 1 etc. The general rule is the
> "classid's minor number - 1" upto a max of DMA queues
> supported by the NIC (64 in the case of the ixgbe). By definition, this is
> how tc classids are intended to be used i.e they select queues (in this
> case hardware ingress queues)."

So we're faking mqprio behavior on ingress? I guess that's fine.

Wouldn't SKBEDIT_F_QUEUE_MAPPING be a more natural fit here?