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.4 22 Mar 2017 21:37:24 -0000 1.33
> +++ share/man/man4/pppoe.4 12 Jun 2017 13:20:51 -0000
> @@ -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 -0000 1.164
> +++ sys/net/if_spppsubr.c 12 Jun 2017 13:10:20 -0000
> @@ -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.
>
