| Message ID | 20200930210707.10717-1-tparkin@katalix.com |
|---|---|
| Headers | show |
| Series | l2tp: add ac/pppoe driver | expand |
On 30/09/2020 22:07, 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. > > The new driver, l2tp_ac_pppoe, adds support for the existing pseudowire > type L2TP_PWTYPE_PPP_AC, and is instantiated using the existing L2TP > netlink L2TP_CMD_SESSION_CREATE. It is expected to be used as follows: > > * A userspace PPPoE daemon running on the LAC handles the PPPoE > discovery process up to the point of assigning a PPPoE session ID and > sending the PADS packet to the PPPoE peer to establish the PPPoE > session. > * Userspace code running on the LAC then instantiates an L2TP tunnel > and session with the LNS using the L2TP control protocol. > * Finally, the data path for PPPoE session frames through the L2TP > session to the LAC is instantiated by sending a genetlink > L2TP_CMD_SESSION_CREATE command to the kernel, including > the PPPoE-specific metadata required for L2TP_PWTYPE_PPP_AC sessions > (this is documented in the patch series commit comments). > > Supporting this driver submission we have two examples of userspace > projects which use L2TP_PWTYPE_PPP_AC: > > * https://github.com/katalix/l2tp-ktest > > This is a unit-test suite for the kernel L2TP subsystem which has > been updated to include basic lifetime and datapath tests for > l2tp_ac_pppoe. > > The new tests are automatically enabled when l2tp_ac_pppoe > availability is detected, and hence support for l2tp_ac_pppoe is on > the master branch of the git repository. > > * https://github.com/katalix/go-l2tp > > This is a Go library for building L2TP applications on Linux, and > includes a suite of example daemons which utilise the library. > > The daemon kpppoed implements the PPPoE discovery protocol, and spawns > an instance of a daemon kl2tpd which handles the L2TP control protocol > and instantiates the kernel data path. > > The code utilising l2tp_ac_pppoe is on the branch tp_002_pppoe_1 > pending merge of this patchset in the kernel. > > Notes on the patchset itself: > > * Patches 1-4 lay groundwork for the addition of the new driver, making > tweaks to the l2tp netlink code to allow l2tp_ac_pppoe to access the > netlink attributes it requires. > * Patch 5 adds the new driver itself and hooks it into the kernel > configuration and build system. > * Patch 6 updates the l2tp documentation under Documentation/ to > include information about the new driver. > > Tom Parkin (6): > l2tp: add netlink info to session create callback > l2tp: tweak netlink session create to allow L2TPv2 ac_pppoe > l2tp: allow v2 netlink session create to pass ifname attribute > l2tp: add netlink attributes for ac_ppp session creation > l2tp: add ac_pppoe pseudowire driver > docs: networking: update l2tp.rst to document PPP_AC pseudowires > > Documentation/networking/l2tp.rst | 69 +++-- > include/uapi/linux/l2tp.h | 2 + > net/l2tp/Kconfig | 7 + > net/l2tp/Makefile | 1 + > net/l2tp/l2tp_ac_pppoe.c | 446 ++++++++++++++++++++++++++++++ > net/l2tp/l2tp_core.h | 4 +- > net/l2tp/l2tp_eth.c | 3 +- > net/l2tp/l2tp_netlink.c | 20 +- > net/l2tp/l2tp_ppp.c | 3 +- > 9 files changed, 527 insertions(+), 28 deletions(-) > create mode 100644 net/l2tp/l2tp_ac_pppoe.c > Reviewed-by: James Chapman <jchapman@katalix.com>
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
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
On Thu, Oct 01, 2020 at 03:57:29PM +0100, Tom Parkin wrote: > 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? I unfortunately don't. All I saved from my previous ongoing L2TP and PPP work is a (long) TODO list.