netdev - Re: [PATCH net-next v3 05/15] net: bridge: mcast: factor out port group del

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

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sun, 6 Sep 2020 13:56:04 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Nikolay Aleksandrov <nikolay@...ulusnetworks.com>
Cc:     netdev@...r.kernel.org, roopa@...dia.com,
        bridge@...ts.linux-foundation.org, davem@...emloft.net
Subject: Re: [PATCH net-next v3 05/15] net: bridge: mcast: factor out port
 group del

On Sat,  5 Sep 2020 11:24:00 +0300 Nikolay Aleksandrov wrote:
> @@ -843,24 +843,11 @@ static int __br_mdb_del(struct net_bridge *br, struct br_mdb_entry *entry)
>  		if (!p->port || p->port->dev->ifindex != entry->ifindex)
>  			continue;
>  
> -		if (!hlist_empty(&p->src_list)) {
> -			err = -EINVAL;
> -			goto unlock;
> -		}
> -
>  		if (p->port->state == BR_STATE_DISABLED)
>  			goto unlock;
>  
> -		__mdb_entry_fill_flags(entry, p->flags);

Just from staring at the code it's unclear why the list_empty() check
and this __mdb_entry_fill_flags() are removed as well.

> -		rcu_assign_pointer(*pp, p->next);
> -		hlist_del_init(&p->mglist);
> -		del_timer(&p->timer);
> -		kfree_rcu(p, rcu);
> +		br_multicast_del_pg(mp, p, pp);
>  		err = 0;
> -
> -		if (!mp->ports && !mp->host_joined &&
> -		    netif_running(br->dev))
> -			mod_timer(&mp->timer, jiffies);
>  		break;


> +void br_multicast_del_pg(struct net_bridge_mdb_entry *mp,
> +			 struct net_bridge_port_group *pg,
> +			 struct net_bridge_port_group __rcu **pp)
> +{
> +	struct net_bridge *br = pg->port->br;
> +	struct net_bridge_group_src *ent;
> +	struct hlist_node *tmp;
> +
> +	rcu_assign_pointer(*pp, pg->next);
> +	hlist_del_init(&pg->mglist);
> +	del_timer(&pg->timer);
> +	hlist_for_each_entry_safe(ent, tmp, &pg->src_list, node)
> +		br_multicast_del_group_src(ent);
> +	br_mdb_notify(br->dev, pg->port, &pg->addr, RTM_DELMDB, pg->flags);
> +	kfree_rcu(pg, rcu);
> +
> +	if (!mp->ports && !mp->host_joined && netif_running(br->dev))
> +		mod_timer(&mp->timer, jiffies);
> +}

> @@ -1641,16 +1647,7 @@ br_multicast_leave_group(struct net_bridge *br,
>  			if (p->flags & MDB_PG_FLAGS_PERMANENT)
>  				break;
>  
> -			rcu_assign_pointer(*pp, p->next);
> -			hlist_del_init(&p->mglist);
> -			del_timer(&p->timer);
> -			kfree_rcu(p, rcu);
> -			br_mdb_notify(br->dev, port, group, RTM_DELMDB,
> -				      p->flags | MDB_PG_FLAGS_FAST_LEAVE);

And here we'll loose MDB_PG_FLAGS_FAST_LEAVE potentially?

> -			if (!mp->ports && !mp->host_joined &&
> -			    netif_running(br->dev))
> -				mod_timer(&mp->timer, jiffies);
> +			br_multicast_del_pg(mp, p, pp);