[RFC PATCH 0/3] improve FEC API usage

[RFC PATCH 0/3] improve FEC API usage

[Denis Pryazhennikov]

The documentation for the FEC API is currently incomplete and contains
inaccuracies in its descriptions of function parameters. 
Specifically, the semantics of the fec_capa parameter in rte_eth_fec_set()
is not well-defined. It does not provide information on what should
be done if only AUTO bit is set or one of the specified FEC modes is 
not supported. Additionally, the fec_capa parameter in rte_eth_fec_get()
implies that more than one FEC mode can be obtained, but it is 
wrong. Furthermore, the behaviour is undefined in 
rte_eth_fec_set() when the fec_capa parameter is zero.

To address these issues, a patch series has been created that updates
the FEC API documentation, renames one of the parameters to improve 
its clarity and adds a check for zero fec_capability.

Denis Pryazhennikov (3):
  ethdev: update documentation for API to set FEC
  ethdev: check that at least one FEC mode is specified
  ethdev: rename parameter in API to get FEC

 lib/ethdev/rte_ethdev.c | 13 +++++++++----
 lib/ethdev/rte_ethdev.h | 25 ++++++++++++++-----------
 2 files changed, 23 insertions(+), 15 deletions(-)

-- 
2.37.0 (Apple Git-136)

[RFC PATCH 1/3] ethdev: update documentation for API to set FEC

[Denis Pryazhennikov]

The documentation for the rte_eth_fec_set() is updated
to provide more detailed information about how FEC modes are
handled. It also includes a description of the case when only
the AUTO bit is set.

Signed-off-by: Denis Pryazhennikov <denis.pryazhennikov@arknetworks.am>
Acked-by: Ivan Malov <ivan.malov@arknetworks.am>
Acked-by: Viacheslav Galaktionov <viacheslav.galaktionov@arknetworks.am>
---
 lib/ethdev/rte_ethdev.h | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index 99fe9e238b20..0f10ac944061 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -4227,13 +4227,19 @@ int rte_eth_fec_get(uint16_t port_id, uint32_t *fec_capa);
  * @param port_id
  *   The port identifier of the Ethernet device.
  * @param fec_capa
- *   A bitmask of allowed FEC modes. If AUTO bit is set, other
- *   bits specify FEC modes which may be negotiated. If AUTO
- *   bit is clear, specify FEC modes to be used (only one valid
- *   mode per speed may be set).
+ *   A bitmask of allowed FEC modes.
+ *   If only the AUTO bit is set, the decision on which FEC
+ *   mode to use will be made by HW/FW or driver.
+ *   If the AUTO bit is set, other bits specify FEC modes
+ *   which may be negotiated. It means that only specified
+ *   FEC modes can be set.
+ *   If AUTO bit is clear, specify FEC mode to be used
+ *   (only one valid mode per speed may be set).
+ *   NOFEC will be used if specified FEC modes are not
+ *   supported.
  * @return
  *   - (0) if successful.
- *   - (-EINVAL) if the FEC mode is not valid.
+ *   - (-EINVAL) if *fec_capa* is not valid.
  *   - (-ENOTSUP) if underlying hardware OR driver doesn't support.
  *   - (-EIO) if device is removed.
  *   - (-ENODEV)  if *port_id* invalid.
-- 
2.37.0 (Apple Git-136)

[RFC PATCH 2/3] ethdev: check that at least one FEC mode is specified

[Denis Pryazhennikov]

The behaviour is undefined in the rte_eth_fec_set() function
when the fec_capa parameter is equal to zero.
Add a check to handle this case.

Signed-off-by: Denis Pryazhennikov <denis.pryazhennikov@arknetworks.am>
Acked-by: Ivan Malov <ivan.malov@arknetworks.am>
Acked-by: Viacheslav Galaktionov <viacheslav.galaktionov@arknetworks.am>
---
 lib/ethdev/rte_ethdev.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index 4d0325568322..d02ee161cf6d 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -4751,6 +4751,11 @@ rte_eth_fec_set(uint16_t port_id, uint32_t fec_capa)
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
 	dev = &rte_eth_devices[port_id];
 
