Re: [dpdk-dev] [PATCH v3 2/2] eal: handle compressed firmwares

["Wang, Haiyue" <haiyue.wang@intel.com>]

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of David Marchand
> Sent: Tuesday, June 29, 2021 16:07
> To: dev@dpdk.org
> Cc: Igor Russkikh <irusskikh@marvell.com>; Aaron Conole <aconole@redhat.com>; Michael Santana
> <maicolgabriel@hotmail.com>; Richardson, Bruce <bruce.richardson@intel.com>; Rasesh Mody
> <rmody@marvell.com>; Shahed Shaikh <shshaikh@marvell.com>; Yang, Qiming <qiming.yang@intel.com>; Zhang,
> Qi Z <qi.z.zhang@intel.com>; Heinrich Kuhn <heinrich.kuhn@netronome.com>; Devendra Singh Rawat
> <dsinghrawat@marvell.com>; Ray Kinsella <mdr@ashroe.eu>; Neil Horman <nhorman@tuxdriver.com>; Dmitry
> Kozlyuk <dmitry.kozliuk@gmail.com>; Narcisa Ana Maria Vasile <navasile@linux.microsoft.com>; Dmitry
> Malloy <dmitrym@microsoft.com>; Kadam, Pallavi <pallavi.kadam@intel.com>
> Subject: [dpdk-dev] [PATCH v3 2/2] eal: handle compressed firmwares
> 
> Introduce an internal firmware loading helper to remove code duplication
> in our drivers and handle xz compressed firmwares by calling libarchive.
> 
> This helper tries to look for .xz suffixes so that drivers are not aware
> the firmwares have been compressed.
> 
> libarchive is set as an optional dependency: without libarchive, a
> runtime warning is emitted so that users know there is a compressed
> firmware.
> 
> Windows implementation is left as an empty stub.
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> Reviewed-by: Igor Russkikh <irusskikh@marvell.com>
> ---
> Changes since v2:
> - added a comment on libarchive link dependency,
> 
> Changes since v1:
> - used pkg-config for libarchive detection,
> - updated doxygen annotations,
> - added internal helpers in eal_firmware.c to enhance readability,
> - dropped whitespace damage in version.map,
> 
> ---
>  .github/workflows/build.yml    |   1 +
>  .travis.yml                    |   1 +
>  config/meson.build             |  10 +++
>  drivers/net/bnx2x/bnx2x.c      |  35 +++-----
>  drivers/net/ice/ice_ethdev.c   |  60 +++----------
>  drivers/net/nfp/nfp_net.c      |  57 +++----------
>  drivers/net/qede/qede_main.c   |  45 ++++------
>  lib/eal/include/rte_firmware.h |  32 +++++++
>  lib/eal/unix/eal_firmware.c    | 149 +++++++++++++++++++++++++++++++++
>  lib/eal/unix/meson.build       |   1 +
>  lib/eal/version.map            |   1 +
>  lib/eal/windows/eal.c          |   9 ++
>  12 files changed, 259 insertions(+), 142 deletions(-)
>  create mode 100644 lib/eal/include/rte_firmware.h
>  create mode 100644 lib/eal/unix/eal_firmware.c
> 


> +int
> +rte_firmware_read(const char *name, void **buf, size_t *bufsz)
> +{
> +	char path[PATH_MAX];
> +	int ret;
> +
> +	ret = firmware_read(name, buf, bufsz);
> +	if (ret < 0) {
> +		snprintf(path, sizeof(path), "%s.xz", name);
> +		path[PATH_MAX - 1] = '\0';
> +#ifndef RTE_HAS_LIBARCHIVE
> +		if (access(path, F_OK) == 0) {
> +			RTE_LOG(WARNING, EAL, "libarchive not available, %s cannot be decompressed\n",
> +				path);
> +		}
> +#else
> +		ret = firmware_read(path, buf, bufsz);
> +#endif
> +	}
> +	return ret;
> +}


Since ice PMD needs to check if the firmware file with different name can be accessed
by some kind of order, before doing the final firmware selection. Should we also add
the firmware access API for handling this ?

bool
rte_firmware_access(const char *name)
{
	char path[PATH_MAX];

	if (access(name, F_OK) == 0)
		return true;

	snprintf(path, sizeof(path), "%s.xz", name);
	path[PATH_MAX - 1] = '\0';
	if (access(path, F_OK) == 0)
		return true;

	return false;
}


> --
> 2.23.0