diff mbox series

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

Message ID 20221121212850.3212649-4-horatiu.vultur@microchip.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series net: lan966x: Extend xdp support | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next, async
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Series has a cover letter
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers success CCed 8 of 8 maintainers
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 28 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Horatiu Vultur Nov. 21, 2022, 9:28 p.m. UTC
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(-)

Comments

Alexander Lobakin Nov. 22, 2022, 11:30 a.m. UTC | #1
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
Horatiu Vultur Nov. 22, 2022, 9:21 p.m. UTC | #2
The 11/22/2022 12:30, Alexander Lobakin wrote:
> 
> 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?

The length is always positive. I will update this in the next version.

> 
> >       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.

Thanks for the suggestion.
I will make sure that lan966x_tx_dcb_buf will not have any holes. In
this patch I will arrange the members and in the next patches where
I will modify the struct, I will add them at the right place.

> 
> >       bool used;
> >       bool ptp;
> > --
> > 2.38.0
> 
> Thanks,
> Olek
diff mbox series

Patch

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;
 	dma_addr_t dma_addr;
 	bool used;
 	bool ptp;