9 Sep
2011
9 Sep
'11
3:05 a.m.
From: Pablo Neira Ayuso <pablo(a)gnumonks.org>
Hi,
These patchset contains the port of libctrl to use the IPA
infrastructure available in libosmo-abis.
I've also made one small cleanup to bail out in case that
we cannot bind to the telnet port. Another patch follows up
to avoid disabling nagle (to avoid problems with TCP
segmentation, we still have to support this appropriately),
I noticed this while doing the port work.
Pablo Neira Ayuso (3):
ctrl: use generic IPA socket infrastructure available in libosmo-abis
src: check for error returned by controlif_setup()
ctrl: do not disable nagle algorithm until we fully support
segmentation
openbsc/include/openbsc/control_cmd.h | 17 +---
openbsc/include/openbsc/control_if.h | 2 +-
openbsc/src/libctrl/control_if.c | 153 +++++++--------------------------
openbsc/src/osmo-bsc/osmo_bsc_main.c | 6 +-
openbsc/src/osmo-bsc_nat/bsc_nat.c | 6 +-
openbsc/src/osmo-nitb/bsc_hack.c | 5 +-
6 files changed, 50 insertions(+), 139 deletions(-)
--
1.7.2.5
9 Sep
9 Sep
3:05 a.m.
New subject: [PATCH 1/3] ctrl: use generic IPA socket infrastructure available in libosmo-abis
From: Pablo Neira Ayuso <pablo(a)gnumonks.org>
This patch puts libctrl into diet by using the generic IPA
socket infrastructure available in libosmo-abis.
We save 93 lines of code with this and we remove the use of the
deprecated socket infrastructure like make_sock(...).
---
openbsc/include/openbsc/control_cmd.h | 17 +----
openbsc/include/openbsc/control_if.h | 2 +-
openbsc/src/libctrl/control_if.c | 148 ++++++++-------------------------
3 files changed, 37 insertions(+), 130 deletions(-)
diff --git a/openbsc/include/openbsc/control_cmd.h
b/openbsc/include/openbsc/control_cmd.h
index 2a5391f..87db33f 100644
--- a/openbsc/include/openbsc/control_cmd.h
+++ b/openbsc/include/openbsc/control_cmd.h
@@ -29,21 +29,8 @@ enum ctrl_type {
CTRL_TYPE_ERROR
};
-struct ctrl_connection {
- struct llist_head list_entry;
-
- /* The queue for sending data back */
- struct osmo_wqueue write_queue;
-
- /* Callback if the connection was closed */
- void (*closed_cb)(struct ctrl_connection *conn);
-
- /* Pending commands for this connection */
- struct llist_head cmds;
-};
-
struct ctrl_cmd {
- struct ctrl_connection *ccon;
+ struct ipa_server_conn *ipa_link;
enum ctrl_type type;
char *id;
void *node;
@@ -74,7 +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(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);
struct ctrl_cmd *ctrl_cmd_cpy(void *ctx, struct ctrl_cmd *cmd);
diff --git a/openbsc/include/openbsc/control_if.h b/openbsc/include/openbsc/control_if.h
index 96fbf6b..50ab962 100644
--- a/openbsc/include/openbsc/control_if.h
+++ b/openbsc/include/openbsc/control_if.h
@@ -5,7 +5,7 @@
#include <openbsc/control_cmd.h>
#include <openbsc/gsm_data.h>
-int ctrl_cmd_send(struct osmo_wqueue *queue, struct ctrl_cmd *cmd);
+int ctrl_cmd_send(struct ipa_server_conn *ipa_server_conn, struct ctrl_cmd *cmd);
int ctrl_cmd_handle(struct ctrl_cmd *cmd, void *data);
int controlif_setup(struct gsm_network *gsmnet, uint16_t port);
diff --git a/openbsc/src/libctrl/control_if.c b/openbsc/src/libctrl/control_if.c
index 8198ae6..58476ec 100644
--- a/openbsc/src/libctrl/control_if.c
+++ b/openbsc/src/libctrl/control_if.c
@@ -63,15 +63,14 @@
#include <osmocom/abis/ipa.h>
struct ctrl_handle {
- struct osmo_fd listen_fd;
+ struct ipa_server_link *ipa_link;
struct gsm_network *gsmnet;
};
vector ctrl_node_vec;
-int ctrl_cmd_send(struct osmo_wqueue *queue, struct ctrl_cmd *cmd)
+int ctrl_cmd_send(struct ipa_server_conn *ipa_server_conn, struct ctrl_cmd *cmd)
{
- int ret;
struct msgb *msg;
msg = ctrl_cmd_make(cmd);
@@ -83,12 +82,8 @@ int ctrl_cmd_send(struct osmo_wqueue *queue, struct ctrl_cmd *cmd)
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.\n");
- msgb_free(msg);
- }
- return ret;
+ ipa_server_conn_send(ipa_server_conn, msg);
+ return 0;
}
int ctrl_cmd_handle(struct ctrl_cmd *cmd, void *data)
@@ -192,43 +187,14 @@ err:
return ret;
}
-static void control_close_conn(struct ctrl_connection *ccon)
-{
- close(ccon->write_queue.bfd.fd);
- osmo_fd_unregister(&ccon->write_queue.bfd);
- if (ccon->closed_cb)
- ccon->closed_cb(ccon);
- talloc_free(ccon);
-}
-
-static int handle_control_read(struct osmo_fd * bfd)
+static int
+handle_control_read(struct ipa_server_conn *ipa_server_conn, struct msgb *msg)
{
int ret = -1;
- struct osmo_wqueue *queue;
- struct ctrl_connection *ccon;
struct ipaccess_head *iph;
struct ipaccess_head_ext *iph_ext;
- struct msgb *msg;
struct ctrl_cmd *cmd;
- struct ctrl_handle *ctrl = bfd->data;
-
- queue = container_of(bfd, struct osmo_wqueue, bfd);
- ccon = container_of(queue, struct ctrl_connection, write_queue);
-
- ret = ipa_msg_recv(bfd->fd, &msg);
- if (ret <= 0) {
- if (ret == 0)
- LOGP(DCTRL, LOGL_INFO, "The control connection was closed\n");
- else
- LOGP(DCTRL, LOGL_ERROR, "Failed to parse ip access message: %d\n", ret);
-
- goto err;
- }
-
- if (msg->len < sizeof(*iph) + sizeof(*iph_ext)) {
- LOGP(DCTRL, LOGL_ERROR, "The message is too short.\n");
- goto err;
- }
+ struct ctrl_handle *ctrl = ipa_server_conn->data;
iph = (struct ipaccess_head *) msg->data;
if (iph->proto != IPAC_PROTO_OSMO) {
@@ -244,23 +210,23 @@ static int handle_control_read(struct osmo_fd * bfd)
msg->l2h = iph_ext->data;
- cmd = ctrl_cmd_parse(ccon, msg);
+ cmd = ctrl_cmd_parse(ipa_server_conn, msg);
if (cmd) {
- cmd->ccon = ccon;
+ cmd->ipa_link = ipa_server_conn;
if (ctrl_cmd_handle(cmd, ctrl->gsmnet) != CTRL_CMD_HANDLED) {
- ctrl_cmd_send(queue, cmd);
+ ctrl_cmd_send(ipa_server_conn, cmd);
talloc_free(cmd);
}
} else {
- cmd = talloc_zero(ccon, struct ctrl_cmd);
+ cmd = talloc_zero(ipa_server_conn, struct ctrl_cmd);
if (!cmd)
goto err;
LOGP(DCTRL, LOGL_ERROR, "Command parser error.\n");
cmd->type = CTRL_TYPE_ERROR;
cmd->id = "err";
cmd->reply = "Command parser error.";
- ctrl_cmd_send(queue, cmd);
+ ctrl_cmd_send(ipa_server_conn, cmd);
talloc_free(cmd);
}
@@ -268,82 +234,31 @@ static int handle_control_read(struct osmo_fd * bfd)
return 0;
err:
- control_close_conn(ccon);
+ ipa_server_conn_destroy(ipa_server_conn);
msgb_free(msg);
return ret;
}
-static int control_write_cb(struct osmo_fd *bfd, struct msgb *msg)
-{
- int rc;
-
- rc = write(bfd->fd, msg->data, msg->len);
- if (rc != msg->len)
- LOGP(DCTRL, LOGL_ERROR, "Failed to write message to the control
connection.\n");
-
- return rc;
-}
-
-static struct ctrl_connection *ctrl_connection_alloc(void *ctx)
-{
- struct ctrl_connection *ccon = talloc_zero(ctx, struct ctrl_connection);
- if (!ccon)
- return NULL;
-
- osmo_wqueue_init(&ccon->write_queue, 100);
- /* Error handling here? */
-
- INIT_LLIST_HEAD(&ccon->cmds);
- return ccon;
-}
-
-static int listen_fd_cb(struct osmo_fd *listen_bfd, unsigned int what)
+static int ctrl_accept_cb(struct ipa_server_link *ipa_link, int fd)
{
- int ret, fd, on;
- struct ctrl_connection *ccon;
- struct sockaddr_in sa;
- socklen_t sa_len = sizeof(sa);
-
-
- if (!(what & BSC_FD_READ))
- return 0;
-
- fd = accept(listen_bfd->fd, (struct sockaddr *) &sa, &sa_len);
- if (fd < 0) {
- perror("accept");
- return fd;
- }
- LOGP(DCTRL, LOGL_INFO, "accept()ed new control connection from %s\n",
- inet_ntoa(sa.sin_addr));
+ int ret, on = 1;
+ struct ipa_server_conn *ipa_peer_link;
- on = 1;
ret = setsockopt(fd, IPPROTO_TCP, TCP_NODELAY, &on, sizeof(on));
if (ret != 0) {
LOGP(DNAT, LOGL_ERROR, "Failed to set TCP_NODELAY: %s\n", strerror(errno));
close(fd);
return ret;
}
- ccon = ctrl_connection_alloc(listen_bfd->data);
- if (!ccon) {
- LOGP(DCTRL, LOGL_ERROR, "Failed to allocate.\n");
- close(fd);
- return -1;
- }
-
- ccon->write_queue.bfd.data = listen_bfd->data;
- ccon->write_queue.bfd.fd = fd;
- ccon->write_queue.bfd.when = BSC_FD_READ;
- ccon->write_queue.read_cb = handle_control_read;
- ccon->write_queue.write_cb = control_write_cb;
-
- ret = osmo_fd_register(&ccon->write_queue.bfd);
- if (ret < 0) {
- LOGP(DCTRL, LOGL_ERROR, "Could not register FD.\n");
- close(ccon->write_queue.bfd.fd);
- talloc_free(ccon);
+ ipa_peer_link = ipa_server_conn_create(tall_bsc_ctx, ipa_link, fd,
+ handle_control_read,
+ ipa_link->data);
+ if (!ipa_peer_link) {
+ LOGP(DCTRL, LOGL_ERROR, "Failed to register peer connection.\n");
+ return -ENOMEM;
}
- return ret;
+ return 0;
}
static uint64_t get_rate_ctr_value(const struct rate_ctr *ctr, int intv)
@@ -601,7 +516,6 @@ static int verify_counter(struct ctrl_cmd *cmd, const char *value,
void *data)
int controlif_setup(struct gsm_network *gsmnet, uint16_t port)
{
- int ret;
struct ctrl_handle *ctrl;
ctrl = talloc_zero(tall_bsc_ctx, struct ctrl_handle);
@@ -615,15 +529,21 @@ int controlif_setup(struct gsm_network *gsmnet, uint16_t port)
return -ENOMEM;
/* Listen for control connections */
- ret = make_sock(&ctrl->listen_fd, IPPROTO_TCP, INADDR_LOOPBACK, port,
- 0, listen_fd_cb, ctrl);
- if (ret < 0) {
+ ctrl->ipa_link = ipa_server_link_create(ctrl, NULL,
+ "127.0.0.1", port,
+ ctrl_accept_cb, ctrl);
+ if (!ctrl->ipa_link) {
talloc_free(ctrl);
- return ret;
+ return -ENOMEM;
+ }
+
+ if (ipa_server_link_open(ctrl->ipa_link) < 0) {
+ talloc_free(ctrl);
+ return -EIO;
}
ctrl_cmd_install(CTRL_NODE_ROOT, &cmd_rate_ctr);
ctrl_cmd_install(CTRL_NODE_ROOT, &cmd_counter);
- return ret;
+ return 0;
}
--
1.7.2.5
3:05 a.m.
New subject: [PATCH 2/3] src: check for error returned by controlif_setup()
From: Pablo Neira Ayuso <pablo(a)gnumonks.org>
Bail out in case that we cannot bind the telnet interface.
---
openbsc/src/osmo-bsc/osmo_bsc_main.c | 6 +++++-
openbsc/src/osmo-bsc_nat/bsc_nat.c | 6 +++++-
openbsc/src/osmo-nitb/bsc_hack.c | 5 ++++-
3 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/openbsc/src/osmo-bsc/osmo_bsc_main.c b/openbsc/src/osmo-bsc/osmo_bsc_main.c
index 9a799c0..fe6526d 100644
--- a/openbsc/src/osmo-bsc/osmo_bsc_main.c
+++ b/openbsc/src/osmo-bsc/osmo_bsc_main.c
@@ -422,7 +422,11 @@ int main(int argc, char **argv)
}
bsc_api_init(bsc_gsmnet, osmo_bsc_api());
- controlif_setup(bsc_gsmnet, 4249);
+ if (controlif_setup(bsc_gsmnet, 4249) < 0) {
+ fprintf(stderr, "CTRL: Cannot bind to port 4249\n");
+ exit(1);
+ }
+
ctrl_cmd_install(CTRL_NODE_NET, &cmd_net_loc);
ctrl_cmd_install(CTRL_NODE_NET, &cmd_net_rf_lock);
ctrl_cmd_install(CTRL_NODE_TRX, &cmd_trx_rf_lock);
diff --git a/openbsc/src/osmo-bsc_nat/bsc_nat.c b/openbsc/src/osmo-bsc_nat/bsc_nat.c
index 295535a..326d04e 100644
--- a/openbsc/src/osmo-bsc_nat/bsc_nat.c
+++ b/openbsc/src/osmo-bsc_nat/bsc_nat.c
@@ -1760,7 +1760,11 @@ int main(int argc, char **argv)
exit(1);
}
- controlif_setup(NULL, 4250);
+ if (controlif_setup(NULL, 4250) < 0) {
+ fprintf(stderr, "CTRL: Cannot bind to port 4250\n");
+ exit(1);
+ }
+
ctrl_cmd_install(CTRL_NODE_ROOT, &cmd_fwd_cmd);
nat->msc_con->connection_loss = msc_connection_was_lost;
diff --git a/openbsc/src/osmo-nitb/bsc_hack.c b/openbsc/src/osmo-nitb/bsc_hack.c
index 001d8f9..5f9b7e5 100644
--- a/openbsc/src/osmo-nitb/bsc_hack.c
+++ b/openbsc/src/osmo-nitb/bsc_hack.c
@@ -254,7 +254,10 @@ int main(int argc, char **argv)
exit(1);
bsc_api_init(bsc_gsmnet, msc_bsc_api());
- controlif_setup(bsc_gsmnet, 4249);
+ if (controlif_setup(bsc_gsmnet, 4249) < 0) {
+ fprintf(stderr, "CTRL: Cannot bind to port 4249\n");
+ exit(1);
+ }
/* seed the PRNG */
srand(time(NULL));
--
1.7.2.5
3:05 a.m.
New subject: [PATCH 3/3] ctrl: do not disable nagle algorithm until we fully support segmentation
From: Pablo Neira Ayuso <pablo(a)gnumonks.org>
We shouldn't disable nagle, we have to finish TCP segmentation
support appropriately first.
---
openbsc/src/libctrl/control_if.c | 7 -------
1 files changed, 0 insertions(+), 7 deletions(-)
diff --git a/openbsc/src/libctrl/control_if.c b/openbsc/src/libctrl/control_if.c
index 58476ec..7e2ae16 100644
--- a/openbsc/src/libctrl/control_if.c
+++ b/openbsc/src/libctrl/control_if.c
@@ -241,15 +241,8 @@ err:
static int ctrl_accept_cb(struct ipa_server_link *ipa_link, int fd)
{
- int ret, on = 1;
struct ipa_server_conn *ipa_peer_link;
- ret = setsockopt(fd, IPPROTO_TCP, TCP_NODELAY, &on, sizeof(on));
- if (ret != 0) {
- LOGP(DNAT, LOGL_ERROR, "Failed to set TCP_NODELAY: %s\n", strerror(errno));
- close(fd);
- return ret;
- }
ipa_peer_link = ipa_server_conn_create(tall_bsc_ctx, ipa_link, fd,
handle_control_read,
ipa_link->data);
--
1.7.2.5
10:21 a.m.
New subject: [PATCH 3/3] ctrl: do not disable nagle algorithm until we fully support segmentation
Hi Pablo,
thanks for your patches.
On Fri, Sep 09, 2011 at 03:05:29AM +0200, pablo(a)gnumonks.org wrote:
We shouldn't disable nagle, we have to finish TCP
segmentation
support appropriately first.
I would like to hear Daniel's comment on those patches, particularly on
the reason he was disabling nagle...
--
- Harald Welte <laforge(a)gnumonks.org> http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
(ETSI EN 300 175-7 Ch. A6)
3:14 a.m.
On Fri, Sep 09, 2011 at 03:05:26AM +0200, pablo(a)gnumonks.org wrote:
From: Pablo Neira Ayuso <pablo(a)gnumonks.org>
Hi,
These patchset contains the port of libctrl to use the IPA
infrastructure available in libosmo-abis.
I've also made one small cleanup to bail out in case that
we cannot bind to the telnet port. Another patch follows up
to avoid disabling nagle (to avoid problems with TCP
segmentation, we still have to support this appropriately),
I noticed this while doing the port work.
Pablo Neira Ayuso (3):
ctrl: use generic IPA socket infrastructure available in libosmo-abis
src: check for error returned by controlif_setup()
ctrl: do not disable nagle algorithm until we fully support
segmentation
I forgot to say that they are available in the pablo/ctrl-updates
branch.
BTW, I have used the following command line interactive client to test
the changes. I made it because I wanted to test the IPA socket
infrastructure from the client side. Just in case you find it of any
use. You can find it attached.
3:51 p.m.
Pablo Neira Ayuso <pablo(a)gnumonks.org> wrote:
On Fri, Sep 09, 2011 at 03:05:26AM +0200,
pablo(a)gnumonks.org wrote:
> From: Pablo Neira Ayuso <pablo(a)gnumonks.org>
>
> Hi,
>
> These patchset contains the port of libctrl to use the IPA
> infrastructure available in libosmo-abis.
>
> I've also made one small cleanup to bail out in case that
> we cannot bind to the telnet port. Another patch follows up
> to avoid disabling nagle (to avoid problems with TCP
> segmentation, we still have to support this appropriately),
> I noticed this while doing the port work.
Thanks Pablo, I'll take a look at them tonight. Not sure about the nodelay issue, I
think it was an issue of doing it similar to other places in the code, but I'll
check.
Regards, Daniel
--
Sent from my Android phone with K-9 Mail. Please excuse my brevity.
10 Sep
10 Sep
3:57 p.m.
On Fri, Sep 09, 2011 at 03:51:08PM +0200, Daniel Willmann wrote:
Pablo Neira Ayuso <pablo(a)gnumonks.org> wrote:
The LOGP call in the code to disable nagle was using DNAT logging
level, so I guess it's a leftover. I still think we have to remove
this option until we fully support TCP segmentation (I'm working on
some patches for that, they will come but until that not sure what's
the benefit of disabling nagle anyway).
Wait for your comments!
On Fri, Sep 09, 2011 at 03:05:26AM +0200,
pablo(a)gnumonks.org wrote:
> From: Pablo Neira Ayuso <pablo(a)gnumonks.org>
>
> Hi,
>
> These patchset contains the port of libctrl to use the IPA
> infrastructure available in libosmo-abis.
>
> I've also made one small cleanup to bail out in case that
> we cannot bind to the telnet port. Another patch follows up
> to avoid disabling nagle (to avoid problems with TCP
> segmentation, we still have to support this appropriately),
> I noticed this while doing the port work.
Thanks Pablo, I'll take a look at them tonight. Not sure about the nodelay issue, I
think it was an issue of doing it similar to other places in the code, but I'll check.
11 Sep
11 Sep
7:22 a.m.
On 09/09/2011 03:14 AM, Pablo Neira Ayuso wrote:
> I've also made one small cleanup to bail out
in case that
> we cannot bind to the telnet port. Another patch follows up
> to avoid disabling nagle (to avoid problems with TCP
> segmentation, we still have to support this appropriately),
> I noticed this while doing the port work.
Hi Pablo,
besides having problems with TCP Segmentation in the reading code of the
ipaccess socket the NODELAY setsockopt was added to send out messages more
quickly.
holger
10 Sep
10 Sep
5:14 p.m.
Hello Pablo,
sorry, but this is going to take me longer than I previously thought.
Just some short remarks on what I noticed so far. After Tuesday I'll
have more time on my hands to help with porting.
On Fri, 9 Sep 2011 03:05:26 +0200
pablo(a)gnumonks.org wrote:
From: Pablo Neira Ayuso <pablo(a)gnumonks.org>
These patchset contains the port of libctrl to use the IPA
infrastructure available in libosmo-abis.
I've also made one small cleanup to bail out in
case that
we cannot bind to the telnet port. Another patch follows up
to avoid disabling nagle (to avoid problems with TCP
segmentation, we still have to support this appropriately),
I noticed this while doing the port work.
First of all thanks for starting the port. I had a brief look into that
a week ago, but ran into different problems.
Pablo Neira Ayuso (3):
ctrl: use generic IPA socket infrastructure available in
libosmo-abis
The control commands need to be sent both over the dedicated control
connection as well as between osmo-bsc and nat. It seems that the
bsc_msc_connection is still using osmo_wqueues which complicates
matters as I originally just used ctrl_cmd_send to send the response to
whatever osmo_wqueue the command came in from or forward commands from
the nat to the appropriate osmo-bsc.
bsc_nat also needs to know if a a control connection is closed so it
can remove any commands pending response from the pending list. The
list is needed to generate errors if the msc_bsc connection is reset or
a timeout occurs while a command is in transit. Could you add a
callback to ipa_server_conn that gets called if the connection is
closed?
src: check for error returned by controlif_setup()
That's certainly useful, thanks.
ctrl: do not disable nagle algorithm until we fully
support
segmentation
I had a look at commit 4462f8 and I believe I just tried to mimic the
behavior of msc_connection_connected() from ./osmo-bsc/osmo_bsc_msc.c
at that time.
Anyway, I'd be interested in the problems you encountered. I know that
ipaccess_read_msg mishandled the case where the data read returned only
a partial packet, but shouldn't that only occur if the data to be sent
is larger than the current MSS? In any case I would have expected
disabling the nagle algorithm to help in that regard...
So I guess there's still some work to do. I'll reserve some time on
Thursday to look at the issues.
Regards,
Daniel Willmann
11 Sep
11 Sep
1:04 p.m.
Hi Daniel,
On Sat, Sep 10, 2011 at 05:14:58PM +0200, Daniel Willmann wrote:
I think disabling nagle in that code is fine. Basically, if we
disable nagle we obtain the same behaviour that in UDP sockets, which
means that one send() call means one packet, and this is what we have
to do by now to avoid TCP segmentation issues (that we don't handle
appropriately). Drop the patch.
Hello Pablo,
sorry, but this is going to take me longer than I previously thought.
No problem, we're not in the rush :-).
Just some short remarks on what I noticed so far.
After Tuesday I'll
have more time on my hands to help with porting.
On Fri, 9 Sep 2011 03:05:26 +0200
pablo(a)gnumonks.org wrote:
Can you come with some idea to overcome this situation? Probably we
can use osmo_wqueues in the generic IPA infrastructure, let me give it
some spins.
From: Pablo Neira Ayuso
<pablo(a)gnumonks.org>
These patchset contains the port of libctrl to use the IPA
infrastructure available in libosmo-abis.
I've also made one small cleanup to bail out
in case that
we cannot bind to the telnet port. Another patch follows up
to avoid disabling nagle (to avoid problems with TCP
segmentation, we still have to support this appropriately),
I noticed this while doing the port work.
First of all thanks for starting the port. I had a brief look into that
a week ago, but ran into different problems.
Pablo Neira Ayuso (3):
ctrl: use generic IPA socket infrastructure available in
libosmo-abis
The control commands need to be sent both over the dedicated control
connection as well as between osmo-bsc and nat. It seems that the
bsc_msc_connection is still using osmo_wqueues which complicates
matters as I originally just used ctrl_cmd_send to send the response to
whatever osmo_wqueue the command came in from or forward commands from
the nat to the appropriate osmo-bsc. bsc_nat also needs to know if a a control connection
is closed so it
can remove any commands pending response from the pending list. The
list is needed to generate errors if the msc_bsc connection is reset or
a timeout occurs while a command is in transit. Could you add a
callback to ipa_server_conn that gets called if the connection is
closed?
Sure, we can add that. The idea is to make the IPA infrastructure
generic enough so we can use in as many places as we can.
src: check
for error returned by controlif_setup()
That's certainly useful, thanks.
ctrl: do not disable nagle algorithm until we
fully support
segmentation
I had a look at commit 4462f8 and I believe I just tried to mimic the
behavior of msc_connection_connected() from ./osmo-bsc/osmo_bsc_msc.c
at that time.
Anyway, I'd be interested in the problems you encountered. I know that
ipaccess_read_msg mishandled the case where the data read returned only
a partial packet, but shouldn't that only occur if the data to be sent
is larger than the current MSS? In any case I would have expected
disabling the nagle algorithm to help in that regard... So I guess there's still some work to do. I'll
reserve some time on
Thursday to look at the issues.
Fine, count to me if I can be of any help. I'll have a look at the
issues that you mentions along this week.
5034
days inactive
5036
days old
10 comments
5 participants
participants (5)
-
Daniel Willmann -
Harald Welte -
Holger Hans Peter Freyther -
Pablo Neira Ayuso -
pablo＠gnumonks.org