Isolating DSA Slave Interfaces on a Bridge with Bridge Offloading

Isolating DSA Slave Interfaces on a Bridge with Bridge Offloading

[Arınç ÜNAL]

Hi all,

The company I work with has got a product with a Mediatek MT7530 switch. 
They want to offer isolation for the switch ports on a bridge. I have 
run a test on a slightly modified 5.17-rc1 kernel. These commands below 
should prevent communication between the two interfaces:

bridge link set dev sw0p4 isolated on

bridge link set dev sw0p3 isolated on

However, computers connected to each of these ports can still 
communicate with each other. Bridge TX forwarding offload is implemented 
on the MT7530 DSA driver.

What I understand is isolation works on the software and because of the 
bridge offloading feature, the frames never reach the CPU where we can 
block it.

Two solutions I can think of:

- Disable bridge offloading when isolation is enabled on a DSA slave 
interface. Not the best solution but seems easy to implement.

- When isolation is enabled on a DSA slave interface, do not mirror the 
related FDB entries to the switch hardware so we can keep the bridge 
offloading feature for other ports.

I suppose this could only be achieved on switch specific DSA drivers so 
the implementation would differ by each driver.

Cheers.
Arınç

Re: Isolating DSA Slave Interfaces on a Bridge with Bridge Offloading

[Vladimir Oltean]

Hello Arınç,

On Sun, Mar 13, 2022 at 02:23:47PM +0300, Arınç ÜNAL wrote:
> Hi all,
> 
> The company I work with has got a product with a Mediatek MT7530 switch.
> They want to offer isolation for the switch ports on a bridge. I have run a
> test on a slightly modified 5.17-rc1 kernel. These commands below should
> prevent communication between the two interfaces:
> 
> bridge link set dev sw0p4 isolated on
> 
> bridge link set dev sw0p3 isolated on
> 
> However, computers connected to each of these ports can still communicate
> with each other. Bridge TX forwarding offload is implemented on the MT7530
> DSA driver.
> 
> What I understand is isolation works on the software and because of the
> bridge offloading feature, the frames never reach the CPU where we can block
> it.
> 
> Two solutions I can think of:
> 
> - Disable bridge offloading when isolation is enabled on a DSA slave
> interface. Not the best solution but seems easy to implement.
> 
> - When isolation is enabled on a DSA slave interface, do not mirror the
> related FDB entries to the switch hardware so we can keep the bridge
> offloading feature for other ports.
> 
> I suppose this could only be achieved on switch specific DSA drivers so the
> implementation would differ by each driver.
> 
> Cheers.
> Arınç

To be clear, are you talking about a patched or unpatched upstream
kernel? Because mt7530 doesn't implement bridge TX forwarding offload.
This can be seen because it is missing the "*tx_fwd_offload = true;"
line from its ->port_bridge_join handler (of course, not only that).

You are probably confused as to why is the BR_ISOLATED brport flag is
not rejected by a switchdev interface when it will only lead to
incorrect behavior.

In fact we've had this discussion with Qingfang, who sent this patch to
allow switchdev drivers to *at the very least* reject the flag, but
didn't want to write up a correct commit description for the change:
https://lore.kernel.org/all/CALW65jbotyW0MSOd-bd1TH_mkiBWhhRCQ29jgn+d12rXdj2pZA@mail.gmail.com/T/
https://patchwork.kernel.org/project/netdevbpf/patch/20210811135247.1703496-1-dqfext@gmail.com/
https://patchwork.kernel.org/project/netdevbpf/patch/20210812142213.2251697-1-dqfext@gmail.com/

As a result, currently not even the correctness issue has still not yet
been fixed, let alone having any driver act upon the feature correctly.

In my opinion, it isn't mandatory that bridge ports with BR_ISOLATED
have forwarding handled in software. All that needs to change is that
their forwarding domain, as managed by the ->port_bridge_join and
->port_bridge_leave callbacks, is further restricted. Isolated ports can
forward only to non-isolated ports, and non-isolated ports cannot
forward to isolated ports. Things may become more interesting with
bridge TX forwarding offload though, but as I mentioned, at least
upstream this isn't a concern for mt7530 (yet).

Re: Isolating DSA Slave Interfaces on a Bridge with Bridge Offloading

[Arınç ÜNAL]

Hey Vladimir,

