Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 6 Apr 2015 15:22:33 +0000 (UTC)
From:      Gleb Smirnoff <glebius@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-projects@freebsd.org
Subject:   svn commit: r281151 - projects/ifnet/sys/net
Message-ID:  <201504061522.t36FMXwX092973@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: glebius
Date: Mon Apr  6 15:22:32 2015
New Revision: 281151
URL: https://svnweb.freebsd.org/changeset/base/281151

Log:
  Provide software context store in struct ifnet for any facility. To add
  a new facility, one needs to grab a value in the ift_feature enum, and
  not need to modify struct ifnet.
  
  The store also has a cache to return frequently requested values,
  which resembles kobj(9) method cache.
  
  The plan is to move into the softc store almost all possible software
  contexts that hang off the struct ifnet, leaving static only the driver
  softc, AF_INET, AF_INET6 pointers, and other frequently used pointer.
  
  Sponsored by:	Nginx, Inc.
  Sponsored by:	Netflix

Modified:
  projects/ifnet/sys/net/if.c
  projects/ifnet/sys/net/if.h
  projects/ifnet/sys/net/if_var.h

Modified: projects/ifnet/sys/net/if.c
==============================================================================
--- projects/ifnet/sys/net/if.c	Mon Apr  6 15:10:54 2015	(r281150)
+++ projects/ifnet/sys/net/if.c	Mon Apr  6 15:22:32 2015	(r281151)
@@ -538,6 +538,9 @@ if_attach(struct if_attach_args *ifat)
 	}
 
 	ifp = malloc(sizeof(struct ifnet), M_IFNET, M_WAITOK | M_ZERO);
+	ifp->if_scstore = malloc(sizeof(struct ifsoftc) * SOFTC_CACHE_SIZE,
+	    M_IFNET, M_WAITOK | M_ZERO);
+	ifp->if_nsoftcs = SOFTC_CACHE_SIZE;
 	for (int i = 0; i < IFCOUNTERS; i++)
 		ifp->if_counters[i] = counter_u64_alloc(M_WAITOK);
 #ifdef MAC
