| 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: <530E0B45.1090707@redhat.com>
Date: Wed, 26 Feb 2014 10:41:57 -0500
From: Vlad Yasevich <vyasevic@...hat.com>
To: "Michael S. Tsirkin" <mst@...hat.com>
CC: netdev@...r.kernel.org, bridge@...ts.linux-foundation.org,
shemminger@...tta.com, jhs@...atatu.com, john.r.fastabend@...el.com
Subject: Re: [PATCH 2/7] bridge: Keep track of ports capable of flooding.
On 02/26/2014 10:41 AM, Michael S. Tsirkin wrote:
> On Wed, Feb 26, 2014 at 10:18:20AM -0500, Vlad Yasevich wrote:
>> Keep track of bridge ports that have unicast flooding turned on.
>> This will later be used by the algorithm to automatically manage
>> address programming and promisc mode.
>>
>> Signed-off-by: Vlad Yasevich <vyasevic@...hat.com>
>> ---
>> net/bridge/br_if.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++--
>> net/bridge/br_netlink.c | 3 +++
>> net/bridge/br_private.h | 4 ++++
>> net/bridge/br_sysfs_if.c | 6 ++++-
>> 4 files changed, 70 insertions(+), 3 deletions(-)
>>
>> diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
>> index 54d207d..f072b34 100644
>> --- a/net/bridge/br_if.c
>> +++ b/net/bridge/br_if.c
>> @@ -27,6 +27,9 @@
>>
>> #include "br_private.h"
>>
>> +static void br_add_flood_port(struct net_bridge_port *p, struct net_bridge *br);
>> +static void br_del_flood_port(struct net_bridge_port *p, struct net_bridge *br);
>> +
>
> same nitpick about forward declarations of static
> functions.
> maybe they are ok for bridge, but that's my $.02
>
Ok. Will take a look. Just means that I might need to re-order the
code which may scatter it a bit through the file.
>
>> /*
>> * Determine initial path cost based on speed.
>> * using recommendations from 802.1d standard
>> @@ -141,11 +144,14 @@ static void del_nbp(struct net_bridge_port *p)
>>
>> br_ifinfo_notify(RTM_DELLINK, p);
>>
>> + list_del_rcu(&p->list);
>> +
>> + if (p->flags & BR_FLOOD)
>> + br_del_flood_port(p, br);
>> +
>> nbp_vlan_flush(p);
>> br_fdb_delete_by_port(br, p, 1);
>>
>> - list_del_rcu(&p->list);
>> -
>> dev->priv_flags &= ~IFF_BRIDGE_PORT;
>>
>> netdev_rx_handler_unregister(dev);
>
> ok so it doesn't matter in which order we do
> list_del_rcu and nbp_vlan_flush then?
Nope. vlan_flush knows which port its working and doesn't touch
the port list.
What does touch the port list if br_fdb_delete_by_port, but that only
touches it in fdb_delete_local(), and it's ok there since we are trying
to point an existing fdb out a different port.
Thanks
-vlad
>
>> @@ -383,6 +389,9 @@ int br_add_if(struct net_bridge *br, struct net_device *dev)
>> dev_disable_lro(dev);
>>
>> list_add_rcu(&p->list, &br->port_list);
>> +
>> + if (p->flags & BR_FLOOD)
>> + br_add_flood_port(p, br);
>>
>> netdev_update_features(br->dev);
>>
>> @@ -455,3 +464,50 @@ int br_del_if(struct net_bridge *br, struct net_device *dev)
>>
>> return 0;
>> }
>> +
>> +static void br_add_flood_port(struct net_bridge_port *p, struct net_bridge *br)
>> +{
>> + /* Increment the number of flooding ports, and if we
>> + * only have 1 flooding port cache if for future use.
>> + */
>> + br->n_flood_ports++;
>> + if (br->n_flood_ports == 1)
>> + br->c_flood_port = p;
>> +}
>> +
>> +static void br_del_flood_port(struct net_bridge_port *p, struct net_bridge *br)
>> +{
>> + struct net_bridge_port *port;
>> +
>> + /* Decrement the number of flood port.
>> + * If we are deleting the current flood port, clear
>> + * the cached port. If we are down to 1 flood port,
>> + * set it if it is not set.
>> + */
>> + br->n_flood_ports--;
>> + if (p == br->c_flood_port)
>> + br->c_flood_port = NULL;
>> +
>> + if (br->n_flood_ports == 1) {
>> + list_for_each_entry(port, &p->br->port_list, list) {
>> + if (port->flags & BR_FLOOD) {
>> + br->c_flood_port = port;
>> + break;
>> + }
>> + }
>> + }
>> +}
>> +
>> +void br_port_flags_change(struct net_bridge_port *p, unsigned long mask)
>> +{
>> + struct net_bridge *br = p->br;
>> +
>> + /* We are only interested FLOOD flag */
>> + if (!(mask & BR_FLOOD))
>> + return;
>> +
>> + if (p->flags & BR_FLOOD)
>> + br_add_flood_port(p, br);
>> + else
>> + br_del_flood_port(p, br);
>> +}
>> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
>> index e74b6d53..01382b9 100644
>> --- a/net/bridge/br_netlink.c
>> +++ b/net/bridge/br_netlink.c
>> @@ -328,6 +328,7 @@ static void br_set_port_flag(struct net_bridge_port *p, struct nlattr *tb[],
>> static int br_setport(struct net_bridge_port *p, struct nlattr *tb[])
>> {
>> int err;
>> + unsigned long old_flags = p->flags;
>>
>> br_set_port_flag(p, tb, IFLA_BRPORT_MODE, BR_HAIRPIN_MODE);
>> br_set_port_flag(p, tb, IFLA_BRPORT_GUARD, BR_BPDU_GUARD);
>> @@ -353,6 +354,8 @@ static int br_setport(struct net_bridge_port *p, struct nlattr *tb[])
>> if (err)
>> return err;
>> }
>> +
>> + br_port_flags_change(p, old_flags ^ p->flags);
>> return 0;
>> }
>>
>> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
>> index 3ba11bc..26a3987 100644
>> --- a/net/bridge/br_private.h
>> +++ b/net/bridge/br_private.h
>> @@ -290,6 +290,8 @@ struct net_bridge
>> struct timer_list topology_change_timer;
>> struct timer_list gc_timer;
>> struct kobject *ifobj;
>> + struct net_bridge_port *c_flood_port;
>> + u32 n_flood_ports;
>
> any hints on locking for these fields?
> are these all under rtnl?
>
>> #ifdef CONFIG_BRIDGE_VLAN_FILTERING
>> u8 vlan_enabled;
>> struct net_port_vlans __rcu *vlan_info;
>> @@ -415,6 +417,8 @@ int br_del_if(struct net_bridge *br, struct net_device *dev);
>> int br_min_mtu(const struct net_bridge *br);
>> 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);
>>
>> /* br_input.c */
>> int br_handle_frame_finish(struct sk_buff *skb);
>> diff --git a/net/bridge/br_sysfs_if.c b/net/bridge/br_sysfs_if.c
>> index 7f66aa4..9ff6691 100644
>> --- a/net/bridge/br_sysfs_if.c
>> +++ b/net/bridge/br_sysfs_if.c
>> @@ -51,7 +51,10 @@ static BRPORT_ATTR(_name, S_IRUGO | S_IWUSR, \
>> static int store_flag(struct net_bridge_port *p, unsigned long v,
>> unsigned long mask)
>> {
>> - unsigned long flags = p->flags;
>> + unsigned long flags;
>> + unsigned long old_flags;
>> +
>> + old_flags = flags = p->flags;
>>
>> if (v)
>> flags |= mask;
>> @@ -60,6 +63,7 @@ static int store_flag(struct net_bridge_port *p, unsigned long v,
>>
>> if (flags != p->flags) {
>> p->flags = flags;
>> + br_port_flags_change(p, old_flags ^ flags);
>> br_ifinfo_notify(RTM_NEWLINK, p);
>> }
>> return 0;
>> --
>> 1.8.5.3
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists