Netdev List
 help / color / mirror / Atom feed
* [PATCH RFC] r8169: minimal rtl8111e-vl support
From: Francois Romieu @ 2011-06-28 21:44 UTC (permalink / raw)
  To: Hayes Wang; +Cc: netdev

[-- Attachment #1: Type: text/plain, Size: 9909 bytes --]

Mostly bits from version 8.023.00 of Realtek's own r8168 driver. It applies
on top of davem's net-next branch + a small, uninteresting attached patch.

I have given it a short testing w/o the new-format rtl8168e-3_0.0.1 firmware
and it is fairly encouraging.

The code is still incomplete :
- WoL needs some care. No difficulty here.
- rtl8168e_2_hw_phy_config imho deserves a few comments similar to those in
  rtl8168e_1_hw_phy_config. Hayes, can you take care of it ?
- I have excluded a set of completely unidentified registers / bits
  operations, for instance:
  - Config5
      BIT_0
  - Config2
      BIT_5
      BIT_7
  - TxConfig
      BIT_7
  - 0x1a
      BIT_2
      BIT_3
  - 0x1b
      0xf8 / 0x07
  - 0xb0,
      0xee480010
  - 0xd0
      BIT_6
  - 0xd3
      BIT_7
  - 0xf2
      BIT_6
  Either they are not needed or someone will have to name them adequately.
  Hayes ?
- Short packets apparently need to be checksummed by the driver (?) but
  the work-around seems strange. It is not clear to me what Realtek's
  driver is trying to achieve in hard_start_xmit. Hayes, can you elaborate ?

Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>
---
 drivers/net/r8169.c |  254 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 254 insertions(+), 0 deletions(-)

diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
index f5b8d52..1b38a0f 100644
--- a/drivers/net/r8169.c
+++ b/drivers/net/r8169.c
@@ -41,6 +41,7 @@
 #define FIRMWARE_8168D_2	"rtl_nic/rtl8168d-2.fw"
 #define FIRMWARE_8168E_1	"rtl_nic/rtl8168e-1.fw"
 #define FIRMWARE_8168E_2	"rtl_nic/rtl8168e-2.fw"
+#define FIRMWARE_8168E_3	"rtl_nic/rtl8168e-3.fw"
 #define FIRMWARE_8105E_1	"rtl_nic/rtl8105e-1.fw"
 
 #ifdef RTL8169_DEBUG
@@ -133,6 +134,7 @@ enum mac_version {
 	RTL_GIGA_MAC_VER_31,
 	RTL_GIGA_MAC_VER_32,
 	RTL_GIGA_MAC_VER_33,
+	RTL_GIGA_MAC_VER_34,
 	RTL_GIGA_MAC_NONE   = 0xff,
 };
 
@@ -217,6 +219,8 @@ static const struct {
 		_R("RTL8168e/8111e",	RTL_TD_1, FIRMWARE_8168E_1),
 	[RTL_GIGA_MAC_VER_33] =
 		_R("RTL8168e/8111e",	RTL_TD_1, FIRMWARE_8168E_2),
+	[RTL_GIGA_MAC_VER_34] =
+		_R("RTL8168evl/8111evl",RTL_TD_1, FIRMWARE_8168E_3),
 };
 #undef _R
 
@@ -715,6 +719,76 @@ static int rtl8169_poll(struct napi_struct *napi, int budget);
 static const unsigned int rtl8169_rx_config =
 	(RX_FIFO_THRESH << RxCfgFIFOShift) | (RX_DMA_BURST << RxCfgDMAShift);
 
+static void rtl_eri_cmd(void __iomem *ioaddr, int type, int addr, u32 cmd)
+{
+	int i;
+
+	RTL_W32(ERIAR, cmd | type | ERIAR_BYTEEN | addr);
+
+	for (i = 0; i < 10; i++) {
+		udelay(100);
+
+		if ((RTL_R32(ERIAR) ^ cmd) & ERIAR_FLAG)
+			break;
+	}
+}
+
+static u32 eri_data_mask[] = { 0x000000ff, 0x0000ffff, 0x00ffffff, 0xffffffff };
+
+u32 rtl_eri_read(void __iomem *ioaddr, int addr, int len, int type)
+{
+	int done = 0;
+	u32 val = 0;
+
+	BUG_ON((len > 4) || (len <= 0));
+
+	while ((len <= 4) && (len > 0)) {
+		int offset = addr % ERIAR_ADDR_BYTE_ALIGN;
+		int avail = ERIAR_ADDR_BYTE_ALIGN - offset;
+		u32 data;
+
+		addr &= ~(ERIAR_ADDR_BYTE_ALIGN - 1);
+
+		rtl_eri_cmd(ioaddr, type, addr, ERIAR_READ_CMD);
+
+		data = (RTL_R32(ERIDR) >> (8 * offset)) & eri_data_mask[len -1];
+		val |= data << (8 * done);
+		done += min(avail, len);
+		len -= done;
+		addr += ERIAR_ADDR_BYTE_ALIGN;
+	}
+
+	return val;
+}
+
+static void rtl_eri_write(void __iomem *ioaddr, int addr, int len, u32 val,
+			  int type)
+{
+	BUG_ON((len > 4) || (len <= 0));
+
+	while ((len <= 4) && (len > 0)) {
+		int offset = addr % ERIAR_ADDR_BYTE_ALIGN;
+		int avail = ERIAR_ADDR_BYTE_ALIGN - offset;
+		u32 data;
+		int done;
+
+		addr &= ~(ERIAR_ADDR_BYTE_ALIGN - 1);
+
+		data  = rtl_eri_read(ioaddr, addr, 4, type);
+		data &= ~(eri_data_mask[len - 1] << (8 * offset));
+		data |= val << (8 * offset);
+
+		RTL_W32(ERIDR, data);
+
+		rtl_eri_cmd(ioaddr, type, addr, ERIAR_WRITE_CMD);
+
+		done = min(avail, len);
+		val >>= 8 * done;
+		len -= done;
+		addr += ERIAR_ADDR_BYTE_ALIGN;
+	}
+}
+
 static u32 ocp_read(struct rtl8169_private *tp, u8 mask, u16 reg)
 {
 	void __iomem *ioaddr = tp->mmio_addr;
@@ -1641,6 +1715,7 @@ static void rtl8169_get_mac_version(struct rtl8169_private *tp,
 		int mac_version;
 	} mac_info[] = {
 		/* 8168E family. */
+		{ 0x7cf00000, 0x2c800000,	RTL_GIGA_MAC_VER_34 },
 		{ 0x7cf00000, 0x2c200000,	RTL_GIGA_MAC_VER_33 },
 		{ 0x7cf00000, 0x2c100000,	RTL_GIGA_MAC_VER_32 },
 		{ 0x7c800000, 0x2c000000,	RTL_GIGA_MAC_VER_33 },
@@ -2691,6 +2766,109 @@ static void rtl8168e_1_hw_phy_config(struct rtl8169_private *tp)
 	rtl_writephy(tp, 0x0d, 0x0000);
 }
 
+static void rtl8168e_2_hw_phy_config(struct rtl8169_private *tp)
+{
+	static const struct phy_reg phy_reg_init_1[] = {
+		/* ? */
+		{ 0x1f, 0x0001 },
+		{ 0x0b, 0x6c14 },
+		{ 0x14, 0x7f3d },
+		{ 0x1c, 0xfafe },
+		{ 0x08, 0x07c5 },
+		{ 0x10, 0xf090 },
+		{ 0x1f, 0x0003 },
+		{ 0x14, 0x641a },
+		{ 0x1a, 0x0606 },
+		{ 0x12, 0xf480 },
+		{ 0x13, 0x0747 },
+		{ 0x1f, 0x0000 },
+
+		/* ? */
+		{ 0x1f, 0x0004 },
+		{ 0x1f, 0x0007 },
+		{ 0x1e, 0x0078 },
+		{ 0x15, 0xa408 },
+		{ 0x17, 0x5100 },
+		{ 0x19, 0x0008 },
+		{ 0x1f, 0x0002 },
+		{ 0x1f, 0x0000 },
+
+		/* ? */
+		{ 0x1f, 0x0003 },
+		{ 0x0d, 0x0207 },
+		{ 0x02, 0x5fd0 },
+		{ 0x1f, 0x0000 }
+	}, phy_reg_init_2[] = {
+		/* ? */
+		{ 0x1f, 0x0004 },
+		{ 0x1f, 0x0007 },
+		{ 0x1e, 0x00ac },
+		{ 0x18, 0x0006 },
+		{ 0x1f, 0x0002 },
+		{ 0x1f, 0x0000 },
+
+		/* ? */
+		{ 0x1f, 0x0003 },
+		{ 0x09, 0xa20f },
+		{ 0x1f, 0x0000 },
+
+		/* ? */
+		{ 0x1f, 0x0005 },
+		{ 0x05, 0x8b5b },
+		{ 0x06, 0x9222 },
+		{ 0x05, 0x8b6d },
+		{ 0x06, 0x8000 },
+		{ 0x05, 0x8b76 },
+		{ 0x06, 0x8000 },
+		{ 0x1f, 0x0000 }
+	};
+
+	rtl_apply_firmware(tp);
+
+	/* ? */
+	rtl_writephy(tp, 0x1f, 0x0005);
+	rtl_writephy(tp, 0x05, 0x8b80);
+	rtl_w1w0_phy(tp, 0x06, 0x0006, 0x0000);
+	rtl_writephy(tp, 0x1f, 0x0000);
+
+	/* ? */
+	rtl_writephy(tp, 0x1f, 0x0004);
+	rtl_writephy(tp, 0x1f, 0x0007);
+	rtl_writephy(tp, 0x1e, 0x002d);
+	rtl_w1w0_phy(tp, 0x18, 0x0010, 0x0000);
+	rtl_writephy(tp, 0x1f, 0x0002);
+	rtl_writephy(tp, 0x1f, 0x0000);
+	rtl_w1w0_phy(tp, 0x14, 0x8000, 0x0000);
+
+	rtl_writephy(tp, 0x1f, 0x0000);
+	rtl_writephy(tp, 0x15, 0x1006);
+
+	rtl_writephy(tp, 0x1f, 0x0005);
+	rtl_writephy(tp, 0x05, 0x8b86);
+	rtl_w1w0_phy(tp, 0x06, 0x0001, 0x0000);
+	rtl_writephy(tp, 0x1f, 0x0000);
+
+	rtl_writephy_batch(tp, phy_reg_init_1, ARRAY_SIZE(phy_reg_init_1));
+
+	/* ? */
+	rtl_writephy(tp, 0x1f, 0x0004);
+	rtl_writephy(tp, 0x1f, 0x0007);
+	rtl_writephy(tp, 0x1e, 0x00a1);
+	rtl_w1w0_phy(tp, 0x1a, 0x0000, 0x0004);
+	rtl_writephy(tp, 0x1f, 0x0002);
+	rtl_writephy(tp, 0x1f, 0x0000);
+
+	/* ? */
+	rtl_writephy(tp, 0x1f, 0x0004);
+	rtl_writephy(tp, 0x1f, 0x0007);
+	rtl_writephy(tp, 0x1e, 0x002d);
+	rtl_w1w0_phy(tp, 0x16, 0x0020, 0x0000);
+	rtl_writephy(tp, 0x1f, 0x0002);
+	rtl_writephy(tp, 0x1f, 0x0000);
+
+	rtl_writephy_batch(tp, phy_reg_init_2, ARRAY_SIZE(phy_reg_init_2));
+}
+
 static void rtl8102e_hw_phy_config(struct rtl8169_private *tp)
 {
 	static const struct phy_reg phy_reg_init[] = {
@@ -2812,6 +2990,9 @@ static void rtl_hw_phy_config(struct net_device *dev)
 	case RTL_GIGA_MAC_VER_33:
 		rtl8168e_1_hw_phy_config(tp);
 		break;
+	case RTL_GIGA_MAC_VER_34:
+		rtl8168e_2_hw_phy_config(tp);
+		break;
 
 	default:
 		break;
@@ -3170,6 +3351,7 @@ static void r8168_phy_power_down(struct rtl8169_private *tp)
 	switch (tp->mac_version) {
 	case RTL_GIGA_MAC_VER_32:
 	case RTL_GIGA_MAC_VER_33:
+	case RTL_GIGA_MAC_VER_34:
 		rtl_writephy(tp, MII_BMCR, BMCR_ANENABLE | BMCR_PDOWN);
 		break;
 
@@ -3926,6 +4108,20 @@ static void rtl_csi_access_enable_2(void __iomem *ioaddr)
 	rtl_csi_access_enable(ioaddr, 0x27000000);
 }
 
+struct ephy_reg {
+	u16 offset;
+	u16 val;
+};
+
+static void rtl_write_ephy_batch(void __iomem *ioaddr,
+				 const struct ephy_reg *regs, int len)
+{
+	while (len-- > 0) {
+		rtl_ephy_write(ioaddr, regs->offset, regs->val);
+		regs++;
+	}
+}
+
 struct ephy_info {
 	unsigned int offset;
 	u16 mask;
@@ -4184,6 +4380,60 @@ static void rtl_hw_start_8168e_1(void __iomem *ioaddr, struct pci_dev *pdev)
 	RTL_W8(Config5, RTL_R8(Config5) & ~Spi_en);
 }
 
+struct eri_reg {
+	u16 addr;
+	u16 len;
+	u32 data;
+};
+
+static void rtl_write_eri_batch(void __iomem * ioaddr,
+				const struct eri_reg *regs, int len, int type)
+{
+	while (len-- > 0) {
+		rtl_eri_write(ioaddr, regs->addr, regs->len, regs->data, type);
+		regs++;
+	}
+}
+
+static void rtl_hw_start_8168e_2(void __iomem *ioaddr, struct pci_dev *pdev)
+{
+	static const struct ephy_reg ephy_reg_init[] = {
+		{ 0x06, 0xf020 },
+		{ 0x07, 0x01ff },
+		{ 0x00, 0x5027 },
+		{ 0x01, 0x0003 },
+		{ 0x02, 0x2d16 },
+		{ 0x03, 0x6d49 },
+		{ 0x08, 0x0006 },
+		{ 0x0a, 0x00c8 },
+	};
+	static const struct ephy_info e_info_8168e[] = {
+		{ 0x09, 0x0000, 0x0080 },
+		{ 0x19, 0x0000, 0x0224 }
+	};
+	static const struct eri_reg eri_reg_init[] = {
+		{ 0xd5, 1, 0x0000000c },
+		{ 0xc0, 2, 0x00000000 },
+		{ 0xb8, 2, 0x00000000 },
+		{ 0xc8, 4, 0x00100002 },
+		{ 0xe8, 4, 0x00100006 }
+	};
+
+	rtl_csi_access_enable_1(ioaddr);
+	rtl_tx_performance_tweak(pdev, 0x5 << MAX_READ_REQUEST_SHIFT);
+	RTL_W8(MaxTxPacketSize, TxPacketMax);
+
+	rtl_write_eri_batch(ioaddr, eri_reg_init, ARRAY_SIZE(eri_reg_init),
+			    ERIAR_EXGMAC);
+
+	rtl_eri_write(ioaddr, 0x01dc, 1, 0x64, ERIAR_EXGMAC);
+
+	rtl_write_ephy_batch(ioaddr, ephy_reg_init, ARRAY_SIZE(ephy_reg_init));
+	rtl_ephy_init(ioaddr, e_info_8168e, ARRAY_SIZE(e_info_8168e));
+
+	rtl_disable_clock_request(pdev);
+}
+
 static void rtl_hw_start_8168(struct net_device *dev)
 {
 	struct rtl8169_private *tp = netdev_priv(dev);
@@ -4275,6 +4525,10 @@ static void rtl_hw_start_8168(struct net_device *dev)
 		rtl_hw_start_8168e_1(ioaddr, pdev);
 		break;
 
+	case RTL_GIGA_MAC_VER_34:
+		rtl_hw_start_8168e_2(ioaddr, pdev);
+		break;
+
 	default:
 		printk(KERN_ERR PFX "%s: unknown chipset (mac_version = %d).\n",
 			dev->name, tp->mac_version);
-- 
1.7.4.4


[-- Attachment #2: 0001-r8169-noise-redux.patch --]
[-- Type: text/plain, Size: 3375 bytes --]

>From d777443c097b65b06f1d0e7da37db1368185db8e Mon Sep 17 00:00:00 2001
From: Francois Romieu <romieu@fr.zoreil.com>
Date: Tue, 28 Jun 2011 17:01:29 +0200
Subject: [PATCH 1/3] r8169: noise redux.

- insert an empty record in rtl_chip_infos for future chipsets.
  NB: the array is only accessed through the RTL_GIGA_MAC_VER_xy enum.
- ready-to-use ERIAR bits definitions. Those were not used.
- ERIDR / ERIAR confusion. This one is already in Linus's tree. I'll
  remove it before the final submission.
- make room for different 8168e hw_phy_config and hw_start methods.

Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>
---
 drivers/net/r8169.c |   23 ++++++++++++-----------
 1 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
index fbd6838..f5b8d52 100644
--- a/drivers/net/r8169.c
+++ b/drivers/net/r8169.c
@@ -216,7 +216,7 @@ static const struct {
 	[RTL_GIGA_MAC_VER_32] =
 		_R("RTL8168e/8111e",	RTL_TD_1, FIRMWARE_8168E_1),
 	[RTL_GIGA_MAC_VER_33] =
-		_R("RTL8168e/8111e",	RTL_TD_1, FIRMWARE_8168E_2)
+		_R("RTL8168e/8111e",	RTL_TD_1, FIRMWARE_8168E_2),
 };
 #undef _R
 
@@ -351,12 +351,12 @@ enum rtl8168_registers {
 #define ERIAR_WRITE_CMD			0x80000000
 #define ERIAR_READ_CMD			0x00000000
 #define ERIAR_ADDR_BYTE_ALIGN		4
-#define ERIAR_EXGMAC			0
-#define ERIAR_MSIX			1
-#define ERIAR_ASF			2
 #define ERIAR_TYPE_SHIFT		16
-#define ERIAR_BYTEEN			0x0f
+#define ERIAR_EXGMAC			(0x00 << ERIAR_TYPE_SHIFT)
+#define ERIAR_MSIX			(0x01 << ERIAR_TYPE_SHIFT)
+#define ERIAR_ASF			(0x02 << ERIAR_TYPE_SHIFT)
 #define ERIAR_BYTEEN_SHIFT		12
+#define ERIAR_BYTEEN			(0x0f << ERIAR_BYTEEN_SHIFT)
 	EPHY_RXER_NUM		= 0x7c,
 	OCPDR			= 0xb0,	/* OCP GPHY access */
 #define OCPDR_WRITE_CMD			0x80000000
@@ -749,11 +749,12 @@ static void rtl8168_oob_notify(struct rtl8169_private *tp, u8 cmd)
 	int i;
 
 	RTL_W8(ERIDR, cmd);
-	RTL_W32(ERIAR, 0x800010e8);
+	/* Could 0x10e8 be replaced with (ERIAR_BYTEEN | 0x00e8) ? */
+	RTL_W32(ERIAR, ERIAR_WRITE_CMD | ERIAR_EXGMAC | 0x10e8);
 	msleep(2);
 	for (i = 0; i < 5; i++) {
 		udelay(100);
-		if (!(RTL_R32(ERIDR) & ERIAR_FLAG))
+		if (!(RTL_R32(ERIAR) & ERIAR_FLAG))
 			break;
 	}
 
@@ -2617,7 +2618,7 @@ static void rtl8168d_4_hw_phy_config(struct rtl8169_private *tp)
 	rtl_patchphy(tp, 0x0d, 1 << 5);
 }
 
-static void rtl8168e_hw_phy_config(struct rtl8169_private *tp)
+static void rtl8168e_1_hw_phy_config(struct rtl8169_private *tp)
 {
 	static const struct phy_reg phy_reg_init[] = {
 		/* Enable Delay cap */
@@ -2809,7 +2810,7 @@ static void rtl_hw_phy_config(struct net_device *dev)
 		break;
 	case RTL_GIGA_MAC_VER_32:
 	case RTL_GIGA_MAC_VER_33:
-		rtl8168e_hw_phy_config(tp);
+		rtl8168e_1_hw_phy_config(tp);
 		break;
 
 	default:
@@ -4148,7 +4149,7 @@ static void rtl_hw_start_8168d_4(void __iomem *ioaddr, struct pci_dev *pdev)
 	rtl_enable_clock_request(pdev);
 }
 
-static void rtl_hw_start_8168e(void __iomem *ioaddr, struct pci_dev *pdev)
+static void rtl_hw_start_8168e_1(void __iomem *ioaddr, struct pci_dev *pdev)
 {
 	static const struct ephy_info e_info_8168e[] = {
 		{ 0x00, 0x0200,	0x0100 },
@@ -4271,7 +4272,7 @@ static void rtl_hw_start_8168(struct net_device *dev)
 
 	case RTL_GIGA_MAC_VER_32:
 	case RTL_GIGA_MAC_VER_33:
-		rtl_hw_start_8168e(ioaddr, pdev);
+		rtl_hw_start_8168e_1(ioaddr, pdev);
 		break;
 
 	default:
-- 
1.7.4.4


^ permalink raw reply related

* [PATCH 1/2] bridge: ignore pause & bonding frames
From: David Lamparter @ 2011-06-28 22:03 UTC (permalink / raw)
  To: netdev; +Cc: Nick Carter, David Lamparter, Stephen Hemminger, davem
In-Reply-To: <20110628214637.GE2121496@jupiter.n2.diac24.net>

this patch adds bonding frames to the special treatment party and has
both pause and bonding frames delivered on the underlying bridge member
device. we thus become 802.1AX section 5.2.1 compliant which quite
clearly has the link aggregation directly on top of the MAC layer, below
any bridging.

the matching is changed to mimic hardware switches, which match
802.3x/pause and 802.3ad/lacp by the hardware address (keep in mind the
existence of LLC/SNAP headers).

Signed-off-by: David Lamparter <equinox@diac24.net>
---
note: this should actually fix some issues i was having with bridging
screwing up my bonding. i've resolved those by "brouting" LACP frames
in ebtables... (which this patch will also result in)

compile-tested only

 net/bridge/br_input.c |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
index f3ac1e8..c873db5 100644
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -160,9 +160,12 @@ rx_handler_result_t br_handle_frame(struct sk_buff **pskb)
 	p = br_port_get_rcu(skb->dev);
 
 	if (unlikely(is_link_local(dest))) {
-		/* Pause frames shouldn't be passed up by driver anyway */
-		if (skb->protocol == htons(ETH_P_PAUSE))
-			goto drop;
+		/* Pause/3x frames shouldn't be passed up by driver anyway
+		 * LACP/3ad can never be allowed to cross even a dumb hub
+		 *
+		 * both are usually matched by group address */
+		if (dest[5] == 0x01 || dest[5] == 0x02)
+			return RX_HANDLER_PASS;
 
 		/* If STP is turned off, then forward */
 		if (p->br->stp_enabled == BR_NO_STP && dest[5] == 0)
-- 
1.7.5.3


^ permalink raw reply related

* [PATCH 2/2] bridge: pass through 802.1X & co. in 'dumb' mode
From: David Lamparter @ 2011-06-28 22:03 UTC (permalink / raw)
  To: netdev; +Cc: Nick Carter, David Lamparter, Stephen Hemminger, davem
In-Reply-To: <1309298599-11266-1-git-send-email-equinox@diac24.net>

when operating without STP, we're a dumb switch and should be able to
forward ethernet management protocols like 802.1X, LLDP and GVRP.

if this is not desired, it can be enacted as local policy through
ebtables.

if we're in STP mode we basically claim to be an intelligent switch and
should implement these protocols properly (in userspace).

Signed-off-by: David Lamparter <equinox@diac24.net>
---
compile-tested only

 net/bridge/br_input.c |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
index c873db5..4cee1b5 100644
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -167,16 +167,19 @@ rx_handler_result_t br_handle_frame(struct sk_buff **pskb)
 		if (dest[5] == 0x01 || dest[5] == 0x02)
 			return RX_HANDLER_PASS;
 
-		/* If STP is turned off, then forward */
-		if (p->br->stp_enabled == BR_NO_STP && dest[5] == 0)
+		/* If STP is turned off, we're a dumb switch and therefore
+		 * forward the remaining link-locals. (STP, 802.1X, LLDP,
+		 * GVRP & co.) */
+		if (p->br->stp_enabled == BR_NO_STP)
 			goto forward;
 
 		if (NF_HOOK(NFPROTO_BRIDGE, NF_BR_LOCAL_IN, skb, skb->dev,
 			    NULL, br_handle_local_finish)) {
 			return RX_HANDLER_CONSUMED; /* consumed by filter */
 		} else {
+			/* stay on physdev for userspace implementation */
 			*pskb = skb;
-			return RX_HANDLER_PASS;	/* continue processing */
+			return RX_HANDLER_PASS;
 		}
 	}
 
-- 
1.7.5.3


^ permalink raw reply related

* [PATCH v2] bridge: ignore pause & bonding frames
From: David Lamparter @ 2011-06-28 22:10 UTC (permalink / raw)
  To: netdev; +Cc: Nick Carter, David Lamparter, Stephen Hemminger, davem
In-Reply-To: <1309298599-11266-1-git-send-email-equinox@diac24.net>

this patch adds bonding frames to the special treatment party and has
both pause and bonding frames delivered on the underlying bridge member
device. we thus become 802.1AX section 5.2.1 compliant which quite
clearly has the link aggregation directly on top of the MAC layer, below
any bridging.

the matching is changed to mimic hardware switches, which match
802.3x/pause and 802.3ad/lacp by the hardware address (keep in mind the
existence of LLC/SNAP headers).

Signed-off-by: David Lamparter <equinox@diac24.net>
---
[v2:] ... and obviously i forget the "*pskb = skb;" - it's getting late.

note: this should actually fix some issues i was having with bridging
screwing up my bonding. i've resolved those by "brouting" LACP frames
in ebtables... (which this patch will also result in)

compile-tested only

 net/bridge/br_input.c |   11 ++++++++---
 1 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
index f3ac1e8..3ef2861 100644
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -160,9 +160,14 @@ rx_handler_result_t br_handle_frame(struct sk_buff **pskb)
 	p = br_port_get_rcu(skb->dev);
 
 	if (unlikely(is_link_local(dest))) {
-		/* Pause frames shouldn't be passed up by driver anyway */
-		if (skb->protocol == htons(ETH_P_PAUSE))
-			goto drop;
+		/* Pause/3x frames shouldn't be passed up by driver anyway
+		 * LACP/3ad can never be allowed to cross even a dumb hub
+		 *
+		 * both are usually matched by group address */
+		if (dest[5] == 0x01 || dest[5] == 0x02) {
+			*pskb = skb;
+			return RX_HANDLER_PASS;
+		}
 
 		/* If STP is turned off, then forward */
 		if (p->br->stp_enabled == BR_NO_STP && dest[5] == 0)
-- 
1.7.5.3


^ permalink raw reply related

* RE: [PATCH] [v3][net][bna] Fix call trace when interrupts are disabled while sleeping function kzalloc is called
From: Rasesh Mody @ 2011-06-28 23:32 UTC (permalink / raw)
  To: Shyam Iyer, netdev@vger.kernel.org
  Cc: Debashis Dutt, Jing Huang, davem@davemloft.net, Shyam Iyer
In-Reply-To: <1309287485-6077-1-git-send-email-shyam_iyer@dell.com>

>From: Shyam Iyer [mailto:shyam.iyer.t@gmail.com]
>Sent: Tuesday, June 28, 2011 11:58 AM
>
>request_threaded irq will call kzalloc that can sleep. Initializing the
>flags variable outside of spin_lock_irqsave/restore in
>bnad_mbox_irq_alloc will avoid call traces like below.
>
>@@ -1125,18 +1125,17 @@ bnad_mbox_irq_alloc(struct bnad *bnad,
> 	if (bnad->cfg_flags & BNAD_CF_MSIX) {
> 		irq_handler = (irq_handler_t)bnad_msix_mbox_handler;
> 		irq = bnad->msix_table[bnad->msix_num - 1].vector;
>-		flags = 0;
> 		intr_info->intr_type = BNA_INTR_T_MSIX;
> 		intr_info->idl[0].vector = bnad->msix_num - 1;
> 	} else {
> 		irq_handler = (irq_handler_t)bnad_isr;
> 		irq = bnad->pcidev->irq;
>-		flags = IRQF_SHARED;
>+		irq_flags = IRQF_SHARED;
> 		intr_info->intr_type = BNA_INTR_T_INTX;
> 		/* intr_info->idl.vector = 0 ? */
> 	}
> 	spin_unlock_irqrestore(&bnad->bna_lock, flags);
>-
>+	flags = irq_flags;
> 	sprintf(bnad->mbox_irq_name, "%s", BNAD_NAME);
>
> 	/*

Patch looks fine.

Acked-by: Rasesh Mody <rmody@brocade.com>

^ permalink raw reply

* Re: [PATCH V7 2/4 net-next] skbuff: Add userspace zero-copy buffers in skb
From: David Miller @ 2011-06-28 23:35 UTC (permalink / raw)
  To: mashirle; +Cc: mst, eric.dumazet, avi, arnd, netdev, kvm, linux-kernel
In-Reply-To: <1309279892.3559.6.camel@localhost.localdomain>

From: Shirley Ma <mashirle@us.ibm.com>
Date: Tue, 28 Jun 2011 09:51:32 -0700

> On Mon, 2011-06-27 at 15:54 -0700, David Miller wrote:
>> From: Shirley Ma <mashirle@us.ibm.com>
>> Date: Mon, 27 Jun 2011 08:45:10 -0700
>> 
>> > To support skb zero-copy, a pointer is needed to add to skb share
>> info.
>> > Do you agree with this approach? If not, do you have any other
>> > suggestions?
>> 
>> I really can't form an opinion unless I am shown the complete
>> implementation, what this give us in return, what the impact is, etc. 
 ..
> You can see the overall CPU saved 50% w/i zero-copy.
> 
> The impact is every skb allocation consumed one more pointer in skb
> share info, and a pointer check in skb release when last reference is
> gone.
> 
> For skb clone, skb expand private head and skb copy, it still keeps copy
> the buffers to kernel, so we can avoid user application, like tcpdump to
> hold the user-space buffers too long.

Ok, now show me the "complete implementation".  I'm as interested in
the code as I am in the numbers, that's why I asked for both.

^ permalink raw reply

* Re: Bug#631945: linux-image-2.6.39-2-amd64: HFSC puts lots of "WARNING: at .../sch_hfsc.c:1427 hfsc_dequeue+0x155/0x28b [sch_hfsc]()" in dmesg
From: Ben Hutchings @ 2011-06-29  4:08 UTC (permalink / raw)
  To: Patrick McHardy, Michal Soltys; +Cc: 631945, Michal Pokrywka, netdev
In-Reply-To: <20110628131310.9941.39557.reportbug@wenus.h1.pl>

[-- Attachment #1: Type: text/plain, Size: 10697 bytes --]

On Tue, 2011-06-28 at 15:13 +0200, Michal Pokrywka wrote:
> Package: linux-2.6
> Version: 2.6.39-2
> Severity: important
> Tags: upstream
> 
> 
> I had working HFSC configuration on gateway machine with kernel
> 2.6.26-21~bpo40+1. Now I switched to new machine and kernel 2.6.39-2 with the
> same configuration script. Shaping works but after a couple of minutes
> houndreds of warnings per second fills dmesg severly slowing whole system
> (kernel log attached below).

This is the warning in hfsc_schedule_watchdog().  Patrick, Michal, it
looks like you made some changes in this area since 2.6.32 (the last
version reported as OK).  Can you see how this can happen?

Ben.

> My configuration on eth0 involves SFQ queues connected to HFSC leafs, and one
> RED queue connected to one HFSC leaf. Similar configuration runs on ifb0.
> All incoming to eth0 traffic is directed to ifb0 using egress action mirred.
> Also there is some hashtable filtering based in IP addresses.
> 
> When I erased all queues and put for testing purposes only one HFSC queue on
> eth0 with one default class I noticed that the problem exists no more.
> So I connected one SFQ queue to this single HFSC class and problem still
> not occurs.
> 
> Then I did the same thing on ifb0 and redirected traffic from eth0 egress
> and noticed still no warnings in dmesg.
> 
> Connecting RED to HFSC leaf also produces no warnings.
> 
> Switching in udev eth0->eth1 and eth1->eth0 (to make HFSC work with different
> ethernet driver: forcedeth->tg3) does not solve the problem.
> 
> Maybe someone can suggest what else could be done to trace the problem.
> 
> Here are the HFSC script that triggers the bug (without repeating stuff).
> It is a bit complicated but it works with different users on three different
> machines (different ethernet devices) with 2.6.32 kernel.
> It should work on every machine which has 192.168.20.0/24 network on eth0
> and internet access on eth1.
> 
> -- CUT HERE (hfsc.sh begins) --
> modprobe ifb numifbs=1
> ### DOWNLOAD ###
> tc qdisc del dev eth0 root 2>&1 >/dev/null
> tc qdisc add dev eth0 root handle 1: hfsc default 4
> tc class add dev eth0 parent 1:  classid 1:1 hfsc ls rate 100000kbit ul rate 100000kbit # main eth0 download
> 
> tc class add dev eth0 parent 1:1 classid 1:2 hfsc ls rate 3891kbit ul rate 3891kbit # main internet download
> tc class add dev eth0 parent 1:2 classid 1:4 hfsc ls rate 50kbit ul rate 100kbit # bulk class
> tc class add dev eth0 parent 1:2 classid 1:5 hfsc ls rate 200kbit ul rate 1000kbit # wireless class
> tc qdisc add dev eth0 parent 1:4 handle 4: sfq perturb 5 quantum 1920 # bulk qdisc
> tc qdisc add dev eth0 parent 1:5 handle 5: sfq perturb 5 quantum 1920 # wifi qdisc
> 
> tc class add dev eth0 parent 1:1 classid 1:3 hfsc ls rate 94904kbit ul rate 94904kbit # local class
> tc qdisc add dev eth0 parent 1:3 handle 3: red min 75000 max 225000 probability 0.02 limit 675000 avpkt 1000 burst 120 bandwidth 94904
> 
> tc filter add dev eth0 parent 1: protocol ip prio 3 u32 match ip src 192.168.20.0/24 match ip dst 62.***.***.88/29 flowid 1:3
> tc filter add dev eth0 parent 1: protocol ip prio 3 u32 match ip src 192.168.20.0/24 match ip dst 62.***.***.96/28 flowid 1:3
> tc filter add dev eth0 parent 1: protocol ip prio 3 u32 match ip dst 192.168.20.0/24 match ip src 62.***.***.88/29 flowid 1:3
> tc filter add dev eth0 parent 1: protocol ip prio 3 u32 match ip dst 192.168.20.0/24 match ip src 62.***.***.96/28 flowid 1:3
> tc filter add dev eth0 parent 1: protocol ip prio 3 u32 match ip src 192.168.20.91 match ip dst 192.168.20.0/24 flowid 1:3
> 
> tc filter add dev eth0 parent 1: protocol ip prio 3 u32 match ip dst 10.0.0.0/8 flowid 1:5
> 
> tc filter add dev eth0 parent 1: prio 5 handle 100: protocol ip u32 divisor 256
> tc filter add dev eth0 protocol ip parent 1: prio 5 u32 ht 800:: match ip dst 192.168.20.0/24 hashkey mask 0x000000ff at 16 link 100:
> 
> # example_user / 192.168.20.16
> tc class add dev eth0 parent 1:2    classid 1:0016  hfsc ls m1 400kbit d 4s m2 100kbit ul rate 3696kbit
> tc class add dev eth0 parent 1:0016 classid 1:1016  hfsc rt rate 336bit ls m1 400kbit d 4s m2 100kbit ul rate 3696kbit # ack
> tc qdisc add dev eth0 parent 1:1016 handle    1016: sfq perturb 10 quantum 1920
> tc class add dev eth0 parent 1:0016 classid 1:2016  hfsc rt rate 94566bit ls m1 400kbit d 4s m2 100kbit ul rate 3696kbit # data
> tc qdisc add dev eth0 parent 1:2016 handle    2016: sfq perturb 10 quantum 1920
> 
> tc filter add dev eth0 parent 1: protocol ip prio  5 u32 ht 100:10 match ip protocol 6 0xff match u8 0x10 0xff at nexthdr+33 match u16 0x0000 0xffc0 at 2 flowid 1:1016 # ack
> tc filter add dev eth0 parent 1: protocol ip prio 10 u32 ht 100:10 match u32 0 0 flowid 1:2016 # data
> 
> (... more users ...)
> 
> ### UPLOAD ###
> ifconfig ifb0 up
> 
> tc qdisc del dev ifb0 root 2>&1 >/dev/null
> tc qdisc add dev ifb0 root handle 1: hfsc default 4
> tc class add dev ifb0 parent 1:  classid 1:1 hfsc ls rate 100000kbit ul rate 100000kbit # main eth0 upload
> tc class add dev ifb0 parent 1:1 classid 1:2 hfsc ls rate 460kbit ul rate 460kbit # main internet upload
> 
> tc class add dev ifb0 parent 1:2 classid 1:4 hfsc ls rate 50kbit ul rate 10kbit # bulk class
> tc class add dev ifb0 parent 1:2 classid 1:5 hfsc ls rate 200kbit ul rate 100kbit # wireless class
> tc qdisc add dev ifb0 parent 1:4 handle 4: sfq perturb 5 quantum 1920 # bulk qdisc
> tc qdisc add dev ifb0 parent 1:5 handle 5: sfq perturb 5 quantum 1920 # wifi qdisc
> 
> tc class add dev ifb0 parent 1:1 classid 1:3 hfsc ls rate 94904kbit ul rate 94904kbit # local class
> tc qdisc add dev ifb0 parent 1:3 handle 3: red min 75000 max 225000 probability 0.02 limit 675000 avpkt 1000 burst 120 bandwidth 94904
> 
> tc filter add dev ifb0 parent 1: protocol ip prio 2 u32 match ip src 192.168.20.0/24 match ip dst 62.***.***.88/29 flowid 1:3
> tc filter add dev ifb0 parent 1: protocol ip prio 2 u32 match ip src 192.168.20.0/24 match ip dst 62.***.***.96/28 flowid 1:3
> tc filter add dev ifb0 parent 1: protocol ip prio 2 u32 match ip dst 192.168.20.0/24 match ip src 62.***.***.88/29 flowid 1:3
> tc filter add dev ifb0 parent 1: protocol ip prio 2 u32 match ip dst 192.168.20.0/24 match ip src 62.***.***.96/28 flowid 1:3
> tc filter add dev ifb0 parent 1: protocol ip prio 2 u32 match ip dst 192.168.20.91 match ip src 192.168.20.0/24 flowid 1:3
> tc filter add dev ifb0 parent 1: protocol ip prio 2 u32 match ip src 192.168.20.91 match ip dst 192.168.20.0/24 flowid 1:3
> 
> tc filter add dev ifb0 parent 1: protocol ip prio 3 u32 match ip src 10.0.0.0/8 flowid 1:5
> tc filter add dev ifb0 parent 1: prio 5 handle 100: protocol ip u32 divisor 256
> 
> # example_user / 192.168.20.16
> tc class add dev ifb0 parent 1:2    classid 1:0016  hfsc ls m1 400kbit d 4s m2 100kbit ul rate 437kbit
> tc class add dev ifb0 parent 1:0016 classid 1:1016  hfsc rt rate 2847bit ls m1 400kbit d 4s m2 100kbit ul rate 437kbit # ack
> tc qdisc add dev ifb0 parent 1:1016 handle    1016: sfq perturb 10 quantum 1920
> tc class add dev ifb0 parent 1:0016 classid 1:2016  hfsc rt rate 8372bit ls m1 400kbit d 4s m2 100kbit ul rate 437kbit # data
> tc qdisc add dev ifb0 parent 1:2016 handle    2016: sfq perturb 10 quantum 1920
> 
> tc filter add dev ifb0 parent 1: protocol ip prio  5 u32 ht 100:10 match ip protocol 6 0xff match u8 0x10 0xff at nexthdr+33 match u16 0x0000 0xffc0 at 2 flowid 1:1016 # ack
> tc filter add dev ifb0 parent 1: protocol ip prio 10 u32 ht 100:10 match u32 0 0 flowid 1:2016 # data
> 
> (... more users ...)
> 
> tc qdisc del dev eth0 ingress 2>&1 >/dev/null
> tc qdisc add dev eth0 ingress
> 
> tc filter add dev eth0 parent ffff: protocol ip prio 40 u32 match ip src 192.168.20.0/24 flowid 1: action mirred egress redirect dev ifb0
> tc filter add dev eth0 parent ffff: protocol ip prio 40 u32 match ip src 10.0.0.0/8  flowid 1: action mirred egress redirect dev ifb0
> 
> # Local to hashtable
> tc filter add dev ifb0 protocol ip parent 1: prio 40 u32 ht 800:: match ip src 192.168.20.0/24 hashkey mask 0x000000ff at 12 link 100:
> 
> -- CUT HERE (hfsc.sh ends) --
> 
> -- Package-specific info:
> ** Version:
> Linux version 2.6.39-2-amd64 (Debian 2.6.39-2) (ben@decadent.org.uk) (gcc version 4.4.6 (Debian 4.4.6-3) ) #1 SMP Wed Jun 8 11:01:04 UTC 2011
> 
> ** Command line:
> BOOT_IMAGE=/boot/vmlinuz-2.6.39-2-amd64 root=UUID=b9cc0f29-98ad-4034-ba84-167769b10cd6 ro quiet
> 
> ** Tainted: W (512)
>  * Taint on warning.
> 
> ** Kernel log:
> [ 4522.772401] ------------[ cut here ]------------
> [ 4522.772405] WARNING: at /build/buildd-linux-2.6_2.6.39-2-amd64-kuqdRa/linux-2.6-2.6.39/debian/build/source_amd64_none/net/sched/sch_hfsc.c:1427 hfsc_dequeue+0x155/0x28b [sch_hfsc]()
> [ 4522.772410] Hardware name: S2865
> [ 4522.772412] Modules linked in: act_mirred sch_ingress cls_u32 sch_red sch_sfq sch_hfsc ifb xt_tcpudp xt_state iptable_filter iptable_nat ip_tables x_tables nf_nat_ftp nf_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_conntrack_ftp nf_conntrack xfs dme1737 hwmon_vid loop snd_pcm snd_timer amd64_edac_mod snd soundcore snd_page_alloc k8temp edac_core serio_raw edac_mce_amd pcspkr evdev joydev parport_pc parport i2c_nforce2 processor button i2c_core ext4 mbcache jbd2 crc16 raid1 md_mod sg usbhid hid sr_mod sd_mod crc_t10dif cdrom ohci_hcd ata_generic pata_amd sata_nv libata ehci_hcd tg3 scsi_mod thermal usbcore floppy fan thermal_sys libphy forcedeth [last unloaded: scsi_wait_scan]
> [ 4522.772453] Pid: 3, comm: ksoftirqd/0 Tainted: G        W    2.6.39-2-amd64 #1
> [ 4522.772455] Call Trace:
> [ 4522.772459]  [<ffffffff810458b4>] ? warn_slowpath_common+0x78/0x8c
> [ 4522.772463]  [<ffffffffa039ab3f>] ? hfsc_dequeue+0x155/0x28b [sch_hfsc]
> [ 4522.772467]  [<ffffffff81291962>] ? __qdisc_run+0x93/0x11a
> [ 4522.772471]  [<ffffffff8127696f>] ? net_tx_action+0x10f/0x186
> [ 4522.772475]  [<ffffffff8104b4cf>] ? __do_softirq+0xc3/0x19e
> [ 4522.772479]  [<ffffffff8104b626>] ? run_ksoftirqd+0x7c/0x122
> [ 4522.772483]  [<ffffffff8104b5aa>] ? __do_softirq+0x19e/0x19e
> [ 4522.772486]  [<ffffffff8104b5aa>] ? __do_softirq+0x19e/0x19e
> [ 4522.772490]  [<ffffffff8105ef05>] ? kthread+0x7a/0x82
> [ 4522.772494]  [<ffffffff81339ee4>] ? kernel_thread_helper+0x4/0x10
> [ 4522.772498]  [<ffffffff8105ee8b>] ? kthread_worker_fn+0x147/0x147
> [ 4522.772502]  [<ffffffff81339ee0>] ? gs_change+0x13/0x13
> [ 4522.772505] ---[ end trace 9a0810cc4dbd421e ]---
[...]

-- 
Ben Hutchings
If at first you don't succeed, you're doing about average.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

^ permalink raw reply

* Re: [PATCH V7 2/4 net-next] skbuff: Add userspace zero-copy buffers in skb
From: Shirley Ma @ 2011-06-29  4:28 UTC (permalink / raw)
  To: David Miller; +Cc: mst, eric.dumazet, avi, arnd, netdev, kvm, linux-kernel
In-Reply-To: <20110628.163508.222352070705159851.davem@davemloft.net>

I have submitted this patchset from Version 1 to Version 7 already in
the past few months.
Here is the link to the patchset:

http://lists.openwall.net/netdev/2011/05/28/

I am working on V8 now.

Thanks
Shirley


On Tue, 2011-06-28 at 16:35 -0700, David Miller wrote:
> From: Shirley Ma <mashirle@us.ibm.com>
> Date: Tue, 28 Jun 2011 09:51:32 -0700
> 
> > On Mon, 2011-06-27 at 15:54 -0700, David Miller wrote:
> >> From: Shirley Ma <mashirle@us.ibm.com>
> >> Date: Mon, 27 Jun 2011 08:45:10 -0700
> >> 
> >> > To support skb zero-copy, a pointer is needed to add to skb share
> >> info.
> >> > Do you agree with this approach? If not, do you have any other
> >> > suggestions?
> >> 
> >> I really can't form an opinion unless I am shown the complete
> >> implementation, what this give us in return, what the impact is,
> etc. 
>  ..
> > You can see the overall CPU saved 50% w/i zero-copy.
> > 
> > The impact is every skb allocation consumed one more pointer in skb
> > share info, and a pointer check in skb release when last reference
> is
> > gone.
> > 
> > For skb clone, skb expand private head and skb copy, it still keeps
> copy
> > the buffers to kernel, so we can avoid user application, like
> tcpdump to
> > hold the user-space buffers too long.
> 
> Ok, now show me the "complete implementation".  I'm as interested in
> the code as I am in the numbers, that's why I asked for both. 


^ permalink raw reply

* Re: linux-next: build failure after merge of the final tree (net tree related)
From: Stephen Rothwell @ 2011-06-29  6:01 UTC (permalink / raw)
  To: David Miller, netdev; +Cc: linux-next, linux-kernel, Alexey Dobriyan
In-Reply-To: <20110627153522.8884b125.sfr@canb.auug.org.au>

Hi all,

On Mon, 27 Jun 2011 15:35:22 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> On Thu, 23 Jun 2011 15:25:35 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> >
> > After merging the final tree, today's linux-next build (powerpc
> > allyesconfig) failed like this:
> > 
> > drivers/net/can/sja1000/sja1000_of_platform.c: In function 'sja1000_ofp_read_reg':
> > drivers/net/can/sja1000/sja1000_of_platform.c:61:2: error: implicit declaration of function 'in_8'
> > drivers/net/can/sja1000/sja1000_of_platform.c: In function 'sja1000_ofp_write_reg':
> > drivers/net/can/sja1000/sja1000_of_platform.c:67:2: error: implicit declaration of function 'out_8'
> > drivers/net/can/sja1000/sja1000_of_platform.c: In function 'sja1000_ofp_remove':
> > drivers/net/can/sja1000/sja1000_of_platform.c:81:2: error: implicit declaration of function 'iounmap'
> > drivers/net/can/sja1000/sja1000_of_platform.c: In function 'sja1000_ofp_probe':
> > drivers/net/can/sja1000/sja1000_of_platform.c:113:2: error: implicit declaration of function 'ioremap_nocache'
> > drivers/net/can/sja1000/sja1000_of_platform.c:113:7: warning: assignment makes pointer from integer without a cast
> > 
> > Since this file has not been changed recently, I suspect that this was
> > caused by commit b7f080cfe223 ("net: remove mm.h inclusion from
> > netdevice.h").
> > 
> > I have left the build broken for now since it is also broken for other
> > reasons.
> 
> I am still getting these, is there a fix pending?

I have to wonder why this has taken so long to be fixed ...

I have applied this patch for today:

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Wed, 29 Jun 2011 15:52:00 +1000
Subject: [PATCH] net: include io.h in for iounmap etc

fixes these build errors:

drivers/net/can/sja1000/sja1000_of_platform.c: In function 'sja1000_ofp_read_reg':
drivers/net/can/sja1000/sja1000_of_platform.c:61:2: error: implicit declaration of function 'in_8'
drivers/net/can/sja1000/sja1000_of_platform.c: In function 'sja1000_ofp_write_reg':
drivers/net/can/sja1000/sja1000_of_platform.c:67:2: error: implicit declaration of function 'out_8'
drivers/net/can/sja1000/sja1000_of_platform.c: In function 'sja1000_ofp_remove':
drivers/net/can/sja1000/sja1000_of_platform.c:81:2: error: implicit declaration of function 'iounmap'
drivers/net/can/sja1000/sja1000_of_platform.c: In function 'sja1000_ofp_probe':
drivers/net/can/sja1000/sja1000_of_platform.c:113:2: error: implicit declaration of function 'ioremap_nocache'
drivers/net/can/sja1000/sja1000_of_platform.c:113:7: warning: assignment makes pointer from integer without a cast

Caused by commit b7f080cfe223 ("net: remove mm.h inclusion from
netdevice.h").

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 drivers/net/can/sja1000/sja1000_of_platform.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/net/can/sja1000/sja1000_of_platform.c b/drivers/net/can/sja1000/sja1000_of_platform.c
index 9793df6..cee6ba2 100644
--- a/drivers/net/can/sja1000/sja1000_of_platform.c
+++ b/drivers/net/can/sja1000/sja1000_of_platform.c
@@ -38,6 +38,7 @@
 #include <linux/interrupt.h>
 #include <linux/netdevice.h>
 #include <linux/delay.h>
+#include <linux/io.h>
 #include <linux/can/dev.h>
 
 #include <linux/of_platform.h>
-- 
1.7.5.4

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

^ permalink raw reply related

* Re: linux-next: build failure after merge of the final tree (net tree related)
From: Stephen Rothwell @ 2011-06-29  6:09 UTC (permalink / raw)
  To: Alexey Dobriyan; +Cc: David Miller, netdev, linux-next, linux-kernel
In-Reply-To: <20110627152927.943c34dc.sfr@canb.auug.org.au>

Hi all,

On Mon, 27 Jun 2011 15:29:27 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> On Thu, 23 Jun 2011 16:04:13 +0300 Alexey Dobriyan <adobriyan@gmail.com> wrote:
> >
> > On Thu, Jun 23, 2011 at 8:29 AM, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> > > After merging the final tree, today's linux-next build (powerpc
> > > allyesconfig) failed like this:
> > 
> > I build on all powerpc defconfigs, somehow this driver is not pinned
> > by any of them :^)
> 
> Well, yes, but an allmodconfig or allyesconfig build will get the error.
> 
> > > drivers/net/ll_temac_main.c:209:4: error: implicit declaration of function 'dma_unmap_single'
> 
> Is there a fix?

Again, this has taken some time to fix ...

Dave, I think you have raised my expectations too far ;-)

I have added the following patch for today:

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Wed, 29 Jun 2011 16:03:18 +1000
Subject: [PATCH] net: include dma-mapping.h for dma_map_single etc

fixes thses build errors:

drivers/net/ll_temac_main.c: In function 'temac_dma_bd_release':
drivers/net/ll_temac_main.c:209:4: error: implicit declaration of function 'dma_unmap_single'
drivers/net/ll_temac_main.c:215:3: error: implicit declaration of function 'dma_free_coherent'
drivers/net/ll_temac_main.c: In function 'temac_dma_bd_init':
drivers/net/ll_temac_main.c:243:2: error: implicit declaration of function 'dma_alloc_coherent'
drivers/net/ll_temac_main.c:243:14: warning: assignment makes pointer from integer without a cast
drivers/net/ll_temac_main.c:251:14: warning: assignment makes pointer from integer without a cast
drivers/net/ll_temac_main.c:280:3: error: implicit declaration of function 'dma_map_single'
drivers/net/ll_temac_main.c: In function 'temac_start_xmit_done':
drivers/net/ll_temac_main.c:628:22: warning: cast to pointer from integer of different size

Caused by commit commit b7f080cfe223 ("net: remove mm.h inclusion from
netdevice.h").

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 drivers/net/ll_temac_main.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/net/ll_temac_main.c b/drivers/net/ll_temac_main.c
index e3f1925..728fe41 100644
--- a/drivers/net/ll_temac_main.c
+++ b/drivers/net/ll_temac_main.c
@@ -49,6 +49,7 @@
 #include <linux/ip.h>
 #include <linux/slab.h>
 #include <linux/interrupt.h>
+#include <linux/dma-mapping.h>
 
 #include "ll_temac.h"
 
-- 
1.7.5.4

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

^ permalink raw reply related

* [patch] wanxl: remove a stray irq enable
From: Dan Carpenter @ 2011-06-29  6:29 UTC (permalink / raw)
  To: Krzysztof Halasa; +Cc: open list:NETWORKING DRIVERS, kernel-janitors

This is error path calls unlock_irq() where we haven't disabled the
IRQs.  The comment says that this error path can never happen.

Signed-off-by: Dan Carpenter <error27@gmail.com>
---
This is a static checker fix and I don't have the hardware to test.

diff --git a/drivers/net/wan/wanxl.c b/drivers/net/wan/wanxl.c
index 8d7aa43..44b7071 100644
--- a/drivers/net/wan/wanxl.c
+++ b/drivers/net/wan/wanxl.c
@@ -284,7 +284,7 @@ static netdev_tx_t wanxl_xmit(struct sk_buff *skb, struct net_device *dev)
                 printk(KERN_DEBUG "%s: transmitter buffer full\n", dev->name);
 #endif
 		netif_stop_queue(dev);
-		spin_unlock_irq(&port->lock);
+		spin_unlock(&port->lock);
 		return NETDEV_TX_BUSY;       /* request packet to be queued */
 	}
 

^ permalink raw reply related

* Re: RFT: virtio_net: limit xmit polling
From: Michael S. Tsirkin @ 2011-06-29  8:42 UTC (permalink / raw)
  To: Tom Lendacky
  Cc: Krishna Kumar2, habanero-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
	lguest-uLR06cmDAlY/bJ5BZ2RsiQ, Shirley Ma,
	kvm-u79uwXL29TY76Z2rM5mHXA, Carsten Otte,
	linux-s390-u79uwXL29TY76Z2rM5mHXA, Heiko Carstens,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	virtualization-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	steved-r/Jw6+rmf7HQT0dZR+AlfA, Christian Borntraeger,
	netdev-u79uwXL29TY76Z2rM5mHXA, Martin Schwidefsky,
	linux390-tA70FqPdS9bQT0dZR+AlfA, roprabhu-FYB4Gu1CFyUAvxtiuMwx3w
In-Reply-To: <201106281108.09285.tahm-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>

On Tue, Jun 28, 2011 at 11:08:07AM -0500, Tom Lendacky wrote:
> On Sunday, June 19, 2011 05:27:00 AM Michael S. Tsirkin wrote:
> > OK, different people seem to test different trees.  In the hope to get
> > everyone on the same page, I created several variants of this patch so
> > they can be compared. Whoever's interested, please check out the
> > following, and tell me how these compare:
> > 
> > kernel:
> > 
> > git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git
> > 
> > virtio-net-limit-xmit-polling/base - this is net-next baseline to test
> > against virtio-net-limit-xmit-polling/v0 - fixes checks on out of capacity
> > virtio-net-limit-xmit-polling/v1 - previous revision of the patch
> > 		this does xmit,free,xmit,2*free,free
> > virtio-net-limit-xmit-polling/v2 - new revision of the patch
> > 		this does free,xmit,2*free,free
> > 
> 
> Here's a summary of the results.  I've also attached an ODS format spreadsheet
> (30 KB in size) that might be easier to analyze and also has some pinned VM
> results data.  I broke the tests down into a local guest-to-guest scenario
> and a remote host-to-guest scenario.
> 
> Within the local guest-to-guest scenario I ran:
>   - TCP_RR tests using two different messsage sizes and four different
>     instance counts among 1 pair of VMs and 2 pairs of VMs.
>   - TCP_STREAM tests using four different message sizes and two different
>     instance counts among 1 pair of VMs and 2 pairs of VMs.
> 
> Within the remote host-to-guest scenario I ran:
>   - TCP_RR tests using two different messsage sizes and four different
>     instance counts to 1 VM and 4 VMs.
>   - TCP_STREAM and TCP_MAERTS tests using four different message sizes and
>     two different instance counts to 1 VM and 4 VMs.
> over a 10GbE link.

roprabhu, Tom,

Thanks very much for the testing. So on the first glance
one seems to see a significant performance gain in V0 here,
and a slightly less significant in V2, with V1
being worse than base. But I'm afraid that's not the
whole story, and we'll need to work some more to
know what really goes on, please see below.


Some comments on the results: I found out that V0 because of mistake
on my part was actually almost identical to base.
I pushed out virtio-net-limit-xmit-polling/v1a instead that
actually does what I intended to check. However,
the fact we get such a huge distribution in the results by Tom
most likely means that the noise factor is very large.


>From my experience one way to get stable results is to
divide the throughput by the host CPU utilization
(measured by something like mpstat).
Sometimes throughput doesn't increase (e.g. guest-host)
by CPU utilization does decrease. So it's interesting.


Another issue is that we are trying to improve the latency
of a busy queue here. However STREAM/MAERTS tests ignore the latency
(more or less) while TCP_RR by default runs a single packet per queue.
Without arguing about whether these are practically interesting
workloads, these results are thus unlikely to be significantly affected
by the optimization in question.

What we are interested in, thus, is either TCP_RR with a -b flag
(configure with  --enable-burst) or multiple concurrent
TCP_RRs.



> *** Local Guest-to-Guest ***
> 
> Here's the local guest-to-guest summary for 1 VM pair doing TCP_RR with
> 256/256 request/response message size in transactions per second:
> 
> Instances	Base		V0		V1		V2
> 1		 8,151.56	 8,460.72	 8,439.16	 9,990.37
> 25		48,761.74	51,032.62	51,103.25	49,533.52
> 50		55,687.38	55,974.18	56,854.10	54,888.65
> 100		58,255.06	58,255.86	60,380.90	59,308.36
> 
> Here's the local guest-to-guest summary for 2 VM pairs doing TCP_RR with
> 256/256 request/response message size in transactions per second:
> 
> Instances	Base		V0		V1		V2
> 1		18,758.48	19,112.50	18,597.07	19,252.04
> 25		80,500.50	78,801.78	80,590.68	78,782.07
> 50		80,594.20	77,985.44	80,431.72	77,246.90
> 100		82,023.23	81,325.96	81,303.32	81,727.54
> 
> Here's the local guest-to-guest summary for 1 VM pair doing TCP_STREAM with
> 256, 1K, 4K and 16K message size in Mbps:
> 
> 256:
> Instances	Base		V0		V1		V2
> 1		   961.78	 1,115.92	   794.02	   740.37
> 4		 2,498.33	 2,541.82	 2,441.60	 2,308.26
> 
> 1K:					
> 1		 3,476.61	 3,522.02	 2,170.86	 1,395.57
> 4		 6,344.30	 7,056.57	 7,275.16	 7,174.09
> 
> 4K:					
> 1		 9,213.57	10,647.44	 9,883.42	 9,007.29
> 4		11,070.66	11,300.37	11,001.02	12,103.72
> 
> 16K:
> 1		12,065.94	 9,437.78	11,710.60	 6,989.93
> 4		12,755.28	13,050.78	12,518.06	13,227.33
> 
> Here's the local guest-to-guest summary for 2 VM pairs doing TCP_STREAM with
> 256, 1K, 4K and 16K message size in Mbps:
> 
> 256:
> Instances	Base		V0		V1		V2
> 1		 2,434.98	 2,403.23	 2,308.69	 2,261.35
> 4		 5,973.82	 5,729.48	 5,956.76	 5,831.86
> 
> 1K:
> 1		 5,305.99	 5,148.72	 4,960.67	 5,067.76
> 4		10,628.38	10,649.49	10,098.90	10,380.09
> 
> 4K:
> 1		11,577.03	10,710.33	11,700.53	10,304.09
> 4		14,580.66	14,881.38	14,551.17	15,053.02
> 
> 16K:
> 1		16,801.46	16,072.50	15,773.78	15,835.66
> 4		17,194.00	17,294.02	17,319.78	17,121.09
> 
> 
> *** Remote Host-to-Guest ***
> 
> Here's the remote host-to-guest summary for 1 VM doing TCP_RR with
> 256/256 request/response message size in transactions per second:
> 
> Instances	Base		V0		V1		V2
> 1		 9,732.99	10,307.98	10,529.82	 8,889.28
> 25		43,976.18	49,480.50	46,536.66	45,682.38
> 50		63,031.33	67,127.15	60,073.34	65,748.62
> 100		64,778.43	65,338.07	66,774.12	69,391.22
> 
> Here's the remote host-to-guest summary for 4 VMs doing TCP_RR with
> 256/256 request/response message size in transactions per second:
> 
> Instances	Base		V0		V1		V2
> 1		 39,270.42	 38,253.60	 39,353.10	 39,566.33
> 25		207,120.91	207,964.50	211,539.70	213,882.21
> 50		218,801.54	221,490.56	220,529.48	223,594.25
> 100		218,432.62	215,061.44	222,011.61	223,480.47
> 
> Here's the remote host-to-guest summary for 1 VM doing TCP_STREAM with
> 256, 1K, 4K and 16K message size in Mbps:
> 
> 256:
> Instances	Base		V0		V1		V2
> 1		2,274.74	2,220.38	2,245.26	2,212.30
> 4		5,689.66	5,953.86	5,984.80	5,827.94
> 
> 1K:
> 1		7,804.38	7,236.29	6,716.58	7,485.09
> 4		7,722.42	8,070.38	7,700.45	7,856.76
> 
> 4K:
> 1		8,976.14	9,026.77	9,147.32	9,095.58
> 4		7,532.25	7,410.80	7,683.81	7,524.94
> 
> 16K:
> 1		8,991.61	9,045.10	9,124.58	9,238.34
> 4		7,406.10	7,626.81	7,711.62	7,345.37
> 
> Here's the remote host-to-guest summary for 1 VM doing TCP_MAERTS with
> 256, 1K, 4K and 16K message size in Mbps:
> 
> 256:
> Instances	Base		V0		V1		V2
> 1		1,165.69	1,181.92	1,152.20	1,104.68
> 4		2,580.46	2,545.22	2,436.30	2,601.74
> 
> 1K:
> 1		2,393.34	2,457.22	2,128.86	2,258.92
> 4		7,152.57	7,606.60	8,004.64	7,576.85
> 
> 4K:
> 1		9,258.93	8,505.06	9,309.78	9,215.05
> 4		9,374.20	9,363.48	9,372.53	9,352.00
> 
> 16K:
> 1		9,244.70	9,287.72	9,298.60	9,322.28
> 4		9,380.02	9,347.50	9,377.46	9,372.98
> 
> Here's the remote host-to-guest summary for 4 VMs doing TCP_STREAM with
> 256, 1K, 4K and 16K message size in Mbps:
> 
> 256:
> Instances	Base		V0		V1		V2
> 1		9,392.37	9,390.74	9,395.58	9,392.46
> 4		9,394.24	9,394.46	9,395.42	9,394.05
> 
> 1K:
> 1		9,396.34	9,397.46	9,396.64	9,443.26
> 4		9,397.14	9,402.25	9,398.67	9,391.09
> 
> 4K:
> 1		9,397.16	9,398.07	9,397.30	9,396.33
> 4		9,395.64	9,400.25	9,397.54	9,397.75
> 
> 16K:
> 1		9,396.58	9,397.01	9,397.58	9,397.70
> 4		9,399.15	9,400.02	9,399.66	9,400.16
> 
> 
> Here's the remote host-to-guest summary for 4 VMs doing TCP_MAERTS with
> 256, 1K, 4K and 16K message size in Mbps:
> 
> 256:
> Instances	Base		V0		V1		V2
> 1		5,048.66	5,007.26	5,074.98	4,974.86
> 4		9,217.23	9,245.14	9,263.97	9,294.23
> 
> 1K:
> 1		9,378.32	9,387.12	9,386.21	9,361.55
> 4		9,384.42	9,384.02	9,385.50	9,385.55
> 
> 4K:
> 1		9,391.10	9,390.28	9,389.70	9,391.02
> 4		9,384.38	9,383.39	9,384.74	9,384.19
> 
> 16K:
> 1		9,390.77	9,389.62	9,388.07	9,388.19
> 4		9,381.86	9,382.37	9,385.54	9,383.88
> 
> 
> Tom
> 
> > There's also this on top:
> > virtio-net-limit-xmit-polling/v3 -> don't delay avail index update
> > I don't think it's important to test this one, yet
> > 
> > Userspace to use: event index work is not yet merged upstream
> > so the revision to use is still this:
> > git://git.kernel.org/pub/scm/linux/kernel/git/mst/qemu-kvm.git
> > virtio-net-event-idx-v3

^ permalink raw reply

* Re: [PATCH V7 4/4 net-next] vhost: vhost TX zero-copy support
From: Michael S. Tsirkin @ 2011-06-29  9:13 UTC (permalink / raw)
  To: Shirley Ma
  Cc: David Miller, Eric Dumazet, Avi Kivity, Arnd Bergmann, netdev,
	kvm, linux-kernel
In-Reply-To: <1306611267.5180.97.camel@localhost.localdomain>

On Sat, May 28, 2011 at 12:34:27PM -0700, Shirley Ma wrote:
> Hello Michael,
> 
> In order to use wait for completion in shutting down, seems to me
> another work thread is needed to call vhost_zerocopy_add_used,

Hmm I don't see vhost_zerocopy_add_used here.

> it seems
> too much work to address a minor issue here. Do we really need it?

Assuming you mean vhost_zerocopy_signal_used, here's how I would do it:
add a kref and a completion, signal completion in kref_put
callback, when backend is set - kref_get, on cleanup,
kref_put and then wait_for_completion_interruptible.
Where's the need for another thread coming from?

If you like, post a patch with busywait + a FIXME comment,
and I can write up a patch on top.

(BTW, ideally the function that does the signalling should be
in core networking bits so that it's still around
even if the vhost module gets removed).

> Right now, the approach I am using is to ignore outstanding userspace
> buffers during shutting down if any, the device might DMAed some wrong
> data to the wire, do we really care?
> 
> Thanks
> Shirley

I think so, yes, guest is told that memory can be reused so
it might put the credit card number or whatever there :)

> ------------
> 
> This patch maintains the outstanding userspace buffers in the 
> sequence it is delivered to vhost. The outstanding userspace buffers 
> will be marked as done once the lower device buffers DMA has finished. 
> This is monitored through last reference of kfree_skb callback. Two
> buffer index are used for this purpose.
> 
> The vhost passes the userspace buffers info to lower device skb 
> through message control. Since there will be some done DMAs when
> entering vhost handle_tx. The worse case is all buffers in the vq are
> in pending/done status, so we need to notify guest to release DMA done 
> buffers first before get any new buffers from the vq.
> 
> Signed-off-by: Shirley <xma@us.ibm.com>
> ---
> 
>  drivers/vhost/net.c   |   46 +++++++++++++++++++++++++++++++++++++++++++++-
>  drivers/vhost/vhost.c |   47 +++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/vhost/vhost.h |   15 +++++++++++++++
>  3 files changed, 107 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index 2f7c76a..e2eaba6 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -32,6 +32,11 @@
>   * Using this limit prevents one virtqueue from starving others. */
>  #define VHOST_NET_WEIGHT 0x80000
>  
> +/* MAX number of TX used buffers for outstanding zerocopy */
> +#define VHOST_MAX_PEND 128
> +/* change it to 256 when small message size performance issue is addressed */
> +#define VHOST_GOODCOPY_LEN 2048
> +
>  enum {
>  	VHOST_NET_VQ_RX = 0,
>  	VHOST_NET_VQ_TX = 1,
> @@ -151,6 +156,10 @@ static void handle_tx(struct vhost_net *net)
>  	hdr_size = vq->vhost_hlen;
>  
>  	for (;;) {
> +		/* Release DMAs done buffers first */
> +		if (atomic_read(&vq->refcnt) > VHOST_MAX_PEND)
> +			vhost_zerocopy_signal_used(vq, false);
> +
>  		head = vhost_get_vq_desc(&net->dev, vq, vq->iov,
>  					 ARRAY_SIZE(vq->iov),
>  					 &out, &in,
> @@ -166,6 +175,12 @@ static void handle_tx(struct vhost_net *net)
>  				set_bit(SOCK_ASYNC_NOSPACE, &sock->flags);
>  				break;
>  			}
> +			/* If more outstanding DMAs, queue the work */
> +			if (atomic_read(&vq->refcnt) > VHOST_MAX_PEND) {
> +				tx_poll_start(net, sock);
> +				set_bit(SOCK_ASYNC_NOSPACE, &sock->flags);
> +				break;
> +			}
>  			if (unlikely(vhost_enable_notify(vq))) {
>  				vhost_disable_notify(vq);
>  				continue;
> @@ -188,6 +203,26 @@ static void handle_tx(struct vhost_net *net)
>  			       iov_length(vq->hdr, s), hdr_size);
>  			break;
>  		}
> +		/* use msg_control to pass vhost zerocopy ubuf info to skb */
> +		if (sock_flag(sock->sk, SOCK_ZEROCOPY)) {
> +			vq->heads[vq->upend_idx].id = head;
> +			if (len < VHOST_GOODCOPY_LEN)
> +				/* copy don't need to wait for DMA done */
> +				vq->heads[vq->upend_idx].len =
> +							VHOST_DMA_DONE_LEN;
> +			else {
> +				struct ubuf_info *ubuf = &vq->ubuf_info[head];
> +
> +				vq->heads[vq->upend_idx].len = len;
> +				ubuf->callback = vhost_zerocopy_callback;
> +				ubuf->arg = vq;
> +				ubuf->desc = vq->upend_idx;
> +				msg.msg_control = ubuf;
> +				msg.msg_controllen = sizeof(ubuf);
> +			}
> +			atomic_inc(&vq->refcnt);
> +			vq->upend_idx = (vq->upend_idx + 1) % UIO_MAXIOV;
> +		}
>  		/* TODO: Check specific error and bomb out unless ENOBUFS? */
>  		err = sock->ops->sendmsg(NULL, sock, &msg, len);
>  		if (unlikely(err < 0)) {
> @@ -198,12 +233,21 @@ static void handle_tx(struct vhost_net *net)
>  		if (err != len)
>  			pr_debug("Truncated TX packet: "
>  				 " len %d != %zd\n", err, len);
> -		vhost_add_used_and_signal(&net->dev, vq, head, 0);
> +		if (!sock_flag(sock->sk, SOCK_ZEROCOPY))
> +			vhost_add_used_and_signal(&net->dev, vq, head, 0);
>  		total_len += len;
>  		if (unlikely(total_len >= VHOST_NET_WEIGHT)) {
>  			vhost_poll_queue(&vq->poll);
>  			break;
>  		}
> +		/* if upend_idx is full, then wait for free more */
> +/*
> +		if (unlikely(vq->upend_idx == vq->done_idx)) {
> +			tx_poll_start(net, sock);
> +			set_bit(SOCK_ASYNC_NOSPACE, &sock->flags);
> +				break;
> +		}
> +*/

What's this BTW?


>  	}
>  
>  	mutex_unlock(&vq->mutex);
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index 7aa4eea..b76e42a 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -174,6 +174,9 @@ static void vhost_vq_reset(struct vhost_dev *dev,
>  	vq->call_ctx = NULL;
>  	vq->call = NULL;
>  	vq->log_ctx = NULL;
> +	vq->upend_idx = 0;
> +	vq->done_idx = 0;
> +	atomic_set(&vq->refcnt, 0);
>  }
>  
>  static int vhost_worker(void *data)
> @@ -232,6 +235,8 @@ static long vhost_dev_alloc_iovecs(struct vhost_dev *dev)
>  					  GFP_KERNEL);
>  		dev->vqs[i].heads = kmalloc(sizeof *dev->vqs[i].heads *
>  					    UIO_MAXIOV, GFP_KERNEL);
> +		dev->vqs[i].ubuf_info = kmalloc(sizeof *dev->vqs[i].ubuf_info *
> +					    UIO_MAXIOV, GFP_KERNEL);
>  
>  		if (!dev->vqs[i].indirect || !dev->vqs[i].log ||
>  			!dev->vqs[i].heads)
> @@ -244,6 +249,7 @@ err_nomem:
>  		kfree(dev->vqs[i].indirect);
>  		kfree(dev->vqs[i].log);
>  		kfree(dev->vqs[i].heads);
> +		kfree(dev->vqs[i].ubuf_info);
>  	}
>  	return -ENOMEM;
>  }
> @@ -385,6 +391,30 @@ long vhost_dev_reset_owner(struct vhost_dev *dev)
>  	return 0;
>  }
>  
> +/* In case of DMA done not in order in lower device driver for some reason.
> + * upend_idx is used to track end of used idx, done_idx is used to track head
> + * of used idx. Once lower device DMA done contiguously, we will signal KVM
> + * guest used idx.
> + */
> +void vhost_zerocopy_signal_used(struct vhost_virtqueue *vq, bool shutdown)
> +{
> +	int i, j = 0;
> +
> +	for (i = vq->done_idx; i != vq->upend_idx; i = ++i % UIO_MAXIOV) {
> +		if ((vq->heads[i].len == VHOST_DMA_DONE_LEN) || shutdown) {
> +			vq->heads[i].len = VHOST_DMA_CLEAR_LEN;
> +			vhost_add_used_and_signal(vq->dev, vq,
> +						  vq->heads[i].id, 0);
> +			++j;
> +		} else
> +			break;
> +	}
> +	if (j) {
> +		vq->done_idx = i;
> +		atomic_sub(j, &vq->refcnt);
> +	}
> +}
> +
>  /* Caller should have device mutex */
>  void vhost_dev_cleanup(struct vhost_dev *dev)
>  {
> @@ -395,6 +425,9 @@ void vhost_dev_cleanup(struct vhost_dev *dev)
>  			vhost_poll_stop(&dev->vqs[i].poll);
>  			vhost_poll_flush(&dev->vqs[i].poll);
>  		}
> +		/* Wait for all lower device DMAs done */
> +		if (atomic_read(&dev->vqs[i].refcnt))
> +			vhost_zerocopy_signal_used(&dev->vqs[i], true);
>  		if (dev->vqs[i].error_ctx)
>  			eventfd_ctx_put(dev->vqs[i].error_ctx);
>  		if (dev->vqs[i].error)
> @@ -603,6 +636,10 @@ static long vhost_set_vring(struct vhost_dev *d, int ioctl, void __user *argp)
>  
>  	mutex_lock(&vq->mutex);
>  
> +	/* clean up lower device outstanding DMAs, before setting ring */
> +	if (atomic_read(&vq->refcnt))
> +		vhost_zerocopy_signal_used(vq, true);
> +
>  	switch (ioctl) {
>  	case VHOST_SET_VRING_NUM:
>  		/* Resizing ring with an active backend?
> @@ -1416,3 +1453,13 @@ void vhost_disable_notify(struct vhost_virtqueue *vq)
>  		vq_err(vq, "Failed to enable notification at %p: %d\n",
>  		       &vq->used->flags, r);
>  }
> +
> +void vhost_zerocopy_callback(void *arg)
> +{
> +	struct ubuf_info *ubuf = (struct ubuf_info *)arg;
> +	struct vhost_virtqueue *vq;
> +
> +	vq = (struct vhost_virtqueue *)ubuf->arg;
> +	/* set len = 1 to mark this desc buffers done DMA */
> +	vq->heads[ubuf->desc].len = VHOST_DMA_DONE_LEN;
> +}
> diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> index b3363ae..0b11734 100644
> --- a/drivers/vhost/vhost.h
> +++ b/drivers/vhost/vhost.h
> @@ -13,6 +13,11 @@
>  #include <linux/virtio_ring.h>
>  #include <asm/atomic.h>
>  
> +/* This is for zerocopy, used buffer len is set to 1 when lower device DMA
> + * done */
> +#define VHOST_DMA_DONE_LEN	1
> +#define VHOST_DMA_CLEAR_LEN	0
> +
>  struct vhost_device;
>  
>  struct vhost_work;
> @@ -108,6 +113,14 @@ struct vhost_virtqueue {
>  	/* Log write descriptors */
>  	void __user *log_base;
>  	struct vhost_log *log;
> +	/* vhost zerocopy support */
> +	atomic_t refcnt; /* num of outstanding zerocopy DMAs */
> +	/* last used idx for outstanding DMA zerocopy buffers */
> +	int upend_idx;
> +	/* first used idx for DMA done zerocopy buffers */
> +	int done_idx;
> +	/* an array of userspace buffers info */
> +	struct ubuf_info *ubuf_info;
>  };
>  
>  struct vhost_dev {
> @@ -154,6 +167,8 @@ bool vhost_enable_notify(struct vhost_virtqueue *);
>  
>  int vhost_log_write(struct vhost_virtqueue *vq, struct vhost_log *log,
>  		    unsigned int log_num, u64 len);
> +void vhost_zerocopy_callback(void *arg);
> +void vhost_zerocopy_signal_used(struct vhost_virtqueue *vq, bool shutdown);
>  
>  #define vq_err(vq, fmt, ...) do {                                  \
>  		pr_debug(pr_fmt(fmt), ##__VA_ARGS__);       \
> 

^ permalink raw reply

* Re: linux-next: build failure after merge of the final tree (net tree related)
From: David Miller @ 2011-06-29  9:56 UTC (permalink / raw)
  To: sfr; +Cc: netdev, linux-next, linux-kernel, adobriyan
In-Reply-To: <20110629160133.372e53ed.sfr@canb.auug.org.au>

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Wed, 29 Jun 2011 16:01:33 +1000

> From: Stephen Rothwell <sfr@canb.auug.org.au>
> Date: Wed, 29 Jun 2011 15:52:00 +1000
> Subject: [PATCH] net: include io.h in for iounmap etc
> 
> fixes these build errors:
 ...
> Caused by commit b7f080cfe223 ("net: remove mm.h inclusion from
> netdevice.h").
> 
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>

Applied.

^ permalink raw reply

* Re: linux-next: build failure after merge of the final tree (net tree related)
From: David Miller @ 2011-06-29  9:58 UTC (permalink / raw)
  To: sfr; +Cc: adobriyan, netdev, linux-next, linux-kernel
In-Reply-To: <20110629160901.2d3cd6da.sfr@canb.auug.org.au>

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Wed, 29 Jun 2011 16:09:01 +1000

> Again, this has taken some time to fix ...
> 
> Dave, I think you have raised my expectations too far ;-)

Sorry.  I try to give the offender time to fix the build failure
himself, but as you saw this time that didn't work out.

To be honest I'm going to be quite reluctant to apply these kinds of
header removal patches in the future if this is how responsive the
patch submitter is going to be to build fallout. :-/

> From: Stephen Rothwell <sfr@canb.auug.org.au>
> Date: Wed, 29 Jun 2011 16:03:18 +1000
> Subject: [PATCH] net: include dma-mapping.h for dma_map_single etc
> 
> fixes thses build errors:
 ...
> Caused by commit commit b7f080cfe223 ("net: remove mm.h inclusion from
> netdevice.h").
> 
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>

Applied, thanks!

^ permalink raw reply

* Re: [PATCH] vmxnet3: Convert to new vlan model.
From: Michał Mirosław @ 2011-06-29 11:06 UTC (permalink / raw)
  To: Jesse Gross; +Cc: David Miller, netdev, Shreyas Bhatewara, VMware PV-Drivers
In-Reply-To: <1308870279-30773-1-git-send-email-jesse@nicira.com>

2011/6/24 Jesse Gross <jesse@nicira.com>:
> This converts the vmxnet3 driver to use the new vlan model.  In doing so
> it fixes missing tags in tcpdump and failure to do checksum offload when
> tx vlan offload is disabled.
[...]
> --- a/drivers/net/vmxnet3/vmxnet3_drv.c
> +++ b/drivers/net/vmxnet3/vmxnet3_drv.c
[...]
> @@ -2639,12 +2588,13 @@ vmxnet3_declare_features(struct vmxnet3_adapter *adapter, bool dma64)
>
>        netdev->hw_features = NETIF_F_SG | NETIF_F_RXCSUM |
>                NETIF_F_HW_CSUM | NETIF_F_HW_VLAN_TX |
> -               NETIF_F_TSO | NETIF_F_TSO6 | NETIF_F_LRO;
> +               NETIF_F_HW_VLAN_RX | NETIF_F_TSO | NETIF_F_TSO6 |
> +               NETIF_F_LRO;
>        if (dma64)
>                netdev->features |= NETIF_F_HIGHDMA;

***

> -       netdev->vlan_features = netdev->hw_features & ~NETIF_F_HW_VLAN_TX;
> -       netdev->features = netdev->hw_features |
> -               NETIF_F_HW_VLAN_RX | NETIF_F_HW_VLAN_FILTER;
> +       netdev->vlan_features = netdev->hw_features &
> +                               ~(NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_RX);
> +       netdev->features = netdev->hw_features | NETIF_F_HW_VLAN_FILTER;

This will disable NETIF_F_HIGHDMA even if dma64 == true. I propose a
fix that sets NETIF_F_HIGHDMA
in hw_features instead of features at line before '***' mark.

Best Regards,
Michał Mirosław

^ permalink raw reply

* Re: [PATCH] net/core: Convert to current logging forms
From: Neil Horman @ 2011-06-29 11:11 UTC (permalink / raw)
  To: Joe Perches; +Cc: netdev, David S. Miller, linux-kernel
In-Reply-To: <385ebf7e98e377e6e6c384beb961b65d4a95fb18.1309289792.git.joe@perches.com>

On Tue, Jun 28, 2011 at 12:40:10PM -0700, Joe Perches wrote:
> Use pr_fmt, pr_<level>, and netdev_<level> as appropriate.
> 
> Coalesce long formats.
> 
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
>  net/core/dev.c           |  121 +++++++++++++++++++---------------------------
>  net/core/drop_monitor.c  |   10 ++--
>  net/core/neighbour.c     |   15 +++---
>  net/core/net_namespace.c |    6 ++-
>  net/core/netpoll.c       |   60 +++++++++++-----------
>  net/core/pktgen.c        |   27 +++++-----
>  net/core/rtnetlink.c     |    9 ++--
>  net/core/skbuff.c        |   24 ++++------
>  net/core/sock.c          |   21 ++++----
>  9 files changed, 136 insertions(+), 157 deletions(-)
> 
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 6b6ef14..3401227 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -72,6 +72,8 @@
>   *				        - netif_rx() feedback
>   */
>  
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
>  #include <asm/uaccess.h>
>  #include <asm/system.h>
>  #include <linux/bitops.h>
> @@ -433,7 +435,7 @@ void __dev_remove_pack(struct packet_type *pt)
>  		}
>  	}
>  
> -	printk(KERN_WARNING "dev_remove_pack: %p not found.\n", pt);
> +	pr_warn("dev_remove_pack: %p not found\n", pt);
>  out:
>  	spin_unlock(&ptype_lock);
>  }
> @@ -1026,9 +1028,8 @@ rollback:
>  			memcpy(dev->name, oldname, IFNAMSIZ);
>  			goto rollback;
>  		} else {
> -			printk(KERN_ERR
> -			       "%s: name change rollback failed: %d.\n",
> -			       dev->name, ret);
> +			netdev_err(dev, "name change rollback failed: %d\n",
> +				   ret);
>  		}
>  	}
>  
> @@ -1126,9 +1127,10 @@ void dev_load(struct net *net, const char *name)
>  		no_module = request_module("netdev-%s", name);
>  	if (no_module && capable(CAP_SYS_MODULE)) {
>  		if (!request_module("%s", name))
> -			pr_err("Loading kernel module for a network device "
> -"with CAP_SYS_MODULE (deprecated).  Use CAP_NET_ADMIN and alias netdev-%s "
> -"instead\n", name);
> +			pr_err(
> +"Loading kernel module for a network device with CAP_SYS_MODULE (deprecated)\n"
> +"Use CAP_NET_ADMIN and alias netdev-%s instead\n",
> +			       name);
>  	}
>  }
>  EXPORT_SYMBOL(dev_load);
> @@ -1569,10 +1571,8 @@ static void dev_queue_xmit_nit(struct sk_buff *skb, struct net_device *dev)
>  			if (skb_network_header(skb2) < skb2->data ||
>  			    skb2->network_header > skb2->tail) {
>  				if (net_ratelimit())
> -					printk(KERN_CRIT "protocol %04x is "
> -					       "buggy, dev %s\n",
> -					       ntohs(skb2->protocol),
> -					       dev->name);
> +					netdev_crit(dev, "protocol %04x is buggy\n",
> +						    ntohs(skb2->protocol));
>  				skb_reset_network_header(skb2);
>  			}
>  
> @@ -1605,9 +1605,7 @@ static void netif_setup_tc(struct net_device *dev, unsigned int txq)
>  
>  	/* If TC0 is invalidated disable TC mapping */
>  	if (tc->offset + tc->count > txq) {
> -		pr_warning("Number of in use tx queues changed "
> -			   "invalidating tc mappings. Priority "
> -			   "traffic classification disabled!\n");
> +		netdev_warn(dev, "Number of in use tx queues changed invalidating tc mappings. Priority traffic classification disabled!\n");
>  		dev->num_tc = 0;
>  		return;
>  	}
> @@ -1618,11 +1616,8 @@ static void netif_setup_tc(struct net_device *dev, unsigned int txq)
>  
>  		tc = &dev->tc_to_txq[q];
>  		if (tc->offset + tc->count > txq) {
> -			pr_warning("Number of in use tx queues "
> -				   "changed. Priority %i to tc "
> -				   "mapping %i is no longer valid "
> -				   "setting map to 0\n",
> -				   i, q);
> +			netdev_warn(dev, "Number of in use tx queues changed. Priority %i to tc mapping %i is no longer valid. Setting map to 0.\n",
> +				    i, q);
>  			netdev_set_prio_tc_map(dev, i, 0);
>  		}
>  	}
> @@ -1919,8 +1914,7 @@ EXPORT_SYMBOL(skb_gso_segment);
>  void netdev_rx_csum_fault(struct net_device *dev)
>  {
>  	if (net_ratelimit()) {
> -		printk(KERN_ERR "%s: hw csum failure.\n",
> -			dev ? dev->name : "<unknown>");
> +		netdev_err(dev, "hw csum failure\n");
>  		dump_stack();
>  	}
>  }
> @@ -2233,9 +2227,8 @@ static inline u16 dev_cap_txqueue(struct net_device *dev, u16 queue_index)
>  {
>  	if (unlikely(queue_index >= dev->real_num_tx_queues)) {
>  		if (net_ratelimit()) {
> -			pr_warning("%s selects TX queue %d, but "
> -				"real number of TX queues is %d\n",
> -				dev->name, queue_index, dev->real_num_tx_queues);
> +			netdev_warn(dev, "selects TX queue %d, but real number of TX queues is %d\n",
> +				    queue_index, dev->real_num_tx_queues);
>  		}
>  		return 0;
>  	}
> @@ -2465,16 +2458,14 @@ int dev_queue_xmit(struct sk_buff *skb)
>  			}
>  			HARD_TX_UNLOCK(dev, txq);
>  			if (net_ratelimit())
> -				printk(KERN_CRIT "Virtual device %s asks to "
> -				       "queue packet!\n", dev->name);
> +				netdev_crit(dev, "Virtual device asks to queue packet!\n");
>  		} else {
>  			/* Recursion is detected! It is possible,
>  			 * unfortunately
>  			 */
>  recursion_alert:
>  			if (net_ratelimit())
> -				printk(KERN_CRIT "Dead loop on virtual device "
> -				       "%s, fix it urgently!\n", dev->name);
> +				netdev_crit(dev, "Dead loop on virtual device, fix it urgently!\n");
>  		}
>  	}
>  
> @@ -2996,8 +2987,8 @@ static int ing_filter(struct sk_buff *skb, struct netdev_queue *rxq)
>  
>  	if (unlikely(MAX_RED_LOOP < ttl++)) {
>  		if (net_ratelimit())
> -			pr_warning( "Redir loop detected Dropping packet (%d->%d)\n",
> -			       skb->skb_iif, dev->ifindex);
> +			netdev_warn(dev, "Redir loop detected - Dropping packet (%d->%d)\n",
> +				    skb->skb_iif, dev->ifindex);
>  		return TC_ACT_SHOT;
>  	}
>  
> @@ -4366,16 +4357,13 @@ static int __dev_set_promiscuity(struct net_device *dev, int inc)
>  			dev->flags &= ~IFF_PROMISC;
>  		else {
>  			dev->promiscuity -= inc;
> -			printk(KERN_WARNING "%s: promiscuity touches roof, "
> -				"set promiscuity failed, promiscuity feature "
> -				"of device might be broken.\n", dev->name);
> +			netdev_warn(dev, "promiscuity touches roof, set promiscuity failed, promiscuity feature of device might be broken\n");
>  			return -EOVERFLOW;
>  		}
>  	}
>  	if (dev->flags != old_flags) {
> -		printk(KERN_INFO "device %s %s promiscuous mode\n",
> -		       dev->name, (dev->flags & IFF_PROMISC) ? "entered" :
> -							       "left");
> +		netdev_info(dev, "%s promiscuous mode\n",
> +			    (dev->flags & IFF_PROMISC) ? "entered" : "left");
>  		if (audit_enabled) {
>  			current_uid_gid(&uid, &gid);
>  			audit_log(current->audit_context, GFP_ATOMIC,
> @@ -4448,9 +4436,7 @@ int dev_set_allmulti(struct net_device *dev, int inc)
>  			dev->flags &= ~IFF_ALLMULTI;
>  		else {
>  			dev->allmulti -= inc;
> -			printk(KERN_WARNING "%s: allmulti touches roof, "
> -				"set allmulti failed, allmulti feature of "
> -				"device might be broken.\n", dev->name);
> +			netdev_warn(dev, "allmulti touches roof, set allmulti failed, allmulti feature of device might be broken\n");
>  			return -EOVERFLOW;
>  		}
>  	}
> @@ -5127,8 +5113,8 @@ static void rollback_registered_many(struct list_head *head)
>  		 * devices and proceed with the remaining.
>  		 */
>  		if (dev->reg_state == NETREG_UNINITIALIZED) {
> -			pr_debug("unregister_netdevice: device %s/%p never "
> -				 "was registered\n", dev->name, dev);
> +			netdev_dbg(dev, "unregister_netdevice: (%p) device never was registered\n",
> +				   dev);
>  
>  			WARN_ON(1);
>  			list_del(&dev->unreg_list);
> @@ -5204,27 +5190,26 @@ u32 netdev_fix_features(struct net_device *dev, u32 features)
>  	/* Fix illegal checksum combinations */
>  	if ((features & NETIF_F_HW_CSUM) &&
>  	    (features & (NETIF_F_IP_CSUM|NETIF_F_IPV6_CSUM))) {
> -		netdev_warn(dev, "mixed HW and IP checksum settings.\n");
> +		netdev_warn(dev, "mixed HW and IP checksum settings\n");
>  		features &= ~(NETIF_F_IP_CSUM|NETIF_F_IPV6_CSUM);
>  	}
>  
>  	if ((features & NETIF_F_NO_CSUM) &&
>  	    (features & (NETIF_F_HW_CSUM|NETIF_F_IP_CSUM|NETIF_F_IPV6_CSUM))) {
> -		netdev_warn(dev, "mixed no checksumming and other settings.\n");
> +		netdev_warn(dev, "mixed no checksumming and other settings\n");
>  		features &= ~(NETIF_F_IP_CSUM|NETIF_F_IPV6_CSUM|NETIF_F_HW_CSUM);
>  	}
>  
>  	/* Fix illegal SG+CSUM combinations. */
>  	if ((features & NETIF_F_SG) &&
>  	    !(features & NETIF_F_ALL_CSUM)) {
> -		netdev_dbg(dev,
> -			"Dropping NETIF_F_SG since no checksum feature.\n");
> +		netdev_dbg(dev, "Dropping NETIF_F_SG since no checksum feature\n");
>  		features &= ~NETIF_F_SG;
>  	}
>  
>  	/* TSO requires that SG is present as well. */
>  	if ((features & NETIF_F_ALL_TSO) && !(features & NETIF_F_SG)) {
> -		netdev_dbg(dev, "Dropping TSO features since no SG feature.\n");
> +		netdev_dbg(dev, "Dropping TSO features since no SG feature\n");
>  		features &= ~NETIF_F_ALL_TSO;
>  	}
>  
> @@ -5234,7 +5219,7 @@ u32 netdev_fix_features(struct net_device *dev, u32 features)
>  
>  	/* Software GSO depends on SG. */
>  	if ((features & NETIF_F_GSO) && !(features & NETIF_F_SG)) {
> -		netdev_dbg(dev, "Dropping NETIF_F_GSO since no SG feature.\n");
> +		netdev_dbg(dev, "Dropping NETIF_F_GSO since no SG feature\n");
>  		features &= ~NETIF_F_GSO;
>  	}
>  
> @@ -5244,14 +5229,12 @@ u32 netdev_fix_features(struct net_device *dev, u32 features)
>  		if (!((features & NETIF_F_GEN_CSUM) ||
>  		    (features & (NETIF_F_IP_CSUM|NETIF_F_IPV6_CSUM))
>  			    == (NETIF_F_IP_CSUM|NETIF_F_IPV6_CSUM))) {
> -			netdev_dbg(dev,
> -				"Dropping NETIF_F_UFO since no checksum offload features.\n");
> +			netdev_dbg(dev, "Dropping NETIF_F_UFO since no checksum offload features\n");
>  			features &= ~NETIF_F_UFO;
>  		}
>  
>  		if (!(features & NETIF_F_SG)) {
> -			netdev_dbg(dev,
> -				"Dropping NETIF_F_UFO since no NETIF_F_SG feature.\n");
> +			netdev_dbg(dev, "Dropping NETIF_F_UFO since no NETIF_F_SG feature\n");
>  			features &= ~NETIF_F_UFO;
>  		}
>  	}
> @@ -5279,15 +5262,14 @@ int __netdev_update_features(struct net_device *dev)
>  		return 0;
>  
>  	netdev_dbg(dev, "Features changed: 0x%08x -> 0x%08x\n",
> -		dev->features, features);
> +		   dev->features, features);
>  
>  	if (dev->netdev_ops->ndo_set_features)
>  		err = dev->netdev_ops->ndo_set_features(dev, features);
>  
>  	if (unlikely(err < 0)) {
> -		netdev_err(dev,
> -			"set_features() failed (%d); wanted 0x%08x, left 0x%08x\n",
> -			err, features, dev->features);
> +		netdev_err(dev, "set_features() failed (%d); wanted 0x%08x, left 0x%08x\n",
> +			   err, features, dev->features);
>  		return -1;
>  	}
>  
> @@ -5366,7 +5348,8 @@ static int netif_alloc_rx_queues(struct net_device *dev)
>  
>  	rx = kcalloc(count, sizeof(struct netdev_rx_queue), GFP_KERNEL);
>  	if (!rx) {
> -		pr_err("netdev: Unable to allocate %u rx queues.\n", count);
> +		netdev_err(dev, "netdev: Unable to allocate %u rx queues\n",
> +			   count);
>  		return -ENOMEM;
>  	}
>  	dev->_rx = rx;
> @@ -5397,8 +5380,8 @@ static int netif_alloc_netdev_queues(struct net_device *dev)
>  
>  	tx = kcalloc(count, sizeof(struct netdev_queue), GFP_KERNEL);
>  	if (!tx) {
> -		pr_err("netdev: Unable to allocate %u tx queues.\n",
> -		       count);
> +		netdev_err(dev, "netdev: Unable to allocate %u tx queues\n",
> +			   count);
>  		return -ENOMEM;
>  	}
Don't all of these get called prior to device registration?  This change will
have us printing out unregistered net device: ... rather than netdev: ... on
these errors.  Not tragic, but I rather think its nicer to just say netdev:...

>  	dev->_tx = tx;
> @@ -5658,10 +5641,8 @@ static void netdev_wait_allrefs(struct net_device *dev)
>  		refcnt = netdev_refcnt_read(dev);
>  
>  		if (time_after(jiffies, warning_time + 10 * HZ)) {
> -			printk(KERN_EMERG "unregister_netdevice: "
> -			       "waiting for %s to become free. Usage "
> -			       "count = %d\n",
> -			       dev->name, refcnt);
> +			netdev_emerg(dev, "unregister_netdevice: waiting to become free. Usage count = %d\n",
> +				     refcnt);
>  			warning_time = jiffies;
>  		}
>  	}
> @@ -5706,8 +5687,8 @@ void netdev_run_todo(void)
>  		list_del(&dev->todo_list);
>  
>  		if (unlikely(dev->reg_state != NETREG_UNREGISTERING)) {
> -			printk(KERN_ERR "network todo '%s' but state %d\n",
> -			       dev->name, dev->reg_state);
> +			netdev_err(dev, "network todo but state %d\n",
> +				   dev->reg_state);
>  			dump_stack();
>  			continue;
>  		}
Ditto on the unregistered net device thing.

