Re: [PATCH net-next v4 6/8] net: bcmgenet: pad transmit frames out of the packet ready window

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The new field tx_thld_len is used only on transmit: it is set in
  bcmgenet_set_mtu_regs() and read in bcmgenet_xmit().

--

net: bcmgenet: pad transmit frames out of the packet ready window

The commit programs a transmit packet ready threshold that depends on the
MTU. Frames that land just past that threshold are padded so they no
longer stall the transmitter. A frame longer than the threshold gets its
checksum computed in software, and the skb is unshared before either path
writes to it.

[ ... ]

> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.h b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
> index 28395cb72fb60..7703e1fb73b6c 100644
> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.h
> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
> @@ -624,6 +624,7 @@ struct bcmgenet_priv {
>  	struct enet_cb *rx_cbs;
>  	unsigned int num_rx_bds;
>  	unsigned int rx_buf_len;
> +	unsigned int tx_thld_len;

[Severity: Low]
This isn't a bug, but tx_thld_len only matters on transmit. It is set in
bcmgenet_set_mtu_regs() and read in bcmgenet_xmit().

In struct bcmgenet_priv it sits under the "/* receive variables */" comment,
next to rx_buf_len:

	/* receive variables */
	void __iomem *rx_bds;
	struct enet_cb *rx_cbs;
	unsigned int num_rx_bds;
	unsigned int rx_buf_len;
	unsigned int tx_thld_len;

Could it go under the "/* transmit variables */" block with num_tx_bds?

>  	struct bcmgenet_rxnfc_rule rxnfc_rules[MAX_NUM_OF_FS_RULES];
>  	struct list_head rxnfc_list;
>

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261012-nb-genet-mtu-nn-v2-v4-0-2e2e3bfaf8cc%40tipi-net.de