[openib-general] [PATCH] Get rid of /proc/infiniband/ipoib_vlan

Roland Dreier roland at topspin.com
Mon Nov 15 20:43:06 PST 2004


This kills off /proc/infiniband/ipoib_vlan in favor of a simpler sysfs
interface.  To create ib0.8001, you can now just do

    # echo 0x8001 > /sys/class/net/ib0/create_child 

and to get rid of the interface,

    # echo 0x8001 > /sys/class/net/ib0/delete_child

(Better names for these files gladly accepted)

To see a child interface's parent (in case interfaces have been
renamed to something nonobvious):

    # cat /sys/class/net/ib0.8001/parent
    ib0

and to check the P_Key of an interface:

    # cat /sys/class/net/ib0/pkey
    0xffff

 - Roland

Index: infiniband/ulp/ipoib/ipoib_vlan.c
===================================================================
--- infiniband/ulp/ipoib/ipoib_vlan.c	(revision 1239)
+++ infiniband/ulp/ipoib/ipoib_vlan.c	(working copy)
@@ -32,43 +32,62 @@
 
 #include "ipoib.h"
 
-struct ipoib_vlan_iter {
-	struct list_head *pintf_cur;
-	struct list_head *intf_cur;
-};
+/*
+ * We use this mutex to serialize child interface creation.  This
+ * closes the race where userspace might create the same child
+ * interface twice at exactly the same time.
+ */
+static DECLARE_MUTEX(vlan_mutex);
 
-static DECLARE_MUTEX(proc_mutex);
+static ssize_t show_parent(struct class_device *class_dev, char *buf)
+{
+	struct net_device *dev =
+		container_of(class_dev, struct net_device, class_dev);
+	struct ipoib_dev_priv *priv = netdev_priv(dev);
 
-int ipoib_vlan_add(struct net_device *pdev, char *intf_name,
-		   unsigned short pkey)
+	return sprintf(buf, "%s\n", priv->parent->name);
+}
+static CLASS_DEVICE_ATTR(parent, S_IRUGO, show_parent, NULL);
+
+int ipoib_vlan_add(struct net_device *pdev, unsigned short pkey)
 {
 	struct ipoib_dev_priv *ppriv, *priv;
-	int result = -ENOMEM;
+	char intf_name[IFNAMSIZ];
+	int result;
 
 	if (!capable(CAP_NET_ADMIN))
 		return -EPERM;
 
+	down(&vlan_mutex);
+
 	ppriv = netdev_priv(pdev);
 
 	/*
 	 * First ensure this isn't a duplicate. We check the parent device and
 	 * then all of the child interfaces to make sure the Pkey doesn't match.
 	 */
-	if (ppriv->pkey == pkey)
-		return -ENOTUNIQ;
+	if (ppriv->pkey == pkey) {
+		result = -ENOTUNIQ;
+		goto err;
+	}
 
 	down(&ipoib_device_mutex);
 	list_for_each_entry(priv, &ppriv->child_intfs, list) {
 		if (priv->pkey == pkey) {
 			up(&ipoib_device_mutex);
-			return -ENOTUNIQ;
+			result = -ENOTUNIQ;
+			goto err;
 		}
 	}
 	up(&ipoib_device_mutex);
 
+	snprintf(intf_name, sizeof intf_name, "%s.%04x",
+		 ppriv->dev->name, pkey);
 	priv = ipoib_intf_alloc(intf_name);
-	if (!priv)
-		goto alloc_mem_failed;
+	if (!priv) {
+		result = -ENOMEM;
+		goto err;
+	}
 
 	set_bit(IPOIB_FLAG_SUBINTERFACE, &priv->flags);
 
@@ -92,19 +111,33 @@
 		goto register_failed;
 	}
 
+	priv->parent = ppriv->dev;
+
+	if (ipoib_add_pkey_attr(priv->dev))
+		goto sysfs_failed;
+
+	if (class_device_create_file(&priv->dev->class_dev,
+				     &class_device_attr_parent))
+		goto sysfs_failed;
+
 	down(&ipoib_device_mutex);
 	list_add_tail(&priv->list, &ppriv->child_intfs);
 	up(&ipoib_device_mutex);
