On Wed, Jun 03, 2020 at 02:03:20AM +0300, Dmitry Kozlyuk wrote:
> Introduce OS-independent wrappers in order to support common EAL code
> on Unix and Windows:
>
> * eal_file_create: open an existing file.
> * eal_file_open: create a file or open it if exists.
> * eal_file_lock: lock or unlock an open file.
> * eal_file_truncate: enforce a given size for an open file.
>
> Implementation for Linux and FreeBSD is placed in "unix" subdirectory,
> which is intended for common code between the two. These thin wrappers
> require no special maintenance.
>
> Common code supporting multi-process doesn't use the new wrappers,
> because it is inherently Unix-specific and would impose excessive
> requirements on the wrappers.
>
> Signed-off-by: Dmitry Kozlyuk <dmitry.kozl...@gmail.com>
> ---
> MAINTAINERS | 3 +
> lib/librte_eal/common/eal_common_fbarray.c | 31 ++++-----
> lib/librte_eal/common/eal_private.h | 74 +++++++++++++++++++++
> lib/librte_eal/freebsd/Makefile | 4 ++
> lib/librte_eal/linux/Makefile | 4 ++
> lib/librte_eal/meson.build | 4 ++
> lib/librte_eal/unix/eal_file.c | 76 ++++++++++++++++++++++
> lib/librte_eal/unix/meson.build | 6 ++
> 8 files changed, 183 insertions(+), 19 deletions(-)
> create mode 100644 lib/librte_eal/unix/eal_file.c
> create mode 100644 lib/librte_eal/unix/meson.build
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d2b286701..1d9aff26d 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -323,6 +323,9 @@ FreeBSD UIO
> M: Bruce Richardson <bruce.richard...@intel.com>
> F: kernel/freebsd/nic_uio/
>
> +Unix shared files
> +F: lib/librte_eal/unix/
> +
> Windows support
> M: Harini Ramakrishnan <harini.ramakrish...@microsoft.com>
> M: Omar Cardona <ocard...@microsoft.com>
> diff --git a/lib/librte_eal/common/eal_common_fbarray.c
> b/lib/librte_eal/common/eal_common_fbarray.c
> index 4f8f1af73..81ce4bd42 100644
> --- a/lib/librte_eal/common/eal_common_fbarray.c
> +++ b/lib/librte_eal/common/eal_common_fbarray.c
> @@ -8,8 +8,8 @@
> #include <sys/mman.h>
> #include <stdint.h>
> #include <errno.h>
> -#include <sys/file.h>
> #include <string.h>
> +#include <unistd.h>
>
> #include <rte_common.h>
> #include <rte_log.h>
> @@ -85,10 +85,8 @@ resize_and_map(int fd, void *addr, size_t len)
> char path[PATH_MAX];
> void *map_addr;
>
> - if (ftruncate(fd, len)) {
> + if (eal_file_truncate(fd, len)) {
> RTE_LOG(ERR, EAL, "Cannot truncate %s\n", path);
> - /* pass errno up the chain */
> - rte_errno = errno;
> return -1;
> }
>
> @@ -772,15 +770,15 @@ rte_fbarray_init(struct rte_fbarray *arr, const char
> *name, unsigned int len,
> * and see if we succeed. If we don't, someone else is using it
> * already.
> */
> - fd = open(path, O_CREAT | O_RDWR, 0600);
> + fd = eal_file_create(path);
> if (fd < 0) {
> RTE_LOG(DEBUG, EAL, "%s(): couldn't open %s: %s\n",
> - __func__, path, strerror(errno));
> - rte_errno = errno;
> + __func__, path, rte_strerror(rte_errno));
> goto fail;
> - } else if (flock(fd, LOCK_EX | LOCK_NB)) {
> + } else if (eal_file_lock(
> + fd, EAL_FLOCK_EXCLUSIVE, EAL_FLOCK_RETURN)) {
> RTE_LOG(DEBUG, EAL, "%s(): couldn't lock %s: %s\n",
> - __func__, path, strerror(errno));
> + __func__, path, rte_strerror(rte_errno));
> rte_errno = EBUSY;
> goto fail;
> }
> @@ -789,10 +787,8 @@ rte_fbarray_init(struct rte_fbarray *arr, const char
> *name, unsigned int len,
> * still attach to it, but no other process could reinitialize
> * it.
> */
> - if (flock(fd, LOCK_SH | LOCK_NB)) {
> - rte_errno = errno;
> + if (eal_file_lock(fd, EAL_FLOCK_SHARED, EAL_FLOCK_RETURN))
> goto fail;
> - }
>
> if (resize_and_map(fd, data, mmap_len))
> goto fail;
> @@ -888,17 +884,14 @@ rte_fbarray_attach(struct rte_fbarray *arr)
>
> eal_get_fbarray_path(path, sizeof(path), arr->name);
>
> - fd = open(path, O_RDWR);
> + fd = eal_file_open(path, true);
> if (fd < 0) {
> - rte_errno = errno;
> goto fail;
> }
>
> /* lock the file, to let others know we're using it */
> - if (flock(fd, LOCK_SH | LOCK_NB)) {
> - rte_errno = errno;
> + if (eal_file_lock(fd, EAL_FLOCK_SHARED, EAL_FLOCK_RETURN))
> goto fail;
> - }
>
> if (resize_and_map(fd, data, mmap_len))
> goto fail;
> @@ -1025,7 +1018,7 @@ rte_fbarray_destroy(struct rte_fbarray *arr)
> * has been detached by all other processes
> */
> fd = tmp->fd;
> - if (flock(fd, LOCK_EX | LOCK_NB)) {
> + if (eal_file_lock(fd, EAL_FLOCK_EXCLUSIVE, EAL_FLOCK_RETURN)) {
> RTE_LOG(DEBUG, EAL, "Cannot destroy fbarray - another
> process is using it\n");
> rte_errno = EBUSY;
> ret = -1;
> @@ -1042,7 +1035,7 @@ rte_fbarray_destroy(struct rte_fbarray *arr)
> * we're still holding an exclusive lock, so drop it to
> * shared.
> */
> - flock(fd, LOCK_SH | LOCK_NB);
> + eal_file_lock(fd, EAL_FLOCK_SHARED, EAL_FLOCK_RETURN);
>
> ret = -1;
> goto out;
> diff --git a/lib/librte_eal/common/eal_private.h
> b/lib/librte_eal/common/eal_private.h
> index 869ce183a..727f26881 100644
> --- a/lib/librte_eal/common/eal_private.h
> +++ b/lib/librte_eal/common/eal_private.h
> @@ -420,4 +420,78 @@ eal_malloc_no_trace(const char *type, size_t size,
> unsigned int align);
>
> void eal_free_no_trace(void *addr);
>
> +/**
> + * Create a file or open it if exits.
> + *
> + * Newly created file is only accessible to the owner (0600 equivalent).
> + * Returned descriptor is always read/write.
> + *
> + * @param path
> + * Path to the file.
> + * @return
> + * Open file descriptor on success, (-1) on failure and rte_errno is set.
> + */
> +int
> +eal_file_create(const char *path);
> +
> +/**
> + * Open an existing file.
> + *
> + * @param path
> + * Path to the file.
> + * @param writable
> + * Whether to open file read/write or read-only.
> + * @return
> + * Open file descriptor on success, (-1) on failure and rte_errno is set.
> + */
> +int
> +eal_file_open(const char *path, bool writable);
> +
> +/** File locking operation. */
> +enum eal_flock_op {
> + EAL_FLOCK_SHARED, /**< Acquire a shared lock. */
> + EAL_FLOCK_EXCLUSIVE, /**< Acquire an exclusive lock. */
> + EAL_FLOCK_UNLOCK /**< Release a previously taken lock. */
> +};
> +
> +/** Behavior on file locking conflict. */
> +enum eal_flock_mode {
> + EAL_FLOCK_WAIT, /**< Wait until the file gets unlocked to lock it. */
> + EAL_FLOCK_RETURN /**< Return immediately if the file is locked. */
> +};
> +
> +/**
> + * Lock or unlock the file.
> + *
> + * On failure @code rte_errno @endcode is set to the error code
> + * specified by POSIX flock(3) description.
> + *
> + * @param fd
> + * Opened file descriptor.
> + * @param op
> + * Operation to perform.
> + * @param mode
> + * Behavior on conflict.
> + * @return
> + * 0 on success, (-1) on failure.
> + */
> +int
> +eal_file_lock(int fd, enum eal_flock_op op, enum eal_flock_mode mode);
> +
> +/**
> + * Truncate or extend the file to the specified size.
> + *
> + * On failure @code rte_errno @endcode is set to the error code
> + * specified by POSIX ftruncate(3) description.
> + *
> + * @param fd
> + * Opened file descriptor.
> + * @param size
> + * Desired file size.
> + * @return
> + * 0 on success, (-1) on failure.
> + */
> +int
> +eal_file_truncate(int fd, ssize_t size);
> +
> #endif /* _EAL_PRIVATE_H_ */
> diff --git a/lib/librte_eal/freebsd/Makefile b/lib/librte_eal/freebsd/Makefile
> index af95386d4..0f8741d96 100644
> --- a/lib/librte_eal/freebsd/Makefile
> +++ b/lib/librte_eal/freebsd/Makefile
> @@ -7,6 +7,7 @@ LIB = librte_eal.a
>
> ARCH_DIR ?= $(RTE_ARCH)
> VPATH += $(RTE_SDK)/lib/librte_eal/$(ARCH_DIR)
> +VPATH += $(RTE_SDK)/lib/librte_eal/unix
> VPATH += $(RTE_SDK)/lib/librte_eal/common
>
> CFLAGS += -I$(SRCDIR)/include
> @@ -74,6 +75,9 @@ SRCS-$(CONFIG_RTE_EXEC_ENV_FREEBSD) += rte_service.c
> SRCS-$(CONFIG_RTE_EXEC_ENV_FREEBSD) += rte_random.c
> SRCS-$(CONFIG_RTE_EXEC_ENV_FREEBSD) += rte_reciprocal.c
>
> +# from unix dir
> +SRCS-$(CONFIG_RTE_EXEC_ENV_FREEBSD) += eal_file.c
> +
> # from arch dir
> SRCS-$(CONFIG_RTE_EXEC_ENV_FREEBSD) += rte_cpuflags.c
> SRCS-$(CONFIG_RTE_EXEC_ENV_FREEBSD) += rte_hypervisor.c
> diff --git a/lib/librte_eal/linux/Makefile b/lib/librte_eal/linux/Makefile
> index 48cc34844..331489f99 100644
> --- a/lib/librte_eal/linux/Makefile
> +++ b/lib/librte_eal/linux/Makefile
> @@ -7,6 +7,7 @@ LIB = librte_eal.a
>
> ARCH_DIR ?= $(RTE_ARCH)
> VPATH += $(RTE_SDK)/lib/librte_eal/$(ARCH_DIR)
> +VPATH += $(RTE_SDK)/lib/librte_eal/unix
> VPATH += $(RTE_SDK)/lib/librte_eal/common
>
> CFLAGS += -I$(SRCDIR)/include
> @@ -81,6 +82,9 @@ SRCS-$(CONFIG_RTE_EXEC_ENV_LINUX) += rte_service.c
> SRCS-$(CONFIG_RTE_EXEC_ENV_LINUX) += rte_random.c
> SRCS-$(CONFIG_RTE_EXEC_ENV_LINUX) += rte_reciprocal.c
>
> +# from unix dir
> +SRCS-$(CONFIG_RTE_EXEC_ENV_LINUX) += eal_file.c
> +
> # from arch dir
> SRCS-$(CONFIG_RTE_EXEC_ENV_LINUX) += rte_cpuflags.c
> SRCS-$(CONFIG_RTE_EXEC_ENV_LINUX) += rte_hypervisor.c
> diff --git a/lib/librte_eal/meson.build b/lib/librte_eal/meson.build
> index e301f4558..8d492897d 100644
> --- a/lib/librte_eal/meson.build
> +++ b/lib/librte_eal/meson.build
> @@ -6,6 +6,10 @@ subdir('include')
>
> subdir('common')
>
> +if not is_windows
> + subdir('unix')
> +endif
> +
> dpdk_conf.set('RTE_EXEC_ENV_' + exec_env.to_upper(), 1)
> subdir(exec_env)
>
> diff --git a/lib/librte_eal/unix/eal_file.c b/lib/librte_eal/unix/eal_file.c
> new file mode 100644
> index 000000000..7b3ffa629
> --- /dev/null
> +++ b/lib/librte_eal/unix/eal_file.c
> @@ -0,0 +1,76 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2020 Dmitry Kozlyuk
> + */
> +
> +#include <sys/file.h>
> +#include <sys/mman.h>
> +#include <unistd.h>
> +
> +#include <rte_errno.h>
> +
> +#include "eal_private.h"
> +
> +int
> +eal_file_create(const char *path)
> +{
> + int ret;
> +
> + ret = open(path, O_CREAT | O_RDWR, 0600);
> + if (ret < 0)
> + rte_errno = errno;
> +
> + return ret;
> +}
> +
You don't need this call if you support the oflags option in the open call
below.
> +int
> +eal_file_open(const char *path, bool writable)
> +{
> + int ret, flags;
> +
> + flags = writable ? O_RDWR : O_RDONLY;
> + ret = open(path, flags);
> + if (ret < 0)
> + rte_errno = errno;
> +
> + return ret;
> +}
> +
why are you changing this api from the posix file format (with oflags
specified). As far as I can see both unix and windows platforms support that
> +int
> +eal_file_truncate(int fd, ssize_t size)
> +{
> + int ret;
> +
> + ret = ftruncate(fd, size);
> + if (ret)
> + rte_errno = errno;
> +
> + return ret;
> +}
> +
> +int
> +eal_file_lock(int fd, enum eal_flock_op op, enum eal_flock_mode mode)
> +{
> + int sys_flags = 0;
> + int ret;
> +
> + if (mode == EAL_FLOCK_RETURN)
> + sys_flags |= LOCK_NB;
> +
> + switch (op) {
> + case EAL_FLOCK_EXCLUSIVE:
> + sys_flags |= LOCK_EX;
> + break;
> + case EAL_FLOCK_SHARED:
> + sys_flags |= LOCK_SH;
> + break;
> + case EAL_FLOCK_UNLOCK:
> + sys_flags |= LOCK_UN;
> + break;
> + }
> +
> + ret = flock(fd, sys_flags);
> + if (ret)
> + rte_errno = errno;
> +
> + return ret;
> +}
> diff --git a/lib/librte_eal/unix/meson.build b/lib/librte_eal/unix/meson.build
> new file mode 100644
> index 000000000..21029ba1a
> --- /dev/null
> +++ b/lib/librte_eal/unix/meson.build
> @@ -0,0 +1,6 @@
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright(c) 2020 Dmitry Kozlyuk
> +
> +sources += files(
> + 'eal_file.c',
> +)
> --
> 2.25.4
>
>
