Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH 0/7] AlacrityVM guest drivers Reply-To:
From: Gregory Haskins @ 2009-08-06 14:06 UTC (permalink / raw)
  To: Avi Kivity
  Cc: alacrityvm-devel, Michael S. Tsirkin, kvm, linux-kernel, netdev
In-Reply-To: <4A7AE150.7040009@redhat.com>

>>> On 8/6/2009 at  9:57 AM, in message <4A7AE150.7040009@redhat.com>, Avi Kivity
<avi@redhat.com> wrote: 
> On 08/06/2009 04:45 PM, Gregory Haskins wrote:
>>
>>> (though still rooting for virtio).
>>>      
>>
>> Heh...not to belabor the point to death, but virtio is orthogonal (you keep 
> forgetting that ;).
>>
>> Its really the vbus device-model vs the qemu device-model (and possibly vs the 
> "in-kernel pci emulation" model that I believe Michael is working on).
>>
>> You can run virtio on any of those three.
>>    
> 
> It's not orthogonal.  virtio is one set of ABI+guest drivers+host 
> support to get networking on kvm guests.  AlacrityVM's vbus-based 
> drivers are another set of ABI+guest drivers+host support to get 
> networking on kvm guests.  That makes them competitors (two different 
> ways to do one thing), not orthogonal.

Thats not accurate, though.

The virtio stack is modular.  For instance, with virtio-net, you have

  (guest-side)
|--------------------------
| virtio-net
|--------------------------
| virtio-ring
|--------------------------
| virtio-bus
|--------------------------
| virtio-pci
|--------------------------
                      |
                   (pci)
                      |
|--------------------------
| kvm.ko
|--------------------------
| qemu
|--------------------------
| tun-tap
|--------------------------
| netif
|--------------------------
     (host-side)

We can exchange out the "virtio-pci" module like this:

  (guest-side)
|--------------------------
| virtio-net
|--------------------------
| virtio-ring
|--------------------------
| virtio-bus
|--------------------------
| virtio-vbus
|--------------------------
| vbus-proxy
|--------------------------
| vbus-connector
|--------------------------
                      |
                   (vbus)
                      |
|--------------------------
| kvm.ko
|--------------------------
| vbus-connector
|--------------------------
| vbus
|--------------------------
| virtio-net-tap (vbus model)
|--------------------------
| netif
|--------------------------
     (host-side)


So virtio-net runs unmodified.  What is "competing" here is "virtio-pci" vs "virtio-vbus".  Also, venet vs virtio-net are technically competing.  But to say "virtio vs vbus" is inaccurate, IMO.

HTH
-Greg


^ permalink raw reply

* Re: [PATCH 0/7] AlacrityVM guest drivers Reply-To:
From: Michael S. Tsirkin @ 2009-08-06 13:59 UTC (permalink / raw)
  To: Gregory Haskins; +Cc: Avi Kivity, alacrityvm-devel, kvm, linux-kernel, netdev
In-Reply-To: <4A7AA63A0200005A00051BE2@sinclair.provo.novell.com>

On Thu, Aug 06, 2009 at 07:45:30AM -0600, Gregory Haskins wrote:
> > (though still rooting for virtio).
> 
> Heh...not to belabor the point to death, but virtio is orthogonal (you keep forgetting that ;).

venet and virtio aren't orthogonal, are they?

-- 
MST

^ permalink raw reply

* Re: [PATCH 0/7] AlacrityVM guest drivers Reply-To:
From: Avi Kivity @ 2009-08-06 13:57 UTC (permalink / raw)
  To: Gregory Haskins
  Cc: alacrityvm-devel, Michael S. Tsirkin, kvm, linux-kernel, netdev
In-Reply-To: <4A7AA63A0200005A00051BE2@sinclair.provo.novell.com>

On 08/06/2009 04:45 PM, Gregory Haskins wrote:
>
>> (though still rooting for virtio).
>>      
>
> Heh...not to belabor the point to death, but virtio is orthogonal (you keep forgetting that ;).
>
> Its really the vbus device-model vs the qemu device-model (and possibly vs the "in-kernel pci emulation" model that I believe Michael is working on).
>
> You can run virtio on any of those three.
>    

It's not orthogonal.  virtio is one set of ABI+guest drivers+host 
support to get networking on kvm guests.  AlacrityVM's vbus-based 
drivers are another set of ABI+guest drivers+host support to get 
networking on kvm guests.  That makes them competitors (two different 
ways to do one thing), not orthogonal.

-- 
error compiling committee.c: too many arguments to function


^ permalink raw reply

* Re: [PATCH 1/7] shm-signal: shared-memory signals
From: Arnd Bergmann @ 2009-08-06 13:56 UTC (permalink / raw)
  To: Gregory Haskins; +Cc: linux-kernel, alacrityvm-devel, netdev
In-Reply-To: <20090803171735.17268.37490.stgit@dev.haskins.net>

On Monday 03 August 2009, Gregory Haskins wrote:
> shm-signal provides a generic shared-memory based bidirectional
> signaling mechanism.  It is used in conjunction with an existing
> signal transport (such as posix-signals, interrupts, pipes, etc) to
> increase the efficiency of the transport since the state information
> is directly accessible to both sides of the link.  The shared-memory
> design provides very cheap access to features such as event-masking
> and spurious delivery mititgation, and is useful implementing higher
> level shared-memory constructs such as rings.

Looks like a very useful feature in general.

> +struct shm_signal_irq {
> +       __u8                  enabled;
> +       __u8                  pending;
> +       __u8                  dirty;
> +};

Won't this layout cause cache line ping pong? Other schemes I have
seen try to separate the bits so that each cache line is written to
by only one side. This gets much more interesting if the two sides
are on remote ends of an I/O link, e.g. using a nontransparent
PCI bridge, where you only want to send stores over the wire, but
never fetches or even read-modify-write cycles.

Your code is probably optimal if you only communicate between host
and guest code on the same CPU, but not so good if it crosses NUMA
nodes or worse.

> +struct shm_signal_desc {
> +       __u32                 magic;
> +       __u32                 ver;
> +       struct shm_signal_irq irq[2];
> +};

This data structure has implicit padding of two bytes at the end.
How about adding another '__u16 reserved' to make it explicit?

> +	/*
> +	 * We always mark the remote side as dirty regardless of whether
> +	 * they need to be notified.
> +	 */
> +	irq->dirty = 1;
> +	wmb();   /* dirty must be visible before we test the pending state */
> +
> +	if (irq->enabled && !irq->pending) {
> +		rmb();
> +
> +		/*
> +		 * If the remote side has enabled notifications, and we do
> +		 * not see a notification pending, we must inject a new one.
> +		 */
> +		irq->pending = 1;
> +		wmb(); /* make it visible before we do the injection */
> +
> +		s->ops->inject(s);
> +	}

Barriers always confuse me, but the rmb() looks slightly wrong. AFAIU
it only prevents reads after the barrier from being done before the
barrier, but you don't do any reads after it.

The (irq->enabled && !irq->pending) check could be done before the
irq->dirty = 1 arrives at the bus, but that does not seem to hurt, it
would at most cause a duplicate ->inject().

Regarding the scope of the barrier, did you intentionally use the
global versions (rmb()/wmb()) and not the lighter single-system
(smp_rmb()/smp_wmb()) versions? Your version should cope with remote
links over PCI but looks otherwise optimized for local use, as I
wrote above.

	Arnd <><

^ permalink raw reply

