From: renaud@openbsd.org Subject: Re: pppoe(4): replace PPPOE_TERM_UNKNOWN_SESSIONS with ifconfig knob To: tech@openbsd.org Cc: Bjorn Ketelaars Date: Thu, 15 Oct 2026 08:20:17 +0200 Any OK for this? For convenience, I have put the patch inline in this mail On Thu, Oct 15, 2026 at 07:15:37AM +0200, Bjorn Ketelaars wrote: > From: Bjorn Ketelaars > To: Renaud Allard > Cc: tech@openbsd.org > Subject: Re: pppoe(4): replace PPPOE_TERM_UNKNOWN_SESSIONS with ifconfig > knob > Date: Thu, 15 Oct 2026 07:15:37 +0200 > > On Wed 14/10/2026 10:51, Renaud Allard wrote: > > 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. > > > > Thanks for testing this further, and sorry I missed the IFF_UP issue. I > did not check which paths could change that flag before giving my OK. > Removing the check brings the behaviour closer to my proposal, which > worked for me at the time, while retaining your improvements to logging > and avoiding duplicate PADTs. > > Something appears to have changed on my ISP's side, beyond my control. > I no longer experience minutes-long connectivity loss after an > uncontrolled reboot, so I cannot reproduce the original stale-session > problem. I have built and run the updated patch, as I did with the > previous version. > > I have now also tested the unknown-session handling using Scapy on a > second machine, sending through igc1 to gateway's bge0, the parent of > pppoe9. The script was: > > from scapy.all import Ether, IP, PPP, PPPoE, sendp > > packet = ( > Ether(dst="d0:bf:9c:46:de:14", type=0x8864) > / PPPoE(version=1, type=1, code=0, sessionid=0x1234) > / PPP(proto=0x0021) > / IP(src="192.0.2.1", dst="192.0.2.2") > ) > > sendp(packet, iface="igc1", count=1, verbose=False) > > I ran the script as root and captured on gateway with: > > doas tcpdump -n -e -vv -i bge0 'ether proto 0x8863 or ether proto 0x8864' > > With: > > doas ifconfig pppoe9 link2 down > > I observed the injected frame followed by one PADT: > > 06:56:46.165141 00:00:00:00:00:00 d0:bf:9c:46:de:14 8864 60: PPPoE-Session > code Session, version 1, type 1, id 0x1234, length 22 > IP 192.0.2.1 > 192.0.2.2: ip-proto-0 0 (ttl 64, id 1, len 20) > 06:56:46.165187 d0:bf:9c:46:de:14 00:00:00:00:00:00 8863 20: PPPoE-Discovery > code Terminate, version 1, type 1, id 0x1234, length 0 > > With: > > doas ifconfig pppoe9 -link2 down > > The same injected frame was captured, but no PADT was observed: > > 06:57:09.493644 00:00:00:00:00:00 d0:bf:9c:46:de:14 8864 60: PPPoE-Session > code Session, version 1, type 1, id 0x1234, length 22 > IP 192.0.2.1 > 192.0.2.2: ip-proto-0 0 (ttl 64, id 1, len 20) > > So the updated patch does the right thing in these tests: link2 controls > the termination response, and the interface being down does not prevent > it. This verifies PADT generation, although I have not tested the AC > releasing a stale session and accepting a new connection. > > Thanks again for catching this and improving the diff. 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