net/e1000: igb PF never removes VF multicast hashes from shared MTA

https://bugs.dpdk.org/show_bug.cgi?id=2081

Bug ID: 2081
Summary: net/e1000: igb PF never removes VF multicast hashes
from shared MTA
Product: DPDK
Version: 26.11
Hardware: All
OS: All
Status: UNCONFIRMED
Severity: normal
Priority: Normal
Component: ethdev
Assignee: [email protected]
Reporter: [email protected]
Target Milestone: ---

Created attachment 429
--> https://bugs.dpdk.org/attachment.cgi?id=429&action=edit
AI proposed patch

============================================================================
ANT-2026-YE47Z1IV [NOT SECURITY: ordinary bug]
net/e1000: igb PF never removes VF multicast hashes from shared MTA
Location: drivers/net/intel/e1000/igb_pf.c:357 in igb_vf_set_multicast()
Commit: 9a45341f18a1ab47af7123e63d942b90c1b79f38
============================================================================
In the igb PF driver, multicast hash bits that a VF asks for are added to the port's shared multicast filter (MTA) but never removed. Over time that filter only widens, and every pool that accepts hash-matched multicast (all VFs and the PF pool) receives multicast frames nobody subscribed to. The reverse also
happens: when the PF application calls rte_eth_dev_set_mc_addr_list(), the table is rebuilt from the PF's list alone and every VF silently loses the multicast groups it registered.

The cause is in igb_vf_set_multicast() in drivers/net/intel/e1000/igb_pf.c. The VF driver sends its complete list on every update, but the handler ORs the hash values into MTA rather than replacing that VF's previous bits. It also saves the list into vfinfo[0] whichever VF sent it, and that saved list is never read back to rebuild the table. igb_vf_reset_event() only zeroes a counter, so neither an empty list nor a VF reset clears anything. Only a PF-side multicast list update or a port restart does, and both drop the VFs' entries. The attached reproducer runs the unmodified PF code on RAM-backed 82576 registers with two VFs. After VF1 sends 137 lists of up to 30 hash values, all 4096 MTA bits are set, and they stay set after VF1 sends an empty list and resets. A well-behaved VF that replaces a 2-entry list with a 1-entry list likewise leaves 5 bits set instead of 3. There is no crash or memory effect, and the effect on frame delivery is inferred from register meaning rather than observed on hardware. A patch is attached that rebuilds MTA from the PF's shadow list plus each VF's saved list, following the Linux igb driver.

This is an ordinary bug, not a security issue: a VF can already choose any 30 multicast hash bits and change them at
will, and can already make every other pool receive frames by sending broadcast, so a multicast filter that stops
narrowing gives it nothing it did not have.

## Details

igb_vf_set_multicast() handles a VF's E1000_VF_SET_MULTICAST mailbox message by ORing the VF's hash values into
MTA, the one multicast hash filter of the port, which the PF and all VFs share. Nothing takes a VF's bits out again,
so the limit of 30 hash values, which is what fits in one mailbox message (base/e1000_vf.c:386), applies to one
message, not to what a VF has accumulated (drivers/net/intel/e1000/igb_pf.c:335):

```c
        /* only so many hash values supported */
        entries = RTE_MIN(entries, E1000_MAX_VF_MC_ENTRIES);
        …
        vfinfo->num_vf_mc_hashes = (uint16_t)entries;
        …
        for (i = 0; i < vfinfo->num_vf_mc_hashes; i++) {
                vector_reg = (vfinfo->vf_mc_hashes[i] >> 5) & 0x7F;
                vector_bit = vfinfo->vf_mc_hashes[i] & 0x1F;
                mta_reg = E1000_READ_REG_ARRAY(hw, E1000_MTA, vector_reg);
                mta_reg |= (1 << vector_bit);
                E1000_WRITE_REG_ARRAY(hw, E1000_MTA, vector_reg, mta_reg);
        }
```

