-
Notifications
You must be signed in to change notification settings - Fork 23
port: handle RTE_ETH_EVENT_INTR_RESET events #464
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.
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
Conversation
📝 WalkthroughWalkthroughThis change adds port reset handling: a new public 📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.{c,h}⚙️ CodeRabbit configuration file
Files:
🧠 Learnings (1)📓 Common learnings🔇 Additional comments (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
modules/infra/control/gr_port.h(1 hunks)modules/infra/control/port.c(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{c,h}
⚙️ CodeRabbit configuration file
**/*.{c,h}: -gr_vec_*()functions cannot fail. No need to check their return value.
gr_vec_free(x)always setsx = NULL. There is no risk of double free.ec_node_*()functions consume theirec_nodearguments. No leaks on error.rte_node->ctxis an uint8_t array of size 16, not a pointer.- Never suggest to replace
assert()with graceful error checking.- We compile with
-std=gnu2x. Unnamed parameters in function definitions are valid.
Files:
modules/infra/control/gr_port.hmodules/infra/control/port.c
🧠 Learnings (1)
📓 Common learnings
Learnt from: maxime-leroy
Repo: DPDK/grout PR: 372
File: smoke/cross_vrf_forward_test.sh:18-18
Timestamp: 2025-11-05T13:55:26.189Z
Learning: In the DPDK/grout codebase, VRF interfaces (named gr-vrf<id>) are automatically created when an interface is added to a non-existing VRF using port_add. The VRF creation is handled automatically by the event system in vrf_netlink.c, so no explicit VRF interface creation commands are needed in test scripts.
🧬 Code graph analysis (1)
modules/infra/control/port.c (3)
modules/infra/control/port_test.c (1)
gr_datapath_rcu(27-30)modules/infra/datapath/main_loop.c (1)
gr_datapath_rcu(300-302)modules/infra/control/gr_iface.h (1)
iface(16-25)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: build-and-tests (gcc-13, none, debugoptimized, ubuntu-24.04, -Dfrr=enabled, false)
- GitHub Check: build-and-tests (clang-15, none, debugoptimized, ubuntu-22.04, false)
- GitHub Check: rpm
- GitHub Check: build-and-tests (clang-18, none, debugoptimized, ubuntu-24.04, false)
- GitHub Check: build-and-tests (gcc-14, address, debug, ubuntu-24.04, -Dfrr=enabled, true)
- GitHub Check: deb
- GitHub Check: build-and-tests (clang-16, none, debugoptimized, ubuntu-24.04, false)
| struct gr_iface_info_port api = {.mac = {{0}}}; | ||
| LOG(INFO, "%s: port %u reset", iface->name, pid); | ||
| port->needs_reset = true; | ||
| iface_port_reconfig(iface, GR_PORT_SET_MAC, NULL, &api); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIUC, this will override any manually configured MAC on the port, won't it ?
On a side note, on a mac change event, do we trigger a gratuitous ARP and a NA message to tell all neighbors that the hardware address is modified ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIUC, this will override any manually configured MAC on the port, won't it ?
Yes indeed. Unfortunately, we don't have a way of knowing if the user had configured a specific mac address.
On a side note, on a mac change event, do we trigger a gratuitous ARP and a NA message to tell all neighbors that the hardware address is modified ?
We don't, but we should. Not only when there is a VF reset by the way. I can address this in a separate patch if that is OK.
Register a callback for RTE_ETH_EVENT_INTR_RESET which is raised by some DPDK drivers when the hardware requires a reset. This typically happens when the PF driver modifies VF settings from the host side, for example when changing a VF MAC address via ip link: ip link set $pf vf $vf_num mac $mac When the reset event is received, the port is fully reset and reconfigured via rte_eth_dev_reset() followed by rte_eth_dev_configure() and rte_eth_dev_start(). The MAC address is re-read from hardware to pick up any changes made by the PF. The DPDK callback may be invoked from internal threads and multiple reset events can occur before the main event loop processes them. To handle this, port IDs are queued in a mutex-protected vector and processed in batch when the libevent callback fires. Note that RTE_ETH_EVENT_INTR_RESET support varies by driver. The Intel iavf driver reports this event when the VF MAC is changed from the PF. The mlx5 driver does not report this event in the same scenario. Signed-off-by: Robin Jarry <[email protected]> Reviewed-by: Christophe Fontaine <[email protected]>
Register a callback for RTE_ETH_EVENT_INTR_RESET which is raised by some DPDK drivers when the hardware requires a reset. This typically happens when the PF driver modifies VF settings from the host side, for example when changing a VF MAC address via ip link:
When the reset event is received, the port is fully reset and reconfigured via rte_eth_dev_reset() followed by rte_eth_dev_configure() and rte_eth_dev_start(). The MAC address is re-read from hardware to pick up any changes made by the PF.
The callback is invoked from DPDK internal threads, so we use event_active() to defer the actual reset handling to the main control plane event loop.
Note that RTE_ETH_EVENT_INTR_RESET support varies by driver. The Intel iavf driver reports this event when the VF MAC is changed from the PF. The mlx5 driver does not report this event in the same scenario.