From: Tom Parkin <tparkin@katalix•com>
To: Jakub Kicinski <kuba@kernel•org>
Cc: Guillaume Nault <gnault@redhat•com>,
netdev@vger•kernel.org, jchapman@katalix•com
Subject: Re: [RFC PATCH 0/2] add ppp_generic ioctl to bridge channels
Date: Tue, 17 Nov 2020 12:54:22 +0000 [thread overview]
Message-ID: <20201117125422.GC4640@katalix.com> (raw)
In-Reply-To: <20201110084740.3e3418c0@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
[-- Attachment #1: Type: text/plain, Size: 4075 bytes --]
On Tue, Nov 10, 2020 at 08:47:40 -0800, Jakub Kicinski wrote:
> 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.
Jakub, could I quickly poll you on your current gut-feel level of
opposition to the ioctl-based approach?
Guillaume has given good feedback on the RFC code which I can work
into an actual patch submission, but I don't really want to if you're
totally opposed to the whole idea :-)
I appreciate you may want to reserve judgement pending a recap of the
ppp subsystem as it stands.
Thanks!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
next prev parent reply other threads:[~2020-11-17 12:54 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-06 18:16 [RFC PATCH 0/2] add ppp_generic ioctl to bridge channels Tom Parkin
2020-11-06 18:16 ` [RFC PATCH 1/2] ppp: add PPPIOCBRIDGECHAN ioctl Tom Parkin
2020-11-09 23:24 ` Guillaume Nault
2020-11-10 12:04 ` Tom Parkin
2020-11-15 16:28 ` Guillaume Nault
2020-11-17 12:26 ` Tom Parkin
2020-11-17 14:06 ` Guillaume Nault
2020-11-06 18:16 ` [RFC PATCH 2/2] docs: update ppp_generic.rst to describe ioctl PPPIOCBRIDGECHAN Tom Parkin
2020-11-09 22:51 ` [RFC PATCH 0/2] add ppp_generic ioctl to bridge channels Guillaume Nault
2020-11-10 11:54 ` Tom Parkin
2020-11-15 11:59 ` Guillaume Nault
2020-11-17 12:12 ` Tom Parkin
2020-11-09 23:52 ` Jakub Kicinski
2020-11-10 9:28 ` Guillaume Nault
2020-11-10 12:42 ` Tom Parkin
2020-11-10 15:02 ` Guillaume Nault
2020-11-10 16:47 ` Jakub Kicinski
2020-11-17 12:54 ` Tom Parkin [this message]
2020-11-17 14:17 ` Guillaume Nault
2020-11-17 16:52 ` Jakub Kicinski
2020-11-18 20:24 ` Guillaume Nault
2020-11-20 1:17 ` Jakub Kicinski
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=20201117125422.GC4640@katalix.com \
--to=tparkin@katalix$(echo .)com \
--cc=gnault@redhat$(echo .)com \
--cc=jchapman@katalix$(echo .)com \
--cc=kuba@kernel$(echo .)org \
--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