Netdev List
 help / color / mirror / Atom feed
* [patch net] net: correct check in dev_addr_del()
From: Jiri Pirko @ 2012-11-14 12:51 UTC (permalink / raw)
  To: netdev; +Cc: davem, eric.dumazet, shemminger, john.r.fastabend

Check (ha->addr == dev->dev_addr) is always true because dev_addr_init()
sets this. Correct the check to behave properly on addr removal.

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
 net/core/dev_addr_lists.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/core/dev_addr_lists.c b/net/core/dev_addr_lists.c
index 87cc17d..b079c7b 100644
--- a/net/core/dev_addr_lists.c
+++ b/net/core/dev_addr_lists.c
@@ -319,7 +319,8 @@ int dev_addr_del(struct net_device *dev, const unsigned char *addr,
 	 */
 	ha = list_first_entry(&dev->dev_addrs.list,
 			      struct netdev_hw_addr, list);
-	if (ha->addr == dev->dev_addr && ha->refcount == 1)
+	if (!memcmp(ha->addr, addr, dev->addr_len) &&
+	    ha->type == addr_type && ha->refcount == 1)
 		return -ENOENT;
 
 	err = __hw_addr_del(&dev->dev_addrs, addr, dev->addr_len,
-- 
1.8.0

^ permalink raw reply related

* [PATCH 0/2] iputils: minor fixes
From: Jan Synacek @ 2012-11-14 12:57 UTC (permalink / raw)
  To: yoshfuji; +Cc: netdev, Jan Synacek

Jan Synacek (2):
  ping,ping6: Add newline to error message.
  ping: Don't free an unintialized value.

 ping.c        | 8 +++++---
 ping_common.c | 2 +-
 2 files changed, 6 insertions(+), 4 deletions(-)

-- 
1.7.11.7

^ permalink raw reply

* [PATCH 1/2] ping,ping6: Add newline to error message.
From: Jan Synacek @ 2012-11-14 12:57 UTC (permalink / raw)
  To: yoshfuji; +Cc: netdev, Jan Synacek
In-Reply-To: <1352897836-17603-1-git-send-email-jsynacek@redhat.com>

---
 ping_common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ping_common.c b/ping_common.c
index 86dbeaa..1605ca9 100644
--- a/ping_common.c
+++ b/ping_common.c
@@ -265,7 +265,7 @@ void common_options(int ch)
 		char *endp;
 		mark = (int)strtoul(optarg, &endp, 10);
 		if (mark < 0 || *endp != '\0') {
-			fprintf(stderr, "mark cannot be negative");
+			fprintf(stderr, "mark cannot be negative\n");
 			exit(2);
 		}
 		options |= F_MARK;
-- 
1.7.11.7

^ permalink raw reply related

* [PATCH 2/2] ping: Don't free an unintialized value.
From: Jan Synacek @ 2012-11-14 12:57 UTC (permalink / raw)
  To: yoshfuji; +Cc: netdev, Jan Synacek
In-Reply-To: <1352897836-17603-1-git-send-email-jsynacek@redhat.com>

---
 ping.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/ping.c b/ping.c
index fe9ff8a..9de3d08 100644
--- a/ping.c
+++ b/ping.c
@@ -122,7 +122,7 @@ main(int argc, char **argv)
 	u_char *packet;
 	char *target;
 #ifdef USE_IDN
-	char *hnamebuf;
+	char *hnamebuf = NULL;
 #else
 	char hnamebuf[MAX_HOSTNAMELEN];
 #endif
@@ -263,8 +263,10 @@ main(int argc, char **argv)
 #ifdef USE_IDN
 			int rc;
 
-			free(hnamebuf);
-			hnamebuf = NULL;
+			if (hnamebuf) {
+				free(hnamebuf);
+				hnamebuf = NULL;
+			}
 
 			rc = idna_to_ascii_lz(target, &idn, 0);
 			if (rc != IDNA_SUCCESS) {
-- 
1.7.11.7

^ permalink raw reply related

* Re: [RFC PATCH 01/13] net:  Add generic packet offload infrastructure.
From: Vlad Yasevich @ 2012-11-14 13:03 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev, davem
In-Reply-To: <1352859861.4497.0.camel@edumazet-glaptop>

On 11/13/2012 09:24 PM, Eric Dumazet wrote:
> On Tue, 2012-11-13 at 20:24 -0500, Vlad Yasevich wrote:
>> Create a new data structure to contain the GRO/GSO callbacks and add
>> a new registration mechanism.
>>
>> Singed-off-by: Vlad Yasevich <vyasevic@redhat.com>
>> ---
>>   include/linux/netdevice.h |   15 ++++++++
>>   net/core/dev.c            |   80 +++++++++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 95 insertions(+), 0 deletions(-)
>>
>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>> index f8eda02..d15af51 100644
>> --- a/include/linux/netdevice.h
>> +++ b/include/linux/netdevice.h
>> @@ -1511,6 +1511,18 @@ struct packet_type {
>>   	struct list_head	list;
>>   };
>>
>> +struct packet_offload {
>> +	__be16			type;	/* This is really htons(ether_type). */
>> +	struct net_device	*dev;	/* NULL is wildcarded here	     */
>
> Shouldnt this dev pointer be removed at some point in the patch serie ?

yes.  i was thinking about this as well.  I actually shouldn't have been 
carried into this struct to begin with since its not really being used 
by the offload calls.

-vlad

>
>> +	struct sk_buff		*(*gso_segment)(struct sk_buff *skb,
>> +						netdev_features_t features);
>> +	int			(*gso_send_check)(struct sk_buff *skb);
>> +	struct sk_buff		**(*gro_receive)(struct sk_buff **head,
>> +					       struct sk_buff *skb);
>> +	int			(*gro_complete)(struct sk_buff *skb);
>> +	struct list_head	list;
>> +};
>> +
>>   #include <linux/notifier.h>
>>
>

^ permalink raw reply

* [PATCH net-next 0/3] myri10ge: LRO to GRO conversion
From: Andrew Gallatin @ 2012-11-14 13:06 UTC (permalink / raw)
  To: netdev

Hi,

The following patchset converts myri10ge from using the old inet_lro
interface to GRO.

Note that a naive LRO->GRO conversion of myri10ge will result in a
performance regression for vlan tagged frames.  This is because
myri10ge does not offer hardware vlan tag offload, and because GRO
requires hardware vlan tag offload to aggregate vlan tagged frames.

To address this performance regression, I have implemented vlan tag
popping in the myri10ge driver, as it seems to be the lesser of two
evils.  As eric.dumazet@gmail.com commented when I asked about this on
netdev last week: "Given GRO assumes NIC does hardware vlan
offloading, I guess I would chose to do that.  It seems unfortunate to
add vlan decap in GRO path, already very complex."


Andrew Gallatin (3):
myri10ge: Convert from LRO to GRO
myri10ge: Add vlan rx for better GRO perf.
myri10ge: Use skb_fill_page_desc().


  drivers/net/ethernet/myricom/Kconfig             |    1 -
  drivers/net/ethernet/myricom/myri10ge/myri10ge.c |  281 
++++++----------------
  2 files changed, 80 insertions(+), 202 deletions(-)

^ permalink raw reply

* [PATCH net-next 2/3] myri10ge: Add vlan rx for better GRO perf.
From: Andrew Gallatin @ 2012-11-14 13:06 UTC (permalink / raw)
  To: netdev



Unlike LRO, GRO requires that vlan tags be removed before
aggregation can occur.  Since the myri10ge NIC does not support
hardware vlan tag offload, we must remove the tag in the driver
to achieve performance comparable to LRO for vlan tagged frames.

Signed-off-by: Andrew Gallatin <gallatin@myri.com>
---
  drivers/net/ethernet/myricom/myri10ge/myri10ge.c |   47 
++++++++++++++++++++++
  1 file changed, 47 insertions(+)

diff --git a/drivers/net/ethernet/myricom/myri10ge/myri10ge.c 
b/drivers/net/ethernet/myricom/myri10ge/myri10ge.c
index a5ab2f2..b9b6dfd 100644
--- a/drivers/net/ethernet/myricom/myri10ge/myri10ge.c
+++ b/drivers/net/ethernet/myricom/myri10ge/myri10ge.c
@@ -1264,6 +1264,48 @@ myri10ge_unmap_rx_page(struct pci_dev *pdev,
  	}
  }

+/*
+ * GRO does not support acceleration of tagged vlan frames, and
+ * this NIC does not support vlan tag offload, so we must pop
+ * the tag ourselves to be able to achieve GRO performance that
+ * is comparable to LRO.
+ */
+
+static inline void
+myri10ge_vlan_rx(struct net_device *dev, void *addr, struct sk_buff *skb)
+{
+	u8 *va;
+	struct vlan_ethhdr *veh;
+	struct ethhdr *eh;
+	struct skb_frag_struct *frag;
+	u16 proto;
+
+	va = addr;
+	va += MXGEFW_PAD;
+	veh = (struct vlan_ethhdr *) va;
+	if ((dev->features & (NETIF_F_HW_VLAN_RX | NETIF_F_GRO)) ==
+	    (NETIF_F_HW_VLAN_RX | NETIF_F_GRO) &&
+	    (veh->h_vlan_proto == ntohs(ETH_P_8021Q))) {
+		/* fixup csum if needed */
+		if (skb->ip_summed == CHECKSUM_COMPLETE)
+			skb->csum = csum_sub(skb->csum,
+					     csum_partial(va + ETH_HLEN,
+							  VLAN_HLEN, 0));
+		/* pop tag */
+		__vlan_hwaccel_put_tag(skb, ntohs(veh->h_vlan_TCI));
+		proto = veh->h_vlan_encapsulated_proto;
+		memmove(va + VLAN_HLEN, va, ETH_HLEN);
+		va += VLAN_HLEN;
+		eh = (struct ethhdr *)va;
+		eh->h_proto = proto;
+		skb->len -= VLAN_HLEN;
+		skb->data_len -= VLAN_HLEN;
+		frag = skb_shinfo(skb)->frags;
+		frag->page_offset += VLAN_HLEN;
+		skb_frag_size_set(frag, skb_frag_size(frag) - VLAN_HLEN);
+	}
+}
+
  static inline int
  myri10ge_rx_done(struct myri10ge_slice_state *ss, int len, __wsum csum)
  {
@@ -1329,6 +1371,7 @@ myri10ge_rx_done(struct myri10ge_slice_state *ss, 
int len, __wsum csum)
  		skb->ip_summed = CHECKSUM_COMPLETE;
  		skb->csum = csum;
  	}
+	myri10ge_vlan_rx(mgp->dev, va, skb);
  	skb_record_rx_queue(skb, ss - &mgp->ss[0]);

  	napi_gro_frags(&ss->napi);
@@ -3854,6 +3897,10 @@ static int myri10ge_probe(struct pci_dev *pdev, 
const struct pci_device_id *ent)
  	netdev->netdev_ops = &myri10ge_netdev_ops;
  	netdev->mtu = myri10ge_initial_mtu;
  	netdev->hw_features = mgp->features | NETIF_F_RXCSUM;
+
+	/* fake NETIF_F_HW_VLAN_RX for good GRO performance */
+	netdev->hw_features |= NETIF_F_HW_VLAN_RX;
+
  	netdev->features = netdev->hw_features;

  	if (dac_enabled)
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH net-next 3/3] myri10ge: Use skb_fill_page_desc().
From: Andrew Gallatin @ 2012-11-14 13:06 UTC (permalink / raw)
  To: netdev


Now that LRO is gone, the receive routine is much simpler, and
we are able to use the standard skb_fill_page_desc() in myri10ge.

Signed-off-by: Andrew Gallatin <gallatin@myri.com>
---
  drivers/net/ethernet/myricom/myri10ge/myri10ge.c |   11 ++++-------
  1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/myricom/myri10ge/myri10ge.c 
b/drivers/net/ethernet/myricom/myri10ge/myri10ge.c
index b9b6dfd..82e9dcc 100644
--- a/drivers/net/ethernet/myricom/myri10ge/myri10ge.c
+++ b/drivers/net/ethernet/myricom/myri10ge/myri10ge.c
@@ -1347,17 +1347,14 @@ myri10ge_rx_done(struct myri10ge_slice_state 
*ss, int len, __wsum csum)
  	/* Fill skb_frag_struct(s) with data from our receive */
  	for (i = 0, remainder = len; remainder > 0; i++) {
  		myri10ge_unmap_rx_page(pdev, &rx->info[idx], bytes);
-		__skb_frag_set_page(&rx_frags[i], rx->info[idx].page);
-		rx_frags[i].page_offset = rx->info[idx].page_offset;
-		if (remainder < MYRI10GE_ALLOC_SIZE)
-			skb_frag_size_set(&rx_frags[i], remainder);
-		else
-			skb_frag_size_set(&rx_frags[i], MYRI10GE_ALLOC_SIZE);
+		skb_fill_page_desc(skb, i, rx->info[idx].page,
+				   rx->info[idx].page_offset,
+				   remainder < MYRI10GE_ALLOC_SIZE ?
+				   remainder : MYRI10GE_ALLOC_SIZE);
  		rx->cnt++;
  		idx = rx->cnt & rx->mask;
  		remainder -= MYRI10GE_ALLOC_SIZE;
  	}
-	skb_shinfo(skb)->nr_frags = i;

  	/* remove padding */
  	rx_frags[0].page_offset += MXGEFW_PAD;
-- 
1.7.9.5

^ permalink raw reply related

* Re: [RFC PATCH 03/13] net: Add net protocol offload registration infrustructure
From: Vlad Yasevich @ 2012-11-14 13:08 UTC (permalink / raw)
  To: nicolas.dichtel; +Cc: netdev, eric.dumazet, davem
In-Reply-To: <50A354C6.20805@6wind.com>

On 11/14/2012 03:22 AM, Nicolas Dichtel wrote:
> Le 14/11/2012 02:24, Vlad Yasevich a écrit :
>> diff --git a/net/ipv4/protocol.c b/net/ipv4/protocol.c
>> index 8918eff..1278db8 100644
>> --- a/net/ipv4/protocol.c
>> +++ b/net/ipv4/protocol.c
>> @@ -29,6 +29,7 @@
>>   #include <net/protocol.h>
>>
>>   const struct net_protocol __rcu *inet_protos[MAX_INET_PROTOS]
>> __read_mostly;
>> +const struct net_offload __rcu *inet_offloads[MAX_INET_PROTOS]
>> __read_mostly;
>>
>>   /*
>>    *    Add a protocol handler to the hash tables
>> @@ -41,6 +42,13 @@ int inet_add_protocol(const struct net_protocol
>> *prot, unsigned char protocol)
>>   }
>>   EXPORT_SYMBOL(inet_add_protocol);
>>
>> +int inet_add_offload(const struct net_offload *prot, unsigned char
>> protocol)
>> +{
>> +    return !cmpxchg((const struct net_offload
>> **)&inet_offloads[protocol],
>> +            NULL, prot) ? 0 : -1;
>> +}
>> +EXPORT_SYMBOL(inet_add_offload);
>> +
>>   /*
>>    *    Remove a protocol from the hash tables.
>>    */
>> @@ -56,4 +64,16 @@ int inet_del_protocol(const struct net_protocol
>> *prot, unsigned char protocol)
>>
>>       return ret;
>>   }
>> -EXPORT_SYMBOL(inet_del_protocol);
> This line should probably not be removed ;-)

Yep,  good catch... thanks...

-vald

^ permalink raw reply

* Re: [RFC PATCH 00/13] Always build GSO/GRO functionality into the kernel
From: Vlad Yasevich @ 2012-11-14 13:10 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev, davem
In-Reply-To: <1352859928.4497.1.camel@edumazet-glaptop>

On 11/13/2012 09:25 PM, Eric Dumazet wrote:
> On Tue, 2012-11-13 at 20:24 -0500, Vlad Yasevich wrote:
>> This patch series is a revision suggested by Eric to solve the problem where
>> a host without IPv6 support drops GSO frames from the guest.
>>
>> The problem is that GSO/GRO support is per protocol, and when said protocol
>> is not loaded or is disabled, packets attempting to go through GSO/GRO code paths
>> are dropped.  This causes retransmissions and a two orders of magnitude drop in
>> performance.
>>
>> Prior attempt to solve the problem simply enabled enough GSO/GRO functionality
>> for IPv6 protocol when IPv6 was diabled.  This did not solve the problem when
>> the protocol was not build in or was blacklisted.
>> To solve the problem, it was suggested that we separate GSO/GRO callback
>> registration from packet processing registrations.  That way
>> GSO/GRO callbacks can be built into the kernel and always be there.
>> This patch series attempts to do just that.
>> * Patches 1 and 2 split the GSO/GRO handlers from packet_type and convert
>>    to the new structure.
>> * Patches 3, 4 and 5 do the same thing for net_protocol structure.
>> * The rest of the patches try to incrementally move the IPv6 GSO/GRO
>>    code out of the module and into the static kernel build.  Some IPv6
>>    helper functions also had to move as well.
>>
>> I am currently testing the patches, but if folks could look this over
>> and send me any comments, I'd appreciate it.
>
> Seems very nice !
>
> I am just wondering if GSO/GRO is fully enabled at every step ?
>

I think so.  I ran basic tests along most of the steps and it seemed to 
be enabled.  That's why this is a 13 patch series :)  Tried to do it 
incrementally without impacting functionality.

-vlad

^ permalink raw reply

* [PATCH net-next 1/3] myri10ge: Convert from LRO to GRO
From: Andrew Gallatin @ 2012-11-14 13:06 UTC (permalink / raw)
  To: netdev



Convert myri10ge from LRO to GRO, and simplify the driver by removing
various LRO-related code which is no longer needed including
ndo_fix_features op, custom skb building from frags, and LRO
header parsing.

Signed-off-by: Andrew Gallatin <gallatin@myri.com>
---
  drivers/net/ethernet/myricom/Kconfig             |    1 -
  drivers/net/ethernet/myricom/myri10ge/myri10ge.c |  227 
+++-------------------
  2 files changed, 31 insertions(+), 197 deletions(-)

diff --git a/drivers/net/ethernet/myricom/Kconfig 
b/drivers/net/ethernet/myricom/Kconfig
index 540f0c6..3932d08 100644
--- a/drivers/net/ethernet/myricom/Kconfig
+++ b/drivers/net/ethernet/myricom/Kconfig
@@ -23,7 +23,6 @@ config MYRI10GE
  	depends on PCI && INET
  	select FW_LOADER
  	select CRC32
-	select INET_LRO
  	---help---
  	  This driver supports Myricom Myri-10G Dual Protocol interface in
  	  Ethernet mode. If the eeprom on your board is not recent enough,
diff --git a/drivers/net/ethernet/myricom/myri10ge/myri10ge.c 
b/drivers/net/ethernet/myricom/myri10ge/myri10ge.c
index 83516e3..a5ab2f2 100644
--- a/drivers/net/ethernet/myricom/myri10ge/myri10ge.c
+++ b/drivers/net/ethernet/myricom/myri10ge/myri10ge.c
@@ -50,7 +50,6 @@
  #include <linux/etherdevice.h>
  #include <linux/if_ether.h>
  #include <linux/if_vlan.h>
-#include <linux/inet_lro.h>
  #include <linux/dca.h>
  #include <linux/ip.h>
  #include <linux/inet.h>
@@ -96,8 +95,6 @@ MODULE_LICENSE("Dual BSD/GPL");

  #define MYRI10GE_EEPROM_STRINGS_SIZE 256
  #define MYRI10GE_MAX_SEND_DESC_TSO ((65536 / 2048) * 2)
-#define MYRI10GE_MAX_LRO_DESCRIPTORS 8
-#define MYRI10GE_LRO_MAX_PKTS 64

  #define MYRI10GE_NO_CONFIRM_DATA htonl(0xffffffff)
  #define MYRI10GE_NO_RESPONSE_RESULT 0xffffffff
@@ -165,8 +162,6 @@ struct myri10ge_rx_done {
  	dma_addr_t bus;
  	int cnt;
  	int idx;
-	struct net_lro_mgr lro_mgr;
-	struct net_lro_desc lro_desc[MYRI10GE_MAX_LRO_DESCRIPTORS];
  };

  struct myri10ge_slice_netstats {
@@ -338,11 +333,6 @@ static int myri10ge_debug = -1;	/* defaults above */
  module_param(myri10ge_debug, int, 0);
  MODULE_PARM_DESC(myri10ge_debug, "Debug level (0=none,...,16=all)");

-static int myri10ge_lro_max_pkts = MYRI10GE_LRO_MAX_PKTS;
-module_param(myri10ge_lro_max_pkts, int, S_IRUGO);
-MODULE_PARM_DESC(myri10ge_lro_max_pkts,
-		 "Number of LRO packets to be aggregated");
-
  static int myri10ge_fill_thresh = 256;
  module_param(myri10ge_fill_thresh, int, S_IRUGO | S_IWUSR);
  MODULE_PARM_DESC(myri10ge_fill_thresh, "Number of empty rx slots 
allowed");
@@ -1197,36 +1187,6 @@ static inline void myri10ge_vlan_ip_csum(struct 
sk_buff *skb, __wsum hw_csum)
  	}
  }

