On Wed, Nov 23, 2022 at 08:24:11AM +0000, Klemens Nanni wrote:
> *if_afdata[] and struct domain's dom_if{at,de}tach() are only used with
> IPv6 Neighbour Discovery in6_dom{at,de}tach(), which allocate/init and
> free single struct nd_ifinfo.
>
> Set up a new ND-specific *if_nd member directly to avoid yet another
> layer of indirection and thus make the generic domain API obsolete.
>
> The per-interface data is only accessed in nd6.c and nd6_nbr.c through
> the ND_IFINFO() macro; it is allocated and freed exactly once during
> interface at/detach, so document it as [I]mmutable.
>
> Next up as separate commits:
> - remove *if_afdata[] struct domain's dom_if{at,de}tach()
> - nd6_if{at,de}tach() return/argument type cleanup
> - inline ND_IFINFO()
>
> Feedback? Objection? OK?
I like this but one comment below.
> diff --git a/sys/net/if.c b/sys/net/if.c
> index f3fba33de3f..bdaa41aea9f 100644
> --- a/sys/net/if.c
> +++ b/sys/net/if.c
> @@ -453,16 +453,25 @@ if_idxmap_remove(struct ifnet *ifp)
> */
> void
> if_attachsetup(struct ifnet *ifp)
> {
> unsigned long ifidx;
> +#ifdef INET6
> + int s;
> +#endif
>
> NET_ASSERT_LOCKED();
>
> if_addgroup(ifp, IFG_ALL);
>
> if_attachdomain(ifp);
> +#ifdef INET6
> + s = splnet();
> + ifp->if_nd = nd6_ifattach(ifp);
> + splx(s);
There is no need to do this splnet() dance here.
nd6_ifattach calls malloc() and then sets some values of that memory and
returns the pointer. There is no need for splnet() around that code
nothing can see the returned value ahead of time.
Shouldn't we also remove if_afdata in the same commit? You killed the last
user of this and I don't like to alter somewhat common structs over and
over. Removeing this part of the code as well wont make this diff much
larger.
> +#endif
> +
> #if NPF > 0
> pfi_attach_ifnet(ifp);
> #endif
>
> timeout_set(&ifp->if_slowtimo, if_slowtimo, ifp);
> @@ -1125,10 +1134,13 @@ if_detach(struct ifnet *ifp)
> for (i = 0; (dp = domains[i]) != NULL; i++) {
> if (dp->dom_ifdetach && ifp->if_afdata[dp->dom_family])
> (*dp->dom_ifdetach)(ifp,
> ifp->if_afdata[dp->dom_family]);
> }
> +#ifdef INET6
> + nd6_ifdetach(ifp->if_nd);
> +#endif
>
> /* Announce that the interface is gone. */
> rtm_ifannounce(ifp, IFAN_DEPARTURE);
> splx(s);
> NET_UNLOCK();
> diff --git a/sys/net/if_var.h b/sys/net/if_var.h
> index 3a418bf0547..3bf07ed3071 100644
> --- a/sys/net/if_var.h
> +++ b/sys/net/if_var.h
> @@ -185,10 +185,11 @@ struct ifnet { /* and the
> entries */
> unsigned int if_niqs; /* [I] number of input queues */
>
> struct sockaddr_dl *if_sadl; /* [N] pointer to our sockaddr_dl */
>
> void *if_afdata[AF_MAX];
> + struct nd_ifinfo *if_nd; /* [I] IPv6 Neighour Discovery info */
> };
> #define if_mtu if_data.ifi_mtu
> #define if_type if_data.ifi_type
> #define if_addrlen if_data.ifi_addrlen
> #define if_hdrlen if_data.ifi_hdrlen
> diff --git a/sys/netinet6/in6.c b/sys/netinet6/in6.c
> index 7971f79f44c..3fe9386f8af 100644
> --- a/sys/netinet6/in6.c
> +++ b/sys/netinet6/in6.c
> @@ -1595,17 +1595,5 @@ in6if_do_dad(struct ifnet *ifp)
> return (0);
>
> return (1);
> }
> }
> -
> -void *
> -in6_domifattach(struct ifnet *ifp)
> -{
> - return nd6_ifattach(ifp);
> -}
> -
> -void
> -in6_domifdetach(struct ifnet *ifp, void *aux)
> -{
> - nd6_ifdetach(aux);
> -}
> diff --git a/sys/netinet6/in6_proto.c b/sys/netinet6/in6_proto.c
> index 808422f6eab..e302f39cddf 100644
> --- a/sys/netinet6/in6_proto.c
> +++ b/sys/netinet6/in6_proto.c
> @@ -336,12 +336,10 @@ const struct domain inet6domain = {
> .dom_protosw = inet6sw,
> .dom_protoswNPROTOSW = &inet6sw[nitems(inet6sw)],
> .dom_sasize = sizeof(struct sockaddr_in6),
> .dom_rtoffset = offsetof(struct sockaddr_in6, sin6_addr),
> .dom_maxplen = 128,
> - .dom_ifattach = in6_domifattach,
> - .dom_ifdetach = in6_domifdetach
> };
>
> /*
> * Internet configuration info
> */
> diff --git a/sys/netinet6/nd6.h b/sys/netinet6/nd6.h
> index f6920137cea..1bbbc5f9e23 100644
> --- a/sys/netinet6/nd6.h
> +++ b/sys/netinet6/nd6.h
> @@ -75,11 +75,11 @@ struct in6_ndireq {
> #ifdef _KERNEL
>
> #include <sys/queue.h>
>
> #define ND_IFINFO(ifp) \
> - ((struct nd_ifinfo *)(ifp)->if_afdata[AF_INET6])
> + ((ifp)->if_nd)
>
> struct llinfo_nd6 {
> TAILQ_ENTRY(llinfo_nd6) ln_list;
> struct rtentry *ln_rt;
> struct mbuf *ln_hold; /* last packet until resolved/timeout */
--
:wq Claudio
