All of lore.kernel.org
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] ethdev: add RSS offload type for L3 checksum
@ 2021-06-03  3:12 Alvin Zhang
  2021-06-03  7:14 ` Andrew Rybchenko
  2021-06-03  8:03 ` [dpdk-dev] [PATCH v2] ethdev: add IPv4 checksum RSS offload type Alvin Zhang
  0 siblings, 2 replies; 39+ messages in thread
From: Alvin Zhang @ 2021-06-03  3:12 UTC (permalink / raw)
  To: ferruh.yigit, qi.z.zhang; +Cc: dev, Alvin Zhang

Add ETH_RSS_L3_CHKSUM macro.

Signed-off-by: Alvin Zhang <alvinx.zhang@intel.com>
---
 lib/ethdev/rte_ethdev.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index faf3bd9..4220ec5 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -537,6 +537,7 @@ struct rte_eth_rss_conf {
 #define ETH_RSS_PPPOE		   (1ULL << 31)
 #define ETH_RSS_ECPRI		   (1ULL << 32)
 #define ETH_RSS_MPLS		   (1ULL << 33)
+#define ETH_RSS_L3_CHKSUM	   (1ULL << 34)
 
 /*
  * We use the following macros to combine with above ETH_RSS_* for
-- 
1.8.3.1


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

* Re: [dpdk-dev] [PATCH] ethdev: add RSS offload type for L3 checksum
  2021-06-03  3:12 [dpdk-dev] [PATCH] ethdev: add RSS offload type for L3 checksum Alvin Zhang
@ 2021-06-03  7:14 ` Andrew Rybchenko
  2021-06-03  7:28   ` Zhang, AlvinX
  2021-06-03  8:03 ` [dpdk-dev] [PATCH v2] ethdev: add IPv4 checksum RSS offload type Alvin Zhang
  1 sibling, 1 reply; 39+ messages in thread
From: Andrew Rybchenko @ 2021-06-03  7:14 UTC (permalink / raw)
  To: Alvin Zhang, ferruh.yigit, qi.z.zhang; +Cc: dev

On 6/3/21 6:12 AM, Alvin Zhang wrote:
> Add ETH_RSS_L3_CHKSUM macro.

Sorry, too short description. I could be an existing practice
to add more RSS offload types, but I want to break it.
Please, add motivation in the description why it is useful etc.

> 
> Signed-off-by: Alvin Zhang <alvinx.zhang@intel.com>
> ---
>  lib/ethdev/rte_ethdev.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
> index faf3bd9..4220ec5 100644
> --- a/lib/ethdev/rte_ethdev.h
> +++ b/lib/ethdev/rte_ethdev.h
> @@ -537,6 +537,7 @@ struct rte_eth_rss_conf {
>  #define ETH_RSS_PPPOE		   (1ULL << 31)
>  #define ETH_RSS_ECPRI		   (1ULL << 32)
>  #define ETH_RSS_MPLS		   (1ULL << 33)

Please, add a comment what does it mean in various cases
covering various L3 protocols. IMHO, we must be explicit
here since addition of a new L3 protocol will make it
ambiguous if some drivers/HW use its checksum, but some
do not.

> +#define ETH_RSS_L3_CHKSUM	   (1ULL << 34)
>  
>  /*
>   * We use the following macros to combine with above ETH_RSS_* for
> 


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

* Re: [dpdk-dev] [PATCH] ethdev: add RSS offload type for L3 checksum
  2021-06-03  7:14 ` Andrew Rybchenko
@ 2021-06-03  7:28   ` Zhang, AlvinX
  0 siblings, 0 replies; 39+ messages in thread
From: Zhang, AlvinX @ 2021-06-03  7:28 UTC (permalink / raw)
  To: Andrew Rybchenko, Yigit, Ferruh, Zhang, Qi Z; +Cc: dev

Hi Rybchenko,

Thanks for your review.
We will add more descriptions and update testpmd app in V2.

> -----Original Message-----
> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Sent: Thursday, June 3, 2021 3:15 PM
> To: Zhang, AlvinX <alvinx.zhang@intel.com>; Yigit, Ferruh
> <ferruh.yigit@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] ethdev: add RSS offload type for L3 checksum
> 
> On 6/3/21 6:12 AM, Alvin Zhang wrote:
> > Add ETH_RSS_L3_CHKSUM macro.
> 
> Sorry, too short description. I could be an existing practice to add more RSS
> offload types, but I want to break it.
> Please, add motivation in the description why it is useful etc.
> 
> >
> > Signed-off-by: Alvin Zhang <alvinx.zhang@intel.com>
> > ---
> >  lib/ethdev/rte_ethdev.h | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h index
> > faf3bd9..4220ec5 100644
> > --- a/lib/ethdev/rte_ethdev.h
> > +++ b/lib/ethdev/rte_ethdev.h
> > @@ -537,6 +537,7 @@ struct rte_eth_rss_conf {
> >  #define ETH_RSS_PPPOE		   (1ULL << 31)
> >  #define ETH_RSS_ECPRI		   (1ULL << 32)
> >  #define ETH_RSS_MPLS		   (1ULL << 33)
> 
> Please, add a comment what does it mean in various cases covering various L3
> protocols. IMHO, we must be explicit here since addition of a new L3 protocol
> will make it ambiguous if some drivers/HW use its checksum, but some do not.
> 
> > +#define ETH_RSS_L3_CHKSUM	   (1ULL << 34)
> >
> >  /*
> >   * We use the following macros to combine with above ETH_RSS_* for
> >

BRs,
Alvin Zhang

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

* [dpdk-dev] [PATCH v2] ethdev: add IPv4 checksum RSS offload type
  2021-06-03  3:12 [dpdk-dev] [PATCH] ethdev: add RSS offload type for L3 checksum Alvin Zhang
  2021-06-03  7:14 ` Andrew Rybchenko
@ 2021-06-03  8:03 ` Alvin Zhang
  2021-06-03  8:17   ` Andrew Rybchenko
                     ` (2 more replies)
  1 sibling, 3 replies; 39+ messages in thread
From: Alvin Zhang @ 2021-06-03  8:03 UTC (permalink / raw)
  To: qi.z.zhang, andrew.rybchenko; +Cc: dev, Alvin Zhang

This patch defines new RSS offload type for IPv4 checksum,
which is required when users want to distribute packets based
on the IPv4 checksum field.

For example "flow create 0 ingress pattern eth / ipv4 / end
actions rss types ipv4-chksum end queues end / end", this flow
causes all matching packets to be distributed to queues on
basis of IPv4 checksum.

Signed-off-by: Alvin Zhang <alvinx.zhang@intel.com>
---
 app/test-pmd/cmdline.c  | 2 ++
 app/test-pmd/config.c   | 1 +
 lib/ethdev/rte_ethdev.h | 1 +
 3 files changed, 4 insertions(+)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 0268b18..32da066 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -2254,6 +2254,8 @@ struct cmd_config_rss {
 		rss_conf.rss_hf = ETH_RSS_ECPRI;
 	else if (!strcmp(res->value, "mpls"))
 		rss_conf.rss_hf = ETH_RSS_MPLS;
+	else if (!strcmp(res->value, "ipv4-chksum"))
+		rss_conf.rss_hf = ETH_RSS_IPV4_CHKSUM;
 	else if (!strcmp(res->value, "none"))
 		rss_conf.rss_hf = 0;
 	else if (!strcmp(res->value, "level-default")) {
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 43c79b5..2c0c415 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -140,6 +140,7 @@
 	{ "gtpu", ETH_RSS_GTPU },
 	{ "ecpri", ETH_RSS_ECPRI },
 	{ "mpls", ETH_RSS_MPLS },
+	{ "ipv4-chksum", ETH_RSS_IPV4_CHKSUM },
 	{ NULL, 0 },
 };
 
diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index faf3bd9..f10d834 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -537,6 +537,7 @@ struct rte_eth_rss_conf {
 #define ETH_RSS_PPPOE		   (1ULL << 31)
 #define ETH_RSS_ECPRI		   (1ULL << 32)
 #define ETH_RSS_MPLS		   (1ULL << 33)
+#define ETH_RSS_IPV4_CHKSUM	   (1ULL << 34)
 
 /*
  * We use the following macros to combine with above ETH_RSS_* for
-- 
1.8.3.1


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

* Re: [dpdk-dev] [PATCH v2] ethdev: add IPv4 checksum RSS offload type
  2021-06-03  8:03 ` [dpdk-dev] [PATCH v2] ethdev: add IPv4 checksum RSS offload type Alvin Zhang
@ 2021-06-03  8:17   ` Andrew Rybchenko
  2021-06-04  2:23     ` Zhang, AlvinX
  2021-06-07 18:31   ` Ajit Khaparde
  2021-06-15  8:19   ` [dpdk-dev] [PATCH v3] ethdev: add IPv4 and L4 checksum RSS offload types Alvin Zhang
  2 siblings, 1 reply; 39+ messages in thread
From: Andrew Rybchenko @ 2021-06-03  8:17 UTC (permalink / raw)
  To: Alvin Zhang, qi.z.zhang; +Cc: dev

On 6/3/21 11:03 AM, Alvin Zhang wrote:
> This patch defines new RSS offload type for IPv4 checksum,
> which is required when users want to distribute packets based
> on the IPv4 checksum field.
> 
> For example "flow create 0 ingress pattern eth / ipv4 / end
> actions rss types ipv4-chksum end queues end / end", this flow
> causes all matching packets to be distributed to queues on
> basis of IPv4 checksum.
> 
> Signed-off-by: Alvin Zhang <alvinx.zhang@intel.com>

The patch LGTM now. I hope we'll have a PMD in the release
cycle which supports it.

In fact it is a funny idea. Let's consider a TCP connection
and routing change with one extra hop on the way while
the connection is alive. Or a UDP stream in the same
conditions. If so, TTL will different, checksum changes and
connection packets may be delivered to a different Rx queue.
May be sometimes it is not important and/or can't happen.

L4 checksum sounds better from this point of view since
it covers IP addresses and transport ports and less bits
to hash. The only concern is distribution fairness, but
I keep the topic to those who understand corresponding
math better.

> ---
>  app/test-pmd/cmdline.c  | 2 ++
>  app/test-pmd/config.c   | 1 +
>  lib/ethdev/rte_ethdev.h | 1 +
>  3 files changed, 4 insertions(+)
> 
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index 0268b18..32da066 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -2254,6 +2254,8 @@ struct cmd_config_rss {
>  		rss_conf.rss_hf = ETH_RSS_ECPRI;
>  	else if (!strcmp(res->value, "mpls"))
>  		rss_conf.rss_hf = ETH_RSS_MPLS;
> +	else if (!strcmp(res->value, "ipv4-chksum"))
> +		rss_conf.rss_hf = ETH_RSS_IPV4_CHKSUM;
>  	else if (!strcmp(res->value, "none"))
>  		rss_conf.rss_hf = 0;
>  	else if (!strcmp(res->value, "level-default")) {
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index 43c79b5..2c0c415 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -140,6 +140,7 @@
>  	{ "gtpu", ETH_RSS_GTPU },
>  	{ "ecpri", ETH_RSS_ECPRI },
>  	{ "mpls", ETH_RSS_MPLS },
> +	{ "ipv4-chksum", ETH_RSS_IPV4_CHKSUM },
>  	{ NULL, 0 },
>  };
>  
> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
> index faf3bd9..f10d834 100644
> --- a/lib/ethdev/rte_ethdev.h
> +++ b/lib/ethdev/rte_ethdev.h
> @@ -537,6 +537,7 @@ struct rte_eth_rss_conf {
>  #define ETH_RSS_PPPOE		   (1ULL << 31)
>  #define ETH_RSS_ECPRI		   (1ULL << 32)
>  #define ETH_RSS_MPLS		   (1ULL << 33)
> +#define ETH_RSS_IPV4_CHKSUM	   (1ULL << 34)
>  
>  /*
>   * We use the following macros to combine with above ETH_RSS_* for
> 


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

* Re: [dpdk-dev] [PATCH v2] ethdev: add IPv4 checksum RSS offload type
  2021-06-03  8:17   ` Andrew Rybchenko
@ 2021-06-04  2:23     ` Zhang, AlvinX
  0 siblings, 0 replies; 39+ messages in thread
From: Zhang, AlvinX @ 2021-06-04  2:23 UTC (permalink / raw)
  To: Andrew Rybchenko, Zhang, Qi Z; +Cc: dev

> > This patch defines new RSS offload type for IPv4 checksum, which is
> > required when users want to distribute packets based on the IPv4
> > checksum field.
> >
> > For example "flow create 0 ingress pattern eth / ipv4 / end actions
> > rss types ipv4-chksum end queues end / end", this flow causes all
> > matching packets to be distributed to queues on basis of IPv4
> > checksum.
> >
> > Signed-off-by: Alvin Zhang <alvinx.zhang@intel.com>
> 
> The patch LGTM now. I hope we'll have a PMD in the release cycle which
> supports it.
> 
> In fact it is a funny idea. Let's consider a TCP connection and routing change
> with one extra hop on the way while the connection is alive. Or a UDP stream in
> the same conditions. If so, TTL will different, checksum changes and connection
> packets may be delivered to a different Rx queue.
> May be sometimes it is not important and/or can't happen.
> 
> L4 checksum sounds better from this point of view since it covers IP addresses
> and transport ports and less bits to hash. The only concern is distribution fairness,
> but I keep the topic to those who understand corresponding math better.
> 
> > ---

We are preparing a PMD patch to support this feature, if ready, we will submit it ASAP.

Thanks,
Alvin

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

* Re: [dpdk-dev] [PATCH v2] ethdev: add IPv4 checksum RSS offload type
  2021-06-03  8:03 ` [dpdk-dev] [PATCH v2] ethdev: add IPv4 checksum RSS offload type Alvin Zhang
  2021-06-03  8:17   ` Andrew Rybchenko
@ 2021-06-07 18:31   ` Ajit Khaparde
  2021-06-15  8:19   ` [dpdk-dev] [PATCH v3] ethdev: add IPv4 and L4 checksum RSS offload types Alvin Zhang
  2 siblings, 0 replies; 39+ messages in thread
From: Ajit Khaparde @ 2021-06-07 18:31 UTC (permalink / raw)
  To: Alvin Zhang; +Cc: Qi Zhang, Andrew Rybchenko, dpdk-dev

[-- Attachment #1: Type: text/plain, Size: 2184 bytes --]

On Thu, Jun 3, 2021 at 1:04 AM Alvin Zhang <alvinx.zhang@intel.com> wrote:
>
> This patch defines new RSS offload type for IPv4 checksum,
> which is required when users want to distribute packets based
> on the IPv4 checksum field.
>
> For example "flow create 0 ingress pattern eth / ipv4 / end
> actions rss types ipv4-chksum end queues end / end", this flow
> causes all matching packets to be distributed to queues on
> basis of IPv4 checksum.
>
> Signed-off-by: Alvin Zhang <alvinx.zhang@intel.com>
Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>

> ---
>  app/test-pmd/cmdline.c  | 2 ++
>  app/test-pmd/config.c   | 1 +
>  lib/ethdev/rte_ethdev.h | 1 +
>  3 files changed, 4 insertions(+)
>
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index 0268b18..32da066 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -2254,6 +2254,8 @@ struct cmd_config_rss {
>                 rss_conf.rss_hf = ETH_RSS_ECPRI;
>         else if (!strcmp(res->value, "mpls"))
>                 rss_conf.rss_hf = ETH_RSS_MPLS;
> +       else if (!strcmp(res->value, "ipv4-chksum"))
> +               rss_conf.rss_hf = ETH_RSS_IPV4_CHKSUM;
>         else if (!strcmp(res->value, "none"))
>                 rss_conf.rss_hf = 0;
>         else if (!strcmp(res->value, "level-default")) {
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index 43c79b5..2c0c415 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -140,6 +140,7 @@
>         { "gtpu", ETH_RSS_GTPU },
>         { "ecpri", ETH_RSS_ECPRI },
>         { "mpls", ETH_RSS_MPLS },
> +       { "ipv4-chksum", ETH_RSS_IPV4_CHKSUM },
>         { NULL, 0 },
>  };
>
> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
> index faf3bd9..f10d834 100644
> --- a/lib/ethdev/rte_ethdev.h
> +++ b/lib/ethdev/rte_ethdev.h
> @@ -537,6 +537,7 @@ struct rte_eth_rss_conf {
>  #define ETH_RSS_PPPOE             (1ULL << 31)
>  #define ETH_RSS_ECPRI             (1ULL << 32)
>  #define ETH_RSS_MPLS              (1ULL << 33)
> +#define ETH_RSS_IPV4_CHKSUM       (1ULL << 34)
>
>  /*
>   * We use the following macros to combine with above ETH_RSS_* for
> --
> 1.8.3.1
>

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

* [dpdk-dev] [PATCH v3] ethdev: add IPv4 and L4 checksum RSS offload types
  2021-06-03  8:03 ` [dpdk-dev] [PATCH v2] ethdev: add IPv4 checksum RSS offload type Alvin Zhang
  2021-06-03  8:17   ` Andrew Rybchenko
  2021-06-07 18:31   ` Ajit Khaparde
@ 2021-06-15  8:19   ` Alvin Zhang
  2021-06-15  8:26     ` Jerin Jacob
                       ` (2 more replies)
  2 siblings, 3 replies; 39+ messages in thread
From: Alvin Zhang @ 2021-06-15  8:19 UTC (permalink / raw)
  To: qi.z.zhang, andrew.rybchenko, ajit.khaparde; +Cc: dev, Alvin Zhang

This patch defines new RSS offload types for IPv4 and L4 checksum,
which are required when users want to distribute packets based on the
IPv4 or L4 checksum field.

For example "flow create 0 ingress pattern eth / ipv4 / end
actions rss types ipv4-chksum end queues end / end", this flow
causes all matching packets to be distributed to queues on
basis of IPv4 checksum.

Signed-off-by: Alvin Zhang <alvinx.zhang@intel.com>
Reviewed-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
---

v3: Add L4 checksum RSS offload type
---
 app/test-pmd/cmdline.c  | 4 ++++
 app/test-pmd/config.c   | 2 ++
 lib/ethdev/rte_ethdev.h | 2 ++
 3 files changed, 8 insertions(+)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 0268b18..6148d84 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -2254,6 +2254,10 @@ struct cmd_config_rss {
 		rss_conf.rss_hf = ETH_RSS_ECPRI;
 	else if (!strcmp(res->value, "mpls"))
 		rss_conf.rss_hf = ETH_RSS_MPLS;
+	else if (!strcmp(res->value, "ipv4-chksum"))
+		rss_conf.rss_hf = ETH_RSS_IPV4_CHKSUM;
+	else if (!strcmp(res->value, "l4-chksum"))
+		rss_conf.rss_hf = ETH_RSS_L4_CHKSUM;
 	else if (!strcmp(res->value, "none"))
 		rss_conf.rss_hf = 0;
 	else if (!strcmp(res->value, "level-default")) {
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 43c79b5..14968bf 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -140,6 +140,8 @@
 	{ "gtpu", ETH_RSS_GTPU },
 	{ "ecpri", ETH_RSS_ECPRI },
 	{ "mpls", ETH_RSS_MPLS },
+	{ "ipv4-chksum", ETH_RSS_IPV4_CHKSUM },
+	{ "l4-chksum", ETH_RSS_L4_CHKSUM },
 	{ NULL, 0 },
 };
 
diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index faf3bd9..1268729 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -537,6 +537,8 @@ struct rte_eth_rss_conf {
 #define ETH_RSS_PPPOE		   (1ULL << 31)
 #define ETH_RSS_ECPRI		   (1ULL << 32)
 #define ETH_RSS_MPLS		   (1ULL << 33)
+#define ETH_RSS_IPV4_CHKSUM	   (1ULL << 34)
+#define ETH_RSS_L4_CHKSUM	   (1ULL << 35)
 
 /*
  * We use the following macros to combine with above ETH_RSS_* for
-- 
1.8.3.1


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

* Re: [dpdk-dev] [PATCH v3] ethdev: add IPv4 and L4 checksum RSS offload types
  2021-06-15  8:19   ` [dpdk-dev] [PATCH v3] ethdev: add IPv4 and L4 checksum RSS offload types Alvin Zhang
@ 2021-06-15  8:26     ` Jerin Jacob
  2021-06-16 15:18       ` Zhang, Qi Z
  2021-07-01 14:26     ` Andrew Rybchenko
  2021-07-13  1:13     ` [dpdk-dev] [PATCH v4] " Alvin Zhang
  2 siblings, 1 reply; 39+ messages in thread
From: Jerin Jacob @ 2021-06-15  8:26 UTC (permalink / raw)
  To: Alvin Zhang; +Cc: Qi Zhang, Andrew Rybchenko, Ajit Khaparde, dpdk-dev

On Tue, Jun 15, 2021 at 1:50 PM Alvin Zhang <alvinx.zhang@intel.com> wrote:
>
> This patch defines new RSS offload types for IPv4 and L4 checksum,
> which are required when users want to distribute packets based on the
> IPv4 or L4 checksum field.

What is the usecase for distribution based on L4/IPv4 checksum?
Is it something like HW has the feature so expose it or there is some
real use case for this application?



> For example "flow create 0 ingress pattern eth / ipv4 / end
> actions rss types ipv4-chksum end queues end / end", this flow
> causes all matching packets to be distributed to queues on
> basis of IPv4 checksum.
>
> Signed-off-by: Alvin Zhang <alvinx.zhang@intel.com>
> Reviewed-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> ---
>
> v3: Add L4 checksum RSS offload type
> ---
>  app/test-pmd/cmdline.c  | 4 ++++
>  app/test-pmd/config.c   | 2 ++
>  lib/ethdev/rte_ethdev.h | 2 ++
>  3 files changed, 8 insertions(+)
>
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index 0268b18..6148d84 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -2254,6 +2254,10 @@ struct cmd_config_rss {
>                 rss_conf.rss_hf = ETH_RSS_ECPRI;
>         else if (!strcmp(res->value, "mpls"))
>                 rss_conf.rss_hf = ETH_RSS_MPLS;
> +       else if (!strcmp(res->value, "ipv4-chksum"))
> +               rss_conf.rss_hf = ETH_RSS_IPV4_CHKSUM;
> +       else if (!strcmp(res->value, "l4-chksum"))
> +               rss_conf.rss_hf = ETH_RSS_L4_CHKSUM;
>         else if (!strcmp(res->value, "none"))
>                 rss_conf.rss_hf = 0;
>         else if (!strcmp(res->value, "level-default")) {
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index 43c79b5..14968bf 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -140,6 +140,8 @@
>         { "gtpu", ETH_RSS_GTPU },
>         { "ecpri", ETH_RSS_ECPRI },
>         { "mpls", ETH_RSS_MPLS },
> +       { "ipv4-chksum", ETH_RSS_IPV4_CHKSUM },
> +       { "l4-chksum", ETH_RSS_L4_CHKSUM },
>         { NULL, 0 },
>  };
>
> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
> index faf3bd9..1268729 100644
> --- a/lib/ethdev/rte_ethdev.h
> +++ b/lib/ethdev/rte_ethdev.h
> @@ -537,6 +537,8 @@ struct rte_eth_rss_conf {
>  #define ETH_RSS_PPPOE             (1ULL << 31)
>  #define ETH_RSS_ECPRI             (1ULL << 32)
>  #define ETH_RSS_MPLS              (1ULL << 33)
> +#define ETH_RSS_IPV4_CHKSUM       (1ULL << 34)
> +#define ETH_RSS_L4_CHKSUM         (1ULL << 35)
>
>  /*
>   * We use the following macros to combine with above ETH_RSS_* for
> --
> 1.8.3.1
>

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

* Re: [dpdk-dev] [PATCH v3] ethdev: add IPv4 and L4 checksum RSS offload types
  2021-06-15  8:26     ` Jerin Jacob
@ 2021-06-16 15:18       ` Zhang, Qi Z
  2021-06-22  8:20         ` Singh, Aman Deep
  0 siblings, 1 reply; 39+ messages in thread
From: Zhang, Qi Z @ 2021-06-16 15:18 UTC (permalink / raw)
  To: Jerin Jacob, Zhang, AlvinX; +Cc: Andrew Rybchenko, Ajit Khaparde, dpdk-dev



> -----Original Message-----
> From: Jerin Jacob <jerinjacobk@gmail.com>
> Sent: Tuesday, June 15, 2021 4:26 PM
> To: Zhang, AlvinX <alvinx.zhang@intel.com>
> Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; Andrew Rybchenko
> <andrew.rybchenko@oktetlabs.ru>; Ajit Khaparde
> <ajit.khaparde@broadcom.com>; dpdk-dev <dev@dpdk.org>
> Subject: Re: [dpdk-dev] [PATCH v3] ethdev: add IPv4 and L4 checksum RSS
> offload types
> 
> On Tue, Jun 15, 2021 at 1:50 PM Alvin Zhang <alvinx.zhang@intel.com>
> wrote:
> >
> > This patch defines new RSS offload types for IPv4 and L4 checksum,
> > which are required when users want to distribute packets based on the
> > IPv4 or L4 checksum field.
> 
> What is the usecase for distribution based on L4/IPv4 checksum?
> Is it something like HW has the feature so expose it or there is some real use
> case for this application?

This is for real use case, some research by using TCP checksum for FDIR on ixgbe.
https://hsadok.com/papers/sprayer-hotnets18.pdf

and we are looking for similar solution in ice, and checksum RSS is the feature we need to have.

> 
> 
> 
> > For example "flow create 0 ingress pattern eth / ipv4 / end actions
> > rss types ipv4-chksum end queues end / end", this flow causes all
> > matching packets to be distributed to queues on basis of IPv4
> > checksum.
> >
> > Signed-off-by: Alvin Zhang <alvinx.zhang@intel.com>
> > Reviewed-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> > Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> > ---
> >
> > v3: Add L4 checksum RSS offload type
> > ---
> >  app/test-pmd/cmdline.c  | 4 ++++
> >  app/test-pmd/config.c   | 2 ++
> >  lib/ethdev/rte_ethdev.h | 2 ++
> >  3 files changed, 8 insertions(+)
> >
> > diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index
> > 0268b18..6148d84 100644
> > --- a/app/test-pmd/cmdline.c
> > +++ b/app/test-pmd/cmdline.c
> > @@ -2254,6 +2254,10 @@ struct cmd_config_rss {
> >                 rss_conf.rss_hf = ETH_RSS_ECPRI;
> >         else if (!strcmp(res->value, "mpls"))
> >                 rss_conf.rss_hf = ETH_RSS_MPLS;
> > +       else if (!strcmp(res->value, "ipv4-chksum"))
> > +               rss_conf.rss_hf = ETH_RSS_IPV4_CHKSUM;
> > +       else if (!strcmp(res->value, "l4-chksum"))
> > +               rss_conf.rss_hf = ETH_RSS_L4_CHKSUM;
> >         else if (!strcmp(res->value, "none"))
> >                 rss_conf.rss_hf = 0;
> >         else if (!strcmp(res->value, "level-default")) { diff --git
> > a/app/test-pmd/config.c b/app/test-pmd/config.c index 43c79b5..14968bf
> > 100644
> > --- a/app/test-pmd/config.c
> > +++ b/app/test-pmd/config.c
> > @@ -140,6 +140,8 @@
> >         { "gtpu", ETH_RSS_GTPU },
> >         { "ecpri", ETH_RSS_ECPRI },
> >         { "mpls", ETH_RSS_MPLS },
> > +       { "ipv4-chksum", ETH_RSS_IPV4_CHKSUM },
> > +       { "l4-chksum", ETH_RSS_L4_CHKSUM },
> >         { NULL, 0 },
> >  };
> >
> > diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h index
> > faf3bd9..1268729 100644
> > --- a/lib/ethdev/rte_ethdev.h
> > +++ b/lib/ethdev/rte_ethdev.h
> > @@ -537,6 +537,8 @@ struct rte_eth_rss_conf {
> >  #define ETH_RSS_PPPOE             (1ULL << 31)
> >  #define ETH_RSS_ECPRI             (1ULL << 32)
> >  #define ETH_RSS_MPLS              (1ULL << 33)
> > +#define ETH_RSS_IPV4_CHKSUM       (1ULL << 34)
> > +#define ETH_RSS_L4_CHKSUM         (1ULL << 35)
> >
> >  /*
> >   * We use the following macros to combine with above ETH_RSS_* for
> > --
> > 1.8.3.1
> >

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

* Re: [dpdk-dev] [PATCH v3] ethdev: add IPv4 and L4 checksum RSS offload types
  2021-06-16 15:18       ` Zhang, Qi Z
@ 2021-06-22  8:20         ` Singh, Aman Deep
  0 siblings, 0 replies; 39+ messages in thread
From: Singh, Aman Deep @ 2021-06-22  8:20 UTC (permalink / raw)
  To: dev

Acked-by: Aman Deep Singh <aman.deep.singh@intel.com>


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

* Re: [dpdk-dev] [PATCH v3] ethdev: add IPv4 and L4 checksum RSS offload types
  2021-06-15  8:19   ` [dpdk-dev] [PATCH v3] ethdev: add IPv4 and L4 checksum RSS offload types Alvin Zhang
  2021-06-15  8:26     ` Jerin Jacob
@ 2021-07-01 14:26     ` Andrew Rybchenko
  2021-07-06  6:14       ` Zhang, AlvinX
  2021-07-06  7:05       ` Zhang, AlvinX
  2021-07-13  1:13     ` [dpdk-dev] [PATCH v4] " Alvin Zhang
  2 siblings, 2 replies; 39+ messages in thread
From: Andrew Rybchenko @ 2021-07-01 14:26 UTC (permalink / raw)
  To: Alvin Zhang, qi.z.zhang, ajit.khaparde; +Cc: dev

On 6/15/21 11:19 AM, Alvin Zhang wrote:
> This patch defines new RSS offload types for IPv4 and L4 checksum,
> which are required when users want to distribute packets based on the
> IPv4 or L4 checksum field.
> 
> For example "flow create 0 ingress pattern eth / ipv4 / end
> actions rss types ipv4-chksum end queues end / end", this flow
> causes all matching packets to be distributed to queues on
> basis of IPv4 checksum.
> 
> Signed-off-by: Alvin Zhang <alvinx.zhang@intel.com>
> Reviewed-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> ---
> 
> v3: Add L4 checksum RSS offload type
> ---
>  app/test-pmd/cmdline.c  | 4 ++++
>  app/test-pmd/config.c   | 2 ++
>  lib/ethdev/rte_ethdev.h | 2 ++
>  3 files changed, 8 insertions(+)
> 
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index 0268b18..6148d84 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -2254,6 +2254,10 @@ struct cmd_config_rss {
>  		rss_conf.rss_hf = ETH_RSS_ECPRI;
>  	else if (!strcmp(res->value, "mpls"))
>  		rss_conf.rss_hf = ETH_RSS_MPLS;
> +	else if (!strcmp(res->value, "ipv4-chksum"))
> +		rss_conf.rss_hf = ETH_RSS_IPV4_CHKSUM;
> +	else if (!strcmp(res->value, "l4-chksum"))
> +		rss_conf.rss_hf = ETH_RSS_L4_CHKSUM;
>  	else if (!strcmp(res->value, "none"))
>  		rss_conf.rss_hf = 0;
>  	else if (!strcmp(res->value, "level-default")) {
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index 43c79b5..14968bf 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -140,6 +140,8 @@
>  	{ "gtpu", ETH_RSS_GTPU },
>  	{ "ecpri", ETH_RSS_ECPRI },
>  	{ "mpls", ETH_RSS_MPLS },
> +	{ "ipv4-chksum", ETH_RSS_IPV4_CHKSUM },
> +	{ "l4-chksum", ETH_RSS_L4_CHKSUM },
>  	{ NULL, 0 },
>  };
>  
> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
> index faf3bd9..1268729 100644
> --- a/lib/ethdev/rte_ethdev.h
> +++ b/lib/ethdev/rte_ethdev.h
> @@ -537,6 +537,8 @@ struct rte_eth_rss_conf {
>  #define ETH_RSS_PPPOE		   (1ULL << 31)
>  #define ETH_RSS_ECPRI		   (1ULL << 32)
>  #define ETH_RSS_MPLS		   (1ULL << 33)
> +#define ETH_RSS_IPV4_CHKSUM	   (1ULL << 34)
> +#define ETH_RSS_L4_CHKSUM	   (1ULL << 35)

What does efine which L4 protocols are supported? How user will
know?

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

* Re: [dpdk-dev] [PATCH v3] ethdev: add IPv4 and L4 checksum RSS offload types
  2021-07-01 14:26     ` Andrew Rybchenko
@ 2021-07-06  6:14       ` Zhang, AlvinX
  2021-07-06  7:05       ` Zhang, AlvinX
  1 sibling, 0 replies; 39+ messages in thread
From: Zhang, AlvinX @ 2021-07-06  6:14 UTC (permalink / raw)
  To: Andrew Rybchenko, Zhang, Qi Z, ajit.khaparde; +Cc: dev

> > diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h index
> > faf3bd9..1268729 100644
> > --- a/lib/ethdev/rte_ethdev.h
> > +++ b/lib/ethdev/rte_ethdev.h
> > @@ -537,6 +537,8 @@ struct rte_eth_rss_conf {
> >  #define ETH_RSS_PPPOE		   (1ULL << 31)
> >  #define ETH_RSS_ECPRI		   (1ULL << 32)
> >  #define ETH_RSS_MPLS		   (1ULL << 33)
> > +#define ETH_RSS_IPV4_CHKSUM	   (1ULL << 34)
> > +#define ETH_RSS_L4_CHKSUM	   (1ULL << 35)
> 
> What does efine which L4 protocols are supported? How user will know?

I think TCP/UDP/SCTP can be supported, but this is determined by PMD, here we only provide a general interface.

BRs,
Alvin

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

* Re: [dpdk-dev] [PATCH v3] ethdev: add IPv4 and L4 checksum RSS offload types
  2021-07-01 14:26     ` Andrew Rybchenko
  2021-07-06  6:14       ` Zhang, AlvinX
@ 2021-07-06  7:05       ` Zhang, AlvinX
  2021-07-06  7:18         ` Zhang, Qi Z
  1 sibling, 1 reply; 39+ messages in thread
From: Zhang, AlvinX @ 2021-07-06  7:05 UTC (permalink / raw)
  To: Andrew Rybchenko, Zhang, Qi Z, ajit.khaparde; +Cc: dev

> > @@ -537,6 +537,8 @@ struct rte_eth_rss_conf {
> >  #define ETH_RSS_PPPOE		   (1ULL << 31)
> >  #define ETH_RSS_ECPRI		   (1ULL << 32)
> >  #define ETH_RSS_MPLS		   (1ULL << 33)
> > +#define ETH_RSS_IPV4_CHKSUM	   (1ULL << 34)
> > +#define ETH_RSS_L4_CHKSUM	   (1ULL << 35)
> 
> What does efine which L4 protocols are supported? How user will know?

I think if we want to support L4 checksum RSS by using below command
port config all rss (all|default|eth|vlan|...)

We must define TCP/UDP/SCTP checksum RSS separately: 
#define ETH_RSS_TCP_CHKSUM	(1ULL << 35)
#define ETH_RSS_UDP_CHKSUM	(1ULL << 36)
#deifne ETH_RSS_SCTP_CHKSUM	(1ULL << 37)

Here 3 bits are occupied, this is not good for there are not many bits available.

If we only want to using it in flows, we only need to define ETH_RSS_L4_CHKSUM, 
because the flow pattern pointed out the L4 protocol type.
flow create 0 ingress pattern eth / ipv4 / tcp / end actions rss types l4-chksum end queues end / end

So what's your opinions?

Thanks
Alvin

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

* Re: [dpdk-dev] [PATCH v3] ethdev: add IPv4 and L4 checksum RSS offload types
  2021-07-06  7:05       ` Zhang, AlvinX
@ 2021-07-06  7:18         ` Zhang, Qi Z
  2021-07-06  8:04           ` Andrew Rybchenko
  0 siblings, 1 reply; 39+ messages in thread
From: Zhang, Qi Z @ 2021-07-06  7:18 UTC (permalink / raw)
  To: Zhang, AlvinX, Andrew Rybchenko, ajit.khaparde; +Cc: dev



> -----Original Message-----
> From: Zhang, AlvinX <alvinx.zhang@intel.com>
> Sent: Tuesday, July 6, 2021 3:06 PM
> To: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>; Zhang, Qi Z
> <qi.z.zhang@intel.com>; ajit.khaparde@broadcom.com
> Cc: dev@dpdk.org
> Subject: RE: [PATCH v3] ethdev: add IPv4 and L4 checksum RSS offload types
> 
> > > @@ -537,6 +537,8 @@ struct rte_eth_rss_conf {
> > >  #define ETH_RSS_PPPOE		   (1ULL << 31)
> > >  #define ETH_RSS_ECPRI		   (1ULL << 32)
> > >  #define ETH_RSS_MPLS		   (1ULL << 33)
> > > +#define ETH_RSS_IPV4_CHKSUM	   (1ULL << 34)
> > > +#define ETH_RSS_L4_CHKSUM	   (1ULL << 35)
> >
> > What does efine which L4 protocols are supported? How user will know?
> 
> I think if we want to support L4 checksum RSS by using below command port
> config all rss (all|default|eth|vlan|...)
> 
> We must define TCP/UDP/SCTP checksum RSS separately:
> #define ETH_RSS_TCP_CHKSUM	(1ULL << 35)
> #define ETH_RSS_UDP_CHKSUM	(1ULL << 36)
> #deifne ETH_RSS_SCTP_CHKSUM	(1ULL << 37)
> 
> Here 3 bits are occupied, this is not good for there are not many bits available.
> 
> If we only want to using it in flows, we only need to define
> ETH_RSS_L4_CHKSUM, because the flow pattern pointed out the L4 protocol
> type.
> flow create 0 ingress pattern eth / ipv4 / tcp / end actions rss types l4-chksum
> end queues end / end

+1, the pattern already give the hint to avoid the ambiguity and I think we already have ETH_RSS_LEVEL to figure out inner or outer.
 
> 
> So what's your opinions?
> 
> Thanks
> Alvin


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

* Re: [dpdk-dev] [PATCH v3] ethdev: add IPv4 and L4 checksum RSS offload types
  2021-07-06  7:18         ` Zhang, Qi Z
@ 2021-07-06  8:04           ` Andrew Rybchenko
  2021-07-07  3:23             ` Zhang, Qi Z
  0 siblings, 1 reply; 39+ messages in thread
From: Andrew Rybchenko @ 2021-07-06  8:04 UTC (permalink / raw)
  To: Zhang, Qi Z, Zhang, AlvinX, ajit.khaparde; +Cc: dev

On 7/6/21 10:18 AM, Zhang, Qi Z wrote:
> 
> 
>> -----Original Message-----
>> From: Zhang, AlvinX <alvinx.zhang@intel.com>
>> Sent: Tuesday, July 6, 2021 3:06 PM
>> To: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>; Zhang, Qi Z
>> <qi.z.zhang@intel.com>; ajit.khaparde@broadcom.com
>> Cc: dev@dpdk.org
>> Subject: RE: [PATCH v3] ethdev: add IPv4 and L4 checksum RSS offload types
>>
>>>> @@ -537,6 +537,8 @@ struct rte_eth_rss_conf {
>>>>  #define ETH_RSS_PPPOE		   (1ULL << 31)
>>>>  #define ETH_RSS_ECPRI		   (1ULL << 32)
>>>>  #define ETH_RSS_MPLS		   (1ULL << 33)
>>>> +#define ETH_RSS_IPV4_CHKSUM	   (1ULL << 34)
>>>> +#define ETH_RSS_L4_CHKSUM	   (1ULL << 35)
>>>
>>> What does efine which L4 protocols are supported? How user will know?
>>
>> I think if we want to support L4 checksum RSS by using below command port
>> config all rss (all|default|eth|vlan|...)
>>
>> We must define TCP/UDP/SCTP checksum RSS separately:
>> #define ETH_RSS_TCP_CHKSUM	(1ULL << 35)
>> #define ETH_RSS_UDP_CHKSUM	(1ULL << 36)
>> #deifne ETH_RSS_SCTP_CHKSUM	(1ULL << 37)
>>
>> Here 3 bits are occupied, this is not good for there are not many bits available.
>>
>> If we only want to using it in flows, we only need to define
>> ETH_RSS_L4_CHKSUM, because the flow pattern pointed out the L4 protocol
>> type.
>> flow create 0 ingress pattern eth / ipv4 / tcp / end actions rss types l4-chksum
>> end queues end / end
> 
> +1, the pattern already give the hint to avoid the ambiguity and I think we already have ETH_RSS_LEVEL to figure out inner or outer.

The problem that it may be used in generic RSS flags which
has no the context. Also even in the case of flow API
context could have no L4 protocol at all.

Is UDP checksum 0 treated as no checksum and go to default
queue or treated as a regular checksum with value equal to 0?

I tend to agree that 3 flags is too much for the feature,
but one flag without properly defined meaning is not good
as well.

I just want rules to be defined and documented.

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

* Re: [dpdk-dev] [PATCH v3] ethdev: add IPv4 and L4 checksum RSS offload types
  2021-07-06  8:04           ` Andrew Rybchenko
@ 2021-07-07  3:23             ` Zhang, Qi Z
  2021-07-07  9:49               ` Andrew Rybchenko
  0 siblings, 1 reply; 39+ messages in thread
From: Zhang, Qi Z @ 2021-07-07  3:23 UTC (permalink / raw)
  To: Andrew Rybchenko, Zhang, AlvinX, ajit.khaparde; +Cc: dev



> -----Original Message-----
> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Sent: Tuesday, July 6, 2021 4:05 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Zhang, AlvinX
> <alvinx.zhang@intel.com>; ajit.khaparde@broadcom.com
> Cc: dev@dpdk.org
> Subject: Re: [PATCH v3] ethdev: add IPv4 and L4 checksum RSS offload types
> 
> On 7/6/21 10:18 AM, Zhang, Qi Z wrote:
> >
> >
> >> -----Original Message-----
> >> From: Zhang, AlvinX <alvinx.zhang@intel.com>
> >> Sent: Tuesday, July 6, 2021 3:06 PM
> >> To: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>; Zhang, Qi Z
> >> <qi.z.zhang@intel.com>; ajit.khaparde@broadcom.com
> >> Cc: dev@dpdk.org
> >> Subject: RE: [PATCH v3] ethdev: add IPv4 and L4 checksum RSS offload
> >> types
> >>
> >>>> @@ -537,6 +537,8 @@ struct rte_eth_rss_conf {
> >>>>  #define ETH_RSS_PPPOE		   (1ULL << 31)
> >>>>  #define ETH_RSS_ECPRI		   (1ULL << 32)
> >>>>  #define ETH_RSS_MPLS		   (1ULL << 33)
> >>>> +#define ETH_RSS_IPV4_CHKSUM	   (1ULL << 34)
> >>>> +#define ETH_RSS_L4_CHKSUM	   (1ULL << 35)
> >>>
> >>> What does efine which L4 protocols are supported? How user will know?
> >>
> >> I think if we want to support L4 checksum RSS by using below command
> >> port config all rss (all|default|eth|vlan|...)
> >>
> >> We must define TCP/UDP/SCTP checksum RSS separately:
> >> #define ETH_RSS_TCP_CHKSUM	(1ULL << 35)
> >> #define ETH_RSS_UDP_CHKSUM	(1ULL << 36)
> >> #deifne ETH_RSS_SCTP_CHKSUM	(1ULL << 37)
> >>
> >> Here 3 bits are occupied, this is not good for there are not many bits
> available.
> >>
> >> If we only want to using it in flows, we only need to define
> >> ETH_RSS_L4_CHKSUM, because the flow pattern pointed out the L4
> >> protocol type.
> >> flow create 0 ingress pattern eth / ipv4 / tcp / end actions rss
> >> types l4-chksum end queues end / end
> >
> > +1, the pattern already give the hint to avoid the ambiguity and I think we
> already have ETH_RSS_LEVEL to figure out inner or outer.
> 
> The problem that it may be used in generic RSS flags which has no the context.
> Also even in the case of flow API context could have no L4 protocol at all.

For generic case, it can simply assume it cover all L4 checksum cases and I'm not sure if any user intend to use it as generic RSS, pmd can simply reject it if it's not necessary to support.
In flow API, if no l4 protocol in pattern , the PMD should return failure (or maybe some default behavior), and I think this is not a new question as it happens all the cases
e.g.:
pattern eth / vlan / end action rss type ipv4 .

> 
> Is UDP checksum 0 treated as no checksum and go to default queue or treated
> as a regular checksum with value equal to 0?

I think we can treat it as value 0, as least our hardware behavior like this, is this any issue?

> 
> I tend to agree that 3 flags is too much for the feature, but one flag without
> properly defined meaning is not good as well.
> 
> I just want rules to be defined and documented.'

Agree, we need more document for this. if you agree above proposal.

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

* Re: [dpdk-dev] [PATCH v3] ethdev: add IPv4 and L4 checksum RSS offload types
  2021-07-07  3:23             ` Zhang, Qi Z
@ 2021-07-07  9:49               ` Andrew Rybchenko
  2021-07-07 13:00                 ` Zhang, Qi Z
  0 siblings, 1 reply; 39+ messages in thread
From: Andrew Rybchenko @ 2021-07-07  9:49 UTC (permalink / raw)
  To: Zhang, Qi Z, Zhang, AlvinX, ajit.khaparde; +Cc: dev

On 7/7/21 6:23 AM, Zhang, Qi Z wrote:
> 
> 
>> -----Original Message-----
>> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>> Sent: Tuesday, July 6, 2021 4:05 PM
>> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Zhang, AlvinX
>> <alvinx.zhang@intel.com>; ajit.khaparde@broadcom.com
>> Cc: dev@dpdk.org
>> Subject: Re: [PATCH v3] ethdev: add IPv4 and L4 checksum RSS offload types
>>
>> On 7/6/21 10:18 AM, Zhang, Qi Z wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Zhang, AlvinX <alvinx.zhang@intel.com>
>>>> Sent: Tuesday, July 6, 2021 3:06 PM
>>>> To: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>; Zhang, Qi Z
>>>> <qi.z.zhang@intel.com>; ajit.khaparde@broadcom.com
>>>> Cc: dev@dpdk.org
>>>> Subject: RE: [PATCH v3] ethdev: add IPv4 and L4 checksum RSS offload
>>>> types
>>>>
>>>>>> @@ -537,6 +537,8 @@ struct rte_eth_rss_conf {
>>>>>>  #define ETH_RSS_PPPOE		   (1ULL << 31)
>>>>>>  #define ETH_RSS_ECPRI		   (1ULL << 32)
>>>>>>  #define ETH_RSS_MPLS		   (1ULL << 33)
>>>>>> +#define ETH_RSS_IPV4_CHKSUM	   (1ULL << 34)
>>>>>> +#define ETH_RSS_L4_CHKSUM	   (1ULL << 35)
>>>>>
>>>>> What does efine which L4 protocols are supported? How user will know?
>>>>
>>>> I think if we want to support L4 checksum RSS by using below command
>>>> port config all rss (all|default|eth|vlan|...)
>>>>
>>>> We must define TCP/UDP/SCTP checksum RSS separately:
>>>> #define ETH_RSS_TCP_CHKSUM	(1ULL << 35)
>>>> #define ETH_RSS_UDP_CHKSUM	(1ULL << 36)
>>>> #deifne ETH_RSS_SCTP_CHKSUM	(1ULL << 37)
>>>>
>>>> Here 3 bits are occupied, this is not good for there are not many bits
>> available.
>>>>
>>>> If we only want to using it in flows, we only need to define
>>>> ETH_RSS_L4_CHKSUM, because the flow pattern pointed out the L4
>>>> protocol type.
>>>> flow create 0 ingress pattern eth / ipv4 / tcp / end actions rss
>>>> types l4-chksum end queues end / end
>>>
>>> +1, the pattern already give the hint to avoid the ambiguity and I think we
>> already have ETH_RSS_LEVEL to figure out inner or outer.
>>
>> The problem that it may be used in generic RSS flags which has no the context.
>> Also even in the case of flow API context could have no L4 protocol at all.
> 
> For generic case, it can simply assume it cover all L4 checksum cases and I'm not sure if any user intend to use it as generic RSS, pmd can simply reject it if it's not necessary to support.

Try to look at it from an application point of view which does
not know any specifics of the driver.

 * Get dev_info and see ETH_RSS_L4_CHKSUM, good!, would like to
   use it.

 * If I try to use it in default RSS config, but the request
   fail, it could be very confusing.

 * Will it distribute TCP packets? UDP packets? SCTP packets?
   Or should I care about RSS for some of them based on other
   supported fields? E.g. if SCTP is not supported by the NIC,
   I need to install RSS flow rule for the IP protocol to do
   RSS based on IPv4/IPv6 addresses. But if SCTP is supported,
   I'm happy to use ETH_RSS_L4_CHKSUM for it as well.

> In flow API, if no l4 protocol in pattern , the PMD should return failure (or maybe some default behavior), and I think this is not a new question as it happens all the cases
> e.g.:
> pattern eth / vlan / end action rss type ipv4 .

IMHO, it would be pretty logical to apply RSS to IPv4 packets
only and send everything else to default queue.

>>
>> Is UDP checksum 0 treated as no checksum and go to default queue or treated
>> as a regular checksum with value equal to 0?
> 
> I think we can treat it as value 0, as least our hardware behavior like this, is this any issue?

OK, no problem. Just document it.

>>
>> I tend to agree that 3 flags is too much for the feature, but one flag without
>> properly defined meaning is not good as well.
>>
>> I just want rules to be defined and documented.'
> 
> Agree, we need more document for this. if you agree above proposal.
> 


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

* Re: [dpdk-dev] [PATCH v3] ethdev: add IPv4 and L4 checksum RSS offload types
  2021-07-07  9:49               ` Andrew Rybchenko
@ 2021-07-07 13:00                 ` Zhang, Qi Z
  2021-07-07 13:10                   ` Andrew Rybchenko
  0 siblings, 1 reply; 39+ messages in thread
From: Zhang, Qi Z @ 2021-07-07 13:00 UTC (permalink / raw)
  To: Andrew Rybchenko, Zhang, AlvinX, ajit.khaparde; +Cc: dev



> -----Original Message-----
> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Sent: Wednesday, July 7, 2021 5:49 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Zhang, AlvinX
> <alvinx.zhang@intel.com>; ajit.khaparde@broadcom.com
> Cc: dev@dpdk.org
> Subject: Re: [PATCH v3] ethdev: add IPv4 and L4 checksum RSS offload types
> 
> On 7/7/21 6:23 AM, Zhang, Qi Z wrote:
> >
> >
> >> -----Original Message-----
> >> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> >> Sent: Tuesday, July 6, 2021 4:05 PM
> >> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Zhang, AlvinX
> >> <alvinx.zhang@intel.com>; ajit.khaparde@broadcom.com
> >> Cc: dev@dpdk.org
> >> Subject: Re: [PATCH v3] ethdev: add IPv4 and L4 checksum RSS offload
> >> types
> >>
> >> On 7/6/21 10:18 AM, Zhang, Qi Z wrote:
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: Zhang, AlvinX <alvinx.zhang@intel.com>
> >>>> Sent: Tuesday, July 6, 2021 3:06 PM
> >>>> To: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>; Zhang, Qi Z
> >>>> <qi.z.zhang@intel.com>; ajit.khaparde@broadcom.com
> >>>> Cc: dev@dpdk.org
> >>>> Subject: RE: [PATCH v3] ethdev: add IPv4 and L4 checksum RSS
> >>>> offload types
> >>>>
> >>>>>> @@ -537,6 +537,8 @@ struct rte_eth_rss_conf {
> >>>>>>  #define ETH_RSS_PPPOE		   (1ULL << 31)
> >>>>>>  #define ETH_RSS_ECPRI		   (1ULL << 32)
> >>>>>>  #define ETH_RSS_MPLS		   (1ULL << 33)
> >>>>>> +#define ETH_RSS_IPV4_CHKSUM	   (1ULL << 34)
> >>>>>> +#define ETH_RSS_L4_CHKSUM	   (1ULL << 35)
> >>>>>
> >>>>> What does efine which L4 protocols are supported? How user will know?
> >>>>
> >>>> I think if we want to support L4 checksum RSS by using below
> >>>> command port config all rss (all|default|eth|vlan|...)
> >>>>
> >>>> We must define TCP/UDP/SCTP checksum RSS separately:
> >>>> #define ETH_RSS_TCP_CHKSUM	(1ULL << 35)
> >>>> #define ETH_RSS_UDP_CHKSUM	(1ULL << 36)
> >>>> #deifne ETH_RSS_SCTP_CHKSUM	(1ULL << 37)
> >>>>
> >>>> Here 3 bits are occupied, this is not good for there are not many
> >>>> bits
> >> available.
> >>>>
> >>>> If we only want to using it in flows, we only need to define
> >>>> ETH_RSS_L4_CHKSUM, because the flow pattern pointed out the L4
> >>>> protocol type.
> >>>> flow create 0 ingress pattern eth / ipv4 / tcp / end actions rss
> >>>> types l4-chksum end queues end / end
> >>>
> >>> +1, the pattern already give the hint to avoid the ambiguity and I
> >>> +think we
> >> already have ETH_RSS_LEVEL to figure out inner or outer.
> >>
> >> The problem that it may be used in generic RSS flags which has no the
> context.
> >> Also even in the case of flow API context could have no L4 protocol at all.
> >
> > For generic case, it can simply assume it cover all L4 checksum cases and I'm
> not sure if any user intend to use it as generic RSS, pmd can simply reject it if
> it's not necessary to support.
> 
> Try to look at it from an application point of view which does not know any
> specifics of the driver.
> 
>  * Get dev_info and see ETH_RSS_L4_CHKSUM, good!, would like to
>    use it.


The PMD should not expose it if it don't want to (or not able to) support all l4 checksum from generic RSS configure

And we should assume this is only apply for generic RSS configure but not for flow API.

Because the rte_flow_validate is the recommended method to check if a RSS action is supported in flow API or not.

> 
>  * If I try to use it in default RSS config, but the request
>    fail, it could be very confusing.
> 
>  * Will it distribute TCP packets? UDP packets? SCTP packets?
>    Or should I care about RSS for some of them based on other
>    supported fields? E.g. if SCTP is not supported by the NIC,
>    I need to install RSS flow rule for the IP protocol to do
>    RSS based on IPv4/IPv6 addresses. But if SCTP is supported,
>    I'm happy to use ETH_RSS_L4_CHKSUM for it as well.
> 
> > In flow API, if no l4 protocol in pattern , the PMD should return
> > failure (or maybe some default behavior), and I think this is not a
> > new question as it happens all the cases
> > e.g.:
> > pattern eth / vlan / end action rss type ipv4 .
> 
> IMHO, it would be pretty logical to apply RSS to IPv4 packets only and send
> everything else to default queue.

Yes, this also make sense to me, but I think PMD's flow parser still can have more strict check, as it does not drop any feature that the NIC can support.

> 
> >>
> >> Is UDP checksum 0 treated as no checksum and go to default queue or
> >> treated as a regular checksum with value equal to 0?
> >
> > I think we can treat it as value 0, as least our hardware behavior like this, is
> this any issue?
> 
> OK, no problem. Just document it.
> 
> >>
> >> I tend to agree that 3 flags is too much for the feature, but one
> >> flag without properly defined meaning is not good as well.
> >>
> >> I just want rules to be defined and documented.'
> >
> > Agree, we need more document for this. if you agree above proposal.
> >


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

* Re: [dpdk-dev] [PATCH v3] ethdev: add IPv4 and L4 checksum RSS offload types
  2021-07-07 13:00                 ` Zhang, Qi Z
@ 2021-07-07 13:10                   ` Andrew Rybchenko
  2021-07-08  1:07                     ` Zhang, Qi Z
  0 siblings, 1 reply; 39+ messages in thread
From: Andrew Rybchenko @ 2021-07-07 13:10 UTC (permalink / raw)
  To: Zhang, Qi Z, Zhang, AlvinX, ajit.khaparde; +Cc: dev

On 7/7/21 4:00 PM, Zhang, Qi Z wrote:
> 
> 
>> -----Original Message-----
>> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>> Sent: Wednesday, July 7, 2021 5:49 PM
>> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Zhang, AlvinX
>> <alvinx.zhang@intel.com>; ajit.khaparde@broadcom.com
>> Cc: dev@dpdk.org
>> Subject: Re: [PATCH v3] ethdev: add IPv4 and L4 checksum RSS offload types
>>
>> On 7/7/21 6:23 AM, Zhang, Qi Z wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>>>> Sent: Tuesday, July 6, 2021 4:05 PM
>>>> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Zhang, AlvinX
>>>> <alvinx.zhang@intel.com>; ajit.khaparde@broadcom.com
>>>> Cc: dev@dpdk.org
>>>> Subject: Re: [PATCH v3] ethdev: add IPv4 and L4 checksum RSS offload
>>>> types
>>>>
>>>> On 7/6/21 10:18 AM, Zhang, Qi Z wrote:
>>>>>
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Zhang, AlvinX <alvinx.zhang@intel.com>
>>>>>> Sent: Tuesday, July 6, 2021 3:06 PM
>>>>>> To: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>; Zhang, Qi Z
>>>>>> <qi.z.zhang@intel.com>; ajit.khaparde@broadcom.com
>>>>>> Cc: dev@dpdk.org
>>>>>> Subject: RE: [PATCH v3] ethdev: add IPv4 and L4 checksum RSS
>>>>>> offload types
>>>>>>
>>>>>>>> @@ -537,6 +537,8 @@ struct rte_eth_rss_conf {
>>>>>>>>  #define ETH_RSS_PPPOE		   (1ULL << 31)
>>>>>>>>  #define ETH_RSS_ECPRI		   (1ULL << 32)
>>>>>>>>  #define ETH_RSS_MPLS		   (1ULL << 33)
>>>>>>>> +#define ETH_RSS_IPV4_CHKSUM	   (1ULL << 34)
>>>>>>>> +#define ETH_RSS_L4_CHKSUM	   (1ULL << 35)
>>>>>>>
>>>>>>> What does efine which L4 protocols are supported? How user will know?
>>>>>>
>>>>>> I think if we want to support L4 checksum RSS by using below
>>>>>> command port config all rss (all|default|eth|vlan|...)
>>>>>>
>>>>>> We must define TCP/UDP/SCTP checksum RSS separately:
>>>>>> #define ETH_RSS_TCP_CHKSUM	(1ULL << 35)
>>>>>> #define ETH_RSS_UDP_CHKSUM	(1ULL << 36)
>>>>>> #deifne ETH_RSS_SCTP_CHKSUM	(1ULL << 37)
>>>>>>
>>>>>> Here 3 bits are occupied, this is not good for there are not many
>>>>>> bits
>>>> available.
>>>>>>
>>>>>> If we only want to using it in flows, we only need to define
>>>>>> ETH_RSS_L4_CHKSUM, because the flow pattern pointed out the L4
>>>>>> protocol type.
>>>>>> flow create 0 ingress pattern eth / ipv4 / tcp / end actions rss
>>>>>> types l4-chksum end queues end / end
>>>>>
>>>>> +1, the pattern already give the hint to avoid the ambiguity and I
>>>>> +think we
>>>> already have ETH_RSS_LEVEL to figure out inner or outer.
>>>>
>>>> The problem that it may be used in generic RSS flags which has no the
>> context.
>>>> Also even in the case of flow API context could have no L4 protocol at all.
>>>
>>> For generic case, it can simply assume it cover all L4 checksum cases and I'm
>> not sure if any user intend to use it as generic RSS, pmd can simply reject it if
>> it's not necessary to support.
>>
>> Try to look at it from an application point of view which does not know any
>> specifics of the driver.
>>
>>  * Get dev_info and see ETH_RSS_L4_CHKSUM, good!, would like to
>>    use it.
> 
> 
> The PMD should not expose it if it don't want to (or not able to) support all l4 checksum from generic RSS configure

Document what is "all L4".

> 
> And we should assume this is only apply for generic RSS configure but not for flow API.

I don't think so. IMHO, it should report all RSS capabilities
regardless generic vs flow API RSS action.

It is just RSS capabilities reporting w/o any context.

> 
> Because the rte_flow_validate is the recommended method to check if a RSS action is supported in flow API or not.

It could restrict the subset. But superset should be reported
in caps.

> 
>>
>>  * If I try to use it in default RSS config, but the request
>>    fail, it could be very confusing.
>>
>>  * Will it distribute TCP packets? UDP packets? SCTP packets?
>>    Or should I care about RSS for some of them based on other
>>    supported fields? E.g. if SCTP is not supported by the NIC,
>>    I need to install RSS flow rule for the IP protocol to do
>>    RSS based on IPv4/IPv6 addresses. But if SCTP is supported,
>>    I'm happy to use ETH_RSS_L4_CHKSUM for it as well.
>>
>>> In flow API, if no l4 protocol in pattern , the PMD should return
>>> failure (or maybe some default behavior), and I think this is not a
>>> new question as it happens all the cases
>>> e.g.:
>>> pattern eth / vlan / end action rss type ipv4 .
>>
>> IMHO, it would be pretty logical to apply RSS to IPv4 packets only and send
>> everything else to default queue.
> 
> Yes, this also make sense to me, but I think PMD's flow parser still can have more strict check, as it does not drop any feature that the NIC can support.
> 
>>
>>>>
>>>> Is UDP checksum 0 treated as no checksum and go to default queue or
>>>> treated as a regular checksum with value equal to 0?
>>>
>>> I think we can treat it as value 0, as least our hardware behavior like this, is
>> this any issue?
>>
>> OK, no problem. Just document it.
>>
>>>>
>>>> I tend to agree that 3 flags is too much for the feature, but one
>>>> flag without properly defined meaning is not good as well.
>>>>
>>>> I just want rules to be defined and documented.'
>>>
>>> Agree, we need more document for this. if you agree above proposal.
>>>
> 


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

* Re: [dpdk-dev] [PATCH v3] ethdev: add IPv4 and L4 checksum RSS offload types
  2021-07-07 13:10                   ` Andrew Rybchenko
@ 2021-07-08  1:07                     ` Zhang, Qi Z
  2021-07-08  7:45                       ` Andrew Rybchenko
  0 siblings, 1 reply; 39+ messages in thread
From: Zhang, Qi Z @ 2021-07-08  1:07 UTC (permalink / raw)
  To: Andrew Rybchenko, Zhang, AlvinX, ajit.khaparde; +Cc: dev



> -----Original Message-----
> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Sent: Wednesday, July 7, 2021 9:11 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Zhang, AlvinX
> <alvinx.zhang@intel.com>; ajit.khaparde@broadcom.com
> Cc: dev@dpdk.org
> Subject: Re: [PATCH v3] ethdev: add IPv4 and L4 checksum RSS offload types
> 
> On 7/7/21 4:00 PM, Zhang, Qi Z wrote:
> >
> >
> >> -----Original Message-----
> >> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> >> Sent: Wednesday, July 7, 2021 5:49 PM
> >> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Zhang, AlvinX
> >> <alvinx.zhang@intel.com>; ajit.khaparde@broadcom.com
> >> Cc: dev@dpdk.org
> >> Subject: Re: [PATCH v3] ethdev: add IPv4 and L4 checksum RSS offload
> >> types
> >>
> >> On 7/7/21 6:23 AM, Zhang, Qi Z wrote:
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> >>>> Sent: Tuesday, July 6, 2021 4:05 PM
> >>>> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Zhang, AlvinX
> >>>> <alvinx.zhang@intel.com>; ajit.khaparde@broadcom.com
> >>>> Cc: dev@dpdk.org
> >>>> Subject: Re: [PATCH v3] ethdev: add IPv4 and L4 checksum RSS
> >>>> offload types
> >>>>
> >>>> On 7/6/21 10:18 AM, Zhang, Qi Z wrote:
> >>>>>
> >>>>>
> >>>>>> -----Original Message-----
> >>>>>> From: Zhang, AlvinX <alvinx.zhang@intel.com>
> >>>>>> Sent: Tuesday, July 6, 2021 3:06 PM
> >>>>>> To: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>; Zhang, Qi Z
> >>>>>> <qi.z.zhang@intel.com>; ajit.khaparde@broadcom.com
> >>>>>> Cc: dev@dpdk.org
> >>>>>> Subject: RE: [PATCH v3] ethdev: add IPv4 and L4 checksum RSS
> >>>>>> offload types
> >>>>>>
> >>>>>>>> @@ -537,6 +537,8 @@ struct rte_eth_rss_conf {
> >>>>>>>>  #define ETH_RSS_PPPOE		   (1ULL << 31)
> >>>>>>>>  #define ETH_RSS_ECPRI		   (1ULL << 32)
> >>>>>>>>  #define ETH_RSS_MPLS		   (1ULL << 33)
> >>>>>>>> +#define ETH_RSS_IPV4_CHKSUM	   (1ULL << 34)
> >>>>>>>> +#define ETH_RSS_L4_CHKSUM	   (1ULL << 35)
> >>>>>>>
> >>>>>>> What does efine which L4 protocols are supported? How user will
> know?
> >>>>>>
> >>>>>> I think if we want to support L4 checksum RSS by using below
> >>>>>> command port config all rss (all|default|eth|vlan|...)
> >>>>>>
> >>>>>> We must define TCP/UDP/SCTP checksum RSS separately:
> >>>>>> #define ETH_RSS_TCP_CHKSUM	(1ULL << 35)
> >>>>>> #define ETH_RSS_UDP_CHKSUM	(1ULL << 36)
> >>>>>> #deifne ETH_RSS_SCTP_CHKSUM	(1ULL << 37)
> >>>>>>
> >>>>>> Here 3 bits are occupied, this is not good for there are not many
> >>>>>> bits
> >>>> available.
> >>>>>>
> >>>>>> If we only want to using it in flows, we only need to define
> >>>>>> ETH_RSS_L4_CHKSUM, because the flow pattern pointed out the L4
> >>>>>> protocol type.
> >>>>>> flow create 0 ingress pattern eth / ipv4 / tcp / end actions rss
> >>>>>> types l4-chksum end queues end / end
> >>>>>
> >>>>> +1, the pattern already give the hint to avoid the ambiguity and I
> >>>>> +think we
> >>>> already have ETH_RSS_LEVEL to figure out inner or outer.
> >>>>
> >>>> The problem that it may be used in generic RSS flags which has no
> >>>> the
> >> context.
> >>>> Also even in the case of flow API context could have no L4 protocol at all.
> >>>
> >>> For generic case, it can simply assume it cover all L4 checksum
> >>> cases and I'm
> >> not sure if any user intend to use it as generic RSS, pmd can simply
> >> reject it if it's not necessary to support.
> >>
> >> Try to look at it from an application point of view which does not
> >> know any specifics of the driver.
> >>
> >>  * Get dev_info and see ETH_RSS_L4_CHKSUM, good!, would like to
> >>    use it.
> >
> >
> > The PMD should not expose it if it don't want to (or not able to)
> > support all l4 checksum from generic RSS configure
> 
> Document what is "all L4".
> 
> >
> > And we should assume this is only apply for generic RSS configure but not for
> flow API.
> 
> I don't think so. IMHO, it should report all RSS capabilities regardless generic vs
> flow API RSS action.


The RSS action in flow API could cover lots of possibility.
for example an ETH_RSS_IPV4 can be applied on a GTPU flow for inner but may not work for a VxLan flow's inner l3 at the same time.
it's difficult to accurately describe all of these by a 64 bits capability, it's more practice to just rely on rte_flow_validation.
Otherwise it will always leading the confusing you mentioned in previous mail.

It is more reasonable for me, the driver just expose some basic RSS bit that everybody can easiely understand,(e.g.: 5 tuple.), and left all the complexity capability probe to flow API.

> 
> It is just RSS capabilities reporting w/o any context.




> 
> >
> > Because the rte_flow_validate is the recommended method to check if a RSS
> action is supported in flow API or not.
> 
> It could restrict the subset. But superset should be reported in caps.
> 
> >
> >>
> >>  * If I try to use it in default RSS config, but the request
> >>    fail, it could be very confusing.
> >>
> >>  * Will it distribute TCP packets? UDP packets? SCTP packets?
> >>    Or should I care about RSS for some of them based on other
> >>    supported fields? E.g. if SCTP is not supported by the NIC,
> >>    I need to install RSS flow rule for the IP protocol to do
> >>    RSS based on IPv4/IPv6 addresses. But if SCTP is supported,
> >>    I'm happy to use ETH_RSS_L4_CHKSUM for it as well.
> >>
> >>> In flow API, if no l4 protocol in pattern , the PMD should return
> >>> failure (or maybe some default behavior), and I think this is not a
> >>> new question as it happens all the cases
> >>> e.g.:
> >>> pattern eth / vlan / end action rss type ipv4 .
> >>
> >> IMHO, it would be pretty logical to apply RSS to IPv4 packets only
> >> and send everything else to default queue.
> >
> > Yes, this also make sense to me, but I think PMD's flow parser still can have
> more strict check, as it does not drop any feature that the NIC can support.
> >
> >>
> >>>>
> >>>> Is UDP checksum 0 treated as no checksum and go to default queue or
> >>>> treated as a regular checksum with value equal to 0?
> >>>
> >>> I think we can treat it as value 0, as least our hardware behavior
> >>> like this, is
> >> this any issue?
> >>
> >> OK, no problem. Just document it.
> >>
> >>>>
> >>>> I tend to agree that 3 flags is too much for the feature, but one
> >>>> flag without properly defined meaning is not good as well.
> >>>>
> >>>> I just want rules to be defined and documented.'
> >>>
> >>> Agree, we need more document for this. if you agree above proposal.
> >>>
> >


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

* Re: [dpdk-dev] [PATCH v3] ethdev: add IPv4 and L4 checksum RSS offload types
  2021-07-08  1:07                     ` Zhang, Qi Z
@ 2021-07-08  7:45                       ` Andrew Rybchenko
  2021-07-10  8:38                         ` Thomas Monjalon
  0 siblings, 1 reply; 39+ messages in thread
From: Andrew Rybchenko @ 2021-07-08  7:45 UTC (permalink / raw)
  To: Zhang, Qi Z, Zhang, AlvinX, ajit.khaparde
  Cc: dev, Thomas Monjalon, Ferruh Yigit, Ori Kam

@Thomas, @Ferruh, @Ori I need your opinion on the discussion.

On 7/8/21 4:07 AM, Zhang, Qi Z wrote:
> 
> 
>> -----Original Message-----
>> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>> Sent: Wednesday, July 7, 2021 9:11 PM
>> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Zhang, AlvinX
>> <alvinx.zhang@intel.com>; ajit.khaparde@broadcom.com
>> Cc: dev@dpdk.org
>> Subject: Re: [PATCH v3] ethdev: add IPv4 and L4 checksum RSS offload types
>>
>> On 7/7/21 4:00 PM, Zhang, Qi Z wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>>>> Sent: Wednesday, July 7, 2021 5:49 PM
>>>> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Zhang, AlvinX
>>>> <alvinx.zhang@intel.com>; ajit.khaparde@broadcom.com
>>>> Cc: dev@dpdk.org
>>>> Subject: Re: [PATCH v3] ethdev: add IPv4 and L4 checksum RSS offload
>>>> types
>>>>
>>>> On 7/7/21 6:23 AM, Zhang, Qi Z wrote:
>>>>>
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>>>>>> Sent: Tuesday, July 6, 2021 4:05 PM
>>>>>> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Zhang, AlvinX
>>>>>> <alvinx.zhang@intel.com>; ajit.khaparde@broadcom.com
>>>>>> Cc: dev@dpdk.org
>>>>>> Subject: Re: [PATCH v3] ethdev: add IPv4 and L4 checksum RSS
>>>>>> offload types
>>>>>>
>>>>>> On 7/6/21 10:18 AM, Zhang, Qi Z wrote:
>>>>>>>
>>>>>>>
>>>>>>>> -----Original Message-----
>>>>>>>> From: Zhang, AlvinX <alvinx.zhang@intel.com>
>>>>>>>> Sent: Tuesday, July 6, 2021 3:06 PM
>>>>>>>> To: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>; Zhang, Qi Z
>>>>>>>> <qi.z.zhang@intel.com>; ajit.khaparde@broadcom.com
>>>>>>>> Cc: dev@dpdk.org
>>>>>>>> Subject: RE: [PATCH v3] ethdev: add IPv4 and L4 checksum RSS
>>>>>>>> offload types
>>>>>>>>
>>>>>>>>>> @@ -537,6 +537,8 @@ struct rte_eth_rss_conf {
>>>>>>>>>>  #define ETH_RSS_PPPOE		   (1ULL << 31)
>>>>>>>>>>  #define ETH_RSS_ECPRI		   (1ULL << 32)
>>>>>>>>>>  #define ETH_RSS_MPLS		   (1ULL << 33)
>>>>>>>>>> +#define ETH_RSS_IPV4_CHKSUM	   (1ULL << 34)
>>>>>>>>>> +#define ETH_RSS_L4_CHKSUM	   (1ULL << 35)
>>>>>>>>>
>>>>>>>>> What does efine which L4 protocols are supported? How user will
>> know?
>>>>>>>>
>>>>>>>> I think if we want to support L4 checksum RSS by using below
>>>>>>>> command port config all rss (all|default|eth|vlan|...)
>>>>>>>>
>>>>>>>> We must define TCP/UDP/SCTP checksum RSS separately:
>>>>>>>> #define ETH_RSS_TCP_CHKSUM	(1ULL << 35)
>>>>>>>> #define ETH_RSS_UDP_CHKSUM	(1ULL << 36)
>>>>>>>> #deifne ETH_RSS_SCTP_CHKSUM	(1ULL << 37)
>>>>>>>>
>>>>>>>> Here 3 bits are occupied, this is not good for there are not many
>>>>>>>> bits
>>>>>> available.
>>>>>>>>
>>>>>>>> If we only want to using it in flows, we only need to define
>>>>>>>> ETH_RSS_L4_CHKSUM, because the flow pattern pointed out the L4
>>>>>>>> protocol type.
>>>>>>>> flow create 0 ingress pattern eth / ipv4 / tcp / end actions rss
>>>>>>>> types l4-chksum end queues end / end
>>>>>>>
>>>>>>> +1, the pattern already give the hint to avoid the ambiguity and I
>>>>>>> +think we
>>>>>> already have ETH_RSS_LEVEL to figure out inner or outer.
>>>>>>
>>>>>> The problem that it may be used in generic RSS flags which has no
>>>>>> the
>>>> context.
>>>>>> Also even in the case of flow API context could have no L4 protocol at all.
>>>>>
>>>>> For generic case, it can simply assume it cover all L4 checksum
>>>>> cases and I'm
>>>> not sure if any user intend to use it as generic RSS, pmd can simply
>>>> reject it if it's not necessary to support.
>>>>
>>>> Try to look at it from an application point of view which does not
>>>> know any specifics of the driver.
>>>>
>>>>  * Get dev_info and see ETH_RSS_L4_CHKSUM, good!, would like to
>>>>    use it.
>>>
>>>
>>> The PMD should not expose it if it don't want to (or not able to)
>>> support all l4 checksum from generic RSS configure
>>
>> Document what is "all L4".
>>
>>>
>>> And we should assume this is only apply for generic RSS configure but not for
>> flow API.
>>
>> I don't think so. IMHO, it should report all RSS capabilities regardless generic vs
>> flow API RSS action.
> 
> 
> The RSS action in flow API could cover lots of possibility.
> for example an ETH_RSS_IPV4 can be applied on a GTPU flow for inner but may not work for a VxLan flow's inner l3 at the same time.
> it's difficult to accurately describe all of these by a 64 bits capability, it's more practice to just rely on rte_flow_validation.
> Otherwise it will always leading the confusing you mentioned in previous mail.
> 
> It is more reasonable for me, the driver just expose some basic RSS bit that everybody can easiely understand,(e.g.: 5 tuple.), and left all the complexity capability probe to flow API.

May be it is OK to report subset in
dev_info->flow_type_rss_offloads, but I'm very
uncomfortable with the approach. Superset sounds
more logical to me, but has drawbacks as well.

> 
>>
>> It is just RSS capabilities reporting w/o any context.
> 
> 
> 
> 
>>
>>>
>>> Because the rte_flow_validate is the recommended method to check if a RSS
>> action is supported in flow API or not.
>>
>> It could restrict the subset. But superset should be reported in caps.
>>
>>>
>>>>
>>>>  * If I try to use it in default RSS config, but the request
>>>>    fail, it could be very confusing.
>>>>
>>>>  * Will it distribute TCP packets? UDP packets? SCTP packets?
>>>>    Or should I care about RSS for some of them based on other
>>>>    supported fields? E.g. if SCTP is not supported by the NIC,
>>>>    I need to install RSS flow rule for the IP protocol to do
>>>>    RSS based on IPv4/IPv6 addresses. But if SCTP is supported,
>>>>    I'm happy to use ETH_RSS_L4_CHKSUM for it as well.
>>>>
>>>>> In flow API, if no l4 protocol in pattern , the PMD should return
>>>>> failure (or maybe some default behavior), and I think this is not a
>>>>> new question as it happens all the cases
>>>>> e.g.:
>>>>> pattern eth / vlan / end action rss type ipv4 .
>>>>
>>>> IMHO, it would be pretty logical to apply RSS to IPv4 packets only
>>>> and send everything else to default queue.
>>>
>>> Yes, this also make sense to me, but I think PMD's flow parser still can have
>> more strict check, as it does not drop any feature that the NIC can support.
>>>
>>>>
>>>>>>
>>>>>> Is UDP checksum 0 treated as no checksum and go to default queue or
>>>>>> treated as a regular checksum with value equal to 0?
>>>>>
>>>>> I think we can treat it as value 0, as least our hardware behavior
>>>>> like this, is
>>>> this any issue?
>>>>
>>>> OK, no problem. Just document it.
>>>>
>>>>>>
>>>>>> I tend to agree that 3 flags is too much for the feature, but one
>>>>>> flag without properly defined meaning is not good as well.
>>>>>>
>>>>>> I just want rules to be defined and documented.'
>>>>>
>>>>> Agree, we need more document for this. if you agree above proposal.
>>>>>
>>>
> 


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

* Re: [dpdk-dev] [PATCH v3] ethdev: add IPv4 and L4 checksum RSS offload types
  2021-07-08  7:45                       ` Andrew Rybchenko
@ 2021-07-10  8:38                         ` Thomas Monjalon
  0 siblings, 0 replies; 39+ messages in thread
From: Thomas Monjalon @ 2021-07-10  8:38 UTC (permalink / raw)
  To: Zhang, Qi Z, Zhang, AlvinX, Andrew Rybchenko
  Cc: ajit.khaparde, dev, Ferruh Yigit, Ori Kam, olivier.matz

08/07/2021 09:45, Andrew Rybchenko:
> @Thomas, @Ferruh, @Ori I need your opinion on the discussion.
> 
> On 7/8/21 4:07 AM, Zhang, Qi Z wrote:
> > From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> >>> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> >>>> On 7/7/21 6:23 AM, Zhang, Qi Z wrote:
> >>>>> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> >>>>>> On 7/6/21 10:18 AM, Zhang, Qi Z wrote:
> >>>>>>> From: Zhang, AlvinX <alvinx.zhang@intel.com>
> >>>>>>>>
> >>>>>>>>>> @@ -537,6 +537,8 @@ struct rte_eth_rss_conf {
> >>>>>>>>>>  #define ETH_RSS_PPPOE		   (1ULL << 31)
> >>>>>>>>>>  #define ETH_RSS_ECPRI		   (1ULL << 32)
> >>>>>>>>>>  #define ETH_RSS_MPLS		   (1ULL << 33)
> >>>>>>>>>> +#define ETH_RSS_IPV4_CHKSUM	   (1ULL << 34)
> >>>>>>>>>> +#define ETH_RSS_L4_CHKSUM	   (1ULL << 35)
> >>>>>>>>>
> >>>>>>>>> What does efine which L4 protocols are supported? How user will
> >> know?
> >>>>>>>>
> >>>>>>>> I think if we want to support L4 checksum RSS by using below
> >>>>>>>> command port config all rss (all|default|eth|vlan|...)
> >>>>>>>>
> >>>>>>>> We must define TCP/UDP/SCTP checksum RSS separately:
> >>>>>>>> #define ETH_RSS_TCP_CHKSUM	(1ULL << 35)
> >>>>>>>> #define ETH_RSS_UDP_CHKSUM	(1ULL << 36)
> >>>>>>>> #deifne ETH_RSS_SCTP_CHKSUM	(1ULL << 37)
> >>>>>>>>
> >>>>>>>> Here 3 bits are occupied, this is not good for there are not many
> >>>>>>>> bits
> >>>>>> available.
> >>>>>>>>
> >>>>>>>> If we only want to using it in flows, we only need to define
> >>>>>>>> ETH_RSS_L4_CHKSUM, because the flow pattern pointed out the L4
> >>>>>>>> protocol type.
> >>>>>>>> flow create 0 ingress pattern eth / ipv4 / tcp / end actions rss
> >>>>>>>> types l4-chksum end queues end / end
> >>>>>>>
> >>>>>>> +1, the pattern already give the hint to avoid the ambiguity and I
> >>>>>>> +think we
> >>>>>> already have ETH_RSS_LEVEL to figure out inner or outer.
> >>>>>>
> >>>>>> The problem that it may be used in generic RSS flags which has no
> >>>>>> the
> >>>> context.
> >>>>>> Also even in the case of flow API context could have no L4 protocol at all.
> >>>>>
> >>>>> For generic case, it can simply assume it cover all L4 checksum
> >>>>> cases and I'm
> >>>> not sure if any user intend to use it as generic RSS, pmd can simply
> >>>> reject it if it's not necessary to support.
> >>>>
> >>>> Try to look at it from an application point of view which does not
> >>>> know any specifics of the driver.
> >>>>
> >>>>  * Get dev_info and see ETH_RSS_L4_CHKSUM, good!, would like to
> >>>>    use it.
> >>>
> >>>
> >>> The PMD should not expose it if it don't want to (or not able to)
> >>> support all l4 checksum from generic RSS configure

That's restrictive to allow only full-support,
but I'm fine with the trade-off to avoid wasting bits.

> >>
> >> Document what is "all L4".

List of L4 protocols should be explicit.


> >>> And we should assume this is only apply for generic RSS configure but not for
> >> flow API.
> >>
> >> I don't think so. IMHO, it should report all RSS capabilities regardless generic vs
> >> flow API RSS action.
> > 
> > 
> > The RSS action in flow API could cover lots of possibility.
> > for example an ETH_RSS_IPV4 can be applied on a GTPU flow for inner but may not work for a VxLan flow's inner l3 at the same time.
> > it's difficult to accurately describe all of these by a 64 bits capability, it's more practice to just rely on rte_flow_validation.
> > Otherwise it will always leading the confusing you mentioned in previous mail.
> > 
> > It is more reasonable for me, the driver just expose some basic RSS bit that everybody can easiely understand,(e.g.: 5 tuple.), and left all the complexity capability probe to flow API.
> 
> May be it is OK to report subset in
> dev_info->flow_type_rss_offloads, but I'm very
> uncomfortable with the approach. Superset sounds
> more logical to me, but has drawbacks as well.

Yes superset should be reported, this is the meaning of capabilities:
the driver is capable but there are some limitations
which cannot be advertised, so rte_flow_validate checks the limitations
in the dynamic context.


> >> It is just RSS capabilities reporting w/o any context.
> > 
> >>>
> >>> Because the rte_flow_validate is the recommended method to check if a RSS
> >> action is supported in flow API or not.
> >>
> >> It could restrict the subset. But superset should be reported in caps.
> >>
> >>>
> >>>>
> >>>>  * If I try to use it in default RSS config, but the request
> >>>>    fail, it could be very confusing.
> >>>>
> >>>>  * Will it distribute TCP packets? UDP packets? SCTP packets?
> >>>>    Or should I care about RSS for some of them based on other
> >>>>    supported fields? E.g. if SCTP is not supported by the NIC,
> >>>>    I need to install RSS flow rule for the IP protocol to do
> >>>>    RSS based on IPv4/IPv6 addresses. But if SCTP is supported,
> >>>>    I'm happy to use ETH_RSS_L4_CHKSUM for it as well.
> >>>>
> >>>>> In flow API, if no l4 protocol in pattern , the PMD should return
> >>>>> failure (or maybe some default behavior), and I think this is not a
> >>>>> new question as it happens all the cases
> >>>>> e.g.:
> >>>>> pattern eth / vlan / end action rss type ipv4 .
> >>>>
> >>>> IMHO, it would be pretty logical to apply RSS to IPv4 packets only
> >>>> and send everything else to default queue.
> >>>
> >>> Yes, this also make sense to me, but I think PMD's flow parser still can have
> >> more strict check, as it does not drop any feature that the NIC can support.
> >>>
> >>>>
> >>>>>>
> >>>>>> Is UDP checksum 0 treated as no checksum and go to default queue or
> >>>>>> treated as a regular checksum with value equal to 0?
> >>>>>
> >>>>> I think we can treat it as value 0, as least our hardware behavior
> >>>>> like this, is
> >>>> this any issue?
> >>>>
> >>>> OK, no problem. Just document it.
> >>>>
> >>>>>>
> >>>>>> I tend to agree that 3 flags is too much for the feature, but one
> >>>>>> flag without properly defined meaning is not good as well.
> >>>>>>
> >>>>>> I just want rules to be defined and documented.'
> >>>>>
> >>>>> Agree, we need more document for this. if you agree above proposal.

Yes please do not add a new flag in rte_ethdev.h without doc.




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

* [dpdk-dev] [PATCH v4] ethdev: add IPv4 and L4 checksum RSS offload types
  2021-06-15  8:19   ` [dpdk-dev] [PATCH v3] ethdev: add IPv4 and L4 checksum RSS offload types Alvin Zhang
  2021-06-15  8:26     ` Jerin Jacob
  2021-07-01 14:26     ` Andrew Rybchenko
@ 2021-07-13  1:13     ` Alvin Zhang
  2021-07-13  7:54       ` Andrew Rybchenko
  2021-08-18  2:32       ` [dpdk-dev] [PATCH v5] " Alvin Zhang
  2 siblings, 2 replies; 39+ messages in thread
From: Alvin Zhang @ 2021-07-13  1:13 UTC (permalink / raw)
  To: qi.z.zhang, andrew.rybchenko, ajit.khaparde
  Cc: dev, Alvin Zhang, Aman Deep Singh

This patch defines new RSS offload types for IPv4 and
L4(TCP/UDP/SCTP) checksum, which are required when users want
to distribute packets based on the IPv4 or L4 checksum field.

For example "flow create 0 ingress pattern eth / ipv4 / end
actions rss types ipv4-chksum end queues end / end", this flow
causes all matching packets to be distributed to queues on
basis of IPv4 checksum.

Signed-off-by: Alvin Zhang <alvinx.zhang@intel.com>
Reviewed-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
Acked-by: Aman Deep Singh <aman.deep.singh@intel.com>
---

v3: Add L4 checksum RSS offload type
v4: Add doc and help string, update commit log
---
 app/test-pmd/cmdline.c  | 12 +++++++++---
 app/test-pmd/config.c   |  2 ++
 lib/ethdev/rte_ethdev.h |  2 ++
 3 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 0268b18..93543d8 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -2254,6 +2254,10 @@ struct cmd_config_rss {
 		rss_conf.rss_hf = ETH_RSS_ECPRI;
 	else if (!strcmp(res->value, "mpls"))
 		rss_conf.rss_hf = ETH_RSS_MPLS;
+	else if (!strcmp(res->value, "ipv4-chksum"))
+		rss_conf.rss_hf = ETH_RSS_IPV4_CHKSUM;
+	else if (!strcmp(res->value, "l4-chksum"))
+		rss_conf.rss_hf = ETH_RSS_L4_CHKSUM;
 	else if (!strcmp(res->value, "none"))
 		rss_conf.rss_hf = 0;
 	else if (!strcmp(res->value, "level-default")) {
@@ -2325,7 +2329,7 @@ struct cmd_config_rss {
 	.help_str = "port config all rss "
 		"all|default|eth|vlan|ip|tcp|udp|sctp|ether|port|vxlan|geneve|"
 		"nvgre|vxlan-gpe|l2tpv3|esp|ah|pfcp|ecpri|mpls|none|level-default|"
-		"level-outer|level-inner|<flowtype_id>",
+		"level-outer|level-inner|ipv4-chksum|l4-chksum|<flowtype_id>",
 	.tokens = {
 		(void *)&cmd_config_rss_port,
 		(void *)&cmd_config_rss_keyword,
@@ -2438,7 +2442,8 @@ struct cmd_config_rss_hash_key {
 				 "ipv6-tcp-ex#ipv6-udp-ex#"
 				 "l3-src-only#l3-dst-only#l4-src-only#l4-dst-only#"
 				 "l2-src-only#l2-dst-only#s-vlan#c-vlan#"
-				 "l2tpv3#esp#ah#pfcp#pppoe#gtpu#ecpri#mpls");
+				 "l2tpv3#esp#ah#pfcp#pppoe#gtpu#ecpri#mpls#"
+				 "ipv4-chksum#l4-chksum");
 cmdline_parse_token_string_t cmd_config_rss_hash_key_value =
 	TOKEN_STRING_INITIALIZER(struct cmd_config_rss_hash_key, key, NULL);
 
@@ -2451,7 +2456,8 @@ struct cmd_config_rss_hash_key {
 		"l2-payload|ipv6-ex|ipv6-tcp-ex|ipv6-udp-ex|"
 		"l3-src-only|l3-dst-only|l4-src-only|l4-dst-only|"
 		"l2-src-only|l2-dst-only|s-vlan|c-vlan|"
-		"l2tpv3|esp|ah|pfcp|pppoe|gtpu|ecpri|mpls "
+		"l2tpv3|esp|ah|pfcp|pppoe|gtpu|ecpri|mpls|"
+		"ipv4-chksum|l4-chksum "
 		"<string of hex digits (variable length, NIC dependent)>",
 	.tokens = {
 		(void *)&cmd_config_rss_hash_key_port,
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 43c79b5..14968bf 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -140,6 +140,8 @@
 	{ "gtpu", ETH_RSS_GTPU },
 	{ "ecpri", ETH_RSS_ECPRI },
 	{ "mpls", ETH_RSS_MPLS },
+	{ "ipv4-chksum", ETH_RSS_IPV4_CHKSUM },
+	{ "l4-chksum", ETH_RSS_L4_CHKSUM },
 	{ NULL, 0 },
 };
 
diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index faf3bd9..63b0321 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -537,6 +537,8 @@ struct rte_eth_rss_conf {
 #define ETH_RSS_PPPOE		   (1ULL << 31)
 #define ETH_RSS_ECPRI		   (1ULL << 32)
 #define ETH_RSS_MPLS		   (1ULL << 33)
+#define ETH_RSS_IPV4_CHKSUM	   (1ULL << 34)
+#define ETH_RSS_L4_CHKSUM	   (1ULL << 35)	/* TCP/UDP/SCTP */
 
 /*
  * We use the following macros to combine with above ETH_RSS_* for
-- 
1.8.3.1


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

* Re: [dpdk-dev] [PATCH v4] ethdev: add IPv4 and L4 checksum RSS offload types
  2021-07-13  1:13     ` [dpdk-dev] [PATCH v4] " Alvin Zhang
@ 2021-07-13  7:54       ` Andrew Rybchenko
  2021-07-13  9:38         ` Zhang, AlvinX
  2021-08-18  2:32       ` [dpdk-dev] [PATCH v5] " Alvin Zhang
  1 sibling, 1 reply; 39+ messages in thread
From: Andrew Rybchenko @ 2021-07-13  7:54 UTC (permalink / raw)
  To: Alvin Zhang, qi.z.zhang, ajit.khaparde; +Cc: dev, Aman Deep Singh

On 7/13/21 4:13 AM, Alvin Zhang wrote:
> This patch defines new RSS offload types for IPv4 and
> L4(TCP/UDP/SCTP) checksum, which are required when users want
> to distribute packets based on the IPv4 or L4 checksum field.
> 
> For example "flow create 0 ingress pattern eth / ipv4 / end
> actions rss types ipv4-chksum end queues end / end", this flow
> causes all matching packets to be distributed to queues on
> basis of IPv4 checksum.
> 
> Signed-off-by: Alvin Zhang <alvinx.zhang@intel.com>
> Reviewed-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>

I've failed to find mail where I've added the tag. I've replied
with "LGTM" for v2 which has IPv4 checksum only and many
comments on L4 checksum added in v3. So, I think it is
incorrect to inherit "LGTM" as Reviewed-by.

> Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> Acked-by: Aman Deep Singh <aman.deep.singh@intel.com>
> ---
> 
> v3: Add L4 checksum RSS offload type
> v4: Add doc and help string, update commit log
> ---
>  app/test-pmd/cmdline.c  | 12 +++++++++---
>  app/test-pmd/config.c   |  2 ++
>  lib/ethdev/rte_ethdev.h |  2 ++
>  3 files changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index 0268b18..93543d8 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -2254,6 +2254,10 @@ struct cmd_config_rss {
>  		rss_conf.rss_hf = ETH_RSS_ECPRI;
>  	else if (!strcmp(res->value, "mpls"))
>  		rss_conf.rss_hf = ETH_RSS_MPLS;
> +	else if (!strcmp(res->value, "ipv4-chksum"))
> +		rss_conf.rss_hf = ETH_RSS_IPV4_CHKSUM;
> +	else if (!strcmp(res->value, "l4-chksum"))
> +		rss_conf.rss_hf = ETH_RSS_L4_CHKSUM;
>  	else if (!strcmp(res->value, "none"))
>  		rss_conf.rss_hf = 0;
>  	else if (!strcmp(res->value, "level-default")) {
> @@ -2325,7 +2329,7 @@ struct cmd_config_rss {
>  	.help_str = "port config all rss "
>  		"all|default|eth|vlan|ip|tcp|udp|sctp|ether|port|vxlan|geneve|"
>  		"nvgre|vxlan-gpe|l2tpv3|esp|ah|pfcp|ecpri|mpls|none|level-default|"
> -		"level-outer|level-inner|<flowtype_id>",
> +		"level-outer|level-inner|ipv4-chksum|l4-chksum|<flowtype_id>",
>  	.tokens = {
>  		(void *)&cmd_config_rss_port,
>  		(void *)&cmd_config_rss_keyword,
> @@ -2438,7 +2442,8 @@ struct cmd_config_rss_hash_key {
>  				 "ipv6-tcp-ex#ipv6-udp-ex#"
>  				 "l3-src-only#l3-dst-only#l4-src-only#l4-dst-only#"
>  				 "l2-src-only#l2-dst-only#s-vlan#c-vlan#"
> -				 "l2tpv3#esp#ah#pfcp#pppoe#gtpu#ecpri#mpls");
> +				 "l2tpv3#esp#ah#pfcp#pppoe#gtpu#ecpri#mpls#"
> +				 "ipv4-chksum#l4-chksum");
>  cmdline_parse_token_string_t cmd_config_rss_hash_key_value =
>  	TOKEN_STRING_INITIALIZER(struct cmd_config_rss_hash_key, key, NULL);
>  
> @@ -2451,7 +2456,8 @@ struct cmd_config_rss_hash_key {
>  		"l2-payload|ipv6-ex|ipv6-tcp-ex|ipv6-udp-ex|"
>  		"l3-src-only|l3-dst-only|l4-src-only|l4-dst-only|"
>  		"l2-src-only|l2-dst-only|s-vlan|c-vlan|"
> -		"l2tpv3|esp|ah|pfcp|pppoe|gtpu|ecpri|mpls "
> +		"l2tpv3|esp|ah|pfcp|pppoe|gtpu|ecpri|mpls|"
> +		"ipv4-chksum|l4-chksum "
>  		"<string of hex digits (variable length, NIC dependent)>",
>  	.tokens = {
>  		(void *)&cmd_config_rss_hash_key_port,
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index 43c79b5..14968bf 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -140,6 +140,8 @@
>  	{ "gtpu", ETH_RSS_GTPU },
>  	{ "ecpri", ETH_RSS_ECPRI },
>  	{ "mpls", ETH_RSS_MPLS },
> +	{ "ipv4-chksum", ETH_RSS_IPV4_CHKSUM },
> +	{ "l4-chksum", ETH_RSS_L4_CHKSUM },
>  	{ NULL, 0 },
>  };
>  
> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
> index faf3bd9..63b0321 100644
> --- a/lib/ethdev/rte_ethdev.h
> +++ b/lib/ethdev/rte_ethdev.h
> @@ -537,6 +537,8 @@ struct rte_eth_rss_conf {
>  #define ETH_RSS_PPPOE		   (1ULL << 31)
>  #define ETH_RSS_ECPRI		   (1ULL << 32)
>  #define ETH_RSS_MPLS		   (1ULL << 33)
> +#define ETH_RSS_IPV4_CHKSUM	   (1ULL << 34)
> +#define ETH_RSS_L4_CHKSUM	   (1ULL << 35)	/* TCP/UDP/SCTP */

It does not reply on my questions at all.

Nack

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

* Re: [dpdk-dev] [PATCH v4] ethdev: add IPv4 and L4 checksum RSS offload types
  2021-07-13  7:54       ` Andrew Rybchenko
@ 2021-07-13  9:38         ` Zhang, AlvinX
  2021-07-13 10:24           ` Andrew Rybchenko
  0 siblings, 1 reply; 39+ messages in thread
From: Zhang, AlvinX @ 2021-07-13  9:38 UTC (permalink / raw)
  To: Andrew Rybchenko, Zhang, Qi Z, ajit.khaparde; +Cc: dev, Singh, Aman Deep

> -----Original Message-----
> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Sent: Tuesday, July 13, 2021 3:55 PM
> To: Zhang, AlvinX <alvinx.zhang@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>; ajit.khaparde@broadcom.com
> Cc: dev@dpdk.org; Singh, Aman Deep <aman.deep.singh@intel.com>
> Subject: Re: [PATCH v4] ethdev: add IPv4 and L4 checksum RSS offload types
> 
> On 7/13/21 4:13 AM, Alvin Zhang wrote:
> > This patch defines new RSS offload types for IPv4 and
> > L4(TCP/UDP/SCTP) checksum, which are required when users want to
> > distribute packets based on the IPv4 or L4 checksum field.
> >
> > For example "flow create 0 ingress pattern eth / ipv4 / end actions
> > rss types ipv4-chksum end queues end / end", this flow causes all
> > matching packets to be distributed to queues on basis of IPv4
> > checksum.
> >
> > Signed-off-by: Alvin Zhang <alvinx.zhang@intel.com>
> > Reviewed-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> 
> I've failed to find mail where I've added the tag. I've replied with "LGTM" for v2
> which has IPv4 checksum only and many comments on L4 checksum added in v3.
> So, I think it is incorrect to inherit "LGTM" as Reviewed-by.
> 
 
I'll correct it at next version.         
    
> > Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> > Acked-by: Aman Deep Singh <aman.deep.singh@intel.com>
> > ---
> >
> > v3: Add L4 checksum RSS offload type
> > v4: Add doc and help string, update commit log
> > ---
> >  app/test-pmd/cmdline.c  | 12 +++++++++---
> >  app/test-pmd/config.c   |  2 ++
> >  lib/ethdev/rte_ethdev.h |  2 ++
> >  3 files changed, 13 insertions(+), 3 deletions(-)
> >
> > diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index
> > 0268b18..93543d8 100644
> > --- a/app/test-pmd/cmdline.c
> > +++ b/app/test-pmd/cmdline.c
> > @@ -2254,6 +2254,10 @@ struct cmd_config_rss {
> >  		rss_conf.rss_hf = ETH_RSS_ECPRI;
> >  	else if (!strcmp(res->value, "mpls"))
> >  		rss_conf.rss_hf = ETH_RSS_MPLS;
> > +	else if (!strcmp(res->value, "ipv4-chksum"))
> > +		rss_conf.rss_hf = ETH_RSS_IPV4_CHKSUM;
> > +	else if (!strcmp(res->value, "l4-chksum"))
> > +		rss_conf.rss_hf = ETH_RSS_L4_CHKSUM;
> >  	else if (!strcmp(res->value, "none"))
> >  		rss_conf.rss_hf = 0;
> >  	else if (!strcmp(res->value, "level-default")) { @@ -2325,7 +2329,7
> > @@ struct cmd_config_rss {
> >  	.help_str = "port config all rss "
> >  		"all|default|eth|vlan|ip|tcp|udp|sctp|ether|port|vxlan|geneve|"
> >
> 	"nvgre|vxlan-gpe|l2tpv3|esp|ah|pfcp|ecpri|mpls|none|level-default|"
> > -		"level-outer|level-inner|<flowtype_id>",
> > +		"level-outer|level-inner|ipv4-chksum|l4-chksum|<flowtype_id>",
> >  	.tokens = {
> >  		(void *)&cmd_config_rss_port,
> >  		(void *)&cmd_config_rss_keyword,
> > @@ -2438,7 +2442,8 @@ struct cmd_config_rss_hash_key {
> >  				 "ipv6-tcp-ex#ipv6-udp-ex#"
> >  				 "l3-src-only#l3-dst-only#l4-src-only#l4-dst-only#"
> >  				 "l2-src-only#l2-dst-only#s-vlan#c-vlan#"
> > -				 "l2tpv3#esp#ah#pfcp#pppoe#gtpu#ecpri#mpls");
> > +				 "l2tpv3#esp#ah#pfcp#pppoe#gtpu#ecpri#mpls#"
> > +				 "ipv4-chksum#l4-chksum");
> >  cmdline_parse_token_string_t cmd_config_rss_hash_key_value =
> >  	TOKEN_STRING_INITIALIZER(struct cmd_config_rss_hash_key, key, NULL);
> >
> > @@ -2451,7 +2456,8 @@ struct cmd_config_rss_hash_key {
> >  		"l2-payload|ipv6-ex|ipv6-tcp-ex|ipv6-udp-ex|"
> >  		"l3-src-only|l3-dst-only|l4-src-only|l4-dst-only|"
> >  		"l2-src-only|l2-dst-only|s-vlan|c-vlan|"
> > -		"l2tpv3|esp|ah|pfcp|pppoe|gtpu|ecpri|mpls "
> > +		"l2tpv3|esp|ah|pfcp|pppoe|gtpu|ecpri|mpls|"
> > +		"ipv4-chksum|l4-chksum "
> >  		"<string of hex digits (variable length, NIC dependent)>",
> >  	.tokens = {
> >  		(void *)&cmd_config_rss_hash_key_port, diff --git
> > a/app/test-pmd/config.c b/app/test-pmd/config.c index 43c79b5..14968bf
> > 100644
> > --- a/app/test-pmd/config.c
> > +++ b/app/test-pmd/config.c
> > @@ -140,6 +140,8 @@
> >  	{ "gtpu", ETH_RSS_GTPU },
> >  	{ "ecpri", ETH_RSS_ECPRI },
> >  	{ "mpls", ETH_RSS_MPLS },
> > +	{ "ipv4-chksum", ETH_RSS_IPV4_CHKSUM },
> > +	{ "l4-chksum", ETH_RSS_L4_CHKSUM },
> >  	{ NULL, 0 },
> >  };
> >
> > diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h index
> > faf3bd9..63b0321 100644
> > --- a/lib/ethdev/rte_ethdev.h
> > +++ b/lib/ethdev/rte_ethdev.h
> > @@ -537,6 +537,8 @@ struct rte_eth_rss_conf {
> >  #define ETH_RSS_PPPOE		   (1ULL << 31)
> >  #define ETH_RSS_ECPRI		   (1ULL << 32)
> >  #define ETH_RSS_MPLS		   (1ULL << 33)
> > +#define ETH_RSS_IPV4_CHKSUM	   (1ULL << 34)
> > +#define ETH_RSS_L4_CHKSUM	   (1ULL << 35)	/* TCP/UDP/SCTP */
> 
> It does not reply on my questions at all.
  
You have said:       
> >> I don't think so. IMHO, it should report all RSS capabilities      
> >> regardless generic vs flow API RSS action.       
> >      
> May be it is OK to report subset in      
> dev_info->flow_type_rss_offloads, but I'm very uncomfortable with the       
> approach. Superset sounds more logical to me, but has drawbacks as        
> well.         

Here I have another question:              
There are flow type definition and RSS offload type definition,         
#define RTE_ETH_FLOW_RAW                 1       
#define RTE_ETH_FLOW_IPV4                2     
#define RTE_ETH_FLOW_FRAG_IPV4           3       
#define RTE_ETH_FLOW_NONFRAG_IPV4_TCP    4        

#define ETH_RSS_IPV4               (1ULL << 2)      
#define ETH_RSS_FRAG_IPV4          (1ULL << 3)         
#define ETH_RSS_NONFRAG_IPV4_TCP   (1ULL << 4)        
  
are they the different expressions of the same concept?             

If yes, why they have been decoupled by the commit: fce6b66893.   
Then what the flow type of ETH_RSS_PORT, it's UDP, TCP, or SCTP?      

If not, the PMDs can report supported RSS flow type by dev_info->flow_type_rss_offloads,   
but have no way to report supported RSS offload types.          

Thanks Alvin

> 
> Nack

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

* Re: [dpdk-dev] [PATCH v4] ethdev: add IPv4 and L4 checksum RSS offload types
  2021-07-13  9:38         ` Zhang, AlvinX
@ 2021-07-13 10:24           ` Andrew Rybchenko
  2021-07-14  2:38             ` Zhang, AlvinX
  0 siblings, 1 reply; 39+ messages in thread
From: Andrew Rybchenko @ 2021-07-13 10:24 UTC (permalink / raw)
  To: Zhang, AlvinX, Zhang, Qi Z, ajit.khaparde; +Cc: dev, Singh, Aman Deep

On 7/13/21 12:38 PM, Zhang, AlvinX wrote:
>> -----Original Message-----
>> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>> Sent: Tuesday, July 13, 2021 3:55 PM
>> To: Zhang, AlvinX <alvinx.zhang@intel.com>; Zhang, Qi Z
>> <qi.z.zhang@intel.com>; ajit.khaparde@broadcom.com
>> Cc: dev@dpdk.org; Singh, Aman Deep <aman.deep.singh@intel.com>
>> Subject: Re: [PATCH v4] ethdev: add IPv4 and L4 checksum RSS offload types
>>
>> On 7/13/21 4:13 AM, Alvin Zhang wrote:
>>> This patch defines new RSS offload types for IPv4 and
>>> L4(TCP/UDP/SCTP) checksum, which are required when users want to
>>> distribute packets based on the IPv4 or L4 checksum field.
>>>
>>> For example "flow create 0 ingress pattern eth / ipv4 / end actions
>>> rss types ipv4-chksum end queues end / end", this flow causes all
>>> matching packets to be distributed to queues on basis of IPv4
>>> checksum.
>>>
>>> Signed-off-by: Alvin Zhang <alvinx.zhang@intel.com>
>>> Reviewed-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>>
>> I've failed to find mail where I've added the tag. I've replied with "LGTM" for v2
>> which has IPv4 checksum only and many comments on L4 checksum added in v3.
>> So, I think it is incorrect to inherit "LGTM" as Reviewed-by.
>>
>  
> I'll correct it at next version.         
>     
>>> Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
>>> Acked-by: Aman Deep Singh <aman.deep.singh@intel.com>
>>> ---
>>>
>>> v3: Add L4 checksum RSS offload type
>>> v4: Add doc and help string, update commit log
>>> ---
>>>  app/test-pmd/cmdline.c  | 12 +++++++++---
>>>  app/test-pmd/config.c   |  2 ++
>>>  lib/ethdev/rte_ethdev.h |  2 ++
>>>  3 files changed, 13 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index
>>> 0268b18..93543d8 100644
>>> --- a/app/test-pmd/cmdline.c
>>> +++ b/app/test-pmd/cmdline.c
>>> @@ -2254,6 +2254,10 @@ struct cmd_config_rss {
>>>  		rss_conf.rss_hf = ETH_RSS_ECPRI;
>>>  	else if (!strcmp(res->value, "mpls"))
>>>  		rss_conf.rss_hf = ETH_RSS_MPLS;
>>> +	else if (!strcmp(res->value, "ipv4-chksum"))
>>> +		rss_conf.rss_hf = ETH_RSS_IPV4_CHKSUM;
>>> +	else if (!strcmp(res->value, "l4-chksum"))
>>> +		rss_conf.rss_hf = ETH_RSS_L4_CHKSUM;
>>>  	else if (!strcmp(res->value, "none"))
>>>  		rss_conf.rss_hf = 0;
>>>  	else if (!strcmp(res->value, "level-default")) { @@ -2325,7 +2329,7
>>> @@ struct cmd_config_rss {
>>>  	.help_str = "port config all rss "
>>>  		"all|default|eth|vlan|ip|tcp|udp|sctp|ether|port|vxlan|geneve|"
>>>
>> 	"nvgre|vxlan-gpe|l2tpv3|esp|ah|pfcp|ecpri|mpls|none|level-default|"
>>> -		"level-outer|level-inner|<flowtype_id>",
>>> +		"level-outer|level-inner|ipv4-chksum|l4-chksum|<flowtype_id>",
>>>  	.tokens = {
>>>  		(void *)&cmd_config_rss_port,
>>>  		(void *)&cmd_config_rss_keyword,
>>> @@ -2438,7 +2442,8 @@ struct cmd_config_rss_hash_key {
>>>  				 "ipv6-tcp-ex#ipv6-udp-ex#"
>>>  				 "l3-src-only#l3-dst-only#l4-src-only#l4-dst-only#"
>>>  				 "l2-src-only#l2-dst-only#s-vlan#c-vlan#"
>>> -				 "l2tpv3#esp#ah#pfcp#pppoe#gtpu#ecpri#mpls");
>>> +				 "l2tpv3#esp#ah#pfcp#pppoe#gtpu#ecpri#mpls#"
>>> +				 "ipv4-chksum#l4-chksum");
>>>  cmdline_parse_token_string_t cmd_config_rss_hash_key_value =
>>>  	TOKEN_STRING_INITIALIZER(struct cmd_config_rss_hash_key, key, NULL);
>>>
>>> @@ -2451,7 +2456,8 @@ struct cmd_config_rss_hash_key {
>>>  		"l2-payload|ipv6-ex|ipv6-tcp-ex|ipv6-udp-ex|"
>>>  		"l3-src-only|l3-dst-only|l4-src-only|l4-dst-only|"
>>>  		"l2-src-only|l2-dst-only|s-vlan|c-vlan|"
>>> -		"l2tpv3|esp|ah|pfcp|pppoe|gtpu|ecpri|mpls "
>>> +		"l2tpv3|esp|ah|pfcp|pppoe|gtpu|ecpri|mpls|"
>>> +		"ipv4-chksum|l4-chksum "
>>>  		"<string of hex digits (variable length, NIC dependent)>",
>>>  	.tokens = {
>>>  		(void *)&cmd_config_rss_hash_key_port, diff --git
>>> a/app/test-pmd/config.c b/app/test-pmd/config.c index 43c79b5..14968bf
>>> 100644
>>> --- a/app/test-pmd/config.c
>>> +++ b/app/test-pmd/config.c
>>> @@ -140,6 +140,8 @@
>>>  	{ "gtpu", ETH_RSS_GTPU },
>>>  	{ "ecpri", ETH_RSS_ECPRI },
>>>  	{ "mpls", ETH_RSS_MPLS },
>>> +	{ "ipv4-chksum", ETH_RSS_IPV4_CHKSUM },
>>> +	{ "l4-chksum", ETH_RSS_L4_CHKSUM },
>>>  	{ NULL, 0 },
>>>  };
>>>
>>> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h index
>>> faf3bd9..63b0321 100644
>>> --- a/lib/ethdev/rte_ethdev.h
>>> +++ b/lib/ethdev/rte_ethdev.h
>>> @@ -537,6 +537,8 @@ struct rte_eth_rss_conf {
>>>  #define ETH_RSS_PPPOE		   (1ULL << 31)
>>>  #define ETH_RSS_ECPRI		   (1ULL << 32)
>>>  #define ETH_RSS_MPLS		   (1ULL << 33)
>>> +#define ETH_RSS_IPV4_CHKSUM	   (1ULL << 34)
>>> +#define ETH_RSS_L4_CHKSUM	   (1ULL << 35)	/* TCP/UDP/SCTP */
>>
>> It does not reply on my questions at all.

Above list of Layer 4 protocols does not say if the flag may
be reported when all above protocols are supported or some
protocols are supported only.

Also I had a question about UDP checksum 0 since it could be
treated in a different ways since logically it is no checksum
at all.

> You have said:       
>>>> I don't think so. IMHO, it should report all RSS capabilities      
>>>> regardless generic vs flow API RSS action.       
>>>      
>> May be it is OK to report subset in      
>> dev_info->flow_type_rss_offloads, but I'm very uncomfortable with the       
>> approach. Superset sounds more logical to me, but has drawbacks as        
>> well.         
> 
> Here I have another question:              
> There are flow type definition and RSS offload type definition,         
> #define RTE_ETH_FLOW_RAW                 1       
> #define RTE_ETH_FLOW_IPV4                2     
> #define RTE_ETH_FLOW_FRAG_IPV4           3       
> #define RTE_ETH_FLOW_NONFRAG_IPV4_TCP    4        
> 
> #define ETH_RSS_IPV4               (1ULL << 2)      
> #define ETH_RSS_FRAG_IPV4          (1ULL << 3)         
> #define ETH_RSS_NONFRAG_IPV4_TCP   (1ULL << 4)        
>   
> are they the different expressions of the same concept?

Sorry, but I don't understand the question.

> 
> If yes, why they have been decoupled by the commit: fce6b66893.

Do you mean that motivation in the changeset description is not
clear?

> Then what the flow type of ETH_RSS_PORT, it's UDP, TCP, or SCTP?      

I have no answer to the question as well. That's why I'm trying
to avoid it in the patch.

> 
> If not, the PMDs can report supported RSS flow type by dev_info->flow_type_rss_offloads,   
> but have no way to report supported RSS offload types.          

Sorry, don't understand.

>>
>> Nack

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

* Re: [dpdk-dev] [PATCH v4] ethdev: add IPv4 and L4 checksum RSS offload types
  2021-07-13 10:24           ` Andrew Rybchenko
@ 2021-07-14  2:38             ` Zhang, AlvinX
  0 siblings, 0 replies; 39+ messages in thread
From: Zhang, AlvinX @ 2021-07-14  2:38 UTC (permalink / raw)
  To: Andrew Rybchenko, Zhang, Qi Z, ajit.khaparde; +Cc: dev, Singh, Aman Deep

> -----Original Message-----
> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Sent: Tuesday, July 13, 2021 6:24 PM
> To: Zhang, AlvinX <alvinx.zhang@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>; ajit.khaparde@broadcom.com
> Cc: dev@dpdk.org; Singh, Aman Deep <aman.deep.singh@intel.com>
> Subject: Re: [PATCH v4] ethdev: add IPv4 and L4 checksum RSS offload types
> 
> On 7/13/21 12:38 PM, Zhang, AlvinX wrote:
> >> -----Original Message-----
> >> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> >> Sent: Tuesday, July 13, 2021 3:55 PM
> >> To: Zhang, AlvinX <alvinx.zhang@intel.com>; Zhang, Qi Z
> >> <qi.z.zhang@intel.com>; ajit.khaparde@broadcom.com
> >> Cc: dev@dpdk.org; Singh, Aman Deep <aman.deep.singh@intel.com>
> >> Subject: Re: [PATCH v4] ethdev: add IPv4 and L4 checksum RSS offload
> >> types
> >>
> >> On 7/13/21 4:13 AM, Alvin Zhang wrote:
> >>> This patch defines new RSS offload types for IPv4 and
> >>> L4(TCP/UDP/SCTP) checksum, which are required when users want to
> >>> distribute packets based on the IPv4 or L4 checksum field.
> >>>
> >>> For example "flow create 0 ingress pattern eth / ipv4 / end actions
> >>> rss types ipv4-chksum end queues end / end", this flow causes all
> >>> matching packets to be distributed to queues on basis of IPv4
> >>> checksum.
> >>>
> >>> Signed-off-by: Alvin Zhang <alvinx.zhang@intel.com>
> >>> Reviewed-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> >>
> >> I've failed to find mail where I've added the tag. I've replied with
> >> "LGTM" for v2 which has IPv4 checksum only and many comments on L4
> checksum added in v3.
> >> So, I think it is incorrect to inherit "LGTM" as Reviewed-by.
> >>
> >
> > I'll correct it at next version.
> >
> >>> Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> >>> Acked-by: Aman Deep Singh <aman.deep.singh@intel.com>
> >>> ---
> >>>
> >>> v3: Add L4 checksum RSS offload type
> >>> v4: Add doc and help string, update commit log
> >>> ---
> >>>  app/test-pmd/cmdline.c  | 12 +++++++++---
> >>>  app/test-pmd/config.c   |  2 ++
> >>>  lib/ethdev/rte_ethdev.h |  2 ++
> >>>  3 files changed, 13 insertions(+), 3 deletions(-)
> >>>
> >>> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index
> >>> 0268b18..93543d8 100644
> >>> --- a/app/test-pmd/cmdline.c
> >>> +++ b/app/test-pmd/cmdline.c
> >>> @@ -2254,6 +2254,10 @@ struct cmd_config_rss {
> >>>  		rss_conf.rss_hf = ETH_RSS_ECPRI;
> >>>  	else if (!strcmp(res->value, "mpls"))
> >>>  		rss_conf.rss_hf = ETH_RSS_MPLS;
> >>> +	else if (!strcmp(res->value, "ipv4-chksum"))
> >>> +		rss_conf.rss_hf = ETH_RSS_IPV4_CHKSUM;
> >>> +	else if (!strcmp(res->value, "l4-chksum"))
> >>> +		rss_conf.rss_hf = ETH_RSS_L4_CHKSUM;
> >>>  	else if (!strcmp(res->value, "none"))
> >>>  		rss_conf.rss_hf = 0;
> >>>  	else if (!strcmp(res->value, "level-default")) { @@ -2325,7
> >>> +2329,7 @@ struct cmd_config_rss {
> >>>  	.help_str = "port config all rss "
> >>>
> 	"all|default|eth|vlan|ip|tcp|udp|sctp|ether|port|vxlan|geneve|"
> >>>
> >> 	"nvgre|vxlan-gpe|l2tpv3|esp|ah|pfcp|ecpri|mpls|none|level-default|"
> >>> -		"level-outer|level-inner|<flowtype_id>",
> >>> +		"level-outer|level-inner|ipv4-chksum|l4-chksum|<flowtype_id>",
> >>>  	.tokens = {
> >>>  		(void *)&cmd_config_rss_port,
> >>>  		(void *)&cmd_config_rss_keyword,
> >>> @@ -2438,7 +2442,8 @@ struct cmd_config_rss_hash_key {
> >>>  				 "ipv6-tcp-ex#ipv6-udp-ex#"
> >>>  				 "l3-src-only#l3-dst-only#l4-src-only#l4-dst-only#"
> >>>  				 "l2-src-only#l2-dst-only#s-vlan#c-vlan#"
> >>> -				 "l2tpv3#esp#ah#pfcp#pppoe#gtpu#ecpri#mpls");
> >>> +				 "l2tpv3#esp#ah#pfcp#pppoe#gtpu#ecpri#mpls#"
> >>> +				 "ipv4-chksum#l4-chksum");
> >>>  cmdline_parse_token_string_t cmd_config_rss_hash_key_value =
> >>>  	TOKEN_STRING_INITIALIZER(struct cmd_config_rss_hash_key, key,
> >>> NULL);
> >>>
> >>> @@ -2451,7 +2456,8 @@ struct cmd_config_rss_hash_key {
> >>>  		"l2-payload|ipv6-ex|ipv6-tcp-ex|ipv6-udp-ex|"
> >>>  		"l3-src-only|l3-dst-only|l4-src-only|l4-dst-only|"
> >>>  		"l2-src-only|l2-dst-only|s-vlan|c-vlan|"
> >>> -		"l2tpv3|esp|ah|pfcp|pppoe|gtpu|ecpri|mpls "
> >>> +		"l2tpv3|esp|ah|pfcp|pppoe|gtpu|ecpri|mpls|"
> >>> +		"ipv4-chksum|l4-chksum "
> >>>  		"<string of hex digits (variable length, NIC dependent)>",
> >>>  	.tokens = {
> >>>  		(void *)&cmd_config_rss_hash_key_port, diff --git
> >>> a/app/test-pmd/config.c b/app/test-pmd/config.c index
> >>> 43c79b5..14968bf
> >>> 100644
> >>> --- a/app/test-pmd/config.c
> >>> +++ b/app/test-pmd/config.c
> >>> @@ -140,6 +140,8 @@
> >>>  	{ "gtpu", ETH_RSS_GTPU },
> >>>  	{ "ecpri", ETH_RSS_ECPRI },
> >>>  	{ "mpls", ETH_RSS_MPLS },
> >>> +	{ "ipv4-chksum", ETH_RSS_IPV4_CHKSUM },
> >>> +	{ "l4-chksum", ETH_RSS_L4_CHKSUM },
> >>>  	{ NULL, 0 },
> >>>  };
> >>>
> >>> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h index
> >>> faf3bd9..63b0321 100644
> >>> --- a/lib/ethdev/rte_ethdev.h
> >>> +++ b/lib/ethdev/rte_ethdev.h
> >>> @@ -537,6 +537,8 @@ struct rte_eth_rss_conf {
> >>>  #define ETH_RSS_PPPOE		   (1ULL << 31)
> >>>  #define ETH_RSS_ECPRI		   (1ULL << 32)
> >>>  #define ETH_RSS_MPLS		   (1ULL << 33)
> >>> +#define ETH_RSS_IPV4_CHKSUM	   (1ULL << 34)
> >>> +#define ETH_RSS_L4_CHKSUM	   (1ULL << 35)	/* TCP/UDP/SCTP */
> >>
> >> It does not reply on my questions at all.
> 
> Above list of Layer 4 protocols does not say if the flag may be reported when all
> above protocols are supported or some protocols are supported only.
> 
> Also I had a question about UDP checksum 0 since it could be treated in a
> different ways since logically it is no checksum at all.

I'll add comment for UDP checksum in next patch.   
Thanks.   

> 
> > You have said:
> >>>> I don't think so. IMHO, it should report all RSS capabilities
> >>>> regardless generic vs flow API RSS action.
> >>>
> >> May be it is OK to report subset in
> >> dev_info->flow_type_rss_offloads, but I'm very uncomfortable with the
> >> approach. Superset sounds more logical to me, but has drawbacks as
> >> well.
> >
> > Here I have another question:
> > There are flow type definition and RSS offload type definition,
> > #define RTE_ETH_FLOW_RAW                 1
> > #define RTE_ETH_FLOW_IPV4                2
> > #define RTE_ETH_FLOW_FRAG_IPV4           3
> > #define RTE_ETH_FLOW_NONFRAG_IPV4_TCP    4
> >
> > #define ETH_RSS_IPV4               (1ULL << 2)
> > #define ETH_RSS_FRAG_IPV4          (1ULL << 3)
> > #define ETH_RSS_NONFRAG_IPV4_TCP   (1ULL << 4)
> >
> > are they the different expressions of the same concept?
> 
> Sorry, but I don't understand the question.
> 
> >
> > If yes, why they have been decoupled by the commit: fce6b66893.
> 
> Do you mean that motivation in the changeset description is not clear?
> 
> > Then what the flow type of ETH_RSS_PORT, it's UDP, TCP, or SCTP?
> 
> I have no answer to the question as well. That's why I'm trying to avoid it in the
> patch.

IMHO, the flow type is different from RSS Offload type.  
That's why they have been decoupled in commit fce6b66893   
    ethdev: decouple flow types and RSS offload types   

    This patch decouples RTE_ETH_FLOW_* and ETH_RSS_*. The former defines   
    flow types and the latter defines RSS offload types.   

There are total 25 flow types, but 30+ RSS offload types.  
The PMDs report flow type in dev_info->flow_type_rss_offloads, but they don't report RSS offload type.   

> 
> >
> > If not, the PMDs can report supported RSS flow type by
> dev_info->flow_type_rss_offloads,
> > but have no way to report supported RSS offload types.
> 
> Sorry, don't understand.
> 
> >>
> >> Nack

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

* [dpdk-dev] [PATCH v5] ethdev: add IPv4 and L4 checksum RSS offload types
  2021-07-13  1:13     ` [dpdk-dev] [PATCH v4] " Alvin Zhang
  2021-07-13  7:54       ` Andrew Rybchenko
@ 2021-08-18  2:32       ` Alvin Zhang
  2021-08-29 12:07         ` Zhang, Qi Z
  2021-08-31  9:44         ` [dpdk-dev] [PATCH v6] " Alvin Zhang
  1 sibling, 2 replies; 39+ messages in thread
From: Alvin Zhang @ 2021-08-18  2:32 UTC (permalink / raw)
  To: qi.z.zhang, junfeng.guo; +Cc: dev, Alvin Zhang, Ajit Khaparde, Aman Deep Singh

This patch defines new RSS offload types for IPv4 and
L4(TCP/UDP/SCTP) checksum, which are required when users want
to distribute packets based on the IPv4 or L4 checksum field.

For example "flow create 0 ingress pattern eth / ipv4 / end
actions rss types ipv4-chksum end queues end / end", this flow
causes all matching packets to be distributed to queues on
basis of IPv4 checksum.

Signed-off-by: Alvin Zhang <alvinx.zhang@intel.com>
Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
Acked-by: Aman Deep Singh <aman.deep.singh@intel.com>
---

v5: Add release note and code commands.
---
 app/test-pmd/cmdline.c                 |  4 ++-
 app/test-pmd/config.c                  |  2 ++
 doc/guides/rel_notes/release_21_11.rst | 61 ++++++++++++++++++++++++++++++++++
 lib/ethdev/rte_ethdev.h                | 23 +++++++++++++
 4 files changed, 89 insertions(+), 1 deletion(-)
 create mode 100644 doc/guides/rel_notes/release_21_11.rst

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 82253bc..656a311 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -2252,6 +2252,8 @@ struct cmd_config_rss {
 		rss_conf.rss_hf = ETH_RSS_ECPRI;
 	else if (!strcmp(res->value, "mpls"))
 		rss_conf.rss_hf = ETH_RSS_MPLS;
+	else if (!strcmp(res->value, "ipv4-chksum"))
+		rss_conf.rss_hf = ETH_RSS_IPV4_CHKSUM;
 	else if (!strcmp(res->value, "none"))
 		rss_conf.rss_hf = 0;
 	else if (!strcmp(res->value, "level-default")) {
@@ -2323,7 +2325,7 @@ struct cmd_config_rss {
 	.help_str = "port config all rss "
 		"all|default|eth|vlan|ip|tcp|udp|sctp|ether|port|vxlan|geneve|"
 		"nvgre|vxlan-gpe|l2tpv3|esp|ah|pfcp|ecpri|mpls|none|level-default|"
-		"level-outer|level-inner|<flowtype_id>",
+		"level-outer|level-inner|ipv4-chksum|<flowtype_id>",
 	.tokens = {
 		(void *)&cmd_config_rss_port,
 		(void *)&cmd_config_rss_keyword,
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 31d8ba1..ece78f2 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -140,6 +140,8 @@
 	{ "gtpu", ETH_RSS_GTPU },
 	{ "ecpri", ETH_RSS_ECPRI },
 	{ "mpls", ETH_RSS_MPLS },
+	{ "ipv4-chksum", ETH_RSS_IPV4_CHKSUM },
+	{ "l4-chksum", ETH_RSS_L4_CHKSUM },
 	{ NULL, 0 },
 };
 
diff --git a/doc/guides/rel_notes/release_21_11.rst b/doc/guides/rel_notes/release_21_11.rst
new file mode 100644
index 0000000..1017550
--- /dev/null
+++ b/doc/guides/rel_notes/release_21_11.rst
@@ -0,0 +1,61 @@
+.. SPDX-License-Identifier: BSD-3-Clause
+   Copyright 2021 The DPDK contributors
+
+.. include:: <isonum.txt>
+
+DPDK Release 21.11
+==================
+
+.. **Read this first.**
+
+   The text in the sections below explains how to update the release notes.
+
+   Use proper spelling, capitalization and punctuation in all sections.
+
+   Variable and config names should be quoted as fixed width text:
+   ``LIKE_THIS``.
+
+   Build the docs and view the output file to ensure the changes are correct::
+
+      make doc-guides-html
+      xdg-open build/doc/html/guides/rel_notes/release_21_11.html
+
+
+New Features
+------------
+
+.. This section should contain new features added in this release.
+   Sample format:
+
+   * **Add a title in the past tense with a full stop.**
+
+     Add a short 1-2 sentence description in the past tense.
+     The description should be enough to allow someone scanning
+     the release notes to understand the new feature.
+
+     If the feature adds a lot of sub-features you can use a bullet list
+     like this:
+
+     * Added feature foo to do something.
+     * Enhanced feature bar to do something else.
+
+     Refer to the previous release notes for examples.
+
+     Suggested order in release notes items:
+     * Core libs (EAL, mempool, ring, mbuf, buses)
+     * Device abstraction libs and PMDs (ordered alphabetically by vendor name)
+       - ethdev (lib, PMDs)
+       - cryptodev (lib, PMDs)
+       - eventdev (lib, PMDs)
+       - etc
+     * Other libs
+     * Apps, Examples, Tools (if significant)
+
+     This section is a comment. Do not overwrite or remove it.
+     Also, make sure to start the actual text at the margin.
+     =======================================================
+
+* **Add new RSS offload types for IPv4/L4 checksum in RSS flow.**
+
+     Add macros ETH_RSS_IPV4_CHKSUM and ETH_RSS_L4_CHKSUM, now IPv4 and
+     TCP/UDP/SCTP header checksum field can be used as input set for RSS.
diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index d2b27c3..9a59e7b 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -537,6 +537,29 @@ struct rte_eth_rss_conf {
 #define ETH_RSS_PPPOE		   (1ULL << 31)
 #define ETH_RSS_ECPRI		   (1ULL << 32)
 #define ETH_RSS_MPLS		   (1ULL << 33)
+#define ETH_RSS_IPV4_CHKSUM	   (1ULL << 34)
+
+/**
+ * The ETH_RSS_L4_CHKSUM generally refers to a type of checksum field for
+ * any L4 header, such as TCP, UDP and SCTP. It is similar to ETH_RSS_PORT,
+ * it does not specify the type of L4 header.
+ * We use this macro to replace below macro for constricting the use of RSS
+ * offload bits:
+ * ETH_RSS_IPV4_TCP_CHKSUM
+ * ETH_RSS_IPV4_UDP_CHKSUM
+ * ETH_RSS_IPV4_SCTP_CHKSUM
+ * ETH_RSS_IPV6_TCP_CHKSUM
+ * ETH_RSS_IPV6_UDP_CHKSUM
+ * ETH_RSS_IPV6_SCTP_CHKSUM
+ *
+ * Then how to use this macro? We can use it in RSS flow where the pattern
+ * type will specify the L4 header type, for example "flow create 0 ingress \
+ * pattern eth / ipv4 / tcp / end actions rss types l4-chksum  end queues end \
+ * / end"
+ * For UDP, the checksum is not required, if the checksum is 0, it will be
+ * treated as a regular checksum with value equal to 0.
+ */
+#define ETH_RSS_L4_CHKSUM	   (1ULL << 35)
 
 /*
  * We use the following macros to combine with above ETH_RSS_* for
-- 
1.8.3.1


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

* Re: [dpdk-dev] [PATCH v5] ethdev: add IPv4 and L4 checksum RSS offload types
  2021-08-18  2:32       ` [dpdk-dev] [PATCH v5] " Alvin Zhang
@ 2021-08-29 12:07         ` Zhang, Qi Z
  2021-08-31  9:44         ` [dpdk-dev] [PATCH v6] " Alvin Zhang
  1 sibling, 0 replies; 39+ messages in thread
From: Zhang, Qi Z @ 2021-08-29 12:07 UTC (permalink / raw)
  To: Zhang, AlvinX, Guo, Junfeng; +Cc: dev, Ajit Khaparde, Singh, Aman Deep



> -----Original Message-----
> From: Zhang, AlvinX <alvinx.zhang@intel.com>
> Sent: Wednesday, August 18, 2021 10:32 AM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Guo, Junfeng
> <junfeng.guo@intel.com>
> Cc: dev@dpdk.org; Zhang, AlvinX <alvinx.zhang@intel.com>; Ajit Khaparde
> <ajit.khaparde@broadcom.com>; Singh, Aman Deep
> <aman.deep.singh@intel.com>
> Subject: [PATCH v5] ethdev: add IPv4 and L4 checksum RSS offload types
> 
> This patch defines new RSS offload types for IPv4 and
> L4(TCP/UDP/SCTP) checksum, which are required when users want to
> distribute packets based on the IPv4 or L4 checksum field.
> 
> For example "flow create 0 ingress pattern eth / ipv4 / end actions rss types
> ipv4-chksum end queues end / end", this flow causes all matching packets to be
> distributed to queues on basis of IPv4 checksum.
> 
> Signed-off-by: Alvin Zhang <alvinx.zhang@intel.com>
> Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> Acked-by: Aman Deep Singh <aman.deep.singh@intel.com>
> ---
> 
> v5: Add release note and code commands.
> ---
>  app/test-pmd/cmdline.c                 |  4 ++-
>  app/test-pmd/config.c                  |  2 ++
>  doc/guides/rel_notes/release_21_11.rst | 61
> ++++++++++++++++++++++++++++++++++
>  lib/ethdev/rte_ethdev.h                | 23 +++++++++++++
>  4 files changed, 89 insertions(+), 1 deletion(-)  create mode 100644
> doc/guides/rel_notes/release_21_11.rst
> 
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index
> 82253bc..656a311 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -2252,6 +2252,8 @@ struct cmd_config_rss {
>  		rss_conf.rss_hf = ETH_RSS_ECPRI;
>  	else if (!strcmp(res->value, "mpls"))
>  		rss_conf.rss_hf = ETH_RSS_MPLS;
> +	else if (!strcmp(res->value, "ipv4-chksum"))
> +		rss_conf.rss_hf = ETH_RSS_IPV4_CHKSUM;
>  	else if (!strcmp(res->value, "none"))
>  		rss_conf.rss_hf = 0;
>  	else if (!strcmp(res->value, "level-default")) { @@ -2323,7 +2325,7 @@
> struct cmd_config_rss {
>  	.help_str = "port config all rss "
>  		"all|default|eth|vlan|ip|tcp|udp|sctp|ether|port|vxlan|geneve|"
> 
> 	"nvgre|vxlan-gpe|l2tpv3|esp|ah|pfcp|ecpri|mpls|none|level-default|"
> -		"level-outer|level-inner|<flowtype_id>",
> +		"level-outer|level-inner|ipv4-chksum|<flowtype_id>",
>  	.tokens = {
>  		(void *)&cmd_config_rss_port,
>  		(void *)&cmd_config_rss_keyword,
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index
> 31d8ba1..ece78f2 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -140,6 +140,8 @@
>  	{ "gtpu", ETH_RSS_GTPU },
>  	{ "ecpri", ETH_RSS_ECPRI },
>  	{ "mpls", ETH_RSS_MPLS },
> +	{ "ipv4-chksum", ETH_RSS_IPV4_CHKSUM },
> +	{ "l4-chksum", ETH_RSS_L4_CHKSUM },
>  	{ NULL, 0 },
>  };
> 
> diff --git a/doc/guides/rel_notes/release_21_11.rst
> b/doc/guides/rel_notes/release_21_11.rst
> new file mode 100644
> index 0000000..1017550
> --- /dev/null
> +++ b/doc/guides/rel_notes/release_21_11.rst
> @@ -0,0 +1,61 @@
> +.. SPDX-License-Identifier: BSD-3-Clause
> +   Copyright 2021 The DPDK contributors
> +
> +.. include:: <isonum.txt>
> +
> +DPDK Release 21.11
> +==================
> +
> +.. **Read this first.**
> +
> +   The text in the sections below explains how to update the release notes.
> +
> +   Use proper spelling, capitalization and punctuation in all sections.
> +
> +   Variable and config names should be quoted as fixed width text:
> +   ``LIKE_THIS``.
> +
> +   Build the docs and view the output file to ensure the changes are correct::
> +
> +      make doc-guides-html
> +      xdg-open build/doc/html/guides/rel_notes/release_21_11.html
> +
> +
> +New Features
> +------------
> +
> +.. This section should contain new features added in this release.
> +   Sample format:
> +
> +   * **Add a title in the past tense with a full stop.**
> +
> +     Add a short 1-2 sentence description in the past tense.
> +     The description should be enough to allow someone scanning
> +     the release notes to understand the new feature.
> +
> +     If the feature adds a lot of sub-features you can use a bullet list
> +     like this:
> +
> +     * Added feature foo to do something.
> +     * Enhanced feature bar to do something else.
> +
> +     Refer to the previous release notes for examples.
> +
> +     Suggested order in release notes items:
> +     * Core libs (EAL, mempool, ring, mbuf, buses)
> +     * Device abstraction libs and PMDs (ordered alphabetically by vendor
> name)
> +       - ethdev (lib, PMDs)
> +       - cryptodev (lib, PMDs)
> +       - eventdev (lib, PMDs)
> +       - etc
> +     * Other libs
> +     * Apps, Examples, Tools (if significant)
> +
> +     This section is a comment. Do not overwrite or remove it.
> +     Also, make sure to start the actual text at the margin.
> +     =======================================================
> +
> +* **Add new RSS offload types for IPv4/L4 checksum in RSS flow.**
> +
> +     Add macros ETH_RSS_IPV4_CHKSUM and ETH_RSS_L4_CHKSUM, now
> IPv4 and
> +     TCP/UDP/SCTP header checksum field can be used as input set for RSS.
> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h index
> d2b27c3..9a59e7b 100644
> --- a/lib/ethdev/rte_ethdev.h
> +++ b/lib/ethdev/rte_ethdev.h
> @@ -537,6 +537,29 @@ struct rte_eth_rss_conf {
>  #define ETH_RSS_PPPOE		   (1ULL << 31)
>  #define ETH_RSS_ECPRI		   (1ULL << 32)
>  #define ETH_RSS_MPLS		   (1ULL << 33)
> +#define ETH_RSS_IPV4_CHKSUM	   (1ULL << 34)
> +
> +/**
> + * The ETH_RSS_L4_CHKSUM generally refers to a type of checksum field
> +for
> + * any L4 header, such as TCP, UDP and SCTP. It is similar to
> +ETH_RSS_PORT,
> + * it does not specify the type of L4 header.
> + * We use this macro to replace below macro for constricting the use of
> +RSS
> + * offload bits:
> + * ETH_RSS_IPV4_TCP_CHKSUM
> + * ETH_RSS_IPV4_UDP_CHKSUM
> + * ETH_RSS_IPV4_SCTP_CHKSUM
> + * ETH_RSS_IPV6_TCP_CHKSUM
> + * ETH_RSS_IPV6_UDP_CHKSUM
> + * ETH_RSS_IPV6_SCTP_CHKSUM
> + *
> + * Then how to use this macro? We can use it in RSS flow where the
> +pattern
> + * type will specify the L4 header type, for example "flow create 0
> +ingress \
> + * pattern eth / ipv4 / tcp / end actions rss types l4-chksum  end
> +queues end \
> + * / end"
> + * For UDP, the checksum is not required, if the checksum is 0, it will

Do you mean
For UDP , the checksum is optional, if the checksum is not used, it will...?

> +be
> + * treated as a regular checksum with value equal to 0.
> + */

Or maybe just reword to below
For the case that checksum is not used in a UDP header, it take the reserved value 0 as input for the hash function.

> +#define ETH_RSS_L4_CHKSUM	   (1ULL << 35)
> 
>  /*
>   * We use the following macros to combine with above ETH_RSS_* for
> --
> 1.8.3.1


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

* [dpdk-dev] [PATCH v6] ethdev: add IPv4 and L4 checksum RSS offload types
  2021-08-18  2:32       ` [dpdk-dev] [PATCH v5] " Alvin Zhang
  2021-08-29 12:07         ` Zhang, Qi Z
@ 2021-08-31  9:44         ` Alvin Zhang
  2021-08-31  9:52           ` [dpdk-dev] [PATCH v7] " Alvin Zhang
  1 sibling, 1 reply; 39+ messages in thread
From: Alvin Zhang @ 2021-08-31  9:44 UTC (permalink / raw)
  To: qi.z.zhang, junfeng.guo; +Cc: dev, Alvin Zhang, Ajit Khaparde, Aman Deep Singh

This patch defines new RSS offload types for IPv4 and
L4(TCP/UDP/SCTP) checksum, which are required when users want
to distribute packets based on the IPv4 or L4 checksum field.

For example "flow create 0 ingress pattern eth / ipv4 / end
actions rss types ipv4-chksum end queues end / end", this flow
causes all matching packets to be distributed to queues on
basis of IPv4 checksum.

Signed-off-by: Alvin Zhang <alvinx.zhang@intel.com>
Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
Acked-by: Aman Deep Singh <aman.deep.singh@intel.com>
---

v6: rebase to eeedef70, update some note
---
 app/test-pmd/cmdline.c                 |  4 +++-
 app/test-pmd/config.c                  |  2 ++
 doc/guides/rel_notes/release_21_11.rst |  4 ++++
 lib/ethdev/rte_ethdev.h                | 24 ++++++++++++++++++++++++
 4 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 82253bc..656a311 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -2252,6 +2252,8 @@ struct cmd_config_rss {
 		rss_conf.rss_hf = ETH_RSS_ECPRI;
 	else if (!strcmp(res->value, "mpls"))
 		rss_conf.rss_hf = ETH_RSS_MPLS;
+	else if (!strcmp(res->value, "ipv4-chksum"))
+		rss_conf.rss_hf = ETH_RSS_IPV4_CHKSUM;
 	else if (!strcmp(res->value, "none"))
 		rss_conf.rss_hf = 0;
 	else if (!strcmp(res->value, "level-default")) {
@@ -2323,7 +2325,7 @@ struct cmd_config_rss {
 	.help_str = "port config all rss "
 		"all|default|eth|vlan|ip|tcp|udp|sctp|ether|port|vxlan|geneve|"
 		"nvgre|vxlan-gpe|l2tpv3|esp|ah|pfcp|ecpri|mpls|none|level-default|"
-		"level-outer|level-inner|<flowtype_id>",
+		"level-outer|level-inner|ipv4-chksum|<flowtype_id>",
 	.tokens = {
 		(void *)&cmd_config_rss_port,
 		(void *)&cmd_config_rss_keyword,
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 31d8ba1..ece78f2 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -140,6 +140,8 @@
 	{ "gtpu", ETH_RSS_GTPU },
 	{ "ecpri", ETH_RSS_ECPRI },
 	{ "mpls", ETH_RSS_MPLS },
+	{ "ipv4-chksum", ETH_RSS_IPV4_CHKSUM },
+	{ "l4-chksum", ETH_RSS_L4_CHKSUM },
 	{ NULL, 0 },
 };
 
diff --git a/doc/guides/rel_notes/release_21_11.rst b/doc/guides/rel_notes/release_21_11.rst
index d707a55..e8a64f5 100644
--- a/doc/guides/rel_notes/release_21_11.rst
+++ b/doc/guides/rel_notes/release_21_11.rst
@@ -55,6 +55,10 @@ New Features
      Also, make sure to start the actual text at the margin.
      =======================================================
 
+* **Add new RSS offload types for IPv4/L4 checksum in RSS flow.**
+  Add macros ETH_RSS_IPV4_CHKSUM and ETH_RSS_L4_CHKSUM, now IPv4 and
+  TCP/UDP/SCTP header checksum field can be used as input set for RSS.
+
 
 Removed Items
 -------------
diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index d2b27c3..e6734df 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -537,6 +537,30 @@ struct rte_eth_rss_conf {
 #define ETH_RSS_PPPOE		   (1ULL << 31)
 #define ETH_RSS_ECPRI		   (1ULL << 32)
 #define ETH_RSS_MPLS		   (1ULL << 33)
+#define ETH_RSS_IPV4_CHKSUM	   (1ULL << 34)
+
+/**
+ * The ETH_RSS_L4_CHKSUM generally refers to a type of checksum field for
+ * any L4 header, such as TCP, UDP and SCTP. It is similar to ETH_RSS_PORT,
+ * it does not specify the type of L4 header.
+ * We use this macro to replace below macro for constricting the use of RSS
+ * offload bits:
+ * ETH_RSS_IPV4_TCP_CHKSUM
+ * ETH_RSS_IPV4_UDP_CHKSUM
+ * ETH_RSS_IPV4_SCTP_CHKSUM
+ * ETH_RSS_IPV6_TCP_CHKSUM
+ * ETH_RSS_IPV6_UDP_CHKSUM
+ * ETH_RSS_IPV6_SCTP_CHKSUM
+ *
+ * Then how to use this macro? We can use it in RSS flow where the pattern
+ * type will specify the L4 header type, for example "flow create 0 ingress \
+ * pattern eth / ipv4 / tcp / end actions rss types l4-chksum  end queues end \
+ * / end"
+ *
+ * For the case that checksum is not used in a UDP header, it takes the
+ * reserved value 0 as input for the hash function.
+ */
+#define ETH_RSS_L4_CHKSUM	   (1ULL << 35)
 
 /*
  * We use the following macros to combine with above ETH_RSS_* for
-- 
1.8.3.1


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

* [dpdk-dev] [PATCH v7] ethdev: add IPv4 and L4 checksum RSS offload types
  2021-08-31  9:44         ` [dpdk-dev] [PATCH v6] " Alvin Zhang
@ 2021-08-31  9:52           ` Alvin Zhang
  2021-09-06  0:28             ` Zhang, Qi Z
                               ` (2 more replies)
  0 siblings, 3 replies; 39+ messages in thread
From: Alvin Zhang @ 2021-08-31  9:52 UTC (permalink / raw)
  To: qi.z.zhang, junfeng.guo; +Cc: dev, Alvin Zhang, Ajit Khaparde, Aman Deep Singh

This patch defines new RSS offload types for IPv4 and
L4(TCP/UDP/SCTP) checksum, which are required when users want
to distribute packets based on the IPv4 or L4 checksum field.

For example "flow create 0 ingress pattern eth / ipv4 / end
actions rss types ipv4-chksum end queues end / end", this flow
causes all matching packets to be distributed to queues on
basis of IPv4 checksum.

Signed-off-by: Alvin Zhang <alvinx.zhang@intel.com>
Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
Acked-by: Aman Deep Singh <aman.deep.singh@intel.com>
---

v6: rebase to eeedef70, update some note
v7: fix code style issues
---
 app/test-pmd/cmdline.c                 |  4 +++-
 app/test-pmd/config.c                  |  2 ++
 doc/guides/rel_notes/release_21_11.rst |  5 +++++
 lib/ethdev/rte_ethdev.h                | 24 ++++++++++++++++++++++++
 4 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 82253bc..656a311 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -2252,6 +2252,8 @@ struct cmd_config_rss {
 		rss_conf.rss_hf = ETH_RSS_ECPRI;
 	else if (!strcmp(res->value, "mpls"))
 		rss_conf.rss_hf = ETH_RSS_MPLS;
+	else if (!strcmp(res->value, "ipv4-chksum"))
+		rss_conf.rss_hf = ETH_RSS_IPV4_CHKSUM;
 	else if (!strcmp(res->value, "none"))
 		rss_conf.rss_hf = 0;
 	else if (!strcmp(res->value, "level-default")) {
@@ -2323,7 +2325,7 @@ struct cmd_config_rss {
 	.help_str = "port config all rss "
 		"all|default|eth|vlan|ip|tcp|udp|sctp|ether|port|vxlan|geneve|"
 		"nvgre|vxlan-gpe|l2tpv3|esp|ah|pfcp|ecpri|mpls|none|level-default|"
-		"level-outer|level-inner|<flowtype_id>",
+		"level-outer|level-inner|ipv4-chksum|<flowtype_id>",
 	.tokens = {
 		(void *)&cmd_config_rss_port,
 		(void *)&cmd_config_rss_keyword,
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 31d8ba1..ece78f2 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -140,6 +140,8 @@
 	{ "gtpu", ETH_RSS_GTPU },
 	{ "ecpri", ETH_RSS_ECPRI },
 	{ "mpls", ETH_RSS_MPLS },
+	{ "ipv4-chksum", ETH_RSS_IPV4_CHKSUM },
+	{ "l4-chksum", ETH_RSS_L4_CHKSUM },
 	{ NULL, 0 },
 };
 
diff --git a/doc/guides/rel_notes/release_21_11.rst b/doc/guides/rel_notes/release_21_11.rst
index d707a55..fa29b13 100644
--- a/doc/guides/rel_notes/release_21_11.rst
+++ b/doc/guides/rel_notes/release_21_11.rst
@@ -55,6 +55,11 @@ New Features
      Also, make sure to start the actual text at the margin.
      =======================================================
 
+* **Add new RSS offload types for IPv4/L4 checksum in RSS flow.**
+
+  Add macros ETH_RSS_IPV4_CHKSUM and ETH_RSS_L4_CHKSUM, now IPv4 and
+  TCP/UDP/SCTP header checksum field can be used as input set for RSS.
+
 
 Removed Items
 -------------
diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index d2b27c3..e6734df 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -537,6 +537,30 @@ struct rte_eth_rss_conf {
 #define ETH_RSS_PPPOE		   (1ULL << 31)
 #define ETH_RSS_ECPRI		   (1ULL << 32)
 #define ETH_RSS_MPLS		   (1ULL << 33)
+#define ETH_RSS_IPV4_CHKSUM	   (1ULL << 34)
+
+/**
+ * The ETH_RSS_L4_CHKSUM generally refers to a type of checksum field for
+ * any L4 header, such as TCP, UDP and SCTP. It is similar to ETH_RSS_PORT,
+ * it does not specify the type of L4 header.
+ * We use this macro to replace below macro for constricting the use of RSS
+ * offload bits:
+ * ETH_RSS_IPV4_TCP_CHKSUM
+ * ETH_RSS_IPV4_UDP_CHKSUM
+ * ETH_RSS_IPV4_SCTP_CHKSUM
+ * ETH_RSS_IPV6_TCP_CHKSUM
+ * ETH_RSS_IPV6_UDP_CHKSUM
+ * ETH_RSS_IPV6_SCTP_CHKSUM
+ *
+ * Then how to use this macro? We can use it in RSS flow where the pattern
+ * type will specify the L4 header type, for example "flow create 0 ingress \
+ * pattern eth / ipv4 / tcp / end actions rss types l4-chksum  end queues end \
+ * / end"
+ *
+ * For the case that checksum is not used in a UDP header, it takes the
+ * reserved value 0 as input for the hash function.
+ */
+#define ETH_RSS_L4_CHKSUM	   (1ULL << 35)
 
 /*
  * We use the following macros to combine with above ETH_RSS_* for
-- 
1.8.3.1


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

* Re: [dpdk-dev] [PATCH v7] ethdev: add IPv4 and L4 checksum RSS offload types
  2021-08-31  9:52           ` [dpdk-dev] [PATCH v7] " Alvin Zhang
@ 2021-09-06  0:28             ` Zhang, Qi Z
  2021-09-14 14:00             ` Ferruh Yigit
  2021-09-15  5:47             ` [dpdk-dev] [PATCH v8] " Alvin Zhang
  2 siblings, 0 replies; 39+ messages in thread
From: Zhang, Qi Z @ 2021-09-06  0:28 UTC (permalink / raw)
  To: Zhang, AlvinX, Guo, Junfeng; +Cc: dev, Ajit Khaparde, Singh, Aman Deep



> -----Original Message-----
> From: Zhang, AlvinX <alvinx.zhang@intel.com>
> Sent: Tuesday, August 31, 2021 5:53 PM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>; Guo, Junfeng
> <junfeng.guo@intel.com>
> Cc: dev@dpdk.org; Zhang, AlvinX <alvinx.zhang@intel.com>; Ajit Khaparde
> <ajit.khaparde@broadcom.com>; Singh, Aman Deep
> <aman.deep.singh@intel.com>
> Subject: [PATCH v7] ethdev: add IPv4 and L4 checksum RSS offload types
> 
> This patch defines new RSS offload types for IPv4 and
> L4(TCP/UDP/SCTP) checksum, which are required when users want to
> distribute packets based on the IPv4 or L4 checksum field.
> 
> For example "flow create 0 ingress pattern eth / ipv4 / end actions rss types
> ipv4-chksum end queues end / end", this flow causes all matching packets to be
> distributed to queues on basis of IPv4 checksum.
> 
> Signed-off-by: Alvin Zhang <alvinx.zhang@intel.com>
> Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> Acked-by: Aman Deep Singh <aman.deep.singh@intel.com>
> ---
> 
> v6: rebase to eeedef70, update some note
> v7: fix code style issues
> ---
>  app/test-pmd/cmdline.c                 |  4 +++-
>  app/test-pmd/config.c                  |  2 ++
>  doc/guides/rel_notes/release_21_11.rst |  5 +++++
>  lib/ethdev/rte_ethdev.h                | 24 ++++++++++++++++++++++++
>  4 files changed, 34 insertions(+), 1 deletion(-)
> 
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index
> 82253bc..656a311 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -2252,6 +2252,8 @@ struct cmd_config_rss {
>  		rss_conf.rss_hf = ETH_RSS_ECPRI;
>  	else if (!strcmp(res->value, "mpls"))
>  		rss_conf.rss_hf = ETH_RSS_MPLS;
> +	else if (!strcmp(res->value, "ipv4-chksum"))
> +		rss_conf.rss_hf = ETH_RSS_IPV4_CHKSUM;
>  	else if (!strcmp(res->value, "none"))
>  		rss_conf.rss_hf = 0;
>  	else if (!strcmp(res->value, "level-default")) { @@ -2323,7 +2325,7 @@
> struct cmd_config_rss {
>  	.help_str = "port config all rss "
>  		"all|default|eth|vlan|ip|tcp|udp|sctp|ether|port|vxlan|geneve|"
> 
> 	"nvgre|vxlan-gpe|l2tpv3|esp|ah|pfcp|ecpri|mpls|none|level-default|"
> -		"level-outer|level-inner|<flowtype_id>",
> +		"level-outer|level-inner|ipv4-chksum|<flowtype_id>",
>  	.tokens = {
>  		(void *)&cmd_config_rss_port,
>  		(void *)&cmd_config_rss_keyword,
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index
> 31d8ba1..ece78f2 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -140,6 +140,8 @@
>  	{ "gtpu", ETH_RSS_GTPU },
>  	{ "ecpri", ETH_RSS_ECPRI },
>  	{ "mpls", ETH_RSS_MPLS },
> +	{ "ipv4-chksum", ETH_RSS_IPV4_CHKSUM },
> +	{ "l4-chksum", ETH_RSS_L4_CHKSUM },
>  	{ NULL, 0 },
>  };
> 
> diff --git a/doc/guides/rel_notes/release_21_11.rst
> b/doc/guides/rel_notes/release_21_11.rst
> index d707a55..fa29b13 100644
> --- a/doc/guides/rel_notes/release_21_11.rst
> +++ b/doc/guides/rel_notes/release_21_11.rst
> @@ -55,6 +55,11 @@ New Features
>       Also, make sure to start the actual text at the margin.
>       =======================================================
> 
> +* **Add new RSS offload types for IPv4/L4 checksum in RSS flow.**
> +
> +  Add macros ETH_RSS_IPV4_CHKSUM and ETH_RSS_L4_CHKSUM, now IPv4
> and
> + TCP/UDP/SCTP header checksum field can be used as input set for RSS.
> +
> 
>  Removed Items
>  -------------
> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h index
> d2b27c3..e6734df 100644
> --- a/lib/ethdev/rte_ethdev.h
> +++ b/lib/ethdev/rte_ethdev.h
> @@ -537,6 +537,30 @@ struct rte_eth_rss_conf {
>  #define ETH_RSS_PPPOE		   (1ULL << 31)
>  #define ETH_RSS_ECPRI		   (1ULL << 32)
>  #define ETH_RSS_MPLS		   (1ULL << 33)
> +#define ETH_RSS_IPV4_CHKSUM	   (1ULL << 34)
> +
> +/**
> + * The ETH_RSS_L4_CHKSUM generally refers to a type of checksum field
> +for
> + * any L4 header, such as TCP, UDP and SCTP. It is similar to
> +ETH_RSS_PORT,
> + * it does not specify the type of L4 header.
> + * We use this macro to replace below macro for constricting the use of
> +RSS
> + * offload bits:
> + * ETH_RSS_IPV4_TCP_CHKSUM
> + * ETH_RSS_IPV4_UDP_CHKSUM
> + * ETH_RSS_IPV4_SCTP_CHKSUM
> + * ETH_RSS_IPV6_TCP_CHKSUM
> + * ETH_RSS_IPV6_UDP_CHKSUM
> + * ETH_RSS_IPV6_SCTP_CHKSUM
> + *
> + * Then how to use this macro? We can use it in RSS flow where the
> +pattern
> + * type will specify the L4 header type, for example "flow create 0
> +ingress \
> + * pattern eth / ipv4 / tcp / end actions rss types l4-chksum  end
> +queues end \
> + * / end"
> + *
> + * For the case that checksum is not used in a UDP header, it takes the
> + * reserved value 0 as input for the hash function.
> + */
> +#define ETH_RSS_L4_CHKSUM	   (1ULL << 35)
> 
>  /*
>   * We use the following macros to combine with above ETH_RSS_* for
> --
> 1.8.3.1

Reviewed-by: Qi Zhang <qi.z.zhang@intel.com>


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

* Re: [dpdk-dev] [PATCH v7] ethdev: add IPv4 and L4 checksum RSS offload types
  2021-08-31  9:52           ` [dpdk-dev] [PATCH v7] " Alvin Zhang
  2021-09-06  0:28             ` Zhang, Qi Z
@ 2021-09-14 14:00             ` Ferruh Yigit
  2021-09-15  1:36               ` Zhang, AlvinX
  2021-09-15  5:47             ` [dpdk-dev] [PATCH v8] " Alvin Zhang
  2 siblings, 1 reply; 39+ messages in thread
From: Ferruh Yigit @ 2021-09-14 14:00 UTC (permalink / raw)
  To: Alvin Zhang, qi.z.zhang, junfeng.guo; +Cc: dev, Ajit Khaparde, Aman Deep Singh

On 8/31/2021 10:52 AM, Alvin Zhang wrote:
> This patch defines new RSS offload types for IPv4 and
> L4(TCP/UDP/SCTP) checksum, which are required when users want
> to distribute packets based on the IPv4 or L4 checksum field.
> 
> For example "flow create 0 ingress pattern eth / ipv4 / end
> actions rss types ipv4-chksum end queues end / end", this flow
> causes all matching packets to be distributed to queues on
> basis of IPv4 checksum.
> 
> Signed-off-by: Alvin Zhang <alvinx.zhang@intel.com>
> Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> Acked-by: Aman Deep Singh <aman.deep.singh@intel.com>
> ---
> 
> v6: rebase to eeedef70, update some note
> v7: fix code style issues
> ---
>  app/test-pmd/cmdline.c                 |  4 +++-
>  app/test-pmd/config.c                  |  2 ++
>  doc/guides/rel_notes/release_21_11.rst |  5 +++++
>  lib/ethdev/rte_ethdev.h                | 24 ++++++++++++++++++++++++
>  4 files changed, 34 insertions(+), 1 deletion(-)
> 
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index 82253bc..656a311 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -2252,6 +2252,8 @@ struct cmd_config_rss {
>  		rss_conf.rss_hf = ETH_RSS_ECPRI;
>  	else if (!strcmp(res->value, "mpls"))
>  		rss_conf.rss_hf = ETH_RSS_MPLS;
> +	else if (!strcmp(res->value, "ipv4-chksum"))
> +		rss_conf.rss_hf = ETH_RSS_IPV4_CHKSUM;
>  	else if (!strcmp(res->value, "none"))
>  		rss_conf.rss_hf = 0;
>  	else if (!strcmp(res->value, "level-default")) {
> @@ -2323,7 +2325,7 @@ struct cmd_config_rss {
>  	.help_str = "port config all rss "
>  		"all|default|eth|vlan|ip|tcp|udp|sctp|ether|port|vxlan|geneve|"
>  		"nvgre|vxlan-gpe|l2tpv3|esp|ah|pfcp|ecpri|mpls|none|level-default|"
> -		"level-outer|level-inner|<flowtype_id>",
> +		"level-outer|level-inner|ipv4-chksum|<flowtype_id>",
>  	.tokens = {
>  		(void *)&cmd_config_rss_port,
>  		(void *)&cmd_config_rss_keyword,
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index 31d8ba1..ece78f2 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -140,6 +140,8 @@
>  	{ "gtpu", ETH_RSS_GTPU },
>  	{ "ecpri", ETH_RSS_ECPRI },
>  	{ "mpls", ETH_RSS_MPLS },
> +	{ "ipv4-chksum", ETH_RSS_IPV4_CHKSUM },
> +	{ "l4-chksum", ETH_RSS_L4_CHKSUM },>  	{ NULL, 0 },
>  };
>  
> diff --git a/doc/guides/rel_notes/release_21_11.rst b/doc/guides/rel_notes/release_21_11.rst
> index d707a55..fa29b13 100644
> --- a/doc/guides/rel_notes/release_21_11.rst
> +++ b/doc/guides/rel_notes/release_21_11.rst
> @@ -55,6 +55,11 @@ New Features
>       Also, make sure to start the actual text at the margin.
>       =======================================================
>  
> +* **Add new RSS offload types for IPv4/L4 checksum in RSS flow.**
> +
> +  Add macros ETH_RSS_IPV4_CHKSUM and ETH_RSS_L4_CHKSUM, now IPv4 and
> +  TCP/UDP/SCTP header checksum field can be used as input set for RSS.
> +
>  
>  Removed Items
>  -------------
> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
> index d2b27c3..e6734df 100644
> --- a/lib/ethdev/rte_ethdev.h
> +++ b/lib/ethdev/rte_ethdev.h
> @@ -537,6 +537,30 @@ struct rte_eth_rss_conf {
>  #define ETH_RSS_PPPOE		   (1ULL << 31)
>  #define ETH_RSS_ECPRI		   (1ULL << 32)
>  #define ETH_RSS_MPLS		   (1ULL << 33)
> +#define ETH_RSS_IPV4_CHKSUM	   (1ULL << 34)
> +
> +/**
> + * The ETH_RSS_L4_CHKSUM generally refers to a type of checksum field for

what does 'generally' means here? Is there a case it refers to something else?

> + * any L4 header, such as TCP, UDP and SCTP. It is similar to ETH_RSS_PORT,
> + * it does not specify the type of L4 header.
> + * We use this macro to replace below macro for constricting the use of RSS
> + * offload bits:
> + * ETH_RSS_IPV4_TCP_CHKSUM
> + * ETH_RSS_IPV4_UDP_CHKSUM
> + * ETH_RSS_IPV4_SCTP_CHKSUM
> + * ETH_RSS_IPV6_TCP_CHKSUM
> + * ETH_RSS_IPV6_UDP_CHKSUM
> + * ETH_RSS_IPV6_SCTP_CHKSUM

As I get you are listing them here to say the 'ETH_RSS_L4_CHKSUM' replaces
possible usage of above list, but my concern is it may confuse people as those
macros exists (or did exist in the past), so what do you think to remove them?


And just to confirm, we can't use this flag, 'ETH_RSS_L4_CHKSUM' anymore with
'rte_eth_rss_conf.rss_hf', right? Since it will be missing some context for it.
Which means some old APIs (and configuration) won't support this new offload,
but only rte_flow will support it.
If above is correct should it be highlighted in above comment?

> + *
> + * Then how to use this macro? We can use it in RSS flow where the pattern

Can we convert this question to a description just to be a little more formal?

> + * type will specify the L4 header type, for example "flow create 0 ingress \
> + * pattern eth / ipv4 / tcp / end actions rss types l4-chksum  end queues end \
> + * / end"
> + *
> + * For the case that checksum is not used in a UDP header, it takes the
> + * reserved value 0 as input for the hash function.
> + */
> +#define ETH_RSS_L4_CHKSUM	   (1ULL << 35)
>  
>  /*
>   * We use the following macros to combine with above ETH_RSS_* for
> 


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

* Re: [dpdk-dev] [PATCH v7] ethdev: add IPv4 and L4 checksum RSS offload types
  2021-09-14 14:00             ` Ferruh Yigit
@ 2021-09-15  1:36               ` Zhang, AlvinX
  0 siblings, 0 replies; 39+ messages in thread
From: Zhang, AlvinX @ 2021-09-15  1:36 UTC (permalink / raw)
  To: Yigit, Ferruh, Zhang, Qi Z, Guo, Junfeng
  Cc: dev, Ajit Khaparde, Singh, Aman Deep

> -----Original Message-----
> From: Yigit, Ferruh <ferruh.yigit@intel.com>
> Sent: Tuesday, September 14, 2021 10:01 PM
> To: Zhang, AlvinX <alvinx.zhang@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>; Guo, Junfeng <junfeng.guo@intel.com>
> Cc: dev@dpdk.org; Ajit Khaparde <ajit.khaparde@broadcom.com>; Singh,
> Aman Deep <aman.deep.singh@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v7] ethdev: add IPv4 and L4 checksum RSS
> offload types
> 
> On 8/31/2021 10:52 AM, Alvin Zhang wrote:
> > This patch defines new RSS offload types for IPv4 and
> > L4(TCP/UDP/SCTP) checksum, which are required when users want to
> > distribute packets based on the IPv4 or L4 checksum field.
> >
> > For example "flow create 0 ingress pattern eth / ipv4 / end actions
> > rss types ipv4-chksum end queues end / end", this flow causes all
> > matching packets to be distributed to queues on basis of IPv4
> > checksum.
> >
> > Signed-off-by: Alvin Zhang <alvinx.zhang@intel.com>
> > Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> > Acked-by: Aman Deep Singh <aman.deep.singh@intel.com>
> > ---
> >
> > v6: rebase to eeedef70, update some note
> > v7: fix code style issues
> > ---
> >  app/test-pmd/cmdline.c                 |  4 +++-
> >  app/test-pmd/config.c                  |  2 ++
> >  doc/guides/rel_notes/release_21_11.rst |  5 +++++
> >  lib/ethdev/rte_ethdev.h                | 24
> ++++++++++++++++++++++++
> >  4 files changed, 34 insertions(+), 1 deletion(-)
> >
> > diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index
> > 82253bc..656a311 100644
> > --- a/app/test-pmd/cmdline.c
> > +++ b/app/test-pmd/cmdline.c
> > @@ -2252,6 +2252,8 @@ struct cmd_config_rss {
> >  		rss_conf.rss_hf = ETH_RSS_ECPRI;
> >  	else if (!strcmp(res->value, "mpls"))
> >  		rss_conf.rss_hf = ETH_RSS_MPLS;
> > +	else if (!strcmp(res->value, "ipv4-chksum"))
> > +		rss_conf.rss_hf = ETH_RSS_IPV4_CHKSUM;
> >  	else if (!strcmp(res->value, "none"))
> >  		rss_conf.rss_hf = 0;
> >  	else if (!strcmp(res->value, "level-default")) { @@ -2323,7 +2325,7
> > @@ struct cmd_config_rss {
> >  	.help_str = "port config all rss "
> >  		"all|default|eth|vlan|ip|tcp|udp|sctp|ether|port|vxlan|geneve|"
> >
> 	"nvgre|vxlan-gpe|l2tpv3|esp|ah|pfcp|ecpri|mpls|none|level-default|"
> > -		"level-outer|level-inner|<flowtype_id>",
> > +		"level-outer|level-inner|ipv4-chksum|<flowtype_id>",
> >  	.tokens = {
> >  		(void *)&cmd_config_rss_port,
> >  		(void *)&cmd_config_rss_keyword,
> > diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index
> > 31d8ba1..ece78f2 100644
> > --- a/app/test-pmd/config.c
> > +++ b/app/test-pmd/config.c
> > @@ -140,6 +140,8 @@
> >  	{ "gtpu", ETH_RSS_GTPU },
> >  	{ "ecpri", ETH_RSS_ECPRI },
> >  	{ "mpls", ETH_RSS_MPLS },
> > +	{ "ipv4-chksum", ETH_RSS_IPV4_CHKSUM },
> > +	{ "l4-chksum", ETH_RSS_L4_CHKSUM },>  	{ NULL, 0 },
> >  };
> >
> > diff --git a/doc/guides/rel_notes/release_21_11.rst
> > b/doc/guides/rel_notes/release_21_11.rst
> > index d707a55..fa29b13 100644
> > --- a/doc/guides/rel_notes/release_21_11.rst
> > +++ b/doc/guides/rel_notes/release_21_11.rst
> > @@ -55,6 +55,11 @@ New Features
> >       Also, make sure to start the actual text at the margin.
> >       =======================================================
> >
> > +* **Add new RSS offload types for IPv4/L4 checksum in RSS flow.**
> > +
> > +  Add macros ETH_RSS_IPV4_CHKSUM and ETH_RSS_L4_CHKSUM, now IPv4
> and
> > + TCP/UDP/SCTP header checksum field can be used as input set for RSS.
> > +
> >
> >  Removed Items
> >  -------------
> > diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h index
> > d2b27c3..e6734df 100644
> > --- a/lib/ethdev/rte_ethdev.h
> > +++ b/lib/ethdev/rte_ethdev.h
> > @@ -537,6 +537,30 @@ struct rte_eth_rss_conf {
> >  #define ETH_RSS_PPPOE		   (1ULL << 31)
> >  #define ETH_RSS_ECPRI		   (1ULL << 32)
> >  #define ETH_RSS_MPLS		   (1ULL << 33)
> > +#define ETH_RSS_IPV4_CHKSUM	   (1ULL << 34)
> > +
> > +/**
> > + * The ETH_RSS_L4_CHKSUM generally refers to a type of checksum field
> > +for
> 
> what does 'generally' means here? Is there a case it refers to something else?
> 
> > + * any L4 header, such as TCP, UDP and SCTP. It is similar to
> > + ETH_RSS_PORT,
> > + * it does not specify the type of L4 header.
> > + * We use this macro to replace below macro for constricting the use
> > + of RSS
> > + * offload bits:
> > + * ETH_RSS_IPV4_TCP_CHKSUM
> > + * ETH_RSS_IPV4_UDP_CHKSUM
> > + * ETH_RSS_IPV4_SCTP_CHKSUM
> > + * ETH_RSS_IPV6_TCP_CHKSUM
> > + * ETH_RSS_IPV6_UDP_CHKSUM
> > + * ETH_RSS_IPV6_SCTP_CHKSUM
> 
> As I get you are listing them here to say the 'ETH_RSS_L4_CHKSUM' replaces
> possible usage of above list, but my concern is it may confuse people as those
> macros exists (or did exist in the past), so what do you think to remove them?
> 
> 
> And just to confirm, we can't use this flag, 'ETH_RSS_L4_CHKSUM' anymore with
> 'rte_eth_rss_conf.rss_hf', right? Since it will be missing some context for it.
> Which means some old APIs (and configuration) won't support this new offload,
> but only rte_flow will support it.
> If above is correct should it be highlighted in above comment?
> 
> > + *
> > + * Then how to use this macro? We can use it in RSS flow where the
> > + pattern
> 
> Can we convert this question to a description just to be a little more formal?
> 
> > + * type will specify the L4 header type, for example "flow create 0
> > +ingress \
> > + * pattern eth / ipv4 / tcp / end actions rss types l4-chksum  end
> > +queues end \
> > + * / end"
> > + *
> > + * For the case that checksum is not used in a UDP header, it takes
> > +the
> > + * reserved value 0 as input for the hash function.
> > + */
> > +#define ETH_RSS_L4_CHKSUM	   (1ULL << 35)
> >
> >  /*
> >   * We use the following macros to combine with above ETH_RSS_* for
> >

I'll refine the notes.
Thanks,
Alvin


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

* [dpdk-dev] [PATCH v8] ethdev: add IPv4 and L4 checksum RSS offload types
  2021-08-31  9:52           ` [dpdk-dev] [PATCH v7] " Alvin Zhang
  2021-09-06  0:28             ` Zhang, Qi Z
  2021-09-14 14:00             ` Ferruh Yigit
@ 2021-09-15  5:47             ` Alvin Zhang
  2021-09-21  8:26               ` Ferruh Yigit
  2021-09-28 13:09               ` Ferruh Yigit
  2 siblings, 2 replies; 39+ messages in thread
From: Alvin Zhang @ 2021-09-15  5:47 UTC (permalink / raw)
  To: qi.z.zhang, ferruh.yigit; +Cc: dev, Alvin Zhang, Ajit Khaparde, Aman Deep Singh

This patch defines new RSS offload types for IPv4 and
L4(TCP/UDP/SCTP) checksum, which are required when users want
to distribute packets based on the IPv4 or L4 checksum field.

For example "flow create 0 ingress pattern eth / ipv4 / end
actions rss types ipv4-chksum end queues end / end", this flow
causes all matching packets to be distributed to queues on
basis of IPv4 checksum.

Signed-off-by: Alvin Zhang <alvinx.zhang@intel.com>
Reviewed-by: Qi Zhang <qi.z.zhang@intel.com>
Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
Acked-by: Aman Deep Singh <aman.deep.singh@intel.com>
---

v6: rebase to eeedef70, update some note
v7: fix code style issues
v8: rebase to 22a9f34e67ea, update some note
---
 app/test-pmd/cmdline.c                 |  4 +++-
 app/test-pmd/config.c                  |  2 ++
 doc/guides/rel_notes/release_21_11.rst |  5 +++++
 lib/ethdev/rte_ethdev.h                | 18 ++++++++++++++++++
 4 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 7dd3965..a9efd02 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -2252,6 +2252,8 @@ struct cmd_config_rss {
 		rss_conf.rss_hf = ETH_RSS_ECPRI;
 	else if (!strcmp(res->value, "mpls"))
 		rss_conf.rss_hf = ETH_RSS_MPLS;
+	else if (!strcmp(res->value, "ipv4-chksum"))
+		rss_conf.rss_hf = ETH_RSS_IPV4_CHKSUM;
 	else if (!strcmp(res->value, "none"))
 		rss_conf.rss_hf = 0;
 	else if (!strcmp(res->value, "level-default")) {
@@ -2323,7 +2325,7 @@ struct cmd_config_rss {
 	.help_str = "port config all rss "
 		"all|default|eth|vlan|ip|tcp|udp|sctp|ether|port|vxlan|geneve|"
 		"nvgre|vxlan-gpe|l2tpv3|esp|ah|pfcp|ecpri|mpls|none|level-default|"
-		"level-outer|level-inner|<flowtype_id>",
+		"level-outer|level-inner|ipv4-chksum|<flowtype_id>",
 	.tokens = {
 		(void *)&cmd_config_rss_port,
 		(void *)&cmd_config_rss_keyword,
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index f5765b3..9c66329 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -140,6 +140,8 @@
 	{ "gtpu", ETH_RSS_GTPU },
 	{ "ecpri", ETH_RSS_ECPRI },
 	{ "mpls", ETH_RSS_MPLS },
+	{ "ipv4-chksum", ETH_RSS_IPV4_CHKSUM },
+	{ "l4-chksum", ETH_RSS_L4_CHKSUM },
 	{ NULL, 0 },
 };
 
diff --git a/doc/guides/rel_notes/release_21_11.rst b/doc/guides/rel_notes/release_21_11.rst
index 43d367b..388e6bd 100644
--- a/doc/guides/rel_notes/release_21_11.rst
+++ b/doc/guides/rel_notes/release_21_11.rst
@@ -87,6 +87,11 @@ New Features
   Added command-line options to specify total number of processes and
   current process ID. Each process owns subset of Rx and Tx queues.
 
+* **Add new RSS offload types for IPv4/L4 checksum in RSS flow.**
+
+  Add macros ETH_RSS_IPV4_CHKSUM and ETH_RSS_L4_CHKSUM, now IPv4 and
+  TCP/UDP/SCTP header checksum field can be used as input set for RSS.
+
 
 Removed Items
 -------------
diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index d2b27c3..d0bbe33 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -537,6 +537,24 @@ struct rte_eth_rss_conf {
 #define ETH_RSS_PPPOE		   (1ULL << 31)
 #define ETH_RSS_ECPRI		   (1ULL << 32)
 #define ETH_RSS_MPLS		   (1ULL << 33)
+#define ETH_RSS_IPV4_CHKSUM	   (1ULL << 34)
+
+/**
+ * The ETH_RSS_L4_CHKSUM works on checksum field of any L4 header.
+ * It is similar to ETH_RSS_PORT that they don't specify the specific type of
+ * L4 header. We define this macro to replace some specific L4 (TCP/UDP/SCTP)
+ * checksum type for constricting the use of RSS offload bits.
+ *
+ * Due to above reason, some old APIs (and configuration) won't support
+ * ETH_RSS_L4_CHKSUM, rte_flow will support it, in a RSS flow the pattern type
+ * can specify the L4 header type, for example
+ * "flow create 0 ingress pattern eth / ipv4 / tcp / end actions rss types \
+ * l4-chksum end queues end / end"
+ *
+ * For the case that checksum is not used in a UDP header, it takes the
+ * reserved value 0 as input for the hash function.
+ */
+#define ETH_RSS_L4_CHKSUM	   (1ULL << 35)
 
 /*
  * We use the following macros to combine with above ETH_RSS_* for
-- 
1.8.3.1


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

* Re: [dpdk-dev] [PATCH v8] ethdev: add IPv4 and L4 checksum RSS offload types
  2021-09-15  5:47             ` [dpdk-dev] [PATCH v8] " Alvin Zhang
@ 2021-09-21  8:26               ` Ferruh Yigit
  2021-09-28 13:09               ` Ferruh Yigit
  1 sibling, 0 replies; 39+ messages in thread
From: Ferruh Yigit @ 2021-09-21  8:26 UTC (permalink / raw)
  To: Alvin Zhang, qi.z.zhang; +Cc: dev, Ajit Khaparde, Aman Deep Singh

On 9/15/2021 6:47 AM, Alvin Zhang wrote:
> This patch defines new RSS offload types for IPv4 and
> L4(TCP/UDP/SCTP) checksum, which are required when users want
> to distribute packets based on the IPv4 or L4 checksum field.
> 
> For example "flow create 0 ingress pattern eth / ipv4 / end
> actions rss types ipv4-chksum end queues end / end", this flow
> causes all matching packets to be distributed to queues on
> basis of IPv4 checksum.
> 
> Signed-off-by: Alvin Zhang <alvinx.zhang@intel.com>
> Reviewed-by: Qi Zhang <qi.z.zhang@intel.com>
> Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> Acked-by: Aman Deep Singh <aman.deep.singh@intel.com>

Acked-by: Ferruh Yigit <ferruh.yigit@intel.com>

Applied to dpdk-next-net/main, thanks.


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

* Re: [dpdk-dev] [PATCH v8] ethdev: add IPv4 and L4 checksum RSS offload types
  2021-09-15  5:47             ` [dpdk-dev] [PATCH v8] " Alvin Zhang
  2021-09-21  8:26               ` Ferruh Yigit
@ 2021-09-28 13:09               ` Ferruh Yigit
  2021-09-29  2:27                 ` Zhang, AlvinX
  1 sibling, 1 reply; 39+ messages in thread
From: Ferruh Yigit @ 2021-09-28 13:09 UTC (permalink / raw)
  To: Alvin Zhang, qi.z.zhang
  Cc: dev, Ajit Khaparde, Aman Deep Singh, Thomas Monjalon

On 9/15/2021 6:47 AM, Alvin Zhang wrote:
> This patch defines new RSS offload types for IPv4 and
> L4(TCP/UDP/SCTP) checksum, which are required when users want
> to distribute packets based on the IPv4 or L4 checksum field.
> 
> For example "flow create 0 ingress pattern eth / ipv4 / end
> actions rss types ipv4-chksum end queues end / end", this flow
> causes all matching packets to be distributed to queues on
> basis of IPv4 checksum.
> 
> Signed-off-by: Alvin Zhang <alvinx.zhang@intel.com>
> Reviewed-by: Qi Zhang <qi.z.zhang@intel.com>
> Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> Acked-by: Aman Deep Singh <aman.deep.singh@intel.com>

<...>

> --- a/doc/guides/rel_notes/release_21_11.rst
> +++ b/doc/guides/rel_notes/release_21_11.rst
> @@ -87,6 +87,11 @@ New Features
>    Added command-line options to specify total number of processes and
>    current process ID. Each process owns subset of Rx and Tx queues.
>  
> +* **Add new RSS offload types for IPv4/L4 checksum in RSS flow.**
> +
> +  Add macros ETH_RSS_IPV4_CHKSUM and ETH_RSS_L4_CHKSUM, now IPv4 and
> +  TCP/UDP/SCTP header checksum field can be used as input set for RSS.
> +
>  

Thomas reported the issue that release notes items should be in past tense,
updated the documentation in next-net, as s/Add/Added/ , fyi.

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

* Re: [dpdk-dev] [PATCH v8] ethdev: add IPv4 and L4 checksum RSS offload types
  2021-09-28 13:09               ` Ferruh Yigit
@ 2021-09-29  2:27                 ` Zhang, AlvinX
  0 siblings, 0 replies; 39+ messages in thread
From: Zhang, AlvinX @ 2021-09-29  2:27 UTC (permalink / raw)
  To: Yigit, Ferruh, Zhang, Qi Z
  Cc: dev, Ajit Khaparde, Singh, Aman Deep, Thomas Monjalon

> -----Original Message-----
> From: Yigit, Ferruh <ferruh.yigit@intel.com>
> Sent: Tuesday, September 28, 2021 9:09 PM
> To: Zhang, AlvinX <alvinx.zhang@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>
> Cc: dev@dpdk.org; Ajit Khaparde <ajit.khaparde@broadcom.com>; Singh,
> Aman Deep <aman.deep.singh@intel.com>; Thomas Monjalon
> <thomas@monjalon.net>
> Subject: Re: [PATCH v8] ethdev: add IPv4 and L4 checksum RSS offload types
> 
> On 9/15/2021 6:47 AM, Alvin Zhang wrote:
> > This patch defines new RSS offload types for IPv4 and
> > L4(TCP/UDP/SCTP) checksum, which are required when users want to
> > distribute packets based on the IPv4 or L4 checksum field.
> >
> > For example "flow create 0 ingress pattern eth / ipv4 / end actions
> > rss types ipv4-chksum end queues end / end", this flow causes all
> > matching packets to be distributed to queues on basis of IPv4
> > checksum.
> >
> > Signed-off-by: Alvin Zhang <alvinx.zhang@intel.com>
> > Reviewed-by: Qi Zhang <qi.z.zhang@intel.com>
> > Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> > Acked-by: Aman Deep Singh <aman.deep.singh@intel.com>
> 
> <...>
> 
> > --- a/doc/guides/rel_notes/release_21_11.rst
> > +++ b/doc/guides/rel_notes/release_21_11.rst
> > @@ -87,6 +87,11 @@ New Features
> >    Added command-line options to specify total number of processes and
> >    current process ID. Each process owns subset of Rx and Tx queues.
> >
> > +* **Add new RSS offload types for IPv4/L4 checksum in RSS flow.**
> > +
> > +  Add macros ETH_RSS_IPV4_CHKSUM and ETH_RSS_L4_CHKSUM, now IPv4
> and
> > + TCP/UDP/SCTP header checksum field can be used as input set for RSS.
> > +
> >
> 
> Thomas reported the issue that release notes items should be in past tense,
> updated the documentation in next-net, as s/Add/Added/ , fyi.

Ok, Thanks.

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

end of thread, other threads:[~2021-09-29  2:27 UTC | newest]

Thread overview: 39+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-03  3:12 [dpdk-dev] [PATCH] ethdev: add RSS offload type for L3 checksum Alvin Zhang
2021-06-03  7:14 ` Andrew Rybchenko
2021-06-03  7:28   ` Zhang, AlvinX
2021-06-03  8:03 ` [dpdk-dev] [PATCH v2] ethdev: add IPv4 checksum RSS offload type Alvin Zhang
2021-06-03  8:17   ` Andrew Rybchenko
2021-06-04  2:23     ` Zhang, AlvinX
2021-06-07 18:31   ` Ajit Khaparde
2021-06-15  8:19   ` [dpdk-dev] [PATCH v3] ethdev: add IPv4 and L4 checksum RSS offload types Alvin Zhang
2021-06-15  8:26     ` Jerin Jacob
2021-06-16 15:18       ` Zhang, Qi Z
2021-06-22  8:20         ` Singh, Aman Deep
2021-07-01 14:26     ` Andrew Rybchenko
2021-07-06  6:14       ` Zhang, AlvinX
2021-07-06  7:05       ` Zhang, AlvinX
2021-07-06  7:18         ` Zhang, Qi Z
2021-07-06  8:04           ` Andrew Rybchenko
2021-07-07  3:23             ` Zhang, Qi Z
2021-07-07  9:49               ` Andrew Rybchenko
2021-07-07 13:00                 ` Zhang, Qi Z
2021-07-07 13:10                   ` Andrew Rybchenko
2021-07-08  1:07                     ` Zhang, Qi Z
2021-07-08  7:45                       ` Andrew Rybchenko
2021-07-10  8:38                         ` Thomas Monjalon
2021-07-13  1:13     ` [dpdk-dev] [PATCH v4] " Alvin Zhang
2021-07-13  7:54       ` Andrew Rybchenko
2021-07-13  9:38         ` Zhang, AlvinX
2021-07-13 10:24           ` Andrew Rybchenko
2021-07-14  2:38             ` Zhang, AlvinX
2021-08-18  2:32       ` [dpdk-dev] [PATCH v5] " Alvin Zhang
2021-08-29 12:07         ` Zhang, Qi Z
2021-08-31  9:44         ` [dpdk-dev] [PATCH v6] " Alvin Zhang
2021-08-31  9:52           ` [dpdk-dev] [PATCH v7] " Alvin Zhang
2021-09-06  0:28             ` Zhang, Qi Z
2021-09-14 14:00             ` Ferruh Yigit
2021-09-15  1:36               ` Zhang, AlvinX
2021-09-15  5:47             ` [dpdk-dev] [PATCH v8] " Alvin Zhang
2021-09-21  8:26               ` Ferruh Yigit
2021-09-28 13:09               ` Ferruh Yigit
2021-09-29  2:27                 ` Zhang, AlvinX

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.