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 EA08CA2E8D for ; Thu, 5 Sep 2019 18:32:32 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 79B461F054; Thu, 5 Sep 2019 18:32:32 +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 2404E1BF5A for ; Thu, 5 Sep 2019 18:32:31 +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 96F92980053; Thu, 5 Sep 2019 16:32:29 +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, 5 Sep 2019 17:32:24 +0100 To: =?UTF-8?Q?Ga=c3=abtan_Rivet?= CC: , Ivan Ilchenko References: <1567699852-31693-1-git-send-email-arybchenko@solarflare.com> <1567699852-31693-3-git-send-email-arybchenko@solarflare.com> <20190905162505.fsyabet322zymtj7@bidouze.vm.6wind.com> From: Andrew Rybchenko Message-ID: <05bf6ed3-2290-75ed-3199-99b6a24bfa56@solarflare.com> Date: Thu, 5 Sep 2019 19:32:20 +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: <20190905162505.fsyabet322zymtj7@bidouze.vm.6wind.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-24890.003 X-TM-AS-Result: No-9.445400-8.000000-10 X-TMASE-MatchedRID: 6lay9u8oTUO8rRvefcjeTQR6aubfrqNo69aS+7/zbj+qvcIF1TcLYLLX y+lnwmMx2YzWddhUqRa/UScO8V00kiw76RUR9W5CT7O/YHJhINB/9Mg6o+wMiwQsw9A3PIlLyJN a6DYLgM24oPkqxH17jHDlPghqPnfyYlldA0POS1ICg1rav4R3DS9Xl/s/QdUMbPfGfHxv+pG/zr JDu2kS5xdWqetdJs3imLib1iTz4lOMkFI4QIufqjhiciQR/+DQer1wlVoR6TRpsnGGIgWMmeqE5 42nLZnupMD5hFR2A2q0mUNgT7cCJEQb08eHfFUF9VjtTc1fwmB8yGO3dvk8/VcbfIj2Ta9sP84r 9LuHKWGNKIBh68PaAWuRi/mMYtyx6BQEtCkk8tIvj6wHfIGxyVg46Sze14QB2Yajy1P9W1XSUaS e3PSxAfdoK1MEIGGXeTjw/FyRX6S/WXZS/HqJ2lZ0V5tYhzdWbGVEmIfjf3sI+sr2AjRqC4pMnj uWKmKuJJ4ZjTTr0bjeTCgbejB0oAq0rwwD1TnJ9u0Vpq3PAZCGI4KunPJeegAQnemP+v90Q9s0A l5IFS0= X-TM-AS-User-Approved-Sender: Yes X-TM-AS-User-Blocked-Sender: No X-TMASE-Result: 10--9.445400-8.000000 X-TMASE-Version: SMEX-12.5.0.1300-8.5.1010-24890.003 X-MDID: 1567701150-L5TNW3Unzj6P Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 8bit X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [PATCH 02/13] net/failsafe: check code of promiscuous mode switch 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/5/19 7:25 PM, Gaëtan Rivet wrote: > Hi, > > On Thu, Sep 05, 2019 at 05:10:40PM +0100, Andrew Rybchenko wrote: >> From: Ivan Ilchenko >> >> rte_eth_promiscuous_enable()/rte_eth_promiscuous_disable() return >> value was changed from void to int, so this patch modify usage >> of these functions across net/failsafe according to new return type. >> >> Try to keep promiscuous mode consistent across all active >> devices in the case of failure. >> >> Signed-off-by: Ivan Ilchenko >> Signed-off-by: Andrew Rybchenko >> --- >> drivers/net/failsafe/failsafe_ether.c | 8 +++-- >> drivers/net/failsafe/failsafe_ops.c | 44 ++++++++++++++++++++++++--- >> 2 files changed, 46 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/net/failsafe/failsafe_ether.c b/drivers/net/failsafe/failsafe_ether.c >> index 504c76edb0..bd38f1c1e4 100644 >> --- a/drivers/net/failsafe/failsafe_ether.c >> +++ b/drivers/net/failsafe/failsafe_ether.c >> @@ -126,9 +126,13 @@ fs_eth_dev_conf_apply(struct rte_eth_dev *dev, >> if (dev->data->promiscuous != edev->data->promiscuous) { >> DEBUG("Configuring promiscuous"); >> if (dev->data->promiscuous) >> - rte_eth_promiscuous_enable(PORT_ID(sdev)); >> + ret = rte_eth_promiscuous_enable(PORT_ID(sdev)); >> else >> - rte_eth_promiscuous_disable(PORT_ID(sdev)); >> + ret = rte_eth_promiscuous_disable(PORT_ID(sdev)); >> + if (ret != 0) { >> + ERROR("Failed to apply promiscuous mode"); >> + return ret; >> + } >> } else { >> DEBUG("promiscuous already set"); >> } >> diff --git a/drivers/net/failsafe/failsafe_ops.c b/drivers/net/failsafe/failsafe_ops.c >> index cc14bc2bcc..cbf143fb3c 100644 >> --- a/drivers/net/failsafe/failsafe_ops.c >> +++ b/drivers/net/failsafe/failsafe_ops.c >> @@ -659,11 +659,29 @@ fs_promiscuous_enable(struct rte_eth_dev *dev) >> { >> struct sub_device *sdev; >> uint8_t i; >> + int ret = 0; >> >> fs_lock(dev, 0); >> - FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) >> - rte_eth_promiscuous_enable(PORT_ID(sdev)); >> + FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) { >> + ret = rte_eth_promiscuous_enable(PORT_ID(sdev)); >> + if (ret != 0) { >> + ERROR("Promiscuous mode enable failed for subdevice %d", >> + PORT_ID(sdev)); >> + break; >> + } >> + } >> + if (ret != 0) { >> + /* Rollback in the case of failure */ >> + FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) { >> + ret = rte_eth_promiscuous_disable(PORT_ID(sdev)); >> + if (ret != 0) >> + ERROR("Promiscuous mode disable failed for subdevice %d", >> + PORT_ID(sdev)); >> + } >> + } >> fs_unlock(dev, 0); >> + >> + return; > This patch should be applied after the ethdev change to avoid breaking > the build, I think? As far as I tested it does not break the build. Sorry for confusing return at the end of function without return value. The function is still void and it is updated to forward ret when callback has return value. > You should be able to change the ethdev API, leaving the fail-safe > internals ignore the return value, then apply this patch and fix it. > This way the patchset should not break the build mid-series. > > Otherwise good implementation with the rollback. Thanks.