mbox series

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

Message ID 20230508114707.89629-1-denis.pryazhennikov@arknetworks.am (mailing list archive)
Headers
Series improve FEC API usage |

Message

Denis Pryazhennikov May 8, 2023, 11:47 a.m. UTC
  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(-)
  

Comments

Ferruh Yigit May 12, 2023, 11:57 a.m. UTC | #1
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.
  
Ferruh Yigit June 7, 2023, 6:41 p.m. UTC | #2
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.
  
Ferruh Yigit June 19, 2023, 12:36 p.m. UTC | #3
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