Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 31 Aug 2014 06:46:22 +0000 (UTC)
From:      Gleb Smirnoff <glebius@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r270870 - head/sys/net
Message-ID:  <201408310646.s7V6kM1X050678@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: glebius
Date: Sun Aug 31 06:46:21 2014
New Revision: 270870
URL: http://svnweb.freebsd.org/changeset/base/270870

Log:
  o Remove struct if_data from struct ifnet. Now it is merely API structure
    for route(4) socket and ifmib(4) sysctl.
  o Move fields from if_data to ifnet, but keep all statistic counters
    separate, since they should disappear later.
  o Provide function if_data_copy() to fill if_data, utilize it in routing
    socket and ifmib handler.
  o Provide overridable ifnet(9) method to fetch counters. If no provided,
    if_get_counters_compat() would be used, that returns old counters.
  
  Sponsored by:	Netflix
  Sponsored by:	Nginx, Inc.

Modified:
  head/sys/net/if.c
  head/sys/net/if_mib.c
  head/sys/net/if_var.h
  head/sys/net/rtsock.c

Modified: head/sys/net/if.c
==============================================================================
--- head/sys/net/if.c	Sun Aug 31 06:30:50 2014	(r270869)
+++ head/sys/net/if.c	Sun Aug 31 06:46:21 2014	(r270870)
@@ -605,8 +605,7 @@ if_attach_internal(struct ifnet *ifp, in
 	if_addgroup(ifp, IFG_ALL);
 
 	getmicrotime(&ifp->if_lastchange);
-	ifp->if_data.ifi_epoch = time_uptime;
-	ifp->if_data.ifi_datalen = sizeof(struct if_data);
+	ifp->if_epoch = time_uptime;
 
 	KASSERT((ifp->if_transmit == NULL && ifp->if_qflush == NULL) ||
 	    (ifp->if_transmit != NULL && ifp->if_qflush != NULL),
@@ -615,7 +614,10 @@ if_attach_internal(struct ifnet *ifp, in
 		ifp->if_transmit = if_transmit;
 		ifp->if_qflush = if_qflush;
 	}
-	
+
+	if (ifp->if_get_counter == NULL)
+		ifp->if_get_counter = if_get_counter_compat;
+
 	if (!vmove) {
 #ifdef MAC
 		mac_ifnet_create(ifp);
@@ -1384,6 +1386,77 @@ if_rtdel(struct radix_node *rn, void *ar
 }
 
 /*
+ * Return counter values from old racy non-pcpu counters.
+ */
+uint64_t
+if_get_counter_compat(struct ifnet *ifp, ifnet_counter cnt)
+{
+
+	switch (cnt) {
+		case IFCOUNTER_IPACKETS:
+			return (ifp->if_ipackets);
+		case IFCOUNTER_IERRORS:
+			return (ifp->if_ierrors);
+		case IFCOUNTER_OPACKETS:
+			return (ifp->if_opackets);
+		case IFCOUNTER_OERRORS:
+			return (ifp->if_oerrors);
+		case IFCOUNTER_COLLISIONS:
+			return (ifp->if_collisions);
+		case IFCOUNTER_IBYTES:
+			return (ifp->if_ibytes);
+		case IFCOUNTER_OBYTES:
+			return (ifp->if_obytes);
+		case IFCOUNTER_IMCASTS:
+			return (ifp->if_imcasts);
+		case IFCOUNTER_OMCASTS:
+			return (ifp->if_omcasts);
+		case IFCOUNTER_IQDROPS:
+			return (ifp->if_iqdrops);
+		case IFCOUNTER_OQDROPS:
+			return (ifp->if_oqdrops);
+		case IFCOUNTER_NOPROTO:
+			return (ifp->if_noproto);
+	}
+	panic("%s: unknown counter %d", __func__, cnt);
+}
+
+/*
+ * Copy data from ifnet to userland API structure if_data.
+ */
+void
+if_data_copy(struct ifnet *ifp, struct if_data *ifd)
+{
+
+	ifd->ifi_type = ifp->if_type;
+	ifd->ifi_physical = 0;
+	ifd->ifi_addrlen = ifp->if_addrlen;
+	ifd->ifi_hdrlen = ifp->if_hdrlen;
+	ifd->ifi_link_state = ifp->if_link_state;
+	ifd->ifi_vhid = 0;
+	ifd->ifi_datalen = sizeof(struct if_data);
+	ifd->ifi_mtu = ifp->if_mtu;
+	ifd->ifi_metric = ifp->if_metric;
+	ifd->ifi_baudrate = ifp->if_baudrate;
+	ifd->ifi_hwassist = ifp->if_hwassist;
+	ifd->ifi_epoch = ifp->if_epoch;
+	ifd->ifi_lastchange = ifp->if_lastchange;
+
+	ifd->ifi_ipackets = ifp->if_get_counter(ifp, IFCOUNTER_IPACKETS);
+	ifd->ifi_ierrors = ifp->if_get_counter(ifp, IFCOUNTER_IERRORS);
+	ifd->ifi_opackets = ifp->if_get_counter(ifp, IFCOUNTER_OPACKETS);
+	ifd->ifi_oerrors = ifp->if_get_counter(ifp, IFCOUNTER_OERRORS);
+	ifd->ifi_collisions = ifp->if_get_counter(ifp, IFCOUNTER_COLLISIONS);
+	ifd->ifi_ibytes = ifp->if_get_counter(ifp, IFCOUNTER_IBYTES);
+	ifd->ifi_obytes = ifp->if_get_counter(ifp, IFCOUNTER_OBYTES);
+	ifd->ifi_imcasts = ifp->if_get_counter(ifp, IFCOUNTER_IMCASTS);
+	ifd->ifi_omcasts = ifp->if_get_counter(ifp, IFCOUNTER_OMCASTS);
+	ifd->ifi_iqdrops = ifp->if_get_counter(ifp, IFCOUNTER_IQDROPS);
+	ifd->ifi_oqdrops = ifp->if_get_counter(ifp, IFCOUNTER_OQDROPS);
+	ifd->ifi_noproto = ifp->if_get_counter(ifp, IFCOUNTER_NOPROTO);
+}
+
+/*
  * Wrapper functions for struct ifnet address list locking macros.  These are
  * used by kernel modules to avoid encoding programming interface or binary
  * interface assumptions that may be violated when kernel-internal locking
@@ -2167,7 +2240,8 @@ ifhwioctl(u_long cmd, struct ifnet *ifp,
 		break;
 
 	case SIOCGIFPHYS:
-		ifr->ifr_phys = ifp->if_physical;
+		/* XXXGL: did this ever worked? */
+		ifr->ifr_phys = 0;
 		break;
 
 	case SIOCGIFDESCR:
@@ -3915,7 +3989,7 @@ if_sendq_prepend(if_t ifp, struct mbuf *
 int
 if_setifheaderlen(if_t ifp, int len)
 {
-	((struct ifnet *)ifp)->if_data.ifi_hdrlen = len;
+	((struct ifnet *)ifp)->if_hdrlen = len;
 	return (0);
 }
 

Modified: head/sys/net/if_mib.c
==============================================================================
--- head/sys/net/if_mib.c	Sun Aug 31 06:30:50 2014	(r270869)
+++ head/sys/net/if_mib.c	Sun Aug 31 06:46:21 2014	(r270870)
@@ -99,10 +99,9 @@ sysctl_ifdata(SYSCTL_HANDLER_ARGS) /* XX
 		bzero(&ifmd, sizeof(ifmd));
 		strlcpy(ifmd.ifmd_name, ifp->if_xname, sizeof(ifmd.ifmd_name));
 
-#define COPY(fld) ifmd.ifmd_##fld = ifp->if_##fld
-		COPY(pcount);
-		COPY(data);
-#undef COPY
+		ifmd.ifmd_pcount = ifp->if_pcount;
+		if_data_copy(ifp, &ifmd.ifmd_data);
+
 		ifmd.ifmd_flags = ifp->if_flags | ifp->if_drv_flags;
 		ifmd.ifmd_snd_len = ifp->if_snd.ifq_len;
 		ifmd.ifmd_snd_maxlen = ifp->if_snd.ifq_maxlen;

Modified: head/sys/net/if_var.h
==============================================================================
--- head/sys/net/if_var.h	Sun Aug 31 06:30:50 2014	(r270869)
+++ head/sys/net/if_var.h	Sun Aug 31 06:46:21 2014	(r270870)
@@ -94,11 +94,27 @@ VNET_DECLARE(struct pfil_head, link_pfil
 #define	V_link_pfil_hook	VNET(link_pfil_hook)
 #endif /* _KERNEL */
 
+typedef enum {
+	IFCOUNTER_IPACKETS = 1,
+	IFCOUNTER_IERRORS,
+	IFCOUNTER_OPACKETS,
+	IFCOUNTER_OERRORS,
+	IFCOUNTER_COLLISIONS,
+	IFCOUNTER_IBYTES,
+	IFCOUNTER_OBYTES,
+	IFCOUNTER_IMCASTS,
+	IFCOUNTER_OMCASTS,
+	IFCOUNTER_IQDROPS,
+	IFCOUNTER_OQDROPS,
+	IFCOUNTER_NOPROTO,
+} ifnet_counter;
+
 typedef	void (*if_start_fn_t)(struct ifnet *);
 typedef	int (*if_ioctl_fn_t)(struct ifnet *, u_long, caddr_t);
 typedef	void (*if_init_fn_t)(void *);
 typedef void (*if_qflush_fn_t)(struct ifnet *);
 typedef int (*if_transmit_fn_t)(struct ifnet *, struct mbuf *);
+typedef	uint64_t (*if_get_counter_t)(struct ifnet *, ifnet_counter);
 
 /* Opaque object pointing to interface structure (ifnet) */
 typedef void *if_t;
@@ -136,8 +152,21 @@ struct ifnet {
 	size_t	if_linkmiblen;		/* length of above data */
 	int	if_drv_flags;		/* driver-managed status flags */
 	u_int	if_refcount;		/* reference count */
+
+	/* These fields are shared with struct if_data. */
+	uint8_t		if_type;	/* ethernet, tokenring, etc */
+	uint8_t		if_addrlen;	/* media address length */
+	uint8_t		if_hdrlen;	/* media header length */
+	uint8_t		if_link_state;	/* current link state */
+	uint32_t	if_spare32;
+	uint32_t	if_mtu;		/* maximum transmission unit */
+	uint32_t	if_metric;	/* routing metric (external only) */
+	uint64_t	if_baudrate;	/* linespeed */
+	uint64_t	if_hwassist;	/* HW offload capabilities, see IFCAP */
+	time_t		if_epoch;	/* uptime at attach or stat reset */
+	struct timeval	if_lastchange;	/* time of last administrative change */
+
 	struct  ifaltq if_snd;		/* output queue (includes altq) */
-	struct	if_data if_data;	/* type information and statistics */
 	struct	task if_linktask;	/* task for link change events */
 
 	/* Addresses of different protocol families assigned to this if. */
@@ -190,6 +219,7 @@ struct ifnet {
 
 	void	(*if_reassign)		/* reassign to vnet routine */
 		(struct ifnet *, struct vnet *, char *);
+	if_get_counter_t if_get_counter; /* get counter values */
 
 	/* Stuff that's only temporary and doesn't belong here. */
 	u_int	if_hw_tsomax;		/* tso burst length limit, the minimum
@@ -197,45 +227,31 @@ struct ifnet {
 					 * XXXAO: Have to find a better place
 					 * for it eventually. */
 	/*
-	 * Spare fields are added so that we can modify sensitive data
-	 * structures without changing the kernel binary interface, and must
-	 * be used with care where binary compatibility is required.
+	 * Old, racy and expensive statistics, should not be used in
+	 * new drivers.
+	 */
+	uint64_t	if_ipackets;	/* packets received on interface */
+	uint64_t	if_ierrors;	/* input errors on interface */
+	uint64_t	if_opackets;	/* packets sent on interface */
+	uint64_t	if_oerrors;	/* output errors on interface */
+	uint64_t	if_collisions;	/* collisions on csma interfaces */
+	uint64_t	if_ibytes;	/* total number of octets received */
+	uint64_t	if_obytes;	/* total number of octets sent */
+	uint64_t	if_imcasts;	/* packets received via multicast */
+	uint64_t	if_omcasts;	/* packets sent via multicast */
+	uint64_t	if_iqdrops;	/* dropped on input */
+	uint64_t	if_oqdrops;	/* dropped on output */
+	uint64_t	if_noproto;	/* destined for unsupported protocol */
+
+	/*
+	 * Spare fields to be added before branching a stable branch, so
+	 * that structure can be enhanced without changing the kernel
+	 * binary interface.
 	 */
-	char	if_cspare[3];
-	int	if_ispare[4];
-	void	*if_unused[2];
-	void	*if_pspare[8];		/* 1 netmap, 7 TDB */
 };
 
 #include <net/ifq.h>	/* XXXAO: temporary unconditional include */
 
-/*
- * XXX These aliases are terribly dangerous because they could apply
- * to anything.
- */
-#define	if_mtu		if_data.ifi_mtu
-#define	if_type		if_data.ifi_type
-#define if_physical	if_data.ifi_physical
-#define	if_addrlen	if_data.ifi_addrlen
-#define	if_hdrlen	if_data.ifi_hdrlen
-#define	if_metric	if_data.ifi_metric
-#define	if_link_state	if_data.ifi_link_state
-#define	if_baudrate	if_data.ifi_baudrate
-#define	if_hwassist	if_data.ifi_hwassist
-#define	if_ipackets	if_data.ifi_ipackets
-#define	if_ierrors	if_data.ifi_ierrors
-#define	if_opackets	if_data.ifi_opackets
-#define	if_oerrors	if_data.ifi_oerrors
-#define	if_collisions	if_data.ifi_collisions
-#define	if_ibytes	if_data.ifi_ibytes
-#define	if_obytes	if_data.ifi_obytes
-#define	if_imcasts	if_data.ifi_imcasts
-#define	if_omcasts	if_data.ifi_omcasts
-#define	if_iqdrops	if_data.ifi_iqdrops
-#define	if_oqdrops	if_data.ifi_oqdrops
-#define	if_noproto	if_data.ifi_noproto
-#define	if_lastchange	if_data.ifi_lastchange
-
 /* for compatibility with other BSDs */
 #define	if_addrlist	if_addrhead
 #define	if_list		if_link
@@ -513,6 +529,8 @@ typedef	void *if_com_alloc_t(u_char type
 typedef	void if_com_free_t(void *com, u_char type);
 void	if_register_com_alloc(u_char type, if_com_alloc_t *a, if_com_free_t *f);
 void	if_deregister_com_alloc(u_char type);
+void	if_data_copy(struct ifnet *, struct if_data *);
+uint64_t if_get_counter_compat(struct ifnet *, ifnet_counter);
 
 #define IF_LLADDR(ifp)							\
     LLADDR((struct sockaddr_dl *)((ifp)->if_addr->ifa_addr))

Modified: head/sys/net/rtsock.c
==============================================================================
--- head/sys/net/rtsock.c	Sun Aug 31 06:30:50 2014	(r270869)
+++ head/sys/net/rtsock.c	Sun Aug 31 06:46:21 2014	(r270870)
@@ -1252,7 +1252,7 @@ rt_ifmsg(struct ifnet *ifp)
 	ifm = mtod(m, struct if_msghdr *);
 	ifm->ifm_index = ifp->if_index;
 	ifm->ifm_flags = ifp->if_flags | ifp->if_drv_flags;
-	ifm->ifm_data = ifp->if_data;
+	if_data_copy(ifp, &ifm->ifm_data);
 	ifm->ifm_addrs = 0;
 	rt_dispatch(m, AF_UNSPEC);
 }
@@ -1574,7 +1574,7 @@ sysctl_iflist_ifml(struct ifnet *ifp, st
 		ifd = &ifm->ifm_data;
 	}
 
-	*ifd = ifp->if_data;
+	if_data_copy(ifp, ifd);
 
 	/* Some drivers still use ifqueue(9), add its stats. */
 	ifd->ifi_oqdrops += ifp->if_snd.ifq_drops;
@@ -1609,7 +1609,7 @@ sysctl_iflist_ifm(struct ifnet *ifp, str
 		ifd = &ifm->ifm_data;
 	}
 
-	*ifd = ifp->if_data;
+	if_data_copy(ifp, ifd);
 
 	/* Some drivers still use ifqueue(9), add its stats. */
 	ifd->ifi_oqdrops += ifp->if_snd.ifq_drops;



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