+	up(&vlan_mutex);
 
 	return 0;
 
+sysfs_failed:
+	unregister_netdev(priv->dev);
+
 register_failed:
 	ipoib_dev_cleanup(priv->dev);
 
 device_init_failed:
 	free_netdev(priv->dev);
 
-alloc_mem_failed:
+err:
+	up(&vlan_mutex);
 	return result;
 }
 
@@ -120,13 +153,8 @@
 	down(&ipoib_device_mutex);
 	list_for_each_entry_safe(priv, tpriv, &ppriv->child_intfs, list) {
 		if (priv->pkey == pkey) {
-			if (priv->dev->flags & IFF_UP) {
-				up(&ipoib_device_mutex);
-				return -EBUSY;
-			}
-
-			ipoib_dev_cleanup(priv->dev);
 			unregister_netdev(priv->dev);
+			ipoib_dev_cleanup(priv->dev);
 
 			list_del(&priv->list);
 
@@ -140,219 +168,3 @@
 
 	return -ENOENT;
 }
-
-/* =============================================================== */
-/*..ipoib_vlan_iter_next -- incr. iter. -- return non-zero at end  */
-int ipoib_vlan_iter_next(struct ipoib_vlan_iter *iter)
-{
-	while (1) {
-		struct ipoib_dev_priv *priv;
-
-		priv = list_entry(iter->pintf_cur, struct ipoib_dev_priv, list);
-		if (!iter->intf_cur)
-			iter->intf_cur = priv->child_intfs.next;
-		else
-			iter->intf_cur = iter->intf_cur->next;
-
-		if (iter->intf_cur == &priv->child_intfs) {
-			iter->pintf_cur = iter->pintf_cur->next;
-			if (iter->pintf_cur == &ipoib_device_list)
-				return 1;
-
-			iter->intf_cur = NULL;
-			return 0;
-		} else
-			return 0;
-	}
-}
-
-/* =============================================================== */
-/*.._ipoib_vlan_seq_start -- seq file handling                     */
-static void *_ipoib_vlan_seq_start(struct seq_file *file, loff_t *pos)
-{
-	struct ipoib_vlan_iter *iter;
-	loff_t n = *pos;
-
-	iter = kmalloc(sizeof(*iter), GFP_KERNEL);
-	if (!iter)
-		return NULL;
-
-	iter->pintf_cur = ipoib_device_list.next;
-	iter->intf_cur = NULL;
-
-	while (n--) {
-		if (ipoib_vlan_iter_next(iter)) {
-			kfree(iter);
-			return NULL;
-		}
-	}
-
-	return iter;
-}
-
-/* =============================================================== */
-/*.._ipoib_vlan_seq_next -- seq file handling                      */
-static void *_ipoib_vlan_seq_next(struct seq_file *file, void *iter_ptr,
-				  loff_t *pos)
-{
-	struct ipoib_vlan_iter *iter = iter_ptr;
-
-	(*pos)++;
-
-	if (ipoib_vlan_iter_next(iter)) {
-		kfree(iter);
-		return NULL;
-	}
-
-	return iter;
-}
-
-/* =============================================================== */
-/*.._ipoib_vlan_seq_stop -- seq file handling                      */
-static void _ipoib_vlan_seq_stop(struct seq_file *file, void *iter_ptr)
-{
-	struct ipoib_vlan_iter *iter = iter_ptr;
-
-	kfree(iter);
-}
-
-/* =============================================================== */
-/*.._ipoib_vlan_seq_show -- seq file handling                      */
-static int _ipoib_vlan_seq_show(struct seq_file *file, void *iter_ptr)
-{
-	struct ipoib_vlan_iter *iter = iter_ptr;
-
-	if (iter) {
-		struct ipoib_dev_priv *ppriv;
-
-		ppriv = list_entry(iter->pintf_cur, struct ipoib_dev_priv, list);
-
-		if (!iter->intf_cur)
-			seq_printf(file, "%s 0x%04x\n", ppriv->dev->name,
-				   ppriv->pkey);
-		else {
-			struct ipoib_dev_priv *priv;
-
-			priv = list_entry(iter->intf_cur, struct ipoib_dev_priv,
-				          list);
-
-			seq_printf(file, " %s %s 0x%04x\n", ppriv->dev->name,
-				   priv->dev->name, priv->pkey);
-		}
-	}
-
-	return 0;
-}
-
-static struct seq_operations ipoib_vlan_seq_operations = {
-	.start = _ipoib_vlan_seq_start,
-	.next = _ipoib_vlan_seq_next,
-	.stop = _ipoib_vlan_seq_stop,
-	.show = _ipoib_vlan_seq_show,
-};
-
-/* =============================================================== */
-/*.._ipoib_vlan_proc_open -- proc file handling                    */
-static int _ipoib_vlan_proc_open(struct inode *inode, struct file *file)
-{
-	if (down_interruptible(&proc_mutex))
-		return -ERESTARTSYS;
-
-	return seq_open(file, &ipoib_vlan_seq_operations);
-}
-
-/* =============================================================== */
-/*.._ipoib_vlan_proc_write -- proc file handling                   */
-static ssize_t _ipoib_vlan_proc_write(struct file *file,
-				      const char __user *buffer,
-				      size_t count, loff_t *pos)
-{
-	int result;
-	char kernel_buf[256];
-	char intf_parent[128], intf_name[128];
-	unsigned int pkey;
-	struct net_device *pdev;
-
-	count = min(count, sizeof(kernel_buf));
-
-	if (copy_from_user(kernel_buf, buffer, count))
-		return -EFAULT;
-
-	kernel_buf[count - 1] = '\0';
-
-	if (sscanf(kernel_buf, "add %128s %128s %i", intf_parent, intf_name,
-		   &pkey) == 3) {
-		if (pkey > 0xffff)
-			return -EINVAL;
-
-		pdev = dev_get_by_name(intf_parent);
-		if (!pdev)
-			return -ENOENT;
-
-		result = ipoib_vlan_add(pdev, intf_name, pkey);
-
-		dev_put(pdev);
-
-		if (result < 0)
-			return result;
-	} else if (sscanf(kernel_buf, "del %128s %i", intf_parent,
-			  &pkey) == 2) {
-		if (pkey > 0xffff)
-			return -EINVAL;
-
-		pdev = dev_get_by_name(intf_parent);
-		if (!pdev)
-			return -ENOENT;
-
-		result = ipoib_vlan_delete(pdev, pkey);
-
-		dev_put(pdev);
-
-		if (result < 0)
-			return result;
-	} else
-		return -EINVAL;
-
-	return count;
-}
-
-/* =============================================================== */
-/*.._ipoib_vlan_proc_release -- proc file handling                 */
-static int _ipoib_vlan_proc_release(struct inode *inode, struct file *file)
-{
-	up(&proc_mutex);
-
-	return seq_release(inode, file);
-}
-
-static struct file_operations ipoib_vlan_proc_operations = {
-	.owner 	 = THIS_MODULE,
-	.open 	 = _ipoib_vlan_proc_open,
-	.read 	 = seq_read,
-	.write 	 = _ipoib_vlan_proc_write,
-	.llseek  = seq_lseek,
-	.release = _ipoib_vlan_proc_release,
-};
-
-struct proc_dir_entry *vlan_proc_entry;
-
-int ipoib_vlan_init(void)
-{
-	vlan_proc_entry = create_proc_entry("ipoib_vlan",
-					    S_IRUGO | S_IWUGO, ipoib_proc_dir);
-
-	if (!vlan_proc_entry) {
-		printk(KERN_WARNING "Can't create ipoib_vlan in /proc\n");
-		return -ENOMEM;
-	}
-
-	vlan_proc_entry->proc_fops = &ipoib_vlan_proc_operations;
-
-	return 0;
-}
-
-void ipoib_vlan_cleanup(void)
-{
-	if (vlan_proc_entry)
-		remove_proc_entry("ipoib_vlan", ipoib_proc_dir);
-}
Index: infiniband/ulp/ipoib/ipoib_main.c
===================================================================
--- infiniband/ulp/ipoib/ipoib_main.c	(revision 1239)
+++ infiniband/ulp/ipoib/ipoib_main.c	(working copy)
@@ -609,7 +609,6 @@
 void ipoib_dev_cleanup(struct net_device *dev)
 {
 	struct ipoib_dev_priv *priv = netdev_priv(dev), *cpriv, *tcpriv;
-	int i;
 
 	/* Delete any child interfaces first */
 	/* Safe since it's either protected by ipoib_device_mutex or empty */
@@ -626,19 +625,11 @@
 	ipoib_ib_dev_cleanup(dev);
 
 	if (priv->rx_ring) {
-		for (i = 0; i < IPOIB_RX_RING_SIZE; ++i)
-			if (priv->rx_ring[i].skb)
-				dev_kfree_skb_any(priv->rx_ring[i].skb);
-
 		kfree(priv->rx_ring);
 		priv->rx_ring = NULL;
 	}
 
 	if (priv->tx_ring) {
-		for (i = 0; i < IPOIB_TX_RING_SIZE; ++i)
-			if (priv->tx_ring[i].skb)
-				dev_kfree_skb_any(priv->tx_ring[i].skb);
-
 		kfree(priv->tx_ring);
 		priv->tx_ring = NULL;
 	}
@@ -714,6 +705,60 @@
 	return netdev_priv(dev);
 }
 
+static ssize_t show_pkey(struct class_device *cdev, char *buf)
+{
+	struct ipoib_dev_priv *priv =
+		netdev_priv(container_of(cdev, struct net_device, class_dev));
+
+	return sprintf(buf, "0x%04x\n", priv->pkey);
+}
+static CLASS_DEVICE_ATTR(pkey, S_IRUGO, show_pkey, NULL);
+
+static ssize_t create_child(struct class_device *cdev,
+			    const char *buf, size_t count)
+{
+	int pkey;
+	int ret;
+
+	if (sscanf(buf, "%i", &pkey) != 1)
+		return -EINVAL;
+
+	if (pkey < 0 || pkey > 0xffff)
+		return -EINVAL;
+
+	ret = ipoib_vlan_add(container_of(cdev, struct net_device, class_dev),
+			     pkey);
+
+	return ret ? ret : count;
+}
+static CLASS_DEVICE_ATTR(create_child, S_IWUGO, NULL, create_child);
+
+static ssize_t delete_child(struct class_device *cdev,
+			    const char *buf, size_t count)
+{
+	int pkey;
+	int ret;
+
+	if (sscanf(buf, "%i", &pkey) != 1)
+		return -EINVAL;
+
+	if (pkey < 0 || pkey > 0xffff)
+		return -EINVAL;
+
+	ret = ipoib_vlan_delete(container_of(cdev, struct net_device, class_dev),
+				pkey);
+
+	return ret ? ret : count;
+
+}
+static CLASS_DEVICE_ATTR(delete_child, S_IWUGO, NULL, delete_child);
+
+int ipoib_add_pkey_attr(struct net_device *dev)
+{
+	return class_device_create_file(&dev->class_dev,
+					&class_device_attr_pkey);
+}
+
 static int ipoib_add_port(const char *format, struct ib_device *hca, u8 port)
 {
 	struct ipoib_dev_priv *priv;
@@ -771,6 +816,15 @@
 	if (ipoib_proc_dev_init(priv->dev))
 		goto proc_failed;
 
+	if (ipoib_add_pkey_attr(priv->dev))
+		goto proc_failed;
+	if (class_device_create_file(&priv->dev->class_dev,
+				     &class_device_attr_create_child))
+		goto proc_failed;
+	if (class_device_create_file(&priv->dev->class_dev,
+				     &class_device_attr_delete_child))
+		goto proc_failed;
+
 	down(&ipoib_device_mutex);
 	list_add_tail(&priv->list, &ipoib_device_list);
 	up(&ipoib_device_mutex);
@@ -860,8 +914,6 @@
 	if (ret)
 		goto err_wq;
 
-	ipoib_vlan_init();
-
 	return 0;
 
 err_wq:
@@ -875,7 +927,6 @@
 
 static void __exit ipoib_cleanup_module(void)
 {
-	ipoib_vlan_cleanup();
 	ib_unregister_client(&ipoib_client);
 	remove_proc_entry("infiniband", NULL);
 	destroy_workqueue(ipoib_workqueue);
Index: infiniband/ulp/ipoib/ipoib.h
===================================================================
--- infiniband/ulp/ipoib/ipoib.h	(revision 1239)
+++ infiniband/ulp/ipoib/ipoib.h	(working copy)
@@ -124,7 +124,6 @@
 
 	union ib_gid local_gid;
 	u16          local_lid;
-	u32          local_qpn;
 
 	unsigned int admin_mtu;
 	unsigned int mcast_mtu;
@@ -145,6 +144,7 @@
 
 	struct net_device_stats stats;
 
+	struct net_device *parent;
 	struct list_head child_intfs;
 	struct list_head list;
 };
@@ -186,6 +186,8 @@
 	kref_put(&ah->ref, ipoib_free_ah);
 }
 
+int ipoib_add_pkey_attr(struct net_device *dev);
+
 void ipoib_send(struct net_device *dev, struct sk_buff *skb,
 		struct ipoib_ah *address, u32 qpn);
 void ipoib_reap_ah(void *dev_ptr);
@@ -240,8 +242,8 @@
 void ipoib_event(struct ib_event_handler *handler,
 		 struct ib_event *record);
 
-int ipoib_vlan_init(void);
-void ipoib_vlan_cleanup(void);
+int ipoib_vlan_add(struct net_device *pdev, unsigned short pkey);
+int ipoib_vlan_delete(struct net_device *pdev, unsigned short pkey);
 
 void ipoib_pkey_poll(void *dev);
 int ipoib_pkey_dev_delay_open(struct net_device *dev);
Index: infiniband/ulp/ipoib/ipoib_ib.c
===================================================================
--- infiniband/ulp/ipoib/ipoib_ib.c	(revision 1239)
+++ infiniband/ulp/ipoib/ipoib_ib.c	(working copy)
@@ -229,7 +229,6 @@
 		priv->stats.tx_bytes += tx_req->skb->len;
 
 		dev_kfree_skb_any(tx_req->skb);
-		tx_req->skb = NULL;
 
 		spin_lock_irqsave(&priv->lock, flags);
 		++priv->tx_tail;
@@ -336,7 +335,6 @@
 		      address->ah, qpn, addr, skb->len)) {
 		ipoib_warn(priv, "post_send failed\n");
 		++priv->stats.tx_errors;
-		tx_req->skb = NULL;
 		dev_kfree_skb_any(skb);
 	} else {
 		unsigned long flags;
@@ -485,6 +483,8 @@
 	while (priv->tx_head != priv->tx_tail || recvs_pending(dev))
 		yield();
 
+	ipoib_dbg(priv, "All sends and receives done.\n");
+
 	qp_attr.qp_state = IB_QPS_RESET;
 	attr_mask        = IB_QP_STATE;
 	if (ib_modify_qp(priv->qp, &qp_attr, attr_mask))
@@ -499,12 +499,9 @@
 		yield();
 	}
 
-	for (i = 0; i < IPOIB_RX_RING_SIZE; ++i) {
-		if (priv->rx_ring[i].skb) {
-			dev_kfree_skb_any(priv->rx_ring[i].skb);
-			priv->rx_ring[i].skb = NULL;
-		}
-	}
+	for (i = 0; i < IPOIB_RX_RING_SIZE; ++i)
+		if (priv->rx_ring[i].skb)
+			ipoib_warn(priv, "Recv skb still around @ %d\n", i);
 
 	return 0;
 }



More information about the general mailing list