From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124])
	by inbox.dpdk.org (Postfix) with ESMTP id 0DED3A034F;
	Mon, 22 Mar 2021 03:59:08 +0100 (CET)
Received: from [217.70.189.124] (localhost [127.0.0.1])
	by mails.dpdk.org (Postfix) with ESMTP id EE54C140E8F;
	Mon, 22 Mar 2021 03:59:07 +0100 (CET)
Received: from smtpbgbr2.qq.com (smtpbgbr2.qq.com [54.207.22.56])
 by mails.dpdk.org (Postfix) with ESMTP id 8A8ED4069C
 for <dev@dpdk.org>; Mon, 22 Mar 2021 03:59:05 +0100 (CET)
X-QQ-mid: bizesmtp29t1616381938t0m72xi7
Received: from jiawenwu (unknown [183.129.236.74])
 by esmtp6.qq.com (ESMTP) with 
 id ; Mon, 22 Mar 2021 10:58:57 +0800 (CST)
X-QQ-SSF: 01400000002000C0E000B00A0000000
X-QQ-FEAT: tSE3IANCkAuPWeAYKbIEPA4ScVS9KfgUaRkFLBtCqRpuyTbV1bO63tfO9poeU
 4jqRlIAHlcVv6kWOskTfh5Y6mJRtpq/X40PXTyRHs3ovIuwUc9SI6fld0rO7heCcvPN2RCk
 RgiEcBxPnZSm66BgOIPOlM+sgy33APY6/w+Mkuxue3oS2eFpZNDGyas3fYcJk7q+2GmUK5x
 +3Udntjm+VP/d+FtDVfnq1Whhi7ieVhvzy+G8dFU7g8JwIRy4U4YmaGgZfmAL54O/cqKCoX
 UwkfQLuxEA6U5mVqJ4U+8xKmfORl+OSH2jjSJigii/dVBwFZOgkmnrhYTauJu1KilFv9f2x
 cpalrxd
X-QQ-GoodBg: 2
From: "Jiawen Wu" <jiawenwu@trustnetic.com>
To: "'Stephen Hemminger'" <stephen@networkplumber.org>
Cc: <dev@dpdk.org>
References: <20210319072628.10000-1-jiawenwu@trustnetic.com>	<20210319072628.10000-5-jiawenwu@trustnetic.com>
 <20210319104729.0390e3dc@hermes.local>
In-Reply-To: <20210319104729.0390e3dc@hermes.local>
Date: Mon, 22 Mar 2021 10:58:57 +0800
Message-ID: <00a601d71ec7$4dbd0f40$e9372dc0$@trustnetic.com>+B68BDF9E586C6D96
MIME-Version: 1.0
Content-Type: text/plain;
	charset="us-ascii"
Content-Transfer-Encoding: 7bit
X-Mailer: Microsoft Outlook 16.0
Thread-Index: AQGc4/gqYrQKIQmR30PC1cZPtg4mCwJP3I8YAkLqFmSq3yoXMA==
Content-Language: zh-cn
X-QQ-SENDSIZE: 520
Feedback-ID: bizesmtp:trustnetic.com:qybgforeign:qybgforeign5
X-QQ-Bgrelay: 1
Subject: Re: [dpdk-dev] [PATCH v2 4/6] net/ngbe: add device init and uninit
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

On Saturday, March 20, 2021 1:47 AM, Stephen Hemminger wrote:
> On Fri, 19 Mar 2021 15:26:26 +0800
> Jiawen Wu <jiawenwu@trustnetic.com> wrote:
> 
> > +++ b/drivers/net/ngbe/base/ngbe_osdep.h
> > @@ -0,0 +1,172 @@
> 
> > +#define false               0
> > +#define true                1
> 
> These will conflict with normal definitions in <stdbool.h> Why are you not
using
> that?

I think, "#ifndef false" and "#ifndef true" can be used to avoid adding
<stdbool.h> repeatedly.

> 
> > +#define min(a, b)	RTE_MIN(a, b)
> > +#define max(a, b)	RTE_MAX(a, b)
> > +
> > +/* Bunch of defines for shared code bogosity */
> > +
> > +static inline void UNREFERENCED(const char *a __rte_unused, ...) {}
> > +#define UNREFERENCED_PARAMETER(args...) UNREFERENCED("", ##args)
> > +
> 
> I have seen that before... porting code from Windows?

I am not sure if it was ported from Windows.
It is ported from TXGBE, and TXGBE was coding by another workmate several
years before.

> 
> > +/* Check whether address is multicast. This is little-endian specific
> > +check.*/ #define NGBE_IS_MULTICAST(address) \
> > +		(bool)(((u8 *)(address))[0] & ((u8)0x01))
> > +
> 
> Why not rte_ether_is_multicast_ether_addr?
> 
> 
> > +/* Check whether an address is broadcast. */ #define
> > +NGBE_IS_BROADCAST(address) \
> > +		({typeof(address)addr = (address); \
> > +		(((u8 *)(addr))[0] == ((u8)0xff)) && \
> > +		(((u8 *)(addr))[1] == ((u8)0xff)); })
> > +
> 
> Your code is not correct since it would match ff:ff:01:02:03:04
> 
> Why not rte_ether_is_broadcast_ether_addr?

Thanks for your review.
Such in the case, rte_is_multicast_ether_addr and
rte_is_broadcast_ether_addr should be used.