| Message ID | 20240711045926.756958-1-elliot.ayrey@alliedtelesis.co.nz (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net,v2] net: bridge: mst: Check vlan state for egress decision | expand |
On 11/07/2024 07:59, Elliot Ayrey wrote: > If a port is blocking in the common instance but forwarding in an MST > instance, traffic egressing the bridge will be dropped because the > state of the common instance is overriding that of the MST instance. > > Fix this by skipping the port state check in MST mode to allow > checking the vlan state via br_allowed_egress(). This is similar to > what happens in br_handle_frame_finish() when checking ingress > traffic, which was introduced in the change below. > > Fixes: ec7328b59176 ("net: bridge: mst: Multiple Spanning Tree (MST) mode") > Signed-off-by: Elliot Ayrey <elliot.ayrey@alliedtelesis.co.nz> > --- > > v2: > - Restructure the MST mode check to make it read better > v1: https://lore.kernel.org/all/20240705030041.1248472-1-elliot.ayrey@alliedtelesis.co.nz/ > > net/bridge/br_forward.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/net/bridge/br_forward.c b/net/bridge/br_forward.c > index d97064d460dc..e63d6f6308f8 100644 > --- a/net/bridge/br_forward.c > +++ b/net/bridge/br_forward.c > @@ -25,8 +25,8 @@ static inline int should_deliver(const struct net_bridge_port *p, > > vg = nbp_vlan_group_rcu(p); > return ((p->flags & BR_HAIRPIN_MODE) || skb->dev != p->dev) && > - p->state == BR_STATE_FORWARDING && br_allowed_egress(vg, skb) && > - nbp_switchdev_allowed_egress(p, skb) && > + (br_mst_is_enabled(p->br) || state == BR_STATE_FORWARDING) && Does this compile at all? How exactly did you test this change? There is no "state" variable in that context.
On Thu, 11 Jul 2024, Nikolay Aleksandrov wrote: > On 11/07/2024 07:59, Elliot Ayrey wrote: > > If a port is blocking in the common instance but forwarding in an MST > > instance, traffic egressing the bridge will be dropped because the > > state of the common instance is overriding that of the MST instance. > > > > Fix this by skipping the port state check in MST mode to allow > > checking the vlan state via br_allowed_egress(). This is similar to > > what happens in br_handle_frame_finish() when checking ingress > > traffic, which was introduced in the change below. > > > > Fixes: ec7328b59176 ("net: bridge: mst: Multiple Spanning Tree (MST) mode") > > Signed-off-by: Elliot Ayrey <elliot.ayrey@alliedtelesis.co.nz> > > --- > > > > v2: > > - Restructure the MST mode check to make it read better > > v1: https://scanmail.trustwave.com/?c=20988&d=i-GP5uRIMfh6vd5ovR02aBzmN2wu2NxHqGSNFOAFMA&u=https%3a%2f%2flore%2ekernel%2eorg%2fall%2f20240705030041%2e1248472-1-elliot%2eayrey%40alliedtelesis%2eco%2enz%2f > > > > net/bridge/br_forward.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/net/bridge/br_forward.c b/net/bridge/br_forward.c > > index d97064d460dc..e63d6f6308f8 100644 > > --- a/net/bridge/br_forward.c > > +++ b/net/bridge/br_forward.c > > @@ -25,8 +25,8 @@ static inline int should_deliver(const struct net_bridge_port *p, > > > > vg = nbp_vlan_group_rcu(p); > > return ((p->flags & BR_HAIRPIN_MODE) || skb->dev != p->dev) && > > - p->state == BR_STATE_FORWARDING && br_allowed_egress(vg, skb) && > > - nbp_switchdev_allowed_egress(p, skb) && > > + (br_mst_is_enabled(p->br) || state == BR_STATE_FORWARDING) && > > Does this compile at all? How exactly did you test this change? > There is no "state" variable in that context. > My apologies I must have sent an older patch. I will re-test and submit a v3.
diff --git a/net/bridge/br_forward.c b/net/bridge/br_forward.c index d97064d460dc..e63d6f6308f8 100644 --- a/net/bridge/br_forward.c +++ b/net/bridge/br_forward.c @@ -25,8 +25,8 @@ static inline int should_deliver(const struct net_bridge_port *p, vg = nbp_vlan_group_rcu(p); return ((p->flags & BR_HAIRPIN_MODE) || skb->dev != p->dev) && - p->state == BR_STATE_FORWARDING && br_allowed_egress(vg, skb) && - nbp_switchdev_allowed_egress(p, skb) && + (br_mst_is_enabled(p->br) || state == BR_STATE_FORWARDING) && + br_allowed_egress(vg, skb) && nbp_switchdev_allowed_egress(p, skb) && !br_skb_isolated(p, skb); }
If a port is blocking in the common instance but forwarding in an MST instance, traffic egressing the bridge will be dropped because the state of the common instance is overriding that of the MST instance. Fix this by skipping the port state check in MST mode to allow checking the vlan state via br_allowed_egress(). This is similar to what happens in br_handle_frame_finish() when checking ingress traffic, which was introduced in the change below. Fixes: ec7328b59176 ("net: bridge: mst: Multiple Spanning Tree (MST) mode") Signed-off-by: Elliot Ayrey <elliot.ayrey@alliedtelesis.co.nz> --- v2: - Restructure the MST mode check to make it read better v1: https://lore.kernel.org/all/20240705030041.1248472-1-elliot.ayrey@alliedtelesis.co.nz/ net/bridge/br_forward.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)