[S] Change in osmo-e1d[master]: usb: Deal with truncated ISO IN transfers
laforge has submitted this change. (
https://gerrit.osmocom.org/c/osmo-e1d/+/27579?usp=email )
Change subject: usb: Deal with truncated ISO IN transfers
..
usb: Deal with truncated ISO IN transfers
It seems that in some circumstances, an ISO IN transfer can be
truncated by the bus / host. In such situation we'd currently pass
a non-modulo-32 length to the mux_demux (deframer) code, and it ASSERTs
on that. Let's try to handle this more gracefully by substituting
random garbage and letting higher layers deal with massive bit errors.
Related: OS#5490
Change-Id: Ic453325b93b0e12727625a1495a948d96df4b542
---
M src/e1d.h
M src/intf_line.c
M src/usb.c
3 files changed, 45 insertions(+), 0 deletions(-)
Approvals:
manawyrm: Looks good to me, approved
Jenkins Builder: Verified
diff --git a/src/e1d.h b/src/e1d.h
index 1c7c4c4..45ecb82 100644
--- a/src/e1d.h
+++ b/src/e1d.h
@@ -59,6 +59,7 @@
LINE_CTR_RX_REMOTE_A,
LINE_CTR_FRAMES_MUXED_E1T,
LINE_CTR_FRAMES_DEMUXED_E1O,
+ LINE_CTR_USB_ISO_TRUNC,
};
enum e1d_line_stat_item {
diff --git a/src/intf_line.c b/src/intf_line.c
index 3895653..f41214e 100644
--- a/src/intf_line.c
+++ b/src/intf_line.c
@@ -60,6 +60,7 @@
[LINE_CTR_RX_REMOTE_A] ={ "rx:remote_alarm","Rx Frames
Reporting Remote Alarm"},
[LINE_CTR_FRAMES_MUXED_E1T] = { "tx:frames_muxed", "E1 Tx Frames
multiplexed" },
[LINE_CTR_FRAMES_DEMUXED_E1O] = { "rx:frames_demuxed", "E1 Rx Frames
demultiplexed" },
+ [LINE_CTR_USB_ISO_TRUNC] = { "rx:usb_iso_trunc","USB ISO
packets truncated" },
};
static const struct rate_ctr_group_desc line_ctrg_desc = {
diff --git a/src/usb.c b/src/usb.c
index 1ff9b43..fcffc12 100644
--- a/src/usb.c
+++ b/src/usb.c
@@ -131,6 +131,33 @@
{
if (len == 0)
return 0;
+
+ if (len < 4) {
+ LOGPLI(flow->line, DE1D, LOGL_ERROR, "IN EP %02x ISO packet
truncated: len = %u\n",
+ flow->ep, len);
+ line_ctr_add(flow->line, LINE_CTR_USB_ISO_TRUNC, 1);
+ return 0;
+ }
+
+ if (len > 4 && (len - 4) % 32) {
+ /* some ISO IN packet was truncated. Apparently this
+* does happen, see https://osmocom.org/issues/5490 -
+* there is little we can do here, but instead of the
+* earlier ASSERT, we just feed some garbage for the
+* last few timeslots, resulting in bit errors etc. */
+ LOGPLI(flow->line, DE1D, LOGL_ERROR, "IN EP %02x ISO packet
truncated: len-4 = %u\n",
+ flow->ep, len - 4);
+ line_ctr_add(flow->line, LINE_CTR_USB_ISO_TRUNC, 1);
+
+ /* The assumption here is that only the last E1 frame is
+* truncated, as we have no idea how many E1 frames the
+* USB device firmware wanted to send us. */
+ len += 32 - (len % 32);
+ /* don't overflow the underlying buffer */
+ if (len > (int) size)
+ len = size;
+ }
+
return e1_line_demux_in(flow->line, buf + 4, len - 4, buf[3] & 0xf);
}
--
To view, visit https://gerrit.osmocom.org/c/osmo-e1d/+/27579?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-e1d
Gerrit-Branch: master
Gerrit-Change-Id: Ic453325b93b0e12727625a1495a948d96df4b542
Gerrit-Change-Number: 27579
Gerrit-PatchSet: 6
Gerrit-Owner: laforge
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge
Gerrit-Reviewer: manawyrm
Gerrit-Reviewer: tnt
Gerrit-MessageType: merged
[S] Change in osmo-e1d[master]: usb: Deal with truncated ISO IN transfers
Attention is currently required from: laforge, tnt. manawyrm has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-e1d/+/27579?usp=email ) Change subject: usb: Deal with truncated ISO IN transfers .. Patch Set 5: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-e1d/+/27579?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-e1d Gerrit-Branch: master Gerrit-Change-Id: Ic453325b93b0e12727625a1495a948d96df4b542 Gerrit-Change-Number: 27579 Gerrit-PatchSet: 5 Gerrit-Owner: laforge Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: manawyrm Gerrit-Reviewer: tnt Gerrit-Attention: laforge Gerrit-Attention: tnt Gerrit-Comment-Date: Sun, 17 Dec 2023 14:11:02 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[S] Change in osmo-e1d[master]: usb: Deal with truncated ISO IN transfers
Attention is currently required from: manawyrm, tnt. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-e1d/+/27579?usp=email ) Change subject: usb: Deal with truncated ISO IN transfers .. Patch Set 5: (1 comment) Patchset: PS1: > ok, will see if I can push it down into the callback. This has been implemented in the more recent versions of the patch and marked as resolve. Is there still anything else preventing approval? -- To view, visit https://gerrit.osmocom.org/c/osmo-e1d/+/27579?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-e1d Gerrit-Branch: master Gerrit-Change-Id: Ic453325b93b0e12727625a1495a948d96df4b542 Gerrit-Change-Number: 27579 Gerrit-PatchSet: 5 Gerrit-Owner: laforge Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: manawyrm Gerrit-Reviewer: tnt Gerrit-Attention: manawyrm Gerrit-Attention: tnt Gerrit-Comment-Date: Sun, 17 Dec 2023 13:30:43 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: laforge Comment-In-Reply-To: tnt Gerrit-MessageType: comment
[S] Change in osmo-e1d[master]: usb: Deal with truncated ISO IN transfers
Attention is currently required from: laforge, manawyrm, tnt. Hello Jenkins Builder, manawyrm, tnt, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-e1d/+/27579?usp=email to look at the new patch set (#5). The following approvals got outdated and were removed: Verified-1 by Jenkins Builder Change subject: usb: Deal with truncated ISO IN transfers .. usb: Deal with truncated ISO IN transfers It seems that in some circumstances, an ISO IN transfer can be truncated by the bus / host. In such situation we'd currently pass a non-modulo-32 length to the mux_demux (deframer) code, and it ASSERTs on that. Let's try to handle this more gracefully by substituting random garbage and letting higher layers deal with massive bit errors. Related: OS#5490 Change-Id: Ic453325b93b0e12727625a1495a948d96df4b542 --- M src/e1d.h M src/intf_line.c M src/usb.c 3 files changed, 45 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-e1d refs/changes/79/27579/5 -- To view, visit https://gerrit.osmocom.org/c/osmo-e1d/+/27579?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-e1d Gerrit-Branch: master Gerrit-Change-Id: Ic453325b93b0e12727625a1495a948d96df4b542 Gerrit-Change-Number: 27579 Gerrit-PatchSet: 5 Gerrit-Owner: laforge Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: manawyrm Gerrit-Reviewer: tnt Gerrit-Attention: manawyrm Gerrit-Attention: laforge Gerrit-Attention: tnt Gerrit-MessageType: newpatchset
[S] Change in osmo-e1d[master]: usb: Deal with truncated ISO IN transfers
Attention is currently required from: manawyrm, tnt.
Jenkins Builder has posted comments on this change. (
https://gerrit.osmocom.org/c/osmo-e1d/+/27579?usp=email )
Change subject: usb: Deal with truncated ISO IN transfers
..
Patch Set 4:
(1 comment)
File src/intf_line.c:
Robot Comment from checkpatch (run ID jenkins-gerrit-lint-12185):
https://gerrit.osmocom.org/c/osmo-e1d/+/27579/comment/ebe55d76_34061d59
PS4, Line 63: [LINE_CTR_USB_ISO_TRUNC] = { "rx:usb_iso_trunc","USB
ISO packets truncated" },
please, no space before tabs
--
To view, visit https://gerrit.osmocom.org/c/osmo-e1d/+/27579?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-e1d
Gerrit-Branch: master
Gerrit-Change-Id: Ic453325b93b0e12727625a1495a948d96df4b542
Gerrit-Change-Number: 27579
Gerrit-PatchSet: 4
Gerrit-Owner: laforge
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: manawyrm
Gerrit-Reviewer: tnt
Gerrit-Attention: manawyrm
Gerrit-Attention: tnt
Gerrit-Comment-Date: Mon, 30 Oct 2023 11:36:09 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
[S] Change in osmo-e1d[master]: usb: Deal with truncated ISO IN transfers
Attention is currently required from: manawyrm, tnt. Hello Jenkins Builder, manawyrm, tnt, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-e1d/+/27579?usp=email to look at the new patch set (#4). The following approvals got outdated and were removed: Verified+1 by Jenkins Builder Change subject: usb: Deal with truncated ISO IN transfers .. usb: Deal with truncated ISO IN transfers It seems that in some circumstances, an ISO IN transfer can be truncated by the bus / host. In such situation we'd currently pass a non-modulo-32 length to the mux_demux (deframer) code, and it ASSERTs on that. Let's try to handle this more gracefully by substituting random garbage and letting higher layers deal with massive bit errors. Related: OS#5490 Change-Id: Ic453325b93b0e12727625a1495a948d96df4b542 --- M src/e1d.h M src/intf_line.c M src/usb.c 3 files changed, 45 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-e1d refs/changes/79/27579/4 -- To view, visit https://gerrit.osmocom.org/c/osmo-e1d/+/27579?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-e1d Gerrit-Branch: master Gerrit-Change-Id: Ic453325b93b0e12727625a1495a948d96df4b542 Gerrit-Change-Number: 27579 Gerrit-PatchSet: 4 Gerrit-Owner: laforge Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: manawyrm Gerrit-Reviewer: tnt Gerrit-Attention: manawyrm Gerrit-Attention: tnt Gerrit-MessageType: newpatchset
[S] Change in osmo-e1d[master]: usb: Deal with truncated ISO IN transfers
Attention is currently required from: manawyrm, tnt. Hello Jenkins Builder, manawyrm, tnt, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-e1d/+/27579?usp=email to look at the new patch set (#3). The following approvals got outdated and were removed: Verified+1 by Jenkins Builder Change subject: usb: Deal with truncated ISO IN transfers .. usb: Deal with truncated ISO IN transfers It seems that in some circumstances, an ISO IN transfer can be truncated by the bus / host. In such situation we'd currently pass a non-modulo-32 length to the mux_demux (deframer) code, and it ASSERTs on that. Let's try to handle this more gracefully by substituting random garbage and letting higher layers deal with massive bit errors. Related: OS#5490 Change-Id: Ic453325b93b0e12727625a1495a948d96df4b542 --- M src/usb.c 1 file changed, 41 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-e1d refs/changes/79/27579/3 -- To view, visit https://gerrit.osmocom.org/c/osmo-e1d/+/27579?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-e1d Gerrit-Branch: master Gerrit-Change-Id: Ic453325b93b0e12727625a1495a948d96df4b542 Gerrit-Change-Number: 27579 Gerrit-PatchSet: 3 Gerrit-Owner: laforge Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: manawyrm Gerrit-Reviewer: tnt Gerrit-Attention: manawyrm Gerrit-Attention: tnt Gerrit-MessageType: newpatchset
[S] Change in osmo-e1d[master]: usb: Deal with truncated ISO IN transfers
Attention is currently required from: manawyrm, tnt. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-e1d/+/27579?usp=email ) Change subject: usb: Deal with truncated ISO IN transfers .. Patch Set 2: (1 comment) Patchset: PS2: > This still isn't merged, but it probably should be. […] it should be pushed into the flow->cb callback instead of being in the generic code path. also, we probably want a rate_ctr for observability. And possibly a rate limit to give up if this happens too frequently. -- To view, visit https://gerrit.osmocom.org/c/osmo-e1d/+/27579?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-e1d Gerrit-Branch: master Gerrit-Change-Id: Ic453325b93b0e12727625a1495a948d96df4b542 Gerrit-Change-Number: 27579 Gerrit-PatchSet: 2 Gerrit-Owner: laforge Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: manawyrm Gerrit-Reviewer: tnt Gerrit-Attention: manawyrm Gerrit-Attention: tnt Gerrit-Comment-Date: Wed, 25 Oct 2023 08:30:13 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: manawyrm Gerrit-MessageType: comment
[S] Change in osmo-e1d[master]: usb: Deal with truncated ISO IN transfers
Attention is currently required from: laforge, tnt. manawyrm has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-e1d/+/27579 ) Change subject: usb: Deal with truncated ISO IN transfers .. Patch Set 2: (1 comment) Patchset: PS2: This still isn't merged, but it probably should be. I don't feel qualified to comment on USB changes (as I have absolutely 0 knowledge about USB), but I've been running this change for a while now and it seems to work fine. -- To view, visit https://gerrit.osmocom.org/c/osmo-e1d/+/27579 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-e1d Gerrit-Branch: master Gerrit-Change-Id: Ic453325b93b0e12727625a1495a948d96df4b542 Gerrit-Change-Number: 27579 Gerrit-PatchSet: 2 Gerrit-Owner: laforge Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: manawyrm Gerrit-Reviewer: tnt Gerrit-Attention: laforge Gerrit-Attention: tnt Gerrit-Comment-Date: Mon, 31 Jul 2023 12:52:16 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
