OK. My memory consumption comment is just quibbling. I'd like to get in the fix, then we can reduce the memory consumption if need be.
On Tue, Jun 03, 2014 at 03:14:01PM -0700, Alex Wang wrote: > Yes, I have confirmed that this patch fix the bug. > > > On Tue, Jun 3, 2014 at 3:11 PM, Ben Pfaff <b...@nicira.com> wrote: > > > 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
