On Wed, Nov 10, 2021 at 07:35:26AM +0100, Bjorn Ketelaars wrote:
> On Mon 08/11/2021 11:52, Bjorn Ketelaars wrote:
> > Diff below does two things:
> > 1. add PPP IPCP extensions for name server addresses (rfc1877) to
> > sppp(4)
> > 2. propose negotiated name servers from sppp(4) to resolvd(8) using
> > RTM_PROPOSAL_STATIC route messages.
>
>
> Updated diff below, based on feedback from kn@ and claudio@:
>
> - fix forgotten parentheses with `sizeof`
> - instead of using `u_int32_t` use `struct in_addr` for holding dns
> addresses. Makes it more clear what the data is
> - decouple `IPCP_OPT` definitions from the bitmask values to
> enable/disable an option. Makes the code look a bit better
> - use `memcpy`
> - fit code within 80 columns
>
> While here add RFC to sppp(4)'s STANDARDS section.
>
> @kn, is this still OK for you?
>
> Other OK's?
There is one point which bother me a bit: you are using
RTP_PROPOSAL_STATIC for sending the proposal, whereas all others
sources (dhcpleased/dhclient, slaacd, umb) are using a specific value.
By using RTP_PROPOSAL_STATIC, it means also that route(8) nameserver
subcommand might interfere with it.
Using a new specific value (like RTP_PROPOSAL_SPPP) would make sense
to me. But no objection if RTM_PROPOSAL_STATIC is preferred.
> +void
> +sppp_update_dns(struct ifnet *ifp)
> +{
> + struct rt_addrinfo info;
> + struct sockaddr_rtdns rtdns;
> + struct sppp *sp = ifp->if_softc;
> + size_t sz = 0;
> + int i, flag = 0;
> +
> + memset(&rtdns, 0, sizeof(rtdns));
> + memset(&info, 0, sizeof(info));
> +
> + for (i = 0; i < IPCP_MAX_DNSSRV; i++) {
> + if (sp->ipcp.dns[i].s_addr == INADDR_ANY)
> + break;
> + sz = sizeof(sp->ipcp.dns[i].s_addr);
> + memcpy(rtdns.sr_dns + i * sz, &sp->ipcp.dns[i].s_addr, sz);
> + flag = RTF_UP;
> + }
> +
> + rtdns.sr_family = AF_INET;
> + rtdns.sr_len = 2 + i * sz;
> + info.rti_info[RTAX_DNS] = srtdnstosa(&rtdns);
> +
> + rtm_proposal(ifp, &info, flag, RTP_PROPOSAL_STATIC);
> +}
Thanks.
--
Sebastien Marie
