| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Wed, 2 Mar 2022 07:33:56 +0100
From: Mattias Forsblad <mattias.forsblad@...il.com>
To: Nikolay Aleksandrov <razor@...ckwall.org>, netdev@...r.kernel.org
Cc: "David S . Miller" <davem@...emloft.net>,
Jakub Kicinski <kuba@...nel.org>, Andrew Lunn <andrew@...n.ch>,
Florian Fainelli <f.fainelli@...il.com>,
Vivien Didelot <vivien.didelot@...il.com>,
Roopa Prabhu <roopa@...dia.com>,
Mattias Forsblad <mattias.forsblad+netdev@...il.com>
Subject: Re: [PATCH 1/3] net: bridge: Implement bridge flag local_receive
On 2022-03-01 23:43, Nikolay Aleksandrov wrote:
> On 1 March 2022 13:31:02 CET, Mattias Forsblad <mattias.forsblad@...il.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@...il.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);
>
Powered by blists - more mailing lists