Index | Thread | Search

From:
Bjorn Ketelaars <bket@openbsd.org>
Subject:
Re: pppoe(4): replace PPPOE_TERM_UNKNOWN_SESSIONS with ifconfig knob
To:
Renaud Allard <renaud@allard.it>
Cc:
tech@openbsd.org
Date:
Thu, 15 Oct 2026 07:15:37 +0200

Download raw body.

Thread
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.