From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 55E2AA0C40; Fri, 16 Apr 2021 12:22:13 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id D0736141CCA; Fri, 16 Apr 2021 12:22:12 +0200 (CEST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by mails.dpdk.org (Postfix) with ESMTP id AF63E141CB2 for ; Fri, 16 Apr 2021 12:22:11 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1618568530; 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=0aCL2R/UU6jYRTTEMChUvBo4/7mVRKg+S+ncO9HOeqQ=; b=X7o9pJ/oIuPDNhi+SaE1Aa9l+4ZYFWNFdbhZe9b6R3z7hWdNq0ng6cjOfJ95rGE34UVowc 9lVXaD0j05Kto5O403okbWyOTs3NV/gbM6iF1I0Kg+A8wpAVKWVB76PjYDk3j8W5+7twSn tnmOMxMpzXZoDLuMhs0UnhwgERPGEfI= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-315-GObS3iBzPBSk2qb2ywwYgw-1; Fri, 16 Apr 2021 06:22:06 -0400 X-MC-Unique: GObS3iBzPBSk2qb2ywwYgw-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 735B79F92B; Fri, 16 Apr 2021 10:22:05 +0000 (UTC) Received: from [10.36.112.36] (ovpn-112-36.ams2.redhat.com [10.36.112.36]) by smtp.corp.redhat.com (Postfix) with ESMTP id B886E60BE5; Fri, 16 Apr 2021 10:22:03 +0000 (UTC) To: "Min Hu (Connor)" , dev@dpdk.org Cc: ferruh.yigit@intel.com, thomas@monjalon.net, andrew.rybchenko@oktetlabs.ru References: <1618046334-39857-1-git-send-email-humin29@huawei.com> <1618555931-25858-1-git-send-email-humin29@huawei.com> From: Kevin Traynor Message-ID: Date: Fri, 16 Apr 2021 11:22:02 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.1 MIME-Version: 1.0 In-Reply-To: <1618555931-25858-1-git-send-email-humin29@huawei.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=ktraynor@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH v6] ethdev: add sanity checks in control APIs 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" On 16/04/2021 07:52, Min Hu (Connor) wrote: > This patch adds more sanity checks in control path APIs. > > Fixes: 214ed1acd125 ("ethdev: add iterator to match devargs input") > Fixes: 3d98f921fbe9 ("ethdev: unify prefix for static functions and variables") > Fixes: 0366137722a0 ("ethdev: check for invalid device name") > Fixes: d948f596fee2 ("ethdev: fix port data mismatched in multiple process model") > Fixes: 5b7ba31148a8 ("ethdev: add port ownership") > Fixes: f8244c6399d9 ("ethdev: increase port id range") > Cc: stable@dpdk.org > > Signed-off-by: Min Hu (Connor) > Reviewed-by: Andrew Rybchenko > @@ -1298,9 +1342,15 @@ rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q, > uint16_t old_mtu; > > RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); > - > dev = &rte_eth_devices[port_id]; > > + if (dev_conf == NULL) { > + RTE_ETHDEV_LOG(ERR, > + "Cannot configure ethdev port %u to NULL dev_conf\n", The others use a natural sounding names instead of argument name. If you wanted to match that it could be "..to NULL conf" > + port_id); > + return -EINVAL; > + } > + > RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_configure, -ENOTSUP); > > if (dev->data->dev_started) { > @@ -2609,6 +2680,13 @@ rte_eth_link_get(uint16_t port_id, struct rte_eth_link *eth_link) > RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); > dev = &rte_eth_devices[port_id]; > > + if (eth_link == NULL) { > + RTE_ETHDEV_LOG(ERR, > + "Cannot get ethdev port %u link for NULL eth_link\n", ^^^ > + port_id); > + return -EINVAL; > + } > + > if (dev->data->dev_conf.intr_conf.lsc && > dev->data->dev_started) > rte_eth_linkstatus_get(dev, eth_link); > @@ -2629,6 +2707,13 @@ rte_eth_link_get_nowait(uint16_t port_id, struct rte_eth_link *eth_link) > RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV); > dev = &rte_eth_devices[port_id]; > > + if (eth_link == NULL) { > + RTE_ETHDEV_LOG(ERR, > + "Cannot get nowait ethdev port %u for NULL link\n", ^^^ I would probably stick with "link" for both these functions, rather than argument name "eth_link" in one "link" in other. > + port_id); > + return -EINVAL; > + } > + > if (dev->data->dev_conf.intr_conf.lsc && > dev->data->dev_started) > rte_eth_linkstatus_get(dev, eth_link); Thanks Connor. There are only minor nits, so Acked-by: Kevin Traynor