From: Claudio Jeker Subject: Re: pppoe(4): replace PPPOE_TERM_UNKNOWN_SESSIONS with ifconfig knob To: renaud@openbsd.org Cc: tech@openbsd.org, Bjorn Ketelaars Date: Thu, 15 Oct 2026 08:51:12 +0200 On Thu, Oct 15, 2026 at 08:20:17AM +0200, renaud@openbsd.org wrote: > > Any OK for this? Diff looks ok to me. I don't use pppoe(4) so I can't test. Using an ifconfig link flag for this is certainly an improvement over the config option. > 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 > -- :wq Claudio