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

[Shahaf Shuler]

Thursday, August 24, 2017 1:06 AM, Thomas Monjalon:
> 23/08/2017 15:13, Shahaf Shuler:
> > Wednesday, August 23, 2017 3:29 PM, Ananyev, Konstantin:
> > > From: Shahaf Shuler
> > > > 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".
> 
> > > >  int
> > > >  rte_eth_rx_queue_setup(uint8_t port_id, uint16_t rx_queue_id,
> [...]
> > > > +	} 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 ;)

Yes. I tried to make it as simple as possible for application to move to the new API.
Defining it as error flow, will enforce the application to check the PMD offload mode and branch accordingly. The conversion functions are a good helpers, yet the code remains complex due to the different cases with the different PMDs.

Considering the re-configuration is risky, and without other ideas I will need to fall back to the error flow case.
Are we OK with that?