Would this incremental alleviate your concerns?
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index e8f3dbf..aa8cea1 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -3000,11 +3000,11 @@ xlate_learn_action(struct xlate_ctx *ctx, const
struct ofpact_learn *learn)
entry = xlate_cache_add_entry(ctx->xin->xcache, XC_LEARN);
entry->u.learn.ofproto = ctx->xbridge->ofproto;
entry->u.learn.fm = xmalloc(sizeof *entry->u.learn.fm);
- entry->u.learn.ofpacts = ofpbuf_new(1024);
+ entry->u.learn.ofpacts = ofpbuf_new(64);
xlate_learn_action__(ctx, learn, entry->u.learn.fm,
entry->u.learn.ofpacts);
} else if (ctx->xin->may_learn) {
- uint64_t ofpacts_stub[1024 / 8];
+ uint64_t ofpacts_stub[64 / 8];
struct ofputil_flow_mod fm;
struct ofpbuf ofpacts;
On 4 June 2014 10:11, 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
