Skip to content

datapath: cache optimization for 12% throughput improvement #495

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
rjarry wants to merge 6 commits into from
Closed

datapath: cache optimization for 12% throughput improvement #495

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9eb521a
treewide: use UINT_NUM_VALUES for edge registration arrays
rjarry Feb 1, 2026
f6f5c43
iface: remove redundant type name field
rjarry Feb 2, 2026
df2e50d
iface: store types in a static array
rjarry Feb 2, 2026
4da99a6
infra: accumulate interface stats before flushing
rjarry Feb 2, 2026
14c2d86
infra: move RCU quiescent state report to housekeeping interval
rjarry Feb 3, 2026
ad404b0
port_rx: only access ether type when port is bonded
rjarry Feb 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions docs/graph.svg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
3 changes: 1 addition & 2 deletions modules/infra/control/bond.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -415,9 +415,8 @@ static void bond_to_api(void *info, const struct iface *iface) {
}
}

static struct iface_type iface_type_bond = {
static const struct iface_type iface_type_bond = {
.id = GR_IFACE_TYPE_BOND,
.name = "bond",
.pub_size = sizeof(struct gr_iface_info_bond),
.priv_size = sizeof(struct iface_info_bond),
.init = bond_init,
Expand Down
4 changes: 1 addition & 3 deletions modules/infra/control/gr_iface.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,9 @@ struct iface_type {
int (*set_promisc)(struct iface *, bool enabled);
void (*to_api)(void *api_info, const struct iface *);
void (*metrics_collect)(struct gr_metrics_ctx *, const struct iface *);
const char *name;
STAILQ_ENTRY(iface_type) next;
};

void iface_type_register(struct iface_type *);
void iface_type_register(const struct iface_type *);
const struct iface_type *iface_type_get(gr_iface_type_t type_id);
struct iface *iface_create(const struct gr_iface *conf, const void *api_info);
int iface_reconfig(
Expand Down
16 changes: 7 additions & 9 deletions modules/infra/control/iface.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
#include <vrf_priv.h>
#include <wchar.h>

static STAILQ_HEAD(, iface_type) types = STAILQ_HEAD_INITIALIZER(types);
static const struct iface_type *iface_types[UINT_NUM_VALUES(gr_iface_type_t)];

struct iface_stats iface_stats[MAX_IFACES][RTE_MAX_LCORE];

Expand All @@ -42,19 +42,15 @@ static bool iface_type_valid(gr_iface_type_t type) {
}

const struct iface_type *iface_type_get(gr_iface_type_t type_id) {
struct iface_type *t;
STAILQ_FOREACH (t, &types, next)
if (t->id == type_id)
return t;
return errno_set_null(ENODEV);
return iface_types[type_id];
}
Comment on lines 44 to 46
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Feb 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Add bounds guard in iface_type_get to prevent OOB reads.

iface_types[type_id] is now indexed directly. If type_id comes from untrusted config (e.g., API input) and exceeds UINT_NUM_VALUES(gr_iface_type_t), this becomes an out-of-bounds read and potential memory-safety issue. Return NULL (or set errno) when the id is invalid so callers fail safely.

🛡️ Proposed fix 
 const struct iface_type *iface_type_get(gr_iface_type_t type_id) {
-	return iface_types[type_id];
+	if ((unsigned)type_id >= UINT_NUM_VALUES(gr_iface_type_t))
+		return NULL;
+	return iface_types[type_id];
 }
🤖 Prompt for AI Agents 
In `@modules/infra/control/iface.c` around lines 44 - 46, Add a bounds check in
iface_type_get so it validates the incoming gr_iface_type_t type_id against
UINT_NUM_VALUES(gr_iface_type_t) before indexing iface_types; if the id is out
of range, return NULL (and optionally set errno = EINVAL) to avoid an
out-of-bounds read instead of directly returning iface_types[type_id]. Ensure
callers that expect non-NULL handle the NULL return.


void iface_type_register(struct iface_type *type) {
void iface_type_register(const struct iface_type *type) {
if (!iface_type_valid(type->id))
ABORT("invalid iface type id: %u", type->id);
if (iface_type_get(type->id) != NULL)
ABORT("duplicate iface type id: %u", type->id);
STAILQ_INSERT_TAIL(&types, type, next);
iface_types[type->id] = type;
}

#define IFACE_ID_FIRST GR_IFACE_ID_UNDEF + 1
Expand Down Expand Up @@ -93,8 +89,10 @@ struct iface *iface_create(const struct gr_iface *conf, const void *api_info) {
bool vrf_ref = false;
uint16_t ifid;

if (type == NULL)
if (type == NULL) {
errno = ENODEV;
goto fail;
}
if (charset_check(conf->name, GR_IFACE_NAME_SIZE) < 0)
goto fail;
while ((iface = iface_next(GR_IFACE_TYPE_UNDEF, iface)) != NULL) {
Expand Down
3 changes: 1 addition & 2 deletions modules/infra/control/loopback.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,9 +307,8 @@ static void loopback_module_fini(struct event_base *) {

static void iface_loopback_to_api(void * /* info */, const struct iface * /* iface */) { }

static struct iface_type iface_type_loopback = {
static const struct iface_type iface_type_loopback = {
.id = GR_IFACE_TYPE_LOOPBACK,
.name = "loopback",
.pub_size = 0,
.priv_size = sizeof(struct iface_info_loopback),
.init = iface_loopback_init,
Expand Down
3 changes: 1 addition & 2 deletions modules/infra/control/port.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -810,9 +810,8 @@ static void port_fini(struct event_base *) {
gr_vec_free(reset_ports);
}

static struct iface_type iface_type_port = {
static const struct iface_type iface_type_port = {
.id = GR_IFACE_TYPE_PORT,
.name = "port",
.pub_size = sizeof(struct gr_iface_info_port),
.priv_size = sizeof(struct iface_info_port),
.init = iface_port_init,
Expand Down
2 changes: 1 addition & 1 deletion modules/infra/control/port_test.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ 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 iface_type_register(const struct iface_type *) { }
void gr_event_push(uint32_t, const void *) { }
void gr_event_subscribe(struct gr_event_subscription *) { }
mock_func(struct rte_mempool *, gr_pktmbuf_pool_get(int8_t, uint32_t));
Expand Down
3 changes: 1 addition & 2 deletions modules/infra/control/vlan.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,9 +184,8 @@ static void vlan_to_api(void *info, const struct iface *iface) {
*api = vlan->base;
}

static struct iface_type iface_type_vlan = {
static const struct iface_type iface_type_vlan = {
.id = GR_IFACE_TYPE_VLAN,
.name = "vlan",
.pub_size = sizeof(struct gr_iface_info_vlan),
.priv_size = sizeof(struct iface_info_vlan),
.init = iface_vlan_init,
Expand Down
2 changes: 1 addition & 1 deletion modules/infra/control/worker_test.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ struct gr_config gr_config;
void gr_register_api_handler(struct gr_api_handler *) { }
void gr_register_module(struct gr_module *) { }
void gr_event_subscribe(struct gr_event_subscription *) { }
void iface_type_register(struct iface_type *) { }
void iface_type_register(const struct iface_type *) { }
void gr_metrics_ctx_init(struct gr_metrics_ctx *, struct gr_metrics_writer *, ...) { }
void gr_metrics_labels_add(struct gr_metrics_ctx *, ...) { }
void gr_metric_emit(struct gr_metrics_ctx *, const struct gr_metric *, uint64_t) { }
Expand Down
34 changes: 27 additions & 7 deletions modules/infra/datapath/bond_output.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,13 +202,19 @@ bond_select_tx_member(const struct rte_mbuf *m, const struct iface_info_bond *bo
static uint16_t
bond_output_process(struct rte_graph *graph, struct rte_node *node, void **objs, uint16_t nb_objs) {
const struct iface_info_bond *bond;
const struct iface *iface, *member;
uint16_t packets, last_iface_id;
const struct iface *member;
struct iface_stats *stats;
rte_edge_t edge;
uint64_t bytes;

last_iface_id = GR_IFACE_ID_UNDEF;
packets = 0;
bytes = 0;

for (unsigned i = 0; i < nb_objs; i++) {
struct rte_mbuf *mbuf = objs[i];
iface = mbuf_data(mbuf)->iface;
bond = iface_info_bond(iface);
bond = iface_info_bond(mbuf_data(mbuf)->iface);

// Select output member port
member = bond_select_tx_member(mbuf, bond);
Expand All @@ -224,16 +230,30 @@ bond_output_process(struct rte_graph *graph, struct rte_node *node, void **objs,
t->member_iface_id = member->id;
}

// Update bond statistics
struct iface_stats *stats = iface_get_stats(rte_lcore_id(), iface->id);
stats->tx_packets += 1;
stats->tx_bytes += rte_pktmbuf_pkt_len(mbuf);
if (member->id != last_iface_id) {
if (packets > 0) {
stats = iface_get_stats(rte_lcore_id(), last_iface_id);
stats->tx_packets += packets;
stats->tx_bytes += bytes;
}
last_iface_id = member->id;
packets = 0;
bytes = 0;
}
packets += 1;
bytes += rte_pktmbuf_pkt_len(mbuf);

edge = PORT_OUTPUT;
next:
rte_node_enqueue_x1(graph, node, edge, mbuf);
}

if (packets > 0) {
stats = iface_get_stats(rte_lcore_id(), last_iface_id);
stats->tx_packets += packets;
stats->tx_bytes += bytes;
}

return nb_objs;
}

Expand Down
60 changes: 36 additions & 24 deletions modules/infra/datapath/eth_input.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ enum {
NB_EDGES,
};

static rte_edge_t l2l3_edges[1 << 16] = {UNKNOWN_ETHER_TYPE};
static rte_edge_t l2l3_edges[UINT_NUM_VALUES(rte_be16_t)] = {UNKNOWN_ETHER_TYPE};

void gr_eth_input_add_type(rte_be16_t eth_type, const char *next_node) {
LOG(DEBUG, "eth_input: type=0x%04x -> %s", rte_be_to_cpu_16(eth_type), next_node);
Expand All @@ -34,7 +34,7 @@ void gr_eth_input_add_type(rte_be16_t eth_type, const char *next_node) {

static uint16_t
eth_input_process(struct rte_graph *graph, struct rte_node *node, void **objs, uint16_t nb_objs) {
uint16_t vlan_id, last_iface_id, last_vlan_id;
uint16_t vlan_id, last_iface_id, last_vlan_id, packets;
const struct iface *vlan_iface, *iface;
struct eth_input_mbuf_data *eth_in;
struct rte_ether_addr iface_mac;
Expand All @@ -45,11 +45,14 @@ eth_input_process(struct rte_graph *graph, struct rte_node *node, void **objs, u
struct rte_mbuf *m;
size_t l2_hdr_size;
rte_edge_t edge;
uint64_t bytes;

iface = NULL;
vlan_iface = NULL;
last_iface_id = UINT16_MAX;
last_iface_id = GR_IFACE_ID_UNDEF;
last_vlan_id = UINT16_MAX;
packets = 0;
bytes = 0;

for (uint16_t i = 0; i < nb_objs; i++) {
m = objs[i];
Expand Down Expand Up @@ -86,34 +89,38 @@ eth_input_process(struct rte_graph *graph, struct rte_node *node, void **objs, u
eth_in->iface = vlan_iface;
}

if (unlikely(rte_be_to_cpu_16(eth_type) < SNAP_MAX_LEN)) {
edge = SNAP;
goto snap;
}

edge = l2l3_edges[eth_type];

if (iface == NULL || iface->id != eth_in->iface->id) {
if (iface_get_eth_addr(eth_in->iface, &iface_mac) < 0) {
edge = INVALID_IFACE;
goto next;
}
if (packets > 0) {
stats = iface_get_stats(rte_lcore_id(), iface->id);
stats->rx_packets += packets;
stats->rx_bytes += bytes;
}
iface = eth_in->iface;
stats = iface_get_stats(rte_lcore_id(), eth_in->iface->id);
packets = 0;
bytes = 0;
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
packets += 1;
bytes += rte_pktmbuf_pkt_len(m);

stats->rx_packets += 1;
stats->rx_bytes += rte_pktmbuf_pkt_len(m);

if (unlikely(rte_is_multicast_ether_addr(&eth->dst_addr))) {
if (rte_is_broadcast_ether_addr(&eth->dst_addr))
eth_in->domain = ETH_DOMAIN_BROADCAST;
else
eth_in->domain = ETH_DOMAIN_MULTICAST;
} else if (rte_is_same_ether_addr(&eth->dst_addr, &iface_mac)) {
eth_in->domain = ETH_DOMAIN_LOCAL;
if (unlikely(rte_be_to_cpu_16(eth_type) < SNAP_MAX_LEN)) {
edge = SNAP;
} else {
eth_in->domain = ETH_DOMAIN_OTHER;
if (unlikely(rte_is_multicast_ether_addr(&eth->dst_addr))) {
if (rte_is_broadcast_ether_addr(&eth->dst_addr))
eth_in->domain = ETH_DOMAIN_BROADCAST;
else
eth_in->domain = ETH_DOMAIN_MULTICAST;
} else if (rte_is_same_ether_addr(&eth->dst_addr, &iface_mac)) {
eth_in->domain = ETH_DOMAIN_LOCAL;
} else {
eth_in->domain = ETH_DOMAIN_OTHER;
}
rte_pktmbuf_adj(m, l2_hdr_size);
edge = l2l3_edges[eth_type];
}
next:
if (gr_mbuf_is_traced(m)
Expand All @@ -125,10 +132,15 @@ eth_input_process(struct rte_graph *graph, struct rte_node *node, void **objs, u
t->vlan_id = vlan_id;
t->iface_id = eth_in->iface->id;
}
rte_pktmbuf_adj(m, l2_hdr_size);
snap:
rte_node_enqueue_x1(graph, node, edge, m);
}

if (packets > 0) {
stats = iface_get_stats(rte_lcore_id(), iface->id);
stats->rx_packets += packets;
stats->rx_bytes += bytes;
}

return nb_objs;
}

Expand Down
Loading