[dpdk-dev] [PATCH v2 4/5] examples/ipsec-secgw: handle ESN soft limit event

[Akhil Goyal]

Hi Anoob,

On 3/21/2018 10:50 AM, Anoob Joseph wrote:
> Hi Akhil,
> 
> If you are fine with the existing code, I'll send a revised patchset 
> incorporating the comment change you had suggested for 3rd patch. Shall 
> I proceed?
> 
> Thanks,
> Anoob
> 

Yes you can send the patchset with existing code.
BTW we are open for an approach to add sa rediscovery in the application 
in future.

Thanks,
Akhil

> On 14/03/18 11:36, Anoob Joseph wrote:
>> Hi Akhil,
>>
>> Please see inline.
>>
>> Thanks,
>> Anoob
>>
>> On 13/03/18 17:54, Akhil Goyal wrote:
>>> Hi Anoob,
>>>
>>> On 3/1/2018 2:51 PM, Anoob Joseph wrote:
>>>> For inline protocol processing, the PMD/device is required to maintain
>>>> the ESN. But the application is required to monitor ESN overflow to
>>>> initiate SA expiry.
>>>>
>>>> For such cases, application would set the ESN soft limit. An IPsec 
>>>> event
>>>> would be raised by rte_eth_event framework, when ESN hits the soft 
>>>> limit
>>>> set by the application.
>>>>
>>>> Signed-off-by: Anoob Joseph <anoob.joseph at caviumnetworks.com>
>>>> ---
>>>> v2:
>>>> * No change
>>>>
>>>>   examples/ipsec-secgw/ipsec-secgw.c | 56 
>>>> ++++++++++++++++++++++++++++++++++++++
>>>>   examples/ipsec-secgw/ipsec.c       | 10 +++++--
>>>>   examples/ipsec-secgw/ipsec.h       |  2 ++
>>>>   3 files changed, 65 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/examples/ipsec-secgw/ipsec-secgw.c 
>>>> b/examples/ipsec-secgw/ipsec-secgw.c
>>>> index 3a8562e..5726fd3 100644
>>>> --- a/examples/ipsec-secgw/ipsec-secgw.c
>>>> +++ b/examples/ipsec-secgw/ipsec-secgw.c
>>>> @@ -40,6 +40,7 @@
>>>>   #include <rte_hash.h>
>>>>   #include <rte_jhash.h>
>>>>   #include <rte_cryptodev.h>
>>>> +#include <rte_security.h>
>>>>     #include "ipsec.h"
>>>>   #include "parser.h"
>>>> @@ -1640,6 +1641,58 @@ pool_init(struct socket_ctx *ctx, int32_t 
>>>> socket_id, uint32_t nb_mbuf)
>>>>           printf("Allocated mbuf pool on socket %d\n", socket_id);
>>>>   }
>>>>   +static inline int
>>>> +inline_ipsec_event_esn_overflow(struct rte_security_ctx *ctx, 
>>>> uint64_t md)
>>>> +{
>>>> +    struct ipsec_sa *sa;
>>>> +
>>>> +    /* For inline protocol processing, the metadata in the event will
>>>> +     * uniquely identify the security session which raised the event.
>>>> +     * Application would then need the userdata it had registered 
>>>> with the
>>>> +     * security session to process the event.
>>>> +     */
>>>> +
>>>> +    sa = (struct ipsec_sa *)rte_security_get_userdata(ctx, md);
>>>> +
>>>> +    if (sa == NULL) {
>>>> +        /* userdata could not be retrieved */
>>>> +        return -1;
>>>> +    }
>>>> +
>>>> +    /* Sequence number over flow. SA need to be re-established */
>>>
>>>
>>> With this patchset, application will be able to get notification if 
>>> the error has occurred. But it is not re-configuring the SA.
>>> Do you intend to add the same?
>> Ideally the application should initiate a SA renegotiation sequence 
>> (with IKE etc). But ipsec-secgw uses predetermined SAs, and so 
>> addition of SA renegotiation might not fit in with the current design. 
>> I was just adding this as a place holder for future expansion (and a 
>> model for real applications).
>>
>> What are your thoughts on addition here? Similar handling would be 
>> needed for byte & time expiry as well, when that is added. May be we 
>> could just log the event and leave it be.
>>>
>>>> +    RTE_SET_USED(sa);
>>>> +    return 0;
>>>> +}
>>>> +
>>>> +static int
>>>> +inline_ipsec_event_callback(uint16_t port_id, enum 
>>>> rte_eth_event_type type,
>>>> +         void *param, void *ret_param)
>>>> +{
>>>> +    struct rte_eth_event_ipsec_desc *event_desc = NULL;
>>>> +    struct rte_security_ctx *ctx = (struct rte_security_ctx *)
>>>> +                    rte_eth_dev_get_sec_ctx(port_id);
>>>> +
>>>> +    RTE_SET_USED(param);
>>>> +
>>>> +    if (type != RTE_ETH_EVENT_IPSEC)
>>>> +        return -1;
>>>> +
>>>> +    event_desc = ret_param;
>>>> +    if (event_desc == NULL) {
>>>> +        printf("Event descriptor not set\n");
>>>> +        return -1;
>>>> +    }
>>>> +
>>>> +    if (event_desc->stype == RTE_ETH_EVENT_IPSEC_ESN_OVERFLOW)
>>>> +        return inline_ipsec_event_esn_overflow(ctx, event_desc->md);
>>>> +    else if (event_desc->stype >= RTE_ETH_EVENT_IPSEC_MAX) {
>>>> +        printf("Invalid IPsec event reported\n");
>>>> +        return -1;
>>>> +    }
>>>> +
>>>> +    return -1;
>>>> +}
>>>> +
>>>>   int32_t
>>>>   main(int32_t argc, char **argv)
>>>>   {
>>>> @@ -1727,6 +1780,9 @@ main(int32_t argc, char **argv)
>>>>            */
>>>>           if (promiscuous_on)
>>>>               rte_eth_promiscuous_enable(portid);
>>>> +
>>>> +        rte_eth_dev_callback_register(portid,
>>>> +            RTE_ETH_EVENT_IPSEC, inline_ipsec_event_callback, NULL);
>>>>       }
>>>>         check_all_ports_link_status(nb_ports, enabled_port_mask);
>>>> diff --git a/examples/ipsec-secgw/ipsec.c 
>>>> b/examples/ipsec-secgw/ipsec.c
>>>> index 5fb5bc1..acdd189 100644
>>>> --- a/examples/ipsec-secgw/ipsec.c
>>>> +++ b/examples/ipsec-secgw/ipsec.c
>>>> @@ -36,6 +36,7 @@ set_ipsec_conf(struct ipsec_sa *sa, struct 
>>>> rte_security_ipsec_xform *ipsec)
>>>>           }
>>>>           /* TODO support for Transport and IPV6 tunnel */
>>>>       }
>>>> +    ipsec->esn_soft_limit = IPSEC_OFFLOAD_ESN_SOFTLIMIT;
>>>>   }
>>>>     static inline int
>>>> @@ -270,11 +271,14 @@ create_session(struct ipsec_ctx *ipsec_ctx, 
>>>> struct ipsec_sa *sa)
>>>>                * the packet is received, this userdata will be
>>>>                * retrieved using the metadata from the packet.
>>>>                *
>>>> -             * This is required only for inbound SAs.
>>>> +             * The PMD is expected to set similar metadata for other
>>>> +             * operations, like rte_eth_event, which are tied to
>>>> +             * security session. In such cases, the userdata could
>>>> +             * be obtained to uniquely identify the security
>>>> +             * parameters denoted.
>>>>                */
>>>>   -            if (sa->direction == RTE_SECURITY_IPSEC_SA_DIR_INGRESS)
>>>> -                sess_conf.userdata = (void *) sa;
>>>> +            sess_conf.userdata = (void *) sa;
>>>>                 sa->sec_session = rte_security_session_create(ctx,
>>>>                       &sess_conf, ipsec_ctx->session_pool);
>>>> diff --git a/examples/ipsec-secgw/ipsec.h 
>>>> b/examples/ipsec-secgw/ipsec.h
>>>> index 6059f6c..c1450f6 100644
>>>> --- a/examples/ipsec-secgw/ipsec.h
>>>> +++ b/examples/ipsec-secgw/ipsec.h
>>>> @@ -21,6 +21,8 @@
>>>>     #define MAX_DIGEST_SIZE 32 /* Bytes -- 256 bits */
>>>>   +#define IPSEC_OFFLOAD_ESN_SOFTLIMIT 0xffffff00
>>>> +
>>>>   #define IV_OFFSET        (sizeof(struct rte_crypto_op) + \
>>>>                   sizeof(struct rte_crypto_sym_op))
>>>>
>>>
>>
> 
>