[PATCH] libabis: Enable TCP keepalive by default on all BTS connections
From: Daniel Willmann <daniel@totalueberwachung.de> This way we can find out fast if the connection is broken. Ticket: OW#1060 --- src/input/ipaccess.c | 35 ++++++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/src/input/ipaccess.c b/src/input/ipaccess.c index 4a56569..371b21c 100644 --- a/src/input/ipaccess.c +++ b/src/input/ipaccess.c @@ -28,6 +28,7 @@ #include <stdlib.h> #include <stdbool.h> #include <errno.h> +#include <netinet/tcp.h> #include <string.h> #include <time.h> #include <sys/fcntl.h> @@ -50,6 +51,10 @@ static void *tall_ipa_ctx; #define TS1_ALLOC_SIZE 900 +#define DEFAULT_TCP_KEEPALIVE_IDLE_TIMEOUT 30 +#define DEFAULT_TCP_KEEPALIVE_INTERVAL 3 +#define DEFAULT_TCP_KEEPALIVE_RETRY_COUNT 10 + /* * Common propietary IPA messages: * - PONG: in reply to PING. @@ -605,7 +610,7 @@ static int ipaccess_bsc_oml_cb(struct ipa_server_link *link, int fd) { int ret; int idx = 0; - int i; + int i, val; struct e1inp_line *line; struct e1inp_ts *e1i_ts; struct osmo_fd *bfd; @@ -638,6 +643,34 @@ static int ipaccess_bsc_oml_cb(struct ipa_server_link *link, int fd) goto err_line; } + /* Enable TCP keepalive to find out if the connection is gone */ + val = 1; + ret = setsockopt(bfd->fd, SOL_SOCKET, SO_KEEPALIVE, &val, sizeof(val)); + if (ret < 0) + LOGP(DLINP, LOGL_NOTICE, "Failed to set keepalive: %s\n", + strerror(errno)); + else + LOGP(DLINP, LOGL_NOTICE, "Keepalive is set: %i\n", ret); + +#if defined(TCP_KEEPIDLE) && defined(TCP_KEEPINTVL) && defined(TCP_KEEPCNT) + /* The following options are not portable! */ + val = DEFAULT_TCP_KEEPALIVE_IDLE_TIMEOUT; + ret = setsockopt(bfd->fd, IPPROTO_TCP, TCP_KEEPIDLE, &val, sizeof(val)); + if (ret < 0) + LOGP(DLINP, LOGL_NOTICE, "Failed to set keepalive idle time: %s\n", + strerror(errno)); + val = DEFAULT_TCP_KEEPALIVE_INTERVAL; + ret = setsockopt(bfd->fd, IPPROTO_TCP, TCP_KEEPINTVL, &val, sizeof(val)); + if (ret < 0) + LOGP(DLINP, LOGL_NOTICE, "Failed to set keepalive interval: %s\n", + strerror(errno)); + val = DEFAULT_TCP_KEEPALIVE_RETRY_COUNT; + ret = setsockopt(bfd->fd, IPPROTO_TCP, TCP_KEEPCNT, &val, sizeof(val)); + if (ret < 0) + LOGP(DLINP, LOGL_NOTICE, "Failed to set keepalive count: %s\n", + strerror(errno)); +#endif + /* Request ID. FIXME: request LOCATION, HW/SW VErsion, Unit Name, Serno */ ret = ipaccess_send_id_req(bfd->fd); if (ret < 0) { -- 1.7.9.5
From: Daniel Willmann <daniel@totalueberwachung.de> This way we can find out fast if the connection is broken. Ticket: OW#1060 --- src/input/ipaccess.c | 35 ++++++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/src/input/ipaccess.c b/src/input/ipaccess.c index 9722b2f..8f9865e 100644 --- a/src/input/ipaccess.c +++ b/src/input/ipaccess.c @@ -28,6 +28,7 @@ #include <stdlib.h> #include <stdbool.h> #include <errno.h> +#include <netinet/tcp.h> #include <string.h> #include <time.h> #include <sys/fcntl.h> @@ -50,6 +51,10 @@ static void *tall_ipa_ctx; #define TS1_ALLOC_SIZE 900 +#define DEFAULT_TCP_KEEPALIVE_IDLE_TIMEOUT 30 +#define DEFAULT_TCP_KEEPALIVE_INTERVAL 3 +#define DEFAULT_TCP_KEEPALIVE_RETRY_COUNT 10 + /* * Common propietary IPA messages: * - PONG: in reply to PING. @@ -610,7 +615,7 @@ static int ipaccess_bsc_oml_cb(struct ipa_server_link *link, int fd) { int ret; int idx = 0; - int i; + int i, val; struct e1inp_line *line; struct e1inp_ts *e1i_ts; struct osmo_fd *bfd; @@ -643,6 +648,34 @@ static int ipaccess_bsc_oml_cb(struct ipa_server_link *link, int fd) goto err_line; } + /* Enable TCP keepalive to find out if the connection is gone */ + val = 1; + ret = setsockopt(bfd->fd, SOL_SOCKET, SO_KEEPALIVE, &val, sizeof(val)); + if (ret < 0) + LOGP(DLINP, LOGL_NOTICE, "Failed to set keepalive: %s\n", + strerror(errno)); + else + LOGP(DLINP, LOGL_NOTICE, "Keepalive is set: %i\n", ret); + +#if defined(TCP_KEEPIDLE) && defined(TCP_KEEPINTVL) && defined(TCP_KEEPCNT) + /* The following options are not portable! */ + val = DEFAULT_TCP_KEEPALIVE_IDLE_TIMEOUT; + ret = setsockopt(bfd->fd, IPPROTO_TCP, TCP_KEEPIDLE, &val, sizeof(val)); + if (ret < 0) + LOGP(DLINP, LOGL_NOTICE, "Failed to set keepalive idle time: %s\n", + strerror(errno)); + val = DEFAULT_TCP_KEEPALIVE_INTERVAL; + ret = setsockopt(bfd->fd, IPPROTO_TCP, TCP_KEEPINTVL, &val, sizeof(val)); + if (ret < 0) + LOGP(DLINP, LOGL_NOTICE, "Failed to set keepalive interval: %s\n", + strerror(errno)); + val = DEFAULT_TCP_KEEPALIVE_RETRY_COUNT; + ret = setsockopt(bfd->fd, IPPROTO_TCP, TCP_KEEPCNT, &val, sizeof(val)); + if (ret < 0) + LOGP(DLINP, LOGL_NOTICE, "Failed to set keepalive count: %s\n", + strerror(errno)); +#endif + /* Request ID. FIXME: request LOCATION, HW/SW VErsion, Unit Name, Serno */ ret = ipaccess_send_id_req(bfd->fd); if (ret < 0) { -- 1.7.9.5
Keep alive has only been used for OML so far. This patch refactors the socket configuration into an own function and uses it for RSL, too. Ticket: OW#1060 Sponsored-by: On-Waves ehf --- src/input/ipaccess.c | 73 +++++++++++++++++++++++++++++++------------------- 1 file changed, 45 insertions(+), 28 deletions(-) diff --git a/src/input/ipaccess.c b/src/input/ipaccess.c index 8f9865e..6b9d93e 100644 --- a/src/input/ipaccess.c +++ b/src/input/ipaccess.c @@ -610,12 +610,54 @@ struct e1inp_driver ipaccess_driver = { .default_delay = 0, }; +static void update_fd_settings(struct e1inp_line *line, int fd) +{ + int ret; + int val; + + if (DEFAULT_TCP_KEEPALIVE_RETRY_COUNT) { + /* Enable TCP keepalive to find out if the connection is gone */ + val = 1; + ret = setsockopt(fd, SOL_SOCKET, SO_KEEPALIVE, &val, sizeof(val)); + if (ret < 0) + LOGP(DLINP, LOGL_NOTICE, "Failed to set keepalive: %s\n", + strerror(errno)); + else + LOGP(DLINP, LOGL_NOTICE, "Keepalive is set: %i\n", ret); + +#if defined(TCP_KEEPIDLE) && defined(TCP_KEEPINTVL) && defined(TCP_KEEPCNT) + /* The following options are not portable! */ + val = DEFAULT_TCP_KEEPALIVE_IDLE_TIMEOUT; + ret = setsockopt(fd, IPPROTO_TCP, TCP_KEEPIDLE, + &val, sizeof(val)); + if (ret < 0) + LOGP(DLINP, LOGL_NOTICE, + "Failed to set keepalive idle time: %s\n", + strerror(errno)); + val = DEFAULT_TCP_KEEPALIVE_INTERVAL; + ret = setsockopt(fd, IPPROTO_TCP, TCP_KEEPINTVL, + &val, sizeof(val)); + if (ret < 0) + LOGP(DLINP, LOGL_NOTICE, + "Failed to set keepalive interval: %s\n", + strerror(errno)); + val = DEFAULT_TCP_KEEPALIVE_RETRY_COUNT; + ret = setsockopt(fd, IPPROTO_TCP, TCP_KEEPCNT, + &val, sizeof(val)); + if (ret < 0) + LOGP(DLINP, LOGL_NOTICE, + "Failed to set keepalive count: %s\n", + strerror(errno)); + } +#endif +} + /* callback of the OML listening filedescriptor */ static int ipaccess_bsc_oml_cb(struct ipa_server_link *link, int fd) { int ret; int idx = 0; - int i, val; + int i; struct e1inp_line *line; struct e1inp_ts *e1i_ts; struct osmo_fd *bfd; @@ -648,33 +690,7 @@ static int ipaccess_bsc_oml_cb(struct ipa_server_link *link, int fd) goto err_line; } - /* Enable TCP keepalive to find out if the connection is gone */ - val = 1; - ret = setsockopt(bfd->fd, SOL_SOCKET, SO_KEEPALIVE, &val, sizeof(val)); - if (ret < 0) - LOGP(DLINP, LOGL_NOTICE, "Failed to set keepalive: %s\n", - strerror(errno)); - else - LOGP(DLINP, LOGL_NOTICE, "Keepalive is set: %i\n", ret); - -#if defined(TCP_KEEPIDLE) && defined(TCP_KEEPINTVL) && defined(TCP_KEEPCNT) - /* The following options are not portable! */ - val = DEFAULT_TCP_KEEPALIVE_IDLE_TIMEOUT; - ret = setsockopt(bfd->fd, IPPROTO_TCP, TCP_KEEPIDLE, &val, sizeof(val)); - if (ret < 0) - LOGP(DLINP, LOGL_NOTICE, "Failed to set keepalive idle time: %s\n", - strerror(errno)); - val = DEFAULT_TCP_KEEPALIVE_INTERVAL; - ret = setsockopt(bfd->fd, IPPROTO_TCP, TCP_KEEPINTVL, &val, sizeof(val)); - if (ret < 0) - LOGP(DLINP, LOGL_NOTICE, "Failed to set keepalive interval: %s\n", - strerror(errno)); - val = DEFAULT_TCP_KEEPALIVE_RETRY_COUNT; - ret = setsockopt(bfd->fd, IPPROTO_TCP, TCP_KEEPCNT, &val, sizeof(val)); - if (ret < 0) - LOGP(DLINP, LOGL_NOTICE, "Failed to set keepalive count: %s\n", - strerror(errno)); -#endif + update_fd_settings(line, bfd->fd); /* Request ID. FIXME: request LOCATION, HW/SW VErsion, Unit Name, Serno */ ret = ipaccess_send_id_req(bfd->fd); @@ -736,6 +752,7 @@ static int ipaccess_bsc_rsl_cb(struct ipa_server_link *link, int fd) strerror(errno)); goto err_socket; } + update_fd_settings(line, bfd->fd); return ret; err_socket: -- 1.7.9.5
On Thu, Jan 09, 2014 at 02:30:56PM +0100, Jacob Erlbeck wrote:
Keep alive has only been used for OML so far.
This patch refactors the socket configuration into an own function and uses it for RSL, too.
Good Morning, with the modified 1/4 patch this patch does not apply anymore. Can you please publish your branch so I can merge it today. thanks holger
On Mon, Jan 20, 2014 at 08:05:25AM +0100, Holger Hans Peter Freyther wrote:
On Thu, Jan 09, 2014 at 02:30:56PM +0100, Jacob Erlbeck wrote:
Keep alive has only been used for OML so far.
This patch refactors the socket configuration into an own function and uses it for RSL, too.
Good Morning,
with the modified 1/4 patch this patch does not apply anymore. Can you please publish your branch so I can merge it today.
Nevermind. You updated 3/4 so I got the order of the patches wrong.
This patch adds a generic keep alive configuration layer that mainly consists of additional fields in e1_input structs and VTY commands and extensions. Ticket: OW#1060 Sponsored-by: On-Waves ehf --- include/osmocom/abis/e1_input.h | 7 ++++ src/e1_input_vty.c | 69 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+) diff --git a/include/osmocom/abis/e1_input.h b/include/osmocom/abis/e1_input.h index 0abf0b8..70df565 100644 --- a/include/osmocom/abis/e1_input.h +++ b/include/osmocom/abis/e1_input.h @@ -12,6 +12,7 @@ #include <osmocom/abis/lapd.h> #define NUM_E1_TS 32 +#define E1INP_USE_DEFAULT (-1) enum e1inp_sign_type { E1INP_SIGN_NONE, @@ -134,6 +135,7 @@ struct e1inp_driver { void (*close)(struct e1inp_sign_link *link); void (*vty_show)(struct vty *vty, struct e1inp_line *line); int default_delay; + int has_keepalive; }; struct e1inp_line_ops { @@ -163,6 +165,11 @@ struct e1inp_line { unsigned int port_nr; struct rate_ctr_group *rate_ctr; + /* keepalive configuration */ + int keepalive_idle_timeout; /* 0: disable, secs, or E1INP_USE_DEFAULT */ + int keepalive_num_probes; /* 0: disable, num, or E1INP_USE_DEFAULT */ + int keepalive_probe_interval; /* 0: disable, secs, or E1INP_USE_DEFAULT */ + /* array of timestlots */ struct e1inp_ts ts[NUM_E1_TS]; unsigned int num_ts; diff --git a/src/e1_input_vty.c b/src/e1_input_vty.c index d99c853..0b4adb2 100644 --- a/src/e1_input_vty.c +++ b/src/e1_input_vty.c @@ -88,6 +88,56 @@ DEFUN(cfg_e1line_port, cfg_e1_line_port_cmd, return CMD_SUCCESS; } +#define KEEPALIVE_HELP "Enable keep-alive probing\n" +static int set_keepalive_params(struct vty *vty, int e1_nr, + int idle, int num_probes, int probe_interval) +{ + struct e1inp_line *line = e1inp_line_find(e1_nr); + + if (!line) { + vty_out(vty, "%% Line %d doesn't exist%s", e1_nr, VTY_NEWLINE); + return CMD_WARNING; + } + if (!line->driver->has_keepalive && num_probes != 0) { + vty_out(vty, "%% Driver '%s' does not support keep alive%s", + line->driver->name, VTY_NEWLINE); + return CMD_WARNING; + } + + line->keepalive_idle_timeout = idle; + line->keepalive_num_probes = num_probes; + line->keepalive_probe_interval = probe_interval; + + return CMD_SUCCESS; +} + +DEFUN(cfg_e1line_keepalive, cfg_e1_line_keepalive_cmd, + "e1_line <0-255> keepalive", + E1_LINE_HELP KEEPALIVE_HELP) +{ + return set_keepalive_params(vty, atoi(argv[0]), + E1INP_USE_DEFAULT, E1INP_USE_DEFAULT, + E1INP_USE_DEFAULT); +} + +DEFUN(cfg_e1line_keepalive_params, cfg_e1_line_keepalive_params_cmd, + "e1_line <0-255> keepalive <1-300> <1-20> <1-300>", + E1_LINE_HELP KEEPALIVE_HELP + "Idle interval in seconds before probes are sent\n" + "Number of probes to sent\n" + "Delay between probe packets in seconds\n") +{ + return set_keepalive_params(vty, atoi(argv[0]), + atoi(argv[1]), atoi(argv[2]), atoi(argv[3])); +} + +DEFUN(cfg_e1line_no_keepalive, cfg_e1_line_no_keepalive_cmd, + "no e1_line <0-255> keepalive", + NO_STR E1_LINE_HELP KEEPALIVE_HELP) +{ + return set_keepalive_params(vty, atoi(argv[0]), 0, 0, 0); +} + DEFUN(cfg_e1line_name, cfg_e1_line_name_cmd, "e1_line <0-255> name .LINE", E1_LINE_HELP "Set name for this line\n" "Human readable name\n") @@ -135,6 +185,22 @@ static int e1inp_config_write(struct vty *vty) if (line->name) vty_out(vty, " e1_line %u name %s%s", line->num, line->name, VTY_NEWLINE); + if (!line->keepalive_num_probes) + vty_out(vty, " no e1_line %u keepalive%s", line->num, + VTY_NEWLINE); + else if (line->keepalive_idle_timeout == E1INP_USE_DEFAULT && + line->keepalive_num_probes == E1INP_USE_DEFAULT && + line->keepalive_probe_interval == E1INP_USE_DEFAULT) + vty_out(vty, " e1_line %u keepalive%s", line->num, + VTY_NEWLINE); + else + vty_out(vty, " e1_line %u keepalive %d %d %d%s", + line->num, + line->keepalive_idle_timeout, + line->keepalive_num_probes, + line->keepalive_probe_interval, + VTY_NEWLINE); + } return CMD_SUCCESS; } @@ -281,6 +347,9 @@ int e1inp_vty_init(void) install_element(L_E1INP_NODE, &cfg_e1_line_driver_cmd); install_element(L_E1INP_NODE, &cfg_e1_line_port_cmd); install_element(L_E1INP_NODE, &cfg_e1_line_name_cmd); + install_element(L_E1INP_NODE, &cfg_e1_line_keepalive_cmd); + install_element(L_E1INP_NODE, &cfg_e1_line_keepalive_params_cmd); + install_element(L_E1INP_NODE, &cfg_e1_line_no_keepalive_cmd); install_element_ve(&show_e1drv_cmd); install_element_ve(&show_e1line_cmd); -- 1.7.9.5
On Thu, Jan 09, 2014 at 02:30:57PM +0100, Jacob Erlbeck wrote:
This patch adds a generic keep alive configuration layer that mainly consists of additional fields in e1_input structs and VTY commands and extensions.
enum e1inp_sign_type { E1INP_SIGN_NONE, @@ -134,6 +135,7 @@ struct e1inp_driver { void (*close)(struct e1inp_sign_link *link); void (*vty_show)(struct vty *vty, struct e1inp_line *line); int default_delay; + int has_keepalive; };
This changes ABI and we should establish a generic way to error when making a new release and forgetting to update the LIBVERSION. I can think of doing: * Modify configure.ac to check the version number and error if a LIBVERSION change is pending. * Use #error in the header file when the XXX version is changed?
On Mon, Jan 13, 2014 at 10:27:07AM +0100, Holger Hans Peter Freyther wrote: Hi all,
This changes ABI and we should establish a generic way to error when making a new release and forgetting to update the LIBVERSION. I can think of doing:
* Modify configure.ac to check the version number and error if a LIBVERSION change is pending. * Use #error in the header file when the XXX version is changed?
the current proposal is to introduce a process that will create a new file in the root of a project where we document the ABI changes. The content of the file will look like this: #library what description / commit summary line
On 16.01.2014 20:53, Holger Hans Peter Freyther wrote:
On Mon, Jan 13, 2014 at 10:27:07AM +0100, Holger Hans Peter Freyther wrote:
Hi all,
the current proposal is to introduce a process that will create a new file in the root of a project where we document the ABI changes. The content of the file will look like this:
#library what description / commit summary line
More precisely, the idea was to have a TODO-RELEASE file in the project's top directory containing a single line per 'what' and commit which is part of the commit itself. When this file is empty (except for empty lines and comment lines starting with a '#'), a release can be finished. 'what' is one of the following strings (more to come): 'abi-change' the ABI (structs, function parameters) has changed e.g. due to modifications or removals. When there is at least one such line, the LIBVERSION 'age' field must be reset and the 'current' field must be incremented when doing a release. All lines marked with 'abi-change' or 'abi-add' can be removed then. 'abi-add' a function or struct has been added but the ABI has not changed otherwise (so it remains compatible). When there is at least one such line (and no 'abi-change' line'), the LIBVERSION 'age' and 'current' fields must be incremented when doing a release. All such lines can be removed then. The 'abi-add' variant is probably less important but I've included it nevertheless for completeness. Jacob
On Thu, Jan 09, 2014 at 02:30:57PM +0100, Jacob Erlbeck wrote:
+ /* keepalive configuration */ + int keepalive_idle_timeout; /* 0: disable, secs, or E1INP_USE_DEFAULT */ + int keepalive_num_probes; /* 0: disable, num, or E1INP_USE_DEFAULT */ + int keepalive_probe_interval; /* 0: disable, secs, or E1INP_USE_DEFAULT */
"disable" is not fully accurate. keepalive_num_probes is used in the ipaccess.c and the normal code to check if the keepalive handling is enabled? Do you want to update the comments?
This patch adds a generic keep alive configuration layer that mainly consists of additional fields in e1_input structs and VTY commands and extensions. Ticket: OW#1060 Sponsored-by: On-Waves ehf --- TODO-RELEASE | 2 ++ include/osmocom/abis/e1_input.h | 7 ++++ src/e1_input_vty.c | 69 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 78 insertions(+) create mode 100644 TODO-RELEASE diff --git a/TODO-RELEASE b/TODO-RELEASE new file mode 100644 index 0000000..98b9a64 --- /dev/null +++ b/TODO-RELEASE @@ -0,0 +1,2 @@ +#library what description / commit summary line +libosmoabis abi-change input: Make keep alive configurable (generic) diff --git a/include/osmocom/abis/e1_input.h b/include/osmocom/abis/e1_input.h index 0abf0b8..70df565 100644 --- a/include/osmocom/abis/e1_input.h +++ b/include/osmocom/abis/e1_input.h @@ -12,6 +12,7 @@ #include <osmocom/abis/lapd.h> #define NUM_E1_TS 32 +#define E1INP_USE_DEFAULT (-1) enum e1inp_sign_type { E1INP_SIGN_NONE, @@ -134,6 +135,7 @@ struct e1inp_driver { void (*close)(struct e1inp_sign_link *link); void (*vty_show)(struct vty *vty, struct e1inp_line *line); int default_delay; + int has_keepalive; }; struct e1inp_line_ops { @@ -163,6 +165,11 @@ struct e1inp_line { unsigned int port_nr; struct rate_ctr_group *rate_ctr; + /* keepalive configuration */ + int keepalive_idle_timeout; /* 0: disable, secs, or E1INP_USE_DEFAULT */ + int keepalive_num_probes; /* 0: disable, num, or E1INP_USE_DEFAULT */ + int keepalive_probe_interval; /* 0: disable, secs, or E1INP_USE_DEFAULT */ + /* array of timestlots */ struct e1inp_ts ts[NUM_E1_TS]; unsigned int num_ts; diff --git a/src/e1_input_vty.c b/src/e1_input_vty.c index d99c853..0b4adb2 100644 --- a/src/e1_input_vty.c +++ b/src/e1_input_vty.c @@ -88,6 +88,56 @@ DEFUN(cfg_e1line_port, cfg_e1_line_port_cmd, return CMD_SUCCESS; } +#define KEEPALIVE_HELP "Enable keep-alive probing\n" +static int set_keepalive_params(struct vty *vty, int e1_nr, + int idle, int num_probes, int probe_interval) +{ + struct e1inp_line *line = e1inp_line_find(e1_nr); + + if (!line) { + vty_out(vty, "%% Line %d doesn't exist%s", e1_nr, VTY_NEWLINE); + return CMD_WARNING; + } + if (!line->driver->has_keepalive && num_probes != 0) { + vty_out(vty, "%% Driver '%s' does not support keep alive%s", + line->driver->name, VTY_NEWLINE); + return CMD_WARNING; + } + + line->keepalive_idle_timeout = idle; + line->keepalive_num_probes = num_probes; + line->keepalive_probe_interval = probe_interval; + + return CMD_SUCCESS; +} + +DEFUN(cfg_e1line_keepalive, cfg_e1_line_keepalive_cmd, + "e1_line <0-255> keepalive", + E1_LINE_HELP KEEPALIVE_HELP) +{ + return set_keepalive_params(vty, atoi(argv[0]), + E1INP_USE_DEFAULT, E1INP_USE_DEFAULT, + E1INP_USE_DEFAULT); +} + +DEFUN(cfg_e1line_keepalive_params, cfg_e1_line_keepalive_params_cmd, + "e1_line <0-255> keepalive <1-300> <1-20> <1-300>", + E1_LINE_HELP KEEPALIVE_HELP + "Idle interval in seconds before probes are sent\n" + "Number of probes to sent\n" + "Delay between probe packets in seconds\n") +{ + return set_keepalive_params(vty, atoi(argv[0]), + atoi(argv[1]), atoi(argv[2]), atoi(argv[3])); +} + +DEFUN(cfg_e1line_no_keepalive, cfg_e1_line_no_keepalive_cmd, + "no e1_line <0-255> keepalive", + NO_STR E1_LINE_HELP KEEPALIVE_HELP) +{ + return set_keepalive_params(vty, atoi(argv[0]), 0, 0, 0); +} + DEFUN(cfg_e1line_name, cfg_e1_line_name_cmd, "e1_line <0-255> name .LINE", E1_LINE_HELP "Set name for this line\n" "Human readable name\n") @@ -135,6 +185,22 @@ static int e1inp_config_write(struct vty *vty) if (line->name) vty_out(vty, " e1_line %u name %s%s", line->num, line->name, VTY_NEWLINE); + if (!line->keepalive_num_probes) + vty_out(vty, " no e1_line %u keepalive%s", line->num, + VTY_NEWLINE); + else if (line->keepalive_idle_timeout == E1INP_USE_DEFAULT && + line->keepalive_num_probes == E1INP_USE_DEFAULT && + line->keepalive_probe_interval == E1INP_USE_DEFAULT) + vty_out(vty, " e1_line %u keepalive%s", line->num, + VTY_NEWLINE); + else + vty_out(vty, " e1_line %u keepalive %d %d %d%s", + line->num, + line->keepalive_idle_timeout, + line->keepalive_num_probes, + line->keepalive_probe_interval, + VTY_NEWLINE); + } return CMD_SUCCESS; } @@ -281,6 +347,9 @@ int e1inp_vty_init(void) install_element(L_E1INP_NODE, &cfg_e1_line_driver_cmd); install_element(L_E1INP_NODE, &cfg_e1_line_port_cmd); install_element(L_E1INP_NODE, &cfg_e1_line_name_cmd); + install_element(L_E1INP_NODE, &cfg_e1_line_keepalive_cmd); + install_element(L_E1INP_NODE, &cfg_e1_line_keepalive_params_cmd); + install_element(L_E1INP_NODE, &cfg_e1_line_no_keepalive_cmd); install_element_ve(&show_e1drv_cmd); install_element_ve(&show_e1line_cmd); -- 1.7.9.5
This patch adds a generic keep alive configuration layer that mainly consists of additional fields in e1_input structs and VTY commands and extensions. Ticket: OW#1060 Sponsored-by: On-Waves ehf --- TODO-RELEASE | 2 ++ include/osmocom/abis/e1_input.h | 7 ++++ src/e1_input_vty.c | 69 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 78 insertions(+) create mode 100644 TODO-RELEASE diff --git a/TODO-RELEASE b/TODO-RELEASE new file mode 100644 index 0000000..98b9a64 --- /dev/null +++ b/TODO-RELEASE @@ -0,0 +1,2 @@ +#library what description / commit summary line +libosmoabis abi-change input: Make keep alive configurable (generic) diff --git a/include/osmocom/abis/e1_input.h b/include/osmocom/abis/e1_input.h index 0abf0b8..9b77893 100644 --- a/include/osmocom/abis/e1_input.h +++ b/include/osmocom/abis/e1_input.h @@ -12,6 +12,7 @@ #include <osmocom/abis/lapd.h> #define NUM_E1_TS 32 +#define E1INP_USE_DEFAULT (-1) enum e1inp_sign_type { E1INP_SIGN_NONE, @@ -134,6 +135,7 @@ struct e1inp_driver { void (*close)(struct e1inp_sign_link *link); void (*vty_show)(struct vty *vty, struct e1inp_line *line); int default_delay; + int has_keepalive; }; struct e1inp_line_ops { @@ -163,6 +165,11 @@ struct e1inp_line { unsigned int port_nr; struct rate_ctr_group *rate_ctr; + /* keepalive configuration */ + int keepalive_num_probes; /* 0: disable, num, or E1INP_USE_DEFAULT */ + int keepalive_idle_timeout; /* secs, or E1INP_USE_DEFAULT */ + int keepalive_probe_interval; /* secs or E1INP_USE_DEFAULT */ + /* array of timestlots */ struct e1inp_ts ts[NUM_E1_TS]; unsigned int num_ts; diff --git a/src/e1_input_vty.c b/src/e1_input_vty.c index d99c853..0b4adb2 100644 --- a/src/e1_input_vty.c +++ b/src/e1_input_vty.c @@ -88,6 +88,56 @@ DEFUN(cfg_e1line_port, cfg_e1_line_port_cmd, return CMD_SUCCESS; } +#define KEEPALIVE_HELP "Enable keep-alive probing\n" +static int set_keepalive_params(struct vty *vty, int e1_nr, + int idle, int num_probes, int probe_interval) +{ + struct e1inp_line *line = e1inp_line_find(e1_nr); + + if (!line) { + vty_out(vty, "%% Line %d doesn't exist%s", e1_nr, VTY_NEWLINE); + return CMD_WARNING; + } + if (!line->driver->has_keepalive && num_probes != 0) { + vty_out(vty, "%% Driver '%s' does not support keep alive%s", + line->driver->name, VTY_NEWLINE); + return CMD_WARNING; + } + + line->keepalive_idle_timeout = idle; + line->keepalive_num_probes = num_probes; + line->keepalive_probe_interval = probe_interval; + + return CMD_SUCCESS; +} + +DEFUN(cfg_e1line_keepalive, cfg_e1_line_keepalive_cmd, + "e1_line <0-255> keepalive", + E1_LINE_HELP KEEPALIVE_HELP) +{ + return set_keepalive_params(vty, atoi(argv[0]), + E1INP_USE_DEFAULT, E1INP_USE_DEFAULT, + E1INP_USE_DEFAULT); +} + +DEFUN(cfg_e1line_keepalive_params, cfg_e1_line_keepalive_params_cmd, + "e1_line <0-255> keepalive <1-300> <1-20> <1-300>", + E1_LINE_HELP KEEPALIVE_HELP + "Idle interval in seconds before probes are sent\n" + "Number of probes to sent\n" + "Delay between probe packets in seconds\n") +{ + return set_keepalive_params(vty, atoi(argv[0]), + atoi(argv[1]), atoi(argv[2]), atoi(argv[3])); +} + +DEFUN(cfg_e1line_no_keepalive, cfg_e1_line_no_keepalive_cmd, + "no e1_line <0-255> keepalive", + NO_STR E1_LINE_HELP KEEPALIVE_HELP) +{ + return set_keepalive_params(vty, atoi(argv[0]), 0, 0, 0); +} + DEFUN(cfg_e1line_name, cfg_e1_line_name_cmd, "e1_line <0-255> name .LINE", E1_LINE_HELP "Set name for this line\n" "Human readable name\n") @@ -135,6 +185,22 @@ static int e1inp_config_write(struct vty *vty) if (line->name) vty_out(vty, " e1_line %u name %s%s", line->num, line->name, VTY_NEWLINE); + if (!line->keepalive_num_probes) + vty_out(vty, " no e1_line %u keepalive%s", line->num, + VTY_NEWLINE); + else if (line->keepalive_idle_timeout == E1INP_USE_DEFAULT && + line->keepalive_num_probes == E1INP_USE_DEFAULT && + line->keepalive_probe_interval == E1INP_USE_DEFAULT) + vty_out(vty, " e1_line %u keepalive%s", line->num, + VTY_NEWLINE); + else + vty_out(vty, " e1_line %u keepalive %d %d %d%s", + line->num, + line->keepalive_idle_timeout, + line->keepalive_num_probes, + line->keepalive_probe_interval, + VTY_NEWLINE); + } return CMD_SUCCESS; } @@ -281,6 +347,9 @@ int e1inp_vty_init(void) install_element(L_E1INP_NODE, &cfg_e1_line_driver_cmd); install_element(L_E1INP_NODE, &cfg_e1_line_port_cmd); install_element(L_E1INP_NODE, &cfg_e1_line_name_cmd); + install_element(L_E1INP_NODE, &cfg_e1_line_keepalive_cmd); + install_element(L_E1INP_NODE, &cfg_e1_line_keepalive_params_cmd); + install_element(L_E1INP_NODE, &cfg_e1_line_no_keepalive_cmd); install_element_ve(&show_e1drv_cmd); install_element_ve(&show_e1line_cmd); -- 1.7.9.5
This patch changes the implementation to use the keep-alive configuration fields instead of constants. Ticket: OW#1060 Sponsored-by: On-Waves ehf --- src/input/ipaccess.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/input/ipaccess.c b/src/input/ipaccess.c index 6b9d93e..4684520 100644 --- a/src/input/ipaccess.c +++ b/src/input/ipaccess.c @@ -608,6 +608,7 @@ struct e1inp_driver ipaccess_driver = { .line_update = ipaccess_line_update, .close = ipaccess_close, .default_delay = 0, + .has_keepalive = 1, }; static void update_fd_settings(struct e1inp_line *line, int fd) @@ -615,7 +616,7 @@ static void update_fd_settings(struct e1inp_line *line, int fd) int ret; int val; - if (DEFAULT_TCP_KEEPALIVE_RETRY_COUNT) { + if (line->keepalive_num_probes) { /* Enable TCP keepalive to find out if the connection is gone */ val = 1; ret = setsockopt(fd, SOL_SOCKET, SO_KEEPALIVE, &val, sizeof(val)); @@ -627,21 +628,27 @@ static void update_fd_settings(struct e1inp_line *line, int fd) #if defined(TCP_KEEPIDLE) && defined(TCP_KEEPINTVL) && defined(TCP_KEEPCNT) /* The following options are not portable! */ - val = DEFAULT_TCP_KEEPALIVE_IDLE_TIMEOUT; + val = line->keepalive_idle_timeout > 0 ? + line->keepalive_idle_timeout : + DEFAULT_TCP_KEEPALIVE_IDLE_TIMEOUT; ret = setsockopt(fd, IPPROTO_TCP, TCP_KEEPIDLE, &val, sizeof(val)); if (ret < 0) LOGP(DLINP, LOGL_NOTICE, "Failed to set keepalive idle time: %s\n", strerror(errno)); - val = DEFAULT_TCP_KEEPALIVE_INTERVAL; + val = line->keepalive_probe_interval > -1 ? + line->keepalive_probe_interval : + DEFAULT_TCP_KEEPALIVE_INTERVAL; ret = setsockopt(fd, IPPROTO_TCP, TCP_KEEPINTVL, &val, sizeof(val)); if (ret < 0) LOGP(DLINP, LOGL_NOTICE, "Failed to set keepalive interval: %s\n", strerror(errno)); - val = DEFAULT_TCP_KEEPALIVE_RETRY_COUNT; + val = line->keepalive_num_probes > 0 ? + line->keepalive_num_probes : + DEFAULT_TCP_KEEPALIVE_RETRY_COUNT; ret = setsockopt(fd, IPPROTO_TCP, TCP_KEEPCNT, &val, sizeof(val)); if (ret < 0) -- 1.7.9.5
On Thu, Jan 09, 2014 at 02:30:55PM +0100, Jacob Erlbeck wrote:
From: Daniel Willmann <daniel@totalueberwachung.de>
This way we can find out fast if the connection is broken.
s/fast/quickly?
On 13.01.2014 10:04, Holger Hans Peter Freyther wrote:
On Thu, Jan 09, 2014 at 02:30:55PM +0100, Jacob Erlbeck wrote:
From: Daniel Willmann <daniel@totalueberwachung.de>
This way we can find out fast if the connection is broken.
s/fast/quickly?
'fast' is an adverb, too. So I guess it should be okay syntactically. But I'd rather go for the comparative and use 'faster' (or 'more quickly'). Jacob
On Tue, 2014-01-14 at 09:30, Jacob Erlbeck wrote:
On 13.01.2014 10:04, Holger Hans Peter Freyther wrote:
On Thu, Jan 09, 2014 at 02:30:55PM +0100, Jacob Erlbeck wrote:
From: Daniel Willmann <daniel@totalueberwachung.de>
This way we can find out fast if the connection is broken.
s/fast/quickly?
'fast' is an adverb, too. So I guess it should be okay syntactically. But I'd rather go for the comparative and use 'faster' (or 'more quickly').
Reading it here I think I'd prefer (more) quickly. In any case I think it's a minor cosmetical change and either of the possibilities would work. Regards, Daniel
participants (3)
-
Daniel Willmann -
Holger Hans Peter Freyther -
Jacob Erlbeck