qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Jason Wang <jasowang@redhat.com>
To: qemu-devel@nongnu.org, peter.maydell@linaro.org
Cc: Akihiko Odaki <akihiko.odaki@daynix.com>,
	Sriram Yagnaraman <sriram.yagnaraman@est.tech>,
	Jason Wang <jasowang@redhat.com>
Subject: [PULL 04/50] e1000x: Fix BPRC and MPRC
Date: Tue, 23 May 2023 15:31:52 +0800	[thread overview]
Message-ID: <20230523073238.54236-5-jasowang@redhat.com> (raw)
In-Reply-To: <20230523073238.54236-1-jasowang@redhat.com>

From: Akihiko Odaki <akihiko.odaki@daynix.com>

Before this change, e1000 and the common code updated BPRC and MPRC
depending on the matched filter, but e1000e and igb decided to update
those counters by deriving the packet type independently. This
inconsistency caused a multicast packet to be counted twice.

Updating BPRC and MPRC depending on are fundamentally flawed anyway as
a filter can be used for different types of packets. For example, it is
possible to filter broadcast packets with MTA.

Always determine what counters to update by inspecting the packets.

Fixes: 3b27430177 ("e1000: Implementing various counters")
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Reviewed-by: Sriram Yagnaraman <sriram.yagnaraman@est.tech>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/net/e1000.c         |  6 +++---
 hw/net/e1000e_core.c   | 20 +++-----------------
 hw/net/e1000x_common.c | 25 +++++++++++++++++++------
 hw/net/e1000x_common.h |  5 +++--
 hw/net/igb_core.c      | 22 +++++-----------------
 5 files changed, 33 insertions(+), 45 deletions(-)

diff --git a/hw/net/e1000.c b/hw/net/e1000.c
index 59bacb5..18eb6d8 100644
--- a/hw/net/e1000.c
+++ b/hw/net/e1000.c
@@ -826,12 +826,10 @@ receive_filter(E1000State *s, const uint8_t *buf, int size)
     }
 
     if (ismcast && (rctl & E1000_RCTL_MPE)) {          /* promiscuous mcast */
-        e1000x_inc_reg_if_not_full(s->mac_reg, MPRC);
         return 1;
     }
 
     if (isbcast && (rctl & E1000_RCTL_BAM)) {          /* broadcast enabled */
-        e1000x_inc_reg_if_not_full(s->mac_reg, BPRC);
         return 1;
     }
 
@@ -922,6 +920,7 @@ e1000_receive_iov(NetClientState *nc, const struct iovec *iov, int iovcnt)
     size_t desc_offset;
     size_t desc_size;
     size_t total_size;
+    eth_pkt_types_e pkt_type;
 
     if (!e1000x_hw_rx_enabled(s->mac_reg)) {
         return -1;
@@ -971,6 +970,7 @@ e1000_receive_iov(NetClientState *nc, const struct iovec *iov, int iovcnt)
         size -= 4;
     }
 
+    pkt_type = get_eth_packet_type(PKT_GET_ETH_HDR(filter_buf));
     rdh_start = s->mac_reg[RDH];
     desc_offset = 0;
     total_size = size + e1000x_fcs_len(s->mac_reg);
@@ -1036,7 +1036,7 @@ e1000_receive_iov(NetClientState *nc, const struct iovec *iov, int iovcnt)
         }
     } while (desc_offset < total_size);
 
-    e1000x_update_rx_total_stats(s->mac_reg, size, total_size);
+    e1000x_update_rx_total_stats(s->mac_reg, pkt_type, size, total_size);
 
     n = E1000_ICS_RXT0;
     if ((rdt = s->mac_reg[RDT]) < s->mac_reg[RDH])
diff --git a/hw/net/e1000e_core.c b/hw/net/e1000e_core.c
index 15821a7..c2d864a 100644
--- a/hw/net/e1000e_core.c
+++ b/hw/net/e1000e_core.c
@@ -1488,24 +1488,10 @@ e1000e_write_to_rx_buffers(E1000ECore *core,
 }
 
 static void
