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 <tho...@monjalon.net>
> Sent: Monday, February 27, 2023 11:46 PM
> To: Andrew Rybchenko <andrew.rybche...@oktetlabs.ru>; Jerin Jacob
> Kollanukkaran <jer...@marvell.com>; Zhang, Qi Z <qi.z.zh...@intel.com>;
> David Marchand <david.march...@redhat.com>; Ferruh Yigit
> <ferruh.yi...@amd.com>
> Cc: dev@dpdk.org; Liu, Mingxia <mingxia....@intel.com>; Zhang, Yuying
> <yuying.zh...@intel.com>; Xing, Beilei <beilei.x...@intel.com>;
> techbo...@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.
>
