[ovs-dev] [PATCH 3/5] upcall: Refactor ukey creation and dump handling

Joe Stringer joestringer at nicira.com
Wed Feb 26 00:10:35 UTC 2014


Ah, great. The original refactoring was mostly split out from the patchset
to get rid of the flow_dumper thread, which made it easier to split these
statistics fixes out. I hadn't realised there was a bit more that we could
do.


On 25 February 2014 15:51, Ben Pfaff <blp at nicira.com> wrote:

> On Tue, Feb 25, 2014 at 03:41:36PM -0800, Ben Pfaff wrote:
> > On Tue, Feb 25, 2014 at 03:25:35PM -0800, Ben Pfaff wrote:
> > > On Tue, Feb 11, 2014 at 01:55:34PM -0800, Joe Stringer wrote:
> > > > This splits out functions for re-use by later patches, and compacts
> the
> > > > udump revalidation code.
> > > >
> > > > Co-authored-by: Ethan Jackson <ethan at nicira.com>
> > > > Signed-off-by: Joe Stringer <joestringer at nicira.com>
> > >
> > > I have this queued up to apply, thanks!
> >
> > It looks like we can avoid more code duplication in the following
> > patch by folding in the following.  I'm tentatively doing that.
>
> Ditto for the following.
>
> diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
> index 1bced67..343181b 100644
> --- a/ofproto/ofproto-dpif-upcall.c
> +++ b/ofproto/ofproto-dpif-upcall.c
> @@ -1412,8 +1412,10 @@ dump_op_init(struct dump_op *op, const struct
> nlattr *key, size_t key_len,
>  }
>
>  static void
> -push_dump_ops(struct udpif *udpif, struct dump_op *ops, size_t n_ops)
> +push_dump_ops(struct revalidator *revalidator,
> +              struct dump_op *ops, size_t n_ops)
>  {
> +    struct udpif *udpif = revalidator->udpif;
>      struct dpif_op *opsp[REVALIDATE_MAX_BATCH];
>      size_t i;
>
> @@ -1463,6 +1465,18 @@ push_dump_ops(struct udpif *udpif, struct dump_op
> *ops, size_t n_ops)
>              }
>          }
>      }
> +
> +    for (i = 0; i < n_ops; i++) {
> +        struct udpif_key *ukey = ops[i].ukey;
> +
> +        /* Look up the ukey to prevent double-free in case 'ops' contains
> a
> +         * given ukey more than once (which can happen if the datapath
> dumps a
> +         * given flow more than once). */
> +        ukey = ukey_lookup(revalidator, ops[i].udump);
> +        if (ukey) {
> +            ukey_delete(revalidator, ukey);
> +        }
> +    }
>  }
>
>  static void
> @@ -1472,7 +1486,7 @@ revalidate_udumps(struct revalidator *revalidator,
> struct list *udumps)
>
>      struct dump_op ops[REVALIDATE_MAX_BATCH];
>      struct udpif_flow_dump *udump, *next_udump;
> -    size_t n_ops, i, n_flows;
> +    size_t n_ops, n_flows;
>      unsigned int flow_limit;
>      long long int max_idle;
>      bool must_del;
> @@ -1524,17 +1538,7 @@ revalidate_udumps(struct revalidator *revalidator,
> struct list *udumps)
>          free(udump);
>      }
>
> -    push_dump_ops(udpif, ops, n_ops);
> -    for (i = 0; i < n_ops; i++) {
> -        struct udpif_key *ukey = ops[i].ukey;
> -
> -        /* Look up the ukey to prevent double-free if the datapath dumps
> the
> -         * same flow twice. */
> -        ukey = ukey_lookup(revalidator, ops[i].udump);
> -        if (ukey) {
> -            ukey_delete(revalidator, ukey);
> -        }
> -    }
> +    push_dump_ops(revalidator, ops, n_ops);
>
>      LIST_FOR_EACH_SAFE (udump, next_udump, list_node, udumps) {
>          list_remove(&udump->list_node);
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openvswitch.org/pipermail/ovs-dev/attachments/20140225/734cb26d/attachment-0005.html>


More information about the dev mailing list