-e1000e_update_rx_stats(E1000ECore *core,
-                       size_t data_size,
-                       size_t data_fcs_size)
+e1000e_update_rx_stats(E1000ECore *core, size_t pkt_size, size_t pkt_fcs_size)
 {
-    e1000x_update_rx_total_stats(core->mac, data_size, data_fcs_size);
-
-    switch (net_rx_pkt_get_packet_type(core->rx_pkt)) {
-    case ETH_PKT_BCAST:
-        e1000x_inc_reg_if_not_full(core->mac, BPRC);
-        break;
-
-    case ETH_PKT_MCAST:
-        e1000x_inc_reg_if_not_full(core->mac, MPRC);
-        break;
-
-    default:
-        break;
-    }
+    eth_pkt_types_e pkt_type = net_rx_pkt_get_packet_type(core->rx_pkt);
+    e1000x_update_rx_total_stats(core->mac, pkt_type, pkt_size, pkt_fcs_size);
 }
 
 static inline bool
diff --git a/hw/net/e1000x_common.c b/hw/net/e1000x_common.c
index 4c8e7dc..7694673 100644
--- a/hw/net/e1000x_common.c
+++ b/hw/net/e1000x_common.c
@@ -80,7 +80,6 @@ bool e1000x_rx_group_filter(uint32_t *mac, const uint8_t *buf)
     f = mta_shift[(rctl >> E1000_RCTL_MO_SHIFT) & 3];
     f = (((buf[5] << 8) | buf[4]) >> f) & 0xfff;
     if (mac[MTA + (f >> 5)] & (1 << (f & 0x1f))) {
-        e1000x_inc_reg_if_not_full(mac, MPRC);
         return true;
     }
 
