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. >