[dpdk-dev] [PATCH] eal: map PCI memory resources after hugepages

[dpdk-dev] [PATCH] eal: map PCI memory resources after hugepages

[Liang Xu]

A multiple process DPDK application must mmap hugepages and pci resources into 
same virtual addresses. By default the virtual addresses chosen by the primary 
process automatically when calling the mmap. But sometime the chosen virtual 
addresses isn't usable at secondary process. Such as the secondary process 
linked with more libraries than primary process. The library has been mapped 
into this virtual address. The command line parameter 'base-virtaddr' has been 
added for this situation. If it's configured, the hugepages will be mapped into 
this base address. But the virtual address of pci resources mapped still does 
not refer to the parameter. In that case "EAL: pci_map_resource(): cannot mmap" 
will be got.

This patch try to map pci resources after hugepages. So the error can be 
resolved by set base-virtaddr into free virtual address space.

Signed-off-by: Liang Xu <liang.xu@cinfotech.cn>
---
 lib/librte_eal/linuxapp/eal/eal_pci.c | 32 +++++++++++++++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
index ddb0535..502eef2 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
@@ -97,14 +97,42 @@ error:
 	return -1;
 }
 
+static void *
+pci_find_max_end_va(void)
+{
+	const struct rte_memseg *seg = rte_eal_get_physmem_layout();
+	const struct rte_memseg *last = seg;
+	unsigned i = 0;
+
+	for (i = 0; i < RTE_MAX_MEMSEG; i++, seg++) {
+		if (seg->addr == NULL)
+			break;
+
+		if (seg->addr > last->addr)
+			last = seg;
+
+	}
+	return RTE_PTR_ADD(last->addr, last->len);
+}
+
 /* map a particular resource from a file */
 void *
 pci_map_resource(void *requested_addr, int fd, off_t offset, size_t size)
 {
 	void *mapaddr;
 
+	/* By default the PCI memory resource will be mapped after hugepages */
+	static void *default_map_addr;
+	if (NULL == requested_addr) {
+		if (NULL == default_map_addr)
+			default_map_addr = pci_find_max_end_va();
+		mapaddr = default_map_addr;
+	} else {
+	    mapaddr = requested_addr;
+	}
+
 	/* Map the PCI memory resource of device */
-	mapaddr = mmap(requested_addr, size, PROT_READ | PROT_WRITE,
+	mapaddr = mmap(mapaddr, size, PROT_READ | PROT_WRITE,
 			MAP_SHARED, fd, offset);
 	if (mapaddr == MAP_FAILED ||
 			(requested_addr != NULL && mapaddr != requested_addr)) {
@@ -114,6 +142,8 @@ pci_map_resource(void *requested_addr, int fd, off_t offset, size_t size)
 			strerror(errno), mapaddr);
 		goto fail;
 	}
+	if (NULL == requested_addr)
+		default_map_addr = RTE_PTR_ADD(mapaddr, size);
 
 	RTE_LOG(DEBUG, EAL, "  PCI memory mapped at %p\n", mapaddr);
 
-- 
1.9.1

Re: [dpdk-dev] [PATCH] eal: map PCI memory resources after hugepages

[Burakov, Anatoly]

Hi Liang

I don't think that overriding the value passed to pci_map_resource as argument is the way to go. While it results in less code, it looks weird, in my opinion at least, as I believe tracking the correctness of address being requested should be the responsibility of the caller, i.e. either UIO or VFIO code. Which is why I keep insisting that you make requested_pci_addr global to linuxapp EAL PCI section and put it into include/eal_pci_init.h. Would you mind if I made a patch for this issue based on your code?

Thanks,
Anatoly

-----Original Message-----
From: Liang Xu [mailto:liang.xu@cinfotech.cn] 
Sent: Saturday, November 8, 2014 3:32 AM
To: dev@dpdk.org
Cc: Burakov, Anatoly; thomas.monjalon@6wind.com
Subject: [PATCH] eal: map PCI memory resources after hugepages

A multiple process DPDK application must mmap hugepages and pci resources into same virtual addresses. By default the virtual addresses chosen by the primary process automatically when calling the mmap. But sometime the chosen virtual addresses isn't usable at secondary process. Such as the secondary process linked with more libraries than primary process. The library has been mapped into this virtual address. The command line parameter 'base-virtaddr' has been added for this situation. If it's configured, the hugepages will be mapped into this base address. But the virtual address of pci resources mapped still does not refer to the parameter. In that case "EAL: pci_map_resource(): cannot mmap" 
will be got.

This patch try to map pci resources after hugepages. So the error can be resolved by set base-virtaddr into free virtual address space.

Signed-off-by: Liang Xu <liang.xu@cinfotech.cn>
---
 lib/librte_eal/linuxapp/eal/eal_pci.c | 32 +++++++++++++++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
index ddb0535..502eef2 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
@@ -97,14 +97,42 @@ error:
 	return -1;
 }
 
