Skip site navigation (1)Skip section navigation (2) 
Date:      Sat, 18 Aug 2018 10:58:45 +0000 (UTC)
From:      Eugene Grosbein <eugen@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r338013 - head/contrib/bsnmp/snmp_mibII
Message-ID:  <201808181058.w7IAwjEV094618@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: eugen
Date: Sat Aug 18 10:58:44 2018
New Revision: 338013
URL: https://svnweb.freebsd.org/changeset/base/338013

Log:
  bsnmpd(8): fix and optimize interface description processing
  
  * correctly prepare a buffer to obtain interface description from a kernel and
    truncate long description instead of dropping it altogether and
    spamming logs;
  * skip calling strlen() for each description and each SNMP request
    for MIB-II/ifXTable's ifAlias.
  * teach bsnmpd to allocate memory dynamically for interface descriptions
    to decrease memory usage for common case and not to break
    if long description occurs;
  
  PR:			217763
  Reviewed by:		harti and others
  MFC after:		1 week
  Differential Revision:	https://reviews.freebsd.org/D16459

Modified:
  head/contrib/bsnmp/snmp_mibII/mibII.c
  head/contrib/bsnmp/snmp_mibII/mibII.h
  head/contrib/bsnmp/snmp_mibII/mibII_interfaces.c
  head/contrib/bsnmp/snmp_mibII/snmp_mibII.h

Modified: head/contrib/bsnmp/snmp_mibII/mibII.c
==============================================================================
--- head/contrib/bsnmp/snmp_mibII/mibII.c	Sat Aug 18 10:14:02 2018	(r338012)
+++ head/contrib/bsnmp/snmp_mibII/mibII.c	Sat Aug 18 10:58:44 2018	(r338013)
@@ -439,11 +439,15 @@ mibif_restart_mibII_poll_timer(void)
 int
 mib_fetch_ifmib(struct mibif *ifp)
 {
+	static int kmib[2] = { -1, 0 }; /* for sysctl net.ifdescr_maxlen */
+
 	int name[6];
+	size_t kmiblen = nitems(kmib);
 	size_t len;
 	void *newmib;
 	struct ifmibdata oldmib = ifp->mib;
 	struct ifreq irr;
+	unsigned int alias_maxlen = MIBIF_ALIAS_SIZE_MAX;
 
 	if (fetch_generic_mib(ifp, &oldmib) == -1)
 		return (-1);
@@ -515,18 +519,69 @@ mib_fetch_ifmib(struct mibif *ifp)
 	}
 
   out:
+
+	/*
+	 * Find sysctl mib for net.ifdescr_maxlen (one time).
+	 * kmib[0] == -1 at first call to mib_fetch_ifmib().
+	 * Then kmib[0] > 0 if we found sysctl mib for net.ifdescr_maxlen.
+	 * Else, kmib[0] == 0 (unexpected error from a kernel).
+	 */
+	if (kmib[0] < 0 &&
+	    sysctlnametomib("net.ifdescr_maxlen", kmib, &kmiblen) < 0) {
+		kmib[0] = 0;
+		syslog(LOG_WARNING, "sysctlnametomib net.ifdescr_maxlen: %m");
+	}
+
+	/*
+	 * Fetch net.ifdescr_maxlen value every time to catch up with changes.
+	 */
+	len = sizeof(alias_maxlen);
+	if (kmib[0] > 0 && sysctl(kmib, 2, &alias_maxlen, &len, NULL, 0) < 0) {
+		/* unexpected error from the kernel, use default value */
+		alias_maxlen = MIBIF_ALIAS_SIZE_MAX;
+		syslog(LOG_WARNING, "sysctl net.ifdescr_maxlen: %m");
+	}
+
+	/*
+	 * Kernel limit might be decreased after interfaces got
+	 * their descriptions assigned. Try to obtain them anyway.
+	 */
+	if (alias_maxlen == 0)
+		alias_maxlen = MIBIF_ALIAS_SIZE_MAX;
+
+	/*
+	 * Allocate maximum memory for a buffer and later reallocate
+	 * to free extra memory.
+	 */
+	if ((ifp->alias = malloc(alias_maxlen)) == NULL) {
+		syslog(LOG_WARNING, "malloc(%d) failed: %m", (int)alias_maxlen);
+		goto fin;
+	}
+
 	strlcpy(irr.ifr_name, ifp->name, sizeof(irr.ifr_name));
