From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 567F8A0508; Thu, 14 Apr 2022 13:08:33 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id EA15540687; Thu, 14 Apr 2022 13:08:32 +0200 (CEST) Received: from EUR02-HE1-obe.outbound.protection.outlook.com (mail-eopbgr10121.outbound.protection.outlook.com [40.107.1.121]) by mails.dpdk.org (Postfix) with ESMTP id 4780440041; Thu, 14 Apr 2022 13:08:31 +0200 (CEST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=CrHdhDow4wuDCvsVvhLpjDaXRZva2l5Tdda+ZcF58JBXoIuMo13lLrc6fmP/yqlUS/LfJDjwJBsXf22id0OQ9fHvoz3EitSj5VJRdvbZ30yW96FPT2Cwx+z+vF7FlQnrl1heIkfKgyIZVJZJtEpS2UnTfvET1QxymRuQW4xr/9ofSOd3irt7351aT3j+MTAKZv4OabHn6C/MkN1Zj7uNFCYlVKFrig0EEebQBHiY7BHLEhJ2dKaQFEfNzvsEZB4oN3OmHh15zWMAFr3Sp5ulp+M8X9GxjYZoHGSS1EiooleFrM3g+robXENLzAWcnbftiGEnqZGPA8vHrIPRuiEfzw== 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-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=1OC+V3DFkLkLZLc/GovdKQXsyYliR0ZoYURiCV89pJY=; b=my//wyYK1zl7pxZvscd5oP9cIzJi4/yeN0dcqt2NNAseleoRlYvepB7ZN3Do9QfWVsHLELTYYQB1Voib1rPnHF6qqXyPnHBozrPJemfGbCMN/OxnjQPF506hJ2bmMxdsfqSA+WqEMgcfOZfKDJgIe9P0e0RaRwQlubyfA/Fyu/hNPlPLrnxWsOz4QtRHLi/f4r77IR9hs+bDq77RZDsn2h5EK4ophrd/cCTqX1Bn0XwVQEbcJtdPPTbIWzMOsEGXdvdL8TLp8SSGnaIVFRrwIMaiaWm7FMl44CuF7rF8POvfOir4vPkCFKXl5YyOpCXgo9tm834GW33AtFA2fcroHg== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=silicom-usa.com; dmarc=pass action=none header.from=silicom-usa.com; dkim=pass header.d=silicom-usa.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=SILICOMLTD.onmicrosoft.com; s=selector2-SILICOMLTD-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=1OC+V3DFkLkLZLc/GovdKQXsyYliR0ZoYURiCV89pJY=; b=EAIxHNBYO0xFtjfUjqdT5tvSANg9EQdKU6jTDZ3a/f9jI5WDvY2YhiA3zElyPzdRWNO8VnoO4qpT7kT7jNTfl8chKIf4odEpaePE7HxXr9fZ77ho8F/sBilo7OiB9nZEfcOwkj/1TdTMzfEjj3G1k+k/0vflXkB6JkjOTL9ZDQw= Received: from VI1PR0402MB3517.eurprd04.prod.outlook.com (2603:10a6:803:b::16) by HE1PR0402MB3466.eurprd04.prod.outlook.com (2603:10a6:7:7f::16) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5164.18; Thu, 14 Apr 2022 11:08:28 +0000 Received: from VI1PR0402MB3517.eurprd04.prod.outlook.com ([fe80::5dd5:47e1:1cef:cc4e]) by VI1PR0402MB3517.eurprd04.prod.outlook.com ([fe80::5dd5:47e1:1cef:cc4e%6]) with mapi id 15.20.5144.029; Thu, 14 Apr 2022 11:08:28 +0000 From: Jeff Daly To: Jeff Daly , "Wang, Haiyue" , "dev@dpdk.org" CC: "stable@dpdk.org" , "Yang, Qiming" Subject: RE: [PATCH v6 2/2] net/ixgbe: Fix SFP detection and linking on hotplug Thread-Topic: [PATCH v6 2/2] net/ixgbe: Fix SFP detection and linking on hotplug Thread-Index: AQHYTpSxgnie9TYA7EmxFtoINkPBYqztJEkAgAIXZ6CAAAXZAA== Date: Thu, 14 Apr 2022 11:08:28 +0000 Message-ID: References: <20220228152937.21247-1-jeffd@silicom-usa.com> <20220412174220.31195-1-jeffd@silicom-usa.com> <20220412174220.31195-3-jeffd@silicom-usa.com> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=silicom-usa.com; x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 8085d79e-73cd-40ca-e118-08da1e071a54 x-ms-traffictypediagnostic: HE1PR0402MB3466:EE_ x-microsoft-antispam-prvs: x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: XCvlVNWM/heL4B2SDuq11KLRjeVeiav0TzjP2U83ZWn3INbWfQE9Vs4rsfcFkkwe5f4MiIT0Tqb65WptNbQZMVFSOWZEq13CWwwzx2wQT3nGCCAWYuggg3l6cH+yWUpOkFbZj2AfWJ6NhJtu0eEdSQDMmgLjn3D0LCf0hehDmfwLBVLYW0C6It6HqwADL32WpLnco2lcG7OzhUEDgI13WRevVcKIeYOt0+XXmZaFrZHsiylVYgWL2Gn3x2LeAeMUSM5Rvnwlb4duB8otjG1/89R84c3zQ8SPTm1LG/+5FBxdPGjKZWsv7vhiqgZ5lQMaJujVVMRSPcMdoc/tx4bI3hDp5dSSy5EzBEM/PEknbkxyV3LQitiiyiwjHNpDREcxJLfAww4E8ejG4mf7gzqn/acvYushSQ3/EPJKxIX0ms/GEu6hrtLqSOQIlFBg50osvAG7JB/pYQ0ySE2vyTxffWxdpNG7oIVbw8cdbRPhDVEfbrDYWtOA8w6EHrullwoI07+YEj+0rNe7win5ELElb4VegdxyMNw+jb4xTzYoCLhxH7Hr5Mt2dM3r8cv2vE3dbP9P0q3Olm6hHiA5e/CA0JPsluzUenJM2CjS3xsiJNb4RmnxDjTnARn1lKHIDyyCwd3mn/MxpS3yKfGrLx4kl8KW0aWoMZmm24rzPQ9HgFPQrAeQTpntQJqJdYSuJv8yEm4SUrOq7hjsOSxoXy1jhw== x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:VI1PR0402MB3517.eurprd04.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230001)(4636009)(366004)(66946007)(66446008)(52536014)(8676002)(33656002)(66556008)(316002)(110136005)(186003)(55016003)(6506007)(83380400001)(54906003)(66476007)(38070700005)(76116006)(86362001)(4326008)(38100700002)(9686003)(2940100002)(122000001)(7696005)(64756008)(26005)(508600001)(71200400001)(5660300002)(45080400002)(8936002)(2906002)(53546011); DIR:OUT; SFP:1102; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?2xxRyCkY8f8MjbzT7hq1UEEmQCvTXjxaPDJzyJRfzsShluNHoWoH/C26gc9B?= =?us-ascii?Q?x9Gflb26tLfktZr9Gs6tVW69LohuNG7x8Pq1clagpt2QTOa1j2mwVnYBy0lM?= =?us-ascii?Q?sExcIKRvefv46jGoYrm7RnVMTl126qGxJlrrnDWzG0q+/fF/4E4XxMwEJiCW?= =?us-ascii?Q?g9LjfkbCPV4cg2YHn6o4BNrFvx+X8w0cJTX1zi4XONADPYRE8LkLZv/9BAeg?= =?us-ascii?Q?Op24y7vBryJqqE0NxtX+NCKxvq3XyC3CO1E/VJvVr8MYa5tHIxc2l8pStdf4?= =?us-ascii?Q?6gVk0shcBt+RtvRera8Nx3w5uPJYmBijgSQnYA1+MUPx138hRYAz8Urhzbgb?= =?us-ascii?Q?rkYmKZJ7xY12CnAwut0sU2yoZlyipW+TlGx4cdawgfriUp6C+4p42CNuB3Wr?= =?us-ascii?Q?NdNcAp5//MrkLvo1dKmlsiGmMCCEwntLW593q7pYdKM639NB119kiKwh/7lR?= =?us-ascii?Q?NX0ha/ilBdql2j2RaEMnqesJCr4glBCXABDTETSulEMXwcHGOc9VioeeVa1C?= =?us-ascii?Q?MH3XP7EtfOdboDyF5Rgw+tCAm/wI3mDfCt0jJuDB0ZI5eypclVx0phQV/Ptn?= =?us-ascii?Q?+pXbpb6gl374KgYS1gJwnOHMHkxrra1+wtDgYOzjY/ehvjwJntT1lwP1aWay?= =?us-ascii?Q?XjLMb9c45Fk0m7xNN/lr1YUo/8ZmpGNAwd44ScKEh2dM7JdP0DUu15cgf5wB?= =?us-ascii?Q?iK0ZrTETRJHJI3RRdwe7TMQRjqBMe4IpQr6VMlY2SOJfEv6oLPeDr4qpjPAn?= =?us-ascii?Q?qsiPcyf4o71lcaIeQMq4mAyDQQbyJqyVPYSreUKotq0iC4kK1ETF/ik9gTgl?= =?us-ascii?Q?qo9vOc7tdjE6Fg+DPhi46LOFt5ogbSopdknTyoffOCNbexj8sFQCtccPVDX8?= =?us-ascii?Q?8aE8WeTXkj8Rkwa/Y+NiW571WLAje+vDbAzhqmd7W1pAuoLLnZkF73U24I6H?= =?us-ascii?Q?OTsxay9GgxMkEFEKdyUsCfnvU9+qc6OImK8qNVGXa85Dbuwmt0GWDR0zw4ow?= =?us-ascii?Q?GUrMSKkenRsmMns0TtLAA2EU9JXQgas1863OYIYijO3GY2RI7+XMLkPK5t8K?= =?us-ascii?Q?aMPSpWDzrxNJWQX6F8D6358CTvj24m64FmpN/sFgWlmm0as2UdxjkvCO/Bz6?= =?us-ascii?Q?klnNtfFznv8fUeg2HCadwtCdr6s32QCKPPC86NWakn0R93jjjVEK3E9nq6FU?= =?us-ascii?Q?BuClh0z/ROaT4XO/mZFPhGDhhPmNBzKeTJ2RitukJSJ+EwWptAFlQ17+6FKG?= =?us-ascii?Q?ZRUPb1CNnJuftKon/EaefYE4ZR5+u9BoUeTdotubIxJkN/T1NGY+hBONnp3k?= =?us-ascii?Q?EaZ2UdVuMrFHvNke2LIZTGP4DTLy9cY3cmXUYcabVVYy1jy6fzAIarakS8tI?= =?us-ascii?Q?GyxKhYTFV/DIZ9qvucCutAJkR/ev/x5u7o+11cMrBwB022qZkHp29bTeeR7f?= =?us-ascii?Q?fh7pyVjNGPldIR4GPwLJG5mW5LWUIcQKV7bJrRtQXCBDDmbJ5VXFngN+hcth?= =?us-ascii?Q?7FJx9mHqwuTcZ5U2pr1YbJWpkkC6tRFLrQHw1KC/jkfpOzhb5ObY2sTPkdxD?= =?us-ascii?Q?TPVNRsLbQPAlNvij2s1Bkma3sTlgpOF6gUVIxlq8WYA8RoZPGozUjlKt5OHR?= =?us-ascii?Q?jGO12a7rIUjj7Jq1O7LIfseHmMTu9ptNV9XkdsTHDQqyq0FkkAwOhHcVdnjH?= =?us-ascii?Q?YzJ/0KLl/nxpaaiB3yFQIRXI7jBQt9CLa99t2kw1ZaQemIS26rCa/2AQ5ZSR?= =?us-ascii?Q?A+3VHdK2pg=3D=3D?= Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: silicom-usa.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: VI1PR0402MB3517.eurprd04.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 8085d79e-73cd-40ca-e118-08da1e071a54 X-MS-Exchange-CrossTenant-originalarrivaltime: 14 Apr 2022 11:08:28.0627 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: c9e326d8-ce47-4930-8612-cc99d3c87ad1 X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: lCRUqAlsx851JNMPdjirYbIAisuclYcjn9mDbTBQQ6OR4MvW588DfoMbHDPOHMA8w0fGY5uxowLbw6zZ498nig== X-MS-Exchange-Transport-CrossTenantHeadersStamped: HE1PR0402MB3466 X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org > -----Original Message----- > From: Jeff Daly > Sent: Thursday, April 14, 2022 6:50 AM > To: Wang, Haiyue ; dev@dpdk.org > Cc: stable@dpdk.org; Yang, Qiming > Subject: RE: [PATCH v6 2/2] net/ixgbe: Fix SFP detection and linking on > hotplug >=20 > Caution: This is an external email. Please take care when clicking links = or > opening attachments. >=20 >=20 > > -----Original Message----- > > From: Wang, Haiyue > > Sent: Tuesday, April 12, 2022 10:47 PM > > To: Jeff Daly ; dev@dpdk.org > > Cc: stable@dpdk.org; Stephen Douthit ; Yang, > > Qiming > > Subject: RE: [PATCH v6 2/2] net/ixgbe: Fix SFP detection and linking > > on hotplug > > > > Caution: This is an external email. Please take care when clicking > > links or opening attachments. > > > > > > > -----Original Message----- > > > From: Jeff Daly > > > Sent: Wednesday, April 13, 2022 01:42 > > > To: dev@dpdk.org > > > Cc: stable@dpdk.org; Stephen Douthit ; > > Wang, > > > Haiyue > > > Subject: [PATCH v6 2/2] net/ixgbe: Fix SFP detection and linking on > > > hotplug > > > > > > Currently the ixgbe driver does not ID any SFP except for the first > > > one plugged in. This can lead to no-link, or incorrect speed conditio= ns. > > > > > > For example: > > > > > > * If link is initially established with a 1G SFP, and later a 1G/10G > > > multispeed part is later installed, then the MAC link setup > > > functions are never called to change from 1000BASE-X to 10GBASE-R > > > mode, and the link stays running at the slower rate. > > > > > > * If link is initially established with a 1G SFP, and later a 10G > > > only module is later installed, no link is established, since we are > > > still trasnsmitting in 1000BASE-X mode to a 10GBASE-R only partner. > > > > > > Refactor the SFP ID/setup, and link setup code, to more closely > > > match the flow of the mainline kernel driver which does not have > > > these issues. In that driver a service task runs periodically to > > > handle these operations based on bit flags that have been set > > > (usually via interrupt or userspace request), and then get cleared > > > once the requested subtask has been completed. > > > > > > Fixes: af75078fece ("first public release") > > > Cc: stable@dpdk.org > > > > > > > So BIG change for new platform, DON'T CC to stable! > > > > > Signed-off-by: Stephen Douthit > > > Signed-off-by: Jeff Daly > > > --- > > > drivers/net/ixgbe/ixgbe_ethdev.c | 533 > > > +++++++++++++++++++++++-------- drivers/net/ixgbe/ixgbe_ethdev.h | > > > 14 +- > > > 2 files changed, 410 insertions(+), 137 deletions(-) > > > > > > diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c > > > b/drivers/net/ixgbe/ixgbe_ethdev.c > > > index f31bbb7895..9e720eee47 100644 > > > --- a/drivers/net/ixgbe/ixgbe_ethdev.c > > > +++ b/drivers/net/ixgbe/ixgbe_ethdev.c > > > @@ -236,9 +236,6 @@ static int ixgbe_dev_interrupt_get_status(struct > > > rte_eth_dev *dev); static int ixgbe_dev_interrupt_action(struct > > > rte_eth_dev *dev); static void ixgbe_dev_interrupt_handler(void > > > *param); static void ixgbe_dev_interrupt_delayed_handler(void > > > *param); -static void *ixgbe_dev_setup_link_thread_handler(void > > > *param); -static int ixgbe_dev_wait_setup_link_complete(struct > > rte_eth_dev *dev, > > > - uint32_t timeout_ms); > > > > > > > > > > + /* TODO - Even for platforms where ixgbe_check_sfp_cage() > > > + gives a > > clear > > > + * status result, if there's no interrupts, or no interrupt for= the SFP > > > + * cage present pin, even if other interrupts exist, then we st= ill need > > > + * to poll here to set the flag. > > > + */ > > > +#ifndef RTE_EXEC_ENV_FREEBSD > > > + struct rte_pci_device *pci_dev =3D RTE_ETH_DEV_TO_PCI(dev); > > > + struct rte_intr_handle *intr_handle =3D pci_dev->intr_handle; > > > + if (rte_intr_allow_others(intr_handle)) { > > > + /* check if lsc interrupt is enabled */ > > > + if (dev->data->dev_conf.intr_conf.lsc) > > > + have_int =3D true; > > > > > > > + /* Check for loss of SFP */ > > > + /* TODO - For platforms that don't have this flag, do we need t= o set > > > + * NEED_SFP_SETUP on LSC if we're a SFP platform? > > > + */ > > > + if (hw->mac.type =3D=3D ixgbe_mac_X550EM_a && > > > + (eicr & IXGBE_EICR_GPI_SDP0_X550EM_a)) > > > + intr->flags |=3D IXGBE_FLAG_NEED_SFP_SETUP; > > > + > > > return 0; > > > } > > > > > > TODO ? > > >=20 > These were notes that some further refinements could be possible. I can > remove the comments if that makes it more acceptable. The first TODO was > a note that (for FreeBSD mostly) w/o interrupts the code below (not shown= , > which checks for 'have_int') will run vs. exiting early and relying on th= e > NEED_SFP_SETUP flag being set elsewhere. The 2nd TODO was a question > from the original developer to himself where (I believe) he was consideri= ng > other implementations which may not use MOD_ABS the same that other > platforms do. I don't think this is an issue (that I know of) so removin= g this > commend and just moving on would be the best thing, and if someone else > knows of a situation where this consideration might need to be made, then > another patch can be submitted to address it. >=20 Ugh. Apologies for the dumb Outlook editor not wrapping the lines properly= , if you are reading this on patches.dpdk.org it runs off the page. Repeatin= g below: These were notes that some further refinements could be possible. I can=20 remove the comments if that makes it more acceptable. The first TODO was a note that (for FreeBSD mostly) w/o interrupts the code below (not shown,= =20 which checks for 'have_int') will run vs. exiting early and relying on the= =20 NEED_SFP_SETUP flag being set elsewhere. The 2nd TODO was a question=20 from the original developer to himself where (I believe) he was considering other implementations which may not use MOD_ABS the same that other platforms do. I don't think this is an issue (that I know of) so removing = this comment and just moving on would be the best thing, and if someone else knows of a situation where this consideration might need to be made, then another patch can be submitted to address it. > > > -- > > > 2.25.1