diff mbox series

[v4] net/ice: support customized search path for DDP package

Message ID 20240913061537.2077253-1-zhichaox.zeng@intel.com (mailing list archive)
State Superseded
Delegated to: Bruce Richardson
Headers
Series [v4] net/ice: support customized search path for DDP package |

Checks

Context Check Description
ci/loongarch-compilation success Compilation OK
ci/checkpatch success coding style OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/github-robot: build success github build: passed
ci/intel-Functional success Functional PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-compile-amd64-testing success Testing PASS
ci/iol-unit-arm64-testing success Testing PASS
ci/iol-unit-amd64-testing success Testing PASS
ci/iol-compile-arm64-testing success Testing PASS
ci/iol-marvell-Functional success Functional Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-sample-apps-testing success Testing PASS

Commit Message

Zhichao Zeng Sept. 13, 2024, 6:15 a.m. UTC 
This patch adds support for customizing firmware search path for
DDP package like the kernel behavior, it will read the search path
from "/sys/module/firmware_class/parameters/path", and try to load
DDP package.

Also, updates documentation for loading the DDP package in ice.rst.

Signed-off-by: Zhichao Zeng <zhichaox.zeng@intel.com>

---
v4: fix CI error
v3: update doc, fix code error
v2: separate the patch and rewrite the log
---
 doc/guides/nics/ice.rst      | 63 +++++++++++++++++++-----------------
 drivers/net/ice/ice_ethdev.c | 44 +++++++++++++++++++++++++
 drivers/net/ice/ice_ethdev.h |  1 +
 3 files changed, 79 insertions(+), 29 deletions(-)

Comments

Bruce Richardson Sept. 13, 2024, 10:57 a.m. UTC | #1 
On Fri, Sep 13, 2024 at 02:15:37PM +0800, Zhichao Zeng wrote:
> This patch adds support for customizing firmware search path for
> DDP package like the kernel behavior, it will read the search path
> from "/sys/module/firmware_class/parameters/path", and try to load
> DDP package.
> 
> Also, updates documentation for loading the DDP package in ice.rst.
> 
> Signed-off-by: Zhichao Zeng <zhichaox.zeng@intel.com>
> 
> ---
> v4: fix CI error
> v3: update doc, fix code error
> v2: separate the patch and rewrite the log
> ---
>  doc/guides/nics/ice.rst      | 63 +++++++++++++++++++-----------------
>  drivers/net/ice/ice_ethdev.c | 44 +++++++++++++++++++++++++
>  drivers/net/ice/ice_ethdev.h |  1 +
>  3 files changed, 79 insertions(+), 29 deletions(-)
> 
> diff --git a/doc/guides/nics/ice.rst b/doc/guides/nics/ice.rst
> index ae975d19ad..7f347684e7 100644
> --- a/doc/guides/nics/ice.rst
> +++ b/doc/guides/nics/ice.rst
> @@ -80,6 +80,40 @@ are listed in the Tested Platforms section of the Release Notes for each release
>     |    24.03  |     1.13.7    |      1.3.35     |  1.3.45   |    1.3.13    |    4.4    |
>     +-----------+---------------+-----------------+-----------+--------------+-----------+
>  
> +Dynamic Device Personalization (DDP) package loading
> +----------------------------------------------------
> +
> +The Intel E810 requires a programmable pipeline package be downloaded
> +by the driver to support normal operations. The E810 has a limited
> +functionality built in to allow PXE boot and other use cases, but the
> +driver must download a package file during the driver initialization
> +stage.

Hi Zhichao,

I realise that this text above is not written by you, but when moving this
text we can update it a little.

* in second sentence, drop the word "a" before "limited functionality".
* I feel we are missing a few words in the second sentence to clarify the
  need for DDP support. For example, after the word "but" we could clarify
  with "for normal use" or "for DPDK use" or some similar phrase.

> +
> +The default DDP package file name is ice.pkg. For a specific NIC, the
> +DDP package supposed to be loaded can have a filename: ice-xxxxxx.pkg,

For the filenames: ice.pkg and ice-xxxxxx.pkg, put them in `` quotes to
make them monospaced/literal text. Same for filenames below.

> +where 'xxxxxx' is the 64-bit PCIe Device Serial Number of the NIC. For
> +example, if the NIC's device serial number is 00-CC-BB-FF-FF-AA-05-68,
> +the device-specific DDP package filename is ice-00ccbbffffaa0568.pkg
> +(in hex and all low case), please review README from
> +`Intel® Ethernet 800 Series Dynamic Device Personalization (DDP) for Telecommunication (Comms) Package
> +<https://www.intel.com/content/www/us/en/download/19660/intel-ethernet-800-series-dynamic-device-personalization-ddp-for-telecommunication-comms-package.html>`_
> +for more information. A symbolic link to the DDP package file is also ok.
> +The same package file is used by both the kernel driver and the ICE PMD.
> +
> +ICE PMD support customized DDP search path, driver will read search path

"support" -> "supports using a"
", driver" -> ". The driver"
"search path" -> "the search path"