* [PATCH] irda: Fix irda_getname() leak
From: Eric Dumazet @ 2009-08-06 13:55 UTC (permalink / raw)
  To: David S. Miller; +Cc: Linux Netdev List

irda_getname() can leak kernel memory to user.

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
diff --git a/net/irda/af_irda.c b/net/irda/af_irda.c
index cb762c8..3ec2b43 100644
--- a/net/irda/af_irda.c
+++ b/net/irda/af_irda.c
@@ -714,6 +714,7 @@ static int irda_getname(struct socket *sock, struct sockaddr *uaddr,
 	struct sock *sk = sock->sk;
 	struct irda_sock *self = irda_sk(sk);
 
+	memset(&saddr, 0, sizeof(saddr));
 	if (peer) {
 		if (sk->sk_state != TCP_ESTABLISHED)
 			return -ENOTCONN;

^ permalink raw reply related

* [PATCH] econet: Fix econet_getname() leak
From: Eric Dumazet @ 2009-08-06 13:48 UTC (permalink / raw)
  To: David S. Miller; +Cc: Linux Netdev List

econet_getname() can leak kernel memory to user.

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
diff --git a/net/econet/af_econet.c b/net/econet/af_econet.c
index e114da7..0e0254f 100644
--- a/net/econet/af_econet.c
+++ b/net/econet/af_econet.c
@@ -520,6 +520,7 @@ static int econet_getname(struct socket *sock, struct sockaddr *uaddr,
 	if (peer)
 		return -EOPNOTSUPP;
 
+	memset(sec, 0, sizeof(*sec));
 	mutex_lock(&econet_mutex);
 
 	sk = sock->sk;

^ permalink raw reply related

* Re: [PATCH 0/7] AlacrityVM guest drivers Reply-To:
From: Gregory Haskins @ 2009-08-06 13:45 UTC (permalink / raw)
  To: Avi Kivity
  Cc: alacrityvm-devel, Michael S. Tsirkin, kvm, linux-kernel, netdev
In-Reply-To: <4A7ADE23.5010208@redhat.com>

>>> On 8/6/2009 at  9:44 AM, in message <4A7ADE23.5010208@redhat.com>, Avi Kivity
<avi@redhat.com> wrote: 
> On 08/06/2009 04:03 PM, Gregory Haskins wrote:
>>> It's true that vbus is a separate project (in fact even virtio is
>>> completely separate from kvm).  Still I think it would be of interest to
>>> many kvm@ readers.
>>>      
>>
>> Well, my goal was to not annoy KVM readers. ;)  So if you feel as though 
> there is benefit to having all of KVM CC'd and I won't be annoying everyone, 
> I see no problem in cross posting.
>>    
> 
> I can only speak for myself, I'm interested in this project

In that case, the best solution is probably to have you (and anyone else interested) to sign up, then:

https://lists.sourceforge.net/lists/listinfo/alacrityvm-devel
https://lists.sourceforge.net/lists/listinfo/alacrityvm-users


> (though still rooting for virtio).

Heh...not to belabor the point to death, but virtio is orthogonal (you keep forgetting that ;).

Its really the vbus device-model vs the qemu device-model (and possibly vs the "in-kernel pci emulation" model that I believe Michael is working on).

You can run virtio on any of those three.

> 
>> Would you like to see all conversations, or just ones related to code (and, 
> of course, KVM relevant items)
> 
> I guess internal vbus changes won't be too interesting for most readers, 
> but new releases, benchmarks, and kvm-related stuff will be welcome on 
> the kvm list.

Ok, I was planning on that anyway.

Regards,
-Greg





^ permalink raw reply

* Re: [PATCH 0/7] AlacrityVM guest drivers Reply-To:
From: Avi Kivity @ 2009-08-06 13:44 UTC (permalink / raw)
  To: Gregory Haskins
  Cc: alacrityvm-devel, Michael S. Tsirkin, kvm, linux-kernel, netdev
In-Reply-To: <4A7A9C6B0200005A00051BB9@sinclair.provo.novell.com>

On 08/06/2009 04:03 PM, Gregory Haskins wrote:
>> It's true that vbus is a separate project (in fact even virtio is
>> completely separate from kvm).  Still I think it would be of interest to
>> many kvm@ readers.
>>      
>
> Well, my goal was to not annoy KVM readers. ;)  So if you feel as though there is benefit to having all of KVM CC'd and I won't be annoying everyone, I see no problem in cross posting.
>    

I can only speak for myself, I'm interested in this project (though 
still rooting for virtio).

> Would you like to see all conversations, or just ones related to code (and, of course, KVM relevant items)

I guess internal vbus changes won't be too interesting for most readers, 
but new releases, benchmarks, and kvm-related stuff will be welcome on 
the kvm list.

-- 
error compiling committee.c: too many arguments to function

^ permalink raw reply

* Re: [RFC PATCH v1] tun: Cleanup error handling in tun_set_iff()
From: Herbert Xu @ 2009-08-06 13:37 UTC (permalink / raw)
  To: Eric W. Biederman; +Cc: Paul Moore, netdev, David Miller
In-Reply-To: <m1hbwlkzt6.fsf@fess.ebiederm.org>

On Thu, Aug 06, 2009 at 03:21:41AM -0700, Eric W. Biederman wrote:
>
> Two threads one file descriptor.  Both simultaneously attempt to 
> attach to a tun device.  One will fail, the other succeed.
> 
> At least that is how I read the locking.

Yes but the "race" fixed by this patch is centred on the tun_attach
call for a newly created network device.  As tun_set_iff occurs
under RTNL, the second thread cannot start attaching until the
creation thread has completed.  IOW the thread that creates the
net device should always succeed in attaching.

If two threads try to attach to the same device that was already
created then yes one will fail and the other succeed.  However,
AFAICS that case has nothing to do with this patch.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* [PATCH] rose: Fix rose_getname() leak
From: Eric Dumazet @ 2009-08-06 13:34 UTC (permalink / raw)
  To: David S. Miller; +Cc: Linux Netdev List

