Index | Thread | Search

From:
Vitaliy Makkoveev <mvs@openbsd.org>
Subject:
Copy if_data stuff to ifnet structure
To:
Alexander Bluhm <bluhm@openbsd.org>, tech@openbsd.org
Date:
Mon, 20 Jan 2025 00:16:22 +0300

Download raw body.

Thread
'if_data' structure contains interface data like type or baudrate mixed 
with statistics counters. Each interface descriptor 'ifnet' contains the
instance of 'if_data' for deliver to the userland by request. It is not
clean which lock protects this data. Some old drivers rely on kernel
lock, some rely on net lock, ifioctl() relies on both. Moreover, network
interface could have per-cpu counters, but in such case both `if_data'
counters and per-cpu counters will be used by some pseudo drivers like
bridge(4). I want clean this mess.

This diff copies 'if_data' stuff into 'ifnet' descriptor to separate
interface counters from the rest data. I want to introduce the wrappers
around interface statistics like below and use it in the net/* layer, so
the non per-cpu counters represented as array and accessed in the
per-cpu counters style. This will be done later. 

static inline void
if_counters_inc(struct ifnet *ifp, unsigned int c)
{
	if (ifp->if_counters)
		counters_inc(...);
	else
		ifp->if_data_counters[c]++;
}

In the far future I want to start introduce the special mutex(9) to
protect this 'ifnet' side 'if_data' representation.

ok?

Index: sys/net/if.c
===================================================================
RCS file: /cvs/src/sys/net/if.c,v
retrieving revision 1.722
diff -u -p -r1.722 if.c
--- sys/net/if.c	16 Jan 2025 17:20:23 -0000	1.722
+++ sys/net/if.c	19 Jan 2025 20:19:56 -0000
@@ -2798,7 +2798,29 @@ if_getdata(struct ifnet *ifp, struct if_
 {
 	unsigned int i;
 
-	*data = ifp->if_data;
+	data->ifi_type = ifp->if_type;
+	data->ifi_addrlen = ifp->if_addrlen;
+	data->ifi_hdrlen = ifp->if_hdrlen;
+	data->ifi_link_state = ifp->if_link_state;
+	data->ifi_mtu = ifp->if_mtu;
+	data->ifi_metric = ifp->if_metric;
+	data->ifi_baudrate = ifp->if_baudrate;
+	data->ifi_capabilities = ifp->if_capabilities;
+	data->ifi_rdomain = ifp->if_rdomain;
+	data->ifi_lastchange = ifp->if_lastchange;
+
+	data->ifi_ipackets = ifp->if_data_counters[ifc_ipackets];
+	data->ifi_ierrors = ifp->if_data_counters[ifc_ierrors];
+	data->ifi_opackets = ifp->if_data_counters[ifc_opackets];
+	data->ifi_oerrors = ifp->if_data_counters[ifc_oerrors];
+	data->ifi_collisions = ifp->if_data_counters[ifc_collisions];
+	data->ifi_ibytes = ifp->if_data_counters[ifc_ibytes];
+	data->ifi_obytes = ifp->if_data_counters[ifc_obytes];
+	data->ifi_imcasts = ifp->if_data_counters[ifc_imcasts];
+	data->ifi_omcasts = ifp->if_data_counters[ifc_omcasts];
+	data->ifi_iqdrops = ifp->if_data_counters[ifc_iqdrops];
+	data->ifi_oqdrops = ifp->if_data_counters[ifc_oqdrops];
+	data->ifi_noproto = ifp->if_data_counters[ifc_noproto];
 
 	if (ifp->if_counters != NULL) {
 		uint64_t counters[ifc_ncounters];
Index: sys/net/if_var.h
===================================================================
RCS file: /cvs/src/sys/net/if_var.h,v
retrieving revision 1.133
diff -u -p -r1.133 if_var.h
--- sys/net/if_var.h	12 Oct 2024 23:18:10 -0000	1.133
+++ sys/net/if_var.h	19 Jan 2025 20:19:56 -0000
@@ -110,6 +110,23 @@ struct if_clone {
   .ifc_destroy	= destroy,						\
 }
 
+enum if_counters {
+	ifc_ipackets,		/* packets received on interface */
+	ifc_ierrors,		/* input errors on interface */
+	ifc_opackets,		/* packets sent on interface */
+	ifc_oerrors,		/* output errors on interface */
+	ifc_collisions,		/* collisions on csma interfaces */
+	ifc_ibytes,		/* total number of octets received */
+	ifc_obytes,		/* total number of octets sent */
+	ifc_imcasts,		/* packets received via multicast */
+	ifc_omcasts,		/* packets sent via multicast */
+	ifc_iqdrops,		/* dropped on input, this interface */
+	ifc_oqdrops,		/* dropped on output, this interface */
+	ifc_noproto,		/* destined for unsupported protocol */
+
+	ifc_ncounters
+};
+
 /*
  * Structure defining a queue for a network interface.
  *
@@ -147,7 +164,20 @@ struct ifnet {				/* and the entries */
 	short	if_timer;		/* time 'til if_watchdog called */
 	unsigned short if_flags;	/* [N] up/down, broadcast, etc. */
 	int	if_xflags;		/* [N] extra softnet flags */
-	struct	if_data if_data;	/* stats and other data about if */
+
+	/* Stats and other data about if. Should be in sync with if_data. */
+	u_char if_type;
+	u_char if_addrlen;
+	u_char if_hdrlen;
+	u_char if_link_state;
+	uint32_t if_mtu;
+	uint32_t if_metric;
+	uint64_t if_baudrate;
+	uint32_t if_capabilities;
+	uint32_t if_rdomain;
+	struct  timeval if_lastchange;	/* [c] last op. state change */
+	uint64_t if_data_counters[ifc_ncounters];
+
 	struct	cpumem *if_counters;	/* per cpu stats */
 	uint32_t if_hardmtu;		/* [d] maximum MTU device supports */
 	char	if_description[IFDESCRSIZE]; /* [c] interface description */
@@ -187,45 +217,19 @@ struct ifnet {				/* and the entries */
 
 	struct	nd_ifinfo *if_nd;	/* [I] IPv6 Neighbor Discovery info */
 };
-#define	if_mtu		if_data.ifi_mtu
-#define	if_type		if_data.ifi_type
-#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_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	/* [c] last op. state change */
-#define	if_capabilities	if_data.ifi_capabilities
-#define	if_rdomain	if_data.ifi_rdomain
 
-enum if_counters {
-	ifc_ipackets,		/* packets received on interface */
-	ifc_ierrors,		/* input errors on interface */
-	ifc_opackets,		/* packets sent on interface */
-	ifc_oerrors,		/* output errors on interface */
-	ifc_collisions,		/* collisions on csma interfaces */
-	ifc_ibytes,		/* total number of octets received */
-	ifc_obytes,		/* total number of octets sent */
-	ifc_imcasts,		/* packets received via multicast */
-	ifc_omcasts,		/* packets sent via multicast */
-	ifc_iqdrops,		/* dropped on input, this interface */
-	ifc_oqdrops,		/* dropped on output, this interface */
-	ifc_noproto,		/* destined for unsupported protocol */
-
-	ifc_ncounters
-};
+#define if_ipackets	if_data_counters[ifc_ipackets]
+#define if_ierrors	if_data_counters[ifc_ierrors]
+#define if_opackets	if_data_counters[ifc_opackets]
+#define if_oerrors	if_data_counters[ifc_oerrors]
+#define if_collisions	if_data_counters[ifc_collisions]
+#define if_ibytes	if_data_counters[ifc_ibytes]
+#define if_obytes	if_data_counters[ifc_obytes]
+#define if_imcasts	if_data_counters[ifc_imcasts]
+#define if_omcasts	if_data_counters[ifc_omcasts]
+#define if_iqdrops	if_data_counters[ifc_iqdrops]
+#define if_oqdrops	if_data_counters[ifc_oqdrops]
+#define if_noproto	if_data_counters[ifc_noproto]
 
 /*
  * The ifaddr structure contains information about one address