+static void *
+pci_find_max_end_va(void)
+{
+	const struct rte_memseg *seg = rte_eal_get_physmem_layout();
+	const struct rte_memseg *last = seg;
+	unsigned i = 0;
+
+	for (i = 0; i < RTE_MAX_MEMSEG; i++, seg++) {
+		if (seg->addr == NULL)
+			break;
+
+		if (seg->addr > last->addr)
+			last = seg;
+
+	}
+	return RTE_PTR_ADD(last->addr, last->len); }
+
 /* map a particular resource from a file */  void *  pci_map_resource(void *requested_addr, int fd, off_t offset, size_t size)  {
 	void *mapaddr;
 
+	/* By default the PCI memory resource will be mapped after hugepages */
+	static void *default_map_addr;
+	if (NULL == requested_addr) {
+		if (NULL == default_map_addr)
+			default_map_addr = pci_find_max_end_va();
+		mapaddr = default_map_addr;
+	} else {
+	    mapaddr = requested_addr;
+	}
+
 	/* Map the PCI memory resource of device */
-	mapaddr = mmap(requested_addr, size, PROT_READ | PROT_WRITE,
+	mapaddr = mmap(mapaddr, size, PROT_READ | PROT_WRITE,
 			MAP_SHARED, fd, offset);
 	if (mapaddr == MAP_FAILED ||
 			(requested_addr != NULL && mapaddr != requested_addr)) { @@ -114,6 +142,8 @@ pci_map_resource(void *requested_addr, int fd, off_t offset, size_t size)
 			strerror(errno), mapaddr);
 		goto fail;
 	}
+	if (NULL == requested_addr)
+		default_map_addr = RTE_PTR_ADD(mapaddr, size);
 
 	RTE_LOG(DEBUG, EAL, "  PCI memory mapped at %p\n", mapaddr);
 
--
1.9.1

Re: [dpdk-dev] [PATCH] eal: map PCI memory resources after hugepages

[XU Liang]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="UTF-8", Size: 7580 bytes --]

It is a default value when the requested_addr isn't exist, not an overide. When the pci_map_resource is called at the primary process, the requested_addr is NULL. The default value will be provided by default_map_addr.
When the pci_map_resource is called at the secondery process, the requested_addr is exist. Then everything isn't be changed.------------------------------------------------------------------From:Burakov, Anatoly <anatoly.burakov@intel.com>Time:2014 Nov 10 (Mon) 17 : 54To:徐亮 <liang.xu@cinfotech.cn>, dev@dpdk.org <dev@dpdk.org>Cc:thomas.monjalon@6wind.com <thomas.monjalon@6wind.com>Subject:RE: [PATCH] eal: map PCI memory resources after hugepages
Hi Liang

I don't think that overriding the value passed to pci_map_resource as argument is the way to go. While it results in less code, it looks weird, in my opinion at least, as I believe tracking the correctness of address being requested should be the responsibility of the caller, i.e. either UIO or VFIO code. Which is why I keep insisting that you make requested_pci_addr global to linuxapp EAL PCI section and put it into include/eal_pci_init.h. Would you mind if I made a patch for this issue based on your code?

Thanks,
Anatoly

-----Original Message-----
From: Liang Xu [mailto:liang.xu@cinfotech.cn] 
Sent: Saturday, November 8, 2014 3:32 AM
To: dev@dpdk.org
Cc: Burakov, Anatoly; thomas.monjalon@6wind.com
Subject: [PATCH] eal: map PCI memory resources after hugepages

A multiple process DPDK application must mmap hugepages and pci resources into same virtual addresses. By default the virtual addresses chosen by the primary process automatically when calling the mmap. But sometime the chosen virtual addresses isn't usable at secondary process. Such as the secondary process linked with more libraries than primary process. The library has been mapped into this virtual address. The command line parameter 'base-virtaddr' has been added for this situation. If it's configured, the hugepages will be mapped into this base address. But the virtual address of pci resources mapped still does not refer to the parameter. In that case "EAL: pci_map_resource(): cannot mmap" 
will be got.

This patch try to map pci resources after hugepages. So the error can be resolved by set base-virtaddr into free virtual address space.

Signed-off-by: Liang Xu <liang.xu@cinfotech.cn>
---
 lib/librte_eal/linuxapp/eal/eal_pci.c | 32 +++++++++++++++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
index ddb0535..502eef2 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
@@ -97,14 +97,42 @@ error:
 	return -1;
 }
 
