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 <mb-tmp-tvguho.pbz@gromit.dyndns.org>

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

src/mroute.c

@@ -29,6 +29,8 @@
 #include <arpa/inet.h>
 #include <net/if.h>
 #include <unistd.h>
+#include <sys/time.h>
+#include <sys/ioctl.h>
 
 #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;
 	}
 }
 

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);
 

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))