Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next 7/7] r8152: add comments
From: Hayes Wang @ 2013-08-16  8:09 UTC (permalink / raw)
  To: romieu, netdev; +Cc: nic_swsd, linux-kernel, linux-usb, Hayes Wang
In-Reply-To: <1376640578-4258-1-git-send-email-hayeswang@realtek.com>

Add comments.

Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
 drivers/net/usb/r8152.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 825edfe..f3fce41 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -790,6 +790,9 @@ static void read_bulk_callback(struct urb *urb)
 		return;
 
 	netdev = tp->netdev;
+
+	/* When link down, the driver would cancel all bulks. */
+	/* This avoid the re-submitting bulk */
 	if (!netif_carrier_ok(netdev))
 		return;
 
@@ -1296,6 +1299,8 @@ static void bottom_half(unsigned long data)
 	if (!test_bit(WORK_ENABLE, &tp->flags))
 		return;
 
+	/* When link down, the driver would cancel all bulks. */
+	/* This avoid the re-submitting bulk */
 	if (!netif_carrier_ok(tp->netdev))
 		return;
 
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH net-next 5/7] r8152: move some declearation of variables
From: Hayes Wang @ 2013-08-16  8:09 UTC (permalink / raw)
  To: romieu-W8zweXLXuWQS+FvcfC7Uqw, netdev-u79uwXL29TY76Z2rM5mHXA
  Cc: nic_swsd-Rasf1IRRPZFBDgjK7y7TUQ,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA, Hayes Wang
In-Reply-To: <1376640578-4258-1-git-send-email-hayeswang-Rasf1IRRPZFBDgjK7y7TUQ@public.gmane.org>

Move some declearation of variables in rx_bottom().

