On 16 May 2017 at 21:01, Darrell Ball <[email protected]> wrote:
>
>
> On 5/15/17, 6:00 AM, "[email protected] on behalf of Bodireddy, 
> Bhanuprakash" <[email protected] on behalf of 
> [email protected]> 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 <[email protected]>
>     >Signed-off-by: Daniele Di Proietto <[email protected]>
>     >---
>     >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

How do we make sure that ct_orig_tuple isn't used uninitialized? Do we
need to zero out the ct_orig_tuple proto?
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to