Re: [PATCH] net/ice: improve log messages for DDP loading

[Bruce Richardson <bruce.richardson@intel.com>]

On Sat, May 16, 2026 at 12:19:41PM +0200, David Marchand wrote:
> Some nics may not provide a serial number (PCI capability
> RTE_PCI_EXT_CAP_ID_DSN).
> 
> This results in a confusing ERROR log:
> ICE_INIT: ice_dev_init(): Failed to read device serial number
> 
> This is confusing as DDP loading does *not* require the serial number to
> be present for the port to be functional afterwards.
> 
> Besides, after trying various path, if the default DDP is not present on
> the runtime system, the port initialisation ends up with a vague error:
> ICE_INIT: ice_load_pkg(): failed to search file path
> 
> Improve the situation with adjusting the log level when reading the
> SN fails, then add more debug context to DDP file loading and end up
> with a ERROR log mentioning the expected file.
> 
> ICE_INIT: ice_firmware_read(): Cannot read DDP file
> 	/lib/firmware/updates/intel/ice/ddp/ice-b49691ffffe6e69c.pkg
> ICE_INIT: ice_firmware_read(): Cannot read DDP file
> 	/lib/firmware/intel/ice/ddp/ice-b49691ffffe6e69c.pkg
> ICE_INIT: ice_firmware_read(): Cannot read DDP file
> 	/lib/firmware/updates/intel/ice/ddp/ice.pkg
> ICE_INIT: ice_firmware_read(): Cannot read DDP file
> 	/lib/firmware/intel/ice/ddp/ice.pkg
> ICE_INIT: ice_load_pkg(): Failed to load default DDP package
> 	/lib/firmware/intel/ice/ddp/ice.pkg
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
>  drivers/net/intel/ice/ice_ethdev.c | 31 ++++++++++++++++++++----------
>  1 file changed, 21 insertions(+), 10 deletions(-)
> 
On top of the changes made in this patch, how about something like the
below to extend things?

Rather than just debug logging the failed paths, we change the debug logs
to always log each load attempt. We also record the path as it's attempted,
and then on failure to load any path we use that record to print out an
error message for each failure, so the user can see in the error logs ALL
the failed paths, rather than having to re-run the app at debug level to
find them.

WDYT?

/Bruce

diff --git a/drivers/net/intel/ice/ice_ethdev.c b/drivers/net/intel/ice/ice_ethdev.c
index e065581ccf..926b0febea 100644
--- a/drivers/net/intel/ice/ice_ethdev.c
+++ b/drivers/net/intel/ice/ice_ethdev.c
@@ -8,6 +8,7 @@
 #include <ctype.h>
 #include <fcntl.h>
 #include <stdio.h>
+#include <sys/queue.h>
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <unistd.h>
@@ -2003,15 +2004,23 @@ static int ice_read_customized_path(char *pkg_file, uint16_t buff_len)
        return n;
 }
 
+struct ddp_path_list_entry {
+       TAILQ_ENTRY(ddp_path_list_entry) next;
+       char path[ICE_MAX_PKG_FILENAME_SIZE];
+};
+TAILQ_HEAD(ddp_paths_head, ddp_path_list_entry);
+
 static int
-ice_firmware_read(const char *file, void *buf, size_t *bufsz)
+ice_firmware_read(const char *file, void *buf, size_t *bufsz, struct ddp_paths_head *searched_paths)
 {
-       int ret = rte_firmware_read(file, buf, bufsz);
-
-       if (ret < 0)
-               PMD_INIT_LOG(DEBUG, "Cannot read DDP file %s", file);
-
-       return ret;
+       /* record paths searched for DDP files */
+       struct ddp_path_list_entry *path = malloc(sizeof(struct ddp_path_list_entry));
+       if (path) {
+               strlcpy(path->path, file, sizeof(path->path));
+               TAILQ_INSERT_TAIL(searched_paths, path, next);
+       }
+       PMD_INIT_LOG(DEBUG, "Trying to load DDP file from: %s", file);
+       return rte_firmware_read(file, buf, bufsz);
 }
 
 int ice_load_pkg(struct ice_adapter *adapter, bool use_dsn, uint64_t dsn)
@@ -2020,6 +2029,8 @@ int ice_load_pkg(struct ice_adapter *adapter, bool use_dsn, uint64_t dsn)
        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];
+       struct ddp_paths_head searched_paths = TAILQ_HEAD_INITIALIZER(searched_paths);
+       struct ddp_path_list_entry *path, *tmp;  /* used to free the paths list */
        void *buf;
        size_t bufsz;
        int err;
