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=-8.1 required=3.0 tests=DKIM_INVALID,DKIM_SIGNED, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 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 56FE8C3815B for ; Mon, 20 Apr 2020 19:16:41 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 21F882074F for ; Mon, 20 Apr 2020 19:16:41 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=microchip.com header.i=@microchip.com header.b="vyTKZ1Yl" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727944AbgDTTQk (ORCPT ); Mon, 20 Apr 2020 15:16:40 -0400 Received: from esa2.microchip.iphmx.com ([68.232.149.84]:64359 "EHLO esa2.microchip.iphmx.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725897AbgDTTQj (ORCPT ); Mon, 20 Apr 2020 15:16:39 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=microchip.com; i=@microchip.com; q=dns/txt; s=mchp; t=1587410199; x=1618946199; h=date:from:to:cc:subject:message-id:references: mime-version:in-reply-to; bh=y3VTXHw8/f/xzZpAPTTosZewiTx9SOPuKmC7E3cHTv4=; b=vyTKZ1YlHRD39N+dAJDm5+EWYwLsLIGAYdQO5b1KXZy6dheNbJtsXSnq WsIW5w569Gei58SpYh61qJk3yvxO2cK2nItduE5PyLx7zLSUdBQVfrE8a +3IRCqNDiJ9TL9o4XoV7uK3e9Jgl2C5dwgz8XaZVp5rSlwO7XBvL8+cab Zd4aYzlpbVAIUO0/wWknSuKeiKMPFqCoMmJ3AmUrWhtZcI4w+G6xVmyaF xqcupmWcxlrjjsxUU7AuDUCzWgfzUnB1CuZ+gH0EGKpuR1BDIS1BXt+Kj awRmfdpT7R3LvEQjJwUebjImzzQDkcb70L6ch9r27lnDU147y61AldPGA Q==; IronPort-SDR: NyJlXEwE8/F6tkBUNF9bQ4OUeLqf3BkWSbD0EjmwcE3yA1OXfvfHqLCnyOHT3GOB0aQnU7jwZO /Qb1smcr7uj+3CrNQK6zenih5jjXOA+nnXc6HQ+2Hle9g/BHj+rdQWI655Pc/AAPN1VGKAP+WL lLqOmv8bN+I3cb+DkW8OfUtCOx4+rytacmd7Q4tEHf8XvuWIKf6totsgHWJqdpEUT1D5bseDX0 rhuiu7epO35UMOJq+ioLl7hD2cNqSmmSR80SuPA2waW7H4bQQf7t5JgkVvj8gwo5w8q/hq4jqd k4M= X-IronPort-AV: E=Sophos;i="5.72,407,1580799600"; d="scan'208";a="72780182" Received: from smtpout.microchip.com (HELO email.microchip.com) ([198.175.253.82]) by esa2.microchip.iphmx.com with ESMTP/TLS/AES256-SHA256; 20 Apr 2020 12:16:39 -0700 Received: from chn-vm-ex03.mchp-main.com (10.10.85.151) by chn-vm-ex01.mchp-main.com (10.10.85.143) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Mon, 20 Apr 2020 12:16:45 -0700 Received: from localhost (10.10.115.15) by chn-vm-ex03.mchp-main.com (10.10.85.151) with Microsoft SMTP Server id 15.1.1713.5 via Frontend Transport; Mon, 20 Apr 2020 12:16:38 -0700 Date: Mon, 20 Apr 2020 21:16:37 +0200 From: Horatiu Vultur To: Nikolay Aleksandrov CC: , , , , , , , , , , Subject: Re: [PATCH net-next 10/13] bridge: mrp: Implement netlink interface to configure MRP Message-ID: <20200420191637.5skfpcayidzkb43w@soft-dev3.microsemi.net> References: <20200420150947.30974-1-horatiu.vultur@microchip.com> <20200420150947.30974-11-horatiu.vultur@microchip.com> <066720c4-ddc4-ce71-734f-932b6a342e01@cumulusnetworks.com> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Disposition: inline In-Reply-To: <066720c4-ddc4-ce71-734f-932b6a342e01@cumulusnetworks.com> User-Agent: NeoMutt/20180716 Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org The 04/20/2020 20:18, Nikolay Aleksandrov wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > On 20/04/2020 18:09, Horatiu Vultur wrote: > > Implement netlink interface to configure MRP. The implementation > > will do sanity checks over the attributes and then eventually call the MRP > > interface. > > > > Signed-off-by: Horatiu Vultur > > --- > > net/bridge/br_mrp_netlink.c | 117 ++++++++++++++++++++++++++++++++++++ > > 1 file changed, 117 insertions(+) > > create mode 100644 net/bridge/br_mrp_netlink.c > > > > diff --git a/net/bridge/br_mrp_netlink.c b/net/bridge/br_mrp_netlink.c > > new file mode 100644 > > index 000000000000..0ff42e7c7f57 > > --- /dev/null > > +++ b/net/bridge/br_mrp_netlink.c > > @@ -0,0 +1,117 @@ > > +// SPDX-License-Identifier: GPL-2.0-or-later > > + > > +#include > > + > > +#include > > +#include "br_private.h" > > +#include "br_private_mrp.h" > > + > > +static const struct nla_policy br_mrp_policy[IFLA_BRIDGE_MRP_MAX + 1] = { > > + [IFLA_BRIDGE_MRP_UNSPEC] = { .type = NLA_REJECT }, > > + [IFLA_BRIDGE_MRP_INSTANCE] = { .type = NLA_EXACT_LEN, > > + .len = sizeof(struct br_mrp_instance)}, > > + [IFLA_BRIDGE_MRP_PORT_STATE] = { .type = NLA_U32 }, > > + [IFLA_BRIDGE_MRP_PORT_ROLE] = { .type = NLA_EXACT_LEN, > > + .len = sizeof(struct br_mrp_port_role)}, > > + [IFLA_BRIDGE_MRP_RING_STATE] = { .type = NLA_EXACT_LEN, > > + .len = sizeof(struct br_mrp_ring_state)}, > > + [IFLA_BRIDGE_MRP_RING_ROLE] = { .type = NLA_EXACT_LEN, > > + .len = sizeof(struct br_mrp_ring_role)}, > > + [IFLA_BRIDGE_MRP_START_TEST] = { .type = NLA_EXACT_LEN, > > + .len = sizeof(struct br_mrp_start_test)}, > > +}; > > + > > +int br_mrp_parse(struct net_bridge *br, struct net_bridge_port *p, > > + struct nlattr *attr, int cmd, struct netlink_ext_ack *extack) > > +{ > > + struct nlattr *tb[IFLA_BRIDGE_MRP_MAX + 1]; > > + int err; > > + > > + if (br->stp_enabled != BR_NO_STP) { > > + NL_SET_ERR_MSG_MOD(extack, "MRP can't be enabled if STP is already enabled\n"); > > + return -EINVAL; > > + } > > + > > + err = nla_parse_nested(tb, IFLA_BRIDGE_MRP_MAX, attr, > > + NULL, extack); > > + if (err) > > + return err; > > + > > + if (tb[IFLA_BRIDGE_MRP_INSTANCE]) { > > + struct br_mrp_instance *instance = > > + nla_data(tb[IFLA_BRIDGE_MRP_INSTANCE]); > > + > > + if (cmd == RTM_SETLINK) > > + err = br_mrp_add(br, instance); > > + else > > + err = br_mrp_del(br, instance); > > + if (err) > > + return err; > > + } > > + > > + if (tb[IFLA_BRIDGE_MRP_PORT_STATE]) { > > + enum br_mrp_port_state_type state = > > + nla_get_u32(tb[IFLA_BRIDGE_MRP_PORT_STATE]); > > + > > + err = br_mrp_set_port_state(p, state); > > + if (err) > > + return err; > > + } > > + > > + if (tb[IFLA_BRIDGE_MRP_PORT_ROLE]) { > > + struct br_mrp_port_role *role = > > + nla_data(tb[IFLA_BRIDGE_MRP_PORT_ROLE]); > > + > > + err = br_mrp_set_port_role(p, role); > > + if (err) > > + return err; > > + } > > + > > + if (tb[IFLA_BRIDGE_MRP_RING_STATE]) { > > + struct br_mrp_ring_state *state = > > + nla_data(tb[IFLA_BRIDGE_MRP_RING_STATE]); > > + > > + err = br_mrp_set_ring_state(br, state); > > + if (err) > > + return err; > > + } > > + > > + if (tb[IFLA_BRIDGE_MRP_RING_ROLE]) { > > + struct br_mrp_ring_role *role = > > + nla_data(tb[IFLA_BRIDGE_MRP_RING_ROLE]); > > + > > + err = br_mrp_set_ring_role(br, role); > > + if (err) > > + return err; > > + } > > + > > + if (tb[IFLA_BRIDGE_MRP_START_TEST]) { > > + struct br_mrp_start_test *test = > > + nla_data(tb[IFLA_BRIDGE_MRP_START_TEST]); > > + > > + err = br_mrp_start_test(br, test); > > + if (err) > > + return err; > > + } > > + > > + return 0; > > +} > > + > > +int br_mrp_port_open(struct net_device *dev, u8 loc) > > +{ > > + struct net_bridge_port *p; > > + int err = 0; > > + > > + p = br_port_get_rcu(dev); > > + if (!p) { > > + err = -EINVAL; > > + goto out; > > + } > > + > > + p->loc = loc; > > + br_ifinfo_notify(RTM_NEWLINK, NULL, p); > > + > > +out: > > + return err; > > +} > > +EXPORT_SYMBOL(br_mrp_port_open); > > > > I just noticed the EXPORT_SYMBOL() here, why do you need it? Actually is not needed. But I am thinking to drop patch 4. The reason for patch nr 4, was that the drivers should notify when the ports lost the continuity. But currently there is no driver using the MRP then there is no point to have. I will add it back once the drivers start to use the MRP. > -- /Horatiu