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.0 required=3.0 tests=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 0715AC4360F for ; Wed, 3 Apr 2019 17:44:44 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id C279E206DF for ; Wed, 3 Apr 2019 17:44:43 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726183AbfDCRom (ORCPT ); Wed, 3 Apr 2019 13:44:42 -0400 Received: from mx0b-00191d01.pphosted.com ([67.231.157.136]:50874 "EHLO mx0a-00191d01.pphosted.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1726151AbfDCRom (ORCPT ); Wed, 3 Apr 2019 13:44:42 -0400 Received: from pps.filterd (m0083689.ppops.net [127.0.0.1]) by m0083689.ppops.net-00191d01. (8.16.0.27/8.16.0.27) with SMTP id x33HPgvU021688; Wed, 3 Apr 2019 13:44:40 -0400 Received: from alpi155.enaf.aldc.att.com (sbcsmtp7.sbc.com [144.160.229.24]) by m0083689.ppops.net-00191d01. with ESMTP id 2rn0te1frt-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 03 Apr 2019 13:44:40 -0400 Received: from enaf.aldc.att.com (localhost [127.0.0.1]) by alpi155.enaf.aldc.att.com (8.14.5/8.14.5) with ESMTP id x33Hid1K014326; Wed, 3 Apr 2019 13:44:40 -0400 Received: from zlp27127.vci.att.com (zlp27127.vci.att.com [135.66.87.31]) by alpi155.enaf.aldc.att.com (8.14.5/8.14.5) with ESMTP id x33HibbU014278 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Wed, 3 Apr 2019 13:44:38 -0400 Received: from zlp27127.vci.att.com (zlp27127.vci.att.com [127.0.0.1]) by zlp27127.vci.att.com (Service) with ESMTP id 8602E4009E6F; Wed, 3 Apr 2019 17:44:37 +0000 (GMT) Received: from mlpi432.sfdc.sbc.com (unknown [144.151.223.11]) by zlp27127.vci.att.com (Service) with ESMTP id 680FA4009E62; Wed, 3 Apr 2019 17:44:37 +0000 (GMT) Received: from sfdc.sbc.com (localhost [127.0.0.1]) by mlpi432.sfdc.sbc.com (8.14.5/8.14.5) with ESMTP id x33HibVF016555; Wed, 3 Apr 2019 13:44:37 -0400 Received: from mail.eng.vyatta.net (mail.eng.vyatta.net [10.156.50.82]) by mlpi432.sfdc.sbc.com (8.14.5/8.14.5) with ESMTP id x33HiX4p016460; Wed, 3 Apr 2019 13:44:33 -0400 Received: from [10.156.47.138] (unknown [10.156.47.138]) by mail.eng.vyatta.net (Postfix) with ESMTPA id 60FBB3600D9; Wed, 3 Apr 2019 10:44:32 -0700 (PDT) Reply-To: mmanning@vyatta.att-mail.com Subject: Re: [PATCH net-next 4/4] bridge: update vlan dev state when port added to or deleted from vlan To: Nikolay Aleksandrov , netdev@vger.kernel.org References: <20190402153543.6277-1-mmanning@vyatta.att-mail.com> <20190402153543.6277-5-mmanning@vyatta.att-mail.com> <0904f40f-c47b-b1f3-60e6-3673575d2387@cumulusnetworks.com> From: Mike Manning Message-ID: Date: Wed, 3 Apr 2019 18:44:31 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.5.1 MIME-Version: 1.0 In-Reply-To: <0904f40f-c47b-b1f3-60e6-3673575d2387@cumulusnetworks.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Content-Language: en-US X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:,, definitions=2019-04-03_10:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_policy_notspam policy=outbound_policy score=0 priorityscore=1501 malwarescore=0 suspectscore=2 phishscore=0 bulkscore=0 spamscore=0 clxscore=1011 lowpriorityscore=0 mlxscore=0 impostorscore=0 mlxlogscore=999 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1810050000 definitions=main-1904030118 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On 02/04/2019 21:10, Nikolay Aleksandrov wrote: > On 02/04/2019 18:35, Mike Manning wrote: >> If vlan bridge binding is enabled, then the link state of a vlan device >> that is an upper device of the bridge should track the state of bridge >> ports that are members of that vlan. So if a bridge port becomes or >> stops being a member of a vlan, then update the link state of the >> vlan device if necessary. >> >> Signed-off-by: Mike Manning >> --- >> net/bridge/br_vlan.c | 23 +++++++++++++++++++++++ >> 1 file changed, 23 insertions(+) >> >> diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c >> index 642373231386..7c11607cf1f4 100644 >> --- a/net/bridge/br_vlan.c >> +++ b/net/bridge/br_vlan.c >> @@ -7,6 +7,9 @@ >> #include "br_private.h" >> #include "br_private_tunnel.h" >> >> +static void nbp_vlan_set_vlan_dev_state(struct net_bridge_port *p, >> + struct net_bridge *br, u16 vid); >> + >> static inline int br_vlan_cmp(struct rhashtable_compare_arg *arg, >> const void *ptr) >> { >> @@ -294,6 +297,9 @@ static int __vlan_add(struct net_bridge_vlan *v, u16 flags, >> >> __vlan_add_list(v); >> __vlan_add_flags(v, flags); >> + >> + if (p) >> + nbp_vlan_set_vlan_dev_state(p, br, v->vid); > since you need this as a last action after vlan_add and acts only on ports > why not move it to nbp_vlan_add() ? The call is in __vlan_add() rather than nbp_vlan_add() only for reasons of consistency with the delete case, please see below. > >> out: >> return err; >> >> @@ -358,6 +364,8 @@ static int __vlan_del(struct net_bridge_vlan *v) >> rhashtable_remove_fast(&vg->vlan_hash, &v->vnode, >> br_vlan_rht_params); >> __vlan_del_list(v); >> + if (p) >> + nbp_vlan_set_vlan_dev_state(p, p->br, v->vid); > p is guaranteed to be set here, but also there's an if (p) earlier in the function. > this can probably also move to nbp_vlan_delete if the other set_state moves to > nbp_vlan_add > Agreed re p being set here, I was just safeguarding against future changes where this might not be true, but I will get rid of this. The call is made here so as to be after the removal of the node from the rhashtable. The check is also needed as a result of calling nbp_vlan_flush(), hence making the call here rather than in nbp_vlan_delete(). >> call_rcu(&v->rcu, nbp_vlan_rcu_free); >> } >> >> @@ -1357,6 +1365,21 @@ static void br_vlan_set_vlan_dev_state(struct net_bridge *br, >> } >> } >> >> +static void nbp_vlan_set_vlan_dev_state(struct net_bridge_port *p, >> + struct net_bridge *br, u16 vid) >> +{ >> + struct net_device *vlan_dev; >> + >> + if (!br->vlan_bridge_binding) >> + return; >> + >> + vlan_dev = br_vlan_get_upper_bind_vlan_dev(br->dev, vid); >> + if (vlan_dev) { >> + br_vlan_set_vlan_dev_state(br, vlan_dev); >> + dev_put(vlan_dev); > I think this is running under rtnl. Agreed, I will get rid of this and the other instance of this use.