[PATCH v7 01/21] net/cpfl: support device initialization

[Liu, Mingxia]

Thanks you all!
It's a good question, but as it is experimental version and rc2 is approaching,
we won't optimize this function now, and will do it at the time of the official product release.

> -----Original Message-----
> From: Thomas Monjalon <thomas at monjalon.net>
> Sent: Monday, February 27, 2023 11:46 PM
> To: Andrew Rybchenko <andrew.rybchenko at oktetlabs.ru>; Jerin Jacob
> Kollanukkaran <jerinj at marvell.com>; Zhang, Qi Z <qi.z.zhang at intel.com>;
> David Marchand <david.marchand at redhat.com>; Ferruh Yigit
> <ferruh.yigit at amd.com>
> Cc: dev at dpdk.org; Liu, Mingxia <mingxia.liu at intel.com>; Zhang, Yuying
> <yuying.zhang at intel.com>; Xing, Beilei <beilei.xing at intel.com>;
> techboard at dpdk.org
> Subject: Re: [PATCH v7 01/21] net/cpfl: support device initialization
> 
> 27/02/2023 14:46, Ferruh Yigit:
> > On 2/16/2023 12:29 AM, Mingxia Liu wrote:
> > > +static int
> > > +cpfl_dev_configure(struct rte_eth_dev *dev) {
> > > +	struct rte_eth_conf *conf = &dev->data->dev_conf;
> > > +
> > > +	if (conf->link_speeds & RTE_ETH_LINK_SPEED_FIXED) {
> > > +		PMD_INIT_LOG(ERR, "Setting link speed is not supported");
> > > +		return -ENOTSUP;
> > > +	}
> > > +
> > > +	if (conf->txmode.mq_mode != RTE_ETH_MQ_TX_NONE) {
> > > +		PMD_INIT_LOG(ERR, "Multi-queue TX mode %d is not
> supported",
> > > +			     conf->txmode.mq_mode);
> > > +		return -ENOTSUP;
> > > +	}
> > > +
> > > +	if (conf->lpbk_mode != 0) {
> > > +		PMD_INIT_LOG(ERR, "Loopback operation mode %d is not
> supported",
> > > +			     conf->lpbk_mode);
> > > +		return -ENOTSUP;
> > > +	}
> > > +
> > > +	if (conf->dcb_capability_en != 0) {
> > > +		PMD_INIT_LOG(ERR, "Priority Flow Control(PFC) if not
> supported");
> > > +		return -ENOTSUP;
> > > +	}
> > > +
> > > +	if (conf->intr_conf.lsc != 0) {
> > > +		PMD_INIT_LOG(ERR, "LSC interrupt is not supported");
> > > +		return -ENOTSUP;
> > > +	}
> > > +
> > > +	if (conf->intr_conf.rxq != 0) {
> > > +		PMD_INIT_LOG(ERR, "RXQ interrupt is not supported");
> > > +		return -ENOTSUP;
> > > +	}
> > > +
> > > +	if (conf->intr_conf.rmv != 0) {
> > > +		PMD_INIT_LOG(ERR, "RMV interrupt is not supported");
> > > +		return -ENOTSUP;
> > > +	}
> > > +
> > > +	return 0;
> >
> > This is '.dev_configure()' dev ops of a driver, there is nothing wrong
> > with the function but it is a good example to highlight a point.
> >
> >
> > 'rte_eth_dev_configure()' can fail from various reasons, what can an
> > application do in this case?
> > It is not clear why configuration failed, there is no way to figure
> > out failed config option dynamically.
> 
> There are some capabilities to read before calling "configure".
> 
> > Application developer can read the log and find out what caused the
> > failure, but what can do next? Put a conditional check for the
> > particular device, assuming application supports multiple devices,
> > before configuration?
> 
> Which failures cannot be guessed with capability flags?
> 
> > I think we need better error value, to help application detect what
> > went wrong and adapt dynamically, perhaps a bitmask of errors one per
> > each config option, what do you think?
> 
> I am not sure we can change such an old API.
> 
> > And I think this is another reason why we should not make a single API
> > too overloaded and complex.
> 
> Right, and I would support a work to have some of those "configure"
> features available as small functions.
>