Here are the concepts I came up with and started to implement.
neels: what do you think, would this be a good API to work with?
laforge: FYI, if you are interested in the details.
osmo-hlr¶
First I thought, the proper location to forward messages would somewhere in gsup-server.c. But I've realized that this only handles the encoded message as sent on the wire, not the decoded version. We need to look at the decoded version, to figure out if it needs to be forwarded to another client.
So I've extended read_cb() in osmo_hlr.c with:
+ if (gsup.destination_name_len)
+ return read_cb_forward(msg, &gsup);
and added a read_cb_forward() function, which will do the following:
- check if all session IEs are present
- verify source name, destination name length (before we use string compare functions on them)
- check if source name matches where it comes from
- when any check of the above fails, abort
- otherwise send the original message to the new destination (without re-encoding it, so osmo-hlr may not remove IEs it does not know about if osmo-hlr is compiled against an older libosmocore version compared to what the clients were built against)
... and that's all there is to it from osmo-hlr side. I would do all the session checking etc. in the client.
libosmo-gsup-client¶
I've created new gsup_client_session.{c,h} files in src/gsupclient. Picking a good name wasn't trivial for me, because there are already different kinds of sessions in hlr_ussd.c. But I went with client_session, and kept almost everything in the client (vs. the other sessions, which are handled in the server), so it should be reasonable to differentiate.
Receiving messages¶
Just like before, the client using libosmo-gsup-client uses osmo_gsup_client_create() with a callback function:
osmo_gsup_client_create(ctx, "msc-a", "127.0.0.1", OSMO_GSUP_PORT, read_cb_client, NULL);
int read_cb_client(struct osmo_gsup_client *client, struct msgb *msg)
{
static struct osmo_gsup_message gsup_msg;
osmo_gsup_decode(msgb_l2(msg), msgb_l2len(msg), &gsup_msg); /* error handling omitted */
if (!osmo_gsup_client_session_sanity_check(&gsup_msg))
goto error;
...
error:
/* free up gsup_msg etc. */
}
The only new thing in the callback is the osmo_gsup_client_session_sanity_check() call. It does the following things:
- check if the GSUP message type needs to have session data attached or not (if not, skip everything else and return true)
- check if all session IEs are there
- verify source name, destination name length (before we use string compare functions on them)
- check if session state makes sense, depending on the message
- if the session state is "begin", try to create a new session (there's a global list of currently open sessions, register it there)
- check if the session is valid
- update the session's watchdog/guard timer
- when anything is wrong with this message, print a meaningful message to the log and return false
So when the sanity check went through, we are sure that the GSUP message is valid and can be used.
Sending messages¶
It should work roughly like this:
struct osmo_gsup_message gsup_msg = {0};
struct osmo_gsup_client_session *session = osmo_gsup_client_session_alloc(ctx, gsup_client, "msc-b");
gsup_msg.message_type = OSMO_GSUP_MSGT_E_PREPARE_HANDOVER_REQUEST;
strncpy(gsup_msg.imsi, "123456", sizeof(gsup_msg.imsi));
gsup_msg.an_apdu = /* ... */;
osmo_gsup_client_session_enc_send(session, &gsup_msg);
The first new function is osmo_gsup_client_session_alloc(), which adds a new entry to the client's global session list:
struct osmo_gsup_client_session {
struct llist_head list;
struct osmo_gsup_client *source;
const char *destination_name;
uint32_t session_id;
enum osmo_gsup_session_state session_state;
};
The session_id is randomly generated, and tested against the existing list entries with the same source and destination to make sure that there are no duplicates (yes, performance could be optimized here by making one list per destination for example, but this isn't needed here, right?).
Then it gets sent out by the other new function osmo_gsup_client_session_enc_send(), which does some error checking, fills out all the session IEs and finally sends the message with osmo_gsup_client_enc_send:
int osmo_gsup_client_session_enc_send(const struct osmo_gsup_client_session *session,
struct osmo_gsup_message *gsup_msg)
{
const char *source_name = session->source->ipa_dev->unit_name;
if (gsup_msg->session_id || gsup_msg->session_state || gsup_msg->source_name || gsup_msg->source_name_len
|| gsup_msg->destination_name || gsup_msg->destination_name_len) {
/* TODO: log error! */
return -EINVAL;
}
gsup_msg->session_id = session->session_id;
gsup_msg->session_state = session->session_state;
gsup_msg->source_name = (uint8_t *)source_name;
gsup_msg->source_name_len = strlen(source_name);
gsup_msg->destination_name = (uint8_t *)session->destination_name;
gsup_msg->destination_name_len = strlen(session->destination_name);
return osmo_gsup_client_enc_send(session->source, gsup_msg);
}
Tests¶
My first approach was creating a C test, which starts the gsup server and two gsup clients, and tries to send messages between them. This would need read_cb() from hlr.c, so I figured it would be a good idea to build upon hlr.c, but replace main() with my own test main() function. This compiles and links properly now, but as I'm trying to run it, this does not seem like the best approach anymore. Because I need a lot of stuff from the original main() to get the server running properly, but the main loop would need to be replaced with testing logic, and in the end we aren't really running osmo-hlr's original gsup server anymore, but instead some modified version that may do quite a few things differently.
So I'm trying to run the osmo-hlr binary now, with a C test that creates two clients with the libosmo-gsup-client functions I've described above.
I created a prepare.sh and cleanup.sh shell script, which would run osmo-hlr in the background with the example config, save the pid, and kill it afterwards. In testsuite.at it looks like this:
AT_SETUP([gsup_client_session])
AT_KEYWORDS([gsup_client_session])
cat $abs_srcdir/gsup_client_session/gsup_client_session_test.ok > expout
cat $abs_srcdir/gsup_client_session/gsup_client_session_test.err > experr
$abs_srcdir/gsup_client_session/prepare.sh "$abs_top_builddir" "$abs_top_srcdir"
AT_CHECK([$abs_top_builddir/tests/gsup_client_session/gsup_client_session_test], [], [expout], [experr])
$abs_srcdir/gsup_client_session/cleanup.sh "$abs_top_builddir"
AT_CLEANUP
However, this doesn't work as expected: the testsuite will hang until osmo-hlr gets killed :\
My conclusion is, that we should rather do something like the external python tests do; create a python script that will run osmo-hlr, but instead of accessing the vty, run the test program and redirect its output. Then kill osmo-hlr when the test program quits, and pass the test program's exit code. Maybe something like that:
$ export DAEMON="osmo-hlr -c=..."
$ some-wrapper.py -- path/to/test_program
From what I can tell, we don't have anything similar in osmo-python-tests.git, so next week I would start with creating that.