From mboxrd@z Thu Jan 1 00:00:00 1970 From: Akhil Goyal Subject: Re: [PATCH v2 4/5] examples/ipsec-secgw: handle ESN soft limit event Date: Wed, 21 Mar 2018 13:00:52 +0530 Message-ID: References: <1519191430-19201-1-git-send-email-anoob.joseph@caviumnetworks.com> <1519896103-32479-1-git-send-email-anoob.joseph@caviumnetworks.com> <1519896103-32479-5-git-send-email-anoob.joseph@caviumnetworks.com> <3b57c323-69c0-a20e-b846-d686576ac1da@nxp.com> <1a37eafa-9b6a-64fb-7295-dbfef9c81ff2@caviumnetworks.com> <8e5ecbf8-d739-d6ee-1de5-49eaea2ebf1a@caviumnetworks.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Cc: Jerin Jacob , Narayana Prasad , Nelio Laranjeiro , dev@dpdk.org To: Anoob Joseph , Declan Doherty , Radu Nicolau Return-path: Received: from EUR01-VE1-obe.outbound.protection.outlook.com (mail-ve1eur01on0063.outbound.protection.outlook.com [104.47.1.63]) by dpdk.org (Postfix) with ESMTP id 9357B2C36 for ; Wed, 21 Mar 2018 08:31:20 +0100 (CET) In-Reply-To: <8e5ecbf8-d739-d6ee-1de5-49eaea2ebf1a@caviumnetworks.com> Content-Language: en-US List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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 >>>> --- >>>> 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 >>>>   #include >>>>   #include >>>> +#include >>>>     #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)) >>>> >>> >> > >