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 210D6454B7; Fri, 21 Jun 2024 15:38:19 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id E5E654026A; Fri, 21 Jun 2024 15:38:18 +0200 (CEST) Received: from gmmr-2.centrum.cz (gmmr-2.centrum.cz [46.255.227.203]) by mails.dpdk.org (Postfix) with ESMTP id 188104026A for ; Fri, 21 Jun 2024 12:51:18 +0200 (CEST) Received: from gmmr-2.cent (localhost [127.0.0.1]) by gmmr-2.centrum.cz (Postfix) with ESMTP id C8230200D8CD; Fri, 21 Jun 2024 12:51:17 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=centrum.cz; s=mail; t=1718967077; bh=JucGNFm6VBDmku7BmdXflbpK+IjLI1qzJTQtHjKdEh0=; h=Date:From:To:Subject:From; b=Zwf2Yzql5pfPFQI2kr1wAU/WMJ5I7scIwRowbJTnHMMK8kZA2bX8nSUwKBEwUlvEk G68zrc9Ira2cFoN3RSty6VA+ccASBOwp+wIbTSNuA+jwXBL0tq9WTfYW5DZ+q9qxpS 8IyQIgezji3nb/n2Fvpl2TxoLdltI6zbbhCmj3+g= X-SpamDetected: 1 Received: from antispam34.centrum.cz (envoy-stl.cent [10.32.56.18]) by gmmr-2.centrum.cz (Postfix) with ESMTP id C48631006A1D; Fri, 21 Jun 2024 10:07:41 +0200 (CEST) X-CSE-ConnectionGUID: wxrM4rGgRHaoQE0ARTmTcQ== X-CSE-MsgGUID: At1MFHQhTOiG6mY5CtYc3g== X-IPAS-Result: =?us-ascii?q?A2FK/gAwNHVm/0vj/y5aHAEBATwBAQQEAQECAQEHAQEVC?= =?us-ascii?q?YFIgVoCgWJLgRkIAYRNkWqDKIJVjE8chUyEBYFqgSw+DwEBAQEBAQEBAQkUA?= =?us-ascii?q?QElCQQBAQMEg2uKFCc4EwECBAEBAQEDAgMBAQEBAQEBAQEFAggBAQEBBgcCg?= =?us-ascii?q?RmFL0UNgniBDoEmAQEBAQEBAQEBAQEBAQEBAQEBFwINgSUEeg0CJgISg2CCL?= =?us-ascii?q?wE0FAavb38zGgJl3G0FgSAfgWYGgRosAgEBiBUaAWyEcoUUgVVEgRWGKwcXA?= =?us-ascii?q?oFGZ4MOgmkEiAgBkUmMDiYLQ4x5gVEcA1khARIBSwo1RxYCFgMbGDAPCQsmK?= =?us-ascii?q?gY5AhIMBgYGWTQJBCMDCAQDQgMgcREDBBoECwc4P4MlBBNEA4E3iWyCB4Eyg?= =?us-ascii?q?huEZIRugWsMYYJ6iDyEaYEVhBKBDR1AA3g9NRQbBqp8g3s8USwgYFoEH1HFO?= =?us-ascii?q?DQHhBaBXAYMgl2HRJYCg3KNAIZfA5JSLodYkGCNdpVCCIVKgXyBfzMag1tRG?= =?us-ascii?q?Y48FoNYzAp2OwIHCwEBAwmKagEB?= IronPort-Data: A9a23:PerobKNqTghezyrvrR3vlsFynXyQoLVcMsEvi/4bfWQNrUol0DJRn 2IaUWiHM63YYTTwLo0nPYTjoEgCvpLRmINkTHM5pCpnJ55oRWspJjg7wmPYZX76whjrFRo/h ykmQoCdap1yFzmE+0rF3oHJ9RFUzbuPSqf3FNnKMyVwQR4MYCo6gHqPocZg6mJTqYb/W1PlV e/a+ZWFZAf7gWcsawr41orawP9RlKWv0N8nlgNmDRx7lAe2v2UYCpsZOZawIxPQKqFIHvS3T vr017qw+GXU5X8FUrtJRZ6mGqGiaue60Tmm0hK6aYD76vRxjnBaPpIACRYpQRw/ZwOhxIktl YoX5fRcfi9yVkHEsLx1vxC1iEiSN4UekFPMCSDXXcB+UyQq2pYjqhljJBheAGEWxgp4KX5e7 9lEMh8CVDCKgNubmJuBduxQjf12eaEHPKtH0p1h5T7cSO0jXYiaGuPB6NlExio1wMtcdRrcT 5ZHL2AyMVKaOUIJZQp/5JEWxY9EglHhciFR7licubAz6kDYwQptyqXodtHHEjCPbZ4FxhzE/ z+fl4j/KjgGbfmx6QOFzlWtqbHKrH7QRLMZT7Lto5aGh3XWnAT/EiY+S1qnqL+zg1KkX9t3I EES5jAzqO455iSDQtTjdxGgrH3CuQQTM+e8CMVmtkfXl/eSuVzGQDdZJtJcVOEbWAYNbWRC/ je0cxnBWVSDbJX9paqhy4qp IronPort-HdrOrdr: A9a23:TUN2KqqUxSxElMLK6bf8aNoaV5pOeYIsimQD101hICG9Vvbo9P xG/c566faaslwssR0b9OxoW5PgfZq/z/BICOAqVN/IYOCMggSVxe9Zgbff/w== X-Talos-CUID: =?us-ascii?q?9a23=3A4zfw+2lK7bk2CWHeFkQ2ZJg1HeHXOWLN1lKMOWC?= =?us-ascii?q?3M0ZgT4S4EXaqxKZYmPM7zg=3D=3D?= X-Talos-MUID: 9a23:YVci9gl7oFSyHXwrJuM9dnpLKJ5NzYSXCHwqkLpYvvjZHjBBMGyS2WE= X-IronPort-Anti-Spam-Filtered: true X-SpamDetected: 1 X-IronPort-AV: E=Sophos;i="6.08,254,1712613600"; d="scan'208";a="38024151" Received: from unknown (HELO gm-smtp11.centrum.cz) ([46.255.227.75]) by antispam34.centrum.cz with ESMTP; 21 Jun 2024 10:07:41 +0200 Received: from ultra (109-81-175-189.rct.o2.cz [109.81.175.189]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by gm-smtp11.centrum.cz (Postfix) with ESMTPSA id 5D5A7100CD4D9; Fri, 21 Jun 2024 10:07:41 +0200 (CEST) Date: Fri, 21 Jun 2024 10:07:37 +0200 From: Vita Batrla To: dev@dpdk.org Subject: inconsistency in i40e_aq_get_phy_capabilities() [i40e_common.c] Message-ID: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline X-Mailman-Approved-At: Fri, 21 Jun 2024 15:38:17 +0200 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 Hello developers, I noticed an inconsistency in i40e_common.c file [1]. I don't think it is causing any functionality issue, but it looks interesting to me. The function i40e_aq_get_phy_capabilities() has different behavior for X722 and XL710 MACs. An early call to i40e_aq_get_phy_capabilities(report_init = true) results in: * function call i40e_aq_get_phy_capabilities() -> i40e_aq_get_link_info() if the MAC is XL710 * no call to i40e_aq_get_link_info() if MAC is X722 As a consequence, after the first call to i40e_aq_get_phy_capabilities() the NIC driver fills fields in i40e->hw.phy.link_info structure, but only for XL710 (and only if FW version is not too old). Why the driver doesn't call i40e_aq_get_phy_capabilities() for X722 (if the FW version is not too old)? It looks quite strange to me, I could not find the reason why the difference is there. If there's no underlying issue, I would like to see the code treating X722 and XL710 equally. Yes I noticed the comments in code that the response to AQ cmd 0x604 is a little bit different on X722, the FW doesn't seem to provide valid link_type* fields in response, but the code might deal with that. Please see an example (pseudo-patch): i40e_aq_get_link_info() ... if (report_init) { if (hw->mac.type == I40E_MAC_XL710 && hw->aq.api_maj_ver == I40E_FW_API_VERSION_MAJOR && hw->aq.api_min_ver >= I40E_MINOR_VER_GET_LINK_INFO_XL710) { status = i40e_aq_get_link_info(hw, true, NULL, NULL); - } else { - hw->phy.phy_types = LE32_TO_CPU(abilities->phy_type); - hw->phy.phy_types |= - ((u64)abilities->phy_type_ext << 32); + } else if (hw->mac.type == I40E_MAC_X722 && + hw->aq.api_maj_ver == I40E_FW_API_VERSION_MAJOR && + hw->aq.api_min_ver >= I40E_MINOR_VER_GET_LINK_INFO_X722) { status = i40e_aq_get_link_info(hw, true, NULL, NULL); } } The pseudo-patch doesn't solve the 'hw->phy.phy_types' question. There are two options what to do with 'hw->phy.phy_types': a) remove it completely (including declaration), it seems nobody reads it b) initialize 'hw->phy.phy_types' sometimes in i40e_aq_get_link_info() and sometimes in i40e_aq_get_phy_capabilities() as the current code does. The i40e_aq_get_phy_capabilities() could check if 'phy_types' were set by i40e_aq_get_link_info(), if not, then do the initialization in i40e_aq_get_phy_capabilities(), e.g. keep the 3 lines that are removed in the above pseudo-patch, just add them behind some check to execute them only if the i40e_aq_get_link_info() did not already set that field. Thanks, Vita [1] https://github.com/DPDK/dpdk/blob/main/drivers/net/i40e/base/i40e_common.c