netdev - Re: [PATCH net-next v3] net: mvneta: Use min macro

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] [day] [month] [year] [list]

Message-ID: <20240901183046.GB23170@kernel.org>
Date: Sun, 1 Sep 2024 19:30:46 +0100
From: Simon Horman <horms@...nel.org>
To: David Laight <David.Laight@...lab.com>
Cc: 'Yan Zhen' <yanzhen@...o.com>,
	"marcin.s.wojtas@...il.com" <marcin.s.wojtas@...il.com>,
	"davem@...emloft.net" <davem@...emloft.net>,
	"edumazet@...gle.com" <edumazet@...gle.com>,
	"kuba@...nel.org" <kuba@...nel.org>,
	"pabeni@...hat.com" <pabeni@...hat.com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"opensource.kernel@...o.com" <opensource.kernel@...o.com>
Subject: Re: [PATCH net-next v3] net: mvneta: Use min macro

On Sun, Sep 01, 2024 at 10:52:38AM +0000, David Laight wrote:
> From: Yan Zhen
> > Sent: 30 August 2024 02:04
> > To: marcin.s.wojtas@...il.com; davem@...emloft.net; edumazet@...gle.com; kuba@...nel.org;
> > 
> > Using the real macro is usually more intuitive and readable,
> > When the original file is guaranteed to contain the minmax.h header file
> > and compile correctly.
> > 
> > Signed-off-by: Yan Zhen <yanzhen@...o.com>
> > ---
> > 
> > Changes in v3:
> > - Rewrite the subject.
> > 
> >  drivers/net/ethernet/marvell/mvneta.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
> > index d72b2d5f96db..08d277165f40 100644
> > --- a/drivers/net/ethernet/marvell/mvneta.c
> > +++ b/drivers/net/ethernet/marvell/mvneta.c
> > @@ -4750,8 +4750,7 @@ mvneta_ethtool_set_ringparam(struct net_device *dev,
> > 
> >  	if ((ring->rx_pending == 0) || (ring->tx_pending == 0))
> >  		return -EINVAL;
> > -	pp->rx_ring_size = ring->rx_pending < MVNETA_MAX_RXD ?
> > -		ring->rx_pending : MVNETA_MAX_RXD;
> > +	pp->rx_ring_size = umin(ring->rx_pending, MVNETA_MAX_RXD);
> 
> Why did you use umin() instead of min() ?

Possibly because I mistakenly advised it is appropriate, sorry about that.
Given your explanation elsewhere [1], I now agree min() is appropriate.

[1] https://lore.kernel.org/netdev/20240901171150.GA23170@kernel.org/T/#mebc52fc11de13eff8a610e3a63c5d1026d527492

> >  	pp->tx_ring_size = clamp_t(u16, ring->tx_pending,
> >  				   MVNETA_MAX_SKB_DESCS * 2, MVNETA_MAX_TXD);
> 
> Hmmm how about a patch to fix the bug in that line?
> A typical example of the complete misuse of the '_t' variants.
> The fact that the LHS is u16 doesn't mean that it is anyway
> correct to cast the RHS value to u16.
> In this case if someone tries to set the ring size to 64k they'll