Hello Pablo,
attached are patches against the current pablo/ctrl-updates branch that fix the compile issues. It needs the closed callback in libosmo-abis to work. As long as the bsc_msc_connection is still using write_queues I have added the old ctrl_cmd_send function again (under a different name) So we can still send commands over that connection. I've tested the communication/timeout/closed_cb handling and nothing problematic showed up.
Having two send commands is unfortunate, but I guess it's okay for the time being. I'm guessing that the other connections will be switching to libosmo-abis as well in the foreseeable future and then this issue will resolve itself.
Feel free to squash the patches as you see fit in order to get them merged.
Regards, Daniel Willmann (5): libctrl: Catch up with API change for ipa_server_conn_create nat: Use libosmo-abis infrastructure in the pending entries libctrl: Add a new function to send commands through an osmo_wqueue osmo-bsc: Use ctrl_cmd_send_wqueue to send cmds over the nat-bsc link nat: Use ctrl_cmd_send_wqueue to send cmds over the nat-bsc link
openbsc/include/openbsc/bsc_nat.h | 2 +- openbsc/include/openbsc/control_cmd.h | 1 + openbsc/src/libctrl/control_if.c | 24 +++++++++++++++++++++++- openbsc/src/osmo-bsc/osmo_bsc_msc.c | 4 ++-- openbsc/src/osmo-bsc_nat/bsc_nat.c | 19 ++++++++++--------- 5 files changed, 37 insertions(+), 13 deletions(-)
--- openbsc/src/libctrl/control_if.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/openbsc/src/libctrl/control_if.c b/openbsc/src/libctrl/control_if.c index 7e2ae16..99e4ccf 100644 --- a/openbsc/src/libctrl/control_if.c +++ b/openbsc/src/libctrl/control_if.c @@ -244,7 +244,7 @@ static int ctrl_accept_cb(struct ipa_server_link *ipa_link, int fd) struct ipa_server_conn *ipa_peer_link;
ipa_peer_link = ipa_server_conn_create(tall_bsc_ctx, ipa_link, fd, - handle_control_read, + handle_control_read, NULL, ipa_link->data); if (!ipa_peer_link) { LOGP(DCTRL, LOGL_ERROR, "Failed to register peer connection.\n");
The ctrl_cmds are now using struct ipa_server_conn so the pending entries need to use it as well. This patch needs the closed_cb patch for libosmo-abis to work. --- openbsc/include/openbsc/bsc_nat.h | 2 +- openbsc/src/osmo-bsc_nat/bsc_nat.c | 15 ++++++++------- 2 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/openbsc/include/openbsc/bsc_nat.h b/openbsc/include/openbsc/bsc_nat.h index b4f07e0..eca019d 100644 --- a/openbsc/include/openbsc/bsc_nat.h +++ b/openbsc/include/openbsc/bsc_nat.h @@ -78,7 +78,7 @@ struct bsc_cmd_list { int nat_id;
/* The control connection from which the command originated */ - struct ctrl_connection *ccon; + struct ipa_server_conn *ipa_link;
/* The command from the control connection */ struct ctrl_cmd *cmd; diff --git a/openbsc/src/osmo-bsc_nat/bsc_nat.c b/openbsc/src/osmo-bsc_nat/bsc_nat.c index 326d04e..2753065 100644 --- a/openbsc/src/osmo-bsc_nat/bsc_nat.c +++ b/openbsc/src/osmo-bsc_nat/bsc_nat.c @@ -882,7 +882,7 @@ void bsc_close_connection(struct bsc_connection *connection) llist_for_each_entry_safe(cmd_entry, cmd_tmp, &connection->cmd_pending, list_entry) { cmd_entry->cmd->type = CTRL_TYPE_ERROR; cmd_entry->cmd->reply = "BSC closed the connection"; - ctrl_cmd_send(&cmd_entry->ccon->write_queue, cmd_entry->cmd); + ctrl_cmd_send(cmd_entry->ipa_link, cmd_entry->cmd); bsc_del_pending(cmd_entry); }
@@ -1232,7 +1232,7 @@ static int handle_ctrlif_msg(struct bsc_connection *bsc, struct msgb *msg) goto err; } cmd->id = id; - ctrl_cmd_send(&pending->ccon->write_queue, cmd); + ctrl_cmd_send(pending->ipa_link, cmd); bsc_del_pending(pending); } else { /* We need to handle TRAPS here */ @@ -1575,22 +1575,23 @@ static void pending_timeout_cb(void *data) LOGP(DNAT, LOGL_ERROR, "Command timed out\n"); pending->cmd->type = CTRL_TYPE_ERROR; pending->cmd->reply = "Command timed out"; - ctrl_cmd_send(&pending->ccon->write_queue, pending->cmd); + ctrl_cmd_send(pending->ipa_link, pending->cmd);
bsc_del_pending(pending); }
-static void ctrl_conn_closed_cb(struct ctrl_connection *connection) +static int ctrl_conn_closed_cb(struct ipa_server_conn *connection) { struct bsc_connection *bsc; struct bsc_cmd_list *pending, *tmp;
llist_for_each_entry(bsc, &nat->bsc_connections, list_entry) { llist_for_each_entry_safe(pending, tmp, &bsc->cmd_pending, list_entry) { - if (pending->ccon == connection) + if (pending->ipa_link == connection) bsc_del_pending(pending); } } + return 0; }
static int forward_to_bsc(struct ctrl_cmd *cmd) @@ -1660,8 +1661,8 @@ static int forward_to_bsc(struct ctrl_cmd *cmd) cmd->reply = "Sending failed"; goto err; } - pending->ccon = cmd->ccon; - pending->ccon->closed_cb = ctrl_conn_closed_cb; + pending->ipa_link = cmd->ipa_link; + pending->ipa_link->closed_cb = ctrl_conn_closed_cb; pending->cmd = cmd;
/* Setup the timeout */
This is essentially the old ctrl_cmd_send function with a different name. As long as the connection between nat and osmo-bsc is not using the generic socket infrastructure from libosmo-abis it is still needed. --- openbsc/include/openbsc/control_cmd.h | 1 + openbsc/src/libctrl/control_if.c | 22 ++++++++++++++++++++++ 2 files changed, 23 insertions(+), 0 deletions(-)
diff --git a/openbsc/include/openbsc/control_cmd.h b/openbsc/include/openbsc/control_cmd.h index 87db33f..8eeed99 100644 --- a/openbsc/include/openbsc/control_cmd.h +++ b/openbsc/include/openbsc/control_cmd.h @@ -61,6 +61,7 @@ struct ctrl_cmd_map { int ctrl_cmd_exec(vector vline, struct ctrl_cmd *command, vector node, void *data); int ctrl_cmd_install(enum ctrl_node_type node, struct ctrl_cmd_element *cmd); int ctrl_cmd_handle(struct ctrl_cmd *cmd, void *data); +int ctrl_cmd_send_wqueue(struct osmo_wqueue *queue, struct ctrl_cmd *cmd); int ctrl_cmd_send(struct ipa_server_conn *ipa_server_conn, struct ctrl_cmd *cmd); struct ctrl_cmd *ctrl_cmd_parse(void *ctx, struct msgb *msg); struct msgb *ctrl_cmd_make(struct ctrl_cmd *cmd); diff --git a/openbsc/src/libctrl/control_if.c b/openbsc/src/libctrl/control_if.c index 99e4ccf..5476179 100644 --- a/openbsc/src/libctrl/control_if.c +++ b/openbsc/src/libctrl/control_if.c @@ -86,6 +86,28 @@ int ctrl_cmd_send(struct ipa_server_conn *ipa_server_conn, struct ctrl_cmd *cmd) return 0; }
+int ctrl_cmd_send_wqueue(struct osmo_wqueue *queue, struct ctrl_cmd *cmd) +{ + int ret; + struct msgb *msg; + + msg = ctrl_cmd_make(cmd); + if (!msg) { + LOGP(DCTRL, LOGL_ERROR, "Could not generate msg\n"); + return -1; + } + + ipaccess_prepend_header_ext(msg, IPAC_PROTO_EXT_CTRL); + ipaccess_prepend_header(msg, IPAC_PROTO_OSMO); + + ret = osmo_wqueue_enqueue(queue, msg); + if (ret != 0) { + LOGP(DCTRL, LOGL_ERROR, "Failed to enqueue the command"); + msgb_free(msg); + } + return ret; +} + int ctrl_cmd_handle(struct ctrl_cmd *cmd, void *data) { char *token, *request;
--- openbsc/src/osmo-bsc/osmo_bsc_msc.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/openbsc/src/osmo-bsc/osmo_bsc_msc.c b/openbsc/src/osmo-bsc/osmo_bsc_msc.c index 04cfb99..eeaaf72 100644 --- a/openbsc/src/osmo-bsc/osmo_bsc_msc.c +++ b/openbsc/src/osmo-bsc/osmo_bsc_msc.c @@ -211,7 +211,7 @@ static void handle_ctrl(struct osmo_msc_data *msc, struct msgb *msg) cmd->id = "err"; cmd->reply = "Failed to parse control message.";
- ctrl_cmd_send(&msc->msc_con->write_queue, cmd); + ctrl_cmd_send_wqueue(&msc->msc_con->write_queue, cmd); talloc_free(cmd);
return; @@ -219,7 +219,7 @@ static void handle_ctrl(struct osmo_msc_data *msc, struct msgb *msg)
ret = ctrl_cmd_handle(cmd, msc->network); if (ret != CTRL_CMD_HANDLED) - ctrl_cmd_send(&msc->msc_con->write_queue, cmd); + ctrl_cmd_send_wqueue(&msc->msc_con->write_queue, cmd); talloc_free(cmd); }
--- openbsc/src/osmo-bsc_nat/bsc_nat.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/openbsc/src/osmo-bsc_nat/bsc_nat.c b/openbsc/src/osmo-bsc_nat/bsc_nat.c index 2753065..f432622 100644 --- a/openbsc/src/osmo-bsc_nat/bsc_nat.c +++ b/openbsc/src/osmo-bsc_nat/bsc_nat.c @@ -1249,7 +1249,7 @@ static int handle_ctrlif_msg(struct bsc_connection *bsc, struct msgb *msg) talloc_free(cmd); return 0; err: - ctrl_cmd_send(&bsc->write_queue, cmd); + ctrl_cmd_send_wqueue(&bsc->write_queue, cmd); talloc_free(cmd); return 0; } @@ -1657,7 +1657,7 @@ static int forward_to_bsc(struct ctrl_cmd *cmd) goto err; }
- if (ctrl_cmd_send(&bsc->write_queue, bsc_cmd)) { + if (ctrl_cmd_send_wqueue(&bsc->write_queue, bsc_cmd)) { cmd->reply = "Sending failed"; goto err; }
On Thu, Sep 15, 2011 at 04:04:48PM +0200, Daniel Willmann wrote:
Hello Pablo,
attached are patches against the current pablo/ctrl-updates branch that fix the compile issues. It needs the closed callback in libosmo-abis to work. As long as the bsc_msc_connection is still using write_queues I have added the old ctrl_cmd_send function again (under a different name) So we can still send commands over that connection. I've tested the communication/timeout/closed_cb handling and nothing problematic showed up.
Thanks Daniel, I'll rebase the pablo/ctrl-updates change with these patches and resubmit.
Having two send commands is unfortunate, but I guess it's okay for the time being. I'm guessing that the other connections will be switching to libosmo-abis as well in the foreseeable future and then this issue will resolve itself.
Indeed, make sense to me for the time being.
I'll look for more candidates that can be ported to the generic IPA infrastructure. It can be interesting to see if they have any other specific need, I think we're still in time to make not-very-painful API/ABI changes.
-
Daniel Willmann -
Pablo Neira Ayuso