diff mbox series

[v2] app/testpmd: fix to add offloads confguration for queue

Message ID 1562218537-33583-1-git-send-email-wei.zhao1@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers
Series [v2] app/testpmd: fix to add offloads confguration for queue |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/intel-Performance-Testing success Performance Testing PASS
ci/mellanox-Performance-Testing success Performance Testing PASS
ci/Intel-compilation fail apply issues

Commit Message

Zhao1, Wei July 4, 2019, 5:35 a.m. UTC 
  When adding offloads from commandline, not only port
related configuration bits should be set, but also queue
related offloads configuration bits, or it will cause error.
For example, test in this process for ixgbe:
(1)./x86_64-native-linuxapp-gcc/app/testpmd -c 0x6 -n 4
-- -i --portmask=0x1 --port-topology=loop --disable-crc-strip
(2)port stop all
(3)port config all crc-strip on
(4)port start all
we will see "Fail to configure port 0 rx queues" of warning info.

Fixes: 0074d02fca21 ("app/testpmd: convert to new Rx offloads API")
Cc: stable@dpdk.org

Signed-off-by: wei zhao <wei.zhao1@intel.com>

---

v2:

merge to one function for apply offloads configguration
---
 app/test-pmd/cmdline.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

Comments

Iremonger, Bernard July 4, 2019, 8:53 a.m. UTC | #1 
> -----Original Message-----
> From: Zhao1, Wei
> Sent: Thursday, July 4, 2019 6:36 AM
> To: dev@dpdk.org
> Cc: stable@dpdk.org; Iremonger, Bernard <bernard.iremonger@intel.com>;
> Zhao1, Wei <wei.zhao1@intel.com>
> Subject: [PATCH v2] app/testpmd: fix to add offloads confguration for queue
> 
> When adding offloads from commandline, not only port related configuration
> bits should be set, but also queue related offloads configuration bits, or it will
> cause error.
> For example, test in this process for ixgbe:
> (1)./x86_64-native-linuxapp-gcc/app/testpmd -c 0x6 -n 4
> -- -i --portmask=0x1 --port-topology=loop --disable-crc-strip (2)port stop all
> (3)port config all crc-strip on (4)port start all we will see "Fail to configure port
> 0 rx queues" of warning info.
> 
> Fixes: 0074d02fca21 ("app/testpmd: convert to new Rx offloads API")
> Cc: stable@dpdk.org
> 
> Signed-off-by: wei zhao <wei.zhao1@intel.com>

Acked-by: Bernard Iremonger <bernard.iremonger@intel.com>
Ferruh Yigit July 4, 2019, 11:54 p.m. UTC | #2 
On 7/4/2019 9:53 AM, Iremonger, Bernard wrote:
>> -----Original Message-----
>> From: Zhao1, Wei
>> Sent: Thursday, July 4, 2019 6:36 AM
>> To: dev@dpdk.org
>> Cc: stable@dpdk.org; Iremonger, Bernard <bernard.iremonger@intel.com>;
>> Zhao1, Wei <wei.zhao1@intel.com>
>> Subject: [PATCH v2] app/testpmd: fix to add offloads confguration for queue
>>
>> When adding offloads from commandline, not only port related configuration
>> bits should be set, but also queue related offloads configuration bits, or it will
>> cause error.
>> For example, test in this process for ixgbe:
>> (1)./x86_64-native-linuxapp-gcc/app/testpmd -c 0x6 -n 4
>> -- -i --portmask=0x1 --port-topology=loop --disable-crc-strip (2)port stop all
>> (3)port config all crc-strip on (4)port start all we will see "Fail to configure port
>> 0 rx queues" of warning info.
>>
>> Fixes: 0074d02fca21 ("app/testpmd: convert to new Rx offloads API")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: wei zhao <wei.zhao1@intel.com>
> 
> Acked-by: Bernard Iremonger <bernard.iremonger@intel.com>
> 

