Re: Multicast macros and global list of addresses
On 02/10/13(Wed) 21:33, Stuart Henderson wrote: On 2013/09/19 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? No issues here with sapWatch, igmpproxy, dhcpd multicast sync, or typical ipv6 use. I haven't had chance to try ospfd/ospf6d yet though. Thanks for testing. Unless somebody object, I plan to commit this diff next week, but oks are still welcome ;) Martin
Re: Multicast macros and global list of addresses
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.c2 May 2013 11:54:10 - 1.33
+++ netinet/igmp.c13 Sep 2013 09:07:42 -
@@ -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 {
Re: Multicast macros and global list of addresses
On 2013/09/19 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? No issues here with sapWatch, igmpproxy, dhcpd multicast sync, or typical ipv6 use. I haven't had chance to try ospfd/ospf6d yet though.
Re: Multicast macros and global list of addresses
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.c2 May 2013 11:54:10 - 1.33
+++ netinet/igmp.c13 Sep 2013 09:07:42 -
@@ -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 - 1.21
+++
Re: Multicast macros and global list of addresses
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.c2 May 2013 11:54:10 - 1.33
+++ netinet/igmp.c13 Sep 2013 09:07:42 -
@@ -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
Multicast macros and global list of addresses
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?
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 - 1.33
+++ netinet/igmp.c 13 Sep 2013 09:07:42 -
@@ -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.h28 Aug 2013 21:19:16 - 1.21
+++ netinet/in_var.h13 Sep 2013 09:07:42 -
@@ -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
