15 Sep
2011
15 Sep
'11
10:04 a.m.
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(-)
--
1.7.6.1
15 Sep
15 Sep
10:04 a.m.
New subject: [PATCH 1/5] libctrl: Catch up with API change for ipa_server_conn_create
---
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");
--
1.7.6.1
10:04 a.m.
New subject: [PATCH 2/5] nat: Use libosmo-abis infrastructure in the pending entries
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 */
--
1.7.6.1
10:04 a.m.
New subject: [PATCH 3/5] libctrl: Add a new function to send commands through an osmo_wqueue
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;
--
1.7.6.1
10:04 a.m.
New subject: [PATCH 4/5] osmo-bsc: Use ctrl_cmd_send_wqueue to send cmds over the nat-bsc link
---
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);
}
--
1.7.6.1
10:04 a.m.
New subject: [PATCH 5/5] nat: Use ctrl_cmd_send_wqueue to send cmds over the nat-bsc link
---
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;
}
--
1.7.6.1
7:17 p.m.
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.
5058
days inactive
5058
days old
6 comments
2 participants
participants (2)
-
Daniel Willmann -
Pablo Neira Ayuso