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=-5.5 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, 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 AF194C49ED7 for ; Fri, 13 Sep 2019 21:05:23 +0000 (UTC) Received: from dpdk.org (dpdk.org [92.243.14.124]) by mail.kernel.org (Postfix) with ESMTP id 467BB20CC7 for ; Fri, 13 Sep 2019 21:05:23 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 467BB20CC7 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 53DC21F054; Fri, 13 Sep 2019 23:05:22 +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 D75541F040 for ; Fri, 13 Sep 2019 23:05:19 +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 9109F8006C; Fri, 13 Sep 2019 21:05:18 +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 22:05:13 +0100 To: Matan Azrad , Slava Ovsiienko CC: "dev@dpdk.org" 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> From: Andrew Rybchenko Message-ID: <16295f51-c9e7-c637-aab4-8aa67c382d63@solarflare.com> Date: Sat, 14 Sep 2019 00:05:09 +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: 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-11.257400-8.000000-10 X-TMASE-MatchedRID: eVEkOcJu0F7A46G+uSzVzdQ+QNegkd8z69aS+7/zbj+qvcIF1TcLYKxl G/oyG2Kc1bHHHwIutLj4mypBE9GvHAvpfXyH90tsGMURfhQkELJXjjsM2/Dfxkhcmj54ab4Uzv+ 8/a5ZVEEr9LvSTOZ+/usvuz7LvPbl9IKRKjO372G7B1QwzOcQD9i5W7Rf+s6QB2QWi8BF5SiArq oIZrVn1+BizFzQ56dqSYLQbFLvsnfhtVvI3rIgyfFanwH6mNos1JP9NndNOkVSMUnCl2ZytIggk HfdF/Q7fZ1wWAXO/vUS1GbvyBysMReiahjHbAoOTauf2PrRb1vOaxVMGFqGSqtNdpFrZXd8+HmB PyReSgxF77kuQaIYHpGTpe1iiCJq71zr0FZRMbBWdFebWIc3VsRB0bsfrpPIfiAqrjYtFiS6UN3 Qf+qhNgHt4Ed7VR2cvRtTnZXl8Ox2IKgXf2WRen7cGd19dSFd X-TM-AS-User-Approved-Sender: Yes X-TM-AS-User-Blocked-Sender: No X-TMASE-Result: 10--11.257400-8.000000 X-TMASE-Version: SMEX-12.5.0.1300-8.5.1010-24908.002 X-MDID: 1568408719-PM_6Fcb2ZVrA 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/10/19 10:53 AM, Matan Azrad wrote: > Hi > > Review for mlx5 part: > > Added not very important comment below. > You can stay it as is if no new version will be created. > > Acked-by: Matan Azrad Thank you, will fix in the next version. > Thanks. > > From: Andrew Rybchenko >> 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. >> >> drivers/net/mlx4/mlx4.h | 4 +- >> drivers/net/mlx4/mlx4_ethdev.c | 24 ++++++++--- >> drivers/net/mlx5/mlx5.h | 4 +- >> drivers/net/mlx5/mlx5_rxmode.c | 40 ++++++++++++++--- >> >> static void >> diff --git a/drivers/net/mlx4/mlx4.h b/drivers/net/mlx4/mlx4.h >> index 7730b530af..21517d70a2 100644 >> --- a/drivers/net/mlx4/mlx4.h >> +++ b/drivers/net/mlx4/mlx4.h >> @@ -205,8 +205,8 @@ int mlx4_mtu_get(struct mlx4_priv *priv, uint16_t >> *mtu); >> int mlx4_mtu_set(struct rte_eth_dev *dev, uint16_t mtu); >> int mlx4_dev_set_link_down(struct rte_eth_dev *dev); >> int mlx4_dev_set_link_up(struct rte_eth_dev *dev); >> -void mlx4_promiscuous_enable(struct rte_eth_dev *dev); >> -void mlx4_promiscuous_disable(struct rte_eth_dev *dev); >> +int mlx4_promiscuous_enable(struct rte_eth_dev *dev); >> +int mlx4_promiscuous_disable(struct rte_eth_dev *dev); >> void mlx4_allmulticast_enable(struct rte_eth_dev *dev); >> void mlx4_allmulticast_disable(struct rte_eth_dev *dev); >> void mlx4_mac_addr_remove(struct rte_eth_dev *dev, uint32_t index); >> diff --git a/drivers/net/mlx4/mlx4_ethdev.c >> b/drivers/net/mlx4/mlx4_ethdev.c >> index 623ebd88cb..c8a73bc1f4 100644 >> --- a/drivers/net/mlx4/mlx4_ethdev.c >> +++ b/drivers/net/mlx4/mlx4_ethdev.c >> @@ -341,13 +341,17 @@ enum rxmode_toggle { >> * Pointer to Ethernet device structure. >> * @param toggle >> * Toggle to set. >> + * >> + * @return >> + * 0 on success, a negative errno value otherwise and rte_errno is set. >> */ >> -static void >> +static int >> mlx4_rxmode_toggle(struct rte_eth_dev *dev, enum rxmode_toggle >> toggle) >> { >> struct mlx4_priv *priv = dev->data->dev_private; >> const char *mode; >> struct rte_flow_error error; >> + int ret; >> >> switch (toggle) { >> case RXMODE_TOGGLE_PROMISC_OFF: >> @@ -363,12 +367,16 @@ mlx4_rxmode_toggle(struct rte_eth_dev *dev, >> enum rxmode_toggle toggle) >> default: >> mode = "undefined"; >> } >> - if (!mlx4_flow_sync(priv, &error)) >> - return; >> + >> + ret = mlx4_flow_sync(priv, &error); >> + if (!ret) >> + return 0; >> + >> ERROR("cannot toggle %s mode (code %d, \"%s\")," >> " flow error type %d, cause %p, message: %s", >> mode, rte_errno, strerror(rte_errno), error.type, error.cause, >> error.message ? error.message : "(unspecified)"); >> + return ret; >> } >> >> /** >> @@ -376,8 +384,11 @@ mlx4_rxmode_toggle(struct rte_eth_dev *dev, >> enum rxmode_toggle toggle) >> * >> * @param dev >> * Pointer to Ethernet device structure. >> + * >> + * @return >> + * 0 on success, a negative errno value otherwise and rte_errno is set. >> */ >> -void >> +int >> mlx4_promiscuous_enable(struct rte_eth_dev *dev) >> { >> mlx4_rxmode_toggle(dev, RXMODE_TOGGLE_PROMISC_ON); >> @@ -388,8 +399,11 @@ mlx4_promiscuous_enable(struct rte_eth_dev >> *dev) >> * >> * @param dev >> * Pointer to Ethernet device structure. >> + * >> + * @return >> + * 0 on success, a negative errno value otherwise and rte_errno is set. >> */ >> -void >> +int >> mlx4_promiscuous_disable(struct rte_eth_dev *dev) >> { >> mlx4_rxmode_toggle(dev, RXMODE_TOGGLE_PROMISC_OFF); >> diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h >> index 8ddbb7a17c..11d540c3a5 100644 >> --- a/drivers/net/mlx5/mlx5.h >> +++ b/drivers/net/mlx5/mlx5.h >> @@ -752,8 +752,8 @@ int mlx5_dev_rss_reta_update(struct rte_eth_dev >> *dev, >> >> /* mlx5_rxmode.c */ >> >> -void mlx5_promiscuous_enable(struct rte_eth_dev *dev); >> -void mlx5_promiscuous_disable(struct rte_eth_dev *dev); >> +int mlx5_promiscuous_enable(struct rte_eth_dev *dev); >> +int mlx5_promiscuous_disable(struct rte_eth_dev *dev); >> void mlx5_allmulticast_enable(struct rte_eth_dev *dev); >> void mlx5_allmulticast_disable(struct rte_eth_dev *dev); >> >> diff --git a/drivers/net/mlx5/mlx5_rxmode.c >> b/drivers/net/mlx5/mlx5_rxmode.c >> index d5077db0db..c862fc9520 100644 >> --- a/drivers/net/mlx5/mlx5_rxmode.c >> +++ b/drivers/net/mlx5/mlx5_rxmode.c >> @@ -28,8 +28,11 @@ >> * >> * @param dev >> * Pointer to Ethernet device structure. >> + * >> + * @return >> + * 0 on success, a negative errno value otherwise and rte_errno is set. >> */ >> -void >> +int >> mlx5_promiscuous_enable(struct rte_eth_dev *dev) >> { >> struct mlx5_priv *priv = dev->data->dev_private; >> @@ -41,14 +44,24 @@ mlx5_promiscuous_enable(struct rte_eth_dev *dev) >> "port %u cannot enable promiscuous mode" >> " in flow isolation mode", >> dev->data->port_id); >> - return; >> + return 0; >> + } >> + if (priv->config.vf) { >> + ret = mlx5_nl_promisc(dev, 1); >> + if (ret) >> + goto error; > No need the jump, just return ret here and below. > >> } >> - if (priv->config.vf) >> - mlx5_nl_promisc(dev, 1); >> ret = mlx5_traffic_restart(dev); >> if (ret) >> DRV_LOG(ERR, "port %u cannot enable promiscuous mode: >> %s", >> dev->data->port_id, strerror(rte_errno)); >> + >> +error: >> + /* >> + * rte_eth_dev_promiscuous_enable() rollback >> + * dev->data->promiscuous in the case of failure. >> + */ >> + return ret; >> } >> >> /** >> @@ -56,20 +69,33 @@ mlx5_promiscuous_enable(struct rte_eth_dev *dev) >> * >> * @param dev >> * Pointer to Ethernet device structure. >> + * >> + * @return >> + * 0 on success, a negative errno value otherwise and rte_errno is set. >> */ >> -void >> +int >> mlx5_promiscuous_disable(struct rte_eth_dev *dev) >> { >> struct mlx5_priv *priv = dev->data->dev_private; >> int ret; >> >> dev->data->promiscuous = 0; >> - if (priv->config.vf) >> - mlx5_nl_promisc(dev, 0); >> + if (priv->config.vf) { >> + ret = mlx5_nl_promisc(dev, 0); >> + if (ret) >> + goto error; > Same here. > >> + } >> ret = mlx5_traffic_restart(dev); >> if (ret) >> DRV_LOG(ERR, "port %u cannot disable promiscuous mode: >> %s", >> dev->data->port_id, strerror(rte_errno)); >> + >> +error: >> + /* >> + * rte_eth_dev_promiscuous_disable() rollback >> + * dev->data->promiscuous in the case of failure. >> + */ >> + return ret; >> } >> >> /**