netdev - Re: [PATCH net-next] bridge: allow setting hash_max + multicast

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:	Thu, 21 May 2015 20:11:32 -0700
From:	Cong Wang <cwang@...pensource.com>
To:	Linus Lüssing <linus.luessing@...3.blue>
Cc:	netdev <netdev@...r.kernel.org>,
	"bridge@...ts.linux-foundation.org" 
	<bridge@...ts.linux-foundation.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Stephen Hemminger <stephen@...workplumber.org>,
	Herbert Xu <herbert@...dor.apana.org.au>,
	"David S. Miller" <davem@...emloft.net>
Subject: Re: [PATCH net-next] bridge: allow setting hash_max +
 multicast_router if interface is down

On Thu, May 21, 2015 at 5:56 PM, Linus Lüssing <linus.luessing@...3.blue> wrote:
> diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
> index 2d69d5c..066199e 100644
> --- a/net/bridge/br_multicast.c
> +++ b/net/bridge/br_multicast.c
> @@ -1775,8 +1775,6 @@ int br_multicast_set_router(struct net_bridge *br, unsigned long val)
>         int err = -ENOENT;
>

Nit: err can be initialized to -EINVAL now.

>         spin_lock_bh(&br->multicast_lock);
> -       if (!netif_running(br->dev))
> -               goto unlock;
>
>         switch (val) {
>         case 0:
> @@ -1793,7 +1791,6 @@ int br_multicast_set_router(struct net_bridge *br, unsigned long val)
>                 break;
>         }
>
> -unlock:
>         spin_unlock_bh(&br->multicast_lock);
>
>         return err;
> @@ -1802,18 +1799,15 @@ unlock:
>  int br_multicast_set_port_router(struct net_bridge_port *p, unsigned long val)
>  {
>         struct net_bridge *br = p->br;
> -       int err = -ENOENT;
> +       int err = 0;
>
>         spin_lock(&br->multicast_lock);


Not related with your patch, but why we don't need to disable bh here?

For me it looks like we do use p->rlist in BH context, but I could easily
miss something here.

> -       if (!netif_running(br->dev) || p->state == BR_STATE_DISABLED)
> -               goto unlock;
>
>         switch (val) {
>         case 0:
>         case 1:
>         case 2:
>                 p->multicast_router = val;
> -               err = 0;
>
>                 if (val < 2 && !hlist_unhashed(&p->rlist))
>                         hlist_del_init_rcu(&p->rlist);
> @@ -1834,7 +1828,6 @@ int br_multicast_set_port_router(struct net_bridge_port *p, unsigned long val)
>                 break;
>         }
>
> -unlock:
>         spin_unlock(&br->multicast_lock);
>
>         return err;


Thanks!
--
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