On 13/03/2022 16:32, Vladimir Oltean wrote:
> Hello Arınç,
> 
> On Sun, Mar 13, 2022 at 02:23:47PM +0300, Arınç ÜNAL wrote:
>> Hi all,
>>
>> The company I work with has got a product with a Mediatek MT7530 switch.
>> They want to offer isolation for the switch ports on a bridge. I have run a
>> test on a slightly modified 5.17-rc1 kernel. These commands below should
>> prevent communication between the two interfaces:
>>
>> bridge link set dev sw0p4 isolated on
>>
>> bridge link set dev sw0p3 isolated on
>>
>> However, computers connected to each of these ports can still communicate
>> with each other. Bridge TX forwarding offload is implemented on the MT7530
>> DSA driver.
>>
>> What I understand is isolation works on the software and because of the
>> bridge offloading feature, the frames never reach the CPU where we can block
>> it.
>>
>> Two solutions I can think of:
>>
>> - Disable bridge offloading when isolation is enabled on a DSA slave
>> interface. Not the best solution but seems easy to implement.
>>
>> - When isolation is enabled on a DSA slave interface, do not mirror the
>> related FDB entries to the switch hardware so we can keep the bridge
>> offloading feature for other ports.
>>
>> I suppose this could only be achieved on switch specific DSA drivers so the
>> implementation would differ by each driver.
>>
>> Cheers.
>> Arınç
> 
> To be clear, are you talking about a patched or unpatched upstream
> kernel? Because mt7530 doesn't implement bridge TX forwarding offload.
> This can be seen because it is missing the "*tx_fwd_offload = true;"
> line from its ->port_bridge_join handler (of course, not only that).

I'm compiling the kernel using OpenWrt SDK so it's patched but mt7530 
DSA driver is untouched. I've seen this commit which made me think of that:
https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/drivers/net/dsa/mt7530.c?id=b079922ba2acf072b23d82fa246a0d8de198f0a2

I'm very inexperienced in coding so I must've made a wrong assumption.

I've tested that mt7530 DSA driver delivers frames between switch ports 
without the frames appearing on either the slave interfaces or the 
master interface. I suppose I was confusing this feature with 
tx_fwd_offload?

For example, the current state of the rtl8365mb DSA driver doesn't do 
this. Each frame goes in and out of the DSA master interface to deliver 
frames between the switch ports.

> 
> You are probably confused as to why is the BR_ISOLATED brport flag is
> not rejected by a switchdev interface when it will only lead to
> incorrect behavior.
> 
> In fact we've had this discussion with Qingfang, who sent this patch to
> allow switchdev drivers to *at the very least* reject the flag, but
> didn't want to write up a correct commit description for the change:
> https://lore.kernel.org/all/CALW65jbotyW0MSOd-bd1TH_mkiBWhhRCQ29jgn+d12rXdj2pZA@mail.gmail.com/T/
> https://patchwork.kernel.org/project/netdevbpf/patch/20210811135247.1703496-1-dqfext@gmail.com/
> https://patchwork.kernel.org/project/netdevbpf/patch/20210812142213.2251697-1-dqfext@gmail.com/

With v2 applied, when I issue the port isolation command, I get 
"RTNETLINK answers: Not supported" as it's not implemented in mt7530 yet?

> 
> As a result, currently not even the correctness issue has still not yet
> been fixed, let alone having any driver act upon the feature correctly.

It's unfortunate that this patch was put on hold just because of 
incomplete commit description. In my eyes, this is a significant fix.

> 
> In my opinion, it isn't mandatory that bridge ports with BR_ISOLATED
> have forwarding handled in software. All that needs to change is that
> their forwarding domain, as managed by the ->port_bridge_join and
> ->port_bridge_leave callbacks, is further restricted. Isolated ports can
> forward only to non-isolated ports, and non-isolated ports cannot
> forward to isolated ports. Things may become more interesting with
> bridge TX forwarding offload though, but as I mentioned, at least
> upstream this isn't a concern for mt7530 (yet).

Makes sense, thank you.

Arınç

Re: Isolating DSA Slave Interfaces on a Bridge with Bridge Offloading

[Vladimir Oltean]