rose_getname() can leak kernel memory to user.

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
diff --git a/net/rose/af_rose.c b/net/rose/af_rose.c
index f0a76f6..e5f478c 100644
--- a/net/rose/af_rose.c
+++ b/net/rose/af_rose.c
@@ -954,6 +954,7 @@ static int rose_getname(struct socket *sock, struct sockaddr *uaddr,
 	struct rose_sock *rose = rose_sk(sk);
 	int n;
 
+	memset(srose, 0, sizeof(*srose));
 	if (peer != 0) {
 		if (sk->sk_state != TCP_ESTABLISHED)
 			return -ENOTCONN;

^ permalink raw reply related

* [PATCH] netrom: Fix nr_getname() leak
From: Eric Dumazet @ 2009-08-06 13:31 UTC (permalink / raw)
  To: David S. Miller; +Cc: Linux Netdev List

nr_getname() can leak kernel memory to user.

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
diff --git a/net/netrom/af_netrom.c b/net/netrom/af_netrom.c
index ce51ce0..ce1a34b 100644
--- a/net/netrom/af_netrom.c
+++ b/net/netrom/af_netrom.c
@@ -847,6 +847,7 @@ static int nr_getname(struct socket *sock, struct sockaddr *uaddr,
 		sax->fsa_ax25.sax25_family = AF_NETROM;
 		sax->fsa_ax25.sax25_ndigis = 1;
 		sax->fsa_ax25.sax25_call   = nr->user_addr;
+		memset(sax->fsa_digipeater, 0, sizeof(sax->fsa_digipeater));
 		sax->fsa_digipeater[0]     = nr->dest_addr;
 		*uaddr_len = sizeof(struct full_sockaddr_ax25);
 	} else {

^ permalink raw reply related

* Re: [PATCH 0/7] AlacrityVM guest drivers Reply-To:
From: Gregory Haskins @ 2009-08-06 13:03 UTC (permalink / raw)
  To: Avi Kivity
  Cc: alacrityvm-devel, Michael S. Tsirkin, kvm, linux-kernel, netdev
In-Reply-To: <4A7AD29E.50800@redhat.com>

>>> On 8/6/2009 at  8:54 AM, in message <4A7AD29E.50800@redhat.com>, Avi Kivity
<avi@redhat.com> wrote: 
> On 08/06/2009 03:08 PM, Gregory Haskins wrote:
>>> Merging the guest first means relying on
>>> kernel interface from an out of tree driver, which well might change
>>> before it goes in.
>>>      
>>
>> ABI compatibility is already addressed/handled, so even if that is true its 
> not a problem.
>>
>>    
> 
> Really the correct way to address the ABI is to publish a spec and write 
> both host and guest drivers to that.  Unfortunately we didn't do this 
> with virtio.
> 
> It becomes more important when you have multiple implementations (e.g. 
> Windows drivers).
> 
>>>> This series implements the guest-side drivers for accelerated IO
>>>> when running on top of the AlacrityVM hypervisor, the details of
>>>> which you can find here:
>>>>
>>>> http://developer.novell.com/wiki/index.php/AlacrityVM
>>>>        
>>> Since AlacrityVM is kvm based, Cc kvm@vger.kernel.org.
>>>      
>>
>> I *can* do that, but there is nothing in these drivers that is KVM specific 
> (its all pure PCI and VBUS).  I've already made the general announcement 
> about the project/ml cross posted to KVM for anyone that might be interested, 
> but I figure I will spare the general KVM list the details unless something 
> specifically pertains to, or affects, KVM.  For instance, when I get to 
> pushing the hypervisor side, I still need to work on getting that 
> 'xinterface' patch to you guys.  I would certainly be CC'ing kvm@vger when 
> that happens since it modifies the KVM code.
>>
>> So instead, I would just encourage anyone interested (such as yourself) to 
> join the alacrity list so I don't bother the KVM community unless absolutely 
> necessary.
>>    
> 
> It's true that vbus is a separate project (in fact even virtio is 
> completely separate from kvm).  Still I think it would be of interest to 
> many kvm@ readers.

Well, my goal was to not annoy KVM readers. ;)  So if you feel as though there is benefit to having all of KVM CC'd and I won't be annoying everyone, I see no problem in cross posting.

Would you like to see all conversations, or just ones related to code (and, of course, KVM relevant items)?

Regards,
-Greg




^ permalink raw reply

* Re: [PATCH 0/7] AlacrityVM guest drivers Reply-To:
From: Gregory Haskins @ 2009-08-06 13:00 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: alacrityvm-devel, kvm, linux-kernel, netdev
In-Reply-To: <20090806122449.GC11038@redhat.com>

>>> On 8/6/2009 at  8:24 AM, in message <20090806122449.GC11038@redhat.com>,
"Michael S. Tsirkin" <mst@redhat.com> wrote: 
> On Thu, Aug 06, 2009 at 06:08:27AM -0600, Gregory Haskins wrote:
>> Hi Michael,
>> 
>> >>> On 8/6/2009 at  4:19 AM, in message <20090806081955.GA9752@redhat.com>,
>> "Michael S. Tsirkin" <mst@redhat.com> wrote: 
>> > On Mon, Aug 03, 2009 at 01:17:30PM -0400, Gregory Haskins wrote:
>> >> (Applies to v2.6.31-rc5, proposed for linux-next after review is complete)
>> > 
>> > These are guest drivers, right?
>> 
>> Yep.
>> 
>> > Merging the guest first means relying on
>> > kernel interface from an out of tree driver, which well might change
>> > before it goes in.
>> 
>> ABI compatibility is already addressed/handled, so even if that is true its 
> not a problem.
> 
> It is? With versioning? Presumably this:
> 
> +       params.devid   = vdev->id;
> +       params.version = version;
> +
> +       ret = vbus_pci_hypercall(VBUS_PCI_HC_DEVOPEN,
> +                                &params, sizeof(params));
> +       if (ret < 0)
> +               return ret;

This is part of it.  There are various ABI version components (which, by the way, are only expected to only allow change while the code is experimental/alpha).  The other component is capability functions (such as NEGCAP in the venet driver).

> 
> Even assuming host even knows how to decode this structure (e.g.  some
> other host module doesn't use VBUS_PCI_HC_DEVOPEN),

This argument demonstrates a fundamental lack of understanding on how AlacrityVM works.  Please study the code more closely and you will see that your concern is illogical.  If it's still not clear, let me know and I will walk it through for you.

> checks the version
> and denies older guests, this might help guest not to crash, but guest
> still won't work.

Thats ok.  As I said above, the version number is just there for gross ABI protection and generally will never be changed once a driver is "official" (if at all).  We use things like capability-bit negotiation to allow backwards compat.

For an example, see drivers/net/vbus-enet.c, line 703:

http://git.kernel.org/?p=linux/kernel/git/ghaskins/alacrityvm/linux-2.6.git;a=blob;f=drivers/net/vbus-enet.c;h=7220f43723adc5b0bece1bc37974fae1b034cd9e;hb=b3b2339efbd4e754b1c85f8bc8f85f21a1a1f509#l703

venet exposes a verb "NEGCAP" (negotiate capabilities), which is used to extend the ABI.  The version number you quote above (on the device open) is really just a check to make sure the NEGCAP ABI is compatible.  The rest of the abi is negotiated at runtime with capability feature bits.

FWIW; I decided to not built a per-device capability into the low-level vbus protocol (e.g. there is no VBUS_PCI_HC_NEGCAP) because I felt as though the individual devices could better express their own capability mechanism, rather than try to generalize it.  Therefore it is up to each device to define its own mechanism, presumably using a verb from its own private call() namespace (as venet has done).

> 
>> > Would it make more sense to start merging with the host side of the 
> project?
>> 
>> Not necessarily, no.  These are drivers for a "device", so its no
>> different than merging any other driver really.  This is especially
>> true since the hypervisor is also already published and freely
>> available today, so anyone can start using it.
> 
> The difference is clear to me: devices do not get to set kernel/userspace
> interfaces. This "device" depends on a specific interface between
> kernel and (guest) userspace.

This doesn't really parse for me, but I think the gist of it is based on an incorrect assumption.

Can you elaborate?

Kind Regards,
-Greg

^ permalink raw reply

* Re: [PATCH 0/7] AlacrityVM guest drivers Reply-To:
From: Avi Kivity @ 2009-08-06 12:54 UTC (permalink / raw)
  To: Gregory Haskins
  Cc: Michael S. Tsirkin, alacrityvm-devel, kvm, linux-kernel, netdev
In-Reply-To: <4A7A8F7B0200005A00051B84@sinclair.provo.novell.com>

On 08/06/2009 03:08 PM, Gregory Haskins wrote:
>> Merging the guest first means relying on
>> kernel interface from an out of tree driver, which well might change
>> before it goes in.
>>      
>
> ABI compatibility is already addressed/handled, so even if that is true its not a problem.
>
>    

Really the correct way to address the ABI is to publish a spec and write 
both host and guest drivers to that.  Unfortunately we didn't do this 
with virtio.

It becomes more important when you have multiple implementations (e.g. 
Windows drivers).

>>> This series implements the guest-side drivers for accelerated IO
>>> when running on top of the AlacrityVM hypervisor, the details of
>>> which you can find here:
>>>
>>> http://developer.novell.com/wiki/index.php/AlacrityVM
>>>        
>> Since AlacrityVM is kvm based, Cc kvm@vger.kernel.org.
>>      
>
> I *can* do that, but there is nothing in these drivers that is KVM specific (its all pure PCI and VBUS).  I've already made the general announcement about the project/ml cross posted to KVM for anyone that might be interested, but I figure I will spare the general KVM list the details unless something specifically pertains to, or affects, KVM.  For instance, when I get to pushing the hypervisor side, I still need to work on getting that 'xinterface' patch to you guys.  I would certainly be CC'ing kvm@vger when that happens since it modifies the KVM code.
>
> So instead, I would just encourage anyone interested (such as yourself) to join the alacrity list so I don't bother the KVM community unless absolutely necessary.
>    

It's true that vbus is a separate project (in fact even virtio is 
completely separate from kvm).  Still I think it would be of interest to 
many kvm@ readers.


-- 
error compiling committee.c: too many arguments to function

^ permalink raw reply

* Re: [PATCH, RFC] bonding: prevent outgoing packets on inactive slaves
From: Eric Dumazet @ 2009-08-06 12:28 UTC (permalink / raw)
  To: Jiri Bohac; +Cc: fubar, davem, netdev
In-Reply-To: <20090806104356.GA8024@midget.suse.cz>

Jiri Bohac a écrit :
> On Wed, Aug 05, 2009 at 06:57:37PM +0200, Eric Dumazet wrote:
>> Jiri Bohac a écrit :
>>> This patch makes sure the TX queues on inactive slaves are
>>> deactivated.
>>>
>> Wont this break ARP link monitoring ?
> 
> No, ARP monitoring sends the ARP requests from the active slave.
> 

Yes, this makes sense now you say it :)

