[dpdk-dev] [RFC] ethdev: reserve the RX offload most-significant bits for PMD scartch

[dpdk-dev] [RFC] ethdev: reserve the RX offload most-significant bits for PMD scartch

[Haiyue Wang]

Generally speaking, the DEV_RX_OFFLOAD_xxx for RX offload capabilities
of a device is one-bit field definition, it has 64 different values at
most.

Nowdays the receiving queue of NIC has rich features not just checksum
offload, like it can extract the network protocol header fields to its
RX descriptors for quickly handling. But this kind of feature is not so
common, and it is hardware related. Normally, this can be done through
rte_devargs driver parameters, but the scope is per device. This is not
so nice for per queue design.

The per queue API 'rte_eth_rx_queue_setup' and data structure 'struct
rte_eth_rxconf' are stable now, and be common for all PMDs. For keeping
the ethdev API & ABI compatibility, and the application can make good
use of the NIC's specific features, reserving the most-significant bits
of RX offload seems an compromise method.

Then the PMDs redefine these bits as they want, all PMDs share the same
bit positions and expose their new definitions with the header file.

The experimental reserved bits number is 6 currently. Tt can be one-bit
for each features up to the the maximum number 6. It can also be some
bits encoding: e.g, 6 bits can stand for 63 maximum number of features.

We call these reserved bits as DEV_RX_OFFLOAD_PMD_SCRATCH. And the left
unused bits number is : 64 - 19 (currently defined) - 6 (PMD scartch) =
39.

