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 9CB1CA055D; Mon, 1 Mar 2021 05:48:11 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 21DCD40684; Mon, 1 Mar 2021 05:48:11 +0100 (CET) Received: from mail-qv1-f47.google.com (mail-qv1-f47.google.com [209.85.219.47]) by mails.dpdk.org (Postfix) with ESMTP id C37544014E for ; Mon, 1 Mar 2021 05:48:08 +0100 (CET) Received: by mail-qv1-f47.google.com with SMTP id r5so7516798qvv.9 for ; Sun, 28 Feb 2021 20:48:08 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=broadcom.com; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=fwt0nPAPht51csd5bVlpB8O5TYoVEBjoOVaoAtCYaKo=; b=Yc5CijjWKj6kQToi3h8hE3u/To682XjW0P87TfQmxQCR2XxDwvJao4+qD2xpNy0Q32 Jvt22L9QuJ3k5AXsw1ek94u32TZI+UJ4B9sFW6Tw6LvAmtXRtb9Tm8zyzg4m/FXNFX1M wKzGh74EtjCo6hwEHk1RU7ADnqfiEFPZTCG60= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=fwt0nPAPht51csd5bVlpB8O5TYoVEBjoOVaoAtCYaKo=; b=iU2DF+Cmpli/dggxekB/Na1Vv6z9zpDJiVwkrrUpdgpCOrjUB54aFQ4Pg/eUJkyQO6 i5NkWsraMFW10vNfqQX6h+XWVEY9N2Vu6UwTQRieIxrQTYLdXqDjGKHC3y8+jq0jJih4 HNlkO/DeN7L+fVR8seMFvFIdrG8xyv5JSPG1M9W5gyG8PWhR4wG7RrZRl2D5IbIZsN6f D7eCSdBXG81GgPIAT85Gdqa1JlAIjiVdwg8LdG4XXRTgoUfygFu79x1zStbkKI2rRJVk HqgqNg7M3Yw1Z2kdgZpul0HR1CkmgV2LFHTISExTn0HE/kEEdDoqkFQTNE4bCu1NwR62 +S+g== X-Gm-Message-State: AOAM532nH0Sh8hPaZB3ODtz89D7SGkB6AKU37fpgZYkk5rpgGoP9MWss +CvNKp8Gb20SiBUdSKcw+bIcGMw0fFRiNKwrY2Mxtg== X-Google-Smtp-Source: ABdhPJwujaG01VrOlbm/6mQCPuLpT/STz6T4He6dkBrCpFhv8T8+YRdqf6M4sZZVRhxYdbMeNK3BCGGiRhMO/nCdtjM= X-Received: by 2002:a0c:b9a6:: with SMTP id v38mr12611756qvf.35.1614574087953; Sun, 28 Feb 2021 20:48:07 -0800 (PST) MIME-Version: 1.0 References: <20210222191824.40230-1-ajit.khaparde@broadcom.com> <3a90da98-da41-70fb-2b6b-99e68c260b62@intel.com> <1dbfb659-4f91-90fe-0e0c-23d49d53f03b@intel.com> In-Reply-To: <1dbfb659-4f91-90fe-0e0c-23d49d53f03b@intel.com> From: Ajit Khaparde Date: Sun, 28 Feb 2021 20:47:51 -0800 Message-ID: To: Ferruh Yigit Cc: Andrew Rybchenko , dpdk-dev , Qi Zhang , Thomas Monjalon Content-Type: multipart/signed; protocol="application/pkcs7-signature"; micalg=sha-256; boundary="00000000000000b3f505bc725300" X-Content-Filtered-By: Mailman/MimeDel 2.1.29 Subject: Re: [dpdk-dev] [PATCH] app/testpmd: add support for forced ethernet speed 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 Sender: "dev" --00000000000000b3f505bc725300 Content-Type: text/plain; charset="UTF-8" On Fri, Feb 26, 2021 at 3:21 AM Ferruh Yigit wrote: > > On 2/26/2021 6:43 AM, Andrew Rybchenko wrote: > > On 2/25/21 9:25 PM, Ferruh Yigit wrote: > >> On 2/22/2021 7:18 PM, Ajit Khaparde wrote: > >>> Add support for forced ethernet speed setting. > >>> Currently testpmd tries to configure the Ethernet port in autoneg mode. > >>> It is not possible to set the Ethernet port to a specific speed while > >>> starting testpmd. In some cases capability to configure a forced speed > >>> for the Ethernet port during initialization may be necessary. This patch > >>> tries to add this support. > >>> > >>> The patch assumes full duplex setting and does not attempt to change > >>> that. > >>> So speeds like 10M, 100M are not configurable using this method. > >>> > >>> The command line to configure a forced speed of 10G: > >>> dpdk-testpmd -c 0xff -- -i --eth-link-speed 10000 > >>> > >>> The command line to configure a forced speed of 50G: > >>> dpdk-testpmd -c 0xff -- -i --eth-link-speed 50000 > >>> > >>> Signed-off-by: Ajit Khaparde > >>> --- > >>> app/test-pmd/parameters.c | 42 +++++++++++++++++++++++++++ > >>> app/test-pmd/testpmd.c | 4 +++ > >>> app/test-pmd/testpmd.h | 1 + > >>> doc/guides/testpmd_app_ug/run_app.rst | 11 +++++++ > >>> 4 files changed, 58 insertions(+) > >> > >> Can you also update the release notes to document the new parameter? > >> > >>> > >>> diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c > >>> index c8acd5d1b7..e10f7d38fb 100644 > >>> --- a/app/test-pmd/parameters.c > >>> +++ b/app/test-pmd/parameters.c > >>> @@ -224,6 +224,7 @@ usage(char* progname) > >>> printf(" --hairpin-mode=0xXX: bitmask set the hairpin port > >>> mode.\n " > >>> " 0x10 - explicit Tx rule, 0x02 - hairpin ports paired\n" > >>> " 0x01 - hairpin ports loop, 0x00 - hairpin port > >>> self\n"); > >>> + printf(" --eth-link-speed: forced link speed.\n"); > >>> } > >>> #ifdef RTE_LIB_CMDLINE > >>> @@ -485,6 +486,41 @@ parse_event_printing_config(const char *optarg, > >>> int enable) > >>> return 0; > >>> } > >>> +static int > >>> +parse_link_speed(int n) > >>> +{ > >>> + uint32_t speed; > >>> + > >>> + switch (n) { > >> > >> OK to not support "10M, 100M", not sure if anybody really uses them, but > >> what do you think checking them and return an error? > >> > >>> + case 1000: > >>> + speed = ETH_LINK_SPEED_1G; > >>> + break; > >>> + case 10000: > >>> + speed = ETH_LINK_SPEED_10G; > >>> + break; > >>> + case 25000: > >>> + speed = ETH_LINK_SPEED_25G; > >>> + break; > >>> + case 40000: > >>> + speed = ETH_LINK_SPEED_40G; > >>> + break; > >>> + case 50000: > >>> + speed = ETH_LINK_SPEED_50G; > >>> + break; > >>> + case 100000: > >>> + speed = ETH_LINK_SPEED_100G; > >>> + break; > >>> + case 200000: > >>> + speed = ETH_LINK_SPEED_200G; > >>> + break; > >>> + default: > >>> + speed = ETH_LINK_SPEED_AUTONEG; > >>> + break; > >> > >> Isn't this function to set a fixed link speed, why falling back to autoneg? > >> > >> Also shouldn't this function set 'ETH_LINK_SPEED_FIXED' too? > > > > It should. Previous time I've tried to fix corresponding > > bug in CLI commands, it ended up with rollback because > > of Intel drivers do not handle it correctly. I can set the ETH_LINK_SPEED_FIXED bit while setting the fixed speed. > > > > See "app/testpmd: set fixed flag for exact link speed" and > > corresponding revert. I took a look at the patch. Looks like we were setting ETH_LINK_SPEED_FIXED in parse_and_check_speed_duplex(). Which is an existing function. I am adding the fixed speed capability during application init itself. So it is a little different than what was done earlier. I will send a v2 with the suggestions I got so far. > > > > Thanks for the reminder Andrew, you have a good memory :) > For reference: > http://inbox.dpdk.org/dev/20190507100928.pOyue5JiSaPL-NSHiueAU3HlgisgF9bYynJGpTjyvMw@z/ > > It seems that patch reverted with the pressure of the release, what do you think > applying it again while we have enough time to fix the PMDs before release? > > From previous discussions, long term actions listed as: > " > 1) Implement 'fixed' link speed support in the missing drivers. > 2) Send a new version of the testpmd patch with a "fixed" argument, so that we > can support all three above > " > > Not sure having (2) explicitly is required, we have already "auto" speed, not > having it implies the fixed speed. > So we can just re-apply your old patch. I think that's a good idea. If there are any issues with the drivers, there is time to address them. --00000000000000b3f505bc725300--