test/bonding: fix failures after hiding ethdev internal structures
Checks
Commit Message
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(-)
Comments
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>
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
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.
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
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
@@ -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;
}