-static inline void
-myri10ge_rx_skb_build(struct sk_buff *skb, u8 * va,
-		      struct skb_frag_struct *rx_frags, int len, int hlen)
-{
-	struct skb_frag_struct *skb_frags;
-
-	skb->len = skb->data_len = len;
-	/* attach the page(s) */
-
-	skb_frags = skb_shinfo(skb)->frags;
-	while (len > 0) {
-		memcpy(skb_frags, rx_frags, sizeof(*skb_frags));
-		len -= skb_frag_size(rx_frags);
-		skb_frags++;
-		rx_frags++;
-		skb_shinfo(skb)->nr_frags++;
-	}
-
-	/* pskb_may_pull is not available in irq context, but
-	 * skb_pull() (for ether_pad and eth_type_trans()) requires
-	 * the beginning of the packet in skb_headlen(), move it
-	 * manually */
-	skb_copy_to_linear_data(skb, va, hlen);
-	skb_shinfo(skb)->frags[0].page_offset += hlen;
-	skb_frag_size_sub(&skb_shinfo(skb)->frags[0], hlen);
-	skb->data_len -= hlen;
-	skb->tail += hlen;
-	skb_pull(skb, MXGEFW_PAD);
-}
-
  static void
  myri10ge_alloc_rx_pages(struct myri10ge_priv *mgp, struct 
myri10ge_rx_buf *rx,
  			int bytes, int watchdog)
@@ -1304,18 +1264,14 @@ myri10ge_unmap_rx_page(struct pci_dev *pdev,
  	}
  }

-#define MYRI10GE_HLEN 64	/* The number of bytes to copy from a
-				 * page into an skb */
-
  static inline int
