[ovs-dev] [RFC] packets: Do not initialize ct_orig_tuple.

[Darrell Ball]

On 5/15/17, 6:00 AM, "ovs-dev-bounces at openvswitch.org on behalf of Bodireddy, Bhanuprakash" <ovs-dev-bounces at openvswitch.org on behalf of bhanuprakash.bodireddy at intel.com> wrote:

    >Commit daf4d3c18da4("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.
    >
    >CC: Jarno Rajahalme <jarno at ovn.org>
    >Signed-off-by: Daniele Di Proietto <diproiettod at vmware.com>
    >---
    >I'm sending this as an RFC because I didn't check very carefully if we can really
    >avoid initializing ct_orig_tuple.
    >
    >Maybe there are better solutions to this problem.
    >---
    > lib/packets.h | 2 +-
    > 1 file changed, 1 insertion(+), 1 deletion(-)
    >
    >diff --git a/lib/packets.h b/lib/packets.h index a5a483bc8..6f1791c7a 100644
    >--- a/lib/packets.h
    >+++ b/lib/packets.h
    >@@ -129,7 +129,7 @@ pkt_metadata_init(struct pkt_metadata *md,
    >odp_port_t port)
    >     /* 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));
    >+    memset(md, 0, offsetof(struct pkt_metadata, ct_orig_tuple));
    >     md->tunnel.ip_dst = 0;
    >     md->tunnel.ipv6_dst = in6addr_any;
    >
    
    It's been a while this RFC patch has been submitted to fix performance drop on Master. This indeed fixes the OvS-DPDK performance drop that was introduced by the conntrack commit.
    Is there a better fix than what is suggested above?  


This affects both kernel and userspace.
I tested this on both datapaths.
LGTM

Acked-by: Darrell Ball dlu998 at gmail.com





    
    - Bhanuprakash
    _______________________________________________
    dev mailing list
    dev at openvswitch.org
    https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=9pzwIiUqntRsGpAI2SFxxlP2mBBuQgQltn3COwYTT28&s=LQUeKvLUtcwi3hmXfVtufiOt0zWaZ5kBHa1aJLw7G9Q&e=