mbox series

[v3,iproute2,0/4] Multiple Spanning Tree (MST) Support

Message ID 20240702120805.2391594-1-tobias@waldekranz.com (mailing list archive)
Headers show
Series Multiple Spanning Tree (MST) Support | expand

Message

Tobias Waldekranz July 2, 2024, 12:08 p.m. UTC
This series adds support for:

- Enabling MST on a bridge:

      ip link set dev <BR> type bridge mst_enable 1

- (Re)associating VLANs with an MSTI:

      bridge vlan global set dev <BR> vid <X> msti <Y>

- Setting the port state in a given MSTI:

      bridge mst set dev <PORT> msti <Y> state <Z>

- Listing the current port MST states:

      bridge mst show

NOTE: Multiple spanning tree support was added to Linux a couple of
years ago[1], but the corresponding iproute2 patches were never
posted. Mea culpa. Some time ago, this was brought to my attention[2],
which is why you are seeing them now.

[1]: https://lore.kernel.org/netdev/20220316150857.2442916-1-tobias@waldekranz.com/
[2]: https://lore.kernel.org/netdev/Zmsc54cVKF1wpzj7@Laptop-X1/

v2 -> v3:
- Added 2/4 bridge: Remove duplicated textification macros (Nikolay)
- Fold a conditional in to a switch mst.c (Nikolay)
- Give the full command to set a VLAN's MSTI in the man page (Nikolay)
- Use proper type for stp state (Stephen)

v1 -> v2:
- Require exact match for "mst_enabled" bridge option (Liu)

Tobias Waldekranz (4):
  ip: bridge: add support for mst_enabled
  bridge: Remove duplicated textification macros
  bridge: vlan: Add support for setting a VLANs MSTI
  bridge: mst: Add get/set support for MST states

 bridge/Makefile       |   2 +-
 bridge/br_common.h    |   1 +
 bridge/bridge.c       |   3 +-
 bridge/mst.c          | 258 ++++++++++++++++++++++++++++++++++++++++++
 bridge/vlan.c         |  54 +++++----
 bridge/vni.c          |  15 +--
 ip/iplink_bridge.c    |  19 ++++
 man/man8/bridge.8     |  66 ++++++++++-
 man/man8/ip-link.8.in |  14 +++
 9 files changed, 398 insertions(+), 34 deletions(-)
 create mode 100644 bridge/mst.c

Comments

patchwork-bot+netdevbpf@kernel.org July 5, 2024, 5:31 p.m. UTC | #1
Hello:

This series was applied to iproute2/iproute2.git (main)
by Stephen Hemminger <stephen@networkplumber.org>:

On Tue,  2 Jul 2024 14:08:01 +0200 you wrote:
> This series adds support for:
> 
> - Enabling MST on a bridge:
> 
>       ip link set dev <BR> type bridge mst_enable 1
> 
> - (Re)associating VLANs with an MSTI:
> 
> [...]

Here is the summary with links:
  - [v3,iproute2,1/4] ip: bridge: add support for mst_enabled
    https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/commit/?id=3018ea24f388
  - [v3,iproute2,2/4] bridge: Remove duplicated textification macros
    (no matching commit)
  - [v3,iproute2,3/4] bridge: vlan: Add support for setting a VLANs MSTI
    https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/commit/?id=151db40f1d3d
  - [v3,iproute2,4/4] bridge: mst: Add get/set support for MST states
    (no matching commit)

You are awesome, thank you!
David Ahern July 6, 2024, 12:53 a.m. UTC | #2
On 7/5/24 11:31 AM, patchwork-bot+netdevbpf@kernel.org wrote:
> Hello:
> 
> This series was applied to iproute2/iproute2.git (main)
> by Stephen Hemminger <stephen@networkplumber.org>:
> 

