[PATCH] bonding: fix to rejoin multicast groups immediately

[PATCH] bonding: fix to rejoin multicast groups immediately

[Flavio Leitner]

It should rejoin multicast groups immediately when
the failover happens to restore the multicast traffic.

Signed-off-by: Flavio Leitner <fleitner@redhat.com>
---
 net/ipv4/igmp.c |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
index 1fdcacd..b81d674 100644
--- a/net/ipv4/igmp.c
+++ b/net/ipv4/igmp.c
@@ -1257,14 +1257,14 @@ void ip_mc_rejoin_group(struct ip_mc_list *im)
 	if (im->multiaddr == IGMP_ALL_HOSTS)
 		return;
 
-	if (IGMP_V1_SEEN(in_dev) || IGMP_V2_SEEN(in_dev)) {
-		igmp_mod_timer(im, IGMP_Initial_Report_Delay);
-		return;
-	}
-	/* else, v3 */
-	im->crcount = in_dev->mr_qrv ? in_dev->mr_qrv :
-		IGMP_Unsolicited_Report_Count;
-	igmp_ifc_event(in_dev);
+	/* a failover is happening and switches
+	 * must be notified immediately */
+	if (IGMP_V1_SEEN(in_dev))
+		igmp_send_report(in_dev, im, IGMP_HOST_MEMBERSHIP_REPORT);
+	else if (IGMP_V2_SEEN(in_dev))
+		igmp_send_report(in_dev, im, IGMPV2_HOST_MEMBERSHIP_REPORT);
+	else
+		igmp_send_report(in_dev, im, IGMPV3_HOST_MEMBERSHIP_REPORT);
 #endif
 }
 EXPORT_SYMBOL(ip_mc_rejoin_group);
-- 
1.7.2.3

Re: [PATCH] bonding: fix to rejoin multicast groups immediately

[Flavio Leitner]

forgot CC to bonding-devel@lists.sourceforge.net

On Wed, Sep 29, 2010 at 04:12:07AM -0300, Flavio Leitner wrote:
> It should rejoin multicast groups immediately when
> the failover happens to restore the multicast traffic.
> 
> Signed-off-by: Flavio Leitner <fleitner@redhat.com>
> ---
>  net/ipv4/igmp.c |   16 ++++++++--------
>  1 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
> index 1fdcacd..b81d674 100644
> --- a/net/ipv4/igmp.c
> +++ b/net/ipv4/igmp.c
> @@ -1257,14 +1257,14 @@ void ip_mc_rejoin_group(struct ip_mc_list *im)
>  	if (im->multiaddr == IGMP_ALL_HOSTS)
>  		return;
>  
> -	if (IGMP_V1_SEEN(in_dev) || IGMP_V2_SEEN(in_dev)) {
> -		igmp_mod_timer(im, IGMP_Initial_Report_Delay);
> -		return;
> -	}
> -	/* else, v3 */
> -	im->crcount = in_dev->mr_qrv ? in_dev->mr_qrv :
> -		IGMP_Unsolicited_Report_Count;
> -	igmp_ifc_event(in_dev);
> +	/* a failover is happening and switches
> +	 * must be notified immediately */
> +	if (IGMP_V1_SEEN(in_dev))
> +		igmp_send_report(in_dev, im, IGMP_HOST_MEMBERSHIP_REPORT);
> +	else if (IGMP_V2_SEEN(in_dev))
> +		igmp_send_report(in_dev, im, IGMPV2_HOST_MEMBERSHIP_REPORT);
> +	else
> +		igmp_send_report(in_dev, im, IGMPV3_HOST_MEMBERSHIP_REPORT);
>  #endif
>  }
>  EXPORT_SYMBOL(ip_mc_rejoin_group);
> -- 
> 1.7.2.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Flavio

Re: [PATCH] bonding: fix to rejoin multicast groups immediately

[David Miller]

From: Flavio Leitner <fleitner@redhat.com>
Date: Wed, 29 Sep 2010 04:12:07 -0300

> It should rejoin multicast groups immediately when
> the failover happens to restore the multicast traffic.
> 
> Signed-off-by: Flavio Leitner <fleitner@redhat.com>

I suspect the IGMPv3 handling via a delayed action, as is currently
implemented, is on purpose and is done so to follow the specification
of the IGMPv3 RFCs.

Therefore you have to explain why your new behavior is so desirable
and in particular why something as undesirable as violating the RFCs
is therefore warranted.

Re: [PATCH] bonding: fix to rejoin multicast groups immediately

[Flavio Leitner]

On Tue, Oct 05, 2010 at 12:13:38AM -0700, David Miller wrote:
> From: Flavio Leitner <fleitner@redhat.com>
> Date: Wed, 29 Sep 2010 04:12:07 -0300
> 
> > It should rejoin multicast groups immediately when
> > the failover happens to restore the multicast traffic.
> > 
> > Signed-off-by: Flavio Leitner <fleitner@redhat.com>
> 
> I suspect the IGMPv3 handling via a delayed action, as is currently
> implemented, is on purpose and is done so to follow the specification
> of the IGMPv3 RFCs.
> 
> Therefore you have to explain why your new behavior is so desirable
> and in particular why something as undesirable as violating the RFCs
> is therefore warranted.

