[PATCH 0/7] libosmo-abis: updates for generic IPA infrastructure
From: Pablo Neira Ayuso <pablo@gnumonks.org> Hi! This patchset contains several updates for the generic IPA infrastructure which provides an abstraction upon the generic socket infrastructure for IPA traffic. After this patchset, it will follow up one patch to use it in openBSC. You can find these changes in the pablo/ipa-updates branch of libosmo-abis. Please, merge them. Pablo Neira Ayuso (7): ipa: remove bogus driver_name parameter from ipa_client_create ipa: rename all reference to ipa_*_peer to ipa_*_conn tests: remove reference to internal headers in ipa_proxy_test ipa: use default write callback in ipa_client_conn_create if not specified ipa: fix segfault in ipa_client_conn_create if no E1 timeslot is specified ipa: better log error messages for ipa_msg_recv() ipa: use DEBUG level instead of NOTICE for debugging log messages include/osmocom/abis/ipa.h | 34 +++++----- src/input/hsl.c | 22 +++--- src/input/ipa.c | 153 ++++++++++++++++++++++---------------------- src/input/ipaccess.c | 26 ++++---- src/ipa_proxy.c | 22 +++--- tests/ipa_proxy_test.c | 4 +- 6 files changed, 128 insertions(+), 133 deletions(-) -- 1.7.2.5
From: Pablo Neira Ayuso <pablo@gnumonks.org> This parameter is not required since the line that we pass as parameter already has one driver attached. --- include/osmocom/abis/ipa.h | 2 +- src/input/hsl.c | 2 +- src/input/ipa.c | 8 ++------ src/input/ipaccess.c | 4 ++-- src/ipa_proxy.c | 2 +- 5 files changed, 7 insertions(+), 11 deletions(-) diff --git a/include/osmocom/abis/ipa.h b/include/osmocom/abis/ipa.h index 1a10ba9..1c6be15 100644 --- a/include/osmocom/abis/ipa.h +++ b/include/osmocom/abis/ipa.h @@ -54,7 +54,7 @@ struct ipa_client_link { void *data; }; -struct ipa_client_link *ipa_client_link_create(void *ctx, struct e1inp_ts *ts, const char *driver_name, int priv_nr, const char *addr, uint16_t port, int (*connect)(struct ipa_client_link *link), int (*read_cb)(struct ipa_client_link *link, struct msgb *msgb), int (*write_cb)(struct ipa_client_link *link), void *data); +struct ipa_client_link *ipa_client_link_create(void *ctx, struct e1inp_ts *ts, int priv_nr, const char *addr, uint16_t port, int (*connect)(struct ipa_client_link *link), int (*read_cb)(struct ipa_client_link *link, struct msgb *msgb), int (*write_cb)(struct ipa_client_link *link), void *data); void ipa_client_link_destroy(struct ipa_client_link *link); int ipa_client_write_default_cb(struct ipa_client_link *link); diff --git a/src/input/hsl.c b/src/input/hsl.c index 60eea17..040dbce 100644 --- a/src/input/hsl.c +++ b/src/input/hsl.c @@ -507,7 +507,7 @@ static int hsl_line_update(struct e1inp_line *line) link = ipa_client_link_create(tall_hsl_ctx, &line->ts[E1INP_SIGN_OML-1], - "hsl", E1INP_SIGN_OML, + E1INP_SIGN_OML, line->ops->cfg.ipa.addr, HSL_TCP_PORT, hsl_bts_connect, diff --git a/src/input/ipa.c b/src/input/ipa.c index b9bde68..83863f5 100644 --- a/src/input/ipa.c +++ b/src/input/ipa.c @@ -205,7 +205,7 @@ static int ipa_client_fd_cb(struct osmo_fd *ofd, unsigned int what) static void ipa_link_timer_cb(void *data); struct ipa_client_link * -ipa_client_link_create(void *ctx, struct e1inp_ts *ts, const char *driver_name, +ipa_client_link_create(void *ctx, struct e1inp_ts *ts, int priv_nr, const char *addr, uint16_t port, int (*connect_cb)(struct ipa_client_link *link), int (*read_cb)(struct ipa_client_link *link, @@ -220,14 +220,10 @@ ipa_client_link_create(void *ctx, struct e1inp_ts *ts, const char *driver_name, return NULL; if (ts) { - struct e1inp_driver *driver; - - driver = e1inp_driver_find(driver_name); - if (driver == NULL) { + if (ts->line->driver == NULL) { talloc_free(ipa_link); return NULL; } - ts->line->driver = driver; ipa_link->ofd = &ts->driver.ipaccess.fd; } else { ipa_link->ofd = talloc_zero(ctx, struct osmo_fd); diff --git a/src/input/ipaccess.c b/src/input/ipaccess.c index ea04e8d..3163a8c 100644 --- a/src/input/ipaccess.c +++ b/src/input/ipaccess.c @@ -881,7 +881,7 @@ static int ipaccess_line_update(struct e1inp_line *line) link = ipa_client_link_create(tall_ipa_ctx, &line->ts[E1INP_SIGN_OML-1], - "ipa", E1INP_SIGN_OML, + E1INP_SIGN_OML, line->ops->cfg.ipa.addr, IPA_TCP_PORT_OML, NULL, @@ -902,7 +902,7 @@ static int ipaccess_line_update(struct e1inp_line *line) } rsl_link = ipa_client_link_create(tall_ipa_ctx, &line->ts[E1INP_SIGN_RSL-1], - "ipa", E1INP_SIGN_RSL, + E1INP_SIGN_RSL, line->ops->cfg.ipa.addr, IPA_TCP_PORT_RSL, NULL, diff --git a/src/ipa_proxy.c b/src/ipa_proxy.c index 53da996..dadfd2f 100644 --- a/src/ipa_proxy.c +++ b/src/ipa_proxy.c @@ -185,7 +185,7 @@ ipa_sock_src_accept_cb(struct ipa_server_link *link, int fd) LOGP(DLINP, LOGL_NOTICE, "now trying to connect to destination\n"); - conn->dst = ipa_client_link_create(NULL, NULL, NULL, 0, + conn->dst = ipa_client_link_create(NULL, NULL, 0, route->shared->dst.inst->net.addr, route->shared->dst.inst->net.port, NULL, -- 1.7.2.5
From: Pablo Neira Ayuso <pablo@gnumonks.org> This patch is a cleanup. It renames several function from _peer to _conn which seems to me like a more logical name. There are no clients of this code out of the libosmo-abis tree so far, so this shouldn't break anything. --- include/osmocom/abis/ipa.h | 34 ++++++------ src/input/hsl.c | 20 ++++---- src/input/ipa.c | 118 ++++++++++++++++++++++---------------------- src/input/ipaccess.c | 22 ++++---- src/ipa_proxy.c | 20 ++++---- 5 files changed, 107 insertions(+), 107 deletions(-) diff --git a/include/osmocom/abis/ipa.h b/include/osmocom/abis/ipa.h index 1c6be15..fdf4b66 100644 --- a/include/osmocom/abis/ipa.h +++ b/include/osmocom/abis/ipa.h @@ -20,49 +20,49 @@ void ipa_server_link_destroy(struct ipa_server_link *link); int ipa_server_link_open(struct ipa_server_link *link); void ipa_server_link_close(struct ipa_server_link *link); -struct ipa_server_peer { +struct ipa_server_conn { struct ipa_server_link *server; struct osmo_fd ofd; struct llist_head tx_queue; - int (*cb)(struct ipa_server_peer *peer, struct msgb *msg); + int (*cb)(struct ipa_server_conn *peer, struct msgb *msg); void *data; }; -struct ipa_server_peer *ipa_server_peer_create(void *ctx, struct ipa_server_link *link, int fd, int (*cb)(struct ipa_server_peer *peer, struct msgb *msg), void *data); -void ipa_server_peer_destroy(struct ipa_server_peer *peer); +struct ipa_server_conn *ipa_server_conn_create(void *ctx, struct ipa_server_link *link, int fd, int (*cb)(struct ipa_server_conn *peer, struct msgb *msg), void *data); +void ipa_server_conn_destroy(struct ipa_server_conn *peer); -void ipa_server_peer_send(struct ipa_server_peer *peer, struct msgb *msg); +void ipa_server_conn_send(struct ipa_server_conn *peer, struct msgb *msg); -enum ipa_client_link_state { +enum ipa_client_conn_state { IPA_CLIENT_LINK_STATE_NONE = 0, IPA_CLIENT_LINK_STATE_CONNECTING = 1, IPA_CLIENT_LINK_STATE_CONNECTED = 2, IPA_CLIENT_LINK_STATE_MAX }; -struct ipa_client_link { +struct ipa_client_conn { struct e1inp_line *line; struct osmo_fd *ofd; struct llist_head tx_queue; struct osmo_timer_list timer; - enum ipa_client_link_state state; + enum ipa_client_conn_state state; const char *addr; uint16_t port; - int (*connect_cb)(struct ipa_client_link *link); - int (*read_cb)(struct ipa_client_link *link, struct msgb *msg); - int (*write_cb)(struct ipa_client_link *link); + int (*connect_cb)(struct ipa_client_conn *link); + int (*read_cb)(struct ipa_client_conn *link, struct msgb *msg); + int (*write_cb)(struct ipa_client_conn *link); void *data; }; -struct ipa_client_link *ipa_client_link_create(void *ctx, struct e1inp_ts *ts, int priv_nr, const char *addr, uint16_t port, int (*connect)(struct ipa_client_link *link), int (*read_cb)(struct ipa_client_link *link, struct msgb *msgb), int (*write_cb)(struct ipa_client_link *link), void *data); -void ipa_client_link_destroy(struct ipa_client_link *link); +struct ipa_client_conn *ipa_client_conn_create(void *ctx, struct e1inp_ts *ts, int priv_nr, const char *addr, uint16_t port, int (*connect)(struct ipa_client_conn *link), int (*read_cb)(struct ipa_client_conn *link, struct msgb *msgb), int (*write_cb)(struct ipa_client_conn *link), void *data); +void ipa_client_conn_destroy(struct ipa_client_conn *link); -int ipa_client_write_default_cb(struct ipa_client_link *link); +int ipa_client_write_default_cb(struct ipa_client_conn *link); -int ipa_client_link_open(struct ipa_client_link *link); -void ipa_client_link_close(struct ipa_client_link *link); +int ipa_client_conn_open(struct ipa_client_conn *link); +void ipa_client_conn_close(struct ipa_client_conn *link); -void ipa_client_link_send(struct ipa_client_link *link, struct msgb *msg); +void ipa_client_conn_send(struct ipa_client_conn *link, struct msgb *msg); int ipa_msg_recv(int fd, struct msgb **rmsg); diff --git a/src/input/hsl.c b/src/input/hsl.c index 040dbce..408228e 100644 --- a/src/input/hsl.c +++ b/src/input/hsl.c @@ -273,7 +273,7 @@ static int handle_ts1_write(struct osmo_fd *bfd) return __handle_ts1_write(bfd, line); } -int hsl_bts_write(struct ipa_client_link *link) +int hsl_bts_write(struct ipa_client_conn *link) { struct e1inp_line *line = link->line; @@ -383,7 +383,7 @@ static int listen_fd_cb(struct osmo_fd *listen_bfd, unsigned int what) return ret; } -static int hsl_bts_process(struct ipa_client_link *link, struct msgb *msg) +static int hsl_bts_process(struct ipa_client_conn *link, struct msgb *msg) { struct ipaccess_head *hh; struct e1inp_sign_link *sign_link; @@ -415,7 +415,7 @@ static int hsl_bts_process(struct ipa_client_link *link, struct msgb *msg) return 0; } -static int hsl_bts_connect(struct ipa_client_link *link) +static int hsl_bts_connect(struct ipa_client_conn *link) { struct msgb *msg; uint8_t *serno; @@ -442,7 +442,7 @@ static int hsl_bts_connect(struct ipa_client_link *link) if (!link->line->ops->sign_link_up) { LOGP(DLINP, LOGL_ERROR, "Unable to set signal link, closing socket.\n"); - ipa_client_link_close(link); + ipa_client_conn_close(link); return -EINVAL; } sign_link = link->line->ops->sign_link_up(&unit, @@ -450,7 +450,7 @@ static int hsl_bts_connect(struct ipa_client_link *link) if (sign_link == NULL) { LOGP(DLINP, LOGL_ERROR, "Unable to set signal link, closing socket.\n"); - ipa_client_link_close(link); + ipa_client_conn_close(link); return -EINVAL; } return 0; @@ -501,11 +501,11 @@ static int hsl_line_update(struct e1inp_line *line) } break; case E1INP_LINE_R_BTS: { - struct ipa_client_link *link; + struct ipa_client_conn *link; LOGP(DLINP, LOGL_NOTICE, "enabling hsl BTS mode\n"); - link = ipa_client_link_create(tall_hsl_ctx, + link = ipa_client_conn_create(tall_hsl_ctx, &line->ts[E1INP_SIGN_OML-1], E1INP_SIGN_OML, line->ops->cfg.ipa.addr, @@ -519,11 +519,11 @@ static int hsl_line_update(struct e1inp_line *line) strerror(errno)); return -ENOMEM; } - if (ipa_client_link_open(link) < 0) { + if (ipa_client_conn_open(link) < 0) { LOGP(DLINP, LOGL_ERROR, "cannot open BTS link: %s\n", strerror(errno)); - ipa_client_link_close(link); - ipa_client_link_destroy(link); + ipa_client_conn_close(link); + ipa_client_conn_destroy(link); return -EIO; } ret = 0; diff --git a/src/input/ipa.c b/src/input/ipa.c index 83863f5..020a730 100644 --- a/src/input/ipa.c +++ b/src/input/ipa.c @@ -90,24 +90,24 @@ int ipa_msg_recv(int fd, struct msgb **rmsg) return ret; } -void ipa_client_link_close(struct ipa_client_link *link); +void ipa_client_conn_close(struct ipa_client_conn *link); -static void ipa_client_retry(struct ipa_client_link *link) +static void ipa_client_retry(struct ipa_client_conn *link) { LOGP(DLINP, LOGL_NOTICE, "connection closed\n"); - ipa_client_link_close(link); + ipa_client_conn_close(link); LOGP(DLINP, LOGL_NOTICE, "retrying in 5 seconds...\n"); osmo_timer_schedule(&link->timer, 5, 0); link->state = IPA_CLIENT_LINK_STATE_CONNECTING; } -void ipa_client_link_close(struct ipa_client_link *link) +void ipa_client_conn_close(struct ipa_client_conn *link) { osmo_fd_unregister(link->ofd); close(link->ofd->fd); } -static void ipa_client_read(struct ipa_client_link *link) +static void ipa_client_read(struct ipa_client_conn *link) { struct osmo_fd *ofd = link->ofd; struct msgb *msg; @@ -133,13 +133,13 @@ static void ipa_client_read(struct ipa_client_link *link) link->read_cb(link, msg); } -static void ipa_client_write(struct ipa_client_link *link) +static void ipa_client_write(struct ipa_client_conn *link) { if (link->write_cb) link->write_cb(link); } -int ipa_client_write_default_cb(struct ipa_client_link *link) +int ipa_client_write_default_cb(struct ipa_client_conn *link) { struct osmo_fd *ofd = link->ofd; struct msgb *msg; @@ -169,7 +169,7 @@ int ipa_client_write_default_cb(struct ipa_client_link *link) static int ipa_client_fd_cb(struct osmo_fd *ofd, unsigned int what) { - struct ipa_client_link *link = ofd->data; + struct ipa_client_conn *link = ofd->data; int error, ret; socklen_t len = sizeof(error); @@ -204,18 +204,18 @@ static int ipa_client_fd_cb(struct osmo_fd *ofd, unsigned int what) static void ipa_link_timer_cb(void *data); -struct ipa_client_link * -ipa_client_link_create(void *ctx, struct e1inp_ts *ts, +struct ipa_client_conn * +ipa_client_conn_create(void *ctx, struct e1inp_ts *ts, int priv_nr, const char *addr, uint16_t port, - int (*connect_cb)(struct ipa_client_link *link), - int (*read_cb)(struct ipa_client_link *link, + int (*connect_cb)(struct ipa_client_conn *link), + int (*read_cb)(struct ipa_client_conn *link, struct msgb *msgb), - int (*write_cb)(struct ipa_client_link *link), + int (*write_cb)(struct ipa_client_conn *link), void *data) { - struct ipa_client_link *ipa_link; + struct ipa_client_conn *ipa_link; - ipa_link = talloc_zero(ctx, struct ipa_client_link); + ipa_link = talloc_zero(ctx, struct ipa_client_conn); if (!ipa_link) return NULL; @@ -252,12 +252,12 @@ ipa_client_link_create(void *ctx, struct e1inp_ts *ts, return ipa_link; } -void ipa_client_link_destroy(struct ipa_client_link *link) +void ipa_client_conn_destroy(struct ipa_client_conn *link) { talloc_free(link); } -int ipa_client_link_open(struct ipa_client_link *link) +int ipa_client_conn_open(struct ipa_client_conn *link) { int ret; @@ -278,20 +278,20 @@ int ipa_client_link_open(struct ipa_client_link *link) static void ipa_link_timer_cb(void *data) { - struct ipa_client_link *link = data; + struct ipa_client_conn *link = data; LOGP(DLINP, LOGL_NOTICE, "reconnecting.\n"); switch(link->state) { case IPA_CLIENT_LINK_STATE_CONNECTING: - ipa_client_link_open(link); + ipa_client_conn_open(link); break; default: break; } } -void ipa_client_link_send(struct ipa_client_link *link, struct msgb *msg) +void ipa_client_conn_send(struct ipa_client_conn *link, struct msgb *msg) { msgb_enqueue(&link->tx_queue, msg); link->ofd->when |= BSC_FD_WRITE; @@ -372,9 +372,9 @@ void ipa_server_link_close(struct ipa_server_link *link) close(link->ofd.fd); } -static void ipa_server_peer_read(struct ipa_server_peer *peer) +static void ipa_server_conn_read(struct ipa_server_conn *conn) { - struct osmo_fd *ofd = &peer->ofd; + struct osmo_fd *ofd = &conn->ofd; struct msgb *msg; int ret; @@ -390,91 +390,91 @@ static void ipa_server_peer_read(struct ipa_server_peer *peer) return; } else if (ret == 0) { LOGP(DLINP, LOGL_ERROR, "connection closed with server\n"); - ipa_server_peer_destroy(peer); + ipa_server_conn_destroy(conn); return; } - if (peer->cb) - peer->cb(peer, msg); + if (conn->cb) + conn->cb(conn, msg); return; } -static void ipa_server_peer_write(struct ipa_server_peer *peer) +static void ipa_server_conn_write(struct ipa_server_conn *conn) { - struct osmo_fd *ofd = &peer->ofd; + struct osmo_fd *ofd = &conn->ofd; struct msgb *msg; struct llist_head *lh; int ret; LOGP(DLINP, LOGL_NOTICE, "sending data\n"); - if (llist_empty(&peer->tx_queue)) { + if (llist_empty(&conn->tx_queue)) { ofd->when &= ~BSC_FD_WRITE; return; } - lh = peer->tx_queue.next; + lh = conn->tx_queue.next; llist_del(lh); msg = llist_entry(lh, struct msgb, list); - ret = send(peer->ofd.fd, msg->data, msg->len, 0); + ret = send(conn->ofd.fd, msg->data, msg->len, 0); if (ret < 0) { LOGP(DLINP, LOGL_ERROR, "error to send\n"); } msgb_free(msg); } -static int ipa_server_peer_cb(struct osmo_fd *ofd, unsigned int what) +static int ipa_server_conn_cb(struct osmo_fd *ofd, unsigned int what) { - struct ipa_server_peer *peer = ofd->data; + struct ipa_server_conn *conn = ofd->data; LOGP(DLINP, LOGL_NOTICE, "connected read/write\n"); if (what & BSC_FD_READ) - ipa_server_peer_read(peer); + ipa_server_conn_read(conn); if (what & BSC_FD_WRITE) - ipa_server_peer_write(peer); + ipa_server_conn_write(conn); return 0; } -struct ipa_server_peer * -ipa_server_peer_create(void *ctx, struct ipa_server_link *link, int fd, - int (*cb)(struct ipa_server_peer *peer, struct msgb *msg), +struct ipa_server_conn * +ipa_server_conn_create(void *ctx, struct ipa_server_link *link, int fd, + int (*cb)(struct ipa_server_conn *conn, struct msgb *msg), void *data) { - struct ipa_server_peer *peer; + struct ipa_server_conn *conn; - peer = talloc_zero(ctx, struct ipa_server_peer); - if (peer == NULL) { + conn = talloc_zero(ctx, struct ipa_server_conn); + if (conn == NULL) { LOGP(DLINP, LOGL_ERROR, "cannot allocate new peer in server, " "reason=`%s'\n", strerror(errno)); return NULL; } - peer->server = link; - peer->ofd.fd = fd; - peer->ofd.data = peer; - peer->ofd.cb = ipa_server_peer_cb; - peer->ofd.when = BSC_FD_READ; - peer->cb = cb; - peer->data = data; - INIT_LLIST_HEAD(&peer->tx_queue); - - if (osmo_fd_register(&peer->ofd) < 0) { + conn->server = link; + conn->ofd.fd = fd; + conn->ofd.data = conn; + conn->ofd.cb = ipa_server_conn_cb; + conn->ofd.when = BSC_FD_READ; + conn->cb = cb; + conn->data = data; + INIT_LLIST_HEAD(&conn->tx_queue); + + if (osmo_fd_register(&conn->ofd) < 0) { LOGP(DLINP, LOGL_ERROR, "could not register FD\n"); - talloc_free(peer); + talloc_free(conn); return NULL; } - return peer; + return conn; } -void ipa_server_peer_destroy(struct ipa_server_peer *peer) +void ipa_server_conn_destroy(struct ipa_server_conn *conn) { - close(peer->ofd.fd); - osmo_fd_unregister(&peer->ofd); - talloc_free(peer); + close(conn->ofd.fd); + osmo_fd_unregister(&conn->ofd); + talloc_free(conn); } -void ipa_server_peer_send(struct ipa_server_peer *peer, struct msgb *msg) +void ipa_server_conn_send(struct ipa_server_conn *conn, struct msgb *msg) { - msgb_enqueue(&peer->tx_queue, msg); - peer->ofd.when |= BSC_FD_WRITE; + msgb_enqueue(&conn->tx_queue, msg); + conn->ofd.when |= BSC_FD_WRITE; } diff --git a/src/input/ipaccess.c b/src/input/ipaccess.c index 3163a8c..9c501de 100644 --- a/src/input/ipaccess.c +++ b/src/input/ipaccess.c @@ -531,7 +531,7 @@ static int handle_ts1_write(struct osmo_fd *bfd) return __handle_ts1_write(bfd, line); } -static int ipaccess_bts_write_cb(struct ipa_client_link *link) +static int ipaccess_bts_write_cb(struct ipa_client_conn *link) { struct e1inp_line *line = link->line; @@ -735,7 +735,7 @@ static struct msgb *ipa_bts_id_ack(void) return nmsg2; } -static int ipaccess_bts_cb(struct ipa_client_link *link, struct msgb *msg) +static int ipaccess_bts_cb(struct ipa_client_conn *link, struct msgb *msg) { struct ipaccess_head *hh = (struct ipaccess_head *) msg->data; struct e1inp_ts *e1i_ts = NULL; @@ -875,11 +875,11 @@ static int ipaccess_line_update(struct e1inp_line *line) break; } case E1INP_LINE_R_BTS: { - struct ipa_client_link *link, *rsl_link; + struct ipa_client_conn *link, *rsl_link; LOGP(DLINP, LOGL_NOTICE, "enabling ipaccess BTS mode\n"); - link = ipa_client_link_create(tall_ipa_ctx, + link = ipa_client_conn_create(tall_ipa_ctx, &line->ts[E1INP_SIGN_OML-1], E1INP_SIGN_OML, line->ops->cfg.ipa.addr, @@ -893,14 +893,14 @@ static int ipaccess_line_update(struct e1inp_line *line) "BTS link: %s\n", strerror(errno)); return -ENOMEM; } - if (ipa_client_link_open(link) < 0) { + if (ipa_client_conn_open(link) < 0) { LOGP(DLINP, LOGL_ERROR, "cannot open OML BTS link: %s\n", strerror(errno)); - ipa_client_link_close(link); - ipa_client_link_destroy(link); + ipa_client_conn_close(link); + ipa_client_conn_destroy(link); return -EIO; } - rsl_link = ipa_client_link_create(tall_ipa_ctx, + rsl_link = ipa_client_conn_create(tall_ipa_ctx, &line->ts[E1INP_SIGN_RSL-1], E1INP_SIGN_RSL, line->ops->cfg.ipa.addr, @@ -914,11 +914,11 @@ static int ipaccess_line_update(struct e1inp_line *line) "BTS link: %s\n", strerror(errno)); return -ENOMEM; } - if (ipa_client_link_open(rsl_link) < 0) { + if (ipa_client_conn_open(rsl_link) < 0) { LOGP(DLINP, LOGL_ERROR, "cannot open RSL BTS link: %s\n", strerror(errno)); - ipa_client_link_close(rsl_link); - ipa_client_link_destroy(rsl_link); + ipa_client_conn_close(rsl_link); + ipa_client_conn_destroy(rsl_link); return -EIO; } ret = 0; diff --git a/src/ipa_proxy.c b/src/ipa_proxy.c index dadfd2f..6f2525a 100644 --- a/src/ipa_proxy.c +++ b/src/ipa_proxy.c @@ -104,15 +104,15 @@ enum ipa_conn_state { struct ipa_proxy_conn { struct llist_head head; - struct ipa_server_peer *src; - struct ipa_client_link *dst; + struct ipa_server_conn *src; + struct ipa_client_conn *dst; struct ipa_proxy_route *route; }; /* * socket callbacks used by IPA VTY commands */ -static int ipa_sock_dst_cb(struct ipa_client_link *link, struct msgb *msg) +static int ipa_sock_dst_cb(struct ipa_client_conn *link, struct msgb *msg) { struct ipaccess_head *hh; struct ipa_proxy_conn *conn = link->data; @@ -132,11 +132,11 @@ static int ipa_sock_dst_cb(struct ipa_client_link *link, struct msgb *msg) /* mangle message, if required. */ hh->proto = conn->route->shared->src.streamid[hh->proto]; - ipa_server_peer_send(conn->src, msg); + ipa_server_conn_send(conn->src, msg); return 0; } -static int ipa_sock_src_cb(struct ipa_server_peer *peer, struct msgb *msg) +static int ipa_sock_src_cb(struct ipa_server_conn *peer, struct msgb *msg) { struct ipaccess_head *hh; struct ipa_proxy_conn *conn = peer->data; @@ -155,7 +155,7 @@ static int ipa_sock_src_cb(struct ipa_server_peer *peer, struct msgb *msg) /* mangle message, if required. */ hh->proto = conn->route->shared->dst.streamid[hh->proto]; - ipa_client_link_send(conn->dst, msg); + ipa_client_conn_send(conn->dst, msg); return 0; } @@ -175,7 +175,7 @@ ipa_sock_src_accept_cb(struct ipa_server_link *link, int fd) } conn->route = route; - conn->src = ipa_server_peer_create(tall_ipa_proxy_ctx, link, fd, + conn->src = ipa_server_conn_create(tall_ipa_proxy_ctx, link, fd, ipa_sock_src_cb, conn); if (conn->src == NULL) { LOGP(DLINP, LOGL_ERROR, "could not create server peer: %s\n", @@ -185,7 +185,7 @@ ipa_sock_src_accept_cb(struct ipa_server_link *link, int fd) LOGP(DLINP, LOGL_NOTICE, "now trying to connect to destination\n"); - conn->dst = ipa_client_link_create(NULL, NULL, 0, + conn->dst = ipa_client_conn_create(NULL, NULL, 0, route->shared->dst.inst->net.addr, route->shared->dst.inst->net.port, NULL, @@ -197,7 +197,7 @@ ipa_sock_src_accept_cb(struct ipa_server_link *link, int fd) strerror(errno)); return -ENOMEM; } - if (ipa_client_link_open(conn->dst) < 0) { + if (ipa_client_conn_open(conn->dst) < 0) { LOGP(DLINP, LOGL_ERROR, "could not start client: %s\n", strerror(errno)); return -ENOMEM; @@ -546,7 +546,7 @@ DEFUN(ipa_route_del, ipa_route_del_cmd, /* nobody else using this route, release all resources. */ llist_for_each_entry_safe(conn, tmp, &matching_route->shared->conn_list, head) { - ipa_server_peer_destroy(conn->src); + ipa_server_conn_destroy(conn->src); llist_del(&conn->head); talloc_free(conn); } -- 1.7.2.5
From: Pablo Neira Ayuso <pablo@gnumonks.org> This example has to compile out of the tree, including internal.h is not required and using PACKAGE_VERSION here, which is internal to the library, does not seem to me like a good idea. --- tests/ipa_proxy_test.c | 4 +--- 1 files changed, 1 insertions(+), 3 deletions(-) diff --git a/tests/ipa_proxy_test.c b/tests/ipa_proxy_test.c index 6288a3a..2ed30ff 100644 --- a/tests/ipa_proxy_test.c +++ b/tests/ipa_proxy_test.c @@ -8,8 +8,6 @@ #include <osmocom/vty/vty.h> #include <osmocom/vty/command.h> #include <osmocom/vty/telnet_interface.h> -#include "internal.h" -#include "config.h" static void *tall_test; @@ -52,7 +50,7 @@ static enum node_type bsc_vty_go_parent(struct vty *vty) static struct vty_app_info vty_info = { .name = "ipa-proxy-test", - .version = PACKAGE_VERSION, + .version = "1.0", .go_parent_cb = bsc_vty_go_parent, .is_config_node = bsc_vty_is_config_node, }; -- 1.7.2.5
On 09/09/2011 02:36 AM, pablo@gnumonks.org wrote:
From: Pablo Neira Ayuso <pablo@gnumonks.org>
This example has to compile out of the tree, including internal.h is not required and using PACKAGE_VERSION here, which is internal to the library, does not seem to me like a good idea.
Hi Pablo, why should it compile out of tree? Ideally it would be hooked into make check and use GNU autotest (to match test output with expected results). In case we want to have it as an example, I think we could move it into a/the documentation folder and somehow include it in doxygen documentation? holger
Hi Holger, On Sun, Sep 11, 2011 at 11:42:46PM +0200, Holger Hans Peter Freyther wrote:
On 09/09/2011 02:36 AM, pablo@gnumonks.org wrote:
From: Pablo Neira Ayuso <pablo@gnumonks.org>
This example has to compile out of the tree, including internal.h is not required and using PACKAGE_VERSION here, which is internal to the library, does not seem to me like a good idea.
why should it compile out of tree? Ideally it would be hooked into make check and use GNU autotest (to match test output with expected results).
Although they are integrated in the build system, they are examples. Thus, someone may take of those files and use part of it to integrate it into the own application. That's why I think it's a good idea that they can be compiled out of tree.
In case we want to have it as an example, I think we could move it into a/the documentation folder and somehow include it in doxygen documentation?
Probably test is not a good name for the folder, we could rename it to utils/ or examples/. I can add some note in the documentation to tell about these examples. I'll add these to my "to do" list.
From: Pablo Neira Ayuso <pablo@gnumonks.org> If no write callback is specified, use the default write callback. Thus, we don't need to export ipa_client_write_default_cb. No clients of this function outside libosmo-abis, so no breakages should be expected. --- include/osmocom/abis/ipa.h | 2 -- src/input/ipa.c | 6 ++++-- src/ipa_proxy.c | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/include/osmocom/abis/ipa.h b/include/osmocom/abis/ipa.h index fdf4b66..43422dc 100644 --- a/include/osmocom/abis/ipa.h +++ b/include/osmocom/abis/ipa.h @@ -57,8 +57,6 @@ struct ipa_client_conn { struct ipa_client_conn *ipa_client_conn_create(void *ctx, struct e1inp_ts *ts, int priv_nr, const char *addr, uint16_t port, int (*connect)(struct ipa_client_conn *link), int (*read_cb)(struct ipa_client_conn *link, struct msgb *msgb), int (*write_cb)(struct ipa_client_conn *link), void *data); void ipa_client_conn_destroy(struct ipa_client_conn *link); -int ipa_client_write_default_cb(struct ipa_client_conn *link); - int ipa_client_conn_open(struct ipa_client_conn *link); void ipa_client_conn_close(struct ipa_client_conn *link); diff --git a/src/input/ipa.c b/src/input/ipa.c index 020a730..774d578 100644 --- a/src/input/ipa.c +++ b/src/input/ipa.c @@ -139,7 +139,7 @@ static void ipa_client_write(struct ipa_client_conn *link) link->write_cb(link); } -int ipa_client_write_default_cb(struct ipa_client_conn *link) +static int ipa_client_write_default_cb(struct ipa_client_conn *link) { struct osmo_fd *ofd = link->ofd; struct msgb *msg; @@ -244,7 +244,9 @@ ipa_client_conn_create(void *ctx, struct e1inp_ts *ts, ipa_link->port = port; ipa_link->connect_cb = connect_cb; ipa_link->read_cb = read_cb; - ipa_link->write_cb = write_cb; + /* default to generic write callback if not set. */ + if (write_cb == NULL) + ipa_link->write_cb = ipa_client_write_default_cb; ipa_link->line = ts->line; ipa_link->data = data; INIT_LLIST_HEAD(&ipa_link->tx_queue); diff --git a/src/ipa_proxy.c b/src/ipa_proxy.c index 6f2525a..f4e1df8 100644 --- a/src/ipa_proxy.c +++ b/src/ipa_proxy.c @@ -190,7 +190,7 @@ ipa_sock_src_accept_cb(struct ipa_server_link *link, int fd) route->shared->dst.inst->net.port, NULL, ipa_sock_dst_cb, - ipa_client_write_default_cb, + NULL, conn); if (conn->dst == NULL) { LOGP(DLINP, LOGL_ERROR, "could not create client: %s\n", -- 1.7.2.5
From: Pablo Neira Ayuso <pablo@gnumonks.org> Fix segfault if IPA client is not used as signalling link (in that case E1 timeslot is NULL). --- src/input/ipa.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/src/input/ipa.c b/src/input/ipa.c index 774d578..4773dff 100644 --- a/src/input/ipa.c +++ b/src/input/ipa.c @@ -247,7 +247,8 @@ ipa_client_conn_create(void *ctx, struct e1inp_ts *ts, /* default to generic write callback if not set. */ if (write_cb == NULL) ipa_link->write_cb = ipa_client_write_default_cb; - ipa_link->line = ts->line; + if (ts) + ipa_link->line = ts->line; ipa_link->data = data; INIT_LLIST_HEAD(&ipa_link->tx_queue); -- 1.7.2.5
From: Pablo Neira Ayuso <pablo@gnumonks.org> More descriptive errors help to debug problems. --- src/input/ipa.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/input/ipa.c b/src/input/ipa.c index 4773dff..a519f07 100644 --- a/src/input/ipa.c +++ b/src/input/ipa.c @@ -63,6 +63,7 @@ int ipa_msg_recv(int fd, struct msgb **rmsg) msgb_free(msg); return ret; } else if (ret != sizeof(*hh)) { + LOGP(DLINP, LOGL_ERROR, "too small message received\n"); msgb_free(msg); return -EIO; } @@ -73,6 +74,8 @@ int ipa_msg_recv(int fd, struct msgb **rmsg) len = ntohs(hh->len); if (len < 0 || IPA_ALLOC_SIZE < len + sizeof(*hh)) { + LOGP(DLINP, LOGL_ERROR, "bad message length of %d bytes, " + "received %d bytes\n", len, ret); msgb_free(msg); return -EIO; } @@ -82,6 +85,7 @@ int ipa_msg_recv(int fd, struct msgb **rmsg) msgb_free(msg); return ret; } else if (ret < len) { + LOGP(DLINP, LOGL_ERROR, "trunked message received\n"); msgb_free(msg); return -EIO; } @@ -119,8 +123,6 @@ static void ipa_client_read(struct ipa_client_conn *link) if (ret < 0) { if (errno == EPIPE || errno == ECONNRESET) { LOGP(DLINP, LOGL_ERROR, "lost connection with server\n"); - } else { - LOGP(DLINP, LOGL_ERROR, "unknown error\n"); } ipa_client_retry(link); return; @@ -387,8 +389,6 @@ static void ipa_server_conn_read(struct ipa_server_conn *conn) if (ret < 0) { if (errno == EPIPE || errno == ECONNRESET) { LOGP(DLINP, LOGL_ERROR, "lost connection with server\n"); - } else { - LOGP(DLINP, LOGL_ERROR, "unknown error\n"); } return; } else if (ret == 0) { -- 1.7.2.5
From: Pablo Neira Ayuso <pablo@gnumonks.org> Use DEBUG instead of NOTICE for several messages that are actually used for debugging purposes. --- src/input/ipa.c | 14 +++++++------- 1 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/input/ipa.c b/src/input/ipa.c index a519f07..2441de4 100644 --- a/src/input/ipa.c +++ b/src/input/ipa.c @@ -117,7 +117,7 @@ static void ipa_client_read(struct ipa_client_conn *link) struct msgb *msg; int ret; - LOGP(DLINP, LOGL_NOTICE, "message received\n"); + LOGP(DLINP, LOGL_DEBUG, "message received\n"); ret = ipa_msg_recv(ofd->fd, &msg); if (ret < 0) { @@ -148,7 +148,7 @@ static int ipa_client_write_default_cb(struct ipa_client_conn *link) struct llist_head *lh; int ret; - LOGP(DLINP, LOGL_NOTICE, "sending data\n"); + LOGP(DLINP, LOGL_DEBUG, "sending data\n"); if (llist_empty(&link->tx_queue)) { ofd->when &= ~BSC_FD_WRITE; @@ -190,11 +190,11 @@ static int ipa_client_fd_cb(struct osmo_fd *ofd, unsigned int what) break; case IPA_CLIENT_LINK_STATE_CONNECTED: if (what & BSC_FD_READ) { - LOGP(DLINP, LOGL_NOTICE, "connected read\n"); + LOGP(DLINP, LOGL_DEBUG, "connected read\n"); ipa_client_read(link); } if (what & BSC_FD_WRITE) { - LOGP(DLINP, LOGL_NOTICE, "connected write\n"); + LOGP(DLINP, LOGL_DEBUG, "connected write\n"); ipa_client_write(link); } break; @@ -383,7 +383,7 @@ static void ipa_server_conn_read(struct ipa_server_conn *conn) struct msgb *msg; int ret; - LOGP(DLINP, LOGL_NOTICE, "message received\n"); + LOGP(DLINP, LOGL_DEBUG, "message received\n"); ret = ipa_msg_recv(ofd->fd, &msg); if (ret < 0) { @@ -409,7 +409,7 @@ static void ipa_server_conn_write(struct ipa_server_conn *conn) struct llist_head *lh; int ret; - LOGP(DLINP, LOGL_NOTICE, "sending data\n"); + LOGP(DLINP, LOGL_DEBUG, "sending data\n"); if (llist_empty(&conn->tx_queue)) { ofd->when &= ~BSC_FD_WRITE; @@ -430,7 +430,7 @@ static int ipa_server_conn_cb(struct osmo_fd *ofd, unsigned int what) { struct ipa_server_conn *conn = ofd->data; - LOGP(DLINP, LOGL_NOTICE, "connected read/write\n"); + LOGP(DLINP, LOGL_DEBUG, "connected read/write\n"); if (what & BSC_FD_READ) ipa_server_conn_read(conn); if (what & BSC_FD_WRITE) -- 1.7.2.5
Hi Pablo, thanks for your patches On Fri, Sep 09, 2011 at 02:36:54AM +0200, pablo@gnumonks.org wrote:
This patchset contains several updates for the generic IPA infrastructure which provides an abstraction upon the generic socket infrastructure for IPA traffic.
thanks, merged. (though I think the rename from ipa_socket_peer to ipa_socket_conn was somewhat unneeded). -- - Harald Welte <laforge@gnumonks.org> http://laforge.gnumonks.org/ ============================================================================ "Privacy in residential applications is a desirable marketing option." (ETSI EN 300 175-7 Ch. A6)
participants (4)
-
Harald Welte -
Holger Hans Peter Freyther -
Pablo Neira Ayuso -
pablo＠gnumonks.org