Attention is currently required from: fixeria.

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

Change subject: libosmo-trx/ep: add TRX endpoint module
......................................................................


Patch Set 1:

(4 comments)

File libosmo-trx/include/osmocom/trx/ep.h:

https://gerrit.osmocom.org/c/osmo-trx/+/43111/comment/8209a7f2_c432ddb6?usp=email
 :
PS1, Line 27: struct osmo_trx_ep_cfg {
Let's please have separate setter functions for all these, like osmo_stream, so 
we don't need to break ABI in the future.


File libosmo-trx/src/trx_ep.c:

https://gerrit.osmocom.org/c/osmo-trx/+/43111/comment/dd48d19b_8fc72c33?usp=email
 :
PS1, Line 60:   uint8_t pdu_ver;                /* TRXD PDU version in use */
iirc PDU VER stuff is defined/envisioned only for TRX0, so this can be moved to 
struct osmo_trx_ep.


https://gerrit.osmocom.org/c/osmo-trx/+/43111/comment/896192e5_e31a77da?usp=email
 :
PS1, Line 70:   struct osmo_trx_ep_chan *chans; /* array of cfg.num_chans 
channels */
If the chans array is allocated at the end of the struct, it probably makes 
sense to use "struct osmo_trx_ep_chan chans[]" here?


https://gerrit.osmocom.org/c/osmo-trx/+/43111/comment/ab46a500_80719b45?usp=email
 :
PS1, Line 302:  ep->chans = talloc_zero_array(ep, struct osmo_trx_ep_chan, 
cfg->num_chans);
Ah I see you are allocating the array here as a pointer to a seaprate memory 
region.
It probably makes sense to allocate it at the end just by defining it with the 
array [0] (see comment at the start of the file), and then doing:

ep = talloc_zero(ctx, sizeof(struct osmo_trx_ep) + sizeof(struct 
osmo_trx_ep_chan) * cfg->num_chans);

This way you avoid fragmenting memory and probably already preload mem cache of 
the channels, which may be useful here for performance of trxd. Up to you.



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

Gerrit-MessageType: comment
Gerrit-Project: osmo-trx
Gerrit-Branch: master
Gerrit-Change-Id: I767fa43a9ca88be40c385f6dcb9de22891a6afc3
Gerrit-Change-Number: 43111
Gerrit-PatchSet: 1
Gerrit-Owner: fixeria <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: pespin <[email protected]>
Gerrit-Attention: fixeria <[email protected]>
Gerrit-Comment-Date: Tue, 28 Jul 2026 11:24:25 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No

Reply via email to