Re: [RFC PATCH 0/2] add ppp_generic ioctl to bridge channels

[Tom Parkin <tparkin@katalix.com>]

[-- Attachment #1: Type: text/plain, Size: 2175 bytes --]

On  Tue, Nov 10, 2020 at 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.
> > 
> > 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.

One potential downside of this approach is the addition of two virtual
interfaces for each pppoe->pppol2tp mapping: the concern here
primarily being the cost of doing so.

I'm not saying the cost is necessarily prohibitive, but the "bridge the
channels" approach in the RFC is certainly cheaper.

Another concern would be the possibility of the virtual devices being
misconfigured in such a way as to e.g. allow locally generated
broadcast packets to go out on one of the interfaces.  Possibly this
would be easy to avoid, I'm not sure.

> 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.?

FWIW I would tend to agree.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]