[dpdk-dev] [V5 1/3] ethdev: introduce FEC API
Andrew Rybchenko
arybchenko at solarflare.com
Sat Sep 19 10:42:01 CEST 2020
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) <humin29 at huawei.com>
>>> Reviewed-by: Wei Hu (Xavier) <xavier.huwei at huawei.com>
>>> Reviewed-by: Chengwen Feng <fengchengwen at huawei.com>
>>> Reviewed-by: Chengchang Tang <tangchengchang at huawei.com>
>>> ---
>>> 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]
>> .
>>
More information about the dev
mailing list