patches for DPDK stable branches
 help / color / mirror / Atom feed
* [dpdk-stable] [PATCH v5] app/testpmd: fix setting maximum packet length
       [not found] <20210125083202.38267-1-stevex.yang@intel.com>
@ 2021-01-25 18:15 ` Ferruh Yigit
  2021-01-25 19:41   ` Lance Richardson
                     ` (4 more replies)
  0 siblings, 5 replies; 17+ messages in thread
From: Ferruh Yigit @ 2021-01-25 18:15 UTC (permalink / raw)
  To: Wenzhuo Lu, Xiaoyun Li, Bernard Iremonger, Steve Yang
  Cc: Ferruh Yigit, dev, stable, lance.richardson, oulijun, wisamm, lihuisong

From: Steve Yang <stevex.yang@intel.com>

"port config all max-pkt-len" command fails because it doesn't set the
'DEV_RX_OFFLOAD_JUMBO_FRAME' offload flag properly.

Commit in the fixes line moved the 'DEV_RX_OFFLOAD_JUMBO_FRAME' offload
flag update from 'cmd_config_max_pkt_len_parsed()' to 'init_config()'.
'init_config()' function is only called during testpmd startup, but the
flag status needs to be calculated whenever 'max_rx_pkt_len' changes.

The issue can be reproduce as [1], where the 'max-pkt-len' reduced and
'DEV_RX_OFFLOAD_JUMBO_FRAME' offload flag should be cleared but it
didn't.

Adding the 'update_jumbo_frame_offload()' helper function to update
'DEV_RX_OFFLOAD_JUMBO_FRAME' offload flag and 'max_rx_pkt_len'. This
function is called both by 'init_config()' and
'cmd_config_max_pkt_len_parsed()'.

Default 'max-pkt-len' value set to zero, 'update_jumbo_frame_offload()'
updates it to "RTE_ETHER_MTU + PMD specific Ethernet overhead" when it
is zero.
If '--max-pkt-len=N' argument provided, it will be used instead.
And with each "port config all max-pkt-len" command, the
'DEV_RX_OFFLOAD_JUMBO_FRAME' offload flag, 'max-pkt-len' and MTU is
updated.

[1]
--------------------------------------------------------------------------
dpdk-testpmd -c 0xf -n 4 -- -i --max-pkt-len=9000 --tx-offloads=0x8000
	--rxq=4 --txq=4 --disable-rss
testpmd>  set verbose 3
testpmd>  port stop all
testpmd>  port config all max-pkt-len 1518
testpmd>  port start all

// Got fail error info without this patch
Configuring Port 0 (socket 1)
Ethdev port_id=0 rx_queue_id=0, new added offloads 0x800 must be
within per-queue offload capabilities 0x0 in rte_eth_rx_queue_setup()
Fail to configure port 0 rx queues //<-- Fail error info;
--------------------------------------------------------------------------

Fixes: 761c4d66900f ("app/testpmd: fix max Rx packet length for VLAN packets")
Cc: stable@dpdk.org

Signed-off-by: Steve Yang <stevex.yang@intel.com>
Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---

v5:
* 'update_jumbo_frame_offload()' helper updated
  * check zero 'max-pkt-len' value
  * Update how queue offload flags updated
  * Update MTU if JUMBO_FRAME flag is not set
* Default testpmd 'max-pkt-len' value set to zero

Cc: lance.richardson@broadcom.com
Cc: oulijun@huawei.com
Cc: wisamm@mellanox.com
Cc: lihuisong@huawei.com
---
 app/test-pmd/cmdline.c |  13 ++++++
 app/test-pmd/testpmd.c | 102 +++++++++++++++++++++++++++++++++--------
 app/test-pmd/testpmd.h |   1 +
 3 files changed, 97 insertions(+), 19 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 89034c8b7272..9ada4316c6c0 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -1877,7 +1877,9 @@ cmd_config_max_pkt_len_parsed(void *parsed_result,
 				__rte_unused void *data)
 {
 	struct cmd_config_max_pkt_len_result *res = parsed_result;
+	uint32_t max_rx_pkt_len_backup = 0;
 	portid_t pid;
+	int ret;
 
 	if (!all_ports_stopped()) {
 		printf("Please stop all ports first\n");
@@ -1896,7 +1898,18 @@ cmd_config_max_pkt_len_parsed(void *parsed_result,
 			if (res->value == port->dev_conf.rxmode.max_rx_pkt_len)
 				return;
 
+			ret = eth_dev_info_get_print_err(pid, &port->dev_info);
+			if (ret != 0) {
+				printf("rte_eth_dev_info_get() failed for port %u\n",
+					pid);
+				return;
+			}
+
+			max_rx_pkt_len_backup = port->dev_conf.rxmode.max_rx_pkt_len;
+
 			port->dev_conf.rxmode.max_rx_pkt_len = res->value;
+			if (update_jumbo_frame_offload(pid) != 0)
+				port->dev_conf.rxmode.max_rx_pkt_len = max_rx_pkt_len_backup;
 		} else {
 			printf("Unknown parameter\n");
 			return;
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index c256e719aea2..b69fcd3fde72 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -443,8 +443,11 @@ lcoreid_t latencystats_lcore_id = -1;
  * Ethernet device configuration.
  */
 struct rte_eth_rxmode rx_mode = {
-	.max_rx_pkt_len = RTE_ETHER_MAX_LEN,
-		/**< Default maximum frame length. */
+	/* Default maximum frame length.
+	 * Zero is converted to "RTE_ETHER_MTU + PMD Ethernet overhead"
+	 * in init_config().
+	 */
+	.max_rx_pkt_len = 0,
 };
 
 struct rte_eth_txmode tx_mode = {
@@ -1410,7 +1413,6 @@ init_config(void)
 	struct rte_gro_param gro_param;
 	uint32_t gso_types;
 	uint16_t data_size;
-	uint16_t eth_overhead;
 	bool warning = 0;
 	int k;
 	int ret;
@@ -1447,22 +1449,10 @@ init_config(void)
 			rte_exit(EXIT_FAILURE,
 				 "rte_eth_dev_info_get() failed\n");
 
-		/* Update the max_rx_pkt_len to have MTU as RTE_ETHER_MTU */
-		if (port->dev_info.max_mtu != UINT16_MAX &&
-		    port->dev_info.max_rx_pktlen > port->dev_info.max_mtu)
-			eth_overhead = port->dev_info.max_rx_pktlen -
-				port->dev_info.max_mtu;
-		else
-			eth_overhead =
-				RTE_ETHER_HDR_LEN + RTE_ETHER_CRC_LEN;
-
-		if (port->dev_conf.rxmode.max_rx_pkt_len <=
-			(uint32_t)(RTE_ETHER_MTU + eth_overhead))
-			port->dev_conf.rxmode.max_rx_pkt_len =
-					RTE_ETHER_MTU + eth_overhead;
-		else
-			port->dev_conf.rxmode.offloads |=
-					DEV_RX_OFFLOAD_JUMBO_FRAME;
+		ret = update_jumbo_frame_offload(pid);
+		if (ret != 0)
+			printf("Updating jumbo frame offload failed for port %u\n",
+				pid);
 
 		if (!(port->dev_info.tx_offload_capa &
 		      DEV_TX_OFFLOAD_MBUF_FAST_FREE))
@@ -3358,6 +3348,80 @@ rxtx_port_config(struct rte_port *port)
 	}
 }
 
+/*
+ * Helper function to arrange max_rx_pktlen value and JUMBO_FRAME offload,
+ * MTU is also aligned if JUMBO_FRAME offload is not set.
+ *
+ * port->dev_info should be get before calling this function.
+ *
+ * return 0 on success, negative on error
+ */
+int
+update_jumbo_frame_offload(portid_t portid)
+{
+	struct rte_port *port = &ports[portid];
+	uint32_t eth_overhead;
+	uint64_t rx_offloads;
+	int ret;
+	bool on;
+
+	/* Update the max_rx_pkt_len to have MTU as RTE_ETHER_MTU */
+	if (port->dev_info.max_mtu != UINT16_MAX &&
+	    port->dev_info.max_rx_pktlen > port->dev_info.max_mtu)
+		eth_overhead = port->dev_info.max_rx_pktlen -
+				port->dev_info.max_mtu;
+	else
+		eth_overhead = RTE_ETHER_HDR_LEN + RTE_ETHER_CRC_LEN;
+
+	rx_offloads = port->dev_conf.rxmode.offloads;
+
+	/* Default config value is 0 to use PMD specific overhead */
+	if (port->dev_conf.rxmode.max_rx_pkt_len == 0)
+		port->dev_conf.rxmode.max_rx_pkt_len = RTE_ETHER_MTU + eth_overhead;
+
+	if (port->dev_conf.rxmode.max_rx_pkt_len <= RTE_ETHER_MTU + eth_overhead) {
+		rx_offloads &= ~DEV_RX_OFFLOAD_JUMBO_FRAME;
+		on = false;
+	} else {
+		if ((port->dev_info.rx_offload_capa & DEV_RX_OFFLOAD_JUMBO_FRAME) == 0) {
+			printf("Frame size (%u) is not supported by port %u\n",
+				port->dev_conf.rxmode.max_rx_pkt_len,
+				portid);
+			return -1;
+		}
+		rx_offloads |= DEV_RX_OFFLOAD_JUMBO_FRAME;
+		on = true;
+	}
+
+	if (rx_offloads != port->dev_conf.rxmode.offloads) {
+		uint16_t qid;
+
+		port->dev_conf.rxmode.offloads = rx_offloads;
+
+		/* Apply JUMBO_FRAME offload configuration to Rx queue(s) */
+		for (qid = 0; qid < port->dev_info.nb_rx_queues; qid++) {
+			if (on)
+				port->rx_conf[qid].offloads |= DEV_RX_OFFLOAD_JUMBO_FRAME;
+			else
+				port->rx_conf[qid].offloads &= ~DEV_RX_OFFLOAD_JUMBO_FRAME;
+		}
+	}
+
+	/* If JUMBO_FRAME is set MTU conversion done by ethdev layer,
+	 * if unset do it here
+	 */
+	if ((rx_offloads & DEV_RX_OFFLOAD_JUMBO_FRAME) == 0) {
+		ret = rte_eth_dev_set_mtu(portid,
+				port->dev_conf.rxmode.max_rx_pkt_len - eth_overhead);
+		if (ret)
+			printf("Failed to set MTU to %u for port %u\n",
+				port->dev_conf.rxmode.max_rx_pkt_len - eth_overhead,
+				portid);
+	}
+
+	return 0;
+}
+
 void
 init_port_config(void)
 {
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 5f2316210726..2f8f5a92e46a 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -1005,6 +1005,7 @@ uint16_t tx_pkt_set_dynf(uint16_t port_id, __rte_unused uint16_t queue,
 			 __rte_unused void *user_param);
 void add_tx_dynf_callback(portid_t portid);
 void remove_tx_dynf_callback(portid_t portid);
+int update_jumbo_frame_offload(portid_t portid);
 
 /*
  * Work-around of a compilation error with ICC on invocations of the
-- 
2.29.2


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-stable] [PATCH v5] app/testpmd: fix setting maximum packet length
  2021-01-25 18:15 ` [dpdk-stable] [PATCH v5] app/testpmd: fix setting maximum packet length Ferruh Yigit
@ 2021-01-25 19:41   ` Lance Richardson
  2021-01-26  0:44     ` Ferruh Yigit
  2021-01-26  9:02   ` Wisam Monther
                     ` (3 subsequent siblings)
  4 siblings, 1 reply; 17+ messages in thread
From: Lance Richardson @ 2021-01-25 19:41 UTC (permalink / raw)
  To: Ferruh Yigit
  Cc: Wenzhuo Lu, Xiaoyun Li, Bernard Iremonger, Steve Yang, dev,
	stable, oulijun, wisamm, lihuisong

