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 557A4A0542; Wed, 5 Oct 2022 03:56:37 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id EB89540694; Wed, 5 Oct 2022 03:56:34 +0200 (CEST) Received: from szxga01-in.huawei.com (szxga01-in.huawei.com [45.249.212.187]) by mails.dpdk.org (Postfix) with ESMTP id 4D97B40041 for ; Wed, 5 Oct 2022 03:56:32 +0200 (CEST) Received: from dggpeml500024.china.huawei.com (unknown [172.30.72.53]) by szxga01-in.huawei.com (SkyGuard) with ESMTP id 4MhyHR5pl0zlXX7; Wed, 5 Oct 2022 09:52:03 +0800 (CST) Received: from [10.82.212.125] (10.82.212.125) by dggpeml500024.china.huawei.com (7.185.36.10) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.31; Wed, 5 Oct 2022 09:56:28 +0800 Message-ID: Date: Wed, 5 Oct 2022 09:56:29 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.2.1 Subject: Re: [PATCH v9 1/5] ethdev: support get port error handling mode To: Ferruh Yigit , CC: , , , , , References: <20220128124831.427-1-kalesh-anakkur.purayil@broadcom.com> <20220922074151.39450-1-fengchengwen@huawei.com> <20220922074151.39450-2-fengchengwen@huawei.com> Content-Language: en-US From: fengchengwen In-Reply-To: Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.82.212.125] X-ClientProxiedBy: dggpeml500023.china.huawei.com (7.185.36.114) To dggpeml500024.china.huawei.com (7.185.36.10) X-CFilter-Loop: Reflected 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 Hi Ferruh, On 2022/10/4 1:35, Ferruh Yigit wrote: > On 9/22/2022 8:41 AM, Chengwen Feng wrote: >> This patch support gets port's error handling mode by >> rte_eth_dev_info_get() API. >> >> Currently, the defined modes include: >> 1) NONE: it means no error handling modes are supported by this port. >> 2) PASSIVE: passive error handling, after the PMD detect that a reset >> is required, the PMD reports RTE_ETH_EVENT_INTR_RESET event, and >> application invoke rte_eth_dev_reset() to recover the port. >> >> Signed-off-by: Chengwen Feng > > <...> > >> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h >> index de9e970d4d..930b0a2fff 100644 >> --- a/lib/ethdev/rte_ethdev.h >> +++ b/lib/ethdev/rte_ethdev.h >> @@ -1848,6 +1848,19 @@ enum rte_eth_representor_type { >>       RTE_ETH_REPRESENTOR_PF,   /**< representor of Physical >> Function. */ >>   }; >>   +/** >> + * Ethernet device error handling mode. > > Needs to be experimental, if decides to keep. will fix in v10 > >> + */ >> +enum rte_eth_err_handle_mode { >> +    /** No error handling modes are supported. */ >> +    RTE_ETH_ERROR_HANDLE_MODE_NONE, >> +    /** Passive error handling, after the PMD detect that a reset is >> +     * required, the PMD reports @see RTE_ETH_EVENT_INTR_RESET >> event, and >> +     * application invoke @see rte_eth_dev_reset to recover the port. >> +     */ >> +    RTE_ETH_ERROR_HANDLE_MODE_PASSIVE, > > Hi Chengwen, > > Is the intention of 'PASSIVE' / 'PROACTIVE' mode to let application > decide which event to register? Like some kind of capability? > > If mode == RTE_ETH_ERROR_HANDLE_MODE_PASSIVE >     register RTE_ETH_EVENT_INTR_RESET > > if mode == RTE_ETH_ERROR_HANDLE_MODE_PROACTIVE >     register ERR_RECOVERING | RECOVERY_SUCCESS | RECOVERY_FAILED > It mainly to standardize the two error handling modes to avoid poor perception. In the concept space, the reset mode is separated, so that it is not difficult to understand. > Can't a PMD support both? Currently, I find that no PMD support both. If the new PMD supports both the two types, it can be extended well, for example, it can be defined as bitmask, and value should not change because PASSIVE correspond 1(1<<0), and PROACTIVE correspond 2(1<<1) > > Or is application really needs to know this, what happens if it > register all events and implements related actions for it? For simpler, the application could register all events and do what the framework requirements. > > >> +}; >> + >>   /** >>    * A structure used to retrieve the contextual information of >>    * an Ethernet device, such as the controlling driver of the >> @@ -1908,8 +1921,12 @@ struct rte_eth_dev_info { >>        * embedded managed interconnect/switch. >>        */ >>       struct rte_eth_switch_info switch_info; >> +    /** Supported error handling mode. @see enum >> rte_eth_err_handle_mode */ >> +    uint8_t err_handle_mode; > > I guess 'uint8_t' is used to save space, but 'enum' is mostly integer > (although as far as I remember compiler can select smaller type is > cases fit it), so I concern if it case any warning. If not agree to > use smaller type, since we know possible number of handler type is > limited and small. Yes, uint8_t is used to save space. It will depend on compiler if use enum here, so I think it's OK to use deterministic type. as for warning, I have not get such converting warning yeth. > >> -    uint64_t reserved_64s[2]; /**< Reserved for future fields */ >> +    uint8_t reserved_8;       /**< Reserved for future fields  */ >> +    uint16_t reserved_16s[3]; /**< Reserved for future fields  */ >> +    uint64_t reserved_64;     /**< Reserved for future fields */ >>       void *reserved_ptrs[2];   /**< Reserved for future fields */ >>   }; >