-myri10ge_rx_done(struct myri10ge_slice_state *ss, int len, __wsum csum,
-		 bool lro_enabled)
+myri10ge_rx_done(struct myri10ge_slice_state *ss, int len, __wsum csum)
  {
  	struct myri10ge_priv *mgp = ss->mgp;
  	struct sk_buff *skb;
-	struct skb_frag_struct rx_frags[MYRI10GE_MAX_FRAGS_PER_FRAME];
+	struct skb_frag_struct *rx_frags;
  	struct myri10ge_rx_buf *rx;
-	int i, idx, hlen, remainder, bytes;
+	int i, idx, remainder, bytes;
  	struct pci_dev *pdev = mgp->pdev;
  	struct net_device *dev = mgp->dev;
  	u8 *va;
@@ -1332,6 +1288,20 @@ myri10ge_rx_done(struct myri10ge_slice_state *ss, 
int len, __wsum csum,
  	idx = rx->cnt & rx->mask;
  	va = page_address(rx->info[idx].page) + rx->info[idx].page_offset;
  	prefetch(va);
+
+	skb = napi_get_frags(&ss->napi);
+	if (unlikely(skb == NULL)) {
+		ss->stats.rx_dropped++;
+		for (i = 0, remainder = len; remainder > 0; i++) {
+			myri10ge_unmap_rx_page(pdev, &rx->info[idx], bytes);
+			put_page(rx->info[idx].page);
+			rx->cnt++;
+			idx = rx->cnt & rx->mask;
+			remainder -= MYRI10GE_ALLOC_SIZE;
+		}
+		return 0;
+	}
+	rx_frags = skb_shinfo(skb)->frags;
  	/* Fill skb_frag_struct(s) with data from our receive */
  	for (i = 0, remainder = len; remainder > 0; i++) {
  		myri10ge_unmap_rx_page(pdev, &rx->info[idx], bytes);
@@ -1345,54 +1315,23 @@ myri10ge_rx_done(struct myri10ge_slice_state 
*ss, int len, __wsum csum,
  		idx = rx->cnt & rx->mask;
  		remainder -= MYRI10GE_ALLOC_SIZE;
  	}
+	skb_shinfo(skb)->nr_frags = i;

-	if (lro_enabled) {
-		rx_frags[0].page_offset += MXGEFW_PAD;
-		skb_frag_size_sub(&rx_frags[0], MXGEFW_PAD);
-		len -= MXGEFW_PAD;
-		lro_receive_frags(&ss->rx_done.lro_mgr, rx_frags,
-				  /* opaque, will come back in get_frag_header */
-				  len, len,
-				  (void *)(__force unsigned long)csum, csum);
+	/* remove padding */
+	rx_frags[0].page_offset += MXGEFW_PAD;
+	rx_frags[0].size -= MXGEFW_PAD;
+	len -= MXGEFW_PAD;

-		return 1;
-	}
-
-	hlen = MYRI10GE_HLEN > len ? len : MYRI10GE_HLEN;
-
-	/* allocate an skb to attach the page(s) to. This is done
-	 * after trying LRO, so as to avoid skb allocation overheads */
-
-	skb = netdev_alloc_skb(dev, MYRI10GE_HLEN + 16);
-	if (unlikely(skb == NULL)) {
-		ss->stats.rx_dropped++;
-		do {
-			i--;
-			__skb_frag_unref(&rx_frags[i]);
-		} while (i != 0);
-		return 0;
-	}
-
-	/* Attach the pages to the skb, and trim off any padding */
-	myri10ge_rx_skb_build(skb, va, rx_frags, len, hlen);
-	if (skb_frag_size(&skb_shinfo(skb)->frags[0]) <= 0) {
-		skb_frag_unref(skb, 0);
-		skb_shinfo(skb)->nr_frags = 0;
-	} else {
-		skb->truesize += bytes * skb_shinfo(skb)->nr_frags;
+	skb->len = len;
+	skb->data_len = len;
+	skb->truesize += len;
+	if (dev->features & NETIF_F_RXCSUM) {
+		skb->ip_summed = CHECKSUM_COMPLETE;
+		skb->csum = csum;
  	}
-	skb->protocol = eth_type_trans(skb, dev);
  	skb_record_rx_queue(skb, ss - &mgp->ss[0]);

-	if (dev->features & NETIF_F_RXCSUM) {
-		if ((skb->protocol == htons(ETH_P_IP)) ||
-		    (skb->protocol == htons(ETH_P_IPV6))) {
-			skb->csum = csum;
-			skb->ip_summed = CHECKSUM_COMPLETE;
-		} else
-			myri10ge_vlan_ip_csum(skb, csum);
-	}
-	netif_receive_skb(skb);
+	napi_gro_frags(&ss->napi);
  	return 1;
  }

@@ -1480,18 +1419,11 @@ myri10ge_clean_rx_done(struct 
myri10ge_slice_state *ss, int budget)
  	u16 length;
  	__wsum checksum;

-	/*
-	 * Prevent compiler from generating more than one ->features memory
-	 * access to avoid theoretical race condition with functions that
-	 * change NETIF_F_LRO flag at runtime.
-	 */
-	bool lro_enabled = !!(ACCESS_ONCE(mgp->dev->features) & NETIF_F_LRO);
-
  	while (rx_done->entry[idx].length != 0 && work_done < budget) {
  		length = ntohs(rx_done->entry[idx].length);
  		rx_done->entry[idx].length = 0;
  		checksum = csum_unfold(rx_done->entry[idx].checksum);
-		rx_ok = myri10ge_rx_done(ss, length, checksum, lro_enabled);
+		rx_ok = myri10ge_rx_done(ss, length, checksum);
  		rx_packets += rx_ok;
  		rx_bytes += rx_ok * (unsigned long)length;
  		cnt++;
@@ -1503,9 +1435,6 @@ myri10ge_clean_rx_done(struct myri10ge_slice_state 
*ss, int budget)
  	ss->stats.rx_packets += rx_packets;
  	ss->stats.rx_bytes += rx_bytes;

-	if (lro_enabled)
-		lro_flush_all(&rx_done->lro_mgr);
-
  	/* restock receive rings if needed */
  	if (ss->rx_small.fill_cnt - ss->rx_small.cnt < myri10ge_fill_thresh)
  		myri10ge_alloc_rx_pages(mgp, &ss->rx_small,
@@ -1779,7 +1708,6 @@ static const char 
myri10ge_gstrings_slice_stats[][ETH_GSTRING_LEN] = {
  	"tx_pkt_start", "tx_pkt_done", "tx_req", "tx_done",
  	"rx_small_cnt", "rx_big_cnt",
  	"wake_queue", "stop_queue", "tx_linearized",
-	"LRO aggregated", "LRO flushed", "LRO avg aggr", "LRO no_desc",
  };

  #define MYRI10GE_NET_STATS_LEN      21
@@ -1880,14 +1808,6 @@ myri10ge_get_ethtool_stats(struct net_device *netdev,
  		data[i++] = (unsigned int)ss->tx.wake_queue;
  		data[i++] = (unsigned int)ss->tx.stop_queue;
  		data[i++] = (unsigned int)ss->tx.linearized;
-		data[i++] = ss->rx_done.lro_mgr.stats.aggregated;
-		data[i++] = ss->rx_done.lro_mgr.stats.flushed;
-		if (ss->rx_done.lro_mgr.stats.flushed)
-			data[i++] = ss->rx_done.lro_mgr.stats.aggregated /
-			    ss->rx_done.lro_mgr.stats.flushed;
-		else
-			data[i++] = 0;
-		data[i++] = ss->rx_done.lro_mgr.stats.no_desc;
  	}
  }

@@ -2271,67 +2191,6 @@ static void myri10ge_free_irq(struct 
myri10ge_priv *mgp)
  		pci_disable_msix(pdev);
  }

-static int
-myri10ge_get_frag_header(struct skb_frag_struct *frag, void **mac_hdr,
-			 void **ip_hdr, void **tcpudp_hdr,
-			 u64 * hdr_flags, void *priv)
-{
-	struct ethhdr *eh;
-	struct vlan_ethhdr *veh;
-	struct iphdr *iph;
-	u8 *va = skb_frag_address(frag);
-	unsigned long ll_hlen;
-	/* passed opaque through lro_receive_frags() */
-	__wsum csum = (__force __wsum) (unsigned long)priv;
-
-	/* find the mac header, aborting if not IPv4 */
-
-	eh = (struct ethhdr *)va;
-	*mac_hdr = eh;
-	ll_hlen = ETH_HLEN;
-	if (eh->h_proto != htons(ETH_P_IP)) {
-		if (eh->h_proto == htons(ETH_P_8021Q)) {
-			veh = (struct vlan_ethhdr *)va;
-			if (veh->h_vlan_encapsulated_proto != htons(ETH_P_IP))
-				return -1;
-
-			ll_hlen += VLAN_HLEN;
-
-			/*
-			 *  HW checksum starts ETH_HLEN bytes into
-			 *  frame, so we must subtract off the VLAN
-			 *  header's checksum before csum can be used
-			 */
-			csum = csum_sub(csum, csum_partial(va + ETH_HLEN,
-							   VLAN_HLEN, 0));
-		} else {
-			return -1;
-		}
-	}
-	*hdr_flags = LRO_IPV4;
-
-	iph = (struct iphdr *)(va + ll_hlen);
-	*ip_hdr = iph;
-	if (iph->protocol != IPPROTO_TCP)
-		return -1;
-	if (ip_is_fragment(iph))
-		return -1;
-	*hdr_flags |= LRO_TCP;
-	*tcpudp_hdr = (u8 *) (*ip_hdr) + (iph->ihl << 2);
-
-	/* verify the IP checksum */
-	if (unlikely(ip_fast_csum((u8 *) iph, iph->ihl)))
-		return -1;
-
-	/* verify the  checksum */
-	if (unlikely(csum_tcpudp_magic(iph->saddr, iph->daddr,
-				       ntohs(iph->tot_len) - (iph->ihl << 2),
-				       IPPROTO_TCP, csum)))
-		return -1;
-
-	return 0;
-}
-
  static int myri10ge_get_txrx(struct myri10ge_priv *mgp, int slice)
  {
  	struct myri10ge_cmd cmd;
@@ -2402,7 +2261,6 @@ static int myri10ge_open(struct net_device *dev)
  	struct myri10ge_cmd cmd;
  	int i, status, big_pow2, slice;
  	u8 *itable;
-	struct net_lro_mgr *lro_mgr;

  	if (mgp->running != MYRI10GE_ETH_STOPPED)
  		return -EBUSY;
@@ -2513,19 +2371,6 @@ static int myri10ge_open(struct net_device *dev)
  			goto abort_with_rings;
  		}

-		lro_mgr = &ss->rx_done.lro_mgr;
-		lro_mgr->dev = dev;
-		lro_mgr->features = LRO_F_NAPI;
-		lro_mgr->ip_summed = CHECKSUM_COMPLETE;
-		lro_mgr->ip_summed_aggr = CHECKSUM_UNNECESSARY;
-		lro_mgr->max_desc = MYRI10GE_MAX_LRO_DESCRIPTORS;
-		lro_mgr->lro_arr = ss->rx_done.lro_desc;
-		lro_mgr->get_frag_header = myri10ge_get_frag_header;
-		lro_mgr->max_aggr = myri10ge_lro_max_pkts;
-		lro_mgr->frag_align_pad = 2;
-		if (lro_mgr->max_aggr > MAX_SKB_FRAGS)
-			lro_mgr->max_aggr = MAX_SKB_FRAGS;
-
  		/* must happen prior to any irq */
  		napi_enable(&(ss)->napi);
  	}
@@ -3143,15 +2988,6 @@ static int myri10ge_set_mac_address(struct 
net_device *dev, void *addr)
  	return 0;
  }

-static netdev_features_t myri10ge_fix_features(struct net_device *dev,
-	netdev_features_t features)
-{
-	if (!(features & NETIF_F_RXCSUM))
-		features &= ~NETIF_F_LRO;
-
-	return features;
-}
-
  static int myri10ge_change_mtu(struct net_device *dev, int new_mtu)
  {
  	struct myri10ge_priv *mgp = netdev_priv(dev);
@@ -3878,7 +3714,6 @@ static const struct net_device_ops 
myri10ge_netdev_ops = {
  	.ndo_get_stats64	= myri10ge_get_stats,
  	.ndo_validate_addr	= eth_validate_addr,
  	.ndo_change_mtu		= myri10ge_change_mtu,
-	.ndo_fix_features	= myri10ge_fix_features,
  	.ndo_set_rx_mode	= myri10ge_set_multicast_list,
  	.ndo_set_mac_address	= myri10ge_set_mac_address,
  };
@@ -4018,7 +3853,7 @@ static int myri10ge_probe(struct pci_dev *pdev, 
const struct pci_device_id *ent)

  	netdev->netdev_ops = &myri10ge_netdev_ops;
  	netdev->mtu = myri10ge_initial_mtu;
-	netdev->hw_features = mgp->features | NETIF_F_LRO | NETIF_F_RXCSUM;
+	netdev->hw_features = mgp->features | NETIF_F_RXCSUM;
  	netdev->features = netdev->hw_features;

  	if (dac_enabled)
-- 
1.7.9.5

^ permalink raw reply related

* iputils-s20121114
From: yoshfuji @ 2012-11-14 13:50 UTC (permalink / raw)
  To: netdev

Hello.

iputils-s20121114 comes with a lot of bug fixes and improvements, thanks to
distro bug reports.

Files:
	https://sourceforge.net/projects/iputils/files/
	http://www.skbuff.net/iputils/
Tree:
	http://www.linux-ipv6.org/gitweb/gitweb.cgi?p=gitroot/iputils.git
	https://sourceforge.net/p/iputils/code/ci/HEAD/tree/

Regards,

--yoshfuji

Changes between s20121106 and s20121112:

Sergey Fionov (1):
      ping,ping6: Fallback to numeric addresses while exiting

YOSHIFUJI Hideaki (19):
      ping,ping6: Rework capability support and Make sure -m and -I options work.
      ping,tracepath: Spelling fixes in manpages.
      ping,ping6: Fix integer overflow with large interval value (-i option).
      clockdiff: Make it work with large pid.
      ping,ping6: Make in_pr_addr volatile.
      arping: Do not quit too early with large deadline value (-w option).
      arping: Maintain minimum capabilities for SO_BINDTODEVICE(-I option).
      ping: Fix recorded route comparison.
      arping: Use getifaddrs() to get broadcast address.
      ping6: Fix typo in error message.
      ping6: Generate NI Group Address and Subject Name at once.
      ping,ping6: Unmask signals on start-up.
      arping: Build with USE_CAP=no.
      arping,ping,ping6,tracepath,tracepath6,traceroute6: Experimental IDN support.
      ping6: IDN support for the Subject Name in NI Query.
      tracepath,tracepath6: Introduce -p option for port.
      ping6: Add missing definitions/declarations for flowlabel management (-F option).
      makefile: Do not include merge commits in RELNOTES.
      iputils-s20121112

Changes since iputils-s20121112:

Jan Synacek (2):
      clockdiff: remove unused variable
      ping: Wrap SO_BINDTODEVICE with the correct capability.

YOSHIFUJI Hideaki (14):
      ping: IP_MULTICAST_IF does not need CAP_NET_RAW.
      ping6: Check ranges of flowlabel (-F option) and tclass (-Q option) arguments.
      ping6: Accept 0x-notation for flowlabel (-F option) and tclass (-Q option) arguments.
      ping,ping6: Manual update regarding -F, -Q and -N option.
      arping,ping,ping6: Defer exitting to allow users to see usage.
      arping,ping,ping6,ninfod: Change euid to uid (non-root) even if capabiliy is enabled.
      ninfod: Add configure.
      ninfod: libcap support to drop capabilities.
      ninfod: Add run as user (-u user) option.
      ninfod: Fix usage message.
      arping,clockdiff,rarpd,rdisc,tftpd: Change RFC source to tools.ietf.org.
      ninfod: Add ninfod(8) manpage.
      makefile: Add ninfod, distclean targets.
      iputils-s20121114

^ permalink raw reply

* Re: [PATCHES] Networking fixes for -stable.
From: Peter Senna Tschudin @ 2012-11-14 14:02 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: David Miller, Greg Kroah-Hartman, stable, netdev
In-Reply-To: <1352836286.16264.102.camel@deadeye.wl.decadent.org.uk>

Hi Ben,

On Tue, Nov 13, 2012 at 8:51 PM, Ben Hutchings <ben@decadent.org.uk> wrote:
> On Mon, 2012-11-12 at 00:25 -0500, David Miller wrote:
>> Please queue up the following networking bug fixes for
>> 3.0.x, 3.2.x, 3.4.x, and 3.6.x, respectively.
> [...]
>> From 2204849a85383fbde75680aa199142abe504adbb Mon Sep 17 00:00:00 2001
>> From: Peter Senna Tschudin <peter.senna@gmail.com>
>> Date: Sun, 28 Oct 2012 06:12:01 +0000
>> Subject: [PATCH 7/9] drivers/net/phy/mdio-bitbang.c: Call mdiobus_unregister
>>  before mdiobus_free
>>
>> [ Upstream commit aa731872f7d33dcb8b54dad0cfb82d4e4d195d7e ]
>>
>> Based on commit b27393aecf66199f5ddad37c302d3e0cfadbe6c0
>>
>> Calling mdiobus_free without calling mdiobus_unregister causes
>> BUG_ON(). This patch fixes the issue.
> [...]
>
> This introduces a regresssion, as mdiobus_unregister() is not safe to
> call if the bus isn't registered.  Registration is controlled by the
> drivers that use mdio-bitbang, and they should take care of
> unregistration too - and most of them do.

Is mdiobus_free() safe to call if the bus isn't registered?

I found calls to free_mdio_bitbang() after call to
mdiobus_unregister() that may be the worst cases for my patch. But I
also found places that call free_mdio_bitbang() without first calling
mdiobus_unregister(). See:

linux-next/arch/powerpc/platforms/82xx/ep8248e.c:152
linux-next/drivers/net/ethernet/8390/ax88796.c:647
linux-next/drivers/net/ethernet/freescale/fs_enet/mii-bitbang.c:197
linux-next/drivers/net/phy/mdio-gpio.c:157
linux-next/drivers/net/phy/mdio-gpio.c:172

Are those calls to free_mdio_bitbang() wrong by not calling
mdiobus_unregister() before?

>
> This should be reverted in mainline and not applied to any stable
> series.
>
> Ben.
>
> --
> Ben Hutchings
> For every complex problem
> there is a solution that is simple, neat, and wrong.

Thank you for reviewing,

Peter

--
Peter

^ permalink raw reply

* Re: [PATCH v6 2/7] net: mvneta: driver for Marvell Armada 370/XP network unit
From: Thomas Petazzoni @ 2012-11-14 14:04 UTC (permalink / raw)
  To: Francois Romieu
  Cc: David S. Miller, Lennert Buytenhek, netdev, linux-arm-kernel,
	Jason Cooper, Andrew Lunn, Gregory Clement, Lior Amsalem,
	Dmitri Epshtein
In-Reply-To: <20121113231248.GA30391@electric-eye.fr.zoreil.com>

Francois,

Thanks again for your detailed review. I have a few comments/questions
below.

On Wed, 14 Nov 2012 00:12:48 +0100, Francois Romieu wrote:
> Thomas Petazzoni <thomas.petazzoni@free-electrons.com> :
> [...]
> > +static int mvneta_mac_addr_set(struct mvneta_port *pp, unsigned
> > char *addr,
> > +			       int queue)
> > +{
> > +	unsigned int mac_h;
> > +	unsigned int mac_l;
> > +
> > +	if (queue >= 1) {
> > +		netdev_err(pp->dev, "RX queue #%d is out of
> > range\n", queue);
> > +		return -EINVAL;
> > +	}
> 
> (whence q <= 0)

After changing the code as per your below comments, I have removed this
check.

> > +
> > +	if (queue != -1) {
> > +		mac_l = (addr[4] << 8) | (addr[5]);
> > +		mac_h = (addr[0] << 24) | (addr[1] << 16) |
> > +			(addr[2] << 8) | (addr[3] << 0);
> > +
> > +		mvreg_write(pp, MVNETA_MAC_ADDR_LOW, mac_l);
> > +		mvreg_write(pp, MVNETA_MAC_ADDR_HIGH, mac_h);
> > +	}
> 
> What is it trying to achieve with the "queue" argument ?

Basically: queue == -1 is a special value to say "I want to discard the
entries in the ucast table". It's used when switching from one MAC
address to another: we remove the old entries by specifying queue=-1,
and then add the new entries by specifying queue=0.

The thing is that the driver does not yet support the multiqueue
aspects of the device, but since many registers have queue-related
aspects, we still have to worry about queues a little bit in the
driver. Complete multiqueue support is planned as a second step.

> [...]
> > +/* Refill processing */
> > +static int mvneta_rx_refill(struct mvneta_port *pp,
> > +			    struct mvneta_rx_desc *rx_desc)
> > +
> > +{
> > +	dma_addr_t phys_addr;
> > +	struct sk_buff *skb;
> > +
> > +	skb = netdev_alloc_skb(pp->dev, pp->pkt_size);
> > +	if (!skb)
> > +		return -ENOMEM;
> 
> Could netdev_alloc_skb_ip_align be an option ?

If I'm correct netdev_alloc_skb_ip_align() automatically reserve the
two bytes at the beginning of the Ethernet header to ensure that the IP
header will be aligned.

However, with the Marvell hardware, it isn't needed: the hardware
automatically pads with two empty bytes in the receiving RX buffer
before putting the real data (Ethernet header).

> [...]
> > +static int mvneta_rx(struct mvneta_port *pp, int rx_todo,
> > +		     struct mvneta_rx_queue *rxq)
> > +{
> > +	struct net_device *dev = pp->dev;
> > +	int rx_done, rx_filled;
> > +
> > +	/* Get number of received packets */
> > +	rx_done = mvneta_rxq_busy_desc_num_get(pp, rxq);
> > +
> > +	if (rx_todo > rx_done)
> > +		rx_todo = rx_done;
> > +
> > +	rx_done = 0;
> > +	rx_filled = 0;
> > +
> > +	/* Fairness NAPI loop */
> > +	while (rx_done < rx_todo) {
> > +		struct mvneta_rx_desc *rx_desc =
> > mvneta_rxq_next_desc_get(rxq);
> > +		struct sk_buff *skb;
> > +		u32 rx_status;
> > +		int rx_bytes, err;
> > +
> > +		prefetch(rx_desc);
> > +		rx_done++;
> > +		rx_filled++;
> > +		rx_status = rx_desc->status;
> > +		skb = (struct sk_buff *)rx_desc->buf_cookie;
> > +
> > +		if (!mvneta_rxq_desc_is_first_last(rx_desc) ||
> > +		    (rx_status & MVNETA_RXD_ERR_SUMMARY)) {
> > +			dev->stats.rx_errors++;
> > +			mvneta_rx_error(pp, rx_desc);
> > +			mvneta_rx_desc_fill(rx_desc,
> > rx_desc->buf_phys_addr,
> > +					    (u32)skb);
> > +			continue;
> > +		}
> > +
> > +		dma_unmap_single(pp->dev->dev.parent,
> > rx_desc->buf_phys_addr,
> > +				 rx_desc->data_size,
> > DMA_FROM_DEVICE); +
> > +		rx_bytes = rx_desc->data_size -
> > +			(MVNETA_ETH_CRC_SIZE + MVNETA_MH_SIZE);
> > +		u64_stats_update_begin(&pp->rx_stats.syncp);
> > +		pp->rx_stats.packets++;
> > +		pp->rx_stats.bytes += rx_bytes;
> > +		u64_stats_update_end(&pp->rx_stats.syncp);
> > +
> > +		/* Linux processing */
> > +		skb_reserve(skb, MVNETA_MH_SIZE);
> > +		skb_put(skb, rx_bytes);
> 
> (I suggested to use skb_reserve / skb_put instead of hand-crafted
> code but I may have ignored the elephant in the living room)
> 
> I understand the skb_put, ok. What is the purpose of the skb_reserve ?

As explained above, to skip the starting 2 bytes filled with zeroes by
the Marvell hardware, before passing the SKB up to the networking stack.

> Are MVNETA_ETH_CRC_SIZE (4) and MVNETA_MH_SIZE (2) really different
> from ETH_FCS_LEN and NET_IP_ALIGN ?

I have replaced MVNETA_ETH_CRC_SIZE with ETH_FCS_LEN since they are
indeed the same. However, I am not sure using NET_IP_ALIGN directly
instead of MVNETA_MH_SIZE is correct: the Marvell Header (MH) is always
two bytes: those two bytes are filled with zeroes in the normal case,
or otherwise they are used for some custom interaction between the
Marvell Ethernet controller and specific Marvell switches (this
feature is not supported by the driver being submitted). On the other
hand, the NET_IP_ALIGN value is not necessarily zero apparently, so I
have the feeling that those things should remain two separate values.

> > +	if (napi_schedule_prep(&pp->napi))
> > +		__napi_schedule(&pp->napi);
> 
> Hand-crafted napi_schedule.

Agreed, fixed.

> > +	ret = mvneta_mac_addr_set(pp, dev->dev_addr, rxq_def);
> > +	if (ret < 0) {
> > +		netdev_err(dev, "mvneta_mac_addr_set failed\n");
> > +		goto mac_addr_set_failure;
> > +	}
> 
> AFAIU mvneta_mac_addr_set can only fail when (module parameter)
> rxq_def is not set correctly. It imho calls for a check in
> probe/remove and a removal of the failure case in mvneta_mac_addr_set.

Good point, fixed.

> > +	/* Connect to port interrupt line */
> > +	ret = request_irq(pp->dev->irq, mvneta_isr, IRQF_DISABLED,
> > +			  MVNETA_DRIVER_NAME, pp);
> 
> include/linux/interrupt.h
> [...]
>  * IRQF_DISABLED - keep irqs disabled when calling the action handler.
>  *                 DEPRECATED. This flag is a NOOP and scheduled to
> be removed

Fixed.

> > +	if (mvneta_init(pp, phy_addr)) {
> > +		dev_err(&pdev->dev, "can't init eth hal\n");
> > +		err = -ENODEV;
> > +		goto err_base;
> > +	}
> 
> The error code from mvneta_init() should be used.

Fixed.

> > +	if (register_netdev(dev)) {
> > +		dev_err(&pdev->dev, "failed to register\n");
> > +		err = ENOMEM;
> > +		goto err_base;
> > +	}
> 
> - The error code from register_netdev() should be used.
> - Leak: allocations in mvneta_init() are not balanced.
> - Nit: the "err_where_did_it_trigger_first" style of label shows its
>   downside when compared to the "err_what_should_be_done" when
>   it gets used several times.

Fixed.

> > +/* Device removal routine */
> > +static int __devexit mvneta_remove(struct platform_device *pdev)
> > +{
> > +	struct net_device  *dev = platform_get_drvdata(pdev);
> > +	struct mvneta_port *pp = netdev_priv(dev);
> > +
> > +	iounmap(pp->base);
> > +
> > +	unregister_netdev(dev);
> > +	irq_dispose_mapping(dev->irq);
> > +	free_netdev(dev);
> > +	mvneta_deinit(pp);
> 
> One may expect the same ordering as the mvneta_probe unroll sequence.

Fixed as well!

Thanks,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

^ permalink raw reply

* Re: [patch net] net: correct check in dev_addr_del()
From: Eric Dumazet @ 2012-11-14 14:18 UTC (permalink / raw)
  To: Jiri Pirko; +Cc: netdev, davem, shemminger, john.r.fastabend
In-Reply-To: <1352897464-832-1-git-send-email-jiri@resnulli.us>

On Wed, 2012-11-14 at 13:51 +0100, Jiri Pirko wrote:
> Check (ha->addr == dev->dev_addr) is always true because dev_addr_init()
> sets this. Correct the check to behave properly on addr removal.
> 
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
> ---

Please add in the changelog which commit added the bug, to ease
stable teams work.

Thanks

^ permalink raw reply

* Re: [PATCH net-next 0/3] myri10ge: LRO to GRO conversion
From: Eric Dumazet @ 2012-11-14 14:27 UTC (permalink / raw)
  To: Andrew Gallatin; +Cc: netdev
In-Reply-To: <50A39747.3030207@myri.com>

On Wed, 2012-11-14 at 08:06 -0500, Andrew Gallatin wrote:
> Hi,
> 
> The following patchset converts myri10ge from using the old inet_lro
> interface to GRO.

Very nice work, it seems INET_LRO could die eventually ;)

^ permalink raw reply

* Re: [patch net] net: correct check in dev_addr_del()
From: Jiri Pirko @ 2012-11-14 14:29 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev, davem, shemminger, john.r.fastabend
In-Reply-To: <1352902715.4497.3.camel@edumazet-glaptop>

Wed, Nov 14, 2012 at 03:18:35PM CET, eric.dumazet@gmail.com wrote:
>On Wed, 2012-11-14 at 13:51 +0100, Jiri Pirko wrote:
>> Check (ha->addr == dev->dev_addr) is always true because dev_addr_init()
>> sets this. Correct the check to behave properly on addr removal.
>> 
>> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
>> ---
>
>Please add in the changelog which commit added the bug, to ease
>stable teams work.

bug introduced by:

commit a748ee2426817a95b1f03012d8f339c45c722ae1
Author: Jiri Pirko <jpirko@redhat.com>
Date:   Thu Apr 1 21:22:09 2010 +0000

    net: move address list functions to a separate file


>
>Thanks
>
>

^ permalink raw reply

* Re: [patch net] net: correct check in dev_addr_del()
From: Eric Dumazet @ 2012-11-14 14:38 UTC (permalink / raw)
  To: Jiri Pirko; +Cc: netdev, davem, shemminger, john.r.fastabend
In-Reply-To: <20121114142941.GA2531@minipsycho.brq.redhat.com>

On Wed, 2012-11-14 at 15:29 +0100, Jiri Pirko wrote:
> Wed, Nov 14, 2012 at 03:18:35PM CET, eric.dumazet@gmail.com wrote:
> >On Wed, 2012-11-14 at 13:51 +0100, Jiri Pirko wrote:
> >> Check (ha->addr == dev->dev_addr) is always true because dev_addr_init()
> >> sets this. Correct the check to behave properly on addr removal.
> >> 
> >> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
> >> ---
> >
> >Please add in the changelog which commit added the bug, to ease
> >stable teams work.
> 
> bug introduced by:
> 
> commit a748ee2426817a95b1f03012d8f339c45c722ae1
> Author: Jiri Pirko <jpirko@redhat.com>
> Date:   Thu Apr 1 21:22:09 2010 +0000
> 
>     net: move address list functions to a separate file

Good, the usual way is then to add in the changelog :

Bug added in commit a748ee242681
(net: move address list functions to a separate file)


(No need to give the author/date, and we can shorten the SHA1 to 10 or
12 digits)

Thanks

^ permalink raw reply

* Re: [PATCH net-next 2/3] myri10ge: Add vlan rx for better GRO perf.
From: Eric Dumazet @ 2012-11-14 14:46 UTC (permalink / raw)
  To: Andrew Gallatin; +Cc: netdev
In-Reply-To: <50A3975B.7020608@myri.com>

On Wed, 2012-11-14 at 08:06 -0500, Andrew Gallatin wrote:
> 
> Unlike LRO, GRO requires that vlan tags be removed before
> aggregation can occur.  Since the myri10ge NIC does not support
> hardware vlan tag offload, we must remove the tag in the driver
> to achieve performance comparable to LRO for vlan tagged frames.
> 
> Signed-off-by: Andrew Gallatin <gallatin@myri.com>
> ---
>   drivers/net/ethernet/myricom/myri10ge/myri10ge.c |   47 
> ++++++++++++++++++++++
>   1 file changed, 47 insertions(+)
> 
> diff --git a/drivers/net/ethernet/myricom/myri10ge/myri10ge.c 
> b/drivers/net/ethernet/myricom/myri10ge/myri10ge.c
> index a5ab2f2..b9b6dfd 100644
> --- a/drivers/net/ethernet/myricom/myri10ge/myri10ge.c
> +++ b/drivers/net/ethernet/myricom/myri10ge/myri10ge.c
> @@ -1264,6 +1264,48 @@ myri10ge_unmap_rx_page(struct pci_dev *pdev,
>   	}
>   }
> 
> +/*
> + * GRO does not support acceleration of tagged vlan frames, and
> + * this NIC does not support vlan tag offload, so we must pop
> + * the tag ourselves to be able to achieve GRO performance that
> + * is comparable to LRO.
> + */
> +
> +static inline void
> +myri10ge_vlan_rx(struct net_device *dev, void *addr, struct sk_buff *skb)
> +{
> +	u8 *va;
> +	struct vlan_ethhdr *veh;
> +	struct ethhdr *eh;
> +	struct skb_frag_struct *frag;
> +	u16 proto;
> +
> +	va = addr;
> +	va += MXGEFW_PAD;
> +	veh = (struct vlan_ethhdr *) va;
> +	if ((dev->features & (NETIF_F_HW_VLAN_RX | NETIF_F_GRO)) ==
> +	    (NETIF_F_HW_VLAN_RX | NETIF_F_GRO) &&
> +	    (veh->h_vlan_proto == ntohs(ETH_P_8021Q))) {
> +		/* fixup csum if needed */
> +		if (skb->ip_summed == CHECKSUM_COMPLETE)
> +			skb->csum = csum_sub(skb->csum,
> +					     csum_partial(va + ETH_HLEN,
> +							  VLAN_HLEN, 0));
> +		/* pop tag */
> +		__vlan_hwaccel_put_tag(skb, ntohs(veh->h_vlan_TCI));
> +		proto = veh->h_vlan_encapsulated_proto;
I am not sure you need this @proto ?

> +		memmove(va + VLAN_HLEN, va, ETH_HLEN);

You could only memmove the mac addresses (2 * ETH_ALEN)
To not touch the proto (and avoid possible aliasing problems)

> +		va += VLAN_HLEN;
> +		eh = (struct ethhdr *)va;
> +		eh->h_proto = proto;

and this should not be needed ?


> +		skb->len -= VLAN_HLEN;
> +		skb->data_len -= VLAN_HLEN;
> +		frag = skb_shinfo(skb)->frags;

> +		frag->page_offset += VLAN_HLEN;
> +		skb_frag_size_set(frag, skb_frag_size(frag) - VLAN_HLEN);
> +	}
> +}
> +

^ permalink raw reply

* [PATCH v7] Network driver for the Armada 370 and Armada XP ARM Marvell SoCs
From: Thomas Petazzoni @ 2012-11-14 14:56 UTC (permalink / raw)
  To: David S. Miller
  Cc: Francois Romieu, Lennert Buytenhek, netdev, linux-arm-kernel,
	Jason Cooper, Andrew Lunn, Gregory Clement, Lior Amsalem,
	Dmitri Epshtein

David,

This patch set adds a new network driver for the network unit
available in the newest Marvell ARM SoCs Armada 370 and Armada XP, as
well as the necessary Device Tree information to use this driver in
the two evaluation platforms of those SoCs.

The previous versions of this patch set have been sent on September
4th (v1), October 11th (v2), October 23rd (v3), October 26th (v4),
November 12th (v5), November 13th (v6) and now comes the v7 of the
driver. The number of comments over the last versions have been really
small, and I would really appreciate if this driver could land into
the 3.8 kernel release.

People interested in testing this driver can find it at:

 git@github.com:MISL-EBU-System-SW/mainline-public.git marvell-neta-v7

In details:

 * Patch 1 contains a small driver for the MDIO interface of this
   Ethernet controller. Having a separate driver is useful to more
   easily handle concurrent accesses on this MDIO interface that is
   shared between all Ethernet ports.

 * Patch 3 contains the driver itself. The commit log contains a
   detailed explanation about why a new driver is needed for this new
   Marvell SoC, compared to older Marvell SoCs (Orion, Kirkwood, Dove)
   that use the mv643xx_eth driver.

 * Patch 4 adds the necessary entry to the MAINTAINERS file.

 * Patch 5 adds the SoC-level Device Tree information for Armada 370
   and Armada XP.

 * Patch 6 adds the board-level Device Tree information for the
   Marvell evaluation boards of Armada 370 and Armada XP.

 * Patch 6 adds the board-level Device Tree information for the
   PlatHome OpenBlocks AX3-4 platform (based on the Armada XP SoC).

 * Patch 7 adds the board-level Device Tree information for the
   GlobalScale Mirabox platform (based on the Armada 370 SoC).

Changes since v6:
 * Add more macros to make the MVNETA_RX_PKT_SIZE() math easier to
   understand.
 * Use ETH_FCS_LEN instead of our custom
   MVNETA_ETH_CRC_SIZE. Suggested by François Romieu.
 * Add a comment explaining why we have MVNETA_MH_SIZE.
 * Added missing newlines at the end of some error/info
   messages. Suggested by Joe Perches.
 * Removed the useless error handling of mvneta_mac_addr_set(), and
   tested once for all the value of rxq_def at probe() time. Suggested
   by Francois Romieu.
 * Use napi_schedule() instead of an hand-crafted version of
   time. Suggested by Francois Romieu.
 * Don't use IRQF_DISABLED. Suggested by Francois Romieu.
 * Fix leak in mvneta_probe() on an error path (missing call to
   mvneta_deinit()). Re-arranged the goto labels for better
   clarity. Suggested by Francois Romieu.
 * Use the return value of mvneta_init() and register_netdev() as the
   return value of the ->probe() function on failure. Suggested by
   Francois Romieu.

Changes since v5:
 * Take into account comments from François Romieu (mainly coding
   style fixes + addition of a mvneta_rxq_desc_is_first_last() helper
   function)
 * Fix a wrong argument passed to dma_alloc_coherent():
   DMA_BIDIRECTIONAL should have been GFP_KERNEL. Thanks to François
   Romieu for having pointed the issue in his review.
 * Removed calls to smp_call_function_many() that were useless.
 * Ordered alphabetically entries in the Kconfig and Makefile.

Changes since v4:
 * Added a separate MDIO driver, which allow to easily handle
   concurrent accesses to the MDIO interface.
 * The Device Tree now has separate nodes for the PHY devices, which
   belong to the MDIO bus handled by the separate MDIO driver.
 * Fix tabulation issues in some Device Tree files.
 * Rebased on top of 3.7-rc5
 * Added the Device Tree code necessary for the GlobalScale Mirabox
   platform and the PlatHome OpenBlocks AX3-4 platform.

Changes since v3:
 * Use phy_find_first() to get the correct PHY. Suggested by Florian
   Fainelli.
 * Make pp->cause_rx_tx a simple variable instead of a per-CPU array
   since it is not useful. Fixes a comment raised by David Miller.

Changes since v2:
 * Change compatible string from 'marvell,neta' to
   'marvell,armada-370-neta'. Requested by Rob Herring.
 * Rename Ethernet DT nodes from eth@... to ethernet@... Requested by
   Rob Herring.
 * Remove device_type DT property. Requested by Rob Herring.
 * Change the PHY interface for eth0/eth1 to be rgmii-id, which allows
   to enable TX/RX delay mechanisms at the PHY level. This fixes CRC
   errors on received packets during iperf tests (it was a bug in v2).
 * Remove the mvneta_ prefix from module parameters. Requested by
   Baruch Siach.
 * Many code style improvements suggested by François Romieu.
 * Properly stop/restart the TX queue when the number of TX
   descriptors available becomes low, instead of returning
   NETDEV_TX_BUSY. Requested by François Romieu.
 * Properly drop packets on the TX path when DMA mapping functions
   return an error, instead of returning NETDEV_TX_BUSY. Requested by
   François Romieu.
 * Rebased on top of Linux 3.7-rc2.

Changes since v1:
 * Reduced the Cc: list in order to make the patch set acceptable for
   the netdev@ mailing list.
 * Merge the mvneta.h contents into mvneta.c, since the header was
   only used by the driver. Requested by Arnd Bergmann.
 * Completely reorganize the organization of the register list and
   register values, in order to make it more consistent, and hopefully
   easier to read (especially easier to match register values with the
   corresponding register).
 * Integrate with the phylib, as suggested by Florian Fainelli, and
   remove the link management code that has become useless as the
   result of this integration
 * Fix many small details suggested by Florian Fainelli in his review
   of the first driver
 * Simplify various parts of the driver (descriptors array allocation,
   data structures, etc.)

Thanks,

Thomas Petazzoni

^ permalink raw reply

* [PATCH v7 1/6] net: mvmdio: new Marvell MDIO driver
From: Thomas Petazzoni @ 2012-11-14 14:56 UTC (permalink / raw)
  To: David S. Miller
  Cc: Francois Romieu, Lennert Buytenhek, netdev, linux-arm-kernel,
	Jason Cooper, Andrew Lunn, Gregory Clement, Lior Amsalem,
	Dmitri Epshtein
In-Reply-To: <1352905010-24172-1-git-send-email-thomas.petazzoni@free-electrons.com>

This patch adds a separate driver for the MDIO interface of the
Marvell Ethernet controllers. There are two reasons to have a separate
driver rather than including it inside the MAC driver itself:

 *) The MDIO interface is shared by all Ethernet ports, so a driver
    must guarantee non-concurrent accesses to this MDIO interface. The
    most logical way is to have a separate driver that handles this
    single MDIO interface, used by all Ethernet ports.

 *) The MDIO interface is the same between the existing mv643xx_eth
    driver and the new mvneta driver. Even though it is for now only
    used by the mvneta driver, it will in the future be used by the
    mv643xx_eth driver as well.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 .../devicetree/bindings/net/marvell-orion-mdio.txt |   35 +++
 drivers/net/ethernet/marvell/Kconfig               |   11 +
 drivers/net/ethernet/marvell/Makefile              |    1 +
 drivers/net/ethernet/marvell/mvmdio.c              |  230 ++++++++++++++++++++
 4 files changed, 277 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/marvell-orion-mdio.txt
 create mode 100644 drivers/net/ethernet/marvell/mvmdio.c

diff --git a/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt b/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt
new file mode 100644
index 0000000..34e7aaf
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/marvell-orion-mdio.txt
@@ -0,0 +1,35 @@
+* Marvell MDIO Ethernet Controller interface
+
+The Ethernet controllers of the Marvel Kirkwood, Dove, Orion5x,
+MV78xx0, Armada 370 and Armada XP have an identical unit that provides
+an interface with the MDIO bus. This driver handles this MDIO
+interface.
+
+Required properties:
+- compatible: "marvell,orion-mdio"
+- reg: address and length of the SMI register
+
+The child nodes of the MDIO driver are the individual PHY devices
+connected to this MDIO bus. They must have a "reg" property given the
+PHY address on the MDIO bus.
+
+Example at the SoC level:
+
+mdio {
+	#address-cells = <1>;
+	#size-cells = <0>;
+	compatible = "marvell,orion-mdio";
+	reg = <0xd0072004 0x4>;
+};
+
+And at the board level:
+
+mdio {
+	phy0: ethernet-phy@0 {
+		reg = <0>;
+	};
+
+	phy1: ethernet-phy@1 {
+		reg = <1>;
+	};
+}
diff --git a/drivers/net/ethernet/marvell/Kconfig b/drivers/net/ethernet/marvell/Kconfig
index 0029934..232ccb3 100644
--- a/drivers/net/ethernet/marvell/Kconfig
+++ b/drivers/net/ethernet/marvell/Kconfig
@@ -31,6 +31,17 @@ config MV643XX_ETH
 	  Some boards that use the Discovery chipset are the Momenco
 	  Ocelot C and Jaguar ATX and Pegasos II.
 
+config MVMDIO
+	tristate "Marvell MDIO interface support"
+	---help---
+	  This driver supports the MDIO interface found in the network
+	  interface units of the Marvell EBU SoCs (Kirkwood, Orion5x,
+	  Dove, Armada 370 and Armada XP).
+
+	  For now, this driver is only needed for the MVNETA driver
+	  (used on Armada 370 and XP), but it could be used in the
+	  future by the MV643XX_ETH driver.
+
 config PXA168_ETH
 	tristate "Marvell pxa168 ethernet support"
 	depends on CPU_PXA168
diff --git a/drivers/net/ethernet/marvell/Makefile b/drivers/net/ethernet/marvell/Makefile
index 57e3234..0438599 100644
--- a/drivers/net/ethernet/marvell/Makefile
+++ b/drivers/net/ethernet/marvell/Makefile
@@ -3,6 +3,7 @@
 #
 
 obj-$(CONFIG_MV643XX_ETH) += mv643xx_eth.o
+obj-$(CONFIG_MVMDIO) += mvmdio.o
 obj-$(CONFIG_PXA168_ETH) += pxa168_eth.o
 obj-$(CONFIG_SKGE) += skge.o
 obj-$(CONFIG_SKY2) += sky2.o
diff --git a/drivers/net/ethernet/marvell/mvmdio.c b/drivers/net/ethernet/marvell/mvmdio.c
new file mode 100644
index 0000000..82fbd23
--- /dev/null
+++ b/drivers/net/ethernet/marvell/mvmdio.c
@@ -0,0 +1,230 @@
+/*
+ * Driver for the MDIO interface of Marvell network interfaces.
+ *
+ * Since the MDIO interface of Marvell network interfaces is shared
+ * between all network interfaces, having a single driver allows to
+ * handle concurrent accesses properly (you may have four Ethernet
+ * ports, but they in fact share the same SMI interface to access the
+ * MDIO bus). Moreover, this MDIO interface code is similar between
+ * the mv643xx_eth driver and the mvneta driver. For now, it is only
+ * used by the mvneta driver, but it could later be used by the
+ * mv643xx_eth driver as well.
+ *
+ * Copyright (C) 2012 Marvell
+ *
+ * Thomas Petazzoni <thomas.petazzoni@free-electrons.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/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/phy.h>
+#include <linux/of_address.h>
+#include <linux/of_mdio.h>
+#include <linux/platform_device.h>
+
+#include <asm/delay.h>
+
+#define MVMDIO_SMI_DATA_SHIFT              0
+#define MVMDIO_SMI_PHY_ADDR_SHIFT          16
+#define MVMDIO_SMI_PHY_REG_SHIFT           21
+#define MVMDIO_SMI_READ_OPERATION          BIT(26)
+#define MVMDIO_SMI_WRITE_OPERATION         0
+#define MVMDIO_SMI_READ_VALID              BIT(27)
+#define MVMDIO_SMI_BUSY                    BIT(28)
+
+struct orion_mdio_dev {
+	struct mutex lock;
+	void __iomem *smireg;
+};
+
+/*
+ * Wait for the SMI unit to be ready for another operation
+ */
+static int orion_mdio_wait_ready(struct mii_bus *bus)
+{
+	struct orion_mdio_dev *dev = bus->priv;
+	int count;
+	u32 val;
+
+	count = 0;
+	while (1) {
+		val = readl(dev->smireg);
+		if (!(val & MVMDIO_SMI_BUSY))
+			break;
+
+		if (count > 100) {
+			dev_err(bus->parent, "Timeout: SMI busy for too long\n");
+			return -ETIMEDOUT;
+		}
+
+		udelay(10);
+		count++;
+	}
+
+	return 0;
+}
+
+static int orion_mdio_read(struct mii_bus *bus, int mii_id,
+			   int regnum)
+{
+	struct orion_mdio_dev *dev = bus->priv;
+	int count;
+	u32 val;
+	int ret;
+
+	mutex_lock(&dev->lock);
+
+	ret = orion_mdio_wait_ready(bus);
+	if (ret < 0) {
+		mutex_unlock(&dev->lock);
+		return ret;
+	}
+
+	writel(((mii_id << MVMDIO_SMI_PHY_ADDR_SHIFT) |
+		(regnum << MVMDIO_SMI_PHY_REG_SHIFT)  |
+		MVMDIO_SMI_READ_OPERATION),
+	       dev->smireg);
+
+	/* Wait for the value to become available */
+	count = 0;
+	while (1) {
+		val = readl(dev->smireg);
+		if (val & MVMDIO_SMI_READ_VALID)
+			break;
+
+		if (count > 100) {
+			dev_err(bus->parent, "Timeout when reading PHY\n");
+			mutex_unlock(&dev->lock);
+			return -ETIMEDOUT;
+		}
+
+		udelay(10);
+		count++;
+	}
+
+	mutex_unlock(&dev->lock);
+
+	return val & 0xFFFF;
+}
+
+static int orion_mdio_write(struct mii_bus *bus, int mii_id,
+			    int regnum, u16 value)
+{
+	struct orion_mdio_dev *dev = bus->priv;
+	int ret;
+
+	mutex_lock(&dev->lock);
+
+	ret = orion_mdio_wait_ready(bus);
+	if (ret < 0) {
+		mutex_unlock(&dev->lock);
+		return ret;
+	}
+
+	writel(((mii_id << MVMDIO_SMI_PHY_ADDR_SHIFT) |
+		(regnum << MVMDIO_SMI_PHY_REG_SHIFT)  |
+		MVMDIO_SMI_WRITE_OPERATION            |
+		(value << MVMDIO_SMI_DATA_SHIFT)),
+	       dev->smireg);
+
+	mutex_unlock(&dev->lock);
+
+	return 0;
+}
+
+static int orion_mdio_reset(struct mii_bus *bus)
+{
+	return 0;
+}
+
+static int __devinit orion_mdio_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct mii_bus *bus;
+	struct orion_mdio_dev *dev;
+	int i, ret;
+
+	bus = mdiobus_alloc_size(sizeof(struct orion_mdio_dev));
+	if (!bus) {
+		dev_err(&pdev->dev, "Cannot allocate MDIO bus\n");
+		return -ENOMEM;
+	}
+
+	bus->name = "orion_mdio_bus";
+	bus->read = orion_mdio_read;
+	bus->write = orion_mdio_write;
+	bus->reset = orion_mdio_reset;
+	snprintf(bus->id, MII_BUS_ID_SIZE, "%s-mii",
+		 dev_name(&pdev->dev));
+	bus->parent = &pdev->dev;
+
+	bus->irq = kmalloc(sizeof(int) * PHY_MAX_ADDR, GFP_KERNEL);
+	if (!bus->irq) {
+		dev_err(&pdev->dev, "Cannot allocate PHY IRQ array\n");
+		mdiobus_free(bus);
+		return -ENOMEM;
+	}
+
+	for (i = 0; i < PHY_MAX_ADDR; i++)
+		bus->irq[i] = PHY_POLL;
+
+	dev = bus->priv;
+	dev->smireg = of_iomap(pdev->dev.of_node, 0);
+	if (!dev->smireg) {
+		dev_err(&pdev->dev, "No SMI register address given in DT\n");
+		kfree(bus->irq);
+		mdiobus_free(bus);
+		return -ENODEV;
+	}
+
+	mutex_init(&dev->lock);
+
+	ret = of_mdiobus_register(bus, np);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "Cannot register MDIO bus (%d)\n", ret);
+		iounmap(dev->smireg);
+		kfree(bus->irq);
+		mdiobus_free(bus);
+		return ret;
+	}
+
+	platform_set_drvdata(pdev, bus);
+
+	return 0;
+}
+
+static int __devexit orion_mdio_remove(struct platform_device *pdev)
+{
+	struct mii_bus *bus = platform_get_drvdata(pdev);
+	mdiobus_unregister(bus);
+	kfree(bus->irq);
+	mdiobus_free(bus);
+	return 0;
+}
+
+static const struct of_device_id orion_mdio_match[] = {
+	{ .compatible = "marvell,orion-mdio" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, orion_mdio_match);
+
+static struct platform_driver orion_mdio_driver = {
+	.probe = orion_mdio_probe,
+	.remove = __devexit_p(orion_mdio_remove),
+	.driver = {
+		.name = "orion-mdio",
+		.of_match_table = orion_mdio_match,
+	},
+};
+
+module_platform_driver(orion_mdio_driver);
+
+MODULE_DESCRIPTION("Marvell MDIO interface driver");
+MODULE_AUTHOR("Thomas Petazzoni <thomas.petazzoni@free-electrons.com>");
+MODULE_LICENSE("GPL");
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH v7 4/6] arm: mvebu: add Ethernet controllers using mvneta driver for Armada 370/XP
From: Thomas Petazzoni @ 2012-11-14 14:56 UTC (permalink / raw)
  To: David S. Miller
  Cc: Francois Romieu, Lennert Buytenhek, netdev, linux-arm-kernel,
	Jason Cooper, Andrew Lunn, Gregory Clement, Lior Amsalem,
	Dmitri Epshtein
In-Reply-To: <1352905010-24172-1-git-send-email-thomas.petazzoni@free-electrons.com>

The Armada 370 SoC has two network units, while the Armada XP has four
network units. The first two network units are common to both the
Armada XP and Armada 370, so they are added to armada-370-xp.dtsi,
while the other two network units are specific to the Armada XP and
therefore added to armada-xp.dtsi.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 arch/arm/boot/dts/armada-370-xp.dtsi |   21 +++++++++++++++++++++
 arch/arm/boot/dts/armada-xp.dtsi     |   14 ++++++++++++++
 2 files changed, 35 insertions(+)

diff --git a/arch/arm/boot/dts/armada-370-xp.dtsi b/arch/arm/boot/dts/armada-370-xp.dtsi
index b113e0b..0cd3331 100644
--- a/arch/arm/boot/dts/armada-370-xp.dtsi
+++ b/arch/arm/boot/dts/armada-370-xp.dtsi
@@ -68,6 +68,27 @@
 			compatible = "marvell,armada-addr-decoding-controller";
 			reg = <0xd0020000 0x258>;
 		};
