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 ?
>
> 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.
