On Thu, Oct 01, 2020 at 14:26:17 +0200, Guillaume Nault wrote: > On Wed, Sep 30, 2020 at 10:07:01PM +0100, Tom Parkin wrote: > > L2TPv2 tunnels are often used as a part of a home broadband connection, > > using a PPP link to connect the subscriber network into the Internet > > Service Provider's network. > > > > In this scenario, PPPoE is widely used between the L2TP Access > > Concentrator (LAC) and the subscriber. The LAC effectively acts as a > > PPPoE server, switching PPP frames from incoming PPPoE packets into an > > L2TP session. The PPP session is then terminated at the L2TP Network > > Server (LNS) on the edge of the ISP's IP network. > > > > This patchset adds a driver to the L2TP subsystem to support this mode > > of operation. > > Hi Tom, > > Nice to see someone working on this use case. However, have you > considered other implementation approaches? > > This new module reimplements PPPoE in net/l2tp (ouch!), so we'd now > have two PPPoE implementations with two different packet handlers for > ETH_P_PPP_SES. Also this implementation doesn't take into account other > related use cases, like forwarding PPP frames between two L2TP sessions > (not even talking about PPTP). > > A much simpler and more general approach would be to define a new PPP > ioctl, to "bridge" two PPP channels together. I discussed this with > DaveM at netdevconf 2.2 (Seoul, 2017) and we agreed that it was > probably the best way forward.
Hi Guillaume, Thank you for reviewing the patchset. I hadn't considered supporting this usecase in the ppp subsystem directly, so thank you for that suggestion. I can definitely see the appeal of avoiding reimplementing the PPPoE session packet handling. Having looked at the ppp code, I think it'd be a smaller change overall than this series, so that's also appealing. I'll wait on a little to let any other review comments come in, but if doing as you suggest is still the preferred approach I'll happily look at implementing it -- assuming you don't have a patch ready to go? Best regards, Tom
signature.asc
Description: PGP signature
