[teamd] Add Warm-reboot startup and shutdown mode for teamd #2173

pavel-shirshov · 2018-10-20T02:10:46Z

I'll add teamd WB mode integrations into this PR and sonic-utilities as soon as I've tested this patch with the read testbed.
To implement WB make sure that.

You start teamd with -L option with a valid directory.
You stop it with USR1 signal
You start teamd with -w flag for WB mode
You make admin up for portchannel interface, after teamd starts

- What I did
Added:

Warm-boot shutdown mode to allow teamd saving information to restart its state on Warm-boot startup
Warm-boot startup mode. To allow start without resynchronization with the partner.

- How I did it

I've added SIGUSR1 signal to request teamd stopping in WB mode. When teamd receive this signal it will:
1. save last received LACP frame for every LAG member port.
2. send last lacp update to its neighbors
3. exit, keeping both portchannel and LAG member ports active
When teamd starts in WB mode it will read saved state from the disk and start from the same state it was before the WR.
For that I've added two extra parameters to teamd:
w - WR mode
L - directory where lacp frames will be saved.

- How to verify it

Build and install everything on your DUT. Make sure that you used -L option with a valid directory
Restart your DUT and check that LAG interface is up and running
send -USR1 signal to your teamd: pkill -USR1 teamd
Check that teamd send last LACP frame to its neighbors
Check that both LAG and member LAG interface still running
Check that lacp frames were saved under -L directory
restart box using kexec
start your teamd with -w -L flags. Ensure that teamd started in "current" state, not "expired" state.

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

jipanyang

Is the change able to support teamd docker warm restart too?
In case of teamd docker warm restart , the port channel in Linux kernel should not be touched to avoid affecting all the configuration on LAG.

jipanyang · 2018-10-20T20:16:58Z

src/libteam/0005-libteam-Add-warm_reboot-mode.patch

+	/* Read data from file and process it */
+	if (ctx->warm_reboot && ctx->lacp_directory) {
+		(void)lacpdu_read(lacp_port);
+	}


When will the dumped data file for each port be removed? erase after read or upon port remove, or outside of the teamd process? Leaving the file permanently there might cause confusion if there are multiple reboot/restart (Mixed warm/cold).

it should be outside program to remove the file, teamd already close it and read it.

My design is to remove the files right after the read. The information in the file is outdated right after the read.

Addressed in the latest patch

jipanyang · 2018-10-20T20:25:11Z

src/libteam/0005-libteam-Add-warm_reboot-mode.patch

+ 	struct lacp_port *agg_lead; /* leading port of aggregator.
+ 				     * NULL in case this port is not selected */
+ 	enum lacp_port_state state;
+	bool lacpdu_received;


Didn't see it being used.

Yes. You're absolutely right. I've found this issue too, but didn't have a chance to fix it. I'm going to fix it on the next iteration.

lguohan · 2018-10-30T09:32:07Z

src/libteam/0005-libteam-Add-warm_reboot-mode.patch