@@ -2027,9 +2038,14 @@ int ice_load_pkg(struct ice_adapter *adapter, bool use_dsn, uint64_t dsn)
        /* first read any explicitly referenced DDP file*/
        if (adapter->devargs.ddp_filename != NULL) {
                strlcpy(pkg_file, adapter->devargs.ddp_filename, sizeof(pkg_file));
-               if (ice_firmware_read(pkg_file, &buf, &bufsz) == 0) {
+               if (ice_firmware_read(pkg_file, &buf, &bufsz, &searched_paths) == 0) {
                        goto load_fw;
                } else {
+                       /* free the searched paths list */
+                       RTE_TAILQ_FOREACH_SAFE(path, &searched_paths, next, tmp) {
+                               TAILQ_REMOVE(&searched_paths, path, next);
+                               free(path);
+                       }
                        PMD_INIT_LOG(ERR, "Cannot load DDP file: %s", pkg_file);
                        return -1;
                }
@@ -2043,11 +2059,11 @@ int ice_load_pkg(struct ice_adapter *adapter, bool use_dsn, uint64_t dsn)
                if (use_dsn) {
                        snprintf(pkg_file, RTE_DIM(pkg_file), "%s/%s",
                                        customized_path, opt_ddp_filename);
-                       if (ice_firmware_read(pkg_file, &buf, &bufsz) == 0)
+                       if (ice_firmware_read(pkg_file, &buf, &bufsz, &searched_paths) == 0)
                                goto load_fw;
                }
                snprintf(pkg_file, RTE_DIM(pkg_file), "%s/%s", customized_path, "ice.pkg");
-               if (ice_firmware_read(pkg_file, &buf, &bufsz) == 0)
+               if (ice_firmware_read(pkg_file, &buf, &bufsz, &searched_paths) == 0)
                        goto load_fw;
        }
 
@@ -2057,27 +2073,39 @@ int ice_load_pkg(struct ice_adapter *adapter, bool use_dsn, uint64_t dsn)
        strncpy(pkg_file, ICE_PKG_FILE_SEARCH_PATH_UPDATES,
                ICE_MAX_PKG_FILENAME_SIZE);
        strcat(pkg_file, opt_ddp_filename);
-       if (ice_firmware_read(pkg_file, &buf, &bufsz) == 0)
+       if (ice_firmware_read(pkg_file, &buf, &bufsz, &searched_paths) == 0)
                goto load_fw;
 
        strncpy(pkg_file, ICE_PKG_FILE_SEARCH_PATH_DEFAULT,
                ICE_MAX_PKG_FILENAME_SIZE);
        strcat(pkg_file, opt_ddp_filename);
-       if (ice_firmware_read(pkg_file, &buf, &bufsz) == 0)
+       if (ice_firmware_read(pkg_file, &buf, &bufsz, &searched_paths) == 0)
                goto load_fw;
 
 no_dsn:
        strncpy(pkg_file, ICE_PKG_FILE_UPDATES, ICE_MAX_PKG_FILENAME_SIZE);
-       if (ice_firmware_read(pkg_file, &buf, &bufsz) == 0)
+       if (ice_firmware_read(pkg_file, &buf, &bufsz, &searched_paths) == 0)
                goto load_fw;
 
        strncpy(pkg_file, ICE_PKG_FILE_DEFAULT, ICE_MAX_PKG_FILENAME_SIZE);
-       if (ice_firmware_read(pkg_file, &buf, &bufsz) < 0) {
-               PMD_INIT_LOG(ERR, "Failed to load default DDP package " ICE_PKG_FILE_DEFAULT);
+       if (ice_firmware_read(pkg_file, &buf, &bufsz, &searched_paths) < 0) {
+               /* list out all the failed package reads, emptying the list */
+               RTE_TAILQ_FOREACH_SAFE(path, &searched_paths, next, tmp) {
+                       PMD_INIT_LOG(ERR, "Failed to load DDP package from: %s", path->path);
+                       TAILQ_REMOVE(&searched_paths, path, next);
+                       free(path);
+               }
+               PMD_INIT_LOG(ERR, "Error: Cannot load any DDP package");
                return -1;
        }
 
 load_fw:
+       /* we have success opening a package, so clear the load list */
+       RTE_TAILQ_FOREACH_SAFE(path, &searched_paths, next, tmp) {
+               TAILQ_REMOVE(&searched_paths, path, next);
+               free(path);
+       }
+
        PMD_INIT_LOG(DEBUG, "DDP package name: %s", pkg_file);
 
        err = ice_copy_and_init_pkg(hw, buf, bufsz, adapter->devargs.ddp_load_sched);