From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr0-f182.google.com (mail-wr0-f182.google.com [209.85.128.182]) by dpdk.org (Postfix) with ESMTP id 40C822B9B for ; Mon, 3 Apr 2017 11:18:25 +0200 (CEST) Received: by mail-wr0-f182.google.com with SMTP id w11so159036974wrc.3 for ; Mon, 03 Apr 2017 02:18:25 -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=ZBqeSZAW6fRMhWfeNyo2ZYxSNcetw1yblRbPHZVGTJE=; b=uXAoTMRji6O9mPAdp8KSTu54RK6EAW7pUllDOgYmAzlXZzrAObU4LqBp9QNtnkn7Vt Mt+CUlXHP8C/5hYjxuiMSKoO7JZ7WMgg28CdhzKr7WbuXrXMWJdOYuwogla6SzAxp6DC 0qc8NLooJ0ddxk8+LA9UCh+IRcTeLYOhvBM3OpkfmaNOJ/G8gN59M9HvO+Hh+Nf9+8f4 giowZpo2G429g/Cjj8BpKPPlbJ0Rauk4JC+t1MIK1A+QhsBd4DqJdXn9B4v+8ZuWjj2d 4CxY/lz1y3jyNuQF+uIx/V6+bo0aa6WIuB4WES8G2eYgyH3fFnhTDImJGPtjbQDSM1y+ uexw== 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=ZBqeSZAW6fRMhWfeNyo2ZYxSNcetw1yblRbPHZVGTJE=; b=XEBUM8cNX2BZ/EOJo2bloGNAMUXip1pB7XxXvSnN9e9zDa7HGyq0CgrrXs5GzGJfeB Bavy2NEzAPJwt7aG1z1ETO2oo2YwrOIp9IuNgV/ETF2rVXx6cMNGRXDc18nxQhLIYpFs zGsws20W7XWCvuwHAZZA694n2oQe16d7UVLJTyz32E//vYiWAugpk7DfwUdL1f5x1e/u ByeJQyNc0G26FXlea6ah/1VF3flzKttN3QcIJoKGud9zWiKqVQ5T08uf48pyCuxn/UR3 APuFv5m8VOb3DjwmC+gY92StSxc+AZFFaiA943tVsa0hkUjt5nO+UThueiVJwX5UgnJS gJhA== X-Gm-Message-State: AFeK/H3yO2FRY/QEL31qbQbcPkyElg0KRFefPZdVgO0/HBrDvaSDT/x5vW7UjCLXiIqy53MS X-Received: by 10.223.156.18 with SMTP id f18mr9087764wrc.113.1491211105087; Mon, 03 Apr 2017 02:18:25 -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 l141sm13511557wma.32.2017.04.03.02.18.24 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 03 Apr 2017 02:18:24 -0700 (PDT) Date: Mon, 3 Apr 2017 11:18:16 +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: <20170403091816.GC16796@autoinstall.dev.6wind.com> References: <1491033797-14186-1-git-send-email-wei.dai@intel.com> <1491033797-14186-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: <1491033797-14186-2-git-send-email-wei.dai@intel.com> User-Agent: Mutt/1.5.23 (2014-03-12) Subject: Re: [dpdk-stable] [PATCH 1/2] ethdev: fix adding invalid MAC addr X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 03 Apr 2017 09:18:26 -0000 Hi, Please see below some comments, On Sat, Apr 01, 2017 at 04:03:16PM +0800, Wei Dai wrote: >[...] > diff --git a/drivers/net/mlx4/mlx4.c b/drivers/net/mlx4/mlx4.c > index 79efaaa..d61bb27 100644 > --- a/drivers/net/mlx4/mlx4.c > +++ b/drivers/net/mlx4/mlx4.c > @@ -4630,26 +4630,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 Please keep function documentation up to date. > 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 -1; Should not it return ENOSUP instead? > (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 -2; > + } > + re = priv_mac_addr_add(priv, index, > (const uint8_t (*)[ETHER_ADDR_LEN]) > mac_addr->addr_bytes); You have an issue here, internal function of mlx drivers return positives errno (as described in their functions documentation). It should be negated before returning. > end: > priv_unlock(priv); > + return re; > } > > /** > diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h > index 879da5e..cca7a36 100644 > --- a/drivers/net/mlx5/mlx5.h > +++ b/drivers/net/mlx5/mlx5.h > @@ -229,8 +229,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); > 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..db93f4e 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 -1; > > (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 = -2; > 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); > end: > priv_unlock(priv); > + return re; > } >[...] Same remarks here, Thanks, -- Nélio Laranjeiro 6WIND