On Thu, Oct 02, 2014 at 09:20:32AM -0700, Jarno Rajahalme wrote:
>
> On Sep 26, 2014, at 11:33 AM, Ben Pfaff <b...@nicira.com> wrote:
>
> > On Wed, Sep 24, 2014 at 11:24:01AM -0700, Jarno Rajahalme wrote:
> >> Earlier I have seen the frags checking suspiciously high in perf
> >> reports, but did not understand why. Maybe this explains it:
> >>
> >> Previously we checked frags handling before each flow table lookup.
> >> As the nw_frags field is not writeable, it suffices to check them
> >> once, before the first table lookup. Also, ofproto-dpif-xlate already
> >> has code for this, but it was run after the first table lookup. This
> >> check is now done only once, before the first table lookup.
> >>
> >> Signed-off-by: Jarno Rajahalme <jrajaha...@nicira.com>
> >
> > By "frags checking suspiciously high in perf reports", do you mean
> > these deleted lines of code in rule_dpif_lookup_in_table() were
> > showing up high in perf reports?
> >
> > - if (ofproto->up.frag_handling != OFPC_FRAG_NX_MATCH) {
> > - /* We always unwildcard dl_type and nw_frag (for IP), so they
> > - * need not be unwildcarded here. */
> > -
> > - if (flow->nw_frag & FLOW_NW_FRAG_ANY) {
> >
>
> Yes they did. I could make it a bit better just by changing the
> order of the checks, checking flow->nw_frag first, and
> ofproto->ip.frag_handling later, so it must have been a cache miss
> on ofproto->up.frag_handling.
> This was already some months ago, and I have not repeated the tests recently.
>
> > I'm really surprised to hear that.
> >
>
> To be clear, it was in the range of < 1%, so it was not that high,
> but I was surprised to see it there at all. Maybe I should just
> remove the word ?high? from the commit message?
That would help.
Was the improvement about 1% also?
The new code is at least a little harder to understand. I am trying
to figure out whether the performance improvement outweighs that cost.
What is your opinion?
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
