Message-ID: <20201001122617.GA9528@pc-2.home>
Date:   Thu, 1 Oct 2020 14:26:17 +0200
From:   Guillaume Nault <gnault@...hat.com>
To:     Tom Parkin <tparkin@...alix.com>
Cc:     netdev@...r.kernel.org, jchapman@...alix.com
Subject: Re: [PATCH net-next 0/6] l2tp: add ac/pppoe driver

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.

It's just a matter of extending struct channel (in ppp_generic.c) with
a pointer to another channel, then testing this pointer in ppp_input().
If the pointer is NULL, use the classical path, if not, forward the PPP
frame using the ->start_xmit function of the peer channel. There are a
few details to take into account of course (crossing netns, locking),
but nothing big (I could implement it the following night in my hotel
room before leaving Seoul). This approach should work for forwarding
PPP frames between any type of PPP transport.

I unfortunately didn't propose the code upstream at that time, because
I didn't want to add this kernel feature without having a userspace
implementation making use of it and ready to release (and I finally
left the company before that happened). But I know that this
implementation worked fine as it did receive quite a lot of testing.

Yet another way to implement this feature would to define virtual PPPoE
and L2TP devices, working in external mode. In practice, one PPPoE and
one L2TP network device would be enough for handling all the traffic.
Then TC could be used to pass the PPP frames between PPPoE and L2TP.

Example (assuming flower and tunnel_key were extented to support PPPoE
and L2TP):

# Forward PPPoE frames with Session-ID 5 to L2TP tunnel 1 session 1
$ tc filter add dev pppoe0 ingress flower pppoe_sid 5   \
    action tunnel_key src_ip 192.0.2.1 dst_ip 192.0.2.2 \
                      l2tp_tid 1 l2tp_peertid 1         \
                      l2tp_sid 1 l2tp_peer_sid 1        \
    action mirred egress redirect dev l2tp0

# Reverse path
$ tc filter add dev l2tp0 ingress flower l2tp_tid 1 l2tp_sid 1            \
    action tunnel_key dst_mac 02:00:00:00:00:01 src_mac 02:00:00:00:00:02 \
                      id 5                                                \
    action mirred egress redirect dev pppoe0

Of course the commands would be a bit longer in practice (one would
probably want to match on the src and dst IP addresses in the reverse
path, or set the L2TP version, etc.), but that's the general idea.

Such approach would probably not allow the use of L2TP sequence numbers
though (which might not be a bad thing in the end). It'd also require
more work, but would avoid going through the PPP layer and might even
be offloadable (if a NIC vendor ever wants to support it).

Regards,

Guillaume

Powered by blists - more mailing lists