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 4E05AA0A0C; Fri, 2 Jul 2021 12:08:46 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id ECDAE41353; Fri, 2 Jul 2021 12:08:45 +0200 (CEST) Received: from shelob.oktetlabs.ru (shelob.oktetlabs.ru [91.220.146.113]) by mails.dpdk.org (Postfix) with ESMTP id 2C92340686 for ; Fri, 2 Jul 2021 12:08:44 +0200 (CEST) Received: from [192.168.38.17] (aros.oktetlabs.ru [192.168.38.17]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by shelob.oktetlabs.ru (Postfix) with ESMTPSA id A425D7F56B; Fri, 2 Jul 2021 13:08:43 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 shelob.oktetlabs.ru A425D7F56B DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=oktetlabs.ru; s=default; t=1625220523; bh=P9avAmvAt+KmA1kZWxaYDaJclEoFDLIOQvumXNAZmgQ=; h=Subject:To:Cc:References:From:Date:In-Reply-To; b=rrAek1xtt/XUvolDFh69A8dDHJeJBf/P+68Bi6gEaPi2upx3I+MG9WG56v6+Erqky czZR/l7SqH6Ng/3Te5viJsC4JaP2ynJMk4ALcKLo1YDDK7HGhF162EbKLVFReRigur Co7hMQ5mMSEzkDLtlkbS18EtN8CcaYNrQjO5m40Q= To: Huisong Li , dev@dpdk.org Cc: ferruh.yigit@intel.com, Thomas Monjalon References: <1620460836-38506-1-git-send-email-lihuisong@huawei.com> <644f214d-887a-02ae-5476-af8e6d100221@huawei.com> From: Andrew Rybchenko Organization: OKTET Labs Message-ID: <256aa58e-7455-e861-ee22-eb87ddd2db67@oktetlabs.ru> Date: Fri, 2 Jul 2021 13:08:43 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: <644f214d-887a-02ae-5476-af8e6d100221@huawei.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [RFC] lib/ethdev: add dev configured flag 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" @Thomas, @Ferruh, I tend to accept it (with minor style fixes), but I need your opinion on it before doing it. Thanks, Andrew. On 6/29/21 5:27 AM, Huisong Li wrote: > > 在 2021/6/14 23:37, Andrew Rybchenko 写道: >> Summary should start from "ethdev: " >> >> Don't forget to include all maintainers in Cc the next time. >> Just use --cc-cmd or --to-cmd options. > ok, thanks! >> >> Adding Thomas. >> >> On 5/8/21 11:00 AM, Huisong Li wrote: >>> Currently, if dev_configure is not invoked or fails to be invoked, users >>> can still invoke dev_start successfully. This patch adds a >>> "dev_configured" >>> flag in "rte_eth_dev_data" to control whether dev_start can be invoked. >> >> In theory there is an indirect condition. If number of configured Tx >> *and* Rx queues is 0, device is not configured. > > That's true. If the framework doesn't have this check, each driver needs > to do this. > > But it's a common thing, and it's probably more reasonable to put it in > the ethdev layer. > >> >> I have no strong opinion on the topic. Extra flag requires >> extra housekeeping. Indirect conditions are not always good >> and could be a subject to change. >> >>> Signed-off-by: Huisong Li >>> --- >>>   lib/ethdev/rte_ethdev.c      | 11 +++++++++++ >>>   lib/ethdev/rte_ethdev_core.h |  6 +++++- >>>   2 files changed, 16 insertions(+), 1 deletion(-) >>> >>> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c >>> index a187976..7d74b17 100644 >>> --- a/lib/ethdev/rte_ethdev.c >>> +++ b/lib/ethdev/rte_ethdev.c >>> @@ -1604,6 +1604,8 @@ rte_eth_dev_configure(uint16_t port_id, >>> uint16_t nb_rx_q, uint16_t nb_tx_q, >>>       } >>>         rte_ethdev_trace_configure(port_id, nb_rx_q, nb_tx_q, >>> dev_conf, 0); >>> +    dev->data->dev_configured = 1; >>> + >>>       return 0; >>>   reset_queues: >>>       eth_dev_rx_queue_config(dev, 0); >>> @@ -1614,6 +1616,8 @@ rte_eth_dev_configure(uint16_t port_id, >>> uint16_t nb_rx_q, uint16_t nb_tx_q, >>>           dev->data->mtu = old_mtu; >>>         rte_ethdev_trace_configure(port_id, nb_rx_q, nb_tx_q, >>> dev_conf, ret); >>> +    dev->data->dev_configured = 0; >>> + >>>       return ret; >>>   } >>>   @@ -1749,6 +1753,13 @@ rte_eth_dev_start(uint16_t port_id) >>>         RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_start, -ENOTSUP); >>>   +    if (dev->data->dev_configured == 0) { >>> +        RTE_ETHDEV_LOG(INFO, >>> +            "Device with port_id=%"PRIu16" is not configured.\n", >>> +            port_id); >>> +        return -EINVAL; >>> +    } >>> + >>>       if (dev->data->dev_started != 0) { >>>           RTE_ETHDEV_LOG(INFO, >>>               "Device with port_id=%"PRIu16" already started\n", >>> diff --git a/lib/ethdev/rte_ethdev_core.h b/lib/ethdev/rte_ethdev_core.h >>> index 4679d94..b508769 100644 >>> --- a/lib/ethdev/rte_ethdev_core.h >>> +++ b/lib/ethdev/rte_ethdev_core.h >>> @@ -167,7 +167,11 @@ struct rte_eth_dev_data { >>>           scattered_rx : 1,  /**< RX of scattered packets is ON(1) / >>> OFF(0) */ >>>           all_multicast : 1, /**< RX all multicast mode ON(1) / >>> OFF(0). */ >>>           dev_started : 1,   /**< Device state: STARTED(1) / >>> STOPPED(0). */ >>> -        lro         : 1;   /**< RX LRO is ON(1) / OFF(0) */ >>> +        lro         : 1,  /**< RX LRO is ON(1) / OFF(0) */ >>> +        dev_configured : 1; >>> +        /**< Device configuration state: >>> +         * CONFIGURED(1) / NOT CONFIGURED(0). >>> +         */ >>>       uint8_t rx_queue_state[RTE_MAX_QUEUES_PER_PORT]; >>>           /**< Queues state: HAIRPIN(2) / STARTED(1) / STOPPED(0). */ >>>       uint8_t tx_queue_state[RTE_MAX_QUEUES_PER_PORT]; >>> >> >> .