From: Andrew Rybchenko <arybchenko@solarflare.com>
To: Ferruh Yigit <ferruh.yigit@intel.com>,
Bruce Richardson <bruce.richardson@intel.com>
Cc: Ciara Power <ciara.power@intel.com>, <mtetsuyah@gmail.com>,
<dev@dpdk.org>, Thomas Monjalon <thomas@monjalon.net>
Subject: Re: [dpdk-dev] [RFC] net/null: add empty promiscuous mode functions
Date: Fri, 18 Oct 2019 11:30:18 +0300 [thread overview]
Message-ID: <0752225f-fbf1-a115-77be-72d6ff199bfa@solarflare.com> (raw)
In-Reply-To: <bc9009a4-d406-4c1d-1e41-4acf3aeedc20@intel.com>
On 10/18/19 11:18 AM, Ferruh Yigit wrote:
> On 10/17/2019 4:33 PM, Ferruh Yigit wrote:
>> On 10/17/2019 2:43 PM, Bruce Richardson wrote:
>>> On Thu, Oct 17, 2019 at 12:05:56PM +0100, Ferruh Yigit wrote:
>>>> On 10/17/2019 11:51 AM, Andrew Rybchenko wrote:
>>>>> On 10/17/19 1:47 PM, Ferruh Yigit wrote:
>>>>>> 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.
>>>>> If I test virtual PMD promiscuous mode, I would prefer enable/disable
>>>>> callback to say me truth.
>>>>>
>>>>> If application really does not care, it should be in the application code.
>>>> Application can't change this because they may be caring return result for the
>>>> physical devices.
>>>>
>>>> Up until this release these missing dev_ops in virtual PMDs were silently
>>>> ignored, now APIs are more strict on this (which is good) but to get close the
>>>> previous behavior for virtual PMDs we need to relax on these features (like
>>>> saying success on promiscuous disable although it didn't).
>>>>
>>> The other variable here is how often an app is going to request promiscuous
>>> disabling? Given that most ports generally come up in that state anyway,
>>> and one needs to request enabling it, surely the disable case is relatively
>>> rare? In that case I'd tend to agree with having disabling it returning
>>> error for vpmds.
>>>
>> Yes disabling most probably rare, but still it will generate an error and
>> application is failing because of ring PMD promiscuous disable doesn't look
>> right to me.
>>
>> Perhaps application should differentiate between -ENOTSUP error and operation
>> fail error, but that looks to me adding unnecessary complexity to the app.
>>
>> With a common function shared by all PMDs for both promisc and allmuticast will
>> add a little code and an easier solution.
>>
> btw, initialize promiscuous as enabled at PMD init won't help with current APIs
> because in API dev_ops check is earlier and will still cause -ENOTSUP.
My bad, I think it should be fixed.
> rte_eth_promiscuous_enable
> RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->promiscuous_enable, -ENOTSUP);
> if (dev->data->promiscuous == 0)
> diag = (*dev->dev_ops->promiscuous_enable)(dev);
> dev->data->promiscuous = (diag == 0) ? 1 : 0;
> return
next prev parent reply other threads:[~2019-10-18 8:30 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-16 15:46 Ciara Power
2019-10-16 18:07 ` Ferruh Yigit
2019-10-17 10:37 ` Andrew Rybchenko
2019-10-17 10:47 ` Ferruh Yigit
2019-10-17 10:51 ` Andrew Rybchenko
2019-10-17 11:05 ` Ferruh Yigit
2019-10-17 13:43 ` Bruce Richardson
2019-10-17 15:33 ` Ferruh Yigit
2019-10-18 8:18 ` Ferruh Yigit
2019-10-18 8:30 ` Andrew Rybchenko [this message]
2019-10-18 10:13 ` Bruce Richardson
2019-10-18 11:38 ` Ferruh Yigit
2019-10-18 11:57 ` Andrew Rybchenko
2019-10-18 13:02 ` Ferruh Yigit
2019-10-18 13:12 ` Bruce Richardson
2019-10-18 13:38 ` Ferruh Yigit
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=0752225f-fbf1-a115-77be-72d6ff199bfa@solarflare.com \
--to=arybchenko@solarflare.com \
--cc=bruce.richardson@intel.com \
--cc=ciara.power@intel.com \
--cc=dev@dpdk.org \
--cc=ferruh.yigit@intel.com \
--cc=mtetsuyah@gmail.com \
--cc=thomas@monjalon.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).