On Thu, Feb 18, 2016 at 01:35:20PM -0800, Joe Stringer wrote: > On 17 February 2016 at 07:24, Aaron Conole <acon...@redhat.com> wrote: > > Greetings Thadeu, > > > > Thadeu Lima de Souza Cascardo <casca...@redhat.com> writes: > > > >> The revalidator thread may set may_learn and call xlate_actions with no > >> packet > >> data. If the revalidated flow is IGMPv3 or MLD, vswitchd will crash when > >> trying > >> to access the NULL packet. > >> > >> Only process IGMP and MLD flows when there is a packet. This is a similar > >> behavior than what we have for other special packets. > > > > Just a question on the approach, would it make sense to check for the > > bundle being null as well? I don't know if the case where packet == NULL > > but bundle != NULL could ever exist so I could be off my rocker. > > If you're referring to "in_xbundle", one of the first things that > xlate_normal() does is to ensure it is non-NULL. > > > Alternately, maybe it's better to just patch the mcast functions to bail > > early (maybe with a ratelimited warning) in this case, so that if bundle > > != NULL but packet == NULL, some of the mcast group LRU cache can be > > updated? Don't know if that makes sense, either (maybe there's a > > security reason not to do that?). > > This codepath shouldn't assume that it has a packet. It is called from > revalidator threads to attribute stats and do side-effects (like > learning, cache refresh). That path doesn't have a copy of a packet to > use for translation. If 'may_learn' is true, it indicates that packets > have hit this flow since the last time stats were attributed, so > learning/cache refresh should occur if applicable. In the case of > special handling of packets which are slowpathed (as indicated by the > "ctx->xout->slow |= SLOW_ACTION" line), it is fine to do nothing when > there is no packet. The slow path handling code will deal with > side-effects.
Thanks for the explanation, Joe. I guess this means the patch is good as it is, though it would be nice to have Yi Ba Tested-by. Regards. Cascardo. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
