On 15/01/16(Fri) 12:00, Martin Pieuchot wrote:
> One of the checks missing to have an unlocked forwarding path is related
> to multicast. We must ensure that the list of multicast groups attached
> to an ifp is not modified when the CPU processing a packet is traversing
> it.
>
> In order to prepare for such change the diff below introduces a new
> function to abstract the list traversal.
>
> It is a simple refactoring but multiples reviews are more than welcome
> at this stage of the release cycle.
>
> ok?
I'm still looking for reviews.
> Index: netinet/in.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet/in.c,v
> retrieving revision 1.125
> diff -u -p -r1.125 in.c
> --- netinet/in.c 3 Dec 2015 21:57:59 -0000 1.125
> +++ netinet/in.c 15 Jan 2016 10:33:36 -0000
> @@ -880,6 +880,21 @@ in_delmulti(struct in_multi *inm)
> }
> }
>
> +/*
> + * Return 1 if the multicast group represented by ``ap'' has been
> + * joined by interface ``ifp'', 0 otherwise.
> + */
> +int
> +in_hasmulti(struct in_addr *ap, struct ifnet *ifp)
> +{
> + struct in_multi *inm;
> + int joined;
> +
> + IN_LOOKUP_MULTI(*ap, ifp, inm);
> + joined = (inm != NULL);
> +
> + return (joined);
> +}
>
> void
> in_ifdetach(struct ifnet *ifp)
> Index: netinet/in_var.h
> ===================================================================
> RCS file: /cvs/src/sys/netinet/in_var.h,v
> retrieving revision 1.37
> diff -u -p -r1.37 in_var.h
> --- netinet/in_var.h 3 Dec 2015 21:57:59 -0000 1.37
> +++ netinet/in_var.h 15 Jan 2016 10:34:02 -0000
> @@ -154,6 +154,7 @@ int in_ifinit(struct ifnet *,
> struct in_ifaddr *, struct sockaddr_in *, int);
> struct in_multi *in_addmulti(struct in_addr *, struct ifnet *);
> void in_delmulti(struct in_multi *);
> +int in_hasmulti(struct in_addr *, struct ifnet *);
> void in_ifscrub(struct ifnet *, struct in_ifaddr *);
> int in_control(struct socket *, u_long, caddr_t, struct ifnet *);
> void in_prefixlen2mask(struct in_addr *, int);
> Index: netinet/ip_carp.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet/ip_carp.c,v
> retrieving revision 1.285
> diff -u -p -r1.285 ip_carp.c
> --- netinet/ip_carp.c 12 Jan 2016 09:22:01 -0000 1.285
> +++ netinet/ip_carp.c 15 Jan 2016 10:47:19 -0000
> @@ -1809,17 +1809,13 @@ carp_addr_updated(void *v)
>
> /* We received address changes from if_addrhooks callback */
> if (new_naddrs != sc->sc_naddrs || new_naddrs6 != sc->sc_naddrs6) {
> - struct in_addr mc_addr;
> - struct in_multi *inm;
>
> sc->sc_naddrs = new_naddrs;
> sc->sc_naddrs6 = new_naddrs6;
>
> /* Re-establish multicast membership removed by in_control */
> if (IN_MULTICAST(sc->sc_peer.s_addr)) {
> - mc_addr.s_addr = sc->sc_peer.s_addr;
> - IN_LOOKUP_MULTI(mc_addr, &sc->sc_if, inm);
> - if (inm == NULL) {
> + if (!in_hasmulti(&sc->sc_peer, &sc->sc_if)) {
> struct in_multi **imm =
> sc->sc_imo.imo_membership;
> u_int16_t maxmem =
> Index: netinet/ip_input.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet/ip_input.c,v
> retrieving revision 1.265
> diff -u -p -r1.265 ip_input.c
> --- netinet/ip_input.c 3 Dec 2015 21:11:53 -0000 1.265
> +++ netinet/ip_input.c 15 Jan 2016 10:34:30 -0000
> @@ -346,8 +346,6 @@ ipv4_input(struct mbuf *m)
> }
>
> if (IN_MULTICAST(ip->ip_dst.s_addr)) {
> - struct in_multi *inm;
> -
> /*
> * Make sure M_MCAST is set. It should theoretically
> * already be there, but let's play safe because upper
> @@ -402,8 +400,7 @@ ipv4_input(struct mbuf *m)
> * See if we belong to the destination multicast group on the
> * arrival interface.
> */
> - IN_LOOKUP_MULTI(ip->ip_dst, ifp, inm);
> - if (inm == NULL) {
> + if (!in_hasmulti(&ip->ip_dst, ifp)) {
> ipstat.ips_notmember++;
> if (!IN_LOCAL_GROUP(ip->ip_dst.s_addr))
> ipstat.ips_cantforward++;
> Index: netinet/ip_output.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet/ip_output.c,v
> retrieving revision 1.316
> diff -u -p -r1.316 ip_output.c
> --- netinet/ip_output.c 13 Jan 2016 09:38:36 -0000 1.316
> +++ netinet/ip_output.c 15 Jan 2016 10:43:47 -0000
> @@ -241,7 +241,6 @@ reroute:
>
> if (IN_MULTICAST(ip->ip_dst.s_addr) ||
> (ip->ip_dst.s_addr == INADDR_BROADCAST)) {
> - struct in_multi *inm;
>
> m->m_flags |= (ip->ip_dst.s_addr == INADDR_BROADCAST) ?
> M_BCAST : M_MCAST;
> @@ -295,9 +294,8 @@ reroute:
> ip->ip_src = ia->ia_addr.sin_addr;
> }
>
> - IN_LOOKUP_MULTI(ip->ip_dst, ifp, inm);
> - if (inm != NULL &&
> - (imo == NULL || imo->imo_loop)) {
> + if ((imo == NULL || imo->imo_loop) &&
> + in_hasmulti(&ip->ip_dst, ifp)) {
> /*
> * If we belong to the destination multicast group
> * on the outgoing interface, and the caller did not
> Index: netinet6/in6.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet6/in6.c,v
> retrieving revision 1.182
> diff -u -p -r1.182 in6.c
> --- netinet6/in6.c 22 Dec 2015 10:01:01 -0000 1.182
> +++ netinet6/in6.c 15 Jan 2016 10:43:04 -0000
> @@ -1378,6 +1378,22 @@ in6_delmulti(struct in6_multi *in6m)
> }
> }
>
> +/*
> + * Return 1 if the multicast group represented by ``maddr6'' has been
> + * joined by interface ``ifp'', 0 otherwise.
> + */
> +int
> +in6_hasmulti(struct in6_addr *maddr6, struct ifnet *ifp)
> +{
> + struct in6_multi *in6m;
> + int joined;
> +
> + IN6_LOOKUP_MULTI(*maddr6, ifp, in6m);
> + joined = (in6m != NULL);
> +
> + return (joined);
> +}
> +
> struct in6_multi_mship *
> in6_joingroup(struct ifnet *ifp, struct in6_addr *addr, int *errorp)
> {
> Index: netinet6/in6_var.h
> ===================================================================
> RCS file: /cvs/src/sys/netinet6/in6_var.h,v
> retrieving revision 1.58
> diff -u -p -r1.58 in6_var.h
> --- netinet6/in6_var.h 18 Nov 2015 13:58:02 -0000 1.58
> +++ netinet6/in6_var.h 15 Jan 2016 10:37:33 -0000
> @@ -495,6 +495,7 @@ do {
> \
>
> struct in6_multi *in6_addmulti(struct in6_addr *, struct ifnet *, int
> *);
> void in6_delmulti(struct in6_multi *);
> +int in6_hasmulti(struct in6_addr *, struct ifnet *);
> struct in6_multi_mship *in6_joingroup(struct ifnet *, struct in6_addr *, int
> *);
> int in6_leavegroup(struct in6_multi_mship *);
> int in6_control(struct socket *, u_long, caddr_t, struct ifnet *);
> Index: netinet6/ip6_input.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet6/ip6_input.c,v
> retrieving revision 1.153
> diff -u -p -r1.153 ip6_input.c
> --- netinet6/ip6_input.c 6 Jan 2016 10:02:42 -0000 1.153
> +++ netinet6/ip6_input.c 15 Jan 2016 10:39:08 -0000
> @@ -388,7 +388,6 @@ ip6_input(struct mbuf *m)
> * Multicast check
> */
> if (IN6_IS_ADDR_MULTICAST(&ip6->ip6_dst)) {
> - struct in6_multi *in6m = NULL;
>
> /*
> * Make sure M_MCAST is set. It should theoretically
> @@ -401,8 +400,7 @@ ip6_input(struct mbuf *m)
> * See if we belong to the destination multicast group on the
> * arrival interface.
> */
> - IN6_LOOKUP_MULTI(ip6->ip6_dst, ifp, in6m);
> - if (in6m)
> + if (in6_hasmulti(&ip6->ip6_dst, ifp))
> ours = 1;
> #ifdef MROUTING
> else if (!ip6_mforwarding || !ip6_mrouter)
> Index: netinet6/ip6_mroute.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet6/ip6_mroute.c,v
> retrieving revision 1.101
> diff -u -p -r1.101 ip6_mroute.c
> --- netinet6/ip6_mroute.c 3 Dec 2015 13:13:42 -0000 1.101
> +++ netinet6/ip6_mroute.c 15 Jan 2016 10:48:16 -0000
> @@ -1372,7 +1372,6 @@ phyint_send6(struct ip6_hdr *ip6, struct
> int error = 0;
> int s = splsoftnet();
> static struct route_in6 ro;
> - struct in6_multi *in6m;
> struct sockaddr_in6 *dst6;
>
> /*
> @@ -1416,8 +1415,7 @@ phyint_send6(struct ip6_hdr *ip6, struct
> * on the outgoing interface, loop back a copy.
> */
> dst6 = &ro.ro_dst;
> - IN6_LOOKUP_MULTI(ip6->ip6_dst, ifp, in6m);
> - if (in6m != NULL) {
> + if (in6_hasmulti(&ip6->ip6_dst, ifp)) {
> dst6->sin6_len = sizeof(struct sockaddr_in6);
> dst6->sin6_family = AF_INET6;
> dst6->sin6_addr = ip6->ip6_dst;
> Index: netinet6/ip6_output.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet6/ip6_output.c,v
> retrieving revision 1.203
> diff -u -p -r1.203 ip6_output.c
> --- netinet6/ip6_output.c 13 Jan 2016 09:38:37 -0000 1.203
> +++ netinet6/ip6_output.c 15 Jan 2016 10:40:56 -0000
> @@ -566,7 +566,6 @@ reroute:
> m->m_flags &= ~(M_BCAST | M_MCAST); /* just in case */
> } else {
> /* Multicast */
> - struct in6_multi *in6m;
>
> m->m_flags = (m->m_flags & ~M_BCAST) | M_MCAST;
>
> @@ -579,9 +578,8 @@ reroute:
> goto bad;
> }
>
> - IN6_LOOKUP_MULTI(ip6->ip6_dst, ifp, in6m);
> - if (in6m != NULL &&
> - (im6o == NULL || im6o->im6o_loop)) {
> + if ((im6o == NULL || im6o->im6o_loop) &&
> + in6_hasmulti(&ip6->ip6_dst, ifp)) {
> /*
> * If we belong to the destination multicast group
> * on the outgoing interface, and the caller did not
>