+static void *
+pci_find_max_end_va(void)
+{
+	const struct rte_memseg *seg = rte_eal_get_physmem_layout();
+	const struct rte_memseg *last = seg;
+	unsigned i = 0;
+
+	for (i = 0; i < RTE_MAX_MEMSEG; i++, seg++) {
+		if (seg->addr == NULL)
+			break;
+
+		if (seg->addr > last->addr)
+			last = seg;
+
+	}
+	return RTE_PTR_ADD(last->addr, last->len); }
+
 /* map a particular resource from a file */  void *  pci_map_resource(void *requested_addr, int fd, off_t offset, size_t size)  {
 	void *mapaddr;
 
+	/* By default the PCI memory resource will be mapped after hugepages */
+	static void *default_map_addr;
+	if (NULL == requested_addr) {
+		if (NULL == default_map_addr)
+			default_map_addr = pci_find_max_end_va();
+		mapaddr = default_map_addr;
+	} else {
+	    mapaddr = requested_addr;
+	}
+
 	/* Map the PCI memory resource of device */
-	mapaddr = mmap(requested_addr, size, PROT_READ | PROT_WRITE,
+	mapaddr = mmap(mapaddr, size, PROT_READ | PROT_WRITE,
 			MAP_SHARED, fd, offset);
 	if (mapaddr == MAP_FAILED ||
 			(requested_addr != NULL && mapaddr != requested_addr)) { @@ -114,6 +142,8 @@ pci_map_resource(void *requested_addr, int fd, off_t offset, size_t size)
 			strerror(errno), mapaddr);
 		goto fail;
 	}
+	if (NULL == requested_addr)
+		default_map_addr = RTE_PTR_ADD(mapaddr, size);
 
 	RTE_LOG(DEBUG, EAL, "  PCI memory mapped at %p\n", mapaddr);
 
--
1.9.1
From thomas.monjalon@6wind.com  Mon Nov 10 10:54:11 2014
Return-Path: <thomas.monjalon@6wind.com>
Received: from mail-wi0-f172.google.com (mail-wi0-f172.google.com
 [209.85.212.172]) by dpdk.org (Postfix) with ESMTP id C38582E89
 for <dev@dpdk.org>; Mon, 10 Nov 2014 10:54:11 +0100 (CET)
Received: by mail-wi0-f172.google.com with SMTP id bs8so9734150wib.11
 for <dev@dpdk.org>; Mon, 10 Nov 2014 02:03:56 -0800 (PST)
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d\x1e100.net; s 130820;
 h=x-gm-message-state:from:to:cc:subject:date:message-id:organization
 :user-agent:in-reply-to:references:mime-version
 :content-transfer-encoding:content-type;
 bh=boS8myAu9T2LhsGLlflh1gTZs6fn3na8zppnVl5Nkt0=;
 b=IaJOJUS/JrF4U3PkAUw2VydRtST+peS3IpSMmXPLNsuEZYzxeO43UJso5BOmcI/mMq
 QzHhckAcg6gVVNbR3zSknV5t95rBtKFzqMOBSZkBNp5zx6lraLSV7cOWsxz69QSTn5aa
 IJScbNMrzQ4oFhnBddxnewYwgjrbYfZCKkMlb1aXAmD0oowSG9fNpwqPNsiBjfKCdIni
 r1qNErpwflVOVCJD3Cd8Fg/ZEbkrnS0fgSp4bdJqzIDLIYxnlWX/iRhPQ28Er+mIWKUA
 WdoZgxmnG7FNDPwINoehsEZy2Jk0gj01APXuEO6fotBAxBcy9cgGjNBS0YFnOgefwHtX
 WKew=X-Gm-Message-State: ALoCoQlQQ/m8Xk75za5kGhi2XnYX3zSCMpPf7ucHc7ycV0rAJmgo0ezWQbkZpGOT69QBzeeh1O1E
X-Received: by 10.180.19.164 with SMTP id g4mr28712962wie.51.1415613836517;
 Mon, 10 Nov 2014 02:03:56 -0800 (PST)
Received: from xps13.localnet (136-92-190-109.dsl.ovh.fr. [109.190.92.136])
 by mx.google.com with ESMTPSA id hk9sm22516289wjb.46.2014.11.10.02.03.55
 for <multiple recipients>
 (version=TLSv1.2 cipherìDHE-RSA-AES128-GCM-SHA256 bits\x128/128);
 Mon, 10 Nov 2014 02:03:55 -0800 (PST)
From: Thomas Monjalon <thomas.monjalon@6wind.com>
To: "Sujith Sankar (ssujith)" <ssujith@cisco.com>
Date: Mon, 10 Nov 2014 11:03:37 +0100
Message-ID: <2661634.q9QAmChB5I@xps13>
Organization: 6WIND
User-Agent: KMail/4.14.2 (Linux/3.17.2-1-ARCH; KDE/4.14.2; x86_64; ; )
In-Reply-To: <D08682B4.27321%ssujith@cisco.com>
References: <1415390747-9532-1-git-send-email-ssujith@cisco.com>
 <1794590.eegJuKmrRB@xps13> <D08682B4.27321%ssujith@cisco.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 7Bit
Content-Type: text/plain; charset="us-ascii"
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH 0/7] Cisco Systems Inc. VIC Ethernet PMD -
	ENIC PMD
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Mon, 10 Nov 2014 09:54:12 -0000

