I will send a v2 soon based on Andy's feedback.
Thanks
Guolin
On Thu, Aug 29, 2013 at 8:43 PM, Ben Pfaff <b...@nicira.com> wrote:
> 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
