On Tue, Aug 09, 2022 at 08:42:37AM +0000, Klemens Nanni wrote:
> All of them are passed to inspect/copy out fields, none of the functions
> writes to the struct.
>
> This makes it easier to argue about code (in MP context).
>
> For this to work, ifa_ifwithaddr(), ifa_ifwithdstaddr() and
> ifaof_ifpforaddr() need const arguments as well (matching the behaviour).
>
> Expand three satosin6() calls and use a local variable to be able to add
> const and reduce duplication.
>
> Builds and runs fine on sparc64.
> Feedback? OK?
I'm not in favor of this move. The introduction of const just spreads like
a virus and so does the amount of dubious casts. I don't think it makes it
that much easier to argue about MP. The same can be achieved using other
methods.
> diff --git a/sys/net/if.c b/sys/net/if.c
> index cdb43e83d15..d71927b9aa9 100644
> --- a/sys/net/if.c
> +++ b/sys/net/if.c
> @@ -1414,7 +1414,7 @@ if_congested(void)
> * Locate an interface based on a complete address.
> */
> struct ifaddr *
> -ifa_ifwithaddr(struct sockaddr *addr, u_int rtableid)
> +ifa_ifwithaddr(const struct sockaddr *addr, u_int rtableid)
> {
> struct ifnet *ifp;
> struct ifaddr *ifa;
> @@ -1444,7 +1444,7 @@ ifa_ifwithaddr(struct sockaddr *addr, u_int rtableid)
> * Locate the point to point interface with a given destination address.
> */
> struct ifaddr *
> -ifa_ifwithdstaddr(struct sockaddr *addr, u_int rdomain)
> +ifa_ifwithdstaddr(const struct sockaddr *addr, u_int rdomain)
> {
> struct ifnet *ifp;
> struct ifaddr *ifa;
> @@ -1475,11 +1475,11 @@ ifa_ifwithdstaddr(struct sockaddr *addr, u_int
> rdomain)
> * a given address.
> */
> struct ifaddr *
> -ifaof_ifpforaddr(struct sockaddr *addr, struct ifnet *ifp)
> +ifaof_ifpforaddr(const struct sockaddr *addr, struct ifnet *ifp)
> {
> struct ifaddr *ifa;
> - char *cp, *cp2, *cp3;
> - char *cplim;
> + const char *cp, *cp2, *cp3;
> + const char *cplim;
> struct ifaddr *ifa_maybe = NULL;
> u_int af = addr->sa_family;
>
> diff --git a/sys/net/if_var.h b/sys/net/if_var.h
> index 400afd319b0..f511af7dfbd 100644
> --- a/sys/net/if_var.h
> +++ b/sys/net/if_var.h
> @@ -330,9 +330,9 @@ void p2p_rtrequest(struct ifnet *, int, struct
> rtentry *);
> void p2p_input(struct ifnet *, struct mbuf *);
> int p2p_bpf_mtap(caddr_t, const struct mbuf *, u_int);
>
> -struct ifaddr *ifa_ifwithaddr(struct sockaddr *, u_int);
> -struct ifaddr *ifa_ifwithdstaddr(struct sockaddr *, u_int);
> -struct ifaddr *ifaof_ifpforaddr(struct sockaddr *, struct ifnet *);
> +struct ifaddr *ifa_ifwithaddr(const struct sockaddr *, u_int);
> +struct ifaddr *ifa_ifwithdstaddr(const struct sockaddr *, u_int);
> +struct ifaddr *ifaof_ifpforaddr(const struct sockaddr *, struct ifnet
> *);
> void ifafree(struct ifaddr *);
>
> int if_isconnected(const struct ifnet *, unsigned int);
> diff --git a/sys/netinet6/nd6.c b/sys/netinet6/nd6.c
> index d4aedb92187..5259b0790c7 100644
> --- a/sys/netinet6/nd6.c
> +++ b/sys/netinet6/nd6.c
> @@ -1329,14 +1329,15 @@ nd6_slowtimo(void *ignored_arg)
>
> int
> nd6_resolve(struct ifnet *ifp, struct rtentry *rt0, struct mbuf *m,
> - struct sockaddr *dst, u_char *desten)
> + const struct sockaddr *dst, u_char *desten)
> {
> struct sockaddr_dl *sdl;
> struct rtentry *rt;
> struct llinfo_nd6 *ln = NULL;
> + const struct in6_addr *in6 = &((struct sockaddr_in6 *)dst)->sin6_addr;
>
> if (m->m_flags & M_MCAST) {
> - ETHER_MAP_IPV6_MULTICAST(&satosin6(dst)->sin6_addr, desten);
> + ETHER_MAP_IPV6_MULTICAST(in6, desten);
> return (0);
> }
>
> @@ -1404,8 +1405,7 @@ nd6_resolve(struct ifnet *ifp, struct rtentry *rt0,
> struct mbuf *m,
> char addr[INET6_ADDRSTRLEN];
> log(LOG_DEBUG, "%s: %s: incorrect nd6 information\n",
> __func__,
> - inet_ntop(AF_INET6, &satosin6(dst)->sin6_addr,
> - addr, sizeof(addr)));
> + inet_ntop(AF_INET6, in6, addr, sizeof(addr)));
> m_freem(m);
> return (EINVAL);
> }
> @@ -1431,7 +1431,7 @@ nd6_resolve(struct ifnet *ifp, struct rtentry *rt0,
> struct mbuf *m,
> if (!ND6_LLINFO_PERMANENT(ln) && ln->ln_asked == 0) {
> ln->ln_asked++;
> nd6_llinfo_settimer(ln, ND_IFINFO(ifp)->retrans / 1000);
> - nd6_ns_output(ifp, NULL, &satosin6(dst)->sin6_addr, ln, 0);
> + nd6_ns_output(ifp, NULL, in6, ln, 0);
> }
> return (EAGAIN);
> }
> diff --git a/sys/netinet6/nd6.h b/sys/netinet6/nd6.h
> index 980524f16ea..254a613ba42 100644
> --- a/sys/netinet6/nd6.h
> +++ b/sys/netinet6/nd6.h
> @@ -171,12 +171,12 @@ int nd6_ioctl(u_long, caddr_t, struct ifnet *);
> void nd6_cache_lladdr(struct ifnet *, const struct in6_addr *, char *,
> int, int, int);
> int nd6_resolve(struct ifnet *, struct rtentry *, struct mbuf *,
> - struct sockaddr *, u_char *);
> + const struct sockaddr *, u_char *);
> int nd6_need_cache(struct ifnet *);
>
> void nd6_na_input(struct mbuf *, int, int);
> void nd6_na_output(struct ifnet *, const struct in6_addr *,
> - const struct in6_addr *, u_long, int, struct sockaddr *);
> + const struct in6_addr *, u_long, int, const struct sockaddr *);
> void nd6_ns_input(struct mbuf *, int, int);
> void nd6_ns_output(struct ifnet *, const struct in6_addr *,
> const struct in6_addr *, const struct llinfo_nd6 *, int);
> diff --git a/sys/netinet6/nd6_nbr.c b/sys/netinet6/nd6_nbr.c
> index 57433b47d41..c079e6a133a 100644
> --- a/sys/netinet6/nd6_nbr.c
> +++ b/sys/netinet6/nd6_nbr.c
> @@ -873,7 +873,7 @@ nd6_na_input(struct mbuf *m, int off, int icmp6len)
> void
> nd6_na_output(struct ifnet *ifp, const struct in6_addr *daddr6,
> const struct in6_addr *taddr6, u_long flags, int tlladdr,
> - struct sockaddr *sdl0)
> + const struct sockaddr *sdl0)
> {
> struct mbuf *m;
> struct rtentry *rt = NULL;
> @@ -985,8 +985,9 @@ nd6_na_output(struct ifnet *ifp, const struct in6_addr
> *daddr6,
> if (sdl0 == NULL) {
> mac = nd6_ifptomac(ifp);
> } else if (sdl0->sa_family == AF_LINK) {
> - struct sockaddr_dl *sdl;
> - sdl = satosdl(sdl0);
> + const struct sockaddr_dl *sdl;
> +
> + sdl = (struct sockaddr_dl *)sdl0;
> if (sdl->sdl_alen == ifp->if_addrlen)
> mac = LLADDR(sdl);
> }
> --
> 2.34.2
>
--
:wq Claudio