2014-11-10 09:27, Sujith Sankar:
> On 07/11/14 9:17 pm, "Thomas Monjalon" <thomas.monjalon@6wind.com> wrote:
> >It seems that this PMD is based on DPDK 1.7.
> >Could you rebase it on HEAD?
>
> This patch is based on 1.7.1.  Thought that is the latest.  And I got the
> diff from origin.
> What made you feel that the patch is from 1.7?

By saying 1.7, I meant 1.7.0 or 1.7.1.
In current HEAD (future 1.8.0), there is a lot of changes which make your PMD
incompatible. That's why the rule is to base the patches on the latest HEAD.

Thanks for your efforts.
--
Thomas

Re: [dpdk-dev] [PATCH] eal: map PCI memory resources after hugepages

[XU Liang]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="UTF-8", Size: 8520 bytes --]

Of course you can take this job. Thanks you for your help.------------------------------------------------------------------From:徐亮 <liang.xu@cinfotech.cn>Time:2014 Nov 10 (Mon) 18 : 01To:Burakov, Anatoly <anatoly.burakov@intel.com>, dev@dpdk.org <dev@dpdk.org>Cc:thomas.monjalon@6wind.com <thomas.monjalon@6wind.com>Subject:Re: [PATCH] eal: map PCI memory resources after hugepages
It is a default value when the requested_addr isn't exist, not an overide. When the pci_map_resource is called at the primary process, the requested_addr is NULL. The default value will be provided by default_map_addr.
When the pci_map_resource is called at the secondery process, the requested_addr is exist. Then everything isn't be changed.------------------------------------------------------------------From:Burakov, Anatoly <anatoly.burakov@intel.com>Time:2014 Nov 10 (Mon) 17 : 54To:徐亮 <liang.xu@cinfotech.cn>, dev@dpdk.org <dev@dpdk.org>Cc:thomas.monjalon@6wind.com <thomas.monjalon@6wind.com>Subject:RE: [PATCH] eal: map PCI memory resources after hugepages
Hi Liang

I don't think that overriding the value passed to pci_map_resource as argument is the way to go. While it results in less code, it looks weird, in my opinion at least, as I believe tracking the correctness of address being requested should be the responsibility of the caller, i.e. either UIO or VFIO code. Which is why I keep insisting that you make requested_pci_addr global to linuxapp EAL PCI section and put it into include/eal_pci_init.h. Would you mind if I made a patch for this issue based on your code?

Thanks,
Anatoly

-----Original Message-----
From: Liang Xu [mailto:liang.xu@cinfotech.cn] 
Sent: Saturday, November 8, 2014 3:32 AM
To: dev@dpdk.org
Cc: Burakov, Anatoly; thomas.monjalon@6wind.com
Subject: [PATCH] eal: map PCI memory resources after hugepages

A multiple process DPDK application must mmap hugepages and pci resources into same virtual addresses. By default the virtual addresses chosen by the primary process automatically when calling the mmap. But sometime the chosen virtual addresses isn't usable at secondary process. Such as the secondary process linked with more libraries than primary process. The library has been mapped into this virtual address. The command line parameter 'base-virtaddr' has been added for this situation. If it's configured, the hugepages will be mapped into this base address. But the virtual address of pci resources mapped still does not refer to the parameter. In that case "EAL: pci_map_resource(): cannot mmap" 
will be got.

This patch try to map pci resources after hugepages. So the error can be resolved by set base-virtaddr into free virtual address space.

Signed-off-by: Liang Xu <liang.xu@cinfotech.cn>
---
 lib/librte_eal/linuxapp/eal/eal_pci.c | 32 +++++++++++++++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
index ddb0535..502eef2 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
@@ -97,14 +97,42 @@ error:
 	return -1;
 }
 