@@ -1541,7 +1544,13 @@ if_rtdel(struct radix_node *rn, void *ar
 void *
 if_getsoftc(struct ifnet *ifp, ift_feature f)
 {
+	struct ifsoftc *sc;
 
+	/*
+	 * Some softcs are non-optional either for performance reasons,
+	 * since they always exist and are often dereferenced, or for
+	 * historical reasons.
+	 */
 	switch (f) {
 	case IF_DRIVER_SOFTC:
 		return (ifp->if_softc);
@@ -1554,8 +1563,97 @@ if_getsoftc(struct ifnet *ifp, ift_featu
 	case IF_VLAN:
 		return (ifp->if_vlantrunk);
 	default:
-		panic("%s: unknown feature %d", __func__, f);
+		/* fall through */
+		;
 	};
+
+	/*
+	 * Rest of softc live in the store and in the cache.
+	 * First check the cache.
+	 */
+	sc = ifp->if_sccache[f & (SOFTC_CACHE_SIZE - 1)];
+	if (sc != NULL && sc->ifsc_desc == f)
+		return (sc->ifsc_ptr);
+
+	/*
+	 * Then check the store.
+	 * We can do lookup lockless, since if_nsoftcs only grows.
+	 */
+	for (int i = 0; i < ifp->if_nsoftcs; i++) {
+		sc = &ifp->if_scstore[i];
+		if (sc->ifsc_desc == f) {
+			ifp->if_sccache[f & (SOFTC_CACHE_SIZE - 1)] = sc;
+			return (sc->ifsc_ptr);
+		}
+	}
+
+	/*
+	 * XXXGL: a negative cache would be not bad.
+	 */
+	return (NULL);
+}
+
+/*
+ * Set arbitrary context identified by ift_feature key.  It is responsibility
+ * of the caller to establish race safety against two if_setsoftc()s.  The
+ * function may sleep when setting new context.  The function will not sleep
+ * when clearing previously set context.  May fail only if associated context
+ * is already set.
+ */
+int
+if_setsoftc(struct ifnet *ifp, ift_feature f, void *softc)
+{
+	int i;
+
+	IF_WLOCK(ifp);
+retry:
+	for (i = 0; i < ifp->if_nsoftcs; i++)
+		if (ifp->if_scstore[i].ifsc_desc == f) {
+			IF_WUNLOCK(ifp);
+			return (EEXIST);
+		}
+
+	for (i = 0; i < ifp->if_nsoftcs; i++)
+		if (ifp->if_scstore[i].ifsc_desc == 0)
+			break;
+
+	if (i == ifp->if_nsoftcs) {
+		struct ifsoftc *new, *old;
+		u_int size;
+
+		old = ifp->if_scstore;
+		size = ifp->if_nsoftcs;
+		IF_WUNLOCK(ifp);
+		new = malloc(sizeof(struct ifsoftc) * size * 2,
+		    M_IFNET, M_WAITOK | M_ZERO);
+		IF_WLOCK(ifp);
+		if (ifp->if_scstore != old) {
+			free(new, M_IFNET);
+			goto retry;
+		}
+		bcopy(ifp->if_scstore, new, sizeof(struct ifsoftc) * size);
+		ifp->if_scstore = new;
+		ifp->if_nsoftcs = size * 2;
+		/*
+		 * XXXGL: of course there is a race here against if_getsoftc(),
+		 * which runs lockless.  We lack RCU or lightweight reference
+		 * counting.
+		 */
+		free(old, M_IFNET);
+	}
+
+	if (softc != NULL) {
+		ifp->if_scstore[i].ifsc_ptr = softc;
+		ifp->if_scstore[i].ifsc_desc = f;
+		ifp->if_sccache[f & (SOFTC_CACHE_SIZE - 1)] =
+		    &ifp->if_scstore[i];
+	} else {
+		ifp->if_scstore[i].ifsc_desc = 0;
+		ifp->if_scstore[i].ifsc_ptr = NULL;
+		ifp->if_sccache[f & (SOFTC_CACHE_SIZE - 1)] = NULL;
+	}
+	IF_WUNLOCK(ifp);
+	return (0);
 }
 
 /*

Modified: projects/ifnet/sys/net/if.h
==============================================================================
--- projects/ifnet/sys/net/if.h	Mon Apr  6 15:10:54 2015	(r281150)
+++ projects/ifnet/sys/net/if.h	Mon Apr  6 15:22:32 2015	(r281151)
@@ -572,11 +572,18 @@ typedef enum {
 } ift_counter;
 
 typedef enum {
-	IF_DRIVER_SOFTC,
+	IF_DRIVER_SOFTC = 0,
 	IF_LLADDR,
 	IF_BPF,
 	IF_NAME,
 	IF_VLAN,
+	/*
+	 * Values do matter, since we want to avoid aliasing of frequently
+	 * used features in if_softcs cache.
+	 */
+	IF_AF_INET = 8,
+	IF_AF_INET6 = 9,
+	IF_CARP = 10,
 } ift_feature;
 
 typedef struct ifnet * if_t;
@@ -709,6 +716,7 @@ void	if_inc_txcounters(if_t, struct mbuf
 void	if_setbaudrate(if_t, uint64_t);
 void	if_link_state_change(if_t, int);
 void *	if_getsoftc(if_t, ift_feature);
+int	if_setsoftc(if_t, ift_feature, void *);
 int	if_printf(if_t, const char *, ...) __printflike(2, 3);
 int	if_drvioctl(if_t, u_long, void *, struct thread *);
 uint64_t if_get_counter_default(if_t, ift_counter);

Modified: projects/ifnet/sys/net/if_var.h
==============================================================================
--- projects/ifnet/sys/net/if_var.h	Mon Apr  6 15:10:54 2015	(r281150)
+++ projects/ifnet/sys/net/if_var.h	Mon Apr  6 15:22:32 2015	(r281151)
@@ -80,33 +80,49 @@ struct iftype {
 };
 
 /*
+ * Many network stack modules want to store their software context associated
+ * with an interface.  We used to give a pointer for everyone, but that yield
+ * to sizeof(struct ifnet) growing and permanent need for new pointers added
+ * to the struct.  Now we keep a tiny cache of recently used features and
+ * dynamically allocated store for them.
+ * Note: this could be generalized with kobj(9).
+ */
+#define	SOFTC_CACHE_SIZE	4
+struct ifsoftc {
+	ift_feature     ifsc_desc;
+	void            *ifsc_ptr;
+};
+
+/*
  * Structure defining a network interface.
  *
  * (Would like to call this struct ``if'', but C isn't PL/1.)
  */
 struct ifnet {
 	struct ifops	*if_ops;	/* driver ops (or overridden) */
-	void		*if_softc;	/* driver soft state */
+	void		*if_softc;	/* driver software context */
 	struct ifdriver	*if_drv;	/* driver static definition */
-	struct iftype	*if_type;	/* if type static def (optional)*/
+	struct ifsoftc  *if_sccache[SOFTC_CACHE_SIZE];	/* cache of softcs */
 	struct iftsomax	*if_tsomax;	/* TSO limits */
+	struct iftype	*if_type;	/* if type static def (optional)*/
+
+	struct	rwlock	if_lock;	/* lock to protect the ifnet */
+
+	struct ifsoftc		*if_scstore;	/* store of different softcs */
+	TAILQ_ENTRY(ifnet)	if_link; 	/* on global list */
+	LIST_ENTRY(ifnet)	if_clones;	/* on if_cloner list */
+	TAILQ_HEAD(, ifg_list)	if_groups;	/* groups of this ifnet */
 
-	struct	rwlock if_lock;		/* lock to protect the ifnet */
-	
-	/* General book keeping of interface lists. */
-	TAILQ_ENTRY(ifnet) if_link; 	/* all struct ifnets are chained */
-	LIST_ENTRY(ifnet) if_clones;	/* interfaces of a cloner */
-	TAILQ_HEAD(, ifg_list) if_groups; /* linked list of groups per if */
-					/* protected by if_lock */
 	void	*if_llsoftc;		/* link layer softc */
 	void	*if_l2com;		/* pointer to protocol bits */
+	uint32_t if_nsoftcs;		/* elements in if_scstore */
 	int	if_dunit;		/* unit or IF_DUNIT_NONE */
 	u_short	if_index;		/* numeric abbreviation for this if  */
 	short	if_index_reserved;	/* spare space to grow if_index */
 	char	if_xname[IFNAMSIZ];	/* external name (name + unit) */
 	char	*if_description;	/* interface description */
 
-	/* Variable fields that are touched by the stack and drivers. */
+	/* Variable fields that are touched by the stack . */
 	uint32_t	if_flags;	/* up/down, broadcast, etc. */
 	uint32_t	if_capabilities;/* interface features & capabilities */
 	uint32_t	if_capenable;	/* enabled features & capabilities */



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