Netdev List
 help / color / mirror / Atom feed
From: Guillaume Nault <g.nault@alphalink.fr>
To: Sam Protsenko <semen.protsenko@linaro.org>
Cc: James Chapman <jchapman@katalix.com>,
	"David S. Miller" <davem@davemloft.net>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/2] l2tp: Add Protocol field compression
Date: Sun, 16 Dec 2018 17:30:59 +0100	[thread overview]
Message-ID: <20181216163059.i5nadsfzyvcwa4o6@kdev> (raw)
In-Reply-To: <20181214211242.9721-2-semen.protsenko@linaro.org>

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.

  reply	other threads:[~2018-12-16 16:30 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-14 21:12 [PATCH 1/2] l2tp: Bring back ->flags to struct pppol2tp_session Sam Protsenko
2018-12-14 21:12 ` [PATCH 2/2] l2tp: Add Protocol field compression Sam Protsenko
2018-12-16 16:30   ` Guillaume Nault [this message]
2018-12-16 18:46     ` Sam Protsenko
2018-12-16 19:33       ` Guillaume Nault

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=20181216163059.i5nadsfzyvcwa4o6@kdev \
    --to=g.nault@alphalink.fr \
    --cc=davem@davemloft.net \
    --cc=jchapman@katalix.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=semen.protsenko@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
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