IGMP translations wasn't setting enough bits in the wildcards to ensure different packets were handled differently.
Reported-by: "O'Reilly, Darragh" <darragh.orei...@hpe.com> Reported-at: http://openvswitch.org/pipermail/discuss/2016-April/021036.html Signed-off-by: Ben Pfaff <b...@ovn.org> --- AUTHORS | 1 + lib/flow.h | 69 ++++++++++++++++++++++++++++++++------------ lib/meta-flow.c | 10 +++---- lib/nx-match.c | 4 +-- lib/odp-util.c | 4 +-- ofproto/ofproto-dpif-xlate.c | 28 +++++++++++++----- 6 files changed, 80 insertions(+), 36 deletions(-) diff --git a/AUTHORS b/AUTHORS index cc82388..22f527c 100644 --- a/AUTHORS +++ b/AUTHORS @@ -292,6 +292,7 @@ Christian Stigen Larsen cslar...@gmail.com Christopher Paggen cpag...@cisco.com Chunhe Li lichu...@huawei.com Daniel Badea daniel.ba...@windriver.com +Darragh O'Reilly darragh.orei...@hpe.com Dave Walker davewal...@ubuntu.com David Evans davidjoshuaev...@gmail.com David Palma pa...@onesource.pt diff --git a/lib/flow.h b/lib/flow.h index 0196ee7..22b245c 100644 --- a/lib/flow.h +++ b/lib/flow.h @@ -832,41 +832,72 @@ static inline bool is_ip_any(const struct flow *flow) return dl_type_is_ip_any(flow->dl_type); } -static inline bool is_icmpv4(const struct flow *flow) +static inline bool is_icmpv4(const struct flow *flow, + struct flow_wildcards *wc) { - return (flow->dl_type == htons(ETH_TYPE_IP) - && flow->nw_proto == IPPROTO_ICMP); + if (flow->dl_type == htons(ETH_TYPE_IP)) { + if (wc) { + memset(&wc->masks.nw_proto, 0xff, sizeof wc->masks.nw_proto); + } + return flow->nw_proto == IPPROTO_ICMP; + } + return false; } -static inline bool is_icmpv6(const struct flow *flow) +static inline bool is_icmpv6(const struct flow *flow, + struct flow_wildcards *wc) { - return (flow->dl_type == htons(ETH_TYPE_IPV6) - && flow->nw_proto == IPPROTO_ICMPV6); + if (flow->dl_type == htons(ETH_TYPE_IPV6)) { + if (wc) { + memset(&wc->masks.nw_proto, 0xff, sizeof wc->masks.nw_proto); + } + return flow->nw_proto == IPPROTO_ICMPV6; + } + return false; } -static inline bool is_igmp(const struct flow *flow) +static inline bool is_igmp(const struct flow *flow, struct flow_wildcards *wc) { - return (flow->dl_type == htons(ETH_TYPE_IP) - && flow->nw_proto == IPPROTO_IGMP); + if (flow->dl_type == htons(ETH_TYPE_IP)) { + if (wc) { + memset(&wc->masks.nw_proto, 0xff, sizeof wc->masks.nw_proto); + } + return flow->nw_proto == IPPROTO_IGMP; + } + return false; } -static inline bool is_mld(const struct flow *flow) +static inline bool is_mld(const struct flow *flow, + struct flow_wildcards *wc) { - return is_icmpv6(flow) - && (flow->tp_src == htons(MLD_QUERY) - || flow->tp_src == htons(MLD_REPORT) - || flow->tp_src == htons(MLD_DONE) - || flow->tp_src == htons(MLD2_REPORT)); + if (is_icmpv6(flow, wc)) { + if (wc) { + memset(&wc->masks.tp_src, 0xff, sizeof wc->masks.tp_src); + } + return (flow->tp_src == htons(MLD_QUERY) + || flow->tp_src == htons(MLD_REPORT) + || flow->tp_src == htons(MLD_DONE) + || flow->tp_src == htons(MLD2_REPORT)); + } + return false; } -static inline bool is_mld_query(const struct flow *flow) +static inline bool is_mld_query(const struct flow *flow, + struct flow_wildcards *wc) { - return is_icmpv6(flow) && flow->tp_src == htons(MLD_QUERY); + if (is_icmpv6(flow, wc)) { + if (wc) { + memset(&wc->masks.tp_src, 0xff, sizeof wc->masks.tp_src); + } + return flow->tp_src == htons(MLD_QUERY); + } + return false; } -static inline bool is_mld_report(const struct flow *flow) +static inline bool is_mld_report(const struct flow *flow, + struct flow_wildcards *wc) { - return is_mld(flow) && !is_mld_query(flow); + return is_mld(flow, wc) && !is_mld_query(flow, wc); } static inline bool is_stp(const struct flow *flow) diff --git a/lib/meta-flow.c b/lib/meta-flow.c index ce60f22..a23c01f 100644 --- a/lib/meta-flow.c +++ b/lib/meta-flow.c @@ -393,21 +393,21 @@ mf_are_prereqs_ok(const struct mf_field *mf, const struct flow *flow) return is_ip_any(flow) && flow->nw_proto == IPPROTO_SCTP && !(flow->nw_frag & FLOW_NW_FRAG_LATER); case MFP_ICMPV4: - return is_icmpv4(flow); + return is_icmpv4(flow, NULL); case MFP_ICMPV6: - return is_icmpv6(flow); + return is_icmpv6(flow, NULL); case MFP_ND: - return (is_icmpv6(flow) + return (is_icmpv6(flow, NULL) && flow->tp_dst == htons(0) && (flow->tp_src == htons(ND_NEIGHBOR_SOLICIT) || flow->tp_src == htons(ND_NEIGHBOR_ADVERT))); case MFP_ND_SOLICIT: - return (is_icmpv6(flow) + return (is_icmpv6(flow, NULL) && flow->tp_dst == htons(0) && (flow->tp_src == htons(ND_NEIGHBOR_SOLICIT))); case MFP_ND_ADVERT: - return (is_icmpv6(flow) + return (is_icmpv6(flow, NULL) && flow->tp_dst == htons(0) && (flow->tp_src == htons(ND_NEIGHBOR_ADVERT))); } diff --git a/lib/nx-match.c b/lib/nx-match.c index 3c48570..aad6e02 100644 --- a/lib/nx-match.c +++ b/lib/nx-match.c @@ -861,7 +861,7 @@ nxm_put_ip(struct ofpbuf *b, const struct match *match, enum ofp_version oxm) match->wc.masks.tp_src); nxm_put_16m(b, MFF_SCTP_DST, oxm, flow->tp_dst, match->wc.masks.tp_dst); - } else if (is_icmpv4(flow)) { + } else if (is_icmpv4(flow, NULL)) { if (match->wc.masks.tp_src) { nxm_put_8(b, MFF_ICMPV4_TYPE, oxm, ntohs(flow->tp_src)); @@ -870,7 +870,7 @@ nxm_put_ip(struct ofpbuf *b, const struct match *match, enum ofp_version oxm) nxm_put_8(b, MFF_ICMPV4_CODE, oxm, ntohs(flow->tp_dst)); } - } else if (is_icmpv6(flow)) { + } else if (is_icmpv6(flow, NULL)) { if (match->wc.masks.tp_src) { nxm_put_8(b, MFF_ICMPV6_TYPE, oxm, ntohs(flow->tp_src)); diff --git a/lib/odp-util.c b/lib/odp-util.c index e0a1ad4..9144e56 100644 --- a/lib/odp-util.c +++ b/lib/odp-util.c @@ -5719,9 +5719,9 @@ commit_set_icmp_action(const struct flow *flow, struct flow *base_flow, struct ovs_key_icmp key, mask, base; enum ovs_key_attr attr; - if (is_icmpv4(flow)) { + if (is_icmpv4(flow, NULL)) { attr = OVS_KEY_ATTR_ICMP; - } else if (is_icmpv6(flow)) { + } else if (is_icmpv6(flow, NULL)) { attr = OVS_KEY_ATTR_ICMPV6; } else { return 0; diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index cdf5a83..08eabf5 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -2359,6 +2359,20 @@ xlate_normal_flood(struct xlate_ctx *ctx, struct xbundle *in_xbundle, ctx->nf_output_iface = NF_OUT_FLOOD; } +static bool +is_ip_local_multicast(const struct flow *flow, struct flow_wildcards *wc) +{ + if (flow->dl_type == htons(ETH_TYPE_IP)) { + memset(&wc->masks.nw_dst, 0xff, sizeof wc->masks.nw_dst); + return ip_is_local_multicast(flow->nw_dst); + } else if (flow->dl_type == htons(ETH_TYPE_IPV6)) { + memset(&wc->masks.ipv6_dst, 0xff, sizeof wc->masks.ipv6_dst); + return ipv6_is_all_hosts(&flow->ipv6_dst); + } else { + return false; + } +} + static void xlate_normal(struct xlate_ctx *ctx) { @@ -2442,7 +2456,8 @@ xlate_normal(struct xlate_ctx *ctx) struct mcast_snooping *ms = ctx->xbridge->ms; struct mcast_group *grp = NULL; - if (is_igmp(flow)) { + if (is_igmp(flow, wc)) { + memset(&wc->masks.tp_src, 0xff, sizeof wc->masks.tp_src); if (mcast_snooping_is_membership(flow->tp_src) || mcast_snooping_is_query(flow->tp_src)) { if (ctx->xin->may_learn && ctx->xin->packet) { @@ -2475,13 +2490,13 @@ xlate_normal(struct xlate_ctx *ctx) xlate_normal_flood(ctx, in_xbundle, vlan); } return; - } else if (is_mld(flow)) { + } else if (is_mld(flow, wc)) { ctx->xout->slow |= SLOW_ACTION; if (ctx->xin->may_learn && ctx->xin->packet) { update_mcast_snooping_table(ctx->xbridge, flow, vlan, in_xbundle, ctx->xin->packet); } - if (is_mld_report(flow)) { + if (is_mld_report(flow, wc)) { ovs_rwlock_rdlock(&ms->rwlock); xlate_normal_mcast_send_mrouters(ctx, ms, in_xbundle, vlan); xlate_normal_mcast_send_rports(ctx, ms, in_xbundle, vlan); @@ -2491,10 +2506,7 @@ xlate_normal(struct xlate_ctx *ctx) xlate_normal_flood(ctx, in_xbundle, vlan); } } else { - if ((flow->dl_type == htons(ETH_TYPE_IP) - && ip_is_local_multicast(flow->nw_dst)) - || (flow->dl_type == htons(ETH_TYPE_IPV6) - && ipv6_is_all_hosts(&flow->ipv6_dst))) { + if (is_ip_local_multicast(flow, wc)) { /* RFC4541: section 2.1.2, item 2: Packets with a dst IP * address in the 224.0.0.x range which are not IGMP must * be forwarded on all ports */ @@ -5030,7 +5042,7 @@ xlate_wc_finish(struct xlate_ctx *ctx) * Avoid the problem here by making sure that only the low 8 bits of * either field can be unwildcarded for ICMP. */ - if (is_icmpv4(&ctx->xin->flow) || is_icmpv6(&ctx->xin->flow)) { + if (is_icmpv4(&ctx->xin->flow, NULL) || is_icmpv6(&ctx->xin->flow, NULL)) { ctx->wc->masks.tp_src &= htons(UINT8_MAX); ctx->wc->masks.tp_dst &= htons(UINT8_MAX); } -- 2.1.3 _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
