Re: [ovs-dev] [PATCH v6 13/18] lib/rstp: More robust thread safety.
Acked-by: Daniele Venturino
2014-08-21 1:57 GMT+02:00 Jarno Rajahalme :
> Current code expects there to be a single thread that is responsible
> for creating rstp and creating and deleting rstp_port objects. rstp
> objects are also deleted from other threads, as managed by reference
> counting.
>
> rstp port objects are not reference counted, which means that
> references to rstp ports may only be held while holding the rstp
> mutex, or by the thread that creates and deletes them.
>
> This patch adds reference counting to RSTP ports, which allows ports
> to be passed from ofproto-dpif to ofproto-dpif-xlate without using the
> RSTP port number. This simplifies RSTP port reconfiguration, as the
> port need not be resynchronized with xlate if just the port number
> changes. This also avoids lookups on the processing of RSTP BPDUs.
>
> This patch also:
>
> 1. Exposes the rstp mutex so that related thread safety annotations
>can be used also within rstp-state-machines.c.
>
> 2. Internal variants of most setter an getter functions are defined,
>suffixed with two underscores. These are annotated to be callable
>only when the mutex is held.
>
> 3. Port setters were only called in a specific pattern. The new external
>port setter combines them in a single rspt_port_set() function.
>
> Signed-off-by: Jarno Rajahalme
> ---
> lib/rstp-common.h| 202 +-
> lib/rstp-state-machines.c| 111 --
> lib/rstp-state-machines.h| 12 +-
> lib/rstp.c | 884
> +-
> lib/rstp.h | 174 ++---
> ofproto/ofproto-dpif-xlate.c | 53 +--
> ofproto/ofproto-dpif-xlate.h |3 +-
> ofproto/ofproto-dpif.c | 90 ++---
> tests/test-rstp.c| 36 +-
> 9 files changed, 931 insertions(+), 634 deletions(-)
>
> diff --git a/lib/rstp-common.h b/lib/rstp-common.h
> index a67115b..6852263 100644
> --- a/lib/rstp-common.h
> +++ b/lib/rstp-common.h
> @@ -261,10 +261,12 @@ enum rstp_rcvd_info {
> };
>
> struct rstp_port {
> -struct rstp *rstp;
> -struct list node; /* Node in rstp->ports list. */
> -void *aux;
> -struct rstp_bpdu received_bpdu_buffer;
> +struct ovs_refcount ref_cnt;
> +
> +struct rstp *rstp OVS_GUARDED_BY(rstp_mutex);
> +struct list node OVS_GUARDED_BY(rstp_mutex); /* Node in rstp->ports
> list. */
> +void *aux OVS_GUARDED_BY(rstp_mutex);
> +struct rstp_bpdu received_bpdu_buffer OVS_GUARDED_BY(rstp_mutex);
>
> /*
> * MAC status parameters
>
> /
> @@ -273,10 +275,10 @@ struct rstp_port {
> * to transmit and/or receive frames, and its use is permitted by
> * management.
> */
> -bool mac_operational;
> +bool mac_operational OVS_GUARDED_BY(rstp_mutex);
>
> /* [14.8.2.2] Administrative Bridge Port State */
> -bool is_administrative_bridge_port;
> +bool is_administrative_bridge_port OVS_GUARDED_BY(rstp_mutex);
>
> /* [6.4.3 - operPointToPointMAC]
> * a) True. The MAC is connected to a point-to-point LAN; i.e.,
> there is
> @@ -288,7 +290,7 @@ struct rstp_port {
> * shall be set True. If adminPointToPointMAC is set to ForceFalse,
> then
> * operPointToPointMAC shall be set False.
> */
> -bool oper_point_to_point_mac;
> +bool oper_point_to_point_mac OVS_GUARDED_BY(rstp_mutex);
>
> /* [6.4.3 - adminPointToPointMAC]
> * a) ForceTrue. The administrator requires the MAC to be treated as
> if it
> @@ -301,7 +303,7 @@ struct rstp_port {
> * MAC to be determined in accordance with the specific MAC
> procedures
> * defined in 6.5.
> */
> -enum rstp_admin_point_to_point_mac_state admin_point_to_point_mac;
> +enum rstp_admin_point_to_point_mac_state admin_point_to_point_mac
> OVS_GUARDED_BY(rstp_mutex);
>
>
>
> /*
> @@ -312,12 +314,12 @@ struct rstp_port {
> /* [17.13.1 - Admin Edge Port]
> * The AdminEdgePort parameter for the Port (14.8.2).
> */
> -bool admin_edge;
> +bool admin_edge OVS_GUARDED_BY(rstp_mutex);
>
> /* [17.13.3 - AutoEdge]
> * The AutoEdgePort parameter for the Port (14.8.2).
> */
> -bool auto_edge;
> +bool auto_edge OVS_GUARDED_BY(rstp_mutex);
>
>
>
> /*
> @@ -327,18 +329,18 @@ struct rstp_port {
> /* Port number and priority
> * >=1 (max 12 bits [9.2.7])
> */
> -uint16_t port_number;
> +uint16_t port_number OVS_GUARDED_BY(rstp_mutex);
>
> /* Port priority
> * Range: 0-240 in steps of 16 (table 17-2)
> */
> -uint8_t priority;
> +uint8_t priority OVS_GUARDED_BY(rstp_mutex);
>
> /* [17.13.11 - PortPathCost]
> * The Po
[ovs-dev] [PATCH v6 13/18] lib/rstp: More robust thread safety.
Current code expects there to be a single thread that is responsible
for creating rstp and creating and deleting rstp_port objects. rstp
objects are also deleted from other threads, as managed by reference
counting.
rstp port objects are not reference counted, which means that
references to rstp ports may only be held while holding the rstp
mutex, or by the thread that creates and deletes them.
This patch adds reference counting to RSTP ports, which allows ports
to be passed from ofproto-dpif to ofproto-dpif-xlate without using the
RSTP port number. This simplifies RSTP port reconfiguration, as the
port need not be resynchronized with xlate if just the port number
changes. This also avoids lookups on the processing of RSTP BPDUs.
This patch also:
1. Exposes the rstp mutex so that related thread safety annotations
can be used also within rstp-state-machines.c.
2. Internal variants of most setter an getter functions are defined,
suffixed with two underscores. These are annotated to be callable
only when the mutex is held.
3. Port setters were only called in a specific pattern. The new external
port setter combines them in a single rspt_port_set() function.
Signed-off-by: Jarno Rajahalme
---
lib/rstp-common.h| 202 +-
lib/rstp-state-machines.c| 111 --
lib/rstp-state-machines.h| 12 +-
lib/rstp.c | 884 +-
lib/rstp.h | 174 ++---
ofproto/ofproto-dpif-xlate.c | 53 +--
ofproto/ofproto-dpif-xlate.h |3 +-
ofproto/ofproto-dpif.c | 90 ++---
tests/test-rstp.c| 36 +-
9 files changed, 931 insertions(+), 634 deletions(-)
diff --git a/lib/rstp-common.h b/lib/rstp-common.h
index a67115b..6852263 100644
--- a/lib/rstp-common.h
+++ b/lib/rstp-common.h
@@ -261,10 +261,12 @@ enum rstp_rcvd_info {
};
struct rstp_port {
-struct rstp *rstp;
-struct list node; /* Node in rstp->ports list. */
-void *aux;
-struct rstp_bpdu received_bpdu_buffer;
+struct ovs_refcount ref_cnt;
+
+struct rstp *rstp OVS_GUARDED_BY(rstp_mutex);
+struct list node OVS_GUARDED_BY(rstp_mutex); /* Node in rstp->ports list.
*/
+void *aux OVS_GUARDED_BY(rstp_mutex);
+struct rstp_bpdu received_bpdu_buffer OVS_GUARDED_BY(rstp_mutex);
/*
* MAC status parameters
/
@@ -273,10 +275,10 @@ struct rstp_port {
* to transmit and/or receive frames, and its use is permitted by
* management.
*/
-bool mac_operational;
+bool mac_operational OVS_GUARDED_BY(rstp_mutex);
/* [14.8.2.2] Administrative Bridge Port State */
-bool is_administrative_bridge_port;
+bool is_administrative_bridge_port OVS_GUARDED_BY(rstp_mutex);
/* [6.4.3 - operPointToPointMAC]
* a) True. The MAC is connected to a point-to-point LAN; i.e., there is
@@ -288,7 +290,7 @@ struct rstp_port {
* shall be set True. If adminPointToPointMAC is set to ForceFalse, then
* operPointToPointMAC shall be set False.
*/
-bool oper_point_to_point_mac;
+bool oper_point_to_point_mac OVS_GUARDED_BY(rstp_mutex);
/* [6.4.3 - adminPointToPointMAC]
* a) ForceTrue. The administrator requires the MAC to be treated as if it
@@ -301,7 +303,7 @@ struct rstp_port {
* MAC to be determined in accordance with the specific MAC procedures
* defined in 6.5.
*/
-enum rstp_admin_point_to_point_mac_state admin_point_to_point_mac;
+enum rstp_admin_point_to_point_mac_state admin_point_to_point_mac
OVS_GUARDED_BY(rstp_mutex);
/*
@@ -312,12 +314,12 @@ struct rstp_port {
/* [17.13.1 - Admin Edge Port]
* The AdminEdgePort parameter for the Port (14.8.2).
*/
-bool admin_edge;
+bool admin_edge OVS_GUARDED_BY(rstp_mutex);
/* [17.13.3 - AutoEdge]
* The AutoEdgePort parameter for the Port (14.8.2).
*/
-bool auto_edge;
+bool auto_edge OVS_GUARDED_BY(rstp_mutex);
/*
@@ -327,18 +329,18 @@ struct rstp_port {
/* Port number and priority
* >=1 (max 12 bits [9.2.7])
*/
-uint16_t port_number;
+uint16_t port_number OVS_GUARDED_BY(rstp_mutex);
/* Port priority
* Range: 0-240 in steps of 16 (table 17-2)
*/
-uint8_t priority;
+uint8_t priority OVS_GUARDED_BY(rstp_mutex);
/* [17.13.11 - PortPathCost]
* The Port's contribution, when it is the Root Port, to the Root Path Cost
* (17.3.1, 17.5, 17.6) for the Bridge.
*/
-uint32_t port_path_cost;
+uint32_t port_path_cost OVS_GUARDED_BY(rstp_mutex);
/*
