On 10/17/2019 11:37 AM, Andrew Rybchenko wrote:
> On 10/16/19 9:07 PM, Ferruh Yigit wrote:
>> On 10/16/2019 4:46 PM, Ciara Power wrote:
>>> Adding promiscuous functions prevents sample applications failing when run
>>> on this virtual PMD. The sample applications call promiscuous functions,
>>> and fail if this function call returns an error, which occurs when the
>>> virtual PMD does not support the promiscuous function being called.
>>>
>>> This change will be implemented for all virtual PMDs that currently do not
>>> have existing promiscuous functions. Multicast functions will also be
>>> added for virtual PMDs to prevent sample application breakages here also.
>> +Andrew
>>
>> With the some ethdev APIs returning error code, some sample applications stop
>> working with virtual interfaces,
>>
>> We can,
>> 1- update sample applications to ignore the errors
>> 2- Add dummy dev_ops support to (almost all) virtual PMDs (what this RFC
>> suggests)
>>
>> (1) puts us back to before the ethdev APIs updated status, and this may be
>> wrong
>> for the physical devices case, so I am for this RFC.
>>
>> Only perhaps we can have some common empty function and keep assigning that
>> one
>> to reduce the dummy code, what do you think?
>
> I don't like the idea to have common empty/dummy functions.
> If virtual PMD behaves in accordance with enabled promiscuous mode,
> it should initialize it properly on init:
> eth_dev->data->promiscuous = 1;
> If so, if application requires promiscuous mode, attempt to enable will
> do nothing. If application requires non-promiscuous mode, disable will
> fail and it is good.
It is technically correct that we can't disable promiscuous mode in virtual PMDs
but I think mainly we don't really care so it returning error may make the
applications fail/exit unnecessarily with virtual PMDs.
>
>>> Signed-off-by: Ciara Power <ciara.po...@intel.com>
>>> ---
>>> drivers/net/null/rte_eth_null.c | 14 ++++++++++++++
>>> 1 file changed, 14 insertions(+)
>>>
>>> diff --git a/drivers/net/null/rte_eth_null.c
>>> b/drivers/net/null/rte_eth_null.c
>>> index e2ff41a22..b8472a0cf 100644
>>> --- a/drivers/net/null/rte_eth_null.c
>>> +++ b/drivers/net/null/rte_eth_null.c
>>> @@ -441,11 +441,25 @@ eth_mac_address_set(__rte_unused struct rte_eth_dev
>>> *dev,
>>> return 0;
>>> }
>>>
>>> +static int
>>> +eth_dev_promiscuous_enable(__rte_unused struct rte_eth_dev *dev)
>>> +{
>>> + return 0;
>>> +}
>>> +
>>> +static int
>>> +eth_dev_promiscuous_disable(__rte_unused struct rte_eth_dev *dev)
>>> +{
>>> + return 0;
>>> +}
>>> +
>>> static const struct eth_dev_ops ops = {
>>> .dev_start = eth_dev_start,
>>> .dev_stop = eth_dev_stop,
>>> .dev_configure = eth_dev_configure,
>>> .dev_infos_get = eth_dev_info,
>>> + .promiscuous_enable = eth_dev_promiscuous_enable,
>>> + .promiscuous_disable = eth_dev_promiscuous_disable,
>>> .rx_queue_setup = eth_rx_queue_setup,
>>> .tx_queue_setup = eth_tx_queue_setup,
>>> .rx_queue_release = eth_queue_release,
>>>
>
