Re: [PATCH net-next v3 3/7] net: lan966x: Add len field to lan966x_tx_dcb_buf

Alexander Lobakin
2022-11-22T11:36:28Z

From: Horatiu Vultur <horatiu.vultur@microchip.com>
Date: Mon, 21 Nov 2022 22:28:46 +0100

> Currently when a frame was transmitted, it is required to unamp the
> frame that was transmitted. The length of the frame was taken from the
> transmitted skb. In the future we might not have an skb, therefore store
> the length skb directly in the lan966x_tx_dcb_buf and use this one to
> unamp the frame.
>
> Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> ---
> drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c | 5 +++--
> drivers/net/ethernet/microchip/lan966x/lan966x_main.h | 1 +
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c b/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c
> index 94c720e59caee..384ed34197d58 100644
> --- a/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c
> +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c
> @@ -391,12 +391,12 @@ static void lan966x_fdma_tx_clear_buf(struct lan966x *lan966x, int weight)
> continue;
>
> dcb_buf->dev->stats.tx_packets++;
> - dcb_buf->dev->stats.tx_bytes += dcb_buf->skb->len;
> + dcb_buf->dev->stats.tx_bytes += dcb_buf->len;
>
> dcb_buf->used = false;
> dma_unmap_single(lan966x->dev,
> dcb_buf->dma_addr,
> - dcb_buf->skb->len,
> + dcb_buf->len,
> DMA_TO_DEVICE);
> if (!dcb_buf->ptp)
> dev_kfree_skb_any(dcb_buf->skb);
> @@ -709,6 +709,7 @@ int lan966x_fdma_xmit(struct sk_buff *skb, __be32 *ifh, struct net_device *dev)
> /* Fill up the buffer */
> next_dcb_buf = &tx->dcbs_buf[next_to_use];
> next_dcb_buf->skb = skb;
> + next_dcb_buf->len = skb->len;
> next_dcb_buf->dma_addr = dma_addr;
> next_dcb_buf->used = true;
> next_dcb_buf->ptp = false;
> diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_main.h b/drivers/net/ethernet/microchip/lan966x/lan966x_main.h
> index bc93051aa0798..7bb9098496f60 100644
> --- a/drivers/net/ethernet/microchip/lan966x/lan966x_main.h
> +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_main.h
> @@ -175,6 +175,7 @@ struct lan966x_rx {
> struct lan966x_tx_dcb_buf {
> struct net_device *dev;
> struct sk_buff *skb;
> + int len;

Nit: perhaps you can define it as `u32` since fram length can't be
negative?

> dma_addr_t dma_addr;

Oh, also, on platforms with 64-bit addressing, `int len` placed in
between ::skb and ::dma_addr would create a 4-byte hole in the
structure. Placing `len` after ::dma_addr would make it holeless on
any architercture.

> bool used;
> bool ptp;
> --
> 2.38.0

Thanks,
Olek