Message ID | 1540521054-17825-1-git-send-email-liuhangbin@gmail.com |
---|---|
State | Changes Requested, archived |
Delegated to: | David Miller |
Headers | show |
Series | [net] ipv6/mcast: update mc_qrv when join new group | expand |
From: Hangbin Liu <liuhangbin@gmail.com> Date: Fri, 26 Oct 2018 10:30:54 +0800 > Currently we only set mc_qrv to sysctl_mld_qrv when interface up. If we > change sysctl_mld_qrv after interface up, it will has no effect. > > Fix it by assigning latest sysctl_mld_qrv to idev mc_qrv when join new group. > > Reported-by: Ying Xu <yinxu@redhat.com> > Signed-off-by: Hangbin Liu <liuhangbin@gmail.com> Why isn't mld_update_qrv() taking care of this? At a minimum, you must make your change take MLD_QRV_DEFAULT etc. into account like mld_update_qrv() does.
On Fri, Oct 26, 2018 at 10:30:54AM +0800, Hangbin Liu wrote: > Currently we only set mc_qrv to sysctl_mld_qrv when interface up. If we > change sysctl_mld_qrv after interface up, it will has no effect. > > Fix it by assigning latest sysctl_mld_qrv to idev mc_qrv when join new group. > > Reported-by: Ying Xu <yinxu@redhat.com> > Signed-off-by: Hangbin Liu <liuhangbin@gmail.com> Hi David, Any comments for this patch? Thanks Hangbin > --- > net/ipv6/mcast.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c > index dbab62e..bed4890 100644 > --- a/net/ipv6/mcast.c > +++ b/net/ipv6/mcast.c > @@ -680,6 +680,7 @@ static void igmp6_group_added(struct ifmcaddr6 *mc) > if (!(dev->flags & IFF_UP) || (mc->mca_flags & MAF_NOREPORT)) > return; > > + mc->idev->mc_qrv = sysctl_mld_qrv; > if (mld_in_v1_mode(mc->idev)) { > igmp6_join_group(mc); > return; > -- > 2.5.5 >
From: Hangbin Liu <liuhangbin@gmail.com> Date: Thu, 8 Nov 2018 15:44:10 +0800 > On Fri, Oct 26, 2018 at 10:30:54AM +0800, Hangbin Liu wrote: >> Currently we only set mc_qrv to sysctl_mld_qrv when interface up. If we >> change sysctl_mld_qrv after interface up, it will has no effect. >> >> Fix it by assigning latest sysctl_mld_qrv to idev mc_qrv when join new group. >> >> Reported-by: Ying Xu <yinxu@redhat.com> >> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com> > > Hi David, > > Any comments for this patch? I did give you feedback and you never responded: https://patchwork.ozlabs.org/patch/989422/ So I tossed your patch.
On Thu, Nov 08, 2018 at 03:44:10PM +0800, Hangbin Liu wrote: > On Fri, Oct 26, 2018 at 10:30:54AM +0800, Hangbin Liu wrote: > > Currently we only set mc_qrv to sysctl_mld_qrv when interface up. If we > > change sysctl_mld_qrv after interface up, it will has no effect. > > > > Fix it by assigning latest sysctl_mld_qrv to idev mc_qrv when join new group. > > > > Reported-by: Ying Xu <yinxu@redhat.com> > > Signed-off-by: Hangbin Liu <liuhangbin@gmail.com> > > Hi David, > > Any comments for this patch? > > --- > > net/ipv6/mcast.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c > > index dbab62e..bed4890 100644 > > --- a/net/ipv6/mcast.c > > +++ b/net/ipv6/mcast.c > > @@ -680,6 +680,7 @@ static void igmp6_group_added(struct ifmcaddr6 *mc) > > if (!(dev->flags & IFF_UP) || (mc->mca_flags & MAF_NOREPORT)) > > return; > > > > + mc->idev->mc_qrv = sysctl_mld_qrv; > > if (mld_in_v1_mode(mc->idev)) { > > igmp6_join_group(mc); > > return; Hi David, Sorry for the late reply. Today I find your reply in https://patchwork.ozlabs.org/patch/989422/, but I could not find your reply in my local mailbox, even on gmail web side. I can receive other kernel mails correctly, which is weird...I will check it later. For your question: > Why isn't mld_update_qrv() taking care of this? mld_update_qrv() is used for processing recieved mldv2 query message. This patch is to init new sysctl_mld_qrv for new joined groups, which haven't received any mld query message. e.g. - ipv6_mc_up() - ipv6_mc_reset() set idev->mc_qrv to default sysctl_mld_qrv, which is 2 - Join group A with qrv = 2 - user set sysctl_mld_qrv to 3 - Join group B with qrv = 2 <- Here the new group should set qrv to 3 as user updated sysctl_mld_qrv. This is the issue this patch want to fix - Recived MLDv2 query message - Group A, B update new qrv via mld_update_qrv() > At a minimum, you must make your change take MLD_QRV_DEFAULT etc. > into account like mld_update_qrv() does. We should not init mc_qrv to min(MLD_QRV_DEFAULT, sysctl_mld_qrv), or user will never be able to set mc_qrv greater than 2. Thanks Hangbin
On Wed, Nov 14, 2018 at 10:37:31AM +0800, Hangbin Liu wrote: > On Thu, Nov 08, 2018 at 03:44:10PM +0800, Hangbin Liu wrote: > > On Fri, Oct 26, 2018 at 10:30:54AM +0800, Hangbin Liu wrote: > > > Currently we only set mc_qrv to sysctl_mld_qrv when interface up. If we > > > change sysctl_mld_qrv after interface up, it will has no effect. > > > > > > Fix it by assigning latest sysctl_mld_qrv to idev mc_qrv when join new group. > > > > > > Reported-by: Ying Xu <yinxu@redhat.com> > > > Signed-off-by: Hangbin Liu <liuhangbin@gmail.com> > > > > Hi David, > > > > Any comments for this patch? > > > --- > > > net/ipv6/mcast.c | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c > > > index dbab62e..bed4890 100644 > > > --- a/net/ipv6/mcast.c > > > +++ b/net/ipv6/mcast.c > > > @@ -680,6 +680,7 @@ static void igmp6_group_added(struct ifmcaddr6 *mc) > > > if (!(dev->flags & IFF_UP) || (mc->mca_flags & MAF_NOREPORT)) > > > return; > > > > > > + mc->idev->mc_qrv = sysctl_mld_qrv; > > > if (mld_in_v1_mode(mc->idev)) { > > > igmp6_join_group(mc); > > > return; > > Hi David, > > Sorry for the late reply. Today I find your reply in > https://patchwork.ozlabs.org/patch/989422/, but I could not find your reply > in my local mailbox, even on gmail web side. I can receive other kernel > mails correctly, which is weird...I will check it later. > > For your question: > > > Why isn't mld_update_qrv() taking care of this? > > mld_update_qrv() is used for processing recieved mldv2 query message. This > patch is to init new sysctl_mld_qrv for new joined groups, which haven't > received any mld query message. e.g. > > - ipv6_mc_up() - ipv6_mc_reset() set idev->mc_qrv to default sysctl_mld_qrv, which is 2 > - Join group A with qrv = 2 > - user set sysctl_mld_qrv to 3 > - Join group B with qrv = 2 <- Here the new group should set qrv to 3 as > user updated sysctl_mld_qrv. This is the > issue this patch want to fix > - Recived MLDv2 query message > - Group A, B update new qrv via mld_update_qrv() > > > > At a minimum, you must make your change take MLD_QRV_DEFAULT etc. > > into account like mld_update_qrv() does. > > We should not init mc_qrv to min(MLD_QRV_DEFAULT, sysctl_mld_qrv), or > user will never be able to set mc_qrv greater than 2. > Hi David, Any comments? Should I re-send a new patch? Thanks Hangbin
diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c index dbab62e..bed4890 100644 --- a/net/ipv6/mcast.c +++ b/net/ipv6/mcast.c @@ -680,6 +680,7 @@ static void igmp6_group_added(struct ifmcaddr6 *mc) if (!(dev->flags & IFF_UP) || (mc->mca_flags & MAF_NOREPORT)) return; + mc->idev->mc_qrv = sysctl_mld_qrv; if (mld_in_v1_mode(mc->idev)) { igmp6_join_group(mc); return;
Currently we only set mc_qrv to sysctl_mld_qrv when interface up. If we change sysctl_mld_qrv after interface up, it will has no effect. Fix it by assigning latest sysctl_mld_qrv to idev mc_qrv when join new group. Reported-by: Ying Xu <yinxu@redhat.com> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com> --- net/ipv6/mcast.c | 1 + 1 file changed, 1 insertion(+)