From: Radu Nicolau <radu.nicolau@intel.com>
To: Anoob Joseph <anoob.joseph@caviumnetworks.com>,
Akhil Goyal <akhil.goyal@nxp.com>,
Declan Doherty <declan.doherty@intel.com>,
Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
Cc: Narayana Prasad <narayanaprasad.athreya@caviumnetworks.com>,
Jerin Jacob <jerin.jacob@caviumnetworks.com>,
dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH 1/2] lib/security: add support for saving app cookie
Date: Tue, 21 Nov 2017 10:15:32 +0000 [thread overview]
Message-ID: <a37c277e-53aa-f24e-1fe5-0091f89da413@intel.com> (raw)
In-Reply-To: <b661ad3c-b334-4b82-30d9-e769f14892f6@caviumnetworks.com>
On 11/20/2017 7:09 PM, Anoob Joseph wrote:
>
> Hi
>
> See inline.
>
> On 20-11-2017 23:19, Radu Nicolau wrote:
>> Hi
>>
>>
>> On 11/20/2017 3:32 PM, Anoob wrote:
>>> Hi,
>>>
>>> Having something like "get_pkt_metadata" should be fine for inline
>>> protocol usage. But we will still need a "get cookie" call to get
>>> the cookie, as the application would need something it can interpret.
>> Why can't you have a get_pkt_metadata that returns the "cookie" -
>> which I would call it userdata or similar? What I'm trying to say is
>> that we should try to keep it as generic as possible. For example, I
>> wouldn't assume that the cookie is stored in pkt->udata64 in the
>> application.
> I agree to your suggestion. The only problem is in the asymmetry of
> how we would set the "cookie" (or userdata) and get it back. Right now
> we are passing this as a member in conf. Do you have any thoughts on
> that? For a more meaningful approach, we could pass this as another
> argument in create_session API. I was thinking of a scenario of
> supporting more items. So added it in the structure.
>
> Naming is open for suggestions. I'll use userdata instead.
I think keeping it in the conf is best, but it can be either way.
>>> And, even though it seems both are symmetric operations(get & set
>>> pkt metadata), there are some minor differences in what they would
>>> do. Set metadata would be setting metadata(udata64 member) in mbuf,
>>> while get metadata is not exactly returning metadata. We use the
>>> actual metadata to get something else(security session in the
>>> proposed patch). That was the primary motive for adding
>>> "session_get" API.
>> What they do exactly is left to the PMD implementation. From the
>> user's perspective, it does not matter.
>> There is no requirement that set pkt metadata will set the udata64
>> member.
> May be I misunderstood the terminology. "udata64" in mbuf was
> documented as
> |RTE_STD_C11 union { void *userdata; /**< Can be used for external
> metadata */ uint64_t udata64; /**< Allow 8-byte userdata on 32-bit */ };|
>
> I thought the metadata in set_pkt_metadata was coming from this. And
> we were setting this member itself in ixgbe driver.
We're using it in the ixgbe because it is the most obvious choice when
there is only a small data set to be passed (an table index in this
case) but it was intended to allow the driver to implement any behavior
necessary.
>
> But yes, it makes sense not to expose it that way. The API can take
> mbuf pointer and then, the PMD could dictate how it had set the
> metadata in rx path.
>
>>>
>>> Thanks,
>>> Anoob
>>>
>>> On 11/20/2017 05:42 PM, Radu Nicolau wrote:
>>>> Hi,
>>>>
>>>>
>>>> Why not have something similar to rte_security_set_pkt_metadata,
>>>> for example:
>>>>
>>>> void *
>>>> rte_security_get_pkt_metadata(struct rte_security_ctx *instance,
>>>> struct rte_mbuf *mb);
>>>>
>>>> and keep internally in the PMD all the required references. The
>>>> returned value will be device-specific, so it's flexible enough to
>>>> include anything required (just as void *params is in the
>>>> set_pkt_metadata).
>>>>
>>>> I think it will make a cleaner and more consistent implementation.
>>>>
>>>>
>>>> Regards,
>>>>
>>>> Radu
>>>>
>>>>
>>>>
>>>> On 11/20/2017 10:31 AM, Anoob Joseph wrote:
>>>>> In case of inline protocol processed ingress traffic, the packet
>>>>> may not
>>>>> have enough information to determine the security parameters with
>>>>> which
>>>>> the packet was processed. In such cases, the application could
>>>>> register
>>>>> a cookie, which will be saved in the the security session.
>>>>>
>>>>> As the ingress packets are received in the application, it will have
>>>>> some metadata set in the mbuf. Application can pass this metadata to
>>>>> "rte_security_session_get" API to retrieve the security session. Once
>>>>> the security session is determined, another driver call with the
>>>>> security session will give the application the cookie it had
>>>>> registered.
>>>>>
>>>>> The cookie will be registered while creating the security session.
>>>>> Without the cookie, the selector check (SP-SA check) for the incoming
>>>>> IPsec traffic won't be possible in the application.
>>>>>
>>>>> Application can choose what it should register as the cookie. It can
>>>>> register SPI or a pointer to SA.
>>>>>
>>>>> Signed-off-by: Anoob Joseph <anoob.joseph@caviumnetworks.com>
>>>>> ---
>>>>> lib/librte_security/rte_security.c | 26
>>>>> +++++++++++++++++++++++
>>>>> lib/librte_security/rte_security.h | 30
>>>>> +++++++++++++++++++++++++++
>>>>> lib/librte_security/rte_security_driver.h | 34
>>>>> +++++++++++++++++++++++++++++++
>>>>> 3 files changed, 90 insertions(+)
>>>>> <snip>
>>>
>>
> I'll rework the patch to include your suggestions. I'll send a v2
> after doing this.
>
> Thanks for the feedback,
> Anoob
>
next prev parent reply other threads:[~2017-11-21 10:15 UTC|newest]
Thread overview: 67+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-11-20 10:31 [dpdk-dev] [PATCH 0/2] add inline protocol support Anoob Joseph
2017-11-20 10:31 ` [dpdk-dev] [PATCH 1/2] lib/security: add support for saving app cookie Anoob Joseph
2017-11-20 12:12 ` Radu Nicolau
2017-11-20 15:32 ` Anoob
2017-11-20 17:49 ` Radu Nicolau
2017-11-20 19:09 ` Anoob Joseph
2017-11-21 10:15 ` Radu Nicolau [this message]
2017-11-20 10:31 ` [dpdk-dev] [PATCH 2/2] examples/ipsec-secgw: add support for inline protocol Anoob Joseph
2017-11-22 6:55 ` [dpdk-dev] [PATCH v2 0/2] add inline protocol support Anoob Joseph
2017-11-22 6:55 ` [dpdk-dev] [PATCH v2 1/2] lib/security: add support for get metadata Anoob Joseph
2017-11-22 11:29 ` Radu Nicolau
2017-11-22 11:52 ` Anoob
2017-11-22 12:12 ` Radu Nicolau
2017-11-22 13:27 ` Neil Horman
2017-11-22 14:13 ` Anoob
2017-11-27 13:55 ` Neil Horman
2017-11-22 6:55 ` [dpdk-dev] [PATCH v2 2/2] examples/ipsec-secgw: add support for inline protocol Anoob Joseph
2017-11-22 12:21 ` [dpdk-dev] [PATCH v2 0/2] add inline protocol support Nelio Laranjeiro
2017-11-22 12:55 ` Anoob
2017-11-22 13:05 ` Nelio Laranjeiro
2017-11-22 13:38 ` Anoob
2017-11-22 13:53 ` Anoob
2017-11-22 15:13 ` Anoob
2017-11-22 15:25 ` Nelio Laranjeiro
2017-11-23 11:19 ` [dpdk-dev] [PATCH v3 " Anoob Joseph
2017-11-23 11:19 ` [dpdk-dev] [PATCH v3 1/2] lib/security: add support for get metadata Anoob Joseph
2017-11-24 8:50 ` Akhil Goyal
2017-11-24 9:39 ` Radu Nicolau
2017-11-24 10:55 ` Akhil Goyal
2017-11-24 11:17 ` Radu Nicolau
2017-11-24 11:34 ` Akhil Goyal
2017-11-24 11:59 ` Radu Nicolau
2017-11-24 12:03 ` Akhil Goyal
2017-12-06 7:30 ` Anoob
2017-12-06 9:43 ` Radu Nicolau
2017-12-11 7:21 ` Anoob
2017-12-12 8:55 ` Akhil Goyal
2017-12-12 13:50 ` Anoob Joseph
2017-12-13 14:38 ` Akhil Goyal
2017-11-24 12:22 ` Anoob
2017-11-29 5:43 ` Anoob
2017-12-04 9:28 ` Akhil Goyal
2017-12-04 10:16 ` Anoob
2017-11-23 11:19 ` [dpdk-dev] [PATCH v3 2/2] examples/ipsec-secgw: add support for inline protocol Anoob Joseph
2017-12-11 11:02 ` Radu Nicolau
2017-12-15 8:30 ` [dpdk-dev] [PATCH v4 0/2] add inline protocol support Anoob Joseph
2017-12-15 8:30 ` [dpdk-dev] [PATCH v4 1/2] lib/security: add support for get userdata Anoob Joseph
2017-12-15 8:30 ` [dpdk-dev] [PATCH v4 2/2] examples/ipsec-secgw: add support for inline protocol Anoob Joseph
2017-12-15 8:43 ` [dpdk-dev] [PATCH v5 0/2] add inline protocol support Anoob Joseph
2017-12-15 8:43 ` [dpdk-dev] [PATCH v5 1/2] lib/security: add support for get userdata Anoob Joseph
2017-12-15 10:01 ` Akhil Goyal
2017-12-15 10:53 ` Anoob Joseph
2017-12-15 10:58 ` Akhil Goyal
2017-12-15 8:43 ` [dpdk-dev] [PATCH v5 2/2] examples/ipsec-secgw: add support for inline protocol Anoob Joseph
2017-12-15 9:39 ` Nelio Laranjeiro
2017-12-15 11:03 ` Anoob Joseph
2017-12-15 13:35 ` Nelio Laranjeiro
2017-12-15 10:04 ` Akhil Goyal
2017-12-15 11:16 ` Anoob Joseph
2017-12-18 7:15 ` [dpdk-dev] [PATCH v6 0/2] add inline protocol support Anoob Joseph
2017-12-18 7:15 ` [dpdk-dev] [PATCH v6 1/2] lib/security: add support for get userdata Anoob Joseph
2017-12-18 7:34 ` Akhil Goyal
2017-12-18 7:15 ` [dpdk-dev] [PATCH v6 2/2] examples/ipsec-secgw: add support for inline protocol Anoob Joseph
2018-01-08 16:10 ` De Lara Guarch, Pablo
2018-01-09 9:12 ` Akhil Goyal
2018-01-16 11:00 ` Nicolau, Radu
2018-01-09 16:05 ` [dpdk-dev] [PATCH v6 0/2] add inline protocol support De Lara Guarch, Pablo
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=a37c277e-53aa-f24e-1fe5-0091f89da413@intel.com \
--to=radu.nicolau@intel.com \
--cc=akhil.goyal@nxp.com \
--cc=anoob.joseph@caviumnetworks.com \
--cc=declan.doherty@intel.com \
--cc=dev@dpdk.org \
--cc=jerin.jacob@caviumnetworks.com \
--cc=narayanaprasad.athreya@caviumnetworks.com \
--cc=sergio.gonzalez.monroy@intel.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).