Netdev List
 help / color / mirror / Atom feed
* [PATCHv3 NEXT 1/2] qlcnic: support rcv ring configuration through ethtool
From: amit.salecha @ 2011-04-28 21:48 UTC (permalink / raw)
  To: davem
  Cc: netdev, ameen.rahman, anirban.chakraborty, Sucheta Chakraborty,
	Amit Kumar Salecha
In-Reply-To: <1304027299-1960-1-git-send-email-amit.salecha@qlogic.com>

From: Sucheta Chakraborty <sucheta.chakraborty@qlogic.com>

o Support ethtool command ETHTOOL_GCHANNELS and ETHTOOL_SCHANNELS.
o Number of rcv rings configuration depend upon number of msix vector.

Signed-off-by: Sucheta Chakraborty <sucheta.chakraborty@qlogic.com>
Signed-off-by: Amit Kumar Salecha <amit.salecha@qlogic.com>
---
 drivers/net/qlcnic/qlcnic.h         |    8 +-
 drivers/net/qlcnic/qlcnic_ethtool.c |   35 ++++++++
 drivers/net/qlcnic/qlcnic_main.c    |  146 +++++++++++++++++++++++++++-------
 3 files changed, 156 insertions(+), 33 deletions(-)

diff --git a/drivers/net/qlcnic/qlcnic.h b/drivers/net/qlcnic/qlcnic.h
index f7acb80..1934ed9 100644
--- a/drivers/net/qlcnic/qlcnic.h
+++ b/drivers/net/qlcnic/qlcnic.h
@@ -118,7 +118,6 @@
 #define PHAN_PEG_RCV_INITIALIZED	0xff01
 
 #define NUM_RCV_DESC_RINGS	3
-#define NUM_STS_DESC_RINGS	4
 
 #define RCV_RING_NORMAL 0
 #define RCV_RING_JUMBO	1
@@ -871,7 +870,8 @@ struct qlcnic_ipaddr {
 #define QLCNIC_IS_MSI_FAMILY(adapter) \
 	((adapter)->flags & (QLCNIC_MSI_ENABLED | QLCNIC_MSIX_ENABLED))
 
-#define MSIX_ENTRIES_PER_ADAPTER	NUM_STS_DESC_RINGS
+#define QLCNIC_DEF_NUM_STS_DESC_RINGS	4
+#define QLCNIC_MIN_NUM_RSS_RINGS	2
 #define QLCNIC_MSIX_TBL_SPACE		8192
 #define QLCNIC_PCI_REG_MSIX_TBL 	0x44
 #define QLCNIC_MSIX_TBL_PGSIZE		4096
@@ -987,7 +987,7 @@ struct qlcnic_adapter {
 	void __iomem	*crb_int_state_reg;
 	void __iomem	*isr_int_vec;
 
-	struct msix_entry msix_entries[MSIX_ENTRIES_PER_ADAPTER];
+	struct msix_entry *msix_entries;
 
 	struct delayed_work fw_work;
 
@@ -1262,6 +1262,8 @@ u32 qlcnic_issue_cmd(struct qlcnic_adapter *adapter,
 void qlcnic_diag_free_res(struct net_device *netdev, int max_sds_rings);
 int qlcnic_diag_alloc_res(struct net_device *netdev, int test);
 netdev_tx_t qlcnic_xmit_frame(struct sk_buff *skb, struct net_device *netdev);
+int qlcnic_validate_max_rss(struct net_device *netdev, u8 max_hw, u8 val);
+int qlcnic_set_max_rss(struct qlcnic_adapter *adapter, u8 data);
 
 /* Management functions */
 int qlcnic_get_mac_address(struct qlcnic_adapter *, u8*);
diff --git a/drivers/net/qlcnic/qlcnic_ethtool.c b/drivers/net/qlcnic/qlcnic_ethtool.c
index de65847..8db1d19 100644
--- a/drivers/net/qlcnic/qlcnic_ethtool.c
+++ b/drivers/net/qlcnic/qlcnic_ethtool.c
@@ -474,6 +474,39 @@ qlcnic_set_ringparam(struct net_device *dev,
 	return qlcnic_reset_context(adapter);
 }
 
+static void qlcnic_get_channels(struct net_device *dev,
+		struct ethtool_channels *channel)
+{
+	struct qlcnic_adapter *adapter = netdev_priv(dev);
+
+	channel->max_rx = rounddown_pow_of_two(min_t(int,
+			adapter->max_rx_ques, num_online_cpus()));
+	channel->max_tx = adapter->max_tx_ques;
+
+	channel->rx_count = adapter->max_sds_rings;
+	channel->tx_count = adapter->max_tx_ques;
+}
+
+static int qlcnic_set_channels(struct net_device *dev,
+		struct ethtool_channels *channel)
+{
+	struct qlcnic_adapter *adapter = netdev_priv(dev);
+	int err;
+
+	if (channel->other_count || channel->combined_count ||
+	    channel->tx_count != channel->max_tx)
+		return -EINVAL;
+
+	err = qlcnic_validate_max_rss(dev, channel->max_rx, channel->rx_count);
+	if (err)
+		return err;
+
+	err = qlcnic_set_max_rss(adapter, channel->rx_count);
+	netdev_info(dev, "allocated 0x%x sds rings\n",
+				 adapter->max_sds_rings);
+	return err;
+}
+
 static void
 qlcnic_get_pauseparam(struct net_device *netdev,
 			  struct ethtool_pauseparam *pause)
@@ -949,6 +982,8 @@ const struct ethtool_ops qlcnic_ethtool_ops = {
 	.get_eeprom = qlcnic_get_eeprom,
 	.get_ringparam = qlcnic_get_ringparam,
 	.set_ringparam = qlcnic_set_ringparam,
+	.get_channels = qlcnic_get_channels,
+	.set_channels = qlcnic_set_channels,
 	.get_pauseparam = qlcnic_get_pauseparam,
 	.set_pauseparam = qlcnic_set_pauseparam,
 	.get_wol = qlcnic_get_wol,
diff --git a/drivers/net/qlcnic/qlcnic_main.c b/drivers/net/qlcnic/qlcnic_main.c
index 6e61951..d6cc4d4 100644
--- a/drivers/net/qlcnic/qlcnic_main.c
+++ b/drivers/net/qlcnic/qlcnic_main.c
@@ -18,6 +18,7 @@
 #include <linux/inetdevice.h>
 #include <linux/sysfs.h>
 #include <linux/aer.h>
+#include <linux/log2.h>
 
 MODULE_DESCRIPTION("QLogic 1/10 GbE Converged/Intelligent Ethernet Driver");
 MODULE_LICENSE("GPL");
@@ -350,39 +351,17 @@ static struct qlcnic_nic_template qlcnic_vf_ops = {
 	.start_firmware = qlcnicvf_start_firmware
 };
 
-static void
-qlcnic_setup_intr(struct qlcnic_adapter *adapter)
+static int qlcnic_enable_msix(struct qlcnic_adapter *adapter, u32 num_msix)
 {
-	const struct qlcnic_legacy_intr_set *legacy_intrp;
 	struct pci_dev *pdev = adapter->pdev;
-	int err, num_msix;
-
-	if (adapter->msix_supported) {
-		num_msix = (num_online_cpus() >= MSIX_ENTRIES_PER_ADAPTER) ?
-			MSIX_ENTRIES_PER_ADAPTER : 2;
-	} else
-		num_msix = 1;
+	int err = -1;
 
 	adapter->max_sds_rings = 1;
-
 	adapter->flags &= ~(QLCNIC_MSI_ENABLED | QLCNIC_MSIX_ENABLED);
-
-	legacy_intrp = &legacy_intr[adapter->ahw->pci_func];
-
-	adapter->int_vec_bit = legacy_intrp->int_vec_bit;
-	adapter->tgt_status_reg = qlcnic_get_ioaddr(adapter,
-			legacy_intrp->tgt_status_reg);
-	adapter->tgt_mask_reg = qlcnic_get_ioaddr(adapter,
-			legacy_intrp->tgt_mask_reg);
-	adapter->isr_int_vec = qlcnic_get_ioaddr(adapter, ISR_INT_VECTOR);
-
-	adapter->crb_int_state_reg = qlcnic_get_ioaddr(adapter,
-			ISR_INT_STATE_REG);
-
 	qlcnic_set_msix_bit(pdev, 0);
 
 	if (adapter->msix_supported) {
-
+ enable_msix:
 		qlcnic_init_msix_entries(adapter, num_msix);
 		err = pci_enable_msix(pdev, adapter->msix_entries, num_msix);
 		if (err == 0) {
@@ -392,14 +371,22 @@ qlcnic_setup_intr(struct qlcnic_adapter *adapter)
 			adapter->max_sds_rings = num_msix;
 
 			dev_info(&pdev->dev, "using msi-x interrupts\n");
-			return;
+			return err;
 		}
+		if (err > 0) {
+			num_msix = rounddown_pow_of_two(err);
+			if (num_msix)
+				goto enable_msix;
+		}
+	}
+	return err;
+}
 
-		if (err > 0)
-			pci_disable_msix(pdev);
 
-		/* fall through for msi */
-	}
+static void qlcnic_enable_msi_legacy(struct qlcnic_adapter *adapter)
+{
+	const struct qlcnic_legacy_intr_set *legacy_intrp;
+	struct pci_dev *pdev = adapter->pdev;
 
 	if (use_msi && !pci_enable_msi(pdev)) {
 		adapter->flags |= QLCNIC_MSI_ENABLED;
@@ -410,11 +397,41 @@ qlcnic_setup_intr(struct qlcnic_adapter *adapter)
 		return;
 	}
 
+	legacy_intrp = &legacy_intr[adapter->ahw->pci_func];
+
+	adapter->int_vec_bit = legacy_intrp->int_vec_bit;
+	adapter->tgt_status_reg = qlcnic_get_ioaddr(adapter,
+			legacy_intrp->tgt_status_reg);
+	adapter->tgt_mask_reg = qlcnic_get_ioaddr(adapter,
+			legacy_intrp->tgt_mask_reg);
+	adapter->isr_int_vec = qlcnic_get_ioaddr(adapter, ISR_INT_VECTOR);
+
+	adapter->crb_int_state_reg = qlcnic_get_ioaddr(adapter,
+			ISR_INT_STATE_REG);
 	dev_info(&pdev->dev, "using legacy interrupts\n");
 	adapter->msix_entries[0].vector = pdev->irq;
 }
 
 static void
+qlcnic_setup_intr(struct qlcnic_adapter *adapter)
+{
+	int num_msix;
+
+	if (adapter->msix_supported) {
+		num_msix = (num_online_cpus() >=
+			QLCNIC_DEF_NUM_STS_DESC_RINGS) ?
+			QLCNIC_DEF_NUM_STS_DESC_RINGS :
+			QLCNIC_MIN_NUM_RSS_RINGS;
+	} else
+		num_msix = 1;
+
+	if (!qlcnic_enable_msix(adapter, num_msix))
+		return;
+
+	qlcnic_enable_msi_legacy(adapter);
+}
+
+static void
 qlcnic_teardown_intr(struct qlcnic_adapter *adapter)
 {
 	if (adapter->flags & QLCNIC_MSIX_ENABLED)
@@ -1493,6 +1510,19 @@ static int qlcnic_set_dma_mask(struct pci_dev *pdev, u8 *pci_using_dac)
 	return 0;
 }
 
+static int
+qlcnic_alloc_msix_entries(struct qlcnic_adapter *adapter, u16 count)
+{
+	adapter->msix_entries = kcalloc(count, sizeof(struct msix_entry),
+					GFP_KERNEL);
+
+	if (adapter->msix_entries)
+		return 0;
+
+	dev_err(&adapter->pdev->dev, "failed allocating msix_entries\n");
+	return -ENOMEM;
+}
+
 static int __devinit
 qlcnic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 {
@@ -1587,6 +1617,10 @@ qlcnic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	qlcnic_clear_stats(adapter);
 
+	err = qlcnic_alloc_msix_entries(adapter, adapter->max_rx_ques);
+	if (err)
+		goto err_out_decr_ref;
+
 	qlcnic_setup_intr(adapter);
 
 	err = qlcnic_setup_netdev(adapter, netdev, pci_using_dac);
@@ -1615,6 +1649,7 @@ qlcnic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 err_out_disable_msi:
 	qlcnic_teardown_intr(adapter);
+	kfree(adapter->msix_entries);
 
 err_out_decr_ref:
 	qlcnic_clr_all_drv_state(adapter, 0);
@@ -1666,6 +1701,7 @@ static void __devexit qlcnic_remove(struct pci_dev *pdev)
 	qlcnic_free_lb_filters_mem(adapter);
 
 	qlcnic_teardown_intr(adapter);
+	kfree(adapter->msix_entries);
 
 	qlcnic_remove_diag_entries(adapter);
 
@@ -3299,6 +3335,56 @@ static struct device_attribute dev_attr_diag_mode = {
 	.store = qlcnic_store_diag_mode,
 };
 
+int qlcnic_validate_max_rss(struct net_device *netdev, u8 max_hw, u8 val)
+{
+	if (!use_msi_x && !use_msi) {
+		netdev_info(netdev, "no msix or msi support, hence no rss\n");
+		return -EINVAL;
+	}
+
+	if ((val > max_hw) || (val <  2) || !is_power_of_2(val)) {
+		netdev_info(netdev, "rss_ring valid range [2 - %x] in "
+			" powers of 2\n", max_hw);
+		return -EINVAL;
+	}
+	return 0;
+
+}
+
+int qlcnic_set_max_rss(struct qlcnic_adapter *adapter, u8 data)
+{
+	struct net_device *netdev = adapter->netdev;
+	int err = 0;
+
+	if (test_and_set_bit(__QLCNIC_RESETTING, &adapter->state))
+		return -EBUSY;
+
+	netif_device_detach(netdev);
+	if (netif_running(netdev))
+		__qlcnic_down(adapter, netdev);
+	qlcnic_detach(adapter);
+	qlcnic_teardown_intr(adapter);
+
+	if (qlcnic_enable_msix(adapter, data)) {
+		netdev_info(netdev, "failed setting max_rss; rss disabled\n");
+		qlcnic_enable_msi_legacy(adapter);
+	}
+
+	if (netif_running(netdev)) {
+		err = qlcnic_attach(adapter);
+		if (err)
+			goto done;
+		err = __qlcnic_up(adapter, netdev);
+		if (err)
+			goto done;
+		qlcnic_restore_indev_addr(netdev, NETDEV_UP);
+	}
+ done:
+	netif_device_attach(netdev);
+	clear_bit(__QLCNIC_RESETTING, &adapter->state);
+	return err;
+}
+
 static int
 qlcnic_sysfs_validate_crb(struct qlcnic_adapter *adapter,
 		loff_t offset, size_t size)
-- 
1.6.3.3


^ permalink raw reply related

* [PATCHv3 NEXT 0/2]qlcnic: updates
From: amit.salecha @ 2011-04-28 21:48 UTC (permalink / raw)
  To: davem; +Cc: netdev, ameen.rahman, anirban.chakraborty

Hi,
    Please discard series 2 as sendmail fail to send mail to netdev list.
    This is v3 to remove any ambiguity.

-Thanks
Amit

^ permalink raw reply

* [PATCHv3 NEXT 2/2] qlcnic: Support for GBE port settings
From: amit.salecha @ 2011-04-28 21:48 UTC (permalink / raw)
  To: davem
  Cc: netdev, ameen.rahman, anirban.chakraborty, Sony Chacko,
	Amit Kumar Salecha
In-Reply-To: <1304027299-1960-1-git-send-email-amit.salecha@qlogic.com>

From: Sony Chacko <sony.chacko@qlogic.com>

Enable setting speed and auto negotiation parameters for GbE ports.
Hardware do not support half duplex setting currently.

o Update driver version to 5.0.17.

Signed-off-by: Sony Chacko <sony.chacko@qlogic.com>
Signed-off-by: Amit Kumar Salecha <amit.salecha@qlogic.com>
---
 drivers/net/qlcnic/qlcnic.h         |    9 ++--
 drivers/net/qlcnic/qlcnic_ctx.c     |   26 ++-----------
 drivers/net/qlcnic/qlcnic_ethtool.c |   72 ++++++++++++++++-------------------
 3 files changed, 42 insertions(+), 65 deletions(-)

diff --git a/drivers/net/qlcnic/qlcnic.h b/drivers/net/qlcnic/qlcnic.h
index 1934ed9..f729363 100644
--- a/drivers/net/qlcnic/qlcnic.h
+++ b/drivers/net/qlcnic/qlcnic.h
@@ -36,8 +36,8 @@
 
 #define _QLCNIC_LINUX_MAJOR 5
 #define _QLCNIC_LINUX_MINOR 0
-#define _QLCNIC_LINUX_SUBVERSION 16
-#define QLCNIC_LINUX_VERSIONID  "5.0.16"
+#define _QLCNIC_LINUX_SUBVERSION 17
+#define QLCNIC_LINUX_VERSIONID  "5.0.17"
 #define QLCNIC_DRV_IDC_VER  0x01
 #define QLCNIC_DRIVER_VERSION  ((_QLCNIC_LINUX_MAJOR << 16) |\
 		 (_QLCNIC_LINUX_MINOR << 8) | (_QLCNIC_LINUX_SUBVERSION))
@@ -573,8 +573,10 @@ struct qlcnic_recv_context {
 #define QLCNIC_CDRP_CMD_CONFIGURE_ESWITCH	0x00000028
 #define QLCNIC_CDRP_CMD_GET_ESWITCH_PORT_CONFIG	0x00000029
 #define QLCNIC_CDRP_CMD_GET_ESWITCH_STATS	0x0000002a
+#define QLCNIC_CDRP_CMD_CONFIG_PORT		0x0000002E
 
 #define QLCNIC_RCODE_SUCCESS		0
+#define QLCNIC_RCODE_NOT_SUPPORTED	9
 #define QLCNIC_RCODE_TIMEOUT		17
 #define QLCNIC_DESTROY_CTX_RESET	0
 
@@ -1155,8 +1157,7 @@ struct qlcnic_esw_statistics {
 	struct __qlcnic_esw_statistics tx;
 };
 
-int qlcnic_fw_cmd_query_phy(struct qlcnic_adapter *adapter, u32 reg, u32 *val);
-int qlcnic_fw_cmd_set_phy(struct qlcnic_adapter *adapter, u32 reg, u32 val);
+int qlcnic_fw_cmd_set_port(struct qlcnic_adapter *adapter, u32 config);
 
 u32 qlcnic_hw_read_wx_2M(struct qlcnic_adapter *adapter, ulong off);
 int qlcnic_hw_write_wx_2M(struct qlcnic_adapter *, ulong off, u32 data);
diff --git a/drivers/net/qlcnic/qlcnic_ctx.c b/drivers/net/qlcnic/qlcnic_ctx.c
index 050fa5a..3a99886 100644
--- a/drivers/net/qlcnic/qlcnic_ctx.c
+++ b/drivers/net/qlcnic/qlcnic_ctx.c
@@ -359,33 +359,15 @@ qlcnic_fw_cmd_destroy_tx_ctx(struct qlcnic_adapter *adapter)
 }
 
 int
-qlcnic_fw_cmd_query_phy(struct qlcnic_adapter *adapter, u32 reg, u32 *val)
-{
-
-	if (qlcnic_issue_cmd(adapter,
-			adapter->ahw->pci_func,
-			adapter->fw_hal_version,
-			reg,
-			0,
-			0,
-			QLCNIC_CDRP_CMD_READ_PHY)) {
-
-		return -EIO;
-	}
-
-	return QLCRD32(adapter, QLCNIC_ARG1_CRB_OFFSET);
-}
-
-int
-qlcnic_fw_cmd_set_phy(struct qlcnic_adapter *adapter, u32 reg, u32 val)
+qlcnic_fw_cmd_set_port(struct qlcnic_adapter *adapter, u32 config)
 {
 	return qlcnic_issue_cmd(adapter,
 			adapter->ahw->pci_func,
 			adapter->fw_hal_version,
-			reg,
-			val,
+			config,
+			0,
 			0,
-			QLCNIC_CDRP_CMD_WRITE_PHY);
+			QLCNIC_CDRP_CMD_CONFIG_PORT);
 }
 
 int qlcnic_alloc_hw_resources(struct qlcnic_adapter *adapter)
diff --git a/drivers/net/qlcnic/qlcnic_ethtool.c b/drivers/net/qlcnic/qlcnic_ethtool.c
index 8db1d19..27726eb 100644
--- a/drivers/net/qlcnic/qlcnic_ethtool.c
+++ b/drivers/net/qlcnic/qlcnic_ethtool.c
@@ -284,50 +284,44 @@ skip:
 static int
 qlcnic_set_settings(struct net_device *dev, struct ethtool_cmd *ecmd)
 {
+	u32 config = 0;
+	u32 ret = 0;
 	struct qlcnic_adapter *adapter = netdev_priv(dev);
-	__u32 status;
+
+	if (adapter->ahw->port_type != QLCNIC_GBE)
+		return -EOPNOTSUPP;
 
 	/* read which mode */
-	if (adapter->ahw->port_type == QLCNIC_GBE) {
-		/* autonegotiation */
-		if (qlcnic_fw_cmd_set_phy(adapter,
-			       QLCNIC_NIU_GB_MII_MGMT_ADDR_AUTONEG,
-			       ecmd->autoneg) != 0)
-			return -EIO;
-		else
-			adapter->link_autoneg = ecmd->autoneg;
+	if (ecmd->duplex)
+		config |= 0x1;
 
-		if (qlcnic_fw_cmd_query_phy(adapter,
-			      QLCNIC_NIU_GB_MII_MGMT_ADDR_PHY_STATUS,
-			      &status) != 0)
-			return -EIO;
+	if (ecmd->autoneg)
+		config |= 0x2;
 
-		switch (ecmd->speed) {
-		case SPEED_10:
-			qlcnic_set_phy_speed(status, 0);
-			break;
-		case SPEED_100:
-			qlcnic_set_phy_speed(status, 1);
-			break;
-		case SPEED_1000:
-			qlcnic_set_phy_speed(status, 2);
-			break;
-		}
+	switch (ethtool_cmd_speed(ecmd)) {
+	case SPEED_10:
+		config |= (0 << 8);
+		break;
+	case SPEED_100:
+		config |= (1 << 8);
+		break;
+	case SPEED_1000:
+		config |= (10 << 8);
+		break;
+	default:
+		return -EIO;
+	}
 
-		if (ecmd->duplex == DUPLEX_HALF)
-			qlcnic_clear_phy_duplex(status);
-		if (ecmd->duplex == DUPLEX_FULL)
-			qlcnic_set_phy_duplex(status);
-		if (qlcnic_fw_cmd_set_phy(adapter,
-			       QLCNIC_NIU_GB_MII_MGMT_ADDR_PHY_STATUS,
-			       *((int *)&status)) != 0)
-			return -EIO;
-		else {
-			adapter->link_speed = ecmd->speed;
-			adapter->link_duplex = ecmd->duplex;
-		}
-	} else
+	ret = qlcnic_fw_cmd_set_port(adapter, config);
+
+	if (ret == QLCNIC_RCODE_NOT_SUPPORTED)
 		return -EOPNOTSUPP;
+	else if (ret)
+		return -EIO;
+
+	adapter->link_speed = ethtool_cmd_speed(ecmd);
+	adapter->link_duplex = ecmd->duplex;
+	adapter->link_autoneg = ecmd->autoneg;
 
 	if (!netif_running(dev))
 		return 0;
-- 
1.6.3.3


^ permalink raw reply related

* A race in register_netdevice()
From: Kalle Valo @ 2011-04-28 22:36 UTC (permalink / raw)
  To: netdev-u79uwXL29TY76Z2rM5mHXA; +Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA

Hi,

there seems to be a race in register_netdevice(), which is reported here:

https://bugzilla.kernel.org/show_bug.cgi?id=15606

This is visible at least with flimflam and ath6kl. Basically what
happens is this:

Apr 29 00:21:35 roska flimflamd[2598]: src/udev.c:add_net_device() 
Apr 29 00:21:35 roska flimflamd[2598]: connman_inet_ifname: SIOCGIFNAME(index
4): No such device
Apr 29 00:21:45 roska flimflamd[2598]: src/rtnl.c:rtnl_message() buf
0xbfefda3c len 1004
Apr 29 00:21:45 roska flimflamd[2598]: src/rtnl.c:rtnl_message()
NEWLINK len 1004 type 16 flags 0x0000 seq 0

(ignore the 10 s delay, I added that to reproduce the issue easily)

There are two ways to fix this, first is to move kobject registration
after the call to list_netdevice():

--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5425,11 +5425,6 @@ int register_netdevice(struct net_device *dev)
        if (ret)
                goto err_uninit;
 
-       ret = netdev_register_kobject(dev);
-       if (ret)
-               goto err_uninit;
-       dev->reg_state = NETREG_REGISTERED;
-
        netdev_update_features(dev);
 
        /*
@@ -5443,6 +5438,11 @@ int register_netdevice(struct net_device *dev)
        dev_hold(dev);
        list_netdevice(dev);
 
+       ret = netdev_register_kobject(dev);
+       if (ret)
+               goto err_uninit;
+       dev->reg_state = NETREG_REGISTERED;
+
        /* Notify protocols, that a new device appeared. */
        ret = call_netdevice_notifiers(NETDEV_REGISTER, dev);
        ret = notifier_to_errno(ret);

Other option, noticed by Jouni Malinen, is to take rtnl for
SIOCGIFNAME. For some reason it's currently unprotected:

--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4917,8 +4917,12 @@ int dev_ioctl(struct net *net, unsigned int
cmd, void __user *arg)
          rtnl_unlock();
                return ret;
                }
-               if (cmd == SIOCGIFNAME)
-                  return dev_ifname(net, (struct ifreq __user *)arg);
+                  if (cmd == SIOCGIFNAME) {
+                     rtnl_lock();
+                       ret = dev_ifname(net, (struct ifreq __user
-                  *)arg);
+                       rtnl_unlock();
+                               return ret;
+                               }
 
        if (copy_from_user(&ifr, arg, sizeof(struct ifreq)))
           return -EFAULT;

I have confirmed that both of these patches fix the issue. Now I'm
wondering which one is the best way forward. Or is there a better way
to fix this?

-- 
Kalle Valo
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" 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

* usbnet: why is by default dev->rx_urb_size equal to dev->hard_mtu?
From: DMITRIY GRUZMAN @ 2011-04-28 23:00 UTC (permalink / raw)
  To: netdev

Hello,

Currently, usbnet_probe() in drivers/net/usb/usbnet.c driver sets
dev->rx_urb_size equal to dev->hard_mtu unless the dev->rx_urb size is
previously set in the bind() call.  I do not think that there is any
correlation between those values and I think that setting
dev->rx_urb_size to dev->maxpacket would be more appropriate.

Please provide your comments so I know if I should proceed with making
such a change.

Thanks,

Dmitriy Gruzman

^ permalink raw reply

* [PATCH 1/2] net: Allow ethtool to set interface in loopback mode.
From: Mahesh Bandewar @ 2011-04-28 23:33 UTC (permalink / raw)
  To: David Miller, Matt Carlson
  Cc: netdev, Michael Chan, Ben Hutchings, Michał Mirosław,
	Mahesh Bandewar
In-Reply-To: <1304033599-8395-1-git-send-email-maheshb@google.com>

This patch enables ethtool to set the loopback mode on a given interface.
By configuring the interface in loopback mode in conjunction with a policy
route / rule, a userland application can stress the egress / ingress path
exposing the flows of the change in progress and potentially help developer(s)
understand the impact of those changes without even sending a packet out
on the network.

Following set of commands illustrates one such example -
    a) ip -4 addr add 192.168.1.1/24 dev eth1
    b) ip -4 rule add from all iif eth1 lookup 250
    c) ip -4 route add local 0/0 dev lo proto kernel scope host table 250
    d) arp -Ds 192.168.1.100 eth1
    e) arp -Ds 192.168.1.200 eth1
    f) sysctl -w net.ipv4.ip_nonlocal_bind=1
    g) sysctl -w net.ipv4.conf.all.accept_local=1
    # Assuming that the machine has 8 cores
    h) taskset 000f netserver -L 192.168.1.200
    i) taskset 00f0 netperf -t TCP_CRR -L 192.168.1.100 -H 192.168.1.200 -l 30
