Reduce the NETLINK pointer complexity

Make the NETLINK connection pointer and user counter
local global variables inside netlink.c only.  Where NETLINK
calls via libnl is required, rather use get_nlc() to get a
NETLINK connection.

This also prepares the next step, to get rid of the
struct etherinfo_obj_data wrapper.

Signed-off-by: David Sommerseth <davids@redhat.com>

Author: David Sommerseth

python-ethtool/etherinfo.c

@@ -142,7 +142,7 @@ static void callback_nl_address(struct nl_object *obj, void *arg)
  *
  * @return Returns 1 on success, otherwise 0.
  */
-int get_etherinfo(struct etherinfo_obj_data *data, nlQuery query)
+int get_etherinfo(etherinfo_py *self, nlQuery query)
 {
 	struct nl_cache *link_cache;
 	struct nl_cache *addr_cache;
@@ -153,13 +153,13 @@ int get_etherinfo(struct etherinfo_obj_data *data, nlQuery query)
 
 	int ret = 0;
 
-	if( !data || !data->ethinfo ) {
+	if( !self || !self->data || !self->data->ethinfo ) {
 		return 0;
 	}
-	ethinf = data->ethinfo;
+	ethinf = self->data->ethinfo;
 
 	/* Open a NETLINK connection on-the-fly */
-	if( !open_netlink(data) ) {
+	if( !open_netlink(self) ) {
 		PyErr_Format(PyExc_RuntimeError,
 			     "Could not open a NETLINK connection for %s",
 			     ethinf->device);
@@ -171,7 +171,7 @@ int get_etherinfo(struct etherinfo_obj_data *data, nlQuery query)
 	 * interface index if we have that
 	 */
 	if( ethinf->index < 0 ) {
-		if( rtnl_link_alloc_cache(*data->nlc, AF_UNSPEC, &link_cache) < 0) {
+		if( rtnl_link_alloc_cache(get_nlc(), AF_UNSPEC, &link_cache) < 0) {
                         return 0;
                 }
 
@@ -195,7 +195,7 @@ int get_etherinfo(struct etherinfo_obj_data *data, nlQuery query)
 	switch( query ) {
 	case NLQRY_LINK:
 		/* Extract MAC/hardware address of the interface */
-		if( rtnl_link_alloc_cache(*data->nlc, AF_UNSPEC, &link_cache) < 0) {
+		if( rtnl_link_alloc_cache(get_nlc(), AF_UNSPEC, &link_cache) < 0) {
                         return 0;
                 }
 		link = rtnl_link_alloc();
@@ -209,7 +209,7 @@ int get_etherinfo(struct etherinfo_obj_data *data, nlQuery query)
         case NLQRY_ADDR4:
         case NLQRY_ADDR6:
 		/* Extract IP address information */
-		if( rtnl_addr_alloc_cache(*data->nlc, &addr_cache) < 0) {
+		if( rtnl_addr_alloc_cache(get_nlc(), &addr_cache) < 0) {
 			nl_cache_free(addr_cache);
                         return 0;
                 }

python-ethtool/etherinfo.h

@@ -19,10 +19,11 @@
 
 typedef enum {NLQRY_LINK, NLQRY_ADDR4, NLQRY_ADDR6} nlQuery; /**<  Supported query types in the etherinfo code */
 
-int get_etherinfo(struct etherinfo_obj_data *data, nlQuery query);
+int get_etherinfo(etherinfo_py *data, nlQuery query);
 void free_etherinfo(struct etherinfo *ptr);
 
-int open_netlink(struct etherinfo_obj_data *);
-void close_netlink(struct etherinfo_obj_data *);
+int open_netlink(etherinfo_py *);
+struct nl_sock * get_nlc();
+void close_netlink(etherinfo_py *);
 
 #endif

python-ethtool/etherinfo_obj.c

@@ -41,7 +41,7 @@ extern PyTypeObject ethtool_etherinfoIPv6Type;
 void _ethtool_etherinfo_dealloc(etherinfo_py *self)
 {
 	if( self->data ) {
-		close_netlink(self->data);
+		close_netlink(self);
 
 		if( self->data->ethinfo ) {
 			free_etherinfo(self->data->ethinfo);
@@ -151,11 +151,11 @@ PyObject *_ethtool_etherinfo_getter(etherinfo_py *self, PyObject *attr_o)
                         return Py_INCREF(Py_None), Py_None;
                 }
 	} else if( strcmp(attr, "mac_address") == 0 ) {
-		get_etherinfo(self->data, NLQRY_LINK);
+		get_etherinfo(self, NLQRY_LINK);
                 Py_INCREF(self->data->ethinfo->hwaddress);
 		return self->data->ethinfo->hwaddress;
 	} else if( strcmp(attr, "ipv4_address") == 0 ) {
-		get_etherinfo(self->data, NLQRY_ADDR4);
+		get_etherinfo(self, NLQRY_ADDR4);
 		/* For compatiblity with old approach, return last IPv4 address: */
 		py_addr = get_last_ipv4_address(self);
 		if (py_addr) {
@@ -166,14 +166,14 @@ PyObject *_ethtool_etherinfo_getter(etherinfo_py *self, PyObject *attr_o)
 		}
 		Py_RETURN_NONE;
 	} else if( strcmp(attr, "ipv4_netmask") == 0 ) {
-		get_etherinfo(self->data, NLQRY_ADDR4);
+		get_etherinfo(self, NLQRY_ADDR4);
 		py_addr = get_last_ipv4_address(self);
 		if (py_addr) {
 		  return PyInt_FromLong(py_addr->prefixlen);
 		}
 		return PyInt_FromLong(0);
 	} else if( strcmp(attr, "ipv4_broadcast") == 0 ) {
-		get_etherinfo(self->data, NLQRY_ADDR4);
+		get_etherinfo(self, NLQRY_ADDR4);
 		py_addr = get_last_ipv4_address(self);
 		if (py_addr) {
 		  if (py_addr->ipv4_broadcast) {
@@ -215,14 +215,14 @@ PyObject *_ethtool_etherinfo_str(etherinfo_py *self)
 {
 	PyObject *ret = NULL;
 
-	if( !self || !self->data || !self->data->nlc || !self->data->ethinfo ) {
+	if( !self || !self->data || !self->data->ethinfo ) {
 		PyErr_SetString(PyExc_AttributeError, "No data available");
 		return NULL;
 	}
 
-	get_etherinfo(self->data, NLQRY_LINK);
-	get_etherinfo(self->data, NLQRY_ADDR4);
-	get_etherinfo(self->data, NLQRY_ADDR6);
+	get_etherinfo(self, NLQRY_LINK);
+	get_etherinfo(self, NLQRY_ADDR4);
+	get_etherinfo(self, NLQRY_ADDR6);
 
 	ret = PyString_FromFormat("Device %s:\n", self->data->ethinfo->device);
 	if( self->data->ethinfo->hwaddress ) {
@@ -281,7 +281,7 @@ static PyObject *_ethtool_etherinfo_get_ipv4_addresses(etherinfo_py *self, PyObj
 		return NULL;
 	}
 
-	get_etherinfo(self->data, NLQRY_ADDR4);
+	get_etherinfo(self, NLQRY_ADDR4);
 
 	/* Transfer ownership of reference: */
 	ret = self->data->ethinfo->ipv4_addresses;
@@ -307,7 +307,7 @@ static PyObject *_ethtool_etherinfo_get_ipv6_addresses(etherinfo_py *self, PyObj
 		return NULL;
 	}
 
-	get_etherinfo(self->data, NLQRY_ADDR6);
+	get_etherinfo(self, NLQRY_ADDR6);
 
 	/* Transfer ownership of reference: */
 	ret = self->data->ethinfo->ipv6_addresses;

python-ethtool/etherinfo_struct.h

@@ -56,9 +56,6 @@ extern PyTypeObject ethtool_netlink_ip_address_Type;
  *
  */
 struct etherinfo_obj_data {
-	struct nl_sock **nlc;           /**< Contains NETLINK connection info (global) */
-	unsigned int *nlc_users;	/**< Resource counter for the NETLINK connection (global) */
-	unsigned short nlc_active;	/**< Is this instance using NETLINK? */
 	struct etherinfo *ethinfo;      /**< Contains info about our current interface */
 };
 
@@ -69,6 +66,7 @@ struct etherinfo_obj_data {
 typedef struct {
 	PyObject_HEAD
 	struct etherinfo_obj_data *data; /* IPv4 and IPv6 address information, only one element used */
+	unsigned short nlc_active;	/**< Is this instance using NETLINK? */
 } etherinfo_py;
 
 

python-ethtool/ethtool.c

@@ -32,8 +32,6 @@
 #include "etherinfo_obj.h"
 #include "etherinfo.h"
 
-static struct nl_sock *nlconnection = NULL;
-unsigned int nlconnection_users = 0;  /* How many NETLINK users are active? */
 extern PyTypeObject ethtool_etherinfoType;
 
 #ifndef IFF_DYNAMIC
@@ -292,8 +290,6 @@ static PyObject *get_interfaces_info(PyObject *self __unused, PyObject *args) {
 		 */
 		objdata->ethinfo->device = strdup(fetch_devs[i]);
 		objdata->ethinfo->index = -1;
-		objdata->nlc = &nlconnection;
-		objdata->nlc_users = &nlconnection_users;
 
 		/* Instantiate a new etherinfo object with the device information */
 		ethinf_py = PyCObject_FromVoidPtr(objdata, NULL);

python-ethtool/netlink.c

@@ -24,85 +24,98 @@
 #include "etherinfo_struct.h"
 
 pthread_mutex_t nlc_counter_mtx = PTHREAD_MUTEX_INITIALIZER;
+static struct nl_sock *nlconnection = NULL;
+static unsigned int nlconnection_users = 0;  /* How many NETLINK users are active? */
 
 
 /**
  * Connects to the NETLINK interface.  This will be called
  * for each etherinfo object being generated, and it will
  * keep a separate file descriptor open for each object
  *
- * @param data etherinfo_obj_data structure
+ * @param ethi etherinfo_py structure (basically the "self" object)
  *
  * @return Returns 1 on success, otherwise 0.
  */
-int open_netlink(struct etherinfo_obj_data *data)
+int open_netlink(etherinfo_py *ethi)
 {
-	if( !data ) {
+	if( !ethi ) {
 		return 0;
 	}
 
 	/* Reuse already established NETLINK connection, if a connection exists */
-	if( *data->nlc ) {
+	if( nlconnection ) {
 		/* If this object has not used NETLINK earlier, tag it as a user */
-		if( !data->nlc_active ) {
+		if( !ethi->nlc_active ) {
 			pthread_mutex_lock(&nlc_counter_mtx);
-			(*data->nlc_users)++;
+			nlconnection_users++;
 			pthread_mutex_unlock(&nlc_counter_mtx);
 		}
-		data->nlc_active = 1;
+		ethi->nlc_active = 1;
 		return 1;
 	}
 
 	/* No earlier connections exists, establish a new one */
-	*data->nlc = nl_socket_alloc();
-	nl_connect(*data->nlc, NETLINK_ROUTE);
-	if( (*data->nlc != NULL) ) {
+	nlconnection = nl_socket_alloc();
+	nl_connect(nlconnection, NETLINK_ROUTE);
+	if( (nlconnection != NULL) ) {
 		/* Force O_CLOEXEC flag on the NETLINK socket */
-		if( fcntl(nl_socket_get_fd(*data->nlc), F_SETFD, FD_CLOEXEC) == -1 ) {
+		if( fcntl(nl_socket_get_fd(nlconnection), F_SETFD, FD_CLOEXEC) == -1 ) {
 			fprintf(stderr,
 				"**WARNING** Failed to set O_CLOEXEC on NETLINK socket: %s\n",
 				strerror(errno));
 		}
 
 		/* Tag this object as an active user */
 		pthread_mutex_lock(&nlc_counter_mtx);
-		(*data->nlc_users)++;
+		nlconnection_users++;
 		pthread_mutex_unlock(&nlc_counter_mtx);
-		data->nlc_active = 1;
+		ethi->nlc_active = 1;
 		return 1;
 	} else {
 		return 0;
 	}
 }
 
 
+/**
+ * Return a reference to the global netlink connection
+ *
+ * @returns Returns a pointer to a NETLINK connection libnl functions can use
+ */
+struct nl_sock * get_nlc()
+{
+	assert(nlconnection);
+	return nlconnection;
+}
+
 /**
  * Closes the NETLINK connection.  This should be called automatically whenever
  * the corresponding etherinfo object is deleted.
  *
- * @param ptr  Pointer to the pointer of struct nl_handle, which contains the NETLINK connection
+ * @param ethi etherinfo_py structure (basically the "self" object)
  */
-void close_netlink(struct etherinfo_obj_data *data)
+void close_netlink(etherinfo_py *ethi)
 {
-	if( !data || !(*data->nlc) ) {
+	if( !ethi || !nlconnection ) {
 		return;
 	}
 
 	/* Untag this object as a NETLINK user */
-	data->nlc_active = 0;
+	ethi->nlc_active = 0;
 	pthread_mutex_lock(&nlc_counter_mtx);
-	(*data->nlc_users)--;
+	nlconnection_users--;
 	pthread_mutex_unlock(&nlc_counter_mtx);
 
 	/* Don't close the connection if there are more users */
-	if( *data->nlc_users > 0) {
+	if( nlconnection_users > 0) {
 		return;
 	}
 
 	/* Close NETLINK connection */
-	nl_close(*data->nlc);
-	nl_socket_free(*data->nlc);
-	*data->nlc = NULL;
+	nl_close(nlconnection);
+	nl_socket_free(nlconnection);
+	nlconnection = NULL;
 }
 
 /*