Re: [ovs-dev] [PATCH v6 14/18] lib/rstp: Remove lock recursion.
Acked-by: Daniele Venturino
2014-08-21 1:57 GMT+02:00 Jarno Rajahalme :
> Change the RSTP send_bpdu interface so that a recursive mutex is not
> needed.
>
> Signed-off-by: Jarno Rajahalme
> ---
> 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 unkno
[ovs-dev] [PATCH v6 14/18] lib/rstp: Remove lock recursion.
Change the RSTP send_bpdu interface so that a recursive mutex is not
needed.
Signed-off-by: Jarno Rajahalme
---
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 a3
