[dpdk-dev,v2,1/2] lib/security: add support for get metadata

Message ID 1511333716-11955-2-git-send-email-anoob.joseph@caviumnetworks.com (mailing list archive)
State Superseded, archived
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Anoob Joseph Nov. 22, 2017, 6:55 a.m. UTC
  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. For such cases, application could register a
64 bit metadata in security session, which could be retrieved from the
packet using "rte_security_get_pkt_metadata" API. Application can use
this metadata to identify the parameters it need.

Application can choose what it should register as the metadata. It can
register SPI or a pointer to SA.

Signed-off-by: Anoob Joseph <anoob.joseph@caviumnetworks.com>
---
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(+)
  

Comments

Radu Nicolau Nov. 22, 2017, 11:29 a.m. UTC | #1
On 11/22/2017 6:55 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. For such cases, application could register a
> 64 bit metadata in security session, which could be retrieved from the
> packet using "rte_security_get_pkt_metadata" API. Application can use
> this metadata to identify the parameters it need.
>
> Application can choose what it should register as the metadata. It can
> register SPI or a pointer to SA.
>
> Signed-off-by: Anoob Joseph <anoob.joseph@caviumnetworks.com>
> ---
> 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..804f11f 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);
>   }
>   
> +uint64_t
> +rte_security_get_pkt_metadata(struct rte_security_ctx *instance,
> +			      struct rte_mbuf *pkt)
> +{
> +	uint64_t mdata = 0;
> +
> +	RTE_FUNC_PTR_OR_ERR_RET(*instance->ops->get_pkt_metadata, 0);
> +	if (instance->ops->get_pkt_metadata(instance->device, pkt, &mdata))
> +		return 0;
> +
> +	return mdata;
> +}
> +
Can you change the returned type to void *?
>   const struct rte_security_capability *
>   rte_security_capabilities_get(struct rte_security_ctx *instance)
>   {
> diff --git a/lib/librte_security/rte_security.h b/lib/librte_security/rte_security.h
> index 653929b..aa3a471 100644
> --- a/lib/librte_security/rte_security.h
> +++ b/lib/librte_security/rte_security.h
> @@ -274,6 +274,8 @@ struct rte_security_session_conf {
>   	/**< Configuration parameters for security session */
>   	struct rte_crypto_sym_xform *crypto_xform;
>   	/**< Security Session Crypto Transformations */
> +	uint64_t metadata;
> +	/**< Metadata registered by application */
>   };
Can you rename it to userdata?
>   
>   struct rte_security_session {
> @@ -346,6 +348,23 @@ rte_security_set_pkt_metadata(struct rte_security_ctx *instance,
>   			      struct rte_mbuf *mb, void *params);
>   
>   /**
> + * Get metadata from the packet. This is an application registered 64 bit
> + * value, associated with the security session which processed the packet.
> + *
> + * This is valid only for inline processed ingress packets.
> + *
> + * @param   instance	security instance
> + * @param   pkt		packet mbuf
> + *
> + * @return
> + *  - On success, metadata
> + *  - On failure, 0
> + */
> +uint64_t
> +rte_security_get_pkt_metadata(struct rte_security_ctx *instance,
> +			      struct rte_mbuf *pkt);
> +
> +/**
>    * Attach a session to a symmetric crypto operation
>    *
>    * @param	sym_op	crypto operation
> diff --git a/lib/librte_security/rte_security_driver.h b/lib/librte_security/rte_security_driver.h
> index 997fbe7..da0ebf4 100644
> --- a/lib/librte_security/rte_security_driver.h
> +++ b/lib/librte_security/rte_security_driver.h
> @@ -122,6 +122,20 @@ typedef int (*security_set_pkt_metadata_t)(void *device,
>   		void *params);
>   
>   /**
> + * Get application interpretable metadata from the packet.
> + *
> + * @param	device		Crypto/eth device pointer
> + * @param	pkt		Packet mbuf
> + * @param	mt		Pointer to receive metadata
> + *
> + * @return
> + *  - Returns 0 if metadata is retrieved successfully.
> + *  - Returns -ve value for errors.
> + */
> +typedef int (*security_get_pkt_metadata_t)(void *device,
> +		struct rte_mbuf *pkt, uint64_t *mt);
> +
> +/**
>    * Get security capabilities of the device.
>    *
>    * @param	device		crypto/eth device pointer
> @@ -145,6 +159,8 @@ struct rte_security_ops {
>   	/**< Clear a security sessions private data. */
>   	security_set_pkt_metadata_t set_pkt_metadata;
>   	/**< Update mbuf metadata. */
> +	security_get_pkt_metadata_t get_pkt_metadata;
> +	/**< Get metadata from packet. */
>   	security_capabilities_get_t capabilities_get;
>   	/**< Get security capabilities. */
>   };
  
Anoob Joseph Nov. 22, 2017, 11:52 a.m. UTC | #2
Hi,

See inline.


Thanks,
Anoob

On 11/22/2017 04:59 PM, Radu Nicolau wrote:
>
>
> On 11/22/2017 6:55 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. For such cases, application could register a
>> 64 bit metadata in security session, which could be retrieved from the
>> packet using "rte_security_get_pkt_metadata" API. Application can use
>> this metadata to identify the parameters it need.
>>
>> Application can choose what it should register as the metadata. It can
>> register SPI or a pointer to SA.
>>
>> Signed-off-by: Anoob Joseph <anoob.joseph@caviumnetworks.com>
>> ---
>> 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..804f11f 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);
>>   }
>>   +uint64_t
>> +rte_security_get_pkt_metadata(struct rte_security_ctx *instance,
>> +                  struct rte_mbuf *pkt)
>> +{
>> +    uint64_t mdata = 0;
>> +
>> + RTE_FUNC_PTR_OR_ERR_RET(*instance->ops->get_pkt_metadata, 0);
>> +    if (instance->ops->get_pkt_metadata(instance->device, pkt, &mdata))
>> +        return 0;
>> +
>> +    return mdata;
>> +}
>> +
> Can you change the returned type to void *?
Will do that.
>>   const struct rte_security_capability *
>>   rte_security_capabilities_get(struct rte_security_ctx *instance)
>>   {
>> diff --git a/lib/librte_security/rte_security.h 
>> b/lib/librte_security/rte_security.h
>> index 653929b..aa3a471 100644
>> --- a/lib/librte_security/rte_security.h
>> +++ b/lib/librte_security/rte_security.h
>> @@ -274,6 +274,8 @@ struct rte_security_session_conf {
>>       /**< Configuration parameters for security session */
>>       struct rte_crypto_sym_xform *crypto_xform;
>>       /**< Security Session Crypto Transformations */
>> +    uint64_t metadata;
>> +    /**< Metadata registered by application */
>>   };
> Can you rename it to userdata?
Will do it. Thought it would be confusing, as this will be returned by 
rte_security_get_pkt_metadata. So get_metadata would give userdata of 
the security session. I can document it that way and proceed, right?
>>     struct rte_security_session {
>> @@ -346,6 +348,23 @@ rte_security_set_pkt_metadata(struct 
>> rte_security_ctx *instance,
>>                     struct rte_mbuf *mb, void *params);
>>     /**
>> + * Get metadata from the packet. This is an application registered 
>> 64 bit
>> + * value, associated with the security session which processed the 
>> packet.
>> + *
>> + * This is valid only for inline processed ingress packets.
>> + *
>> + * @param   instance    security instance
>> + * @param   pkt        packet mbuf
>> + *
>> + * @return
>> + *  - On success, metadata
>> + *  - On failure, 0
>> + */
>> +uint64_t
>> +rte_security_get_pkt_metadata(struct rte_security_ctx *instance,
>> +                  struct rte_mbuf *pkt);
>> +
>> +/**
>>    * Attach a session to a symmetric crypto operation
>>    *
>>    * @param    sym_op    crypto operation
>> diff --git a/lib/librte_security/rte_security_driver.h 
>> b/lib/librte_security/rte_security_driver.h
>> index 997fbe7..da0ebf4 100644
>> --- a/lib/librte_security/rte_security_driver.h
>> +++ b/lib/librte_security/rte_security_driver.h
>> @@ -122,6 +122,20 @@ typedef int (*security_set_pkt_metadata_t)(void 
>> *device,
>>           void *params);
>>     /**
>> + * Get application interpretable metadata from the packet.
>> + *
>> + * @param    device        Crypto/eth device pointer
>> + * @param    pkt        Packet mbuf
>> + * @param    mt        Pointer to receive metadata
>> + *
>> + * @return
>> + *  - Returns 0 if metadata is retrieved successfully.
>> + *  - Returns -ve value for errors.
>> + */
>> +typedef int (*security_get_pkt_metadata_t)(void *device,
>> +        struct rte_mbuf *pkt, uint64_t *mt);
>> +
>> +/**
>>    * Get security capabilities of the device.
>>    *
>>    * @param    device        crypto/eth device pointer
>> @@ -145,6 +159,8 @@ struct rte_security_ops {
>>       /**< Clear a security sessions private data. */
>>       security_set_pkt_metadata_t set_pkt_metadata;
>>       /**< Update mbuf metadata. */
>> +    security_get_pkt_metadata_t get_pkt_metadata;
>> +    /**< Get metadata from packet. */
>>       security_capabilities_get_t capabilities_get;
>>       /**< Get security capabilities. */
>>   };
>
  
Radu Nicolau Nov. 22, 2017, 12:12 p.m. UTC | #3
On 11/22/2017 11:52 AM, Anoob wrote:
> Hi,
>
> See inline.
>
>
> Thanks,
> Anoob
>
> On 11/22/2017 04:59 PM, Radu Nicolau wrote:
>>
>>
>> On 11/22/2017 6:55 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. For such cases, application could register a
>>> 64 bit metadata in security session, which could be retrieved from the
>>> packet using "rte_security_get_pkt_metadata" API. Application can use
>>> this metadata to identify the parameters it need.
>>>
>>> Application can choose what it should register as the metadata. It can
>>> register SPI or a pointer to SA.
>>>
>>> Signed-off-by: Anoob Joseph <anoob.joseph@caviumnetworks.com>
>>> ---
>>> 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..804f11f 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);
>>>   }
>>>   +uint64_t
>>> +rte_security_get_pkt_metadata(struct rte_security_ctx *instance,
>>> +                  struct rte_mbuf *pkt)
>>> +{
>>> +    uint64_t mdata = 0;
>>> +
>>> + RTE_FUNC_PTR_OR_ERR_RET(*instance->ops->get_pkt_metadata, 0);
>>> +    if (instance->ops->get_pkt_metadata(instance->device, pkt, 
>>> &mdata))
>>> +        return 0;
>>> +
>>> +    return mdata;
>>> +}
>>> +
>> Can you change the returned type to void *?
> Will do that.
>>>   const struct rte_security_capability *
>>>   rte_security_capabilities_get(struct rte_security_ctx *instance)
>>>   {
>>> diff --git a/lib/librte_security/rte_security.h 
>>> b/lib/librte_security/rte_security.h
>>> index 653929b..aa3a471 100644
>>> --- a/lib/librte_security/rte_security.h
>>> +++ b/lib/librte_security/rte_security.h
>>> @@ -274,6 +274,8 @@ struct rte_security_session_conf {
>>>       /**< Configuration parameters for security session */
>>>       struct rte_crypto_sym_xform *crypto_xform;
>>>       /**< Security Session Crypto Transformations */
>>> +    uint64_t metadata;
>>> +    /**< Metadata registered by application */
>>>   };
>> Can you rename it to userdata?
> Will do it. Thought it would be confusing, as this will be returned by 
> rte_security_get_pkt_metadata. So get_metadata would give userdata of 
> the security session. I can document it that way and proceed, right?
Also make it a pointer please.
You should document it in the PMD documentation, as this is a PMD 
specific behavior. Your PMD returns just the userdata pointer set in the 
security_conf struct, but other PMDs can return different things.
>>>     struct rte_security_session {
>>> @@ -346,6 +348,23 @@ rte_security_set_pkt_metadata(struct 
>>> rte_security_ctx *instance,
>>>                     struct rte_mbuf *mb, void *params);
>>>     /**
>>> + * Get metadata from the packet. This is an application registered 
>>> 64 bit
>>> + * value, associated with the security session which processed the 
>>> packet.
>>> + *
>>> + * This is valid only for inline processed ingress packets.
>>> + *
>>> + * @param   instance    security instance
>>> + * @param   pkt        packet mbuf
>>> + *
>>> + * @return
>>> + *  - On success, metadata
>>> + *  - On failure, 0
>>> + */
>>> +uint64_t
>>> +rte_security_get_pkt_metadata(struct rte_security_ctx *instance,
>>> +                  struct rte_mbuf *pkt);
>>> +
>>> +/**
>>>    * Attach a session to a symmetric crypto operation
>>>    *
>>>    * @param    sym_op    crypto operation
>>> diff --git a/lib/librte_security/rte_security_driver.h 
>>> b/lib/librte_security/rte_security_driver.h
>>> index 997fbe7..da0ebf4 100644
>>> --- a/lib/librte_security/rte_security_driver.h
>>> +++ b/lib/librte_security/rte_security_driver.h
>>> @@ -122,6 +122,20 @@ typedef int (*security_set_pkt_metadata_t)(void 
>>> *device,
>>>           void *params);
>>>     /**
>>> + * Get application interpretable metadata from the packet.
>>> + *
>>> + * @param    device        Crypto/eth device pointer
>>> + * @param    pkt        Packet mbuf
>>> + * @param    mt        Pointer to receive metadata
>>> + *
>>> + * @return
>>> + *  - Returns 0 if metadata is retrieved successfully.
>>> + *  - Returns -ve value for errors.
>>> + */
>>> +typedef int (*security_get_pkt_metadata_t)(void *device,
>>> +        struct rte_mbuf *pkt, uint64_t *mt);
>>> +
>>> +/**
>>>    * Get security capabilities of the device.
>>>    *
>>>    * @param    device        crypto/eth device pointer
>>> @@ -145,6 +159,8 @@ struct rte_security_ops {
>>>       /**< Clear a security sessions private data. */
>>>       security_set_pkt_metadata_t set_pkt_metadata;
>>>       /**< Update mbuf metadata. */
>>> +    security_get_pkt_metadata_t get_pkt_metadata;
>>> +    /**< Get metadata from packet. */
>>>       security_capabilities_get_t capabilities_get;
>>>       /**< Get security capabilities. */
>>>   };
>>
>
  
Neil Horman Nov. 22, 2017, 1:27 p.m. UTC | #4
On Wed, Nov 22, 2017 at 06:55:15AM +0000, 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. For such cases, application could register a
> 64 bit metadata in security session, which could be retrieved from the
> packet using "rte_security_get_pkt_metadata" API. Application can use
> this metadata to identify the parameters it need.
> 
> Application can choose what it should register as the metadata. It can
> register SPI or a pointer to SA.
> 
> Signed-off-by: Anoob Joseph <anoob.joseph@caviumnetworks.com>
> ---
> 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..804f11f 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);
>  }
>  
> +uint64_t
> +rte_security_get_pkt_metadata(struct rte_security_ctx *instance,
> +			      struct rte_mbuf *pkt)
> +{
> +	uint64_t mdata = 0;
> +
> +	RTE_FUNC_PTR_OR_ERR_RET(*instance->ops->get_pkt_metadata, 0);
> +	if (instance->ops->get_pkt_metadata(instance->device, pkt, &mdata))
> +		return 0;
> +
> +	return mdata;
> +}
> +
>  const struct rte_security_capability *
>  rte_security_capabilities_get(struct rte_security_ctx *instance)
>  {
> diff --git a/lib/librte_security/rte_security.h b/lib/librte_security/rte_security.h
> index 653929b..aa3a471 100644
> --- a/lib/librte_security/rte_security.h
> +++ b/lib/librte_security/rte_security.h
> @@ -274,6 +274,8 @@ struct rte_security_session_conf {
>  	/**< Configuration parameters for security session */
>  	struct rte_crypto_sym_xform *crypto_xform;
>  	/**< Security Session Crypto Transformations */
> +	uint64_t metadata;
> +	/**< Metadata registered by application */
This is going to break ABI. You need to announce the change so that application
providers can be prepared for it.

>  };
>  
>  struct rte_security_session {
> @@ -346,6 +348,23 @@ rte_security_set_pkt_metadata(struct rte_security_ctx *instance,
>  			      struct rte_mbuf *mb, void *params);
>  
>  /**
> + * Get metadata from the packet. This is an application registered 64 bit
> + * value, associated with the security session which processed the packet.
> + *
> + * This is valid only for inline processed ingress packets.
> + *
> + * @param   instance	security instance
> + * @param   pkt		packet mbuf
> + *
> + * @return
> + *  - On success, metadata
> + *  - On failure, 0
> + */
> +uint64_t
> +rte_security_get_pkt_metadata(struct rte_security_ctx *instance,
> +			      struct rte_mbuf *pkt);
> +
> +/**
>   * Attach a session to a symmetric crypto operation
>   *
>   * @param	sym_op	crypto operation
> diff --git a/lib/librte_security/rte_security_driver.h b/lib/librte_security/rte_security_driver.h
> index 997fbe7..da0ebf4 100644
> --- a/lib/librte_security/rte_security_driver.h
> +++ b/lib/librte_security/rte_security_driver.h
> @@ -122,6 +122,20 @@ typedef int (*security_set_pkt_metadata_t)(void *device,
>  		void *params);
>  
>  /**
> + * Get application interpretable metadata from the packet.
> + *
> + * @param	device		Crypto/eth device pointer
> + * @param	pkt		Packet mbuf
> + * @param	mt		Pointer to receive metadata
> + *
> + * @return
> + *  - Returns 0 if metadata is retrieved successfully.
> + *  - Returns -ve value for errors.
> + */
> +typedef int (*security_get_pkt_metadata_t)(void *device,
> +		struct rte_mbuf *pkt, uint64_t *mt);
> +
> +/**
>   * Get security capabilities of the device.
>   *
>   * @param	device		crypto/eth device pointer
> @@ -145,6 +159,8 @@ struct rte_security_ops {
>  	/**< Clear a security sessions private data. */
>  	security_set_pkt_metadata_t set_pkt_metadata;
>  	/**< Update mbuf metadata. */
> +	security_get_pkt_metadata_t get_pkt_metadata;
> +	/**< Get metadata from packet. */
>  	security_capabilities_get_t capabilities_get;
>  	/**< Get security capabilities. */
>  };
> -- 
> 2.7.4
> 
>
  
Anoob Joseph Nov. 22, 2017, 2:13 p.m. UTC | #5
Hi,

Please see inline.


On 11/22/2017 06:57 PM, Neil Horman wrote:
> On Wed, Nov 22, 2017 at 06:55:15AM +0000, 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. For such cases, application could register a
>> 64 bit metadata in security session, which could be retrieved from the
>> packet using "rte_security_get_pkt_metadata" API. Application can use
>> this metadata to identify the parameters it need.
>>
>> Application can choose what it should register as the metadata. It can
>> register SPI or a pointer to SA.
>>
>> Signed-off-by: Anoob Joseph <anoob.joseph@caviumnetworks.com>
>> ---
>> 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..804f11f 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);
>>   }
>>   
>> +uint64_t
>> +rte_security_get_pkt_metadata(struct rte_security_ctx *instance,
>> +			      struct rte_mbuf *pkt)
>> +{
>> +	uint64_t mdata = 0;
>> +
>> +	RTE_FUNC_PTR_OR_ERR_RET(*instance->ops->get_pkt_metadata, 0);
>> +	if (instance->ops->get_pkt_metadata(instance->device, pkt, &mdata))
>> +		return 0;
>> +
>> +	return mdata;
>> +}
>> +
>>   const struct rte_security_capability *
>>   rte_security_capabilities_get(struct rte_security_ctx *instance)
>>   {
>> diff --git a/lib/librte_security/rte_security.h b/lib/librte_security/rte_security.h
>> index 653929b..aa3a471 100644
>> --- a/lib/librte_security/rte_security.h
>> +++ b/lib/librte_security/rte_security.h
>> @@ -274,6 +274,8 @@ struct rte_security_session_conf {
>>   	/**< Configuration parameters for security session */
>>   	struct rte_crypto_sym_xform *crypto_xform;
>>   	/**< Security Session Crypto Transformations */
>> +	uint64_t metadata;
>> +	/**< Metadata registered by application */
> This is going to break ABI. You need to announce the change so that application
> providers can be prepared for it.
The security library is under experimental tag. So is the announcement 
required?
>
>>   };
>>   
>>   struct rte_security_session {
>> @@ -346,6 +348,23 @@ rte_security_set_pkt_metadata(struct rte_security_ctx *instance,
>>   			      struct rte_mbuf *mb, void *params);
>>   
>>   /**
>> + * Get metadata from the packet. This is an application registered 64 bit
>> + * value, associated with the security session which processed the packet.
>> + *
>> + * This is valid only for inline processed ingress packets.
>> + *
>> + * @param   instance	security instance
>> + * @param   pkt		packet mbuf
>> + *
>> + * @return
>> + *  - On success, metadata
>> + *  - On failure, 0
>> + */
>> +uint64_t
>> +rte_security_get_pkt_metadata(struct rte_security_ctx *instance,
>> +			      struct rte_mbuf *pkt);
>> +
>> +/**
>>    * Attach a session to a symmetric crypto operation
>>    *
>>    * @param	sym_op	crypto operation
>> diff --git a/lib/librte_security/rte_security_driver.h b/lib/librte_security/rte_security_driver.h
>> index 997fbe7..da0ebf4 100644
>> --- a/lib/librte_security/rte_security_driver.h
>> +++ b/lib/librte_security/rte_security_driver.h
>> @@ -122,6 +122,20 @@ typedef int (*security_set_pkt_metadata_t)(void *device,
>>   		void *params);
>>   
>>   /**
>> + * Get application interpretable metadata from the packet.
>> + *
>> + * @param	device		Crypto/eth device pointer
>> + * @param	pkt		Packet mbuf
>> + * @param	mt		Pointer to receive metadata
>> + *
>> + * @return
>> + *  - Returns 0 if metadata is retrieved successfully.
>> + *  - Returns -ve value for errors.
>> + */
>> +typedef int (*security_get_pkt_metadata_t)(void *device,
>> +		struct rte_mbuf *pkt, uint64_t *mt);
>> +
>> +/**
>>    * Get security capabilities of the device.
>>    *
>>    * @param	device		crypto/eth device pointer
>> @@ -145,6 +159,8 @@ struct rte_security_ops {
>>   	/**< Clear a security sessions private data. */
>>   	security_set_pkt_metadata_t set_pkt_metadata;
>>   	/**< Update mbuf metadata. */
>> +	security_get_pkt_metadata_t get_pkt_metadata;
>> +	/**< Get metadata from packet. */
>>   	security_capabilities_get_t capabilities_get;
>>   	/**< Get security capabilities. */
>>   };
>> -- 
>> 2.7.4
>>
>>
  
Neil Horman Nov. 27, 2017, 1:55 p.m. UTC | #6
On Wed, Nov 22, 2017 at 07:43:13PM +0530, Anoob wrote:
> Hi,
> 
> Please see inline.
> 
> 
> On 11/22/2017 06:57 PM, Neil Horman wrote:
> > On Wed, Nov 22, 2017 at 06:55:15AM +0000, 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. For such cases, application could register a
> > > 64 bit metadata in security session, which could be retrieved from the
> > > packet using "rte_security_get_pkt_metadata" API. Application can use
> > > this metadata to identify the parameters it need.
> > > 
> > > Application can choose what it should register as the metadata. It can
> > > register SPI or a pointer to SA.
> > > 
> > > Signed-off-by: Anoob Joseph <anoob.joseph@caviumnetworks.com>
> > > ---
> > > 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..804f11f 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);
> > >   }
> > > +uint64_t
> > > +rte_security_get_pkt_metadata(struct rte_security_ctx *instance,
> > > +			      struct rte_mbuf *pkt)
> > > +{
> > > +	uint64_t mdata = 0;
> > > +
> > > +	RTE_FUNC_PTR_OR_ERR_RET(*instance->ops->get_pkt_metadata, 0);
> > > +	if (instance->ops->get_pkt_metadata(instance->device, pkt, &mdata))
> > > +		return 0;
> > > +
> > > +	return mdata;
> > > +}
> > > +
> > >   const struct rte_security_capability *
> > >   rte_security_capabilities_get(struct rte_security_ctx *instance)
> > >   {
> > > diff --git a/lib/librte_security/rte_security.h b/lib/librte_security/rte_security.h
> > > index 653929b..aa3a471 100644
> > > --- a/lib/librte_security/rte_security.h
> > > +++ b/lib/librte_security/rte_security.h
> > > @@ -274,6 +274,8 @@ struct rte_security_session_conf {
> > >   	/**< Configuration parameters for security session */
> > >   	struct rte_crypto_sym_xform *crypto_xform;
> > >   	/**< Security Session Crypto Transformations */
> > > +	uint64_t metadata;
> > > +	/**< Metadata registered by application */
> > This is going to break ABI. You need to announce the change so that application
> > providers can be prepared for it.
> The security library is under experimental tag. So is the announcement
> required?
Ah, thats correct, its not required.  That said, thats probably the fourth time
I've reviewed a patch and tripped over the fact that it was experimental.  We
should probably look at ways at making that fact more visible at review, compile
and run time

Neil

> > 
> > >   };
> > >   struct rte_security_session {
> > > @@ -346,6 +348,23 @@ rte_security_set_pkt_metadata(struct rte_security_ctx *instance,
> > >   			      struct rte_mbuf *mb, void *params);
> > >   /**
> > > + * Get metadata from the packet. This is an application registered 64 bit
> > > + * value, associated with the security session which processed the packet.
> > > + *
> > > + * This is valid only for inline processed ingress packets.
> > > + *
> > > + * @param   instance	security instance
> > > + * @param   pkt		packet mbuf
> > > + *
> > > + * @return
> > > + *  - On success, metadata
> > > + *  - On failure, 0
> > > + */
> > > +uint64_t
> > > +rte_security_get_pkt_metadata(struct rte_security_ctx *instance,
> > > +			      struct rte_mbuf *pkt);
> > > +
> > > +/**
> > >    * Attach a session to a symmetric crypto operation
> > >    *
> > >    * @param	sym_op	crypto operation
> > > diff --git a/lib/librte_security/rte_security_driver.h b/lib/librte_security/rte_security_driver.h
> > > index 997fbe7..da0ebf4 100644
> > > --- a/lib/librte_security/rte_security_driver.h
> > > +++ b/lib/librte_security/rte_security_driver.h
> > > @@ -122,6 +122,20 @@ typedef int (*security_set_pkt_metadata_t)(void *device,
> > >   		void *params);
> > >   /**
> > > + * Get application interpretable metadata from the packet.
> > > + *
> > > + * @param	device		Crypto/eth device pointer
> > > + * @param	pkt		Packet mbuf
> > > + * @param	mt		Pointer to receive metadata
> > > + *
> > > + * @return
> > > + *  - Returns 0 if metadata is retrieved successfully.
> > > + *  - Returns -ve value for errors.
> > > + */
> > > +typedef int (*security_get_pkt_metadata_t)(void *device,
> > > +		struct rte_mbuf *pkt, uint64_t *mt);
> > > +
> > > +/**
> > >    * Get security capabilities of the device.
> > >    *
> > >    * @param	device		crypto/eth device pointer
> > > @@ -145,6 +159,8 @@ struct rte_security_ops {
> > >   	/**< Clear a security sessions private data. */
> > >   	security_set_pkt_metadata_t set_pkt_metadata;
> > >   	/**< Update mbuf metadata. */
> > > +	security_get_pkt_metadata_t get_pkt_metadata;
> > > +	/**< Get metadata from packet. */
> > >   	security_capabilities_get_t capabilities_get;
> > >   	/**< Get security capabilities. */
> > >   };
> > > -- 
> > > 2.7.4
> > > 
> > > 
> 
>
  

Patch

diff --git a/lib/librte_security/rte_security.c b/lib/librte_security/rte_security.c
index 1227fca..804f11f 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);
 }
 
