[dpdk-dev] [PATCH] test/bonding: fix failures after hiding ethdev internal structures

[dpdk-dev] [PATCH] test/bonding: fix failures after hiding ethdev internal structures

[Konstantin Ananyev]

link bounding auto-test internally creates emulated ethdev.
Some tests change Rx/Tx functions of this emulated device on the fly:
by directly modifying rte_eth_dev fields and without doing stop/start
for these devices.
As now ethdev uses rte_eth_fp_ops[] for fast-path functions, these
direct changes doesn't make expected effect.
Fix the problem by guarding fast-path functions changes with
rte_eth_dev_stop()/rte_eth_dev_start().

Fixes: 7a0935239b9e ("ethdev: make fast-path functions to use new flat array")

Reported-by: Lewei Yang <leweix.yang@intel.com>
Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
---
 app/test/virtual_pmd.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/app/test/virtual_pmd.c b/app/test/virtual_pmd.c
index 7e15b47eb0..ccdb418490 100644
--- a/app/test/virtual_pmd.c
+++ b/app/test/virtual_pmd.c
@@ -415,10 +415,14 @@ virtual_ethdev_rx_burst_fn_set_success(uint16_t port_id, uint8_t success)
 {
 	struct rte_eth_dev *vrtl_eth_dev = &rte_eth_devices[port_id];
 
+	rte_eth_dev_stop(port_id);
+
 	if (success)
 		vrtl_eth_dev->rx_pkt_burst = virtual_ethdev_rx_burst_success;
 	else
 		vrtl_eth_dev->rx_pkt_burst = virtual_ethdev_rx_burst_fail;
+
+	rte_eth_dev_start(port_id);
 }
 
 
@@ -428,6 +432,7 @@ virtual_ethdev_tx_burst_fn_set_success(uint16_t port_id, uint8_t success)
 	struct virtual_ethdev_private *dev_private = NULL;
 	struct rte_eth_dev *vrtl_eth_dev = &rte_eth_devices[port_id];
 
+	rte_eth_dev_stop(port_id);
 	dev_private = vrtl_eth_dev->data->dev_private;
 
 	if (success)
@@ -436,6 +441,7 @@ virtual_ethdev_tx_burst_fn_set_success(uint16_t port_id, uint8_t success)
 		vrtl_eth_dev->tx_pkt_burst = virtual_ethdev_tx_burst_fail;
 
 	dev_private->tx_burst_fail_count = 0;
+	rte_eth_dev_start(port_id);
 }
 
 void
@@ -445,7 +451,6 @@ virtual_ethdev_tx_burst_fn_set_tx_pkt_fail_count(uint16_t port_id,
 	struct virtual_ethdev_private *dev_private = NULL;
 	struct rte_eth_dev *vrtl_eth_dev = &rte_eth_devices[port_id];
 
-
 	dev_private = vrtl_eth_dev->data->dev_private;
 	dev_private->tx_burst_fail_count = packet_fail_count;
 }
-- 
2.26.3

Re: [dpdk-dev] [PATCH] test/bonding: fix failures after hiding ethdev internal structures

[Ferruh Yigit]

On 10/22/2021 2:26 PM, Konstantin Ananyev wrote:
> link bounding auto-test internally creates emulated ethdev.
> Some tests change Rx/Tx functions of this emulated device on the fly:
> by directly modifying rte_eth_dev fields and without doing stop/start
> for these devices.
> As now ethdev uses rte_eth_fp_ops[] for fast-path functions, these
> direct changes doesn't make expected effect.
> Fix the problem by guarding fast-path functions changes with
> rte_eth_dev_stop()/rte_eth_dev_start().
> 
> Fixes: 7a0935239b9e ("ethdev: make fast-path functions to use new flat array")
> 
> Reported-by: Lewei Yang <leweix.yang@intel.com>
> Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

Tested 'link_bonding_autotest', it succeed after this patch (failing before)

Tested-by: Ferruh Yigit <ferruh.yigit@intel.com>

Re: [dpdk-dev] [PATCH] test/bonding: fix failures after hiding ethdev internal structures

[Ferruh Yigit]

On 10/22/2021 2:47 PM, Ferruh Yigit wrote:
> On 10/22/2021 2:26 PM, Konstantin Ananyev wrote:
>> link bounding auto-test internally creates emulated ethdev.
>> Some tests change Rx/Tx functions of this emulated device on the fly:
>> by directly modifying rte_eth_dev fields and without doing stop/start
>> for these devices.
>> As now ethdev uses rte_eth_fp_ops[] for fast-path functions, these
>> direct changes doesn't make expected effect.
>> Fix the problem by guarding fast-path functions changes with
>> rte_eth_dev_stop()/rte_eth_dev_start().
>>
>> Fixes: 7a0935239b9e ("ethdev: make fast-path functions to use new flat array")
>>
>> Reported-by: Lewei Yang <leweix.yang@intel.com>
>> Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> 
> Tested 'link_bonding_autotest', it succeed after this patch (failing before)
> 
> Tested-by: Ferruh Yigit <ferruh.yigit@intel.com>
> 

Hi Connor, Chas,

Unrelated with this patch, while I am testing bonding unit test, I recognized
that there are other bonding unit tests failing:
link_bonding_rssconf_autotest
link_bonding_mode4_autotest

They are testing with v21.08 too.
I am not sure if I am missing in my environment, but can you please check
them?

Thanks,
ferruh

Re: [dpdk-dev] [PATCH] test/bonding: fix failures after hiding ethdev internal structures

[Ferruh Yigit]

On 10/22/2021 2:47 PM, Ferruh Yigit wrote:
> On 10/22/2021 2:26 PM, Konstantin Ananyev wrote:
>> link bounding auto-test internally creates emulated ethdev.
>> Some tests change Rx/Tx functions of this emulated device on the fly:
>> by directly modifying rte_eth_dev fields and without doing stop/start
>> for these devices.
>> As now ethdev uses rte_eth_fp_ops[] for fast-path functions, these
>> direct changes doesn't make expected effect.
>> Fix the problem by guarding fast-path functions changes with
>> rte_eth_dev_stop()/rte_eth_dev_start().
>>
>> Fixes: 7a0935239b9e ("ethdev: make fast-path functions to use new flat array")
>>
>> Reported-by: Lewei Yang <leweix.yang@intel.com>
>> Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> 
> Tested 'link_bonding_autotest', it succeed after this patch (failing before)
> 
> Tested-by: Ferruh Yigit <ferruh.yigit@intel.com>
> 

Applied to dpdk-next-net/main, thanks.

Re: [dpdk-dev] [PATCH] test/bonding: fix failures after hiding ethdev internal structures

[David Marchand]

Hello guys,

On Fri, Oct 22, 2021 at 3:27 PM Konstantin Ananyev
<konstantin.ananyev@intel.com> wrote:
>
> link bounding auto-test internally creates emulated ethdev.
> Some tests change Rx/Tx functions of this emulated device on the fly:
> by directly modifying rte_eth_dev fields and without doing stop/start
> for these devices.
> As now ethdev uses rte_eth_fp_ops[] for fast-path functions, these
> direct changes doesn't make expected effect.
> Fix the problem by guarding fast-path functions changes with
> rte_eth_dev_stop()/rte_eth_dev_start().
>
> Fixes: 7a0935239b9e ("ethdev: make fast-path functions to use new flat array")
>
> Reported-by: Lewei Yang <leweix.yang@intel.com>
> Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

I suspect there is something broken in
(bitratestats_autotest|latencystats_autotest|pdump_autotest) tests
that make use of app/test/sample_packet_forward.* code.
This code creates a portid from a ring, but I guess this port is not
properly setup wrt to API.

It goes unnoticed in CI atm, with tests returning ok.


Looking at main branch test in UNH, testlog.txt files are millions of
lines filled with:

test_packet_forward() line 104: Error sending packet to port 0
send pkts Failed

or:

tx_pkt_burst for not ready port

There may be more issues, the files are just too huge.


Example:
https://lab.dpdk.org/results/dashboard/results/results-uploads/test_runs/1dd418abe77644c18417dcb589e994aa/log_upload_file/2021/10/dpdk_2e437f1cd65e_2021-10-23_22-04-34_NA.zip




-- 
David Marchand

Re: [dpdk-dev] [PATCH] test/bonding: fix failures after hiding ethdev internal structures

[Ananyev, Konstantin]

Hi David,

> Hello guys,
> 
> On Fri, Oct 22, 2021 at 3:27 PM Konstantin Ananyev
> <konstantin.ananyev@intel.com> wrote:
> >
> > link bounding auto-test internally creates emulated ethdev.
> > Some tests change Rx/Tx functions of this emulated device on the fly:
> > by directly modifying rte_eth_dev fields and without doing stop/start
> > for these devices.
> > As now ethdev uses rte_eth_fp_ops[] for fast-path functions, these
> > direct changes doesn't make expected effect.
> > Fix the problem by guarding fast-path functions changes with
> > rte_eth_dev_stop()/rte_eth_dev_start().
> >
> > Fixes: 7a0935239b9e ("ethdev: make fast-path functions to use new flat array")
> >
> > Reported-by: Lewei Yang <leweix.yang@intel.com>
> > Signed-off-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> 
> I suspect there is something broken in
> (bitratestats_autotest|latencystats_autotest|pdump_autotest) tests
> that make use of app/test/sample_packet_forward.* code.
> This code creates a portid from a ring, but I guess this port is not
> properly setup wrt to API.
> It goes unnoticed in CI atm, with tests returning ok.
> 
> 
> Looking at main branch test in UNH, testlog.txt files are millions of
> lines filled with:
> 
> test_packet_forward() line 104: Error sending packet to port 0
> send pkts Failed
> 
> or:
> 
> tx_pkt_burst for not ready port
> 
> There may be more issues, the files are just too huge.
> 
> 
> Example:
> https://lab.dpdk.org/results/dashboard/results/results-
> uploads/test_runs/1dd418abe77644c18417dcb589e994aa/log_upload_file/2021/10/dpdk_2e437f1cd65e_2021-10-23_22-04-34_NA.zip
> 

Sure, I'll have a look next week.
Konstantin