From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.5 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6910CC4CEC9 for ; Fri, 13 Sep 2019 19:58:14 +0000 (UTC) Received: from dpdk.org (dpdk.org [92.243.14.124]) by mail.kernel.org (Postfix) with ESMTP id E5C0620692 for ; Fri, 13 Sep 2019 19:58:13 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org E5C0620692 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=solarflare.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=dev-bounces@dpdk.org Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 4A81B1EAD6; Fri, 13 Sep 2019 21:58:12 +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 4B1901C1C7 for ; Fri, 13 Sep 2019 21:58:11 +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-us3.ppe-hosted.com (PPE Hosted ESMTP Server) with ESMTPS id 9F073B80065; Fri, 13 Sep 2019 19:58:06 +0000 (UTC) Received: from [192.168.1.192] (188.242.181.57) by ukex01.SolarFlarecom.com (10.17.10.4) with Microsoft SMTP Server (TLS) id 15.0.1395.4; Fri, 13 Sep 2019 20:57:43 +0100 To: Ferruh Yigit , "John W. Linville" , Xiaolong Ye , Qi Zhang , Igor Russkikh , "Pavel Belous" , Allain Legacy , Matt Peters , "Ravi Kumar" , Rasesh Mody , Shahed Shaikh , Ajit Khaparde , "Somnath Kotur" , Chas Williams , "Rahul Lakkireddy" , Hemant Agrawal , Sachin Saxena , Wenzhuo Lu , Gagandeep Singh , John Daley , Hyong Youb Kim , Gaetan Rivet , Xiao Wang , Ziyang Xuan , Xiaoyun Wang , Guoyang Zhou , Beilei Xing , Jingjing Wu , Qiming Yang , Rosen Xu , Konstantin Ananyev , Shijith Thotton , Srisivasubramanian Srinivasan , Matan Azrad , Shahaf Shuler , Yongseok Koh , Viacheslav Ovsiienko , "Zyta Szpak" , Liron Himi , Tomasz Duszynski , Stephen Hemminger , "K. Y. Srinivasan" , Haiyang Zhang , Rastislav Cernay , Jan Remes , Alejandro Lucero , Jerin Jacob , Nithin Dabilpuram , "Kiran Kumar K" , Keith Wiles , Maciej Czekaj , Maxime Coquelin , Tiwei Bie , Zhihong Wang , Yong Wang , Thomas Monjalon CC: References: <1567699852-31693-1-git-send-email-arybchenko@solarflare.com> <1568030331-16526-1-git-send-email-arybchenko@solarflare.com> <1568030331-16526-5-git-send-email-arybchenko@solarflare.com> <5093c27d-4251-e997-f4bb-d1f56a60443a@intel.com> <0aec5016-2130-26f9-dfd6-f65222514ced@solarflare.com> <1c8c01b2-b244-5cf9-8687-9ee897c73f7c@intel.com> From: Andrew Rybchenko Message-ID: <60e4ec91-7f2c-890a-b8d6-237f09b17c11@solarflare.com> Date: Fri, 13 Sep 2019 22:57:31 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 MIME-Version: 1.0 In-Reply-To: <1c8c01b2-b244-5cf9-8687-9ee897c73f7c@intel.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-US X-Originating-IP: [188.242.181.57] 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-24908.002 X-TM-AS-Result: No-4.919000-8.000000-10 X-TMASE-MatchedRID: QfHZjzml1E/A46G+uSzVzSLVdThWsHxY69aS+7/zbj+qvcIF1TcLYM7/ vP2uWVRBK/S70kzmfv7rL7s+y7z25R1YpEPWJiyzqJSK+HSPY+/pVMb1xnESMrZk7gsuflVKhWD 4TtDcfzQe27BljbjQO7q5QJeil1hr9+5WkBOeNwd2GcWKGZufBam9/6ObPjnDEdZrmoJ0iw1576 my5IxjuqhPDwZeB+AQRPk8NBFRp6qRwaU8+7ToHvKR06Kw3DzKUg5zxCPHJW2kiBq4w8bMxaPFj JEFr+olA9Mriq0CDAgBi3kqJOK62QtuKBGekqUpnH7sbImOEBSCiBdBesYlRxUEvyyrCq6Bn3yk qmNtlIRz4CoemfqrXkyDB5SdC4sSlaZDeahzFvF2skDl/2L20AAo3urHBczBa+WDHcJTeFmHzGT HoCwyHhlNKSp2rPkW5wiX7RWZGYs2CWDRVNNHu2A7bUFBqh2V X-TM-AS-User-Approved-Sender: Yes X-TM-AS-User-Blocked-Sender: No X-TMASE-Result: 10--4.919000-8.000000 X-TMASE-Version: SMEX-12.5.0.1300-8.5.1010-24908.002 X-MDID: 1568404690-F-1X0wMqJ600 Subject: Re: [dpdk-dev] [PATCH v2 04/13] ethdev: change promiscuous callbacks to return status 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 9/13/19 7:34 PM, Ferruh Yigit wrote: > On 9/13/2019 5:05 PM, Andrew Rybchenko wrote: >> On 9/13/19 6:39 PM, Ferruh Yigit wrote: >>> On 9/9/2019 12:58 PM, Andrew Rybchenko wrote: >>>> Enabling/disabling of promiscuous mode is not always successful and >>>> it should be taken into account to be able to handle it properly. >>>> >>>> When correct return status is unclear from driver code, -EAGAIN is used. >>>> >>>> Signed-off-by: Andrew Rybchenko >>> <...> >>> >>>> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c >>>> index f85458c3cd..41612ce838 100644 >>>> --- a/drivers/net/tap/rte_eth_tap.c >>>> +++ b/drivers/net/tap/rte_eth_tap.c >>>> @@ -1100,28 +1100,60 @@ tap_link_update(struct rte_eth_dev *dev, int wait_to_complete __rte_unused) >>>> return 0; >>>> } >>>> >>>> -static void >>>> +static int >>>> tap_promisc_enable(struct rte_eth_dev *dev) >>>> { >>>> struct pmd_internals *pmd = dev->data->dev_private; >>>> struct ifreq ifr = { .ifr_flags = IFF_PROMISC }; >>>> + int ret; >>>> >>>> - dev->data->promiscuous = 1; >>>> - tap_ioctl(pmd, SIOCSIFFLAGS, &ifr, 1, LOCAL_AND_REMOTE); >>>> - if (pmd->remote_if_index && !pmd->flow_isolate) >>>> - tap_flow_implicit_create(pmd, TAP_REMOTE_PROMISC); >>>> + ret = tap_ioctl(pmd, SIOCSIFFLAGS, &ifr, 1, LOCAL_AND_REMOTE); >>>> + if (ret != 0) >>>> + return ret; >>>> + >>>> + if (pmd->remote_if_index && !pmd->flow_isolate) { >>>> + dev->data->promiscuous = 1; >>> I think PMD shouldn't be setting this variable, it is already set by the API. >>> I quickly checked if an internal function requires this but it looks like it has >>> been set by mistake, I think we can remove this. >> It is set after callback in the case of enable. > I see, but do we need it enabled earlier? Not sure that I understand the question, but tap_ioctl() does not use it. So, it is safe to move just before tap_flow_implicit_create(). >>>> + ret = tap_flow_implicit_create(pmd, TAP_REMOTE_PROMISC); >>>> + if (ret != 0) { >>>> + /* Rollback promisc flag */ >>>> + tap_ioctl(pmd, SIOCSIFFLAGS, &ifr, 0, LOCAL_AND_REMOTE); >>>> + /* >>>> + * rte_eth_dev_promiscuous_enable() rollback >>>> + * dev->data->promiscuous in the case of failure. >>>> + */ >>>> + return ret; >>>> + } >>>> + } >>>> + >>>> + return 0; >>>> } >>>> >>>> -static void >>>> +static int >>>> tap_promisc_disable(struct rte_eth_dev *dev) >>>> { >>>> struct pmd_internals *pmd = dev->data->dev_private; >>>> struct ifreq ifr = { .ifr_flags = IFF_PROMISC }; >>>> + int ret; >>>> >>>> - dev->data->promiscuous = 0; >>>> - tap_ioctl(pmd, SIOCSIFFLAGS, &ifr, 0, LOCAL_AND_REMOTE); >>>> - if (pmd->remote_if_index && !pmd->flow_isolate) >>>> - tap_flow_implicit_destroy(pmd, TAP_REMOTE_PROMISC); >>>> + ret = tap_ioctl(pmd, SIOCSIFFLAGS, &ifr, 0, LOCAL_AND_REMOTE); >>>> + if (ret != 0) >>>> + return ret; >>>> + >>>> + if (pmd->remote_if_index && !pmd->flow_isolate) { >>>> + dev->data->promiscuous = 0; >>> Ditto >>> >>>> + ret = tap_flow_implicit_destroy(pmd, TAP_REMOTE_PROMISC); >>>> + if (ret != 0) { >>>> + /* Rollback promisc flag */ >>>> + tap_ioctl(pmd, SIOCSIFFLAGS, &ifr, 1, LOCAL_AND_REMOTE); >>>> + /* >>>> + * rte_eth_dev_promiscuous_disable() rollback >>>> + * dev->data->promiscuous in the case of failure. >>>> + */ >>>> + return ret; >>>> + } >>>> + } >>>> + >>>> + return 0; >>>> }