Message ID | 20210713011342.19688-1-alvinx.zhang@intel.com (mailing list archive) |
---|---|
State | Superseded, archived |
Delegated to: | Ferruh Yigit |
Headers | show |
Series | [v4] ethdev: add IPv4 and L4 checksum RSS offload types | expand |
Context | Check | Description |
---|---|---|
ci/iol-intel-Performance | fail | Performance Testing issues |
ci/iol-testing | fail | Testing issues |
ci/iol-abi-testing | warning | Testing issues |
ci/iol-intel-Functional | success | Functional Testing PASS |
ci/intel-Testing | success | Testing PASS |
ci/Intel-compilation | success | Compilation OK |
ci/github-robot | success | github build: passed |
ci/checkpatch | success | coding style OK |
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
> -----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
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
> -----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
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