DPDK patches and discussions
 help / color / mirror / Atom feed
From: Adrien Mazarguil <adrien.mazarguil@6wind.com>
To: Shahaf Shuler <shahafs@mellanox.com>
Cc: Ophir Munk <ophirmu@mellanox.com>, "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH 2/2] net/mlx4: refactor RSS conversion functions
Date: Mon, 21 May 2018 14:23:31 +0200	[thread overview]
Message-ID: <20180521122331.GS6497@6wind.com> (raw)
In-Reply-To: <DB7PR05MB4426DE4E87C6FE8D078B8E34C3950@DB7PR05MB4426.eurprd05.prod.outlook.com>

On Mon, May 21, 2018 at 10:59:36AM +0000, Shahaf Shuler wrote:
> Hi Adrien,
> 
> Please help me walkthrough this logic. Something doesn't end up correctly. 
> 
> Friday, May 18, 2018 12:49 PM, Adrien Mazarguil:
> > Cc: Shahaf Shuler <shahafs@mellanox.com>; dev@dpdk.org
> > Subject: Re: [PATCH 2/2] net/mlx4: refactor RSS conversion functions
> > > > Signed-off-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> > > > Cc: Ophir Munk <ophirmu@mellanox.com>
> > > >   *
> > > >   * @return
> > > > - *   A valid Verbs RSS hash fields mask for mlx4 on success, (uint64_t)-1
> > > > - *   otherwise and rte_errno is set.
> > > > + *   Converted RSS hash fields on success, (uint64_t)-1 otherwise and
> > > > + *   rte_errno is set.
> > > >   */
> > > >  uint64_t
> > > > -mlx4_conv_rss_types(struct priv *priv, uint64_t types)
> > > > +mlx4_conv_rss_types(struct priv *priv, uint64_t types, int
> > > > +verbs_to_dpdk)
> > > >  {
> 
> 
> Let's assume:
> Types = IBV_RX_HASH_SRC_IPV4 | IBV_RX_HASH_DST_IPV4
> And verbs_to_dpdk=1
> 
> > > > -	enum { IPV4, IPV6, TCP, UDP, };
> > > > -	static const uint64_t in[] = {
> > > > +	enum {
> > > > +		INNER, IPV4, IPV6, TCP, UDP,
> > > > +		IPV4_TCP, IPV4_UDP, IPV6_TCP, IPV6_UDP,
> > > > +	};
> > > > +	static const uint64_t dpdk[] = {
> > > > +		[INNER] = 0,
> > > >  		[IPV4] = (ETH_RSS_IPV4 |
> > > >  			  ETH_RSS_FRAG_IPV4 |
> > > > -			  ETH_RSS_NONFRAG_IPV4_TCP |
> > > > -			  ETH_RSS_NONFRAG_IPV4_UDP |
> > > >  			  ETH_RSS_NONFRAG_IPV4_OTHER),
> > > >  		[IPV6] = (ETH_RSS_IPV6 |
> > > >  			  ETH_RSS_FRAG_IPV6 |
> > > > -			  ETH_RSS_NONFRAG_IPV6_TCP |
> > > > -			  ETH_RSS_NONFRAG_IPV6_UDP |
> > > >  			  ETH_RSS_NONFRAG_IPV6_OTHER |
> > > > -			  ETH_RSS_IPV6_EX |
> > > > -			  ETH_RSS_IPV6_TCP_EX |
> > > > -			  ETH_RSS_IPV6_UDP_EX),
> > > > -		[TCP] = (ETH_RSS_NONFRAG_IPV4_TCP |
> > > > -			 ETH_RSS_NONFRAG_IPV6_TCP |
> > > > -			 ETH_RSS_IPV6_TCP_EX),
> > > > -		[UDP] = (ETH_RSS_NONFRAG_IPV4_UDP |
> > > > -			 ETH_RSS_NONFRAG_IPV6_UDP |
> > > > -			 ETH_RSS_IPV6_UDP_EX),
> > > > +			  ETH_RSS_IPV6_EX),
> > > > +		[TCP] = 0,
> > > > +		[UDP] = 0,
> > > > +		[IPV4_TCP] = ETH_RSS_NONFRAG_IPV4_TCP,
> > > > +		[IPV4_UDP] = ETH_RSS_NONFRAG_IPV4_UDP,
> > > > +		[IPV6_TCP] = (ETH_RSS_NONFRAG_IPV6_TCP |
> > > > +			      ETH_RSS_IPV6_TCP_EX),
> > > > +		[IPV6_UDP] = (ETH_RSS_NONFRAG_IPV6_UDP |
> > > > +			      ETH_RSS_IPV6_UDP_EX),
> > > >  	};
> > > > -	static const uint64_t out[RTE_DIM(in)] = {
> > > > +	const uint64_t verbs[RTE_DIM(dpdk)] = {
> > > > +		[INNER] = IBV_RX_HASH_INNER,
> > > >  		[IPV4] = IBV_RX_HASH_SRC_IPV4 |
> > IBV_RX_HASH_DST_IPV4,
> > > >  		[IPV6] = IBV_RX_HASH_SRC_IPV6 |
> > IBV_RX_HASH_DST_IPV6,
> > > >  		[TCP] = IBV_RX_HASH_SRC_PORT_TCP |
> > IBV_RX_HASH_DST_PORT_TCP,
> > > >  		[UDP] = IBV_RX_HASH_SRC_PORT_UDP |
> > IBV_RX_HASH_DST_PORT_UDP,
> > > > +		[IPV4_TCP] = verbs[IPV4] | verbs[TCP],
> > > > +		[IPV4_UDP] = verbs[IPV4] | verbs[UDP],
> > > > +		[IPV6_TCP] = verbs[IPV6] | verbs[TCP],
> > > > +		[IPV6_UDP] = verbs[IPV6] | verbs[UDP],
> > > >  	};
> > > > +	const uint64_t *in = verbs_to_dpdk ? verbs : dpdk;
> > > > +	const uint64_t *out = verbs_to_dpdk ? dpdk : verbs;
> > > >  	uint64_t seen = 0;
> > > >  	uint64_t conv = 0;
> > > >  	unsigned int i;
> > > >
> > > > -	if (!types)
> 
> Types != 0 , we skip. 
> 
> > > > -		return priv->hw_rss_sup;
> > > > -	for (i = 0; i != RTE_DIM(in); ++i)
> > > > +	if (!types) {
> > > > +		if (!verbs_to_dpdk)
> > > > +			return priv->hw_rss_sup;
> > > > +		types = priv->hw_rss_sup;
> > > > +	}
> > > > +	for (i = 0; i != RTE_DIM(dpdk); ++i)
> > > >  		if (types & in[i]) {
> > > >  			seen |= types & in[i];
> > > >  			conv |= out[i];
> > > >  		}
> 
> 
> After this loop:
> seen = (IBV_RX_HASH_SRC_PORT_IPV4 | IBV_RX_HASH_DST_IPV4)
> conv = dpdk[IPV4] | dpdk[IPV4_TCP] | dpdk[IPV4_UDP]

Now I understand the mistake.

> > > > -	if ((conv & priv->hw_rss_sup) == conv && !(types & ~seen))
> > > > +	if ((verbs_to_dpdk || (conv & priv->hw_rss_sup) == conv) &&
> > > > +	    !(types & ~seen))
> 
> All types were seen && verbs_to_dpdk hence returning that PMD support RSS based on IPV4 | TCPV4 | UDPV4
> While according to the reported capabilities form kernel only IPV4 is supported. 

Indeed, although this combination never occurs in practice, that's why I
didn't see any issue at first. I'll submit a fixed version. Thanks.

> > > >  		return conv;
> > > >  	rte_errno = ENOTSUP;
> > > >  	return (uint64_t)-1;
> > > >  }
> > > >
> > >

-- 
Adrien Mazarguil
6WIND

  reply	other threads:[~2018-05-21 12:23 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-15 15:51 [dpdk-dev] [PATCH 1/2] net/mlx4: fix inadequate default in RSS converter Adrien Mazarguil
2018-05-15 15:51 ` [dpdk-dev] [PATCH 2/2] net/mlx4: refactor RSS conversion functions Adrien Mazarguil
2018-05-17 17:46   ` Ophir Munk
2018-05-18  9:49     ` Adrien Mazarguil
2018-05-21 10:59       ` Shahaf Shuler
2018-05-21 12:23         ` Adrien Mazarguil [this message]
2018-05-21 15:50 ` [dpdk-dev] [PATCH v2 1/2] net/mlx4: fix inadequate default in RSS converter Adrien Mazarguil
2018-05-21 15:50   ` [dpdk-dev] [PATCH v2 2/2] net/mlx4: refactor RSS conversion functions Adrien Mazarguil
2018-05-22  9:41     ` Ferruh Yigit
2018-05-22  9:58       ` Adrien Mazarguil
2018-05-22 11:26     ` [dpdk-dev] [PATCH] net/mlx4: fix undefined behavior of RSS conversion Adrien Mazarguil
2018-05-22 13:01       ` Ferruh Yigit
2018-05-22 13:21         ` Adrien Mazarguil
2018-05-22  7:28   ` [dpdk-dev] [PATCH v2 1/2] net/mlx4: fix inadequate default in RSS converter Shahaf Shuler

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=20180521122331.GS6497@6wind.com \
    --to=adrien.mazarguil@6wind.com \
    --cc=dev@dpdk.org \
    --cc=ophirmu@mellanox.com \
    --cc=shahafs@mellanox.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).