Skip to content

Connect to teamd before adding the lag to STATE_DB #3984

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Connect to teamd before adding the lag to STATE_DB #3984

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f9abf24
Connect to teamd before adding the lag to STATE_DB
saiarcot895 Aug 26, 2025
3dc6420
Merge branch 'master' into fix-teamdsyncd-race-condition
saiarcot895 Nov 10, 2025
f9cae00
Bug and style fixes
saiarcot895 Nov 11, 2025
8c48f08
Merge branch 'fix-teamdsyncd-race-condition' of github.com:saiarcot89…
saiarcot895 Nov 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion teamsyncd/Makefile.am
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ teamsyncd_SOURCES = teamsyncd.cpp teamsync.cpp

teamsyncd_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_ASAN)
teamsyncd_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_ASAN)
teamsyncd_LDADD = $(LDFLAGS_ASAN) -lnl-3 -lnl-route-3 -lhiredis -lswsscommon -lteam
teamsyncd_LDADD = $(LDFLAGS_ASAN) -lnl-3 -lnl-route-3 -lhiredis -lswsscommon -lteam -lteamdctl

if GCOV_ENABLED
teamsyncd_SOURCES += ../gcovpreload/gcovpreload.cpp
Expand Down
44 changes: 39 additions & 5 deletions teamsyncd/teamsync.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
#include "warm_restart.h"
#include "teamsync.h"

#include <team.h>
#include <teamdctl.h>
Comment on lines +15 to +16
Copy link

Copilot AI Nov 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maintainability: These includes are already present in teamsync.h (lines 12-13), which is included on line 13. The duplicate includes are redundant and should be removed.

Suggested change
#include <team.h>
#include <teamdctl.h>

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

@saiarcot895 saiarcot895 Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a stylistic choice to explicitly include what I'm using in this file, so that if the header files change, this file doesn't require changing.

#include <unistd.h>

using namespace std;
Expand Down Expand Up @@ -279,19 +281,51 @@ TeamSync::TeamPortSync::TeamPortSync(const string &lagName, int ifindex,
"Unable to register port change event");
}

struct teamdctl *m_teamdctl = teamdctl_alloc();
if (!m_teamdctl)
{
team_free(m_team);
m_team = NULL;
throw system_error(make_error_code(errc::address_not_available),
"Unable to allocate teamdctl socket");
}

err = teamdctl_connect(m_teamdctl, lagName.c_str(), nullptr, "usock");
if (err)
{
team_free(m_team);
m_team = NULL;
teamdctl_free(m_teamdctl);
throw system_error(make_error_code(errc::connection_refused),
"Unable to connect to teamd");
}

char *response;
err = teamdctl_config_get_raw_direct(m_teamdctl, &response);
Copy link

Copilot AI Nov 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Memory leak - the response pointer returned by teamdctl_config_get_raw_direct is not freed. According to libteamdctl documentation, the response needs to be freed using teamdctl_config_get_raw_direct_free(response) or free(response) to prevent memory leaks.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

@saiarcot895 saiarcot895 Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/jpirko/libteam/blob/master/libteamdctl/libteamdctl.c#L521-L532

if (err)
{
team_free(m_team);
m_team = NULL;
teamdctl_disconnect(m_teamdctl);
teamdctl_free(m_teamdctl);
throw system_error(make_error_code(errc::io_error),
"Unable to get config from teamd (to prove that it is running and alive)");
}

teamdctl_disconnect(m_teamdctl);
teamdctl_free(m_teamdctl);

break;
}
catch (const system_error& e)
{
SWSS_LOG_WARN("Failed to initialize team handler. LAG=%s error=%d:%s, attempt=%d",
lagName.c_str(), e.code().value(), e.what(), count);

if (++count == max_retries)
{
throw;
}
else
{
SWSS_LOG_WARN("Failed to initialize team handler. LAG=%s error=%d:%s, attempt=%d",
lagName.c_str(), e.code().value(), e.what(), count);
}

sleep(1);
}
Expand Down
Loading