Change in ...osmo-trx[master]: lms: Drop rx_underruns rate ctr, add tx_drop_* rate ctr

pespin gerrit-no-reply at lists.osmocom.org
Thu Aug 1 12:04:21 UTC 2019


pespin has submitted this change and it was merged. ( https://gerrit.osmocom.org/c/osmo-trx/+/14989 )

Change subject: lms: Drop rx_underruns rate ctr, add tx_drop_* rate ctr
......................................................................

lms: Drop rx_underruns rate ctr, add tx_drop_* rate ctr

After discussion in [1] and further look at the code, it became obvios
rx_underrun events are not happening in general for any SDR (don't
exist), so let's drop that counter. Instead, add Tx Dropped Packet counters,
which were not accounted prior to this commit.

[1] https://github.com/osmocom/osmo-trx/commit/bde55afd29fc9aae10eb11f6515821afa39b772d

Change-Id: Iff1535c219a4695a511d383d7c4b06ef6eff959d
---
M CommonLibs/osmo_signal.h
M CommonLibs/trx_rate_ctr.cpp
M CommonLibs/trx_rate_ctr.h
M CommonLibs/trx_vty.c
M Transceiver52M/device/lms/LMSDevice.cpp
M Transceiver52M/device/lms/LMSDevice.h
M doc/manuals/chapters/counters_generated.adoc
M doc/manuals/vty/trx_vty_reference.xml
8 files changed, 105 insertions(+), 54 deletions(-)

Approvals:
  daniel: Looks good to me, but someone else must approve
  pespin: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/CommonLibs/osmo_signal.h b/CommonLibs/osmo_signal.h
index ee7e2a4..ceb7d6f 100644
--- a/CommonLibs/osmo_signal.h
+++ b/CommonLibs/osmo_signal.h
@@ -48,9 +48,10 @@
 /* signal cb for signal <SS_DEVICE,S_DEVICE_COUNTER_CHANGE> */
 struct device_counters {
 	size_t chan;
-	unsigned int rx_underruns;
 	unsigned int rx_overruns;
 	unsigned int tx_underruns;
 	unsigned int rx_dropped_events;
 	unsigned int rx_dropped_samples;
+	unsigned int tx_dropped_events;
+	unsigned int tx_dropped_samples;
 };
diff --git a/CommonLibs/trx_rate_ctr.cpp b/CommonLibs/trx_rate_ctr.cpp
index 43e4189..a9ef88c 100644
--- a/CommonLibs/trx_rate_ctr.cpp
+++ b/CommonLibs/trx_rate_ctr.cpp
@@ -93,20 +93,22 @@
 };
 
 const struct value_string trx_chan_ctr_names[] = {
-	{ TRX_CTR_RX_UNDERRUNS,	"rx_underruns" },
 	{ TRX_CTR_RX_OVERRUNS,	"rx_overruns" },
 	{ TRX_CTR_TX_UNDERRUNS,	"tx_underruns" },
 	{ TRX_CTR_RX_DROP_EV,	"rx_drop_events" },
 	{ TRX_CTR_RX_DROP_SMPL,	"rx_drop_samples" },
+	{ TRX_CTR_TX_DROP_EV,	"tx_drop_events" },
+	{ TRX_CTR_TX_DROP_SMPL,	"tx_drop_samples" },
 	{ 0, NULL }
 };
 
 static const struct rate_ctr_desc trx_chan_ctr_desc[] = {
-	[TRX_CTR_RX_UNDERRUNS]		= { "device:rx_underruns",	"Number of Rx underruns" },
-	[TRX_CTR_RX_OVERRUNS]		= { "device:rx_overruns",	"Number of Rx overruns" },
-	[TRX_CTR_TX_UNDERRUNS]		= { "device:tx_underruns",	"Number of Tx underruns" },
+	[TRX_CTR_RX_OVERRUNS]		= { "device:rx_overruns",	"Number of Rx overruns in FIFO queue" },
+	[TRX_CTR_TX_UNDERRUNS]		= { "device:tx_underruns",	"Number of Tx underruns in FIFO queue" },
 	[TRX_CTR_RX_DROP_EV]		= { "device:rx_drop_events",	"Number of times Rx samples were dropped by HW" },
 	[TRX_CTR_RX_DROP_SMPL]		= { "device:rx_drop_samples",	"Number of Rx samples dropped by HW" },
+	[TRX_CTR_TX_DROP_EV]		= { "device:tx_drop_events",	"Number of times Tx samples were dropped by HW" },
+	[TRX_CTR_TX_DROP_SMPL]		= { "device:tx_drop_samples",	"Number of Tx samples dropped by HW" }
 };
 
 static const struct rate_ctr_group_desc trx_chan_ctr_group_desc = {
@@ -126,8 +128,6 @@
 		if (ctrs_pending[chan].chan == PENDING_CHAN_NONE)
 			continue;
 		LOGCHAN(chan, DMAIN, INFO) << "rate_ctr update";
-		ctr = &rate_ctrs[chan]->ctr[TRX_CTR_RX_UNDERRUNS];
-		rate_ctr_add(ctr, ctrs_pending[chan].rx_underruns - ctr->current);
 		ctr = &rate_ctrs[chan]->ctr[TRX_CTR_RX_OVERRUNS];
 		rate_ctr_add(ctr, ctrs_pending[chan].rx_overruns - ctr->current);
 		ctr = &rate_ctrs[chan]->ctr[TRX_CTR_TX_UNDERRUNS];
@@ -136,6 +136,10 @@
 		rate_ctr_add(ctr, ctrs_pending[chan].rx_dropped_events - ctr->current);
 		ctr = &rate_ctrs[chan]->ctr[TRX_CTR_RX_DROP_SMPL];
 		rate_ctr_add(ctr, ctrs_pending[chan].rx_dropped_samples - ctr->current);
+		ctr = &rate_ctrs[chan]->ctr[TRX_CTR_TX_DROP_EV];
+		rate_ctr_add(ctr, ctrs_pending[chan].tx_dropped_events - ctr->current);
+		ctr = &rate_ctrs[chan]->ctr[TRX_CTR_TX_DROP_SMPL];
+		rate_ctr_add(ctr, ctrs_pending[chan].tx_dropped_samples - ctr->current);
 
 		/* Mark as done */
 		ctrs_pending[chan].chan = PENDING_CHAN_NONE;
diff --git a/CommonLibs/trx_rate_ctr.h b/CommonLibs/trx_rate_ctr.h
index 6e4fa4d..155f413 100644
--- a/CommonLibs/trx_rate_ctr.h
+++ b/CommonLibs/trx_rate_ctr.h
@@ -4,11 +4,12 @@
 #include <osmocom/vty/command.h>
 
 enum TrxCtr {
-	TRX_CTR_RX_UNDERRUNS,
 	TRX_CTR_RX_OVERRUNS,
 	TRX_CTR_TX_UNDERRUNS,
 	TRX_CTR_RX_DROP_EV,
 	TRX_CTR_RX_DROP_SMPL,
+	TRX_CTR_TX_DROP_EV,
+	TRX_CTR_TX_DROP_SMPL,
 };
 
 struct ctr_threshold {
diff --git a/CommonLibs/trx_vty.c b/CommonLibs/trx_vty.c
index e184f49..bac9653 100644
--- a/CommonLibs/trx_vty.c
+++ b/CommonLibs/trx_vty.c
@@ -384,14 +384,15 @@
 	return -1;
 }
 
-#define THRESHOLD_ARGS "(rx_underruns|rx_overruns|tx_underruns|rx_drop_events|rx_drop_samples)"
+#define THRESHOLD_ARGS "(rx_overruns|tx_underruns|rx_drop_events|rx_drop_samples|tx_drop_events|tx_drop_samples)"
 #define THRESHOLD_STR_VAL(s) "Set threshold value for rate_ctr device:" OSMO_STRINGIFY_VAL(s) "\n"
 #define THRESHOLD_STRS \
-	THRESHOLD_STR_VAL(rx_underruns) \
 	THRESHOLD_STR_VAL(rx_overruns) \
 	THRESHOLD_STR_VAL(tx_underruns) \
 	THRESHOLD_STR_VAL(rx_drop_events) \
-	THRESHOLD_STR_VAL(rx_drop_samples)
+	THRESHOLD_STR_VAL(rx_drop_samples) \
+	THRESHOLD_STR_VAL(tx_drop_events) \
+	THRESHOLD_STR_VAL(tx_drop_samples)
 #define INTV_ARGS "(per-second|per-minute|per-hour|per-day)"
 #define INTV_STR_VAL(s) "Threshold value sampled " OSMO_STRINGIFY_VAL(s) "\n"
 #define INTV_STRS \
diff --git a/Transceiver52M/device/lms/LMSDevice.cpp b/Transceiver52M/device/lms/LMSDevice.cpp
index 0eaf0a3..f55b555 100644
--- a/Transceiver52M/device/lms/LMSDevice.cpp
+++ b/Transceiver52M/device/lms/LMSDevice.cpp
@@ -574,38 +574,37 @@
 	/* UNUSED on limesdr (only used on usrp1/2) */
 	return GSM::Time(0,0);
 }
