[ovs-dev] V2: Hashing: Add truly symmetric L3+L4 fields option for multipath and bundle hashing

Ben Pfaff blp at nicira.com
Mon Jul 6 15:48:25 UTC 2015


On Sat, Jul 04, 2015 at 04:47:37PM -0500, Jeroen van Bemmel wrote:
> The symmetric_l4 function implements a hash over various fields
> including L2 fields such as ethernet source and destination MAC.
> Inspite of its name, there are situations in which this hash does not
> yield symmetric results ( e.g. when using VRRP, where the router
> receives packets on a virtual MAC but responds from its physical MAC )
> 
> This patch (v2) adds new 'symmetric_l3l4' and 'symmetric_l3l4+udp' functions
> using murmur3 hashing. They are identical, except that the latter also includes
> UDP src/dst ports in the hash.
> 
> Signed-off-by:Jeroen van Bemmel <jvb127 at gmail.com>
> Suggested-by: Ben Pfaff <blp at nicira.com>

Hi, thanks for the second version!

I can't get this patch to apply properly:
    fatal: corrupt patch at line 71
    Patch failed at 0001 V2: Hashing: Add truly symmetric L3+L4 fields option for multipath and bundle hashing

Usually this indicates that your mailer is word-wrapping lines or
inserting or removing whitespace.  Sometimes you can configure your
mailer not to do that, e.g. using the instructions at
	https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/plain/Documentation/email-clients.txt
There are other methods too.  You can use "git send-email" to email
patches (this is preferred).  You can submit a "pull-request" via the
Open vSwitch github project.  Finally, you can always generate a patch
with "git format-patch" and then attach it to an email.

I have some comments on the patch itself, see below.

Please don't insert a space after ( or before ), as I see done somewhat
consistently in this patch (see CodingStyle.md).

Please don't use // comments (see CodingStyle.md).

> +/* Hashes 'flow' based on its L3 through L4 protocol information */
> +uint32_t
> +flow_hash_symmetric_l3l4(const struct flow *flow, uint32_t basis,
> +                         int inc_udp_ports )

Please use 'bool' for inc_udp_ports.

In flow_mask_hash_fields() here, please always put {} around conditional
statements (see CodingStyle.md):
> +            } else break; // non-IP flow

Please use 'true' and 'false' for Booleans here:
> +    case NX_HASH_FIELDS_SYMMETRIC_L3L4:
> +        return flow_hash_symmetric_l3l4(flow, basis, 0);
> +
> +    case NX_HASH_FIELDS_SYMMETRIC_L3L4_UDP:
> +        return flow_hash_symmetric_l3l4(flow, basis, 1);
> +
>      }

Thanks,

Ben.



More information about the dev mailing list