DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [RFC] ethdev: support input set change by RSS action
@ 2019-07-04  4:47 simei
  2019-07-04  9:07 ` Adrien Mazarguil
  0 siblings, 1 reply; 6+ messages in thread
From: simei @ 2019-07-04  4:47 UTC (permalink / raw)
  To: qi.z.zhang, jingjing.wu, beilei.xing, qiming.yang; +Cc: dev, simei.su

From: Simei Su <simei.su@intel.com>

This RFC introduces inputset structure to rte_flow_action_rss to
support input set specific configuration by rte_flow RSS action.

We can give an testpmd command line example to make it more clear.

For example, below flow selects the l4 port as inputset for any
eth/ipv4/tcp packet: #flow create 0 ingress pattern eth / ipv4 / tcp /
end actions rss inputset tcp src mask 0xffff dst mask 0xffff /end

Signed-off-by: Simei Su <simei.su@intel.com>
---
 lib/librte_ethdev/rte_flow.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
index f3a8fb1..2a455b6 100644
--- a/lib/librte_ethdev/rte_flow.h
+++ b/lib/librte_ethdev/rte_flow.h
@@ -1796,6 +1796,9 @@ struct rte_flow_action_rss {
 	uint32_t queue_num; /**< Number of entries in @p queue. */
 	const uint8_t *key; /**< Hash key. */
 	const uint16_t *queue; /**< Queue indices to use. */
+	struct rte_flow_item *inputset; /** Provide more specific inputset configuration.
+					 * ignore spec, only mask.
+					 */
 };
 
 /**
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-dev] [RFC] ethdev: support input set change by RSS action
  2019-07-04  4:47 [dpdk-dev] [RFC] ethdev: support input set change by RSS action simei
@ 2019-07-04  9:07 ` Adrien Mazarguil
  2019-07-04 13:55   ` Zhang, Qi Z
  0 siblings, 1 reply; 6+ messages in thread
From: Adrien Mazarguil @ 2019-07-04  9:07 UTC (permalink / raw)
  To: simei; +Cc: qi.z.zhang, jingjing.wu, beilei.xing, qiming.yang, dev

On Thu, Jul 04, 2019 at 12:47:09PM +0800, simei wrote:
> From: Simei Su <simei.su@intel.com>
> 
> This RFC introduces inputset structure to rte_flow_action_rss to
> support input set specific configuration by rte_flow RSS action.
> 
> We can give an testpmd command line example to make it more clear.
> 
> For example, below flow selects the l4 port as inputset for any
> eth/ipv4/tcp packet: #flow create 0 ingress pattern eth / ipv4 / tcp /
> end actions rss inputset tcp src mask 0xffff dst mask 0xffff /end
> 
> Signed-off-by: Simei Su <simei.su@intel.com>
> ---
>  lib/librte_ethdev/rte_flow.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/lib/librte_ethdev/rte_flow.h b/lib/librte_ethdev/rte_flow.h
> index f3a8fb1..2a455b6 100644
> --- a/lib/librte_ethdev/rte_flow.h
> +++ b/lib/librte_ethdev/rte_flow.h
> @@ -1796,6 +1796,9 @@ struct rte_flow_action_rss {
>  	uint32_t queue_num; /**< Number of entries in @p queue. */
>  	const uint8_t *key; /**< Hash key. */
>  	const uint16_t *queue; /**< Queue indices to use. */
> +	struct rte_flow_item *inputset; /** Provide more specific inputset configuration.
> +					 * ignore spec, only mask.
> +					 */
>  };
>  
>  /**

To make sure I understand, is this kind of a more flexible version of
rte_flow_action_rss.types?

For instance while specifying .types = ETH_RSS_IPV4 normally covers both
source and destination addresses, does this approach enable users to perform
RSS on source IP only? In which case, what value does the Toeplitz algorithm
assume for the destination, 0x0? (note: must be documented)

My opinion is that, unless you know of a hardware which can perform RSS on
random bytes of a packet, this approach is a bit overkill at this point.

How about simply adding the needed ETH_RSS_* definitions
(e.g. ETH_RSS_IPV4_(SRC|DST))? How many are needed?

There are currently 20 used bits and struct rte_flow_action_rss.types is
64-bit wide. I'm sure we can manage something without harming the ABI. Even
better, you wouldn't need a deprecation notice.

If you use the suggested approach, please update testpmd and its
documentation as part of the same patch, thanks.

-- 
Adrien Mazarguil
6WIND

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-dev] [RFC] ethdev: support input set change by RSS action
  2019-07-04  9:07 ` Adrien Mazarguil
