|
| 1 | +From b9bde92fcc0586b327c577c41304e2ef938cff10 Mon Sep 17 00:00:00 2001 |
| 2 | +From: Petr Machata < [email protected]> |
| 3 | +Date: Wed, 13 Nov 2019 13:26:47 +0000 |
| 4 | +Subject: [PATCH] teamd: Disregard current state when considering port |
| 5 | + enablement |
| 6 | + |
| 7 | +On systems where carrier is gained very quickly, there is a race between |
| 8 | +teamd and the kernel that sometimes leads to all team slaves being stuck in |
| 9 | +enabled=false state. |
| 10 | + |
| 11 | +When a port is enslaved to a team device, the kernel sends a netlink |
| 12 | +message marking the port as enabled. teamd's lb_event_watch_port_added() |
| 13 | +calls team_set_port_enabled(false), because link is down at that point. The |
| 14 | +kernel responds with a message marking the port as disabled. At this point, |
| 15 | +there are two outstanding messages: the initial one marking port as |
| 16 | +enabled, and the second one marking it as disabled. teamd has not processed |
| 17 | +either of these. |
| 18 | + |
| 19 | +Next teamd gets the netlink message that sets enabled=true, and updates its |
| 20 | +internal cache accordingly. If at this point ethtool link-watch wakes up, |
| 21 | +teamd considers (in teamd_port_check_enable()) enabling the port. After |
| 22 | +consulting the cache, it concludes the port is already up, and neglects to |
| 23 | +do so. Only then does teamd get the netlink message informing it of setting |
| 24 | +enabled=false. |
| 25 | + |
| 26 | +The problem is that the teamd cache is not synchronous with respect to the |
| 27 | +kernel state. If the carrier takes a while to come up (as is normally the |
| 28 | +case), this is not a problem, because teamd caches up quickly enough. But |
| 29 | +this may not always be the case, and particularly on a simulated system, |
| 30 | +the carrier is gained almost immediately. |
| 31 | + |
| 32 | +Fix this by not suppressing the enablement message. |
| 33 | + |
| 34 | +Signed-off-by: Petr Machata < [email protected]> |
| 35 | +Signed-off-by: Jiri Pirko < [email protected]> |
| 36 | +--- |
| 37 | + teamd/teamd_per_port.c | 8 ++------ |
| 38 | + 1 file changed, 2 insertions(+), 6 deletions(-) |
| 39 | + |
| 40 | +diff --git a/teamd/teamd_per_port.c b/teamd/teamd_per_port.c |
| 41 | +index d10cfb2..8c63fec 100644 |
| 42 | +--- a/teamd/teamd_per_port.c |
| 43 | ++++ b/teamd/teamd_per_port.c |
| 44 | +@@ -448,18 +448,14 @@ int teamd_port_check_enable(struct teamd_context *ctx, |
| 45 | + bool should_enable, bool should_disable) |
| 46 | + { |
| 47 | + bool new_enabled_state; |
| 48 | +- bool curr_enabled_state; |
| 49 | + int err; |
| 50 | + |
| 51 | + if (!teamd_port_present(ctx, tdport)) |
| 52 | + return 0; |
| 53 | +- err = teamd_port_enabled(ctx, tdport, &curr_enabled_state); |
| 54 | +- if (err) |
| 55 | +- return err; |
| 56 | + |
| 57 | +- if (!curr_enabled_state && should_enable) |
| 58 | ++ if (should_enable) |
| 59 | + new_enabled_state = true; |
| 60 | +- else if (curr_enabled_state && should_disable) |
| 61 | ++ else if (should_disable) |
| 62 | + new_enabled_state = false; |
| 63 | + else |
| 64 | + return 0; |
| 65 | +-- |
| 66 | +2.17.1.windows.2 |
| 67 | + |
0 commit comments