Bug #5027
closedlogging_gsmtap.c code not filling PID field in pkt header
100%
Description
It was recently discovered that the PID field was always sent as 0.
This first patch fixes the issue:
https://gerrit.osmocom.org/c/libosmocore/+/22920
During patch review, a discussion was started on possible improvements.
The idea is to instead of getpid(), fill it gettid() to be able to distinguish threads.
gettid() support in system is sometimes a bit flacky. osmo-trx already solves that by implemeting its own wrapper "my_gettid" based on autoconf detection in debug.c.
We should move that "my_gettid" implementation as "osmo_gettid()" and put it in some "thread_compat.h" in libosmocore, similar to what we do with "timer_compat.h" timespec operations.
Then, in libosmocore's logging_gsmtap.c, change the static global variable logging_gsmtap_pid to be __thread, and update it each time _gsmtap_raw_output() is called if value is 0.
Finally, remove the "my_gettid" from osmo-trx and use the libosmocore one.
Related issues
Updated by laforge over 3 years ago
- Status changed from New to Resolved
- % Done changed from 0 to 100
patch merged
Updated by laforge over 3 years ago
- Status changed from Resolved to Stalled
- % Done changed from 100 to 80
Updated by pespin over 3 years ago
osmo_gettid() added to libosmocore:
https://gerrit.osmocom.org/c/libosmocore/+/22949 Introduce osmo_gettid() API
osmo-trx using the new libosmocore API:
https://gerrit.osmocom.org/c/osmo-trx/+/22950 Replace my_gettid with libosmocore osmo_gettid API
- use new osmo_gettid() API in logging_gsmtap.c
Updated by pespin over 3 years ago
- Status changed from Stalled to Feedback
- % Done changed from 80 to 90
I fixed PID not being filled as big endian:
https://gerrit.osmocom.org/c/libosmocore/+/22951 logging: gsmtap: Fix fill PID field not stored in network byte order
Then, patch submitted to use TID instead of PID (did a few tests with osmo-trx and it looks good):
https://gerrit.osmocom.org/c/libosmocore/+/22952 logging: gsmtap: Store TID instead of PID in pkt hdr
Once those are merged, we can close the ticket.
Updated by pespin over 3 years ago
- Status changed from Feedback to Resolved
- % Done changed from 90 to 100
Merged, closing.
Updated by pespin over 3 years ago
- Related to Feature #5032: Add VTY option to write TID in log line prefix added