[v5,1/2] ethdev: fix ethdev configuration state on reset

Message ID 20221221020713.2803232-1-hpothula@marvell.com (mailing list archive)
State Changes Requested, archived
Delegated to: Ferruh Yigit
Headers
Series [v5,1/2] ethdev: fix ethdev configuration state on reset |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation warning apply patch failure

Commit Message

Hanumanth Pothula Dec. 21, 2022, 2:07 a.m. UTC
  Presently, on device reset, ethdev configuration state,
dev_configured, is not reset.

On device reset, reset ethdev configuration state to make
sure device reconfiguration happens cleanly.

Signed-off-by: Hanumanth Pothula <hpothula@marvell.com>
---
v5:
 - Move nic-to-pmd-rx-metadata declaration to struct rte_port.
v4:
 - As per spec rte_eth_rx_metadata_negotiate() is processed only when
   dev_configured is set. Hence can't enable automatically when a flow
   command requests metadata.
 - Add new testpmd command to allow NIC to PMD Rx metadata negotiation.
v3:
 - Updated run_app.rst with the new command line argument,
   nic-to-pmd-rx-metadata.
 - Updated commit text.
v2:
 - taken cared alignment issues
 - renamed command line argument from rx-metadata to nic-to-pmd-rx-metadata
 - renamed variable name from rx-metadata to nic_to_pmd_rx_metadata
---
 lib/ethdev/rte_ethdev.c | 2 ++
 1 file changed, 2 insertions(+)
  

Comments

Hanumanth Pothula Jan. 16, 2023, 10:43 a.m. UTC | #1
Ping

> -----Original Message-----
> From: Hanumanth Pothula <hpothula@marvell.com>
> Sent: Wednesday, December 21, 2022 7:37 AM
> To: Thomas Monjalon <thomas@monjalon.net>; Ferruh Yigit
> <ferruh.yigit@amd.com>; Andrew Rybchenko
> <andrew.rybchenko@oktetlabs.ru>
> Cc: dev@dpdk.org; viacheslavo@nvidia.com; Jerin Jacob Kollanukkaran
> <jerinj@marvell.com>; Nithin Kumar Dabilpuram
> <ndabilpuram@marvell.com>; Hanumanth Reddy Pothula
> <hpothula@marvell.com>
> Subject: [PATCH v5 1/2] ethdev: fix ethdev configuration state on reset
> 
> Presently, on device reset, ethdev configuration state, dev_configured, is
> not reset.
> 
> On device reset, reset ethdev configuration state to make sure device
> reconfiguration happens cleanly.
> 
> Signed-off-by: Hanumanth Pothula <hpothula@marvell.com>
> ---
> v5:
>  - Move nic-to-pmd-rx-metadata declaration to struct rte_port.
> v4:
>  - As per spec rte_eth_rx_metadata_negotiate() is processed only when
>    dev_configured is set. Hence can't enable automatically when a flow
>    command requests metadata.
>  - Add new testpmd command to allow NIC to PMD Rx metadata
> negotiation.
> v3:
>  - Updated run_app.rst with the new command line argument,
>    nic-to-pmd-rx-metadata.
>  - Updated commit text.
> v2:
>  - taken cared alignment issues
>  - renamed command line argument from rx-metadata to nic-to-pmd-rx-
> metadata
>  - renamed variable name from rx-metadata to nic_to_pmd_rx_metadata
> ---
>  lib/ethdev/rte_ethdev.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c index
> 5d5e18db1e..18c59044bc 100644
> --- a/lib/ethdev/rte_ethdev.c
> +++ b/lib/ethdev/rte_ethdev.c
> @@ -1629,6 +1629,8 @@ rte_eth_dev_reset(uint16_t port_id)
>  			port_id, rte_strerror(-ret));
>  	}
>  	ret = dev->dev_ops->dev_reset(dev);
> +	if (!ret)
> +		dev->data->dev_configured = 0;
> 
>  	return eth_err(port_id, ret);
>  }
> --
> 2.25.1
  
Thomas Monjalon Jan. 18, 2023, 10:29 a.m. UTC | #2
21/12/2022 03:07, Hanumanth Pothula:
> Presently, on device reset, ethdev configuration state,
> dev_configured, is not reset.
> 
> On device reset, reset ethdev configuration state to make
> sure device reconfiguration happens cleanly.
> 
> Signed-off-by: Hanumanth Pothula <hpothula@marvell.com>

A "Fixes" line is missing to show the root cause and help backports.

> --- a/lib/ethdev/rte_ethdev.c
> +++ b/lib/ethdev/rte_ethdev.c
> @@ -1629,6 +1629,8 @@ rte_eth_dev_reset(uint16_t port_id)
>  			port_id, rte_strerror(-ret));
>  	}
>  	ret = dev->dev_ops->dev_reset(dev);
> +	if (!ret)

Should be if (ret == 0)

> +		dev->data->dev_configured = 0;
  
Ferruh Yigit Jan. 24, 2023, 6:14 p.m. UTC | #3
On 1/18/2023 10:29 AM, Thomas Monjalon wrote:
> 21/12/2022 03:07, Hanumanth Pothula:
>> Presently, on device reset, ethdev configuration state,
>> dev_configured, is not reset.
>>
>> On device reset, reset ethdev configuration state to make
>> sure device reconfiguration happens cleanly.
>>
>> Signed-off-by: Hanumanth Pothula <hpothula@marvell.com>
> 
> A "Fixes" line is missing to show the root cause and help backports.
> 

It should be following, that is when 'dev_configured' flag is introduced
but not updated in 'rte_eth_dev_reset()':

Fixes: 02edbfab1e7d ("ethdev: add dev configured flag")
Cc: stable@dpdk.org
Cc: lihuisong@huawei.com


@Hanumanth, can you please send a new version with above fixes line and
below syntax fix?


>> --- a/lib/ethdev/rte_ethdev.c
>> +++ b/lib/ethdev/rte_ethdev.c
>> @@ -1629,6 +1629,8 @@ rte_eth_dev_reset(uint16_t port_id)
>>  			port_id, rte_strerror(-ret));
>>  	}
>>  	ret = dev->dev_ops->dev_reset(dev);
>> +	if (!ret)
> 
> Should be if (ret == 0)
> 
>> +		dev->data->dev_configured = 0;
> 
> 
>
  

Patch

diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index 5d5e18db1e..18c59044bc 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -1629,6 +1629,8 @@  rte_eth_dev_reset(uint16_t port_id)
 			port_id, rte_strerror(-ret));
 	}
 	ret = dev->dev_ops->dev_reset(dev);
+	if (!ret)
+		dev->data->dev_configured = 0;
 
 	return eth_err(port_id, ret);
 }