From patchwork Fri Feb 24 15:11:39 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Marchand X-Patchwork-Id: 124537 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id C7EEC41D5F; Fri, 24 Feb 2023 16:13:39 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id CD20F42D4F; Fri, 24 Feb 2023 16:12:43 +0100 (CET) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mails.dpdk.org (Postfix) with ESMTP id B4C4540A87 for ; Fri, 24 Feb 2023 16:12:41 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1677251561; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=XH0Q63zUNT3pFmnDlCqConhXmxmDu4cPjrPGC8/AXiQ=; b=HPqlz70FTDJueT/HHIL7KEhqE1wRDvR7Yn9Iaj3zxKKe/wCPhC1mL8o0wan3wspmC1RggG 2p9MwpoB8JinIYsz5NBRvXlyrDREhY8q5BAubT4MzIhxYqdHJji+BDnFG8uctxhrvfK+PJ xyGnadaq1rimIpG6koWSUAHp9Ff4d0c= Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-568-X1JhKFepOZOQi8Ksfzrixw-1; Fri, 24 Feb 2023 10:12:37 -0500 X-MC-Unique: X1JhKFepOZOQi8Ksfzrixw-1 Received: from smtp.corp.redhat.com (int-mx10.intmail.prod.int.rdu2.redhat.com [10.11.54.10]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 8EC3E29A9CA0; Fri, 24 Feb 2023 15:12:37 +0000 (UTC) Received: from dmarchan.redhat.com (unknown [10.45.224.55]) by smtp.corp.redhat.com (Postfix) with ESMTP id A126A492B13; Fri, 24 Feb 2023 15:12:36 +0000 (UTC) From: David Marchand To: dev@dpdk.org Cc: thomas@monjalon.net, Gaetan Rivet Subject: [PATCH v2 16/20] net/failsafe: fix mutex locking Date: Fri, 24 Feb 2023 16:11:39 +0100 Message-Id: <20230224151143.3274897-17-david.marchand@redhat.com> In-Reply-To: <20230224151143.3274897-1-david.marchand@redhat.com> References: <20230224081642.2566619-1-david.marchand@redhat.com> <20230224151143.3274897-1-david.marchand@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.10 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org The pthread mutex API describes cases where locking might fail. Check fts_enter wrapper return code. Signed-off-by: David Marchand Acked-by: Gaetan Rivet --- drivers/net/failsafe/failsafe_ether.c | 3 +- drivers/net/failsafe/failsafe_flow.c | 23 +++-- drivers/net/failsafe/failsafe_ops.c | 142 +++++++++++++++++++------- 3 files changed, 123 insertions(+), 45 deletions(-) diff --git a/drivers/net/failsafe/failsafe_ether.c b/drivers/net/failsafe/failsafe_ether.c index 10b90fd837..031f3eb13f 100644 --- a/drivers/net/failsafe/failsafe_ether.c +++ b/drivers/net/failsafe/failsafe_ether.c @@ -592,7 +592,8 @@ failsafe_eth_rmv_event_callback(uint16_t port_id __rte_unused, { struct sub_device *sdev = cb_arg; - fs_lock(fs_dev(sdev), 0); + if (fs_lock(fs_dev(sdev), 0) != 0) + return -1; /* Switch as soon as possible tx_dev. */ fs_switch_dev(fs_dev(sdev), sdev); /* Use safe bursts in any case. */ diff --git a/drivers/net/failsafe/failsafe_flow.c b/drivers/net/failsafe/failsafe_flow.c index 354f9fec20..707e6c63b5 100644 --- a/drivers/net/failsafe/failsafe_flow.c +++ b/drivers/net/failsafe/failsafe_flow.c @@ -72,7 +72,9 @@ fs_flow_validate(struct rte_eth_dev *dev, uint8_t i; int ret; - fs_lock(dev, 0); + ret = fs_lock(dev, 0); + if (ret != 0) + return ret; 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), @@ -99,7 +101,8 @@ fs_flow_create(struct rte_eth_dev *dev, struct rte_flow *flow; uint8_t i; - fs_lock(dev, 0); + if (fs_lock(dev, 0) != 0) + return NULL; flow = fs_flow_allocate(attr, patterns, actions); FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) { flow->flows[i] = rte_flow_create(PORT_ID(sdev), @@ -137,8 +140,9 @@ fs_flow_destroy(struct rte_eth_dev *dev, ERROR("Invalid flow"); return -EINVAL; } - ret = 0; - fs_lock(dev, 0); + ret = fs_lock(dev, 0); + if (ret != 0) + return ret; FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) { int local_ret; @@ -169,7 +173,9 @@ fs_flow_flush(struct rte_eth_dev *dev, uint8_t i; int ret; - fs_lock(dev, 0); + ret = fs_lock(dev, 0); + if (ret != 0) + return ret; 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); @@ -197,7 +203,8 @@ fs_flow_query(struct rte_eth_dev *dev, { struct sub_device *sdev; - fs_lock(dev, 0); + if (fs_lock(dev, 0) != 0) + return -1; sdev = TX_SUBDEV(dev); if (sdev != NULL) { int ret = rte_flow_query(PORT_ID(sdev), @@ -223,7 +230,9 @@ fs_flow_isolate(struct rte_eth_dev *dev, uint8_t i; int ret; - fs_lock(dev, 0); + ret = fs_lock(dev, 0); + if (ret != 0) + return ret; FOREACH_SUBDEV(sdev, i, dev) { if (sdev->state < DEV_PROBED) continue; diff --git a/drivers/net/failsafe/failsafe_ops.c b/drivers/net/failsafe/failsafe_ops.c index d357e1bc83..35649b6244 100644 --- a/drivers/net/failsafe/failsafe_ops.c +++ b/drivers/net/failsafe/failsafe_ops.c @@ -28,7 +28,9 @@ fs_dev_configure(struct rte_eth_dev *dev) uint8_t i; int ret; - fs_lock(dev, 0); + ret = fs_lock(dev, 0); + if (ret != 0) + return ret; FOREACH_SUBDEV(sdev, i, dev) { int rmv_interrupt = 0; int lsc_interrupt = 0; @@ -129,7 +131,9 @@ fs_dev_start(struct rte_eth_dev *dev) uint8_t i; int ret; - fs_lock(dev, 0); + ret = fs_lock(dev, 0); + if (ret != 0) + return ret; ret = failsafe_rx_intr_install(dev); if (ret) { fs_unlock(dev, 0); @@ -189,7 +193,9 @@ fs_dev_stop(struct rte_eth_dev *dev) uint8_t i; int ret; - fs_lock(dev, 0); + ret = fs_lock(dev, 0); + if (ret != 0) + return ret; PRIV(dev)->state = DEV_STARTED - 1; FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_STARTED) { ret = rte_eth_dev_stop(PORT_ID(sdev)); @@ -217,7 +223,9 @@ fs_dev_set_link_up(struct rte_eth_dev *dev) uint8_t i; int ret; - fs_lock(dev, 0); + ret = fs_lock(dev, 0); + if (ret != 0) + return ret; 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)); @@ -239,7 +247,9 @@ fs_dev_set_link_down(struct rte_eth_dev *dev) uint8_t i; int ret; - fs_lock(dev, 0); + ret = fs_lock(dev, 0); + if (ret != 0) + return ret; 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)); @@ -263,7 +273,9 @@ fs_rx_queue_stop(struct rte_eth_dev *dev, uint16_t rx_queue_id) int err = 0; bool failure = true; - fs_lock(dev, 0); + ret = fs_lock(dev, 0); + if (ret != 0) + return ret; FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) { uint16_t port_id = ETH(sdev)->data->port_id; @@ -289,7 +301,9 @@ fs_rx_queue_start(struct rte_eth_dev *dev, uint16_t rx_queue_id) uint8_t i; int ret; - fs_lock(dev, 0); + ret = fs_lock(dev, 0); + if (ret != 0) + return ret; FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) { uint16_t port_id = ETH(sdev)->data->port_id; @@ -316,7 +330,9 @@ fs_tx_queue_stop(struct rte_eth_dev *dev, uint16_t tx_queue_id) int err = 0; bool failure = true; - fs_lock(dev, 0); + ret = fs_lock(dev, 0); + if (ret != 0) + return ret; FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) { uint16_t port_id = ETH(sdev)->data->port_id; @@ -342,7 +358,9 @@ fs_tx_queue_start(struct rte_eth_dev *dev, uint16_t tx_queue_id) uint8_t i; int ret; - fs_lock(dev, 0); + ret = fs_lock(dev, 0); + if (ret != 0) + return ret; FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) { uint16_t port_id = ETH(sdev)->data->port_id; @@ -369,7 +387,8 @@ fs_rx_queue_release(struct rte_eth_dev *dev, uint16_t qid) if (rxq == NULL) return; - fs_lock(dev, 0); + if (fs_lock(dev, 0) != 0) + return; if (rxq->event_fd >= 0) close(rxq->event_fd); FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) { @@ -395,7 +414,9 @@ fs_rx_queue_setup(struct rte_eth_dev *dev, uint8_t i; int ret; - fs_lock(dev, 0); + ret = fs_lock(dev, 0); + if (ret != 0) + return ret; if (rx_conf->rx_deferred_start) { FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_PROBED) { if (SUBOPS(sdev, rx_queue_start) == NULL) { @@ -466,7 +487,9 @@ fs_rx_intr_enable(struct rte_eth_dev *dev, uint16_t idx) int ret; int rc = 0; - fs_lock(dev, 0); + ret = fs_lock(dev, 0); + if (ret != 0) + return ret; if (idx >= dev->data->nb_rx_queues) { rc = -EINVAL; goto unlock; @@ -506,7 +529,9 @@ fs_rx_intr_disable(struct rte_eth_dev *dev, uint16_t idx) int rc = 0; int ret; - fs_lock(dev, 0); + ret = fs_lock(dev, 0); + if (ret != 0) + return ret; if (idx >= dev->data->nb_rx_queues) { rc = -EINVAL; goto unlock; @@ -542,7 +567,8 @@ fs_tx_queue_release(struct rte_eth_dev *dev, uint16_t qid) if (txq == NULL) return; - fs_lock(dev, 0); + if (fs_lock(dev, 0) != 0) + return; FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) { if (ETH(sdev)->data->tx_queues != NULL && ETH(sdev)->data->tx_queues[txq->qid] != NULL) @@ -565,7 +591,9 @@ fs_tx_queue_setup(struct rte_eth_dev *dev, uint8_t i; int ret; - fs_lock(dev, 0); + ret = fs_lock(dev, 0); + if (ret != 0) + return ret; if (tx_conf->tx_deferred_start) { FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_PROBED) { if (SUBOPS(sdev, tx_queue_start) == NULL) { @@ -639,7 +667,9 @@ failsafe_eth_dev_close(struct rte_eth_dev *dev) uint8_t i; int err, ret = 0; - fs_lock(dev, 0); + ret = fs_lock(dev, 0); + if (ret != 0) + return ret; failsafe_hotplug_alarm_cancel(dev); if (PRIV(dev)->state == DEV_STARTED) { ret = dev->dev_ops->dev_stop(dev); @@ -693,7 +723,9 @@ fs_promiscuous_enable(struct rte_eth_dev *dev) uint8_t i; int ret = 0; - fs_lock(dev, 0); + ret = fs_lock(dev, 0); + if (ret != 0) + return ret; FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) { ret = rte_eth_promiscuous_enable(PORT_ID(sdev)); ret = fs_err(sdev, ret); @@ -725,7 +757,9 @@ fs_promiscuous_disable(struct rte_eth_dev *dev) uint8_t i; int ret = 0; - fs_lock(dev, 0); + ret = fs_lock(dev, 0); + if (ret != 0) + return ret; FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) { ret = rte_eth_promiscuous_disable(PORT_ID(sdev)); ret = fs_err(sdev, ret); @@ -757,7 +791,9 @@ fs_allmulticast_enable(struct rte_eth_dev *dev) uint8_t i; int ret = 0; - fs_lock(dev, 0); + ret = fs_lock(dev, 0); + if (ret != 0) + return ret; FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) { ret = rte_eth_allmulticast_enable(PORT_ID(sdev)); ret = fs_err(sdev, ret); @@ -789,7 +825,9 @@ fs_allmulticast_disable(struct rte_eth_dev *dev) uint8_t i; int ret = 0; - fs_lock(dev, 0); + ret = fs_lock(dev, 0); + if (ret != 0) + return ret; FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) { ret = rte_eth_allmulticast_disable(PORT_ID(sdev)); ret = fs_err(sdev, ret); @@ -822,7 +860,9 @@ fs_link_update(struct rte_eth_dev *dev, uint8_t i; int ret; - fs_lock(dev, 0); + ret = fs_lock(dev, 0); + if (ret != 0) + return ret; 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); @@ -859,7 +899,9 @@ fs_stats_get(struct rte_eth_dev *dev, uint8_t i; int ret; - fs_lock(dev, 0); + ret = fs_lock(dev, 0); + if (ret != 0) + return ret; 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; @@ -893,7 +935,9 @@ fs_stats_reset(struct rte_eth_dev *dev) uint8_t i; int ret; - fs_lock(dev, 0); + ret = fs_lock(dev, 0); + if (ret != 0) + return ret; FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) { ret = rte_eth_stats_reset(PORT_ID(sdev)); if (ret) { @@ -983,7 +1027,9 @@ fs_xstats_get_names(struct rte_eth_dev *dev, { int ret; - fs_lock(dev, 0); + ret = fs_lock(dev, 0); + if (ret != 0) + return ret; ret = __fs_xstats_get_names(dev, xstats_names, limit); fs_unlock(dev, 0); return ret; @@ -1035,7 +1081,9 @@ fs_xstats_get(struct rte_eth_dev *dev, { int ret; - fs_lock(dev, 0); + ret = fs_lock(dev, 0); + if (ret != 0) + return ret; ret = __fs_xstats_get(dev, xstats, n); fs_unlock(dev, 0); @@ -1048,9 +1096,11 @@ fs_xstats_reset(struct rte_eth_dev *dev) { struct sub_device *sdev; uint8_t i; - int r = 0; + int r; - fs_lock(dev, 0); + r = fs_lock(dev, 0); + if (r != 0) + return r; FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) { r = rte_eth_xstats_reset(PORT_ID(sdev)); if (r < 0) @@ -1238,7 +1288,8 @@ fs_dev_supported_ptypes_get(struct rte_eth_dev *dev) struct rte_eth_dev *edev; const uint32_t *ret; - fs_lock(dev, 0); + if (fs_lock(dev, 0) != 0) + return NULL; sdev = TX_SUBDEV(dev); if (sdev == NULL) { ret = NULL; @@ -1270,7 +1321,9 @@ fs_mtu_set(struct rte_eth_dev *dev, uint16_t mtu) uint8_t i; int ret; - fs_lock(dev, 0); + ret = fs_lock(dev, 0); + if (ret != 0) + return ret; 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); @@ -1292,7 +1345,9 @@ fs_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on) uint8_t i; int ret; - fs_lock(dev, 0); + ret = fs_lock(dev, 0); + if (ret != 0) + return ret; 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); @@ -1314,7 +1369,9 @@ fs_flow_ctrl_get(struct rte_eth_dev *dev, struct sub_device *sdev; int ret; - fs_lock(dev, 0); + ret = fs_lock(dev, 0); + if (ret != 0) + return ret; sdev = TX_SUBDEV(dev); if (sdev == NULL) { ret = 0; @@ -1338,7 +1395,9 @@ fs_flow_ctrl_set(struct rte_eth_dev *dev, uint8_t i; int ret; - fs_lock(dev, 0); + ret = fs_lock(dev, 0); + if (ret != 0) + return ret; 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); @@ -1359,7 +1418,8 @@ fs_mac_addr_remove(struct rte_eth_dev *dev, uint32_t index) struct sub_device *sdev; uint8_t i; - fs_lock(dev, 0); + if (fs_lock(dev, 0) != 0) + return; /* No check: already done within the rte_eth_dev_mac_addr_remove * call for the fail-safe device. */ @@ -1381,7 +1441,9 @@ fs_mac_addr_add(struct rte_eth_dev *dev, uint8_t i; RTE_ASSERT(index < FAILSAFE_MAX_ETHADDR); - fs_lock(dev, 0); + ret = fs_lock(dev, 0); + if (ret != 0) + return ret; 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))) { @@ -1407,7 +1469,9 @@ fs_mac_addr_set(struct rte_eth_dev *dev, struct rte_ether_addr *mac_addr) uint8_t i; int ret; - fs_lock(dev, 0); + ret = fs_lock(dev, 0); + if (ret != 0) + return ret; FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) { ret = rte_eth_dev_default_mac_addr_set(PORT_ID(sdev), mac_addr); ret = fs_err(sdev, ret); @@ -1432,7 +1496,9 @@ fs_set_mc_addr_list(struct rte_eth_dev *dev, int ret; void *mcast_addrs; - fs_lock(dev, 0); + ret = fs_lock(dev, 0); + if (ret != 0) + return ret; FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) { ret = rte_eth_dev_set_mc_addr_list(PORT_ID(sdev), @@ -1480,7 +1546,9 @@ fs_rss_hash_update(struct rte_eth_dev *dev, uint8_t i; int ret; - fs_lock(dev, 0); + ret = fs_lock(dev, 0); + if (ret != 0) + return ret; FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) { ret = rte_eth_dev_rss_hash_update(PORT_ID(sdev), rss_conf); ret = fs_err(sdev, ret);