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) > 2. propose negotiated name servers from sppp(4) to resolvd(8) using > RTM_PROPOSAL_STATIC route messages. > > 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). > > 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 > > > 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) */
I would very much prefer to use struct in_addr here (or maybe in_addr_t). It makes it more clear what the data is. > #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 */ As kn@ noted this underscore version is a bit rough. I wonder if IPCP_OPT definitions (from the standard) should be decoupled from the bitmask values to enable/disable a feature. > #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]; Please use memcpy and here. Since this remains in network byte order it should just work. > + 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]; See above. > + 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); > > 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 > */]; This line is too long now. I feel this should be documented in a different way. > 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]; Again this calls for memcpy() > + } > + > + 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); > +} > Apart from that good work :) -- :wq Claudio