><snip>

> diff --git a/net/core/neighbour.c b/net/core/neighbour.c
> index ceb505b..adcf198 100644
> --- a/net/core/neighbour.c
> +++ b/net/core/neighbour.c
> @@ -15,6 +15,8 @@
>   *	Harald Welte		Add neighbour cache statistics like rtstat
>   */
>  
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
>  #include <linux/slab.h>
>  #include <linux/types.h>
>  #include <linux/kernel.h>
> @@ -693,14 +695,13 @@ void neigh_destroy(struct neighbour *neigh)
>  	NEIGH_CACHE_STAT_INC(neigh->tbl, destroys);
>  
>  	if (!neigh->dead) {
> -		printk(KERN_WARNING
> -		       "Destroying alive neighbour %p\n", neigh);
> +		pr_warn("Destroying alive neighbour %p\n", neigh);
>  		dump_stack();
>  		return;
>  	}
>  
>  	if (neigh_del_timer(neigh))
> -		printk(KERN_WARNING "Impossible event.\n");
> +		pr_warn("Impossible event\n");
>  
>  	while ((hh = neigh->hh) != NULL) {
>  		neigh->hh = hh->hh_next;
> @@ -882,7 +883,7 @@ static void neigh_timer_handler(unsigned long arg)
>  
>  	if (!(state & NUD_IN_TIMER)) {
>  #ifndef CONFIG_SMP
> -		printk(KERN_WARNING "neigh: timer & !nud_in_timer\n");
> +		pr_warn("timer & !nud_in_timer\n");
>  #endif
>  		goto out;
>  	}
> @@ -1575,8 +1576,8 @@ void neigh_table_init(struct neigh_table *tbl)
>  	write_unlock(&neigh_tbl_lock);
>  
>  	if (unlikely(tmp)) {
> -		printk(KERN_ERR "NEIGH: Registering multiple tables for "
> -		       "family %d\n", tbl->family);
> +		pr_err("Registering multiple tables for family %d\n",
> +		       tbl->family);
>  		dump_stack();
>  	}
>  }
> @@ -1592,7 +1593,7 @@ int neigh_table_clear(struct neigh_table *tbl)
>  	pneigh_queue_purge(&tbl->proxy_queue);
>  	neigh_ifdown(tbl, NULL);
>  	if (atomic_read(&tbl->entries))
> -		printk(KERN_CRIT "neighbour leakage\n");
> +		pr_crit("neighbour leakage\n");
>  	write_lock(&neigh_tbl_lock);
>  	for (tp = &neigh_tables; *tp; tp = &(*tp)->next) {
>  		if (*tp == tbl) {
> diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
> index ea489db..95ac0de 100644
> --- a/net/core/net_namespace.c
> +++ b/net/core/net_namespace.c
> @@ -1,3 +1,5 @@
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
>  #include <linux/workqueue.h>
>  #include <linux/rtnetlink.h>
>  #include <linux/cache.h>
> @@ -202,8 +204,8 @@ static void net_free(struct net *net)
>  {
>  #ifdef NETNS_REFCNT_DEBUG
>  	if (unlikely(atomic_read(&net->use_count) != 0)) {
> -		printk(KERN_EMERG "network namespace not free! Usage: %d\n",
> -			atomic_read(&net->use_count));
> +		pr_emerg("network namespace not free! Usage: %d\n",
> +			 atomic_read(&net->use_count));
>  		return;
>  	}
>  #endif
> diff --git a/net/core/netpoll.c b/net/core/netpoll.c
> index 18d9cbd..25e9af5 100644
> --- a/net/core/netpoll.c
> +++ b/net/core/netpoll.c
> @@ -9,6 +9,8 @@
>   * Copyright (C) 2002  Red Hat, Inc.
>   */
>  
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
>  #include <linux/moduleparam.h>
>  #include <linux/netdevice.h>
>  #include <linux/etherdevice.h>
> @@ -629,18 +631,18 @@ out:
>  
>  void netpoll_print_options(struct netpoll *np)
>  {
> -	printk(KERN_INFO "%s: local port %d\n",
> -			 np->name, np->local_port);
> -	printk(KERN_INFO "%s: local IP %pI4\n",
> -			 np->name, &np->local_ip);
> -	printk(KERN_INFO "%s: interface '%s'\n",
> -			 np->name, np->dev_name);
> -	printk(KERN_INFO "%s: remote port %d\n",
> -			 np->name, np->remote_port);
> -	printk(KERN_INFO "%s: remote IP %pI4\n",
> -			 np->name, &np->remote_ip);
> -	printk(KERN_INFO "%s: remote ethernet address %pM\n",
> -	                 np->name, np->remote_mac);
> +	pr_info("%s: local port %d\n",
> +		np->name, np->local_port);
> +	pr_info("%s: local IP %pI4\n",
> +		np->name, &np->local_ip);
> +	pr_info("%s: interface '%s'\n",
> +		np->name, np->dev_name);
> +	pr_info("%s: remote port %d\n",
> +		np->name, np->remote_port);
> +	pr_info("%s: remote IP %pI4\n",
> +		np->name, &np->remote_ip);
> +	pr_info("%s: remote ethernet address %pM\n",
> +		np->name, np->remote_mac);
>  }
>  EXPORT_SYMBOL(netpoll_print_options);
>  
> @@ -682,8 +684,8 @@ int netpoll_parse_options(struct netpoll *np, char *opt)
>  			goto parse_failed;
>  		*delim = 0;
>  		if (*cur == ' ' || *cur == '\t')
> -			printk(KERN_INFO "%s: warning: whitespace"
> -					"is not allowed\n", np->name);
> +			pr_info("%s: warning: whitespace is not allowed\n",
> +				np->name);
>  		np->remote_port = simple_strtol(cur, NULL, 10);
>  		cur = delim;
>  	}
> @@ -707,8 +709,8 @@ int netpoll_parse_options(struct netpoll *np, char *opt)
>  	return 0;
>  
>   parse_failed:
> -	printk(KERN_INFO "%s: couldn't parse config at '%s'!\n",
> -	       np->name, cur);
> +	pr_info("%s: couldn't parse config at '%s'!\n",
> +		np->name, cur);
>  	return -1;
>  }
>  EXPORT_SYMBOL(netpoll_parse_options);
> @@ -723,7 +725,7 @@ int __netpoll_setup(struct netpoll *np)
>  
>  	if ((ndev->priv_flags & IFF_DISABLE_NETPOLL) ||
>  	    !ndev->netdev_ops->ndo_poll_controller) {
> -		printk(KERN_ERR "%s: %s doesn't support polling, aborting.\n",
> +		pr_err("%s: %s doesn't support polling, aborting\n",
>  		       np->name, np->dev_name);
>  		err = -ENOTSUPP;
>  		goto out;
> @@ -787,13 +789,13 @@ int netpoll_setup(struct netpoll *np)
>  	if (np->dev_name)
>  		ndev = dev_get_by_name(&init_net, np->dev_name);
>  	if (!ndev) {
> -		printk(KERN_ERR "%s: %s doesn't exist, aborting.\n",
> +		pr_err("%s: %s doesn't exist, aborting\n",
>  		       np->name, np->dev_name);
>  		return -ENODEV;
>  	}
>  
>  	if (ndev->master) {
> -		printk(KERN_ERR "%s: %s is a slave device, aborting.\n",
> +		pr_err("%s: %s is a slave device, aborting\n",
>  		       np->name, np->dev_name);
>  		err = -EBUSY;
>  		goto put;
> @@ -802,15 +804,15 @@ int netpoll_setup(struct netpoll *np)
>  	if (!netif_running(ndev)) {
>  		unsigned long atmost, atleast;
>  
> -		printk(KERN_INFO "%s: device %s not up yet, forcing it\n",
> -		       np->name, np->dev_name);
> +		pr_info("%s: device %s not up yet, forcing it\n",
> +			np->name, np->dev_name);
>  
>  		rtnl_lock();
>  		err = dev_open(ndev);
>  		rtnl_unlock();
>  
>  		if (err) {
> -			printk(KERN_ERR "%s: failed to open %s\n",
> +			pr_err("%s: failed to open %s\n",
>  			       np->name, ndev->name);
>  			goto put;
>  		}
> @@ -819,9 +821,8 @@ int netpoll_setup(struct netpoll *np)
>  		atmost = jiffies + carrier_timeout * HZ;
>  		while (!netif_carrier_ok(ndev)) {
>  			if (time_after(jiffies, atmost)) {
> -				printk(KERN_NOTICE
> -				       "%s: timeout waiting for carrier\n",
> -				       np->name);
> +				pr_notice("%s: timeout waiting for carrier\n",
> +					  np->name);
>  				break;
>  			}
>  			msleep(1);
> @@ -833,9 +834,8 @@ int netpoll_setup(struct netpoll *np)
>  		 */
>  
>  		if (time_before(jiffies, atleast)) {
> -			printk(KERN_NOTICE "%s: carrier detect appears"
> -			       " untrustworthy, waiting 4 seconds\n",
> -			       np->name);
> +			pr_notice("%s: carrier detect appears untrustworthy, waiting 4 seconds\n",
> +				  np->name);
>  			msleep(4000);
>  		}
>  	}
> @@ -846,7 +846,7 @@ int netpoll_setup(struct netpoll *np)
>  
>  		if (!in_dev || !in_dev->ifa_list) {
>  			rcu_read_unlock();
> -			printk(KERN_ERR "%s: no IP address for %s, aborting\n",
> +			pr_err("%s: no IP address for %s, aborting\n",
>  			       np->name, np->dev_name);
>  			err = -EDESTADDRREQ;
>  			goto put;
> @@ -854,7 +854,7 @@ int netpoll_setup(struct netpoll *np)
>  
>  		np->local_ip = in_dev->ifa_list->ifa_local;
>  		rcu_read_unlock();
> -		printk(KERN_INFO "%s: local IP %pI4\n", np->name, &np->local_ip);
> +		pr_info("%s: local IP %pI4\n", np->name, &np->local_ip);
>  	}
>  
>  	np->dev = ndev;
> diff --git a/net/core/pktgen.c b/net/core/pktgen.c
> index f76079c..be6224c 100644
> --- a/net/core/pktgen.c
> +++ b/net/core/pktgen.c
> @@ -505,7 +505,7 @@ static ssize_t pgctrl_write(struct file *file, const char __user *buf,
>  		pktgen_reset_all_threads();
>  
>  	else
> -		pr_warning("Unknown command: %s\n", data);
> +		pr_warn("Unknown command: %s\n", data);
>  
>  	err = count;
>  
> @@ -855,14 +855,14 @@ static ssize_t pktgen_if_write(struct file *file,
>  	pg_result = &(pkt_dev->result[0]);
>  
>  	if (count < 1) {
> -		pr_warning("wrong command format\n");
> +		pr_warn("wrong command format\n");
>  		return -EINVAL;
>  	}
>  
>  	max = count;
>  	tmp = count_trail_chars(user_buffer, max);
>  	if (tmp < 0) {
> -		pr_warning("illegal format\n");
> +		pr_warn("illegal format\n");
>  		return tmp;
>  	}
>  	i = tmp;
> @@ -2020,15 +2020,15 @@ static void pktgen_setup_inject(struct pktgen_dev *pkt_dev)
>  	ntxq = pkt_dev->odev->real_num_tx_queues;
>  
>  	if (ntxq <= pkt_dev->queue_map_min) {
> -		pr_warning("WARNING: Requested queue_map_min (zero-based) (%d) exceeds valid range [0 - %d] for (%d) queues on %s, resetting\n",
> -			   pkt_dev->queue_map_min, (ntxq ?: 1) - 1, ntxq,
> -			   pkt_dev->odevname);
> +		pr_warn("WARNING: Requested queue_map_min (zero-based) (%d) exceeds valid range [0 - %d] for (%d) queues on %s, resetting\n",
> +			pkt_dev->queue_map_min, (ntxq ?: 1) - 1, ntxq,
> +			pkt_dev->odevname);
>  		pkt_dev->queue_map_min = ntxq - 1;
>  	}
>  	if (pkt_dev->queue_map_max >= ntxq) {
> -		pr_warning("WARNING: Requested queue_map_max (zero-based) (%d) exceeds valid range [0 - %d] for (%d) queues on %s, resetting\n",
> -			   pkt_dev->queue_map_max, (ntxq ?: 1) - 1, ntxq,
> -			   pkt_dev->odevname);
> +		pr_warn("WARNING: Requested queue_map_max (zero-based) (%d) exceeds valid range [0 - %d] for (%d) queues on %s, resetting\n",
> +			pkt_dev->queue_map_max, (ntxq ?: 1) - 1, ntxq,
> +			pkt_dev->odevname);
>  		pkt_dev->queue_map_max = ntxq - 1;
>  	}
>  
> @@ -3159,8 +3159,7 @@ static int pktgen_stop_device(struct pktgen_dev *pkt_dev)
>  	int nr_frags = pkt_dev->skb ? skb_shinfo(pkt_dev->skb)->nr_frags : -1;
>  
>  	if (!pkt_dev->running) {
> -		pr_warning("interface: %s is already stopped\n",
> -			   pkt_dev->odevname);
> +		pr_warn("interface: %s is already stopped\n", pkt_dev->odevname);
>  		return -EINVAL;
>  	}
>  
> @@ -3675,7 +3674,7 @@ static int pktgen_remove_device(struct pktgen_thread *t,
>  	pr_debug("remove_device pkt_dev=%p\n", pkt_dev);
>  
>  	if (pkt_dev->running) {
> -		pr_warning("WARNING: trying to remove a running interface, stopping it now\n");
> +		pr_warn("WARNING: trying to remove a running interface, stopping it now\n");
>  		pktgen_stop_device(pkt_dev);
>  	}
>  
> @@ -3729,8 +3728,8 @@ static int __init pg_init(void)
>  
>  		err = pktgen_create_thread(cpu);
>  		if (err)
> -			pr_warning("WARNING: Cannot create thread for cpu %d (%d)\n",
> -				   cpu, err);
> +			pr_warn("WARNING: Cannot create thread for cpu %d (%d)\n",
> +				cpu, err);
>  	}
>  
>  	if (list_empty(&pktgen_threads)) {
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index a798fc6..53fc9f6 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -16,6 +16,8 @@
>   *	Vitaly E. Lavrov		RTA_OK arithmetics was wrong.
>   */
>  
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
>  #include <linux/errno.h>
>  #include <linux/module.h>
>  #include <linux/types.h>
> @@ -1468,10 +1470,9 @@ static int do_setlink(struct net_device *dev, struct ifinfomsg *ifm,
>  
>  errout:
>  	if (err < 0 && modified && net_ratelimit())
> -		printk(KERN_WARNING "A link change request failed with "
> -		       "some changes committed already. Interface %s may "
> -		       "have been left with an inconsistent configuration, "
> -		       "please check.\n", dev->name);
> +		netdev_warn(dev,
> +"A link change request failed with some changes committed already.\n"
> +"Interface may have been left with an inconsistent configuration, please check.\n");
>  
>  	if (send_addr_notify)
>  		call_netdevice_notifiers(NETDEV_CHANGEADDR, dev);
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 46cbd28..e67b4a4 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -120,11 +120,9 @@ static const struct pipe_buf_operations sock_pipe_buf_ops = {
>   */
>  static void skb_over_panic(struct sk_buff *skb, int sz, void *here)
>  {
> -	printk(KERN_EMERG "skb_over_panic: text:%p len:%d put:%d head:%p "
> -			  "data:%p tail:%#lx end:%#lx dev:%s\n",
> -	       here, skb->len, sz, skb->head, skb->data,
> -	       (unsigned long)skb->tail, (unsigned long)skb->end,
> -	       skb->dev ? skb->dev->name : "<NULL>");
> +	netdev_emerg(skb->dev, "skb_over_panic: text:%p len:%d put:%d head:%p data:%p tail:%#lx end:%#lx\n",
> +		     here, skb->len, sz, skb->head, skb->data,
> +		     (unsigned long)skb->tail, (unsigned long)skb->end);
>  	BUG();
>  }
>  
Are you guaranteed to have skb->dev be non-null here?  netdev_printk handles
that, but flaggin the fact that we have a NULL net device when thats not really
an issue seems like it would destract from the purpose of this printk.  Same
with the others in this file below

> @@ -139,11 +137,9 @@ static void skb_over_panic(struct sk_buff *skb, int sz, void *here)
>  
>  static void skb_under_panic(struct sk_buff *skb, int sz, void *here)
>  {
> -	printk(KERN_EMERG "skb_under_panic: text:%p len:%d put:%d head:%p "
> -			  "data:%p tail:%#lx end:%#lx dev:%s\n",
> -	       here, skb->len, sz, skb->head, skb->data,
> -	       (unsigned long)skb->tail, (unsigned long)skb->end,
> -	       skb->dev ? skb->dev->name : "<NULL>");
> +	netdev_emerg(skb->dev, "skb_under_panic: text:%p len:%d put:%d head:%p data:%p tail:%#lx end:%#lx\n",
> +		     here, skb->len, sz, skb->head, skb->data,
> +		     (unsigned long)skb->tail, (unsigned long)skb->end);
>  	BUG();
>  }
>  
> @@ -3061,9 +3057,8 @@ bool skb_partial_csum_set(struct sk_buff *skb, u16 start, u16 off)
>  	if (unlikely(start > skb_headlen(skb)) ||
>  	    unlikely((int)start + off > skb_headlen(skb) - 2)) {
>  		if (net_ratelimit())
> -			printk(KERN_WARNING
> -			       "bad partial csum: csum=%u/%u len=%u\n",
> -			       start, off, skb_headlen(skb));
> +			netdev_warn(skb->dev, "bad partial csum: csum=%u/%u len=%u\n",
> +				    start, off, skb_headlen(skb));
>  		return false;
>  	}
>  	skb->ip_summed = CHECKSUM_PARTIAL;
> @@ -3076,7 +3071,6 @@ EXPORT_SYMBOL_GPL(skb_partial_csum_set);
>  void __skb_warn_lro_forwarding(const struct sk_buff *skb)
>  {
>  	if (net_ratelimit())
> -		pr_warning("%s: received packets cannot be forwarded"
> -			   " while LRO is enabled\n", skb->dev->name);
> +		netdev_warn(skb->dev, "received packets cannot be forwarded while LRO is enabled\n");
>  }
>  EXPORT_SYMBOL(__skb_warn_lro_forwarding);
><snip>
Neil

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

* Re: [RFC v2 2/2] e100: Support RXFCS feature flag.
From: Michał Mirosław @ 2011-06-29 11:37 UTC (permalink / raw)
  To: greearb; +Cc: netdev
In-Reply-To: <1308942371-12748-3-git-send-email-greearb@candelatech.com>

2011/6/24  <greearb@candelatech.com>:
> From: Ben Greear <greearb@candelatech.com>
>
> This allows e100 to be configured to append the
> Ethernet FCS to the skb.
>
> Useful for sniffing networks.
>
> Signed-off-by: Ben Greear <greearb@candelatech.com>
> ---
> :100644 100644 c1352c6... 761f6f5... M  drivers/net/e100.c
>  drivers/net/e100.c |   15 ++++++++++++---
>  1 files changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/e100.c b/drivers/net/e100.c
> index c1352c6..761f6f5 100644
> --- a/drivers/net/e100.c
> +++ b/drivers/net/e100.c
> @@ -1089,6 +1089,7 @@ static void e100_configure(struct nic *nic, struct cb *cb, struct sk_buff *skb)
>  {
>        struct config *config = &cb->u.config;
>        u8 *c = (u8 *)config;
> +       struct net_device *netdev = nic->netdev;
>
>        cb->command = cpu_to_le16(cb_config);
>
> @@ -1132,6 +1133,9 @@ static void e100_configure(struct nic *nic, struct cb *cb, struct sk_buff *skb)
>                config->promiscuous_mode = 0x1;         /* 1=on, 0=off */
>        }
>
> +       if (netdev->wanted_features & NETIF_F_RXFCS)
> +               config->rx_crc_transfer = 0x1;  /* 1=save, 0=discard */
> +

You should check netdev->features here.

>        if (nic->flags & multicast_all)
>                config->multicast_all = 0x1;            /* 1=accept, 0=no */
>
> @@ -1919,6 +1923,7 @@ static int e100_rx_indicate(struct nic *nic, struct rx *rx,
>        struct sk_buff *skb = rx->skb;
>        struct rfd *rfd = (struct rfd *)skb->data;
>        u16 rfd_status, actual_size;
> +       u16 fcs_pad = 0;
>
>        if (unlikely(work_done && *work_done >= work_to_do))
>                return -EAGAIN;
> @@ -1951,9 +1956,11 @@ static int e100_rx_indicate(struct nic *nic, struct rx *rx,
>        }
>
>        /* Get actual data size */
> +       if (dev->wanted_features & NETIF_F_RXFCS)
> +               fcs_pad = 4;

Same here.

Best Regards,
Michał Mirosław

^ permalink raw reply

* Re: [PATCH] net/core: Convert to current logging forms
From: David Miller @ 2011-06-29 11:55 UTC (permalink / raw)
  To: nhorman; +Cc: joe, netdev, linux-kernel
In-Reply-To: <20110629111109.GA1613@hmsreliant.think-freely.org>


Neil, please do not quote a entire large patch just to comment on one
or two hunks of it.  Just quote the patch hunks you want to provide
feedback on.

What you're doing makes it completely unmanageable when I review the
feedback a patch gets, both in my mailbox and in patchwork.

Thanks.

^ permalink raw reply

* [PATCH net-next] net: am79c961a: Omit check for multicast bit in netdev_for_each_mc_addr
From: Tobias Klauser @ 2011-06-29 12:14 UTC (permalink / raw)
  To: Russell King, David S. Miller; +Cc: netdev, linux-arm-kernel

There is no need to check for the address being a multicast address in
the netdev_for_each_mc_addr loop, so remove it.

Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
---
 drivers/net/arm/am79c961a.c |   14 ++++++--------
 1 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/net/arm/am79c961a.c b/drivers/net/arm/am79c961a.c
index 7b3e23f..52fe21e 100644
--- a/drivers/net/arm/am79c961a.c
+++ b/drivers/net/arm/am79c961a.c
@@ -199,17 +199,15 @@ am79c961_ramtest(struct net_device *dev, unsigned int val)
 
 static void am79c961_mc_hash(char *addr, u16 *hash)
 {
-	if (addr[0] & 0x01) {
-		int idx, bit;
-		u32 crc;
+	int idx, bit;
+	u32 crc;
 
-		crc = ether_crc_le(ETH_ALEN, addr);
+	crc = ether_crc_le(ETH_ALEN, addr);
 
-		idx = crc >> 30;
-		bit = (crc >> 26) & 15;
+	idx = crc >> 30;
+	bit = (crc >> 26) & 15;
 
-		hash[idx] |= 1 << bit;
-	}
+	hash[idx] |= 1 << bit;
 }
 
 static unsigned int am79c961_get_rx_mode(struct net_device *dev, u16 *hash)
-- 
1.7.5.4

^ permalink raw reply related

* RE: [PATCH RFC] r8169: minimal rtl8111e-vl support
From: hayeswang @ 2011-06-29 12:18 UTC (permalink / raw)
  To: 'Francois Romieu'; +Cc: netdev
In-Reply-To: <20110628214410.GA4163@electric-eye.fr.zoreil.com>

Hi,

The driver of Realtek includes testing chips which are used internally and don't
need be supported.
I plan to start implementing the new chips next week. Maybe you could continue
after that time if you don't mind.
 
Best Regards,
Hayes


-----Original Message-----
From: Francois Romieu [mailto:romieu@fr.zoreil.com] 
Sent: Wednesday, June 29, 2011 5:44 AM
To: Hayeswang
Cc: netdev@vger.kernel.org
Subject: [PATCH RFC] r8169: minimal rtl8111e-vl support

Mostly bits from version 8.023.00 of Realtek's own r8168 driver. It applies on
top of davem's net-next branch + a small, uninteresting attached patch.

I have given it a short testing w/o the new-format rtl8168e-3_0.0.1 firmware and
it is fairly encouraging.

The code is still incomplete :
- WoL needs some care. No difficulty here.
- rtl8168e_2_hw_phy_config imho deserves a few comments similar to those in
  rtl8168e_1_hw_phy_config. Hayes, can you take care of it ?
- I have excluded a set of completely unidentified registers / bits
  operations, for instance:
  - Config5
      BIT_0
  - Config2
      BIT_5
      BIT_7
  - TxConfig
      BIT_7
  - 0x1a
      BIT_2
      BIT_3
  - 0x1b
      0xf8 / 0x07
  - 0xb0,
      0xee480010
  - 0xd0
      BIT_6
  - 0xd3
      BIT_7
  - 0xf2
      BIT_6
  Either they are not needed or someone will have to name them adequately.
  Hayes ?
- Short packets apparently need to be checksummed by the driver (?) but
  the work-around seems strange. It is not clear to me what Realtek's
  driver is trying to achieve in hard_start_xmit. Hayes, can you elaborate ?

Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>
---


^ permalink raw reply

* [PATCH net-next] net: iseries_veth: Omit check for multicast bit in netdev_for_each_mc_addr
From: Tobias Klauser @ 2011-06-29 12:15 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev

There is no need to check for the address being a multicast address in
the netdev_for_each_mc_addr loop, so remove it.

Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
---
 drivers/net/iseries_veth.c |    8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/net/iseries_veth.c b/drivers/net/iseries_veth.c
index b6c296f..242bf52 100644
--- a/drivers/net/iseries_veth.c
+++ b/drivers/net/iseries_veth.c
@@ -964,11 +964,9 @@ static void veth_set_multicast_list(struct net_device *dev)
 			u8 *addr = ha->addr;
 			u64 xaddr = 0;
 
-			if (addr[0] & 0x01) {/* multicast address? */
-				memcpy(&xaddr, addr, ETH_ALEN);
-				port->mcast_addr[port->num_mcast] = xaddr;
-				port->num_mcast++;
-			}
+			memcpy(&xaddr, addr, ETH_ALEN);
+			port->mcast_addr[port->num_mcast] = xaddr;
+			port->num_mcast++;
 		}
 	}
 
-- 
1.7.5.4


^ permalink raw reply related

* [PATCH net-next] net: depca: Omit check for multicast bit in netdev_for_each_mc_addr
From: Tobias Klauser @ 2011-06-29 12:15 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev

There is no need to check for the address being a multicast address in
the netdev_for_each_mc_addr loop, so remove it.

Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
---
 drivers/net/depca.c |   21 ++++++++-------------
 1 files changed, 8 insertions(+), 13 deletions(-)

diff --git a/drivers/net/depca.c b/drivers/net/depca.c
index d54a0e9..a7ccaa6 100644
--- a/drivers/net/depca.c
+++ b/drivers/net/depca.c
@@ -1270,7 +1270,6 @@ static void SetMulticastFilter(struct net_device *dev)
 {
 	struct depca_private *lp = netdev_priv(dev);
 	struct netdev_hw_addr *ha;
-	char *addrs;
 	int i, j, bit, byte;
 	u16 hashcode;
 	u32 crc;
@@ -1285,19 +1284,15 @@ static void SetMulticastFilter(struct net_device *dev)
 		}
 		/* Add multicast addresses */
 		netdev_for_each_mc_addr(ha, dev) {
-			addrs = ha->addr;
-			if ((*addrs & 0x01) == 1) {	/* multicast address? */
-				crc = ether_crc(ETH_ALEN, addrs);
-				hashcode = (crc & 1);	/* hashcode is 6 LSb of CRC ... */
-				for (j = 0; j < 5; j++) {	/* ... in reverse order. */
-					hashcode = (hashcode << 1) | ((crc >>= 1) & 1);
-				}
-
-
-				byte = hashcode >> 3;	/* bit[3-5] -> byte in filter */
-				bit = 1 << (hashcode & 0x07);	/* bit[0-2] -> bit in byte */
-				lp->init_block.mcast_table[byte] |= bit;
+			crc = ether_crc(ETH_ALEN, ha->addr);
+			hashcode = (crc & 1);	/* hashcode is 6 LSb of CRC ... */
+			for (j = 0; j < 5; j++) {	/* ... in reverse order. */
+				hashcode = (hashcode << 1) | ((crc >>= 1) & 1);
 			}
+
+			byte = hashcode >> 3;	/* bit[3-5] -> byte in filter */
+			bit = 1 << (hashcode & 0x07);	/* bit[0-2] -> bit in byte */
+			lp->init_block.mcast_table[byte] |= bit;
 		}
 	}
 }
-- 
1.7.5.4


^ permalink raw reply related

* [PATCH net-next] net: ucc_geth:  Omit check for multicast bit in netdev_for_each_mc_addr
From: Tobias Klauser @ 2011-06-29 12:16 UTC (permalink / raw)
  To: David S. Miller, Li Yang; +Cc: netdev, linuxppc-dev

There is no need to check for the address being a multicast address in
the netdev_for_each_mc_addr loop, so remove it.

Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
---
 drivers/net/ucc_geth.c |    5 -----
 1 files changed, 0 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ucc_geth.c b/drivers/net/ucc_geth.c
index 3127700..d3465ab 100644
--- a/drivers/net/ucc_geth.c
+++ b/drivers/net/ucc_geth.c
@@ -2030,11 +2030,6 @@ static void ucc_geth_set_multi(struct net_device *dev)
 			out_be32(&p_82xx_addr_filt->gaddr_l, 0x0);
 
 			netdev_for_each_mc_addr(ha, dev) {
-				/* Only support group multicast for now.
-				 */
-				if (!is_multicast_ether_addr(ha->addr))
-					continue;
-
 				/* Ask CPM to run CRC and set bit in
 				 * filter mask.
 				 */
-- 
1.7.5.4


^ permalink raw reply related

* [PATCH net-next] net: de4x5: Omit check for multicast bit in netdev_for_each_mc_addr
From: Tobias Klauser @ 2011-06-29 12:16 UTC (permalink / raw)
  To: David S. Miller, Grant Grundler; +Cc: netdev

There is no need to check for the address being a multicast address in
the netdev_for_each_mc_addr loop, so remove it.

Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
---
 drivers/net/tulip/de4x5.c |    6 +-----
 1 files changed, 1 insertions(+), 5 deletions(-)

diff --git a/drivers/net/tulip/de4x5.c b/drivers/net/tulip/de4x5.c
index efaa1d6..0c3151a 100644
--- a/drivers/net/tulip/de4x5.c
+++ b/drivers/net/tulip/de4x5.c
@@ -1954,7 +1954,6 @@ SetMulticastFilter(struct net_device *dev)
     u16 hashcode;
     u32 omr, crc;
     char *pa;
-    unsigned char *addrs;
 
     omr = inl(DE4X5_OMR);
     omr &= ~(OMR_PR | OMR_PM);
@@ -1964,9 +1963,7 @@ SetMulticastFilter(struct net_device *dev)
 	omr |= OMR_PM;                       /* Pass all multicasts */
     } else if (lp->setup_f == HASH_PERF) {   /* Hash Filtering */
 	netdev_for_each_mc_addr(ha, dev) {
-	    addrs = ha->addr;
-	    if ((*addrs & 0x01) == 1) {      /* multicast address? */
-		crc = ether_crc_le(ETH_ALEN, addrs);
+		crc = ether_crc_le(ETH_ALEN, ha->addr);
 		hashcode = crc & HASH_BITS;  /* hashcode is 9 LSb of CRC */
 
 		byte = hashcode >> 3;        /* bit[3-8] -> byte in filter */
@@ -1977,7 +1974,6 @@ SetMulticastFilter(struct net_device *dev)
 		    byte -= 1;
 		}
 		lp->setup_frame[byte] |= bit;
-	    }
 	}
     } else {                                 /* Perfect filtering */
 	netdev_for_each_mc_addr(ha, dev) {
-- 
1.7.5.4


^ 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