| Message ID | 20220301123104.226731-2-mattias.forsblad+netdev@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | bridge: dsa: switchdev: mv88e6xxx: Implement local_receive bridge flag | expand |
On Tue, Mar 01, 2022 at 01:31:02PM +0100, Mattias Forsblad wrote: > This patch implements the bridge flag local_receive. When this > flag is cleared packets received on bridge ports will not be forwarded up. > This makes is possible to only forward traffic between the port members > of the bridge. > > Signed-off-by: Mattias Forsblad <mattias.forsblad+netdev@gmail.com> > --- > include/linux/if_bridge.h | 6 ++++++ > include/net/switchdev.h | 2 ++ Nik might ask you to split the offload part from the bridge implementation. Please wait for his feedback as he might be AFK right now > include/uapi/linux/if_bridge.h | 1 + > include/uapi/linux/if_link.h | 1 + > net/bridge/br.c | 18 ++++++++++++++++++ > net/bridge/br_device.c | 1 + > net/bridge/br_input.c | 3 +++ > net/bridge/br_ioctl.c | 1 + > net/bridge/br_netlink.c | 14 +++++++++++++- > net/bridge/br_private.h | 2 ++ > net/bridge/br_sysfs_br.c | 23 +++++++++++++++++++++++ I believe the bridge doesn't implement sysfs for new attributes > net/bridge/br_vlan.c | 8 ++++++++ > 12 files changed, 79 insertions(+), 1 deletion(-) [...] > diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c > index e0c13fcc50ed..5864b61157d3 100644 > --- a/net/bridge/br_input.c > +++ b/net/bridge/br_input.c > @@ -163,6 +163,9 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb > break; > } > > + if (local_rcv && !br_opt_get(br, BROPT_LOCAL_RECEIVE)) > + local_rcv = false; > + I don't think the description in the commit message is accurate: "packets received on bridge ports will not be forwarded up". From the code it seems that if packets hit a local FDB entry, then they will be "forwarded up". Instead, it seems that packets will not be flooded towards the bridge. In which case, why not maintain the same granularity we have for the rest of the ports and split this into unicast / multicast / broadcast? BTW, while the patch honors local FDB entries, it overrides host MDB entries which seems wrong / inconsistent. > if (dst) { > unsigned long now = jiffies;
On 1 March 2022 17:43:27 CET, Ido Schimmel <idosch@idosch.org> wrote: >On Tue, Mar 01, 2022 at 01:31:02PM +0100, Mattias Forsblad wrote: >> This patch implements the bridge flag local_receive. When this >> flag is cleared packets received on bridge ports will not be forwarded up. >> This makes is possible to only forward traffic between the port members >> of the bridge. >> >> Signed-off-by: Mattias Forsblad <mattias.forsblad+netdev@gmail.com> >> --- >> include/linux/if_bridge.h | 6 ++++++ >> include/net/switchdev.h | 2 ++ > >Nik might ask you to split the offload part from the bridge >implementation. Please wait for his feedback as he might be AFK right >now > Indeed, I'm traveling and won't have pc access until end of week (Sun). I'll try to review the patches through my phoneas much as I can. Ack on the split. >> include/uapi/linux/if_bridge.h | 1 + >> include/uapi/linux/if_link.h | 1 + >> net/bridge/br.c | 18 ++++++++++++++++++ >> net/bridge/br_device.c | 1 + >> net/bridge/br_input.c | 3 +++ >> net/bridge/br_ioctl.c | 1 + >> net/bridge/br_netlink.c | 14 +++++++++++++- >> net/bridge/br_private.h | 2 ++ >> net/bridge/br_sysfs_br.c | 23 +++++++++++++++++++++++ > >I believe the bridge doesn't implement sysfs for new attributes > Right, no new sysfs please. >> net/bridge/br_vlan.c | 8 ++++++++ >> 12 files changed, 79 insertions(+), 1 deletion(-) > >[...] > >> diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c >> index e0c13fcc50ed..5864b61157d3 100644 >> --- a/net/bridge/br_input.c >> +++ b/net/bridge/br_input.c >> @@ -163,6 +163,9 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb >> break; >> } >> >> + if (local_rcv && !br_opt_get(br, BROPT_LOCAL_RECEIVE)) >> + local_rcv = false; >> + > >I don't think the description in the commit message is accurate: >"packets received on bridge ports will not be forwarded up". From the >code it seems that if packets hit a local FDB entry, then they will be >"forwarded up". Instead, it seems that packets will not be flooded >towards the bridge. In which case, why not maintain the same granularity >we have for the rest of the ports and split this into unicast / >multicast / broadcast? > Exactly my first thought - why not implement the same control for the bridge? Also try to minimize the fast-path hit, you can keep the needed changes localized only to the cases where they are needed. I'll send a few more comments in a reply to the patch. >BTW, while the patch honors local FDB entries, it overrides host MDB >entries which seems wrong / inconsistent. > >> if (dst) { >> unsigned long now = jiffies;
On 1 March 2022 13:31:02 CET, Mattias Forsblad <mattias.forsblad@gmail.com> wrote: >This patch implements the bridge flag local_receive. When this >flag is cleared packets received on bridge ports will not be forwarded up. >This makes is possible to only forward traffic between the port members >of the bridge. > >Signed-off-by: Mattias Forsblad <mattias.forsblad+netdev@gmail.com> >--- > include/linux/if_bridge.h | 6 ++++++ > include/net/switchdev.h | 2 ++ > include/uapi/linux/if_bridge.h | 1 + > include/uapi/linux/if_link.h | 1 + > net/bridge/br.c | 18 ++++++++++++++++++ > net/bridge/br_device.c | 1 + > net/bridge/br_input.c | 3 +++ > net/bridge/br_ioctl.c | 1 + > net/bridge/br_netlink.c | 14 +++++++++++++- > net/bridge/br_private.h | 2 ++ > net/bridge/br_sysfs_br.c | 23 +++++++++++++++++++++++ > net/bridge/br_vlan.c | 8 ++++++++ > 12 files changed, 79 insertions(+), 1 deletion(-) > >diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h >index 3aae023a9353..e6b77d18c1d2 100644 >--- a/include/linux/if_bridge.h >+++ b/include/linux/if_bridge.h >@@ -157,6 +157,7 @@ static inline int br_vlan_get_info_rcu(const struct net_device *dev, u16 vid, > struct net_device *br_fdb_find_port(const struct net_device *br_dev, > const unsigned char *addr, > __u16 vid); >+bool br_local_receive_enabled(const struct net_device *dev); > void br_fdb_clear_offload(const struct net_device *dev, u16 vid); > bool br_port_flag_is_set(const struct net_device *dev, unsigned long flag); > u8 br_port_get_stp_state(const struct net_device *dev); >@@ -170,6 +171,11 @@ br_fdb_find_port(const struct net_device *br_dev, > return NULL; > } > >+static inline bool br_local_receive_enabled(const struct net_device *dev) >+{ >+ return true; >+} >+ > static inline void br_fdb_clear_offload(const struct net_device *dev, u16 vid) > { > } >diff --git a/include/net/switchdev.h b/include/net/switchdev.h >index 3e424d40fae3..aec5c1f9b5c7 100644 >--- a/include/net/switchdev.h >+++ b/include/net/switchdev.h >@@ -25,6 +25,7 @@ enum switchdev_attr_id { > SWITCHDEV_ATTR_ID_BRIDGE_AGEING_TIME, > SWITCHDEV_ATTR_ID_BRIDGE_VLAN_FILTERING, > SWITCHDEV_ATTR_ID_BRIDGE_VLAN_PROTOCOL, >+ SWITCHDEV_ATTR_ID_BRIDGE_LOCAL_RECEIVE, > SWITCHDEV_ATTR_ID_BRIDGE_MC_DISABLED, > SWITCHDEV_ATTR_ID_BRIDGE_MROUTER, > SWITCHDEV_ATTR_ID_MRP_PORT_ROLE, >@@ -50,6 +51,7 @@ struct switchdev_attr { > u16 vlan_protocol; /* BRIDGE_VLAN_PROTOCOL */ > bool mc_disabled; /* MC_DISABLED */ > u8 mrp_port_role; /* MRP_PORT_ROLE */ >+ bool local_receive; /* BRIDGE_LOCAL_RECEIVE */ > } u; > }; > >diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h >index 2711c3522010..fc889b5ccd69 100644 >--- a/include/uapi/linux/if_bridge.h >+++ b/include/uapi/linux/if_bridge.h >@@ -72,6 +72,7 @@ struct __bridge_info { > __u32 tcn_timer_value; > __u32 topology_change_timer_value; > __u32 gc_timer_value; >+ __u8 local_receive; > }; > > struct __port_info { >diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h >index e315e53125f4..bb7c25e1c89c 100644 >--- a/include/uapi/linux/if_link.h >+++ b/include/uapi/linux/if_link.h >@@ -482,6 +482,7 @@ enum { > IFLA_BR_VLAN_STATS_PER_PORT, > IFLA_BR_MULTI_BOOLOPT, > IFLA_BR_MCAST_QUERIER_STATE, >+ IFLA_BR_LOCAL_RECEIVE, Please use the boolopt api for new boolean options We're trying to limit the nl options expansion as the bridge is the largest user. > __IFLA_BR_MAX, > }; > >diff --git a/net/bridge/br.c b/net/bridge/br.c >index b1dea3febeea..ff7eb4f269ec 100644 >--- a/net/bridge/br.c >+++ b/net/bridge/br.c >@@ -325,6 +325,24 @@ void br_boolopt_multi_get(const struct net_bridge *br, > bm->optmask = GENMASK((BR_BOOLOPT_MAX - 1), 0); > } > >+int br_local_receive_change(struct net_bridge *p, >+ bool local_receive) >+{ >+ struct switchdev_attr attr = { >+ .orig_dev = p->dev, >+ .id = SWITCHDEV_ATTR_ID_BRIDGE_LOCAL_RECEIVE, >+ .flags = SWITCHDEV_F_DEFER, >+ .u.local_receive = local_receive, >+ }; >+ int ret; >+ >+ ret = switchdev_port_attr_set(p->dev, &attr, NULL); >+ if (!ret) >+ br_opt_toggle(p, BROPT_LOCAL_RECEIVE, local_receive); >+ >+ return ret; >+} >+ > /* private bridge options, controlled by the kernel */ > void br_opt_toggle(struct net_bridge *br, enum net_bridge_opts opt, bool on) > { >diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c >index 8d6bab244c4a..7cd9c5880d18 100644 >--- a/net/bridge/br_device.c >+++ b/net/bridge/br_device.c >@@ -524,6 +524,7 @@ void br_dev_setup(struct net_device *dev) > br->bridge_hello_time = br->hello_time = 2 * HZ; > br->bridge_forward_delay = br->forward_delay = 15 * HZ; > br->bridge_ageing_time = br->ageing_time = BR_DEFAULT_AGEING_TIME; >+ br_opt_toggle(br, BROPT_LOCAL_RECEIVE, true); > dev->max_mtu = ETH_MAX_MTU; > > br_netfilter_rtable_init(br); >diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c >index e0c13fcc50ed..5864b61157d3 100644 >--- a/net/bridge/br_input.c >+++ b/net/bridge/br_input.c >@@ -163,6 +163,9 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb > break; > } > >+ if (local_rcv && !br_opt_get(br, BROPT_LOCAL_RECEIVE)) >+ local_rcv = false; >+ this affects the whole fast path, it can be better localized to make sure it will not affect all use cases > if (dst) { > unsigned long now = jiffies; > >diff --git a/net/bridge/br_ioctl.c b/net/bridge/br_ioctl.c >index f213ed108361..abe538129290 100644 >--- a/net/bridge/br_ioctl.c >+++ b/net/bridge/br_ioctl.c >@@ -177,6 +177,7 @@ int br_dev_siocdevprivate(struct net_device *dev, struct ifreq *rq, > b.topology_change = br->topology_change; > b.topology_change_detected = br->topology_change_detected; > b.root_port = br->root_port; >+ b.local_receive = br_opt_get(br, BROPT_LOCAL_RECEIVE) ? 1 : 0; ioctl is not being extended anymore, please drop it > > b.stp_enabled = (br->stp_enabled != BR_NO_STP); > b.ageing_time = jiffies_to_clock_t(br->ageing_time); >diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c >index 7d4432ca9a20..5e7f99950195 100644 >--- a/net/bridge/br_netlink.c >+++ b/net/bridge/br_netlink.c >@@ -1163,6 +1163,7 @@ static const struct nla_policy br_policy[IFLA_BR_MAX + 1] = { > [IFLA_BR_MCAST_IGMP_VERSION] = { .type = NLA_U8 }, > [IFLA_BR_MCAST_MLD_VERSION] = { .type = NLA_U8 }, > [IFLA_BR_VLAN_STATS_PER_PORT] = { .type = NLA_U8 }, >+ [IFLA_BR_LOCAL_RECEIVE] = { .type = NLA_U8 }, > [IFLA_BR_MULTI_BOOLOPT] = > NLA_POLICY_EXACT_LEN(sizeof(struct br_boolopt_multi)), > }; >@@ -1434,6 +1435,14 @@ static int br_changelink(struct net_device *brdev, struct nlattr *tb[], > return err; > } > >+ if (data[IFLA_BR_LOCAL_RECEIVE]) { >+ u8 val = nla_get_u8(data[IFLA_BR_LOCAL_RECEIVE]); >+ >+ err = br_local_receive_change(br, !!val); >+ if (err) >+ return err; >+ } >+ > return 0; > } > >@@ -1514,6 +1523,7 @@ static size_t br_get_size(const struct net_device *brdev) > nla_total_size(sizeof(u8)) + /* IFLA_BR_NF_CALL_ARPTABLES */ > #endif > nla_total_size(sizeof(struct br_boolopt_multi)) + /* IFLA_BR_MULTI_BOOLOPT */ >+ nla_total_size(sizeof(u8)) + /* IFLA_BR_LOCAL_RECEIVE */ > 0; > } > >@@ -1527,6 +1537,7 @@ static int br_fill_info(struct sk_buff *skb, const struct net_device *brdev) > u32 stp_enabled = br->stp_enabled; > u16 priority = (br->bridge_id.prio[0] << 8) | br->bridge_id.prio[1]; > u8 vlan_enabled = br_vlan_enabled(br->dev); >+ u8 local_receive = br_opt_get(br, BROPT_LOCAL_RECEIVE) ? 1 : 0; > struct br_boolopt_multi bm; > u64 clockval; > >@@ -1563,7 +1574,8 @@ static int br_fill_info(struct sk_buff *skb, const struct net_device *brdev) > nla_put_u8(skb, IFLA_BR_TOPOLOGY_CHANGE_DETECTED, > br->topology_change_detected) || > nla_put(skb, IFLA_BR_GROUP_ADDR, ETH_ALEN, br->group_addr) || >- nla_put(skb, IFLA_BR_MULTI_BOOLOPT, sizeof(bm), &bm)) >+ nla_put(skb, IFLA_BR_MULTI_BOOLOPT, sizeof(bm), &bm) || >+ nla_put_u8(skb, IFLA_BR_LOCAL_RECEIVE, local_receive)) > return -EMSGSIZE; > > #ifdef CONFIG_BRIDGE_VLAN_FILTERING >diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h >index 48bc61ebc211..01fa5426094b 100644 >--- a/net/bridge/br_private.h >+++ b/net/bridge/br_private.h >@@ -445,6 +445,7 @@ enum net_bridge_opts { > BROPT_NO_LL_LEARN, > BROPT_VLAN_BRIDGE_BINDING, > BROPT_MCAST_VLAN_SNOOPING_ENABLED, >+ BROPT_LOCAL_RECEIVE, > }; > > struct net_bridge { >@@ -720,6 +721,7 @@ int br_boolopt_multi_toggle(struct net_bridge *br, > void br_boolopt_multi_get(const struct net_bridge *br, > struct br_boolopt_multi *bm); > void br_opt_toggle(struct net_bridge *br, enum net_bridge_opts opt, bool on); >+int br_local_receive_change(struct net_bridge *p, bool local_receive); > > /* br_device.c */ > void br_dev_setup(struct net_device *dev); >diff --git a/net/bridge/br_sysfs_br.c b/net/bridge/br_sysfs_br.c >index 3f7ca88c2aa3..9af0c2ba929c 100644 >--- a/net/bridge/br_sysfs_br.c >+++ b/net/bridge/br_sysfs_br.c >@@ -84,6 +84,28 @@ static ssize_t forward_delay_store(struct device *d, > } > static DEVICE_ATTR_RW(forward_delay); > >+static ssize_t local_receive_show(struct device *d, >+ struct device_attribute *attr, char *buf) >+{ >+ struct net_bridge *br = to_bridge(d); >+ >+ return sprintf(buf, "%u\n", br_opt_get(br, BROPT_LOCAL_RECEIVE)); >+} >+ >+static int set_local_receive(struct net_bridge *br, unsigned long val, >+ struct netlink_ext_ack *extack) >+{ >+ return br_local_receive_change(br, !!val); >+} >+ >+static ssize_t local_receive_store(struct device *d, >+ struct device_attribute *attr, >+ const char *buf, size_t len) >+{ >+ return store_bridge_parm(d, buf, len, set_local_receive); >+} >+static DEVICE_ATTR_RW(local_receive); >+ Drop sysfs too, netlink only > static ssize_t hello_time_show(struct device *d, struct device_attribute *attr, > char *buf) > { >@@ -950,6 +972,7 @@ static struct attribute *bridge_attrs[] = { > &dev_attr_group_addr.attr, > &dev_attr_flush.attr, > &dev_attr_no_linklocal_learn.attr, >+ &dev_attr_local_receive.attr, > #ifdef CONFIG_BRIDGE_IGMP_SNOOPING > &dev_attr_multicast_router.attr, > &dev_attr_multicast_snooping.attr, >diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c >index 7557e90b60e1..57dd14d5e360 100644 >--- a/net/bridge/br_vlan.c >+++ b/net/bridge/br_vlan.c >@@ -905,6 +905,14 @@ bool br_vlan_enabled(const struct net_device *dev) > } > EXPORT_SYMBOL_GPL(br_vlan_enabled); > >+bool br_local_receive_enabled(const struct net_device *dev) >+{ >+ struct net_bridge *br = netdev_priv(dev); >+ >+ return br_opt_get(br, BROPT_LOCAL_RECEIVE); >+} >+EXPORT_SYMBOL_GPL(br_local_receive_enabled); >+ What the hell is this doing in br_vlan.c??? > int br_vlan_get_proto(const struct net_device *dev, u16 *p_proto) > { > struct net_bridge *br = netdev_priv(dev);
On 3/1/22 08:43, Ido Schimmel wrote: > On Tue, Mar 01, 2022 at 01:31:02PM +0100, Mattias Forsblad wrote: >> This patch implements the bridge flag local_receive. When this >> flag is cleared packets received on bridge ports will not be forwarded up. >> This makes is possible to only forward traffic between the port members >> of the bridge. >> >> Signed-off-by: Mattias Forsblad <mattias.forsblad+netdev@gmail.com> >> --- >> include/linux/if_bridge.h | 6 ++++++ >> include/net/switchdev.h | 2 ++ > Nik might ask you to split the offload part from the bridge > implementation. Please wait for his feedback as he might be AFK right > now > >> include/uapi/linux/if_bridge.h | 1 + >> include/uapi/linux/if_link.h | 1 + >> net/bridge/br.c | 18 ++++++++++++++++++ >> net/bridge/br_device.c | 1 + >> net/bridge/br_input.c | 3 +++ >> net/bridge/br_ioctl.c | 1 + >> net/bridge/br_netlink.c | 14 +++++++++++++- >> net/bridge/br_private.h | 2 ++ >> net/bridge/br_sysfs_br.c | 23 +++++++++++++++++++++++ > I believe the bridge doesn't implement sysfs for new attributes > >> net/bridge/br_vlan.c | 8 ++++++++ >> 12 files changed, 79 insertions(+), 1 deletion(-) > [...] > >> diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c >> index e0c13fcc50ed..5864b61157d3 100644 >> --- a/net/bridge/br_input.c >> +++ b/net/bridge/br_input.c >> @@ -163,6 +163,9 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb >> break; >> } >> >> + if (local_rcv && !br_opt_get(br, BROPT_LOCAL_RECEIVE)) >> + local_rcv = false; >> + > I don't think the description in the commit message is accurate: > "packets received on bridge ports will not be forwarded up". From the > code it seems that if packets hit a local FDB entry, then they will be > "forwarded up". Instead, it seems that packets will not be flooded > towards the bridge. In which case, why not maintain the same granularity > we have for the rest of the ports and split this into unicast / > multicast / broadcast? > > BTW, while the patch honors local FDB entries, it overrides host MDB > entries which seems wrong / inconsistent. +1
On 2022-03-01 23:36, Nikolay Aleksandrov wrote: > On 1 March 2022 17:43:27 CET, Ido Schimmel <idosch@idosch.org> wrote: >> On Tue, Mar 01, 2022 at 01:31:02PM +0100, Mattias Forsblad wrote: >>> This patch implements the bridge flag local_receive. When this >>> flag is cleared packets received on bridge ports will not be forwarded up. >>> This makes is possible to only forward traffic between the port members >>> of the bridge. >>> >>> Signed-off-by: Mattias Forsblad <mattias.forsblad+netdev@gmail.com> >>> --- >>> include/linux/if_bridge.h | 6 ++++++ >>> include/net/switchdev.h | 2 ++ >> >> Nik might ask you to split the offload part from the bridge >> implementation. Please wait for his feedback as he might be AFK right >> now >> > > Indeed, I'm traveling and won't have pc access until end of week (Sun). > I'll try to review the patches through my phoneas much as I can. > Ack on the split. > I'll split the patch, thanks! >>> include/uapi/linux/if_bridge.h | 1 + >>> include/uapi/linux/if_link.h | 1 + >>> net/bridge/br.c | 18 ++++++++++++++++++ >>> net/bridge/br_device.c | 1 + >>> net/bridge/br_input.c | 3 +++ >>> net/bridge/br_ioctl.c | 1 + >>> net/bridge/br_netlink.c | 14 +++++++++++++- >>> net/bridge/br_private.h | 2 ++ >>> net/bridge/br_sysfs_br.c | 23 +++++++++++++++++++++++ >> >> I believe the bridge doesn't implement sysfs for new attributes >> > > Right, no new sysfs please. > Ok, I wasn't aware of that. I'll drop that part, thanks! >>> net/bridge/br_vlan.c | 8 ++++++++ >>> 12 files changed, 79 insertions(+), 1 deletion(-) >> >> [...] >> >>> diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c >>> index e0c13fcc50ed..5864b61157d3 100644 >>> --- a/net/bridge/br_input.c >>> +++ b/net/bridge/br_input.c >>> @@ -163,6 +163,9 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb >>> break; >>> } >>> >>> + if (local_rcv && !br_opt_get(br, BROPT_LOCAL_RECEIVE)) >>> + local_rcv = false; >>> + >> >> I don't think the description in the commit message is accurate: >> "packets received on bridge ports will not be forwarded up". From the >> code it seems that if packets hit a local FDB entry, then they will be >> "forwarded up". Instead, it seems that packets will not be flooded >> towards the bridge. In which case, why not maintain the same granularity >> we have for the rest of the ports and split this into unicast / >> multicast / broadcast? >> > > Exactly my first thought - why not implement the same control for the bridge? > Also try to minimize the fast-path hit, you can keep the needed changes > localized only to the cases where they are needed. > I'll send a few more comments in a reply to the patch. > Soo, if I understand you correctly, you want to have three different options? local_receive_unicast local_receive_multicast local_receive_broadcast >> BTW, while the patch honors local FDB entries, it overrides host MDB >> entries which seems wrong / inconsistent. >> >>> if (dst) { >>> unsigned long now = jiffies; >
On 2022-03-01 23:43, Nikolay Aleksandrov wrote: > On 1 March 2022 13:31:02 CET, Mattias Forsblad <mattias.forsblad@gmail.com> wrote: >> This patch implements the bridge flag local_receive. When this >> flag is cleared packets received on bridge ports will not be forwarded up. >> This makes is possible to only forward traffic between the port members >> of the bridge. >> >> Signed-off-by: Mattias Forsblad <mattias.forsblad+netdev@gmail.com> >> --- >> include/linux/if_bridge.h | 6 ++++++ >> include/net/switchdev.h | 2 ++ >> include/uapi/linux/if_bridge.h | 1 + >> include/uapi/linux/if_link.h | 1 + >> net/bridge/br.c | 18 ++++++++++++++++++ >> net/bridge/br_device.c | 1 + >> net/bridge/br_input.c | 3 +++ >> net/bridge/br_ioctl.c | 1 + >> net/bridge/br_netlink.c | 14 +++++++++++++- >> net/bridge/br_private.h | 2 ++ >> net/bridge/br_sysfs_br.c | 23 +++++++++++++++++++++++ >> net/bridge/br_vlan.c | 8 ++++++++ >> 12 files changed, 79 insertions(+), 1 deletion(-) >> >> diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h >> index 3aae023a9353..e6b77d18c1d2 100644 >> --- a/include/linux/if_bridge.h >> +++ b/include/linux/if_bridge.h >> @@ -157,6 +157,7 @@ static inline int br_vlan_get_info_rcu(const struct net_device *dev, u16 vid, >> struct net_device *br_fdb_find_port(const struct net_device *br_dev, >> const unsigned char *addr, >> __u16 vid); >> +bool br_local_receive_enabled(const struct net_device *dev); >> void br_fdb_clear_offload(const struct net_device *dev, u16 vid); >> bool br_port_flag_is_set(const struct net_device *dev, unsigned long flag); >> u8 br_port_get_stp_state(const struct net_device *dev); >> @@ -170,6 +171,11 @@ br_fdb_find_port(const struct net_device *br_dev, >> return NULL; >> } >> >> +static inline bool br_local_receive_enabled(const struct net_device *dev) >> +{ >> + return true; >> +} >> + >> static inline void br_fdb_clear_offload(const struct net_device *dev, u16 vid) >> { >> } >> diff --git a/include/net/switchdev.h b/include/net/switchdev.h >> index 3e424d40fae3..aec5c1f9b5c7 100644 >> --- a/include/net/switchdev.h >> +++ b/include/net/switchdev.h >> @@ -25,6 +25,7 @@ enum switchdev_attr_id { >> SWITCHDEV_ATTR_ID_BRIDGE_AGEING_TIME, >> SWITCHDEV_ATTR_ID_BRIDGE_VLAN_FILTERING, >> SWITCHDEV_ATTR_ID_BRIDGE_VLAN_PROTOCOL, >> + SWITCHDEV_ATTR_ID_BRIDGE_LOCAL_RECEIVE, >> SWITCHDEV_ATTR_ID_BRIDGE_MC_DISABLED, >> SWITCHDEV_ATTR_ID_BRIDGE_MROUTER, >> SWITCHDEV_ATTR_ID_MRP_PORT_ROLE, >> @@ -50,6 +51,7 @@ struct switchdev_attr { >> u16 vlan_protocol; /* BRIDGE_VLAN_PROTOCOL */ >> bool mc_disabled; /* MC_DISABLED */ >> u8 mrp_port_role; /* MRP_PORT_ROLE */ >> + bool local_receive; /* BRIDGE_LOCAL_RECEIVE */ >> } u; >> }; >> >> diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h >> index 2711c3522010..fc889b5ccd69 100644 >> --- a/include/uapi/linux/if_bridge.h >> +++ b/include/uapi/linux/if_bridge.h >> @@ -72,6 +72,7 @@ struct __bridge_info { >> __u32 tcn_timer_value; >> __u32 topology_change_timer_value; >> __u32 gc_timer_value; >> + __u8 local_receive; >> }; >> >> struct __port_info { >> diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h >> index e315e53125f4..bb7c25e1c89c 100644 >> --- a/include/uapi/linux/if_link.h >> +++ b/include/uapi/linux/if_link.h >> @@ -482,6 +482,7 @@ enum { >> IFLA_BR_VLAN_STATS_PER_PORT, >> IFLA_BR_MULTI_BOOLOPT, >> IFLA_BR_MCAST_QUERIER_STATE, >> + IFLA_BR_LOCAL_RECEIVE, > > Please use the boolopt api for new boolean options > We're trying to limit the nl options expansion as the bridge is the > largest user. > I'll move to that api, thanks! >> __IFLA_BR_MAX, >> }; >> >> diff --git a/net/bridge/br.c b/net/bridge/br.c >> index b1dea3febeea..ff7eb4f269ec 100644 >> --- a/net/bridge/br.c >> +++ b/net/bridge/br.c >> @@ -325,6 +325,24 @@ void br_boolopt_multi_get(const struct net_bridge *br, >> bm->optmask = GENMASK((BR_BOOLOPT_MAX - 1), 0); >> } >> >> +int br_local_receive_change(struct net_bridge *p, >> + bool local_receive) >> +{ >> + struct switchdev_attr attr = { >> + .orig_dev = p->dev, >> + .id = SWITCHDEV_ATTR_ID_BRIDGE_LOCAL_RECEIVE, >> + .flags = SWITCHDEV_F_DEFER, >> + .u.local_receive = local_receive, >> + }; >> + int ret; >> + >> + ret = switchdev_port_attr_set(p->dev, &attr, NULL); >> + if (!ret) >> + br_opt_toggle(p, BROPT_LOCAL_RECEIVE, local_receive); >> + >> + return ret; >> +} >> + >> /* private bridge options, controlled by the kernel */ >> void br_opt_toggle(struct net_bridge *br, enum net_bridge_opts opt, bool on) >> { >> diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c >> index 8d6bab244c4a..7cd9c5880d18 100644 >> --- a/net/bridge/br_device.c >> +++ b/net/bridge/br_device.c >> @@ -524,6 +524,7 @@ void br_dev_setup(struct net_device *dev) >> br->bridge_hello_time = br->hello_time = 2 * HZ; >> br->bridge_forward_delay = br->forward_delay = 15 * HZ; >> br->bridge_ageing_time = br->ageing_time = BR_DEFAULT_AGEING_TIME; >> + br_opt_toggle(br, BROPT_LOCAL_RECEIVE, true); >> dev->max_mtu = ETH_MAX_MTU; >> >> br_netfilter_rtable_init(br); >> diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c >> index e0c13fcc50ed..5864b61157d3 100644 >> --- a/net/bridge/br_input.c >> +++ b/net/bridge/br_input.c >> @@ -163,6 +163,9 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb >> break; >> } >> >> + if (local_rcv && !br_opt_get(br, BROPT_LOCAL_RECEIVE)) >> + local_rcv = false; >> + > > this affects the whole fast path, it can be better localized to make sure > it will not affect all use cases > >> if (dst) { >> unsigned long now = jiffies; >> >> diff --git a/net/bridge/br_ioctl.c b/net/bridge/br_ioctl.c >> index f213ed108361..abe538129290 100644 >> --- a/net/bridge/br_ioctl.c >> +++ b/net/bridge/br_ioctl.c >> @@ -177,6 +177,7 @@ int br_dev_siocdevprivate(struct net_device *dev, struct ifreq *rq, >> b.topology_change = br->topology_change; >> b.topology_change_detected = br->topology_change_detected; >> b.root_port = br->root_port; >> + b.local_receive = br_opt_get(br, BROPT_LOCAL_RECEIVE) ? 1 : 0; > > ioctl is not being extended anymore, please drop it > I'll drop it, thanks! >> >> b.stp_enabled = (br->stp_enabled != BR_NO_STP); >> b.ageing_time = jiffies_to_clock_t(br->ageing_time); >> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c >> index 7d4432ca9a20..5e7f99950195 100644 >> --- a/net/bridge/br_netlink.c >> +++ b/net/bridge/br_netlink.c >> @@ -1163,6 +1163,7 @@ static const struct nla_policy br_policy[IFLA_BR_MAX + 1] = { >> [IFLA_BR_MCAST_IGMP_VERSION] = { .type = NLA_U8 }, >> [IFLA_BR_MCAST_MLD_VERSION] = { .type = NLA_U8 }, >> [IFLA_BR_VLAN_STATS_PER_PORT] = { .type = NLA_U8 }, >> + [IFLA_BR_LOCAL_RECEIVE] = { .type = NLA_U8 }, >> [IFLA_BR_MULTI_BOOLOPT] = >> NLA_POLICY_EXACT_LEN(sizeof(struct br_boolopt_multi)), >> }; >> @@ -1434,6 +1435,14 @@ static int br_changelink(struct net_device *brdev, struct nlattr *tb[], >> return err; >> } >> >> + if (data[IFLA_BR_LOCAL_RECEIVE]) { >> + u8 val = nla_get_u8(data[IFLA_BR_LOCAL_RECEIVE]); >> + >> + err = br_local_receive_change(br, !!val); >> + if (err) >> + return err; >> + } >> + >> return 0; >> } >> >> @@ -1514,6 +1523,7 @@ static size_t br_get_size(const struct net_device *brdev) >> nla_total_size(sizeof(u8)) + /* IFLA_BR_NF_CALL_ARPTABLES */ >> #endif >> nla_total_size(sizeof(struct br_boolopt_multi)) + /* IFLA_BR_MULTI_BOOLOPT */ >> + nla_total_size(sizeof(u8)) + /* IFLA_BR_LOCAL_RECEIVE */ >> 0; >> } >> >> @@ -1527,6 +1537,7 @@ static int br_fill_info(struct sk_buff *skb, const struct net_device *brdev) >> u32 stp_enabled = br->stp_enabled; >> u16 priority = (br->bridge_id.prio[0] << 8) | br->bridge_id.prio[1]; >> u8 vlan_enabled = br_vlan_enabled(br->dev); >> + u8 local_receive = br_opt_get(br, BROPT_LOCAL_RECEIVE) ? 1 : 0; >> struct br_boolopt_multi bm; >> u64 clockval; >> >> @@ -1563,7 +1574,8 @@ static int br_fill_info(struct sk_buff *skb, const struct net_device *brdev) >> nla_put_u8(skb, IFLA_BR_TOPOLOGY_CHANGE_DETECTED, >> br->topology_change_detected) || >> nla_put(skb, IFLA_BR_GROUP_ADDR, ETH_ALEN, br->group_addr) || >> - nla_put(skb, IFLA_BR_MULTI_BOOLOPT, sizeof(bm), &bm)) >> + nla_put(skb, IFLA_BR_MULTI_BOOLOPT, sizeof(bm), &bm) || >> + nla_put_u8(skb, IFLA_BR_LOCAL_RECEIVE, local_receive)) >> return -EMSGSIZE; >> >> #ifdef CONFIG_BRIDGE_VLAN_FILTERING >> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h >> index 48bc61ebc211..01fa5426094b 100644 >> --- a/net/bridge/br_private.h >> +++ b/net/bridge/br_private.h >> @@ -445,6 +445,7 @@ enum net_bridge_opts { >> BROPT_NO_LL_LEARN, >> BROPT_VLAN_BRIDGE_BINDING, >> BROPT_MCAST_VLAN_SNOOPING_ENABLED, >> + BROPT_LOCAL_RECEIVE, >> }; >> >> struct net_bridge { >> @@ -720,6 +721,7 @@ int br_boolopt_multi_toggle(struct net_bridge *br, >> void br_boolopt_multi_get(const struct net_bridge *br, >> struct br_boolopt_multi *bm); >> void br_opt_toggle(struct net_bridge *br, enum net_bridge_opts opt, bool on); >> +int br_local_receive_change(struct net_bridge *p, bool local_receive); >> >> /* br_device.c */ >> void br_dev_setup(struct net_device *dev); >> diff --git a/net/bridge/br_sysfs_br.c b/net/bridge/br_sysfs_br.c >> index 3f7ca88c2aa3..9af0c2ba929c 100644 >> --- a/net/bridge/br_sysfs_br.c >> +++ b/net/bridge/br_sysfs_br.c >> @@ -84,6 +84,28 @@ static ssize_t forward_delay_store(struct device *d, >> } >> static DEVICE_ATTR_RW(forward_delay); >> >> +static ssize_t local_receive_show(struct device *d, >> + struct device_attribute *attr, char *buf) >> +{ >> + struct net_bridge *br = to_bridge(d); >> + >> + return sprintf(buf, "%u\n", br_opt_get(br, BROPT_LOCAL_RECEIVE)); >> +} >> + >> +static int set_local_receive(struct net_bridge *br, unsigned long val, >> + struct netlink_ext_ack *extack) >> +{ >> + return br_local_receive_change(br, !!val); >> +} >> + >> +static ssize_t local_receive_store(struct device *d, >> + struct device_attribute *attr, >> + const char *buf, size_t len) >> +{ >> + return store_bridge_parm(d, buf, len, set_local_receive); >> +} >> +static DEVICE_ATTR_RW(local_receive); >> + > > Drop sysfs too, netlink only > Yes, will do, thanks! >> static ssize_t hello_time_show(struct device *d, struct device_attribute *attr, >> char *buf) >> { >> @@ -950,6 +972,7 @@ static struct attribute *bridge_attrs[] = { >> &dev_attr_group_addr.attr, >> &dev_attr_flush.attr, >> &dev_attr_no_linklocal_learn.attr, >> + &dev_attr_local_receive.attr, >> #ifdef CONFIG_BRIDGE_IGMP_SNOOPING >> &dev_attr_multicast_router.attr, >> &dev_attr_multicast_snooping.attr, >> diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c >> index 7557e90b60e1..57dd14d5e360 100644 >> --- a/net/bridge/br_vlan.c >> +++ b/net/bridge/br_vlan.c >> @@ -905,6 +905,14 @@ bool br_vlan_enabled(const struct net_device *dev) >> } >> EXPORT_SYMBOL_GPL(br_vlan_enabled); >> >> +bool br_local_receive_enabled(const struct net_device *dev) >> +{ >> + struct net_bridge *br = netdev_priv(dev); >> + >> + return br_opt_get(br, BROPT_LOCAL_RECEIVE); >> +} >> +EXPORT_SYMBOL_GPL(br_local_receive_enabled); >> + > > What the hell is this doing in br_vlan.c??? > >> int br_vlan_get_proto(const struct net_device *dev, u16 *p_proto) >> { >> struct net_bridge *br = netdev_priv(dev); >
On 2022-03-01 23:43, Nikolay Aleksandrov wrote: > On 1 March 2022 13:31:02 CET, Mattias Forsblad <mattias.forsblad@gmail.com> wrote: >> This patch implements the bridge flag local_receive. When this >> flag is cleared packets received on bridge ports will not be forwarded up. >> This makes is possible to only forward traffic between the port members >> of the bridge. >> >> Signed-off-by: Mattias Forsblad <mattias.forsblad+netdev@gmail.com> >> --- >> diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c >> index 7557e90b60e1..57dd14d5e360 100644 >> --- a/net/bridge/br_vlan.c >> +++ b/net/bridge/br_vlan.c >> @@ -905,6 +905,14 @@ bool br_vlan_enabled(const struct net_device *dev) >> } >> EXPORT_SYMBOL_GPL(br_vlan_enabled); >> >> +bool br_local_receive_enabled(const struct net_device *dev) >> +{ >> + struct net_bridge *br = netdev_priv(dev); >> + >> + return br_opt_get(br, BROPT_LOCAL_RECEIVE); >> +} >> +EXPORT_SYMBOL_GPL(br_local_receive_enabled); >> + > > What the hell is this doing in br_vlan.c??? > I'm truly sorry to have made an error, might I inqire for a better approach? >> int br_vlan_get_proto(const struct net_device *dev, u16 *p_proto) >> { >> struct net_bridge *br = netdev_priv(dev); >
On Wed, Mar 02, 2022 at 07:27:25AM +0100, Mattias Forsblad wrote: > On 2022-03-01 23:36, Nikolay Aleksandrov wrote: > > On 1 March 2022 17:43:27 CET, Ido Schimmel <idosch@idosch.org> wrote: > >> On Tue, Mar 01, 2022 at 01:31:02PM +0100, Mattias Forsblad wrote: > >>> diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c > >>> index e0c13fcc50ed..5864b61157d3 100644 > >>> --- a/net/bridge/br_input.c > >>> +++ b/net/bridge/br_input.c > >>> @@ -163,6 +163,9 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb > >>> break; > >>> } > >>> > >>> + if (local_rcv && !br_opt_get(br, BROPT_LOCAL_RECEIVE)) > >>> + local_rcv = false; > >>> + > >> > >> I don't think the description in the commit message is accurate: > >> "packets received on bridge ports will not be forwarded up". From the > >> code it seems that if packets hit a local FDB entry, then they will be > >> "forwarded up". Instead, it seems that packets will not be flooded > >> towards the bridge. In which case, why not maintain the same granularity > >> we have for the rest of the ports and split this into unicast / > >> multicast / broadcast? > >> > > > > Exactly my first thought - why not implement the same control for the bridge? > > Also try to minimize the fast-path hit, you can keep the needed changes > > localized only to the cases where they are needed. > > I'll send a few more comments in a reply to the patch. > > > > Soo, if I understand you correctly, you want to have three different options? > local_receive_unicast > local_receive_multicast > local_receive_broadcast My understanding of the feature is that you want to prevent flooding towards the bridge. In which case, it makes sense to keep the same granularity as for regular bridge ports and also name the options similarly. We already have several options that are applicable to both the bridge and bridge ports (e.g., 'mcast_router'). I suggest: $ ip link help bridge Usage: ... bridge [ fdb_flush ] ... [ flood {on | off} ] [ mcast_flood {on | off} ] [ bcast_flood {on | off} ] This is consistent with "bridge_slave".
On Mon, Mar 14, 2022 at 06:29:58PM +0200, Ido Schimmel wrote: > On Wed, Mar 02, 2022 at 07:27:25AM +0100, Mattias Forsblad wrote: > > On 2022-03-01 23:36, Nikolay Aleksandrov wrote: > > > On 1 March 2022 17:43:27 CET, Ido Schimmel <idosch@idosch.org> wrote: > > >> On Tue, Mar 01, 2022 at 01:31:02PM +0100, Mattias Forsblad wrote: > > >>> diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c > > >>> index e0c13fcc50ed..5864b61157d3 100644 > > >>> --- a/net/bridge/br_input.c > > >>> +++ b/net/bridge/br_input.c > > >>> @@ -163,6 +163,9 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb > > >>> break; > > >>> } > > >>> > > >>> + if (local_rcv && !br_opt_get(br, BROPT_LOCAL_RECEIVE)) > > >>> + local_rcv = false; > > >>> + > > >> > > >> I don't think the description in the commit message is accurate: > > >> "packets received on bridge ports will not be forwarded up". From the > > >> code it seems that if packets hit a local FDB entry, then they will be > > >> "forwarded up". Instead, it seems that packets will not be flooded > > >> towards the bridge. In which case, why not maintain the same granularity > > >> we have for the rest of the ports and split this into unicast / > > >> multicast / broadcast? > > >> > > > > > > Exactly my first thought - why not implement the same control for the bridge? > > > Also try to minimize the fast-path hit, you can keep the needed changes > > > localized only to the cases where they are needed. > > > I'll send a few more comments in a reply to the patch. > > > > > > > Soo, if I understand you correctly, you want to have three different options? > > local_receive_unicast > > local_receive_multicast > > local_receive_broadcast > > My understanding of the feature is that you want to prevent flooding > towards the bridge. In which case, it makes sense to keep the same > granularity as for regular bridge ports and also name the options > similarly. We already have several options that are applicable to both > the bridge and bridge ports (e.g., 'mcast_router'). > > I suggest: > > $ ip link help bridge > Usage: ... bridge [ fdb_flush ] > ... > [ flood {on | off} ] > [ mcast_flood {on | off} ] > [ bcast_flood {on | off} ] > > This is consistent with "bridge_slave". And please add a selftest. See this commit for reference: https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=b2b681a412517bf477238de62b1d227361fa04fe It should allow you to test both the software data path (using veth pairs) and the hardware data path (using physical loopbacks).
On 2022-03-14 17:29, Ido Schimmel wrote: > On Wed, Mar 02, 2022 at 07:27:25AM +0100, Mattias Forsblad wrote: >> On 2022-03-01 23:36, Nikolay Aleksandrov wrote: >>> On 1 March 2022 17:43:27 CET, Ido Schimmel <idosch@idosch.org> wrote: >>>> On Tue, Mar 01, 2022 at 01:31:02PM +0100, Mattias Forsblad wrote: >>>>> diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c >>>>> index e0c13fcc50ed..5864b61157d3 100644 >>>>> --- a/net/bridge/br_input.c >>>>> +++ b/net/bridge/br_input.c >>>>> @@ -163,6 +163,9 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb >>>>> break; >>>>> } >>>>> >>>>> + if (local_rcv && !br_opt_get(br, BROPT_LOCAL_RECEIVE)) >>>>> + local_rcv = false; >>>>> + >>>> >>>> I don't think the description in the commit message is accurate: >>>> "packets received on bridge ports will not be forwarded up". From the >>>> code it seems that if packets hit a local FDB entry, then they will be >>>> "forwarded up". Instead, it seems that packets will not be flooded >>>> towards the bridge. In which case, why not maintain the same granularity >>>> we have for the rest of the ports and split this into unicast / >>>> multicast / broadcast? >>>> >>> >>> Exactly my first thought - why not implement the same control for the bridge? >>> Also try to minimize the fast-path hit, you can keep the needed changes >>> localized only to the cases where they are needed. >>> I'll send a few more comments in a reply to the patch. >>> >> >> Soo, if I understand you correctly, you want to have three different options? >> local_receive_unicast >> local_receive_multicast >> local_receive_broadcast > > My understanding of the feature is that you want to prevent flooding > towards the bridge. In which case, it makes sense to keep the same > granularity as for regular bridge ports and also name the options > similarly. We already have several options that are applicable to both > the bridge and bridge ports (e.g., 'mcast_router'). > > I suggest: > > $ ip link help bridge > Usage: ... bridge [ fdb_flush ] > ... > [ flood {on | off} ] > [ mcast_flood {on | off} ] > [ bcast_flood {on | off} ] > > This is consistent with "bridge_slave". Many thanks for your input. I'll have a go at a V2. BR Mattias Forsblad
diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h index 3aae023a9353..e6b77d18c1d2 100644 --- a/include/linux/if_bridge.h +++ b/include/linux/if_bridge.h @@ -157,6 +157,7 @@ static inline int br_vlan_get_info_rcu(const struct net_device *dev, u16 vid, struct net_device *br_fdb_find_port(const struct net_device *br_dev, const unsigned char *addr, __u16 vid); +bool br_local_receive_enabled(const struct net_device *dev); void br_fdb_clear_offload(const struct net_device *dev, u16 vid); bool br_port_flag_is_set(const struct net_device *dev, unsigned long flag); u8 br_port_get_stp_state(const struct net_device *dev); @@ -170,6 +171,11 @@ br_fdb_find_port(const struct net_device *br_dev, return NULL; } +static inline bool br_local_receive_enabled(const struct net_device *dev) +{ + return true; +} + static inline void br_fdb_clear_offload(const struct net_device *dev, u16 vid) { } diff --git a/include/net/switchdev.h b/include/net/switchdev.h index 3e424d40fae3..aec5c1f9b5c7 100644 --- a/include/net/switchdev.h +++ b/include/net/switchdev.h @@ -25,6 +25,7 @@ enum switchdev_attr_id { SWITCHDEV_ATTR_ID_BRIDGE_AGEING_TIME, SWITCHDEV_ATTR_ID_BRIDGE_VLAN_FILTERING, SWITCHDEV_ATTR_ID_BRIDGE_VLAN_PROTOCOL, + SWITCHDEV_ATTR_ID_BRIDGE_LOCAL_RECEIVE, SWITCHDEV_ATTR_ID_BRIDGE_MC_DISABLED, SWITCHDEV_ATTR_ID_BRIDGE_MROUTER, SWITCHDEV_ATTR_ID_MRP_PORT_ROLE, @@ -50,6 +51,7 @@ struct switchdev_attr { u16 vlan_protocol; /* BRIDGE_VLAN_PROTOCOL */ bool mc_disabled; /* MC_DISABLED */ u8 mrp_port_role; /* MRP_PORT_ROLE */ + bool local_receive; /* BRIDGE_LOCAL_RECEIVE */ } u; }; diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h index 2711c3522010..fc889b5ccd69 100644 --- a/include/uapi/linux/if_bridge.h +++ b/include/uapi/linux/if_bridge.h @@ -72,6 +72,7 @@ struct __bridge_info { __u32 tcn_timer_value; __u32 topology_change_timer_value; __u32 gc_timer_value; + __u8 local_receive; }; struct __port_info { diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h index e315e53125f4..bb7c25e1c89c 100644 --- a/include/uapi/linux/if_link.h +++ b/include/uapi/linux/if_link.h @@ -482,6 +482,7 @@ enum { IFLA_BR_VLAN_STATS_PER_PORT, IFLA_BR_MULTI_BOOLOPT, IFLA_BR_MCAST_QUERIER_STATE, + IFLA_BR_LOCAL_RECEIVE, __IFLA_BR_MAX, }; diff --git a/net/bridge/br.c b/net/bridge/br.c index b1dea3febeea..ff7eb4f269ec 100644 --- a/net/bridge/br.c +++ b/net/bridge/br.c @@ -325,6 +325,24 @@ void br_boolopt_multi_get(const struct net_bridge *br, bm->optmask = GENMASK((BR_BOOLOPT_MAX - 1), 0); } +int br_local_receive_change(struct net_bridge *p, + bool local_receive) +{ + struct switchdev_attr attr = { + .orig_dev = p->dev, + .id = SWITCHDEV_ATTR_ID_BRIDGE_LOCAL_RECEIVE, + .flags = SWITCHDEV_F_DEFER, + .u.local_receive = local_receive, + }; + int ret; + + ret = switchdev_port_attr_set(p->dev, &attr, NULL); + if (!ret) + br_opt_toggle(p, BROPT_LOCAL_RECEIVE, local_receive); + + return ret; +} + /* private bridge options, controlled by the kernel */ void br_opt_toggle(struct net_bridge *br, enum net_bridge_opts opt, bool on) { diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c index 8d6bab244c4a..7cd9c5880d18 100644 --- a/net/bridge/br_device.c +++ b/net/bridge/br_device.c @@ -524,6 +524,7 @@ void br_dev_setup(struct net_device *dev) br->bridge_hello_time = br->hello_time = 2 * HZ; br->bridge_forward_delay = br->forward_delay = 15 * HZ; br->bridge_ageing_time = br->ageing_time = BR_DEFAULT_AGEING_TIME; + br_opt_toggle(br, BROPT_LOCAL_RECEIVE, true); dev->max_mtu = ETH_MAX_MTU; br_netfilter_rtable_init(br); diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c index e0c13fcc50ed..5864b61157d3 100644 --- a/net/bridge/br_input.c +++ b/net/bridge/br_input.c @@ -163,6 +163,9 @@ int br_handle_frame_finish(struct net *net, struct sock *sk, struct sk_buff *skb break; } + if (local_rcv && !br_opt_get(br, BROPT_LOCAL_RECEIVE)) + local_rcv = false; + if (dst) { unsigned long now = jiffies; diff --git a/net/bridge/br_ioctl.c b/net/bridge/br_ioctl.c index f213ed108361..abe538129290 100644 --- a/net/bridge/br_ioctl.c +++ b/net/bridge/br_ioctl.c @@ -177,6 +177,7 @@ int br_dev_siocdevprivate(struct net_device *dev, struct ifreq *rq, b.topology_change = br->topology_change; b.topology_change_detected = br->topology_change_detected; b.root_port = br->root_port; + b.local_receive = br_opt_get(br, BROPT_LOCAL_RECEIVE) ? 1 : 0; b.stp_enabled = (br->stp_enabled != BR_NO_STP); b.ageing_time = jiffies_to_clock_t(br->ageing_time); diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c index 7d4432ca9a20..5e7f99950195 100644 --- a/net/bridge/br_netlink.c +++ b/net/bridge/br_netlink.c @@ -1163,6 +1163,7 @@ static const struct nla_policy br_policy[IFLA_BR_MAX + 1] = { [IFLA_BR_MCAST_IGMP_VERSION] = { .type = NLA_U8 }, [IFLA_BR_MCAST_MLD_VERSION] = { .type = NLA_U8 }, [IFLA_BR_VLAN_STATS_PER_PORT] = { .type = NLA_U8 }, + [IFLA_BR_LOCAL_RECEIVE] = { .type = NLA_U8 }, [IFLA_BR_MULTI_BOOLOPT] = NLA_POLICY_EXACT_LEN(sizeof(struct br_boolopt_multi)), }; @@ -1434,6 +1435,14 @@ static int br_changelink(struct net_device *brdev, struct nlattr *tb[], return err; } + if (data[IFLA_BR_LOCAL_RECEIVE]) { + u8 val = nla_get_u8(data[IFLA_BR_LOCAL_RECEIVE]); + + err = br_local_receive_change(br, !!val); + if (err) + return err; + } + return 0; } @@ -1514,6 +1523,7 @@ static size_t br_get_size(const struct net_device *brdev) nla_total_size(sizeof(u8)) + /* IFLA_BR_NF_CALL_ARPTABLES */ #endif nla_total_size(sizeof(struct br_boolopt_multi)) + /* IFLA_BR_MULTI_BOOLOPT */ + nla_total_size(sizeof(u8)) + /* IFLA_BR_LOCAL_RECEIVE */ 0; } @@ -1527,6 +1537,7 @@ static int br_fill_info(struct sk_buff *skb, const struct net_device *brdev) u32 stp_enabled = br->stp_enabled; u16 priority = (br->bridge_id.prio[0] << 8) | br->bridge_id.prio[1]; u8 vlan_enabled = br_vlan_enabled(br->dev); + u8 local_receive = br_opt_get(br, BROPT_LOCAL_RECEIVE) ? 1 : 0; struct br_boolopt_multi bm; u64 clockval; @@ -1563,7 +1574,8 @@ static int br_fill_info(struct sk_buff *skb, const struct net_device *brdev) nla_put_u8(skb, IFLA_BR_TOPOLOGY_CHANGE_DETECTED, br->topology_change_detected) || nla_put(skb, IFLA_BR_GROUP_ADDR, ETH_ALEN, br->group_addr) || - nla_put(skb, IFLA_BR_MULTI_BOOLOPT, sizeof(bm), &bm)) + nla_put(skb, IFLA_BR_MULTI_BOOLOPT, sizeof(bm), &bm) || + nla_put_u8(skb, IFLA_BR_LOCAL_RECEIVE, local_receive)) return -EMSGSIZE; #ifdef CONFIG_BRIDGE_VLAN_FILTERING diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h index 48bc61ebc211..01fa5426094b 100644 --- a/net/bridge/br_private.h +++ b/net/bridge/br_private.h @@ -445,6 +445,7 @@ enum net_bridge_opts { BROPT_NO_LL_LEARN, BROPT_VLAN_BRIDGE_BINDING, BROPT_MCAST_VLAN_SNOOPING_ENABLED, + BROPT_LOCAL_RECEIVE, }; struct net_bridge { @@ -720,6 +721,7 @@ int br_boolopt_multi_toggle(struct net_bridge *br, void br_boolopt_multi_get(const struct net_bridge *br, struct br_boolopt_multi *bm); void br_opt_toggle(struct net_bridge *br, enum net_bridge_opts opt, bool on); +int br_local_receive_change(struct net_bridge *p, bool local_receive); /* br_device.c */ void br_dev_setup(struct net_device *dev); diff --git a/net/bridge/br_sysfs_br.c b/net/bridge/br_sysfs_br.c index 3f7ca88c2aa3..9af0c2ba929c 100644 --- a/net/bridge/br_sysfs_br.c +++ b/net/bridge/br_sysfs_br.c @@ -84,6 +84,28 @@ static ssize_t forward_delay_store(struct device *d, } static DEVICE_ATTR_RW(forward_delay); +static ssize_t local_receive_show(struct device *d, + struct device_attribute *attr, char *buf) +{ + struct net_bridge *br = to_bridge(d); + + return sprintf(buf, "%u\n", br_opt_get(br, BROPT_LOCAL_RECEIVE)); +} + +static int set_local_receive(struct net_bridge *br, unsigned long val, + struct netlink_ext_ack *extack) +{ + return br_local_receive_change(br, !!val); +} + +static ssize_t local_receive_store(struct device *d, + struct device_attribute *attr, + const char *buf, size_t len) +{ + return store_bridge_parm(d, buf, len, set_local_receive); +} +static DEVICE_ATTR_RW(local_receive); + static ssize_t hello_time_show(struct device *d, struct device_attribute *attr, char *buf) { @@ -950,6 +972,7 @@ static struct attribute *bridge_attrs[] = { &dev_attr_group_addr.attr, &dev_attr_flush.attr, &dev_attr_no_linklocal_learn.attr, + &dev_attr_local_receive.attr, #ifdef CONFIG_BRIDGE_IGMP_SNOOPING &dev_attr_multicast_router.attr, &dev_attr_multicast_snooping.attr, diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c index 7557e90b60e1..57dd14d5e360 100644 --- a/net/bridge/br_vlan.c +++ b/net/bridge/br_vlan.c @@ -905,6 +905,14 @@ bool br_vlan_enabled(const struct net_device *dev) } EXPORT_SYMBOL_GPL(br_vlan_enabled); +bool br_local_receive_enabled(const struct net_device *dev) +{ + struct net_bridge *br = netdev_priv(dev); + + return br_opt_get(br, BROPT_LOCAL_RECEIVE); +} +EXPORT_SYMBOL_GPL(br_local_receive_enabled); + int br_vlan_get_proto(const struct net_device *dev, u16 *p_proto) { struct net_bridge *br = netdev_priv(dev);
This patch implements the bridge flag local_receive. When this flag is cleared packets received on bridge ports will not be forwarded up. This makes is possible to only forward traffic between the port members of the bridge. Signed-off-by: Mattias Forsblad <mattias.forsblad+netdev@gmail.com> --- include/linux/if_bridge.h | 6 ++++++ include/net/switchdev.h | 2 ++ include/uapi/linux/if_bridge.h | 1 + include/uapi/linux/if_link.h | 1 + net/bridge/br.c | 18 ++++++++++++++++++ net/bridge/br_device.c | 1 + net/bridge/br_input.c | 3 +++ net/bridge/br_ioctl.c | 1 + net/bridge/br_netlink.c | 14 +++++++++++++- net/bridge/br_private.h | 2 ++ net/bridge/br_sysfs_br.c | 23 +++++++++++++++++++++++ net/bridge/br_vlan.c | 8 ++++++++ 12 files changed, 79 insertions(+), 1 deletion(-)