> +from '/sys/module/firmware_class/parameters/path' as CUSTOMIZED_PATH.
> +During initialization, the driver searches in the following paths in order:
> +CUSTOMIZED_PATH(mentioned above), /lib/firmware/updates/intel/ice/ddp and

you can drop "(mentioned above)".

> +/lib/firmware/intel/ice/ddp. The corresponding device-specific DDP
> +package will be downloaded first if the file exists. If not, then the
> +driver tries to load the default package. The type of loaded package
> +is stored in ``ice_adapter->active_pkg_type``.

The behaviour is not exactly clear here. The text seems to imply (at least
ot me) that the search order is to check each directory first for the
serial-number package and then for ice.pkg, before moving on to the next
directory. However, the actual search pattern in the code  is to check each
directory in turn for a serial number package, and then scan each directory
in turn for an ice.pkg file. Consider the case of having two files:

$CUSTOMIZED_PATH/ice.pkg
/lib/firmware/intel/ice/ddp/ice-xxxxxxx.pkg

As described in the doc text above, I would expect the first of these two
packages to be used, since it's in the customized path, but in fact the
second will be used by the code since it has a serial number on it so is
found in the initial scan for a numbered-package.

> +
> +   .. Note::
> +
> +      Windows support: The DDP package is not supported on Windows so,
> +      loading of the package is disabled on Windows.

I notice that the comma "," is in the wrong place in
the original text - it should come before the "so" not after it - but
overall the whole line can just be shortened to:
	"Windows support: DDP packages are not supported on Windows"

<snip for brevity>

Thanks,
/Bruce
diff mbox series

Patch

diff --git a/doc/guides/nics/ice.rst b/doc/guides/nics/ice.rst
index ae975d19ad..7f347684e7 100644
--- a/doc/guides/nics/ice.rst
+++ b/doc/guides/nics/ice.rst
@@ -80,6 +80,40 @@  are listed in the Tested Platforms section of the Release Notes for each release
    |    24.03  |     1.13.7    |      1.3.35     |  1.3.45   |    1.3.13    |    4.4    |
    +-----------+---------------+-----------------+-----------+--------------+-----------+
 
+Dynamic Device Personalization (DDP) package loading
+----------------------------------------------------
+
+The Intel E810 requires a programmable pipeline package be downloaded
+by the driver to support normal operations. The E810 has a limited
+functionality built in to allow PXE boot and other use cases, but the
+driver must download a package file during the driver initialization
+stage.
+
+The default DDP package file name is ice.pkg. For a specific NIC, the
+DDP package supposed to be loaded can have a filename: ice-xxxxxx.pkg,
+where 'xxxxxx' is the 64-bit PCIe Device Serial Number of the NIC. For
+example, if the NIC's device serial number is 00-CC-BB-FF-FF-AA-05-68,
+the device-specific DDP package filename is ice-00ccbbffffaa0568.pkg
+(in hex and all low case), please review README from
+`Intel® Ethernet 800 Series Dynamic Device Personalization (DDP) for Telecommunication (Comms) Package
+<https://www.intel.com/content/www/us/en/download/19660/intel-ethernet-800-series-dynamic-device-personalization-ddp-for-telecommunication-comms-package.html>`_
+for more information. A symbolic link to the DDP package file is also ok.
+The same package file is used by both the kernel driver and the ICE PMD.
+
+ICE PMD support customized DDP search path, driver will read search path
+from '/sys/module/firmware_class/parameters/path' as CUSTOMIZED_PATH.
+During initialization, the driver searches in the following paths in order:
+CUSTOMIZED_PATH(mentioned above), /lib/firmware/updates/intel/ice/ddp and
+/lib/firmware/intel/ice/ddp. The corresponding device-specific DDP
+package will be downloaded first if the file exists. If not, then the
+driver tries to load the default package. The type of loaded package
+is stored in ``ice_adapter->active_pkg_type``.
+
+   .. Note::
+
+      Windows support: The DDP package is not supported on Windows so,
+      loading of the package is disabled on Windows.
+
 Configuration
 -------------
 
@@ -487,32 +521,3 @@  Usage::
 
 In "brief" mode, all scheduling nodes in the tree are displayed.
 In "detail" mode, each node's configuration parameters are also displayed.
-
-Limitations or Known issues
----------------------------
-
-The Intel E810 requires a programmable pipeline package be downloaded
-by the driver to support normal operations. The E810 has a limited
-functionality built in to allow PXE boot and other use cases, but the
-driver must download a package file during the driver initialization
-stage.
-
-The default DDP package file name is ice.pkg. For a specific NIC, the
-DDP package supposed to be loaded can have a filename: ice-xxxxxx.pkg,
-where 'xxxxxx' is the 64-bit PCIe Device Serial Number of the NIC. For
-example, if the NIC's device serial number is 00-CC-BB-FF-FF-AA-05-68,
-the device-specific DDP package filename is ice-00ccbbffffaa0568.pkg
-(in hex and all low case). During initialization, the driver searches
-in the following paths in order: /lib/firmware/updates/intel/ice/ddp
-and /lib/firmware/intel/ice/ddp. The corresponding device-specific DDP
-package will be downloaded first if the file exists. If not, then the
-driver tries to load the default package. The type of loaded package
-is stored in ``ice_adapter->active_pkg_type``.
-
-A symbolic link to the DDP package file is also ok. The same package
-file is used by both the kernel driver and the DPDK PMD.
-
-   .. Note::
-
-      Windows support: The DDP package is not supported on Windows so,
-      loading of the package is disabled on Windows.
diff --git a/drivers/net/ice/ice_ethdev.c b/drivers/net/ice/ice_ethdev.c
index 304f959b7e..13b3d79a3d 100644
--- a/drivers/net/ice/ice_ethdev.c
+++ b/drivers/net/ice/ice_ethdev.c
@@ -1873,21 +1873,60 @@  ice_load_pkg_type(struct ice_hw *hw)
 	return package_type;
 }
 
