> -----Original Message----- > From: Akhil Goyal [mailto:akhil.go...@nxp.com] > Sent: Wednesday, January 2, 2019 1:51 PM > To: Ananyev, Konstantin <konstantin.anan...@intel.com>; dev@dpdk.org > Cc: sta...@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v5 03/10] examples/ipsec-secgw: fix crypto-op > might never get dequeued > > > > On 1/2/2019 7:13 PM, Ananyev, Konstantin wrote: > > > >> On 12/28/2018 9:03 PM, Konstantin Ananyev wrote: > >>> In some cases crypto-ops could never be dequeued from the crypto-device. > >>> The easiest way to reproduce: > >>> start ipsec-secgw with crypto-dev and send to it less then 32 packets. > >>> none packets will be forwarded. > >>> Reason for that is that the application does dequeue() from crypto-queues > >>> only when new packets arrive. > >>> This patch makes sure it calls dequeue() on a regular basis. > >>> > >>> Fixes: c64278c0c18b ("examples/ipsec-secgw: rework processing loop") > >>> Cc: sta...@dpdk.org > >>> > >>> Signed-off-by: Konstantin Ananyev <konstantin.anan...@intel.com> > >>> Acked-by: Radu Nicolau <radu.nico...@intel.com> > >>> --- > >>> examples/ipsec-secgw/ipsec-secgw.c | 136 ++++++++++++++++++++++++----- > >>> examples/ipsec-secgw/ipsec.c | 60 ++++++++----- > >>> examples/ipsec-secgw/ipsec.h | 11 +++ > >>> 3 files changed, 165 insertions(+), 42 deletions(-) > >> [snip] > >>> + > >>> /* main processing loop */ > >>> static int32_t > >>> main_loop(__attribute__((unused)) void *dummy) > >>> @@ -866,7 +958,8 @@ main_loop(__attribute__((unused)) void *dummy) > >>> diff_tsc = cur_tsc - prev_tsc; > >>> > >>> if (unlikely(diff_tsc > drain_tsc)) { > >>> - drain_buffers(qconf); > >>> + drain_tx_buffers(qconf); > >>> + drain_crypto_buffers(qconf); > >>> prev_tsc = cur_tsc; > >>> } > >>> > >>> @@ -880,6 +973,9 @@ main_loop(__attribute__((unused)) void *dummy) > >>> if (nb_rx > 0) > >>> process_pkts(qconf, pkts, nb_rx, > >>> portid); > >>> } > >>> + > >>> + drain_inbound_crypto_queues(qconf, &qconf->inbound); > >>> + drain_outbound_crypto_queues(qconf, &qconf->outbound); > >> drain_inbound_crypto_queues and drain_outbound_crypto_queues should be > >> called based on diff_tsc. > >> moving these two lines above after drain_crypto_buffers will improve the > >> performance drop caused due to this patch. > > Thanks, good to know. > > To make what you suggest above to work properly with non-legacy mode ('-l') > > extra changes > > would be needed... > What changes do you see?
Non-legacy mode relies on a drain_crypto_queues() to dequeuer crypto-ops. It doesn't do that as part of process_pkts(). It is doable, but it means I have to rework my patches a bit. > > Do you have an idea - what exactly causing a slowdown? > > Just an extra function calls (drain_inbound_crypto_queues/ > > drain_outbound_crypto_queues)? > > Or is that because we do dequeue() from crypto PMD more often then before? > I have not profiled it, but it should be because of more dequeues. On a > single call to dequeue, a burst of packets get dequeued. but now there > will be a lot more dequeues which have lesser packets than the burst > size which will deteriorate the performance as it would be wasting the > dequeue cycles. > > This patch is causing around 5% drop out of the 10% that I mentioned in > the other mail. > With the change that I suggested, I am almost able to get back those 5%. Great, any idea what causing other 5%? Konstantin
