[V12,1/4] ethdev: introduce FEC API
Checks
Commit Message
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@huawei.com>
Reviewed-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
Reviewed-by: Chengwen Feng <fengchengwen@huawei.com>
Reviewed-by: Chengchang Tang <tangchengchang@huawei.com>
Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
---
v10->v11:
allow to report capabilities per link speed.
specify what should be reported if link is down
when get FEC.
change mode to capa bitmask.
---
v9->v10:
add macro RTE_ETH_FEC_MODE_CAPA_MASK(x) to indicate
different FEC mode capa.
---
v8->v9:
added reviewed-by and acked-by.
---
v7->v8:
put AUTO just after NOFEC in rte_fec_mode definition.
---
v6->v7:
deleted RTE_ETH_FEC_NUM to prevent ABI breakage.
add new macro to indicate translation from fec mode
to capa.
---
v5->v6:
modified release notes.
deleted check duplicated for FEC API
fixed code styles according to DPDK coding style.
added _eth prefix.
---
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.
---
lib/librte_ethdev/rte_ethdev.c | 37 +++++++++++++
lib/librte_ethdev/rte_ethdev.h | 91 ++++++++++++++++++++++++++++++++
lib/librte_ethdev/rte_ethdev_driver.h | 82 ++++++++++++++++++++++++++++
lib/librte_ethdev/rte_ethdev_version.map | 3 ++
4 files changed, 213 insertions(+)
Comments
On 9/24/20 4:05 PM, 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.
Almost good now. See my notes below.
Many thanks for hard work and patience.
>
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> Reviewed-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
> Reviewed-by: Chengwen Feng <fengchengwen@huawei.com>
> Reviewed-by: Chengchang Tang <tangchengchang@huawei.com>
> Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
I think that tags for Ajit and Konstantin should be
dropped, since API changes significantly after their review.
> ---
> v10->v11:
> allow to report capabilities per link speed.
> specify what should be reported if link is down
> when get FEC.
> change mode to capa bitmask.
>
> ---
> v9->v10:
> add macro RTE_ETH_FEC_MODE_CAPA_MASK(x) to indicate
> different FEC mode capa.
>
> ---
> v8->v9:
> added reviewed-by and acked-by.
>
> ---
> v7->v8:
> put AUTO just after NOFEC in rte_fec_mode definition.
>
> ---
> v6->v7:
> deleted RTE_ETH_FEC_NUM to prevent ABI breakage.
> add new macro to indicate translation from fec mode
> to capa.
>
> ---
> v5->v6:
> modified release notes.
> deleted check duplicated for FEC API
> fixed code styles according to DPDK coding style.
> added _eth prefix.
>
> ---
> 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.
>
> ---
> lib/librte_ethdev/rte_ethdev.c | 37 +++++++++++++
> lib/librte_ethdev/rte_ethdev.h | 91 ++++++++++++++++++++++++++++++++
> lib/librte_ethdev/rte_ethdev_driver.h | 82 ++++++++++++++++++++++++++++
> lib/librte_ethdev/rte_ethdev_version.map | 3 ++
> 4 files changed, 213 insertions(+)
>
> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
> index dfe5c1b..b614bfc 100644
> --- a/lib/librte_ethdev/rte_ethdev.c
> +++ b/lib/librte_ethdev/rte_ethdev.c
> @@ -3679,6 +3679,43 @@ rte_eth_led_off(uint16_t port_id)
> return eth_err(port_id, (*dev->dev_ops->dev_led_off)(dev));
> }
>
> +int
> +rte_eth_fec_get_capability(uint16_t port_id, uint32_t *num,
> + struct rte_eth_fec_capa *speed_fec_capa)
> +{
> + struct rte_eth_dev *dev;
> +
> + if (num == NULL || speed_fec_capa == NULL)
> + return -EINVAL;
I think it is OK to have speed_fec_cap==NULL if *num is 0.
I.e. a request to get number of required array entries.
> +
> + RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> + dev = &rte_eth_devices[port_id];
> + RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->fec_get_capability, -ENOTSUP);
> + return eth_err(port_id, (*dev->dev_ops->fec_get_capability)(dev, num,
> + speed_fec_capa));
> +}
> +
> +int
> +rte_eth_fec_get(uint16_t port_id, uint32_t *mode)
> +{
> + struct rte_eth_dev *dev;
I think it would be good to check that mode is not NULL here.
> +
> + RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> + dev = &rte_eth_devices[port_id];
> + RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->fec_get, -ENOTSUP);
> + return eth_err(port_id, (*dev->dev_ops->fec_get)(dev, mode));
> +}
> +
> +int
> +rte_eth_fec_set(uint16_t port_id, uint32_t mode)
> +{
> + struct rte_eth_dev *dev;
> +
> + dev = &rte_eth_devices[port_id];
> + RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->fec_set, -ENOTSUP);
> + return eth_err(port_id, (*dev->dev_ops->fec_set)(dev, mode));
> +}
> +
> /*
> * Returns index into MAC address array of addr. Use 00:00:00:00:00:00 to find
> * an empty spot.
> diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
> index 645a186..104181d 100644
> --- a/lib/librte_ethdev/rte_ethdev.h
> +++ b/lib/librte_ethdev/rte_ethdev.h
> @@ -1544,6 +1544,29 @@ struct rte_eth_dcb_info {
> struct rte_eth_dcb_tc_queue_mapping tc_queue;
> };
>
> +/**
> + * This enum indicates the possible (forward error correction)FEC modes
(forward error correction)FEC -> Forward Error Correction (FEC)
> + * of an ethdev port.
> + */
> +enum rte_eth_fec_mode {
> + RTE_ETH_FEC_NOFEC = 0, /**< FEC is off */
> + RTE_ETH_FEC_AUTO, /**< FEC autonegotiation modes */
> + RTE_ETH_FEC_BASER, /**< FEC using common algorithm */
> + RTE_ETH_FEC_RS, /**< FEC using RS algorithm */
> +};
> +
> +/* Translate from FEC mode to FEC capa */
> +#define RTE_ETH_FEC_MODE_TO_CAPA(x) (1U << (x))
> +
> +/* This macro indicates FEC capa mask*/
Add missing space before */
> +#define RTE_ETH_FEC_MODE_CAPA_MASK(x) (1U << (RTE_ETH_FEC_ ## x))
> +
> +/* A structure used to get capabilities per link speed */
> +struct rte_eth_fec_capa {
> + uint32_t speed; /**< Link speed (see ETH_SPEED_NUM_*) */
> + uint32_t capa; /**< FEC capabilities bitmask (see RTE_FEC_CAPA_*) */
> +};
> +
> #define RTE_ETH_ALL RTE_MAX_ETHPORTS
>
> /* Macros to check for valid port */
> @@ -3397,6 +3420,74 @@ 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 num
> + * the num is in/out with a number of elements in an array.
Please, see below my notes on callback description.
> + * @param speed_fec_capa
> + * speed_fec_capa is out only with per-speed capabilities.
> + *
> + * @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.
> + * - (-EINVAL) if *num* or *speed_fec_capa* invalid
> + */
> +__rte_experimental
> +int rte_eth_fec_get_capability(uint16_t port_id, uint32_t *num,
> + struct rte_eth_fec_capa *speed_fec_capa);
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
> + *
> + * Get current Forward Error Correction(FEC) mode.
> + * If link is down and AUTO is enabled, AUTO is returned, otherwise,
> + * configured FEC mode is returned.
> + * If link is up, current FEC mode is returned.
> + *
> + * @param port_id
> + * The port identifier of the Ethernet device.
> + * @param mode
> + * returns the FEC mode from the device.
> + * @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(uint16_t port_id, uint32_t *mode);
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
> + *
> + * Set Forward Error Correction(FEC) mode.
> + *
> + * @param port_id
> + * The port identifier of the Ethernet device.
> + * @param mode
> + * the FEC mode.
See below.
> + * @return
> + * - (0) if successful.
> + * - (-EINVAL) if the FEC mode is not valid.
> + * - (-ENOTSUP) if underlying hardware OR driver doesn't support.
> + * - (-EIO) if device is removed.
> + * - (-ENODEV) if *port_id* invalid.
> + */
> +__rte_experimental
> +int rte_eth_fec_set(uint16_t port_id, uint32_t mode);
> +
> +/**
> * Get current status of the Ethernet link flow control for Ethernet device
> *
> * @param port_id
> diff --git a/lib/librte_ethdev/rte_ethdev_driver.h b/lib/librte_ethdev/rte_ethdev_driver.h
> index 23cc1e0..625b8c5 100644
> --- a/lib/librte_ethdev/rte_ethdev_driver.h
> +++ b/lib/librte_ethdev/rte_ethdev_driver.h
> @@ -575,6 +575,81 @@ typedef int (*eth_tx_hairpin_queue_setup_t)
> const struct rte_eth_hairpin_conf *hairpin_conf);
>
> /**
> + * @internal
> + * Get Forward Error Correction(FEC) capability.
> + *
> + * @param dev
> + * ethdev handle of port.
> + * @param num
> + * the num is in/out with a number of elements in an array.
I'm sorry, I should do it before my previous suggestion, but:
Looking at rte_eth_xstats_get_names() and trying to be
consistent I'd like to suggest to put the argument after
speed_fec_capa and make it input only with a number of
array elements.
Positive return values should be used to provide number of
filled in array elements. If the returned value is greater
than 'num', just provided elements are filled in, but
it is indication as well, that num is too small.
> + * @param speed_fec_capa
> + * speed_fec_capa is out only with per-speed capabilities.
> + *
> + * @return
> + * Negative errno value on error, 0 on success.
> + *
> + * @retval 0
> + * Success, get FEC success.
See above.
> + * @retval -ENOTSUP
> + * operation is not supported.
Should start from upper case letter
> + * @retval -EIO
> + * device is removed.
Should start from upper case letter
> + * @retval -ENODEV
> + * Device is gone.
What's the difference between "device is remove" and
"Device is gone"
> + * @retval -EINVAL
> + * *num* or *speed_fec_capa* invalid.
> + */
> +typedef int (*eth_fec_get_capability_t)(struct rte_eth_dev *dev, uint32_t *num,
> + struct rte_eth_fec_capa *speed_fec_capa);
> +
> +/**
> + * @internal
> + * Get Forward Error Correction(FEC) mode.
> + *
> + * @param dev
> + * ethdev handle of port.
> + * @param mode
> + * returns the FEC mode from the device.
> + *
> + * @return
> + * Negative errno value on error, 0 on success.
> + *
> + * @retval 0
> + * Success, get FEC success.
> + * @retval -ENOTSUP
> + * operation is not supported.
Should start from upper case letter
> + * @retval -EIO
> + * device is removed.
Should start from upper case letter
> + * @retval -ENODEV
> + * Device is gone.
> + */
> +typedef int (*eth_fec_get_t)(struct rte_eth_dev *dev,
> + uint32_t *mode);
> +
> +/**
> + * @internal
> + * Set Forward Error Correction(FEC) mode.
Remove extra spaces before "Set"
> + *
> + * @param dev
> + * ethdev handle of port.
> + * @param mode
> + * the FEC mode.
The description is insufficient and misleading.
It should be fec_capa and described as:
Bitmask of allowed FEC modes. If must be only one
if AUTO is disabled. If AUTO is enabled, other
bits specify FEC modes which may be negotiated.
> + *
> + * @return
> + * Negative errno value on error, 0 on success.
> + *
> + * @retval 0
> + * Success, set FEC success.
> + * @retval -ENOTSUP
> + * operation is not supported.
Should start from upper case letter
What about -EINVAL in the case of unsupported FEC mode
requested? It is listed above in API function but
missing here.
> + * @retval -EIO
> + * device is removed.
Should start from upper case letter
> + * @retval -ENODEV
> + * Device is gone.
What's the difference between "device is remove" and
"Device is gone"
> + */
> +typedef int (*eth_fec_set_t)(struct rte_eth_dev *dev, uint32_t mode);
> +
> +/**
> * @internal A structure containing the functions exported by an Ethernet driver.
> */
> struct eth_dev_ops {
> @@ -713,6 +788,13 @@ struct eth_dev_ops {
> /**< Set up device RX hairpin queue. */
> eth_tx_hairpin_queue_setup_t tx_hairpin_queue_setup;
> /**< Set up device TX hairpin queue. */
> +
> + eth_fec_get_capability_t fec_get_capability;
> + /**< Get Forward Error Correction(FEC) capability; */
It should be a dot (.) at the end, not semicolon (;).
> + eth_fec_get_t fec_get;
> + /**< Get Forward Error Correction(FEC) mode; */
same
> + eth_fec_set_t fec_set;
> + /**< Set Forward Error Correction(FEC) mode; */
same
> };
>
> /**
> diff --git a/lib/librte_ethdev/rte_ethdev_version.map b/lib/librte_ethdev/rte_ethdev_version.map
> index c95ef51..b9ace3a 100644
> --- a/lib/librte_ethdev/rte_ethdev_version.map
> +++ b/lib/librte_ethdev/rte_ethdev_version.map
> @@ -229,6 +229,9 @@ EXPERIMENTAL {
> # added in 20.11
> rte_eth_link_speed_to_str;
> rte_eth_link_to_str;
> + rte_eth_fec_get_capability;
> + rte_eth_fec_get;
> + rte_eth_fec_set;
> };
>
> INTERNAL {
>
HI，Andrew，
I fix it in V13 according to your advice.
Thanks for your patient review. Please check it out again.
By the way, there is always a coding check warning, like this:
WARNING:PREFER_FALLTHROUGH: Prefer 'fallthrough;' over fallthrough comment
#264: FILE: drivers/net/hns3/hns3_ethdev.c:5601:
+ /* fallthrough */
total: 0 errors, 1 warnings, 0 checks, 439 lines checked.
I have tried some ways, but it does not help.
Could you giver some advice?
thanks.
在 2020/9/24 22:46, Andrew Rybchenko 写道:
> On 9/24/20 4:05 PM, 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.
>
> Almost good now. See my notes below.
> Many thanks for hard work and patience.
>
>>
>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>> Reviewed-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
>> Reviewed-by: Chengwen Feng <fengchengwen@huawei.com>
>> Reviewed-by: Chengchang Tang <tangchengchang@huawei.com>
>> Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
>> Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
>
> I think that tags for Ajit and Konstantin should be
> dropped, since API changes significantly after their review.
>
>> ---
>> v10->v11:
>> allow to report capabilities per link speed.
>> specify what should be reported if link is down
>> when get FEC.
>> change mode to capa bitmask.
>>
>> ---
>> v9->v10:
>> add macro RTE_ETH_FEC_MODE_CAPA_MASK(x) to indicate
>> different FEC mode capa.
>>
>> ---
>> v8->v9:
>> added reviewed-by and acked-by.
>>
>> ---
>> v7->v8:
>> put AUTO just after NOFEC in rte_fec_mode definition.
>>
>> ---
>> v6->v7:
>> deleted RTE_ETH_FEC_NUM to prevent ABI breakage.
>> add new macro to indicate translation from fec mode
>> to capa.
>>
>> ---
>> v5->v6:
>> modified release notes.
>> deleted check duplicated for FEC API
>> fixed code styles according to DPDK coding style.
>> added _eth prefix.
>>
>> ---
>> 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.
>>
>> ---
>> lib/librte_ethdev/rte_ethdev.c | 37 +++++++++++++
>> lib/librte_ethdev/rte_ethdev.h | 91 ++++++++++++++++++++++++++++++++
>> lib/librte_ethdev/rte_ethdev_driver.h | 82 ++++++++++++++++++++++++++++
>> lib/librte_ethdev/rte_ethdev_version.map | 3 ++
>> 4 files changed, 213 insertions(+)
>>
>> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
>> index dfe5c1b..b614bfc 100644
>> --- a/lib/librte_ethdev/rte_ethdev.c
>> +++ b/lib/librte_ethdev/rte_ethdev.c
>> @@ -3679,6 +3679,43 @@ rte_eth_led_off(uint16_t port_id)
>> return eth_err(port_id, (*dev->dev_ops->dev_led_off)(dev));
>> }
>>
>> +int
>> +rte_eth_fec_get_capability(uint16_t port_id, uint32_t *num,
>> + struct rte_eth_fec_capa *speed_fec_capa)
>> +{
>> + struct rte_eth_dev *dev;
>> +
>> + if (num == NULL || speed_fec_capa == NULL)
>> + return -EINVAL;
>
> I think it is OK to have speed_fec_cap==NULL if *num is 0.
> I.e. a request to get number of required array entries.
>
>> +
>> + RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
>> + dev = &rte_eth_devices[port_id];
>> + RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->fec_get_capability, -ENOTSUP);
>> + return eth_err(port_id, (*dev->dev_ops->fec_get_capability)(dev, num,
>> + speed_fec_capa));
>> +}
>> +
>> +int
>> +rte_eth_fec_get(uint16_t port_id, uint32_t *mode)
>> +{
>> + struct rte_eth_dev *dev;
>
> I think it would be good to check that mode is not NULL here.
>
>> +
>> + RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
>> + dev = &rte_eth_devices[port_id];
>> + RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->fec_get, -ENOTSUP);
>> + return eth_err(port_id, (*dev->dev_ops->fec_get)(dev, mode));
>> +}
>> +
>> +int
>> +rte_eth_fec_set(uint16_t port_id, uint32_t mode)
>> +{
>> + struct rte_eth_dev *dev;
>> +
>> + dev = &rte_eth_devices[port_id];
>> + RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->fec_set, -ENOTSUP);
>> + return eth_err(port_id, (*dev->dev_ops->fec_set)(dev, mode));
>> +}
>> +
>> /*
>> * Returns index into MAC address array of addr. Use 00:00:00:00:00:00 to find
>> * an empty spot.
>> diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
>> index 645a186..104181d 100644
>> --- a/lib/librte_ethdev/rte_ethdev.h
>> +++ b/lib/librte_ethdev/rte_ethdev.h
>> @@ -1544,6 +1544,29 @@ struct rte_eth_dcb_info {
>> struct rte_eth_dcb_tc_queue_mapping tc_queue;
>> };
>>
>> +/**
>> + * This enum indicates the possible (forward error correction)FEC modes
>
> (forward error correction)FEC -> Forward Error Correction (FEC)
>
>> + * of an ethdev port.
>> + */
>> +enum rte_eth_fec_mode {
>> + RTE_ETH_FEC_NOFEC = 0, /**< FEC is off */
>> + RTE_ETH_FEC_AUTO, /**< FEC autonegotiation modes */
>> + RTE_ETH_FEC_BASER, /**< FEC using common algorithm */
>> + RTE_ETH_FEC_RS, /**< FEC using RS algorithm */
>> +};
>> +
>> +/* Translate from FEC mode to FEC capa */
>> +#define RTE_ETH_FEC_MODE_TO_CAPA(x) (1U << (x))
>> +
>> +/* This macro indicates FEC capa mask*/
>
> Add missing space before */
>
>> +#define RTE_ETH_FEC_MODE_CAPA_MASK(x) (1U << (RTE_ETH_FEC_ ## x))
>> +
>> +/* A structure used to get capabilities per link speed */
>> +struct rte_eth_fec_capa {
>> + uint32_t speed; /**< Link speed (see ETH_SPEED_NUM_*) */
>> + uint32_t capa; /**< FEC capabilities bitmask (see RTE_FEC_CAPA_*) */
>> +};
>> +
>> #define RTE_ETH_ALL RTE_MAX_ETHPORTS
>>
>> /* Macros to check for valid port */
>> @@ -3397,6 +3420,74 @@ 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 num
>> + * the num is in/out with a number of elements in an array.
>
> Please, see below my notes on callback description.
>
>> + * @param speed_fec_capa
>> + * speed_fec_capa is out only with per-speed capabilities.
>> + *
>> + * @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.
>> + * - (-EINVAL) if *num* or *speed_fec_capa* invalid
>> + */
>> +__rte_experimental
>> +int rte_eth_fec_get_capability(uint16_t port_id, uint32_t *num,
>> + struct rte_eth_fec_capa *speed_fec_capa);
>> +
>> +/**
>> + * @warning
>> + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
>> + *
>> + * Get current Forward Error Correction(FEC) mode.
>> + * If link is down and AUTO is enabled, AUTO is returned, otherwise,
>> + * configured FEC mode is returned.
>> + * If link is up, current FEC mode is returned.
>> + *
>> + * @param port_id
>> + * The port identifier of the Ethernet device.
>> + * @param mode
>> + * returns the FEC mode from the device.
>> + * @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(uint16_t port_id, uint32_t *mode);
>> +
>> +/**
>> + * @warning
>> + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
>> + *
>> + * Set Forward Error Correction(FEC) mode.
>> + *
>> + * @param port_id
>> + * The port identifier of the Ethernet device.
>> + * @param mode
>> + * the FEC mode.
>
> See below.
>
>> + * @return
>> + * - (0) if successful.
>> + * - (-EINVAL) if the FEC mode is not valid.
>> + * - (-ENOTSUP) if underlying hardware OR driver doesn't support.
>> + * - (-EIO) if device is removed.
>> + * - (-ENODEV) if *port_id* invalid.
>> + */
>> +__rte_experimental
>> +int rte_eth_fec_set(uint16_t port_id, uint32_t mode);
>> +
>> +/**
>> * Get current status of the Ethernet link flow control for Ethernet device
>> *
>> * @param port_id
>> diff --git a/lib/librte_ethdev/rte_ethdev_driver.h b/lib/librte_ethdev/rte_ethdev_driver.h
>> index 23cc1e0..625b8c5 100644
>> --- a/lib/librte_ethdev/rte_ethdev_driver.h
>> +++ b/lib/librte_ethdev/rte_ethdev_driver.h
>> @@ -575,6 +575,81 @@ typedef int (*eth_tx_hairpin_queue_setup_t)
>> const struct rte_eth_hairpin_conf *hairpin_conf);
>>
>> /**
>> + * @internal
>> + * Get Forward Error Correction(FEC) capability.
>> + *
>> + * @param dev
>> + * ethdev handle of port.
>> + * @param num
>> + * the num is in/out with a number of elements in an array.
>
> I'm sorry, I should do it before my previous suggestion, but:
> Looking at rte_eth_xstats_get_names() and trying to be
> consistent I'd like to suggest to put the argument after
> speed_fec_capa and make it input only with a number of
> array elements.
> Positive return values should be used to provide number of
> filled in array elements. If the returned value is greater
> than 'num', just provided elements are filled in, but
> it is indication as well, that num is too small.
>
>> + * @param speed_fec_capa
>> + * speed_fec_capa is out only with per-speed capabilities.
>> + *
>> + * @return
>> + * Negative errno value on error, 0 on success.
>> + *
>> + * @retval 0
>> + * Success, get FEC success.
>
> See above.
>
>> + * @retval -ENOTSUP
>> + * operation is not supported.
>
> Should start from upper case letter
>
>> + * @retval -EIO
>> + * device is removed.
>
> Should start from upper case letter
>
>> + * @retval -ENODEV
>> + * Device is gone.
>
> What's the difference between "device is remove" and
> "Device is gone"
>
>> + * @retval -EINVAL
>> + * *num* or *speed_fec_capa* invalid.
>> + */
>> +typedef int (*eth_fec_get_capability_t)(struct rte_eth_dev *dev, uint32_t *num,
>> + struct rte_eth_fec_capa *speed_fec_capa);
>> +
>> +/**
>> + * @internal
>> + * Get Forward Error Correction(FEC) mode.
>> + *
>> + * @param dev
>> + * ethdev handle of port.
>> + * @param mode
>> + * returns the FEC mode from the device.
>> + *
>> + * @return
>> + * Negative errno value on error, 0 on success.
>> + *
>> + * @retval 0
>> + * Success, get FEC success.
>> + * @retval -ENOTSUP
>> + * operation is not supported.
>
> Should start from upper case letter
>
>> + * @retval -EIO
>> + * device is removed.
>
> Should start from upper case letter
>
>> + * @retval -ENODEV
>> + * Device is gone.
>> + */
>> +typedef int (*eth_fec_get_t)(struct rte_eth_dev *dev,
>> + uint32_t *mode);
>> +
>> +/**
>> + * @internal
>> + * Set Forward Error Correction(FEC) mode.
>
> Remove extra spaces before "Set"
>
>> + *
>> + * @param dev
>> + * ethdev handle of port.
>> + * @param mode
>> + * the FEC mode.
>
> The description is insufficient and misleading.
> It should be fec_capa and described as:
> Bitmask of allowed FEC modes. If must be only one
> if AUTO is disabled. If AUTO is enabled, other
> bits specify FEC modes which may be negotiated.
>
>> + *
>> + * @return
>> + * Negative errno value on error, 0 on success.
>> + *
>> + * @retval 0
>> + * Success, set FEC success.
>> + * @retval -ENOTSUP
>> + * operation is not supported.
>
> Should start from upper case letter
>
> What about -EINVAL in the case of unsupported FEC mode
> requested? It is listed above in API function but
> missing here.
>
>> + * @retval -EIO
>> + * device is removed.
>
> Should start from upper case letter
>
>> + * @retval -ENODEV
>> + * Device is gone.
>
> What's the difference between "device is remove" and
> "Device is gone"
>
>> + */
>> +typedef int (*eth_fec_set_t)(struct rte_eth_dev *dev, uint32_t mode);
>> +
>> +/**
>> * @internal A structure containing the functions exported by an Ethernet driver.
>> */
>> struct eth_dev_ops {
>> @@ -713,6 +788,13 @@ struct eth_dev_ops {
>> /**< Set up device RX hairpin queue. */
>> eth_tx_hairpin_queue_setup_t tx_hairpin_queue_setup;
>> /**< Set up device TX hairpin queue. */
>> +
>> + eth_fec_get_capability_t fec_get_capability;
>> + /**< Get Forward Error Correction(FEC) capability; */
>
> It should be a dot (.) at the end, not semicolon (;).
>
>> + eth_fec_get_t fec_get;
>> + /**< Get Forward Error Correction(FEC) mode; */
>
> same
>
>> + eth_fec_set_t fec_set;
>> + /**< Set Forward Error Correction(FEC) mode; */
>
> same
>
>> };
>>
>> /**
>> diff --git a/lib/librte_ethdev/rte_ethdev_version.map b/lib/librte_ethdev/rte_ethdev_version.map
>> index c95ef51..b9ace3a 100644
>> --- a/lib/librte_ethdev/rte_ethdev_version.map
>> +++ b/lib/librte_ethdev/rte_ethdev_version.map
>> @@ -229,6 +229,9 @@ EXPERIMENTAL {
>> # added in 20.11
>> rte_eth_link_speed_to_str;
>> rte_eth_link_to_str;
>> + rte_eth_fec_get_capability;
>> + rte_eth_fec_get;
>> + rte_eth_fec_set;
>> };
>>
>> INTERNAL {
>>
>
> .
>
On Fri, Sep 25, 2020 at 1:47 AM Min Hu (Connor) <humin29@huawei.com> wrote:
>
> HI，Andrew，
> I fix it in V13 according to your advice.
> Thanks for your patient review. Please check it out again.
>
> By the way, there is always a coding check warning, like this:
>
> WARNING:PREFER_FALLTHROUGH: Prefer 'fallthrough;' over fallthrough comment
> #264: FILE: drivers/net/hns3/hns3_ethdev.c:5601:
> + /* fallthrough */
Try /* FALLTHROUGH */
>
> total: 0 errors, 1 warnings, 0 checks, 439 lines checked.
>
> I have tried some ways, but it does not help.
> Could you giver some advice?
> thanks.
>
>
> 在 2020/9/24 22:46, Andrew Rybchenko 写道:
> > On 9/24/20 4:05 PM, 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.
> >
> > Almost good now. See my notes below.
> > Many thanks for hard work and patience.
> >
> >>
> >> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> >> Reviewed-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
> >> Reviewed-by: Chengwen Feng <fengchengwen@huawei.com>
> >> Reviewed-by: Chengchang Tang <tangchengchang@huawei.com>
> >> Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> >> Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> >
> > I think that tags for Ajit and Konstantin should be
> > dropped, since API changes significantly after their review.
> >
> >> ---
> >> v10->v11:
> >> allow to report capabilities per link speed.
> >> specify what should be reported if link is down
> >> when get FEC.
> >> change mode to capa bitmask.
> >>
> >> ---
> >> v9->v10:
> >> add macro RTE_ETH_FEC_MODE_CAPA_MASK(x) to indicate
> >> different FEC mode capa.
> >>
> >> ---
> >> v8->v9:
> >> added reviewed-by and acked-by.
> >>
> >> ---
> >> v7->v8:
> >> put AUTO just after NOFEC in rte_fec_mode definition.
> >>
> >> ---
> >> v6->v7:
> >> deleted RTE_ETH_FEC_NUM to prevent ABI breakage.
> >> add new macro to indicate translation from fec mode
> >> to capa.
> >>
> >> ---
> >> v5->v6:
> >> modified release notes.
> >> deleted check duplicated for FEC API
> >> fixed code styles according to DPDK coding style.
> >> added _eth prefix.
> >>
> >> ---
> >> 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.
> >>
> >> ---
> >> lib/librte_ethdev/rte_ethdev.c | 37 +++++++++++++
> >> lib/librte_ethdev/rte_ethdev.h | 91 ++++++++++++++++++++++++++++++++
> >> lib/librte_ethdev/rte_ethdev_driver.h | 82 ++++++++++++++++++++++++++++
> >> lib/librte_ethdev/rte_ethdev_version.map | 3 ++
> >> 4 files changed, 213 insertions(+)
> >>
> >> diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
> >> index dfe5c1b..b614bfc 100644
> >> --- a/lib/librte_ethdev/rte_ethdev.c
> >> +++ b/lib/librte_ethdev/rte_ethdev.c
> >> @@ -3679,6 +3679,43 @@ rte_eth_led_off(uint16_t port_id)
> >> return eth_err(port_id, (*dev->dev_ops->dev_led_off)(dev));
> >> }
> >>
> >> +int
> >> +rte_eth_fec_get_capability(uint16_t port_id, uint32_t *num,
> >> + struct rte_eth_fec_capa *speed_fec_capa)
> >> +{
> >> + struct rte_eth_dev *dev;
> >> +
> >> + if (num == NULL || speed_fec_capa == NULL)
> >> + return -EINVAL;
> >
> > I think it is OK to have speed_fec_cap==NULL if *num is 0.
> > I.e. a request to get number of required array entries.
> >
> >> +
> >> + RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> >> + dev = &rte_eth_devices[port_id];
> >> + RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->fec_get_capability, -ENOTSUP);
> >> + return eth_err(port_id, (*dev->dev_ops->fec_get_capability)(dev, num,
> >> + speed_fec_capa));
> >> +}
> >> +
> >> +int
> >> +rte_eth_fec_get(uint16_t port_id, uint32_t *mode)
> >> +{
> >> + struct rte_eth_dev *dev;
> >
> > I think it would be good to check that mode is not NULL here.
> >
> >> +
> >> + RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
> >> + dev = &rte_eth_devices[port_id];
> >> + RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->fec_get, -ENOTSUP);
> >> + return eth_err(port_id, (*dev->dev_ops->fec_get)(dev, mode));
> >> +}
> >> +
> >> +int
> >> +rte_eth_fec_set(uint16_t port_id, uint32_t mode)
> >> +{
> >> + struct rte_eth_dev *dev;
> >> +
> >> + dev = &rte_eth_devices[port_id];
> >> + RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->fec_set, -ENOTSUP);
> >> + return eth_err(port_id, (*dev->dev_ops->fec_set)(dev, mode));
> >> +}
> >> +
> >> /*
> >> * Returns index into MAC address array of addr. Use 00:00:00:00:00:00 to find
> >> * an empty spot.
> >> diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
> >> index 645a186..104181d 100644
> >> --- a/lib/librte_ethdev/rte_ethdev.h
> >> +++ b/lib/librte_ethdev/rte_ethdev.h
> >> @@ -1544,6 +1544,29 @@ struct rte_eth_dcb_info {
> >> struct rte_eth_dcb_tc_queue_mapping tc_queue;
> >> };
> >>
> >> +/**
> >> + * This enum indicates the possible (forward error correction)FEC modes
> >
> > (forward error correction)FEC -> Forward Error Correction (FEC)
> >
> >> + * of an ethdev port.
> >> + */
> >> +enum rte_eth_fec_mode {
> >> + RTE_ETH_FEC_NOFEC = 0, /**< FEC is off */
> >> + RTE_ETH_FEC_AUTO, /**< FEC autonegotiation modes */
> >> + RTE_ETH_FEC_BASER, /**< FEC using common algorithm */
> >> + RTE_ETH_FEC_RS, /**< FEC using RS algorithm */
> >> +};
> >> +
> >> +/* Translate from FEC mode to FEC capa */
> >> +#define RTE_ETH_FEC_MODE_TO_CAPA(x) (1U << (x))
> >> +
> >> +/* This macro indicates FEC capa mask*/
> >
> > Add missing space before */
> >
> >> +#define RTE_ETH_FEC_MODE_CAPA_MASK(x) (1U << (RTE_ETH_FEC_ ## x))
> >> +
> >> +/* A structure used to get capabilities per link speed */
> >> +struct rte_eth_fec_capa {
> >> + uint32_t speed; /**< Link speed (see ETH_SPEED_NUM_*) */
> >> + uint32_t capa; /**< FEC capabilities bitmask (see RTE_FEC_CAPA_*) */
> >> +};
> >> +
> >> #define RTE_ETH_ALL RTE_MAX_ETHPORTS
> >>
> >> /* Macros to check for valid port */
> >> @@ -3397,6 +3420,74 @@ 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 num
> >> + * the num is in/out with a number of elements in an array.
> >
> > Please, see below my notes on callback description.
> >
> >> + * @param speed_fec_capa
> >> + * speed_fec_capa is out only with per-speed capabilities.
> >> + *
> >> + * @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.
> >> + * - (-EINVAL) if *num* or *speed_fec_capa* invalid
> >> + */
> >> +__rte_experimental
> >> +int rte_eth_fec_get_capability(uint16_t port_id, uint32_t *num,
> >> + struct rte_eth_fec_capa *speed_fec_capa);
> >> +
> >> +/**
> >> + * @warning
> >> + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
> >> + *
> >> + * Get current Forward Error Correction(FEC) mode.
> >> + * If link is down and AUTO is enabled, AUTO is returned, otherwise,
> >> + * configured FEC mode is returned.
> >> + * If link is up, current FEC mode is returned.
> >> + *
> >> + * @param port_id
> >> + * The port identifier of the Ethernet device.
> >> + * @param mode
> >> + * returns the FEC mode from the device.
> >> + * @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(uint16_t port_id, uint32_t *mode);
> >> +
> >> +/**
> >> + * @warning
> >> + * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
> >> + *
> >> + * Set Forward Error Correction(FEC) mode.
> >> + *
> >> + * @param port_id
> >> + * The port identifier of the Ethernet device.
> >> + * @param mode
> >> + * the FEC mode.
> >
> > See below.
> >
> >> + * @return
> >> + * - (0) if successful.
> >> + * - (-EINVAL) if the FEC mode is not valid.
> >> + * - (-ENOTSUP) if underlying hardware OR driver doesn't support.
> >> + * - (-EIO) if device is removed.
> >> + * - (-ENODEV) if *port_id* invalid.
> >> + */
> >> +__rte_experimental
> >> +int rte_eth_fec_set(uint16_t port_id, uint32_t mode);
> >> +
> >> +/**
> >> * Get current status of the Ethernet link flow control for Ethernet device
> >> *
> >> * @param port_id
> >> diff --git a/lib/librte_ethdev/rte_ethdev_driver.h b/lib/librte_ethdev/rte_ethdev_driver.h
> >> index 23cc1e0..625b8c5 100644
> >> --- a/lib/librte_ethdev/rte_ethdev_driver.h
> >> +++ b/lib/librte_ethdev/rte_ethdev_driver.h
> >> @@ -575,6 +575,81 @@ typedef int (*eth_tx_hairpin_queue_setup_t)
> >> const struct rte_eth_hairpin_conf *hairpin_conf);
> >>
> >> /**
> >> + * @internal
> >> + * Get Forward Error Correction(FEC) capability.
> >> + *
> >> + * @param dev
> >> + * ethdev handle of port.
> >> + * @param num
> >> + * the num is in/out with a number of elements in an array.
> >
> > I'm sorry, I should do it before my previous suggestion, but:
> > Looking at rte_eth_xstats_get_names() and trying to be
> > consistent I'd like to suggest to put the argument after
> > speed_fec_capa and make it input only with a number of
> > array elements.
> > Positive return values should be used to provide number of
> > filled in array elements. If the returned value is greater
> > than 'num', just provided elements are filled in, but
> > it is indication as well, that num is too small.
> >
> >> + * @param speed_fec_capa
> >> + * speed_fec_capa is out only with per-speed capabilities.
> >> + *
> >> + * @return
> >> + * Negative errno value on error, 0 on success.
> >> + *
> >> + * @retval 0
> >> + * Success, get FEC success.
> >
> > See above.
> >
> >> + * @retval -ENOTSUP
> >> + * operation is not supported.
> >
> > Should start from upper case letter
> >
> >> + * @retval -EIO
> >> + * device is removed.
> >
> > Should start from upper case letter
> >
> >> + * @retval -ENODEV
> >> + * Device is gone.
> >
> > What's the difference between "device is remove" and
> > "Device is gone"
> >
> >> + * @retval -EINVAL
> >> + * *num* or *speed_fec_capa* invalid.
> >> + */
> >> +typedef int (*eth_fec_get_capability_t)(struct rte_eth_dev *dev, uint32_t *num,
> >> + struct rte_eth_fec_capa *speed_fec_capa);
> >> +
> >> +/**
> >> + * @internal
> >> + * Get Forward Error Correction(FEC) mode.
> >> + *
> >> + * @param dev
> >> + * ethdev handle of port.
> >> + * @param mode
> >> + * returns the FEC mode from the device.
> >> + *
> >> + * @return
> >> + * Negative errno value on error, 0 on success.
> >> + *
> >> + * @retval 0
> >> + * Success, get FEC success.
> >> + * @retval -ENOTSUP
> >> + * operation is not supported.
> >
> > Should start from upper case letter
> >
> >> + * @retval -EIO
> >> + * device is removed.
> >
> > Should start from upper case letter
> >
> >> + * @retval -ENODEV
> >> + * Device is gone.
> >> + */
> >> +typedef int (*eth_fec_get_t)(struct rte_eth_dev *dev,
> >> + uint32_t *mode);
> >> +
> >> +/**
> >> + * @internal
> >> + * Set Forward Error Correction(FEC) mode.
> >
> > Remove extra spaces before "Set"
> >
> >> + *
> >> + * @param dev
> >> + * ethdev handle of port.
> >> + * @param mode
> >> + * the FEC mode.
> >
> > The description is insufficient and misleading.
> > It should be fec_capa and described as:
> > Bitmask of allowed FEC modes. If must be only one
> > if AUTO is disabled. If AUTO is enabled, other
> > bits specify FEC modes which may be negotiated.
> >
> >> + *
> >> + * @return
> >> + * Negative errno value on error, 0 on success.
> >> + *
> >> + * @retval 0
> >> + * Success, set FEC success.
> >> + * @retval -ENOTSUP
> >> + * operation is not supported.
> >
> > Should start from upper case letter
> >
> > What about -EINVAL in the case of unsupported FEC mode
> > requested? It is listed above in API function but
> > missing here.
> >
> >> + * @retval -EIO
> >> + * device is removed.
> >
> > Should start from upper case letter
> >
> >> + * @retval -ENODEV
> >> + * Device is gone.
> >
> > What's the difference between "device is remove" and
> > "Device is gone"
> >
> >> + */
> >> +typedef int (*eth_fec_set_t)(struct rte_eth_dev *dev, uint32_t mode);
> >> +
> >> +/**
> >> * @internal A structure containing the functions exported by an Ethernet driver.
> >> */
> >> struct eth_dev_ops {
> >> @@ -713,6 +788,13 @@ struct eth_dev_ops {
> >> /**< Set up device RX hairpin queue. */
> >> eth_tx_hairpin_queue_setup_t tx_hairpin_queue_setup;
> >> /**< Set up device TX hairpin queue. */
> >> +
> >> + eth_fec_get_capability_t fec_get_capability;
> >> + /**< Get Forward Error Correction(FEC) capability; */
> >
> > It should be a dot (.) at the end, not semicolon (;).
> >
> >> + eth_fec_get_t fec_get;
> >> + /**< Get Forward Error Correction(FEC) mode; */
> >
> > same
> >
> >> + eth_fec_set_t fec_set;
> >> + /**< Set Forward Error Correction(FEC) mode; */
> >
> > same
> >
> >> };
> >>
> >> /**
> >> diff --git a/lib/librte_ethdev/rte_ethdev_version.map b/lib/librte_ethdev/rte_ethdev_version.map
> >> index c95ef51..b9ace3a 100644
> >> --- a/lib/librte_ethdev/rte_ethdev_version.map
> >> +++ b/lib/librte_ethdev/rte_ethdev_version.map
> >> @@ -229,6 +229,9 @@ EXPERIMENTAL {
> >> # added in 20.11
> >> rte_eth_link_speed_to_str;
> >> rte_eth_link_to_str;
> >> + rte_eth_fec_get_capability;
> >> + rte_eth_fec_get;
> >> + rte_eth_fec_set;
> >> };
> >>
> >> INTERNAL {
> >>
> >
> > .
> >
On Fri, 25 Sep 2020 08:36:06 -0700
Ajit Khaparde <ajit.khaparde@broadcom.com> wrote:
> >
> > WARNING:PREFER_FALLTHROUGH: Prefer 'fallthrough;' over fallthrough comment
> > #264: FILE: drivers/net/hns3/hns3_ethdev.c:5601:
> > + /* fallthrough */
> Try /* FALLTHROUGH */
That won't work either.
This is a new warning from upstream Linux kernel checkpatch.
The Linux kernel has converted code to use a new macro.
The warning should be silenced by the DPDK checkpatch script.
diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
index 78a408ef9823..923ff6f7d9ee 100755
--- a/devtools/checkpatches.sh
+++ b/devtools/checkpatches.sh
@@ -33,7 +33,7 @@ VOLATILE,PREFER_PACKED,PREFER_ALIGNED,PREFER_PRINTF,\
PREFER_KERNEL_TYPES,PREFER_FALLTHROUGH,BIT_MACRO,CONST_STRUCT,\
SPLIT_STRING,LONG_LINE_STRING,C99_COMMENT_TOLERANCE,\
LINE_SPACING,PARENTHESIS_ALIGNMENT,NETWORKING_BLOCK_COMMENT_STYLE,\
-NEW_TYPEDEFS,COMPARISON_TO_NULL"
+NEW_TYPEDEFS,COMPARISON_TO_NULL,PREFER_FALLTHROUGH"
options="$options $DPDK_CHECKPATCH_OPTIONS"
On 9/25/2020 5:12 PM, Stephen Hemminger wrote:
> On Fri, 25 Sep 2020 08:36:06 -0700
> Ajit Khaparde <ajit.khaparde@broadcom.com> wrote:
>
>>>
>>> WARNING:PREFER_FALLTHROUGH: Prefer 'fallthrough;' over fallthrough comment
>>> #264: FILE: drivers/net/hns3/hns3_ethdev.c:5601:
>>> + /* fallthrough */
>> Try /* FALLTHROUGH */
>
> That won't work either.
>
> This is a new warning from upstream Linux kernel checkpatch.
> The Linux kernel has converted code to use a new macro.
>
> The warning should be silenced by the DPDK checkpatch script.
>
> diff --git a/devtools/checkpatches.sh b/devtools/checkpatches.sh
> index 78a408ef9823..923ff6f7d9ee 100755
> --- a/devtools/checkpatches.sh
> +++ b/devtools/checkpatches.sh
> @@ -33,7 +33,7 @@ VOLATILE,PREFER_PACKED,PREFER_ALIGNED,PREFER_PRINTF,\
> PREFER_KERNEL_TYPES,PREFER_FALLTHROUGH,BIT_MACRO,CONST_STRUCT,\
> SPLIT_STRING,LONG_LINE_STRING,C99_COMMENT_TOLERANCE,\
> LINE_SPACING,PARENTHESIS_ALIGNMENT,NETWORKING_BLOCK_COMMENT_STYLE,\
> -NEW_TYPEDEFS,COMPARISON_TO_NULL"
> +NEW_TYPEDEFS,COMPARISON_TO_NULL,PREFER_FALLTHROUGH"
> options="$options $DPDK_CHECKPATCH_OPTIONS"
>
It should be already applied: https://patches.dpdk.org/patch/75173/
https://git.dpdk.org/dpdk/commit/?id=fcea4c8e29b6
@@ -3679,6 +3679,43 @@ rte_eth_led_off(uint16_t port_id)
return eth_err(port_id, (*dev->dev_ops->dev_led_off)(dev));
}
+int
+rte_eth_fec_get_capability(uint16_t port_id, uint32_t *num,
+ struct rte_eth_fec_capa *speed_fec_capa)
+{
+ struct rte_eth_dev *dev;
+
+ if (num == NULL || speed_fec_capa == NULL)
+ return -EINVAL;
+
+ RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+ dev = &rte_eth_devices[port_id];
+ RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->fec_get_capability, -ENOTSUP);
+ return eth_err(port_id, (*dev->dev_ops->fec_get_capability)(dev, num,
+ speed_fec_capa));
+}
+
+int
+rte_eth_fec_get(uint16_t port_id, uint32_t *mode)
+{
+ struct rte_eth_dev *dev;
+
+ RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+ dev = &rte_eth_devices[port_id];
+ RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->fec_get, -ENOTSUP);
+ return eth_err(port_id, (*dev->dev_ops->fec_get)(dev, mode));
+}
+
+int
+rte_eth_fec_set(uint16_t port_id, uint32_t mode)
+{
+ struct rte_eth_dev *dev;
+
+ dev = &rte_eth_devices[port_id];
+ RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->fec_set, -ENOTSUP);
+ return eth_err(port_id, (*dev->dev_ops->fec_set)(dev, mode));
+}
+
/*
* Returns index into MAC address array of addr. Use 00:00:00:00:00:00 to find
* an empty spot.
@@ -1544,6 +1544,29 @@ 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_eth_fec_mode {
+ RTE_ETH_FEC_NOFEC = 0, /**< FEC is off */
+ RTE_ETH_FEC_AUTO, /**< FEC autonegotiation modes */
+ RTE_ETH_FEC_BASER, /**< FEC using common algorithm */
+ RTE_ETH_FEC_RS, /**< FEC using RS algorithm */
+};
+
+/* Translate from FEC mode to FEC capa */
+#define RTE_ETH_FEC_MODE_TO_CAPA(x) (1U << (x))
+
+/* This macro indicates FEC capa mask*/
+#define RTE_ETH_FEC_MODE_CAPA_MASK(x) (1U << (RTE_ETH_FEC_ ## x))
+
+/* A structure used to get capabilities per link speed */
+struct rte_eth_fec_capa {
+ uint32_t speed; /**< Link speed (see ETH_SPEED_NUM_*) */
+ uint32_t capa; /**< FEC capabilities bitmask (see RTE_FEC_CAPA_*) */
+};
+
#define RTE_ETH_ALL RTE_MAX_ETHPORTS
/* Macros to check for valid port */
@@ -3397,6 +3420,74 @@ 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 num
+ * the num is in/out with a number of elements in an array.
+ * @param speed_fec_capa
+ * speed_fec_capa is out only with per-speed capabilities.
+ *
+ * @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.
+ * - (-EINVAL) if *num* or *speed_fec_capa* invalid
+ */
+__rte_experimental
+int rte_eth_fec_get_capability(uint16_t port_id, uint32_t *num,
+ struct rte_eth_fec_capa *speed_fec_capa);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
+ *
+ * Get current Forward Error Correction(FEC) mode.
+ * If link is down and AUTO is enabled, AUTO is returned, otherwise,
+ * configured FEC mode is returned.
+ * If link is up, current FEC mode is returned.
+ *
+ * @param port_id
+ * The port identifier of the Ethernet device.
+ * @param mode
+ * returns the FEC mode from the device.
+ * @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(uint16_t port_id, uint32_t *mode);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change, or be removed, without prior notice
+ *
+ * Set Forward Error Correction(FEC) mode.
+ *
+ * @param port_id
+ * The port identifier of the Ethernet device.
+ * @param mode
+ * the FEC mode.
+ * @return
+ * - (0) if successful.
+ * - (-EINVAL) if the FEC mode is not valid.
+ * - (-ENOTSUP) if underlying hardware OR driver doesn't support.
+ * - (-EIO) if device is removed.
+ * - (-ENODEV) if *port_id* invalid.
+ */
+__rte_experimental
+int rte_eth_fec_set(uint16_t port_id, uint32_t mode);
+
+/**
* Get current status of the Ethernet link flow control for Ethernet device
*
* @param port_id
@@ -575,6 +575,81 @@ typedef int (*eth_tx_hairpin_queue_setup_t)
const struct rte_eth_hairpin_conf *hairpin_conf);
/**
+ * @internal
+ * Get Forward Error Correction(FEC) capability.
+ *
+ * @param dev
+ * ethdev handle of port.
+ * @param num
+ * the num is in/out with a number of elements in an array.
+ * @param speed_fec_capa
+ * speed_fec_capa is out only with per-speed capabilities.
+ *
+ * @return
+ * Negative errno value on error, 0 on success.
+ *
+ * @retval 0
+ * Success, get FEC success.
+ * @retval -ENOTSUP
+ * operation is not supported.
+ * @retval -EIO
+ * device is removed.
+ * @retval -ENODEV
+ * Device is gone.
+ * @retval -EINVAL
+ * *num* or *speed_fec_capa* invalid.
+ */
+typedef int (*eth_fec_get_capability_t)(struct rte_eth_dev *dev, uint32_t *num,
+ struct rte_eth_fec_capa *speed_fec_capa);
+
+/**
+ * @internal
+ * Get Forward Error Correction(FEC) mode.
+ *
+ * @param dev
+ * ethdev handle of port.
+ * @param mode
+ * returns the FEC mode from the device.
+ *
+ * @return
+ * Negative errno value on error, 0 on success.
+ *
+ * @retval 0
+ * Success, get FEC success.
+ * @retval -ENOTSUP
+ * operation is not supported.
+ * @retval -EIO
+ * device is removed.
+ * @retval -ENODEV
+ * Device is gone.
+ */
+typedef int (*eth_fec_get_t)(struct rte_eth_dev *dev,
+ uint32_t *mode);
+
+/**
+ * @internal
+ * Set Forward Error Correction(FEC) mode.
+ *
+ * @param dev
+ * ethdev handle of port.
+ * @param mode
+ * the FEC mode.
+ *
+ * @return
+ * Negative errno value on error, 0 on success.
+ *
+ * @retval 0
+ * Success, set FEC success.
+ * @retval -ENOTSUP
+ * operation is not supported.
+ * @retval -EIO
+ * device is removed.
+ * @retval -ENODEV
+ * Device is gone.
+ */
+typedef int (*eth_fec_set_t)(struct rte_eth_dev *dev, uint32_t mode);
+
+/**
* @internal A structure containing the functions exported by an Ethernet driver.
*/
struct eth_dev_ops {
@@ -713,6 +788,13 @@ struct eth_dev_ops {
/**< Set up device RX hairpin queue. */
eth_tx_hairpin_queue_setup_t tx_hairpin_queue_setup;
/**< Set up device TX hairpin queue. */
+
+ eth_fec_get_capability_t fec_get_capability;
+ /**< Get Forward Error Correction(FEC) capability; */
+ eth_fec_get_t fec_get;
+ /**< Get Forward Error Correction(FEC) mode; */
+ eth_fec_set_t fec_set;
+ /**< Set Forward Error Correction(FEC) mode; */
};
/**
@@ -229,6 +229,9 @@ EXPERIMENTAL {
# added in 20.11
rte_eth_link_speed_to_str;
rte_eth_link_to_str;
+ rte_eth_fec_get_capability;
+ rte_eth_fec_get;
+ rte_eth_fec_set;
};
INTERNAL {