[PATCH net 0/2] net: enetc: correct the statistics of rx bytes

[PATCH net 0/2] net: enetc: correct the statistics of rx bytes

[wei.fang]

From: Wei Fang <wei.fang@nxp.com>

The purpose of this patch set is to fix the issue of rx bytes
statistics. The first patch corrects the rx bytes statistics
of normal kernel protocol stack path, and the second patch is
used to correct the rx bytes statistics of XDP.

Wei Fang (2):
  net: enetc: correct the statistics of rx bytes
  net: enetc: correct rx_bytes statistics of XDP

 drivers/net/ethernet/freescale/enetc/enetc.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

-- 
2.25.1

[PATCH net 1/2] net: enetc: correct the statistics of rx bytes

[wei.fang]

From: Wei Fang <wei.fang@nxp.com>

The rx_bytes of struct net_device_stats should count the length of
ethernet frames excluding the FCS. However, there are two problems
with the rx_bytes statistics of the current enetc driver. one is
that the length of VLAN header is not counted if the VLAN extraction
feature is enabled. The other is that the length of L2 header is not
counted, because eth_type_trans() is invoked before updating rx_bytes
which will subtract the length of L2 header from skb->len.
BTW, the rx_bytes statistics of XDP path also have similar problem,
I will fix it in another patch.

Fixes: a800abd3ecb9 ("net: enetc: move skb creation into enetc_build_skb")
Signed-off-by: Wei Fang <wei.fang@nxp.com>
---
 drivers/net/ethernet/freescale/enetc/enetc.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c b/drivers/net/ethernet/freescale/enetc/enetc.c
