From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR01-DB5-obe.outbound.protection.outlook.com (mail-db5eur01on0052.outbound.protection.outlook.com [104.47.2.52]) by dpdk.org (Postfix) with ESMTP id 56BD71B33E; Sun, 11 Feb 2018 18:25:00 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=Mellanox.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version; bh=c2NIkmPIO82OKPmNYNwW+Fe3mzMKmIQO96YO0F6SMk8=; b=sl2rvxKG2FLfz5WiIsmgURwyW0H9vQG61HByTHq9n+d/xCZMlxPwTlQA6piDkR5bHV7gEvx1sRrXmnqnxL4v6AaY1yWHzk1jcq52dRRaIpZ3VNJNSrM2nHLywExSs3VJPiptlvHKUw4ubsM7ChZmoTWyhcgSba1ZPlXQmh3qMoc= Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=matan@mellanox.com; Received: from mellanox.com (37.142.13.130) by AM4PR0501MB2657.eurprd05.prod.outlook.com (2603:10a6:200:59::19) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id 15.20.485.10; Sun, 11 Feb 2018 17:24:58 +0000 From: Matan Azrad To: Gaetan Rivet Cc: dev@dpdk.org, stable@dpdk.org Date: Sun, 11 Feb 2018 17:24:32 +0000 Message-Id: <1518369872-12324-4-git-send-email-matan@mellanox.com> X-Mailer: git-send-email 1.8.3.1 In-Reply-To: <1518369872-12324-1-git-send-email-matan@mellanox.com> References: <1518107653-15466-1-git-send-email-matan@mellanox.com> <1518369872-12324-1-git-send-email-matan@mellanox.com> MIME-Version: 1.0 Content-Type: text/plain X-Originating-IP: [37.142.13.130] X-ClientProxiedBy: VI1PR0501CA0022.eurprd05.prod.outlook.com (2603:10a6:800:92::32) To AM4PR0501MB2657.eurprd05.prod.outlook.com (2603:10a6:200:59::19) X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-HT: Tenant X-MS-Office365-Filtering-Correlation-Id: b936e106-6897-4833-e296-08d571746052 X-Microsoft-Antispam: UriScan:; BCL:0; PCL:0; RULEID:(7020095)(4652020)(48565401081)(5600026)(4604075)(4534165)(4627221)(201703031133081)(201702281549075)(2017052603307)(7153060)(7193020); SRVR:AM4PR0501MB2657; X-Microsoft-Exchange-Diagnostics: 1; AM4PR0501MB2657; 3:BNHDbEeQbo9pIxe/UkqOQf0SXEcCJgrVdEvfkNME9KrkF5ga5B2V6jIjQPsOQwcMup+7CI5fxgeIS58/Arw6M8eFOYKmwi953wB7CjD/OR5xrVsSPjlBZPSh7aM9K5MLSOg61gtlGdKnBn66o5fGf8M5/uOCrhrSLsabe0KtS+S1q7U/7cBNSQOInlGcJ3Jjd79+MMmAIXNDpvBy3TS3i7/18csCFjyWUiJ4vRG6Lq0jy/1eZl6kpbjG7Qy7z+e8; 25:adUHvyRWWolTtmN34Uc4cSnKTBLoz+sjWrqAanZ8EHND2dJ1ld0Wibc2ZxGNuqrYTodB/Mez9cMykTd65W4MwcKgesEZzF4B73dMlLvW7DtqUQ1bWE16rTiSoydGXVV9E5KegybLfqL8UUgVdBkOLgoGcjL7p1mZuFP/d+aAjIoS0Jwmu6lji/2+IFyglHU6EOJbIXiceJ/YVth2KVkv1eVL9WmeosvN9ANGkfj84e0LfLJwbcNoVO9EUj4tJs8B4rx/Uv8oUXQIufEA66CSuNbzI4S8XpSEduJOv9TUBIbajmTR9BP460jrobh0O/aG7kuDoSTCAjd3Pkkb60m2MA==; 31:f48KyWsG4r0njsUIsuxgJBgbmvrrMbH0xNFQyyOfTD1YNMvpgZZbhFI9/MhrzCBJLkGQ9HdZ2mIiml54nh5XlhYTQrTU5tmxb0xDhY9cjvJvF9gy4J6uHcYSiFmE78tl5Pvi1NxxPGOZvNXjx8fkhJSGiYQ3n4T1oM2ZN2+N7cWEMmsCalENSFGPOyymsWoVTV8m0N13SH1YxXt5diOTiedJe5r9RDppACbglZ8CDnQ= X-MS-TrafficTypeDiagnostic: AM4PR0501MB2657: X-Microsoft-Exchange-Diagnostics: 1; AM4PR0501MB2657; 20:p6y+vmV6/SsUujytjEQr2Srpk3LZkF2G/KbF3TvpZqXbreeCbpestRlBQvJ6qCE3opAEnYYKKavEyRbQZJlewROBViUsEifbfMSuUG0PuRy/96NsJOXHICmzF2LtWq7iultqHBUSh4vYxsLnA5EYpZefqeRwM80icCj3EWdotNy10jMlk1/AeZzKJpyK98M/ddWPI91L7dOoIaO9T+T55xciw6umHcMxmVqfCD4V54LAEti+BYmB/u9fXJkgK9zo4eSEoiN6KrM1VeoyF94aOFuzp7W13LbNUHdekhOVieZLk1UrBfY7o8St8JasTKKnQhN9s+7wiShuJg7LM14qD0EBzwrP97itEDvOpgGEflrYeSKQxLO3F9fZEXQhuKB9BhABabLGB+IZdRwjEktxsRaLVNIQpZmew/jkjCAi9Sn7txuZ44qIE1OhUkBLq5V2B562Y6aApEuOK2uYoNK5WtSIe1qSXyWv+V9WT9UqMTZ4MSqSI1MQ6drTg2DyWtdw; 4:RiHPkwm+jfvBNApAsuME+olPlqPfIgHXTSEHeWrI6ZiuhlAtSc42V7yr1NVr+wg/JSUGZtCLOym0Alw3Njcox2ZTFPRdBdl6xbvetJuJ5aDiD139FrJQ8e4OaNoh2TX3CXHj1W/H+Esid4MBkvqxpNaK1DCNmWIFqNmdWN1QyMC6b9mfcwiV4HleLCvOjRqi9iTb8OGYzbUgQzCYrIUjKdgX4V/3AZZRcuMu54xVVYuFjE3QVHRKQVlS8O7mMKi4L7Pwak2q7jPinis6ITxdc+KrYpKDRC5BdPrgFR7tM0EYc/TxvLe3LrE+nT4Oxelm X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:(60795455431006); X-Exchange-Antispam-Report-CFA-Test: BCL:0; PCL:0; RULEID:(6040501)(2401047)(8121501046)(5005006)(93006095)(93001095)(10201501046)(3231101)(2400082)(944501161)(3002001)(6055026)(6041288)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(20161123558120)(20161123560045)(20161123564045)(20161123562045)(6072148)(201708071742011); SRVR:AM4PR0501MB2657; BCL:0; PCL:0; RULEID:; SRVR:AM4PR0501MB2657; X-Forefront-PRVS: 058043A388 X-Forefront-Antispam-Report: SFV:NSPM; SFS:(10009020)(396003)(346002)(376002)(39380400002)(39860400002)(366004)(189003)(199004)(50466002)(5660300001)(16526019)(81156014)(8676002)(4326008)(16586007)(186003)(69596002)(81166006)(26005)(76176011)(33026002)(48376002)(105586002)(53936002)(21086003)(386003)(25786009)(2906002)(4720700003)(6666003)(97736004)(106356001)(68736007)(51416003)(6116002)(316002)(2950100002)(478600001)(305945005)(55016002)(47776003)(36756003)(7736002)(86362001)(575784001)(52116002)(8936002)(50226002)(7696005)(66066001)(59450400001)(6916009)(3846002); DIR:OUT; SFP:1101; SCL:1; SRVR:AM4PR0501MB2657; H:mellanox.com; FPR:; SPF:None; PTR:InfoNoRecords; MX:1; A:1; LANG:en; Received-SPF: None (protection.outlook.com: mellanox.com does not designate permitted sender hosts) X-Microsoft-Exchange-Diagnostics: =?us-ascii?Q?1; AM4PR0501MB2657; 23:/vpIo6NHKXQFxdc0HhWKW4gMbx6mRwft/ru0QLS?= =?us-ascii?Q?dKTm5f1rxja7IYS+sD13l4Z2zdbz+BH9ZL+OegDASbhe2BK/ZWQOVZh3SSFM?= =?us-ascii?Q?+hefsuRmFOYetri+7PDiKXjLM6FNUfk+ZsBDwm+rBVfCiQ08Ll4eu5d3nnTs?= =?us-ascii?Q?LfmgoXIw+CaEe5F9JE5c+dMh2v744YOGRof26kpkwsuTUJX0XPhL4by4jn8+?= =?us-ascii?Q?CJa1pOCZGYqvM9QRXThNwuUNgDByF5Vd+nVegS+JwHXcr7T7pkSoI/9XkDMi?= =?us-ascii?Q?BHxBAtbqUGPB43rapCoWzsMWHVuIESgJv75aAsJ2OTltvTrxoK9tfbssIoOc?= =?us-ascii?Q?b8rv6mhQe63jJjRX3Y1vZeKynAbu8Ff6Fm75OSn09hoMT/dZbiWrHi+tTVuL?= =?us-ascii?Q?EtG8xsagaKegZHMmhHk8yN7jsciZCRToSSqKB/BIcihwoVPW0PtSwg3A1zqf?= =?us-ascii?Q?6QBfqsKTLDDPYaxQG9Y5jPgWs2B8zEDLTxQcmc5316zviV/EN0B65v/Nvrxv?= =?us-ascii?Q?n+5mEatMSy0vGT+ydLmPoYXOOYUt3Ft14AWSWyuLYg8mXFtuWxxrj9ZDWhwa?= =?us-ascii?Q?mAxA/2jJCaFEPm7tILhCJrhAwDyRIe7f+76Qcl+aeu7zjQMD3o6I38Xx+U2K?= =?us-ascii?Q?Eaf1HjBKrx381kY8uh5qcXPajQwafLSytWL/VonLs7AQwLq51NBRaxA8I/2e?= =?us-ascii?Q?g36xzjN8LDLfliLAZIfpnHRRlvzBnBKZwzeaYiB5Ped7iCos9UyiU1vLpnzc?= =?us-ascii?Q?bHDJwSgLAg+zQKop4b7l4SnNkBiUj3ueMkfsnEt8TPqE8zYthaBlab6dDSGU?= =?us-ascii?Q?vzrTFcO7OI9dS1ok0pDRqjkXujdPgUB3qVVxMbeQr64QapxELpVVSPOxv+VL?= =?us-ascii?Q?NTlbiM11oEEOzyM7wMq8zdpU37ZkmVTzNXpcXBIFjrFXNRN+fx5pSFKLiYBx?= =?us-ascii?Q?wy+8GNF81FIerTMDA6Von/6mSR3IAa5r9OS/DrkRoSNBvZ2kBFl/WIGV7lXa?= =?us-ascii?Q?yZoZDspR6PVBD//Sd+cCwMLJ9qqY2ek3041rznvGQ4nNiDNbhN3RK0eefVe+?= =?us-ascii?Q?H7V4PuxgN5l5QWaYnX2r+lgSPB+I4RRYuc3rjQ7oYvxTlNKyiOujPfm7/ytF?= =?us-ascii?Q?CXmfaKdAfH1PHKBebPLlUs/RvIQa0x8iQnvBDwOa1JyXRAjhyui9Z1xeI4r5?= =?us-ascii?Q?k47Pt/vH+1q1cSk07j2Jik1wziniyzX5tX955aMU7BvnlAQwewadkEn9RlQ?= =?us-ascii?Q?=3D=3D?= X-Microsoft-Exchange-Diagnostics: 1; AM4PR0501MB2657; 6:gfxXpcB4r0vfUIgRqt/qVxXoV86d7MK1nvD8kyf6wlQy53u/t8h8f5zcN+jkOx2dMeWc40WUA/feUN7gxnpr1zbevoR+u3Xgxiz/dwQNttW1d/VP08ATJD4ZfvVRnGNT/n+WQj64MBFplJEt1Jau35NXSO2rRsYOV57+IAVugEZZKTRyFxiU4hUAqLMYnSFVuZUts2KfAogm4KVDKLAjA3AGTsKLqTx9h6SY1cH5K/Hfo36vGQEaNTJlzrDPKiJS0ck8hpK6Nx4uWFa3D55NKBYJxZ4z0hj8tKbZzHLeSt9Hftytws5sxkuQ40+Yd3orJOuS0z06enR6PzUjpDH+2r6WgPEDF6gZvQP6DywBxqE=; 5:xHmsk7HEWoMiLDqp7epnVNAWTKFMce3BQDi3+7MQozLj/LDIZxiW3BduTR3Qg0GO6muXXh9nGoHO/X5UEgZ2JRB8vyvp+dIvh327YbnAbfr6rc5whQWHJfDQl6b2m9UlulVFXZ7O3Kap+OfNLmrq7NA3OAkoM3jPJ+4L0bMFiyA=; 24:U66DiH12pPikI98yWWe5YSBfKZMurD8AAS73mJ3zxRGPX1Cz2ubAhryl2pVpUUVt2GmaStuogaFHY4SB8TUCyVR14NyBOt9mS/w42eFkYdU=; 7:a8Hi8NsSrAmnna+MzDA7bB8EfAi9YsA9vRStoedIGFSfKvWZ3idP8ZryzyTCwMiEq4SNn/pYbTmvmEIldGSV/F1owjEkLSaEWfb5fC0xt/xPFmIJalHOmLbo8dFfH7tJACyLB2rRPUsxMIrFVSfqXrqAJ72b6E03HsVQcTkvOmPqv3R26wji25/osVOKTSfdXoApmmoIGwaRc/CLd1GraMGkPNTk1fUQgkWLdwWQYVB6/9BUZT9gV7WDniOkUB2f SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: Mellanox.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 11 Feb 2018 17:24:58.1170 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: b936e106-6897-4833-e296-08d571746052 X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: a652971c-7d2e-4d9b-a6a4-d149256f461b X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM4PR0501MB2657 Subject: [dpdk-dev] [PATCH v6 3/3] net/failsafe: fix hotplug races X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 11 Feb 2018 17:25:00 -0000 Fail-safe uses periodic alarm mechanism, running from the host thread, to manage the hot-plug events of its sub-devices. This management requires a lot of sub-devices PMDs operations (stop,close,start,etc). While the hot-plug alarm runs in the host thread, the application may call fail-safe operations which directly trigger the sub-devices PMDs operations too, This call may occur from any thread decided by the application (probably the master thread). So, more than one operation can execute to a sub-device in same time what can cause a lot of races in the sub-PMDs. Moreover, some control operations update the fail-safe internal databases which can be used by the alarm mechanism in the same time, what also can cause to races and crashes. Fail-safe is the owner of its sub-devices and must to synchronize their use according to the ETHDEV ownership rules. Synchronize hot-plug management by a new lock mechanism uses a mutex to atomically defend each critical section in the fail-safe hot-plug mechanism and control operations to prevent any races between them. Fixes: a46f8d5 ("net/failsafe: add fail-safe PMD") Cc: stable@dpdk.org Signed-off-by: Matan Azrad --- drivers/net/failsafe/Makefile | 1 + drivers/net/failsafe/failsafe.c | 35 ++++++++ drivers/net/failsafe/failsafe_ether.c | 6 +- drivers/net/failsafe/failsafe_flow.c | 20 ++++- drivers/net/failsafe/failsafe_ops.c | 148 ++++++++++++++++++++++++++------ drivers/net/failsafe/failsafe_private.h | 62 +++++++++++-- 6 files changed, 239 insertions(+), 33 deletions(-) diff --git a/drivers/net/failsafe/Makefile b/drivers/net/failsafe/Makefile index d1ae899..bd2f019 100644 --- a/drivers/net/failsafe/Makefile +++ b/drivers/net/failsafe/Makefile @@ -68,5 +68,6 @@ CFLAGS += -pedantic LDLIBS += -lrte_eal -lrte_mbuf -lrte_mempool -lrte_ring LDLIBS += -lrte_ethdev -lrte_net -lrte_kvargs LDLIBS += -lrte_bus_vdev +LDLIBS += -lpthread include $(RTE_SDK)/mk/rte.lib.mk diff --git a/drivers/net/failsafe/failsafe.c b/drivers/net/failsafe/failsafe.c index 7b2cdbb..c499bfb 100644 --- a/drivers/net/failsafe/failsafe.c +++ b/drivers/net/failsafe/failsafe.c @@ -113,17 +113,46 @@ break; /* if we have non-probed device */ if (i != PRIV(dev)->subs_tail) { + if (fs_lock(dev, 1) != 0) + goto reinstall; ret = failsafe_eth_dev_state_sync(dev); + fs_unlock(dev, 1); if (ret) ERROR("Unable to synchronize sub_device state"); } failsafe_dev_remove(dev); +reinstall: ret = failsafe_hotplug_alarm_install(dev); if (ret) ERROR("Unable to set up next alarm"); } static int +fs_mutex_init(struct fs_priv *priv) +{ + int ret; + pthread_mutexattr_t attr; + + ret = pthread_mutexattr_init(&attr); + if (ret) { + ERROR("Cannot initiate mutex attributes - %s", strerror(ret)); + return ret; + } + /* Allow mutex relocks for the thread holding the mutex. */ + ret = pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE); + if (ret) { + ERROR("Cannot set mutex type - %s", strerror(ret)); + return ret; + } + ret = pthread_mutex_init(&priv->hotplug_mutex, &attr); + if (ret) { + ERROR("Cannot initiate mutex - %s", strerror(ret)); + return ret; + } + return 0; +} + +static int fs_eth_dev_create(struct rte_vdev_device *vdev) { struct rte_eth_dev *dev; @@ -176,6 +205,9 @@ ret = failsafe_eal_init(dev); if (ret) goto free_args; + ret = fs_mutex_init(priv); + if (ret) + goto free_args; ret = failsafe_hotplug_alarm_install(dev); if (ret) { ERROR("Could not set up plug-in event detection"); @@ -250,6 +282,9 @@ ERROR("Error while uninitializing sub-EAL"); failsafe_args_free(dev); fs_sub_device_free(dev); + ret = pthread_mutex_destroy(&PRIV(dev)->hotplug_mutex); + if (ret) + ERROR("Error while destroying hotplug mutex"); rte_free(PRIV(dev)); rte_eth_dev_release_port(dev); return ret; diff --git a/drivers/net/failsafe/failsafe_ether.c b/drivers/net/failsafe/failsafe_ether.c index d820faf..8672819 100644 --- a/drivers/net/failsafe/failsafe_ether.c +++ b/drivers/net/failsafe/failsafe_ether.c @@ -328,8 +328,11 @@ FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) if (sdev->remove && fs_rxtx_clean(sdev)) { + if (fs_lock(dev, 1) != 0) + return; fs_dev_stats_save(sdev); fs_dev_remove(sdev); + fs_unlock(dev, 1); } } @@ -428,7 +431,7 @@ void *cb_arg, void *out __rte_unused) { struct sub_device *sdev = cb_arg; - + fs_lock(sdev->fs_dev, 0); /* Switch as soon as possible tx_dev. */ fs_switch_dev(sdev->fs_dev, sdev); /* Use safe bursts in any case. */ @@ -438,6 +441,7 @@ * the callback at the source of the current thread context. */ sdev->remove = 1; + fs_unlock(sdev->fs_dev, 0); return 0; } diff --git a/drivers/net/failsafe/failsafe_flow.c b/drivers/net/failsafe/failsafe_flow.c index 4d18e8e..ec8c909 100644 --- a/drivers/net/failsafe/failsafe_flow.c +++ b/drivers/net/failsafe/failsafe_flow.c @@ -55,6 +55,7 @@ uint8_t i; int ret; + fs_lock(dev, 0); FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) { DEBUG("Calling rte_flow_validate on sub_device %d", i); ret = rte_flow_validate(PORT_ID(sdev), @@ -62,9 +63,11 @@ if ((ret = fs_err(sdev, ret))) { ERROR("Operation rte_flow_validate failed for sub_device %d" " with error %d", i, ret); + fs_unlock(dev, 0); return ret; } } + fs_unlock(dev, 0); return 0; } @@ -79,6 +82,7 @@ struct rte_flow *flow; uint8_t i; + fs_lock(dev, 0); flow = fs_flow_allocate(attr, patterns, actions); FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) { flow->flows[i] = rte_flow_create(PORT_ID(sdev), @@ -90,6 +94,7 @@ } } TAILQ_INSERT_TAIL(&PRIV(dev)->flow_list, flow, next); + fs_unlock(dev, 0); return flow; err: FOREACH_SUBDEV(sdev, i, dev) { @@ -98,6 +103,7 @@ flow->flows[i], error); } fs_flow_release(&flow); + fs_unlock(dev, 0); return NULL; } @@ -115,6 +121,7 @@ return -EINVAL; } ret = 0; + fs_lock(dev, 0); FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) { int local_ret; @@ -131,6 +138,7 @@ } TAILQ_REMOVE(&PRIV(dev)->flow_list, flow, next); fs_flow_release(&flow); + fs_unlock(dev, 0); return ret; } @@ -144,12 +152,14 @@ uint8_t i; int ret; + fs_lock(dev, 0); FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) { DEBUG("Calling rte_flow_flush on sub_device %d", i); ret = rte_flow_flush(PORT_ID(sdev), error); if ((ret = fs_err(sdev, ret))) { ERROR("Operation rte_flow_flush failed for sub_device %d" " with error %d", i, ret); + fs_unlock(dev, 0); return ret; } } @@ -157,6 +167,7 @@ TAILQ_REMOVE(&PRIV(dev)->flow_list, flow, next); fs_flow_release(&flow); } + fs_unlock(dev, 0); return 0; } @@ -169,15 +180,19 @@ { struct sub_device *sdev; + fs_lock(dev, 0); sdev = TX_SUBDEV(dev); if (sdev != NULL) { int ret = rte_flow_query(PORT_ID(sdev), flow->flows[SUB_ID(sdev)], type, arg, error); - if ((ret = fs_err(sdev, ret))) + if ((ret = fs_err(sdev, ret))) { + fs_unlock(dev, 0); return ret; + } } + fs_unlock(dev, 0); WARN("No active sub_device to query about its flow"); return -1; } @@ -191,6 +206,7 @@ uint8_t i; int ret; + fs_lock(dev, 0); FOREACH_SUBDEV(sdev, i, dev) { if (sdev->state < DEV_PROBED) continue; @@ -202,11 +218,13 @@ if ((ret = fs_err(sdev, ret))) { ERROR("Operation rte_flow_isolate failed for sub_device %d" " with error %d", i, ret); + fs_unlock(dev, 0); return ret; } sdev->flow_isolated = set; } PRIV(dev)->flow_isolated = set; + fs_unlock(dev, 0); return 0; } diff --git a/drivers/net/failsafe/failsafe_ops.c b/drivers/net/failsafe/failsafe_ops.c index f0e48c1..fe64c68 100644 --- a/drivers/net/failsafe/failsafe_ops.c +++ b/drivers/net/failsafe/failsafe_ops.c @@ -94,6 +94,7 @@ uint8_t i; int ret; + fs_lock(dev, 0); supp_tx_offloads = PRIV(dev)->infos.tx_offload_capa; tx_offloads = dev->data->dev_conf.txmode.offloads; if ((tx_offloads & supp_tx_offloads) != tx_offloads) { @@ -101,6 +102,7 @@ ERROR("Some Tx offloads are not supported, " "requested 0x%" PRIx64 " supported 0x%" PRIx64, tx_offloads, supp_tx_offloads); + fs_unlock(dev, 0); return -rte_errno; } FOREACH_SUBDEV(sdev, i, dev) { @@ -139,6 +141,7 @@ if (!fs_err(sdev, ret)) continue; ERROR("Could not configure sub_device %d", i); + fs_unlock(dev, 0); return ret; } if (rmv_interrupt) { @@ -165,6 +168,7 @@ } if (PRIV(dev)->state < DEV_ACTIVE) PRIV(dev)->state = DEV_ACTIVE; + fs_unlock(dev, 0); return 0; } @@ -175,9 +179,12 @@ uint8_t i; int ret; + fs_lock(dev, 0); ret = failsafe_rx_intr_install(dev); - if (ret) + if (ret) { + fs_unlock(dev, 0); return ret; + } FOREACH_SUBDEV(sdev, i, dev) { if (sdev->state != DEV_ACTIVE) continue; @@ -186,6 +193,7 @@ if (ret) { if (!fs_err(sdev, ret)) continue; + fs_unlock(dev, 0); return ret; } ret = failsafe_rx_intr_install_subdevice(sdev); @@ -193,6 +201,7 @@ if (!fs_err(sdev, ret)) continue; rte_eth_dev_stop(PORT_ID(sdev)); + fs_unlock(dev, 0); return ret; } sdev->state = DEV_STARTED; @@ -200,6 +209,7 @@ if (PRIV(dev)->state < DEV_STARTED) PRIV(dev)->state = DEV_STARTED; fs_switch_dev(dev, NULL); + fs_unlock(dev, 0); return 0; } @@ -209,6 +219,7 @@ struct sub_device *sdev; uint8_t i; + fs_lock(dev, 0); PRIV(dev)->state = DEV_STARTED - 1; FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_STARTED) { rte_eth_dev_stop(PORT_ID(sdev)); @@ -216,6 +227,7 @@ sdev->state = DEV_STARTED - 1; } failsafe_rx_intr_uninstall(dev); + fs_unlock(dev, 0); } static int @@ -225,15 +237,18 @@ uint8_t i; int ret; + fs_lock(dev, 0); FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) { DEBUG("Calling rte_eth_dev_set_link_up on sub_device %d", i); ret = rte_eth_dev_set_link_up(PORT_ID(sdev)); if ((ret = fs_err(sdev, ret))) { ERROR("Operation rte_eth_dev_set_link_up failed for sub_device %d" " with error %d", i, ret); + fs_unlock(dev, 0); return ret; } } + fs_unlock(dev, 0); return 0; } @@ -244,15 +259,18 @@ uint8_t i; int ret; + fs_lock(dev, 0); FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) { DEBUG("Calling rte_eth_dev_set_link_down on sub_device %d", i); ret = rte_eth_dev_set_link_down(PORT_ID(sdev)); if ((ret = fs_err(sdev, ret))) { ERROR("Operation rte_eth_dev_set_link_down failed for sub_device %d" " with error %d", i, ret); + fs_unlock(dev, 0); return ret; } } + fs_unlock(dev, 0); return 0; } @@ -263,6 +281,7 @@ struct sub_device *sdev; uint8_t i; + fs_lock(dev, 0); failsafe_hotplug_alarm_cancel(dev); if (PRIV(dev)->state == DEV_STARTED) dev->dev_ops->dev_stop(dev); @@ -273,6 +292,7 @@ sdev->state = DEV_ACTIVE - 1; } fs_dev_free_queues(dev); + fs_unlock(dev, 0); } static bool @@ -305,14 +325,16 @@ if (queue == NULL) return; rxq = queue; + dev = rxq->priv->dev; + fs_lock(dev, 0); if (rxq->event_fd > 0) close(rxq->event_fd); - dev = rxq->priv->dev; FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) SUBOPS(sdev, rx_queue_release) (ETH(sdev)->data->rx_queues[rxq->qid]); dev->data->rx_queues[rxq->qid] = NULL; rte_free(rxq); + fs_unlock(dev, 0); } static int @@ -338,6 +360,7 @@ uint8_t i; int ret; + fs_lock(dev, 0); rxq = dev->data->rx_queues[rx_queue_id]; if (rxq != NULL) { fs_rx_queue_release(rxq); @@ -353,14 +376,17 @@ dev->data->dev_conf.rxmode.offloads, PRIV(dev)->infos.rx_offload_capa | PRIV(dev)->infos.rx_queue_offload_capa); + fs_unlock(dev, 0); return -rte_errno; } rxq = rte_zmalloc(NULL, sizeof(*rxq) + sizeof(rte_atomic64_t) * PRIV(dev)->subs_tail, RTE_CACHE_LINE_SIZE); - if (rxq == NULL) + if (rxq == NULL) { + fs_unlock(dev, 0); return -ENOMEM; + } FOREACH_SUBDEV(sdev, i, dev) rte_atomic64_init(&rxq->refcnt[i]); rxq->qid = rx_queue_id; @@ -371,8 +397,10 @@ rxq->priv = PRIV(dev); rxq->sdev = PRIV(dev)->subs; ret = rte_intr_efd_enable(&intr_handle, 1); - if (ret < 0) + if (ret < 0) { + fs_unlock(dev, 0); return ret; + } rxq->event_fd = intr_handle.efds[0]; dev->data->rx_queues[rx_queue_id] = rxq; FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) { @@ -385,9 +413,11 @@ goto free_rxq; } } + fs_unlock(dev, 0); return 0; free_rxq: fs_rx_queue_release(rxq); + fs_unlock(dev, 0); return ret; } @@ -400,20 +430,21 @@ int ret; int rc = 0; + fs_lock(dev, 0); if (idx >= dev->data->nb_rx_queues) { - rte_errno = EINVAL; - return -rte_errno; + rc = -EINVAL; + goto unlock; } rxq = dev->data->rx_queues[idx]; if (rxq == NULL || rxq->event_fd <= 0) { - rte_errno = EINVAL; - return -rte_errno; + rc = -EINVAL; + goto unlock; } /* Fail if proxy service is nor running. */ if (PRIV(dev)->rxp.sstate != SS_RUNNING) { ERROR("failsafe interrupt services are not running"); - rte_errno = EAGAIN; - return -rte_errno; + rc = -EAGAIN; + goto unlock; } rxq->enable_events = 1; FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) { @@ -422,6 +453,8 @@ if (ret) rc = ret; } +unlock: + fs_unlock(dev, 0); if (rc) rte_errno = -rc; return rc; @@ -437,14 +470,15 @@ int rc = 0; int ret; + fs_lock(dev, 0); if (idx >= dev->data->nb_rx_queues) { - rte_errno = EINVAL; - return -rte_errno; + rc = -EINVAL; + goto unlock; } rxq = dev->data->rx_queues[idx]; if (rxq == NULL || rxq->event_fd <= 0) { - rte_errno = EINVAL; - return -rte_errno; + rc = -EINVAL; + goto unlock; } rxq->enable_events = 0; FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) { @@ -456,6 +490,8 @@ /* Clear pending events */ while (read(rxq->event_fd, &u64, sizeof(uint64_t)) > 0) ; +unlock: + fs_unlock(dev, 0); if (rc) rte_errno = -rc; return rc; @@ -492,11 +528,13 @@ return; txq = queue; dev = txq->priv->dev; + fs_lock(dev, 0); FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) SUBOPS(sdev, tx_queue_release) (ETH(sdev)->data->tx_queues[txq->qid]); dev->data->tx_queues[txq->qid] = NULL; rte_free(txq); + fs_unlock(dev, 0); } static int @@ -511,6 +549,7 @@ uint8_t i; int ret; + fs_lock(dev, 0); txq = dev->data->tx_queues[tx_queue_id]; if (txq != NULL) { fs_tx_queue_release(txq); @@ -531,14 +570,17 @@ dev->data->dev_conf.txmode.offloads, PRIV(dev)->infos.tx_offload_capa | PRIV(dev)->infos.tx_queue_offload_capa); + fs_unlock(dev, 0); return -rte_errno; } txq = rte_zmalloc("ethdev TX queue", sizeof(*txq) + sizeof(rte_atomic64_t) * PRIV(dev)->subs_tail, RTE_CACHE_LINE_SIZE); - if (txq == NULL) + if (txq == NULL) { + fs_unlock(dev, 0); return -ENOMEM; + } FOREACH_SUBDEV(sdev, i, dev) rte_atomic64_init(&txq->refcnt[i]); txq->qid = tx_queue_id; @@ -557,9 +599,11 @@ goto free_txq; } } + fs_unlock(dev, 0); return 0; free_txq: fs_tx_queue_release(txq); + fs_unlock(dev, 0); return ret; } @@ -586,8 +630,10 @@ struct sub_device *sdev; uint8_t i; + fs_lock(dev, 0); FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) rte_eth_promiscuous_enable(PORT_ID(sdev)); + fs_unlock(dev, 0); } static void @@ -596,8 +642,10 @@ struct sub_device *sdev; uint8_t i; + fs_lock(dev, 0); FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) rte_eth_promiscuous_disable(PORT_ID(sdev)); + fs_unlock(dev, 0); } static void @@ -606,8 +654,10 @@ struct sub_device *sdev; uint8_t i; + fs_lock(dev, 0); FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) rte_eth_allmulticast_enable(PORT_ID(sdev)); + fs_unlock(dev, 0); } static void @@ -616,8 +666,10 @@ struct sub_device *sdev; uint8_t i; + fs_lock(dev, 0); FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) rte_eth_allmulticast_disable(PORT_ID(sdev)); + fs_unlock(dev, 0); } static int @@ -628,6 +680,7 @@ uint8_t i; int ret; + fs_lock(dev, 0); FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) { DEBUG("Calling link_update on sub_device %d", i); ret = (SUBOPS(sdev, link_update))(ETH(sdev), wait_to_complete); @@ -635,6 +688,7 @@ rte_eth_dev_is_removed(PORT_ID(sdev)) == 0) { ERROR("Link update failed for sub_device %d with error %d", i, ret); + fs_unlock(dev, 0); return ret; } } @@ -646,9 +700,11 @@ l2 = Ð(TX_SUBDEV(dev))->data->dev_link; if (memcmp(l1, l2, sizeof(*l1))) { *l1 = *l2; + fs_unlock(dev, 0); return 0; } } + fs_unlock(dev, 0); return -1; } @@ -661,6 +717,7 @@ uint8_t i; int ret; + fs_lock(dev, 0); rte_memcpy(stats, &PRIV(dev)->stats_accumulator, sizeof(*stats)); FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) { struct rte_eth_stats *snapshot = &sdev->stats_snapshot.stats; @@ -676,12 +733,14 @@ ERROR("Operation rte_eth_stats_get failed for sub_device %d with error %d", i, ret); *timestamp = 0; + fs_unlock(dev, 0); return ret; } *timestamp = rte_rdtsc(); inc: failsafe_stats_increment(stats, snapshot); } + fs_unlock(dev, 0); return 0; } @@ -691,11 +750,13 @@ struct sub_device *sdev; uint8_t i; + fs_lock(dev, 0); FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) { rte_eth_stats_reset(PORT_ID(sdev)); memset(&sdev->stats_snapshot, 0, sizeof(struct rte_eth_stats)); } memset(&PRIV(dev)->stats_accumulator, 0, sizeof(struct rte_eth_stats)); + fs_unlock(dev, 0); } /** @@ -771,14 +832,20 @@ { struct sub_device *sdev; struct rte_eth_dev *edev; + const uint32_t *ret; + fs_lock(dev, 0); sdev = TX_SUBDEV(dev); - if (sdev == NULL) - return NULL; + if (sdev == NULL) { + ret = NULL; + goto unlock; + } edev = ETH(sdev); /* ENOTSUP: counts as no supported ptypes */ - if (SUBOPS(sdev, dev_supported_ptypes_get) == NULL) - return NULL; + if (SUBOPS(sdev, dev_supported_ptypes_get) == NULL) { + ret = NULL; + goto unlock; + } /* * The API does not permit to do a clean AND of all ptypes, * It is also incomplete by design and we do not really care @@ -786,7 +853,10 @@ * We just return the ptypes of the device of highest * priority, usually the PREFERRED device. */ - return SUBOPS(sdev, dev_supported_ptypes_get)(edev); + ret = SUBOPS(sdev, dev_supported_ptypes_get)(edev); +unlock: + fs_unlock(dev, 0); + return ret; } static int @@ -796,15 +866,18 @@ uint8_t i; int ret; + fs_lock(dev, 0); FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) { DEBUG("Calling rte_eth_dev_set_mtu on sub_device %d", i); ret = rte_eth_dev_set_mtu(PORT_ID(sdev), mtu); if ((ret = fs_err(sdev, ret))) { ERROR("Operation rte_eth_dev_set_mtu failed for sub_device %d with error %d", i, ret); + fs_unlock(dev, 0); return ret; } } + fs_unlock(dev, 0); return 0; } @@ -815,15 +888,18 @@ uint8_t i; int ret; + fs_lock(dev, 0); FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) { DEBUG("Calling rte_eth_dev_vlan_filter on sub_device %d", i); ret = rte_eth_dev_vlan_filter(PORT_ID(sdev), vlan_id, on); if ((ret = fs_err(sdev, ret))) { ERROR("Operation rte_eth_dev_vlan_filter failed for sub_device %d" " with error %d", i, ret); + fs_unlock(dev, 0); return ret; } } + fs_unlock(dev, 0); return 0; } @@ -832,13 +908,22 @@ struct rte_eth_fc_conf *fc_conf) { struct sub_device *sdev; + int ret; + fs_lock(dev, 0); sdev = TX_SUBDEV(dev); - if (sdev == NULL) - return 0; - if (SUBOPS(sdev, flow_ctrl_get) == NULL) - return -ENOTSUP; - return SUBOPS(sdev, flow_ctrl_get)(ETH(sdev), fc_conf); + if (sdev == NULL) { + ret = 0; + goto unlock; + } + if (SUBOPS(sdev, flow_ctrl_get) == NULL) { + ret = -ENOTSUP; + goto unlock; + } + ret = SUBOPS(sdev, flow_ctrl_get)(ETH(sdev), fc_conf); +unlock: + fs_unlock(dev, 0); + return ret; } static int @@ -849,15 +934,18 @@ uint8_t i; int ret; + fs_lock(dev, 0); FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) { DEBUG("Calling rte_eth_dev_flow_ctrl_set on sub_device %d", i); ret = rte_eth_dev_flow_ctrl_set(PORT_ID(sdev), fc_conf); if ((ret = fs_err(sdev, ret))) { ERROR("Operation rte_eth_dev_flow_ctrl_set failed for sub_device %d" " with error %d", i, ret); + fs_unlock(dev, 0); return ret; } } + fs_unlock(dev, 0); return 0; } @@ -867,6 +955,7 @@ struct sub_device *sdev; uint8_t i; + fs_lock(dev, 0); /* No check: already done within the rte_eth_dev_mac_addr_remove * call for the fail-safe device. */ @@ -874,6 +963,7 @@ rte_eth_dev_mac_addr_remove(PORT_ID(sdev), &dev->data->mac_addrs[index]); PRIV(dev)->mac_addr_pool[index] = 0; + fs_unlock(dev, 0); } static int @@ -887,11 +977,13 @@ uint8_t i; RTE_ASSERT(index < FAILSAFE_MAX_ETHADDR); + fs_lock(dev, 0); FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) { ret = rte_eth_dev_mac_addr_add(PORT_ID(sdev), mac_addr, vmdq); if ((ret = fs_err(sdev, ret))) { ERROR("Operation rte_eth_dev_mac_addr_add failed for sub_device %" PRIu8 " with error %d", i, ret); + fs_unlock(dev, 0); return ret; } } @@ -900,6 +992,7 @@ PRIV(dev)->nb_mac_addr = index; } PRIV(dev)->mac_addr_pool[index] = vmdq; + fs_unlock(dev, 0); return 0; } @@ -909,8 +1002,10 @@ struct sub_device *sdev; uint8_t i; + fs_lock(dev, 0); FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) rte_eth_dev_default_mac_addr_set(PORT_ID(sdev), mac_addr); + fs_unlock(dev, 0); } static int @@ -928,15 +1023,18 @@ *(const void **)arg = &fs_flow_ops; return 0; } + fs_lock(dev, 0); FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) { DEBUG("Calling rte_eth_dev_filter_ctrl on sub_device %d", i); ret = rte_eth_dev_filter_ctrl(PORT_ID(sdev), type, op, arg); if ((ret = fs_err(sdev, ret))) { ERROR("Operation rte_eth_dev_filter_ctrl failed for sub_device %d" " with error %d", i, ret); + fs_unlock(dev, 0); return ret; } } + fs_unlock(dev, 0); return 0; } diff --git a/drivers/net/failsafe/failsafe_private.h b/drivers/net/failsafe/failsafe_private.h index f3be152..ef1f63b 100644 --- a/drivers/net/failsafe/failsafe_private.h +++ b/drivers/net/failsafe/failsafe_private.h @@ -7,6 +7,7 @@ #define _RTE_ETH_FAILSAFE_PRIVATE_H_ #include +#include #include #include @@ -161,6 +162,9 @@ struct fs_priv { * appropriate failsafe Rx queue. */ struct rx_proxy rxp; + pthread_mutex_t hotplug_mutex; + /* Hot-plug mutex is locked by the alarm mechanism. */ + volatile unsigned int alarm_lock:1; unsigned int pending_alarm:1; /* An alarm is pending */ /* flow isolation state */ int flow_isolated:1; @@ -255,12 +259,6 @@ int failsafe_eth_lsc_event_callback(uint16_t port_id, s != NULL; \ s = fs_find_next((dev), i + 1, state, &i)) -/** - * Iterator construct over fail-safe sub-devices: - * s: (struct sub_device *), iterator - * i: (uint8_t), increment - * dev: (struct rte_eth_dev *), fail-safe ethdev - */ #define FOREACH_SUBDEV(s, i, dev) \ FOREACH_SUBDEV_STATE(s, i, dev, DEV_UNDEFINED) @@ -347,6 +345,58 @@ int failsafe_eth_lsc_event_callback(uint16_t port_id, } /* + * Lock hot-plug mutex. + * is_alarm means that the caller is, for sure, the hot-plug alarm mechanism. + */ +static inline int +fs_lock(struct rte_eth_dev *dev, unsigned int is_alarm) +{ + int ret; + + if (is_alarm) { + ret = pthread_mutex_trylock(&PRIV(dev)->hotplug_mutex); + if (ret) { + DEBUG("Hot-plug mutex lock trying failed(%s), will try" + " again later...", strerror(ret)); + return ret; + } + PRIV(dev)->alarm_lock = 1; + } else { + ret = pthread_mutex_lock(&PRIV(dev)->hotplug_mutex); + if (ret) { + ERROR("Cannot lock mutex(%s)", strerror(ret)); + return ret; + } + } + DEBUG("Hot-plug mutex was locked by thread %lu%s", pthread_self(), + PRIV(dev)->alarm_lock ? " by the hot-plug alarm" : ""); + return ret; +} + +/* + * Unlock hot-plug mutex. + * is_alarm means that the caller is, for sure, the hot-plug alarm mechanism. + */ +static inline void +fs_unlock(struct rte_eth_dev *dev, unsigned int is_alarm) +{ + int ret; + unsigned int prev_alarm_lock = PRIV(dev)->alarm_lock; + + if (is_alarm) { + RTE_ASSERT(PRIV(dev)->alarm_lock == 1); + PRIV(dev)->alarm_lock = 0; + } + ret = pthread_mutex_unlock(&PRIV(dev)->hotplug_mutex); + if (ret) + ERROR("Cannot unlock hot-plug mutex(%s)", strerror(ret)); + else + DEBUG("Hot-plug mutex was unlocked by thread %lu%s", + pthread_self(), + prev_alarm_lock ? " by the hot-plug alarm" : ""); +} + +/* * Switch emitting device. * If banned is set, banned must not be considered for * the role of emitting device. -- 1.8.3.1