diff --git a/GNUmakefile b/GNUmakefile index d266fa99f..562717e7a 100644 --- a/GNUmakefile +++ b/GNUmakefile @@ -4,6 +4,7 @@ BUILDDIR ?= build BUILDTYPE ?= debugoptimized SANITIZE ?= none +COVERAGE ?= false V ?= 0 ifeq ($V,1) ninja_opts = --verbose @@ -19,6 +20,7 @@ all: $(BUILDDIR)/build.ninja .PHONY: debug debug: BUILDTYPE = debug debug: SANITIZE = address +debug: COVERAGE = true debug: all .PHONY: unit-tests @@ -42,8 +44,11 @@ update-graph: all coverage: $Q mkdir -p $(BUILDDIR)/coverage $Q gcovr --html-details $(BUILDDIR)/coverage/index.html --txt \ - -e '.*_test.c' -e 'subprojects/*' --gcov-ignore-parse-errors \ - -ur . $(BUILDDIR) + -e '.*_test.c' -e 'subprojects/.*' --gcov-ignore-parse-errors \ + --gcov-executable `$(CC) -print-prog-name=gcov` \ + --object-directory $(BUILDDIR) \ + --sort uncovered-percent \ + -r . $(BUILDDIR) @echo Coverage data is present in $(BUILDDIR)/coverage/index.html .PHONY: all @@ -55,7 +60,7 @@ install: $(BUILDDIR)/build.ninja $Q meson install -C $(BUILDDIR) --skip-subprojects meson_opts = --buildtype=$(BUILDTYPE) --werror --warnlevel=2 -meson_opts += -Db_sanitize=$(SANITIZE) -Ddpdk_static=true +meson_opts += -Db_sanitize=$(SANITIZE) -Db_coverage=$(COVERAGE) -Ddpdk_static=true meson_opts += $(MESON_EXTRA_OPTS) $(BUILDDIR)/build.ninja: diff --git a/meson.build b/meson.build index f5ec3efd6..98318ad4f 100644 --- a/meson.build +++ b/meson.build @@ -132,19 +132,13 @@ install_headers(api_headers) cmocka_dep = dependency('cmocka', required: get_option('tests')) if cmocka_dep.found() fs = import('fs') - coverage_c_args = [] - coverage_link_args = [] - if compiler.get_id() == 'gcc' - coverage_c_args += ['-coverage'] - coverage_link_args += ['-lgcov'] - endif foreach t : tests name = fs.replace_suffix(t['sources'].get(0), '').underscorify() t += { 'sources': t['sources'] + files('api/string.c'), 'include_directories': inc + api_inc, - 'c_args': ['-D__GROUT_MAIN__', '-D__GROUT_UNIT_TEST__'] + coverage_c_args, - 'link_args': t['link_args'] + coverage_link_args, + 'c_args': ['-D__GROUT_MAIN__', '-D__GROUT_UNIT_TEST__'], + 'link_args': t['link_args'], 'dependencies': [dpdk_dep, ev_core_dep, ev_thread_dep, numa_dep, ecoli_dep, cmocka_dep], } test(name, executable(name, kwargs: t), suite: 'unit') diff --git a/modules/infra/api/gr_infra.h b/modules/infra/api/gr_infra.h index cddb5a80d..5786e0f26 100644 --- a/modules/infra/api/gr_infra.h +++ b/modules/infra/api/gr_infra.h @@ -35,6 +35,8 @@ typedef enum : uint16_t { // Interface state flags typedef enum : uint16_t { GR_IFACE_S_RUNNING = GR_BIT16(0), + GR_IFACE_S_PROMISC_FIXED = GR_BIT16(1), + GR_IFACE_S_ALLMULTI_FIXED = GR_BIT16(2), } gr_iface_state_t; // Interface reconfig attributes diff --git a/modules/infra/cli/iface.c b/modules/infra/cli/iface.c index 66c79f4c0..8c6b78410 100644 --- a/modules/infra/cli/iface.c +++ b/modules/infra/cli/iface.c @@ -124,9 +124,13 @@ static ssize_t iface_flags_format(char *buf, size_t len, const struct gr_iface * SAFE_BUF(snprintf, len, "down"); if (iface->state & GR_IFACE_S_RUNNING) SAFE_BUF(snprintf, len, " running"); - if (iface->flags & GR_IFACE_F_PROMISC) + if (iface->state & GR_IFACE_S_PROMISC_FIXED) + SAFE_BUF(snprintf, len, " promisc(fixed)"); + else if (iface->flags & GR_IFACE_F_PROMISC) SAFE_BUF(snprintf, len, " promisc"); - if (iface->flags & GR_IFACE_F_ALLMULTI) + if (iface->state & GR_IFACE_S_ALLMULTI_FIXED) + SAFE_BUF(snprintf, len, " allmulti(fixed)"); + else if (iface->flags & GR_IFACE_F_ALLMULTI) SAFE_BUF(snprintf, len, " allmulti"); if (iface->flags & GR_IFACE_F_PACKET_TRACE) SAFE_BUF(snprintf, len, " tracing"); diff --git a/modules/infra/control/meson.build b/modules/infra/control/meson.build index 2ccca3e3a..08ec8538a 100644 --- a/modules/infra/control/meson.build +++ b/modules/infra/control/meson.build @@ -41,5 +41,19 @@ tests += [ '-Wl,--wrap=rte_pktmbuf_pool_create', '-Wl,--wrap=rte_zmalloc', ], - } + }, + { + 'sources': files('port_test.c', 'port.c'), + 'link_args': [ + '-Wl,--wrap=rte_eth_allmulticast_disable', + '-Wl,--wrap=rte_eth_allmulticast_enable', + '-Wl,--wrap=rte_eth_allmulticast_get', + '-Wl,--wrap=rte_eth_dev_mac_addr_add', + '-Wl,--wrap=rte_eth_dev_mac_addr_remove', + '-Wl,--wrap=rte_eth_dev_set_mc_addr_list', + '-Wl,--wrap=rte_eth_promiscuous_disable', + '-Wl,--wrap=rte_eth_promiscuous_enable', + '-Wl,--wrap=rte_eth_promiscuous_get', + ], + }, ] diff --git a/modules/infra/control/port.c b/modules/infra/control/port.c index 36901766d..295b59a90 100644 --- a/modules/infra/control/port.c +++ b/modules/infra/control/port.c @@ -1,6 +1,7 @@ // SPDX-License-Identifier: BSD-3-Clause // Copyright (c) 2023 Robin Jarry +#include "port_priv.h" #include "worker_priv.h" #include @@ -194,10 +195,13 @@ static int port_mac_set(struct iface *iface, const struct rte_ether_addr *mac) { return 0; } -static int port_promisc_set(struct iface *iface, bool enabled) { +int port_promisc_set(struct iface *iface, bool enabled) { struct iface_info_port *p = iface_info_port(iface); int ret; + if (iface->state & GR_IFACE_S_PROMISC_FIXED) + return 0; // promisc is forced to filter unicast addresses, leave it as-is + if (enabled) ret = rte_eth_promiscuous_enable(p->port_id); else @@ -220,10 +224,13 @@ static int port_promisc_set(struct iface *iface, bool enabled) { return 0; } -static int port_allmulti_set(struct iface *iface, bool enabled) { +int port_allmulti_set(struct iface *iface, bool enabled) { struct iface_info_port *p = iface_info_port(iface); int ret; + if (iface->state & GR_IFACE_S_ALLMULTI_FIXED) + return 0; // allmulti is forced to filter multicast addresses, leave it as-is + if (enabled) ret = rte_eth_allmulticast_enable(p->port_id); else @@ -496,7 +503,7 @@ static int port_mac_get(const struct iface *iface, struct rte_ether_addr *mac) { return 0; } -static int port_mac_add(struct iface *iface, const struct rte_ether_addr *mac) { +int port_mac_add(struct iface *iface, const struct rte_ether_addr *mac) { struct iface_info_port *port = iface_info_port(iface); struct mac_filter *filter; const char *mac_type; @@ -566,8 +573,12 @@ static int port_mac_add(struct iface *iface, const struct rte_ether_addr *mac) { else ret = rte_eth_promiscuous_enable(port->port_id); - if (ret == 0) + if (ret == 0) { filter->flags |= MAC_FILTER_F_ALL; + iface->state |= multicast ? + GR_IFACE_S_ALLMULTI_FIXED : + GR_IFACE_S_PROMISC_FIXED; + } } if (ret < 0) { @@ -575,10 +586,22 @@ static int port_mac_add(struct iface *iface, const struct rte_ether_addr *mac) { return errno_log(-ret, mac_type); } + if (filter->flags & MAC_FILTER_F_ALL) { + // Purge the device from all explicit addresses. + // We will add them back when removing the forced allmulti/promisc. + // Ignore the return values. They don't matter. + if (multicast) { + rte_eth_dev_set_mc_addr_list(port->port_id, filter->mac, 0); + } else { + for (i = 0; i < filter->hw_limit; i++) + rte_eth_dev_mac_addr_remove(port->port_id, &filter->mac[i]); + } + } + return 0; } -static int port_mac_del(struct iface *iface, const struct rte_ether_addr *mac) { +int port_mac_del(struct iface *iface, const struct rte_ether_addr *mac) { struct iface_info_port *port = iface_info_port(iface); struct mac_filter *filter; const char *mac_type; @@ -648,15 +671,39 @@ static int port_mac_del(struct iface *iface, const struct rte_ether_addr *mac) { iface->name, multicast ? "allmulti" : "promisc", rte_strerror(-ret)); - } + else + iface->state &= multicast ? + ~GR_IFACE_S_ALLMULTI_FIXED : + ~GR_IFACE_S_PROMISC_FIXED; + + // Reinstall all addresses after disabling allmulti/promisc. + if (multicast) { + ret = rte_eth_dev_set_mc_addr_list( + port->port_id, filter->mac, filter->count + ); + } else { + for (i = 0; i < filter->count; i++) { + int r = rte_eth_dev_mac_addr_add(port->port_id, &filter->mac[i], 0); + if (r < 0 && ret == 0) + ret = r; + } + } + if (ret < 0) + LOG(WARNING, "%s: %s: %s", iface->name, mac_type, rte_strerror(-ret)); - if (multicast) - ret = rte_eth_dev_set_mc_addr_list(port->port_id, filter->mac, filter->count); - else - ret = rte_eth_dev_mac_addr_remove(port->port_id, (struct rte_ether_addr *)mac); + } else { + if (multicast) + ret = rte_eth_dev_set_mc_addr_list( + port->port_id, filter->mac, filter->count + ); + else + ret = rte_eth_dev_mac_addr_remove( + port->port_id, (struct rte_ether_addr *)mac + ); - if (ret < 0) - LOG(WARNING, "%s: %s: %s", iface->name, mac_type, rte_strerror(-ret)); + if (ret < 0) + LOG(WARNING, "%s: %s: %s", iface->name, mac_type, rte_strerror(-ret)); + } return 0; } diff --git a/modules/infra/control/port_priv.h b/modules/infra/control/port_priv.h new file mode 100644 index 000000000..30ebf2e44 --- /dev/null +++ b/modules/infra/control/port_priv.h @@ -0,0 +1,13 @@ +// SPDX-License-Identifier: BSD-3-Clause +// Copyright (c) 2025 Robin Jarry + +#pragma once + +#include + +#include + +int port_mac_add(struct iface *, const struct rte_ether_addr *); +int port_mac_del(struct iface *, const struct rte_ether_addr *); +int port_promisc_set(struct iface *, bool enabled); +int port_allmulti_set(struct iface *, bool enabled); diff --git a/modules/infra/control/port_test.c b/modules/infra/control/port_test.c new file mode 100644 index 000000000..c8d0ff5e1 --- /dev/null +++ b/modules/infra/control/port_test.c @@ -0,0 +1,192 @@ +// SPDX-License-Identifier: BSD-3-Clause +// Copyright (c) 2024 Robin Jarry + +#include "port_priv.h" +#include "worker_priv.h" + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +// mocked types/functions +int gr_rte_log_type; +struct gr_config gr_config; +struct workers workers; +void gr_register_api_handler(struct gr_api_handler *) { } +void gr_register_module(struct gr_module *) { } +void iface_type_register(struct iface_type *) { } +void gr_event_push(uint32_t, const void *) { } +mock_func(struct rte_mempool *, gr_pktmbuf_pool_get(int8_t, uint32_t)); +void gr_pktmbuf_pool_release(struct rte_mempool *, uint32_t) { } +struct rte_rcu_qsbr *gr_datapath_rcu(void) { + static struct rte_rcu_qsbr rcu; + return &rcu; +} +mock_func(struct iface *, iface_from_id(uint16_t)); +mock_func(struct iface *, iface_next(gr_iface_type_t, const struct iface *)); +mock_func(int, port_unplug(struct iface_info_port *)); +mock_func(int, port_plug(struct iface_info_port *)); +mock_func(unsigned, worker_count(void)); +mock_func(int, worker_queue_distribute(const cpu_set_t *, gr_vec struct iface_info_port **)); +mock_func(int, __wrap_rte_eth_allmulticast_disable(uint16_t)); +mock_func(int, __wrap_rte_eth_allmulticast_enable(uint16_t)); +mock_func(int, __wrap_rte_eth_allmulticast_get(uint16_t)); +mock_func(int, __wrap_rte_eth_dev_mac_addr_add(uint16_t, struct rte_ether_addr *, uint32_t)); +mock_func(int, __wrap_rte_eth_dev_mac_addr_remove(uint16_t, struct rte_ether_addr *)); +mock_func(int, __wrap_rte_eth_dev_set_mc_addr_list(uint16_t, struct rte_ether_addr *, uint32_t)); +mock_func(int, __wrap_rte_eth_promiscuous_disable(uint16_t)); +mock_func(int, __wrap_rte_eth_promiscuous_enable(uint16_t)); +mock_func(int, __wrap_rte_eth_promiscuous_get(uint16_t)); + +// test harness init +static const struct rte_ether_addr default_mac = {{0x02, 0xf0, 0x00, 0xb4, 0x47, 0x01}}; +static struct iface *iface; + +static int setup(void **) { + struct iface_info_port *port; + + iface = calloc(1, sizeof(*iface) + sizeof(*port)); + assert_non_null(iface); + iface->name = strdup("p0"); + iface->type = GR_IFACE_TYPE_PORT; + iface->flags = GR_IFACE_F_UP; + iface->state = GR_IFACE_S_RUNNING; + port = iface_info_port(iface); + port->started = true; + port->port_id = 42; + port->n_rxq = 1; + port->n_txq = 2; + port->mac = default_mac; + + return 0; +} + +static int teardown(void **) { + free(iface->name); + free(iface); + return 0; +} + +static const struct rte_ether_addr ucast1 = {{0x2c, 0x4c, 0x15, 0x07, 0x99, 0x22}}; +static const struct rte_ether_addr ucast2 = {{0x30, 0x3e, 0xa7, 0x0b, 0xea, 0x78}}; +static const struct rte_ether_addr ucast3 = {{0xe6, 0x2c, 0xd9, 0xa5, 0xe7, 0x6e}}; + +static void mac_add_unicast(void **) { + const struct iface_info_port *port = iface_info_port(iface); + + assert_int_equal(port_mac_add(iface, NULL), -EINVAL); + assert_return_code(port_mac_add(iface, &default_mac), errno); + assert_int_equal(port->ucast_filter.count, 0); + + will_return(__wrap_rte_eth_dev_mac_addr_add, 0); + assert_return_code(port_mac_add(iface, &ucast1), errno); + assert_int_equal(port->ucast_filter.count, 1); + assert_int_equal(port->ucast_filter.refcnt[0], 1); + + will_return(__wrap_rte_eth_dev_mac_addr_add, 0); + assert_return_code(port_mac_add(iface, &ucast2), errno); + assert_int_equal(port->ucast_filter.count, 2); + assert_int_equal(port->ucast_filter.refcnt[1], 1); + + assert_return_code(port_mac_add(iface, &ucast1), errno); + assert_int_equal(port->ucast_filter.count, 2); + assert_int_equal(port->ucast_filter.refcnt[0], 2); + + will_return(__wrap_rte_eth_dev_mac_addr_add, -ENOSPC); + will_return(__wrap_rte_eth_promiscuous_enable, 0); + will_return(__wrap_rte_eth_dev_mac_addr_remove, 0); + will_return(__wrap_rte_eth_dev_mac_addr_remove, 0); + assert_return_code(port_mac_add(iface, &ucast3), errno); + assert_true(port->ucast_filter.flags & MAC_FILTER_F_NOSPC); + assert_true(port->ucast_filter.flags & MAC_FILTER_F_ALL); + assert_true(iface->state & GR_IFACE_S_PROMISC_FIXED); + assert_int_equal(port->ucast_filter.count, 3); + assert_int_equal(port->ucast_filter.hw_limit, 2); +} + +static void mac_del_unicast(void **) { + const struct iface_info_port *port = iface_info_port(iface); + + assert_return_code(port_promisc_set(iface, false), errno); + assert_true(iface->state & GR_IFACE_S_PROMISC_FIXED); + + assert_return_code(port_mac_del(iface, &ucast1), errno); + assert_true(iface->state & GR_IFACE_S_PROMISC_FIXED); + + will_return(__wrap_rte_eth_promiscuous_disable, 0); + will_return(__wrap_rte_eth_dev_mac_addr_add, 0); + will_return(__wrap_rte_eth_dev_mac_addr_add, 0); + assert_return_code(port_mac_del(iface, &ucast1), errno); + assert_false(iface->state & GR_IFACE_S_PROMISC_FIXED); + assert_int_equal(port->ucast_filter.count, 2); + assert_int_equal(port->ucast_filter.hw_limit, 0); + assert_memory_equal(&port->ucast_filter.mac[0], &ucast2, sizeof(ucast2)); + assert_memory_equal(&port->ucast_filter.mac[1], &ucast3, sizeof(ucast3)); +} + +static const struct rte_ether_addr mcast1 = {{0x33, 0x33, 0x00, 0x00, 0x00, 0x01}}; +static const struct rte_ether_addr mcast2 = {{0x33, 0x33, 0x00, 0x00, 0x00, 0xfb}}; +static const struct rte_ether_addr mcast3 = {{0x01, 0x80, 0xc2, 0x00, 0x00, 0x02}}; + +static void mac_add_multicast(void **) { + const struct iface_info_port *port = iface_info_port(iface); + + will_return(__wrap_rte_eth_dev_set_mc_addr_list, 0); + assert_return_code(port_mac_add(iface, &mcast1), errno); + assert_int_equal(port->mcast_filter.count, 1); + assert_int_equal(port->mcast_filter.refcnt[0], 1); + + will_return(__wrap_rte_eth_dev_set_mc_addr_list, 0); + assert_return_code(port_mac_add(iface, &mcast2), errno); + assert_int_equal(port->mcast_filter.count, 2); + assert_int_equal(port->mcast_filter.refcnt[1], 1); + + assert_return_code(port_mac_add(iface, &mcast1), errno); + assert_int_equal(port->mcast_filter.count, 2); + assert_int_equal(port->mcast_filter.refcnt[0], 2); + + will_return(__wrap_rte_eth_dev_set_mc_addr_list, -ENOSPC); + will_return(__wrap_rte_eth_allmulticast_enable, 0); + will_return(__wrap_rte_eth_dev_set_mc_addr_list, 0); + assert_return_code(port_mac_add(iface, &mcast3), errno); + assert_true(port->mcast_filter.flags & MAC_FILTER_F_NOSPC); + assert_true(port->mcast_filter.flags & MAC_FILTER_F_ALL); + assert_true(iface->state & GR_IFACE_S_ALLMULTI_FIXED); + assert_int_equal(port->mcast_filter.count, 3); + assert_int_equal(port->mcast_filter.hw_limit, 2); +} + +static void mac_del_multicast(void **) { + const struct iface_info_port *port = iface_info_port(iface); + + assert_return_code(port_allmulti_set(iface, false), errno); + assert_true(iface->state & GR_IFACE_S_ALLMULTI_FIXED); + + assert_return_code(port_mac_del(iface, &mcast1), errno); + assert_true(iface->state & GR_IFACE_S_ALLMULTI_FIXED); + + will_return(__wrap_rte_eth_allmulticast_disable, 0); + will_return(__wrap_rte_eth_dev_set_mc_addr_list, 0); + assert_return_code(port_mac_del(iface, &mcast1), errno); + assert_false(iface->state & GR_IFACE_S_ALLMULTI_FIXED); + assert_int_equal(port->mcast_filter.count, 2); + assert_int_equal(port->mcast_filter.hw_limit, 0); + assert_memory_equal(&port->mcast_filter.mac[0], &mcast2, sizeof(mcast2)); + assert_memory_equal(&port->mcast_filter.mac[1], &mcast3, sizeof(mcast3)); +} + +int main(void) { + const struct CMUnitTest tests[] = { + cmocka_unit_test(mac_add_unicast), + cmocka_unit_test(mac_del_unicast), + cmocka_unit_test(mac_add_multicast), + cmocka_unit_test(mac_del_multicast), + }; + return cmocka_run_group_tests(tests, setup, teardown); +}