| Message ID | 1536731757-76774-1-git-send-email-martinxu9.ovs@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ovs-dev,v1] bundle: add symmetric_l3 hash method for multipath | expand |
Bleep bloop. Greetings Martin Xu, I am a robot and I have tried out your patch.
Thanks for your contribution.
I encountered some error that I wasn't expecting. See the details below.
checkpatch:
WARNING: Line lacks whitespace around operator
WARNING: Line lacks whitespace around operator
#79 FILE: lib/flow.c:2325:
for (i=0; i<16; i++) {
Lines checked: 162, Warnings: 2, Errors: 0
build:
/bin/sh ./libtool --tag=CC --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Wno-null-pointer-arithmetic -Werror -Werror -g -O2 -MT lib/entropy.lo -MD -MP -MF $depbase.Tpo -c -o lib/entropy.lo lib/entropy.c &&\
mv -f $depbase.Tpo $depbase.Plo
libtool: compile: gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Wno-null-pointer-arithmetic -Werror -Werror -g -O2 -MT lib/entropy.lo -MD -MP -MF lib/.deps/entropy.Tpo -c lib/entropy.c -o lib/entropy.o
depbase=`echo lib/fat-rwlock.lo | sed 's|[^/]*$|.deps/&|;s|\.lo$||'`;\
/bin/sh ./libtool --tag=CC --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Wno-null-pointer-arithmetic -Werror -Werror -g -O2 -MT lib/fat-rwlock.lo -MD -MP -MF $depbase.Tpo -c -o lib/fat-rwlock.lo lib/fat-rwlock.c &&\
mv -f $depbase.Tpo $depbase.Plo
libtool: compile: gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Wno-null-pointer-arithmetic -Werror -Werror -g -O2 -MT lib/fat-rwlock.lo -MD -MP -MF lib/.deps/fat-rwlock.Tpo -c lib/fat-rwlock.c -o lib/fat-rwlock.o
depbase=`echo lib/fatal-signal.lo | sed 's|[^/]*$|.deps/&|;s|\.lo$||'`;\
/bin/sh ./libtool --tag=CC --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Wno-null-pointer-arithmetic -Werror -Werror -g -O2 -MT lib/fatal-signal.lo -MD -MP -MF $depbase.Tpo -c -o lib/fatal-signal.lo lib/fatal-signal.c &&\
mv -f $depbase.Tpo $depbase.Plo
libtool: compile: gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Wno-null-pointer-arithmetic -Werror -Werror -g -O2 -MT lib/fatal-signal.lo -MD -MP -MF lib/.deps/fatal-signal.Tpo -c lib/fatal-signal.c -o lib/fatal-signal.o
depbase=`echo lib/flow.lo | sed 's|[^/]*$|.deps/&|;s|\.lo$||'`;\
/bin/sh ./libtool --tag=CC --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Wno-null-pointer-arithmetic -Werror -Werror -g -O2 -MT lib/flow.lo -MD -MP -MF $depbase.Tpo -c -o lib/flow.lo lib/flow.c &&\
mv -f $depbase.Tpo $depbase.Plo
libtool: compile: gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Wno-null-pointer-arithmetic -Werror -Werror -g -O2 -MT lib/flow.lo -MD -MP -MF lib/.deps/flow.Tpo -c lib/flow.c -o lib/flow.o
lib/flow.c:2306:1: error: no previous prototype for 'flow_hash_symmetric_l3' [-Werror=missing-prototypes]
flow_hash_symmetric_l3(const struct flow *flow, uint32_t basis)
^
cc1: error: unrecognized command line option "-Wno-null-pointer-arithmetic" [-Werror]
cc1: all warnings being treated as errors
make[2]: *** [lib/flow.lo] Error 1
make[2]: Leaving directory `/var/lib/jenkins/jobs/upstream_build_from_pw/workspace'
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory `/var/lib/jenkins/jobs/upstream_build_from_pw/workspace'
make: *** [all] Error 2
Please check this out. If you feel there has been an error, please email aconole@bytheb.org
Thanks,
0-day Robot
diff --git a/include/openflow/nicira-ext.h b/include/openflow/nicira-ext.h index 1f5cbbf..dc12101 100644 --- a/include/openflow/nicira-ext.h +++ b/include/openflow/nicira-ext.h @@ -109,8 +109,10 @@ enum nx_hash_fields { NX_HASH_FIELDS_NW_SRC, /* Network destination address (NXM_OF_IP_DST) only. */ - NX_HASH_FIELDS_NW_DST + NX_HASH_FIELDS_NW_DST, + /* Both network destination and source destination addresses. */ + NX_HASH_FIELDS_SYMMETRIC_L3 }; /* NXT_PACKET_IN (analogous to OFPT_PACKET_IN). diff --git a/lib/bundle.c b/lib/bundle.c index edf6676..558e890 100644 --- a/lib/bundle.c +++ b/lib/bundle.c @@ -198,6 +198,8 @@ bundle_parse__(const char *s, const struct ofputil_port_map *port_map, bundle->fields = NX_HASH_FIELDS_NW_SRC; } else if (!strcasecmp(fields, "nw_dst")) { bundle->fields = NX_HASH_FIELDS_NW_DST; + } else if (!strcasecmp(fields, "symmetric_l3")) { + bundle->fields = NX_HASH_FIELDS_SYMMETRIC_L3; } else { return xasprintf("%s: unknown fields `%s'", s, fields); } diff --git a/lib/flow.c b/lib/flow.c index 77ed3d9..0a9d8f5 100644 --- a/lib/flow.c +++ b/lib/flow.c @@ -2301,6 +2301,34 @@ flow_hash_symmetric_l3l4(const struct flow *flow, uint32_t basis, return hash_finish(hash, basis); } +/* Hashes 'flow' based on its nw_dst and nw_src for multipath. */ +uint32_t +flow_hash_symmetric_l3(const struct flow *flow, uint32_t basis) +{ + struct { + union { + ovs_be32 ipv4_addr; + struct in6_addr ipv6_addr; + }; + } fields; + + int i; + + memset(&fields, 0, sizeof fields); + if (flow->dl_type == htons(ETH_TYPE_IP)) { + fields.ipv4_addr = flow->nw_src ^ flow->nw_dst; + } else if (flow->dl_type == htons(ETH_TYPE_IPV6)) { + const uint8_t *a = &flow->ipv6_src.s6_addr[0]; + const uint8_t *b = &flow->ipv6_dst.s6_addr[0]; + uint8_t *ipv6_addr = &fields.ipv6_addr.s6_addr[0]; + + for (i=0; i<16; i++) { + ipv6_addr[i] = a[i] ^ b[i]; + } + } + return jhash_bytes(&fields, sizeof fields, basis); +} + /* Initialize a flow with random fields that matter for nx_hash_fields. */ void flow_random_hash_fields(struct flow *flow) @@ -2416,6 +2444,16 @@ flow_mask_hash_fields(const struct flow *flow, struct flow_wildcards *wc, } break; + case NX_HASH_FIELDS_SYMMETRIC_L3: + if (flow->dl_type == htons(ETH_TYPE_IP)) { + memset(&wc->masks.nw_src, 0xff, sizeof wc->masks.nw_src); + memset(&wc->masks.nw_dst, 0xff, sizeof wc->masks.nw_dst); + } else if (flow->dl_type == htons(ETH_TYPE_IPV6)) { + memset(&wc->masks.ipv6_src, 0xff, sizeof wc->masks.ipv6_src); + memset(&wc->masks.ipv6_dst, 0xff, sizeof wc->masks.ipv6_dst); + } + break; + default: OVS_NOT_REACHED(); } @@ -2458,6 +2496,8 @@ flow_hash_fields(const struct flow *flow, enum nx_hash_fields fields, return basis; } + case NX_HASH_FIELDS_SYMMETRIC_L3: + return flow_hash_symmetric_l3(flow, basis); } OVS_NOT_REACHED(); @@ -2474,6 +2514,7 @@ flow_hash_fields_to_str(enum nx_hash_fields fields) case NX_HASH_FIELDS_SYMMETRIC_L3L4_UDP: return "symmetric_l3l4+udp"; case NX_HASH_FIELDS_NW_SRC: return "nw_src"; case NX_HASH_FIELDS_NW_DST: return "nw_dst"; + case NX_HASH_FIELDS_SYMMETRIC_L3: return "symmetric_l3"; default: return "<unknown>"; } } @@ -2487,7 +2528,8 @@ flow_hash_fields_valid(enum nx_hash_fields fields) || fields == NX_HASH_FIELDS_SYMMETRIC_L3L4 || fields == NX_HASH_FIELDS_SYMMETRIC_L3L4_UDP || fields == NX_HASH_FIELDS_NW_SRC - || fields == NX_HASH_FIELDS_NW_DST; + || fields == NX_HASH_FIELDS_NW_DST + || fields == NX_HASH_FIELDS_SYMMETRIC_L3; } /* Returns a hash value for the bits of 'flow' that are active based on diff --git a/lib/multipath.c b/lib/multipath.c index 659c0bd..e6ba4c6 100644 --- a/lib/multipath.c +++ b/lib/multipath.c @@ -171,6 +171,8 @@ multipath_parse__(struct ofpact_multipath *mp, const char *s_, char *s) mp->fields = NX_HASH_FIELDS_NW_SRC; } else if (!strcasecmp(fields, "nw_dst")) { mp->fields = NX_HASH_FIELDS_NW_DST; + } else if (!strcasecmp(fields, "symmetric_l3")) { + mp->fields = NX_HASH_FIELDS_SYMMETRIC_L3; } else { return xasprintf("%s: unknown fields `%s'", s_, fields); } diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in index 4850f4a..e93ce59 100644 --- a/utilities/ovs-ofctl.8.in +++ b/utilities/ovs-ofctl.8.in @@ -1421,6 +1421,8 @@ and IPv6, which hash to a constant zero. Like \fBsymmetric_l3l4+udp\fR, but UDP ports are included in the hash. This is a more effective hash when asymmetric UDP protocols such as VXLAN are not a consideration. +.IP \fBsymmetric_l3\fR +Hashes Network source address and Network destination address. .IP \fBnw_src\fR Hashes Network source address only. .IP \fBnw_dst\fR
Add a symmetric_l3 hash method that uses both network destination address and network source address. VMware-BZ: #2112940 Signed-off-by: Martin Xu <martinxu9.ovs@gmail.com> CC: Ben Pfaff <blp@ovn.org> --- include/openflow/nicira-ext.h | 4 +++- lib/bundle.c | 2 ++ lib/flow.c | 44 ++++++++++++++++++++++++++++++++++++++++++- lib/multipath.c | 2 ++ utilities/ovs-ofctl.8.in | 2 ++ 5 files changed, 52 insertions(+), 2 deletions(-)