This is not so nice for applications, they need to check PMD's driver
name for lookuping their DEV_RX_OFFLOAD_PMD_SCRATCH definitions. But it
is good for the applications to make use of the hardware compatibility.

Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
---
 app/test-pmd/cmdline.c         | 18 ++++++++++++------
 lib/librte_ethdev/rte_ethdev.c |  6 ++++++
 lib/librte_ethdev/rte_ethdev.h | 12 ++++++++++++
 3 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index d1e0d44..ffcc835 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -879,7 +879,8 @@ static void cmd_help_long_parsed(void *parsed_result,
 			"ipv4_cksum|udp_cksum|tcp_cksum|tcp_lro|qinq_strip|"
 			"outer_ipv4_cksum|macsec_strip|header_split|"
 			"vlan_filter|vlan_extend|jumbo_frame|crc_strip|"
-			"scatter|timestamp|security|keep_crc on|off\n"
+			"scatter|timestamp|security|keep_crc|"
+			"pmd_scratch0|pmd_scratch1|pmd_scratch2|pmd_scratch3|pmd_scratch4|pmd_scratch5 on|off\n"
 			"     Enable or disable a per port Rx offloading"
 			" on all Rx queues of a port\n\n"
 
@@ -887,7 +888,8 @@ static void cmd_help_long_parsed(void *parsed_result,
 			"ipv4_cksum|udp_cksum|tcp_cksum|tcp_lro|qinq_strip|"
 			"outer_ipv4_cksum|macsec_strip|header_split|"
 			"vlan_filter|vlan_extend|jumbo_frame|crc_strip|"
-			"scatter|timestamp|security|keep_crc on|off\n"
+			"scatter|timestamp|security|keep_crc|"
+			"pmd_scratch0|pmd_scratch1|pmd_scratch2|pmd_scratch3|pmd_scratch4|pmd_scratch5 on|off\n"
 			"    Enable or disable a per queue Rx offloading"
 			" only on a specific Rx queue\n\n"
 
@@ -17991,7 +17993,8 @@ cmdline_parse_token_string_t cmd_config_per_port_rx_offload_result_offload =
 		 offload, "vlan_strip#ipv4_cksum#udp_cksum#tcp_cksum#tcp_lro#"
 			   "qinq_strip#outer_ipv4_cksum#macsec_strip#"
 			   "header_split#vlan_filter#vlan_extend#jumbo_frame#"
-			   "crc_strip#scatter#timestamp#security#keep_crc");
+			   "crc_strip#scatter#timestamp#security#keep_crc#"
+			   "pmd_scratch0#pmd_scratch1#pmd_scratch2#pmd_scratch3#pmd_scratch4#pmd_scratch5");
 cmdline_parse_token_string_t cmd_config_per_port_rx_offload_result_on_off =
 	TOKEN_STRING_INITIALIZER
 		(struct cmd_config_per_port_rx_offload_result,
@@ -18067,7 +18070,8 @@ cmdline_parse_inst_t cmd_config_per_port_rx_offload = {
 	.help_str = "port config <port_id> rx_offload vlan_strip|ipv4_cksum|"
 		    "udp_cksum|tcp_cksum|tcp_lro|qinq_strip|outer_ipv4_cksum|"
 		    "macsec_strip|header_split|vlan_filter|vlan_extend|"
-		    "jumbo_frame|crc_strip|scatter|timestamp|security|keep_crc "
+		    "jumbo_frame|crc_strip|scatter|timestamp|security|keep_crc|"
+		    "pmd_scratch0|pmd_scratch1|pmd_scratch2|pmd_scratch3|pmd_scratch4|pmd_scratch5 "
 		    "on|off",
 	.tokens = {
 		(void *)&cmd_config_per_port_rx_offload_result_port,
@@ -18117,7 +18121,8 @@ cmdline_parse_token_string_t cmd_config_per_queue_rx_offload_result_offload =
 		 offload, "vlan_strip#ipv4_cksum#udp_cksum#tcp_cksum#tcp_lro#"
 			   "qinq_strip#outer_ipv4_cksum#macsec_strip#"
 			   "header_split#vlan_filter#vlan_extend#jumbo_frame#"
-			   "crc_strip#scatter#timestamp#security#keep_crc");
+			   "crc_strip#scatter#timestamp#security#keep_crc#"
+			   "pmd_scratch0#pmd_scratch1#pmd_scratch2#pmd_scratch3#pmd_scratch4#pmd_scratch5");
 cmdline_parse_token_string_t cmd_config_per_queue_rx_offload_result_on_off =
 	TOKEN_STRING_INITIALIZER
 		(struct cmd_config_per_queue_rx_offload_result,
@@ -18169,7 +18174,8 @@ cmdline_parse_inst_t cmd_config_per_queue_rx_offload = {
 		    "vlan_strip|ipv4_cksum|"
 		    "udp_cksum|tcp_cksum|tcp_lro|qinq_strip|outer_ipv4_cksum|"
 		    "macsec_strip|header_split|vlan_filter|vlan_extend|"
-		    "jumbo_frame|crc_strip|scatter|timestamp|security|keep_crc "
+		    "jumbo_frame|crc_strip|scatter|timestamp|security|keep_crc|"
+		    "pmd_scratch0|pmd_scratch1|pmd_scratch2|pmd_scratch3|pmd_scratch4|pmd_scratch5 "
 		    "on|off",
 	.tokens = {
 		(void *)&cmd_config_per_queue_rx_offload_result_port,
diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 8ac3016..bc6a676 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -129,6 +129,12 @@ static const struct {
 	RTE_RX_OFFLOAD_BIT2STR(KEEP_CRC),
 	RTE_RX_OFFLOAD_BIT2STR(SCTP_CKSUM),
 	RTE_RX_OFFLOAD_BIT2STR(OUTER_UDP_CKSUM),
+	RTE_RX_OFFLOAD_BIT2STR(PMD_SCRATCH0),
+	RTE_RX_OFFLOAD_BIT2STR(PMD_SCRATCH1),
+	RTE_RX_OFFLOAD_BIT2STR(PMD_SCRATCH2),
+	RTE_RX_OFFLOAD_BIT2STR(PMD_SCRATCH3),
+	RTE_RX_OFFLOAD_BIT2STR(PMD_SCRATCH4),
+	RTE_RX_OFFLOAD_BIT2STR(PMD_SCRATCH5),
 };
 
 #undef RTE_RX_OFFLOAD_BIT2STR
diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
index 50c6936..45bac1c 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -1011,6 +1011,12 @@ struct rte_eth_conf {
 #define DEV_RX_OFFLOAD_KEEP_CRC		0x00010000
 #define DEV_RX_OFFLOAD_SCTP_CKSUM	0x00020000
 #define DEV_RX_OFFLOAD_OUTER_UDP_CKSUM  0x00040000
+#define DEV_RX_OFFLOAD_PMD_SCRATCH0	(1ULL << 58) /* Defined by PMD */
+#define DEV_RX_OFFLOAD_PMD_SCRATCH1	(1ULL << 59) /* Defined by PMD */
+#define DEV_RX_OFFLOAD_PMD_SCRATCH2	(1ULL << 60) /* Defined by PMD */
+#define DEV_RX_OFFLOAD_PMD_SCRATCH3	(1ULL << 61) /* Defined by PMD */
+#define DEV_RX_OFFLOAD_PMD_SCRATCH4	(1ULL << 62) /* Defined by PMD */
+#define DEV_RX_OFFLOAD_PMD_SCRATCH5	(1ULL << 63) /* Defined by PMD */
 
 #define DEV_RX_OFFLOAD_CHECKSUM (DEV_RX_OFFLOAD_IPV4_CKSUM | \
 				 DEV_RX_OFFLOAD_UDP_CKSUM | \
@@ -1018,6 +1024,12 @@ struct rte_eth_conf {
 #define DEV_RX_OFFLOAD_VLAN (DEV_RX_OFFLOAD_VLAN_STRIP | \
 			     DEV_RX_OFFLOAD_VLAN_FILTER | \
 			     DEV_RX_OFFLOAD_VLAN_EXTEND)
+#define DEV_RX_OFFLOAD_PMD_SCRATCH (DEV_RX_OFFLOAD_PMD_SCRATCH0 | \
+				    DEV_RX_OFFLOAD_PMD_SCRATCH1 | \
+				    DEV_RX_OFFLOAD_PMD_SCRATCH2 | \
+				    DEV_RX_OFFLOAD_PMD_SCRATCH3 | \
+				    DEV_RX_OFFLOAD_PMD_SCRATCH4 | \
+				    DEV_RX_OFFLOAD_PMD_SCRATCH5)
 
 /*
  * If new Rx offload capabilities are defined, they also must be
-- 
2.7.4

Re: [dpdk-dev] [RFC] ethdev: reserve the RX offload most-significant bits for PMD scartch

[Andrew Rybchenko]

CC ethdev maintainers

On 6/20/19 10:25 AM, Haiyue Wang wrote:
> Generally speaking, the DEV_RX_OFFLOAD_xxx for RX offload capabilities
> of a device is one-bit field definition, it has 64 different values at
> most.
>
> Nowdays the receiving queue of NIC has rich features not just checksum
> offload, like it can extract the network protocol header fields to its
> RX descriptors for quickly handling. But this kind of feature is not so
> common, and it is hardware related. Normally, this can be done through
> rte_devargs driver parameters, but the scope is per device. This is not
> so nice for per queue design.
>
> The per queue API 'rte_eth_rx_queue_setup' and data structure 'struct
> rte_eth_rxconf' are stable now, and be common for all PMDs. For keeping
> the ethdev API & ABI compatibility, and the application can make good
> use of the NIC's specific features, reserving the most-significant bits
> of RX offload seems an compromise method.
>
> Then the PMDs redefine these bits as they want, all PMDs share the same
> bit positions and expose their new definitions with the header file.
>
> The experimental reserved bits number is 6 currently. Tt can be one-bit
> for each features up to the the maximum number 6. It can also be some
> bits encoding: e.g, 6 bits can stand for 63 maximum number of features.
>
> We call these reserved bits as DEV_RX_OFFLOAD_PMD_SCRATCH. And the left
> unused bits number is : 64 - 19 (currently defined) - 6 (PMD scartch) =
> 39.
>
> This is not so nice for applications, they need to check PMD's driver
> name for lookuping their DEV_RX_OFFLOAD_PMD_SCRATCH definitions. But it
> is good for the applications to make use of the hardware compatibility.
>
> Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>

I would say that it very bad for applications. It sounds like reserved bits
in registers which have meaning in fact and sometimes different meaning.
Of course, it is not that bad when rules are defined, but such kind of
features tend to spread and clutter up interfaces. IMHO, the feature is
really frightening.

Re: [dpdk-dev] [RFC] ethdev: reserve the RX offload most-significant bits for PMD scartch

[Stephen Hemminger]

On Thu, 20 Jun 2019 15:25:52 +0800
Haiyue Wang <haiyue.wang@intel.com> wrote:

> Generally speaking, the DEV_RX_OFFLOAD_xxx for RX offload capabilities
> of a device is one-bit field definition, it has 64 different values at
> most.
> 
> Nowdays the receiving queue of NIC has rich features not just checksum
> offload, like it can extract the network protocol header fields to its
> RX descriptors for quickly handling. But this kind of feature is not so
> common, and it is hardware related. Normally, this can be done through
> rte_devargs driver parameters, but the scope is per device. This is not
> so nice for per queue design.
> 
> The per queue API 'rte_eth_rx_queue_setup' and data structure 'struct
> rte_eth_rxconf' are stable now, and be common for all PMDs. For keeping
> the ethdev API & ABI compatibility, and the application can make good
> use of the NIC's specific features, reserving the most-significant bits
> of RX offload seems an compromise method.
> 
> Then the PMDs redefine these bits as they want, all PMDs share the same
> bit positions and expose their new definitions with the header file.
> 
> The experimental reserved bits number is 6 currently. Tt can be one-bit
> for each features up to the the maximum number 6. It can also be some
> bits encoding: e.g, 6 bits can stand for 63 maximum number of features.
> 
> We call these reserved bits as DEV_RX_OFFLOAD_PMD_SCRATCH. And the left
> unused bits number is : 64 - 19 (currently defined) - 6 (PMD scartch) =
> 39.
> 
> This is not so nice for applications, they need to check PMD's driver
> name for lookuping their DEV_RX_OFFLOAD_PMD_SCRATCH definitions. But it
> is good for the applications to make use of the hardware compatibility.
> 
> Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
> ---

Anything that is per device type is useless for a generic application.
The goal of the DPDK should be to provide a high performance platform
that works for many device types. Too often, I see patches from hardware
vendors that are just "we can enable are cool proprietary hardware
feature in DPDK". This would just encourage that bad practice.

Re: [dpdk-dev] [RFC] ethdev: reserve the RX offload most-significant bits for PMD scartch

[Wang, Haiyue]

Hi

> -----Original Message-----
> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Friday, June 21, 2019 02:36
> To: Wang, Haiyue <haiyue.wang@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [RFC] ethdev: reserve the RX offload most-significant bits for PMD scartch
> 
> On Thu, 20 Jun 2019 15:25:52 +0800
> Haiyue Wang <haiyue.wang@intel.com> wrote:
> 
> > Generally speaking, the DEV_RX_OFFLOAD_xxx for RX offload capabilities
> > of a device is one-bit field definition, it has 64 different values at
> > most.
> >
> > Nowdays the receiving queue of NIC has rich features not just checksum
> > offload, like it can extract the network protocol header fields to its
> > RX descriptors for quickly handling. But this kind of feature is not so
> > common, and it is hardware related. Normally, this can be done through
> > rte_devargs driver parameters, but the scope is per device. This is not
> > so nice for per queue design.
> >
> > The per queue API 'rte_eth_rx_queue_setup' and data structure 'struct
> > rte_eth_rxconf' are stable now, and be common for all PMDs. For keeping
> > the ethdev API & ABI compatibility, and the application can make good
> > use of the NIC's specific features, reserving the most-significant bits
> > of RX offload seems an compromise method.
> >
> > Then the PMDs redefine these bits as they want, all PMDs share the same
> > bit positions and expose their new definitions with the header file.
> >
> > The experimental reserved bits number is 6 currently. Tt can be one-bit
> > for each features up to the the maximum number 6. It can also be some
> > bits encoding: e.g, 6 bits can stand for 63 maximum number of features.
> >
> > We call these reserved bits as DEV_RX_OFFLOAD_PMD_SCRATCH. And the left
> > unused bits number is : 64 - 19 (currently defined) - 6 (PMD scartch) =
> > 39.
> >
> > This is not so nice for applications, they need to check PMD's driver
> > name for lookuping their DEV_RX_OFFLOAD_PMD_SCRATCH definitions. But it
> > is good for the applications to make use of the hardware compatibility.
> >
> > Signed-off-by: Haiyue Wang <haiyue.wang@intel.com>
> > ---
> 
> Anything that is per device type is useless for a generic application.
> The goal of the DPDK should be to provide a high performance platform
> that works for many device types. Too often, I see patches from hardware
> vendors that are just "we can enable are cool proprietary hardware
> feature in DPDK". This would just encourage that bad practice.

Understand the DPDK's dream. :) This patch wants to make the bad application
and bad vender to use the DPDK's generic high performance platform features,
plus some bad practice like doing special hardware optimization.

Very appreciate your feedback. The PMDs should limit their desires. ;-)

Re: [dpdk-dev] [RFC] ethdev: reserve the RX offload most-significant bits for PMD scartch

[Wang, Haiyue]

Not so frightening in real world for an application to be aware of its NICs:
https://github.com/Juniper/contrail-vrouter/blob/master/dpdk/vr_dpdk_ethdev.c#L387

Yes, we need to avoid this kind of design.

BR,
Haiyue

From: Andrew Rybchenko [mailto:arybchenko@solarflare.com]
Sent: Friday, June 21, 2019 02:30
To: Wang, Haiyue <haiyue.wang@intel.com>; dev@dpdk.org
Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Thomas Monjalon <thomas@monjalon.net>
Subject: Re: [dpdk-dev] [RFC] ethdev: reserve the RX offload most-significant bits for PMD scartch

CC ethdev maintainers

On 6/20/19 10:25 AM, Haiyue Wang wrote:

Generally speaking, the DEV_RX_OFFLOAD_xxx for RX offload capabilities

of a device is one-bit field definition, it has 64 different values at

most.



Nowdays the receiving queue of NIC has rich features not just checksum

offload, like it can extract the network protocol header fields to its

RX descriptors for quickly handling. But this kind of feature is not so

common, and it is hardware related. Normally, this can be done through

rte_devargs driver parameters, but the scope is per device. This is not

so nice for per queue design.



The per queue API 'rte_eth_rx_queue_setup' and data structure 'struct

rte_eth_rxconf' are stable now, and be common for all PMDs. For keeping

the ethdev API & ABI compatibility, and the application can make good

use of the NIC's specific features, reserving the most-significant bits

of RX offload seems an compromise method.



Then the PMDs redefine these bits as they want, all PMDs share the same

bit positions and expose their new definitions with the header file.



The experimental reserved bits number is 6 currently. Tt can be one-bit

for each features up to the the maximum number 6. It can also be some

bits encoding: e.g, 6 bits can stand for 63 maximum number of features.



We call these reserved bits as DEV_RX_OFFLOAD_PMD_SCRATCH. And the left

unused bits number is : 64 - 19 (currently defined) - 6 (PMD scartch) =

39.



This is not so nice for applications, they need to check PMD's driver

name for lookuping their DEV_RX_OFFLOAD_PMD_SCRATCH definitions. But it

is good for the applications to make use of the hardware compatibility.



Signed-off-by: Haiyue Wang <haiyue.wang@intel.com><mailto:haiyue.wang@intel.com>

I would say that it very bad for applications. It sounds like reserved bits
in registers which have meaning in fact and sometimes different meaning.
Of course, it is not that bad when rules are defined, but such kind of
features tend to spread and clutter up interfaces. IMHO, the feature is
really frightening.

Re: [dpdk-dev] [RFC] ethdev: reserve the RX offload most-significant bits for PMD scartch

[Andrew Rybchenko]

On 6/21/19 4:12 AM, Wang, Haiyue wrote:
>
> Not so frightening in real world for an application to be aware of its 
> NICs:
>
> https://github.com/Juniper/contrail-vrouter/blob/master/dpdk/vr_dpdk_ethdev.c#L387 
> <https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_Juniper_contrail-2Dvrouter_blob_master_dpdk_vr-5Fdpdk-5Fethdev.c-23L387&d=DwMGaQ&c=euGZstcaTDllvimEN8b7jXrwqOf-v5A_CdpgnVfiiMM&r=flTOx6Av679My7o_iScb5sOlLD68bpUyE2RUtfW3SWQ&m=XSIm84nALkE7O1aeqpJkVJJWzepVsGEJsTeiDCxoLK4&s=L1vEJ5GeVHbammKc0iJn0YdoeKf0GqeeNJy-q5xCi6E&e=>
>
>

In this particular case it is just a workaround for bonding and bnxt.
Driver name is provided and sufficient to make it possible when
absolutely required.

> Yes, we need to avoid this kind of design.
>
> BR,
>
> Haiyue
>
> *From:*Andrew Rybchenko [mailto:arybchenko@solarflare.com]
> *Sent:* Friday, June 21, 2019 02:30
> *To:* Wang, Haiyue <haiyue.wang@intel.com>; dev@dpdk.org
> *Cc:* Yigit, Ferruh <ferruh.yigit@intel.com>; Thomas Monjalon 
> <thomas@monjalon.net>
> *Subject:* Re: [dpdk-dev] [RFC] ethdev: reserve the RX offload 
> most-significant bits for PMD scartch
>
> CC ethdev maintainers
>
> On 6/20/19 10:25 AM, Haiyue Wang wrote:
>
>     Generally speaking, the DEV_RX_OFFLOAD_xxx for RX offload capabilities
>
>     of a device is one-bit field definition, it has 64 different values at
>
>     most.
>
>     Nowdays the receiving queue of NIC has rich features not just checksum
>
>     offload, like it can extract the network protocol header fields to its
>
>     RX descriptors for quickly handling. But this kind of feature is not so
>
>     common, and it is hardware related. Normally, this can be done through
>
>     rte_devargs driver parameters, but the scope is per device. This is not
>
>     so nice for per queue design.
>
>     The per queue API 'rte_eth_rx_queue_setup' and data structure 'struct
>
>     rte_eth_rxconf' are stable now, and be common for all PMDs. For keeping
>
>     the ethdev API & ABI compatibility, and the application can make good
>
>     use of the NIC's specific features, reserving the most-significant bits
>
>     of RX offload seems an compromise method.
>
>     Then the PMDs redefine these bits as they want, all PMDs share the same
>
>     bit positions and expose their new definitions with the header file.
>
>     The experimental reserved bits number is 6 currently. Tt can be one-bit
>
>     for each features up to the the maximum number 6. It can also be some
>
>     bits encoding: e.g, 6 bits can stand for 63 maximum number of features.
>
>     We call these reserved bits as DEV_RX_OFFLOAD_PMD_SCRATCH. And the left
>
>     unused bits number is : 64 - 19 (currently defined) - 6 (PMD scartch) =
>
>     39.
>
>     This is not so nice for applications, they need to check PMD's driver
>
>     name for lookuping their DEV_RX_OFFLOAD_PMD_SCRATCH definitions. But it
>
>     is good for the applications to make use of the hardware compatibility.
>
>     Signed-off-by: Haiyue Wang<haiyue.wang@intel.com>  <mailto:haiyue.wang@intel.com>
>
>
> I would say that it very bad for applications. It sounds like reserved 
> bits
> in registers which have meaning in fact and sometimes different meaning.
> Of course, it is not that bad when rules are defined, but such kind of
> features tend to spread and clutter up interfaces. IMHO, the feature is
> really frightening.
>

Re: [dpdk-dev] [RFC] ethdev: reserve the RX offload most-significant bits for PMD scartch

[Wang, Haiyue]

Then this is not so generic if a workaround is needed. In other words, no one is so perfect. ☺

BR,
Haiyue

From: Andrew Rybchenko [mailto:arybchenko@solarflare.com]
Sent: Friday, June 21, 2019 15:34
To: Wang, Haiyue <haiyue.wang@intel.com>; dev@dpdk.org
Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Thomas Monjalon <thomas@monjalon.net>
Subject: Re: [dpdk-dev] [RFC] ethdev: reserve the RX offload most-significant bits for PMD scartch

On 6/21/19 4:12 AM, Wang, Haiyue wrote:
Not so frightening in real world for an application to be aware of its NICs:
https://github.com/Juniper/contrail-vrouter/blob/master/dpdk/vr_dpdk_ethdev.c#L387<https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_Juniper_contrail-2Dvrouter_blob_master_dpdk_vr-5Fdpdk-5Fethdev.c-23L387&d=DwMGaQ&c=euGZstcaTDllvimEN8b7jXrwqOf-v5A_CdpgnVfiiMM&r=flTOx6Av679My7o_iScb5sOlLD68bpUyE2RUtfW3SWQ&m=XSIm84nALkE7O1aeqpJkVJJWzepVsGEJsTeiDCxoLK4&s=L1vEJ5GeVHbammKc0iJn0YdoeKf0GqeeNJy-q5xCi6E&e=>



In this particular case it is just a workaround for bonding and bnxt.
Driver name is provided and sufficient to make it possible when
absolutely required.



Yes, we need to avoid this kind of design.

BR,
Haiyue

From: Andrew Rybchenko [mailto:arybchenko@solarflare.com]
Sent: Friday, June 21, 2019 02:30
To: Wang, Haiyue <haiyue.wang@intel.com><mailto:haiyue.wang@intel.com>; dev@dpdk.org<mailto:dev@dpdk.org>
Cc: Yigit, Ferruh <ferruh.yigit@intel.com><mailto:ferruh.yigit@intel.com>; Thomas Monjalon <thomas@monjalon.net><mailto:thomas@monjalon.net>
Subject: Re: [dpdk-dev] [RFC] ethdev: reserve the RX offload most-significant bits for PMD scartch

CC ethdev maintainers

On 6/20/19 10:25 AM, Haiyue Wang wrote:

Generally speaking, the DEV_RX_OFFLOAD_xxx for RX offload capabilities

of a device is one-bit field definition, it has 64 different values at

most.



Nowdays the receiving queue of NIC has rich features not just checksum

offload, like it can extract the network protocol header fields to its

RX descriptors for quickly handling. But this kind of feature is not so

common, and it is hardware related. Normally, this can be done through

rte_devargs driver parameters, but the scope is per device. This is not

so nice for per queue design.



The per queue API 'rte_eth_rx_queue_setup' and data structure 'struct

rte_eth_rxconf' are stable now, and be common for all PMDs. For keeping

the ethdev API & ABI compatibility, and the application can make good

use of the NIC's specific features, reserving the most-significant bits

of RX offload seems an compromise method.



Then the PMDs redefine these bits as they want, all PMDs share the same

bit positions and expose their new definitions with the header file.



The experimental reserved bits number is 6 currently. Tt can be one-bit

for each features up to the the maximum number 6. It can also be some

bits encoding: e.g, 6 bits can stand for 63 maximum number of features.



We call these reserved bits as DEV_RX_OFFLOAD_PMD_SCRATCH. And the left

unused bits number is : 64 - 19 (currently defined) - 6 (PMD scartch) =

39.



This is not so nice for applications, they need to check PMD's driver

name for lookuping their DEV_RX_OFFLOAD_PMD_SCRATCH definitions. But it

is good for the applications to make use of the hardware compatibility.



Signed-off-by: Haiyue Wang <haiyue.wang@intel.com><mailto:haiyue.wang@intel.com>

I would say that it very bad for applications. It sounds like reserved bits
in registers which have meaning in fact and sometimes different meaning.
Of course, it is not that bad when rules are defined, but such kind of
features tend to spread and clutter up interfaces. IMHO, the feature is
really frightening.

Re: [dpdk-dev] [RFC] ethdev: reserve the RX offload most-significant bits for PMD scartch

[Andrew Rybchenko]

On 6/21/19 10:37 AM, Wang, Haiyue wrote:
>
> Then this is not so generic if a workaround is needed. In other words, 
> no one is so perfect. J
>

Yes, it is a bug. No one is perfect.

> BR,
>
> Haiyue
>
> *From:*Andrew Rybchenko [mailto:arybchenko@solarflare.com]
> *Sent:* Friday, June 21, 2019 15:34
> *To:* Wang, Haiyue <haiyue.wang@intel.com>; dev@dpdk.org
> *Cc:* Yigit, Ferruh <ferruh.yigit@intel.com>; Thomas Monjalon 
> <thomas@monjalon.net>
> *Subject:* Re: [dpdk-dev] [RFC] ethdev: reserve the RX offload 
> most-significant bits for PMD scartch
>
> On 6/21/19 4:12 AM, Wang, Haiyue wrote:
>
>     Not so frightening in real world for an application to be aware of
>     its NICs:
>
>     https://github.com/Juniper/contrail-vrouter/blob/master/dpdk/vr_dpdk_ethdev.c#L387
>     <https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_Juniper_contrail-2Dvrouter_blob_master_dpdk_vr-5Fdpdk-5Fethdev.c-23L387&d=DwMGaQ&c=euGZstcaTDllvimEN8b7jXrwqOf-v5A_CdpgnVfiiMM&r=flTOx6Av679My7o_iScb5sOlLD68bpUyE2RUtfW3SWQ&m=XSIm84nALkE7O1aeqpJkVJJWzepVsGEJsTeiDCxoLK4&s=L1vEJ5GeVHbammKc0iJn0YdoeKf0GqeeNJy-q5xCi6E&e=>
>
>
>
>
> In this particular case it is just a workaround for bonding and bnxt.
> Driver name is provided and sufficient to make it possible when
> absolutely required.
>
>
>
>     Yes, we need to avoid this kind of design.
>
>     BR,
>
>     Haiyue
>
>     *From:*Andrew Rybchenko [mailto:arybchenko@solarflare.com]
>     *Sent:* Friday, June 21, 2019 02:30
>     *To:* Wang, Haiyue <haiyue.wang@intel.com>
>     <mailto:haiyue.wang@intel.com>; dev@dpdk.org <mailto:dev@dpdk.org>
>     *Cc:* Yigit, Ferruh <ferruh.yigit@intel.com>
>     <mailto:ferruh.yigit@intel.com>; Thomas Monjalon
>     <thomas@monjalon.net> <mailto:thomas@monjalon.net>
>     *Subject:* Re: [dpdk-dev] [RFC] ethdev: reserve the RX offload
>     most-significant bits for PMD scartch
>
>     CC ethdev maintainers
>
>     On 6/20/19 10:25 AM, Haiyue Wang wrote:
>
>         Generally speaking, the DEV_RX_OFFLOAD_xxx for RX offload capabilities
>
>         of a device is one-bit field definition, it has 64 different values at
>
>         most.
>
>           
>
>         Nowdays the receiving queue of NIC has rich features not just checksum
>
>         offload, like it can extract the network protocol header fields to its
>
>         RX descriptors for quickly handling. But this kind of feature is not so
>
>         common, and it is hardware related. Normally, this can be done through
>
>         rte_devargs driver parameters, but the scope is per device. This is not
>
>         so nice for per queue design.
>
>           
>
>         The per queue API 'rte_eth_rx_queue_setup' and data structure 'struct
>
>         rte_eth_rxconf' are stable now, and be common for all PMDs. For keeping
>
>         the ethdev API & ABI compatibility, and the application can make good
>
>         use of the NIC's specific features, reserving the most-significant bits
>
>         of RX offload seems an compromise method.
>
>           
>
>         Then the PMDs redefine these bits as they want, all PMDs share the same
>
>         bit positions and expose their new definitions with the header file.
>
>           
>
>         The experimental reserved bits number is 6 currently. Tt can be one-bit
>
>         for each features up to the the maximum number 6. It can also be some
>
>         bits encoding: e.g, 6 bits can stand for 63 maximum number of features.
>
>           
>
>         We call these reserved bits as DEV_RX_OFFLOAD_PMD_SCRATCH. And the left
>
>         unused bits number is : 64 - 19 (currently defined) - 6 (PMD scartch) =
>
>         39.
>
>           
>
>         This is not so nice for applications, they need to check PMD's driver
>
>         name for lookuping their DEV_RX_OFFLOAD_PMD_SCRATCH definitions. But it
>
>         is good for the applications to make use of the hardware compatibility.
>
>           
>
>         Signed-off-by: Haiyue Wang<haiyue.wang@intel.com>  <mailto:haiyue.wang@intel.com>
>
>
>     I would say that it very bad for applications. It sounds like
>     reserved bits
>     in registers which have meaning in fact and sometimes different
>     meaning.
>     Of course, it is not that bad when rules are defined, but such kind of
>     features tend to spread and clutter up interfaces. IMHO, the
>     feature is
>     really frightening.
>

Re: [dpdk-dev] [RFC] ethdev: reserve the RX offload most-significant bits for PMD scartch

[Wang, Haiyue]

I know my patch is ugly for making customers happy, I will try to find other method not to
break the beautiful rte_ethdev desgin.

Really thanks for your reply. This helps me understand the PMD design practice more.

BR,
Haiyue

From: Andrew Rybchenko [mailto:arybchenko@solarflare.com]
Sent: Friday, June 21, 2019 15:40
To: Wang, Haiyue <haiyue.wang@intel.com>; dev@dpdk.org
Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Thomas Monjalon <thomas@monjalon.net>
Subject: Re: [dpdk-dev] [RFC] ethdev: reserve the RX offload most-significant bits for PMD scartch

On 6/21/19 10:37 AM, Wang, Haiyue wrote:
Then this is not so generic if a workaround is needed. In other words, no one is so perfect. ☺

Yes, it is a bug. No one is perfect.



BR,
Haiyue

From: Andrew Rybchenko [mailto:arybchenko@solarflare.com]
Sent: Friday, June 21, 2019 15:34
To: Wang, Haiyue <haiyue.wang@intel.com><mailto:haiyue.wang@intel.com>; dev@dpdk.org<mailto:dev@dpdk.org>
Cc: Yigit, Ferruh <ferruh.yigit@intel.com><mailto:ferruh.yigit@intel.com>; Thomas Monjalon <thomas@monjalon.net><mailto:thomas@monjalon.net>
Subject: Re: [dpdk-dev] [RFC] ethdev: reserve the RX offload most-significant bits for PMD scartch

On 6/21/19 4:12 AM, Wang, Haiyue wrote:
Not so frightening in real world for an application to be aware of its NICs:
https://github.com/Juniper/contrail-vrouter/blob/master/dpdk/vr_dpdk_ethdev.c#L387<https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_Juniper_contrail-2Dvrouter_blob_master_dpdk_vr-5Fdpdk-5Fethdev.c-23L387&d=DwMGaQ&c=euGZstcaTDllvimEN8b7jXrwqOf-v5A_CdpgnVfiiMM&r=flTOx6Av679My7o_iScb5sOlLD68bpUyE2RUtfW3SWQ&m=XSIm84nALkE7O1aeqpJkVJJWzepVsGEJsTeiDCxoLK4&s=L1vEJ5GeVHbammKc0iJn0YdoeKf0GqeeNJy-q5xCi6E&e=>




In this particular case it is just a workaround for bonding and bnxt.
Driver name is provided and sufficient to make it possible when
absolutely required.





Yes, we need to avoid this kind of design.

BR,
Haiyue

From: Andrew Rybchenko [mailto:arybchenko@solarflare.com]
Sent: Friday, June 21, 2019 02:30
To: Wang, Haiyue <haiyue.wang@intel.com><mailto:haiyue.wang@intel.com>; dev@dpdk.org<mailto:dev@dpdk.org>
Cc: Yigit, Ferruh <ferruh.yigit@intel.com><mailto:ferruh.yigit@intel.com>; Thomas Monjalon <thomas@monjalon.net><mailto:thomas@monjalon.net>
Subject: Re: [dpdk-dev] [RFC] ethdev: reserve the RX offload most-significant bits for PMD scartch

CC ethdev maintainers

On 6/20/19 10:25 AM, Haiyue Wang wrote:

Generally speaking, the DEV_RX_OFFLOAD_xxx for RX offload capabilities

of a device is one-bit field definition, it has 64 different values at

most.



Nowdays the receiving queue of NIC has rich features not just checksum

offload, like it can extract the network protocol header fields to its

RX descriptors for quickly handling. But this kind of feature is not so

common, and it is hardware related. Normally, this can be done through

rte_devargs driver parameters, but the scope is per device. This is not

so nice for per queue design.



The per queue API 'rte_eth_rx_queue_setup' and data structure 'struct

rte_eth_rxconf' are stable now, and be common for all PMDs. For keeping

the ethdev API & ABI compatibility, and the application can make good

use of the NIC's specific features, reserving the most-significant bits

of RX offload seems an compromise method.



Then the PMDs redefine these bits as they want, all PMDs share the same

bit positions and expose their new definitions with the header file.



The experimental reserved bits number is 6 currently. Tt can be one-bit

for each features up to the the maximum number 6. It can also be some

bits encoding: e.g, 6 bits can stand for 63 maximum number of features.



We call these reserved bits as DEV_RX_OFFLOAD_PMD_SCRATCH. And the left

unused bits number is : 64 - 19 (currently defined) - 6 (PMD scartch) =

39.



This is not so nice for applications, they need to check PMD's driver

name for lookuping their DEV_RX_OFFLOAD_PMD_SCRATCH definitions. But it

is good for the applications to make use of the hardware compatibility.



Signed-off-by: Haiyue Wang <haiyue.wang@intel.com><mailto:haiyue.wang@intel.com>

I would say that it very bad for applications. It sounds like reserved bits
in registers which have meaning in fact and sometimes different meaning.
Of course, it is not that bad when rules are defined, but such kind of
features tend to spread and clutter up interfaces. IMHO, the feature is
really frightening.

Re: [dpdk-dev] [RFC] ethdev: reserve the RX offload most-significant bits for PMD scartch

[Stephen Hemminger]

On Fri, 21 Jun 2019 07:43:13 +0000
"Wang, Haiyue" <haiyue.wang@intel.com> wrote:

> I know my patch is ugly for making customers happy, I will try to find other method not to
> break the beautiful rte_ethdev desgin.
> 
> Really thanks for your reply. This helps me understand the PMD design practice more.
> 
> BR,
> Haiyue
> 
> From: Andrew Rybchenko [mailto:arybchenko@solarflare.com]
> Sent: Friday, June 21, 2019 15:40
> To: Wang, Haiyue <haiyue.wang@intel.com>; dev@dpdk.org
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Thomas Monjalon <thomas@monjalon.net>
> Subject: Re: [dpdk-dev] [RFC] ethdev: reserve the RX offload most-significant bits for PMD scartch
> 
> On 6/21/19 10:37 AM, Wang, Haiyue wrote:
> Then this is not so generic if a workaround is needed. In other words, no one is so perfect. ☺
> 
> Yes, it is a bug. No one is perfect.
> 
> 
> 
> BR,
> Haiyue
> 
> From: Andrew Rybchenko [mailto:arybchenko@solarflare.com]
> Sent: Friday, June 21, 2019 15:34
> To: Wang, Haiyue <haiyue.wang@intel.com><mailto:haiyue.wang@intel.com>; dev@dpdk.org<mailto:dev@dpdk.org>
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com><mailto:ferruh.yigit@intel.com>; Thomas Monjalon <thomas@monjalon.net><mailto:thomas@monjalon.net>
> Subject: Re: [dpdk-dev] [RFC] ethdev: reserve the RX offload most-significant bits for PMD scartch
> 
> On 6/21/19 4:12 AM, Wang, Haiyue wrote:
> Not so frightening in real world for an application to be aware of its NICs:
> https://github.com/Juniper/contrail-vrouter/blob/master/dpdk/vr_dpdk_ethdev.c#L387<https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_Juniper_contrail-2Dvrouter_blob_master_dpdk_vr-5Fdpdk-5Fethdev.c-23L387&d=DwMGaQ&c=euGZstcaTDllvimEN8b7jXrwqOf-v5A_CdpgnVfiiMM&r=flTOx6Av679My7o_iScb5sOlLD68bpUyE2RUtfW3SWQ&m=XSIm84nALkE7O1aeqpJkVJJWzepVsGEJsTeiDCxoLK4&s=L1vEJ5GeVHbammKc0iJn0YdoeKf0GqeeNJy-q5xCi6E&e=>
> 
> 
> 
> 
> In this particular case it is just a workaround for bonding and bnxt.
> Driver name is provided and sufficient to make it possible when
> absolutely required.
> 
> 
> 
> 
> 
> Yes, we need to avoid this kind of design.
> 
> BR,
> Haiyue
> 
> From: Andrew Rybchenko [mailto:arybchenko@solarflare.com]
> Sent: Friday, June 21, 2019 02:30
> To: Wang, Haiyue <haiyue.wang@intel.com><mailto:haiyue.wang@intel.com>; dev@dpdk.org<mailto:dev@dpdk.org>
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com><mailto:ferruh.yigit@intel.com>; Thomas Monjalon <thomas@monjalon.net><mailto:thomas@monjalon.net>
> Subject: Re: [dpdk-dev] [RFC] ethdev: reserve the RX offload most-significant bits for PMD scartch
> 
> CC ethdev maintainers
> 
> On 6/20/19 10:25 AM, Haiyue Wang wrote:
> 
> Generally speaking, the DEV_RX_OFFLOAD_xxx for RX offload capabilities
> 
> of a device is one-bit field definition, it has 64 different values at
> 
> most.
> 
> 
> 
> Nowdays the receiving queue of NIC has rich features not just checksum
> 
> offload, like it can extract the network protocol header fields to its
> 
> RX descriptors for quickly handling. But this kind of feature is not so
> 
> common, and it is hardware related. Normally, this can be done through
> 
> rte_devargs driver parameters, but the scope is per device. This is not
> 
> so nice for per queue design.
> 
> 
> 
> The per queue API 'rte_eth_rx_queue_setup' and data structure 'struct
> 
> rte_eth_rxconf' are stable now, and be common for all PMDs. For keeping
> 
> the ethdev API & ABI compatibility, and the application can make good
> 
> use of the NIC's specific features, reserving the most-significant bits
> 
> of RX offload seems an compromise method.
> 
> 
> 
> Then the PMDs redefine these bits as they want, all PMDs share the same
> 
> bit positions and expose their new definitions with the header file.
> 
> 
> 
> The experimental reserved bits number is 6 currently. Tt can be one-bit
> 
> for each features up to the the maximum number 6. It can also be some
> 
> bits encoding: e.g, 6 bits can stand for 63 maximum number of features.
> 
> 
> 
> We call these reserved bits as DEV_RX_OFFLOAD_PMD_SCRATCH. And the left
> 
> unused bits number is : 64 - 19 (currently defined) - 6 (PMD scartch) =
> 
> 39.
> 
> 
> 
> This is not so nice for applications, they need to check PMD's driver
> 
> name for lookuping their DEV_RX_OFFLOAD_PMD_SCRATCH definitions. But it
> 
> is good for the applications to make use of the hardware compatibility.
> 
> 
> 
> Signed-off-by: Haiyue Wang <haiyue.wang@intel.com><mailto:haiyue.wang@intel.com>
> 
> I would say that it very bad for applications. It sounds like reserved bits
> in registers which have meaning in fact and sometimes different meaning.
> Of course, it is not that bad when rules are defined, but such kind of
> features tend to spread and clutter up interfaces. IMHO, the feature is
> really frightening.

There are two issues. First, having more OFFLOAD capability feature bits
is good. As long as these feature bits are well defined. If only one vendor
implements that feature that is fine. Another vendor can implement the
same thing, and application can know what it is asking for.

The other issue is the limited number of feature bits. I expect that some
time soon the bits will have to grow into an array and cause API/ABI
break. That can be fixed when the last bit is exhausted.

Re: [dpdk-dev] [RFC] ethdev: reserve the RX offload most-significant bits for PMD scartch

[Wang, Haiyue]

> -----Original Message-----
> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Friday, June 21, 2019 23:14
> To: Wang, Haiyue <haiyue.wang@intel.com>
> Cc: Andrew Rybchenko <arybchenko@solarflare.com>; dev@dpdk.org; Yigit, Ferruh <ferruh.yigit@intel.com>;
> Thomas Monjalon <thomas@monjalon.net>
> Subject: Re: [dpdk-dev] [RFC] ethdev: reserve the RX offload most-significant bits for PMD scartch
> 
> On Fri, 21 Jun 2019 07:43:13 +0000
> "Wang, Haiyue" <haiyue.wang@intel.com> wrote:
> 
> > The experimental reserved bits number is 6 currently. Tt can be one-bit
> >
> > for each features up to the the maximum number 6. It can also be some
> >
> > bits encoding: e.g, 6 bits can stand for 63 maximum number of features.
> >
> >
> >
> > We call these reserved bits as DEV_RX_OFFLOAD_PMD_SCRATCH. And the left
> >
> > unused bits number is : 64 - 19 (currently defined) - 6 (PMD scartch) =
> >
> > 39.
> >
> >
> >
> > This is not so nice for applications, they need to check PMD's driver
> >
> > name for lookuping their DEV_RX_OFFLOAD_PMD_SCRATCH definitions. But it
> >
> > is good for the applications to make use of the hardware compatibility.
> >
> >
> >
> > Signed-off-by: Haiyue Wang <haiyue.wang@intel.com><mailto:haiyue.wang@intel.com>
> >
> > I would say that it very bad for applications. It sounds like reserved bits
> > in registers which have meaning in fact and sometimes different meaning.
> > Of course, it is not that bad when rules are defined, but such kind of
> > features tend to spread and clutter up interfaces. IMHO, the feature is
> > really frightening.
> 
> There are two issues. First, having more OFFLOAD capability feature bits
> is good. As long as these feature bits are well defined. If only one vendor
> implements that feature that is fine. Another vendor can implement the
> same thing, and application can know what it is asking for.
> 
> The other issue is the limited number of feature bits. I expect that some
> time soon the bits will have to grow into an array and cause API/ABI
> break. That can be fixed when the last bit is exhausted.
> 
> 

If one bit for one feature, then it will be exhausted soon. That's why I said
using DEV_RX_OFFLOAD_PMD_SCRATCH bits to *encode* the PMD's offload if it is no
so common now, such as 6 bits will give the vendor 63 different types to select
their own features. And have 39 for common features defined in the future.

Frankly speaking, if we open some bits for PMD using, like the __rte_experimental
API style, then PMD will have more rich feature for open, customer can use the
experimental features, and these experimental features may be common in some day.

Well, just my two cents. I've also provided the testpmd cmdline to help more
vendors can test their SCRATCH RX OFFLOADS if they have.

Re: [dpdk-dev] [RFC] ethdev: reserve the RX offload most-significant bits for PMD scartch

[David Marchand]

On Fri, Jun 21, 2019 at 6:37 PM Wang, Haiyue <haiyue.wang@intel.com> wrote:

> > -----Original Message-----
> > From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> > Sent: Friday, June 21, 2019 23:14
> > To: Wang, Haiyue <haiyue.wang@intel.com>
> > Cc: Andrew Rybchenko <arybchenko@solarflare.com>; dev@dpdk.org; Yigit,
> Ferruh <ferruh.yigit@intel.com>;
> > Thomas Monjalon <thomas@monjalon.net>
> > Subject: Re: [dpdk-dev] [RFC] ethdev: reserve the RX offload
> most-significant bits for PMD scartch
> >
> > On Fri, 21 Jun 2019 07:43:13 +0000
> > "Wang, Haiyue" <haiyue.wang@intel.com> wrote:
> >
> > > The experimental reserved bits number is 6 currently. Tt can be one-bit
> > >
> > > for each features up to the the maximum number 6. It can also be some
> > >
> > > bits encoding: e.g, 6 bits can stand for 63 maximum number of features.
> > >
> > >
> > >
> > > We call these reserved bits as DEV_RX_OFFLOAD_PMD_SCRATCH. And the left
> > >
> > > unused bits number is : 64 - 19 (currently defined) - 6 (PMD scartch) =
> > >
> > > 39.
> > >
> > >
> > >
> > > This is not so nice for applications, they need to check PMD's driver
> > >
> > > name for lookuping their DEV_RX_OFFLOAD_PMD_SCRATCH definitions. But it
> > >
> > > is good for the applications to make use of the hardware compatibility.
> > >
> > >
> > >
> > > Signed-off-by: Haiyue Wang <haiyue.wang@intel.com><mailto:
> haiyue.wang@intel.com>
> > >
> > > I would say that it very bad for applications. It sounds like reserved
> bits
> > > in registers which have meaning in fact and sometimes different
> meaning.
> > > Of course, it is not that bad when rules are defined, but such kind of
> > > features tend to spread and clutter up interfaces. IMHO, the feature is
> > > really frightening.
> >
> > There are two issues. First, having more OFFLOAD capability feature bits
> > is good. As long as these feature bits are well defined. If only one
> vendor
> > implements that feature that is fine. Another vendor can implement the
> > same thing, and application can know what it is asking for.
> >
> > The other issue is the limited number of feature bits. I expect that some
> > time soon the bits will have to grow into an array and cause API/ABI
> > break. That can be fixed when the last bit is exhausted.
> >
> >
>
> If one bit for one feature, then it will be exhausted soon. That's why I
> said
> using DEV_RX_OFFLOAD_PMD_SCRATCH bits to *encode* the PMD's offload if it
> is no
> so common now, such as 6 bits will give the vendor 63 different types to
> select
> their own features. And have 39 for common features defined in the future.
>
> Frankly speaking, if we open some bits for PMD using, like the
> __rte_experimental
> API style, then PMD will have more rich feature for open, customer can use
> the
> experimental features, and these experimental features may be common in
> some day.
>

Just try to imagine what it would mean for the dataplane handling a packet:

if (is_port_vendor_X(portid)) {
  handle_exotic_vendor_X();
} else if (is_port_vendor_Y(portid)) {
  handle_exotic_vendor_Y();
} else {
  generic_handle();
}

Add to this changes with versions of dpdk since this would be out of the
ABI/API stability and this is a huge mess.
This is a NAK for me.


-- 
David Marchand

Re: [dpdk-dev] [RFC] ethdev: reserve the RX offload most-significant bits for PMD scartch

[Wang, Haiyue]

NAK is expected. ☺ Just like the __rte_experimental API may be remove one day,
the application will fight for compiling, or just keep their own DPDK version.

Well, private handling is not a good thing, but HW is different, reserving some bits may
also be another API style. In real world, many workarounds exist, then a good solution
comes.

BR,
Haiyue

From: David Marchand [mailto:david.marchand@redhat.com]
Sent: Saturday, June 22, 2019 00:46
To: Wang, Haiyue <haiyue.wang@intel.com>
Cc: Stephen Hemminger <stephen@networkplumber.org>; Andrew Rybchenko <arybchenko@solarflare.com>; dev@dpdk.org; Yigit, Ferruh <ferruh.yigit@intel.com>; Thomas Monjalon <thomas@monjalon.net>
Subject: Re: [dpdk-dev] [RFC] ethdev: reserve the RX offload most-significant bits for PMD scartch



On Fri, Jun 21, 2019 at 6:37 PM Wang, Haiyue <haiyue.wang@intel.com<mailto:haiyue.wang@intel.com>> wrote:
> -----Original Message-----
> From: Stephen Hemminger [mailto:stephen@networkplumber.org<mailto:stephen@networkplumber.org>]
> Sent: Friday, June 21, 2019 23:14
> To: Wang, Haiyue <haiyue.wang@intel.com<mailto:haiyue.wang@intel.com>>
> Cc: Andrew Rybchenko <arybchenko@solarflare.com<mailto:arybchenko@solarflare.com>>; dev@dpdk.org<mailto:dev@dpdk.org>; Yigit, Ferruh <ferruh.yigit@intel.com<mailto:ferruh.yigit@intel.com>>;
> Thomas Monjalon <thomas@monjalon.net<mailto:thomas@monjalon.net>>
> Subject: Re: [dpdk-dev] [RFC] ethdev: reserve the RX offload most-significant bits for PMD scartch
>
> On Fri, 21 Jun 2019 07:43:13 +0000
> "Wang, Haiyue" <haiyue.wang@intel.com<mailto:haiyue.wang@intel.com>> wrote:
>
> > The experimental reserved bits number is 6 currently. Tt can be one-bit
> >
> > for each features up to the the maximum number 6. It can also be some
> >
> > bits encoding: e.g, 6 bits can stand for 63 maximum number of features.
> >
> >
> >
> > We call these reserved bits as DEV_RX_OFFLOAD_PMD_SCRATCH. And the left
> >
> > unused bits number is : 64 - 19 (currently defined) - 6 (PMD scartch) =
> >
> > 39.
> >
> >
> >
> > This is not so nice for applications, they need to check PMD's driver
> >
> > name for lookuping their DEV_RX_OFFLOAD_PMD_SCRATCH definitions. But it
> >
> > is good for the applications to make use of the hardware compatibility.
> >
> >
> >
> > Signed-off-by: Haiyue Wang <haiyue.wang@intel.com<mailto:haiyue.wang@intel.com>><mailto:haiyue.wang@intel.com<mailto:haiyue.wang@intel.com>>
> >
> > I would say that it very bad for applications. It sounds like reserved bits
> > in registers which have meaning in fact and sometimes different meaning.
> > Of course, it is not that bad when rules are defined, but such kind of
> > features tend to spread and clutter up interfaces. IMHO, the feature is
> > really frightening.
>
> There are two issues. First, having more OFFLOAD capability feature bits
> is good. As long as these feature bits are well defined. If only one vendor
> implements that feature that is fine. Another vendor can implement the
> same thing, and application can know what it is asking for.
>
> The other issue is the limited number of feature bits. I expect that some
> time soon the bits will have to grow into an array and cause API/ABI
> break. That can be fixed when the last bit is exhausted.
>
>

If one bit for one feature, then it will be exhausted soon. That's why I said
using DEV_RX_OFFLOAD_PMD_SCRATCH bits to *encode* the PMD's offload if it is no
so common now, such as 6 bits will give the vendor 63 different types to select
their own features. And have 39 for common features defined in the future.

Frankly speaking, if we open some bits for PMD using, like the __rte_experimental
API style, then PMD will have more rich feature for open, customer can use the
experimental features, and these experimental features may be common in some day.

Just try to imagine what it would mean for the dataplane handling a packet:

if (is_port_vendor_X(portid)) {
  handle_exotic_vendor_X();
} else if (is_port_vendor_Y(portid)) {
  handle_exotic_vendor_Y();
} else {
  generic_handle();
}

Add to this changes with versions of dpdk since this would be out of the ABI/API stability and this is a huge mess.
This is a NAK for me.


--
David Marchand