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.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.
>
