Bug #5809
openapplications disrespect potentially VTY-configured tcp/telnet port
90%
Description
Originally we had the telnet_init() function, which later on was replaced by telnet_init_dynif() - both requiring the application to know the TCP port (and possibly local address)
Later we added VTY commands for this, and there is telnet_init_default() which uses vty_get_bind{port,addr}() to make sur the VTY-configured IP/port are used.
The problem is that most if not all applications still use telnet_init_dynif() to which they pass the "normal" port like OSMO_VTY_PORT_HNODEB.
This means the user may configure a different port in the VTY, but that port won't actually be used.
All the osmo-* projects need to be converted to telnet_init_default().
We should also [marc ase] deprecate the old telnet_init() and telnet_init_dynif() functions for exactly that reason.
Files
Checklist
- osmo-uecups
- osmo-trx
- osmo-smlc
- osmo-sip-connector
- osmo-sgsn
- osmo-pcap
- osmo-msc
- osmo-mgw
- osmo-iuh (tests)
- libosmo-sccp
- osmo-bsc
Updated by arehbein over 1 year ago
- Status changed from New to Feedback
- % Done changed from 0 to 90
Here's the list of repositories I created patches for (if they contained the function as a definition or call:)
- libosmocore
- osmo-bsc
- osmo-bts
- osmo-ccid-firmware
- osmocom-bb
- osmo-dev
- osmo-e1-hardware
- osmo-gbproxy
- osmo-gsm-manuals
- osmo-hlr
- osmo-asf4-dfu
- osmo-bsc-nat
- osmo-cbc
- osmo-ci
- osmocom-lcs
- osmo-e1d
- osmo-e1-recorder
- osmo-ggsn
- osmo-gsm-tester
- osmo-hnbgw
Updated by osmith over 1 year ago
Note: it's good practice to link related patches in the issue. In case there are a lot of patches like here, a topic name can be used.
Updated by arehbein over 1 year ago
osmith I was wondering about something like that, thanks for the hint! Is it possible to change the topic by pushing for the same Change-ID, but with another topic? Also, what would a good topic be... the issue number maybe (OS_5809) ?
Updated by msuraev over 1 year ago
arehbein wrote in #note-3:
Is it possible to change the topic by pushing for the same Change-ID, but with another topic?
Sure. You can change it through webUI. Or if you push via git-review it'll take branch name as a topic.
Updated by osmith over 1 year ago
arehbein wrote in #note-3:
Also, what would a good topic be... the issue number maybe (OS_5809) ?
That would work, or something more descriptive like telnet_init_default.
Updated by arehbein over 1 year ago
msuraev wrote in #note-4:
arehbein wrote in #note-3:
Is it possible to change the topic by pushing for the same Change-ID, but with another topic?
Sure. You can change it through webUI. Or if you pish via git-review it'll take branch name as a topic.
I tried but I suppose unless I change the patch itself, the server will reject pushing the same change with a different topic. Even for my second patchset on libosmocore it only worked because I used git push --no-thin .
Updated by arehbein over 1 year ago
pespin I corrected the patch https://gerrit.osmocom.org/c/libosmocore/+/30703 as requested by you, I suppose your (re-)review is needed since you put a '-1' (?)
Updated by arehbein about 1 year ago
- Status changed from Feedback to Resolved
- % Done changed from 90 to 100
Updated by fixeria about 1 year ago
- Status changed from Resolved to New
- % Done changed from 100 to 90
laforge wrote:
All the osmo-* projects need to be converted to
telnet_init_default().
Unfortunately, not all projects have been updated to use the telnet_init_default(). I am still seeing many -Wdeprecated-declarations warnings when building [lib]osmo-* repositories. Even though some (not all!) callers do invoke vty_get_bind_addr() rather than passing port values, it's generally better to have a clean buildlog without deprecation warnings. This way it's easier to spot new warnings. Reopening this ticket.
Updated by fixeria about 1 year ago
- Checklist item osmo-uecups added
- Checklist item osmo-trx added
- Checklist item osmo-smlc added
- Checklist item osmo-sip-connector added
- Checklist item osmo-sgsn added
- Checklist item osmo-pcap added
- Checklist item osmo-msc added
- Checklist item osmo-mgw added
- Checklist item osmo-iuh (tests) added
- Checklist item libosmo-sccp added
- % Done changed from 90 to 80
Adding a list of repositories which still need to be updated. There might be more.
Updated by arehbein about 1 year ago
- Status changed from New to In Progress
fixeria wrote in #note-10:
Adding a list of repositories which still need to be updated. There might be more.
Thanks for the list fixeria, I'm on it. I listed the projects I patched initially because I wasn't sure the list was complete.
I don't see some (or even all?) of the projects you listed on Gerrit (screens of my output of repositories listed to me attached). Where can I find a comprehensive list of all the repositories I have access to?
Edit: Sorry just noticed I can't upload files with reply-edits (?)...
Updated by arehbein about 1 year ago
- File gerrit_projects_screen_01.png gerrit_projects_screen_01.png added
- File gerrit_projects_screen_02.png gerrit_projects_screen_02.png added
- File gerrit_projects_screen_03.png gerrit_projects_screen_03.png added
- File gerrit_projects_screen_04.png gerrit_projects_screen_04.png added
A screen of list of projects I see on Gerrit is attached
Updated by neels about 1 year ago
Are you aware of the "Page 1 >" in the lower right of the gerrit repos browser?
A better listing of all osmo repositories was the legacy https://cgit.osmocom.org/
However some newer repositories are not being added in cgit anymore.
So on the web, you're stuck with clicking throught the repository pages in gerrit,
but then again not all repositories live in gerrit.
So on the web, you're stuck with clicking through gitea: https://gitea.osmocom.org/explore/repos
Not sure if all osmo repositories are mirrored to gitea (at least some of them sync only every 8 hours, i reconfigure to 15 minutes whenever i see one)
However, the list of important core network repositories, which I generally consider "all repositories I need to worry about", is a smaller subset. Some of use use the top-level makefile from osmo-dev.git, which clones and builds the osmo core network infrastructure, and I generally grep in all repositories that osmo-dev clones.
https://gitea.osmocom.org/osmocom/osmo-dev/src/branch/master/all.deps
(I find osmo-dev.git quite helpful and convenient, maybe you also like it.
You can clone it and check out the readme to get started.)
Updated by arehbein about 1 year ago
neels thanks. I don't know why I only skipped through the first two pages, today and back then, even though the arrow kept showing. The list in osmo-dev sounds useful for getting all names for the clones, thanks.
Updated by arehbein about 1 year ago
- Checklist item osmo-uecups set to Done
- Checklist item osmo-smlc set to Done
- Checklist item osmo-sip-connector set to Done
- Checklist item osmo-sgsn set to Done
- Checklist item osmo-pcap set to Done
- Checklist item osmo-msc set to Done
- Checklist item osmo-mgw set to Done
- Checklist item osmo-iuh (tests) set to Done
- Checklist item libosmo-sccp set to Done
- % Done changed from 80 to 90
I went through repositories from osmo-dev/all.deps and pushed fixes to topic 'os5809':
https://gerrit.osmocom.org/q/topic:os5809
osmo-trx fails on the endianness test. I tried to rebase to see if that's the problem but Gerrit tells me the branch is up to date. Not sure what the problem is, the endianness test isn't run as part of make check in osmo-trx itself, looks like something 'exclusive' from osmo-ci. I took a first look at the project but couldn't quickly find how exactly endianness tests are run/how I could run them to debug stuff live.
Running ./contrib/jenkins.sh from osmo-trx doesn't work for me, had to install dependencies and I stopped when one of the calls to configure inside the script kept asking for the package LimeSuite - installing llimesuite limesuite-dbgsym, then limesuite-udev via apt-get didn't work.
Maybe someone else knows what the problem is? General compilation seems to work out after all, and the jenkins script downloads a current libosmocore.
Maybe the code in osmo-trx/Transceiver52M still links against an older version of libosmocore before the API change? Not sure how to look that up. I'm not sure how to find out where the binaries for Transceiver52M/osmo-trx.cpp land to check on linkage, either. I couldn't find any such binaries in the repository directory.
Don't know much about the 'compilation layout', tried looking at Transceiver52M/Makefile.am, but there's probably some better way of trying to understand build layout with autotools that I don't yet know.
Link to failed test:
[endianness] https://jenkins.osmocom.org/jenkins/job/gerrit-pipeline-endianness/176/consoleFull
Updated by fixeria about 1 year ago
arehbein wrote in #note-15:
osmo-trx fails on the endianness test. I tried to rebase to see if that's the problem but Gerrit tells me the branch is up to date. Not sure what the problem is, the endianness test isn't run as part of
make checkin osmo-trx itself, looks like something 'exclusive' from osmo-ci.I took a first look at the project but couldn't quickly find how exactly endianness tests are run/how I could run them to debug stuff live.
It's a bit more complicated. Looking at the build log, I see struct_endianness.py is crashing:
UnicodeDecodeError: 'utf-8' codec can't decode byte 0x80 in position 25144: invalid start byte
while processing a file in osmocom-bb submodule of osmo-trx.git. osmocom-bb.git has been fixed by https://gerrit.osmocom.org/q/I0daebccf819595ee64fb4c9713a4f0ce219c58be, but the submodule is at 040bf4102892ddba1a4574d85b31653131c5ede4, predating this commit.
We can update the submodule, but I believe struct_endianness.py needs to be fixed, so that it does not crash but skip files it fails to process. I'll submit a patch.
Updated by arehbein about 1 year ago
fixeria alright, so I'll either wait until your patch is in osmocom-bb, and then update the submodule
or update it now to some commit. What do you suggest?
Question would also be which commit to choose I suppose.
Updated by osmith about 1 year ago
Regarding that CI error: it's best if the struct endianness check ignores the submodules, fixed it here: https://gerrit.osmocom.org/c/osmo-ci/+/31535
With that, CI for your osmo-trx patch should be passing, I've retriggered it.
Updated by arehbein about 1 year ago
- Status changed from In Progress to Resolved
- % Done changed from 90 to 100
Updated by pespin about 1 year ago
- Status changed from Resolved to Feedback
Re-opening since I still see deprecation warning in osmo-bsc/src/osmo-bsc/neighbor_ident.c:
CC neighbor_ident.lo
neighbor_ident.c: In function 'neighbor_controlif_setup':
neighbor_ident.c:493:2: warning: 'ctrl_interface_setup_dynip2' is deprecated: For internal use inside libosmocore only. [-Wdeprecated-declarations]
493 | return ctrl_interface_setup_dynip2(net, net->neigh_ctrl.addr, net->neigh_ctrl.port,
| ^~~~~~
In file included from neighbor_ident.c:37:
/usr/local/include/osmocom/ctrl/control_if.h:45:21: note: declared here
45 | struct ctrl_handle *ctrl_interface_setup_dynip2(void *data,
|
Updated by arehbein about 1 year ago
- Assignee changed from arehbein to msuraev
msuraev do you maybe still have a patch for that sitting in one of your working trees that you forgot to upload? pespin this was another interface Max was working on (where he tagged this issue as related https://gerrit.osmocom.org/c/libosmocore/+/30444).
If Max doesn't have anything, I can of course also fix this up. Just keep me posted
Updated by msuraev about 1 year ago
arehbein wrote in #note-23:
msuraev do you maybe still have a patch for that sitting in one of your working trees that you forgot to upload? pespin this was another interface Max was working on (where he tagged this issue as related https://gerrit.osmocom.org/c/libosmocore/+/30444).
If Max doesn't have anything, I can of course also fix this up. Just keep me posted
That's deprecated interface which should not be used anyway so there's no point updating it. I'll submit patch removing this altogether.
Updated by msuraev about 1 year ago
- Checklist item osmo-bsc added
- % Done changed from 100 to 90
https://gerrit.osmocom.org/c/osmo-pcu/+/32275 and
https://gerrit.osmocom.org/c/osmo-bsc/+/32277
are removing obsolete ctrl interface for neighbor resolution and thus get rid of deprecated ctrl init used by it.
