From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr0-f171.google.com (mail-wr0-f171.google.com [209.85.128.171]) by dpdk.org (Postfix) with ESMTP id C5957316B for ; Wed, 12 Apr 2017 11:30:28 +0200 (CEST) Received: by mail-wr0-f171.google.com with SMTP id c55so13472909wrc.3 for ; Wed, 12 Apr 2017 02:30:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to :user-agent; bh=JjNsHWC/wjBr6C3QfDAlHtCMTcvmES/AA0gfr4CEaHw=; b=OkLhh1HoRX0YSPOeKHt+3rUibLZ59rYp+xTott+mL9Drb1oCcCbJj4WHhHhO4rfCVD PQI54/yse5VrKEpHnxpPkt0J4g2qWlAM8WeJgYF3/i/Ln2begQNQhdQjzsi3JL1xqQ61 EXqoIPruT/GfvNj0UgMQoiFdC5SXC8YKWUOHn1j5N68l1524rG+En7WB+CkUi2dRd88+ 4q9589AXagWtk0n9Dn1yFflOC0tmOAe6yPNs46ZouvFMt2kHPnpHpVG+ujlaLwzsOGeI 1EmAQvPwqeDSW5jNlBRi5ceQxTjpPZy2nauUQdZf0r+t4A0A+Vv4Cwlm8KK5JkjS5UIt yxpA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=JjNsHWC/wjBr6C3QfDAlHtCMTcvmES/AA0gfr4CEaHw=; b=VuvnxCk1jiX8PKgFLJSi9wH0cYvNNU2Tf9nVtg5/FoVbwC9Hqats/+ShrsmMqASBnh 6EWXbX7bTE1P0IxSKKYGlFLt7rQ/ZVAbgKpLysvI3meZT9QR9ixxXAesNwaRw0WW+jDL DNcwWPruUzx7dZN2EFrA3373lChh2tFXuTYclNvsg0rtKvUTu4+LMznE4y1J8rtnLSFr nAaxmvY+vt2U69TFTsE2HzUSv6RXuyuS9xpNAEK5GnBe8YnXmooMYrd0wGAWbiQylrmx uf2sti7Nl7l4oMjYjIO95O3mu/qj1BYnhJKHHKQ0q//sAS1KmVDRxaukOPANQl05PBK2 DlTA== X-Gm-Message-State: AN3rC/4bOpFAHi4ytODsk8jyjO0R00khBosoWjLYQH3xnM8X5KD9dHJjdHMYHMxiqzDKfr5I X-Received: by 10.223.169.34 with SMTP id u31mr2102345wrc.54.1491989428474; Wed, 12 Apr 2017 02:30:28 -0700 (PDT) Received: from autoinstall.dev.6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id h68sm5865748wmd.19.2017.04.12.02.30.27 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 12 Apr 2017 02:30:28 -0700 (PDT) Date: Wed, 12 Apr 2017 11:30:20 +0200 From: =?iso-8859-1?Q?N=E9lio?= Laranjeiro To: Wei Dai Cc: dev@dpdk.org, thomas.monjalon@6wind.com, harish.patil@cavium.com, rasesh.mody@cavium.com, stephen.hurd@broadcom.com, ajit.khaparde@broadcom.com, wenzhuo.lu@intel.com, helin.zhang@intel.com, konstantin.ananyev@intel.com, jingjing.wu@intel.com, jing.d.chen@intel.com, adrien.mazarguil@6wind.com, bruce.richardson@intel.com, yuanhan.liu@linux.intel.com, maxime.coquelin@redhat.com, stable@dpdk.org Message-ID: <20170412093020.GC16796@autoinstall.dev.6wind.com> References: <1491987746-10155-1-git-send-email-wei.dai@intel.com> <1491987746-10155-2-git-send-email-wei.dai@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1491987746-10155-2-git-send-email-wei.dai@intel.com> User-Agent: Mutt/1.5.23 (2014-03-12) Subject: Re: [dpdk-dev] [PATCH v3 1/3] ethdev: fix adding invalid MAC addr 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: Wed, 12 Apr 2017 09:30:29 -0000 Hi, note, please use the --thread option in addition of the in-reply-to to new series. Small comments inline, On Wed, Apr 12, 2017 at 05:02:24PM +0800, Wei Dai wrote: >[...] > diff --git a/drivers/net/mlx4/mlx4.c b/drivers/net/mlx4/mlx4.c > index ff903e6..91fc4c4 100644 > --- a/drivers/net/mlx4/mlx4.c > +++ b/drivers/net/mlx4/mlx4.c > @@ -4475,26 +4475,30 @@ mlx4_mac_addr_remove(struct rte_eth_dev *dev, uint32_t index) > * @param vmdq > * VMDq pool index to associate address with (ignored). > */ > -static void > +static int > mlx4_mac_addr_add(struct rte_eth_dev *dev, struct ether_addr *mac_addr, > uint32_t index, uint32_t vmdq) > { > struct priv *priv = dev->data->dev_private; > + int re; > > if (mlx4_is_secondary()) > - return; > + return -ENOTSUP; > (void)vmdq; > priv_lock(priv); > DEBUG("%p: adding MAC address at index %" PRIu32, > (void *)dev, index); > /* Last array entry is reserved for broadcast. */ > - if (index >= (elemof(priv->mac) - 1)) > - goto end; > - priv_mac_addr_add(priv, index, > + if (index >= (elemof(priv->mac) - 1)) { > + priv_unlock(priv); > + return -EINVAL; > + } > + re = priv_mac_addr_add(priv, index, > (const uint8_t (*)[ETHER_ADDR_LEN]) > mac_addr->addr_bytes); Please keep the coding style consistency among the file, this last snippet should be: re = priv_mac_addr_add(priv, index, (const uint8_t (*)[ETHER_ADDR_LEN]) mac_addr->addr_bytes); > end: > priv_unlock(priv); > + return -re; > } > > /** > diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h > index d26d465..3c5de07 100644 > --- a/drivers/net/mlx5/mlx5.h > +++ b/drivers/net/mlx5/mlx5.h > @@ -238,8 +238,8 @@ int hash_rxq_mac_addrs_add(struct hash_rxq *); > int priv_mac_addr_add(struct priv *, unsigned int, > const uint8_t (*)[ETHER_ADDR_LEN]); > int priv_mac_addrs_enable(struct priv *); > -void mlx5_mac_addr_add(struct rte_eth_dev *, struct ether_addr *, uint32_t, > - uint32_t); > +int mlx5_mac_addr_add(struct rte_eth_dev *dev, struct ether_addr *mac_addr, > + uint32_t index, uint32_t vmdq); In the whole file, function prototypes does not provide variable names, please keep it as is. Same point about the indentation, the second line should be aligned with the '('. > void mlx5_mac_addr_set(struct rte_eth_dev *, struct ether_addr *); > > /* mlx5_rss.c */ > diff --git a/drivers/net/mlx5/mlx5_mac.c b/drivers/net/mlx5/mlx5_mac.c > index 4fcfd3b..3cc6f8b 100644 > --- a/drivers/net/mlx5/mlx5_mac.c > +++ b/drivers/net/mlx5/mlx5_mac.c > @@ -470,26 +470,30 @@ priv_mac_addrs_enable(struct priv *priv) > * @param vmdq > * VMDq pool index to associate address with (ignored). > */ > -void > +int > mlx5_mac_addr_add(struct rte_eth_dev *dev, struct ether_addr *mac_addr, > uint32_t index, uint32_t vmdq) > { > struct priv *priv = dev->data->dev_private; > + int re; > > if (mlx5_is_secondary()) > - return; > + return -ENOTSUP; > > (void)vmdq; > priv_lock(priv); > DEBUG("%p: adding MAC address at index %" PRIu32, > (void *)dev, index); > - if (index >= RTE_DIM(priv->mac)) > + if (index >= RTE_DIM(priv->mac)) { > + re = -EINVAL; > goto end; > - priv_mac_addr_add(priv, index, > + } > + re = priv_mac_addr_add(priv, index, > (const uint8_t (*)[ETHER_ADDR_LEN]) > mac_addr->addr_bytes); Same remark here about the indentation. > end: > priv_unlock(priv); > + return -re; > } Thanks, -- Nélio Laranjeiro 6WIND