+
+		mdio {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "marvell,orion-mdio";
+			reg = <0xd0072004 0x4>;
+		};
+
+		ethernet@d0070000 {
+				compatible = "marvell,armada-370-neta";
+				reg = <0xd0070000 0x2500>;
+				interrupts = <8>;
+				status = "disabled";
+		};
+
+		ethernet@d0074000 {
+				compatible = "marvell,armada-370-neta";
+				reg = <0xd0074000 0x2500>;
+				interrupts = <10>;
+				status = "disabled";
+		};
 	};
 };
 
diff --git a/arch/arm/boot/dts/armada-xp.dtsi b/arch/arm/boot/dts/armada-xp.dtsi
index 71d6b5d..3bbbccf 100644
--- a/arch/arm/boot/dts/armada-xp.dtsi
+++ b/arch/arm/boot/dts/armada-xp.dtsi
@@ -51,5 +51,19 @@
 				compatible = "marvell,armada-370-xp-system-controller";
 				reg = <0xd0018200 0x500>;
 		};
+
+		ethernet@d0030000 {
+				compatible = "marvell,armada-370-neta";
+				reg = <0xd0030000 0x2500>;
+				interrupts = <12>;
+				status = "disabled";
+		};
+
+		ethernet@d0034000 {
+				compatible = "marvell,armada-370-neta";
+				reg = <0xd0034000 0x2500>;
+				interrupts = <14>;
+				status = "disabled";
+		};
 	};
 };
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH v7 5/6] arm: mvebu: enable Ethernet controllers on Armada 370/XP eval boards
From: Thomas Petazzoni @ 2012-11-14 14:56 UTC (permalink / raw)
  To: David S. Miller
  Cc: Francois Romieu, Lennert Buytenhek, netdev, linux-arm-kernel,
	Jason Cooper, Andrew Lunn, Gregory Clement, Lior Amsalem,
	Dmitri Epshtein
