Connect to teamd before adding the lag to STATE_DB #3984
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,6 +12,8 @@ | |
| #include "warm_restart.h" | ||
| #include "teamsync.h" | ||
|
|
||
| #include <team.h> | ||
| #include <teamdctl.h> | ||
| #include <unistd.h> | ||
|
|
||
| using namespace std; | ||
|
|
@@ -279,19 +281,51 @@ TeamSync::TeamPortSync::TeamPortSync(const string &lagName, int ifindex, | |
| "Unable to register port change event"); | ||
| } | ||
|
|
||
| struct teamdctl *m_teamdctl = teamdctl_alloc(); | ||
|
Contributor
There was a problem hiding this comment. If teamd exits, shouldn't we get netlink for interface down, as we registered earlier for netlink events in teamsyncd ? netlink.registerGroup(RTNLGRP_LINK);
Contributor
Author
There was a problem hiding this comment. We will, but by the time we get the netlink event, portsorch may have already started processing the port channel interface creation. |
||
| 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"); | ||
judyjoseph marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| 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); | ||
|
There was a problem hiding this comment. Bug: Memory leak - the
Contributor
Author
There was a problem hiding this comment. |
||
| 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; | ||
| } | ||
|
|
||
| sleep(1); | ||
| } | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,185 @@ | ||
| #include "gmock/gmock.h" | ||
| #include "gtest/gtest.h" | ||
| #include <dlfcn.h> | ||
| #include <stdexcept> | ||
| #include <team.h> | ||
| #include <teamdctl.h> | ||
| #include "teamsync.h" | ||
|
|
||
| static unsigned int (*callback_sleep)(unsigned int seconds) = NULL; | ||
| static int (*callback_team_init)(struct team_handle *th, uint32_t ifindex) = NULL; | ||
| static int (*callback_team_change_handler)(struct team_handle *th, struct team_change_handler *handler, void *priv) = NULL; | ||
| static int (*callback_teamdctl_connect)(struct teamdctl *tdc, const char *team_name, const char *addr, const char *cli_type) = NULL; | ||
| static int (*callback_teamdctl_config_get_raw_direct)(struct teamdctl *tdc, char **response) = NULL; | ||
| static void (*callback_teamdctl_disconnect)(struct teamdctl *tdc) = NULL; | ||
|
|
||
| static unsigned int cb_sleep(unsigned int seconds) | ||
| { | ||
| return 0; | ||
| } | ||
|
|
||
| unsigned int sleep(unsigned int seconds) | ||
| { | ||
| if (callback_sleep) | ||
| { | ||
| return callback_sleep(seconds); | ||
| } | ||
| unsigned int (*realfunc)(unsigned int) = | ||
| (unsigned int (*)(unsigned int))(dlsym (RTLD_NEXT, "sleep")); | ||
| return realfunc(seconds); | ||
| } | ||
|
|
||
|
|
||
| static int cb_team_init(struct team_handle *th, uint32_t ifindex) | ||
| { | ||
| return 0; | ||
| } | ||
|
|
||
| int team_init(struct team_handle *th, uint32_t ifindex) | ||
| { | ||
| if (callback_team_init) | ||
| { | ||
| return callback_team_init(th, ifindex); | ||
| } | ||
| int (*realfunc)(struct team_handle *, uint32_t) = | ||
| (int (*)(struct team_handle *, uint32_t))(dlsym (RTLD_NEXT, "team_init")); | ||
| return realfunc(th, ifindex); | ||
| } | ||
|
|
||
| static int cb_team_change_handler(struct team_handle *th, struct team_change_handler *handler, void *priv) | ||
| { | ||
| return 0; | ||
| } | ||
|
|
||
| int team_change_handler(struct team_handle *th, struct team_change_handler *handler, void *priv) | ||
| { | ||
| if (callback_team_change_handler) | ||
| { | ||
| return callback_team_change_handler(th, handler, priv); | ||
| } | ||
| int (*realfunc)(struct team_handle *, struct team_change_handler*, void*) = | ||
| (int (*)(struct team_handle *, struct team_change_handler*, void*))(dlsym (RTLD_NEXT, "team_change_handler")); | ||
| return realfunc(th, handler, priv); | ||
| } | ||
|
|
||
| static int cb_teamdctl_connect(struct teamdctl *tdc, const char *team_name, const char *addr, const char *cli_type) | ||
| { | ||
| return 0; | ||
| } | ||
|
|
||
| int teamdctl_connect(struct teamdctl *tdc, const char *team_name, const char *addr, const char *cli_type) | ||
| { | ||
| if (callback_teamdctl_connect) | ||
| { | ||
| return callback_teamdctl_connect(tdc, team_name, addr, cli_type); | ||
| } | ||
| int (*realfunc)(struct teamdctl *, const char *, const char *, const char *) = | ||
| (int (*)(struct teamdctl *, const char *, const char *, const char *))(dlsym (RTLD_NEXT, "teamdctl_connect")); | ||
| return realfunc(tdc, team_name, addr, cli_type); | ||
| } | ||
|
|
||
| static int cb_teamdctl_config_get_raw_direct_force_error(struct teamdctl *tdc, char **response) | ||
| { | ||
| // Forced error | ||
| return 1; | ||
| } | ||
|
|
||
| static int cb_teamdctl_config_get_raw_direct_success(struct teamdctl *tdc, char **response) | ||
| { | ||
| return 0; | ||
| } | ||
|
|
||
| int teamdctl_config_get_raw_direct(struct teamdctl *tdc, char **response) | ||
| { | ||
| if (callback_teamdctl_config_get_raw_direct) | ||
| { | ||
| return callback_teamdctl_config_get_raw_direct(tdc, response); | ||
| } | ||
| int (*realfunc)(struct teamdctl *, char **) = | ||
| (int (*)(struct teamdctl *, char **))(dlsym (RTLD_NEXT, "teamdctl_config_get_raw_direct")); | ||
| return realfunc(tdc, response); | ||
| } | ||
|
|
||
| static void cb_teamdctl_disconnect(struct teamdctl *tdc) | ||
| { | ||
| } | ||
|
|
||
| void teamdctl_disconnect(struct teamdctl *tdc) | ||
| { | ||
| if (callback_teamdctl_disconnect) | ||
| { | ||
| callback_teamdctl_disconnect(tdc); | ||
| return; | ||
| } | ||
| int (*realfunc)(struct teamdctl *) = | ||
| (int (*)(struct teamdctl *))(dlsym (RTLD_NEXT, "teamdctl_disconnect")); | ||
| realfunc(tdc); | ||
| } | ||
|
|
||
| namespace teamportsync_test | ||
| { | ||
| struct TeamPortSyncTest : public ::testing::Test | ||
| { | ||
| virtual void SetUp() override | ||
| { | ||
| callback_sleep = cb_sleep; | ||
| callback_team_init = NULL; | ||
| callback_team_change_handler = NULL; | ||
| callback_teamdctl_connect = NULL; | ||
| callback_teamdctl_config_get_raw_direct = cb_teamdctl_config_get_raw_direct_force_error; | ||
| callback_teamdctl_disconnect = cb_teamdctl_disconnect; | ||
| } | ||
|
|
||
| virtual void TearDown() override | ||
| { | ||
| callback_sleep = NULL; | ||
| callback_team_init = NULL; | ||
| callback_team_change_handler = NULL; | ||
| callback_teamdctl_connect = NULL; | ||
| callback_teamdctl_config_get_raw_direct = NULL; | ||
| callback_teamdctl_disconnect = NULL; | ||
| } | ||
| }; | ||
|
|
||
| TEST_F(TeamPortSyncTest, TestInvalidIfIndex) | ||
| { | ||
| try { | ||
| swss::TeamSync::TeamPortSync("testLag", 0, NULL); | ||
| FAIL(); | ||
| } catch (std::runtime_error &exception) { | ||
| EXPECT_THAT(exception.what(), testing::HasSubstr("Unable to initialize team socket")); | ||
| } | ||
| } | ||
|
|
||
| TEST_F(TeamPortSyncTest, NoLagPresent) | ||
| { | ||
| try { | ||
| swss::TeamSync::TeamPortSync("testLag", 4, NULL); | ||
| FAIL(); | ||
| } catch (std::runtime_error &exception) { | ||
| EXPECT_THAT(exception.what(), testing::HasSubstr("Unable to initialize team socket")); | ||
| } | ||
| } | ||
|
|
||
| TEST_F(TeamPortSyncTest, TeamdctlNoConfig) | ||
| { | ||
| callback_team_init = cb_team_init; | ||
| callback_team_change_handler = cb_team_change_handler; | ||
| callback_teamdctl_connect = cb_teamdctl_connect; | ||
| try { | ||
| swss::TeamSync::TeamPortSync("testLag", 4, NULL); | ||
| FAIL(); | ||
| } catch (std::runtime_error &exception) { | ||
| EXPECT_THAT(exception.what(), testing::HasSubstr("Unable to get config from teamd")); | ||
| } | ||
| } | ||
|
|
||
| TEST_F(TeamPortSyncTest, AllSuccess) | ||
| { | ||
| callback_team_init = cb_team_init; | ||
| callback_team_change_handler = cb_team_change_handler; | ||
| callback_teamdctl_connect = cb_teamdctl_connect; | ||
| callback_teamdctl_config_get_raw_direct = cb_teamdctl_config_get_raw_direct_success; | ||
| swss::TeamSync::TeamPortSync("testLag", 4, NULL); | ||
| } | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.