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 9955CA318B for ; Fri, 18 Oct 2019 10:30:27 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id C1AC51C11D; Fri, 18 Oct 2019 10:30:26 +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 CF1A31C0C1 for ; Fri, 18 Oct 2019 10:30:25 +0200 (CEST) X-Virus-Scanned: Proofpoint Essentials engine Received: from webmail.solarflare.com (webmail.solarflare.com [12.187.104.26]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by mx1-us4.ppe-hosted.com (PPE Hosted ESMTP Server) with ESMTPS id 717EEB4005B; Fri, 18 Oct 2019 08:30:24 +0000 (UTC) Received: from [192.168.38.17] (91.220.146.112) by ocex03.SolarFlarecom.com (10.20.40.36) with Microsoft SMTP Server (TLS) id 15.0.1395.4; Fri, 18 Oct 2019 01:30:20 -0700 To: Ferruh Yigit , Bruce Richardson CC: Ciara Power , , , Thomas Monjalon References: <20191016154606.39218-1-ciara.power@intel.com> <5b6af628-7101-484b-01db-16272025105f@solarflare.com> <04b0cc0a-633f-212e-22ab-147b7f3e6a1a@intel.com> <20191017134341.GA912@bricha3-MOBL.ger.corp.intel.com> <983d5c2f-e287-ccc2-5d7d-0b56e9b2da3a@intel.com> From: Andrew Rybchenko Message-ID: <0752225f-fbf1-a115-77be-72d6ff199bfa@solarflare.com> Date: Fri, 18 Oct 2019 11:30:18 +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 ocex03.SolarFlarecom.com (10.20.40.36) X-TM-AS-Product-Ver: SMEX-12.5.0.1300-8.5.1010-24984.005 X-TM-AS-Result: No-12.807000-4.000000-10 X-TMASE-MatchedRID: qsaWi0FWcYvmLzc6AOD8DfHkpkyUphL91mc05rOhnpLNWDA/tkxh/1Tm giBTbJevOj3xN8utVCedA5Lw7x/MKRTynB8ACpSldhnFihmbnwXLRD51bz5RZPeawpJ3WDjF2jN t44OdemViveUebyLf6LblYix0V8kYIg67HHizFeGie2FBq5CMBrvGYJkNeu61kOBHeeXGAHrv6F j2Xtj47Acm4twMjJx/NJzOzmWbO4QGIaHm10uKh0rM69p7lDSspYXNuN0M8fXkMnUVL5d0E7aGt +9v8VND0fYfjeKjgllSfaDNVJL5Shy/DZ8tz1CLQuFiD+xrWCwP6OWzRxLAk4pc3JtqeiRP2YYj bHXT/UmCXoRNGj51FxRePlympFUe2gl/2oj6CXRl2ityh8f8afTWYbN2f76BWoQBC/aPk1uCP1L TUo0L+LQ/6v1d5HILwzvsWxCP1hZR9D0WPDCIuLrbxxduc6FPqUdpDBnLMO07FE26mju9Ox0Grn AdMXjaNhw+wGY62+KzPYUrKL7w8Fvmaxq5YzoOZ93oz43dfXFBmlBF/IJ0fMnZhDdFIXZEp56Aj xnx9+xVnff0AuJXvZcQ82BPM5lFetI/1G0ZligdyHiBnyiVv//dUP8atA+/myiLZetSf8my5/tF Zu9S3Ku6xVHLhqfx33fj+sMArfOEbaqKQSlAZaBkusDmid3dfkgpzNQUSNg4gWJNfZscx2nqBDi /NgwwDimh5zvxMX8= X-TM-AS-User-Approved-Sender: No X-TM-AS-User-Blocked-Sender: No X-TMASE-Result: 10--12.807000-4.000000 X-TMASE-Version: SMEX-12.5.0.1300-8.5.1010-24984.005 X-MDID: 1571387425-4k9Ttovre29N 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/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