port_output: route packets for unplugged ports to a drop node#534
port_output: route packets for unplugged ports to a drop node#534christophefontaine merged 1 commit intoDPDK:mainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe change adds a sentinel edge 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 |
modules/infra/datapath/port_output.c
Outdated
| if (unlikely(edge == RTE_EDGE_ID_INVALID)) { | ||
| // Port may have been unplugged while in-flight packets | ||
| // still reference it (e.g. during interface deletion). | ||
| rte_pktmbuf_free(mbuf); |
There was a problem hiding this comment.
Shouldn't we send these packets to a drop node ?
As of now, the packets are silently dropped, without any stat.
There was a problem hiding this comment.
The issue is that port_output has a 1:1 mapping to port_tx clones (one per txq). We cannot add other edges so we cannot send packets to a "drop" node.
This would be an ideal candidate for xstats.
rjarry
force-pushed
the
port-output-invalid
branch
2 times, most recently
from
3ab0d77 to
011d6b5
Compare
rjarry
changed the title
rjarry
force-pushed
the
port-output-invalid
branch
from
011d6b5 to
afcc804
Compare
During interface deletion, port_unplug_cb is called as a PRE_REMOVE
event subscriber and rebuilds the datapath graph without the deleted
port's TX edge. Other PRE_REMOVE subscribers (nexthop, route and address
cleanup) may run after port_unplug_cb, leaving a window where in-flight
packets still reference the deleted port via their iface pointer.
These packets eventually reach port_output and hit an invalid edge:
grout: port_output.c:33: port_output_process:
Assertion `edge != RTE_EDGE_ID_INVALID' failed.
Thread 5 (control plane):
worker_graph_reload ../modules/infra/control/graph.c:264
worker_graph_reload_all ../modules/infra/control/graph.c:365
port_unplug ../modules/infra/control/worker.c:176
iface_destroy ../modules/infra/control/iface.c:609
Thread 1 (datapath worker):
port_output_process ../modules/infra/datapath/port_output.c:33
Register a port_output_invalid drop node and append it as the last edge
of port_output. Initialize all port_output context edges to point to
that drop edge by default, so that packets destined for an unplugged
port are silently dropped instead of hitting an assertion.
Fixes: 587d5e4 ("rx,tx: use one node per port queue")
Signed-off-by: Robin Jarry <rjarry@redhat.com>
Reviewed-by: Christophe Fontaine <cfontain@redhat.com>
grout-bot
force-pushed
the
port-output-invalid
branch
from
afcc804 to
5965d78
Compare
2 participants
During interface deletion, port_unplug_cb is called as a PRE_REMOVE event subscriber and rebuilds the datapath graph without the deleted port's TX edge. Other PRE_REMOVE subscribers (nexthop, route and address cleanup) may run after port_unplug_cb, leaving a window where in-flight packets still reference the deleted port via their iface pointer.
These packets eventually reach port_output and hit an invalid edge:
Register a port_output_invalid drop node and append it as the last edge of port_output. Initialize all port_output context edges to point to that drop edge by default, so that packets destined for an unplugged port are silently dropped instead of hitting an assertion.
Fixes: 587d5e4 ("rx,tx: use one node per port queue")
Summary by CodeRabbit
Bug Fixes
Infrastructure Improvements