On Mon, Mar 14, 2022 at 04:13:47PM +0300, Arınç ÜNAL wrote:
> Hey Vladimir,
> 
> On 13/03/2022 16:32, Vladimir Oltean wrote:
> > Hello Arınç,
> > 
> > On Sun, Mar 13, 2022 at 02:23:47PM +0300, Arınç ÜNAL wrote:
> > > Hi all,
> > > 
> > > The company I work with has got a product with a Mediatek MT7530 switch.
> > > They want to offer isolation for the switch ports on a bridge. I have run a
> > > test on a slightly modified 5.17-rc1 kernel. These commands below should
> > > prevent communication between the two interfaces:
> > > 
> > > bridge link set dev sw0p4 isolated on
> > > 
> > > bridge link set dev sw0p3 isolated on
> > > 
> > > However, computers connected to each of these ports can still communicate
> > > with each other. Bridge TX forwarding offload is implemented on the MT7530
> > > DSA driver.
> > > 
> > > What I understand is isolation works on the software and because of the
> > > bridge offloading feature, the frames never reach the CPU where we can block
> > > it.
> > > 
> > > Two solutions I can think of:
> > > 
> > > - Disable bridge offloading when isolation is enabled on a DSA slave
> > > interface. Not the best solution but seems easy to implement.
> > > 
> > > - When isolation is enabled on a DSA slave interface, do not mirror the
> > > related FDB entries to the switch hardware so we can keep the bridge
> > > offloading feature for other ports.
> > > 
> > > I suppose this could only be achieved on switch specific DSA drivers so the
> > > implementation would differ by each driver.
> > > 
> > > Cheers.
> > > Arınç
> > 
> > To be clear, are you talking about a patched or unpatched upstream
> > kernel? Because mt7530 doesn't implement bridge TX forwarding offload.
> > This can be seen because it is missing the "*tx_fwd_offload = true;"
> > line from its ->port_bridge_join handler (of course, not only that).
> 
> I'm compiling the kernel using OpenWrt SDK so it's patched but mt7530 DSA
> driver is untouched. I've seen this commit which made me think of that:
> https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/drivers/net/dsa/mt7530.c?id=b079922ba2acf072b23d82fa246a0d8de198f0a2
> 
> I'm very inexperienced in coding so I must've made a wrong assumption.
> 
> I've tested that mt7530 DSA driver delivers frames between switch ports
> without the frames appearing on either the slave interfaces or the master
> interface. I suppose I was confusing this feature with tx_fwd_offload?
> 
> For example, the current state of the rtl8365mb DSA driver doesn't do this.
> Each frame goes in and out of the DSA master interface to deliver frames
> between the switch ports.

That is just "forwarding offload". It means "for each packet that a
bridged switch port receives from the outside world, look up the
hardware FDB and send the packet just to its intended destination. If
there is no FDB entry, flood the packet in the entire forwarding domain
and also to the CPU. The packet flooded to the CPU will have the
skb->offload_fwd_mark bit set to true by the hardware driver to indicate
to the bridge that flooding to this port's hardware domain has already
been taken care of. Only flooding towards other (foreign) bridge ports
needs to be done for this packet."

TX forwarding offload means "when the bridge wants to send a packet that
needs to be replicated multiple times (either flooded or plain multicast)
to swp0, swp1, swp2, swp3 (all in the same hardware forwarding domain),
avoid replicating it in software (skb_clone) and send it just once to
the device driver dealing with a port from that hardware domain (swp0).
The driver will inject that single packet into the hardware in such a
way that the packet is forwarded based on consulting the hardware FDB.
If the hardware FDB and the bridge's software FDB are in sync, then
(a) the packet was flooded by the bridge => the bridge has no FDB entry
    for it. The hardware FDB has no entry for it either => the hardware
    floods it too.
(b) the packet was multicast => the bridge has an MDB entry, and so does
    the hardware. The packet is forwarded to the ports in the MDB
    entry."

This feature is basically the TX equivalent of the basic forwarding
offload, hence the name. TX packets for which the bridge leaves the FDB
lookup (and replication) responsibility to hardware have
skb->offload_fwd_mark set to true, but by the bridge, this time.

I think it's very important to understand what the TX forwarding offload
feature was meant to do and how it was meant to operate, because there
was a discussion with Qingfang on this topic too, and it may be that
mt7530 switch supports this in a different way.
https://patchwork.kernel.org/project/netdevbpf/cover/20210703115705.1034112-1-vladimir.oltean@nxp.com/#24290759
The approach of populating a port mask with more than 1 bit set in the
tagger is complicated and may not even achieve all the benefits of TX
forwarding offload. Forcing an FDB/MDB lookup is what is desirable,
since typically this should also make those packets go through the
address learning process (they will be treated as data plane packets).
Address learning on the CPU port is desirable for packets send on behalf
of a software bridge, and undesirable for packets sent from a standalone
port.
If you can enable address learning in the tagger for the packets that
are sent with skb->offload_fwd_mark == true, then you should be able to
remove ds->assisted_learning_on_cpu_port and still get the benefit
described here:
https://patchwork.kernel.org/project/netdevbpf/patch/20210106095136.224739-7-olteanv@gmail.com/