Applied to dpdk-next-net/master, thanks.
Thomas Monjalon July 6, 2019, 4:24 p.m. UTC | #3 
04/07/2019 07:35, Wei Zhao:
> When adding offloads from commandline, not only port
> related configuration bits should be set, but also queue
> related offloads configuration bits, or it will cause error.
> For example, test in this process for ixgbe:
> (1)./x86_64-native-linuxapp-gcc/app/testpmd -c 0x6 -n 4
> -- -i --portmask=0x1 --port-topology=loop --disable-crc-strip
> (2)port stop all
> (3)port config all crc-strip on
> (4)port start all
> we will see "Fail to configure port 0 rx queues" of warning info.

I'm really surprised it was so much broken.
I did not check the code. Someone else did?
Thomas Monjalon July 6, 2019, 4:25 p.m. UTC | #4 
06/07/2019 18:24, Thomas Monjalon:
> 04/07/2019 07:35, Wei Zhao:
> > When adding offloads from commandline, not only port
> > related configuration bits should be set, but also queue
> > related offloads configuration bits, or it will cause error.
> > For example, test in this process for ixgbe:
> > (1)./x86_64-native-linuxapp-gcc/app/testpmd -c 0x6 -n 4
> > -- -i --portmask=0x1 --port-topology=loop --disable-crc-strip
> > (2)port stop all
> > (3)port config all crc-strip on
> > (4)port start all
> > we will see "Fail to configure port 0 rx queues" of warning info.
> 
> I'm really surprised it was so much broken.
> I did not check the code. Someone else did?

Adding more Cc for double check.
David Marchand July 8, 2019, 9:09 a.m. UTC | #5 
Hello Wei,

On Sat, Jul 6, 2019 at 6:26 PM Thomas Monjalon <thomas@monjalon.net> wrote:

> 06/07/2019 18:24, Thomas Monjalon:
> > 04/07/2019 07:35, Wei Zhao:
> > > When adding offloads from commandline, not only port
> > > related configuration bits should be set, but also queue
> > > related offloads configuration bits, or it will cause error.
> > > For example, test in this process for ixgbe:
> > > (1)./x86_64-native-linuxapp-gcc/app/testpmd -c 0x6 -n 4
> > > -- -i --portmask=0x1 --port-topology=loop --disable-crc-strip
> > > (2)port stop all
> > > (3)port config all crc-strip on
> > > (4)port start all
> > > we will see "Fail to configure port 0 rx queues" of warning info.
> >
> > I'm really surprised it was so much broken.
> > I did not check the code. Someone else did?
>
> Adding more Cc for double check.
>
>
Did not check the code yet, but the Fixes: line is about RX offloads, and
you touched both rx and tx offloads.
The incriminated commit comes from 18.02.

I can't reproduce your issue with ixgbe ports.

In 18.11, testpmd starts fine and accepts the configuration change.
In master (before this change), I can see a different issue which has to do
with global offloads vs per queue offloads:

# ./master/app/testpmd -w 0000:03:00.0 -w 0000:03:00.1 -- -i
--disable-crc-strip
EAL: Detected 28 lcore(s)
EAL: Detected 1 NUMA nodes
EAL: Multi-process socket /var/run/dpdk/rte/mp_socket
EAL: Selected IOVA mode 'PA'
EAL: Probing VFIO support...
EAL: VFIO support initialized
EAL: PCI device 0000:03:00.0 on NUMA socket 0
EAL:   probe driver: 8086:154d net_ixgbe
EAL:   using IOMMU type 1 (Type 1)
EAL: Ignore mapping IO port bar(2)
EAL: PCI device 0000:03:00.1 on NUMA socket 0
EAL:   probe driver: 8086:154d net_ixgbe
EAL: Ignore mapping IO port bar(2)
Interactive-mode selected
testpmd: create a new mbuf pool <mbuf_pool_socket_0>: n=155456, size=2176,
socket=0
testpmd: preferred mempool ops selected: ring_mp_mc
Configuring Port 0 (socket 0)
Port 0: B4:96:91:1B:67:10
Configuring Port 1 (socket 0)
Port 1: B4:96:91:1B:67:12
Checking link statuses...
Done
testpmd> port stop all
Stopping ports...
Checking link statuses...
Done
testpmd> port config all crc-strip on
testpmd> port start all
Configuring Port 0 (socket 0)
Ethdev port_id=0 rx_queue_id=0, new added offloads 0x10000 must be within
per-queue offload capabilities 0x1 in rte_eth_rx_queue_setup()
Fail to configure port 0 rx queues

