Skip to content

infra: expose link speed attribute for all interface types #401

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

Merged
christophefontaine merged 2 commits into from
Nov 24, 2025
Merged

infra: expose link speed attribute for all interface types #401

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6bb6923
infra: expose speed attribute for all interface types
rjarry Nov 19, 2025
07298ba
smoke: enable logging for all daemons
rjarry Nov 20, 2025
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
2 changes: 1 addition & 1 deletion modules/infra/api/gr_infra.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ struct __gr_iface_base {
uint16_t vrf_id; // L3 addressing and routing domain
uint16_t domain_id; // L2 xconnect peer interface id
};
uint32_t speed; //!< Link speed in Megabit/sec.
};

struct gr_iface {
Expand All @@ -93,7 +94,6 @@ struct __gr_iface_info_port_base {
uint16_t rxq_size;
uint16_t txq_size;
uint16_t bond_iface_id;
uint32_t link_speed; //!< Physical link speed in Megabit/sec.
struct rte_ether_addr mac;
};

Expand Down
4 changes: 2 additions & 2 deletions modules/infra/cli/bond.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,10 @@ static void bond_show(struct gr_api_client *c, const struct gr_iface *iface) {
const struct gr_iface_info_port *port;
port = (const struct gr_iface_info_port *)member->info;
printf(" mac: " ETH_F "\n", &port->mac);
if (port->link_speed == UINT32_MAX)
if (member->speed == UINT32_MAX)
printf(" speed: unknown\n");
else
printf(" speed: %u Mb/s\n", port->link_speed);
printf(" speed: %u Mb/s\n", member->speed);
}

free(member);
Expand Down
4 changes: 4 additions & 0 deletions modules/infra/cli/iface.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -464,6 +464,10 @@ static cmd_status_t iface_show(struct gr_api_client *c, const struct ec_pnode *p
printf("flags: %s\n", buf);
printf("vrf: %u\n", iface->vrf_id);
printf("mtu: %u\n", iface->mtu);
if (iface->speed == UINT32_MAX)
printf("speed: unknown\n");
else
printf("speed: %u Mb/s\n", iface->speed);

type = type_from_id(iface->type);
assert(type != NULL);
Expand Down
4 changes: 0 additions & 4 deletions modules/infra/cli/port.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,6 @@ static void port_show(struct gr_api_client *c, const struct gr_iface *iface) {
printf("bond: %u\n", port->bond_iface_id);
free(bond);
}
if (port->link_speed == UINT32_MAX)
printf("speed: unknown\n");
else
printf("speed: %u Mb/s\n", port->link_speed);
printf("n_rxq: %u\n", port->n_rxq);
printf("n_txq: %u\n", port->n_txq);
printf("rxq_size: %u\n", port->rxq_size);
Expand Down
12 changes: 10 additions & 2 deletions modules/infra/control/bond.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,7 @@ void bond_update_active_members(struct iface *iface) {
struct iface_info_bond *bond = iface_info_bond(iface);
const struct iface *member;
uint8_t *active_ids = NULL;
uint32_t speed = 0;

switch (bond->mode) {
case GR_BOND_MODE_ACTIVE_BACKUP:
Expand All @@ -248,6 +249,7 @@ void bond_update_active_members(struct iface *iface) {
for (uint8_t i = 0; i < bond->n_members; i++) {
member = bond->members[i].iface;
if (i == active_member) {
speed = member->speed;
gr_vec_add(active_ids, i);
LOG(INFO,
"bond %s active member is now %s",
Expand All @@ -261,7 +263,6 @@ void bond_update_active_members(struct iface *iface) {
case GR_BOND_MODE_LACP:
for (uint8_t i = 0; i < bond->n_members; i++) {
struct bond_member *member = &bond->members[i];
const struct iface_info_port *port = iface_info_port(member->iface);

// The port_number must *never* be zero,
// otherwise some switches reject the LACP packets.
Expand All @@ -272,7 +273,7 @@ void bond_update_active_members(struct iface *iface) {
member->local.system_mac = bond->mac;
// Key based on port speed (in Mb/s): simplified encoding for aggregation
// Ports with same speed can aggregate together
member->local.key = rte_cpu_to_be_16(port->link_speed);
member->local.key = rte_cpu_to_be_16(member->iface->speed);
if (member->last_rx == 0) {
member->local.state = LACP_STATE_ACTIVE | LACP_STATE_AGGREGATABLE
| LACP_STATE_FAST | LACP_STATE_DEFAULTED
Expand All @@ -294,11 +295,18 @@ void bond_update_active_members(struct iface *iface) {
iface->name,
member->iface->name);
gr_vec_add(active_ids, i);
if (member->iface->speed != RTE_ETH_SPEED_NUM_UNKNOWN)
speed += member->iface->speed;
}
}
break;
}

if (speed != 0)
iface->speed = speed;
else
iface->speed = RTE_ETH_SPEED_NUM_UNKNOWN;

if (gr_vec_len(active_ids) > 0) {
for (unsigned i = 0; i < ARRAY_DIM(bond->redirection_table); i++) {
bond->redirection_table[i] = active_ids[i % gr_vec_len(active_ids)];
Expand Down
4 changes: 4 additions & 0 deletions modules/infra/control/iface.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include <gr_vec.h>

#include <event2/event.h>
#include <rte_ethdev.h>
#include <rte_malloc.h>

#include <errno.h>
Expand Down Expand Up @@ -107,6 +108,7 @@ struct iface *iface_create(const struct gr_iface *conf, const void *api_info) {
goto fail;

iface->base = conf->base;
iface->speed = RTE_ETH_SPEED_NUM_UNKNOWN;
iface->id = ifid;
// this is only accessed by the API, no need to copy the name to DPDK memory (hugepages)
iface->name = strndup(conf->name, GR_IFACE_NAME_SIZE);
Expand Down Expand Up @@ -513,13 +515,15 @@ static void iface_event(uint32_t event, const void *obj) {
str = "STATUS_UP";
gr_vec_foreach (struct iface *s, iface->subinterfaces) {
s->state |= GR_IFACE_S_RUNNING;
s->speed = iface->speed;
gr_event_push(event, s);
}
break;
case GR_EVENT_IFACE_STATUS_DOWN:
str = "STATUS_DOWN";
gr_vec_foreach (struct iface *s, iface->subinterfaces) {
s->state &= ~GR_IFACE_S_RUNNING;
s->speed = RTE_ETH_SPEED_NUM_UNKNOWN;
gr_event_push(event, s);
}
break;
Expand Down
2 changes: 2 additions & 0 deletions modules/infra/control/loopback.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

#include <event2/event.h>
#include <rte_errno.h>
#include <rte_ethdev.h>
#include <rte_malloc.h>

#include <fcntl.h>
Expand Down Expand Up @@ -260,6 +261,7 @@ static int iface_loopback_init(struct iface *iface, const void * /* api_info */)

iface->flags = GR_IFACE_F_UP;
iface->state = GR_IFACE_S_RUNNING;
iface->speed = RTE_ETH_SPEED_NUM_10G;
lo->ev = event_new(
ev_base,
lo->fd,
Expand Down
5 changes: 1 addition & 4 deletions modules/infra/control/port.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -643,7 +643,6 @@ static struct event *link_event;
static void link_event_cb(evutil_socket_t, short /*what*/, void * /*priv*/) {
unsigned max_sleep_us, rx_buffer_us;
struct rte_eth_rxq_info qinfo;
struct iface_info_port *port;
struct rte_eth_link link;
struct queue_map *qmap;
struct worker *worker;
Expand All @@ -664,13 +663,11 @@ static void link_event_cb(evutil_socket_t, short /*what*/, void * /*priv*/) {
if (iface == NULL)
continue;

port = iface_info_port(iface);

if (rte_eth_link_get_nowait(qmap->port_id, &link) < 0) {
LOG(WARNING, "rte_eth_link_get_nowait: %s", strerror(rte_errno));
continue;
}
port->link_speed = link.link_speed;
iface->speed = link.link_speed;

if (link.link_status == RTE_ETH_LINK_UP) {
if (!(iface->state & GR_IFACE_S_RUNNING)) {
Expand Down
1 change: 1 addition & 0 deletions modules/infra/control/vlan.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ static int iface_vlan_reconfig(
iface_add_subinterface(next_parent, iface);
iface->state = next_parent->state;
iface->mtu = next_parent->mtu;
iface->speed = next_parent->speed;

if ((ret = rte_hash_add_key_data(vlan_hash, &next_key, iface)) < 0)
return errno_log(-ret, "rte_hash_add_key_data");
Expand Down
1 change: 1 addition & 0 deletions modules/ipip/control.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ static int iface_ipip_init(struct iface *iface, const void *api_info) {
struct gr_iface conf;
int ret;

iface->speed = RTE_ETH_SPEED_NUM_10G;
if (iface->mtu == 0)
iface->mtu = 1480;

Expand Down
4 changes: 2 additions & 2 deletions smoke/_init.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,8 +224,8 @@ if [ "$test_frr" = true ] && [ "$run_frr" = true ]; then
cat >$builddir/frr_install/etc/frr/daemons <<EOF
bgpd=yes
vtysh_enable=yes
zebra_options="-A 127.0.0.1 -s 90000000 --log file:$flog -M dplane_grout"
bgpd_options="-A 127.0.0.1 --log file:$flog"
frr_global_options="-A 127.0.0.1 --log file:$flog"
zebra_options="-s 90000000 -M dplane_grout"
Comment on lines +227 to +228
Copy link

@coderabbitai coderabbitai bot Nov 24, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify frrinit.sh integration with frr_global_options.

The new frr_global_options variable is non-standard for FRR daemons files and requires that frrinit.sh explicitly handles this variable when launching daemons. Confirm that the initialization script has been updated to apply these global options correctly.

🏁 Script executed:

#!/bin/bash
# Verify that frrinit.sh recognizes and applies frr_global_options

# Search for frr_global_options handling in frrinit.sh
rg -n 'frr_global_options' --type sh

Length of output: 152

🏁 Script executed:

#!/bin/bash
# Locate frrinit.sh and verify it handles frr_global_options from daemons config

# Find frrinit.sh
fd 'frrinit\.sh' -t f

# Search for how daemons config is sourced/used
rg -n 'source.*daemons|\..*daemons|frr_global_options' --type sh -C 2

Length of output: 775

🏁 Script executed:

#!/bin/bash
# Search for how daemons config is actually used

# Look for references to the daemons file or FRR startup
rg -n 'daemons' --type sh -C 1 | head -50

# Search for how daemon options are applied
rg -n 'zebra_options|bgpd_options' --type sh -C 2

Length of output: 1415

🏁 Script executed:

#!/bin/bash
# Check if there's any FRR documentation or scripts that handle frr_global_options
# and look for any sourcing of the daemons file

rg -n 'source|^\.' --type sh | grep -E 'daemons|frr' | head -20

# Look for any references to how options are applied to daemons
rg -n 'frr.*start|daemon.*start|exec.*frr' --type sh -C 2

Length of output: 1488

🏁 Script executed:

#!/bin/bash
# Check FRR version/requirements and whether frr_global_options is documented

fd 'frr' -t f | grep -iE 'readme|doc|version|require' | head -10

# Look for any FRR documentation in the repo
find . -type f \( -name '*frr*' -o -name 'README*' \) | head -20

Length of output: 389

🏁 Script executed:

#!/bin/bash
# Check FRR version and configuration documentation

cat subprojects/frr.wrap 2>/dev/null || echo "No frr.wrap found"

# Check if frr_global_options is mentioned in documentation
rg -n 'frr_global_options|global_options' . --type md --type txt

# Look for patches or FRR modifications
find . -type f -name '*.patch' | xargs grep -l 'global_options' 2>/dev/null || echo "No patches found"

Length of output: 188

FRR 10.4.1 does not support frr_global_options in the daemons file format.

Standard FRR only recognizes per-daemon options (e.g., zebra_options, bgpd_options). The frr_global_options variable written to the daemons config will be silently ignored unless:

  1. A custom patch to FRR is applied (not found in this repo)
  2. Per-daemon options are used instead

Verify this configuration actually applies the intended global flags to FRR daemons, or refactor to use standard FRR option syntax.

🤖 Prompt for AI Agents 
In smoke/_init.sh around lines 227-228, the script sets frr_global_options which
FRR 10.4.1 does not recognize; remove the unsupported frr_global_options
assignment and instead apply the intended flags (e.g., -A 127.0.0.1 and --log
file:$flog) to each daemon’s options (append them to zebra_options,
bgpd_options, ospfd_options, etc. as needed), then verify the resulting
per-daemon variables are written to the daemons file so the flags actually take
effect.

EOF
cat >$builddir/frr_install/etc/frr/frr.conf <<EOF
hostname grout
Expand Down
4 changes: 2 additions & 2 deletions smoke/_init_frr.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,8 +247,8 @@ start_frr_on_namespace() {
cat >${frr_namespace_folder}/daemons <<EOF
bgpd=yes
vtysh_enable=yes
zebra_options="--daemon -A 127.0.0.1 -s 90000000 --log file:$flog"
bgpd_options="--daemon -A 127.0.0.1 --log file:$flog"
frr_global_options="--daemon -A 127.0.0.1 --log file:$flog"
zebra_options="-s 90000000"
watchfrr_options="--netns=$namespace"
EOF
cat >$frr_namespace_folder/frr.conf <<EOF
Expand Down