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.
//Eelco
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
