This patch can slove the problem. But I think wc->masks.tp_src should not be masked in userspace as igmp type is not supported in datapath. Otherwise, flow_wildcards_has_extra() in revalidate_ukey__ always return true for igmp megaflow, and igmp flow is never kept in datapath.
Thanks, Huanle On Wednesday, July 12, 2017, Ben Pfaff <b...@ovn.org> wrote: > On Thu, Jun 22, 2017 at 01:04:48AM +0800, Huanle Han wrote: > > In "Normal" action, igmp report packet is expected to processed in slow > > path. > > However, the igmp_type(flow->tp_src) is not supported to be masked in > > datapath. > > Then ovs-vswitchd revalidate the flow with igmp_type(flow->tp_src) == 0. > It > > leads to a "multicast traffic, flooding" action and overwrites the > > "userspace()" in datapath. > > > > This is code segment from function "xlate_normal": > > > > 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)) { <<<<<<<<< > > flow->tp_src is 0 > > if (ctx->xin->allow_side_effects && ctx->xin->packet) { > > update_mcast_snooping_table(ctx, flow, vlan, > > in_xbundle, > > ctx->xin->packet); > > } > > /* > > * IGMP packets need to take the slow path, in order to > be > > * processed for mdb updates. That will prevent expires > > * firing off even after hosts have sent reports. > > */ > > ctx->xout->slow |= SLOW_ACTION; > > } > > ....... > > } > > > > To reproduce this bug, you need to ovs-appctl upcall/disable-megaflows > > Thanks for the bug report. > > Does the following patch fix the problem? > > diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c > index 089c7f170d18..5030dafd9f42 100644 > --- a/ofproto/ofproto-dpif-xlate.c > +++ b/ofproto/ofproto-dpif-xlate.c > @@ -2720,6 +2720,13 @@ xlate_normal(struct xlate_ctx *ctx) > struct mcast_group *grp = NULL; > > if (is_igmp(flow, wc)) { > + /* > + * IGMP packets need to take the slow path, in order to be > + * processed for mdb updates. That will prevent expires > + * firing off even after hosts have sent reports. > + */ > + ctx->xout->slow |= SLOW_ACTION; > + > 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)) { > @@ -2727,12 +2734,6 @@ xlate_normal(struct xlate_ctx *ctx) > update_mcast_snooping_table(ctx, flow, vlan, > in_xbundle, > ctx->xin->packet); > } > - /* > - * IGMP packets need to take the slow path, in order to be > - * processed for mdb updates. That will prevent expires > - * firing off even after hosts have sent reports. > - */ > - ctx->xout->slow |= SLOW_ACTION; > } > > if (mcast_snooping_is_membership(flow->tp_src)) { > _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev