[XL] Change in osmocom-bb[master]: {trxcon,virt_phy}: shared GPRS L1 (MAC) implementation

Attention is currently required from: laforge, pespin. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/30743 ) Change subject: {trxcon,virt_phy}: shared GPRS L1 (MAC) implementation ...................................................................... Patch Set 11: (5 comments) File include/l1ctl_proto.h: https://gerrit.osmocom.org/c/osmocom-bb/+/30743/comment/4395d2b1_3a6490a1 PS10, Line 367: uint8_t padding[3]; I already tried to explain and additionally brought this topic up in the IRC. Let's please avoid spending even more time discussing this. Padding is already present in existing L1CTL messages, so it will be in the new ones too. File src/host/virt_phy/src/virt_prim_pdch.c: https://gerrit.osmocom.org/c/osmocom-bb/+/30743/comment/942ed7ba_34ff2bc0 PS10, Line 34: void l1ctl_rx_gprs_uldl_tbf_cfg_req(struct l1_model_ms *ms, struct msgb *msg) I think it's fine to have a single function for handling both L1CTL messages, because a) we want to check `ms->gprs` against NULL, and b) we need to `pull()` the `l1ctl_hdr`. This is common for both messages and having two separate functions would be code duplication. The two different functions below are "external" ones from l1gprs. So I disagree that it's "totally unnecesary" and prefer to do it this way. It's not like I am mixing up all GPRS related L1CTL messages here, just the two related ones. https://gerrit.osmocom.org/c/osmocom-bb/+/30743/comment/7f2ed3a0_7fad25de PS10, Line 69: if (OSMO_UNLIKELY(fn_sched != req.hdr.fn)) { No. The frame number must match, otherwise we're sending stuff earlier or later then requested, what is unacceptable in GPRS. We're violating this and not returning early below, but this is rather a temporary hack. https://gerrit.osmocom.org/c/osmocom-bb/+/30743/comment/67a04d89_0da714d1 PS10, Line 73: /* FIXME: return; */ Well, at the moment we always hit this block. So if we return, all UL blocks will be discarded. The root problem needs to be fixed first. Either virtphy is scheduling stuff in advance, or the upper layers are sending block too late. For now it's a FIXME. File src/shared/l1gprs.c: https://gerrit.osmocom.org/c/osmocom-bb/+/30743/comment/ac1f7c60_60c68060 PS10, Line 83: static struct l1gprs_tbf *l1gprs_find_tbf(struct l1gprs_state *gprs, What do we win by having two different lists? -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/30743 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: I9567d64f9d00262e36147e8d7e541e5e246bda5f Gerrit-Change-Number: 30743 Gerrit-PatchSet: 11 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Thu, 16 Mar 2023 14:28:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment