Attention is currently required from: pespin.

View Change

6 comments:

• File include/osmocom/bsc_nat/bsc_nat.h:

  • Patch Set #1, Line 57: struct msc *bsc_nat_msc_add_from_addr_book(struct bsc_nat *bsc_nat);

    I wonder what is this function doing.

    See the other comment.

  • Patch Set #1, Line 58: struct msc *bsc_nat_msc_get(struct bsc_nat *bsc_nat);

    you get it identified by nothing?

    Yes, since there is currently only one MSC. I'd adjust the function when support for multiple MSCs gets added. As I understand it, MSC pooling is outside of the scope of SYS#5560, OS#2545.

    Related: https://osmocom.org/projects/osmo-bscnat/wiki/AoIP_OsmoBSCNAT#MSC-pooling

• File src/osmo-bsc-nat/bsc_nat.c:

  • Patch Set #1, Line 70: struct msc *bsc_nat_msc_add_from_addr_book(struct bsc_nat *bsc_nat)

    Not sure we really need this function, just use the content directly, since this will go away at som […]

    This function adds the msc from the following block in the config:

     cs7 instance 0
      sccp-address msc
       routing-indicator PC
       point-code 0.23.1
       subsystem-number 254

    When OsmoBSCNAT supports multiple MSCs, it does not go away, but rather we need to read multiple msc entries from the config (msc0, msc1, ... probably).

    This is necessary, because the BSCNAT needs to send the RESET towards the MSC, so it needs to know its address. (We don't need to know the address of the BSCs, since they send the RESET to BSCNAT and then we know the address.)

    Therefore I'd keep a function for adding the MSC from the address book. But the return value isn't really used and doesn't make sense once this parses multiple MSCs, I've changed it to int, return 0 on success and -ENOENT on error.

  • Patch Set #1, Line 93: void bsc_nat_msc_del(struct bsc_nat *bsc_nat, struct msc *msc)

    IMHO this should be an API of the MSC object (see the bsc_nat is actually not used at all). […]

    Done

• File src/osmo-bsc-nat/main.c:

  • Patch Set #1, Line 202: if (!bsc_nat_msc_add_from_addr_book(g_bsc_nat))

    You can probably call the content of the function here directly. […]

    IMHO it makes main() a bit more readable if this remains inside a function.

• File src/osmo-bsc-nat/msc_fsm.c:

  • Patch Set #1, Line 44: { 0, NULL }

    this half done FSM shouldn't be submitted this way. […]

    Done

To view, visit change 27473. To unsubscribe, or for help writing mail filters, visit settings.