* [PATCH BlueZ 1/4] monitor: Fix median packet size
@ 2021-08-06 21:35 Luiz Augusto von Dentz
2021-08-06 21:35 ` [PATCH BlueZ 2/4] monitor: Fix minimun packet latency Luiz Augusto von Dentz
` (3 more replies)
0 siblings, 4 replies; 5+ messages in thread
From: Luiz Augusto von Dentz @ 2021-08-06 21:35 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Calculating the median packet based on the current median + size / 2
does not account for last packet could smaller e.g. opp transfer could
end with just 1 byte which would cut the median in a half, so this
switch to more traditional means of calculating by doing total bytes
sent / num of packets so each an every packet has the same weight.
---
monitor/analyze.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/monitor/analyze.c b/monitor/analyze.c
index 839c2f7e9..5e0957ad1 100644
--- a/monitor/analyze.c
+++ b/monitor/analyze.c
@@ -61,6 +61,7 @@ struct hci_conn {
unsigned long rx_num;
unsigned long tx_num;
unsigned long tx_num_comp;
+ size_t tx_bytes;
struct timeval last_tx;
struct timeval last_tx_comp;
struct timeval tx_lat_min;
@@ -99,6 +100,8 @@ static void conn_destroy(void *data)
break;
}
+ conn->tx_pkt_med = conn->tx_bytes / conn->tx_num;
+
printf(" Found %s connection with handle %u\n", str, conn->handle);
printf(" BD_ADDR %2.2X:%2.2X:%2.2X:%2.2X:%2.2X:%2.2X\n",
conn->bdaddr[5], conn->bdaddr[4], conn->bdaddr[3],
@@ -485,16 +488,12 @@ static void acl_pkt(struct timeval *tv, uint16_t index, bool out,
if (out) {
conn->tx_num++;
conn->last_tx = *tv;
+ conn->tx_bytes += size;
if (!conn->tx_pkt_min || size < conn->tx_pkt_min)
conn->tx_pkt_min = size;
if (!conn->tx_pkt_max || size > conn->tx_pkt_max)
conn->tx_pkt_max = size;
- if (conn->tx_pkt_med) {
- conn->tx_pkt_med += (size + 1);
- conn->tx_pkt_med /= 2;
- } else
- conn->tx_pkt_med = size;
} else {
conn->rx_num++;
}
--
2.31.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH BlueZ 2/4] monitor: Fix minimun packet latency
2021-08-06 21:35 [PATCH BlueZ 1/4] monitor: Fix median packet size Luiz Augusto von Dentz
@ 2021-08-06 21:35 ` Luiz Augusto von Dentz
2021-08-06 21:35 ` [PATCH BlueZ 3/4] monitor: Fix not accouting for multiple outstanding packets Luiz Augusto von Dentz
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Luiz Augusto von Dentz @ 2021-08-06 21:35 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
It seems timer_sub can produce negative values leading to median packet
latency to be negative e.g conn->last_tx_compl is ahead of
conn->last_tx, in which case it should be discarded.
---
monitor/analyze.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/monitor/analyze.c b/monitor/analyze.c
index 5e0957ad1..d504c8d84 100644
--- a/monitor/analyze.c
+++ b/monitor/analyze.c
@@ -386,8 +386,9 @@ static void evt_num_completed_packets(struct hci_dev *dev, struct timeval *tv,
if (timerisset(&conn->last_tx)) {
timersub(&conn->last_tx_comp, &conn->last_tx, &res);
- if (!timerisset(&conn->tx_lat_min) ||
- timercmp(&res, &conn->tx_lat_min, <))
+ if ((!timerisset(&conn->tx_lat_min) ||
+ timercmp(&res, &conn->tx_lat_min, <)) &&
+ res.tv_sec >= 0 && res.tv_usec >= 0)
conn->tx_lat_min = res;
if (!timerisset(&conn->tx_lat_max) ||
@@ -408,6 +409,8 @@ static void evt_num_completed_packets(struct hci_dev *dev, struct timeval *tv,
tmp.tv_usec -= 1000000;
}
}
+
+ conn->tx_lat_med = tmp;
} else
conn->tx_lat_med = res;
--
2.31.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH BlueZ 3/4] monitor: Fix not accouting for multiple outstanding packets
2021-08-06 21:35 [PATCH BlueZ 1/4] monitor: Fix median packet size Luiz Augusto von Dentz
2021-08-06 21:35 ` [PATCH BlueZ 2/4] monitor: Fix minimun packet latency Luiz Augusto von Dentz
@ 2021-08-06 21:35 ` Luiz Augusto von Dentz
2021-08-06 21:35 ` [PATCH BlueZ 4/4] monitor: Make --analyze output latencies in msec Luiz Augusto von Dentz
2021-08-06 21:57 ` [BlueZ,1/4] monitor: Fix median packet size bluez.test.bot
3 siblings, 0 replies; 5+ messages in thread
From: Luiz Augusto von Dentz @ 2021-08-06 21:35 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Analyze code was not accounting for the fact that multiple outstanding
packets could be pending which will cause the last_tx to be overwritten
but its latency would be calculated against the very first packet
complete.
---
monitor/analyze.c | 20 +++++++++++++-------
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/monitor/analyze.c b/monitor/analyze.c
index d504c8d84..aae153f94 100644
--- a/monitor/analyze.c
+++ b/monitor/analyze.c
@@ -62,8 +62,7 @@ struct hci_conn {
unsigned long tx_num;
unsigned long tx_num_comp;
size_t tx_bytes;
- struct timeval last_tx;
- struct timeval last_tx_comp;
+ struct queue *tx_queue;
struct timeval tx_lat_min;
struct timeval tx_lat_max;
struct timeval tx_lat_med;
@@ -121,6 +120,7 @@ static void conn_destroy(void *data)
printf(" %u octets TX max packet size\n", conn->tx_pkt_max);
printf(" %u octets TX median packet size\n", conn->tx_pkt_med);
+ queue_destroy(conn->tx_queue, free);
free(conn);
}
@@ -133,6 +133,7 @@ static struct hci_conn *conn_alloc(struct hci_dev *dev, uint16_t handle,
conn->handle = handle;
conn->type = type;
+ conn->tx_queue = queue_new();
return conn;
}
@@ -372,6 +373,7 @@ static void evt_num_completed_packets(struct hci_dev *dev, struct timeval *tv,
uint16_t count = get_le16(data + 2);
struct hci_conn *conn;
struct timeval res;
+ struct timeval *last_tx;
data += 4;
size -= 4;
@@ -381,10 +383,10 @@ static void evt_num_completed_packets(struct hci_dev *dev, struct timeval *tv,
continue;
conn->tx_num_comp += count;
- conn->last_tx_comp = *tv;
- if (timerisset(&conn->last_tx)) {
- timersub(&conn->last_tx_comp, &conn->last_tx, &res);
+ last_tx = queue_pop_head(conn->tx_queue);
+ if (last_tx) {
+ timersub(tv, last_tx, &res);
if ((!timerisset(&conn->tx_lat_min) ||
timercmp(&res, &conn->tx_lat_min, <)) &&
@@ -414,7 +416,7 @@ static void evt_num_completed_packets(struct hci_dev *dev, struct timeval *tv,
} else
conn->tx_lat_med = res;
- timerclear(&conn->last_tx);
+ free(last_tx);
}
}
}
@@ -489,8 +491,12 @@ static void acl_pkt(struct timeval *tv, uint16_t index, bool out,
return;
if (out) {
+ struct timeval *last_tx;
+
conn->tx_num++;
- conn->last_tx = *tv;
+ last_tx = new0(struct timeval, 1);
+ memcpy(last_tx, tv, sizeof(*tv));
+ queue_push_tail(conn->tx_queue, last_tx);
conn->tx_bytes += size;
if (!conn->tx_pkt_min || size < conn->tx_pkt_min)
--
2.31.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH BlueZ 4/4] monitor: Make --analyze output latencies in msec
2021-08-06 21:35 [PATCH BlueZ 1/4] monitor: Fix median packet size Luiz Augusto von Dentz
2021-08-06 21:35 ` [PATCH BlueZ 2/4] monitor: Fix minimun packet latency Luiz Augusto von Dentz
2021-08-06 21:35 ` [PATCH BlueZ 3/4] monitor: Fix not accouting for multiple outstanding packets Luiz Augusto von Dentz
@ 2021-08-06 21:35 ` Luiz Augusto von Dentz
2021-08-06 21:57 ` [BlueZ,1/4] monitor: Fix median packet size bluez.test.bot
3 siblings, 0 replies; 5+ messages in thread
From: Luiz Augusto von Dentz @ 2021-08-06 21:35 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
Milisecconds is probably the best unit to have since it is unlikely that
the controller can respond in under 1 msec as well as most time
sensitive connection e.g. A2DP, HFP, etc, also don't expect the
latencies to be over 1 sec.
---
monitor/analyze.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/monitor/analyze.c b/monitor/analyze.c
index aae153f94..bee05f467 100644
--- a/monitor/analyze.c
+++ b/monitor/analyze.c
@@ -110,12 +110,15 @@ static void conn_destroy(void *data)
printf(" %lu RX packets\n", conn->rx_num);
printf(" %lu TX packets\n", conn->tx_num);
printf(" %lu TX completed packets\n", conn->tx_num_comp);
- printf(" %ld.%06ld seconds min latency\n",
- conn->tx_lat_min.tv_sec, conn->tx_lat_min.tv_usec);
- printf(" %ld.%06ld seconds max latency\n",
- conn->tx_lat_max.tv_sec, conn->tx_lat_max.tv_usec);
- printf(" %ld.%06ld seconds median latency\n",
- conn->tx_lat_med.tv_sec, conn->tx_lat_med.tv_usec);
+ printf(" %ld msec min latency\n",
+ conn->tx_lat_min.tv_sec * 1000 +
+ conn->tx_lat_min.tv_usec / 1000);
+ printf(" %ld msec max latency\n",
+ conn->tx_lat_max.tv_sec * 1000 +
+ conn->tx_lat_max.tv_usec / 1000);
+ printf(" %ld msec median latency\n",
+ conn->tx_lat_med.tv_sec * 1000 +
+ conn->tx_lat_med.tv_usec / 1000);
printf(" %u octets TX min packet size\n", conn->tx_pkt_min);
printf(" %u octets TX max packet size\n", conn->tx_pkt_max);
printf(" %u octets TX median packet size\n", conn->tx_pkt_med);
--
2.31.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* RE: [BlueZ,1/4] monitor: Fix median packet size
2021-08-06 21:35 [PATCH BlueZ 1/4] monitor: Fix median packet size Luiz Augusto von Dentz
` (2 preceding siblings ...)
2021-08-06 21:35 ` [PATCH BlueZ 4/4] monitor: Make --analyze output latencies in msec Luiz Augusto von Dentz
@ 2021-08-06 21:57 ` bluez.test.bot
3 siblings, 0 replies; 5+ messages in thread
From: bluez.test.bot @ 2021-08-06 21:57 UTC (permalink / raw)
To: linux-bluetooth, luiz.dentz
[-- Attachment #1: Type: text/plain, Size: 1953 bytes --]
This is automated email and please do not reply to this email!
Dear submitter,
Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=527765
---Test result---
Test Summary:
CheckPatch PASS 0.91 seconds
GitLint PASS 0.49 seconds
Prep - Setup ELL PASS 47.25 seconds
Build - Prep PASS 0.10 seconds
Build - Configure PASS 8.43 seconds
Build - Make PASS 202.22 seconds
Make Check PASS 9.20 seconds
Make Distcheck PASS 241.47 seconds
Build w/ext ELL - Configure PASS 8.36 seconds
Build w/ext ELL - Make PASS 196.00 seconds
Details
##############################
Test: CheckPatch - PASS
Desc: Run checkpatch.pl script with rule in .checkpatch.conf
##############################
Test: GitLint - PASS
Desc: Run gitlint with rule in .gitlint
##############################
Test: Prep - Setup ELL - PASS
Desc: Clone, build, and install ELL
##############################
Test: Build - Prep - PASS
Desc: Prepare environment for build
##############################
Test: Build - Configure - PASS
Desc: Configure the BlueZ source tree
##############################
Test: Build - Make - PASS
Desc: Build the BlueZ source tree
##############################
Test: Make Check - PASS
Desc: Run 'make check'
##############################
Test: Make Distcheck - PASS
Desc: Run distcheck to check the distribution
##############################
Test: Build w/ext ELL - Configure - PASS
Desc: Configure BlueZ source with '--enable-external-ell' configuration
##############################
Test: Build w/ext ELL - Make - PASS
Desc: Build BlueZ source with '--enable-external-ell' configuration
---
Regards,
Linux Bluetooth
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-08-06 21:57 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-06 21:35 [PATCH BlueZ 1/4] monitor: Fix median packet size Luiz Augusto von Dentz
2021-08-06 21:35 ` [PATCH BlueZ 2/4] monitor: Fix minimun packet latency Luiz Augusto von Dentz
2021-08-06 21:35 ` [PATCH BlueZ 3/4] monitor: Fix not accouting for multiple outstanding packets Luiz Augusto von Dentz
2021-08-06 21:35 ` [PATCH BlueZ 4/4] monitor: Make --analyze output latencies in msec Luiz Augusto von Dentz
2021-08-06 21:57 ` [BlueZ,1/4] monitor: Fix median packet size bluez.test.bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).