From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id C93B8DE5 for ; Tue, 31 Jan 2017 17:06:59 +0100 (CET) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga101.fm.intel.com with ESMTP; 31 Jan 2017 08:06:41 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,315,1477983600"; d="scan'208";a="1120230782" Received: from fmsmsx104.amr.corp.intel.com ([10.18.124.202]) by fmsmga002.fm.intel.com with ESMTP; 31 Jan 2017 08:06:41 -0800 Received: from FMSMSX109.amr.corp.intel.com (10.18.116.9) by fmsmsx104.amr.corp.intel.com (10.18.124.202) with Microsoft SMTP Server (TLS) id 14.3.248.2; Tue, 31 Jan 2017 08:06:41 -0800 Received: from fmsmsx113.amr.corp.intel.com ([169.254.13.230]) by FMSMSX109.amr.corp.intel.com ([169.254.15.41]) with mapi id 14.03.0248.002; Tue, 31 Jan 2017 08:06:41 -0800 From: "Wiles, Keith" To: Pascal Mazon CC: "Yigit, Ferruh" , "dev@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH 1/6] net/tap: use correct tap name Thread-Index: AQHSe6ayvsixiw0FGEaIJx56SFpb9qFTFF8AgAAVogCAABIhgIAAAJMAgAACS4CAAAGGAIAAABoAgAAGMAA= Date: Tue, 31 Jan 2017 16:06:40 +0000 Message-ID: <74B7FC24-638D-4A48-A02D-C80B5B4E7E2B@intel.com> References: <1485855778-15496-1-git-send-email-pascal.mazon@6wind.com> <39a46c11-66c4-d8ed-a2bc-d9421cb4afd0@6wind.com> <9fa2c783-349c-e20c-0e57-0c24e67a4683@intel.com> <9C574760-33CF-4163-94D4-A0C919F4592C@intel.com> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.252.141.150] Content-Type: text/plain; charset="us-ascii" Content-ID: Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH 1/6] net/tap: use correct tap name 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, 31 Jan 2017 16:07:00 -0000 > On Jan 31, 2017, at 9:44 AM, Pascal Mazon wrote: >=20 > On 01/31/2017 04:44 PM, Wiles, Keith wrote: >>=20 >>> On Jan 31, 2017, at 9:38 AM, Yigit, Ferruh wro= te: >>>=20 >>> On 1/31/2017 3:30 PM, Pascal Mazon wrote: >>>> On 01/31/2017 04:28 PM, Ferruh Yigit wrote: >>>>> On 1/31/2017 2:23 PM, Pascal Mazon wrote: >>>>>> On 01/31/2017 02:06 PM, Ferruh Yigit wrote:> On 1/31/2017 9:42 AM, >>>>>> Pascal Mazon wrote: >>>>>>>> dev->data->name contains "net_tap", the device driver name. >>>>>>>=20 >>>>>>> I see what patch does, just as a note to commit log: >>>>>>>=20 >>>>>>> AFAIK, "dev->data->name" is device name, and for this case it is >>>>>>> "net_tap#", like "net_tap0", "net_tap1" ... >>>>>>>=20 >>>>>>> "dev->data_drv_name" is the driver name which is "net_tap" >>>>>>=20 >>>>>> Indeed, dev->data->name is the device name, looking like "net_tap#", >>>>>> with number increasing for each vdev. >>>>>> I'll put the following commit log line if that's ok: >>>>>>=20 >>>>>> dev->data->name contains the device name, e.g. "net_tap0". >>>>>>=20 >>>>>>>=20 >>>>>>>> dev->data->dev_private->name contains the actual iface name, >>>>>>>> e.g. "dtap0". >>>>>>>=20 >>>>>>> Right, I agree this is correct comparing "dev->data->name" >>>>>>>=20 >>>>>>> But the problem is pmd->name is per eth_dev. >>>>>>>=20 >>>>>>> If I read code correct, for multiple queue support, each queue pair= will >>>>>>> create a tap device, so each needs a different name. >>>>>>>=20 >>>>>>> So can't just use pmd->name. Need to create a name per queue pair, = it >>>>>>> can be combination of pmd->name + "_" + queue_id? Or can keep a nam= e per >>>>>>> queue pair, instead of eth_dev. >>>>>>>=20 >>>>>>> What do you think? >>>>>>=20 >>>>>> Actually that's not exactly how it goes. >>>>>> Adding a queue to a netdevice requires to open("/dev/net/tun") and s= etting >>>>>> TUNSETIFF (through ioctl) on the resulting fd. >>>>>> That's the important part: queues for a given tap device must set TU= NSETIFF >>>>>> with a common ifreq.ifr_name (in our case, pmd->name). Looking at the changes to set the link up/down and the adding the two funct= ions. I noticed in the stop/start routines I was set the link in DPDK and n= ot adjusting the interface link. Should the stop/start routine also do the = same thing? >>>>>>=20 >>>>>> This is best explained in the kernel doc, there: >>>>>=20 >>>>> Thank you for the clarification. >>>>> If so, why PMD keeps a fd per queue? >>>>>=20 >>>>> Overall, patch looks good except mentioned detail in commit log. >>>>>=20 >>>>> I suggest waiting Keith's patch, and rebase this set on top of it. >>>>>=20 >>>>> Thanks, >>>>> ferruh >>>>>=20 >>>>>>=20 >>>>>> [1] >>>>>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree= /Documentation/networking/tuntap.txt#n108 >>>>>>=20 >>>>> <...> >>>>>=20 >>>>=20 >>>> I would say it is because dev->{r/t}x_pkt_burst() is done per queue. >>>> In these functions, we need to make our read() and write() accesses on= the >>>> right fd considering the queue we're required to process. >>>=20 >>> If fd is same for all queues, it is possible to keep one instance in pm= d >>> private_data, and access it from queues. I think it is confusing to hav= e >>> multiple copy of it, but I see your point. >>=20 >> In my changes I removed the fds[] array as it was not required only the = rx/tx_queue has an fd variable. >=20 > Sounds good to me. >=20 > Ferruh, just to make sure we're on the same page, in multiqueue, there is= definitely a different fd for each queue. >=20 >>=20 >>>=20 >>>>=20 >>>> I'll wait for Keith's patch, then. >>>=20 >>> Thanks again. >>>=20 >>>>=20 >>>> Best regards, >>=20 >> Regards, >> Keith >>=20 >=20 >=20 > --=20 > Pascal Mazon > www.6wind.com Regards, Keith