From patchwork Fri Oct 20 04:35:07 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Xin Long X-Patchwork-Id: 10018893 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 3EB2060224 for ; Fri, 20 Oct 2017 04:35:27 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 252BF28E93 for ; Fri, 20 Oct 2017 04:35:27 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 16C7B28E99; Fri, 20 Oct 2017 04:35:27 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.3 required=2.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, RCVD_IN_SORBS_SPAM, T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from bastion.fedoraproject.org (bastion01.fedoraproject.org [209.132.181.2]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id D1DF428E93 for ; Fri, 20 Oct 2017 04:35:25 +0000 (UTC) Received: from mailman01.phx2.fedoraproject.org (mailman01.phx2.fedoraproject.org [10.5.126.36]) by bastion01.phx2.fedoraproject.org (Postfix) with ESMTP id 8532C604281C; Fri, 20 Oct 2017 04:35:23 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 bastion01.phx2.fedoraproject.org 8532C604281C Authentication-Results: bastion01.phx2.fedoraproject.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="snoqhzDF" Received: from mailman01.phx2.fedoraproject.org (localhost [IPv6:::1]) by mailman01.phx2.fedoraproject.org (Postfix) with ESMTP id 63CF421780016; Fri, 20 Oct 2017 04:35:23 +0000 (UTC) Received: by mailman01.phx2.fedoraproject.org (Postfix, from userid 991) id 61D632178000B; Fri, 20 Oct 2017 04:35:17 +0000 (UTC) Received: from smtp-mm-tummy01.fedoraproject.org (smtp-mm-tummy01.vpn.fedoraproject.org [192.168.1.82]) by mailman01.phx2.fedoraproject.org (Postfix) with ESMTP id 654CF21780016 for ; Fri, 20 Oct 2017 04:35:16 +0000 (UTC) Received: from mail-pf0-f193.google.com (mail-pf0-f193.google.com [209.85.192.193]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by smtp-mm-tummy01.fedoraproject.org (Postfix) with ESMTPS id 120956087E05 for ; Fri, 20 Oct 2017 04:35:16 +0000 (UTC) Received: by mail-pf0-f193.google.com with SMTP id b79so9257936pfk.5 for ; Thu, 19 Oct 2017 21:35:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id; bh=QWu6MLErpRcl6kRqxrWHW5BmgTzSUf2o0hc+rsqTzBA=; b=snoqhzDFvCkO1l/a7gnGoqwjjz8heb7faB7sI4wK/yqm2d13NMYvf+fwfeUj1WOf9Y 87FQ6+JXYzIQp5dGCP5BGMPDRFv2naegQZ+4Zx0hEVUo0Dr/05hyXpEankGK1rJsEvzA AbBTvBKQLC1tCFw/JnaAWC94bz3IxwIEMC9cN0jKEDCet70FMsGDNQm5X+Ma3uHoR547 D3OKUck63on1T5ctAs9B7327OWQzZVZNTp3dn+BPCkfnQarWHtsAlOdZysfQ5O1k0KU0 tjtFAR0/SrX+0GVB78h29okf2J+5UWYLzPANGDIPOG8H8PS+/ULM2UrBwfqBE7Kk+tBm oLwg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=QWu6MLErpRcl6kRqxrWHW5BmgTzSUf2o0hc+rsqTzBA=; b=lVlhxlrGAY+s9/Us4PJkUcVdjc/R8TH6glbRljdxCkbPauM1hM0fOi5W4af31JNMLl BnFEenwfgUYsw5oo9GzVze83aQKs2mGgxo/r4VQP66PMO/K/alRqqlFBbPAfz+HlBi5c lquLCmAhJDkNyraPGWkHeaVzw8WddvgGJ9Mu4gjilc7Hj6ECmUYZW932/4vEGpKiy9Kb nwbv6mKz5pA/Ry3VMXGYW6mkHlE1LiYOeYivDI/CuWwjivJZ8UGdJKjdwmG7P0BNjeLv whd6x9xJIYuHsTTC/xnVN/Q/HhosMrVAtbN63hxdCRvqty/+rUHjLQLoSGVLw7UM6Y3i 6E+w== X-Gm-Message-State: AMCzsaXKUKBK3TVYK3DDW27pmSvJM4XMPaos6Z0BqaCehiYPUshYJ+z+ DHlqQNKm7vySvTXggrgj90LiRwJl X-Google-Smtp-Source: ABhQp+SvKe8PtN+i64S3fYS3RzAFqH0TYV+SyCIVQZ6+wa8DSRX3rnze4GF+wX6SHp7Av6SzkRJTLw== X-Received: by 10.101.92.68 with SMTP id v4mr927455pgr.45.1508474115207; Thu, 19 Oct 2017 21:35:15 -0700 (PDT) Received: from localhost ([209.132.188.80]) by smtp.gmail.com with ESMTPSA id i62sm190597pfe.73.2017.10.19.21.35.14 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 19 Oct 2017 21:35:14 -0700 (PDT) From: Xin Long To: libteam@lists.fedorahosted.org, Jiri Pirko Subject: [PATCHv2] teamd: add port_hwaddr_changed for ab runner Date: Fri, 20 Oct 2017 12:35:07 +0800 Message-Id: X-Mailer: git-send-email 2.1.0 Message-ID-Hash: LDBOVP47UKUHBUELWOZLZMMGTFWBGPHV X-Message-ID-Hash: LDBOVP47UKUHBUELWOZLZMMGTFWBGPHV X-MailFrom: lucien.xin@gmail.com X-Mailman-Rule-Misses: dmarc-mitigation; approved; emergency; loop; banned-address; member-moderation; header-match-config-1; header-match-config-2; header-match-config-3; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; suspicious-header CC: Jon Nikolakakis X-Mailman-Version: 3.1.0 Precedence: list List-Id: Archived-At: List-Archive: List-Help: List-Post: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Virus-Scanned: ClamAV using ClamSMTP This patch to fix an events processing race issue when adding two ports into one team dev with ab mode with same_all hwaddr policy: team0 original hwaddr: 00:00:00:00:00:0a port1 original hwaddr: 00:00:00:00:00:01 port2 original hwaddr: 00:00:00:00:00:02 There are two sockets in teamd: nl_cli.sock_event for ifinfo updates and nl_sock_event for ports/options changes. During adding two ports, the events on these two sockets could be: nl_sock_event: [1] -- [2] -- [1]: port1 added event (added by enslaving port1) [2]: port2 added event (added by enslaving port2) nl_cli.sock_event: [a1] -- [b0] -- [c1] -- [d2] -- [e2] -- [f1] -- [a1]: port1 ifinfo event (added by setting port1's master) [b0]: team0 ifinfo event (added by setting team0's hwaddr) [c1]: port1 ifinfo event (added by set port1's hwaddr) [d2]: port2 ifinfo event (added by set port2's master) [e2]: port2 ifinfo event (added by set port2's hwaddr) [f1]: port1 ifinfo event (added by set port1's hwaddr) teamd can make sure the order for their processing is as above on the same socket, but not between two sockets. So if these events processing order is (monitoring team/ports' ifinfo, hwaddr, master): [ 1]: team0->ifinfo = 00:00:00:00:00:0a team0->hwaddr = 00:00:00:00:00:01 port1->hwaddr = 00:00:00:00:00:0a [a1]: port1->ifinfo = 00:00:00:00:00:01 port1->master = team0 [ 2]: port2->ifinfo = 00:00:00:00:00:02 port2->hwaddr = 00:00:00:00:00:0a (team0->ifinfo is not updated, it's still 00:00:00:00:00:0a) [b0]: team0->ifinfo = 00:00:00:00:00:01 port1->hwaddr = 00:00:00:00:00:01 (port2->master is not yet set, port2->hwaddr couldn't be updated) [c1]: no changes [d2]: port2->ifinfo = 00:00:00:00:00:0a port2->master = team0 (too late !!!) [e2]: no changes [f1]: no changes Then: team0 final hwaddr: 00:00:00:00:00:01 port1 final hwaddr: 00:00:00:00:00:01 port2 final hwaddr: 00:00:00:00:00:0a <----- issue This patch is to add port_hwaddr_changed for ab runner, in [e2] where we set it's hwaddr with team0 (port2->hwaddr = 00:00:00:00:00:01) IF port2->hwaddr != team0->ifinfo. I think the same issue also exists in lacp and lb mode for which I will fix them in another patches. v1 -> v2: fix some typos in changelog and couple of style problems in codes Reported-by: Jon Nikolakakis Signed-off-by: Xin Long --- teamd/teamd_runner_activebackup.c | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) -- 2.1.0 diff --git a/teamd/teamd_runner_activebackup.c b/teamd/teamd_runner_activebackup.c index aec3a73..8a3447f 100644 --- a/teamd/teamd_runner_activebackup.c +++ b/teamd/teamd_runner_activebackup.c @@ -39,6 +39,8 @@ struct ab_hwaddr_policy { const char *name; int (*hwaddr_changed)(struct teamd_context *ctx, struct ab *ab); + int (*port_hwaddr_changed)(struct teamd_context *ctx, struct ab *ab, + struct teamd_port *tdport); int (*port_added)(struct teamd_context *ctx, struct ab *ab, struct teamd_port *tdport); int (*active_set)(struct teamd_context *ctx, struct ab *ab, @@ -95,6 +97,26 @@ static int ab_hwaddr_policy_same_all_hwaddr_changed(struct teamd_context *ctx, return 0; } +static int +ab_hwaddr_policy_same_all_port_hwaddr_changed(struct teamd_context *ctx, + struct ab *ab, + struct teamd_port *tdport) +{ + int err; + + if (!memcmp(team_get_ifinfo_hwaddr(tdport->team_ifinfo), + ctx->hwaddr, ctx->hwaddr_len)) + return 0; + + err = team_hwaddr_set(ctx->th, tdport->ifindex, ctx->hwaddr, + ctx->hwaddr_len); + if (err) + teamd_log_err("%s: Failed to set port hardware address.", + tdport->ifname); + + return err; +} + static int ab_hwaddr_policy_same_all_port_added(struct teamd_context *ctx, struct ab *ab, struct teamd_port *tdport) @@ -114,6 +136,7 @@ static int ab_hwaddr_policy_same_all_port_added(struct teamd_context *ctx, static const struct ab_hwaddr_policy ab_hwaddr_policy_same_all = { .name = "same_all", .hwaddr_changed = ab_hwaddr_policy_same_all_hwaddr_changed, + .port_hwaddr_changed = ab_hwaddr_policy_same_all_port_hwaddr_changed, .port_added = ab_hwaddr_policy_same_all_port_added, }; @@ -411,6 +434,21 @@ static int ab_event_watch_hwaddr_changed(struct teamd_context *ctx, void *priv) return 0; } +static int ab_event_watch_port_hwaddr_changed(struct teamd_context *ctx, + struct teamd_port *tdport, + void *priv) +{ + struct ab *ab = priv; + + if (!teamd_port_present(ctx, tdport)) + return 0; + + if (ab->hwaddr_policy->port_hwaddr_changed) + return ab->hwaddr_policy->port_hwaddr_changed(ctx, ab, tdport); + + return 0; +} + static int ab_port_load_config(struct teamd_context *ctx, struct ab_port *ab_port) { @@ -491,6 +529,7 @@ static int ab_event_watch_prio_option_changed(struct teamd_context *ctx, static const struct teamd_event_watch_ops ab_event_watch_ops = { .hwaddr_changed = ab_event_watch_hwaddr_changed, + .port_hwaddr_changed = ab_event_watch_port_hwaddr_changed, .port_added = ab_event_watch_port_added, .port_link_changed = ab_event_watch_port_link_changed, .option_changed = ab_event_watch_prio_option_changed,