From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 48FE6A04C0; Fri, 25 Sep 2020 22:30:56 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 9D43A1E9C3; Fri, 25 Sep 2020 22:30:54 +0200 (CEST) Received: from NAM10-BN7-obe.outbound.protection.outlook.com (mail-bn7nam10on2130.outbound.protection.outlook.com [40.107.92.130]) by dpdk.org (Postfix) with ESMTP id E5BA51E9C1 for ; Fri, 25 Sep 2020 22:30:52 +0200 (CEST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=fJ0BzT4/4kaBWi2uLk6WlS1RtHsnNGZ6kKoNdNnuC6GosqDPfs3uf0CYYzerJ2VOH87JeoIoIDaGlEmY4GFtSDdsJHvMYB6acVUC/2Lq1s2kB29hcTyUYP5kJMOv4/MJ3pJADQPbKAKjKZ7ZDHx0k9gyMSJtjJdfWA3HoJx2E2AavGQG0AXfpnc3lEQqUiuAxsFqiPuoH+6ikb5bTBb75TrfdMMrHp8bHSoQE1cpn4hlkhdgEqThoxCZrOJySYfIIoz4yR5eh2vo199zJ4KSzWiFmLiabDbY4iXLR1Bb7CjmFeFxWyOSdmbVaOOsF7Jg7gEAxWPznV4q/LaDNvPCQg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=/962eZYU6bUQusdz/fpB0GaeZ8hnBtyFZY2a0QjnVaI=; b=jmDwEURb4fbgRrDeStVUCksh/RNDW97GRMbepPg4vgh6WiGbpEf19Aetcdppbj4S9iETxeDOpY+hW/qG/foZHTPSeIVNXl8xRsIyXC7exIHOLpF4Si6nlXwj9Vqbm0eQN3mvoAOXNn+4Sg5hYrhhdviYGsSOi/jxjRgEEp4dIl5MCV9XMeiB7YlrjJsxOJuuNkfETVM7lIRZN2QdDFnb/7L9XN1B/PQLkThIwCBvdcG1K6aRQ29oqWOH4L2FtbkIyPS0KjS8mBkowlLJRXtnYzDfrfRVm2vBIa88W/zTQKkW4Zu2tceTPn9tDZUIbb6TU0cIfPDoKbl7Tr1pxDnlaA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=microsoft.com; dmarc=pass action=none header.from=microsoft.com; dkim=pass header.d=microsoft.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=selector2; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=/962eZYU6bUQusdz/fpB0GaeZ8hnBtyFZY2a0QjnVaI=; b=hhcyTpsafnh8mDfSZQ5c15f795xHsDFsU0zFpPqUPX+4885sGl/7JBybnVDC/JkLJ5G2nC+485LCo4AP+eR51ZAtCHShcMmzuGHb6pXch5/m0TGopz1SACNU90rCHb3fx3ap1rI4R+4Q7CBJJYW9l+uJT1FJEH35NtB2MJERwr8= Received: from BN8PR21MB1155.namprd21.prod.outlook.com (2603:10b6:408:73::10) by BN8PR21MB1297.namprd21.prod.outlook.com (2603:10b6:408:a3::15) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3433.7; Fri, 25 Sep 2020 20:30:50 +0000 Received: from BN8PR21MB1155.namprd21.prod.outlook.com ([fe80::ccf4:7bc9:ac7a:1a19]) by BN8PR21MB1155.namprd21.prod.outlook.com ([fe80::ccf4:7bc9:ac7a:1a19%3]) with mapi id 15.20.3433.020; Fri, 25 Sep 2020 20:30:50 +0000 From: Long Li To: Matan Azrad , Stephen Hemminger CC: "matan@mellanox.com" , "grive@u246.net" , "dev@dpdk.org" , Raslan Darawsheh Thread-Topic: [dpdk-dev] [PATCH] net/vdev_netvsc: handle removal of associated pci device Thread-Index: AQHWhHw1BCnIHP4dAEiYvVGoM+pMX6lc03CAgAxMP3CAADMEgIAQlYcT Date: Fri, 25 Sep 2020 20:30:50 +0000 Message-ID: References: <20200819175333.19601-1-stephen@networkplumber.org> <20200906113309.4a320400@hermes.lan> , In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: msip_labels: MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_Enabled=True; MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_SiteId=72f988bf-86f1-41af-91ab-2d7cd011db47; MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_SetDate=2020-09-25T20:30:49.750Z; MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_Name=General; MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_ContentBits=0; MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_Method=Standard; authentication-results: nvidia.com; dkim=none (message not signed) header.d=none;nvidia.com; dmarc=none action=none header.from=microsoft.com; x-originating-ip: [67.168.111.68] x-ms-publictraffictype: Email x-ms-office365-filtering-ht: Tenant x-ms-office365-filtering-correlation-id: 7ed68c10-7e10-4524-8e7e-08d86191e483 x-ms-traffictypediagnostic: BN8PR21MB1297: x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:9508; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: 1AjR+0xGt305HVGVfU9/nRB3IPEqbth7GmxrHHuYip9o56k1bUSAxOt8qpOUNBi1nZNiBf9ZzlAK2jQmwkWeX8Ro4lqJ1OoEmSZtYM0+XL/uS0NezxdbjnJVYbdeKkFtrufuVWOgyt0VhiKPh1Bn4SFRGh94qrQGnTmSQG/2RY7gXSyN5qehvWcFjkVLdKgFx6pRtFBUV/3Ss9k4af2YfCezIdhxzZjmPxCXuvPQG/qg+gynnodf3F5LWy5LiQ3TbK39PA6Joixy1sp6R8hjCmBdsJ5zmfG5+85bCpbQng/fOD43keA54GB9TfzYjTpN8v4T28YUInsroRhuwSkh+g== x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:BN8PR21MB1155.namprd21.prod.outlook.com; PTR:; CAT:NONE; SFS:(4636009)(39860400002)(376002)(366004)(396003)(136003)(346002)(186003)(8676002)(9686003)(8936002)(10290500003)(478600001)(86362001)(53546011)(55016002)(4326008)(54906003)(33656002)(82960400001)(110136005)(71200400001)(82950400001)(5660300002)(2906002)(7696005)(52536014)(66476007)(76116006)(19627405001)(316002)(8990500004)(64756008)(6506007)(66446008)(83380400001)(66946007)(66556008)(26005); DIR:OUT; SFP:1102; x-ms-exchange-antispam-messagedata: 3BEHzgCbN9ZC+c6cvZO8lsvN15CJ8wlh+QjQaXo8PWmd7V6XCzw94X8eMqTU5wOCWHMOCu77zG19ThTNpDGSA+68shMoWL1L0BklK/+6tC9pTxY8GbM+cIWpcnmYuX+jPqYW64i3ZPL3dRxZhtlu+lB2jrgW+ubpN/DirV7HKHC7o2SaOQA8p8EkykDFqUmWsT3/xnCnbq54iNhnZE+VBtn0xPHS4aN4kmFJHOT1cNAJPoS5RVsSF2mbq6IG4xTGwNwZjOrcE0MimHAWgw9s6aYqibCMZKbFnTXgJ/K1VsoyHX6uhCBSfmulgRmudfvOTZ04FG8GOHOYiujxZcSdtNFj+ljef8sTTuX8NO5ZYJHFjLFSPk0eNarwv5MVIb26yRzcqFtrSbQRxMWWfDDY+8DkDT7l5DPhI6Asn7nScmGyAZYFKJXIVHLm14dXCqwnmljkYn7hc/dhf+y2paVJAaJsQAmZ66KZ3/tLSoJbXrXR12vmiymuqlNOdcujireZBrcXQD/tSkFgxYfwII+jvB1X6h5IscO1AVxA3wc7zH/ie2PzwcadS9SnVMPZw7vG+90z67yKmTNC6IhvOh7gu+1uszNrw1h3FGFemxinAomrdkRE+DfsN1QmcmovzdnmxUNA54hv2qVtyJIbFGmWAQ== x-ms-exchange-transport-forked: True MIME-Version: 1.0 X-OriginatorOrg: microsoft.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: BN8PR21MB1155.namprd21.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 7ed68c10-7e10-4524-8e7e-08d86191e483 X-MS-Exchange-CrossTenant-originalarrivaltime: 25 Sep 2020 20:30:50.4064 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 72f988bf-86f1-41af-91ab-2d7cd011db47 X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: ll5d/IRnZBJ6COZrd5Gut/dAhaDw2R5qF87ogrpsfr2pD2tY7RU1579GuxzO49N5e/l9Qx6UwMNRcWFV1diYHA== X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN8PR21MB1297 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [PATCH] net/vdev_netvsc: handle removal of associated pci device X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" HI Matan, While troubleshooting a failure in DPDK on device removal when VF device br= iefly disappears and comes back, I notice the failsafe driver is trying rep= eatedly to start a sub device (after this sub device has been successfully = configured, but later hot removed from the kernel). This is due to repeated= alarms calling fs_dev_start(). I trace into this commit: ae80146 net/failsafe: fix removed device handling The implementation of fs_err() is interesting: +fs_err(struct sub_device *sdev, int err) +{ + /* A device removal shouldn't be reported as an error. */ + if (sdev->remove =3D=3D 1 || err =3D=3D -EIO) + return rte_errno =3D 0; + return err; +} If I change this function to: @@ -497,7 +497,7 @@ int failsafe_eth_new_event_callback(uint16_t port_id fs_err(struct sub_device *sdev, int err) { /* A device removal shouldn't be reported as an error. */ - if (sdev->remove =3D=3D 1 || err =3D=3D -EIO) + if (sdev->remove =3D=3D 1 && err =3D=3D -EIO) return rte_errno =3D 0; return err; } The hung is going away. I don't know the reason why we use a || in the if()= . If a call to rte_eth_dev_start() returning EIO (as the case in fs_dev_sta= rt), the best choice would be bail out and fail this sub device. Can you please take a look? Thanks, Long ________________________________ From: Matan Azrad Sent: Tuesday, September 15, 2020 12:00 AM To: Long Li ; Stephen Hemminger Cc: matan@mellanox.com ; grive@u246.net ; dev@dpdk.org ; Raslan Darawsheh Subject: RE: [dpdk-dev] [PATCH] net/vdev_netvsc: handle removal of associat= ed pci device Hi Li From: Long Li > >Subject: Re: [dpdk-dev] [PATCH] net/vdev_netvsc: handle removal of > >associated pci device > > > >Hi Stephen > > > >From: Stephen Hemminger: > >> On Sun, 6 Sep 2020 12:38:18 +0000 > >> Matan Azrad wrote: > >> > >> > Hi Stephen > >> > > >> > From: Stephen Hemminger: > >> > > The vdev_netvsc was not detecting when the associated PCI device > >> > > (SRIOV) was removed. Because of that it would keep feeding the > >> > > same > >> > > (removed) device to failsafe PMD which would then unsuccessfully > >> > > try and probe for it. > >> > > > >> > > Change to use a mark/sweep method to detect that PCI device was > >> > > removed, and also only tell failsafe about new PCI devices. > >> > > Vdev_netvsc does not have to keep stuffing the pipe with the same > >> > > already existing PCI device. > >> > > >> > As I know, the vdev_netvsc driver doesn't call to failsafe if the > >> > PCI device is > >> not detected by the readlink command(considered as removed)... > >> > Am I missing something? > >> > >> The original code is broken because ctx_yield is not cleared, it > >> keeps sending the same value. > > > >Looking on the code again, It looks like ctx->yield has no effect on > >the next pipe write, It is just used for log. > > > >After the PCI interface matching to the netvsc interface, the pipe > >write is triggered only if the readlink commands success to see the > >plugged-in PCI > >device: > >readlink /sys/class/net/[iface]/device/subsystem shows "pci" > >readlink /sys/class/net/[iface]/device shows the pci device ID. > > > >So, the assumption is when the above readlink failed on the interface > >the device is removed(plugged-out) and the fd write will not happen. > > > >The code will continue to retry probe again and again until success > >only for plugged-in pci device matched the netvsc device. > > Hi Matan, > > The original code keeps writing to pipe even it's the same PCI device. Yes, the vdev_netvsc writes any plugged-in device to the associated netvsc = device fd. > The > new code writes to pipe for a new device, only once. See the following co= de: > > + /* Skip if this is same device already sent to failsafe */ > + if (strcmp(addr, ctx->yield) =3D=3D 0) > + return 0; > I understand you want to optimize the pipe writing to be written only after= plugged-in hot event. The current solution suffers from race: the PCI device may be plugged-out a= nd plugged-in in short time shorter than the driver alarm delay, then the P= CI device plugged-in detection will lost. My suggestion: Add validation to the plugged-in device probing state and that it is owned = by failsafe(using ownership API) - don't write the pipe if so. Matan > This patch also saves lots of CPU since it no longer writes to pipe all t= he time. > You are correct about the code will continue to probe on a new PCI device= . > But someone has to do it to handle hot-add. > > Thanks, > Long > > > > > >> It looks like device removal and add was never tested. > > > >This is basic test we have to test plug-in plug-out and it passed every > >day in the last years. > > > >Maybe something new and special in your setup? > > > >> If you test removal you will see that vdev_netvsc: > >> 1. Sends same PCI device repeatedly to failsafe (every alarm call) > >> This is harmless, but useless. > >> 2. When device is removed, keeps doing #1