In-Reply-To: <1352905010-24172-1-git-send-email-thomas.petazzoni@free-electrons.com>

This patch enables the two network interfaces of the Armada 370
official Marvell evaluation platform, and the four network interfaces
of the Armada XP official Marvell evaluation platform.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 arch/arm/boot/dts/armada-370-db.dts |   23 +++++++++++++++++++
 arch/arm/boot/dts/armada-xp-db.dts  |   43 +++++++++++++++++++++++++++++++++++
 2 files changed, 66 insertions(+)

diff --git a/arch/arm/boot/dts/armada-370-db.dts b/arch/arm/boot/dts/armada-370-db.dts
index fffd5c2..76362f7 100644
--- a/arch/arm/boot/dts/armada-370-db.dts
+++ b/arch/arm/boot/dts/armada-370-db.dts
@@ -38,5 +38,28 @@
 			clock-frequency = <600000000>;
 			status = "okay";
 		};
+
+		mdio {
+			phy0: ethernet-phy@0 {
+				reg = <0>;
+			};
+
+			phy1: ethernet-phy@1 {
+				reg = <1>;
+			};
+		};
+
+		ethernet@d0070000 {
+			clock-frequency = <200000000>;
+			status = "okay";
+			phy = <&phy0>;
+			phy-mode = "rgmii-id";
+		};
+		ethernet@d0074000 {
+			clock-frequency = <200000000>;
+			status = "okay";
+			phy = <&phy1>;
+			phy-mode = "rgmii-id";
+		};
 	};
 };
