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

2017-01-10 Thread Jarno Rajahalme
Thanks for the update! Pushed to master with small edits for clarification.

  Jarno


> On Jan 6, 2017, at 1:45 AM, antonio.fische...@intel.com wrote:
> 
> 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 
> Signed-off-by: Bhanuprakash Bodireddy 
> Co-authored-by: Bhanuprakash Bodireddy 
> Signed-off-by: Jarno Rajahalme 
> Co-authored-by: Jarno Rajahalme 
> ---
> 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(_flow->cr.mask->mf, );
> +/* 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(, match);
> +match->wc.masks.in_port.odp_port = ODPP_NONE;
> /* Make sure wc does not have metadata. */
> ovs_assert(!FLOWMAP_HAS_FIELD(, metadata)
>&& !FLOWMAP_HAS_FIELD(, regs));
> -- 
> 2.4.11
> 

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


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

2017-01-06 Thread antonio . fischetti
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 
Signed-off-by: Bhanuprakash Bodireddy 
Co-authored-by: Bhanuprakash Bodireddy 
Signed-off-by: Jarno Rajahalme 
Co-authored-by: Jarno Rajahalme 
---
 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(_flow->cr.mask->mf, );
+/* 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(, match);
+match->wc.masks.in_port.odp_port = ODPP_NONE;
 /* Make sure wc does not have metadata. */
 ovs_assert(!FLOWMAP_HAS_FIELD(, metadata)
&& !FLOWMAP_HAS_FIELD(, regs));
-- 
2.4.11

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev