[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 <fdinitto@redhat.com>

diff --git a/libknet/internals.h b/libknet/internals.h
index c1c715091e44b68ad750e7c34aa06624166142e9..70e635fee8c7554a04acb49c8ac24ab17f1e37e2 100644 (file)
--- 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 701e0fd6b59ac09d146aab7b5b0785a3c6468529..4e727e913bd30d7183e0ce0246382d923025f478 100644 (file)
--- 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 4b2826b2f910d8fac689745dabd269a204f02ccb..aec42d07f49f97f55aaae40fb86bc884c91fcb65 100644 (file)
--- 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