On Tue, Mar 16, 2021 at 12:23:31AM +0100, Klemens Nanni wrote:
> On Sat, Mar 13, 2021 at 11:45:30PM +0100, Claudio Jeker wrote:
> > On Sat, Mar 13, 2021 at 11:31:05PM +0100, Klemens Nanni wrote:
> > > First off: I've never used mpe(4), mpw(4) or mpip(4); this occured to
> > > me while looking at ifconfig.{c,8} in general.
> > >
> > >
> > > 1. bug: ifconfig(8) forgets to document both `-tunneldomain' and
> > > `-mplslabel' in the first place, diff below fixes that.
> > >
> > >
> > > 2. bug: mpe(4) supports SIOCDELLABEL it but the driver manual's list
> > > does not list it as such; instead of adding it, remove the rather
> > > useless list completely (neither of the other two drivers lists ioctls
> > > as most manuals do not). diff below fixes that.
> > >
> >
> > I think this is bad. Driver specific ioctls should be documented. How else
> > will you know how to configure a device?
> Fair point, that'd drop the last reference to them; I thought of the
> netintro(4)'s INTERFACES when writing the diff but that section
> obviously doesn't cover driver specifics (doh!).
>
> New diff below which
> - documents SIOCDELLABEL
> - uses an IOCTL section losely adopted from bridge(4)
> - mentions the other two drivers as well
> - .Xr all three MPLS drivers among each other
>
> I briefly checked the ioctl handlers for both mpip(4)'s and mpe(4)'s
> which *do* and *do not* unset the label respectively, but their code is
> the same; I'd appreciate looks from someone knowledgable in MPLS.
>
> Hence this is only the first step, i.e. write the docs - then we can
> still improve in-tree possibly while fixing/improving the code.
>
> None of the MPLS drivers document any of the other MPLS specific ioctls
> and I don't want to throw everything into one big diff anyway.
>
> The ifconfig.8 bits should be fine already since they're simply
> documenting what ifconfig.c really does.
>
> Feedback? Objections OK?
>
Looks like a good start. On comment below.
> Index: share/man/man4/mpe.4
> ===================================================================
> RCS file: /cvs/src/share/man/man4/mpe.4,v
> retrieving revision 1.10
> diff -u -p -r1.10 mpe.4
> --- share/man/man4/mpe.4 12 Jan 2018 04:36:44 -0000 1.10
> +++ share/man/man4/mpe.4 15 Mar 2021 23:08:19 -0000
> @@ -39,9 +39,15 @@ configuration file for
> The interface itself can be configured with
> .Xr ifconfig 8 ;
> see its manual page for more information.
> -.Pp
> -.Nm
> -interfaces support the following unique ioctls:
> +.Sh IOCTLS
> +The following
> +.Nm ioctl 2
> +calls are specific to
> +.Nm ,
> +.Xr mpip 4
> +and
> +.Xr mpw 4
> +interfaces:
> .Bl -tag -width "SIOCSETLABEL" -offset 3n
> .It SIOCSETLABEL
> Encapsulate packets entering this interface in MPLS using
> @@ -49,17 +55,21 @@ the specified label.
> .It SIOCGETLABEL
> Report the label that packets entering this interface will be
> tagged with.
> +.It SIOCDELLABEL
> +Unset the MPLS label.
> .El
> .\"
> .Sh SEE ALSO
> .Xr sysctl 2 ,
> +.Xr mpip 4 ,
> +.Xr mpw 4 ,
> .Xr hostname.if 5 ,
> .Xr ifconfig 8 ,
> .Xr ldpd 8 ,
> .Xr netstart 8
> .\"
> .Sh HISTORY
> -The
> +The ,
Why did you add this ',' that looks strange to me.
> .Nm
> device first appeared in
> .Ox 4.4 .
> Index: share/man/man4/mpip.4
> ===================================================================
> RCS file: /cvs/src/share/man/man4/mpip.4,v
> retrieving revision 1.2
> diff -u -p -r1.2 mpip.4
> --- share/man/man4/mpip.4 11 Mar 2019 18:55:29 -0000 1.2
> +++ share/man/man4/mpip.4 15 Mar 2021 22:15:41 -0000
> @@ -60,6 +60,8 @@ Transport of Pseudowires:
> # ifconfig mpip5 -pwecw pwefat
> .Ed
> .Sh SEE ALSO
> +.Xr mpe 4 ,
> +.Xr mpw 4 ,
> .Xr hostname.if 5 ,
> .Xr ifconfig 8 ,
> .Xr ldpd 8 ,
> Index: share/man/man4/mpw.4
> ===================================================================
> RCS file: /cvs/src/share/man/man4/mpw.4,v
> retrieving revision 1.8
> diff -u -p -r1.8 mpw.4
> --- share/man/man4/mpw.4 3 Apr 2019 06:24:07 -0000 1.8
> +++ share/man/man4/mpw.4 15 Mar 2021 22:15:43 -0000
> @@ -85,6 +85,8 @@ using different identifiers for their pr
> # ifconfig bridge0 protected mpw12 1
> .Ed
> .Sh SEE ALSO
> +.Xr mpe 4 ,
> +.Xr mpip 4 ,
> .Xr hostname.if 5 ,
> .Xr ifconfig 8 ,
> .Xr ldpd 8 ,
> Index: sbin/ifconfig/ifconfig.8
> ===================================================================
> RCS file: /cvs/src/sbin/ifconfig/ifconfig.8,v
> retrieving revision 1.369
> diff -u -p -r1.369 ifconfig.8
> --- sbin/ifconfig/ifconfig.8 11 Mar 2021 21:07:16 -0000 1.369
> +++ sbin/ifconfig/ifconfig.8 13 Mar 2021 22:11:14 -0000
> @@ -1433,11 +1433,11 @@ is omitted, it is decreased by 1.
> .Bk -words
> .Nm ifconfig
> .Ar mpls-interface
> -.Op Cm mplslabel Ar mpls-label
> +.Op Oo Fl Oc Ns Cm mplslabel Ar mpls-label
> .Op Oo Fl Oc Ns Cm pwecw
> .Op Oo Fl Oc Ns Cm pwefat
> .Op Cm pweneighbor Ar mpls-label Ar neighbor
> -.Op Cm tunneldomain Ar rdomain
> +.Op Oo Fl Oc Ns Cm tunneldomain Ar rdomain
> .Ek
> .nr nS 0
> .Pp
> @@ -1455,12 +1455,16 @@ MPLS packets sent to this label on the l
> decapsulated for input.
> An MPLS label is a 20-bit number.
> Labels 0 to 15 inclusive are reserved labels and cannot be used.
> +.It Cm -mplslabel
> +Unset the local MPLS label.
> .It Cm tunneldomain Ar rdomain
> -Use the route domain
> +Use the routing domain
> .Ar rdomain
> for MPLS transit.
> The MPLS encapsulated traffic does not need to terminate in the same
> routing domain as the interface itself.
> +.It Cm -tunneldomain
> +Use the default routing domain 0 for MPLS transit.
> .El
> .Pp
> The following options are available for the
>
--
:wq Claudio
