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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20201001145728.GA4708@katalix.com>
Date:   Thu, 1 Oct 2020 15:57:29 +0100
From:   Tom Parkin <tparkin@...alix.com>
To:     Guillaume Nault <gnault@...hat.com>
Cc:     netdev@...r.kernel.org, jchapman@...alix.com
Subject: Re: [PATCH net-next 0/6] l2tp: add ac/pppoe driver

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

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)