On 5/17/17, 2:19 PM, "Joe Stringer" <[email protected]> wrote:
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?
There are a couple places where we explicitly set or reset the pkt metadata
ct_orig_tuple;
one is in pkt_metadata_from_flow().
I know there is a check for ct_orig_tuple proto in odp_key_from_pkt_metadata()
Can you find a case where can run this code without a set or reset of
ct_orig_tuple pkt md
or you are not sure ?
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
