DPDK patches and discussions
 help / color / mirror / Atom feed
From: Sagi Grimberg <sagi@grimberg.me>
To: "Shahaf Shuler" <shahafs@mellanox.com>,
	"Nélio Laranjeiro" <nelio.laranjeiro@6wind.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>, Yongseok Koh <yskoh@mellanox.com>,
	Roy Shterman <roys@lightbitslabs.com>,
	Alexander Solganik <sashas@lightbitslabs.com>,
	Leon Romanovsky <leonro@mellanox.com>
Subject: Re: [dpdk-dev] Question on mlx5 PMD txq memory registration
Date: Thu, 20 Jul 2017 19:22:09 +0300	[thread overview]
Message-ID: <67476089-ba6e-905c-0fdd-3a1551de97d4@grimberg.me> (raw)
In-Reply-To: <VI1PR05MB314963FFD3B58C1E9559C68BC3A70@VI1PR05MB3149.eurprd05.prod.outlook.com>


>> As I said, there are primitives which are designed to handle frequent reads
>> and rare mutations.
> 
> Even with such primitives, rarely lock is more than never lock.

You do realize that the cache mutation involves ibv_dereg_mr() right?
Any locking scheme for mutation is negligible compared to that, and
rcu_read_lock is practically free.

>>> It is one possibility discussed also with Mellanox guys, the point is
>>> this breaks the security point of view which is also an important stuff.
>>
>> What security aspect? The entire dpdk model builds on top of physical
>> addresses awareness running under root permissions.
>> I'm not saying
>> exposing it to the application nor granting remote permissions to the physical
>> space.
>>
>> mlx5_pmd is a network driver, and as a driver, it should allowed to use the
>> device dma lkey as it sees fit. I honestly think its pretty much mandatory
>> considering the work-arounds mlx5_pmd tries to do (which we agreed are
>> broken).
> 
> True, There are many PMDs which can work only with physical memory.
> However Mellanox NICs have the option to work with virtual one thus provide more security.

I don't understand the security argument. Its completely private to the
driver. anything under librte is equivalent to an OS wrt networking, so
I fail to see what is the security feature your talking about.

> The fact running under root doesn't mean you have privileges to access every physical page on the server (even if you try very hard to be aware).

But dpdk core mbufs structs are built this way.

> The issue here, AFAIU, is performance.
> We are now looking into ways to provide the same performance as if it was only a single lkey, while preserving the security feature.

Hmm, What exactly do you have in mind?

I'm hoping that you are not referring to ODP. If you are, I think
that latency unpredictability would be a huge non-starter, page-faults
are way too expensive for dpdk users.

Again, personally, I don't think that using virtual memory registration
are very useful for a network driver, It's a driver, not an application.

>>> If this is added in the future it will certainly be as an option, this
>>> way both will be possible, the application could then choose about
>>> security vs performance.
>>
>> Why should the application even be aware of that? Does any other driver
>> expose the user how it maps pkt mbufs to the NIC? Just like the MR
>> handling, its 100% internal to mlx5 and no reason why the user should ever
>> be exposed to any of these details.
> 
> Other option is the reserved lkey as you suggested, but it will lose the security guarantees.

OK, obviously I'm missing something. Can you articulate what do you mean
by "security guarantees"?

> Like every performance optimization it should be the application decision.

I tend to disagree with you on this. The application must never be aware
of the nitty details of the underlying driver implementation. In fact,
propagating this information to the application sounds like a layering
violation.

> In fact, there are some discussions on the ML of exposing the option to use va instead of pa. [1]

My understanding is that the correct proposal was to hide this
knowledge from the user. Also the use-case is not security (which
I'm not even sure what it means yet).

>>> This is also a in progress in PMD part, it should be part of the next
>>> DPDK release.
>>
>> That is *very* good to hear! Can you guys share a branch? I'm willing to take
>> it for testing.
> 
> The branch is still pre-mature. It may be good enough for external testing in about two weeks.
> Contact me directly and I will provide it to you.

Awesome!

Thanks.

  reply	other threads:[~2017-07-20 16:22 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-07-17 13:29 Sagi Grimberg
2017-07-17 21:02 ` Nélio Laranjeiro
2017-07-19  6:21   ` Sagi Grimberg
2017-07-20 13:55     ` Nélio Laranjeiro
2017-07-20 14:06       ` Sagi Grimberg
2017-07-20 15:20         ` Shahaf Shuler
2017-07-20 16:22           ` Sagi Grimberg [this message]
2017-07-23  8:17             ` Shahaf Shuler
2017-07-23  9:03               ` Sagi Grimberg
2017-07-24 13:44                 ` Bruce Richardson
2017-07-27 10:48                   ` Sagi Grimberg

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=67476089-ba6e-905c-0fdd-3a1551de97d4@grimberg.me \
    --to=sagi@grimberg.me \
    --cc=dev@dpdk.org \
    --cc=leonro@mellanox.com \
    --cc=nelio.laranjeiro@6wind.com \
    --cc=roys@lightbitslabs.com \
    --cc=sashas@lightbitslabs.com \
    --cc=shahafs@mellanox.com \
    --cc=yskoh@mellanox.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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).