Thanks

^ permalink raw reply

* [PATCH] appletalk: fix atalk_getname() leak
From: Eric Dumazet @ 2009-08-06 12:27 UTC (permalink / raw)
  To: David S. Miller; +Cc: Linux Netdev List

atalk_getname() can leak 8 bytes of kernel memory to user

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
diff --git a/net/appletalk/ddp.c b/net/appletalk/ddp.c
index 590b839..9ef6ff2 100644
--- a/net/appletalk/ddp.c
+++ b/net/appletalk/ddp.c
@@ -1237,6 +1237,7 @@ static int atalk_getname(struct socket *sock, struct sockaddr *uaddr,
 			return -ENOBUFS;
 
 	*uaddr_len = sizeof(struct sockaddr_at);
+	memset(&sat.sat_zero, 0, sizeof(sat.sat_zero));
 
 	if (peer) {
 		if (sk->sk_state != TCP_ESTABLISHED)

^ permalink raw reply related

* [PATCH 2/2] af_ieee802154: provide dummy get/setsockopt
From: Dmitry Eremin-Solenikov @ 2009-08-06 12:26 UTC (permalink / raw)
  To: netdev; +Cc: David S. Miller, Sergey Lapin, linux-zigbee-devel
In-Reply-To: <1249561586-26927-2-git-send-email-dbaryshkov@gmail.com>

Provide dummt get/setsockopt implementations to stop these
syscalls from oopsing on our sockets.

Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
---
 net/ieee802154/dgram.c |   14 ++++++++++++++
 net/ieee802154/raw.c   |   14 ++++++++++++++
 2 files changed, 28 insertions(+), 0 deletions(-)

diff --git a/net/ieee802154/dgram.c b/net/ieee802154/dgram.c
index 14d3984..ba8b214 100644
--- a/net/ieee802154/dgram.c
+++ b/net/ieee802154/dgram.c
@@ -377,6 +377,18 @@ int ieee802154_dgram_deliver(struct net_device *dev, struct sk_buff *skb)
 	return ret;
 }
 
+static int dgram_getsockopt(struct sock *sk, int level, int optname,
+		    char __user *optval, int __user *optlen)
+{
+	return -EOPNOTSUPP;
+}
+
+static int dgram_setsockopt(struct sock *sk, int level, int optname,
+		    char __user *optval, int __user optlen)
+{
+	return -EOPNOTSUPP;
+}
+
 struct proto ieee802154_dgram_prot = {
 	.name		= "IEEE-802.15.4-MAC",
 	.owner		= THIS_MODULE,
@@ -391,5 +403,7 @@ struct proto ieee802154_dgram_prot = {
 	.connect	= dgram_connect,
 	.disconnect	= dgram_disconnect,
 	.ioctl		= dgram_ioctl,
+	.getsockopt	= dgram_getsockopt,
+	.setsockopt	= dgram_setsockopt,
 };
 
diff --git a/net/ieee802154/raw.c b/net/ieee802154/raw.c
index fca44d5..9315977 100644
--- a/net/ieee802154/raw.c
+++ b/net/ieee802154/raw.c
@@ -238,6 +238,18 @@ void ieee802154_raw_deliver(struct net_device *dev, struct sk_buff *skb)
 	read_unlock(&raw_lock);
 }
 
+static int raw_getsockopt(struct sock *sk, int level, int optname,
+		    char __user *optval, int __user *optlen)
+{
+	return -EOPNOTSUPP;
+}
+
+static int raw_setsockopt(struct sock *sk, int level, int optname,
+		    char __user *optval, int __user optlen)
+{
+	return -EOPNOTSUPP;
+}
+
 struct proto ieee802154_raw_prot = {
 	.name		= "IEEE-802.15.4-RAW",
 	.owner		= THIS_MODULE,
@@ -250,5 +262,7 @@ struct proto ieee802154_raw_prot = {
 	.unhash		= raw_unhash,
 	.connect	= raw_connect,
 	.disconnect	= raw_disconnect,
+	.getsockopt	= raw_getsockopt,
+	.setsockopt	= raw_setsockopt,
 };
 
-- 
1.6.3.3


^ permalink raw reply related

* [PATCH 1/2] af_ieee802154: fix ioctl processing
From: Dmitry Eremin-Solenikov @ 2009-08-06 12:26 UTC (permalink / raw)
  To: netdev; +Cc: David S. Miller, Sergey Lapin, linux-zigbee-devel
In-Reply-To: <1249561586-26927-1-git-send-email-dbaryshkov@gmail.com>

fix two errors in ioctl processing:
1) if the ioctl isn't supported one should return -ENOIOCTLCMD
2) don't call ndo_do_ioctl if the device doesn't provide it

Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
---
 net/ieee802154/af_ieee802154.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/net/ieee802154/af_ieee802154.c b/net/ieee802154/af_ieee802154.c