index 3c4fa26f0f9b..d6c0f3f46c2a 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc.c
@@ -1229,7 +1229,13 @@ static int enetc_clean_rx_ring(struct enetc_bdr *rx_ring,
 		if (!skb)
 			break;
 
-		rx_byte_cnt += skb->len;
+		/* When set, the outer VLAN header is extracted and reported
+		 * in the receive buffer descriptor. So rx_byte_cnt should
+		 * add the length of the extracted VLAN header.
+		 */
+		if (bd_status & ENETC_RXBD_FLAG_VLAN)
+			rx_byte_cnt += VLAN_HLEN;
+		rx_byte_cnt += skb->len + ETH_HLEN;
 		rx_frm_cnt++;
 
 		napi_gro_receive(napi, skb);
-- 
2.25.1

Re: [PATCH net 1/2] net: enetc: correct the statistics of rx bytes

[Vladimir Oltean]

Hi Wei,

On Fri, Jun 02, 2023 at 05:46:58PM +0800, wei.fang@nxp.com wrote:
> From: Wei Fang <wei.fang@nxp.com>
> 
> The rx_bytes of struct net_device_stats should count the length of
> ethernet frames excluding the FCS. However, there are two problems
> with the rx_bytes statistics of the current enetc driver. one is
> that the length of VLAN header is not counted if the VLAN extraction
> feature is enabled. The other is that the length of L2 header is not
> counted, because eth_type_trans() is invoked before updating rx_bytes
> which will subtract the length of L2 header from skb->len.

Thanks for noticing the issue and for sending a patch.

> BTW, the rx_bytes statistics of XDP path also have similar problem,
> I will fix it in another patch.
> 
> Fixes: a800abd3ecb9 ("net: enetc: move skb creation into enetc_build_skb")
> Signed-off-by: Wei Fang <wei.fang@nxp.com>
> ---
>  drivers/net/ethernet/freescale/enetc/enetc.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c b/drivers/net/ethernet/freescale/enetc/enetc.c
> index 3c4fa26f0f9b..d6c0f3f46c2a 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc.c
> @@ -1229,7 +1229,13 @@ static int enetc_clean_rx_ring(struct enetc_bdr *rx_ring,
>  		if (!skb)
>  			break;
>  
> -		rx_byte_cnt += skb->len;
> +		/* When set, the outer VLAN header is extracted and reported
> +		 * in the receive buffer descriptor. So rx_byte_cnt should
> +		 * add the length of the extracted VLAN header.
> +		 */
> +		if (bd_status & ENETC_RXBD_FLAG_VLAN)
> +			rx_byte_cnt += VLAN_HLEN;
> +		rx_byte_cnt += skb->len + ETH_HLEN;

Hmm, to avoid the conditional, have you considered adding an "int *rx_byte_cnt"
argument to enetc_build_skb()? It can be updated with "(*rx_byte_cnt) += size"
from all places where we already have "(*cleaned_cnt)++".

>  		rx_frm_cnt++;
>  
>  		napi_gro_receive(napi, skb);
> -- 
> 2.25.1
>

Re: [PATCH net 1/2] net: enetc: correct the statistics of rx bytes

[Vladimir Oltean]

On Fri, Jun 02, 2023 at 01:27:10PM +0300, Vladimir Oltean wrote:
> Hmm, to avoid the conditional, have you considered adding an "int *rx_byte_cnt"
> argument to enetc_build_skb()? It can be updated with "(*rx_byte_cnt) += size"
> from all places where we already have "(*cleaned_cnt)++".

Ah, don't mind me. We can't avoid the conditional, since the VLAN header
is offloaded as you said. It's not in the buffer.

Re: [PATCH net 1/2] net: enetc: correct the statistics of rx bytes

[Vladimir Oltean]

On Fri, Jun 02, 2023 at 05:46:58PM +0800, wei.fang@nxp.com wrote:
> From: Wei Fang <wei.fang@nxp.com>
> 
> The rx_bytes of struct net_device_stats should count the length of
> ethernet frames excluding the FCS. However, there are two problems
> with the rx_bytes statistics of the current enetc driver. one is
> that the length of VLAN header is not counted if the VLAN extraction
> feature is enabled. The other is that the length of L2 header is not
> counted, because eth_type_trans() is invoked before updating rx_bytes
> which will subtract the length of L2 header from skb->len.
> BTW, the rx_bytes statistics of XDP path also have similar problem,
> I will fix it in another patch.
> 
> Fixes: a800abd3ecb9 ("net: enetc: move skb creation into enetc_build_skb")
> Signed-off-by: Wei Fang <wei.fang@nxp.com>
> ---

Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com>

By the way, you mangled Jakub's email (kuba@kernel.or) - I've fixed it up.

RE: [PATCH net 1/2] net: enetc: correct the statistics of rx bytes

[Wei Fang]

> -----Original Message-----
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> Sent: 2023年6月2日 18:32
> To: Wei Fang <wei.fang@nxp.com>
> Cc: Claudiu Manoil <claudiu.manoil@nxp.com>; davem@davemloft.net;
> edumazet@google.com; kuba@kernel.org; pabeni@redhat.com;
> ast@kernel.org; daniel@iogearbox.net; hawk@kernel.org;
> john.fastabend@gmail.com; netdev@vger.kernel.org;
> linux-kernel@vger.kernel.org
> Subject: Re: [PATCH net 1/2] net: enetc: correct the statistics of rx bytes
> 
> On Fri, Jun 02, 2023 at 05:46:58PM +0800, wei.fang@nxp.com wrote:
> > From: Wei Fang <wei.fang@nxp.com>
> >
> > The rx_bytes of struct net_device_stats should count the length of
> > ethernet frames excluding the FCS. However, there are two problems
> > with the rx_bytes statistics of the current enetc driver. one is that
> > the length of VLAN header is not counted if the VLAN extraction
> > feature is enabled. The other is that the length of L2 header is not
> > counted, because eth_type_trans() is invoked before updating rx_bytes
> > which will subtract the length of L2 header from skb->len.
> > BTW, the rx_bytes statistics of XDP path also have similar problem, I
> > will fix it in another patch.
> >
> > Fixes: a800abd3ecb9 ("net: enetc: move skb creation into
> > enetc_build_skb")
> > Signed-off-by: Wei Fang <wei.fang@nxp.com>
> > ---
> 
> Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> By the way, you mangled Jakub's email (kuba@kernel.or) - I've fixed it up.

Yes, I realized this after sending the email. Thank you so much!

[PATCH net 2/2] net: enetc: correct rx_bytes statistics of XDP

[wei.fang]

From: Wei Fang <wei.fang@nxp.com>

The rx_bytes statistics of XDP are always zero, because rx_byte_cnt
is not updated after it is initialized to 0. So fix it.

Fixes: d1b15102dd16 ("net: enetc: add support for XDP_DROP and XDP_PASS")
Signed-off-by: Wei Fang <wei.fang@nxp.com>
---
 drivers/net/ethernet/freescale/enetc/enetc.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c b/drivers/net/ethernet/freescale/enetc/enetc.c
index d6c0f3f46c2a..9e1b2536e9a9 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc.c
@@ -1571,6 +1571,14 @@ static int enetc_clean_rx_ring_xdp(struct enetc_bdr *rx_ring,
 		enetc_build_xdp_buff(rx_ring, bd_status, &rxbd, &i,
 				     &cleaned_cnt, &xdp_buff);
 
+		/* When set, the outer VLAN header is extracted and reported
+		 * in the receive buffer descriptor. So rx_byte_cnt should
+		 * add the length of the extracted VLAN header.
+		 */
+		if (bd_status & ENETC_RXBD_FLAG_VLAN)
+			rx_byte_cnt += VLAN_HLEN;
+		rx_byte_cnt += xdp_get_buff_len(&xdp_buff);
+
 		xdp_act = bpf_prog_run_xdp(prog, &xdp_buff);
 
 		switch (xdp_act) {
-- 
2.25.1

Re: [PATCH net 2/2] net: enetc: correct rx_bytes statistics of XDP

[Vladimir Oltean]

On Fri, Jun 02, 2023 at 05:46:59PM +0800, wei.fang@nxp.com wrote:
> From: Wei Fang <wei.fang@nxp.com>
> 
> The rx_bytes statistics of XDP are always zero, because rx_byte_cnt
> is not updated after it is initialized to 0. So fix it.
> 
> Fixes: d1b15102dd16 ("net: enetc: add support for XDP_DROP and XDP_PASS")
> Signed-off-by: Wei Fang <wei.fang@nxp.com>
> ---

Reviewed-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Thanks!

Re: [PATCH net 0/2] net: enetc: correct the statistics of rx bytes

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (main)
by David S. Miller <davem@davemloft.net>:

On Fri,  2 Jun 2023 17:46:57 +0800 you wrote:
> From: Wei Fang <wei.fang@nxp.com>
> 
> The purpose of this patch set is to fix the issue of rx bytes
> statistics. The first patch corrects the rx bytes statistics
> of normal kernel protocol stack path, and the second patch is
> used to correct the rx bytes statistics of XDP.
> 
> [...]

Here is the summary with links:
  - [net,1/2] net: enetc: correct the statistics of rx bytes
    https://git.kernel.org/netdev/net/c/7190d0ff0e17
  - [net,2/2] net: enetc: correct rx_bytes statistics of XDP
    https://git.kernel.org/netdev/net/c/fdebd850cc06

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html