From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 051B8A3179 for ; Thu, 17 Oct 2019 12:51:51 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id C16011E93B; Thu, 17 Oct 2019 12:51:50 +0200 (CEST) Received: from dispatch1-us1.ppe-hosted.com (dispatch1-us1.ppe-hosted.com [67.231.154.164]) by dpdk.org (Postfix) with ESMTP id 898271E938 for ; Thu, 17 Oct 2019 12:51:49 +0200 (CEST) X-Virus-Scanned: Proofpoint Essentials engine Received: from webmail.solarflare.com (uk.solarflare.com [193.34.186.16]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by mx1-us2.ppe-hosted.com (PPE Hosted ESMTP Server) with ESMTPS id 1335B8006D; Thu, 17 Oct 2019 10:51:40 +0000 (UTC) Received: from [192.168.38.17] (91.220.146.112) by ukex01.SolarFlarecom.com (10.17.10.4) with Microsoft SMTP Server (TLS) id 15.0.1395.4; Thu, 17 Oct 2019 11:51:33 +0100 To: Ferruh Yigit , Ciara Power , CC: , Thomas Monjalon References: <20191016154606.39218-1-ciara.power@intel.com> <5b6af628-7101-484b-01db-16272025105f@solarflare.com> From: Andrew Rybchenko Message-ID: Date: Thu, 17 Oct 2019 13:51:29 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.9.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-GB X-Originating-IP: [91.220.146.112] X-ClientProxiedBy: ocex03.SolarFlarecom.com (10.20.40.36) To ukex01.SolarFlarecom.com (10.17.10.4) X-TM-AS-Product-Ver: SMEX-12.5.0.1300-8.5.1010-24982.003 X-TM-AS-Result: No-5.278300-8.000000-10 X-TMASE-MatchedRID: gzVbiXtWD9vmLzc6AOD8DfHkpkyUphL9cVr+FAe3UDUOOOIzzESoE9xw X69jh9hhgLpjL6KnVPOcxc73N69MaYyQUjhAi5+qMpVOsYwN78Py++SyyVe4t70rWM4nIpJrn68 UTiD6dZhh9M64k1dYu85frr4hnN1KgxRTSLSLS/xwju9EALAXQjVvI0Ic6AC1kaEC8FJraL/haP 2JoKUCSe1VmHwWGO0bRodo9/GXhRjNnWwqhmCQdczSKGx9g8xhfkuZtv/FS5qfcVJ8cL8ZOztWz Je6wcdX9MvSNF22SK0L11Qsi8d5hsaZnhgjGx37IAjxomarSPAvsOOmgOo1mW5AmxiyBtC4G/+u OKd48Qdice309HKOwgz83tK7FFNg+1P6Z0rX8IGolIr4dI9j7+lUxvXGcRIykY8eITaSJPhAHOg 8qEtqyNoUzlM8Yt0IKcCLQaAlV2PZfZ+ej5sCRub3p4cnIXGNSoCG4sefl8RBbp4JobErApbVGh 42G3hF585VzGMOFzABi3kqJOK62QtuKBGekqUpnH7sbImOEBSw6BIeqvhnJoHjy0/VcfGElaNgR /tLubpMHgyns/auj9gMJ75nJ45jUt/K7xh62Yu96HFUITQKqCY8U9ywjDXnRcc312QRDNuHzGTH oCwyHhlNKSp2rPkW5wiX7RWZGYs2CWDRVNNHu2A7bUFBqh2V X-TM-AS-User-Approved-Sender: Yes X-TM-AS-User-Blocked-Sender: No X-TMASE-Result: 10--5.278300-8.000000 X-TMASE-Version: SMEX-12.5.0.1300-8.5.1010-24982.003 X-MDID: 1571309506-TXTysprgszbW Subject: Re: [dpdk-dev] [RFC] net/null: add empty promiscuous mode functions X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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. >>>> Signed-off-by: Ciara Power >>>> --- >>>> 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, >>>>