Netdev List
 help / color / mirror / Atom feed
* [patch] netfilter: information leaks building packet message
From: Dan Carpenter @ 2013-08-01  9:36 UTC (permalink / raw)
  To: Pablo Neira Ayuso
  Cc: Patrick McHardy, Jozsef Kadlecsik, David S. Miller,
	netfilter-devel, netfilter, coreteam, netdev, kernel-janitors

These structs have a "_pad" member.  Also the "phw" structs have an 8
byte "hw_addr[]" array but sometimes only the first 6 bytes are
initialized.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
Applies to -next and -mainline.

diff --git a/net/netfilter/nfnetlink_log.c b/net/netfilter/nfnetlink_log.c
index 962e979..d92cc31 100644
--- a/net/netfilter/nfnetlink_log.c
+++ b/net/netfilter/nfnetlink_log.c
@@ -419,6 +419,7 @@ __build_packet_message(struct nfnl_log_net *log,
 	nfmsg->version = NFNETLINK_V0;
 	nfmsg->res_id = htons(inst->group_num);
 
+	memset(&pmsg, 0, sizeof(pmsg));
 	pmsg.hw_protocol	= skb->protocol;
 	pmsg.hook		= hooknum;
 
@@ -498,7 +499,10 @@ __build_packet_message(struct nfnl_log_net *log,
 	if (indev && skb->dev &&
 	    skb->mac_header != skb->network_header) {
 		struct nfulnl_msg_packet_hw phw;
-		int len = dev_parse_header(skb, phw.hw_addr);
+		int len;
+
+		memset(&phw, 0, sizeof(phw));
+		len = dev_parse_header(skb, phw.hw_addr);
 		if (len > 0) {
 			phw.hw_addrlen = htons(len);
 			if (nla_put(inst->skb, NFULA_HWADDR, sizeof(phw), &phw))
diff --git a/net/netfilter/nfnetlink_queue_core.c b/net/netfilter/nfnetlink_queue_core.c
index 971ea14..8a703c3 100644
--- a/net/netfilter/nfnetlink_queue_core.c
+++ b/net/netfilter/nfnetlink_queue_core.c
@@ -463,7 +463,10 @@ nfqnl_build_packet_message(struct nfqnl_instance *queue,
 	if (indev && entskb->dev &&
 	    entskb->mac_header != entskb->network_header) {
 		struct nfqnl_msg_packet_hw phw;
-		int len = dev_parse_header(entskb, phw.hw_addr);
+		int len;
+
+		memset(&phw, 0, sizeof(phw));
+		len = dev_parse_header(entskb, phw.hw_addr);
 		if (len) {
 			phw.hw_addrlen = htons(len);
 			if (nla_put(skb, NFQA_HWADDR, sizeof(phw), &phw))

^ permalink raw reply related

* [PATCH v4] net: Add MOXA ART SoCs ethernet driver
From: Jonas Jensen @ 2013-08-01  9:39 UTC (permalink / raw)
  To: netdev
  Cc: davem, linux-doc, devicetree, linux-arm-kernel, linux-kernel, arm,
	florian, joe, Jonas Jensen
In-Reply-To: <1375280401-5564-1-git-send-email-jonas.jensen@gmail.com>

The MOXA UC-711X hardware(s) has an ethernet controller that seem to be
developed internally. The IC used is "RTL8201CP".

Since there is no public documentation, this driver is mostly the one
published by MOXA that has been heavily cleaned up / ported from linux 2.6.9.

Signed-off-by: Jonas Jensen <jonas.jensen@gmail.com>
---

Notes:
    Thanks, changes from your feedback is enclosed.
    
    I have since found a problem occurring on high load. Please point out
    anything the driver can do differently to eliminate it.
    
    The error only occurs during external ping, i.e. ping -s 50000 from
    10.0.1.200 to hardware.
    
    After the error, ping reports 100% packet loss from the hardware, but
    it's still possible to ping in outgoing direction.
    
    Changes since v3:
    
    1. don't flood the log, use net_dbg_ratelimited(), not netdev_err()
    2. free TX skb:s after stats increment tx_bytes, not before
    3. don't print extra message on alloc failure
    4. use %pM extended format printk specifier
    5. increment rx_errors if build_skb() fails
    
    Applies to next-20130731
    
    Interestingly, on the old port, 1980 is the largest allowed size,
    ping segfaults between sizes 1976 to 1980:
    
    uname -a
    Linux Moxa 2.6.9-uc0 #230 Fri Mar 5 11:04:24 CST 2010 armv4tl unknown
    root@Moxa:/# ping -s 1981 10.0.1.200
    ping: packet size too large.
    root@Moxa:/# ping -s 1980 10.0.1.200
    Segmentation fault
    root@Moxa:/# ping -s 1976 10.0.1.200
    Segmentation fault
    root@Moxa:/# ping -s 1975 10.0.1.200
    PING  (10.0.1.200): 1975 data bytes
    1983 bytes from 10.0.1.200: icmp_seq=0 ttl=64 time=6.9 ms
    1983 bytes from 10.0.1.200: icmp_seq=1 ttl=64 time=16.2 ms
    
    uname -a
    Linux (none) 3.11.0-rc3-next-20130731+ #200 PREEMPT Thu Aug 1 10:09:16 CEST 2013
     armv4l GNU/Linux
    ping -s 50000 10.0.1.200
    PING 10.0.1.200 (10.0.1.200): 50000 data bytes
    50008 bytes from 10.0.1.200: seq=0 ttl=64 time=36.294 ms
    50008 bytes from 10.0.1.200: seq=1 ttl=64 time=35.708 ms
    50008 bytes from 10.0.1.200: seq=2 ttl=64 time=35.763 ms
    50008 bytes from 10.0.1.200: seq=3 ttl=64 time=35.703 ms
    50008 bytes from 10.0.1.200: seq=4 ttl=64 time=35.789 ms
    50008 bytes from 10.0.1.200: seq=5 ttl=64 time=35.647 ms
    50008 bytes from 10.0.1.200: seq=6 ttl=64 time=36.012 ms
    50008 bytes from 10.0.1.200: seq=7 ttl=64 time=35.650 ms
    50008 bytes from 10.0.1.200: seq=8 ttl=64 time=35.700 ms
    50008 bytes from 10.0.1.200: seq=9 ttl=64 time=35.845 ms
    50008 bytes from 10.0.1.200: seq=10 ttl=64 time=35.886 ms
    ^C
    --- 10.0.1.200 ping statistics ---
    11 packets transmitted, 11 packets received, 0% packet loss
    round-trip min/avg/max = 35.647/35.817/36.294 ms
    ping -s 50000 10.0.1.200
    PING 10.0.1.200 (10.0.1.200): 50000 data bytes
    50008 bytes from 10.0.1.200: seq=0 ttl=64 time=35.694 ms
    ping: recvfrom: Bad address
    [   33.340000] BUG: Bad page state in process ping  pfn:019c0
    [   33.350000] page:c082e800 count:-59 mapcount:0 mapping:  (null) index:0x0
    [   33.360000] page flags: 0x0()
    [   33.360000] CPU: 0 PID: 229 Comm: ping Not tainted 3.11.0-rc3-next-20130731+ #198
    [   33.390000] [<c000c680>] (unwind_backtrace+0x0/0xf0) from [<c000b294>] (show_stack+0x10/0x14)
    [   33.400000] [<c000b294>] (show_stack+0x10/0x14) from [<c0060404>] (bad_page+0xa8/0x108)
    [   33.410000] [<c0060404>] (bad_page+0xa8/0x108) from [<c00609dc>] (get_page_from_freelist+0x474/0x594)
    [   33.420000] [<c00609dc>] (get_page_from_freelist+0x474/0x594) from [<c0061424>] (__alloc_pages_nodemask+0xcc/0x794)
    [   33.430000] [<c0061424>] (__alloc_pages_nodemask+0xcc/0x794) from [<c00852d0>] (new_slab+0x64/0x2dc)
    [   33.440000] [<c00852d0>] (new_slab+0x64/0x2dc) from [<c021b17c>] (__slab_alloc.isra.57.constprop.59+0x4cc/0x5f0)
    [   33.450000] [<c021b17c>] (__slab_alloc.isra.57.constprop.59+0x4cc/0x5f0) from [<c0086f20>] (__kmalloc_track_caller+0x120/0x180)
    [   33.460000] [<c0086f20>] (__kmalloc_track_caller+0x120/0x180) from [<c01a33c4>] (__kmalloc_reserve.isra.16+0x24/0x70)
    [   33.470000] [<c01a33c4>] (__kmalloc_reserve.isra.16+0x24/0x70) from [<c01a34bc>] (__alloc_skb+0x5c/0x148)
    [   33.480000] [<c01a34bc>] (__alloc_skb+0x5c/0x148) from [<c01a00d8>] (sock_wmalloc+0x38/0x88)
    [   33.490000] [<c01a00d8>] (sock_wmalloc+0x38/0x88) from [<c01d3dfc>] (__ip_append_data+0x8cc/0x980)
    [   33.500000] [<c01d3dfc>] (__ip_append_data+0x8cc/0x980) from [<c01d59b4>] (ip_append_data+0x94/0xbc)
    [   33.510000] [<c01d59b4>] (ip_append_data+0x94/0xbc) from [<c01f6350>] (raw_sendmsg+0x2e8/0x858)
    [   33.520000] [<c01f6350>] (raw_sendmsg+0x2e8/0x858) from [<c020247c>] (inet_sendmsg+0x3c/0x70)
    [   33.530000] [<c020247c>] (inet_sendmsg+0x3c/0x70) from [<c019c5c4>] (sock_sendmsg+0x78/0x8c)
    [   33.540000] [<c019c5c4>] (sock_sendmsg+0x78/0x8c) from [<c019d830>] (SyS_sendto+0xb4/0xd4)
    [   33.550000] [<c019d830>] (SyS_sendto+0xb4/0xd4) from [<c0009080>] (ret_fast_syscall+0x0/0x44)
    [   33.560000] BUG: Bad page state in process ping  pfn:019c4
    [   33.560000] page:c082e880 count:-38 mapcount:0 mapping:  (null) index:0x0
    [   33.570000] page flags: 0x0()
    [   33.570000] CPU: 0 PID: 229 Comm: ping Tainted: G    B        3.11.0-rc3-next-20130731+ #198
    [   33.580000] [<c000c680>] (unwind_backtrace+0x0/0xf0) from [<c000b294>] (show_stack+0x10/0x14)
    [   33.590000] [<c000b294>] (show_stack+0x10/0x14) from [<c0060404>] (bad_page+0xa8/0x108)
    [   33.600000] [<c0060404>] (bad_page+0xa8/0x108) from [<c00609dc>] (get_page_from_freelist+0x474/0x594)
    [   33.610000] [<c00609dc>] (get_page_from_freelist+0x474/0x594) from [<c0061424>] (__alloc_pages_nodemask+0xcc/0x794)
    [   33.620000] [<c0061424>] (__alloc_pages_nodemask+0xcc/0x794) from [<c00852d0>] (new_slab+0x64/0x2dc)
    [   33.630000] [<c00852d0>] (new_slab+0x64/0x2dc) from [<c021b17c>] (__slab_alloc.isra.57.constprop.59+0x4cc/0x5f0)
    [   33.640000] [<c021b17c>] (__slab_alloc.isra.57.constprop.59+0x4cc/0x5f0) from [<c0086f20>] (__kmalloc_track_caller+0x120/0x180)
    [   33.650000] [<c0086f20>] (__kmalloc_track_caller+0x120/0x180) from [<c01a33c4>] (__kmalloc_reserve.isra.16+0x24/0x70)
    [   33.660000] [<c01a33c4>] (__kmalloc_reserve.isra.16+0x24/0x70) from [<c01a34bc>] (__alloc_skb+0x5c/0x148)
    [   33.670000] [<c01a34bc>] (__alloc_skb+0x5c/0x148) from [<c01a00d8>] (sock_wmalloc+0x38/0x88)
    [   33.680000] [<c01a00d8>] (sock_wmalloc+0x38/0x88) from [<c01d3dfc>] (__ip_append_data+0x8cc/0x980)
    [   33.690000] [<c01d3dfc>] (__ip_append_data+0x8cc/0x980) from [<c01d59b4>] (ip_append_data+0x94/0xbc)
    [   33.700000] [<c01d59b4>] (ip_append_data+0x94/0xbc) from [<c01f6350>] (raw_sendmsg+0x2e8/0x858)
    [   33.710000] [<c01f6350>] (raw_sendmsg+0x2e8/0x858) from [<c020247c>] (inet_sendmsg+0x3c/0x70)
    [   33.720000] [<c020247c>] (inet_sendmsg+0x3c/0x70) from [<c019c5c4>] (sock_sendmsg+0x78/0x8c)
    [   33.730000] [<c019c5c4>] (sock_sendmsg+0x78/0x8c) from [<c019d830>] (SyS_sendto+0xb4/0xd4)
    [   33.740000] [<c019d830>] (SyS_sendto+0xb4/0xd4) from [<c0009080>] (ret_fast_syscall+0x0/0x44)
    ...
    http://ideone.com/2FkfMB

 .../devicetree/bindings/net/moxa,moxart-mac.txt    |  25 +
 drivers/net/ethernet/Kconfig                       |   1 +
 drivers/net/ethernet/Makefile                      |   1 +
 drivers/net/ethernet/moxa/Kconfig                  |  30 ++
 drivers/net/ethernet/moxa/Makefile                 |   5 +
 drivers/net/ethernet/moxa/moxart_ether.c           | 577 +++++++++++++++++++++
 drivers/net/ethernet/moxa/moxart_ether.h           | 513 ++++++++++++++++++
 7 files changed, 1152 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/moxa,moxart-mac.txt
 create mode 100644 drivers/net/ethernet/moxa/Kconfig
 create mode 100644 drivers/net/ethernet/moxa/Makefile
 create mode 100644 drivers/net/ethernet/moxa/moxart_ether.c
 create mode 100644 drivers/net/ethernet/moxa/moxart_ether.h

diff --git a/Documentation/devicetree/bindings/net/moxa,moxart-mac.txt b/Documentation/devicetree/bindings/net/moxa,moxart-mac.txt
new file mode 100644
index 0000000..1fc44ff
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/moxa,moxart-mac.txt
@@ -0,0 +1,25 @@
+MOXA ART Ethernet Controller
+
+Required properties:
+
+- compatible : Must be "moxa,moxart-mac"
+- reg : Should contain registers location and length
+  index 0 : main register
+  index 1 : mac address (stored on flash)
+- interrupts : Should contain the mac interrupt number
+
+Example:
+
+	mac0: mac@90900000 {
+		compatible = "moxa,moxart-mac";
+		reg =	<0x90900000 0x100>,
+			<0x80000050 0x6>;
+		interrupts = <25 0>;
+	};
+
+	mac1: mac@92000000 {
+		compatible = "moxa,moxart-mac";
+		reg =	<0x92000000 0x100>,
+			<0x80000056 0x6>;
+		interrupts = <27 0>;
+	};
diff --git a/drivers/net/ethernet/Kconfig b/drivers/net/ethernet/Kconfig
index 2037080..506b024 100644
--- a/drivers/net/ethernet/Kconfig
+++ b/drivers/net/ethernet/Kconfig
@@ -90,6 +90,7 @@ source "drivers/net/ethernet/marvell/Kconfig"
 source "drivers/net/ethernet/mellanox/Kconfig"
 source "drivers/net/ethernet/micrel/Kconfig"
 source "drivers/net/ethernet/microchip/Kconfig"
+source "drivers/net/ethernet/moxa/Kconfig"
 source "drivers/net/ethernet/myricom/Kconfig"
 
 config FEALNX
diff --git a/drivers/net/ethernet/Makefile b/drivers/net/ethernet/Makefile
index 390bd0b..c0b8789 100644
--- a/drivers/net/ethernet/Makefile
+++ b/drivers/net/ethernet/Makefile
@@ -42,6 +42,7 @@ obj-$(CONFIG_NET_VENDOR_MARVELL) += marvell/
 obj-$(CONFIG_NET_VENDOR_MELLANOX) += mellanox/
 obj-$(CONFIG_NET_VENDOR_MICREL) += micrel/
 obj-$(CONFIG_NET_VENDOR_MICROCHIP) += microchip/
+obj-$(CONFIG_NET_VENDOR_MOXART) += moxa/
 obj-$(CONFIG_NET_VENDOR_MYRI) += myricom/
 obj-$(CONFIG_FEALNX) += fealnx.o
 obj-$(CONFIG_NET_VENDOR_NATSEMI) += natsemi/
diff --git a/drivers/net/ethernet/moxa/Kconfig b/drivers/net/ethernet/moxa/Kconfig
new file mode 100644
index 0000000..1731e05
--- /dev/null
+++ b/drivers/net/ethernet/moxa/Kconfig
@@ -0,0 +1,30 @@
+#
+# MOXART device configuration
+#
+
+config NET_VENDOR_MOXART
+	bool "MOXA ART devices"
+	default y
+	depends on (ARM && ARCH_MOXART)
+	---help---
+	  If you have a network (Ethernet) card belonging to this class, say Y
+	  and read the Ethernet-HOWTO, available from
+	  <http://www.tldp.org/docs.html#howto>.
+
+	  Note that the answer to this question doesn't directly affect the
+	  kernel: saying N will just cause the configurator to skip all
+	  the questions about MOXA ART devices. If you say Y, you will be asked
+	  for your specific card in the following questions.
+
+if NET_VENDOR_MOXART
+
+config ARM_MOXART_ETHER
+	tristate "MOXART Ethernet support"
+	depends on ARM && ARCH_MOXART
+	select NET_CORE
+	---help---
+	  If you wish to compile a kernel for a hardware with MOXA ART SoC and
+	  want to use the internal ethernet then you should answer Y to this.
+
+
+endif # NET_VENDOR_MOXART
diff --git a/drivers/net/ethernet/moxa/Makefile b/drivers/net/ethernet/moxa/Makefile
new file mode 100644
index 0000000..aa3c73e9
--- /dev/null
+++ b/drivers/net/ethernet/moxa/Makefile
@@ -0,0 +1,5 @@
+#
+# Makefile for the MOXART network device drivers.
+#
+
+obj-$(CONFIG_ARM_MOXART_ETHER) += moxart_ether.o
diff --git a/drivers/net/ethernet/moxa/moxart_ether.c b/drivers/net/ethernet/moxa/moxart_ether.c
new file mode 100644
index 0000000..8856f4e
--- /dev/null
+++ b/drivers/net/ethernet/moxa/moxart_ether.c
@@ -0,0 +1,577 @@
+/* MOXA ART Ethernet (RTL8201CP) driver.
+ *
+ * Copyright (C) 2013 Jonas Jensen
+ *
+ * Jonas Jensen <jonas.jensen@gmail.com>
+ *
+ * Based on code from
+ * Moxa Technology Co., Ltd. <www.moxa.com>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2.  This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/netdevice.h>
+#include <linux/etherdevice.h>
+#include <linux/skbuff.h>
+#include <linux/dma-mapping.h>
+#include <linux/ethtool.h>
+#include <linux/platform_device.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/crc32.h>
+#include <linux/crc32c.h>
+#include <linux/dma-mapping.h>
+
+#include "moxart_ether.h"
+
+static inline unsigned long moxart_emac_read(struct net_device *ndev,
+					     unsigned int reg)
+{
+	struct moxart_mac_priv_t *priv = netdev_priv(ndev);
+
+	return readl(priv->base + reg);
+}
+
+static inline void moxart_emac_write(struct net_device *ndev,
+				     unsigned int reg, unsigned long value)
+{
+	struct moxart_mac_priv_t *priv = netdev_priv(ndev);
+
+	writel(value, priv->base + reg);
+}
+
+static void moxart_update_mac_address(struct net_device *ndev)
+{
+	moxart_emac_write(ndev, MAC_MADR_REG_OFFSET,
+			  ((ndev->dev_addr[0] << 8) | (ndev->dev_addr[1])));
+	moxart_emac_write(ndev, MAC_MADR_REG_OFFSET + 4,
+			  ((ndev->dev_addr[2] << 24) |
+			   (ndev->dev_addr[3] << 16) |
+			   (ndev->dev_addr[4] << 8) |
+			   (ndev->dev_addr[5])));
+}
+
+static int moxart_set_mac_address(struct net_device *ndev, void *addr)
+{
+	struct sockaddr *address = addr;
+
+	if (!is_valid_ether_addr(address->sa_data))
+		return -EADDRNOTAVAIL;
+
+	memcpy(ndev->dev_addr, address->sa_data, ndev->addr_len);
+	moxart_update_mac_address(ndev);
+
+	return 0;
+}
+
+static void moxart_mac_free_memory(struct net_device *ndev)
+{
+	struct moxart_mac_priv_t *priv = netdev_priv(ndev);
+	int i;
+
+	for (i = 0; i < RX_DESC_NUM; i++)
+		dma_unmap_single(&ndev->dev, priv->rx_mapping[i],
+				 priv->rx_buf_size, DMA_FROM_DEVICE);
+
+	if (priv->tx_desc_base)
+		dma_free_coherent(NULL, sizeof(struct tx_desc_t)*TX_DESC_NUM,
+				  priv->tx_desc_base, priv->tx_base);
+	if (priv->rx_desc_base)
+		dma_free_coherent(NULL, sizeof(struct rx_desc_t)*RX_DESC_NUM,
+				  priv->rx_desc_base, priv->rx_base);
+
+	kfree(priv->tx_buf_base);
+	kfree(priv->rx_buf_base);
+}
+
+static void moxart_mac_reset(struct net_device *ndev)
+{
+	struct moxart_mac_priv_t *priv = netdev_priv(ndev);
+
+	writel(SW_RST, priv->base + MACCR_REG_OFFSET);
+	while (readl(priv->base + MACCR_REG_OFFSET) & SW_RST)
+		mdelay(10);
+
+	writel(0, priv->base + IMR_REG_OFFSET);
+
+	priv->reg_maccr = RX_BROADPKT | FULLDUP | CRC_APD | RX_FTL;
+}
+
+static void moxart_mac_enable(struct net_device *ndev)
+{
+	struct moxart_mac_priv_t *priv = netdev_priv(ndev);
+
+	writel(0x00001010, priv->base + ITC_REG_OFFSET);
+	writel(0x00000001, priv->base + APTC_REG_OFFSET);
+	writel(0x00000390, priv->base + DBLAC_REG_OFFSET);
+
+	priv->reg_imr |= (RPKT_FINISH_M | XPKT_FINISH_M);
+	writel(priv->reg_imr, priv->base + IMR_REG_OFFSET);
+
+	priv->reg_maccr |= (RCV_EN | XMT_EN | RDMA_EN | XDMA_EN);
+	writel(priv->reg_maccr, priv->base + MACCR_REG_OFFSET);
+}
+
+static void moxart_mac_setup_desc_ring(struct net_device *ndev)
+{
+	struct moxart_mac_priv_t *priv = netdev_priv(ndev);
+	struct tx_desc_t *txdesc;
+	struct rx_desc_t *rxdesc;
+	int i;
+
+	for (i = 0; i < TX_DESC_NUM; i++) {
+		txdesc = &priv->tx_desc_base[i];
+		memset(txdesc, 0, sizeof(struct tx_desc_t));
+
+		priv->tx_buf[i] = priv->tx_buf_base + priv->tx_buf_size * i;
+	}
+	priv->tx_desc_base[TX_DESC_NUM - 1].txdes1.ubit.edotr = 1;
+	priv->tx_head = 0;
+	priv->tx_tail = 0;
+
+	for (i = 0; i < RX_DESC_NUM; i++) {
+		rxdesc = &priv->rx_desc_base[i];
+		memset(rxdesc, 0, sizeof(struct rx_desc_t));
+		rxdesc->rxdes0.ubit.rx_dma_own = 1;
+		rxdesc->rxdes1.ubit.rx_buf_size = RX_BUF_SIZE;
+
+		priv->rx_buf[i] = priv->rx_buf_base + priv->rx_buf_size * i;
+		priv->rx_mapping[i] = dma_map_single(&ndev->dev,
+						     priv->rx_buf[i],
+						     priv->rx_buf_size,
+						     DMA_FROM_DEVICE);
+		if (dma_mapping_error(&ndev->dev, priv->rx_mapping[i]))
+			netdev_err(ndev, "DMA mapping error\n");
+
+		rxdesc->rxdes2.addr_phys = priv->rx_mapping[i];
+		rxdesc->rxdes2.addr_virt = priv->rx_buf[i];
+	}
+	priv->rx_desc_base[RX_DESC_NUM - 1].rxdes1.ubit.edorr = 1;
+	priv->rx_head = 0;
+
+	/* reset the MAC controler TX/RX desciptor base address */
+	writel(priv->tx_base, priv->base + TXR_BADR_REG_OFFSET);
+	writel(priv->rx_base, priv->base + RXR_BADR_REG_OFFSET);
+}
+
+static int moxart_mac_open(struct net_device *ndev)
+{
+	struct moxart_mac_priv_t *priv = netdev_priv(ndev);
+
+	if (!is_valid_ether_addr(ndev->dev_addr))
+		return -EADDRNOTAVAIL;
+
+	napi_enable(&priv->napi);
+
+	moxart_mac_reset(ndev);
+	moxart_update_mac_address(ndev);
+	moxart_mac_setup_desc_ring(ndev);
+	moxart_mac_enable(ndev);
+	netif_start_queue(ndev);
+
+	netdev_dbg(ndev, "%s: IMR=0x%x, MACCR=0x%x\n",
+		   __func__, readl(priv->base + IMR_REG_OFFSET),
+		   readl(priv->base + MACCR_REG_OFFSET));
+
+	return 0;
+}
+
+static int moxart_mac_stop(struct net_device *ndev)
+{
+	struct moxart_mac_priv_t *priv = netdev_priv(ndev);
+
+	napi_disable(&priv->napi);
+
+	netif_stop_queue(ndev);
+
+	/* disable all interrupts */
+	writel(0, priv->base + IMR_REG_OFFSET);
+
+	/* disable all functions */
+	writel(0, priv->base + MACCR_REG_OFFSET);
+
+	return 0;
+}
+
+static int moxart_rx_poll(struct napi_struct *napi, int budget)
+{
+	struct moxart_mac_priv_t *priv = container_of(napi,
+						      struct moxart_mac_priv_t,
+						      napi);
+	struct net_device *ndev = priv->ndev;
+	struct rx_desc_t *rxdesc;
+	struct sk_buff *skb;
+	unsigned int ui, len;
+	int rx_head = priv->rx_head;
+	int rx = 0;
+
+	while (1) {
+		rxdesc = &priv->rx_desc_base[rx_head];
+		ui = rxdesc->rxdes0.ui;
+
+		if (ui & RXDMA_OWN)
+			break;
+
+		if (ui & (RX_ERR | CRC_ERR | FTL | RUNT | RX_ODD_NB)) {
+			net_dbg_ratelimited("packet error\n");
+			priv->stats.rx_dropped++;
+			priv->stats.rx_errors++;
+			continue;
+		}
+
+		len = ui & RFL_MASK;
+
+		if (len > RX_BUF_SIZE)
+			len = RX_BUF_SIZE;
+
+		skb = build_skb(priv->rx_buf[rx_head], priv->rx_buf_size);
+		if (unlikely(!skb)) {
+			net_dbg_ratelimited("build_skb failed\n");
+			priv->stats.rx_dropped++;
+			priv->stats.rx_errors++;
+		}
+
+		skb_put(skb, len);
+		skb->protocol = eth_type_trans(skb, ndev);
+		napi_gro_receive(&priv->napi, skb);
+		rx++;
+
+		ndev->last_rx = jiffies;
+		priv->stats.rx_packets++;
+		priv->stats.rx_bytes += len;
+		if (ui & MULTICAST_RXDES0)
+			priv->stats.multicast++;
+
+		rxdesc->rxdes0.ui = RXDMA_OWN;
+
+		rx_head = RX_NEXT(rx_head);
+		priv->rx_head = rx_head;
+
+		if (rx >= budget)
+			break;
+	}
+
+	if (rx < budget) {
+		napi_gro_flush(napi, false);
+		__napi_complete(napi);
+	}
+
+	priv->reg_imr |= RPKT_FINISH_M;
+	writel(priv->reg_imr, priv->base + IMR_REG_OFFSET);
+
+	return rx;
+}
+
+static void moxart_tx_finished(struct net_device *ndev)
+{
+	struct moxart_mac_priv_t *priv = netdev_priv(ndev);
+	unsigned tx_head = priv->tx_head;
+	unsigned tx_tail = priv->tx_tail;
+
+	while (tx_tail != tx_head) {
+		dma_unmap_single(&ndev->dev, priv->tx_mapping[tx_tail],
+				 priv->tx_len[tx_tail], DMA_TO_DEVICE);
+
+		priv->stats.tx_packets++;
+		priv->stats.tx_bytes += priv->tx_skb[tx_tail]->len;
+
+		dev_kfree_skb_irq(priv->tx_skb[tx_tail]);
+		priv->tx_skb[tx_tail] = NULL;
+
+		tx_tail = TX_NEXT(tx_tail);
+	}
+	priv->tx_tail = tx_tail;
+}
+
+static irqreturn_t moxart_mac_interrupt(int irq, void *dev_id)
+{
+	struct net_device *ndev = (struct net_device *) dev_id;
+	struct moxart_mac_priv_t *priv = netdev_priv(ndev);
+	unsigned int ists = readl(priv->base + ISR_REG_OFFSET);
+
+	if (ists & XPKT_OK_INT_STS)
+		moxart_tx_finished(ndev);
+
+	if (ists & RPKT_FINISH) {
+		if (napi_schedule_prep(&priv->napi)) {
+			priv->reg_imr &= ~RPKT_FINISH_M;
+			writel(priv->reg_imr, priv->base + IMR_REG_OFFSET);
+			__napi_schedule(&priv->napi);
+		}
+	}
+
+	return IRQ_HANDLED;
+}
+
+static int moxart_mac_start_xmit(struct sk_buff *skb, struct net_device *ndev)
+{
+	struct moxart_mac_priv_t *priv = netdev_priv(ndev);
+	struct tx_desc_t *txdesc;
+	unsigned int len;
+	unsigned int tx_head = priv->tx_head;
+
+	spin_lock_irq(&priv->txlock);
+
+	txdesc = &priv->tx_desc_base[tx_head];
+
+	if (txdesc->txdes0.ubit.tx_dma_own) {
+		net_dbg_ratelimited("no TX space for packet\n");
+		priv->stats.tx_dropped++;
+		return NETDEV_TX_BUSY;
+	}
+
+	len = skb->len > TX_BUF_SIZE ? TX_BUF_SIZE : skb->len;
+
+	priv->tx_mapping[tx_head] = dma_map_single(&ndev->dev, skb->data,
+						   len, DMA_TO_DEVICE);
+	if (dma_mapping_error(&ndev->dev, priv->tx_mapping[tx_head])) {
+		netdev_err(ndev, "DMA mapping error\n");
+		return NETDEV_TX_BUSY;
+	}
+
+	priv->tx_len[tx_head] = len;
+	priv->tx_skb[tx_head] = skb;
+
+	txdesc->txdes2.addr_phys = priv->tx_mapping[tx_head];
+	txdesc->txdes2.addr_virt = skb->data;
+
+	if (skb->len < ETH_ZLEN) {
+		memset(&txdesc->txdes2.addr_virt[skb->len],
+		       0, ETH_ZLEN - skb->len);
+		len = ETH_ZLEN;
+	}
+
+	txdesc->txdes1.ubit.lts = 1;
+	txdesc->txdes1.ubit.fts = 1;
+	txdesc->txdes1.ubit.tx2_fic = 0;
+	txdesc->txdes1.ubit.tx_ic = 0;
+	txdesc->txdes1.ubit.tx_buf_size = len;
+	txdesc->txdes0.ui = TXDMA_OWN;
+
+	/* start to send packet */
+	writel(0xffffffff, priv->base + TXPD_REG_OFFSET);
+
+	priv->tx_head = TX_NEXT(tx_head);
+
+	ndev->trans_start = jiffies;
+
+	spin_unlock_irq(&priv->txlock);
+
+	return NETDEV_TX_OK;
+}
+
+static struct net_device_stats *moxart_mac_get_stats(struct net_device *ndev)
+{
+	struct moxart_mac_priv_t *priv = netdev_priv(ndev);
+
+	return &priv->stats;
+}
+
+static void moxart_mac_setmulticast(struct net_device *ndev)
+{
+	struct moxart_mac_priv_t *priv = netdev_priv(ndev);
+	struct netdev_hw_addr *ha;
+	int crc_val;
+
+	netdev_for_each_mc_addr(ha, ndev) {
+		crc_val = crc32_le(~0, ha->addr, ETH_ALEN);
+		crc_val = (crc_val >> 26) & 0x3f;
+		if (crc_val >= 32) {
+			writel(readl(priv->base + MATH1_REG_OFFSET) |
+			       (1UL << (crc_val - 32)),
+			       priv->base + MATH1_REG_OFFSET);
+		} else {
+			writel(readl(priv->base + MATH0_REG_OFFSET) |
+			       (1UL << crc_val),
+			       priv->base + MATH0_REG_OFFSET);
+		}
+	}
+}
+
+static void moxart_mac_set_rx_mode(struct net_device *ndev)
+{
+	struct moxart_mac_priv_t *priv = netdev_priv(ndev);
+
+	spin_lock_irq(&priv->txlock);
+
+	(ndev->flags & IFF_PROMISC) ? (priv->reg_maccr |= RCV_ALL) :
+				      (priv->reg_maccr &= ~RCV_ALL);
+
+	(ndev->flags & IFF_ALLMULTI) ? (priv->reg_maccr |= RX_MULTIPKT) :
+				       (priv->reg_maccr &= ~RX_MULTIPKT);
+
+	if ((ndev->flags & IFF_MULTICAST) && netdev_mc_count(ndev)) {
+		priv->reg_maccr |= HT_MULTI_EN;
+		moxart_mac_setmulticast(ndev);
+	} else {
+		priv->reg_maccr &= ~HT_MULTI_EN;
+	}
+
+	writel(priv->reg_maccr, priv->base + MACCR_REG_OFFSET);
+
+	spin_unlock_irq(&priv->txlock);
+}
+
+static struct net_device_ops moxart_netdev_ops = {
+	.ndo_open		= moxart_mac_open,
+	.ndo_stop		= moxart_mac_stop,
+	.ndo_start_xmit		= moxart_mac_start_xmit,
+	.ndo_get_stats		= moxart_mac_get_stats,
+	.ndo_set_rx_mode	= moxart_mac_set_rx_mode,
+	.ndo_set_mac_address	= moxart_set_mac_address,
+	.ndo_validate_addr	= eth_validate_addr,
+	.ndo_change_mtu		= eth_change_mtu,
+};
+
+static void moxart_get_mac_address(struct net_device *ndev)
+{
+	struct moxart_mac_priv_t *priv = netdev_priv(ndev);
+	int i;
+
+	for (i = 0; i <= 5; i++)
+		ndev->dev_addr[i] = readb(priv->flash_base + i);
+}
+
+static int moxart_mac_probe(struct platform_device *pdev)
+{
+	struct device *p_dev = &pdev->dev;
+	struct device_node *node = p_dev->of_node;
+	struct net_device *ndev;
+	struct moxart_mac_priv_t *priv;
+	struct resource *res;
+	unsigned int irq;
+	void *tmp;
+
+	ndev = alloc_etherdev(sizeof(struct moxart_mac_priv_t));
+	if (!ndev)
+		return -ENOMEM;
+
+	irq = irq_of_parse_and_map(node, 0);
+
+	priv = netdev_priv(ndev);
+	priv->ndev = ndev;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	ndev->base_addr = res->start;
+	priv->base = devm_ioremap_resource(p_dev, res);
+	if (IS_ERR(priv->base)) {
+		dev_err(p_dev, "devm_ioremap_resource failed\n");
+		goto init_fail;
+	}
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+
+	/* the flash driver (physmap_of) requests the same region
+	 * so use ioremap instead of devm_ioremap_resource
+	 */
+	priv->flash_base = ioremap(res->start, resource_size(res));
+	if (IS_ERR(priv->flash_base)) {
+		dev_err(p_dev, "devm_ioremap_resource failed\n");
+		goto init_fail;
+	}
+
+	spin_lock_init(&priv->txlock);
+
+	priv->tx_buf_size = TX_BUF_SIZE;
+	priv->rx_buf_size = RX_BUF_SIZE +
+			    SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
+
+	tmp = dma_alloc_coherent(NULL, sizeof(struct tx_desc_t) * TX_DESC_NUM,
+				 &priv->tx_base, GFP_DMA | GFP_KERNEL);
+	priv->tx_desc_base = (struct tx_desc_t *) tmp;
+	if (priv->tx_desc_base == NULL)
+		goto init_fail;
+
+	tmp = dma_alloc_coherent(NULL, sizeof(struct rx_desc_t) * RX_DESC_NUM,
+				 &priv->rx_base, GFP_DMA | GFP_KERNEL);
+	priv->rx_desc_base = (struct rx_desc_t *) tmp;
+	if (priv->rx_desc_base == NULL)
+		goto init_fail;
+
+	priv->tx_buf_base = kmalloc(priv->tx_buf_size * TX_DESC_NUM,
+				    GFP_ATOMIC);
+	if (!priv->tx_buf_base)
+		goto init_fail;
+
+	priv->rx_buf_base = kmalloc(priv->rx_buf_size * RX_DESC_NUM,
+				    GFP_ATOMIC);
+	if (!priv->rx_buf_base)
+		goto init_fail;
+
+	platform_set_drvdata(pdev, ndev);
+
+	ether_setup(ndev);
+	ndev->netdev_ops = &moxart_netdev_ops;
+	netif_napi_add(ndev, &priv->napi, moxart_rx_poll, RX_DESC_NUM);
+	ndev->priv_flags |= IFF_UNICAST_FLT;
+
+	SET_NETDEV_DEV(ndev, &pdev->dev);
+
+	moxart_get_mac_address(ndev);
+	moxart_update_mac_address(ndev);
+
+	if (register_netdev(ndev)) {
+		free_netdev(ndev);
+		goto init_fail;
+	}
+
+	ndev->irq = irq;
+
+	if (devm_request_irq(p_dev, irq, moxart_mac_interrupt, 0,
+			     pdev->name, ndev)) {
+		netdev_err(ndev, "devm_request_irq failed\n");
+		free_netdev(ndev);
+		return -EBUSY;
+	}
+
+	netdev_dbg(ndev, "%s: IRQ=%d address=%pM\n",
+		   __func__, ndev->irq, ndev->dev_addr);
+
+	return 0;
+
+init_fail:
+	netdev_err(ndev, "%s: init failed\n", __func__);
+	moxart_mac_free_memory(ndev);
+
+	return -ENOMEM;
+}
+
+static int moxart_remove(struct platform_device *pdev)
+{
+	struct net_device *ndev = platform_get_drvdata(pdev);
+
+	unregister_netdev(ndev);
+	free_irq(ndev->irq, ndev);
+	moxart_mac_free_memory(ndev);
+	platform_set_drvdata(pdev, NULL);
+	free_netdev(ndev);
+
+	return 0;
+}
+
+static const struct of_device_id moxart_mac_match[] = {
+	{ .compatible = "moxa,moxart-mac" },
+	{ }
+};
+
+struct __initdata platform_driver moxart_mac_driver = {
+	.probe	= moxart_mac_probe,
+	.remove	= moxart_remove,
+	.driver	= {
+		.name		= "moxart-ethernet",
+		.owner		= THIS_MODULE,
+		.of_match_table	= moxart_mac_match,
+	},
+};
+module_platform_driver(moxart_mac_driver);
+
+MODULE_DESCRIPTION("MOXART RTL8201CP Ethernet driver");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Jonas Jensen <jonas.jensen@gmail.com>");
+
diff --git a/drivers/net/ethernet/moxa/moxart_ether.h b/drivers/net/ethernet/moxa/moxart_ether.h
new file mode 100644
index 0000000..790b6a9
--- /dev/null
+++ b/drivers/net/ethernet/moxa/moxart_ether.h
@@ -0,0 +1,513 @@
+/* MOXA ART Ethernet (RTL8201CP) driver.
+ *
+ * Copyright (C) 2013 Jonas Jensen
+ *
+ * Jonas Jensen <jonas.jensen@gmail.com>
+ *
+ * Based on code from
+ * Moxa Technology Co., Ltd. <www.moxa.com>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2.  This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#ifndef _MOXART_ETHERNET_H
+#define _MOXART_ETHERNET_H
+
+#define TX_DESC_NUM		64
+#define TX_DESC_NUM_MASK	(TX_DESC_NUM-1)
+#define TX_NEXT(N)		(((N) + 1) & (TX_DESC_NUM_MASK))
+#define TX_BUF_SIZE		1600
+
+#define RX_DESC_NUM		64
+#define RX_DESC_NUM_MASK	(RX_DESC_NUM-1)
+#define RX_NEXT(N)		(((N) + 1) & (RX_DESC_NUM_MASK))
+#define RX_BUF_SIZE		1600
+
+struct tx_desc_t {
+	union {
+#define TXDMA_OWN		BIT(31)
+#define TXPKT_EXSCOL		BIT(1)
+#define TXPKT_LATECOL		BIT(0)
+		unsigned int ui;
+
+		struct {
+			/* is aborted due to late collision */
+			unsigned int tx_pkt_late_col:1;
+
+			/* is aborted after 16 collisions */
+			unsigned int rx_pkt_exs_col:1;
+
+			unsigned int reserved1:29;
+
+			/* is owned by the MAC controller */
+			unsigned int tx_dma_own:1;
+		} ubit;
+	} txdes0;
+
+	union {
+#define EDOTR			BIT(31)
+#define TXIC			BIT(30)
+#define TX2FIC			BIT(29)
+#define FTS			BIT(28)
+#define LTS			BIT(27)
+#define TXBUF_SIZE_MASK		0x7ff
+#define TXBUF_SIZE_MAX		(TXBUF_SIZE_MASK+1)
+		unsigned int ui;
+
+		struct {
+			/* transmit buffer size in byte */
+			unsigned int tx_buf_size:11;
+
+			unsigned int reserved2:16;
+
+			/* is the last descriptor of a Tx packet */
+			unsigned int lts:1;
+
+			/* is the first descriptor of a Tx packet */
+			unsigned int fts:1;
+
+			/* transmit to FIFO interrupt on completion */
+			unsigned int tx2_fic:1;
+
+			/* transmit interrupt on completion */
+			unsigned int tx_ic:1;
+
+			/* end descriptor of transmit ring */
+			unsigned int edotr:1;
+		} ubit;
+	} txdes1;
+
+	struct {
+		/* transmit buffer physical base address */
+		unsigned int addr_phys;
+
+		/* transmit buffer virtual base address */
+		unsigned char *addr_virt;
+	} txdes2;
+};
+
+struct rx_desc_t {
+	union {
+#define RXDMA_OWN		BIT(31)
+#define FRS			BIT(29)
+#define LRS			BIT(28)
+#define RX_ODD_NB		BIT(22)
+#define RUNT			BIT(21)
+#define FTL			BIT(20)
+#define CRC_ERR			BIT(19)
+#define RX_ERR			BIT(18)
+#define BROADCAST_RXDES0	BIT(17)
+#define MULTICAST_RXDES0	BIT(16)
+#define RFL_MASK		0x7ff
+#define RFL_MAX			(RFL_MASK+1)
+		unsigned int ui;
+
+		struct {
+			/* receive frame length */
+			unsigned int recv_frame_len:11;
+			unsigned int reserved1:5;
+
+			/* multicast frame */
+			unsigned int multicast:1;
+
+			/* broadcast frame */
+			unsigned int broadcast:1;
+			unsigned int rx_err:1;		/* receive error */
+			unsigned int crc_err:1;		/* CRC error */
+			unsigned int ftl:1;		/* frame too long */
+
+			/* runt packet, less than 64 bytes */
+			unsigned int runt:1;
+
+			/* receive odd nibbles */
+			unsigned int rx_odd_nb:1;
+			unsigned int reserved2:5;
+
+			/* last receive segment descriptor */
+			unsigned int lrs:1;
+
+			/* first receive segment descriptor */
+			unsigned int frs:1;
+
+			unsigned int reserved3:1;
+			unsigned int rx_dma_own:1;	/* RXDMA onwership */
+		} ubit;
+	} rxdes0;
+
+	union {
+#define EDORR			BIT(31)
+#define RXBUF_SIZE_MASK		0x7ff
+#define RXBUF_SIZE_MAX		(RXBUF_SIZE_MASK+1)
+		unsigned int ui;
+
+		struct {
+			/* receive buffer size */
+			unsigned int rx_buf_size:11;
+
+			unsigned int reserved4:20;
+
+			/* end descriptor of receive ring */
+			unsigned int edorr:1;
+		} ubit;
+	} rxdes1;
+
+	struct {
+		/* receive buffer physical base address */
+		unsigned int addr_phys;
+
+		/* receive buffer virtual base address */
+		unsigned char *addr_virt;
+	} rxdes2;
+};
+
+struct mac_control_reg_t {
+
+/* RXDMA has received packets into RX buffer successfully */
+#define RPKT_FINISH		BIT(0)
+/* receive buffer unavailable */
+#define NORXBUF			BIT(1)
+/* TXDMA has moved data into the TX FIFO */
+#define XPKT_FINISH		BIT(2)
+/* transmit buffer unavailable */
+#define NOTXBUF			BIT(3)
+/* packets transmitted to ethernet successfully */
+#define XPKT_OK_INT_STS		BIT(4)
+/* packets transmitted to ethernet lost due to late
+ * collision or excessive collision
+ */
+#define XPKT_LOST_INT_STS	BIT(5)
+/* packets received into RX FIFO successfully */
+#define RPKT_SAV		BIT(6)
+/* received packet lost due to RX FIFO full */
+#define RPKT_LOST_INT_STS	BIT(7)
+#define AHB_ERR			BIT(8)		/* AHB error */
+#define PHYSTS_CHG		BIT(9)		/* PHY link status change */
+	unsigned int isr;			/* interrupt status, 0x0 */
+
+#define RPKT_FINISH_M		BIT(0)		/* interrupt mask of ISR[0] */
+#define NORXBUF_M		BIT(1)		/* interrupt mask of ISR[1] */
+#define XPKT_FINISH_M		BIT(2)		/* interrupt mask of ISR[2] */
+#define NOTXBUF_M		BIT(3)		/* interrupt mask of ISR[3] */
+#define XPKT_OK_M		BIT(4)		/* interrupt mask of ISR[4] */
+#define XPKT_LOST_M		BIT(5)		/* interrupt mask of ISR[5] */
+#define RPKT_SAV_M		BIT(6)		/* interrupt mask of ISR[6] */
+#define RPKT_LOST_M		BIT(7)		/* interrupt mask of ISR[7] */
+#define AHB_ERR_M		BIT(8)		/* interrupt mask of ISR[8] */
+#define PHYSTS_CHG_M		BIT(9)		/* interrupt mask of ISR[9] */
+	unsigned int imr;			/* interrupt mask, 0x4 */
+
+/* the most significant 2 bytes of MAC address */
+#define MAC_MADR_MASK		0xffff
+	/* MAC most significant address, 0x8 */
+	unsigned int mac_madr;
+
+	/* MAC least significant address, 0xc */
+	unsigned int mac_ldar;
+
+	/* multicast address hash table 0, 0x10 */
+	unsigned int matht0;
+
+	/* multicast address hash table 1, 0x14 */
+	unsigned int matht1;
+
+	/* transmit poll demand, 0x18 */
+	unsigned int txpd;
+
+	/* receive poll demand, 0x1c */
+	unsigned int rxpd;
+
+	/* transmit ring base address, 0x20 */
+	unsigned int txr_badr;
+
+	/* receive ring base address, 0x24 */
+	unsigned int rxr_badr;
+
+/* defines the period of TX cycle time */
+#define TXINT_TIME_SEL		BIT(15)
+#define TXINT_THR_MASK		0x7000
+#define TXINT_CNT_MASK		0xf00
+/* defines the period of RX cycle time */
+#define RXINT_TIME_SEL		BIT(7)
+#define RXINT_THR_MASK		0x70
+#define RXINT_CNT_MASK		0xF
+	/* interrupt timer control, 0x28 */
+	unsigned int itc;
+
+/* defines the period of TX poll time */
+#define TXPOLL_TIME_SEL		BIT(12)
+#define TXPOLL_CNT_MASK		0xf00
+#define TXPOLL_CNT_SHIFT_BIT	8
+/* defines the period of RX poll time */
+#define RXPOLL_TIME_SEL		BIT(4)
+#define RXPOLL_CNT_MASK		0xF
+#define RXPOLL_CNT_SHIFT_BIT	0
+	/* automatic polling timer control, 0x2c */
+	unsigned int aptc;
+
+/* enable RX FIFO threshold arbitration */
+#define RX_THR_EN		BIT(9)
+#define RXFIFO_HTHR_MASK	0x1c0
+#define RXFIFO_LTHR_MASK	0x38
+/* use INCR16 burst command in AHB bus */
+#define INCR16_EN		BIT(2)
+/* use INCR8 burst command in AHB bus */
+#define INCR8_EN		BIT(1)
+/* use INCR4 burst command in AHB bus */
+#define INCR4_EN		BIT(0)
+	/* DMA burst length and arbitration control, 0x30 */
+	unsigned int dblac;
+
+	unsigned int reserved1[21];		/* 0x34 - 0x84 */
+
+#define RX_BROADPKT		BIT(17)		/* receive boradcast packet */
+/* receive all multicast packet */
+#define RX_MULTIPKT		BIT(16)
+#define FULLDUP			BIT(15)		/* full duplex */
+/* append CRC to transmitted packet */
+#define CRC_APD			BIT(14)
+/* do not check incoming packet's destination address */
+#define RCV_ALL			BIT(12)
+/* store incoming packet even if its length is great than 1518 bytes */
+#define RX_FTL			BIT(11)
+/* store incoming packet even if its length is less than 64 bytes */
+#define RX_RUNT			BIT(10)
+/* enable storing incoming packet if the packet passes hash table
+ * address filtering and is a multicast packet
+ */
+#define HT_MULTI_EN		BIT(9)
+#define RCV_EN			BIT(8)		/* receiver enable */
+/* enable packet reception when transmitting packet in half duplex mode */
+#define ENRX_IN_HALFTX		BIT(6)
+#define XMT_EN			BIT(5)		/* transmitter enable */
+/* disable CRC check when receiving packets */
+#define CRC_DIS			BIT(4)
+#define LOOP_EN			BIT(3)		/* internal loop-back */
+/* software reset, last 64 AHB bus clocks */
+#define SW_RST			BIT(2)
+#define RDMA_EN			BIT(1)		/* enable receive DMA chan */
+#define XDMA_EN			BIT(0)		/* enable transmit DMA chan */
+	unsigned int maccr;			/* MAC control, 0x88 */
+
+#define COL_EXCEED		BIT(11)		/* collisions exceeds 16 */
+/* transmitter detects late collision */
+#define LATE_COL		BIT(10)
+/* packet transmitted to ethernet lost due to late collision
+ * or excessive collision
+ */
+#define XPKT_LOST		BIT(9)
+/* packets transmitted to ethernet successfully */
+#define XPKT_OK			BIT(8)
+/* receiver detects a runt packet */
+#define RUNT_MAC_STS		BIT(7)
+/* receiver detects a frame that is too long */
+#define FTL_MAC_STS		BIT(6)
+#define CRC_ERR_MAC_STS		BIT(5)
+/* received packets list due to RX FIFO full */
+#define RPKT_LOST		BIT(4)
+/* packets received into RX FIFO successfully */
+#define RPKT_SAVE		BIT(3)
+/* incoming packet dropped due to collision */
+#define COL			BIT(2)
+#define MCPU_BROADCAST		BIT(1)
+#define MCPU_MULTICAST		BIT(0)
+	unsigned int macsr;			/* MAC status, 0x8c */
+
+/* initialize a write sequence to PHY by setting this bit to 1
+ * this bit would be auto cleared after the write operation is finished.
+ */
+#define MIIWR			BIT(27)
+#define MIIRD			BIT(26)
+#define REGAD_MASK		0x3e00000
+#define PHYAD_MASK		0x1f0000
+#define MIIRDATA_MASK		0xffff
+	unsigned int phycr;			/* PHY control, 0x90 */
+
+#define MIIWDATA_MASK		0xffff
+	unsigned int phywdata;			/* PHY write data, 0x94 */
+
+#define PAUSE_TIME_MASK		0xffff0000
+#define FC_HIGH_MASK		0xf000
+#define FC_LOW_MASK		0xf00
+#define RX_PAUSE		BIT(4)		/* receive pause frame */
+/* packet transmission is paused due to receive */
+#define TXPAUSED		BIT(3)
+	/* pause frame */
+/* enable flow control threshold mode. */
+#define FCTHR_EN		BIT(2)
+#define TX_PAUSE		BIT(1)		/* transmit pause frame */
+#define FC_EN			BIT(0)		/* flow control mode enable */
+	unsigned int fcr;			/* flow control, 0x98 */
+
+#define BK_LOW_MASK		0xf00
+#define BKJAM_LEN_MASK		0xf0
+#define BK_MODE			BIT(1)		/* back pressure address mode */
+#define BK_EN			BIT(0)		/* back pressure mode enable */
+	unsigned int bpr;			/* back pressure, 0x9c */
+
+	unsigned int reserved2[9];		/* 0xa0 - 0xc0 */
+
+#define TEST_SEED_MASK      0x3fff
+	unsigned int ts;			/* test seed, 0xc4 */
+
+#define TXD_REQ			BIT(31)		/* TXDMA request */
+#define RXD_REQ			BIT(30)		/* RXDMA request */
+#define DARB_TXGNT		BIT(29)		/* TXDMA grant */
+#define DARB_RXGNT		BIT(28)		/* RXDMA grant */
+#define TXFIFO_EMPTY		BIT(27)		/* TX FIFO is empty */
+#define RXFIFO_EMPTY		BIT(26)		/* RX FIFO is empty */
+#define TXDMA2_SM_MASK		0x7000
+#define TXDMA1_SM_MASK		0xf00
+#define RXDMA2_SM_MASK		0x70
+#define RXDMA1_SM_MASK		0xF
+	unsigned int dmafifos;			/* DMA/FIFO state, 0xc8 */
+
+#define SINGLE_PKT		BIT(26)		/* single packet mode */
+/* automatic polling timer test mode */
+#define PTIMER_TEST		BIT(25)
+#define ITIMER_TEST		BIT(24)		/* interrupt timer test mode */
+#define TEST_SEED_SEL		BIT(22)		/* test seed select */
+#define SEED_SEL		BIT(21)		/* seed select */
+#define TEST_MODE		BIT(20)		/* transmission test mode */
+#define TEST_TIME_MASK		0xffc00
+#define TEST_EXCEL_MASK		0x3e0
+	unsigned int tm;			/* test mode, 0xcc */
+
+	unsigned int reserved3;			/* 0xd0 */
+
+#define TX_MCOL_MASK		0xffff0000
+#define TX_MCOL_SHIFT_BIT	16
+#define TX_SCOL_MASK		0xffff
+#define TX_SCOL_SHIFT_BIT	0
+	/* TX_MCOL and TX_SCOL counter, 0xd4 */
+	unsigned int txmcol_xscol;
+
+#define RPF_MASK		0xffff0000
+#define RPF_SHIFT_BIT		16
+#define AEP_MASK		0xffff
+#define AEP_SHIFT_BIT		0
+	unsigned int rpf_aep;			/* RPF and AEP counter, 0xd8 */
+
+#define XM_MASK			0xffff0000
+#define XM_SHIFT_BIT		16
+#define PG_MASK			0xffff
+#define PG_SHIFT_BIT		0
+	unsigned int xm_pg;			/* XM and PG counter, 0xdc */
+
+#define RUNT_CNT_MASK		0xffff0000
+#define RUNT_CNT_SHIFT_BIT	16
+#define TLCC_MASK		0xffff
+#define TLCC_SHIFT_BIT		0
+	/* RUNT_CNT and TLCC counter, 0xe0 */
+	unsigned int runtcnt_tlcc;
+
+#define CRCER_CNT_MASK		0xffff0000
+#define CRCER_CNT_SHIFT_BIT	16
+#define FTL_CNT_MASK		0xffff
+#define FTL_CNT_SHIFT_BIT	0
+	/* CRCER_CNT and FTL_CNT counter, 0xe4 */
+	unsigned int crcercnt_ftlcnt;
+
+#define RLC_MASK		0xffff0000
+#define RLC_SHIFT_BIT		16
+#define RCC_MASK		0xffff
+#define RCC_SHIFT_BIT		0
+	unsigned int rlc_rcc;		/* RLC and RCC counter, 0xe8 */
+
+	unsigned int broc;		/* BROC counter, 0xec */
+	unsigned int mulca;		/* MULCA counter, 0xf0 */
+	unsigned int rp;		/* RP counter, 0xf4 */
+	unsigned int xp;		/* XP counter, 0xf8 */
+};
+
+#define ISR_REG_OFFSET			0x0
+#define IMR_REG_OFFSET			0x4
+#define MAC_MADR_REG_OFFSET		0x8
+#define MAC_LADR_REG_OFFSET		0xC
+#define MATH0_REG_OFFSET		0x10
+#define MATH1_REG_OFFSET		0x14
+#define TXPD_REG_OFFSET			0x18
+#define RXPD_REG_OFFSET			0x1C
+#define TXR_BADR_REG_OFFSET		0x20
+#define RXR_BADR_REG_OFFSET		0x24
+#define ITC_REG_OFFSET			0x28
+#define APTC_REG_OFFSET			0x2C
+#define DBLAC_REG_OFFSET		0x30
+#define MACCR_REG_OFFSET		0x88
+#define MACSR_REG_OFFSET		0x8C
+#define PHYCR_REG_OFFSET		0x90
+#define PHYWDATA_REG_OFFSET		0x94
+#define FCR_REG_OFFSET			0x98
+#define BPR_REG_OFFSET			0x9C
+#define TS_REG_OFFSET			0xC4
+#define DMAFIFOS_REG_OFFSET		0xC8
+#define TM_REG_OFFSET			0xCC
+#define TX_MCOL_TX_SCOL_REG_OFFSET	0xD4
+#define RPF_AEP_REG_OFFSET		0xD8
+#define XM_PG_REG_OFFSET		0xDC
+#define RUNT_CNT_TLCC_REG_OFFSET	0xE0
+#define CRCER_CNT_FTL_CNT_REG_OFFSET	0xE4
+#define RLC_RCC_REG_OFFSET		0xE8
+#define BROC_REG_OFFSET			0xEC
+#define MULCA_REG_OFFSET		0xF0
+#define RP_REG_OFFSET			0xF4
+#define XP_REG_OFFSET			0xF8
+#define PHY_CNTL_REG			0x0
+#define PHY_STATUS_REG			0x1
+#define PHY_ID_REG1			0x2
+#define PHY_ID_REG2			0x3
+#define PHY_ANA_REG			0x4
+#define PHY_ANLPAR_REG			0x5
+#define PHY_ANE_REG			0x6
+#define PHY_ECNTL_REG1			0x10
+#define PHY_QPDS_REG			0x11
+#define PHY_10BOP_REG			0x12
+#define PHY_ECNTL_REG2			0x13
+#define FTMAC100_REG_PHY_WRITE		0x8000000
+#define FTMAC100_REG_PHY_READ		0x4000000
+
+/* PHY Status register */
+#define AN_COMPLETE			0x20
+
+#define LINK_STATUS			0x4
+
+struct moxart_mac_priv_t {
+	void __iomem *base;
+	void __iomem *flash_base;
+	struct net_device_stats stats;
+	unsigned int reg_maccr;
+	unsigned int reg_imr;
+	struct napi_struct napi;
+	struct net_device *ndev;
+
+	dma_addr_t rx_base;
+	dma_addr_t rx_mapping[RX_DESC_NUM];
+	struct rx_desc_t *rx_desc_base;
+	unsigned char *rx_buf_base;
+	unsigned char *rx_buf[RX_DESC_NUM];
+	unsigned int rx_head;
+	unsigned int rx_buf_size;
+
+	dma_addr_t tx_base;
+	dma_addr_t tx_mapping[TX_DESC_NUM];
+	struct tx_desc_t *tx_desc_base;
+	unsigned char *tx_buf_base;
+	unsigned char *tx_buf[RX_DESC_NUM];
+	unsigned int tx_head;
+	unsigned int tx_buf_size;
+
+	spinlock_t txlock;
+	unsigned int tx_len[TX_DESC_NUM];
+	struct sk_buff *tx_skb[TX_DESC_NUM];
+	unsigned int tx_tail;
+};
+
+#if TX_BUF_SIZE >= TXBUF_SIZE_MAX
+#error MOXA ART Ethernet device driver Tx buffer size too large !
+#endif
+#if RX_BUF_SIZE >= RXBUF_SIZE_MAX
+#error MOXA ART Ethernet device driver Rx buffer size too large !
+#endif
+
+#endif
-- 
1.8.2.1


^ permalink raw reply related

* [PATCH net-next] ipv6: bump genid when delete/add address
From: Fan Du @ 2013-08-01  9:44 UTC (permalink / raw)
  To: davem, yoshfuji, jmorris, hannes, vyasevich; +Cc: netdev

     Server           Client
2001:1::803/64  <-> 2001:1::805/64
2001:2::804/64  <-> 2001:2::806/64

Server side fib binary tree looks like this:

                                   (2001:/64)
                                   /
                                  /
                   ffff88002103c380
                 /                 \
     (2)        /                   \
 (2001::803/128)                     ffff880037ac07c0
                                    /               \
                                   /                 \  (3)
                      ffff880037ac0640               (2001::806/128)
                       /             \
             (1)      /               \
        (2001::804/128)               (2001::805/128)

Delete 2001::804/64 won't cause prefix route deleted as well as rt in (3)
destinate to 2001::806 with source address as 2001::804/64. That's because
2001::803/64 is still alive, which make onlink=1 in ipv6_del_addr, this is
where the substantial difference between same prefix configuration and
different prefix configuration :) So packet are still transmitted out to
2001::806 with source address as 2001::804/64.

So bump genid will clear rt in (3), and up layer protocol will eventually
find the right one for themselves.

This problem arised from the discussion in here:
http://marc.info/?l=linux-netdev&m=137404469219410&w=4

Signed-off-by: Fan Du <fan.du@windriver.com>
---
 net/ipv6/addrconf.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index cfdcf7b..e7780d7 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -4653,6 +4653,7 @@ static void __ipv6_ifa_notify(int event, struct inet6_ifaddr *ifp)
 		break;
 	}
 	atomic_inc(&net->ipv6.dev_addr_genid);
+	rt_genid_bump_ipv6(net);
 }
 
 static void ipv6_ifa_notify(int event, struct inet6_ifaddr *ifp)
