From patchwork Thu Oct 21 21:42:15 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Vipul Ashri X-Patchwork-Id: 102609 X-Patchwork-Delegate: ferruh.yigit@amd.com 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 9AB87A0C43; Thu, 21 Oct 2021 23:43:09 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 1D17240040; Thu, 21 Oct 2021 23:43:09 +0200 (CEST) Received: from mx0a-00069f02.pphosted.com (mx0a-00069f02.pphosted.com [205.220.165.32]) by mails.dpdk.org (Postfix) with ESMTP id 042BB4003F; Thu, 21 Oct 2021 23:43:06 +0200 (CEST) Received: from pps.filterd (m0246627.ppops.net [127.0.0.1]) by mx0b-00069f02.pphosted.com (8.16.1.2/8.16.1.2) with SMTP id 19LLCuNc030748; Thu, 21 Oct 2021 21:43:05 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oracle.com; h=from : to : cc : subject : date : message-id : in-reply-to : references : mime-version : content-transfer-encoding; s=corp-2021-07-09; bh=0pEiVA7/UmeapkF8eb0abs3OMrfNwZ7QROVq8B6JqJU=; b=dWCE67J7LKDTDS6Yt5q+LwnYqOD9Zknb+zulnIOIdvVY7IBOmciGsEjvrglc57ajWzuw mv9COQyHKgYW+aYSlcOyiTyJFbY4wtcUiz3kmK5HYi+VHDx3Cx1cfMdVq8fuCchCEkWX OGa1xjzfemle7oGFlmOO295Zv3R5FIXW+giFUyhaKxz7rAHSH00czCriBAZz93I3LUqC x5euZB7rdgKLIYYRA6+lAjPu5nlYo3XxdKhyfAjz5kaRoLJ+YEcmmSdqNMdIbkH1fcOO ACOwskQvs5BDNxxpMsj/+n805fuThVqfOjf30qqWaIo+ML6keqYs27p+IDRgS2VkXpXO MA== Received: from userp3020.oracle.com (userp3020.oracle.com [156.151.31.79]) by mx0b-00069f02.pphosted.com with ESMTP id 3btkxa1efg-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Thu, 21 Oct 2021 21:43:05 +0000 Received: from pps.filterd (userp3020.oracle.com [127.0.0.1]) by userp3020.oracle.com (8.16.1.2/8.16.1.2) with SMTP id 19LLfejf163312; Thu, 21 Oct 2021 21:43:03 GMT Received: from pps.reinject (localhost [127.0.0.1]) by userp3020.oracle.com with ESMTP id 3br8gwsscn-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Thu, 21 Oct 2021 21:43:03 +0000 Received: from userp3020.oracle.com (userp3020.oracle.com [127.0.0.1]) by pps.reinject (8.16.0.36/8.16.0.36) with SMTP id 19LLh3jk167265; Thu, 21 Oct 2021 21:43:03 GMT Received: from vashri-in.in.oracle.com (dhcp-10-166-165-156.vpn.oracle.com [10.166.165.156]) by userp3020.oracle.com with ESMTP id 3br8gwss9g-1; Thu, 21 Oct 2021 21:43:02 +0000 From: vipul.ashri@oracle.com To: dev@dpdk.org Cc: grive@u256.net, Vipul Ashri , stable@dpdk.org Date: Fri, 22 Oct 2021 03:12:15 +0530 Message-Id: <20211021214215.1633-1-vipul.ashri@oracle.com> X-Mailer: git-send-email 2.28.0.windows.1 In-Reply-To: <20211021115139.2634-1-vipul.ashri@oracle.com> References: <20211021115139.2634-1-vipul.ashri@oracle.com> MIME-Version: 1.0 X-Proofpoint-ORIG-GUID: 3UxXJJZSs-94rElE_DZhbYvK1vKaHco0 X-Proofpoint-GUID: 3UxXJJZSs-94rElE_DZhbYvK1vKaHco0 Subject: [dpdk-dev] [PATCH v2] net/failsafe: link_update request crashing at boot 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 Sender: "dev" From: Vipul Ashri failsafe crashed while sending early link_update request during boot time initialization. Based on debugging we found failsafe device was good but sub- devices were progressing towards initialization and SUBOPS macro where expanding macro gives [partial_dev]->dev_ops->link_update() execution of which triggered crash because dev_ops==0. similar crash seen at failsafe_eth_dev_close() Failsafe driver need a separate check for subdevices similar to "RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);" which is called to almost every eth_dev function. Fixes: a46f8d5 ("net/failsafe: add fail-safe PMD") Cc: stable@dpdk.org Signed-off-by: Vipul Ashri --- drivers/net/failsafe/failsafe_ops.c | 45 +++++++++++++++++++++++-- drivers/net/failsafe/failsafe_private.h | 6 ++++ 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/drivers/net/failsafe/failsafe_ops.c b/drivers/net/failsafe/failsafe_ops.c index 29de39910c..8e128b9802 100644 --- a/drivers/net/failsafe/failsafe_ops.c +++ b/drivers/net/failsafe/failsafe_ops.c @@ -371,7 +371,8 @@ fs_rx_queue_release(struct rte_eth_dev *dev, uint16_t qid) close(rxq->event_fd); FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) { if (ETH(sdev)->data->rx_queues != NULL && - ETH(sdev)->data->rx_queues[rxq->qid] != NULL) + ETH(sdev)->data->rx_queues[rxq->qid] != NULL && + SUBDEV_VALID_PORTID(sdev)) SUBOPS(sdev, rx_queue_release)(ETH(sdev), rxq->qid); } dev->data->rx_queues[rxq->qid] = NULL; @@ -405,6 +406,12 @@ fs_rx_queue_setup(struct rte_eth_dev *dev, fs_lock(dev, 0); if (rx_conf->rx_deferred_start) { FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_PROBED) { + if (!SUBDEV_VALID_PORTID(sdev)) { + ERROR("%s: Invalid sub-device port_id=%u\n", + __func__, PORT_ID(sdev)); + fs_unlock(dev, 0); + return -ENODEV; + } if (SUBOPS(sdev, rx_queue_start) == NULL) { ERROR("Rx queue deferred start is not " "supported for subdevice %d", i); @@ -548,7 +555,8 @@ fs_tx_queue_release(struct rte_eth_dev *dev, uint16_t qid) fs_lock(dev, 0); FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) { if (ETH(sdev)->data->tx_queues != NULL && - ETH(sdev)->data->tx_queues[txq->qid] != NULL) + ETH(sdev)->data->tx_queues[txq->qid] != NULL && + SUBDEV_VALID_PORTID(sdev)) SUBOPS(sdev, tx_queue_release)(ETH(sdev), txq->qid); } dev->data->tx_queues[txq->qid] = NULL; @@ -571,6 +579,12 @@ fs_tx_queue_setup(struct rte_eth_dev *dev, fs_lock(dev, 0); if (tx_conf->tx_deferred_start) { FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_PROBED) { + if (!SUBDEV_VALID_PORTID(sdev)) { + ERROR("%s: Invalid sub-device port_id=%u\n", + __func__, PORT_ID(sdev)); + fs_unlock(dev, 0); + return -ENODEV; + } if (SUBOPS(sdev, tx_queue_start) == NULL) { ERROR("Tx queue deferred start is not " "supported for subdevice %d", i); @@ -645,6 +659,12 @@ failsafe_eth_dev_close(struct rte_eth_dev *dev) fs_lock(dev, 0); failsafe_hotplug_alarm_cancel(dev); if (PRIV(dev)->state == DEV_STARTED) { + if (!rte_eth_dev_is_valid_port(dev->data->port_id)) { + ERROR("%s: Invalid sub-device port_id=%u\n", + __func__, dev->data->port_id); + fs_unlock(dev, 0); + return -ENODEV; + } ret = dev->dev_ops->dev_stop(dev); if (ret != 0) { fs_unlock(dev, 0); @@ -827,6 +847,12 @@ fs_link_update(struct rte_eth_dev *dev, fs_lock(dev, 0); FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_ACTIVE) { + if (!SUBDEV_VALID_PORTID(sdev)) { + ERROR("%s: Invalid Sub-device port_id=%u\n", + __func__, PORT_ID(sdev)); + fs_unlock(dev, 0); + return -ENODEV; + } DEBUG("Calling link_update on sub_device %d", i); ret = (SUBOPS(sdev, link_update))(ETH(sdev), wait_to_complete); if (ret && ret != -1 && sdev->remove == 0 && @@ -1249,6 +1275,15 @@ fs_dev_supported_ptypes_get(struct rte_eth_dev *dev) goto unlock; } edev = ETH(sdev); + + if (!SUBDEV_VALID_PORTID(sdev)) { + ERROR("%s: Invalid TX_SUBDEV port_id=%u\n", + __func__, PORT_ID(sdev)); + rte_errno = -ENODEV; + ret = NULL; + goto unlock; + } + /* ENOTSUP: counts as no supported ptypes */ if (SUBOPS(sdev, dev_supported_ptypes_get) == NULL) { ret = NULL; @@ -1324,6 +1359,12 @@ fs_flow_ctrl_get(struct rte_eth_dev *dev, ret = 0; goto unlock; } + if (!SUBDEV_VALID_PORTID(sdev)) { + ERROR("%s: Invalid TX_SUBDEV port_id=%u\n", + __func__, PORT_ID(sdev)); + ret = -ENODEV; + goto unlock; + } if (SUBOPS(sdev, flow_ctrl_get) == NULL) { ret = -ENOTSUP; goto unlock; diff --git a/drivers/net/failsafe/failsafe_private.h b/drivers/net/failsafe/failsafe_private.h index cd39d103c6..0227060bcb 100644 --- a/drivers/net/failsafe/failsafe_private.h +++ b/drivers/net/failsafe/failsafe_private.h @@ -308,6 +308,12 @@ extern int failsafe_mac_from_arg; : (PRIV(dev)->subs[PRIV(dev)->subs_tx].state < DEV_PROBED ? NULL \ : &PRIV(dev)->subs[PRIV(dev)->subs_tx])) +/** + * check for fail-safe sub-device valid port + */ +#define SUBDEV_VALID_PORTID(s) \ + rte_eth_dev_is_valid_port(PORT_ID(s)) + /** * s: (struct sub_device *) * ops: (struct eth_dev_ops) member