git: f7926a6d0c10 - main - net: iflib: fix vlan processing in the drivers

From: Vincenzo Maffione <vmaffione_at_FreeBSD.org>
Date: Tue, 28 Dec 2021 11:12:35 UTC
The branch main has been updated by vmaffione:

URL: https://cgit.FreeBSD.org/src/commit/?id=f7926a6d0c1029c8da265769e7c57b4065faa2df

commit f7926a6d0c1029c8da265769e7c57b4065faa2df
Author:     Vincenzo Maffione <vmaffione@FreeBSD.org>
AuthorDate: 2021-12-28 11:05:31 +0000
Commit:     Vincenzo Maffione <vmaffione@FreeBSD.org>
CommitDate: 2021-12-28 11:11:41 +0000

    net: iflib: fix vlan processing in the drivers
    
    The logic that sets iri_vtag and M_VLANTAG does not handle the
    case where the 802.11q VLAN tag is 0. Fix this issue across
    the iflib drivers. While there, also improve and align the
    VLAN tag check extraction, by moving it outside the RX descriptor
    loop, eliminating a local variable and additional checks.
    
    PR:             260068
    Reviewed by:    kbowling, gallatin
    Reported by:    erj
    MFC after:      1 month
    Differential Revision:  https://reviews.freebsd.org/D33156
---
 sys/dev/e1000/em_txrx.c        | 11 ++++-------
 sys/dev/e1000/igb_txrx.c       | 21 +++++++++------------
 sys/dev/iavf/iavf_txrx_iflib.c | 13 +++++--------
 sys/dev/ice/ice_iflib_txrx.c   | 13 +++++--------
 sys/dev/igc/igc_txrx.c         |  8 +++-----
 sys/dev/ixgbe/ix_txrx.c        | 15 +++++----------
 sys/dev/ixl/ixl_txrx.c         | 13 +++++--------
 7 files changed, 36 insertions(+), 58 deletions(-)

diff --git a/sys/dev/e1000/em_txrx.c b/sys/dev/e1000/em_txrx.c
index 58f9345ea19d..aebafca73cca 100644
--- a/sys/dev/e1000/em_txrx.c
+++ b/sys/dev/e1000/em_txrx.c
@@ -671,9 +671,9 @@ em_isc_rxd_pkt_get(void *arg, if_rxd_info_t ri)
 	u32 pkt_info;
 	u32 staterr = 0;
 	bool eop;
-	int i, cidx, vtag;
+	int i, cidx;
 
-	i = vtag = 0;
+	i = 0;
 	cidx = ri->iri_cidx;
 
 	do {
@@ -710,12 +710,9 @@ em_isc_rxd_pkt_get(void *arg, if_rxd_info_t ri)
 		em_receive_checksum(staterr, staterr >> 24, ri);
 
 	if (staterr & E1000_RXD_STAT_VP) {
-		vtag = le16toh(rxd->wb.upper.vlan);
-	}
-
-	ri->iri_vtag = vtag;
-	if (vtag)
+		ri->iri_vtag = le16toh(rxd->wb.upper.vlan);
 		ri->iri_flags |= M_VLANTAG;
+	}
 
 	ri->iri_flowid = le32toh(rxd->wb.lower.hi_dword.rss);
 	ri->iri_rsstype = em_determine_rsstype(pkt_info);
diff --git a/sys/dev/e1000/igb_txrx.c b/sys/dev/e1000/igb_txrx.c
index 548ceee8ab46..1c1500e78d19 100644
--- a/sys/dev/e1000/igb_txrx.c
+++ b/sys/dev/e1000/igb_txrx.c
@@ -436,12 +436,12 @@ igb_isc_rxd_pkt_get(void *arg, if_rxd_info_t ri)
 	struct rx_ring *rxr = &que->rxr;
 	union e1000_adv_rx_desc *rxd;
 
-	uint16_t pkt_info, len, vtag;
+	uint16_t pkt_info, len;
 	uint32_t ptype, staterr;
 	int i, cidx;
 	bool eop;
 
