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

William Tu u9012063 at gmail.com
Mon Nov 4 22:20:50 UTC 2019


On Mon, Nov 4, 2019 at 2:10 PM Pravin Shelar <pshelar at ovn.org> wrote:
>
> On Mon, Nov 4, 2019 at 6:00 AM William Tu <u9012063 at gmail.com> wrote:
> >
> > On Sat, Nov 2, 2019 at 11:50 PM Pravin Shelar <pshelar at ovn.org> wrote:
> > >
> > > On Fri, Nov 1, 2019 at 7:24 AM <xiangxia.m.yue at gmail.com> wrote:
> > > >
> > > > From: Tonghao Zhang <xiangxia.m.yue at gmail.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 at gmail.com>
> > > > Tested-by: Greg Rose <gvrose8192 at gmail.com>
> > > > ---
> > > Acked-by: Pravin B Shelar <pshelar at ovn.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).
> >
> Hi William,
>
> The cache invalidation would occur in case of changes to flow mask
> list. I think in general datapath processing there should not be too
> many changes to mask list since a mask is typically shared between
> multiple mega flows. Flow-table changes does not always result in mask
> list updates. In last round Tonghao has posted number to to support
> this patch.
> If you are worried about some other use case can you provide
> performance numbers, that way we can take this discussion forward.
>
I see, I don't have any use case to provide.
Thanks,
William


More information about the dev mailing list