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=-4.1 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_PASS,URIBL_BLOCKED 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 7AC5EC4360F for ; Wed, 3 Apr 2019 18:23:22 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 34A2420700 for ; Wed, 3 Apr 2019 18:23:22 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=cumulusnetworks.com header.i=@cumulusnetworks.com header.b="bMj+1Zze" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726412AbfDCSXU (ORCPT ); Wed, 3 Apr 2019 14:23:20 -0400 Received: from mail-wm1-f65.google.com ([209.85.128.65]:35474 "EHLO mail-wm1-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726144AbfDCSXU (ORCPT ); Wed, 3 Apr 2019 14:23:20 -0400 Received: by mail-wm1-f65.google.com with SMTP id y197so9548131wmd.0 for ; Wed, 03 Apr 2019 11:23:18 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cumulusnetworks.com; s=google; h=subject:from:to:references:message-id:date:user-agent:mime-version :in-reply-to:content-language:content-transfer-encoding; bh=sRpjw/2oMZwWVjQQCC7WRM3lh1Yuhrn7tBdyiUHK86M=; b=bMj+1ZzeuOQorrje1jnj6hlyHmMV266uh4BnwK+NJS4zg389ItChKnLVCwdmGtZb+h zL1Hd36sgvetXmv9f0b1kaQaZCKcOLL2IFaxhy5bOdWHFF8In4vOC4UTsM7aM/Ib4rMn 3nMxczQWcpC7dRvq8TOgi2sfSnEc9TN461Uxk= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:from:to:references:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=sRpjw/2oMZwWVjQQCC7WRM3lh1Yuhrn7tBdyiUHK86M=; b=SFjQiQKxfgCL0P9UCwWncpv5kq+OM7gkuCE8QHQ7QKJS15WKAUf2GDHasM2g7aC9Qy qM0xQHC732Ro/pUqVcc3NrL5Ipe2vU/fxXkIroFD7ZVSOmh2dHPAqifkSPg7Ewg+ufZk NhyheS7R/+O+/9jRfVYnVoZA56rUekBdBMgLbtPc9taO+Jvj0gRVea/sAWuk0AyuacOm 7gJgygcSDlDNj9G+AS+MpnyvGxnhjRVceb9I9vuhS2Pg0qedUg+SmbcQSyCq+NC5ryRW AmZFUd6GRGaXXl93wLslfgHvH+wKMI/YncVdBOdja5NiPl3/Q9Qytw8APMq08SFHkCgP SglA== X-Gm-Message-State: APjAAAWZMwRk2EJKEw7rv0n1zzKhTsm/ryrNsnyXdkmVUkIgGCEPDXUV XBGxqNUTsfeTGKlq5hnSGLCnFAnocu+xlA== X-Google-Smtp-Source: APXvYqx1kBfB315DYJv48yiS0dqsx8uDCrrKtrSMo9F4fV99R0t7DSFnIbQvcz7zYHCUvmlkv/W8Rg== X-Received: by 2002:a1c:acc8:: with SMTP id v191mr1012043wme.72.1554315797410; Wed, 03 Apr 2019 11:23:17 -0700 (PDT) Received: from [192.168.0.107] (84-238-136-197.ip.btc-net.bg. [84.238.136.197]) by smtp.gmail.com with ESMTPSA id n6sm14724892wmn.48.2019.04.03.11.23.16 (version=TLS1_3 cipher=AEAD-AES128-GCM-SHA256 bits=128/128); Wed, 03 Apr 2019 11:23:16 -0700 (PDT) Subject: Re: [PATCH net-next 3/4] bridge: support binding vlan dev link state to vlan member bridge ports From: Nikolay Aleksandrov To: mmanning@vyatta.att-mail.com, netdev@vger.kernel.org References: <20190402153543.6277-1-mmanning@vyatta.att-mail.com> <20190402153543.6277-4-mmanning@vyatta.att-mail.com> <660463ff-9e87-0221-11d9-6d04e57d901f@vyatta.att-mail.com> <604bbb25-88e0-28cc-9940-909ca5613ec4@cumulusnetworks.com> <675dfcb7-76e8-a14c-f798-728ac4aa17f3@cumulusnetworks.com> Message-ID: Date: Wed, 3 Apr 2019 21:23:15 +0300 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: <675dfcb7-76e8-a14c-f798-728ac4aa17f3@cumulusnetworks.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 03/04/2019 21:17, Nikolay Aleksandrov wrote: > On 03/04/2019 20:53, Nikolay Aleksandrov wrote: >> On 03/04/2019 20:43, Mike Manning wrote: >>> On 02/04/2019 20:22, Nikolay Aleksandrov wrote: >>>> On 02/04/2019 18:35, Mike Manning wrote: >>>>> In the case of vlan filtering on bridges, the bridge may also have the >>>>> corresponding vlan devices as upper devices. A vlan bridge binding mode >>>>> is added to allow the link state of the vlan device to track only the >>>>> state of the subset of bridge ports that are also members of the vlan, >>>>> rather than that of all bridge ports. This mode is set with a vlan flag >>>>> rather than a bridge sysfs so that the 8021q module is aware that it >>>>> should not set the link state for the vlan device. >>>>> >>>>> If bridge vlan is configured, the bridge device event handling results >>>>> in the link state for an upper device being set, if it is a vlan device >>>>> with the vlan bridge binding mode enabled. This also sets a >>>>> vlan_bridge_binding flag so that subsequent UP/DOWN/CHANGE events for >>>>> the ports in that bridge result in a link state update of the vlan >>>>> device if required. >>>>> >>>>> The link state of the vlan device is up if there is at least one bridge >>>>> port that is a vlan member that is admin & oper up, otherwise its oper >>>>> state is IF_OPER_LOWERLAYERDOWN. >>>>> >>>>> Signed-off-by: Mike Manning >>>>> --- >>>>> net/bridge/br.c | 23 ++++++-- >>>>> net/bridge/br_private.h | 17 ++++++ >>>>> net/bridge/br_vlan.c | 143 ++++++++++++++++++++++++++++++++++++++++++++++++ >>>>> 3 files changed, 179 insertions(+), 4 deletions(-) >>>>> >>>> Hi, >>>> Please CC bridge maintainers when sending bridge patches. >>> Thank you very much for the review, I will CC you and Roopa when I have >>> the v1 series ready. >>>> One question/thought - can't we add a ports_up counter in the vlan's master >>>> struct and keep how many ports are up for that vlan ? >>> >>> This would have been my preferred choice, but for this one would need to >>> know the old link state for a port so as to determine if/what link state >>> transition has occurred for a NETDEV_CHANGE notification. This is if >>> only a single counter is kept for the vlan for all ports (also it might >>> be difficult to recover from an error in the counter). I could see it >>> working if one kept track of the operational state for each port in the >>> vlan in a data structure specific to this purpose i.e. that is more >>> efficient than the existing walk. However, speed in processing these >>> state changes is not that important, also the link state is quickly >>> determined when it might matter more, i.e. on link up of a port. >>> >> >> Indeed, the NETDEV_CHANGE is harder, but we can keep the last known carrier state >> in the per-port structure and make a decision based on that and the new state. >> That wouldn't require any additional structures. Speed is important to us when >> we deploy the bridge at scale, we have tests with thousands of vlans and devices >> where this walk would become expensive on link flaps. >> > > In fact we already have a similar tracking field used for the port state, maybe > it can be used as an indicator. That state needs to be taken into account anyway > or the carrier state would be wrong. > Nevermind the last sentence, spoke too quickly. An additional structure may be needed after all, this will need some investigating. >>>> The important part would be to keep it correct, i.e. vlan_add/del should inc/dec >>>> as well as port up/down. Then we can directly update its carrier on port event >>>> without doing a possible O(n^2) walk, we just need to walk over the port vlans >>>> and adjust counters which is always O(n) based on num of that port's vlans. >>>> >>>> Some more comments below. >>> I will make all the other changes you have requested. >>> >> >> Thanks! >> >