diff --git a/arch/arm/boot/dts/armada-xp-db.dts b/arch/arm/boot/dts/armada-xp-db.dts
index b1fc728..b614bd0 100644
--- a/arch/arm/boot/dts/armada-xp-db.dts
+++ b/arch/arm/boot/dts/armada-xp-db.dts
@@ -46,5 +46,48 @@
 			clock-frequency = <250000000>;
 			status = "okay";
 		};
+
+		mdio {
+			phy0: ethernet-phy@0 {
+				reg = <0>;
+			};
+
+			phy1: ethernet-phy@1 {
+				reg = <1>;
+			};
+
+			phy2: ethernet-phy@2 {
+				reg = <25>;
+			};
+
+			phy3: ethernet-phy@3 {
+				reg = <27>;
+			};
+		};
+
+		ethernet@d0070000 {
+			clock-frequency = <250000000>;
+			status = "okay";
+			phy = <&phy0>;
+			phy-mode = "rgmii-id";
+		};
+		ethernet@d0074000 {
+			clock-frequency = <250000000>;
+			status = "okay";
+			phy = <&phy1>;
+			phy-mode = "rgmii-id";
+		};
+		ethernet@d0030000 {
+			clock-frequency = <250000000>;
+			status = "okay";
+			phy = <&phy2>;
+			phy-mode = "sgmii";
+		};
+		ethernet@d0034000 {
+			clock-frequency = <250000000>;
+			status = "okay";
+			phy = <&phy3>;
+			phy-mode = "sgmii";
+		};
 	};
 };
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH v7 6/6] arm: mvebu: enable Ethernet controllers on OpenBlocks AX3-4 platform
From: Thomas Petazzoni @ 2012-11-14 14:56 UTC (permalink / raw)
  To: David S. Miller
  Cc: Francois Romieu, Lennert Buytenhek, netdev, linux-arm-kernel,
	Jason Cooper, Andrew Lunn, Gregory Clement, Lior Amsalem,
	Dmitri Epshtein
