Putting !ifinfo check in __team_port_str() is not nice,
I don't think it's so bad idea to be there in any case, as ifinfo is updated
from events to different socket than what's triggering teamd to call this.
Actually, I think the possibility of crash to NULL ifinfo has always
been there, with e.g. two ports unregistered at the same time and
messages arriving to two different sockets in specific order.
-antti
From: Xin Long <lucien.xin(a)gmail.com>
Sent: Wednesday, May 2, 2018 1:46 PM
To: Jiri Pirko
Cc: Tiainen, Antti; libteam(a)lists.fedorahosted.org
Subject: EXTERNAL: Re: [PATCH] libteam: don't crash when trying to print unregistered
device name
On Wed, May 2, 2018 at 6:22 PM, Jiri Pirko <jiri(a)resnulli.us> wrote:
Fri, Apr 27, 2018 at 12:02:21PM CEST, atiainen(a)forcepoint.com wrote:
>team_port_str() will crash when trying to print port name that was
>just unregistered, if dellink event is handled before port removal
>event.
>
>This is regression from Commit 046fb6ba0aec ("libteam: resynchronize
>ifinfo after lost RTNLGRP_LINK notifications"), which made it free
>all removed interfaces after ifinfo handlers are called.
>
>Put the ifinfo_destroy_removed() back to dellink/newlink handlers as
>it was before that commit. Clean up the ifinfo list after change handlers
>only if it refreshed the entire ifinfo list after lost events.
>
>There's still a rare possibility that dellink event is missed due to
>full socket receive buffer, which would cause ifinfo refresh and clearing
>removed interfaces. For this, add NULL check to team_port_str() so it
>doesn't try to print port device name in this situation.
>
>Signed-off-by: Antti Tiainen <atiainen(a)forcepoint.com>
Xin, what do you think about this patch. Is it working for you?
Putting !ifinfo
check in __team_port_str() is not nice,
but well, !ifinfo->port check in ifinfo_destroy_removed()
in the other patch is not nice either.
But both will fix the issue. it's a matter of which ugliness we can bear :)
So you decide please, either way to go is fine to me.
Thanks.