IB/ipoib: Consolidate checking of the proposed child interface

Move all the checking for pkey and other validity to the __ipoib_vlan_add
function. This removes the last difference from the control flow
of the __ipoib_vlan_add to make the overall design simpler to
understand.

Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>
Signed-off-by: Erez Shitrit <erezsh@mellanox.com>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>

diff --git a/drivers/infiniband/ulp/ipoib/ipoib_netlink.c b/drivers/infiniband/ulp/ipoib/ipoib_netlink.c
index 7e093b7aad8f5a126cf4c32424a24569268c16aa..d4d553a51fa98373802be96196c5f6fc6a574e7f 100644 (file)
--- a/drivers/infiniband/ulp/ipoib/ipoib_netlink.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_netlink.c
@@ -122,9 +122,6 @@ static int ipoib_new_child_link(struct net *src_net, struct net_device *dev,
        } else
                child_pkey  = nla_get_u16(data[IFLA_IPOIB_PKEY]);
 
-       if (child_pkey == 0 || child_pkey == 0x8000)
-               return -EINVAL;
-
        err = __ipoib_vlan_add(ppriv, ipoib_priv(dev),
                               child_pkey, IPOIB_RTNL_CHILD);
 

diff --git a/drivers/infiniband/ulp/ipoib/ipoib_vlan.c b/drivers/infiniband/ulp/ipoib/ipoib_vlan.c
index ca3a7f6c0998c73c976c8e077c66fb2f4c38e089..341753fbda54dc1da2ec06bcc00eeb7895875046 100644 (file)
--- a/drivers/infiniband/ulp/ipoib/ipoib_vlan.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_vlan.c
@@ -50,6 +50,39 @@ static ssize_t show_parent(struct device *d, struct device_attribute *attr,
 }
 static DEVICE_ATTR(parent, S_IRUGO, show_parent, NULL);
 
+static bool is_child_unique(struct ipoib_dev_priv *ppriv,
+                           struct ipoib_dev_priv *priv)
+{
+       struct ipoib_dev_priv *tpriv;
+
+       ASSERT_RTNL();
+
+       /*
+        * Since the legacy sysfs interface uses pkey for deletion it cannot
+        * support more than one interface with the same pkey, it creates
+        * ambiguity.  The RTNL interface deletes using the netdev so it does
+        * not have a problem to support duplicated pkeys.
+        */
+       if (priv->child_type != IPOIB_LEGACY_CHILD)
+               return true;
+
+       /*
+        * First ensure this isn't a duplicate. We check the parent device and
+        * then all of the legacy child interfaces to make sure the Pkey
+        * doesn't match.
+        */
+       if (ppriv->pkey == priv->pkey)
+               return false;
+
+       list_for_each_entry(tpriv, &ppriv->child_intfs, list) {
+               if (tpriv->pkey == priv->pkey &&
+                   tpriv->child_type == IPOIB_LEGACY_CHILD)
+                       return false;
+       }
+
+       return true;
+}
+
 /*
  * NOTE: If this function fails then the priv->dev will remain valid, however
  * priv can have been freed and must not be touched by caller in the error
@@ -73,10 +106,20 @@ int __ipoib_vlan_add(struct ipoib_dev_priv *ppriv, struct ipoib_dev_priv *priv,
         */
        WARN_ON(ppriv->dev->reg_state != NETREG_REGISTERED);
 
+       if (pkey == 0 || pkey == 0x8000) {
+               result = -EINVAL;
+               goto out_early;
+       }
+
        priv->parent = ppriv->dev;
        priv->pkey = pkey;
        priv->child_type = type;
 
+       if (!is_child_unique(ppriv, priv)) {
+               result = -ENOTUNIQ;
+               goto out_early;
+       }
+
        /* We do not need to touch priv if register_netdevice fails */
        ndev->priv_destructor = ipoib_intf_free;
 
@@ -88,9 +131,7 @@ int __ipoib_vlan_add(struct ipoib_dev_priv *ppriv, struct ipoib_dev_priv *priv,
                 * register_netdevice sometimes calls priv_destructor,
                 * sometimes not. Make sure it was done.
                 */
-               if (ndev->priv_destructor)
-                       ndev->priv_destructor(ndev);
-               return result;
+               goto out_early;
        }
 
        /* RTNL childs don't need proprietary sysfs entries */
@@ -111,6 +152,11 @@ int __ipoib_vlan_add(struct ipoib_dev_priv *ppriv, struct ipoib_dev_priv *priv,
 sysfs_failed:
        unregister_netdevice(priv->dev);
        return -ENOMEM;
+
+out_early:
+       if (ndev->priv_destructor)
+               ndev->priv_destructor(ndev);
+       return result;
 }
 
 int ipoib_vlan_add(struct net_device *pdev, unsigned short pkey)
@@ -118,17 +164,11 @@ int ipoib_vlan_add(struct net_device *pdev, unsigned short pkey)
        struct ipoib_dev_priv *ppriv, *priv;
        char intf_name[IFNAMSIZ];
        struct net_device *ndev;
-       struct ipoib_dev_priv *tpriv;
        int result;
 
        if (!capable(CAP_NET_ADMIN))
                return -EPERM;
 
-       ppriv = ipoib_priv(pdev);
-
-       snprintf(intf_name, sizeof(intf_name), "%s.%04x",
-                ppriv->dev->name, pkey);
-
        if (!rtnl_trylock())
                return restart_syscall();
 
@@ -137,23 +177,10 @@ int ipoib_vlan_add(struct net_device *pdev, unsigned short pkey)
                return -EPERM;
        }
 
-       /*
-        * First ensure this isn't a duplicate. We check the parent device and
-        * then all of the legacy child interfaces to make sure the Pkey
-        * doesn't match.
-        */
-       if (ppriv->pkey == pkey) {
-               result = -ENOTUNIQ;
-               goto out;
-       }
+       ppriv = ipoib_priv(pdev);
 
-       list_for_each_entry(tpriv, &ppriv->child_intfs, list) {
-               if (tpriv->pkey == pkey &&
-                   tpriv->child_type == IPOIB_LEGACY_CHILD) {
-                       result = -ENOTUNIQ;
-                       goto out;
-               }
-       }
+       snprintf(intf_name, sizeof(intf_name), "%s.%04x",
+                ppriv->dev->name, pkey);
 
        priv = ipoib_intf_alloc(ppriv->ca, ppriv->port, intf_name);
        if (!priv) {