DPDK patches and discussions
 help / color / mirror / Atom feed
From: Lukas Bartosik <lbartosik@marvell.com>
To: Akhil Goyal <akhil.goyal@nxp.com>, Anoob Joseph <anoobj@marvell.com>
Cc: Jerin Jacob Kollanukkaran <jerinj@marvell.com>,
	"Narayana Prasad Raju Athreya" <pathreya@marvell.com>,
	Ankur Dwivedi <adwivedi@marvell.com>,
	"Archana Muniganti" <marchana@marvell.com>,
	Tejasree Kondoj <ktejasree@marvell.com>,
	Vamsi Krishna Attunuru <vattunuru@marvell.com>,
	Konstantin Ananyev <konstantin.ananyev@intel.com>,
	"dev@dpdk.org" <dev@dpdk.org>,
	"Thomas Monjalon" <thomas@monjalon.net>,
	Radu Nicolau <radu.nicolau@intel.com>
Subject: Re: [dpdk-dev] [EXT] RE: [PATCH v4 12/15] examples/ipsec-secgw: add app mode worker
Date: Thu, 27 Feb 2020 14:31:31 +0000	[thread overview]
Message-ID: <bac8ca96-5533-4ffe-8c3b-1823bcbf5947@marvell.com> (raw)
In-Reply-To: <VE1PR04MB6639CF657B91F6E63B03F94FE6EB0@VE1PR04MB6639.eurprd04.prod.outlook.com>

Hi Akhil,

Please see my answer below.

Thanks,
Lukasz

On 27.02.2020 13:07, Akhil Goyal wrote:
>>
>> Hi Lukasz,
>>
>>>>
>>>> Is it not possible to use the existing functions for finding routes, checking
>>> packet types and checking security policies.
>>>> It will be very difficult to manage two separate functions for same work. I
>> can
>>> see that the pkt->data_offs
>>>> Are not required to be updated in the inline case, but can we split the existing
>>> functions in two so that they can be
>>>> Called in the appropriate cases.
>>>>
>>>> As you have said in the cover note as well to add lookaside protocol support.
>> I
>>> also tried adding it, and it will get very
>>>> Difficult to manage separate functions for separate code paths.
>>>>
>>>
>>> [Lukasz] This was also Konstantin's comment during review of one of previous
>>> revisions.
>>> The prepare_one_packet() and prepare_tx_pkt() do much more than we need
>>> and for performance reasons
>>> we crafted new functions. For example, process_ipsec_get_pkt_type function
>>> returns nlp and whether
>>> packet type is plain or IPsec. That's all. Prepare_one_packet() process packets
>> in
>>> chunks and does much more -
>>> it adjusts mbuf and packet length then it demultiplex packets into plain and
>> IPsec
>>> flows and finally does
>>> inline checks. This is similar for update_mac_addrs() vs prepare_tx_pkt() and
>>> check_sp() vs inbound_sp_sa()
>>> that prepare_tx_pkt() and inbound_sp_sa() do more that we need in event
>> mode.
>>>
>>> I understand your concern from the perspective of code maintenance but on
>> the
>>> other hand we are concerned with performance.
>>> The current code is not optimized to support multiple mode processing
>>> introduced with rte_security. We can work on a common
>>> routines once we have other modes also added, so that we can come up with
>> a
>>> better solution than what we have today.
>>>
>>
>> Yes that is correct, but we should split the existing functions so that the part
>> which is common
>> In both mode should stay common and we do not have duplicate code in the app.
>>
>> I believe we should take care of this when we add lookaside cases. We shall
>> remove all duplicate
>> Code. Ideally it should be part of this patchset. But we can postpone it to the
>> lookaside case addition.
>>
>>
> 
> I believe the route(4/6)_pkts and route(4/6)_pkt can be made uniform quite easily.
> Now you can call either send_single_pkt() or rte_event_eth_tx_adapter_enqueue() from
> the caller of route4_pkts.
> I don’t think this will impact the performance at all.
> Instead of having 3 for loops, now there will be only 2 and nothing else is getting changed for
> anybody. In fact we can reduce 1 more, if we can call send pkts from inside the route4_pkts.
> I think that can also be done, but it may increase the lookup duration as there may be cache miss.
> But that need to be experimented. What say??
> 
> static inline void
> route4_pkts(struct rt_ctx *rt_ctx, struct rte_mbuf *pkts[], uint32_t *hop[],
>                 uint8_t nb_pkts)
> {
>         uint32_t dst_ip;
>         uint16_t i, offset;
> 
>         if (nb_pkts == 0)
>                 return;
> 
>         /* Need to do an LPM lookup for non-inline packets. Inline packets will
>          * have port ID in the SA
>          */
> 
>         for (i = 0; i < nb_pkts; i++) {
>                 if (!(pkts[i]->ol_flags & PKT_TX_SEC_OFFLOAD)) {
>                         /* Security offload not enabled. So an LPM lookup is
>                          * required to get the hop
>                          */
>                         offset = offsetof(struct ip, ip_dst);
>                         dst_ip = *rte_pktmbuf_mtod_offset(pkts[i],
>                                         uint32_t *, offset);
>                         dst_ip = rte_be_to_cpu_32(dst_ip);
>                         if (rte_lpm_lookup((struct rte_lpm *)rt_ctx, dst_ip, hop[i]))
>                                 rte_pktmbuf_free(pkts[i]);
>                 } else {
>                         *hop[i] = get_hop_for_offload_pkt(pkts[i], 0);
>                 }
>         }
> }
> 

[Lukasz] Thank you for your suggestion. Looking at the proposed change I have major concern related
to performance. Current rout4_pkts uses rte_lpm_lookup_bulk() which can benefit from SIMD instructions.
Replacing it with rte_lpm_lookup might introduce substantial performance degradation. I will start 
experimenting with processing functions (routing packets, checking packet type, checking sp policies) to make
them as much common as possible between poll and event modes. As agreed the plan is to make processing functions common
with the addition of lookaside event mode. In the meantime I will send V5 event mode patches which address your other comments.

  reply	other threads:[~2020-02-27 14:31 UTC|newest]

Thread overview: 147+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-08 12:30 [dpdk-dev] [PATCH 00/14] add eventmode to ipsec-secgw Anoob Joseph
2019-12-08 12:30 ` [dpdk-dev] [PATCH 01/14] examples/ipsec-secgw: add default rte_flow for inline Rx Anoob Joseph
2019-12-16 14:20   ` Ananyev, Konstantin
2019-12-16 15:58     ` Anoob Joseph
2020-01-09 12:01       ` Lukas Bartosik
2020-01-09 19:09         ` Ananyev, Konstantin
2020-01-13 11:40           ` Ananyev, Konstantin
2019-12-08 12:30 ` [dpdk-dev] [PATCH 02/14] examples/ipsec-secgw: add framework for eventmode helper Anoob Joseph
2019-12-08 12:30 ` [dpdk-dev] [PATCH 03/14] examples/ipsec-secgw: add eventdev port-lcore link Anoob Joseph
2019-12-08 12:30 ` [dpdk-dev] [PATCH 04/14] examples/ipsec-secgw: add Rx adapter support Anoob Joseph
2019-12-11 11:33   ` Akhil Goyal
2019-12-12  5:18     ` Anoob Joseph
2019-12-23 18:48   ` Ananyev, Konstantin
2020-01-07  6:12     ` Anoob Joseph
2020-01-07 14:32       ` Ananyev, Konstantin
2019-12-08 12:30 ` [dpdk-dev] [PATCH 05/14] examples/ipsec-secgw: add Tx " Anoob Joseph
2019-12-08 12:30 ` [dpdk-dev] [PATCH 06/14] examples/ipsec-secgw: add routines to display config Anoob Joseph
2019-12-08 12:30 ` [dpdk-dev] [PATCH 07/14] examples/ipsec-secgw: add routines to launch workers Anoob Joseph
2019-12-08 12:30 ` [dpdk-dev] [PATCH 08/14] examples/ipsec-secgw: add support for internal ports Anoob Joseph
2019-12-08 12:30 ` [dpdk-dev] [PATCH 09/14] examples/ipsec-secgw: add eventmode to ipsec-secgw Anoob Joseph
2019-12-23 16:43   ` Ananyev, Konstantin
2020-01-03 10:18     ` Anoob Joseph
2020-01-06 15:45       ` Ananyev, Konstantin
2020-01-09  6:17         ` Anoob Joseph
2019-12-24 12:47   ` Ananyev, Konstantin
2020-01-03 10:20     ` Anoob Joseph
2020-01-06 16:50       ` Ananyev, Konstantin
2020-01-07  6:56         ` Anoob Joseph
2020-01-07 14:38           ` Ananyev, Konstantin
2019-12-08 12:30 ` [dpdk-dev] [PATCH 10/14] examples/ipsec-secgw: add app inbound worker Anoob Joseph
2019-12-08 12:30 ` [dpdk-dev] [PATCH 11/14] examples/ipsec-secgw: add app processing code Anoob Joseph
2019-12-23 16:49   ` Ananyev, Konstantin
2020-01-10 14:28     ` [dpdk-dev] [EXT] " Lukas Bartosik
2019-12-24 13:13   ` [dpdk-dev] " Ananyev, Konstantin
2020-01-10 14:36     ` [dpdk-dev] [EXT] " Lukas Bartosik
2019-12-25 15:18   ` [dpdk-dev] " Ananyev, Konstantin
2020-01-07  6:16     ` Anoob Joseph
2019-12-08 12:30 ` [dpdk-dev] [PATCH 12/14] examples/ipsec-secgw: add driver outbound worker Anoob Joseph
2019-12-23 17:28   ` Ananyev, Konstantin
2020-01-04 10:58     ` Anoob Joseph
2020-01-06 17:46       ` Ananyev, Konstantin
2020-01-07  4:32         ` Anoob Joseph
2020-01-07 14:30           ` Ananyev, Konstantin
2020-01-09 11:49             ` Anoob Joseph
2019-12-08 12:30 ` [dpdk-dev] [PATCH 13/14] examples/ipsec-secgw: add app " Anoob Joseph
2019-12-08 12:30 ` [dpdk-dev] [PATCH 14/14] examples/ipsec-secgw: add cmd line option for bufs Anoob Joseph
2019-12-23 16:14   ` Ananyev, Konstantin
2019-12-23 16:16     ` Ananyev, Konstantin
2020-01-03  5:42       ` Anoob Joseph
2020-01-06 15:21         ` Ananyev, Konstantin
2020-01-20 13:45 ` [dpdk-dev] [PATCH v2 00/12] add eventmode to ipsec-secgw Anoob Joseph
2020-01-20 13:45   ` [dpdk-dev] [PATCH v2 01/12] examples/ipsec-secgw: add default rte_flow for inline Rx Anoob Joseph
2020-01-20 13:45   ` [dpdk-dev] [PATCH v2 02/12] examples/ipsec-secgw: add framework for eventmode helper Anoob Joseph
2020-01-20 13:45   ` [dpdk-dev] [PATCH v2 03/12] examples/ipsec-secgw: add eventdev port-lcore link Anoob Joseph
2020-01-20 13:45   ` [dpdk-dev] [PATCH v2 04/12] examples/ipsec-secgw: add Rx adapter support Anoob Joseph
2020-01-20 13:45   ` [dpdk-dev] [PATCH v2 05/12] examples/ipsec-secgw: add Tx " Anoob Joseph
2020-01-20 13:45   ` [dpdk-dev] [PATCH v2 06/12] examples/ipsec-secgw: add routines to display config Anoob Joseph
2020-01-20 13:45   ` [dpdk-dev] [PATCH v2 07/12] examples/ipsec-secgw: add routines to launch workers Anoob Joseph
2020-01-20 13:45   ` [dpdk-dev] [PATCH v2 08/12] examples/ipsec-secgw: add support for internal ports Anoob Joseph
2020-01-20 13:45   ` [dpdk-dev] [PATCH v2 09/12] examples/ipsec-secgw: add eventmode to ipsec-secgw Anoob Joseph
2020-01-29 23:31     ` Ananyev, Konstantin
2020-01-30 11:04       ` [dpdk-dev] [EXT] " Lukas Bartosik
2020-01-30 11:13         ` Ananyev, Konstantin
2020-01-30 22:21           ` Ananyev, Konstantin
2020-01-31  1:09             ` Lukas Bartosik
2020-02-02 23:00               ` Lukas Bartosik
2020-02-03  7:50                 ` Ananyev, Konstantin
2020-01-20 13:45   ` [dpdk-dev] [PATCH v2 10/12] examples/ipsec-secgw: add driver mode worker Anoob Joseph
2020-01-29 22:22     ` Ananyev, Konstantin
2020-01-20 13:45   ` [dpdk-dev] [PATCH v2 11/12] examples/ipsec-secgw: add app " Anoob Joseph
2020-01-29 15:34     ` Ananyev, Konstantin
2020-01-29 17:18       ` Anoob Joseph
2020-01-20 13:45   ` [dpdk-dev] [PATCH v2 12/12] examples/ipsec-secgw: add cmd line option for bufs Anoob Joseph
2020-01-29 14:40     ` Ananyev, Konstantin
2020-01-29 17:14       ` Anoob Joseph
2020-01-28  5:02   ` [dpdk-dev] [PATCH v2 00/12] add eventmode to ipsec-secgw Anoob Joseph
2020-01-28 13:00     ` Ananyev, Konstantin
2020-02-04 13:58   ` [dpdk-dev] [PATCH v3 00/13] " Lukasz Bartosik
2020-02-04 13:58     ` [dpdk-dev] [PATCH v3 01/13] examples/ipsec-secgw: add default rte flow for inline Rx Lukasz Bartosik
2020-02-04 13:58     ` [dpdk-dev] [PATCH v3 02/13] examples/ipsec-secgw: add framework for eventmode helper Lukasz Bartosik
2020-02-04 13:58     ` [dpdk-dev] [PATCH v3 03/13] examples/ipsec-secgw: add eventdev port-lcore link Lukasz Bartosik
2020-02-04 13:58     ` [dpdk-dev] [PATCH v3 04/13] examples/ipsec-secgw: add Rx adapter support Lukasz Bartosik
2020-02-04 13:58     ` [dpdk-dev] [PATCH v3 05/13] examples/ipsec-secgw: add Tx " Lukasz Bartosik
2020-02-04 13:58     ` [dpdk-dev] [PATCH v3 06/13] examples/ipsec-secgw: add routines to display config Lukasz Bartosik
2020-02-04 13:58     ` [dpdk-dev] [PATCH v3 07/13] examples/ipsec-secgw: add routines to launch workers Lukasz Bartosik
2020-02-04 13:58     ` [dpdk-dev] [PATCH v3 08/13] examples/ipsec-secgw: add support for internal ports Lukasz Bartosik
2020-02-04 13:58     ` [dpdk-dev] [PATCH v3 09/13] examples/ipsec-secgw: add event helper config init/uninit Lukasz Bartosik
2020-02-04 13:58     ` [dpdk-dev] [PATCH v3 10/13] examples/ipsec-secgw: add eventmode to ipsec-secgw Lukasz Bartosik
2020-02-04 13:58     ` [dpdk-dev] [PATCH v3 11/13] examples/ipsec-secgw: add driver mode worker Lukasz Bartosik
2020-02-04 13:58     ` [dpdk-dev] [PATCH v3 12/13] examples/ipsec-secgw: add app " Lukasz Bartosik
2020-02-04 13:58     ` [dpdk-dev] [PATCH v3 13/13] examples/ipsec-secgw: make number of buffers dynamic Lukasz Bartosik
2020-02-05 13:42       ` Ananyev, Konstantin
2020-02-05 16:08         ` [dpdk-dev] [EXT] " Lukas Bartosik
2020-02-20  8:01     ` [dpdk-dev] [PATCH v4 00/15] add eventmode to ipsec-secgw Lukasz Bartosik
2020-02-20  8:01       ` [dpdk-dev] [PATCH v4 01/15] examples/ipsec-secgw: add default rte flow for inline Rx Lukasz Bartosik
2020-02-20  8:01       ` [dpdk-dev] [PATCH v4 02/15] examples/ipsec-secgw: add framework for eventmode helper Lukasz Bartosik
2020-02-20  8:01       ` [dpdk-dev] [PATCH v4 03/15] examples/ipsec-secgw: add eventdev port-lcore link Lukasz Bartosik
2020-02-20  8:01       ` [dpdk-dev] [PATCH v4 04/15] examples/ipsec-secgw: add Rx adapter support Lukasz Bartosik
2020-02-20  8:01       ` [dpdk-dev] [PATCH v4 05/15] examples/ipsec-secgw: add Tx " Lukasz Bartosik
2020-02-20  8:01       ` [dpdk-dev] [PATCH v4 06/15] examples/ipsec-secgw: add routines to display config Lukasz Bartosik
2020-02-20  8:01       ` [dpdk-dev] [PATCH v4 07/15] examples/ipsec-secgw: add routines to launch workers Lukasz Bartosik
2020-02-20  8:02       ` [dpdk-dev] [PATCH v4 08/15] examples/ipsec-secgw: add support for internal ports Lukasz Bartosik
2020-02-20  8:02       ` [dpdk-dev] [PATCH v4 09/15] examples/ipsec-secgw: add event helper config init/uninit Lukasz Bartosik
2020-02-20  8:02       ` [dpdk-dev] [PATCH v4 10/15] examples/ipsec-secgw: add eventmode to ipsec-secgw Lukasz Bartosik
2020-02-20  8:02       ` [dpdk-dev] [PATCH v4 11/15] examples/ipsec-secgw: add driver mode worker Lukasz Bartosik
2020-02-20  8:02       ` [dpdk-dev] [PATCH v4 12/15] examples/ipsec-secgw: add app " Lukasz Bartosik
2020-02-24 14:13         ` Akhil Goyal
2020-02-25 11:50           ` [dpdk-dev] [EXT] " Lukas Bartosik
2020-02-25 12:13             ` Anoob Joseph
2020-02-25 16:03               ` Ananyev, Konstantin
2020-02-26  4:33                 ` Anoob Joseph
2020-02-26  5:55                   ` Akhil Goyal
2020-02-26 12:36                     ` Ananyev, Konstantin
2020-02-26  6:04             ` Akhil Goyal
2020-02-26 10:32               ` Lukas Bartosik
2020-02-27 12:07               ` Akhil Goyal
2020-02-27 14:31                 ` Lukas Bartosik [this message]
2020-02-20  8:02       ` [dpdk-dev] [PATCH v4 13/15] examples/ipsec-secgw: make number of buffers dynamic Lukasz Bartosik
2020-02-20  8:02       ` [dpdk-dev] [PATCH v4 14/15] doc: add event mode support to ipsec-secgw Lukasz Bartosik
2020-02-20  8:02       ` [dpdk-dev] [PATCH v4 15/15] examples/ipsec-secgw: reserve crypto queues in event mode Lukasz Bartosik
2020-02-24  5:20       ` [dpdk-dev] [PATCH v4 00/15] add eventmode to ipsec-secgw Anoob Joseph
2020-02-24 13:40       ` Akhil Goyal
2020-02-25 12:09         ` [dpdk-dev] [EXT] " Lukas Bartosik
2020-02-27 16:18       ` [dpdk-dev] [PATCH v5 " Lukasz Bartosik
2020-02-27 16:18         ` [dpdk-dev] [PATCH v5 01/15] examples/ipsec-secgw: add default rte flow for inline Rx Lukasz Bartosik
2020-02-27 16:18         ` [dpdk-dev] [PATCH v5 02/15] examples/ipsec-secgw: add framework for eventmode helper Lukasz Bartosik
2020-02-27 16:18         ` [dpdk-dev] [PATCH v5 03/15] examples/ipsec-secgw: add eventdev port-lcore link Lukasz Bartosik
2020-02-27 16:18         ` [dpdk-dev] [PATCH v5 04/15] examples/ipsec-secgw: add Rx adapter support Lukasz Bartosik
2020-02-27 16:18         ` [dpdk-dev] [PATCH v5 05/15] examples/ipsec-secgw: add Tx " Lukasz Bartosik
2020-02-27 16:18         ` [dpdk-dev] [PATCH v5 06/15] examples/ipsec-secgw: add routines to display config Lukasz Bartosik
2020-02-27 16:18         ` [dpdk-dev] [PATCH v5 07/15] examples/ipsec-secgw: add routines to launch workers Lukasz Bartosik
2020-02-27 16:18         ` [dpdk-dev] [PATCH v5 08/15] examples/ipsec-secgw: add support for internal ports Lukasz Bartosik
2020-02-27 16:18         ` [dpdk-dev] [PATCH v5 09/15] examples/ipsec-secgw: add event helper config init/uninit Lukasz Bartosik
2020-02-27 16:18         ` [dpdk-dev] [PATCH v5 10/15] examples/ipsec-secgw: add eventmode to ipsec-secgw Lukasz Bartosik
2020-02-27 16:18         ` [dpdk-dev] [PATCH v5 11/15] examples/ipsec-secgw: add driver mode worker Lukasz Bartosik
2020-02-27 16:18         ` [dpdk-dev] [PATCH v5 12/15] examples/ipsec-secgw: add app " Lukasz Bartosik
2020-02-27 16:18         ` [dpdk-dev] [PATCH v5 13/15] examples/ipsec-secgw: make number of buffers dynamic Lukasz Bartosik
2020-02-27 16:18         ` [dpdk-dev] [PATCH v5 14/15] doc: add event mode support to ipsec-secgw Lukasz Bartosik
2020-04-12 16:37           ` Thomas Monjalon
2020-02-27 16:18         ` [dpdk-dev] [PATCH v5 15/15] examples/ipsec-secgw: reserve crypto queues in event mode Lukasz Bartosik
2020-03-02  8:47         ` [dpdk-dev] [PATCH v5 00/15] add eventmode to ipsec-secgw Anoob Joseph
2020-03-02  8:57           ` Akhil Goyal
2020-03-03 18:00         ` Ananyev, Konstantin
2020-03-12  5:32           ` Anoob Joseph
2020-03-12  5:55             ` Akhil Goyal
2020-03-12  9:57               ` [dpdk-dev] [EXT] " Lukas Bartosik
2020-03-12 13:25                 ` Akhil Goyal

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=bac8ca96-5533-4ffe-8c3b-1823bcbf5947@marvell.com \
    --to=lbartosik@marvell.com \
    --cc=adwivedi@marvell.com \
    --cc=akhil.goyal@nxp.com \
    --cc=anoobj@marvell.com \
    --cc=dev@dpdk.org \
    --cc=jerinj@marvell.com \
    --cc=konstantin.ananyev@intel.com \
    --cc=ktejasree@marvell.com \
    --cc=marchana@marvell.com \
    --cc=pathreya@marvell.com \
    --cc=radu.nicolau@intel.com \
    --cc=thomas@monjalon.net \
    --cc=vattunuru@marvell.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).