+	struct lacp_port *lacp_port = lacp_port_get(lacp, tdport);
+	if(lacp_port->lacpdu_saved && lacp_port->ctx->lacp_directory) {
+		char filename[PATH_MAX];
+		strcpy(filename, lacp_port->ctx->lacp_directory);


in case the directory is not ended with '/', you may want to add here.

lguohan · 2018-10-30T09:34:37Z

src/libteam/0005-libteam-Add-warm_reboot-mode.patch

+			ctx->warm_reboot = true;
+			break;
+		case 'L':
+			ctx->lacp_directory = strdup(optarg);


can you make sure the directory exist here? we do not want to get error msg when we try to dump the lacp pdu to disk, if directory does not exist, we'd like to know the error msg early.

What to do after the error message? exit?

Fixed. Just error message, no exit

lguohan · 2018-10-30T09:46:36Z

@jipanyang , teamd docker warm restart is not planned. the scenario for teamd docker warm restart is not clearly defined, should we remove teamd netdev in this case or not? are we assuming bgp connection not affected at all in this case, or are we assuming bgp docker will be warm restart as well?

lguohan · 2018-10-30T09:47:19Z

we need to add some test in the vs test to test teamd warm restart.

lguohan

as comments.

jipanyang · 2018-10-30T16:23:43Z

@lguohan I think the ideal teamd docker warm restart scenario is clear: Teamd software upgrade(teamd, teamsyncd, teammgrd), self-contained, no impact on data plane or other dockers including bgp.

It may not be in current phase of development, while doing the implementation it will be good to keep that in mind whenever possible.

qiluo-msft · 2018-10-31T21:53:30Z

src/libteam/0005-libteam-Add-warm_reboot-mode.patch

+ 	char			ifname[IFNAMSIZ];
+ 	uint32_t		master_ifindex;
+ 	bool			admin_state;
+        bool                    orig_addr_updated; /* FIXME: Check this. I think we don't need this flag */


[](start = 1, length = 8)

Better follow existing code's indentation style. #Closed

this chunk was removed. It was needed only if netdev wasn't removed

qiluo-msft · 2018-10-31T22:49:54Z

src/libteam/0005-libteam-Add-warm_reboot-mode.patch

+             "    -U --usock-enable        Enable UNIX domain socket interface\n"
+-            "    -u --usock-disable       Disable UNIX domain socket interface\n",
+            "    -u --usock-disable       Disable UNIX domain socket interface\n"
+            "    -w --warm-reboot         Warm-reboot startup mode\n"


warm-reboot [](start = 23, length = 11)

I believe these was discussion on the terminology. Use 'warm-start'? @lguohan #Closed

agree, warm-start is better

qiluo-msft · 2018-10-31T23:07:00Z

src/libteam/0005-libteam-Add-warm_reboot-mode.patch

+ {
+ 	teamd_run_loop_sent_ctrl_byte(ctx, 'r');
+@@ -694,6 +716,10 @@ static int callback_daemon_signal(struct teamd_context *ctx, int events,
+ 		teamd_log_warn("Got SIGINT, SIGQUIT or SIGTERM.");


SIGQUIT [](start = 31, length = 7)

We may reuse sigterm instead of sigusr1.
If lacp-directory is provided, we can save state, otherwise just normal quit.

It's no harm to save for cold system reboot or cold docker restart. #Closed

The warm stop is not about file saving only. It includes other activities, like remove portchannel interface, put down physical ports. Also I think explicit signal is better than changed behavior as side effect of some setting.

qiluo-msft · 2018-10-31T23:13:04Z

src/libteam/0005-libteam-Add-warm_reboot-mode.patch

+-            "    -u --usock-disable       Disable UNIX domain socket interface\n",
+            "    -u --usock-disable       Disable UNIX domain socket interface\n"
+            "    -w --warm-reboot         Warm-reboot startup mode\n"
+            "    -L --lacp-directory      Directory for saving lacp pdu dumps\n",


lacp-directory [](start = 23, length = 14)

Suggest users specify the lacp-filename directly. #Closed

This is not that easy. We have not just one name, but list of names, one filename per interface from LAG.

qiluo-msft · 2018-10-31T23:14:09Z

src/libteam/0005-libteam-Add-warm_reboot-mode.patch

+		return err;
+	}
+
+        return lacpdu_process(lacp_port, &lacpdu);


[](start = 1, length = 8)

less indentation? #Closed

Actually you use 8 blanks, and original style is tab.

In reply to: 229898243 [](ancestors = 229898243)

Should be fixed now

qiluo-msft

Suggest upstream official teamd when the feature ready.

Update sonic-utilities submodule pointer to include the following: * Convert IPv6 addresses to lowercase in apply-patch ([sonic-net#2299](sonic-net/sonic-utilities#2299)) * [CLI] Move hostname, mgmt interface/vrf config to hostcfgd ([sonic-net#2173](sonic-net/sonic-utilities#2173)) * [config][muxcable] add support to enable/disable ycable telemetry ([sonic-net#2297](sonic-net/sonic-utilities#2297)) Signed-off-by: dprital <drorp@nvidia.com>

Update sonic-utilities submodule pointer to include the following: * Convert IPv6 addresses to lowercase in apply-patch ([#2299](sonic-net/sonic-utilities#2299)) * [CLI] Move hostname, mgmt interface/vrf config to hostcfgd ([#2173](sonic-net/sonic-utilities#2173)) * [config][muxcable] add support to enable/disable ycable telemetry ([#2297](sonic-net/sonic-utilities#2297)) Signed-off-by: dprital <drorp@nvidia.com>

Add Warm-reboot startup and shutdown mode for teamd

7bd9369

pavel-shirshov added the Enhancement ➕ label Oct 20, 2018

pavel-shirshov self-assigned this Oct 20, 2018

pavel-shirshov requested a review from lguohan October 20, 2018 02:10

jipanyang reviewed Oct 20, 2018

View reviewed changes

lguohan reviewed Oct 30, 2018

View reviewed changes

lguohan requested changes Oct 30, 2018

View reviewed changes

qiluo-msft reviewed Oct 31, 2018

View reviewed changes

qiluo-msft requested changes Oct 31, 2018

View reviewed changes

Address comments, fix some bugs

18670fd

lguohan approved these changes Nov 5, 2018

View reviewed changes

Use tab instead of 8 spaces

857977e

qiluo-msft approved these changes Nov 5, 2018

View reviewed changes

jipanyang approved these changes Nov 5, 2018

View reviewed changes

pavel-shirshov merged commit 4157f1d into master Nov 6, 2018

pavel-shirshov deleted the pavelsh/wr_teamd branch November 6, 2018 00:38

aravindmani-1 mentioned this pull request Nov 18, 2022

[Submodule] Update sonic-utilities pointer #12760

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[teamd] Add Warm-reboot startup and shutdown mode for teamd #2173

[teamd] Add Warm-reboot startup and shutdown mode for teamd #2173

pavel-shirshov commented Oct 20, 2018 •

edited

Loading

jipanyang left a comment

jipanyang Oct 20, 2018

lguohan Oct 30, 2018

pavel-shirshov Nov 1, 2018

pavel-shirshov Nov 2, 2018

jipanyang Oct 20, 2018

pavel-shirshov Oct 22, 2018

pavel-shirshov Nov 2, 2018

lguohan Oct 30, 2018

pavel-shirshov Nov 2, 2018

lguohan Oct 30, 2018

pavel-shirshov Nov 1, 2018

pavel-shirshov Nov 2, 2018

lguohan commented Oct 30, 2018

lguohan commented Oct 30, 2018

lguohan left a comment

jipanyang commented Oct 30, 2018

qiluo-msft Oct 31, 2018 •

edited

Loading

pavel-shirshov Nov 2, 2018

qiluo-msft Oct 31, 2018 •

edited

Loading

lguohan Oct 31, 2018

pavel-shirshov Nov 2, 2018

qiluo-msft Oct 31, 2018 •

edited

Loading

pavel-shirshov Nov 1, 2018 •

edited

Loading

qiluo-msft Oct 31, 2018 •

edited

Loading

pavel-shirshov Nov 1, 2018

qiluo-msft Oct 31, 2018 •

edited

Loading

qiluo-msft Nov 5, 2018

pavel-shirshov Nov 5, 2018

qiluo-msft left a comment

[teamd] Add Warm-reboot startup and shutdown mode for teamd #2173

[teamd] Add Warm-reboot startup and shutdown mode for teamd #2173

Conversation

pavel-shirshov commented Oct 20, 2018 • edited Loading

jipanyang left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lguohan commented Oct 30, 2018

lguohan commented Oct 30, 2018

lguohan left a comment

Choose a reason for hiding this comment

jipanyang commented Oct 30, 2018

qiluo-msft Oct 31, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Oct 31, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Oct 31, 2018 • edited Loading

Choose a reason for hiding this comment

pavel-shirshov Nov 1, 2018 • edited Loading

Choose a reason for hiding this comment

qiluo-msft Oct 31, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Oct 31, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft left a comment

Choose a reason for hiding this comment

pavel-shirshov commented Oct 20, 2018 •

edited

Loading

qiluo-msft Oct 31, 2018 •

edited

Loading

qiluo-msft Oct 31, 2018 •

edited

Loading

qiluo-msft Oct 31, 2018 •

edited

Loading

pavel-shirshov Nov 1, 2018 •

edited

Loading

qiluo-msft Oct 31, 2018 •

edited

Loading

qiluo-msft Oct 31, 2018 •

edited

Loading