From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.1 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3B69FC4360F for ; Sun, 17 Feb 2019 10:59:02 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id EB9E82147C for ; Sun, 17 Feb 2019 10:59:01 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=cumulusnetworks.com header.i=@cumulusnetworks.com header.b="bSdZwBFF" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728420AbfBQK6i (ORCPT ); Sun, 17 Feb 2019 05:58:38 -0500 Received: from mail-wm1-f67.google.com ([209.85.128.67]:34676 "EHLO mail-wm1-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726003AbfBQK6h (ORCPT ); Sun, 17 Feb 2019 05:58:37 -0500 Received: by mail-wm1-f67.google.com with SMTP id y185so9954944wmd.1 for ; Sun, 17 Feb 2019 02:58:36 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cumulusnetworks.com; s=google; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=x6ASLS41FmWJ216MwoYDhpeq7b8n5bfsLtl4fOvOw4s=; b=bSdZwBFFsQ/M/s6ksxp3FxtjdVUgr0MLfziqUNjEi057OdalRgbwPneJ+aSiXu/w8d YngyoQtMRsDe0DwIicE+cjDS8woixyxQf2D9LqSfxcCnROCzvwvew2o6gR3ImlE/1YpK 2Xs16MDKs2Aut1/thcVj/pxD/XdU6Z2R6REig= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=x6ASLS41FmWJ216MwoYDhpeq7b8n5bfsLtl4fOvOw4s=; b=Pa/ixCkKNnE1MDuGcdZRRw0OPwJBwSAWGInUd+VOplS662fxRtFvJr3i3bWMaBin+4 l4ztBAFeIPOP3no7IOGuOEeZyAh5U32OTGpz24kWjVqHZx+bFKz6e7vV8X1LmPsf+sgC BBdctLNqh61BUywQCZM7gmeGTEfg1kyweF6F08o+TVGwbTl9xKXoiCsqPSJ5MG3uhitV fYAHLMYNrBsQLjCKdpeKelPeXC4qbbXLUHVzGb6/eynPWNHD99lcUiU/eCAdH8R/SYRW Y+SDjHhmI7uTbjwxpelVkIzMu63c3JTkLmK5a6Ahc5f8/0pWmmgyPKjKWQWeYYoejlN4 GeVw== X-Gm-Message-State: AHQUAuYjvKNqzeyzFHVv+D2wGJ7Db1k6okElnPzefBjGaVzU99TTNZak 0qaLLHJbh/jsEdRdrmU2+a30/A== X-Google-Smtp-Source: AHgI3IYpEozaVowRHEHWmuCpDqlkqmTtfM5bIAl7llfwSlA+medt05pkT24o/A3Pkuf01usPANKitQ== X-Received: by 2002:a1c:d18a:: with SMTP id i132mr12279184wmg.27.1550401115403; Sun, 17 Feb 2019 02:58:35 -0800 (PST) Received: from [192.168.0.107] (79-100-158-105.ip.btc-net.bg. [79.100.158.105]) by smtp.gmail.com with ESMTPSA id o5sm37989732wrh.34.2019.02.17.02.58.33 (version=TLS1_3 cipher=AEAD-AES128-GCM-SHA256 bits=128/128); Sun, 17 Feb 2019 02:58:34 -0800 (PST) Subject: Re: [PATCH RFC] net: bridge: don't flood known multicast traffic when snooping is enabled To: Florian Fainelli , netdev@vger.kernel.org Cc: roopa@cumulusnetworks.com, wkok@cumulusnetworks.com, anuradhak@cumulusnetworks.com, bridge@lists.linux-foundation.org, linus.luessing@c0d3.blue, davem@davemloft.net, stephen@networkplumber.org References: <20190215130427.29824-1-nikolay@cumulusnetworks.com> <2bb7baaa-affb-451c-658d-bc5412a14c31@gmail.com> From: Nikolay Aleksandrov Message-ID: Date: Sun, 17 Feb 2019 12:58:32 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0 MIME-Version: 1.0 In-Reply-To: <2bb7baaa-affb-451c-658d-bc5412a14c31@gmail.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On 17/02/2019 05:05, Florian Fainelli wrote: > > > On 2/15/2019 5:04 AM, Nikolay Aleksandrov wrote: >> The behaviour since b00589af3b04 ("bridge: disable snooping if there is >> no querier") is wrong, we shouldn't be flooding multicast traffic when >> there is an mdb entry and we know where it should be forwarded to when >> multicast snooping is enabled. This patch changes the behaviour to not >> flood known unicast traffic. > > You mean multicast traffic in the last part of the sentence, right? > Right. The change I wanted to discuss is when there is no querier and we have a known mdst - forward only to its registered ports. The rest of the traffic follows the current rules (i.e. no querier - flood unknown mcast). I'll send an updated RFC version since this patch has issues as noted in the discussion and we can continue from there. >> I'll give two obviously broken cases: >> - most obvious: static mdb created by the user with snooping enabled >> - user-space daemon controlling the mdb table (e.g. MLAG) >> >> Every user would expect to have traffic forwarded only to the configured >> mdb destination when snooping is enabled, instead now to get that one >> needs to enable both snooping and querier. Enabling querier on all >> switches could be problematic and is not a good solution, for example >> as summarized by our multicast experts: >> "every switch would send an IGMP query for any random multicast traffic it >> received across the entire domain and it would send it forever as long as a >> host exists wanting that stream even if it has no downstream/directly >> connected receivers" >> >> Sending as an RFC to get the discussion going, but I'm strongly for >> removing this behaviour and would like to send this patch officially. >> >> We could make this behaviour possible via a knob if necessary, but >> it really should not be the default. >> >> Signed-off-by: Nikolay Aleksandrov >> --- >> net/bridge/br_device.c | 3 +-- >> net/bridge/br_input.c | 3 +-- >> 2 files changed, 2 insertions(+), 4 deletions(-) >> >> diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c >> index 013323b6dbe4..2aa8a6509924 100644 >> --- a/net/bridge/br_device.c >> +++ b/net/bridge/br_device.c >> @@ -96,8 +96,7 @@ netdev_tx_t br_dev_xmit(struct sk_buff *skb, struct net_device *dev) >> } >> >> mdst = br_mdb_get(br, skb, vid); >> - if ((mdst || BR_INPUT_SKB_CB_MROUTERS_ONLY(skb)) && >> - br_multicast_querier_exists(br, eth_hdr(skb))) >> + if (mdst || BR_INPUT_SKB_CB_MROUTERS_ONLY(skb)) >> br_multicast_flood(mdst, skb, false, true); >> else >> br_flood(br, skb, BR_PKT_MULTICAST, false, true); >> diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c >> index 5ea7e56119c1..aae78095cf67 100644 >> --- a/net/bridge/br_input.c >> +++ b/net/bridge/br_input.c >> @@ -136,8 +136,7 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb >> switch (pkt_type) { >> case BR_PKT_MULTICAST: >> mdst = br_mdb_get(br, skb, vid); >> - if ((mdst || BR_INPUT_SKB_CB_MROUTERS_ONLY(skb)) && >> - br_multicast_querier_exists(br, eth_hdr(skb))) { >> + if (mdst || BR_INPUT_SKB_CB_MROUTERS_ONLY(skb)) { >> if ((mdst && mdst->host_joined) || >> br_multicast_is_router(br)) { >> local_rcv = true; >> > From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cumulusnetworks.com; s=google; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=x6ASLS41FmWJ216MwoYDhpeq7b8n5bfsLtl4fOvOw4s=; b=bSdZwBFFsQ/M/s6ksxp3FxtjdVUgr0MLfziqUNjEi057OdalRgbwPneJ+aSiXu/w8d YngyoQtMRsDe0DwIicE+cjDS8woixyxQf2D9LqSfxcCnROCzvwvew2o6gR3ImlE/1YpK 2Xs16MDKs2Aut1/thcVj/pxD/XdU6Z2R6REig= References: <20190215130427.29824-1-nikolay@cumulusnetworks.com> <2bb7baaa-affb-451c-658d-bc5412a14c31@gmail.com> From: Nikolay Aleksandrov Message-ID: Date: Sun, 17 Feb 2019 12:58:32 +0200 MIME-Version: 1.0 In-Reply-To: <2bb7baaa-affb-451c-658d-bc5412a14c31@gmail.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Bridge] [PATCH RFC] net: bridge: don't flood known multicast traffic when snooping is enabled List-Id: Linux Ethernet Bridging List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Florian Fainelli , netdev@vger.kernel.org Cc: roopa@cumulusnetworks.com, bridge@lists.linux-foundation.org, wkok@cumulusnetworks.com, anuradhak@cumulusnetworks.com, davem@davemloft.net On 17/02/2019 05:05, Florian Fainelli wrote: > > > On 2/15/2019 5:04 AM, Nikolay Aleksandrov wrote: >> The behaviour since b00589af3b04 ("bridge: disable snooping if there is >> no querier") is wrong, we shouldn't be flooding multicast traffic when >> there is an mdb entry and we know where it should be forwarded to when >> multicast snooping is enabled. This patch changes the behaviour to not >> flood known unicast traffic. > > You mean multicast traffic in the last part of the sentence, right? > Right. The change I wanted to discuss is when there is no querier and we have a known mdst - forward only to its registered ports. The rest of the traffic follows the current rules (i.e. no querier - flood unknown mcast). I'll send an updated RFC version since this patch has issues as noted in the discussion and we can continue from there. >> I'll give two obviously broken cases: >> - most obvious: static mdb created by the user with snooping enabled >> - user-space daemon controlling the mdb table (e.g. MLAG) >> >> Every user would expect to have traffic forwarded only to the configured >> mdb destination when snooping is enabled, instead now to get that one >> needs to enable both snooping and querier. Enabling querier on all >> switches could be problematic and is not a good solution, for example >> as summarized by our multicast experts: >> "every switch would send an IGMP query for any random multicast traffic it >> received across the entire domain and it would send it forever as long as a >> host exists wanting that stream even if it has no downstream/directly >> connected receivers" >> >> Sending as an RFC to get the discussion going, but I'm strongly for >> removing this behaviour and would like to send this patch officially. >> >> We could make this behaviour possible via a knob if necessary, but >> it really should not be the default. >> >> Signed-off-by: Nikolay Aleksandrov >> --- >> net/bridge/br_device.c | 3 +-- >> net/bridge/br_input.c | 3 +-- >> 2 files changed, 2 insertions(+), 4 deletions(-) >> >> diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c >> index 013323b6dbe4..2aa8a6509924 100644 >> --- a/net/bridge/br_device.c >> +++ b/net/bridge/br_device.c >> @@ -96,8 +96,7 @@ netdev_tx_t br_dev_xmit(struct sk_buff *skb, struct net_device *dev) >> } >> >> mdst = br_mdb_get(br, skb, vid); >> - if ((mdst || BR_INPUT_SKB_CB_MROUTERS_ONLY(skb)) && >> - br_multicast_querier_exists(br, eth_hdr(skb))) >> + if (mdst || BR_INPUT_SKB_CB_MROUTERS_ONLY(skb)) >> br_multicast_flood(mdst, skb, false, true); >> else >> br_flood(br, skb, BR_PKT_MULTICAST, false, true); >> diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c >> index 5ea7e56119c1..aae78095cf67 100644 >> --- a/net/bridge/br_input.c >> +++ b/net/bridge/br_input.c >> @@ -136,8 +136,7 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb >> switch (pkt_type) { >> case BR_PKT_MULTICAST: >> mdst = br_mdb_get(br, skb, vid); >> - if ((mdst || BR_INPUT_SKB_CB_MROUTERS_ONLY(skb)) && >> - br_multicast_querier_exists(br, eth_hdr(skb))) { >> + if (mdst || BR_INPUT_SKB_CB_MROUTERS_ONLY(skb)) { >> if ((mdst && mdst->host_joined) || >> br_multicast_is_router(br)) { >> local_rcv = true; >> >