On 01/10/13(Tue) 19:53, Loganaden Velvindron wrote:
> On Tue, Oct 1, 2013 at 3:33 PM, Martin Pieuchot <mpieuc...@nolizard.org>
> wrote:
> > 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?
>
> What kind of test coverage are you looking for specifically ?
Anything that uses multicast (8 Make sure it doesn't break your setup.
> >
> > 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
> >>
> >
>
>
>
> --
> This message is strictly personal and the opinions expressed do not
> represent those of my employers, either past or present.
>
