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: <10a14ce9-1cbb-4906-8363-99c8fc3c7fb6@lunn.ch>
Date: Fri, 5 Apr 2024 17:41:47 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Alexander Lobakin <aleksander.lobakin@...el.com>
Cc: "David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	David Ahern <dsahern@...nel.org>,
	Xuan Zhuo <xuanzhuo@...ux.alibaba.com>,
	nex.sw.ncis.osdt.itp.upstreaming@...el.com, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH RFC net-next 1/7] netdev_features: remove unused
 __UNUSED_NETIF_F_1

On Fri, Apr 05, 2024 at 05:15:58PM +0200, Alexander Lobakin wrote:
> From: Andrew Lunn <andrew@...n.ch>
> Date: Fri, 5 Apr 2024 16:12:50 +0200
> 
> > On Fri, Apr 05, 2024 at 03:37:25PM +0200, Alexander Lobakin wrote:
> >> NETIF_F_NO_CSUM was removed in 3.2-rc2 by commit 34324dc2bf27
> >> ("net: remove NETIF_F_NO_CSUM feature bit") and became
> >> __UNUSED_NETIF_F_1. It's not used anywhere in the code.
> >> Remove this bit waste.
> >>
> >> Signed-off-by: Alexander Lobakin <aleksander.lobakin@...el.com>
> >> ---
> >>  include/linux/netdev_features.h | 1 -
> >>  1 file changed, 1 deletion(-)
> >>
> >> diff --git a/include/linux/netdev_features.h b/include/linux/netdev_features.h
> >> index 7c2d77d75a88..44c428d62db4 100644
> >> --- a/include/linux/netdev_features.h
> >> +++ b/include/linux/netdev_features.h
> >> @@ -14,7 +14,6 @@ typedef u64 netdev_features_t;
> >>  enum {
> >>  	NETIF_F_SG_BIT,			/* Scatter/gather IO. */
> >>  	NETIF_F_IP_CSUM_BIT,		/* Can checksum TCP/UDP over IPv4. */
> >> -	__UNUSED_NETIF_F_1,
> >>  	NETIF_F_HW_CSUM_BIT,		/* Can checksum all the packets. */
> >>  	NETIF_F_IPV6_CSUM_BIT,		/* Can checksum TCP/UDP over IPV6 */
> >>  	NETIF_F_HIGHDMA_BIT,		/* Can DMA to high memory. */
> > 
> > Are you sure this enum is not ABI?
> 
> Why should this be ABI? It's not a part of UAPI and Ethtool receives
> these bits together with string names.

As a reviewer, i think about ABI. When looking at a change like this,
it is the first thing i think of. Our code is not always clean, there
could well be things outside of include/uapi which influence the
ABI. For some reason ("net: remove NETIF_F_NO_CSUM feature bit")
renamed rather than removed it? Why?

I assume you have looked into the code in this respect, you have
tested both and ioctl and netlink code, and concluded it does not
cause an ABI change. It could of been removed in 3.2-rc2. But i don't
see anything in the cover letter or commit message which indicates you
have done that. That is partially what the cover letter and the commit
message is about. Explaining things you have done, but cannot be seen
in the code change.

	Andrew

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