From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
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 <dev@dpdk.org>; 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 <vita.batrla@centrum.cz>
To: dev@dpdk.org
Subject: inconsistency in i40e_aq_get_phy_capabilities() [i40e_common.c]
Message-ID: <ZnU0yW2r_mSlcjnB@ultra>
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 <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=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