On 1 Jun 2026, at 16:12, Eli Britstein wrote:
> On 01/06/2026 16:40, Eelco Chaudron wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> On 1 Jun 2026, at 15:09, Eli Britstein wrote:
>>
>>> On 01/06/2026 15:21, Eelco Chaudron wrote:
>>>> External email: Use caution opening links or attachments
>>>>
>>>>
>>>> On 24 May 2026, at 9:55, Eli Britstein wrote:
>>>>
>>>>> Upon 'ip link set <old> name <new>' operation, <old> might be added
>>>>> as an altname to <new>. Remove it. Ignore failures not to fail the
>>>>> test in case it doesn't.
>>>> Hi Eli,
>>>>
>>>> Some small comments below, which I can apply on commit.
>>>> Let me know your thoughts.
>>>>
>>>> //Eelco
>>>>
>>>>> Fixes: 289e9f6baa7c ("tests: Add a simple DPDK rte_flow test framework.")
>>>>> Signed-off-by: Eli Britstein <[email protected]>
>>>>> ---
>>>>> tests/system-dpdk-offloads-macros.at | 2 ++
>>>>> utilities/checkpatch_dict.txt | 1 +
>>>>> 2 files changed, 3 insertions(+)
>>>>>
>>>>> diff --git a/tests/system-dpdk-offloads-macros.at
>>>>> b/tests/system-dpdk-offloads-macros.at
>>>>> index 3c6cce1a8..050521f36 100644
>>>>> --- a/tests/system-dpdk-offloads-macros.at
>>>>> +++ b/tests/system-dpdk-offloads-macros.at
>>>>> @@ -111,6 +111,7 @@ m4_define([ADD_VF],
>>>>> AT_CHECK([ip link set $VF down])
>>>>> AT_CHECK([ip link set $VF netns $2])
>>>>> AT_CHECK([ip netns exec $2 ip link set $VF name $1 && printf
>>>>> '%s\n' "$VF" > ORIG_$1])
>>>>> + AT_CHECK([ip netns exec $2 ip link property del dev $1 altname $VF
>>>>> 2>/dev/null || true])
>>>> The '|| true' does not make sense in combination with AT_CHECK().
>>>> What about:
>>>>
>>>> AT_CHECK([ip netns exec $2 ip link property del dev $1 altname
>>>> $VF],
>>>> [ignore], [], [ignore])
>>> LGTM
>>>>> AT_CHECK([ovs-vsctl add-port $3 ovs-$1 -- \
>>>>> set interface ovs-$1 external-ids:iface-id="$1" -- \
>>>>> set interface ovs-$1 type=dpdk -- \
>>>>> @@ -131,6 +132,7 @@ m4_define([ADD_VF],
>>>>> rm -f ORIG_$1; \
>>>>> ip netns exec $2 ip link set $1 down; \
>>>>> ip netns exec $2 ip link set $1 name \"\$orig\"; \
>>>>> + ip netns exec $2 ip link property del dev \"\$orig\"
>>>>> altname $1 2>/dev/null || true; \
>>>> We should remove the 2>/dev/null part, as it might give us
>>>> some hints if something is really wrong (which I had when
>>>> checking the patch). So:
>>>>
>>>> - ip netns exec $2 ip link property del dev \"\$orig\"
>>>> altname $1 2>/dev/null || true; \
>>>> + ip netns exec $2 ip link property del dev \"\$orig\"
>>>> altname $1 || true; \
>>> I think normally there wouldn't be any altname, so failure to remove it is
>>> not "really wrong".
>>>
>>> If we remove it, we will (probably) always get an "error" there which might
>>> indicate to the user something is wrong, while it isn't.
>>>
>>> As I wrote in the cover letter, I'm not even sure it is possible there will
>>> ever be an altname to remove and this is just "to make sure".
>>>
>>> If you think we should take the "just in case" approach, please let me know
>>> if you want me to send v4 (or you'd change on apply). If not, we can
>>> abandon this specific commit.
>> Ok, I thought you were encountering this issue and that was why you
>> added this patch to the series. I wasn't able to reproduce it on RHEL,
>> Fedora, or Ubuntu. So if you do not see the problem either, I would
>> just drop this patch from the series.
>>
>> If you agree, there is no need for a v4. I can just apply the first two
>> patches.
Thanks for the series Eli!
I have applied the change of the first two patches to main.
//Eelco
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