Can you describe your setup, like which driver is used, and the full traces
of testpmd ?

Thanks.
Zhao1, Wei July 8, 2019, 9:16 a.m. UTC | #6 
Hi,  Marchand

  That is my list for this issue.
Yes, I have also fix tx related code, but this test case is for rx offloads.



Environment
·         DPDK version:
19.05.0-rc4
·         NIC hardware: i40e, ixgbe
Test Setup

export RTE_TARGET=x86_64-native-linuxapp-gcc
export RTE_SDK=`pwd`
make -j 62 install T=x86_64-native-linuxapp-gcc
modprobe uio
rmmod -f igb_uio
insmod ./x86_64-native-linuxapp-gcc/kmod/igb_uio.ko
usertools/dpdk-devbind.py --force --bind=igb_uio 0000:03:00.0 0000:03:00.1
On DUT:
./x86_64-native-linuxapp-gcc/app/testpmd -c 0x6 -n 4  -- -i --portmask=0x1 --port-topology=loop --disable-crc-strip
port stop all
port config all crc-strip on
port start all


The output is:
Configuring Port 0 (socket 0)
Ethdev port_id=0 rx_queue_id=0, new added offloads 0x10000 must be within per-queue offload capabilities 0x1 in rte_eth_rx_queue_setup()
Fail to configure port 0 rx queues



The first bad commit id:
5e91aeef218c452c370aacf74265c7a42b67dffa


From: David Marchand [mailto:david.marchand@redhat.com]
Sent: Monday, July 8, 2019 5:09 PM
To: Zhao1, Wei <wei.zhao1@intel.com>
Cc: dev <dev@dpdk.org>; dpdk stable <stable@dpdk.org>; Iremonger, Bernard <bernard.iremonger@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>; Andrew Rybchenko <arybchenko@solarflare.com>; Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ananyev, Konstantin <konstantin.ananyev@intel.com>; Olivier Matz <olivier.matz@6wind.com>; Adrien Mazarguil <adrien.mazarguil@6wind.com>; Thomas Monjalon <thomas@monjalon.net>
Subject: Re: [dpdk-stable] [PATCH v2] app/testpmd: fix to add offloads confguration for queue

Hello Wei,

On Sat, Jul 6, 2019 at 6:26 PM Thomas Monjalon <thomas@monjalon.net<mailto:thomas@monjalon.net>> wrote:
06/07/2019 18:24, Thomas Monjalon:
> 04/07/2019 07:35, Wei Zhao:
> > When adding offloads from commandline, not only port
> > related configuration bits should be set, but also queue
> > related offloads configuration bits, or it will cause error.
> > For example, test in this process for ixgbe:
> > (1)./x86_64-native-linuxapp-gcc/app/testpmd -c 0x6 -n 4
> > -- -i --portmask=0x1 --port-topology=loop --disable-crc-strip
> > (2)port stop all
> > (3)port config all crc-strip on
> > (4)port start all
> > we will see "Fail to configure port 0 rx queues" of warning info.
>
> I'm really surprised it was so much broken.
> I did not check the code. Someone else did?

Adding more Cc for double check.

Did not check the code yet, but the Fixes: line is about RX offloads, and you touched both rx and tx offloads.
The incriminated commit comes from 18.02.

I can't reproduce your issue with ixgbe ports.

In 18.11, testpmd starts fine and accepts the configuration change.
In master (before this change), I can see a different issue which has to do with global offloads vs per queue offloads:

# ./master/app/testpmd -w 0000:03:00.0 -w 0000:03:00.1 -- -i --disable-crc-strip
EAL: Detected 28 lcore(s)
EAL: Detected 1 NUMA nodes
EAL: Multi-process socket /var/run/dpdk/rte/mp_socket
EAL: Selected IOVA mode 'PA'
EAL: Probing VFIO support...
EAL: VFIO support initialized
EAL: PCI device 0000:03:00.0 on NUMA socket 0
EAL:   probe driver: 8086:154d net_ixgbe
EAL:   using IOMMU type 1 (Type 1)
EAL: Ignore mapping IO port bar(2)
EAL: PCI device 0000:03:00.1 on NUMA socket 0
EAL:   probe driver: 8086:154d net_ixgbe
EAL: Ignore mapping IO port bar(2)
Interactive-mode selected
testpmd: create a new mbuf pool <mbuf_pool_socket_0>: n=155456, size=2176, socket=0
testpmd: preferred mempool ops selected: ring_mp_mc
Configuring Port 0 (socket 0)
Port 0: B4:96:91:1B:67:10
Configuring Port 1 (socket 0)
Port 1: B4:96:91:1B:67:12
Checking link statuses...
Done
testpmd> port stop all
Stopping ports...
Checking link statuses...
Done
testpmd> port config all crc-strip on
testpmd> port start all
Configuring Port 0 (socket 0)
Ethdev port_id=0 rx_queue_id=0, new added offloads 0x10000 must be within per-queue offload capabilities 0x1 in rte_eth_rx_queue_setup()
Fail to configure port 0 rx queues

Can you describe your setup, like which driver is used, and the full traces of testpmd ?

Thanks.


--
David Marchand
Zhao1, Wei July 8, 2019, 9:22 a.m. UTC | #7 
Hi,  marchand

From: David Marchand [mailto:david.marchand@redhat.com]
Sent: Monday, July 8, 2019 5:09 PM
To: Zhao1, Wei <wei.zhao1@intel.com>
Cc: dev <dev@dpdk.org>; dpdk stable <stable@dpdk.org>; Iremonger, Bernard <bernard.iremonger@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>; Andrew Rybchenko <arybchenko@solarflare.com>; Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ananyev, Konstantin <konstantin.ananyev@intel.com>; Olivier Matz <olivier.matz@6wind.com>; Adrien Mazarguil <adrien.mazarguil@6wind.com>; Thomas Monjalon <thomas@monjalon.net>
Subject: Re: [dpdk-stable] [PATCH v2] app/testpmd: fix to add offloads confguration for queue

Hello Wei,

On Sat, Jul 6, 2019 at 6:26 PM Thomas Monjalon <thomas@monjalon.net<mailto:thomas@monjalon.net>> wrote:
06/07/2019 18:24, Thomas Monjalon:
> 04/07/2019 07:35, Wei Zhao:
> > When adding offloads from commandline, not only port
> > related configuration bits should be set, but also queue
> > related offloads configuration bits, or it will cause error.
> > For example, test in this process for ixgbe:
> > (1)./x86_64-native-linuxapp-gcc/app/testpmd -c 0x6 -n 4
> > -- -i --portmask=0x1 --port-topology=loop --disable-crc-strip
> > (2)port stop all
> > (3)port config all crc-strip on
> > (4)port start all
> > we will see "Fail to configure port 0 rx queues" of warning info.
>
> I'm really surprised it was so much broken.
> I did not check the code. Someone else did?

Adding more Cc for double check.

Did not check the code yet, but the Fixes: line is about RX offloads, and you touched both rx and tx offloads.
The incriminated commit comes from 18.02.

I can't reproduce your issue with ixgbe ports.

In 18.11, testpmd starts fine and accepts the configuration change.
In master (before this change), I can see a different issue which has to do with global offloads vs per queue offloads:


Yes,  my patch is just for this per queue offloads!

