All of lore.kernel.org
 help / color / mirror / Atom feed
* [patch net-next-2.6] net: consolidate and fix ethtool_ops->get_settings calling
@ 2011-09-01 19:28 ` Jiri Pirko
  0 siblings, 0 replies; 25+ messages in thread
From: Jiri Pirko @ 2011-09-01 19:28 UTC (permalink / raw)
  To: netdev
  Cc: ralf, fubar, andy, kaber, bprakash, JBottomley, robert.w.love,
	davem, shemminger, decot, bhutchings, mirq-linux,
	alexander.h.duyck, amit.salecha, eric.dumazet, therbert, paulmck,
	laijs, xiaosuo, greearb, loke.chetan, linux-mips, linux-scsi,
	devel, bridge

This patch does several things:
- introduces __ethtool_get_settings which is called from ethtool code and
  from dev_ethtool_get_settings() as well.
- dev_ethtool_get_settings() becomes rtnl wrapper for
  __ethtool_get_settings()
- changes calling in drivers so rtnl locking is respected. In
  iboe_get_rate was previously ->get_settings() called unlocked. This
  fixes it
- introduces rtnl_lock in bnx2fc_vport_create() and fcoe_vport_create()
  so bnx2fc_if_create() and fcoe_if_create() are called locked as they
  are from other places.
- prb_calc_retire_blk_tmo() in af_packet.c was not calling get_settings
  with rtnl_lock. So use dev_ethtool_get_settings here.
- use __ethtool_get_settings() in bonding code

Signed-off-by: Jiri Pirko <jpirko@redhat.com>
---
 arch/mips/txx9/generic/setup_tx4939.c |    2 +-
 drivers/net/bonding/bond_main.c       |   13 ++++------
 drivers/net/macvlan.c                 |    3 +-
 drivers/scsi/bnx2fc/bnx2fc_fcoe.c     |    4 ++-
 drivers/scsi/fcoe/fcoe.c              |    4 ++-
 include/linux/ethtool.h               |    3 ++
 net/8021q/vlan_dev.c                  |    3 +-
 net/bridge/br_if.c                    |    2 +-
 net/core/dev.c                        |   17 +++++---------
 net/core/ethtool.c                    |   17 +++++++++----
 net/core/net-sysfs.c                  |    4 +-
 net/packet/af_packet.c                |   41 +++++++++++++++-----------------
 12 files changed, 59 insertions(+), 54 deletions(-)

diff --git a/arch/mips/txx9/generic/setup_tx4939.c b/arch/mips/txx9/generic/setup_tx4939.c
index e9f95dc..ba3cec3 100644
--- a/arch/mips/txx9/generic/setup_tx4939.c
+++ b/arch/mips/txx9/generic/setup_tx4939.c
@@ -321,7 +321,7 @@ void __init tx4939_sio_init(unsigned int sclk, unsigned int cts_mask)
 static u32 tx4939_get_eth_speed(struct net_device *dev)
 {
 	struct ethtool_cmd cmd;
-	if (dev_ethtool_get_settings(dev, &cmd))
+	if (__ethtool_get_settings(dev, &cmd))
 		return 100;	/* default 100Mbps */
 
 	return ethtool_cmd_speed(&cmd);
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 8cb75a6..1dcb07c 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -557,7 +557,7 @@ down:
 static int bond_update_speed_duplex(struct slave *slave)
 {
 	struct net_device *slave_dev = slave->dev;
-	struct ethtool_cmd etool = { .cmd = ETHTOOL_GSET };
+	struct ethtool_cmd ecmd;
 	u32 slave_speed;
 	int res;
 
@@ -565,18 +565,15 @@ static int bond_update_speed_duplex(struct slave *slave)
 	slave->speed = SPEED_100;
 	slave->duplex = DUPLEX_FULL;
 
-	if (!slave_dev->ethtool_ops || !slave_dev->ethtool_ops->get_settings)
-		return -1;
-
-	res = slave_dev->ethtool_ops->get_settings(slave_dev, &etool);
+	res = __ethtool_get_settings(slave_dev, &ecmd);
 	if (res < 0)
 		return -1;
 
-	slave_speed = ethtool_cmd_speed(&etool);
+	slave_speed = ethtool_cmd_speed(&ecmd);
 	if (slave_speed == 0 || slave_speed == ((__u32) -1))
 		return -1;
 
-	switch (etool.duplex) {
+	switch (ecmd.duplex) {
 	case DUPLEX_FULL:
 	case DUPLEX_HALF:
 		break;
@@ -585,7 +582,7 @@ static int bond_update_speed_duplex(struct slave *slave)
 	}
 
 	slave->speed = slave_speed;
-	slave->duplex = etool.duplex;
+	slave->duplex = ecmd.duplex;
 
 	return 0;
 }
diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index 836e13f..b100c90 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -543,7 +543,8 @@ static int macvlan_ethtool_get_settings(struct net_device *dev,
 					struct ethtool_cmd *cmd)
 {
 	const struct macvlan_dev *vlan = netdev_priv(dev);
-	return dev_ethtool_get_settings(vlan->lowerdev, cmd);
+
+	return __ethtool_get_settings(vlan->lowerdev, cmd);
 }
 
 static const struct ethtool_ops macvlan_ethtool_ops = {
diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
index 2c780a7..820a184 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
@@ -673,7 +673,7 @@ static void bnx2fc_link_speed_update(struct fc_lport *lport)
 	struct net_device *netdev = interface->netdev;
 	struct ethtool_cmd ecmd;
 
-	if (!dev_ethtool_get_settings(netdev, &ecmd)) {
+	if (!__ethtool_get_settings(netdev, &ecmd)) {
 		lport->link_supported_speeds &=
 			~(FC_PORTSPEED_1GBIT | FC_PORTSPEED_10GBIT);
 		if (ecmd.supported & (SUPPORTED_1000baseT_Half |
@@ -1001,9 +1001,11 @@ static int bnx2fc_vport_create(struct fc_vport *vport, bool disabled)
 			"this interface\n");
 		return -EIO;
 	}
+	rtnl_lock();
 	mutex_lock(&bnx2fc_dev_lock);
 	vn_port = bnx2fc_if_create(interface, &vport->dev, 1);
 	mutex_unlock(&bnx2fc_dev_lock);
+	rtnl_unlock();
 
 	if (IS_ERR(vn_port)) {
 		printk(KERN_ERR PFX "bnx2fc_vport_create (%s) failed\n",
diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index 3416ab6..83aa3ac 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -2043,7 +2043,7 @@ int fcoe_link_speed_update(struct fc_lport *lport)
 	struct net_device *netdev = fcoe_netdev(lport);
 	struct ethtool_cmd ecmd;
 
-	if (!dev_ethtool_get_settings(netdev, &ecmd)) {
+	if (!__ethtool_get_settings(netdev, &ecmd)) {
 		lport->link_supported_speeds &=
 			~(FC_PORTSPEED_1GBIT | FC_PORTSPEED_10GBIT);
 		if (ecmd.supported & (SUPPORTED_1000baseT_Half |
@@ -2452,7 +2452,9 @@ static int fcoe_vport_create(struct fc_vport *vport, bool disabled)
 	}
 
 	mutex_lock(&fcoe_config_mutex);
+	rtnl_lock();
 	vn_port = fcoe_if_create(fcoe, &vport->dev, 1);
+	rtnl_unlock();
 	mutex_unlock(&fcoe_config_mutex);
 
 	if (IS_ERR(vn_port)) {
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 3829712..8571f18 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -728,6 +728,9 @@ enum ethtool_sfeatures_retval_bits {
 /* needed by dev_disable_lro() */
 extern int __ethtool_set_flags(struct net_device *dev, u32 flags);
 
+extern int __ethtool_get_settings(struct net_device *dev,
+				  struct ethtool_cmd *cmd);
+
 /**
  * enum ethtool_phys_id_state - indicator state for physical identification
  * @ETHTOOL_ID_INACTIVE: Physical ID indicator should be deactivated
diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index eba705b..c8cf939 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -610,7 +610,8 @@ static int vlan_ethtool_get_settings(struct net_device *dev,
 				     struct ethtool_cmd *cmd)
 {
 	const struct vlan_dev_info *vlan = vlan_dev_info(dev);
-	return dev_ethtool_get_settings(vlan->real_dev, cmd);
+
+	return __ethtool_get_settings(vlan->real_dev, cmd);
 }
 
 static void vlan_ethtool_get_drvinfo(struct net_device *dev,
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index b365bba..043a5eb 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -35,7 +35,7 @@ static int port_cost(struct net_device *dev)
 {
 	struct ethtool_cmd ecmd;
 
-	if (!dev_ethtool_get_settings(dev, &ecmd)) {
+	if (!__ethtool_get_settings(dev, &ecmd)) {
 		switch (ethtool_cmd_speed(&ecmd)) {
 		case SPEED_10000:
 			return 2;
diff --git a/net/core/dev.c b/net/core/dev.c
index 11b0fc7..abdc0e3 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4570,22 +4570,17 @@ void dev_set_rx_mode(struct net_device *dev)
  *	@dev: device
  *	@cmd: memory area for ethtool_ops::get_settings() result
  *
- *      The cmd arg is initialized properly (cleared and
- *      ethtool_cmd::cmd field set to ETHTOOL_GSET).
- *
- *	Return device's ethtool_ops::get_settings() result value or
- *	-EOPNOTSUPP when device doesn't expose
- *	ethtool_ops::get_settings() operation.
+ *	This is wrapper taking rtnl lock for __ethtool_get_settings()
  */
 int dev_ethtool_get_settings(struct net_device *dev,
 			     struct ethtool_cmd *cmd)
 {
-	if (!dev->ethtool_ops || !dev->ethtool_ops->get_settings)
-		return -EOPNOTSUPP;
+	int ret;
 
-	memset(cmd, 0, sizeof(struct ethtool_cmd));
-	cmd->cmd = ETHTOOL_GSET;
-	return dev->ethtool_ops->get_settings(dev, cmd);
+	rtnl_lock();
+	ret = __ethtool_get_settings(dev, cmd);
+	rtnl_unlock();
+	return ret;
 }
 EXPORT_SYMBOL(dev_ethtool_get_settings);
 
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 6cdba5f..94326d4 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -569,15 +569,22 @@ int __ethtool_set_flags(struct net_device *dev, u32 data)
 	return 0;
 }
 
+int __ethtool_get_settings(struct net_device *dev, struct ethtool_cmd *cmd)
+{
+	if (!dev->ethtool_ops || !dev->ethtool_ops->get_settings)
+		return -EOPNOTSUPP;
+
+	memset(cmd, 0, sizeof(struct ethtool_cmd));
+	cmd->cmd = ETHTOOL_GSET;
+	return dev->ethtool_ops->get_settings(dev, cmd);
+}
+
 static int ethtool_get_settings(struct net_device *dev, void __user *useraddr)
 {
-	struct ethtool_cmd cmd = { .cmd = ETHTOOL_GSET };
 	int err;
+	struct ethtool_cmd cmd;
 
-	if (!dev->ethtool_ops->get_settings)
-		return -EOPNOTSUPP;
-
-	err = dev->ethtool_ops->get_settings(dev, &cmd);
+	err = __ethtool_get_settings(dev, &cmd);
 	if (err < 0)
 		return err;
 
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index 90fdb46..48e6279 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -172,7 +172,7 @@ static ssize_t show_speed(struct device *dev,
 
 	if (netif_running(netdev)) {
 		struct ethtool_cmd cmd;
-		if (!dev_ethtool_get_settings(netdev, &cmd))
+		if (!__ethtool_get_settings(netdev, &cmd))
 			ret = sprintf(buf, fmt_udec, ethtool_cmd_speed(&cmd));
 	}
 	rtnl_unlock();
@@ -190,7 +190,7 @@ static ssize_t show_duplex(struct device *dev,
 
 	if (netif_running(netdev)) {
 		struct ethtool_cmd cmd;
-		if (!dev_ethtool_get_settings(netdev, &cmd))
+		if (!__ethtool_get_settings(netdev, &cmd))
 			ret = sprintf(buf, "%s\n",
 				      cmd.duplex ? "full" : "half");
 	}
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 2ea3d63..1bcc794 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -530,33 +530,30 @@ static int prb_calc_retire_blk_tmo(struct packet_sock *po,
 {
 	struct net_device *dev;
 	unsigned int mbits = 0, msec = 0, div = 0, tmo = 0;
+	struct ethtool_cmd ecmd;
 
 	dev = dev_get_by_index(sock_net(&po->sk), po->ifindex);
 	if (unlikely(dev == NULL))
 		return DEFAULT_PRB_RETIRE_TOV;
 
-	if (dev->ethtool_ops && dev->ethtool_ops->get_settings) {
-		struct ethtool_cmd ecmd = { .cmd = ETHTOOL_GSET, };
-
-		if (!dev->ethtool_ops->get_settings(dev, &ecmd)) {
-			switch (ecmd.speed) {
-			case SPEED_10000:
-				msec = 1;
-				div = 10000/1000;
-				break;
-			case SPEED_1000:
-				msec = 1;
-				div = 1000/1000;
-				break;
-			/*
-			 * If the link speed is so slow you don't really
-			 * need to worry about perf anyways
-			 */
-			case SPEED_100:
-			case SPEED_10:
-			default:
-				return DEFAULT_PRB_RETIRE_TOV;
-			}
+	if (!dev_ethtool_get_settings(dev, &ecmd)) {
+		switch (ecmd.speed) {
+		case SPEED_10000:
+			msec = 1;
+			div = 10000/1000;
+			break;
+		case SPEED_1000:
+			msec = 1;
+			div = 1000/1000;
+			break;
+		/*
+		 * If the link speed is so slow you don't really
+		 * need to worry about perf anyways
+		 */
+		case SPEED_100:
+		case SPEED_10:
+		default:
+			return DEFAULT_PRB_RETIRE_TOV;
 		}
 	}
 
-- 
1.7.6

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [Bridge] [patch net-next-2.6] net: consolidate and fix ethtool_ops->get_settings calling
@ 2011-09-01 19:28 ` Jiri Pirko
  0 siblings, 0 replies; 25+ messages in thread
From: Jiri Pirko @ 2011-09-01 19:28 UTC (permalink / raw)
  To: netdev
  Cc: amit.salecha, bridge, linux-mips, devel, linux-scsi, decot,
	therbert, robert.w.love, eric.dumazet, fubar, JBottomley,
	xiaosuo, paulmck, alexander.h.duyck, mirq-linux, bhutchings,
	greearb, bprakash, laijs, ralf, davem, loke.chetan

This patch does several things:
- introduces __ethtool_get_settings which is called from ethtool code and
  from dev_ethtool_get_settings() as well.
- dev_ethtool_get_settings() becomes rtnl wrapper for
  __ethtool_get_settings()
- changes calling in drivers so rtnl locking is respected. In
  iboe_get_rate was previously ->get_settings() called unlocked. This
  fixes it
- introduces rtnl_lock in bnx2fc_vport_create() and fcoe_vport_create()
  so bnx2fc_if_create() and fcoe_if_create() are called locked as they
  are from other places.
- prb_calc_retire_blk_tmo() in af_packet.c was not calling get_settings
  with rtnl_lock. So use dev_ethtool_get_settings here.
- use __ethtool_get_settings() in bonding code

Signed-off-by: Jiri Pirko <jpirko@redhat.com>
---
 arch/mips/txx9/generic/setup_tx4939.c |    2 +-
 drivers/net/bonding/bond_main.c       |   13 ++++------
 drivers/net/macvlan.c                 |    3 +-
 drivers/scsi/bnx2fc/bnx2fc_fcoe.c     |    4 ++-
 drivers/scsi/fcoe/fcoe.c              |    4 ++-
 include/linux/ethtool.h               |    3 ++
 net/8021q/vlan_dev.c                  |    3 +-
 net/bridge/br_if.c                    |    2 +-
 net/core/dev.c                        |   17 +++++---------
 net/core/ethtool.c                    |   17 +++++++++----
 net/core/net-sysfs.c                  |    4 +-
 net/packet/af_packet.c                |   41 +++++++++++++++-----------------
 12 files changed, 59 insertions(+), 54 deletions(-)

