From 8e91bb875bdd1c6542d083e1b4ecac961df4004c Mon Sep 17 00:00:00 2001 From: Lorenzo Miniero Date: Fri, 29 Jan 2021 18:25:36 +0100 Subject: [PATCH 1/3] Add new option to configure ICE nomination mode, if libnice is recent enough --- conf/janus.jcfg.sample.in | 6 +++++- configure.ac | 6 ++++++ ice.c | 25 +++++++++++++++++++++++++ ice.h | 8 ++++++++ janus.c | 11 +++++++++++ 5 files changed, 55 insertions(+), 1 deletion(-) diff --git a/conf/janus.jcfg.sample.in b/conf/janus.jcfg.sample.in index 7b9c384062..284944530e 100644 --- a/conf/janus.jcfg.sample.in +++ b/conf/janus.jcfg.sample.in @@ -239,7 +239,10 @@ media: { # configured to do full-trickle (Janus also trickles its candidates to # users) rather than the default half-trickle (Janus supports trickle # candidates from users, but sends its own within the SDP), and whether -# it should work in ICE-Lite mode (by default it doesn't). Finally, +# it should work in ICE-Lite mode (by default it doesn't). If libnice is +# at least 0.1.15, you can choose which ICE nomination mode to use: valid +# values are "regular" and "aggressive" (the default depends on the libnice +# version itself; if we can set it, we set regular nomination). Finally, # you can also enable ICE-TCP support (beware that this may lead to problems # if you do not enable ICE Lite as well), choose which interfaces should # be used for gathering candidates, and enable or disable the @@ -249,6 +252,7 @@ nat: { #stun_port = 3478 nice_debug = false #full_trickle = true + #ice_nomination = "regular" #ice_lite = true #ice_tcp = true diff --git a/configure.ac b/configure.ac index c75c6b98de..84ad30aec7 100644 --- a/configure.ac +++ b/configure.ac @@ -402,6 +402,12 @@ AC_CHECK_LIB([nice], [AC_MSG_NOTICE([libnice version does not have nice_agent_close_async])] ) +AC_CHECK_LIB([nice], + [nice_agent_new_full], + [AC_DEFINE(HAVE_ICE_NOMINATION)], + [AC_MSG_NOTICE([libnice version does not have nice_agent_new_full])] + ) + AC_CHECK_LIB([dl], [dlopen], [JANUS_MANUAL_LIBS="${JANUS_MANUAL_LIBS} -ldl"], diff --git a/ice.c b/ice.c index 8ebaaeefe5..28ff5c61de 100644 --- a/ice.c +++ b/ice.c @@ -101,6 +101,28 @@ gboolean janus_ice_is_ipv6_enabled(void) { return janus_ipv6_enabled; } +#ifdef HAVE_ICE_NOMINATION +/* Since libnice 0.1.15, we can configure the ICE nomination mode: it was + * always "aggressive" before, we set it to "regular" by default if we can */ +static NiceNominationMode janus_ice_nomination = NICE_NOMINATION_MODE_REGULAR; +void janus_ice_set_nomination_mode(const char *nomination) { + if(nomination == NULL) { + JANUS_LOG(LOG_WARN, "Invalid ICE nomination mode, falling back to 'regular'\n"); + } else if(!strcasecmp(nomination, "regular")) { + JANUS_LOG(LOG_INFO, "Configuring Janus to use ICE regular nomination\n"); + janus_ice_nomination = NICE_NOMINATION_MODE_REGULAR; + } else if(!strcasecmp(nomination, "aggressive")) { + JANUS_LOG(LOG_INFO, "Configuring Janus to use ICE aggressive nomination\n"); + janus_ice_nomination = NICE_NOMINATION_MODE_AGGRESSIVE; + } else { + JANUS_LOG(LOG_WARN, "Unsupported ICE nomination mode '%s', falling back to 'regular'\n", nomination); + } +} +const char *janus_ice_get_nomination_mode(void) { + return (janus_ice_nomination == NICE_NOMINATION_MODE_REGULAR ? "regular" : "aggressive"); +} +#endif + /* Opaque IDs set by applications are by default only passed to event handlers * for correlation purposes, but not sent back to the user or application in * the related Janus API responses or events, unless configured otherwise */ @@ -3461,6 +3483,9 @@ int janus_ice_setup_local(janus_ice_handle *handle, int offer, int audio, int vi "main-context", handle->mainctx, "reliable", FALSE, "full-mode", janus_ice_lite_enabled ? FALSE : TRUE, +#ifdef HAVE_ICE_NOMINATION + "nomination-mode", janus_ice_nomination, +#endif #ifdef HAVE_LIBNICE_TCP "ice-udp", TRUE, "ice-tcp", janus_ice_tcp_enabled ? TRUE : FALSE, diff --git a/ice.h b/ice.h index 8fe22c5c33..9428fe4dcb 100644 --- a/ice.h +++ b/ice.h @@ -129,6 +129,14 @@ gboolean janus_ice_is_mdns_enabled(void); /*! \brief Method to check whether IPv6 candidates are enabled/supported or not * @returns true if IPv6 candidates are enabled/supported, false otherwise */ gboolean janus_ice_is_ipv6_enabled(void); +#ifdef HAVE_ICE_NOMINATION +/*! \brief Method to configure the ICE nomination mode (regular or aggressive) + * @param[in] nomination The ICE nomination mode (regular or aggressive) */ +void janus_ice_set_nomination_mode(const char *nomination); +/*! \brief Method to return a string description of the configured ICE nomination mode + * @returns "regular" or "aggressive" */ +const char *janus_ice_get_nomination_mode(void); +#endif /*! \brief Method to modify the min NACK value (i.e., the minimum time window of packets per handle to store for retransmissions) * @param[in] mnq The new min NACK value */ void janus_set_min_nack_queue(uint16_t mnq); diff --git a/janus.c b/janus.c index 7e103a15f3..0159acb216 100644 --- a/janus.c +++ b/janus.c @@ -346,6 +346,9 @@ static json_t *janus_info(const char *transaction) { json_object_set_new(info, "ipv6", janus_ice_is_ipv6_enabled() ? json_true() : json_false()); json_object_set_new(info, "ice-lite", janus_ice_is_ice_lite_enabled() ? json_true() : json_false()); json_object_set_new(info, "ice-tcp", janus_ice_is_ice_tcp_enabled() ? json_true() : json_false()); +#ifdef HAVE_ICE_NOMINATION + json_object_set_new(info, "ice-nomination", json_string(janus_ice_get_nomination_mode())); +#endif json_object_set_new(info, "full-trickle", janus_ice_is_full_trickle_enabled() ? json_true() : json_false()); json_object_set_new(info, "mdns-enabled", janus_ice_is_mdns_enabled() ? json_true() : json_false()); json_object_set_new(info, "min-nack-queue", json_integer(janus_get_min_nack_queue())); @@ -4787,6 +4790,14 @@ gint main(int argc, char *argv[]) JANUS_LOG(LOG_ERR, "Invalid STUN address %s:%u. STUN will be disabled\n", stun_server, stun_port); } } + item = janus_config_get(config, config_nat, janus_config_type_item, "ice_nomination"); + if(item && item->value) { +#ifndef HAVE_ICE_NOMINATION + JANUS_LOG(LOG_WARN, "This version of libnice doesn't support setting the ICE nomination mode, ignoring '%s'\n", item->value); +#else + janus_ice_set_nomination_mode(item->value); +#endif + } if(janus_ice_set_turn_server(turn_server, turn_port, turn_type, turn_user, turn_pwd) < 0) { if(!ignore_unreachable_ice_server) { JANUS_LOG(LOG_FATAL, "Invalid TURN address %s:%u\n", turn_server, turn_port); From 5f68c3ece27ae141d79aa789175ed9db0ce11094 Mon Sep 17 00:00:00 2001 From: Lorenzo Miniero Date: Thu, 4 Feb 2021 16:35:25 +0100 Subject: [PATCH 2/3] Added support for libnice keepalive-conncheck property --- conf/janus.jcfg.sample.in | 5 ++++- ice.c | 13 +++++++++++++ ice.h | 10 +++++++++- janus.c | 4 ++++ 4 files changed, 30 insertions(+), 2 deletions(-) diff --git a/conf/janus.jcfg.sample.in b/conf/janus.jcfg.sample.in index 284944530e..f978b87390 100644 --- a/conf/janus.jcfg.sample.in +++ b/conf/janus.jcfg.sample.in @@ -242,7 +242,9 @@ media: { # it should work in ICE-Lite mode (by default it doesn't). If libnice is # at least 0.1.15, you can choose which ICE nomination mode to use: valid # values are "regular" and "aggressive" (the default depends on the libnice -# version itself; if we can set it, we set regular nomination). Finally, +# version itself; if we can set it, we set regular nomination). You can +# also configure whether to use connectivity checks as keep-alives, which +# might help detecting when a peer is no longer available. Finally, # you can also enable ICE-TCP support (beware that this may lead to problems # if you do not enable ICE Lite as well), choose which interfaces should # be used for gathering candidates, and enable or disable the @@ -253,6 +255,7 @@ nat: { nice_debug = false #full_trickle = true #ice_nomination = "regular" + #ice_keepalive_conncheck = true #ice_lite = true #ice_tcp = true diff --git a/ice.c b/ice.c index 28ff5c61de..5540f5b7a5 100644 --- a/ice.c +++ b/ice.c @@ -123,6 +123,18 @@ const char *janus_ice_get_nomination_mode(void) { } #endif +/* Keepalive via connectivity checks */ +static gboolean janus_ice_keepalive_connckecks = FALSE; +void janus_ice_set_keepalive_conncheck_enabled(gboolean enabled) { + janus_ice_keepalive_connckecks = enabled; + if(janus_ice_keepalive_connckecks) { + JANUS_LOG(LOG_INFO, "Using connectivity checks as PeerConnection keep-alives\n"); + } +} +gboolean janus_ice_is_keepalive_conncheck_enabled(void) { + return janus_ice_keepalive_connckecks; +} + /* Opaque IDs set by applications are by default only passed to event handlers * for correlation purposes, but not sent back to the user or application in * the related Janus API responses or events, unless configured otherwise */ @@ -3486,6 +3498,7 @@ int janus_ice_setup_local(janus_ice_handle *handle, int offer, int audio, int vi #ifdef HAVE_ICE_NOMINATION "nomination-mode", janus_ice_nomination, #endif + "keepalive-conncheck", janus_ice_keepalive_connckecks ? FALSE : TRUE, #ifdef HAVE_LIBNICE_TCP "ice-udp", TRUE, "ice-tcp", janus_ice_tcp_enabled ? TRUE : FALSE, diff --git a/ice.h b/ice.h index 9428fe4dcb..015e940306 100644 --- a/ice.h +++ b/ice.h @@ -137,6 +137,14 @@ void janus_ice_set_nomination_mode(const char *nomination); * @returns "regular" or "aggressive" */ const char *janus_ice_get_nomination_mode(void); #endif +/*! \brief Method to enable/disable connectivity checks as keepalives for PeerConnections. + * \note The main rationale behind this setting is provided in the libnice documentation: + * https://libnice.freedesktop.org/libnice/NiceAgent.html#NiceAgent--keepalive-conncheck + * @param[in] enabled Whether the functionality should be enabled or disabled */ +void janus_ice_set_keepalive_conncheck_enabled(gboolean enabled); +/*! \brief Method to check whether connectivity checks will be used as keepalives + * @returns true if enabled, false (default) otherwise */ +gboolean janus_ice_is_keepalive_conncheck_enabled(void); /*! \brief Method to modify the min NACK value (i.e., the minimum time window of packets per handle to store for retransmissions) * @param[in] mnq The new min NACK value */ void janus_set_min_nack_queue(uint16_t mnq); @@ -148,7 +156,7 @@ uint16_t janus_get_min_nack_queue(void); * keyframe, as any missing packet won't be needed since the keyframe will allow the * media recipient to still restore a complete image anyway. Since this optimization * seems to cause some issues in some edge cases, it's disabled by default. - * @param[in] optimize Whether the opzimization should be enabled or disabled */ + * @param[in] optimize Whether the optimization should be enabled or disabled */ void janus_set_nack_optimizations_enabled(gboolean optimize); /*! \brief Method to check whether NACK optimizations on outgoing keyframes are enabled or not * @returns optimize if optimizations are enabled, false otherwise */ diff --git a/janus.c b/janus.c index 0159acb216..db1ca89689 100644 --- a/janus.c +++ b/janus.c @@ -349,6 +349,7 @@ static json_t *janus_info(const char *transaction) { #ifdef HAVE_ICE_NOMINATION json_object_set_new(info, "ice-nomination", json_string(janus_ice_get_nomination_mode())); #endif + json_object_set_new(info, "ice-keepalive-conncheck", janus_ice_is_keepalive_conncheck_enabled() ? json_true() : json_false()); json_object_set_new(info, "full-trickle", janus_ice_is_full_trickle_enabled() ? json_true() : json_false()); json_object_set_new(info, "mdns-enabled", janus_ice_is_mdns_enabled() ? json_true() : json_false()); json_object_set_new(info, "min-nack-queue", json_integer(janus_get_min_nack_queue())); @@ -4798,6 +4799,9 @@ gint main(int argc, char *argv[]) janus_ice_set_nomination_mode(item->value); #endif } + item = janus_config_get(config, config_nat, janus_config_type_item, "ice_keepalive_conncheck"); + if(item && item->value) + janus_ice_set_keepalive_conncheck_enabled(janus_is_true(item->value)); if(janus_ice_set_turn_server(turn_server, turn_port, turn_type, turn_user, turn_pwd) < 0) { if(!ignore_unreachable_ice_server) { JANUS_LOG(LOG_FATAL, "Invalid TURN address %s:%u\n", turn_server, turn_port); From e877a224e01b58e6a7f31102ca59eddfda44b731 Mon Sep 17 00:00:00 2001 From: Lorenzo Miniero Date: Fri, 12 Feb 2021 15:27:03 +0100 Subject: [PATCH 3/3] Added warning when enabling ice_keepalive_conncheck --- conf/janus.jcfg.sample.in | 5 ++++- ice.c | 1 + 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/conf/janus.jcfg.sample.in b/conf/janus.jcfg.sample.in index f978b87390..d1a4a0b518 100644 --- a/conf/janus.jcfg.sample.in +++ b/conf/janus.jcfg.sample.in @@ -244,7 +244,10 @@ media: { # values are "regular" and "aggressive" (the default depends on the libnice # version itself; if we can set it, we set regular nomination). You can # also configure whether to use connectivity checks as keep-alives, which -# might help detecting when a peer is no longer available. Finally, +# might help detecting when a peer is no longer available (notice that +# current libnice master is breaking connections after 50 seconds when +# keepalive-conncheck is being used, so if you want to use it, better +# sticking to 0.1.18 until the issue is addressed upstream). Finally, # you can also enable ICE-TCP support (beware that this may lead to problems # if you do not enable ICE Lite as well), choose which interfaces should # be used for gathering candidates, and enable or disable the diff --git a/ice.c b/ice.c index 5540f5b7a5..fb1eb38a33 100644 --- a/ice.c +++ b/ice.c @@ -129,6 +129,7 @@ void janus_ice_set_keepalive_conncheck_enabled(gboolean enabled) { janus_ice_keepalive_connckecks = enabled; if(janus_ice_keepalive_connckecks) { JANUS_LOG(LOG_INFO, "Using connectivity checks as PeerConnection keep-alives\n"); + JANUS_LOG(LOG_WARN, "Notice that the current libnice master is breaking connections after 50s when keepalive-conncheck enabled. As such, better to stick to 0.1.18 until the issue is addressed upstream\n"); } } gboolean janus_ice_is_keepalive_conncheck_enabled(void) {