From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 69A98A0093; Tue, 26 Apr 2022 03:38:54 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 3761B406A2; Tue, 26 Apr 2022 03:38:54 +0200 (CEST) Received: from szxga03-in.huawei.com (szxga03-in.huawei.com [45.249.212.189]) by mails.dpdk.org (Postfix) with ESMTP id E9B3F40691 for ; Tue, 26 Apr 2022 03:38:52 +0200 (CEST) Received: from kwepemi100016.china.huawei.com (unknown [172.30.72.53]) by szxga03-in.huawei.com (SkyGuard) with ESMTP id 4KnPYk5DR3zCsLy for ; Tue, 26 Apr 2022 09:34:18 +0800 (CST) Received: from kwepemm600004.china.huawei.com (7.193.23.242) by kwepemi100016.china.huawei.com (7.221.188.123) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.24; Tue, 26 Apr 2022 09:38:49 +0800 Received: from [10.67.103.231] (10.67.103.231) by kwepemm600004.china.huawei.com (7.193.23.242) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.24; Tue, 26 Apr 2022 09:38:49 +0800 Message-ID: Date: Tue, 26 Apr 2022 09:38:49 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.2.0 Subject: Re: [PATCH 2/2] app/testpmd: fix incorrect MTU verification To: References: <20220406084537.16799-1-humin29@huawei.com> <20220406084537.16799-3-humin29@huawei.com> From: "lihuisong (C)" In-Reply-To: Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.67.103.231] X-ClientProxiedBy: dggems701-chm.china.huawei.com (10.3.19.178) To kwepemm600004.china.huawei.com (7.193.23.242) X-CFilter-Loop: Reflected X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org 在 2022/4/26 0:25, Singh, Aman Deep 写道: > > On 4/6/2022 2:15 PM, Min Hu (Connor) wrote: >> From: Huisong Li >> >> 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 >> Signed-off-by: Min Hu (Connor) >> --- >>   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. > .