git: 6b488550ee57 - releng/13.2 - epair: Avoid loading m_flags into a short

From: Mark Johnston <markj_at_FreeBSD.org>
Date: Wed, 15 Mar 2023 14:49:12 UTC
The branch releng/13.2 has been updated by markj:

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

commit 6b488550ee57dc09b7d564aa22f9feb61778a89f
Author:     Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2023-03-06 14:39:17 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2023-03-15 13:39:34 +0000

    epair: Avoid loading m_flags into a short
    
    The m_flags field of struct mbuf is 24 bits wide and so gets truncated
    in a couple of places in the epair code.  Instead of preserving the
    entire flag set, just remember whether M_BCAST or M_MCAST is set.
    
    Approved by:    re (cperciva)
    MFC after:      1 week
    Sponsored by:   Klara, Inc.
    
    (cherry picked from commit 48227d1c6db8fceaceebbf8578612302d64ca170)
    (cherry picked from commit c3bd32f225ec093ba0f7cd7fc1a000b02aad5211)
---
 sys/net/if_epair.c | 97 +++++++++++++++++++++++++++++++-----------------------
 1 file changed, 55 insertions(+), 42 deletions(-)

diff --git a/sys/net/if_epair.c b/sys/net/if_epair.c
index e7257f5fa551..e2c61f83d940 100644
--- a/sys/net/if_epair.c
+++ b/sys/net/if_epair.c
@@ -188,39 +188,14 @@ epair_tx_start_deferred(void *arg, int pending)
 	if_rele(sc->ifp);
 }
 
-static int
-epair_menq(struct mbuf *m, struct epair_softc *osc)
+static struct epair_queue *
+epair_select_queue(struct epair_softc *sc, struct mbuf *m)
 {
-	struct ifnet *ifp, *oifp;
-	int len, ret;
-	int ridx;
-	short mflags;
-	struct epair_queue *q = NULL;
 	uint32_t bucket;
 #ifdef RSS
 	struct ether_header *eh;
-#endif
-
-	/*
-	 * I know this looks weird. We pass the "other sc" as we need that one
-	 * and can get both ifps from it as well.
-	 */
-	oifp = osc->ifp;
-	ifp = osc->oifp;
-
-	M_ASSERTPKTHDR(m);
-	epair_clear_mbuf(m);
-	if_setrcvif(m, oifp);
-	M_SETFIB(m, oifp->if_fib);
-
-	/* Save values as once the mbuf is queued, it's not ours anymore. */
-	len = m->m_pkthdr.len;
-	mflags = m->m_flags;
-
-	MPASS(m->m_nextpkt == NULL);
-	MPASS((m->m_pkthdr.csum_flags & CSUM_SND_TAG) == 0);
+	int ret;
 
-#ifdef RSS
 	ret = rss_m2bucket(m, &bucket);
 	if (ret) {
 		/* Actually hash the packet. */
@@ -242,11 +217,47 @@ epair_menq(struct mbuf *m, struct epair_softc *osc)
 			break;
 		}
 	}
-	bucket %= osc->num_queues;
+	bucket %= sc->num_queues;
 #else
 	bucket = 0;
 #endif
-	q = &osc->queues[bucket];
+	return (&sc->queues[bucket]);
+}
+
+static void
+epair_prepare_mbuf(struct mbuf *m, struct ifnet *src_ifp)
+{
+	M_ASSERTPKTHDR(m);
+	epair_clear_mbuf(m);
+	if_setrcvif(m, src_ifp);
+	M_SETFIB(m, src_ifp->if_fib);
+
+	MPASS(m->m_nextpkt == NULL);
+	MPASS((m->m_pkthdr.csum_flags & CSUM_SND_TAG) == 0);
+}
+
+static void
+epair_menq(struct mbuf *m, struct epair_softc *osc)
+{
+	struct ifnet *ifp, *oifp;
+	int len, ret;
+	int ridx;
+	bool mcast;
+
+	/*
+	 * I know this looks weird. We pass the "other sc" as we need that one
+	 * and can get both ifps from it as well.
+	 */
+	oifp = osc->ifp;
+	ifp = osc->oifp;
+
+	epair_prepare_mbuf(m, oifp);
+
+	/* Save values as once the mbuf is queued, it's not ours anymore. */
+	len = m->m_pkthdr.len;
+	mcast = (m->m_flags & (M_BCAST | M_MCAST)) != 0;
+
+	struct epair_queue *q = epair_select_queue(osc, m);
 
 	atomic_set_long(&q->state, (1 << BIT_MBUF_QUEUED));
 	ridx = atomic_load_int(&q->ridx);
@@ -255,7 +266,7 @@ epair_menq(struct mbuf *m, struct epair_softc *osc)
 		/* Ring is full. */
 		if_inc_counter(ifp, IFCOUNTER_OQDROPS, 1);
 		m_freem(m);
-		return (0);
+		return;
 	}
 
 	if_inc_counter(ifp, IFCOUNTER_OPACKETS, 1);
@@ -265,15 +276,13 @@ epair_menq(struct mbuf *m, struct epair_softc *osc)
 	 * the logic another time.
 	 */
 	if_inc_counter(ifp, IFCOUNTER_OBYTES, len);
-	if (mflags & (M_BCAST|M_MCAST))
+	if (mcast)
 		if_inc_counter(ifp, IFCOUNTER_OMCASTS, 1);
 	/* Someone else received the packet. */
 	if_inc_counter(oifp, IFCOUNTER_IPACKETS, 1);
 
 	if (!atomic_testandset_long(&q->state, BIT_QUEUE_TASK))
-		taskqueue_enqueue(epair_tasks.tq[bucket], &q->tx_task);
-
-	return (0);
+		taskqueue_enqueue(epair_tasks.tq[q->id], &q->tx_task);
 }
 
 static void
@@ -317,8 +326,10 @@ epair_transmit(struct ifnet *ifp, struct mbuf *m)
 {
 	struct epair_softc *sc;
 	struct ifnet *oifp;
-	int error, len;
-	short mflags;
+#ifdef ALTQ
+	int len;
+	bool mcast;
+#endif
 
 	if (m == NULL)
 		return (0);
@@ -355,10 +366,12 @@ epair_transmit(struct ifnet *ifp, struct mbuf *m)
 		m_freem(m);
 		return (0);
 	}
-	len = m->m_pkthdr.len;
-	mflags = m->m_flags;
 
 #ifdef ALTQ
+	len = m->m_pkthdr.len;
+	mcast = (m->m_flags & (M_BCAST | M_MCAST)) != 0;
+	int error = 0;
+
 	/* Support ALTQ via the classic if_start() path. */
 	IF_LOCK(&ifp->if_snd);
 	if (ALTQ_IS_ENABLED(&ifp->if_snd)) {
@@ -368,7 +381,7 @@ epair_transmit(struct ifnet *ifp, struct mbuf *m)
 		IF_UNLOCK(&ifp->if_snd);
 		if (!error) {
 			if_inc_counter(ifp, IFCOUNTER_OBYTES, len);
-			if (mflags & (M_BCAST|M_MCAST))
+			if (mcast)
 				if_inc_counter(ifp, IFCOUNTER_OMCASTS, 1);
 			epair_start(ifp);
 		}
@@ -377,8 +390,8 @@ epair_transmit(struct ifnet *ifp, struct mbuf *m)
 	IF_UNLOCK(&ifp->if_snd);
 #endif
 
-	error = epair_menq(m, oifp->if_softc);
-	return (error);
+	epair_menq(m, oifp->if_softc);
+	return (0);
 }
 
 static int