* Re: [PATCHv2 0/2] vhost: a kernel-level virtio server
From: Gregory Haskins @ 2009-08-12 16:13 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Arnd Bergmann, netdev, virtualization, kvm, linux-kernel, mingo,
linux-mm, Andrew Morton, hpa, Patrick Mullaney
In-Reply-To: <20090812140224.GA29345@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 1805 bytes --]
Michael S. Tsirkin wrote:
> On Wed, Aug 12, 2009 at 09:51:45AM -0400, Gregory Haskins wrote:
>> Arnd Bergmann wrote:
>>> On Wednesday 12 August 2009, Michael S. Tsirkin wrote:
>>>>> If I understand it correctly, you can at least connect a veth pair
>>>>> to a bridge, right? Something like
>>>>>
>>>>> veth0 - veth1 - vhost - guest 1
>>>>> eth0 - br0-|
>>>>> veth2 - veth3 - vhost - guest 2
>>>>>
>>>> Heh, you don't need a bridge in this picture:
>>>>
>>>> guest 1 - vhost - veth0 - veth1 - vhost guest 2
>>> Sure, but the setup I described is the one that I would expect
>>> to see in practice because it gives you external connectivity.
>>>
>>> Measuring two guests communicating over a veth pair is
>>> interesting for finding the bottlenecks, but of little
>>> practical relevance.
>>>
>>> Arnd <><
>> Yeah, this would be the config I would be interested in.
>
> Hmm, this wouldn't be the config to use for the benchmark though: there
> are just too many variables. If you want both guest to guest and guest
> to host, create 2 nics in the guest.
>
> Here's one way to do this:
>
> -net nic,model=virtio,vlan=0 -net user,vlan=0
> -net nic,vlan=1,model=virtio,vhost=veth0
> -redir tcp:8022::22
>
> -net nic,model=virtio,vlan=0 -net user,vlan=0
> -net nic,vlan=1,model=virtio,vhost=veth1
> -redir tcp:8023::22
>
> In guests, for simplicity, configure eth1 and eth0
> to use separate subnets.
I can try to do a few variations, but what I am interested is in
performance in a real-world L2 configuration. This would generally mean
all hosts (virtual or physical) in the same L2 domain.
If I get a chance, though, I will try to also wire them up in isolation
as another data point.
Regards,
-Greg
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 267 bytes --]
^ permalink raw reply
* Re: [PATCHv2 2/2] vhost_net: a kernel-level virtio server
From: Paul E. McKenney @ 2009-08-12 16:06 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Gregory Haskins, netdev, virtualization, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org, Ingo Molnar, linux-mm,
Andrew Morton, hpa
In-Reply-To: <20090812155154.GA29797@redhat.com>
On Wed, Aug 12, 2009 at 06:51:54PM +0300, Michael S. Tsirkin wrote:
> On Wed, Aug 12, 2009 at 08:26:39AM -0700, Paul E. McKenney wrote:
> > On Wed, Aug 12, 2009 at 05:15:59PM +0300, Michael S. Tsirkin wrote:
> > > On Wed, Aug 12, 2009 at 07:11:07AM -0700, Paul E. McKenney wrote:
> > > > On Wed, Aug 12, 2009 at 04:25:40PM +0300, Michael S. Tsirkin wrote:
> > > > > On Wed, Aug 12, 2009 at 09:01:35AM -0400, Gregory Haskins wrote:
> > > > > > I think I understand what your comment above meant: You don't need to
> > > > > > do synchronize_rcu() because you can flush the workqueue instead to
> > > > > > ensure that all readers have completed.
> > > > >
> > > > > Yes.
> > > > >
> > > > > > But if thats true, to me, the
> > > > > > rcu_dereference itself is gratuitous,
> > > > >
> > > > > Here's a thesis on what rcu_dereference does (besides documentation):
> > > > >
> > > > > reader does this
> > > > >
> > > > > A: sock = n->sock
> > > > > B: use *sock
> > > > >
> > > > > Say writer does this:
> > > > >
> > > > > C: newsock = allocate socket
> > > > > D: initialize(newsock)
> > > > > E: n->sock = newsock
> > > > > F: flush
> > > > >
> > > > >
> > > > > On Alpha, reads could be reordered. So, on smp, command A could get
> > > > > data from point F, and command B - from point D (uninitialized, from
> > > > > cache). IOW, you get fresh pointer but stale data.
> > > > > So we need to stick a barrier in there.
> > > > >
> > > > > > and that pointer is *not* actually
> > > > > > RCU protected (nor does it need to be).
> > > > >
> > > > > Heh, if readers are lockless and writer does init/update/sync,
> > > > > this to me spells rcu.
> > > >
> > > > If you are using call_rcu(), synchronize_rcu(), or one of the
> > > > similar primitives, then you absolutely need rcu_read_lock() and
> > > > rcu_read_unlock(), or one of the similar pairs of primitives.
> > >
> > > Right. I don't use any of these though.
> > >
> > > > If you -don't- use rcu_read_lock(), then you are pretty much restricted
> > > > to adding data, but never removing it.
> > > >
> > > > Make sense? ;-)
> > >
> > > Since I only access data from a workqueue, I replaced synchronize_rcu
> > > with workqueue flush. That's why I don't need rcu_read_lock.
> >
> > Well, you -do- need -something- that takes on the role of rcu_read_lock(),
> > and in your case you in fact actually do. Your equivalent of
> > rcu_read_lock() is the beginning of execution of a workqueue item, and
> > the equivalent of rcu_read_unlock() is the end of execution of that same
> > workqueue item. Implicit, but no less real.
>
> Well put. I'll add this to comments in my code.
Very good, thank you!!!
> > If a couple more uses like this show up, I might need to add this to
> > Documentation/RCU. ;-)
And I idly wonder if this approach could replace SRCU. Probably not
for protecting the CPU-hotplug notifier chains, but worth some thought.
Thanx, Paul
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: Receive side performance issue with multi-10-GigE and NUMA
From: Jesse Barnes @ 2009-08-12 16:00 UTC (permalink / raw)
To: Bill Fink
Cc: Brandeburg, Jesse, Neil Horman, Andrew Gallatin, Brice Goglin,
Linux Network Developers, Yinghai Lu
In-Reply-To: <20090812003824.26c9c8fb.billfink@mindspring.com>
On Wed, 12 Aug 2009 00:38:24 -0400
Bill Fink <billfink@mindspring.com> wrote:
> On Tue, 11 Aug 2009, Brandeburg, Jesse wrote:
>
> > Bill Fink wrote:
> > > On Sat, 8 Aug 2009, Neil Horman wrote:
> > >
> > >> On Sat, Aug 08, 2009 at 02:21:36PM -0400, Andrew Gallatin wrote:
> > >>> Neil Horman wrote:
> > >>>> On Sat, Aug 08, 2009 at 07:08:20AM -0400, Andrew Gallatin
> > >>>> wrote:
> > >>>>> Bill Fink wrote:
> > >>>>>> On Fri, 07 Aug 2009, Andrew Gallatin wrote:
> > >>>>>>
> > >>>>>>> Bill Fink wrote:
> > >>>>>>>
> > >>>>>>>> All sysfs local_cpus values are the same
> > >>>>>>>> (00000000,000000ff), so yes they are also wrong.
> >
> > bill, I recently helped Jesse Barnes push a patch that addresses
> > this kind of issue on CoreI7, the root cause was the numa_node
> > variable was initialized based on slot on AMD systems, but needed
> > to be set to -1 by default on systems with a uniform IOH to slot
> > architecture.
> >
> > here is the commit ID:
> > http://git.kernel.org/?p=linux/kernel/git/sfr/linux-next.git;a=commit;h=3c38
> > d674be519109696746192943a6d524019f7f
> >
> > I'm not sure it is in linus' tree yet, this link is to net-next
> >
> > Maybe see if it helps?
>
> It's worth a shot.
>
> Hopefully I can get a chance to build a new kernel tomorrow to check
> out some of the suggestions, like this one, the setting of ACPI_DEBUG,
> and the new ftrace module for checking NUMA affinity of skbs.
It's a fairly significant change so I wasn't planning on sending it to
Linus for 2.6.31. If you think it *should* go into 2.6.31 (and stable
for that matter), please let me know soon.
Thanks,
--
Jesse Barnes, Intel Open Source Technology Center
^ permalink raw reply
* Re: [PATCHv2 2/2] vhost_net: a kernel-level virtio server
From: Michael S. Tsirkin @ 2009-08-12 15:51 UTC (permalink / raw)
To: Paul E. McKenney
Cc: Gregory Haskins, netdev, virtualization, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org, Ingo Molnar, linux-mm,
Andrew Morton, hpa
In-Reply-To: <20090812152639.GA6779@linux.vnet.ibm.com>
On Wed, Aug 12, 2009 at 08:26:39AM -0700, Paul E. McKenney wrote:
> On Wed, Aug 12, 2009 at 05:15:59PM +0300, Michael S. Tsirkin wrote:
> > On Wed, Aug 12, 2009 at 07:11:07AM -0700, Paul E. McKenney wrote:
> > > On Wed, Aug 12, 2009 at 04:25:40PM +0300, Michael S. Tsirkin wrote:
> > > > On Wed, Aug 12, 2009 at 09:01:35AM -0400, Gregory Haskins wrote:
> > > > > I think I understand what your comment above meant: You don't need to
> > > > > do synchronize_rcu() because you can flush the workqueue instead to
> > > > > ensure that all readers have completed.
> > > >
> > > > Yes.
> > > >
> > > > > But if thats true, to me, the
> > > > > rcu_dereference itself is gratuitous,
> > > >
> > > > Here's a thesis on what rcu_dereference does (besides documentation):
> > > >
> > > > reader does this
> > > >
> > > > A: sock = n->sock
> > > > B: use *sock
> > > >
> > > > Say writer does this:
> > > >
> > > > C: newsock = allocate socket
> > > > D: initialize(newsock)
> > > > E: n->sock = newsock
> > > > F: flush
> > > >
> > > >
> > > > On Alpha, reads could be reordered. So, on smp, command A could get
> > > > data from point F, and command B - from point D (uninitialized, from
> > > > cache). IOW, you get fresh pointer but stale data.
> > > > So we need to stick a barrier in there.
> > > >
> > > > > and that pointer is *not* actually
> > > > > RCU protected (nor does it need to be).
> > > >
> > > > Heh, if readers are lockless and writer does init/update/sync,
> > > > this to me spells rcu.
> > >
> > > If you are using call_rcu(), synchronize_rcu(), or one of the
> > > similar primitives, then you absolutely need rcu_read_lock() and
> > > rcu_read_unlock(), or one of the similar pairs of primitives.
> >
> > Right. I don't use any of these though.
> >
> > > If you -don't- use rcu_read_lock(), then you are pretty much restricted
> > > to adding data, but never removing it.
> > >
> > > Make sense? ;-)
> >
> > Since I only access data from a workqueue, I replaced synchronize_rcu
> > with workqueue flush. That's why I don't need rcu_read_lock.
>
> Well, you -do- need -something- that takes on the role of rcu_read_lock(),
> and in your case you in fact actually do. Your equivalent of
> rcu_read_lock() is the beginning of execution of a workqueue item, and
> the equivalent of rcu_read_unlock() is the end of execution of that same
> workqueue item. Implicit, but no less real.
Well put. I'll add this to comments in my code.
> If a couple more uses like this show up, I might need to add this to
> Documentation/RCU. ;-)
>
> Thanx, Paul
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* [PATCH] ipv6: Log the explicit address that triggered DAD failure
From: Jens Rosenboom @ 2009-08-12 14:58 UTC (permalink / raw)
To: Linux Network Developers; +Cc: David Miller
If an interface has multiple addresses, the current message for DAD
failure isn't really helpful, so this patch adds the address itself to
the printk.
Signed-off-by: Jens Rosenboom <jens@mcbone.net>
---
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 43b3c9f..01a4b25 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -1403,8 +1403,8 @@ void addrconf_dad_failure(struct inet6_ifaddr
*ifp)
struct inet6_dev *idev = ifp->idev;
if (net_ratelimit())
- printk(KERN_INFO "%s: IPv6 duplicate address detected!\n",
- ifp->idev->dev->name);
+ printk(KERN_INFO "%s: IPv6 duplicate address %pI6 detected!\n",
+ ifp->idev->dev->name, &ifp->addr);
if (idev->cnf.accept_dad > 1 && !idev->cnf.disable_ipv6) {
struct in6_addr addr;
^ permalink raw reply related
* [RFC] ipv6: Change %pI6 format to output compacted addresses?
From: Jens Rosenboom @ 2009-08-12 15:39 UTC (permalink / raw)
To: Linux Network Developers
Currently the output looks like 2001:0db8:0000:0000:0000:0000:0000:0001
which might be compacted to 2001:db8::1. The code to do this could be
adapted from inet_ntop in glibc, which would add about 80 lines to
lib/vsprintf.c. How do you guys value the tradeoff between more readable
logging and increased kernel size?
This was already mentioned in
http://kerneltrap.org/mailarchive/linux-netdev/2008/11/25/4231684 but
noone seems to have taken up on it.
^ permalink raw reply
* Re: [PATCH 5/5] c/r: Add AF_UNIX support (v8)
From: Dan Smith @ 2009-08-12 15:36 UTC (permalink / raw)
To: Oren Laadan; +Cc: netdev
In-Reply-To: <4A82DFE0.4040108@librato.com>
OL> Before pulling this one, I took a quick look at this patch, and I
OL> saw that it still uses skb_morph despite the changelog and my
OL> memory...
That's correct. We've been through several ways of allocating the
skb's, so it's definitely confusing. We're back to skb_morph()
because I'm pre-allocating them for lock safety when traversing the
queues. The only thing that is allocated is the actual skb structure
itself; the buffers are still shared like with skb_clone().
OL> 1) Move 'struct ckpt_hdr_socket' et-al to checkpoint_hdr.h
Okay, yeah, I guess we never really resolved the question of where
they really belong, but I'll put them back there for now.
OL> 2) Move everything that is af_unix specific from net/checkpoint.c
OL> to (a new) net/unix/checkpoint.c (prototypes probably in af_unix.h
OL> ?)
Um, I guess I can. That will end up with a bunch of other
externally-defined interfaces between the generic and the specific
code, but I suppose I don't really have a solid argument against it.
OL> 3) Make sure that af_unix code does not compile (and is not called
OL> from net/checkpoint.c) unless CONFIG_UNIX is defined.
Okay.
OL> I pulled all the other patches already, will add this one once
OL> you resend.
Alright, well, ignore v9 I guess and hopefully we can settle on a nice
even number 10 :)
--
Dan Smith
IBM Linux Technology Center
email: danms@us.ibm.com
^ permalink raw reply
* Re: [PATCH 5/5] c/r: Add AF_UNIX support (v8)
From: Oren Laadan @ 2009-08-12 15:29 UTC (permalink / raw)
To: Dan Smith; +Cc: netdev
In-Reply-To: <1249918379-29414-6-git-send-email-danms@us.ibm.com>
Dan,
I just noticed that this message wasn't posted last night.
So hitting "send" now ... sorry about that.
-----
Before pulling this one, I took a quick look at this patch, and
I saw that it still uses skb_morph despite the changelog and my
memory...
Can you please verify that this is the latest ?
Also, while trying to pull it, I'd like to ask for three cosmetic
changes, if it isn't too much -
1) Move 'struct ckpt_hdr_socket' et-al to checkpoint_hdr.h
2) Move everything that is af_unix specific from net/checkpoint.c
to (a new) net/unix/checkpoint.c (prototypes probably in af_unix.h ?)
3) Make sure that af_unix code does not compile (and is not called
from net/checkpoint.c) unless CONFIG_UNIX is defined.
I pulled all the other patches already, will add this one once
you resend.
-----
Thanks,
Oren.
Dan Smith wrote:
> This patch adds basic checkpoint/restart support for AF_UNIX sockets. It
> has been tested with a single and multiple processes, and with data inflight
> at the time of checkpoint. It supports socketpair()s, path-based, and
> abstract sockets.
>
> Changes in v8:
> - Fix stale dev_alloc_skb() from before the conversion to skb_clone()
> - Fix a couple of broken error paths
> - Fix memory leak of kvec.iov_base on successful return from sendmsg()
> - Fix condition for deciding when to run sock_cptrst_verify()
> - Fix buffer queue copy algorithm to hold the lock during walk(s)
> - Log the errno when either getname() or getpeer() fails
> - Add comments about ancillary messages in the UNIX queue
> - Add TODO comments for credential restore and flags via setsockopt()
> - Add TODO comment about strangely-connected dgram sockets and the use
> of sendmsg(peer)
>
> Changes in v7:
> - Fix failure to free iov_base in error path of sock_read_buffer()
> - Change sock_read_buffer() to use _ckpt_read_obj_type() to get the
> header length and then use ckpt_kread() directly to read the payload
> - Change sock_read_buffers() to sock_unix_read_buffers() and break out
> some common functionality to better accommodate the subsequent INET
> patch
> - Generalize sock_unix_getnames() into sock_getnames() so INET can use it
> - Change skb_morph() to skb_clone() which uses the more common path and
> still avoids the copy
> - Add check to validate the socket type before creating socket
> on restore
> - Comment the CAP_NET_ADMIN override in sock_read_buffer_hdr
> - Strengthen the comment about priming the buffer limits
> - Change the objhash functions to deny direct checkpoint of sockets and
> remove the reference counting function
> - Change SOCKET_BUFFERS to SOCKET_QUEUE
> - Change this,peer objrefs to signed integers
> - Remove names from internal socket structures
> - Fix handling of sock_copy_buffers() result
> - Use ckpt_fill_fname() instead of d_path() for writing CWD
> - Use sock_getname() and sock_getpeer() for proper security hookage
> - Return -ENOSYS for unsupported socket families in checkpoint and restart
> - Use sock_setsockopt() and sock_getsockopt() where possible to save and
> restore socket option values
> - Check for SOCK_DESTROY flag in the global verify function because none
> of our supported socket types use it
> - Check for SOCK_USE_WRITE_QUEUE in AF_UNIX restore function because
> that flag should not be used on such a socket
> - Check socket state in UNIX restart path to validate the subset of valid
> values
[...]
^ permalink raw reply
* Re: [PATCHv2 2/2] vhost_net: a kernel-level virtio server
From: Paul E. McKenney @ 2009-08-12 15:26 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Gregory Haskins, netdev, virtualization, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org, Ingo Molnar, linux-mm,
Andrew Morton, hpa
In-Reply-To: <20090812141559.GA29387@redhat.com>
On Wed, Aug 12, 2009 at 05:15:59PM +0300, Michael S. Tsirkin wrote:
> On Wed, Aug 12, 2009 at 07:11:07AM -0700, Paul E. McKenney wrote:
> > On Wed, Aug 12, 2009 at 04:25:40PM +0300, Michael S. Tsirkin wrote:
> > > On Wed, Aug 12, 2009 at 09:01:35AM -0400, Gregory Haskins wrote:
> > > > I think I understand what your comment above meant: You don't need to
> > > > do synchronize_rcu() because you can flush the workqueue instead to
> > > > ensure that all readers have completed.
> > >
> > > Yes.
> > >
> > > > But if thats true, to me, the
> > > > rcu_dereference itself is gratuitous,
> > >
> > > Here's a thesis on what rcu_dereference does (besides documentation):
> > >
> > > reader does this
> > >
> > > A: sock = n->sock
> > > B: use *sock
> > >
> > > Say writer does this:
> > >
> > > C: newsock = allocate socket
> > > D: initialize(newsock)
> > > E: n->sock = newsock
> > > F: flush
> > >
> > >
> > > On Alpha, reads could be reordered. So, on smp, command A could get
> > > data from point F, and command B - from point D (uninitialized, from
> > > cache). IOW, you get fresh pointer but stale data.
> > > So we need to stick a barrier in there.
> > >
> > > > and that pointer is *not* actually
> > > > RCU protected (nor does it need to be).
> > >
> > > Heh, if readers are lockless and writer does init/update/sync,
> > > this to me spells rcu.
> >
> > If you are using call_rcu(), synchronize_rcu(), or one of the
> > similar primitives, then you absolutely need rcu_read_lock() and
> > rcu_read_unlock(), or one of the similar pairs of primitives.
>
> Right. I don't use any of these though.
>
> > If you -don't- use rcu_read_lock(), then you are pretty much restricted
> > to adding data, but never removing it.
> >
> > Make sense? ;-)
>
> Since I only access data from a workqueue, I replaced synchronize_rcu
> with workqueue flush. That's why I don't need rcu_read_lock.
Well, you -do- need -something- that takes on the role of rcu_read_lock(),
and in your case you in fact actually do. Your equivalent of
rcu_read_lock() is the beginning of execution of a workqueue item, and
the equivalent of rcu_read_unlock() is the end of execution of that same
workqueue item. Implicit, but no less real.
If a couple more uses like this show up, I might need to add this to
Documentation/RCU. ;-)
Thanx, Paul
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* [PATCH] c/r: Add AF_UNIX support (v9)
From: Dan Smith @ 2009-08-12 15:12 UTC (permalink / raw)
To: orenl; +Cc: containers, Oren Laaden, Alexey Dobriyan, netdev
This patch adds basic checkpoint/restart support for AF_UNIX sockets. It
has been tested with a single and multiple processes, and with data inflight
at the time of checkpoint. It supports socketpair()s, path-based, and
abstract sockets.
Changes in v9:
- Fix double-free of skb's in the list and target holding queue in the
error path of sock_copy_buffers()
- Adjust use of ckpt_read_string() to match new signature
Changes in v8:
- Fix stale dev_alloc_skb() from before the conversion to skb_clone()
- Fix a couple of broken error paths
- Fix memory leak of kvec.iov_base on successful return from sendmsg()
- Fix condition for deciding when to run sock_cptrst_verify()
- Fix buffer queue copy algorithm to hold the lock during walk(s)
- Log the errno when either getname() or getpeer() fails
- Add comments about ancillary messages in the UNIX queue
- Add TODO comments for credential restore and flags via setsockopt()
- Add TODO comment about strangely-connected dgram sockets and the use
of sendmsg(peer)
Changes in v7:
- Fix failure to free iov_base in error path of sock_read_buffer()
- Change sock_read_buffer() to use _ckpt_read_obj_type() to get the
header length and then use ckpt_kread() directly to read the payload
- Change sock_read_buffers() to sock_unix_read_buffers() and break out
some common functionality to better accommodate the subsequent INET
patch
- Generalize sock_unix_getnames() into sock_getnames() so INET can use it
- Change skb_morph() to skb_clone() which uses the more common path and
still avoids the copy
- Add check to validate the socket type before creating socket
on restore
- Comment the CAP_NET_ADMIN override in sock_read_buffer_hdr
- Strengthen the comment about priming the buffer limits
- Change the objhash functions to deny direct checkpoint of sockets and
remove the reference counting function
- Change SOCKET_BUFFERS to SOCKET_QUEUE
- Change this,peer objrefs to signed integers
- Remove names from internal socket structures
- Fix handling of sock_copy_buffers() result
- Use ckpt_fill_fname() instead of d_path() for writing CWD
- Use sock_getname() and sock_getpeer() for proper security hookage
- Return -ENOSYS for unsupported socket families in checkpoint and restart
- Use sock_setsockopt() and sock_getsockopt() where possible to save and
restore socket option values
- Check for SOCK_DESTROY flag in the global verify function because none
of our supported socket types use it
- Check for SOCK_USE_WRITE_QUEUE in AF_UNIX restore function because
that flag should not be used on such a socket
- Check socket state in UNIX restart path to validate the subset of valid
values
Changes in v6:
- Moved the socket addresses to the per-type header
- Eliminated the HASCWD flag
- Remove use of ckpt_write_err() in restart paths
- Change the order in which buffers are read so that we can set the
socket's limit equal to the size of the image's buffers (if appropriate)
and then restore the original values afterwards.
- Use the ckpt_validate_errno() helper
- Add a check to make sure that we didn't restore a (UNIX) socket with
any skb's in the send buffer
- Fix up sock_unix_join() to not leave addr uninitialized for socketpair
- Remove inclusion of checkpoint_hdr.h in the socket files
- Make sock_unix_write_cwd() use ckpt_write_string() and use the new
ckpt_read_string() for reading the cwd
- Use the restored realcred credentials in sock_unix_join()
- Fix error path of the chdir_and_bind
- Change the algorithm for reloading the socket buffers to use sendmsg()
on the socket's peer for better accounting
- For DGRAM sockets, check the backlog value against the system max
to avoid letting a restart bypass the overloaded queue length
- Use sock_bind() instead of sock->ops->bind() to gain the security hook
- Change "restart" to "restore" in some of the function names
Changes in v5:
- Change laddr and raddr buffers in socket header to be long enough
for INET6 addresses
- Place socket.c and sock.h function definitions inside #ifdef
CONFIG_CHECKPOINT
- Add explicit check in sock_unix_makeaddr() to refuse if the
checkpoint image specifies an addr length of 0
- Split sock_unix_restart() into a few pieces to facilitate:
- Changed behavior of the unix restore code so that unlinked LISTEN
sockets don't do a bind()...unlink()
- Save the base path of a bound socket's path so that we can chdir()
to the base before bind() if it is a relative path
- Call bind() for any socket that is not established but has a
non-zero-length local address
- Enforce the current sysctl limit on socket buffer size during restart
unless the user holds CAP_NET_ADMIN
- Unlink a path-based socket before calling bind()
Changes in v4:
- Changed the signdness of rcvlowat, rcvtimeo, sndtimeo, and backlog
to match their struct sock definitions. This should avoid issues
with sign extension.
- Add a sock_cptrst_verify() function to be run at restore time to
validate several of the values in the checkpoint image against
limits, flag masks, etc.
- Write an error string with ctk_write_err() in the obscure cases
- Don't write socket buffers for listen sockets
- Sanity check address lengths before we agree to allocate memory
- Check the result of inserting the peer object in the objhash on
restart
- Check return value of sock_cptrst() on restart
- Change logic in remote getname() phase of checkpoint to not fail for
closed (et al) sockets
- Eliminate the memory copy while reading socket buffers on restart
Changes in v3:
- Move sock_file_checkpoint() above sock_file_restore()
- Change __sock_file_*() functions to do_sock_file_*()
- Adjust some of the struct cr_hdr_socket alignment
- Improve the sock_copy_buffers() algorithm to avoid locking the source
queue for the entire operation
- Fix alignment in the socket header struct(s)
- Move the per-protocol structure (ckpt_hdr_socket_un) out of the
common socket header and read/write it separately
- Fix missing call to sock_cptrst() in restore path
- Break out the socket joining into another function
- Fix failure to restore the socket address thus fixing getname()
- Check the state values on restart
- Fix case of state being TCP_CLOSE, which allows dgram sockets to be
properly connected (if appropriate) to their peer and maintain the
sockaddr for getname() operation
- Fix restoring a listening socket that has been unlink()'d
- Fix checkpointing sockets with an in-flight FD-passing SKB. Fail
with EBUSY.
- Fix checkpointing listening sockets with an unaccepted connection.
Fail with EBUSY.
- Changed 'un' to 'unix' in function and structure names
Changes in v2:
- Change GFP_KERNEL to GFP_ATOMIC in sock_copy_buffers() (this seems
to be rather common in other uses of skb_copy())
- Move the ckpt_hdr_socket structure definition to linux/socket.h
- Fix whitespace issue
- Move sock_file_checkpoint() to net/socket.c for symmetry
Cc: Oren Laaden <orenl@cs.columbia.edu>
Cc: Alexey Dobriyan <adobriyan@gmail.com>
Cc: netdev@vger.kernel.org
Signed-off-by: Dan Smith <danms@us.ibm.com>
---
checkpoint/files.c | 7 +
checkpoint/objhash.c | 19 +
include/linux/checkpoint_hdr.h | 12 +
include/linux/socket.h | 65 +++
include/net/sock.h | 11 +
net/Makefile | 2 +
net/checkpoint.c | 954 ++++++++++++++++++++++++++++++++++++++++
net/socket.c | 85 ++++
8 files changed, 1155 insertions(+), 0 deletions(-)
create mode 100644 net/checkpoint.c
diff --git a/checkpoint/files.c b/checkpoint/files.c
index 7aecac9..8729e00 100644
--- a/checkpoint/files.c
+++ b/checkpoint/files.c
@@ -21,6 +21,7 @@
#include <linux/syscalls.h>
#include <linux/checkpoint.h>
#include <linux/checkpoint_hdr.h>
+#include <net/sock.h>
/**************************************************************************
@@ -565,6 +566,12 @@ static struct restore_file_ops restore_file_ops[] = {
.file_type = CKPT_FILE_FIFO,
.restore = fifo_file_restore,
},
+ /* socket */
+ {
+ .file_name = "SOCKET",
+ .file_type = CKPT_FILE_SOCKET,
+ .restore = sock_file_restore,
+ },
};
static struct file *do_restore_file(struct ckpt_ctx *ctx)
diff --git a/checkpoint/objhash.c b/checkpoint/objhash.c
index a2fb278..4bfa1d8 100644
--- a/checkpoint/objhash.c
+++ b/checkpoint/objhash.c
@@ -20,6 +20,7 @@
#include <linux/user_namespace.h>
#include <linux/checkpoint.h>
#include <linux/checkpoint_hdr.h>
+#include <net/sock.h>
struct ckpt_obj;
struct ckpt_obj_ops;
@@ -234,6 +235,17 @@ static void obj_groupinfo_drop(void *ptr)
put_group_info((struct group_info *) ptr);
}
+static int obj_sock_grab(void *ptr)
+{
+ sock_hold((struct sock *) ptr);
+ return 0;
+}
+
+static void obj_sock_drop(void *ptr)
+{
+ sock_put((struct sock *) ptr);
+}
+
static struct ckpt_obj_ops ckpt_obj_ops[] = {
/* ignored object */
{
@@ -356,6 +368,13 @@ static struct ckpt_obj_ops ckpt_obj_ops[] = {
.checkpoint = checkpoint_groupinfo,
.restore = restore_groupinfo,
},
+ /* sock object */
+ {
+ .obj_name = "SOCKET",
+ .obj_type = CKPT_OBJ_SOCK,
+ .ref_drop = obj_sock_drop,
+ .ref_grab = obj_sock_grab,
+ },
};
diff --git a/include/linux/checkpoint_hdr.h b/include/linux/checkpoint_hdr.h
index 4b85956..d8816ed 100644
--- a/include/linux/checkpoint_hdr.h
+++ b/include/linux/checkpoint_hdr.h
@@ -88,6 +88,12 @@ enum {
CKPT_HDR_SIGHAND = 601,
+ CKPT_HDR_FD_SOCKET = 701,
+ CKPT_HDR_SOCKET,
+ CKPT_HDR_SOCKET_QUEUE,
+ CKPT_HDR_SOCKET_BUFFER,
+ CKPT_HDR_SOCKET_UNIX,
+
CKPT_HDR_TAIL = 9001,
CKPT_HDR_ERROR = 9999,
@@ -122,6 +128,7 @@ enum obj_type {
CKPT_OBJ_CRED,
CKPT_OBJ_USER,
CKPT_OBJ_GROUPINFO,
+ CKPT_OBJ_SOCK,
CKPT_OBJ_MAX
};
@@ -327,6 +334,7 @@ enum file_type {
CKPT_FILE_GENERIC,
CKPT_FILE_PIPE,
CKPT_FILE_FIFO,
+ CKPT_FILE_SOCKET,
CKPT_FILE_MAX
};
@@ -350,6 +358,10 @@ struct ckpt_hdr_file_pipe {
__s32 pipe_objref;
} __attribute__((aligned(8)));
+struct ckpt_hdr_file_socket {
+ struct ckpt_hdr_file common;
+} __attribute__((aligned(8)));
+
struct ckpt_hdr_utsns {
struct ckpt_hdr h;
char sysname[__NEW_UTS_LEN + 1];
diff --git a/include/linux/socket.h b/include/linux/socket.h
index 3b461df..7d5fd48 100644
--- a/include/linux/socket.h
+++ b/include/linux/socket.h
@@ -23,6 +23,7 @@ struct __kernel_sockaddr_storage {
#include <linux/uio.h> /* iovec support */
#include <linux/types.h> /* pid_t */
#include <linux/compiler.h> /* __user */
+#include <linux/checkpoint.h> /* struct ckpt_hdr */
#ifdef __KERNEL__
# ifdef CONFIG_PROC_FS
@@ -328,5 +329,69 @@ extern int move_addr_to_kernel(void __user *uaddr, int ulen, struct sockaddr *ka
extern int put_cmsg(struct msghdr*, int level, int type, int len, void *data);
#endif
+
+#ifdef CONFIG_CHECKPOINT
+#include <linux/un.h> /* sockaddr_un */
+
+#define CKPT_UNIX_LINKED 1
+struct ckpt_hdr_socket_unix {
+ struct ckpt_hdr h;
+ __s32 this;
+ __s32 peer;
+ __u32 flags;
+ __u32 laddr_len;
+ __u32 raddr_len;
+ struct sockaddr_un laddr;
+ struct sockaddr_un raddr;
+} __attribute__ ((aligned(8)));
+
+struct ckpt_hdr_socket {
+ struct ckpt_hdr h;
+
+ struct { /* struct socket */
+ __u64 flags;
+ __u8 state;
+ } socket __attribute__ ((aligned(8)));
+
+ struct { /* struct sock_common */
+ __u32 bound_dev_if;
+ __u32 reuse;
+ __u16 family;
+ __u8 state;
+ } sock_common __attribute__ ((aligned(8)));
+
+ struct { /* struct sock */
+ __s64 rcvlowat;
+ __u64 flags;
+
+ __u32 err;
+ __u32 err_soft;
+ __u32 priority;
+ __s32 rcvbuf;
+ __s32 sndbuf;
+ __u16 type;
+ __s16 backlog;
+
+ __u8 protocol;
+ __u8 state;
+ __u8 shutdown;
+ __u8 userlocks;
+ __u8 no_check;
+
+ struct linger linger;
+ struct timeval rcvtimeo;
+ struct timeval sndtimeo;
+
+ } sock __attribute__ ((aligned(8)));
+
+} __attribute__ ((aligned(8)));
+
+struct ckpt_hdr_socket_queue {
+ struct ckpt_hdr h;
+ __u32 skb_count;
+ __u32 total_bytes;
+} __attribute__ ((aligned(8)));
+#endif /* CONFIG_CHECKPOINT */
+
#endif /* not kernel and not glibc */
#endif /* _LINUX_SOCKET_H */
diff --git a/include/net/sock.h b/include/net/sock.h
index 43b9599..da75f2f 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -1640,4 +1640,15 @@ extern int sysctl_optmem_max;
extern __u32 sysctl_wmem_default;
extern __u32 sysctl_rmem_default;
+#ifdef CONFIG_CHECKPOINT
+/* Checkpoint/Restart Functions */
+struct ckpt_ctx;
+struct ckpt_hdr_socket;
+extern int sock_file_checkpoint(struct ckpt_ctx *, void *);
+extern void *sock_file_restore(struct ckpt_ctx *);
+extern struct socket *do_sock_file_restore(struct ckpt_ctx *,
+ struct ckpt_hdr_socket *);
+extern int do_sock_file_checkpoint(struct ckpt_ctx *ctx, struct file *file);
+#endif
+
#endif /* _SOCK_H */
diff --git a/net/Makefile b/net/Makefile
index ba324ae..91d12fe 100644
--- a/net/Makefile
+++ b/net/Makefile
@@ -66,3 +66,5 @@ ifeq ($(CONFIG_NET),y)
obj-$(CONFIG_SYSCTL) += sysctl_net.o
endif
obj-$(CONFIG_WIMAX) += wimax/
+
+obj-$(CONFIG_CHECKPOINT) += checkpoint.o
diff --git a/net/checkpoint.c b/net/checkpoint.c
new file mode 100644
index 0000000..5199d76
--- /dev/null
+++ b/net/checkpoint.c
@@ -0,0 +1,954 @@
+/*
+ * Copyright 2009 IBM Corporation
+ *
+ * Author: Dan Smith <danms@us.ibm.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation, version 2 of the
+ * License.
+ */
+
+#include <linux/socket.h>
+#include <linux/mount.h>
+#include <linux/file.h>
+#include <linux/namei.h>
+#include <linux/syscalls.h>
+#include <linux/sched.h>
+#include <linux/fs_struct.h>
+
+#include <net/af_unix.h>
+#include <net/tcp_states.h>
+
+#include <linux/checkpoint.h>
+#include <linux/checkpoint_hdr.h>
+
+#define UNIX_ADDR_EMPTY(a) (a <= sizeof(short))
+
+static inline int sock_unix_need_cwd(struct sockaddr_un *addr,
+ unsigned long len)
+{
+ return (!UNIX_ADDR_EMPTY(len)) &&
+ addr->sun_path[0] &&
+ (addr->sun_path[0] != '/');
+}
+
+static int sock_copy_buffers(struct sk_buff_head *from,
+ struct sk_buff_head *to,
+ uint32_t *total_bytes)
+{
+ int count1 = 0;
+ int count2 = 0;
+ int i;
+ struct sk_buff *skb;
+ struct sk_buff **skbs;
+
+ *total_bytes = 0;
+
+ spin_lock(&from->lock);
+ skb_queue_walk(from, skb)
+ count1++;
+ spin_unlock(&from->lock);
+
+ skbs = kzalloc(sizeof(*skbs) * count1, GFP_KERNEL);
+ if (!skbs)
+ return -ENOMEM;
+
+ for (i = 0; i < count1; i++) {
+ skbs[i] = dev_alloc_skb(0);
+ if (!skbs[i])
+ goto err;
+ }
+
+ i = 0;
+ spin_lock(&from->lock);
+ skb_queue_walk(from, skb) {
+ if (++count2 > count1)
+ break; /* The queue changed as we read it */
+
+ skb_morph(skbs[i], skb);
+ skb_queue_tail(to, skbs[i]);
+
+ *total_bytes += skb->len;
+ i++;
+ }
+ spin_unlock(&from->lock);
+
+ if (count1 != count2)
+ goto err;
+
+ kfree(skbs);
+
+ return count1;
+ err:
+ while (skb_dequeue(to))
+ ; /* Pull all the buffers out of the queue */
+ for (i = 0; i < count1; i++)
+ kfree_skb(skbs[i]);
+ kfree(skbs);
+
+ return -EAGAIN;
+}
+
+static int __sock_write_buffers(struct ckpt_ctx *ctx,
+ struct sk_buff_head *queue)
+{
+ struct sk_buff *skb;
+ int ret = 0;
+
+ skb_queue_walk(queue, skb) {
+ /* FIXME: This could be a false positive for non-unix
+ * buffers, so add a type check here in the
+ * future
+ */
+ if (UNIXCB(skb).fp) {
+ ckpt_write_err(ctx, "fd-passing is not supported");
+ return -EBUSY;
+ }
+
+ /* The other ancillary messages are always present
+ * unlike descriptors. Even though we can't detect
+ * them and fail the checkpoint, we're not at risk
+ * because we don't save out (or restore) the control
+ * information contained in the skb.
+ */
+
+ ret = ckpt_write_obj_type(ctx, skb->data, skb->len,
+ CKPT_HDR_SOCKET_BUFFER);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
+static int sock_write_buffers(struct ckpt_ctx *ctx, struct sk_buff_head *queue)
+{
+ struct ckpt_hdr_socket_queue *h;
+ struct sk_buff_head tmpq;
+ int ret = -ENOMEM;
+
+ h = ckpt_hdr_get_type(ctx, sizeof(*h), CKPT_HDR_SOCKET_QUEUE);
+ if (!h)
+ return -ENOMEM;
+
+ skb_queue_head_init(&tmpq);
+
+ ret = sock_copy_buffers(queue, &tmpq, &h->total_bytes);
+ if (ret < 0)
+ goto out;
+
+ h->skb_count = ret;
+ ret = ckpt_write_obj(ctx, (struct ckpt_hdr *) h);
+ if (!ret)
+ ret = __sock_write_buffers(ctx, &tmpq);
+
+ out:
+ ckpt_hdr_put(ctx, h);
+ __skb_queue_purge(&tmpq);
+
+ return ret;
+}
+
+static int sock_unix_write_cwd(struct ckpt_ctx *ctx,
+ struct sock *sock,
+ const char *sockpath)
+{
+ struct path path;
+ char *buf;
+ char *fqpath;
+ int offset;
+ int len = PATH_MAX;
+ int ret = -ENOENT;
+
+ buf = kmalloc(len, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ path.dentry = unix_sk(sock)->dentry;
+ path.mnt = unix_sk(sock)->mnt;
+
+ fqpath = ckpt_fill_fname(&path, &ctx->fs_mnt, buf, &len);
+ if (IS_ERR(fqpath)) {
+ ret = PTR_ERR(fqpath);
+ goto out;
+ }
+
+ offset = strlen(fqpath) - strlen(sockpath);
+ if (offset <= 0) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ fqpath[offset] = '\0';
+
+ ckpt_debug("writing socket directory: %s\n", fqpath);
+ ret = ckpt_write_string(ctx, fqpath, strlen(fqpath));
+ out:
+ kfree(buf);
+ return ret;
+}
+
+static int sock_getnames(struct ckpt_ctx *ctx,
+ struct socket *socket,
+ struct sockaddr *loc, unsigned *loc_len,
+ struct sockaddr *rem, unsigned *rem_len)
+{
+ int ret;
+
+ ret = sock_getname(socket, loc, loc_len);
+ if (ret) {
+ ckpt_write_err(ctx, "Unable to getname of local: %i", ret);
+ return -EINVAL;
+ }
+
+ ret = sock_getpeer(socket, rem, rem_len);
+ if (ret) {
+ if ((socket->sk->sk_type != SOCK_DGRAM) &&
+ (socket->sk->sk_state == TCP_ESTABLISHED)) {
+ ckpt_write_err(ctx, "Unable to getname of remote: %i",
+ ret);
+ return -EINVAL;
+ }
+ *rem_len = 0;
+ }
+
+ return 0;
+}
+
+static int sock_unix_checkpoint(struct ckpt_ctx *ctx,
+ struct socket *socket,
+ struct ckpt_hdr_socket *h)
+{
+ struct unix_sock *sk = unix_sk(socket->sk);
+ struct unix_sock *pr = unix_sk(sk->peer);
+ struct ckpt_hdr_socket_unix *un;
+ int new;
+ int ret = -ENOMEM;
+
+ if ((socket->sk->sk_state == TCP_LISTEN) &&
+ !skb_queue_empty(&socket->sk->sk_receive_queue)) {
+ ckpt_write_err(ctx, "listening socket has unaccepted peers");
+ return -EBUSY;
+ }
+
+ un = ckpt_hdr_get_type(ctx, sizeof(*un), CKPT_HDR_SOCKET_UNIX);
+ if (!un)
+ return -EINVAL;
+
+ ret = sock_getnames(ctx, socket,
+ (struct sockaddr *)&un->laddr, &un->laddr_len,
+ (struct sockaddr *)&un->raddr, &un->raddr_len);
+ if (ret)
+ goto out;
+
+ if (sk->dentry && (sk->dentry->d_inode->i_nlink > 0))
+ un->flags |= CKPT_UNIX_LINKED;
+
+ un->this = ckpt_obj_lookup_add(ctx, sk, CKPT_OBJ_SOCK, &new);
+ if (un->this < 0)
+ goto out;
+
+ if (sk->peer)
+ un->peer = ckpt_obj_lookup_add(ctx, pr, CKPT_OBJ_SOCK, &new);
+ else
+ un->peer = 0;
+
+ if (un->peer < 0) {
+ ret = un->peer;
+ goto out;
+ }
+
+ ret = ckpt_write_obj(ctx, (struct ckpt_hdr *) h);
+ if (ret < 0)
+ goto out;
+
+ ret = ckpt_write_obj(ctx, (struct ckpt_hdr *) un);
+ if (ret < 0)
+ goto out;
+
+ if (sock_unix_need_cwd(&un->laddr, un->laddr_len))
+ ret = sock_unix_write_cwd(ctx, socket->sk, un->laddr.sun_path);
+ out:
+ ckpt_hdr_put(ctx, un);
+
+ return ret;
+}
+
+static int sock_cptrst_verify(struct ckpt_hdr_socket *h)
+{
+ uint8_t userlocks_mask = SOCK_SNDBUF_LOCK | SOCK_RCVBUF_LOCK |
+ SOCK_BINDADDR_LOCK | SOCK_BINDPORT_LOCK;
+
+ if (h->sock.shutdown & ~SHUTDOWN_MASK)
+ return -EINVAL;
+ if (h->sock.userlocks & ~userlocks_mask)
+ return -EINVAL;
+ if (!ckpt_validate_errno(h->sock.err))
+ return -EINVAL;
+
+ /* None of our supported types use this flag */
+ if (h->sock.flags & SOCK_DESTROY)
+ return -EINVAL;
+
+ return 0;
+}
+
+static int sock_cptrst_opt(int op, struct socket *socket,
+ int optname, char *opt, int len)
+{
+ mm_segment_t fs;
+ int ret;
+
+ fs = get_fs();
+ set_fs(KERNEL_DS);
+
+ if (op == CKPT_CPT)
+ ret = sock_getsockopt(socket, SOL_SOCKET, optname, opt, &len);
+ else
+ ret = sock_setsockopt(socket, SOL_SOCKET, optname, opt, len);
+
+ set_fs(fs);
+
+ return ret;
+}
+
+#define CKPT_COPY_SOPT(op, sock, name, opt) \
+ sock_cptrst_opt(op, sock->sk_socket, name, (char *)opt, sizeof(*opt))
+
+static int sock_cptrst_bufopts(int op, struct sock *sock,
+ struct ckpt_hdr_socket *h)
+
+{
+ if (CKPT_COPY_SOPT(op, sock, SO_RCVBUF, &h->sock.rcvbuf))
+ if ((op == CKPT_RST) &&
+ CKPT_COPY_SOPT(op, sock, SO_RCVBUFFORCE, &h->sock.rcvbuf)) {
+ ckpt_debug("Failed to set SO_RCVBUF");
+ return -EINVAL;
+ }
+
+ if (CKPT_COPY_SOPT(op, sock, SO_SNDBUF, &h->sock.sndbuf))
+ if ((op == CKPT_RST) &&
+ CKPT_COPY_SOPT(op, sock, SO_SNDBUFFORCE, &h->sock.sndbuf)) {
+ ckpt_debug("Failed to set SO_SNDBUF");
+ return -EINVAL;
+ }
+
+ /* It's silly that we have to fight ourselves here, but
+ * sock_setsockopt() doubles the initial value, so divide here
+ * to store the user's value and avoid doubling on restart
+ */
+ if ((op == CKPT_CPT) && (h->sock.rcvbuf != SOCK_MIN_RCVBUF))
+ h->sock.rcvbuf >>= 1;
+
+ if ((op == CKPT_CPT) && (h->sock.sndbuf != SOCK_MIN_SNDBUF))
+ h->sock.sndbuf >>= 1;
+
+ return 0;
+}
+
+static int sock_cptrst(struct ckpt_ctx *ctx,
+ struct sock *sock,
+ struct ckpt_hdr_socket *h,
+ int op)
+{
+ if (sock->sk_socket) {
+ CKPT_COPY(op, h->socket.flags, sock->sk_socket->flags);
+ CKPT_COPY(op, h->socket.state, sock->sk_socket->state);
+ }
+
+ CKPT_COPY(op, h->sock_common.bound_dev_if, sock->sk_bound_dev_if);
+ CKPT_COPY(op, h->sock_common.family, sock->sk_family);
+
+ CKPT_COPY(op, h->sock.shutdown, sock->sk_shutdown);
+ CKPT_COPY(op, h->sock.userlocks, sock->sk_userlocks);
+ CKPT_COPY(op, h->sock.no_check, sock->sk_no_check);
+ CKPT_COPY(op, h->sock.protocol, sock->sk_protocol);
+ CKPT_COPY(op, h->sock.err, sock->sk_err);
+ CKPT_COPY(op, h->sock.err_soft, sock->sk_err_soft);
+ CKPT_COPY(op, h->sock.type, sock->sk_type);
+ CKPT_COPY(op, h->sock.state, sock->sk_state);
+ CKPT_COPY(op, h->sock.backlog, sock->sk_max_ack_backlog);
+
+ /* TODO:
+ * Break out setting each of the flags to use setsockopt() or
+ * perform proper security check
+ */
+ CKPT_COPY(op, h->sock.flags, sock->sk_flags);
+
+ if (sock_cptrst_bufopts(op, sock, h))
+ return -EINVAL;
+
+ if (CKPT_COPY_SOPT(op, sock, SO_REUSEADDR, &h->sock_common.reuse)) {
+ ckpt_debug("Failed to set SO_REUSEADDR");
+ return -EINVAL;
+ }
+
+ if (CKPT_COPY_SOPT(op, sock, SO_PRIORITY, &h->sock.priority)) {
+ ckpt_debug("Failed to set SO_PRIORITY");
+ return -EINVAL;
+ }
+
+ if (CKPT_COPY_SOPT(op, sock, SO_RCVLOWAT, &h->sock.rcvlowat)) {
+ ckpt_debug("Failed to set SO_RCVLOWAT");
+ return -EINVAL;
+ }
+
+ if (CKPT_COPY_SOPT(op, sock, SO_LINGER, &h->sock.linger)) {
+ ckpt_debug("Failed to set SO_LINGER");
+ return -EINVAL;
+ }
+
+ if (CKPT_COPY_SOPT(op, sock, SO_SNDTIMEO, &h->sock.sndtimeo)) {
+ ckpt_debug("Failed to set SO_SNDTIMEO");
+ return -EINVAL;
+ }
+
+ if (CKPT_COPY_SOPT(op, sock, SO_RCVTIMEO, &h->sock.rcvtimeo)) {
+ ckpt_debug("Failed to set SO_RCVTIMEO");
+ return -EINVAL;
+ }
+
+ if ((h->socket.state == SS_CONNECTED) &&
+ (h->sock.state != TCP_ESTABLISHED)) {
+ ckpt_debug("socket/sock in inconsistent state: %i/%i",
+ h->socket.state, h->sock.state);
+ return -EINVAL;
+ } else if ((h->sock.state < TCP_ESTABLISHED) ||
+ (h->sock.state >= TCP_MAX_STATES)) {
+ ckpt_debug("sock in invalid state: %i", h->sock.state);
+ return -EINVAL;
+ } else if ((h->socket.state < SS_FREE) ||
+ (h->socket.state > SS_DISCONNECTING)) {
+ ckpt_debug("socket in invalid state: %i",
+ h->socket.state);
+ return -EINVAL;
+ }
+
+ if (op == CKPT_RST)
+ return sock_cptrst_verify(h);
+ else
+ return 0;
+}
+
+int do_sock_file_checkpoint(struct ckpt_ctx *ctx, struct file *file)
+{
+ struct socket *socket = file->private_data;
+ struct sock *sock = socket->sk;
+ struct ckpt_hdr_socket *h;
+ int ret = 0;
+
+ h = ckpt_hdr_get_type(ctx, sizeof(*h), CKPT_HDR_SOCKET);
+ if (!h)
+ return -ENOMEM;
+
+ ret = sock_cptrst(ctx, sock, h, CKPT_CPT);
+ if (ret)
+ goto out;
+
+ if (sock->sk_family == AF_UNIX) {
+ ret = sock_unix_checkpoint(ctx, socket, h);
+ if (ret)
+ goto out;
+ } else {
+ ckpt_write_err(ctx, "unsupported socket family %i",
+ sock->sk_family);
+ ret = -ENOSYS;
+ goto out;
+ }
+
+ if (sock->sk_state != TCP_LISTEN) {
+ ret = sock_write_buffers(ctx, &sock->sk_receive_queue);
+ if (ret)
+ goto out;
+
+ ret = sock_write_buffers(ctx, &sock->sk_write_queue);
+ if (ret)
+ goto out;
+ }
+ out:
+ ckpt_hdr_put(ctx, h);
+
+ return ret;
+}
+
+static int sock_read_buffer_sendmsg(struct ckpt_ctx *ctx, struct sock *sock)
+{
+ struct msghdr msg;
+ struct kvec kvec;
+ int ret = 0;
+ int len;
+
+ memset(&msg, 0, sizeof(msg));
+
+ len = _ckpt_read_obj_type(ctx, NULL, 0, CKPT_HDR_SOCKET_BUFFER);
+ if (len < 0)
+ return len;
+
+ if (len > SKB_MAX_ALLOC) {
+ ckpt_debug("Socket buffer too big (%i > %lu)",
+ len, SKB_MAX_ALLOC);
+ return -ENOSPC;
+ }
+
+ kvec.iov_len = len;
+ kvec.iov_base = kmalloc(len, GFP_KERNEL);
+ if (!kvec.iov_base)
+ return -ENOMEM;
+
+ ret = ckpt_kread(ctx, kvec.iov_base, len);
+ if (ret < 0)
+ goto out;
+
+ ret = kernel_sendmsg(sock->sk_socket, &msg, &kvec, 1, len);
+ ckpt_debug("kernel_sendmsg(%i): %i\n", len, ret);
+ if ((ret > 0) && (ret != len))
+ ret = -ENOMEM;
+ out:
+ kfree(kvec.iov_base);
+
+ return ret;
+}
+
+static struct ckpt_hdr_socket_queue *sock_read_buffer_hdr(struct ckpt_ctx *ctx,
+ uint32_t *bufsize)
+{
+ struct ckpt_hdr_socket_queue *h;
+ int err = 0;
+
+ h = ckpt_read_obj_type(ctx, sizeof(*h), CKPT_HDR_SOCKET_QUEUE);
+ if (IS_ERR(h))
+ return h;
+
+ if (!bufsize) {
+ if (h->total_bytes != 0) {
+ ckpt_debug("Expected empty buffer, got %u\n",
+ h->total_bytes);
+ err = -EINVAL;
+ }
+ } else if (h->total_bytes > *bufsize) {
+ /* NB: We let CAP_NET_ADMIN override the system buffer limit
+ * as setsockopt() does
+ */
+ if (capable(CAP_NET_ADMIN))
+ *bufsize = h->total_bytes;
+ else {
+ ckpt_debug("Buffer total %u exceeds limit %u\n",
+ h->total_bytes, *bufsize);
+ err = -EINVAL;
+ }
+ }
+
+ if (err) {
+ ckpt_hdr_put(ctx, h);
+ return ERR_PTR(err);
+ } else
+ return h;
+}
+
+static int sock_unix_read_buffers(struct ckpt_ctx *ctx,
+ struct sock *sock,
+ uint32_t *bufsize)
+{
+ uint8_t sock_shutdown;
+ struct ckpt_hdr_socket_queue *h;
+ int ret = 0;
+ int i;
+
+ h = sock_read_buffer_hdr(ctx, bufsize);
+ if (IS_ERR(h))
+ return PTR_ERR(h);
+
+ /* If peer is shutdown, unshutdown it for this process */
+ sock_shutdown = sock->sk_shutdown;
+ sock->sk_shutdown &= ~SHUTDOWN_MASK;
+
+ for (i = 0; i < h->skb_count; i++) {
+ ret = sock_read_buffer_sendmsg(ctx, sock);
+ ckpt_debug("read_buffer_sendmsg(%i): %i\n", i, ret);
+ if (ret < 0)
+ break;
+
+ if (ret > h->total_bytes) {
+ ckpt_debug("Buffers exceeded claim");
+ ret = -EINVAL;
+ break;
+ }
+
+ h->total_bytes -= ret;
+ ret = 0;
+ }
+
+ sock->sk_shutdown = sock_shutdown;
+ ckpt_hdr_put(ctx, h);
+
+ return ret;
+}
+
+static struct unix_address *sock_unix_makeaddr(struct sockaddr_un *sun_addr,
+ unsigned len)
+{
+ struct unix_address *addr;
+
+ if (len > sizeof(struct sockaddr_un))
+ return ERR_PTR(-EINVAL);
+
+ addr = kmalloc(sizeof(*addr) + len, GFP_KERNEL);
+ if (!addr)
+ return ERR_PTR(-ENOMEM);
+
+ memcpy(addr->name, sun_addr, len);
+ addr->len = len;
+ atomic_set(&addr->refcnt, 1);
+
+ return addr;
+}
+
+static int sock_unix_join(struct ckpt_ctx *ctx,
+ struct sock *a,
+ struct sock *b,
+ struct ckpt_hdr_socket_unix *un)
+{
+ struct unix_address *addr = NULL;
+
+ /* FIXME: Do we need to call some security hooks here? */
+
+ sock_hold(a);
+ sock_hold(b);
+
+ unix_sk(a)->peer = b;
+ unix_sk(b)->peer = a;
+
+ /* TODO:
+ * Checkpoint the credentials, restore them here if the values match
+ * the restored creds or we may_setuid()
+ */
+
+ a->sk_peercred.pid = task_tgid_vnr(current);
+ a->sk_peercred.uid = ctx->realcred->uid;
+ a->sk_peercred.gid = ctx->realcred->gid;
+
+ b->sk_peercred.pid = a->sk_peercred.pid;
+ b->sk_peercred.uid = a->sk_peercred.uid;
+ b->sk_peercred.gid = a->sk_peercred.gid;
+
+ if (!UNIX_ADDR_EMPTY(un->raddr_len))
+ addr = sock_unix_makeaddr(&un->raddr, un->raddr_len);
+ else if (!UNIX_ADDR_EMPTY(un->laddr_len))
+ addr = sock_unix_makeaddr(&un->laddr, un->laddr_len);
+
+ if (IS_ERR(addr))
+ return PTR_ERR(addr);
+ else if (addr) {
+ atomic_inc(&addr->refcnt); /* Held by both ends */
+ unix_sk(a)->addr = unix_sk(b)->addr = addr;
+ }
+
+ return 0;
+}
+
+static int sock_unix_restore_connected(struct ckpt_ctx *ctx,
+ struct ckpt_hdr_socket *h,
+ struct ckpt_hdr_socket_unix *un,
+ struct socket *socket)
+{
+ struct sock *this = ckpt_obj_fetch(ctx, un->this, CKPT_OBJ_SOCK);
+ struct sock *peer = ckpt_obj_fetch(ctx, un->peer, CKPT_OBJ_SOCK);
+ struct socket *tmp = NULL;
+ int ret;
+
+ if (!IS_ERR(this) && !IS_ERR(peer)) {
+ /* We're last */
+ struct socket *old = this->sk_socket;
+
+ old->sk = NULL;
+ sock_release(old);
+ sock_graft(this, socket);
+
+ } else if ((PTR_ERR(this) == -EINVAL) && (PTR_ERR(peer) == -EINVAL)) {
+ /* We're first */
+ int family = socket->sk->sk_family;
+ int type = socket->sk->sk_type;
+
+ ret = sock_create(family, type, 0, &tmp);
+ ckpt_debug("sock_create: %i\n", ret);
+ if (ret)
+ goto out;
+
+ this = socket->sk;
+ peer = tmp->sk;
+
+ ret = ckpt_obj_insert(ctx, this, un->this, CKPT_OBJ_SOCK);
+ if (ret < 0)
+ goto out;
+
+ ret = ckpt_obj_insert(ctx, peer, un->peer, CKPT_OBJ_SOCK);
+ if (ret < 0)
+ goto out;
+
+ ret = sock_unix_join(ctx, this, peer, un);
+ ckpt_debug("sock_unix_join: %i\n", ret);
+ if (ret)
+ goto out;
+
+ } else {
+ ckpt_debug("Order Error\n");
+ ret = PTR_ERR(this);
+ goto out;
+ }
+
+ /* Prime the socket's buffer limit with the maximum. These will be
+ * overwritten with the values in the checkpoint stream in a later
+ * phase.
+ */
+ peer->sk_userlocks |= SOCK_SNDBUF_LOCK;
+ peer->sk_sndbuf = sysctl_wmem_max;
+
+ /* Read my buffers and sendmsg() them back to me via my peer */
+
+ /* TODO: handle the unconnected case, as well, as the case
+ * where sendto() has been used on some of the buffers
+ */
+
+ ret = sock_unix_read_buffers(ctx, peer, &peer->sk_sndbuf);
+ ckpt_debug("sock_unix_read_buffers: %i\n", ret);
+ if (ret)
+ goto out;
+
+ /* Read peer's buffers and expect 0 */
+ ret = sock_unix_read_buffers(ctx, peer, NULL);
+ out:
+ if (tmp && ret)
+ sock_release(tmp);
+
+ return ret;
+}
+
+static int sock_unix_unlink(const char *name)
+{
+ struct path spath;
+ struct path ppath;
+ int ret;
+
+ ret = kern_path(name, 0, &spath);
+ if (ret)
+ return ret;
+
+ ret = kern_path(name, LOOKUP_PARENT, &ppath);
+ if (ret)
+ goto out_s;
+
+ if (!spath.dentry) {
+ ckpt_debug("No dentry found for %s\n", name);
+ ret = -ENOENT;
+ goto out_p;
+ }
+
+ if (!ppath.dentry || !ppath.dentry->d_inode) {
+ ckpt_debug("No inode for parent of %s\n", name);
+ ret = -ENOENT;
+ goto out_p;
+ }
+
+ ret = vfs_unlink(ppath.dentry->d_inode, spath.dentry);
+ out_p:
+ path_put(&ppath);
+ out_s:
+ path_put(&spath);
+
+ return ret;
+}
+
+/* Call bind() for socket, optionally changing (temporarily) to @path first
+ * if non-NULL
+ */
+static int sock_unix_chdir_and_bind(struct socket *socket,
+ const char *path,
+ struct sockaddr *addr,
+ unsigned long addrlen)
+{
+ struct sockaddr_un *un = (struct sockaddr_un *)addr;
+ int ret;
+ struct path cur;
+ struct path dir;
+
+ if (path) {
+ ckpt_debug("switching to cwd %s for unix bind", path);
+
+ ret = kern_path(path, 0, &dir);
+ if (ret)
+ return ret;
+
+ ret = inode_permission(dir.dentry->d_inode,
+ MAY_EXEC | MAY_ACCESS);
+ if (ret)
+ goto out;
+
+ write_lock(¤t->fs->lock);
+ cur = current->fs->pwd;
+ current->fs->pwd = dir;
+ write_unlock(¤t->fs->lock);
+ }
+
+ ret = sock_unix_unlink(un->sun_path);
+ ckpt_debug("unlink(%s): %i\n", un->sun_path, ret);
+ if ((ret == 0) || (ret == -ENOENT))
+ ret = sock_bind(socket, addr, addrlen);
+
+ if (path) {
+ write_lock(¤t->fs->lock);
+ current->fs->pwd = cur;
+ write_unlock(¤t->fs->lock);
+ }
+ out:
+ if (path)
+ path_put(&dir);
+
+ return ret;
+}
+
+static int sock_unix_fakebind(struct socket *socket,
+ struct sockaddr_un *addr,
+ unsigned long len)
+{
+ struct unix_address *uaddr;
+
+ uaddr = sock_unix_makeaddr(addr, len);
+ if (IS_ERR(uaddr))
+ return PTR_ERR(uaddr);
+
+ unix_sk(socket->sk)->addr = uaddr;
+
+ return 0;
+}
+
+static int sock_unix_bind(struct ckpt_hdr_socket *h,
+ struct ckpt_hdr_socket_unix *un,
+ struct socket *socket,
+ const char *path)
+{
+ struct sockaddr *addr = (struct sockaddr *)&un->laddr;
+ unsigned long len = un->laddr_len;
+
+ if (!un->laddr.sun_path[0])
+ return sock_bind(socket, addr, len);
+ else if (!(un->flags & CKPT_UNIX_LINKED))
+ return sock_unix_fakebind(socket, &un->laddr, len);
+ else
+ return sock_unix_chdir_and_bind(socket, path, addr, len);
+}
+
+/* Some easy pre-flight checks before we get underway */
+static int sock_unix_precheck(struct socket *socket,
+ struct ckpt_hdr_socket *h)
+{
+ struct net *net = sock_net(socket->sk);
+
+ if ((h->socket.state == SS_CONNECTING) ||
+ (h->socket.state == SS_DISCONNECTING) ||
+ (h->socket.state == SS_FREE)) {
+ ckpt_debug("AF_UNIX socket can't be SS_(DIS)CONNECTING");
+ return -EINVAL;
+ }
+
+ /* AF_UNIX overloads the backlog setting to define the maximum
+ * queue length for DGRAM sockets. Make sure we don't let the
+ * caller exceed that value on restart.
+ */
+ if ((h->sock.type == SOCK_DGRAM) &&
+ (h->sock.backlog > net->unx.sysctl_max_dgram_qlen)) {
+ ckpt_debug("DGRAM backlog of %i exceeds system max of %i\n",
+ h->sock.backlog, net->unx.sysctl_max_dgram_qlen);
+ return -EINVAL;
+ }
+
+ if (h->sock.flags & SOCK_USE_WRITE_QUEUE) {
+ ckpt_debug("AF_UNIX socket has SOCK_USE_WRITE_QUEUE set");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int sock_unix_restore(struct ckpt_ctx *ctx,
+ struct ckpt_hdr_socket *h,
+ struct socket *socket)
+{
+ struct ckpt_hdr_socket_unix *un;
+ int ret = -EINVAL;
+ char *cwd = NULL;
+
+ ret = sock_unix_precheck(socket, h);
+ if (ret)
+ return ret;
+
+ un = ckpt_read_obj_type(ctx, sizeof(*un), CKPT_HDR_SOCKET_UNIX);
+ if (IS_ERR(un))
+ return PTR_ERR(un);
+
+ if (un->peer < 0)
+ goto out;
+
+ if (sock_unix_need_cwd(&un->laddr, un->laddr_len)) {
+ cwd = ckpt_read_string(ctx, PATH_MAX);
+ if (IS_ERR(cwd)) {
+ ret = PTR_ERR(cwd);
+ goto out;
+ }
+ }
+
+ if ((h->sock.state != TCP_ESTABLISHED) &&
+ !UNIX_ADDR_EMPTY(un->laddr_len)) {
+ ret = sock_unix_bind(h, un, socket, cwd);
+ if (ret)
+ goto out;
+ }
+
+ if ((h->sock.state == TCP_ESTABLISHED) || (h->sock.state == TCP_CLOSE))
+ ret = sock_unix_restore_connected(ctx, h, un, socket);
+ else if (h->sock.state == TCP_LISTEN)
+ ret = socket->ops->listen(socket, h->sock.backlog);
+ else
+ ckpt_debug("unsupported UNIX socket state %i\n", h->sock.state);
+ out:
+ ckpt_hdr_put(ctx, un);
+ kfree(cwd);
+ return ret;
+}
+
+struct socket *do_sock_file_restore(struct ckpt_ctx *ctx,
+ struct ckpt_hdr_socket *h)
+{
+ struct socket *socket;
+ int ret;
+
+ if ((h->sock.type != SOCK_DGRAM) && (h->sock.type != SOCK_STREAM)) {
+ ckpt_debug("Socket type %i not supported", h->sock.type);
+ return ERR_PTR(-EINVAL);
+ }
+
+ ret = sock_create(h->sock_common.family, h->sock.type, 0, &socket);
+ if (ret < 0)
+ return ERR_PTR(ret);
+
+ if (h->sock_common.family == AF_UNIX) {
+ ret = sock_unix_restore(ctx, h, socket);
+ ckpt_debug("sock_unix_restore: %i\n", ret);
+ } else {
+ ckpt_debug("unsupported family %i\n", h->sock_common.family);
+ ret = -ENOSYS;
+ }
+
+ if (ret)
+ goto out;
+
+ ret = sock_cptrst(ctx, socket->sk, h, CKPT_RST);
+ out:
+ if (ret) {
+ sock_release(socket);
+ socket = ERR_PTR(ret);
+ }
+
+ return socket;
+}
+
diff --git a/net/socket.c b/net/socket.c
index 65e7698..8732fe2 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -96,6 +96,8 @@
#include <net/sock.h>
#include <linux/netfilter.h>
+#include <linux/checkpoint.h>
+
static int sock_no_open(struct inode *irrelevant, struct file *dontcare);
static ssize_t sock_aio_read(struct kiocb *iocb, const struct iovec *iov,
unsigned long nr_segs, loff_t pos);
@@ -140,6 +142,9 @@ static const struct file_operations socket_file_ops = {
.sendpage = sock_sendpage,
.splice_write = generic_splice_sendpage,
.splice_read = sock_splice_read,
+#ifdef CONFIG_CHECKPOINT
+ .checkpoint = sock_file_checkpoint,
+#endif
};
/*
@@ -415,6 +420,86 @@ int sock_map_fd(struct socket *sock, int flags)
return fd;
}
+#ifdef CONFIG_CHECKPOINT
+int sock_file_checkpoint(struct ckpt_ctx *ctx, void *ptr)
+{
+ struct ckpt_hdr_file_socket *h;
+ int ret;
+ struct file *file = ptr;
+
+ h = ckpt_hdr_get_type(ctx, sizeof(*h), CKPT_HDR_FILE);
+ if (!h)
+ return -ENOMEM;
+
+ h->common.f_type = CKPT_FILE_SOCKET;
+
+ ret = checkpoint_file_common(ctx, file, &h->common);
+ if (ret < 0)
+ goto out;
+ ret = ckpt_write_obj(ctx, (struct ckpt_hdr *) h);
+ if (ret < 0)
+ goto out;
+
+ ret = do_sock_file_checkpoint(ctx, file);
+ out:
+ ckpt_hdr_put(ctx, h);
+ return ret;
+}
+
+static struct file *sock_alloc_attach_fd(struct socket *socket)
+{
+ struct file *file;
+ int err;
+
+ file = get_empty_filp();
+ if (!file)
+ return ERR_PTR(ENOMEM);
+
+ err = sock_attach_fd(socket, file, 0);
+ if (err < 0) {
+ put_filp(file);
+ file = ERR_PTR(err);
+ }
+
+ return file;
+}
+
+void *sock_file_restore(struct ckpt_ctx *ctx)
+{
+ struct ckpt_hdr_socket *h = NULL;
+ struct socket *socket = NULL;
+ struct file *file = NULL;
+ int err;
+
+ h = ckpt_read_obj_type(ctx, sizeof(*h), CKPT_HDR_SOCKET);
+ if (IS_ERR(h))
+ return h;
+
+ socket = do_sock_file_restore(ctx, h);
+ if (IS_ERR(socket)) {
+ err = PTR_ERR(socket);
+ goto err_put;
+ }
+
+ file = sock_alloc_attach_fd(socket);
+ if (IS_ERR(file)) {
+ err = PTR_ERR(file);
+ goto err_release;
+ }
+
+ ckpt_hdr_put(ctx, h);
+
+ return file;
+
+ err_release:
+ sock_release(socket);
+ err_put:
+ ckpt_hdr_put(ctx, h);
+
+ return ERR_PTR(err);
+}
+#endif /* CONFIG_CHECKPOINT */
+
static struct socket *sock_from_file(struct file *file, int *err)
{
if (file->f_op == &socket_file_ops)
--
1.6.2.5
^ permalink raw reply related
* net/unix : possible race bug at unix_create1()
From: 홍신 shin hong @ 2009-08-12 15:00 UTC (permalink / raw)
To: netdev
Hi. I am reporting a possible race bug at unix_create1()
in net/unix/af_unix.c of Linux 2.6.30.4.
Concurrent executions of unix_create1() function
in two different threads may result race condition
when unix_nr_socks +1 == 2 * get_max_files().
It is possible that no thread can pass the if-condition
checking if two atomic_inc() operations are executed
before.
It seems that it would be better to combine two
atomic operations into one atomic_inc_and_return().
Please examine the code and let me know your opinion.
Thank you
Sincerely
Shin Hong
^ permalink raw reply
* Re: [RFC PATCH v2 2/2] selinux: Support for the new TUN LSM hooks
From: Paul Moore @ 2009-08-12 14:59 UTC (permalink / raw)
To: Eric Paris; +Cc: linux-security-module, netdev, selinux
In-Reply-To: <7e0fb38c0908111336uadf57efx7d87be7761c0e138@mail.gmail.com>
On Tuesday 11 August 2009 04:36:22 pm Eric Paris wrote:
> On Mon, Aug 10, 2009 at 1:28 PM, Paul Moore<paul.moore@hp.com> wrote:
> > Add support for the new TUN LSM hooks: security_tun_dev_create(),
> > security_tun_dev_post_create() and security_tun_dev_attach(). This
> > includes the addition of a new object class, tun_socket, which represents
> > the socks associated with TUN devices. The _tun_dev_create() and
> > _tun_dev_post_create() hooks are fairly similar to the standard socket
> > functions but _tun_dev_attach() is a bit special. The _tun_dev_attach()
> > is unique because it involves a domain attaching to an existing TUN
> > device and its associated tun_socket object, an operation which does not
> > exist with standard sockets and most closely resembles a relabel
> > operation.
>
> Looks good to me, feel free to add my Ack
Thanks, I added both acks.
--
paul moore
linux @ hp
^ permalink raw reply
* RE: [PATCH][RFC] net/bridge: add basic VEPA support
From: Fischer, Anna @ 2009-08-12 14:32 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Paul Congdon (UC Davis), 'Stephen Hemminger',
bridge@lists.linux-foundation.org, linux-kernel@vger.kernel.org,
netdev@vger.kernel.org, virtualization@lists.linux-foundation.org,
evb@yahoogroups.com, davem@davemloft.net, kaber@trash.net,
adobriyan@gmail.com, 'Eric Biederman'
In-Reply-To: <200908121519.36808.arnd@arndb.de>
> Subject: Re: [PATCH][RFC] net/bridge: add basic VEPA support
>
> On Tuesday 11 August 2009, Paul Congdon (UC Davis) wrote:
> > > >
> > > > The patch from Eric Biederman to allow macvlan to bridge between
> > > > its slave ports is at
> > > >
> > > > http://kerneltrap.org/mailarchive/linux-netdev/2009/3/9/5125774
> > >
> > > Looking through the discussions here, it does not seem as if a
> decision
> > > was made to integrate those patches, because they would make the
> > > macvlan interface behave too much like a bridge.
>
> Right, that question is still open, and dont't see it as very important
> right now, as long as we can still use it for VEPA.
Yes, for the basic VEPA this is not important. For MultiChannel VEPA, it
would be nice if a macvlan device could operate as VEPA and as a typical
VEB (VEB = traditional bridge but no learning).
Basically, what we would need to be able to support is running a VEB and
a VEPA simultaneously on the same uplink port (e.g. the physical device).
A new component (called the S-Component) would then multiplex frames
to the VEB or the VEPA based on a tagging scheme.
I could see this potentially working with macvlan, if it can operate in
both VEPA and VEB mode. But you are right that for basic VEPA, it would
not be an immediate requirement.
> > Also, is there a solution, or plans for a solution, to address
> macvtap
> > interfaces that are set to 'promiscuous' mode? It would seem fairly
> easy to
> > support this for interfaces that are simply trying to listen to the
> port
> > (e.g. Wireshark).
>
> If you want to use tcpdump or wireshark on all ports simulateously in a
> pure
> VEPA, you can still attach it to the 'lowerdev', e.g. eth0 or eth0.2
> (for macvlan
> nested in vlan).
> If we allow bridge ports, we might want to extend the local delivery
> to also go through all the hooks of the external port, so that you can
> attach packet sockets there.
I think the question here was whether there is a way for a macvlan interface
to be set to promiscuous mode. At the moment, I believe a macvlan interface
only receives packets based on its destination address (this is for unicast
packets now). What if a macvlan interface wanted to get all packets that
are being received (either on the physical device, or on a particular
VLAN if using macvlan nested in vlan). Would this work easily? Imagine
you have a virtual machine attached to that macvlan / macvtap device and
this VM wants to do packet inspection or network traffic monitoring on
all packets flowing through the virtualized server.
Anna
^ permalink raw reply
* Re: [PATCHv2 2/2] vhost_net: a kernel-level virtio server
From: Michael S. Tsirkin @ 2009-08-12 14:15 UTC (permalink / raw)
To: Paul E. McKenney
Cc: Gregory Haskins, netdev, virtualization, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org, Ingo Molnar, linux-mm,
Andrew Morton, hpa
In-Reply-To: <20090812141107.GD6833@linux.vnet.ibm.com>
On Wed, Aug 12, 2009 at 07:11:07AM -0700, Paul E. McKenney wrote:
> On Wed, Aug 12, 2009 at 04:25:40PM +0300, Michael S. Tsirkin wrote:
> > On Wed, Aug 12, 2009 at 09:01:35AM -0400, Gregory Haskins wrote:
> > > I think I understand what your comment above meant: You don't need to
> > > do synchronize_rcu() because you can flush the workqueue instead to
> > > ensure that all readers have completed.
> >
> > Yes.
> >
> > > But if thats true, to me, the
> > > rcu_dereference itself is gratuitous,
> >
> > Here's a thesis on what rcu_dereference does (besides documentation):
> >
> > reader does this
> >
> > A: sock = n->sock
> > B: use *sock
> >
> > Say writer does this:
> >
> > C: newsock = allocate socket
> > D: initialize(newsock)
> > E: n->sock = newsock
> > F: flush
> >
> >
> > On Alpha, reads could be reordered. So, on smp, command A could get
> > data from point F, and command B - from point D (uninitialized, from
> > cache). IOW, you get fresh pointer but stale data.
> > So we need to stick a barrier in there.
> >
> > > and that pointer is *not* actually
> > > RCU protected (nor does it need to be).
> >
> > Heh, if readers are lockless and writer does init/update/sync,
> > this to me spells rcu.
>
> If you are using call_rcu(), synchronize_rcu(), or one of the
> similar primitives, then you absolutely need rcu_read_lock() and
> rcu_read_unlock(), or one of the similar pairs of primitives.
Right. I don't use any of these though.
> If you -don't- use rcu_read_lock(), then you are pretty much restricted
> to adding data, but never removing it.
>
> Make sense? ;-)
>
> Thanx, Paul
Since I only access data from a workqueue, I replaced synchronize_rcu
with workqueue flush. That's why I don't need rcu_read_lock.
--
MST
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* LUCKY WINNER
From: Mrs Joan Thomas @ 2009-08-12 14:11 UTC (permalink / raw)
To: info
950.000.00 GBP has been Awarded to you in our LG Electronics,send our office your
Names:............
Address:..........
Country:..........
^ permalink raw reply
* Re: [PATCHv2 2/2] vhost_net: a kernel-level virtio server
From: Paul E. McKenney @ 2009-08-12 14:11 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Gregory Haskins, netdev, virtualization, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org, Ingo Molnar, linux-mm,
Andrew Morton, hpa
In-Reply-To: <20090812132539.GD29200@redhat.com>
On Wed, Aug 12, 2009 at 04:25:40PM +0300, Michael S. Tsirkin wrote:
> On Wed, Aug 12, 2009 at 09:01:35AM -0400, Gregory Haskins wrote:
> > I think I understand what your comment above meant: You don't need to
> > do synchronize_rcu() because you can flush the workqueue instead to
> > ensure that all readers have completed.
>
> Yes.
>
> > But if thats true, to me, the
> > rcu_dereference itself is gratuitous,
>
> Here's a thesis on what rcu_dereference does (besides documentation):
>
> reader does this
>
> A: sock = n->sock
> B: use *sock
>
> Say writer does this:
>
> C: newsock = allocate socket
> D: initialize(newsock)
> E: n->sock = newsock
> F: flush
>
>
> On Alpha, reads could be reordered. So, on smp, command A could get
> data from point F, and command B - from point D (uninitialized, from
> cache). IOW, you get fresh pointer but stale data.
> So we need to stick a barrier in there.
>
> > and that pointer is *not* actually
> > RCU protected (nor does it need to be).
>
> Heh, if readers are lockless and writer does init/update/sync,
> this to me spells rcu.
If you are using call_rcu(), synchronize_rcu(), or one of the
similar primitives, then you absolutely need rcu_read_lock() and
rcu_read_unlock(), or one of the similar pairs of primitives.
If you -don't- use rcu_read_lock(), then you are pretty much restricted
to adding data, but never removing it.
Make sense? ;-)
Thanx, Paul
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCHv2 0/2] vhost: a kernel-level virtio server
From: Michael S. Tsirkin @ 2009-08-12 14:02 UTC (permalink / raw)
To: Gregory Haskins
Cc: Arnd Bergmann, netdev, virtualization, kvm, linux-kernel, mingo,
linux-mm, Andrew Morton, hpa, Patrick Mullaney
In-Reply-To: <4A82C8F1.4030703@gmail.com>
On Wed, Aug 12, 2009 at 09:51:45AM -0400, Gregory Haskins wrote:
> Arnd Bergmann wrote:
> > On Wednesday 12 August 2009, Michael S. Tsirkin wrote:
> >>> If I understand it correctly, you can at least connect a veth pair
> >>> to a bridge, right? Something like
> >>>
> >>> veth0 - veth1 - vhost - guest 1
> >>> eth0 - br0-|
> >>> veth2 - veth3 - vhost - guest 2
> >>>
> >> Heh, you don't need a bridge in this picture:
> >>
> >> guest 1 - vhost - veth0 - veth1 - vhost guest 2
> >
> > Sure, but the setup I described is the one that I would expect
> > to see in practice because it gives you external connectivity.
> >
> > Measuring two guests communicating over a veth pair is
> > interesting for finding the bottlenecks, but of little
> > practical relevance.
> >
> > Arnd <><
>
> Yeah, this would be the config I would be interested in.
Hmm, this wouldn't be the config to use for the benchmark though: there
are just too many variables. If you want both guest to guest and guest
to host, create 2 nics in the guest.
Here's one way to do this:
-net nic,model=virtio,vlan=0 -net user,vlan=0
-net nic,vlan=1,model=virtio,vhost=veth0
-redir tcp:8022::22
-net nic,model=virtio,vlan=0 -net user,vlan=0
-net nic,vlan=1,model=virtio,vhost=veth1
-redir tcp:8023::22
In guests, for simplicity, configure eth1 and eth0
to use separate subnets.
Long term, I hope macvlan will be extended to support
guest to guest.
> Regards,
> -Greg
>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCHv2 0/2] vhost: a kernel-level virtio server
From: Gregory Haskins @ 2009-08-12 13:51 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Michael S. Tsirkin, netdev, virtualization, kvm, linux-kernel,
mingo, linux-mm, Andrew Morton, hpa, Patrick Mullaney
In-Reply-To: <200908121540.44928.arnd@arndb.de>
[-- Attachment #1: Type: text/plain, Size: 813 bytes --]
Arnd Bergmann wrote:
> On Wednesday 12 August 2009, Michael S. Tsirkin wrote:
>>> If I understand it correctly, you can at least connect a veth pair
>>> to a bridge, right? Something like
>>>
>>> veth0 - veth1 - vhost - guest 1
>>> eth0 - br0-|
>>> veth2 - veth3 - vhost - guest 2
>>>
>> Heh, you don't need a bridge in this picture:
>>
>> guest 1 - vhost - veth0 - veth1 - vhost guest 2
>
> Sure, but the setup I described is the one that I would expect
> to see in practice because it gives you external connectivity.
>
> Measuring two guests communicating over a veth pair is
> interesting for finding the bottlenecks, but of little
> practical relevance.
>
> Arnd <><
Yeah, this would be the config I would be interested in.
Regards,
-Greg
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 267 bytes --]
^ permalink raw reply
* Re: [PATCHv2 2/2] vhost_net: a kernel-level virtio server
From: Michael S. Tsirkin @ 2009-08-12 13:47 UTC (permalink / raw)
To: Gregory Haskins
Cc: netdev, virtualization, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org, Ingo Molnar, linux-mm,
Andrew Morton, hpa, paulmck
In-Reply-To: <4A82C68F.8070306@gmail.com>
On Wed, Aug 12, 2009 at 09:41:35AM -0400, Gregory Haskins wrote:
> Michael S. Tsirkin wrote:
> > On Wed, Aug 12, 2009 at 09:01:35AM -0400, Gregory Haskins wrote:
> >> I think I understand what your comment above meant: You don't need to
> >> do synchronize_rcu() because you can flush the workqueue instead to
> >> ensure that all readers have completed.
> >
> > Yes.
> >
> >> But if thats true, to me, the
> >> rcu_dereference itself is gratuitous,
> >
> > Here's a thesis on what rcu_dereference does (besides documentation):
> >
> > reader does this
> >
> > A: sock = n->sock
> > B: use *sock
> >
> > Say writer does this:
> >
> > C: newsock = allocate socket
> > D: initialize(newsock)
> > E: n->sock = newsock
> > F: flush
> >
> >
> > On Alpha, reads could be reordered. So, on smp, command A could get
> > data from point F, and command B - from point D (uninitialized, from
> > cache). IOW, you get fresh pointer but stale data.
> > So we need to stick a barrier in there.
>
> Yes, that is understood. Perhaps you should just use a normal barrier,
> however. (Or at least a comment that says "I am just using this for its
> barrier").
>
> >
> >> and that pointer is *not* actually
> >> RCU protected (nor does it need to be).
> >
> > Heh, if readers are lockless and writer does init/update/sync,
> > this to me spells rcu.
>
> More correctly: it "smells like" RCU, but its not. ;) It's rcu-like,
> but you are not really using the rcu facilities. I think anyone that
> knows RCU and reads your code will likely be scratching their heads as well.
>
> Its probably not a big deal, as I understand your code now. Just a
> suggestion to help clarify it.
>
> Regards,
> -Greg
>
OK, I'll add some comments about that.
Thanks for the review!
--
MST
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCHv2 0/2] vhost: a kernel-level virtio server
From: Michael S. Tsirkin @ 2009-08-12 13:42 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Gregory Haskins, netdev, virtualization, kvm, linux-kernel, mingo,
linux-mm, Andrew Morton, hpa, Patrick Mullaney
In-Reply-To: <200908121540.44928.arnd@arndb.de>
On Wed, Aug 12, 2009 at 03:40:44PM +0200, Arnd Bergmann wrote:
> On Wednesday 12 August 2009, Michael S. Tsirkin wrote:
> > > If I understand it correctly, you can at least connect a veth pair
> > > to a bridge, right? Something like
> > >
> > > veth0 - veth1 - vhost - guest 1
> > > eth0 - br0-|
> > > veth2 - veth3 - vhost - guest 2
> > >
> > Heh, you don't need a bridge in this picture:
> >
> > guest 1 - vhost - veth0 - veth1 - vhost guest 2
>
> Sure, but the setup I described is the one that I would expect
> to see in practice because it gives you external connectivity.
>
> Measuring two guests communicating over a veth pair is
> interesting for finding the bottlenecks, but of little
> practical relevance.
>
> Arnd <><
Oh, hopefully macvlan will soon allow that.
--
MST
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCHv2 2/2] vhost_net: a kernel-level virtio server
From: Gregory Haskins @ 2009-08-12 13:41 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: netdev, virtualization, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org, Ingo Molnar, linux-mm,
Andrew Morton, hpa, paulmck
In-Reply-To: <20090812132539.GD29200@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 1596 bytes --]
Michael S. Tsirkin wrote:
> On Wed, Aug 12, 2009 at 09:01:35AM -0400, Gregory Haskins wrote:
>> I think I understand what your comment above meant: You don't need to
>> do synchronize_rcu() because you can flush the workqueue instead to
>> ensure that all readers have completed.
>
> Yes.
>
>> But if thats true, to me, the
>> rcu_dereference itself is gratuitous,
>
> Here's a thesis on what rcu_dereference does (besides documentation):
>
> reader does this
>
> A: sock = n->sock
> B: use *sock
>
> Say writer does this:
>
> C: newsock = allocate socket
> D: initialize(newsock)
> E: n->sock = newsock
> F: flush
>
>
> On Alpha, reads could be reordered. So, on smp, command A could get
> data from point F, and command B - from point D (uninitialized, from
> cache). IOW, you get fresh pointer but stale data.
> So we need to stick a barrier in there.
Yes, that is understood. Perhaps you should just use a normal barrier,
however. (Or at least a comment that says "I am just using this for its
barrier").
>
>> and that pointer is *not* actually
>> RCU protected (nor does it need to be).
>
> Heh, if readers are lockless and writer does init/update/sync,
> this to me spells rcu.
More correctly: it "smells like" RCU, but its not. ;) It's rcu-like,
but you are not really using the rcu facilities. I think anyone that
knows RCU and reads your code will likely be scratching their heads as well.
Its probably not a big deal, as I understand your code now. Just a
suggestion to help clarify it.
Regards,
-Greg
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 267 bytes --]
^ permalink raw reply
* Re: [PATCHv2 0/2] vhost: a kernel-level virtio server
From: Arnd Bergmann @ 2009-08-12 13:40 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Gregory Haskins, netdev, virtualization, kvm, linux-kernel, mingo,
linux-mm, Andrew Morton, hpa, Patrick Mullaney
In-Reply-To: <20090812130612.GC29200@redhat.com>
On Wednesday 12 August 2009, Michael S. Tsirkin wrote:
> > If I understand it correctly, you can at least connect a veth pair
> > to a bridge, right? Something like
> >
> > veth0 - veth1 - vhost - guest 1
> > eth0 - br0-|
> > veth2 - veth3 - vhost - guest 2
> >
> Heh, you don't need a bridge in this picture:
>
> guest 1 - vhost - veth0 - veth1 - vhost guest 2
Sure, but the setup I described is the one that I would expect
to see in practice because it gives you external connectivity.
Measuring two guests communicating over a veth pair is
interesting for finding the bottlenecks, but of little
practical relevance.
Arnd <><
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCHv2 2/2] vhost_net: a kernel-level virtio server
From: Michael S. Tsirkin @ 2009-08-12 13:25 UTC (permalink / raw)
To: Gregory Haskins
Cc: netdev, virtualization, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org, Ingo Molnar, linux-mm,
Andrew Morton, hpa, paulmck
In-Reply-To: <4A82BD2F.7080405@gmail.com>
On Wed, Aug 12, 2009 at 09:01:35AM -0400, Gregory Haskins wrote:
> I think I understand what your comment above meant: You don't need to
> do synchronize_rcu() because you can flush the workqueue instead to
> ensure that all readers have completed.
Yes.
> But if thats true, to me, the
> rcu_dereference itself is gratuitous,
Here's a thesis on what rcu_dereference does (besides documentation):
reader does this
A: sock = n->sock
B: use *sock
Say writer does this:
C: newsock = allocate socket
D: initialize(newsock)
E: n->sock = newsock
F: flush
On Alpha, reads could be reordered. So, on smp, command A could get
data from point F, and command B - from point D (uninitialized, from
cache). IOW, you get fresh pointer but stale data.
So we need to stick a barrier in there.
> and that pointer is *not* actually
> RCU protected (nor does it need to be).
Heh, if readers are lockless and writer does init/update/sync,
this to me spells rcu.
--
MST
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH][RFC] net/bridge: add basic VEPA support
From: Arnd Bergmann @ 2009-08-12 13:19 UTC (permalink / raw)
To: Paul Congdon (UC Davis)
Cc: 'Fischer, Anna', 'Stephen Hemminger', bridge,
linux-kernel, netdev, virtualization, evb, davem, kaber,
adobriyan, 'Eric Biederman'
In-Reply-To: <001001ca1a93$b6e50ee0$24af2ca0$@edu>
On Tuesday 11 August 2009, Paul Congdon (UC Davis) wrote:
> > >
> > > The patch from Eric Biederman to allow macvlan to bridge between
> > > its slave ports is at
> > >
> > > http://kerneltrap.org/mailarchive/linux-netdev/2009/3/9/5125774
> >
> > Looking through the discussions here, it does not seem as if a decision
> > was made to integrate those patches, because they would make the
> > macvlan interface behave too much like a bridge.
Right, that question is still open, and dont't see it as very important
right now, as long as we can still use it for VEPA.
> > Also, it seems as if there was still a problem with doing
> > multicast/broadcast delivery when enabling local VM-to-VM
> > communication. Is that solved by now?
Not yet, but I guess it comes as a natural extension when I fix
multicast/broadcast delivery from the reflective relay for VEPA.
The logic that I would use there is:
broadcast from a dowstream port:
if (bridge_mode(source_port)) {
forward_to_upstream(frame);
for_each_downstream(port) {
/* deliver to all bridge ports except self, do
not deliver to any VEPA port. */
if (bridge_mode(port) && port != source_port) {
forward_to_downstream(frame, port);
}
}
} else {
forward_to_upstream(frame);
}
broadcast from the upstream port
if (bridge_mode(frame.source)) {
/* comes from a port in bridge mode, so has already been
delivered to all other bridge ports */
for_each_downstream(port) {
if (!bridge_mode(port)) {
forward_to_downstream(frame, port);
}
}
} else if (vepa_mode(frame.source)) {
/* comes from VEPA port, so need to deliver to all
bridge and all vepa ports except self */
for_each_downstream(port) {
if (port != frame.source)
forward_to_downstream(frame, port);
} else {
/* external source, so flood to everyone */
for_each_downstream(port) {
forward_to_downstream(frame, port);
}
For multicast, we can do the same, or optionally add a per-port filter
as you mentioned, if it becomes a bottleneck.
Do you think this addresses the problem, or did I miss something important?
> Also, is there a solution, or plans for a solution, to address macvtap
> interfaces that are set to 'promiscuous' mode? It would seem fairly easy to
> support this for interfaces that are simply trying to listen to the port
> (e.g. Wireshark).
If you want to use tcpdump or wireshark on all ports simulateously in a pure
VEPA, you can still attach it to the 'lowerdev', e.g. eth0 or eth0.2 (for macvlan
nested in vlan).
If we allow bridge ports, we might want to extend the local delivery
to also go through all the hooks of the external port, so that you can
attach packet sockets there.
> If the port was being used by something like a firewall
> then the VEPA filtering doesn't work too well.
Not sure what you mean. Are you talking about a firewall separating the guests
from the outside, between the VEPA and the reflective relay, or a firewall between
the guests in case of local delivery?
Arnd <><
^ permalink raw reply
* Re: [PATCH 1/2] mac802154: add a software MAC 802.15.4 implementation
From: Johannes Berg @ 2009-08-12 13:13 UTC (permalink / raw)
To: Dmitry Eremin-Solenikov; +Cc: netdev, linux-wireless, Sergey Lapin
In-Reply-To: <20090812130605.GA17169@doriath.ww600.siemens.net>
[-- Attachment #1: Type: text/plain, Size: 1086 bytes --]
On Wed, 2009-08-12 at 17:06 +0400, Dmitry Eremin-Solenikov wrote:
> > In addition to that, you can't put anything into skb->cb, then push the
> > frame to the master netdev, and expect things in skb->cb to still be
> > there when the frame arrives at the master netdev. Not sure you do that
> > (I hope not because that would be very buggy), but eventually you'll
> > probably find that you do want that, etc.
>
> Hmm. It works for us. Could you please tell me more about the problems
> with skb->cb ?
Uh, well, you don't own the skb->cb between dev_queue_xmit() and
ndo_start_xmit(). In fact, your phy_cb data will be overwritten by the
qdisc.
So your code is completely and utterly broken just like mac80211 was for
a long time.
Also, looking into this a bit more, I see no reason to allocate a work
struct every time you get a frame -- better just queue them up and stick
everything else into skb->cb. In fact, you don't need a master netdev
here anyway, just move all code from ieee802154_master_hard_start_xmit()
into ieee802154_net_xmit().
johannes
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox