Re: [PATCH 2/2] l2tp: Add Protocol field compression

[Guillaume Nault <g.nault@alphalink.fr>]

On Sun, Dec 16, 2018 at 08:46:19PM +0200, Sam Protsenko wrote:
> Hi Guillaume,
> 
> On Sun, Dec 16, 2018 at 6:30 PM Guillaume Nault <g.nault@alphalink.fr> wrote:
> >
> > On Fri, Dec 14, 2018 at 11:12:42PM +0200, Sam Protsenko wrote:
> > > When Protocol Field Compression (PFC) is enabled, the "Protocol" field
> > > in PPP packet should be transmitted without leading 0x00. See section
> > > 6.5 in RFC 1661 for details. Let's compress protocol field if needed,
> > > the same way it's done in drivers/net/ppp/pptp.c.
> > >
> > > To actually enable PFC, one should issue corresponding ioctl to L2TP
> > > driver from user-space, like this:
> > >
> > >     ioctl(fd, PPPIOCGFLAGS, &flags);
> > >     flags |= SC_COMP_PROT;
> > >     ioctl(fd, PPPIOCSFLAGS, &flags);
> > >
> > > It can be done e.g. from pppol2tp plugin (pppd), when pcomp option was
> > > negotiated with peer.
> > >
> > > Of course, we don't compress Protocol field when sending LCP packets. As
> > > stated in RFC 1661, section 6.5:
> > >
> > >     The Protocol field is never compressed when sending any LCP
> > >     packet.  This rule guarantees unambiguous recognition of LCP
> > >     packets.
> > >
> > Again, I'm sorry, but I must oppose this change. Although I'm lacking
> > time to keep sanitising L2TP, at least I'd like to avoid making the
> > situation worse.
> >
> > L2TP's uapi is already messy enough. Please don't add non-L2TP features
> > there.
> >
> > Activating PFC should be done on PPP file descriptors, not no L2TP
> > sockets. We certainly don't want L2TP to snoop on PPP data, much less
> > modify them.
> 
> Makes sense. I thought about this, too, just found that it's done that
> way in PPTP code and decided not to be too smart about this. Let me
> try and re-work this one. Will send v2 soon.
> 
While at it, be sure to target net-next and to post a cover letter if
you have more than one patch in the series.

Also, the code will have to be compatible with those layers that
already implement PFC using their own API (pptp.c, ppp_async.c, etc.).
I haven't looked at MP-PPP for a while, but multi-link might have to be
taken into account too.

But, for now, fixing the reception part is more important, IMO.