[-- Attachment #1: Type: text/plain, Size: 3216 bytes --]

On Mon, Jan 25, 2021 at 1:15 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>
> From: Steve Yang <stevex.yang@intel.com>
>
> "port config all max-pkt-len" command fails because it doesn't set the
> 'DEV_RX_OFFLOAD_JUMBO_FRAME' offload flag properly.
>
> Commit in the fixes line moved the 'DEV_RX_OFFLOAD_JUMBO_FRAME' offload
> flag update from 'cmd_config_max_pkt_len_parsed()' to 'init_config()'.
> 'init_config()' function is only called during testpmd startup, but the
> flag status needs to be calculated whenever 'max_rx_pkt_len' changes.
>
> The issue can be reproduce as [1], where the 'max-pkt-len' reduced and
> 'DEV_RX_OFFLOAD_JUMBO_FRAME' offload flag should be cleared but it
> didn't.
>
> Adding the 'update_jumbo_frame_offload()' helper function to update
> 'DEV_RX_OFFLOAD_JUMBO_FRAME' offload flag and 'max_rx_pkt_len'. This
> function is called both by 'init_config()' and
> 'cmd_config_max_pkt_len_parsed()'.
>
> Default 'max-pkt-len' value set to zero, 'update_jumbo_frame_offload()'
> updates it to "RTE_ETHER_MTU + PMD specific Ethernet overhead" when it
> is zero.
> If '--max-pkt-len=N' argument provided, it will be used instead.
> And with each "port config all max-pkt-len" command, the
> 'DEV_RX_OFFLOAD_JUMBO_FRAME' offload flag, 'max-pkt-len' and MTU is
> updated.
>
> [1]

<snip>

> +/*
> + * Helper function to arrange max_rx_pktlen value and JUMBO_FRAME offload,
> + * MTU is also aligned if JUMBO_FRAME offload is not set.
> + *
> + * port->dev_info should be get before calling this function.

Should this be "port->dev_info should be set ..." instead?


<snip>

> +       if (rx_offloads != port->dev_conf.rxmode.offloads) {
> +               uint16_t qid;
> +
> +               port->dev_conf.rxmode.offloads = rx_offloads;
> +
> +               /* Apply JUMBO_FRAME offload configuration to Rx queue(s) */
> +               for (qid = 0; qid < port->dev_info.nb_rx_queues; qid++) {
> +                       if (on)
> +                               port->rx_conf[qid].offloads |= DEV_RX_OFFLOAD_JUMBO_FRAME;
> +                       else
> +                               port->rx_conf[qid].offloads &= ~DEV_RX_OFFLOAD_JUMBO_FRAME;
> +               }

Is it correct to set per-queue offloads that aren't advertised by the PMD
as supported in rx_queue_offload_capa?

> +       }
> +
> +       /* If JUMBO_FRAME is set MTU conversion done by ethdev layer,
> +        * if unset do it here
> +        */
> +       if ((rx_offloads & DEV_RX_OFFLOAD_JUMBO_FRAME) == 0) {
> +               ret = rte_eth_dev_set_mtu(portid,
> +                               port->dev_conf.rxmode.max_rx_pkt_len - eth_overhead);
> +               if (ret)
> +                       printf("Failed to set MTU to %u for port %u\n",
> +                               port->dev_conf.rxmode.max_rx_pkt_len - eth_overhead,
> +                               portid);
> +       }
> +
> +       return 0;
> +}
> +

Applied and tested with a few iterations of configuring max packet size
back and forth between jumbo and non-jumbo sizes, also tried setting
max packet size using the command-line option, all seems good based
on rx offloads and packet forwarding.

Two minor questions above, otherwise LGTM.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-stable] [PATCH v5] app/testpmd: fix setting maximum packet length
  2021-01-25 19:41   ` Lance Richardson
@ 2021-01-26  0:44     ` Ferruh Yigit
  2021-01-26  3:22       ` Lance Richardson
  2021-01-26  3:45       ` Lance Richardson
  0 siblings, 2 replies; 17+ messages in thread
From: Ferruh Yigit @ 2021-01-26  0:44 UTC (permalink / raw)
  To: Lance Richardson
  Cc: Wenzhuo Lu, Xiaoyun Li, Bernard Iremonger, Steve Yang, dev,
	stable, oulijun, wisamm, lihuisong

On 1/25/2021 7:41 PM, Lance Richardson wrote:
> On Mon, Jan 25, 2021 at 1:15 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>>
>> From: Steve Yang <stevex.yang@intel.com>
>>
>> "port config all max-pkt-len" command fails because it doesn't set the
>> 'DEV_RX_OFFLOAD_JUMBO_FRAME' offload flag properly.
>>
>> Commit in the fixes line moved the 'DEV_RX_OFFLOAD_JUMBO_FRAME' offload
>> flag update from 'cmd_config_max_pkt_len_parsed()' to 'init_config()'.
>> 'init_config()' function is only called during testpmd startup, but the
>> flag status needs to be calculated whenever 'max_rx_pkt_len' changes.
>>
>> The issue can be reproduce as [1], where the 'max-pkt-len' reduced and
>> 'DEV_RX_OFFLOAD_JUMBO_FRAME' offload flag should be cleared but it
>> didn't.
>>
>> Adding the 'update_jumbo_frame_offload()' helper function to update
>> 'DEV_RX_OFFLOAD_JUMBO_FRAME' offload flag and 'max_rx_pkt_len'. This
>> function is called both by 'init_config()' and
>> 'cmd_config_max_pkt_len_parsed()'.
>>
>> Default 'max-pkt-len' value set to zero, 'update_jumbo_frame_offload()'
>> updates it to "RTE_ETHER_MTU + PMD specific Ethernet overhead" when it
>> is zero.
>> If '--max-pkt-len=N' argument provided, it will be used instead.
>> And with each "port config all max-pkt-len" command, the
>> 'DEV_RX_OFFLOAD_JUMBO_FRAME' offload flag, 'max-pkt-len' and MTU is
>> updated.
>>
>> [1]
> 
> <snip>
> 
>> +/*
>> + * Helper function to arrange max_rx_pktlen value and JUMBO_FRAME offload,
>> + * MTU is also aligned if JUMBO_FRAME offload is not set.
>> + *
>> + * port->dev_info should be get before calling this function.
> 
> Should this be "port->dev_info should be set ..." instead?
> 

Ack.

> 
> <snip>
> 
>> +       if (rx_offloads != port->dev_conf.rxmode.offloads) {
>> +               uint16_t qid;
>> +
>> +               port->dev_conf.rxmode.offloads = rx_offloads;
>> +
>> +               /* Apply JUMBO_FRAME offload configuration to Rx queue(s) */
>> +               for (qid = 0; qid < port->dev_info.nb_rx_queues; qid++) {
>> +                       if (on)
>> +                               port->rx_conf[qid].offloads |= DEV_RX_OFFLOAD_JUMBO_FRAME;
>> +                       else
>> +                               port->rx_conf[qid].offloads &= ~DEV_RX_OFFLOAD_JUMBO_FRAME;
>> +               }
> 
> Is it correct to set per-queue offloads that aren't advertised by the PMD
> as supported in rx_queue_offload_capa?
> 

'port->rx_conf[]' is testpmd struct, and 'port->dev_conf.rxmode.offloads' values 
are reflected to 'port->rx_conf[].offloads' for all queues.

We should set the offload in 'port->rx_conf[].offloads' if it is set in 
'port->dev_conf.rxmode.offloads'.

If a port has capability for 'JUMBO_FRAME', 'port->rx_conf[].offloads' can have 
it. And the port level capability is already checked above.

>> +       }
>> +
>> +       /* If JUMBO_FRAME is set MTU conversion done by ethdev layer,
>> +        * if unset do it here
>> +        */
>> +       if ((rx_offloads & DEV_RX_OFFLOAD_JUMBO_FRAME) == 0) {
>> +               ret = rte_eth_dev_set_mtu(portid,
>> +                               port->dev_conf.rxmode.max_rx_pkt_len - eth_overhead);
>> +               if (ret)
>> +                       printf("Failed to set MTU to %u for port %u\n",
>> +                               port->dev_conf.rxmode.max_rx_pkt_len - eth_overhead,
>> +                               portid);
>> +       }
>> +
>> +       return 0;
>> +}
>> +
> 
> Applied and tested with a few iterations of configuring max packet size
> back and forth between jumbo and non-jumbo sizes, also tried setting
> max packet size using the command-line option, all seems good based
> on rx offloads and packet forwarding.
> 
> Two minor questions above, otherwise LGTM.
> 

Thanks for testing. I will wait for more comments before new version.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-stable] [PATCH v5] app/testpmd: fix setting maximum packet length
  2021-01-26  0:44     ` Ferruh Yigit
@ 2021-01-26  3:22       ` Lance Richardson
  2021-01-26  3:45       ` Lance Richardson
  1 sibling, 0 replies; 17+ messages in thread
From: Lance Richardson @ 2021-01-26  3:22 UTC (permalink / raw)
  To: Ferruh Yigit
  Cc: Wenzhuo Lu, Xiaoyun Li, Bernard Iremonger, Steve Yang, dev,
	stable, oulijun, wisamm, lihuisong

[-- Attachment #1: Type: text/plain, Size: 4181 bytes --]

Acked-by: Lance Richardson <lance.richardson@broadcom.com>

Thanks,
    Lance

On Mon, Jan 25, 2021 at 7:44 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>
> On 1/25/2021 7:41 PM, Lance Richardson wrote:
> > On Mon, Jan 25, 2021 at 1:15 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> >>
> >> From: Steve Yang <stevex.yang@intel.com>
> >>
> >> "port config all max-pkt-len" command fails because it doesn't set the
> >> 'DEV_RX_OFFLOAD_JUMBO_FRAME' offload flag properly.
> >>
> >> Commit in the fixes line moved the 'DEV_RX_OFFLOAD_JUMBO_FRAME' offload
> >> flag update from 'cmd_config_max_pkt_len_parsed()' to 'init_config()'.
> >> 'init_config()' function is only called during testpmd startup, but the
> >> flag status needs to be calculated whenever 'max_rx_pkt_len' changes.
> >>
> >> The issue can be reproduce as [1], where the 'max-pkt-len' reduced and
> >> 'DEV_RX_OFFLOAD_JUMBO_FRAME' offload flag should be cleared but it
> >> didn't.
> >>
> >> Adding the 'update_jumbo_frame_offload()' helper function to update
> >> 'DEV_RX_OFFLOAD_JUMBO_FRAME' offload flag and 'max_rx_pkt_len'. This
> >> function is called both by 'init_config()' and
> >> 'cmd_config_max_pkt_len_parsed()'.
> >>
> >> Default 'max-pkt-len' value set to zero, 'update_jumbo_frame_offload()'
> >> updates it to "RTE_ETHER_MTU + PMD specific Ethernet overhead" when it
> >> is zero.
> >> If '--max-pkt-len=N' argument provided, it will be used instead.
> >> And with each "port config all max-pkt-len" command, the
> >> 'DEV_RX_OFFLOAD_JUMBO_FRAME' offload flag, 'max-pkt-len' and MTU is
> >> updated.
> >>
> >> [1]
> >
> > <snip>
> >
> >> +/*
> >> + * Helper function to arrange max_rx_pktlen value and JUMBO_FRAME offload,
> >> + * MTU is also aligned if JUMBO_FRAME offload is not set.
> >> + *
> >> + * port->dev_info should be get before calling this function.
> >
> > Should this be "port->dev_info should be set ..." instead?
> >
>
> Ack.
>
> >
> > <snip>
> >
> >> +       if (rx_offloads != port->dev_conf.rxmode.offloads) {
> >> +               uint16_t qid;
> >> +
> >> +               port->dev_conf.rxmode.offloads = rx_offloads;
> >> +
> >> +               /* Apply JUMBO_FRAME offload configuration to Rx queue(s) */
> >> +               for (qid = 0; qid < port->dev_info.nb_rx_queues; qid++) {
> >> +                       if (on)
> >> +                               port->rx_conf[qid].offloads |= DEV_RX_OFFLOAD_JUMBO_FRAME;
> >> +                       else
> >> +                               port->rx_conf[qid].offloads &= ~DEV_RX_OFFLOAD_JUMBO_FRAME;
> >> +               }
> >
> > Is it correct to set per-queue offloads that aren't advertised by the PMD
> > as supported in rx_queue_offload_capa?
> >
>
> 'port->rx_conf[]' is testpmd struct, and 'port->dev_conf.rxmode.offloads' values
> are reflected to 'port->rx_conf[].offloads' for all queues.
>
> We should set the offload in 'port->rx_conf[].offloads' if it is set in
> 'port->dev_conf.rxmode.offloads'.
>
> If a port has capability for 'JUMBO_FRAME', 'port->rx_conf[].offloads' can have
> it. And the port level capability is already checked above.
>
> >> +       }
> >> +
> >> +       /* If JUMBO_FRAME is set MTU conversion done by ethdev layer,
> >> +        * if unset do it here
> >> +        */
> >> +       if ((rx_offloads & DEV_RX_OFFLOAD_JUMBO_FRAME) == 0) {
> >> +               ret = rte_eth_dev_set_mtu(portid,
> >> +                               port->dev_conf.rxmode.max_rx_pkt_len - eth_overhead);
> >> +               if (ret)
> >> +                       printf("Failed to set MTU to %u for port %u\n",
> >> +                               port->dev_conf.rxmode.max_rx_pkt_len - eth_overhead,
> >> +                               portid);
> >> +       }
> >> +
> >> +       return 0;
> >> +}
> >> +
> >
> > Applied and tested with a few iterations of configuring max packet size
> > back and forth between jumbo and non-jumbo sizes, also tried setting
> > max packet size using the command-line option, all seems good based
> > on rx offloads and packet forwarding.
> >
> > Two minor questions above, otherwise LGTM.
> >
>
> Thanks for testing. I will wait for more comments before new version.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-stable] [PATCH v5] app/testpmd: fix setting maximum packet length
  2021-01-26  0:44     ` Ferruh Yigit
  2021-01-26  3:22       ` Lance Richardson
@ 2021-01-26  3:45       ` Lance Richardson
  2021-01-26  7:54         ` Li, Xiaoyun
  2021-01-26 11:01         ` Ferruh Yigit
  1 sibling, 2 replies; 17+ messages in thread
