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 ECD76A2EEB for ; Fri, 13 Sep 2019 17:55:27 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id B9B3D1F06A; Fri, 13 Sep 2019 17:55:26 +0200 (CEST) Received: from dispatch1-us1.ppe-hosted.com (dispatch1-us1.ppe-hosted.com [148.163.129.52]) by dpdk.org (Postfix) with ESMTP id ACCC91F05F for ; Fri, 13 Sep 2019 17:55:24 +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-us4.ppe-hosted.com (PPE Hosted ESMTP Server) with ESMTPS id 8C7514C007D; Fri, 13 Sep 2019 15:55:20 +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; Fri, 13 Sep 2019 16:54:53 +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> <7415b465-bcc5-71b4-1a2c-fda5fe359182@intel.com> From: Andrew Rybchenko Message-ID: Date: Fri, 13 Sep 2019 18:54:49 +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: <7415b465-bcc5-71b4-1a2c-fda5fe359182@intel.com> 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-24908.002 X-TM-AS-Result: No-8.236000-8.000000-10 X-TMASE-MatchedRID: gjZGo2H/wj/A46G+uSzVzSLVdThWsHxY69aS+7/zbj+qvcIF1TcLYM7/ vP2uWVRBK/S70kzmfv7rL7s+y7z25R1YpEPWJiyzPwKTD1v8YV4P6OWzRxLAk2AMM0WKD4asE4J phT5oKDTtf01K5STsIGWT7yUCxZYzdyFOLWt/1U68coKUcaOOvTxWJr0lgcJAgkyVL6QwZtx7+n Bu7fbl6lOMZ3bzSu1KFIyjaBEWAakT/rs24a2knzKVTrGMDe/DYLBTqCjiZ+aFlw4432id29AYW Uo4HSIkgkbNOZ/kfLupaNKvECOILC5SJ1mHWOoNimHWEC28pk1qxNo3a1iwD9EsTITobgNEH0Gq 6gh+ZlCGNFHQUYLxIRfRej1lriKkQm0CRa3IZy7BtFDYGmaWKjcy5Kr1d07TDC/Vm90If4UVcJR 4XudE5kXMeswpFkgKHcN4x+ltYL1zySdTkJm1SnEFgt4Wa1LtqmRbFDx9ddDZhqPLU/1bVd8mfx iZsPBGnyoHgIDVtNRTep303c9PgN9faxl/I4mhngIgpj8eDcC063Wh9WVqgtQdB5NUNSsi1GcRA JRT6POUlhCENklet7KsOPspnbwT2tkWsrRBh5oSN4tHUUmsxP2j4ercpx6lKsSaV+UlQAGqI5D4 dG9KcfLJN++y4o+S6geAKit+P7gYu0DNhY4GPwLIUzjlCcsu1lSvyMs+imqxgGBpwu6JdilLuHf wVi03VpjQvmWEqNldAgwYiy8q3A9KELnpt+YbdhRyrkI0FLbGI3gPDvMGGw== X-TM-AS-User-Approved-Sender: Yes X-TM-AS-User-Blocked-Sender: No X-TMASE-Result: 10--8.236000-8.000000 X-TMASE-Version: SMEX-12.5.0.1300-8.5.1010-24908.002 X-MDID: 1568390123-wOtwpY1_NvI2 Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit X-Content-Filtered-By: Mailman/MimeDel 2.1.15 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 6:34 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/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c >> index b97dd8aa85..f2e6b4c83b 100644 >> --- a/lib/librte_ethdev/rte_ethdev.c >> +++ b/lib/librte_ethdev/rte_ethdev.c >> @@ -1892,30 +1892,38 @@ int >> rte_eth_promiscuous_enable(uint16_t port_id) >> { >> struct rte_eth_dev *dev; >> + uint8_t old_promiscuous; >> + int diag; >> >> RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); >> dev = &rte_eth_devices[port_id]; >> >> RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->promiscuous_enable, -ENOTSUP); >> - (*dev->dev_ops->promiscuous_enable)(dev); >> - dev->data->promiscuous = 1; >> + old_promiscuous = dev->data->promiscuous; >> + diag = (*dev->dev_ops->promiscuous_enable)(dev); >> + dev->data->promiscuous = (diag == 0) ? 1 : old_promiscuous; >> >> - return 0; >> + return eth_err(port_id, diag); >> } >> >> int >> rte_eth_promiscuous_disable(uint16_t port_id) >> { >> struct rte_eth_dev *dev; >> + uint8_t old_promiscuous; >> + int diag; >> >> RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); >> dev = &rte_eth_devices[port_id]; >> >> RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->promiscuous_disable, -ENOTSUP); >> + old_promiscuous = dev->data->promiscuous; >> dev->data->promiscuous = 0; >> - (*dev->dev_ops->promiscuous_disable)(dev); >> + diag = (*dev->dev_ops->promiscuous_disable)(dev); >> + if (diag != 0) >> + dev->data->promiscuous = old_promiscuous; > Not a real issue, but the enable/disable does exact same thing, slightly > different way, it makes double check if logic is different, > can you adapt one of them for both please. > > " > diag = foo(); > dev->data->promiscuous = (diag == 0) ? 1 : old_promiscuous; > " > vs > > " > dev->data->promiscuous = 0; > diag = bar(); > if (diag != 0) > dev->data->promiscuous = old_promiscuous; > " I simply did not want to touch the logic in big patch series. Don't know why, but enable sets promiscuous=1 after callback, but disable does it before callback. That's why the difference. Logically it is a separate change and I definitely don't want to mix it. >> >> - return 0; >> + return eth_err(port_id, diag); >> } >> >> int >> diff --git a/lib/librte_ethdev/rte_ethdev_core.h b/lib/librte_ethdev/rte_ethdev_core.h >> index 2394b32c83..6322348d17 100644 >> --- a/lib/librte_ethdev/rte_ethdev_core.h >> +++ b/lib/librte_ethdev/rte_ethdev_core.h >> @@ -52,10 +52,10 @@ typedef int (*eth_dev_reset_t)(struct rte_eth_dev *dev); >> typedef int (*eth_is_removed_t)(struct rte_eth_dev *dev); >> /**< @internal Function used to detect an Ethernet device removal. */ >> >> -typedef void (*eth_promiscuous_enable_t)(struct rte_eth_dev *dev); >> +typedef int (*eth_promiscuous_enable_t)(struct rte_eth_dev *dev); >> /**< @internal Function used to enable the RX promiscuous mode of an Ethernet device. */ >> >> -typedef void (*eth_promiscuous_disable_t)(struct rte_eth_dev *dev); >> +typedef int (*eth_promiscuous_disable_t)(struct rte_eth_dev *dev); >> /**< @internal Function used to disable the RX promiscuous mode of an Ethernet device. */ > Should we add a note what is expected return value? This information is not > available anyplace, it may help driver developers. Makes sense.