I tested and found similar ballpark performance increase (approx. 10%) in the most simple case with decreasing benefit as the pipeline gets more realistic and useful.
This ideal case incremental beyond the original fix patch (from Daniele) shows a small decrease in performance of approx 100k pps (0.7 %). I cannot explain that right now. However, I think there is advantage in clearly defining what needs initialization and was does not and the additional incremental by Bhanu does that. In realistic cases, I expect the 0.7 % difference, if it is real, to be much less anyways. Hence, this patch looks fine except for some comments inline. On 6/22/17, 2:08 PM, "[email protected] on behalf of Bhanuprakash Bodireddy" <[email protected] on behalf of [email protected]> wrote: Commit "odp: Support conntrack orig tuple key." introduced new fields in struct 'pkt_metadata'. pkt_metadata_init() is called for every packet in the userspace datapath. When testing a simple single flow case with DPDK, we observe a lower throughput after the above commit (it was 14.88 Mpps before, it is 13 Mpps after). This patch skips initializing ct_orig_tuple in pkt_metadata_init(). It should be enough to initialize ct_state, because nobody should look at ct_orig_tuple unless ct_state is != 0. It's discussed at: https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_pipermail_ovs-2Ddev_2017-2DMay_332419.html&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=8ZtjBQ0W_pVzaasUyrsZM_cp_rmDylXtUvA7D7F2UOw&s=min0TAxgYGI118BI-LEDaDap8G8XuU4C9xJXtLLqIaE&e= Fixes: daf4d3c18da4("odp: Support conntrack orig tuple key.") Signed-off-by: Daniele Di Proietto <[email protected]> Signed-off-by: Bhanuprakash Bodireddy <[email protected]> Co-authored-by: Bhanuprakash Bodireddy <[email protected]> --- Original RFC was posted by Daniele here: https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_pipermail_ovs-2Ddev_2017-2DMarch_329679.html&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=8ZtjBQ0W_pVzaasUyrsZM_cp_rmDylXtUvA7D7F2UOw&s=QivDZb3mJt6__TUIssDosbGfZlpAtRN7P_4p-lc4Zls&e= In this patch moved the offset from ct_orig_tuple to 'ct_orig_tuple_ipv6'. This patch fixes the performance drop(~2.3Mpps for P2P - 64 byte pkts) with OvS-DPDK on Master. lib/packets.h | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/lib/packets.h b/lib/packets.h index a9d5e84..94c3dcc 100644 --- a/lib/packets.h +++ b/lib/packets.h @@ -126,10 +126,19 @@ pkt_metadata_init_tnl(struct pkt_metadata *md) static inline void pkt_metadata_init(struct pkt_metadata *md, odp_port_t port) { + /* This is called for every packet in userspace datapath and affects + * performance if all the metadata is initialized. I think you meant the sentence “Hence absolutely + * necessary fields should be zeroed out.” to be something like “Hence, fields should only be zeroed out when necessary.” + * + * Initialize only the first 17 bytes of metadata (till ct_state). Do we really need to discuss “17 bytes” ? Can the sentence be: Initialize only till ct_state. + * Once the ct_state is zeroed out rest of ct fields will not be looked + * at unless ct_state != 0. + */ + memset(md, 0, offsetof(struct pkt_metadata, ct_orig_tuple_ipv6)); + /* It can be expensive to zero out all of the tunnel metadata. However, * we can just zero out ip_dst and the rest of the data will never be * looked at. */ - memset(md, 0, offsetof(struct pkt_metadata, in_port)); md->tunnel.ip_dst = 0; md->tunnel.ipv6_dst = in6addr_any; md->in_port.odp_port = port; -- 2.4.11 _______________________________________________ dev mailing list [email protected] https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=8ZtjBQ0W_pVzaasUyrsZM_cp_rmDylXtUvA7D7F2UOw&s=pGKK0PBsxExr3Bxhim-OKiWgLg3rhYtj2MqiosQ2Rfc&e= _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
