diff mbox series

[30/36] net/sfc: fix Rx and Tx queue state

Message ID 20230908112901.1169869-31-haijie1@huawei.com (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers
Series fix Rx and Tx queue state |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Jie Hai Sept. 8, 2023, 11:28 a.m. UTC 
  The DPDK framework reports the queue state, which is stored in
dev->data->tx_queue_state and dev->data->rx_queue_state. The
state is maintained by the driver. Users may determine whether
a queue participates in packet forwarding based on the state.
Therefore, the driver needs to modify the queue state in time
according to the actual situation.

Fixes: 9ad9ff476cac ("ethdev: add queue state in queried queue information")
Cc: stable@dpdk.org

Signed-off-by: Jie Hai <haijie1@huawei.com>
---
 drivers/net/sfc/sfc_repr.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

Comments

Andrew Rybchenko Sept. 8, 2023, 12:01 p.m. UTC | #1 
On 9/8/23 14:28, Jie Hai wrote:
> The DPDK framework reports the queue state, which is stored in
> dev->data->tx_queue_state and dev->data->rx_queue_state. The
> state is maintained by the driver. Users may determine whether
> a queue participates in packet forwarding based on the state.
> Therefore, the driver needs to modify the queue state in time
> according to the actual situation.
> 
> Fixes: 9ad9ff476cac ("ethdev: add queue state in queried queue information")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Jie Hai <haijie1@huawei.com>
> ---
>   drivers/net/sfc/sfc_repr.c | 12 ++++++++++++
>   1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/net/sfc/sfc_repr.c b/drivers/net/sfc/sfc_repr.c
> index 6c7727d56980..278e37477530 100644
> --- a/drivers/net/sfc/sfc_repr.c
> +++ b/drivers/net/sfc/sfc_repr.c
> @@ -263,6 +263,7 @@ static int
>   sfc_repr_dev_start(struct rte_eth_dev *dev)
>   {
>   	struct sfc_repr *sr = sfc_repr_by_eth_dev(dev);
> +	uint16_t i;
>   	int ret;
>   
>   	sfcr_info(sr, "entry");
> @@ -274,6 +275,11 @@ sfc_repr_dev_start(struct rte_eth_dev *dev)
>   	if (ret != 0)
>   		goto fail_start;
>   
> +	for (i = 0; i < dev->data->nb_rx_queues; i++)
> +		dev->data->rx_queue_state[i] = RTE_ETH_QUEUE_STATE_STARTED;
> +	for (i = 0; i < dev->data->nb_tx_queues; i++)
> +		dev->data->tx_queue_state[i] = RTE_ETH_QUEUE_STATE_STARTED;

May be I miss something, but the driver does when queue is
really started in sfc_rx_qstart() and sfc_tx_qstart().
Also the patch is wrong in general in the case of deferred
start queues.

Same for stop.

> +
>   	sfcr_info(sr, "done");
>   
>   	return 0;
> @@ -338,6 +344,7 @@ static int
>   sfc_repr_dev_stop(struct rte_eth_dev *dev)
>   {
>   	struct sfc_repr *sr = sfc_repr_by_eth_dev(dev);
> +	uint16_t i;
>   	int ret;
>   
>   	sfcr_info(sr, "entry");
> @@ -352,6 +359,11 @@ sfc_repr_dev_stop(struct rte_eth_dev *dev)
>   
>   	sfc_repr_unlock(sr);
>   
> +	for (i = 0; i < dev->data->nb_rx_queues; i++)
> +		dev->data->rx_queue_state[i] = RTE_ETH_QUEUE_STATE_STOPPED;
> +	for (i = 0; i < dev->data->nb_tx_queues; i++)
> +		dev->data->tx_queue_state[i] = RTE_ETH_QUEUE_STATE_STOPPED;
> +
>   	sfcr_info(sr, "done");
>   
>   	return 0;
Jie Hai Sept. 12, 2023, 2:39 a.m. UTC | #2 
On 2023/9/8 20:01, Andrew Rybchenko wrote:
> On 9/8/23 14:28, Jie Hai wrote:
>> The DPDK framework reports the queue state, which is stored in
>> dev->data->tx_queue_state and dev->data->rx_queue_state. The
>> state is maintained by the driver. Users may determine whether
>> a queue participates in packet forwarding based on the state.
>> Therefore, the driver needs to modify the queue state in time
>> according to the actual situation.
>>
>> Fixes: 9ad9ff476cac ("ethdev: add queue state in queried queue 
>> information")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Jie Hai <haijie1@huawei.com>
>> ---
>>   drivers/net/sfc/sfc_repr.c | 12 ++++++++++++
>>   1 file changed, 12 insertions(+)
>>
>> diff --git a/drivers/net/sfc/sfc_repr.c b/drivers/net/sfc/sfc_repr.c
>> index 6c7727d56980..278e37477530 100644
>> --- a/drivers/net/sfc/sfc_repr.c
>> +++ b/drivers/net/sfc/sfc_repr.c
>> @@ -263,6 +263,7 @@ static int
>>   sfc_repr_dev_start(struct rte_eth_dev *dev)
>>   {
>>       struct sfc_repr *sr = sfc_repr_by_eth_dev(dev);
>> +    uint16_t i;
>>       int ret;
>>       sfcr_info(sr, "entry");
>> @@ -274,6 +275,11 @@ sfc_repr_dev_start(struct rte_eth_dev *dev)
>>       if (ret != 0)
>>           goto fail_start;
>> +    for (i = 0; i < dev->data->nb_rx_queues; i++)
>> +        dev->data->rx_queue_state[i] = RTE_ETH_QUEUE_STATE_STARTED;
>> +    for (i = 0; i < dev->data->nb_tx_queues; i++)
>> +        dev->data->tx_queue_state[i] = RTE_ETH_QUEUE_STATE_STARTED;
> 
> May be I miss something, but the driver does when queue is
> really started in sfc_rx_qstart() and sfc_tx_qstart().
> Also the patch is wrong in general in the case of deferred
> start queues.
> 
> Same for stop.
Hi, Andrew Rybchenko,

Thanks for your review.

There are two ops implementations,
one in file sfc_ethdev.c(eg. sfc_dev_start)
and the other in file sfc_repr.c (eg. sfc_repr_dev_start).
This patch is for the second one, which does not support deferred start, 
please see 'sfc_repr_rx_qcheck_conf' and 'sfc_repr_tx_qcheck_conf'.

The function process of ‘sfc_repr_dev_start’and ‘sfc_repr_dev_stop’ is 
as follows:
sfc_repr_dev_start
   -->sfc_repr_start
     -->sfc_repr_proxy_start_repr
       -->sfc_repr_proxy_start
       	-->sfc_repr_proxy_rxq_start
       	  -->sfc_rx_qstart -- RTE_ETH_QUEUE_STATE_STARTED
       	-->sfc_repr_proxy_txq_start

sfc_repr_dev_stop
   -->sfc_repr_stop
     -->sfc_repr_proxy_stop_repr
       -->sfc_repr_proxy_stop
       	-->sfc_repr_proxy_rxq_stop
       	  -->sfc_rx_qstop -- RTE_ETH_QUEUE_STATE_STOPPED
       	-->sfc_repr_proxy_txq_stop

Since the Rx has been modified, Maybe I should modify the TX queue 
status separately in 'sfc_repr_proxy_txq_start/stop'.

--
Best regards,
Jie Hai
> 
>> +
>>       sfcr_info(sr, "done");
>>       return 0;
>> @@ -338,6 +344,7 @@ static int
>>   sfc_repr_dev_stop(struct rte_eth_dev *dev)
>>   {
>>       struct sfc_repr *sr = sfc_repr_by_eth_dev(dev);
>> +    uint16_t i;
>>       int ret;
>>       sfcr_info(sr, "entry");
>> @@ -352,6 +359,11 @@ sfc_repr_dev_stop(struct rte_eth_dev *dev)
>>       sfc_repr_unlock(sr);
>> +    for (i = 0; i < dev->data->nb_rx_queues; i++)
>> +        dev->data->rx_queue_state[i] = RTE_ETH_QUEUE_STATE_STOPPED;
>> +    for (i = 0; i < dev->data->nb_tx_queues; i++)
>> +        dev->data->tx_queue_state[i] = RTE_ETH_QUEUE_STATE_STOPPED;
>> +
>>       sfcr_info(sr, "done");
>>       return 0;
> 
> .
diff mbox series

Patch

diff --git a/drivers/net/sfc/sfc_repr.c b/drivers/net/sfc/sfc_repr.c
index 6c7727d56980..278e37477530 100644
--- a/drivers/net/sfc/sfc_repr.c
+++ b/drivers/net/sfc/sfc_repr.c
@@ -263,6 +263,7 @@  static int
 sfc_repr_dev_start(struct rte_eth_dev *dev)
 {
 	struct sfc_repr *sr = sfc_repr_by_eth_dev(dev);
+	uint16_t i;
 	int ret;
 
 	sfcr_info(sr, "entry");
@@ -274,6 +275,11 @@  sfc_repr_dev_start(struct rte_eth_dev *dev)
 	if (ret != 0)
 		goto fail_start;
 
+	for (i = 0; i < dev->data->nb_rx_queues; i++)
+		dev->data->rx_queue_state[i] = RTE_ETH_QUEUE_STATE_STARTED;
+	for (i = 0; i < dev->data->nb_tx_queues; i++)
+		dev->data->tx_queue_state[i] = RTE_ETH_QUEUE_STATE_STARTED;
+
 	sfcr_info(sr, "done");
 
 	return 0;
@@ -338,6 +344,7 @@  static int
 sfc_repr_dev_stop(struct rte_eth_dev *dev)
 {
 	struct sfc_repr *sr = sfc_repr_by_eth_dev(dev);
+	uint16_t i;
 	int ret;
 
 	sfcr_info(sr, "entry");
@@ -352,6 +359,11 @@  sfc_repr_dev_stop(struct rte_eth_dev *dev)
 
 	sfc_repr_unlock(sr);
 
+	for (i = 0; i < dev->data->nb_rx_queues; i++)
+		dev->data->rx_queue_state[i] = RTE_ETH_QUEUE_STATE_STOPPED;
+	for (i = 0; i < dev->data->nb_tx_queues; i++)
+		dev->data->tx_queue_state[i] = RTE_ETH_QUEUE_STATE_STOPPED;
+
 	sfcr_info(sr, "done");
 
 	return 0;