Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 27 Sep 2014 13:57:48 +0000 (UTC)
From:      "Alexander V. Chernikov" <melifaro@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r272211 - head/sys/net
Message-ID:  <201409271357.s8RDvmTC072149@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: melifaro
Date: Sat Sep 27 13:57:48 2014
New Revision: 272211
URL: http://svnweb.freebsd.org/changeset/base/272211

Log:
  Use underlying ports counters to get lagg statistics instead of
  per-packet accounting.
  This introduce user-visible changes like aggregating error counters.
  
  Reviewed by:	asomers (prev.version), glebius
  CR:		D781
  MFC after:	2 weeks
  Sponsored by:	Yandex LLC

Modified:
  head/sys/net/if_lagg.c
  head/sys/net/if_lagg.h
  head/sys/net/if_var.h

Modified: head/sys/net/if_lagg.c
==============================================================================
--- head/sys/net/if_lagg.c	Sat Sep 27 10:57:34 2014	(r272210)
+++ head/sys/net/if_lagg.c	Sat Sep 27 13:57:48 2014	(r272211)
@@ -115,6 +115,7 @@ static int	lagg_ether_cmdmulti(struct la
 static	int	lagg_setflag(struct lagg_port *, int, int,
 		    int (*func)(struct ifnet *, int));
 static	int	lagg_setflags(struct lagg_port *, int status);
+static uint64_t lagg_get_counter(struct ifnet *ifp, ift_counter cnt);
 static int	lagg_transmit(struct ifnet *, struct mbuf *);
 static void	lagg_qflush(struct ifnet *);
 static int	lagg_media_change(struct ifnet *);
@@ -158,8 +159,6 @@ static struct mbuf *lagg_lacp_input(stru
 		    struct mbuf *);
 static void	lagg_lacp_lladdr(struct lagg_softc *);
 
-static void	lagg_callout(void *);
-
 /* lagg protocol table */
 static const struct lagg_proto {
 	lagg_proto	pr_num;
@@ -456,11 +455,6 @@ lagg_clone_create(struct if_clone *ifc, 
 		return (ENOSPC);
 	}
 
-	sc->sc_ipackets = counter_u64_alloc(M_WAITOK);
-	sc->sc_opackets = counter_u64_alloc(M_WAITOK);
-	sc->sc_ibytes = counter_u64_alloc(M_WAITOK);
-	sc->sc_obytes = counter_u64_alloc(M_WAITOK);
-
 	sysctl_ctx_init(&sc->ctx);
 	snprintf(num, sizeof(num), "%u", unit);
 	sc->use_flowid = def_use_flowid;
@@ -490,16 +484,9 @@ lagg_clone_create(struct if_clone *ifc, 
 	lagg_proto_attach(sc, LAGG_PROTO_DEFAULT);
 
 	LAGG_LOCK_INIT(sc);
-	LAGG_CALLOUT_LOCK_INIT(sc);
 	SLIST_INIT(&sc->sc_ports);
 	TASK_INIT(&sc->sc_lladdr_task, 0, lagg_port_setlladdr, sc);
 
-	/*
-	 * This uses the callout lock rather than the rmlock; one can't
-	 * hold said rmlock during SWI.
-	 */
-	callout_init_mtx(&sc->sc_callout, &sc->sc_call_mtx, 0);
-
 	/* Initialise pseudo media types */
 	ifmedia_init(&sc->sc_media, 0, lagg_media_change,
 	    lagg_media_status);
@@ -512,6 +499,7 @@ lagg_clone_create(struct if_clone *ifc, 
 	ifp->if_qflush = lagg_qflush;
 	ifp->if_init = lagg_init;
 	ifp->if_ioctl = lagg_ioctl;
+	ifp->if_get_counter = lagg_get_counter;
 	ifp->if_flags = IFF_SIMPLEX | IFF_BROADCAST | IFF_MULTICAST;
 	ifp->if_capenable = ifp->if_capabilities = IFCAP_HWSTATS;
 
@@ -531,8 +519,6 @@ lagg_clone_create(struct if_clone *ifc, 
 	SLIST_INSERT_HEAD(&lagg_list, sc, sc_entries);
 	mtx_unlock(&lagg_list_mtx);
 
-	callout_reset(&sc->sc_callout, hz, lagg_callout, sc);
-
 	return (0);
 }
 
@@ -561,22 +547,12 @@ lagg_clone_destroy(struct ifnet *ifp)
 	ether_ifdetach(ifp);
 	if_free(ifp);
 
-	/* This grabs sc_callout_mtx, serialising it correctly */
-	callout_drain(&sc->sc_callout);
-
-	/* At this point it's drained; we can free this */
-	counter_u64_free(sc->sc_ipackets);
-	counter_u64_free(sc->sc_opackets);
-	counter_u64_free(sc->sc_ibytes);
-	counter_u64_free(sc->sc_obytes);
-
 	mtx_lock(&lagg_list_mtx);
 	SLIST_REMOVE(&lagg_list, sc, lagg_softc, sc_entries);
 	mtx_unlock(&lagg_list_mtx);
 
 	taskqueue_drain(taskqueue_swi, &sc->sc_lladdr_task);
 	LAGG_LOCK_DESTROY(sc);
-	LAGG_CALLOUT_LOCK_DESTROY(sc);
 	free(sc, M_DEVBUF);
 }
 
@@ -712,7 +688,8 @@ lagg_port_create(struct lagg_softc *sc, 
 {
 	struct lagg_softc *sc_ptr;
 	struct lagg_port *lp, *tlp;
-	int error;
+	int error, i;
+	uint64_t *pval;
 
 	LAGG_WLOCK_ASSERT(sc);
 
@@ -836,6 +813,10 @@ lagg_port_create(struct lagg_softc *sc, 
 	lagg_capabilities(sc);
 	lagg_linkstate(sc);
 
+	/* Read port counters */
+	pval = lp->port_counters.val;
+	for (i = IFCOUNTER_IPACKETS; i <= IFCOUNTER_LAST; i++, pval++)
+		*pval = ifp->if_get_counter(ifp, i);
 	/* Add multicast addresses and interface flags to this port */
 	lagg_ether_cmdmulti(lp, 1);
 	lagg_setflags(lp, 1);
@@ -877,6 +858,8 @@ lagg_port_destroy(struct lagg_port *lp, 
 	struct lagg_port *lp_ptr;
 	struct lagg_llq *llq;
 	struct ifnet *ifp = lp->lp_ifp;
+	uint64_t *pval, vdiff;
+	int i;
 
 	LAGG_WLOCK_ASSERT(sc);
 
@@ -899,6 +882,13 @@ lagg_port_destroy(struct lagg_port *lp, 
 	ifp->if_output = lp->lp_output;
 	ifp->if_lagg = NULL;
 
+	/* Update detached port counters */
+	pval = lp->port_counters.val;
+	for (i = IFCOUNTER_IPACKETS; i <= IFCOUNTER_LAST; i++, pval++) {
+		vdiff = ifp->if_get_counter(ifp, i) - *pval;
+		sc->detached_counters.val[i - 1] += vdiff;
+	}
+
 	/* Finally, remove the port from the lagg */
 	SLIST_REMOVE(&sc->sc_ports, lp, lagg_port, lp_entries);
 	sc->sc_count--;
@@ -1012,6 +1002,61 @@ fallback:
 }
 
 /*
+ * Requests counter @cnt data. 
+ *
+ * Counter value is calculated the following way:
+ * 1) for each port, sum  difference between current and "initial" measurements.
+ * 2) add lagg logical interface counters.
+ * 3) add data from detached_counters array.
+ *
+ * We also do the following things on ports attach/detach:
+ * 1) On port attach we store all counters it has into port_counter array. 
+ * 2) On port detach we add the different between "initial" and
+ *   current counters data to detached_counters array.
+ */
+static uint64_t
+lagg_get_counter(struct ifnet *ifp, ift_counter cnt)
+{
+	struct lagg_softc *sc;
+	struct lagg_port *lp;
+	struct ifnet *lpifp;
+	struct rm_priotracker tracker;
+	uint64_t newval, oldval, vsum;
+
+	if (cnt <= 0 || cnt > IFCOUNTER_LAST)
+		return (if_get_counter_default(ifp, cnt));
+
+	sc = (struct lagg_softc *)ifp->if_softc;
+	LAGG_RLOCK(sc, &tracker);
+
+	vsum = 0;
+	SLIST_FOREACH(lp, &sc->sc_ports, lp_entries) {
+		/* Saved attached value */
+		oldval = lp->port_counters.val[cnt - 1];
+		/* current value */
+		lpifp = lp->lp_ifp;
+		newval = lpifp->if_get_counter(lpifp, cnt);
+		/* Calculate diff and save new */
+		vsum += newval - oldval;
+	}
+
+	/*
+	 * Add counter data which might be added by upper
+	 * layer protocols operating on logical interface.
+	 */
+	vsum += if_get_counter_default(ifp, cnt);
+
+	/*
+	 * Add counter data from detached ports counters
+	 */
+	vsum += sc->detached_counters.val[cnt - 1];
+
+	LAGG_RUNLOCK(sc, &tracker);
+
+	return (vsum);
+}
+
+/*
  * For direct output to child ports.
  */
 static int
@@ -1449,11 +1494,7 @@ lagg_transmit(struct ifnet *ifp, struct 
 	error = lagg_proto_start(sc, m);
 	LAGG_RUNLOCK(sc, &tracker);
 
-	if (error == 0) {
-		counter_u64_add(sc->sc_opackets, 1);
-		counter_u64_add(sc->sc_obytes, len);
-		ifp->if_omcasts += mcast;
-	} else
+	if (error != 0)
 		ifp->if_oerrors++;
 
 	return (error);
@@ -1489,9 +1530,6 @@ lagg_input(struct ifnet *ifp, struct mbu
 	m = lagg_proto_input(sc, lp, m);
 
 	if (m != NULL) {
-		counter_u64_add(sc->sc_ipackets, 1);
-		counter_u64_add(sc->sc_ibytes, m->m_pkthdr.len);
-
 		if (scifp->if_flags & IFF_MONITOR) {
 			m_freem(m);
 			m = NULL;
@@ -2124,16 +2162,3 @@ lagg_lacp_input(struct lagg_softc *sc, s
 	return (m);
 }
 
-static void
-lagg_callout(void *arg)
-{
-	struct lagg_softc *sc = (struct lagg_softc *)arg;
-	struct ifnet *ifp = sc->sc_ifp;
-
-	ifp->if_ipackets = counter_u64_fetch(sc->sc_ipackets);
-	ifp->if_opackets = counter_u64_fetch(sc->sc_opackets);
-	ifp->if_ibytes = counter_u64_fetch(sc->sc_ibytes);
-	ifp->if_obytes = counter_u64_fetch(sc->sc_obytes);
-
-	callout_reset(&sc->sc_callout, hz, lagg_callout, sc);
-}

Modified: head/sys/net/if_lagg.h
==============================================================================
--- head/sys/net/if_lagg.h	Sat Sep 27 10:57:34 2014	(r272210)
+++ head/sys/net/if_lagg.h	Sat Sep 27 13:57:48 2014	(r272211)
@@ -140,8 +140,6 @@ struct lagg_reqflags {
 
 #ifdef _KERNEL
 
-#include <sys/counter.h>
-
 /*
  * Internal kernel part
  */
@@ -187,10 +185,13 @@ struct lagg_llq {
 	SLIST_ENTRY(lagg_llq)	llq_entries;
 };
 
+struct lagg_counters {
+	uint64_t	val[IFCOUNTER_LAST];
+};
+
 struct lagg_softc {
 	struct ifnet			*sc_ifp;	/* virtual interface */
 	struct rmlock			sc_mtx;
-	struct mtx			sc_call_mtx;
 	int				sc_proto;	/* lagg protocol */
 	u_int				sc_count;	/* number of ports */
 	u_int				sc_active;	/* active port count */
@@ -202,11 +203,6 @@ struct lagg_softc {
 	uint32_t			sc_seq;		/* sequence counter */
 	uint32_t			sc_flags;
 
-	counter_u64_t			sc_ipackets;
-	counter_u64_t			sc_opackets;
-	counter_u64_t			sc_ibytes;
-	counter_u64_t			sc_obytes;
-
 	SLIST_HEAD(__tplhd, lagg_port)	sc_ports;	/* list of interfaces */
 	SLIST_ENTRY(lagg_softc)	sc_entries;
 
@@ -220,6 +216,7 @@ struct lagg_softc {
 	struct sysctl_oid		*sc_oid;	/* sysctl tree oid */
 	int				use_flowid;	/* use M_FLOWID */
 	int				flowid_shift;	/* shift the flowid */
+	struct lagg_counters		detached_counters; /* detached ports sum */
 };
 
 struct lagg_port {
@@ -241,6 +238,7 @@ struct lagg_port {
 	int	(*lp_ioctl)(struct ifnet *, u_long, caddr_t);
 	int	(*lp_output)(struct ifnet *, struct mbuf *,
 		     const struct sockaddr *, struct route *);
+	struct lagg_counters		port_counters;	/* ifp counters copy */
 
 	SLIST_ENTRY(lagg_port)		lp_entries;
 };
@@ -254,11 +252,6 @@ struct lagg_port {
 #define	LAGG_RLOCK_ASSERT(_sc)	rm_assert(&(_sc)->sc_mtx, RA_RLOCKED)
 #define	LAGG_WLOCK_ASSERT(_sc)	rm_assert(&(_sc)->sc_mtx, RA_WLOCKED)
 
-#define	LAGG_CALLOUT_LOCK_INIT(_sc)					\
-	    mtx_init(&(_sc)->sc_call_mtx, "if_lagg callout mutex", NULL,\
-	    MTX_DEF)
-#define	LAGG_CALLOUT_LOCK_DESTROY(_sc)	mtx_destroy(&(_sc)->sc_call_mtx)
-
 extern struct mbuf *(*lagg_input_p)(struct ifnet *, struct mbuf *);
 extern void	(*lagg_linkstate_p)(struct ifnet *, int );
 

Modified: head/sys/net/if_var.h
==============================================================================
--- head/sys/net/if_var.h	Sat Sep 27 10:57:34 2014	(r272210)
+++ head/sys/net/if_var.h	Sat Sep 27 13:57:48 2014	(r272211)
@@ -109,6 +109,7 @@ typedef enum {
 	IFCOUNTER_OQDROPS,
 	IFCOUNTER_NOPROTO,
 } ift_counter;
+#define	IFCOUNTER_LAST	IFCOUNTER_NOPROTO
 
 typedef struct ifnet * if_t;
 



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