On Mon, Nov 08, 2021 at 11:52:52AM +0100, Bjorn Ketelaars wrote: > Diff below does two things: > 1. add PPP IPCP extensions for name server addresses (rfc1877) to > sppp(4)
You could add that RFC to sppp(4)'s STANDARDS section. > 2. propose negotiated name servers from sppp(4) to resolvd(8) using > RTM_PROPOSAL_STATIC route messages. Needs mentioning in resolvd(8)'s sentence about umb(4). unwind(8) should mention it as well (it does not even mention umb at this point). > With this I'm able to use DNS servers as provided by my ISP who uses > PPPoE. resolv.conf is updated by resolvd(8) as function of status > changes of pppoe(4). Works as expected on octeon behind german VDSL2. I use unbound(8) with a static 'nameserver ::1' entry in resolv.conf(5) and resolvd(8) is running but does not get any DNS proposals, hence the unbound entry wins. With this diff pppoe(4) sends two nameservers and thus wins, but that is expected and setups such as mine must either - disable resolvd - enable resolvd but also enable unwind (unwind also learns DNS proposals but always wins in resolv.conf) - enable resolvd and clear pppoe(4) proposals, e.g. `route nameserver pppoe0' - do whatever else fits their setup > rfc1877 implementation is derived from code from NetBSD, and inspired by > [0] and [1]. Borrowed code from umb(4) for the route messages. > > Opinions/comments/OK? > [0] https://marc.info/?l=openbsd-tech&m=134943767022961&w=2 > [1] https://marc.info/?l=openbsd-tech&m=159405677416423&w=2 I guess the ifconfig(8) bits to print proposed nameservers are still useful, even nowadays where resolvd/unwind pick them up and/or print them (`unwindctl status autoconf'). As per above, I'd like to improve the bigger picture, but this diff is OK kn as-is; see inline comments. > diff --git sys/net/if_sppp.h sys/net/if_sppp.h > index ff559fcc369..f2dab61e46b 100644 > --- sys/net/if_sppp.h > +++ sys/net/if_sppp.h > @@ -132,6 +132,8 @@ struct sipcp { > * original one here, in network byte order */ > u_int32_t req_hisaddr; /* remote address requested (IPv4) */ > u_int32_t req_myaddr; /* local address requested (IPv4) */ > +#define IPCP_MAX_DNSSRV 2 > + u_int32_t dns[IPCP_MAX_DNSSRV]; /* IPv4 DNS servers (RFC 1877) */ > #ifdef INET6 > struct in6_aliasreq req_ifid; /* local ifid requested (IPv6) */ > #endif > diff --git sys/net/if_spppsubr.c sys/net/if_spppsubr.c > index ac1dc9a709d..225ad8f5a3e 100644 > --- sys/net/if_spppsubr.c > +++ sys/net/if_spppsubr.c > @@ -132,6 +132,10 @@ > #define IPCP_OPT_ADDRESSES 1 /* both IP addresses; deprecated */ > #define IPCP_OPT_COMPRESSION 2 /* IP compression protocol (VJ) */ > #define IPCP_OPT_ADDRESS 3 /* local IP address */ > +#define IPCP_OPT_PRIMDNS 129 /* primary remote dns address */ > +#define _IPCP_OPT_PRIMDNS_ 4 /* set/check option */ > +#define IPCP_OPT_SECDNS 131 /* secondary remote dns address > */ > +#define _IPCP_OPT_SECDNS_ 5 /* set/check option */ This _*_ looks ugly. NetBSD has `IPCP_OPT_PRIMDNS 129' and `SPPP_IPCP_OPT_PRIMDNS __BIT(3)' which we don't have. Optimally, we could even out this difference, but I have not yet looked closer into this. > #define IPV6CP_OPT_IFID 1 /* interface identifier */ > #define IPV6CP_OPT_COMPRESSION 2 /* IPv6 compression protocol */ > @@ -338,6 +342,8 @@ void sppp_update_gw(struct ifnet *ifp); > void sppp_set_ip_addrs(void *); > void sppp_clear_ip_addrs(void *); > void sppp_set_phase(struct sppp *sp); > +void sppp_update_dns(struct ifnet *ifp); > +void sppp_rtrequest(struct ifnet *ifp, int req, struct rtentry *rt); > > /* our control protocol descriptors */ > static const struct cp lcp = { > @@ -701,6 +707,7 @@ sppp_attach(struct ifnet *ifp) > > sp->pp_if.if_type = IFT_PPP; > sp->pp_if.if_output = sppp_output; > + sp->pp_if.if_rtrequest = sppp_rtrequest; > ifq_set_maxlen(&sp->pp_if.if_snd, 50); > mq_init(&sp->pp_cpq, 50, IPL_NET); > sp->pp_loopcnt = 0; > @@ -2519,6 +2526,12 @@ sppp_ipcp_RCN_rej(struct sppp *sp, struct lcp_header > *h, int len) > sp->ipcp.opts &= ~(1 << IPCP_OPT_COMPRESS); > break; > #endif > + case IPCP_OPT_PRIMDNS: > + sp->ipcp.opts &= ~(1 << _IPCP_OPT_PRIMDNS_); > + break; > + case IPCP_OPT_SECDNS: > + sp->ipcp.opts &= ~(1 << _IPCP_OPT_SECDNS_); > + break; > } > } > if (debug) > @@ -2584,6 +2597,16 @@ sppp_ipcp_RCN_nak(struct sppp *sp, struct lcp_header > *h, int len) > */ > break; > #endif > + case IPCP_OPT_PRIMDNS: > + if (len >= 6 && p[1] == 6) > + sp->ipcp.dns[0] = p[2] << 24 | p[3] << 16 | > + p[4] << 8 | p[5]; > + break; > + case IPCP_OPT_SECDNS: > + if (len >= 6 && p[1] == 6) > + sp->ipcp.dns[1] = p[2] << 24 | p[3] << 16 | > + p[4] << 8 | p[5]; > + break; > } > } > if (debug) > @@ -2612,6 +2635,7 @@ sppp_ipcp_tls(struct sppp *sp) > IPCP_MYADDR_DYN|IPCP_HISADDR_DYN); > sp->ipcp.req_myaddr = 0; > sp->ipcp.req_hisaddr = 0; > + memset(&sp->ipcp.dns, 0, sizeof sp->ipcp.dns); I'd very much prefer parantheses (for new code) with `sizeof'. > > sppp_get_ip_addrs(sp, &myaddr, &hisaddr, 0); > /* > @@ -2644,6 +2668,10 @@ sppp_ipcp_tls(struct sppp *sp) > sp->ipcp.flags |= IPCP_HISADDR_DYN; > } > > + /* negotiate name server addresses */ > + sp->ipcp.opts |= (1 << _IPCP_OPT_PRIMDNS_); > + sp->ipcp.opts |= (1 << _IPCP_OPT_SECDNS_); > + > /* indicate to LCP that it must stay alive */ > sp->lcp.protos |= (1 << IDX_IPCP); > } > @@ -2663,7 +2691,7 @@ sppp_ipcp_tlf(struct sppp *sp) > void > sppp_ipcp_scr(struct sppp *sp) > { > - char opt[6 /* compression */ + 6 /* address */]; > + char opt[6 /* compression */ + 6 /* address */ + 12 /* dns addresses > */]; > u_int32_t ouraddr; > int i = 0; > > @@ -2692,6 +2720,24 @@ sppp_ipcp_scr(struct sppp *sp) > opt[i++] = ouraddr; > } > > + if (sp->ipcp.opts & (1 << _IPCP_OPT_PRIMDNS_)) { > + opt[i++] = IPCP_OPT_PRIMDNS; > + opt[i++] = 6; > + opt[i++] = sp->ipcp.dns[0] >> 24; > + opt[i++] = sp->ipcp.dns[0] >> 16; > + opt[i++] = sp->ipcp.dns[0] >> 8; > + opt[i++] = sp->ipcp.dns[0]; > + } > + > + if (sp->ipcp.opts & (1 << _IPCP_OPT_SECDNS_)) { > + opt[i++] = IPCP_OPT_SECDNS; > + opt[i++] = 6; > + opt[i++] = sp->ipcp.dns[1] >> 24; > + opt[i++] = sp->ipcp.dns[1] >> 16; > + opt[i++] = sp->ipcp.dns[1] >> 8; > + opt[i++] = sp->ipcp.dns[1]; > + } > + > sp->confid[IDX_IPCP] = ++sp->pp_seq; > sppp_cp_send(sp, PPP_IPCP, CONF_REQ, sp->confid[IDX_IPCP], i, opt); > } > @@ -4242,6 +4288,7 @@ sppp_set_ip_addrs(void *arg1) > goto out; > } > sppp_update_gw(ifp); > + sppp_update_dns(ifp); > } > out: > NET_UNLOCK(); > @@ -4302,6 +4349,9 @@ sppp_clear_ip_addrs(void *arg1) > goto out; > } > sppp_update_gw(ifp); > + > + memset(sp->ipcp.dns, 0, sizeof(sp->ipcp.dns)); > + sppp_update_dns(ifp); > } > out: > NET_UNLOCK(); > @@ -4721,6 +4771,8 @@ sppp_ipcp_opt_name(u_char opt) > case IPCP_OPT_ADDRESSES: return "addresses"; > case IPCP_OPT_COMPRESSION: return "compression"; > case IPCP_OPT_ADDRESS: return "address"; > + case IPCP_OPT_PRIMDNS: return "primdns"; > + case IPCP_OPT_SECDNS: return "secdns"; > } > snprintf (buf, sizeof buf, "0x%x", opt); > return buf; > @@ -4851,3 +4903,45 @@ sppp_set_phase(struct sppp *sp) > if_link_state_change(ifp); > } > } > + > +void > +sppp_update_dns(struct ifnet *ifp) > +{ > + struct rt_addrinfo info; > + struct sockaddr_rtdns rtdns; > + struct sppp *sp = ifp->if_softc; > + u_int32_t dns; > + int i, flag = 0; > + size_t sz = 0; > + > + memset(&rtdns, 0, sizeof(rtdns)); > + memset(&info, 0, sizeof(info)); > + > + for (i = 0; i < IPCP_MAX_DNSSRV; i++) { > + dns = ntohl(sp->ipcp.dns[i]); > + if (dns == INADDR_ANY) > + break; > + sz = sizeof(dns); > + memcpy(rtdns.sr_dns + i * sz, &dns, 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); > +} > + > +void > +sppp_rtrequest(struct ifnet *ifp, int req, struct rtentry *rt) > +{ > + if (req == RTM_PROPOSAL) { > + KERNEL_LOCK(); > + sppp_update_dns(ifp); > + KERNEL_UNLOCK(); > + return; > + } > + > + p2p_rtrequest(ifp, req, rt); > +} >
