netdev - Re: [PATCH net-next] eea: Add basic driver framework for Alibaba Elastic Ethernet Adaptor

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]

Message-ID: <3cb1810d-377d-4988-bf8a-75274f7b8216@lunn.ch>
Date: Wed, 16 Jul 2025 18:32:14 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Xuan Zhuo <xuanzhuo@...ux.alibaba.com>
Cc: Simon Horman <horms@...nel.org>, netdev@...r.kernel.org,
	Andrew Lunn <andrew+netdev@...n.ch>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Wen Gu <guwen@...ux.alibaba.com>,
	Philo Lu <lulie@...ux.alibaba.com>,
	Lorenzo Bianconi <lorenzo@...nel.org>,
	Lukas Bulwahn <lukas.bulwahn@...hat.com>,
	Parthiban Veerasooran <Parthiban.Veerasooran@...rochip.com>,
	Geert Uytterhoeven <geert+renesas@...der.be>,
	Alexander Duyck <alexanderduyck@...com>,
	Dust Li <dust.li@...ux.alibaba.com>
Subject: Re: [PATCH net-next] eea: Add basic driver framework for Alibaba
 Elastic Ethernet Adaptor

> > > +	ret = eea_adminq_submit(enet, cmd, req_addr, res_addr, req_size, res_size);
> >
> > Please arrange Networking code so that it is 80 columns wide or less,
> > where that can be done without reducing readability. E.g. don't split
> > strings across multiple lines. Do wrap lines like the one above like this:
> >
> > 	ret = eea_adminq_submit(enet, cmd, req_addr, res_addr, req_size,
> > 				res_size);
> >
> > Note that the start of the non-whitespace portion of the 2nd line
> > is aligned to be exactly inside the opening parentheses of the previous
> > line.
> >
> > checkpatch.pl --max-line-length=80 is useful here.
> 
> We are aware of the current limit of 100 characters, and we have been coding
> according to that guideline. Of course, we try to keep lines within 80
> characters where possible. However, in some cases, we find that using up to 100
> characters improves readability, so 80 is not a strict requirement for us.
> 
> Is there a specific rule or convention in the networking area that we should
> follow? Sorry, I have not heard of such a rule before.

That suggests to me you are not subscribed to the netdev list and are
not reading reviews made to other drivers. This comes up every couple
of weeks. You should be spending a little bit of time very day just
looking at the comments other patches get, and make sure you don't
make the same mistakes.

In this particularly case, i don't think wrapping the line makes any
difference to readability. There are some cases where it does, which
is why you don't 100% enforce checkpatch. But in general, you should
keep with 80 for networking.

> > > +#define EEA_NET_PT_UDPv6_EX  9
> > > +	__le16 pkt_type:10,
> > > +	       reserved1:6;
> >
> > Sparse complains about the above. And I'm not at all sure that
> > a __le16 bitfield works as intended on a big endian system.
> >
> > I would suggest some combination of: FIELD_PREP, FIELD_GET, GENMASK,
> > cpu_to_le16() and le16_to_cpu().
> >
> > Also, please do make sure patches don't introduce new Sparse warnings.
> 
> I will try.

FYI: We take sparse warnings pretty seriously. So please try quite
hard.

	Andrew