---
 include/linux/netdevice.h |    3 ++-
 net/core/ethtool.c        |    2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index e03af35..0e17c81 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1067,6 +1067,7 @@ struct net_device {
 #define NETIF_F_RXHASH		(1 << 28) /* Receive hashing offload */
 #define NETIF_F_RXCSUM		(1 << 29) /* Receive checksumming offload */
 #define NETIF_F_NOCACHE_COPY	(1 << 30) /* Use no-cache copyfromuser */
+#define NETIF_F_LOOPBACK	(1 << 31) /* Enable loopback */
 
 	/* Segmentation offload features */
 #define NETIF_F_GSO_SHIFT	16
@@ -1082,7 +1083,7 @@ struct net_device {
 	/* = all defined minus driver/device-class-related */
 #define NETIF_F_NEVER_CHANGE	(NETIF_F_VLAN_CHALLENGED | \
 				  NETIF_F_LLTX | NETIF_F_NETNS_LOCAL)
-#define NETIF_F_ETHTOOL_BITS	(0x7f3fffff & ~NETIF_F_NEVER_CHANGE)
+#define NETIF_F_ETHTOOL_BITS	(0xff3fffff & ~NETIF_F_NEVER_CHANGE)
 
 	/* List of features with software fallbacks. */
 #define NETIF_F_GSO_SOFTWARE	(NETIF_F_TSO | NETIF_F_TSO_ECN | \
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index d8b1a8d..f26649d 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -362,7 +362,7 @@ static const char netdev_features_strings[ETHTOOL_DEV_FEATURE_WORDS * 32][ETH_GS
 	/* NETIF_F_RXHASH */          "rx-hashing",
 	/* NETIF_F_RXCSUM */          "rx-checksum",
 	/* NETIF_F_NOCACHE_COPY */    "tx-nocache-copy"
-	"",
+	/* NETIF_F_LOOPBACK */        "loopback",
 };
 
 static int __ethtool_get_sset_count(struct net_device *dev, int sset)
-- 
1.7.3.1


^ permalink raw reply related

* [PATCH 0/2] Loopback
From: Mahesh Bandewar @ 2011-04-28 23:33 UTC (permalink / raw)
  To: David Miller, Matt Carlson
  Cc: netdev, Michael Chan, Ben Hutchings, Michał Mirosław,
	Mahesh Bandewar

First patch is the repost of the earlier loopback patch. tg3 implementation / patch demonstrates one such usage.

Mahesh Bandewar (2):
  net: Allow ethtool to set interface in loopback mode.
  tg3: Add code to allow ethtool to enable/disable loopback.

 drivers/net/tg3.c         |   32 ++++++++++++++++++++++++++++++++
 include/linux/netdevice.h |    3 ++-
 net/core/ethtool.c        |    2 +-
 3 files changed, 35 insertions(+), 2 deletions(-)

-- 
1.7.3.1


^ permalink raw reply

* [PATCH 2/2] tg3: Add code to allow ethtool to enable/disable loopback.
From: Mahesh Bandewar @ 2011-04-28 23:33 UTC (permalink / raw)
  To: David Miller, Matt Carlson
  Cc: netdev, Michael Chan, Ben Hutchings, Michał Mirosław,
	Mahesh Bandewar
In-Reply-To: <1304033599-8395-2-git-send-email-maheshb@google.com>

---
 drivers/net/tg3.c |   32 ++++++++++++++++++++++++++++++++
 1 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
index fa57e3d..208884d 100644
--- a/drivers/net/tg3.c
+++ b/drivers/net/tg3.c
@@ -6319,6 +6319,33 @@ static u32 tg3_fix_features(struct net_device *dev, u32 features)
 	return features;
 }
 
