[ovs-dev] [PATCH v3] dpcls: Avoid one 8-byte chunk in subtable mask.

Fischetti, Antonio antonio.fischetti at intel.com
Tue Jan 10 10:50:01 UTC 2017


Hi Jarno,
I've run make check with and without this patch and I got the same results.

For both cases the 'dpif-netdev' and 'flow classifier' unit-tests were ok.
For both cases I got the same failures on

977: PMD - stats                                     FAILED (pmd.at:198)

and many IPv6 tests like

1095: ofproto-dpif - sFlow packet sampling - IPv6 collector FAILED (ofproto-dpif.at:5769)

1134: ofproto-dpif megaflow - netflow - IPv6 collector FAILED (ofproto-dpif.at:7319)
...

Thanks,
Antonio

> -----Original Message-----
> From: Fischetti, Antonio
> Sent: Friday, January 6, 2017 9:45 AM
> To: dev at openvswitch.org
> Cc: jarno at ovn.org; Bodireddy, Bhanuprakash
> <bhanuprakash.bodireddy at intel.com>; Fischetti, Antonio
> <antonio.fischetti at intel.com>
> Subject: [PATCH v3] dpcls: Avoid one 8-byte chunk in subtable mask.
> 
> This patch allows to skip the chunk comprising of dp_hash and in_port
> in the subtable mask when the packet is not recirculated.  This will
> slightly speed up the hash computation as one expensive function call
> to hash_add64() can be skipped.
> For each new netdev flow we wildcard in_port in the mask, so in the
> physical case where dp_hash is also wildcarded, the resulting 8-byte
> chunk will not be part of the subtable mask.
> 
> The idea behind is that all the packets within a given dpcls will
> have the same in_port value and typically dp_hash == 0.  So they will
> have the same 8-bytes chunk in their {dp_hash, in_port} portion of the
> miniflow. This doesn't contribute effectively in spreading hash values
> and avoiding collisions.
> 
> v2: Using ovs_assert.
> v3: Fix for dumped flows missing the in_port match.
> 
> Signed-off-by: Antonio Fischetti <antonio.fischetti at intel.com>
> Signed-off-by: Bhanuprakash Bodireddy <bhanuprakash.bodireddy at intel.com>
> Co-authored-by: Bhanuprakash Bodireddy <bhanuprakash.bodireddy at intel.com>
> Signed-off-by: Jarno Rajahalme <jarno at ovn.org>
> Co-authored-by: Jarno Rajahalme <jarno at ovn.org>
> ---
>  lib/dpif-netdev.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 1a47a45..4425f85 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -2120,6 +2120,9 @@ dp_netdev_flow_to_dpif_flow(const struct
> dp_netdev_flow *netdev_flow,
>          };
> 
>          miniflow_expand(&netdev_flow->cr.mask->mf, &wc.masks);
> +        /* in_port is exact matched, but we have left it out from the
> mask for
> +         * optimization reasons.  Add in_port back to the mask. */
> +        wc.masks.in_port.odp_port = ODPP_NONE;
> 
>          /* Key */
>          offset = key_buf->size;
> @@ -2278,7 +2281,17 @@ dp_netdev_flow_add(struct dp_netdev_pmd_thread
> *pmd,
>      struct dpcls *cls;
>      odp_port_t in_port = match->flow.in_port.odp_port;
> 
> +    /* As we select the dpcls based on the port number, each netdev flow
> +     * belonging to the same dpcls will have the same odp_port value.
> +     * For performance reasons here we wildcard odp_port in the mask.  In
> the
> +     * physical case where dp_hash is also wildcarded, the resulting 8-
> byte
> +     * chunk {dp_hash, in_port} will be ignored by
> netdev_flow_mask_init() and
> +     * will not be part of the subtable mask.
> +     * This will speed up the hash computation during dpcls_lookup()
> because
> +     * one call to hash_add64() will be skipped. */
> +    match->wc.masks.in_port.odp_port = 0;
>      netdev_flow_mask_init(&mask, match);
> +    match->wc.masks.in_port.odp_port = ODPP_NONE;
>      /* Make sure wc does not have metadata. */
>      ovs_assert(!FLOWMAP_HAS_FIELD(&mask.mf.map, metadata)
>                 && !FLOWMAP_HAS_FIELD(&mask.mf.map, regs));
> --
> 2.4.11



More information about the dev mailing list