Project

General

Profile

Actions

Bug #5027

closed

logging_gsmtap.c code not filling PID field in pkt header

Added by pespin about 3 years ago. Updated about 3 years ago.

Status:
Resolved
Priority:
Normal
Assignee:
Category:
-
Target version:
-
Start date:
02/16/2021
Due date:
% Done:

100%

Spec Reference:

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

Related to libosmocore - Feature #5032: Add VTY option to write TID in log line prefixResolvedpespin02/17/2021

Actions
Actions #1

Updated by laforge about 3 years ago

  • Status changed from New to Resolved
  • % Done changed from 0 to 100

patch merged

Actions #2

Updated by laforge about 3 years ago

  • Status changed from Resolved to Stalled
  • % Done changed from 100 to 80
Actions #3

Updated by pespin about 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

TODO:
  • use new osmo_gettid() API in logging_gsmtap.c
Actions #4

Updated by pespin about 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.

Actions #5

Updated by pespin about 3 years ago

  • Status changed from Feedback to Resolved
  • % Done changed from 90 to 100

Merged, closing.

Actions #6

Updated by pespin about 3 years ago

  • Related to Feature #5032: Add VTY option to write TID in log line prefix added
Actions

Also available in: Atom PDF

Add picture from clipboard (Maximum size: 48.8 MB)