From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id 0E5B02C6B; Fri, 7 Apr 2017 03:38:44 +0200 (CEST) Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga105.fm.intel.com with ESMTP; 06 Apr 2017 18:38:40 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.37,162,1488873600"; d="scan'208";a="85684427" Received: from pgsmsx101.gar.corp.intel.com ([10.221.44.78]) by fmsmga005.fm.intel.com with ESMTP; 06 Apr 2017 18:38:36 -0700 Received: from pgsmsx106.gar.corp.intel.com ([169.254.9.5]) by PGSMSX101.gar.corp.intel.com ([169.254.1.174]) with mapi id 14.03.0319.002; Fri, 7 Apr 2017 09:38:34 +0800 From: "Dai, Wei" To: =?iso-8859-1?Q?N=E9lio_Laranjeiro?= CC: "dev@dpdk.org" , "thomas.monjalon@6wind.com" , "harish.patil@cavium.com" , "rasesh.mody@cavium.com" , "stephen.hurd@broadcom.com" , "ajit.khaparde@broadcom.com" , "Lu, Wenzhuo" , "Zhang, Helin" , "Ananyev, Konstantin" , "Wu, Jingjing" , "Chen, Jing D" , "adrien.mazarguil@6wind.com" , "Richardson, Bruce" , "yuanhan.liu@linux.intel.com" , "maxime.coquelin@redhat.com" , "stable@dpdk.org" Thread-Topic: [PATCH 1/2] ethdev: fix adding invalid MAC addr Thread-Index: AQHSqr95bo8FT2Q1LkKSA2/H93aTL6Gy2tgAgAZORQA= Date: Fri, 7 Apr 2017 01:38:34 +0000 Message-ID: <49759EB36A64CF4892C1AFEC9231E8D650A52D88@PGSMSX106.gar.corp.intel.com> References: <1491033797-14186-1-git-send-email-wei.dai@intel.com> <1491033797-14186-2-git-send-email-wei.dai@intel.com> <20170403091816.GC16796@autoinstall.dev.6wind.com> In-Reply-To: <20170403091816.GC16796@autoinstall.dev.6wind.com> Accept-Language: zh-CN, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMDNiMzYzZmMtM2ZhMC00ZWFlLWJjNDgtOTMxZTNjNmRkZTMzIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE1LjkuNi42IiwiVHJ1c3RlZExhYmVsSGFzaCI6Im5Bc3ZmSU43SWtSNW1uZUtWQ0dudFFqeFVyVk1rMXFzNXd4RjBpVEVcL1VRPSJ9 x-ctpclassification: CTP_IC x-originating-ip: [172.30.20.206] Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH 1/2] 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: Fri, 07 Apr 2017 01:38:45 -0000 Hi, N=E9lio Laranjeiro=20 Thanks for your feedback. I will change them in my next version of patch set. -Wei > -----Original Message----- > From: N=E9lio Laranjeiro [mailto:nelio.laranjeiro@6wind.com] > Sent: Monday, April 3, 2017 5:18 PM > To: Dai, Wei > Cc: dev@dpdk.org; thomas.monjalon@6wind.com; harish.patil@cavium.com; > rasesh.mody@cavium.com; stephen.hurd@broadcom.com; > ajit.khaparde@broadcom.com; Lu, Wenzhuo ; Zhang, > Helin ; Ananyev, Konstantin > ; Wu, Jingjing ; Che= n, > Jing D ; adrien.mazarguil@6wind.com; Richardson, > Bruce ; yuanhan.liu@linux.intel.com; > maxime.coquelin@redhat.com; stable@dpdk.org > Subject: Re: [PATCH 1/2] ethdev: fix adding invalid MAC addr >=20 > Hi, >=20 > Please see below some comments, >=20 > 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 >=20 > Please keep function documentation up to date. >=20 > > mlx4_mac_addr_add(struct rte_eth_dev *dev, struct ether_addr > *mac_addr, > > uint32_t index, uint32_t vmdq) > > { > > struct priv *priv =3D dev->data->dev_private; > > + int re; > > > > if (mlx4_is_secondary()) > > - return; > > + return -1; >=20 > Should not it return ENOSUP instead? >=20 > > (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 >=3D (elemof(priv->mac) - 1)) > > - goto end; > > - priv_mac_addr_add(priv, index, > > + if (index >=3D (elemof(priv->mac) - 1)) { > > + priv_unlock(priv); > > + return -2; > > + } > > + re =3D priv_mac_addr_add(priv, index, > > (const uint8_t (*)[ETHER_ADDR_LEN]) > > mac_addr->addr_bytes); >=20 > 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. >=20 > > 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 =3D 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 >=3D RTE_DIM(priv->mac)) > > + if (index >=3D RTE_DIM(priv->mac)) { > > + re =3D -2; > > goto end; > > - priv_mac_addr_add(priv, index, > > + } > > + re =3D priv_mac_addr_add(priv, index, > > (const uint8_t (*)[ETHER_ADDR_LEN]) > > mac_addr->addr_bytes); > > end: > > priv_unlock(priv); > > + return re; > > } > >[...] >=20 > Same remarks here, >=20 > Thanks, >=20 > -- > N=E9lio Laranjeiro > 6WIND