Thanks for verifying the fix. I applied this to master. I think you're right that there's a remaining mismatch here. If you have time to figure out a full and proper solution, then please do so; I'd appreciate it.
On Sat, Jul 15, 2017 at 11:11:40AM +0800, Huanle Han wrote: > 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 <[email protected]> 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 [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