In-Reply-To: <1352905010-24172-1-git-send-email-thomas.petazzoni@free-electrons.com>

The PlatHome OpenBlocks AX3-4 platform has 4 Ethernet ports, connected
to a single quad-port PHY through SGMII.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 arch/arm/boot/dts/armada-xp-openblocks-ax3-4.dts |   24 ++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/arch/arm/boot/dts/armada-xp-openblocks-ax3-4.dts b/arch/arm/boot/dts/armada-xp-openblocks-ax3-4.dts
index cb86853..1a07e09 100644
--- a/arch/arm/boot/dts/armada-xp-openblocks-ax3-4.dts
+++ b/arch/arm/boot/dts/armada-xp-openblocks-ax3-4.dts
@@ -65,5 +65,29 @@
 				linux,default-trigger = "heartbeat";
 			};
 		};
+		ethernet@d0070000 {
+			clock-frequency = <250000000>;
+			status = "okay";
+			phy-mode = "sgmii";
+			phy-addr = <0>;
+		};
+		ethernet@d0074000 {
+			clock-frequency = <250000000>;
+			status = "okay";
+			phy-mode = "sgmii";
+			phy-addr = <1>;
+		};
+		ethernet@d0030000 {
+			clock-frequency = <250000000>;
+			status = "okay";
+			phy-mode = "sgmii";
+			phy-addr = <2>;
+		};
+		ethernet@d0034000 {
+			clock-frequency = <250000000>;
+			status = "okay";
+			phy-mode = "sgmii";
+			phy-addr = <3>;
+		};
 	};
 };
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH v7 3/6] net: mvneta: update MAINTAINERS file for the mvneta maintainers
From: Thomas Petazzoni @ 2012-11-14 14:56 UTC (permalink / raw)
  To: David S. Miller
  Cc: Francois Romieu, Lennert Buytenhek, netdev, linux-arm-kernel,
	Jason Cooper, Andrew Lunn, Gregory Clement, Lior Amsalem,
	Dmitri Epshtein
In-Reply-To: <1352905010-24172-1-git-send-email-thomas.petazzoni@free-electrons.com>

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 MAINTAINERS |    6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 59203e7..3010f1a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4700,6 +4700,12 @@ S:	Maintained
 F:	drivers/net/ethernet/marvell/mv643xx_eth.*
 F:	include/linux/mv643xx.h
 
+MARVELL MVNETA ETHERNET DRIVER
+M:	Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
+L:	netdev@vger.kernel.org
+S:	Maintained
+F:	drivers/net/ethernet/marvell/mvneta.*
+
 MARVELL MWIFIEX WIRELESS DRIVER
 M:	Bing Zhao <bzhao@marvell.com>
 L:	linux-wireless@vger.kernel.org
-- 
1.7.9.5

^ 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