On 12/12/2017 7:20 PM, Anoob Joseph wrote:
Hi Akhil, Radu
On 12/12/2017 02:25 PM, Akhil Goyal wrote:
Hi Anoob,
On 12/11/2017 12:51 PM, Anoob wrote:
Hi Akhil,
Can you confirm if you are fine with the approach explained inline.
Thanks,
Anoob
On 12/06/2017 03:13 PM, Radu Nicolau wrote:
Hi,
On 12/6/2017 7:30 AM, Anoob wrote:
Hi Akhil, Radu,
Please see inline.
Thanks,
Anoob
On 11/24/2017 05:33 PM, Akhil Goyal wrote:
On 11/24/2017 5:29 PM, Radu Nicolau wrote:
On 11/24/2017 11:34 AM, Akhil Goyal wrote:
Hi Radu,
On 11/24/2017 4:47 PM, Radu Nicolau wrote:
On 11/24/2017 10:55 AM, Akhil Goyal wrote:
On 11/24/2017 3:09 PM, Radu Nicolau wrote:
Hi,
Comment inline
On 11/24/2017 8:50 AM, Akhil Goyal wrote:
Hi Anoob, Radu,
On 11/23/2017 4:49 PM, 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, application could
get metadata
from the packet which could be used to identify the
security parameters
with which the packet was processed.
Signed-off-by: Anoob Joseph <anoob.jos...@caviumnetworks.com>
---
v3:
* Replaced 64 bit metadata in conf with (void *)userdata
* The API(rte_security_get_pkt_metadata) would return void
* instead of
uint64_t
v2:
* Replaced get_session and get_cookie APIs with
get_pkt_metadata API
lib/librte_security/rte_security.c | 13 +++++++++++++
lib/librte_security/rte_security.h | 19 +++++++++++++++++++
lib/librte_security/rte_security_driver.h | 16
++++++++++++++++
3 files changed, 48 insertions(+)
diff --git a/lib/librte_security/rte_security.c
b/lib/librte_security/rte_security.c
index 1227fca..a1d78b6 100644
--- a/lib/librte_security/rte_security.c
+++ b/lib/librte_security/rte_security.c
@@ -108,6 +108,19 @@ rte_security_set_pkt_metadata(struct
rte_security_ctx *instance,
sess, m, params);
}
+void *
+rte_security_get_pkt_metadata(struct rte_security_ctx
*instance,
+ struct rte_mbuf *pkt)
Can we rename pkt with m. Just to make it consistent with
the set API.
+{
+ void *md = NULL;
+
+ RTE_FUNC_PTR_OR_ERR_RET(*instance->ops->get_pkt_metadata,
NULL);
+ if (instance->ops->get_pkt_metadata(instance->device,
pkt, &md))
+ return NULL;
+
+ return md;
+}
Pkt metadata should be set by user i.e. the application, and
the driver need not be aware of the format and the values of
the metadata.
So setting the metadata in the driver and getting it back
from the driver does not look a good idea.
Is it possible, that the application define the metadata on
its own and set it in the library itself without the call to
the driver ops.
I'm not sure I understand here; even in our case (ixgbe) the
driver sets the metadata and it is aware of the format - that
is the whole idea. This is why we added the set_metadata API,
to allow the driver to inject extra information into the
mbuf, information that is driver specific and derived from
the security session, so it makes sense to also have a
symmetric get_metadata.
Private data is the one that follows those rules, i.e.
application specific and driver transparent.
As per my understanding of the user metadata, it should be in
control of the application, and the application shall know the
format of that. Setting in driver will disallow this.
Please let me know if my understanding is incorrect.
If at all, some information is needed to be set on the basis
of driver, then application can get that information from the
driver and then set it in the packet metadata in its own
way/format.
The rte_security_set_pkt_metadata() doc defines the metadata as
"device-specific defined metadata" and also takes a device
specific params pointer, so the symmetric function is to be
expected to work in the same way, i.e. return device specific
metadata associated with the security session and instance and
mbuf. How is this metadata stored is not specified in the
security API, so the PMD implementation have the flexibility.
Is rte_security_get_pkt_metadata() expected to return a "device
specific" pointer? If that's the case, we would need another call
(something like, rte_security_get_userdata()) to get back the
userdata, right? Or is it fine, if the application assumes it will
get userdata (the one passed in conf while creating security
session) with rte_security_get_pkt_metadata()?
Yes, this will be my assumption, a "device specific" pointer
(similar to the "void *params" parameter of the
rte_security_set_pkt_metadata function), which will contain an
arbitrary defined structure that will be decoded by calling a PMD
defined function.
But I think Akhil has a different view on this.
I am ok with the approach, if we are adding this as a limitation of
using udata in the documentation for inline cases.
The ideal approach should be such that driver should not be knowing
the content of the udata. But, if we cannot do away with it, we can
mention it in the documentation.
Will document the limitation of udata64's usage. Since we are
documenting that udata64 will have some device defined metadata, do we
need another API call for getting the "metadata"
(rte_security_get_pkt_metadata). The driver can set this metadata in
udata64 field, and in ingress, the userdata could be obtained with a
single API call (rte_security_get_userdata(*instance, udata64)).
The application will be aware that the udata64 in the mbuf will be the
metadata from the receive side, and userdata can be retrieved only with
that. If application need to use the udata64 field, it should save this
rx metadata. Userdata can be obtained anytime by passing this metadata
to the driver.
To summarize,
1) udata64 of the ingress traffic will be device-specific metadata
(documentation change)
2) Pass this field to rte_security_get_userdata() to get back the
application registered pointer.
Is this fine?
It looks ok as of now.
Yes it was defined that way and I did not noticed this one at
the time of it's implementation.
Here, my point is that the application may be using mbuf udata
for it's own functionality, it should not be modified in the
driver.
However, if we need to do this, then we may need to clarify in
the documentation that for security, udata shall be set with the
rte_security_set_pkt_metadata() and not otherwise.
Indeed, we should update the doc stating that the set_metadata
may change the mbuf userdata field so the application should use
only private data if needed.
Agreed, but it is dependent on which driver/mode(inline or
lookaside), it will be used.
Lookaside may not need this API as of now. Other implementations
may also don't require. So this shall be documented that way.
-Akhil
