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

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 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]

Date:   Sat, 30 Apr 2022 02:00:05 +0000
From:   "Nambiar, Amritha" <amritha.nambiar@...el.com>
To:     Jakub Kicinski <kuba@...nel.org>
CC:     "Nguyen, Anthony L" <anthony.l.nguyen@...el.com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "pabeni@...hat.com" <pabeni@...hat.com>,
        "edumazet@...gle.com" <edumazet@...gle.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "Mogilappagari, Sudheer" <sudheer.mogilappagari@...el.com>,
        "Samudrala, Sridhar" <sridhar.samudrala@...el.com>,
        "Sreenivas, Bharathi" <bharathi.sreenivas@...el.com>,
        Jamal Hadi Salim <jhs@...atatu.com>
Subject: RE: [PATCH net-next 01/11] ice: Add support for classid based queue
 selection

> -----Original Message-----
> From: Jakub Kicinski <kuba@...nel.org>
> Sent: Friday, April 29, 2022 5:17 PM
> To: Nambiar, Amritha <amritha.nambiar@...el.com>
> Cc: Nguyen, Anthony L <anthony.l.nguyen@...el.com>;
> davem@...emloft.net; pabeni@...hat.com; edumazet@...gle.com;
> netdev@...r.kernel.org; Mogilappagari, Sudheer
> <sudheer.mogilappagari@...el.com>; Samudrala, Sridhar
> <sridhar.samudrala@...el.com>; Sreenivas, Bharathi
> <bharathi.sreenivas@...el.com>; Jamal Hadi Salim <jhs@...atatu.com>
> Subject: Re: [PATCH net-next 01/11] ice: Add support for classid based queue
> selection
> 
> 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?

IIUC, currently the action skbedit queue_mapping is for transmit queue selection,
and the bound checking is w.r.to dev->real_num_tx_queues. Also, based on my
discussion with Alex (https://www.spinics.net/lists/netdev/msg761581.html), it
looks like this currently applies at the qdisc enqueue stage and not at the
classifier level. Alex had proposed a solution to make this more generic to work
with qdisc, ingress and egress hooks, but I haven't had a chance to get to it (being
on medical leave).