Merge pull request #1615 from qlyoung/fix-bgpd-atomic-usage

bgpd: use atomic_* ops on _Atomic variables

bgpd/bgp_fsm.c

@@ -264,14 +264,16 @@ static struct peer *peer_xfer_conn(struct peer *from_peer)
 		}
 	}
 
+
+	// Note: peer_xfer_stats() must be called with I/O turned OFF
+	if (from_peer)
+		peer_xfer_stats(peer, from_peer);
+
 	bgp_reads_on(peer);
 	bgp_writes_on(peer);
 	thread_add_timer_msec(bm->master, bgp_process_packet, peer, 0,
 			      &peer->t_process_packet);
 
-	if (from_peer)
-		peer_xfer_stats(peer, from_peer);
-
 	return (peer);
 }
 

bgpd/bgp_packet.c

@@ -595,6 +595,7 @@ static int bgp_write_notify(struct peer *peer)
 	assert(type == BGP_MSG_NOTIFY);
 
 	/* Type should be notify. */
+	atomic_fetch_add_explicit(&peer->notify_out, 1, memory_order_relaxed);
 	peer->notify_out++;
 
 	/* Double start timer. */
@@ -1682,7 +1683,7 @@ static int bgp_notify_receive(struct peer *peer, bgp_size_t size)
 	}
 
 	/* peer count update */
-	peer->notify_in++;
+	atomic_fetch_add_explicit(&peer->notify_in, 1, memory_order_relaxed);
 
 	peer->last_reset = PEER_DOWN_NOTIFY_RECEIVED;
 
@@ -2192,7 +2193,8 @@ int bgp_process_packet(struct thread *thread)
 		 */
 		switch (type) {
 		case BGP_MSG_OPEN:
-			peer->open_in++;
+			atomic_fetch_add_explicit(&peer->open_in, 1,
+						  memory_order_relaxed);
 			mprc = bgp_open_receive(peer, size);
 			if (mprc == BGP_Stop)
 				zlog_err(
@@ -2200,7 +2202,8 @@ int bgp_process_packet(struct thread *thread)
 					__FUNCTION__, peer->host);
 			break;
 		case BGP_MSG_UPDATE:
-			peer->update_in++;
+			atomic_fetch_add_explicit(&peer->update_in, 1,
+						  memory_order_relaxed);
 			peer->readtime = monotime(NULL);
 			mprc = bgp_update_receive(peer, size);
 			if (mprc == BGP_Stop)
@@ -2209,7 +2212,8 @@ int bgp_process_packet(struct thread *thread)
 					__FUNCTION__, peer->host);
 			break;
 		case BGP_MSG_NOTIFY:
-			peer->notify_in++;
+			atomic_fetch_add_explicit(&peer->notify_in, 1,
+						  memory_order_relaxed);
 			mprc = bgp_notify_receive(peer, size);
 			if (mprc == BGP_Stop)
 				zlog_err(
@@ -2218,7 +2222,8 @@ int bgp_process_packet(struct thread *thread)
 			break;
 		case BGP_MSG_KEEPALIVE:
 			peer->readtime = monotime(NULL);
-			peer->keepalive_in++;
+			atomic_fetch_add_explicit(&peer->keepalive_in, 1,
+						  memory_order_relaxed);
 			mprc = bgp_keepalive_receive(peer, size);
 			if (mprc == BGP_Stop)
 				zlog_err(
@@ -2227,7 +2232,8 @@ int bgp_process_packet(struct thread *thread)
 			break;
 		case BGP_MSG_ROUTE_REFRESH_NEW:
 		case BGP_MSG_ROUTE_REFRESH_OLD:
-			peer->refresh_in++;
+			atomic_fetch_add_explicit(&peer->refresh_in, 1,
+						  memory_order_relaxed);
 			mprc = bgp_route_refresh_receive(peer, size);
 			if (mprc == BGP_Stop)
 				zlog_err(
@@ -2235,7 +2241,8 @@ int bgp_process_packet(struct thread *thread)
 					__FUNCTION__, peer->host);
 			break;
 		case BGP_MSG_CAPABILITY:
-			peer->dynamic_cap_in++;
+			atomic_fetch_add_explicit(&peer->dynamic_cap_in, 1,
+						  memory_order_relaxed);
 			mprc = bgp_capability_receive(peer, size);
 			if (mprc == BGP_Stop)
 				zlog_err(

bgpd/bgp_snmp.c

@@ -512,6 +512,7 @@ static u_char *bgpPeerTable(struct variable *v, oid name[], size_t *length,
 {
 	static struct in_addr addr;
 	struct peer *peer;
+	uint32_t ui, uo;
 
 	if (smux_header_table(v, name, length, exact, var_len, write_method)
 	    == MATCH_FAILED)
@@ -571,21 +572,20 @@ static u_char *bgpPeerTable(struct variable *v, oid name[], size_t *length,
 		return SNMP_INTEGER(peer->as);
 		break;
 	case BGPPEERINUPDATES:
-		return SNMP_INTEGER(peer->update_in);
+		ui = atomic_load_explicit(&peer->update_in,
+					  memory_order_relaxed);
+		return SNMP_INTEGER(ui);
 		break;
 	case BGPPEEROUTUPDATES:
-		return SNMP_INTEGER(peer->update_out);
+		uo = atomic_load_explicit(&peer->update_out,
+					  memory_order_relaxed);
+		return SNMP_INTEGER(uo);
 		break;
 	case BGPPEERINTOTALMESSAGES:
-		return SNMP_INTEGER(peer->open_in + peer->update_in
+		return SNMP_INTEGER(PEER_TOTAL_RX(peer));
-				    + peer->keepalive_in + peer->notify_in
-				    + peer->refresh_in + peer->dynamic_cap_in);
 		break;
 	case BGPPEEROUTTOTALMESSAGES:
-		return SNMP_INTEGER(peer->open_out + peer->update_out
+		return SNMP_INTEGER(PEER_TOTAL_TX(peer));
-				    + peer->keepalive_out + peer->notify_out
-				    + peer->refresh_out
-				    + peer->dynamic_cap_out);
 		break;
 	case BGPPEERLASTERROR: {
 		static u_char lasterror[2];

bgpd/bgp_vty.c

@@ -7053,17 +7053,9 @@ static int bgp_show_summary(struct vty *vty, struct bgp *bgp, int afi, int safi,
 			json_object_int_add(json_peer, "remoteAs", peer->as);
 			json_object_int_add(json_peer, "version", 4);
 			json_object_int_add(json_peer, "msgRcvd",
-					    peer->open_in + peer->update_in
+					    PEER_TOTAL_RX(peer));
-						    + peer->keepalive_in
-						    + peer->notify_in
-						    + peer->refresh_in
-						    + peer->dynamic_cap_in);
 			json_object_int_add(json_peer, "msgSent",
-					    peer->open_out + peer->update_out
+					    PEER_TOTAL_TX(peer));
-						    + peer->keepalive_out
-						    + peer->notify_out
-						    + peer->refresh_out
-						    + peer->dynamic_cap_out);
 
 			json_object_int_add(json_peer, "tableVersion",
 					    peer->version[afi][safi]);
@@ -7120,42 +7112,9 @@ static int bgp_show_summary(struct vty *vty, struct bgp *bgp, int afi, int safi,
 					" ");
 
 			vty_out(vty, "4 %10u %7u %7u %8" PRIu64 " %4d %4zd %8s",
-				peer->as,
+				peer->as, PEER_TOTAL_RX(peer),
-				atomic_load_explicit(&peer->open_in,
+				PEER_TOTAL_TX(peer), peer->version[afi][safi],
-						     memory_order_relaxed)
+				0, peer->obuf->count,
-					+ atomic_load_explicit(
-						  &peer->update_in,
-						  memory_order_relaxed)
-					+ atomic_load_explicit(
-						  &peer->keepalive_in,
-						  memory_order_relaxed)
-					+ atomic_load_explicit(
-						  &peer->notify_in,
-						  memory_order_relaxed)
-					+ atomic_load_explicit(
-						  &peer->refresh_in,
-						  memory_order_relaxed)
-					+ atomic_load_explicit(
-						  &peer->dynamic_cap_in,
-						  memory_order_relaxed),
-				atomic_load_explicit(&peer->open_out,
-						     memory_order_relaxed)
-					+ atomic_load_explicit(
-						  &peer->update_out,
-						  memory_order_relaxed)
-					+ atomic_load_explicit(
-						  &peer->keepalive_out,
-						  memory_order_relaxed)
-					+ atomic_load_explicit(
-						  &peer->notify_out,
-						  memory_order_relaxed)
-					+ atomic_load_explicit(
-						  &peer->refresh_out,
-						  memory_order_relaxed)
-					+ atomic_load_explicit(
-						  &peer->dynamic_cap_out,
-						  memory_order_relaxed),
-				peer->version[afi][safi], 0, peer->obuf->count,
 				peer_uptime(peer->uptime, timebuf,
 					    BGP_UPTIME_LEN, 0, NULL));
 
@@ -9329,34 +9288,44 @@ static void bgp_show_peer(struct vty *vty, struct peer *p, u_char use_json,
 		json_object_int_add(json_stat, "depthInq", 0);
 		json_object_int_add(json_stat, "depthOutq",
 				    (unsigned long)p->obuf->count);
-		json_object_int_add(json_stat, "opensSent", p->open_out);
+		json_object_int_add(json_stat, "opensSent",
-		json_object_int_add(json_stat, "opensRecv", p->open_in);
+				    atomic_load_explicit(&p->open_out,
+							 memory_order_relaxed));
+		json_object_int_add(json_stat, "opensRecv",
+				    atomic_load_explicit(&p->open_in,
+							 memory_order_relaxed));
 		json_object_int_add(json_stat, "notificationsSent",
-				    p->notify_out);
+				    atomic_load_explicit(&p->notify_out,
+							 memory_order_relaxed));
 		json_object_int_add(json_stat, "notificationsRecv",
-				    p->notify_in);
+				    atomic_load_explicit(&p->notify_in,
-		json_object_int_add(json_stat, "updatesSent", p->update_out);
+							 memory_order_relaxed));
-		json_object_int_add(json_stat, "updatesRecv", p->update_in);
+		json_object_int_add(json_stat, "updatesSent",
+				    atomic_load_explicit(&p->update_out,
+							 memory_order_relaxed));
+		json_object_int_add(json_stat, "updatesRecv",
+				    atomic_load_explicit(&p->update_in,
+							 memory_order_relaxed));
 		json_object_int_add(json_stat, "keepalivesSent",
-				    p->keepalive_out);
+				    atomic_load_explicit(&p->keepalive_out,
+							 memory_order_relaxed));
 		json_object_int_add(json_stat, "keepalivesRecv",
-				    p->keepalive_in);
+				    atomic_load_explicit(&p->keepalive_in,
+							 memory_order_relaxed));
 		json_object_int_add(json_stat, "routeRefreshSent",
-				    p->refresh_out);
+				    atomic_load_explicit(&p->refresh_out,
+							 memory_order_relaxed));
 		json_object_int_add(json_stat, "routeRefreshRecv",
-				    p->refresh_in);
+				    atomic_load_explicit(&p->refresh_in,
+							 memory_order_relaxed));
 		json_object_int_add(json_stat, "capabilitySent",
-				    p->dynamic_cap_out);
+				    atomic_load_explicit(&p->dynamic_cap_out,
+							 memory_order_relaxed));
 		json_object_int_add(json_stat, "capabilityRecv",
-				    p->dynamic_cap_in);
+				    atomic_load_explicit(&p->dynamic_cap_in,
-		json_object_int_add(json_stat, "totalSent",
+							 memory_order_relaxed));
-				    p->open_out + p->notify_out + p->update_out
+		json_object_int_add(json_stat, "totalSent", PEER_TOTAL_TX(p));
-					    + p->keepalive_out + p->refresh_out
+		json_object_int_add(json_stat, "totalRecv", PEER_TOTAL_RX(p));
-					    + p->dynamic_cap_out);
-		json_object_int_add(json_stat, "totalRecv",
-				    p->open_in + p->notify_in + p->update_in
-					    + p->keepalive_in + p->refresh_in
-					    + p->dynamic_cap_in);
 		json_object_object_add(json_neigh, "messageStats", json_stat);
 	} else {
 		/* Packet counts. */
@@ -9365,25 +9334,38 @@ static void bgp_show_peer(struct vty *vty, struct peer *p, u_char use_json,
 		vty_out(vty, "    Outq depth is %lu\n",
 			(unsigned long)p->obuf->count);
 		vty_out(vty, "                         Sent       Rcvd\n");
-		vty_out(vty, "    Opens:         %10d %10d\n", p->open_out,
+		vty_out(vty, "    Opens:         %10d %10d\n",
-			p->open_in);
+			atomic_load_explicit(&p->open_out,
-		vty_out(vty, "    Notifications: %10d %10d\n", p->notify_out,
+					     memory_order_relaxed),
-			p->notify_in);
+			atomic_load_explicit(&p->open_in,
-		vty_out(vty, "    Updates:       %10d %10d\n", p->update_out,
+					     memory_order_relaxed));
-			p->update_in);
+		vty_out(vty, "    Notifications: %10d %10d\n",
-		vty_out(vty, "    Keepalives:    %10d %10d\n", p->keepalive_out,
+			atomic_load_explicit(&p->notify_out,
-			p->keepalive_in);
+					     memory_order_relaxed),
-		vty_out(vty, "    Route Refresh: %10d %10d\n", p->refresh_out,
+			atomic_load_explicit(&p->notify_in,
-			p->refresh_in);
+					     memory_order_relaxed));
+		vty_out(vty, "    Updates:       %10d %10d\n",
+			atomic_load_explicit(&p->update_out,
+					     memory_order_relaxed),
+			atomic_load_explicit(&p->update_in,
+					     memory_order_relaxed));
+		vty_out(vty, "    Keepalives:    %10d %10d\n",
+			atomic_load_explicit(&p->keepalive_out,
+					     memory_order_relaxed),
+			atomic_load_explicit(&p->keepalive_in,
+					     memory_order_relaxed));
+		vty_out(vty, "    Route Refresh: %10d %10d\n",
+			atomic_load_explicit(&p->refresh_out,
+					     memory_order_relaxed),
+			atomic_load_explicit(&p->refresh_in,
+					     memory_order_relaxed));
 		vty_out(vty, "    Capability:    %10d %10d\n",
-			p->dynamic_cap_out, p->dynamic_cap_in);
+			atomic_load_explicit(&p->dynamic_cap_out,
-		vty_out(vty, "    Total:         %10d %10d\n",
+					     memory_order_relaxed),
-			p->open_out + p->notify_out + p->update_out
+			atomic_load_explicit(&p->dynamic_cap_in,
-				+ p->keepalive_out + p->refresh_out
+					     memory_order_relaxed));
-				+ p->dynamic_cap_out,
+		vty_out(vty, "    Total:         %10d %10d\n", PEER_TOTAL_TX(p),
-			p->open_in + p->notify_in + p->update_in
+			PEER_TOTAL_RX(p));
-				+ p->keepalive_in + p->refresh_in
-				+ p->dynamic_cap_in);
 	}
 
 	if (use_json) {

bgpd/bgpd.h

@@ -829,6 +829,22 @@ struct peer {
 	/* workqueues */
 	struct work_queue *clear_node_queue;
 
+#define PEER_TOTAL_RX(peer) \
+	atomic_load_explicit(&peer->open_in, memory_order_relaxed) +		\
+	atomic_load_explicit(&peer->update_in, memory_order_relaxed) +		\
+	atomic_load_explicit(&peer->notify_in, memory_order_relaxed) +		\
+	atomic_load_explicit(&peer->refresh_in, memory_order_relaxed) +		\
+	atomic_load_explicit(&peer->keepalive_in, memory_order_relaxed) +	\
+	atomic_load_explicit(&peer->dynamic_cap_in, memory_order_relaxed)
+
+#define PEER_TOTAL_TX(peer) \
+	atomic_load_explicit(&peer->open_out, memory_order_relaxed) +		\
+	atomic_load_explicit(&peer->update_out, memory_order_relaxed) +		\
+	atomic_load_explicit(&peer->notify_out, memory_order_relaxed) +		\
+	atomic_load_explicit(&peer->refresh_out, memory_order_relaxed) +	\
+	atomic_load_explicit(&peer->keepalive_out, memory_order_relaxed) +	\
+	atomic_load_explicit(&peer->dynamic_cap_out, memory_order_relaxed)
+
 	/* Statistics field */
 	_Atomic uint32_t open_in;         /* Open message input count */
 	_Atomic uint32_t open_out;        /* Open message output count */