Sebastien Marie <semarie@kapouay.eu.org> writes:

> David Gwynne <david@gwynne.id.au> writes:
>
>> this let's packets being sent out pppoe interfaces bypass queues and go
>> straight onto the underlying interface.
>>
>> it's complementary to src/sys/net/if_pppoe.c r1.85 "let pppoe data
>> packets go through if_vinput instead of the pppoeinq", but where that
>> skipped a queue on the rx side, this diff does it on the tx side.
>>
>> i don't use pppoe anymore, so i need someone to test this diff before i
>> can commiti it.
>
> it doesn't apply cleanly in my tree (/* $OpenBSD: if_pppoe.c,v 1.86 2025/03/02 21:28:32 bluhm Exp $ */)
>
> patching file if_pppoe.c
> Hunk #1 FAILED at 42.
> Hunk #2 succeeded at 160 (offset 4 lines).
> Hunk #3 succeeded at 249 with fuzz 2 (offset 17 lines).
> Hunk #4 succeeded at 266 with fuzz 2 (offset 18 lines).
> Hunk #5 succeeded at 1544 (offset 152 lines).
> Hunk #6 FAILED at 1452.
> 2 out of 6 hunks FAILED

I adapted the diff to match -current, and tested it.

But on startup (when starting network interfaces), the kernel panics
with the following trace:

panic: kernel diagnostic assertion "ifp->if_counters == NULL" failed: file "/sy
s/net/if.c", line 2795
Starting stack trace...
panic(ffffffff8266389a) at panic+0x175
__assert(ffffffff8261233f,ffffffff8257b628,aeb,ffffffff82609cce) at __assert+0x29
if_counters_alloc(ffff80000071d000) at if_counters_alloc+0x7f
pppoe_clone_create(ffffffff829ef910,0) at pppoe_clone_create+0x190
if_clone_create(ffff800042808840,0) at if_clone_create+0xb5
ifioctl(ffff800016e62008,8020697a,ffff800042808840,ffff8000ffff4a68) at ifioctl+0x1ec
sys_ioctl(ffff8000ffff4a68,ffff8000428089f0,ffff800042808960) at sys_ioctl+0x301
syscall(ffff8000428089f0) at syscall+0x5ec
Xsyscall() at Xsyscall+0x128
end of kernel
end trace frame: 0x7cd554e4bd30, count: 248
End of stack trace.

Below the used diff (to check if the bug is mine).

Thanks.
-- 
Sebastien Marie


Commit ID: 78a188743907c943967ca06da1c9b64ce48e1e5e
Change ID: ntmxvxtvtlqzrqklosutvqnyzppqrkrm
Author   : Sebastien Marie <semarie@kapouay.eu.org> (2025-05-14 08:36:59)
Committer: Sebastien Marie <semarie@kapouay.eu.org> (2025-05-14 09:03:12)

    let's packets being sent out pppoe interfaces bypass queues and go straight onto the underlying interface.

    from dlg@ in id:aCPw6hi1M2/zToMq@animata.net

    patch updated manually for -current

diff --git a/sys/net/if_pppoe.c b/sys/net/if_pppoe.c
index b212ac929d..69571439fb 100644
--- a/sys/net/if_pppoe.c
+++ b/sys/net/if_pppoe.c
@@ -160,6 +160,7 @@
 static int  pppoe_ioctl(struct ifnet *, unsigned long, caddr_t);
 static void pppoe_tls(struct sppp *);
 static void pppoe_tlf(struct sppp *);
+static int  pppoe_enqueue(struct ifnet *, struct mbuf *);
 static void pppoe_start(struct ifnet *);
 
 /* internal timeout handling */
@@ -248,6 +249,7 @@
 	sc->sc_sppp.pp_if.if_input = p2p_input;
 	sc->sc_sppp.pp_if.if_bpf_mtap = p2p_bpf_mtap;
 	sc->sc_sppp.pp_if.if_ioctl = pppoe_ioctl;
+	sc->sc_sppp.pp_if.if_enqueue = pppoe_enqueue;
 	sc->sc_sppp.pp_if.if_start = pppoe_start;
 	sc->sc_sppp.pp_if.if_rtrequest = p2p_rtrequest;
 	sc->sc_sppp.pp_if.if_xflags = IFXF_CLONED;
@@ -264,6 +266,7 @@
 	if_attach(&sc->sc_sppp.pp_if);
 	if_alloc_sadl(&sc->sc_sppp.pp_if);
 	sppp_attach(&sc->sc_sppp.pp_if);
+	if_counters_alloc(&sc->sc_sppp.pp_if);
 #if NBPFILTER > 0
 	bpfattach(&sc->sc_bpf, &sc->sc_sppp.pp_if, DLT_PPP_ETHER, 0);
 	bpfattach(&sc->sc_sppp.pp_if.if_bpf, &sc->sc_sppp.pp_if,
@@ -1541,13 +1544,55 @@
 	timeout_add_msec(&sc->sc_timeout, 20);
 }
 
+int
+pppoe_transmit(struct pppoe_softc *sc, struct mbuf *m)
+{
+	size_t len;
+	uint8_t *p;
+
+	len = m->m_pkthdr.len;
+	M_PREPEND(m, PPPOE_HEADERLEN, M_DONTWAIT);
+	if (m == NULL)
+		return (ENOBUFS);
+
+	p = mtod(m, u_int8_t *);
+	PPPOE_ADD_HEADER(p, 0, sc->sc_session, len);
+
+#if NBPFILTER > 0
+	if (sc->sc_sppp.pp_if.if_bpf)
+		bpf_mtap(sc->sc_sppp.pp_if.if_bpf, m, BPF_DIRECTION_OUT);
+#endif
+
+	return (pppoe_output(sc, m));
+}
+
+int
+pppoe_enqueue(struct ifnet *ifp, struct mbuf *m)
+{
+	struct pppoe_softc *sc;
+	int error;
+
+	if (!ifq_is_priq(&ifp->if_snd))
+		return (if_enqueue_ifq(ifp, m));
+
+	sc = ifp->if_softc;
+	if (sc->sc_state < PPPOE_STATE_SESSION) {
+		m_freem(m);
+		return (ENETDOWN);
+	}
+
+	error = pppoe_transmit(sc, m);
+	if (error != 0)
+		counters_inc(ifp->if_counters, ifc_oerrors);
+
+	return (error);
+}
+
 static void
 pppoe_start(struct ifnet *ifp)
 {
 	struct pppoe_softc *sc = (void *)ifp;
 	struct mbuf *m;
-	size_t len;
-	u_int8_t *p;
 
 	if (sppp_isempty(ifp))
 		return;
@@ -1558,21 +1603,6 @@
 		return;
 	}
 
-	while ((m = sppp_dequeue(ifp)) != NULL) {
-		len = m->m_pkthdr.len;
-		M_PREPEND(m, PPPOE_HEADERLEN, M_DONTWAIT);
-		if (m == NULL) {
-			ifp->if_oerrors++;
-			continue;
-		}
-		p = mtod(m, u_int8_t *);
-		PPPOE_ADD_HEADER(p, 0, sc->sc_session, len);
-
-#if NBPFILTER > 0
-		if (sc->sc_bpf)
-			bpf_mtap(sc->sc_bpf, m, BPF_DIRECTION_OUT);
-#endif
-
-		pppoe_output(sc, m);
-	}
+	while ((m = sppp_dequeue(ifp)) != NULL)
+		pppoe_transmit(sc, m);
 }