+	if (fec_capa == 0) {
+		RTE_ETHDEV_LOG(ERR, "At least one FEC mode should be specified\n");
+		return -EINVAL;
+	}
+
 	if (*dev->dev_ops->fec_set == NULL)
 		return -ENOTSUP;
 	ret = eth_err(port_id, (*dev->dev_ops->fec_set)(dev, fec_capa));
-- 
2.37.0 (Apple Git-136)

[RFC PATCH 3/3] ethdev: rename parameter in API to get FEC

[Denis Pryazhennikov]

Only one valid FEC mode can be get by rte_eth_fec_get().
The previous name implied that more than one FEC mode
can be obtained.
Documentation was updated accordingly.

Signed-off-by: Denis Pryazhennikov <denis.pryazhennikov@arknetworks.am>
Acked-by: Ivan Malov <ivan.malov@arknetworks.am>
Acked-by: Viacheslav Galaktionov <viacheslav.galaktionov@arknetworks.am>
---
 lib/ethdev/rte_ethdev.c | 8 ++++----
 lib/ethdev/rte_ethdev.h | 9 +++------
 2 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index d02ee161cf6d..b046d4ea2f06 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -4718,7 +4718,7 @@ rte_eth_fec_get_capability(uint16_t port_id,
 }
 
 int
