On 2/28/2023 2:06 AM, Liu, Mingxia wrote:
> 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.
>
Hi Mingxia,
The discussion is not specific to the driver, it is for ethdev API and
for long term.
>> -----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.
>>
>
