On Thu, Feb 16, 2017 at 5:39 PM, Jiri Pirko <jiri(a)resnulli.us> wrote:
Tue, Feb 07, 2017 at 05:03:32PM CET, lucien.xin(a)gmail.com wrote:
>I found this err in "loadbalance" and "lacp" runner when adding
ports.
>It's caused by trying to set "enabled" option in .port_link_changed()
>or .port_changed().
>
>When a new port is added, the first 'port changed event' process is
>earlier than CMD TEAM_CMD_OPTIONS_GET, in this CMD, all
>the options are synchronized from kernel.
>
>It means there's no 'enabled' option yet when calling port_link_changed
>in the first 'port changed event' process. In
lb_event_watch_port_link_changed
>and lacp_event_watch_port_changed, they call teamd_port_check_enable
>to set 'enabled' option. this err is triggered.
>
>I'm not sure why teamd_port_check_enable needs to check if
>'enabled' option exists. I checked the ab's .port_link_changed(),
>it just sets it by calling team_set_port_enabled(), instead of
>checking 'enabled' option first.
So if the teamd_port_enabled fails, you would assume
curr_enabled_state == false
and continue on?
I'm thinking why ab runner doesn't need to check if
'enabled' option exists
before setting it, ab's .port_link_changed just calls team_set_port_enabled
directly.
can't we just do it for lb and lacp runner ? is there any special reason
we should check if 'enabled' option exists first ?
>
> I would add a teamd_log_dbg print if that happens, but I don't see any
> problem in that.
>
>
>
>>
>>can we just use team_set_port_enabled to set it directly in
>>.port(_link)_changed OR improve teamd_port_check_enable
>>to avoid this err ?
>>
>>Thanks.