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 DC520A034F; Fri, 26 Feb 2021 12:21:55 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 591D8407FF; Fri, 26 Feb 2021 12:21:55 +0100 (CET) Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by mails.dpdk.org (Postfix) with ESMTP id 8B19840692 for ; Fri, 26 Feb 2021 12:21:53 +0100 (CET) IronPort-SDR: Gbn75SDrtIeUsI+1COpfy2tR9DOxtBidYTCZneVhhjTU+ny75L38k5xIhSo2g1o//qGQVUMCDq aXyTkZk16MxQ== X-IronPort-AV: E=McAfee;i="6000,8403,9906"; a="270825801" X-IronPort-AV: E=Sophos;i="5.81,208,1610438400"; d="scan'208";a="270825801" Received: from orsmga008.jf.intel.com ([10.7.209.65]) by fmsmga105.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 26 Feb 2021 03:21:52 -0800 IronPort-SDR: AJ7s7/jjVaaDYXe8AFo7Lc5/o42UbAppqsD4iAp1CdEY3M4VLu/9Q8TyMqWkGhs8eTLa30B5+W FklXZtPl1Fbw== X-IronPort-AV: E=Sophos;i="5.81,208,1610438400"; d="scan'208";a="404860960" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.252.17.170]) ([10.252.17.170]) by orsmga008-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 26 Feb 2021 03:21:50 -0800 To: Andrew Rybchenko , Ajit Khaparde , dev@dpdk.org References: <20210222191824.40230-1-ajit.khaparde@broadcom.com> <3a90da98-da41-70fb-2b6b-99e68c260b62@intel.com> From: Ferruh Yigit Cc: Qi Zhang , Thomas Monjalon X-User: ferruhy Message-ID: <1dbfb659-4f91-90fe-0e0c-23d49d53f03b@intel.com> Date: Fri, 26 Feb 2021 11:21:47 +0000 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit 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" 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. > > See "app/testpmd: set fixed flag for exact link speed" and > corresponding revert. > 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.