Index | Thread | Search

From:
Vitaliy Makkoveev <mvs@openbsd.org>
Subject:
Kill pipexintr()
To:
Alexander Bluhm <bluhm@openbsd.org>, YASUOKA Masahiko <yasuoka@openbsd.org>, tech@openbsd.org
Date:
Tue, 28 Oct 2025 15:53:29 +0300

Download raw body.

Thread
  • Vitaliy Makkoveev:

    Kill pipexintr()

It was used because the netlock state is unpredictable while
pipex_pppoe_output() was called. So the PPPoE output was deferred to
netisr to make sure the locks are held. However, the protocol of our
peer is always AF_UNSPEC, so ether_output() and following ether_resolve()
are just "memcpy(eh, dst->sa_data, sizeof(*eh));" where `dst' is the
immutable data which belongs to pipex session. 

So I propose to manually fill ethernet header and enqueue packet.
Someone could say this the copy-paste from ether_output(), but it is
very small and this time I intentionally want to use it instead of
non-obvious ifp->if_output().

ok?

Index: sys/net/if.c
===================================================================
RCS file: /cvs/src/sys/net/if.c,v
diff -u -p -r1.741 if.c
--- sys/net/if.c	9 Sep 2025 09:16:18 -0000	1.741
+++ sys/net/if.c	28 Oct 2025 12:20:05 -0000
@@ -1103,10 +1103,6 @@ if_netisr(void *unused)
 		if (n & (1 << NETISR_BRIDGE))
 			bridgeintr();
 #endif
-#ifdef PIPEX
-		if (n & (1 << NETISR_PIPEX))
-			pipexintr();
-#endif
 #if NPPPOE > 0
 		if (n & (1 << NETISR_PPPOE)) {
 			KERNEL_LOCK();
Index: sys/net/netisr.h
===================================================================
RCS file: /cvs/src/sys/net/netisr.h,v
diff -u -p -r1.61 netisr.h
--- sys/net/netisr.h	6 Jul 2023 04:55:05 -0000	1.61
+++ sys/net/netisr.h	28 Oct 2025 12:20:05 -0000
@@ -44,7 +44,6 @@
 #define NETISR_IP	2		/* same as AF_INET */
 #define NETISR_ARP	18		/* same as AF_LINK */
 #define NETISR_IPV6	24		/* same as AF_INET6 */
-#define NETISR_PIPEX	27		/* for pipex processing */
 #define NETISR_PPP	28		/* for PPP processing */
 #define NETISR_BRIDGE	29		/* for bridge processing */
 #define NETISR_PPPOE	30		/* for pppoe processing */
@@ -63,7 +62,6 @@ void	ipintr(void);
 void	ip6intr(void);
 void	pppintr(void);
 void	bridgeintr(void);
-void	pipexintr(void);
 void	pppoeintr(void);
 
 #define	schednetisr(anisr)						\
Index: sys/net/pipex.c
===================================================================
RCS file: /cvs/src/sys/net/pipex.c,v
diff -u -p -r1.161 pipex.c
--- sys/net/pipex.c	27 Aug 2025 22:39:04 -0000	1.161
+++ sys/net/pipex.c	28 Oct 2025 12:20:05 -0000
@@ -183,46 +183,6 @@ pipex_ioctl(void *ownersc, u_long cmd, c
 }
 
 /************************************************************************
- * Software Interrupt Handler
- ************************************************************************/
-
-void
-pipexintr(void)
-{
-	struct mbuf_list ml;
-	struct mbuf *m;
-	struct pipex_session *session;
-
-	NET_ASSERT_LOCKED();
-
-	mq_delist(&pipexoutq, &ml);
-
-	while ((m = ml_dequeue(&ml)) != NULL) {
-		struct ifnet *ifp;
-
-		session = m->m_pkthdr.ph_cookie;
-
-		ifp = if_get(session->proto.pppoe.over_ifidx);
-		if (ifp != NULL) {
-			struct pipex_pppoe_header *pppoe;
-			int len;
-
-			pppoe = mtod(m, struct pipex_pppoe_header *);
-			len = ntohs(pppoe->length);
-			ifp->if_output(ifp, m, &session->peer.sa, NULL);
-			counters_pkt(session->stat_counters, pxc_opackets,
-			    pxc_obytes, len);
-		} else {
-			m_freem(m);
-			counters_inc(session->stat_counters, pxc_oerrors);
-		}
-		if_put(ifp);
-
-		pipex_rele_session(session);
-	}
-}
-
-/************************************************************************
  * Session management functions
  ************************************************************************/
 int
@@ -1335,6 +1295,7 @@ pipex_pppoe_input(struct mbuf *m0, struc
 void
 pipex_pppoe_output(struct mbuf *m0, struct pipex_session *session)
 {
+	struct ifnet *ifp;
 	struct pipex_pppoe_header *pppoe;
 	int len, padlen;
 
@@ -1345,7 +1306,7 @@ pipex_pppoe_output(struct mbuf *m0, stru
 	M_PREPEND(m0, sizeof(struct pipex_pppoe_header), M_NOWAIT);
 	if (m0 == NULL) {
 		PIPEX_DBG((NULL, LOG_ERR,
-		    "<%s> cannot prepend header.", __func__));
+		    "<%s> cannot prepend pppoe header.", __func__));
 		counters_inc(session->stat_counters, pxc_oerrors);
 		return;
 	}
@@ -1362,15 +1323,37 @@ pipex_pppoe_output(struct mbuf *m0, stru
 	pppoe->length = htons(len);
 
 	m0->m_pkthdr.ph_ifidx = session->proto.pppoe.over_ifidx;
-	refcnt_take(&session->pxs_refcnt);
-	m0->m_pkthdr.ph_cookie = session;
 	m0->m_flags &= ~(M_BCAST|M_MCAST);
 
-	if (mq_enqueue(&pipexoutq, m0) != 0) {
+	M_PREPEND(m0, ETHER_ALIGN + sizeof(struct ether_header), M_NOWAIT);
+	if (m0 == NULL) {
+		PIPEX_DBG((NULL, LOG_ERR,
+		    "<%s> cannot prepend ethernet header.", __func__));
 		counters_inc(session->stat_counters, pxc_oerrors);
-		pipex_rele_session(session);
-	} else
-		schednetisr(NETISR_PIPEX);
+		return;
+	}
+
+	ifp = if_get(session->proto.pppoe.over_ifidx);
+	if (ifp != NULL) {
+		struct arpcom *ac = (struct arpcom *)ifp;
+		struct ether_header *eh;
+
+		/* setup ethernet header information */
+		m_adj(m0, ETHER_ALIGN);
+		eh = mtod(m0, struct ether_header *);
+		memcpy(eh, session->peer.sa.sa_data, sizeof(*eh));
+		memcpy(eh->ether_shost, ac->ac_enaddr, sizeof(eh->ether_shost));
+
+		if (if_enqueue(ifp, m0) == 0)
+			counters_pkt(session->stat_counters, pxc_opackets,
+			    pxc_obytes, len);
+		else
+			counters_inc(session->stat_counters, pxc_oerrors);
+	} else {
+		m_freem(m0);
+		counters_inc(session->stat_counters, pxc_oerrors);
+	}
+	if_put(ifp);
 }
 #endif /* PIPEX_PPPOE */