+static void *
+pci_find_max_end_va(void)
+{
+	const struct rte_memseg *seg = rte_eal_get_physmem_layout();
+	const struct rte_memseg *last = seg;
+	unsigned i = 0;
+
+	for (i = 0; i < RTE_MAX_MEMSEG; i++, seg++) {
+		if (seg->addr == NULL)
+			break;
+
+		if (seg->addr > last->addr)
+			last = seg;
+
+	}
+	return RTE_PTR_ADD(last->addr, last->len); }
+
 /* map a particular resource from a file */  void *  pci_map_resource(void *requested_addr, int fd, off_t offset, size_t size)  {
 	void *mapaddr;
 
+	/* By default the PCI memory resource will be mapped after hugepages */
+	static void *default_map_addr;
+	if (NULL == requested_addr) {
+		if (NULL == default_map_addr)
+			default_map_addr = pci_find_max_end_va();
+		mapaddr = default_map_addr;
+	} else {
+	    mapaddr = requested_addr;
+	}
+
 	/* Map the PCI memory resource of device */
-	mapaddr = mmap(requested_addr, size, PROT_READ | PROT_WRITE,
+	mapaddr = mmap(mapaddr, size, PROT_READ | PROT_WRITE,
 			MAP_SHARED, fd, offset);
 	if (mapaddr == MAP_FAILED ||
 			(requested_addr != NULL && mapaddr != requested_addr)) { @@ -114,6 +142,8 @@ pci_map_resource(void *requested_addr, int fd, off_t offset, size_t size)
 			strerror(errno), mapaddr);
 		goto fail;
 	}
+	if (NULL == requested_addr)
+		default_map_addr = RTE_PTR_ADD(mapaddr, size);
 
 	RTE_LOG(DEBUG, EAL, "  PCI memory mapped at %p\n", mapaddr);
 
--
1.9.1
From ssujith@cisco.com  Mon Nov 10 10:57:17 2014
Return-Path: <ssujith@cisco.com>
Received: from alln-iport-3.cisco.com (alln-iport-3.cisco.com [173.37.142.90])
 by dpdk.org (Postfix) with ESMTP id 44EAC6828
 for <dev@dpdk.org>; Mon, 10 Nov 2014 10:57:17 +0100 (CET)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple;
 d=cisco.com; i=@cisco.com; lw4; q=dns/txt; s=iport;
 t\x1415614022; x\x1416823622;
 h=from:to:cc:subject:date:message-id:references:
 in-reply-to:content-id:content-transfer-encoding: mime-version;
 bh=uFCpu3xF3k70iotuGG9s8nhdOekv3HF6V7jwnh1pucI=;
 b=nDLdStQdJC6SKZYRqIVHzP8YPwAkN7ffsBIGOeq7/ByWvV/MITZhBSb+
 qj8z3rchoIzb0lU8EbRUukjSB26JgUcegPbi/lyN9l2z2lMangcIPeO8K
 OaTS6DJ0NfumrNhIl14VUl+YxnLs0UnewvrQiL3EjCutjz+jENvqE4ecV Y=;
X-IronPort-Anti-Spam-Filtered: true
X-IronPort-Anti-Spam-Result: AhEFACWNYFStJV2b/2dsb2JhbABbgw6BLQTTRAKBJRYBAQEBAX2EAwEBBHkQAgEIRjIlAgQOBYhBzhMBAQEBAQEBAQEBAQEBAQEBAQEBGZEVB4RLAQSLRoZri3SBNJEihAqDemyBSIEDAQEB
X-IronPort-AV: E=Sophos;i="5.07,351,1413244800"; d="scan'208";a="95059985"
Received: from rcdn-core-4.cisco.com ([173.37.93.155])
 by alln-iport-3.cisco.com with ESMTP; 10 Nov 2014 10:07:01 +0000
Received: from xhc-rcd-x04.cisco.com (xhc-rcd-x04.cisco.com [173.37.183.78])
 by rcdn-core-4.cisco.com (8.14.5/8.14.5) with ESMTP id sAAA71SJ000931
 (version=TLSv1/SSLv3 cipher®S128-SHA bits\x128 verifyúIL);
 Mon, 10 Nov 2014 10:07:01 GMT
Received: from xmb-aln-x07.cisco.com ([169.254.2.60]) by xhc-rcd-x04.cisco.com
 ([fe80::200:5efe:173.37.183.34%12]) with mapi id 14.03.0195.001;
 Mon, 10 Nov 2014 04:07:01 -0600
From: "Sujith Sankar (ssujith)" <ssujith@cisco.com>
To: Thomas Monjalon <thomas.monjalon@6wind.com>
Thread-Topic: [dpdk-dev] [PATCH 0/7] Cisco Systems Inc. VIC Ethernet PMD -
 ENIC PMD
Thread-Index: AQHP+na8i7p0q5/9ukKS4GqBREUQP5xVs+WAgASpMAD//63DgIAAXSuA
Date: Mon, 10 Nov 2014 10:07:00 +0000
Message-ID: <D0868BD9.27351%ssujith@cisco.com>
References: <1415390747-9532-1-git-send-email-ssujith@cisco.com>
 <1794590.eegJuKmrRB@xps13> <D08682B4.27321%ssujith@cisco.com>
 <2661634.q9QAmChB5I@xps13>
In-Reply-To: <2661634.q9QAmChB5I@xps13>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
user-agent: Microsoft-MacOutlook/14.3.9.131030
x-originating-ip: [10.127.148.111]
Content-Type: text/plain; charset="iso-8859-1"
Content-ID: <DF103A805D2A6C4AAA21BFAEF841F571@emea.cisco.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH 0/7] Cisco Systems Inc. VIC Ethernet PMD -
 ENIC PMD
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Mon, 10 Nov 2014 09:57:17 -0000

