Index | Thread | Search

From:
renaud@openbsd.org
Subject:
Re: pppoe(4): replace PPPOE_TERM_UNKNOWN_SESSIONS with ifconfig knob
To:
Bjorn Ketelaars <bket@openbsd.org>
Cc:
tech@openbsd.org
Date:
Wed, 14 Oct 2026 10:51:06 +0200

Download raw body.

Thread
On Tue, Oct 13, 2026 at 08:07:17PM +0200, Bjorn Ketelaars wrote:
> Thanks for looking at this! Your version addresses the issue more
> neatly. Avoiding sending when down, keeping the logging quieter, and
> sending only one PADT per stale frame all make sense to me.

Actually, I made more tests and testing for IFF_UP defeats the whole
thing. After the refused login, sppp_lcp_down() calls if_down() itself,
so the interface is down exactly when the stale session has to be
killed. Client losing its session without telling the AC.
Live testing with my ISP is quite complex as it can bring my line down
for 30-60+ minutes.


Index: sys/net/if_pppoe.c
===================================================================
RCS file: /cvs/src/sys/net/if_pppoe.c,v
diff -u -p -r1.91 if_pppoe.c
--- sys/net/if_pppoe.c	17 Jun 2026 17:02:03 -0000	1.91
+++ sys/net/if_pppoe.c	14 Oct 2026 08:11:24 -0000
@@ -834,17 +834,14 @@ pppoe_data_input(struct mbuf *m)
 	struct pppoe_softc *sc;
 	struct pppoehdr *ph;
 	u_int16_t session, plen;
-#ifdef PPPOE_TERM_UNKNOWN_SESSIONS
 	u_int8_t shost[ETHER_ADDR_LEN];
-#endif
+
 	if (LIST_EMPTY(&pppoe_softc_list))
 		goto drop;
 
 	KASSERT(m->m_flags & M_PKTHDR);
 
-#ifdef PPPOE_TERM_UNKNOWN_SESSIONS
 	memcpy(shost, mtod(m, struct ether_header*)->ether_shost, ETHER_ADDR_LEN);
-#endif
 	m_adj(m, sizeof(struct ether_header));
 	if (m->m_pkthdr.len <= PPPOE_HEADERLEN) {
 		printf("pppoe (data): dropping too short packet: %d bytes\n",
@@ -870,11 +867,20 @@ pppoe_data_input(struct mbuf *m)
 	session = ntohs(ph->session);
 	sc = pppoe_find_softc_by_session(session, m->m_pkthdr.ph_ifidx);
 	if (sc == NULL) {
-#ifdef PPPOE_TERM_UNKNOWN_SESSIONS
-		printf("pppoe (data): input for unknown session 0x%x, sending PADT\n",
-		    session);
-		pppoe_send_padt(m->m_pkthdr.ph_ifidx, session, shost, 0);
-#endif
+		/*
+		 * The peer may still hold a session from before a crash and
+		 * refuse to open a new one. If link2 is set, terminate it.
+		 */
+		LIST_FOREACH(sc, &pppoe_softc_list, sc_list)
+			if (sc->sc_eth_ifidx == m->m_pkthdr.ph_ifidx &&
+			    (sc->sc_sppp.pp_if.if_flags & IFF_LINK2))
+				break;
+		if (sc != NULL) {
+			PPPOEDEBUG(("%s: input for unknown session 0x%x, "
+			    "sending PADT\n", sc->sc_sppp.pp_if.if_xname,
+			    session));
+			pppoe_send_padt(m->m_pkthdr.ph_ifidx, session, shost, 0);
+		}
 		goto drop;
 	}
 
Index: share/man/man4/pppoe.4
===================================================================
RCS file: /cvs/src/share/man/man4/pppoe.4,v
diff -u -p -r1.37 pppoe.4
--- share/man/man4/pppoe.4	17 Jun 2026 17:02:03 -0000	1.37
+++ share/man/man4/pppoe.4	14 Oct 2026 08:11:24 -0000
@@ -126,7 +126,8 @@ the gateway will be changed to the sugge
 .Pp
 Otherwise, PPP negotiation will only agree to exactly the IPv4 addresses
 which are configured on the interface.
-.Sh KERNEL OPTIONS
+.Pp
+By default,
 .Nm
 does not interfere with other PPPoE implementations
 running on the same machine.
@@ -137,13 +138,22 @@ reestablish a new PPPoE connection
 because there is already an open session.
 This would be indicated by the client sending a high number of PADI packets
 before successfully connecting.
-The
+Setting
+.Em link2
+makes
 .Nm
-driver can be told to kill all unknown PPPoE sessions
-by sending a PADT packet to explicitly terminate the old session.
-Add the following to the kernel config file:
-.Pp
-.Dl option PPPOE_TERM_UNKNOWN_SESSIONS
+answer packets for unknown sessions on its Ethernet interface
+with a PADT packet to explicitly terminate the old session,
+as long as no
+.Nm
+interface has an established session.
+Sessions of other PPPoE implementations
+using the same Ethernet interface are unknown to
+.Nm
+and would be terminated as well,
+so
+.Em link2
+should not be set in that case.
 .Sh PPPOE AND MTU/MSS
 PPPoE has an 8-byte header.
 When run over a network interface with the
Index: share/man/man4/options.4
===================================================================
RCS file: /cvs/src/share/man/man4/options.4,v
diff -u -p -r1.273 options.4
--- share/man/man4/options.4	1 Jun 2025 00:32:54 -0000	1.273
+++ share/man/man4/options.4	14 Oct 2026 08:11:24 -0000
@@ -385,10 +385,6 @@ Enables BSD compressor for PPP connectio
 .It Cd option PPP_DEFLATE
 For use in conjunction with PPP_BSDCOMP; provides an interface to zlib for PPP
 for deflate compression/decompression.
-.It Cd option PPPOE_TERM_UNKNOWN_SESSIONS
-Send PADT to terminate open sessions before connecting.
-See
-.Xr pppoe 4 .
 .It Cd option SOCKET_SPLICE
 Enables zero-copy socket splicing in the kernel.
 See


> With the kernel option going away, it would also be useful to mention
> the replacement in https://www.openbsd.org/faq/current.html, so people
> using PPPOE_TERM_UNKNOWN_SESSIONS know to enable link2 instead. Perhaps:
> 
> The kernel option PPPOE_TERM_UNKNOWN_SESSIONS has been removed. To
> enable termination of unknown PPPoE sessions, add link2 to
> /etc/hostname.pppoe0 (adjusting the interface name as needed). This only
> takes effect while the interface is up. Do not enable it if another
> PPPoE implementation uses the same Ethernet interface, as its sessions
> may also be terminated.

Agreed, but that's a separate commit.

> 
> I have built this and am currently running it on my home connection. I
> have not experienced the issue with my ISP for some time now, so I no
> longer seem to need this and have not been able to see the stale-session
> recovery. Still, so far nothing has exploded.