| Message ID | 20240905063645.586354-1-yanzhen@vivo.com (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next,v1] net: mvneta: Avoid the misuse of the '_t' variants | expand |
From: Yan Zhen > Sent: 05 September 2024 07:37 > > Type conversions (especially when narrowing down types) can lead to > unexpected behavior and should be done with extreme caution. > > In this case if someone tries to set the tx_pending to 65536(0x10000), > after forcing it to convert to u16, it becomes 0x0000, they will get > the minimum supported size and not the maximum. > > Based on previous discussions [1], such behavior may confuse users or even > create unexpected errors. > > [1] https://lore.kernel.org/netdev/d23dfbf563714d7090d163a075ca9a51@AcuMS.aculab.com/ > > Signed-off-by: Yan Zhen <yanzhen@vivo.com> Reviewed-by: david.laight@aculab.com > --- > drivers/net/ethernet/marvell/mvneta.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c > index d72b2d5f96db..b41de182cc88 100644 > --- a/drivers/net/ethernet/marvell/mvneta.c > +++ b/drivers/net/ethernet/marvell/mvneta.c > @@ -4753,8 +4753,8 @@ mvneta_ethtool_set_ringparam(struct net_device *dev, > pp->rx_ring_size = ring->rx_pending < MVNETA_MAX_RXD ? > ring->rx_pending : MVNETA_MAX_RXD; > > - pp->tx_ring_size = clamp_t(u16, ring->tx_pending, > - MVNETA_MAX_SKB_DESCS * 2, MVNETA_MAX_TXD); > + pp->tx_ring_size = clamp(ring->tx_pending, > + MVNETA_MAX_SKB_DESCS * 2, MVNETA_MAX_TXD); > if (pp->tx_ring_size != ring->tx_pending) > netdev_warn(dev, "TX queue size set to %u (requested %u)\n", > pp->tx_ring_size, ring->tx_pending); > -- > 2.34.1 - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
On Thu, 5 Sep 2024 14:36:45 +0800 Yan Zhen wrote: > In this case if someone tries to set the tx_pending to 65536(0x10000), > after forcing it to convert to u16, it becomes 0x0000, they will get > the minimum supported size and not the maximum. Core validates against tx_max_pending before calling the driver.
diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c index d72b2d5f96db..b41de182cc88 100644 --- a/drivers/net/ethernet/marvell/mvneta.c +++ b/drivers/net/ethernet/marvell/mvneta.c @@ -4753,8 +4753,8 @@ mvneta_ethtool_set_ringparam(struct net_device *dev, pp->rx_ring_size = ring->rx_pending < MVNETA_MAX_RXD ? ring->rx_pending : MVNETA_MAX_RXD; - pp->tx_ring_size = clamp_t(u16, ring->tx_pending, - MVNETA_MAX_SKB_DESCS * 2, MVNETA_MAX_TXD); + pp->tx_ring_size = clamp(ring->tx_pending, + MVNETA_MAX_SKB_DESCS * 2, MVNETA_MAX_TXD); if (pp->tx_ring_size != ring->tx_pending) netdev_warn(dev, "TX queue size set to %u (requested %u)\n", pp->tx_ring_size, ring->tx_pending);
Type conversions (especially when narrowing down types) can lead to unexpected behavior and should be done with extreme caution. In this case if someone tries to set the tx_pending to 65536(0x10000), after forcing it to convert to u16, it becomes 0x0000, they will get the minimum supported size and not the maximum. Based on previous discussions [1], such behavior may confuse users or even create unexpected errors. [1] https://lore.kernel.org/netdev/d23dfbf563714d7090d163a075ca9a51@AcuMS.aculab.com/ Signed-off-by: Yan Zhen <yanzhen@vivo.com> --- drivers/net/ethernet/marvell/mvneta.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)