Re: [PATCH net-next v1 2/4] net: hsr: fix VLAN add unwind on slave errors
From: Felix Maurer <hidden>
Date: 2026-03-26 14:29:40
Also in:
lkml
On Tue, Mar 24, 2026 at 03:35:01PM +0100, luka.gejak@linux.dev wrote:
From: Luka Gejak <redacted> When vlan_vid_add() fails for a secondary slave, the error path calls vlan_vid_del() on the failing port instead of the peer slave that had already succeeded. This results in asymmetric VLAN state across the HSR pair. Fix this by ensuring the cleanup path targets the previously programmed slave device when the second slave fails to add the VID.
The cross-dev cleanup is already complex enough and I don't think this makes it much better. How about having a central cleanup at the end that cleans whatever ports have the VID added?
quoted hunk ↗ jump to hunk
Signed-off-by: Luka Gejak <redacted> --- net/hsr/hsr_device.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)diff --git a/net/hsr/hsr_device.c b/net/hsr/hsr_device.c index 5c3eca2235ce..6185f9f2630f 100644 --- a/net/hsr/hsr_device.c +++ b/net/hsr/hsr_device.c@@ -534,6 +534,8 @@ static int hsr_ndo_vlan_rx_add_vid(struct net_device *dev, { bool is_slave_a_added = false; bool is_slave_b_added = false; + struct net_device *slave_a_dev = NULL; + struct net_device *slave_b_dev = NULL;
No matter if my previous comment, the is_slave_{a,b}_added bools are not
necessary with your changes but just duplicate state. Testing
slave_{a,b}_dev == NULL serves the same purpose.
quoted hunk ↗ jump to hunk
struct hsr_port *port; struct hsr_priv *hsr; int ret = 0;@@ -552,11 +554,12 @@ static int hsr_ndo_vlan_rx_add_vid(struct net_device *dev, /* clean up Slave-B */ netdev_err(dev, "add vid failed for Slave-A\n"); if (is_slave_b_added) - vlan_vid_del(port->dev, proto, vid); + vlan_vid_del(slave_b_dev, proto, vid); return ret; } is_slave_a_added = true; + slave_a_dev = port->dev; break; case HSR_PT_SLAVE_B:@@ -564,11 +567,12 @@ static int hsr_ndo_vlan_rx_add_vid(struct net_device *dev, /* clean up Slave-A */ netdev_err(dev, "add vid failed for Slave-B\n"); if (is_slave_a_added) - vlan_vid_del(port->dev, proto, vid); + vlan_vid_del(slave_a_dev, proto, vid); return ret; } is_slave_b_added = true; + slave_b_dev = port->dev; break; default: break; --2.53.0