Why was this merged to the main repro? As a new feature to iproute2 this
should be committed to next and only put in main on the next dev cycle.
Stephen Hemminger July 6, 2024, 3:49 a.m. UTC | #3
On Fri, 5 Jul 2024 18:53:47 -0600
David Ahern <dsahern@kernel.org> wrote:

> On 7/5/24 11:31 AM, patchwork-bot+netdevbpf@kernel.org wrote:
> > Hello:
> > 
> > This series was applied to iproute2/iproute2.git (main)
> > by Stephen Hemminger <stephen@networkplumber.org>:
> >   
> 
> Why was this merged to the main repro? As a new feature to iproute2 this
> should be committed to next and only put in main on the next dev cycle.

Because the kernel support was already added, I prefer to not force waiting
for code that is non-intrusive and kernel support is already present.
David Ahern July 6, 2024, 3:26 p.m. UTC | #4
On 7/5/24 9:49 PM, Stephen Hemminger wrote:
> On Fri, 5 Jul 2024 18:53:47 -0600
> David Ahern <dsahern@kernel.org> wrote:
> 
>> On 7/5/24 11:31 AM, patchwork-bot+netdevbpf@kernel.org wrote:
>>> Hello:
>>>
>>> This series was applied to iproute2/iproute2.git (main)
>>> by Stephen Hemminger <stephen@networkplumber.org>:
>>>   
>>
>> Why was this merged to the main repro? As a new feature to iproute2 this
>> should be committed to next and only put in main on the next dev cycle.
> 
> Because the kernel support was already added, I prefer to not force waiting
> for code that is non-intrusive and kernel support is already present.

I have told multiple people - with you in CC - that is not how iproute2
branching works. People need to send userspace patches for iproute2 in
the same dev cycle as the kernel patches. You are now selectively
undermining that process. What is the point of -next branch then?
Stephen Hemminger July 6, 2024, 7:56 p.m. UTC | #5
On Sat, 6 Jul 2024 09:26:46 -0600
David Ahern <dsahern@kernel.org> wrote:

> On 7/5/24 9:49 PM, Stephen Hemminger wrote:
> > On Fri, 5 Jul 2024 18:53:47 -0600
> > David Ahern <dsahern@kernel.org> wrote:
> >   
> >> On 7/5/24 11:31 AM, patchwork-bot+netdevbpf@kernel.org wrote:  
> >>> Hello:
> >>>
> >>> This series was applied to iproute2/iproute2.git (main)
> >>> by Stephen Hemminger <stephen@networkplumber.org>:
> >>>     
> >>
> >> Why was this merged to the main repro? As a new feature to iproute2 this
> >> should be committed to next and only put in main on the next dev cycle.  
> > 
> > Because the kernel support was already added, I prefer to not force waiting
> > for code that is non-intrusive and kernel support is already present.  
> 
> I have told multiple people - with you in CC - that is not how iproute2
> branching works. People need to send userspace patches for iproute2 in
> the same dev cycle as the kernel patches. You are now selectively
> undermining that process. What is the point of -next branch then?
 
The original point was to have kernel -next and iproute2 -next branches
and have support arrive at same time on both sides. The problem is when
developers get behind, and the iproute2 patches arrive after the kernel cycle
and then would end up get delayed another 3 to 4 months.

Example:
	If mst had been submitted during 6.9 -next open window, then
	it would have arrived in iproute2 when -next was merged in May 2024 and
	would get released concurrently with 6.10 (July 2024).
	When MST was submitted later, if it goes through -next, then it would
	get merged to main in August 2024 and released concurrently with 6.11
	in October. By merging to main, it will be in July.

I understand your concern, and probably better not to have done it.
The problem with accepting things early is the review process gets
truncated, and new features often have lots of feedback.
David Ahern July 7, 2024, 4:16 p.m. UTC | #6
On 7/6/24 1:56 PM, Stephen Hemminger wrote:
> The original point was to have kernel -next and iproute2 -next branches
> and have support arrive at same time on both sides. The problem is when
> developers get behind, and the iproute2 patches arrive after the kernel cycle
> and then would end up get delayed another 3 to 4 months.

