Project

General

Profile

Bug #5018

neighbor config broken: need separate neighbor section, but 'write file' puts the config back inline

Added by neels 3 months ago. Updated about 2 months ago.

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

100%

Spec Reference:

Description

when reading the config OsmoBSC resolves the neighbor relations between cells immediately.
So when writing 'neighbor bts 23', that cell must already exist and be configured.

Same for identifying a neighbor by Cell Global ID / LAC-CI etc:
the config parsing immediately resolves the id, and classifies it as local or remote BTS.
Hence a local neighbor BTS must already be configured beforehand.

A consequence of this is that the config file must first configure all cells,
and in another section below configure only the neighbor relations:

network
 # first set up all cells
 bts 0
  description my test BTS 0
  type sysmobts
  band GSM-1800
  ip.access unit_id 0 0
  location_area_code 23
  [...]
 bts 1
  description my test BTS 1
  type sysmobts
  band GSM-1800
  ip.access unit_id 1 0
  location_area_code 42
  [...]

 # neighbor relations
 bts 0
  neighbor bts 1
 bts 1
  neighbor bts 0

However, the 'write file' feature puts the neighbor relations back inline,
combining the two "config sections" and hence breaks the resulting config.

One solution would be to change 'write file' so that it puts neighbor relations in a separate section.
However, having to put neighbor config separately also potentially confuses users.
So a nicer solution would be to allow putting a neighbor relation in the config before that neighbor is configured.

Associated revisions

Revision 91d54d70 (diff)
Added by Neels Hofmeyr 2 months ago

refactor handover penalty timers

So far the list of penalty timers was stored for an opaque target
pointer. That was either a gsm_bts pointer for a local BTS, or a cell
identifier list pointer for a remote-BSS cell.

Reasons to refactor penalty timers:

- The cell identifier list pointer came from the neighbor configuration
storage, but the way cell neighbor config is stored will change in a
subsequent patch. There will be no more cell identifier lists there.

- Storing object pointers is inherently unsafe -- if an object gets
removed and another gets allocated, the penalty timer could
theoretically remain in force for an unrelated object.

Rather store penalty timers for specific Cell IDs. Since remote-BSS
neighbors can be requested by a cell identifier list, use a
gsm0808_cell_id_list2 as key in the list of penalty timers.

Fix handover_test.c: have different CI for each local BTS. So far it was
the same LAC+CI for all BTSes, which now would make the test fail,
because any penalty timer would appear to apply to all local cells.

Related: OS#5018
Change-Id: I72dd6226a6d69c3f653a3174c6f55bf4eecc6885

Revision 579fcdde (diff)
Added by Neels Hofmeyr 2 months ago

drop neighbor_ident_test.c

This tests the opaquely designed neighbor config storage. However, a
subsequent patch will refactor the neighbor config storage, and this
neighbor ident API will change fundamentally. No need to test this.

The new storage will use the usual scheme of transparent struct and
llist, the opaque design is not necessary and just bloats. There is no
need to test a plain llist, so this test needs no replacement.

Related: OS#5018
Change-Id: I6522796bf0bbb9ab83e49168bcbff7bc70fd6c6d

Revision 055ab692 (diff)
Added by Neels Hofmeyr 2 months ago

refactor neighbor config

The neighbor configuration storage is fundamentally broken: it requires
all local cells to be configured before being able to list them as
neighbors of each other. Upon config write-back, the neighbor config
however is placed back inline with the other config, and hence a
written-out neighbor config no longer works on program restart.

The cause of this problem is that the config is stored as explicit
pointers between local cells (struct gsm_bts), which of course requires
the pointer to exist before being able to reference it.

Instead, store the actual configuration that the user entered as-is,
without pointers or references to objects that need to be ready. Resolve
the neighbors every time a neighbor is needed.

