Netdev List
 help / color / mirror / Atom feed
* [GIT] net merged into net-next
From: David Miller @ 2012-06-13  5:18 UTC (permalink / raw)
  To: netdev-u79uwXL29TY76Z2rM5mHXA
  Cc: linville-2XuSBdqkA4R54TAoqtyWWQ,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA


John please double check my work as the only merge hassles
were wireless bits.  I'm pretty sure I resolved things in
a reasonable way.

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

^ permalink raw reply

* Re: [PATCH net-next 4/4] 6lowpan: len field is not stored and accessed properly
From: Tony Cheneau @ 2012-06-13  4:54 UTC (permalink / raw)
  To: Alexander Smirnov
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-zigbee-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f
In-Reply-To: <CAJmB2rAYZe9FEfUcxd_g6kX247MExv89h=Cjfxmvfb+=6qGSgQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

Hi again,

Thank you for your comment. See my answer inline.

Le Tue, 12 Jun 2012 22:20:13 +0400,
Alexander Smirnov <alex.bluesman.smirnov@gmail.com> a écrit :

> 2012/6/11 Tony Cheneau <tony.cheneauh@amnesiak.org>:
> > Lenght field should be encoded (and accessed) the other way around.
> > As it is currently written, it could lead to interroperability
> > issues.
> 
> What kind of "interroperability issues" can occur? Please describe it
> in details, I can't read your mind. And again, can these problems be
> caused by the byte-order mismatch?
Sorry if I wasn't clear. Yes, I believe it is a byte-order mismatch.
I wasn't able to verify interoperability with other implementations
myself, but I compared with some packet capture I obtained from some
other implementations, where the byte-order was different. I'll double
check that when I'll have access to my lab again.
The reason I choose the term "interoperability issues" is that this
patch focuses on the length field where patch 3 focused on the tag
field. If you encode the tag field incorrectly, it is not a big deal,
because the recipient will decode wrongly, but all fragment of a same
packet will share a same tag (regardless of you decode it properly or
not). However, when the same thing happen with the length field, if you
endianness is wrong, the recipient will interpret a totally different
length and will reserve a wrong amount of memory in order to store the
packet (this is mainly a matter of concern if you are performing
fragment reassembly and do not allocate enough memory for the complete
frame). Do that make sense?

Just like the previous patch, I'll rework it as well.

As a side note, do you have any opinions on the first two patches 

Regards,
	Tony



> >
> > Also, I rewrote the code so that iphc0 argument of
> > lowpan_alloc_new_frame could be removed.
> > ---
> >  net/ieee802154/6lowpan.c |   20 ++++++++++++--------
> >  1 files changed, 12 insertions(+), 8 deletions(-)
> >
> > diff --git a/net/ieee802154/6lowpan.c b/net/ieee802154/6lowpan.c
> > index af2f12e..b400156 100644
> > --- a/net/ieee802154/6lowpan.c
> > +++ b/net/ieee802154/6lowpan.c
> > @@ -654,7 +654,7 @@ static void
> > lowpan_fragment_timer_expired(unsigned long entry_addr) }
> >
> >  static struct lowpan_fragment *
> > -lowpan_alloc_new_frame(struct sk_buff *skb, u8 iphc0, u8 len, u16
> > tag) +lowpan_alloc_new_frame(struct sk_buff *skb, u16 len, u16 tag)
> >  {
> >        struct lowpan_fragment *frame;
> >
> > @@ -665,7 +665,7 @@ lowpan_alloc_new_frame(struct sk_buff *skb, u8
> > iphc0, u8 len, u16 tag)
> >        INIT_LIST_HEAD(&frame->list);
> >
> > -       frame->length = (iphc0 & 7) | (len << 3);
> > +       frame->length = len;
> >        frame->tag = tag;
> >
> >        /* allocate buffer for frame assembling */
> > @@ -721,13 +721,17 @@ lowpan_process_data(struct sk_buff *skb)
> >        case LOWPAN_DISPATCH_FRAGN:
> >        {
> >                struct lowpan_fragment *frame;
> > -               u8 len, offset;
> > -               u16 tag;
> > +               /* slen stores the rightmost 8 bits of the 11 bits
> > length */
> > +               u8 slen, offset;
> > +               u16 len, tag;
> >                bool found = false;
> >
> > -               len = lowpan_fetch_skb_u8(skb); /* frame length */
> > +               slen = lowpan_fetch_skb_u8(skb); /* frame length */
> >                tag = lowpan_fetch_skb_u16(skb);
> >
> > +               /* adds the 3 MSB to the 8 LSB to retrieve the 11
> > bits length */
> > +               len = ((iphc0 & 7) << 8) | slen;
> > +
> >                /*
> >                 * check if frame assembling with the same tag is
> >                 * already in progress
> > @@ -742,7 +746,7 @@ lowpan_process_data(struct sk_buff *skb)
> >
> >                /* alloc new frame structure */
> >                if (!found) {
> > -                       frame = lowpan_alloc_new_frame(skb, iphc0,
> > len, tag);
> > +                       frame = lowpan_alloc_new_frame(skb, len,
> > tag); if (!frame)
> >                                goto unlock_and_drop;
> >                }
> > @@ -1000,8 +1004,8 @@ lowpan_skb_fragmentation(struct sk_buff *skb)
> >        tag = fragment_tag++;
> >
> >        /* first fragment header */
> > -       head[0] = LOWPAN_DISPATCH_FRAG1 | (payload_length & 0x7);
> > -       head[1] = (payload_length >> 3) & 0xff;
> > +       head[0] = LOWPAN_DISPATCH_FRAG1 | ((payload_length >> 8) &
> > 0x7);
> > +       head[1] = payload_length & 0xff;
> >        head[2] = tag >> 8;
> >        head[3] = tag & 0xff;
> >
> > --
> > 1.7.3.4
> >
> 
> ------------------------------------------------------------------------------
> Live Security Virtual Conference
> Exclusive live event will cover all the ways today's security and 
> threat landscape has changed and how IT managers can respond.
> Discussions will include endpoint security, mobile security and the
> latest in malware threats.
> http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
> _______________________________________________ Linux-zigbee-devel
> mailing list Linux-zigbee-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-zigbee-devel

------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
Linux-zigbee-devel mailing list
Linux-zigbee-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-zigbee-devel

^ permalink raw reply

* Re: [PATCH 4/7] usbnet: remove EVENT_DEV_OPEN flag
From: Ming Lei @ 2012-06-13  4:47 UTC (permalink / raw)
  To: Oliver Neukum; +Cc: David S. Miller, Greg Kroah-Hartman, netdev, linux-usb
In-Reply-To: <CACVXFVPf+Q9i-Ju3GFapiTB2wEWZSBrZ2Gf=WHsLWfWfTnWGpA@mail.gmail.com>

On Wed, Jun 13, 2012 at 10:12 AM, Ming Lei <tom.leiming@gmail.com> wrote:
> On Wed, Jun 13, 2012 at 2:14 AM, Oliver Neukum <oliver@neukum.org> wrote:
>> Am Dienstag, 12. Juni 2012, 03:19:42 schrieb Ming Lei:
>>> EVENT_DEV_OPEN is introduced to mark if the interface is opened or
>>> not, but we already have IFF_UP to handle it, so just
>>> remove the flag and use IFF_UP.
>>
>> When is IFF_UP cleared? The flag is tested in usbnet_resume(),
>
> The flag is cleared just after usbnet_stop completes.

Looks we can use the below to replace EVENT_DEV_OPEN:

     (netif_running((dev)->net) && ((dev)->net->flags & IFF_UP))


Thanks,
-- 
Ming Lei

^ permalink raw reply

* Re: [PATCH net-next 3/4] 6lowpan: the two bytes of u16 tag needs to be stored/accessed the other way around
From: Tony Cheneau @ 2012-06-13  4:42 UTC (permalink / raw)
  To: Alexander Smirnov; +Cc: netdev, linux-zigbee-devel
In-Reply-To: <CAJmB2rB20UL_8u1PmJM67dQJ1FaW09irJhz6-OusZwWkgs0kLQ@mail.gmail.com>

Hello Alexander,

Please see my response inline.

Le Tue, 12 Jun 2012 22:07:12 +0400,
Alexander Smirnov <alex.bluesman.smirnov@gmail.com> a écrit :

> 2012/6/11 Tony Cheneau <tony.cheneauh@amnesiak.org>:
> > Or else, tag values, as displayed with a trafic analyser, such a
> > Wireshark, are not properly displayed (e.g. 0x01 00 insted of 0x00
> > 01, and so on). ---
> >  net/ieee802154/6lowpan.c |    6 +++---
> >  1 files changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/net/ieee802154/6lowpan.c b/net/ieee802154/6lowpan.c
> > index af4f29b..af2f12e 100644
> > --- a/net/ieee802154/6lowpan.c
> > +++ b/net/ieee802154/6lowpan.c
> > @@ -307,7 +307,7 @@ static u16 lowpan_fetch_skb_u16(struct sk_buff
> > *skb)
> >
> >        BUG_ON(!pskb_may_pull(skb, 2));
> >
> > -       ret = skb->data[0] | (skb->data[1] << 8);
> > +       ret = (skb->data[0] << 8) | skb->data[1];
> 
> This function aimed to obtain u16 from skb, why did you change the
> byte-order here instead of 'tag' variable byte-shifting? 
This probably reflects my lack of practice in writing/reading network
code. I witnessed in my network that the byte order was wrong when
sending/receiving packet (using Wireshark) and though it would be a
quick fix. Also, I assumed skb->data would store data using network
ordering, so that skb->data[0] would have been the MSB and
skb->data[1] would have been the LSB. I'll look more into that.

> Will this
> code work properly on the both little and big-endian machines?
I haven't checked that. But, just for clarification, if my changes are
not working properly on both architectures, it means that the original
code doesn't either, right?

> Please
> rework this to keep order properly for all the architectures.
I will read some more network code to see how it is handled in other
part of the kernel and rewrite the patch accordingly (if at all
needed). 

Thank you for your advises.

> >        skb_pull(skb, 2);
> >        return ret;
> >  }
> > @@ -1002,8 +1002,8 @@ lowpan_skb_fragmentation(struct sk_buff *skb)
> >        /* first fragment header */
> >        head[0] = LOWPAN_DISPATCH_FRAG1 | (payload_length & 0x7);
> >        head[1] = (payload_length >> 3) & 0xff;
> > -       head[2] = tag & 0xff;
> > -       head[3] = tag >> 8;
> > +       head[2] = tag >> 8;
> > +       head[3] = tag & 0xff;
> >
> >        err = lowpan_fragment_xmit(skb, head, header_length, 0, 0);
> >
> > --
> > 1.7.3.4
> >
> >
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Regards,
	Tony

^ permalink raw reply

* Re: ethtool: allow ETHTOOL_GSSET_INFO for users
From: David Miller @ 2012-06-13  4:33 UTC (permalink / raw)
  To: bhutchings; +Cc: netdev, mirq-linux
In-Reply-To: <1339543678.15266.7.camel@bwh-desktop.uk.solarflarecom.com>

From: Ben Hutchings <bhutchings@solarflare.com>
Date: Wed, 13 Jun 2012 00:27:58 +0100

> Please consider this for stable 3.0.y and 3.2.y:
> 
> commit f80400a26a2e8bff541de12834a1134358bb6642
> Author: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> Date:   Sun Jan 22 00:20:40 2012 +0000
> 
>     ethtool: allow ETHTOOL_GSSET_INFO for users

Queued up, thanks.

^ permalink raw reply

* Re: [PATCH 2/5] ipv4: Kill ip_rt_frag_needed().
From: David Miller @ 2012-06-13  4:22 UTC (permalink / raw)
  To: steffen.klassert; +Cc: netdev
In-Reply-To: <20120612.133333.527780673034196147.davem@davemloft.net>

From: David Miller <davem@davemloft.net>
Date: Tue, 12 Jun 2012 13:33:33 -0700 (PDT)

> What we possibly could do is adjust the socket's IP_PMTUDISC_* setting
> from IP_PMTUDISC_WANT to IP_PMTUDISC_DONT in response to PMTU
> messages.
> 
> This seems to solve all the problems.  Individual RAW and UDP sockets
> get the behavior they did before, and route cache PMTU poisoning is
> less of an issue.

Here is an implementation of that idea:

diff --git a/include/linux/ipv6.h b/include/linux/ipv6.h
index 8260ef7..61cb532 100644
--- a/include/linux/ipv6.h
+++ b/include/linux/ipv6.h
@@ -416,6 +416,16 @@ static inline struct ipv6_pinfo * inet6_sk(const struct sock *__sk)
 	return inet_sk(__sk)->pinet6;
 }
 