> > 
> > You are probably confused as to why is the BR_ISOLATED brport flag is
> > not rejected by a switchdev interface when it will only lead to
> > incorrect behavior.
> > 
> > In fact we've had this discussion with Qingfang, who sent this patch to
> > allow switchdev drivers to *at the very least* reject the flag, but
> > didn't want to write up a correct commit description for the change:
> > https://lore.kernel.org/all/CALW65jbotyW0MSOd-bd1TH_mkiBWhhRCQ29jgn+d12rXdj2pZA@mail.gmail.com/T/
> > https://patchwork.kernel.org/project/netdevbpf/patch/20210811135247.1703496-1-dqfext@gmail.com/
> > https://patchwork.kernel.org/project/netdevbpf/patch/20210812142213.2251697-1-dqfext@gmail.com/
> 
> With v2 applied, when I issue the port isolation command, I get "RTNETLINK
> answers: Not supported" as it's not implemented in mt7530 yet?

The entire idea is to get -EINVAL as returned from here, because
BR_ISOLATED will be present in flags.mask:

static int
mt7530_port_pre_bridge_flags(struct dsa_switch *ds, int port,
			     struct switchdev_brport_flags flags,
			     struct netlink_ext_ack *extack)
{
	if (flags.mask & ~(BR_LEARNING | BR_FLOOD | BR_MCAST_FLOOD |
			   BR_BCAST_FLOOD))
		return -EINVAL;

	return 0;
}

Now, the entire error handling from br_switchdev_set_port_flag() is a
bit of a disaster.
https://elixir.bootlin.com/linux/latest/source/net/bridge/br_switchdev.c#L77

	/* We run from atomic context here */
	err = call_switchdev_notifiers(SWITCHDEV_PORT_ATTR_SET, p->dev,
				       &info.info, extack);
	err = notifier_to_errno(err);
	if (err == -EOPNOTSUPP) // Drivers need to return -EINVAL here to distinguish themselves from "no one responded to this notifier", which is a non-error.
		return 0;

	if (err) {
		if (extack && !extack->_msg)
			NL_SET_ERR_MSG_MOD(extack,
					   "bridge flag offload is not supported");
		return -EOPNOTSUPP; // However, any other error code passed from the driver, like -EINVAL, is ignored and replaced with -EOPNOTSUPP by the bridge.
	}

I think this is where the "Not supported" error message comes from, in
your case. It would be good to double-check with prints, though.

Also, you might want to consider compiling iproute2 with libmnl support,
to get extack messages from the kernel. Here it seems like you're
missing "bridge flag offload is not supported", which would have been a
clear indication that this is the code path that was taken.

> > As a result, currently not even the correctness issue has still not yet
> > been fixed, let alone having any driver act upon the feature correctly.
> 
> It's unfortunate that this patch was put on hold just because of incomplete
> commit description. In my eyes, this is a significant fix.

I agree here. You could send the patch yourself, after making some tests.

> > In my opinion, it isn't mandatory that bridge ports with BR_ISOLATED
> > have forwarding handled in software. All that needs to change is that
> > their forwarding domain, as managed by the ->port_bridge_join and
> > ->port_bridge_leave callbacks, is further restricted. Isolated ports can
> > forward only to non-isolated ports, and non-isolated ports cannot
> > forward to isolated ports. Things may become more interesting with
> > bridge TX forwarding offload though, but as I mentioned, at least
> > upstream this isn't a concern for mt7530 (yet).
> 
> Makes sense, thank you.
> 
> Arınç

Re: Isolating DSA Slave Interfaces on a Bridge with Bridge Offloading

[Arınç ÜNAL]