Thanks for the clear response.  I¹ll take a look at it.

Regards,
-Sujith

On 10/11/14 3:33 pm, "Thomas Monjalon" <thomas.monjalon@6wind.com> wrote:

>2014-11-10 09:27, Sujith Sankar:
>> On 07/11/14 9:17 pm, "Thomas Monjalon" <thomas.monjalon@6wind.com>
>>wrote:
>> >It seems that this PMD is based on DPDK 1.7.
>> >Could you rebase it on HEAD?
>> 
>> This patch is based on 1.7.1.  Thought that is the latest.  And I got
>>the
>> diff from origin.
>> What made you feel that the patch is from 1.7?
>
>By saying 1.7, I meant 1.7.0 or 1.7.1.
>In current HEAD (future 1.8.0), there is a lot of changes which make your
>PMD
>incompatible. That's why the rule is to base the patches on the latest
>HEAD.
>
>Thanks for your efforts.
>-- 
>Thomas

Re: [dpdk-dev] [PATCH] eal: map PCI memory resources after hugepages

[XU Liang]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="UTF-8", Size: 11770 bytes --]

By the way, the pci_map_resource will check the mapaddr == requested_addr. So you must provide a usable requested_addr. It's a reason I modified the pci_map_resource().------------------------------------------------------------------From:徐亮 <liang.xu@cinfotech.cn>Time:2014 Nov 10 (Mon) 18 : 04To:Burakov, Anatoly <anatoly.burakov@intel.com>, dev@dpdk.org <dev@dpdk.org>Subject:Re: [dpdk-dev] [PATCH] eal: map PCI memory resources after hugepages
Of course you can take this job. Thanks you for your help.------------------------------------------------------------------From:徐亮 <liang.xu@cinfotech.cn>Time:2014 Nov 10 (Mon) 18 : 01To:Burakov, Anatoly <anatoly.burakov@intel.com>, dev@dpdk.org <dev@dpdk.org>Cc:thomas.monjalon@6wind.com <thomas.monjalon@6wind.com>Subject:Re: [PATCH] eal: map PCI memory resources after hugepages
It is a default value when the requested_addr isn't exist, not an overide. When the pci_map_resource is called at the primary process, the requested_addr is NULL. The default value will be provided by default_map_addr.
When the pci_map_resource is called at the secondery process, the requested_addr is exist. Then everything isn't be changed.------------------------------------------------------------------From:Burakov, Anatoly <anatoly.burakov@intel.com>Time:2014 Nov 10 (Mon) 17 : 54To:徐亮 <liang.xu@cinfotech.cn>, dev@dpdk.org <dev@dpdk.org>Cc:thomas.monjalon@6wind.com <thomas.monjalon@6wind.com>Subject:RE: [PATCH] eal: map PCI memory resources after hugepages
Hi Liang

I don't think that overriding the value passed to pci_map_resource as argument is the way to go. While it results in less code, it looks weird, in my opinion at least, as I believe tracking the correctness of address being requested should be the responsibility of the caller, i.e. either UIO or VFIO code. Which is why I keep insisting that you make requested_pci_addr global to linuxapp EAL PCI section and put it into include/eal_pci_init.h. Would you mind if I made a patch for this issue based on your code?

Thanks,
Anatoly

-----Original Message-----
From: Liang Xu [mailto:liang.xu@cinfotech.cn] 
Sent: Saturday, November 8, 2014 3:32 AM
To: dev@dpdk.org
Cc: Burakov, Anatoly; thomas.monjalon@6wind.com
Subject: [PATCH] eal: map PCI memory resources after hugepages

A multiple process DPDK application must mmap hugepages and pci resources into same virtual addresses. By default the virtual addresses chosen by the primary process automatically when calling the mmap. But sometime the chosen virtual addresses isn't usable at secondary process. Such as the secondary process linked with more libraries than primary process. The library has been mapped into this virtual address. The command line parameter 'base-virtaddr' has been added for this situation. If it's configured, the hugepages will be mapped into this base address. But the virtual address of pci resources mapped still does not refer to the parameter. In that case "EAL: pci_map_resource(): cannot mmap" 
will be got.

This patch try to map pci resources after hugepages. So the error can be resolved by set base-virtaddr into free virtual address space.

Signed-off-by: Liang Xu <liang.xu@cinfotech.cn>
---
 lib/librte_eal/linuxapp/eal/eal_pci.c | 32 +++++++++++++++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
