From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dispatch1-us1.ppe-hosted.com (dispatch1-us1.ppe-hosted.com [67.231.154.164]) by dpdk.org (Postfix) with ESMTP id 1431D1B130; Tue, 23 Apr 2019 17:44:46 +0200 (CEST) X-Virus-Scanned: Proofpoint Essentials engine Received: from webmail.solarflare.com (uk.solarflare.com [193.34.186.16]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by mx1-us1.ppe-hosted.com (Proofpoint Essentials ESMTP Server) with ESMTPS id C16D6780069; Tue, 23 Apr 2019 15:44:44 +0000 (UTC) Received: from [192.168.38.17] (91.220.146.112) by ukex01.SolarFlarecom.com (10.17.10.4) with Microsoft SMTP Server (TLS) id 15.0.1395.4; Tue, 23 Apr 2019 16:44:37 +0100 To: Ferruh Yigit , Wenzhuo Lu , Jingjing Wu , Bernard Iremonger CC: , , "Li, WenjieX A" , Stephen Hemminger References: <1555074753-9098-1-git-send-email-arybchenko@solarflare.com> <04bcdfba-420b-9d5e-6e10-9d8b6762c378@intel.com> <0362242f-a51a-b37b-a2e7-61f1c8bbea35@intel.com> From: Andrew Rybchenko Message-ID: Date: Tue, 23 Apr 2019 18:44:33 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 MIME-Version: 1.0 In-Reply-To: <0362242f-a51a-b37b-a2e7-61f1c8bbea35@intel.com> Content-Language: en-GB X-Originating-IP: [91.220.146.112] X-ClientProxiedBy: ocex03.SolarFlarecom.com (10.20.40.36) To ukex01.SolarFlarecom.com (10.17.10.4) X-TM-AS-Product-Ver: SMEX-12.5.0.1300-8.5.1010-24568.003 X-TM-AS-Result: No-13.149200-8.000000-10 X-TMASE-MatchedRID: vbSD0OnL8/IOwH4pD14DsPHkpkyUphL99fvWztwgm5MT7lsB95pa6nLY yr7vQ13A4Agjcq8I+mD9vStSyrKbfFDgk8FLzzw6FYJUGv4DL3wuhg66Itb65RxUkJPe1WBqkKo IKmbPsauNWH3MgdMzZR3x6SoWWjOpSe3iZ10QiJOjrlYm3WTU72GNLTRnb5YtfXg4vWBMtBRlWG AuNI6SxunmptgQI8s0bpqPyYh+hJVDeuA2fujoFf28T9BUNXkhKhNpTcvbdUJ794qAHfSq0rPyA EfYVzOl+FVAbsNxrCvMJYD0aRF0RRoQP43UnryWwbRQ2BpmliqZEoWHC6Rh/R9Oluq8LbzVzdlo 26al4KH2vPhulcaXWIFoPFs7wHUtBawxeDgsyEmdOp1TjDrlaS2VljVYB9GNOyxd6V963VS48Wu RFQVdauLzNWBegCW2RYvisGWbbS+3sNbcHjySQbI7zVffJqTzJejTrPTdBJHozgZuKr6W/dUvAU tXg6Zo90YHo66SUcXlEsbtrbZ1xEPJqT/+qmkiiMsjiJ970qnPHctIxhKEF57V6Ro5mxCB X-TM-AS-User-Approved-Sender: Yes X-TM-AS-User-Blocked-Sender: No X-TMASE-Result: 10--13.149200-8.000000 X-TMASE-Version: SMEX-12.5.0.1300-8.5.1010-24568.003 X-MDID: 1556034285-iUxyLj7gM-PZ Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [PATCH] app/testpmd: set fixed flag when exact link speed is chosen X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 23 Apr 2019 15:44:46 -0000 On 4/23/19 6:04 PM, Ferruh Yigit wrote: > On 4/23/2019 4:02 PM, Ferruh Yigit wrote: >> On 4/12/2019 2:12 PM, Andrew Rybchenko wrote: >>> Setting exact link speed makes sense if auto-negotiation is >>> disabled. Fixed flag is required to disable auto-negotiation. >> Hi Andrew, >> >> Fixed link speed is not supported by all PMDs and this change is breaking them >> to set the speed in testpmd. Not all. sfc definitely supports it. It looks like bnxt, e1000, igb support it as well. >> As long as device speed set correct, I believe the command doesn't really care >> about if link mode is auto-negotiation or not. Sometimes it is really important to disable auto-negotiation. >> I am for reverting this commit, is there any objection to revert it? > cc'ing Wenjie who reported the issue. May be I misunderstood the purpose of the command. Typically if someone wants to set link speed, it is assumed that auto-negotiation should be disabled since user specifies what he really wants. Of course, it is still valid request to keep auto-negotiation enabled, but limit set of advertised speeds (may be keep only one). So, I'm not happy to revert it completely. There is an option to revert to old behaviour and add optional argument to disable auto-negotiation, but I can't say that I like it, since it would make sense if the command allows more than one speed to be advertised (to be auto-negotiated). If it is only one speed, the default should be autoneg disabled. Anyway documentation of the command should be improved. Andrew. >>> Fixes: 88fbedcd5e5a ("app/testpmd: move speed and duplex parsing in a function") >>> Cc: stable@dpdk.org >>> >>> Signed-off-by: Andrew Rybchenko >>> --- >>> app/test-pmd/cmdline.c | 12 ++++++------ >>> 1 file changed, 6 insertions(+), 6 deletions(-) >>> >>> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c >>> index 2ab03c111..691d818a6 100644 >>> --- a/app/test-pmd/cmdline.c >>> +++ b/app/test-pmd/cmdline.c >>> @@ -1440,17 +1440,17 @@ parse_and_check_speed_duplex(char *speedstr, char *duplexstr, uint32_t *speed) >>> return -1; >>> } >>> if (!strcmp(speedstr, "1000")) { >>> - *speed = ETH_LINK_SPEED_1G; >>> + *speed = ETH_LINK_SPEED_1G | ETH_LINK_SPEED_FIXED; >>> } else if (!strcmp(speedstr, "10000")) { >>> - *speed = ETH_LINK_SPEED_10G; >>> + *speed = ETH_LINK_SPEED_10G | ETH_LINK_SPEED_FIXED; >>> } else if (!strcmp(speedstr, "25000")) { >>> - *speed = ETH_LINK_SPEED_25G; >>> + *speed = ETH_LINK_SPEED_25G | ETH_LINK_SPEED_FIXED; >>> } else if (!strcmp(speedstr, "40000")) { >>> - *speed = ETH_LINK_SPEED_40G; >>> + *speed = ETH_LINK_SPEED_40G | ETH_LINK_SPEED_FIXED; >>> } else if (!strcmp(speedstr, "50000")) { >>> - *speed = ETH_LINK_SPEED_50G; >>> + *speed = ETH_LINK_SPEED_50G | ETH_LINK_SPEED_FIXED; >>> } else if (!strcmp(speedstr, "100000")) { >>> - *speed = ETH_LINK_SPEED_100G; >>> + *speed = ETH_LINK_SPEED_100G | ETH_LINK_SPEED_FIXED; >>> } else if (!strcmp(speedstr, "auto")) { >>> *speed = ETH_LINK_SPEED_AUTONEG; >>> } else { >>> From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by dpdk.space (Postfix) with ESMTP id BC96FA05D3 for ; Tue, 23 Apr 2019 17:44:47 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 8998B1B4CD; Tue, 23 Apr 2019 17:44:47 +0200 (CEST) Received: from dispatch1-us1.ppe-hosted.com (dispatch1-us1.ppe-hosted.com [67.231.154.164]) by dpdk.org (Postfix) with ESMTP id 1431D1B130; Tue, 23 Apr 2019 17:44:46 +0200 (CEST) X-Virus-Scanned: Proofpoint Essentials engine Received: from webmail.solarflare.com (uk.solarflare.com [193.34.186.16]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by mx1-us1.ppe-hosted.com (Proofpoint Essentials ESMTP Server) with ESMTPS id C16D6780069; Tue, 23 Apr 2019 15:44:44 +0000 (UTC) Received: from [192.168.38.17] (91.220.146.112) by ukex01.SolarFlarecom.com (10.17.10.4) with Microsoft SMTP Server (TLS) id 15.0.1395.4; Tue, 23 Apr 2019 16:44:37 +0100 To: Ferruh Yigit , Wenzhuo Lu , Jingjing Wu , Bernard Iremonger CC: , , "Li, WenjieX A" , Stephen Hemminger References: <1555074753-9098-1-git-send-email-arybchenko@solarflare.com> <04bcdfba-420b-9d5e-6e10-9d8b6762c378@intel.com> <0362242f-a51a-b37b-a2e7-61f1c8bbea35@intel.com> From: Andrew Rybchenko Message-ID: Date: Tue, 23 Apr 2019 18:44:33 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 MIME-Version: 1.0 In-Reply-To: <0362242f-a51a-b37b-a2e7-61f1c8bbea35@intel.com> Content-Language: en-GB X-Originating-IP: [91.220.146.112] X-ClientProxiedBy: ocex03.SolarFlarecom.com (10.20.40.36) To ukex01.SolarFlarecom.com (10.17.10.4) X-TM-AS-Product-Ver: SMEX-12.5.0.1300-8.5.1010-24568.003 X-TM-AS-Result: No-13.149200-8.000000-10 X-TMASE-MatchedRID: vbSD0OnL8/IOwH4pD14DsPHkpkyUphL99fvWztwgm5MT7lsB95pa6nLY yr7vQ13A4Agjcq8I+mD9vStSyrKbfFDgk8FLzzw6FYJUGv4DL3wuhg66Itb65RxUkJPe1WBqkKo IKmbPsauNWH3MgdMzZR3x6SoWWjOpSe3iZ10QiJOjrlYm3WTU72GNLTRnb5YtfXg4vWBMtBRlWG AuNI6SxunmptgQI8s0bpqPyYh+hJVDeuA2fujoFf28T9BUNXkhKhNpTcvbdUJ794qAHfSq0rPyA EfYVzOl+FVAbsNxrCvMJYD0aRF0RRoQP43UnryWwbRQ2BpmliqZEoWHC6Rh/R9Oluq8LbzVzdlo 26al4KH2vPhulcaXWIFoPFs7wHUtBawxeDgsyEmdOp1TjDrlaS2VljVYB9GNOyxd6V963VS48Wu RFQVdauLzNWBegCW2RYvisGWbbS+3sNbcHjySQbI7zVffJqTzJejTrPTdBJHozgZuKr6W/dUvAU tXg6Zo90YHo66SUcXlEsbtrbZ1xEPJqT/+qmkiiMsjiJ970qnPHctIxhKEF57V6Ro5mxCB X-TM-AS-User-Approved-Sender: Yes X-TM-AS-User-Blocked-Sender: No X-TMASE-Result: 10--13.149200-8.000000 X-TMASE-Version: SMEX-12.5.0.1300-8.5.1010-24568.003 X-MDID: 1556034285-iUxyLj7gM-PZ Content-Type: text/plain; charset="UTF-8"; format="flowed" Content-Transfer-Encoding: 7bit X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [PATCH] app/testpmd: set fixed flag when exact link speed is chosen X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 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" Message-ID: <20190423154433.HNu_NDOgjQx2EonZxqsbCeKH_tkNKmPFjTWsNbZjjvA@z> On 4/23/19 6:04 PM, Ferruh Yigit wrote: > On 4/23/2019 4:02 PM, Ferruh Yigit wrote: >> On 4/12/2019 2:12 PM, Andrew Rybchenko wrote: >>> Setting exact link speed makes sense if auto-negotiation is >>> disabled. Fixed flag is required to disable auto-negotiation. >> Hi Andrew, >> >> Fixed link speed is not supported by all PMDs and this change is breaking them >> to set the speed in testpmd. Not all. sfc definitely supports it. It looks like bnxt, e1000, igb support it as well. >> As long as device speed set correct, I believe the command doesn't really care >> about if link mode is auto-negotiation or not. Sometimes it is really important to disable auto-negotiation. >> I am for reverting this commit, is there any objection to revert it? > cc'ing Wenjie who reported the issue. May be I misunderstood the purpose of the command. Typically if someone wants to set link speed, it is assumed that auto-negotiation should be disabled since user specifies what he really wants. Of course, it is still valid request to keep auto-negotiation enabled, but limit set of advertised speeds (may be keep only one). So, I'm not happy to revert it completely. There is an option to revert to old behaviour and add optional argument to disable auto-negotiation, but I can't say that I like it, since it would make sense if the command allows more than one speed to be advertised (to be auto-negotiated). If it is only one speed, the default should be autoneg disabled. Anyway documentation of the command should be improved. Andrew. >>> Fixes: 88fbedcd5e5a ("app/testpmd: move speed and duplex parsing in a function") >>> Cc: stable@dpdk.org >>> >>> Signed-off-by: Andrew Rybchenko >>> --- >>> app/test-pmd/cmdline.c | 12 ++++++------ >>> 1 file changed, 6 insertions(+), 6 deletions(-) >>> >>> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c >>> index 2ab03c111..691d818a6 100644 >>> --- a/app/test-pmd/cmdline.c >>> +++ b/app/test-pmd/cmdline.c >>> @@ -1440,17 +1440,17 @@ parse_and_check_speed_duplex(char *speedstr, char *duplexstr, uint32_t *speed) >>> return -1; >>> } >>> if (!strcmp(speedstr, "1000")) { >>> - *speed = ETH_LINK_SPEED_1G; >>> + *speed = ETH_LINK_SPEED_1G | ETH_LINK_SPEED_FIXED; >>> } else if (!strcmp(speedstr, "10000")) { >>> - *speed = ETH_LINK_SPEED_10G; >>> + *speed = ETH_LINK_SPEED_10G | ETH_LINK_SPEED_FIXED; >>> } else if (!strcmp(speedstr, "25000")) { >>> - *speed = ETH_LINK_SPEED_25G; >>> + *speed = ETH_LINK_SPEED_25G | ETH_LINK_SPEED_FIXED; >>> } else if (!strcmp(speedstr, "40000")) { >>> - *speed = ETH_LINK_SPEED_40G; >>> + *speed = ETH_LINK_SPEED_40G | ETH_LINK_SPEED_FIXED; >>> } else if (!strcmp(speedstr, "50000")) { >>> - *speed = ETH_LINK_SPEED_50G; >>> + *speed = ETH_LINK_SPEED_50G | ETH_LINK_SPEED_FIXED; >>> } else if (!strcmp(speedstr, "100000")) { >>> - *speed = ETH_LINK_SPEED_100G; >>> + *speed = ETH_LINK_SPEED_100G | ETH_LINK_SPEED_FIXED; >>> } else if (!strcmp(speedstr, "auto")) { >>> *speed = ETH_LINK_SPEED_AUTONEG; >>> } else { >>>