Re: [PATCH 1/1] e1000e: Fix tx/rx counters

[Akihiko Odaki <akihiko.odaki@daynix.com>]

On 2023/04/11 0:27, timothee.cocault@gmail.com wrote:
> The bytes and packets counter registers are cleared on read.
> 
> Copying the "total counter" registers to the "good counter" registers has
> side effects.
> If the "total" register is never read by the OS, it only gets incremented.
> This leads to exponential growth of the "good" register.
> 
> This commit increments the counters individually to avoid this.
> 
> Signed-off-by: Timothée Cocault <timothee.cocault@gmail.com>

Reviewed-by: Akihiko Odaki <akihiko.odaki@daynix.com>

> ---
>   hw/net/e1000.c         | 5 ++---
>   hw/net/e1000e_core.c   | 5 ++---
>   hw/net/e1000x_common.c | 5 ++---
>   hw/net/igb_core.c      | 5 ++---
>   4 files changed, 8 insertions(+), 12 deletions(-)
> 
> diff --git a/hw/net/e1000.c b/hw/net/e1000.c
> index 23d660619f..59bacb5d3b 100644
> --- a/hw/net/e1000.c
> +++ b/hw/net/e1000.c
> @@ -637,9 +637,8 @@ xmit_seg(E1000State *s)
>   
>       e1000x_inc_reg_if_not_full(s->mac_reg, TPT);
>       e1000x_grow_8reg_if_not_full(s->mac_reg, TOTL, s->tx.size + 4);
> -    s->mac_reg[GPTC] = s->mac_reg[TPT];
> -    s->mac_reg[GOTCL] = s->mac_reg[TOTL];
> -    s->mac_reg[GOTCH] = s->mac_reg[TOTH];
> +    e1000x_inc_reg_if_not_full(s->mac_reg, GPTC);
> +    e1000x_grow_8reg_if_not_full(s->mac_reg, GOTCL, s->tx.size + 4);
>   }
>   
>   static void
> diff --git a/hw/net/e1000e_core.c b/hw/net/e1000e_core.c
> index c0c09b6965..cfa3f55e96 100644
> --- a/hw/net/e1000e_core.c
> +++ b/hw/net/e1000e_core.c
> @@ -711,9 +711,8 @@ e1000e_on_tx_done_update_stats(E1000ECore *core, struct NetTxPkt *tx_pkt)
>           g_assert_not_reached();
>       }
>   
> -    core->mac[GPTC] = core->mac[TPT];
> -    core->mac[GOTCL] = core->mac[TOTL];
> -    core->mac[GOTCH] = core->mac[TOTH];
> +    e1000x_inc_reg_if_not_full(core->mac, GPTC);
> +    e1000x_grow_8reg_if_not_full(core->mac, GOTCL, tot_len);
>   }
>   
>   static void
> diff --git a/hw/net/e1000x_common.c b/hw/net/e1000x_common.c
> index b844af590a..4c8e7dcf70 100644
> --- a/hw/net/e1000x_common.c
> +++ b/hw/net/e1000x_common.c
> @@ -220,15 +220,14 @@ e1000x_update_rx_total_stats(uint32_t *mac,
>   
>       e1000x_increase_size_stats(mac, PRCregs, data_fcs_size);
>       e1000x_inc_reg_if_not_full(mac, TPR);
> -    mac[GPRC] = mac[TPR];
> +    e1000x_inc_reg_if_not_full(mac, GPRC);
>       /* TOR - Total Octets Received:
>       * This register includes bytes received in a packet from the <Destination
>       * Address> field through the <CRC> field, inclusively.
>       * Always include FCS length (4) in size.
>       */
>       e1000x_grow_8reg_if_not_full(mac, TORL, data_size + 4);
> -    mac[GORCL] = mac[TORL];
> -    mac[GORCH] = mac[TORH];
> +    e1000x_grow_8reg_if_not_full(mac, GORCL, data_size + 4);
>   }
>   
>   void
> diff --git a/hw/net/igb_core.c b/hw/net/igb_core.c
> index d733fed6cf..826e7a6cf1 100644
> --- a/hw/net/igb_core.c
> +++ b/hw/net/igb_core.c
> @@ -538,9 +538,8 @@ igb_on_tx_done_update_stats(IGBCore *core, struct NetTxPkt *tx_pkt, int qn)
>           g_assert_not_reached();
>       }
>   
> -    core->mac[GPTC] = core->mac[TPT];
> -    core->mac[GOTCL] = core->mac[TOTL];
> -    core->mac[GOTCH] = core->mac[TOTH];
> +    e1000x_inc_reg_if_not_full(core->mac, GPTC);
> +    e1000x_grow_8reg_if_not_full(core->mac, GOTCL, tot_len);
>   
>       if (core->mac[MRQC] & 1) {
>           uint16_t pool = qn % IGB_NUM_VM_POOLS;