-- 
1.7.1

^ permalink raw reply related

* [PATCH net-next v3] bonding: fix system hang due to fast igmp timer rescheduling
From: Nikolay Aleksandrov @ 2013-08-01  9:51 UTC (permalink / raw)
  To: netdev; +Cc: davem, andy, fubar

From: "Nikolay Aleksandrov" <nikolay@redhat.com>

After commit 4aa5dee4d9 ("net: convert resend IGMP to notifier event")
we try to acquire rtnl in bond_resend_igmp_join_requests but it can be
scheduled with rtnl already held (e.g. when bond_change_active_slave is
called with rtnl) causing a loop of immediate reschedules + calls because
rtnl_trylock fails each time since it's being already held.
For me this issue leads to system hangs very easy:
modprobe bonding; ifconfig bond0 up; ifenslave bond0 eth0; rmmod
bonding;

The fix is to introduce a small (1 jiffy) delay which is enough for the
sections holding rtnl to finish without putting any strain on the system.
Also adjust the timer in bond_change_active_slave to be 1 jiffy, since
most of the time it's called with rtnl already held.

Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
v3: learn to spell my name :-)
v2: adjust bond_change_active_slave's mcast_work delay to 1 jiffy because
it's called with rtnl held most of the time as Jay suggested

 drivers/net/bonding/bond_main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index da3af63..bc3578e 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -723,7 +723,7 @@ static int bond_set_allmulti(struct bonding *bond, int inc)
 static void bond_resend_igmp_join_requests(struct bonding *bond)
 {
 	if (!rtnl_trylock()) {
-		queue_delayed_work(bond->wq, &bond->mcast_work, 0);
+		queue_delayed_work(bond->wq, &bond->mcast_work, 1);
 		return;
 	}
 	call_netdevice_notifiers(NETDEV_RESEND_IGMP, bond->dev);
@@ -1084,7 +1084,7 @@ void bond_change_active_slave(struct bonding *bond, struct slave *new_active)
 	    ((USES_PRIMARY(bond->params.mode) && new_active) ||
 	     bond->params.mode == BOND_MODE_ROUNDROBIN)) {
 		bond->igmp_retrans = bond->params.resend_igmp;
-		queue_delayed_work(bond->wq, &bond->mcast_work, 0);
+		queue_delayed_work(bond->wq, &bond->mcast_work, 1);
 	}
 }
 
