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 AA450A2E1B for ; Wed, 4 Sep 2019 18:41:03 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 479621E883; Wed, 4 Sep 2019 18:41:02 +0200 (CEST) Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id 06B601C19A for ; Wed, 4 Sep 2019 18:41:00 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 04 Sep 2019 09:40:59 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,467,1559545200"; d="scan'208";a="383532987" Received: from fyigit-mobl.ger.corp.intel.com (HELO [10.237.221.4]) ([10.237.221.4]) by fmsmga006.fm.intel.com with ESMTP; 04 Sep 2019 09:40:58 -0700 To: Andrew Rybchenko , Neil Horman , John McNamara , Marko Kovacevic , Thomas Monjalon Cc: dev@dpdk.org, Ivan Ilchenko References: <1566915962-5472-1-git-send-email-arybchenko@solarflare.com> <1567519051-28189-1-git-send-email-arybchenko@solarflare.com> <1567519051-28189-3-git-send-email-arybchenko@solarflare.com> From: Ferruh Yigit Openpgp: preference=signencrypt Autocrypt: addr=ferruh.yigit@intel.com; prefer-encrypt=mutual; keydata= mQINBFXZCFABEADCujshBOAaqPZpwShdkzkyGpJ15lmxiSr3jVMqOtQS/sB3FYLT0/d3+bvy qbL9YnlbPyRvZfnP3pXiKwkRoR1RJwEo2BOf6hxdzTmLRtGtwWzI9MwrUPj6n/ldiD58VAGQ +iR1I/z9UBUN/ZMksElA2D7Jgg7vZ78iKwNnd+vLBD6I61kVrZ45Vjo3r+pPOByUBXOUlxp9 GWEKKIrJ4eogqkVNSixN16VYK7xR+5OUkBYUO+sE6etSxCr7BahMPKxH+XPlZZjKrxciaWQb +dElz3Ab4Opl+ZT/bK2huX+W+NJBEBVzjTkhjSTjcyRdxvS1gwWRuXqAml/sh+KQjPV1PPHF YK5LcqLkle+OKTCa82OvUb7cr+ALxATIZXQkgmn+zFT8UzSS3aiBBohg3BtbTIWy51jNlYdy ezUZ4UxKSsFuUTPt+JjHQBvF7WKbmNGS3fCid5Iag4tWOfZoqiCNzxApkVugltxoc6rG2TyX CmI2rP0mQ0GOsGXA3+3c1MCdQFzdIn/5tLBZyKy4F54UFo35eOX8/g7OaE+xrgY/4bZjpxC1 1pd66AAtKb3aNXpHvIfkVV6NYloo52H+FUE5ZDPNCGD0/btFGPWmWRmkPybzColTy7fmPaGz cBcEEqHK4T0aY4UJmE7Ylvg255Kz7s6wGZe6IR3N0cKNv++O7QARAQABtCVGZXJydWggWWln aXQgPGZlcnJ1aC55aWdpdEBpbnRlbC5jb20+iQJUBBMBCgA+AhsDAh4BAheABQsJCAcDBRUK CQgLBRYCAwEAFiEE0jZTh0IuwoTjmYHH+TPrQ98TYR8FAl1meboFCQlupOoACgkQ+TPrQ98T YR9ACBAAv2tomhyxY0Tp9Up7mNGLfEdBu/7joB/vIdqMRv63ojkwr9orQq5V16V/25+JEAD0 60cKodBDM6HdUvqLHatS8fooWRueSXHKYwJ3vxyB2tWDyZrLzLI1jxEvunGodoIzUOtum0Ce gPynnfQCelXBja0BwLXJMplM6TY1wXX22ap0ZViC0m714U5U4LQpzjabtFtjT8qOUR6L7hfy YQ72PBuktGb00UR/N5UrR6GqB0x4W41aZBHXfUQnvWIMmmCrRUJX36hOTYBzh+x86ULgg7H2 1499tA4o6rvE13FiGccplBNWCAIroAe/G11rdoN5NBgYVXu++38gTa/MBmIt6zRi6ch15oLA Ln2vHOdqhrgDuxjhMpG2bpNE36DG/V9WWyWdIRlz3NYPCDM/S3anbHlhjStXHOz1uHOnerXM 1jEjcsvmj1vSyYoQMyRcRJmBZLrekvgZeh7nJzbPHxtth8M7AoqiZ/o/BpYU+0xZ+J5/szWZ aYxxmIRu5ejFf+Wn9s5eXNHmyqxBidpCWvcbKYDBnkw2+Y9E5YTpL0mS0dCCOlrO7gca27ux ybtbj84aaW1g0CfIlUnOtHgMCmz6zPXThb+A8H8j3O6qmPoVqT3qnq3Uhy6GOoH8Fdu2Vchh TWiF5yo+pvUagQP6LpslffufSnu+RKAagkj7/RSuZV25Ag0EV9ZMvgEQAKc0Db17xNqtSwEv mfp4tkddwW9XA0tWWKtY4KUdd/jijYqc3fDD54ESYpV8QWj0xK4YM0dLxnDU2IYxjEshSB1T qAatVWz9WtBYvzalsyTqMKP3w34FciuL7orXP4AibPtrHuIXWQOBECcVZTTOdZYGAzaYzxiA ONzF9eTiwIqe9/oaOjTwTLnOarHt16QApTYQSnxDUQljeNvKYt1lZE/gAUUxNLWsYyTT+22/ vU0GDUahsJxs1+f1yEr+OGrFiEAmqrzpF0lCS3f/3HVTU6rS9cK3glVUeaTF4+1SK5ZNO35p iVQCwphmxa+dwTG/DvvHYCtgOZorTJ+OHfvCnSVjsM4kcXGjJPy3JZmUtyL9UxEbYlrffGPQ I3gLXIGD5AN5XdAXFCjjaID/KR1c9RHd7Oaw0Pdcq9UtMLgM1vdX8RlDuMGPrj5sQrRVbgYH fVU/TQCk1C9KhzOwg4Ap2T3tE1umY/DqrXQgsgH71PXFucVjOyHMYXXugLT8YQ0gcBPHy9mZ qw5mgOI5lCl6d4uCcUT0l/OEtPG/rA1lxz8ctdFBVOQOxCvwRG2QCgcJ/UTn5vlivul+cThi 6ERPvjqjblLncQtRg8izj2qgmwQkvfj+h7Ex88bI8iWtu5+I3K3LmNz/UxHBSWEmUnkg4fJl Rr7oItHsZ0ia6wWQ8lQnABEBAAGJAjwEGAEKACYCGwwWIQTSNlOHQi7ChOOZgcf5M+tD3xNh HwUCXWZ5wAUJB3FgggAKCRD5M+tD3xNhH2O+D/9OEz62YuJQLuIuOfL67eFTIB5/1+0j8Tsu o2psca1PUQ61SZJZOMl6VwNxpdvEaolVdrpnSxUF31kPEvR0Igy8HysQ11pj8AcgH0a9FrvU /8k2Roccd2ZIdpNLkirGFZR7LtRw41Kt1Jg+lafI0efkiHKMT/6D/P1EUp1RxOBNtWGV2hrd 0Yg9ds+VMphHHU69fDH02SwgpvXwG8Qm14Zi5WQ66R4CtTkHuYtA63sS17vMl8fDuTCtvfPF HzvdJLIhDYN3Mm1oMjKLlq4PUdYh68Fiwm+boJoBUFGuregJFlO3hM7uHBDhSEnXQr5mqpPM 6R/7Q5BjAxrwVBisH0yQGjsWlnysRWNfExAE2sRePSl0or9q19ddkRYltl6X4FDUXy2DTXa9 a+Fw4e1EvmcF3PjmTYs9IE3Vc64CRQXkhujcN4ZZh5lvOpU8WgyDxFq7bavFnSS6kx7Tk29/ wNJBp+cf9qsQxLbqhW5kfORuZGecus0TLcmpZEFKKjTJBK9gELRBB/zoN3j41hlEl7uTUXTI JQFLhpsFlEdKLujyvT/aCwP3XWT+B2uZDKrMAElF6ltpTxI53JYi22WO7NH7MR16Fhi4R6vh FHNBOkiAhUpoXRZXaCR6+X4qwA8CwHGqHRBfYFSU/Ulq1ZLR+S3hNj2mbnSx0lBs1eEqe2vh cA== Message-ID: <928f04e3-4c03-bdeb-fcd9-b0743ee12c0a@intel.com> Date: Wed, 4 Sep 2019 17:40:57 +0100 MIME-Version: 1.0 In-Reply-To: <1567519051-28189-3-git-send-email-arybchenko@solarflare.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH v2 02/54] ethdev: change rte_eth_dev_info_get() return value to int 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/3/2019 2:56 PM, Andrew Rybchenko wrote: > From: Ivan Ilchenko > > Change rte_eth_dev_info_get() return value from void to int and return > negative errno values in case of error conditions. > Modify rte_eth_dev_info_get() usage across the ethdev according > to new return type. > > Signed-off-by: Ivan Ilchenko > Signed-off-by: Andrew Rybchenko <...> > @@ -1144,7 +1143,9 @@ struct rte_eth_dev * The diff output seems not able to detect the function/block which makes it harder to review, same patch I am getting a diff output like below: @@ -1144,7 +1143,9 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q, Can you please either check a newer version of git, as far as I can see you are using '1.8.3.1', or can you please try by removing/updating '.gitattributes' file? > */ > memcpy(&dev->data->dev_conf, dev_conf, sizeof(dev->data->dev_conf)); > > - rte_eth_dev_info_get(port_id, &dev_info); > + ret = rte_eth_dev_info_get(port_id, &dev_info); > + if (ret != 0) > + return ret; 'dev->data->dev_conf' should be restored from 'orig_conf' on failure, before return. <...> > -void > +int > rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info) > { > struct rte_eth_dev *dev; > @@ -2558,7 +2566,7 @@ struct rte_eth_dev * > */ > memset(dev_info, 0, sizeof(struct rte_eth_dev_info)); > > - RTE_ETH_VALID_PORTID_OR_RET(port_id); > + RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); > dev = &rte_eth_devices[port_id]; > > dev_info->rx_desc_lim = lim; > @@ -2567,13 +2575,15 @@ struct rte_eth_dev * > dev_info->min_mtu = RTE_ETHER_MIN_MTU; > dev_info->max_mtu = UINT16_MAX; > > - RTE_FUNC_PTR_OR_RET(*dev->dev_ops->dev_infos_get); > + RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_infos_get, -ENOTSUP); > (*dev->dev_ops->dev_infos_get)(dev, dev_info); > dev_info->driver_name = dev->device->driver->name; > dev_info->nb_rx_queues = dev->data->nb_rx_queues; > dev_info->nb_tx_queues = dev->data->nb_tx_queues; > > dev_info->dev_flags = &dev->data->dev_flags; > + > + return 0; Should API use "return eth_err(port_id, ret);" syntax, as other APIs do, I am not very clear about the 'eth_err()', Thomas can provide better description/suggestion on this. <...> > @@ -3100,9 +3118,13 @@ struct rte_eth_dev * > struct rte_eth_dev_info dev_info; > struct rte_eth_dev *dev = &rte_eth_devices[port_id]; > unsigned i; > + int ret; > > RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); > - rte_eth_dev_info_get(port_id, &dev_info); > + > + ret = rte_eth_dev_info_get(port_id, &dev_info); > + if (ret != 0) > + return ret; This is 'get_mac_addr_index()' static function, and it should either send negative value for error, or 'index' value in case of success. So should we be sure here only negative value sent? Incase 'rte_eth_dev_info_get()' returns positive value in the future. > > for (i = 0; i < dev_info.max_mac_addrs; i++) > if (memcmp(addr, &dev->data->mac_addrs[i], > @@ -3233,8 +3255,14 @@ struct rte_eth_dev * > struct rte_eth_dev_info dev_info; > struct rte_eth_dev *dev = &rte_eth_devices[port_id]; > unsigned i; > + int ret; > + > + RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); > + > + ret = rte_eth_dev_info_get(port_id, &dev_info); > + if (ret != 0) > + return ret; Same comment as above, for this 'get_hash_mac_addr_index()' static function.