diff mbox series

app/testpmd: fix MTU configuration before device start

Message ID 20211024164237.2116166-1-andrew.rybchenko@oktetlabs.ru (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers
Series app/testpmd: fix MTU configuration before device start |

Checks

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

Commit Message

Andrew Rybchenko Oct. 24, 2021, 4:42 p.m. UTC 
  There is no point to do rte_eth_dev_mtu_set() before configure since
set MTU value is overwritten on configure anyway. So, setting of MTU
before configure is rejected now on ethdev level.

If testpmd is going to do configure (e.g. just after testpmd start
with disabled devices start up or any configuration changes in stopped
state which require reconfigure), just save requested MTU in device
config to be applied on reconfigure.

Fixes: 1bb4a528c41f ("ethdev: fix max Rx packet length")
Fixes: be0433cf953c ("ethdev: forbid MTU set before device configure")

Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
---
 app/test-pmd/config.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

Comments

Ajit Khaparde Oct. 25, 2021, 6:06 p.m. UTC | #1 
On Sun, Oct 24, 2021 at 9:42 AM Andrew Rybchenko
<andrew.rybchenko@oktetlabs.ru> wrote:
>
> There is no point to do rte_eth_dev_mtu_set() before configure since
> set MTU value is overwritten on configure anyway. So, setting of MTU
> before configure is rejected now on ethdev level.
>
> If testpmd is going to do configure (e.g. just after testpmd start
> with disabled devices start up or any configuration changes in stopped
> state which require reconfigure), just save requested MTU in device
> config to be applied on reconfigure.
>
> Fixes: 1bb4a528c41f ("ethdev: fix max Rx packet length")
> Fixes: be0433cf953c ("ethdev: forbid MTU set before device configure")
>
> Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>

> ---
>  app/test-pmd/config.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index a18871d461..3b97164302 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -1241,10 +1241,12 @@ port_mtu_set(portid_t port_id, uint16_t mtu)
>         if (port_id_is_invalid(port_id, ENABLED_WARN))
>                 return;
>
> -       diag = rte_eth_dev_set_mtu(port_id, mtu);
> -       if (diag != 0) {
> -               fprintf(stderr, "Set MTU failed. diag=%d\n", diag);
> -               return;
> +       if (port->need_reconfig == 0) {
> +               diag = rte_eth_dev_set_mtu(port_id, mtu);
> +               if (diag != 0) {
> +                       fprintf(stderr, "Set MTU failed. diag=%d\n", diag);
> +                       return;
> +               }
>         }
>
>         port->dev_conf.rxmode.mtu = mtu;
> --
> 2.30.2
>
Ferruh Yigit Oct. 28, 2021, 6:23 p.m. UTC | #2 
On 10/25/2021 7:06 PM, Ajit Khaparde wrote:
> On Sun, Oct 24, 2021 at 9:42 AM Andrew Rybchenko
> <andrew.rybchenko@oktetlabs.ru> wrote:
>>
>> There is no point to do rte_eth_dev_mtu_set() before configure since
>> set MTU value is overwritten on configure anyway. So, setting of MTU
>> before configure is rejected now on ethdev level.
>>
>> If testpmd is going to do configure (e.g. just after testpmd start
>> with disabled devices start up or any configuration changes in stopped
>> state which require reconfigure), just save requested MTU in device
>> config to be applied on reconfigure.
>>
>> Fixes: 1bb4a528c41f ("ethdev: fix max Rx packet length")
>> Fixes: be0433cf953c ("ethdev: forbid MTU set before device configure")
>>
>> Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> 

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

Applied to dpdk-next-net/main, thanks.
Yu Jiang Nov. 2, 2021, 2:37 a.m. UTC | #3 
> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Ferruh Yigit
> Sent: Friday, October 29, 2021 2:23 AM
> To: Ajit Khaparde <ajit.khaparde@broadcom.com>; Andrew Rybchenko
> <andrew.rybchenko@oktetlabs.ru>
> Cc: Li, Xiaoyun <xiaoyun.li@intel.com>; Xu, Rosen <rosen.xu@intel.com>;
> Ananyev, Konstantin <konstantin.ananyev@intel.com>; Ivan Ilchenko
> <ivan.ilchenko@oktetlabs.ru>; dpdk-dev <dev@dpdk.org>
> Subject: Re: [dpdk-dev] [PATCH] app/testpmd: fix MTU configuration before
> device start
> 
> On 10/25/2021 7:06 PM, Ajit Khaparde wrote:
> > On Sun, Oct 24, 2021 at 9:42 AM Andrew Rybchenko
> > <andrew.rybchenko@oktetlabs.ru> wrote:
> >>
> >> There is no point to do rte_eth_dev_mtu_set() before configure since
> >> set MTU value is overwritten on configure anyway. So, setting of MTU
> >> before configure is rejected now on ethdev level.
> >>
> >> If testpmd is going to do configure (e.g. just after testpmd start
> >> with disabled devices start up or any configuration changes in
> >> stopped state which require reconfigure), just save requested MTU in
> >> device config to be applied on reconfigure.
> >>
> >> Fixes: 1bb4a528c41f ("ethdev: fix max Rx packet length")
> >> Fixes: be0433cf953c ("ethdev: forbid MTU set before device
> >> configure")
> >>
> >> Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> > Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> >
> 
> Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
> 
> Applied to dpdk-next-net/main, thanks.
verify patch https://patches.dpdk.org/project/dpdk/patch/20211024164237.2116166-1-andrew.rybchenko@oktetlabs.ru/ on baseline 6cc51b1293ceac4a77d4bf7ac91a8bbd59e1f78c
Test failed on nic XXV710 for 25GbE SFP28 158b/os Red Hat Enterprise Linux 8.4
Yu Jiang Nov. 10, 2021, 7:23 a.m. UTC | #4 
> -----Original Message-----
> From: Jiang, YuX
> Sent: Tuesday, November 2, 2021 10:38 AM
> To: 'Ferruh Yigit' <ferruh.yigit@intel.com>; Ajit Khaparde
> <ajit.khaparde@broadcom.com>; Andrew Rybchenko
> <andrew.rybchenko@oktetlabs.ru>
> Cc: Li, Xiaoyun <xiaoyun.li@intel.com>; Xu, Rosen <rosen.xu@intel.com>;
> Ananyev, Konstantin <konstantin.ananyev@intel.com>; Ivan Ilchenko
> <ivan.ilchenko@oktetlabs.ru>; dpdk-dev <dev@dpdk.org>
> Subject: RE: [dpdk-dev] [PATCH] app/testpmd: fix MTU configuration before
> device start
> 
> > -----Original Message-----
> > From: dev <dev-bounces@dpdk.org> On Behalf Of Ferruh Yigit
> > Sent: Friday, October 29, 2021 2:23 AM
> > To: Ajit Khaparde <ajit.khaparde@broadcom.com>; Andrew Rybchenko
> > <andrew.rybchenko@oktetlabs.ru>
> > Cc: Li, Xiaoyun <xiaoyun.li@intel.com>; Xu, Rosen
> > <rosen.xu@intel.com>; Ananyev, Konstantin
> > <konstantin.ananyev@intel.com>; Ivan Ilchenko
> > <ivan.ilchenko@oktetlabs.ru>; dpdk-dev <dev@dpdk.org>
> > Subject: Re: [dpdk-dev] [PATCH] app/testpmd: fix MTU configuration
> > before device start
> >
> > On 10/25/2021 7:06 PM, Ajit Khaparde wrote:
> > > On Sun, Oct 24, 2021 at 9:42 AM Andrew Rybchenko
> > > <andrew.rybchenko@oktetlabs.ru> wrote:
> > >>
> > >> There is no point to do rte_eth_dev_mtu_set() before configure
> > >> since set MTU value is overwritten on configure anyway. So, setting
> > >> of MTU before configure is rejected now on ethdev level.
> > >>
> > >> If testpmd is going to do configure (e.g. just after testpmd start
> > >> with disabled devices start up or any configuration changes in
> > >> stopped state which require reconfigure), just save requested MTU
> > >> in device config to be applied on reconfigure.
> > >>
> > >> Fixes: 1bb4a528c41f ("ethdev: fix max Rx packet length")
> > >> Fixes: be0433cf953c ("ethdev: forbid MTU set before device
> > >> configure")
> > >>
> > >> Signed-off-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> > > Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> > >
> >
> > Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
> >
> > Applied to dpdk-next-net/main, thanks.
> verify patch
> https://patches.dpdk.org/project/dpdk/patch/20211024164237.2116166-1-
> andrew.rybchenko@oktetlabs.ru/ on baseline
> 6cc51b1293ceac4a77d4bf7ac91a8bbd59e1f78c
> Test failed on nic XXV710 for 25GbE SFP28 158b/os Red Hat Enterprise Linux
> 8.4

Hi Ferruh and Andrew,
Since the patch https://patches.dpdk.org/project/dpdk/patch/20211024164237.2116166-1-andrew.rybchenko@oktetlabs.ru/
Has been merged into dpdk21.11rc2 main branch, we verify the main, 
This bug(https://bugs.dpdk.org/show_bug.cgi?id=864) also can be reproduced, could you pls have a look again?
diff mbox series

Patch

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index a18871d461..3b97164302 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -1241,10 +1241,12 @@  port_mtu_set(portid_t port_id, uint16_t mtu)
 	if (port_id_is_invalid(port_id, ENABLED_WARN))
 		return;
 
-	diag = rte_eth_dev_set_mtu(port_id, mtu);
-	if (diag != 0) {
-		fprintf(stderr, "Set MTU failed. diag=%d\n", diag);
-		return;
+	if (port->need_reconfig == 0) {
+		diag = rte_eth_dev_set_mtu(port_id, mtu);
+		if (diag != 0) {
+			fprintf(stderr, "Set MTU failed. diag=%d\n", diag);
+			return;
+		}
 	}
 
 	port->dev_conf.rxmode.mtu = mtu;