diff --git a/modules/infra/api/gr_infra.h b/modules/infra/api/gr_infra.h index ee1bb861e..f0895d65a 100644 --- a/modules/infra/api/gr_infra.h +++ b/modules/infra/api/gr_infra.h @@ -248,6 +248,10 @@ enum gr_infra_requests : uint32_t { GR_PACKET_TRACE_SET, GR_AFFINITY_CPU_GET, GR_AFFINITY_CPU_SET, + GR_IFACE_MAC_ADD, + GR_IFACE_MAC_DEL, + GR_IFACE_MAC_LIST, + GR_IFACE_MAC_SET, }; enum gr_infra_events : uint32_t { @@ -312,6 +316,44 @@ struct gr_iface_list_req { GR_REQ_STREAM(GR_IFACE_LIST, struct gr_iface_list_req, struct gr_iface); +// Add a secondary MAC address to an interface. +struct gr_iface_mac_add_req { + uint16_t iface_id; + struct rte_ether_addr mac; +}; + +GR_REQ(GR_IFACE_MAC_ADD, struct gr_iface_mac_add_req, struct gr_empty); + +// Remove a secondary MAC address from an interface. +struct gr_iface_mac_del_req { + uint16_t iface_id; + struct rte_ether_addr mac; +}; + +GR_REQ(GR_IFACE_MAC_DEL, struct gr_iface_mac_del_req, struct gr_empty); + +// List MAC addresses on an interface. +struct gr_iface_mac_list_req { + uint16_t iface_id; // GR_IFACE_ID_UNDEF for all interfaces. +}; + +struct gr_iface_mac { + uint16_t iface_id; + uint16_t refcnt; + bool primary; + struct rte_ether_addr mac; +}; + +GR_REQ_STREAM(GR_IFACE_MAC_LIST, struct gr_iface_mac_list_req, struct gr_iface_mac); + +// Set the primary MAC address of an interface. +struct gr_iface_mac_set_req { + uint16_t iface_id; + struct rte_ether_addr mac; +}; + +GR_REQ(GR_IFACE_MAC_SET, struct gr_iface_mac_set_req, struct gr_empty); + // Modify an existing interface. // MTU changes on parent interfaces propagate to VLAN sub-interfaces. struct gr_iface_set_req { diff --git a/modules/infra/api/iface.c b/modules/infra/api/iface.c index 45e27bac2..9d25f4e69 100644 --- a/modules/infra/api/iface.c +++ b/modules/infra/api/iface.c @@ -109,6 +109,87 @@ static struct api_out iface_list(const void *request, struct api_ctx *ctx) { return api_out(ret, 0, NULL); } +static struct api_out iface_mac_add(const void *request, struct api_ctx *) { + const struct gr_iface_mac_add_req *req = request; + struct iface *iface; + int ret; + + if ((iface = iface_from_id(req->iface_id)) == NULL) + return api_out(ENODEV, 0, NULL); + + ret = iface_add_eth_addr(iface, &req->mac); + if (ret < 0) + return api_out(errno, 0, NULL); + + return api_out(0, 0, NULL); +} + +static struct api_out iface_mac_del(const void *request, struct api_ctx *) { + const struct gr_iface_mac_del_req *req = request; + struct iface *iface; + int ret; + + if ((iface = iface_from_id(req->iface_id)) == NULL) + return api_out(ENODEV, 0, NULL); + + ret = iface_del_eth_addr(iface, &req->mac); + if (ret < 0) + return api_out(errno, 0, NULL); + + return api_out(0, 0, NULL); +} + +static struct api_out iface_mac_list(const void *request, struct api_ctx *ctx) { + const struct gr_iface_mac_list_req *req = request; + const struct iface *iface = NULL; + int ret = 0; + + while ((iface = iface_next(GR_IFACE_TYPE_UNDEF, iface)) != NULL) { + if (req->iface_id != GR_IFACE_ID_UNDEF && iface->id != req->iface_id) + continue; + + // Send primary MAC address first + struct rte_ether_addr primary_mac; + if (iface_get_eth_addr(iface, &primary_mac) == 0) { + struct gr_iface_mac mac; + + mac.iface_id = iface->id; + mac.refcnt = 0; + mac.primary = true; + mac.mac = primary_mac; + api_send(ctx, sizeof(mac), &mac); + } + + // Send secondary MAC addresses + vec_foreach_ref (struct iface_mac *m, iface->macs) { + struct gr_iface_mac mac; + + mac.iface_id = iface->id; + mac.refcnt = m->refcnt; + mac.primary = false; + mac.mac = m->mac; + api_send(ctx, sizeof(mac), &mac); + } + } + + return api_out(ret, 0, NULL); +} + +static struct api_out iface_mac_set(const void *request, struct api_ctx *) { + const struct gr_iface_mac_set_req *req = request; + struct iface *iface; + int ret; + + if ((iface = iface_from_id(req->iface_id)) == NULL) + return api_out(ENODEV, 0, NULL); + + ret = iface_set_eth_addr(iface, &req->mac); + if (ret < 0) + return api_out(errno, 0, NULL); + + return api_out(0, 0, NULL); +} + static struct api_out iface_set(const void *request, struct api_ctx *) { const struct gr_iface_set_req *req = request; int ret; @@ -232,6 +313,10 @@ RTE_INIT(infra_api_init) { api_handler(GR_IFACE_DEL, iface_del); api_handler(GR_IFACE_GET, iface_get); api_handler(GR_IFACE_LIST, iface_list); + api_handler(GR_IFACE_MAC_ADD, iface_mac_add); + api_handler(GR_IFACE_MAC_DEL, iface_mac_del); + api_handler(GR_IFACE_MAC_LIST, iface_mac_list); + api_handler(GR_IFACE_MAC_SET, iface_mac_set); api_handler(GR_IFACE_SET, iface_set); event_serializer(GR_EVENT_IFACE_ADD, iface_event_serialize); event_serializer(GR_EVENT_IFACE_POST_ADD, iface_event_serialize); diff --git a/modules/infra/cli/mac.c b/modules/infra/cli/mac.c new file mode 100644 index 000000000..0f0467f50 --- /dev/null +++ b/modules/infra/cli/mac.c @@ -0,0 +1,162 @@ +// SPDX-License-Identifier: BSD-3-Clause +// Copyright (c) 2026 David Marchand + +#include "cli.h" +#include "cli_iface.h" +#include "display.h" + +#include +#include +#include + +#include + +#include + +static cmd_status_t mac_add(struct gr_api_client *c, const struct ec_pnode *p) { + struct gr_iface_mac_add_req req = {0}; + + if (arg_iface(c, p, "IFACE", GR_IFACE_TYPE_UNDEF, &req.iface_id) < 0) + return CMD_ERROR; + + if (arg_eth_addr(p, "MAC", &req.mac) < 0) + return CMD_ERROR; + + if (gr_api_client_send_recv(c, GR_IFACE_MAC_ADD, sizeof(req), &req, NULL) < 0) + return CMD_ERROR; + + return CMD_SUCCESS; +} + +static cmd_status_t mac_del(struct gr_api_client *c, const struct ec_pnode *p) { + struct gr_iface_mac_del_req req = {0}; + + if (arg_iface(c, p, "IFACE", GR_IFACE_TYPE_UNDEF, &req.iface_id) < 0) + return CMD_ERROR; + + if (arg_eth_addr(p, "MAC", &req.mac) < 0) + return CMD_ERROR; + + if (gr_api_client_send_recv(c, GR_IFACE_MAC_DEL, sizeof(req), &req, NULL) < 0) + return CMD_ERROR; + + return CMD_SUCCESS; +} + +static cmd_status_t mac_list(struct gr_api_client *c, const struct ec_pnode *p) { + struct gr_iface_mac_list_req req = {.iface_id = GR_IFACE_ID_UNDEF}; + const struct gr_iface_mac *mac; + int ret; + + if (arg_str(p, "IFACE") != NULL) { + if (arg_iface(c, p, "IFACE", GR_IFACE_TYPE_UNDEF, &req.iface_id) < 0) + return CMD_ERROR; + } + + struct gr_table *table = gr_table_new(); + gr_table_column(table, "IFACE", GR_DISP_LEFT); + gr_table_column(table, "MAC", GR_DISP_LEFT); + gr_table_column(table, "REFCNT", GR_DISP_RIGHT | GR_DISP_INT); + + gr_api_client_stream_foreach (mac, ret, c, GR_IFACE_MAC_LIST, sizeof(req), &req) { + gr_table_cell(table, 0, "%s", iface_name_from_id(c, mac->iface_id)); + gr_table_cell(table, 1, ETH_F, &mac->mac); + if (mac->primary) + gr_table_cell(table, 2, "-"); + else + gr_table_cell(table, 2, "%u", mac->refcnt); + if (gr_table_print_row(table) < 0) + break; + } + + gr_table_free(table); + + return ret < 0 ? CMD_ERROR : CMD_SUCCESS; +} + +static cmd_status_t mac_set(struct gr_api_client *c, const struct ec_pnode *p) { + struct gr_iface_mac_set_req req = {0}; + + if (arg_iface(c, p, "IFACE", GR_IFACE_TYPE_UNDEF, &req.iface_id) < 0) + return CMD_ERROR; + + if (arg_eth_addr(p, "MAC", &req.mac) < 0) + return CMD_ERROR; + + if (gr_api_client_send_recv(c, GR_IFACE_MAC_SET, sizeof(req), &req, NULL) < 0) + return CMD_ERROR; + + return CMD_SUCCESS; +} + +#define MAC_CTX(root) CLI_CONTEXT(root, CTX_ARG("mac", "MAC address management.")) + +static int ctx_init(struct ec_node *root) { + int ret; + + ret = CLI_COMMAND( + MAC_CTX(root), + "add MAC iface IFACE", + mac_add, + "Add a secondary MAC address to an interface.", + with_help("MAC address.", ec_node_re("MAC", ETH_ADDR_RE)), + with_help( + "Interface name.", + ec_node_dyn("IFACE", complete_iface_names, INT2PTR(GR_IFACE_TYPE_UNDEF)) + ) + ); + if (ret < 0) + return ret; + + ret = CLI_COMMAND( + MAC_CTX(root), + "del MAC iface IFACE", + mac_del, + "Remove a secondary MAC address from an interface.", + with_help("MAC address.", ec_node_re("MAC", ETH_ADDR_RE)), + with_help( + "Interface name.", + ec_node_dyn("IFACE", complete_iface_names, INT2PTR(GR_IFACE_TYPE_UNDEF)) + ) + ); + if (ret < 0) + return ret; + + ret = CLI_COMMAND( + MAC_CTX(root), + "set MAC iface IFACE", + mac_set, + "Set the primary MAC address of an interface.", + with_help("MAC address.", ec_node_re("MAC", ETH_ADDR_RE)), + with_help( + "Interface name.", + ec_node_dyn("IFACE", complete_iface_names, INT2PTR(GR_IFACE_TYPE_UNDEF)) + ) + ); + if (ret < 0) + return ret; + + ret = CLI_COMMAND( + MAC_CTX(root), + "[show] [iface IFACE]", + mac_list, + "Display MAC addresses.", + with_help( + "Interface name.", + ec_node_dyn("IFACE", complete_iface_names, INT2PTR(GR_IFACE_TYPE_UNDEF)) + ) + ); + if (ret < 0) + return ret; + + return 0; +} + +static struct cli_context ctx = { + .name = "mac", + .init = ctx_init, +}; + +static void __attribute__((constructor, used)) init(void) { + cli_context_register(&ctx); +} diff --git a/modules/infra/cli/meson.build b/modules/infra/cli/meson.build index 911031c36..222f72196 100644 --- a/modules/infra/cli/meson.build +++ b/modules/infra/cli/meson.build @@ -9,6 +9,7 @@ cli_src += files( 'graph.c', 'icmp.c', 'iface.c', + 'mac.c', 'vrf.c', 'nexthop.c', 'port.c', diff --git a/modules/infra/control/bond.c b/modules/infra/control/bond.c index 668b37397..34a8c531a 100644 --- a/modules/infra/control/bond.c +++ b/modules/infra/control/bond.c @@ -40,32 +40,16 @@ bond_all_member_del_mac(const struct iface_info_bond *bond, const struct rte_eth return 0; } -static int bond_mac_add(struct iface *iface, const struct rte_ether_addr *mac) { +static int bond_mac_add(struct iface *iface, struct iface_mac *m) { struct iface_info_bond *bond = iface_info_bond(iface); - int ret; - - // Add MAC address to all member ports - if ((ret = bond_all_member_add_mac(bond, mac)) < 0) - return ret; - vec_add(bond->extra_macs, *mac); - - return 0; + return bond_all_member_add_mac(bond, &m->mac); } -static int bond_mac_del(struct iface *iface, const struct rte_ether_addr *mac) { +static int bond_mac_del(struct iface *iface, struct iface_mac *m) { struct iface_info_bond *bond = iface_info_bond(iface); - // Remove MAC address from all member ports - bond_all_member_del_mac(bond, mac); - - for (unsigned i = 0; i < vec_len(bond->extra_macs); i++) { - if (rte_is_same_ether_addr(&bond->extra_macs[i], mac)) { - vec_del(bond->extra_macs, i); - break; - } - } - + bond_all_member_del_mac(bond, &m->mac); return 0; } @@ -173,8 +157,8 @@ static int bond_attach_member(struct iface *iface, struct iface *member) { if (bond->n_members == ARRAY_DIM(bond->members)) return errno_set(EUSERS); - vec_foreach_ref (struct rte_ether_addr *mac, bond->extra_macs) { - if (iface_add_eth_addr(member, mac) < 0) + vec_foreach_ref (struct iface_mac *m, iface->macs) { + if (iface_add_eth_addr(member, &m->mac) < 0) return errno_log(errno, "iface_add_eth_addr(member)"); } @@ -223,8 +207,8 @@ static int bond_detach_member(struct iface *iface, struct iface *member) { bond->n_members--; if (bond->primary_member >= bond->n_members) bond->primary_member--; - vec_foreach_ref (struct rte_ether_addr *mac, bond->extra_macs) { - if (iface_del_eth_addr(member, mac) < 0 && errno != ENOENT) { + vec_foreach_ref (struct iface_mac *m, iface->macs) { + if (iface_del_eth_addr(member, &m->mac) < 0 && errno != ENOENT) { LOG(WARNING, "failed to unconfigure mac address on member %s: %s", member->name, @@ -412,8 +396,6 @@ static int bond_fini(struct iface *iface) { event_push(GR_EVENT_IFACE_POST_RECONFIG, member); } - vec_free(bond->extra_macs); - return 0; } diff --git a/modules/infra/control/bond.h b/modules/infra/control/bond.h index 4a800bb8d..371c5bad5 100644 --- a/modules/infra/control/bond.h +++ b/modules/infra/control/bond.h @@ -5,7 +5,6 @@ #include "iface.h" #include "lacp.h" -#include "vec.h" #include @@ -34,8 +33,6 @@ GR_IFACE_INFO(GR_IFACE_TYPE_BOND, iface_info_bond, { uint8_t n_members; struct bond_member members[MEMBERS_MAX_LEN]; - vec struct rte_ether_addr *extra_macs; - uint8_t redirection_table[256]; }); diff --git a/modules/infra/control/iface.c b/modules/infra/control/iface.c index f209067be..e712c9ed6 100644 --- a/modules/infra/control/iface.c +++ b/modules/infra/control/iface.c @@ -465,6 +465,13 @@ struct iface *iface_from_id(uint16_t ifid) { return iface; } +static bool iface_is_default_mac(struct iface *iface, const struct rte_ether_addr *mac) { + struct rte_ether_addr default_mac; + + return iface_get_eth_addr(iface, &default_mac) == 0 + && rte_is_same_ether_addr(&default_mac, mac); +} + int iface_get_eth_addr(const struct iface *iface, struct rte_ether_addr *mac) { const struct iface_type *type; @@ -517,30 +524,89 @@ int iface_set_eth_addr(struct iface *iface, const struct rte_ether_addr *mac) { int iface_add_eth_addr(struct iface *iface, const struct rte_ether_addr *mac) { const struct iface_type *type; + struct iface_mac new_m; + int ret; - if (iface == NULL) + if (iface == NULL || mac == NULL) return errno_set(EINVAL); + if (rte_is_multicast_ether_addr(mac)) + return 0; // ALLMULTI is always on + + if (iface_is_default_mac(iface, mac)) + return 0; + type = iface_type_get(iface->type); assert(type != NULL); if (type->add_eth_addr == NULL) return errno_set(EOPNOTSUPP); - return type->add_eth_addr(iface, mac); + vec_foreach_ref (struct iface_mac *m, iface->macs) { + if (!rte_is_same_ether_addr(mac, &m->mac)) + continue; + + if (m->refcnt == UINT_NUM_VALUES(m->refcnt) - 1) + return errno_set(EOVERFLOW); + + LOG(DEBUG, + "%s: mac " ETH_F " already filtered (refs=%u)", + iface->name, + mac, + m->refcnt); + m->refcnt++; + return 0; + } + + new_m = (struct iface_mac) {.refcnt = 1, .mac = *mac, .hardware = false}; + ret = type->add_eth_addr(iface, &new_m); + if (ret == 0) + vec_add(iface->macs, new_m); + return ret; } int iface_del_eth_addr(struct iface *iface, const struct rte_ether_addr *mac) { const struct iface_type *type; - if (iface == NULL) + if (iface == NULL || mac == NULL) return errno_set(EINVAL); + if (rte_is_multicast_ether_addr(mac)) + return 0; // ALLMULTI is always on + + if (iface_is_default_mac(iface, mac)) + return 0; + type = iface_type_get(iface->type); assert(type != NULL); if (type->del_eth_addr == NULL) return errno_set(EOPNOTSUPP); - return type->del_eth_addr(iface, mac); + vec_foreach_ref (struct iface_mac *m, iface->macs) { + int ret; + + if (!rte_is_same_ether_addr(mac, &m->mac)) + continue; + + if (m->refcnt == 0) + return errno_set(EOVERFLOW); + + if (m->refcnt > 1) { + m->refcnt--; + LOG(DEBUG, + "%s: mac " ETH_F " still filtered (refs=%u)", + iface->name, + mac, + m->refcnt); + ret = 0; + } else { + ret = type->del_eth_addr(iface, m); + vec_del(iface->macs, m - iface->macs); + } + + return ret; + } + + return errno_set(ENOENT); } int iface_set_mtu(struct iface *iface, uint16_t mtu) { @@ -658,6 +724,7 @@ int iface_destroy(struct iface *iface) { free(iface->name); free(iface->description); vec_free(iface->subinterfaces); + vec_free(iface->macs); rte_free(iface); return ret; diff --git a/modules/infra/control/iface.h b/modules/infra/control/iface.h index 337a1d090..37ed8ecb2 100644 --- a/modules/infra/control/iface.h +++ b/modules/infra/control/iface.h @@ -11,6 +11,12 @@ #include #include +struct iface_mac { + uint8_t refcnt; + bool hardware; + struct rte_ether_addr mac; +}; + struct __rte_cache_aligned iface { BASE(__gr_iface_base); @@ -22,6 +28,7 @@ struct __rte_cache_aligned iface { struct event *cp_ev; // libevent to poll cp_fd unsigned promisc; bool user_promisc; + vec struct iface_mac *macs; alignas(alignof(void *)) uint8_t info[/* size depends on type */]; }; @@ -50,8 +57,8 @@ struct iface_type { int (*detach_domain)(struct iface *domain, struct iface *iface); int (*get_eth_addr)(const struct iface *, struct rte_ether_addr *); int (*set_eth_addr)(struct iface *, const struct rte_ether_addr *); - int (*add_eth_addr)(struct iface *, const struct rte_ether_addr *); - int (*del_eth_addr)(struct iface *, const struct rte_ether_addr *); + int (*add_eth_addr)(struct iface *, struct iface_mac *); + int (*del_eth_addr)(struct iface *, struct iface_mac *); int (*set_up_down)(struct iface *, bool up); int (*set_mtu)(struct iface *, uint16_t mtu); int (*set_promisc)(struct iface *, bool enabled); diff --git a/modules/infra/control/iface_test.c b/modules/infra/control/iface_test.c new file mode 100644 index 000000000..5612764ec --- /dev/null +++ b/modules/infra/control/iface_test.c @@ -0,0 +1,273 @@ +// SPDX-License-Identifier: BSD-3-Clause +// Copyright (c) 2024 Robin Jarry + +#include "_cmocka.h" +#include "config.h" +#include "event.h" +#include "iface.h" +#include "log.h" +#include "mempool.h" +#include "module.h" +#include "netlink.h" +#include "port.h" +#include "rcu.h" +#include "vlan.h" +#include "vrf.h" +#include "worker.h" + +// mocked types/functions +int gr_rte_log_type; +struct log_types log_types = STAILQ_HEAD_INITIALIZER(log_types); +struct gr_config gr_config; +struct workers workers; +void module_register(struct module *) { } +void event_push(uint32_t, const void *) { } +void event_subscribe(uint32_t, event_sub_cb_t) { } +int netlink_link_set_name(uint32_t, const char *) { + return 0; +} +int netlink_set_ifalias(uint32_t, const char *) { + return 0; +} +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; +} +int vrf_incref(uint16_t) { + return 0; +} +void vrf_decref(uint16_t) { } +bool vrf_has_interfaces(uint16_t) { + return false; +} +void control_queue_drain(uint32_t, const void *) { } +mock_func(struct iface *, __wrap_iface_from_id(uint16_t)); +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 *, vec struct iface_info_port **)); +mock_func(int, __wrap_rte_eth_allmulticast_enable(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_promiscuous_disable(uint16_t)); +mock_func(int, __wrap_rte_eth_promiscuous_enable(uint16_t)); +mock_func(int, __wrap_rte_eth_promiscuous_get(uint16_t)); +void metrics_ctx_init(struct metrics_ctx *, struct metrics_writer *, ...) { } +void metrics_labels_add(struct metrics_ctx *, ...) { } +void metric_emit(struct metrics_ctx *, const struct metric *, uint64_t) { } + +// test harness init +static const struct rte_ether_addr default_mac = {{0x02, 0xf0, 0x00, 0xb4, 0x47, 0x01}}; +static struct iface *iface; +static const struct rte_ether_addr default_vlan_mac = {{0x02, 0xff, 0x00, 0xb4, 0x47, 0x01}}; +static struct iface *vlan_iface; + +static int setup(void **) { + struct iface_info_port *port; + struct iface_info_vlan *vlan; + + 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; + + vlan_iface = calloc(1, sizeof(*vlan_iface) + sizeof(*vlan)); + assert_non_null(vlan_iface); + vlan_iface->name = strdup("p0.23"); + vlan_iface->type = GR_IFACE_TYPE_VLAN; + vlan_iface->flags = GR_IFACE_F_UP; + vlan_iface->state = GR_IFACE_S_RUNNING; + vlan = iface_info_vlan(vlan_iface); + vlan->parent_id = 42; + vlan->vlan_id = 23; + vlan->mac = default_vlan_mac; + + return 0; +} + +static int teardown(void **) { + free(iface->name); + vec_free(iface->macs); + free(iface); + free(vlan_iface->name); + vec_free(vlan_iface->macs); + free(vlan_iface); + return 0; +} + +static const struct rte_ether_addr mcast1 = {{0x33, 0x33, 0x00, 0x00, 0x00, 0x01}}; +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 port_mac_add_multicast(void **) { + assert_return_code(iface_add_eth_addr(iface, &mcast1), errno); + assert_int_equal(vec_len(iface->macs), 0); +} + +static void port_mac_add_unicast(void **) { + assert_int_equal(iface_add_eth_addr(iface, NULL), -EINVAL); + + assert_return_code(iface_add_eth_addr(iface, &default_mac), errno); + assert_int_equal(vec_len(iface->macs), 0); + + will_return(__wrap_rte_eth_dev_mac_addr_add, 0); + assert_return_code(iface_add_eth_addr(iface, &ucast1), errno); + assert_false(iface->state & GR_IFACE_S_PROMISC_FIXED); + assert_int_equal(vec_len(iface->macs), 1); + assert_memory_equal(&iface->macs[0].mac, &ucast1, sizeof(ucast1)); + assert_int_equal(iface->macs[0].refcnt, 1); + assert_true(iface->macs[0].hardware); + + will_return(__wrap_rte_eth_dev_mac_addr_add, 0); + assert_return_code(iface_add_eth_addr(iface, &ucast2), errno); + assert_false(iface->state & GR_IFACE_S_PROMISC_FIXED); + assert_int_equal(vec_len(iface->macs), 2); + assert_memory_equal(&iface->macs[1].mac, &ucast2, sizeof(ucast2)); + assert_int_equal(iface->macs[1].refcnt, 1); + assert_true(iface->macs[1].hardware); + + assert_return_code(iface_add_eth_addr(iface, &ucast1), errno); + assert_int_equal(vec_len(iface->macs), 2); + assert_memory_equal(&iface->macs[0].mac, &ucast1, sizeof(ucast1)); + assert_int_equal(iface->macs[0].refcnt, 2); + assert_true(iface->macs[0].hardware); + + will_return(__wrap_rte_eth_dev_mac_addr_add, -ENOSPC); + will_return(__wrap_rte_eth_promiscuous_enable, 0); + assert_return_code(iface_add_eth_addr(iface, &ucast3), errno); + assert_true(iface->state & GR_IFACE_S_PROMISC_FIXED); + assert_int_equal(vec_len(iface->macs), 3); + assert_memory_equal(&iface->macs[2].mac, &ucast3, sizeof(ucast3)); + assert_int_equal(iface->macs[2].refcnt, 1); + assert_false(iface->macs[2].hardware); +} + +static void port_mac_del_unicast(void **) { + assert_return_code(iface_set_promisc(iface, false), errno); + assert_true(iface->state & GR_IFACE_S_PROMISC_FIXED); + + assert_return_code(iface_del_eth_addr(iface, &ucast1), errno); + assert_true(iface->state & GR_IFACE_S_PROMISC_FIXED); + assert_memory_equal(&iface->macs[0].mac, &ucast1, sizeof(ucast1)); + assert_int_equal(iface->macs[0].refcnt, 1); + assert_true(iface->macs[0].hardware); + + will_return(__wrap_rte_eth_dev_mac_addr_remove, 0); + will_return(__wrap_rte_eth_dev_mac_addr_add, 0); + will_return(__wrap_rte_eth_promiscuous_disable, 0); + assert_return_code(iface_del_eth_addr(iface, &ucast1), errno); + assert_false(iface->state & GR_IFACE_S_PROMISC_FIXED); + assert_int_equal(vec_len(iface->macs), 2); + assert_memory_equal(&iface->macs[0].mac, &ucast2, sizeof(ucast2)); + assert_int_equal(iface->macs[0].refcnt, 1); + assert_true(iface->macs[0].hardware); + assert_memory_equal(&iface->macs[1].mac, &ucast3, sizeof(ucast3)); + assert_int_equal(iface->macs[1].refcnt, 1); + assert_true(iface->macs[1].hardware); +} + +static void vlan_mac_add_multicast(void **) { + assert_return_code(iface_add_eth_addr(vlan_iface, &mcast1), errno); + assert_int_equal(vec_len(vlan_iface->macs), 0); +} + +static void vlan_mac_add_unicast(void **) { + assert_int_equal(iface_add_eth_addr(vlan_iface, NULL), -EINVAL); + + assert_return_code(iface_add_eth_addr(vlan_iface, &default_vlan_mac), errno); + assert_int_equal(vec_len(vlan_iface->macs), 0); + + // the underlying port iface still knows ucast2 and ucast3 + will_return(__wrap_rte_eth_dev_mac_addr_add, -ENOSPC); + will_return(__wrap_rte_eth_promiscuous_enable, 0); + will_return(__wrap_iface_from_id, iface); + assert_return_code(iface_add_eth_addr(vlan_iface, &ucast1), errno); + assert_int_equal(vec_len(vlan_iface->macs), 1); + assert_memory_equal(&vlan_iface->macs[0].mac, &ucast1, sizeof(ucast1)); + assert_int_equal(vlan_iface->macs[0].refcnt, 1); + assert_true(iface->state & GR_IFACE_S_PROMISC_FIXED); + assert_int_equal(vec_len(iface->macs), 3); + assert_memory_equal(&iface->macs[2].mac, &ucast1, sizeof(ucast1)); + assert_int_equal(iface->macs[2].refcnt, 1); + assert_false(iface->macs[2].hardware); + + will_return(__wrap_iface_from_id, iface); + assert_return_code(iface_add_eth_addr(vlan_iface, &ucast2), errno); + assert_int_equal(vec_len(vlan_iface->macs), 2); + assert_memory_equal(&vlan_iface->macs[1].mac, &ucast2, sizeof(ucast2)); + assert_int_equal(vlan_iface->macs[1].refcnt, 1); + assert_true(iface->state & GR_IFACE_S_PROMISC_FIXED); + assert_int_equal(vec_len(iface->macs), 3); + assert_memory_equal(&iface->macs[0].mac, &ucast2, sizeof(ucast2)); + assert_int_equal(iface->macs[0].refcnt, 2); + assert_true(iface->macs[0].hardware); + + assert_return_code(iface_add_eth_addr(vlan_iface, &ucast1), errno); + assert_int_equal(vec_len(vlan_iface->macs), 2); + assert_memory_equal(&vlan_iface->macs[0].mac, &ucast1, sizeof(ucast1)); + assert_int_equal(vlan_iface->macs[0].refcnt, 2); + assert_true(iface->state & GR_IFACE_S_PROMISC_FIXED); + assert_int_equal(vec_len(iface->macs), 3); + assert_memory_equal(&iface->macs[2].mac, &ucast1, sizeof(ucast1)); + assert_int_equal(iface->macs[2].refcnt, 1); + assert_false(iface->macs[2].hardware); + + will_return(__wrap_iface_from_id, iface); + assert_return_code(iface_add_eth_addr(vlan_iface, &ucast3), errno); + assert_int_equal(vec_len(vlan_iface->macs), 3); + assert_memory_equal(&vlan_iface->macs[2].mac, &ucast3, sizeof(ucast3)); + assert_int_equal(vlan_iface->macs[2].refcnt, 1); + assert_true(iface->state & GR_IFACE_S_PROMISC_FIXED); + assert_int_equal(vec_len(iface->macs), 3); + assert_memory_equal(&iface->macs[1].mac, &ucast3, sizeof(ucast3)); + assert_int_equal(iface->macs[1].refcnt, 2); + assert_true(iface->macs[1].hardware); +} + +static void vlan_mac_del_unicast(void **) { + assert_return_code(iface_del_eth_addr(vlan_iface, &ucast1), errno); + assert_memory_equal(&vlan_iface->macs[0].mac, &ucast1, sizeof(ucast1)); + assert_int_equal(vlan_iface->macs[0].refcnt, 1); + assert_true(iface->state & GR_IFACE_S_PROMISC_FIXED); + assert_int_equal(vec_len(iface->macs), 3); + assert_memory_equal(&iface->macs[2].mac, &ucast1, sizeof(ucast1)); + assert_int_equal(iface->macs[2].refcnt, 1); + assert_false(iface->macs[2].hardware); + + will_return(__wrap_iface_from_id, iface); + will_return(__wrap_rte_eth_promiscuous_disable, 0); + assert_return_code(iface_del_eth_addr(vlan_iface, &ucast1), errno); + assert_int_equal(vec_len(vlan_iface->macs), 2); + assert_memory_equal(&vlan_iface->macs[0].mac, &ucast2, sizeof(ucast2)); + assert_int_equal(vlan_iface->macs[0].refcnt, 1); + assert_memory_equal(&vlan_iface->macs[1].mac, &ucast3, sizeof(ucast3)); + assert_int_equal(vlan_iface->macs[1].refcnt, 1); + assert_int_equal(vec_len(iface->macs), 2); + assert_memory_equal(&iface->macs[0].mac, &ucast2, sizeof(ucast2)); + assert_int_equal(iface->macs[0].refcnt, 2); + assert_memory_equal(&iface->macs[1].mac, &ucast3, sizeof(ucast3)); + assert_int_equal(iface->macs[1].refcnt, 2); +} + +int main(void) { + const struct CMUnitTest tests[] = { + cmocka_unit_test(port_mac_add_multicast), + cmocka_unit_test(port_mac_add_unicast), + cmocka_unit_test(port_mac_del_unicast), + cmocka_unit_test(vlan_mac_add_multicast), + cmocka_unit_test(vlan_mac_add_unicast), + cmocka_unit_test(vlan_mac_del_unicast), + }; + return cmocka_run_group_tests(tests, setup, teardown); +} diff --git a/modules/infra/control/meson.build b/modules/infra/control/meson.build index c9008981c..4870ae4cf 100644 --- a/modules/infra/control/meson.build +++ b/modules/infra/control/meson.build @@ -48,14 +48,12 @@ tests += [ ], }, { - 'sources': files('port_test.c', 'port.c'), + 'sources': files('iface_test.c', 'iface.c', 'port.c', 'vlan.c'), 'link_args': [ - '-Wl,--wrap=rte_eth_allmulticast_disable', + '-Wl,--wrap=iface_from_id', '-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 7ff89d82d..a9c1e90b9 100644 --- a/modules/infra/control/port.c +++ b/modules/infra/control/port.c @@ -219,7 +219,7 @@ static int port_mac_set(struct iface *iface, const struct rte_ether_addr *mac) { return 0; } -int port_promisc_set(struct iface *iface, bool enabled) { +static int port_promisc_set(struct iface *iface, bool enabled) { struct iface_info_port *p = iface_info_port(iface); int ret; @@ -653,147 +653,76 @@ static int port_mac_get(const struct iface *iface, struct rte_ether_addr *mac) { return 0; } -int port_mac_add(struct iface *iface, const struct rte_ether_addr *mac) { +static int port_mac_add(struct iface *iface, struct iface_mac *m) { struct iface_info_port *port = iface_info_port(iface); - struct port_mac *m; int ret; - if (mac == NULL) - return errno_set(EINVAL); - if (rte_is_multicast_ether_addr(mac)) - return 0; // ALLMULTI is always on - - if (rte_is_same_ether_addr(mac, &port->mac)) - return 0; - - for (unsigned i = 0; i < port->filter.count; i++) { - m = &port->filter.macs[i]; - if (rte_is_same_ether_addr(&m->mac, mac)) { - LOG(DEBUG, - "%s: mac " ETH_F " already filtered (refs=%u)", - iface->name, - mac, - m->refcnt); - m->refcnt++; - return 0; - } - } - if (port->filter.count >= ARRAY_DIM(port->filter.macs)) - return errno_set(ENOSPC); - - m = &port->filter.macs[port->filter.count++]; - m->refcnt = 1; - m->mac = *mac; - - LOG(INFO, "%s: enabling " ETH_F " mac filter", iface->name, mac); - - if (iface->state & GR_IFACE_S_PROMISC_FIXED) - return 0; - - ret = rte_eth_dev_mac_addr_add(port->port_id, (struct rte_ether_addr *)mac, 0); - if (ret == -ENOSPC || ret == -EOPNOTSUPP) { - if (ret == -ENOSPC) { - port->filter.flags |= MAC_FILTER_F_NOSPC; - port->filter.hw_limit = port->filter.count - 1; - LOG(INFO, "%s: %s", iface->name, rte_strerror(-ret)); - } else { - port->filter.flags |= MAC_FILTER_F_UNSUPP; + ret = rte_eth_dev_mac_addr_add(port->port_id, &m->mac, 0); + if (ret == 0) { + m->hardware = true; + } else if (ret == -ENOSPC || ret == -EOPNOTSUPP) { + if (ret == -ENOSPC) LOG(INFO, "%s: %s", iface->name, rte_strerror(-ret)); + else + LOG(DEBUG, "%s: %s", iface->name, rte_strerror(-ret)); + + ret = 0; + if ((iface->state & GR_IFACE_S_PROMISC_FIXED) == 0) { + ret = rte_eth_promiscuous_enable(port->port_id); + if (ret == 0) { + LOG(INFO, "%s: enabled promisc", iface->name); + iface->state |= GR_IFACE_S_PROMISC_FIXED; + } else { + LOG(INFO, "%s: %s", iface->name, rte_strerror(-ret)); + } } - - LOG(INFO, "%s: enabling promisc", iface->name); - - // promisc enable is a noop if already enabled - ret = rte_eth_promiscuous_enable(port->port_id); - if (ret == 0) - iface->state |= GR_IFACE_S_PROMISC_FIXED; } - if (ret < 0) { - port->filter.count--; + if (ret < 0) return errno_set(-ret); - } - - if (iface->state & GR_IFACE_S_PROMISC_FIXED) { - // 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. - for (unsigned i = 0; i < port->filter.count - 1; i++) { - m = &port->filter.macs[i]; - rte_eth_dev_mac_addr_remove(port->port_id, &m->mac); - } - } return 0; } -int port_mac_del(struct iface *iface, const struct rte_ether_addr *mac) { +static int port_mac_del(struct iface *iface, struct iface_mac *m) { struct iface_info_port *port = iface_info_port(iface); - struct port_mac *m; - uint8_t i; int ret; - if (mac == NULL) - return errno_set(EINVAL); - if (rte_is_multicast_ether_addr(mac)) - return 0; // ALLMULTI is always on - - if (rte_is_same_ether_addr(mac, &port->mac)) - return 0; - - for (i = 0; i < port->filter.count; i++) { - m = &port->filter.macs[i]; - if (rte_is_same_ether_addr(&m->mac, mac)) - goto found; - } - return errno_set(ENOENT); - -found: - if (--m->refcnt > 0) { - LOG(DEBUG, - "%s: mac " ETH_F " still filtered (refs=%u)", - iface->name, - mac, - m->refcnt); - return 0; + if (m->hardware) { + ret = rte_eth_dev_mac_addr_remove(port->port_id, &m->mac); + if (ret < 0 && ret != -ENOTSUP) + LOG(WARNING, "%s: %s", iface->name, rte_strerror(-ret)); } - LOG(INFO, "%s: removing " ETH_F " mac filter", iface->name, mac); + if (iface->state & GR_IFACE_S_PROMISC_FIXED) { + bool disable_promisc = true; - if (port->filter.count > 1) { - // Swap the removed address with the last one - port->filter.macs[i] = port->filter.macs[port->filter.count - 1]; - } - port->filter.count--; + vec_foreach_ref (struct iface_mac *m2, iface->macs) { + if (m == m2 || m2->hardware) + continue; + ret = rte_eth_dev_mac_addr_add(port->port_id, &m2->mac, 0); + if (ret < 0) { + LOG(INFO, "%s: %s", iface->name, rte_strerror(-ret)); + disable_promisc = false; + break; + } + m2->hardware = true; + } - if (iface->state & GR_IFACE_S_PROMISC_FIXED) { - if (port->filter.count > 0 && port->filter.flags & MAC_FILTER_F_UNSUPP) - return 0; - if (port->filter.count > port->filter.hw_limit - && port->filter.flags & MAC_FILTER_F_NOSPC) - return 0; - port->filter.flags = 0; - port->filter.hw_limit = 0; - ret = rte_eth_promiscuous_disable(port->port_id); - if (ret < 0 && ret != -ENOTSUP) - LOG(NOTICE, "%s: promisc disable: %s", iface->name, rte_strerror(-ret)); - else - iface->state &= ~GR_IFACE_S_PROMISC_FIXED; - - // Reinstall all addresses after disabling promisc. - for (i = 0; i < port->filter.count; i++) { - m = &port->filter.macs[i]; - int r = rte_eth_dev_mac_addr_add(port->port_id, &m->mac, 0); - if (r < 0 && ret == 0) - ret = r; + if (disable_promisc) { + ret = rte_eth_promiscuous_disable(port->port_id); + if (ret < 0 && ret != -ENOTSUP) { + LOG(NOTICE, + "%s: promisc disable: %s", + iface->name, + rte_strerror(-ret)); + } else { + LOG(INFO, "%s: disabled promisc", iface->name); + iface->state &= ~GR_IFACE_S_PROMISC_FIXED; + } } - } else { - ret = rte_eth_dev_mac_addr_remove(port->port_id, (struct rte_ether_addr *)mac); } - if (ret < 0 && ret != -ENOTSUP) - LOG(WARNING, "%s: %s", iface->name, rte_strerror(-ret)); - return 0; } diff --git a/modules/infra/control/port.h b/modules/infra/control/port.h index adbd2e3b0..a82ee4156 100644 --- a/modules/infra/control/port.h +++ b/modules/infra/control/port.h @@ -14,16 +14,6 @@ #include -typedef enum { - MAC_FILTER_F_UNSUPP = GR_BIT8(0), - MAC_FILTER_F_NOSPC = GR_BIT8(1), -} mac_filter_flags_t; - -struct port_mac { - uint16_t refcnt; - struct rte_ether_addr mac; -}; - GR_IFACE_INFO(GR_IFACE_TYPE_PORT, iface_info_port, { BASE(__gr_iface_info_port_base); @@ -37,16 +27,6 @@ GR_IFACE_INFO(GR_IFACE_TYPE_PORT, iface_info_port, { bool virtio_offloads; uint64_t rx_offloads; rte_spinlock_t txq_locks[RTE_MAX_QUEUES_PER_PORT]; - struct { - mac_filter_flags_t flags; - unsigned hw_limit; - unsigned count; - struct port_mac macs[RTE_ETH_NUM_RECEIVE_MAC_ADDR]; - } filter; }); const struct iface *port_get_iface(uint16_t port_id); - -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); diff --git a/modules/infra/control/port_test.c b/modules/infra/control/port_test.c deleted file mode 100644 index b843df5ca..000000000 --- a/modules/infra/control/port_test.c +++ /dev/null @@ -1,157 +0,0 @@ -// SPDX-License-Identifier: BSD-3-Clause -// Copyright (c) 2024 Robin Jarry - -#include "_cmocka.h" -#include "config.h" -#include "event.h" -#include "log.h" -#include "mempool.h" -#include "module.h" -#include "netlink.h" -#include "port.h" -#include "rcu.h" -#include "vrf.h" -#include "worker.h" - -// mocked types/functions -int gr_rte_log_type; -struct log_types log_types = STAILQ_HEAD_INITIALIZER(log_types); -struct gr_config gr_config; -struct workers workers; -void __api_handler(uint32_t, api_handler_func, const char *, size_t) { } -void module_register(struct module *) { } -void iface_type_register(const struct iface_type *) { } -void event_push(uint32_t, const void *) { } -void event_subscribe(uint32_t, event_sub_cb_t) { } -int netlink_link_set_name(uint32_t, const char *) { - return 0; -} -int netlink_set_ifalias(uint32_t, const char *) { - return 0; -} -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; -} -uint16_t vrf_default_get_or_create(void) { - return 0; -} -int vrf_incref(uint16_t) { - return 0; -} -int iface_set_eth_addr(struct iface *, const struct rte_ether_addr *) { - return 0; -} -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 *, 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)); -void metrics_ctx_init(struct metrics_ctx *, struct metrics_writer *, ...) { } -void metrics_labels_add(struct metrics_ctx *, ...) { } -void metric_emit(struct metrics_ctx *, const struct metric *, uint64_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->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->filter.count, 1); - assert_int_equal(port->filter.macs[0].refcnt, 1); - - will_return(__wrap_rte_eth_dev_mac_addr_add, 0); - assert_return_code(port_mac_add(iface, &ucast2), errno); - assert_int_equal(port->filter.count, 2); - assert_int_equal(port->filter.macs[1].refcnt, 1); - - assert_return_code(port_mac_add(iface, &ucast1), errno); - assert_int_equal(port->filter.count, 2); - assert_int_equal(port->filter.macs[0].refcnt, 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->filter.flags & MAC_FILTER_F_NOSPC); - assert_true(iface->state & GR_IFACE_S_PROMISC_FIXED); - assert_int_equal(port->filter.count, 3); - assert_int_equal(port->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->filter.count, 2); - assert_int_equal(port->filter.hw_limit, 0); - assert_memory_equal(&port->filter.macs[0].mac, &ucast3, sizeof(ucast3)); - assert_memory_equal(&port->filter.macs[1].mac, &ucast2, sizeof(ucast2)); -} - -int main(void) { - const struct CMUnitTest tests[] = { - cmocka_unit_test(mac_add_unicast), - cmocka_unit_test(mac_del_unicast), - }; - return cmocka_run_group_tests(tests, setup, teardown); -} diff --git a/modules/infra/control/vlan.c b/modules/infra/control/vlan.c index b3902cd8b..a44d3d233 100644 --- a/modules/infra/control/vlan.c +++ b/modules/infra/control/vlan.c @@ -200,24 +200,25 @@ static int iface_vlan_set_eth_addr(struct iface *iface, const struct rte_ether_a return 0; } -static int iface_vlan_add_eth_addr(struct iface *iface, const struct rte_ether_addr *mac) { +static int iface_vlan_add_eth_addr(struct iface *iface, struct iface_mac *m) { const struct iface_info_vlan *vlan = iface_info_vlan(iface); struct iface *parent = iface_from_id(vlan->parent_id); - if (mac == NULL || !rte_is_multicast_ether_addr(mac)) - return errno_set(EINVAL); - - return iface_add_eth_addr(parent, mac); + return iface_add_eth_addr(parent, &m->mac); } -static int iface_vlan_del_eth_addr(struct iface *iface, const struct rte_ether_addr *mac) { +static int iface_vlan_del_eth_addr(struct iface *iface, struct iface_mac *m) { const struct iface_info_vlan *vlan = iface_info_vlan(iface); struct iface *parent = iface_from_id(vlan->parent_id); - if (mac == NULL || !rte_is_multicast_ether_addr(mac)) - return errno_set(EINVAL); + return iface_del_eth_addr(parent, &m->mac); +} + +static int iface_vlan_promisc_set(struct iface *iface, bool enabled) { + const struct iface_info_vlan *vlan = iface_info_vlan(iface); + struct iface *parent = iface_from_id(vlan->parent_id); - return iface_del_eth_addr(parent, mac); + return iface_set_promisc(parent, enabled); } static void vlan_to_api(void *info, const struct iface *iface) { @@ -239,6 +240,7 @@ static const struct iface_type iface_type_vlan = { .set_eth_addr = iface_vlan_set_eth_addr, .add_eth_addr = iface_vlan_add_eth_addr, .del_eth_addr = iface_vlan_del_eth_addr, + .set_promisc = iface_vlan_promisc_set, .to_api = vlan_to_api, }; diff --git a/modules/l2/control/bridge.c b/modules/l2/control/bridge.c index 9223a4efa..90cd4995f 100644 --- a/modules/l2/control/bridge.c +++ b/modules/l2/control/bridge.c @@ -58,6 +58,8 @@ static int bridge_attach_member(struct iface *bridge, struct iface *member) { member->vrf_id = GR_VRF_ID_UNDEF; member->mode = GR_IFACE_MODE_BRIDGE; + fdb_sync_hardware(bridge, member, true); + return 0; } @@ -67,6 +69,9 @@ static int bridge_detach_member(struct iface *bridge, struct iface *member) { for (unsigned i = 0; i < br->n_members; i++) { if (br->members[i] == member) { unsigned last = br->n_members - 1; + + fdb_sync_hardware(bridge, member, false); + if (i < last) br->members[i] = br->members[last]; br->n_members--; @@ -86,6 +91,7 @@ static int bridge_fini(struct iface *iface) { for (unsigned i = 0; i < bridge->n_members; i++) { struct iface *member = bridge->members[i]; + fdb_sync_hardware(iface, member, false); iface_set_promisc(member, false); member->vrf_id = vrf_default_get_or_create(); if (member->vrf_id != GR_VRF_ID_UNDEF) diff --git a/modules/l2/control/fdb.c b/modules/l2/control/fdb.c index 7d3edc370..85a86347a 100644 --- a/modules/l2/control/fdb.c +++ b/modules/l2/control/fdb.c @@ -332,6 +332,67 @@ static struct api_out fdb_config_set(const void *request, struct api_ctx *) { return api_out(0, 0, NULL); } +static void push_mac_to_hw(struct iface *iface, const struct rte_ether_addr *mac, bool add) { + int ret; + + if (add) + ret = iface_add_eth_addr(iface, mac); + else + ret = iface_del_eth_addr(iface, mac); + if (ret < 0) { + LOG(DEBUG, + "failed to %s mac " ETH_F " to %s: %s", + add ? "add" : "del", + mac, + iface->name, + strerror(errno)); + } +} + +static void fdb_event_cb(uint32_t event, const void *obj) { + const struct iface_info_bridge *bridge_info; + const struct gr_fdb_entry *fdb = obj; + const struct iface *bridge; + + bridge = iface_from_id(fdb->bridge_id); + if (bridge == NULL) { + LOG(ERR, "unknown bridge %u", fdb->bridge_id); + return; + } + + if ((fdb->flags & GR_FDB_F_EXTERN) == 0) + return; + + bridge_info = iface_info_bridge(bridge); + for (unsigned i = 0; i < bridge_info->n_members; i++) { + struct iface *member = bridge_info->members[i]; + + // we have no clear idea what to do with a vlan_id if one got pushed by FRR + assert(fdb->vlan_id == 0); + // FIXME: ideally, we should ask for adding the mac in hw only when getting the + // *first* transition from !EXTERN to EXTERN to handle both learning mac on the + // wire and from FRR. + push_mac_to_hw(member, &fdb->mac, event != GR_EVENT_FDB_DEL); + } +} + +void fdb_sync_hardware(const struct iface *bridge, struct iface *member, bool add) { + struct gr_fdb_entry *fdb; + uint32_t next = 0; + const void *key; + void *data; + + while (rte_hash_iterate(fdb_hash, &key, &data, &next) >= 0) { + if (!fdb_match(data, GR_FDB_F_EXTERN, bridge->id, GR_IFACE_ID_UNDEF, NULL)) + continue; + + fdb = data; + // we have no clear idea what to do with a vlan_id if one got pushed by FRR + assert(fdb->vlan_id == 0); + push_mac_to_hw(member, &fdb->mac, add); + } +} + static void fdb_ageing_cb(evutil_socket_t, short /*what*/, void * /*priv*/) { const struct iface *bridge; struct gr_fdb_entry *fdb; @@ -409,6 +470,8 @@ RTE_INIT(init) { api_handler(GR_FDB_LIST, fdb_list); api_handler(GR_FDB_CONFIG_GET, fdb_config_get); api_handler(GR_FDB_CONFIG_SET, fdb_config_set); + event_subscribe(GR_EVENT_FDB_ADD, fdb_event_cb); + event_subscribe(GR_EVENT_FDB_DEL, fdb_event_cb); event_serializer(GR_EVENT_FDB_ADD, NULL); event_serializer(GR_EVENT_FDB_DEL, NULL); event_serializer(GR_EVENT_FDB_UPDATE, NULL); diff --git a/modules/l2/control/l2.h b/modules/l2/control/l2.h index 9b37c581c..82f137db2 100644 --- a/modules/l2/control/l2.h +++ b/modules/l2/control/l2.h @@ -39,6 +39,9 @@ void fdb_learn( // Delete all FDB entries referencing the provided interface. void fdb_purge_iface(uint16_t iface_id); +// Push or remove mac addresses from HW Rx filters. +void fdb_sync_hardware(const struct iface *bridge, struct iface *member, bool add); + // Delete all FDB entries referencing the provided bridge. void fdb_purge_bridge(uint16_t bridge_id); diff --git a/smoke/iface_mac_test.sh b/smoke/iface_mac_test.sh new file mode 100755 index 000000000..2591e6973 --- /dev/null +++ b/smoke/iface_mac_test.sh @@ -0,0 +1,126 @@ +#!/bin/bash +# SPDX-License-Identifier: BSD-3-Clause +# Copyright (c) 2026 David Marchand + +. $(dirname $0)/_init.sh + +# Create a test port +grcli interface add port p0 devargs net_tap0,iface=x-p0 + +# Test adding secondary MAC addresses +mac1="02:00:00:00:00:01" +mac2="02:00:00:00:00:02" +mac3="02:00:00:00:00:03" + +echo "Adding secondary MAC addresses..." +grcli mac add "$mac1" iface p0 +grcli mac add "$mac2" iface p0 +grcli mac add "$mac3" iface p0 + +# List all MAC addresses +echo "Listing MAC addresses on p0:" +grcli mac show iface p0 + +# Verify MAC addresses are present (3 secondary + 1 primary = 4 total) +count=$(grcli -j mac show iface p0 | jq 'length') +if [ "$count" -ne 4 ]; then + fail "Expected 4 MACs (1 primary + 3 secondary), found $count" +fi + +# Verify specific MAC is present with refcnt 1 +refcnt=$(grcli -j mac show iface p0 | jq -r --arg mac "$mac1" '.[] | select(.mac == $mac) | .refcnt') +if [ "$refcnt" -ne 1 ]; then + fail "MAC $mac1 should have refcnt 1, found $refcnt" +fi + +# Test deleting a MAC address +echo "Deleting MAC $mac2..." +grcli mac del "$mac2" iface p0 + +# Verify MAC was deleted (1 primary + 2 secondary = 3 total) +count=$(grcli -j mac show iface p0 | jq 'length') +if [ "$count" -ne 3 ]; then + fail "Expected 3 MACs (1 primary + 2 secondary) after deletion, found $count" +fi + +# Verify deleted MAC is gone +if grcli -j mac show iface p0 | jq -e --arg mac "$mac2" '.[] | select(.mac == $mac)' 2>/dev/null; then + fail "MAC $mac2 should have been deleted" +fi + +# Test adding duplicate MAC (should succeed, refcount should increase) +echo "Adding duplicate MAC $mac1..." +grcli mac add "$mac1" iface p0 + +# Still should have 3 entries (1 primary + 2 secondary, refcount increased) +count=$(grcli -j mac show iface p0 | jq 'length') +if [ "$count" -ne 3 ]; then + fail "Expected 3 MACs (1 primary + 2 secondary) after duplicate add, found $count" +fi + +# Verify refcnt increased to 2 +refcnt=$(grcli -j mac show iface p0 | jq -r --arg mac "$mac1" '.[] | select(.mac == $mac) | .refcnt') +if [ "$refcnt" -ne 2 ]; then + fail "MAC $mac1 should have refcnt 2 after duplicate add, found $refcnt" +fi + +# Delete once, refcnt should decrease to 1 +echo "Deleting MAC $mac1 (first time)..." +grcli mac del "$mac1" iface p0 + +count=$(grcli -j mac show iface p0 | jq 'length') +if [ "$count" -ne 3 ]; then + fail "Expected 3 MACs (1 primary + 2 secondary) after first delete, found $count" +fi + +refcnt=$(grcli -j mac show iface p0 | jq -r --arg mac "$mac1" '.[] | select(.mac == $mac) | .refcnt') +if [ "$refcnt" -ne 1 ]; then + fail "MAC $mac1 should have refcnt 1 after first delete, found $refcnt" +fi + +# Delete again, MAC should be removed +echo "Deleting MAC $mac1 (second time)..." +grcli mac del "$mac1" iface p0 + +count=$(grcli -j mac show iface p0 | jq 'length') +if [ "$count" -ne 2 ]; then + fail "Expected 2 MACs (1 primary + 1 secondary) after second delete, found $count" +fi + +# Verify mac1 is completely gone +if grcli -j mac show iface p0 | jq -e --arg mac "$mac1" '.[] | select(.mac == $mac)' 2>/dev/null; then + fail "MAC $mac1 should have been completely removed" +fi + +# Create VLAN interface to test MAC inheritance +echo "Creating VLAN interface..." +grcli interface add vlan p0.100 parent p0 vlan 100 + +# Add MAC to VLAN, should also be added to parent port +mac4="02:00:00:00:00:04" +grcli mac add "$mac4" iface p0.100 + +# Verify MAC is on VLAN +grcli -j mac show iface p0.100 | jq -e --arg mac "$mac4" '.[] | select(.mac == $mac)' \ + || fail "MAC $mac4 not found on VLAN" + +# List all MACs across all interfaces +echo "Listing all MAC addresses:" +grcli mac show + +# Test error handling: try to add invalid MAC +if grcli mac add "invalid-mac" iface p0 2>/dev/null; then + fail "Adding invalid MAC should have failed" +fi + +# Test error handling: try to add MAC to non-existent interface +if grcli mac add "$mac1" iface nonexistent 2>/dev/null; then + fail "Adding MAC to non-existent interface should have failed" +fi + +# Test error handling: try to delete non-existent MAC +if grcli mac del "02:00:00:00:00:99" iface p0 2>/dev/null; then + fail "Deleting non-existent MAC should have failed" +fi + +echo "All MAC address tests passed"