[ovs-dev] [threads v2 08/13] byteq: Make the queue size variable instead of fixed at BYTEQ_SIZE bytes.

[Alex Wang]

Looks good to me,

Just one question, for the code below in "lib/byteq.c"

"""
/* Returns the number of bytes current queued in 'q'. */
int
byteq_used(const struct byteq *q)
{
    return q->head - q->tail;
}
"""

Is it possible that the jsonrpc session last so long that the 'q->head'
gets overflowed? or the chance is so slim that we do not care,

Kind Regards,
Alex Wang,


On Fri, Jul 12, 2013 at 2:54 PM, Ben Pfaff <blp at nicira.com> wrote:

> Signed-off-by: Ben Pfaff <blp at nicira.com>
> ---
>  lib/byteq.c   |   26 +++++++++++++++-----------
>  lib/byteq.h   |   10 ++++------
>  lib/jsonrpc.c |    5 +++--
>  3 files changed, 22 insertions(+), 19 deletions(-)
>
> diff --git a/lib/byteq.c b/lib/byteq.c
> index 2ee4a65..3f865cf 100644
> --- a/lib/byteq.c
> +++ b/lib/byteq.c
> @@ -1,4 +1,4 @@
> -/* Copyright (c) 2008, 2009, 2012 Nicira, Inc.
> +/* Copyright (c) 2008, 2009, 2012, 2013 Nicira, Inc.
>   *
>   * Licensed under the Apache License, Version 2.0 (the "License");
>   * you may not use this file except in compliance with the License.
> @@ -20,13 +20,17 @@
>  #include <unistd.h>
>  #include "util.h"
>
> -/* The queue size must be a power of 2. */
> -BUILD_ASSERT_DECL(!(BYTEQ_SIZE & (BYTEQ_SIZE - 1)));
> -
> -/* Initializes 'q' as empty. */
> +/* Initializes 'q' as an empty byteq that uses the 'size' bytes of
> 'buffer' to
> + * store data.  'size' must be a power of 2.
> + *
> + * The caller must ensure that 'buffer' remains available to the byteq as
> long
> + * as 'q' is in use. */
>  void
> -byteq_init(struct byteq *q)
> +byteq_init(struct byteq *q, uint8_t *buffer, size_t size)
>  {
> +    ovs_assert(is_pow2(size));
> +    q->buffer = buffer;
> +    q->size = size;
>      q->head = q->tail = 0;
>  }
>
> @@ -41,7 +45,7 @@ byteq_used(const struct byteq *q)
>  int
>  byteq_avail(const struct byteq *q)
>  {
> -    return BYTEQ_SIZE - byteq_used(q);
> +    return q->size - byteq_used(q);
>  }
>
>  /* Returns true if no bytes are queued in 'q',
> @@ -147,7 +151,7 @@ int
>  byteq_tailroom(const struct byteq *q)
>  {
>      int used = byteq_used(q);
> -    int tail_to_end = BYTEQ_SIZE - (q->tail & (BYTEQ_SIZE - 1));
> +    int tail_to_end = q->size - (q->tail & (q->size - 1));
>      return MIN(used, tail_to_end);
>  }
>
> @@ -156,7 +160,7 @@ byteq_tailroom(const struct byteq *q)
>  const uint8_t *
>  byteq_tail(const struct byteq *q)
>  {
> -    return &q->buffer[q->tail & (BYTEQ_SIZE - 1)];
> +    return &q->buffer[q->tail & (q->size - 1)];
>  }
>
>  /* Removes 'n' bytes from the tail of 'q', which must have at least 'n'
> bytes
> @@ -173,7 +177,7 @@ byteq_advance_tail(struct byteq *q, unsigned int n)
>  uint8_t *
>  byteq_head(struct byteq *q)
>  {
> -    return &q->buffer[q->head & (BYTEQ_SIZE - 1)];
> +    return &q->buffer[q->head & (q->size - 1)];
>  }
>
>  /* Returns the number of contiguous bytes of free space starting at the
> head
> @@ -182,7 +186,7 @@ int
>  byteq_headroom(const struct byteq *q)
>  {
>      int avail = byteq_avail(q);
> -    int head_to_end = BYTEQ_SIZE - (q->head & (BYTEQ_SIZE - 1));
> +    int head_to_end = q->size - (q->head & (q->size - 1));
>      return MIN(avail, head_to_end);
>  }
>
> diff --git a/lib/byteq.h b/lib/byteq.h
> index 5fa51fd..d73e368 100644
> --- a/lib/byteq.h
> +++ b/lib/byteq.h
> @@ -1,4 +1,4 @@
> -/* Copyright (c) 2008, 2009 Nicira, Inc.
> +/* Copyright (c) 2008, 2009, 2013 Nicira, Inc.
>   *
>   * Licensed under the Apache License, Version 2.0 (the "License");
>   * you may not use this file except in compliance with the License.
> @@ -20,17 +20,15 @@
>  #include <stddef.h>
>  #include <stdint.h>
>
> -/* Maximum number of bytes in a byteq. */
> -#define BYTEQ_SIZE 512
> -
>  /* General-purpose circular queue of bytes. */
>  struct byteq {
> -    uint8_t buffer[BYTEQ_SIZE]; /* Circular queue. */
> +    uint8_t *buffer;            /* Circular queue. */
> +    unsigned int size;          /* Number of bytes allocated for
> 'buffer'. */
>      unsigned int head;          /* Head of queue. */
>      unsigned int tail;          /* Chases the head. */
>  };
>
> -void byteq_init(struct byteq *);
> +void byteq_init(struct byteq *, uint8_t *buffer, size_t size);
>  int byteq_used(const struct byteq *);
>  int byteq_avail(const struct byteq *);
>  bool byteq_is_empty(const struct byteq *);
> diff --git a/lib/jsonrpc.c b/lib/jsonrpc.c
> index 56b4cce..b4bbc84 100644
> --- a/lib/jsonrpc.c
> +++ b/lib/jsonrpc.c
> @@ -41,6 +41,7 @@ struct jsonrpc {
>
>      /* Input. */
>      struct byteq input;
> +    uint8_t input_buffer[512];
>      struct json_parser *parser;
>      struct jsonrpc_msg *received;
>
> @@ -87,7 +88,7 @@ jsonrpc_open(struct stream *stream)
>      rpc = xzalloc(sizeof *rpc);
>      rpc->name = xstrdup(stream_get_name(stream));
>      rpc->stream = stream;
> -    byteq_init(&rpc->input);
> +    byteq_init(&rpc->input, rpc->input_buffer, sizeof rpc->input_buffer);
>      list_init(&rpc->output);
>
>      return rpc;
> @@ -330,7 +331,7 @@ jsonrpc_recv(struct jsonrpc *rpc, struct jsonrpc_msg
> **msgp)
>                  jsonrpc_received(rpc);
>                  if (rpc->status) {
>                      const struct byteq *q = &rpc->input;
> -                    if (q->head <= BYTEQ_SIZE) {
> +                    if (q->head <= q->size) {
>                          stream_report_content(q->buffer, q->head,
>                                                STREAM_JSONRPC,
>                                                THIS_MODULE, rpc->name);
> --
> 1.7.2.5
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openvswitch.org/pipermail/ovs-dev/attachments/20130717/f4035e43/attachment-0003.html>