From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f67.google.com (mail-wm0-f67.google.com [74.125.82.67]) by dpdk.org (Postfix) with ESMTP id B00871B794 for ; Wed, 25 Oct 2017 08:27:40 +0200 (CEST) Received: by mail-wm0-f67.google.com with SMTP id m72so19608982wmc.1 for ; Tue, 24 Oct 2017 23:27:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-transfer-encoding:content-language; bh=t0TZ4Gi8Vjk2IMr2axFoSMRAlOCnON8/QhE1fjmwBEQ=; b=j3KWNeMMbzkAoMXK6T+sihkm5zfzVdEBkYWnQKsoKStwChks2utF5kjVPw8l1TCvtE LSg6HqNvwOsugjEA6SecpJhYvM9HjgxYHxMhnnbvzj83NB4F6qib2sexlibLlMIkP1sT 4SkspSHk2dtlwDf1JRoo34V2BGMkBeTRdcxpD2W0RaZxWrJiA5BWvuWDgf9+jPmP5BU2 6zlhwR/A7C4eAGHqvrpt67yXJxIEAOwS1F4mJcLTozVIR21Y0AFVQTneywNKtWtv9Yqo k6eTJ8j7DNE8L997C2NhYOze5C0WoDhY5AwPSfnLdiA4JMtrClsEjkuTMUKtMnUuprV6 rkIg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding :content-language; bh=t0TZ4Gi8Vjk2IMr2axFoSMRAlOCnON8/QhE1fjmwBEQ=; b=qQ8SMC5/Dj9eDPHcJar+P0G+j9smIdl5eVUweuI+3lgVYxUnWkAzvpSzAusJ9oB9+q 0a1d51Anf7q8faMIbsbHNCQFxmpmYoCHojTrgRmcMpMF/s2qi1t1m/2ESXiOf+qvQ6Nc 7QdZfEskaBMzd8URKU6MX/olTnQtcIiJfMOmt8AtafEzBuPrfg62YB8QSG/E7MQ9ZT6V zDvPi9F8u36q1tuyWNKO+CEJvu6DFPrjXoyB/roCHvo+0jXZ1CpL8qWhFmo5h3DTPO9L vk7ds1/sW3AtuHds4ODSXmOjLMwGhrvYUHCXFiNjJLITdEfEVsSYAg5Mj4WXFPFWUNQ1 qtzw== X-Gm-Message-State: AMCzsaW1xUEHyPiO1ubQyoiETnFqyZz2HjC4/vBdrmMc6gwfRJ0IE5kR 48hANz+KqCamJbSts+IfVMD69w== X-Google-Smtp-Source: ABhQp+SrJR555r1O9kGDdjEOInOOdstIf2/popgf4X9zh3ycsBKFzWQXeaJ9qg335roQaRWXrPvpkQ== X-Received: by 10.80.213.214 with SMTP id g22mr16586195edj.277.1508912860216; Tue, 24 Oct 2017 23:27:40 -0700 (PDT) Received: from [192.168.1.79] (82.107.69.91.rev.sfr.net. [91.69.107.82]) by smtp.gmail.com with ESMTPSA id v17sm1215162eda.70.2017.10.24.23.27.39 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 24 Oct 2017 23:27:39 -0700 (PDT) To: Ferruh Yigit 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> <9e4a5217-ef03-861f-e17c-7eed22018b62@intel.com> From: Pascal Mazon Message-ID: <836bf8c7-c86d-16a3-6887-9a7964af7b39@6wind.com> Date: Wed, 25 Oct 2017 08:27:38 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 MIME-Version: 1.0 In-Reply-To: <9e4a5217-ef03-861f-e17c-7eed22018b62@intel.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Content-Language: en-US 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 06:27:40 -0000 On 25/10/2017 03:24, Ferruh Yigit wrote: > 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 Hi Ferruh, I concur with you, it definitely looks like speed is hardcoded in the kernel. And there is no set_settings() callback defined either, so it won't be possible to set anything using ethtool. I haven't seen the error and I don't think Vipin Varghese sent a log for that. I'm still not sure what the patch was fixing; it would be good to have that feedback to improve the patch. Best regards, Pascal > >>> + 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. >>