| Message ID | 1501007967-75916-1-git-send-email-azhou@ovn.org |
|---|---|
| State | Accepted |
| Headers | show |
On 25.07.2017 21:39, Andy Zhou wrote: > Commit 42781e77035d (bond: Unify hash functions in hash action and entry > lookup.) changed the BM_TCP's hash function, but did not update > hash mask fields accordingly. Found by inspection. > > CC: Ilya Maximets <i.maximets@samsung.com> > Signed-off-by: Andy Zhou <azhou@ovn.org> > --- Good catch. I forget about this. One minor comment below, but it's not important. Acked-by: Ilya Maximets <i.maximets@samsung.com> > ofproto/bond.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/ofproto/bond.c b/ofproto/bond.c > index e09136efbd98..7d8d6560c690 100644 > --- a/ofproto/bond.c > +++ b/ofproto/bond.c > @@ -1798,11 +1798,12 @@ choose_output_slave(const struct bond *bond, const struct flow *flow, > return NULL; > } > if (wc) { > - flow_mask_hash_fields(flow, wc, NX_HASH_FIELDS_SYMMETRIC_L4); > + flow_mask_hash_fields(flow, wc, > + NX_HASH_FIELDS_SYMMETRIC_L3L4_UDP); Above function call, actually, fits in 79 characters limit and can be written in one line. > } > /* Fall Through. */ > case BM_SLB: > - if (wc) { > + if (wc && balance == BM_SLB) { > flow_mask_hash_fields(flow, wc, NX_HASH_FIELDS_ETH_SRC); > } > e = lookup_bond_entry(bond, flow, vlan); >
On Wed, Jul 26, 2017 at 12:06 AM, Ilya Maximets <i.maximets@samsung.com> wrote: > On 25.07.2017 21:39, Andy Zhou wrote: >> Commit 42781e77035d (bond: Unify hash functions in hash action and entry >> lookup.) changed the BM_TCP's hash function, but did not update >> hash mask fields accordingly. Found by inspection. >> >> CC: Ilya Maximets <i.maximets@samsung.com> >> Signed-off-by: Andy Zhou <azhou@ovn.org> >> --- > > Good catch. I forget about this. > One minor comment below, but it's not important. > > Acked-by: Ilya Maximets <i.maximets@samsung.com> Thanks for the review. Pushed. >> + flow_mask_hash_fields(flow, wc, >> + NX_HASH_FIELDS_SYMMETRIC_L3L4_UDP); > > Above function call, actually, fits in 79 characters limit and can be > written in one line. > You are right. Thanks. I merged those two lines before pushing.
diff --git a/ofproto/bond.c b/ofproto/bond.c index e09136efbd98..7d8d6560c690 100644 --- a/ofproto/bond.c +++ b/ofproto/bond.c @@ -1798,11 +1798,12 @@ choose_output_slave(const struct bond *bond, const struct flow *flow, return NULL; } if (wc) { - flow_mask_hash_fields(flow, wc, NX_HASH_FIELDS_SYMMETRIC_L4); + flow_mask_hash_fields(flow, wc, + NX_HASH_FIELDS_SYMMETRIC_L3L4_UDP); } /* Fall Through. */ case BM_SLB: - if (wc) { + if (wc && balance == BM_SLB) { flow_mask_hash_fields(flow, wc, NX_HASH_FIELDS_ETH_SRC); } e = lookup_bond_entry(bond, flow, vlan);
Commit 42781e77035d (bond: Unify hash functions in hash action and entry lookup.) changed the BM_TCP's hash function, but did not update hash mask fields accordingly. Found by inspection. CC: Ilya Maximets <i.maximets@samsung.com> Signed-off-by: Andy Zhou <azhou@ovn.org> --- ofproto/bond.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)