On Mon, Sep 14, 2020 at 4:06 PM Jiri Pirko <jiri(a)resnulli.us> wrote:
Tue, Sep 01, 2020 at 07:59:27AM CEST, lucien.xin(a)gmail.com wrote:
>This reverts commit deadb5b715227429a1879b187f5906b39151eca9.
>
>As Patrick noticed, with that commit, teamd_port_check_enable()
>would set the team port to the new state unconditionally, which
>triggers another change message from kernel to userspace, then
>teamd_port_check_enable() is called again to set the team port
>to the new state.
>
>This would go around and around to update the team port state,
>and even cause teamd to consume 100% cpu.
>
>As the issue caused by that commit is serious, it has to be
>reverted. As for the issued fixed by that commit, I would
>propose a new fix later.
Hi Xin. Any progress with the fix? I would like to do a libteam release
soon but I'm happy to wait couple of days if the fix is in pipes.
Hi, Jiri,
I could never reproduce this issue by:
# ./mirror_gre_lag_lacp.sh veth{5..12}
especially after the kernel patch:
commit 1c0522b4a2e143fa6e55e4bd2308415c81184ec7
Author: Petr Machata <petrm at mellanox.com>
Date: Fri May 29 14:16:53 2020 +0300
selftests: forwarding: mirror_lib: Use mausezahn
I will need to add some code to try to reproduce it.
Give me another 2 days, I will let you know soon.
Thanks.
>
> Thanks!
>
>
> >---
> > teamd/teamd_per_port.c | 8 ++++++--
> > 1 file changed, 6 insertions(+), 2 deletions(-)
> >
> >diff --git a/teamd/teamd_per_port.c b/teamd/teamd_per_port.c
> >index 166da57..d429753 100644
> >--- a/teamd/teamd_per_port.c
> >+++ b/teamd/teamd_per_port.c
> >@@ -442,14 +442,18 @@ int teamd_port_check_enable(struct teamd_context *ctx,
> > bool should_enable, bool should_disable)
> > {
> > bool new_enabled_state;
> >+ bool curr_enabled_state;
> > int err;
> >
> > if (!teamd_port_present(ctx, tdport))
> > return 0;
> >+ err = teamd_port_enabled(ctx, tdport, &curr_enabled_state);
> >+ if (err)
> >+ return err;
> >
> >- if (should_enable)
> >+ if (!curr_enabled_state && should_enable)
> > new_enabled_state = true;
> >- else if (should_disable)
> >+ else if (curr_enabled_state && should_disable)
> > new_enabled_state = false;
> > else
> > return 0;
> >--
> >2.1.0
> >