Running this code unmodified on RAM-backed registers (commit 9a45341f18, default gcc build; see Reproducing): VF1 sends
137 E1000_VF_SET_MULTICAST messages of at most 30 hash values each. MTA has 30 bits set after the first message and
all 4096 after the last. VF1 then sends an empty list and E1000_VF_RESET; 4096 bits stay set.

The message reaches the handler without any per-VF state check. ICR.VMMB makes eth_igb_interrupt_action() call
igb_pf_mbx_process() (igb_ethdev.c:3011), and igb_rcv_msg_from_vf() copies the 16 mailbox words
(igb_pf.c:431), dispatches on msgbuf[0] & 0xFFFF (igb_pf.c:450) and calls igb_vf_set_multicast()
(igb_pf.c:455). The entry count is bits 16-23 of word 0 and the hash values are the 30 16-bit halves of words 1-15,
all written by the VF.

On the VF side the message carries the complete list:
e1000_update_mc_addr_list_vf() (base/e1000_vf.c:376) sends every hash value on each update. The PF handler treats it as "add these". Its comment says the list is saved "for
later use to restore when the PF multi cast list changes" (igb_pf.c:338), but vf_mc_hashes is read nowhere except
in the loop above, and the save itself goes to vfinfo[0] whichever VF sent the message (vfinfo->…; the vf
argument is __rte_unused). So a well-behaved VF leaves bits behind too: in the same setup, VF0 holding 2 hash values
and VF1 replacing a list of 2 with a list of 1 leaves 5 bits set, not 3.

A VF reset does not help. igb_vf_reset_event() only zeroes a counter (igb_pf.c:247):

```c
        /* reset multicast table array for vf */
        vfinfo[vf].num_vf_mc_hashes = 0;
```

MTA bits are cleared in two places only, both driven by the PF application:
e1000_update_mc_addr_list_generic()
(base/e1000_mac.c:583), reached from rte_eth_dev_set_mc_addr_list(), rewrites the table from the PF's own list,
and e1000_init_hw_base() (base/e1000_base.c:72) zeroes it when the port is started. Neither includes the VFs'
saved lists, so the opposite also happens: when the PF application calls rte_eth_dev_set_mc_addr_list(), every VF
silently loses its multicast groups (3 VF groups before, 0 after, in the same RAM-register setup).

A set bit is not private to the VF that set it. A pool accepts frames that match MTA when its VMOLR.ROMPE is set (base/e1000_82575.h:283), and the PF sets that for every VF at reset (igb_pf.c:241) and for its own pool whenever
the port is not in promiscuous or all-multicast mode (igb_pf.c:217). E1000_VF_SET_PROMISC is not implemented in
igb_pf.c: it has no case in the dispatcher and is answered with a NACK (igb_pf.c:463). The ixgbe PF grants
all-multicast to any VF that asks (ixgbe_pf.c:760).

## Impact

Affected: an 82576 or I350 port whose PF is driven by the DPDK igb PMD with VFs enabled. Default build, no offload or
devarg.

The port's multicast hash filter only grows while VFs are in use. Every time a VF driver changes its multicast list,
the hash bits of the old list stay set, so over time every pool with ROMPE (all VFs and the PF pool) is handed
multicast frames nobody subscribed to, which costs Rx descriptors and CPU and has to be dropped in software. A VF that
sends many lists (137, in the reproducer) opens the filter completely. The stale bits outlive the VF: an empty list or
a VF reset does not clear them; only the PF application rewriting its multicast list or restarting the port does. The
problem users are more likely to notice is the reverse: a call to rte_eth_dev_set_mc_addr_list() on the PF drops
every VF's multicast groups, and the VFs stop receiving the multicast traffic they asked for.

None of this gives a VF more than it already has. It may pick any 30 hash bits and replace them at will, so it can
already listen to any group it names; the 30 is the mailbox size, not a quota. It can already load every other pool by
sending broadcast, since the PF enables VF-to-VF loopback (igb_pf.c:151) and broadcast accept for every VF pool
(igb_pf.c:241). There is no memory effect, no crash, and no effect on unicast or VLAN filtering. The effect on frame
delivery follows from the register state; it was not observed on hardware.

## Reproducing

