[ovs-dev] [PATCH] ofproto-dpif-upcall: Fix logic error in handler/revalidator threads creation and deletion.

[Ethan Jackson]

LGTM as well

On Mon, Apr 21, 2014 at 9:09 PM, Alex Wang <alexw at nicira.com> wrote:
> Commit 1f8675481e (ofproto-dpif-upcall: Fix ovs-vswitchd crash.)
> directly copied the udpif_set_threads() logic to udpif_stop_threads()
> and udpif_start_threads().  In fact, this was erroneous and caused
> unittest failures.
>
> This commit fixes the above issue by correcting the checks in
> udpif_stop_threads() and udpif_start_threads(), and adding necessary
> checks in udpif_set_threads().
>
> Acked-by: Ethan Jackson <ethan at nicira.com>
> Signed-off-by: Alex Wang <alexw at nicira.com>
>
> ---
> PATCH -> V2:
> - refine the checks.
> ---
>  ofproto/ofproto-dpif-upcall.c |   16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
> index 8e43e84..416cfdc 100644
> --- a/ofproto/ofproto-dpif-upcall.c
> +++ b/ofproto/ofproto-dpif-upcall.c
> @@ -296,9 +296,7 @@ udpif_destroy(struct udpif *udpif)
>  static void
>  udpif_stop_threads(struct udpif *udpif)
>  {
> -    if (udpif->handlers &&
> -        (udpif->n_handlers != n_handlers
> -         || udpif->n_revalidators != n_revalidators)) {
> +    if (udpif && (udpif->n_handlers != 0 || udpif->n_revalidators != 0)) {
>          size_t i;
>
>          latch_set(&udpif->exit_latch);
> @@ -360,7 +358,7 @@ static void
>  udpif_start_threads(struct udpif *udpif, size_t n_handlers,
>                      size_t n_revalidators)
>  {
> -    if (!udpif->handlers && n_handlers) {
> +    if (udpif && (!udpif->handlers && !udpif->revalidators)) {
>          size_t i;
>
>          udpif->n_handlers = n_handlers;
> @@ -403,10 +401,14 @@ udpif_set_threads(struct udpif *udpif, size_t n_handlers,
>  {
>      int error;
>
> +    ovs_assert(udpif);
>      ovs_assert(n_handlers && n_revalidators);
>
>      ovsrcu_quiesce_start();
> -    udpif_stop_threads(udpif);
> +    if (udpif->n_handlers != n_handlers
> +        || udpif->n_revalidators != n_revalidators) {
> +        udpif_stop_threads(udpif);
> +    }
>
>      error = dpif_handlers_set(udpif->dpif, n_handlers);
>      if (error) {
> @@ -415,7 +417,9 @@ udpif_set_threads(struct udpif *udpif, size_t n_handlers,
>          return;
>      }
>
> -    udpif_start_threads(udpif, n_handlers, n_revalidators);
> +    if (!udpif->handlers && !udpif->revalidators) {
> +        udpif_start_threads(udpif, n_handlers, n_revalidators);
> +    }
>      ovsrcu_quiesce_end();
>  }
>
> --
> 1.7.9.5
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev