| 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
| ||
|
Message-ID: <4f31ae8b-352e-d2ab-cd71-4b31f76e666a@cumulusnetworks.com>
Date: Thu, 16 Nov 2017 09:31:04 +0200
From: Nikolay Aleksandrov <nikolay@...ulusnetworks.com>
To: Sarah Newman <srn@...mr.com>, netdev@...r.kernel.org
Cc: roopa <roopa@...ulusnetworks.com>
Subject: Re: [PATCH] net: bridge: add max_fdb_count
On 15/11/17 21:27, Sarah Newman wrote:
> Current memory and CPU usage for managing bridge fdb entries is unbounded.
> Add a parameter max_fdb_count, controlled from sysfs, which places an upper
> limit on the number of entries. Defaults to 1024.
>
> When max_fdb_count is met or exceeded, whether traffic is sent out a
> given port should depend on its flooding behavior.
>
> This may instead be mitigated by filtering mac address entries in the
> PREROUTING chain of the ebtables nat table, but this is only practical
> when mac addresses are known in advance.
>
One alternative solution: if limit is the only requirement it could be done
in user-space (even with a shell script) looking at fdb notifications and
if you reach some limit then remove the learning flag from ports, later if
enough expire turn it back on. In fact you can make any policy and if you
catch an offending port - you can disable only its learning flag and leave the
rest.
> Signed-off-by: Sarah Newman <srn@...mr.com>
> ---
> net/bridge/br_device.c | 2 ++
> net/bridge/br_fdb.c | 25 ++++++++++++++++++++-----
> net/bridge/br_private.h | 3 +++
> net/bridge/br_sysfs_br.c | 24 ++++++++++++++++++++++++
> 4 files changed, 49 insertions(+), 5 deletions(-)
>
As others have mentioned placing a default limit would break existing users,
so this must be a config option that is off (or very large) by default.
> diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
> index af5b8c8..aa7a7f4 100644
> --- a/net/bridge/br_device.c
> +++ b/net/bridge/br_device.c
> @@ -432,6 +432,8 @@ 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->max_fdb_count = 1024;> + br->fdb_count = 0;
No need to zero this explicitly, it is already zero.
> dev->max_mtu = ETH_MAX_MTU;
>
> br_netfilter_rtable_init(br);
> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
> index 4ea5c8b..0422d48 100644
> --- a/net/bridge/br_fdb.c
> +++ b/net/bridge/br_fdb.c
> @@ -179,6 +179,7 @@ static void fdb_delete(struct net_bridge *br, struct net_bridge_fdb_entry *f)
>
> hlist_del_init_rcu(&f->hlist);
> fdb_notify(br, f, RTM_DELNEIGH);
> + br->fdb_count -= 1;> call_rcu(&f->rcu, fdb_rcu_free);
> }
>
> @@ -478,7 +479,8 @@ int br_fdb_fillbuf(struct net_bridge *br, void *buf,
> return num;
> }
>
> -static struct net_bridge_fdb_entry *fdb_create(struct hlist_head *head,
> +static struct net_bridge_fdb_entry *fdb_create(struct net_bridge *br,
> + struct hlist_head *head,
> struct net_bridge_port *source,
> const unsigned char *addr,
> __u16 vid,
> @@ -498,6 +500,7 @@ static struct net_bridge_fdb_entry *fdb_create(struct hlist_head *head,
> fdb->added_by_external_learn = 0;
> fdb->offloaded = 0;
> fdb->updated = fdb->used = jiffies;
> + br->fdb_count += 1;
> hlist_add_head_rcu(&fdb->hlist, head);
> }
> return fdb;
> @@ -524,7 +527,7 @@ static int fdb_insert(struct net_bridge *br, struct net_bridge_port *source,
> fdb_delete(br, fdb);
> }
>
> - fdb = fdb_create(head, source, addr, vid, 1, 1);
> + fdb = fdb_create(br, head, source, addr, vid, 1, 1);
> if (!fdb)
> return -ENOMEM;
>
> @@ -556,6 +559,10 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
> if (hold_time(br) == 0)
> return;
>
> + /* Place maximum on number of learned entries. */
> + if (br->max_fdb_count <= br->fdb_count)
> + return;
> +
checking for fdb_count in the beginning of br_fdb_update would cause stale fdb
entries, that is fdb moves between ports will stop and fdb updates will stop too
(i.e. everything expires)
> /* ignore packets unless we are using this port */
> if (!(source->state == BR_STATE_LEARNING ||
> source->state == BR_STATE_FORWARDING))
> @@ -591,7 +598,7 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
> } else {
> spin_lock(&br->hash_lock);
> if (likely(!fdb_find_rcu(head, addr, vid))) {
> - fdb = fdb_create(head, source, addr, vid, 0, 0);
> + fdb = fdb_create(br, head, source, addr, vid, 0, 0);
> if (fdb) {
> if (unlikely(added_by_user))
> fdb->added_by_user = 1;
> @@ -787,7 +794,7 @@ static int fdb_add_entry(struct net_bridge *br, struct net_bridge_port *source,
> if (!(flags & NLM_F_CREATE))
> return -ENOENT;
>
> - fdb = fdb_create(head, source, addr, vid, 0, 0);
> + fdb = fdb_create(br, head, source, addr, vid, 0, 0);
> if (!fdb)
> return -ENOMEM;
>
> @@ -1081,7 +1088,7 @@ int br_fdb_external_learn_add(struct net_bridge *br, struct net_bridge_port *p,
> head = &br->hash[br_mac_hash(addr, vid)];
> fdb = br_fdb_find(br, addr, vid);
> if (!fdb) {
> - fdb = fdb_create(head, p, addr, vid, 0, 0);
> + fdb = fdb_create(br, head, p, addr, vid, 0, 0);
> if (!fdb) {
> err = -ENOMEM;
> goto err_unlock;
> @@ -1147,3 +1154,11 @@ void br_fdb_offloaded_set(struct net_bridge *br, struct net_bridge_port *p,
>
> spin_unlock_bh(&br->hash_lock);
> }
> +
> +int br_set_max_fdb_count(struct net_bridge *br, unsigned long max_fdb_count)
> +{
> + spin_lock_bh(&br->lock);
> + br->max_fdb_count = max_fdb_count;
> + spin_unlock_bh(&br->lock);
No need to take the spin lock here as this value is checked without it anyway,
and all bridge dev option changes are done under rtnl lock already.
> + return 0;
> +}
> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> index 1312b8d..98aae1f 100644
> --- a/net/bridge/br_private.h
> +++ b/net/bridge/br_private.h
> @@ -343,6 +343,8 @@ struct net_bridge {
> unsigned long bridge_hello_time;
> unsigned long bridge_forward_delay;
> unsigned long bridge_ageing_time;
> + unsigned long max_fdb_count;
> + unsigned long fdb_count;>
> u8 group_addr[ETH_ALEN];
> bool group_addr_set;
> @@ -549,6 +551,7 @@ int br_fdb_external_learn_del(struct net_bridge *br, struct net_bridge_port *p,
> const unsigned char *addr, u16 vid);
> void br_fdb_offloaded_set(struct net_bridge *br, struct net_bridge_port *p,
> const unsigned char *addr, u16 vid);
> +int br_set_max_fdb_count(struct net_bridge *br, unsigned long x);
>
> /* br_forward.c */
> enum br_pkt_type {
> diff --git a/net/bridge/br_sysfs_br.c b/net/bridge/br_sysfs_br.c
> index 723f25e..18fabdf 100644
> --- a/net/bridge/br_sysfs_br.c
> +++ b/net/bridge/br_sysfs_br.c
> @@ -335,6 +335,28 @@ static ssize_t flush_store(struct device *d,
> }
> static DEVICE_ATTR_WO(flush);
>
> +static ssize_t max_fdb_count_show(struct device *d, struct device_attribute *attr,
> + char *buf)
> +{
> + struct net_bridge *br = to_bridge(d);
> + return sprintf(buf, "%lu\n", br->max_fdb_count);
> +}
> +
> +static ssize_t max_fdb_count_store(struct device *d, struct device_attribute *attr,
> + const char *buf, size_t len)
> +{
> + return store_bridge_parm(d, buf, len, br_set_max_fdb_count);
> +}
> +static DEVICE_ATTR_RW(max_fdb_count);
> +
> +static ssize_t fdb_count_show(struct device *d, struct device_attribute *attr,
> + char *buf)
> +{
> + struct net_bridge *br = to_bridge(d);
> + return sprintf(buf, "%lu\n", br->fdb_count);
> +}
> +static DEVICE_ATTR_RO(fdb_count);
> +
> #ifdef CONFIG_BRIDGE_IGMP_SNOOPING
> static ssize_t multicast_router_show(struct device *d,
> struct device_attribute *attr, char *buf)
> @@ -830,6 +852,8 @@ static ssize_t vlan_stats_enabled_store(struct device *d,
> &dev_attr_gc_timer.attr,
> &dev_attr_group_addr.attr,
> &dev_attr_flush.attr,
> + &dev_attr_max_fdb_count.attr,
> + &dev_attr_fdb_count.attr,
> #ifdef CONFIG_BRIDGE_IGMP_SNOOPING
> &dev_attr_multicast_router.attr,
> &dev_attr_multicast_snooping.attr,
>
Powered by blists - more mailing lists