+/* We don't want to update the routing tables because there is no way
+ * to validate the legitimacy of this PMTU event.  Instead, downgrade
+ * the PMTU setting of the socket.
+ */
+static inline void inet6_datagram_pmtu_event(struct ipv6_pinfo *np)
+{
+	if (np->pmtudisc == IP_PMTUDISC_WANT)
+		np->pmtudisc = IP_PMTUDISC_DONT;
+}
+
 static inline struct inet6_request_sock *
 			inet6_rsk(const struct request_sock *rsk)
 {
diff --git a/include/net/inet_sock.h b/include/net/inet_sock.h
index ae17e13..2199afb 100644
--- a/include/net/inet_sock.h
+++ b/include/net/inet_sock.h
@@ -199,6 +199,16 @@ static inline void inet_sk_copy_descendant(struct sock *sk_to,
 }
 #endif
 
+/* We don't want to update the routing cache because there is no way
+ * to validate the legitimacy of this PMTU event.  Instead, downgrade
+ * the PMTU setting of the socket.
+ */
+static inline void inet_datagram_pmtu_event(struct inet_sock *inet)
+{
+	if (inet->pmtudisc == IP_PMTUDISC_WANT)
+		inet->pmtudisc = IP_PMTUDISC_DONT;
+}
+
 extern int inet_sk_rebuild_header(struct sock *sk);
 
 extern u32 inet_ehash_secret;
diff --git a/net/ipv4/ping.c b/net/ipv4/ping.c
index 2c00e8b..c6becc1 100644
--- a/net/ipv4/ping.c
+++ b/net/ipv4/ping.c
@@ -374,6 +374,7 @@ void ping_err(struct sk_buff *skb, u32 info)
 			if (inet_sock->pmtudisc != IP_PMTUDISC_DONT) {
 				err = EMSGSIZE;
 				harderr = 1;
+				inet_datagram_pmtu_event(inet_sock);
 				break;
 			}
 			goto out;
diff --git a/net/ipv4/raw.c b/net/ipv4/raw.c
index 4032b81..ed24b05 100644
--- a/net/ipv4/raw.c
+++ b/net/ipv4/raw.c
@@ -244,6 +244,7 @@ static void raw_err(struct sock *sk, struct sk_buff *skb, u32 info)
 		if (code == ICMP_FRAG_NEEDED) {
 			harderr = inet->pmtudisc != IP_PMTUDISC_DONT;
 			err = EMSGSIZE;
+			inet_datagram_pmtu_event(inet);
 		}
 	}
 
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index eaca736..40cf013 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -618,6 +618,7 @@ void __udp4_lib_err(struct sk_buff *skb, u32 info, struct udp_table *udptable)
 			if (inet->pmtudisc != IP_PMTUDISC_DONT) {
 				err = EMSGSIZE;
 				harderr = 1;
+				inet_datagram_pmtu_event(inet);
 				break;
 			}
 			goto out;