+static int tg3_set_features(struct net_device *dev, u32 features)
+{
+	struct tg3 *tp = netdev_priv(dev);
+	u32 cur_mode = 0;
+	int err = 0;
+
+	spin_lock_bh(&tp->lock);
+	cur_mode = tr32(MAC_MODE);
+
+	if (features & NETIF_F_LOOPBACK) {
+		/* Enable internal MAC loopback mode */
+		tw32(MAC_MODE, cur_mode | MAC_MODE_PORT_INT_LPBACK);
+		netif_carrier_on(tp->dev);
+		netdev_info(dev, "Internal MAC loopback mode enabled.\n");
+	} else {
+		/* Disable internal MAC loopback mode */
+		tw32(MAC_MODE, cur_mode & ~MAC_MODE_PORT_INT_LPBACK);
+		/* Force link status check */
+		if (netif_running(dev))
+			tg3_setup_phy(tp, 1);
+		netdev_info(dev, "Internal MAC loopback mode disabled.\n");
+	}
+	spin_unlock_bh(&tp->lock);
+
+	return err;
+}
+
 static inline void tg3_set_mtu(struct net_device *dev, struct tg3 *tp,
 			       int new_mtu)
 {
@@ -15028,6 +15055,7 @@ static const struct net_device_ops tg3_netdev_ops = {
 	.ndo_tx_timeout		= tg3_tx_timeout,
 	.ndo_change_mtu		= tg3_change_mtu,
 	.ndo_fix_features	= tg3_fix_features,
+	.ndo_set_features	= tg3_set_features,
 #ifdef CONFIG_NET_POLL_CONTROLLER
 	.ndo_poll_controller	= tg3_poll_controller,
 #endif
@@ -15044,6 +15072,7 @@ static const struct net_device_ops tg3_netdev_ops_dma_bug = {
 	.ndo_do_ioctl		= tg3_ioctl,
 	.ndo_tx_timeout		= tg3_tx_timeout,
 	.ndo_change_mtu		= tg3_change_mtu,
+	.ndo_set_features	= tg3_set_features,
 #ifdef CONFIG_NET_POLL_CONTROLLER
 	.ndo_poll_controller	= tg3_poll_controller,
 #endif
@@ -15241,6 +15270,9 @@ static int __devinit tg3_init_one(struct pci_dev *pdev,
 	dev->features |= hw_features;
 	dev->vlan_features |= hw_features;
 
+	/* Add the loopback capability */
+	dev->hw_features |= NETIF_F_LOOPBACK;
+
 	if (tp->pci_chip_rev_id == CHIPREV_ID_5705_A1 &&
 	    !tg3_flag(tp, TSO_CAPABLE) &&
 	    !(tr32(TG3PCI_PCISTATE) & PCISTATE_BUS_SPEED_HIGH)) {
-- 
1.7.3.1


^ permalink raw reply related

* Re: A race in register_netdevice()
From: Stephen Hemminger @ 2011-04-28 23:52 UTC (permalink / raw)
  To: Kalle Valo
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <87y62ugg0a.fsf-5ukZ45wKbUHoml4zekdYB16hYfS7NtTn@public.gmane.org>

On Fri, 29 Apr 2011 01:36:37 +0300
Kalle Valo <kvalo-BkwN83ws05HQT0dZR+AlfA@public.gmane.org> wrote:

> Hi,
> 
> there seems to be a race in register_netdevice(), which is reported here:
> 
> https://bugzilla.kernel.org/show_bug.cgi?id=15606
> 
> This is visible at least with flimflam and ath6kl. Basically what
> happens is this:
> 
> Apr 29 00:21:35 roska flimflamd[2598]: src/udev.c:add_net_device() 
> Apr 29 00:21:35 roska flimflamd[2598]: connman_inet_ifname: SIOCGIFNAME(index
> 4): No such device
> Apr 29 00:21:45 roska flimflamd[2598]: src/rtnl.c:rtnl_message() buf
> 0xbfefda3c len 1004
> Apr 29 00:21:45 roska flimflamd[2598]: src/rtnl.c:rtnl_message()
> NEWLINK len 1004 type 16 flags 0x0000 seq 0
> 
> (ignore the 10 s delay, I added that to reproduce the issue easily)
> 
> There are two ways to fix this, first is to move kobject registration
> after the call to list_netdevice():
> 
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -5425,11 +5425,6 @@ int register_netdevice(struct net_device *dev)
>         if (ret)
>                 goto err_uninit;
>  
> -       ret = netdev_register_kobject(dev);
> -       if (ret)
> -               goto err_uninit;
> -       dev->reg_state = NETREG_REGISTERED;
> -
>         netdev_update_features(dev);
>  
>         /*
> @@ -5443,6 +5438,11 @@ int register_netdevice(struct net_device *dev)
>         dev_hold(dev);
>         list_netdevice(dev);
>  
> +       ret = netdev_register_kobject(dev);
> +       if (ret)
> +               goto err_uninit;
> +       dev->reg_state = NETREG_REGISTERED;
> +
>         /* Notify protocols, that a new device appeared. */
>         ret = call_netdevice_notifiers(NETDEV_REGISTER, dev);
>         ret = notifier_to_errno(ret);
> 
> Other option, noticed by Jouni Malinen, is to take rtnl for
> SIOCGIFNAME. For some reason it's currently unprotected:
> 
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -4917,8 +4917,12 @@ int dev_ioctl(struct net *net, unsigned int
> cmd, void __user *arg)
>           rtnl_unlock();
>                 return ret;
>                 }
> -               if (cmd == SIOCGIFNAME)
> -                  return dev_ifname(net, (struct ifreq __user *)arg);
> +                  if (cmd == SIOCGIFNAME) {
> +                     rtnl_lock();
> +                       ret = dev_ifname(net, (struct ifreq __user
> -                  *)arg);
> +                       rtnl_unlock();
> +                               return ret;
> +                               }
>  
>         if (copy_from_user(&ifr, arg, sizeof(struct ifreq)))
>            return -EFAULT;
> 
> I have confirmed that both of these patches fix the issue. Now I'm
> wondering which one is the best way forward. Or is there a better way
> to fix this?
> 

I see no problem with moving this.
SIOCGIFNAME should not need to hold rtnl.


-- 
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" 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

* Re: [PATCH 2/2] tg3: Add code to allow ethtool to enable/disable loopback.
From: Matt Carlson @ 2011-04-29  0:28 UTC (permalink / raw)
  To: Mahesh Bandewar
  Cc: David Miller, Matthew Carlson, netdev, Michael Chan,
	Ben Hutchings, Micha? Miros?aw
In-Reply-To: <1304033599-8395-3-git-send-email-maheshb@google.com>

On Thu, Apr 28, 2011 at 04:33:19PM -0700, Mahesh Bandewar wrote:
> ---
>  drivers/net/tg3.c |   32 ++++++++++++++++++++++++++++++++
>  1 files changed, 32 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
> index fa57e3d..208884d 100644
> --- a/drivers/net/tg3.c
> +++ b/drivers/net/tg3.c
> @@ -6319,6 +6319,33 @@ static u32 tg3_fix_features(struct net_device *dev, u32 features)
>  	return features;
>  }
>  
> +static int tg3_set_features(struct net_device *dev, u32 features)
> +{
> +	struct tg3 *tp = netdev_priv(dev);
> +	u32 cur_mode = 0;
> +	int err = 0;
> +
> +	spin_lock_bh(&tp->lock);
> +	cur_mode = tr32(MAC_MODE);
> +
> +	if (features & NETIF_F_LOOPBACK) {
> +		/* Enable internal MAC loopback mode */
> +		tw32(MAC_MODE, cur_mode | MAC_MODE_PORT_INT_LPBACK);
> +		netif_carrier_on(tp->dev);
> +		netdev_info(dev, "Internal MAC loopback mode enabled.\n");
> +	} else {
> +		/* Disable internal MAC loopback mode */
> +		tw32(MAC_MODE, cur_mode & ~MAC_MODE_PORT_INT_LPBACK);
> +		/* Force link status check */
> +		if (netif_running(dev))
> +			tg3_setup_phy(tp, 1);
> +		netdev_info(dev, "Internal MAC loopback mode disabled.\n");
> +	}

I think we should be checking netif_running() before touching the
hardware at all.  It might be in a low power state.

Also, wouldn't it be better to verify this particular bit changed before
doing any of these operations?

> +	spin_unlock_bh(&tp->lock);
> +
> +	return err;
> +}
> +
>  static inline void tg3_set_mtu(struct net_device *dev, struct tg3 *tp,
>  			       int new_mtu)
>  {
> @@ -15028,6 +15055,7 @@ static const struct net_device_ops tg3_netdev_ops = {
>  	.ndo_tx_timeout		= tg3_tx_timeout,
>  	.ndo_change_mtu		= tg3_change_mtu,
>  	.ndo_fix_features	= tg3_fix_features,
> +	.ndo_set_features	= tg3_set_features,
>  #ifdef CONFIG_NET_POLL_CONTROLLER
>  	.ndo_poll_controller	= tg3_poll_controller,
>  #endif
> @@ -15044,6 +15072,7 @@ static const struct net_device_ops tg3_netdev_ops_dma_bug = {
>  	.ndo_do_ioctl		= tg3_ioctl,
>  	.ndo_tx_timeout		= tg3_tx_timeout,
>  	.ndo_change_mtu		= tg3_change_mtu,
> +	.ndo_set_features	= tg3_set_features,
>  #ifdef CONFIG_NET_POLL_CONTROLLER
>  	.ndo_poll_controller	= tg3_poll_controller,
>  #endif
> @@ -15241,6 +15270,9 @@ static int __devinit tg3_init_one(struct pci_dev *pdev,
>  	dev->features |= hw_features;
>  	dev->vlan_features |= hw_features;
>  
> +	/* Add the loopback capability */
> +	dev->hw_features |= NETIF_F_LOOPBACK;

Not all tg3 devices can do MAC loopback.  I'd suggest qualifying this
with:

		if (GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_5780 ||
		    (tp->tg3_flags & TG3_FLAG_CPMU_PRESENT))

But that will exclude a lot of our newer devices.  Does it matter what
type of loopback is used?  Newer devices prefer internal phy loopback
over MAC loopback.

>  	if (tp->pci_chip_rev_id == CHIPREV_ID_5705_A1 &&
>  	    !tg3_flag(tp, TSO_CAPABLE) &&
>  	    !(tr32(TG3PCI_PCISTATE) & PCISTATE_BUS_SPEED_HIGH)) {
> -- 
> 1.7.3.1
> 
> 


^ permalink raw reply

* [PATCHv2 1/2] net: Export dev_queue_xmit_nit for use by macvlan driver
From: David Ward @ 2011-04-29  0:22 UTC (permalink / raw)
  To: netdev; +Cc: David Ward, Patrick McHardy
In-Reply-To: <1304036552-1589-1-git-send-email-david.ward@ll.mit.edu>

Export dev_queue_xmit_nit for use by the macvlan virtual network device
driver. Also, use 'dev' instead of 'skb->dev' in this function.

Signed-off-by: David Ward <david.ward@ll.mit.edu>
---
 include/linux/netdevice.h |    2 ++
 net/core/dev.c            |   14 +++++++++-----
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index cb8178a..b63e517 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2099,6 +2099,8 @@ extern int		dev_hard_start_xmit(struct sk_buff *skb,
 					    struct netdev_queue *txq);
 extern int		dev_forward_skb(struct net_device *dev,
 					struct sk_buff *skb);
+extern void		dev_queue_xmit_nit(struct sk_buff *skb,
+					   struct net_device *dev);
 
 extern int		netdev_budget;
 
diff --git a/net/core/dev.c b/net/core/dev.c
index 3bbb4c2..b15622e 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1521,11 +1521,13 @@ static inline int deliver_skb(struct sk_buff *skb,
 }
 
 /*
- *	Support routine. Sends outgoing frames to any network
- *	taps currently in use.
+ * dev_queue_xmit_nit - send outgoing frame to AF_PACKET sockets
+ *
+ * @skb: buffer to send
+ * @dev: network device that AF_PACKET sockets are attached to (if any)
  */
 
-static void dev_queue_xmit_nit(struct sk_buff *skb, struct net_device *dev)
+void dev_queue_xmit_nit(struct sk_buff *skb, struct net_device *dev)
 {
 	struct packet_type *ptype;
 	struct sk_buff *skb2 = NULL;
@@ -1540,7 +1542,8 @@ static void dev_queue_xmit_nit(struct sk_buff *skb, struct net_device *dev)
 		    (ptype->af_packet_priv == NULL ||
 		     (struct sock *)ptype->af_packet_priv != skb->sk)) {
 			if (pt_prev) {
-				deliver_skb(skb2, pt_prev, skb->dev);
+				atomic_inc(&skb2->users);
+				pt_prev->func(skb2, dev, pt_prev, dev);
 				pt_prev = ptype;
 				continue;
 			}
@@ -1573,9 +1576,10 @@ static void dev_queue_xmit_nit(struct sk_buff *skb, struct net_device *dev)
 		}
 	}
 	if (pt_prev)
-		pt_prev->func(skb2, skb->dev, pt_prev, skb->dev);
+		pt_prev->func(skb2, dev, pt_prev, dev);
 	rcu_read_unlock();
 }
+EXPORT_SYMBOL(dev_queue_xmit_nit);
 
 /* netif_setup_tc - Handle tc mappings on real_num_tx_queues change
  * @dev: Network device
-- 
1.7.4.4


^ permalink raw reply related

* [PATCHv2 0/2] Resolve packet capturing on macvlan lowerdev
From: David Ward @ 2011-04-29  0:22 UTC (permalink / raw)
  To: netdev; +Cc: David Ward, Patrick McHardy

Change in v2: Declare variables in basic block before other code appears

The following two patches address situations where macvlan interfaces on 
the same lowerdev are created inside separate containers/namespaces, and 
traffic between these interfaces needs to be captured by monitoring the 
lowerdev outside the containers/namespaces using tcpdump or Wireshark. 
The only case where this doesn't work now is for unicast frames when the 
macvlan interfaces are operating in bridge mode; this fixes that case.

Should the dev_queue_xmit_nit function be renamed to something more 
meaningful, which would indicate its role in sending outgoing frames to 
AF_PACKET sockets? It is currently a misnomer: this function used to be 
invoked by dev_queue_xmit, but that is no longer the case.

Thanks,

David

David Ward (2):
  net: Export dev_queue_xmit_nit for use by macvlan driver
  macvlan: Send frames to AF_PACKET sockets attached to lowerdev

 drivers/net/macvlan.c     |    5 ++++-
 include/linux/netdevice.h |    2 ++
 net/core/dev.c            |   14 +++++++++-----
 3 files changed, 15 insertions(+), 6 deletions(-)

-- 
1.7.4.4


^ permalink raw reply

* [PATCHv2 2/2] macvlan: Send frames to AF_PACKET sockets attached to lowerdev
From: David Ward @ 2011-04-29  0:22 UTC (permalink / raw)
  To: netdev; +Cc: David Ward, Patrick McHardy
In-Reply-To: <1304036552-1589-1-git-send-email-david.ward@ll.mit.edu>

In bridge mode, unicast frames can be forwarded directly between macvlan
interfaces attached to the same lowerdev without calling dev_queue_xmit.
These frames should still be sent to any AF_PACKET sockets (network taps)
attached to the lowerdev.

Signed-off-by: David Ward <david.ward@ll.mit.edu>
---
 drivers/net/macvlan.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index 3ad5425..25c7632 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -238,7 +238,10 @@ static int macvlan_queue_xmit(struct sk_buff *skb, struct net_device *dev)
 		dest = macvlan_hash_lookup(port, eth->h_dest);
 		if (dest && dest->mode == MACVLAN_MODE_BRIDGE) {
 			unsigned int length = skb->len + ETH_HLEN;
-			int ret = dest->forward(dest->dev, skb);
+			int ret = NET_RX_DROP;
+
+			dev_queue_xmit_nit(skb, vlan->lowerdev);
+			ret = dest->forward(dest->dev, skb);
 			macvlan_count_rx(dest, length,
 					 ret == NET_RX_SUCCESS, 0);
 
-- 
1.7.4.4


^ permalink raw reply related

* Re: [PATCH 2/2] tg3: Add code to allow ethtool to enable/disable loopback.
From: Mahesh Bandewar @ 2011-04-29  1:42 UTC (permalink / raw)
  To: Matt Carlson
  Cc: David Miller, netdev, Michael Chan, Ben Hutchings,
	Micha? Miros?aw
In-Reply-To: <20110429002855.GC19665@mcarlson.broadcom.com>

On Thu, Apr 28, 2011 at 5:28 PM, Matt Carlson <mcarlson@broadcom.com> wrote:
> On Thu, Apr 28, 2011 at 04:33:19PM -0700, Mahesh Bandewar wrote:
>> ---
>>  drivers/net/tg3.c |   32 ++++++++++++++++++++++++++++++++
>>  1 files changed, 32 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
>> index fa57e3d..208884d 100644
>> --- a/drivers/net/tg3.c
>> +++ b/drivers/net/tg3.c
>> @@ -6319,6 +6319,33 @@ static u32 tg3_fix_features(struct net_device *dev, u32 features)
>>       return features;
>>  }
>>
>> +static int tg3_set_features(struct net_device *dev, u32 features)
>> +{
>> +     struct tg3 *tp = netdev_priv(dev);
>> +     u32 cur_mode = 0;
>> +     int err = 0;
>> +
>> +     spin_lock_bh(&tp->lock);
>> +     cur_mode = tr32(MAC_MODE);
>> +
>> +     if (features & NETIF_F_LOOPBACK) {
>> +             /* Enable internal MAC loopback mode */
>> +             tw32(MAC_MODE, cur_mode | MAC_MODE_PORT_INT_LPBACK);
>> +             netif_carrier_on(tp->dev);
>> +             netdev_info(dev, "Internal MAC loopback mode enabled.\n");
>> +     } else {
>> +             /* Disable internal MAC loopback mode */
>> +             tw32(MAC_MODE, cur_mode & ~MAC_MODE_PORT_INT_LPBACK);
>> +             /* Force link status check */
>> +             if (netif_running(dev))
>> +                     tg3_setup_phy(tp, 1);
>> +             netdev_info(dev, "Internal MAC loopback mode disabled.\n");
>> +     }
>
> I think we should be checking netif_running() before touching the
> hardware at all.  It might be in a low power state.
>
makes sense. I'll change that.

> Also, wouldn't it be better to verify this particular bit changed before
> doing any of these operations?
>
Right! Since netdev infrastructure initiates ndo_set_features() only
when the feature-set is changed, and loopback is the only thing that
is set; I lazily ignored that. But in future additional features could
be set and it's not going to hurt checking before setting that bit.

>> +     spin_unlock_bh(&tp->lock);
>> +
>> +     return err;
>> +}
>> +
>>  static inline void tg3_set_mtu(struct net_device *dev, struct tg3 *tp,
>>                              int new_mtu)
>>  {
>> @@ -15028,6 +15055,7 @@ static const struct net_device_ops tg3_netdev_ops = {
>>       .ndo_tx_timeout         = tg3_tx_timeout,
>>       .ndo_change_mtu         = tg3_change_mtu,
>>       .ndo_fix_features       = tg3_fix_features,
>> +     .ndo_set_features       = tg3_set_features,
>>  #ifdef CONFIG_NET_POLL_CONTROLLER
>>       .ndo_poll_controller    = tg3_poll_controller,
>>  #endif
>> @@ -15044,6 +15072,7 @@ static const struct net_device_ops tg3_netdev_ops_dma_bug = {
>>       .ndo_do_ioctl           = tg3_ioctl,
>>       .ndo_tx_timeout         = tg3_tx_timeout,
>>       .ndo_change_mtu         = tg3_change_mtu,
>> +     .ndo_set_features       = tg3_set_features,
>>  #ifdef CONFIG_NET_POLL_CONTROLLER
>>       .ndo_poll_controller    = tg3_poll_controller,
>>  #endif
>> @@ -15241,6 +15270,9 @@ static int __devinit tg3_init_one(struct pci_dev *pdev,
>>       dev->features |= hw_features;
>>       dev->vlan_features |= hw_features;
>>
>> +     /* Add the loopback capability */
>> +     dev->hw_features |= NETIF_F_LOOPBACK;
>
> Not all tg3 devices can do MAC loopback.  I'd suggest qualifying this
> with:
>
>                if (GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_5780 ||
>                    (tp->tg3_flags & TG3_FLAG_CPMU_PRESENT))
>
> But that will exclude a lot of our newer devices.  Does it matter what
> type of loopback is used?  Newer devices prefer internal phy loopback
> over MAC loopback.
>
As long as device supports some sort of loopback, we should be setting
this capability and move this logic (or similar) to set_features() and
choose the method that is supported. Since several devices support
loopback at various levels, to keep it consistent, we should be
setting the loopback closest to the host. So can I simply set the
int-phy loopback in the else part of the above 'provided if' or would
need other logic? or in other words -

 if (GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_5780 ||
(tp->tg3_flags & TG3_FLAG_CPMU_PRESENT))
    supported_mode = MAC;
else
    supported_mode = INTPHY;

if (supported_mode == MAC)
    cur_mode = tr32(MAC_MODE);
else
    tg3_readphy(tp, MII_BMCR, &cur_mode);

Would something like this work?

Thanks,
--mahesh..

>>       if (tp->pci_chip_rev_id == CHIPREV_ID_5705_A1 &&
>>           !tg3_flag(tp, TSO_CAPABLE) &&
>>           !(tr32(TG3PCI_PCISTATE) & PCISTATE_BUS_SPEED_HIGH)) {
>> --
>> 1.7.3.1
>>
>>
>
>

^ permalink raw reply

* Re: r8169 :  always copying the rx buffer to new skb
From: John Lumby @ 2011-04-29  1:55 UTC (permalink / raw)
  To: Francois Romieu; +Cc: netdev, Ben Hutchings, nic_swsd
In-Reply-To: <20110427203544.GB19708@electric-eye.fr.zoreil.com>

On 04/27/11 16:35, Francois Romieu wrote:
>
> The patch mixes different changes. Please avoid it.

Sorry about that,  I'll rewrite with only the changes absolutely needed 
for avoiding memcpy (and maybe the setting of num_rx_bufs ring param?)

> Your MUA damaged the patch. Documentation/SubmittingPatches
> could help if you have not read it yet.

I see some truncation happened,  will fix that in next submission

> The patch makes some gratuitous changes which needlessly
> increase the differences (dirty_xy rename for instance).

will revert those

> A set_ringparam() method which does nothing until open()
> is used does not exactly ring like "least surprize behavior"
> to me.

Please see questions below

> The behavior under memory pressure is still unknown.

I have run some initial tests with memory pressure  -  the pressure 
provided by running n concurrent memory hogs,  each of which loops 
endlessly on allocating 1024 blocks of 1MB bytes each,  writing 
something into all bytes in each block,   then freeing each block,   
then repeating.  result:

*
copybreak        numhogs     workload throughput  swapping alloc 
failures?     dropped packets
                                    
Mb/sec                                   or other NIC err reports?

  16383              0              1043            none           
no                 no
     64              0              1086              |            
no                 no

  16383              1               935           moderate        
no                 no
     64              1               902              |            
no                 no

  16383              2               854           heavy           
no                 no
     64              2               851              |           yes, 
many           no

  16383              3               817           very heavy      
no                 no
     64              3            did not attempt     |
*
   Conclusions  :
     . setting copybreak to 16383 seems to be a valid way of avoiding 
alloc failures when under heavy memory pressure,  although the alloc 
failures don't seem to cause much trouble in these runs.
     .  But I am surprised to see how well the copybreak=16383 case runs 
with no memory pressure,   much better than I saw for the unpatched 
2.6.39rc2 earlier on,  and I need to run some more tests to check 
that.     I will also run same tests on the vanilla 2.6.39.

> I am mildly convinced by the implementation.
>

Thanks for all comments.

I do have a couple more questions:

    .    for my next patch submission  -   what should I base it on?     
Is there a git project which has the "latest" version of r8169.c?    I 
think it's not  torvalds/linux-2.6.git as fixes to r8169.c in that 
project go only to 2011-03-21.   Sorry if this is dumb question.
    .     regarding setting the ring param  -  I understand your comment 
but is it safe to close and open the NIC when called by ethtool 
ioctl?        Would some locking be needed?
    .     and again on the ring params  -   what is the minimum and 
maximum valid value for num rx bufs and separately for num  tx bufs   
that the r8169 supports?


Cheers,   John Lumby

^ permalink raw reply

* linux-next: manual merge of the suspend tree with the net tree
From: Stephen Rothwell @ 2011-04-29  2:05 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: linux-next, linux-kernel, Eric Dumazet, David Miller, netdev

Hi Rafael,

Today's linux-next merge of the suspend tree got a conflict in
arch/x86/Kconfig between commit 0a14842f5a3c ("net: filter: Just In Time
compiler for x86-64") from the  tree and commit d9f1ce017fcf ("PM: Remove
sysdev suspend, resume and shutdown operations") from the suspend tree.

Just context changes.  I fixed it up (see below) and can carry the fix as
necessary.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

diff --cc arch/x86/Kconfig
index 92d07bd,140e254..0000000
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@@ -71,8 -71,6 +71,7 @@@ config X8
  	select GENERIC_IRQ_SHOW
  	select IRQ_FORCED_THREADING
  	select USE_GENERIC_SMP_HELPERS if SMP
- 	select ARCH_NO_SYSDEV_OPS
 +	select HAVE_BPF_JIT if X86_64
  
  config INSTRUCTION_DECODER
  	def_bool (KPROBES || PERF_EVENTS)

^ permalink raw reply

* Re: [PATCH 2/2] tg3: Add code to allow ethtool to enable/disable loopback.
From: Matt Carlson @ 2011-04-29  2:23 UTC (permalink / raw)
  To: Mahesh Bandewar
  Cc: David Miller, Matthew Carlson, netdev, Michael Chan,
	Ben Hutchings, Micha? Miros?aw
In-Reply-To: <1304033599-8395-3-git-send-email-maheshb@google.com>

On Thu, Apr 28, 2011 at 04:33:19PM -0700, Mahesh Bandewar wrote:
> ---
>  drivers/net/tg3.c |   32 ++++++++++++++++++++++++++++++++
>  1 files changed, 32 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
> index fa57e3d..208884d 100644
> --- a/drivers/net/tg3.c
> +++ b/drivers/net/tg3.c
> @@ -6319,6 +6319,33 @@ static u32 tg3_fix_features(struct net_device *dev, u32 features)
>  	return features;
>  }
>  
> +static int tg3_set_features(struct net_device *dev, u32 features)
> +{
> +	struct tg3 *tp = netdev_priv(dev);
> +	u32 cur_mode = 0;
> +	int err = 0;
> +
> +	spin_lock_bh(&tp->lock);
> +	cur_mode = tr32(MAC_MODE);
> +
> +	if (features & NETIF_F_LOOPBACK) {
> +		/* Enable internal MAC loopback mode */
> +		tw32(MAC_MODE, cur_mode | MAC_MODE_PORT_INT_LPBACK);

I didn't notice this before, but you will want to clear
MAC_MODE_HALF_DUPLEX when going into loopback mode.  You won't get
packets back if you happen to negotiate to HD.



^ permalink raw reply

* Re: [PATCH 2/2] tg3: Add code to allow ethtool to enable/disable loopback.
From: Matt Carlson @ 2011-04-29  2:46 UTC (permalink / raw)
  To: Mahesh Bandewar
  Cc: Matthew Carlson, David Miller, netdev, Michael Chan,
	Ben Hutchings, Micha? Miros?aw
In-Reply-To: <BANLkTimA9YGFKjB0_UMsAJeSGEbKGoFJXQ@mail.gmail.com>

On Thu, Apr 28, 2011 at 06:42:02PM -0700, Mahesh Bandewar wrote:
> >> + ? ? spin_unlock_bh(&tp->lock);
> >> +
> >> + ? ? return err;
> >> +}
> >> +
> >> ?static inline void tg3_set_mtu(struct net_device *dev, struct tg3 *tp,
> >> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?int new_mtu)
> >> ?{
> >> @@ -15028,6 +15055,7 @@ static const struct net_device_ops tg3_netdev_ops = {
> >> ? ? ? .ndo_tx_timeout ? ? ? ? = tg3_tx_timeout,
> >> ? ? ? .ndo_change_mtu ? ? ? ? = tg3_change_mtu,
> >> ? ? ? .ndo_fix_features ? ? ? = tg3_fix_features,
> >> + ? ? .ndo_set_features ? ? ? = tg3_set_features,
> >> ?#ifdef CONFIG_NET_POLL_CONTROLLER
> >> ? ? ? .ndo_poll_controller ? ?= tg3_poll_controller,
> >> ?#endif
> >> @@ -15044,6 +15072,7 @@ static const struct net_device_ops tg3_netdev_ops_dma_bug = {
> >> ? ? ? .ndo_do_ioctl ? ? ? ? ? = tg3_ioctl,
> >> ? ? ? .ndo_tx_timeout ? ? ? ? = tg3_tx_timeout,
> >> ? ? ? .ndo_change_mtu ? ? ? ? = tg3_change_mtu,
> >> + ? ? .ndo_set_features ? ? ? = tg3_set_features,
> >> ?#ifdef CONFIG_NET_POLL_CONTROLLER
> >> ? ? ? .ndo_poll_controller ? ?= tg3_poll_controller,
> >> ?#endif
> >> @@ -15241,6 +15270,9 @@ static int __devinit tg3_init_one(struct pci_dev *pdev,
> >> ? ? ? dev->features |= hw_features;
> >> ? ? ? dev->vlan_features |= hw_features;
> >>
> >> + ? ? /* Add the loopback capability */
> >> + ? ? dev->hw_features |= NETIF_F_LOOPBACK;
> >
> > Not all tg3 devices can do MAC loopback. ?I'd suggest qualifying this
> > with:
> >
> > ? ? ? ? ? ? ? ?if (GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_5780 ||
> > ? ? ? ? ? ? ? ? ? ?(tp->tg3_flags & TG3_FLAG_CPMU_PRESENT))
> >
> > But that will exclude a lot of our newer devices. ?Does it matter what
> > type of loopback is used? ?Newer devices prefer internal phy loopback
> > over MAC loopback.
> >
> As long as device supports some sort of loopback, we should be setting
> this capability and move this logic (or similar) to set_features() and
> choose the method that is supported. Since several devices support
> loopback at various levels, to keep it consistent, we should be
> setting the loopback closest to the host. So can I simply set the
> int-phy loopback in the else part of the above 'provided if' or would
> need other logic? or in other words -
> 
>  if (GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_5780 ||
> (tp->tg3_flags & TG3_FLAG_CPMU_PRESENT))
>     supported_mode = MAC;
> else
>     supported_mode = INTPHY;
> 
> if (supported_mode == MAC)
>     cur_mode = tr32(MAC_MODE);
> else
>     tg3_readphy(tp, MII_BMCR, &cur_mode);
> 
> Would something like this work?

That might be where we want to end up, but it'll be some work to get
there.  Maybe it makes sense to just keep MAC loopback mode for now and
only enable it for a subset of devices.  Adding phy loopback mode sounds
like it will require some surgery.


^ permalink raw reply

* Re: [PATCH 08/13] netvm: Allow skb allocation to use PFMEMALLOC reserves
From: NeilBrown @ 2011-04-29  2:55 UTC (permalink / raw)
  To: Mel Gorman; +Cc: Linux-MM, Linux-Netdev, LKML, David Miller, Peter Zijlstra
In-Reply-To: <20110428111854.GV4658@suse.de>

On Thu, 28 Apr 2011 12:18:54 +0100 Mel Gorman <mgorman@suse.de> wrote:

> On Thu, Apr 28, 2011 at 08:47:55PM +1000, NeilBrown wrote:
> > On Thu, 28 Apr 2011 11:05:06 +0100 Mel Gorman <mgorman@suse.de> wrote:
> > 
> > > On Thu, Apr 28, 2011 at 04:19:33PM +1000, NeilBrown wrote:
> > > > On Wed, 27 Apr 2011 17:08:06 +0100 Mel Gorman <mgorman@suse.de> wrote:
> > > > 
> > > > 
> > > > > @@ -1578,7 +1589,7 @@ static inline struct sk_buff *netdev_alloc_skb_ip_align(struct net_device *dev,
> > > > >   */
> > > > >  static inline struct page *__netdev_alloc_page(struct net_device *dev, gfp_t gfp_mask)
> > > > >  {
> > > > > -	return alloc_pages_node(NUMA_NO_NODE, gfp_mask, 0);
> > > > > +	return alloc_pages_node(NUMA_NO_NODE, gfp_mask | __GFP_MEMALLOC, 0);
> > > > >  }
> > > > >  
> > > > 
> > > > I'm puzzling a bit over this change.
> > > > __netdev_alloc_page appears to be used to get pages to put in ring buffer
> > > > for a network card to DMA received packets into.  So it is OK to use
> > > > __GFP_MEMALLOC for these allocations providing we mark the resulting skb as
> > > > 'pfmemalloc' if a reserved page was used.
> > > > 
> > > > However I don't see where that marking is done.
> > > > I think it should be in skb_fill_page_desc, something like:
> > > > 
> > > >   if (page->pfmemalloc)
> > > > 	skb->pfmemalloc = true;
> > > > 
> > > > Is this covered somewhere else that I am missing?
> > > > 
> > > 
> > > You're not missing anything.
> > > 
> > > >From the context of __netdev_alloc_page, we do not know if the skb
> > > is suitable for marking pfmemalloc or not (we don't have SKB_ALLOC_RX
> > > flag for example that __alloc_skb has). The reserves are potentially
> > > being dipped into for an unsuitable packet but it gets dropped in
> > > __netif_receive_skb() and the memory is returned. If we mark the skb
> > > pfmemalloc as a result of __netdev_alloc_page using a reserve page, the
> > > packets would not get dropped as expected.
> > > 
> > 
> > The only code in __netif_receive_skb that seems to drop packets is
> > 
> > +	if (skb_pfmemalloc(skb) && !skb_pfmemalloc_protocol(skb))
> > +		goto drop;
> > +
> > 
> > which requires that the skb have pfmemalloc set before it will be dropped.
> > 
> 
> Yes, I only wanted to drop the packet if we were under pressure
> when skb was allocated. If we hit pressure between when skb was
> allocated and when __netdev_alloc_page is called, then the PFMEMALLOC
> reserves may be used for packet receive unnecessarily but the next skb
> allocation that grows slab will have the flag set appropriately. There
> is a window during which we use reserves where we did not have to
> but it's limited. Again, the throttling if pfmemalloc reserves gets too
> depleted comes into play.

I don't find this very convincing...
It seems inconsistent that you are doing precise accounting inside slab so
that you know which object used reserved memory and which did not, yet you
get sloppy with the accounting of whole pages on network receive.

Is there a clear upper bound on how many reserve pages could slip into
non-reserve skbs before skbs start getting the pfmalloc flag set?

I just think it is safer to mark an skb as pfmalloc if any part of the memory
associated with it came from reserves.

Also I find the throttling argument hard to reason about.  Certainly
some things get throttles, but incoming packets don't...

I'm certainly not saying that the code is clearly wrong, but I'm having
trouble convincing myself that it is clearly right (or at least 'safe').

> 
> > Actually ... I'm expecting to find code that says:
> >    if (skb_pfmalloc(skb) && !sock_flag(sk, SOCK_MEMALLOC))
> > 	drop_packet();
> > 
> > but I cannot find it.  Where is the code that discard pfmalloc packets for
> > non-memalloc sockets?
> > 
> > I can see similar code in sk_filter but that doesn't drop the packet, it just
> > avoids filtering it.
> > 
> 
> hmm, if sk_filter is returning -ENOMEM then things like
> sock_queue_rcv_skb() return error and the skb does not get queued and I
> expected it to get dropped. What did I miss?
> 

Just that I was making incorrect assumptions about code that I wasn't
familiar with.
Make sense now.

Thanks,
NeilBrown

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* Re: [ethtool PATCH 6/6] Update documentation for -u/-U operations
From: Ben Hutchings @ 2011-04-29  2:57 UTC (permalink / raw)
  To: Alexander Duyck
  Cc: davem@davemloft.net, Kirsher, Jeffrey T, netdev@vger.kernel.org
In-Reply-To: <4DB9D0D9.2000401@intel.com>

On Thu, 2011-04-28 at 13:40 -0700, Alexander Duyck wrote:
> On 4/27/2011 11:23 AM, Ben Hutchings wrote:
> > On Thu, 2011-04-21 at 13:40 -0700, Alexander Duyck wrote:
[...]
> >> --- a/ethtool.8.in
> >> +++ b/ethtool.8.in
> >> @@ -42,10 +42,20 @@
> >>   [\\fB\\$1\\fP\ \\fIN\\fP]
> >>   ..
> >>   .\"
> >> +.\"	.BM - same as above but has a mask field for format "[value N [value-mask N]]"
> >> +.\"
> >> +.de BM
> >> +[\\fB\\$1\\fP\ \\fIN\\fP\ [\\fB\\$1\-mask\\fP\ \\fIN\\fP]]
> >
> > You've changed the code to accept 'm' as an alternative to
> > <field>  '-mask', so this should be changed accordingly.
> 
> What would be the preferred way of stating that?  For now I just 
> replaced the \\$1\-mask with m.  However I am assuming that probably 
> isn't the best approach either.  Should I state somewhere that m can be 
> replaced with "field name"-mask?

I think that's reasonable.

[...]
> >> +.BN class-rule-del
> >> +.RB [\  flow-type \ \*(NC
> >> +.RB [ src \ \*(MA\ [ src-mask \ \*(MA]]
> >> +.RB [ dst \ \*(MA\ [ dst-mask \ \*(MA]]
> >> +.BM proto
> >> +.RB [ src-ip \ \*(PA\ [ src-ip-mask \ \*(PA]]
> >> +.RB [ dst-ip \ \*(PA\ [ dst-ip-mask \ \*(PA]]
> >> +.BM tos
> >> +.BM l4proto
> >> +.BM src-port
> >> +.BM dst-port
> >> +.BM spi
> >> +.BM vlan-etype
> >> +.BM vlan
> >> +.BM user-def
> >> +.BN action
> >> +.BN loc
> >> +.RB ]
> >
> > But these options aren't all applicable to all flow-types.
> 
> This is the part that gets messy and I am not sure what the best 
> approach is.  I have more comments on that below.  For now what I am 
> planning to implement to address this is that in the "DESCRIPTION" 
> section below I add a statement to each specifier that has restrictions 
> by stating "Valid for flow-types X, Y, and Z."

OK.

> > [...]
> >> diff --git a/ethtool.c b/ethtool.c
> >> index 421fe20..e65979d 100644
> >> --- a/ethtool.c
> >> +++ b/ethtool.c
> >> @@ -243,20 +243,26 @@ static struct option {
> >>   		"		equal N | weight W0 W1 ...\n" },
> >>       { "-U", "--config-ntuple", MODE_SCLSRULE, "Configure Rx ntuple filters "
> >>   		"and actions",
> >> -		"		{ flow-type tcp4|udp4|sctp4\n"
> >> -		"		  [ src-ip ADDR [src-ip-mask MASK] ]\n"
> >> -		"		  [ dst-ip ADDR [dst-ip-mask MASK] ]\n"
> >> -		"		  [ src-port PORT [src-port-mask MASK] ]\n"
> >> -		"		  [ dst-port PORT [dst-port-mask MASK] ]\n"
> >> -		"		| flow-type ether\n"
> >> -		"		  [ src MAC-ADDR [src-mask MASK] ]\n"
> >> -		"		  [ dst MAC-ADDR [dst-mask MASK] ]\n"
> >> -		"		  [ proto N [proto-mask MASK] ] }\n"
> >> -		"		[ vlan VLAN-TAG [vlan-mask MASK] ]\n"
> >> -		"		[ user-def DATA [user-def-mask MASK] ]\n"
> >> -		"		action N\n" },
> >> +		"		[ class-rule-del %d ] |\n"
> >> +		"		[ flow-type ether|ip4|tcp4|udp4|sctp4|ah4|esp4\n"
> >> +		"			[ src %x:%x:%x:%x:%x:%x [src-mask %x:%x:%x:%x:%x:%x] ]\n"
> >> +		"			[ dst %x:%x:%x:%x:%x:%x [dst-mask %x:%x:%x:%x:%x:%x] ]\n"
> >> +		"			[ proto %d [proto-mask MASK] ]\n"
> >> +		"			[ src-ip %d.%d.%d.%d [src-ip-mask %d.%d.%d.%d] ]\n"
> >> +		"			[ dst-ip %d.%d.%d.%d [dst-ip-mask %d.%d.%d.%d] ]\n"
> >> +		"			[ tos %d [tos-mask %x] ]\n"
> >> +		"			[ l4proto %d [l4proto-mask MASK] ]\n"
> >> +		"			[ src-port %d [src-port-mask %x] ]\n"
> >> +		"			[ dst-port %d [dst-port-mask %x] ]\n"
> >> +		"			[ spi %d [spi-mask %x] ]\n"
> >> +		"			[ vlan-etype %x [vlan-etype-mask %x] ]\n"
> >> +		"			[ vlan %x [vlan-mask %x] ]\n"
> >> +		"			[ user-def %x [user-def-mask %x] ]\n"
> >> +		"			[ action %d ]\n"
> >> +		"			[ loc %d]]\n" },
> > [...]
> >
> > Again, it's not clear which options apply to which flow-types, and the
> > 'm' shortcut is not documented.
> 
> The 'm' part I agree with 100%, however the flow types are going to 
> become kinda hairy using that approach.  You basically end up with 
> something like this:
[...]

Yes, I see the problem.

> As you can see it will be a bit oversized to go through and specify 
> which flow-type options support what fields.  If that is what you want I 
> can implement it that way but for now I would prefer calling out the 
> flow-type limitations of the fields in the "DESCRIPTION" portion of the 
> man page.

In fact, even with this patch, the help for -U is pretty oversized.  It
might be better to replace the list of field names with '...' here and
only list them in full in the man page.

Ben.

-- 
Ben Hutchings, Senior Software 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

* Re: pull request: wireless-next-2.6 2011-04-25
From: Gustavo F. Padovan @ 2011-04-29  3:11 UTC (permalink / raw)
  To: Larry Finger; +Cc: David Miller, linville, linux-wireless, netdev
In-Reply-To: <4DB5DFBC.3070203@lwfinger.net>

* Larry Finger <Larry.Finger@lwfinger.net> [2011-04-25 15:55:24 -0500]:

> On 04/25/2011 03:04 PM, David Miller wrote:
> > From: David Miller<davem@davemloft.net>
> > Date: Mon, 25 Apr 2011 12:58:35 -0700 (PDT)
> >
> >> From: "John W. Linville"<linville@tuxdriver.com>
> >> Date: Mon, 25 Apr 2011 15:30:17 -0400
> >>
> >>> Here is another big batch of updates intended for 2.6.40...
> >>>
> >>> There is the usual huge batch of changes for ath9k, and iwlagn, a bunch
> >>> for ath9k_htc, rt2x00, and ath5k, a few more for mwifiex, and a handful
> >>> of others.  Also included is a big batch of Bluetooth updates as well.
> >>>
> >>> Please let me know if there are problems!
> >>
> >> Pulled, thanks a lot John.
> >
> > I guess watching the build logs for new warnings is too old fashioned
> > for people, and besides Dave will do it for everyone anyways right?
> >
> > And this one is a real bug too. :-/
> >
> > I'll push this out to net-next-2.6 on top of the wireless-next pull,
> > but please be more mindful in the future.
> 
> Unfortunately, the 64-bit variant of gcc v4.5.1 will miss some of these. I find 
> I need to compile code on a 32-bit system and using an older compiler to catch 
> all. Perhaps that explains the error that crept through here.

That could be a reason, I have gcc 4.5.2 (64-bit). I've built this code many times
before send pull request and never saw that warning, and it's clearly a bug,
indeed. 

-- 
Gustavo F. Padovan
http://profusion.mobi

^ permalink raw reply

* Re: Is 802.3ad mode in bonding useful ?
From: WeipingPan @ 2011-04-29  3:17 UTC (permalink / raw)
  To: Neil Horman; +Cc: netdev
In-Reply-To: <20110428122102.GB4165@hmsreliant.think-freely.org>

On 04/28/2011 08:21 PM, Neil Horman wrote:
> On Thu, Apr 28, 2011 at 03:33:50PM +0800, WeipingPan wrote:
>> Hi, all,
>>
>> 802.3ad mode in bonding implements 802.3ad standard.
>>
>> I am just wondering  802.3ad mode is useful,
>> since  bonding has many modes like balance-rr, active-backup, etc.
>>
> Yes, of course its usefull.  For switches which support 802.3ad, this mode
> allows for both peers to understand that the links in the bond are acting as an
> aggregate, which makes it easier to prevent things like inadvertently looped
> back frames, for which the other modes have to have all sorts of hacks to
> prevent.
What is looped back frames here ?
I didn't see any special code to handle looped back frames in other 
modes in bonding,
can you take an example ?

thanks
Weiping Pan

> Neil
>


^ permalink raw reply

* Re: pull request: wireless-next-2.6 2011-04-25
From: David Miller @ 2011-04-29  3:31 UTC (permalink / raw)
  To: padovan-Y3ZbgMPKUGA34EUeqzHoZw
  Cc: Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ,
	linville-2XuSBdqkA4R54TAoqtyWWQ,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20110429031143.GA2353@joana>

From: "Gustavo F. Padovan" <padovan-Y3ZbgMPKUGA34EUeqzHoZw@public.gmane.org>
Date: Fri, 29 Apr 2011 00:11:43 -0300

> * Larry Finger <Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ@public.gmane.org> [2011-04-25 15:55:24 -0500]:
> 
>> Unfortunately, the 64-bit variant of gcc v4.5.1 will miss some of these. I find 
>> I need to compile code on a 32-bit system and using an older compiler to catch 
>> all. Perhaps that explains the error that crept through here.
> 
> That could be a reason, I have gcc 4.5.2 (64-bit). I've built this code many times
> before send pull request and never saw that warning, and it's clearly a bug,
> indeed. 

I would buy this line of reasoning.

Except that I do all of my test builds with 64-bit gcc's, on sparc64
and x86-64.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" 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

* Re: r8169 :  always copying the rx buffer to new skb
From: Eric Dumazet @ 2011-04-29  4:54 UTC (permalink / raw)
  To: John Lumby; +Cc: Francois Romieu, netdev, Ben Hutchings, nic_swsd
In-Reply-To: <4DBA1A9A.3000703@hotmail.com>

Le jeudi 28 avril 2011 à 21:55 -0400, John Lumby a écrit :
> *
>    Conclusions  :
>      . setting copybreak to 16383 seems to be a valid way of avoiding 
> alloc failures when under heavy memory pressure,  although the alloc 
> failures don't seem to cause much trouble in these runs.
>      .  But I am surprised to see how well the copybreak=16383 case runs 
> with no memory pressure,   much better than I saw for the unpatched 
> 2.6.39rc2 earlier on,  and I need to run some more tests to check 
> that.     I will also run same tests on the vanilla 2.6.39.

Doing the copy of data and building an exact size skb has benefit of
providing 'right' skb->truesize (might reduce RCVBUF contention and
avoid backlog drops) and already cached data (hot in cpu caches). Next
'copy' is almost free (L1 cache access)

It all depends on workload. If you want to receive a huge number of
small datagrams, [and feed them to several cpus], results might be
completely different.


>     .    for my next patch submission  -   what should I base it on?     
> Is there a git project which has the "latest" version of r8169.c?    I 
> think it's not  torvalds/linux-2.6.git as fixes to r8169.c in that 
> project go only to 2011-03-21.   Sorry if this is dumb question.

This one ?

http://git.kernel.org/?p=linux/kernel/git/davem/net-next-2.6.git




^ permalink raw reply

* [PATCH 1/2] net: use NETIF_F_ALL_TSO for vlan features
From: Shan Wei @ 2011-04-29  5:20 UTC (permalink / raw)
  To: David Miller, netdev, eilong, dm, leedom, mirqus, bhutchings, dm


As Dimitris Michailidis suggested, use NETIF_F_ALL_TSO for vlan_features,
which is a mask, but not hw_features. 

Compile test.


Signed-off-by: Shan Wei <shanwei@cn.fujitsu.com>
---
 drivers/net/bnx2x/bnx2x_main.c     |    2 +-
 drivers/net/cxgb4/cxgb4_main.c     |    2 +-
 drivers/net/cxgb4vf/cxgb4vf_main.c |    2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/bnx2x/bnx2x_main.c b/drivers/net/bnx2x/bnx2x_main.c
index bfd7ac9..67b1011 100644
--- a/drivers/net/bnx2x/bnx2x_main.c
+++ b/drivers/net/bnx2x/bnx2x_main.c
@@ -9429,7 +9429,7 @@ static int __devinit bnx2x_init_dev(struct pci_dev *pdev,
 		NETIF_F_RXCSUM | NETIF_F_LRO | NETIF_F_HW_VLAN_TX;
 
 	dev->vlan_features = NETIF_F_SG | NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
-		NETIF_F_TSO | NETIF_F_TSO_ECN | NETIF_F_TSO6 | NETIF_F_HIGHDMA;
+		NETIF_F_ALL_TSO | NETIF_F_HIGHDMA;
 
 	dev->features |= dev->hw_features | NETIF_F_HW_VLAN_RX;
 	if (bp->flags & USING_DAC_FLAG)
diff --git a/drivers/net/cxgb4/cxgb4_main.c b/drivers/net/cxgb4/cxgb4_main.c
index bdc868c..d6b1a91 100644
--- a/drivers/net/cxgb4/cxgb4_main.c
+++ b/drivers/net/cxgb4/cxgb4_main.c
@@ -3526,7 +3526,7 @@ static void free_some_resources(struct adapter *adapter)
 }
 
 #define TSO_FLAGS (NETIF_F_TSO | NETIF_F_TSO6 | NETIF_F_TSO_ECN)
-#define VLAN_FEAT (NETIF_F_SG | NETIF_F_IP_CSUM | TSO_FLAGS | \
+#define VLAN_FEAT (NETIF_F_SG | NETIF_F_IP_CSUM | NETIF_F_ALL_TSO | \
 		   NETIF_F_IPV6_CSUM | NETIF_F_HIGHDMA)
 
 static int __devinit init_one(struct pci_dev *pdev,
diff --git a/drivers/net/cxgb4vf/cxgb4vf_main.c b/drivers/net/cxgb4vf/cxgb4vf_main.c
index 8cf9890..b735c52 100644
--- a/drivers/net/cxgb4vf/cxgb4vf_main.c
+++ b/drivers/net/cxgb4vf/cxgb4vf_main.c
@@ -2603,7 +2603,7 @@ static int __devinit cxgb4vf_pci_probe(struct pci_dev *pdev,
 		netdev->hw_features = NETIF_F_SG | TSO_FLAGS |
 			NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
 			NETIF_F_HW_VLAN_TX | NETIF_F_RXCSUM;
-		netdev->vlan_features = NETIF_F_SG | TSO_FLAGS |
+		netdev->vlan_features = NETIF_F_SG | NETIF_F_ALL_TSO |
 			NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
 			NETIF_F_HIGHDMA;
 		netdev->features = netdev->hw_features |
-- 
1.7.3.1

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox