External email: Use caution opening links or attachments
On 5/18/26 5:04 PM, Reuven Plevinsky via dev wrote:
The kernel datapath cannot hold ports whose names are IFNAMSIZ bytes
or longer, so dpif_netlink_port_query_by_name() can never find them.
During restart the ofproto construct path calls dpif_port_exists() for
every configured interface. For ports with long names (for example
patch ports) this reached the kernel, which returned an error that was
logged as a spurious "failed to query port <long_port_name>: Invalid
argument" warning.
Return ENODEV early when the name is too long, avoiding the round-trip
and the misleading log message.
Fixes: c19e653509de ("datapath: Change userspace vport interface to use Netlink
attributes.")
Signed-off-by: Reuven Plevinsky <[email protected]>
Hi, Reuven. This patch seems AI-generated (mainly judging by the comments).
If that's the case, you need to disclose that with the appropriate tags,
see the contribution guide.
---
lib/dpif-netlink.c | 4 ++++
tests/system-interface.at | 31 +++++++++++++++++++++++++++++++
2 files changed, 35 insertions(+)
diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index f22a87934089..0f2bc5e7eed2 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -1263,6 +1263,10 @@ dpif_netlink_port_query_by_name(const struct dpif
*dpif_, const char *devname,
{
struct dpif_netlink *dpif = dpif_netlink_cast(dpif_);
+ if (strlen(devname) >= IFNAMSIZ) {
+ return ENODEV;
+ }
+
return dpif_netlink_port_query__(dpif, 0, devname, dpif_port);
}
diff --git a/tests/system-interface.at b/tests/system-interface.at
index 20a882d1cfe6..e7b11773fe90 100644
--- a/tests/system-interface.at
+++ b/tests/system-interface.at
@@ -231,3 +231,34 @@ AT_CHECK([grep 'name too long' stderr], [0], [ignore])
OVS_TRAFFIC_VSWITCHD_STOP(["/could not open network device.*name too long/d"])
AT_CLEANUP
+
+AT_SETUP([interface - patch port name exceeds IFNAMSIZ on vswitchd restart])
+
+OVS_TRAFFIC_VSWITCHD_START()
+
+dnl Kernel datapath only (see system-kmod-testsuite).
system-userspace-testsuite
+dnl also includes this file but uses netdev datapath — same pattern as "datapath
ports gc".
This comment includes a lot of unnecessary details and generally pointless.
use a short comment as other tests do.
+AT_SKIP_IF([! ovs-appctl dpctl/show 2>/dev/null | grep -q ovs-system])
+
+dnl Port names longer than IFNAMSIZ-1 (Linux). Uses kernel (system) datapath
— see
+dnl system-kmod-macros _ADD_BR / _OVS_VSWITCHD_START (no --disable-system).
This is hard to understand and mostly pointless as well. Also, avoid the
em-dashes.
+AT_CHECK([ovs-vsctl -- \
+ add-br br1 -- set bridge br1 fail-mode=secure \
+ -- add-port br0 long_patch_peer_one \
+ -- set interface long_patch_peer_one type=patch \
+ options:peer=long_patch_peer_two \
+ -- add-port br1 long_patch_peer_two \
+ -- set interface long_patch_peer_two type=patch \
+ options:peer=long_patch_peer_one])
+
+dnl Restart ovs-vswitchd with a fresh ovs-vswitchd.log
+OVS_APP_EXIT_AND_WAIT([ovs-vswitchd])
+mv ovs-vswitchd.log ovs-vswitchd_1.log
Why removing the old one?
+AT_CHECK([ovs-vswitchd --detach --no-chdir --pidfile --log-file -vvconn
-vofproto_dpif -vunixctl],
+ [0], [], [stderr])
+OVS_WAIT_UNTIL([ovs-appctl -t ovs-vswitchd version 2>/dev/null | grep -q 'Open
vSwitch'])
Not sure this is needed.
+
+AT_CHECK([grep -E 'failed to query port.*Invalid argument' ovs-vswitchd.log],
[1], [], [])
This is also not needed, the warnings will be checked at the end. Just add a
comment.
Looking for specific warnings is unreliable.
+
+OVS_TRAFFIC_VSWITCHD_STOP
+AT_CLEANUP