On 14/03/2022 17:00, Vladimir Oltean wrote:
> On Mon, Mar 14, 2022 at 04:13:47PM +0300, Arınç ÜNAL wrote:
>> Hey Vladimir,
>>
>> On 13/03/2022 16:32, Vladimir Oltean wrote:
>>> Hello Arınç,
>>>
>>> On Sun, Mar 13, 2022 at 02:23:47PM +0300, Arınç ÜNAL wrote:
>>>> Hi all,
>>>>
>>>> The company I work with has got a product with a Mediatek MT7530 switch.
>>>> They want to offer isolation for the switch ports on a bridge. I have run a
>>>> test on a slightly modified 5.17-rc1 kernel. These commands below should
>>>> prevent communication between the two interfaces:
>>>>
>>>> bridge link set dev sw0p4 isolated on
>>>>
>>>> bridge link set dev sw0p3 isolated on
>>>>
>>>> However, computers connected to each of these ports can still communicate
>>>> with each other. Bridge TX forwarding offload is implemented on the MT7530
>>>> DSA driver.
>>>>
>>>> What I understand is isolation works on the software and because of the
>>>> bridge offloading feature, the frames never reach the CPU where we can block
>>>> it.
>>>>
>>>> Two solutions I can think of:
>>>>
>>>> - Disable bridge offloading when isolation is enabled on a DSA slave
>>>> interface. Not the best solution but seems easy to implement.
>>>>
>>>> - When isolation is enabled on a DSA slave interface, do not mirror the
>>>> related FDB entries to the switch hardware so we can keep the bridge
>>>> offloading feature for other ports.
>>>>
>>>> I suppose this could only be achieved on switch specific DSA drivers so the
>>>> implementation would differ by each driver.
>>>>
>>>> Cheers.
>>>> Arınç
>>>
>>> To be clear, are you talking about a patched or unpatched upstream
>>> kernel? Because mt7530 doesn't implement bridge TX forwarding offload.
>>> This can be seen because it is missing the "*tx_fwd_offload = true;"
>>> line from its ->port_bridge_join handler (of course, not only that).
>>
>> I'm compiling the kernel using OpenWrt SDK so it's patched but mt7530 DSA
>> driver is untouched. I've seen this commit which made me think of that:
>> https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/drivers/net/dsa/mt7530.c?id=b079922ba2acf072b23d82fa246a0d8de198f0a2
>>
>> I'm very inexperienced in coding so I must've made a wrong assumption.
>>
>> I've tested that mt7530 DSA driver delivers frames between switch ports
>> without the frames appearing on either the slave interfaces or the master
>> interface. I suppose I was confusing this feature with tx_fwd_offload?
>>
>> For example, the current state of the rtl8365mb DSA driver doesn't do this.
>> Each frame goes in and out of the DSA master interface to deliver frames
>> between the switch ports.
> 
> That is just "forwarding offload". It means "for each packet that a
> bridged switch port receives from the outside world, look up the
> hardware FDB and send the packet just to its intended destination. If
> there is no FDB entry, flood the packet in the entire forwarding domain
> and also to the CPU. The packet flooded to the CPU will have the
> skb->offload_fwd_mark bit set to true by the hardware driver to indicate
> to the bridge that flooding to this port's hardware domain has already
> been taken care of. Only flooding towards other (foreign) bridge ports
> needs to be done for this packet."
> 
> TX forwarding offload means "when the bridge wants to send a packet that
> needs to be replicated multiple times (either flooded or plain multicast)
> to swp0, swp1, swp2, swp3 (all in the same hardware forwarding domain),
> avoid replicating it in software (skb_clone) and send it just once to
> the device driver dealing with a port from that hardware domain (swp0).
> The driver will inject that single packet into the hardware in such a
> way that the packet is forwarded based on consulting the hardware FDB.
> If the hardware FDB and the bridge's software FDB are in sync, then
> (a) the packet was flooded by the bridge => the bridge has no FDB entry
>      for it. The hardware FDB has no entry for it either => the hardware
>      floods it too.
> (b) the packet was multicast => the bridge has an MDB entry, and so does
>      the hardware. The packet is forwarded to the ports in the MDB
>      entry."
> 
> This feature is basically the TX equivalent of the basic forwarding
> offload, hence the name. TX packets for which the bridge leaves the FDB
> lookup (and replication) responsibility to hardware have
> skb->offload_fwd_mark set to true, but by the bridge, this time.
> 
> I think it's very important to understand what the TX forwarding offload
> feature was meant to do and how it was meant to operate, because there
> was a discussion with Qingfang on this topic too, and it may be that
> mt7530 switch supports this in a different way.
> https://patchwork.kernel.org/project/netdevbpf/cover/20210703115705.1034112-1-vladimir.oltean@nxp.com/#24290759
> The approach of populating a port mask with more than 1 bit set in the
> tagger is complicated and may not even achieve all the benefits of TX
> forwarding offload. Forcing an FDB/MDB lookup is what is desirable,
> since typically this should also make those packets go through the
> address learning process (they will be treated as data plane packets).
> Address learning on the CPU port is desirable for packets send on behalf
> of a software bridge, and undesirable for packets sent from a standalone
> port.
> If you can enable address learning in the tagger for the packets that
> are sent with skb->offload_fwd_mark == true, then you should be able to
> remove ds->assisted_learning_on_cpu_port and still get the benefit
> described here:
> https://patchwork.kernel.org/project/netdevbpf/patch/20210106095136.224739-7-olteanv@gmail.com/
> 
>>>
>>> You are probably confused as to why is the BR_ISOLATED brport flag is
>>> not rejected by a switchdev interface when it will only lead to
>>> incorrect behavior.
>>>
>>> In fact we've had this discussion with Qingfang, who sent this patch to
>>> allow switchdev drivers to *at the very least* reject the flag, but
>>> didn't want to write up a correct commit description for the change:
>>> https://lore.kernel.org/all/CALW65jbotyW0MSOd-bd1TH_mkiBWhhRCQ29jgn+d12rXdj2pZA@mail.gmail.com/T/
>>> https://patchwork.kernel.org/project/netdevbpf/patch/20210811135247.1703496-1-dqfext@gmail.com/
>>> https://patchwork.kernel.org/project/netdevbpf/patch/20210812142213.2251697-1-dqfext@gmail.com/
>>
>> With v2 applied, when I issue the port isolation command, I get "RTNETLINK
>> answers: Not supported" as it's not implemented in mt7530 yet?
> 
> The entire idea is to get -EINVAL as returned from here, because
> BR_ISOLATED will be present in flags.mask:
> 
> static int
> mt7530_port_pre_bridge_flags(struct dsa_switch *ds, int port,
> 			     struct switchdev_brport_flags flags,
> 			     struct netlink_ext_ack *extack)
> {
> 	if (flags.mask & ~(BR_LEARNING | BR_FLOOD | BR_MCAST_FLOOD |
> 			   BR_BCAST_FLOOD))
> 		return -EINVAL;
> 
> 	return 0;
> }
> 
> Now, the entire error handling from br_switchdev_set_port_flag() is a
> bit of a disaster.
> https://elixir.bootlin.com/linux/latest/source/net/bridge/br_switchdev.c#L77
> 
> 	/* We run from atomic context here */
> 	err = call_switchdev_notifiers(SWITCHDEV_PORT_ATTR_SET, p->dev,
> 				       &info.info, extack);
> 	err = notifier_to_errno(err);
> 	if (err == -EOPNOTSUPP) // Drivers need to return -EINVAL here to distinguish themselves from "no one responded to this notifier", which is a non-error.
> 		return 0;
> 
> 	if (err) {
> 		if (extack && !extack->_msg)
> 			NL_SET_ERR_MSG_MOD(extack,
> 					   "bridge flag offload is not supported");
> 		return -EOPNOTSUPP; // However, any other error code passed from the driver, like -EINVAL, is ignored and replaced with -EOPNOTSUPP by the bridge.
> 	}
> 
> I think this is where the "Not supported" error message comes from, in
> your case. It would be good to double-check with prints, though.
> 
> Also, you might want to consider compiling iproute2 with libmnl support,
> to get extack messages from the kernel. Here it seems like you're
> missing "bridge flag offload is not supported", which would have been a
> clear indication that this is the code path that was taken.

