snipped
> diff --git a/lib/librte_if_proxy/Makefile b/lib/librte_if_proxy/Makefile
> new file mode 100644
> index 000000000..43cb702a2
> --- /dev/null
> +++ b/lib/librte_if_proxy/Makefile
> @@ -0,0 +1,29 @@
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright(C) 2020 Marvell International Ltd.
> +
> +include $(RTE_SDK)/mk/rte.vars.mk
> +
> +# library name
> +LIB = librte_if_proxy.a
> +
> +CFLAGS += -DALLOW_EXPERIMENTAL_API
> +CFLAGS += -O3
> +CFLAGS += $(WERROR_FLAGS) -I$(SRCDIR)
> +LDLIBS += -lrte_eal -lrte_ethdev
> +
> +EXPORT_MAP := rte_if_proxy_version.map
> +
> +LIBABIVER := 1
> +
> +# all source are stored in SRCS-y
> +SRCS-$(CONFIG_RTE_LIBRTE_IF_PROXY) := if_proxy_common.c
> +
> +SYSDIR := $(patsubst "%app",%,$(CONFIG_RTE_EXEC_ENV))
> +include $(SRCDIR)/$(SYSDIR)/Makefile
> +
Should there be check `ifeq ($(CONFIG_RTE_LIBRTE_KNI),y)` and `ifeq
($(CONFIG_RTE_LIBRTE_TAP),y)`?
> +SRCS-$(CONFIG_RTE_LIBRTE_IF_PROXY) += $(addprefix $(SYSDIR)/,$(SRCS))
> +
> +# install this header file
> +SYMLINK-$(CONFIG_RTE_LIBRTE_IF_PROXY)-include := rte_if_proxy.h
> +
> +include $(RTE_SDK)/mk/rte.lib.mk
Snipped
> +
> +uint64_t rte_ifpx_events_available(void)
> +{
> + /* All events are supported on Linux. */
> + return (1ULL << RTE_IFPX_NUM_EVENTS) - 1;
Should we give the available from the used count?
> +}
> +
Snipped
> +
> +void rte_ifpx_callbacks_unregister(void)
> +{
> + rte_spinlock_lock(&ifpx_lock);
> + memset(&ifpx_callbacks.cbs, 0, sizeof(ifpx_callbacks.cbs));
What would happen to pending events, are agreeing to drop all?
> + rte_spinlock_unlock(&ifpx_lock);
> +}
> +
> +uint16_t rte_ifpx_proxy_get(uint16_t port_id)
> +{
> + if (port_id >= RTE_MAX_ETHPORTS)
> + return RTE_MAX_ETHPORTS;
In the init function, the default value is set with RTE_MAX_ETHPORTS. Will
there be a scenario port_id can be greater?
> +
> + return ifpx_ports[port_id];
> +}
> +
> +unsigned int rte_ifpx_port_get(uint16_t proxy_id,
> + uint16_t *ports, unsigned int num)
> +{
> + unsigned int p, cnt = 0;
> +
> + for (p = 0; p < RTE_DIM(ifpx_ports); ++p) {
> + if (ifpx_ports[p] == proxy_id && ifpx_ports[p] != p) {
> + ++cnt;
> + if (ports && num > 0) {
> + *ports++ = p;
> + --num;
> + }
> + }
> + }
Application can dynamically ports to DPDK. if this is correct, will this
require lock to make this thread safe?
> + return cnt;
> +}
> +
> +const struct rte_ifpx_info *rte_ifpx_info_get(uint16_t port_id)
> +{
> + struct ifpx_proxy_node *px;
> +
> + if (port_id >= RTE_MAX_ETHPORTS ||
> + ifpx_ports[port_id] == RTE_MAX_ETHPORTS)
> + return NULL;
> +
> + rte_spinlock_lock(&ifpx_lock);
> + TAILQ_FOREACH(px, &ifpx_proxies, elem) {
> + if (px->proxy_id == ifpx_ports[port_id])
> + break;
> + }
> + rte_spinlock_unlock(&ifpx_lock);
> + RTE_ASSERT(px && "Internal IF Proxy library error");
Can you help me understand the assert logic with const string?
> +
> + return &px->info;
> +}
> +
> +static
> +void queue_event(const struct rte_ifpx_event *ev, struct rte_ring *r)
> +{
> + struct rte_ifpx_event *e = malloc(sizeof(*ev));
Is there specific reason not to use rte_malloc?
> +
> + if (!e) {
> + IFPX_LOG(ERR, "Failed to allocate event!");
> + return;
> + }
> + RTE_ASSERT(r);
> +
> + *e = *ev;
> + rte_ring_sp_enqueue(r, e);
> +}
> +
> +void ifpx_notify_event(struct rte_ifpx_event *ev, struct ifpx_proxy_node *px)
> +{
> + struct ifpx_queue_node *q;
> + int done = 0;
> + uint16_t p, proxy_id;
> +
> + if (px) {
> + if (px->state & DEL_PENDING)
> + return;
> + proxy_id = px->proxy_id;
> + RTE_ASSERT(proxy_id != RTE_MAX_ETHPORTS);
> + px->state |= IN_USE;
> + } else
> + proxy_id = RTE_MAX_ETHPORTS;
> +
> + RTE_ASSERT(ev);
> + /* This function is expected to be called with a lock held. */
> + RTE_ASSERT(rte_spinlock_trylock(&ifpx_lock) == 0);
> +
> + if (ifpx_callbacks.funcs[ev->type].f_ptr) {
> + union cb_ptr_t cb = ifpx_callbacks.funcs[ev->type];
> +
> + /* Drop the lock for the time of callback call. */
> + rte_spinlock_unlock(&ifpx_lock);
> + if (px) {
> + for (p = 0; p < RTE_DIM(ifpx_ports); ++p) {
> + if (ifpx_ports[p] != proxy_id ||
> + ifpx_ports[p] == p)
> + continue;
> + ev->data.port_id = p;
> + done = cb.f_ptr(&ev->data) || done;
> + }
> + } else {
> + RTE_ASSERT(ev->type == RTE_IFPX_CFG_DONE);
> + done = cb.cfg_done();
> + }
> + rte_spinlock_lock(&ifpx_lock);
> + }
> + if (done)
> + goto exit;
> +
> + /* Event not "consumed" yet so try to notify via queues. */
Is there a chance when trying to use queues the events are consumed by method
above by listener?
> + TAILQ_FOREACH(q, &ifpx_queues, elem) {
> + if (px) {
> + for (p = 0; p < RTE_DIM(ifpx_ports); ++p) {
> + if (ifpx_ports[p] != proxy_id ||
> + ifpx_ports[p] == p)
> + continue;
> + /* Set the port_id - the remaining params
> should
> + * be filled before calling this function.
> + */
> + ev->data.port_id = p;
> + queue_event(ev, q->r);
> + }
> + } else
> + queue_event(ev, q->r);
> + }
> +exit:
> + if (px)
> + px->state &= ~IN_USE;
> +}
Snipped
> +
> +RTE_INIT(if_proxy_init)
> +{
> + unsigned int i;
Is IF_PROXY supported for vdev also?
> + for (i = 0; i < RTE_DIM(ifpx_ports); ++i)
> + ifpx_ports[i] = RTE_MAX_ETHPORTS;
> +
> + ifpx_log_type = rte_log_register("lib.if_proxy");
> + if (ifpx_log_type >= 0)
> + rte_log_set_level(ifpx_log_type, RTE_LOG_WARNING);
> +
> + if (ifpx_platform.init)
> + ifpx_platform.init();
> +}
Snipped
> +SRCS += if_proxy.c
> diff --git a/lib/librte_if_proxy/linux/if_proxy.c
> b/lib/librte_if_proxy/linux/if_proxy.c
> new file mode 100644
> index 000000000..bf851c096
> --- /dev/null
> +++ b/lib/librte_if_proxy/linux/if_proxy.c
> @@ -0,0 +1,552 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(C) 2020 Marvell International Ltd.
> + */
Assuming all the events are executed `if and only if` the current process if
Primary? If it is secondary for physical interface certain `rte_eth_api` will
fail. Can we have check the events are processed for primary only?
Snipped
> diff --git a/lib/librte_if_proxy/meson.build b/lib/librte_if_proxy/meson.build
> new file mode 100644
> index 000000000..f0c1a6e15
> --- /dev/null
> +++ b/lib/librte_if_proxy/meson.build
> @@ -0,0 +1,19 @@
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright(C) 2020 Marvell International Ltd.
> +
> +# Currently only implemented on Linux
> +if not is_linux
> + build = false
> + reason = 'only supported on linux'
> +endif
> +
> +version = 1
> +allow_experimental_apis = true
> +
> +deps += ['ethdev']
> +sources = files('if_proxy_common.c')
> +headers = files('rte_if_proxy.h')
Does the if_proxy have dependency on TAP and KNI. Should not we add check as `
if dpdk_conf.has('RTE_LIBRTE_KNI')` and ` if dpdk_conf.has('RTE_LIBRTE_TAP')`?
> +
> +if is_linux
> + sources += files('linux/if_proxy.c')
> +endif
Snipped
