From: Pablo Neira Ayuso pablo@gnumonks.org
This patchset is the second in the series to cleanup redundant code in ipaccess tools.
They have been tested by bootstraping the nanoBTS successfully in a small setup with two laptops, one acting as proxy and another with osmo-nitb.
They are available in the pablo/cleanups branch in openbsc git tree.
Please, merge it! Thanks.
Pablo Neira Ayuso (6): ipaccess-proxy: get rid of make_listen_sock() and use make_sock() instead libabis: ipaccess: conclude new ipaccess_send_*() functions and use them libabis: ipaccess: export ipaccess_idtag_name() ipaccess-proxy: get rid of ipac_idtag_parse() libabis: ipaccess: export ipaccess_parse_unitid ipaccess-proxy: remove hardcoded IP for options `-l' and `-b'
openbsc/include/openbsc/ipaccess.h | 3 + openbsc/src/ipaccess/Makefile.am | 7 +- openbsc/src/ipaccess/ipaccess-proxy.c | 188 ++++++--------------------------- openbsc/src/libabis/input/ipaccess.c | 75 +++++++++---- 4 files changed, 94 insertions(+), 179 deletions(-)
From: Pablo Neira Ayuso pablo@gnumonks.org
This patch replaces make_listen_sock() by the generic make_sock() available in libcommon/socket. --- openbsc/src/ipaccess/ipaccess-proxy.c | 51 ++------------------------------ 1 files changed, 4 insertions(+), 47 deletions(-)
diff --git a/openbsc/src/ipaccess/ipaccess-proxy.c b/openbsc/src/ipaccess/ipaccess-proxy.c index a6b433f..d1c7a33 100644 --- a/openbsc/src/ipaccess/ipaccess-proxy.c +++ b/openbsc/src/ipaccess/ipaccess-proxy.c @@ -1078,49 +1078,6 @@ static int gprs_ns_cb(struct bsc_fd *bfd, unsigned int what) return 0; }
-static int make_listen_sock(struct bsc_fd *bfd, u_int16_t port, int priv_nr, - int (*cb)(struct bsc_fd *fd, unsigned int what)) -{ - struct sockaddr_in addr; - int ret, on = 1; - - bfd->fd = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP); - bfd->cb = cb; - bfd->when = BSC_FD_READ; - bfd->priv_nr = priv_nr; - - memset(&addr, 0, sizeof(addr)); - addr.sin_family = AF_INET; - addr.sin_port = htons(port); - if (!listen_ipaddr) - addr.sin_addr.s_addr = INADDR_ANY; - else - inet_aton(listen_ipaddr, &addr.sin_addr); - - setsockopt(bfd->fd, SOL_SOCKET, SO_REUSEADDR, &on, sizeof(on)); - - ret = bind(bfd->fd, (struct sockaddr *) &addr, sizeof(addr)); - if (ret < 0) { - LOGP(DINP, LOGL_ERROR, - "Could not bind listen socket for IP %s with error: %s.\n", - listen_ipaddr, strerror(errno)); - return -EIO; - } - - ret = listen(bfd->fd, 1); - if (ret < 0) { - perror("listen"); - return ret; - } - - ret = bsc_register_fd(bfd); - if (ret < 0) { - perror("register_listen_fd"); - return ret; - } - return 0; -} - static int make_gprs_sock(struct bsc_fd *bfd, int (*cb)(struct bsc_fd*,unsigned int), void *data) { struct sockaddr_in addr; @@ -1206,14 +1163,14 @@ static int ipaccess_proxy_setup(void) ipp->reconn_timer.data = ipp;
/* Listen for OML connections */ - ret = make_listen_sock(&ipp->oml_listen_fd, IPA_TCP_PORT_OML, - OML_FROM_BTS, listen_fd_cb); + ret = make_sock(&ipp->oml_listen_fd, IPPROTO_TCP, INADDR_ANY, + IPA_TCP_PORT_OML, OML_FROM_BTS, listen_fd_cb, NULL); if (ret < 0) return ret;
/* Listen for RSL connections */ - ret = make_listen_sock(&ipp->rsl_listen_fd, IPA_TCP_PORT_RSL, - RSL_FROM_BTS, listen_fd_cb); + ret = make_sock(&ipp->rsl_listen_fd, IPPROTO_TCP, INADDR_ANY, + IPA_TCP_PORT_RSL, RSL_FROM_BTS, listen_fd_cb, NULL);
if (ret < 0) return ret;
From: Pablo Neira Ayuso pablo@gnumonks.org
This patch finishes the new ipaccess_send_*() functions and use them in the ipaccess-proxy code.
I have also cleanup the definition of the PONG, ID_ACK and ID_REQ messages (including some minor documentation about them).
I had to rename ipaccess_recvmsg() in ipaccess-proxy to avoid clashing with the one defined in libabis. --- openbsc/include/openbsc/ipaccess.h | 1 + openbsc/src/ipaccess/Makefile.am | 7 +++- openbsc/src/ipaccess/ipaccess-proxy.c | 29 +++------------ openbsc/src/libabis/input/ipaccess.c | 65 ++++++++++++++++++++++++--------- 4 files changed, 60 insertions(+), 42 deletions(-)
diff --git a/openbsc/include/openbsc/ipaccess.h b/openbsc/include/openbsc/ipaccess.h index 314ca90..b286f93 100644 --- a/openbsc/include/openbsc/ipaccess.h +++ b/openbsc/include/openbsc/ipaccess.h @@ -70,6 +70,7 @@ int ipaccess_connect(struct e1inp_line *line, struct sockaddr_in *sa); int ipaccess_rcvmsg_base(struct msgb *msg, struct bsc_fd *bfd); struct msgb *ipaccess_read_msg(struct bsc_fd *bfd, int *error); void ipaccess_prepend_header(struct msgb *msg, int proto); +int ipaccess_send_pong(int fd); int ipaccess_send_id_ack(int fd); int ipaccess_send_id_req(int fd);
diff --git a/openbsc/src/ipaccess/Makefile.am b/openbsc/src/ipaccess/Makefile.am index c997c29..cff3505 100644 --- a/openbsc/src/ipaccess/Makefile.am +++ b/openbsc/src/ipaccess/Makefile.am @@ -18,4 +18,9 @@ ipaccess_config_LDADD = $(top_builddir)/src/libbsc/libbsc.a \ -ldl -ldbi $(LIBCRYPT)
ipaccess_proxy_SOURCES = ipaccess-proxy.c -ipaccess_proxy_LDADD = $(top_builddir)/src/libcommon/libcommon.a +ipaccess_proxy_LDADD = $(top_builddir)/src/libbsc/libbsc.a \ + $(top_builddir)/src/libmsc/libmsc.a \ + $(top_builddir)/src/libabis/libabis.a \ + $(top_builddir)/src/libbsc/libbsc.a \ + $(top_builddir)/src/libtrau/libtrau.a \ + $(top_builddir)/src/libcommon/libcommon.a diff --git a/openbsc/src/ipaccess/ipaccess-proxy.c b/openbsc/src/ipaccess/ipaccess-proxy.c index d1c7a33..b8b42c0 100644 --- a/openbsc/src/ipaccess/ipaccess-proxy.c +++ b/openbsc/src/ipaccess/ipaccess-proxy.c @@ -131,19 +131,6 @@ static int gprs_ns_cb(struct bsc_fd *bfd, unsigned int what);
#define PROXY_ALLOC_SIZE 1200
-static const u_int8_t pong[] = { 0, 1, IPAC_PROTO_IPACCESS, IPAC_MSGT_PONG }; -static const u_int8_t id_ack[] = { 0, 1, IPAC_PROTO_IPACCESS, IPAC_MSGT_ID_ACK }; -static const u_int8_t id_req[] = { 0, 17, IPAC_PROTO_IPACCESS, IPAC_MSGT_ID_GET, - 0x01, IPAC_IDTAG_UNIT, - 0x01, IPAC_IDTAG_MACADDR, - 0x01, IPAC_IDTAG_LOCATION1, - 0x01, IPAC_IDTAG_LOCATION2, - 0x01, IPAC_IDTAG_EQUIPVERS, - 0x01, IPAC_IDTAG_SWVERSION, - 0x01, IPAC_IDTAG_UNITNAME, - 0x01, IPAC_IDTAG_SERNR, - }; - static const char *idtag_names[] = { [IPAC_IDTAG_SERNR] = "Serial_Number", [IPAC_IDTAG_UNITNAME] = "Unit_Name", @@ -529,13 +516,7 @@ static int ipaccess_rcvmsg(struct ipa_proxy_conn *ipc, struct msgb *msg,
switch (msg_type) { case IPAC_MSGT_PING: - ret = write(bfd->fd, pong, sizeof(pong)); - if (ret < 0) - return ret; - if (ret < sizeof(pong)) { - DEBUGP(DINP, "short write\n"); - return -EIO; - } + ret = ipaccess_send_pong(bfd->fd); break; case IPAC_MSGT_PONG: DEBUGP(DMI, "PONG!\n"); @@ -618,7 +599,7 @@ static int ipaccess_rcvmsg(struct ipa_proxy_conn *ipc, struct msgb *msg, break; case IPAC_MSGT_ID_ACK: DEBUGP(DMI, "ID_ACK? -> ACK!\n"); - ret = write(bfd->fd, id_ack, sizeof(id_ack)); + ret = ipaccess_send_id_ack(bfd->fd); break; default: LOGP(DMI, LOGL_ERROR, "Unhandled IPA type; %d\n", msg_type); @@ -628,7 +609,7 @@ static int ipaccess_rcvmsg(struct ipa_proxy_conn *ipc, struct msgb *msg, return 0; }
-struct msgb *ipaccess_read_msg(struct bsc_fd *bfd, int *error) +struct msgb *ipaccess_proxy_read_msg(struct bsc_fd *bfd, int *error) { struct msgb *msg = msgb_alloc(PROXY_ALLOC_SIZE, "Abis/IP"); struct ipaccess_head *hh; @@ -868,7 +849,7 @@ static int handle_tcp_read(struct bsc_fd *bfd) else btsbsc = "BSC";
- msg = ipaccess_read_msg(bfd, &ret); + msg = ipaccess_proxy_read_msg(bfd, &ret); if (!msg) { if (ret == 0) { logp_ipbc_uid(DINP, LOGL_NOTICE, ipbc, bfd->priv_nr >> 8); @@ -1025,7 +1006,7 @@ static int listen_fd_cb(struct bsc_fd *listen_bfd, unsigned int what) }
/* Request ID. FIXME: request LOCATION, HW/SW VErsion, Unit Name, Serno */ - ret = write(bfd->fd, id_req, sizeof(id_req)); + ret = ipaccess_send_id_req(bfd->fd);
return 0; } diff --git a/openbsc/src/libabis/input/ipaccess.c b/openbsc/src/libabis/input/ipaccess.c index ab1d41d..b652b90 100644 --- a/openbsc/src/libabis/input/ipaccess.c +++ b/openbsc/src/libabis/input/ipaccess.c @@ -62,18 +62,31 @@ static struct ia_e1_handle *e1h;
#define TS1_ALLOC_SIZE 900
-static const u_int8_t pong[] = { 0, 1, IPAC_PROTO_IPACCESS, IPAC_MSGT_PONG }; -static const u_int8_t id_ack[] = { 0, 1, IPAC_PROTO_IPACCESS, IPAC_MSGT_ID_ACK }; -static const u_int8_t id_req[] = { 0, 17, IPAC_PROTO_IPACCESS, IPAC_MSGT_ID_GET, - 0x01, IPAC_IDTAG_UNIT, - 0x01, IPAC_IDTAG_MACADDR, - 0x01, IPAC_IDTAG_LOCATION1, - 0x01, IPAC_IDTAG_LOCATION2, - 0x01, IPAC_IDTAG_EQUIPVERS, - 0x01, IPAC_IDTAG_SWVERSION, - 0x01, IPAC_IDTAG_UNITNAME, - 0x01, IPAC_IDTAG_SERNR, - }; +/* + * Common propietary IPA messages: + * - PONG: in reply to PING. + * - ID_REQUEST: first messages once OML has been established. + * - ID_ACK: in reply to ID_ACK. + */ +const u_int8_t ipa_pong_msg[] = { + 0, 1, IPAC_PROTO_IPACCESS, IPAC_MSGT_PONG +}; + +const u_int8_t ipa_id_ack_msg[] = { + 0, 1, IPAC_PROTO_IPACCESS, IPAC_MSGT_ID_ACK +}; + +const u_int8_t ipa_id_req_msg[] = { + 0, 17, IPAC_PROTO_IPACCESS, IPAC_MSGT_ID_GET, + 0x01, IPAC_IDTAG_UNIT, + 0x01, IPAC_IDTAG_MACADDR, + 0x01, IPAC_IDTAG_LOCATION1, + 0x01, IPAC_IDTAG_LOCATION2, + 0x01, IPAC_IDTAG_EQUIPVERS, + 0x01, IPAC_IDTAG_SWVERSION, + 0x01, IPAC_IDTAG_UNITNAME, + 0x01, IPAC_IDTAG_SERNR, +};
static const char *idtag_names[] = { [IPAC_IDTAG_SERNR] = "Serial_Number", @@ -179,15 +192,33 @@ static int parse_unitid(const char *str, u_int16_t *site_id, u_int16_t *bts_id, return 0; }
-/* send the id ack */ +static int ipaccess_send(int fd, const void *msg, size_t msglen) +{ + int ret; + + ret = write(fd, msg, msglen); + if (ret < 0) + return ret; + if (ret < msglen) { + DEBUGP(DINP, "ipaccess_send: short write\n"); + return -EIO; + } + return ret; +} + +int ipaccess_send_pong(int fd) +{ + return ipaccess_send(fd, ipa_pong_msg, sizeof(ipa_pong_msg)); +} + int ipaccess_send_id_ack(int fd) { - return write(fd, id_ack, sizeof(id_ack)); + return ipaccess_send(fd, ipa_id_ack_msg, sizeof(ipa_id_ack_msg)); }
int ipaccess_send_id_req(int fd) { - return write(fd, id_req, sizeof(id_req)); + return ipaccess_send(fd, ipa_id_req_msg, sizeof(ipa_id_req_msg)); }
/* base handling of the ip.access protocol */ @@ -199,7 +230,7 @@ int ipaccess_rcvmsg_base(struct msgb *msg,
switch (msg_type) { case IPAC_MSGT_PING: - ret = write(bfd->fd, pong, sizeof(pong)); + ret = ipaccess_send_pong(bfd->fd); break; case IPAC_MSGT_PONG: DEBUGP(DMI, "PONG!\n"); @@ -721,7 +752,7 @@ static int rsl_listen_fd_cb(struct bsc_fd *listen_bfd, unsigned int what) return ret; } /* Request ID. FIXME: request LOCATION, HW/SW VErsion, Unit Name, Serno */ - ret = write(bfd->fd, id_req, sizeof(id_req)); + ret = ipaccess_send_id_req(bfd->fd);
return 0; }
On 04/07/2011 02:27 PM, pablo@gnumonks.org wrote:
-/* send the id ack */ +static int ipaccess_send(int fd, const void *msg, size_t msglen) +{
- int ret;
- ret = write(fd, msg, msglen);
- if (ret < 0)
return ret;- if (ret < msglen) {
DEBUGP(DINP, "ipaccess_send: short write\n");return -EIO;- }
- return ret;
+}
Hi Pablo,
I know you are just moving this around but I think this qualifies as LOGP(LOGL_ERROR. Feel free to address this later.
From: Pablo Neira Ayuso pablo@gnumonks.org
Now this is used by ipaccess-proxy, remove redundant implemention in it. --- openbsc/include/openbsc/ipaccess.h | 1 + openbsc/src/ipaccess/ipaccess-proxy.c | 22 +--------------------- openbsc/src/libabis/input/ipaccess.c | 4 ++-- 3 files changed, 4 insertions(+), 23 deletions(-)
diff --git a/openbsc/include/openbsc/ipaccess.h b/openbsc/include/openbsc/ipaccess.h index b286f93..6053efd 100644 --- a/openbsc/include/openbsc/ipaccess.h +++ b/openbsc/include/openbsc/ipaccess.h @@ -74,6 +74,7 @@ int ipaccess_send_pong(int fd); int ipaccess_send_id_ack(int fd); int ipaccess_send_id_req(int fd);
+const char *ipaccess_idtag_name(int tag); int ipaccess_idtag_parse(struct tlv_parsed *dec, unsigned char *buf, int len);
int ipaccess_drop_oml(struct gsm_bts *bts); diff --git a/openbsc/src/ipaccess/ipaccess-proxy.c b/openbsc/src/ipaccess/ipaccess-proxy.c index b8b42c0..c6538ff 100644 --- a/openbsc/src/ipaccess/ipaccess-proxy.c +++ b/openbsc/src/ipaccess/ipaccess-proxy.c @@ -131,26 +131,6 @@ static int gprs_ns_cb(struct bsc_fd *bfd, unsigned int what);
#define PROXY_ALLOC_SIZE 1200
-static const char *idtag_names[] = { - [IPAC_IDTAG_SERNR] = "Serial_Number", - [IPAC_IDTAG_UNITNAME] = "Unit_Name", - [IPAC_IDTAG_LOCATION1] = "Location_1", - [IPAC_IDTAG_LOCATION2] = "Location_2", - [IPAC_IDTAG_EQUIPVERS] = "Equipment_Version", - [IPAC_IDTAG_SWVERSION] = "Software_Version", - [IPAC_IDTAG_IPADDR] = "IP_Address", - [IPAC_IDTAG_MACADDR] = "MAC_Address", - [IPAC_IDTAG_UNIT] = "Unit_ID", -}; - -static const char *ipac_idtag_name(int tag) -{ - if (tag >= ARRAY_SIZE(idtag_names)) - return "unknown"; - - return idtag_names[tag]; -} - static int ipac_idtag_parse(struct tlv_parsed *dec, unsigned char *buf, int len) { u_int8_t t_len; @@ -161,7 +141,7 @@ static int ipac_idtag_parse(struct tlv_parsed *dec, unsigned char *buf, int len) t_len = *cur++; t_tag = *cur++;
- DEBUGPC(DMI, "%s='%s' ", ipac_idtag_name(t_tag), cur); + DEBUGPC(DMI, "%s='%s' ", ipaccess_idtag_name(t_tag), cur);
dec->lv[t_tag].len = t_len; dec->lv[t_tag].val = cur; diff --git a/openbsc/src/libabis/input/ipaccess.c b/openbsc/src/libabis/input/ipaccess.c index b652b90..6b15e47 100644 --- a/openbsc/src/libabis/input/ipaccess.c +++ b/openbsc/src/libabis/input/ipaccess.c @@ -100,7 +100,7 @@ static const char *idtag_names[] = { [IPAC_IDTAG_UNIT] = "Unit_ID", };
-static const char *ipac_idtag_name(int tag) +const char *ipaccess_idtag_name(int tag) { if (tag >= ARRAY_SIZE(idtag_names)) return "unknown"; @@ -126,7 +126,7 @@ int ipaccess_idtag_parse(struct tlv_parsed *dec, unsigned char *buf, int len) return -1; }
- DEBUGPC(DMI, "%s='%s' ", ipac_idtag_name(t_tag), cur); + DEBUGPC(DMI, "%s='%s' ", ipaccess_idtag_name(t_tag), cur);
dec->lv[t_tag].len = t_len; dec->lv[t_tag].val = cur;
On 04/07/2011 02:27 PM, pablo@gnumonks.org wrote:
Hi Pablo,
thanks for the cleaning, the criticism is to the code as it was before.
t_tag = *cur++;
DEBUGPC(DMI, "%s='%s' ", ipac_idtag_name(t_tag), cur);
DEBUGPC(DMI, "%s='%s' ", ipaccess_idtag_name(t_tag), cur);
I think we still rely that the strings on the wire have a null termination? IIRC I once patched one version (of the many) to check for it and otherwise discard it. Would you mind taking a look at it?
-static const char *ipac_idtag_name(int tag) +const char *ipaccess_idtag_name(int tag) { if (tag >= ARRAY_SIZE(idtag_names)) return "unknown";
we probably want to make the parameter uint?
Hi Holger,
On 07/04/11 22:23, Holger Hans Peter Freyther wrote:
On 04/07/2011 02:27 PM, pablo@gnumonks.org wrote: thanks for the cleaning, the criticism is to the code as it was before.
t_tag = *cur++;
DEBUGPC(DMI, "%s='%s' ", ipac_idtag_name(t_tag), cur);
DEBUGPC(DMI, "%s='%s' ", ipaccess_idtag_name(t_tag), cur);I think we still rely that the strings on the wire have a null termination? IIRC I once patched one version (of the many) to check for it and otherwise discard it. Would you mind taking a look at it?
I'll have a look at all string handling.
-static const char *ipac_idtag_name(int tag) +const char *ipaccess_idtag_name(int tag) { if (tag >= ARRAY_SIZE(idtag_names)) return "unknown";
we probably want to make the parameter uint?
I have a patch for this here, I'll send it asap.
From: Pablo Neira Ayuso pablo@gnumonks.org
Use ipaccess_idtag_parse() available in libabis instead. --- openbsc/src/ipaccess/ipaccess-proxy.c | 24 ++---------------------- 1 files changed, 2 insertions(+), 22 deletions(-)
diff --git a/openbsc/src/ipaccess/ipaccess-proxy.c b/openbsc/src/ipaccess/ipaccess-proxy.c index c6538ff..d2bf599 100644 --- a/openbsc/src/ipaccess/ipaccess-proxy.c +++ b/openbsc/src/ipaccess/ipaccess-proxy.c @@ -131,26 +131,6 @@ static int gprs_ns_cb(struct bsc_fd *bfd, unsigned int what);
#define PROXY_ALLOC_SIZE 1200
-static int ipac_idtag_parse(struct tlv_parsed *dec, unsigned char *buf, int len) -{ - u_int8_t t_len; - u_int8_t t_tag; - u_int8_t *cur = buf; - - while (cur < buf + len) { - t_len = *cur++; - t_tag = *cur++; - - DEBUGPC(DMI, "%s='%s' ", ipaccess_idtag_name(t_tag), cur); - - dec->lv[t_tag].len = t_len; - dec->lv[t_tag].val = cur; - - cur += t_len; - } - return 0; -} - static int parse_unitid(const char *str, u_int16_t *site_id, u_int16_t *bts_id, u_int16_t *trx_id) { @@ -504,8 +484,8 @@ static int ipaccess_rcvmsg(struct ipa_proxy_conn *ipc, struct msgb *msg, case IPAC_MSGT_ID_RESP: DEBUGP(DMI, "ID_RESP "); /* parse tags, search for Unit ID */ - ipac_idtag_parse(&tlvp, (u_int8_t *)msg->l2h + 2, - msgb_l2len(msg)-2); + ipaccess_idtag_parse(&tlvp, (u_int8_t *)msg->l2h + 2, + msgb_l2len(msg)-2); DEBUGP(DMI, "\n");
if (!TLVP_PRESENT(&tlvp, IPAC_IDTAG_UNIT)) {
From: Pablo Neira Ayuso pablo@gnumonks.org
Now this is used by ipaccess-proxy, remove redundant implementation in it. --- openbsc/include/openbsc/ipaccess.h | 1 + openbsc/src/ipaccess/ipaccess-proxy.c | 41 +------------------------------- openbsc/src/libabis/input/ipaccess.c | 6 ++-- 3 files changed, 6 insertions(+), 42 deletions(-)
diff --git a/openbsc/include/openbsc/ipaccess.h b/openbsc/include/openbsc/ipaccess.h index 6053efd..135fce2 100644 --- a/openbsc/include/openbsc/ipaccess.h +++ b/openbsc/include/openbsc/ipaccess.h @@ -76,6 +76,7 @@ int ipaccess_send_id_req(int fd);
const char *ipaccess_idtag_name(int tag); int ipaccess_idtag_parse(struct tlv_parsed *dec, unsigned char *buf, int len); +int ipaccess_parse_unitid(const char *str, u_int16_t *site_id, u_int16_t *bts_id, u_int16_t *trx_id);
int ipaccess_drop_oml(struct gsm_bts *bts); int ipaccess_drop_rsl(struct gsm_bts_trx *trx); diff --git a/openbsc/src/ipaccess/ipaccess-proxy.c b/openbsc/src/ipaccess/ipaccess-proxy.c index d2bf599..b6dcad1 100644 --- a/openbsc/src/ipaccess/ipaccess-proxy.c +++ b/openbsc/src/ipaccess/ipaccess-proxy.c @@ -131,43 +131,6 @@ static int gprs_ns_cb(struct bsc_fd *bfd, unsigned int what);
#define PROXY_ALLOC_SIZE 1200
-static int parse_unitid(const char *str, u_int16_t *site_id, u_int16_t *bts_id, - u_int16_t *trx_id) -{ - unsigned long ul; - char *endptr; - const char *nptr; - - nptr = str; - ul = strtoul(nptr, &endptr, 10); - if (endptr <= nptr) - return -EINVAL; - if (site_id) - *site_id = ul & 0xffff; - - if (*endptr++ != '/') - return -EINVAL; - - nptr = endptr; - ul = strtoul(nptr, &endptr, 10); - if (endptr <= nptr) - return -EINVAL; - if (bts_id) - *bts_id = ul & 0xffff; - - if (*endptr++ != '/') - return -EINVAL; - - nptr = endptr; - ul = strtoul(nptr, &endptr, 10); - if (endptr <= nptr) - return -EINVAL; - if (trx_id) - *trx_id = ul & 0xffff; - - return 0; -} - static struct ipa_bts_conn *find_bts_by_unitid(struct ipa_proxy *ipp, u_int16_t site_id, u_int16_t bts_id) @@ -495,8 +458,8 @@ static int ipaccess_rcvmsg(struct ipa_proxy_conn *ipc, struct msgb *msg,
/* lookup BTS, create sign_link, ... */ site_id = bts_id = trx_id = 0; - parse_unitid((char *)TLVP_VAL(&tlvp, IPAC_IDTAG_UNIT), - &site_id, &bts_id, &trx_id); + ipaccess_parse_unitid((char *)TLVP_VAL(&tlvp, IPAC_IDTAG_UNIT), + &site_id, &bts_id, &trx_id); ipbc = find_bts_by_unitid(ipp, site_id, bts_id); if (!ipbc) { /* We have not found an ipbc (per-bts proxy instance) diff --git a/openbsc/src/libabis/input/ipaccess.c b/openbsc/src/libabis/input/ipaccess.c index 6b15e47..ecf7038 100644 --- a/openbsc/src/libabis/input/ipaccess.c +++ b/openbsc/src/libabis/input/ipaccess.c @@ -155,8 +155,8 @@ struct gsm_bts *find_bts_by_unitid(struct gsm_network *net, return NULL; }
-static int parse_unitid(const char *str, u_int16_t *site_id, u_int16_t *bts_id, - u_int16_t *trx_id) +int ipaccess_parse_unitid(const char *str, u_int16_t *site_id, + u_int16_t *bts_id, u_int16_t *trx_id) { unsigned long ul; char *endptr; @@ -274,7 +274,7 @@ static int ipaccess_rcvmsg(struct e1inp_line *line, struct msgb *msg, /* lookup BTS, create sign_link, ... */ unitid = (char *) TLVP_VAL(&tlvp, IPAC_IDTAG_UNIT); unitid[len - 1] = '\0'; - parse_unitid(unitid, &site_id, &bts_id, &trx_id); + ipaccess_parse_unitid(unitid, &site_id, &bts_id, &trx_id); bts = find_bts_by_unitid(e1h->gsmnet, site_id, bts_id); if (!bts) { LOGP(DINP, LOGL_ERROR, "Unable to find BTS configuration for "
From: Pablo Neira Ayuso pablo@gnumonks.org
This patch removes the hardcoded IP addresses for options `-l' and `-b'. --- openbsc/src/ipaccess/ipaccess-proxy.c | 23 +++++++++++++++++++---- 1 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/openbsc/src/ipaccess/ipaccess-proxy.c b/openbsc/src/ipaccess/ipaccess-proxy.c index b6dcad1..d420b63 100644 --- a/openbsc/src/ipaccess/ipaccess-proxy.c +++ b/openbsc/src/ipaccess/ipaccess-proxy.c @@ -1120,11 +1120,19 @@ static void print_help()
static void print_usage() { - printf("Usage: ipaccess-proxy\n"); + printf("Usage: ipaccess-proxy [options]\n"); }
+enum { + IPA_PROXY_OPT_LISTEN_NONE = 0, + IPA_PROXY_OPT_LISTEN_IP = (1 << 0), + IPA_PROXY_OPT_BSC_IP = (1 << 1), +}; + static void handle_options(int argc, char** argv) { + int options_mask = 0; + while (1) { int option_index = 0, c; static struct option long_options[] = { @@ -1150,9 +1158,11 @@ static void handle_options(int argc, char** argv) exit(0); case 'l': listen_ipaddr = optarg; + options_mask |= IPA_PROXY_OPT_LISTEN_IP; break; case 'b': bsc_ipaddr = optarg; + options_mask |= IPA_PROXY_OPT_BSC_IP; break; case 'g': gprs_ns_ipaddr = optarg; @@ -1171,15 +1181,20 @@ static void handle_options(int argc, char** argv) break; } } + if (!((options_mask & (IPA_PROXY_OPT_LISTEN_IP | IPA_PROXY_OPT_BSC_IP)) + == (IPA_PROXY_OPT_LISTEN_IP | IPA_PROXY_OPT_BSC_IP))) { + printf("ERROR: You have to specify `--listen' and `--bsc' " + "options at least.\n"); + print_usage(); + print_help(); + exit(EXIT_FAILURE); + } }
int main(int argc, char **argv) { int rc;
- listen_ipaddr = "192.168.100.11"; - bsc_ipaddr = "192.168.100.239"; - tall_bsc_ctx = talloc_named_const(NULL, 1, "ipaccess-proxy");
log_init(&log_info);
On 07/04/11 14:42, Holger Hans Peter Freyther wrote:
On 04/07/2011 02:27 PM, pablo@gnumonks.org wrote:
}
- if (!((options_mask & (IPA_PROXY_OPT_LISTEN_IP | IPA_PROXY_OPT_BSC_IP))
== (IPA_PROXY_OPT_LISTEN_IP | IPA_PROXY_OPT_BSC_IP))) {why not != ?
Yes, != looks more readable. Although the line above works fine.
I have forced a push with this change that you propose to the pablo/cleanups branch.
New patch attached.
Pablo,
you're my hero, thanks for cleaning up the mess I left in the proxy.
I will delay committing your patddches as I know zecke was also working on some ipaccess related bugfix right now and I don't want to cause unneccessary conflicts.
Holger: Can you simply merge pablos patches before/after your fix, whatever you think is appropriate.
Regards from Amsterdam, Harald
On 04/08/2011 12:44 AM, Harald Welte wrote:
Pablo,
you're my hero, thanks for cleaning up the mess I left in the proxy.
I will delay committing your patddches as I know zecke was also working on some ipaccess related bugfix right now and I don't want to cause unneccessary conflicts.
Holger: Can you simply merge pablos patches before/after your fix, whatever you think is appropriate.
I am trying to do the bisect in some minutes but I don't think Pablo's patches will cause any conflict. So feel free to merge them, I will be out of office tomorrow but can do the merge in the evening.
On 04/07/2011 11:19 PM, Holger Hans Peter Freyther wrote:
I am trying to do the bisect in some minutes but I don't think Pablo's patches will cause any conflict. So feel free to merge them, I will be out of office tomorrow but can do the merge in the evening.
Okay, i figured it is easier to just merge it now and then do my changes instead of causing conflicts myself.
Pablo, if you have time and harald agrees it would be nice if you could make the ipaccess tag parsing more robust. thanks.
holger
On Thu, Apr 07, 2011 at 11:31:02PM +0200, Holger Hans Peter Freyther wrote:
Pablo, if you have time and harald agrees it would be nice if you could make the ipaccess tag parsing more robust. thanks.
ACK.
-
Harald Welte -
Holger Hans Peter Freyther -
Pablo Neira Ayuso -
pablo＠gnumonks.org