index ddb0535..502eef2 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
@@ -97,14 +97,42 @@ error:
 	return -1;
 }
 
+static void *
+pci_find_max_end_va(void)
+{
+	const struct rte_memseg *seg = rte_eal_get_physmem_layout();
+	const struct rte_memseg *last = seg;
+	unsigned i = 0;
+
+	for (i = 0; i < RTE_MAX_MEMSEG; i++, seg++) {
+		if (seg->addr == NULL)
+			break;
+
+		if (seg->addr > last->addr)
+			last = seg;
+
+	}
+	return RTE_PTR_ADD(last->addr, last->len); }
+
 /* map a particular resource from a file */  void *  pci_map_resource(void *requested_addr, int fd, off_t offset, size_t size)  {
 	void *mapaddr;
 
+	/* By default the PCI memory resource will be mapped after hugepages */
+	static void *default_map_addr;
+	if (NULL == requested_addr) {
+		if (NULL == default_map_addr)
+			default_map_addr = pci_find_max_end_va();
+		mapaddr = default_map_addr;
+	} else {
+	    mapaddr = requested_addr;
+	}
+
 	/* Map the PCI memory resource of device */
-	mapaddr = mmap(requested_addr, size, PROT_READ | PROT_WRITE,
+	mapaddr = mmap(mapaddr, size, PROT_READ | PROT_WRITE,
 			MAP_SHARED, fd, offset);
 	if (mapaddr == MAP_FAILED ||
 			(requested_addr != NULL && mapaddr != requested_addr)) { @@ -114,6 +142,8 @@ pci_map_resource(void *requested_addr, int fd, off_t offset, size_t size)
 			strerror(errno), mapaddr);
 		goto fail;
 	}
+	if (NULL == requested_addr)
+		default_map_addr = RTE_PTR_ADD(mapaddr, size);
 
 	RTE_LOG(DEBUG, EAL, "  PCI memory mapped at %p\n", mapaddr);
 
--
1.9.1
From nhorman@tuxdriver.com  Mon Nov 10 12:10:38 2014
Return-Path: <nhorman@tuxdriver.com>
Received: from smtp.tuxdriver.com (charlotte.tuxdriver.com [70.61.120.58])
 by dpdk.org (Postfix) with ESMTP id 104D56828
 for <dev@dpdk.org>; Mon, 10 Nov 2014 12:10:38 +0100 (CET)
Received: from hmsreliant.think-freely.org
 ([2001:470:8:a08:7aac:c0ff:fec2:933b] helo=localhost)
 by smtp.tuxdriver.com with esmtpsa (TLSv1:AES128-SHA:128) (Exim 4.63)
 (envelope-from <nhorman@tuxdriver.com>)
 id 1Xnn14-0000fV-UC; Mon, 10 Nov 2014 06:20:21 -0500
Date: Mon, 10 Nov 2014 06:20:13 -0500
From: Neil Horman <nhorman@tuxdriver.com>
To: "Sujith Sankar (ssujith)" <ssujith@cisco.com>
Message-ID: <20141110112013.GA26613@hmsreliant.think-freely.org>
References: <1415390747-9532-1-git-send-email-ssujith@cisco.com>
 <1415390747-9532-4-git-send-email-ssujith@cisco.com>
 <20141107113449.GC25469@hmsreliant.think-freely.org>
 <D082E709.27161%ssujith@cisco.com>
 <D0868974.2733D%ssujith@cisco.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <D0868974.2733D%ssujith@cisco.com>
User-Agent: Mutt/1.5.23 (2014-03-12)
X-Spam-Score: -2.9 (--)
X-Spam-Status: No
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH 3/7] ENIC PMD Makefile
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Mon, 10 Nov 2014 11:10:38 -0000

On Mon, Nov 10, 2014 at 09:59:45AM +0000, Sujith Sankar (ssujith) wrote:
> Neil,
>
> If I move the DPDK patch that accommodates ENIC PMD (that is the one that
> patches lib/Makefile) to the last in the series, builds between commits
> would succeed, wouldn¹t it?  Moving that to the last is anyway needed.
>
correct, yes.
Neil

