Hi.
I think ip6_mc_add_src(...) should be called when number of sources is zero and filter mode is exclude.
Regards
Signed-off-by: Yan Zheng <[email protected]>
Index: net/ipv6/mcast.c
================================================================================
--- linux-2.6.14/net/ipv6/mcast.c 2005-10-30 23:09:33.000000000 +0800
+++ linux/net/ipv6/mcast.c 2005-10-31 10:37:36.000000000 +0800
@@ -545,8 +545,10 @@ int ip6_mc_msfilter(struct sock *sk, str
sock_kfree_s(sk, newpsl, IP6_SFLSIZE(newpsl->sl_max));
goto done;
}
- } else
+ } else {
newpsl = NULL;
+ ip6_mc_add_src(idev, group, gsf->gf_fmode, 0, NULL, 0);
+ }
psl = pmc->sflist;
if (psl) {
(void) ip6_mc_del_src(idev, group, pmc->sfmode,
You can reproduce this bug by follow codes. This program will cause a
change to include report even though the first socket's filter mode is
exclude.
Please adjust IFINDEX first.
========================================
#include <stdio.h>
#include <unistd.h>
#include <netinet/in.h>
#include <arpa/inet.h>
#include <sys/socket.h>
#include <sys/types.h>
#define IFINDEX 6
int main(int argc, char argv[])
{
int i, sockfds[3];
struct ipv6_mreq req;
struct group_filter filter;
struct sockaddr_in6 *psin6;
req.ipv6mr_interface = IFINDEX;
inet_pton(PF_INET6, "FF02::2", &req.ipv6mr_multiaddr);
for (i = 0; i < 3; ++i) {
sockfds[i] = socket(PF_INET6, SOCK_DGRAM, 0);
setsockopt(sockfds[i], SOL_IPV6, IPV6_ADD_MEMBERSHIP, &req, sizeof(req));
}
filter.gf_interface = IFINDEX;
filter.gf_fmode = MCAST_INCLUDE;
filter.gf_numsrc = 1;
psin6 = (struct sockaddr_in6 *)&filter.gf_group;
psin6->sin6_family = AF_INET6;
inet_pton(PF_INET6, "FF02::2", &psin6->sin6_addr);
psin6 = (struct sockaddr_in6 *)&filter.gf_slist[0];
psin6->sin6_family = AF_INET6;
inet_pton(PF_INET6, "2002:de12:1780::1", &psin6->sin6_addr);
setsockopt(sockfds[1], SOL_IPV6, MCAST_MSFILTER, &filter, sizeof(filter));
filter.gf_fmode = MCAST_EXCLUDE;
filter.gf_numsrc = 0;
setsockopt(sockfds[2], SOL_IPV6, MCAST_MSFILTER, &filter, sizeof(filter));
setsockopt(sockfds[2], SOL_IPV6, MCAST_MSFILTER, &filter, sizeof(filter));
pause();
}
Yan,
Please also make this equivalent change in IPv4 with
ip_mc_msfilter() and ip_mc_add_src().
+-DLS
Acked-by: David L Stevens <[email protected]>
>
> Signed-off-by: Yan Zheng <[email protected]>
>
> Index: net/ipv6/mcast.c
>
================================================================================
> --- linux-2.6.14/net/ipv6/mcast.c 2005-10-30 23:09:33.000000000 +0800
> +++ linux/net/ipv6/mcast.c 2005-10-31 10:37:36.000000000 +0800
> @@ -545,8 +545,10 @@ int ip6_mc_msfilter(struct sock *sk, str
> sock_kfree_s(sk, newpsl, IP6_SFLSIZE(newpsl->sl_max));
> goto done;
> }
> - } else
> + } else {
> newpsl = NULL;
> + ip6_mc_add_src(idev, group, gsf->gf_fmode, 0, NULL, 0);
> + }
> psl = pmc->sflist;
> if (psl) {
> (void) ip6_mc_del_src(idev, group, pmc->sfmode,
> -
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
David Stevens wrote:
> Yan,
> Please also make this equivalent change in IPv4 with
> ip_mc_msfilter() and ip_mc_add_src().
>
> +-DLS
>
> Acked-by: David L Stevens <[email protected]>
To keep code style, I also create a new patch for IPv6. :-)
Signed-off-by: Yan Zheng <[email protected]>
Patch for IPv4
Index:net/ipv4/igmp.c
============================================================
--- linux-2.6.14/net/ipv4/igmp.c 2005-10-28 08:02:08.000000000 +0800
+++ linux/net/ipv4/igmp.c 2005-11-02 07:31:01.000000000 +0800
@@ -1908,8 +1908,11 @@ int ip_mc_msfilter(struct sock *sk, stru
sock_kfree_s(sk, newpsl, IP_SFLSIZE(newpsl->sl_max));
goto done;
}
- } else
+ } else {
newpsl = NULL;
+ (void) ip_mc_add_src(in_dev, &msf->imsf_multiaddr,
+ msf->imsf_fmode, 0, NULL, 0)
+ }
psl = pmc->sflist;
if (psl) {
(void) ip_mc_del_src(in_dev, &msf->imsf_multiaddr, pmc->sfmode,
Patch for IPv6
Index:net/ipv6/mcast.c
============================================================
--- linux-2.6.14/net/ipv6/mcast.c 2005-10-30 23:09:33.000000000 +0800
+++ linux/net/ipv6/mcast.c 2005-11-02 07:19:12.000000000 +0800
@@ -545,8 +545,10 @@ int ip6_mc_msfilter(struct sock *sk, str
sock_kfree_s(sk, newpsl, IP6_SFLSIZE(newpsl->sl_max));
goto done;
}
- } else
+ } else {
newpsl = NULL;
+ (void) ip6_mc_add_src(idev, group, gsf->gf_fmode, 0, NULL, 0);
+ }
psl = pmc->sflist;
if (psl) {
(void) ip6_mc_del_src(idev, group, pmc->sfmode,
Looks good. Thanks, Yan.
+-DLS
Acked-by: David L Stevens <[email protected]>
>
> Signed-off-by: Yan Zheng <[email protected]>
>
> Patch for IPv4
> Index:net/ipv4/igmp.c
> ============================================================
> --- linux-2.6.14/net/ipv4/igmp.c 2005-10-28 08:02:08.000000000 +0800
> +++ linux/net/ipv4/igmp.c 2005-11-02 07:31:01.000000000 +0800
> @@ -1908,8 +1908,11 @@ int ip_mc_msfilter(struct sock *sk, stru
> sock_kfree_s(sk, newpsl, IP_SFLSIZE(newpsl->sl_max));
> goto done;
> }
> - } else
> + } else {
> newpsl = NULL;
> + (void) ip_mc_add_src(in_dev, &msf->imsf_multiaddr,
> + msf->imsf_fmode, 0, NULL, 0)
> + }
> psl = pmc->sflist;
> if (psl) {
> (void) ip_mc_del_src(in_dev, &msf->imsf_multiaddr, pmc->sfmode,
>
>
>
>
> Patch for IPv6
> Index:net/ipv6/mcast.c
> ============================================================
> --- linux-2.6.14/net/ipv6/mcast.c 2005-10-30 23:09:33.000000000 +0800
> +++ linux/net/ipv6/mcast.c 2005-11-02 07:19:12.000000000 +0800
> @@ -545,8 +545,10 @@ int ip6_mc_msfilter(struct sock *sk, str
> sock_kfree_s(sk, newpsl, IP6_SFLSIZE(newpsl->sl_max));
> goto done;
> }
> - } else
> + } else {
> newpsl = NULL;
> + (void) ip6_mc_add_src(idev, group, gsf->gf_fmode, 0, NULL, 0);
> + }
> psl = pmc->sflist;
> if (psl) {
> (void) ip6_mc_del_src(idev, group, pmc->sfmode,
>
>
Hi,
On Tue, Nov 01, 2005 at 06:37:43PM +0800, Yan Zheng wrote:
> You can reproduce this bug by follow codes. This program will cause a
> change to include report even though the first socket's filter mode is
> exclude.
I've not clearly understood the nature of the bug, does it also
affect 2.4 ? Marcelo will be releasing 2.4.32 in a few days, so
it would be wise to merge the fix soon.
Regards,
Willy
Willy Tarreau <[email protected]> wrote on 11/01/2005 09:47:02 PM:
> Hi,
>
> On Tue, Nov 01, 2005 at 06:37:43PM +0800, Yan Zheng wrote:
> > You can reproduce this bug by follow codes. This program will cause a
> > change to include report even though the first socket's filter mode is
> > exclude.
>
> I've not clearly understood the nature of the bug, does it also
> affect 2.4 ? Marcelo will be releasing 2.4.32 in a few days, so
> it would be wise to merge the fix soon.
>
> Regards,
> Willy
>
Yes.
Multicast source filters aren't widely used yet, and that's
really the only feature that's affected if an application actually
exercises this bug, as far as I can tell. An ordinary filter-less
multicast join should still work, and only forwarded multicast
traffic making use of filters and doing empty-source filters with
the MSFILTER ioctl would be at risk of not getting multicast
traffic forwarded to them because the reports generated would not
be based on the correct counts.
But having the fix in is better than having it broken, even
for that case. :-)
+-DLS
On Tue, Nov 01, 2005 at 11:10:29PM -0800, David Stevens wrote:
> Willy Tarreau <[email protected]> wrote on 11/01/2005 09:47:02 PM:
>
> > Hi,
> >
> > On Tue, Nov 01, 2005 at 06:37:43PM +0800, Yan Zheng wrote:
> > > You can reproduce this bug by follow codes. This program will cause a
> > > change to include report even though the first socket's filter mode is
> > > exclude.
> >
> > I've not clearly understood the nature of the bug, does it also
> > affect 2.4 ? Marcelo will be releasing 2.4.32 in a few days, so
> > it would be wise to merge the fix soon.
> >
> > Regards,
> > Willy
> >
>
> Yes.
>
> Multicast source filters aren't widely used yet, and that's
> really the only feature that's affected if an application actually
> exercises this bug, as far as I can tell. An ordinary filter-less
> multicast join should still work, and only forwarded multicast
> traffic making use of filters and doing empty-source filters with
> the MSFILTER ioctl would be at risk of not getting multicast
> traffic forwarded to them because the reports generated would not
> be based on the correct counts.
> But having the fix in is better than having it broken, even
> for that case. :-)
OK, thanks for the clarification, I understand better now :-)
Marcelo, David, does this backport seem appropriate for 2.4.32 ? I verified
that it compiles, nothing more. If it's OK, I've noticed another patch that
Yan posted today and which might be of interest before a very solid release.
Regards,
Willy
diff -urN linux-2.4.32-rc2/net/ipv4/igmp.c linux-2.4.32-rc2-mcast/net/ipv4/igmp.c
--- linux-2.4.32-rc2/net/ipv4/igmp.c 2005-11-02 10:16:03.000000000 +0100
+++ linux-2.4.32-rc2-mcast/net/ipv4/igmp.c 2005-11-02 10:20:33.000000000 +0100
@@ -1876,8 +1876,11 @@
sock_kfree_s(sk, newpsl, IP_SFLSIZE(newpsl->sl_max));
goto done;
}
- } else
- newpsl = 0;
+ } else {
+ newpsl = NULL;
+ (void) ip_mc_add_src(in_dev, &msf->imsf_multiaddr,
+ msf->imsf_fmode, 0, NULL, 0);
+ }
psl = pmc->sflist;
if (psl) {
(void) ip_mc_del_src(in_dev, &msf->imsf_multiaddr, pmc->sfmode,
diff -urN linux-2.4.32-rc2/net/ipv6/mcast.c linux-2.4.32-rc2-mcast/net/ipv6/mcast.c
--- linux-2.4.32-rc2/net/ipv6/mcast.c 2005-11-02 10:16:03.000000000 +0100
+++ linux-2.4.32-rc2-mcast/net/ipv6/mcast.c 2005-11-02 10:21:49.000000000 +0100
@@ -505,8 +505,11 @@
sock_kfree_s(sk, newpsl, IP6_SFLSIZE(newpsl->sl_max));
goto done;
}
- } else
- newpsl = 0;
+ } else {
+ newpsl = NULL;
+ (void) ip6_mc_add_src(idev, group, gsf->gf_fmode, 0, NULL, 0);
+ }
+
psl = pmc->sflist;
if (psl) {
(void) ip6_mc_del_src(idev, group, pmc->sfmode,
Willy Tarreau <[email protected]> wrote on 11/02/2005 01:29:59 AM:
> Marcelo, David, does this backport seem appropriate for 2.4.32 ? I
verified
> that it compiles, nothing more.
Yes.
> If it's OK, I've noticed another patch that
> Yan posted today and which might be of interest before a very solid
release.
I think they should be reviewed first. :-)
+-DLS
On Wed, Nov 02, 2005 at 10:29:59AM +0100, Willy Tarreau wrote:
> On Tue, Nov 01, 2005 at 11:10:29PM -0800, David Stevens wrote:
> > Willy Tarreau <[email protected]> wrote on 11/01/2005 09:47:02 PM:
> >
> > > Hi,
> > >
> > > On Tue, Nov 01, 2005 at 06:37:43PM +0800, Yan Zheng wrote:
> > > > You can reproduce this bug by follow codes. This program will cause a
> > > > change to include report even though the first socket's filter mode is
> > > > exclude.
> > >
> > > I've not clearly understood the nature of the bug, does it also
> > > affect 2.4 ? Marcelo will be releasing 2.4.32 in a few days, so
> > > it would be wise to merge the fix soon.
> > >
> > > Regards,
> > > Willy
> > >
> >
> > Yes.
> >
> > Multicast source filters aren't widely used yet, and that's
> > really the only feature that's affected if an application actually
> > exercises this bug, as far as I can tell. An ordinary filter-less
> > multicast join should still work, and only forwarded multicast
> > traffic making use of filters and doing empty-source filters with
> > the MSFILTER ioctl would be at risk of not getting multicast
> > traffic forwarded to them because the reports generated would not
> > be based on the correct counts.
> > But having the fix in is better than having it broken, even
> > for that case. :-)
>
>
> OK, thanks for the clarification, I understand better now :-)
>
> Marcelo, David, does this backport seem appropriate for 2.4.32 ? I verified
> that it compiles, nothing more. If it's OK, I've noticed another patch that
> Yan posted today and which might be of interest before a very solid release.
Hi Willy,
Given the fact that it is a bug correction, sure.
Could you please prepare a nice e-mail with the full description?
Thanks
Hi Marcelo,
On Wed, Nov 02, 2005 at 11:41:12AM -0200, Marcelo Tosatti wrote:
> Given the fact that it is a bug correction, sure.
> Could you please prepare a nice e-mail with the full description?
Yes, I see... email was not nice enough to the scripts :-)
Here it is complete and clean below.
Regards,
Willy
[PATCH-2.4][MCAST]IPv6: small fix for ip6_mc_msfilter(...)
Multicast source filters aren't widely used yet, and that's really
the only feature that's affected if an application actually exercises
this bug, as far as I can tell. An ordinary filter-less multicast join
should still work, and only forwarded multicast traffic making use of
filters and doing empty-source filters with the MSFILTER ioctl would
be at risk of not getting multicast traffic forwarded to them because
the reports generated would not be based on the correct counts.
Initial 2.6 patch by Yan Zheng, bug explanation by David Stevens,
patch ACKed by David.
Signed-off-by: Willy Tarreau <[email protected]>
diff -urN linux-2.4.32-rc2/net/ipv4/igmp.c linux-2.4.32-rc2-mcast/net/ipv4/igmp.c
--- linux-2.4.32-rc2/net/ipv4/igmp.c 2005-11-02 10:16:03.000000000 +0100
+++ linux-2.4.32-rc2-mcast/net/ipv4/igmp.c 2005-11-02 10:20:33.000000000 +0100
@@ -1876,8 +1876,11 @@
sock_kfree_s(sk, newpsl, IP_SFLSIZE(newpsl->sl_max));
goto done;
}
- } else
- newpsl = 0;
+ } else {
+ newpsl = NULL;
+ (void) ip_mc_add_src(in_dev, &msf->imsf_multiaddr,
+ msf->imsf_fmode, 0, NULL, 0);
+ }
psl = pmc->sflist;
if (psl) {
(void) ip_mc_del_src(in_dev, &msf->imsf_multiaddr, pmc->sfmode,
diff -urN linux-2.4.32-rc2/net/ipv6/mcast.c linux-2.4.32-rc2-mcast/net/ipv6/mcast.c
--- linux-2.4.32-rc2/net/ipv6/mcast.c 2005-11-02 10:16:03.000000000 +0100
+++ linux-2.4.32-rc2-mcast/net/ipv6/mcast.c 2005-11-02 10:21:49.000000000 +0100
@@ -505,8 +505,11 @@
sock_kfree_s(sk, newpsl, IP6_SFLSIZE(newpsl->sl_max));
goto done;
}
- } else
- newpsl = 0;
+ } else {
+ newpsl = NULL;
+ (void) ip6_mc_add_src(idev, group, gsf->gf_fmode, 0, NULL, 0);
+ }
+
psl = pmc->sflist;
if (psl) {
(void) ip6_mc_del_src(idev, group, pmc->sfmode,
On 11/1/05, David Stevens <[email protected]> wrote:
> Looks good. Thanks, Yan.
>
> +-DLS
Thanks everybody, Applied.
- Arnaldo
On 11/2/05, David Stevens <[email protected]> wrote:
> Willy Tarreau <[email protected]> wrote on 11/02/2005 01:29:59 AM:
>
> > Marcelo, David, does this backport seem appropriate for 2.4.32 ? I
> verified
> > that it compiles, nothing more.
>
> Yes.
>
> > If it's OK, I've noticed another patch that
> > Yan posted today and which might be of interest before a very solid
> release.
>
> I think they should be reviewed first. :-)
Sure thing David, and thanks for your much appreciated review of patches for
the area.
- Arnaldo
On 11/1/05, Yan Zheng <[email protected]> wrote:
> David Stevens wrote:
> > Yan,
> > Please also make this equivalent change in IPv4 with
> > ip_mc_msfilter() and ip_mc_add_src().
> >
> > +-DLS
> >
> > Acked-by: David L Stevens <[email protected]>
>
> To keep code style, I also create a new patch for IPv6. :-)
>
> Signed-off-by: Yan Zheng <[email protected]>
>
> Patch for IPv4
> Index:net/ipv4/igmp.c
> ============================================================
> --- linux-2.6.14/net/ipv4/igmp.c 2005-10-28 08:02:08.000000000 +0800
> +++ linux/net/ipv4/igmp.c 2005-11-02 07:31:01.000000000 +0800
> @@ -1908,8 +1908,11 @@ int ip_mc_msfilter(struct sock *sk, stru
> sock_kfree_s(sk, newpsl, IP_SFLSIZE(newpsl->sl_max));
> goto done;
> }
> - } else
> + } else {
> newpsl = NULL;
> + (void) ip_mc_add_src(in_dev, &msf->imsf_multiaddr,
> + msf->imsf_fmode, 0, NULL, 0)
Could you please compile test it next time :-) hint, missing ';'.
Anyway, fixed up by hand.
- Arnaldo
>
> Could you please compile test it next time :-) hint, missing ';'.
> Anyway, fixed up by hand.
>
> - Arnaldo
>
>
I'm so sorry.
============================================================
--- linux-2.6.14/net/ipv4/igmp.c 2005-10-28 08:02:08.000000000 +0800
+++ linux/net/ipv4/igmp.c 2005-11-02 07:31:01.000000000 +0800
@@ -1908,8 +1908,11 @@ int ip_mc_msfilter(struct sock *sk, stru
sock_kfree_s(sk, newpsl, IP_SFLSIZE(newpsl->sl_max));
goto done;
}
- } else
+ } else {
newpsl = NULL;
+ (void) ip_mc_add_src(in_dev, &msf->imsf_multiaddr,
+ msf->imsf_fmode, 0, NULL, 0);
+ }
psl = pmc->sflist;
if (psl) {
(void) ip_mc_del_src(in_dev, &msf->imsf_multiaddr, pmc->sfmode,
On 11/2/05, Yan Zheng <[email protected]> wrote:
> >
> > Could you please compile test it next time :-) hint, missing ';'.
> > Anyway, fixed up by hand.
> >
> > - Arnaldo
> >
> >
>
> I'm so sorry.
Don't worry, I already fixed it and published in my net-2.6.git tree at
http://www.kernel.org, its just that the same confidence that makes one
commit something simple as in your case could introduce something
nasty :-)
- Arnaldo
On Wed, Nov 02, 2005 at 09:42:37AM -0800, David Stevens wrote:
> Willy Tarreau <[email protected]> wrote on 11/02/2005 01:29:59 AM:
>
> > Marcelo, David, does this backport seem appropriate for 2.4.32 ? I
> verified
> > that it compiles, nothing more.
>
> Yes.
>
> > If it's OK, I've noticed another patch that
> > Yan posted today and which might be of interest before a very solid
> release.
>
> I think they should be reviewed first. :-)
Alright, then let it be tested in v2.6 first. Willy, can you queue for
2.4.33-pre ?
On Wed, Nov 02, 2005 at 06:59:26PM -0200, Marcelo Tosatti wrote:
> On Wed, Nov 02, 2005 at 09:42:37AM -0800, David Stevens wrote:
> > Willy Tarreau <[email protected]> wrote on 11/02/2005 01:29:59 AM:
> >
> > > Marcelo, David, does this backport seem appropriate for 2.4.32 ? I
> > verified
> > > that it compiles, nothing more.
> >
> > Yes.
> >
> > > If it's OK, I've noticed another patch that
> > > Yan posted today and which might be of interest before a very solid
> > release.
> >
> > I think they should be reviewed first. :-)
>
> Alright, then let it be tested in v2.6 first. Willy, can you queue for
> 2.4.33-pre ?
OK.
Willy