From patchwork Thu Apr 26 19:43:30 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: "Tiainen, Antti" X-Patchwork-Id: 10366737 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 3D19D6032C for ; Thu, 26 Apr 2018 19:43:50 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 2C42826E90 for ; Thu, 26 Apr 2018 19:43:50 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 20EFE28AB5; Thu, 26 Apr 2018 19:43:50 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.9 required=2.0 tests=BAYES_00, MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from bastion.fedoraproject.org (bastion01.fedoraproject.org [209.132.181.2]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 3F26226E90 for ; Thu, 26 Apr 2018 19:43:48 +0000 (UTC) Received: from mailman01.phx2.fedoraproject.org (mailman01.phx2.fedoraproject.org [10.5.126.36]) by bastion01.phx2.fedoraproject.org (Postfix) with ESMTP id 39E3960C20AE; Thu, 26 Apr 2018 19:43:48 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 bastion01.phx2.fedoraproject.org 39E3960C20AE Received: from mailman01.phx2.fedoraproject.org (localhost [IPv6:::1]) by mailman01.phx2.fedoraproject.org (Postfix) with ESMTP id 2E45B256F201A; Thu, 26 Apr 2018 19:43:48 +0000 (UTC) Received: by mailman01.phx2.fedoraproject.org (Postfix, from userid 991) id BCDE9256F1FEB; Thu, 26 Apr 2018 19:43:44 +0000 (UTC) Received: from smtp-mm-tummy01.fedoraproject.org (smtp-mm-tummy01.vpn.fedoraproject.org [192.168.1.82]) by mailman01.phx2.fedoraproject.org (Postfix) with ESMTP id D021F256F1ED3 for ; Thu, 26 Apr 2018 19:43:43 +0000 (UTC) Received: from esg-sm.forcepoint.com (esg-sm3.forcepoint.com [204.15.67.173]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "*.forcepoint.com", Issuer "Go Daddy Secure Certificate Authority - G2" (verified OK)) by smtp-mm-tummy01.fedoraproject.org (Postfix) with ESMTPS id 971C0608125A for ; Thu, 26 Apr 2018 19:43:43 +0000 (UTC) Received: from SSDEXCH2B.websense.com (unknown [10.64.80.131]) by Forcepoint Email with ESMTPS id 30AFF5CA97848C1E72E9; Thu, 26 Apr 2018 12:43:42 -0700 (PDT) Received: from NAM02-CY1-obe.outbound.protection.outlook.com (207.46.163.51) by webmail.websense.com (10.64.80.131) with Microsoft SMTP Server (TLS) id 14.3.339.0; Thu, 26 Apr 2018 12:43:46 -0700 Received: from CY1PR14MB0203.namprd14.prod.outlook.com (10.163.93.156) by CY1PR14MB0251.namprd14.prod.outlook.com (10.163.93.26) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id 15.20.715.18; Thu, 26 Apr 2018 19:43:30 +0000 Received: from CY1PR14MB0203.namprd14.prod.outlook.com ([fe80::9b2:7a86:23c0:3fd8]) by CY1PR14MB0203.namprd14.prod.outlook.com ([fe80::9b2:7a86:23c0:3fd8%13]) with mapi id 15.20.0696.020; Thu, 26 Apr 2018 19:43:30 +0000 From: "Tiainen, Antti" To: Xin Long , "libteam@lists.fedorahosted.org" , Jiri Pirko Subject: Re: [PATCHv2] libteam: do not destroy the ifinfo of current unregistered slave dev Thread-Topic: EXTERNAL: [PATCHv2] libteam: do not destroy the ifinfo of current unregistered slave dev Thread-Index: AQHT2iulxnYwaa41Qkm3rgfuS7kwBKQOO//5gAU2OuM= Date: Thu, 26 Apr 2018 19:43:30 +0000 Message-ID: References: <064e21f1dc9a34e637411c9ecbcd833057994609.1524395891.git.lucien.xin@gmail.com>, In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [80.220.84.178] x-ms-publictraffictype: Email x-microsoft-exchange-diagnostics: 1; CY1PR14MB0251; 7:EGKtj8jEzPGLuQLY8RT83QEfbpmpCEv07LO8QqEkatZ/5au2u0ZIQAxME6FRk5Lm9oUa1kvSaXV6NFz6lcbsrq6db/4ZyfYae0c1xGfzsIp2YPqgotEQl2IGITrFF7g2W5B+9yiIjoKTWdUfz7yUeYXqhn3ZuK6NEMuVtQ6jHAzTA6pVE7kI3Tz8S1p3QJIGBQv8CVN+t9PLTEY45y0SjgnTJJMwsR9xZ6fmVO3FIGioZq0hKCsRKK/lvSTS/D7U x-ms-exchange-antispam-srfa-diagnostics: SOS; x-microsoft-antispam: UriScan:; BCL:0; PCL:0; RULEID:(7020095)(4652020)(5600026)(4534165)(4627221)(201703031133081)(201702281549075)(2017052603328)(7153060)(7193020); SRVR:CY1PR14MB0251; x-ms-traffictypediagnostic: CY1PR14MB0251: x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(85827821059158); x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(8211001083)(6040522)(2401047)(5005006)(8121501046)(10201501046)(93006095)(93001095)(3002001)(3231232)(944501410)(52105095)(6041310)(20161123564045)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(20161123560045)(20161123562045)(20161123558120)(6072148)(201708071742011); SRVR:CY1PR14MB0251; BCL:0; PCL:0; RULEID:; SRVR:CY1PR14MB0251; x-forefront-prvs: 0654257CF5 x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(396003)(366004)(376002)(346002)(39380400002)(39860400002)(54534003)(199004)(189003)(5250100002)(3660700001)(33656002)(14454004)(2900100001)(76176011)(3846002)(2501003)(7696005)(106356001)(25786009)(97736004)(6436002)(66066001)(446003)(6506007)(575784001)(3280700002)(316002)(110136005)(102836004)(86362001)(53546011)(478600001)(68736007)(8936002)(11346002)(476003)(53936002)(229853002)(6116002)(74316002)(55016002)(5660300001)(26005)(105586002)(2906002)(99286004)(6246003)(39060400002)(7736002)(486006)(305945005)(81166006)(186003)(81156014)(8676002)(9686003); DIR:OUT; SFP:1101; SCL:1; SRVR:CY1PR14MB0251; H:CY1PR14MB0203.namprd14.prod.outlook.com; FPR:; SPF:None; LANG:en; PTR:InfoNoRecords; A:1; MX:1; received-spf: None (protection.outlook.com: forcepoint.com does not designate permitted sender hosts) x-microsoft-antispam-message-info: WDkfAfHHJ87+p2r+s0OlIEjOX3qHAuJ6+o2ijZz/Wgz4xf/aFp7eaCjeCSzG1fgxLDEdbqiGQqolyPZlucwOKcp8/B6xhlXzFbFeNdzpliKALTW6pTFEZNOTfTiVqRxIeaPjDJIZ71WV4WrRR8UygMb0VMLpKGzGuehPq3iDurCiTezFoJPjgYP/QsM6S8Ta spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM MIME-Version: 1.0 X-MS-Office365-Filtering-Correlation-Id: c7f7b8f9-09eb-4a37-248b-08d5abadfd07 X-MS-Exchange-CrossTenant-Network-Message-Id: c7f7b8f9-09eb-4a37-248b-08d5abadfd07 X-MS-Exchange-CrossTenant-originalarrivaltime: 26 Apr 2018 19:43:30.4338 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 9ca08696-fd76-4782-a644-3c97f4dd0a59 X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY1PR14MB0251 X-OriginatorOrg: forcepoint.com Message-ID-Hash: 5PR45J7AKJ3GX7HSJMBVYGTQH537PE4G X-Message-ID-Hash: 5PR45J7AKJ3GX7HSJMBVYGTQH537PE4G X-MailFrom: atiainen@forcepoint.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; header-match-config-1; header-match-config-2; header-match-config-3; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; suspicious-header X-Mailman-Version: 3.1.1 Precedence: list List-Id: Archived-At: List-Archive: List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-Virus-Scanned: ClamAV using ClamSMTP The reason why it didn't crash was because in the first environment I tried was because it got port removal event before dellink (they come to different sockets). In other environment, it did crash. Xin Long's patch does the job, but the port hangs there in ifinfo list and debug output quite a long time. I'll send alternative patch for suggestion how to fix this issue. It would not crash and it removes the deleted port device from ifinfo list, but it will still need a NULL check for debug print for the extremely rare situation that dellink event is lost due to full socket receive buffer. -antti From: Tiainen, Antti Sent: Monday, April 23, 2018 3:01 PM To: Xin Long; libteam@lists.fedorahosted.org; Jiri Pirko Subject: Re: EXTERNAL: [PATCHv2] libteam: do not destroy the ifinfo of current unregistered slave dev   Strange, it doesn't crash for me if I try your example. But if I try with your patch, the deleted interface just hangs there in ifinfo list forever, even when change handlers are called. If this is only a debug print issue, it cannot do simply NULL check in there, and print something (or nothing) for the interface that's completely gone?  -antti From: Xin Long Sent: Sunday, April 22, 2018 2:18:11 PM To: libteam@lists.fedorahosted.org; Jiri Pirko Cc: Tiainen, Antti Subject: EXTERNAL: [PATCHv2] libteam: do not destroy the ifinfo of current unregistered slave dev   When a dev is being unregistered and the corresp team port is being removed, the ifinfo should be delayed to destroy when doing dellink next time, because later on the dbg log still needs it to print the ifname that is saved in this ifinfo:   ...     -1450: veth1: down 0Mbit HD <--     ... The similar process is also used on the team_port's destruction. However, after Commit 046fb6ba0aec ("libteam: resynchronize ifinfo after lost RTNLGRP_LINK notifications"), it would destroy all those ifinfo with REMOVED flag, including the one of current unregistered slave dev. It would cause a crash later when printing the dbg log. An easy way to reproduce:   # ip link add veth1 type veth peer name veth1_peer   # sleep 5 && ip link del veth1 &   # teamd -g -c '{"device":"team0","runner":{"name":"activebackup"},     "ports":{"veth1":{}}}' This patch is to check !ifinfo->port before calling ifinfo_destroy in ifinfo_destroy_removed and clear_changed in ifinfo_destroy_removed. After this fix, the ifinfo of current unregistered slave dev will not be destroyed this time until next time when processing IFINFO_CHANGE flag for another ifinfo's change in check_call_change_handlers. v1->v2:   - improve the changelog, including some typos Jiri noticed. Fixes: 046fb6ba0aec ("libteam: resynchronize ifinfo after lost RTNLGRP_LINK notifications") Signed-off-by: Xin Long ---  libteam/ifinfo.c | 5 +++--  1 file changed, 3 insertions(+), 2 deletions(-) -- 2.1.0 diff --git a/libteam/ifinfo.c b/libteam/ifinfo.c index 5c32a9c..d47c2bf 100644 --- a/libteam/ifinfo.c +++ b/libteam/ifinfo.c @@ -211,7 +211,8 @@ void ifinfo_clear_changed(struct team_handle *th)          struct team_ifinfo *ifinfo;            list_for_each_node_entry(ifinfo, &th->ifinfo_list, list) -               clear_changed(ifinfo); +               if (!ifinfo->port) +                       clear_changed(ifinfo);  }    static struct team_ifinfo *ifinfo_find_create(struct team_handle *th, @@ -245,7 +246,7 @@ void ifinfo_destroy_removed(struct team_handle *th)          struct team_ifinfo *ifinfo, *tmp;            list_for_each_node_entry_safe(ifinfo, tmp, &th->ifinfo_list, list) { -               if (is_changed(ifinfo, CHANGED_REMOVED)) +               if (is_changed(ifinfo, CHANGED_REMOVED) && !ifinfo->port)                          ifinfo_destroy(ifinfo);          }  }