From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dispatch1-us1.ppe-hosted.com (dispatch1-us1.ppe-hosted.com [148.163.129.52]) by dpdk.org (Postfix) with ESMTP id AA9A7201; Mon, 24 Sep 2018 16:10:50 +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 (Proofpoint Essentials ESMTP Server) with ESMTPS id 1906A4C0063; Mon, 24 Sep 2018 14:10:49 +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; Mon, 24 Sep 2018 07:10:45 -0700 To: Alejandro Lucero , CC: References: <1537796604-29378-1-git-send-email-alejandro.lucero@netronome.com> From: Andrew Rybchenko Message-ID: <7a20a3f2-bf64-31d8-1ced-b92fcd204f30@solarflare.com> Date: Mon, 24 Sep 2018 17:10:02 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.0 MIME-Version: 1.0 In-Reply-To: <1537796604-29378-1-git-send-email-alejandro.lucero@netronome.com> Content-Language: ru 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-24114.005 X-TM-AS-Result: No-17.083000-4.000000-10 X-TMASE-MatchedRID: X4bcv0S75KkOwH4pD14DsPHkpkyUphL9gRykyfrH1xmCsBeCv8CM/d/Z UmGP9YkbgEx/MZlaMxVtONcpcaUbf2JZXQNDzktSnu1HSadECDWiNCtus+nPOt9MUoGRBUKy18S 5+8epYHi37cssi5Wn7cmzISteeaIdjE7kWGeWkSxPtFVFuufzC34rryovYbmmGNAPebYwJ/soOR HYwPKf3AuEJPY37ab0QYEHGIEz/kkmek8QIa7UrvSG/+sPtZVkURyIYNXh3ysHiwiF9OOogUqyU ztVRoTBdvgGbsWpCcvPX7sUkKqkssgNYoWONkdvL4+sB3yBsckIUQiRW/XLrKRGpeOWX2wIsOo6 xOOnkSvOH38lNbOoTFxI2ZZX9m4X319rGX8jiaGeAiCmPx4NwLTrdaH1ZWqC1B0Hk1Q1KyLUZxE AlFPo8wi890LaFSrRZZgT9jfCzdTo2c0ap4jKxAM+MNakNYAM4ewAy0pzEJLLOPnslC3v8fEU1n SxmNpj/t0OxUmZaMplZNQZivdvv7IRTNSK/fw1 X-TM-AS-User-Approved-Sender: No X-TM-AS-User-Blocked-Sender: No X-TMASE-Result: 10--17.083000-4.000000 X-TMASE-Version: SMEX-12.5.0.1300-8.5.1010-24114.005 X-MDID: 1537798249-Qr5vo8in-CH0 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] ethdev: fix error handling logic 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: , X-List-Received-Date: Mon, 24 Sep 2018 14:10:51 -0000 On 9/24/18 4:43 PM, Alejandro Lucero wrote: > This patch fixes how function exit is handled when errors inside > rte_eth_dev_create. > > Fixes: e489007a411c ("ethdev: add generic create/destroy ethdev APIs") > Cc: stable@dpdk.org > > Signed-off-by: Alejandro Lucero Minor nit/observation below, but anyway Reviewed-by: Andrew Rybchenko > --- > lib/librte_ethdev/rte_ethdev.c | 12 +++++------- > 1 file changed, 5 insertions(+), 7 deletions(-) > > diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c > index aa7730c..ef99f70 100644 > --- a/lib/librte_ethdev/rte_ethdev.c > +++ b/lib/librte_ethdev/rte_ethdev.c > @@ -3467,10 +3467,8 @@ int rte_eth_set_queue_rate_limit(uint16_t port_id, uint16_t queue_idx, > > if (rte_eal_process_type() == RTE_PROC_PRIMARY) { > ethdev = rte_eth_dev_allocate(name); > - if (!ethdev) { > - retval = -ENODEV; > - goto probe_failed; > - } > + if (!ethdev) > + return -ENODEV; As far as I can see rte_eth_dev_allocate() returns NULL if a device with such name already exists or no free ports left. I'd say that EEXIST and ENOSPC better describe what went wrong, but the patch simply does not change it and there is no easy way to fix it (except may be rte_errno usage). > > if (priv_data_size) { > ethdev->data->dev_private = rte_zmalloc_socket( > @@ -3480,7 +3478,7 @@ int rte_eth_set_queue_rate_limit(uint16_t port_id, uint16_t queue_idx, > if (!ethdev->data->dev_private) { > RTE_LOG(ERR, EAL, "failed to allocate private data"); > retval = -ENOMEM; > - goto probe_failed; > + goto data_alloc_failed; > } > } > } else { > @@ -3488,8 +3486,7 @@ int rte_eth_set_queue_rate_limit(uint16_t port_id, uint16_t queue_idx, > if (!ethdev) { > RTE_LOG(ERR, EAL, "secondary process attach failed, " > "ethdev doesn't exist"); > - retval = -ENODEV; > - goto probe_failed; > + return -ENODEV; Here ENODEV is 100% correct since secondary simply failed to find ethdev with matching name. > } > } > > @@ -3518,6 +3515,7 @@ int rte_eth_set_queue_rate_limit(uint16_t port_id, uint16_t queue_idx, > if (rte_eal_process_type() == RTE_PROC_PRIMARY) > rte_free(ethdev->data->dev_private); > > +data_alloc_failed: > rte_eth_dev_release_port(ethdev); > > return retval;