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.
Yes. Let's drop it. I searched further. I saw that even if there is a udev rule to make an altname from the old name, it would not be applied inside the namespace (after patch #2).

//Eelco

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to