-
-void LMSDevice::update_stream_stats(size_t chan, bool * underrun, bool * overrun)
+/*!
+ * Issue tracking description of several events: https://github.com/myriadrf/LimeSuite/issues/265
+ */
+void LMSDevice::update_stream_stats_rx(size_t chan, bool *overrun)
 {
 	lms_stream_status_t status;
 	bool changed = false;
 
 	if (LMS_GetStreamStatus(&m_lms_stream_rx[chan], &status) != 0) {
-		LOGCHAN(chan, DDEV, ERROR) << "LMS_GetStreamStatus failed";
+		LOGCHAN(chan, DDEV, ERROR) << "Rx LMS_GetStreamStatus failed";
 		return;
 	}
 
-	if (status.underrun) {
-		changed = true;
-		*underrun = true;
-		LOGCHAN(chan, DDEV, ERROR) << "recv Underrun! ("
-					   << m_ctr[chan].rx_underruns << " -> "
-					   << status.underrun << ")";
-	}
-	m_ctr[chan].rx_underruns += status.underrun;
-
+	/* FIFO overrun is counted when Rx FIFO is full but new data comes from
+	   the board and oldest samples in FIFO are overwritte. Value count
+	   since the last call to LMS_GetStreamStatus(stream). */
 	if (status.overrun) {
 		changed = true;
 		*overrun = true;
-		LOGCHAN(chan, DDEV, ERROR) << "recv Overrun! ("
+		LOGCHAN(chan, DDEV, ERROR) << "Rx Overrun! ("
 					   << m_ctr[chan].rx_overruns << " -> "
 					   << status.overrun << ")";
 	}
 	m_ctr[chan].rx_overruns += status.overrun;
 
+	/* Dropped packets in Rx are counted when gaps in Rx timestamps are
+	   detected (likely because buffer oveflow in hardware). Value count
+	   since the last call to LMS_GetStreamStatus(stream). */
 	if (status.droppedPackets) {
 		changed = true;
-		LOGCHAN(chan, DDEV, ERROR) << "recv Dropped packets by HW! ("
+		LOGCHAN(chan, DDEV, ERROR) << "Rx Dropped packets by HW! ("
 					   << m_ctr[chan].rx_dropped_samples << " -> "
 					   << m_ctr[chan].rx_dropped_samples +
 					      status.droppedPackets
@@ -653,7 +652,7 @@
 		while ((avail_smpls = rx_buffers[i]->avail_smpls(timestamp)) < len) {
 			thread_enable_cancel(false);
 			num_smpls = LMS_RecvStream(&m_lms_stream_rx[i], bufs[i], len - avail_smpls, &rx_metadata, 100);
-			update_stream_stats(i, underrun, overrun);
+			update_stream_stats_rx(i, overrun);
 			thread_enable_cancel(true);
 			if (num_smpls <= 0) {
 				LOGCHAN(i, DDEV, ERROR) << "Device receive timed out (" << rc << " vs exp " << len << ").";
@@ -697,13 +696,53 @@
 	return len;
 }
 
+void LMSDevice::update_stream_stats_tx(size_t chan, bool *underrun)
+{
+	lms_stream_status_t status;
+	bool changed = false;
+
+	if (LMS_GetStreamStatus(&m_lms_stream_tx[chan], &status) != 0) {
+		LOGCHAN(chan, DDEV, ERROR) << "Tx LMS_GetStreamStatus failed";
+		return;
+	}
+
+	/* FIFO underrun is counted when Tx is running but FIFO is empty for
+	   >100 ms (500ms in older versions). Value count since the last call to
+	   LMS_GetStreamStatus(stream). */
+	if (status.underrun) {
+		changed = true;
+		*underrun = true;
+		LOGCHAN(chan, DDEV, ERROR) << "Tx Underrun! ("
+					   << m_ctr[chan].tx_underruns << " -> "
+					   << status.underrun << ")";
+	}
+	m_ctr[chan].tx_underruns += status.underrun;
+
+	/* Dropped packets in Tx are counted only when timestamps are enabled
+	   and SDR drops packet because of late timestamp. Value count since the
+	   last call to LMS_GetStreamStatus(stream). */
+	if (status.droppedPackets) {
+		changed = true;
+		LOGCHAN(chan, DDEV, ERROR) << "Tx Dropped packets by HW! ("
+					   << m_ctr[chan].tx_dropped_samples << " -> "
+					   << m_ctr[chan].tx_dropped_samples +
+					      status.droppedPackets
+					   << ")";
+		m_ctr[chan].tx_dropped_events++;
+	}
+	m_ctr[chan].tx_dropped_samples += status.droppedPackets;
+
+	if (changed)
+		osmo_signal_dispatch(SS_DEVICE, S_DEVICE_COUNTER_CHANGE, &m_ctr[chan]);
+
+}
+
 int LMSDevice::writeSamples(std::vector < short *>&bufs, int len,
 			    bool * underrun, unsigned long long timestamp,
 			    bool isControl)
 {
 	int rc = 0;
 	unsigned int i;
-	lms_stream_status_t status;
 	lms_stream_meta_t tx_metadata = {};
 	tx_metadata.flushPartialPacket = false;
 	tx_metadata.waitForTimestamp = true;
@@ -725,19 +764,12 @@
 		LOGCHAN(i, DDEV, DEBUG) << "send buffer of len " << len << " timestamp " << std::hex << tx_metadata.timestamp;
 		thread_enable_cancel(false);
 		rc = LMS_SendStream(&m_lms_stream_tx[i], bufs[i], len, &tx_metadata, 100);
-		if (rc != len) {
-			LOGCHAN(i, DDEV, ERROR) << "LMS: Device send timed out";
-		}
-
-		if (LMS_GetStreamStatus(&m_lms_stream_tx[i], &status) == 0) {
-			if (status.underrun > m_ctr[i].tx_underruns) {
-				*underrun = true;
-				m_ctr[i].tx_underruns = status.underrun;
-				osmo_signal_dispatch(SS_DEVICE, S_DEVICE_COUNTER_CHANGE, &m_ctr[i]);
-			}
-
-		}
+		update_stream_stats_tx(i, underrun);
 		thread_enable_cancel(true);
+		if (rc != len) {
+			LOGCHAN(i, DDEV, ERROR) << "LMS: Device Tx timed out (" << rc << " vs exp " << len << ").";
+			return -1;
+		}
 	}
 
 	return rc;
diff --git a/Transceiver52M/device/lms/LMSDevice.h b/Transceiver52M/device/lms/LMSDevice.h
index 906fbee..bc79f97 100644
--- a/Transceiver52M/device/lms/LMSDevice.h
+++ b/Transceiver52M/device/lms/LMSDevice.h
@@ -65,7 +65,8 @@
 	bool do_filters(size_t chan);
 	int get_ant_idx(const std::string & name, bool dir_tx, size_t chan);
 	bool flush_recv(size_t num_pkts);
-	void update_stream_stats(size_t chan, bool * underrun, bool * overrun);
+	void update_stream_stats_rx(size_t chan, bool *overrun);
+	void update_stream_stats_tx(size_t chan, bool *underrun);
 
 public:
 
diff --git a/doc/manuals/chapters/counters_generated.adoc b/doc/manuals/chapters/counters_generated.adoc
index 6955b18..98634ff 100644
--- a/doc/manuals/chapters/counters_generated.adoc
+++ b/doc/manuals/chapters/counters_generated.adoc
@@ -1,5 +1,6 @@
+
 // autogenerated by show asciidoc counters
-These counters and their description based on OsmoTRX 1.0.0.43-3f7c0 (OsmoTRX).
+These counters and their description are based on OsmoTRX 1.0.0.95-9527 (OsmoTRX).
 
 === Rate Counters
 
@@ -9,9 +10,17 @@
 [options="header"]
 |===
 | Name | Reference | Description
-| device:rx_underruns | <<trx:chan_device:rx_underruns>> | Number of Rx underruns
-| device:rx_overruns | <<trx:chan_device:rx_overruns>> | Number of Rx overruns
-| device:tx_underruns | <<trx:chan_device:tx_underruns>> | Number of Tx underruns
+| device:rx_overruns | <<trx:chan_device:rx_overruns>> | Number of Rx overruns in FIFO queue
+| device:tx_underruns | <<trx:chan_device:tx_underruns>> | Number of Tx underruns in FIFO queue
 | device:rx_drop_events | <<trx:chan_device:rx_drop_events>> | Number of times Rx samples were dropped by HW
 | device:rx_drop_samples | <<trx:chan_device:rx_drop_samples>> | Number of Rx samples dropped by HW
+| device:tx_drop_events | <<trx:chan_device:tx_drop_events>> | Number of times Tx samples were dropped by HW
+| device:tx_drop_samples | <<trx:chan_device:tx_drop_samples>> | Number of Tx samples dropped by HW
 |===
+== Osmo Stat Items
+
+// generating tables for osmo_stat_items
+== Osmo Counters
+
+// generating tables for osmo_counters
+// there are no ungrouped osmo_counters
diff --git a/doc/manuals/vty/trx_vty_reference.xml b/doc/manuals/vty/trx_vty_reference.xml
index ce6d335..38d64c1 100644
--- a/doc/manuals/vty/trx_vty_reference.xml
+++ b/doc/manuals/vty/trx_vty_reference.xml
@@ -1247,26 +1247,21 @@
         <param name='<1-32>' doc='Real time priority' />
       </params>
     </command>
-    <command id='stack-size <0-2147483647>'>
-      <params>
-        <param name='stack-size' doc='Set the stack size for the spawned threads' />
-        <param name='<0-2147483647>' doc='Stack size in BYTE' />
-      </params>
-    </command>
     <command id='filler dummy'>
       <params>
         <param name='filler' doc='Enable C0 filler table' />
         <param name='dummy' doc='Dummy method' />
       </params>
     </command>
-    <command id='ctr-error-threshold (rx_underruns|rx_overruns|tx_underruns|rx_drop_events|rx_drop_samples) <0-65535> (per-second|per-minute|per-hour|per-day)'>
+    <command id='ctr-error-threshold (rx_overruns|tx_underruns|rx_drop_events|rx_drop_samples|tx_drop_events|tx_drop_samples) <0-65535> (per-second|per-minute|per-hour|per-day)'>
       <params>
         <param name='ctr-error-threshold' doc='Threshold rate for error counter' />
-        <param name='rx_underruns' doc='Set threshold value for rate_ctr device:rx_underruns' />
         <param name='rx_overruns' doc='Set threshold value for rate_ctr device:rx_overruns' />
         <param name='tx_underruns' doc='Set threshold value for rate_ctr device:tx_underruns' />
         <param name='rx_drop_events' doc='Set threshold value for rate_ctr device:rx_drop_events' />
         <param name='rx_drop_samples' doc='Set threshold value for rate_ctr device:rx_drop_samples' />
+        <param name='tx_drop_events' doc='Set threshold value for rate_ctr device:tx_drop_events' />
+        <param name='tx_drop_samples' doc='Set threshold value for rate_ctr device:tx_drop_samples' />
         <param name='<0-65535>' doc='Value to set for threshold' />
         <param name='per-second' doc='Threshold value sampled per-second' />
         <param name='per-minute' doc='Threshold value sampled per-minute' />
@@ -1274,15 +1269,16 @@
         <param name='per-day' doc='Threshold value sampled per-day' />
       </params>
     </command>
-    <command id='no ctr-error-threshold (rx_underruns|rx_overruns|tx_underruns|rx_drop_events|rx_drop_samples) <0-65535> (per-second|per-minute|per-hour|per-day)'>
+    <command id='no ctr-error-threshold (rx_overruns|tx_underruns|rx_drop_events|rx_drop_samples|tx_drop_events|tx_drop_samples) <0-65535> (per-second|per-minute|per-hour|per-day)'>
       <params>
         <param name='no' doc='Negate a command or set its defaults' />
         <param name='ctr-error-threshold' doc='Threshold rate for error counter' />
-        <param name='rx_underruns' doc='Set threshold value for rate_ctr device:rx_underruns' />
         <param name='rx_overruns' doc='Set threshold value for rate_ctr device:rx_overruns' />
         <param name='tx_underruns' doc='Set threshold value for rate_ctr device:tx_underruns' />
         <param name='rx_drop_events' doc='Set threshold value for rate_ctr device:rx_drop_events' />
         <param name='rx_drop_samples' doc='Set threshold value for rate_ctr device:rx_drop_samples' />
+        <param name='tx_drop_events' doc='Set threshold value for rate_ctr device:tx_drop_events' />
+        <param name='tx_drop_samples' doc='Set threshold value for rate_ctr device:tx_drop_samples' />
         <param name='<0-65535>' doc='Value to set for threshold' />
         <param name='per-second' doc='Threshold value sampled per-second' />
         <param name='per-minute' doc='Threshold value sampled per-minute' />
@@ -1290,6 +1286,12 @@
         <param name='per-day' doc='Threshold value sampled per-day' />
       </params>
     </command>
+    <command id='stack-size <0-2147483647>'>
+      <params>
+        <param name='stack-size' doc='Set the stack size per thread in BYTE, 0 = OS default' />
+        <param name='<0-2147483647>' doc='Stack size per thread in BYTE' />
+      </params>
+    </command>
     <command id='chan <0-100>'>
       <params>
         <param name='chan' doc='Select a channel to configure' />

-- 
To view, visit https://gerrit.osmocom.org/c/osmo-trx/+/14989
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-trx
Gerrit-Branch: master
Gerrit-Change-Id: Iff1535c219a4695a511d383d7c4b06ef6eff959d
Gerrit-Change-Number: 14989
Gerrit-PatchSet: 3
Gerrit-Owner: pespin <pespin at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Vadim Yanitskiy <axilirator at gmail.com>
Gerrit-Reviewer: daniel <dwillmann at sysmocom.de>
Gerrit-Reviewer: laforge <laforge at gnumonks.org>
Gerrit-Reviewer: pespin <pespin at sysmocom.de>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20190801/6524b0ca/attachment.html>


More information about the gerrit-log mailing list