From patchwork Sun Jun 9 14:17:45 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nitin Katiyar X-Patchwork-Id: 1112641 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=openvswitch.org (client-ip=140.211.169.12; helo=mail.linuxfoundation.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=ericsson.com Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ericsson.com header.i=@ericsson.com header.b="o+rjM5vG"; dkim-atps=neutral Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 45MJJN2Qjqz9s6w for ; Mon, 10 Jun 2019 00:17:55 +1000 (AEST) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id DB080BDC; Sun, 9 Jun 2019 14:17:50 +0000 (UTC) X-Original-To: ovs-dev@openvswitch.org Delivered-To: ovs-dev@mail.linuxfoundation.org Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id E6DF3B8F for ; Sun, 9 Jun 2019 14:17:48 +0000 (UTC) X-Greylist: whitelisted by SQLgrey-1.7.6 Received: from EUR04-DB3-obe.outbound.protection.outlook.com (mail-eopbgr60045.outbound.protection.outlook.com [40.107.6.45]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id B5A07711 for ; Sun, 9 Jun 2019 14:17:47 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ericsson.com; s=selector2; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=KW+KvRomaFYmNPJyMVCBlQhl0k9bpWbzFHx3TMjwwGQ=; b=o+rjM5vG9zBUxtCoExkviuiWwPiFDCs9wjRSUilw8PMbnxIZ2NiIC9wTmUgg+tQyEXlwxg/okCyMBEf4pQFtmssw6OLEgHF3r2SS5eKzBn59e7H2Ws90YdXnTXM8bh7yegfvteY4ouQF/eQs8/Kbt4Oy71YKaGWQ/GSX7x0Vi2Y= Received: from HE1PR0702MB3626.eurprd07.prod.outlook.com (52.133.6.24) by HE1SPR01MB022.eurprd07.prod.outlook.com (10.170.245.154) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1987.3; Sun, 9 Jun 2019 14:17:45 +0000 Received: from HE1PR0702MB3626.eurprd07.prod.outlook.com ([fe80::8df8:f2f9:c16c:dcc7]) by HE1PR0702MB3626.eurprd07.prod.outlook.com ([fe80::8df8:f2f9:c16c:dcc7%7]) with mapi id 15.20.1987.010; Sun, 9 Jun 2019 14:17:45 +0000 From: Nitin Katiyar To: "ovs-dev@openvswitch.org" Thread-Topic: [PATCH v7 1/2] Avoid packet drop on LACP bond after link up Thread-Index: AQHVHs4bEr1IAxhI1EaEmKasnYDPrQ== Date: Sun, 9 Jun 2019 14:17:45 +0000 Message-ID: <1560118744-28169-1-git-send-email-nitin.katiyar@ericsson.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [125.16.213.150] x-mailer: git-send-email 1.9.1 x-clientproxiedby: BMXPR01CA0042.INDPRD01.PROD.OUTLOOK.COM (2603:1096:b00:c::28) To HE1PR0702MB3626.eurprd07.prod.outlook.com (2603:10a6:7:8c::24) authentication-results: spf=none (sender IP is ) smtp.mailfrom=nitin.katiyar@ericsson.com; x-ms-exchange-messagesentrepresentingtype: 1 x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: fdf49666-db81-4f61-02b0-08d6ece53db7 x-microsoft-antispam: BCL:0; PCL:0; RULEID:(2390118)(7020095)(4652040)(8989299)(4534185)(7168020)(4627221)(201703031133081)(201702281549075)(8990200)(5600148)(711020)(4605104)(1401327)(2017052603328)(7193020); SRVR:HE1SPR01MB022; x-ms-traffictypediagnostic: HE1SPR01MB022: x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:1388; x-forefront-prvs: 006339698F x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(376002)(366004)(396003)(346002)(39860400002)(136003)(199004)(189003)(3846002)(5640700003)(6436002)(316002)(2906002)(6116002)(5660300002)(6512007)(2501003)(50226002)(6486002)(2351001)(71200400001)(8936002)(68736007)(71190400001)(52116002)(81166006)(81156014)(66946007)(64756008)(66556008)(256004)(73956011)(26005)(14444005)(54906003)(66446008)(99286004)(8676002)(86362001)(478600001)(4326008)(66476007)(386003)(305945005)(7736002)(25786009)(66066001)(55236004)(6916009)(53936002)(186003)(102836004)(14454004)(6506007)(36756003)(476003)(486006)(44832011)(2616005); DIR:OUT; SFP:1101; SCL:1; SRVR:HE1SPR01MB022; H:HE1PR0702MB3626.eurprd07.prod.outlook.com; FPR:; SPF:None; LANG:en; PTR:InfoNoRecords; A:1; MX:1; received-spf: None (protection.outlook.com: ericsson.com does not designate permitted sender hosts) x-ms-exchange-senderadcheck: 1 x-microsoft-antispam-message-info: ymCfyAjtOmPJSYcHWau/Ntl8yqpP38gLKTFYOAa+O0X0fB7dMRF8R7E8rFkX0iq+5BqrD3yEMd0W5oCPh5zP5YPh/U0ArpFwmhBVUKPM0XmxbyZwLVDXHcamWkfKZFxzMyzRSvJcKvQk8AfRBtNkgbGG5o8JzC91QFbUKOB/rZNcWmEOP1kMXEluXcNkHPNO9uiIwpNFPz+tvPYfc1kmu/9kmi9W57vYuzUuNSyLkt7xV5uRCcrVYeFk5XwUvz3gfJ+FIbdY4Ehv5heIHbNSVk7gCIOBIYgw9iRY8eTriYAbw2qkfIDJ+t5+atH0mjZUXTTsqiPqkG6dIXMOqhKdfQOAieaRka+jdiCwIEI9RVrFZ4vDFEFrkybNBBNaLC7Ubq/gDcdc2OiSMFqehfHR65LU8vEqWR5LtC+gnOioek8= MIME-Version: 1.0 X-OriginatorOrg: ericsson.com X-MS-Exchange-CrossTenant-Network-Message-Id: fdf49666-db81-4f61-02b0-08d6ece53db7 X-MS-Exchange-CrossTenant-originalarrivaltime: 09 Jun 2019 14:17:45.0501 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 92e84ceb-fbfd-47ab-be52-080c6b87953f X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: nitin.katiyar@ericsson.com X-MS-Exchange-Transport-CrossTenantHeadersStamped: HE1SPR01MB022 X-Spam-Status: No, score=-2.0 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp1.linux-foundation.org Cc: Manohar Krishnappa Chidambaraswamy , Nitin Katiyar Subject: [ovs-dev] [PATCH v7 1/2] Avoid packet drop on LACP bond after link up X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: ovs-dev-bounces@openvswitch.org Errors-To: ovs-dev-bounces@openvswitch.org Problem: ======== The OVS state machine that enables and disables bond slaves runs in the OVS main thread. The OVS code that processes received LACP packets runs in a different thread. Until now, when the latter processes a LACP PDU that should enable a slave, the slave was only enabled when the main thread was able to run the state machine. In some cases this led to delays of up to 350ms when the main thread was busy or not scheduled, which led to corresponding delays in which packets were dropped due to the bond-admissibility check. Fix: ==== When a LACP PDU is received, evaluate whether LACP slave can be enabled (slave_may_enable()) and set LACP slave's may_enable from the datapath thread itself. When may_enable = TRUE, it means L1 state is UP and LACP-SYNC is done and it is waiting for the main thread to enable the slave. Relax the check in bond_check_admissibility() to check for both "enable" and "may_enable" of the LACP slave. This would avoid dropping of packets until the main thread enables the slave from bundle_run(). Signed-off-by: Manohar Krishnappa Chidambaraswamy Co-authored-by: Manohar Krishnappa Chidambaraswamy Signed-off-by: Nitin Katiyar --- lib/lacp.c | 10 +++++++++- lib/lacp.h | 2 +- ofproto/bond.c | 22 +++++++++++++++++++--- ofproto/ofproto-dpif-xlate.c | 10 +++++++++- 4 files changed, 38 insertions(+), 6 deletions(-) diff --git a/lib/lacp.c b/lib/lacp.c index d6b36aa..e768012 100644 --- a/lib/lacp.c +++ b/lib/lacp.c @@ -154,6 +154,7 @@ static struct slave *slave_lookup(const struct lacp *, const void *slave) OVS_REQUIRES(mutex); static bool info_tx_equal(struct lacp_info *, struct lacp_info *) OVS_REQUIRES(mutex); +static bool slave_may_enable__(struct slave *slave) OVS_REQUIRES(mutex); static unixctl_cb_func lacp_unixctl_show; static unixctl_cb_func lacp_unixctl_show_stats; @@ -324,7 +325,7 @@ lacp_is_active(const struct lacp *lacp) OVS_EXCLUDED(mutex) /* Processes 'packet' which was received on 'slave_'. This function should be * called on all packets received on 'slave_' with Ethernet Type ETH_TYPE_LACP. */ -void +bool lacp_process_packet(struct lacp *lacp, const void *slave_, const struct dp_packet *packet) OVS_EXCLUDED(mutex) @@ -333,6 +334,7 @@ lacp_process_packet(struct lacp *lacp, const void *slave_, const struct lacp_pdu *pdu; long long int tx_rate; struct slave *slave; + bool lacp_may_enable = false; lacp_lock(); slave = slave_lookup(lacp, slave_); @@ -362,8 +364,14 @@ lacp_process_packet(struct lacp *lacp, const void *slave_, slave->partner = pdu->actor; } + /* Evaluate may_enable here to avoid dropping of packets till main thread + * sets may_enable to true. */ + lacp_may_enable = slave_may_enable__(slave); + out: lacp_unlock(); + + return lacp_may_enable; } /* Returns the lacp_status of the given 'lacp' object (which may be NULL). */ diff --git a/lib/lacp.h b/lib/lacp.h index f35cff5..0dfaef0 100644 --- a/lib/lacp.h +++ b/lib/lacp.h @@ -46,7 +46,7 @@ struct lacp *lacp_ref(const struct lacp *); void lacp_configure(struct lacp *, const struct lacp_settings *); bool lacp_is_active(const struct lacp *); -void lacp_process_packet(struct lacp *, const void *slave, +bool lacp_process_packet(struct lacp *, const void *slave, const struct dp_packet *packet); enum lacp_status lacp_status(const struct lacp *); diff --git a/ofproto/bond.c b/ofproto/bond.c index d2a8b1f..c5d5f2c 100644 --- a/ofproto/bond.c +++ b/ofproto/bond.c @@ -794,6 +794,7 @@ bond_check_admissibility(struct bond *bond, const void *slave_, { enum bond_verdict verdict = BV_DROP; struct bond_slave *slave; + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); ovs_rwlock_rdlock(&rwlock); slave = bond_slave_lookup(bond, slave_); @@ -811,7 +812,11 @@ bond_check_admissibility(struct bond *bond, const void *slave_, * drop all incoming traffic except if lacp_fallback_ab is enabled. */ switch (bond->lacp_status) { case LACP_NEGOTIATED: - verdict = slave->enabled ? BV_ACCEPT : BV_DROP; + /* To reduce packet-drops due to delay in enabling of slave (post + * LACP-SYNC), from main thread, check for may_enable as well. + * When may_enable is TRUE, it means LACP is UP and waiting for the + * main thread to run LACP state machine and enable the slave. */ + verdict = (slave->enabled || slave->may_enable) ? BV_ACCEPT : BV_DROP; goto out; case LACP_CONFIGURED: if (!bond->lacp_fallback_ab) { @@ -847,8 +852,6 @@ bond_check_admissibility(struct bond *bond, const void *slave_, /* Drop all packets which arrive on backup slaves. This is similar to * how Linux bonding handles active-backup bonds. */ if (bond->active_slave != slave) { - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); - VLOG_DBG_RL(&rl, "active-backup bond received packet on backup" " slave (%s) destined for " ETH_ADDR_FMT, slave->name, ETH_ADDR_ARGS(eth_dst)); @@ -870,6 +873,19 @@ bond_check_admissibility(struct bond *bond, const void *slave_, OVS_NOT_REACHED(); out: + if (slave && (verdict != BV_ACCEPT)) { + VLOG_DBG_RL(&rl, "slave (%s): Admissibility verdict is to drop pkt %s." + "active slave: %s, may_enable: %s enable: %s " + "LACP status:%d", + slave->name, + (verdict == BV_DROP_IF_MOVED) ? + "as different port is learned" : "", + (bond->active_slave == slave) ? "true" : "false", + slave->may_enable ? "true" : "false", + slave->enabled ? "true" : "false", + bond->lacp_status); + } + ovs_rwlock_unlock(&rwlock); return verdict; diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index 5cee37f..14a7237 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -3308,6 +3308,7 @@ process_special(struct xlate_ctx *ctx, const struct xport *xport) const struct xbridge *xbridge = ctx->xbridge; const struct dp_packet *packet = ctx->xin->packet; enum slow_path_reason slow; + bool lacp_may_enable; if (!xport) { slow = 0; @@ -3328,7 +3329,14 @@ process_special(struct xlate_ctx *ctx, const struct xport *xport) } else if (xport->xbundle && xport->xbundle->lacp && flow->dl_type == htons(ETH_TYPE_LACP)) { if (packet) { - lacp_process_packet(xport->xbundle->lacp, xport->ofport, packet); + lacp_may_enable = lacp_process_packet(xport->xbundle->lacp, + xport->ofport, packet); + /* Update LACP status in bond-slave to avoid packet-drops until + * LACP state machine is run by the main thread. */ + if (xport->xbundle->bond && lacp_may_enable) { + bond_slave_set_may_enable(xport->xbundle->bond, xport->ofport, + lacp_may_enable); + } } slow = SLOW_LACP; } else if ((xbridge->stp || xbridge->rstp) &&