* [PATCH 0/1] e1000e: Fix tx/rx counters @ 2023-04-10 15:24 timothee.cocault 2023-04-10 15:27 ` [PATCH 1/1] " timothee.cocault 0 siblings, 1 reply; 6+ messages in thread From: timothee.cocault @ 2023-04-10 15:24 UTC (permalink / raw) To: qemu-devel; +Cc: Jason Wang, Dmitry Fleytman, Akihiko Odaki This commit fixes a bug in the stats registers of the e1000* devices reporting higher bandwidth usage. I'm adding a bit of context for affected Googlers: The e1000e is the default NIC used by libvirt for Windows VMs. This bug creates huge slowdowns on BITS transfers (used for Windows Update for example). The task manager shows high usage of bandwidth (>2Gbps inbound), and thus the BITS client throttles the speed (<1kbps). A temporary fix is to switch to the rtl8139 device. Timothée Cocault (1): e1000e: Fix tx/rx counters 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(-) ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/1] e1000e: Fix tx/rx counters 2023-04-10 15:24 [PATCH 0/1] e1000e: Fix tx/rx counters timothee.cocault @ 2023-04-10 15:27 ` timothee.cocault 2023-04-10 15:33 ` Akihiko Odaki 2023-05-10 18:54 ` Michael Tokarev 0 siblings, 2 replies; 6+ messages in thread From: timothee.cocault @ 2023-04-10 15:27 UTC (permalink / raw) To: qemu-devel; +Cc: Jason Wang, Dmitry Fleytman, Akihiko Odaki 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> --- 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; -- 2.40.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] e1000e: Fix tx/rx counters 2023-04-10 15:27 ` [PATCH 1/1] " timothee.cocault @ 2023-04-10 15:33 ` Akihiko Odaki 2023-05-10 5:12 ` Jason Wang 2023-05-10 18:54 ` Michael Tokarev 1 sibling, 1 reply; 6+ messages in thread From: Akihiko Odaki @ 2023-04-10 15:33 UTC (permalink / raw) To: timothee.cocault, qemu-devel; +Cc: Jason Wang, Dmitry Fleytman 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; ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] e1000e: Fix tx/rx counters 2023-04-10 15:33 ` Akihiko Odaki @ 2023-05-10 5:12 ` Jason Wang 0 siblings, 0 replies; 6+ messages in thread From: Jason Wang @ 2023-05-10 5:12 UTC (permalink / raw) To: Akihiko Odaki, timothee.cocault, qemu-devel; +Cc: Dmitry Fleytman 在 2023/4/10 23:33, Akihiko Odaki 写道: > 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> Queued. Thanks > >> --- >> 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; > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] e1000e: Fix tx/rx counters 2023-04-10 15:27 ` [PATCH 1/1] " timothee.cocault 2023-04-10 15:33 ` Akihiko Odaki @ 2023-05-10 18:54 ` Michael Tokarev 2023-05-11 0:30 ` Jason Wang 1 sibling, 1 reply; 6+ messages in thread From: Michael Tokarev @ 2023-05-10 18:54 UTC (permalink / raw) To: timothee.cocault, qemu-devel; +Cc: Jason Wang, Dmitry Fleytman, Akihiko Odaki 10.04.2023 18: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. Smells like a -stable material, is it not? Thanks, /mjt ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] e1000e: Fix tx/rx counters 2023-05-10 18:54 ` Michael Tokarev @ 2023-05-11 0:30 ` Jason Wang 0 siblings, 0 replies; 6+ messages in thread From: Jason Wang @ 2023-05-11 0:30 UTC (permalink / raw) To: Michael Tokarev Cc: timothee.cocault, qemu-devel, Dmitry Fleytman, Akihiko Odaki On Thu, May 11, 2023 at 2:54 AM Michael Tokarev <mjt@tls.msk.ru> wrote: > > 10.04.2023 18: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. > > Smells like a -stable material, is it not? Yes, tagged for stable. Thanks > > Thanks, > > /mjt > ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-05-11 0:31 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-04-10 15:24 [PATCH 0/1] e1000e: Fix tx/rx counters timothee.cocault 2023-04-10 15:27 ` [PATCH 1/1] " timothee.cocault 2023-04-10 15:33 ` Akihiko Odaki 2023-05-10 5:12 ` Jason Wang 2023-05-10 18:54 ` Michael Tokarev 2023-05-11 0:30 ` Jason Wang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).