@@ -212,13 +211,14 @@ e1000x_rxbufsize(uint32_t rctl)
 
 void
 e1000x_update_rx_total_stats(uint32_t *mac,
-                             size_t data_size,
-                             size_t data_fcs_size)
+                             eth_pkt_types_e pkt_type,
+                             size_t pkt_size,
+                             size_t pkt_fcs_size)
 {
     static const int PRCregs[6] = { PRC64, PRC127, PRC255, PRC511,
                                     PRC1023, PRC1522 };
 
-    e1000x_increase_size_stats(mac, PRCregs, data_fcs_size);
+    e1000x_increase_size_stats(mac, PRCregs, pkt_fcs_size);
     e1000x_inc_reg_if_not_full(mac, TPR);
     e1000x_inc_reg_if_not_full(mac, GPRC);
     /* TOR - Total Octets Received:
@@ -226,8 +226,21 @@ e1000x_update_rx_total_stats(uint32_t *mac,
     * 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);
-    e1000x_grow_8reg_if_not_full(mac, GORCL, data_size + 4);
+    e1000x_grow_8reg_if_not_full(mac, TORL, pkt_size + 4);
+    e1000x_grow_8reg_if_not_full(mac, GORCL, pkt_size + 4);
+
+    switch (pkt_type) {
+    case ETH_PKT_BCAST:
+        e1000x_inc_reg_if_not_full(mac, BPRC);
+        break;
+
+    case ETH_PKT_MCAST:
+        e1000x_inc_reg_if_not_full(mac, MPRC);
+        break;
+
+    default:
+        break;
+    }
 }
 
 void
diff --git a/hw/net/e1000x_common.h b/hw/net/e1000x_common.h
index 911abd8..0298e06 100644
--- a/hw/net/e1000x_common.h
+++ b/hw/net/e1000x_common.h
@@ -91,8 +91,9 @@ e1000x_update_regs_on_link_up(uint32_t *mac, uint16_t *phy)
 }
 
 void e1000x_update_rx_total_stats(uint32_t *mac,
-                                  size_t data_size,
-                                  size_t data_fcs_size);
+                                  eth_pkt_types_e pkt_type,
+                                  size_t pkt_size,
+                                  size_t pkt_fcs_size);
 
 void e1000x_core_prepare_eeprom(uint16_t       *eeprom,
                                 const uint16_t *templ,
diff --git a/hw/net/igb_core.c b/hw/net/igb_core.c
index abfdce9..464a41d 100644
--- a/hw/net/igb_core.c
+++ b/hw/net/igb_core.c
@@ -1438,29 +1438,17 @@ igb_write_to_rx_buffers(IGBCore *core,
 
 static void
 igb_update_rx_stats(IGBCore *core, const E1000E_RingInfo *rxi,
-                    size_t data_size, size_t data_fcs_size)
+                    size_t pkt_size, size_t pkt_fcs_size)
 {
-    e1000x_update_rx_total_stats(core->mac, data_size, data_fcs_size);
-
-    switch (net_rx_pkt_get_packet_type(core->rx_pkt)) {
-    case ETH_PKT_BCAST:
-        e1000x_inc_reg_if_not_full(core->mac, BPRC);
-        break;
-
-    case ETH_PKT_MCAST:
-        e1000x_inc_reg_if_not_full(core->mac, MPRC);
-        break;
-
-    default:
-        break;
-    }
+    eth_pkt_types_e pkt_type = net_rx_pkt_get_packet_type(core->rx_pkt);
+    e1000x_update_rx_total_stats(core->mac, pkt_type, pkt_size, pkt_fcs_size);
 
     if (core->mac[MRQC] & 1) {
         uint16_t pool = rxi->idx % IGB_NUM_VM_POOLS;
 
-        core->mac[PVFGORC0 + (pool * 64)] += data_size + 4;
+        core->mac[PVFGORC0 + (pool * 64)] += pkt_size + 4;
         core->mac[PVFGPRC0 + (pool * 64)]++;
-        if (net_rx_pkt_get_packet_type(core->rx_pkt) == ETH_PKT_MCAST) {
+        if (pkt_type == ETH_PKT_MCAST) {
             core->mac[PVFMPRC0 + (pool * 64)]++;
         }
     }
-- 
2.7.4



  parent reply	other threads:[~2023-05-23  7:34 UTC|newest]

Thread overview: 55+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-23  7:31 [PULL 00/50] Net patches Jason Wang
2023-05-23  7:31 ` [PULL 01/50] e1000e: Fix tx/rx counters Jason Wang
2023-05-23  7:31 ` [PULL 02/50] hw/net/net_tx_pkt: Decouple implementation from PCI Jason Wang
2023-05-23  7:31 ` [PULL 03/50] hw/net/net_tx_pkt: Decouple interface " Jason Wang
2023-05-23  7:31 ` Jason Wang [this message]
2023-05-23  7:31 ` [PULL 05/50] igb: Fix Rx packet type encoding Jason Wang
2023-05-23  7:31 ` [PULL 06/50] igb: Do not require CTRL.VME for tx VLAN tagging Jason Wang
2023-05-23  7:31 ` [PULL 07/50] igb: Clear IMS bits when committing ICR access Jason Wang
2023-05-23  7:31 ` [PULL 08/50] net/net_rx_pkt: Use iovec for net_rx_pkt_set_protocols() Jason Wang
2023-05-23  7:31 ` [PULL 09/50] e1000e: Always copy ethernet header Jason Wang
2023-05-23  7:31 ` [PULL 10/50] igb: " Jason Wang
2023-05-23  7:31 ` [PULL 11/50] Fix references to igb Avocado test Jason Wang
2023-05-23  7:32 ` [PULL 12/50] tests/avocado: Remove unused imports Jason Wang
2023-05-23  7:32 ` [PULL 13/50] tests/avocado: Remove test_igb_nomsi_kvm Jason Wang
2023-05-23  7:32 ` [PULL 14/50] hw/net/net_tx_pkt: Remove net_rx_pkt_get_l4_info Jason Wang
2023-05-23  7:32 ` [PULL 15/50] net/eth: Rename eth_setup_vlan_headers_ex Jason Wang
2023-05-23  7:32 ` [PULL 16/50] e1000x: Share more Rx filtering logic Jason Wang
2023-05-23  7:32 ` [PULL 17/50] e1000x: Take CRC into consideration for size check Jason Wang
2023-05-23  7:32 ` [PULL 18/50] e1000x: Rename TcpIpv6 into TcpIpv6Ex Jason Wang
2023-05-23  7:32 ` [PULL 19/50] e1000e: Always log status after building rx metadata Jason Wang
2023-05-23  7:32 ` [PULL 20/50] igb: " Jason Wang
2023-05-23  7:32 ` [PULL 21/50] igb: Remove goto Jason Wang
2023-05-23  7:32 ` [PULL 22/50] igb: Read DCMD.VLE of the first Tx descriptor Jason Wang
2023-05-23  7:32 ` [PULL 23/50] e1000e: Reset packet state after emptying Tx queue Jason Wang
2023-05-23  7:32 ` [PULL 24/50] vmxnet3: " Jason Wang
2023-05-23  7:32 ` [PULL 25/50] igb: Add more definitions for Tx descriptor Jason Wang
2023-05-23  7:32 ` [PULL 26/50] igb: Share common VF constants Jason Wang
2023-05-23  7:32 ` [PULL 27/50] igb: Fix igb_mac_reg_init coding style alignment Jason Wang
2023-05-23  7:32 ` [PULL 28/50] igb: Clear EICR bits for delayed MSI-X interrupts Jason Wang
2023-05-23  7:32 ` [PULL 29/50] e1000e: Rename a variable in e1000e_receive_internal() Jason Wang
2023-05-23  7:32 ` [PULL 30/50] igb: Rename a variable in igb_receive_internal() Jason Wang
2023-05-23  7:32 ` [PULL 31/50] net/eth: Use void pointers Jason Wang
2023-05-23  7:32 ` [PULL 32/50] net/eth: Always add VLAN tag Jason Wang
2023-05-23  7:32 ` [PULL 33/50] hw/net/net_rx_pkt: Enforce alignment for eth_header Jason Wang
2023-05-23  7:32 ` [PULL 34/50] tests/qtest/libqos/igb: Set GPIE.Multiple_MSIX Jason Wang
2023-05-23  7:32 ` [PULL 35/50] igb: Implement MSI-X single vector mode Jason Wang
2023-05-23  7:32 ` [PULL 36/50] igb: Use UDP for RSS hash Jason Wang
2023-05-23  7:32 ` [PULL 37/50] igb: Implement Rx SCTP CSO Jason Wang
2023-05-23  7:32 ` [PULL 38/50] igb: Implement Tx " Jason Wang
2023-05-23  7:32 ` [PULL 39/50] igb: Strip the second VLAN tag for extended VLAN Jason Wang
2023-05-23  7:32 ` [PULL 40/50] igb: Filter with " Jason Wang
2023-05-23  7:32 ` [PULL 41/50] igb: Implement igb-specific oversize check Jason Wang
2023-05-23  7:32 ` [PULL 42/50] igb: Implement Rx PTP2 timestamp Jason Wang
2023-05-23  7:32 ` [PULL 43/50] igb: Implement Tx timestamp Jason Wang
2023-05-23  7:32 ` [PULL 44/50] e1000e: Notify only new interrupts Jason Wang
2023-05-23  7:32 ` [PULL 45/50] igb: " Jason Wang
2023-05-23  7:32 ` [PULL 46/50] igb: Clear-on-read ICR when ICR.INTA is set Jason Wang
2023-05-23  7:32 ` [PULL 47/50] vmxnet3: Do not depend on PC Jason Wang
2023-05-23  7:32 ` [PULL 48/50] MAINTAINERS: Add a reviewer for network packet abstractions Jason Wang
2023-05-23  7:32 ` [PULL 49/50] docs/system/devices/igb: Note igb is tested for DPDK Jason Wang
2023-05-23  7:32 ` [PULL 50/50] rtl8139: fix large_send_mss divide-by-zero Jason Wang
2023-05-23 17:56 ` [PULL 00/50] Net patches Richard Henderson
2023-05-23 19:53 ` Michael Tokarev
2023-05-24  4:06   ` Jason Wang
2023-05-24  4:21     ` Akihiko Odaki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230523073238.54236-5-jasowang@redhat.com \
    --to=jasowang@redhat.com \
    --cc=akihiko.odaki@daynix.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=sriram.yagnaraman@est.tech \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).