Hi Bhanuprakash, Thanks for testing. Comments inline. Best regards, Ilya Maximets.
On 01.08.2017 18:33, Bodireddy, Bhanuprakash wrote: >> This patch-set inspired by [1] from Bhanuprakash Bodireddy. >> Implementation of [1] looks very complex and introduces many pitfalls for >> later code modifications like possible packet stucks. >> >> This version targeted to make simple and flexible output packet batching on >> higher level without introducing and even simplifying netdev layer. >> >> Patch set consists of 3 patches. All the functionality introduced in the >> first >> patch. Two others are just cleanups of netdevs to not do unnecessary things. >> >> Basic testing of 'PVP with OVS bonding on phy ports' scenario shows >> significant performance improvement. >> More accurate and intensive testing required. >> >> [1] [PATCH 0/6] netdev-dpdk: Use intermediate queue during packet >> transmission. >> https://mail.openvswitch.org/pipermail/ovs-dev/2017-June/334762.html >> >> Version 2: >> >> * Rebased on current master. >> * Added time based batching RFC patch. >> * Fixed mixing packets with different sources in same batch. >> > > Applied this series along with other patches[1] and gave initial try. > With this series, approximately half a million throughput drop is observed in > simple test case (P2P - 1stream - udp) vs master + [1]. Could you please describe packet sizes and the overall throughput of this scenario, because it's completely unclear what is the relative throughput difference. Also, was RFC patch applied too? I'm suggesting performance testing without this patch because it was only functionally tested (that is why it was sent as RFC). And your solution of netdev layer doesn't have such functionality right now, so it will be a little unfair if we'll try to compare them later. Beside that there are few possible performance optimizations that can be applied to current implementation of output batching. One of them is not to check boundaries each time we're trying to add packet to the batch. This happens now in each call of 'dp_packet_batch_add()'. We can manually add packets without checking because correctness assured by separate size check. Like this: ------------------------------------------------------------------------------- diff --git a/lib/dp-packet.h b/lib/dp-packet.h index c5fe32e..f624db7 100644 --- a/lib/dp-packet.h +++ b/lib/dp-packet.h @@ -655,11 +655,18 @@ dp_packet_batch_init(struct dp_packet_batch *batch) } static inline void +dp_packet_batch_add_unsafe(struct dp_packet_batch *batch, + struct dp_packet *packet) +{ + batch->packets[batch->count++] = packet; +} + +static inline void dp_packet_batch_add__(struct dp_packet_batch *batch, struct dp_packet *packet, size_t limit) { if (batch->count < limit) { - batch->packets[batch->count++] = packet; + dp_packet_batch_add_unsafe(batch, packet); } else { dp_packet_delete(packet); } @@ -734,7 +741,7 @@ dp_packet_batch_clone(struct dp_packet_batch *dst, dp_packet_batch_init(dst); DP_PACKET_BATCH_FOR_EACH (packet, src) { - dp_packet_batch_add(dst, dp_packet_clone(packet)); + dp_packet_batch_add_unsafe(dst, dp_packet_clone(packet)); } dst->trunc = src->trunc; } diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index e5f8a3d..f0c09d6 100644 --- a/lib/dpif-netdev.c +++ b/lib/dpif-netdev.c @@ -5166,7 +5166,7 @@ dp_execute_cb(void *aux_, struct dp_packet_batch *packets_, } DP_PACKET_BATCH_FOR_EACH (packet, packets_) { - dp_packet_batch_add(&p->output_pkts, packet); + dp_packet_batch_add_unsafe(&p->output_pkts, packet); } return; } ------------------------------------------------------------------------------- I will post dp_packet related part of this as a separate patch to clean up current direct usages of internal batch structure. > The performance improvement is observed with multiple flows (which this > series is meant to address). > > At this stage no latency settings were used. Yet to review and do more > testing. > > [1] Improves performance. > https://mail.openvswitch.org/pipermail/ovs-dev/2017-July/335359.html > https://mail.openvswitch.org/pipermail/ovs-dev/2017-July/336186.html > https://mail.openvswitch.org/pipermail/ovs-dev/2017-July/336187.html > https://mail.openvswitch.org/pipermail/ovs-dev/2017-July/336290.html > > - Bhanuprakash. > > > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