There is no 82576 or I350 in the test image, so the reproducer runs the driver's own PF code on a register file in RAM
and reads the registers back. It uses the default gcc build (/src/build) and takes about a second.

Files:

* repro.d/igb_pf_on_ram_bar0.c (setup: the PF). Links igb_pf.c and the e1000 base code unmodified from the
already built /src/build/drivers/librte_net_e1000.a, points hw->hw_addr
at a 128 KB array and brings up an
82576 PF with two VFs through igb_pf_host_init() and
igb_pf_host_configure(), as eth_igb_dev_init() and
eth_igb_start() do. It then reads one step per stdin line. vf<N> <words>
writes the words to VMBMEM(N), sets
the VF's request bit in MBVFICR as the NIC would, calls
igb_pf_mbx_process() (what the ICR.VMMB interrupt
does) and answers with word 0 of the PF's reply. mta answers with the
number of bits set in MTA. rompe lists
the pools that have VMOLR.ROMPE.
* repro.d/start_igb_pf.py (setup). Compiles that program with gcc against the build and starts it.
* repro (VF1's messages, plus the check). Both VFs first send E1000_VF_RESET, as any VF driver does. VF1 then
sends 137 E1000_VF_SET_MULTICAST messages: the entry count in bits 16-23 of
word 0, then the 16-bit hash values two
per word. Each message holds at most 30 values (136 with 30, one with 16),
and together they cover hash values
0 to 4095. VF1 then sends an empty list and E1000_VF_RESET. If the PF does
not ACK a message, the script stops
there.

Expected output on 9a45341f18:

```
CONTROL MTA bits set before any VF list: 0
CONTROL MTA bits set after VF1's first list of 30: 30
OBSERVED MTA bits set after 137 lists from VF1: 4096 of 4096 (correct: 16,
VF1's last list)
OBSERVED MTA bits set after VF1's empty list: 4096 (correct: 0)
OBSERVED MTA bits set after VF1's reset: 4096 (correct: 0)
OBSERVED pools that accept multicast frames matching the MTA (VMOLR.ROMPE): VF0
VF1 PF
BUG
```

The correct value is the PF's own bits (none) plus the list VF1 currently holds. BUG is printed when VF1's lists
have set all 4096 bits, the bits are still set after the empty list and after the reset, and VF0 and the PF pool have
ROMPE (so they receive frames that match those bits). With the attached patch applied and igb_pf.c linked in place
of the archive member, the same run shows 30, 16, 0 and 0 bits and prints OK.

## Suggested fix

The attached patch (drivers/net/intel/e1000/igb_pf.c) treats MTA as derived state. It adds a static helper,
igb_pf_restore_mta(), that rewrites the whole table from hw->mac.mta_shadow (the PF's own list, maintained by e1000_update_mc_addr_list_generic()) ORed with the saved list of every VF, the way igb_restore_vf_multicasts() does
in the Linux igb driver. igb_vf_set_multicast() now stores the list in vfinfo[vf] instead of vfinfo[0] (the vf
argument loses __rte_unused), drops its OR loop and calls the helper, so a new list replaces the VF's old one.
igb_vf_reset_event() calls the helper after zeroing the VF's count. With it the reproducer shows 30, 16, 0 and 0 bits
and VF0's bits survive a reset of VF1; it was not run on hardware. The table is now rewritten (128 register writes) on
each VF multicast message and VF reset, which is control path only.

The patch is a bare diff: it needs a commit message with Fixes: be2d648a2dd3 ("igb: add PF support") and
Cc: [email protected], and the helper keeps the original 1 << vector_bit, which should be 1U << for bit 31.

Two things the patch leaves alone. eth_igb_set_mc_addr_list() (igb_ethdev.c:4831) and port start still rewrite
MTA without the VFs' entries; they should end with the same helper, which would fix the PF-side loss of VF groups
described above. And ixgbe_pf.c:507, ngbe_pf.c:403 and txgbe_pf.c:491 contain the same store-to-vfinfo[0]-and-OR
loop; those belong in separate per-driver patches.

You are receiving this mail because:
You are the assignee for the bug.