-rte_eth_fec_get(uint16_t port_id, uint32_t *fec_capa)
+rte_eth_fec_get(uint16_t port_id, uint32_t *fec_mode)
 {
 	struct rte_eth_dev *dev;
 	int ret;
@@ -4726,7 +4726,7 @@ rte_eth_fec_get(uint16_t port_id, uint32_t *fec_capa)
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
 	dev = &rte_eth_devices[port_id];
 
-	if (fec_capa == NULL) {
+	if (fec_mode == NULL) {
 		RTE_ETHDEV_LOG(ERR,
 			"Cannot get ethdev port %u current FEC mode to NULL\n",
 			port_id);
@@ -4735,9 +4735,9 @@ rte_eth_fec_get(uint16_t port_id, uint32_t *fec_capa)
 
 	if (*dev->dev_ops->fec_get == NULL)
 		return -ENOTSUP;
-	ret = eth_err(port_id, (*dev->dev_ops->fec_get)(dev, fec_capa));
+	ret = eth_err(port_id, (*dev->dev_ops->fec_get)(dev, fec_mode));
 
-	rte_eth_trace_fec_get(port_id, fec_capa, ret);
+	rte_eth_trace_fec_get(port_id, fec_mode, ret);
 
 	return ret;
 }
diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index 0f10ac944061..aa71dbafc01f 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -4203,11 +4203,8 @@ int rte_eth_fec_get_capability(uint16_t port_id,
  *
  * @param port_id
  *   The port identifier of the Ethernet device.
- * @param fec_capa
- *   A bitmask of enabled FEC modes. If AUTO bit is set, other
- *   bits specify FEC modes which may be negotiated. If AUTO
- *   bit is clear, specify FEC modes to be used (only one valid
- *   mode per speed may be set).
+ * @param fec_mode
+ *   The current FEC mode.
  * @return
  *   - (0) if successful.
  *   - (-ENOTSUP) if underlying hardware OR driver doesn't support.
@@ -4216,7 +4213,7 @@ int rte_eth_fec_get_capability(uint16_t port_id,
  *   - (-ENODEV)  if *port_id* invalid.
  */
 __rte_experimental
-int rte_eth_fec_get(uint16_t port_id, uint32_t *fec_capa);
+int rte_eth_fec_get(uint16_t port_id, uint32_t *fec_mode);
 
 /**
  * @warning
-- 
2.37.0 (Apple Git-136)

Re: [RFC PATCH 1/3] ethdev: update documentation for API to set FEC

[Ferruh Yigit]

On 4/28/2023 11:27 AM, Denis Pryazhennikov wrote:
> The documentation for the rte_eth_fec_set() is updated
> to provide more detailed information about how FEC modes are
> handled. It also includes a description of the case when only
> the AUTO bit is set.
> 
> Signed-off-by: Denis Pryazhennikov <denis.pryazhennikov@arknetworks.am>
> Acked-by: Ivan Malov <ivan.malov@arknetworks.am>
> Acked-by: Viacheslav Galaktionov <viacheslav.galaktionov@arknetworks.am>
> ---
>  lib/ethdev/rte_ethdev.h | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
> index 99fe9e238b20..0f10ac944061 100644
> --- a/lib/ethdev/rte_ethdev.h
> +++ b/lib/ethdev/rte_ethdev.h
> @@ -4227,13 +4227,19 @@ int rte_eth_fec_get(uint16_t port_id, uint32_t *fec_capa);
>   * @param port_id
>   *   The port identifier of the Ethernet device.
>   * @param fec_capa
> - *   A bitmask of allowed FEC modes. If AUTO bit is set, other
> - *   bits specify FEC modes which may be negotiated. If AUTO
> - *   bit is clear, specify FEC modes to be used (only one valid
> - *   mode per speed may be set).
> + *   A bitmask of allowed FEC modes.

ack

> + *   If only the AUTO bit is set, the decision on which FEC
> + *   mode to use will be made by HW/FW or driver.

ack

> + *   If the AUTO bit is set, other bits specify FEC modes
> + *   which may be negotiated. It means that only specified
> + *   FEC modes can be set.

What about some simplification, maybe something like:

"
If the AUTO bit is set with some FEC modes, only specified FEC modes can
be set.
"

> + *   If AUTO bit is clear, specify FEC mode to be used
> + *   (only one valid mode per speed may be set).

ack

> + *   NOFEC will be used if specified FEC modes are not
> + *   supported.

If FEC modes are not supported, I think it is returning error, why
change it?

>   * @return
>   *   - (0) if successful.
> - *   - (-EINVAL) if the FEC mode is not valid.
> + *   - (-EINVAL) if *fec_capa* is not valid.

I think original was correct, if FEC mode is not valid, dev_ops returns
EINVAL, which cause API to return the same.

>   *   - (-ENOTSUP) if underlying hardware OR driver doesn't support.
>   *   - (-EIO) if device is removed.
>   *   - (-ENODEV)  if *port_id* invalid.

Re: [RFC PATCH 2/3] ethdev: check that at least one FEC mode is specified

[Ferruh Yigit]

On 4/28/2023 11:27 AM, Denis Pryazhennikov wrote:
> The behaviour is undefined in the rte_eth_fec_set() function
> when the fec_capa parameter is equal to zero.
> Add a check to handle this case.
> 
> Signed-off-by: Denis Pryazhennikov <denis.pryazhennikov@arknetworks.am>
> Acked-by: Ivan Malov <ivan.malov@arknetworks.am>
> Acked-by: Viacheslav Galaktionov <viacheslav.galaktionov@arknetworks.am>
> ---
>  lib/ethdev/rte_ethdev.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
> index 4d0325568322..d02ee161cf6d 100644
> --- a/lib/ethdev/rte_ethdev.c
> +++ b/lib/ethdev/rte_ethdev.c
> @@ -4751,6 +4751,11 @@ rte_eth_fec_set(uint16_t port_id, uint32_t fec_capa)
>  	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
>  	dev = &rte_eth_devices[port_id];
>  
> +	if (fec_capa == 0) {
> +		RTE_ETHDEV_LOG(ERR, "At least one FEC mode should be specified\n");
> +		return -EINVAL;
> +	}
> +
>  	if (*dev->dev_ops->fec_set == NULL)
>  		return -ENOTSUP;
>  	ret = eth_err(port_id, (*dev->dev_ops->fec_set)(dev, fec_capa));

Acked-by: Ferruh Yigit <ferruh.yigit@amd.com>

Re: [RFC PATCH 3/3] ethdev: rename parameter in API to get FEC

[Ferruh Yigit]

On 4/28/2023 11:27 AM, Denis Pryazhennikov wrote:
> Only one valid FEC mode can be get by rte_eth_fec_get().
> The previous name implied that more than one FEC mode
> can be obtained.

+1 and patch looks good.

But isn't this valid for 'rte_eth_fec_set()', it gets 'fec_mode'. FEC
capability has its own type "struct rte_eth_fec_capa".

Independent from being single FEC mode or not, I think both
'rte_eth_fec_get()' & 'rte_eth_fec_set()' should get 'fec_mode' as
param, what do you think?

> Documentation was updated accordingly.
> 
> Signed-off-by: Denis Pryazhennikov <denis.pryazhennikov@arknetworks.am>
> Acked-by: Ivan Malov <ivan.malov@arknetworks.am>
> Acked-by: Viacheslav Galaktionov <viacheslav.galaktionov@arknetworks.am>

<...>

Re: [RFC PATCH 3/3] ethdev: rename parameter in API to get FEC

[Denis Pryazhennikov]

I think my patch isn't correct.
I would say that
- fec_capa is a set of bits produced by 'RTE_ETH_FEC_MODE_TO_CAPA()';
- fec_mode is an element of 'enum rte_eth_fec_mode'.

Based on this definition, replacing 'fec_capa' with 'fec_mode' should 
involve changing the parameter type.
Probably I shouldn't change the name, but I should add a more detailed 
comment.

 > Independent from being single FEC mode or not, I think both 
'rte_eth_fec_get()' & 'rte_eth_fec_set()' should get 'fec_mode' as 
param, what do you think?

Andrew Rybchenko asked to replace 'mode' with 'fec_capa' for 
'rte_eth_fec_set()' in
https://inbox.dpdk.org/dev/aa745bd1-a564-fa8c-c77b-2d99c97690aa@solarflare.com/
I don't think we need to change it for rte_eth_fec_set().

On 5/2/23 7:02 PM, Ferruh Yigit wrote:
> On 4/28/2023 11:27 AM, Denis Pryazhennikov wrote:
>> Only one valid FEC mode can be get by rte_eth_fec_get().
>> The previous name implied that more than one FEC mode
>> can be obtained.
> +1 and patch looks good.
>
> But isn't this valid for 'rte_eth_fec_set()', it gets 'fec_mode'. FEC
> capability has its own type "struct rte_eth_fec_capa".
>
> Independent from being single FEC mode or not, I think both
> 'rte_eth_fec_get()' & 'rte_eth_fec_set()' should get 'fec_mode' as
> param, what do you think?
>
>> Documentation was updated accordingly.
>>
>> Signed-off-by: Denis Pryazhennikov <denis.pryazhennikov@arknetworks.am>
>> Acked-by: Ivan Malov <ivan.malov@arknetworks.am>
>> Acked-by: Viacheslav Galaktionov <viacheslav.galaktionov@arknetworks.am>
> <...>
>

Re: [RFC PATCH 3/3] ethdev: rename parameter in API to get FEC

[Ferruh Yigit]

On 5/4/2023 8:13 AM, Denis Pryazhennikov wrote:
> I think my patch isn't correct.
> I would say that
> - fec_capa is a set of bits produced by 'RTE_ETH_FEC_MODE_TO_CAPA()';
> - fec_mode is an element of 'enum rte_eth_fec_mode'.
> 
> Based on this definition, replacing 'fec_capa' with 'fec_mode' should
> involve changing the parameter type.
> Probably I shouldn't change the name, but I should add a more detailed
> comment.
> 
>> Independent from being single FEC mode or not, I think both
> 'rte_eth_fec_get()' & 'rte_eth_fec_set()' should get 'fec_mode' as
> param, what do you think?
> 
> Andrew Rybchenko asked to replace 'mode' with 'fec_capa' for
> 'rte_eth_fec_set()' in
> https://inbox.dpdk.org/dev/aa745bd1-a564-fa8c-c77b-2d99c97690aa@solarflare.com/
> I don't think we need to change it for rte_eth_fec_set().
> 

OK

> On 5/2/23 7:02 PM, Ferruh Yigit wrote:
>> On 4/28/2023 11:27 AM, Denis Pryazhennikov wrote:
>>> Only one valid FEC mode can be get by rte_eth_fec_get().
>>> The previous name implied that more than one FEC mode
>>> can be obtained.
>> +1 and patch looks good.
>>
>> But isn't this valid for 'rte_eth_fec_set()', it gets 'fec_mode'. FEC
>> capability has its own type "struct rte_eth_fec_capa".
>>
>> Independent from being single FEC mode or not, I think both
>> 'rte_eth_fec_get()' & 'rte_eth_fec_set()' should get 'fec_mode' as
>> param, what do you think?
>>
>>> Documentation was updated accordingly.
>>>
>>> Signed-off-by: Denis Pryazhennikov <denis.pryazhennikov@arknetworks.am>
>>> Acked-by: Ivan Malov <ivan.malov@arknetworks.am>
>>> Acked-by: Viacheslav Galaktionov <viacheslav.galaktionov@arknetworks.am>
>> <...>
>>

[RFC PATCH v2 0/3] improve FEC API usage

[Denis Pryazhennikov]

The documentation for the FEC API is currently incomplete and contains
inaccuracies in its descriptions of function parameters. 
Specifically, the semantics of the fec_capa parameter in rte_eth_fec_set()
is not well-defined. It does not provide information on what should
be done if only AUTO bit is set or one of the specified FEC modes is 
not supported. Additionally, the fec_capa parameter in rte_eth_fec_get()
implies that more than one FEC mode can be obtained, but it is 
wrong. Furthermore, the behaviour is undefined in 
rte_eth_fec_set() when the fec_capa parameter is zero.

To address these issues, a patch series has been created that updates
the FEC API documentation, renames one of the parameters to improve 
its clarity and adds a check for zero fec_capability.

v2:
* Update documentation for rte_eth_fec_set() to fix review comments.
* Don't rename the fec_capa parameter of rte_eth_fec_get() but
  add a proper description instead.

Denis Pryazhennikov (3):
  ethdev: update documentation for API to set FEC
  ethdev: check that at least one FEC mode is specified
  ethdev: update documentation for API to get FEC

 lib/ethdev/rte_ethdev.c |  5 +++++
 lib/ethdev/rte_ethdev.h | 16 ++++++++--------
 2 files changed, 13 insertions(+), 8 deletions(-)

-- 
2.37.0 (Apple Git-136)

[RFC PATCH v2 1/3] ethdev: update documentation for API to set FEC

[Denis Pryazhennikov]

The documentation for the rte_eth_fec_set() is updated
to provide more detailed information about how FEC modes are
handled. It also includes a description of the case when only
the AUTO bit is set.

Signed-off-by: Denis Pryazhennikov <denis.pryazhennikov@arknetworks.am>
Acked-by: Ivan Malov <ivan.malov@arknetworks.am>
Acked-by: Viacheslav Galaktionov <viacheslav.galaktionov@arknetworks.am>
---
 lib/ethdev/rte_ethdev.h | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index 99fe9e238b20..777dc521494c 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -4227,10 +4227,13 @@ int rte_eth_fec_get(uint16_t port_id, uint32_t *fec_capa);
  * @param port_id
  *   The port identifier of the Ethernet device.
  * @param fec_capa
- *   A bitmask of allowed FEC modes. If AUTO bit is set, other
- *   bits specify FEC modes which may be negotiated. If AUTO
- *   bit is clear, specify FEC modes to be used (only one valid
- *   mode per speed may be set).
+ *   A bitmask of allowed FEC modes.
+ *   If only the AUTO bit is set, the decision on which FEC
+ *   mode to use will be made by HW/FW or driver.
+ *   If the AUTO bit is set with some FEC modes, only specified
+ *   FEC modes can be set.
+ *   If AUTO bit is clear, specify FEC mode to be used
+ *   (only one valid mode per speed may be set).
  * @return
  *   - (0) if successful.
  *   - (-EINVAL) if the FEC mode is not valid.
-- 
2.37.0 (Apple Git-136)

[RFC PATCH v2 2/3] ethdev: check that at least one FEC mode is specified

[Denis Pryazhennikov]

The behaviour is undefined in the rte_eth_fec_set() function
when the fec_capa parameter is equal to zero.
Add a check to handle this case.

Signed-off-by: Denis Pryazhennikov <denis.pryazhennikov@arknetworks.am>
Acked-by: Ivan Malov <ivan.malov@arknetworks.am>
Acked-by: Viacheslav Galaktionov <viacheslav.galaktionov@arknetworks.am>
---
 lib/ethdev/rte_ethdev.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index 4d0325568322..d02ee161cf6d 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -4751,6 +4751,11 @@ rte_eth_fec_set(uint16_t port_id, uint32_t fec_capa)
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
 	dev = &rte_eth_devices[port_id];
 
+	if (fec_capa == 0) {
+		RTE_ETHDEV_LOG(ERR, "At least one FEC mode should be specified\n");
+		return -EINVAL;
+	}
+
 	if (*dev->dev_ops->fec_set == NULL)
 		return -ENOTSUP;
 	ret = eth_err(port_id, (*dev->dev_ops->fec_set)(dev, fec_capa));
-- 
2.37.0 (Apple Git-136)

[RFC PATCH v2 3/3] ethdev: update documentation for API to get FEC

[Denis Pryazhennikov]

The documentation for the rte_eth_fec_get() is updated
to clarify the description for the fec_capa parameter.
The previous description implied that more than one FEC
mode can be obtained.

Signed-off-by: Denis Pryazhennikov <denis.pryazhennikov@arknetworks.am>
---
 lib/ethdev/rte_ethdev.h | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index 777dc521494c..ab9732f21bdf 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -4204,10 +4204,7 @@ int rte_eth_fec_get_capability(uint16_t port_id,
  * @param port_id
  *   The port identifier of the Ethernet device.
  * @param fec_capa
- *   A bitmask of enabled FEC modes. If AUTO bit is set, other
- *   bits specify FEC modes which may be negotiated. If AUTO
- *   bit is clear, specify FEC modes to be used (only one valid
- *   mode per speed may be set).
+ *   A bitmask with the current FEC mode.
  * @return
  *   - (0) if successful.
  *   - (-ENOTSUP) if underlying hardware OR driver doesn't support.
-- 
2.37.0 (Apple Git-136)

Re: [RFC PATCH v2 0/3] improve FEC API usage

[Ferruh Yigit]

On 5/8/2023 12:47 PM, Denis Pryazhennikov wrote:
> The documentation for the FEC API is currently incomplete and contains
> inaccuracies in its descriptions of function parameters. 
> Specifically, the semantics of the fec_capa parameter in rte_eth_fec_set()
> is not well-defined. It does not provide information on what should
> be done if only AUTO bit is set or one of the specified FEC modes is 
> not supported. Additionally, the fec_capa parameter in rte_eth_fec_get()
> implies that more than one FEC mode can be obtained, but it is 
> wrong. Furthermore, the behaviour is undefined in 
> rte_eth_fec_set() when the fec_capa parameter is zero.
> 
> To address these issues, a patch series has been created that updates
> the FEC API documentation, renames one of the parameters to improve 
> its clarity and adds a check for zero fec_capability.
> 
> v2:
> * Update documentation for rte_eth_fec_set() to fix review comments.
> * Don't rename the fec_capa parameter of rte_eth_fec_get() but
>   add a proper description instead.
> 
> Denis Pryazhennikov (3):
>   ethdev: update documentation for API to set FEC
>   ethdev: check that at least one FEC mode is specified
>   ethdev: update documentation for API to get FEC
> 

For series,
Acked-by: Ferruh Yigit <ferruh.yigit@amd.com>


+cc Author and reviewer of original patch, if there is no objection I
can proceed with the set.


@Denis, can you please provide Fixes tags too? If you prefer you can
send a new version with ack and fixes tags.

Re: [RFC PATCH v2 0/3] improve FEC API usage

[Ferruh Yigit]

On 5/12/2023 12:57 PM, Ferruh Yigit wrote:
> On 5/8/2023 12:47 PM, Denis Pryazhennikov wrote:
>> The documentation for the FEC API is currently incomplete and contains
>> inaccuracies in its descriptions of function parameters. 
>> Specifically, the semantics of the fec_capa parameter in rte_eth_fec_set()
>> is not well-defined. It does not provide information on what should
>> be done if only AUTO bit is set or one of the specified FEC modes is 
>> not supported. Additionally, the fec_capa parameter in rte_eth_fec_get()
>> implies that more than one FEC mode can be obtained, but it is 
>> wrong. Furthermore, the behaviour is undefined in 
>> rte_eth_fec_set() when the fec_capa parameter is zero.
>>
>> To address these issues, a patch series has been created that updates
>> the FEC API documentation, renames one of the parameters to improve 
>> its clarity and adds a check for zero fec_capability.
>>
>> v2:
>> * Update documentation for rte_eth_fec_set() to fix review comments.
>> * Don't rename the fec_capa parameter of rte_eth_fec_get() but
>>   add a proper description instead.
>>
>> Denis Pryazhennikov (3):
>>   ethdev: update documentation for API to set FEC
>>   ethdev: check that at least one FEC mode is specified
>>   ethdev: update documentation for API to get FEC
>>
> 
> For series,
> Acked-by: Ferruh Yigit <ferruh.yigit@amd.com>
> 
> 
> +cc Author and reviewer of original patch, if there is no objection I
> can proceed with the set.
> 
> 
> @Denis, can you please provide Fixes tags too? If you prefer you can
> send a new version with ack and fixes tags.
>

@Denis, just a reminder that this patch is waiting for a new version.

Re: [RFC PATCH v2 0/3] improve FEC API usage

[Ferruh Yigit]

On 5/12/2023 12:57 PM, Ferruh Yigit wrote:
> On 5/8/2023 12:47 PM, Denis Pryazhennikov wrote:
>> The documentation for the FEC API is currently incomplete and contains
>> inaccuracies in its descriptions of function parameters. 
>> Specifically, the semantics of the fec_capa parameter in rte_eth_fec_set()
>> is not well-defined. It does not provide information on what should
>> be done if only AUTO bit is set or one of the specified FEC modes is 
>> not supported. Additionally, the fec_capa parameter in rte_eth_fec_get()
>> implies that more than one FEC mode can be obtained, but it is 
>> wrong. Furthermore, the behaviour is undefined in 
>> rte_eth_fec_set() when the fec_capa parameter is zero.
>>
>> To address these issues, a patch series has been created that updates
>> the FEC API documentation, renames one of the parameters to improve 
>> its clarity and adds a check for zero fec_capability.
>>
>> v2:
>> * Update documentation for rte_eth_fec_set() to fix review comments.
>> * Don't rename the fec_capa parameter of rte_eth_fec_get() but
>>   add a proper description instead.
>>
>> Denis Pryazhennikov (3):
>>   ethdev: update documentation for API to set FEC
>>   ethdev: check that at least one FEC mode is specified
>>   ethdev: update documentation for API to get FEC
>>
> 
> For series,
> Acked-by: Ferruh Yigit <ferruh.yigit@amd.com>
> 
> 

Series applied to dpdk-next-net/main, thanks.

> +cc Author and reviewer of original patch, if there is no objection I
> can proceed with the set.
> 
> 
> @Denis, can you please provide Fixes tags too? If you prefer you can
> send a new version with ack and fixes tags.
>

    Fixes: b7ccfb09da95 ("ethdev: introduce FEC API")
    Cc: stable@dpdk.org