From: David Miller <davem@davemloft.net>
To: linus.luessing@web.de
Cc: bridge@lists.linux-foundation.org, stephen@networkplumber.org,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
herbert@gondor.hengli.com.au, amwang@redhat.com,
linux@baker-net.org.uk
Subject: Re: [PATCH] bridge: separate querier and query timer into IGMP/IPv4 and MLD/IPv6 ones
Date: Wed, 28 Aug 2013 20:08:31 -0400 (EDT) [thread overview]
Message-ID: <20130828.200831.289304393401193657.davem@davemloft.net> (raw)
In-Reply-To: <1377511266-14530-1-git-send-email-linus.luessing@web.de>
From: Linus Lüssing <linus.luessing@web.de>
Date: Mon, 26 Aug 2013 12:01:06 +0200
> Currently we would still potentially suffer multicast packet loss if there
> is just either an IGMP or an MLD querier: For the former case, we would
> possibly drop IPv6 multicast packets, for the latter IPv4 ones. This is
> because we are currently assuming that if either an IGMP or MLD querier
> is present that the other one is present, too.
>
> This patch makes the behaviour and fix added in
> "bridge: disable snooping if there is no querier" (b00589af3b04)
> to also work if there is either just an IGMP or an MLD querier on the
> link: It refines the deactivation of the snooping to be protocol
> specific by using separate timers for the snooped IGMP and MLD queries
> as well as separate timers for our internal IGMP and MLD queriers.
>
> Signed-off-by: Linus Lüssing <linus.luessing@web.de>
I think the logic of the fix is fine, but theres a lot of duplication
here.
Why not make a:
struct bridge_mcast_timer {
struct timer_list timer;
u32 queries_sent;
u32 startup_queries_sent;
};
Then in the port structure you have:
struct bridge_mcast_timer ip4;
struct bridge_mcast_timer ip6;
And pass a reference to the right one into generic helper functions.
That sounds a lot better right?
Please make this improvement and resubmit, thanks.
WARNING: multiple messages have this Message-ID (diff)
From: David Miller <davem@davemloft.net>
To: linus.luessing@web.de
Cc: amwang@redhat.com, netdev@vger.kernel.org,
bridge@lists.linux-foundation.org, linux-kernel@vger.kernel.org,
linux@baker-net.org.uk, stephen@networkplumber.org,
herbert@gondor.apana.org.au
Subject: Re: [PATCH] bridge: separate querier and query timer into IGMP/IPv4 and MLD/IPv6 ones
Date: Wed, 28 Aug 2013 20:08:31 -0400 (EDT) [thread overview]
Message-ID: <20130828.200831.289304393401193657.davem@davemloft.net> (raw)
In-Reply-To: <1377511266-14530-1-git-send-email-linus.luessing@web.de>
From: Linus Lüssing <linus.luessing@web.de>
Date: Mon, 26 Aug 2013 12:01:06 +0200
> Currently we would still potentially suffer multicast packet loss if there
> is just either an IGMP or an MLD querier: For the former case, we would
> possibly drop IPv6 multicast packets, for the latter IPv4 ones. This is
> because we are currently assuming that if either an IGMP or MLD querier
> is present that the other one is present, too.
>
> This patch makes the behaviour and fix added in
> "bridge: disable snooping if there is no querier" (b00589af3b04)
> to also work if there is either just an IGMP or an MLD querier on the
> link: It refines the deactivation of the snooping to be protocol
> specific by using separate timers for the snooped IGMP and MLD queries
> as well as separate timers for our internal IGMP and MLD queriers.
>
> Signed-off-by: Linus Lüssing <linus.luessing@web.de>
I think the logic of the fix is fine, but theres a lot of duplication
here.
Why not make a:
struct bridge_mcast_timer {
struct timer_list timer;
u32 queries_sent;
u32 startup_queries_sent;
};
Then in the port structure you have:
struct bridge_mcast_timer ip4;
struct bridge_mcast_timer ip6;
And pass a reference to the right one into generic helper functions.
That sounds a lot better right?
Please make this improvement and resubmit, thanks.
WARNING: multiple messages have this Message-ID (diff)
From: David Miller <davem@davemloft.net>
To: linus.luessing@web.de
Cc: amwang@redhat.com, netdev@vger.kernel.org,
bridge@lists.linux-foundation.org, linux-kernel@vger.kernel.org,
linux@baker-net.org.uk, stephen@networkplumber.org,
herbert@gondor.apana.org.au
Subject: Re: [Bridge] [PATCH] bridge: separate querier and query timer into IGMP/IPv4 and MLD/IPv6 ones
Date: Wed, 28 Aug 2013 20:08:31 -0400 (EDT) [thread overview]
Message-ID: <20130828.200831.289304393401193657.davem@davemloft.net> (raw)
In-Reply-To: <1377511266-14530-1-git-send-email-linus.luessing@web.de>
From: Linus Lüssing <linus.luessing@web.de>
Date: Mon, 26 Aug 2013 12:01:06 +0200
> Currently we would still potentially suffer multicast packet loss if there
> is just either an IGMP or an MLD querier: For the former case, we would
> possibly drop IPv6 multicast packets, for the latter IPv4 ones. This is
> because we are currently assuming that if either an IGMP or MLD querier
> is present that the other one is present, too.
>
> This patch makes the behaviour and fix added in
> "bridge: disable snooping if there is no querier" (b00589af3b04)
> to also work if there is either just an IGMP or an MLD querier on the
> link: It refines the deactivation of the snooping to be protocol
> specific by using separate timers for the snooped IGMP and MLD queries
> as well as separate timers for our internal IGMP and MLD queriers.
>
> Signed-off-by: Linus Lüssing <linus.luessing@web.de>
I think the logic of the fix is fine, but theres a lot of duplication
here.
Why not make a:
struct bridge_mcast_timer {
struct timer_list timer;
u32 queries_sent;
u32 startup_queries_sent;
};
Then in the port structure you have:
struct bridge_mcast_timer ip4;
struct bridge_mcast_timer ip6;
And pass a reference to the right one into generic helper functions.
That sounds a lot better right?
Please make this improvement and resubmit, thanks.
next prev parent reply other threads:[~2013-08-29 0:08 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-08-26 10:01 [PATCH] bridge: separate querier and query timer into IGMP/IPv4 and MLD/IPv6 ones Linus Lüssing
2013-08-26 10:01 ` [Bridge] " Linus Lüssing
2013-08-26 10:01 ` Linus Lüssing
2013-08-29 0:08 ` David Miller [this message]
2013-08-29 0:08 ` [Bridge] " David Miller
2013-08-29 0:08 ` David Miller
2013-08-30 15:28 ` [PATCHv2] " Linus Lüssing
2013-08-30 15:28 ` [Bridge] " Linus Lüssing
2013-08-30 15:28 ` Linus Lüssing
2013-08-30 19:29 ` David Miller
2013-08-30 19:29 ` [Bridge] " David Miller
2013-08-30 19:29 ` David Miller
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130828.200831.289304393401193657.davem@davemloft.net \
--to=davem@davemloft.net \
--cc=amwang@redhat.com \
--cc=bridge@lists.linux-foundation.org \
--cc=herbert@gondor.hengli.com.au \
--cc=linus.luessing@web.de \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@baker-net.org.uk \
--cc=netdev@vger.kernel.org \
--cc=stephen@networkplumber.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.