On 2/9/26 4:49 PM, Eli Britstein wrote:
> 
> On 09/02/2026 17:44, Ilya Maximets wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> On 2/9/26 4:19 PM, Eli Britstein wrote:
>>> A linux interface is limited to IFNAMSIZ length (16). In case a tap
>>> interface is created, add a validity check for it.
>> Hi, Eli.
>>
>> OpenFlow and Linux have the same limit for the port name.
>> How is it possible to create a port that is larger?
> ovs-vsctl add-br a234567890b234567890 -- set br a234567890b234567890 
> datapath_type=netdev

OK, yeah.  I was a little confused, this makes sense.  The datapath
and the OpenFlow are a little decoupled here, so yes, it is possible
to try and create a port with a longer name.  However, this actually
seem to work fine for tap.  The name of the system interface will
just be truncated during creation process, but the file descriptor is
open and everything should function normally.  There is one thing that
doesn't take into account the potential truncation, which is the
netlink request formation in dpif_netlink_vport_to_ofpbuf() and so
OVS can't get stats and some other things from the kernel.

It's a little outside of the documentation, I agree, that states that
Linux port names are limited to 15 bytes, and the truncation behavior
is only documented for the OpenFlow interface, so we may indeed just
deny creation of Linux interfaces with too long names for now, until
we have support for alternative names maybe.

>>
>>> Signed-off-by: Eli Britstein <[email protected]>
>>> ---
>>>   lib/netdev-linux.c |  7 +++++++
>>>   tests/ovs-ofctl.at | 21 +--------------------
>>>   2 files changed, 8 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
>>> index f4d2685a2..32222b683 100644
>>> --- a/lib/netdev-linux.c
>>> +++ b/lib/netdev-linux.c
>>> @@ -962,6 +962,13 @@ netdev_linux_common_construct(struct netdev *netdev_)
>>>                        name);
>>>           return EINVAL;
>>>       }
>>> +    if (strlen(name) >= IFNAMSIZ) {
>>> +        static struct vlog_rate_limit rll = VLOG_RATE_LIMIT_INIT(1, 1);
>>> +
>>> +        VLOG_WARN_RL(&rll, "%s: Linux forbids network device with this 
>>> name "
>>> +                     "(too long)", name);

While we need to explain EINVAL, ENAMETOOLONG seems self-explanatory,
and we will have the failure log from the higher level:

  bridge|WARN|could not open network device a234567890b234567890 (File name too 
long)

So, the extra log seems a bit unnecessary.  It is also worded a bit strangely
for this use case.

>>> +        return ENAMETOOLONG;
>>> +    }
>>>
>>>       /* The device could be in the same network namespace or in another 
>>> one. */
>>>       netnsid_unset(&netdev->netnsid);
>>> diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at
>>> index 751a934e4..871bdcc19 100644
>>> --- a/tests/ovs-ofctl.at
>>> +++ b/tests/ovs-ofctl.at
>>> @@ -3105,33 +3105,14 @@ AT_KEYWORDS([port names])
>>>   OVS_VSWITCHD_START([\
>>>       -- add-port br0 xyzzy -- set Interface xyzzy type=dummy -- \
>>>       -- add-port br0 x-y -- set Interface x-y type=dummy -- \
>>> -    -- add-port br0 abc123 -- set Interface abc123 type=dummy -- \
>>> -    -- add-port br0 reallyverylongportname -- set Interface 
>>> reallyverylongportname type=dummy -- \
>>> -    -- add-port br0 conflictinglongportname1 -- set Interface 
>>> conflictinglongportname1 type=dummy -- \
>>> -    -- add-port br0 conflictinglongportname2 -- set Interface 
>>> conflictinglongportname2 type=dummy])
>>> +    -- add-port br0 abc123 -- set Interface abc123 type=dummy])
>>>
>>>   # These plain port names should be accepted.
>>>   AT_CHECK([ovs-ofctl add-flow br0 in_port=xyzzy,actions=x-y,abc123])
>>>
>>> -# reallyverylongportname is accepted truncated, but not in full.
>>> -AT_CHECK([ovs-ofctl add-flow br0 in_port=reallyverylongp,actions=drop])
>>> -AT_CHECK([ovs-ofctl add-flow br0 
>>> in_port=reallyverylongportname,actions=drop],
>>> -  [1], [], [ovs-ofctl: reallyverylongportname: invalid or unknown port for 
>>> in_port
>>> -])
>>> -
>>> -# conflictinglongportname1 and 2 can't be accepted even truncated, since
>>> -# they conflict when truncated.
>>> -AT_CHECK([ovs-ofctl add-flow br0 
>>> in_port=conflictinglongportname1,actions=drop], [1], [], [ovs-ofctl: 
>>> conflictinglongportname1: invalid or unknown port for in_port
>>> -])
>>> -AT_CHECK([ovs-ofctl add-flow br0 
>>> in_port=conflictinglongportname2,actions=drop], [1], [], [ovs-ofctl: 
>>> conflictinglongportname2: invalid or unknown port for in_port
>>> -])
>>> -AT_CHECK([ovs-ofctl add-flow br0 in_port=conflictinglong,actions=drop], 
>>> [1], [], [ovs-ofctl: conflictinglong: invalid or unknown port for in_port
>>> -])
>>> -
>>>   # Show that the port names get displayed properly and that port names that
>>>   # aren't alphanumeric get quoted.
>>>   AT_CHECK([ovs-ofctl --names dump-flows br0 | ofctl_strip | sort], [0], 
>>> [dnl
>>> - in_port=reallyverylongp actions=drop
>>>    in_port=xyzzy actions=output:"x-y",output:abc123
>>>   ])
>>>   OVS_VSWITCHD_STOP
>> This test seems completely valid to me.  It should stay as is.
> Since I add this validity check, deliberately creating too long names 
> for checking truncating seems wrong to me, as they would fail if not dummy.

The port name length is datapath-specific.  Windows (though deprecated)
supports lengths up to 255.  Also, most dpdk devices will not care
about the port name length.  Bond and patch port name lengths are not
limited by any datapath restriction, as they never appear in the
datapath.  So, it's a valid test case that checks the documented behavior
of port name truncation in OpenFlow messages.  It's unrelated to the
datapath implementation.  And hence it should stay.

I'd suggest adding a system test to tests/system-interface.at that
checks the port name length.

Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to