From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id F22E7A04C8; Sat, 19 Sep 2020 10:42:33 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 3FC231D98A; Sat, 19 Sep 2020 10:42:32 +0200 (CEST) Received: from dispatch1-us1.ppe-hosted.com (dispatch1-us1.ppe-hosted.com [67.231.154.164]) by dpdk.org (Postfix) with ESMTP id C44261D989 for ; Sat, 19 Sep 2020 10:42:30 +0200 (CEST) Received: from mx1-us1.ppe-hosted.com (unknown [10.110.50.150]) by dispatch1-us1.ppe-hosted.com (PPE Hosted ESMTP Server) with ESMTP id 575382004F; Sat, 19 Sep 2020 08:42:30 +0000 (UTC) Received: from us4-mdac16-36.at1.mdlocal (unknown [10.110.51.51]) by mx1-us1.ppe-hosted.com (PPE Hosted ESMTP Server) with ESMTP id 55C23800A3; Sat, 19 Sep 2020 08:42:30 +0000 (UTC) X-Virus-Scanned: Proofpoint Essentials engine Received: from mx1-us1.ppe-hosted.com (unknown [10.110.49.32]) by mx1-us1.ppe-hosted.com (PPE Hosted ESMTP Server) with ESMTPS id E5B0610004F; Sat, 19 Sep 2020 08:42:29 +0000 (UTC) Received: from webmail.solarflare.com (uk.solarflare.com [193.34.186.16]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by mx1-us1.ppe-hosted.com (PPE Hosted ESMTP Server) with ESMTPS id 72BEF280064; Sat, 19 Sep 2020 08:42:29 +0000 (UTC) Received: from [127.0.0.27] (10.17.10.39) by ukex01.SolarFlarecom.com (10.17.10.4) with Microsoft SMTP Server (TLS) id 15.0.1497.2; Sat, 19 Sep 2020 09:42:21 +0100 To: "Min Hu (Connor)" , CC: , , , References: <1599534347-20430-1-git-send-email-humin29@huawei.com> <1600332730-44952-1-git-send-email-humin29@huawei.com> <1600332730-44952-2-git-send-email-humin29@huawei.com> From: Andrew Rybchenko Message-ID: <92806f6d-5bb9-26fa-da50-f6b7ce179a4c@solarflare.com> Date: Sat, 19 Sep 2020 11:42:01 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.12.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8"; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit X-Originating-IP: [10.17.10.39] X-ClientProxiedBy: ocex03.SolarFlarecom.com (10.20.40.36) To ukex01.SolarFlarecom.com (10.17.10.4) X-TM-AS-Product-Ver: SMEX-12.5.0.1300-8.6.1012-25674.003 X-TM-AS-Result: No-14.447000-8.000000-10 X-TMASE-MatchedRID: Jm7Yxmmj9OnmLzc6AOD8DfHkpkyUphL9G3FHx4IjDyP5+tteD5RzhVem QtpKzZIz2qLJOCm+Gy+7oI/ypOeHzpcvKjLILi9VLbjXqdzdtCXR+aCnUtmPaMZltfe/nRav7+h Y9l7Y+OydfavKlXhz5d0uMt6NxjogeRuGNbY/bU12GcWKGZufBXkamQDZ6bhfB2QWi8BF5Sgs8m 1alsmNjcH8WmACRbCuZtENVifdVrNwjdRu/7BbM/VY7U3NX8JgTIR/yqWMcrtpsnGGIgWMmT5lm +VDBI4E/5YyiAZ6NU+tgInUmFisf4BW3tUfIrvS4TnFTgFBlsZXLrDwmXJ6bwyYpy5Q44xz7GL7 8QZli1PUXFOTuiuM1gzfxuSP0X+nioQNoRtg06uvJmjb249RXjeXamXCCu1Yy8ftIFhtAGQ+tME C3cTWKk8oJGCep2oiQ2+8uDemzPk3rw6h71XQ26JVTu7sjgg109pJeIuOy7sZSz1vvG+0mvOPRa /sN+oGC7dsYCBYYXsoMSpqJmWSODcpdZ3fQiLdgxsfzkNRlfKx5amWK2anSPoLR4+zsDTteLhcS g9d5zFt3tz8lGct05YtKbqW8jQoacZzyNex1Z5P3FNr9SBj8w== X-TM-AS-User-Approved-Sender: Yes X-TM-AS-User-Blocked-Sender: No X-TMASE-Result: 10--14.447000-8.000000 X-TMASE-Version: SMEX-12.5.0.1300-8.6.1012-25674.003 X-MDID: 1600504950-BwLPZl1EghHQ Subject: Re: [dpdk-dev] [V5 1/3] ethdev: introduce FEC API X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 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 9/18/20 12:28 PM, Min Hu (Connor) wrote: > > > 在 2020/9/17 17:58, Andrew Rybchenko 写道: >> On 9/17/20 11:52 AM, Min Hu (Connor) wrote: >>> This patch adds Forward error correction(FEC) support for ethdev. >>> Introduce APIs which support query and config FEC information in >>> hardware. >>> >>> Signed-off-by: Min Hu (Connor) >>> Reviewed-by: Wei Hu (Xavier) >>> Reviewed-by: Chengwen Feng >>> Reviewed-by: Chengchang Tang >>> --- >>> v4->v5: >>> Modifies FEC capa definitions using macros. >>> Add RTE_ prefix for public FEC mode enum. >>> add release notes about FEC for dpdk20_11. >>> >>> --- >>> v2->v3: >>> add function return value "-ENOTSUP" for API [snip] >>> diff --git a/lib/librte_ethdev/rte_ethdev.h >>> b/lib/librte_ethdev/rte_ethdev.h >>> index 70295d7..aa79326 100644 >>> --- a/lib/librte_ethdev/rte_ethdev.h >>> +++ b/lib/librte_ethdev/rte_ethdev.h >>> @@ -1511,6 +1511,25 @@ struct rte_eth_dcb_info { >>>       struct rte_eth_dcb_tc_queue_mapping tc_queue; >>>   }; >>> +/** >>> + * This enum indicates the possible (forward error correction)FEC modes >>> + * of an ethdev port. >>> + */ >>> +enum rte_fec_mode { >> >> enum rte_eth_fec_mode >> to be consistent with other enums and to have library prefix. >> > I fixed it in V6，thanks. >>> +    RTE_ETH_FEC_NOFEC = 0,      /**< FEC is off */ >>> +    RTE_ETH_FEC_BASER,          /**< FEC using common algorithm */ >>> +    RTE_ETH_FEC_RS,             /**< FEC using RS algorithm */ >>> +    RTE_ETH_FEC_AUTO,           /**< FEC autonegotiation modes */ >> >> May I suggest to put AUTO just after NOFEC. It would look move >> logical in the future when more FEC modes are added. It would >> look strange with AUTO in the middle of the list. >> >  hns3 PMD is the first to implement FEC function. The FEC mode order >  is in accordance with the bit definition in hardware FEC module. Sorry, it is not a good justification. It does not matter who is the first and who will be the last. >  in other ways, AUTO is flexible mode, which should not be differently >    treated,the location in last item of enum is ok. I still disagree. If it is OK for other ethdev maintainer - no problem, but I'd like to see explicit confirmation. [snip] >>> @@ -3328,6 +3347,70 @@ int  rte_eth_led_on(uint16_t port_id); >>>   int  rte_eth_led_off(uint16_t port_id); >>>   /** >>> + * @warning >>> + * @b EXPERIMENTAL: this API may change, or be removed, without >>> prior notice >>> + * >>> + * Get Forward Error Correction(FEC) capability. >>> + * >>> + * @param port_id >>> + *   The port identifier of the Ethernet device. >>> + * @param fec_cap >>> + *   returns the FEC capability from the device, as follows: >>> + *   RTE_ETH_FEC_CAPA_NOFEC >>> + *   RTE_ETH_FEC_CAPA_BASER >>> + *   RTE_ETH_FEC_CAPA_RS >>> + *   RTE_ETH_FEC_CAPA_AUTO >> >> I'd like to see thoughts about different FEC modes for >> different link speed. How to report it? >> (sorry if I missed it before) > the relation between FEC mode and link speed, we can refer to > hns3 PMD code, as follows: > >     switch (mac->link_speed) { >     case ETH_SPEED_NUM_10G: >     case ETH_SPEED_NUM_40G: >         mode = RTE_ETH_FEC_CAPA_NOFEC | RTE_ETH_FEC_CAPA_BASER | >             RTE_ETH_FEC_CAPA_AUTO; >         break; >     case ETH_SPEED_NUM_25G: >     case ETH_SPEED_NUM_50G: >         mode = RTE_ETH_FEC_CAPA_NOFEC | RTE_ETH_FEC_CAPA_BASER | >             RTE_ETH_FEC_CAPA_RS | RTE_ETH_FEC_CAPA_AUTO; >         break; >     case ETH_SPEED_NUM_100G: >     case ETH_SPEED_NUM_200G: >         mode = RTE_ETH_FEC_CAPA_NOFEC | RTE_ETH_FEC_CAPA_RS | >             RTE_ETH_FEC_CAPA_AUTO; >         break; >     default: >         mode = 0; >         break; >     } I'm sorry, but it does not answer my question. I think we should change the API to report supported FEC modes per supported link speed. >>> + * @return >>> + *   - (0) if successful. >>> + *   - (-ENOTSUP) if underlying hardware OR driver doesn't support. >>> + *     that operation. >>> + *   - (-EIO) if device is removed. >>> + *   - (-ENODEV)  if *port_id* invalid. >>> + */ >>> +__rte_experimental >>> +int rte_eth_fec_get_capability(uint16_t port_id, uint32_t *fec_cap); >> >> [snip] >> . >>