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

[Sam Protsenko]

Hi Guillaume,

On Sun, Dec 16, 2018 at 6:30 PM Guillaume Nault <g.nault@xxxxxxxxxxxx> 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.

Thanks.