From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ferruh Yigit Subject: Re: [PATCH 1/2] net/tap: fix setting speed by argument Date: Tue, 24 Oct 2017 18:24:52 -0700 Message-ID: <9e4a5217-ef03-861f-e17c-7eed22018b62@intel.com> References: <20170918184735.43968-1-ferruh.yigit@intel.com> <8cf71f00-ca73-86b4-b6dd-fa24aac3167f@6wind.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: dev@dpdk.org, Keith Wiles , Vipin Varghese , stable@dpdk.org To: Pascal Mazon Return-path: In-Reply-To: <8cf71f00-ca73-86b4-b6dd-fa24aac3167f@6wind.com> Content-Language: en-US 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 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. >