On Sat, 30 Sep 2017 00:01:24 +0300
Nikolay Aleksandrov <nikolay@xxxxxxxxxxxxxxxxxxx> wrote:
> On 29/09/17 18:14, Stephen Hemminger wrote:
> > On Wed, 27 Sep 2017 16:12:44 +0300
> > Nikolay Aleksandrov <nikolay@xxxxxxxxxxxxxxxxxxx> wrote:
> >
> >> We need to be able to transparently forward most link-local frames via
> >> tunnels (e.g. vxlan, qinq). Currently the bridge's group_fwd_mask has a
> >> mask which restricts the forwarding of STP and LACP, but we need to be able
> >> to forward these over tunnels and control that forwarding on a per-port
> >> basis thus add a new per-port group_fwd_mask option which only disallows
> >> mac pause frames to be forwarded (they're always dropped anyway).
> >> The patch does not change the current default situation - all of the others
> >> are still restricted unless configured for forwarding.
> >> We have successfully tested this patch with LACP and STP forwarding over
> >> VxLAN and qinq tunnels.
> >>
> >> Signed-off-by: Nikolay Aleksandrov <nikolay@xxxxxxxxxxxxxxxxxxx>
> >
> > LACP is fine, but STP must not be forwarded if STP in user or kernel
> > mode is enabled.
> >
> > Please update this patch or revert it.
> >
>
> The default has not changed, STP is still _not_ forwarded. It can be only if explicitly
> requested by the user and that means the port might be participating in STP but not
> the bridge's STP, that is explicitly forward all STP frames from that port.
> I don't think we have to change anything.
>
You need to fail if user does something stupid. And DNR.
>From 16c42db4852234c13288922c1bffbf7650e08434 Mon Sep 17 00:00:00 2001
From: Stephen Hemminger <sthemmin@xxxxxxxxxxxxx>
Date: Fri, 29 Sep 2017 14:48:17 -0700
Subject: [PATCH] bridge: check for attempt to forward STP PDU's with STP
enabled
Move validation into set function and do not allow
configuring forwarding of STP packets if STP is enabled.
Fixes: 5af48b59f35c ("net: bridge: add per-port group_fwd_mask with less restrictions")
Signed-off-by: Stephen Hemminger <stephen@xxxxxxxxxxxxxxxxxx>
---
net/bridge/br_if.c | 13 +++++++++++++
net/bridge/br_netlink.c | 6 +++---
net/bridge/br_private.h | 1 +
net/bridge/br_sysfs_if.c | 6 +-----
4 files changed, 18 insertions(+), 8 deletions(-)
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index f3aef22931ab..a30e12f76266 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -654,3 +654,16 @@ void br_port_flags_change(struct net_bridge_port *p, unsigned long mask)
if (mask & BR_AUTO_MASK)
nbp_update_port_count(br);
}
+
+int br_set_group_fwd_mask(struct net_bridge_port *p, u16 fwd_mask)
+{
+ if (fwd_mask & BR_GROUPFWD_MACPAUSE)
+ return -EINVAL;
+
+ if ((fwd_mask & BR_GROUPFWD_STP) &&
+ (p->br->stp_enabled != BR_NO_STP))
+ return -EINVAL;
+
+ p->group_fwd_mask = fwd_mask;
+ return 0;
+}
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index dea88a255d26..7b16819ecb59 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -780,9 +780,9 @@ static int br_setport(struct net_bridge_port *p, struct nlattr *tb[])
if (tb[IFLA_BRPORT_GROUP_FWD_MASK]) {
u16 fwd_mask = nla_get_u16(tb[IFLA_BRPORT_GROUP_FWD_MASK]);
- if (fwd_mask & BR_GROUPFWD_MACPAUSE)
- return -EINVAL;
- p->group_fwd_mask = fwd_mask;
+ err = br_set_group_fwd_mask(p, fwd_mask);
+ if (err)
+ return err;
}
br_port_flags_change(p, old_flags ^ p->flags);
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 020c709a017f..734933bebb08 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -573,6 +573,7 @@ netdev_features_t br_features_recompute(struct net_bridge *br,
netdev_features_t features);
void br_port_flags_change(struct net_bridge_port *port, unsigned long mask);
void br_manage_promisc(struct net_bridge *br);
+int br_set_group_fwd_mask(struct net_bridge_port *p, u16 mask);
/* br_input.c */
int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb);
diff --git a/net/bridge/br_sysfs_if.c b/net/bridge/br_sysfs_if.c
index 9110d5e56085..f5871be10b24 100644
--- a/net/bridge/br_sysfs_if.c
+++ b/net/bridge/br_sysfs_if.c
@@ -173,11 +173,7 @@ static ssize_t show_group_fwd_mask(struct net_bridge_port *p, char *buf)
static int store_group_fwd_mask(struct net_bridge_port *p,
unsigned long v)
{
- if (v & BR_GROUPFWD_MACPAUSE)
- return -EINVAL;
- p->group_fwd_mask = v;
-
- return 0;
+ return br_set_group_fwd_mask(p, v);
}
static BRPORT_ATTR(group_fwd_mask, S_IRUGO | S_IWUSR, show_group_fwd_mask,
store_group_fwd_mask);
--
2.11.0
