Yes, I moved it out of the function in the next patch.
On Tue, Feb 25, 2014 at 04:06:11PM -0800, Joe Stringer wrote:
> I suspect you may have found that the later patches make use of this
> structure in more locations?
>
> Either way, fine by me.
>
>
> On 25 February 2014 15:16, Ben Pfaff <b...@nicira.com> wrote:
>
> > On Tue, Feb 11, 2014 at 01:55:33PM -0800, Joe Stringer wrote:
> > > It is possible for a datapath to dump the same flow twice, for instance
> > > if the flow is the last in a batch of flows to be dumped, then a new
> > > flow is inserted into the same bucket before the flow dumper fetches
> > > another batch.
> > >
> > > In this case, datapath flow stats may be duplicated: The revalidator
> > > records the stats from the first flow, using the ukey to get the stats
> > > delta. The ukey is deleted, then the revalidator reads the second
> > > (duplicate) flow and cannot lookup the ukey for the delta. As such, it
> > > will push the stats as-is.
> > >
> > > This patch reduces the likelihood of such stats duplications by
> > > deferring ukey deletion until after stats are pushed for deleted flows.
> > >
> > > Signed-off-by: Joe Stringer <joestrin...@nicira.com>
> >
> > Thanks, I'm adding this to my queue for application.
> >
> > Since struct dump_op is only used inside revalidate_udumps() I decided
> > to move the declaration inside revalidate_udumps() too, for clarity:
> >
> > diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
> > index 254a59d..18784ec 100644
> > --- a/ofproto/ofproto-dpif-upcall.c
> > +++ b/ofproto/ofproto-dpif-upcall.c
> > @@ -1376,18 +1376,18 @@ exit:
> > return ok;
> > }
> >
> > -struct dump_op {
> > - struct udpif_key *ukey;
> > - struct udpif_flow_dump *udump;
> > - struct dpif_flow_stats stats; /* Stats for 'op'. */
> > - struct dpif_op op; /* Flow del operation. */
> > -};
> > -
> > static void
> > revalidate_udumps(struct revalidator *revalidator, struct list *udumps)
> > {
> > struct udpif *udpif = revalidator->udpif;
> >
> > + struct dump_op {
> > + struct udpif_key *ukey;
> > + struct udpif_flow_dump *udump;
> > + struct dpif_flow_stats stats; /* Stats for 'op'. */
> > + struct dpif_op op; /* Flow del operation. */
> > + };
> > +
> > struct dump_op ops[REVALIDATE_MAX_BATCH];
> > struct dpif_op *opsp[REVALIDATE_MAX_BATCH];
> > struct udpif_flow_dump *udump, *next_udump;
> >
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
