Re: [RFC PATCH 4/4] ethdev: add helpers to move to the new offloads API

23/08/2017 15:13, Shahaf Shuler:

Wednesday, August 23, 2017 3:29 PM, Ananyev, Konstantin:

quoted

From: Shahaf Shuler

quoted

In order to enable PMDs to support only one of the APIs, and
applications to avoid branching according to the underlying device a
copy functions to/from the old/new APIs were added.

Looks a good intent.
I would prefer the word "convert" instead of "copy".

quoted

quoted

 int
 rte_eth_rx_queue_setup(uint8_t port_id, uint16_t rx_queue_id,

[...]

quoted

quoted

+	} else if ((!(dev->data->dev_flags & RTE_ETH_DEV_RXQ_OFFLOAD)) &&
+		   (dev->data->dev_conf.rxmode.ignore == 1)) {
+		int ret;
+		struct rte_eth_rxmode rxmode;
+
+		rte_eth_copy_rxq_offloads(&rxmode, rx_conf);
+		if (memcmp(&rxmode, &dev->data->dev_conf.rxmode,
+			   sizeof(rxmode))) {
+			/*
+			 * device which work with rxmode offloads API requires
+			 * a re-configuration in order to apply the new offloads
+			 * configuration.
+			 */
+			dev->data->dev_conf.rxmode = rxmode;
+			ret = rte_eth_dev_configure(port_id,
+					dev->data->nb_rx_queues,
+					dev->data->nb_tx_queues,
+					&dev->data->dev_conf);

Hmm, and why we would need to reconfigure our device in the middle of rx
queue setup?

The reason is the old Rx offloads API is configured on device configure. 
This if section is for applications which already moved to the new offload
API however the underlying PMD still uses the old one.

Isn't it risky to re-run configure here?
We could also declare this case as an error.

I think applications which have migrated to the new API,
could use the convert functions themselves before calling configure
to support not migrated PMDs.
The cons of my solution are:
- discourage apps to migrate before all PMDs have migrated
- expose a temporary function to convert API
I propose it anyway because there is always someone to like bad ideas ;)