Then the userspace patches should be sent when the kernel patches are
merged. Period. no excuses. Any delay is on the developer.

> 
> Example:
> 	If mst had been submitted during 6.9 -next open window, then
> 	it would have arrived in iproute2 when -next was merged in May 2024 and
> 	would get released concurrently with 6.10 (July 2024).
> 	When MST was submitted later, if it goes through -next, then it would
> 	get merged to main in August 2024 and released concurrently with 6.11
> 	in October. By merging to main, it will be in July.

Same exact problem with netkit and I told Daniel no. We have a
development policy for new features; it must apply across the board to
all of them.

> 
> I understand your concern, and probably better not to have done it.

You applied patches for a new feature just a week or two before release.
It is just wrong. It would be best to either back up the branch or
revert them.

> The problem with accepting things early is the review process gets
> truncated, and new features often have lots of feedback.
> 

I see no problem here; that is normal development work.
Stephen Hemminger July 7, 2024, 9:16 p.m. UTC | #7
On Sun, 7 Jul 2024 10:16:11 -0600
David Ahern <dsahern@kernel.org> wrote:

> On 7/6/24 1:56 PM, Stephen Hemminger wrote:
> > The original point was to have kernel -next and iproute2 -next branches
> > and have support arrive at same time on both sides. The problem is when
> > developers get behind, and the iproute2 patches arrive after the kernel cycle
> > and then would end up get delayed another 3 to 4 months.  
> 
> Then the userspace patches should be sent when the kernel patches are
> merged. Period. no excuses. Any delay is on the developer.

I would suggest that the netdev maintainers not accept any new
feature to net-next (that uses iproute2) until/unless the iproute2 update
patch has been posted. This prevents this problem, and the problem of
getting userspace API wrong.

> 
> > 
> > Example:
> > 	If mst had been submitted during 6.9 -next open window, then
> > 	it would have arrived in iproute2 when -next was merged in May 2024 and
> > 	would get released concurrently with 6.10 (July 2024).
> > 	When MST was submitted later, if it goes through -next, then it would
> > 	get merged to main in August 2024 and released concurrently with 6.11
> > 	in October. By merging to main, it will be in July.  
> 
> Same exact problem with netkit and I told Daniel no. We have a
> development policy for new features; it must apply across the board to
> all of them.
> 
> > 
> > I understand your concern, and probably better not to have done it.  
> 
> You applied patches for a new feature just a week or two before release.
> It is just wrong. It would be best to either back up the branch or
> revert them.

Will backup the branch since these are the the last patches merged.
patchwork-bot+netdevbpf@kernel.org July 8, 2024, 10:40 p.m. UTC | #8
Hello:

This series was applied to iproute2/iproute2-next.git (main)
by David Ahern <dsahern@kernel.org>:

On Tue,  2 Jul 2024 14:08:01 +0200 you wrote:
> This series adds support for:
> 
> - Enabling MST on a bridge:
> 
>       ip link set dev <BR> type bridge mst_enable 1
> 
> - (Re)associating VLANs with an MSTI:
> 
> [...]

Here is the summary with links:
  - [v3,iproute2,1/4] ip: bridge: add support for mst_enabled
    https://git.kernel.org/pub/scm/network/iproute2/iproute2-next.git/commit/?id=60a95a8a2e45
  - [v3,iproute2,2/4] bridge: Remove duplicated textification macros
    (no matching commit)
  - [v3,iproute2,3/4] bridge: vlan: Add support for setting a VLANs MSTI
    https://git.kernel.org/pub/scm/network/iproute2/iproute2-next.git/commit/?id=ace3c9c1fefd
  - [v3,iproute2,4/4] bridge: mst: Add get/set support for MST states
    (no matching commit)

You are awesome, thank you!