Attention is currently required from: pespin.

fixeria has posted comments on this change by pespin. ( 
https://gerrit.osmocom.org/c/osmo-bts/+/41650?usp=email )

Change subject: bts-trx: Convert TRXC and TRXD sockets to iofd
......................................................................


Patch Set 2: Code-Review-1

(8 comments)

File src/osmo-bts-trx/trx_if.c:

https://gerrit.osmocom.org/c/osmo-bts/+/41650/comment/06b43c48_e9f8dddc?usp=email
 :
PS2, Line 1298: trx_udp_open
I still think having a helper function is better to avoid code duplication and 
copy-paste errors.


https://gerrit.osmocom.org/c/osmo-bts/+/41650/comment/0835df08_e45de9b5?usp=email
 :
PS2, Line 147: msg->data_len
`msgb_length(msg)` here and below


https://gerrit.osmocom.org/c/osmo-bts/+/41650/comment/668e373a_bf35449b?usp=email
 :
PS2, Line 1006:                 LOGPPHI(l1h->phy_inst, DTRX, LOGL_ERROR,
              :
```suggestion
                LOGPPHI(l1h->phy_inst, DTRX, LOGL_ERROR,
                        "recv() failed on TRXD with rc=%d (%s)\n", res, errbuf);
```


https://gerrit.osmocom.org/c/osmo-bts/+/41650/comment/882f6715_116ca7c5?usp=email
 :
PS2, Line 1110: trx_if_send_burst
BTW, we may further improve this API to avoid `static` variables.

My idea is to have the following API (draft):

```
struct msgb *trx_if_data_msgb_alloc(void);
struct msgb *trx_if_data_msgb_push_br(struct msgb *msg,
                                      const struct trx_dl_burst_req *br);
struct msgb *trx_if_data_msgb_send(struct trx_l1h *l1h, struct msgb *msg);
```


https://gerrit.osmocom.org/c/osmo-bts/+/41650/comment/98753226_f4931c4c?usp=email
 :
PS2, Line 1128: buf = msgb_data(trx_data_last_msg);
setting `buf` here is probably not needed?


https://gerrit.osmocom.org/c/osmo-bts/+/41650/comment/78304e94_3a9e81d8?usp=email
 :
PS2, Line 1164:
```suggestion
                        msgb_put_u32(trx_data_last_msg, br->fn);
```


https://gerrit.osmocom.org/c/osmo-bts/+/41650/comment/bfef0671_718b4418?usp=email
 :
PS2, Line 1195:                 LOGPPHI(l1h->phy_inst, DTRX, LOGL_ERROR,
              :                         "send() failed on TRXD with rc=%d 
(%s)\n",
              :
We're no longer calling `send()` here, right?

```suggestion
                LOGPPHI(l1h->phy_inst, DTRX, LOGL_ERROR,
                        "osmo_iofd_write_msgb() failed on TRXD with rc=%d 
(%s)\n",
                        rc, errbuf);
```


https://gerrit.osmocom.org/c/osmo-bts/+/41650/comment/61735b68_bb54b0e5?usp=email
 :
PS2, Line 1319:         i
Copy-paste!

```suggestion
        if (!l1h->trx_data_iofd) {
```



--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/41650?usp=email
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings?usp=email

Gerrit-MessageType: comment
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: I239f91efad43eabd280caf9f852c3aefbc729eaf
Gerrit-Change-Number: 41650
Gerrit-PatchSet: 2
Gerrit-Owner: pespin <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <[email protected]>
Gerrit-Attention: pespin <[email protected]>
Gerrit-Comment-Date: Thu, 11 Dec 2025 18:44:23 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes

Reply via email to