* [PATCH 0/2] bugfix for testpmd
@ 2022-04-06 8:45 Min Hu (Connor)
2022-04-06 8:45 ` [PATCH 1/2] app/testpmd: fix stats get when display fwd stats Min Hu (Connor)
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Min Hu (Connor) @ 2022-04-06 8:45 UTC (permalink / raw)
To: dev; +Cc: ferruh.yigit, thomas, xiaoyun.li, aman.deep.singh, yuying.zhang
This patch set contains two bugfixes for testpmd.
Huisong Li (1):
app/testpmd: fix incorrect MTU verification
Min Hu (Connor) (1):
app/testpmd: fix stats get when display fwd stats
app/test-pmd/cmdline.c | 4 ---
app/test-pmd/config.c | 65 ++++++++++++++++++++++++++++++++++++++++--
app/test-pmd/testpmd.c | 16 +++++++++--
3 files changed, 77 insertions(+), 8 deletions(-)
--
2.33.0
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH 1/2] app/testpmd: fix stats get when display fwd stats 2022-04-06 8:45 [PATCH 0/2] bugfix for testpmd Min Hu (Connor) @ 2022-04-06 8:45 ` Min Hu (Connor) 2022-04-27 12:15 ` Singh, Aman Deep 2022-04-06 8:45 ` [PATCH 2/2] app/testpmd: fix incorrect MTU verification Min Hu (Connor) 2022-04-25 6:52 ` [PATCH 0/2] bugfix for testpmd Min Hu (Connor) 2 siblings, 1 reply; 10+ messages in thread From: Min Hu (Connor) @ 2022-04-06 8:45 UTC (permalink / raw) To: dev; +Cc: ferruh.yigit, thomas, xiaoyun.li, aman.deep.singh, yuying.zhang In function 'fwd_stats_display', if function 'rte_eth_stats_get' fails, 'stats' is uncertainty value. The display result will be abnormal. This patch check the return value of 'rte_eth_stats_get' to avoid display abnormal stats. Fixes: 53324971a14e ("app/testpmd: display/clear forwarding stats on demand") Cc: stable@dpdk.org Signed-off-by: Min Hu (Connor) <humin29@huawei.com> --- app/test-pmd/config.c | 10 ++++++++-- app/test-pmd/testpmd.c | 16 ++++++++++++++-- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index cc8e7aa138..bd689f9f86 100644 --- a/app/test-pmd/config.c +++ b/app/test-pmd/config.c @@ -249,14 +249,20 @@ nic_stats_display(portid_t port_id) diff_ns; uint64_t mpps_rx, mpps_tx, mbps_rx, mbps_tx; struct rte_eth_stats stats; - static const char *nic_stats_border = "########################"; + int ret; if (port_id_is_invalid(port_id, ENABLED_WARN)) { print_valid_ports(); return; } - rte_eth_stats_get(port_id, &stats); + ret = rte_eth_stats_get(port_id, &stats); + if (ret != 0) { + fprintf(stderr, + "%s: Error: failed to get stats (port %u): %d", + __func__, port_id, ret); + return; + } printf("\n %s NIC statistics for port %-2d %s\n", nic_stats_border, port_id, nic_stats_border); diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index fe2ce19f99..79bb23264b 100644 --- a/app/test-pmd/testpmd.c +++ b/app/test-pmd/testpmd.c @@ -1982,6 +1982,7 @@ fwd_stats_display(void) struct rte_port *port; streamid_t sm_id; portid_t pt_id; + int ret; int i; memset(ports_stats, 0, sizeof(ports_stats)); @@ -2013,7 +2014,13 @@ fwd_stats_display(void) pt_id = fwd_ports_ids[i]; port = &ports[pt_id]; - rte_eth_stats_get(pt_id, &stats); + ret = rte_eth_stats_get(pt_id, &stats); + if (ret != 0) { + fprintf(stderr, + "%s: Error: failed to get stats (port %u): %d", + __func__, pt_id, ret); + continue; + } stats.ipackets -= port->stats.ipackets; stats.opackets -= port->stats.opackets; stats.ibytes -= port->stats.ibytes; @@ -2108,11 +2115,16 @@ fwd_stats_reset(void) { streamid_t sm_id; portid_t pt_id; + int ret; int i; for (i = 0; i < cur_fwd_config.nb_fwd_ports; i++) { pt_id = fwd_ports_ids[i]; - rte_eth_stats_get(pt_id, &ports[pt_id].stats); + ret = rte_eth_stats_get(pt_id, &ports[pt_id].stats); + if (ret != 0) + fprintf(stderr, + "%s: Error: failed to clear stats (port %u):%d", + __func__, pt_id, ret); } for (sm_id = 0; sm_id < cur_fwd_config.nb_fwd_streams; sm_id++) { struct fwd_stream *fs = fwd_streams[sm_id]; -- 2.33.0 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] app/testpmd: fix stats get when display fwd stats 2022-04-06 8:45 ` [PATCH 1/2] app/testpmd: fix stats get when display fwd stats Min Hu (Connor) @ 2022-04-27 12:15 ` Singh, Aman Deep 2022-04-28 3:10 ` Min Hu (Connor) 0 siblings, 1 reply; 10+ messages in thread From: Singh, Aman Deep @ 2022-04-27 12:15 UTC (permalink / raw) To: Min Hu (Connor), dev; +Cc: thomas, xiaoyun.li, yuying.zhang On 4/6/2022 2:15 PM, Min Hu (Connor) wrote: > In function 'fwd_stats_display', if function 'rte_eth_stats_get' fails, > 'stats' is uncertainty value. The display result will be abnormal. > > This patch check the return value of 'rte_eth_stats_get' to avoid > display abnormal stats. > > Fixes: 53324971a14e ("app/testpmd: display/clear forwarding stats on demand") > Cc: stable@dpdk.org > > Signed-off-by: Min Hu (Connor) <humin29@huawei.com> > --- > app/test-pmd/config.c | 10 ++++++++-- > app/test-pmd/testpmd.c | 16 ++++++++++++++-- > 2 files changed, 22 insertions(+), 4 deletions(-) > > diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c > index cc8e7aa138..bd689f9f86 100644 > --- a/app/test-pmd/config.c > +++ b/app/test-pmd/config.c > @@ -249,14 +249,20 @@ nic_stats_display(portid_t port_id) > diff_ns; > uint64_t mpps_rx, mpps_tx, mbps_rx, mbps_tx; > struct rte_eth_stats stats; > - > static const char *nic_stats_border = "########################"; > + int ret; > > if (port_id_is_invalid(port_id, ENABLED_WARN)) { > print_valid_ports(); > return; > } > - rte_eth_stats_get(port_id, &stats); > + ret = rte_eth_stats_get(port_id, &stats); > + if (ret != 0) { > + fprintf(stderr, > + "%s: Error: failed to get stats (port %u): %d", > + __func__, port_id, ret); > + return; > + } > printf("\n %s NIC statistics for port %-2d %s\n", > nic_stats_border, port_id, nic_stats_border); > > diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c > index fe2ce19f99..79bb23264b 100644 > --- a/app/test-pmd/testpmd.c > +++ b/app/test-pmd/testpmd.c > @@ -1982,6 +1982,7 @@ fwd_stats_display(void) > struct rte_port *port; > streamid_t sm_id; > portid_t pt_id; > + int ret; > int i; > > memset(ports_stats, 0, sizeof(ports_stats)); > @@ -2013,7 +2014,13 @@ fwd_stats_display(void) > pt_id = fwd_ports_ids[i]; > port = &ports[pt_id]; > > - rte_eth_stats_get(pt_id, &stats); > + ret = rte_eth_stats_get(pt_id, &stats); > + if (ret != 0) { > + fprintf(stderr, > + "%s: Error: failed to get stats (port %u): %d", > + __func__, pt_id, ret); > + continue; > + } > stats.ipackets -= port->stats.ipackets; > stats.opackets -= port->stats.opackets; > stats.ibytes -= port->stats.ibytes; > @@ -2108,11 +2115,16 @@ fwd_stats_reset(void) > { > streamid_t sm_id; > portid_t pt_id; > + int ret; > int i; > > for (i = 0; i < cur_fwd_config.nb_fwd_ports; i++) { > pt_id = fwd_ports_ids[i]; > - rte_eth_stats_get(pt_id, &ports[pt_id].stats); > + ret = rte_eth_stats_get(pt_id, &ports[pt_id].stats); > + if (ret != 0) > + fprintf(stderr, > + "%s: Error: failed to clear stats (port %u):%d", > + __func__, pt_id, ret); Should we clear "ports[pt_id].stats" in this condition. > } > for (sm_id = 0; sm_id < cur_fwd_config.nb_fwd_streams; sm_id++) { > struct fwd_stream *fs = fwd_streams[sm_id]; As such LGTM Acked-by: Aman Singh <aman.deep.singh@intel.com> ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] app/testpmd: fix stats get when display fwd stats 2022-04-27 12:15 ` Singh, Aman Deep @ 2022-04-28 3:10 ` Min Hu (Connor) 0 siblings, 0 replies; 10+ messages in thread From: Min Hu (Connor) @ 2022-04-28 3:10 UTC (permalink / raw) To: Singh, Aman Deep, dev; +Cc: thomas, xiaoyun.li, yuying.zhang Hi, Singh, 在 2022/4/27 20:15, Singh, Aman Deep 写道: > > > On 4/6/2022 2:15 PM, Min Hu (Connor) wrote: >> In function 'fwd_stats_display', if function 'rte_eth_stats_get' fails, >> 'stats' is uncertainty value. The display result will be abnormal. >> >> This patch check the return value of 'rte_eth_stats_get' to avoid >> display abnormal stats. >> >> Fixes: 53324971a14e ("app/testpmd: display/clear forwarding stats on >> demand") >> Cc: stable@dpdk.org >> >> Signed-off-by: Min Hu (Connor) <humin29@huawei.com> >> --- >> app/test-pmd/config.c | 10 ++++++++-- >> app/test-pmd/testpmd.c | 16 ++++++++++++++-- >> 2 files changed, 22 insertions(+), 4 deletions(-) >> >> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c >> index cc8e7aa138..bd689f9f86 100644 >> --- a/app/test-pmd/config.c >> +++ b/app/test-pmd/config.c >> @@ -249,14 +249,20 @@ nic_stats_display(portid_t port_id) >> diff_ns; >> uint64_t mpps_rx, mpps_tx, mbps_rx, mbps_tx; >> struct rte_eth_stats stats; >> - >> static const char *nic_stats_border = "########################"; >> + int ret; >> if (port_id_is_invalid(port_id, ENABLED_WARN)) { >> print_valid_ports(); >> return; >> } >> - rte_eth_stats_get(port_id, &stats); >> + ret = rte_eth_stats_get(port_id, &stats); >> + if (ret != 0) { >> + fprintf(stderr, >> + "%s: Error: failed to get stats (port %u): %d", >> + __func__, port_id, ret); >> + return; >> + } >> printf("\n %s NIC statistics for port %-2d %s\n", >> nic_stats_border, port_id, nic_stats_border); >> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c >> index fe2ce19f99..79bb23264b 100644 >> --- a/app/test-pmd/testpmd.c >> +++ b/app/test-pmd/testpmd.c >> @@ -1982,6 +1982,7 @@ fwd_stats_display(void) >> struct rte_port *port; >> streamid_t sm_id; >> portid_t pt_id; >> + int ret; >> int i; >> memset(ports_stats, 0, sizeof(ports_stats)); >> @@ -2013,7 +2014,13 @@ fwd_stats_display(void) >> pt_id = fwd_ports_ids[i]; >> port = &ports[pt_id]; >> - rte_eth_stats_get(pt_id, &stats); >> + ret = rte_eth_stats_get(pt_id, &stats); >> + if (ret != 0) { >> + fprintf(stderr, >> + "%s: Error: failed to get stats (port %u): %d", >> + __func__, pt_id, ret); >> + continue; >> + } >> stats.ipackets -= port->stats.ipackets; >> stats.opackets -= port->stats.opackets; >> stats.ibytes -= port->stats.ibytes; >> @@ -2108,11 +2115,16 @@ fwd_stats_reset(void) >> { >> streamid_t sm_id; >> portid_t pt_id; >> + int ret; >> int i; >> for (i = 0; i < cur_fwd_config.nb_fwd_ports; i++) { >> pt_id = fwd_ports_ids[i]; >> - rte_eth_stats_get(pt_id, &ports[pt_id].stats); >> + ret = rte_eth_stats_get(pt_id, &ports[pt_id].stats); >> + if (ret != 0) >> + fprintf(stderr, >> + "%s: Error: failed to clear stats (port %u):%d", >> + __func__, pt_id, ret); > Should we clear "ports[pt_id].stats" in this condition. when done 'rte_eth_stats_get', if success, 'ports[pt_id].stats' will be cleared in this function. if failed, no need to clear 'ports[pt_id].stats' as the log said. >> } >> for (sm_id = 0; sm_id < cur_fwd_config.nb_fwd_streams; sm_id++) { >> struct fwd_stream *fs = fwd_streams[sm_id]; > As such LGTM Thanks for your comment. > Acked-by: Aman Singh <aman.deep.singh@intel.com> > . ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] app/testpmd: fix incorrect MTU verification 2022-04-06 8:45 [PATCH 0/2] bugfix for testpmd Min Hu (Connor) 2022-04-06 8:45 ` [PATCH 1/2] app/testpmd: fix stats get when display fwd stats Min Hu (Connor) @ 2022-04-06 8:45 ` Min Hu (Connor) 2022-04-25 16:25 ` Singh, Aman Deep 2022-04-25 6:52 ` [PATCH 0/2] bugfix for testpmd Min Hu (Connor) 2 siblings, 1 reply; 10+ messages in thread From: Min Hu (Connor) @ 2022-04-06 8:45 UTC (permalink / raw) To: dev; +Cc: ferruh.yigit, thomas, xiaoyun.li, aman.deep.singh, yuying.zhang From: Huisong Li <lihuisong@huawei.com> The macro RTE_ETHER_MIN_LEN isn't the minimum value of MTU. But testpmd used it when execute 'port config mtu 0 xx' cmd. This patch fix it. Fixes: 1bb4a528c41f ("ethdev: fix max Rx packet length") Cc: stable@dpdk.org Signed-off-by: Huisong Li <lihuisong@huawei.com> Signed-off-by: Min Hu (Connor) <humin29@huawei.com> --- app/test-pmd/cmdline.c | 4 --- app/test-pmd/config.c | 55 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 4 deletions(-) diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index 6ffea8e21a..91e4090582 100644 --- a/app/test-pmd/cmdline.c +++ b/app/test-pmd/cmdline.c @@ -2050,10 +2050,6 @@ cmd_config_mtu_parsed(void *parsed_result, { struct cmd_config_mtu_result *res = parsed_result; - if (res->value < RTE_ETHER_MIN_LEN) { - fprintf(stderr, "mtu cannot be less than %d\n", RTE_ETHER_MIN_LEN); - return; - } port_mtu_set(res->port_id, res->value); } diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c index bd689f9f86..1b1e738f83 100644 --- a/app/test-pmd/config.c +++ b/app/test-pmd/config.c @@ -1254,6 +1254,57 @@ port_reg_set(portid_t port_id, uint32_t reg_off, uint32_t reg_v) display_port_reg_value(port_id, reg_off, reg_v); } +static uint32_t +eth_dev_get_overhead_len(uint32_t max_rx_pktlen, uint16_t max_mtu) +{ + uint32_t overhead_len; + + if (max_mtu != UINT16_MAX && max_rx_pktlen > max_mtu) + overhead_len = max_rx_pktlen - max_mtu; + else + overhead_len = RTE_ETHER_HDR_LEN + RTE_ETHER_CRC_LEN; + + return overhead_len; +} + +static int +eth_dev_validate_mtu(uint16_t port_id, uint16_t mtu) +{ + struct rte_eth_dev_info dev_info; + uint32_t overhead_len; + uint32_t frame_size; + int ret; + + ret = rte_eth_dev_info_get(port_id, &dev_info); + if (ret != 0) + return ret; + + if (mtu < dev_info.min_mtu) { + fprintf(stderr, + "MTU (%u) < device min MTU (%u) for port_id %u\n", + mtu, dev_info.min_mtu, port_id); + return -EINVAL; + } + if (mtu > dev_info.max_mtu) { + fprintf(stderr, + "MTU (%u) > device max MTU (%u) for port_id %u\n", + mtu, dev_info.max_mtu, port_id); + return -EINVAL; + } + + overhead_len = eth_dev_get_overhead_len(dev_info.max_rx_pktlen, + dev_info.max_mtu); + frame_size = mtu + overhead_len; + if (frame_size > dev_info.max_rx_pktlen) { + fprintf(stderr, + "Frame size (%u) > device max frame size (%u) for port_id %u\n", + frame_size, dev_info.max_rx_pktlen, port_id); + return -EINVAL; + } + + return 0; +} + void port_mtu_set(portid_t port_id, uint16_t mtu) { @@ -1263,6 +1314,10 @@ port_mtu_set(portid_t port_id, uint16_t mtu) if (port_id_is_invalid(port_id, ENABLED_WARN)) return; + diag = eth_dev_validate_mtu(port_id, mtu); + if (diag != 0) + return; + if (port->need_reconfig == 0) { diag = rte_eth_dev_set_mtu(port_id, mtu); if (diag != 0) { -- 2.33.0 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] app/testpmd: fix incorrect MTU verification 2022-04-06 8:45 ` [PATCH 2/2] app/testpmd: fix incorrect MTU verification Min Hu (Connor) @ 2022-04-25 16:25 ` Singh, Aman Deep 2022-04-26 1:38 ` lihuisong (C) 0 siblings, 1 reply; 10+ messages in thread From: Singh, Aman Deep @ 2022-04-25 16:25 UTC (permalink / raw) To: Min Hu (Connor), dev; +Cc: ferruh.yigit, thomas, xiaoyun.li, yuying.zhang On 4/6/2022 2:15 PM, Min Hu (Connor) wrote: > From: Huisong Li <lihuisong@huawei.com> > > The macro RTE_ETHER_MIN_LEN isn't the minimum value of MTU. But testpmd > used it when execute 'port config mtu 0 xx' cmd. This patch fix it. > > Fixes: 1bb4a528c41f ("ethdev: fix max Rx packet length") > Cc: stable@dpdk.org > > Signed-off-by: Huisong Li <lihuisong@huawei.com> > Signed-off-by: Min Hu (Connor) <humin29@huawei.com> > --- > app/test-pmd/cmdline.c | 4 --- > app/test-pmd/config.c | 55 ++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 55 insertions(+), 4 deletions(-) > > diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c > index 6ffea8e21a..91e4090582 100644 > --- a/app/test-pmd/cmdline.c > +++ b/app/test-pmd/cmdline.c > @@ -2050,10 +2050,6 @@ cmd_config_mtu_parsed(void *parsed_result, > { > struct cmd_config_mtu_result *res = parsed_result; > > - if (res->value < RTE_ETHER_MIN_LEN) { > - fprintf(stderr, "mtu cannot be less than %d\n", RTE_ETHER_MIN_LEN); > - return; > - } > port_mtu_set(res->port_id, res->value); > } > > diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c > index bd689f9f86..1b1e738f83 100644 > --- a/app/test-pmd/config.c > +++ b/app/test-pmd/config.c > @@ -1254,6 +1254,57 @@ port_reg_set(portid_t port_id, uint32_t reg_off, uint32_t reg_v) > display_port_reg_value(port_id, reg_off, reg_v); > } > > +static uint32_t > +eth_dev_get_overhead_len(uint32_t max_rx_pktlen, uint16_t max_mtu) > +{ > + uint32_t overhead_len; > + > + if (max_mtu != UINT16_MAX && max_rx_pktlen > max_mtu) > + overhead_len = max_rx_pktlen - max_mtu; > + else > + overhead_len = RTE_ETHER_HDR_LEN + RTE_ETHER_CRC_LEN; > + > + return overhead_len; > +} > + > +static int > +eth_dev_validate_mtu(uint16_t port_id, uint16_t mtu) > +{ > + struct rte_eth_dev_info dev_info; > + uint32_t overhead_len; > + uint32_t frame_size; > + int ret; > + > + ret = rte_eth_dev_info_get(port_id, &dev_info); > + if (ret != 0) > + return ret; > + > + if (mtu < dev_info.min_mtu) { > + fprintf(stderr, > + "MTU (%u) < device min MTU (%u) for port_id %u\n", > + mtu, dev_info.min_mtu, port_id); > + return -EINVAL; > + } > + if (mtu > dev_info.max_mtu) { > + fprintf(stderr, > + "MTU (%u) > device max MTU (%u) for port_id %u\n", > + mtu, dev_info.max_mtu, port_id); > + return -EINVAL; > + } > + > + overhead_len = eth_dev_get_overhead_len(dev_info.max_rx_pktlen, > + dev_info.max_mtu); > + frame_size = mtu + overhead_len; > + if (frame_size > dev_info.max_rx_pktlen) { > + fprintf(stderr, > + "Frame size (%u) > device max frame size (%u) for port_id %u\n", > + frame_size, dev_info.max_rx_pktlen, port_id); > + return -EINVAL; > + } > + > + return 0; > +} > + > void > port_mtu_set(portid_t port_id, uint16_t mtu) > { > @@ -1263,6 +1314,10 @@ port_mtu_set(portid_t port_id, uint16_t mtu) > if (port_id_is_invalid(port_id, ENABLED_WARN)) > return; > > + diag = eth_dev_validate_mtu(port_id, mtu); > + if (diag != 0) > + return; > + > if (port->need_reconfig == 0) { > diag = rte_eth_dev_set_mtu(port_id, mtu); > if (diag != 0) { I just wanted to know if these added functions eth_dev_validate_mtu() & eth_dev_get_overhead_len() are copy of ethdev library API's in file "rte_ethdev.c", which get called by rte_eth_dev_set_mtu. Is our intent, is to call these twice ? ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] app/testpmd: fix incorrect MTU verification 2022-04-25 16:25 ` Singh, Aman Deep @ 2022-04-26 1:38 ` lihuisong (C) 2022-05-12 16:37 ` Ferruh Yigit 0 siblings, 1 reply; 10+ messages in thread From: lihuisong (C) @ 2022-04-26 1:38 UTC (permalink / raw) To: dev 在 2022/4/26 0:25, Singh, Aman Deep 写道: > > On 4/6/2022 2:15 PM, Min Hu (Connor) wrote: >> From: Huisong Li <lihuisong@huawei.com> >> >> The macro RTE_ETHER_MIN_LEN isn't the minimum value of MTU. But testpmd >> used it when execute 'port config mtu 0 xx' cmd. This patch fix it. >> >> Fixes: 1bb4a528c41f ("ethdev: fix max Rx packet length") >> Cc: stable@dpdk.org >> >> Signed-off-by: Huisong Li <lihuisong@huawei.com> >> Signed-off-by: Min Hu (Connor) <humin29@huawei.com> >> --- >> app/test-pmd/cmdline.c | 4 --- >> app/test-pmd/config.c | 55 ++++++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 55 insertions(+), 4 deletions(-) >> >> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c >> index 6ffea8e21a..91e4090582 100644 >> --- a/app/test-pmd/cmdline.c >> +++ b/app/test-pmd/cmdline.c >> @@ -2050,10 +2050,6 @@ cmd_config_mtu_parsed(void *parsed_result, >> { >> struct cmd_config_mtu_result *res = parsed_result; >> - if (res->value < RTE_ETHER_MIN_LEN) { >> - fprintf(stderr, "mtu cannot be less than %d\n", >> RTE_ETHER_MIN_LEN); >> - return; >> - } >> port_mtu_set(res->port_id, res->value); >> } >> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c >> index bd689f9f86..1b1e738f83 100644 >> --- a/app/test-pmd/config.c >> +++ b/app/test-pmd/config.c >> @@ -1254,6 +1254,57 @@ port_reg_set(portid_t port_id, uint32_t >> reg_off, uint32_t reg_v) >> display_port_reg_value(port_id, reg_off, reg_v); >> } >> +static uint32_t >> +eth_dev_get_overhead_len(uint32_t max_rx_pktlen, uint16_t max_mtu) >> +{ >> + uint32_t overhead_len; >> + >> + if (max_mtu != UINT16_MAX && max_rx_pktlen > max_mtu) >> + overhead_len = max_rx_pktlen - max_mtu; >> + else >> + overhead_len = RTE_ETHER_HDR_LEN + RTE_ETHER_CRC_LEN; >> + >> + return overhead_len; >> +} >> + >> +static int >> +eth_dev_validate_mtu(uint16_t port_id, uint16_t mtu) >> +{ >> + struct rte_eth_dev_info dev_info; >> + uint32_t overhead_len; >> + uint32_t frame_size; >> + int ret; >> + >> + ret = rte_eth_dev_info_get(port_id, &dev_info); >> + if (ret != 0) >> + return ret; >> + >> + if (mtu < dev_info.min_mtu) { >> + fprintf(stderr, >> + "MTU (%u) < device min MTU (%u) for port_id %u\n", >> + mtu, dev_info.min_mtu, port_id); >> + return -EINVAL; >> + } >> + if (mtu > dev_info.max_mtu) { >> + fprintf(stderr, >> + "MTU (%u) > device max MTU (%u) for port_id %u\n", >> + mtu, dev_info.max_mtu, port_id); >> + return -EINVAL; >> + } >> + >> + overhead_len = eth_dev_get_overhead_len(dev_info.max_rx_pktlen, >> + dev_info.max_mtu); >> + frame_size = mtu + overhead_len; >> + if (frame_size > dev_info.max_rx_pktlen) { >> + fprintf(stderr, >> + "Frame size (%u) > device max frame size (%u) for >> port_id %u\n", >> + frame_size, dev_info.max_rx_pktlen, port_id); >> + return -EINVAL; >> + } >> + >> + return 0; >> +} >> + >> void >> port_mtu_set(portid_t port_id, uint16_t mtu) >> { >> @@ -1263,6 +1314,10 @@ port_mtu_set(portid_t port_id, uint16_t mtu) >> if (port_id_is_invalid(port_id, ENABLED_WARN)) >> return; >> + diag = eth_dev_validate_mtu(port_id, mtu); >> + if (diag != 0) >> + return; >> + >> if (port->need_reconfig == 0) { >> diag = rte_eth_dev_set_mtu(port_id, mtu); >> if (diag != 0) { > I just wanted to know if these added functions eth_dev_validate_mtu() > & eth_dev_get_overhead_len() > are copy of ethdev library API's in file "rte_ethdev.c", which get > called by rte_eth_dev_set_mtu. > Is our intent, is to call these twice ? If port->need_reconfig is 1, rte_eth_dev_set_mtu doesn't be called, and MTU value is saved to port->dev_conf.rxmode.mtu. This dev_conf.rxmode.mtu will be set to driver in dev_configure(). This check is performed to prevent dev_configure() failure. That's what I think. > . ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] app/testpmd: fix incorrect MTU verification 2022-04-26 1:38 ` lihuisong (C) @ 2022-05-12 16:37 ` Ferruh Yigit 0 siblings, 0 replies; 10+ messages in thread From: Ferruh Yigit @ 2022-05-12 16:37 UTC (permalink / raw) To: lihuisong (C), dev On 4/26/2022 2:38 AM, lihuisong (C) wrote: > > 在 2022/4/26 0:25, Singh, Aman Deep 写道: >> >> On 4/6/2022 2:15 PM, Min Hu (Connor) wrote: >>> From: Huisong Li <lihuisong@huawei.com> >>> >>> The macro RTE_ETHER_MIN_LEN isn't the minimum value of MTU. But testpmd >>> used it when execute 'port config mtu 0 xx' cmd. This patch fix it. >>> >>> Fixes: 1bb4a528c41f ("ethdev: fix max Rx packet length") >>> Cc: stable@dpdk.org >>> >>> Signed-off-by: Huisong Li <lihuisong@huawei.com> >>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com> <...> >>> @@ -1263,6 +1314,10 @@ port_mtu_set(portid_t port_id, uint16_t mtu) >>> if (port_id_is_invalid(port_id, ENABLED_WARN)) >>> return; >>> + diag = eth_dev_validate_mtu(port_id, mtu); >>> + if (diag != 0) >>> + return; >>> + >>> if (port->need_reconfig == 0) { >>> diag = rte_eth_dev_set_mtu(port_id, mtu); >>> if (diag != 0) { >> I just wanted to know if these added functions eth_dev_validate_mtu() >> & eth_dev_get_overhead_len() >> are copy of ethdev library API's in file "rte_ethdev.c", which get >> called by rte_eth_dev_set_mtu. >> Is our intent, is to call these twice ? > If port->need_reconfig is 1, rte_eth_dev_set_mtu doesn't be called, and > MTU value is saved to port->dev_conf.rxmode.mtu. This dev_conf.rxmode.mtu > will be set to driver in dev_configure(). This check is performed to > prevent dev_configure() failure. That's what I think. >> . Testpmd stores the MTU value and process it during start_port(), which may postpone any possible error in rte_eth_dev_configure(), so I think it is OK to duplicate the check in app level. Acked-by: Ferruh Yigit <ferruh.yigit@xilinx.com> ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/2] bugfix for testpmd 2022-04-06 8:45 [PATCH 0/2] bugfix for testpmd Min Hu (Connor) 2022-04-06 8:45 ` [PATCH 1/2] app/testpmd: fix stats get when display fwd stats Min Hu (Connor) 2022-04-06 8:45 ` [PATCH 2/2] app/testpmd: fix incorrect MTU verification Min Hu (Connor) @ 2022-04-25 6:52 ` Min Hu (Connor) 2022-05-12 16:37 ` Ferruh Yigit 2 siblings, 1 reply; 10+ messages in thread From: Min Hu (Connor) @ 2022-04-25 6:52 UTC (permalink / raw) To: dev Cc: ferruh.yigit, thomas, xiaoyun.li, aman.deep.singh, yuying.zhang, ferruh.yigit Hi, Ferruh, what do you think of this patch? 在 2022/4/6 16:45, Min Hu (Connor) 写道: > This patch set contains two bugfixes for testpmd. > > Huisong Li (1): > app/testpmd: fix incorrect MTU verification > > Min Hu (Connor) (1): > app/testpmd: fix stats get when display fwd stats > > app/test-pmd/cmdline.c | 4 --- > app/test-pmd/config.c | 65 ++++++++++++++++++++++++++++++++++++++++-- > app/test-pmd/testpmd.c | 16 +++++++++-- > 3 files changed, 77 insertions(+), 8 deletions(-) > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/2] bugfix for testpmd 2022-04-25 6:52 ` [PATCH 0/2] bugfix for testpmd Min Hu (Connor) @ 2022-05-12 16:37 ` Ferruh Yigit 0 siblings, 0 replies; 10+ messages in thread From: Ferruh Yigit @ 2022-05-12 16:37 UTC (permalink / raw) To: Min Hu (Connor), dev; +Cc: thomas, xiaoyun.li, aman.deep.singh, yuying.zhang On 4/25/2022 7:52 AM, Min Hu (Connor) wrote: > Hi, Ferruh, > what do you think of this patch? > > 在 2022/4/6 16:45, Min Hu (Connor) 写道: >> This patch set contains two bugfixes for testpmd. >> >> Huisong Li (1): >> app/testpmd: fix incorrect MTU verification >> >> Min Hu (Connor) (1): >> app/testpmd: fix stats get when display fwd stats >> Series applied to dpdk-next-net/main, thanks. ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2022-05-12 16:37 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-04-06 8:45 [PATCH 0/2] bugfix for testpmd Min Hu (Connor) 2022-04-06 8:45 ` [PATCH 1/2] app/testpmd: fix stats get when display fwd stats Min Hu (Connor) 2022-04-27 12:15 ` Singh, Aman Deep 2022-04-28 3:10 ` Min Hu (Connor) 2022-04-06 8:45 ` [PATCH 2/2] app/testpmd: fix incorrect MTU verification Min Hu (Connor) 2022-04-25 16:25 ` Singh, Aman Deep 2022-04-26 1:38 ` lihuisong (C) 2022-05-12 16:37 ` Ferruh Yigit 2022-04-25 6:52 ` [PATCH 0/2] bugfix for testpmd Min Hu (Connor) 2022-05-12 16:37 ` 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).