-- 
1.8.1.4

^ permalink raw reply related

* Re: [PATCH RFC] xfrm{4,6}: only report errors back to local sockets if we don't cross address family
From: Steffen Klassert @ 2013-08-01 10:05 UTC (permalink / raw)
  To: Hannes Frederic Sowa; +Cc: netdev, vi0oss
In-Reply-To: <20130801081150.GF3688@order.stressinduktion.org>

On Thu, Aug 01, 2013 at 10:11:50AM +0200, Hannes Frederic Sowa wrote:
> 
> If you have not yet done so, I would try to find a solution over the weekend.
> 

I have not yet done so, please go ahead.

Thanks a lot!

^ permalink raw reply

* [PATCH net] xfrm: Delete hold_timer when destroy policy
From: Fan Du @ 2013-08-01 10:08 UTC (permalink / raw)
  To: steffen.klassert; +Cc: davem, netdev

Both policy timer and hold_timer need to be deleted when destroy policy
Bad mood today, maybe I'm wrong about this...

Signed-off-by: Fan Du <fan.du@windriver.com>
---
 net/xfrm/xfrm_policy.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index d8da6b8..f7078eb 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -308,7 +308,7 @@ void xfrm_policy_destroy(struct xfrm_policy *policy)
 {
 	BUG_ON(!policy->walk.dead);
 
-	if (del_timer(&policy->timer))
+	if (del_timer(&policy->timer) || del_timer(&policy->polq.hold_timer))
 		BUG();
 
 	security_xfrm_policy_free(policy->security);
-- 
1.7.1

^ permalink raw reply related

* Re: [PATCH v2] net/phy: micrel: Add OF configuration support
From: Sascha Hauer @ 2013-08-01 10:24 UTC (permalink / raw)
  To: Sean Cross
  Cc: Duan Fugang-B38611, netdev@vger.kernel.org,
	devicetree@vger.kernel.org, David Miller,
	stephen@networkplumber.org, Steven Rostedt
In-Reply-To: <3050A7D1D42E4A0CBC48C9620889E749@kosagi.com>

On Thu, Aug 01, 2013 at 05:06:13PM +0800, Sean Cross wrote:
> > Given that this patch adds a devicetree binding which I think we now
> > agree that this introduces an ABI I think this needs more thought. Some
> > questions:
> > 
> > - Does binding this also work for MII controllers external to the MAC?
> > Several Marvell SoCs have this situation. MDIO is a bus. With the
> > binding above you assume that all devices on the bus use the same
> > settings
> 
> I'm not quite sure what you mean here.  The board I'm testing on is
> based on an i.MX6q, which has a gigabit MAC and uses a Micrel PHY
> connected via MDIO.  I assumed (perhaps inaccurately) that phy_write()
> would pick the correct PHY.  The example binding is for a single
> Ethernet device, named "enet".  If a board had two Ethernet devices,
> enet1 and enet2, each could have its own micrel definitions.  For
> example:
> 
> &enet1 {
>     micrel,clk-control-pad-skew = <0xf0f0>;
>     micrel,rx-data-pad-skew = <0x0000>;
>     micrel,tx-data-pad-skew = <0xffff>;
>     status = "okay";
> };
> 
> &enet2 {
>     micrel,clk-control-pad-skew = <0x0000>;
>     micrel,rx-data-pad-skew = <0x0000>;
>     micrel,tx-data-pad-skew = <0x0000>;
>     status = "okay";
> };

Here each enet controller has its own mdio bus, but look for example at
Marvell armada-370-db.dts:

mdio {
	phy0: ethernet-phy@0 {
		reg = <0>;
	};

	phy1: ethernet-phy@1 {
		reg = <1>;
	};
};

ethernet@70000 {
	status = "okay";
	phy = <&phy0>;
	phy-mode = "rgmii-id";
};

ethernet@74000 {
	status = "okay";
	phy = <&phy1>;
	phy-mode = "rgmii-id";
};

Here two ethernet controllers share a single mdio bus. This at least
requires the binding in the phy node, not the ethernet node.

> > - You directly put the register contents into dt. This assumes that all
> > micrel phys have a compatible register layout. This may be the case
> > now, but what's with future phys?
> 
> This is a very valid point.  I assumed that most Micrel PHYs had
> similar register sets, but upon closer inspection it seems as though
> the KSZ9021RN is unique in this regard.  I should come up with a new
> function ksz9021_config_init that does this only for that one PHY, and
> rename the devicetree doc to reflect that.
> > - The pad skew settings are needed for other phys aswell. It might
> > be
> > worth introducing a binding which could work for say Artheros phys
> > aswell.
> 
> I can't comment on other phys, as I'm not familiar with them.  How
> would such a system work?  This certainly seems like the most
> open-ended of the questions.

I think we would need the delay for the different lines in ps, not in
hardware register specific values. This would have the additional
benefit that we don't have to hardcode the exact phy type.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply

* Re: [PATCH v2] net/phy: micrel: Add OF configuration support
From: Sean Cross @ 2013-08-01 10:33 UTC (permalink / raw)
  To: Sascha Hauer
  Cc: Duan Fugang-B38611, netdev@vger.kernel.org,
	devicetree@vger.kernel.org, David Miller,
	stephen@networkplumber.org, Steven Rostedt
In-Reply-To: <20130801102416.GF26614@pengutronix.de>

On Thursday, August 1, 2013 at 6:24 PM, Sascha Hauer wrote:
> On Thu, Aug 01, 2013 at 05:06:13PM +0800, Sean Cross wrote:
> > > Given that this patch adds a devicetree binding which I think we now
> > > agree that this introduces an ABI I think this needs more thought. Some
> > > questions:
> > > 
> > > - Does binding this also work for MII controllers external to the MAC?
> > > Several Marvell SoCs have this situation. MDIO is a bus. With the
> > > binding above you assume that all devices on the bus use the same
> > > settings
> > 
> > 
> > 
> > I'm not quite sure what you mean here. The board I'm testing on is
> > based on an i.MX6q, which has a gigabit MAC and uses a Micrel PHY
> > connected via MDIO. I assumed (perhaps inaccurately) that phy_write()
> > would pick the correct PHY. The example binding is for a single
> > Ethernet device, named "enet". If a board had two Ethernet devices,
> > enet1 and enet2, each could have its own micrel definitions. For
> > example:
> > 
> > &enet1 {
> > micrel,clk-control-pad-skew = <0xf0f0>;
> > micrel,rx-data-pad-skew = <0x0000>;
> > micrel,tx-data-pad-skew = <0xffff>;
> > status = "okay";
> > };
> > 
> > &enet2 {
> > micrel,clk-control-pad-skew = <0x0000>;
> > micrel,rx-data-pad-skew = <0x0000>;
> > micrel,tx-data-pad-skew = <0x0000>;
> > status = "okay";
> > };
> 
> 
> 
> Here each enet controller has its own mdio bus, but look for example at
> Marvell armada-370-db.dts:
> 
> mdio {
> phy0: ethernet-phy@0 {
> reg = <0>;
> };
> 
> phy1: ethernet-phy@1 {
> reg = <1>;
> };
> };
> 
> ethernet@70000 {
> status = "okay";
> phy = <&phy0>;
> phy-mode = "rgmii-id";
> };
> 
> ethernet@74000 {
> status = "okay";
> phy = <&phy1>;
> phy-mode = "rgmii-id";
> };
> 
> Here two ethernet controllers share a single mdio bus. This at least
> requires the binding in the phy node, not the ethernet node.

The code handles this case.  It will only read from the ethernet node if there is no phy node present:

    struct device_node *of_node = dev->of_node;
    if (!of_node && dev->parent->of_node)

        of_node = dev->parent->of_node;

I can add an example in the Documentation to reflect the fact that if a phy has a device tree node, you should put the skew values there.
> > > - The pad skew settings are needed for other phys aswell. It might
> > > be
> > > worth introducing a binding which could work for say Artheros phys
> > > aswell.
> > 
> > 
> > 
> > I can't comment on other phys, as I'm not familiar with them. How
> > would such a system work? This certainly seems like the most
> > open-ended of the questions.
> 
> 
> 
> I think we would need the delay for the different lines in ps, not in
> hardware register specific values. This would have the additional
> benefit that we don't have to hardcode the exact phy type.

Ah, I see.  Your proposal would be to break micrel,clk-control-pad-skew into four different values, and rename them to something like: 
rxc-pad-skew
rx-ctl-pad-skew
txc-pad-skew
tx-ctl-pad-skew

Then the device tree entries would look something like:

rxc-pad-skew = <3000>; // picoseconds
rx-ctl-pad-skew <0>; // picoseconds
txc-pad-skew = <3000>; // picoseconds
tx-ctl-pad-skew = <0>; // picoseconds



How would this affect the Atheros driver?  Are there other names that should be preferred?

^ permalink raw reply

* Re: [PATCH 1/3] af_packet: when sending ethernet frames, parse header for skb->protocol
From: Phil Sutter @ 2013-08-01 10:35 UTC (permalink / raw)
  To: David Miller; +Cc: netdev
In-Reply-To: <20130731.173017.624144247713296239.davem@davemloft.net>

Hi,

On Wed, Jul 31, 2013 at 05:30:17PM -0700, David Miller wrote:
> From: Phil Sutter <phil@nwl.cc>
> Date: Wed, 31 Jul 2013 19:27:45 +0200
> 
> > @@ -2324,6 +2328,13 @@ static int packet_snd(struct socket *sock,
> >  
> >  	sock_tx_timestamp(sk, &skb_shinfo(skb)->tx_flags);
> >  
> > +	if (dev->type == ARPHRD_ETHER) {
> > +		skb->protocol = eth_type_trans(skb, dev);
> > +	} else {
> > +		skb->protocol = proto;
> > +		skb->dev = dev;
> > +	}
> > +
> >  	if (!gso_type && (len > dev->mtu + reserve + extra_len)) {
> >  		/* Earlier code assumed this would be a VLAN pkt,
> >  		 * double-check this now that we have the actual
> > @@ -2338,8 +2349,6 @@ static int packet_snd(struct socket *sock,
> >  		}
> >  	}
> >  
> > -	skb->protocol = proto;
> > -	skb->dev = dev;
> >  	skb->priority = sk->sk_priority;
> >  	skb->mark = sk->sk_mark;
> 
> I don't see anything explaining why you are avoiding setting skb->dev when
> you use eth_type_trans() to set the skb->protocol field.

Simply because eth_type_trans() sets skb->dev already. You're right, I
probably should have mentioned this explicitly. At least my introductory
mail elaborated on this.

> Also, why isn't the user setting the protocol field correctly?  Isn't
> that the _real_ source of these problems?

Because the user specifies the value later used for skb->protocol at
either socket(), bind() or sendto() (i.e. flush) stage. Having both
"regular" and VLAN frames in the same TX ring is not possible at least.

> This patch series cannot be applied as is, it's either wrong, or
> explanations are missing from the commit messages.

Hmm. Is it OK to rely on eth_type_trans() setting skb->dev after all?
Depending on that, I would either add a comment or drop the change.

Best wishes, Phil

^ permalink raw reply

* [PATCH] macvlan: better mode validation
From: Michael S. Tsirkin @ 2013-08-01 10:43 UTC (permalink / raw)
  To: linux-kernel
  Cc: Patrick McHardy, David S. Miller, Eric Dumazet, Jiri Pirko,
	netdev

macvlan passthrough mode is special: it's not possible to switch to or
from it through a netlink command.

But if you try, the command will succeed, which is
confusing.

Validate input and return error to user.

Cc:  Sridhar Samudrala <sri@us.ibm.com>
Cc: "David S. Miller" <davem@davemloft.net>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/net/macvlan.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index 18373b6..13937f9 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -863,6 +863,18 @@ static int macvlan_changelink(struct net_device *dev,
 		struct nlattr *tb[], struct nlattr *data[])
 {
 	struct macvlan_dev *vlan = netdev_priv(dev);
+	enum macvlan_mode mode;
+	bool set_mode = false;
+
+	/* Validate mode, but don't set yet: setting flags may fail. */
+	if (data && data[IFLA_MACVLAN_MODE]) {
+		set_mode = true;
+		mode = nla_get_u32(data[IFLA_MACVLAN_MODE]);
+		/* Passthrough mode can't be set or cleared dynamically */
+		if ((mode == MACVLAN_MODE_PASSTHRU) !=
+		    (vlan->mode == MACVLAN_MODE_PASSTHRU))
+			return -EINVAL;
+	}
 
 	if (data && data[IFLA_MACVLAN_FLAGS]) {
 		__u16 flags = nla_get_u16(data[IFLA_MACVLAN_FLAGS]);
@@ -879,8 +891,8 @@ static int macvlan_changelink(struct net_device *dev,
 		}
 		vlan->flags = flags;
 	}
-	if (data && data[IFLA_MACVLAN_MODE])
-		vlan->mode = nla_get_u32(data[IFLA_MACVLAN_MODE]);
+	if (set_mode)
+		vlan->mode = mode;
 	return 0;
 }
 
-- 
MST

^ permalink raw reply related

* [PATCH] macvlan: validate flags
From: Michael S. Tsirkin @ 2013-08-01 10:48 UTC (permalink / raw)
  To: linux-kernel
  Cc: David S. Miller, Roopa Prabhu, John Fastabend, Patrick McHardy,
	netdev

commit df8ef8f3aaa6692970a436204c4429210addb23a
added a flags field to macvlan, which can be
controlled from userspace.
The idea is to make the interface future-proof
so we can add flags and not new fields.

However, flags value isn't validated, as a result,
userspace can't detect which flags are supported.

Cc: "David S. Miller" <davem@davemloft.net>
CC: Roopa Prabhu <roprabhu@cisco.com>
Cc: John Fastabend <john.r.fastabend@intel.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---

Please consider this patch for -stable.

The idea is by the time we add more flags,
everyone has updated to a kernel that
detects errors, so userspace will be able
to detect supported flags cleanly.


 drivers/net/macvlan.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index d0f9c2f..407b447 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -739,6 +739,10 @@ static int macvlan_validate(struct nlattr *tb[], struct nlattr *data[])
 			return -EADDRNOTAVAIL;
 	}
 
+	if (data && data[IFLA_MACVLAN_FLAGS] &&
+	    nla_get_u16(data[IFLA_MACVLAN_FLAGS]) & ~MACVLAN_FLAG_NOPROMISC)
+		return -EINVAL;
+
 	if (data && data[IFLA_MACVLAN_MODE]) {
 		switch (nla_get_u32(data[IFLA_MACVLAN_MODE])) {
 		case MACVLAN_MODE_PRIVATE:
@@ -812,6 +816,9 @@ int macvlan_common_newlink(struct net *src_net, struct net_device *dev,
 	if (data && data[IFLA_MACVLAN_FLAGS])
 		vlan->flags = nla_get_u16(data[IFLA_MACVLAN_FLAGS]);
 
+	if (vlan->flags & ~MACVLAN_FLAG_NOPROMISC)
+		return -EINVAL;
+
 	if (vlan->mode == MACVLAN_MODE_PASSTHRU) {
 		if (port->count)
 			return -EINVAL;
-- 
MST

^ permalink raw reply related

* [PATCH] macvlan: handle set_promiscuity failures
From: Michael S. Tsirkin @ 2013-08-01 10:50 UTC (permalink / raw)
  To: linux-kernel; +Cc: David S. Miller, Patrick McHardy, netdev

It's quite unlikely that dev_set_promiscuity will fail,
but worth checking just in case.

Cc: "David S. Miller" <davem@davemloft.net>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/net/macvlan.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index 13937f9..d0f9c2f 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -337,8 +337,11 @@ static int macvlan_open(struct net_device *dev)
 	int err;
 
 	if (vlan->port->passthru) {
-		if (!(vlan->flags & MACVLAN_FLAG_NOPROMISC))
-			dev_set_promiscuity(lowerdev, 1);
+		if (!(vlan->flags & MACVLAN_FLAG_NOPROMISC)) {
+			err = dev_set_promiscuity(lowerdev, 1);
+			if (err < 0)
+				goto out;
+		}
 		goto hash_add;
 	}
 
-- 
MST

^ permalink raw reply related

* [PATCH v2] sis900: Fix the tx queue timeout issue
From: Denis Kirjanov @ 2013-08-01  7:01 UTC (permalink / raw)
  To: davem, venza; +Cc: netdev, Denis Kirjanov

[  198.720048] ------------[ cut here ]------------
[  198.720108] WARNING: CPU: 0 PID: 0 at net/sched/sch_generic.c:255 dev_watchdog+0x229/0x240()
[  198.720118] NETDEV WATCHDOG: eth0 (sis900): transmit queue 0 timed out
[  198.720125] Modules linked in: bridge stp llc dmfe sundance 3c59x sis900 mii
[  198.720159] CPU: 0 PID: 0 Comm: swapper Not tainted 3.11.0-rc3+ #12
[  198.720167] Hardware name: System Manufacturer System Name/TUSI-M, BIOS ASUS TUSI-M ACPI BIOS
Revision 1013 Beta 001 12/14/2001
[  198.720175]  000000ff c13fa6b9 c169ddcc c12208d6 c169ddf8 c1031e4d c1664a84 c169de24
[  198.720197]  00000000 c165f5ea 000000ff c13fa6b9 00000001 000000ff c1664a84 c169de10
[  198.720217]  c1031f13 00000009 c169de08 c1664a84 c169de24 c169de50 c13fa6b9 c165f5ea
[  198.720240] Call Trace:
[  198.720257]  [<c13fa6b9>] ? dev_watchdog+0x229/0x240
[  198.720274]  [<c12208d6>] dump_stack+0x16/0x20
[  198.720306]  [<c1031e4d>] warn_slowpath_common+0x7d/0xa0
[  198.720318]  [<c13fa6b9>] ? dev_watchdog+0x229/0x240
[  198.720330]  [<c1031f13>] warn_slowpath_fmt+0x33/0x40
[  198.720342]  [<c13fa6b9>] dev_watchdog+0x229/0x240
[  198.720357]  [<c103f158>] call_timer_fn+0x78/0x150
[  198.720369]  [<c103f0e0>] ? internal_add_timer+0x40/0x40
[  198.720381]  [<c13fa490>] ? dev_init_scheduler+0xa0/0xa0
[  198.720392]  [<c103f33f>] run_timer_softirq+0x10f/0x200
[  198.720412]  [<c103954f>] ? __do_softirq+0x6f/0x210
[  198.720424]  [<c13fa490>] ? dev_init_scheduler+0xa0/0xa0
[  198.720435]  [<c1039598>] __do_softirq+0xb8/0x210
[  198.720467]  [<c14b54d2>] ? _raw_spin_unlock+0x22/0x30
[  198.720484]  [<c1003245>] ? handle_irq+0x25/0xd0
[  198.720496]  [<c1039c0c>] irq_exit+0x9c/0xb0
[  198.720508]  [<c14bc9d7>] do_IRQ+0x47/0x94
[  198.720534]  [<c1056078>] ? hrtimer_start+0x28/0x30
[  198.720564]  [<c14bc8b1>] common_interrupt+0x31/0x38
[  198.720589]  [<c1008692>] ? default_idle+0x22/0xa0
[  198.720600]  [<c10083c7>] arch_cpu_idle+0x17/0x30
[  198.720631]  [<c106d23d>] cpu_startup_entry+0xcd/0x180
[  198.720643]  [<c14ae30a>] rest_init+0xaa/0xb0
[  198.720654]  [<c14ae260>] ? reciprocal_value+0x50/0x50
[  198.720668]  [<c17044e0>] ? repair_env_string+0x60/0x60
[  198.720679]  [<c1704bda>] start_kernel+0x29a/0x350
[  198.720690]  [<c17044e0>] ? repair_env_string+0x60/0x60
[  198.720721]  [<c1704269>] i386_start_kernel+0x39/0xa0
[  198.720729] ---[ end trace 81e0a6266f5c73a8 ]---
[  198.720740] eth0: Transmit timeout, status 00000204 00000000

We are trying to initiate a frame transmission even
if we are not ready to do so due auto negotiation progress:

timer routine checks the link status and if it's up calls
netif_carrier_on() allowing upper layer to start the tx queue

Signed-off-by: Denis Kirjanov <kda@linux-powerpc.org>
---
v1->v2: use netdev_dbg() instead of printk()
---
 drivers/net/ethernet/sis/sis900.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/sis/sis900.c b/drivers/net/ethernet/sis/sis900.c
index eb4aea3..f1fe02b 100644
--- a/drivers/net/ethernet/sis/sis900.c
+++ b/drivers/net/ethernet/sis/sis900.c
@@ -1613,9 +1613,12 @@ sis900_start_xmit(struct sk_buff *skb, struct net_device *net_dev)
 	unsigned int  count_dirty_tx;
 
 	/* Don't transmit data before the complete of auto-negotiation */
-	if(!sis_priv->autong_complete){
-		netif_stop_queue(net_dev);
-		return NETDEV_TX_BUSY;
+	if (unlikely(!sis_priv->autong_complete)) {
+		if (netif_msg_tx_err(sis_priv) && net_ratelimit())
+			netdev_dbg(net_dev, "Auto negotiation in progress\n");
+		net_dev->stats.tx_dropped++;
+		dev_kfree_skb(skb);
+		return NETDEV_TX_OK;
 	}
 
 	spin_lock_irqsave(&sis_priv->lock, flags);
-- 
1.8.0.2

^ permalink raw reply related

* Re: locating the 'tc actions' hook
From: Jamal Hadi Salim @ 2013-08-01 11:40 UTC (permalink / raw)
  To: John Fastabend; +Cc: Stephen Hemminger, Eric Dumazet, Tom Herbert, netdev
In-Reply-To: <51F97F4E.5070102@gmail.com>

On 13-07-31 05:19 PM, John Fastabend wrote:
> Hi Jamal,
>
> I'm trying to sort out why I would use 'tc actions' and exactly
> what it is doing.
>
> Its clear I think adding one or more actions to filters will be
> used in the classifier via tcf_exts_exec() through the classify
> hook called from a qdisc.
>
> This is your standard
>
>      # tc filter add ... {u32|fw|tcindex|route|...} ... action ...
>
> When these actions get configured the specific actions tc_action_ops
> will be used to init the action create the hash 'tcf_hash_create' and
> parse the options. All this I can follow in ./net/sched/
>
> But the actions netlink hook does this,
>
> tc_ctl_action()
>      tcf_action_add()
>          tcf_action_init() <- inserts action in the table
>
> So at this point we have the entry in the table but I must be missing
> where the tc_action_ops act() is going to be called because its not via
> tcf_exts_exec().
>


There are two ways to add actions.
a) you specify a filter followed by a graph of actions to be executed
on a match (yes, in the kernel 10 years before Openflow and still
expressively more powerful;->)

b) You can add actions first, then later specify the filter followed by
a graph of actions. Actions in such a case are referenced by their
table indices.

#a tends to be more popular way of provisioning.

> Am I missing something obvious here? Is there a way to link them to
> filters? Sorry if it turns out to be a stupid question.
>

I think the second use case is what you are bumping into. I know from
answering questions this is a very popular use case in some eastern
European countries (where one policer with a specific rate is shared
by many flows); i think they have a setup where you share your DSL
connection with your neighbors. Its quiet a clever setup.


> My motivation here is to use the filters/actions outside the qdisc lock
> for mq, mqprio, and the ingress qdisc.
>

Are you trying to offload these actions into hardware?
Is the classifier in hardware?
Please let me know if you need further help. Example, I could send you
a bunch of examples for either

cheers,
jamal


> .John
>

^ permalink raw reply

* pull-request: can-next 2013-08-01
From: Marc Kleine-Budde @ 2013-08-01 12:00 UTC (permalink / raw)
  To: netdev; +Cc: linux-can@vger.kernel.org, David Miller, kernel@pengutronix.de

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

Hello David,

this is a pull-request for net-next/master. It consists of two patches
by Fabio Estevam. Them first convert the flexcan driver to use
devm_ioremap_resource(), the second adds return value checking for
clk_prepare_enable().

regards,
Marc

---

The following changes since commit 46b3a421903aa2a4bc9731ca4663cee3ea869dab:

  ipv6: fib6_rules should return exact return value (2013-08-01 00:26:22 -0700)

are available in the git repository at:

  git://gitorious.org/linux-can/linux-can-next.git for-davem

for you to fetch changes up to aa10181bdac99a623517f0f2bfa14cd98c749246:

  can: flexcan: Check the return value from clk_prepare_enable() (2013-08-01 12:36:20 +0200)

----------------------------------------------------------------
Fabio Estevam (2):
      can: flexcan: Use devm_ioremap_resource()
      can: flexcan: Check the return value from clk_prepare_enable()

 drivers/net/can/flexcan.c | 81 +++++++++++++++++++----------------------------
 1 file changed, 32 insertions(+), 49 deletions(-)

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]

^ permalink raw reply

* Re: [PATCH net] xfrm: Delete hold_timer when destroy policy
From: Steffen Klassert @ 2013-08-01 12:01 UTC (permalink / raw)
  To: Fan Du; +Cc: davem, netdev
In-Reply-To: <1375351716-3265-1-git-send-email-fan.du@windriver.com>

On Thu, Aug 01, 2013 at 06:08:36PM +0800, Fan Du wrote:
> Both policy timer and hold_timer need to be deleted when destroy policy
> Bad mood today, maybe I'm wrong about this...
> 
> Signed-off-by: Fan Du <fan.du@windriver.com>
> ---
>  net/xfrm/xfrm_policy.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
> index d8da6b8..f7078eb 100644
> --- a/net/xfrm/xfrm_policy.c
> +++ b/net/xfrm/xfrm_policy.c
> @@ -308,7 +308,7 @@ void xfrm_policy_destroy(struct xfrm_policy *policy)
>  {
>  	BUG_ON(!policy->walk.dead);
>  
> -	if (del_timer(&policy->timer))
> +	if (del_timer(&policy->timer) || del_timer(&policy->polq.hold_timer))
>  		BUG();
>  

The timers should be already deleted when xfrm_policy_destroy() is
called. This is just to check if that really happened and to
catch this bug if not. So it's not a bug fix, it just helps to
catch a potential bug. I'll consider to take this into ipsec-next
after some testing.

^ permalink raw reply

* Re: [PATCH 0/8] include/net: next set of extern removals
From: David Howells @ 2013-08-01 12:04 UTC (permalink / raw)
  To: Joe Perches
  Cc: dhowells, netdev, linux-hams, linux-wireless, linux-kernel,
	linux-afs
In-Reply-To: <cover.1375316912.git.joe@perches.com>

Joe Perches <joe@perches.com> wrote:

> Standardize on no extern use on function prototypes

Ugh.  Can we please standardise on _having_ externs on function prototypes?

David

^ permalink raw reply

* [PATCH ethtool]: Add strings to display Supported Ports for Backplane Ethernet
From: Somnath Kotur @ 2013-08-01 12:09 UTC (permalink / raw)
  To: bhutchings; +Cc: netdev, Somnath Kotur

Signed-off-by: Somnath Kotur <somnath.kotur@emulex.com>
---
 ethtool.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/ethtool.c b/ethtool.c
index 3544f0e..b24b572 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -485,6 +485,9 @@ static void dump_supported(struct ethtool_cmd *ep)
 		fprintf(stdout, "MII ");
 	if (mask & SUPPORTED_FIBRE)
 		fprintf(stdout, "FIBRE ");
+	if ((mask & SUPPORTED_10000baseKR_Full) ||
+	    (mask & SUPPORTED_10000baseKX4_Full))
+		fprintf(stdout, "BACKPLANE ");
 	fprintf(stdout, "]\n");
 
 	dump_link_caps("Supported", "Supports", mask, 0);
-- 
1.7.1

^ permalink raw reply related

* [PATCH net-next v2 0/5] bonding: groundwork and initial conversion to RCU
From: Nikolay Aleksandrov @ 2013-08-01 12:10 UTC (permalink / raw)
  To: netdev; +Cc: andy, davem, fubar

From: Nikolay Aleksandrov <Nikolay Aleksandrov nikolay@redhat.com>

Hello,
 This patchset aims to lay the groundwork, and do the initial conversion to
RCUism. I decided that it'll be much better to make the bonding RCU
conversion gradual, so patches can be reviewed and tested better rather
than having one huge patch (which I did in the beginning, before this).
The first patch is straightforward and it converts the bonding to the
standard list API, simplifying a lot of code, removing unnecessary local
variables and allowing to use the nice rculist API later. It also takes
care of some minor styling issues (re-arranging local variables longest ->
shortest, removing brackets for single statement if/else, leaving new line
before return statement etc.).
 The second patch simplifies the conversion by removing unnecessary
read_lock(&bond->curr_slave_lock) in xmit paths that are to be converted
later, because we only care if the pointer is NULL or a slave there, since
we already have bond->lock the slave can't go away.
 The third patch simplifies the broadcast xmit function by removing
the use of curr_active_slave and converting to standard list API. Also this
design of the broadcast xmit function avoids a subtle double packet tx race
when converted to RCU.
 The fourth patch factors out the code that transmits skb through a slave
with given id (i.e. rr_tx_counter in rr mode, hashed value in xor mode) and
simplifies the active-backup xmit path because bond_dev_queue_xmit always
consumes the skb. The new bond_xmit_slave_id function is used in rr and xor
modes currently, but the plans are to use it in 3ad mode as well thus it's
made global. I've left the function prototype to be 81 chars so I wouldn't
break it, if this is an issue I can always break it in more lines.
 The fifth patch introduces RCU by converting attach/detach and release to
RCU. It also converts dereferencing of curr_active_slave to rcu_dereference
although it's not fully converted to RCU, that is needed for the converted
xmit paths. And it converts roundrobin, broadcast, xor and active-backup
xmit paths to RCU. The 3ad and ALB/TLB modes acquire read_lock(&bond->lock)
to make sure that no slave will be removed and to sync properly with
enslave and release as before.
 This way for the price of a little complexity, we'll be able to convert
individual parts of the bonding to RCU, and test them easier in the
process. If this patchset is accepted in some form, I'll post followups
in the next weeks that gradually convert the bonding to RCU and remove the
need for the rwlocks.
 For performance notes please refer to patch 5 (RCU conversion one).

v2 summary:
 patch 1 - new primitives as suggested, removal of slave_cnt for checking
 if there're slaves in the bonding (using list_empty instead), using
 bond_for_each_slave and introducing bond_for_each_slave_continue_reverse
 for consistency, also making bond_next/prev_slave return NULL and
 simplifying further

 patch 5 - included the performance notes, converted curr_active_slave in
 bond_sysfs.c to rcu_dereference, fixed a potential latency issue in slave
 release because TX was unblocked after synchronize_rcu which might take a
 while

Best regards,
 Nikolay Aleksandrov

Nikolay Aleksandrov (5):
  bonding: convert to list API and replace bond's custom list
  bonding: remove unnecessary read_locks of curr_slave_lock
  bonding: simplify broadcast_xmit function
  bonding: factor out slave id tx code and simplify xmit paths
  bonding: initial RCU conversion

 drivers/net/bonding/bond_3ad.c    |  40 ++--
 drivers/net/bonding/bond_alb.c    |  57 +++--
 drivers/net/bonding/bond_main.c   | 433 +++++++++++++++-----------------------
 drivers/net/bonding/bond_procfs.c |  12 +-
 drivers/net/bonding/bond_sysfs.c  |  62 +++---
 drivers/net/bonding/bonding.h     |  85 +++++---
 6 files changed, 306 insertions(+), 383 deletions(-)

-- 
1.8.1.4

^ permalink raw reply

* [PATCH net-next v2 1/5] bonding: convert to list API and replace bond's custom list
From: Nikolay Aleksandrov @ 2013-08-01 12:10 UTC (permalink / raw)
  To: netdev; +Cc: andy, davem, fubar
In-Reply-To: <1375359029-25889-1-git-send-email-nikolay@redhat.com>

This patch aims to remove struct bonding's first_slave and struct
slave's next and prev pointers, and replace them with the standard Linux
list API. The old macros are converted to list API as well and some new
primitives are available now. The checks if there're slaves that used
slave_cnt have been replaced by the list_empty macro.
Also a few small style fixes, changing longest -> shortest line in local
variable declarations, leaving an empty line before return and removing
unnecessary brackets.
This is the first step to gradual RCU conversion.

Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
v2: introduce new primitives, keep using bond_for_each_slave, make
bond_first/last_slave return NULL on empty list, eliminate the use
of slave_cnt for checking if there're slaves in the bond and use
list_empty for that
Because of these changes one macro goes to 81 chars, but I think it would
look ugly if I break it

 drivers/net/bonding/bond_3ad.c    |  38 ++++---
 drivers/net/bonding/bond_alb.c    |  51 ++++-----
 drivers/net/bonding/bond_main.c   | 220 ++++++++++++++++----------------------
 drivers/net/bonding/bond_procfs.c |  12 +--
 drivers/net/bonding/bond_sysfs.c  |  43 ++++----
 drivers/net/bonding/bonding.h     |  71 ++++++------
 6 files changed, 199 insertions(+), 236 deletions(-)

diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index 390061d..b96f9da 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -143,10 +143,9 @@ static inline struct bonding *__get_bond_by_port(struct port *port)
  */
 static inline struct port *__get_first_port(struct bonding *bond)
 {
-	if (bond->slave_cnt == 0)
-		return NULL;
+	struct slave *first_slave = bond_first_slave(bond);
 
-	return &(SLAVE_AD_INFO(bond->first_slave).port);
+	return first_slave ? &(SLAVE_AD_INFO(first_slave).port) : NULL;
 }
 
 /**
@@ -159,13 +158,14 @@ static inline struct port *__get_first_port(struct bonding *bond)
 static inline struct port *__get_next_port(struct port *port)
 {
 	struct bonding *bond = __get_bond_by_port(port);
-	struct slave *slave = port->slave;
+	struct slave *slave = port->slave, *slave_next;
 
 	// If there's no bond for this port, or this is the last slave
-	if ((bond == NULL) || (slave->next == bond->first_slave))
+	if (bond == NULL)
 		return NULL;
+	slave_next = bond_next_slave(bond, slave);
 
-	return &(SLAVE_AD_INFO(slave->next).port);
+	return slave_next ? &(SLAVE_AD_INFO(slave_next).port) : NULL;
 }
 
 /**
@@ -178,12 +178,14 @@ static inline struct port *__get_next_port(struct port *port)
 static inline struct aggregator *__get_first_agg(struct port *port)
 {
 	struct bonding *bond = __get_bond_by_port(port);
+	struct slave *first_slave;
 
 	// If there's no bond for this port, or bond has no slaves
-	if ((bond == NULL) || (bond->slave_cnt == 0))
+	if (bond == NULL)
 		return NULL;
+	first_slave = bond_first_slave(bond);
 
-	return &(SLAVE_AD_INFO(bond->first_slave).aggregator);
+	return first_slave ? &(SLAVE_AD_INFO(first_slave).aggregator) : NULL;
 }
 
 /**
@@ -195,14 +197,15 @@ static inline struct aggregator *__get_first_agg(struct port *port)
  */
 static inline struct aggregator *__get_next_agg(struct aggregator *aggregator)
 {
-	struct slave *slave = aggregator->slave;
+	struct slave *slave = aggregator->slave, *slave_next;
 	struct bonding *bond = bond_get_bond_by_slave(slave);
 
 	// If there's no bond for this aggregator, or this is the last slave
-	if ((bond == NULL) || (slave->next == bond->first_slave))
+	if (bond == NULL)
 		return NULL;
+	slave_next = bond_next_slave(bond, slave);
 
-	return &(SLAVE_AD_INFO(slave->next).aggregator);
+	return slave_next ? &(SLAVE_AD_INFO(slave_next).aggregator) : NULL;
 }
 
 /*
@@ -2110,7 +2113,7 @@ void bond_3ad_state_machine_handler(struct work_struct *work)
 	read_lock(&bond->lock);
 
 	//check if there are any slaves
-	if (bond->slave_cnt == 0)
+	if (list_empty(&bond->slave_list))
 		goto re_arm;
 
 	// check if agg_select_timer timer after initialize is timed out
@@ -2336,8 +2339,12 @@ void bond_3ad_handle_link_change(struct slave *slave, char link)
 int bond_3ad_set_carrier(struct bonding *bond)
 {
 	struct aggregator *active;
+	struct slave *first_slave;
 
-	active = __get_active_agg(&(SLAVE_AD_INFO(bond->first_slave).aggregator));
+	first_slave = bond_first_slave(bond);
+	if (!first_slave)
+		return 0;
+	active = __get_active_agg(&(SLAVE_AD_INFO(first_slave).aggregator));
 	if (active) {
 		/* are enough slaves available to consider link up? */
 		if (active->num_of_ports < bond->params.min_links) {
@@ -2432,7 +2439,7 @@ int bond_3ad_xmit_xor(struct sk_buff *skb, struct net_device *dev)
 
 	slave_agg_no = bond->xmit_hash_policy(skb, slaves_in_agg);
 
-	bond_for_each_slave(bond, slave, i) {
+	bond_for_each_slave(bond, slave) {
 		struct aggregator *agg = SLAVE_AD_INFO(slave).port.aggregator;
 
 		if (agg && (agg->aggregator_identifier == agg_id)) {
@@ -2501,7 +2508,6 @@ int bond_3ad_lacpdu_recv(const struct sk_buff *skb, struct bonding *bond,
  */
 void bond_3ad_update_lacp_rate(struct bonding *bond)
 {
-	int i;
 	struct slave *slave;
 	struct port *port = NULL;
 	int lacp_fast;
@@ -2509,7 +2515,7 @@ void bond_3ad_update_lacp_rate(struct bonding *bond)
 	write_lock_bh(&bond->lock);
 	lacp_fast = bond->params.lacp_fast;
 
-	bond_for_each_slave(bond, slave, i) {
+	bond_for_each_slave(bond, slave) {
 		port = &(SLAVE_AD_INFO(slave).port);
 		if (port->slave == NULL)
 			continue;
diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index 4ea8ed1..4d35196 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -224,13 +224,12 @@ static struct slave *tlb_get_least_loaded_slave(struct bonding *bond)
 {
 	struct slave *slave, *least_loaded;
 	long long max_gap;
-	int i;
 
 	least_loaded = NULL;
 	max_gap = LLONG_MIN;
 
 	/* Find the slave with the largest gap */
-	bond_for_each_slave(bond, slave, i) {
+	bond_for_each_slave(bond, slave) {
 		if (SLAVE_IS_OK(slave)) {
 			long long gap = compute_gap(slave);
 
@@ -386,11 +385,10 @@ static struct slave *rlb_next_rx_slave(struct bonding *bond)
 	struct slave *rx_slave, *slave, *start_at;
 	int i = 0;
 
-	if (bond_info->next_rx_slave) {
+	if (bond_info->next_rx_slave)
 		start_at = bond_info->next_rx_slave;
-	} else {
-		start_at = bond->first_slave;
-	}
+	else
+		start_at = bond_first_slave(bond);
 
 	rx_slave = NULL;
 
@@ -405,7 +403,8 @@ static struct slave *rlb_next_rx_slave(struct bonding *bond)
 	}
 
 	if (rx_slave) {
-		bond_info->next_rx_slave = rx_slave->next;
+		slave = bond_next_slave(bond, rx_slave);
+		bond_info->next_rx_slave = slave;
 	}
 
 	return rx_slave;
@@ -1173,9 +1172,8 @@ static int alb_handle_addr_collision_on_attach(struct bonding *bond, struct slav
 {
 	struct slave *tmp_slave1, *free_mac_slave = NULL;
 	struct slave *has_bond_addr = bond->curr_active_slave;
-	int i;
 
-	if (bond->slave_cnt == 0) {
+	if (list_empty(&bond->slave_list)) {
 		/* this is the first slave */
 		return 0;
 	}
@@ -1196,7 +1194,7 @@ static int alb_handle_addr_collision_on_attach(struct bonding *bond, struct slav
 	/* The slave's address is equal to the address of the bond.
 	 * Search for a spare address in the bond for this slave.
 	 */
-	bond_for_each_slave(bond, tmp_slave1, i) {
+	bond_for_each_slave(bond, tmp_slave1) {
 		if (!bond_slave_has_mac(bond, tmp_slave1->perm_hwaddr)) {
 			/* no slave has tmp_slave1's perm addr
 			 * as its curr addr
@@ -1246,17 +1244,15 @@ static int alb_handle_addr_collision_on_attach(struct bonding *bond, struct slav
  */
 static int alb_set_mac_address(struct bonding *bond, void *addr)
 {
-	struct sockaddr sa;
-	struct slave *slave, *stop_at;
 	char tmp_addr[ETH_ALEN];
+	struct slave *slave;
+	struct sockaddr sa;
 	int res;
-	int i;
 
-	if (bond->alb_info.rlb_enabled) {
+	if (bond->alb_info.rlb_enabled)
 		return 0;
-	}
 
-	bond_for_each_slave(bond, slave, i) {
+	bond_for_each_slave(bond, slave) {
 		/* save net_device's current hw address */
 		memcpy(tmp_addr, slave->dev->dev_addr, ETH_ALEN);
 
@@ -1276,8 +1272,7 @@ unwind:
 	sa.sa_family = bond->dev->type;
 
 	/* unwind from head to the slave that failed */
-	stop_at = slave;
-	bond_for_each_slave_from_to(bond, slave, i, bond->first_slave, stop_at) {
+	bond_for_each_slave_continue_reverse(bond, slave) {
 		memcpy(tmp_addr, slave->dev->dev_addr, ETH_ALEN);
 		dev_set_mac_address(slave->dev, &sa);
 		memcpy(slave->dev->dev_addr, tmp_addr, ETH_ALEN);
@@ -1460,11 +1455,10 @@ void bond_alb_monitor(struct work_struct *work)
 					    alb_work.work);
 	struct alb_bond_info *bond_info = &(BOND_ALB_INFO(bond));
 	struct slave *slave;
-	int i;
 
 	read_lock(&bond->lock);
 
-	if (bond->slave_cnt == 0) {
+	if (list_empty(&bond->slave_list)) {
 		bond_info->tx_rebalance_counter = 0;
 		bond_info->lp_counter = 0;
 		goto re_arm;
@@ -1482,9 +1476,8 @@ void bond_alb_monitor(struct work_struct *work)
 		 */
 		read_lock(&bond->curr_slave_lock);
 
-		bond_for_each_slave(bond, slave, i) {
+		bond_for_each_slave(bond, slave)
 			alb_send_learning_packets(slave, slave->dev->dev_addr);
-		}
 
 		read_unlock(&bond->curr_slave_lock);
 
@@ -1496,7 +1489,7 @@ void bond_alb_monitor(struct work_struct *work)
 
 		read_lock(&bond->curr_slave_lock);
 
-		bond_for_each_slave(bond, slave, i) {
+		bond_for_each_slave(bond, slave) {
 			tlb_clear_slave(bond, slave, 1);
 			if (slave == bond->curr_active_slave) {
 				SLAVE_TLB_INFO(slave).load =
@@ -1602,9 +1595,8 @@ int bond_alb_init_slave(struct bonding *bond, struct slave *slave)
  */
 void bond_alb_deinit_slave(struct bonding *bond, struct slave *slave)
 {
-	if (bond->slave_cnt > 1) {
+	if (!list_empty(&bond->slave_list))
 		alb_change_hw_addr_on_detach(bond, slave);
-	}
 
 	tlb_clear_slave(bond, slave, 0);
 
@@ -1661,9 +1653,8 @@ void bond_alb_handle_active_change(struct bonding *bond, struct slave *new_slave
 {
 	struct slave *swap_slave;
 
-	if (bond->curr_active_slave == new_slave) {
+	if (bond->curr_active_slave == new_slave)
 		return;
-	}
 
 	if (bond->curr_active_slave && bond->alb_info.primary_is_promisc) {
 		dev_set_promiscuity(bond->curr_active_slave->dev, -1);
@@ -1674,9 +1665,8 @@ void bond_alb_handle_active_change(struct bonding *bond, struct slave *new_slave
 	swap_slave = bond->curr_active_slave;
 	bond->curr_active_slave = new_slave;
 
-	if (!new_slave || (bond->slave_cnt == 0)) {
+	if (!new_slave || list_empty(&bond->slave_list))
 		return;
-	}
 
 	/* set the new curr_active_slave to the bonds mac address
 	 * i.e. swap mac addresses of old curr_active_slave and new curr_active_slave
@@ -1689,9 +1679,8 @@ void bond_alb_handle_active_change(struct bonding *bond, struct slave *new_slave
 	 * ignored so we can mess with their MAC addresses without
 	 * fear of interference from transmit activity.
 	 */
-	if (swap_slave) {
+	if (swap_slave)
 		tlb_clear_slave(bond, swap_slave, 1);
-	}
 	tlb_clear_slave(bond, new_slave, 1);
 
 	write_unlock_bh(&bond->curr_slave_lock);
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index da3af63..3636a04 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -441,10 +441,10 @@ static int bond_vlan_rx_add_vid(struct net_device *bond_dev,
 				__be16 proto, u16 vid)
 {
 	struct bonding *bond = netdev_priv(bond_dev);
-	struct slave *slave, *stop_at;
-	int i, res;
+	struct slave *slave;
+	int res;
 
-	bond_for_each_slave(bond, slave, i) {
+	bond_for_each_slave(bond, slave) {
 		res = vlan_vid_add(slave->dev, proto, vid);
 		if (res)
 			goto unwind;
@@ -461,8 +461,7 @@ static int bond_vlan_rx_add_vid(struct net_device *bond_dev,
 
 unwind:
 	/* unwind from head to the slave that failed */
-	stop_at = slave;
-	bond_for_each_slave_from_to(bond, slave, i, bond->first_slave, stop_at)
+	bond_for_each_slave_continue_reverse(bond, slave)
 		vlan_vid_del(slave->dev, proto, vid);
 
 	return res;
@@ -478,9 +477,9 @@ static int bond_vlan_rx_kill_vid(struct net_device *bond_dev,
 {
 	struct bonding *bond = netdev_priv(bond_dev);
 	struct slave *slave;
-	int i, res;
+	int res;
 
-	bond_for_each_slave(bond, slave, i)
+	bond_for_each_slave(bond, slave)
 		vlan_vid_del(slave->dev, proto, vid);
 
 	res = bond_del_vlan(bond, vid);
@@ -532,15 +531,14 @@ static void bond_del_vlans_from_slave(struct bonding *bond,
 static int bond_set_carrier(struct bonding *bond)
 {
 	struct slave *slave;
-	int i;
 
-	if (bond->slave_cnt == 0)
+	if (list_empty(&bond->slave_list))
 		goto down;
 
 	if (bond->params.mode == BOND_MODE_8023AD)
 		return bond_3ad_set_carrier(bond);
 
-	bond_for_each_slave(bond, slave, i) {
+	bond_for_each_slave(bond, slave) {
 		if (slave->link == BOND_LINK_UP) {
 			if (!netif_carrier_ok(bond->dev)) {
 				netif_carrier_on(bond->dev);
@@ -681,8 +679,8 @@ static int bond_set_promiscuity(struct bonding *bond, int inc)
 		}
 	} else {
 		struct slave *slave;
-		int i;
-		bond_for_each_slave(bond, slave, i) {
+
+		bond_for_each_slave(bond, slave) {
 			err = dev_set_promiscuity(slave->dev, inc);
 			if (err)
 				return err;
@@ -705,8 +703,8 @@ static int bond_set_allmulti(struct bonding *bond, int inc)
 		}
 	} else {
 		struct slave *slave;
-		int i;
-		bond_for_each_slave(bond, slave, i) {
+
+		bond_for_each_slave(bond, slave) {
 			err = dev_set_allmulti(slave->dev, inc);
 			if (err)
 				return err;
@@ -935,9 +933,8 @@ static struct slave *bond_find_best_slave(struct bonding *bond)
 	new_active = bond->curr_active_slave;
 
 	if (!new_active) { /* there were no active slaves left */
-		if (bond->slave_cnt > 0)   /* found one slave */
-			new_active = bond->first_slave;
-		else
+		new_active = bond_first_slave(bond);
+		if (!new_active)
 			return NULL; /* still no slave, return NULL */
 	}
 
@@ -1130,17 +1127,7 @@ void bond_select_active_slave(struct bonding *bond)
  */
 static void bond_attach_slave(struct bonding *bond, struct slave *new_slave)
 {
-	if (bond->first_slave == NULL) { /* attaching the first slave */
-		new_slave->next = new_slave;
-		new_slave->prev = new_slave;
-		bond->first_slave = new_slave;
-	} else {
-		new_slave->next = bond->first_slave;
-		new_slave->prev = bond->first_slave->prev;
-		new_slave->next->prev = new_slave;
-		new_slave->prev->next = new_slave;
-	}
-
+	list_add_tail(&new_slave->list, &bond->slave_list);
 	bond->slave_cnt++;
 }
 
@@ -1156,22 +1143,7 @@ static void bond_attach_slave(struct bonding *bond, struct slave *new_slave)
  */
 static void bond_detach_slave(struct bonding *bond, struct slave *slave)
 {
-	if (slave->next)
-		slave->next->prev = slave->prev;
-
-	if (slave->prev)
-		slave->prev->next = slave->next;
-
-	if (bond->first_slave == slave) { /* slave is the first slave */
-		if (bond->slave_cnt > 1) { /* there are more slave */
-			bond->first_slave = slave->next;
-		} else {
-			bond->first_slave = NULL; /* slave was the last one */
-		}
-	}
-
-	slave->next = NULL;
-	slave->prev = NULL;
+	list_del(&slave->list);
 	bond->slave_cnt--;
 }
 
@@ -1222,9 +1194,8 @@ static void bond_netpoll_cleanup(struct net_device *bond_dev)
 {
 	struct bonding *bond = netdev_priv(bond_dev);
 	struct slave *slave;
-	int i;
 
-	bond_for_each_slave(bond, slave, i)
+	bond_for_each_slave(bond, slave)
 		if (IS_UP(slave->dev))
 			slave_disable_netpoll(slave);
 }
@@ -1233,9 +1204,9 @@ static int bond_netpoll_setup(struct net_device *dev, struct netpoll_info *ni, g
 {
 	struct bonding *bond = netdev_priv(dev);
 	struct slave *slave;
-	int i, err = 0;
+	int err = 0;
 
-	bond_for_each_slave(bond, slave, i) {
+	bond_for_each_slave(bond, slave) {
 		err = slave_enable_netpoll(slave);
 		if (err) {
 			bond_netpoll_cleanup(dev);
@@ -1265,11 +1236,10 @@ static netdev_features_t bond_fix_features(struct net_device *dev,
 	struct slave *slave;
 	struct bonding *bond = netdev_priv(dev);
 	netdev_features_t mask;
-	int i;
 
 	read_lock(&bond->lock);
 
-	if (!bond->first_slave) {
+	if (list_empty(&bond->slave_list)) {
 		/* Disable adding VLANs to empty bond. But why? --mq */
 		features |= NETIF_F_VLAN_CHALLENGED;
 		goto out;
@@ -1279,7 +1249,7 @@ static netdev_features_t bond_fix_features(struct net_device *dev,
 	features &= ~NETIF_F_ONE_FOR_ALL;
 	features |= NETIF_F_ALL_FOR_ALL;
 
-	bond_for_each_slave(bond, slave, i) {
+	bond_for_each_slave(bond, slave) {
 		features = netdev_increment_features(features,
 						     slave->dev->features,
 						     mask);
@@ -1303,15 +1273,14 @@ static void bond_compute_features(struct bonding *bond)
 	unsigned short max_hard_header_len = ETH_HLEN;
 	unsigned int gso_max_size = GSO_MAX_SIZE;
 	u16 gso_max_segs = GSO_MAX_SEGS;
-	int i;
 	unsigned int flags, dst_release_flag = IFF_XMIT_DST_RELEASE;
 
 	read_lock(&bond->lock);
 
-	if (!bond->first_slave)
+	if (list_empty(&bond->slave_list))
 		goto done;
 
-	bond_for_each_slave(bond, slave, i) {
+	bond_for_each_slave(bond, slave) {
 		vlan_features = netdev_increment_features(vlan_features,
 			slave->dev->vlan_features, BOND_VLAN_FEATURES);
 
@@ -1499,7 +1468,7 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
 	 * bond ether type mutual exclusion - don't allow slaves of dissimilar
 	 * ether type (eg ARPHRD_ETHER and ARPHRD_INFINIBAND) share the same bond
 	 */
-	if (bond->slave_cnt == 0) {
+	if (list_empty(&bond->slave_list)) {
 		if (bond_dev->type != slave_dev->type) {
 			pr_debug("%s: change device type from %d to %d\n",
 				 bond_dev->name,
@@ -1538,7 +1507,7 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
 	}
 
 	if (slave_ops->ndo_set_mac_address == NULL) {
-		if (bond->slave_cnt == 0) {
+		if (list_empty(&bond->slave_list)) {
 			pr_warning("%s: Warning: The first slave device specified does not support setting the MAC address. Setting fail_over_mac to active.",
 				   bond_dev->name);
 			bond->params.fail_over_mac = BOND_FOM_ACTIVE;
@@ -1554,7 +1523,8 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
 
 	/* If this is the first slave, then we need to set the master's hardware
 	 * address to be the same as the slave's. */
-	if (!bond->slave_cnt && bond->dev->addr_assign_type == NET_ADDR_RANDOM)
+	if (list_empty(&bond->slave_list) &&
+	    bond->dev->addr_assign_type == NET_ADDR_RANDOM)
 		bond_set_dev_addr(bond->dev, slave_dev);
 
 	new_slave = kzalloc(sizeof(struct slave), GFP_KERNEL);
@@ -1562,7 +1532,7 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
 		res = -ENOMEM;
 		goto err_undo_flags;
 	}
-
+	INIT_LIST_HEAD(&new_slave->list);
 	/*
 	 * Set the new_slave's queue_id to be zero.  Queue ID mapping
 	 * is set via sysfs or module option if desired.
@@ -1748,15 +1718,18 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
 		 */
 		bond_set_slave_inactive_flags(new_slave);
 		/* if this is the first slave */
-		if (bond->slave_cnt == 1) {
+		if (bond_first_slave(bond) == new_slave) {
 			SLAVE_AD_INFO(new_slave).id = 1;
 			/* Initialize AD with the number of times that the AD timer is called in 1 second
 			 * can be called only after the mac address of the bond is set
 			 */
 			bond_3ad_initialize(bond, 1000/AD_TIMER_INTERVAL);
 		} else {
+			struct slave *prev_slave;
+
+			prev_slave = bond_prev_slave(bond, new_slave);
 			SLAVE_AD_INFO(new_slave).id =
-				SLAVE_AD_INFO(new_slave->prev).id + 1;
+				SLAVE_AD_INFO(prev_slave).id + 1;
 		}
 
 		bond_3ad_bind_slave(new_slave);
@@ -1875,7 +1848,7 @@ err_free:
 err_undo_flags:
 	bond_compute_features(bond);
 	/* Enslave of first slave has failed and we need to fix master's mac */
-	if (bond->slave_cnt == 0 &&
+	if (list_empty(&bond->slave_list) &&
 	    ether_addr_equal(bond_dev->dev_addr, slave_dev->dev_addr))
 		eth_hw_addr_random(bond_dev);
 
@@ -1931,15 +1904,6 @@ static int __bond_release_one(struct net_device *bond_dev,
 	netdev_rx_handler_unregister(slave_dev);
 	write_lock_bh(&bond->lock);
 
-	if (!all && !bond->params.fail_over_mac) {
-		if (ether_addr_equal(bond_dev->dev_addr, slave->perm_hwaddr) &&
-		    bond->slave_cnt > 1)
-			pr_warning("%s: Warning: the permanent HWaddr of %s - %pM - is still in use by %s. Set the HWaddr of %s to a different address to avoid conflicts.\n",
-				   bond_dev->name, slave_dev->name,
-				   slave->perm_hwaddr,
-				   bond_dev->name, slave_dev->name);
-	}
-
 	/* Inform AD package of unbinding of slave. */
 	if (bond->params.mode == BOND_MODE_8023AD) {
 		/* must be called before the slave is
@@ -1960,6 +1924,15 @@ static int __bond_release_one(struct net_device *bond_dev,
 	/* release the slave from its bond */
 	bond_detach_slave(bond, slave);
 
+	if (!all && !bond->params.fail_over_mac) {
+		if (ether_addr_equal(bond_dev->dev_addr, slave->perm_hwaddr) &&
+		    !list_empty(&bond->slave_list))
+			pr_warn("%s: Warning: the permanent HWaddr of %s - %pM - is still in use by %s. Set the HWaddr of %s to a different address to avoid conflicts.\n",
+				   bond_dev->name, slave_dev->name,
+				   slave->perm_hwaddr,
+				   bond_dev->name, slave_dev->name);
+	}
+
 	if (bond->primary_slave == slave)
 		bond->primary_slave = NULL;
 
@@ -1996,7 +1969,7 @@ static int __bond_release_one(struct net_device *bond_dev,
 		write_lock_bh(&bond->lock);
 	}
 
-	if (bond->slave_cnt == 0) {
+	if (list_empty(&bond->slave_list)) {
 		bond_set_carrier(bond);
 		eth_hw_addr_random(bond_dev);
 
@@ -2011,7 +1984,7 @@ static int __bond_release_one(struct net_device *bond_dev,
 	write_unlock_bh(&bond->lock);
 	unblock_netpoll_tx();
 
-	if (bond->slave_cnt == 0) {
+	if (list_empty(&bond->slave_list)) {
 		call_netdevice_notifiers(NETDEV_CHANGEADDR, bond->dev);
 		call_netdevice_notifiers(NETDEV_RELEASE, bond->dev);
 	}
@@ -2082,7 +2055,7 @@ static int  bond_release_and_destroy(struct net_device *bond_dev,
 	int ret;
 
 	ret = bond_release(bond_dev, slave_dev);
-	if ((ret == 0) && (bond->slave_cnt == 0)) {
+	if (ret == 0 && list_empty(&bond->slave_list)) {
 		bond_dev->priv_flags |= IFF_DISABLE_NETPOLL;
 		pr_info("%s: destroying bond %s.\n",
 			bond_dev->name, bond_dev->name);
@@ -2167,13 +2140,12 @@ static int bond_info_query(struct net_device *bond_dev, struct ifbond *info)
 static int bond_slave_info_query(struct net_device *bond_dev, struct ifslave *info)
 {
 	struct bonding *bond = netdev_priv(bond_dev);
+	int i = 0, res = -ENODEV;
 	struct slave *slave;
-	int i, res = -ENODEV;
 
 	read_lock(&bond->lock);
-
-	bond_for_each_slave(bond, slave, i) {
-		if (i == (int)info->slave_id) {
+	bond_for_each_slave(bond, slave) {
+		if (i++ == (int)info->slave_id) {
 			res = 0;
 			strcpy(info->slave_name, slave->dev->name);
 			info->link = slave->link;
@@ -2182,7 +2154,6 @@ static int bond_slave_info_query(struct net_device *bond_dev, struct ifslave *in
 			break;
 		}
 	}
-
 	read_unlock(&bond->lock);
 
 	return res;
@@ -2193,13 +2164,13 @@ static int bond_slave_info_query(struct net_device *bond_dev, struct ifslave *in
 
 static int bond_miimon_inspect(struct bonding *bond)
 {
+	int link_state, commit = 0;
 	struct slave *slave;
-	int i, link_state, commit = 0;
 	bool ignore_updelay;
 
 	ignore_updelay = !bond->curr_active_slave ? true : false;
 
-	bond_for_each_slave(bond, slave, i) {
+	bond_for_each_slave(bond, slave) {
 		slave->new_link = BOND_LINK_NOCHANGE;
 
 		link_state = bond_check_dev_link(bond, slave->dev, 0);
@@ -2294,9 +2265,8 @@ static int bond_miimon_inspect(struct bonding *bond)
 static void bond_miimon_commit(struct bonding *bond)
 {
 	struct slave *slave;
-	int i;
 
-	bond_for_each_slave(bond, slave, i) {
+	bond_for_each_slave(bond, slave) {
 		switch (slave->new_link) {
 		case BOND_LINK_NOCHANGE:
 			continue;
@@ -2401,7 +2371,7 @@ void bond_mii_monitor(struct work_struct *work)
 
 	delay = msecs_to_jiffies(bond->params.miimon);
 
-	if (bond->slave_cnt == 0)
+	if (list_empty(&bond->slave_list))
 		goto re_arm;
 
 	should_notify_peers = bond_should_notify_peers(bond);
@@ -2681,14 +2651,13 @@ void bond_loadbalance_arp_mon(struct work_struct *work)
 	struct slave *slave, *oldcurrent;
 	int do_failover = 0;
 	int delta_in_ticks, extra_ticks;
-	int i;
 
 	read_lock(&bond->lock);
 
 	delta_in_ticks = msecs_to_jiffies(bond->params.arp_interval);
 	extra_ticks = delta_in_ticks / 2;
 
-	if (bond->slave_cnt == 0)
+	if (list_empty(&bond->slave_list))
 		goto re_arm;
 
 	read_lock(&bond->curr_slave_lock);
@@ -2703,7 +2672,7 @@ void bond_loadbalance_arp_mon(struct work_struct *work)
 	 * TODO: what about up/down delay in arp mode? it wasn't here before
 	 *       so it can wait
 	 */
-	bond_for_each_slave(bond, slave, i) {
+	bond_for_each_slave(bond, slave) {
 		unsigned long trans_start = dev_trans_start(slave->dev);
 
 		if (slave->link != BOND_LINK_UP) {
@@ -2800,10 +2769,10 @@ re_arm:
  */
 static int bond_ab_arp_inspect(struct bonding *bond, int delta_in_ticks)
 {
-	struct slave *slave;
-	int i, commit = 0;
 	unsigned long trans_start;
+	struct slave *slave;
 	int extra_ticks;
+	int commit = 0;
 
 	/* All the time comparisons below need some extra time. Otherwise, on
 	 * fast networks the ARP probe/reply may arrive within the same jiffy
@@ -2812,7 +2781,7 @@ static int bond_ab_arp_inspect(struct bonding *bond, int delta_in_ticks)
 	 */
 	extra_ticks = delta_in_ticks / 2;
 
-	bond_for_each_slave(bond, slave, i) {
+	bond_for_each_slave(bond, slave) {
 		slave->new_link = BOND_LINK_NOCHANGE;
 
 		if (slave->link != BOND_LINK_UP) {
@@ -2891,11 +2860,10 @@ static int bond_ab_arp_inspect(struct bonding *bond, int delta_in_ticks)
  */
 static void bond_ab_arp_commit(struct bonding *bond, int delta_in_ticks)
 {
-	struct slave *slave;
-	int i;
 	unsigned long trans_start;
+	struct slave *slave;
 
-	bond_for_each_slave(bond, slave, i) {
+	bond_for_each_slave(bond, slave) {
 		switch (slave->new_link) {
 		case BOND_LINK_NOCHANGE:
 			continue;
@@ -2968,7 +2936,7 @@ do_failover:
  */
 static void bond_ab_arp_probe(struct bonding *bond)
 {
-	struct slave *slave;
+	struct slave *slave, *next_slave;
 	int i;
 
 	read_lock(&bond->curr_slave_lock);
@@ -2992,7 +2960,7 @@ static void bond_ab_arp_probe(struct bonding *bond)
 	 */
 
 	if (!bond->current_arp_slave) {
-		bond->current_arp_slave = bond->first_slave;
+		bond->current_arp_slave = bond_first_slave(bond);
 		if (!bond->current_arp_slave)
 			return;
 	}
@@ -3000,7 +2968,8 @@ static void bond_ab_arp_probe(struct bonding *bond)
 	bond_set_slave_inactive_flags(bond->current_arp_slave);
 
 	/* search for next candidate */
-	bond_for_each_slave_from(bond, slave, i, bond->current_arp_slave->next) {
+	next_slave = bond_next_slave(bond, bond->current_arp_slave);
+	bond_for_each_slave_from(bond, slave, i, next_slave) {
 		if (IS_UP(slave->dev)) {
 			slave->link = BOND_LINK_BACK;
 			bond_set_slave_active_flags(slave);
@@ -3041,7 +3010,7 @@ void bond_activebackup_arp_mon(struct work_struct *work)
 
 	delta_in_ticks = msecs_to_jiffies(bond->params.arp_interval);
 
-	if (bond->slave_cnt == 0)
+	if (list_empty(&bond->slave_list))
 		goto re_arm;
 
 	should_notify_peers = bond_should_notify_peers(bond);
@@ -3361,13 +3330,12 @@ static int bond_open(struct net_device *bond_dev)
 {
 	struct bonding *bond = netdev_priv(bond_dev);
 	struct slave *slave;
-	int i;
 
 	/* reset slave->backup and slave->inactive */
 	read_lock(&bond->lock);
-	if (bond->slave_cnt > 0) {
+	if (!list_empty(&bond->slave_list)) {
 		read_lock(&bond->curr_slave_lock);
-		bond_for_each_slave(bond, slave, i) {
+		bond_for_each_slave(bond, slave) {
 			if ((bond->params.mode == BOND_MODE_ACTIVEBACKUP)
 				&& (slave != bond->curr_active_slave)) {
 				bond_set_slave_inactive_flags(slave);
@@ -3435,13 +3403,11 @@ static struct rtnl_link_stats64 *bond_get_stats(struct net_device *bond_dev,
 	struct bonding *bond = netdev_priv(bond_dev);
 	struct rtnl_link_stats64 temp;
 	struct slave *slave;
-	int i;
 
 	memset(stats, 0, sizeof(*stats));
 
 	read_lock_bh(&bond->lock);
-
-	bond_for_each_slave(bond, slave, i) {
+	bond_for_each_slave(bond, slave) {
 		const struct rtnl_link_stats64 *sstats =
 			dev_get_stats(slave->dev, &temp);
 
@@ -3471,7 +3437,6 @@ static struct rtnl_link_stats64 *bond_get_stats(struct net_device *bond_dev,
 		stats->tx_heartbeat_errors += sstats->tx_heartbeat_errors;
 		stats->tx_window_errors += sstats->tx_window_errors;
 	}
-
 	read_unlock_bh(&bond->lock);
 
 	return stats;
@@ -3610,7 +3575,6 @@ static void bond_set_rx_mode(struct net_device *bond_dev)
 {
 	struct bonding *bond = netdev_priv(bond_dev);
 	struct slave *slave;
-	int i;
 
 	read_lock(&bond->lock);
 
@@ -3623,7 +3587,7 @@ static void bond_set_rx_mode(struct net_device *bond_dev)
 		}
 		read_unlock(&bond->curr_slave_lock);
 	} else {
-		bond_for_each_slave(bond, slave, i) {
+		bond_for_each_slave(bond, slave) {
 			dev_uc_sync_multiple(slave->dev, bond_dev);
 			dev_mc_sync_multiple(slave->dev, bond_dev);
 		}
@@ -3635,16 +3599,15 @@ static void bond_set_rx_mode(struct net_device *bond_dev)
 static int bond_neigh_init(struct neighbour *n)
 {
 	struct bonding *bond = netdev_priv(n->dev);
-	struct slave *slave = bond->first_slave;
 	const struct net_device_ops *slave_ops;
 	struct neigh_parms parms;
+	struct slave *slave;
 	int ret;
 
+	slave = bond_first_slave(bond);
 	if (!slave)
 		return 0;
-
 	slave_ops = slave->dev->netdev_ops;
-
 	if (!slave_ops->ndo_neigh_setup)
 		return 0;
 
@@ -3687,9 +3650,8 @@ static int bond_neigh_setup(struct net_device *dev,
 static int bond_change_mtu(struct net_device *bond_dev, int new_mtu)
 {
 	struct bonding *bond = netdev_priv(bond_dev);
-	struct slave *slave, *stop_at;
+	struct slave *slave;
 	int res = 0;
-	int i;
 
 	pr_debug("bond=%p, name=%s, new_mtu=%d\n", bond,
 		 (bond_dev ? bond_dev->name : "None"), new_mtu);
@@ -3709,10 +3671,10 @@ static int bond_change_mtu(struct net_device *bond_dev, int new_mtu)
 	 * call to the base driver.
 	 */
 
-	bond_for_each_slave(bond, slave, i) {
+	bond_for_each_slave(bond, slave) {
 		pr_debug("s %p s->p %p c_m %p\n",
 			 slave,
-			 slave->prev,
+			 bond_prev_slave(bond, slave),
 			 slave->dev->netdev_ops->ndo_change_mtu);
 
 		res = dev_set_mtu(slave->dev, new_mtu);
@@ -3737,8 +3699,7 @@ static int bond_change_mtu(struct net_device *bond_dev, int new_mtu)
 
 unwind:
 	/* unwind from head to the slave that failed */
-	stop_at = slave;
-	bond_for_each_slave_from_to(bond, slave, i, bond->first_slave, stop_at) {
+	bond_for_each_slave_continue_reverse(bond, slave) {
 		int tmp_res;
 
 		tmp_res = dev_set_mtu(slave->dev, bond_dev->mtu);
@@ -3762,9 +3723,8 @@ static int bond_set_mac_address(struct net_device *bond_dev, void *addr)
 {
 	struct bonding *bond = netdev_priv(bond_dev);
 	struct sockaddr *sa = addr, tmp_sa;
-	struct slave *slave, *stop_at;
+	struct slave *slave;
 	int res = 0;
-	int i;
 
 	if (bond->params.mode == BOND_MODE_ALB)
 		return bond_alb_set_mac_address(bond_dev, addr);
@@ -3797,7 +3757,7 @@ static int bond_set_mac_address(struct net_device *bond_dev, void *addr)
 	 * call to the base driver.
 	 */
 
-	bond_for_each_slave(bond, slave, i) {
+	bond_for_each_slave(bond, slave) {
 		const struct net_device_ops *slave_ops = slave->dev->netdev_ops;
 		pr_debug("slave %p %s\n", slave, slave->dev->name);
 
@@ -3829,8 +3789,7 @@ unwind:
 	tmp_sa.sa_family = bond_dev->type;
 
 	/* unwind from head to the slave that failed */
-	stop_at = slave;
-	bond_for_each_slave_from_to(bond, slave, i, bond->first_slave, stop_at) {
+	bond_for_each_slave_continue_reverse(bond, slave) {
 		int tmp_res;
 
 		tmp_res = dev_set_mac_address(slave->dev, &tmp_sa);
@@ -3874,7 +3833,7 @@ static int bond_xmit_roundrobin(struct sk_buff *skb, struct net_device *bond_dev
 		 */
 		slave_no = bond->rr_tx_counter++ % bond->slave_cnt;
 
-		bond_for_each_slave(bond, slave, i) {
+		bond_for_each_slave(bond, slave) {
 			slave_no--;
 			if (slave_no < 0)
 				break;
@@ -3940,7 +3899,7 @@ static int bond_xmit_xor(struct sk_buff *skb, struct net_device *bond_dev)
 
 	slave_no = bond->xmit_hash_policy(skb, bond->slave_cnt);
 
-	bond_for_each_slave(bond, slave, i) {
+	bond_for_each_slave(bond, slave) {
 		slave_no--;
 		if (slave_no < 0)
 			break;
@@ -4041,15 +4000,15 @@ static void bond_set_xmit_hash_policy(struct bonding *bond)
 static inline int bond_slave_override(struct bonding *bond,
 				      struct sk_buff *skb)
 {
-	int i, res = 1;
 	struct slave *slave = NULL;
 	struct slave *check_slave;
+	int res = 1;
 
 	if (!skb->queue_mapping)
 		return 1;
 
 	/* Find out if any slaves have the same mapping as this skb. */
-	bond_for_each_slave(bond, check_slave, i) {
+	bond_for_each_slave(bond, check_slave) {
 		if (check_slave->queue_id == skb->queue_mapping) {
 			slave = check_slave;
 			break;
@@ -4136,7 +4095,7 @@ static netdev_tx_t bond_start_xmit(struct sk_buff *skb, struct net_device *dev)
 
 	read_lock(&bond->lock);
 
-	if (bond->slave_cnt)
+	if (!list_empty(&bond->slave_list))
 		ret = __bond_start_xmit(skb, dev);
 	else
 		kfree_skb(skb);
@@ -4182,9 +4141,8 @@ static int bond_ethtool_get_settings(struct net_device *bond_dev,
 				     struct ethtool_cmd *ecmd)
 {
 	struct bonding *bond = netdev_priv(bond_dev);
-	struct slave *slave;
-	int i;
 	unsigned long speed = 0;
+	struct slave *slave;
 
 	ecmd->duplex = DUPLEX_UNKNOWN;
 	ecmd->port = PORT_OTHER;
@@ -4195,7 +4153,7 @@ static int bond_ethtool_get_settings(struct net_device *bond_dev,
 	 * this is an accurate maximum.
 	 */
 	read_lock(&bond->lock);
-	bond_for_each_slave(bond, slave, i) {
+	bond_for_each_slave(bond, slave) {
 		if (SLAVE_IS_OK(slave)) {
 			if (slave->speed != SPEED_UNKNOWN)
 				speed += slave->speed;
@@ -4206,6 +4164,7 @@ static int bond_ethtool_get_settings(struct net_device *bond_dev,
 	}
 	ethtool_cmd_speed_set(ecmd, speed ? : SPEED_UNKNOWN);
 	read_unlock(&bond->lock);
+
 	return 0;
 }
 
@@ -4269,7 +4228,7 @@ static void bond_setup(struct net_device *bond_dev)
 	/* initialize rwlocks */
 	rwlock_init(&bond->lock);
 	rwlock_init(&bond->curr_slave_lock);
-
+	INIT_LIST_HEAD(&bond->slave_list);
 	bond->params = bonding_defaults;
 
 	/* Initialize pointers */
@@ -4326,13 +4285,14 @@ static void bond_setup(struct net_device *bond_dev)
 static void bond_uninit(struct net_device *bond_dev)
 {
 	struct bonding *bond = netdev_priv(bond_dev);
+	struct slave *slave, *tmp_slave;
 	struct vlan_entry *vlan, *tmp;
 
 	bond_netpoll_cleanup(bond_dev);
 
 	/* Release the bonded slaves */
-	while (bond->first_slave != NULL)
-		__bond_release_one(bond_dev, bond->first_slave->dev, true);
+	list_for_each_entry_safe(slave, tmp_slave, &bond->slave_list, list)
+		__bond_release_one(bond_dev, slave->dev, true);
 	pr_info("%s: released all slaves\n", bond_dev->name);
 
 	list_del(&bond->bond_list);
diff --git a/drivers/net/bonding/bond_procfs.c b/drivers/net/bonding/bond_procfs.c
index 4060d41..20a6ee2 100644
--- a/drivers/net/bonding/bond_procfs.c
+++ b/drivers/net/bonding/bond_procfs.c
@@ -12,7 +12,6 @@ static void *bond_info_seq_start(struct seq_file *seq, loff_t *pos)
 	struct bonding *bond = seq->private;
 	loff_t off = 0;
 	struct slave *slave;
-	int i;
 
 	/* make sure the bond won't be taken away */
 	rcu_read_lock();
@@ -21,10 +20,9 @@ static void *bond_info_seq_start(struct seq_file *seq, loff_t *pos)
 	if (*pos == 0)
 		return SEQ_START_TOKEN;
 
-	bond_for_each_slave(bond, slave, i) {
+	bond_for_each_slave(bond, slave)
 		if (++off == *pos)
 			return slave;
-	}
 
 	return NULL;
 }
@@ -36,11 +34,13 @@ static void *bond_info_seq_next(struct seq_file *seq, void *v, loff_t *pos)
 
 	++*pos;
 	if (v == SEQ_START_TOKEN)
-		return bond->first_slave;
+		return bond_first_slave(bond);
 
-	slave = slave->next;
+	if (bond_is_last_slave(bond, slave))
+		return NULL;
+	slave = bond_next_slave(bond, slave);
 
-	return (slave == bond->first_slave) ? NULL : slave;
+	return slave;
 }
 
 static void bond_info_seq_stop(struct seq_file *seq, void *v)
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index ae02c19..0702e91 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -209,12 +209,12 @@ void bond_destroy_slave_symlinks(struct net_device *master,
 static ssize_t bonding_show_slaves(struct device *d,
 				   struct device_attribute *attr, char *buf)
 {
-	struct slave *slave;
-	int i, res = 0;
 	struct bonding *bond = to_bond(d);
+	struct slave *slave;
+	int res = 0;
 
 	read_lock(&bond->lock);
-	bond_for_each_slave(bond, slave, i) {
+	bond_for_each_slave(bond, slave) {
 		if (res > (PAGE_SIZE - IFNAMSIZ)) {
 			/* not enough space for another interface name */
 			if ((PAGE_SIZE - res) > 10)
@@ -227,6 +227,7 @@ static ssize_t bonding_show_slaves(struct device *d,
 	read_unlock(&bond->lock);
 	if (res)
 		buf[res-1] = '\n'; /* eat the leftover space */
+
 	return res;
 }
 
@@ -325,7 +326,7 @@ static ssize_t bonding_store_mode(struct device *d,
 		goto out;
 	}
 
-	if (bond->slave_cnt > 0) {
+	if (!list_empty(&bond->slave_list)) {
 		pr_err("unable to update mode of %s because it has slaves.\n",
 			bond->dev->name);
 		ret = -EPERM;
@@ -507,7 +508,7 @@ static ssize_t bonding_store_fail_over_mac(struct device *d,
 	if (!rtnl_trylock())
 		return restart_syscall();
 
-	if (bond->slave_cnt != 0) {
+	if (!list_empty(&bond->slave_list)) {
 		pr_err("%s: Can't alter fail_over_mac with slaves in bond.\n",
 		       bond->dev->name);
 		ret = -EPERM;
@@ -668,7 +669,7 @@ static ssize_t bonding_store_arp_targets(struct device *d,
 			 &newtarget);
 		/* not to race with bond_arp_rcv */
 		write_lock_bh(&bond->lock);
-		bond_for_each_slave(bond, slave, i)
+		bond_for_each_slave(bond, slave)
 			slave->target_last_arp_rx[ind] = jiffies;
 		targets[ind] = newtarget;
 		write_unlock_bh(&bond->lock);
@@ -694,7 +695,7 @@ static ssize_t bonding_store_arp_targets(struct device *d,
 			&newtarget);
 
 		write_lock_bh(&bond->lock);
-		bond_for_each_slave(bond, slave, i) {
+		bond_for_each_slave(bond, slave) {
 			targets_rx = slave->target_last_arp_rx;
 			j = ind;
 			for (; (j < BOND_MAX_ARP_TARGETS-1) && targets[j+1]; j++)
@@ -1085,10 +1086,9 @@ static ssize_t bonding_store_primary(struct device *d,
 				     struct device_attribute *attr,
 				     const char *buf, size_t count)
 {
-	int i;
-	struct slave *slave;
 	struct bonding *bond = to_bond(d);
 	char ifname[IFNAMSIZ];
+	struct slave *slave;
 
 	if (!rtnl_trylock())
 		return restart_syscall();
@@ -1114,7 +1114,7 @@ static ssize_t bonding_store_primary(struct device *d,
 		goto out;
 	}
 
-	bond_for_each_slave(bond, slave, i) {
+	bond_for_each_slave(bond, slave) {
 		if (strncmp(slave->dev->name, ifname, IFNAMSIZ) == 0) {
 			pr_info("%s: Setting %s as primary slave.\n",
 				bond->dev->name, slave->dev->name);
@@ -1260,16 +1260,14 @@ static ssize_t bonding_store_active_slave(struct device *d,
 					  struct device_attribute *attr,
 					  const char *buf, size_t count)
 {
-	int i;
-	struct slave *slave;
-	struct slave *old_active = NULL;
-	struct slave *new_active = NULL;
+	struct slave *slave, *old_active, *new_active;
 	struct bonding *bond = to_bond(d);
 	char ifname[IFNAMSIZ];
 
 	if (!rtnl_trylock())
 		return restart_syscall();
 
+	old_active = new_active = NULL;
 	block_netpoll_tx();
 	read_lock(&bond->lock);
 	write_lock_bh(&bond->curr_slave_lock);
@@ -1291,7 +1289,7 @@ static ssize_t bonding_store_active_slave(struct device *d,
 		goto out;
 	}
 
-	bond_for_each_slave(bond, slave, i) {
+	bond_for_each_slave(bond, slave) {
 		if (strncmp(slave->dev->name, ifname, IFNAMSIZ) == 0) {
 			old_active = bond->curr_active_slave;
 			new_active = slave;
@@ -1475,15 +1473,15 @@ static ssize_t bonding_show_queue_id(struct device *d,
 				     struct device_attribute *attr,
 				     char *buf)
 {
-	struct slave *slave;
-	int i, res = 0;
 	struct bonding *bond = to_bond(d);
+	struct slave *slave;
+	int res = 0;
 
 	if (!rtnl_trylock())
 		return restart_syscall();
 
 	read_lock(&bond->lock);
-	bond_for_each_slave(bond, slave, i) {
+	bond_for_each_slave(bond, slave) {
 		if (res > (PAGE_SIZE - IFNAMSIZ - 6)) {
 			/* not enough space for another interface_name:queue_id pair */
 			if ((PAGE_SIZE - res) > 10)
@@ -1498,6 +1496,7 @@ static ssize_t bonding_show_queue_id(struct device *d,
 	if (res)
 		buf[res-1] = '\n'; /* eat the leftover space */
 	rtnl_unlock();
+
 	return res;
 }
 
@@ -1512,7 +1511,7 @@ static ssize_t bonding_store_queue_id(struct device *d,
 	struct slave *slave, *update_slave;
 	struct bonding *bond = to_bond(d);
 	u16 qid;
-	int i, ret = count;
+	int ret = count;
 	char *delim;
 	struct net_device *sdev = NULL;
 
@@ -1547,7 +1546,7 @@ static ssize_t bonding_store_queue_id(struct device *d,
 
 	/* Search for thes slave and check for duplicate qids */
 	update_slave = NULL;
-	bond_for_each_slave(bond, slave, i) {
+	bond_for_each_slave(bond, slave) {
 		if (sdev == slave->dev)
 			/*
 			 * We don't need to check the matching
@@ -1599,8 +1598,8 @@ static ssize_t bonding_store_slaves_active(struct device *d,
 					   struct device_attribute *attr,
 					   const char *buf, size_t count)
 {
-	int i, new_value, ret = count;
 	struct bonding *bond = to_bond(d);
+	int new_value, ret = count;
 	struct slave *slave;
 
 	if (sscanf(buf, "%d", &new_value) != 1) {
@@ -1623,7 +1622,7 @@ static ssize_t bonding_store_slaves_active(struct device *d,
 	}
 
 	read_lock(&bond->lock);
-	bond_for_each_slave(bond, slave, i) {
+	bond_for_each_slave(bond, slave) {
 		if (!bond_is_active_slave(slave)) {
 			if (new_value)
 				slave->inactive = 0;
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index 42d1c65..4abc3e0 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -71,6 +71,28 @@
 	set_fs(fs);			\
 	res; })
 
+/* slave list primitives */
+#define bond_to_slave(ptr) list_entry(ptr, struct slave, list)
+
+/* IMPORTANT: bond_first/last_slave can return NULL in case of an empty list */
+#define bond_first_slave(bond) \
+	list_first_entry_or_null(&(bond)->slave_list, struct slave, list)
+#define bond_last_slave(bond) \
+	(list_empty(&(bond)->slave_list) ? NULL : \
+					  bond_to_slave((bond)->slave_list.prev))
+
+#define bond_is_first_slave(bond, pos) ((pos)->list.prev == &(bond)->slave_list)
+#define bond_is_last_slave(bond, pos) ((pos)->list.next == &(bond)->slave_list)
+
+/* Since bond_first/last_slave can return NULL, these can return NULL too */
+#define bond_next_slave(bond, pos) \
+	(bond_is_last_slave(bond, pos) ? bond_first_slave(bond) : \
+					bond_to_slave((pos)->list.next))
+
+#define bond_prev_slave(bond, pos) \
+	(bond_is_first_slave(bond, pos) ? bond_last_slave(bond) : \
+					 bond_to_slave((pos)->list.prev))
+
 /**
  * bond_for_each_slave_from - iterate the slaves list from a starting point
  * @bond:	the bond holding this list.
@@ -80,37 +102,29 @@
  *
  * Caller must hold bond->lock
  */
-#define bond_for_each_slave_from(bond, pos, cnt, start)	\
-	for (cnt = 0, pos = start;				\
-	     cnt < (bond)->slave_cnt;				\
-             cnt++, pos = (pos)->next)
+#define bond_for_each_slave_from(bond, pos, cnt, start) \
+	for (cnt = 0, pos = start; pos && cnt < (bond)->slave_cnt; \
+	     cnt++, pos = bond_next_slave(bond, pos))
 
 /**
- * bond_for_each_slave_from_to - iterate the slaves list from start point to stop point
- * @bond:	the bond holding this list.
- * @pos:	current slave.
- * @cnt:	counter for number max of moves
- * @start:	start point.
- * @stop:	stop point.
+ * bond_for_each_slave - iterate over all slaves
+ * @bond:	the bond holding this list
+ * @pos:	current slave
  *
  * Caller must hold bond->lock
  */
-#define bond_for_each_slave_from_to(bond, pos, cnt, start, stop)	\
-	for (cnt = 0, pos = start;					\
-	     ((cnt < (bond)->slave_cnt) && (pos != (stop)->next));	\
-             cnt++, pos = (pos)->next)
+#define bond_for_each_slave(bond, pos) \
+	list_for_each_entry(pos, &(bond)->slave_list, list)
 
 /**
- * bond_for_each_slave - iterate the slaves list from head
- * @bond:	the bond holding this list.
- * @pos:	current slave.
- * @cnt:	counter for max number of moves
+ * bond_for_each_slave_reverse - iterate in reverse from a given position
+ * @bond:	the bond holding this list
+ * @pos:	slave to continue from
  *
  * Caller must hold bond->lock
  */
-#define bond_for_each_slave(bond, pos, cnt)	\
-		bond_for_each_slave_from(bond, pos, cnt, (bond)->first_slave)
-
+#define bond_for_each_slave_continue_reverse(bond, pos) \
+	list_for_each_entry_continue_reverse(pos, &(bond)->slave_list, list)
 
 #ifdef CONFIG_NET_POLL_CONTROLLER
 extern atomic_t netpoll_block_tx;
@@ -174,8 +188,7 @@ struct vlan_entry {
 
 struct slave {
 	struct net_device *dev; /* first - useful for panic debug */
-	struct slave *next;
-	struct slave *prev;
+	struct list_head list;
 	struct bonding *bond; /* our master */
 	int    delay;
 	unsigned long jiffies;
@@ -215,7 +228,7 @@ struct slave {
  */
 struct bonding {
 	struct   net_device *dev; /* first - useful for panic debug */
-	struct   slave *first_slave;
+	struct   list_head slave_list;
 	struct   slave *curr_active_slave;
 	struct   slave *current_arp_slave;
 	struct   slave *primary_slave;
@@ -270,13 +283,10 @@ static inline struct slave *bond_get_slave_by_dev(struct bonding *bond,
 						  struct net_device *slave_dev)
 {
 	struct slave *slave = NULL;
-	int i;
 
-	bond_for_each_slave(bond, slave, i) {
-		if (slave->dev == slave_dev) {
+	bond_for_each_slave(bond, slave)
+		if (slave->dev == slave_dev)
 			return slave;
-		}
-	}
 
 	return NULL;
 }
@@ -477,10 +487,9 @@ static inline void bond_destroy_proc_dir(struct bond_net *bn)
 static inline struct slave *bond_slave_has_mac(struct bonding *bond,
 					       const u8 *mac)
 {
-	int i = 0;
 	struct slave *tmp;
 
-	bond_for_each_slave(bond, tmp, i)
+	bond_for_each_slave(bond, tmp)
 		if (ether_addr_equal_64bits(mac, tmp->dev->dev_addr))
 			return tmp;
 
-- 
1.8.1.4

^ permalink raw reply related

* [PATCH net-next v2 4/5] bonding: factor out slave id tx code and simplify xmit paths
From: Nikolay Aleksandrov @ 2013-08-01 12:10 UTC (permalink / raw)
  To: netdev; +Cc: andy, davem, fubar
In-Reply-To: <1375359029-25889-1-git-send-email-nikolay@redhat.com>

From: Nikolay Aleksandrov <razor@BlackWall.org>

I factored out the tx xmit code which relies on slave id in
bond_xmit_slave_id. It is global because later it can be used also in
3ad mode xmit. Unnecessary obvious comments are removed. Active-backup
mode is simplified because bond_dev_queue_xmit always consumes the skb.
bond_xmit_xor becomes one line because of bond_xmit_slave_id.
bond_for_each_slave_from is not used in bond_xmit_slave_id because later
when RCU is used we can avoid important race condition by using standard
rculist routines.

Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
v2: no need to NULL slave in xmit_roundrobin

 drivers/net/bonding/bond_main.c | 118 +++++++++++++++++-----------------------
 drivers/net/bonding/bonding.h   |  10 ++++
 2 files changed, 61 insertions(+), 67 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 5f9d2fa..94f9284 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3795,12 +3795,50 @@ unwind:
 	return res;
 }
 
+/**
+ * bond_xmit_slave_id - transmit skb through slave with slave_id
+ * @bond: bonding device that is transmitting
+ * @skb: buffer to transmit
+ * @slave_id: slave id up to slave_cnt-1 through which to transmit
+ *
+ * This function tries to transmit through slave with slave_id but in case
+ * it fails, it tries to find the first available slave for transmission.
+ * The skb is consumed in all cases, thus the function is void.
+ */
+void bond_xmit_slave_id(struct bonding *bond, struct sk_buff *skb, int slave_id)
+{
+	struct slave *slave;
+	int i = slave_id;
+
+	/* Here we start from the slave with slave_id */
+	bond_for_each_slave(bond, slave) {
+		if (--i < 0) {
+			if (slave_can_tx(slave)) {
+				bond_dev_queue_xmit(bond, skb, slave->dev);
+				return;
+			}
+		}
+	}
+
+	/* Here we start from the first slave up to slave_id */
+	i = slave_id;
+	bond_for_each_slave(bond, slave) {
+		if (--i < 0)
+			break;
+		if (slave_can_tx(slave)) {
+			bond_dev_queue_xmit(bond, skb, slave->dev);
+			return;
+		}
+	}
+	/* no slave that can tx has been found */
+	kfree_skb(skb);
+}
+
 static int bond_xmit_roundrobin(struct sk_buff *skb, struct net_device *bond_dev)
 {
 	struct bonding *bond = netdev_priv(bond_dev);
-	struct slave *slave, *start_at;
-	int i, slave_no, res = 1;
 	struct iphdr *iph = ip_hdr(skb);
+	struct slave *slave;
 
 	/*
 	 * Start with the curr_active_slave that joined the bond as the
@@ -3809,46 +3847,20 @@ static int bond_xmit_roundrobin(struct sk_buff *skb, struct net_device *bond_dev
 	 * send the join/membership reports.  The curr_active_slave found
 	 * will send all of this type of traffic.
 	 */
-	if ((iph->protocol == IPPROTO_IGMP) &&
-	    (skb->protocol == htons(ETH_P_IP))) {
+	if (iph->protocol == IPPROTO_IGMP && skb->protocol == htons(ETH_P_IP)) {
 		slave = bond->curr_active_slave;
-		if (!slave)
-			goto out;
+		if (slave && slave_can_tx(slave))
+			bond_dev_queue_xmit(bond, skb, slave->dev);
+		else
+			bond_xmit_slave_id(bond, skb, 0);
 	} else {
-		/*
-		 * Concurrent TX may collide on rr_tx_counter; we accept
-		 * that as being rare enough not to justify using an
-		 * atomic op here.
-		 */
-		slave_no = bond->rr_tx_counter++ % bond->slave_cnt;
-
-		bond_for_each_slave(bond, slave) {
-			slave_no--;
-			if (slave_no < 0)
-				break;
-		}
-	}
-
-	start_at = slave;
-	bond_for_each_slave_from(bond, slave, i, start_at) {
-		if (IS_UP(slave->dev) &&
-		    (slave->link == BOND_LINK_UP) &&
-		    bond_is_active_slave(slave)) {
-			res = bond_dev_queue_xmit(bond, skb, slave->dev);
-			break;
-		}
-	}
-
-out:
-	if (res) {
-		/* no suitable interface, frame not sent */
-		kfree_skb(skb);
+		bond_xmit_slave_id(bond, skb,
+				   bond->rr_tx_counter++ % bond->slave_cnt);
 	}
 
 	return NETDEV_TX_OK;
 }
 
-
 /*
  * in active-backup mode, we know that bond->curr_active_slave is always valid if
  * the bond has a usable interface.
@@ -3857,14 +3869,11 @@ static int bond_xmit_activebackup(struct sk_buff *skb, struct net_device *bond_d
 {
 	struct bonding *bond = netdev_priv(bond_dev);
 	struct slave *slave;
-	int res = 1;
 
 	slave = bond->curr_active_slave;
 	if (slave)
-		res = bond_dev_queue_xmit(bond, skb, slave->dev);
-
-	if (res)
-		/* no suitable interface, frame not sent */
+		bond_dev_queue_xmit(bond, skb, slave->dev);
+	else
 		kfree_skb(skb);
 
 	return NETDEV_TX_OK;
@@ -3878,34 +3887,9 @@ static int bond_xmit_activebackup(struct sk_buff *skb, struct net_device *bond_d
 static int bond_xmit_xor(struct sk_buff *skb, struct net_device *bond_dev)
 {
 	struct bonding *bond = netdev_priv(bond_dev);
-	struct slave *slave, *start_at;
-	int slave_no;
-	int i;
-	int res = 1;
-
-	slave_no = bond->xmit_hash_policy(skb, bond->slave_cnt);
-
-	bond_for_each_slave(bond, slave) {
-		slave_no--;
-		if (slave_no < 0)
-			break;
-	}
-
-	start_at = slave;
 
-	bond_for_each_slave_from(bond, slave, i, start_at) {
-		if (IS_UP(slave->dev) &&
-		    (slave->link == BOND_LINK_UP) &&
-		    bond_is_active_slave(slave)) {
-			res = bond_dev_queue_xmit(bond, skb, slave->dev);
-			break;
-		}
-	}
-
-	if (res) {
-		/* no suitable interface, frame not sent */
-		kfree_skb(skb);
-	}
+	bond_xmit_slave_id(bond, skb,
+			   bond->xmit_hash_policy(skb, bond->slave_cnt));
 
 	return NETDEV_TX_OK;
 }
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index 4abc3e0..e7ab227 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -426,10 +426,20 @@ static inline __be32 bond_confirm_addr(struct net_device *dev, __be32 dst, __be3
 	return addr;
 }
 
+static inline bool slave_can_tx(struct slave *slave)
+{
+	if (IS_UP(slave->dev) && slave->link == BOND_LINK_UP &&
+	    bond_is_active_slave(slave))
+		return true;
+	else
+		return false;
+}
+
 struct bond_net;
 
 struct vlan_entry *bond_next_vlan(struct bonding *bond, struct vlan_entry *curr);
 int bond_dev_queue_xmit(struct bonding *bond, struct sk_buff *skb, struct net_device *slave_dev);
+void bond_xmit_slave_id(struct bonding *bond, struct sk_buff *skb, int slave_id);
 int bond_create(struct net *net, const char *name);
 int bond_create_sysfs(struct bond_net *net);
 void bond_destroy_sysfs(struct bond_net *net);
-- 
1.8.1.4

^ permalink raw reply related

* [PATCH net-next v2 5/5] bonding: initial RCU conversion
From: Nikolay Aleksandrov @ 2013-08-01 12:10 UTC (permalink / raw)
  To: netdev; +Cc: andy, davem, fubar
In-Reply-To: <1375359029-25889-1-git-send-email-nikolay@redhat.com>

This patch does the initial bonding conversion to RCU. After it the
following modes are protected by RCU alone: roundrobin, active-backup,
broadcast and xor. Modes ALB/TLB and 3ad still acquire bond->lock for
reading, and will be dealt with later. curr_active_slave needs to be
dereferenced via rcu in the converted modes because the only thing
protecting the slave after this patch is rcu_read_lock, so we need the
proper barrier for weakly ordered archs and to make sure we don't have
stale pointer. It's not tagged with __rcu yet because there's still work
to be done to remove the curr_slave_lock, so sparse will complain when
rcu_assign_pointer and rcu_dereference are used, but the alternative to use
rcu_dereference_protected would've created much bigger code churn which is
more difficult to test and review. That will be converted in time.

1. Active-backup mode
 1.1 Perf recording while doing iperf -P 4
  - old bonding: iperf spent 0.55% in bonding, system spent 0.29% CPU
                 in bonding
  - new bonding: iperf spent 0.29% in bonding, system spent 0.15% CPU
                 in bonding
 1.2. Bandwidth measurements
  - old bonding: 16.1 gbps consistently
  - new bonding: 17.5 gbps consistently

2. Round-robin mode
 2.1 Perf recording while doing iperf -P 4
  - old bonding: iperf spent 0.51% in bonding, system spent 0.24% CPU
                 in bonding
  - new bonding: iperf spent 0.16% in bonding, system spent 0.11% CPU
                 in bonding
 2.2 Bandwidth measurements
  - old bonding: 8 gbps (variable due to packet reorderings)
  - new bonding: 10 gbps (variable due to packet reorderings)

Of course the latency has improved in all converted modes, and moreover
while
doing enslave/release (since it doesn't affect tx anymore).

Also I've stress tested all modes doing enslave/release in a loop while
transmitting traffic.

Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
v2: include performance notes, convert curr_active_slave use in bond_sysfs
to RCU, fix a potential latency issue in slave release because TX was
unblocked after synchronize_rcu which might take a while, introduce new
primitive bond_for_each_slave_rcu

 drivers/net/bonding/bond_3ad.c   |  2 ++
 drivers/net/bonding/bond_alb.c   |  6 ++++--
 drivers/net/bonding/bond_main.c  | 30 +++++++++++++++---------------
 drivers/net/bonding/bond_sysfs.c | 19 +++++++------------
 drivers/net/bonding/bonding.h    |  4 ++++
 5 files changed, 32 insertions(+), 29 deletions(-)

diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index b96f9da..ad8062b 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -2422,6 +2422,7 @@ int bond_3ad_xmit_xor(struct sk_buff *skb, struct net_device *dev)
 	struct ad_info ad_info;
 	int res = 1;
 
+	read_lock(&bond->lock);
 	if (__bond_3ad_get_active_agg_info(bond, &ad_info)) {
 		pr_debug("%s: Error: __bond_3ad_get_active_agg_info failed\n",
 			 dev->name);
@@ -2471,6 +2472,7 @@ int bond_3ad_xmit_xor(struct sk_buff *skb, struct net_device *dev)
 	}
 
 out:
+	read_unlock(&bond->lock);
 	if (res) {
 		/* no suitable interface, frame not sent */
 		kfree_skb(skb);
diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
index 4d35196..3a5db7b 100644
--- a/drivers/net/bonding/bond_alb.c
+++ b/drivers/net/bonding/bond_alb.c
@@ -1337,6 +1337,7 @@ int bond_alb_xmit(struct sk_buff *skb, struct net_device *bond_dev)
 
 	/* make sure that the curr_active_slave do not change during tx
 	 */
+	read_lock(&bond->lock);
 	read_lock(&bond->curr_slave_lock);
 
 	switch (ntohs(skb->protocol)) {
@@ -1441,11 +1442,12 @@ int bond_alb_xmit(struct sk_buff *skb, struct net_device *bond_dev)
 	}
 
 	read_unlock(&bond->curr_slave_lock);
-
+	read_unlock(&bond->lock);
 	if (res) {
 		/* no suitable interface, frame not sent */
 		kfree_skb(skb);
 	}
+
 	return NETDEV_TX_OK;
 }
 
@@ -1663,7 +1665,7 @@ void bond_alb_handle_active_change(struct bonding *bond, struct slave *new_slave
 	}
 
 	swap_slave = bond->curr_active_slave;
-	bond->curr_active_slave = new_slave;
+	rcu_assign_pointer(bond->curr_active_slave, new_slave);
 
 	if (!new_slave || list_empty(&bond->slave_list))
 		return;
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 94f9284..5678ead 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -77,6 +77,7 @@
 #include <net/net_namespace.h>
 #include <net/netns/generic.h>
 #include <net/pkt_sched.h>
+#include <linux/rculist.h>
 #include "bonding.h"
 #include "bond_3ad.h"
 #include "bond_alb.h"
@@ -1037,7 +1038,7 @@ void bond_change_active_slave(struct bonding *bond, struct slave *new_active)
 		if (new_active)
 			bond_set_slave_active_flags(new_active);
 	} else {
-		bond->curr_active_slave = new_active;
+		rcu_assign_pointer(bond->curr_active_slave, new_active);
 	}
 
 	if (bond->params.mode == BOND_MODE_ACTIVEBACKUP) {
@@ -1127,7 +1128,7 @@ void bond_select_active_slave(struct bonding *bond)
  */
 static void bond_attach_slave(struct bonding *bond, struct slave *new_slave)
 {
-	list_add_tail(&new_slave->list, &bond->slave_list);
+	list_add_tail_rcu(&new_slave->list, &bond->slave_list);
 	bond->slave_cnt++;
 }
 
@@ -1143,7 +1144,7 @@ static void bond_attach_slave(struct bonding *bond, struct slave *new_slave)
  */
 static void bond_detach_slave(struct bonding *bond, struct slave *slave)
 {
-	list_del(&slave->list);
+	list_del_rcu(&slave->list);
 	bond->slave_cnt--;
 }
 
@@ -1751,7 +1752,7 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev)
 		 * so we can change it without calling change_active_interface()
 		 */
 		if (!bond->curr_active_slave && new_slave->link == BOND_LINK_UP)
-			bond->curr_active_slave = new_slave;
+			rcu_assign_pointer(bond->curr_active_slave, new_slave);
 
 		break;
 	} /* switch(bond_mode) */
@@ -1951,7 +1952,7 @@ static int __bond_release_one(struct net_device *bond_dev,
 	}
 
 	if (all) {
-		bond->curr_active_slave = NULL;
+		rcu_assign_pointer(bond->curr_active_slave, NULL);
 	} else if (oldcurrent == slave) {
 		/*
 		 * Note that we hold RTNL over this sequence, so there
@@ -1983,6 +1984,7 @@ static int __bond_release_one(struct net_device *bond_dev,
 
 	write_unlock_bh(&bond->lock);
 	unblock_netpoll_tx();
+	synchronize_rcu();
 
 	if (list_empty(&bond->slave_list)) {
 		call_netdevice_notifiers(NETDEV_CHANGEADDR, bond->dev);
@@ -3811,7 +3813,7 @@ void bond_xmit_slave_id(struct bonding *bond, struct sk_buff *skb, int slave_id)
 	int i = slave_id;
 
 	/* Here we start from the slave with slave_id */
-	bond_for_each_slave(bond, slave) {
+	bond_for_each_slave_rcu(bond, slave) {
 		if (--i < 0) {
 			if (slave_can_tx(slave)) {
 				bond_dev_queue_xmit(bond, skb, slave->dev);
@@ -3822,7 +3824,7 @@ void bond_xmit_slave_id(struct bonding *bond, struct sk_buff *skb, int slave_id)
 
 	/* Here we start from the first slave up to slave_id */
 	i = slave_id;
-	bond_for_each_slave(bond, slave) {
+	bond_for_each_slave_rcu(bond, slave) {
 		if (--i < 0)
 			break;
 		if (slave_can_tx(slave)) {
@@ -3848,7 +3850,7 @@ static int bond_xmit_roundrobin(struct sk_buff *skb, struct net_device *bond_dev
 	 * will send all of this type of traffic.
 	 */
 	if (iph->protocol == IPPROTO_IGMP && skb->protocol == htons(ETH_P_IP)) {
-		slave = bond->curr_active_slave;
+		slave = rcu_dereference(bond->curr_active_slave);
 		if (slave && slave_can_tx(slave))
 			bond_dev_queue_xmit(bond, skb, slave->dev);
 		else
@@ -3870,7 +3872,7 @@ static int bond_xmit_activebackup(struct sk_buff *skb, struct net_device *bond_d
 	struct bonding *bond = netdev_priv(bond_dev);
 	struct slave *slave;
 
-	slave = bond->curr_active_slave;
+	slave = rcu_dereference(bond->curr_active_slave);
 	if (slave)
 		bond_dev_queue_xmit(bond, skb, slave->dev);
 	else
@@ -3900,7 +3902,7 @@ static int bond_xmit_broadcast(struct sk_buff *skb, struct net_device *bond_dev)
 	struct bonding *bond = netdev_priv(bond_dev);
 	struct slave *slave = NULL;
 
-	bond_for_each_slave(bond, slave) {
+	bond_for_each_slave_rcu(bond, slave) {
 		if (bond_is_last_slave(bond, slave))
 			break;
 		if (IS_UP(slave->dev) && slave->link == BOND_LINK_UP) {
@@ -3955,7 +3957,7 @@ static inline int bond_slave_override(struct bonding *bond,
 		return 1;
 
 	/* Find out if any slaves have the same mapping as this skb. */
-	bond_for_each_slave(bond, check_slave) {
+	bond_for_each_slave_rcu(bond, check_slave) {
 		if (check_slave->queue_id == skb->queue_mapping) {
 			slave = check_slave;
 			break;
@@ -4040,14 +4042,12 @@ static netdev_tx_t bond_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	if (is_netpoll_tx_blocked(dev))
 		return NETDEV_TX_BUSY;
 
-	read_lock(&bond->lock);
-
+	rcu_read_lock();
 	if (!list_empty(&bond->slave_list))
 		ret = __bond_start_xmit(skb, dev);
 	else
 		kfree_skb(skb);
-
-	read_unlock(&bond->lock);
+	rcu_read_unlock();
 
 	return ret;
 }
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index 0702e91..0f539de 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -1243,16 +1243,16 @@ static ssize_t bonding_show_active_slave(struct device *d,
 					 struct device_attribute *attr,
 					 char *buf)
 {
-	struct slave *curr;
 	struct bonding *bond = to_bond(d);
+	struct slave *curr;
 	int count = 0;
 
-	read_lock(&bond->curr_slave_lock);
-	curr = bond->curr_active_slave;
-	read_unlock(&bond->curr_slave_lock);
-
+	rcu_read_lock();
+	curr = rcu_dereference(bond->curr_active_slave);
 	if (USES_PRIMARY(bond->params.mode) && curr)
 		count = sprintf(buf, "%s\n", curr->dev->name);
+	rcu_read_unlock();
+
 	return count;
 }
 
@@ -1284,7 +1284,7 @@ static ssize_t bonding_store_active_slave(struct device *d,
 	if (!strlen(ifname) || buf[0] == '\n') {
 		pr_info("%s: Clearing current active slave.\n",
 			bond->dev->name);
-		bond->curr_active_slave = NULL;
+		rcu_assign_pointer(bond->curr_active_slave, NULL);
 		bond_select_active_slave(bond);
 		goto out;
 	}
@@ -1347,14 +1347,9 @@ static ssize_t bonding_show_mii_status(struct device *d,
 				       struct device_attribute *attr,
 				       char *buf)
 {
-	struct slave *curr;
 	struct bonding *bond = to_bond(d);
 
-	read_lock(&bond->curr_slave_lock);
-	curr = bond->curr_active_slave;
-	read_unlock(&bond->curr_slave_lock);
-
-	return sprintf(buf, "%s\n", curr ? "up" : "down");
+	return sprintf(buf, "%s\n", bond->curr_active_slave ? "up" : "down");
 }
 static DEVICE_ATTR(mii_status, S_IRUGO, bonding_show_mii_status, NULL);
 
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index e7ab227..d660b56 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -116,6 +116,10 @@
 #define bond_for_each_slave(bond, pos) \
 	list_for_each_entry(pos, &(bond)->slave_list, list)
 
+/* Caller must have rcu_read_lock */
+#define bond_for_each_slave_rcu(bond, pos) \
+	list_for_each_entry_rcu(pos, &(bond)->slave_list, list)
+
 /**
  * bond_for_each_slave_reverse - iterate in reverse from a given position
  * @bond:	the bond holding this list
-- 
1.8.1.4

^ permalink raw reply related

* [PATCH net-next v2 2/5] bonding: remove unnecessary read_locks of curr_slave_lock
From: Nikolay Aleksandrov @ 2013-08-01 12:10 UTC (permalink / raw)
  To: netdev; +Cc: andy, davem, fubar
In-Reply-To: <1375359029-25889-1-git-send-email-nikolay@redhat.com>

In all the cases we already hold bond->lock for reading, so the slave
can't get away and the check != NULL is sufficient. curr_active_slave
can still change after the read_lock is unlocked prior to use of the
dereferenced value, so there's no need for it. It either contains a
valid slave which we use (and can't get away), or it is NULL which is
checked.
In some places the read_lock of curr_slave_lock was left because we need
it not to change while performing some action (e.g. syncing current
active slave's addresses, sending ARP requests through the active slave)
such cases will be dealt with individually while converting to RCU.

Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
 drivers/net/bonding/bond_main.c | 33 ++++++++-------------------------
 1 file changed, 8 insertions(+), 25 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 3636a04..9bdc701 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2092,23 +2092,19 @@ static int bond_ioctl_change_active(struct net_device *bond_dev, struct net_devi
 
 	read_lock(&bond->lock);
 
-	read_lock(&bond->curr_slave_lock);
 	old_active = bond->curr_active_slave;
-	read_unlock(&bond->curr_slave_lock);
-
 	new_active = bond_get_slave_by_dev(bond, slave_dev);
-
 	/*
 	 * Changing to the current active: do nothing; return success.
 	 */
-	if (new_active && (new_active == old_active)) {
+	if (new_active && new_active == old_active) {
 		read_unlock(&bond->lock);
 		return 0;
 	}
 
-	if ((new_active) &&
-	    (old_active) &&
-	    (new_active->link == BOND_LINK_UP) &&
+	if (new_active &&
+	    old_active &&
+	    new_active->link == BOND_LINK_UP &&
 	    IS_UP(new_active->dev)) {
 		block_netpoll_tx();
 		write_lock_bh(&bond->curr_slave_lock);
@@ -2660,10 +2656,7 @@ void bond_loadbalance_arp_mon(struct work_struct *work)
 	if (list_empty(&bond->slave_list))
 		goto re_arm;
 
-	read_lock(&bond->curr_slave_lock);
 	oldcurrent = bond->curr_active_slave;
-	read_unlock(&bond->curr_slave_lock);
-
 	/* see if any of the previous devices are up now (i.e. they have
 	 * xmt and rcv traffic). the curr_active_slave does not come into
 	 * the picture unless it is null. also, slave->jiffies is not needed
@@ -3818,11 +3811,7 @@ static int bond_xmit_roundrobin(struct sk_buff *skb, struct net_device *bond_dev
 	 */
 	if ((iph->protocol == IPPROTO_IGMP) &&
 	    (skb->protocol == htons(ETH_P_IP))) {
-
-		read_lock(&bond->curr_slave_lock);
 		slave = bond->curr_active_slave;
-		read_unlock(&bond->curr_slave_lock);
-
 		if (!slave)
 			goto out;
 	} else {
@@ -3867,15 +3856,12 @@ out:
 static int bond_xmit_activebackup(struct sk_buff *skb, struct net_device *bond_dev)
 {
 	struct bonding *bond = netdev_priv(bond_dev);
+	struct slave *slave;
 	int res = 1;
 
-	read_lock(&bond->curr_slave_lock);
-
-	if (bond->curr_active_slave)
-		res = bond_dev_queue_xmit(bond, skb,
-			bond->curr_active_slave->dev);
-
-	read_unlock(&bond->curr_slave_lock);
+	slave = bond->curr_active_slave;
+	if (slave)
+		res = bond_dev_queue_xmit(bond, skb, slave->dev);
 
 	if (res)
 		/* no suitable interface, frame not sent */
@@ -3935,10 +3921,7 @@ static int bond_xmit_broadcast(struct sk_buff *skb, struct net_device *bond_dev)
 	int i;
 	int res = 1;
 
-	read_lock(&bond->curr_slave_lock);
 	start_at = bond->curr_active_slave;
-	read_unlock(&bond->curr_slave_lock);
-
 	if (!start_at)
 		goto out;
 
-- 
1.8.1.4

^ permalink raw reply related

* [PATCH net-next v2 3/5] bonding: simplify broadcast_xmit function
From: Nikolay Aleksandrov @ 2013-08-01 12:10 UTC (permalink / raw)
  To: netdev; +Cc: andy, davem, fubar
In-Reply-To: <1375359029-25889-1-git-send-email-nikolay@redhat.com>

From: Nikolay Aleksandrov <razor@BlackWall.org>

We don't need to start from the curr_active_slave as the frame will be
sent to all eligible slaves anyway, so we remove the unnecessary local
variables, checks and comments, and make it use the standard list API.
This has the nice side-effect that later when it's converted to RCU
a race condition will be avoided which could lead to double packet tx.

Signed-off-by: Nikolay Aleksandrov <nikolay@redhat.com>
---
 drivers/net/bonding/bond_main.c | 52 +++++++++++++----------------------------
 1 file changed, 16 insertions(+), 36 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 9bdc701..5f9d2fa 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3910,52 +3910,32 @@ static int bond_xmit_xor(struct sk_buff *skb, struct net_device *bond_dev)
 	return NETDEV_TX_OK;
 }
 
-/*
- * in broadcast mode, we send everything to all usable interfaces.
- */
+/* in broadcast mode, we send everything to all usable interfaces. */
 static int bond_xmit_broadcast(struct sk_buff *skb, struct net_device *bond_dev)
 {
 	struct bonding *bond = netdev_priv(bond_dev);
-	struct slave *slave, *start_at;
-	struct net_device *tx_dev = NULL;
-	int i;
-	int res = 1;
-
-	start_at = bond->curr_active_slave;
-	if (!start_at)
-		goto out;
+	struct slave *slave = NULL;
 
-	bond_for_each_slave_from(bond, slave, i, start_at) {
-		if (IS_UP(slave->dev) &&
-		    (slave->link == BOND_LINK_UP) &&
-		    bond_is_active_slave(slave)) {
-			if (tx_dev) {
-				struct sk_buff *skb2 = skb_clone(skb, GFP_ATOMIC);
-				if (!skb2) {
-					pr_err("%s: Error: bond_xmit_broadcast(): skb_clone() failed\n",
-					       bond_dev->name);
-					continue;
-				}
+	bond_for_each_slave(bond, slave) {
+		if (bond_is_last_slave(bond, slave))
+			break;
+		if (IS_UP(slave->dev) && slave->link == BOND_LINK_UP) {
+			struct sk_buff *skb2 = skb_clone(skb, GFP_ATOMIC);
 
-				res = bond_dev_queue_xmit(bond, skb2, tx_dev);
-				if (res) {
-					kfree_skb(skb2);
-					continue;
-				}
+			if (!skb2) {
+				pr_err("%s: Error: bond_xmit_broadcast(): skb_clone() failed\n",
+				       bond_dev->name);
+				continue;
 			}
-			tx_dev = slave->dev;
+			/* bond_dev_queue_xmit always returns 0 */
+			bond_dev_queue_xmit(bond, skb2, slave->dev);
 		}
 	}
-
-	if (tx_dev)
-		res = bond_dev_queue_xmit(bond, skb, tx_dev);
-
-out:
-	if (res)
-		/* no suitable interface, frame not sent */
+	if (slave && IS_UP(slave->dev) && slave->link == BOND_LINK_UP)
+		bond_dev_queue_xmit(bond, skb, slave->dev);
+	else
 		kfree_skb(skb);
 
-	/* frame sent to all suitable interfaces */
 	return NETDEV_TX_OK;
 }
 
-- 
1.8.1.4

^ permalink raw reply related

* [PATCH] ip link: fix display and filtering of ifgroups
From: Stefan Tomanek @ 2013-08-01 12:16 UTC (permalink / raw)
  To: netdev

This change adds the interface group to the output of "ip link show".

It also makes "ip link" print _all_ devices if no group filter is specified;
previously, only interfaces of the default group (0) were shown.

Signed-off-by: Stefan Tomanek <stefan.tomanek@wertarbyte.de>
---
 ip/ipaddress.c |    9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index 013b4cb..cf4015a 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -418,7 +418,7 @@ int print_linkinfo(const struct sockaddr_nl *who,
 
 	if (tb[IFLA_GROUP]) {
 		int group = *(int*)RTA_DATA(tb[IFLA_GROUP]);
-		if (group != filter.group)
+		if (filter.group != -1 && group != filter.group)
 			return -1;
 	}
 
@@ -458,6 +458,11 @@ int print_linkinfo(const struct sockaddr_nl *who,
 	if (do_link && tb[IFLA_LINKMODE])
 		print_linkmode(fp, tb[IFLA_LINKMODE]);
 
+	if (tb[IFLA_GROUP]) {
+		int group = *(int*)RTA_DATA(tb[IFLA_GROUP]);
+		fprintf(fp, "group %d ", group);
+	}
+
 	if (filter.showqueue)
 		print_queuelen(fp, tb);
 
@@ -1050,7 +1055,7 @@ static int ipaddr_list_flush_or_save(int argc, char **argv, int action)
 	if (filter.family == AF_UNSPEC)
 		filter.family = preferred_family;
 
-	filter.group = INIT_NETDEV_GROUP;
+	filter.group = -1;
 
 	if (action == IPADD_FLUSH) {
 		if (argc <= 0) {
-- 
1.7.10.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