@ 2019-07-04 13:55   ` Zhang, Qi Z
  2019-07-04 14:08     ` Adrien Mazarguil
  0 siblings, 1 reply; 6+ messages in thread
From: Zhang, Qi Z @ 2019-07-04 13:55 UTC (permalink / raw)
  To: Adrien Mazarguil, Su, Simei; +Cc: Wu, Jingjing, Xing, Beilei, Yang, Qiming, dev



> -----Original Message-----
> From: Adrien Mazarguil [mailto:adrien.mazarguil@6wind.com]
> Sent: Thursday, July 4, 2019 5:07 PM
> To: Su, Simei <simei.su@intel.com>
> Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>;
> Xing, Beilei <beilei.xing@intel.com>; Yang, Qiming <qiming.yang@intel.com>;
> dev@dpdk.org
> Subject: Re: [dpdk-dev] [RFC] ethdev: support input set change by RSS action
> 
> On Thu, Jul 04, 2019 at 12:47:09PM +0800, simei wrote:
> > From: Simei Su <simei.su@intel.com>
> >
> > This RFC introduces inputset structure to rte_flow_action_rss to
> > support input set specific configuration by rte_flow RSS action.
> >
> > We can give an testpmd command line example to make it more clear.
> >
> > For example, below flow selects the l4 port as inputset for any
> > eth/ipv4/tcp packet: #flow create 0 ingress pattern eth / ipv4 / tcp /
> > end actions rss inputset tcp src mask 0xffff dst mask 0xffff /end
> >
> > Signed-off-by: Simei Su <simei.su@intel.com>
> > ---
> >  lib/librte_ethdev/rte_flow.h | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/lib/librte_ethdev/rte_flow.h
> > b/lib/librte_ethdev/rte_flow.h index f3a8fb1..2a455b6 100644
> > --- a/lib/librte_ethdev/rte_flow.h
> > +++ b/lib/librte_ethdev/rte_flow.h
> > @@ -1796,6 +1796,9 @@ struct rte_flow_action_rss {
> >  	uint32_t queue_num; /**< Number of entries in @p queue. */
> >  	const uint8_t *key; /**< Hash key. */
> >  	const uint16_t *queue; /**< Queue indices to use. */
> > +	struct rte_flow_item *inputset; /** Provide more specific inputset
> configuration.
> > +					 * ignore spec, only mask.
> > +					 */
> >  };
> >
> >  /**
> 
> To make sure I understand, is this kind of a more flexible version of
> rte_flow_action_rss.types?

Yes
> 
> For instance while specifying .types = ETH_RSS_IPV4 normally covers both
> source and destination addresses, does this approach enable users to perform
> RSS on source IP only? 

Yes, .it is the case to select any subset of 5 tuples or even tunnel header's id for hash

> In which case, what value does the Toeplitz algorithm
> assume for the destination, 0x0? (note: must be documented)

My understanding is src/dst pair is only required for a symmetric case
But for Toeplitz, it is just a hash function, it process a serial of data with specific algorithm, have no idea about which part is src and dst , 
So for ip src only with Toeplitz, dst is not required to be a placeholder..
anything I missed, would you share more insight?

Thanks
Qi

> 
> My opinion is that, unless you know of a hardware which can perform RSS on
> random bytes of a packet, this approach is a bit overkill at this point.
> 
> How about simply adding the needed ETH_RSS_* definitions (e.g.
> ETH_RSS_IPV4_(SRC|DST))? How many are needed?
> 
> There are currently 20 used bits and struct rte_flow_action_rss.types is 64-bit
> wide. I'm sure we can manage something without harming the ABI. Even
> better, you wouldn't need a deprecation notice.
> 
> If you use the suggested approach, please update testpmd and its
> documentation as part of the same patch, thanks.
> 
> --
> Adrien Mazarguil
> 6WIND

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-dev] [RFC] ethdev: support input set change by RSS action
  2019-07-04 13:55   ` Zhang, Qi Z
@ 2019-07-04 14:08     ` Adrien Mazarguil
  2019-07-09  5:41       ` Zhang, Qi Z
  0 siblings, 1 reply; 6+ messages in thread
From: Adrien Mazarguil @ 2019-07-04 14:08 UTC (permalink / raw)
  To: Zhang, Qi Z; +Cc: Su, Simei, Wu, Jingjing, Xing, Beilei, Yang, Qiming, dev

On Thu, Jul 04, 2019 at 01:55:14PM +0000, Zhang, Qi Z wrote:
> 
> 
> > -----Original Message-----
> > From: Adrien Mazarguil [mailto:adrien.mazarguil@6wind.com]
> > Sent: Thursday, July 4, 2019 5:07 PM
> > To: Su, Simei <simei.su@intel.com>
> > Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>;
> > Xing, Beilei <beilei.xing@intel.com>; Yang, Qiming <qiming.yang@intel.com>;
> > dev@dpdk.org
> > Subject: Re: [dpdk-dev] [RFC] ethdev: support input set change by RSS action
> > 
> > On Thu, Jul 04, 2019 at 12:47:09PM +0800, simei wrote:
> > > From: Simei Su <simei.su@intel.com>
> > >
> > > This RFC introduces inputset structure to rte_flow_action_rss to
> > > support input set specific configuration by rte_flow RSS action.
> > >
> > > We can give an testpmd command line example to make it more clear.
> > >
> > > For example, below flow selects the l4 port as inputset for any
> > > eth/ipv4/tcp packet: #flow create 0 ingress pattern eth / ipv4 / tcp /
> > > end actions rss inputset tcp src mask 0xffff dst mask 0xffff /end
> > >
> > > Signed-off-by: Simei Su <simei.su@intel.com>
> > > ---
> > >  lib/librte_ethdev/rte_flow.h | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> > > diff --git a/lib/librte_ethdev/rte_flow.h
> > > b/lib/librte_ethdev/rte_flow.h index f3a8fb1..2a455b6 100644
> > > --- a/lib/librte_ethdev/rte_flow.h
> > > +++ b/lib/librte_ethdev/rte_flow.h
> > > @@ -1796,6 +1796,9 @@ struct rte_flow_action_rss {
> > >  	uint32_t queue_num; /**< Number of entries in @p queue. */
> > >  	const uint8_t *key; /**< Hash key. */
> > >  	const uint16_t *queue; /**< Queue indices to use. */
> > > +	struct rte_flow_item *inputset; /** Provide more specific inputset
> > configuration.
> > > +					 * ignore spec, only mask.
> > > +					 */
> > >  };
> > >
> > >  /**
> > 
> > To make sure I understand, is this kind of a more flexible version of
> > rte_flow_action_rss.types?
> 
> Yes
> > 
> > For instance while specifying .types = ETH_RSS_IPV4 normally covers both
> > source and destination addresses, does this approach enable users to perform
> > RSS on source IP only? 
> 
> Yes, .it is the case to select any subset of 5 tuples or even tunnel header's id for hash
> 
> > In which case, what value does the Toeplitz algorithm
> > assume for the destination, 0x0? (note: must be documented)
> 
> My understanding is src/dst pair is only required for a symmetric case
> But for Toeplitz, it is just a hash function, it process a serial of data with specific algorithm, have no idea about which part is src and dst , 
> So for ip src only with Toeplitz, dst is not required to be a placeholder..

Right, I had symmetric Toeplitz in mind and wondered what would happen when
users would not select the required fields. I guess the PMD would have to
reject unsupported combinations.

> anything I missed, would you share more insight?

No, that answers the question, thanks.

Now what about my suggestion below? In short: extending ETH_RSS_* assuming
there's enough bits left in there, instead of adding a whole new framework
and breaking ABI in the process.

> > My opinion is that, unless you know of a hardware which can perform RSS on
> > random bytes of a packet, this approach is a bit overkill at this point.
> > 
> > How about simply adding the needed ETH_RSS_* definitions (e.g.
> > ETH_RSS_IPV4_(SRC|DST))? How many are needed?
> > 
> > There are currently 20 used bits and struct rte_flow_action_rss.types is 64-bit
> > wide. I'm sure we can manage something without harming the ABI. Even
> > better, you wouldn't need a deprecation notice.
> > 
> > If you use the suggested approach, please update testpmd and its
> > documentation as part of the same patch, thanks.

-- 
Adrien Mazarguil
6WIND

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-dev] [RFC] ethdev: support input set change by RSS action
  2019-07-04 14:08     ` Adrien Mazarguil
@ 2019-07-09  5:41       ` Zhang, Qi Z
  2019-07-09  8:19         ` Jerin Jacob Kollanukkaran
  0 siblings, 1 reply; 6+ messages in thread
From: Zhang, Qi Z @ 2019-07-09  5:41 UTC (permalink / raw)
  To: Adrien Mazarguil; +Cc: Su, Simei, Wu, Jingjing, Xing, Beilei, Yang, Qiming, dev

Hi Adrien:

> -----Original Message-----
> From: Adrien Mazarguil [mailto:adrien.mazarguil@6wind.com]
> Sent: Thursday, July 4, 2019 10:09 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: Su, Simei <simei.su@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>;
> Xing, Beilei <beilei.xing@intel.com>; Yang, Qiming <qiming.yang@intel.com>;
> dev@dpdk.org
> Subject: Re: [dpdk-dev] [RFC] ethdev: support input set change by RSS action
> 
> On Thu, Jul 04, 2019 at 01:55:14PM +0000, Zhang, Qi Z wrote:
> >
> >
> > > -----Original Message-----
> > > From: Adrien Mazarguil [mailto:adrien.mazarguil@6wind.com]
> > > Sent: Thursday, July 4, 2019 5:07 PM
> > > To: Su, Simei <simei.su@intel.com>
> > > Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; Wu, Jingjing
> > > <jingjing.wu@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Yang,
> > > Qiming <qiming.yang@intel.com>; dev@dpdk.org
> > > Subject: Re: [dpdk-dev] [RFC] ethdev: support input set change by
> > > RSS action
> > >
> > > On Thu, Jul 04, 2019 at 12:47:09PM +0800, simei wrote:
> > > > From: Simei Su <simei.su@intel.com>
> > > >
> > > > This RFC introduces inputset structure to rte_flow_action_rss to
> > > > support input set specific configuration by rte_flow RSS action.
> > > >
> > > > We can give an testpmd command line example to make it more clear.
> > > >
> > > > For example, below flow selects the l4 port as inputset for any
> > > > eth/ipv4/tcp packet: #flow create 0 ingress pattern eth / ipv4 /
> > > > tcp / end actions rss inputset tcp src mask 0xffff dst mask 0xffff
> > > > /end
> > > >
> > > > Signed-off-by: Simei Su <simei.su@intel.com>
> > > > ---
> > > >  lib/librte_ethdev/rte_flow.h | 3 +++
> > > >  1 file changed, 3 insertions(+)
> > > >
> > > > diff --git a/lib/librte_ethdev/rte_flow.h
> > > > b/lib/librte_ethdev/rte_flow.h index f3a8fb1..2a455b6 100644
> > > > --- a/lib/librte_ethdev/rte_flow.h
> > > > +++ b/lib/librte_ethdev/rte_flow.h
> > > > @@ -1796,6 +1796,9 @@ struct rte_flow_action_rss {
> > > >  	uint32_t queue_num; /**< Number of entries in @p queue. */
> > > >  	const uint8_t *key; /**< Hash key. */
> > > >  	const uint16_t *queue; /**< Queue indices to use. */
> > > > +	struct rte_flow_item *inputset; /** Provide more specific
> > > > +inputset
> > > configuration.
> > > > +					 * ignore spec, only mask.
> > > > +					 */
> > > >  };
> > > >
> > > >  /**
> > >
> > > To make sure I understand, is this kind of a more flexible version
> > > of rte_flow_action_rss.types?
> >
> > Yes
> > >
> > > For instance while specifying .types = ETH_RSS_IPV4 normally covers
> > > both source and destination addresses, does this approach enable
> > > users to perform RSS on source IP only?
> >
> > Yes, .it is the case to select any subset of 5 tuples or even tunnel
> > header's id for hash
> >
> > > In which case, what value does the Toeplitz algorithm assume for the
> > > destination, 0x0? (note: must be documented)
> >
> > My understanding is src/dst pair is only required for a symmetric case
> > But for Toeplitz, it is just a hash function, it process a serial of
> > data with specific algorithm, have no idea about which part is src and dst , So
> for ip src only with Toeplitz, dst is not required to be a placeholder..
> 
> Right, I had symmetric Toeplitz in mind and wondered what would happen
> when users would not select the required fields. I guess the PMD would have
> to reject unsupported combinations.
> 
> > anything I missed, would you share more insight?
> 
> No, that answers the question, thanks.
> 
> Now what about my suggestion below? In short: extending ETH_RSS_*
> assuming there's enough bits left in there, instead of adding a whole new
> framework and breaking ABI in the process.

Since the hardware can support any combination of input set (not just 5 tuples), we'd like to make it more generic.
Those will be some pain due to ABI break, but we think its worth for long term.

Thanks
Qi

> 
> > > My opinion is that, unless you know of a hardware which can perform
> > > RSS on random bytes of a packet, this approach is a bit overkill at this
> point.
> > >
> > > How about simply adding the needed ETH_RSS_* definitions (e.g.
> > > ETH_RSS_IPV4_(SRC|DST))? How many are needed?
> > >
> > > There are currently 20 used bits and struct
> > > rte_flow_action_rss.types is 64-bit wide. I'm sure we can manage
> > > something without harming the ABI. Even better, you wouldn't need a
> deprecation notice.
> > >
> > > If you use the suggested approach, please update testpmd and its
> > > documentation as part of the same patch, thanks.
> 
> --
> Adrien Mazarguil
> 6WIND

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-dev] [RFC] ethdev: support input set change by RSS action
  2019-07-09  5:41       ` Zhang, Qi Z
@ 2019-07-09  8:19         ` Jerin Jacob Kollanukkaran
  0 siblings, 0 replies; 6+ messages in thread
From: Jerin Jacob Kollanukkaran @ 2019-07-09  8:19 UTC (permalink / raw)
  To: Zhang, Qi Z, Adrien Mazarguil
  Cc: Su, Simei, Wu, Jingjing, Xing, Beilei, Yang, Qiming, dev

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Zhang, Qi Z
> Sent: Tuesday, July 9, 2019 11:11 AM
> To: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> Cc: Su, Simei <simei.su@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>;
> Xing, Beilei <beilei.xing@intel.com>; Yang, Qiming <qiming.yang@intel.com>;
> dev@dpdk.org
> Subject: Re: [dpdk-dev] [RFC] ethdev: support input set change by RSS action
> > Right, I had symmetric Toeplitz in mind and wondered what would happen
> > when users would not select the required fields. I guess the PMD would
> > have to reject unsupported combinations.
> >
> > > anything I missed, would you share more insight?
> >
> > No, that answers the question, thanks.
> >
> > Now what about my suggestion below? In short: extending ETH_RSS_*
> > assuming there's enough bits left in there, instead of adding a whole
> > new framework and breaking ABI in the process.
> 
> Since the hardware can support any combination of input set (not just 5
> tuples), we'd like to make it more generic.
> Those will be some pain due to ABI break, but we think its worth for long
> term.

# IMO, The decided method should be compatible with normal RSS(without rte_flow) case 
as well(Currently is struct rte_eth_conf.rx_adv_conf.rss_conf.rss_hf used 
in the dev_configure()) as from HW perspective it will be same  the HW block which does 
both.
# From Marvell HW point of view, We can support any combination of input set.
But it is more of a micro code(i.e need to first provide the list protocol supported to HW)
and then use it latter. So, I think, extending the ETH_RSS_* would be more portable
approach __without loosing__ the functionality. Since there is around 40 bits are left, I think
more standard protocol can fit in the 40 bits. 





^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-07-09  8:19 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-04  4:47 [dpdk-dev] [RFC] ethdev: support input set change by RSS action simei
2019-07-04  9:07 ` Adrien Mazarguil
2019-07-04 13:55   ` Zhang, Qi Z
2019-07-04 14:08     ` Adrien Mazarguil
2019-07-09  5:41       ` Zhang, Qi Z
2019-07-09  8:19         ` Jerin Jacob Kollanukkaran

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).