# ./master/app/testpmd -w 0000:03:00.0 -w 0000:03:00.1 -- -i --disable-crc-strip
EAL: Detected 28 lcore(s)
EAL: Detected 1 NUMA nodes
EAL: Multi-process socket /var/run/dpdk/rte/mp_socket
EAL: Selected IOVA mode 'PA'
EAL: Probing VFIO support...
EAL: VFIO support initialized
EAL: PCI device 0000:03:00.0 on NUMA socket 0
EAL:   probe driver: 8086:154d net_ixgbe
EAL:   using IOMMU type 1 (Type 1)
EAL: Ignore mapping IO port bar(2)
EAL: PCI device 0000:03:00.1 on NUMA socket 0
EAL:   probe driver: 8086:154d net_ixgbe
EAL: Ignore mapping IO port bar(2)
Interactive-mode selected
testpmd: create a new mbuf pool <mbuf_pool_socket_0>: n=155456, size=2176, socket=0
testpmd: preferred mempool ops selected: ring_mp_mc
Configuring Port 0 (socket 0)
Port 0: B4:96:91:1B:67:10
Configuring Port 1 (socket 0)
Port 1: B4:96:91:1B:67:12
Checking link statuses...
Done
testpmd> port stop all
Stopping ports...
Checking link statuses...
Done
testpmd> port config all crc-strip on
testpmd> port start all
Configuring Port 0 (socket 0)
Ethdev port_id=0 rx_queue_id=0, new added offloads 0x10000 must be within per-queue offload capabilities 0x1 in rte_eth_rx_queue_setup()
Fail to configure port 0 rx queues

“Fail to configure port 0 rx queues”
You are right, this is just the error info, the same to me.



Can you describe your setup, like which driver is used, and the full traces of testpmd ?

Thanks.


--
David Marchand
diff mbox series

Patch

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index d1e0d44..6a69f90 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -2047,6 +2047,7 @@  cmd_config_rx_mode_flag_parsed(void *parsed_result,
 {
 	struct cmd_config_rx_mode_flag *res = parsed_result;
 	portid_t pid;
+	int k;
 
 	if (!all_ports_stopped()) {
 		printf("Please stop all ports first\n");
@@ -2147,6 +2148,10 @@  cmd_config_rx_mode_flag_parsed(void *parsed_result,
 			return;
 		}
 		port->dev_conf.rxmode.offloads = rx_offloads;
+		/* Apply Rx offloads configuration */
+		for (k = 0; k < port->dev_info.max_rx_queues; k++)
+			port->rx_conf[k].offloads =
+				port->dev_conf.rxmode.offloads;
 	}
 
 	init_port_config();
@@ -4360,6 +4365,17 @@  csum_show(int port_id)
 }
 
 static void
+cmd_config_queue_tx_offloads(struct rte_port *port)
+{
+	int k;
+
+	/* Apply queue tx offloads configuration */
+	for (k = 0; k < port->dev_info.max_rx_queues; k++)
+		port->tx_conf[k].offloads =
+			port->dev_conf.txmode.offloads;
+}
+
+static void
 cmd_csum_parsed(void *parsed_result,
 		       __attribute__((unused)) struct cmdline *cl,
 		       __attribute__((unused)) void *data)
@@ -4443,6 +4459,7 @@  cmd_csum_parsed(void *parsed_result,
 			ports[res->port_id].dev_conf.txmode.offloads &=
 							(~csum_offloads);
 		}
+		cmd_config_queue_tx_offloads(&ports[res->port_id]);
 	}
 	csum_show(res->port_id);
 
@@ -4594,6 +4611,7 @@  cmd_tso_set_parsed(void *parsed_result,
 		printf("TSO segment size for non-tunneled packets is %d\n",
 			ports[res->port_id].tso_segsz);
 	}
+	cmd_config_queue_tx_offloads(&ports[res->port_id]);
 
 	/* display warnings if configuration is not supported by the NIC */
 	rte_eth_dev_info_get(res->port_id, &dev_info);
@@ -4749,6 +4767,7 @@  cmd_tunnel_tso_set_parsed(void *parsed_result,
 				"if outer L3 is IPv4; not necessary for IPv6\n");
 	}
 
+	cmd_config_queue_tx_offloads(&ports[res->port_id]);
 	cmd_reconfig_device_queue(res->port_id, 1, 1);
 }