From f6df53ecfce0111b1dcf2350aeea015be109db7f Mon Sep 17 00:00:00 2001 From: Martin Buck Date: Thu, 27 Apr 2017 14:21:38 +0200 Subject: [PATCH] Expire only unused dynamic routes instead of flushing all of them When expiring periodically (option -c), look at packet counters for each dynamic route and flush only those which were not used during the last period. This avoids toggling between different ingress interfaces if traffic arrives on several interfaces simultaneously. In this case, the first selected ingress interface is retained until traffic on it ceases. Not flushing while there is still traffic also avoids unnecessary MFC cache misses in the kernel which may cause packet loss in certain situations (e.g. bursty traffic or too many cache misses for different groups/source addresses at once). Signed-off-by: Martin Buck --- smcroute.8 | 11 +++++-- src/mroute.c | 80 ++++++++++++++++++++++++++++++++++++++++++------- src/mroute.h | 4 ++- src/smcrouted.c | 6 ++-- 4 files changed, 84 insertions(+), 17 deletions(-) diff --git a/smcroute.8 b/smcroute.8 index bbb63468..91bbd7fe 100644 --- a/smcroute.8 +++ b/smcroute.8 @@ -72,13 +72,20 @@ enables only the interfaces needed. Alternate configuration file, default .Pa /etc/smcroute.conf .It Fl c Ar SEC -Flush cache of dynamically learned (*,G) multicast routes every +Flush unused dynamic (*,G) multicast routes every .Ar SEC seconds. This option is intended for systems with topology changes, i.e., when inbound multicast may change both interface and source IP address. If there is no way of detecting such a topology change this option will make sure to periodically flush all dynamically learned -multicast routes so that traffic may resume. See also the +multicast routes so that traffic may resume. Flushing of a specific +route only occurs if it was unused during the last flush interval, +i.e. there was no traffic matching it. This avoids toggling between +different inbound interfaces if traffic arrives on several interfaces +simultaneously. In this case, the first selected inbound interface is +retained until traffic on it ceases. +.Pp +See also the .Nm smcroutectl Ar flush command for another way of handling topology changes. .It Fl e Ar CMD diff --git a/src/mroute.c b/src/mroute.c index 891a051b..8ac3ffa7 100644 --- a/src/mroute.c +++ b/src/mroute.c @@ -29,6 +29,8 @@ #include #include #include +#include +#include #include "log.h" #include "ifvc.h" @@ -366,22 +368,78 @@ int mroute4_dyn_add(struct mroute4 *route) return -1; } +/* Get usage statistics from the kernel for an installed MFC entry */ +static int mroute4_get_stats(struct mroute4 *route, unsigned long *pktcnt, unsigned long *bytecnt, unsigned long *wrong_if) +{ + int result = 0; + struct sioc_sg_req sg_req; + + memset(&sg_req, 0, sizeof(sg_req)); + sg_req.src = route->sender; + sg_req.grp = route->group; + + if (ioctl(mroute4_socket, SIOCGETSGCNT, (void *)&sg_req) < 0) { + result = errno; + smclog(LOG_WARNING, "Failed getting MFC stats: %m"); + } else { + if (pktcnt) *pktcnt = sg_req.pktcnt; + if (bytecnt) *bytecnt = sg_req.bytecnt; + if (wrong_if) *wrong_if = sg_req.wrong_if; + } + + return result; +} + +/* Get valid packet usage statistics (i.e. number of actually forwarded + * packets) from the kernel for an installed MFC entry */ +static unsigned long mroute4_get_stats_valid_pkt(struct mroute4 *route) +{ + unsigned long pktcnt = 0, wrong_if = 0; + + if (mroute4_get_stats(route, &pktcnt, NULL, &wrong_if) < 0) + return 0; + return pktcnt - wrong_if; +} + /** - * mroute4_dyn_flush - Flush dynamically added (*,G) routes + * mroute4_dyn_expire - Expire dynamically added (*,G) routes + * @max_idle: Timeout for routes in seconds, 0 to expire all dynamic routes * - * This function flushes all (*,G) routes. It is currently only called - * on cache-timeout, a command line option, but could also be called on - * topology changes (e.g. VRRP fail-over) or similar. + * This function flushes all (*,G) routes which haven't been used (i.e. no + * packets matching them have been forwarded) in the last max_idle seconds. + * It is called periodically on cache-timeout or on request of smcroutectl. + * The latter is useful in case of topology changes (e.g. VRRP fail-over) + * or similar. */ -void mroute4_dyn_flush(void) +void mroute4_dyn_expire(int max_idle) { - if (LIST_EMPTY(&mroute4_dyn_list)) - return; + struct mroute4 *entry, *next; + struct timeval now = { 0 }; + + gettimeofday(&now, NULL); - while (mroute4_dyn_list.lh_first) { - __mroute4_del((struct mroute4 *)mroute4_dyn_list.lh_first); - LIST_REMOVE(mroute4_dyn_list.lh_first, link); - free(mroute4_dyn_list.lh_first); + entry = LIST_FIRST(&mroute4_dyn_list); + while (entry) { + next = LIST_NEXT(entry, link); + if (!entry->last_use) { + /* New entry */ + entry->last_use = now.tv_sec; + entry->valid_pkt = mroute4_get_stats_valid_pkt(entry); + } + if (entry->last_use + max_idle <= now.tv_sec) { + unsigned long valid_pkt = mroute4_get_stats_valid_pkt(entry); + if (valid_pkt != entry->valid_pkt) { + /* Used since last check, update */ + entry->last_use = now.tv_sec; + entry->valid_pkt = valid_pkt; + } else { + /* Not used, expire */ + __mroute4_del(entry); + LIST_REMOVE(entry, link); + free(entry); + } + } + entry = next; } } diff --git a/src/mroute.h b/src/mroute.h index 27d7bef4..94c4c978 100644 --- a/src/mroute.h +++ b/src/mroute.h @@ -60,6 +60,8 @@ struct mroute4 { short inbound; /* incoming VIF */ uint8_t ttl[MAX_MC_VIFS];/* outgoing VIFs */ + unsigned long valid_pkt; /* packet counter at last mroute4_dyn_expire() */ + time_t last_use; /* timestamp of last forwarded packet */ }; /* @@ -108,7 +110,7 @@ extern int mroute6_socket; int mroute4_enable (int do_vifs); void mroute4_disable (void); int mroute4_dyn_add (struct mroute4 *mroute); -void mroute4_dyn_flush (void); +void mroute4_dyn_expire(int max_idle); int mroute4_add (struct mroute4 *mroute); int mroute4_del (struct mroute4 *mroute); diff --git a/src/smcrouted.c b/src/smcrouted.c index 2dc14fd2..9b1fe5a5 100644 --- a/src/smcrouted.c +++ b/src/smcrouted.c @@ -302,7 +302,7 @@ static void read_ipc_command(void) break; case 'F': - mroute4_dyn_flush(); + mroute4_dyn_expire(0); ipc_send("", 1); break; @@ -391,8 +391,8 @@ static int server_loop(int sd) if (cache_tmo && (last_cache_flush.tv_sec + cache_tmo < now.tv_sec)) { last_cache_flush = now; - smclog(LOG_NOTICE, "Cache timeout, flushing all (*,G) routes!"); - mroute4_dyn_flush(); + smclog(LOG_NOTICE, "Cache timeout, flushing unused (*,G) routes!"); + mroute4_dyn_expire(cache_tmo); } if (FD_ISSET(mroute4_socket, &fds))