+static int ice_read_customized_path(char *pkg_file, uint16_t buff_len)
+{
+	FILE *fp = fopen(ICE_PKG_FILE_CUSTOMIZED_PATH, "r");
+	int ret = 0;
+
+	if (fp == NULL) {
+		PMD_INIT_LOG(INFO, "Failed to read CUSTOMIZED_PATH");
+		return -EIO;
+	}
+
+	if (fgets(pkg_file, buff_len, fp) == NULL) {
+		ret = -EIO;
+		goto exit;
+	}
+
+	if (strlen(pkg_file) <= 1)
+		goto exit;
+
+	if (pkg_file[strlen(pkg_file) - 2] == '/') {
+		pkg_file[strlen(pkg_file) - 1] = '\0';
+	} else {
+		pkg_file[strlen(pkg_file) - 1] = '/';
+		pkg_file[strlen(pkg_file)] = '\0';
+	}
+
+exit:
+	fclose(fp);
+	return ret;
+}
+
 int ice_load_pkg(struct ice_adapter *adapter, bool use_dsn, uint64_t dsn)
 {
 	struct ice_hw *hw = &adapter->hw;
 	char pkg_file[ICE_MAX_PKG_FILENAME_SIZE];
+	char customized_path[ICE_MAX_PKG_FILENAME_SIZE];
 	char opt_ddp_filename[ICE_MAX_PKG_FILENAME_SIZE];
 	void *buf;
 	size_t bufsz;
 	int err;
 
+	ice_read_customized_path(customized_path, ICE_MAX_PKG_FILENAME_SIZE);
+
 	if (!use_dsn)
 		goto no_dsn;
 
 	memset(opt_ddp_filename, 0, ICE_MAX_PKG_FILENAME_SIZE);
 	snprintf(opt_ddp_filename, ICE_MAX_PKG_FILENAME_SIZE,
 		"ice-%016" PRIx64 ".pkg", dsn);
+
+	strlcpy(pkg_file, customized_path, ICE_MAX_PKG_FILENAME_SIZE);
+	strlcat(pkg_file, opt_ddp_filename, ICE_MAX_PKG_FILENAME_SIZE);
+	if (rte_firmware_read(pkg_file, &buf, &bufsz) == 0)
+		goto load_fw;
+
 	strncpy(pkg_file, ICE_PKG_FILE_SEARCH_PATH_UPDATES,
 		ICE_MAX_PKG_FILENAME_SIZE);
 	strcat(pkg_file, opt_ddp_filename);
@@ -1901,6 +1940,11 @@  int ice_load_pkg(struct ice_adapter *adapter, bool use_dsn, uint64_t dsn)
 		goto load_fw;
 
 no_dsn:
+	strlcpy(pkg_file, customized_path, ICE_MAX_PKG_FILENAME_SIZE);
+	strlcat(pkg_file, "ice.pkg", ICE_MAX_PKG_FILENAME_SIZE);
+	if (rte_firmware_read(pkg_file, &buf, &bufsz) == 0)
+		goto load_fw;
+
 	strncpy(pkg_file, ICE_PKG_FILE_UPDATES, ICE_MAX_PKG_FILENAME_SIZE);
 	if (rte_firmware_read(pkg_file, &buf, &bufsz) == 0)
 		goto load_fw;
diff --git a/drivers/net/ice/ice_ethdev.h b/drivers/net/ice/ice_ethdev.h
index 3ea9f37dc8..8b644ed700 100644
--- a/drivers/net/ice/ice_ethdev.h
+++ b/drivers/net/ice/ice_ethdev.h
@@ -51,6 +51,7 @@ 
 #define ICE_PKG_FILE_UPDATES "/lib/firmware/updates/intel/ice/ddp/ice.pkg"
 #define ICE_PKG_FILE_SEARCH_PATH_DEFAULT "/lib/firmware/intel/ice/ddp/"
 #define ICE_PKG_FILE_SEARCH_PATH_UPDATES "/lib/firmware/updates/intel/ice/ddp/"
+#define ICE_PKG_FILE_CUSTOMIZED_PATH "/sys/module/firmware_class/parameters/path"
 #define ICE_MAX_PKG_FILENAME_SIZE   256
 
 #define MAX_ACL_NORMAL_ENTRIES    256