Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 28 May 2021 05:38:19 GMT
From:      Kevin Bowling <kbowling@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org
Subject:   git: 938b01476a3c - stable/13 - e1000: Correct promisc multicast filter handling
Message-ID:  <202105280538.14S5cJDG088675@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch stable/13 has been updated by kbowling (ports committer):

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(-)

diff --git a/sys/dev/e1000/if_em.c b/sys/dev/e1000/if_em.c
index 35cedc6dd5c7..2e9ac31589cd 100644
--- a/sys/dev/e1000/if_em.c
+++ b/sys/dev/e1000/if_em.c
@@ -317,7 +317,6 @@ static int	em_enable_phy_wakeup(struct adapter *);
 static void	em_disable_aspm(struct adapter *);
 
 int		em_intr(void *arg);
-static void	em_disable_promisc(if_ctx_t ctx);
 
 /* MSI-X handlers */
 static int	em_if_msix_intr_assign(if_ctx_t, int);
@@ -1658,11 +1657,20 @@ static int
 em_if_set_promisc(if_ctx_t ctx, int flags)
 {
 	struct adapter *adapter = iflib_get_softc(ctx);
+	struct ifnet *ifp = iflib_get_ifp(ctx);
 	u32 reg_rctl;
-
-	em_disable_promisc(ctx);
+	int mcnt = 0;
 
 	reg_rctl = E1000_READ_REG(&adapter->hw, E1000_RCTL);
+	reg_rctl &= ~(E1000_RCTL_SBP | E1000_RCTL_UPE);
+	if (flags & IFF_ALLMULTI)
+		mcnt = MAX_NUM_MULTICAST_ADDRESSES;
+	else
+		mcnt = min(if_llmaddr_count(ifp), MAX_NUM_MULTICAST_ADDRESSES);
+
+	if (mcnt < MAX_NUM_MULTICAST_ADDRESSES)
+		reg_rctl &= (~E1000_RCTL_MPE);
+	E1000_WRITE_REG(&adapter->hw, E1000_RCTL, reg_rctl);
 
 	if (flags & IFF_PROMISC) {
 		reg_rctl |= (E1000_RCTL_UPE | E1000_RCTL_MPE);
@@ -1678,37 +1686,15 @@ em_if_set_promisc(if_ctx_t ctx, int flags)
 	return (0);
 }
 
-static void
-em_disable_promisc(if_ctx_t ctx)
-{
-	struct adapter *adapter = iflib_get_softc(ctx);
-	struct ifnet *ifp = iflib_get_ifp(ctx);
-	u32 reg_rctl;
-	int mcnt = 0;
-
-	reg_rctl = E1000_READ_REG(&adapter->hw, E1000_RCTL);
-	reg_rctl &= (~E1000_RCTL_UPE);
-	if (if_getflags(ifp) & IFF_ALLMULTI)
-		mcnt = MAX_NUM_MULTICAST_ADDRESSES;
-	else
-		mcnt = if_llmaddr_count(ifp);
-	/* Don't disable if in MAX groups */
-	if (mcnt < MAX_NUM_MULTICAST_ADDRESSES)
-		reg_rctl &=  (~E1000_RCTL_MPE);
-	reg_rctl &=  (~E1000_RCTL_SBP);
-	E1000_WRITE_REG(&adapter->hw, E1000_RCTL, reg_rctl);
-}
-
-
 static u_int
-em_copy_maddr(void *arg, struct sockaddr_dl *sdl, u_int cnt)
+em_copy_maddr(void *arg, struct sockaddr_dl *sdl, u_int idx)
 {
 	u8 *mta = arg;
 
-	if (cnt == MAX_NUM_MULTICAST_ADDRESSES)
-		return (1);
+	if (idx == MAX_NUM_MULTICAST_ADDRESSES)
+		return (0);
 
-	bcopy(LLADDR(sdl), &mta[cnt * ETHER_ADDR_LEN], ETHER_ADDR_LEN);
+	bcopy(LLADDR(sdl), &mta[idx * ETHER_ADDR_LEN], ETHER_ADDR_LEN);
 
 	return (1);
 }
@@ -1719,14 +1705,13 @@ em_copy_maddr(void *arg, struct sockaddr_dl *sdl, u_int cnt)
  *  This routine is called whenever multicast address list is updated.
  *
  **********************************************************************/
-
 static void
 em_if_multi_set(if_ctx_t ctx)
 {
 	struct adapter *adapter = iflib_get_softc(ctx);
 	struct ifnet *ifp = iflib_get_ifp(ctx);
-	u32 reg_rctl = 0;
 	u8  *mta; /* Multicast array memory */
+	u32 reg_rctl = 0;
 	int mcnt = 0;
 
 	IOCTL_DEBUGOUT("em_set_multi: begin");
@@ -1746,11 +1731,20 @@ em_if_multi_set(if_ctx_t ctx)
 
 	mcnt = if_foreach_llmaddr(ifp, em_copy_maddr, mta);
 
-	if (mcnt >= MAX_NUM_MULTICAST_ADDRESSES) {
-		reg_rctl = E1000_READ_REG(&adapter->hw, E1000_RCTL);
+	reg_rctl = E1000_READ_REG(&adapter->hw, E1000_RCTL);
+
+	if (if_getflags(ifp) & IFF_PROMISC)
+		reg_rctl |= (E1000_RCTL_UPE | E1000_RCTL_MPE);
+	else if (mcnt >= MAX_NUM_MULTICAST_ADDRESSES ||
+	    if_getflags(ifp) & IFF_ALLMULTI) {
 		reg_rctl |= E1000_RCTL_MPE;
-		E1000_WRITE_REG(&adapter->hw, E1000_RCTL, reg_rctl);
+		reg_rctl &= ~E1000_RCTL_UPE;
 	} else
+		reg_rctl &= ~(E1000_RCTL_UPE | E1000_RCTL_MPE);
+
+	E1000_WRITE_REG(&adapter->hw, E1000_RCTL, reg_rctl);
+
+	if (mcnt < MAX_NUM_MULTICAST_ADDRESSES)
 		e1000_update_mc_addr_list(&adapter->hw, mta, mcnt);
 
 	if (adapter->hw.mac.type == e1000_82542 &&



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202105280538.14S5cJDG088675>