[PATCHv2] libteam: do not destroy the ifinfo of current unregistered slave dev

Commit Message

Tiainen, Antti April 23, 2018, 12:01 p.m. UTC

Strange, it doesn't crash for me if I try your example. But if I try with your patch, the deleted interface just hangs there in ifinfo list forever, even when change handlers are called.


If this is only a debug print issue, it cannot do simply NULL check in there, and print something (or nothing) for the interface that's completely gone?


-antti

Comments

Xin Long April 23, 2018, 12:15 p.m. UTC | #1

On Mon, Apr 23, 2018 at 8:01 PM, Tiainen, Antti <[e-mail]> wrote:
> Strange, it doesn't crash for me if I try your example. But if I try with

> your patch, the deleted interface just hangs there in ifinfo list forever,

> even when change handlers are called.

>

>

> If this is only a debug print issue, it cannot do simply NULL check in

> there, and print something (or nothing) for the interface that's completely

> gone?

I don't think so, as said in the changelog, that's the dbg log system
which delays the destruction, so that it can show a nice info to users
when removing a port.
Not only here like that, but also in the team_port's destruction.

"""
  ...
  <port_list>
  -1450: veth1: down 0Mbit HD <--
  </port_list>
  ...

The similar process is also used on the team_port's destruction.
"""

So far, that's a 'not bad' way I could see, maybe you have a better fix
for this.

Patch

diff --git a/libteam/ifinfo.c b/libteam/ifinfo.c
index 5c32a9c..d47c2bf 100644
--- a/libteam/ifinfo.c
+++ b/libteam/ifinfo.c
@@ -211,7 +211,8 @@  void ifinfo_clear_changed(struct team_handle *th)
         struct team_ifinfo *ifinfo;

         list_for_each_node_entry(ifinfo, &th->ifinfo_list, list)
-               clear_changed(ifinfo);
+               if (!ifinfo->port)
+                       clear_changed(ifinfo);
 }

 static struct team_ifinfo *ifinfo_find_create(struct team_handle *th,
@@ -245,7 +246,7 @@  void ifinfo_destroy_removed(struct team_handle *th)
         struct team_ifinfo *ifinfo, *tmp;

         list_for_each_node_entry_safe(ifinfo, tmp, &th->ifinfo_list, list) {
-               if (is_changed(ifinfo, CHANGED_REMOVED))
+               if (is_changed(ifinfo, CHANGED_REMOVED) && !ifinfo->port)
                         ifinfo_destroy(ifinfo);
         }
 }