index 3bb6bdb..af66180 100644
--- a/net/ieee802154/af_ieee802154.c
+++ b/net/ieee802154/af_ieee802154.c
@@ -136,7 +136,7 @@ static int ieee802154_dev_ioctl(struct sock *sk, struct ifreq __user *arg,
 		unsigned int cmd)
 {
 	struct ifreq ifr;
-	int ret = -EINVAL;
+	int ret = -ENOIOCTLCMD;
 	struct net_device *dev;
 
 	if (copy_from_user(&ifr, arg, sizeof(struct ifreq)))
@@ -146,8 +146,10 @@ static int ieee802154_dev_ioctl(struct sock *sk, struct ifreq __user *arg,
 
 	dev_load(sock_net(sk), ifr.ifr_name);
 	dev = dev_get_by_name(sock_net(sk), ifr.ifr_name);
-	if (dev->type == ARPHRD_IEEE802154 ||
-	    dev->type == ARPHRD_IEEE802154_PHY)
+
+	if ((dev->type == ARPHRD_IEEE802154 ||
+	     dev->type == ARPHRD_IEEE802154_PHY) &&
+	    dev->netdev_ops->ndo_do_ioctl)
 		ret = dev->netdev_ops->ndo_do_ioctl(dev, &ifr, cmd);
 
 	if (!ret && copy_to_user(arg, &ifr, sizeof(struct ifreq)))
-- 
1.6.3.3


^ permalink raw reply related

* [GIT PULL 0/2] IEEE 802.15.4 last-minute fixes v3
From: Dmitry Eremin-Solenikov @ 2009-08-06 12:26 UTC (permalink / raw)
  To: netdev; +Cc: David S. Miller, Sergey Lapin, linux-zigbee-devel


I hope in this try I don't make any mistakes...

The following changes since commit 0bf52b981770cbf006323bab5177f2858a196766:
  Ingo Molnar (1):
        net: Fix spinlock use in alloc_netdev_mq()

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/lowpan/lowpan.git for-linus

Dmitry Eremin-Solenikov (2):
      af_ieee802154: fix ioctl processing
      af_ieee802154: provide dummy get/setsockopt

 net/ieee802154/af_ieee802154.c |    8 +++++---
 net/ieee802154/dgram.c         |   14 ++++++++++++++
 net/ieee802154/raw.c           |   14 ++++++++++++++
 3 files changed, 33 insertions(+), 3 deletions(-)

^ permalink raw reply

* Re: [PATCH 0/7] AlacrityVM guest drivers Reply-To:
From: Michael S. Tsirkin @ 2009-08-06 12:24 UTC (permalink / raw)
  To: Gregory Haskins; +Cc: alacrityvm-devel, kvm, linux-kernel, netdev
In-Reply-To: <4A7A8F7B0200005A00051B84@sinclair.provo.novell.com>

On Thu, Aug 06, 2009 at 06:08:27AM -0600, Gregory Haskins wrote:
> Hi Michael,
> 
> >>> On 8/6/2009 at  4:19 AM, in message <20090806081955.GA9752@redhat.com>,
> "Michael S. Tsirkin" <mst@redhat.com> wrote: 
> > On Mon, Aug 03, 2009 at 01:17:30PM -0400, Gregory Haskins wrote:
> >> (Applies to v2.6.31-rc5, proposed for linux-next after review is complete)
> > 
> > These are guest drivers, right?
> 
> Yep.
> 
> > Merging the guest first means relying on
> > kernel interface from an out of tree driver, which well might change
> > before it goes in.
> 
> ABI compatibility is already addressed/handled, so even if that is true its not a problem.

It is? With versioning? Presumably this:

+       params.devid   = vdev->id;
+       params.version = version;
+
+       ret = vbus_pci_hypercall(VBUS_PCI_HC_DEVOPEN,
+                                &params, sizeof(params));
+       if (ret < 0)
+               return ret;

Even assuming host even knows how to decode this structure (e.g.  some
other host module doesn't use VBUS_PCI_HC_DEVOPEN), checks the version
and denies older guests, this might help guest not to crash, but guest
still won't work.

> > Would it make more sense to start merging with the host side of the project?
> 
> Not necessarily, no.  These are drivers for a "device", so its no
> different than merging any other driver really.  This is especially
> true since the hypervisor is also already published and freely
> available today, so anyone can start using it.

The difference is clear to me: devices do not get to set kernel/userspace
interfaces. This "device" depends on a specific interface between
kernel and (guest) userspace.

-- 
MST

^ permalink raw reply

* Re: [PATCH 0/7] AlacrityVM guest drivers Reply-To:
From: Gregory Haskins @ 2009-08-06 12:09 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: alacrityvm-devel, kvm, linux-kernel, netdev
In-Reply-To: <20090806101702.GA10605@redhat.com>

>>> On 8/6/2009 at  6:17 AM, in message <20090806101702.GA10605@redhat.com>,
"Michael S. Tsirkin" <mst@redhat.com> wrote: 
> On Thu, Aug 06, 2009 at 11:19:56AM +0300, Michael S. Tsirkin wrote:
>> On Mon, Aug 03, 2009 at 01:17:30PM -0400, Gregory Haskins wrote:
>> > (Applies to v2.6.31-rc5, proposed for linux-next after review is complete)
>> 
>> These are guest drivers, right? Merging the guest first means relying on
>> kernel interface from an out of tree driver, which well might change
>> before it goes in.  Would it make more sense to start merging with the
>> host side of the project?
>> 
>> > This series implements the guest-side drivers for accelerated IO
>> > when running on top of the AlacrityVM hypervisor, the details of
>> > which you can find here:
>> > 
>> > http://developer.novell.com/wiki/index.php/AlacrityVM
>> 
>> Since AlacrityVM is kvm based, Cc kvm@vger.kernel.org.
>> 
>> > This series includes the basic plumbing, as well as the driver for
>> > accelerated 802.x (ethernet) networking.
>> 
>> The graphs comparing virtio with vbus look interesting.
>> However, they do not compare apples to apples, do they?
>> These compare userspace virtio with kernel vbus, where for
>> apples to apples comparison one would need to compare
>> kernel virtio with kernel vbus. Right?
> 
> Or userspace virtio with userspace vbus.

Note: That would be pointless.

-Greg



^ permalink raw reply

* Re: [PATCH 0/7] AlacrityVM guest drivers Reply-To:
From: Gregory Haskins @ 2009-08-06 12:08 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: alacrityvm-devel, kvm, linux-kernel, netdev
In-Reply-To: <20090806081955.GA9752@redhat.com>

Hi Michael,

>>> On 8/6/2009 at  4:19 AM, in message <20090806081955.GA9752@redhat.com>,
"Michael S. Tsirkin" <mst@redhat.com> wrote: 
> On Mon, Aug 03, 2009 at 01:17:30PM -0400, Gregory Haskins wrote:
>> (Applies to v2.6.31-rc5, proposed for linux-next after review is complete)
> 
> These are guest drivers, right?

Yep.

> Merging the guest first means relying on
> kernel interface from an out of tree driver, which well might change
> before it goes in.

ABI compatibility is already addressed/handled, so even if that is true its not a problem.

> Would it make more sense to start merging with the host side of the project?

Not necessarily, no.  These are drivers for a "device", so its no different than merging any other driver really.  This is especially true since the hypervisor is also already published and freely available today, so anyone can start using it.

> 
>> This series implements the guest-side drivers for accelerated IO
>> when running on top of the AlacrityVM hypervisor, the details of
>> which you can find here:
>> 
>> http://developer.novell.com/wiki/index.php/AlacrityVM
> 
> Since AlacrityVM is kvm based, Cc kvm@vger.kernel.org.

I *can* do that, but there is nothing in these drivers that is KVM specific (its all pure PCI and VBUS).  I've already made the general announcement about the project/ml cross posted to KVM for anyone that might be interested, but I figure I will spare the general KVM list the details unless something specifically pertains to, or affects, KVM.  For instance, when I get to pushing the hypervisor side, I still need to work on getting that 'xinterface' patch to you guys.  I would certainly be CC'ing kvm@vger when that happens since it modifies the KVM code.

So instead, I would just encourage anyone interested (such as yourself) to join the alacrity list so I don't bother the KVM community unless absolutely necessary.

> 
>> This series includes the basic plumbing, as well as the driver for
>> accelerated 802.x (ethernet) networking.
> 
> The graphs comparing virtio with vbus look interesting.
> However, they do not compare apples to apples, do they?

Yes, I believe they do.  They represent the best that KVM has to offer (to my knowledge) vs the best that alacrityvm has to offer.

> These compare userspace virtio with kernel vbus,

vbus is a device model (akin to QEMU's device model).  Technically, it was a comparison of userspace virtio-net (via QEMU), to kernel venet (via vbus),
which I again stress is the state of the art for both to my knowledge.

As I have explained before in earlier threads on kvm@vger, virtio is not mutually exclusive here.  You can run the virtio protocol over the vbus model if someone were so inclined.  In fact, I proposed this very idea to you a month or two ago but I believe you decided to go your own way and reinvent some other in-kernel model instead for your own reasons.

>where for apples to apples comparison one would need to compare
> kernel virtio with kernel vbus. Right?

Again, it already *is* apples to apples as far as I am concerned.  

At the time I ran those numbers, there was certainly no in-kernel virtio model to play with.  And to my knowledge, there isn't one now (I was never CC'd on the patches, and a cursory search of the KVM list isn't revealing one that was posted recently).

To reiterate: kernel virtio-net (using ??) to kernel venet (vbus based) to kernel virtio-net (vbus, but doesnt exist yet) would be a fun bakeoff.  If you have something for the kernel virtio-net, point me at it and I will try to include it in the comparison next time.

Kind Regards,
-Greg

^ permalink raw reply

* Re: [PATCH 5/5] net: smsc911x: switch to new dev_pm_ops
From: Steve.Glendinning @ 2009-08-06 11:53 UTC (permalink / raw)
  To: David Miller
  Cc: daniel, linux-kernel, linux-pm, netdev, linux-sh, Ian.Saturley
In-Reply-To: <20090805.202922.241102337.davem@davemloft.net>

David Miller <davem@davemloft.net> wrote on 06/08/2009 04:29:22:

> From: Daniel Mack <daniel@caiaq.de>
> Date: Wed,  5 Aug 2009 20:29:31 +0200
> 
> > Hibernation is unsupported for now, which meets the actual
> > implementation in the driver. For free/thaw, the chip's D2 state 
should
> > be entered.
> > 
> > Signed-off-by: Daniel Mack <daniel@caiaq.de>
> 
> Steve, if it looks good to you I can toss this into net-next-2.6
> 
> Just let me know.

Looks fine.  Compiles ok and doesn't actually change any PM 
implementation.

Acked-by: <steve.glendinning@smsc.com>

Unfortunately I can't test it.  The only dev platform I have in front of 
me
is SH7709S, which doesn't boot since 
4ff29ff8e8723a41e7defd8bc78a7b16cbf940a2
(the sh maintainers are aware and working on it:
http://marc.info/?l=linux-sh&m=124891631203530&w=2)

Steve

^ permalink raw reply

* [PATCH ver2] Avoid enqueuing skb for default qdiscs
From: Krishna Kumar @ 2009-08-06 11:44 UTC (permalink / raw)
  To: davem; +Cc: Jarek Poplawski, netdev, herbert, Krishna Kumar, kaber

From: Krishna Kumar <krkumar2@in.ibm.com>

dev_queue_xmit enqueue's a skb and calls qdisc_run which
dequeue's the skb and xmits it. In most cases, the skb that
is enqueue'd is the same one that is dequeue'd (unless the
queue gets stopped or multiple cpu's write to the same queue
and ends in a race with qdisc_run). For default qdiscs, we
can remove the redundant enqueue/dequeue and simply xmit the
skb since the default qdisc is work-conserving.

The patch uses a new flag - TCQ_F_CAN_BYPASS to identify the
default fast queue. The controversial part of the patch is
incrementing qlen when a skb is requeued - this is to avoid
checks like the second line below:

+  } else if ((q->flags & TCQ_F_CAN_BYPASS) && !qdisc_qlen(q) &&
>>         !q->gso_skb &&
+          !test_and_set_bit(__QDISC_STATE_RUNNING, &q->state)) {

Results of a 2 hour testing for multiple netperf sessions (1,
2, 4, 8, 12 sessions on a 4 cpu system-X). The BW numbers are
aggregate Mb/s across iterations tested with this version on
System-X boxes with Chelsio 10gbps cards:

----------------------------------
Size |  ORG BW          NEW BW   |
----------------------------------
128K |  156964          159381   |
256K |  158650          162042   |
----------------------------------

Changes from ver1:

1. Move sch_direct_xmit declaration from sch_generic.h to
   pkt_sched.h
2. Update qdisc basic statistics for direct xmit path.
3. Set qlen to zero in qdisc_reset.
4. Changed some function names to more meaningful ones.

Thanks,

- KK

Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>
---
 include/net/pkt_sched.h   |    3 +
 include/net/sch_generic.h |   15 +++++
 net/core/dev.c            |   48 +++++++++++++-----
 net/sched/sch_generic.c   |   93 ++++++++++++++++++++++--------------
 4 files changed, 108 insertions(+), 51 deletions(-)

diff -ruNp org2/include/net/pkt_sched.h new6/include/net/pkt_sched.h
--- org2/include/net/pkt_sched.h	2009-08-04 11:41:21.000000000 +0530
+++ new6/include/net/pkt_sched.h	2009-08-05 22:24:41.000000000 +0530
@@ -87,6 +87,9 @@ extern struct qdisc_rate_table *qdisc_ge
 extern void qdisc_put_rtab(struct qdisc_rate_table *tab);
 extern void qdisc_put_stab(struct qdisc_size_table *tab);
 extern void qdisc_warn_nonwc(char *txt, struct Qdisc *qdisc);
+extern int sch_direct_xmit(struct sk_buff *skb, struct Qdisc *q,
+			   struct net_device *dev, struct netdev_queue *txq,
+			   spinlock_t *root_lock);
 
 extern void __qdisc_run(struct Qdisc *q);
 
diff -ruNp org2/include/net/sch_generic.h new6/include/net/sch_generic.h
--- org2/include/net/sch_generic.h	2009-03-24 08:54:16.000000000 +0530
+++ new6/include/net/sch_generic.h	2009-08-05 22:26:40.000000000 +0530
@@ -45,6 +45,7 @@ struct Qdisc
 #define TCQ_F_BUILTIN		1
 #define TCQ_F_THROTTLED		2
 #define TCQ_F_INGRESS		4
+#define TCQ_F_CAN_BYPASS	8
 #define TCQ_F_WARN_NONWC	(1 << 16)
 	int			padded;
 	struct Qdisc_ops	*ops;
@@ -182,6 +183,11 @@ struct qdisc_skb_cb {
 	char			data[];
 };
 
+static inline int qdisc_qlen(struct Qdisc *q)
+{
+	return q->q.qlen;
+}
+
 static inline struct qdisc_skb_cb *qdisc_skb_cb(struct sk_buff *skb)
 {
 	return (struct qdisc_skb_cb *)skb->cb;
@@ -387,13 +393,18 @@ static inline int qdisc_enqueue_root(str
 	return qdisc_enqueue(skb, sch) & NET_XMIT_MASK;
 }
 
+static inline void __qdisc_update_bstats(struct Qdisc *sch, unsigned int len)
+{
+	sch->bstats.bytes += len;
+	sch->bstats.packets++;
+}
+
 static inline int __qdisc_enqueue_tail(struct sk_buff *skb, struct Qdisc *sch,
 				       struct sk_buff_head *list)
 {
 	__skb_queue_tail(list, skb);
 	sch->qstats.backlog += qdisc_pkt_len(skb);
-	sch->bstats.bytes += qdisc_pkt_len(skb);
-	sch->bstats.packets++;
+	__qdisc_update_bstats(sch, qdisc_pkt_len(skb));
 
 	return NET_XMIT_SUCCESS;
 }
diff -ruNp org2/net/core/dev.c new6/net/core/dev.c
--- org2/net/core/dev.c	2009-07-27 09:08:24.000000000 +0530
+++ new6/net/core/dev.c	2009-08-06 11:34:21.000000000 +0530
@@ -1786,6 +1786,40 @@ static struct netdev_queue *dev_pick_tx(
 	return netdev_get_tx_queue(dev, queue_index);
 }
 
+static inline int __dev_xmit_skb(struct sk_buff *skb, struct Qdisc *q,
+				 struct net_device *dev,
+				 struct netdev_queue *txq)
+{
+	spinlock_t *root_lock = qdisc_lock(q);
+	int rc;
+
+	spin_lock(root_lock);
+	if (unlikely(test_bit(__QDISC_STATE_DEACTIVATED, &q->state))) {
+		kfree_skb(skb);
+		rc = NET_XMIT_DROP;
+	} else if ((q->flags & TCQ_F_CAN_BYPASS) && !qdisc_qlen(q) &&
+		   !test_and_set_bit(__QDISC_STATE_RUNNING, &q->state)) {
+		/*
+		 * This is a work-conserving queue; there are no old skbs
+		 * waiting to be sent out; and the qdisc is not running -
+		 * xmit the skb directly.
+		 */
+		__qdisc_update_bstats(q, skb->len);
+		if (sch_direct_xmit(skb, q, dev, txq, root_lock))
+			__qdisc_run(q);
+		else
+			clear_bit(__QDISC_STATE_RUNNING, &q->state);
+
+		rc = NET_XMIT_SUCCESS;
+	} else {
+		rc = qdisc_enqueue_root(skb, q);
+		qdisc_run(q);
+	}
+	spin_unlock(root_lock);
+
+	return rc;
+}
+
 /**
  *	dev_queue_xmit - transmit a buffer
  *	@skb: buffer to transmit
@@ -1859,19 +1893,7 @@ gso:
 	skb->tc_verd = SET_TC_AT(skb->tc_verd,AT_EGRESS);
 #endif
 	if (q->enqueue) {
-		spinlock_t *root_lock = qdisc_lock(q);
-
-		spin_lock(root_lock);
-
-		if (unlikely(test_bit(__QDISC_STATE_DEACTIVATED, &q->state))) {
-			kfree_skb(skb);
-			rc = NET_XMIT_DROP;
-		} else {
-			rc = qdisc_enqueue_root(skb, q);
-			qdisc_run(q);
-		}
-		spin_unlock(root_lock);
-
+		rc = __dev_xmit_skb(skb, q, dev, txq);
 		goto out;
 	}
 
diff -ruNp org2/net/sched/sch_generic.c new6/net/sched/sch_generic.c
--- org2/net/sched/sch_generic.c	2009-05-25 07:48:07.000000000 +0530
+++ new6/net/sched/sch_generic.c	2009-08-05 22:24:41.000000000 +0530
@@ -37,15 +37,11 @@
  * - updates to tree and tree walking are only done under the rtnl mutex.
  */
 
-static inline int qdisc_qlen(struct Qdisc *q)
-{
-	return q->q.qlen;
-}
-
 static inline int dev_requeue_skb(struct sk_buff *skb, struct Qdisc *q)
 {
 	q->gso_skb = skb;
 	q->qstats.requeues++;
+	q->q.qlen++;	/* it's still part of the queue */
 	__netif_schedule(q);
 
 	return 0;
@@ -61,9 +57,11 @@ static inline struct sk_buff *dequeue_sk
 
 		/* check the reason of requeuing without tx lock first */
 		txq = netdev_get_tx_queue(dev, skb_get_queue_mapping(skb));
-		if (!netif_tx_queue_stopped(txq) && !netif_tx_queue_frozen(txq))
+		if (!netif_tx_queue_stopped(txq) &&
+		    !netif_tx_queue_frozen(txq)) {
 			q->gso_skb = NULL;
-		else
+			q->q.qlen--;
+		} else
 			skb = NULL;
 	} else {
 		skb = q->dequeue(q);
@@ -103,44 +101,23 @@ static inline int handle_dev_cpu_collisi
 }
 
 /*
- * NOTE: Called under qdisc_lock(q) with locally disabled BH.
- *
- * __QDISC_STATE_RUNNING guarantees only one CPU can process
- * this qdisc at a time. qdisc_lock(q) serializes queue accesses for
- * this queue.
- *
- *  netif_tx_lock serializes accesses to device driver.
- *
- *  qdisc_lock(q) and netif_tx_lock are mutually exclusive,
- *  if one is grabbed, another must be free.
- *
- * Note, that this procedure can be called by a watchdog timer
+ * Transmit one skb, and handle the return status as required. Holding the
+ * __QDISC_STATE_RUNNING bit guarantees that only one CPU can execute this
+ * function.
  *
  * Returns to the caller:
  *				0  - queue is empty or throttled.
  *				>0 - queue is not empty.
- *
  */
-static inline int qdisc_restart(struct Qdisc *q)
+int sch_direct_xmit(struct sk_buff *skb, struct Qdisc *q,
+		    struct net_device *dev, struct netdev_queue *txq,
+		    spinlock_t *root_lock)
 {
-	struct netdev_queue *txq;
 	int ret = NETDEV_TX_BUSY;
-	struct net_device *dev;
-	spinlock_t *root_lock;
-	struct sk_buff *skb;
-
-	/* Dequeue packet */
-	if (unlikely((skb = dequeue_skb(q)) == NULL))
-		return 0;
-
-	root_lock = qdisc_lock(q);
 
 	/* And release qdisc */
 	spin_unlock(root_lock);
 
-	dev = qdisc_dev(q);
-	txq = netdev_get_tx_queue(dev, skb_get_queue_mapping(skb));
-
 	HARD_TX_LOCK(dev, txq, smp_processor_id());
 	if (!netif_tx_queue_stopped(txq) &&
 	    !netif_tx_queue_frozen(txq))
@@ -177,6 +154,44 @@ static inline int qdisc_restart(struct Q
 	return ret;
 }
 
+/*
+ * NOTE: Called under qdisc_lock(q) with locally disabled BH.
+ *
+ * __QDISC_STATE_RUNNING guarantees only one CPU can process
+ * this qdisc at a time. qdisc_lock(q) serializes queue accesses for
+ * this queue.
+ *
+ *  netif_tx_lock serializes accesses to device driver.
+ *
+ *  qdisc_lock(q) and netif_tx_lock are mutually exclusive,
+ *  if one is grabbed, another must be free.
+ *
+ * Note, that this procedure can be called by a watchdog timer
+ *
+ * Returns to the caller:
+ *				0  - queue is empty or throttled.
+ *				>0 - queue is not empty.
+ *
+ */
+static inline int qdisc_restart(struct Qdisc *q)
+{
+	struct netdev_queue *txq;
+	struct net_device *dev;
+	spinlock_t *root_lock;
+	struct sk_buff *skb;
+
+	/* Dequeue packet */
+	skb = dequeue_skb(q);
+	if (unlikely(!skb))
+		return 0;
+
+	root_lock = qdisc_lock(q);
+	dev = qdisc_dev(q);
+	txq = netdev_get_tx_queue(dev, skb_get_queue_mapping(skb));
+
+	return sch_direct_xmit(skb, q, dev, txq, root_lock);
+}
+
 void __qdisc_run(struct Qdisc *q)
 {
 	unsigned long start_time = jiffies;
@@ -547,8 +562,11 @@ void qdisc_reset(struct Qdisc *qdisc)
 	if (ops->reset)
 		ops->reset(qdisc);
 
-	kfree_skb(qdisc->gso_skb);
-	qdisc->gso_skb = NULL;
+	if (qdisc->gso_skb) {
+		kfree_skb(qdisc->gso_skb);
+		qdisc->gso_skb = NULL;
+		qdisc->q.qlen = 0;
+	}
 }
 EXPORT_SYMBOL(qdisc_reset);
 
@@ -605,6 +623,9 @@ static void attach_one_default_qdisc(str
 			printk(KERN_INFO "%s: activation failed\n", dev->name);
 			return;
 		}
+
+		/* Can by-pass the queue discipline for default qdisc */
+		qdisc->flags |= TCQ_F_CAN_BYPASS;
 	} else {
 		qdisc =  &noqueue_qdisc;
 	}

^ permalink raw reply

* [PATCH] sky2: Serialize access to PCI config space
From: Mike McCormack @ 2009-08-06 11:10 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev, Rene Mayrhofer, Richard Leitner

PCI config space may be shared across two device instances,
 or accessed from an interrupt
 , so serialize access so
 read-update-write operations do not interfere with each other.

Only tested on a single port card, as my sky2 doesn't have dual ports.

Signed-off-by: Mike McCormack <mikem@ring3k.org>
---
 drivers/net/sky2.c |   23 ++++++++++++++++-------
 drivers/net/sky2.h |    1 +
 2 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/drivers/net/sky2.c b/drivers/net/sky2.c
index 1415a83..4d09632 100644
--- a/drivers/net/sky2.c
+++ b/drivers/net/sky2.c
@@ -618,6 +618,7 @@ static void sky2_phy_power_up(struct sky2_hw *hw, unsigned port)
 {
 	u32 reg1;
 
+	spin_lock(&hw->pci_cfg_lock);
 	sky2_write8(hw, B2_TST_CTRL1, TST_CFG_WRITE_ON);
 	reg1 = sky2_pci_read32(hw, PCI_DEV_REG1);
 	reg1 &= ~phy_power[port];
@@ -628,6 +629,7 @@ static void sky2_phy_power_up(struct sky2_hw *hw, unsigned port)
 	sky2_pci_write32(hw, PCI_DEV_REG1, reg1);
 	sky2_write8(hw, B2_TST_CTRL1, TST_CFG_WRITE_OFF);
 	sky2_pci_read32(hw, PCI_DEV_REG1);
+	spin_unlock(&hw->pci_cfg_lock);
 
 	if (hw->chip_id == CHIP_ID_YUKON_FE)
 		gm_phy_write(hw, port, PHY_MARV_CTRL, PHY_CT_ANE);
@@ -681,11 +683,13 @@ static void sky2_phy_power_down(struct sky2_hw *hw, unsigned port)
 		gm_phy_write(hw, port, PHY_MARV_CTRL, PHY_CT_PDOWN);
 	}
 
+	spin_lock(&hw->pci_cfg_lock);
 	sky2_write8(hw, B2_TST_CTRL1, TST_CFG_WRITE_ON);
 	reg1 = sky2_pci_read32(hw, PCI_DEV_REG1);
 	reg1 |= phy_power[port];		/* set PHY to PowerDown/COMA Mode */
 	sky2_pci_write32(hw, PCI_DEV_REG1, reg1);
 	sky2_write8(hw, B2_TST_CTRL1, TST_CFG_WRITE_OFF);
+	spin_unlock(&hw->pci_cfg_lock);
 }
 
 /* Force a renegotiation */
@@ -2596,30 +2600,34 @@ static void sky2_hw_intr(struct sky2_hw *hw)
 	if (status & (Y2_IS_MST_ERR | Y2_IS_IRQ_STAT)) {
 		u16 pci_err;
 
+		spin_lock(&hw->pci_cfg_lock);
 		sky2_write8(hw, B2_TST_CTRL1, TST_CFG_WRITE_ON);
 		pci_err = sky2_pci_read16(hw, PCI_STATUS);
-		if (net_ratelimit())
-			dev_err(&pdev->dev, "PCI hardware error (0x%x)\n",
-			        pci_err);
-
 		sky2_pci_write16(hw, PCI_STATUS,
 				      pci_err | PCI_STATUS_ERROR_BITS);
 		sky2_write8(hw, B2_TST_CTRL1, TST_CFG_WRITE_OFF);
+		spin_unlock(&hw->pci_cfg_lock);
+
+		if (net_ratelimit())
+			dev_err(&pdev->dev, "PCI hardware error (0x%x)\n",
+			        pci_err);
 	}
 
 	if (status & Y2_IS_PCI_EXP) {
 		/* PCI-Express uncorrectable Error occurred */
 		u32 err;
 
+		spin_lock(&hw->pci_cfg_lock);
 		sky2_write8(hw, B2_TST_CTRL1, TST_CFG_WRITE_ON);
 		err = sky2_read32(hw, Y2_CFG_AER + PCI_ERR_UNCOR_STATUS);
 		sky2_write32(hw, Y2_CFG_AER + PCI_ERR_UNCOR_STATUS,
 			     0xfffffffful);
-		if (net_ratelimit())
-			dev_err(&pdev->dev, "PCI Express error (0x%x)\n", err);
-
 		sky2_read32(hw, Y2_CFG_AER + PCI_ERR_UNCOR_STATUS);
 		sky2_write8(hw, B2_TST_CTRL1, TST_CFG_WRITE_OFF);
+		spin_unlock(&hw->pci_cfg_lock);
+
+		if (net_ratelimit())
+			dev_err(&pdev->dev, "PCI Express error (0x%x)\n", err);
 	}
 
 	if (status & Y2_HWE_L1_MASK)
@@ -4485,6 +4493,7 @@ static int __devinit sky2_probe(struct pci_dev *pdev,
 	}
 
 	hw->pdev = pdev;
+	spin_lock_init(&hw->pci_cfg_lock);
 
 	hw->regs = ioremap_nocache(pci_resource_start(pdev, 0), 0x4000);
 	if (!hw->regs) {
diff --git a/drivers/net/sky2.h b/drivers/net/sky2.h
index 4486b06..5a9b0cf 100644
--- a/drivers/net/sky2.h
+++ b/drivers/net/sky2.h
@@ -2087,6 +2087,7 @@ struct sky2_hw {
 	struct timer_list    watchdog_timer;
 	struct work_struct   restart_work;
 	wait_queue_head_t    msi_wait;
+	spinlock_t	     pci_cfg_lock;
 };
 
 static inline int sky2_is_copper(const struct sky2_hw *hw)
-- 
1.5.6.5


^ permalink raw reply related


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