That patch only changes the behavior for bonding during a link
failure, so if we have a bonding in active-backup or any other mode
with current-active-slave, the initialization will happen just fine
following IGMP specs.

However, neither the backup slave interface nor the backup switch
connected to backup slave knows about mcast. Thus when a link failure
happens, we shouldn't rely on timers to not stay out of the mcast
group losing traffic.

E.g. The V1 specs says that we shouldn't send any membership report
if it has been one in the last minute because that means the switch
is notified and the system will receive mcast traffic for that group. 
Therefore, if it sees one and a link failure happens right after that,
the backup slave will send another membership report only one minute
later. During this time the system loses traffic.

-- 
Flavio

Re: [PATCH] bonding: fix to rejoin multicast groups immediately

[David Miller]

From: Flavio Leitner <fbl@redhat.com>
Date: Tue, 5 Oct 2010 11:34:30 -0300

> On Tue, Oct 05, 2010 at 12:13:38AM -0700, David Miller wrote:
>> From: Flavio Leitner <fleitner@redhat.com>
>> Date: Wed, 29 Sep 2010 04:12:07 -0300
>> 
>> > It should rejoin multicast groups immediately when
>> > the failover happens to restore the multicast traffic.
>> > 
>> > Signed-off-by: Flavio Leitner <fleitner@redhat.com>
>> 
>> I suspect the IGMPv3 handling via a delayed action, as is currently
>> implemented, is on purpose and is done so to follow the specification
>> of the IGMPv3 RFCs.
>> 
>> Therefore you have to explain why your new behavior is so desirable
>> and in particular why something as undesirable as violating the RFCs
>> is therefore warranted.
> 
> That patch only changes the behavior for bonding during a link
> failure, so if we have a bonding in active-backup or any other mode
> with current-active-slave, the initialization will happen just fine
> following IGMP specs.
> 
> However, neither the backup slave interface nor the backup switch
> connected to backup slave knows about mcast. Thus when a link failure
> happens, we shouldn't rely on timers to not stay out of the mcast
> group losing traffic.
> 
> E.g. The V1 specs says that we shouldn't send any membership report
> if it has been one in the last minute because that means the switch
> is notified and the system will receive mcast traffic for that group. 
> Therefore, if it sees one and a link failure happens right after that,
> the backup slave will send another membership report only one minute
> later. During this time the system loses traffic.

All of this valuable information belongs in the commit log message.

Please resubmit your patch with all of the necessary contextual
information and reasoning explaining in the commit message.

Thanks.

Re: [PATCH] bonding: fix to rejoin multicast groups immediately

[Flavio Leitner]

On Tue, Oct 05, 2010 at 01:25:25PM -0700, David Miller wrote:
> From: Flavio Leitner <fbl@redhat.com>
> Date: Tue, 5 Oct 2010 11:34:30 -0300
> 
> > On Tue, Oct 05, 2010 at 12:13:38AM -0700, David Miller wrote:
> >> From: Flavio Leitner <fleitner@redhat.com>
> >> Date: Wed, 29 Sep 2010 04:12:07 -0300
> >> 
> >> > It should rejoin multicast groups immediately when
> >> > the failover happens to restore the multicast traffic.
> >> > 
> >> > Signed-off-by: Flavio Leitner <fleitner@redhat.com>
> >> 
> >> I suspect the IGMPv3 handling via a delayed action, as is currently
> >> implemented, is on purpose and is done so to follow the specification
> >> of the IGMPv3 RFCs.
> >> 
> >> Therefore you have to explain why your new behavior is so desirable
> >> and in particular why something as undesirable as violating the RFCs
> >> is therefore warranted.
> > 
> > That patch only changes the behavior for bonding during a link
> > failure, so if we have a bonding in active-backup or any other mode
> > with current-active-slave, the initialization will happen just fine
> > following IGMP specs.
> > 
> > However, neither the backup slave interface nor the backup switch
> > connected to backup slave knows about mcast. Thus when a link failure
> > happens, we shouldn't rely on timers to not stay out of the mcast
> > group losing traffic.
> > 
> > E.g. The V1 specs says that we shouldn't send any membership report
> > if it has been one in the last minute because that means the switch
> > is notified and the system will receive mcast traffic for that group. 
> > Therefore, if it sees one and a link failure happens right after that,
> > the backup slave will send another membership report only one minute
> > later. During this time the system loses traffic.
> 
> All of this valuable information belongs in the commit log message.
> 
> Please resubmit your patch with all of the necessary contextual
> information and reasoning explaining in the commit message.

Sure. I'll post replying to another thread so that the patch
can be applied in correct order.

thanks for reviewing it
-- 
Flavio

Re: [PATCH] bonding: fix to rejoin multicast groups immediately

[David Miller]

From: Flavio Leitner <fbl@redhat.com>
Date: Tue, 5 Oct 2010 19:04:36 -0300

> Sure. I'll post replying to another thread so that the patch
> can be applied in correct order.
> 
> thanks for reviewing it

No problem, thank you.