On 16 August 2012 11:42, Ben Pfaff <b...@nicira.com> wrote:
> On Thu, Aug 16, 2012 at 10:22:21AM -0400, Ed Maste wrote:
>> On 15 August 2012 19:12, Ben Pfaff <b...@nicira.com> wrote:
>> > This is easy enough, so it seems worthwhile now that FreeBSD is starting
>> > to make more use of the "userspace switch".
>> >
>> > CC: Ed Maste <ema...@freebsd.org>
>> > Signed-off-by: Ben Pfaff <b...@nicira.com>
>> > ---
>> >  lib/dpif-netdev.c |   33 +++++++++++++++++++--------------
>> >  1 files changed, 19 insertions(+), 14 deletions(-)
>> >
>> > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>> > index 3d01b17..144b6b6 100644
>> > --- a/lib/dpif-netdev.c
>> > +++ b/lib/dpif-netdev.c
>> > @@ -70,8 +70,13 @@ enum { MAX_QUEUE_LEN = 128 };   /* Maximum number of 
>> > packets per queue. */
>> >  enum { QUEUE_MASK = MAX_QUEUE_LEN - 1 };
>> >  BUILD_ASSERT_DECL(IS_POW2(MAX_QUEUE_LEN));
>> >
>> > +struct dp_netdev_upcall {
>> > +    struct dpif_upcall upcall;  /* Queued upcall information. */
>> > +    struct ofpbuf buf;          /* ofpbuf instance for upcall.packet. */
>> > +};
>> > +
>> >  struct dp_netdev_queue {
>> > -    struct dpif_upcall *upcalls[MAX_QUEUE_LEN];
>> > +    struct dp_netdev_upcall upcalls[MAX_QUEUE_LEN];
>> >      unsigned int head, tail;
>> >  };
>> >
>> > @@ -259,10 +264,8 @@ dp_netdev_purge_queues(struct dp_netdev *dp)
>> >          struct dp_netdev_queue *q = &dp->queues[i];
>> >
>> >          while (q->tail != q->head) {
>> > -            struct dpif_upcall *upcall = q->upcalls[q->tail++ & 
>> > QUEUE_MASK];
>> > -
>> > -            ofpbuf_delete(upcall->packet);
>> > -            free(upcall);
>> > +            struct dp_netdev_upcall *u = &q->upcalls[q->tail++ & 
>> > QUEUE_MASK];
>> > +            ofpbuf_uninit(&u->buf);
>> >          }
>> >      }
>> >  }
>> > @@ -960,13 +963,13 @@ dpif_netdev_recv(struct dpif *dpif, struct 
>> > dpif_upcall *upcall,
>> >  {
>> >      struct dp_netdev_queue *q = find_nonempty_queue(dpif);
>> >      if (q) {
>> > -        struct dpif_upcall *u = q->upcalls[q->tail++ & QUEUE_MASK];
>> > -        *upcall = *u;
>> > -        free(u);
>> > +        struct dp_netdev_upcall *u = &q->upcalls[q->tail++ & QUEUE_MASK];
>> > +
>> > +        *upcall = u->upcall;
>> > +        upcall->packet = buf;
>>
>> Doesn't upcall->key still point into the static buf from upcalls[] in
>> this case?
>
> Yes.  We've copied that buf into the function's argument and transferred
> ownership of its data to the caller.  The next time the static buf gets
> used it will be reinitialized with a newly allocated data buffer.

Ahh, of course.  This looks good to me.

-Ed
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to