On Tue, 10 Nov 2020 10:28:34 +0100 Guillaume Nault wrote: > On Mon, Nov 09, 2020 at 03:52:37PM -0800, Jakub Kicinski wrote: > > On Fri, 6 Nov 2020 18:16:45 +0000 Tom Parkin wrote: > > > This small RFC series implements a suggestion from Guillaume Nault in > > > response to my previous submission to add an ac/pppoe driver to the l2tp > > > subsystem[1]. > > > > > > Following Guillaume's advice, this series adds an ioctl to the ppp code > > > to allow a ppp channel to be bridged to another. Quoting Guillaume: > > > > > > "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." > > > > > > This allows userspace to easily take PPP frames from e.g. a PPPoE > > > session, and forward them over a PPPoL2TP session; accomplishing the > > > same thing my earlier ac/pppoe driver in l2tp did but in much less code! > > > > I have little understanding of the ppp code, but I can't help but > > wonder why this special channel connection is needed? We have great > > many ways to redirect traffic between interfaces - bpf, tc, netfilter, > > is there anything ppp specific that is required here? > > I can see two viable ways to implement this feature. The one described > in this patch series is the simplest. The reason why it doesn't reuse > existing infrastructure is because it has to work at the link layer > (no netfilter) and also has to work on PPP channels (no network > device). > > The alternative, is to implement a virtual network device for the > protocols we want to support (at least PPPoE and L2TP, maybe PPTP) > and teach tunnel_key about them. Then we could use iproute2 commands > like: > # ip link add name pppoe0 up type pppoe external > # ip link add name l2tp0 up type l2tp external > # tc qdisc add dev pppoe0 ingress > # tc qdisc add dev l2tp0 ingress > # tc filter add dev pppoe0 ingress matchall \ > action tunnel_key set l2tp_version 2 l2tp_tid XXX l2tp_sid YYY \ > action mirred egress redirect dev pppoe0 > # tc filter add dev l2tp0 ingress matchall \ > action tunnel_key set pppoe_sid ZZZ \ > action mirred egress redirect dev l2tp0 > > Note: I've used matchall for simplicity, but a real uses case would > have to filter on the L2TP session and tunnel IDs and on the PPPoE > session ID. > > As I said in my reply to the original thread, I like this idea, but > haven't thought much about the details. So there might be some road > blocks. Beyond modernising PPP and making it better integrated into the > stack, that should also bring the possibility of hardware offload (but > would any NIC vendor be interested?).
Integrating with the stack gives you access to all its features, other types of encap, firewalling, bpf, etc. > I think the question is more about long term maintainance. Do we want > to keep PPP related module self contained, with low maintainance code > (the current proposal)? Or are we willing to modernise the > infrastructure, add support and maintain PPP features in other modules > like flower, tunnel_key, etc.? Right, it's really not great to see new IOCTLs being added to drivers, but the alternative would require easily 50 times more code. > Of course, I might have missed other ways to implement this feature. > But that's all I could think of for now. > > And if anyone wants a quick recap about PPP (what are these PPP channel > and unit things? what's the relationship between PPPoE, L2TP and PPP? > etc.), just let me know. Some pointers would be appreciated if you don't mind :)