-	irr.ifr_buffer.buffer = MIBIF_PRIV(ifp)->alias;
-	irr.ifr_buffer.length = sizeof(MIBIF_PRIV(ifp)->alias);
+	irr.ifr_buffer.buffer = ifp->alias;
+	irr.ifr_buffer.length = alias_maxlen;
 	if (ioctl(mib_netsock, SIOCGIFDESCR, &irr) == -1) {
-		MIBIF_PRIV(ifp)->alias[0] = 0;
+		free(ifp->alias);
+		ifp->alias = NULL;
 		if (errno != ENOMSG)
 			syslog(LOG_WARNING, "SIOCGIFDESCR (%s): %m", ifp->name);
 	} else if (irr.ifr_buffer.buffer == NULL) {
-		MIBIF_PRIV(ifp)->alias[0] = 0;
+		free(ifp->alias);
+		ifp->alias = NULL;
 		syslog(LOG_WARNING, "SIOCGIFDESCR (%s): too long (%zu)",
 		    ifp->name, irr.ifr_buffer.length);
+	} else {
+		ifp->alias_size = strnlen(ifp->alias, alias_maxlen) + 1;
+
+		if (ifp->alias_size > MIBIF_ALIAS_SIZE)
+		    ifp->alias_size = MIBIF_ALIAS_SIZE; 
+
+		if (ifp->alias_size < alias_maxlen)
+			ifp->alias = realloc(ifp->alias, ifp->alias_size);
 	}
+
+fin:
 	ifp->mibtick = get_ticks();
 	return (0);
 }
@@ -706,6 +761,10 @@ mibif_free(struct mibif *ifp)
 		mibif_reset_hc_timer();
 	}
 
+	if (ifp->alias != NULL) {
+		free(ifp->alias);
+		ifp->alias = NULL;
+	}
 	free(ifp->private);
 	ifp->private = NULL;
 	free(ifp->physaddr);

Modified: head/contrib/bsnmp/snmp_mibII/mibII.h
==============================================================================
--- head/contrib/bsnmp/snmp_mibII/mibII.h	Sat Aug 18 10:14:02 2018	(r338012)
+++ head/contrib/bsnmp/snmp_mibII/mibII.h	Sat Aug 18 10:58:44 2018	(r338013)
@@ -57,8 +57,9 @@
 #include "snmp_mibII.h"
 #include "mibII_tree.h"
 
-/* maximum size of the interface alias */
+/* maximum size of the interface alias unless overridden with net.ifdescr_maxlen */
 #define	MIBIF_ALIAS_SIZE	(64 + 1)
+#define	MIBIF_ALIAS_SIZE_MAX	1024
 
 /*
  * Interface list and flags.
@@ -81,8 +82,6 @@ struct mibif_private {
 	uint64_t	hc_imcasts;
 	uint64_t	hc_ipackets;
 
-	/* this should be made public */
-	char		alias[MIBIF_ALIAS_SIZE];
 };
 #define	MIBIF_PRIV(IFP) ((struct mibif_private *)((IFP)->private))
 

Modified: head/contrib/bsnmp/snmp_mibII/mibII_interfaces.c
==============================================================================
--- head/contrib/bsnmp/snmp_mibII/mibII_interfaces.c	Sat Aug 18 10:14:02 2018	(r338012)
+++ head/contrib/bsnmp/snmp_mibII/mibII_interfaces.c	Sat Aug 18 10:58:44 2018	(r338013)
@@ -528,7 +528,7 @@ op_ifxtable(struct snmp_context *ctx, struct snmp_valu
 		break;
 
 	  case LEAF_ifAlias:
-		ret = string_get(value, MIBIF_PRIV(ifp)->alias, -1);
+		ret = string_get(value, ifp->alias, ifp->alias_size - 1);
 		break;
 
 	  case LEAF_ifCounterDiscontinuityTime:

Modified: head/contrib/bsnmp/snmp_mibII/snmp_mibII.h
==============================================================================
--- head/contrib/bsnmp/snmp_mibII/snmp_mibII.h	Sat Aug 18 10:14:02 2018	(r338012)
+++ head/contrib/bsnmp/snmp_mibII/snmp_mibII.h	Sat Aug 18 10:58:44 2018	(r338013)
@@ -80,6 +80,9 @@ struct mibif {
 	/* to be set by ifType specific modules. This is ifSpecific. */
 	struct asn_oid	spec_oid;
 
+	char		*alias;
+	size_t		alias_size;
+
 	/* private data - don't touch */
 	void		*private;
 };

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201808181058.w7IAwjEV094618>