I added a pr_info here as it looked easier to test this:

	/* We run from atomic context here */
	err = call_switchdev_notifiers(SWITCHDEV_PORT_ATTR_SET, p->dev,
				       &info.info, extack);
	err = notifier_to_errno(err);
	if (err == -EOPNOTSUPP)
		return 0;

	if (err) {
		if (extack && !extack->_msg)
			pr_info("Port flag is not supported");
			NL_SET_ERR_MSG_MOD(extack,
					   "bridge flag offload is not supported");
		pr_info("Port flag is not supported");
		return -EOPNOTSUPP;
	}

Looks like this is the expected result:

root@OpenWrt:/# bridge link set dev sw0p3 isolated on
[   55.862862] Port flag is not supported
RTNETLINK answers: Not supported
root@OpenWrt:/# bridge link set dev sw0p3 isolated on
[   55.862893] Port flag is not supported
[   58.492025] Port flag is not supported
RTNETLINK answers: Not supported

> 
>>> As a result, currently not even the correctness issue has still not yet
>>> been fixed, let alone having any driver act upon the feature correctly.
>>
>> It's unfortunate that this patch was put on hold just because of incomplete
>> commit description. In my eyes, this is a significant fix.
> 
> I agree here. You could send the patch yourself, after making some tests.

Will do.

Arınç