diff mbox series

[v3] app/testpmd: fix incorrect queues state of secondary process

Message ID 20220906145310.8849-1-peng1x.zhang@intel.com (mailing list archive)
State Changes Requested, archived
Delegated to: Andrew Rybchenko
Headers
Series [v3] app/testpmd: fix incorrect queues state of secondary process |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/github-robot: build success github build: passed
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/Intel-compilation fail Compilation issues
ci/intel-Testing success Testing PASS

Commit Message

Zhang, Peng1X Sept. 6, 2022, 2:53 p.m. UTC 
  Primary process could set up queues state correctly when starting port,
while secondary process not. Under multi-process scenario, "stream_init"
function would get wrong queues state for secondary process.

This commit is to get queues state from ethdev which is located in
shared memory.

Fixes: 3c4426db54fc ("app/testpmd: do not poll stopped queues")
Cc: stable@dpdk.org

Signed-off-by: Peng Zhang <peng1x.zhang@intel.com>

---
 v3:
 - Modify the parameter of rx or tx queue state array 
 v2:
 - Change the way of getting secondary process queues states
---
 app/test-pmd/testpmd.c | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

Comments

lihuisong (C) Sept. 7, 2022, 1:53 a.m. UTC | #1 
在 2022/9/6 22:53, Peng Zhang 写道:
> Primary process could set up queues state correctly when starting port,
> while secondary process not. Under multi-process scenario, "stream_init"
> function would get wrong queues state for secondary process.
>
> This commit is to get queues state from ethdev which is located in
> shared memory.
>
> Fixes: 3c4426db54fc ("app/testpmd: do not poll stopped queues")
> Cc: stable@dpdk.org
>
> Signed-off-by: Peng Zhang <peng1x.zhang@intel.com>
>
> ---
>   v3:
>   - Modify the parameter of rx or tx queue state array
>   v2:
>   - Change the way of getting secondary process queues states
> ---
>   app/test-pmd/testpmd.c | 22 +++++++++++++++++++---
>   1 file changed, 19 insertions(+), 3 deletions(-)
>
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> index addcbcac85..977ec4fa28 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -75,6 +75,8 @@
>   
>   #include "testpmd.h"
>   
> +#include <ethdev_driver.h>
This header file is internal, app shouldn't use it.
> +
>   #ifndef MAP_HUGETLB
>   /* FreeBSD may not have MAP_HUGETLB (in fact, it probably doesn't) */
>   #define HUGE_FLAG (0x40000)
> @@ -2402,10 +2404,24 @@ start_packet_forwarding(int with_tx_first)
>   	if (!pkt_fwd_shared_rxq_check())
>   		return;
>   
> -	if (stream_init != NULL)
> -		for (i = 0; i < cur_fwd_config.nb_fwd_streams; i++)
> -			stream_init(fwd_streams[i]);
> +	if (stream_init != NULL) {
> +		for (i = 0; i < cur_fwd_config.nb_fwd_streams; i++) {
> +			if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
> +				struct fwd_stream *fs = fwd_streams[i];
> +				struct rte_eth_dev_data *dev_rx_data, *dev_tx_data;
> +
> +				dev_rx_data = (&rte_eth_devices[fs->rx_port])->data;
> +				dev_tx_data = (&rte_eth_devices[fs->tx_port])->data;
> +
> +				uint8_t rx_state = dev_rx_data->rx_queue_state[fs->rx_queue];
> +				ports[fs->rx_port].rxq[fs->rx_queue].state = rx_state;
> +				uint8_t tx_state = dev_tx_data->tx_queue_state[fs->tx_queue];
> +				ports[fs->tx_port].txq[fs->tx_queue].state = tx_state;
> +			}
>   
> +			stream_init(fwd_streams[i]);
> +		}
> +	}
>   
Suggest that an API in ethdev layer can be encapsulated to obtain the 
device Rx/Tx queue state.
Both primary and secondary process get or set queue state by this API.
>   	port_fwd_begin = cur_fwd_config.fwd_eng->port_fwd_begin;
>   	if (port_fwd_begin != NULL) {
>   		for (i = 0; i < cur_fwd_config.nb_fwd_ports; i++) {
Zhang, Peng1X Sept. 10, 2022, 9:21 a.m. UTC | #2 
> -----Original Message-----
> From: lihuisong (C) <lihuisong@huawei.com>
> Sent: Wednesday, September 7, 2022 9:53 AM
> To: Zhang, Peng1X <peng1x.zhang@intel.com>; dev@dpdk.org
> Cc: andrew.rybchenko@oktetlabs.ru; Singh, Aman Deep
> <aman.deep.singh@intel.com>; Zhang, Yuying <yuying.zhang@intel.com>;
> stable@dpdk.org
> Subject: Re: [PATCH v3] app/testpmd: fix incorrect queues state of secondary
> process
> 
> 
> 在 2022/9/6 22:53, Peng Zhang 写道:
> > Primary process could set up queues state correctly when starting
> > port, while secondary process not. Under multi-process scenario,
> "stream_init"
> > function would get wrong queues state for secondary process.
> >
> > This commit is to get queues state from ethdev which is located in
> > shared memory.
> >
> > Fixes: 3c4426db54fc ("app/testpmd: do not poll stopped queues")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Peng Zhang <peng1x.zhang@intel.com>
> >
> > ---
> >   v3:
> >   - Modify the parameter of rx or tx queue state array
> >   v2:
> >   - Change the way of getting secondary process queues states
> > ---
> >   app/test-pmd/testpmd.c | 22 +++++++++++++++++++---
> >   1 file changed, 19 insertions(+), 3 deletions(-)
> >
> > diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> > addcbcac85..977ec4fa28 100644
> > --- a/app/test-pmd/testpmd.c
> > +++ b/app/test-pmd/testpmd.c
> > @@ -75,6 +75,8 @@
> >
> >   #include "testpmd.h"
> >
> > +#include <ethdev_driver.h>
> This header file is internal, app shouldn't use it.

In this case not all PMDs implement 'rte_eth_rx/tx_queue_info_get()' and ethdev won't return 'dev->data->rx/tx_queue_state'.

> > +
> >   #ifndef MAP_HUGETLB
> >   /* FreeBSD may not have MAP_HUGETLB (in fact, it probably doesn't) */
> >   #define HUGE_FLAG (0x40000)
> > @@ -2402,10 +2404,24 @@ start_packet_forwarding(int with_tx_first)
> >   	if (!pkt_fwd_shared_rxq_check())
> >   		return;
> >
> > -	if (stream_init != NULL)
> > -		for (i = 0; i < cur_fwd_config.nb_fwd_streams; i++)
> > -			stream_init(fwd_streams[i]);
> > +	if (stream_init != NULL) {
> > +		for (i = 0; i < cur_fwd_config.nb_fwd_streams; i++) {
> > +			if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
> > +				struct fwd_stream *fs = fwd_streams[i];
> > +				struct rte_eth_dev_data *dev_rx_data,
> *dev_tx_data;
> > +
> > +				dev_rx_data = (&rte_eth_devices[fs->rx_port])-
> >data;
> > +				dev_tx_data = (&rte_eth_devices[fs->tx_port])-
> >data;
> > +
> > +				uint8_t rx_state = dev_rx_data-
> >rx_queue_state[fs->rx_queue];
> > +				ports[fs->rx_port].rxq[fs->rx_queue].state =
> rx_state;
> > +				uint8_t tx_state = dev_tx_data-
> >tx_queue_state[fs->tx_queue];
> > +				ports[fs->tx_port].txq[fs->tx_queue].state =
> tx_state;
> > +			}
> >
> > +			stream_init(fwd_streams[i]);
> > +		}
> > +	}
> >
> Suggest that an API in ethdev layer can be encapsulated to obtain the device
> Rx/Tx queue state.
> Both primary and secondary process get or set queue state by this API.

Suggestion sounds good, maybe better to add a new API in ethdev layer.

@andrew, what's your opinion about this solution and huisong's suggestion?

> >   	port_fwd_begin = cur_fwd_config.fwd_eng->port_fwd_begin;
> >   	if (port_fwd_begin != NULL) {
> >   		for (i = 0; i < cur_fwd_config.nb_fwd_ports; i++) {
lihuisong (C) Sept. 13, 2022, 1:26 a.m. UTC | #3 
在 2022/9/10 17:21, Zhang, Peng1X 写道:
>
>> -----Original Message-----
>> From: lihuisong (C) <lihuisong@huawei.com>
>> Sent: Wednesday, September 7, 2022 9:53 AM
>> To: Zhang, Peng1X <peng1x.zhang@intel.com>; dev@dpdk.org
>> Cc: andrew.rybchenko@oktetlabs.ru; Singh, Aman Deep
>> <aman.deep.singh@intel.com>; Zhang, Yuying <yuying.zhang@intel.com>;
>> stable@dpdk.org
>> Subject: Re: [PATCH v3] app/testpmd: fix incorrect queues state of secondary
>> process
>>
>>
>> 在 2022/9/6 22:53, Peng Zhang 写道:
>>> Primary process could set up queues state correctly when starting
>>> port, while secondary process not. Under multi-process scenario,
>> "stream_init"
>>> function would get wrong queues state for secondary process.
>>>
>>> This commit is to get queues state from ethdev which is located in
>>> shared memory.
>>>
>>> Fixes: 3c4426db54fc ("app/testpmd: do not poll stopped queues")
>>> Cc: stable@dpdk.org
>>>
>>> Signed-off-by: Peng Zhang <peng1x.zhang@intel.com>
>>>
>>> ---
>>>    v3:
>>>    - Modify the parameter of rx or tx queue state array
>>>    v2:
>>>    - Change the way of getting secondary process queues states
>>> ---
>>>    app/test-pmd/testpmd.c | 22 +++++++++++++++++++---
>>>    1 file changed, 19 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
>>> addcbcac85..977ec4fa28 100644
>>> --- a/app/test-pmd/testpmd.c
>>> +++ b/app/test-pmd/testpmd.c
>>> @@ -75,6 +75,8 @@
>>>
>>>    #include "testpmd.h"
>>>
>>> +#include <ethdev_driver.h>
>> This header file is internal, app shouldn't use it.
> In this case not all PMDs implement 'rte_eth_rx/tx_queue_info_get()' and ethdev won't return 'dev->data->rx/tx_queue_state'.
I don't think Rx/Tx queue state needs to be put in the API above.
The queue state is modified by calling 'rte_eth_dev_rx/tx_queue_stop/start'.
Can we create a new API to report queue state? like, 
'rte_eth_dev_rx/tx_queue_state_get'.
If some PMDs do not support 'rte_eth_dev_rx/tx_queue_stop/start', the new
API always return 'START' state and preserves its original behavior.

What do you think?
>
>>> +
>>>    #ifndef MAP_HUGETLB
>>>    /* FreeBSD may not have MAP_HUGETLB (in fact, it probably doesn't) */
>>>    #define HUGE_FLAG (0x40000)
>>> @@ -2402,10 +2404,24 @@ start_packet_forwarding(int with_tx_first)
>>>    	if (!pkt_fwd_shared_rxq_check())
>>>    		return;
>>>
>>> -	if (stream_init != NULL)
>>> -		for (i = 0; i < cur_fwd_config.nb_fwd_streams; i++)
>>> -			stream_init(fwd_streams[i]);
>>> +	if (stream_init != NULL) {
>>> +		for (i = 0; i < cur_fwd_config.nb_fwd_streams; i++) {
>>> +			if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
>>> +				struct fwd_stream *fs = fwd_streams[i];
>>> +				struct rte_eth_dev_data *dev_rx_data,
>> *dev_tx_data;
>>> +
>>> +				dev_rx_data = (&rte_eth_devices[fs->rx_port])-
>>> data;
>>> +				dev_tx_data = (&rte_eth_devices[fs->tx_port])-
>>> data;
>>> +
>>> +				uint8_t rx_state = dev_rx_data-
>>> rx_queue_state[fs->rx_queue];
>>> +				ports[fs->rx_port].rxq[fs->rx_queue].state =
>> rx_state;
>>> +				uint8_t tx_state = dev_tx_data-
>>> tx_queue_state[fs->tx_queue];
>>> +				ports[fs->tx_port].txq[fs->tx_queue].state =
>> tx_state;
>>> +			}
>>>
>>> +			stream_init(fwd_streams[i]);
>>> +		}
>>> +	}
>>>
>> Suggest that an API in ethdev layer can be encapsulated to obtain the device
>> Rx/Tx queue state.
>> Both primary and secondary process get or set queue state by this API.
> Suggestion sounds good, maybe better to add a new API in ethdev layer.
>
> @andrew, what's your opinion about this solution and huisong's suggestion?
>
>>>    	port_fwd_begin = cur_fwd_config.fwd_eng->port_fwd_begin;
>>>    	if (port_fwd_begin != NULL) {
>>>    		for (i = 0; i < cur_fwd_config.nb_fwd_ports; i++) {
Yiding Zhou Sept. 29, 2022, 1:58 a.m. UTC | #4 
> > > Primary process could set up queues state correctly when starting
> > > port, while secondary process not. Under multi-process scenario,
> > "stream_init"
> > > function would get wrong queues state for secondary process.
> > >
> > > This commit is to get queues state from ethdev which is located in
> > > shared memory.
> > >
> > > Fixes: 3c4426db54fc ("app/testpmd: do not poll stopped queues")
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Peng Zhang <peng1x.zhang@intel.com>
> > >
> > > ---
> > >   v3:
> > >   - Modify the parameter of rx or tx queue state array
> > >   v2:
> > >   - Change the way of getting secondary process queues states
> > > ---
> > >   app/test-pmd/testpmd.c | 22 +++++++++++++++++++---
> > >   1 file changed, 19 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> > > addcbcac85..977ec4fa28 100644
> > > --- a/app/test-pmd/testpmd.c
> > > +++ b/app/test-pmd/testpmd.c
> > > @@ -75,6 +75,8 @@
> > >
> > >   #include "testpmd.h"
> > >
> > > +#include <ethdev_driver.h>
> > This header file is internal, app shouldn't use it.
> 
> In this case not all PMDs implement 'rte_eth_rx/tx_queue_info_get()' and
> ethdev won't return 'dev->data->rx/tx_queue_state'.
> 
> > > +
> > >   #ifndef MAP_HUGETLB
> > >   /* FreeBSD may not have MAP_HUGETLB (in fact, it probably doesn't) */
> > >   #define HUGE_FLAG (0x40000)
> > > @@ -2402,10 +2404,24 @@ start_packet_forwarding(int with_tx_first)
> > >   	if (!pkt_fwd_shared_rxq_check())
> > >   		return;
> > >
> > > -	if (stream_init != NULL)
> > > -		for (i = 0; i < cur_fwd_config.nb_fwd_streams; i++)
> > > -			stream_init(fwd_streams[i]);
> > > +	if (stream_init != NULL) {
> > > +		for (i = 0; i < cur_fwd_config.nb_fwd_streams; i++) {
> > > +			if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
> > > +				struct fwd_stream *fs = fwd_streams[i];
> > > +				struct rte_eth_dev_data *dev_rx_data,
> > *dev_tx_data;
> > > +
> > > +				dev_rx_data = (&rte_eth_devices[fs-
> >rx_port])-
> > >data;
> > > +				dev_tx_data = (&rte_eth_devices[fs->tx_port])-
> > >data;
> > > +
> > > +				uint8_t rx_state = dev_rx_data-
> > >rx_queue_state[fs->rx_queue];
> > > +				ports[fs->rx_port].rxq[fs->rx_queue].state =
> > rx_state;
> > > +				uint8_t tx_state = dev_tx_data-
> > >tx_queue_state[fs->tx_queue];
> > > +				ports[fs->tx_port].txq[fs->tx_queue].state =
> > tx_state;
> > > +			}
> > >
> > > +			stream_init(fwd_streams[i]);
> > > +		}
> > > +	}
> > >
> > Suggest that an API in ethdev layer can be encapsulated to obtain the
> > device Rx/Tx queue state.
> > Both primary and secondary process get or set queue state by this API.
> 
> Suggestion sounds good, maybe better to add a new API in ethdev layer.
> 
> @andrew, what's your opinion about this solution and huisong's suggestion?
> 
> > >   	port_fwd_begin = cur_fwd_config.fwd_eng->port_fwd_begin;
> > >   	if (port_fwd_begin != NULL) {
> > >   		for (i = 0; i < cur_fwd_config.nb_fwd_ports; i++) {

Hi, @andrew.rybchenko@oktetlabs.ru can you please help review this patch? Thanks.
Yiding Zhou Oct. 13, 2022, 3:01 a.m. UTC | #5 
Hi

> -----Original Message-----
> From: Zhou, YidingX <yidingx.zhou@intel.com>
> Sent: Thursday, September 29, 2022 9:58 AM
> To: Zhang, Peng1X <peng1x.zhang@intel.com>; lihuisong (C)
> <lihuisong@huawei.com>; dev@dpdk.org; andrew.rybchenko@oktetlabs.ru
> Cc: Singh, Aman Deep <aman.deep.singh@intel.com>; Zhang, Yuying
> <yuying.zhang@intel.com>
> Subject: RE: [PATCH v3] app/testpmd: fix incorrect queues state of secondary
> process
> 
> > > > Primary process could set up queues state correctly when starting
> > > > port, while secondary process not. Under multi-process scenario,
> > > "stream_init"
> > > > function would get wrong queues state for secondary process.
> > > >
> > > > This commit is to get queues state from ethdev which is located in
> > > > shared memory.
> > > >
> > > > Fixes: 3c4426db54fc ("app/testpmd: do not poll stopped queues")
> > > > Cc: stable@dpdk.org
> > > >
> > > > Signed-off-by: Peng Zhang <peng1x.zhang@intel.com>
Stephen Hemminger Oct. 13, 2022, 3:33 a.m. UTC | #6 
On Tue,  6 Sep 2022 22:53:10 +0800
Peng Zhang <peng1x.zhang@intel.com> wrote:

> +			if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
> +				struct fwd_stream *fs = fwd_streams[i];
> +				struct rte_eth_dev_data *dev_rx_data, *dev_tx_data;
> +
> +				dev_rx_data = (&rte_eth_devices[fs->rx_port])->data;
> +				dev_tx_data = (&rte_eth_devices[fs->tx_port])->data;
> +
> +				uint8_t rx_state = dev_rx_data->rx_queue_state[fs->rx_queue];
> +				ports[fs->rx_port].rxq[fs->rx_queue].state = rx_state;
> +				uint8_t tx_state = dev_tx_data->tx_queue_state[fs->tx_queue];
> +				ports[fs->tx_port].txq[fs->tx_queue].state = tx_state;
> +			}

Could the logic be put in stream_init() like this?

It keeps with the function pointer model object style model in that code.
Also, it makes testpmd more dependent on data structures that should be
hidden and internal only (rte_eth_devices).
Yiding Zhou Oct. 14, 2022, 10:11 a.m. UTC | #7 
> 
> > +			if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
> > +				struct fwd_stream *fs = fwd_streams[i];
> > +				struct rte_eth_dev_data *dev_rx_data,
> *dev_tx_data;
> > +
> > +				dev_rx_data = (&rte_eth_devices[fs-
> >rx_port])->data;
> > +				dev_tx_data = (&rte_eth_devices[fs->tx_port])-
> >data;
> > +
> > +				uint8_t rx_state = dev_rx_data-
> >rx_queue_state[fs->rx_queue];
> > +				ports[fs->rx_port].rxq[fs->rx_queue].state =
> rx_state;
> > +				uint8_t tx_state = dev_tx_data-
> >tx_queue_state[fs->tx_queue];
> > +				ports[fs->tx_port].txq[fs->tx_queue].state =
> tx_state;
> > +			}
> 
> Could the logic be put in stream_init() like this?
> 
> It keeps with the function pointer model object style model in that code.
> Also, it makes testpmd more dependent on data structures that should be
> hidden and internal only (rte_eth_devices).
> 
'strean_init' is a function pointer, putting this logic in will produce a lot of similar redundant code in all forwarding modules, like 'iofwd' 'macfwd'  and etc.
It's better to hide 'rte_eth_devices' from testpmd, but this requires an API at the EAL layer to access the queue state,
'rte_eth_rx/tx_queue_info_get()' can not get queue's state now.
Andrew Rybchenko Oct. 17, 2022, 8:05 a.m. UTC | #8 
On 9/13/22 04:26, lihuisong (C) wrote:
> 
> 在 2022/9/10 17:21, Zhang, Peng1X 写道:
>>
>>> -----Original Message-----
>>> From: lihuisong (C) <lihuisong@huawei.com>
>>> Sent: Wednesday, September 7, 2022 9:53 AM
>>> To: Zhang, Peng1X <peng1x.zhang@intel.com>; dev@dpdk.org
>>> Cc: andrew.rybchenko@oktetlabs.ru; Singh, Aman Deep
>>> <aman.deep.singh@intel.com>; Zhang, Yuying <yuying.zhang@intel.com>;
>>> stable@dpdk.org
>>> Subject: Re: [PATCH v3] app/testpmd: fix incorrect queues state of 
>>> secondary
>>> process
>>>
>>>
>>> 在 2022/9/6 22:53, Peng Zhang 写道:
>>>> Primary process could set up queues state correctly when starting
>>>> port, while secondary process not. Under multi-process scenario,
>>> "stream_init"
>>>> function would get wrong queues state for secondary process.
>>>>
>>>> This commit is to get queues state from ethdev which is located in
>>>> shared memory.
>>>>
>>>> Fixes: 3c4426db54fc ("app/testpmd: do not poll stopped queues")
>>>> Cc: stable@dpdk.org
>>>>
>>>> Signed-off-by: Peng Zhang <peng1x.zhang@intel.com>
>>>>
>>>> ---
>>>>    v3:
>>>>    - Modify the parameter of rx or tx queue state array
>>>>    v2:
>>>>    - Change the way of getting secondary process queues states
>>>> ---
>>>>    app/test-pmd/testpmd.c | 22 +++++++++++++++++++---
>>>>    1 file changed, 19 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
>>>> addcbcac85..977ec4fa28 100644
>>>> --- a/app/test-pmd/testpmd.c
>>>> +++ b/app/test-pmd/testpmd.c
>>>> @@ -75,6 +75,8 @@
>>>>
>>>>    #include "testpmd.h"
>>>>
>>>> +#include <ethdev_driver.h>
>>> This header file is internal, app shouldn't use it.

+1

>> In this case not all PMDs implement 'rte_eth_rx/tx_queue_info_get()' 
>> and ethdev won't return 'dev->data->rx/tx_queue_state'.
> I don't think Rx/Tx queue state needs to be put in the API above.
> The queue state is modified by calling 
> 'rte_eth_dev_rx/tx_queue_stop/start'.
> Can we create a new API to report queue state? like, 
> 'rte_eth_dev_rx/tx_queue_state_get'.

We can and it does not require driver callback since ethdev
layer knows the queue state, but do we really need it?
Application controls queues state and should know the state
internally.

> If some PMDs do not support 'rte_eth_dev_rx/tx_queue_stop/start', the new
> API always return 'START' state and preserves its original behavior.

It is not required since ethdev layer knows queue state.

> 
> What do you think?
>>
>>>> +
>>>>    #ifndef MAP_HUGETLB
>>>>    /* FreeBSD may not have MAP_HUGETLB (in fact, it probably 
>>>> doesn't) */
>>>>    #define HUGE_FLAG (0x40000)
>>>> @@ -2402,10 +2404,24 @@ start_packet_forwarding(int with_tx_first)
>>>>        if (!pkt_fwd_shared_rxq_check())
>>>>            return;
>>>>
>>>> -    if (stream_init != NULL)
>>>> -        for (i = 0; i < cur_fwd_config.nb_fwd_streams; i++)
>>>> -            stream_init(fwd_streams[i]);
>>>> +    if (stream_init != NULL) {
>>>> +        for (i = 0; i < cur_fwd_config.nb_fwd_streams; i++) {
>>>> +            if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
>>>> +                struct fwd_stream *fs = fwd_streams[i];
>>>> +                struct rte_eth_dev_data *dev_rx_data,
>>> *dev_tx_data;
>>>> +
>>>> +                dev_rx_data = (&rte_eth_devices[fs->rx_port])-
>>>> data;
>>>> +                dev_tx_data = (&rte_eth_devices[fs->tx_port])-
>>>> data;
>>>> +
>>>> +                uint8_t rx_state = dev_rx_data-
>>>> rx_queue_state[fs->rx_queue];
>>>> +                ports[fs->rx_port].rxq[fs->rx_queue].state =
>>> rx_state;
>>>> +                uint8_t tx_state = dev_tx_data-
>>>> tx_queue_state[fs->tx_queue];
>>>> +                ports[fs->tx_port].txq[fs->tx_queue].state =
>>> tx_state;
>>>> +            }
>>>>
>>>> +            stream_init(fwd_streams[i]);
>>>> +        }
>>>> +    }
>>>>
>>> Suggest that an API in ethdev layer can be encapsulated to obtain the 
>>> device
>>> Rx/Tx queue state.
>>> Both primary and secondary process get or set queue state by this API.
>> Suggestion sounds good, maybe better to add a new API in ethdev layer.
>>
>> @andrew, what's your opinion about this solution and huisong's 
>> suggestion?

May be it is really more convenient to have ethdev API to get
queue state, but may be it is too late for the API in the
release cycle phase. I'm sorry for late review.

>>
>>>>        port_fwd_begin = cur_fwd_config.fwd_eng->port_fwd_begin;
>>>>        if (port_fwd_begin != NULL) {
>>>>            for (i = 0; i < cur_fwd_config.nb_fwd_ports; i++) {
diff mbox series

Patch

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index addcbcac85..977ec4fa28 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -75,6 +75,8 @@ 
 
 #include "testpmd.h"
 
+#include <ethdev_driver.h>
+
 #ifndef MAP_HUGETLB
 /* FreeBSD may not have MAP_HUGETLB (in fact, it probably doesn't) */
 #define HUGE_FLAG (0x40000)
@@ -2402,10 +2404,24 @@  start_packet_forwarding(int with_tx_first)
 	if (!pkt_fwd_shared_rxq_check())
 		return;
 
-	if (stream_init != NULL)
-		for (i = 0; i < cur_fwd_config.nb_fwd_streams; i++)
-			stream_init(fwd_streams[i]);
+	if (stream_init != NULL) {
+		for (i = 0; i < cur_fwd_config.nb_fwd_streams; i++) {
+			if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
+				struct fwd_stream *fs = fwd_streams[i];
+				struct rte_eth_dev_data *dev_rx_data, *dev_tx_data;
+
+				dev_rx_data = (&rte_eth_devices[fs->rx_port])->data;
+				dev_tx_data = (&rte_eth_devices[fs->tx_port])->data;
+
+				uint8_t rx_state = dev_rx_data->rx_queue_state[fs->rx_queue];
+				ports[fs->rx_port].rxq[fs->rx_queue].state = rx_state;
+				uint8_t tx_state = dev_tx_data->tx_queue_state[fs->tx_queue];
+				ports[fs->tx_port].txq[fs->tx_queue].state = tx_state;
+			}
 
+			stream_init(fwd_streams[i]);
+		}
+	}
 	port_fwd_begin = cur_fwd_config.fwd_eng->port_fwd_begin;
 	if (port_fwd_begin != NULL) {
 		for (i = 0; i < cur_fwd_config.nb_fwd_ports; i++) {