On Saturday, March 20, 2021 1:47 AM, Stephen Hemminger wrote:
> On Fri, 19 Mar 2021 15:26:26 +0800
> Jiawen Wu <jiawe...@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.
