From 089d0e14e5c81ac9e462831c70d82870997c487c Mon Sep 17 00:00:00 2001 From: "Fabio M. Di Nitto" Date: Sat, 30 Dec 2017 06:19:13 +0100 Subject: [PATCH] [PTMUd] if any threads receives a EMSGSIZE outside of a PMTUD run, force PTMUd to run Scenario: node X has MTU Y on the interface and application is sending packets with size >= Y. The interface MTU is suddenly reduced to < Y Before this change, the kernel would be dropping packets till the next PMTUd run. After this change, the PMTUd will be informed that it has to rerun (overriding the pmtud_interval), reducing the packet drop to a minimum. How to test: force knet_bench to send 1500 size packets with ping_data (requires code change) and start it. reduce MTU on the interface from 1500 to 1300 (for example) Notice an immediate trigger of PMTUd run in debug mode Note: going up, from 1300 to 1500 will wait for the next PMTUd re-run as there is no immediate way to detect this change unless we start listening to kernel netlink sockets with libnl3 (investigation in progress but not critical enough atm). Signed-off-by: Fabio M. Di Nitto --- libknet/internals.h | 2 ++ libknet/threads_pmtud.c | 38 +++++++++++++++++++++++++++----------- libknet/transport_udp.c | 17 +++++++++++++++++ 3 files changed, 46 insertions(+), 11 deletions(-) diff --git a/libknet/internals.h b/libknet/internals.h index c1c7150..70e635f 100644 --- a/libknet/internals.h +++ b/libknet/internals.h @@ -189,6 +189,8 @@ struct knet_handle { pthread_mutex_t kmtu_mutex; /* used to protect kernel_mtu */ uint32_t kernel_mtu; /* contains the MTU detected by the kernel on a given link */ int pmtud_waiting; + int pmtud_running; + int pmtud_forcerun; int pmtud_abort; struct crypto_instance *crypto_instance; size_t sec_header_size; diff --git a/libknet/threads_pmtud.c b/libknet/threads_pmtud.c index 701e0fd..4e727e9 100644 --- a/libknet/threads_pmtud.c +++ b/libknet/threads_pmtud.c @@ -340,25 +340,27 @@ retry: goto restart; } -static int _handle_check_pmtud(knet_handle_t knet_h, struct knet_host *dst_host, struct knet_link *dst_link, unsigned int *min_mtu) +static int _handle_check_pmtud(knet_handle_t knet_h, struct knet_host *dst_host, struct knet_link *dst_link, unsigned int *min_mtu, int force_run) { uint8_t saved_valid_pmtud; unsigned int saved_pmtud; struct timespec clock_now; unsigned long long diff_pmtud, interval; - interval = knet_h->pmtud_interval * 1000000000llu; /* nanoseconds */ + if (!force_run) { + interval = knet_h->pmtud_interval * 1000000000llu; /* nanoseconds */ - if (clock_gettime(CLOCK_MONOTONIC, &clock_now) != 0) { - log_debug(knet_h, KNET_SUB_PMTUD, "Unable to get monotonic clock"); - return 0; - } + if (clock_gettime(CLOCK_MONOTONIC, &clock_now) != 0) { + log_debug(knet_h, KNET_SUB_PMTUD, "Unable to get monotonic clock"); + return 0; + } - timespec_diff(dst_link->pmtud_last, clock_now, &diff_pmtud); + timespec_diff(dst_link->pmtud_last, clock_now, &diff_pmtud); - if (diff_pmtud < interval) { - *min_mtu = dst_link->status.mtu; - return dst_link->has_valid_mtu; + if (diff_pmtud < interval) { + *min_mtu = dst_link->status.mtu; + return dst_link->has_valid_mtu; + } } switch (dst_link->dst_addr.ss_family) { @@ -444,6 +446,7 @@ void *_handle_pmtud_link_thread(void *data) unsigned int min_mtu, have_mtu; unsigned int lower_mtu; int link_has_mtu; + int force_run = 0; knet_h->data_mtu = KNET_PMTUD_MIN_MTU_V4 - KNET_HEADER_ALL_SIZE - knet_h->sec_header_size; @@ -460,8 +463,15 @@ void *_handle_pmtud_link_thread(void *data) continue; } knet_h->pmtud_abort = 0; + knet_h->pmtud_running = 1; + force_run = knet_h->pmtud_forcerun; + knet_h->pmtud_forcerun = 0; pthread_mutex_unlock(&knet_h->pmtud_mutex); + if (force_run) { + log_debug(knet_h, KNET_SUB_PMTUD, "PMTUd request to rerun has been received"); + } + if (pthread_rwlock_rdlock(&knet_h->global_rwlock) != 0) { log_debug(knet_h, KNET_SUB_PMTUD, "Unable to get read lock"); continue; @@ -483,7 +493,7 @@ void *_handle_pmtud_link_thread(void *data) (dst_link->status.dynconnected != 1))) continue; - link_has_mtu = _handle_check_pmtud(knet_h, dst_host, dst_link, &min_mtu); + link_has_mtu = _handle_check_pmtud(knet_h, dst_host, dst_link, &min_mtu, force_run); if (errno == EDEADLK) { goto out_unlock; } @@ -509,6 +519,12 @@ void *_handle_pmtud_link_thread(void *data) } out_unlock: pthread_rwlock_unlock(&knet_h->global_rwlock); + if (pthread_mutex_lock(&knet_h->pmtud_mutex) != 0) { + log_debug(knet_h, KNET_SUB_PMTUD, "Unable to get mutex lock"); + } else { + knet_h->pmtud_running = 0; + pthread_mutex_unlock(&knet_h->pmtud_mutex); + } } return NULL; diff --git a/libknet/transport_udp.c b/libknet/transport_udp.c index 4b2826b..aec42d0 100644 --- a/libknet/transport_udp.c +++ b/libknet/transport_udp.c @@ -330,11 +330,28 @@ static int read_errs_from_sock(knet_handle_t knet_h, int sockfd) case 1: /* local source (EMSGSIZE) */ if (sock_err->ee_errno == EMSGSIZE) { if (pthread_mutex_lock(&knet_h->kmtu_mutex) != 0) { + log_debug(knet_h, KNET_SUB_TRANSP_UDP, "Unable to get mutex lock"); knet_h->kernel_mtu = 0; + break; } else { knet_h->kernel_mtu = sock_err->ee_info; pthread_mutex_unlock(&knet_h->kmtu_mutex); } + + /* + * we can only try to take a lock here. This part of the code + * can be invoked by any thread, including PMTUd that is already + * holding a lock at that stage. + * If PMTUd is holding the lock, mostlikely it is already running + * and we don't need to notify it back. + */ + if (!pthread_mutex_trylock(&knet_h->pmtud_mutex)) { + if (!knet_h->pmtud_running) { + log_debug(knet_h, KNET_SUB_TRANSP_UDP, "Notifying PMTUd to rerun"); + knet_h->pmtud_forcerun = 1; + } + pthread_mutex_unlock(&knet_h->pmtud_mutex); + } } /* * those errors are way too noisy -- 2.39.5