public inbox for netdev@vger.kernel.org 
 help / color / mirror / Atom feed
From: Tom Parkin <tparkin@katalix•com>
To: Guillaume Nault <gnault@redhat•com>
Cc: netdev@vger•kernel.org, jchapman@katalix•com
Subject: Re: [PATCH net-next 0/6] l2tp: add ac/pppoe driver
Date: Thu, 1 Oct 2020 15:57:29 +0100	[thread overview]
Message-ID: <20201001145728.GA4708@katalix.com> (raw)
In-Reply-To: <20201001122617.GA9528@pc-2.home>

[-- 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 --]

  reply	other threads:[~2020-10-01 14:57 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-30 21:07 [PATCH net-next 0/6] l2tp: add ac/pppoe driver Tom Parkin
2020-09-30 21:07 ` [PATCH net-next 1/6] l2tp: add netlink info to session create callback Tom Parkin
2020-09-30 21:07 ` [PATCH net-next 2/6] l2tp: tweak netlink session create to allow L2TPv2 ac_pppoe Tom Parkin
2020-09-30 21:07 ` [PATCH net-next 3/6] l2tp: allow v2 netlink session create to pass ifname attribute Tom Parkin
2020-09-30 21:07 ` [PATCH net-next 4/6] l2tp: add netlink attributes for ac_ppp session creation Tom Parkin
2020-09-30 21:07 ` [PATCH net-next 5/6] l2tp: add ac_pppoe pseudowire driver Tom Parkin
2020-10-01 14:56   ` Jakub Kicinski
2020-10-01 16:24     ` Tom Parkin
2020-09-30 21:07 ` [PATCH net-next 6/6] docs: networking: update l2tp.rst to document PPP_AC pseudowires Tom Parkin
2020-10-01  8:59 ` [PATCH net-next 0/6] l2tp: add ac/pppoe driver James Chapman
2020-10-01 12:26 ` Guillaume Nault
2020-10-01 14:57   ` Tom Parkin [this message]
2020-10-01 18:30     ` Guillaume Nault

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201001145728.GA4708@katalix.com \
    --to=tparkin@katalix$(echo .)com \
    --cc=gnault@redhat$(echo .)com \
    --cc=jchapman@katalix$(echo .)com \
    --cc=netdev@vger$(echo .)kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox