Received: by 2002:a25:1985:0:0:0:0:0 with SMTP id 127csp1470022ybz; Sat, 18 Apr 2020 01:30:11 -0700 (PDT) X-Google-Smtp-Source: APiQypLPBkQg5O3Nj+SfPQob0kdF9eP14eyxSeY9RrmGapQH9no0Y+ojkmelCLvVCLzMdXRuN3IC X-Received: by 2002:a17:906:a38f:: with SMTP id k15mr2669974ejz.181.1587198610931; Sat, 18 Apr 2020 01:30:10 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1587198610; cv=none; d=google.com; s=arc-20160816; b=y3tvHw5NQvUM62oxnt5bvCvP/wJICXHU5H1KGafVuivgI4r7KdQBTA9dzoaMbfvkui 03v8/IxprP5oVNrKCp6kyz43KZvHK5eVVu4S0mDrBL7KBDpAmxFX2V3Ri7baYX66OaRu 9DsPrZ89FOqvfYjHNuk8OyjSlrHr7Ja2+qXrnKRXbsL4/PZcfzf/jzIUB7u1a+WsTbQv gyjivEDavr8bdOHsdumIQgVaxxoSEHb/5gH/ja90ZuKzzZc9Wubggy7/qr0CgFfAH4Mv FHgTwffSUSQYVDJf1jcXJ8xTN8CwVE6igMn4l8POHwktasVG8Fgtife3iCulVQiPTvCy jj4w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding :content-language:in-reply-to:mime-version:user-agent:date :message-id:from:references:to:subject:dkim-signature; bh=VoBnLgHt5ybQx6Jc1Vq2lz4psLsHO2C44rjLR27+D8o=; b=VEar1Zd8skz0YeLwamPYm75xmW5r1CmOy82/CfctpZTTA3nC0kCuoKGzAO+xFcfZCd jz77n3RuLTCho+iPM4e0QaOlWHWgnJzCDOnf+f2ORgdLlENxo8Toaf6tGLvA8B5LeP7c JFXZclH5dOpTIRdea5YJ5RMY9OACLWFym+GdusgjSZLK7SkrtrScj6Gq5cKH1SR02qnX 3cQz6GOHYu9p8iHdNmwPep+z72dk546diCUeN2UlxCY9vTH1u7aKcvpWdFL86+BMZj0Q G4kW29fK5SDvY1miPJfZsHHr4C9DZiK7g00GbciqELmwean04YWQNo/c3WjlQXwvmX+D nQpg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@cumulusnetworks.com header.s=google header.b=B7XaVz+f; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=cumulusnetworks.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id a20si6140754edv.363.2020.04.18.01.29.47; Sat, 18 Apr 2020 01:30:10 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@cumulusnetworks.com header.s=google header.b=B7XaVz+f; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=cumulusnetworks.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726195AbgDRIZv (ORCPT + 99 others); Sat, 18 Apr 2020 04:25:51 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:60176 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725887AbgDRIZv (ORCPT ); Sat, 18 Apr 2020 04:25:51 -0400 Received: from mail-lf1-x141.google.com (mail-lf1-x141.google.com [IPv6:2a00:1450:4864:20::141]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 3D518C061A0F for ; Sat, 18 Apr 2020 01:25:50 -0700 (PDT) Received: by mail-lf1-x141.google.com with SMTP id j14so3711786lfg.9 for ; Sat, 18 Apr 2020 01:25:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cumulusnetworks.com; s=google; h=subject:to:references:from:message-id:date:user-agent:mime-version :in-reply-to:content-language:content-transfer-encoding; bh=VoBnLgHt5ybQx6Jc1Vq2lz4psLsHO2C44rjLR27+D8o=; b=B7XaVz+fKT74RIj6kWVjxTNgcuPRRlk6LUrEeePiwYQXLBZATqshP7u6ClFDNuXBb0 nRl1eDhyVW70bxuJtg288gO2giQZbwx3HnBmVmiwcHHJbzL2M1YiTaPn2s8ZFn8jT6vb 2s0wmkpWtTlmQonWpmB4TqTmRWLb+NMBBtgFI= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=VoBnLgHt5ybQx6Jc1Vq2lz4psLsHO2C44rjLR27+D8o=; b=Olf0y+JNLT61+Ltw9ylNVjyW2C4MF7RK/emMZ/K70URvNK4rZD/V7/Q5wWYPrUAZDG 3h0PMdNsJqb9c+j+Ndo4UpzhjdYzUoeeIkocD6cDPedh4jdJQIhMbVLx5XRaLKfABg4R Q8TLCnGONxtuqdJnHXhZHgKvvng/vOi9xh1WoI/xEF37lvX/yyEy7WYQ3LJHO3hbDXE6 voIO++RgORoXHRShiU3e6G+YxnF3nOo/g5tB3MprN7c/LWtT5zvDmAT3+hZO47khnl/t omvBz8jhm2wPFCx8iuqsbmSsZGUF4MUA+P7gCNlKguoJERQceCBlBVfqOAqaXkgfySgA fphw== X-Gm-Message-State: AGi0PubL+LDWZ9ao2LyXybonq44Fgxpjj5Laolq+PmdAEVNaVrEUmaN3 PX34K5itZEd1kZey1UWaLtTMQA== X-Received: by 2002:ac2:4da7:: with SMTP id h7mr4424535lfe.95.1587198348684; Sat, 18 Apr 2020 01:25:48 -0700 (PDT) Received: from [192.168.0.109] (84-238-136-197.ip.btc-net.bg. [84.238.136.197]) by smtp.gmail.com with ESMTPSA id q16sm5997552ljj.23.2020.04.18.01.25.46 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sat, 18 Apr 2020 01:25:47 -0700 (PDT) Subject: Re: [RFC net-next v5 9/9] bridge: mrp: Integrate MRP into the bridge To: Horatiu Vultur , davem@davemloft.net, jiri@resnulli.us, ivecera@redhat.com, kuba@kernel.org, roopa@cumulusnetworks.com, olteanv@gmail.com, andrew@lunn.ch, linux-kernel@vger.kernel.org, netdev@vger.kernel.org, bridge@lists.linux-foundation.org, UNGLinuxDriver@microchip.com References: <20200414112618.3644-1-horatiu.vultur@microchip.com> <20200414112618.3644-10-horatiu.vultur@microchip.com> From: Nikolay Aleksandrov Message-ID: <48c8e196-5808-d7c8-25c3-dff8f56dea5b@cumulusnetworks.com> Date: Sat, 18 Apr 2020 11:25:46 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.6.0 MIME-Version: 1.0 In-Reply-To: <20200414112618.3644-10-horatiu.vultur@microchip.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 14/04/2020 14:26, Horatiu Vultur wrote: > To integrate MRP into the bridge, the bridge needs to do the following: > - add new flag(BR_MPP_AWARE) to the net bridge ports, this bit will be set when > the port is added to an MRP instance. In this way it knows if the frame was > received on MRP ring port > - detect if the MRP frame was received on MRP ring port in that case it would be > processed otherwise just forward it as usual. > - enable parsing of MRP > - before whenever the bridge was set up, it would set all the ports in > forwarding state. Add an extra check to not set ports in forwarding state if > the port is an MRP ring port. The reason of this change is that if the MRP > instance initially sets the port in blocked state by setting the bridge up it > would overwrite this setting. > > Signed-off-by: Horatiu Vultur > --- > include/linux/if_bridge.h | 1 + > net/bridge/br_device.c | 3 +++ > net/bridge/br_if.c | 2 ++ > net/bridge/br_input.c | 3 +++ > net/bridge/br_netlink.c | 5 +++++ > net/bridge/br_private.h | 35 +++++++++++++++++++++++++++++++++++ > net/bridge/br_stp.c | 6 ++++++ > net/bridge/br_stp_if.c | 5 +++++ > 8 files changed, 60 insertions(+) > > diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h > index 9e57c4411734..10baa9efdae8 100644 > --- a/include/linux/if_bridge.h > +++ b/include/linux/if_bridge.h > @@ -47,6 +47,7 @@ struct br_ip_list { > #define BR_BCAST_FLOOD BIT(14) > #define BR_NEIGH_SUPPRESS BIT(15) > #define BR_ISOLATED BIT(16) > +#define BR_MRP_AWARE BIT(17) > > #define BR_DEFAULT_AGEING_TIME (300 * HZ) > > diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c > index 0e3dbc5f3c34..8ec1362588af 100644 > --- a/net/bridge/br_device.c > +++ b/net/bridge/br_device.c > @@ -463,6 +463,9 @@ void br_dev_setup(struct net_device *dev) > spin_lock_init(&br->lock); > INIT_LIST_HEAD(&br->port_list); > INIT_HLIST_HEAD(&br->fdb_list); > +#if IS_ENABLED(CONFIG_BRIDGE_MRP) > + INIT_LIST_HEAD(&br->mrp_list); > +#endif > spin_lock_init(&br->hash_lock); > > br->bridge_id.prio[0] = 0x80; > diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c > index 4fe30b182ee7..ca685c0cdf95 100644 > --- a/net/bridge/br_if.c > +++ b/net/bridge/br_if.c > @@ -333,6 +333,8 @@ static void del_nbp(struct net_bridge_port *p) > br_stp_disable_port(p); > spin_unlock_bh(&br->lock); > > + br_mrp_port_del(br, p); > + > br_ifinfo_notify(RTM_DELLINK, NULL, p); > > list_del_rcu(&p->list); > diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c > index fcc260840028..d5c34f36f0f4 100644 > --- a/net/bridge/br_input.c > +++ b/net/bridge/br_input.c > @@ -342,6 +342,9 @@ rx_handler_result_t br_handle_frame(struct sk_buff **pskb) > } > } > > + if (unlikely(br_mrp_process(p, skb))) > + return RX_HANDLER_PASS; > + > forward: > switch (p->state) { > case BR_STATE_FORWARDING: > diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c > index 43dab4066f91..8826fcd1eb76 100644 > --- a/net/bridge/br_netlink.c > +++ b/net/bridge/br_netlink.c > @@ -669,6 +669,11 @@ static int br_afspec(struct net_bridge *br, > if (err) > return err; > break; > + case IFLA_BRIDGE_MRP: > + err = br_mrp_parse(br, p, attr, cmd, extack); > + if (err) > + return err; > + break; > } > } > > diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h > index 1f97703a52ff..5835828320b6 100644 > --- a/net/bridge/br_private.h > +++ b/net/bridge/br_private.h > @@ -428,6 +428,10 @@ struct net_bridge { > int offload_fwd_mark; > #endif > struct hlist_head fdb_list; > + > +#if IS_ENABLED(CONFIG_BRIDGE_MRP) > + struct list_head __rcu mrp_list; > +#endif > }; > > struct br_input_skb_cb { > @@ -1304,6 +1308,37 @@ unsigned long br_timer_value(const struct timer_list *timer); > extern int (*br_fdb_test_addr_hook)(struct net_device *dev, unsigned char *addr); > #endif > > +/* br_mrp.c */ > +#if IS_ENABLED(CONFIG_BRIDGE_MRP) > +int br_mrp_parse(struct net_bridge *br, struct net_bridge_port *p, > + struct nlattr *attr, int cmd, struct netlink_ext_ack *extack); > +int br_mrp_process(struct net_bridge_port *p, struct sk_buff *skb); > +bool br_mrp_enabled(struct net_bridge *br); > +void br_mrp_port_del(struct net_bridge *br, struct net_bridge_port *p); > +#else > +static inline int br_mrp_parse(struct net_bridge *br, struct net_bridge_port *p, > + struct nlattr *attr, int cmd, > + struct netlink_ext_ack *extack) > +{ > + return -EOPNOTSUPP; > +} > + > +static inline int br_mrp_process(struct net_bridge_port *p, struct sk_buff *skb) > +{ > + return 0; > +} > + > +static inline bool br_mrp_enabled(struct net_bridge *br) > +{ > + return 0; > +} > + > +static inline void br_mrp_port_del(struct net_bridge *br, > + struct net_bridge_port *p) > +{ > +} > +#endif > + > /* br_netlink.c */ > extern struct rtnl_link_ops br_link_ops; > int br_netlink_init(void); > diff --git a/net/bridge/br_stp.c b/net/bridge/br_stp.c > index 1f14b8455345..3e88be7aa269 100644 > --- a/net/bridge/br_stp.c > +++ b/net/bridge/br_stp.c > @@ -36,6 +36,12 @@ void br_set_state(struct net_bridge_port *p, unsigned int state) > }; > int err; > > + /* Don't change the state of the ports if they are driven by a different > + * protocol. > + */ > + if (p->flags & BR_MRP_AWARE) > + return; > + > p->state = state; > err = switchdev_port_attr_set(p->dev, &attr); > if (err && err != -EOPNOTSUPP) > diff --git a/net/bridge/br_stp_if.c b/net/bridge/br_stp_if.c > index d174d3a566aa..542b212d5033 100644 > --- a/net/bridge/br_stp_if.c > +++ b/net/bridge/br_stp_if.c > @@ -200,6 +200,11 @@ void br_stp_set_enabled(struct net_bridge *br, unsigned long val) > { > ASSERT_RTNL(); > > + if (br_mrp_enabled(br)) { > + br_warn(br, "STP can't be enabled if MRP is already enabled\n"); It'd be nice if this can be returned in an extack if this function is called from netlink. In addition this must return an error - otherwise writing to the sysfs file would be successful while nothing will have changed, so the user will think it worked. Check out set_stp_state(). You can drop the br_warn, just make sure to return proper extack error from netlink (it is the preferred interface over sysfs, so simply returning an error for sysfs would be enough). > + return; > + } > + > if (val) { > if (br->stp_enabled == BR_NO_STP) > br_stp_start(br); >