Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20181214211242.9721-1-semen.protsenko@linaro.org>
 <20181214211242.9721-2-semen.protsenko@linaro.org> <20181216163059.i5nadsfzyvcwa4o6@kdev>
In-Reply-To: <20181216163059.i5nadsfzyvcwa4o6@kdev>
From:   Sam Protsenko <semen.protsenko@linaro.org>
Date:   Sun, 16 Dec 2018 20:46:19 +0200
Message-ID: <CAKaJLVvj2Yv0hOXiRLvJ1Gnm3axOds21LHxmCW5K0YNpx+hk6w@mail.gmail.com>
Subject: Re: [PATCH 2/2] l2tp: Add Protocol field compression
To:     Guillaume Nault <g.nault@alphalink.fr>
Cc:     James Chapman <jchapman@katalix.com>,
        "David S. Miller" <davem@davemloft.net>, netdev@vger.kernel.org,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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.

Thanks.