From patchwork Fri Dec 13 09:29:29 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ding Tianhong X-Patchwork-Id: 300958 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 72A5A2C009C for ; Fri, 13 Dec 2013 20:30:53 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752283Ab3LMJat (ORCPT ); Fri, 13 Dec 2013 04:30:49 -0500 Received: from szxga03-in.huawei.com ([119.145.14.66]:58468 "EHLO szxga03-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752231Ab3LMJar (ORCPT ); Fri, 13 Dec 2013 04:30:47 -0500 Received: from 172.24.2.119 (EHLO szxeml214-edg.china.huawei.com) ([172.24.2.119]) by szxrg03-dlp.huawei.com (MOS 4.4.3-GA FastPath queued) with ESMTP id AHR02466; Fri, 13 Dec 2013 17:29:51 +0800 (CST) Received: from SZXEML458-HUB.china.huawei.com (10.82.67.201) by szxeml214-edg.china.huawei.com (172.24.2.29) with Microsoft SMTP Server (TLS) id 14.3.158.1; Fri, 13 Dec 2013 17:29:36 +0800 Received: from [127.0.0.1] (10.135.72.199) by SZXEML458-HUB.china.huawei.com (10.82.67.201) with Microsoft SMTP Server id 14.3.158.1; Fri, 13 Dec 2013 17:29:31 +0800 Message-ID: <52AAD379.5010307@huawei.com> Date: Fri, 13 Dec 2013 17:29:29 +0800 From: Ding Tianhong User-Agent: Mozilla/5.0 (Windows NT 6.1; rv:24.0) Gecko/20100101 Thunderbird/24.0.1 MIME-Version: 1.0 To: Jay Vosburgh , Andy Gospodarek , "David S. Miller" , Veaceslav Falico , Netdev Subject: [PATCH net 3/3] bonding: protect port for bond_3ad_handle_link_change() X-Originating-IP: [10.135.72.199] X-CFilter-Loop: Reflected Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org The bond_3ad_handle_link_change is called with RTNL only, and the function will modify the port's information with no further locking, it will not mutex against bond state machine and incoming LACPDU which do not hold RTNL, So I add __get_state_machine_lock to protect the port. But it is not a critical bug, it exist since day one, and till now it has never been hit and reported, because changes to speed is very rare, and will not occur critical problem. The comments in the function is very old, cleanup it and add a new pr_debug to debug the port message. Signed-off-by: Ding Tianhong --- drivers/net/bonding/bond_3ad.c | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c index e851a67..4ced594 100644 --- a/drivers/net/bonding/bond_3ad.c +++ b/drivers/net/bonding/bond_3ad.c @@ -2268,15 +2268,21 @@ void bond_3ad_handle_link_change(struct slave *slave, char link) port = &(SLAVE_AD_INFO(slave).port); - // if slave is null, the whole port is not initialized + /* if slave is null, the whole port is not initialized */ if (!port->slave) { pr_warning("Warning: %s: link status changed for uninitialized port on %s\n", slave->bond->dev->name, slave->dev->name); return; } - // on link down we are zeroing duplex and speed since some of the adaptors(ce1000.lan) report full duplex/speed instead of N/A(duplex) / 0(speed) - // on link up we are forcing recheck on the duplex and speed since some of he adaptors(ce1000.lan) report + __get_state_machine_lock(port); + /* on link down we are zeroing duplex and speed since + * some of the adaptors(ce1000.lan) report full duplex/speed + * instead of N/A(duplex) / 0(speed). + * + * on link up we are forcing recheck on the duplex and speed since + * some of he adaptors(ce1000.lan) report. + */ if (link == BOND_LINK_UP) { port->is_enabled = true; port->actor_admin_port_key &= ~AD_DUPLEX_KEY_BITS; @@ -2292,10 +2298,15 @@ void bond_3ad_handle_link_change(struct slave *slave, char link) port->actor_oper_port_key = (port->actor_admin_port_key &= ~AD_SPEED_KEY_BITS); } - //BOND_PRINT_DBG(("Port %d changed link status to %s", port->actor_port_number, ((link == BOND_LINK_UP)?"UP":"DOWN"))); - // there is no need to reselect a new aggregator, just signal the - // state machines to reinitialize + pr_debug("Port %d changed link status to %s", + port->actor_port_number, + (link == BOND_LINK_UP) ? "UP" : "DOWN"); + /* there is no need to reselect a new aggregator, just signal the + * state machines to reinitialize + */ port->sm_vars |= AD_PORT_BEGIN; + + __release_state_machine_lock(port); } /*