From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-vk0-f48.google.com (mail-vk0-f48.google.com [209.85.213.48]) by dpdk.org (Postfix) with ESMTP id E61C25AA5 for ; Fri, 13 May 2016 10:38:58 +0200 (CEST) Received: by mail-vk0-f48.google.com with SMTP id s184so128687090vkb.3 for ; Fri, 13 May 2016 01:38:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=netronome-com.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc; bh=mZclWQlqwMgaA/hu2Jt2lAQ/6XunzZ3h7O9MoAxvGHw=; b=bOJCW30F+Vr76MGARfPDgKxD+Knz8sF+bYD5SDPOMlE4w2fLkt+IZJ5SAmUuyZ/HDY 90mheFzMjgLaDqcmX+w9EFwOPINd4GrjNH2dKF/o5uTVzPcQ28pNNRZFGPvgzwJENjDr 2Ya8yn89pW3e/4MkMdlCE6a4I5hMQcsK3rDIUlQNiSjlj36zO3o3gUupFCK+IZRIPKKA CEnO5S9zPGCAm6zriqX7qj/KxqK9XO+xwDZYbmAs3fsK9Hh4HqP3AC1W9g+qEP1x7M8S DtQIFBJLAgiF72H54y8xOsTS5qhzKTQ17JL3SH7vGG9z4lFc2VXAZ7VomkLETGXsjiKW 7Ekg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc; bh=mZclWQlqwMgaA/hu2Jt2lAQ/6XunzZ3h7O9MoAxvGHw=; b=Lpw4B4HTZTgTwPqk3ojBWxQI+/SV/I8tKQGoK7mS3JGceS7ckDCqhqcUrL6wic6bd0 0Gebm4V7WBnUP8vmsGDO7kynlffAVPtplZXxotluOHXNbrHi/3Z+WsjU60THLhN3qeWs fDtlUDnBxizDGYscmcEiEkFP1mmCPdMM6YWfqqFjBXU8vmo6sYL3kuceF4IO3/kLo1ol Ubsl2PZF4bGDL6HzVF6wEm5PzKL8SoY8wOR0or+SWnruA4p+Hbfew+oq9ep372+Md7+C BCuxapT8iitOlfBBPeHxtzXMI1kv4+PAQhWeL/x8VowO1R7tTRZ3vJmkafmzJFqYPD5h qeug== X-Gm-Message-State: AOPr4FWLikMzoWMVKG8kZgKICyDsSWDum9kWuo3IgF67DHUBnvC7iV/uCtNXZ6wKfJZBv3gQ5CtKO0CpEWuuZyun MIME-Version: 1.0 X-Received: by 10.31.94.197 with SMTP id s188mr6058236vkb.126.1463128738331; Fri, 13 May 2016 01:38:58 -0700 (PDT) Received: by 10.103.112.129 with HTTP; Fri, 13 May 2016 01:38:58 -0700 (PDT) In-Reply-To: <20160512174110.5f0b3551@pcviktorin.fit.vutbr.cz> References: <1463063640-30715-3-git-send-email-alejandro.lucero@netronome.com> <20160512165219.6c6e6bb9@pcviktorin.fit.vutbr.cz> <20160512174110.5f0b3551@pcviktorin.fit.vutbr.cz> Date: Fri, 13 May 2016 09:38:58 +0100 Message-ID: From: Alejandro Lucero To: Jan Viktorin Cc: dev , Thomas Monjalon Content-Type: text/plain; charset=UTF-8 X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [dpdk-dev, 2/3] eth_dev: add support for device dma mask X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 13 May 2016 08:38:59 -0000 On Thu, May 12, 2016 at 4:41 PM, Jan Viktorin wrote: > Hi, > > Just a note, please, when replying inline, do not prepend ">" before your > new text. I could not find your replies. > > My gmail interface does not show that prepend character... It seems I have to leave a white line before my replies. > See below... > > On Thu, 12 May 2016 16:03:14 +0100 > Alejandro Lucero wrote: > > > Hi Jan > > > > On Thu, May 12, 2016 at 3:52 PM, Jan Viktorin > > wrote: > > > > > Hello Alejandro, > > > > > > On Thu, 12 May 2016 15:33:59 +0100 > > > "Alejandro.Lucero" wrote: > > > > > > > - New dma_mask field in rte_eth_dev_data. > > > > - If PMD sets device dma_mask, call to check hugepages within > > > > > > I think that one of the purposes of DPDK is to support DMA transfers > > > in userspace. In other words, I can see no reason to support dma_mask > > > at the ethdev level only. > > > > > > The limitation is a device limitation so I can not see a better place > for > > adding the device dma mask. > > That's what I've meant. It is a _device_ limitation. The ethdev is a > wrapper > around the rte_pci_device. The ethdev just extends semantics of the > generic device. > However, all DPDK devices are expected to be DMA-capable. > > If you get a pointer to the ethdev, you get a pointer to the > rte_pci_device as well > (1 more level of dereference but we are not on the fast path here, so it's > unimportant). > > Consider the cryptodev. If cryptodev has some DMA mask requirements we can > support it > in the generic place, i.e. rte_pci_device and not rte_ethdev because the > cryptodev > is not an ethdev. > > Ok. I was wrongly assuming we had just ethdevs, with the ethdev being the generic and rte_pci_device being a type of ethdev. I can add the dma mask to the rte_pci_dev. The extra level of dereference is not a problem as long as we do not use that dma mask for a more complex allocation API (more about this later). If I understand it right, work is in progress for adding a rte_device. I can not see a problem with adding dma mask to the rte_device struct either. > > > > > > > We should consider adding this to the generic struct rte_device > > > (currently rte_pci_device). Thomas? > > > > > > I guess it could be a non-pci device with such a limitation. I though > > rte_ethdev is more generic. > > When it is added to the rte_pci_device (or rte_device after the planned > changes) > the non-PCI devices get this for free... Otherwise I don't understand the > point > here. > > > > > > > > > supported range. > > > > > > I think, the '-' is unnecessary at the beginning of line. As for me > > > I prefer a fluent text describing the purpose. The '-' is useful for > > > a real list of notes. > > > > > > > > > > > Signed-off-by: Alejandro Lucero > > > > > > > > --- > > > > lib/librte_ether/rte_ethdev.c | 7 +++++++ > > > > lib/librte_ether/rte_ethdev.h | 1 + > > > > 2 files changed, 8 insertions(+) > > > > > > > > diff --git a/lib/librte_ether/rte_ethdev.c > > > b/lib/librte_ether/rte_ethdev.c > > > > index a31018e..c0de88a 100644 > > > > --- a/lib/librte_ether/rte_ethdev.c > > > > +++ b/lib/librte_ether/rte_ethdev.c > > > > @@ -280,9 +280,16 @@ rte_eth_dev_init(struct rte_pci_driver *pci_drv, > > > > > > > > /* Invoke PMD device initialization function */ > > > > diag = (*eth_drv->eth_dev_init)(eth_dev); > > > > + if (diag) > > > > + goto err; > > > > + > > > > + if (eth_dev->data->dma_mask) > > > > + diag = > > > rte_eal_hugepage_check_address_mask(eth_dev->data->dma_mask); > > > > + > > > > > > I don't understand what happens if the memory is out of the DMA mask. > What > > > can the driver > > > do? Does it just fail? > > > > > > I think that this should be considered during a malloc instead. (Well, > > > there is probably > > > no suitable API for this at the moment.) > > > > > > hugepage memory allocation is done before device initialization. I see > > easier to leave the normal hugepage code as it is now and add a later > call > > if a device requires it. > > True. I didn't meant to change hugepages allocation but to change > allocation > of memory for a device. If we reserve a set of hugepages, a subset of them > can > match the DMA mask for a particular device. When allocating memory for a > device, > we can consider the DMA mask and choose only the hugepages we can handle. > > Quite frankly, I am not very aware of the memory allocation internals of > DPDK so > I can be wrong... > > Of course, we could use a device dma mask smartly, and for ethdevs, to allocate device TX and RX rings and mbufs just from mempools with hugepages in the supported range. Any memory allocation by the app not related to packet handling could use another mempool with no dma mask restriction at all. This implies awareness by the app in some way, what I guess it would preferable to avoid. In our case, NFP PMD support, the restriction is 40bits which implies potential problems with machines above 1TB of memory. These type of machines are not likely to be used with our card, and even with those machines a DPDK app could work smoothly while allocated hugepages are below the 1TB. Even more, DPDK in VMs inside those machines will not have problems because the IOMMU (assuming SRIOV) will map the guest physical address which will be (not 100% sure about this for any hypervisor) below 1TB (assuming VMs do not get more than 1TB each, of course). So, a solution where this is managed "roughly" with an error if an allocated hugepage is out of range is acceptable for us. We first thought to just check the amount of memory in the system inside our PMD and return an error if more than 1TB but I prefer to raise an error just if we really need to regarding the allocated hugepages. I understand this could not be the case for other devices with more likeliness to have problems, and then a memory allocation API using devices dma mask makes sense. We could consider this a second step to support device limitation addresses and to implement that if really needed. > > > > The only reasonable thing to do is to fail as the amount of required > memory > > can not be (safely) allocated. > > This is the easiest way and it is not bad. However, what would be a > workaround? > If a user gets some allocated memory but out of the DMA mask, how can she > fix it? > I don't know... Is it deterministic? Would it always allocate memory out > of the > mask or only sometimes? > > Jan > > > > > > > > Regards > > > Jan > > > > > > > if (diag == 0) > > > > return 0; > > > > > > > > +err: > > > > RTE_PMD_DEBUG_TRACE("driver %s: eth_dev_init(vendor_id=0x%u > > > device_id=0x%x) failed\n", > > > > pci_drv->name, > > > > (unsigned) pci_dev->id.vendor_id, > > > > diff --git a/lib/librte_ether/rte_ethdev.h > > > b/lib/librte_ether/rte_ethdev.h > > > > index 2757510..34daa92 100644 > > > > --- a/lib/librte_ether/rte_ethdev.h > > > > +++ b/lib/librte_ether/rte_ethdev.h > > > > @@ -1675,6 +1675,7 @@ struct rte_eth_dev_data { > > > > enum rte_kernel_driver kdrv; /**< Kernel driver passthrough > */ > > > > int numa_node; /**< NUMA node connection */ > > > > const char *drv_name; /**< Driver name */ > > > > + uint64_t dma_mask; /** device supported address space range */ > > > > }; > > > > > > > > /** Device supports hotplug detach */ > > > > > > > > > > > > -- > > > Jan Viktorin E-mail: Viktorin@RehiveTech.com > > > System Architect Web: www.RehiveTech.com > > > RehiveTech > > > Brno, Czech Republic > > > > > > > -- > Jan Viktorin E-mail: Viktorin@RehiveTech.com > System Architect Web: www.RehiveTech.com > RehiveTech > Brno, Czech Republic >