-	staterr = i = vtag = 0;
+	staterr = i = 0;
 	cidx = ri->iri_cidx;
 
 	do {
@@ -460,13 +460,6 @@ igb_isc_rxd_pkt_get(void *arg, if_rxd_info_t ri)
 		rxd->wb.upper.status_error = 0;
 		eop = ((staterr & E1000_RXD_STAT_EOP) == E1000_RXD_STAT_EOP);
 
-		if (((sc->hw.mac.type == e1000_i350) ||
-		    (sc->hw.mac.type == e1000_i354)) &&
-		    (staterr & E1000_RXDEXT_STATERR_LB))
-			vtag = be16toh(rxd->wb.upper.vlan);
-		else
-			vtag = le16toh(rxd->wb.upper.vlan);
-
 		/* Make sure bad packets are discarded */
 		if (eop && ((staterr & E1000_RXDEXT_ERR_FRAME_ERR_MASK) != 0)) {
 			sc->dropped_pkts++;
@@ -495,9 +488,13 @@ igb_isc_rxd_pkt_get(void *arg, if_rxd_info_t ri)
 	if ((scctx->isc_capenable & IFCAP_RXCSUM) != 0)
 		igb_rx_checksum(staterr, ri, ptype);
 
-	if ((scctx->isc_capenable & IFCAP_VLAN_HWTAGGING) != 0 &&
-	    (staterr & E1000_RXD_STAT_VP) != 0) {
-		ri->iri_vtag = vtag;
+	if (staterr & E1000_RXD_STAT_VP) {
+		if (((sc->hw.mac.type == e1000_i350) ||
+		    (sc->hw.mac.type == e1000_i354)) &&
+		    (staterr & E1000_RXDEXT_STATERR_LB))
+			ri->iri_vtag = be16toh(rxd->wb.upper.vlan);
+		else
+			ri->iri_vtag = le16toh(rxd->wb.upper.vlan);
 		ri->iri_flags |= M_VLANTAG;
 	}
 
diff --git a/sys/dev/iavf/iavf_txrx_iflib.c b/sys/dev/iavf/iavf_txrx_iflib.c
index f536f7f23ff5..5f24b9c18452 100644
--- a/sys/dev/iavf/iavf_txrx_iflib.c
+++ b/sys/dev/iavf/iavf_txrx_iflib.c
@@ -670,7 +670,7 @@ iavf_isc_rxd_pkt_get(void *arg, if_rxd_info_t ri)
 	struct rx_ring		*rxr = &que->rxr;
 	union iavf_rx_desc	*cur;
 	u32		status, error;
-	u16		plen, vtag;
+	u16		plen;
 	u64		qword;
 	u8		ptype;
 	bool		eop;
@@ -701,10 +701,6 @@ iavf_isc_rxd_pkt_get(void *arg, if_rxd_info_t ri)
 
 		cur->wb.qword1.status_error_len = 0;
 		eop = (status & (1 << IAVF_RX_DESC_STATUS_EOF_SHIFT));
-		if (status & (1 << IAVF_RX_DESC_STATUS_L2TAG1P_SHIFT))
-			vtag = le16toh(cur->wb.qword0.lo_dword.l2tag1);
-		else
-			vtag = 0;
 
 		/*
 		** Make sure bad packets are discarded,
@@ -731,10 +727,11 @@ iavf_isc_rxd_pkt_get(void *arg, if_rxd_info_t ri)
 		iavf_rx_checksum(ri, status, error, ptype);
 	ri->iri_flowid = le32toh(cur->wb.qword0.hi_dword.rss);
 	ri->iri_rsstype = iavf_ptype_to_hash(ptype);
-	ri->iri_vtag = vtag;
-	ri->iri_nfrags = i;
-	if (vtag)
+	if (status & (1 << IAVF_RX_DESC_STATUS_L2TAG1P_SHIFT)) {
+		ri->iri_vtag = le16toh(cur->wb.qword0.lo_dword.l2tag1);
 		ri->iri_flags |= M_VLANTAG;
+	}
+	ri->iri_nfrags = i;
 	return (0);
 }
 
diff --git a/sys/dev/ice/ice_iflib_txrx.c b/sys/dev/ice/ice_iflib_txrx.c
index 7d89c51ddec0..0ce59d96c0ed 100644
--- a/sys/dev/ice/ice_iflib_txrx.c
+++ b/sys/dev/ice/ice_iflib_txrx.c
@@ -285,7 +285,7 @@ ice_ift_rxd_pkt_get(void *arg, if_rxd_info_t ri)
 	if_softc_ctx_t scctx = sc->scctx;
 	struct ice_rx_queue *rxq = &sc->pf_vsi.rx_queues[ri->iri_qsidx];
 	union ice_32b_rx_flex_desc *cur;
-	u16 status0, plen, vtag, ptype;
+	u16 status0, plen, ptype;
 	bool eop;
 	size_t cidx;
 	int i;
@@ -310,10 +310,6 @@ ice_ift_rxd_pkt_get(void *arg, if_rxd_info_t ri)
 
 		cur->wb.status_error0 = 0;
 		eop = (status0 & BIT(ICE_RX_FLEX_DESC_STATUS0_EOF_S));
-		if (status0 & BIT(ICE_RX_FLEX_DESC_STATUS0_L2TAG1P_S))
-			vtag = le16toh(cur->wb.l2tag1);
-		else
-			vtag = 0;
 
 		/*
 		 * Make sure packets with bad L2 values are discarded.
@@ -340,10 +336,11 @@ ice_ift_rxd_pkt_get(void *arg, if_rxd_info_t ri)
 				&ri->iri_csum_data, status0, ptype);
 	ri->iri_flowid = le32toh(RX_FLEX_NIC(&cur->wb, rss_hash));
 	ri->iri_rsstype = ice_ptype_to_hash(ptype);
-	ri->iri_vtag = vtag;
-	ri->iri_nfrags = i;
-	if (vtag)
+	if (status0 & BIT(ICE_RX_FLEX_DESC_STATUS0_L2TAG1P_S)) {
+		ri->iri_vtag = le16toh(cur->wb.l2tag1);
 		ri->iri_flags |= M_VLANTAG;
+	}
+	ri->iri_nfrags = i;
 	return (0);
 }
 
diff --git a/sys/dev/igc/igc_txrx.c b/sys/dev/igc/igc_txrx.c
index ee4d5a5c0c6b..4f8234c855f6 100644
--- a/sys/dev/igc/igc_txrx.c
+++ b/sys/dev/igc/igc_txrx.c
@@ -461,12 +461,12 @@ igc_isc_rxd_pkt_get(void *arg, if_rxd_info_t ri)
 	struct rx_ring *rxr = &que->rxr;
 	union igc_adv_rx_desc *rxd;
 
-	uint16_t pkt_info, len, vtag;
+	uint16_t pkt_info, len;
 	uint32_t ptype, staterr;
 	int i, cidx;
 	bool eop;
 
-	staterr = i = vtag = 0;
+	staterr = i = 0;
 	cidx = ri->iri_cidx;
 
 	do {
@@ -485,8 +485,6 @@ igc_isc_rxd_pkt_get(void *arg, if_rxd_info_t ri)
 		rxd->wb.upper.status_error = 0;
 		eop = ((staterr & IGC_RXD_STAT_EOP) == IGC_RXD_STAT_EOP);
 
-		vtag = le16toh(rxd->wb.upper.vlan);
-
 		/* Make sure bad packets are discarded */
 		if (eop && ((staterr & IGC_RXDEXT_STATERR_RXE) != 0)) {
 			adapter->dropped_pkts++;
@@ -517,7 +515,7 @@ igc_isc_rxd_pkt_get(void *arg, if_rxd_info_t ri)
 
 	if ((scctx->isc_capenable & IFCAP_VLAN_HWTAGGING) != 0 &&
 	    (staterr & IGC_RXD_STAT_VP) != 0) {
-		ri->iri_vtag = vtag;
+		ri->iri_vtag = le16toh(rxd->wb.upper.vlan);
 		ri->iri_flags |= M_VLANTAG;
 	}
 
diff --git a/sys/dev/ixgbe/ix_txrx.c b/sys/dev/ixgbe/ix_txrx.c
index 7c87b0ec10fc..589f4c72c9ce 100644
--- a/sys/dev/ixgbe/ix_txrx.c
+++ b/sys/dev/ixgbe/ix_txrx.c
@@ -399,7 +399,6 @@ ixgbe_isc_rxd_pkt_get(void *arg, if_rxd_info_t ri)
 	union ixgbe_adv_rx_desc  *rxd;
 
 	uint16_t                  pkt_info, len, cidx, i;
-	uint16_t                  vtag = 0;
 	uint32_t                  ptype;
 	uint32_t                  staterr = 0;
 	bool                      eop;
@@ -424,12 +423,6 @@ ixgbe_isc_rxd_pkt_get(void *arg, if_rxd_info_t ri)
 		rxd->wb.upper.status_error = 0;
 		eop = ((staterr & IXGBE_RXD_STAT_EOP) != 0);
 
-		if ( (rxr->vtag_strip) && (staterr & IXGBE_RXD_STAT_VP) ) {
-			vtag = le16toh(rxd->wb.upper.vlan);
-		} else {
-			vtag = 0;
-		}
-
 		/* Make sure bad packets are discarded */
 		if (eop && (staterr & IXGBE_RXDADV_ERR_FRAME_ERR_MASK) != 0) {
 			if (sc->feat_en & IXGBE_FEATURE_VF)
@@ -463,10 +456,12 @@ ixgbe_isc_rxd_pkt_get(void *arg, if_rxd_info_t ri)
 		else
 			ri->iri_rsstype = M_HASHTYPE_OPAQUE_HASH;
 	}
-	ri->iri_vtag = vtag;
-	ri->iri_nfrags = i;
-	if (vtag)
+	if ((rxr->vtag_strip) && (staterr & IXGBE_RXD_STAT_VP)) {
+		ri->iri_vtag = le16toh(rxd->wb.upper.vlan);
 		ri->iri_flags |= M_VLANTAG;
+	}
+
+	ri->iri_nfrags = i;
 	return (0);
 } /* ixgbe_isc_rxd_pkt_get */
 
diff --git a/sys/dev/ixl/ixl_txrx.c b/sys/dev/ixl/ixl_txrx.c
index 58ae751e5e10..20050e73c09b 100644
--- a/sys/dev/ixl/ixl_txrx.c
+++ b/sys/dev/ixl/ixl_txrx.c
@@ -663,7 +663,7 @@ ixl_isc_rxd_pkt_get(void *arg, if_rxd_info_t ri)
 	struct rx_ring		*rxr = &que->rxr;
 	union i40e_rx_desc	*cur;
 	u32		status, error;
-	u16		plen, vtag;
+	u16		plen;
 	u64		qword;
 	u8		ptype;
 	bool		eop;
@@ -694,10 +694,6 @@ ixl_isc_rxd_pkt_get(void *arg, if_rxd_info_t ri)
 
 		cur->wb.qword1.status_error_len = 0;
 		eop = (status & (1 << I40E_RX_DESC_STATUS_EOF_SHIFT));
-		if (status & (1 << I40E_RX_DESC_STATUS_L2TAG1P_SHIFT))
-			vtag = le16toh(cur->wb.qword0.lo_dword.l2tag1);
-		else
-			vtag = 0;
 
 		/*
 		** Make sure bad packets are discarded,
@@ -724,10 +720,11 @@ ixl_isc_rxd_pkt_get(void *arg, if_rxd_info_t ri)
 		rxr->csum_errs += ixl_rx_checksum(ri, status, error, ptype);
 	ri->iri_flowid = le32toh(cur->wb.qword0.hi_dword.rss);
 	ri->iri_rsstype = ixl_ptype_to_hash(ptype);
-	ri->iri_vtag = vtag;
-	ri->iri_nfrags = i;
-	if (vtag)
+	if (status & (1 << I40E_RX_DESC_STATUS_L2TAG1P_SHIFT)) {
+		ri->iri_vtag = le16toh(cur->wb.qword0.lo_dword.l2tag1);
 		ri->iri_flags |= M_VLANTAG;
+	}
+	ri->iri_nfrags = i;
 	return (0);
 }