Andy, thanks for the review.
Guolin, are you willing to submit a v2?
Thanks,
Ben.
On Thu, Aug 29, 2013 at 05:21:03PM -0700, Andy Zhou wrote:
> Good catch. The logic looks good to me.
>
> When compiling, I got the following warnings:
>
> b/odp-util.c: In function ?odp_flow_key_from_flow__?:
> lib/odp-util.c:2559:17: error: comparison is always false due to limited
> range of data type [-Werror=type-limits]
> lib/odp-util.c:2560:31: error: comparison is always false due to limited
> range of data type [-Werror=type-limits]
>
> It would be nice to describe the bug explicitly in the commit message.
>
>
>
>
> On Thu, Aug 29, 2013 at 4:18 PM, Guolin Yang, VMware <gy...@nicira.com>wrote:
>
> > I found the isue happen for icmpv6 packet, that is why I read the code
> > and found the bug.
> >
> > Thanks
> > Guolin
> >
> > ---------- Forwarded message ----------
> > From: <gy...@nicira.com>
> > Date: Thu, Aug 29, 2013 at 10:35 AM
> > Subject: [PATCH] Fix a bug in conversion between flow/mask and flow key
> > To: d...@openvswitch.com
> > Cc: Guolin Yang <gy...@nicira.com>
> >
> >
> > From: Guolin Yang <gy...@nicira.com>
> >
> > For ICMPv6, when convert flow/mask to flow/mask key, the check
> > for whether there is ND information is incorrect. Similar error
> > in reverse conversion.
> >
> > Signed-off-by: Guolin Yang <gy...@nicira.com>
> > ---
> > lib/odp-util.c | 16 +++++++++++-----
> > 1 file changed, 11 insertions(+), 5 deletions(-)
> >
> > diff --git a/lib/odp-util.c b/lib/odp-util.c
> > index 15ad2be..6ddde9d 100644
> > --- a/lib/odp-util.c
> > +++ b/lib/odp-util.c
> > @@ -2553,8 +2553,12 @@ odp_flow_key_from_flow__(struct ofpbuf *buf, const
> > struct flow *data,
> > icmpv6_key->icmpv6_type = ntohs(data->tp_src);
> > icmpv6_key->icmpv6_code = ntohs(data->tp_dst);
> >
> > - if (icmpv6_key->icmpv6_type == ND_NEIGHBOR_SOLICIT
> > - || icmpv6_key->icmpv6_type == ND_NEIGHBOR_ADVERT) {
> > + if (flow->tp_dst == htons(0) &&
> > + (flow->tp_src == htons(ND_NEIGHBOR_SOLICIT) ||
> > + flow->tp_src == htons(ND_NEIGHBOR_ADVERT)) &&
> > + (!is_mask || (icmpv6_key->icmpv6_type == 0xffff &&
> > + icmpv6_key->icmpv6_code == 0xffff))) {
> > +
> > struct ovs_key_nd *nd_key;
> >
> > nd_key = nl_msg_put_unspec_uninit(buf, OVS_KEY_ATTR_ND,
> > @@ -2965,8 +2969,9 @@ parse_l2_5_onward(const struct nlattr
> > *attrs[OVS_KEY_ATTR_MAX + 1],
> > flow->tp_src = htons(icmpv6_key->icmpv6_type);
> > flow->tp_dst = htons(icmpv6_key->icmpv6_code);
> > expected_bit = OVS_KEY_ATTR_ICMPV6;
> > - if (src_flow->tp_src == htons(ND_NEIGHBOR_SOLICIT) ||
> > - src_flow->tp_src == htons(ND_NEIGHBOR_ADVERT)) {
> > + if (src_flow->tp_dst == 0 &&
> > + (src_flow->tp_src == htons(ND_NEIGHBOR_SOLICIT) ||
> > + src_flow->tp_src == htons(ND_NEIGHBOR_ADVERT))) {
> > if (!is_mask) {
> > expected_attrs |= UINT64_C(1) << OVS_KEY_ATTR_ND;
> > }
> > @@ -2981,7 +2986,8 @@ parse_l2_5_onward(const struct nlattr
> > *attrs[OVS_KEY_ATTR_MAX + 1],
> > if (is_mask) {
> > if (!is_all_zeros((const uint8_t *) nd_key,
> > sizeof *nd_key) &&
> > - flow->tp_src != htons(0xffff)) {
> > + (flow->tp_src != htons(0xffff) &&
> > + flow->tp_dst != htons(0xffff))) {
> > return ODP_FIT_ERROR;
> > } else {
> > expected_attrs |= UINT64_C(1) <<
> > OVS_KEY_ATTR_ND;
> > --
> > 1.7.9.5
> >
> >
> >
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