Signed-off-by: Hayes Wang <hayeswang-Rasf1IRRPZFBDgjK7y7TUQ@public.gmane.org>
---
 drivers/net/usb/r8152.c | 29 ++++++++++++++---------------
 1 file changed, 14 insertions(+), 15 deletions(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 4dee76b..0a88f64 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -1133,25 +1133,19 @@ r8152_tx_csum(struct r8152 *tp, struct tx_desc *desc, struct sk_buff *skb)
 
 static void rx_bottom(struct r8152 *tp)
 {
-	struct net_device_stats *stats;
-	struct net_device *netdev;
-	struct rx_agg *agg;
-	struct rx_desc *rx_desc;
 	unsigned long flags;
 	struct list_head *cursor, *next;
-	struct sk_buff *skb;
-	struct urb *urb;
-	unsigned pkt_len;
-	int len_used;
-	u8 *rx_data;
-	int ret;
-
-	netdev = tp->netdev;
-
-	stats = rtl8152_get_stats(netdev);
 
 	spin_lock_irqsave(&tp->rx_lock, flags);
 	list_for_each_safe(cursor, next, &tp->rx_done) {
+		struct rx_desc *rx_desc;
+		struct rx_agg *agg;
+		unsigned pkt_len;
+		int len_used = 0;
+		struct urb *urb;
+		u8 *rx_data;
+		int ret;
+
 		list_del_init(cursor);
 		spin_unlock_irqrestore(&tp->rx_lock, flags);
 
@@ -1160,16 +1154,21 @@ static void rx_bottom(struct r8152 *tp)
 		if (urb->actual_length < ETH_ZLEN)
 			goto submit;
 
-		len_used = 0;
 		rx_desc = agg->head;
 		rx_data = agg->head;
 		pkt_len = le32_to_cpu(rx_desc->opts1) & RX_LEN_MASK;
 		len_used += sizeof(struct rx_desc) + pkt_len;
 
 		while (urb->actual_length >= len_used) {
+			struct net_device *netdev = tp->netdev;
+			struct net_device_stats *stats;
+			struct sk_buff *skb;
+
 			if (pkt_len < ETH_ZLEN)
 				break;
 
+			stats = rtl8152_get_stats(netdev);
+
 			pkt_len -= 4; /* CRC */
 			rx_data += sizeof(struct rx_desc);
 
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH net-next 4/7] r8152: adjust some duplicated code
From: Hayes Wang @ 2013-08-16  8:09 UTC (permalink / raw)
  To: romieu, netdev; +Cc: nic_swsd, linux-kernel, linux-usb, Hayes Wang
In-Reply-To: <1376640578-4258-1-git-send-email-hayeswang@realtek.com>

- Use r8152_get_tx_agg for getting tx agg list
- Replace submit rx with goto submit

Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
 drivers/net/usb/r8152.c | 55 +++++++++++++++++++++++++------------------------
 1 file changed, 28 insertions(+), 27 deletions(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 41b99ce..4dee76b 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -1067,6 +1067,24 @@ err1:
 	return -ENOMEM;
 }
 
+static struct tx_agg *r8152_get_tx_agg(struct r8152 *tp)
+{
+	struct tx_agg *agg = NULL;
+	unsigned long flags;
+
+	spin_lock_irqsave(&tp->tx_lock, flags);
+	if (!list_empty(&tp->tx_free)) {
+		struct list_head *cursor;
+
+		cursor = tp->tx_free.next;
+		list_del_init(cursor);
+		agg = list_entry(cursor, struct tx_agg, list);
+	}
+	spin_unlock_irqrestore(&tp->tx_lock, flags);
+
+	return agg;
+}
+
 static void
 r8152_tx_csum(struct r8152 *tp, struct tx_desc *desc, struct sk_buff *skb)
 {
@@ -1139,15 +1157,8 @@ static void rx_bottom(struct r8152 *tp)
 
 		agg = list_entry(cursor, struct rx_agg, list);
 		urb = agg->urb;
-		if (urb->actual_length < ETH_ZLEN) {
-			ret = r8152_submit_rx(tp, agg, GFP_ATOMIC);
-			spin_lock_irqsave(&tp->rx_lock, flags);
-			if (ret && ret != -ENODEV) {
-				list_add_tail(&agg->list, next);
-				tasklet_schedule(&tp->tl);
-			}
-			continue;
-		}
+		if (urb->actual_length < ETH_ZLEN)
+			goto submit;
 
 		len_used = 0;
 		rx_desc = agg->head;
@@ -1181,6 +1192,7 @@ static void rx_bottom(struct r8152 *tp)
 			len_used += sizeof(struct rx_desc) + pkt_len;
 		}
 
+submit:
 		ret = r8152_submit_rx(tp, agg, GFP_ATOMIC);
 		spin_lock_irqsave(&tp->rx_lock, flags);
 		if (ret && ret != -ENODEV) {
@@ -1205,16 +1217,10 @@ static void tx_bottom(struct r8152 *tp)
 
 next_agg:
 	agg = NULL;
-	spin_lock_irqsave(&tp->tx_lock, flags);
-	if (!skb_queue_empty(&tp->tx_queue) && !list_empty(&tp->tx_free)) {
-		struct list_head *cursor;
-
-		cursor = tp->tx_free.next;
-		list_del_init(cursor);
-		agg = list_entry(cursor, struct tx_agg, list);
-	}
-	spin_unlock_irqrestore(&tp->tx_lock, flags);
+	if (skb_queue_empty(&tp->tx_queue))
+		return;
 
+	agg = r8152_get_tx_agg(tp);
 	if (!agg)
 		return;
 
@@ -1382,15 +1388,10 @@ static netdev_tx_t rtl8152_start_xmit(struct sk_buff *skb,
 
 	skb_tx_timestamp(skb);
 
-	spin_lock_irqsave(&tp->tx_lock, flags);
-	if (!list_empty(&tp->tx_free) && skb_queue_empty(&tp->tx_queue)) {
-		struct list_head *cursor;
-
-		cursor = tp->tx_free.next;
-		list_del_init(cursor);
-		agg = list_entry(cursor, struct tx_agg, list);
-	}
-	spin_unlock_irqrestore(&tp->tx_lock, flags);
+	/* If tx_queue is not empty, it means at least one previous packt */
+	/* is waiting for sending. Don't send current one before it.      */
+	if (skb_queue_empty(&tp->tx_queue))
+		agg = r8152_get_tx_agg(tp);
 
 	if (!agg) {
 		skb_queue_tail(&tp->tx_queue, skb);
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH net-next 3/7] r8152: replace lockflags with flags
From: Hayes Wang @ 2013-08-16  8:09 UTC (permalink / raw)
  To: romieu, netdev; +Cc: nic_swsd, linux-kernel, linux-usb, Hayes Wang
In-Reply-To: <1376640578-4258-1-git-send-email-hayeswang@realtek.com>

Replace lockflags with flags.

Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
 drivers/net/usb/r8152.c | 48 ++++++++++++++++++++++++------------------------
 1 file changed, 24 insertions(+), 24 deletions(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index a18f02d..41b99ce 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -769,7 +769,7 @@ static struct net_device_stats *rtl8152_get_stats(struct net_device *dev)
 static void read_bulk_callback(struct urb *urb)
 {
 	struct net_device *netdev;
-	unsigned long lockflags;
+	unsigned long flags;
 	int status = urb->status;
 	struct rx_agg *agg;
 	struct r8152 *tp;
@@ -798,9 +798,9 @@ static void read_bulk_callback(struct urb *urb)
 		if (urb->actual_length < ETH_ZLEN)
 			break;
 
-		spin_lock_irqsave(&tp->rx_lock, lockflags);
+		spin_lock_irqsave(&tp->rx_lock, flags);
 		list_add_tail(&agg->list, &tp->rx_done);
-		spin_unlock_irqrestore(&tp->rx_lock, lockflags);
+		spin_unlock_irqrestore(&tp->rx_lock, flags);
 		tasklet_schedule(&tp->tl);
 		return;
 	case -ESHUTDOWN:
@@ -821,9 +821,9 @@ static void read_bulk_callback(struct urb *urb)
 	if (result == -ENODEV) {
 		netif_device_detach(tp->netdev);
 	} else if (result) {
-		spin_lock_irqsave(&tp->rx_lock, lockflags);
+		spin_lock_irqsave(&tp->rx_lock, flags);
 		list_add_tail(&agg->list, &tp->rx_done);
-		spin_unlock_irqrestore(&tp->rx_lock, lockflags);
+		spin_unlock_irqrestore(&tp->rx_lock, flags);
 		tasklet_schedule(&tp->tl);
 	}
 }
@@ -831,7 +831,7 @@ static void read_bulk_callback(struct urb *urb)
 static void write_bulk_callback(struct urb *urb)
 {
 	struct net_device_stats *stats;
-	unsigned long lockflags;
+	unsigned long flags;
 	struct tx_agg *agg;
 	struct r8152 *tp;
 	int status = urb->status;
@@ -853,9 +853,9 @@ static void write_bulk_callback(struct urb *urb)
 		stats->tx_bytes += agg->skb_len;
 	}
 
-	spin_lock_irqsave(&tp->tx_lock, lockflags);
+	spin_lock_irqsave(&tp->tx_lock, flags);
 	list_add_tail(&agg->list, &tp->tx_free);
-	spin_unlock_irqrestore(&tp->tx_lock, lockflags);
+	spin_unlock_irqrestore(&tp->tx_lock, flags);
 
 	if (!netif_carrier_ok(tp->netdev))
 		return;
@@ -1119,7 +1119,7 @@ static void rx_bottom(struct r8152 *tp)
 	struct net_device *netdev;
 	struct rx_agg *agg;
 	struct rx_desc *rx_desc;
-	unsigned long lockflags;
+	unsigned long flags;
 	struct list_head *cursor, *next;
 	struct sk_buff *skb;
 	struct urb *urb;
@@ -1132,16 +1132,16 @@ static void rx_bottom(struct r8152 *tp)
 
 	stats = rtl8152_get_stats(netdev);
 
-	spin_lock_irqsave(&tp->rx_lock, lockflags);
+	spin_lock_irqsave(&tp->rx_lock, flags);
 	list_for_each_safe(cursor, next, &tp->rx_done) {
 		list_del_init(cursor);
-		spin_unlock_irqrestore(&tp->rx_lock, lockflags);
+		spin_unlock_irqrestore(&tp->rx_lock, flags);
 
 		agg = list_entry(cursor, struct rx_agg, list);
 		urb = agg->urb;
 		if (urb->actual_length < ETH_ZLEN) {
 			ret = r8152_submit_rx(tp, agg, GFP_ATOMIC);
-			spin_lock_irqsave(&tp->rx_lock, lockflags);
+			spin_lock_irqsave(&tp->rx_lock, flags);
 			if (ret && ret != -ENODEV) {
 				list_add_tail(&agg->list, next);
 				tasklet_schedule(&tp->tl);
@@ -1182,13 +1182,13 @@ static void rx_bottom(struct r8152 *tp)
 		}
 
 		ret = r8152_submit_rx(tp, agg, GFP_ATOMIC);
-		spin_lock_irqsave(&tp->rx_lock, lockflags);
+		spin_lock_irqsave(&tp->rx_lock, flags);
 		if (ret && ret != -ENODEV) {
 			list_add_tail(&agg->list, next);
 			tasklet_schedule(&tp->tl);
 		}
 	}
-	spin_unlock_irqrestore(&tp->rx_lock, lockflags);
+	spin_unlock_irqrestore(&tp->rx_lock, flags);
 }
 
 static void tx_bottom(struct r8152 *tp)
@@ -1196,7 +1196,7 @@ static void tx_bottom(struct r8152 *tp)
 	struct net_device_stats *stats;
 	struct net_device *netdev;
 	struct tx_agg *agg;
-	unsigned long lockflags;
+	unsigned long flags;
 	u32 remain, total;
 	u8 *tx_data;
 	int res;
@@ -1205,7 +1205,7 @@ static void tx_bottom(struct r8152 *tp)
 
 next_agg:
 	agg = NULL;
-	spin_lock_irqsave(&tp->tx_lock, lockflags);
+	spin_lock_irqsave(&tp->tx_lock, flags);
 	if (!skb_queue_empty(&tp->tx_queue) && !list_empty(&tp->tx_free)) {
 		struct list_head *cursor;
 
@@ -1213,7 +1213,7 @@ next_agg:
 		list_del_init(cursor);
 		agg = list_entry(cursor, struct tx_agg, list);
 	}
-	spin_unlock_irqrestore(&tp->tx_lock, lockflags);
+	spin_unlock_irqrestore(&tp->tx_lock, flags);
 
 	if (!agg)
 		return;
@@ -1268,9 +1268,9 @@ next_agg:
 			netif_warn(tp, tx_err, netdev,
 				   "failed tx_urb %d\n", res);
 			stats->tx_dropped += agg->skb_num;
-			spin_lock_irqsave(&tp->tx_lock, lockflags);
+			spin_lock_irqsave(&tp->tx_lock, flags);
 			list_add_tail(&agg->list, &tp->tx_free);
-			spin_unlock_irqrestore(&tp->tx_lock, lockflags);
+			spin_unlock_irqrestore(&tp->tx_lock, flags);
 		}
 		return;
 	}
@@ -1373,7 +1373,7 @@ static netdev_tx_t rtl8152_start_xmit(struct sk_buff *skb,
 {
 	struct r8152 *tp = netdev_priv(netdev);
 	struct net_device_stats *stats = rtl8152_get_stats(netdev);
-	unsigned long lockflags;
+	unsigned long flags;
 	struct tx_agg *agg = NULL;
 	struct tx_desc *tx_desc;
 	unsigned int len;
@@ -1382,7 +1382,7 @@ static netdev_tx_t rtl8152_start_xmit(struct sk_buff *skb,
 
 	skb_tx_timestamp(skb);
 
-	spin_lock_irqsave(&tp->tx_lock, lockflags);
+	spin_lock_irqsave(&tp->tx_lock, flags);
 	if (!list_empty(&tp->tx_free) && skb_queue_empty(&tp->tx_queue)) {
 		struct list_head *cursor;
 
@@ -1390,7 +1390,7 @@ static netdev_tx_t rtl8152_start_xmit(struct sk_buff *skb,
 		list_del_init(cursor);
 		agg = list_entry(cursor, struct tx_agg, list);
 	}
-	spin_unlock_irqrestore(&tp->tx_lock, lockflags);
+	spin_unlock_irqrestore(&tp->tx_lock, flags);
 
 	if (!agg) {
 		skb_queue_tail(&tp->tx_queue, skb);
@@ -1419,9 +1419,9 @@ static netdev_tx_t rtl8152_start_xmit(struct sk_buff *skb,
 			netif_warn(tp, tx_err, netdev,
 				   "failed tx_urb %d\n", res);
 			stats->tx_dropped++;
-			spin_lock_irqsave(&tp->tx_lock, lockflags);
+			spin_lock_irqsave(&tp->tx_lock, flags);
 			list_add_tail(&agg->list, &tp->tx_free);
-			spin_unlock_irqrestore(&tp->tx_lock, lockflags);
+			spin_unlock_irqrestore(&tp->tx_lock, flags);
 		}
 	}
 
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH net-next 2/7] r8152: replace void * with struct r8152 *
From: Hayes Wang @ 2013-08-16  8:09 UTC (permalink / raw)
  To: romieu, netdev; +Cc: nic_swsd, linux-kernel, linux-usb, Hayes Wang
In-Reply-To: <1376640578-4258-1-git-send-email-hayeswang@realtek.com>

Change the type of contex of tx_agg and rx_agg from void * to
staruc r8152 *.

Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
 drivers/net/usb/r8152.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index c13662b..a18f02d 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -329,10 +329,12 @@ struct tx_desc {
 #define IPV6_CS			(1 << 28) /* Calculate IPv6 checksum */
 };
 
+struct r8152;
+
 struct rx_agg {
 	struct list_head list;
 	struct urb *urb;
-	void *context;
+	struct r8152 *context;
 	void *buffer;
 	void *head;
 };
@@ -340,7 +342,7 @@ struct rx_agg {
 struct tx_agg {
 	struct list_head list;
 	struct urb *urb;
-	void *context;
+	struct r8152 *context;
 	void *buffer;
 	void *head;
 	u32 skb_num;
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH net-next 1/7] r8152: remove clearing the memory to zero for netdev priv
From: Hayes Wang @ 2013-08-16  8:09 UTC (permalink / raw)
  To: romieu-W8zweXLXuWQS+FvcfC7Uqw, netdev-u79uwXL29TY76Z2rM5mHXA
  Cc: nic_swsd-Rasf1IRRPZFBDgjK7y7TUQ,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA, Hayes Wang

Remove memset(tp, 0, sizeof(*tp));

Signed-off-by: Hayes Wang <hayeswang-Rasf1IRRPZFBDgjK7y7TUQ@public.gmane.org>
---
 drivers/net/usb/r8152.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index ef2498c..c13662b 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -2105,7 +2105,6 @@ static int rtl8152_probe(struct usb_interface *intf,
 
 	SET_NETDEV_DEV(netdev, &intf->dev);
 	tp = netdev_priv(netdev);
-	memset(tp, 0, sizeof(*tp));
 	tp->msg_enable = 0x7FFF;
 
 	tasklet_init(&tp->tl, bottom_half, (unsigned long)tp);
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* Re: [PATCH v3 RESEND 5/7] net: sunhme: use platform_{get,set}_drvdata()
From: David Miller @ 2013-08-16  7:32 UTC (permalink / raw)
  To: clbchenlibo.chen
  Cc: jg1.han, wfp5p, netdev, linux-kernel, sergei.shtylyov, lizefan
In-Reply-To: <520D8F91.2060605@huawei.com>


You can't just update one patch and resend it by itself.

You must resubmit the entire series.

^ permalink raw reply

* Re: [PATCH] vhost: Drop linux/socket.h
From: David Miller @ 2013-08-16  7:31 UTC (permalink / raw)
  To: asias; +Cc: netdev, virtualization, kvm, mst
In-Reply-To: <20130816012743.GA5944@hj.localdomain>

From: Asias He <asias@redhat.com>
Date: Fri, 16 Aug 2013 09:27:43 +0800

> On Thu, Aug 15, 2013 at 02:07:40PM -0700, David Miller wrote:
>> From: Asias He <asias@redhat.com>
>> Date: Thu, 15 Aug 2013 11:20:16 +0800
>> 
>> > memcpy_fromiovec is moved to lib/iovec.c. No need to include
>> > linux/socket.h for it.
>> > 
>> > Signed-off-by: Asias He <asias@redhat.com>
>> 
>> You can't do this.
>> 
>> Because this file doesn't include the header file that
>> provides the declaration, which is linux/uio.h
> 
> vhost.c includes drivers/vhost/vhost.h. In drivers/vhost/vhost.h, we
> have linux/uio.h included.

Nothing in vhost.h needs linux/uio.h right?  That's very poor style,
include the header where the dependency exists which is vhost.c

^ permalink raw reply

* Re: [PATCH] ip monitor: Enable prefix labels when monitoring more than 1 object
From: Martin Schwenke @ 2013-08-16  6:52 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20130815234047.0018f3dd@nehalam.linuxnetplumber.net>

On Thu, 15 Aug 2013 23:40:47 -0700, Stephen Hemminger
<stephen@networkplumber.org> wrote:

> On Thu, 15 Aug 2013 07:05:27 +1000
> Martin Schwenke <martin@meltin.net> wrote:
> 
> > Prefix labelling is currently only activated when monitoring "all"
> > objects.  However, the output can still be confusing when monitoring
> > more than 1 object, so enable prefix labels in this case.
> > 
> > Signed-off-by: Martin Schwenke <martin@meltin.net>
> 
> I understand where you are coming from, but changing the output format
> might break people with scripts doing parsing the current format.
> 
> Since there is already a flag to ip monitor to put on prefix labels,
> why not just use that if you need to?

Sorry, I can't see that option in either the usage message or in the
code.  In the current code the only place I can see prefix_banner
being set is in the "all" case.  I think "prefix" is about monitoring
IPv6 prefixes or similar.

Perhaps I could add a "label" option instead?

> This seems like a change likely to upset more people than it pleases.

Yeah, perhaps...  :-(

peace & happiness,
martin

^ permalink raw reply

* Re: [PATCH] ip monitor: Enable prefix labels when monitoring more than 1 object
From: Stephen Hemminger @ 2013-08-16  6:40 UTC (permalink / raw)
  To: Martin Schwenke; +Cc: netdev
In-Reply-To: <20130815070527.1df23bea@martins.ozlabs.org>

On Thu, 15 Aug 2013 07:05:27 +1000
Martin Schwenke <martin@meltin.net> wrote:

> Prefix labelling is currently only activated when monitoring "all"
> objects.  However, the output can still be confusing when monitoring
> more than 1 object, so enable prefix labels in this case.
> 
> Signed-off-by: Martin Schwenke <martin@meltin.net>

I understand where you are coming from, but changing the output format
might break people with scripts doing parsing the current format.

Since there is already a flag to ip monitor to put on prefix labels,
why not just use that if you need to?

This seems like a change likely to upset more people than it pleases.

^ permalink raw reply

* Re: [PATCH net-next v2 1/3] net/usb/r8152: support aggregation
From: Francois Romieu @ 2013-08-16  5:54 UTC (permalink / raw)
  To: hayeswang
  Cc: netdev, 'nic_swsd', linux-kernel, linux-usb,
	'David Miller'
In-Reply-To: <4C3BE5B36641404DBF9ED8943729871B@realtek.com.tw>

hayeswang <hayeswang@realtek.com> :
> Francois Romieu [mailto:romieu@fr.zoreil.com] 
[...]
> > The forward declaration is not needed.
> 
> The r8152_submit_rx() need the declaration of read_bulk_callback(), and the
> read_bulk_callback() need the declaration of r8152_submit_rx(), too. It likes
> a dead lock, so I have no idea how to do it without another declaration.

Ok, send me a brain for Xmas.

[...]
> > How is it related to the subject of the patch ?
> 
> When link down, the driver would cancel all bulks. This avoid the re-submitting
> bulk.

It's quite orthogonal to aggregation and could thus had been done in its own
patch for readability sake.

[...]
> > It should be possible to retrieve more items in the spinlocked section
> > so as to have a chance to batch more work. I have not thought 
> > too deeply
> > about it.
> 
> I only lock when I want to remove or inser the agg list, and unlock as soon as
> possible. I don't think I keep locking for a long time.

Sure. It doesn't make a difference if tp->rx_done doesn't grow much.

Thanks.

-- 
Ueimor

^ permalink raw reply

* [PATCH 6/6] vhost_net: remove the max pending check
From: Jason Wang @ 2013-08-16  5:16 UTC (permalink / raw)
  To: mst, kvm, virtualization, netdev, linux-kernel
In-Reply-To: <1376630190-5912-1-git-send-email-jasowang@redhat.com>

We used to limit the max pending DMAs to prevent guest from pinning too many
pages. But this could be removed since:

- We have the sk_wmem_alloc check in both tun/macvtap to do the same work
- This max pending check were almost useless since it was one done when there's
  no new buffers coming from guest. Guest can easily exceeds the limitation.
- We've already check upend_idx != done_idx and switch to non zerocopy then. So
  even if all vq->heads were used, we can still does the packet transmission.

So remove this check completely.

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 drivers/vhost/net.c |   13 -------------
 1 files changed, 0 insertions(+), 13 deletions(-)

diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index a035a89..ed3f165 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -38,8 +38,6 @@ MODULE_PARM_DESC(experimental_zcopytx, "Enable Zero Copy TX;"
  * Using this limit prevents one virtqueue from starving others. */
 #define VHOST_NET_WEIGHT 0x80000
 
-/* MAX number of TX used buffers for outstanding zerocopy */
-#define VHOST_MAX_PEND 128
 #define VHOST_GOODCOPY_LEN 256
 
 /*
@@ -372,17 +370,6 @@ static void handle_tx(struct vhost_net *net)
 			break;
 		/* Nothing new?  Wait for eventfd to tell us they refilled. */
 		if (head == vq->num) {
-			int num_pends;
-
-			/* If more outstanding DMAs, queue the work.
-			 * Handle upend_idx wrap around
-			 */
-			num_pends = likely(nvq->upend_idx >= nvq->done_idx) ?
-				    (nvq->upend_idx - nvq->done_idx) :
-				    (nvq->upend_idx + UIO_MAXIOV -
-				     nvq->done_idx);
-			if (unlikely(num_pends > VHOST_MAX_PEND))
-				break;
 			if (unlikely(vhost_enable_notify(&net->dev, vq))) {
 				vhost_disable_notify(&net->dev, vq);
 				continue;
-- 
1.7.1

^ permalink raw reply related

* [PATCH 5/6] vhost_net: poll vhost queue after marking DMA is done
From: Jason Wang @ 2013-08-16  5:16 UTC (permalink / raw)
  To: mst, kvm, virtualization, netdev, linux-kernel
In-Reply-To: <1376630190-5912-1-git-send-email-jasowang@redhat.com>

We used to poll vhost queue before making DMA is done, this is racy if vhost
thread were waked up before marking DMA is done which can result the signal to
be missed. Fix this by always poll the vhost thread before DMA is done.

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 drivers/vhost/net.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 70cab75..a035a89 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -308,6 +308,11 @@ static void vhost_zerocopy_callback(struct ubuf_info *ubuf, bool success)
 	struct vhost_virtqueue *vq = ubufs->vq;
 	int cnt = atomic_read(&ubufs->kref.refcount);
 
+	/* set len to mark this desc buffers done DMA */
+	vq->heads[ubuf->desc].len = success ?
+		VHOST_DMA_DONE_LEN : VHOST_DMA_FAILED_LEN;
+	vhost_net_ubuf_put(ubufs);
+
 	/*
 	 * Trigger polling thread if guest stopped submitting new buffers:
 	 * in this case, the refcount after decrement will eventually reach 1
@@ -318,10 +323,6 @@ static void vhost_zerocopy_callback(struct ubuf_info *ubuf, bool success)
 	 */
 	if (cnt <= 2 || !(cnt % 16))
 		vhost_poll_queue(&vq->poll);
-	/* set len to mark this desc buffers done DMA */
-	vq->heads[ubuf->desc].len = success ?
-		VHOST_DMA_DONE_LEN : VHOST_DMA_FAILED_LEN;
-	vhost_net_ubuf_put(ubufs);
 }
 
 /* Expects to be always run from workqueue - which acts as
-- 
1.7.1

^ permalink raw reply related

* [PATCH 4/6] vhost_net: determine whether or not to use zerocopy at one time
From: Jason Wang @ 2013-08-16  5:16 UTC (permalink / raw)
  To: mst, kvm, virtualization, netdev, linux-kernel
In-Reply-To: <1376630190-5912-1-git-send-email-jasowang@redhat.com>

Currently, even if the packet length is smaller than VHOST_GOODCOPY_LEN, if
upend_idx != done_idx we still set zcopy_used to true and rollback this choice
later. This could be avoided by determine zerocopy once by checking all
conditions at one time before.

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 drivers/vhost/net.c |   46 +++++++++++++++++++---------------------------
 1 files changed, 19 insertions(+), 27 deletions(-)

diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 8a6dd0d..70cab75 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -404,43 +404,35 @@ static void handle_tx(struct vhost_net *net)
 			       iov_length(nvq->hdr, s), hdr_size);
 			break;
 		}
-		zcopy_used = zcopy && (len >= VHOST_GOODCOPY_LEN ||
-				       nvq->upend_idx != nvq->done_idx);
+
+		zcopy_used = zcopy && len >= VHOST_GOODCOPY_LEN
+			&& nvq->upend_idx != nvq->done_idx
+			&& vhost_net_tx_select_zcopy(net);
 
 		/* use msg_control to pass vhost zerocopy ubuf info to skb */
 		if (zcopy_used) {
+			struct ubuf_info *ubuf;
+			ubuf = nvq->ubuf_info + nvq->upend_idx;
+
 			vq->heads[nvq->upend_idx].id = head;
-			if (!vhost_net_tx_select_zcopy(net) ||
-			    len < VHOST_GOODCOPY_LEN) {
-				/* copy don't need to wait for DMA done */
-				vq->heads[nvq->upend_idx].len =
-							VHOST_DMA_DONE_LEN;
-				msg.msg_control = NULL;
-				msg.msg_controllen = 0;
-				ubufs = NULL;
-			} else {
-				struct ubuf_info *ubuf;
-				ubuf = nvq->ubuf_info + nvq->upend_idx;
-
-				vq->heads[nvq->upend_idx].len =
-					VHOST_DMA_IN_PROGRESS;
-				ubuf->callback = vhost_zerocopy_callback;
-				ubuf->ctx = nvq->ubufs;
-				ubuf->desc = nvq->upend_idx;
-				msg.msg_control = ubuf;
-				msg.msg_controllen = sizeof(ubuf);
-				ubufs = nvq->ubufs;
-				kref_get(&ubufs->kref);
-			}
+			vq->heads[nvq->upend_idx].len = VHOST_DMA_IN_PROGRESS;
+			ubuf->callback = vhost_zerocopy_callback;
+			ubuf->ctx = nvq->ubufs;
+			ubuf->desc = nvq->upend_idx;
+			msg.msg_control = ubuf;
+			msg.msg_controllen = sizeof(ubuf);
+			ubufs = nvq->ubufs;
+			kref_get(&ubufs->kref);
 			nvq->upend_idx = (nvq->upend_idx + 1) % UIO_MAXIOV;
-		} else
+		} else {
 			msg.msg_control = NULL;
+			ubufs = NULL;
+		}
 		/* TODO: Check specific error and bomb out unless ENOBUFS? */
 		err = sock->ops->sendmsg(NULL, sock, &msg, len);
 		if (unlikely(err < 0)) {
 			if (zcopy_used) {
-				if (ubufs)
-					vhost_net_ubuf_put(ubufs);
+				vhost_net_ubuf_put(ubufs);
 				nvq->upend_idx = ((unsigned)nvq->upend_idx - 1)
 					% UIO_MAXIOV;
 			}
-- 
1.7.1

^ permalink raw reply related

* [PATCH 3/6] vhost: switch to use vhost_add_used_n()
From: Jason Wang @ 2013-08-16  5:16 UTC (permalink / raw)
  To: mst, kvm, virtualization, netdev, linux-kernel
In-Reply-To: <1376630190-5912-1-git-send-email-jasowang@redhat.com>

Let vhost_add_used() to use vhost_add_used_n() to reduce the code duplication.

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 drivers/vhost/vhost.c |   43 ++-----------------------------------------
 1 files changed, 2 insertions(+), 41 deletions(-)

diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index e58cf00..c479452 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -1332,48 +1332,9 @@ EXPORT_SYMBOL_GPL(vhost_discard_vq_desc);
  * want to notify the guest, using eventfd. */
 int vhost_add_used(struct vhost_virtqueue *vq, unsigned int head, int len)
 {
-	struct vring_used_elem __user *used;
+	struct vring_used_elem heads = { head, len };
 
-	/* The virtqueue contains a ring of used buffers.  Get a pointer to the
-	 * next entry in that used ring. */
-	used = &vq->used->ring[vq->last_used_idx % vq->num];
-	if (__put_user(head, &used->id)) {
-		vq_err(vq, "Failed to write used id");
-		return -EFAULT;
-	}
-	if (__put_user(len, &used->len)) {
-		vq_err(vq, "Failed to write used len");
-		return -EFAULT;
-	}
-	/* Make sure buffer is written before we update index. */
-	smp_wmb();
-	if (__put_user(vq->last_used_idx + 1, &vq->used->idx)) {
-		vq_err(vq, "Failed to increment used idx");
-		return -EFAULT;
-	}
-	if (unlikely(vq->log_used)) {
-		/* Make sure data is seen before log. */
-		smp_wmb();
-		/* Log used ring entry write. */
-		log_write(vq->log_base,
-			  vq->log_addr +
-			   ((void __user *)used - (void __user *)vq->used),
-			  sizeof *used);
-		/* Log used index update. */
-		log_write(vq->log_base,
-			  vq->log_addr + offsetof(struct vring_used, idx),
-			  sizeof vq->used->idx);
-		if (vq->log_ctx)
-			eventfd_signal(vq->log_ctx, 1);
-	}
-	vq->last_used_idx++;
-	/* If the driver never bothers to signal in a very long while,
-	 * used index might wrap around. If that happens, invalidate
-	 * signalled_used index we stored. TODO: make sure driver
-	 * signals at least once in 2^16 and remove this. */
-	if (unlikely(vq->last_used_idx == vq->signalled_used))
-		vq->signalled_used_valid = false;
-	return 0;
+	return vhost_add_used_n(vq, &heads, 1);
 }
 EXPORT_SYMBOL_GPL(vhost_add_used);
 
-- 
1.7.1

^ permalink raw reply related

* [PATCH 2/6] vhost_net: use vhost_add_used_and_signal_n() in vhost_zerocopy_signal_used()
From: Jason Wang @ 2013-08-16  5:16 UTC (permalink / raw)
  To: mst, kvm, virtualization, netdev, linux-kernel
In-Reply-To: <1376630190-5912-1-git-send-email-jasowang@redhat.com>

Switch to use vhost_add_used_and_signal_n() to avoid multiple calls to
vhost_add_used_and_signal(). With the patch we will call at most 2 times
(consider done_idx warp around) compared to N times w/o this patch.

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 drivers/vhost/net.c |   13 ++++++++-----
 1 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 280ee66..8a6dd0d 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -281,7 +281,7 @@ static void vhost_zerocopy_signal_used(struct vhost_net *net,
 {
 	struct vhost_net_virtqueue *nvq =
 		container_of(vq, struct vhost_net_virtqueue, vq);
-	int i;
+	int i, add;
 	int j = 0;
 
 	for (i = nvq->done_idx; i != nvq->upend_idx; i = (i + 1) % UIO_MAXIOV) {
@@ -289,14 +289,17 @@ static void vhost_zerocopy_signal_used(struct vhost_net *net,
 			vhost_net_tx_err(net);
 		if (VHOST_DMA_IS_DONE(vq->heads[i].len)) {
 			vq->heads[i].len = VHOST_DMA_CLEAR_LEN;
-			vhost_add_used_and_signal(vq->dev, vq,
-						  vq->heads[i].id, 0);
 			++j;
 		} else
 			break;
 	}
-	if (j)
-		nvq->done_idx = i;
+	while (j) {
+		add = min(UIO_MAXIOV - nvq->done_idx, j);
+		vhost_add_used_and_signal_n(vq->dev, vq,
+					    &vq->heads[nvq->done_idx], add);
+		nvq->done_idx = (nvq->done_idx + add) % UIO_MAXIOV;
+		j -= add;
+	}
 }
 
 static void vhost_zerocopy_callback(struct ubuf_info *ubuf, bool success)
-- 
1.7.1

^ permalink raw reply related

* [PATCH 1/6] vhost_net: make vhost_zerocopy_signal_used() returns void
From: Jason Wang @ 2013-08-16  5:16 UTC (permalink / raw)
  To: mst, kvm, virtualization, netdev, linux-kernel
In-Reply-To: <1376630190-5912-1-git-send-email-jasowang@redhat.com>

None of its caller use its return value, so let it return void.

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 drivers/vhost/net.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 969a859..280ee66 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -276,8 +276,8 @@ static void copy_iovec_hdr(const struct iovec *from, struct iovec *to,
  * of used idx. Once lower device DMA done contiguously, we will signal KVM
  * guest used idx.
  */
-static int vhost_zerocopy_signal_used(struct vhost_net *net,
-				      struct vhost_virtqueue *vq)
+static void vhost_zerocopy_signal_used(struct vhost_net *net,
+				       struct vhost_virtqueue *vq)
 {
 	struct vhost_net_virtqueue *nvq =
 		container_of(vq, struct vhost_net_virtqueue, vq);
@@ -297,7 +297,6 @@ static int vhost_zerocopy_signal_used(struct vhost_net *net,
 	}
 	if (j)
 		nvq->done_idx = i;
-	return j;
 }
 
 static void vhost_zerocopy_callback(struct ubuf_info *ubuf, bool success)
-- 
1.7.1

^ permalink raw reply related

* [PATCH 0/6] vhost code cleanup and minor enhancement
From: Jason Wang @ 2013-08-16  5:16 UTC (permalink / raw)
  To: mst, kvm, virtualization, netdev, linux-kernel

Hi all:

This series tries to unify and simplify vhost codes especially for zerocopy.

Plase review.

Thanks

Jason Wang (6):
  vhost_net: make vhost_zerocopy_signal_used() returns void
  vhost_net: use vhost_add_used_and_signal_n() in
    vhost_zerocopy_signal_used()
  vhost: switch to use vhost_add_used_n()
  vhost_net: determine whether or not to use zerocopy at one time
  vhost_net: poll vhost queue after marking DMA is done
  vhost_net: remove the max pending check

 drivers/vhost/net.c   |   86 +++++++++++++++++++-----------------------------
 drivers/vhost/vhost.c |   43 +-----------------------
 2 files changed, 36 insertions(+), 93 deletions(-)

^ permalink raw reply

* Re: Where are OVS VXLAN patches?
From: Cong Wang @ 2013-08-16  4:50 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev, David S. Miller, Jesse Gross, Pravin B Shelar
In-Reply-To: <CAOaVG16Vx11DV4gxfSf2f0BceOk1y+aP6rtz-_h9b0Ckp0r2Jw@mail.gmail.com>

(Re-adding netdev and davem...)

On Thu, 2013-08-15 at 21:33 -0700, Stephen Hemminger wrote:
> Send them to netdev. The vxlan-next tree was temporary when there were
> 3 people working on VXLAN at
> same time (Pravin, ong, and me). I see no point in keeping a separate
> tree over the long term.
> 

I agree. There is some misunderstanding between you and David on taking
these patches. :)

^ permalink raw reply

* Re: [PATCH net-next 2/2] ipv4: processing ancillary IP_TOS or IP_TTL
From: Eric Dumazet @ 2013-08-16  4:35 UTC (permalink / raw)
  To: Francesco Fusco; +Cc: davem, netdev
In-Reply-To: <d54a214ad9301352128f2b3300c0eaf91cc6e12e.1376494032.git.ffusco@redhat.com>

On Wed, 2013-08-14 at 17:48 +0200, Francesco Fusco wrote:

> @@ -1511,6 +1517,11 @@ void ip_send_unicast_reply(struct net *net, struct sk_buff *skb, __be32 daddr,
>  	inet = &get_cpu_var(unicast_sock);
>  
>  	inet->tos = arg->tos;
> +
> +	ipc.tos = ip_hdr(skb)->tos;

Why both inet->tos and ipc.tos must be set ?

This is very confusing, as if you were not 100% sure of your patch.


> +	ipc.ttl = inet->uc_ttl;

	ipc.ttl = -1;

> +	ipc.priority = skb->priority;
> +
>  	sk = &inet->sk;
>  	sk->sk_priority = skb->priority;

Why both sk->sk_priority and ipc.priority must be set ?

>  	sk->sk_protocol = ip_hdr(skb)->protocol;

^ permalink raw reply

* Re: [PATCH net-next 2/2] ipv4: processing ancillary IP_TOS or IP_TTL
From: Eric Dumazet @ 2013-08-16  4:26 UTC (permalink / raw)
  To: Francesco Fusco; +Cc: davem, netdev
In-Reply-To: <d54a214ad9301352128f2b3300c0eaf91cc6e12e.1376494032.git.ffusco@redhat.com>

On Wed, 2013-08-14 at 17:48 +0200, Francesco Fusco wrote:
> If IP_TOS or IP_TTL are specified as ancillary data, then sendmsg() sends out
> packets with the specified TTL or TOS overriding the socket values specified
> with the traditional setsockopt().
> 
> If there is a per-packet specified tos, the skb->priority is set accordingly.
> The ipv4_is_multicast() function is used to fill in the right TTL value in case
> of multicast destinations.
> 
> Signed-off-by: Francesco Fusco <ffusco@redhat.com>
> ---

> @@ -1327,7 +1332,7 @@ struct sk_buff *__ip_make_skb(struct sock *sk,
>  	iph = (struct iphdr *)skb->data;
>  	iph->version = 4;
>  	iph->ihl = 5;
> -	iph->tos = inet->tos;
> +	iph->tos = (cork->tos != inet->tos) ? cork->tos : inet->tos;

Strange construct, as the following has same meaning.

	iph->tos = cork->tos;

^ permalink raw reply

* Re: [PATCH v4 5/5] ARM: davinci: da850: add OF_DEV_AUXDATA entry for eth0.
From: Prabhakar Lad @ 2013-08-16  4:08 UTC (permalink / raw)
  To: Sergei Shtylyov; +Cc: Sekhar Nori, DLOS, LKML, device-tree, LAK, netdev
In-Reply-To: <520D2CAE.3090501@cogentembedded.com>

Hi Sergei,

On Fri, Aug 16, 2013 at 1:01 AM, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> Hello.
>
>
> On 15-08-2013 10:01, Lad, Prabhakar wrote:
>
>> From: "Lad, Prabhakar" <prabhakar.csengg@gmail.com>
>
>
>> Add OF_DEV_AUXDATA for eth0  driver in da850 board dt
>> file to use emac clock.
>
>
>> Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>
>
> [...]
>
>
>> diff --git a/arch/arm/mach-davinci/da8xx-dt.c
>> b/arch/arm/mach-davinci/da8xx-dt.c
>> index d172563..caa9202 100644
>> --- a/arch/arm/mach-davinci/da8xx-dt.c
>> +++ b/arch/arm/mach-davinci/da8xx-dt.c
>> @@ -44,6 +44,9 @@ static struct of_dev_auxdata da850_auxdata_lookup[]
>> __initdata = {
>>         OF_DEV_AUXDATA("ns16550a", 0x01d0c000, "serial8250.1", NULL),
>>         OF_DEV_AUXDATA("ns16550a", 0x01d0d000, "serial8250.2", NULL),
>>         OF_DEV_AUXDATA("ti,davinci_mdio", 0x01e24000, "davinci_mdio.0",
>> NULL),
>> +       OF_DEV_AUXDATA("ti,davinci-dm6467-emac", 0x01e20000,
>> "davinci_emac.1",
>> +                      NULL),
>> +
>
>
>    Empty line hardly needed here.
>
Ah my bad, will fix and repost.

Regards,
--Prabhakar Lad

^ permalink raw reply

* Re: [PATCH v4 4/5] ARM: davinci: da850: add DT node for eth0.
From: Prabhakar Lad @ 2013-08-16  4:07 UTC (permalink / raw)
  To: Sergei Shtylyov; +Cc: Sekhar Nori, DLOS, LKML, device-tree, LAK, netdev
In-Reply-To: <520D2C50.4040902@cogentembedded.com>

Hi Sergei,

On Fri, Aug 16, 2013 at 1:00 AM, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> Hello.
>
>
> On 15-08-2013 10:01, Lad, Prabhakar wrote:
>
>> From: "Lad, Prabhakar" <prabhakar.csengg@gmail.com>
>
>
>> Add eth0 device tree node information and pinmux for mii to da850 by
>> providing interrupt details and local mac address of eth0.
>
>
>> Signed-off-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>
>
> [...]
>
>
>> @@ -226,6 +242,20 @@
>>                         #size-cells = <0>;
>>                         reg = <0x224000 0x1000>;
>>                 };
>> +               eth0: emac@1e20000 {
>
>
>    According to the ePAPR spec[1] section 2.2.2, the node should be named
> "ethernet".
>
Thanks for pointing it, I'll fix and repost it.

Regards,
--Prabhakar Lad

> [1] http://www.power.org/resources/downloads/Power_ePAPR_APPROVED_v1.0.pdf
>
> WBR, Sergei
>

^ permalink raw reply

* RE: [PATCH net-next v2 1/3] net/usb/r8152: support aggregation
From: hayeswang @ 2013-08-16  3:44 UTC (permalink / raw)
  To: 'Francois Romieu'
  Cc: netdev, 'nic_swsd', linux-kernel, linux-usb,
	'David Miller'
In-Reply-To: <20130815122617.GA18057@electric-eye.fr.zoreil.com>

Francois Romieu [mailto:romieu@fr.zoreil.com] 
> Sent: Thursday, August 15, 2013 8:26 PM
> To: Hayeswang
> Cc: netdev@vger.kernel.org; nic_swsd; 
> linux-kernel@vger.kernel.org; linux-usb@vger.kernel.org; David Miller
> Subject: Re: [PATCH net-next v2 1/3] net/usb/r8152: support 
> aggregation
> 
[...]
> > +static
> > +int r8152_submit_rx(struct r8152 *tp, struct rx_agg *agg, 
> gfp_t mem_flags);
> > +
> 
> It's a new, less than 10 lines function without driver 
> internal dependencies.
> 
> The forward declaration is not needed.

The r8152_submit_rx() need the declaration of read_bulk_callback(), and the
read_bulk_callback() need the declaration of r8152_submit_rx(), too. It likes
a dead lock, so I have no idea how to do it without another declaration.

[...]
> > -	if (!netif_device_present(netdev))
> > +	if (!netif_carrier_ok(netdev))
> >  		return;
> 
> How is it related to the subject of the patch ?

When link down, the driver would cancel all bulks. This avoid the re-submitting
bulk.

> [...]
> > +static void rx_bottom(struct r8152 *tp)
> > +{
> > +	struct net_device_stats *stats;
> > +	struct net_device *netdev;
> > +	struct rx_agg *agg;
> > +	struct rx_desc *rx_desc;
> > +	unsigned long lockflags;
> 
> Idiom: 'flags'.
> 
> > +	struct list_head *cursor, *next;
> > +	struct sk_buff *skb;
> > +	struct urb *urb;
> > +	unsigned pkt_len;
> > +	int len_used;
> > +	u8 *rx_data;
> > +	int ret;
> 
> The scope of these variables is needlessly wide.
> 
> > +
> > +	netdev = tp->netdev;
> > +
> > +	stats = rtl8152_get_stats(netdev);
> > +
> > +	spin_lock_irqsave(&tp->rx_lock, lockflags);
> > +	list_for_each_safe(cursor, next, &tp->rx_done) {
> > +		list_del_init(cursor);
> > +		spin_unlock_irqrestore(&tp->rx_lock, lockflags);
> > +
> > +		agg = list_entry(cursor, struct rx_agg, list);
> > +		urb = agg->urb;
> > +		if (urb->actual_length < ETH_ZLEN) {
> 
> 			goto submit;
> 
> > +			ret = r8152_submit_rx(tp, agg, GFP_ATOMIC);
> > +			spin_lock_irqsave(&tp->rx_lock, lockflags);
> > +			if (ret && ret != -ENODEV) {
> > +				list_add_tail(&agg->list, next);
> > +				tasklet_schedule(&tp->tl);
> > +			}
> > +			continue;
> > +		}
> 
> (remove the line above)
> 
> [...]
> > +			rx_data = rx_agg_align(rx_data + pkt_len + 4);
> > +			rx_desc = (struct rx_desc *)rx_data;
> > +			pkt_len = le32_to_cpu(rx_desc->opts1) & 
> RX_LEN_MASK;
> > +			len_used = (int)(rx_data - (u8 *)agg->head);
> > +			len_used += sizeof(struct rx_desc) + pkt_len;
> > +		}
> > +
> 
> submit:
> 
> > +		ret = r8152_submit_rx(tp, agg, GFP_ATOMIC);
> > +		spin_lock_irqsave(&tp->rx_lock, lockflags);
> > +		if (ret && ret != -ENODEV) {
> > +			list_add_tail(&agg->list, next);
> > +			tasklet_schedule(&tp->tl);
> > +		}
> > +	}
> > +	spin_unlock_irqrestore(&tp->rx_lock, lockflags);
> > +}
> 
> It should be possible to retrieve more items in the spinlocked section
> so as to have a chance to batch more work. I have not thought 
> too deeply
> about it.

I only lock when I want to remove or inser the agg list, and unlock as soon as
possible. I don't think I keep locking for a long time.

 
Best Regards,
Hayes

^ permalink raw reply

* [PATCH v3 RESEND 5/7] net: sunhme: use platform_{get,set}_drvdata()
From: Libo Chen @ 2013-08-16  2:33 UTC (permalink / raw)
  To: David Miller
  Cc: jg1.han, Bill Pemberton, netdev, LKML, Sergei Shtylyov, Li Zefan

Use the wrapper functions for getting and setting the driver data using
platform_device instead of using dev_{get,set}_drvdata() with &of_dev->dev,
so we can directly pass a struct platform_device.

changelog:
	remove modify about happy_meal_pci_probe && happy_meal_pci_remove

Signed-off-by: Libo Chen <libo.chen@huawei.com>
---
 drivers/net/ethernet/sun/sunhme.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ethernet/sun/sunhme.c b/drivers/net/ethernet/sun/sunhme.c
index 171f5b0..b90d311 100644
--- a/drivers/net/ethernet/sun/sunhme.c
+++ b/drivers/net/ethernet/sun/sunhme.c
@@ -3231,7 +3231,7 @@ static int hme_sbus_probe(struct platform_device *op)

 static int hme_sbus_remove(struct platform_device *op)
 {
-	struct happy_meal *hp = dev_get_drvdata(&op->dev);
+	struct happy_meal *hp = platform_get_drvdata(op);
 	struct net_device *net_dev = hp->dev;

 	unregister_netdev(net_dev);
-- 
1.7.1

^ permalink raw reply related


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