diff --git a/net/ipv6/raw.c b/net/ipv6/raw.c
index 93d6983..79e2f7a 100644
--- a/net/ipv6/raw.c
+++ b/net/ipv6/raw.c
@@ -328,9 +328,10 @@ static void rawv6_err(struct sock *sk, struct sk_buff *skb,
 		return;
 
 	harderr = icmpv6_err_convert(type, code, &err);
-	if (type == ICMPV6_PKT_TOOBIG)
+	if (type == ICMPV6_PKT_TOOBIG) {
 		harderr = (np->pmtudisc == IPV6_PMTUDISC_DO);
-
+		inet6_datagram_pmtu_event(np);
+	}
 	if (np->recverr) {
 		u8 *payload = skb->data;
 		if (!inet->hdrincl)
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index f05099f..fb57815 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -481,6 +481,9 @@ void __udp6_lib_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
 
 	np = inet6_sk(sk);
 
+	if (type == ICMPV6_PKT_TOOBIG)
+		inet6_datagram_pmtu_event(np);
+
 	if (!icmpv6_err_convert(type, code, &err) && !np->recverr)
 		goto out;
 

^ permalink raw reply related

* [PATCH 3/3] usbnet: handle remote wakeup asap
From: Ming Lei @ 2012-06-13  4:20 UTC (permalink / raw)
  To: David S. Miller, Greg Kroah-Hartman
  Cc: Oliver Neukum, netdev, linux-usb, Ming Lei, stable
In-Reply-To: <1339561217-18151-1-git-send-email-ming.lei@canonical.com>

If usbnet is resumed by remote wakeup, generally there are
some packets comming to be handled, so allocate and submit
rx URBs in usbnet_resume to avoid delays introduced by tasklet.
Otherwise, usbnet may have been runtime suspended before the
usbnet_bh is executed to schedule Rx URBs.

Without the patch, usbnet can't recieve any packets from peer
in runtime suspend state if runtime PM is enabled and
autosuspend_delay is set as zero.

Cc: stable@vger.kernel.org
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
 drivers/net/usb/usbnet.c |   42 ++++++++++++++++++++++++++----------------
 1 file changed, 26 insertions(+), 16 deletions(-)

diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 9bfa775..4911efa 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -1201,6 +1201,21 @@ deferred:
 }
 EXPORT_SYMBOL_GPL(usbnet_start_xmit);
 
+static void rx_alloc_submit(struct usbnet *dev, gfp_t flags)
+{
+	struct urb	*urb;
+	int		i;
+
+	/* don't refill the queue all at once */
+	for (i = 0; i < 10 && dev->rxq.qlen < RX_QLEN(dev); i++) {
+		urb = usb_alloc_urb(0, GFP_ATOMIC);
+		if (urb != NULL) {
+			if (rx_submit(dev, urb, flags) == -ENOLINK)
+				return;
+		}
+	}
+}
+
 /*-------------------------------------------------------------------------*/
 
 // tasklet (work deferred from completions, in_irq) or timer
@@ -1240,26 +1255,14 @@ static void usbnet_bh (unsigned long param)
 		   !timer_pending (&dev->delay) &&
 		   !test_bit (EVENT_RX_HALT, &dev->flags)) {
 		int	temp = dev->rxq.qlen;
-		int	qlen = RX_QLEN (dev);
-
-		if (temp < qlen) {
-			struct urb	*urb;
-			int		i;
-
-			// don't refill the queue all at once
-			for (i = 0; i < 10 && dev->rxq.qlen < qlen; i++) {
-				urb = usb_alloc_urb (0, GFP_ATOMIC);
-				if (urb != NULL) {
-					if (rx_submit (dev, urb, GFP_ATOMIC) ==
-					    -ENOLINK)
-						return;
-				}
-			}
+
+		if (temp < RX_QLEN(dev)) {
+			rx_alloc_submit(dev, GFP_ATOMIC);
 			if (temp != dev->rxq.qlen)
 				netif_dbg(dev, link, dev->net,
 					  "rxqlen %d --> %d\n",
 					  temp, dev->rxq.qlen);
-			if (dev->rxq.qlen < qlen)
+			if (dev->rxq.qlen < RX_QLEN(dev))
 				tasklet_schedule (&dev->bh);
 		}
 		if (dev->txq.qlen < TX_QLEN (dev))
@@ -1565,6 +1568,13 @@ int usbnet_resume (struct usb_interface *intf)
 		spin_unlock_irq(&dev->txq.lock);
 
 		if (test_bit(EVENT_DEV_OPEN, &dev->flags)) {
+			/* handle remote wakeup ASAP */
+			if (!dev->wait &&
+				netif_device_present(dev->net) &&
+				!timer_pending(&dev->delay) &&
+				!test_bit(EVENT_RX_HALT, &dev->flags))
+					rx_alloc_submit(dev, GFP_KERNEL);
+
 			if (!(dev->txq.qlen >= TX_QLEN(dev)))
 				netif_tx_wake_all_queues(dev->net);
 			tasklet_schedule (&dev->bh);
-- 
1.7.9.5

^ permalink raw reply related

* usbnet: PM related fixes
From: Ming Lei @ 2012-06-13  4:20 UTC (permalink / raw)
  To: David S. Miller, Greg Kroah-Hartman; +Cc: Oliver Neukum, netdev, linux-usb

Hi David,

The 3 patches fix some PM related problems.

Sorry for sending them after the cleanup patches since I found them just
after sending out the cleanup patches. 

 drivers/net/usb/usbnet.c |   47 +++++++++++++++++++++++++++++-----------------
 1 file changed, 30 insertions(+), 17 deletions(-)

^ permalink raw reply

* [PATCH 2/3] usbnet: decrease suspend count if returning -EBUSY for runtime suspend
From: Ming Lei @ 2012-06-13  4:20 UTC (permalink / raw)
  To: David S. Miller, Greg Kroah-Hartman
  Cc: Oliver Neukum, netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA, Ming Lei,
	stable-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1339561217-18151-1-git-send-email-ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>

This patch decreases dev->suspend_count in the -EBUSY failure path
of usbnet_suspend. Without the change, the later runtime suspend
will do nothing except for increasing dev->suspend_count.

Cc: stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Signed-off-by: Ming Lei <ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
---
 drivers/net/usb/usbnet.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index f06cf9b..9bfa775 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -1508,6 +1508,7 @@ int usbnet_suspend (struct usb_interface *intf, pm_message_t message)
 		spin_lock_irq(&dev->txq.lock);
 		/* don't autosuspend while transmitting */
 		if (dev->txq.qlen && PMSG_IS_AUTO(message)) {
+			dev->suspend_count--;
 			spin_unlock_irq(&dev->txq.lock);
 			return -EBUSY;
 		} else {
-- 
1.7.9.5

--
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 1/3] usbnet: clear OPEN flag in failure path
From: Ming Lei @ 2012-06-13  4:20 UTC (permalink / raw)
  To: David S. Miller, Greg Kroah-Hartman
  Cc: Oliver Neukum, netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA, Ming Lei,
	stable-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1339561217-18151-1-git-send-email-ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>

Without clearing OPEN flag in failure path, runtime or system resume
may submit interrupt/rx URB and start tx queue mistakenly on a
interface in DOWN state.

Cc: stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Signed-off-by: Ming Lei <ming.lei-Z7WLFzj8eWMS+FvcfC7Uqw@public.gmane.org>
---
 drivers/net/usb/usbnet.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index ac2e493..f06cf9b 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -793,11 +793,13 @@ int usbnet_open (struct net_device *net)
 	if (info->manage_power) {
 		retval = info->manage_power(dev, 1);
 		if (retval < 0)
-			goto done;
+			goto done_manage_power_error;
 		usb_autopm_put_interface(dev->intf);
 	}
 	return retval;
 
+done_manage_power_error:
+	clear_bit(EVENT_DEV_OPEN, &dev->flags);
 done:
 	usb_autopm_put_interface(dev->intf);
 done_nopm:
-- 
1.7.9.5

--
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 5/5] KVM: Replace test_and_set_bit_le() in mark_page_dirty_in_slot() with set_bit_le()
From: Takuya Yoshikawa @ 2012-06-13  4:05 UTC (permalink / raw)
  To: akpm
  Cc: bhutchings, grundler, arnd, benh, avi, mtosatti,
	linux-net-drivers, netdev, linux-kernel, linux-arch, kvm,
	takuya.yoshikawa
In-Reply-To: <20120613130054.b5695621.yoshikawa.takuya@oss.ntt.co.jp>

From: Takuya Yoshikawa <yoshikawa.takuya@oss.ntt.co.jp>

Now that we have defined generic set_bit_le() we do not need to use
test_and_set_bit_le() for atomically setting a bit.

Signed-off-by: Takuya Yoshikawa <yoshikawa.takuya@oss.ntt.co.jp>
Cc: Avi Kivity <avi@redhat.com>
Cc: Marcelo Tosatti <mtosatti@redhat.com>
---
 virt/kvm/kvm_main.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 02cb440..560c502 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -1485,8 +1485,7 @@ void mark_page_dirty_in_slot(struct kvm *kvm, struct kvm_memory_slot *memslot,
 	if (memslot && memslot->dirty_bitmap) {
 		unsigned long rel_gfn = gfn - memslot->base_gfn;
 
-		/* TODO: introduce set_bit_le() and use it */
-		test_and_set_bit_le(rel_gfn, memslot->dirty_bitmap);
+		set_bit_le(rel_gfn, memslot->dirty_bitmap);
 	}
 }
 
-- 
1.7.5.4

^ permalink raw reply related

* [PATCH 4/5] powerpc: bitops: Introduce {clear,set}_bit_le()
From: Takuya Yoshikawa @ 2012-06-13  4:04 UTC (permalink / raw)
  To: akpm
  Cc: bhutchings, grundler, arnd, benh, avi, mtosatti,
	linux-net-drivers, netdev, linux-kernel, linux-arch, kvm,
	takuya.yoshikawa
In-Reply-To: <20120613130054.b5695621.yoshikawa.takuya@oss.ntt.co.jp>

From: Takuya Yoshikawa <yoshikawa.takuya@oss.ntt.co.jp>

Needed to replace test_and_set_bit_le() in virt/kvm/kvm_main.c which is
being used for this missing function.

Signed-off-by: Takuya Yoshikawa <yoshikawa.takuya@oss.ntt.co.jp>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 arch/powerpc/include/asm/bitops.h |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/include/asm/bitops.h b/arch/powerpc/include/asm/bitops.h
index efdc926..dc2cf9c 100644
--- a/arch/powerpc/include/asm/bitops.h
+++ b/arch/powerpc/include/asm/bitops.h
@@ -288,6 +288,16 @@ static __inline__ int test_bit_le(unsigned long nr,
 	return (tmp[nr >> 3] >> (nr & 7)) & 1;
 }
 
+static inline void set_bit_le(int nr, void *addr)
+{
+	set_bit(nr ^ BITOP_LE_SWIZZLE, addr);
+}
+
+static inline void clear_bit_le(int nr, void *addr)
+{
+	clear_bit(nr ^ BITOP_LE_SWIZZLE, addr);
+}
+
 static inline void __set_bit_le(int nr, void *addr)
 {
 	__set_bit(nr ^ BITOP_LE_SWIZZLE, addr);
-- 
1.7.5.4

^ permalink raw reply related

* [PATCH 3/5] bitops: Introduce generic {clear,set}_bit_le()
From: Takuya Yoshikawa @ 2012-06-13  4:03 UTC (permalink / raw)
  To: akpm
  Cc: bhutchings, grundler, arnd, benh, avi, mtosatti,
	linux-net-drivers, netdev, linux-kernel, linux-arch, kvm,
	takuya.yoshikawa
In-Reply-To: <20120613130054.b5695621.yoshikawa.takuya@oss.ntt.co.jp>

From: Takuya Yoshikawa <yoshikawa.takuya@oss.ntt.co.jp>

Needed to replace test_and_set_bit_le() in virt/kvm/kvm_main.c which is
being used for this missing function.

Signed-off-by: Takuya Yoshikawa <yoshikawa.takuya@oss.ntt.co.jp>
Acked-by: Arnd Bergmann <arnd@arndb.de>
---
 include/asm-generic/bitops/le.h |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/include/asm-generic/bitops/le.h b/include/asm-generic/bitops/le.h
index f95c663..6173154 100644
--- a/include/asm-generic/bitops/le.h
+++ b/include/asm-generic/bitops/le.h
@@ -54,6 +54,16 @@ static inline int test_bit_le(int nr, const void *addr)
 	return test_bit(nr ^ BITOP_LE_SWIZZLE, addr);
 }
 
+static inline void set_bit_le(int nr, void *addr)
+{
+	set_bit(nr ^ BITOP_LE_SWIZZLE, addr);
+}
+
+static inline void clear_bit_le(int nr, void *addr)
+{
+	clear_bit(nr ^ BITOP_LE_SWIZZLE, addr);
+}
+
 static inline void __set_bit_le(int nr, void *addr)
 {
 	__set_bit(nr ^ BITOP_LE_SWIZZLE, addr);
-- 
1.7.5.4

^ permalink raw reply related

* [PATCH 2/5] drivers/net/ethernet/dec/tulip: Use standard __set_bit_le() function
From: Takuya Yoshikawa @ 2012-06-13  4:03 UTC (permalink / raw)
  To: akpm
  Cc: bhutchings, grundler, arnd, benh, avi, mtosatti,
	linux-net-drivers, netdev, linux-kernel, linux-arch, kvm,
	takuya.yoshikawa
In-Reply-To: <20120613130054.b5695621.yoshikawa.takuya@oss.ntt.co.jp>

From: Takuya Yoshikawa <yoshikawa.takuya@oss.ntt.co.jp>

To introduce generic set_bit_le() later, we remove our own definition
and use a proper non-atomic bitops function: __set_bit_le().

Signed-off-by: Takuya Yoshikawa <yoshikawa.takuya@oss.ntt.co.jp>
Acked-by: Grant Grundler <grundler@parisc-linux.org>
---
 drivers/net/ethernet/dec/tulip/de2104x.c    |    7 ++-----
 drivers/net/ethernet/dec/tulip/tulip_core.c |    7 ++-----
 2 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/dec/tulip/de2104x.c b/drivers/net/ethernet/dec/tulip/de2104x.c
index 61cc093..77335853 100644
--- a/drivers/net/ethernet/dec/tulip/de2104x.c
+++ b/drivers/net/ethernet/dec/tulip/de2104x.c
@@ -661,9 +661,6 @@ static netdev_tx_t de_start_xmit (struct sk_buff *skb,
    new frame, not around filling de->setup_frame.  This is non-deterministic
    when re-entered but still correct. */
 
-#undef set_bit_le
-#define set_bit_le(i,p) do { ((char *)(p))[(i)/8] |= (1<<((i)%8)); } while(0)
-
 static void build_setup_frame_hash(u16 *setup_frm, struct net_device *dev)
 {
 	struct de_private *de = netdev_priv(dev);
@@ -673,12 +670,12 @@ static void build_setup_frame_hash(u16 *setup_frm, struct net_device *dev)
 	u16 *eaddrs;
 
 	memset(hash_table, 0, sizeof(hash_table));
-	set_bit_le(255, hash_table); 			/* Broadcast entry */
+	__set_bit_le(255, hash_table);			/* Broadcast entry */
 	/* This should work on big-endian machines as well. */
 	netdev_for_each_mc_addr(ha, dev) {
 		int index = ether_crc_le(ETH_ALEN, ha->addr) & 0x1ff;
 
-		set_bit_le(index, hash_table);
+		__set_bit_le(index, hash_table);
 	}
 
 	for (i = 0; i < 32; i++) {
diff --git a/drivers/net/ethernet/dec/tulip/tulip_core.c b/drivers/net/ethernet/dec/tulip/tulip_core.c
index c4f37ac..885700a 100644
--- a/drivers/net/ethernet/dec/tulip/tulip_core.c
+++ b/drivers/net/ethernet/dec/tulip/tulip_core.c
@@ -1010,9 +1010,6 @@ static int private_ioctl (struct net_device *dev, struct ifreq *rq, int cmd)
    new frame, not around filling tp->setup_frame.  This is non-deterministic
    when re-entered but still correct. */
 
-#undef set_bit_le
-#define set_bit_le(i,p) do { ((char *)(p))[(i)/8] |= (1<<((i)%8)); } while(0)
-
 static void build_setup_frame_hash(u16 *setup_frm, struct net_device *dev)
 {
 	struct tulip_private *tp = netdev_priv(dev);
@@ -1022,12 +1019,12 @@ static void build_setup_frame_hash(u16 *setup_frm, struct net_device *dev)
 	u16 *eaddrs;
 
 	memset(hash_table, 0, sizeof(hash_table));
-	set_bit_le(255, hash_table); 			/* Broadcast entry */
+	__set_bit_le(255, hash_table);			/* Broadcast entry */
 	/* This should work on big-endian machines as well. */
 	netdev_for_each_mc_addr(ha, dev) {
 		int index = ether_crc_le(ETH_ALEN, ha->addr) & 0x1ff;
 
-		set_bit_le(index, hash_table);
+		__set_bit_le(index, hash_table);
 	}
 	for (i = 0; i < 32; i++) {
 		*setup_frm++ = hash_table[i];
-- 
1.7.5.4

^ permalink raw reply related

* [PATCH 1/5] sfc: Use standard __{clear,set}_bit_le() functions
From: Takuya Yoshikawa @ 2012-06-13  4:02 UTC (permalink / raw)
  To: akpm
  Cc: bhutchings, grundler, arnd, benh, avi, mtosatti,
	linux-net-drivers, netdev, linux-kernel, linux-arch, kvm,
	takuya.yoshikawa
In-Reply-To: <20120613130054.b5695621.yoshikawa.takuya@oss.ntt.co.jp>

From: Ben Hutchings <bhutchings@solarflare.com>

There are now standard functions for dealing with little-endian bit
arrays, so use them instead of our own implementations.

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
Signed-off-by: Takuya Yoshikawa <yoshikawa.takuya@oss.ntt.co.jp>
---
 drivers/net/ethernet/sfc/efx.c        |    4 ++--
 drivers/net/ethernet/sfc/net_driver.h |   12 ------------
 drivers/net/ethernet/sfc/nic.c        |    4 ++--
 3 files changed, 4 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/sfc/efx.c b/drivers/net/ethernet/sfc/efx.c
index b95f2e1..ca2a348 100644
--- a/drivers/net/ethernet/sfc/efx.c
+++ b/drivers/net/ethernet/sfc/efx.c
@@ -1976,14 +1976,14 @@ static void efx_set_rx_mode(struct net_device *net_dev)
 		netdev_for_each_mc_addr(ha, net_dev) {
 			crc = ether_crc_le(ETH_ALEN, ha->addr);
 			bit = crc & (EFX_MCAST_HASH_ENTRIES - 1);
-			set_bit_le(bit, mc_hash->byte);
+			__set_bit_le(bit, mc_hash);
 		}
 
 		/* Broadcast packets go through the multicast hash filter.
 		 * ether_crc_le() of the broadcast address is 0xbe2612ff
 		 * so we always add bit 0xff to the mask.
 		 */
-		set_bit_le(0xff, mc_hash->byte);
+		__set_bit_le(0xff, mc_hash);
 	}
 
 	if (efx->port_enabled)
diff --git a/drivers/net/ethernet/sfc/net_driver.h b/drivers/net/ethernet/sfc/net_driver.h
index 0e57535..6f1a7f7 100644
--- a/drivers/net/ethernet/sfc/net_driver.h
+++ b/drivers/net/ethernet/sfc/net_driver.h
@@ -1080,18 +1080,6 @@ static inline struct efx_rx_buffer *efx_rx_buffer(struct efx_rx_queue *rx_queue,
 	return &rx_queue->buffer[index];
 }
 
-/* Set bit in a little-endian bitfield */
-static inline void set_bit_le(unsigned nr, unsigned char *addr)
-{
-	addr[nr / 8] |= (1 << (nr % 8));
-}
-
-/* Clear bit in a little-endian bitfield */
-static inline void clear_bit_le(unsigned nr, unsigned char *addr)
-{
-	addr[nr / 8] &= ~(1 << (nr % 8));
-}
-
 
 /**
  * EFX_MAX_FRAME_LEN - calculate maximum frame length
diff --git a/drivers/net/ethernet/sfc/nic.c b/drivers/net/ethernet/sfc/nic.c
index 4a9a5be..bb0172d 100644
--- a/drivers/net/ethernet/sfc/nic.c
+++ b/drivers/net/ethernet/sfc/nic.c
@@ -473,9 +473,9 @@ void efx_nic_init_tx(struct efx_tx_queue *tx_queue)
 
 		efx_reado(efx, &reg, FR_AA_TX_CHKSM_CFG);
 		if (tx_queue->queue & EFX_TXQ_TYPE_OFFLOAD)
-			clear_bit_le(tx_queue->queue, (void *)&reg);
+			__clear_bit_le(tx_queue->queue, &reg);
 		else
-			set_bit_le(tx_queue->queue, (void *)&reg);
+			__set_bit_le(tx_queue->queue, &reg);
 		efx_writeo(efx, &reg, FR_AA_TX_CHKSM_CFG);
 	}
 
-- 
1.7.5.4

^ permalink raw reply related

* [PATCH 0/5] Introduce generic set_bit_le() -v2
From: Takuya Yoshikawa @ 2012-06-13  4:00 UTC (permalink / raw)
  To: akpm
  Cc: bhutchings, grundler, arnd, benh, avi, mtosatti,
	linux-net-drivers, netdev, linux-kernel, linux-arch, kvm,
	takuya.yoshikawa

  [ Andrew, can you take this or should I send to other person?
    Note: the whole series is against linux-next. ]

KVM is using test_and_set_bit_le() for this missing function; this patch
series corrects this usage.

As some drivers have their own definitions of set_bit_le(), a bit of
preparation is also needed.

  Although these are differently implemented, especially for big-endian
  case, than the generic __set_bit_le(), it should not be a problem to
  use the latter since both maintainers prefer it.

Changes from v1:
  - sfc:     Ben made a patch
  - tulip:   followed suggestion by Grant
  - bitops:  added clear_bit_le -- suggested by Arnd
  - powerpc: added the same code


Ben Hutchings (1):
  sfc: Use standard __{clear,set}_bit_le() functions

Takuya Yoshikawa (4):
  drivers/net/ethernet/dec/tulip: Use standard __set_bit_le() function
  bitops: Introduce generic {clear,set}_bit_le()
  powerpc: bitops: Introduce {clear,set}_bit_le()
  KVM: Replace test_and_set_bit_le() in mark_page_dirty_in_slot() with set_bit_le()

 arch/powerpc/include/asm/bitops.h           |   10 ++++++++++
 drivers/net/ethernet/dec/tulip/de2104x.c    |    7 ++-----
 drivers/net/ethernet/dec/tulip/tulip_core.c |    7 ++-----
 drivers/net/ethernet/sfc/efx.c              |    4 ++--
 drivers/net/ethernet/sfc/net_driver.h       |   12 ------------
 drivers/net/ethernet/sfc/nic.c              |    4 ++--
 include/asm-generic/bitops/le.h             |   10 ++++++++++
 virt/kvm/kvm_main.c                         |    3 +--
 8 files changed, 29 insertions(+), 28 deletions(-)

-- 
1.7.5.4


^ permalink raw reply

* Re: [PATCH 4/7] usbnet: remove EVENT_DEV_OPEN flag
From: Ming Lei @ 2012-06-13  2:12 UTC (permalink / raw)
  To: Oliver Neukum; +Cc: David S. Miller, Greg Kroah-Hartman, netdev, linux-usb
In-Reply-To: <201206122014.28408.oliver@neukum.org>

On Wed, Jun 13, 2012 at 2:14 AM, Oliver Neukum <oliver@neukum.org> wrote:
> Am Dienstag, 12. Juni 2012, 03:19:42 schrieb Ming Lei:
>> EVENT_DEV_OPEN is introduced to mark if the interface is opened or
>> not, but we already have IFF_UP to handle it, so just
>> remove the flag and use IFF_UP.
>
> When is IFF_UP cleared? The flag is tested in usbnet_resume(),

The flag is cleared just after usbnet_stop completes.

> so it must be cleared before usbnet_stop() is called.

Yes, I see, otherwise system or runtime resume may happen
at the same time with usbnet_stop.

Thanking you for point it out.

Thanks,
-- 
Ming Lei

^ permalink raw reply

* Re: net/netfilter/nf_conntrack_proto_tcp.c:1606:9: error: ‘struct nf_proto_net’ has no member named ‘user’
From: Gao feng @ 2012-06-13  2:06 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: David Miller, wfg, netdev
In-Reply-To: <20120612160304.GA31427@1984>

于 2012年06月13日 00:03, Pablo Neira Ayuso 写道:
> On Tue, Jun 12, 2012 at 07:03:31PM +0800, Gao feng wrote:
>> 于 2012年06月12日 17:29, Pablo Neira Ayuso 写道:
>>
>>>> nf_proto_net.users has different meaning when SYSCTL enabled or disabled.
>>>>
>>>> when SYSCTL enabled,it means if both tcpv4 and tcpv6 register the sysctl,
>>>> it is increased when register sysctl success and decreased when unregister sysctl.
>>>> we can regard it as the refcnt of ctl_table.
>>>>
>>>> when SYSCTL disabled,it just used to identify if the proto's pernet data
>>>> has been initialized.
>>>
>>> We have to use two different counters for this. The conditional
>>> meaning of that variable is really confusing.
>>>
>> Hi David & Pablo
>>
>> Please have a look at this patch and tell me if it's OK.
>> it base on Pable's patch.
> 
> I think we have to merge those tcpv4_init_net and tcpv6_init_net
> functions into one single function tcp_init_net. Then, we can pass
> l4proto->l3proto to init_net:
> 
>         if (proto->init_net) {
>                 ret = proto->init_net(net, l4proto->l3proto);
>                 if (ret < 0)
>                         return ret;
>         }
> 
> Thus, we can check if this is IPv4 or IPv6 and initialize the compat
> part accordingly.

Agree, it will be more clearer and will decrease the redundancy codes.

> 
> Still, we have that pn->users thing:
> 
>         if (!pn->users++) {
>                 for (i = 0; i < TCP_CONNTRACK_TIMEOUT_MAX; i++)
>                         tn->timeouts[i] = tcp_timeouts[i];
> 
>                 tn->tcp_loose = nf_ct_tcp_loose;
>                 tn->tcp_be_liberal = nf_ct_tcp_be_liberal;
>                 tn->tcp_max_retrans = nf_ct_tcp_max_retrans;
>         }
> 
> Define some pn->initialized boolean. Set it to true at the end of
> the new tcp_init_net.
> 
> Similar thing for other protocol trackers.
> 
> Let me know if you are going to send me patches. In that case, please
> do it on top of the current tree.

Ok,I will clean up it as you said.

> 
> Once that has been cleaned up, we can prepare follow-up patches to
> move the sysctl code to nf_conntrack_proto_*_sysctl.c to reduce the
> ifdef pollution.
> 

^ permalink raw reply

* Re: [PATCH net-next] ethtool: Make more commands available to unprivileged processes
From: David Miller @ 2012-06-13  1:52 UTC (permalink / raw)
  To: bhutchings; +Cc: netdev, linux-net-drivers
In-Reply-To: <1339542341.15266.3.camel@bwh-desktop.uk.solarflarecom.com>

From: Ben Hutchings <bhutchings@solarflare.com>
Date: Wed, 13 Jun 2012 00:05:41 +0100

> 'Get' commands should generally not require CAP_NET_ADMIN, with
> the exception of those that expose internal state.
> 
> Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
> ---
> The one command I'm not sure about is ETHTOOL_STATS.  It might reveal
> too much detail about network traffic.  That said, /proc/net/dev and
> /sys/class/net/*/statistics are already world-readable.

Applied, it just means we need to scrutinize what people put into the
stats a little bit more.

^ permalink raw reply

* Re: [PATCH 1/1] net-next: add dev_loopback_xmit() to avoid duplicate code
From: David Miller @ 2012-06-13  1:52 UTC (permalink / raw)
  To: michel
  Cc: netdev, linux-kernel, kuznet, jmorris, yoshfuji, kaber, edumazet,
	jpirko, mirq-linux, bhutchings
In-Reply-To: <1339532195.2701.8.camel@Thor>

From: Michel Machado <michel@digirati.com.br>
Date: Tue, 12 Jun 2012 16:16:35 -0400

> Add dev_loopback_xmit() in order to deduplicate functions
> ip_dev_loopback_xmit() (in net/ipv4/ip_output.c) and
> ip6_dev_loopback_xmit() (in net/ipv6/ip6_output.c).
> 
> I was about to reinvent the wheel when I noticed that
> ip_dev_loopback_xmit() and ip6_dev_loopback_xmit() do exactly what I
> need and are not IP-only functions, but they were not available to reuse
> elsewhere.
> 
> ip6_dev_loopback_xmit() does not have line "skb_dst_force(skb);", but I
> understand that this is harmless, and should be in dev_loopback_xmit().
> 
> Signed-off-by: Michel Machado <michel@digirati.com.br>

Applied.

^ permalink raw reply

* Re: [PATCH net-next] bonding: remove packet cloning in recv_probe()
From: David Miller @ 2012-06-13  1:52 UTC (permalink / raw)
  To: fubar; +Cc: eric.dumazet, netdev, andy, jbohac, nicolas.2p.debian, maze
In-Reply-To: <24088.1339548479@death.nxdomain>

From: Jay Vosburgh <fubar@us.ibm.com>
Date: Tue, 12 Jun 2012 17:47:59 -0700

> Eric Dumazet <eric.dumazet@gmail.com> wrote:
> 
>>From: Eric Dumazet <edumazet@google.com>
>>
>>Cloning all packets in input path have a significant cost.
>>
>>Use skb_header_pointer()/skb_copy_bits() instead of pskb_may_pull() so
>>that recv_probe handlers (bond_3ad_lacpdu_recv / bond_arp_rcv /
>>rlb_arp_recv ) dont touch input skb.
>>
>>bond_handle_frame() can avoid the skb_clone()/dev_kfree_skb()
>>
>>Signed-off-by: Eric Dumazet <edumazet@google.com>
>>Cc: Jay Vosburgh <fubar@us.ibm.com>
>>Cc: Andy Gospodarek <andy@greyhouse.net>
>>Cc: Jiri Bohac <jbohac@suse.cz>
>>Cc: Nicolas de Pesloüan <nicolas.2p.debian@free.fr>
>>Cc: Maciej Żenczykowski <maze@google.com>
> 
> 	This looks really good to me.
> 
> 	-J
> 
> Signed-off-by: Jay Vosburgh <fubar@us.ibm.com>

Applied.

^ permalink raw reply

* Re: [PATCH 0/7] usbnet: misc cleanup
From: David Miller @ 2012-06-13  1:51 UTC (permalink / raw)
  To: tom.leiming-Re5JQEeQqe8AvxtiuMwx3w
  Cc: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r, oneukum-l3A5Bk7waGM,
	netdev-u79uwXL29TY76Z2rM5mHXA, linux-usb-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1339463985-9006-1-git-send-email-tom.leiming-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

From: Ming Lei <tom.leiming-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Date: Tue, 12 Jun 2012 09:19:38 +0800

> This patchset does some cleanup on usbnet.

I've applied all of these patches except the EVENT_DEV_OPEN removal
which still needs some discussion.
--
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

* Re: [PATCH net-next] bonding: remove packet cloning in recv_probe()
From: Jay Vosburgh @ 2012-06-13  0:47 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David Miller, netdev, Andy Gospodarek, Jiri Bohac,
	Nicolas de =?ISO-8859-1?Q?Peslo=FCan?=,
	Maciej =?UTF-8?Q?=C5=BBenczykowski?=
In-Reply-To: <1339478587.22704.16.camel@edumazet-glaptop>

Eric Dumazet <eric.dumazet@gmail.com> wrote:

>From: Eric Dumazet <edumazet@google.com>
>
>Cloning all packets in input path have a significant cost.
>
>Use skb_header_pointer()/skb_copy_bits() instead of pskb_may_pull() so
>that recv_probe handlers (bond_3ad_lacpdu_recv / bond_arp_rcv /
>rlb_arp_recv ) dont touch input skb.
>
>bond_handle_frame() can avoid the skb_clone()/dev_kfree_skb()
>
>Signed-off-by: Eric Dumazet <edumazet@google.com>
>Cc: Jay Vosburgh <fubar@us.ibm.com>
>Cc: Andy Gospodarek <andy@greyhouse.net>
>Cc: Jiri Bohac <jbohac@suse.cz>
>Cc: Nicolas de Pesloüan <nicolas.2p.debian@free.fr>
>Cc: Maciej Żenczykowski <maze@google.com>

	This looks really good to me.

	-J

Signed-off-by: Jay Vosburgh <fubar@us.ibm.com>


>---
> drivers/net/bonding/bond_3ad.c  |   11 +++++---
> drivers/net/bonding/bond_3ad.h  |    4 +--
> drivers/net/bonding/bond_alb.c  |   20 ++++------------
> drivers/net/bonding/bond_main.c |   37 ++++++++++++++++--------------
> drivers/net/bonding/bonding.h   |    4 +--
> 5 files changed, 36 insertions(+), 40 deletions(-)
>
>diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
>index 3463b46..3031e04 100644
>--- a/drivers/net/bonding/bond_3ad.c
>+++ b/drivers/net/bonding/bond_3ad.c
>@@ -2460,18 +2460,21 @@ out:
> 	return NETDEV_TX_OK;
> }
>
>-int bond_3ad_lacpdu_recv(struct sk_buff *skb, struct bonding *bond,
>-			  struct slave *slave)
>+int bond_3ad_lacpdu_recv(const struct sk_buff *skb, struct bonding *bond,
>+			 struct slave *slave)
> {
> 	int ret = RX_HANDLER_ANOTHER;
>+	struct lacpdu *lacpdu, _lacpdu;
>+
> 	if (skb->protocol != PKT_TYPE_LACPDU)
> 		return ret;
>
>-	if (!pskb_may_pull(skb, sizeof(struct lacpdu)))
>+	lacpdu = skb_header_pointer(skb, 0, sizeof(_lacpdu), &_lacpdu);
>+	if (!lacpdu)
> 		return ret;
>
> 	read_lock(&bond->lock);
>-	ret = bond_3ad_rx_indication((struct lacpdu *) skb->data, slave, skb->len);
>+	ret = bond_3ad_rx_indication(lacpdu, slave, skb->len);
> 	read_unlock(&bond->lock);
> 	return ret;
> }
>diff --git a/drivers/net/bonding/bond_3ad.h b/drivers/net/bonding/bond_3ad.h
>index 5ee7e3c..0cfaa4a 100644
>--- a/drivers/net/bonding/bond_3ad.h
>+++ b/drivers/net/bonding/bond_3ad.h
>@@ -274,8 +274,8 @@ void bond_3ad_adapter_duplex_changed(struct slave *slave);
> void bond_3ad_handle_link_change(struct slave *slave, char link);
> int  bond_3ad_get_active_agg_info(struct bonding *bond, struct ad_info *ad_info);
> int bond_3ad_xmit_xor(struct sk_buff *skb, struct net_device *dev);
>-int bond_3ad_lacpdu_recv(struct sk_buff *skb, struct bonding *bond,
>-			  struct slave *slave);
>+int bond_3ad_lacpdu_recv(const struct sk_buff *skb, struct bonding *bond,
>+			 struct slave *slave);
> int bond_3ad_set_carrier(struct bonding *bond);
> void bond_3ad_update_lacp_rate(struct bonding *bond);
> #endif //__BOND_3AD_H__
>diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
>index 0f59c15..ef3791a 100644
>--- a/drivers/net/bonding/bond_alb.c
>+++ b/drivers/net/bonding/bond_alb.c
>@@ -342,27 +342,17 @@ static void rlb_update_entry_from_arp(struct bonding *bond, struct arp_pkt *arp)
> 	_unlock_rx_hashtbl_bh(bond);
> }
>
>-static int rlb_arp_recv(struct sk_buff *skb, struct bonding *bond,
>-			 struct slave *slave)
>+static int rlb_arp_recv(const struct sk_buff *skb, struct bonding *bond,
>+			struct slave *slave)
> {
>-	struct arp_pkt *arp;
>+	struct arp_pkt *arp, _arp;
>
> 	if (skb->protocol != cpu_to_be16(ETH_P_ARP))
> 		goto out;
>
>-	arp = (struct arp_pkt *) skb->data;
>-	if (!arp) {
>-		pr_debug("Packet has no ARP data\n");
>+	arp = skb_header_pointer(skb, 0, sizeof(_arp), &_arp);
>+	if (!arp)
> 		goto out;
>-	}
>-
>-	if (!pskb_may_pull(skb, arp_hdr_len(bond->dev)))
>-		goto out;
>-
>-	if (skb->len < sizeof(struct arp_pkt)) {
>-		pr_debug("Packet is too small to be an ARP\n");
>-		goto out;
>-	}
>
> 	if (arp->op_code == htons(ARPOP_REPLY)) {
> 		/* update rx hash table for this ARP */
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index 2ee8cf9..9e2301e 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -1444,8 +1444,8 @@ static rx_handler_result_t bond_handle_frame(struct sk_buff **pskb)
> 	struct sk_buff *skb = *pskb;
> 	struct slave *slave;
> 	struct bonding *bond;
>-	int (*recv_probe)(struct sk_buff *, struct bonding *,
>-				struct slave *);
>+	int (*recv_probe)(const struct sk_buff *, struct bonding *,
>+			  struct slave *);
> 	int ret = RX_HANDLER_ANOTHER;
>
> 	skb = skb_share_check(skb, GFP_ATOMIC);
>@@ -1462,15 +1462,10 @@ static rx_handler_result_t bond_handle_frame(struct sk_buff **pskb)
>
> 	recv_probe = ACCESS_ONCE(bond->recv_probe);
> 	if (recv_probe) {
>-		struct sk_buff *nskb = skb_clone(skb, GFP_ATOMIC);
>-
>-		if (likely(nskb)) {
>-			ret = recv_probe(nskb, bond, slave);
>-			dev_kfree_skb(nskb);
>-			if (ret == RX_HANDLER_CONSUMED) {
>-				consume_skb(skb);
>-				return ret;
>-			}
>+		ret = recv_probe(skb, bond, slave);
>+		if (ret == RX_HANDLER_CONSUMED) {
>+			consume_skb(skb);
>+			return ret;
> 		}
> 	}
>
>@@ -2737,25 +2732,31 @@ static void bond_validate_arp(struct bonding *bond, struct slave *slave, __be32
> 	}
> }
>
>-static int bond_arp_rcv(struct sk_buff *skb, struct bonding *bond,
>-			 struct slave *slave)
>+static int bond_arp_rcv(const struct sk_buff *skb, struct bonding *bond,
>+			struct slave *slave)
> {
>-	struct arphdr *arp;
>+	struct arphdr *arp = (struct arphdr *)skb->data;
> 	unsigned char *arp_ptr;
> 	__be32 sip, tip;
>+	int alen;
>
> 	if (skb->protocol != __cpu_to_be16(ETH_P_ARP))
> 		return RX_HANDLER_ANOTHER;
>
> 	read_lock(&bond->lock);
>+	alen = arp_hdr_len(bond->dev);
>
> 	pr_debug("bond_arp_rcv: bond %s skb->dev %s\n",
> 		 bond->dev->name, skb->dev->name);
>
>-	if (!pskb_may_pull(skb, arp_hdr_len(bond->dev)))
>-		goto out_unlock;
>+	if (alen > skb_headlen(skb)) {
>+		arp = kmalloc(alen, GFP_ATOMIC);
>+		if (!arp)
>+			goto out_unlock;
>+		if (skb_copy_bits(skb, 0, arp, alen) < 0)
>+			goto out_unlock;
>+	}
>
>-	arp = arp_hdr(skb);
> 	if (arp->ar_hln != bond->dev->addr_len ||
> 	    skb->pkt_type == PACKET_OTHERHOST ||
> 	    skb->pkt_type == PACKET_LOOPBACK ||
>@@ -2790,6 +2791,8 @@ static int bond_arp_rcv(struct sk_buff *skb, struct bonding *bond,
>
> out_unlock:
> 	read_unlock(&bond->lock);
>+	if (arp != (struct arphdr *)skb->data)
>+		kfree(arp);
> 	return RX_HANDLER_ANOTHER;
> }
>
>diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
>index 4581aa5..f8af2fc 100644
>--- a/drivers/net/bonding/bonding.h
>+++ b/drivers/net/bonding/bonding.h
>@@ -218,8 +218,8 @@ struct bonding {
> 	struct   slave *primary_slave;
> 	bool     force_primary;
> 	s32      slave_cnt; /* never change this value outside the attach/detach wrappers */
>-	int     (*recv_probe)(struct sk_buff *, struct bonding *,
>-			       struct slave *);
>+	int     (*recv_probe)(const struct sk_buff *, struct bonding *,
>+			      struct slave *);
> 	rwlock_t lock;
> 	rwlock_t curr_slave_lock;
> 	u8	 send_peer_notif;
>
>

^ permalink raw reply

* ethtool 3.4.1 released
From: Ben Hutchings @ 2012-06-13  0:31 UTC (permalink / raw)
  To: netdev

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

ethtool version 3.4.1 has been released.  This fixes a regression in
version 3.4.

Home page: https://ftp.kernel.org/pub/software/network/ethtool/
Download link:
https://ftp.kernel.org/pub/software/network/ethtool/ethtool-3.4.1.tar.gz

Release notes:

	* Fix: Work around failure of ETHTOOL_GSSET_INFO for unprivileged
	  users (-k option)
	* Fix: Report any unexpected error code from ETHTOOL_GSSET_INFO
	  (-k and -K options)
	* Doc: Fix the date of the man page to match the last update

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.


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

^ permalink raw reply

* Re: [PATCH v2] bonding: Fix corrupted queue_mapping
From: Neil Horman @ 2012-06-13  0:26 UTC (permalink / raw)
  To: David Miller; +Cc: eric.dumazet, netdev, therbert, john.r.fastabend, roland
In-Reply-To: <20120612.153058.508089648695433178.davem@davemloft.net>

On Tue, Jun 12, 2012 at 03:30:58PM -0700, David Miller wrote:
> From: Neil Horman <nhorman@tuxdriver.com>
> Date: Tue, 12 Jun 2012 13:16:08 -0400
> 
> > On Tue, Jun 12, 2012 at 06:03:51PM +0200, Eric Dumazet wrote:
> >> From: Eric Dumazet <edumazet@google.com>
> >> 
> >> In the transmit path of the bonding driver, skb->cb is used to
> >> stash the skb->queue_mapping so that the bonding device can set its
> >> own queue mapping.  This value becomes corrupted since the skb->cb is
> >> also used in __dev_xmit_skb.
> >> 
> >> When transmitting through bonding driver, bond_select_queue is
> >> called from dev_queue_xmit.  In bond_select_queue the original
> >> skb->queue_mapping is copied into skb->cb (via bond_queue_mapping)
> >> and skb->queue_mapping is overwritten with the bond driver queue.
> >> 
> >> Subsequently in dev_queue_xmit, __dev_xmit_skb is called which writes
> >> the packet length into skb->cb, thereby overwriting the stashed
> >> queue mappping.  In bond_dev_queue_xmit (called from hard_start_xmit),
> >> the queue mapping for the skb is set to the stashed value which is now
> >> the skb length and hence is an invalid queue for the slave device.
> >> 
> >> If we want to save skb->queue_mapping into skb->cb[], best place is to
> >> add a field in struct qdisc_skb_cb, to make sure it wont conflict with
> >> other layers (eg : Qdiscc, Infiniband...)
> >> 
> >> This patchs also makes sure (struct qdisc_skb_cb)->data is aligned on 8
> >> bytes :
> >> 
> >> netem qdisc for example assumes it can store an u64 in it, without
> >> misalignment penalty.
> >> 
> >> Note : we only have 20 bytes left in (struct qdisc_skb_cb)->data[].
> >> The largest user is CHOKe and it fills it.
> >> 
> >> Based on a previous patch from Tom Herbert.
> >> 
> >> Signed-off-by: Eric Dumazet <edumazet@google.com>
> >> Reported-by: Tom Herbert <therbert@google.com>
> >> Cc: John Fastabend <john.r.fastabend@intel.com>
> >> Cc: Roland Dreier <roland@kernel.org>
> > Acked-by: Neil Horman <nhorman@tuxdriver.com>
> > 
> > Looking at this, it would be really nice if we could have some sort of layer
> > independent space in an skb.  It seems we're often looking to shoehorn more
> > stuff into the control bock.
> 
> Applied.
> 
> The problem is that we always have layers that want to record something before
> the packet goes through the scheduler, then be able to retrieve it afterwards.
> Even more problematic are entities that can encapsulte multiple times for a
> single packet.
> 
Yeah, those are both problematic, although perhaps not insurmountable.  I know
Eric has attempted to implement a destructor chain in the past.  I was more
interested in just being able to get per-context scratch space on an skb as a
start, but clearly the two go hand in hand.
 
> If we supported bonds of bonds, even Eric's patch here is insufficient.
> 
Agreed.  I think I'm going to try my hand at putting something together though,
just out of curiosity.  I'll try have something RFC in a few weeks.

Thanks & Regards
Neil

^ permalink raw reply


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