[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <871qzdolec.fsf@waldekranz.com>
Date: Mon, 07 Mar 2022 16:03:07 +0100
From: Tobias Waldekranz <tobias@...dekranz.com>
To: Roopa Prabhu <roopa@...dia.com>,
Nikolay Aleksandrov <razor@...ckwall.org>, davem@...emloft.net,
kuba@...nel.org
Cc: Andrew Lunn <andrew@...n.ch>,
Vivien Didelot <vivien.didelot@...il.com>,
Florian Fainelli <f.fainelli@...il.com>,
Vladimir Oltean <olteanv@...il.com>,
Jiri Pirko <jiri@...nulli.us>,
Ivan Vecera <ivecera@...hat.com>,
Russell King <linux@...linux.org.uk>,
Petr Machata <petrm@...dia.com>,
Cooper Lees <me@...perlees.com>,
Ido Schimmel <idosch@...dia.com>,
Matt Johnston <matt@...econstruct.com.au>,
linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
bridge@...ts.linux-foundation.org
Subject: Re: [PATCH v2 net-next 03/10] net: bridge: mst: Support setting and
reporting MST port states
On Tue, Mar 01, 2022 at 17:53, Roopa Prabhu <roopa@...dia.com> wrote:
> On 3/1/22 15:19, Nikolay Aleksandrov wrote:
>> On 1 March 2022 11:03:14 CET, Tobias Waldekranz <tobias@...dekranz.com> wrote:
>>> Make it possible to change the port state in a given MSTI. This is
>>> done through a new netlink interface, since the MSTIs are objects in
>>> their own right. The proposed iproute2 interface would be:
>>>
>>> bridge mst set dev <PORT> msti <MSTI> state <STATE>
>>>
>>> Current states in all applicable MSTIs can also be dumped. The
>>> proposed iproute interface looks like this:
>>>
>>> $ bridge mst
>>> port msti
>>> vb1 0
>>> state forwarding
>>> 100
>>> state disabled
>>> vb2 0
>>> state forwarding
>>> 100
>>> state forwarding
>>>
>>> The preexisting per-VLAN states are still valid in the MST
>>> mode (although they are read-only), and can be queried as usual if one
>>> is interested in knowing a particular VLAN's state without having to
>>> care about the VID to MSTI mapping (in this example VLAN 20 and 30 are
>>> bound to MSTI 100):
>>>
>>> $ bridge -d vlan
>>> port vlan-id
>>> vb1 10
>>> state forwarding mcast_router 1
>>> 20
>>> state disabled mcast_router 1
>>> 30
>>> state disabled mcast_router 1
>>> 40
>>> state forwarding mcast_router 1
>>> vb2 10
>>> state forwarding mcast_router 1
>>> 20
>>> state forwarding mcast_router 1
>>> 30
>>> state forwarding mcast_router 1
>>> 40
>>> state forwarding mcast_router 1
>>>
>>> Signed-off-by: Tobias Waldekranz <tobias@...dekranz.com>
>>> ---
>>> include/uapi/linux/if_bridge.h | 16 +++
>>> include/uapi/linux/rtnetlink.h | 5 +
>>> net/bridge/br_mst.c | 244 +++++++++++++++++++++++++++++++++
>>> net/bridge/br_netlink.c | 3 +
>>> net/bridge/br_private.h | 4 +
>>> 5 files changed, 272 insertions(+)
>>>
>>> diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
>>> index b68016f625b7..784482527861 100644
>>> --- a/include/uapi/linux/if_bridge.h
>>> +++ b/include/uapi/linux/if_bridge.h
>>> @@ -785,4 +785,20 @@ enum {
>>> __BRIDGE_QUERIER_MAX
>>> };
>>> #define BRIDGE_QUERIER_MAX (__BRIDGE_QUERIER_MAX - 1)
>>> +
>>> +enum {
>>> + BRIDGE_MST_UNSPEC,
>>> + BRIDGE_MST_ENTRY,
>>> + __BRIDGE_MST_MAX,
>>> +};
>>> +#define BRIDGE_MST_MAX (__BRIDGE_MST_MAX - 1)
>>> +
>>> +enum {
>>> + BRIDGE_MST_ENTRY_UNSPEC,
>>> + BRIDGE_MST_ENTRY_MSTI,
>>> + BRIDGE_MST_ENTRY_STATE,
>>> + __BRIDGE_MST_ENTRY_MAX,
>>> +};
>>> +#define BRIDGE_MST_ENTRY_MAX (__BRIDGE_MST_ENTRY_MAX - 1)
>>> +
>>> #endif /* _UAPI_LINUX_IF_BRIDGE_H */
>>> diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h
>>> index 0970cb4b1b88..4a48f3ce862c 100644
>>> --- a/include/uapi/linux/rtnetlink.h
>>> +++ b/include/uapi/linux/rtnetlink.h
>>> @@ -192,6 +192,11 @@ enum {
>>> RTM_GETTUNNEL,
>>> #define RTM_GETTUNNEL RTM_GETTUNNEL
>>>
>>> + RTM_GETMST = 124 + 2,
>>> +#define RTM_GETMST RTM_GETMST
>>> + RTM_SETMST,
>>> +#define RTM_SETMST RTM_SETMST
>>> +
>> I think you should also update selinux (see nlmsgtab.c)
>> I'll think about this one, if there is some nice way to avoid the new rtm types.
>
> yes, since these are all port attributes, seems like 'bridge link set'
> should work
>
> Tobias, can you pls check if extending RTM_SETLINK (with AF_BRIDGE) is
> an option here ?
>
> ie via br_setlink
Yeah that makes sense. Not sure how I convinced myself that I needed a
separate rtm type for it. I will give it a try. Thanks!
Powered by blists - more mailing lists