On Wed, Jun 04, 2014 at 09:51:16AM +1200, Joe Stringer wrote: > On 4 June 2014 05:39, Alex Wang <al...@nicira.com> wrote: > > > I confirm this patch fix the bug, but have a high level concern, > > > > > > Assume another learnt flow with hard_timeout=10 and was never hit after > > install, now, if there is frequent change to the flow table and > > need_revalidate > > is constantly 'true', this learnt flow will never timeout... since we > > always > > redo the flow_mod during revalidation, > > > > (Correct me if I'm wrong, but this is how I see it:) > For this particular case, we will not redo the flow_mod during > revalidation. The "may_learn" flag is the same as before. Now, we will > create the flow_mod and cache it, but only do the flow_mod if may_learn is > true. > > The "execute_learn" function is perhaps a bit misleading here: It only > assembles the flow_mod, doesn't actually execute it.
I think this is logically correct. Have you confirmed that it fixes the original bug? One concern I have is memory consumption. struct ofputil_flow_mod is 452 bytes (which we use and initialize), plus we allocate another 1024 bytes for actions. I don't see an easy way to get rid of the former, but I think that the actions would normally be pretty small. I think we'd probably be better off with an initial stub for the latter that we then xmemdup() to save. Alternatively, if you changed the starting size of the actions ofpbuf to just, say, 64 bytes, I think that would save a lot of memory in the common case. Assuming it fixes the bug, Acked-by: Ben Pfaff <b...@nicira.com> _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
