netdev - Re: Fwd: [PATCH 1/1] team: remove unnecessary lock

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>] [day] [month] [year] [list]

Message-ID: <20160527145757.GA2151@nanopsycho>
Date:	Fri, 27 May 2016 16:57:57 +0200
From:	Jiri Pirko <jiri@...nulli.us>
To:	zhuyj <zyjzyj2000@...il.com>
Cc:	netdev <netdev@...r.kernel.org>
Subject: Re: Fwd: [PATCH 1/1] team: remove unnecessary lock

Fri, May 27, 2016 at 04:25:29AM CEST, zyjzyj2000@...il.com wrote:
>Hi, Jiri
>
>I delved into the source code of team. And I found that the function
>team_add_slave actually works in the protection of rtnl lock. So in my
>humble opinion, it is not necessary to use the mutex lock to protect
>team_port_add again.
>
>And I made tests in ubuntu 16.04 server, without the mutex lock, the
>team_add_slave can also work well.
>The steps:
>
>1.
>nmcli con add type team con-name team0 ifname team0 config
>'{"runner":{"name":"activebackup"}}'
>2.
>nmcli con add type team-slave con-name team0-port1 ifname eno16777736
>master team0
>nmcli con add type team-slave con-name team0-port2 ifname eno33554960
>master team0
>
>The above test can work well without the mutex lock. So I made the
>following patch, please comment on this patch.

Well, team lock could be replaced by rtnl and removed. Removing just
this tiny usage solves nothing. I decided to use team-specific lock to
do not overload rtnl at the beginning.


>
>Thanks a lot.
>Zhu Yanjun
>
>From: Zhu Yanjun<zyjzyj2000@...il.com>
>
>The function team_add_slave works in the context of the rtnl lock.
>It is not necessary to use the mutex lock since the rtnl lock is
>enough.
>
>Signed-off-by: Zhu Yanjun <zyjzyj2000@...il.com>
>---
> drivers/net/team/team.c | 2 --
> 1 file changed, 2 deletions(-)
>
>diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
>index a0f64cb..ad84069 100644
>--- a/drivers/net/team/team.c
>+++ b/drivers/net/team/team.c
>@@ -1936,9 +1936,7 @@ static int team_add_slave(struct net_device *dev,
>struct net_device *port_dev)
>        struct team *team = netdev_priv(dev);
>        int err;
>
>-       mutex_lock(&team->lock);
>        err = team_port_add(team, port_dev);
>-       mutex_unlock(&team->lock);
>        return err;
> }
>
>--
>1.9.1