DPDK-dev Archive on lore.kernel.org
 help / color / Atom feed
From: Jerin Jacob Kollanukkaran <jerinj@marvell.com>
To: "Burakov, Anatoly" <anatoly.burakov@intel.com>,
	Ferruh Yigit <ferruh.yigit@intel.com>,
	Vamsi Krishna Attunuru <vattunuru@marvell.com>,
	"dev@dpdk.org" <dev@dpdk.org>
Cc: "olivier.matz@6wind.com" <olivier.matz@6wind.com>,
	"arybchenko@solarflare.com" <arybchenko@solarflare.com>
Subject: Re: [dpdk-dev] [PATCH v6 0/4] add IOVA = VA support in KNI
Date: Fri, 12 Jul 2019 11:37:47 +0000
Message-ID: <BYAPR18MB2424164D8F69555486FDB55FC8F20@BYAPR18MB2424.namprd18.prod.outlook.com> (raw)

> -----Original Message-----
> From: Burakov, Anatoly <anatoly.burakov@intel.com>
> Sent: Friday, July 12, 2019 4:19 PM
> To: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; Ferruh Yigit
> <ferruh.yigit@intel.com>; Vamsi Krishna Attunuru
> <vattunuru@marvell.com>; dev@dpdk.org
> Cc: olivier.matz@6wind.com; arybchenko@solarflare.com
> Subject: [EXT] Re: [dpdk-dev] [PATCH v6 0/4] add IOVA = VA support in KNI
> On 12-Jul-19 11:26 AM, Jerin Jacob Kollanukkaran wrote:
> >>>> What do you think?
> >>>
> >>> IMO, If possible we can avoid extra indirection of new config. In
> >>> worst case We can add it. How about following to not have new config
> >>>
> >>> 1) Make MEMPOOL_F_NO_PAGE_BOUND  as default
> >>> http://patches.dpdk.org/patch/55277/
> >>> There is absolutely zero overhead of this flag considering the huge
> >>> page size are minimum 2MB. Typically 512MB or 1GB.
> >>> Any one has any objection?
> >>
> >> Pretty much zero overhead in hugepage case, not so in non-hugepage
> case.
> >> It's rare, but since we support it, we have to account for it.
> >
> > That is a fair concern.
> > How about enable the flag in mempool ONLY when
> rte_eal_has_hugepages()
> > In the common layer?
> 
> Perhaps it's better to check page size of the underlying memory, because 4K
> pages are not necessarily no-huge mode - they could also be external
> memory. That's going to be a bit hard because there may not be a way to
> know which memory we're allocating from in advance, aside from simple
> checks like `(rte_eal_has_hugepages() ||
> rte_malloc_heap_socket_is_external(socket_id))` - but maybe those would
> be sufficient.

Yes.


> 
> >
> >> (also, i don't really like the name NO_PAGE_BOUND since in memzone
> >> API there's a "bounded memzone" allocation API, and this flag's name
> >> reads like objects would not be bounded by page size, not that they
> >> won't cross page
> >> boundary)
> >
> > No strong opinion for the name. What name you suggest?
> 
> How about something like MEMPOOL_F_NO_PAGE_SPLIT?

Looks good to me.

In summary, Change wrt existing patch"
- Change NO_PAGE_BOUND to MEMPOOL_F_NO_PAGE_SPLIT
- Set this flag in  rte_pktmbuf_pool_create() when rte_eal_has_hugepages() ||
 rte_malloc_heap_socket_is_external(socket_id))

Olivier, Any objection?
Ref: http://patches.dpdk.org/patch/55277/

> 
> >
> >>
> >>>
> >>> 2) Introduce rte_kni_mempool_create() API in kni lib to abstract the
> >>> Mempool requirement for KNI. This will enable portable KNI
> applications.
> >>
> >> This means that using KNI is not a drop-in replacement for any other
> >> PMD. If maintainers of KNI are OK with this then sure :)
> >
> > The PMD  don’t have any dependency on NO_PAGE_BOUND flag. Right?
> > If KNI app is using rte_kni_mempool_create() to create the mempool, In
> > what case do you see problem with specific PMD?
> 
> I'm not saying the PMD's have a dependency on the flag, i'm saying that the
> same code cannot be used with and without KNI because you need to call a
> separate API for mempool creation if you want to use it with KNI.

Yes. Need to call the introduced API from 19.08. If we not choose above(first) approach.
It can be documented in "API changes" in release notes. I prefer to have the first 
solution if there is no downside.


> For KNI, the underlying memory must abide by certain constraints that are
> not there for other PMD's, so either you fix all memory to these constraints,
> or you lose the ability to reuse the code with other PMD's as is.
> 
> That is, unless i'm grossly misunderstanding what you're suggesting here :)
> 
> >
> >>
> >> --
> >> Thanks,
> >> Anatoly
> 
> 
> --
> Thanks,
> Anatoly

             reply index

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-12 11:37 Jerin Jacob Kollanukkaran [this message]
2019-07-12 12:09 ` Burakov, Anatoly
2019-07-12 12:28   ` [dpdk-dev] [EXT] " Jerin Jacob Kollanukkaran
2019-07-15  4:54     ` Jerin Jacob Kollanukkaran
2019-07-15  9:38       ` Burakov, Anatoly
2019-07-16  8:46         ` Olivier Matz
2019-07-16  9:40           ` Vamsi Krishna Attunuru
2019-07-16  9:55             ` Olivier Matz
2019-07-16 10:07               ` Vamsi Krishna Attunuru
  -- strict thread matches above, loose matches on Subject: below --
2019-07-12 10:26 [dpdk-dev] " Jerin Jacob Kollanukkaran
2019-07-12 10:48 ` Burakov, Anatoly
2019-07-12  9:17 Jerin Jacob Kollanukkaran
2019-07-12  9:58 ` Burakov, Anatoly
2019-04-22  6:15 [dpdk-dev] [PATCH v5] kni: add IOVA va support for kni kirankumark
2019-06-25  3:56 ` [dpdk-dev] [PATCH v6 0/4] add IOVA = VA support in KNI vattunuru
2019-06-25 10:00   ` Burakov, Anatoly
2019-06-25 11:15     ` Jerin Jacob Kollanukkaran
2019-06-25 11:30       ` Burakov, Anatoly
2019-06-25 13:38         ` Burakov, Anatoly
2019-06-27  9:34           ` Jerin Jacob Kollanukkaran
2019-07-01 13:51             ` Vamsi Krishna Attunuru
2019-07-04  6:42               ` Vamsi Krishna Attunuru
2019-07-04  9:48                 ` Jerin Jacob Kollanukkaran
2019-07-11 16:21                   ` Ferruh Yigit

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=BYAPR18MB2424164D8F69555486FDB55FC8F20@BYAPR18MB2424.namprd18.prod.outlook.com \
    --to=jerinj@marvell.com \
    --cc=anatoly.burakov@intel.com \
    --cc=arybchenko@solarflare.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=olivier.matz@6wind.com \
    --cc=vattunuru@marvell.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

DPDK-dev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/dpdk-dev/0 dpdk-dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dpdk-dev dpdk-dev/ https://lore.kernel.org/dpdk-dev \
		dev@dpdk.org
	public-inbox-index dpdk-dev

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git