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
