diff mbox

[dpdk-dev,14/39] examples/ip_reassembly: convert to new ethdev offloads API

Message ID 20171123121941.144335-5-shahafs@mellanox.com (mailing list archive)
State Superseded, archived
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Shahaf Shuler Nov. 23, 2017, 12:19 p.m. UTC 
  Ethdev offloads API has changed since:

commit ce17eddefc20 ("ethdev: introduce Rx queue offloads API")
commit cba7f53b717d ("ethdev: introduce Tx queue offloads API")

This commit support the new API.

Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
---
 examples/ip_reassembly/main.c | 36 +++++++++++++++++++++++++++++-------
 1 file changed, 29 insertions(+), 7 deletions(-)

Comments

Ananyev, Konstantin Dec. 11, 2017, 3:03 p.m. UTC | #1 
Hi Shahaf,


> +		if ((dev_info.rx_offload_capa & port_conf.rxmode.offloads) !=
> +		    port_conf.rxmode.offloads) {
> +			printf("Some Rx offloads are not supported "
> +			       "by port %d: requested 0x%lx supported 0x%lx\n",
> +			       portid, port_conf.rxmode.offloads,
> +			       dev_info.rx_offload_capa);
> +			port_conf.rxmode.offloads &= dev_info.rx_offload_capa;
> +		}
> +		if ((dev_info.tx_offload_capa & port_conf.txmode.offloads) !=
> +		    port_conf.txmode.offloads) {
> +			printf("Some Tx offloads are not supported "
> +			       "by port %d: requested 0x%lx supported 0x%lx\n",
> +			       portid, port_conf.txmode.offloads,
> +			       dev_info.tx_offload_capa);
> +			port_conf.txmode.offloads &= dev_info.tx_offload_capa;
> +		}

Sort of generic question regarding most examples - wouldn't it be better to do rte_exit() if device doesn't
support the offloads we expect instead of masking off unsupported offloads and continue?
Konstantin 

>  		ret = rte_eth_dev_configure(portid, 1, (uint16_t)n_tx_queue,
>  					    &port_conf);
Shahaf Shuler Dec. 12, 2017, 6:30 a.m. UTC | #2 
Monday, December 11, 2017 5:04 PM, Ananyev, Konstantin:
> > +		if ((dev_info.tx_offload_capa & port_conf.txmode.offloads)
> !=
> > +		    port_conf.txmode.offloads) {
> > +			printf("Some Tx offloads are not supported "
> > +			       "by port %d: requested 0x%lx supported
> 0x%lx\n",
> > +			       portid, port_conf.txmode.offloads,
> > +			       dev_info.tx_offload_capa);
> > +			port_conf.txmode.offloads &=
> dev_info.tx_offload_capa;
> > +		}
> 
> Sort of generic question regarding most examples - wouldn't it be better to
> do rte_exit() if device doesn't support the offloads we expect instead of
> masking off unsupported offloads and continue?
> Konstantin

We already started to discuss this question, see [1].

I agree that it is wrong approach to mask the not supported offloads and continue the application. 
So now I we have 2 options:
1. report the warning and let the PMD to fail the device configuration.
2. like you suggested, report the error and exit the application.

While it is wrong for application to set offloads which are not reported by the device capabilities, the input I got from Radu is that there are a lot of PMDs that will break with option 2, see [1]. 
One example is ixgbe which expects to have CRC offload enabled with IPSEC but don't report it on its caps. 

So my current direction is to make the examples less strict, and give the option for the PMD to fail those if not supported.
Any objection? 

[1] http://dpdk.org/ml/archives/dev/2017-December/083441.html
Ananyev, Konstantin Dec. 12, 2017, 8:49 a.m. UTC | #3 
> -----Original Message-----
> From: Shahaf Shuler [mailto:shahafs@mellanox.com]
> Sent: Tuesday, December 12, 2017 6:31 AM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; dev@dpdk.org; Nicolau, Radu <radu.nicolau@intel.com>
> Subject: RE: [dpdk-dev] [PATCH 14/39] examples/ip_reassembly: convert to new ethdev offloads API
> 
> Monday, December 11, 2017 5:04 PM, Ananyev, Konstantin:
> > > +		if ((dev_info.tx_offload_capa & port_conf.txmode.offloads)
> > !=
> > > +		    port_conf.txmode.offloads) {
> > > +			printf("Some Tx offloads are not supported "
> > > +			       "by port %d: requested 0x%lx supported
> > 0x%lx\n",
> > > +			       portid, port_conf.txmode.offloads,
> > > +			       dev_info.tx_offload_capa);
> > > +			port_conf.txmode.offloads &=
> > dev_info.tx_offload_capa;
> > > +		}
> >
> > Sort of generic question regarding most examples - wouldn't it be better to
> > do rte_exit() if device doesn't support the offloads we expect instead of
> > masking off unsupported offloads and continue?
> > Konstantin
> 
> We already started to discuss this question, see [1].
> 
> I agree that it is wrong approach to mask the not supported offloads and continue the application.
> So now I we have 2 options:
> 1. report the warning and let the PMD to fail the device configuration.
> 2. like you suggested, report the error and exit the application.
> 
> While it is wrong for application to set offloads which are not reported by the device capabilities, the input I got from Radu is that there are
> a lot of PMDs that will break with option 2, see [1].
> One example is ixgbe which expects to have CRC offload enabled with IPSEC but don't report it on its caps.
> 
> So my current direction is to make the examples less strict, and give the option for the PMD to fail those if not supported.
> Any objection?

