Hi Akhil, > >>> 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? > > 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%.
After another thought, I suppose we have 2 options here: 1. move drain_crypto_queues into timeout section (as you suggested above). 2. Make pkt_process() not to dequeue packets from crypto-dev (leave that to drain_crypto_queues). For V6 I choose option #2 - it seems a bit cleaner and easier to me, plus on my box it is ~1-1.5% faster than option #1. Also V6 contains few other fixes for the issues you reported. Please give it a try, if you'll still find it slower than #1 on your HW - NP to use option #1. Konstantin