+uint64_t
+rte_security_get_pkt_metadata(struct rte_security_ctx *instance,
+			      struct rte_mbuf *pkt)
+{
+	uint64_t mdata = 0;
+
+	RTE_FUNC_PTR_OR_ERR_RET(*instance->ops->get_pkt_metadata, 0);
+	if (instance->ops->get_pkt_metadata(instance->device, pkt, &mdata))
+		return 0;
+
+	return mdata;
+}
+
 const struct rte_security_capability *
 rte_security_capabilities_get(struct rte_security_ctx *instance)
 {
diff --git a/lib/librte_security/rte_security.h b/lib/librte_security/rte_security.h
index 653929b..aa3a471 100644
--- a/lib/librte_security/rte_security.h
+++ b/lib/librte_security/rte_security.h
@@ -274,6 +274,8 @@  struct rte_security_session_conf {
 	/**< Configuration parameters for security session */
 	struct rte_crypto_sym_xform *crypto_xform;
 	/**< Security Session Crypto Transformations */
+	uint64_t metadata;
+	/**< Metadata registered by application */
 };
 
 struct rte_security_session {
@@ -346,6 +348,23 @@  rte_security_set_pkt_metadata(struct rte_security_ctx *instance,
 			      struct rte_mbuf *mb, void *params);
 
 /**
+ * Get metadata from the packet. This is an application registered 64 bit
+ * value, associated with the security session which processed the packet.
+ *
+ * This is valid only for inline processed ingress packets.
+ *
+ * @param   instance	security instance
+ * @param   pkt		packet mbuf
+ *
+ * @return
+ *  - On success, metadata
+ *  - On failure, 0
+ */
+uint64_t
+rte_security_get_pkt_metadata(struct rte_security_ctx *instance,
+			      struct rte_mbuf *pkt);
+
+/**
  * Attach a session to a symmetric crypto operation
  *
  * @param	sym_op	crypto operation
diff --git a/lib/librte_security/rte_security_driver.h b/lib/librte_security/rte_security_driver.h
index 997fbe7..da0ebf4 100644
--- a/lib/librte_security/rte_security_driver.h
+++ b/lib/librte_security/rte_security_driver.h
@@ -122,6 +122,20 @@  typedef int (*security_set_pkt_metadata_t)(void *device,
 		void *params);
 
 /**
+ * Get application interpretable metadata from the packet.
+ *
+ * @param	device		Crypto/eth device pointer
+ * @param	pkt		Packet mbuf
+ * @param	mt		Pointer to receive metadata
+ *
+ * @return
+ *  - Returns 0 if metadata is retrieved successfully.
+ *  - Returns -ve value for errors.
+ */
+typedef int (*security_get_pkt_metadata_t)(void *device,
+		struct rte_mbuf *pkt, uint64_t *mt);
+
+/**
  * Get security capabilities of the device.
  *
  * @param	device		crypto/eth device pointer
@@ -145,6 +159,8 @@  struct rte_security_ops {
 	/**< Clear a security sessions private data. */
 	security_set_pkt_metadata_t set_pkt_metadata;
 	/**< Update mbuf metadata. */
+	security_get_pkt_metadata_t get_pkt_metadata;
+	/**< Get metadata from packet. */
 	security_capabilities_get_t capabilities_get;
 	/**< Get security capabilities. */
 };