[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
  

David Marchand <david.marchand@redhat.com> writes:

> 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
>
> diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
> index 7c4d6dcdbf..7dac20ddeb 100644
> --- a/.github/workflows/build.yml
> +++ b/.github/workflows/build.yml
> @@ -93,6 +93,7 @@ jobs:
>        run: sudo apt install -y ccache libnuma-dev python3-setuptools
>          python3-wheel python3-pip python3-pyelftools ninja-build libbsd-dev
>          libpcap-dev libibverbs-dev libcrypto++-dev libfdt-dev libjansson-dev
> +        libarchive-dev
>      - name: Install libabigail build dependencies if no cache is available
>        if: env.ABI_CHECKS == 'true' && steps.libabigail-cache.outputs.cache-hit != 'true'
>        run: sudo apt install -y autoconf automake libtool pkg-config libxml2-dev
> diff --git a/.travis.yml b/.travis.yml
> index 5b702cc9bb..23067d9e3c 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -16,6 +16,7 @@ addons:
>      packages: &required_packages
>        - [libnuma-dev, python3-setuptools, python3-wheel, python3-pip, python3-pyelftools, ninja-build]
>        - [libbsd-dev, libpcap-dev, libibverbs-dev, libcrypto++-dev, libfdt-dev, libjansson-dev]
> +      - [libarchive-dev]
>  
>  _aarch64_packages: &aarch64_packages
>    - *required_packages
> diff --git a/config/meson.build b/config/meson.build
> index 017bb2efbb..639aa74e12 100644
> --- a/config/meson.build
> +++ b/config/meson.build
> @@ -172,6 +172,16 @@ if libexecinfo.found() and cc.has_header('execinfo.h')
>      dpdk_extra_ldflags += '-lexecinfo'
>  endif
>  
> +libarchive = dependency('libarchive', required: false, method: 'pkg-config')
> +if libarchive.found()
> +    dpdk_conf.set('RTE_HAS_LIBARCHIVE', 1)
> +    # Push libarchive link dependency at the project level to support
> +    # statically linking dpdk apps. Details at:
> +    # https://inbox.dpdk.org/dev/20210605004024.660267a1@sovereign/
> +    add_project_link_arguments('-larchive', language: 'c')
> +    dpdk_extra_ldflags += '-larchive'
> +endif
> +
>  # check for libbsd
>  libbsd = dependency('libbsd', required: false, method: 'pkg-config')
>  if libbsd.found()
> diff --git a/drivers/net/bnx2x/bnx2x.c b/drivers/net/bnx2x/bnx2x.c
> index 654878d9de..60292753e2 100644
> --- a/drivers/net/bnx2x/bnx2x.c
> +++ b/drivers/net/bnx2x/bnx2x.c
> @@ -26,7 +26,9 @@
>  #include <arpa/inet.h>
>  #include <fcntl.h>
>  #include <zlib.h>
> +
>  #include <rte_bitops.h>
> +#include <rte_firmware.h>
>  #include <rte_string_fns.h>
>  
>  #define BNX2X_PMD_VER_PREFIX "BNX2X PMD"
> @@ -9655,44 +9657,33 @@ static void bnx2x_init_rte(struct bnx2x_softc *sc)
>  void bnx2x_load_firmware(struct bnx2x_softc *sc)
>  {
>  	const char *fwname;
> -	int f;
> -	struct stat st;
> +	void *buf;
> +	size_t bufsz;
>  
>  	fwname = sc->devinfo.device_id == CHIP_NUM_57711
>  		? FW_NAME_57711 : FW_NAME_57810;
> -	f = open(fwname, O_RDONLY);
> -	if (f < 0) {
> +	if (rte_firmware_read(fwname, &buf, &bufsz) != 0) {
>  		PMD_DRV_LOG(NOTICE, sc, "Can't open firmware file");
>  		return;
>  	}
>  
> -	if (fstat(f, &st) < 0) {
> -		PMD_DRV_LOG(NOTICE, sc, "Can't stat firmware file");
> -		close(f);
> -		return;
> -	}
> -
> -	sc->firmware = rte_zmalloc("bnx2x_fw", st.st_size, RTE_CACHE_LINE_SIZE);
> +	sc->firmware = rte_zmalloc("bnx2x_fw", bufsz, RTE_CACHE_LINE_SIZE);
>  	if (!sc->firmware) {
>  		PMD_DRV_LOG(NOTICE, sc, "Can't allocate memory for firmware");
> -		close(f);
> -		return;
> +		goto out;
>  	}
>  
> -	if (read(f, sc->firmware, st.st_size) != st.st_size) {
> -		PMD_DRV_LOG(NOTICE, sc, "Can't read firmware data");
> -		close(f);
> -		return;
> -	}
> -	close(f);
> -
> -	sc->fw_len = st.st_size;
> +	sc->fw_len = bufsz;
>  	if (sc->fw_len < FW_HEADER_LEN) {
>  		PMD_DRV_LOG(NOTICE, sc,
>  			    "Invalid fw size: %" PRIu64, sc->fw_len);
> -		return;
> +		goto out;
>  	}
> +
> +	memcpy(sc->firmware, buf, sc->fw_len);
>  	PMD_DRV_LOG(DEBUG, sc, "fw_len = %" PRIu64, sc->fw_len);
> +out:
> +	free(buf);
>  }
>  
>  static void
> diff --git a/drivers/net/ice/ice_ethdev.c b/drivers/net/ice/ice_ethdev.c
> index d180b73c32..06da1bbd94 100644
> --- a/drivers/net/ice/ice_ethdev.c
> +++ b/drivers/net/ice/ice_ethdev.c
> @@ -10,6 +10,7 @@
>  #include <sys/stat.h>
>  #include <unistd.h>
>  
> +#include <rte_firmware.h>
>  #include <rte_tailq.h>
>  
>  #include "base/ice_sched.h"
> @@ -1673,22 +1674,14 @@ ice_load_pkg_type(struct ice_hw *hw)
>  	return package_type;
>  }
>  
> -#ifdef RTE_EXEC_ENV_WINDOWS
> -#define ice_access _access
> -#else
> -#define ice_access access
> -#endif
> -
>  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 opt_ddp_filename[ICE_MAX_PKG_FILENAME_SIZE];
> +	void *buf;
> +	size_t bufsz;
>  	int err;
> -	uint8_t *buf = NULL;
> -	int buf_len;
> -	FILE *file;
> -	struct stat fstat;
>  
>  	if (!use_dsn)
>  		goto no_dsn;
> @@ -1698,57 +1691,31 @@ int ice_load_pkg(struct ice_adapter *adapter, bool use_dsn, uint64_t dsn)
>  		"ice-%016" PRIx64 ".pkg", dsn);
>  	strncpy(pkg_file, ICE_PKG_FILE_SEARCH_PATH_UPDATES,
>  		ICE_MAX_PKG_FILENAME_SIZE);
> -	if (!ice_access(strcat(pkg_file, opt_ddp_filename), 0))
> +	strcat(pkg_file, opt_ddp_filename);
> +	if (rte_firmware_read(pkg_file, &buf, &bufsz) == 0)
>  		goto load_fw;
>  
>  	strncpy(pkg_file, ICE_PKG_FILE_SEARCH_PATH_DEFAULT,
>  		ICE_MAX_PKG_FILENAME_SIZE);
> -	if (!ice_access(strcat(pkg_file, opt_ddp_filename), 0))
> +	strcat(pkg_file, opt_ddp_filename);
> +	if (rte_firmware_read(pkg_file, &buf, &bufsz) == 0)
>  		goto load_fw;
>  
>  no_dsn:
>  	strncpy(pkg_file, ICE_PKG_FILE_UPDATES, ICE_MAX_PKG_FILENAME_SIZE);
> -	if (!ice_access(pkg_file, 0))
> +	if (rte_firmware_read(pkg_file, &buf, &bufsz) == 0)
>  		goto load_fw;
> +
>  	strncpy(pkg_file, ICE_PKG_FILE_DEFAULT, ICE_MAX_PKG_FILENAME_SIZE);
> -	if (ice_access(pkg_file, 0)) {
> +	if (rte_firmware_read(pkg_file, &buf, &bufsz) < 0) {
>  		PMD_INIT_LOG(ERR, "failed to search file path\n");
>  		return -1;
>  	}
>  
>  load_fw:
> -	file = fopen(pkg_file, "rb");
> -	if (!file)  {
> -		PMD_INIT_LOG(ERR, "failed to open file: %s\n", pkg_file);
> -		return -1;
> -	}
> -
>  	PMD_INIT_LOG(DEBUG, "DDP package name: %s", pkg_file);
>  
> -	err = stat(pkg_file, &fstat);
> -	if (err) {
> -		PMD_INIT_LOG(ERR, "failed to get file stats\n");
> -		goto out;
> -	}
> -
> -	buf_len = fstat.st_size;
> -	buf = rte_malloc(NULL, buf_len, 0);
> -
> -	if (!buf) {
> -		PMD_INIT_LOG(ERR, "failed to allocate buf of size %d for package\n",
> -				buf_len);
> -		err = -1;
> -		goto out;
> -	}
> -
> -	err = fread(buf, buf_len, 1, file);
> -	if (err != 1) {
> -		PMD_INIT_LOG(ERR, "failed to read package data\n");
> -		err = -1;
> -		goto out;
> -	}
> -
> -	err = ice_copy_and_init_pkg(hw, buf, buf_len);
> +	err = ice_copy_and_init_pkg(hw, buf, bufsz);
>  	if (err) {
>  		PMD_INIT_LOG(ERR, "ice_copy_and_init_hw failed: %d\n", err);
>  		goto out;
> @@ -1758,13 +1725,10 @@ int ice_load_pkg(struct ice_adapter *adapter, bool use_dsn, uint64_t dsn)
>  	adapter->active_pkg_type = ice_load_pkg_type(hw);
>  
>  out:
> -	fclose(file);
> -	rte_free(buf);
> +	free(buf);
>  	return err;
>  }
>  
> -#undef ice_access
> -
>  static void
>  ice_base_queue_get(struct ice_pf *pf)
>  {
> diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c
> index 2ee88fbfc7..879da7ef59 100644
> --- a/drivers/net/nfp/nfp_net.c
> +++ b/drivers/net/nfp/nfp_net.c
> @@ -29,6 +29,7 @@
>  #include <rte_alarm.h>
>  #include <rte_spinlock.h>
>  #include <rte_service_component.h>
> +#include <rte_firmware.h>
>  
>  #include "nfpcore/nfp_cpp.h"
>  #include "nfpcore/nfp_nffw.h"
> @@ -3366,12 +3367,10 @@ static int
>  nfp_fw_upload(struct rte_pci_device *dev, struct nfp_nsp *nsp, char *card)
>  {
>  	struct nfp_cpp *cpp = nsp->cpp;
> -	int fw_f;
> -	char *fw_buf;
> +	void *fw_buf;
>  	char fw_name[125];
>  	char serial[40];
> -	struct stat file_stat;
> -	off_t fsize, bytes;
> +	size_t fsize;
>  
>  	/* Looking for firmware file in order of priority */
>  
> @@ -3384,66 +3383,34 @@ nfp_fw_upload(struct rte_pci_device *dev, struct nfp_nsp *nsp, char *card)
>  
>  	snprintf(fw_name, sizeof(fw_name), "%s/%s.nffw", DEFAULT_FW_PATH,
>  			serial);
> -
>  	PMD_DRV_LOG(DEBUG, "Trying with fw file: %s", fw_name);
> -	fw_f = open(fw_name, O_RDONLY);
> -	if (fw_f >= 0)
> -		goto read_fw;
> +	if (rte_firmware_read(fw_name, &fw_buf, &fsize) == 0)
> +		goto load_fw;
>  
>  	/* Then try the PCI name */
>  	snprintf(fw_name, sizeof(fw_name), "%s/pci-%s.nffw", DEFAULT_FW_PATH,
>  			dev->device.name);
> -
>  	PMD_DRV_LOG(DEBUG, "Trying with fw file: %s", fw_name);
> -	fw_f = open(fw_name, O_RDONLY);
> -	if (fw_f >= 0)
> -		goto read_fw;
> +	if (rte_firmware_read(fw_name, &fw_buf, &fsize) == 0)
> +		goto load_fw;
>  
>  	/* Finally try the card type and media */
>  	snprintf(fw_name, sizeof(fw_name), "%s/%s", DEFAULT_FW_PATH, card);
>  	PMD_DRV_LOG(DEBUG, "Trying with fw file: %s", fw_name);
> -	fw_f = open(fw_name, O_RDONLY);
> -	if (fw_f < 0) {
> +	if (rte_firmware_read(fw_name, &fw_buf, &fsize) < 0) {
>  		PMD_DRV_LOG(INFO, "Firmware file %s not found.", fw_name);
>  		return -ENOENT;
>  	}
>  
> -read_fw:
> -	if (fstat(fw_f, &file_stat) < 0) {
> -		PMD_DRV_LOG(INFO, "Firmware file %s size is unknown", fw_name);
> -		close(fw_f);
> -		return -ENOENT;
> -	}
> -
> -	fsize = file_stat.st_size;
> -	PMD_DRV_LOG(INFO, "Firmware file found at %s with size: %" PRIu64 "",
> -			    fw_name, (uint64_t)fsize);
> -
> -	fw_buf = malloc((size_t)fsize);
> -	if (!fw_buf) {
> -		PMD_DRV_LOG(INFO, "malloc failed for fw buffer");
> -		close(fw_f);
> -		return -ENOMEM;
> -	}
> -	memset(fw_buf, 0, fsize);
> -
> -	bytes = read(fw_f, fw_buf, fsize);
> -	if (bytes != fsize) {
> -		PMD_DRV_LOG(INFO, "Reading fw to buffer failed."
> -				   "Just %" PRIu64 " of %" PRIu64 " bytes read",
> -				   (uint64_t)bytes, (uint64_t)fsize);
> -		free(fw_buf);
> -		close(fw_f);
> -		return -EIO;
> -	}
> +load_fw:
> +	PMD_DRV_LOG(INFO, "Firmware file found at %s with size: %zu",
> +		fw_name, fsize);
>  
>  	PMD_DRV_LOG(INFO, "Uploading the firmware ...");
> -	nfp_nsp_load_fw(nsp, fw_buf, bytes);
> +	nfp_nsp_load_fw(nsp, fw_buf, fsize);
>  	PMD_DRV_LOG(INFO, "Done");
>  
>  	free(fw_buf);
> -	close(fw_f);
> -
>  	return 0;
>  }
>  
> diff --git a/drivers/net/qede/qede_main.c b/drivers/net/qede/qede_main.c
> index caa9d1d4f6..504e2c66a0 100644
> --- a/drivers/net/qede/qede_main.c
> +++ b/drivers/net/qede/qede_main.c
> @@ -5,7 +5,9 @@
>   */
>  
>  #include <limits.h>
> +
>  #include <rte_alarm.h>
> +#include <rte_firmware.h>
>  #include <rte_string_fns.h>
>  
>  #include "qede_ethdev.h"
> @@ -127,51 +129,40 @@ static void qed_free_stream_mem(struct ecore_dev *edev)
>  #ifdef CONFIG_ECORE_BINARY_FW
>  static int qed_load_firmware_data(struct ecore_dev *edev)
>  {
> -	int fd;
> -	struct stat st;
>  	const char *fw = RTE_LIBRTE_QEDE_FW;
> +	void *buf;
> +	size_t bufsz;
> +	int ret;
>  
>  	if (strcmp(fw, "") == 0)
>  		strcpy(qede_fw_file, QEDE_DEFAULT_FIRMWARE);
>  	else
>  		strcpy(qede_fw_file, fw);
>  
> -	fd = open(qede_fw_file, O_RDONLY);
> -	if (fd < 0) {
> -		DP_ERR(edev, "Can't open firmware file\n");
> -		return -ENOENT;
> -	}
> -
> -	if (fstat(fd, &st) < 0) {
> -		DP_ERR(edev, "Can't stat firmware file\n");
> -		close(fd);
> +	if (rte_firmware_read(qede_fw_file, &buf, &bufsz) < 0) {
> +		DP_ERR(edev, "Can't read firmware data: %s\n", qede_fw_file);
>  		return -1;
>  	}
>  
> -	edev->firmware = rte_zmalloc("qede_fw", st.st_size,
> -				    RTE_CACHE_LINE_SIZE);
> +	edev->firmware = rte_zmalloc("qede_fw", bufsz, RTE_CACHE_LINE_SIZE);
>  	if (!edev->firmware) {
>  		DP_ERR(edev, "Can't allocate memory for firmware\n");
> -		close(fd);
> -		return -ENOMEM;
> +		ret = -ENOMEM;
> +		goto out;
>  	}
>  
> -	if (read(fd, edev->firmware, st.st_size) != st.st_size) {
> -		DP_ERR(edev, "Can't read firmware data\n");
> -		close(fd);
> -		return -1;
> -	}
> -
> -	edev->fw_len = st.st_size;
> +	memcpy(edev->firmware, buf, bufsz);
> +	edev->fw_len = bufsz;
>  	if (edev->fw_len < 104) {
>  		DP_ERR(edev, "Invalid fw size: %" PRIu64 "\n",
>  			  edev->fw_len);
> -		close(fd);
> -		return -EINVAL;
> +		ret = -EINVAL;
> +		goto out;
>  	}
> -
> -	close(fd);
> -	return 0;
> +	ret = 0;
> +out:
> +	free(buf);
> +	return ret;
>  }
>  #endif
>  
> diff --git a/lib/eal/include/rte_firmware.h b/lib/eal/include/rte_firmware.h
> new file mode 100644
> index 0000000000..3389e60ca0
> --- /dev/null
> +++ b/lib/eal/include/rte_firmware.h
> @@ -0,0 +1,32 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2021 Red Hat, Inc.
> + */
> +
> +#ifndef __RTE_FIRMWARE_H__
> +#define __RTE_FIRMWARE_H__
> +
> +#include <sys/types.h>
> +
> +#include <rte_compat.h>
> +
> +/**
> + * Load a firmware in a dynamically allocated buffer, dealing with compressed
> + * files if libarchive is available.
> + *
> + * @param[in] name
> + *      Firmware filename to load.
> + * @param[out] buf
> + *      Buffer allocated by this function. If this function succeeds, the
> + *      caller is responsible for calling free() on this buffer.
> + * @param[out] bufsz
> + *      Size of the data in the buffer.
> + *
> + * @return
> + *      0 if successful.
> + *      Negative otherwise, buf and bufsize contents are invalid.
> + */
> +__rte_internal
> +int
> +rte_firmware_read(const char *name, void **buf, size_t *bufsz);
> +
> +#endif
> diff --git a/lib/eal/unix/eal_firmware.c b/lib/eal/unix/eal_firmware.c
> new file mode 100644
> index 0000000000..494cd5f058
> --- /dev/null
> +++ b/lib/eal/unix/eal_firmware.c
> @@ -0,0 +1,149 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2021 Red Hat, Inc.
> + */
> +
> +#ifdef RTE_HAS_LIBARCHIVE
> +#include <archive.h>
> +#endif
> +#include <fcntl.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +
> +#include <rte_common.h>
> +#include <rte_firmware.h>
> +#include <rte_log.h>
> +
> +#ifdef RTE_HAS_LIBARCHIVE
> +
> +struct firmware_read_ctx {
> +	struct archive *a;
> +};
> +
> +static int
> +firmware_open(struct firmware_read_ctx *ctx, const char *name, size_t blocksize)
> +{
> +	struct archive_entry *e;
> +
> +	ctx->a = archive_read_new();
> +	if (ctx->a == NULL)
> +		return -1;
> +	if (archive_read_support_format_raw(ctx->a) != ARCHIVE_OK ||
> +			archive_read_support_filter_xz(ctx->a) != ARCHIVE_OK ||
> +			archive_read_open_filename(ctx->a, name, blocksize) != ARCHIVE_OK ||
> +			archive_read_next_header(ctx->a, &e) != ARCHIVE_OK) {
> +		archive_read_free(ctx->a);
> +		ctx->a = NULL;
> +		return -1;
> +	}
> +	return 0;
> +}
> +
> +static ssize_t
> +firmware_read_block(struct firmware_read_ctx *ctx, void *buf, size_t count)
> +{
> +	return archive_read_data(ctx->a, buf, count);
> +}
> +
> +static void
> +firmware_close(struct firmware_read_ctx *ctx)
> +{
> +	archive_read_free(ctx->a);
> +	ctx->a = NULL;
> +}
> +
> +#else /* !RTE_HAS_LIBARCHIVE */
> +
> +struct firmware_read_ctx {
> +	int fd;
> +};
> +
> +static int
> +firmware_open(struct firmware_read_ctx *ctx, const char *name,
> +	__rte_unused size_t blocksize)
> +{
> +	ctx->fd = open(name, O_RDONLY);
> +	if (ctx->fd < 0)
> +		return -1;
> +	return 0;
> +}
> +
> +static ssize_t
> +firmware_read_block(struct firmware_read_ctx *ctx, void *buf, size_t count)
> +{
> +	return read(ctx->fd, buf, count);
> +}
> +
> +static void
> +firmware_close(struct firmware_read_ctx *ctx)
> +{
> +	close(ctx->fd);
> +	ctx->fd = -1;
> +}
> +
> +#endif /* !RTE_HAS_LIBARCHIVE */
> +
> +static int
> +firmware_read(const char *name, void **buf, size_t *bufsz)
> +{
> +	const size_t blocksize = 4096;
> +	struct firmware_read_ctx ctx;
> +	int ret = -1;
> +	int err;
> +
> +	*buf = NULL;
> +	*bufsz = 0;
> +
> +	if (firmware_open(&ctx, name, blocksize) < 0)
> +		return -1;
> +
> +	do {
> +		void *tmp;
> +
> +		tmp = realloc(*buf, *bufsz + blocksize);
> +		if (tmp == NULL) {
> +			free(*buf);
> +			*buf = NULL;
> +			*bufsz = 0;
> +			goto out;
> +		}
> +		*buf = tmp;
> +
> +		err = firmware_read_block(&ctx, RTE_PTR_ADD(*buf, *bufsz), blocksize);
> +		if (err < 0) {
> +			free(*buf);
> +			*buf = NULL;
> +			*bufsz = 0;
> +			goto out;
> +		}
> +		*bufsz += err;
> +
> +	} while (err != 0);
> +
> +	ret = 0;
> +out:
> +	firmware_close(&ctx);
> +	return ret;
> +}
> +
> +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",

Maybe 'not linked' instead of 'not available'.  A user might decide to
install libarchive and then be confused that the error message still
pops up (without realizing they need to re-compile).

Otherwise,
Acked-by: Aaron Conole <aconole@redhat.com>

> +				path);
> +		}
> +#else
> +		ret = firmware_read(path, buf, bufsz);
> +#endif
> +	}
> +	return ret;
> +}
> diff --git a/lib/eal/unix/meson.build b/lib/eal/unix/meson.build
> index dc711b4240..e3ecd3e956 100644
> --- a/lib/eal/unix/meson.build
> +++ b/lib/eal/unix/meson.build
> @@ -5,5 +5,6 @@ sources += files(
>          'eal_file.c',
>          'eal_unix_memory.c',
>          'eal_unix_timer.c',
> +        'eal_firmware.c',
>          'rte_thread.c',
>  )
> diff --git a/lib/eal/version.map b/lib/eal/version.map
> index fe5c3dac98..2df65c6903 100644
> --- a/lib/eal/version.map
> +++ b/lib/eal/version.map
> @@ -428,6 +428,7 @@ EXPERIMENTAL {
>  INTERNAL {
>  	global:
>  
> +	rte_firmware_read;
>  	rte_mem_lock;
>  	rte_mem_map;
>  	rte_mem_page_size;
> diff --git a/lib/eal/windows/eal.c b/lib/eal/windows/eal.c
> index 8483f6b02c..6fe2bdd282 100644
> --- a/lib/eal/windows/eal.c
> +++ b/lib/eal/windows/eal.c
> @@ -21,6 +21,7 @@
>  #include <eal_private.h>
>  #include <rte_service_component.h>
>  #include <rte_vfio.h>
> +#include <rte_firmware.h>
>  
>  #include "eal_hugepages.h"
>  #include "eal_trace.h"
> @@ -465,3 +466,11 @@ rte_vfio_container_dma_unmap(__rte_unused int container_fd,
>  {
>  	return -1;
>  }
> +
> +int
> +rte_firmware_read(__rte_unused const char *name,
> +			__rte_unused void **buf,
> +			__rte_unused size_t *bufsz)
> +{
> +	return -1;
> +}
  

> -----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
  

On Mon, Jul 5, 2021 at 8:35 AM Wang, Haiyue <haiyue.wang@intel.com> wrote:
> > +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 ?

I don't see the need.
Is the behavior changed for net/ice with this patch?
  

> -----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 +++----------

For ice PMD:

Tested-by: Haiyue Wang <haiyue.wang@intel.com>

>  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
> 


> --
> 2.23.0