From: Lance Richardson @ 2021-01-26  3:45 UTC (permalink / raw)
  To: Ferruh Yigit
  Cc: Wenzhuo Lu, Xiaoyun Li, Bernard Iremonger, Steve Yang, dev,
	stable, oulijun, wisamm, lihuisong

[-- Attachment #1: Type: text/plain, Size: 2216 bytes --]

On Mon, Jan 25, 2021 at 7:44 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>
> >> +       if (rx_offloads != port->dev_conf.rxmode.offloads) {
> >> +               uint16_t qid;
> >> +
> >> +               port->dev_conf.rxmode.offloads = rx_offloads;
> >> +
> >> +               /* Apply JUMBO_FRAME offload configuration to Rx queue(s) */
> >> +               for (qid = 0; qid < port->dev_info.nb_rx_queues; qid++) {
> >> +                       if (on)
> >> +                               port->rx_conf[qid].offloads |= DEV_RX_OFFLOAD_JUMBO_FRAME;
> >> +                       else
> >> +                               port->rx_conf[qid].offloads &= ~DEV_RX_OFFLOAD_JUMBO_FRAME;
> >> +               }
> >
> > Is it correct to set per-queue offloads that aren't advertised by the PMD
> > as supported in rx_queue_offload_capa?
> >
>
> 'port->rx_conf[]' is testpmd struct, and 'port->dev_conf.rxmode.offloads' values
> are reflected to 'port->rx_conf[].offloads' for all queues.
>
> We should set the offload in 'port->rx_conf[].offloads' if it is set in
> 'port->dev_conf.rxmode.offloads'.
>
> If a port has capability for 'JUMBO_FRAME', 'port->rx_conf[].offloads' can have
> it. And the port level capability is already checked above.
>

I'm still not 100% clear about the per-queue offload question.

With this patch, and jumbo max packet size configured (on the command
line in this case), I see:

testpmd> show port 0 rx_offload configuration
Rx Offloading Configuration of port 0 :
  Port : JUMBO_FRAME
  Queue[ 0] : JUMBO_FRAME

testpmd> show port 0 rx_offload capabilities
Rx Offloading Capabilities of port 0 :
  Per Queue :
  Per Port  : VLAN_STRIP IPV4_CKSUM UDP_CKSUM TCP_CKSUM TCP_LRO
OUTER_IPV4_CKSUM VLAN_FILTER VLAN_EXTEND JUMBO_FRAME SCATTER TIMESTAMP
KEEP_CRC OUTER_UDP_CKSUM RSS_HASH

Yet if I configure a jumbo MTU starting with standard max packet size,
jumbo is only enabled at the port level:
testpmd> port config mtu 0 9000
testpmd> port start all

testpmd> show port 0 rx_offload configuration
Rx Offloading Configuration of port 0 :
  Port : JUMBO_FRAME
  Queue[ 0] :

It still seems odd for a per-queue offload to be enabled on a PMD that
doesn't support per-queue receive offloads.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-stable] [PATCH v5] app/testpmd: fix setting maximum packet length
  2021-01-26  3:45       ` Lance Richardson
@ 2021-01-26  7:54         ` Li, Xiaoyun
  2021-01-26 11:01         ` Ferruh Yigit
  1 sibling, 0 replies; 17+ messages in thread
From: Li, Xiaoyun @ 2021-01-26  7:54 UTC (permalink / raw)
  To: Lance Richardson, Yigit, Ferruh
  Cc: Lu, Wenzhuo, Iremonger, Bernard, Yang, SteveX, dev, stable,
	oulijun, wisamm, lihuisong



> -----Original Message-----
> From: Lance Richardson <lance.richardson@broadcom.com>
> Sent: Tuesday, January 26, 2021 11:45
> To: Yigit, Ferruh <ferruh.yigit@intel.com>
> Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Li, Xiaoyun <xiaoyun.li@intel.com>;
> Iremonger, Bernard <bernard.iremonger@intel.com>; Yang, SteveX
> <stevex.yang@intel.com>; dev@dpdk.org; stable@dpdk.org;
> oulijun@huawei.com; wisamm@mellanox.com; lihuisong@huawei.com
> Subject: Re: [PATCH v5] app/testpmd: fix setting maximum packet length
> 
> On Mon, Jan 25, 2021 at 7:44 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> >
> > >> +       if (rx_offloads != port->dev_conf.rxmode.offloads) {
> > >> +               uint16_t qid;
> > >> +
> > >> +               port->dev_conf.rxmode.offloads = rx_offloads;
> > >> +
> > >> +               /* Apply JUMBO_FRAME offload configuration to Rx queue(s) */
> > >> +               for (qid = 0; qid < port->dev_info.nb_rx_queues; qid++) {
> > >> +                       if (on)
> > >> +                               port->rx_conf[qid].offloads |=
> DEV_RX_OFFLOAD_JUMBO_FRAME;
> > >> +                       else
> > >> +                               port->rx_conf[qid].offloads &=
> ~DEV_RX_OFFLOAD_JUMBO_FRAME;
> > >> +               }
> > >
> > > Is it correct to set per-queue offloads that aren't advertised by the PMD
> > > as supported in rx_queue_offload_capa?
> > >
> >
> > 'port->rx_conf[]' is testpmd struct, and 'port->dev_conf.rxmode.offloads'
> values
> > are reflected to 'port->rx_conf[].offloads' for all queues.
> >
> > We should set the offload in 'port->rx_conf[].offloads' if it is set in
> > 'port->dev_conf.rxmode.offloads'.
> >
> > If a port has capability for 'JUMBO_FRAME', 'port->rx_conf[].offloads' can
> have
> > it. And the port level capability is already checked above.
> >
> 
> I'm still not 100% clear about the per-queue offload question.
> 
> With this patch, and jumbo max packet size configured (on the command
> line in this case), I see:
> 
> testpmd> show port 0 rx_offload configuration
> Rx Offloading Configuration of port 0 :
>   Port : JUMBO_FRAME
>   Queue[ 0] : JUMBO_FRAME
> 
> testpmd> show port 0 rx_offload capabilities
> Rx Offloading Capabilities of port 0 :
>   Per Queue :
>   Per Port  : VLAN_STRIP IPV4_CKSUM UDP_CKSUM TCP_CKSUM TCP_LRO
> OUTER_IPV4_CKSUM VLAN_FILTER VLAN_EXTEND JUMBO_FRAME SCATTER
> TIMESTAMP
> KEEP_CRC OUTER_UDP_CKSUM RSS_HASH
> 
> Yet if I configure a jumbo MTU starting with standard max packet size,
> jumbo is only enabled at the port level:
> testpmd> port config mtu 0 9000
> testpmd> port start all
> 
> testpmd> show port 0 rx_offload configuration
> Rx Offloading Configuration of port 0 :
>   Port : JUMBO_FRAME
>   Queue[ 0] :
> 
> It still seems odd for a per-queue offload to be enabled on a PMD that
> doesn't support per-queue receive offloads.

In struct rte_eth_dev_info, rx_offload_capa means All RX offload capabilities including all per-queue ones.
And rx_queue_offload_capa means Device per-queue RX offload capabilities.

The meaning of rx_queue_offload_capa is a bit of confusing between here and driver.
I think here rx_queue_offload_capa means whether a queue supports offloads.

But some drivers like i40e don't use rx_queue_offload_capa, set this as 0 and only use global rx_offload_capa.
I guess it's because the driver doesn't want to support different offloads settings for different queues?
Then rx_queue_offload_capa means differently.

Actually for i40e, it can support different offloads for different queues since there is 'offloads' in struct i40e_rx_queue.
I40e can just set rx_queue_offload_capa as the value for rx_offload_capa.
But maybe some drivers really don't want this? Not sure on this.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-stable] [PATCH v5] app/testpmd: fix setting maximum packet length
  2021-01-25 18:15 ` [dpdk-stable] [PATCH v5] app/testpmd: fix setting maximum packet length Ferruh Yigit
  2021-01-25 19:41   ` Lance Richardson
@ 2021-01-26  9:02   ` Wisam Monther
  2021-01-27  3:04   ` Li, Xiaoyun
                     ` (2 subsequent siblings)
  4 siblings, 0 replies; 17+ messages in thread
From: Wisam Monther @ 2021-01-26  9:02 UTC (permalink / raw)
  To: Ferruh Yigit, Wenzhuo Lu, Xiaoyun Li, Bernard Iremonger, Steve Yang
  Cc: dev, stable, lance.richardson, oulijun, lihuisong

Hi,

> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Monday, January 25, 2021 8:16 PM
> To: Wenzhuo Lu <wenzhuo.lu@intel.com>; Xiaoyun Li
> <xiaoyun.li@intel.com>; Bernard Iremonger
> <bernard.iremonger@intel.com>; Steve Yang <stevex.yang@intel.com>
> Cc: Ferruh Yigit <ferruh.yigit@intel.com>; dev@dpdk.org; stable@dpdk.org;
> lance.richardson@broadcom.com; oulijun@huawei.com; Wisam Monther
> <wisamm@nvidia.com>; lihuisong@huawei.com
> Subject: [PATCH v5] app/testpmd: fix setting maximum packet length
> 
> From: Steve Yang <stevex.yang@intel.com>
> 
> "port config all max-pkt-len" command fails because it doesn't set the
> 'DEV_RX_OFFLOAD_JUMBO_FRAME' offload flag properly.
> 
> Commit in the fixes line moved the 'DEV_RX_OFFLOAD_JUMBO_FRAME'
> offload flag update from 'cmd_config_max_pkt_len_parsed()' to
> 'init_config()'.
> 'init_config()' function is only called during testpmd startup, but the flag
> status needs to be calculated whenever 'max_rx_pkt_len' changes.
> 
> The issue can be reproduce as [1], where the 'max-pkt-len' reduced and
> 'DEV_RX_OFFLOAD_JUMBO_FRAME' offload flag should be cleared but it
> didn't.
> 
> Adding the 'update_jumbo_frame_offload()' helper function to update
> 'DEV_RX_OFFLOAD_JUMBO_FRAME' offload flag and 'max_rx_pkt_len'. This
> function is called both by 'init_config()' and
> 'cmd_config_max_pkt_len_parsed()'.
> 
> Default 'max-pkt-len' value set to zero, 'update_jumbo_frame_offload()'
> updates it to "RTE_ETHER_MTU + PMD specific Ethernet overhead" when it is
> zero.
> If '--max-pkt-len=N' argument provided, it will be used instead.
> And with each "port config all max-pkt-len" command, the
> 'DEV_RX_OFFLOAD_JUMBO_FRAME' offload flag, 'max-pkt-len' and MTU is
> updated.
> 
> [1]
> --------------------------------------------------------------------------
> dpdk-testpmd -c 0xf -n 4 -- -i --max-pkt-len=9000 --tx-offloads=0x8000
> 	--rxq=4 --txq=4 --disable-rss
> testpmd>  set verbose 3
> testpmd>  port stop all
> testpmd>  port config all max-pkt-len 1518  port start all
> 
> // Got fail error info without this patch Configuring Port 0 (socket 1) Ethdev
> port_id=0 rx_queue_id=0, new added offloads 0x800 must be within per-
> queue offload capabilities 0x0 in rte_eth_rx_queue_setup() Fail to configure
> port 0 rx queues //<-- Fail error info;
> --------------------------------------------------------------------------
> 
> Fixes: 761c4d66900f ("app/testpmd: fix max Rx packet length for VLAN
> packets")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Steve Yang <stevex.yang@intel.com>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
> 
> v5:
> * 'update_jumbo_frame_offload()' helper updated
>   * check zero 'max-pkt-len' value
>   * Update how queue offload flags updated
>   * Update MTU if JUMBO_FRAME flag is not set
> * Default testpmd 'max-pkt-len' value set to zero
> 
> Cc: lance.richardson@broadcom.com
> Cc: oulijun@huawei.com
> Cc: wisamm@mellanox.com
> Cc: lihuisong@huawei.com
> ---

It fixed this bug indeed: https://bugs.dpdk.org/show_bug.cgi?id=625
Thanks
Acked-by: Wisam Jaddo <wisamm@nvidia.com>


BRs,
Wisam Jaddo

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-stable] [PATCH v5] app/testpmd: fix setting maximum packet length
  2021-01-26  3:45       ` Lance Richardson
  2021-01-26  7:54         ` Li, Xiaoyun
@ 2021-01-26 11:01         ` Ferruh Yigit
  2021-01-28 21:36           ` Lance Richardson
  1 sibling, 1 reply; 17+ messages in thread
From: Ferruh Yigit @ 2021-01-26 11:01 UTC (permalink / raw)
  To: Lance Richardson
  Cc: Wenzhuo Lu, Xiaoyun Li, Bernard Iremonger, Steve Yang, dev,
	stable, oulijun, wisamm, lihuisong

On 1/26/2021 3:45 AM, Lance Richardson wrote:
> On Mon, Jan 25, 2021 at 7:44 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>>
>>>> +       if (rx_offloads != port->dev_conf.rxmode.offloads) {
>>>> +               uint16_t qid;
>>>> +
>>>> +               port->dev_conf.rxmode.offloads = rx_offloads;
>>>> +
>>>> +               /* Apply JUMBO_FRAME offload configuration to Rx queue(s) */
>>>> +               for (qid = 0; qid < port->dev_info.nb_rx_queues; qid++) {
>>>> +                       if (on)
>>>> +                               port->rx_conf[qid].offloads |= DEV_RX_OFFLOAD_JUMBO_FRAME;
>>>> +                       else
>>>> +                               port->rx_conf[qid].offloads &= ~DEV_RX_OFFLOAD_JUMBO_FRAME;
>>>> +               }
>>>
>>> Is it correct to set per-queue offloads that aren't advertised by the PMD
>>> as supported in rx_queue_offload_capa?
>>>
>>
>> 'port->rx_conf[]' is testpmd struct, and 'port->dev_conf.rxmode.offloads' values
>> are reflected to 'port->rx_conf[].offloads' for all queues.
>>
>> We should set the offload in 'port->rx_conf[].offloads' if it is set in
>> 'port->dev_conf.rxmode.offloads'.
>>
>> If a port has capability for 'JUMBO_FRAME', 'port->rx_conf[].offloads' can have
>> it. And the port level capability is already checked above.
>>
> 
> I'm still not 100% clear about the per-queue offload question.
> 
> With this patch, and jumbo max packet size configured (on the command
> line in this case), I see:
> 
> testpmd> show port 0 rx_offload configuration
> Rx Offloading Configuration of port 0 :
>    Port : JUMBO_FRAME
>    Queue[ 0] : JUMBO_FRAME
> 
> testpmd> show port 0 rx_offload capabilities
> Rx Offloading Capabilities of port 0 :
>    Per Queue :
>    Per Port  : VLAN_STRIP IPV4_CKSUM UDP_CKSUM TCP_CKSUM TCP_LRO
> OUTER_IPV4_CKSUM VLAN_FILTER VLAN_EXTEND JUMBO_FRAME SCATTER TIMESTAMP
> KEEP_CRC OUTER_UDP_CKSUM RSS_HASH
> 

The port level offload is applied to all queues on the port, testpmd config 
structure reflects this logic in implementation.
If Rx offload X is set for a port, it is set for all Rx queue offloads, this is 
not new behavior and not related to this patch.

In the ethdev, lets assume X & Y are port level offloads,
after X, Y are set via 'rte_eth_dev_configure()'
if user calls 'rte_eth_rx_queue_setup()' with X & Y offload, this is a valid 
call and API will return success, since those offloads already enabled in port 
level means they are enabled for all queues.

Because of above ethdev behavior, testpmd keeps all enabled port level offload 
in the queue level offload too, and display them as enabled offloads for the queue.

To request a queue specific offload, it is added to specific queue's config 
before calling queue setup. Lets say that queue specific offload is Z, after 
setup testpmd config struct will show that specific queue has X, Y & Z offloads.

I hope it is more clear now.

> Yet if I configure a jumbo MTU starting with standard max packet size,
> jumbo is only enabled at the port level:
> testpmd> port config mtu 0 9000
> testpmd> port start all
> 
> testpmd> show port 0 rx_offload configuration
> Rx Offloading Configuration of port 0 :
>    Port : JUMBO_FRAME
>    Queue[ 0] :
> 
> It still seems odd for a per-queue offload to be enabled on a PMD that
> doesn't support per-queue receive offloads.
> 

"port config mtu" should take queue offloads into account, it looks wrong right now.



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-stable] [PATCH v5] app/testpmd: fix setting maximum packet length
  2021-01-25 18:15 ` [dpdk-stable] [PATCH v5] app/testpmd: fix setting maximum packet length Ferruh Yigit
  2021-01-25 19:41   ` Lance Richardson
  2021-01-26  9:02   ` Wisam Monther
@ 2021-01-27  3:04   ` Li, Xiaoyun
  2021-01-28  1:57     ` Chen, BoX C
  2021-01-28  9:18   ` Wisam Monther
  2021-01-28 12:07   ` [dpdk-stable] [PATCH v6] " Ferruh Yigit
  4 siblings, 1 reply; 17+ messages in thread
From: Li, Xiaoyun @ 2021-01-27  3:04 UTC (permalink / raw)
  To: Yigit, Ferruh, Lu, Wenzhuo, Iremonger, Bernard, Yang, SteveX
  Cc: dev, stable, lance.richardson, oulijun, wisamm, lihuisong

Except a minor typo inline.
Acked-by: Xiaoyun Li <xiaoyun.li@intel.com>

> -----Original Message-----
> From: Yigit, Ferruh <ferruh.yigit@intel.com>
> Sent: Tuesday, January 26, 2021 02:16
> To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Li, Xiaoyun <xiaoyun.li@intel.com>;
> Iremonger, Bernard <bernard.iremonger@intel.com>; Yang, SteveX
> <stevex.yang@intel.com>
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; dev@dpdk.org; stable@dpdk.org;
> lance.richardson@broadcom.com; oulijun@huawei.com;
> wisamm@mellanox.com; lihuisong@huawei.com
> Subject: [PATCH v5] app/testpmd: fix setting maximum packet length
> 
> From: Steve Yang <stevex.yang@intel.com>
> 
> "port config all max-pkt-len" command fails because it doesn't set the
> 'DEV_RX_OFFLOAD_JUMBO_FRAME' offload flag properly.
> 
> Commit in the fixes line moved the 'DEV_RX_OFFLOAD_JUMBO_FRAME' offload
> flag update from 'cmd_config_max_pkt_len_parsed()' to 'init_config()'.
> 'init_config()' function is only called during testpmd startup, but the flag status
> needs to be calculated whenever 'max_rx_pkt_len' changes.
> 
> The issue can be reproduce as [1], where the 'max-pkt-len' reduced and
reproduced

> 'DEV_RX_OFFLOAD_JUMBO_FRAME' offload flag should be cleared but it didn't.
> 
> Adding the 'update_jumbo_frame_offload()' helper function to update
> 'DEV_RX_OFFLOAD_JUMBO_FRAME' offload flag and 'max_rx_pkt_len'. This
> function is called both by 'init_config()' and 'cmd_config_max_pkt_len_parsed()'.
> 
> Default 'max-pkt-len' value set to zero, 'update_jumbo_frame_offload()'
> updates it to "RTE_ETHER_MTU + PMD specific Ethernet overhead" when it is
> zero.
> If '--max-pkt-len=N' argument provided, it will be used instead.
> And with each "port config all max-pkt-len" command, the
> 'DEV_RX_OFFLOAD_JUMBO_FRAME' offload flag, 'max-pkt-len' and MTU is
> updated.
> 
> [1]
> --------------------------------------------------------------------------
> dpdk-testpmd -c 0xf -n 4 -- -i --max-pkt-len=9000 --tx-offloads=0x8000
> 	--rxq=4 --txq=4 --disable-rss
> testpmd>  set verbose 3
> testpmd>  port stop all
> testpmd>  port config all max-pkt-len 1518  port start all
> 
> // Got fail error info without this patch Configuring Port 0 (socket 1) Ethdev
> port_id=0 rx_queue_id=0, new added offloads 0x800 must be within per-queue
> offload capabilities 0x0 in rte_eth_rx_queue_setup() Fail to configure port 0 rx
> queues //<-- Fail error info;
> --------------------------------------------------------------------------
> 
> Fixes: 761c4d66900f ("app/testpmd: fix max Rx packet length for VLAN packets")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Steve Yang <stevex.yang@intel.com>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
> 
> v5:
> * 'update_jumbo_frame_offload()' helper updated
>   * check zero 'max-pkt-len' value
>   * Update how queue offload flags updated
>   * Update MTU if JUMBO_FRAME flag is not set
> * Default testpmd 'max-pkt-len' value set to zero
> 
> Cc: lance.richardson@broadcom.com
> Cc: oulijun@huawei.com
> Cc: wisamm@mellanox.com
> Cc: lihuisong@huawei.com
> ---
>  app/test-pmd/cmdline.c |  13 ++++++
>  app/test-pmd/testpmd.c | 102 +++++++++++++++++++++++++++++++++--------
>  app/test-pmd/testpmd.h |   1 +
>  3 files changed, 97 insertions(+), 19 deletions(-)
> 
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index
> 89034c8b7272..9ada4316c6c0 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -1877,7 +1877,9 @@ cmd_config_max_pkt_len_parsed(void
> *parsed_result,
>  				__rte_unused void *data)
>  {
>  	struct cmd_config_max_pkt_len_result *res = parsed_result;
> +	uint32_t max_rx_pkt_len_backup = 0;
>  	portid_t pid;
> +	int ret;
> 
>  	if (!all_ports_stopped()) {
>  		printf("Please stop all ports first\n"); @@ -1896,7 +1898,18 @@
> cmd_config_max_pkt_len_parsed(void *parsed_result,
>  			if (res->value == port-
> >dev_conf.rxmode.max_rx_pkt_len)
>  				return;
> 
> +			ret = eth_dev_info_get_print_err(pid, &port->dev_info);
> +			if (ret != 0) {
> +				printf("rte_eth_dev_info_get() failed for
> port %u\n",
> +					pid);
> +				return;
> +			}
> +
> +			max_rx_pkt_len_backup = port-
> >dev_conf.rxmode.max_rx_pkt_len;
> +
>  			port->dev_conf.rxmode.max_rx_pkt_len = res->value;
> +			if (update_jumbo_frame_offload(pid) != 0)
> +				port->dev_conf.rxmode.max_rx_pkt_len =
> max_rx_pkt_len_backup;
>  		} else {
>  			printf("Unknown parameter\n");
>  			return;
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> c256e719aea2..b69fcd3fde72 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -443,8 +443,11 @@ lcoreid_t latencystats_lcore_id = -1;
>   * Ethernet device configuration.
>   */
>  struct rte_eth_rxmode rx_mode = {
> -	.max_rx_pkt_len = RTE_ETHER_MAX_LEN,
> -		/**< Default maximum frame length. */
> +	/* Default maximum frame length.
> +	 * Zero is converted to "RTE_ETHER_MTU + PMD Ethernet overhead"
> +	 * in init_config().
> +	 */
> +	.max_rx_pkt_len = 0,
>  };
> 
>  struct rte_eth_txmode tx_mode = {
> @@ -1410,7 +1413,6 @@ init_config(void)
>  	struct rte_gro_param gro_param;
>  	uint32_t gso_types;
>  	uint16_t data_size;
> -	uint16_t eth_overhead;
>  	bool warning = 0;
>  	int k;
>  	int ret;
> @@ -1447,22 +1449,10 @@ init_config(void)
>  			rte_exit(EXIT_FAILURE,
>  				 "rte_eth_dev_info_get() failed\n");
> 
> -		/* Update the max_rx_pkt_len to have MTU as
> RTE_ETHER_MTU */
> -		if (port->dev_info.max_mtu != UINT16_MAX &&
> -		    port->dev_info.max_rx_pktlen > port->dev_info.max_mtu)
> -			eth_overhead = port->dev_info.max_rx_pktlen -
> -				port->dev_info.max_mtu;
> -		else
> -			eth_overhead =
> -				RTE_ETHER_HDR_LEN + RTE_ETHER_CRC_LEN;
> -
> -		if (port->dev_conf.rxmode.max_rx_pkt_len <=
> -			(uint32_t)(RTE_ETHER_MTU + eth_overhead))
> -			port->dev_conf.rxmode.max_rx_pkt_len =
> -					RTE_ETHER_MTU + eth_overhead;
> -		else
> -			port->dev_conf.rxmode.offloads |=
> -					DEV_RX_OFFLOAD_JUMBO_FRAME;
> +		ret = update_jumbo_frame_offload(pid);
> +		if (ret != 0)
> +			printf("Updating jumbo frame offload failed for
> port %u\n",
> +				pid);
> 
>  		if (!(port->dev_info.tx_offload_capa &
>  		      DEV_TX_OFFLOAD_MBUF_FAST_FREE)) @@ -3358,6
> +3348,80 @@ rxtx_port_config(struct rte_port *port)
>  	}
>  }
> 
> +/*
> + * Helper function to arrange max_rx_pktlen value and JUMBO_FRAME
> +offload,
> + * MTU is also aligned if JUMBO_FRAME offload is not set.
> + *
> + * port->dev_info should be get before calling this function.
> + *
> + * return 0 on success, negative on error  */ int
> +update_jumbo_frame_offload(portid_t portid) {
> +	struct rte_port *port = &ports[portid];
> +	uint32_t eth_overhead;
> +	uint64_t rx_offloads;
> +	int ret;
> +	bool on;
> +
> +	/* Update the max_rx_pkt_len to have MTU as RTE_ETHER_MTU */
> +	if (port->dev_info.max_mtu != UINT16_MAX &&
> +	    port->dev_info.max_rx_pktlen > port->dev_info.max_mtu)
> +		eth_overhead = port->dev_info.max_rx_pktlen -
> +				port->dev_info.max_mtu;
> +	else
> +		eth_overhead = RTE_ETHER_HDR_LEN + RTE_ETHER_CRC_LEN;
> +
> +	rx_offloads = port->dev_conf.rxmode.offloads;
> +
> +	/* Default config value is 0 to use PMD specific overhead */
> +	if (port->dev_conf.rxmode.max_rx_pkt_len == 0)
> +		port->dev_conf.rxmode.max_rx_pkt_len = RTE_ETHER_MTU +
> eth_overhead;
> +
> +	if (port->dev_conf.rxmode.max_rx_pkt_len <= RTE_ETHER_MTU +
> eth_overhead) {
> +		rx_offloads &= ~DEV_RX_OFFLOAD_JUMBO_FRAME;
> +		on = false;
> +	} else {
> +		if ((port->dev_info.rx_offload_capa &
> DEV_RX_OFFLOAD_JUMBO_FRAME) == 0) {
> +			printf("Frame size (%u) is not supported by port %u\n",
> +				port->dev_conf.rxmode.max_rx_pkt_len,
> +				portid);
> +			return -1;
> +		}
> +		rx_offloads |= DEV_RX_OFFLOAD_JUMBO_FRAME;
> +		on = true;
> +	}
> +
> +	if (rx_offloads != port->dev_conf.rxmode.offloads) {
> +		uint16_t qid;
> +
> +		port->dev_conf.rxmode.offloads = rx_offloads;
> +
> +		/* Apply JUMBO_FRAME offload configuration to Rx queue(s)
> */
> +		for (qid = 0; qid < port->dev_info.nb_rx_queues; qid++) {
> +			if (on)
> +				port->rx_conf[qid].offloads |=
> DEV_RX_OFFLOAD_JUMBO_FRAME;
> +			else
> +				port->rx_conf[qid].offloads &=
> ~DEV_RX_OFFLOAD_JUMBO_FRAME;
> +		}
> +	}
> +
> +	/* If JUMBO_FRAME is set MTU conversion done by ethdev layer,
> +	 * if unset do it here
> +	 */
> +	if ((rx_offloads & DEV_RX_OFFLOAD_JUMBO_FRAME) == 0) {
> +		ret = rte_eth_dev_set_mtu(portid,
> +				port->dev_conf.rxmode.max_rx_pkt_len -
> eth_overhead);
> +		if (ret)
> +			printf("Failed to set MTU to %u for port %u\n",
> +				port->dev_conf.rxmode.max_rx_pkt_len -
> eth_overhead,
> +				portid);
> +	}
> +
> +	return 0;
> +}
> +
>  void
>  init_port_config(void)
>  {
> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h index
> 5f2316210726..2f8f5a92e46a 100644
> --- a/app/test-pmd/testpmd.h
> +++ b/app/test-pmd/testpmd.h
> @@ -1005,6 +1005,7 @@ uint16_t tx_pkt_set_dynf(uint16_t port_id,
> __rte_unused uint16_t queue,
>  			 __rte_unused void *user_param);
>  void add_tx_dynf_callback(portid_t portid);  void
> remove_tx_dynf_callback(portid_t portid);
> +int update_jumbo_frame_offload(portid_t portid);
> 
>  /*
>   * Work-around of a compilation error with ICC on invocations of the
> --
> 2.29.2


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-stable] [PATCH v5] app/testpmd: fix setting maximum packet length
  2021-01-27  3:04   ` Li, Xiaoyun
@ 2021-01-28  1:57     ` Chen, BoX C
  0 siblings, 0 replies; 17+ messages in thread
From: Chen, BoX C @ 2021-01-28  1:57 UTC (permalink / raw)
  To: Li, Xiaoyun, Yigit, Ferruh, Lu, Wenzhuo, Iremonger, Bernard,
	Yang, SteveX
  Cc: dev, stable, lance.richardson, oulijun, wisamm, lihuisong

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Li, Xiaoyun
> Sent: January 27, 2021 11:05
> To: Yigit, Ferruh <ferruh.yigit@intel.com>; Lu, Wenzhuo
> <wenzhuo.lu@intel.com>; Iremonger, Bernard
> <bernard.iremonger@intel.com>; Yang, SteveX <stevex.yang@intel.com>
> Cc: dev@dpdk.org; stable@dpdk.org; lance.richardson@broadcom.com;
> oulijun@huawei.com; wisamm@mellanox.com; lihuisong@huawei.com
> Subject: Re: [dpdk-dev] [PATCH v5] app/testpmd: fix setting maximum
> packet length
> 
> Except a minor typo inline.
> Acked-by: Xiaoyun Li <xiaoyun.li@intel.com>
> 

Tested-by: Chen, BoX C <box.c.chen@intel.com>


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-stable] [PATCH v5] app/testpmd: fix setting maximum packet length
  2021-01-25 18:15 ` [dpdk-stable] [PATCH v5] app/testpmd: fix setting maximum packet length Ferruh Yigit
                     ` (2 preceding siblings ...)
  2021-01-27  3:04   ` Li, Xiaoyun
@ 2021-01-28  9:18   ` Wisam Monther
  2021-01-28  9:26     ` Ferruh Yigit
  2021-01-28 12:07   ` [dpdk-stable] [PATCH v6] " Ferruh Yigit
  4 siblings, 1 reply; 17+ messages in thread
From: Wisam Monther @ 2021-01-28  9:18 UTC (permalink / raw)
  To: Ferruh Yigit, Wenzhuo Lu, Xiaoyun Li, Bernard Iremonger, Steve Yang
  Cc: dev, stable, lance.richardson, oulijun, lihuisong

Hi Ferruh,


> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Monday, January 25, 2021 8:16 PM
> To: Wenzhuo Lu <wenzhuo.lu@intel.com>; Xiaoyun Li
> <xiaoyun.li@intel.com>; Bernard Iremonger
> <bernard.iremonger@intel.com>; Steve Yang <stevex.yang@intel.com>
> Cc: Ferruh Yigit <ferruh.yigit@intel.com>; dev@dpdk.org; stable@dpdk.org;
> lance.richardson@broadcom.com; oulijun@huawei.com; Wisam Monther
> <wisamm@nvidia.com>; lihuisong@huawei.com
> Subject: [PATCH v5] app/testpmd: fix setting maximum packet length
> 
> From: Steve Yang <stevex.yang@intel.com>
> 
> "port config all max-pkt-len" command fails because it doesn't set the
> 'DEV_RX_OFFLOAD_JUMBO_FRAME' offload flag properly.
> 
> Commit in the fixes line moved the 'DEV_RX_OFFLOAD_JUMBO_FRAME'
> offload flag update from 'cmd_config_max_pkt_len_parsed()' to
> 'init_config()'.
> 'init_config()' function is only called during testpmd startup, but the flag
> status needs to be calculated whenever 'max_rx_pkt_len' changes.
> 
> The issue can be reproduce as [1], where the 'max-pkt-len' reduced and
> 'DEV_RX_OFFLOAD_JUMBO_FRAME' offload flag should be cleared but it
> didn't.
> 
> Adding the 'update_jumbo_frame_offload()' helper function to update
> 'DEV_RX_OFFLOAD_JUMBO_FRAME' offload flag and 'max_rx_pkt_len'. This
> function is called both by 'init_config()' and
> 'cmd_config_max_pkt_len_parsed()'.
> 
> Default 'max-pkt-len' value set to zero, 'update_jumbo_frame_offload()'
> updates it to "RTE_ETHER_MTU + PMD specific Ethernet overhead" when it is
> zero.
> If '--max-pkt-len=N' argument provided, it will be used instead.
> And with each "port config all max-pkt-len" command, the
> 'DEV_RX_OFFLOAD_JUMBO_FRAME' offload flag, 'max-pkt-len' and MTU is
> updated.
> 
> [1]
> --------------------------------------------------------------------------
> dpdk-testpmd -c 0xf -n 4 -- -i --max-pkt-len=9000 --tx-offloads=0x8000
> 	--rxq=4 --txq=4 --disable-rss
> testpmd>  set verbose 3
> testpmd>  port stop all
> testpmd>  port config all max-pkt-len 1518  port start all
> 
> // Got fail error info without this patch Configuring Port 0 (socket 1) Ethdev
> port_id=0 rx_queue_id=0, new added offloads 0x800 must be within per-
> queue offload capabilities 0x0 in rte_eth_rx_queue_setup() Fail to configure
> port 0 rx queues //<-- Fail error info;
> --------------------------------------------------------------------------
> 
> Fixes: 761c4d66900f ("app/testpmd: fix max Rx packet length for VLAN
> packets")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Steve Yang <stevex.yang@intel.com>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
> 
> v5:
> * 'update_jumbo_frame_offload()' helper updated
>   * check zero 'max-pkt-len' value
>   * Update how queue offload flags updated
>   * Update MTU if JUMBO_FRAME flag is not set
> * Default testpmd 'max-pkt-len' value set to zero
> 
> Cc: lance.richardson@broadcom.com
> Cc: oulijun@huawei.com
> Cc: wisamm@mellanox.com
> Cc: lihuisong@huawei.com
> ---

I think we need to have https://bugs.dpdk.org/show_bug.cgi?id=625 ID in the commit log as fix,
In order to allow the scripts to close related bugs directly from Bugzilla.

BRs,
Wisam Jaddo

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-stable] [PATCH v5] app/testpmd: fix setting maximum packet length
  2021-01-28  9:18   ` Wisam Monther
@ 2021-01-28  9:26     ` Ferruh Yigit
  2021-01-28 11:08       ` Wisam Monther
  0 siblings, 1 reply; 17+ messages in thread
From: Ferruh Yigit @ 2021-01-28  9:26 UTC (permalink / raw)
  To: Wisam Monther, Wenzhuo Lu, Xiaoyun Li, Bernard Iremonger, Steve Yang
  Cc: dev, stable, lance.richardson, oulijun, lihuisong

On 1/28/2021 9:18 AM, Wisam Monther wrote:
> Hi Ferruh,
> 
> 
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>> Sent: Monday, January 25, 2021 8:16 PM
>> To: Wenzhuo Lu <wenzhuo.lu@intel.com>; Xiaoyun Li
>> <xiaoyun.li@intel.com>; Bernard Iremonger
>> <bernard.iremonger@intel.com>; Steve Yang <stevex.yang@intel.com>
>> Cc: Ferruh Yigit <ferruh.yigit@intel.com>; dev@dpdk.org; stable@dpdk.org;
>> lance.richardson@broadcom.com; oulijun@huawei.com; Wisam Monther
>> <wisamm@nvidia.com>; lihuisong@huawei.com
>> Subject: [PATCH v5] app/testpmd: fix setting maximum packet length
>>
>> From: Steve Yang <stevex.yang@intel.com>
>>
>> "port config all max-pkt-len" command fails because it doesn't set the
>> 'DEV_RX_OFFLOAD_JUMBO_FRAME' offload flag properly.
>>
>> Commit in the fixes line moved the 'DEV_RX_OFFLOAD_JUMBO_FRAME'
>> offload flag update from 'cmd_config_max_pkt_len_parsed()' to
>> 'init_config()'.
>> 'init_config()' function is only called during testpmd startup, but the flag
>> status needs to be calculated whenever 'max_rx_pkt_len' changes.
>>
>> The issue can be reproduce as [1], where the 'max-pkt-len' reduced and
>> 'DEV_RX_OFFLOAD_JUMBO_FRAME' offload flag should be cleared but it
>> didn't.
>>
>> Adding the 'update_jumbo_frame_offload()' helper function to update
>> 'DEV_RX_OFFLOAD_JUMBO_FRAME' offload flag and 'max_rx_pkt_len'. This
>> function is called both by 'init_config()' and
>> 'cmd_config_max_pkt_len_parsed()'.
>>
>> Default 'max-pkt-len' value set to zero, 'update_jumbo_frame_offload()'
>> updates it to "RTE_ETHER_MTU + PMD specific Ethernet overhead" when it is
>> zero.
>> If '--max-pkt-len=N' argument provided, it will be used instead.
>> And with each "port config all max-pkt-len" command, the
>> 'DEV_RX_OFFLOAD_JUMBO_FRAME' offload flag, 'max-pkt-len' and MTU is
>> updated.
>>
>> [1]
>> --------------------------------------------------------------------------
>> dpdk-testpmd -c 0xf -n 4 -- -i --max-pkt-len=9000 --tx-offloads=0x8000
>> 	--rxq=4 --txq=4 --disable-rss
>> testpmd>  set verbose 3
>> testpmd>  port stop all
>> testpmd>  port config all max-pkt-len 1518  port start all
>>
>> // Got fail error info without this patch Configuring Port 0 (socket 1) Ethdev
>> port_id=0 rx_queue_id=0, new added offloads 0x800 must be within per-
>> queue offload capabilities 0x0 in rte_eth_rx_queue_setup() Fail to configure
>> port 0 rx queues //<-- Fail error info;
>> --------------------------------------------------------------------------
>>
>> Fixes: 761c4d66900f ("app/testpmd: fix max Rx packet length for VLAN
>> packets")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Steve Yang <stevex.yang@intel.com>
>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>> ---
>>
>> v5:
>> * 'update_jumbo_frame_offload()' helper updated
>>    * check zero 'max-pkt-len' value
>>    * Update how queue offload flags updated
>>    * Update MTU if JUMBO_FRAME flag is not set
>> * Default testpmd 'max-pkt-len' value set to zero
>>
>> Cc: lance.richardson@broadcom.com
>> Cc: oulijun@huawei.com
>> Cc: wisamm@mellanox.com
>> Cc: lihuisong@huawei.com
>> ---
> 
> I think we need to have https://bugs.dpdk.org/show_bug.cgi?id=625 ID in the commit log as fix,


Sure, I will send a new version with suggested updates, most probably today, I 
can add the Bugzilla information too.

> In order to allow the scripts to close related bugs directly from Bugzilla.
> 

Scripts? Do we have scripts that close defects automatically, I wasn't aware of 
it, where does it run?


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-stable] [PATCH v5] app/testpmd: fix setting maximum packet length
  2021-01-28  9:26     ` Ferruh Yigit
@ 2021-01-28 11:08       ` Wisam Monther
  0 siblings, 0 replies; 17+ messages in thread
From: Wisam Monther @ 2021-01-28 11:08 UTC (permalink / raw)
  To: Ferruh Yigit, Wenzhuo Lu, Xiaoyun Li, Bernard Iremonger, Steve Yang
  Cc: dev, stable, lance.richardson, oulijun, lihuisong



> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Thursday, January 28, 2021 11:27 AM
> To: Wisam Monther <wisamm@nvidia.com>; Wenzhuo Lu
> <wenzhuo.lu@intel.com>; Xiaoyun Li <xiaoyun.li@intel.com>; Bernard
> Iremonger <bernard.iremonger@intel.com>; Steve Yang
> <stevex.yang@intel.com>
> Cc: dev@dpdk.org; stable@dpdk.org; lance.richardson@broadcom.com;
> oulijun@huawei.com; lihuisong@huawei.com
> Subject: Re: [PATCH v5] app/testpmd: fix setting maximum packet length
> 
> On 1/28/2021 9:18 AM, Wisam Monther wrote:
> > Hi Ferruh,
> >
> >
> >> -----Original Message-----
> >> From: Ferruh Yigit <ferruh.yigit@intel.com>
> >> Sent: Monday, January 25, 2021 8:16 PM
> >> To: Wenzhuo Lu <wenzhuo.lu@intel.com>; Xiaoyun Li
> >> <xiaoyun.li@intel.com>; Bernard Iremonger
> >> <bernard.iremonger@intel.com>; Steve Yang <stevex.yang@intel.com>
> >> Cc: Ferruh Yigit <ferruh.yigit@intel.com>; dev@dpdk.org;
> >> stable@dpdk.org; lance.richardson@broadcom.com;
> oulijun@huawei.com;
> >> Wisam Monther <wisamm@nvidia.com>; lihuisong@huawei.com
> >> Subject: [PATCH v5] app/testpmd: fix setting maximum packet length
> >>
> >> From: Steve Yang <stevex.yang@intel.com>
> >>
> >> "port config all max-pkt-len" command fails because it doesn't set
> >> the 'DEV_RX_OFFLOAD_JUMBO_FRAME' offload flag properly.
> >>
> >> Commit in the fixes line moved the 'DEV_RX_OFFLOAD_JUMBO_FRAME'
> >> offload flag update from 'cmd_config_max_pkt_len_parsed()' to
> >> 'init_config()'.
> >> 'init_config()' function is only called during testpmd startup, but
> >> the flag status needs to be calculated whenever 'max_rx_pkt_len'
> changes.
> >>
> >> The issue can be reproduce as [1], where the 'max-pkt-len' reduced
> >> and 'DEV_RX_OFFLOAD_JUMBO_FRAME' offload flag should be cleared
> but
> >> it didn't.
> >>
> >> Adding the 'update_jumbo_frame_offload()' helper function to update
> >> 'DEV_RX_OFFLOAD_JUMBO_FRAME' offload flag and 'max_rx_pkt_len'.
> This
> >> function is called both by 'init_config()' and
> >> 'cmd_config_max_pkt_len_parsed()'.
> >>
> >> Default 'max-pkt-len' value set to zero, 'update_jumbo_frame_offload()'
> >> updates it to "RTE_ETHER_MTU + PMD specific Ethernet overhead" when
> >> it is zero.
> >> If '--max-pkt-len=N' argument provided, it will be used instead.
> >> And with each "port config all max-pkt-len" command, the
> >> 'DEV_RX_OFFLOAD_JUMBO_FRAME' offload flag, 'max-pkt-len' and MTU
> is
> >> updated.
> >>
> >> [1]
> >> ---------------------------------------------------------------------
> >> ----- dpdk-testpmd -c 0xf -n 4 -- -i --max-pkt-len=9000
> >> --tx-offloads=0x8000
> >> 	--rxq=4 --txq=4 --disable-rss
> >> testpmd>  set verbose 3
> >> testpmd>  port stop all
> >> testpmd>  port config all max-pkt-len 1518  port start all
> >>
> >> // Got fail error info without this patch Configuring Port 0 (socket
> >> 1) Ethdev
> >> port_id=0 rx_queue_id=0, new added offloads 0x800 must be within per-
> >> queue offload capabilities 0x0 in rte_eth_rx_queue_setup() Fail to
> >> configure port 0 rx queues //<-- Fail error info;
> >> ---------------------------------------------------------------------
> >> -----
> >>
> >> Fixes: 761c4d66900f ("app/testpmd: fix max Rx packet length for VLAN
> >> packets")
> >> Cc: stable@dpdk.org
> >>
> >> Signed-off-by: Steve Yang <stevex.yang@intel.com>
> >> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> >> ---
> >>
> >> v5:
> >> * 'update_jumbo_frame_offload()' helper updated
> >>    * check zero 'max-pkt-len' value
> >>    * Update how queue offload flags updated
> >>    * Update MTU if JUMBO_FRAME flag is not set
> >> * Default testpmd 'max-pkt-len' value set to zero
> >>
> >> Cc: lance.richardson@broadcom.com
> >> Cc: oulijun@huawei.com
> >> Cc: wisamm@mellanox.com
> >> Cc: lihuisong@huawei.com
> >> ---
> >
> > I think we need to have https://bugs.dpdk.org/show_bug.cgi?id=625 ID
> > in the commit log as fix,
> 
> 
> Sure, I will send a new version with suggested updates, most probably today,
> I can add the Bugzilla information too.
> 
> > In order to allow the scripts to close related bugs directly from Bugzilla.
> >
> 
> Scripts? Do we have scripts that close defects automatically, I wasn't aware of
> it, where does it run?

It's something Thomas using and run it every time to time, it's not published yet.
But it closes merged bugs, adding a link to the commit as comment.



^ permalink raw reply	[flat|nested] 17+ messages in thread

* [dpdk-stable] [PATCH v6] app/testpmd: fix setting maximum packet length
  2021-01-25 18:15 ` [dpdk-stable] [PATCH v5] app/testpmd: fix setting maximum packet length Ferruh Yigit
                     ` (3 preceding siblings ...)
  2021-01-28  9:18   ` Wisam Monther
@ 2021-01-28 12:07   ` Ferruh Yigit
  2021-01-29  9:34     ` Ferruh Yigit
  4 siblings, 1 reply; 17+ messages in thread
From: Ferruh Yigit @ 2021-01-28 12:07 UTC (permalink / raw)
  To: Wenzhuo Lu, Xiaoyun Li, Bernard Iremonger, Steve Yang
  Cc: Ferruh Yigit, dev, stable, Lance Richardson, Wisam Jaddo,
	Bo Chen, oulijun, wisamm, lihuisong

From: Steve Yang <stevex.yang@intel.com>

"port config all max-pkt-len" command fails because it doesn't set the
'DEV_RX_OFFLOAD_JUMBO_FRAME' offload flag properly.

Commit in the fixes line moved the 'DEV_RX_OFFLOAD_JUMBO_FRAME' offload
flag update from 'cmd_config_max_pkt_len_parsed()' to 'init_config()'.
'init_config()' function is only called during testpmd startup, but the
flag status needs to be calculated whenever 'max_rx_pkt_len' changes.

The issue can be reproduced as [1], where the 'max-pkt-len' reduced and
'DEV_RX_OFFLOAD_JUMBO_FRAME' offload flag should be cleared but it
didn't.

Adding the 'update_jumbo_frame_offload()' helper function to update
'DEV_RX_OFFLOAD_JUMBO_FRAME' offload flag and 'max_rx_pkt_len'. This
function is called both by 'init_config()' and
'cmd_config_max_pkt_len_parsed()'.

Default 'max-pkt-len' value set to zero, 'update_jumbo_frame_offload()'
updates it to "RTE_ETHER_MTU + PMD specific Ethernet overhead" when it
is zero.
If '--max-pkt-len=N' argument provided, it will be used instead.
And with each "port config all max-pkt-len" command, the
'DEV_RX_OFFLOAD_JUMBO_FRAME' offload flag, 'max-pkt-len' and MTU is
updated.

[1]
--------------------------------------------------------------------------
dpdk-testpmd -c 0xf -n 4 -- -i --max-pkt-len=9000 --tx-offloads=0x8000
	--rxq=4 --txq=4 --disable-rss
testpmd>  set verbose 3
testpmd>  port stop all
testpmd>  port config all max-pkt-len 1518
testpmd>  port start all

// Got fail error info without this patch
Configuring Port 0 (socket 1)
Ethdev port_id=0 rx_queue_id=0, new added offloads 0x800 must be
within per-queue offload capabilities 0x0 in rte_eth_rx_queue_setup()
Fail to configure port 0 rx queues //<-- Fail error info;
--------------------------------------------------------------------------

Bugzilla ID: 625
Fixes: 761c4d66900f ("app/testpmd: fix max Rx packet length for VLAN packets")
Cc: stable@dpdk.org

Signed-off-by: Steve Yang <stevex.yang@intel.com>
Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
Acked-by: Lance Richardson <lance.richardson@broadcom.com>
Acked-by: Wisam Jaddo <wisamm@nvidia.com>
Acked-by: Xiaoyun Li <xiaoyun.li@intel.com>
Tested-by: Bo Chen <box.c.chen@intel.com>
---
v5:
* 'update_jumbo_frame_offload()' helper updated
  * check zero 'max-pkt-len' value
  * Update how queue offload flags updated
  * Update MTU if JUMBO_FRAME flag is not set
* Default testpmd 'max-pkt-len' value set to zero

Cc: lance.richardson@broadcom.com
Cc: oulijun@huawei.com
Cc: wisamm@mellanox.com
Cc: lihuisong@huawei.com

v6:
* Comment wording updated
* Bugzilla id added to the commit log
---
 app/test-pmd/cmdline.c |  13 ++++++
 app/test-pmd/testpmd.c | 102 +++++++++++++++++++++++++++++++++--------
 app/test-pmd/testpmd.h |   1 +
 3 files changed, 97 insertions(+), 19 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index c6ace2dd2546..b338f5fe6333 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -1877,7 +1877,9 @@ cmd_config_max_pkt_len_parsed(void *parsed_result,
 				__rte_unused void *data)
 {
 	struct cmd_config_max_pkt_len_result *res = parsed_result;
+	uint32_t max_rx_pkt_len_backup = 0;
 	portid_t pid;
+	int ret;
 
 	if (!all_ports_stopped()) {
 		printf("Please stop all ports first\n");
@@ -1896,7 +1898,18 @@ cmd_config_max_pkt_len_parsed(void *parsed_result,
 			if (res->value == port->dev_conf.rxmode.max_rx_pkt_len)
 				return;
 
+			ret = eth_dev_info_get_print_err(pid, &port->dev_info);
+			if (ret != 0) {
+				printf("rte_eth_dev_info_get() failed for port %u\n",
+					pid);
+				return;
+			}
+
+			max_rx_pkt_len_backup = port->dev_conf.rxmode.max_rx_pkt_len;
+
 			port->dev_conf.rxmode.max_rx_pkt_len = res->value;
+			if (update_jumbo_frame_offload(pid) != 0)
+				port->dev_conf.rxmode.max_rx_pkt_len = max_rx_pkt_len_backup;
 		} else {
 			printf("Unknown parameter\n");
 			return;
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index c256e719aea2..555852ae5e42 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -443,8 +443,11 @@ lcoreid_t latencystats_lcore_id = -1;
  * Ethernet device configuration.
  */
 struct rte_eth_rxmode rx_mode = {
-	.max_rx_pkt_len = RTE_ETHER_MAX_LEN,
-		/**< Default maximum frame length. */
+	/* Default maximum frame length.
+	 * Zero is converted to "RTE_ETHER_MTU + PMD Ethernet overhead"
+	 * in init_config().
+	 */
+	.max_rx_pkt_len = 0,
 };
 
 struct rte_eth_txmode tx_mode = {
@@ -1410,7 +1413,6 @@ init_config(void)
 	struct rte_gro_param gro_param;
 	uint32_t gso_types;
 	uint16_t data_size;
-	uint16_t eth_overhead;
 	bool warning = 0;
 	int k;
 	int ret;
@@ -1447,22 +1449,10 @@ init_config(void)
 			rte_exit(EXIT_FAILURE,
 				 "rte_eth_dev_info_get() failed\n");
 
-		/* Update the max_rx_pkt_len to have MTU as RTE_ETHER_MTU */
-		if (port->dev_info.max_mtu != UINT16_MAX &&
-		    port->dev_info.max_rx_pktlen > port->dev_info.max_mtu)
-			eth_overhead = port->dev_info.max_rx_pktlen -
-				port->dev_info.max_mtu;
-		else
-			eth_overhead =
-				RTE_ETHER_HDR_LEN + RTE_ETHER_CRC_LEN;
-
-		if (port->dev_conf.rxmode.max_rx_pkt_len <=
-			(uint32_t)(RTE_ETHER_MTU + eth_overhead))
-			port->dev_conf.rxmode.max_rx_pkt_len =
-					RTE_ETHER_MTU + eth_overhead;
-		else
-			port->dev_conf.rxmode.offloads |=
-					DEV_RX_OFFLOAD_JUMBO_FRAME;
+		ret = update_jumbo_frame_offload(pid);
+		if (ret != 0)
+			printf("Updating jumbo frame offload failed for port %u\n",
+				pid);
 
 		if (!(port->dev_info.tx_offload_capa &
 		      DEV_TX_OFFLOAD_MBUF_FAST_FREE))
@@ -3358,6 +3348,80 @@ rxtx_port_config(struct rte_port *port)
 	}
 }
 
+/*
+ * Helper function to arrange max_rx_pktlen value and JUMBO_FRAME offload,
+ * MTU is also aligned if JUMBO_FRAME offload is not set.
+ *
+ * port->dev_info should be set before calling this function.
+ *
+ * return 0 on success, negative on error
+ */
+int
+update_jumbo_frame_offload(portid_t portid)
+{
+	struct rte_port *port = &ports[portid];
+	uint32_t eth_overhead;
+	uint64_t rx_offloads;
+	int ret;
+	bool on;
+
+	/* Update the max_rx_pkt_len to have MTU as RTE_ETHER_MTU */
+	if (port->dev_info.max_mtu != UINT16_MAX &&
+	    port->dev_info.max_rx_pktlen > port->dev_info.max_mtu)
+		eth_overhead = port->dev_info.max_rx_pktlen -
+				port->dev_info.max_mtu;
+	else
+		eth_overhead = RTE_ETHER_HDR_LEN + RTE_ETHER_CRC_LEN;
+
+	rx_offloads = port->dev_conf.rxmode.offloads;
+
+	/* Default config value is 0 to use PMD specific overhead */
+	if (port->dev_conf.rxmode.max_rx_pkt_len == 0)
+		port->dev_conf.rxmode.max_rx_pkt_len = RTE_ETHER_MTU + eth_overhead;
+
+	if (port->dev_conf.rxmode.max_rx_pkt_len <= RTE_ETHER_MTU + eth_overhead) {
+		rx_offloads &= ~DEV_RX_OFFLOAD_JUMBO_FRAME;
+		on = false;
+	} else {
+		if ((port->dev_info.rx_offload_capa & DEV_RX_OFFLOAD_JUMBO_FRAME) == 0) {
+			printf("Frame size (%u) is not supported by port %u\n",
+				port->dev_conf.rxmode.max_rx_pkt_len,
+				portid);
+			return -1;
+		}
+		rx_offloads |= DEV_RX_OFFLOAD_JUMBO_FRAME;
+		on = true;
+	}
+
+	if (rx_offloads != port->dev_conf.rxmode.offloads) {
+		uint16_t qid;
+
+		port->dev_conf.rxmode.offloads = rx_offloads;
+
+		/* Apply JUMBO_FRAME offload configuration to Rx queue(s) */
+		for (qid = 0; qid < port->dev_info.nb_rx_queues; qid++) {
+			if (on)
+				port->rx_conf[qid].offloads |= DEV_RX_OFFLOAD_JUMBO_FRAME;
+			else
+				port->rx_conf[qid].offloads &= ~DEV_RX_OFFLOAD_JUMBO_FRAME;
+		}
+	}
+
+	/* If JUMBO_FRAME is set MTU conversion done by ethdev layer,
+	 * if unset do it here
+	 */
+	if ((rx_offloads & DEV_RX_OFFLOAD_JUMBO_FRAME) == 0) {
+		ret = rte_eth_dev_set_mtu(portid,
+				port->dev_conf.rxmode.max_rx_pkt_len - eth_overhead);
+		if (ret)
+			printf("Failed to set MTU to %u for port %u\n",
+				port->dev_conf.rxmode.max_rx_pkt_len - eth_overhead,
+				portid);
+	}
+
+	return 0;
+}
+
 void
 init_port_config(void)
 {
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 5f2316210726..2f8f5a92e46a 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -1005,6 +1005,7 @@ uint16_t tx_pkt_set_dynf(uint16_t port_id, __rte_unused uint16_t queue,
 			 __rte_unused void *user_param);
 void add_tx_dynf_callback(portid_t portid);
 void remove_tx_dynf_callback(portid_t portid);
+int update_jumbo_frame_offload(portid_t portid);
 
 /*
  * Work-around of a compilation error with ICC on invocations of the
-- 
2.29.2


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-stable] [PATCH v5] app/testpmd: fix setting maximum packet length
  2021-01-26 11:01         ` Ferruh Yigit
@ 2021-01-28 21:36           ` Lance Richardson
  2021-01-28 22:12             ` Ferruh Yigit
  0 siblings, 1 reply; 17+ messages in thread
From: Lance Richardson @ 2021-01-28 21:36 UTC (permalink / raw)
  To: Ferruh Yigit
  Cc: Wenzhuo Lu, Xiaoyun Li, Bernard Iremonger, Steve Yang, dev,
	stable, oulijun, wisamm, lihuisong

[-- Attachment #1: Type: text/plain, Size: 3342 bytes --]

On Tue, Jan 26, 2021 at 6:01 AM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>
> On 1/26/2021 3:45 AM, Lance Richardson wrote:
> > On Mon, Jan 25, 2021 at 7:44 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> >>
> >>>> +       if (rx_offloads != port->dev_conf.rxmode.offloads) {
> >>>> +               uint16_t qid;
> >>>> +
> >>>> +               port->dev_conf.rxmode.offloads = rx_offloads;
> >>>> +
> >>>> +               /* Apply JUMBO_FRAME offload configuration to Rx queue(s) */
> >>>> +               for (qid = 0; qid < port->dev_info.nb_rx_queues; qid++) {
> >>>> +                       if (on)
> >>>> +                               port->rx_conf[qid].offloads |= DEV_RX_OFFLOAD_JUMBO_FRAME;
> >>>> +                       else
> >>>> +                               port->rx_conf[qid].offloads &= ~DEV_RX_OFFLOAD_JUMBO_FRAME;
> >>>> +               }
> >>>
> >>> Is it correct to set per-queue offloads that aren't advertised by the PMD
> >>> as supported in rx_queue_offload_capa?
> >>>
> >>
> >> 'port->rx_conf[]' is testpmd struct, and 'port->dev_conf.rxmode.offloads' values
> >> are reflected to 'port->rx_conf[].offloads' for all queues.
> >>
> >> We should set the offload in 'port->rx_conf[].offloads' if it is set in
> >> 'port->dev_conf.rxmode.offloads'.
> >>
> >> If a port has capability for 'JUMBO_FRAME', 'port->rx_conf[].offloads' can have
> >> it. And the port level capability is already checked above.
> >>
> >
> > I'm still not 100% clear about the per-queue offload question.
> >
> > With this patch, and jumbo max packet size configured (on the command
> > line in this case), I see:
> >
> > testpmd> show port 0 rx_offload configuration
> > Rx Offloading Configuration of port 0 :
> >    Port : JUMBO_FRAME
> >    Queue[ 0] : JUMBO_FRAME
> >
> > testpmd> show port 0 rx_offload capabilities
> > Rx Offloading Capabilities of port 0 :
> >    Per Queue :
> >    Per Port  : VLAN_STRIP IPV4_CKSUM UDP_CKSUM TCP_CKSUM TCP_LRO
> > OUTER_IPV4_CKSUM VLAN_FILTER VLAN_EXTEND JUMBO_FRAME SCATTER TIMESTAMP
> > KEEP_CRC OUTER_UDP_CKSUM RSS_HASH
> >
>
> The port level offload is applied to all queues on the port, testpmd config
> structure reflects this logic in implementation.
> If Rx offload X is set for a port, it is set for all Rx queue offloads, this is
> not new behavior and not related to this patch.
>
OK, is this purely for display purposes within testpmd? I ask because
it appears that all PMDs supporting per-queue offload configuration already
take care of combining port-level and per-queue offloads within their
tx_queue_setup()/rx_queue_setup() functions and then track the combined
set of offloads within a per-queue field, e.g. this line is common to
e1000/i40e/ionic/ixgbe/octeontx2/thunderx/txgbe rx_queue_setup()
implementations:
    offloads = rx_conf->offloads | dev->data->dev_conf.rxmode.offloads;

And rte_ethdev.h says:
   No need to repeat any bit in rx_conf->offloads which has already been
   enabled in rte_eth_dev_configure() at port level. An offloading enabled
  at port level can't be disabled at queue level.

Which I suppose confirms that if testpmd is combining per-port and per-
queue offloads, it's just for the purposes of testpmd.

Apologies for worrying at this even more, I just wanted to be sure that
I understand what the PMD is expected to do.

Regards,
    Lance

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-stable] [PATCH v5] app/testpmd: fix setting maximum packet length
  2021-01-28 21:36           ` Lance Richardson
@ 2021-01-28 22:12             ` Ferruh Yigit
  0 siblings, 0 replies; 17+ messages in thread
From: Ferruh Yigit @ 2021-01-28 22:12 UTC (permalink / raw)
  To: Lance Richardson
  Cc: Wenzhuo Lu, Xiaoyun Li, Bernard Iremonger, Steve Yang, dev,
	stable, oulijun, wisamm, lihuisong

On 1/28/2021 9:36 PM, Lance Richardson wrote:
> On Tue, Jan 26, 2021 at 6:01 AM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>>
>> On 1/26/2021 3:45 AM, Lance Richardson wrote:
>>> On Mon, Jan 25, 2021 at 7:44 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>>>>
>>>>>> +       if (rx_offloads != port->dev_conf.rxmode.offloads) {
>>>>>> +               uint16_t qid;
>>>>>> +
>>>>>> +               port->dev_conf.rxmode.offloads = rx_offloads;
>>>>>> +
>>>>>> +               /* Apply JUMBO_FRAME offload configuration to Rx queue(s) */
>>>>>> +               for (qid = 0; qid < port->dev_info.nb_rx_queues; qid++) {
>>>>>> +                       if (on)
>>>>>> +                               port->rx_conf[qid].offloads |= DEV_RX_OFFLOAD_JUMBO_FRAME;
>>>>>> +                       else
>>>>>> +                               port->rx_conf[qid].offloads &= ~DEV_RX_OFFLOAD_JUMBO_FRAME;
>>>>>> +               }
>>>>>
>>>>> Is it correct to set per-queue offloads that aren't advertised by the PMD
>>>>> as supported in rx_queue_offload_capa?
>>>>>
>>>>
>>>> 'port->rx_conf[]' is testpmd struct, and 'port->dev_conf.rxmode.offloads' values
>>>> are reflected to 'port->rx_conf[].offloads' for all queues.
>>>>
>>>> We should set the offload in 'port->rx_conf[].offloads' if it is set in
>>>> 'port->dev_conf.rxmode.offloads'.
>>>>
>>>> If a port has capability for 'JUMBO_FRAME', 'port->rx_conf[].offloads' can have
>>>> it. And the port level capability is already checked above.
>>>>
>>>
>>> I'm still not 100% clear about the per-queue offload question.
>>>
>>> With this patch, and jumbo max packet size configured (on the command
>>> line in this case), I see:
>>>
>>> testpmd> show port 0 rx_offload configuration
>>> Rx Offloading Configuration of port 0 :
>>>     Port : JUMBO_FRAME
>>>     Queue[ 0] : JUMBO_FRAME
>>>
>>> testpmd> show port 0 rx_offload capabilities
>>> Rx Offloading Capabilities of port 0 :
>>>     Per Queue :
>>>     Per Port  : VLAN_STRIP IPV4_CKSUM UDP_CKSUM TCP_CKSUM TCP_LRO
>>> OUTER_IPV4_CKSUM VLAN_FILTER VLAN_EXTEND JUMBO_FRAME SCATTER TIMESTAMP
>>> KEEP_CRC OUTER_UDP_CKSUM RSS_HASH
>>>
>>
>> The port level offload is applied to all queues on the port, testpmd config
>> structure reflects this logic in implementation.
>> If Rx offload X is set for a port, it is set for all Rx queue offloads, this is
>> not new behavior and not related to this patch.
>>
> OK, is this purely for display purposes within testpmd? I ask because
> it appears that all PMDs supporting per-queue offload configuration already
> take care of combining port-level and per-queue offloads within their
> tx_queue_setup()/rx_queue_setup() functions and then track the combined
> set of offloads within a per-queue field, e.g. this line is common to
> e1000/i40e/ionic/ixgbe/octeontx2/thunderx/txgbe rx_queue_setup()
> implementations:
>      offloads = rx_conf->offloads | dev->data->dev_conf.rxmode.offloads;
> 
> And rte_ethdev.h says:
>     No need to repeat any bit in rx_conf->offloads which has already been
>     enabled in rte_eth_dev_configure() at port level. An offloading enabled
>    at port level can't be disabled at queue level.
> 
> Which I suppose confirms that if testpmd is combining per-port and per-
> queue offloads, it's just for the purposes of testpmd.
> 

Yes it is just for purposed of testpmd (application), but doesn't need to be 
just limited to display purpose, testpmd keeps the config locally for any need.

> Apologies for worrying at this even more, I just wanted to be sure that
> I understand what the PMD is expected to do.
> 

PMDs should not be getting these offloads in queue setup, since ethdev layer 
strips the port level offloads, I mean:

if port level offloads: X, Y

And applications provide following for queue_setup():
a) X
b) Y
c) X, Y

For all three PMD receives empty queue offload request.

d) X, Y, Z

