Re: [PATCH net-next 0/6] l2tp: add ac/pppoe driver

[Tom Parkin <tparkin@katalix.com>]

[-- Attachment #1: Type: text/plain, Size: 2103 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]