diff --git a/arch/mips/txx9/generic/setup_tx4939.c b/arch/mips/txx9/generic/setup_tx4939.c
index e9f95dc..ba3cec3 100644
--- a/arch/mips/txx9/generic/setup_tx4939.c
+++ b/arch/mips/txx9/generic/setup_tx4939.c
@@ -321,7 +321,7 @@ void __init tx4939_sio_init(unsigned int sclk, unsigned int cts_mask)
 static u32 tx4939_get_eth_speed(struct net_device *dev)
 {
 	struct ethtool_cmd cmd;
-	if (dev_ethtool_get_settings(dev, &cmd))
+	if (__ethtool_get_settings(dev, &cmd))
 		return 100;	/* default 100Mbps */
 
 	return ethtool_cmd_speed(&cmd);
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 8cb75a6..1dcb07c 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -557,7 +557,7 @@ down:
 static int bond_update_speed_duplex(struct slave *slave)
 {
 	struct net_device *slave_dev = slave->dev;
-	struct ethtool_cmd etool = { .cmd = ETHTOOL_GSET };
+	struct ethtool_cmd ecmd;
 	u32 slave_speed;
 	int res;
 
@@ -565,18 +565,15 @@ static int bond_update_speed_duplex(struct slave *slave)
 	slave->speed = SPEED_100;
 	slave->duplex = DUPLEX_FULL;
 
-	if (!slave_dev->ethtool_ops || !slave_dev->ethtool_ops->get_settings)
-		return -1;
-
-	res = slave_dev->ethtool_ops->get_settings(slave_dev, &etool);
+	res = __ethtool_get_settings(slave_dev, &ecmd);
 	if (res < 0)
 		return -1;
 
-	slave_speed = ethtool_cmd_speed(&etool);
+	slave_speed = ethtool_cmd_speed(&ecmd);
 	if (slave_speed == 0 || slave_speed == ((__u32) -1))
 		return -1;
 
-	switch (etool.duplex) {
+	switch (ecmd.duplex) {
 	case DUPLEX_FULL:
 	case DUPLEX_HALF:
 		break;
@@ -585,7 +582,7 @@ static int bond_update_speed_duplex(struct slave *slave)
 	}
 
 	slave->speed = slave_speed;
-	slave->duplex = etool.duplex;
+	slave->duplex = ecmd.duplex;
 
 	return 0;
 }
diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index 836e13f..b100c90 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -543,7 +543,8 @@ static int macvlan_ethtool_get_settings(struct net_device *dev,
 					struct ethtool_cmd *cmd)
 {
 	const struct macvlan_dev *vlan = netdev_priv(dev);
-	return dev_ethtool_get_settings(vlan->lowerdev, cmd);
+
+	return __ethtool_get_settings(vlan->lowerdev, cmd);
 }
 
 static const struct ethtool_ops macvlan_ethtool_ops = {
diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
index 2c780a7..820a184 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
@@ -673,7 +673,7 @@ static void bnx2fc_link_speed_update(struct fc_lport *lport)
 	struct net_device *netdev = interface->netdev;
 	struct ethtool_cmd ecmd;
 
-	if (!dev_ethtool_get_settings(netdev, &ecmd)) {
+	if (!__ethtool_get_settings(netdev, &ecmd)) {
 		lport->link_supported_speeds &=
 			~(FC_PORTSPEED_1GBIT | FC_PORTSPEED_10GBIT);
 		if (ecmd.supported & (SUPPORTED_1000baseT_Half |
@@ -1001,9 +1001,11 @@ static int bnx2fc_vport_create(struct fc_vport *vport, bool disabled)
 			"this interface\n");
 		return -EIO;
 	}
+	rtnl_lock();
 	mutex_lock(&bnx2fc_dev_lock);
 	vn_port = bnx2fc_if_create(interface, &vport->dev, 1);
 	mutex_unlock(&bnx2fc_dev_lock);
+	rtnl_unlock();
 
 	if (IS_ERR(vn_port)) {
 		printk(KERN_ERR PFX "bnx2fc_vport_create (%s) failed\n",
diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index 3416ab6..83aa3ac 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -2043,7 +2043,7 @@ int fcoe_link_speed_update(struct fc_lport *lport)
 	struct net_device *netdev = fcoe_netdev(lport);
 	struct ethtool_cmd ecmd;
 
-	if (!dev_ethtool_get_settings(netdev, &ecmd)) {
+	if (!__ethtool_get_settings(netdev, &ecmd)) {
 		lport->link_supported_speeds &=
 			~(FC_PORTSPEED_1GBIT | FC_PORTSPEED_10GBIT);
 		if (ecmd.supported & (SUPPORTED_1000baseT_Half |
@@ -2452,7 +2452,9 @@ static int fcoe_vport_create(struct fc_vport *vport, bool disabled)
 	}
 
 	mutex_lock(&fcoe_config_mutex);
+	rtnl_lock();
 	vn_port = fcoe_if_create(fcoe, &vport->dev, 1);
+	rtnl_unlock();
 	mutex_unlock(&fcoe_config_mutex);
 
 	if (IS_ERR(vn_port)) {
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 3829712..8571f18 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -728,6 +728,9 @@ enum ethtool_sfeatures_retval_bits {
 /* needed by dev_disable_lro() */
 extern int __ethtool_set_flags(struct net_device *dev, u32 flags);
 
+extern int __ethtool_get_settings(struct net_device *dev,
+				  struct ethtool_cmd *cmd);
+
 /**
  * enum ethtool_phys_id_state - indicator state for physical identification
  * @ETHTOOL_ID_INACTIVE: Physical ID indicator should be deactivated
diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index eba705b..c8cf939 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -610,7 +610,8 @@ static int vlan_ethtool_get_settings(struct net_device *dev,
 				     struct ethtool_cmd *cmd)
 {
 	const struct vlan_dev_info *vlan = vlan_dev_info(dev);
-	return dev_ethtool_get_settings(vlan->real_dev, cmd);
+
+	return __ethtool_get_settings(vlan->real_dev, cmd);
 }
 
 static void vlan_ethtool_get_drvinfo(struct net_device *dev,
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index b365bba..043a5eb 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -35,7 +35,7 @@ static int port_cost(struct net_device *dev)
 {
 	struct ethtool_cmd ecmd;
 
-	if (!dev_ethtool_get_settings(dev, &ecmd)) {
+	if (!__ethtool_get_settings(dev, &ecmd)) {
 		switch (ethtool_cmd_speed(&ecmd)) {
 		case SPEED_10000:
 			return 2;
diff --git a/net/core/dev.c b/net/core/dev.c
index 11b0fc7..abdc0e3 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4570,22 +4570,17 @@ void dev_set_rx_mode(struct net_device *dev)
  *	@dev: device
  *	@cmd: memory area for ethtool_ops::get_settings() result
  *
- *      The cmd arg is initialized properly (cleared and
- *      ethtool_cmd::cmd field set to ETHTOOL_GSET).
- *
- *	Return device's ethtool_ops::get_settings() result value or
- *	-EOPNOTSUPP when device doesn't expose
- *	ethtool_ops::get_settings() operation.
+ *	This is wrapper taking rtnl lock for __ethtool_get_settings()
  */
 int dev_ethtool_get_settings(struct net_device *dev,
 			     struct ethtool_cmd *cmd)
 {
-	if (!dev->ethtool_ops || !dev->ethtool_ops->get_settings)
-		return -EOPNOTSUPP;
+	int ret;
 
-	memset(cmd, 0, sizeof(struct ethtool_cmd));
-	cmd->cmd = ETHTOOL_GSET;
-	return dev->ethtool_ops->get_settings(dev, cmd);
+	rtnl_lock();
+	ret = __ethtool_get_settings(dev, cmd);
+	rtnl_unlock();
+	return ret;
 }
 EXPORT_SYMBOL(dev_ethtool_get_settings);
 
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 6cdba5f..94326d4 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -569,15 +569,22 @@ int __ethtool_set_flags(struct net_device *dev, u32 data)
 	return 0;
 }
 
+int __ethtool_get_settings(struct net_device *dev, struct ethtool_cmd *cmd)
+{
+	if (!dev->ethtool_ops || !dev->ethtool_ops->get_settings)
+		return -EOPNOTSUPP;
+
+	memset(cmd, 0, sizeof(struct ethtool_cmd));
+	cmd->cmd = ETHTOOL_GSET;
+	return dev->ethtool_ops->get_settings(dev, cmd);
+}
+
 static int ethtool_get_settings(struct net_device *dev, void __user *useraddr)
 {
-	struct ethtool_cmd cmd = { .cmd = ETHTOOL_GSET };
 	int err;
+	struct ethtool_cmd cmd;
 
-	if (!dev->ethtool_ops->get_settings)
-		return -EOPNOTSUPP;
-
-	err = dev->ethtool_ops->get_settings(dev, &cmd);
+	err = __ethtool_get_settings(dev, &cmd);
 	if (err < 0)
 		return err;
 
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index 90fdb46..48e6279 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -172,7 +172,7 @@ static ssize_t show_speed(struct device *dev,
 
 	if (netif_running(netdev)) {
 		struct ethtool_cmd cmd;
-		if (!dev_ethtool_get_settings(netdev, &cmd))
+		if (!__ethtool_get_settings(netdev, &cmd))
 			ret = sprintf(buf, fmt_udec, ethtool_cmd_speed(&cmd));
 	}
 	rtnl_unlock();
@@ -190,7 +190,7 @@ static ssize_t show_duplex(struct device *dev,
 
 	if (netif_running(netdev)) {
 		struct ethtool_cmd cmd;
-		if (!dev_ethtool_get_settings(netdev, &cmd))
+		if (!__ethtool_get_settings(netdev, &cmd))
 			ret = sprintf(buf, "%s\n",
 				      cmd.duplex ? "full" : "half");
 	}
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 2ea3d63..1bcc794 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -530,33 +530,30 @@ static int prb_calc_retire_blk_tmo(struct packet_sock *po,
 {
 	struct net_device *dev;
 	unsigned int mbits = 0, msec = 0, div = 0, tmo = 0;
+	struct ethtool_cmd ecmd;
 
 	dev = dev_get_by_index(sock_net(&po->sk), po->ifindex);
 	if (unlikely(dev == NULL))
 		return DEFAULT_PRB_RETIRE_TOV;
 
-	if (dev->ethtool_ops && dev->ethtool_ops->get_settings) {
-		struct ethtool_cmd ecmd = { .cmd = ETHTOOL_GSET, };
-
-		if (!dev->ethtool_ops->get_settings(dev, &ecmd)) {
-			switch (ecmd.speed) {
-			case SPEED_10000:
-				msec = 1;
-				div = 10000/1000;
-				break;
-			case SPEED_1000:
-				msec = 1;
-				div = 1000/1000;
-				break;
-			/*
-			 * If the link speed is so slow you don't really
-			 * need to worry about perf anyways
-			 */
-			case SPEED_100:
-			case SPEED_10:
-			default:
-				return DEFAULT_PRB_RETIRE_TOV;
-			}
+	if (!dev_ethtool_get_settings(dev, &ecmd)) {
+		switch (ecmd.speed) {
+		case SPEED_10000:
+			msec = 1;
+			div = 10000/1000;
+			break;
+		case SPEED_1000:
+			msec = 1;
+			div = 1000/1000;
+			break;
+		/*
+		 * If the link speed is so slow you don't really
+		 * need to worry about perf anyways
+		 */
+		case SPEED_100:
+		case SPEED_10:
+		default:
+			return DEFAULT_PRB_RETIRE_TOV;
 		}
 	}
 
-- 
1.7.6


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [patch net-next-2.6 v2] net: consolidate and fix ethtool_ops->get_settings calling
  2011-09-01 19:28 ` [Bridge] " Jiri Pirko
@ 2011-09-02 12:26   ` Jiri Pirko
  -1 siblings, 0 replies; 25+ messages in thread
From: Jiri Pirko @ 2011-09-02 12:26 UTC (permalink / raw)
  To: netdev
  Cc: ralf, fubar, andy, kaber, bprakash, JBottomley, robert.w.love,
	davem, shemminger, decot, bhutchings, mirq-linux,
	alexander.h.duyck, amit.salecha, eric.dumazet, therbert, paulmck,
	laijs, xiaosuo, greearb, loke.chetan, linux-mips, linux-scsi,
	devel, bridge

This patch does several things:
- introduces __ethtool_get_settings which is called from ethtool code and
  from dev_ethtool_get_settings() as well.
- dev_ethtool_get_settings() becomes rtnl wrapper for
  __ethtool_get_settings()
- changes calling in drivers so rtnl locking is respected. In
  iboe_get_rate was previously ->get_settings() called unlocked. This
  fixes it
- introduces rtnl_lock in bnx2fc_vport_create() and fcoe_vport_create()
  so bnx2fc_if_create() and fcoe_if_create() are called locked as they
  are from other places.
- prb_calc_retire_blk_tmo() in af_packet.c was not calling get_settings
  with rntl_lock. So use dev_ethtool_get_settings here.
- use __ethtool_get_settings() in bonding code

Signed-off-by: Jiri Pirko <jpirko@redhat.com>

v1->v2:
	add missing export_symbol 
---
 arch/mips/txx9/generic/setup_tx4939.c |    2 +-
 drivers/net/bonding/bond_main.c       |   13 ++++------
 drivers/net/macvlan.c                 |    3 +-
 drivers/scsi/bnx2fc/bnx2fc_fcoe.c     |    4 ++-
 drivers/scsi/fcoe/fcoe.c              |    4 ++-
 include/linux/ethtool.h               |    3 ++
 net/8021q/vlan_dev.c                  |    3 +-
 net/bridge/br_if.c                    |    2 +-
 net/core/dev.c                        |   17 +++++---------
 net/core/ethtool.c                    |   18 ++++++++++----
 net/core/net-sysfs.c                  |    4 +-
 net/packet/af_packet.c                |   41 +++++++++++++++-----------------
 12 files changed, 60 insertions(+), 54 deletions(-)

diff --git a/arch/mips/txx9/generic/setup_tx4939.c b/arch/mips/txx9/generic/setup_tx4939.c
index e9f95dc..ba3cec3 100644
--- a/arch/mips/txx9/generic/setup_tx4939.c
+++ b/arch/mips/txx9/generic/setup_tx4939.c
@@ -321,7 +321,7 @@ void __init tx4939_sio_init(unsigned int sclk, unsigned int cts_mask)
 static u32 tx4939_get_eth_speed(struct net_device *dev)
 {
 	struct ethtool_cmd cmd;
-	if (dev_ethtool_get_settings(dev, &cmd))
+	if (__ethtool_get_settings(dev, &cmd))
 		return 100;	/* default 100Mbps */
 
 	return ethtool_cmd_speed(&cmd);
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 8cb75a6..1dcb07c 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -557,7 +557,7 @@ down:
 static int bond_update_speed_duplex(struct slave *slave)
 {
 	struct net_device *slave_dev = slave->dev;
-	struct ethtool_cmd etool = { .cmd = ETHTOOL_GSET };
+	struct ethtool_cmd ecmd;
 	u32 slave_speed;
 	int res;
 
@@ -565,18 +565,15 @@ static int bond_update_speed_duplex(struct slave *slave)
 	slave->speed = SPEED_100;
 	slave->duplex = DUPLEX_FULL;
 
-	if (!slave_dev->ethtool_ops || !slave_dev->ethtool_ops->get_settings)
-		return -1;
-
-	res = slave_dev->ethtool_ops->get_settings(slave_dev, &etool);
+	res = __ethtool_get_settings(slave_dev, &ecmd);
 	if (res < 0)
 		return -1;
 
-	slave_speed = ethtool_cmd_speed(&etool);
+	slave_speed = ethtool_cmd_speed(&ecmd);
 	if (slave_speed == 0 || slave_speed == ((__u32) -1))
 		return -1;
 
-	switch (etool.duplex) {
+	switch (ecmd.duplex) {
 	case DUPLEX_FULL:
 	case DUPLEX_HALF:
 		break;
@@ -585,7 +582,7 @@ static int bond_update_speed_duplex(struct slave *slave)
 	}
 
 	slave->speed = slave_speed;
-	slave->duplex = etool.duplex;
+	slave->duplex = ecmd.duplex;
 
 	return 0;
 }
diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index 836e13f..b100c90 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -543,7 +543,8 @@ static int macvlan_ethtool_get_settings(struct net_device *dev,
 					struct ethtool_cmd *cmd)
 {
 	const struct macvlan_dev *vlan = netdev_priv(dev);
-	return dev_ethtool_get_settings(vlan->lowerdev, cmd);
+
+	return __ethtool_get_settings(vlan->lowerdev, cmd);
 }
 
 static const struct ethtool_ops macvlan_ethtool_ops = {
diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
index 2c780a7..820a184 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
@@ -673,7 +673,7 @@ static void bnx2fc_link_speed_update(struct fc_lport *lport)
 	struct net_device *netdev = interface->netdev;
 	struct ethtool_cmd ecmd;
 
-	if (!dev_ethtool_get_settings(netdev, &ecmd)) {
+	if (!__ethtool_get_settings(netdev, &ecmd)) {
 		lport->link_supported_speeds &=
 			~(FC_PORTSPEED_1GBIT | FC_PORTSPEED_10GBIT);
 		if (ecmd.supported & (SUPPORTED_1000baseT_Half |
@@ -1001,9 +1001,11 @@ static int bnx2fc_vport_create(struct fc_vport *vport, bool disabled)
 			"this interface\n");
 		return -EIO;
 	}
+	rtnl_lock();
 	mutex_lock(&bnx2fc_dev_lock);
 	vn_port = bnx2fc_if_create(interface, &vport->dev, 1);
 	mutex_unlock(&bnx2fc_dev_lock);
+	rtnl_unlock();
 
 	if (IS_ERR(vn_port)) {
 		printk(KERN_ERR PFX "bnx2fc_vport_create (%s) failed\n",
diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index 3416ab6..83aa3ac 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -2043,7 +2043,7 @@ int fcoe_link_speed_update(struct fc_lport *lport)
 	struct net_device *netdev = fcoe_netdev(lport);
 	struct ethtool_cmd ecmd;
 
-	if (!dev_ethtool_get_settings(netdev, &ecmd)) {
+	if (!__ethtool_get_settings(netdev, &ecmd)) {
 		lport->link_supported_speeds &=
 			~(FC_PORTSPEED_1GBIT | FC_PORTSPEED_10GBIT);
 		if (ecmd.supported & (SUPPORTED_1000baseT_Half |
@@ -2452,7 +2452,9 @@ static int fcoe_vport_create(struct fc_vport *vport, bool disabled)
 	}
 
 	mutex_lock(&fcoe_config_mutex);
+	rtnl_lock();
 	vn_port = fcoe_if_create(fcoe, &vport->dev, 1);
+	rtnl_unlock();
 	mutex_unlock(&fcoe_config_mutex);
 
 	if (IS_ERR(vn_port)) {
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 3829712..8571f18 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -728,6 +728,9 @@ enum ethtool_sfeatures_retval_bits {
 /* needed by dev_disable_lro() */
 extern int __ethtool_set_flags(struct net_device *dev, u32 flags);
 
+extern int __ethtool_get_settings(struct net_device *dev,
+				  struct ethtool_cmd *cmd);
+
 /**
  * enum ethtool_phys_id_state - indicator state for physical identification
  * @ETHTOOL_ID_INACTIVE: Physical ID indicator should be deactivated
diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index eba705b..c8cf939 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -610,7 +610,8 @@ static int vlan_ethtool_get_settings(struct net_device *dev,
 				     struct ethtool_cmd *cmd)
 {
 	const struct vlan_dev_info *vlan = vlan_dev_info(dev);
-	return dev_ethtool_get_settings(vlan->real_dev, cmd);
+
+	return __ethtool_get_settings(vlan->real_dev, cmd);
 }
 
 static void vlan_ethtool_get_drvinfo(struct net_device *dev,
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index b365bba..043a5eb 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -35,7 +35,7 @@ static int port_cost(struct net_device *dev)
 {
 	struct ethtool_cmd ecmd;
 
-	if (!dev_ethtool_get_settings(dev, &ecmd)) {
+	if (!__ethtool_get_settings(dev, &ecmd)) {
 		switch (ethtool_cmd_speed(&ecmd)) {
 		case SPEED_10000:
 			return 2;
diff --git a/net/core/dev.c b/net/core/dev.c
index 11b0fc7..abdc0e3 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4570,22 +4570,17 @@ void dev_set_rx_mode(struct net_device *dev)
  *	@dev: device
  *	@cmd: memory area for ethtool_ops::get_settings() result
  *
- *      The cmd arg is initialized properly (cleared and
- *      ethtool_cmd::cmd field set to ETHTOOL_GSET).
- *
- *	Return device's ethtool_ops::get_settings() result value or
- *	-EOPNOTSUPP when device doesn't expose
- *	ethtool_ops::get_settings() operation.
+ *	This is wrapper taking rtnl lock for __ethtool_get_settings()
  */
 int dev_ethtool_get_settings(struct net_device *dev,
 			     struct ethtool_cmd *cmd)
 {
-	if (!dev->ethtool_ops || !dev->ethtool_ops->get_settings)
-		return -EOPNOTSUPP;
+	int ret;
 
-	memset(cmd, 0, sizeof(struct ethtool_cmd));
-	cmd->cmd = ETHTOOL_GSET;
-	return dev->ethtool_ops->get_settings(dev, cmd);
+	rtnl_lock();
+	ret = __ethtool_get_settings(dev, cmd);
+	rtnl_unlock();
+	return ret;
 }
 EXPORT_SYMBOL(dev_ethtool_get_settings);
 
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 6cdba5f..307297c 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -569,15 +569,23 @@ int __ethtool_set_flags(struct net_device *dev, u32 data)
 	return 0;
 }
 
+int __ethtool_get_settings(struct net_device *dev, struct ethtool_cmd *cmd)
+{
+	if (!dev->ethtool_ops || !dev->ethtool_ops->get_settings)
+		return -EOPNOTSUPP;
+
+	memset(cmd, 0, sizeof(struct ethtool_cmd));
+	cmd->cmd = ETHTOOL_GSET;
+	return dev->ethtool_ops->get_settings(dev, cmd);
+}
+EXPORT_SYMBOL(__ethtool_get_settings);
+
 static int ethtool_get_settings(struct net_device *dev, void __user *useraddr)
 {
-	struct ethtool_cmd cmd = { .cmd = ETHTOOL_GSET };
 	int err;
+	struct ethtool_cmd cmd;
 
-	if (!dev->ethtool_ops->get_settings)
-		return -EOPNOTSUPP;
-
-	err = dev->ethtool_ops->get_settings(dev, &cmd);
+	err = __ethtool_get_settings(dev, &cmd);
 	if (err < 0)
 		return err;
 
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index 90fdb46..48e6279 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -172,7 +172,7 @@ static ssize_t show_speed(struct device *dev,
 
 	if (netif_running(netdev)) {
 		struct ethtool_cmd cmd;
-		if (!dev_ethtool_get_settings(netdev, &cmd))
+		if (!__ethtool_get_settings(netdev, &cmd))
 			ret = sprintf(buf, fmt_udec, ethtool_cmd_speed(&cmd));
 	}
 	rtnl_unlock();
@@ -190,7 +190,7 @@ static ssize_t show_duplex(struct device *dev,
 
 	if (netif_running(netdev)) {
 		struct ethtool_cmd cmd;
-		if (!dev_ethtool_get_settings(netdev, &cmd))
+		if (!__ethtool_get_settings(netdev, &cmd))
 			ret = sprintf(buf, "%s\n",
 				      cmd.duplex ? "full" : "half");
 	}
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 2ea3d63..1bcc794 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -530,33 +530,30 @@ static int prb_calc_retire_blk_tmo(struct packet_sock *po,
 {
 	struct net_device *dev;
 	unsigned int mbits = 0, msec = 0, div = 0, tmo = 0;
+	struct ethtool_cmd ecmd;
 
 	dev = dev_get_by_index(sock_net(&po->sk), po->ifindex);
 	if (unlikely(dev == NULL))
 		return DEFAULT_PRB_RETIRE_TOV;
 
-	if (dev->ethtool_ops && dev->ethtool_ops->get_settings) {
-		struct ethtool_cmd ecmd = { .cmd = ETHTOOL_GSET, };
-
-		if (!dev->ethtool_ops->get_settings(dev, &ecmd)) {
-			switch (ecmd.speed) {
-			case SPEED_10000:
-				msec = 1;
-				div = 10000/1000;
-				break;
-			case SPEED_1000:
-				msec = 1;
-				div = 1000/1000;
-				break;
-			/*
-			 * If the link speed is so slow you don't really
-			 * need to worry about perf anyways
-			 */
-			case SPEED_100:
-			case SPEED_10:
-			default:
-				return DEFAULT_PRB_RETIRE_TOV;
-			}
+	if (!dev_ethtool_get_settings(dev, &ecmd)) {
+		switch (ecmd.speed) {
+		case SPEED_10000:
+			msec = 1;
+			div = 10000/1000;
+			break;
+		case SPEED_1000:
+			msec = 1;
+			div = 1000/1000;
+			break;
+		/*
+		 * If the link speed is so slow you don't really
+		 * need to worry about perf anyways
+		 */
+		case SPEED_100:
+		case SPEED_10:
+		default:
+			return DEFAULT_PRB_RETIRE_TOV;
 		}
 	}
 
-- 
1.7.6


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [Bridge] [patch net-next-2.6 v2] net: consolidate and fix ethtool_ops->get_settings calling
@ 2011-09-02 12:26   ` Jiri Pirko
  0 siblings, 0 replies; 25+ messages in thread
From: Jiri Pirko @ 2011-09-02 12:26 UTC (permalink / raw)
  To: netdev
  Cc: amit.salecha, bridge, linux-mips, devel, linux-scsi, decot,
	therbert, robert.w.love, eric.dumazet, fubar, JBottomley,
	xiaosuo, paulmck, alexander.h.duyck, mirq-linux, bhutchings,
	greearb, bprakash, laijs, ralf, davem, loke.chetan

This patch does several things:
- introduces __ethtool_get_settings which is called from ethtool code and
  from dev_ethtool_get_settings() as well.
- dev_ethtool_get_settings() becomes rtnl wrapper for
  __ethtool_get_settings()
- changes calling in drivers so rtnl locking is respected. In
  iboe_get_rate was previously ->get_settings() called unlocked. This
  fixes it
- introduces rtnl_lock in bnx2fc_vport_create() and fcoe_vport_create()
  so bnx2fc_if_create() and fcoe_if_create() are called locked as they
  are from other places.
- prb_calc_retire_blk_tmo() in af_packet.c was not calling get_settings
  with rntl_lock. So use dev_ethtool_get_settings here.
- use __ethtool_get_settings() in bonding code

Signed-off-by: Jiri Pirko <jpirko@redhat.com>

v1->v2:
	add missing export_symbol 
---
 arch/mips/txx9/generic/setup_tx4939.c |    2 +-
 drivers/net/bonding/bond_main.c       |   13 ++++------
 drivers/net/macvlan.c                 |    3 +-
 drivers/scsi/bnx2fc/bnx2fc_fcoe.c     |    4 ++-
 drivers/scsi/fcoe/fcoe.c              |    4 ++-
 include/linux/ethtool.h               |    3 ++
 net/8021q/vlan_dev.c                  |    3 +-
 net/bridge/br_if.c                    |    2 +-
 net/core/dev.c                        |   17 +++++---------
 net/core/ethtool.c                    |   18 ++++++++++----
 net/core/net-sysfs.c                  |    4 +-
 net/packet/af_packet.c                |   41 +++++++++++++++-----------------
 12 files changed, 60 insertions(+), 54 deletions(-)

diff --git a/arch/mips/txx9/generic/setup_tx4939.c b/arch/mips/txx9/generic/setup_tx4939.c
index e9f95dc..ba3cec3 100644
--- a/arch/mips/txx9/generic/setup_tx4939.c
+++ b/arch/mips/txx9/generic/setup_tx4939.c
@@ -321,7 +321,7 @@ void __init tx4939_sio_init(unsigned int sclk, unsigned int cts_mask)
 static u32 tx4939_get_eth_speed(struct net_device *dev)
 {
 	struct ethtool_cmd cmd;
-	if (dev_ethtool_get_settings(dev, &cmd))
+	if (__ethtool_get_settings(dev, &cmd))
 		return 100;	/* default 100Mbps */
 
 	return ethtool_cmd_speed(&cmd);
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 8cb75a6..1dcb07c 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -557,7 +557,7 @@ down:
 static int bond_update_speed_duplex(struct slave *slave)
 {
 	struct net_device *slave_dev = slave->dev;
-	struct ethtool_cmd etool = { .cmd = ETHTOOL_GSET };
+	struct ethtool_cmd ecmd;
 	u32 slave_speed;
 	int res;
 
@@ -565,18 +565,15 @@ static int bond_update_speed_duplex(struct slave *slave)
 	slave->speed = SPEED_100;
 	slave->duplex = DUPLEX_FULL;
 
-	if (!slave_dev->ethtool_ops || !slave_dev->ethtool_ops->get_settings)
-		return -1;
-
-	res = slave_dev->ethtool_ops->get_settings(slave_dev, &etool);
+	res = __ethtool_get_settings(slave_dev, &ecmd);
 	if (res < 0)
 		return -1;
 
-	slave_speed = ethtool_cmd_speed(&etool);
+	slave_speed = ethtool_cmd_speed(&ecmd);
 	if (slave_speed == 0 || slave_speed == ((__u32) -1))
 		return -1;
 
-	switch (etool.duplex) {
+	switch (ecmd.duplex) {
 	case DUPLEX_FULL:
 	case DUPLEX_HALF:
 		break;
@@ -585,7 +582,7 @@ static int bond_update_speed_duplex(struct slave *slave)
 	}
 
 	slave->speed = slave_speed;
-	slave->duplex = etool.duplex;
+	slave->duplex = ecmd.duplex;
 
 	return 0;
 }
diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index 836e13f..b100c90 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -543,7 +543,8 @@ static int macvlan_ethtool_get_settings(struct net_device *dev,
 					struct ethtool_cmd *cmd)
 {
 	const struct macvlan_dev *vlan = netdev_priv(dev);
-	return dev_ethtool_get_settings(vlan->lowerdev, cmd);
+
+	return __ethtool_get_settings(vlan->lowerdev, cmd);
 }
 
 static const struct ethtool_ops macvlan_ethtool_ops = {
diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
index 2c780a7..820a184 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
@@ -673,7 +673,7 @@ static void bnx2fc_link_speed_update(struct fc_lport *lport)
 	struct net_device *netdev = interface->netdev;
 	struct ethtool_cmd ecmd;
 
-	if (!dev_ethtool_get_settings(netdev, &ecmd)) {
+	if (!__ethtool_get_settings(netdev, &ecmd)) {
 		lport->link_supported_speeds &=
 			~(FC_PORTSPEED_1GBIT | FC_PORTSPEED_10GBIT);
 		if (ecmd.supported & (SUPPORTED_1000baseT_Half |
@@ -1001,9 +1001,11 @@ static int bnx2fc_vport_create(struct fc_vport *vport, bool disabled)
 			"this interface\n");
 		return -EIO;
 	}
+	rtnl_lock();
 	mutex_lock(&bnx2fc_dev_lock);
 	vn_port = bnx2fc_if_create(interface, &vport->dev, 1);
 	mutex_unlock(&bnx2fc_dev_lock);
+	rtnl_unlock();
 
 	if (IS_ERR(vn_port)) {
 		printk(KERN_ERR PFX "bnx2fc_vport_create (%s) failed\n",
diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index 3416ab6..83aa3ac 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -2043,7 +2043,7 @@ int fcoe_link_speed_update(struct fc_lport *lport)
 	struct net_device *netdev = fcoe_netdev(lport);
 	struct ethtool_cmd ecmd;
 
-	if (!dev_ethtool_get_settings(netdev, &ecmd)) {
+	if (!__ethtool_get_settings(netdev, &ecmd)) {
 		lport->link_supported_speeds &=
 			~(FC_PORTSPEED_1GBIT | FC_PORTSPEED_10GBIT);
 		if (ecmd.supported & (SUPPORTED_1000baseT_Half |
@@ -2452,7 +2452,9 @@ static int fcoe_vport_create(struct fc_vport *vport, bool disabled)
 	}
 
 	mutex_lock(&fcoe_config_mutex);
+	rtnl_lock();
 	vn_port = fcoe_if_create(fcoe, &vport->dev, 1);
+	rtnl_unlock();
 	mutex_unlock(&fcoe_config_mutex);
 
 	if (IS_ERR(vn_port)) {
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 3829712..8571f18 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -728,6 +728,9 @@ enum ethtool_sfeatures_retval_bits {
 /* needed by dev_disable_lro() */
 extern int __ethtool_set_flags(struct net_device *dev, u32 flags);
 
+extern int __ethtool_get_settings(struct net_device *dev,
+				  struct ethtool_cmd *cmd);
+
 /**
  * enum ethtool_phys_id_state - indicator state for physical identification
  * @ETHTOOL_ID_INACTIVE: Physical ID indicator should be deactivated
diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index eba705b..c8cf939 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -610,7 +610,8 @@ static int vlan_ethtool_get_settings(struct net_device *dev,
 				     struct ethtool_cmd *cmd)
 {
 	const struct vlan_dev_info *vlan = vlan_dev_info(dev);
-	return dev_ethtool_get_settings(vlan->real_dev, cmd);
+
+	return __ethtool_get_settings(vlan->real_dev, cmd);
 }
 
 static void vlan_ethtool_get_drvinfo(struct net_device *dev,
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index b365bba..043a5eb 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -35,7 +35,7 @@ static int port_cost(struct net_device *dev)
 {
 	struct ethtool_cmd ecmd;
 
-	if (!dev_ethtool_get_settings(dev, &ecmd)) {
+	if (!__ethtool_get_settings(dev, &ecmd)) {
 		switch (ethtool_cmd_speed(&ecmd)) {
 		case SPEED_10000:
 			return 2;
diff --git a/net/core/dev.c b/net/core/dev.c
index 11b0fc7..abdc0e3 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4570,22 +4570,17 @@ void dev_set_rx_mode(struct net_device *dev)
  *	@dev: device
  *	@cmd: memory area for ethtool_ops::get_settings() result
  *
- *      The cmd arg is initialized properly (cleared and
- *      ethtool_cmd::cmd field set to ETHTOOL_GSET).
- *
- *	Return device's ethtool_ops::get_settings() result value or
- *	-EOPNOTSUPP when device doesn't expose
- *	ethtool_ops::get_settings() operation.
+ *	This is wrapper taking rtnl lock for __ethtool_get_settings()
  */
 int dev_ethtool_get_settings(struct net_device *dev,
 			     struct ethtool_cmd *cmd)
 {
-	if (!dev->ethtool_ops || !dev->ethtool_ops->get_settings)
-		return -EOPNOTSUPP;
+	int ret;
 
-	memset(cmd, 0, sizeof(struct ethtool_cmd));
-	cmd->cmd = ETHTOOL_GSET;
-	return dev->ethtool_ops->get_settings(dev, cmd);
+	rtnl_lock();
+	ret = __ethtool_get_settings(dev, cmd);
+	rtnl_unlock();
+	return ret;
 }
 EXPORT_SYMBOL(dev_ethtool_get_settings);
 
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 6cdba5f..307297c 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -569,15 +569,23 @@ int __ethtool_set_flags(struct net_device *dev, u32 data)
 	return 0;
 }
 
+int __ethtool_get_settings(struct net_device *dev, struct ethtool_cmd *cmd)
+{
+	if (!dev->ethtool_ops || !dev->ethtool_ops->get_settings)
+		return -EOPNOTSUPP;
+
+	memset(cmd, 0, sizeof(struct ethtool_cmd));
+	cmd->cmd = ETHTOOL_GSET;
+	return dev->ethtool_ops->get_settings(dev, cmd);
+}
+EXPORT_SYMBOL(__ethtool_get_settings);
+
 static int ethtool_get_settings(struct net_device *dev, void __user *useraddr)
 {
-	struct ethtool_cmd cmd = { .cmd = ETHTOOL_GSET };
 	int err;
+	struct ethtool_cmd cmd;
 
-	if (!dev->ethtool_ops->get_settings)
-		return -EOPNOTSUPP;
-
-	err = dev->ethtool_ops->get_settings(dev, &cmd);
+	err = __ethtool_get_settings(dev, &cmd);
 	if (err < 0)
 		return err;
 
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index 90fdb46..48e6279 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -172,7 +172,7 @@ static ssize_t show_speed(struct device *dev,
 
 	if (netif_running(netdev)) {
 		struct ethtool_cmd cmd;
-		if (!dev_ethtool_get_settings(netdev, &cmd))
+		if (!__ethtool_get_settings(netdev, &cmd))
 			ret = sprintf(buf, fmt_udec, ethtool_cmd_speed(&cmd));
 	}
 	rtnl_unlock();
@@ -190,7 +190,7 @@ static ssize_t show_duplex(struct device *dev,
 
 	if (netif_running(netdev)) {
 		struct ethtool_cmd cmd;
-		if (!dev_ethtool_get_settings(netdev, &cmd))
+		if (!__ethtool_get_settings(netdev, &cmd))
 			ret = sprintf(buf, "%s\n",
 				      cmd.duplex ? "full" : "half");
 	}
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 2ea3d63..1bcc794 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -530,33 +530,30 @@ static int prb_calc_retire_blk_tmo(struct packet_sock *po,
 {
 	struct net_device *dev;
 	unsigned int mbits = 0, msec = 0, div = 0, tmo = 0;
+	struct ethtool_cmd ecmd;
 
 	dev = dev_get_by_index(sock_net(&po->sk), po->ifindex);
 	if (unlikely(dev == NULL))
 		return DEFAULT_PRB_RETIRE_TOV;
 
-	if (dev->ethtool_ops && dev->ethtool_ops->get_settings) {
-		struct ethtool_cmd ecmd = { .cmd = ETHTOOL_GSET, };
-
-		if (!dev->ethtool_ops->get_settings(dev, &ecmd)) {
-			switch (ecmd.speed) {
-			case SPEED_10000:
-				msec = 1;
-				div = 10000/1000;
-				break;
-			case SPEED_1000:
-				msec = 1;
-				div = 1000/1000;
-				break;
-			/*
-			 * If the link speed is so slow you don't really
-			 * need to worry about perf anyways
-			 */
-			case SPEED_100:
-			case SPEED_10:
-			default:
-				return DEFAULT_PRB_RETIRE_TOV;
-			}
+	if (!dev_ethtool_get_settings(dev, &ecmd)) {
+		switch (ecmd.speed) {
+		case SPEED_10000:
+			msec = 1;
+			div = 10000/1000;
+			break;
+		case SPEED_1000:
+			msec = 1;
+			div = 1000/1000;
+			break;
+		/*
+		 * If the link speed is so slow you don't really
+		 * need to worry about perf anyways
+		 */
+		case SPEED_100:
+		case SPEED_10:
+		default:
+			return DEFAULT_PRB_RETIRE_TOV;
 		}
 	}
 
-- 
1.7.6


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [patch net-next-2.6 v2] net: consolidate and fix ethtool_ops->get_settings calling
  2011-09-02 12:26   ` [Bridge] " Jiri Pirko
@ 2011-09-02 18:46     ` Ben Hutchings
  -1 siblings, 0 replies; 25+ messages in thread
From: Ben Hutchings @ 2011-09-02 18:46 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: netdev, ralf, fubar, andy, kaber, bprakash, JBottomley,
	robert.w.love, davem, shemminger, decot, mirq-linux,
	alexander.h.duyck, amit.salecha, eric.dumazet, therbert, paulmck,
	laijs, xiaosuo, greearb, loke.chetan, linux-mips, linux-scsi,
	devel, bridge

On Fri, 2011-09-02 at 14:26 +0200, Jiri Pirko wrote:
> This patch does several things:
> - introduces __ethtool_get_settings which is called from ethtool code and
>   from dev_ethtool_get_settings() as well.
> - dev_ethtool_get_settings() becomes rtnl wrapper for
>   __ethtool_get_settings()
[...]

I don't like this locking change.  Most other dev_*() functions require
the caller to hold RTNL, and it will break any OOT module calling
dev_ethtool_get_settings() without producing any warning at compile
time.  Why not put an ASSERT_RTNL() in it instead?

The rest of this looks fine.

Ben. 

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Bridge] [patch net-next-2.6 v2] net: consolidate and fix ethtool_ops->get_settings calling
@ 2011-09-02 18:46     ` Ben Hutchings
  0 siblings, 0 replies; 25+ messages in thread
From: Ben Hutchings @ 2011-09-02 18:46 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: amit.salecha, bridge, linux-mips, devel, linux-scsi, decot,
	therbert, robert.w.love, eric.dumazet, fubar, JBottomley,
	xiaosuo, paulmck, alexander.h.duyck, mirq-linux, greearb,
	bprakash, laijs, netdev, ralf, davem, loke.chetan

On Fri, 2011-09-02 at 14:26 +0200, Jiri Pirko wrote:
> This patch does several things:
> - introduces __ethtool_get_settings which is called from ethtool code and
>   from dev_ethtool_get_settings() as well.
> - dev_ethtool_get_settings() becomes rtnl wrapper for
>   __ethtool_get_settings()
[...]

I don't like this locking change.  Most other dev_*() functions require
the caller to hold RTNL, and it will break any OOT module calling
dev_ethtool_get_settings() without producing any warning at compile
time.  Why not put an ASSERT_RTNL() in it instead?

The rest of this looks fine.

Ben. 

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [patch net-next-2.6 v2] net: consolidate and fix ethtool_ops->get_settings calling
  2011-09-02 18:46     ` [Bridge] " Ben Hutchings
@ 2011-09-02 20:42       ` Jiri Pirko
  -1 siblings, 0 replies; 25+ messages in thread
From: Jiri Pirko @ 2011-09-02 20:42 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: netdev, ralf, fubar, andy, kaber, bprakash, JBottomley,
	robert.w.love, davem, shemminger, decot, mirq-linux,
	alexander.h.duyck, amit.salecha, eric.dumazet, therbert, paulmck,
	laijs, xiaosuo, greearb, loke.chetan, linux-mips, linux-scsi,
	devel, bridge

Fri, Sep 02, 2011 at 08:46:01PM CEST, bhutchings@solarflare.com wrote:
>On Fri, 2011-09-02 at 14:26 +0200, Jiri Pirko wrote:
>> This patch does several things:
>> - introduces __ethtool_get_settings which is called from ethtool code and
>>   from dev_ethtool_get_settings() as well.
>> - dev_ethtool_get_settings() becomes rtnl wrapper for
>>   __ethtool_get_settings()
>[...]
>
>I don't like this locking change.  Most other dev_*() functions require
>the caller to hold RTNL, and it will break any OOT module calling
>dev_ethtool_get_settings() without producing any warning at compile
>time.  Why not put an ASSERT_RTNL() in it instead?

Hmm. Okay, then I would remove dev_ethtool_get_settings() from
net/core/dev.c and only put __ethtool_get_settings() to
net/core/ethtool.c. Makes more sense to me to have it there...
ASSERT_RTNL woudl be good there as well.

>
>The rest of this looks fine.
>
>Ben. 
>
>-- 
>Ben Hutchings, Staff Engineer, Solarflare
>Not speaking for my employer; that's the marketing department's job.
>They asked us to note that Solarflare product names are trademarked.
>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Bridge] [patch net-next-2.6 v2] net: consolidate and fix ethtool_ops->get_settings calling
@ 2011-09-02 20:42       ` Jiri Pirko
  0 siblings, 0 replies; 25+ messages in thread
From: Jiri Pirko @ 2011-09-02 20:42 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: amit.salecha, bridge, linux-mips, devel, linux-scsi, decot,
	therbert, robert.w.love, eric.dumazet, fubar, JBottomley,
	xiaosuo, paulmck, alexander.h.duyck, mirq-linux, greearb,
	bprakash, laijs, netdev, ralf, davem, loke.chetan

Fri, Sep 02, 2011 at 08:46:01PM CEST, bhutchings@solarflare.com wrote:
>On Fri, 2011-09-02 at 14:26 +0200, Jiri Pirko wrote:
>> This patch does several things:
>> - introduces __ethtool_get_settings which is called from ethtool code and
>>   from dev_ethtool_get_settings() as well.
>> - dev_ethtool_get_settings() becomes rtnl wrapper for
>>   __ethtool_get_settings()
>[...]
>
>I don't like this locking change.  Most other dev_*() functions require
>the caller to hold RTNL, and it will break any OOT module calling
>dev_ethtool_get_settings() without producing any warning at compile
>time.  Why not put an ASSERT_RTNL() in it instead?

Hmm. Okay, then I would remove dev_ethtool_get_settings() from
net/core/dev.c and only put __ethtool_get_settings() to
net/core/ethtool.c. Makes more sense to me to have it there...
ASSERT_RTNL woudl be good there as well.

>
>The rest of this looks fine.
>
>Ben. 
>
>-- 
>Ben Hutchings, Staff Engineer, Solarflare
>Not speaking for my employer; that's the marketing department's job.
>They asked us to note that Solarflare product names are trademarked.
>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [patch net-next-2.6 v3] net: consolidate and fix ethtool_ops->get_settings calling
  2011-09-02 18:46     ` [Bridge] " Ben Hutchings
@ 2011-09-03 13:34       ` Jiri Pirko
  -1 siblings, 0 replies; 25+ messages in thread
From: Jiri Pirko @ 2011-09-03 13:34 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: netdev, ralf, fubar, andy, kaber, bprakash, JBottomley,
	robert.w.love, davem, shemminger, decot, mirq-linux,
	alexander.h.duyck, amit.salecha, eric.dumazet, therbert, paulmck,
	laijs, xiaosuo, greearb, loke.chetan, linux-mips, linux-scsi,
	devel, bridge

This patch does several things:
- introduces __ethtool_get_settings which is called from ethtool code and
  from drivers as well. Put ASSERT_RTNL there.
- dev_ethtool_get_settings() is replaced by __ethtool_get_settings()
- changes calling in drivers so rtnl locking is respected. In
  iboe_get_rate was previously ->get_settings() called unlocked. This
  fixes it. Also prb_calc_retire_blk_tmo() in af_packet.c had the same
  problem. Also fixed by calling __dev_get_by_index() instead of
  dev_get_by_index() and holding rtnl_lock for both calls.
- introduces rtnl_lock in bnx2fc_vport_create() and fcoe_vport_create()
  so bnx2fc_if_create() and fcoe_if_create() are called locked as they
  are from other places.
- use __ethtool_get_settings() in bonding code

Signed-off-by: Jiri Pirko <jpirko@redhat.com>

v2->v3:
	-removed dev_ethtool_get_settings()
	-added ASSERT_RTNL into __ethtool_get_settings()
	-prb_calc_retire_blk_tmo - use __dev_get_by_index() and lock
	 around it and __ethtool_get_settings() call
v1->v2:
        add missing export_symbol 
 
---
 arch/mips/txx9/generic/setup_tx4939.c |    2 +-
 drivers/net/bonding/bond_main.c       |   13 +++-----
 drivers/net/macvlan.c                 |    3 +-
 drivers/scsi/bnx2fc/bnx2fc_fcoe.c     |    4 ++-
 drivers/scsi/fcoe/fcoe.c              |    4 ++-
 include/linux/ethtool.h               |    3 ++
 include/linux/netdevice.h             |    3 --
 include/rdma/ib_addr.h                |    6 +++-
 net/8021q/vlan_dev.c                  |    3 +-
 net/bridge/br_if.c                    |    2 +-
 net/core/dev.c                        |   24 ---------------
 net/core/ethtool.c                    |   20 +++++++++---
 net/core/net-sysfs.c                  |    4 +-
 net/packet/af_packet.c                |   52 +++++++++++++++++----------------
 14 files changed, 69 insertions(+), 74 deletions(-)

diff --git a/arch/mips/txx9/generic/setup_tx4939.c b/arch/mips/txx9/generic/setup_tx4939.c
index e9f95dc..ba3cec3 100644
--- a/arch/mips/txx9/generic/setup_tx4939.c
+++ b/arch/mips/txx9/generic/setup_tx4939.c
@@ -321,7 +321,7 @@ void __init tx4939_sio_init(unsigned int sclk, unsigned int cts_mask)
 static u32 tx4939_get_eth_speed(struct net_device *dev)
 {
 	struct ethtool_cmd cmd;
-	if (dev_ethtool_get_settings(dev, &cmd))
+	if (__ethtool_get_settings(dev, &cmd))
 		return 100;	/* default 100Mbps */
 
 	return ethtool_cmd_speed(&cmd);
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 8cb75a6..1dcb07c 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -557,7 +557,7 @@ down:
 static int bond_update_speed_duplex(struct slave *slave)
 {
 	struct net_device *slave_dev = slave->dev;
-	struct ethtool_cmd etool = { .cmd = ETHTOOL_GSET };
+	struct ethtool_cmd ecmd;
 	u32 slave_speed;
 	int res;
 
@@ -565,18 +565,15 @@ static int bond_update_speed_duplex(struct slave *slave)
 	slave->speed = SPEED_100;
 	slave->duplex = DUPLEX_FULL;
 
-	if (!slave_dev->ethtool_ops || !slave_dev->ethtool_ops->get_settings)
-		return -1;
-
-	res = slave_dev->ethtool_ops->get_settings(slave_dev, &etool);
+	res = __ethtool_get_settings(slave_dev, &ecmd);
 	if (res < 0)
 		return -1;
 
-	slave_speed = ethtool_cmd_speed(&etool);
+	slave_speed = ethtool_cmd_speed(&ecmd);
 	if (slave_speed == 0 || slave_speed == ((__u32) -1))
 		return -1;
 
-	switch (etool.duplex) {
+	switch (ecmd.duplex) {
 	case DUPLEX_FULL:
 	case DUPLEX_HALF:
 		break;
@@ -585,7 +582,7 @@ static int bond_update_speed_duplex(struct slave *slave)
 	}
 
 	slave->speed = slave_speed;
-	slave->duplex = etool.duplex;
+	slave->duplex = ecmd.duplex;
 
 	return 0;
 }
diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index 836e13f..b100c90 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -543,7 +543,8 @@ static int macvlan_ethtool_get_settings(struct net_device *dev,
 					struct ethtool_cmd *cmd)
 {
 	const struct macvlan_dev *vlan = netdev_priv(dev);
-	return dev_ethtool_get_settings(vlan->lowerdev, cmd);
+
+	return __ethtool_get_settings(vlan->lowerdev, cmd);
 }
 
 static const struct ethtool_ops macvlan_ethtool_ops = {
diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
index 2c780a7..820a184 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
@@ -673,7 +673,7 @@ static void bnx2fc_link_speed_update(struct fc_lport *lport)
 	struct net_device *netdev = interface->netdev;
 	struct ethtool_cmd ecmd;
 
-	if (!dev_ethtool_get_settings(netdev, &ecmd)) {
+	if (!__ethtool_get_settings(netdev, &ecmd)) {
 		lport->link_supported_speeds &=
 			~(FC_PORTSPEED_1GBIT | FC_PORTSPEED_10GBIT);
 		if (ecmd.supported & (SUPPORTED_1000baseT_Half |
@@ -1001,9 +1001,11 @@ static int bnx2fc_vport_create(struct fc_vport *vport, bool disabled)
 			"this interface\n");
 		return -EIO;
 	}
+	rtnl_lock();
 	mutex_lock(&bnx2fc_dev_lock);
 	vn_port = bnx2fc_if_create(interface, &vport->dev, 1);
 	mutex_unlock(&bnx2fc_dev_lock);
+	rtnl_unlock();
 
 	if (IS_ERR(vn_port)) {
 		printk(KERN_ERR PFX "bnx2fc_vport_create (%s) failed\n",
diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index 3416ab6..83aa3ac 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -2043,7 +2043,7 @@ int fcoe_link_speed_update(struct fc_lport *lport)
 	struct net_device *netdev = fcoe_netdev(lport);
 	struct ethtool_cmd ecmd;
 
-	if (!dev_ethtool_get_settings(netdev, &ecmd)) {
+	if (!__ethtool_get_settings(netdev, &ecmd)) {
 		lport->link_supported_speeds &=
 			~(FC_PORTSPEED_1GBIT | FC_PORTSPEED_10GBIT);
 		if (ecmd.supported & (SUPPORTED_1000baseT_Half |
@@ -2452,7 +2452,9 @@ static int fcoe_vport_create(struct fc_vport *vport, bool disabled)
 	}
 
 	mutex_lock(&fcoe_config_mutex);
+	rtnl_lock();
 	vn_port = fcoe_if_create(fcoe, &vport->dev, 1);
+	rtnl_unlock();
 	mutex_unlock(&fcoe_config_mutex);
 
 	if (IS_ERR(vn_port)) {
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 3829712..8571f18 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -728,6 +728,9 @@ enum ethtool_sfeatures_retval_bits {
 /* needed by dev_disable_lro() */
 extern int __ethtool_set_flags(struct net_device *dev, u32 flags);
 
+extern int __ethtool_get_settings(struct net_device *dev,
+				  struct ethtool_cmd *cmd);
+
 /**
  * enum ethtool_phys_id_state - indicator state for physical identification
  * @ETHTOOL_ID_INACTIVE: Physical ID indicator should be deactivated
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index dad7e4d..8b1080b 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2600,9 +2600,6 @@ static inline int netif_is_bond_slave(struct net_device *dev)
 
 extern struct pernet_operations __net_initdata loopback_net_ops;
 
-int dev_ethtool_get_settings(struct net_device *dev,
-			     struct ethtool_cmd *cmd);
-
 static inline u32 dev_ethtool_get_rx_csum(struct net_device *dev)
 {
 	if (dev->features & NETIF_F_RXCSUM)
diff --git a/include/rdma/ib_addr.h b/include/rdma/ib_addr.h
index ae8c68f..639a449 100644
--- a/include/rdma/ib_addr.h
+++ b/include/rdma/ib_addr.h
@@ -218,8 +218,12 @@ static inline int iboe_get_rate(struct net_device *dev)
 {
 	struct ethtool_cmd cmd;
 	u32 speed;
+	int err;
 
-	if (dev_ethtool_get_settings(dev, &cmd))
+	rtnl_lock();
+	err = __ethtool_get_settings(dev, &cmd);
+	rtnl_unlock();
+	if (err)
 		return IB_RATE_PORT_CURRENT;
 
 	speed = ethtool_cmd_speed(&cmd);
diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index eba705b..c8cf939 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -610,7 +610,8 @@ static int vlan_ethtool_get_settings(struct net_device *dev,
 				     struct ethtool_cmd *cmd)
 {
 	const struct vlan_dev_info *vlan = vlan_dev_info(dev);
-	return dev_ethtool_get_settings(vlan->real_dev, cmd);
+
+	return __ethtool_get_settings(vlan->real_dev, cmd);
 }
 
 static void vlan_ethtool_get_drvinfo(struct net_device *dev,
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index b365bba..043a5eb 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -35,7 +35,7 @@ static int port_cost(struct net_device *dev)
 {
 	struct ethtool_cmd ecmd;
 
-	if (!dev_ethtool_get_settings(dev, &ecmd)) {
+	if (!__ethtool_get_settings(dev, &ecmd)) {
 		switch (ethtool_cmd_speed(&ecmd)) {
 		case SPEED_10000:
 			return 2;
diff --git a/net/core/dev.c b/net/core/dev.c
index 11b0fc7..94f3254 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4566,30 +4566,6 @@ void dev_set_rx_mode(struct net_device *dev)
 }
 
 /**
- *	dev_ethtool_get_settings - call device's ethtool_ops::get_settings()
- *	@dev: device
- *	@cmd: memory area for ethtool_ops::get_settings() result
- *
- *      The cmd arg is initialized properly (cleared and
- *      ethtool_cmd::cmd field set to ETHTOOL_GSET).
- *
- *	Return device's ethtool_ops::get_settings() result value or
- *	-EOPNOTSUPP when device doesn't expose
- *	ethtool_ops::get_settings() operation.
- */
-int dev_ethtool_get_settings(struct net_device *dev,
-			     struct ethtool_cmd *cmd)
-{
-	if (!dev->ethtool_ops || !dev->ethtool_ops->get_settings)
-		return -EOPNOTSUPP;
-
-	memset(cmd, 0, sizeof(struct ethtool_cmd));
-	cmd->cmd = ETHTOOL_GSET;
-	return dev->ethtool_ops->get_settings(dev, cmd);
-}
-EXPORT_SYMBOL(dev_ethtool_get_settings);
-
-/**
  *	dev_get_flags - get flags reported to userspace
  *	@dev: device
  *
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 6cdba5f..f444817 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -569,15 +569,25 @@ int __ethtool_set_flags(struct net_device *dev, u32 data)
 	return 0;
 }
 
-static int ethtool_get_settings(struct net_device *dev, void __user *useraddr)
+int __ethtool_get_settings(struct net_device *dev, struct ethtool_cmd *cmd)
 {
-	struct ethtool_cmd cmd = { .cmd = ETHTOOL_GSET };
-	int err;
+	ASSERT_RTNL();
 
-	if (!dev->ethtool_ops->get_settings)
+	if (!dev->ethtool_ops || !dev->ethtool_ops->get_settings)
 		return -EOPNOTSUPP;
 
-	err = dev->ethtool_ops->get_settings(dev, &cmd);
+	memset(cmd, 0, sizeof(struct ethtool_cmd));
+	cmd->cmd = ETHTOOL_GSET;
+	return dev->ethtool_ops->get_settings(dev, cmd);
+}
+EXPORT_SYMBOL(__ethtool_get_settings);
+
+static int ethtool_get_settings(struct net_device *dev, void __user *useraddr)
+{
+	int err;
+	struct ethtool_cmd cmd;
+
+	err = __ethtool_get_settings(dev, &cmd);
 	if (err < 0)
 		return err;
 
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index 90fdb46..48e6279 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -172,7 +172,7 @@ static ssize_t show_speed(struct device *dev,
 
 	if (netif_running(netdev)) {
 		struct ethtool_cmd cmd;
-		if (!dev_ethtool_get_settings(netdev, &cmd))
+		if (!__ethtool_get_settings(netdev, &cmd))
 			ret = sprintf(buf, fmt_udec, ethtool_cmd_speed(&cmd));
 	}
 	rtnl_unlock();
@@ -190,7 +190,7 @@ static ssize_t show_duplex(struct device *dev,
 
 	if (netif_running(netdev)) {
 		struct ethtool_cmd cmd;
-		if (!dev_ethtool_get_settings(netdev, &cmd))
+		if (!__ethtool_get_settings(netdev, &cmd))
 			ret = sprintf(buf, "%s\n",
 				      cmd.duplex ? "full" : "half");
 	}
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 2ea3d63..25e68f5 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -530,33 +530,35 @@ static int prb_calc_retire_blk_tmo(struct packet_sock *po,
 {
 	struct net_device *dev;
 	unsigned int mbits = 0, msec = 0, div = 0, tmo = 0;
+	struct ethtool_cmd ecmd;
+	int err;
 
-	dev = dev_get_by_index(sock_net(&po->sk), po->ifindex);
-	if (unlikely(dev == NULL))
+	rtnl_lock();
+	dev = __dev_get_by_index(sock_net(&po->sk), po->ifindex);
+	if (unlikely(!dev)) {
+		rtnl_unlock();
 		return DEFAULT_PRB_RETIRE_TOV;
-
-	if (dev->ethtool_ops && dev->ethtool_ops->get_settings) {
-		struct ethtool_cmd ecmd = { .cmd = ETHTOOL_GSET, };
-
-		if (!dev->ethtool_ops->get_settings(dev, &ecmd)) {
-			switch (ecmd.speed) {
-			case SPEED_10000:
-				msec = 1;
-				div = 10000/1000;
-				break;
-			case SPEED_1000:
-				msec = 1;
-				div = 1000/1000;
-				break;
-			/*
-			 * If the link speed is so slow you don't really
-			 * need to worry about perf anyways
-			 */
-			case SPEED_100:
-			case SPEED_10:
-			default:
-				return DEFAULT_PRB_RETIRE_TOV;
-			}
+	}
+	err = __ethtool_get_settings(dev, &ecmd);
+	rtnl_unlock();
+	if (!err) {
+		switch (ecmd.speed) {
+		case SPEED_10000:
+			msec = 1;
+			div = 10000/1000;
+			break;
+		case SPEED_1000:
+			msec = 1;
+			div = 1000/1000;
+			break;
+		/*
+		 * If the link speed is so slow you don't really
+		 * need to worry about perf anyways
+		 */
+		case SPEED_100:
+		case SPEED_10:
+		default:
+			return DEFAULT_PRB_RETIRE_TOV;
 		}
 	}
 
-- 
1.7.6

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [Bridge] [patch net-next-2.6 v3] net: consolidate and fix ethtool_ops->get_settings calling
@ 2011-09-03 13:34       ` Jiri Pirko
  0 siblings, 0 replies; 25+ messages in thread
From: Jiri Pirko @ 2011-09-03 13:34 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: amit.salecha, bridge, linux-mips, devel, linux-scsi, decot,
	therbert, robert.w.love, eric.dumazet, fubar, JBottomley,
	xiaosuo, paulmck, alexander.h.duyck, mirq-linux, greearb,
	bprakash, laijs, netdev, ralf, davem, loke.chetan

This patch does several things:
- introduces __ethtool_get_settings which is called from ethtool code and
  from drivers as well. Put ASSERT_RTNL there.
- dev_ethtool_get_settings() is replaced by __ethtool_get_settings()
- changes calling in drivers so rtnl locking is respected. In
  iboe_get_rate was previously ->get_settings() called unlocked. This
  fixes it. Also prb_calc_retire_blk_tmo() in af_packet.c had the same
  problem. Also fixed by calling __dev_get_by_index() instead of
  dev_get_by_index() and holding rtnl_lock for both calls.
- introduces rtnl_lock in bnx2fc_vport_create() and fcoe_vport_create()
  so bnx2fc_if_create() and fcoe_if_create() are called locked as they
  are from other places.
- use __ethtool_get_settings() in bonding code

Signed-off-by: Jiri Pirko <jpirko@redhat.com>

v2->v3:
	-removed dev_ethtool_get_settings()
	-added ASSERT_RTNL into __ethtool_get_settings()
	-prb_calc_retire_blk_tmo - use __dev_get_by_index() and lock
	 around it and __ethtool_get_settings() call
v1->v2:
        add missing export_symbol 
 
---
 arch/mips/txx9/generic/setup_tx4939.c |    2 +-
 drivers/net/bonding/bond_main.c       |   13 +++-----
 drivers/net/macvlan.c                 |    3 +-
 drivers/scsi/bnx2fc/bnx2fc_fcoe.c     |    4 ++-
 drivers/scsi/fcoe/fcoe.c              |    4 ++-
 include/linux/ethtool.h               |    3 ++
 include/linux/netdevice.h             |    3 --
 include/rdma/ib_addr.h                |    6 +++-
 net/8021q/vlan_dev.c                  |    3 +-
 net/bridge/br_if.c                    |    2 +-
 net/core/dev.c                        |   24 ---------------
 net/core/ethtool.c                    |   20 +++++++++---
 net/core/net-sysfs.c                  |    4 +-
 net/packet/af_packet.c                |   52 +++++++++++++++++----------------
 14 files changed, 69 insertions(+), 74 deletions(-)

diff --git a/arch/mips/txx9/generic/setup_tx4939.c b/arch/mips/txx9/generic/setup_tx4939.c
index e9f95dc..ba3cec3 100644
--- a/arch/mips/txx9/generic/setup_tx4939.c
+++ b/arch/mips/txx9/generic/setup_tx4939.c
@@ -321,7 +321,7 @@ void __init tx4939_sio_init(unsigned int sclk, unsigned int cts_mask)
 static u32 tx4939_get_eth_speed(struct net_device *dev)
 {
 	struct ethtool_cmd cmd;
-	if (dev_ethtool_get_settings(dev, &cmd))
+	if (__ethtool_get_settings(dev, &cmd))
 		return 100;	/* default 100Mbps */
 
 	return ethtool_cmd_speed(&cmd);
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 8cb75a6..1dcb07c 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -557,7 +557,7 @@ down:
 static int bond_update_speed_duplex(struct slave *slave)
 {
 	struct net_device *slave_dev = slave->dev;
-	struct ethtool_cmd etool = { .cmd = ETHTOOL_GSET };
+	struct ethtool_cmd ecmd;
 	u32 slave_speed;
 	int res;
 
@@ -565,18 +565,15 @@ static int bond_update_speed_duplex(struct slave *slave)
 	slave->speed = SPEED_100;
 	slave->duplex = DUPLEX_FULL;
 
-	if (!slave_dev->ethtool_ops || !slave_dev->ethtool_ops->get_settings)
-		return -1;
-
-	res = slave_dev->ethtool_ops->get_settings(slave_dev, &etool);
+	res = __ethtool_get_settings(slave_dev, &ecmd);
 	if (res < 0)
 		return -1;
 
-	slave_speed = ethtool_cmd_speed(&etool);
+	slave_speed = ethtool_cmd_speed(&ecmd);
 	if (slave_speed == 0 || slave_speed == ((__u32) -1))
 		return -1;
 
-	switch (etool.duplex) {
+	switch (ecmd.duplex) {
 	case DUPLEX_FULL:
 	case DUPLEX_HALF:
 		break;
@@ -585,7 +582,7 @@ static int bond_update_speed_duplex(struct slave *slave)
 	}
 
 	slave->speed = slave_speed;
-	slave->duplex = etool.duplex;
+	slave->duplex = ecmd.duplex;
 
 	return 0;
 }
diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index 836e13f..b100c90 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -543,7 +543,8 @@ static int macvlan_ethtool_get_settings(struct net_device *dev,
 					struct ethtool_cmd *cmd)
 {
 	const struct macvlan_dev *vlan = netdev_priv(dev);
-	return dev_ethtool_get_settings(vlan->lowerdev, cmd);
+
+	return __ethtool_get_settings(vlan->lowerdev, cmd);
 }
 
 static const struct ethtool_ops macvlan_ethtool_ops = {
diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
index 2c780a7..820a184 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
@@ -673,7 +673,7 @@ static void bnx2fc_link_speed_update(struct fc_lport *lport)
 	struct net_device *netdev = interface->netdev;
 	struct ethtool_cmd ecmd;
 
-	if (!dev_ethtool_get_settings(netdev, &ecmd)) {
+	if (!__ethtool_get_settings(netdev, &ecmd)) {
 		lport->link_supported_speeds &=
 			~(FC_PORTSPEED_1GBIT | FC_PORTSPEED_10GBIT);
 		if (ecmd.supported & (SUPPORTED_1000baseT_Half |
@@ -1001,9 +1001,11 @@ static int bnx2fc_vport_create(struct fc_vport *vport, bool disabled)
 			"this interface\n");
 		return -EIO;
 	}
+	rtnl_lock();
 	mutex_lock(&bnx2fc_dev_lock);
 	vn_port = bnx2fc_if_create(interface, &vport->dev, 1);
 	mutex_unlock(&bnx2fc_dev_lock);
+	rtnl_unlock();
 
 	if (IS_ERR(vn_port)) {
 		printk(KERN_ERR PFX "bnx2fc_vport_create (%s) failed\n",
diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index 3416ab6..83aa3ac 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -2043,7 +2043,7 @@ int fcoe_link_speed_update(struct fc_lport *lport)
 	struct net_device *netdev = fcoe_netdev(lport);
 	struct ethtool_cmd ecmd;
 
-	if (!dev_ethtool_get_settings(netdev, &ecmd)) {
+	if (!__ethtool_get_settings(netdev, &ecmd)) {
 		lport->link_supported_speeds &=
 			~(FC_PORTSPEED_1GBIT | FC_PORTSPEED_10GBIT);
 		if (ecmd.supported & (SUPPORTED_1000baseT_Half |
@@ -2452,7 +2452,9 @@ static int fcoe_vport_create(struct fc_vport *vport, bool disabled)
 	}
 
 	mutex_lock(&fcoe_config_mutex);
+	rtnl_lock();
 	vn_port = fcoe_if_create(fcoe, &vport->dev, 1);
+	rtnl_unlock();
 	mutex_unlock(&fcoe_config_mutex);
 
 	if (IS_ERR(vn_port)) {
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index 3829712..8571f18 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -728,6 +728,9 @@ enum ethtool_sfeatures_retval_bits {
 /* needed by dev_disable_lro() */
 extern int __ethtool_set_flags(struct net_device *dev, u32 flags);
 
+extern int __ethtool_get_settings(struct net_device *dev,
+				  struct ethtool_cmd *cmd);
+
 /**
  * enum ethtool_phys_id_state - indicator state for physical identification
  * @ETHTOOL_ID_INACTIVE: Physical ID indicator should be deactivated
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index dad7e4d..8b1080b 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2600,9 +2600,6 @@ static inline int netif_is_bond_slave(struct net_device *dev)
 
 extern struct pernet_operations __net_initdata loopback_net_ops;
 
-int dev_ethtool_get_settings(struct net_device *dev,
-			     struct ethtool_cmd *cmd);
-
 static inline u32 dev_ethtool_get_rx_csum(struct net_device *dev)
 {
 	if (dev->features & NETIF_F_RXCSUM)
diff --git a/include/rdma/ib_addr.h b/include/rdma/ib_addr.h
index ae8c68f..639a449 100644
--- a/include/rdma/ib_addr.h
+++ b/include/rdma/ib_addr.h
@@ -218,8 +218,12 @@ static inline int iboe_get_rate(struct net_device *dev)
 {
 	struct ethtool_cmd cmd;
 	u32 speed;
+	int err;
 
-	if (dev_ethtool_get_settings(dev, &cmd))
+	rtnl_lock();
+	err = __ethtool_get_settings(dev, &cmd);
+	rtnl_unlock();
+	if (err)
 		return IB_RATE_PORT_CURRENT;
 
 	speed = ethtool_cmd_speed(&cmd);
diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index eba705b..c8cf939 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -610,7 +610,8 @@ static int vlan_ethtool_get_settings(struct net_device *dev,
 				     struct ethtool_cmd *cmd)
 {
 	const struct vlan_dev_info *vlan = vlan_dev_info(dev);
-	return dev_ethtool_get_settings(vlan->real_dev, cmd);
+
+	return __ethtool_get_settings(vlan->real_dev, cmd);
 }
 
 static void vlan_ethtool_get_drvinfo(struct net_device *dev,
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index b365bba..043a5eb 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -35,7 +35,7 @@ static int port_cost(struct net_device *dev)
 {
 	struct ethtool_cmd ecmd;
 
-	if (!dev_ethtool_get_settings(dev, &ecmd)) {
+	if (!__ethtool_get_settings(dev, &ecmd)) {
 		switch (ethtool_cmd_speed(&ecmd)) {
 		case SPEED_10000:
 			return 2;
diff --git a/net/core/dev.c b/net/core/dev.c
index 11b0fc7..94f3254 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4566,30 +4566,6 @@ void dev_set_rx_mode(struct net_device *dev)
 }
 
 /**
- *	dev_ethtool_get_settings - call device's ethtool_ops::get_settings()
- *	@dev: device
- *	@cmd: memory area for ethtool_ops::get_settings() result
- *
- *      The cmd arg is initialized properly (cleared and
- *      ethtool_cmd::cmd field set to ETHTOOL_GSET).
- *
- *	Return device's ethtool_ops::get_settings() result value or
- *	-EOPNOTSUPP when device doesn't expose
- *	ethtool_ops::get_settings() operation.
- */
-int dev_ethtool_get_settings(struct net_device *dev,
-			     struct ethtool_cmd *cmd)
-{
-	if (!dev->ethtool_ops || !dev->ethtool_ops->get_settings)
-		return -EOPNOTSUPP;
-
-	memset(cmd, 0, sizeof(struct ethtool_cmd));
-	cmd->cmd = ETHTOOL_GSET;
-	return dev->ethtool_ops->get_settings(dev, cmd);
-}
-EXPORT_SYMBOL(dev_ethtool_get_settings);
-
-/**
  *	dev_get_flags - get flags reported to userspace
  *	@dev: device
  *
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 6cdba5f..f444817 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -569,15 +569,25 @@ int __ethtool_set_flags(struct net_device *dev, u32 data)
 	return 0;
 }
 
-static int ethtool_get_settings(struct net_device *dev, void __user *useraddr)
+int __ethtool_get_settings(struct net_device *dev, struct ethtool_cmd *cmd)
 {
-	struct ethtool_cmd cmd = { .cmd = ETHTOOL_GSET };
-	int err;
+	ASSERT_RTNL();
 
-	if (!dev->ethtool_ops->get_settings)
+	if (!dev->ethtool_ops || !dev->ethtool_ops->get_settings)
 		return -EOPNOTSUPP;
 
-	err = dev->ethtool_ops->get_settings(dev, &cmd);
+	memset(cmd, 0, sizeof(struct ethtool_cmd));
+	cmd->cmd = ETHTOOL_GSET;
+	return dev->ethtool_ops->get_settings(dev, cmd);
+}
+EXPORT_SYMBOL(__ethtool_get_settings);
+
+static int ethtool_get_settings(struct net_device *dev, void __user *useraddr)
+{
+	int err;
+	struct ethtool_cmd cmd;
+
+	err = __ethtool_get_settings(dev, &cmd);
 	if (err < 0)
 		return err;
 
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index 90fdb46..48e6279 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -172,7 +172,7 @@ static ssize_t show_speed(struct device *dev,
 
 	if (netif_running(netdev)) {
 		struct ethtool_cmd cmd;
-		if (!dev_ethtool_get_settings(netdev, &cmd))
+		if (!__ethtool_get_settings(netdev, &cmd))
 			ret = sprintf(buf, fmt_udec, ethtool_cmd_speed(&cmd));
 	}
 	rtnl_unlock();
@@ -190,7 +190,7 @@ static ssize_t show_duplex(struct device *dev,
 
 	if (netif_running(netdev)) {
 		struct ethtool_cmd cmd;
-		if (!dev_ethtool_get_settings(netdev, &cmd))
+		if (!__ethtool_get_settings(netdev, &cmd))
 			ret = sprintf(buf, "%s\n",
 				      cmd.duplex ? "full" : "half");
 	}
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 2ea3d63..25e68f5 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -530,33 +530,35 @@ static int prb_calc_retire_blk_tmo(struct packet_sock *po,
 {
 	struct net_device *dev;
 	unsigned int mbits = 0, msec = 0, div = 0, tmo = 0;
+	struct ethtool_cmd ecmd;
+	int err;
 
-	dev = dev_get_by_index(sock_net(&po->sk), po->ifindex);
-	if (unlikely(dev == NULL))
+	rtnl_lock();
+	dev = __dev_get_by_index(sock_net(&po->sk), po->ifindex);
+	if (unlikely(!dev)) {
+		rtnl_unlock();
 		return DEFAULT_PRB_RETIRE_TOV;
-
-	if (dev->ethtool_ops && dev->ethtool_ops->get_settings) {
-		struct ethtool_cmd ecmd = { .cmd = ETHTOOL_GSET, };
-
-		if (!dev->ethtool_ops->get_settings(dev, &ecmd)) {
-			switch (ecmd.speed) {
-			case SPEED_10000:
-				msec = 1;
-				div = 10000/1000;
-				break;
-			case SPEED_1000:
-				msec = 1;
-				div = 1000/1000;
-				break;
-			/*
-			 * If the link speed is so slow you don't really
-			 * need to worry about perf anyways
-			 */
-			case SPEED_100:
-			case SPEED_10:
-			default:
-				return DEFAULT_PRB_RETIRE_TOV;
-			}
+	}
+	err = __ethtool_get_settings(dev, &ecmd);
+	rtnl_unlock();
+	if (!err) {
+		switch (ecmd.speed) {
+		case SPEED_10000:
+			msec = 1;
+			div = 10000/1000;
+			break;
+		case SPEED_1000:
+			msec = 1;
+			div = 1000/1000;
+			break;
+		/*
+		 * If the link speed is so slow you don't really
+		 * need to worry about perf anyways
+		 */
+		case SPEED_100:
+		case SPEED_10:
+		default:
+			return DEFAULT_PRB_RETIRE_TOV;
 		}
 	}
 
-- 
1.7.6


^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [patch net-next-2.6 v3] net: consolidate and fix ethtool_ops->get_settings calling
  2011-09-03 13:34       ` [Bridge] " Jiri Pirko
@ 2011-09-03 13:46         ` Ben Hutchings
  -1 siblings, 0 replies; 25+ messages in thread
From: Ben Hutchings @ 2011-09-03 13:46 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: netdev, ralf, fubar, andy, kaber, bprakash, JBottomley,
	robert.w.love, davem, shemminger, decot, mirq-linux,
	alexander.h.duyck, amit.salecha, eric.dumazet, therbert, paulmck,
	laijs, xiaosuo, greearb, loke.chetan, linux-mips, linux-scsi,
	devel, bridge

On Sat, 2011-09-03 at 15:34 +0200, Jiri Pirko wrote:
> This patch does several things:
> - introduces __ethtool_get_settings which is called from ethtool code and
>   from drivers as well. Put ASSERT_RTNL there.
> - dev_ethtool_get_settings() is replaced by __ethtool_get_settings()
> - changes calling in drivers so rtnl locking is respected. In
>   iboe_get_rate was previously ->get_settings() called unlocked. This
>   fixes it. Also prb_calc_retire_blk_tmo() in af_packet.c had the same
>   problem. Also fixed by calling __dev_get_by_index() instead of
>   dev_get_by_index() and holding rtnl_lock for both calls.
> - introduces rtnl_lock in bnx2fc_vport_create() and fcoe_vport_create()
>   so bnx2fc_if_create() and fcoe_if_create() are called locked as they
>   are from other places.
> - use __ethtool_get_settings() in bonding code
> 
> Signed-off-by: Jiri Pirko <jpirko@redhat.com>
Reviewed-by: Ben Hutchings <bhutchings@solarflare.com> [except FCoE bits]

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Bridge] [patch net-next-2.6 v3] net: consolidate and fix ethtool_ops->get_settings calling
@ 2011-09-03 13:46         ` Ben Hutchings
  0 siblings, 0 replies; 25+ messages in thread
From: Ben Hutchings @ 2011-09-03 13:46 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: amit.salecha, bridge, linux-mips, devel, linux-scsi, decot,
	therbert, robert.w.love, eric.dumazet, fubar, JBottomley,
	xiaosuo, paulmck, alexander.h.duyck, mirq-linux, greearb,
	bprakash, laijs, netdev, ralf, davem, loke.chetan

On Sat, 2011-09-03 at 15:34 +0200, Jiri Pirko wrote:
> This patch does several things:
> - introduces __ethtool_get_settings which is called from ethtool code and
>   from drivers as well. Put ASSERT_RTNL there.
> - dev_ethtool_get_settings() is replaced by __ethtool_get_settings()
> - changes calling in drivers so rtnl locking is respected. In
>   iboe_get_rate was previously ->get_settings() called unlocked. This
>   fixes it. Also prb_calc_retire_blk_tmo() in af_packet.c had the same
>   problem. Also fixed by calling __dev_get_by_index() instead of
>   dev_get_by_index() and holding rtnl_lock for both calls.
> - introduces rtnl_lock in bnx2fc_vport_create() and fcoe_vport_create()
>   so bnx2fc_if_create() and fcoe_if_create() are called locked as they
>   are from other places.
> - use __ethtool_get_settings() in bonding code
> 
> Signed-off-by: Jiri Pirko <jpirko@redhat.com>
Reviewed-by: Ben Hutchings <bhutchings@solarflare.com> [except FCoE bits]

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [patch net-next-2.6 v3] net: consolidate and fix ethtool_ops->get_settings calling
  2011-09-03 13:46         ` [Bridge] " Ben Hutchings
                             ` (2 preceding siblings ...)
  (?)
@ 2011-09-06  3:25           ` Zou, Yi
  -1 siblings, 0 replies; 25+ messages in thread
From: Zou, Yi @ 2011-09-06  3:25 UTC (permalink / raw)
  To: Ben Hutchings, Jiri Pirko
  Cc: amit.salecha-h88ZbnxC6KDQT0dZR+AlfA,
	bridge-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-mips-6z/3iImG2C8G8FEW9MqTrA,
	JBottomley-bzQdu9zFT3WakBO8gow8eQ,
	linux-scsi-u79uwXL29TY76Z2rM5mHXA, decot-hpIqsD4AKlfQT0dZR+AlfA,
	shemminger-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
	andy-QlMahl40kYEqcZcGjlUOXw, therbert-hpIqsD4AKlfQT0dZR+AlfA,
	eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w,
	fubar-r/Jw6+rmf7HQT0dZR+AlfA, xiaosuo-Re5JQEeQqe8AvxtiuMwx3w,
	paulmck-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8, Duyck, Alexander H,
	mirq-linux-CoA6ZxLDdyEEUmgCuDUIdw,
	greearb-my8/4N5VtI7c+919tysfdA,
	laijs-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org

> 
> On Sat, 2011-09-03 at 15:34 +0200, Jiri Pirko wrote:
> > This patch does several things:
> > - introduces __ethtool_get_settings which is called from ethtool code
> and
> >   from drivers as well. Put ASSERT_RTNL there.
> > - dev_ethtool_get_settings() is replaced by __ethtool_get_settings()
> > - changes calling in drivers so rtnl locking is respected. In
> >   iboe_get_rate was previously ->get_settings() called unlocked. This
> >   fixes it. Also prb_calc_retire_blk_tmo() in af_packet.c had the same
> >   problem. Also fixed by calling __dev_get_by_index() instead of
> >   dev_get_by_index() and holding rtnl_lock for both calls.
> > - introduces rtnl_lock in bnx2fc_vport_create() and fcoe_vport_create()
> >   so bnx2fc_if_create() and fcoe_if_create() are called locked as they
> >   are from other places.
> > - use __ethtool_get_settings() in bonding code
> >
> > Signed-off-by: Jiri Pirko <jpirko-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Reviewed-by: Ben Hutchings <bhutchings-s/n/eUQHGBpZroRs9YW3xA@public.gmane.org> [except FCoE bits]
> 
> Ben.
FCoE bits look ok to me. Thanks,

Reviewed-by: Yi Zou <yi.zou-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

> 
> --
> Ben Hutchings, Staff Engineer, Solarflare
> Not speaking for my employer; that's the marketing department's job.
> They asked us to note that Solarflare product names are trademarked.
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 25+ messages in thread

* RE: [patch net-next-2.6 v3] net: consolidate and fix ethtool_ops->get_settings calling
@ 2011-09-06  3:25           ` Zou, Yi
  0 siblings, 0 replies; 25+ messages in thread
From: Zou, Yi @ 2011-09-06  3:25 UTC (permalink / raw)
  To: Ben Hutchings, Jiri Pirko
  Cc: netdev, ralf, fubar, andy, kaber, bprakash, JBottomley, Love,
	Robert W, davem, shemminger, decot, mirq-linux, Duyck,
	Alexander H, amit.salecha, eric.dumazet, therbert, paulmck,
	laijs, xiaosuo, greearb, loke.chetan, linux-mips, linux-scsi,
	devel, bridge

> 
> On Sat, 2011-09-03 at 15:34 +0200, Jiri Pirko wrote:
> > This patch does several things:
> > - introduces __ethtool_get_settings which is called from ethtool code
> and
> >   from drivers as well. Put ASSERT_RTNL there.
> > - dev_ethtool_get_settings() is replaced by __ethtool_get_settings()
> > - changes calling in drivers so rtnl locking is respected. In
> >   iboe_get_rate was previously ->get_settings() called unlocked. This
> >   fixes it. Also prb_calc_retire_blk_tmo() in af_packet.c had the same
> >   problem. Also fixed by calling __dev_get_by_index() instead of
> >   dev_get_by_index() and holding rtnl_lock for both calls.
> > - introduces rtnl_lock in bnx2fc_vport_create() and fcoe_vport_create()
> >   so bnx2fc_if_create() and fcoe_if_create() are called locked as they
> >   are from other places.
> > - use __ethtool_get_settings() in bonding code
> >
> > Signed-off-by: Jiri Pirko <jpirko@redhat.com>
> Reviewed-by: Ben Hutchings <bhutchings@solarflare.com> [except FCoE bits]
> 
> Ben.
FCoE bits look ok to me. Thanks,

Reviewed-by: Yi Zou <yi.zou@intel.com>

> 
> --
> Ben Hutchings, Staff Engineer, Solarflare
> Not speaking for my employer; that's the marketing department's job.
> They asked us to note that Solarflare product names are trademarked.
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 25+ messages in thread

* RE: [patch net-next-2.6 v3] net: consolidate and fix ethtool_ops->get_settings calling
@ 2011-09-06  3:25           ` Zou, Yi
  0 siblings, 0 replies; 25+ messages in thread
From: Zou, Yi @ 2011-09-06  3:25 UTC (permalink / raw)
  To: Ben Hutchings, Jiri Pirko
  Cc: netdev, ralf, fubar, andy, kaber, bprakash, JBottomley, Love,
	Robert W, davem, shemminger, decot, mirq-linux, Duyck,
	Alexander H, amit.salecha, eric.dumazet, therbert, paulmck,
	laijs, xiaosuo, greearb, loke.chetan, linux-mips, linux-scsi,
	devel, bridge

> 
> On Sat, 2011-09-03 at 15:34 +0200, Jiri Pirko wrote:
> > This patch does several things:
> > - introduces __ethtool_get_settings which is called from ethtool code
> and
> >   from drivers as well. Put ASSERT_RTNL there.
> > - dev_ethtool_get_settings() is replaced by __ethtool_get_settings()
> > - changes calling in drivers so rtnl locking is respected. In
> >   iboe_get_rate was previously ->get_settings() called unlocked. This
> >   fixes it. Also prb_calc_retire_blk_tmo() in af_packet.c had the same
> >   problem. Also fixed by calling __dev_get_by_index() instead of
> >   dev_get_by_index() and holding rtnl_lock for both calls.
> > - introduces rtnl_lock in bnx2fc_vport_create() and fcoe_vport_create()
> >   so bnx2fc_if_create() and fcoe_if_create() are called locked as they
> >   are from other places.
> > - use __ethtool_get_settings() in bonding code
> >
> > Signed-off-by: Jiri Pirko <jpirko@redhat.com>
> Reviewed-by: Ben Hutchings <bhutchings@solarflare.com> [except FCoE bits]
> 
> Ben.
FCoE bits look ok to me. Thanks,

Reviewed-by: Yi Zou <yi.zou@intel.com>

> 
> --
> Ben Hutchings, Staff Engineer, Solarflare
> Not speaking for my employer; that's the marketing department's job.
> They asked us to note that Solarflare product names are trademarked.
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [patch net-next-2.6 v3] net: consolidate and fix ethtool_ops->get_settings calling
@ 2011-09-06  3:25           ` Zou, Yi
  0 siblings, 0 replies; 25+ messages in thread
From: Zou, Yi @ 2011-09-06  3:25 UTC (permalink / raw)
  To: Ben Hutchings, Jiri Pirko
  Cc: amit.salecha-h88ZbnxC6KDQT0dZR+AlfA,
	bridge-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-mips-6z/3iImG2C8G8FEW9MqTrA,
	JBottomley-bzQdu9zFT3WakBO8gow8eQ,
	linux-scsi-u79uwXL29TY76Z2rM5mHXA, decot-hpIqsD4AKlfQT0dZR+AlfA,
	shemminger-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
	andy-QlMahl40kYEqcZcGjlUOXw, therbert-hpIqsD4AKlfQT0dZR+AlfA,
	eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w,
	fubar-r/Jw6+rmf7HQT0dZR+AlfA, xiaosuo-Re5JQEeQqe8AvxtiuMwx3w,
	paulmck-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8, Duyck, Alexander H,
	mirq-linux-CoA6ZxLDdyEEUmgCuDUIdw,
	greearb-my8/4N5VtI7c+919tysfdA,
	laijs-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org

> 
> On Sat, 2011-09-03 at 15:34 +0200, Jiri Pirko wrote:
> > This patch does several things:
> > - introduces __ethtool_get_settings which is called from ethtool code
> and
> >   from drivers as well. Put ASSERT_RTNL there.
> > - dev_ethtool_get_settings() is replaced by __ethtool_get_settings()
> > - changes calling in drivers so rtnl locking is respected. In
> >   iboe_get_rate was previously ->get_settings() called unlocked. This
> >   fixes it. Also prb_calc_retire_blk_tmo() in af_packet.c had the same
> >   problem. Also fixed by calling __dev_get_by_index() instead of
> >   dev_get_by_index() and holding rtnl_lock for both calls.
> > - introduces rtnl_lock in bnx2fc_vport_create() and fcoe_vport_create()
> >   so bnx2fc_if_create() and fcoe_if_create() are called locked as they
> >   are from other places.
> > - use __ethtool_get_settings() in bonding code
> >
> > Signed-off-by: Jiri Pirko <jpirko-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Reviewed-by: Ben Hutchings <bhutchings-s/n/eUQHGBpZroRs9YW3xA@public.gmane.org> [except FCoE bits]
> 
> Ben.
FCoE bits look ok to me. Thanks,

Reviewed-by: Yi Zou <yi.zou-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

> 
> --
> Ben Hutchings, Staff Engineer, Solarflare
> Not speaking for my employer; that's the marketing department's job.
> They asked us to note that Solarflare product names are trademarked.
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Bridge] [patch net-next-2.6 v3] net: consolidate and fix ethtool_ops->get_settings calling
@ 2011-09-06  3:25           ` Zou, Yi
  0 siblings, 0 replies; 25+ messages in thread
From: Zou, Yi @ 2011-09-06  3:25 UTC (permalink / raw)
  To: Ben Hutchings, Jiri Pirko
  Cc: amit.salecha, bridge, linux-mips, devel, linux-scsi, decot,
	therbert, Love, Robert W, eric.dumazet, fubar, JBottomley,
	xiaosuo, paulmck, Duyck, Alexander H, mirq-linux, greearb,
	bprakash, laijs, netdev, ralf, davem, loke.chetan

> 
> On Sat, 2011-09-03 at 15:34 +0200, Jiri Pirko wrote:
> > This patch does several things:
> > - introduces __ethtool_get_settings which is called from ethtool code
> and
> >   from drivers as well. Put ASSERT_RTNL there.
> > - dev_ethtool_get_settings() is replaced by __ethtool_get_settings()
> > - changes calling in drivers so rtnl locking is respected. In
> >   iboe_get_rate was previously ->get_settings() called unlocked. This
> >   fixes it. Also prb_calc_retire_blk_tmo() in af_packet.c had the same
> >   problem. Also fixed by calling __dev_get_by_index() instead of
> >   dev_get_by_index() and holding rtnl_lock for both calls.
> > - introduces rtnl_lock in bnx2fc_vport_create() and fcoe_vport_create()
> >   so bnx2fc_if_create() and fcoe_if_create() are called locked as they
> >   are from other places.
> > - use __ethtool_get_settings() in bonding code
> >
> > Signed-off-by: Jiri Pirko <jpirko@redhat.com>
> Reviewed-by: Ben Hutchings <bhutchings@solarflare.com> [except FCoE bits]
> 
> Ben.
FCoE bits look ok to me. Thanks,

Reviewed-by: Yi Zou <yi.zou@intel.com>

> 
> --
> Ben Hutchings, Staff Engineer, Solarflare
> Not speaking for my employer; that's the marketing department's job.
> They asked us to note that Solarflare product names are trademarked.
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [patch net-next-2.6 v3] net: consolidate and fix ethtool_ops->get_settings calling
  2011-09-06  3:25           ` Zou, Yi
                                 ` (2 preceding siblings ...)
  (?)
@ 2011-09-06  6:52               ` Bhanu Prakash Gollapudi
  -1 siblings, 0 replies; 25+ messages in thread
From: Bhanu Prakash Gollapudi @ 2011-09-06  6:52 UTC (permalink / raw)
  To: Zou, Yi
  Cc: amit.salecha-h88ZbnxC6KDQT0dZR+AlfA,
	bridge-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-mips-6z/3iImG2C8G8FEW9MqTrA,
	JBottomley-bzQdu9zFT3WakBO8gow8eQ,
	linux-scsi-u79uwXL29TY76Z2rM5mHXA, decot-hpIqsD4AKlfQT0dZR+AlfA,
	shemminger-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
	andy-QlMahl40kYEqcZcGjlUOXw, therbert-hpIqsD4AKlfQT0dZR+AlfA,
	eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w,
	fubar-r/Jw6+rmf7HQT0dZR+AlfA, xiaosuo-Re5JQEeQqe8AvxtiuMwx3w,
	paulmck-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8, Duyck, Alexander H,
	mirq-linux-CoA6ZxLDdyEEUmgCuDUIdw, Ben Hutchings,
	greearb-my8/4N5VtI7c+919tysfdA@public.gmane.org

On 9/5/2011 8:25 PM, Zou, Yi wrote:
>>
>> On Sat, 2011-09-03 at 15:34 +0200, Jiri Pirko wrote:
>>> This patch does several things:
>>> - introduces __ethtool_get_settings which is called from ethtool code
>> and
>>>    from drivers as well. Put ASSERT_RTNL there.
>>> - dev_ethtool_get_settings() is replaced by __ethtool_get_settings()
>>> - changes calling in drivers so rtnl locking is respected. In
>>>    iboe_get_rate was previously ->get_settings() called unlocked. This
>>>    fixes it. Also prb_calc_retire_blk_tmo() in af_packet.c had the same
>>>    problem. Also fixed by calling __dev_get_by_index() instead of
>>>    dev_get_by_index() and holding rtnl_lock for both calls.
>>> - introduces rtnl_lock in bnx2fc_vport_create() and fcoe_vport_create()
>>>    so bnx2fc_if_create() and fcoe_if_create() are called locked as they
>>>    are from other places.
>>> - use __ethtool_get_settings() in bonding code
>>>
>>> Signed-off-by: Jiri Pirko<jpirko-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>> Reviewed-by: Ben Hutchings<bhutchings-s/n/eUQHGBpZroRs9YW3xA@public.gmane.org>  [except FCoE bits]
>>
>> Ben.
> FCoE bits look ok to me. Thanks,
>
> Reviewed-by: Yi Zou<yi.zou-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

bnx2fc changes looks OK to me.
Reviewed-by: Bhanu Prakash Gollapudi <bprakash-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>
>>
>> --
>> Ben Hutchings, Staff Engineer, Solarflare
>> Not speaking for my employer; that's the marketing department's job.
>> They asked us to note that Solarflare product names are trademarked.
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [patch net-next-2.6 v3] net: consolidate and fix ethtool_ops->get_settings calling
@ 2011-09-06  6:52               ` Bhanu Prakash Gollapudi
  0 siblings, 0 replies; 25+ messages in thread
From: Bhanu Prakash Gollapudi @ 2011-09-06  6:52 UTC (permalink / raw)
  To: Zou, Yi
  Cc: Ben Hutchings, Jiri Pirko, netdev, ralf, fubar, andy, kaber,
	JBottomley, Love, Robert W, davem, shemminger, decot, mirq-linux,
	Duyck, Alexander H, amit.salecha, eric.dumazet, therbert,
	paulmck, laijs, xiaosuo, greearb, loke.chetan, linux-mips,
	linux-scsi, devel, bridge

On 9/5/2011 8:25 PM, Zou, Yi wrote:
>>
>> On Sat, 2011-09-03 at 15:34 +0200, Jiri Pirko wrote:
>>> This patch does several things:
>>> - introduces __ethtool_get_settings which is called from ethtool code
>> and
>>>    from drivers as well. Put ASSERT_RTNL there.
>>> - dev_ethtool_get_settings() is replaced by __ethtool_get_settings()
>>> - changes calling in drivers so rtnl locking is respected. In
>>>    iboe_get_rate was previously ->get_settings() called unlocked. This
>>>    fixes it. Also prb_calc_retire_blk_tmo() in af_packet.c had the same
>>>    problem. Also fixed by calling __dev_get_by_index() instead of
>>>    dev_get_by_index() and holding rtnl_lock for both calls.
>>> - introduces rtnl_lock in bnx2fc_vport_create() and fcoe_vport_create()
>>>    so bnx2fc_if_create() and fcoe_if_create() are called locked as they
>>>    are from other places.
>>> - use __ethtool_get_settings() in bonding code
>>>
>>> Signed-off-by: Jiri Pirko<jpirko@redhat.com>
>> Reviewed-by: Ben Hutchings<bhutchings@solarflare.com>  [except FCoE bits]
>>
>> Ben.
> FCoE bits look ok to me. Thanks,
>
> Reviewed-by: Yi Zou<yi.zou@intel.com>

bnx2fc changes looks OK to me.
Reviewed-by: Bhanu Prakash Gollapudi <bprakash@broadcom.com>
>
>>
>> --
>> Ben Hutchings, Staff Engineer, Solarflare
>> Not speaking for my employer; that's the marketing department's job.
>> They asked us to note that Solarflare product names are trademarked.
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [patch net-next-2.6 v3] net: consolidate and fix ethtool_ops->get_settings calling
@ 2011-09-06  6:52               ` Bhanu Prakash Gollapudi
  0 siblings, 0 replies; 25+ messages in thread
From: Bhanu Prakash Gollapudi @ 2011-09-06  6:52 UTC (permalink / raw)
  To: Zou, Yi
  Cc: Ben Hutchings, Jiri Pirko, netdev, ralf, fubar, andy, kaber,
	JBottomley, Love, Robert W, davem, shemminger, decot, mirq-linux,
	Duyck, Alexander H, amit.salecha, eric.dumazet, therbert,
	paulmck, laijs, xiaosuo, greearb, loke.chetan, linux-mips,
	linux-scsi, devel, bridge

On 9/5/2011 8:25 PM, Zou, Yi wrote:
>>
>> On Sat, 2011-09-03 at 15:34 +0200, Jiri Pirko wrote:
>>> This patch does several things:
>>> - introduces __ethtool_get_settings which is called from ethtool code
>> and
>>>    from drivers as well. Put ASSERT_RTNL there.
>>> - dev_ethtool_get_settings() is replaced by __ethtool_get_settings()
>>> - changes calling in drivers so rtnl locking is respected. In
>>>    iboe_get_rate was previously ->get_settings() called unlocked. This
>>>    fixes it. Also prb_calc_retire_blk_tmo() in af_packet.c had the same
>>>    problem. Also fixed by calling __dev_get_by_index() instead of
>>>    dev_get_by_index() and holding rtnl_lock for both calls.
>>> - introduces rtnl_lock in bnx2fc_vport_create() and fcoe_vport_create()
>>>    so bnx2fc_if_create() and fcoe_if_create() are called locked as they
>>>    are from other places.
>>> - use __ethtool_get_settings() in bonding code
>>>
>>> Signed-off-by: Jiri Pirko<jpirko@redhat.com>
>> Reviewed-by: Ben Hutchings<bhutchings@solarflare.com>  [except FCoE bits]
>>
>> Ben.
> FCoE bits look ok to me. Thanks,
>
> Reviewed-by: Yi Zou<yi.zou@intel.com>

bnx2fc changes looks OK to me.
Reviewed-by: Bhanu Prakash Gollapudi <bprakash@broadcom.com>
>
>>
>> --
>> Ben Hutchings, Staff Engineer, Solarflare
>> Not speaking for my employer; that's the marketing department's job.
>> They asked us to note that Solarflare product names are trademarked.
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [patch net-next-2.6 v3] net: consolidate and fix ethtool_ops->get_settings calling
@ 2011-09-06  6:52               ` Bhanu Prakash Gollapudi
  0 siblings, 0 replies; 25+ messages in thread
From: Bhanu Prakash Gollapudi @ 2011-09-06  6:52 UTC (permalink / raw)
  To: Zou, Yi
  Cc: amit.salecha-h88ZbnxC6KDQT0dZR+AlfA,
	bridge-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	linux-mips-6z/3iImG2C8G8FEW9MqTrA,
	JBottomley-bzQdu9zFT3WakBO8gow8eQ,
	linux-scsi-u79uwXL29TY76Z2rM5mHXA, decot-hpIqsD4AKlfQT0dZR+AlfA,
	shemminger-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
	andy-QlMahl40kYEqcZcGjlUOXw, therbert-hpIqsD4AKlfQT0dZR+AlfA,
	eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w,
	fubar-r/Jw6+rmf7HQT0dZR+AlfA, xiaosuo-Re5JQEeQqe8AvxtiuMwx3w,
	paulmck-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8, Duyck, Alexander H,
	mirq-linux-CoA6ZxLDdyEEUmgCuDUIdw, Ben Hutchings,
	greearb-my8/4N5VtI7c+919tysfdA@public.gmane.org

On 9/5/2011 8:25 PM, Zou, Yi wrote:
>>
>> On Sat, 2011-09-03 at 15:34 +0200, Jiri Pirko wrote:
>>> This patch does several things:
>>> - introduces __ethtool_get_settings which is called from ethtool code
>> and
>>>    from drivers as well. Put ASSERT_RTNL there.
>>> - dev_ethtool_get_settings() is replaced by __ethtool_get_settings()
>>> - changes calling in drivers so rtnl locking is respected. In
>>>    iboe_get_rate was previously ->get_settings() called unlocked. This
>>>    fixes it. Also prb_calc_retire_blk_tmo() in af_packet.c had the same
>>>    problem. Also fixed by calling __dev_get_by_index() instead of
>>>    dev_get_by_index() and holding rtnl_lock for both calls.
>>> - introduces rtnl_lock in bnx2fc_vport_create() and fcoe_vport_create()
>>>    so bnx2fc_if_create() and fcoe_if_create() are called locked as they
>>>    are from other places.
>>> - use __ethtool_get_settings() in bonding code
>>>
>>> Signed-off-by: Jiri Pirko<jpirko-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>> Reviewed-by: Ben Hutchings<bhutchings-s/n/eUQHGBpZroRs9YW3xA@public.gmane.org>  [except FCoE bits]
>>
>> Ben.
> FCoE bits look ok to me. Thanks,
>
> Reviewed-by: Yi Zou<yi.zou-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

bnx2fc changes looks OK to me.
Reviewed-by: Bhanu Prakash Gollapudi <bprakash-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>
>>
>> --
>> Ben Hutchings, Staff Engineer, Solarflare
>> Not speaking for my employer; that's the marketing department's job.
>> They asked us to note that Solarflare product names are trademarked.
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Bridge] [patch net-next-2.6 v3] net: consolidate and fix ethtool_ops->get_settings calling
@ 2011-09-06  6:52               ` Bhanu Prakash Gollapudi
  0 siblings, 0 replies; 25+ messages in thread
From: Bhanu Prakash Gollapudi @ 2011-09-06  6:52 UTC (permalink / raw)
  To: Zou, Yi
  Cc: amit.salecha, bridge, linux-mips, devel, linux-scsi, decot,
	therbert, eric.dumazet, fubar, JBottomley, xiaosuo, paulmck,
	Duyck, Alexander H, mirq-linux, Ben Hutchings, greearb, Love,
	Robert W, laijs, Jiri Pirko, netdev, ralf, davem, loke.chetan

On 9/5/2011 8:25 PM, Zou, Yi wrote:
>>
>> On Sat, 2011-09-03 at 15:34 +0200, Jiri Pirko wrote:
>>> This patch does several things:
>>> - introduces __ethtool_get_settings which is called from ethtool code
>> and
>>>    from drivers as well. Put ASSERT_RTNL there.
>>> - dev_ethtool_get_settings() is replaced by __ethtool_get_settings()
>>> - changes calling in drivers so rtnl locking is respected. In
>>>    iboe_get_rate was previously ->get_settings() called unlocked. This
>>>    fixes it. Also prb_calc_retire_blk_tmo() in af_packet.c had the same
>>>    problem. Also fixed by calling __dev_get_by_index() instead of
>>>    dev_get_by_index() and holding rtnl_lock for both calls.
>>> - introduces rtnl_lock in bnx2fc_vport_create() and fcoe_vport_create()
>>>    so bnx2fc_if_create() and fcoe_if_create() are called locked as they
>>>    are from other places.
>>> - use __ethtool_get_settings() in bonding code
>>>
>>> Signed-off-by: Jiri Pirko<jpirko@redhat.com>
>> Reviewed-by: Ben Hutchings<bhutchings@solarflare.com>  [except FCoE bits]
>>
>> Ben.
> FCoE bits look ok to me. Thanks,
>
> Reviewed-by: Yi Zou<yi.zou@intel.com>

bnx2fc changes looks OK to me.
Reviewed-by: Bhanu Prakash Gollapudi <bprakash@broadcom.com>
>
>>
>> --
>> Ben Hutchings, Staff Engineer, Solarflare
>> Not speaking for my employer; that's the marketing department's job.
>> They asked us to note that Solarflare product names are trademarked.
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [patch net-next-2.6 v3] net: consolidate and fix ethtool_ops->get_settings calling
  2011-09-03 13:34       ` [Bridge] " Jiri Pirko
@ 2011-09-06 12:20         ` Ralf Baechle
  -1 siblings, 0 replies; 25+ messages in thread
From: Ralf Baechle @ 2011-09-06 12:20 UTC (permalink / raw)
  To: Jiri Pirko, tg
  Cc: Ben Hutchings, netdev, fubar, andy, kaber, bprakash, JBottomley,
	robert.w.love, davem, shemminger, decot, mirq-linux,
	alexander.h.duyck, amit.salecha, eric.dumazet, therbert, paulmck,
	laijs, xiaosuo, greearb, loke.chetan, linux-mips, linux-scsi,
	devel, bridge

On Sat, Sep 03, 2011 at 03:34:30PM +0200, Jiri Pirko wrote:

>  arch/mips/txx9/generic/setup_tx4939.c |    2 +-

Acked-by: Ralf Baechle <ralf@linux-mips.org>

Feel free to merge this through the net tree.

  Ralf

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Bridge] [patch net-next-2.6 v3] net: consolidate and fix ethtool_ops->get_settings calling
@ 2011-09-06 12:20         ` Ralf Baechle
  0 siblings, 0 replies; 25+ messages in thread
From: Ralf Baechle @ 2011-09-06 12:20 UTC (permalink / raw)
  To: Jiri Pirko, tg
  Cc: amit.salecha, bridge, linux-mips, devel, linux-scsi, decot,
	therbert, robert.w.love, eric.dumazet, fubar, JBottomley,
	xiaosuo, paulmck, alexander.h.duyck, mirq-linux, Ben Hutchings,
	greearb, bprakash, laijs, netdev, davem, loke.chetan

On Sat, Sep 03, 2011 at 03:34:30PM +0200, Jiri Pirko wrote:

>  arch/mips/txx9/generic/setup_tx4939.c |    2 +-

Acked-by: Ralf Baechle <ralf@linux-mips.org>

Feel free to merge this through the net tree.

  Ralf

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [patch net-next-2.6] net: consolidate and fix ethtool_ops->get_settings calling
  2011-09-01 19:28 ` [Bridge] " Jiri Pirko
  (?)
  (?)
@ 2011-09-15 21:33 ` David Miller
  -1 siblings, 0 replies; 25+ messages in thread
From: David Miller @ 2011-09-15 21:33 UTC (permalink / raw)
  To: jpirko
  Cc: netdev, ralf, fubar, andy, kaber, bprakash, JBottomley,
	robert.w.love, shemminger, decot, bhutchings, mirq-linux,
	alexander.h.duyck, amit.salecha, eric.dumazet, therbert, paulmck,
	laijs, xiaosuo, greearb, loke.chetan, linux-mips, linux-scsi,
	devel, bridge

From: Jiri Pirko <jpirko@redhat.com>
Date: Thu,  1 Sep 2011 21:28:24 +0200

> This patch does several things:
> - introduces __ethtool_get_settings which is called from ethtool code and
>   from dev_ethtool_get_settings() as well.
> - dev_ethtool_get_settings() becomes rtnl wrapper for
>   __ethtool_get_settings()
> - changes calling in drivers so rtnl locking is respected. In
>   iboe_get_rate was previously ->get_settings() called unlocked. This
>   fixes it
> - introduces rtnl_lock in bnx2fc_vport_create() and fcoe_vport_create()
>   so bnx2fc_if_create() and fcoe_if_create() are called locked as they
>   are from other places.
> - prb_calc_retire_blk_tmo() in af_packet.c was not calling get_settings
>   with rtnl_lock. So use dev_ethtool_get_settings here.
> - use __ethtool_get_settings() in bonding code
> 
> Signed-off-by: Jiri Pirko <jpirko@redhat.com>

Applied, thanks.

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2011-09-15 21:33 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-09-01 19:28 [patch net-next-2.6] net: consolidate and fix ethtool_ops->get_settings calling Jiri Pirko
2011-09-01 19:28 ` [Bridge] " Jiri Pirko
2011-09-02 12:26 ` [patch net-next-2.6 v2] " Jiri Pirko
2011-09-02 12:26   ` [Bridge] " Jiri Pirko
2011-09-02 18:46   ` Ben Hutchings
2011-09-02 18:46     ` [Bridge] " Ben Hutchings
2011-09-02 20:42     ` Jiri Pirko
2011-09-02 20:42       ` [Bridge] " Jiri Pirko
2011-09-03 13:34     ` [patch net-next-2.6 v3] " Jiri Pirko
2011-09-03 13:34       ` [Bridge] " Jiri Pirko
2011-09-03 13:46       ` Ben Hutchings
2011-09-03 13:46         ` [Bridge] " Ben Hutchings
2011-09-06  3:25         ` Zou, Yi
2011-09-06  3:25           ` [Bridge] " Zou, Yi
2011-09-06  3:25           ` Zou, Yi
2011-09-06  3:25           ` Zou, Yi
2011-09-06  3:25           ` Zou, Yi
     [not found]           ` <5A9BD224CEA58D4CB62235967D650C160A1B7D1C-osO9UTpF0UQd3b2yrw5b5LfspsVTdybXVpNB7YpNyf8@public.gmane.org>
2011-09-06  6:52             ` Bhanu Prakash Gollapudi
2011-09-06  6:52               ` [Bridge] " Bhanu Prakash Gollapudi
2011-09-06  6:52               ` Bhanu Prakash Gollapudi
2011-09-06  6:52               ` Bhanu Prakash Gollapudi
2011-09-06  6:52               ` Bhanu Prakash Gollapudi
2011-09-06 12:20       ` Ralf Baechle
2011-09-06 12:20         ` [Bridge] " Ralf Baechle
2011-09-15 21:33 ` [patch net-next-2.6] " David Miller

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.