On 19/09/13(Thu) 13:59, Martin Pieuchot wrote:
> Diff below change the macros used to iterate over the multicast
> records linked to an interface without using the global lists of
> addresses.
>
> These records are currently link to the first address descriptor,
> respectively v4 and v6, even if they are per-interface. So I
> changed the code to loop over the global interface list instead
> of iterating over all existing addresses.
>
> Tested here with a carp setup. Comments, ok?
I got one ok for this diff but no other feedback.
Did somebody tried it on a different setup? I would really help me to
get this in to move forward, so I appreciate any tests, reviews or ok.
> Index: netinet/igmp.c
> ===================================================================
> RCS file: /home/ncvs/src/sys/netinet/igmp.c,v
> retrieving revision 1.33
> diff -u -p -r1.33 igmp.c
> --- netinet/igmp.c 2 May 2013 11:54:10 -0000 1.33
> +++ netinet/igmp.c 13 Sep 2013 09:07:42 -0000
> @@ -104,6 +104,7 @@ int igmp_timers_are_running;
> static struct router_info *rti_head;
> struct igmpstat igmpstat;
>
> +void igmp_checktimer(struct ifnet *);
> void igmp_sendpkt(struct in_multi *, int, in_addr_t);
> int rti_fill(struct in_multi *);
> struct router_info * rti_find(struct ifnet *);
> @@ -193,7 +194,6 @@ igmp_input(struct mbuf *m, ...)
> int igmplen;
> int minlen;
> struct in_multi *inm;
> - struct in_multistep step;
> struct router_info *rti;
> struct in_ifaddr *ia;
> int timer;
> @@ -266,17 +266,14 @@ igmp_input(struct mbuf *m, ...)
> * except those that are already running and those
> * that belong to a "local" group (224.0.0.X).
> */
> - IN_FIRST_MULTI(step, inm);
> - while (inm != NULL) {
> - if (inm->inm_ia->ia_ifp == ifp &&
> - inm->inm_timer == 0 &&
> + IN_FOREACH_MULTI(ia, ifp, inm) {
> + if (inm->inm_timer == 0 &&
> !IN_LOCAL_GROUP(inm->inm_addr.s_addr)) {
> inm->inm_state = IGMP_DELAYING_MEMBER;
> inm->inm_timer = IGMP_RANDOM_DELAY(
> IGMP_MAX_HOST_REPORT_DELAY *
> PR_FASTHZ);
> igmp_timers_are_running = 1;
> }
> - IN_NEXT_MULTI(step, inm);
> }
> } else {
> if (!IN_MULTICAST(ip->ip_dst.s_addr)) {
> @@ -297,10 +294,8 @@ igmp_input(struct mbuf *m, ...)
> * timers already running, check if they need to be
> * reset.
> */
> - IN_FIRST_MULTI(step, inm);
> - while (inm != NULL) {
> - if (inm->inm_ia->ia_ifp == ifp &&
> - !IN_LOCAL_GROUP(inm->inm_addr.s_addr) &&
> + IN_FOREACH_MULTI(ia, ifp, inm) {
> + if (!IN_LOCAL_GROUP(inm->inm_addr.s_addr) &&
> (ip->ip_dst.s_addr == INADDR_ALLHOSTS_GROUP
> ||
> ip->ip_dst.s_addr ==
> inm->inm_addr.s_addr)) {
> switch (inm->inm_state) {
> @@ -323,7 +318,6 @@ igmp_input(struct mbuf *m, ...)
> break;
> }
> }
> - IN_NEXT_MULTI(step, inm);
> }
> }
>
> @@ -505,8 +499,7 @@ igmp_leavegroup(struct in_multi *inm)
> void
> igmp_fasttimo(void)
> {
> - struct in_multi *inm;
> - struct in_multistep step;
> + struct ifnet *ifp;
> int s;
>
> /*
> @@ -518,8 +511,21 @@ igmp_fasttimo(void)
>
> s = splsoftnet();
> igmp_timers_are_running = 0;
> - IN_FIRST_MULTI(step, inm);
> - while (inm != NULL) {
> + TAILQ_FOREACH(ifp, &ifnet, if_list)
> + igmp_checktimer(ifp);
> + splx(s);
> +}
> +
> +
> +void
> +igmp_checktimer(struct ifnet *ifp)
> +{
> + struct in_multi *inm;
> + struct in_ifaddr *ia;
> +
> + splsoftassert(IPL_SOFTNET);
> +
> + IN_FOREACH_MULTI(ia, ifp, inm) {
> if (inm->inm_timer == 0) {
> /* do nothing */
> } else if (--inm->inm_timer == 0) {
> @@ -535,9 +541,7 @@ igmp_fasttimo(void)
> } else {
> igmp_timers_are_running = 1;
> }
> - IN_NEXT_MULTI(step, inm);
> }
> - splx(s);
> }
>
> void
> Index: netinet/in_var.h
> ===================================================================
> RCS file: /home/ncvs/src/sys/netinet/in_var.h,v
> retrieving revision 1.21
> diff -u -p -r1.21 in_var.h
> --- netinet/in_var.h 28 Aug 2013 21:19:16 -0000 1.21
> +++ netinet/in_var.h 13 Sep 2013 09:07:42 -0000
> @@ -147,17 +147,21 @@ struct in_multi {
>
> #ifdef _KERNEL
> /*
> - * Structure used by macros below to remember position when stepping through
> - * all of the in_multi records.
> + * Macro for iterating over all the in_multi records linked to a given
> + * interface.
> */
> -struct in_multistep {
> - struct in_ifaddr *i_ia;
> - struct in_multi *i_inm;
> -};
> +#define IN_FOREACH_MULTI(ia, ifp, inm)
> \
> + /* struct in_ifaddr *ia; */ \
> + /* struct ifnet *ifp; */ \
> + /* struct in_multi *inm; */ \
> + IFP_TO_IA((ifp), ia); \
> + if (ia != NULL) \
> + LIST_FOREACH((inm), &ia->ia_multiaddrs, inm_list) \
>
> /*
> - * Macro for looking up the in_multi record for a given IP multicast address
> - * on a given interface. If no matching record is found, "inm" returns NULL.
> + * Macro for looking up the in_multi record for a given IP multicast
> + * address on a given interface. If no matching record is found, "inm"
> + * returns NULL.
> */
> #define IN_LOOKUP_MULTI(addr, ifp, inm)
> \
> /* struct in_addr addr; */ \
> @@ -166,48 +170,10 @@ struct in_multistep {
> do { \
> struct in_ifaddr *ia; \
> \
> - IFP_TO_IA((ifp), ia); \
> - if (ia == NULL) \
> - (inm) = NULL; \
> - else \
> - for ((inm) = LIST_FIRST(&ia->ia_multiaddrs); \
> - (inm) != NULL && \
> - (inm)->inm_addr.s_addr != (addr).s_addr; \
> - (inm) = LIST_NEXT(inm, inm_list)) \
> - continue; \
> -} while (/* CONSTCOND */ 0)
> -
> -/*
> - * Macro to step through all of the in_multi records, one at a time.
> - * The current position is remembered in "step", which the caller must
> - * provide. IN_FIRST_MULTI(), below, must be called to initialize "step"
> - * and get the first record. Both macros return a NULL "inm" when there
> - * are no remaining records.
> - */
> -#define IN_NEXT_MULTI(step, inm) \
> - /* struct in_multistep step; */ \
> - /* struct in_multi *inm; */ \
> -do { \
> - if (((inm) = (step).i_inm) != NULL) \
> - (step).i_inm = LIST_NEXT((inm), inm_list); \
> - else \
> - while ((step).i_ia != NULL) { \
> - (inm) = LIST_FIRST(&(step).i_ia->ia_multiaddrs); \
> - (step).i_ia = TAILQ_NEXT((step).i_ia, ia_list); \
> - if ((inm) != NULL) { \
> - (step).i_inm = LIST_NEXT((inm), inm_list); \
> - break; \
> - } \
> - } \
> -} while (/* CONSTCOND */ 0)
> -
> -#define IN_FIRST_MULTI(step, inm) \
> - /* struct in_multistep step; */ \
> - /* struct in_multi *inm; */ \
> -do { \
> - (step).i_ia = TAILQ_FIRST(&in_ifaddr); \
> - (step).i_inm = NULL; \
> - IN_NEXT_MULTI((step), (inm)); \
> + (inm) = NULL; \
> + IN_FOREACH_MULTI(ia, ifp, inm) \
> + if ((inm)->inm_addr.s_addr == (addr).s_addr) \
> + break; \
> } while (/* CONSTCOND */ 0)
>
> int in_ifinit(struct ifnet *,
> Index: netinet6/in6_var.h
> ===================================================================
> RCS file: /home/ncvs/src/sys/netinet6/in6_var.h,v
> retrieving revision 1.41
> diff -u -p -r1.41 in6_var.h
> --- netinet6/in6_var.h 26 Aug 2013 07:15:58 -0000 1.41
> +++ netinet6/in6_var.h 13 Sep 2013 09:07:42 -0000
> @@ -502,70 +502,34 @@ struct in6_multi {
>
> #ifdef _KERNEL
> /*
> - * Structure used by macros below to remember position when stepping through
> - * all of the in6_multi records.
> + * Macro for iterating over all the in6_multi records linked to a given
> + * interface.
> */
> -struct in6_multistep {
> - struct in6_ifaddr *i_ia;
> - struct in6_multi *i_in6m;
> -};
> +#define IN6_FOREACH_MULTI(ia, ifp, in6m) \
> + /* struct in6_ifaddr *ia; */ \
> + /* struct ifnet *ifp; */ \
> + /* struct in6_multi *in6m; */ \
> + IFP_TO_IA6((ifp), ia); \
> + if (ia != NULL) \
> + LIST_FOREACH((in6m), &ia->ia6_multiaddrs, in6m_entry) \
>
> /*
> * Macros for looking up the in6_multi record for a given IP6 multicast
> * address on a given interface. If no matching record is found, "in6m"
> - * returns NLL.
> + * returns NULL.
> */
> -
> -#define IN6_LOOKUP_MULTI(addr, ifp, in6m) \
> -/* struct in6_addr addr; */ \
> -/* struct ifnet *ifp; */ \
> -/* struct in6_multi *in6m; */ \
> -do { \
> - struct in6_ifaddr *ia; \
> - \
> - IFP_TO_IA6((ifp), ia); \
> - if (ia == NULL) \
> - (in6m) = NULL; \
> - else \
> - for ((in6m) = LIST_FIRST(&ia->ia6_multiaddrs); \
> - (in6m) != NULL && \
> - !IN6_ARE_ADDR_EQUAL(&(in6m)->in6m_addr, &(addr)); \
> - (in6m) = LIST_NEXT((in6m), in6m_entry)) \
> - continue; \
> -} while (0)
> -
> -/*
> - * Macro to step through all of the in6_multi records, one at a time.
> - * The current position is remembered in "step", which the caller must
> - * provide. IN6_FIRST_MULTI(), below, must be called to initialize "step"
> - * and get the first record. Both macros return a NULL "in6m" when there
> - * are no remaining records.
> - */
> -#define IN6_NEXT_MULTI(step, in6m) \
> -/* struct in6_multistep step; */ \
> -/* struct in6_multi *in6m; */
> \
> +#define IN6_LOOKUP_MULTI(addr, ifp, in6m) \
> + /* struct in6_addr addr; */ \
> + /* struct ifnet *ifp; */ \
> + /* struct in6_multi *in6m; */ \
> do { \
> - if (((in6m) = (step).i_in6m) != NULL) \
> - (step).i_in6m = LIST_NEXT((in6m), in6m_entry); \
> - else \
> - while ((step).i_ia != NULL) { \
> - (in6m) = LIST_FIRST(&(step).i_ia->ia6_multiaddrs); \
> - (step).i_ia = TAILQ_NEXT((step).i_ia, ia_list); \
> - if ((in6m) != NULL) { \
> - (step).i_in6m = LIST_NEXT((in6m), in6m_entry); \
> - break; \
> - } \
> - } \
> -} while (0)
> -
> -#define IN6_FIRST_MULTI(step, in6m) \
> -/* struct in6_multistep step; */ \
> -/* struct in6_multi *in6m */ \
> -do { \
> - (step).i_ia = TAILQ_FIRST(&in6_ifaddr); \
> - (step).i_in6m = NULL; \
> - IN6_NEXT_MULTI((step), (in6m)); \
> -} while (0)
> + struct in6_ifaddr *ia; \
> + \
> + (in6m) = NULL; \
> + IN6_FOREACH_MULTI(ia, ifp, in6m) \
> + if (IN6_ARE_ADDR_EQUAL(&(in6m)->in6m_addr, &(addr))) \
> + break; \
> +} while (/* CONSTCOND */ 0)
>
> struct in6_multi *in6_addmulti(struct in6_addr *, struct ifnet *, int
> *);
> void in6_delmulti(struct in6_multi *);
> Index: netinet6/mld6.c
> ===================================================================
> RCS file: /home/ncvs/src/sys/netinet6/mld6.c,v
> retrieving revision 1.28
> diff -u -p -r1.28 mld6.c
> --- netinet6/mld6.c 24 Nov 2011 17:39:55 -0000 1.28
> +++ netinet6/mld6.c 13 Sep 2013 09:07:42 -0000
> @@ -89,6 +89,7 @@ static int mld_timers_are_running;
> static struct in6_addr mld_all_nodes_linklocal =
> IN6ADDR_LINKLOCAL_ALLNODES_INIT;
> static struct in6_addr mld_all_routers_linklocal =
> IN6ADDR_LINKLOCAL_ALLROUTERS_INIT;
>
> +void mld6_checktimer(struct ifnet *);
> static void mld6_sendpkt(struct in6_multi *, int, const struct in6_addr *);
>
> void
> @@ -317,10 +318,9 @@ mld6_input(struct mbuf *m, int off)
> }
>
> void
> -mld6_fasttimeo()
> +mld6_fasttimeo(void)
> {
> - struct in6_multi *in6m;
> - struct in6_multistep step;
> + struct ifnet *ifp;
> int s;
>
> /*
> @@ -332,8 +332,20 @@ mld6_fasttimeo()
>
> s = splsoftnet();
> mld_timers_are_running = 0;
> - IN6_FIRST_MULTI(step, in6m);
> - while (in6m != NULL) {
> + TAILQ_FOREACH(ifp, &ifnet, if_list)
> + mld6_checktimer(ifp);
> + splx(s);
> +}
> +
> +void
> +mld6_checktimer(struct ifnet *ifp)
> +{
> + struct in6_multi *in6m;
> + struct in6_ifaddr *ia; \
> +
> + splsoftassert(IPL_SOFTNET);
> +
> + IN6_FOREACH_MULTI(ia, ifp, in6m) {
> if (in6m->in6m_timer == 0) {
> /* do nothing */
> } else if (--in6m->in6m_timer == 0) {
> @@ -342,9 +354,7 @@ mld6_fasttimeo()
> } else {
> mld_timers_are_running = 1;
> }
> - IN6_NEXT_MULTI(step, in6m);
> }
> - splx(s);
> }
>
> static void
>