PMD gets Z if it is in drivers queue specific capabilities.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-stable] [PATCH v6] app/testpmd: fix setting maximum packet length
  2021-01-28 12:07   ` [dpdk-stable] [PATCH v6] " Ferruh Yigit
@ 2021-01-29  9:34     ` Ferruh Yigit
  0 siblings, 0 replies; 17+ messages in thread
From: Ferruh Yigit @ 2021-01-29  9:34 UTC (permalink / raw)
  To: Wenzhuo Lu, Xiaoyun Li, Bernard Iremonger, Steve Yang
  Cc: dev, stable, Lance Richardson, Wisam Jaddo, Bo Chen, oulijun,
	wisamm, lihuisong

On 1/28/2021 12:07 PM, Ferruh Yigit wrote:
> From: Steve Yang <stevex.yang@intel.com>
> 
> "port config all max-pkt-len" command fails because it doesn't set the
> 'DEV_RX_OFFLOAD_JUMBO_FRAME' offload flag properly.
> 
> Commit in the fixes line moved the 'DEV_RX_OFFLOAD_JUMBO_FRAME' offload
> flag update from 'cmd_config_max_pkt_len_parsed()' to 'init_config()'.
> 'init_config()' function is only called during testpmd startup, but the
> flag status needs to be calculated whenever 'max_rx_pkt_len' changes.
> 
> The issue can be reproduced as [1], where the 'max-pkt-len' reduced and
> 'DEV_RX_OFFLOAD_JUMBO_FRAME' offload flag should be cleared but it
> didn't.
> 
> Adding the 'update_jumbo_frame_offload()' helper function to update
> 'DEV_RX_OFFLOAD_JUMBO_FRAME' offload flag and 'max_rx_pkt_len'. This
> function is called both by 'init_config()' and
> 'cmd_config_max_pkt_len_parsed()'.
> 
> Default 'max-pkt-len' value set to zero, 'update_jumbo_frame_offload()'
> updates it to "RTE_ETHER_MTU + PMD specific Ethernet overhead" when it
> is zero.
> If '--max-pkt-len=N' argument provided, it will be used instead.
> And with each "port config all max-pkt-len" command, the
> 'DEV_RX_OFFLOAD_JUMBO_FRAME' offload flag, 'max-pkt-len' and MTU is
> updated.
> 
> [1]
> --------------------------------------------------------------------------
> dpdk-testpmd -c 0xf -n 4 -- -i --max-pkt-len=9000 --tx-offloads=0x8000
> 	--rxq=4 --txq=4 --disable-rss
> testpmd>  set verbose 3
> testpmd>  port stop all
> testpmd>  port config all max-pkt-len 1518
> testpmd>  port start all
> 
> // Got fail error info without this patch
> Configuring Port 0 (socket 1)
> Ethdev port_id=0 rx_queue_id=0, new added offloads 0x800 must be
> within per-queue offload capabilities 0x0 in rte_eth_rx_queue_setup()
> Fail to configure port 0 rx queues //<-- Fail error info;
> --------------------------------------------------------------------------
> 
> Bugzilla ID: 625
> Fixes: 761c4d66900f ("app/testpmd: fix max Rx packet length for VLAN packets")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Steve Yang <stevex.yang@intel.com>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> Acked-by: Lance Richardson <lance.richardson@broadcom.com>
> Acked-by: Wisam Jaddo <wisamm@nvidia.com>
> Acked-by: Xiaoyun Li <xiaoyun.li@intel.com>
> Tested-by: Bo Chen <box.c.chen@intel.com>

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


^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2021-01-29  9:34 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20210125083202.38267-1-stevex.yang@intel.com>
2021-01-25 18:15 ` [dpdk-stable] [PATCH v5] app/testpmd: fix setting maximum packet length Ferruh Yigit
2021-01-25 19:41   ` Lance Richardson
2021-01-26  0:44     ` Ferruh Yigit
2021-01-26  3:22       ` Lance Richardson
2021-01-26  3:45       ` Lance Richardson
2021-01-26  7:54         ` Li, Xiaoyun
2021-01-26 11:01         ` Ferruh Yigit
2021-01-28 21:36           ` Lance Richardson
2021-01-28 22:12             ` Ferruh Yigit
2021-01-26  9:02   ` Wisam Monther
2021-01-27  3:04   ` Li, Xiaoyun
2021-01-28  1:57     ` Chen, BoX C
2021-01-28  9:18   ` Wisam Monther
2021-01-28  9:26     ` Ferruh Yigit
2021-01-28 11:08       ` Wisam Monther
2021-01-28 12:07   ` [dpdk-stable] [PATCH v6] " Ferruh Yigit
2021-01-29  9:34     ` Ferruh Yigit

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).