netdev - RE: [RFC v2 net-next 05/10] qede: Add basic network device support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 25 Sep 2015 05:43:06 +0000
From:	Yuval Mintz <Yuval.Mintz@...gic.com>
To:	Stephen Hemminger <stephen@...workplumber.org>
CC:	netdev <netdev@...r.kernel.org>,
	Ariel Elior <Ariel.Elior@...gic.com>,
	Sudarsana Kalluru <Sudarsana.Kalluru@...gic.com>
Subject: RE: [RFC v2 net-next 05/10] qede: Add basic network device support

> > > +#define QEDE_NAPI_WEIGHT	(NAPI_POLL_WEIGHT)
> > Why not just use existing constant rather than walpapering?

No reason; We'll remove it.

> > > +#define U64_LO(x)			((u32)(((u64)(x)) & 0xffffffff))
> > > +#define U64_HI(x)			((u32)(((u64)(x)) >> 32))
> > > +#define HILO_U64(hi, lo)		((((u64)(hi)) << 32) + (lo))
> > 
> > Why reinvent upper32 and lower32?

Ignorance? And I assume you meant {upper, lower}_32_bits.
Is there also a standard macro doing what HILO_U64 does?

> > +struct qede_rx_queue {
> > +	__le16			*hw_cons_ptr;
> 
> The __ variants of constants should be reserved for use in user visible API's

Really? If so, this needs to be fixed not only here but in lots of places
in the series [e.g., entire HW HSI uses __le variants instead of le].
But why is it so? I.e., I understand that __le16 is defined in the uapi directory and
thus accessible to users, but why the distinction?
[I'm looking at bnx2x_hsi.h for reference which is full with the __le variants;
Not saying that's a good thing, though]

> > > +	int			rx_buf_size;
> > 
> > Shouldn't rx_buf_size be unsigned?

Yes.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html