LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Guillaume Nault <gnault@redhat.com>
To: "Pali Rohár" <pali@kernel.org>
Cc: James Carlson <carlsonj@workingcode.com>,
	Chris Fowler <cfowler@outpostsentinel.com>,
	Jakub Kicinski <kuba@kernel.org>,
	Paul Mackerras <paulus@samba.org>,
	"David S. Miller" <davem@davemloft.net>,
	"linux-ppp@vger.kernel.org" <linux-ppp@vger.kernel.org>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] ppp: Add rtnl attribute IFLA_PPP_UNIT_ID for specifying ppp unit id
Date: Thu, 12 Aug 2021 11:28:47 +0200	[thread overview]
Message-ID: <20210812092847.GB3525@pc-23.home> (raw)
In-Reply-To: <20210811180401.owgmie36ydx62iep@pali>

On Wed, Aug 11, 2021 at 08:04:01PM +0200, Pali Rohár wrote:
> On Wednesday 11 August 2021 19:38:11 Guillaume Nault wrote:
> > On Tue, Aug 10, 2021 at 02:11:11PM -0400, James Carlson wrote:
> > > On 8/10/21 1:16 PM, Pali Rohár wrote:
> > > > On Tuesday 10 August 2021 16:38:32 Chris Fowler wrote:
> > > > > Isn't the UNIT ID the interface number?  As in 'unit 100' will give me ppp100?
> > > > 
> > > > If you do not specify pppd 'ifname' argument then pppd argument 'unit 100'
> > > > will cause that interface name would be ppp100.
> > > > 
> > > > But you are free to rename interface to any string which you like, even
> > > > to "ppp99".
> > > > 
> > > > But this ppp unit id is not interface number. Interface number is
> > > > another number which has nothing with ppp unit id and is assigned to
> > > > every network interface (even loopback). You can see them as the first
> > > > number in 'ip -o l' output. Or you can retrieve it via if_nametoindex()
> > > > function in C.
> > > 
> > > Correct; completely unrelated to the notion of "interface index."
> > > 
> > > > ... So if people are really using pppd's 'unit' argument then I think it
> > > > really make sense to support it also in new rtnl interface.
> > > 
> > > The pppd source base is old.  It dates to the mid-80's.  So it predates not
> > > just rename-able interfaces in Linux but Linux itself.
> > > 
> > > I recall supported platforms in the past (BSD-derived) that didn't support
> > > allowing the user to specify the unit number.  In general, on those
> > > platforms, the option was accepted and just ignored, and there were either
> > > release notes or man page updates (on that platform) that indicated that
> > > "unit N" wouldn't work there.
> > > 
> > > Are there users on Linux who make use of the "unit" option and who would
> > > mourn its loss?  Nobody really knows.  It's an ancient feature that was
> > > originally intended to deal with systems that couldn't rename interfaces
> > > (where one had to make sure that the actual interface selected matched up
> > > with pre-configured filtering rules or static routes or the like), and to
> > > make life nice for administrators (e.g., making sure that serial port 1 maps
> > > to ppp1, port 2 is ppp2, and so on).
> > > 
> > > I would think and hope most users reach for the more-flexible "ifname"
> > > option first, but I certainly can't guarantee it.  It could be buried in a
> > > script somewhere or (god forbid) some kind of GUI or "usability" tool.
> > > 
> > > If I were back at Sun, I'd probably call it suitable only for a "Major"
> > > release, as it removes a publicly documented feature.  But I don't know what
> > > the considerations are here.  Maybe it's just a "don't really care."
> > 
> > I'm pretty sure someone, somewhere, would hate us if we broke the
> > "unit" option. The old PPP ioctl API has been there for so long,
> > there certainly remains tons of old tools, scripts and config files
> > that "just work" without anybody left to debug or upgrade them.
> > 
> > We can't just say, "starting from kernel x.y.z the unit option is a
> > noop, use ifname instead" as affected people surely won't get the
> > message (and there are other tools beyond pppd that may use this
> > kernel API).
> > 
> > But for the netlink API, we don't have to repeat the same mistake.
> 
> ifname is not atomic (first it creates ppp<id> interface and later it is
> renamed) and have issues. Due to bug described here:
> https://lore.kernel.org/netdev/20210807160050.17687-1-pali@kernel.org/
> you may get your kernel into state in which it is not possible to create
> a new ppp interface. And this issue does not happen when using "unit"
> argument.

This is specific to the ioctl api. Netlink doesn't have this problem.

> To fix above issue it is needed to migrate pppd from ioctl API to rtnl.

It would have helped a lot if you had explained that before.

> But this would be possible only after rtnl API starts providing all
> features, including specifying custom "unit" argument...

You can already simulate the "unit" option by setting the interface
name as "ppp${unit}" and retrieving the kernel assigned id with
ioctl(PPPIOCGUNIT). What's wrong with that?

> I hit above problem, so now I'm migrating all pppd setups from "ifname"
> to "unit" option.

Why did you write 3125f26c51482 ("ppp: Fix generating ppp unit id when
ifname is not specified") then?


  reply	other threads:[~2021-08-12  9:29 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-07 16:37 Pali Rohár
2021-08-09 19:25 ` Jakub Kicinski
2021-08-09 19:31   ` Pali Rohár
2021-08-10 15:39     ` Guillaume Nault
2021-08-10 16:04       ` Pali Rohár
2021-08-11 17:19         ` Guillaume Nault
2021-08-11 17:54           ` Pali Rohár
2021-08-12  9:19             ` Guillaume Nault
2021-08-12 14:09               ` Pali Rohár
2021-08-12 19:12                 ` Guillaume Nault
     [not found]       ` <BN0P223MB0327A247724B7AE211D2E84EA7F79@BN0P223MB0327.NAMP223.PROD.OUTLOOK.COM>
2021-08-10 17:16         ` Pali Rohár
2021-08-10 18:11           ` James Carlson
2021-08-11 17:38             ` Guillaume Nault
2021-08-11 18:04               ` Pali Rohár
2021-08-12  9:28                 ` Guillaume Nault [this message]
2021-08-12 13:48                   ` Pali Rohár
2021-08-12 18:26                     ` Guillaume Nault
2021-08-12 19:04                       ` Pali Rohár
2021-08-16 16:11                         ` Guillaume Nault
2021-08-16 16:23                           ` Pali Rohár
2021-08-17 16:05                             ` Guillaume Nault
2021-08-17 16:21                               ` Pali Rohár
2022-07-09 12:09                                 ` Pali Rohár
2022-07-12 17:34                                   ` 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=20210812092847.GB3525@pc-23.home \
    --to=gnault@redhat.com \
    --cc=carlsonj@workingcode.com \
    --cc=cfowler@outpostsentinel.com \
    --cc=davem@davemloft.net \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-ppp@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pali@kernel.org \
    --cc=paulus@samba.org \
    --subject='Re: [PATCH] ppp: Add rtnl attribute IFLA_PPP_UNIT_ID for specifying ppp unit id' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).