From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id 7FC32F1F; Wed, 25 Oct 2017 03:24:53 +0200 (CEST) Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 24 Oct 2017 18:24:52 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.43,430,1503385200"; d="scan'208";a="166686811" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.241.225.60]) ([10.241.225.60]) by fmsmga005.fm.intel.com with ESMTP; 24 Oct 2017 18:24:52 -0700 To: Pascal Mazon Cc: dev@dpdk.org, Keith Wiles , Vipin Varghese , stable@dpdk.org References: <20170918184735.43968-1-ferruh.yigit@intel.com> <8cf71f00-ca73-86b4-b6dd-fa24aac3167f@6wind.com> From: Ferruh Yigit Message-ID: <9e4a5217-ef03-861f-e17c-7eed22018b62@intel.com> Date: Tue, 24 Oct 2017 18:24:52 -0700 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: <8cf71f00-ca73-86b4-b6dd-fa24aac3167f@6wind.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH 1/2] net/tap: fix setting speed by argument 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: Wed, 25 Oct 2017 01:24:55 -0000 On 9/19/2017 5:45 AM, Pascal Mazon wrote: > Hi, > > The patch looks mainly ok to me. > > I'll put some comments inline. > > On 18/09/2017 20:47, Ferruh Yigit wrote: >> From: Vipin Varghese >> >> tap speed argument is not working without generating any error. > Can you describe the error, paste the log? >> >> This patch sets the configured speed during device start. >> >> Fixes: 02f96a0a82d1 ("net/tap: add TUN/TAP device PMD") >> Cc: stable@dpdk.org >> >> Signed-off-by: Vipin Varghese >> --- >> drivers/net/tap/rte_eth_tap.c | 27 +++++++++++++++++++++++++++ >> drivers/net/tap/rte_eth_tap.h | 2 ++ >> 2 files changed, 29 insertions(+) >> >> diff --git a/drivers/net/tap/rte_eth_tap.c b/drivers/net/tap/rte_eth_tap.c >> index 01cba0fa1..00dad167f 100644 >> --- a/drivers/net/tap/rte_eth_tap.c >> +++ b/drivers/net/tap/rte_eth_tap.c >> @@ -545,6 +545,7 @@ tap_ioctl(struct pmd_internals *pmd, unsigned long request, >> case SIOCGIFHWADDR: >> case SIOCSIFHWADDR: >> case SIOCSIFMTU: >> + case SIOCETHTOOL: >> break; >> default: >> RTE_ASSERT(!"unsupported request type: must not happen"); >> @@ -585,6 +586,32 @@ static int >> tap_dev_start(struct rte_eth_dev *dev) >> { >> int err; >> + struct ifreq ifr; >> + struct ethtool_cmd edata = {0}; >> + struct pmd_internals *pmd = dev->data->dev_private; >> + >> + /*set & get speed to device*/ >> + edata.speed = pmd_link.link_speed; >> + edata.cmd = ETHTOOL_SSET; >> + ifr.ifr_data = (caddr_t)&edata; >> + if (tap_ioctl(pmd, SIOCETHTOOL, &ifr, 0, LOCAL_ONLY) < 0) { > I think it would be best to also try setting the speed on the remote > (tap_ioctl(REMOTE_ONLY)), but continue execution even if that fails. Hi Pascal, It looks like this ioctl fails to set tap speed. And as far as I can see speed is hardcoded in kernel [1], do you know if it is possible to set speed of tap interface? Thanks, ferruh [1] http://elixir.free-electrons.com/linux/v4.13.9/source/drivers/net/tun.c#L2460 >> + RTE_LOG(WARNING, PMD, >> + "Could not set param speed %d for ifindex for %s.\n", >> + pmd_link.link_speed, >> + pmd->name); >> + >> + /* fetch current speed of created device */ >> + edata.cmd = ETHTOOL_GSET; >> + ifr.ifr_data = (caddr_t)&edata; >> + if (tap_ioctl(pmd, SIOCETHTOOL, &ifr, 0, LOCAL_ONLY) < 0) >> + return 0; >> + >> + pmd_link.link_speed = edata.speed; >> + RTE_LOG(INFO, PMD, "get speed %d for ifindex for %s.\n", > I would say "got". > >> + pmd_link.link_speed, >> + pmd->name); >> + } >> + dev->data->dev_link = pmd_link; >> >> err = tap_intr_handle_set(dev, 1); >> if (err) >> diff --git a/drivers/net/tap/rte_eth_tap.h b/drivers/net/tap/rte_eth_tap.h >> index 928a0454e..3e91db4ae 100644 >> --- a/drivers/net/tap/rte_eth_tap.h >> +++ b/drivers/net/tap/rte_eth_tap.h >> @@ -40,6 +40,8 @@ >> #include >> >> #include >> +#include >> +#include >> >> #include >> #include > These includes are only useful inside rte_eth_tap.c, I would put them > there only. >