So basically option #1 from the above?
If so, none from me.

> 
> [1] http://dpdk.org/ml/archives/dev/2017-December/083441.html
>
diff mbox

Patch

diff --git a/examples/ip_reassembly/main.c b/examples/ip_reassembly/main.c
index 756f90efb..d899e4c37 100644
--- a/examples/ip_reassembly/main.c
+++ b/examples/ip_reassembly/main.c
@@ -193,11 +193,10 @@  static struct rte_eth_conf port_conf = {
 		.mq_mode        = ETH_MQ_RX_RSS,
 		.max_rx_pkt_len = JUMBO_FRAME_MAX_SIZE,
 		.split_hdr_size = 0,
-		.header_split   = 0, /**< Header Split disabled */
-		.hw_ip_checksum = 1, /**< IP checksum offload enabled */
-		.hw_vlan_filter = 0, /**< VLAN filtering disabled */
-		.jumbo_frame    = 1, /**< Jumbo Frame Support disabled */
-		.hw_strip_crc   = 1, /**< CRC stripped by hardware */
+		.ignore_offload_bitfield = 1,
+		.offloads = (DEV_RX_OFFLOAD_CHECKSUM |
+			     DEV_RX_OFFLOAD_JUMBO_FRAME |
+			     DEV_RX_OFFLOAD_CRC_STRIP),
 	},
 	.rx_adv_conf = {
 			.rss_conf = {
@@ -207,6 +206,8 @@  static struct rte_eth_conf port_conf = {
 	},
 	.txmode = {
 		.mq_mode = ETH_MQ_TX_NONE,
+		.offloads = (DEV_TX_OFFLOAD_IPV4_CKSUM |
+			     DEV_TX_OFFLOAD_MULTI_SEGS),
 	},
 };
 
@@ -1052,6 +1053,8 @@  main(int argc, char **argv)
 
 	/* initialize all ports */
 	for (portid = 0; portid < nb_ports; portid++) {
+		struct rte_eth_rxconf rxq_conf;
+
 		/* skip ports that are not enabled */
 		if ((enabled_port_mask & (1 << portid)) == 0) {
 			printf("\nSkipping disabled port %d\n", portid);
@@ -1104,6 +1107,22 @@  main(int argc, char **argv)
 		n_tx_queue = nb_lcores;
 		if (n_tx_queue > MAX_TX_QUEUE_PER_PORT)
 			n_tx_queue = MAX_TX_QUEUE_PER_PORT;
+		if ((dev_info.rx_offload_capa & port_conf.rxmode.offloads) !=
+		    port_conf.rxmode.offloads) {
+			printf("Some Rx offloads are not supported "
+			       "by port %d: requested 0x%lx supported 0x%lx\n",
+			       portid, port_conf.rxmode.offloads,
+			       dev_info.rx_offload_capa);
+			port_conf.rxmode.offloads &= dev_info.rx_offload_capa;
+		}
+		if ((dev_info.tx_offload_capa & port_conf.txmode.offloads) !=
+		    port_conf.txmode.offloads) {
+			printf("Some Tx offloads are not supported "
+			       "by port %d: requested 0x%lx supported 0x%lx\n",
+			       portid, port_conf.txmode.offloads,
+			       dev_info.tx_offload_capa);
+			port_conf.txmode.offloads &= dev_info.tx_offload_capa;
+		}
 		ret = rte_eth_dev_configure(portid, 1, (uint16_t)n_tx_queue,
 					    &port_conf);
 		if (ret < 0) {
@@ -1114,8 +1133,10 @@  main(int argc, char **argv)
 		}
 
 		/* init one RX queue */
+		rxq_conf = dev_info.default_rxconf;
+		rxq_conf.offloads = port_conf.rxmode.offloads;
 		ret = rte_eth_rx_queue_setup(portid, 0, nb_rxd,
-					     socket, NULL,
+					     socket, &rxq_conf,
 					     rxq->pool);
 		if (ret < 0) {
 			printf("\n");
@@ -1140,7 +1161,8 @@  main(int argc, char **argv)
 			fflush(stdout);
 
 			txconf = &dev_info.default_txconf;
-			txconf->txq_flags = 0;
+			txconf->txq_flags = ETH_TXQ_FLAGS_IGNORE;
+			txconf->offloads = port_conf.txmode.offloads;
 
 			ret = rte_eth_tx_queue_setup(portid, queueid, nb_txd,
 					socket, txconf);