sppp(4)/pppoe(4) dynamic address hack
Currently, sppp(4) interfaces use a destination address of 0.0.0.1 to
indicate that the destination address should be assigned by the peer:
inet 0.0.0.0 255.255.255.255 NONE \
pppoedev em0 authproto pap \
authname 'testcaller' authkey 'donttell' up
dest 0.0.0.1
As a side effect a route to 0.0.0.1 is added to the routing table.
Apparently, in 5.7 this still used to work with mulitple pppoe interfaces
which shared a routing table:
http://marc.info/?l=openbsd-misc&m=149386503001033&w=2
At present only one pppoe interface can be used in this way because
setting a dest address of 0.0.0.1 on additional interfaces now causes
an error: ifconfig: SIOCAIFADDR: File exists
This diff changes the way dynamic addresses are configured in sppp(4).
Instead of using magic IP addresses as wildcards, we allow userland to
control behavior via new ifconfig command flags. 'dynaddr' enables a
dynamic local address, and 'dyndest' enables a dynamic destination address.
To disable either of these again, prepend a minus (e.g. -dynaddr).
The above example becomes:
inet 0.0.0.0 255.255.255.255 NONE \
pppoedev em0 authproto pap \
authname 'testcaller' authkey 'donttell' \
dynaddr dyndest up
Tested on my ADSL line and it works as expected.
I cannot test it on multiple links until next week, but since no route
for 0.0.0.1 is in the routing table anymore, I expect this to work.
Index: sbin/ifconfig/ifconfig.8
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.8,v
retrieving revision 1.282
diff -u -p -r1.282 ifconfig.8
--- sbin/ifconfig/ifconfig.812 May 2017 15:11:02 - 1.282
+++ sbin/ifconfig/ifconfig.89 Jun 2017 16:02:23 -
@@ -1393,6 +1393,8 @@ Clear a previously set service name.
.Op Cm authkey Ar key
.Op Cm authname Ar name
.Op Cm authproto Ar proto
+.Op Oo Fl Oc Ns Cm dynaddr
+.Op Oo Fl Oc Ns Cm dyndest
.Op Oo Fl Oc Ns Cm peerflag Ar flag
.Op Cm peerkey Ar key
.Op Cm peername Ar name
@@ -1419,6 +1421,16 @@ The protocol name can be either
or
.Ql none .
In the latter case, authentication will be turned off.
+.It Cm dynaddr
+The local address will be assigned by the peer.
+.It Cm -dynaddr
+Disable dynamic assignment of the local address.
+This is the default.
+.It Cm dyndest
+The destination address will be assigned by the peer.
+.It Cm -dyndest
+Disable dynamic assignment of the destination address.
+This is the default.
.It Cm peerflag Ar flag
Set a specified PPP flag for the remote authenticator.
The flag name can be either
Index: sbin/ifconfig/ifconfig.c
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
retrieving revision 1.342
diff -u -p -r1.342 ifconfig.c
--- sbin/ifconfig/ifconfig.c5 Jun 2017 05:10:23 - 1.342
+++ sbin/ifconfig/ifconfig.c9 Jun 2017 15:57:40 -
@@ -266,6 +266,9 @@ voidsetseername(const char *, int);
void setseerkey(const char *, int);
void setseerflag(const char *, int);
void unsetseerflag(const char *, int);
+void sipcpinfo(struct sipcpreq *);
+void setspppdynaddr(const char *, int);
+void setspppdyndest(const char *, int);
void sppp_status(void);
void sppp_printproto(const char *, struct sauthreq *);
void setifpriority(const char *, int);
@@ -446,6 +449,10 @@ const struct cmd {
{ "peerkey",NEXTARG,0, setseerkey },
{ "peerflag", NEXTARG,0, setseerflag },
{ "-peerflag", NEXTARG,0, unsetseerflag },
+ { "dynaddr",1, 0, setspppdynaddr },
+ { "-dynaddr", 0, 0, setspppdynaddr },
+ { "dyndest",1, 0, setspppdyndest },
+ { "-dyndest", 0, 0, setspppdyndest },
{ "nwflag", NEXTARG,0, setifnwflag },
{ "-nwflag",NEXTARG,0, unsetifnwflag },
{ "flowsrc",NEXTARG,0, setpflow_sender },
@@ -4878,6 +4885,63 @@ unsetseerflag(const char *val, int d
}
void
+sipcpinfo(struct sipcpreq *req)
+{
+ bzero(req, sizeof(*req));
+
+ ifr.ifr_data = (caddr_t)req;
+ req->cmd = SPPPIOGIPCP;
+ if (ioctl(s, SIOCGSARAMS, &ifr) == -1)
+ err(1, "SIOCGSARAMS(SPPPIOGIPCP)");
+}
+
+void
+setspppdynaddr(const char *val, int d)
+{
+ struct sipcpreq scp;
+
+ sipcpinfo(&scp);
+
+ if (d == 1) {
+ if (scp.flags & SIPCP_MYADDR_DYN)
+ return;
+ scp.flags |= SIPCP_MYADDR_DYN;
+ } else {
+ if ((scp.flags & SIPCP_MYADDR_DYN) == 0)
+ return;
+ scp.flags &= ~SIPCP_MYADDR_DYN;
+ }
+
+ scp
Re: sppp(4)/pppoe(4) dynamic address hack
On Fri, Jun 09, 2017 at 06:33:46PM +0200, Stefan Sperling wrote:
> Currently, sppp(4) interfaces use a destination address of 0.0.0.1 to
> indicate that the destination address should be assigned by the peer:
>
> inet 0.0.0.0 255.255.255.255 NONE \
> pppoedev em0 authproto pap \
> authname 'testcaller' authkey 'donttell' up
> dest 0.0.0.1
>
> As a side effect a route to 0.0.0.1 is added to the routing table.
>
> Apparently, in 5.7 this still used to work with mulitple pppoe interfaces
> which shared a routing table:
> http://marc.info/?l=openbsd-misc&m=149386503001033&w=2
>
> At present only one pppoe interface can be used in this way because
> setting a dest address of 0.0.0.1 on additional interfaces now causes
> an error: ifconfig: SIOCAIFADDR: File exists
>
> This diff changes the way dynamic addresses are configured in sppp(4).
> Instead of using magic IP addresses as wildcards, we allow userland to
> control behavior via new ifconfig command flags. 'dynaddr' enables a
> dynamic local address, and 'dyndest' enables a dynamic destination address.
> To disable either of these again, prepend a minus (e.g. -dynaddr).
>
> The above example becomes:
>
> inet 0.0.0.0 255.255.255.255 NONE \
> pppoedev em0 authproto pap \
> authname 'testcaller' authkey 'donttell' \
>dynaddr dyndest up
>
hi.
why do you have to specify 0.0.0.0 *and* dynaddr?
jmc
> Tested on my ADSL line and it works as expected.
>
> I cannot test it on multiple links until next week, but since no route
> for 0.0.0.1 is in the routing table anymore, I expect this to work.
>
> Index: sbin/ifconfig/ifconfig.8
> ===
> RCS file: /cvs/src/sbin/ifconfig/ifconfig.8,v
> retrieving revision 1.282
> diff -u -p -r1.282 ifconfig.8
> --- sbin/ifconfig/ifconfig.8 12 May 2017 15:11:02 - 1.282
> +++ sbin/ifconfig/ifconfig.8 9 Jun 2017 16:02:23 -
> @@ -1393,6 +1393,8 @@ Clear a previously set service name.
> .Op Cm authkey Ar key
> .Op Cm authname Ar name
> .Op Cm authproto Ar proto
> +.Op Oo Fl Oc Ns Cm dynaddr
> +.Op Oo Fl Oc Ns Cm dyndest
> .Op Oo Fl Oc Ns Cm peerflag Ar flag
> .Op Cm peerkey Ar key
> .Op Cm peername Ar name
> @@ -1419,6 +1421,16 @@ The protocol name can be either
> or
> .Ql none .
> In the latter case, authentication will be turned off.
> +.It Cm dynaddr
> +The local address will be assigned by the peer.
> +.It Cm -dynaddr
> +Disable dynamic assignment of the local address.
> +This is the default.
> +.It Cm dyndest
> +The destination address will be assigned by the peer.
> +.It Cm -dyndest
> +Disable dynamic assignment of the destination address.
> +This is the default.
> .It Cm peerflag Ar flag
> Set a specified PPP flag for the remote authenticator.
> The flag name can be either
> Index: sbin/ifconfig/ifconfig.c
> ===
> RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
> retrieving revision 1.342
> diff -u -p -r1.342 ifconfig.c
> --- sbin/ifconfig/ifconfig.c 5 Jun 2017 05:10:23 - 1.342
> +++ sbin/ifconfig/ifconfig.c 9 Jun 2017 15:57:40 -
> @@ -266,6 +266,9 @@ void setseername(const char *, int);
> void setseerkey(const char *, int);
> void setseerflag(const char *, int);
> void unsetseerflag(const char *, int);
> +void sipcpinfo(struct sipcpreq *);
> +void setspppdynaddr(const char *, int);
> +void setspppdyndest(const char *, int);
> void sppp_status(void);
> void sppp_printproto(const char *, struct sauthreq *);
> void setifpriority(const char *, int);
> @@ -446,6 +449,10 @@ const struct cmd {
> { "peerkey",NEXTARG,0, setseerkey },
> { "peerflag", NEXTARG,0, setseerflag },
> { "-peerflag", NEXTARG,0, unsetseerflag },
> + { "dynaddr",1, 0, setspppdynaddr },
> + { "-dynaddr", 0, 0, setspppdynaddr },
> + { "dyndest",1, 0, setspppdyndest },
> + { "-dyndest", 0, 0, setspppdyndest },
> { "nwflag", NEXTARG,0, setifnwflag },
> { "-nwflag",NEXTARG,0, unsetifnwflag },
> { "flowsrc",NEXTARG,0, setpflow_sender },
> @@ -4878,6 +4885,63 @@ unsetseerflag(const char *val, int d
> }
>
> void
> +sipcpinfo(struct sipcpreq *req)
> +{
> + bzero(req, sizeof(*req));
> +
> + ifr.ifr_data = (caddr_t)req;
> + req->cmd = SPPPIOGIPCP;
> + if (ioctl(s, SIOCGSARAMS, &ifr) == -1)
> + err(1, "SIOCGSARAMS(SPPPIOGIPCP)");
> +}
> +
> +void
> +setspppdynaddr(const char *val, int d)
> +{
> + struct sipcpreq scp;
> +
> + sipcpinfo(&scp);
> +
> + if (d == 1) {
> + if (sc
Re: sppp(4)/pppoe(4) dynamic address hack
Stefan Sperling: > inet 0.0.0.0 255.255.255.255 NONE \ > pppoedev em0 authproto pap \ > authname 'testcaller' authkey 'donttell' \ >dynaddr dyndest up > > Tested on my ADSL line and it works as expected. The example in the pppoe(4) man page also includes !/sbin/route add default -ifp pppoe0 0.0.0.1 to specify the default route. What happens to that? -- Christian "naddy" Weisgerber na...@mips.inka.de
Re: sppp(4)/pppoe(4) dynamic address hack
On Fri, Jun 09, 2017 at 05:37:44PM +0100, Jason McIntyre wrote: > why do you have to specify 0.0.0.0 *and* dynaddr? If there is no address on the interface, the code in sppp_set_ip_addrs() loops over an empty interface address list and hence does nothing. IPCP will still negotiate an address but as a result of how spp_set_ip_addrs() works this address won't show up on the interface :-/ This behaviour is yet another silly implementation detail of sppp(4). It could probably be fixed but I would treat this as an unrelated problem. But I do understand how this can be confusing. I think the documentation could be made more clear by phrasing it as "updating an address" instead of "assigning an address". I'll send an updated patch soon. Regardless, you probably *do* want a dummy address. An address is needed in order to add a dummy default route which will be updated once the pppoe interface comes up (more details forthcoming in my reply to naddy's question).
Re: sppp(4)/pppoe(4) dynamic address hack
On Fri, Jun 09, 2017 at 10:36:45PM +0200, Christian Weisgerber wrote: > The example in the pppoe(4) man page also includes > > !/sbin/route add default -ifp pppoe0 0.0.0.1 > > to specify the default route. What happens to that? Good question. It raises an interesting point: Setting a dest address is still required to use a default route. Luckily, sppp(4) does not care which address we use. All it cases about is a default route with a matching ifindex. If it finds such a route, the destination address is updated. So what happens is that you can now use *any* unique address as 'dest'. The mechanism simply becomes more flexible. For example, you could have two pppoe interfaces like this (note that -mpath is needed in this case): $ cat /etc/hostname.pppoe0 inet 0.0.0.0 255.255.255.255 NONE \ pppoedev em0 authproto pap \ authname 'testcaller' authkey 'donttell' up dest 0.0.0.1 dynaddr dyndest !/sbin/route -T1 add default -mpath -ifp pppoe0 0.0.0.1 $ cat /etc/hostname.pppoe1 inet 0.0.0.0 255.255.255.255 NONE \ pppoedev em1 authproto pap \ authname 'testcaller2' authkey 'donttell2' up dest 0.0.0.2 dynaddr dyndest !/sbin/route -T1 add default -mpath -ifp pppoe1 0.0.0.2 So your initial routing table looks like: DestinationGatewayFlags Refs Use Mtu Prio Iface default0.0.0.1GSP01 - 8 pppoe0 default0.0.0.2GSP00 - 8 pppoe1 0.0.0.1defaultHh 12 - 8 pppoe0 0.0.0.2defaultHh 11 - 8 pppoe1 127.0.0.1 127.0.0.1 UHl00 32768 1 lo0 And once the pppoe0 interface connects, sppp(4) modifies the appropriate default route. It is marked UP and the new default gateway is entered: DestinationGatewayFlags Refs Use Mtu Prio Iface default100.148.133.201UGSP 0 11 - 8 pppoe0 default0.0.0.2GSP00 - 8 pppoe1 0.0.0.2defaultHh 11 - 8 pppoe1 127.0.0.1 127.0.0.1 UHl00 32768 1 lo0 212.60.251.248 212.60.251.248 UHl0 16 - 1 pppoe0 213.148.133.201212.60.251.248 UHh11 - 8 pppoe0 # These 0.0.0.x addresses are ugly, why not use regular private IPs instead? This does the same thing: $ cat /etc/hostname.pppoe0 inet 0.0.0.0 255.255.255.255 NONE \ pppoedev em0 authproto pap \ authname 'testcaller' authkey 'donttell' up dest 192.168.42.1 dynaddr dyndest !/sbin/route -T1 add default -mpath -ifp pppoe0 192.168.42.1 $ cat /etc/hostname.pppoe1 inet 0.0.0.0 255.255.255.255 NONE \ pppoedev em1 authproto pap \ authname 'testcaller2' authkey 'donttell2' up dest 10.42.42.1 dynaddr dyndest !/sbin/route -T1 add default -mpath -ifp pppoe1 10.42.42.1 DestinationGatewayFlags Refs Use Mtu Prio Iface default192.168.42.1 GSP00 - 8 pppoe0 default10.42.42.1 GSP00 - 8 pppoe1 10.42.42.1 defaultHh 11 - 8 pppoe1 192.168.42.1 defaultHh 11 - 8 pppoe0 127.0.0.1 127.0.0.1 UHl00 32768 1 lo0 After connecting: DestinationGatewayFlags Refs Use Mtu Prio Iface default100.148.133.201GSP00 - 8 pppoe0 default10.42.42.1 GSP00 - 8 pppoe1 10.42.42.1 defaultHh 11 - 8 pppoe1 192.168.42.1 defaultHh 11 - 8 pppoe0 127.0.0.1 127.0.0.1 UHl00 32768 1 lo0
Re: sppp(4)/pppoe(4) dynamic address hack
On Fri, Jun 09, 2017 at 06:33:46PM +0200, Stefan Sperling wrote:
> This diff changes the way dynamic addresses are configured in sppp(4).
I was asked in private whether we could avoid a flag day which makes
it inconvenient to upgrade remote boxes only reachable over pppoe(4).
I see no harm in keeping the old magic working for now.
The new mechanism still works, and the goal of allowing more than one pppoe
instance per routing table does not conflict with temporary backwards compat.
So we would release 6.2 with support for both ways, and remove the old way
in 6.3 or later. This gives people some time for the transition.
This diff also contains doc updates requested by jmc@.
Index: sbin/ifconfig/ifconfig.8
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.8,v
retrieving revision 1.282
diff -u -p -r1.282 ifconfig.8
--- sbin/ifconfig/ifconfig.812 May 2017 15:11:02 - 1.282
+++ sbin/ifconfig/ifconfig.810 Jun 2017 07:08:01 -
@@ -1393,6 +1393,8 @@ Clear a previously set service name.
.Op Cm authkey Ar key
.Op Cm authname Ar name
.Op Cm authproto Ar proto
+.Op Oo Fl Oc Ns Cm dynaddr
+.Op Oo Fl Oc Ns Cm dyndest
.Op Oo Fl Oc Ns Cm peerflag Ar flag
.Op Cm peerkey Ar key
.Op Cm peername Ar name
@@ -1419,6 +1421,21 @@ The protocol name can be either
or
.Ql none .
In the latter case, authentication will be turned off.
+.It Cm dynaddr
+The local address will be changed to an address suggested by the peer.
+A deprecated way of achieving the same effect is setting the local address
+to 0.0.0.0.
+.It Cm -dynaddr
+Disable dynamic updates of the local address.
+This is the default.
+.It Cm dyndest
+The destination address will be changed to an address suggested by the peer.
+A deprecated way of achieving the same effect is setting the destination
+address to 0.0.0.1 (multiple SPPP interfaces configured with this destination
+address cannot share a routing table).
+.It Cm -dyndest
+Disable dynamic updates of the destination address.
+This is the default.
.It Cm peerflag Ar flag
Set a specified PPP flag for the remote authenticator.
The flag name can be either
Index: sbin/ifconfig/ifconfig.c
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
retrieving revision 1.342
diff -u -p -r1.342 ifconfig.c
--- sbin/ifconfig/ifconfig.c5 Jun 2017 05:10:23 - 1.342
+++ sbin/ifconfig/ifconfig.c9 Jun 2017 15:57:40 -
@@ -266,6 +266,9 @@ voidsetseername(const char *, int);
void setseerkey(const char *, int);
void setseerflag(const char *, int);
void unsetseerflag(const char *, int);
+void sipcpinfo(struct sipcpreq *);
+void setspppdynaddr(const char *, int);
+void setspppdyndest(const char *, int);
void sppp_status(void);
void sppp_printproto(const char *, struct sauthreq *);
void setifpriority(const char *, int);
@@ -446,6 +449,10 @@ const struct cmd {
{ "peerkey",NEXTARG,0, setseerkey },
{ "peerflag", NEXTARG,0, setseerflag },
{ "-peerflag", NEXTARG,0, unsetseerflag },
+ { "dynaddr",1, 0, setspppdynaddr },
+ { "-dynaddr", 0, 0, setspppdynaddr },
+ { "dyndest",1, 0, setspppdyndest },
+ { "-dyndest", 0, 0, setspppdyndest },
{ "nwflag", NEXTARG,0, setifnwflag },
{ "-nwflag",NEXTARG,0, unsetifnwflag },
{ "flowsrc",NEXTARG,0, setpflow_sender },
@@ -4878,6 +4885,63 @@ unsetseerflag(const char *val, int d
}
void
+sipcpinfo(struct sipcpreq *req)
+{
+ bzero(req, sizeof(*req));
+
+ ifr.ifr_data = (caddr_t)req;
+ req->cmd = SPPPIOGIPCP;
+ if (ioctl(s, SIOCGSARAMS, &ifr) == -1)
+ err(1, "SIOCGSARAMS(SPPPIOGIPCP)");
+}
+
+void
+setspppdynaddr(const char *val, int d)
+{
+ struct sipcpreq scp;
+
+ sipcpinfo(&scp);
+
+ if (d == 1) {
+ if (scp.flags & SIPCP_MYADDR_DYN)
+ return;
+ scp.flags |= SIPCP_MYADDR_DYN;
+ } else {
+ if ((scp.flags & SIPCP_MYADDR_DYN) == 0)
+ return;
+ scp.flags &= ~SIPCP_MYADDR_DYN;
+ }
+
+ scp.cmd = SPPPIOSIPCP;
+ if (ioctl(s, SIOCSSARAMS, &ifr) == -1)
+ err(1, "SIOCSSARAMS(SPPPIOSIPCP)");
+}
+
+
+void
+setspppdyndest(const char *val, int d)
+{
+ struct sipcpreq scp;
+
+ sipcpinfo(&scp);
+
+ if (d == 1) {
+ if (scp.flags & SIPCP_HISADDR_DYN)
+ return;
+ scp.flags |= SIPCP_HISADDR_DYN;
+ } else {
+ if ((scp.flags & SIPCP_HISADDR_DYN) == 0)
+ ret
Re: sppp(4)/pppoe(4) dynamic address hack
On Sat, Jun 10, 2017 at 09:23:45AM +0200, Stefan Sperling wrote: > On Fri, Jun 09, 2017 at 06:33:46PM +0200, Stefan Sperling wrote: > > This diff changes the way dynamic addresses are configured in sppp(4). > > I was asked in private whether we could avoid a flag day which makes > it inconvenient to upgrade remote boxes only reachable over pppoe(4). > > I see no harm in keeping the old magic working for now. > The new mechanism still works, and the goal of allowing more than one pppoe > instance per routing table does not conflict with temporary backwards compat. > So we would release 6.2 with support for both ways, and remove the old way > in 6.3 or later. This gives people some time for the transition. > > This diff also contains doc updates requested by jmc@. > thanks. the ifconfig.8 bits look good, but i think you should also change the examples given in pppoe(4) to show the updated syntax, to prepare folks. actually i think it would be better to discuss the deprecated stuff in pppoe(4), rather than adding it to ifconfig(8). jmc > Index: sbin/ifconfig/ifconfig.8 > === > RCS file: /cvs/src/sbin/ifconfig/ifconfig.8,v > retrieving revision 1.282 > diff -u -p -r1.282 ifconfig.8 > --- sbin/ifconfig/ifconfig.8 12 May 2017 15:11:02 - 1.282 > +++ sbin/ifconfig/ifconfig.8 10 Jun 2017 07:08:01 - > @@ -1393,6 +1393,8 @@ Clear a previously set service name. > .Op Cm authkey Ar key > .Op Cm authname Ar name > .Op Cm authproto Ar proto > +.Op Oo Fl Oc Ns Cm dynaddr > +.Op Oo Fl Oc Ns Cm dyndest > .Op Oo Fl Oc Ns Cm peerflag Ar flag > .Op Cm peerkey Ar key > .Op Cm peername Ar name > @@ -1419,6 +1421,21 @@ The protocol name can be either > or > .Ql none . > In the latter case, authentication will be turned off. > +.It Cm dynaddr > +The local address will be changed to an address suggested by the peer. > +A deprecated way of achieving the same effect is setting the local address > +to 0.0.0.0. > +.It Cm -dynaddr > +Disable dynamic updates of the local address. > +This is the default. > +.It Cm dyndest > +The destination address will be changed to an address suggested by the peer. > +A deprecated way of achieving the same effect is setting the destination > +address to 0.0.0.1 (multiple SPPP interfaces configured with this destination > +address cannot share a routing table). > +.It Cm -dyndest > +Disable dynamic updates of the destination address. > +This is the default. > .It Cm peerflag Ar flag > Set a specified PPP flag for the remote authenticator. > The flag name can be either
Re: sppp(4)/pppoe(4) dynamic address hack
On Sat, Jun 10, 2017 at 08:35:13AM +0100, Jason McIntyre wrote:
> thanks. the ifconfig.8 bits look good, but i think you should also
> change the examples given in pppoe(4) to show the updated syntax, to
> prepare folks.
>
> actually i think it would be better to discuss the deprecated stuff
> in pppoe(4), rather than adding it to ifconfig(8).
Sure. I think it is also worth pointing out that all this is IPv4 only.
Also document sppp(4) behaviour with respect to the default route.
Index: sbin/ifconfig/ifconfig.8
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.8,v
retrieving revision 1.282
diff -u -p -r1.282 ifconfig.8
--- sbin/ifconfig/ifconfig.812 May 2017 15:11:02 - 1.282
+++ sbin/ifconfig/ifconfig.810 Jun 2017 08:17:32 -
@@ -1393,6 +1393,8 @@ Clear a previously set service name.
.Op Cm authkey Ar key
.Op Cm authname Ar name
.Op Cm authproto Ar proto
+.Op Oo Fl Oc Ns Cm dynaddr
+.Op Oo Fl Oc Ns Cm dyndest
.Op Oo Fl Oc Ns Cm peerflag Ar flag
.Op Cm peerkey Ar key
.Op Cm peername Ar name
@@ -1419,6 +1421,19 @@ The protocol name can be either
or
.Ql none .
In the latter case, authentication will be turned off.
+.It Cm dynaddr
+The local IPv4 address will be changed to an address suggested by the peer.
+.It Cm -dynaddr
+Disable dynamic updates of the local IPv4 address.
+This is the default.
+.It Cm dyndest
+The destination IPv4 address will be changed to an address suggested
+by the peer.
+If a default route which uses this interface exists, the gateway will be
+changed to the suggested address as well.
+.It Cm -dyndest
+Disable dynamic updates of the destination IPv4 address.
+This is the default.
.It Cm peerflag Ar flag
Set a specified PPP flag for the remote authenticator.
The flag name can be either
Index: sbin/ifconfig/ifconfig.c
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
retrieving revision 1.342
diff -u -p -r1.342 ifconfig.c
--- sbin/ifconfig/ifconfig.c5 Jun 2017 05:10:23 - 1.342
+++ sbin/ifconfig/ifconfig.c9 Jun 2017 15:57:40 -
@@ -266,6 +266,9 @@ voidsetseername(const char *, int);
void setseerkey(const char *, int);
void setseerflag(const char *, int);
void unsetseerflag(const char *, int);
+void sipcpinfo(struct sipcpreq *);
+void setspppdynaddr(const char *, int);
+void setspppdyndest(const char *, int);
void sppp_status(void);
void sppp_printproto(const char *, struct sauthreq *);
void setifpriority(const char *, int);
@@ -446,6 +449,10 @@ const struct cmd {
{ "peerkey",NEXTARG,0, setseerkey },
{ "peerflag", NEXTARG,0, setseerflag },
{ "-peerflag", NEXTARG,0, unsetseerflag },
+ { "dynaddr",1, 0, setspppdynaddr },
+ { "-dynaddr", 0, 0, setspppdynaddr },
+ { "dyndest",1, 0, setspppdyndest },
+ { "-dyndest", 0, 0, setspppdyndest },
{ "nwflag", NEXTARG,0, setifnwflag },
{ "-nwflag",NEXTARG,0, unsetifnwflag },
{ "flowsrc",NEXTARG,0, setpflow_sender },
@@ -4878,6 +4885,63 @@ unsetseerflag(const char *val, int d
}
void
+sipcpinfo(struct sipcpreq *req)
+{
+ bzero(req, sizeof(*req));
+
+ ifr.ifr_data = (caddr_t)req;
+ req->cmd = SPPPIOGIPCP;
+ if (ioctl(s, SIOCGSARAMS, &ifr) == -1)
+ err(1, "SIOCGSARAMS(SPPPIOGIPCP)");
+}
+
+void
+setspppdynaddr(const char *val, int d)
+{
+ struct sipcpreq scp;
+
+ sipcpinfo(&scp);
+
+ if (d == 1) {
+ if (scp.flags & SIPCP_MYADDR_DYN)
+ return;
+ scp.flags |= SIPCP_MYADDR_DYN;
+ } else {
+ if ((scp.flags & SIPCP_MYADDR_DYN) == 0)
+ return;
+ scp.flags &= ~SIPCP_MYADDR_DYN;
+ }
+
+ scp.cmd = SPPPIOSIPCP;
+ if (ioctl(s, SIOCSSARAMS, &ifr) == -1)
+ err(1, "SIOCSSARAMS(SPPPIOSIPCP)");
+}
+
+
+void
+setspppdyndest(const char *val, int d)
+{
+ struct sipcpreq scp;
+
+ sipcpinfo(&scp);
+
+ if (d == 1) {
+ if (scp.flags & SIPCP_HISADDR_DYN)
+ return;
+ scp.flags |= SIPCP_HISADDR_DYN;
+ } else {
+ if ((scp.flags & SIPCP_HISADDR_DYN) == 0)
+ return;
+ scp.flags &= ~SIPCP_HISADDR_DYN;
+ }
+
+ scp.cmd = SPPPIOSIPCP;
+ if (ioctl(s, SIOCSSARAMS, &ifr) == -1)
+ err(1, "SIOCSSARAMS(SPPPIOSIPCP)");
+}
+
+
+void
sppp_printproto(const char *name, struct sauthreq *auth)
{
if (auth->proto == 0)
@@ -4905,6 +4969,7 @@ sppp_status(
Re: sppp(4)/pppoe(4) dynamic address hack
On Sat, Jun 10, 2017 at 10:22:22AM +0200, Stefan Sperling wrote: > On Sat, Jun 10, 2017 at 08:35:13AM +0100, Jason McIntyre wrote: > > thanks. the ifconfig.8 bits look good, but i think you should also > > change the examples given in pppoe(4) to show the updated syntax, to > > prepare folks. > > > > actually i think it would be better to discuss the deprecated stuff > > in pppoe(4), rather than adding it to ifconfig(8). > > Sure. I think it is also worth pointing out that all this is IPv4 only. > Also document sppp(4) behaviour with respect to the default route. > thanks, i'm fine with this. jmc > Index: sbin/ifconfig/ifconfig.8 > === > RCS file: /cvs/src/sbin/ifconfig/ifconfig.8,v > retrieving revision 1.282 > diff -u -p -r1.282 ifconfig.8 > --- sbin/ifconfig/ifconfig.8 12 May 2017 15:11:02 - 1.282 > +++ sbin/ifconfig/ifconfig.8 10 Jun 2017 08:17:32 - > @@ -1393,6 +1393,8 @@ Clear a previously set service name. > .Op Cm authkey Ar key > .Op Cm authname Ar name > .Op Cm authproto Ar proto > +.Op Oo Fl Oc Ns Cm dynaddr > +.Op Oo Fl Oc Ns Cm dyndest > .Op Oo Fl Oc Ns Cm peerflag Ar flag > .Op Cm peerkey Ar key > .Op Cm peername Ar name > @@ -1419,6 +1421,19 @@ The protocol name can be either > or > .Ql none . > In the latter case, authentication will be turned off. > +.It Cm dynaddr > +The local IPv4 address will be changed to an address suggested by the peer. > +.It Cm -dynaddr > +Disable dynamic updates of the local IPv4 address. > +This is the default. > +.It Cm dyndest > +The destination IPv4 address will be changed to an address suggested > +by the peer. > +If a default route which uses this interface exists, the gateway will be > +changed to the suggested address as well. > +.It Cm -dyndest > +Disable dynamic updates of the destination IPv4 address. > +This is the default. > .It Cm peerflag Ar flag > Set a specified PPP flag for the remote authenticator. > The flag name can be either > Index: share/man/man4/pppoe.4 > === > RCS file: /cvs/src/share/man/man4/pppoe.4,v > retrieving revision 1.33 > diff -u -p -r1.33 pppoe.4 > --- share/man/man4/pppoe.422 Mar 2017 21:37:24 - 1.33 > +++ share/man/man4/pppoe.410 Jun 2017 08:11:35 - > @@ -99,9 +99,10 @@ A typical file looks like this: > inet 0.0.0.0 255.255.255.255 NONE \e > pppoedev em0 authproto pap \e > authname 'testcaller' authkey 'donttell' up > -dest 0.0.0.1 > +dest 10.1.1.1 > +dynaddr dyndest > inet6 eui64 > -!/sbin/route add default -ifp pppoe0 0.0.0.1 > +!/sbin/route add default -ifp pppoe0 10.1.1.1 > !/sbin/route add -inet6 default -ifp pppoe0 fe80::%pppoe0 > .Ed > .Pp > @@ -111,19 +112,28 @@ The physical interface must also be mark > # echo "up" > /etc/hostname.em0 > .Ed > .Pp > +In the above example, 10.1.1.1 is an otherwise unused IP address which > +serves as a placeholder for dynamic address configuration. > Since this is a PPP interface, the addresses assigned to the interface > may change during PPP negotiation. > There is no fine grained control available for deciding > which addresses are acceptable and which are not. > -For the local side and the remote address there is exactly one choice: > -hard coded address or wildcard. > -If a real address is assigned to one side of the connection, > -PPP negotiation will only agree to exactly this address. > -If one side is wildcarded, > -every address suggested by the peer will be accepted. > +For the local address and the remote address each there is exactly one > choice: > +hard coded address or dynamic address. > .Pp > +By default, PPP negotiation will only agree to exactly the IPv4 addresses > +which are configured on the interface. > +If dynamic address configuration is enabled for the local address (dynaddr) > +and/or remote address (dyndest), any address suggested by the peer will > +be accepted and overrides the addresses configured on the interface and > +the corresponding default route. > +.Pp > +A deprecated way of enabling dynamic addresses is by using wildcard > addresses. > To wildcard the local address set it to 0.0.0.0; to wildcard the remote > -address set it to 0.0.0.1. > +address set it to 0.0.0.1 (multiple > +.Nm > +interfaces configured with this wildcard destination address cannot share > +a routing table). > .Sh KERNEL OPTIONS > .Nm > does not interfere with other PPPoE implementations
Re: sppp(4)/pppoe(4) dynamic address hack
On 10/06/17(Sat) 08:35, Stefan Sperling wrote: > On Fri, Jun 09, 2017 at 05:37:44PM +0100, Jason McIntyre wrote: > > why do you have to specify 0.0.0.0 *and* dynaddr? > [...] > Regardless, you probably *do* want a dummy address. An address is needed > in order to add a dummy default route which will be updated once the pppoe > interface comes up (more details forthcoming in my reply to naddy's question). So why not fix the problem? How many years are we going to continue to add workaround for p2p interfaces? I send a diff last year to start doing that: http://marc.info/?l=openbsd-tech&m=146539593821612&w=2 Since nobody jumped in the boat and I already have ETOOMANYTHING to do I dropped the diff.
Re: sppp(4)/pppoe(4) dynamic address hack
On Sat, Jun 10, 2017 at 11:47:27AM +0200, Martin Pieuchot wrote: > On 10/06/17(Sat) 08:35, Stefan Sperling wrote: > > On Fri, Jun 09, 2017 at 05:37:44PM +0100, Jason McIntyre wrote: > > > why do you have to specify 0.0.0.0 *and* dynaddr? > > [...] > > Regardless, you probably *do* want a dummy address. An address is needed > > in order to add a dummy default route which will be updated once the pppoe > > interface comes up (more details forthcoming in my reply to naddy's > > question). > > So why not fix the problem? How many years are we going to continue > to add workaround for p2p interfaces? > > I send a diff last year to start doing that: > http://marc.info/?l=openbsd-tech&m=146539593821612&w=2 > > Since nobody jumped in the boat and I already have ETOOMANYTHING to do > I dropped the diff. > That's cool. But I don't think it conflicts with the change I'm trying to make. Does it? My diff changes how userland enables/disables dynamic address configuration, and removes the need to configure a specific magic address. sppp(4) is messing with address and routes in the same way, regardless.
Re: sppp(4)/pppoe(4) dynamic address hack
On 10/06/17(Sat) 11:55, Stefan Sperling wrote: > On Sat, Jun 10, 2017 at 11:47:27AM +0200, Martin Pieuchot wrote: > > On 10/06/17(Sat) 08:35, Stefan Sperling wrote: > > > On Fri, Jun 09, 2017 at 05:37:44PM +0100, Jason McIntyre wrote: > > > > why do you have to specify 0.0.0.0 *and* dynaddr? > > > [...] > > > Regardless, you probably *do* want a dummy address. An address is needed > > > in order to add a dummy default route which will be updated once the pppoe > > > interface comes up (more details forthcoming in my reply to naddy's > > > question). > > > > So why not fix the problem? How many years are we going to continue > > to add workaround for p2p interfaces? > > > > I send a diff last year to start doing that: > > http://marc.info/?l=openbsd-tech&m=146539593821612&w=2 > > > > Since nobody jumped in the boat and I already have ETOOMANYTHING to do > > I dropped the diff. > > > > That's cool. > But I don't think it conflicts with the change I'm trying to make. > Does it? No it does not, it makes it obsolete because it fixes the problem. Currently there are implicit 'dynaddr' and 'dyndest' arguments. They are just broken. If there's a real need for such arguments, because somebody use pppoe(4) with static IP/route then why not. But that's not what you said. What you said is that the current hack of 0.0.0.0+0.0.0.1 is broken. I'm not fan of your approach because it makes pppoe(4) special, does umb(4) will need 'dynaddr' and 'dyntest' too? Now the 0.0.0.1 hack is broken since 5.8, why not just fix it? EEXIST means the routing table already found an entry with this destination. This is due to the fact that a host route with destination 0.0.0.1 and gateway 0.0.0.0 already exists. Theses values don't matter and I don't see anything in if_spppsubr.c that check for the gateway being 0.0.0.1 So IMHO if what you want is fix the EEXIST, I'd do that. If what you want is fix the crappy config of pppoe(4)/umb(4)/any p2p interface, you have my diff. If what you want is to *not have* dynamic configuration on your pppoe(4) then your diff might make sense.
Re: sppp(4)/pppoe(4) dynamic address hack
On Sat, Jun 10, 2017 at 04:14:16PM +0200, Martin Pieuchot wrote: > If there's a real need for such arguments, because somebody use pppoe(4) > with static IP/route then why not. But that's not what you said. What > you said is that the current hack of 0.0.0.0+0.0.0.1 is broken. Static IP/route setups with pppoe(4) do exist. Just buy business DSL. The problem I want to fix is allowing more than one dynamic connection. Static setups work either way, with or without my fix. In either case, just configure your real addresses upfront to get a fully static setup. > I'm not fan of your approach because it makes pppoe(4) special, does > umb(4) will need 'dynaddr' and 'dyntest' too? I consider this a quirk in sppp(4), not pppoe or p2p interfaces in general. sppp(4) is using these DYN flags which we're toggling. The new ifconfig commands are documented in ifconfig(8)'s 'SPPP' section for that reason. umb(4) does not use sppp(4). In my mind, making some IP addresses act as magic toggles for internal sppp(4) state flags is a worse hack than mine. The old hack even has an XXX comment saying it should be removed, added by its author. > So IMHO if what you want is fix the EEXIST, I'd do that. I see where you are coming from. Fixing the routing table as you suggest would make the old hack work again and also bring other benefits. You were saying your old diff was "the first of 14 steps." That project seems to be substantially more involved than my proposed patch, which took me just a few hours. It does not sound like a project I could start working on now (I am not even done reading xhci.c, let alone actually hacking on it). I have proposed an easy fix which works and which does not preclude future work being done on the routing table. But I won't insist. I don't expect leaving sppp(4) in a broken state will suddenly motivate anyone to do substantial work on the routing table, though. My guess is that sppp(4) will just stay broken for multiple connections.
Re: sppp(4)/pppoe(4) dynamic address hack
On 10/06/17(Sat) 17:45, Stefan Sperling wrote:
> On Sat, Jun 10, 2017 at 04:14:16PM +0200, Martin Pieuchot wrote:
> > I'm not fan of your approach because it makes pppoe(4) special, does
> > umb(4) will need 'dynaddr' and 'dyntest' too?
>
> I consider this a quirk in sppp(4), not pppoe or p2p interfaces in general.
> sppp(4) is using these DYN flags which we're toggling. The new ifconfig
> commands are documented in ifconfig(8)'s 'SPPP' section for that reason.
> umb(4) does not use sppp(4).
>
> In my mind, making some IP addresses act as magic toggles for internal
> sppp(4) state flags is a worse hack than mine. The old hack even has
> an XXX comment saying it should be removed, added by its author.
I agree your hack is better but it's still a hack, limited to sppp(4).
> > So IMHO if what you want is fix the EEXIST, I'd do that.
>
> I see where you are coming from. Fixing the routing table as you suggest
> would make the old hack work again and also bring other benefits.
There's no need to fix the routing table, we could something like:
- if (hisaddr == 1) {
+ if (hisaddr < 10) {
Now I think you have a good point that using a flag is better than a
magic address. But I think the ifconfig(8) interface should be simpler.
What about reusing the 'autoconf' toggle? Could we say:
# ifconfig pppoe0 inet autoconf
Instead of introducing 'dynaddr' and 'dyndest'? Would it be a problem
if somebody wants to run dhclient(8) on top of pppoe(4)?
Re: sppp(4)/pppoe(4) dynamic address hack
On Sat, Jun 10, 2017 at 06:14:02PM +0200, Martin Pieuchot wrote:
> There's no need to fix the routing table, we could something like:
>
> - if (hisaddr == 1) {
> + if (hisaddr < 10) {
>
> Now I think you have a good point that using a flag is better than a
> magic address. But I think the ifconfig(8) interface should be simpler.
>
> What about reusing the 'autoconf' toggle? Could we say:
>
> # ifconfig pppoe0 inet autoconf
>
> Instead of introducing 'dynaddr' and 'dyndest'?
'autoconf' is already used in a context where no addresses are configured
yet (apart from the mandatory link-local ones, which don't exist as such
in IPv4). I would rather not overload it.
I agree that adding new ifconfig(8) commands may not be the ideal solution.
If the routing table were fixed eventually, such commands may become obsolete.
Here's a shorter diff based on your suggestion above, which bypasses the
tedious ifconfig(8) design decisions but still allows us to improve the
current situation.
With this, we keep the magic address hack (even though we don't really
like it), allow multiple dynamic pppoe(4) in one box (255 should be
enough for anyone), document our current hack properly, and don't
require existing pppoe(4) setups to change their configuration after 6.2.
Is this an acceptable short term solution?
Index: share/man/man4/pppoe.4
===
RCS file: /cvs/src/share/man/man4/pppoe.4,v
retrieving revision 1.33
diff -u -p -r1.33 pppoe.4
--- share/man/man4/pppoe.4 22 Mar 2017 21:37:24 - 1.33
+++ share/man/man4/pppoe.4 12 Jun 2017 13:20:51 -
@@ -113,17 +113,19 @@ The physical interface must also be mark
.Pp
Since this is a PPP interface, the addresses assigned to the interface
may change during PPP negotiation.
-There is no fine grained control available for deciding
-which addresses are acceptable and which are not.
-For the local side and the remote address there is exactly one choice:
-hard coded address or wildcard.
-If a real address is assigned to one side of the connection,
-PPP negotiation will only agree to exactly this address.
-If one side is wildcarded,
-every address suggested by the peer will be accepted.
+In the above example, 0.0.0.0 and 0.0.0.1 serve as placeholders for
+dynamic address configuration.
.Pp
-To wildcard the local address set it to 0.0.0.0; to wildcard the remote
-address set it to 0.0.0.1.
+If the local address is set to wildcard address 0.0.0.0, it will
+be changed to an address suggested by the peer.
+.Pp
+If the destination address is set to a wildcard address in the range
+from 0.0.0.1 to 0.0.0.255, it will be changed to an address suggested
+by the peer, and if a default route which uses this interface exists
+the gateway will be changed to the suggested address as well.
+.Pp
+Otherwise, PPP negotiation will only agree to exactly the IPv4 addresses
+which are configured on the interface.
.Sh KERNEL OPTIONS
.Nm
does not interfere with other PPPoE implementations
@@ -254,3 +256,8 @@ The presence of a
.Xr mygate 5
file will interfere with the routing table.
Make sure this file is either empty or does not exist.
+.Pp
+Two
+.Nm
+interfaces configured with the same wildcard destination address
+cannot share a routing table.
Index: sys/net/if_spppsubr.c
===
RCS file: /cvs/src/sys/net/if_spppsubr.c,v
retrieving revision 1.164
diff -u -p -r1.164 if_spppsubr.c
--- sys/net/if_spppsubr.c 30 May 2017 07:50:37 - 1.164
+++ sys/net/if_spppsubr.c 12 Jun 2017 13:10:20 -
@@ -2632,7 +2632,7 @@ sppp_ipcp_tls(struct sppp *sp)
sp->ipcp.flags |= IPCP_MYADDR_DYN;
sp->ipcp.opts |= (1 << IPCP_OPT_ADDRESS);
}
- if (hisaddr == 1) {
+ if (hisaddr >= 1 && hisaddr <= 255) {
/*
* XXX - remove this hack!
* remote has no valid address, we need to get one assigned.
Re: sppp(4)/pppoe(4) dynamic address hack
> > What about reusing the 'autoconf' toggle? Could we say: > > > > # ifconfig pppoe0 inet autoconf > > > > Instead of introducing 'dynaddr' and 'dyndest'? > > 'autoconf' is already used in a context where no addresses are configured > yet (apart from the mandatory link-local ones, which don't exist as such > in IPv4). I would rather not overload it. I'd go even further than that -- the autoconf keyword is an unfortunately named option which only applies to IPV6. stefan is right - don't overload it.
Re: sppp(4)/pppoe(4) dynamic address hack
On 2017/06/12 15:46, Stefan Sperling wrote:
> On Sat, Jun 10, 2017 at 06:14:02PM +0200, Martin Pieuchot wrote:
> > There's no need to fix the routing table, we could something like:
> >
> > - if (hisaddr == 1) {
> > + if (hisaddr < 10) {
> >
> > Now I think you have a good point that using a flag is better than a
> > magic address. But I think the ifconfig(8) interface should be simpler.
> >
> > What about reusing the 'autoconf' toggle? Could we say:
> >
> > # ifconfig pppoe0 inet autoconf
> >
> > Instead of introducing 'dynaddr' and 'dyndest'?
>
> 'autoconf' is already used in a context where no addresses are configured
> yet (apart from the mandatory link-local ones, which don't exist as such
> in IPv4). I would rather not overload it.
Also, to deal with different flavours of broken PPP implementations,
addr and dest need to be togglable to 'dynamic' separately.
> I agree that adding new ifconfig(8) commands may not be the ideal solution.
> If the routing table were fixed eventually, such commands may become obsolete.
>
> Here's a shorter diff based on your suggestion above, which bypasses the
> tedious ifconfig(8) design decisions but still allows us to improve the
> current situation.
>
> With this, we keep the magic address hack (even though we don't really
> like it), allow multiple dynamic pppoe(4) in one box (255 should be
> enough for anyone), document our current hack properly, and don't
> require existing pppoe(4) setups to change their configuration after 6.2.
>
> Is this an acceptable short term solution?
This seems the "least worst" option to me. (It's still racy, the
connection could come up and change the dest before the route is
added, so route addition would fail as we now require a "correct"
address on the p-p route addition, but that's not new). ok with me.
> Index: share/man/man4/pppoe.4
> ===
> RCS file: /cvs/src/share/man/man4/pppoe.4,v
> retrieving revision 1.33
> diff -u -p -r1.33 pppoe.4
> --- share/man/man4/pppoe.422 Mar 2017 21:37:24 - 1.33
> +++ share/man/man4/pppoe.412 Jun 2017 13:20:51 -
> @@ -113,17 +113,19 @@ The physical interface must also be mark
> .Pp
> Since this is a PPP interface, the addresses assigned to the interface
> may change during PPP negotiation.
> -There is no fine grained control available for deciding
> -which addresses are acceptable and which are not.
> -For the local side and the remote address there is exactly one choice:
> -hard coded address or wildcard.
> -If a real address is assigned to one side of the connection,
> -PPP negotiation will only agree to exactly this address.
> -If one side is wildcarded,
> -every address suggested by the peer will be accepted.
> +In the above example, 0.0.0.0 and 0.0.0.1 serve as placeholders for
> +dynamic address configuration.
> .Pp
> -To wildcard the local address set it to 0.0.0.0; to wildcard the remote
> -address set it to 0.0.0.1.
> +If the local address is set to wildcard address 0.0.0.0, it will
> +be changed to an address suggested by the peer.
> +.Pp
> +If the destination address is set to a wildcard address in the range
> +from 0.0.0.1 to 0.0.0.255, it will be changed to an address suggested
> +by the peer, and if a default route which uses this interface exists
> +the gateway will be changed to the suggested address as well.
> +.Pp
> +Otherwise, PPP negotiation will only agree to exactly the IPv4 addresses
> +which are configured on the interface.
> .Sh KERNEL OPTIONS
> .Nm
> does not interfere with other PPPoE implementations
> @@ -254,3 +256,8 @@ The presence of a
> .Xr mygate 5
> file will interfere with the routing table.
> Make sure this file is either empty or does not exist.
> +.Pp
> +Two
> +.Nm
> +interfaces configured with the same wildcard destination address
> +cannot share a routing table.
> Index: sys/net/if_spppsubr.c
> ===
> RCS file: /cvs/src/sys/net/if_spppsubr.c,v
> retrieving revision 1.164
> diff -u -p -r1.164 if_spppsubr.c
> --- sys/net/if_spppsubr.c 30 May 2017 07:50:37 - 1.164
> +++ sys/net/if_spppsubr.c 12 Jun 2017 13:10:20 -
> @@ -2632,7 +2632,7 @@ sppp_ipcp_tls(struct sppp *sp)
> sp->ipcp.flags |= IPCP_MYADDR_DYN;
> sp->ipcp.opts |= (1 << IPCP_OPT_ADDRESS);
> }
> - if (hisaddr == 1) {
> + if (hisaddr >= 1 && hisaddr <= 255) {
> /*
>* XXX - remove this hack!
>* remote has no valid address, we need to get one assigned.
>
Re: sppp(4)/pppoe(4) dynamic address hack
On Mon, Jun 12, 2017 at 02:59:46PM +0100, Stuart Henderson wrote: > This seems the "least worst" option to me. (It's still racy, the > connection could come up and change the dest before the route is > added, so route addition would fail as we now require a "correct" > address on the p-p route addition, but that's not new). ok with me. Yes, this race is a separate problem. I actually have ifstated(8) monitor pppoe interface state and the address of the peer, and it injects a default route once the peer's address changes to something other than 0.0.0.1.
Re: sppp(4)/pppoe(4) dynamic address hack
On 12/06/17(Mon) 15:46, Stefan Sperling wrote:
> On Sat, Jun 10, 2017 at 06:14:02PM +0200, Martin Pieuchot wrote:
> > There's no need to fix the routing table, we could something like:
> >
> > - if (hisaddr == 1) {
> > + if (hisaddr < 10) {
> >
> > Now I think you have a good point that using a flag is better than a
> > magic address. But I think the ifconfig(8) interface should be simpler.
> >
> > What about reusing the 'autoconf' toggle? Could we say:
> >
> > # ifconfig pppoe0 inet autoconf
> >
> > Instead of introducing 'dynaddr' and 'dyndest'?
>
> 'autoconf' is already used in a context where no addresses are configured
> yet (apart from the mandatory link-local ones, which don't exist as such
> in IPv4). I would rather not overload it.
>
> I agree that adding new ifconfig(8) commands may not be the ideal solution.
> If the routing table were fixed eventually, such commands may become obsolete.
>
> Here's a shorter diff based on your suggestion above, which bypasses the
> tedious ifconfig(8) design decisions but still allows us to improve the
> current situation.
>
> With this, we keep the magic address hack (even though we don't really
> like it), allow multiple dynamic pppoe(4) in one box (255 should be
> enough for anyone), document our current hack properly, and don't
> require existing pppoe(4) setups to change their configuration after 6.2.
>
> Is this an acceptable short term solution?
Yes, but I'm not opposed to the previous diff either, both diffs are
correct and ok from my point of view.
> Index: share/man/man4/pppoe.4
> ===
> RCS file: /cvs/src/share/man/man4/pppoe.4,v
> retrieving revision 1.33
> diff -u -p -r1.33 pppoe.4
> --- share/man/man4/pppoe.422 Mar 2017 21:37:24 - 1.33
> +++ share/man/man4/pppoe.412 Jun 2017 13:20:51 -
> @@ -113,17 +113,19 @@ The physical interface must also be mark
> .Pp
> Since this is a PPP interface, the addresses assigned to the interface
> may change during PPP negotiation.
> -There is no fine grained control available for deciding
> -which addresses are acceptable and which are not.
> -For the local side and the remote address there is exactly one choice:
> -hard coded address or wildcard.
> -If a real address is assigned to one side of the connection,
> -PPP negotiation will only agree to exactly this address.
> -If one side is wildcarded,
> -every address suggested by the peer will be accepted.
> +In the above example, 0.0.0.0 and 0.0.0.1 serve as placeholders for
> +dynamic address configuration.
> .Pp
> -To wildcard the local address set it to 0.0.0.0; to wildcard the remote
> -address set it to 0.0.0.1.
> +If the local address is set to wildcard address 0.0.0.0, it will
> +be changed to an address suggested by the peer.
> +.Pp
> +If the destination address is set to a wildcard address in the range
> +from 0.0.0.1 to 0.0.0.255, it will be changed to an address suggested
> +by the peer, and if a default route which uses this interface exists
> +the gateway will be changed to the suggested address as well.
> +.Pp
> +Otherwise, PPP negotiation will only agree to exactly the IPv4 addresses
> +which are configured on the interface.
> .Sh KERNEL OPTIONS
> .Nm
> does not interfere with other PPPoE implementations
> @@ -254,3 +256,8 @@ The presence of a
> .Xr mygate 5
> file will interfere with the routing table.
> Make sure this file is either empty or does not exist.
> +.Pp
> +Two
> +.Nm
> +interfaces configured with the same wildcard destination address
> +cannot share a routing table.
> Index: sys/net/if_spppsubr.c
> ===
> RCS file: /cvs/src/sys/net/if_spppsubr.c,v
> retrieving revision 1.164
> diff -u -p -r1.164 if_spppsubr.c
> --- sys/net/if_spppsubr.c 30 May 2017 07:50:37 - 1.164
> +++ sys/net/if_spppsubr.c 12 Jun 2017 13:10:20 -
> @@ -2632,7 +2632,7 @@ sppp_ipcp_tls(struct sppp *sp)
> sp->ipcp.flags |= IPCP_MYADDR_DYN;
> sp->ipcp.opts |= (1 << IPCP_OPT_ADDRESS);
> }
> - if (hisaddr == 1) {
> + if (hisaddr >= 1 && hisaddr <= 255) {
> /*
>* XXX - remove this hack!
>* remote has no valid address, we need to get one assigned.
>
