netdev - Re: [ovs-dev] [PATCH net-next v6 05/10] net: openvswitch: optimize flow-mask looking up

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 4 Nov 2019 05:59:27 -0800
From:   William Tu <u9012063@...il.com>
To:     Pravin Shelar <pshelar@....org>
Cc:     Tonghao Zhang <xiangxia.m.yue@...il.com>,
        ovs dev <dev@...nvswitch.org>,
        "David S. Miller" <davem@...emloft.net>,
        Linux Kernel Network Developers <netdev@...r.kernel.org>
Subject: Re: [ovs-dev] [PATCH net-next v6 05/10] net: openvswitch: optimize
 flow-mask looking up

On Sat, Nov 2, 2019 at 11:50 PM Pravin Shelar <pshelar@....org> wrote:
>
> On Fri, Nov 1, 2019 at 7:24 AM <xiangxia.m.yue@...il.com> wrote:
> >
> > From: Tonghao Zhang <xiangxia.m.yue@...il.com>
> >
> > The full looking up on flow table traverses all mask array.
> > If mask-array is too large, the number of invalid flow-mask
> > increase, performance will be drop.
> >
> > One bad case, for example: M means flow-mask is valid and NULL
> > of flow-mask means deleted.
> >
> > +-------------------------------------------+
> > | M | NULL | ...                  | NULL | M|
> > +-------------------------------------------+
> >
> > In that case, without this patch, openvswitch will traverses all
> > mask array, because there will be one flow-mask in the tail. This
> > patch changes the way of flow-mask inserting and deleting, and the
> > mask array will be keep as below: there is not a NULL hole. In the
> > fast path, we can "break" "for" (not "continue") in flow_lookup
> > when we get a NULL flow-mask.
> >
> >          "break"
> >             v
> > +-------------------------------------------+
> > | M | M |  NULL |...           | NULL | NULL|
> > +-------------------------------------------+
> >
> > This patch don't optimize slow or control path, still using ma->max
> > to traverse. Slow path:
> > * tbl_mask_array_realloc
> > * ovs_flow_tbl_lookup_exact
> > * flow_mask_find
> >
> > Signed-off-by: Tonghao Zhang <xiangxia.m.yue@...il.com>
> > Tested-by: Greg Rose <gvrose8192@...il.com>
> > ---
> Acked-by: Pravin B Shelar <pshelar@....org>

Nack to this patch.

It makes the mask cache invalid when moving the flow mask
to fill another hole.
And the penalty for miss the mask cache is larger than the
benefit of this patch (avoiding the NULL flow-mask).

William