From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-10.2 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,MAILING_LIST_MULTI, NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3C5D1C63777 for ; Thu, 3 Dec 2020 13:44:49 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id D7417206EC for ; Thu, 3 Dec 2020 13:44:48 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730780AbgLCNoc (ORCPT ); Thu, 3 Dec 2020 08:44:32 -0500 Received: from szxga05-in.huawei.com ([45.249.212.191]:9100 "EHLO szxga05-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727682AbgLCNoc (ORCPT ); Thu, 3 Dec 2020 08:44:32 -0500 Received: from DGGEMS407-HUB.china.huawei.com (unknown [172.30.72.58]) by szxga05-in.huawei.com (SkyGuard) with ESMTP id 4Cmxqd0LtBzLxmb; Thu, 3 Dec 2020 21:43:09 +0800 (CST) Received: from [10.174.179.81] (10.174.179.81) by DGGEMS407-HUB.china.huawei.com (10.3.19.207) with Microsoft SMTP Server id 14.3.487.0; Thu, 3 Dec 2020 21:43:38 +0800 Subject: Re: [PATCH net] net: bridge: Fix a warning when del bridge sysfs To: Nikolay Aleksandrov , Jakub Kicinski CC: , , , , References: <20201201140114.67455-1-wanghai38@huawei.com> <20201202170359.19330bda@kicinski-fedora-pc1c0hjn.DHCP.thefacebook.com> <3d3c0206-c8c4-8a19-c821-2a0cbb941c6b@nvidia.com> From: "wanghai (M)" Message-ID: <5147798b-c1cf-ce5f-524c-4874eb854bc0@huawei.com> Date: Thu, 3 Dec 2020 21:43:38 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 MIME-Version: 1.0 In-Reply-To: <3d3c0206-c8c4-8a19-c821-2a0cbb941c6b@nvidia.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.174.179.81] X-CFilter-Loop: Reflected Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 在 2020/12/3 18:34, Nikolay Aleksandrov 写道: > On 03/12/2020 03:03, Jakub Kicinski wrote: >> On Tue, 1 Dec 2020 22:01:14 +0800 Wang Hai wrote: >>> If adding bridge sysfs fails, br->ifobj will be NULL, there is no >>> need to delete its non-existent sysfs when deleting the bridge device, >>> otherwise, it will cause a warning. So, when br->ifobj == NULL, >>> directly return can fix this bug. >>> >>> br_sysfs_addbr: can't create group bridge4/bridge >>> ------------[ cut here ]------------ >>> sysfs group 'bridge' not found for kobject 'bridge4' >>> WARNING: CPU: 2 PID: 9004 at fs/sysfs/group.c:279 sysfs_remove_group fs/sysfs/group.c:279 [inline] >>> WARNING: CPU: 2 PID: 9004 at fs/sysfs/group.c:279 sysfs_remove_group+0x153/0x1b0 fs/sysfs/group.c:270 >>> Modules linked in: iptable_nat >>> ... >>> Call Trace: >>> br_dev_delete+0x112/0x190 net/bridge/br_if.c:384 >>> br_dev_newlink net/bridge/br_netlink.c:1381 [inline] >>> br_dev_newlink+0xdb/0x100 net/bridge/br_netlink.c:1362 >>> __rtnl_newlink+0xe11/0x13f0 net/core/rtnetlink.c:3441 >>> rtnl_newlink+0x64/0xa0 net/core/rtnetlink.c:3500 >>> rtnetlink_rcv_msg+0x385/0x980 net/core/rtnetlink.c:5562 >>> netlink_rcv_skb+0x134/0x3d0 net/netlink/af_netlink.c:2494 >>> netlink_unicast_kernel net/netlink/af_netlink.c:1304 [inline] >>> netlink_unicast+0x4a0/0x6a0 net/netlink/af_netlink.c:1330 >>> netlink_sendmsg+0x793/0xc80 net/netlink/af_netlink.c:1919 >>> sock_sendmsg_nosec net/socket.c:651 [inline] >>> sock_sendmsg+0x139/0x170 net/socket.c:671 >>> ____sys_sendmsg+0x658/0x7d0 net/socket.c:2353 >>> ___sys_sendmsg+0xf8/0x170 net/socket.c:2407 >>> __sys_sendmsg+0xd3/0x190 net/socket.c:2440 >>> do_syscall_64+0x33/0x40 arch/x86/entry/common.c:46 >>> entry_SYSCALL_64_after_hwframe+0x44/0xa9 >>> >>> Reported-by: Hulk Robot >>> Signed-off-by: Wang Hai >> Nik, is this the way you want to handle this? >> >> Should the notifier not fail if sysfs files cannot be created? >> Currently br_sysfs_addbr() returns an int but the only caller >> ignores it. >> > Hi, > The fix is wrong because this is not the only user of ifobj. The bridge > port sysfs code also uses it and br_sysfs_addif() will create the new > symlink in sysfs_root_kn due to NULL kobj passed which basically means > only one port will be enslaved, the others will fail in creating their > sysfs entries and thus fail to be added as ports. > > I'd prefer to just fail from the notifier based on the return value. > The only catch would be to test it with br_vlan_bridge_event() which > is called on bridge master device events, it should be fine as > br_vlan_find() deals with NULL vlan groups but at least a comment > above it in br.c's notifier would be good so if anyone decides to add > any NETDEVICE_UNREGISTER handling would be warned about it. Thanks for your advice, I will perfect my patch > Also please add proper fixes tag, the bug seems to be since: > bb900b27a2f4 ("bridge: allow creating bridge devices with netlink") > > It actually changed the behaviour, before that the return value of br_sysfs_addbr() > was checked and the device got unregistered on failure. > > Thanks, > Nik > > > . >