> Thanks,
> -Sujith
>
> On 07/11/14 9:16 pm, "Sujith Sankar (ssujith)" <ssujith@cisco.com> wrote:
>
> >Hi Neil,
> >
> >Thanks for the comments.  I shall work on the modifications that you have
> >suggested and get back with V2.
> >
> >Regards,
> >-Sujith
> >
> >On 07/11/14 5:04 pm, "Neil Horman" <nhorman@tuxdriver.com> wrote:
> >
> >>On Sat, Nov 08, 2014 at 01:35:43AM +0530, Sujith Sankar wrote:
> >>> Signed-off-by: Sujith Sankar <ssujith@cisco.com>
> >>> ---
> >>>  lib/librte_pmd_enic/Makefile | 66
> >>>++++++++++++++++++++++++++++++++++++++++++++
> >>>  1 file changed, 66 insertions(+)
> >>>  create mode 100644 lib/librte_pmd_enic/Makefile
> >>>
> >>> diff --git a/lib/librte_pmd_enic/Makefile
> >>>b/lib/librte_pmd_enic/Makefile
> >>> new file mode 100644
> >>> index 0000000..7605a8f
> >>> --- /dev/null
> >>> +++ b/lib/librte_pmd_enic/Makefile
> >>> @@ -0,0 +1,66 @@
> >>> +#   BSD LICENSE
> >>> +#
> >>> +#   Copyright(c) 2010-2013 Intel Corporation. All rights reserved.
> >>> +#   All rights reserved.
> >>> +#
> >>> +#   Redistribution and use in source and binary forms, with or without
> >>> +#   modification, are permitted provided that the following conditions
> >>> +#   are met:
> >>> +#
> >>> +#     * Redistributions of source code must retain the above copyright
> >>> +#       notice, this list of conditions and the following disclaimer.
> >>> +#     * Redistributions in binary form must reproduce the above
> >>>copyright
> >>> +#       notice, this list of conditions and the following disclaimer
> >>>in
> >>> +#       the documentation and/or other materials provided with the
> >>> +#       distribution.
> >>> +#     * Neither the name of Intel Corporation nor the names of its
> >>> +#       contributors may be used to endorse or promote products
> >>>derived
> >>> +#       from this software without specific prior written permission.
> >>> +#
> >>> +#   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
> >>>CONTRIBUTORS
> >>> +#   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> >>> +#   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
> >>>FOR
> >>> +#   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
> >>>COPYRIGHT
> >>> +#   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
> >>>INCIDENTAL,
> >>> +#   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> >>> +#   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
> >>>USE,
> >>> +#   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
> >>>ANY
> >>> +#   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR
> >>>TORT
> >>> +#   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
> >>>USE
> >>> +#   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
> >>>DAMAGE.
> >>> +#
> >>> +
> >>> +include $(RTE_SDK)/mk/rte.vars.mk
> >>> +
> >>> +#
> >>> +# library name
> >>> +#
> >>> +LIB = librte_pmd_enic.a
> >>> +
> >>> +CFLAGS += -I$(RTE_SDK)/lib/librte_hash/
> >>> +CFLAGS += -O3 -Wno-deprecated
> >>> +
> >>> +VPATH += $(RTE_SDK)/lib/librte_pmd_enic/src
> >>> +
> >>> +#
> >>> +# all source are stored in SRCS-y
> >>> +#
> >>> +SRCS-$(CONFIG_RTE_LIBRTE_ENIC_PMD) += enic_main.c
> >>> +SRCS-$(CONFIG_RTE_LIBRTE_ENIC_PMD) += enic_clsf.c
> >>> +SRCS-$(CONFIG_RTE_LIBRTE_ENIC_PMD) += vnic_cq.c
> >>> +SRCS-$(CONFIG_RTE_LIBRTE_ENIC_PMD) += vnic_wq.c
> >>> +SRCS-$(CONFIG_RTE_LIBRTE_ENIC_PMD) += vnic_dev.c
> >>> +SRCS-$(CONFIG_RTE_LIBRTE_ENIC_PMD) += vnic_intr.c
> >>> +SRCS-$(CONFIG_RTE_LIBRTE_ENIC_PMD) += vnic_rq.c
> >>> +SRCS-$(CONFIG_RTE_LIBRTE_ENIC_PMD) += enic_etherdev.c
> >>> +SRCS-$(CONFIG_RTE_LIBRTE_ENIC_PMD) += enic_res.c
> >>> +SRCS-$(CONFIG_RTE_LIBRTE_ENIC_PMD) += vnic_rss.c
> >>> +
> >>> +
> >>> +# this lib depends upon:
> >>> +DEPDIRS-$(CONFIG_RTE_LIBRTE_ENIC_PMD) += lib/librte_eal
> >>>lib/librte_ether
> >>> +DEPDIRS-$(CONFIG_RTE_LIBRTE_ENIC_PMD) += lib/librte_mempool
> >>>lib/librte_mbuf
> >>> +DEPDIRS-$(CONFIG_RTE_LIBRTE_ENIC_PMD) += lib/librte_net
> >>>lib/librte_malloc
> >>> +
> >>> +include $(RTE_SDK)/mk/rte.lib.mk
> >>> +
> >>> --
> >>> 1.9.1
> >>>
> >>>
> >>
> >>Make this the last patch in your series, and merge it with the chunk from
> >>the
> >>last patch that adds the enic directory to the lib/Makefile, so that a
> >>bisect
> >>will build between these commits.
> >>
> >>Neil
> >>
> >
>
>