netdev - RE: [PATCH net-next v2 2/3] dpaa2-eth: Support multiple traffic classes on Tx

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 linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <AM0PR04MB4994174563D3826C2561BFFF94100@AM0PR04MB4994.eurprd04.prod.outlook.com>
Date:   Fri, 7 Jun 2019 07:47:57 +0000
From:   Ioana Ciocoi Radulescu <ruxandra.radulescu@....com>
To:     David Miller <davem@...emloft.net>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Ioana Ciornei <ioana.ciornei@....com>
Subject: RE: [PATCH net-next v2 2/3] dpaa2-eth: Support multiple traffic
 classes on Tx

> -----Original Message-----
> From: David Miller <davem@...emloft.net>
> Sent: Thursday, June 6, 2019 9:03 PM
> To: Ioana Ciocoi Radulescu <ruxandra.radulescu@....com>
> Cc: netdev@...r.kernel.org; Ioana Ciornei <ioana.ciornei@....com>
> Subject: Re: [PATCH net-next v2 2/3] dpaa2-eth: Support multiple traffic
> classes on Tx
> 
> From: Ioana Radulescu <ruxandra.radulescu@....com>
> Date: Thu,  6 Jun 2019 11:50:28 +0300
> 
> > DPNI objects can have multiple traffic classes, as reflected by
> > the num_tc attribute. Until now we ignored its value and only
> > used traffic class 0.
> >
> > This patch adds support for multiple Tx traffic classes; the skb
> > priority information received from the stack is used to select the
> > hardware Tx queue on which to enqueue the frame.
> >
> > Signed-off-by: Ioana Radulescu <ruxandra.radulescu@....com>
> > ---
> > v2: Extra processing on the fast path happens only when TC is used
> >
> >  drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c | 47
> ++++++++++++++++--------
> >  drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.h |  9 ++++-
> >  2 files changed, 40 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
> b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
> > index a12fc45..98de092 100644
> > --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
> > +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth.c
> > @@ -757,6 +757,7 @@ static netdev_tx_t dpaa2_eth_tx(struct sk_buff
> *skb, struct net_device *net_dev)
> >  	u16 queue_mapping;
> >  	unsigned int needed_headroom;
> >  	u32 fd_len;
> > +	u8 prio = 0;
> >  	int err, i;
> >
> >  	percpu_stats = this_cpu_ptr(priv->percpu_stats);
> > @@ -814,6 +815,18 @@ static netdev_tx_t dpaa2_eth_tx(struct sk_buff
> *skb, struct net_device *net_dev)
> >  	 * a queue affined to the same core that processed the Rx frame
> >  	 */
> >  	queue_mapping = skb_get_queue_mapping(skb);
> > +
> > +	if (net_dev->num_tc) {
> > +		prio = netdev_txq_to_tc(net_dev, queue_mapping);
> > +		/* Hardware interprets priority level 0 as being the highest,
> > +		 * so we need to do a reverse mapping to the netdev tc index
> > +		 */
> > +		prio = net_dev->num_tc - prio - 1;
> > +		/* We have only one FQ array entry for all Tx hardware
> queues
> > +		 * with the same flow id (but different priority levels)
> > +		 */
> > +		queue_mapping %= dpaa2_eth_queue_count(priv);
> 
> This doesn't make any sense.
> 
> queue_mapping came from skb_get_queue_mapping().
> 
> The core limits the queue mapping value to whatever you told the
> generic networking layer was the maximum number of queues.
> 
> And you set that to dpaa2_eth_queue_count():
> 
> 	/* Set actual number of queues in the net device */
> 	num_queues = dpaa2_eth_queue_count(priv);
> 	err = netif_set_real_num_tx_queues(net_dev, num_queues);
> 
> Therfore the modulus cannot be needed.

True, at this point it's not needed yet. This patch adds code in
preparation for patch 3/3, which does modify the maximum
number of queues on the device when multiple TCs are configured
via mqprio.

I can move it to the next patch where it's use is more clear.

Thanks,
Ioana