The e1000 driver contains two related bugs in the way it handles multicast promiscuous mode for an interface when there are MAX_NUM_MULTICAST_ADDRESSES (currently, 128) or more multicast addresses in use on an interface. Currently, when there are less than MAX_NUM_MULTICAST_ADDRESSES (currently, 128) multicast addresses in use on an interface, the code downloads the list to the card. When there are MAX_NUM_MULTICAST_ADDRESSES or more multicast addresses in use on an interface, the code instead just sets the card to be in multicast promiscuous mode. The current code (from em_set_multi())is here: if (mcnt >= MAX_NUM_MULTICAST_ADDRESSES) { reg_rctl = E1000_READ_REG(&adapter->hw, E1000_RCTL); reg_rctl |= E1000_RCTL_MPE; E1000_WRITE_REG(&adapter->hw, E1000_RCTL, reg_rctl); } else e1000_update_mc_addr_list(&adapter->hw, mta, mcnt); When there are MAX_NUM_MULTICAST_ADDRESSES or more multicast addresses in use on an interface, the code does not update the multicast address list on the card. Therefore, if multicast promiscuous mode becomes disabled, the card will not have a current and complete list of all multicast addresses. There are two bugs related to the handling of multicast promiscuous mode when there are MAX_NUM_MULTICAST_ADDRESSES (currently, 128) or more multicast addresses in use on an interface: 1) When there are MAX_NUM_MULTICAST_ADDRESSES or more multicast addresses in use on an interface, the code enables multicast promiscuous mode. However, when the number of multicast addresses in use on an interface drops below MAX_NUM_MULTICAST_ADDRESSES, the code does not disable multicast promiscuous mode. This may result in the system unnecessarily receiving extra multicast packets to process. 2) When the number of multicast addresses reaches MAX_NUM_MULTICAST_ADDRESSES, the code enables multicast promiscuous mode. However, when the IFF_ALLMULTI or IFF_PROMISC flag are removed on an interface, the driver will disable multicast promiscuous mode without regard to whether multicast promiscuous mode is still needed for multicast support. This bug could be triggered, for example, if someone ran tcpdump in promiscuous mode (the defaut mode for tcpdump) and then stopped it while the number of multicast groups joined on an interface was greater than or equal to MAX_NUM_MULTICAST_ADDRESSES. In this case, the card may not receive traffic for all joined multicast addresses. I have verified that these bugs are present in the CVS HEAD; however, note that these bugs are present at least as far back as 6.3. Fix: I have written a patch that addresses these bugs in these ways: 1) When the number of multicast addresses in use on an interface is below MAX_NUM_MULTICAST_ADDRESSES, the code checks the E1000_RCTL to see if multicast promiscuous mode is enabled. If it is enabled, it checks to see if the IFF_ALLMULTI or IFF_PROMISC flags are set in the if_flags. If neither is set, it disables multicast promiscuous mode. In order to avoid brief outages, the code only disables multicast promiscuous mode after it has already written the new multicast addresses to the card. 2) When the IFF_ALLMULTI or IFF_PROMISC if_flags change, the code currently calls em_disable_promisc() and then em_set_promisc(). These two functions will reset the unicast promiscuous, multicast promiscuous, and save bad packet modes as appropriate for the if_flags. However, in order to ensure that changes to these flags do not inadvertently disable multicast promiscuous mode when multicast promiscuous mode is needed due to the number of multicast addresses joined on an interface, I added a call to em_set_multi() after em_set_promisc(). This will allow em_set_multi() to re-enable multicast promiscuous mode, if needed. I have compiled and tested this patch on CURRENT (CVS head, updated today). I have also applied this same patch to 8.0-RC3 (CVS RELENG_8_0 tag, updated today) and compiled and tested on that release. I also have patches that I have compiled and tested for older releases, were that to be necessary. Patch attached with submission follows: How-To-Repeat: In order to demonstrate these bugs, I have written four files for use with mtest: mtest-cmds-step1: This joins 99 groups (225.0.0.x, where x is a number between 1 and 99, inclusive). mtest-cmds-step2: This joins 30 groups (225.0.0.x, where x is a number between 100 and 129, inclusive). mtest-cmds-step3: This leaves 30 groups (225.0.0.x, where x is a number between 100 and 129, inclusive). mtest-cmds-step4: This joins 50 groups (225.0.0.x, where x is a number between 100 and 149, inclusive). To use these files to demonstrate this bug, you need the following: a) The device under test: a FreeBSD host with interface em0. (You will probably need two windows open to this host. You can use an interface other than em0 if you edit the mtest files and change the interface in the commands in these instructions.) b) A second host to generate multicast traffic. This host should be on the same network as the device under test's em0 interface. Procedure to demonstrate these bugs: 1) On the host generating the traffic, ping 225.0.0.145. 2) On the device under test, start tcpdump in non-promiscuous mode (for example, 'tcpdump -i em0 -p icmp'). You should not see the multicast traffic yet. 3) On a different window on the device under test, start mtest. 4) Type 'f mtest-cmds-step1' to join the 99 groups. Unless you already have joined 29 other groups on this interface, this should not cause the number of multicast addresses to reach MAX_NUM_MULTICAST_ADDRESSES. Therefore, the tcpdump should not receive the traffic destined to 225.0.0.145 (as that was not one of the addresses joined). 5) Type 'f mtest-cmds-step2' to join 30 additional groups. This should cause the number of multicast addresses to reach MAX_NUM_MULTICAST_ADDRESSES. Therefore, the driver should enable multicast promiscuous mode for the interface and tcpdump should now begin to receive traffic destined to 225.0.0.145 (even though that was not one of the addresses that was joined). 6) Type 'f mtest-cmds-step3' to leave the 30 groups joined in the last step. This should cause the number of multicast addresses to fall below MAX_NUM_MULTICAST_ADDRESSES. That *should* mean that the card should leave multicast promiscuous mode; however, because of the first bug listed above, the card will remain in multicast promiscuous mode. You can verify this by observing that tcpdump is still receiving traffic destined to 225.0.0.145 (even though that is not one of the addresses that is currently joined). 7) Type 'f mtest-cmds-step4' to join 50 additional groups. This should cause the number of multicast addresses to reach MAX_NUM_MULTICAST_ADDRESSES. Therefore, the driver should put the interface in multicast promiscuous mode and tcpdump should receive traffic destined to 225.0.0.145 (and, that is one of the addresses that was joined in this step). 8) Type 'p em0 1' to cause the interface to enter promiscuous mode. Then, type 'p em0 0' to cause the interface to leave promiscuous mode. That *should not* prevent the card from receiving traffic destined for multicast addresses that are joined on the interface; however, because of the second bug listed above, the interface will leave multicast promiscuous mode and the interface will only receive traffic for the multicast addresses last programmed into the card. You can verify this by observing that tcpdump stops receiving traffic destined to 225.0.0.145, even though that multicast address was joined in step 7 (and is still joined).
Responsible Changed From-To: freebsd-bugs->freebsd-net Over to maintainer(s).
Responsible Changed From-To: freebsd-net->jfv Over to maintainer.
This bug is still not fixed in em/igb/lem driver in HEAD. And it looks that similar bug is in the ixgbe driver, ixgbe_set_multi() doesn't even check count of multicast addresses and it will overflow static mta array causing kernel panic. It should enable IXGBE_FCTRL_MPE if mcnt > 128 like em driver! Sincerely, Petr Lampa -- Computer Centre E-mail: lampa@fit.vutbr.cz Faculty of Information Technology Web: http://www.fit.vutbr.cz/ Brno University of Technology Fax: +420 54114-1270 Bozetechova 2, 612 66 Brno, Czech Republic Phone: +420 54114-1225
head still does not have this code in it. Should be reviewed for inclusion.
Because this problem is being propagated to all Intel drivers, this really should be addressed.
I'll take this for review/test.
batch change: For bugs that match the following - Status Is In progress AND - Untouched since 2018-01-01. AND - Affects Base System OR Documentation DO: Reset to open status. Note: I did a quick pass but if you are getting this email it might be worthwhile to double check to see if this bug ought to be closed.
https://reviews.freebsd.org/D29789
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=4b38eed76da9c36f09bff33b5cf15687cd99016f commit 4b38eed76da9c36f09bff33b5cf15687cd99016f Author: Kevin Bowling <kbowling@FreeBSD.org> AuthorDate: 2021-04-16 23:15:50 +0000 Commit: Kevin Bowling <kbowling@FreeBSD.org> CommitDate: 2021-04-19 19:49:55 +0000 e1000: Correct promisc multicast filter handling There are a number of issues in the e1000 multicast filter handling that have been present for a long time. Take the updated approach from ixgbe(4) which does not have the issues. The issues are outlined in the PR, in particular this solves crossing over and under the hardware's filter limit, not programming the hardware filter when we are above its limit, disabling SBP (show bad packets) when the tunable is enabled and exiting promiscuous mode, and an off-by-one error in the em_copy_maddr function. PR: 140647 Reported by: jtl Reviewed by: markj MFC after: 1 month Differential Revision: https://reviews.freebsd.org/D29789 sys/dev/e1000/if_em.c | 62 +++++++++++++++++++++++---------------------------- 1 file changed, 28 insertions(+), 34 deletions(-)
A commit in branch stable/13 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=938b01476a3c0a3c75a0fd263238729b6a38360a commit 938b01476a3c0a3c75a0fd263238729b6a38360a Author: Kevin Bowling <kbowling@FreeBSD.org> AuthorDate: 2021-04-16 23:15:50 +0000 Commit: Kevin Bowling <kbowling@FreeBSD.org> CommitDate: 2021-05-28 05:30:03 +0000 e1000: Correct promisc multicast filter handling There are a number of issues in the e1000 multicast filter handling that have been present for a long time. Take the updated approach from ixgbe(4) which does not have the issues. The issues are outlined in the PR, in particular this solves crossing over and under the hardware's filter limit, not programming the hardware filter when we are above its limit, disabling SBP (show bad packets) when the tunable is enabled and exiting promiscuous mode, and an off-by-one error in the em_copy_maddr function. PR: 140647 Reported by: jtl Reviewed by: markj MFC after: 1 month Differential Revision: https://reviews.freebsd.org/D29789 (cherry picked from commit 4b38eed76da9c36f09bff33b5cf15687cd99016f) sys/dev/e1000/if_em.c | 62 +++++++++++++++++++++++---------------------------- 1 file changed, 28 insertions(+), 34 deletions(-)
A commit in branch stable/12 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=ab177386aa8790c66826be227a1d9e6b482444bb commit ab177386aa8790c66826be227a1d9e6b482444bb Author: Kevin Bowling <kbowling@FreeBSD.org> AuthorDate: 2021-04-16 23:15:50 +0000 Commit: Kevin Bowling <kbowling@FreeBSD.org> CommitDate: 2021-05-28 06:10:48 +0000 e1000: Correct promisc multicast filter handling There are a number of issues in the e1000 multicast filter handling that have been present for a long time. Take the updated approach from ixgbe(4) which does not have the issues. The issues are outlined in the PR, in particular this solves crossing over and under the hardware's filter limit, not programming the hardware filter when we are above its limit, disabling SBP (show bad packets) when the tunable is enabled and exiting promiscuous mode, and an off-by-one error in the em_copy_maddr function. PR: 140647 Reported by: jtl Reviewed by: markj MFC after: 1 month Differential Revision: https://reviews.freebsd.org/D29789 (cherry picked from commit 4b38eed76da9c36f09bff33b5cf15687cd99016f) sys/dev/e1000/if_em.c | 54 +++++++++++++++++++++++---------------------------- 1 file changed, 24 insertions(+), 30 deletions(-)
No MFC to stable11 due to diverged driver but it shouldn't be hard if someone wants to.