Hence the user may enter any config at any place in the config file,
even non-working config (like a BTS number that doesn't exist), and the
relation to actual local or remote neighbor cells is made at runtime.

Related: OS#5018
Change-Id: I9ed992f8bfff888b3933733c0576f92d50f2625b

Revision d88f9a53 (diff)
Added by Neels Hofmeyr about 2 months ago

refactor handover penalty timers

So far the list of penalty timers was stored for an opaque target
pointer. That was either a gsm_bts pointer for a local BTS, or a cell
identifier list pointer for a remote-BSS cell.

Reasons to refactor penalty timers:

- The cell identifier list pointer came from the neighbor configuration
storage, but the way cell neighbor config is stored will change in a
subsequent patch. There will be no more cell identifier lists there.

- Storing object pointers is inherently unsafe -- if an object gets
removed and another gets allocated, the penalty timer could
theoretically remain in force for an unrelated object.

Rather store penalty timers for specific Cell IDs. Since remote-BSS
neighbors can be requested by a cell identifier list, use a
gsm0808_cell_id_list2 as key in the list of penalty timers.

Fix handover_test.c: have different CI for each local BTS. So far it was
the same LAC+CI for all BTSes, which now would make the test fail,
because any penalty timer would appear to apply to all local cells.

Related: OS#5018
Change-Id: I72dd6226a6d69c3f653a3174c6f55bf4eecc6885

Revision dc60505b (diff)
Added by Neels Hofmeyr about 2 months ago

drop neighbor_ident_test.c

This tests the opaquely designed neighbor config storage. However, a
subsequent patch will refactor the neighbor config storage, and this
neighbor ident API will change fundamentally. No need to test this.

The new storage will use the usual scheme of transparent struct and
llist, the opaque design is not necessary and just bloats. There is no
need to test a plain llist, so this test needs no replacement.

Related: OS#5018
Change-Id: I6522796bf0bbb9ab83e49168bcbff7bc70fd6c6d

Revision ee1c200b (diff)
Added by Neels Hofmeyr about 2 months ago

fix/refactor neighbor config

The neighbor configuration storage is fundamentally broken: it requires
all local cells to be configured before being able to list them as
neighbors of each other. Upon config write-back, the neighbor config
however is placed back inline with the other config, and hence a
written-out neighbor config no longer works on program restart.

The cause of this problem is that the config is stored as explicit
pointers between local cells (struct gsm_bts), which of course requires
the pointer to exist before being able to reference it.

Instead, store the actual configuration that the user entered as-is,
without pointers or references to objects that need to be ready. Resolve
the neighbors every time a neighbor is needed.

Hence the user may enter any config at any place in the config file,
even non-working config (like a BTS number that doesn't exist), and the
relation to actual local or remote neighbor cells is made at runtime.

Related: OS#5018
Change-Id: I9ed992f8bfff888b3933733c0576f92d50f2625b

Revision 764449ec (diff)
Added by Neels Hofmeyr about 2 months ago

fix/refactor neighbor config

The neighbor configuration storage is fundamentally broken: it requires
all local cells to be configured before being able to list them as
neighbors of each other. Upon config write-back, the neighbor config
however is placed back inline with the other config, and hence a
written-out neighbor config no longer works on program restart.

The cause of this problem is that the config is stored as explicit
pointers between local cells (struct gsm_bts), which of course requires
the pointer to exist before being able to reference it.

Instead, store the actual configuration that the user entered as-is,
without pointers or references to objects that need to be ready. Resolve
the neighbors every time a neighbor is needed.

Hence the user may enter any config at any place in the config file,
even non-working config (like a BTS number that doesn't exist), and the
relation to actual local or remote neighbor cells is made at runtime.

Abort program startup if the initial neighbor configuration contains
errors.

Related: OS#5018
Change-Id: I9ed992f8bfff888b3933733c0576f92d50f2625b

History

#1 Updated by neels 3 months ago

#2 Updated by neels 3 months ago

#3 Updated by neels 2 months ago

  • Status changed from New to In Progress
  • % Done changed from 0 to 80

#5 Updated by neels about 2 months ago

  • Status changed from In Progress to Resolved
  • % Done changed from 90 to 100

Patches merged, issue should be fixed now.

Neighbor configuration should be permitted in any order, before or after the neighbor target cells are configured in the config file.
Also, neighbor configuration should be written back to the config file exactly as it was originally configured.

If there are neighbor configuration errors (like non-existing BTS number; non-existing local cell identity etc), osmo-bsc should refuse to start up.

When configuring neighbors on the live VTY console, that error checking does not happen; errors will still be logged as osmo-bsc resolves neighbors and encounters config errors. One possible pitfall: when such an erratic config from live VTY is written to file and osmo-bsc is restarted, the startup will complain about neighbor config errors and osmo-bsc will not start.

Also available in: Atom PDF

Add picture from clipboard (Maximum size: 48.8 MB)