[ovs-dev] [PATCH v6 14/18] lib/rstp: Remove lock recursion.

Daniele Venturino venturino.daniele at gmail.com
Tue Sep 9 10:36:49 UTC 2014


Acked-by: Daniele Venturino <daniele.venturino at m3s.it>

2014-08-21 1:57 GMT+02:00 Jarno Rajahalme <jrajahalme at nicira.com>:

> Change the RSTP send_bpdu interface so that a recursive mutex is not
> needed.
>
> Signed-off-by: Jarno Rajahalme <jrajahalme at nicira.com>
> ---
>  lib/rstp-common.h         |    2 +-
>  lib/rstp-state-machines.c |    2 +-
>  lib/rstp.c                |    7 +++----
>  lib/rstp.h                |    5 ++---
>  ofproto/ofproto-dpif.c    |   28 +++++++++++-----------------
>  tests/test-rstp.c         |   12 +++++++-----
>  6 files changed, 25 insertions(+), 31 deletions(-)
>
> diff --git a/lib/rstp-common.h b/lib/rstp-common.h
> index 6852263..d798ba2 100644
> --- a/lib/rstp-common.h
> +++ b/lib/rstp-common.h
> @@ -872,7 +872,7 @@ struct rstp {
>      struct ovs_refcount ref_cnt;
>
>      /* Interface to client. */
> -    void (*send_bpdu)(struct ofpbuf *bpdu, int port_no, void *aux);
> +    void (*send_bpdu)(struct ofpbuf *bpdu, void *port_aux, void
> *rstp_aux);
>      void *aux;
>  };
>
> diff --git a/lib/rstp-state-machines.c b/lib/rstp-state-machines.c
> index 297f7ea..3a408b4 100644
> --- a/lib/rstp-state-machines.c
> +++ b/lib/rstp-state-machines.c
> @@ -678,7 +678,7 @@ rstp_send_bpdu(struct rstp_port *p, const void *bpdu,
> size_t bpdu_size)
>      llc->llc_dsap = STP_LLC_DSAP;
>      llc->llc_ssap = STP_LLC_SSAP;
>      llc->llc_cntl = STP_LLC_CNTL;
> -    p->rstp->send_bpdu(pkt, p->port_number, p->rstp->aux);
> +    p->rstp->send_bpdu(pkt, p->aux, p->rstp->aux);
>  }
>
>  static void
> diff --git a/lib/rstp.c b/lib/rstp.c
> index 72b5f38..223df01 100644
> --- a/lib/rstp.c
> +++ b/lib/rstp.c
> @@ -48,7 +48,7 @@
>
>  VLOG_DEFINE_THIS_MODULE(rstp);
>
> -struct ovs_mutex rstp_mutex;
> +struct ovs_mutex rstp_mutex = OVS_MUTEX_INITIALIZER;
>
>  static struct list all_rstps__ = LIST_INITIALIZER(&all_rstps__);
>  static struct list *const all_rstps OVS_GUARDED_BY(rstp_mutex) =
> &all_rstps__;
> @@ -232,8 +232,6 @@ void
>  rstp_init(void)
>      OVS_EXCLUDED(rstp_mutex)
>  {
> -    ovs_mutex_init_recursive(&rstp_mutex);
> -
>      unixctl_command_register("rstp/tcn", "[bridge]", 0, 1,
> rstp_unixctl_tcn,
>                               NULL);
>  }
> @@ -241,7 +239,8 @@ rstp_init(void)
>  /* Creates and returns a new RSTP instance that initially has no ports. */
>  struct rstp *
>  rstp_create(const char *name, rstp_identifier bridge_address,
> -            void (*send_bpdu)(struct ofpbuf *bpdu, int port_no, void
> *aux),
> +            void (*send_bpdu)(struct ofpbuf *bpdu, void *port_aux,
> +                              void *rstp_aux),
>              void *aux)
>      OVS_EXCLUDED(rstp_mutex)
>  {
> diff --git a/lib/rstp.h b/lib/rstp.h
> index c6c4a71..364a181 100644
> --- a/lib/rstp.h
> +++ b/lib/rstp.h
> @@ -130,13 +130,12 @@ static inline bool rstp_forward_in_state(enum
> rstp_state);
>  static inline bool rstp_learn_in_state(enum rstp_state);
>  static inline bool rstp_should_manage_bpdu(enum rstp_state state);
>
> -/* Must be called before any other rstp function is called. */
>  void rstp_init(void)
>      OVS_EXCLUDED(rstp_mutex);
>
>  struct rstp * rstp_create(const char *, rstp_identifier bridge_id,
> -                          void (*send_bpdu)(struct ofpbuf *, int port_no,
> -                                            void *aux),
> +                          void (*send_bpdu)(struct ofpbuf *, void
> *port_aux,
> +                                            void *rstp_aux),
>                            void *aux)
>      OVS_EXCLUDED(rstp_mutex);
>
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index 207de1d..9c73422 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -1903,27 +1903,21 @@ get_bfd_status(struct ofport *ofport_, struct smap
> *smap)
>
>  /* Spanning Tree. */
>
> +/* Called while rstp_mutex is held. */
>  static void
> -rstp_send_bpdu_cb(struct ofpbuf *pkt, int port_num, void *ofproto_)
> +rstp_send_bpdu_cb(struct ofpbuf *pkt, void *ofport_, void *ofproto_)
>  {
>      struct ofproto_dpif *ofproto = ofproto_;
> -    struct ofport_dpif *ofport;
> -
> -    ofport = rstp_get_port_aux(ofproto->rstp, port_num);
> -    if (!ofport) {
> -        VLOG_WARN_RL(&rl, "%s: cannot send BPDU on unknown RSTP port %d",
> -                     ofproto->up.name, port_num);
> +    struct ofport_dpif *ofport = ofport_;
> +    struct eth_header *eth = ofpbuf_l2(pkt);
> +
> +    netdev_get_etheraddr(ofport->up.netdev, eth->eth_src);
> +    if (eth_addr_is_zero(eth->eth_src)) {
> +        VLOG_WARN_RL(&rl, "%s port %d: cannot send RSTP BPDU on a port
> which "
> +                     "does not have a configured source MAC address.",
> +                     ofproto->up.name, ofp_to_u16(ofport->up.ofp_port));
>      } else {
> -        struct eth_header *eth = ofpbuf_l2(pkt);
> -
> -        netdev_get_etheraddr(ofport->up.netdev, eth->eth_src);
> -        if (eth_addr_is_zero(eth->eth_src)) {
> -            VLOG_WARN_RL(&rl, "%s port %d: cannot send BPDU on RSTP port
> %d "
> -                         "with unknown MAC", ofproto->up.name,
> -                         ofp_to_u16(ofport->up.ofp_port), port_num);
> -        } else {
> -            ofproto_dpif_send_packet(ofport, pkt);
> -        }
> +        ofproto_dpif_send_packet(ofport, pkt);
>      }
>      ofpbuf_delete(pkt);
>  }
> diff --git a/tests/test-rstp.c b/tests/test-rstp.c
> index a365423..afc5b60 100644
> --- a/tests/test-rstp.c
> +++ b/tests/test-rstp.c
> @@ -1,6 +1,6 @@
>  #include <config.h>
>
> -#include "rstp.h"
> +#include "rstp-common.h"
>  #include <assert.h>
>  #include <ctype.h>
>  #include <errno.h>
> @@ -72,11 +72,15 @@ new_test_case(void)
>      return tc;
>  }
>
> +/* This callback is called with rstp_mutex held. */
>  static void
> -send_bpdu(struct ofpbuf *pkt, int port_no, void *b_)
> +send_bpdu(struct ofpbuf *pkt, void *port_, void *b_)
> +    OVS_REQUIRES(rstp_mutex)
>  {
>      struct bridge *b = b_;
>      struct lan *lan;
> +    const struct rstp_port *port = port_;
> +    uint16_t port_no = port->port_number;
>
>      assert(port_no < b->n_ports);
>      lan = b->ports[port_no];
> @@ -116,7 +120,7 @@ new_bridge(struct test_case *tc, int id)
>      b->rstp = rstp_create(name, id, send_bpdu, b);
>      for (i = 1; i < MAX_PORTS; i++) {
>          p = rstp_add_port(b->rstp);
> -        rstp_port_set_aux(p, b);
> +        rstp_port_set_aux(p, p);
>          rstp_port_set_state(p, RSTP_DISABLED);
>          rstp_port_set_mac_operational(p, true);
>      }
> @@ -459,8 +463,6 @@ test_rstp_main(int argc, char *argv[])
>      FILE *input_file;
>      int i;
>
> -    rstp_init();
> -
>      vlog_set_pattern(VLF_CONSOLE, "%c|%p|%m");
>      vlog_set_levels(NULL, VLF_SYSLOG, VLL_OFF);
>
> --
> 1.7.10.4
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>



More information about the dev mailing list