* Re: [PATCH] xen: xenfs: privcmd: check put_user() return code
From: Jeremy Fitzhardinge @ 2010-10-29 17:52 UTC (permalink / raw)
To: Ian Campbell
Cc: Vasiliy Kulikov, kernel-janitors@vger.kernel.org,
Jeremy Fitzhardinge, Konrad Rzeszutek Wilk,
linux-kernel@vger.kernel.org, xen-devel@lists.xen.org,
virtualization@lists.osdl.org
In-Reply-To: <1288374270.8069.41.camel@localhost.localdomain>
On 10/29/2010 10:44 AM, Ian Campbell wrote:
> On Fri, 2010-10-29 at 18:18 +0100, Jeremy Fitzhardinge wrote:
>> On 10/28/2010 04:39 AM, Vasiliy Kulikov wrote:
>>> put_user() may fail. In this case propagate error code from
>>> privcmd_ioctl_mmap_batch().
>> Thanks for looking at this. I'm in two minds about this; the existing
>> logic is such that these put_users can only fail if something else has
>> already failed and its returning an error. I guess it would be useful
>> to get an EFAULT if you've got a problem writing back the results.
>>
>> IanC, any opinion?
> Not a strong one.
>
> Perhaps what we really want in this case is for traverse_pages to return
> the total number of callback failures it encountered rather than
> aborting after the first failure?
>
> On the other hand you are correct that gather_array() has already
> touched all the pages which we are going to be touching here so how
> likely is a new failure at this point anyway?
I could think of two cases: the array is mapped RO, so only the
writeback fails, or someone changes the mapping under our feet from
another thread.
J
> Ian.
>
>> Thanks,
>> J
>>
>>> Signed-off-by: Vasiliy Kulikov <segooon@gmail.com>
>>> ---
>>> Compile tested.
>>>
>>> drivers/xen/xenfs/privcmd.c | 8 ++------
>>> 1 files changed, 2 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/xen/xenfs/privcmd.c b/drivers/xen/xenfs/privcmd.c
>>> index f80be7f..2eb04c8 100644
>>> --- a/drivers/xen/xenfs/privcmd.c
>>> +++ b/drivers/xen/xenfs/privcmd.c
>>> @@ -266,9 +266,7 @@ static int mmap_return_errors(void *data, void *state)
>>> xen_pfn_t *mfnp = data;
>>> struct mmap_batch_state *st = state;
>>>
>>> - put_user(*mfnp, st->user++);
>>> -
>>> - return 0;
>>> + return put_user(*mfnp, st->user++);
>>> }
>>>
>>> static struct vm_operations_struct privcmd_vm_ops;
>>> @@ -323,10 +321,8 @@ static long privcmd_ioctl_mmap_batch(void __user *udata)
>>> up_write(&mm->mmap_sem);
>>>
>>> if (state.err > 0) {
>>> - ret = 0;
>>> -
>>> state.user = m.arr;
>>> - traverse_pages(m.num, sizeof(xen_pfn_t),
>>> + ret = traverse_pages(m.num, sizeof(xen_pfn_t),
>>> &pagelist,
>>> mmap_return_errors, &state);
>>> }
>
^ permalink raw reply
* Re: [PATCH] xen: xenfs: privcmd: check put_user() return code
From: Ian Campbell @ 2010-10-29 17:44 UTC (permalink / raw)
To: Jeremy Fitzhardinge
Cc: Vasiliy Kulikov, kernel-janitors@vger.kernel.org,
Jeremy Fitzhardinge, Konrad Rzeszutek Wilk,
linux-kernel@vger.kernel.org, xen-devel@lists.xen.org,
virtualization@lists.osdl.org
In-Reply-To: <4CCB01C9.4020007@goop.org>
On Fri, 2010-10-29 at 18:18 +0100, Jeremy Fitzhardinge wrote:
> On 10/28/2010 04:39 AM, Vasiliy Kulikov wrote:
> > put_user() may fail. In this case propagate error code from
> > privcmd_ioctl_mmap_batch().
>
> Thanks for looking at this. I'm in two minds about this; the existing
> logic is such that these put_users can only fail if something else has
> already failed and its returning an error. I guess it would be useful
> to get an EFAULT if you've got a problem writing back the results.
>
> IanC, any opinion?
Not a strong one.
Perhaps what we really want in this case is for traverse_pages to return
the total number of callback failures it encountered rather than
aborting after the first failure?
On the other hand you are correct that gather_array() has already
touched all the pages which we are going to be touching here so how
likely is a new failure at this point anyway?
Ian.
>
> Thanks,
> J
>
> > Signed-off-by: Vasiliy Kulikov <segooon@gmail.com>
> > ---
> > Compile tested.
> >
> > drivers/xen/xenfs/privcmd.c | 8 ++------
> > 1 files changed, 2 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/xen/xenfs/privcmd.c b/drivers/xen/xenfs/privcmd.c
> > index f80be7f..2eb04c8 100644
> > --- a/drivers/xen/xenfs/privcmd.c
> > +++ b/drivers/xen/xenfs/privcmd.c
> > @@ -266,9 +266,7 @@ static int mmap_return_errors(void *data, void *state)
> > xen_pfn_t *mfnp = data;
> > struct mmap_batch_state *st = state;
> >
> > - put_user(*mfnp, st->user++);
> > -
> > - return 0;
> > + return put_user(*mfnp, st->user++);
> > }
> >
> > static struct vm_operations_struct privcmd_vm_ops;
> > @@ -323,10 +321,8 @@ static long privcmd_ioctl_mmap_batch(void __user *udata)
> > up_write(&mm->mmap_sem);
> >
> > if (state.err > 0) {
> > - ret = 0;
> > -
> > state.user = m.arr;
> > - traverse_pages(m.num, sizeof(xen_pfn_t),
> > + ret = traverse_pages(m.num, sizeof(xen_pfn_t),
> > &pagelist,
> > mmap_return_errors, &state);
> > }
>
^ permalink raw reply
* Re: [PATCH] xen: xenfs: privcmd: check put_user() return code
From: Jeremy Fitzhardinge @ 2010-10-29 17:18 UTC (permalink / raw)
To: Vasiliy Kulikov
Cc: kernel-janitors, Jeremy Fitzhardinge, Ian Campbell,
Konrad Rzeszutek Wilk, linux-kernel, xen-devel, virtualization
In-Reply-To: <1288265942-12581-1-git-send-email-segooon@gmail.com>
On 10/28/2010 04:39 AM, Vasiliy Kulikov wrote:
> put_user() may fail. In this case propagate error code from
> privcmd_ioctl_mmap_batch().
Thanks for looking at this. I'm in two minds about this; the existing
logic is such that these put_users can only fail if something else has
already failed and its returning an error. I guess it would be useful
to get an EFAULT if you've got a problem writing back the results.
IanC, any opinion?
Thanks,
J
> Signed-off-by: Vasiliy Kulikov <segooon@gmail.com>
> ---
> Compile tested.
>
> drivers/xen/xenfs/privcmd.c | 8 ++------
> 1 files changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/xen/xenfs/privcmd.c b/drivers/xen/xenfs/privcmd.c
> index f80be7f..2eb04c8 100644
> --- a/drivers/xen/xenfs/privcmd.c
> +++ b/drivers/xen/xenfs/privcmd.c
> @@ -266,9 +266,7 @@ static int mmap_return_errors(void *data, void *state)
> xen_pfn_t *mfnp = data;
> struct mmap_batch_state *st = state;
>
> - put_user(*mfnp, st->user++);
> -
> - return 0;
> + return put_user(*mfnp, st->user++);
> }
>
> static struct vm_operations_struct privcmd_vm_ops;
> @@ -323,10 +321,8 @@ static long privcmd_ioctl_mmap_batch(void __user *udata)
> up_write(&mm->mmap_sem);
>
> if (state.err > 0) {
> - ret = 0;
> -
> state.user = m.arr;
> - traverse_pages(m.num, sizeof(xen_pfn_t),
> + ret = traverse_pages(m.num, sizeof(xen_pfn_t),
> &pagelist,
> mmap_return_errors, &state);
> }
^ permalink raw reply
* Re: [Qemu-devel] Re: [PATCH] Implement a virtio GPU transport
From: Anthony Liguori @ 2010-10-29 13:05 UTC (permalink / raw)
To: Rusty Russell
Cc: virtualization, QEMU Developers, virtualization, Ian Molton,
Avi Kivity, linux-kernel
In-Reply-To: <201010292148.01167.rusty@rustcorp.com.au>
On 10/29/2010 06:18 AM, Rusty Russell wrote:
>> Fixed - updated patch tested and attached.
>>
> OK. FWIW, I think this is an awesome idea.
Paravirtual OpenGL or the actual proposed implementation? Have you
looked at the actual code?
If I understand correctly, the implementation is an RPC of opengl
commands across virtio that are then rendered on the host to an
offscreen buffer. The buffer is then sent back to the guest in rendered
form.
But it's possible to mess around with other guests because the opengl
commands are not scrubbed. There have been other proposals in the past
that include a mini JIT that would translate opengl commands into a safe
form.
Exposing just an opengl RPC transport doesn't seem that useful either.
It ought to be part of a VGA card in order for it to be integrated with
the rest of the graphics system without a round trip.
The alternative proposal is Spice which so far noone has mentioned.
Right now, Spice seems to be taking the right approach to guest 3d support.
Regards,
Anthony Liguori
> I understand others are skeptical,
> but this seems simple and if it works and you're happy to maintain it I'm
> happy to let you do it :)
>
> Cheers,
> Rusty.
>
>
^ permalink raw reply
* Re: [Qemu-devel] Re: [PATCH] Implement a virtio GPU transport
From: Ed Tomlinson @ 2010-10-29 11:49 UTC (permalink / raw)
To: Rusty Russell
Cc: virtualization, Ian Molton, Avi Kivity, virtualization,
linux-kernel, QEMU Developers
In-Reply-To: <201010292148.01167.rusty@rustcorp.com.au>
On Friday 29 October 2010 07:18:00 Rusty Russell wrote:
> On Wed, 27 Oct 2010 11:30:31 pm Ian Molton wrote:
> > On 19/10/10 11:39, Avi Kivity wrote:
> > > On 10/19/2010 12:31 PM, Ian Molton wrote:
> >
> > >>> 2. should start with a patch to the virtio-pci spec to document what
> > >>> you're doing
> > >>
> > >> Where can I find that spec?
> > >
> > > http://ozlabs.org/~rusty/virtio-spec/
> >
> > Ok, but I'm not patching that until theres been some review.
>
> Fair enough; it's a bit of a PITA to patch, so it makes sense to get the
> details nailed down first.
>
> > There are links to the associated qemu and guest OS changes in my
> > original email.
> >
> > >> It doesnt, at present... It could be changed fairly easily ithout
> > >> breaking anything if that happens though.
> > >
> > > The hypervisor and the guest can be changed independently. The driver
> > > should be coded so that it doesn't depend on hypervisor implementation
> > > details.
> >
> > Fixed - updated patch tested and attached.
>
> OK. FWIW, I think this is an awesome idea. I understand others are skeptical,
> but this seems simple and if it works and you're happy to maintain it I'm
> happy to let you do it :)
From a kvm user's perspective this is a GREAT idea. Looks like Ian and friends have
though about better solutions down the line and have made their code easy
to superceed. If this is really the case I vote to get this in qemu/kvm asap -
software opengl can be a real pain.
Thanks
Ed Tomlinson
^ permalink raw reply
* Re: [Qemu-devel] Re: [PATCH] Implement a virtio GPU transport
From: Rusty Russell @ 2010-10-29 11:18 UTC (permalink / raw)
To: virtualization
Cc: Ian Molton, Avi Kivity, virtualization, linux-kernel,
QEMU Developers
In-Reply-To: <4CC8226F.5080807@collabora.co.uk>
On Wed, 27 Oct 2010 11:30:31 pm Ian Molton wrote:
> On 19/10/10 11:39, Avi Kivity wrote:
> > On 10/19/2010 12:31 PM, Ian Molton wrote:
>
> >>> 2. should start with a patch to the virtio-pci spec to document what
> >>> you're doing
> >>
> >> Where can I find that spec?
> >
> > http://ozlabs.org/~rusty/virtio-spec/
>
> Ok, but I'm not patching that until theres been some review.
Fair enough; it's a bit of a PITA to patch, so it makes sense to get the
details nailed down first.
> There are links to the associated qemu and guest OS changes in my
> original email.
>
> >> It doesnt, at present... It could be changed fairly easily ithout
> >> breaking anything if that happens though.
> >
> > The hypervisor and the guest can be changed independently. The driver
> > should be coded so that it doesn't depend on hypervisor implementation
> > details.
>
> Fixed - updated patch tested and attached.
OK. FWIW, I think this is an awesome idea. I understand others are skeptical,
but this seems simple and if it works and you're happy to maintain it I'm
happy to let you do it :)
Cheers,
Rusty.
^ permalink raw reply
* Re: [Qemu-devel] Re: [PATCH] Implement a virtio GPU transport
From: Ian Molton @ 2010-10-28 21:41 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Avi Kivity, virtualization, linux-kernel, QEMU Developers
In-Reply-To: <4CC9D9B9.10601@codemonkey.ws>
On 28/10/10 21:14, Anthony Liguori wrote:
>> If this code was invasive to qemus core, I'd say 'no way' but its just
>> not. and as the GL device is versioned, we can keep using it even if
>> the passthrough is replaced by a virtual GPU.
>
> The virtio-gl implementation is basically duplicating virtio-serial. It
> looks like ti creates a totally separate window for the GL session. In
> the current form, is there really any advantage to having the code in
> QEMU? It could just as easily live outside of QEMU.
you could say much the same about any driver in qemu... you could
serialise up the registers and ship the data off to be processed on
another PC if you wanted to...
The code does not, however, create a seperate window for the GL session.
the GL scene is rendered offscreen, and then piped back to the guest for
display, so that it is fully composited into the guests graphical
environment. From a user perspective, its as if the guest had hardware
3D. Performance is very reasonable, around 40fps in ioquake3 on modest
(host) hardware.
In theory, the code *could* use a serial transport and render in a
seperate process, but then that would make it much harder to evolve it
into a GPU-like system in future.
-Ian
^ permalink raw reply
* Re: [Qemu-devel] Re: [PATCH] Implement a virtio GPU transport
From: Anthony Liguori @ 2010-10-28 20:14 UTC (permalink / raw)
To: Ian Molton; +Cc: QEMU Developers, virtualization, Avi Kivity, linux-kernel
In-Reply-To: <4CC9D403.2010805@collabora.co.uk>
On 10/28/2010 02:50 PM, Ian Molton wrote:
> On 28/10/10 15:43, Anthony Liguori wrote:
>> On 10/28/2010 09:24 AM, Avi Kivity wrote:
>>> On 10/28/2010 01:54 PM, Ian Molton wrote:
>>
>>>> True, but then all that would prove is that I can write a spec to
>>>> match the code.
>>>
>>> It would also allow us to check that the spec matches the
>>> requirements. Those two steps are easier than checking that the code
>>> matches the requirements.
>
> There was no formal spec for this. The code was written to replace
> nasty undefined-instruction based data transport hacks in the (already
> existing) GL passthrough code.
>
>> I'm extremely sceptical of any GL passthrough proposal. There have
>> literally been half a dozen over the years and they never seem to leave
>> proof-of-concept phase. My (limited) understanding is that it's a
>> fundamentally hard problem that no one has adequately solved yet.
>
> The code in this case has been presented as a patch to qemu nearly 3
> years ago. I've taken the patches and refactored them to use virtio
> rather than an undefined instruction (which fails under KVM, unlike my
> approach).
>
> Its in use testing meego images and seems to be fairly reliable. it
> can handle compositing window managers, games, video, etc. We're
> currently supporting OpenGL 1.4 including shaders.
>
>> A specifically matters an awful lot less than an explanation of how the
>> problem is being solved in a robust fashion such that it can be reviewed
>> by people with a deeper understanding of the problem space.
>
> I'm not sure there is a way to prevent nefarious tasks upsetting the
> hosts OpenGL with carefully crafted strings of commands, short of
> inspecting every single command, which is insane.
>
> Really this needs to be done at a lower level by presenting a virtual
> GPU to the guest OS but I am not in a position to code that right now.
>
> The code as it is is useful, and can always be superceeded by a
> virtual GPU implementation in future.
>
> At least this breaks the chicken / egg cycle of people wanting GL
> support on virtual machines, but not writing stuff to take advantage
> of it because the support isn't there. its also a neatly encapsulated
> solution - if you dont want people to have access to the passthrough,
> simply tell qemu not to present the virtio-gl device to the guest, via
> qemus existing commandline options.
>
> If this code was invasive to qemus core, I'd say 'no way' but its just
> not. and as the GL device is versioned, we can keep using it even if
> the passthrough is replaced by a virtual GPU.
The virtio-gl implementation is basically duplicating virtio-serial. It
looks like ti creates a totally separate window for the GL session.
In the current form, is there really any advantage to having the code in
QEMU? It could just as easily live outside of QEMU.
Regards,
Anthony Liguori
^ permalink raw reply
* Re: [Qemu-devel] Re: [PATCH] Implement a virtio GPU transport
From: Ian Molton @ 2010-10-28 19:52 UTC (permalink / raw)
To: Avi Kivity; +Cc: virtualization, linux-kernel, QEMU Developers
In-Reply-To: <4CC98784.7020907@redhat.com>
On 28/10/10 15:24, Avi Kivity wrote:
>> The caller is intended to block as the host must perform GL rendering
>> before allowing the guests process to continue.
>
> Why is that? Can't we pipeline the process?
No, not really. the guest may call for the scene to be rendered at any
time and we have to wait for that to happen before we can return the
data to it.
-Ian
^ permalink raw reply
* Re: [Qemu-devel] Re: [PATCH] Implement a virtio GPU transport
From: Ian Molton @ 2010-10-28 19:50 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Avi Kivity, virtualization, linux-kernel, QEMU Developers
In-Reply-To: <4CC98C25.9010207@codemonkey.ws>
On 28/10/10 15:43, Anthony Liguori wrote:
> On 10/28/2010 09:24 AM, Avi Kivity wrote:
>> On 10/28/2010 01:54 PM, Ian Molton wrote:
>
>>> True, but then all that would prove is that I can write a spec to
>>> match the code.
>>
>> It would also allow us to check that the spec matches the
>> requirements. Those two steps are easier than checking that the code
>> matches the requirements.
There was no formal spec for this. The code was written to replace nasty
undefined-instruction based data transport hacks in the (already
existing) GL passthrough code.
> I'm extremely sceptical of any GL passthrough proposal. There have
> literally been half a dozen over the years and they never seem to leave
> proof-of-concept phase. My (limited) understanding is that it's a
> fundamentally hard problem that no one has adequately solved yet.
The code in this case has been presented as a patch to qemu nearly 3
years ago. I've taken the patches and refactored them to use virtio
rather than an undefined instruction (which fails under KVM, unlike my
approach).
Its in use testing meego images and seems to be fairly reliable. it can
handle compositing window managers, games, video, etc. We're currently
supporting OpenGL 1.4 including shaders.
> A specifically matters an awful lot less than an explanation of how the
> problem is being solved in a robust fashion such that it can be reviewed
> by people with a deeper understanding of the problem space.
I'm not sure there is a way to prevent nefarious tasks upsetting the
hosts OpenGL with carefully crafted strings of commands, short of
inspecting every single command, which is insane.
Really this needs to be done at a lower level by presenting a virtual
GPU to the guest OS but I am not in a position to code that right now.
The code as it is is useful, and can always be superceeded by a virtual
GPU implementation in future.
At least this breaks the chicken / egg cycle of people wanting GL
support on virtual machines, but not writing stuff to take advantage of
it because the support isn't there. its also a neatly encapsulated
solution - if you dont want people to have access to the passthrough,
simply tell qemu not to present the virtio-gl device to the guest, via
qemus existing commandline options.
If this code was invasive to qemus core, I'd say 'no way' but its just
not. and as the GL device is versioned, we can keep using it even if the
passthrough is replaced by a virtual GPU.
^ permalink raw reply
* Re: [Qemu-devel] Re: [PATCH] Implement a virtio GPU transport
From: Anthony Liguori @ 2010-10-28 14:43 UTC (permalink / raw)
To: Avi Kivity; +Cc: Ian Molton, virtualization, linux-kernel, QEMU Developers
In-Reply-To: <4CC98784.7020907@redhat.com>
On 10/28/2010 09:24 AM, Avi Kivity wrote:
> On 10/28/2010 01:54 PM, Ian Molton wrote:
>>> Well, I like to review an implementation against a spec.
>>
>>
>> True, but then all that would prove is that I can write a spec to
>> match the code.
>
> It would also allow us to check that the spec matches the
> requirements. Those two steps are easier than checking that the code
> matches the requirements.
I'm extremely sceptical of any GL passthrough proposal. There have
literally been half a dozen over the years and they never seem to leave
proof-of-concept phase. My (limited) understanding is that it's a
fundamentally hard problem that no one has adequately solved yet.
A specifically matters an awful lot less than an explanation of how the
problem is being solved in a robust fashion such that it can be reviewed
by people with a deeper understanding of the problem space.
Regards,
Anthony Liguori
>> The code is proof of concept. the kernel bit is pretty simple, but
>> I'd like to get some idea of whether the rest of the code will be
>> accepted given that theres not much point in having any one (or two)
>> of these components exist without the other.
>
> I guess some graphics people need to be involved.
>
>>
>>> Better, but still unsatisfying. If the server is busy, the caller would
>>> block. I guess it's expected since it's called from ->fsync(). I'm not
>>> sure whether that's the best interface, perhaps aio_writev is better.
>>
>> The caller is intended to block as the host must perform GL rendering
>> before allowing the guests process to continue.
>
> Why is that? Can't we pipeline the process?
>
>>
>> The only real bottleneck is that processes will block trying to
>> submit data if another process is performing rendering, but that will
>> only be solved when the renderer is made multithreaded. The same
>> would happen on a real GPU if it had only one queue too.
>>
>> If you look at the host code, you can see that the data is already
>> buffered per-process, in a pretty sensible way. if the renderer
>> itself were made a seperate thread, then this problem magically
>> disappears (the queuing code on the host is pretty fast).
>
> Well, this is out of my area of expertise. I don't like it, but if
> it's acceptable to the gpu people, okay.
>
>>
>> In testing, the overhead of this was pretty small anyway. Running a
>> few dozen glxgears and a copy of ioquake3 simultaneously on an intel
>> video card managed the same framerate with the same CPU utilisation,
>> both with the old code and the version I just posted. Contention
>> during rendering just isn't much of an issue.
>
^ permalink raw reply
* Re: [Qemu-devel] Re: [PATCH] Implement a virtio GPU transport
From: Avi Kivity @ 2010-10-28 14:24 UTC (permalink / raw)
To: Ian Molton; +Cc: linux-kernel, QEMU Developers, virtualization
In-Reply-To: <4CC9647A.50108@collabora.co.uk>
On 10/28/2010 01:54 PM, Ian Molton wrote:
>> Well, I like to review an implementation against a spec.
>
>
> True, but then all that would prove is that I can write a spec to
> match the code.
It would also allow us to check that the spec matches the requirements.
Those two steps are easier than checking that the code matches the
requirements.
> The code is proof of concept. the kernel bit is pretty simple, but I'd
> like to get some idea of whether the rest of the code will be accepted
> given that theres not much point in having any one (or two) of these
> components exist without the other.
I guess some graphics people need to be involved.
>
>> Better, but still unsatisfying. If the server is busy, the caller would
>> block. I guess it's expected since it's called from ->fsync(). I'm not
>> sure whether that's the best interface, perhaps aio_writev is better.
>
> The caller is intended to block as the host must perform GL rendering
> before allowing the guests process to continue.
Why is that? Can't we pipeline the process?
>
> The only real bottleneck is that processes will block trying to submit
> data if another process is performing rendering, but that will only be
> solved when the renderer is made multithreaded. The same would happen
> on a real GPU if it had only one queue too.
>
> If you look at the host code, you can see that the data is already
> buffered per-process, in a pretty sensible way. if the renderer itself
> were made a seperate thread, then this problem magically disappears
> (the queuing code on the host is pretty fast).
Well, this is out of my area of expertise. I don't like it, but if it's
acceptable to the gpu people, okay.
>
> In testing, the overhead of this was pretty small anyway. Running a
> few dozen glxgears and a copy of ioquake3 simultaneously on an intel
> video card managed the same framerate with the same CPU utilisation,
> both with the old code and the version I just posted. Contention
> during rendering just isn't much of an issue.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply
* Re: [Qemu-devel] Re: [PATCH] Implement a virtio GPU transport
From: Ian Molton @ 2010-10-28 11:54 UTC (permalink / raw)
To: Avi Kivity; +Cc: linux-kernel, QEMU Developers, virtualization
In-Reply-To: <4CC94203.1080207@redhat.com>
On 28/10/10 10:27, Avi Kivity wrote:
> On 10/27/2010 03:00 PM, Ian Molton wrote:
>> On 19/10/10 11:39, Avi Kivity wrote:
>>> On 10/19/2010 12:31 PM, Ian Molton wrote:
>>
>>>>> 2. should start with a patch to the virtio-pci spec to document what
>>>>> you're doing
>>>>
>>>> Where can I find that spec?
>>>
>>> http://ozlabs.org/~rusty/virtio-spec/
>>
>> Ok, but I'm not patching that until theres been some review.
>
> Well, I like to review an implementation against a spec.
True, but then all that would prove is that I can write a spec to match
the code.
The code is proof of concept. the kernel bit is pretty simple, but I'd
like to get some idea of whether the rest of the code will be accepted
given that theres not much point in having any one (or two) of these
components exist without the other.
> Better, but still unsatisfying. If the server is busy, the caller would
> block. I guess it's expected since it's called from ->fsync(). I'm not
> sure whether that's the best interface, perhaps aio_writev is better.
The caller is intended to block as the host must perform GL rendering
before allowing the guests process to continue.
The only real bottleneck is that processes will block trying to submit
data if another process is performing rendering, but that will only be
solved when the renderer is made multithreaded. The same would happen on
a real GPU if it had only one queue too.
If you look at the host code, you can see that the data is already
buffered per-process, in a pretty sensible way. if the renderer itself
were made a seperate thread, then this problem magically disappears (the
queuing code on the host is pretty fast).
In testing, the overhead of this was pretty small anyway. Running a few
dozen glxgears and a copy of ioquake3 simultaneously on an intel video
card managed the same framerate with the same CPU utilisation, both with
the old code and the version I just posted. Contention during rendering
just isn't much of an issue.
-Ian
^ permalink raw reply
* [PATCH] xen: xenfs: privcmd: check put_user() return code
From: Vasiliy Kulikov @ 2010-10-28 11:39 UTC (permalink / raw)
To: kernel-janitors
Cc: Jeremy Fitzhardinge, Konrad Rzeszutek Wilk, Ian Campbell,
xen-devel, virtualization, linux-kernel
put_user() may fail. In this case propagate error code from
privcmd_ioctl_mmap_batch().
Signed-off-by: Vasiliy Kulikov <segooon@gmail.com>
---
Compile tested.
drivers/xen/xenfs/privcmd.c | 8 ++------
1 files changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/xen/xenfs/privcmd.c b/drivers/xen/xenfs/privcmd.c
index f80be7f..2eb04c8 100644
--- a/drivers/xen/xenfs/privcmd.c
+++ b/drivers/xen/xenfs/privcmd.c
@@ -266,9 +266,7 @@ static int mmap_return_errors(void *data, void *state)
xen_pfn_t *mfnp = data;
struct mmap_batch_state *st = state;
- put_user(*mfnp, st->user++);
-
- return 0;
+ return put_user(*mfnp, st->user++);
}
static struct vm_operations_struct privcmd_vm_ops;
@@ -323,10 +321,8 @@ static long privcmd_ioctl_mmap_batch(void __user *udata)
up_write(&mm->mmap_sem);
if (state.err > 0) {
- ret = 0;
-
state.user = m.arr;
- traverse_pages(m.num, sizeof(xen_pfn_t),
+ ret = traverse_pages(m.num, sizeof(xen_pfn_t),
&pagelist,
mmap_return_errors, &state);
}
--
1.7.0.4
^ permalink raw reply related
* Re: [Qemu-devel] Re: [PATCH] Implement a virtio GPU transport
From: Avi Kivity @ 2010-10-28 9:27 UTC (permalink / raw)
To: Ian Molton; +Cc: linux-kernel, QEMU Developers, virtualization
In-Reply-To: <4CC8226F.5080807@collabora.co.uk>
On 10/27/2010 03:00 PM, Ian Molton wrote:
> On 19/10/10 11:39, Avi Kivity wrote:
>> On 10/19/2010 12:31 PM, Ian Molton wrote:
>
>>>> 2. should start with a patch to the virtio-pci spec to document what
>>>> you're doing
>>>
>>> Where can I find that spec?
>>
>> http://ozlabs.org/~rusty/virtio-spec/
>
> Ok, but I'm not patching that until theres been some review.
Well, I like to review an implementation against a spec.
>
> There are links to the associated qemu and guest OS changes in my
> original email.
>
>>> It doesnt, at present... It could be changed fairly easily ithout
>>> breaking anything if that happens though.
>>
>> The hypervisor and the guest can be changed independently. The driver
>> should be coded so that it doesn't depend on hypervisor implementation
>> details.
>
> Fixed - updated patch tested and attached.
> +
> + /* Transfer data */
> + if (virtqueue_add_buf(vq, sg_list, o_page, i_page, gldata)>= 0) {
> + virtqueue_kick(vq);
> + /* Chill out until it's done with the buffer. */
> + wait_for_completion(&gldata->done);
> + }
> +
Better, but still unsatisfying. If the server is busy, the caller would
block. I guess it's expected since it's called from ->fsync(). I'm not
sure whether that's the best interface, perhaps aio_writev is better.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply
* Re: [Xen-devel] Re: [PATCH] x86/pvclock-xen: zero last_value on resume
From: H. Peter Anvin @ 2010-10-27 20:59 UTC (permalink / raw)
To: Jeremy Fitzhardinge, Glauber Costa
Cc: Xen-devel@lists.xensource.com, kvm-devel, Eelco Dolstra,
the arch/x86 maintainers, Linux Kernel Mailing List,
Linux Virtualization, Olivier Hanesse, Avi Kivity
In-Reply-To: <4CC8751E.40306@goop.org>
I'll check it this evening when I'm at a working network again :(
"Jeremy Fitzhardinge" <jeremy@goop.org> wrote:
> On 10/26/2010 10:48 AM, Glauber Costa wrote:
>> On Tue, 2010-10-26 at 09:59 -0700, Jeremy Fitzhardinge wrote:
>>> If the guest domain has been suspend/resumed or migrated, then the
>>> system clock backing the pvclock clocksource may revert to a smaller
>>> value (ie, can be non-monotonic across the migration/save-restore).
>>> Make sure we zero last_value in that case so that the domain
>>> continues to see clock updates.
>>>
>>> [ I don't know if kvm needs an analogous fix or not. ]
>> After migration, save/restore, etc, we issue an ioctl where we tell
>> the host the last clock value. That (in theory) guarantees
>monotonicity.
>>
>> I am not opposed to this patch in any way, however.
>
>Thanks.
>
>HPA, do you want to take this, or shall I send it on?
>
>Thanks,
> J
--
Sent from my mobile phone. Please pardon any lack of formatting.
^ permalink raw reply
* Re: [Xen-devel] Re: [PATCH] x86/pvclock-xen: zero last_value on resume
From: Jeremy Fitzhardinge @ 2010-10-27 18:53 UTC (permalink / raw)
To: Glauber Costa
Cc: Xen-devel@lists.xensource.com, kvm-devel, Eelco Dolstra,
the arch/x86 maintainers, Linux Kernel Mailing List,
Linux Virtualization, Olivier Hanesse, Avi Kivity, H. Peter Anvin
In-Reply-To: <1288115334.3530.7.camel@mothafucka.localdomain>
On 10/26/2010 10:48 AM, Glauber Costa wrote:
> On Tue, 2010-10-26 at 09:59 -0700, Jeremy Fitzhardinge wrote:
>> If the guest domain has been suspend/resumed or migrated, then the
>> system clock backing the pvclock clocksource may revert to a smaller
>> value (ie, can be non-monotonic across the migration/save-restore).
>> Make sure we zero last_value in that case so that the domain
>> continues to see clock updates.
>>
>> [ I don't know if kvm needs an analogous fix or not. ]
> After migration, save/restore, etc, we issue an ioctl where we tell
> the host the last clock value. That (in theory) guarantees monotonicity.
>
> I am not opposed to this patch in any way, however.
Thanks.
HPA, do you want to take this, or shall I send it on?
Thanks,
J
^ permalink raw reply
* Re: [Qemu-devel] Re: [PATCH] Implement a virtio GPU transport
From: Ian Molton @ 2010-10-27 13:00 UTC (permalink / raw)
To: Avi Kivity; +Cc: linux-kernel, QEMU Developers, virtualization
In-Reply-To: <4CBD7560.6080207@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 717 bytes --]
On 19/10/10 11:39, Avi Kivity wrote:
> On 10/19/2010 12:31 PM, Ian Molton wrote:
>>> 2. should start with a patch to the virtio-pci spec to document what
>>> you're doing
>>
>> Where can I find that spec?
>
> http://ozlabs.org/~rusty/virtio-spec/
Ok, but I'm not patching that until theres been some review.
There are links to the associated qemu and guest OS changes in my
original email.
>> It doesnt, at present... It could be changed fairly easily ithout
>> breaking anything if that happens though.
>
> The hypervisor and the guest can be changed independently. The driver
> should be coded so that it doesn't depend on hypervisor implementation
> details.
Fixed - updated patch tested and attached.
-Ian
[-- Attachment #2: virtio_gl.patch --]
[-- Type: text/x-patch, Size: 8972 bytes --]
diff --git a/drivers/gpu/Makefile b/drivers/gpu/Makefile
index 30879df..35699a0 100644
--- a/drivers/gpu/Makefile
+++ b/drivers/gpu/Makefile
@@ -1 +1 @@
-obj-y += drm/ vga/
+obj-y += drm/ vga/ misc/
diff --git a/drivers/gpu/misc/Kconfig b/drivers/gpu/misc/Kconfig
new file mode 100644
index 0000000..50043d3
--- /dev/null
+++ b/drivers/gpu/misc/Kconfig
@@ -0,0 +1,8 @@
+config VIRTIOGL
+ tristate "Virtio userspace memory transport"
+ depends on VIRTIO_PCI
+ default n
+ help
+ A Driver to facilitate transferring data from userspace to a
+ hypervisor (eg. qemu)
+
diff --git a/drivers/gpu/misc/Makefile b/drivers/gpu/misc/Makefile
new file mode 100644
index 0000000..d9ab333
--- /dev/null
+++ b/drivers/gpu/misc/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_VIRTIOGL) += virtio-gl.o
diff --git a/drivers/gpu/misc/virtio-gl.c b/drivers/gpu/misc/virtio-gl.c
new file mode 100644
index 0000000..ad3ba14
--- /dev/null
+++ b/drivers/gpu/misc/virtio-gl.c
@@ -0,0 +1,325 @@
+/*
+ * Copyright (C) 2010 Intel Corporation
+ *
+ * Author: Ian Molton <ian.molton@collabora.co.uk>
+ *
+ * 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; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/fs.h>
+#include <linux/dma-mapping.h>
+#include <linux/sched.h>
+#include <linux/slab.h>
+#include <linux/miscdevice.h>
+#include <linux/virtio.h>
+#include <linux/virtio_ids.h>
+#include <linux/virtio_config.h>
+
+#define DEVICE_NAME "glmem"
+
+/* Define to use debugging checksums on transfers */
+#undef DEBUG_GLIO
+
+struct virtio_gl_data {
+ char *buffer;
+ int pages;
+ unsigned int pid;
+ struct completion done;
+};
+
+struct virtio_gl_header {
+ int pid;
+ int buf_size;
+ int r_buf_size;
+#ifdef DEBUG_GLIO
+ int sum;
+#endif
+ char buffer;
+} __packed;
+
+#define to_virtio_gl_data(a) ((struct virtio_gl_data *)(a)->private_data)
+
+#ifdef DEBUG_GLIO
+#define SIZE_OUT_HEADER (sizeof(int)*4)
+#define SIZE_IN_HEADER (sizeof(int)*2)
+#else
+#define SIZE_OUT_HEADER (sizeof(int)*3)
+#define SIZE_IN_HEADER sizeof(int)
+#endif
+
+static struct virtqueue *vq;
+
+static void glmem_done(struct virtqueue *vq)
+{
+ struct virtio_gl_data *gldata;
+ int count;
+
+ gldata = virtqueue_get_buf(vq, &count);
+
+ if (!gldata)
+ BUG();
+
+ complete(&gldata->done);
+}
+
+/* This is videobuf_vmalloc_to_sg() from videobuf-dma-sg.c with
+ * some modifications
+ */
+static struct scatterlist *vmalloc_to_sg(struct scatterlist *sg_list,
+ unsigned char *virt, unsigned int pages)
+{
+ struct page *pg;
+
+ /* unaligned */
+ BUG_ON((ulong)virt & ~PAGE_MASK);
+
+ /* Fill with elements for the data */
+ while (pages) {
+ pg = vmalloc_to_page(virt);
+ if (!pg)
+ goto err;
+
+ sg_set_page(sg_list, pg, PAGE_SIZE, 0);
+ virt += PAGE_SIZE;
+ sg_list++;
+ pages--;
+ }
+
+ return sg_list;
+
+err:
+ kfree(sg_list);
+ return NULL;
+}
+
+static int put_data(struct virtio_gl_data *gldata)
+{
+ struct scatterlist *sg, *sg_list;
+ unsigned int ret, o_page, i_page, sg_entries;
+ struct virtio_gl_header *header =
+ (struct virtio_gl_header *)gldata->buffer;
+
+ ret = header->buf_size;
+
+ o_page = (header->buf_size + PAGE_SIZE-1) >> PAGE_SHIFT;
+ i_page = (header->r_buf_size + PAGE_SIZE-1) >> PAGE_SHIFT;
+
+ header->pid = gldata->pid;
+
+ if ((o_page && i_page) &&
+ (o_page > gldata->pages || i_page > gldata->pages)) {
+ i_page = 0;
+ }
+
+ if (o_page > gldata->pages)
+ o_page = gldata->pages;
+
+ if (i_page > gldata->pages)
+ i_page = gldata->pages;
+
+ if (!o_page)
+ o_page = 1;
+
+ sg_entries = o_page + i_page;
+
+ sg_list = kcalloc(sg_entries, sizeof(struct scatterlist), GFP_KERNEL);
+
+ if (!sg_list) {
+ ret = -EIO;
+ goto out;
+ }
+
+ sg_init_table(sg_list, sg_entries);
+
+ sg = vmalloc_to_sg(sg_list, gldata->buffer, o_page);
+ sg = vmalloc_to_sg(sg, gldata->buffer, i_page);
+
+ if (!sg) {
+ ret = -EIO;
+ goto out_free;
+ }
+
+ /* Transfer data */
+ if (virtqueue_add_buf(vq, sg_list, o_page, i_page, gldata) >= 0) {
+ virtqueue_kick(vq);
+ /* Chill out until it's done with the buffer. */
+ wait_for_completion(&gldata->done);
+ }
+
+out_free:
+ kfree(sg_list);
+out:
+ return ret;
+}
+
+static void free_buffer(struct virtio_gl_data *gldata)
+{
+ if (gldata->buffer) {
+ vfree(gldata->buffer);
+ gldata->buffer = NULL;
+ }
+}
+
+static int glmem_open(struct inode *inode, struct file *file)
+{
+ struct virtio_gl_data *gldata = kzalloc(sizeof(struct virtio_gl_data),
+ GFP_KERNEL);
+
+ if (!gldata)
+ return -ENXIO;
+
+ gldata->pid = pid_nr(task_pid(current));
+ init_completion(&gldata->done);
+
+ file->private_data = gldata;
+
+ return 0;
+}
+
+static int glmem_mmap(struct file *filp, struct vm_area_struct *vma)
+{
+ struct virtio_gl_data *gldata = to_virtio_gl_data(filp);
+ int pages = (vma->vm_end - vma->vm_start) / PAGE_SIZE;
+
+ /* Set a reasonable limit */
+ if (pages > 16)
+ return -ENOMEM;
+
+ /* for now, just allow one buffer to be mmap()ed. */
+ if (gldata->buffer)
+ return -EIO;
+
+ gldata->buffer = vmalloc_user(pages*PAGE_SIZE);
+
+ if (!gldata->buffer)
+ return -ENOMEM;
+
+ gldata->pages = pages;
+
+ if (remap_vmalloc_range(vma, gldata->buffer, 0) < 0) {
+ vfree(gldata->buffer);
+ return -EIO;
+ }
+
+ vma->vm_flags |= VM_DONTEXPAND;
+
+ return 0;
+}
+
+static int glmem_fsync(struct file *filp, int datasync)
+{
+ struct virtio_gl_data *gldata = to_virtio_gl_data(filp);
+
+ put_data(gldata);
+
+ return 0;
+}
+
+static int glmem_release(struct inode *inode, struct file *file)
+{
+ struct virtio_gl_data *gldata = to_virtio_gl_data(file);
+
+ if (gldata && gldata->buffer) {
+ struct virtio_gl_header *header =
+ (struct virtio_gl_header *)gldata->buffer;
+
+ /* Make sure the host hears about the process ending / dying */
+ header->pid = gldata->pid;
+ header->buf_size = SIZE_OUT_HEADER + 2;
+ header->r_buf_size = SIZE_IN_HEADER;
+ *(short *)(&header->buffer) = -1;
+
+ put_data(gldata);
+ free_buffer(gldata);
+ }
+
+ kfree(gldata);
+
+ return 0;
+}
+
+static const struct file_operations glmem_fops = {
+ .owner = THIS_MODULE,
+ .open = glmem_open,
+ .mmap = glmem_mmap,
+ .fsync = glmem_fsync,
+ .release = glmem_release,
+};
+
+static struct miscdevice glmem_dev = {
+ MISC_DYNAMIC_MINOR,
+ DEVICE_NAME,
+ &glmem_fops
+};
+
+static int glmem_probe(struct virtio_device *vdev)
+{
+ int ret;
+
+ /* We expect a single virtqueue. */
+ vq = virtio_find_single_vq(vdev, glmem_done, "output");
+ if (IS_ERR(vq))
+ return PTR_ERR(vq);
+
+ ret = misc_register(&glmem_dev);
+ if (ret) {
+ printk(KERN_ERR "glmem: cannot register glmem_dev as misc");
+ return -ENODEV;
+ }
+
+ return 0;
+}
+
+static void __devexit glmem_remove(struct virtio_device *vdev)
+{
+ vdev->config->reset(vdev);
+ misc_deregister(&glmem_dev);
+ vdev->config->del_vqs(vdev);
+}
+
+static struct virtio_device_id id_table[] = {
+ { VIRTIO_ID_GL, VIRTIO_DEV_ANY_ID },
+ { 0 },
+};
+
+static struct virtio_driver virtio_gl_driver = {
+ .driver = {
+ .name = KBUILD_MODNAME,
+ .owner = THIS_MODULE,
+ },
+ .id_table = id_table,
+ .probe = glmem_probe,
+ .remove = __devexit_p(glmem_remove),
+};
+
+static int __init glmem_init(void)
+{
+ return register_virtio_driver(&virtio_gl_driver);
+}
+
+static void __exit glmem_exit(void)
+{
+ unregister_virtio_driver(&virtio_gl_driver);
+}
+
+module_init(glmem_init);
+module_exit(glmem_exit);
+
+MODULE_DEVICE_TABLE(virtio, id_table);
+MODULE_DESCRIPTION("Virtio gl passthrough driver");
+MODULE_LICENSE("GPL v2");
+
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 3d94a14..9a9a6cc 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -16,6 +16,7 @@ source "drivers/char/agp/Kconfig"
source "drivers/gpu/vga/Kconfig"
source "drivers/gpu/drm/Kconfig"
+source "drivers/gpu/misc/Kconfig"
config VGASTATE
tristate
diff --git a/include/linux/virtio_ids.h b/include/linux/virtio_ids.h
index 06660c0..663b496 100644
--- a/include/linux/virtio_ids.h
+++ b/include/linux/virtio_ids.h
@@ -12,6 +12,7 @@
#define VIRTIO_ID_CONSOLE 3 /* virtio console */
#define VIRTIO_ID_RNG 4 /* virtio ring */
#define VIRTIO_ID_BALLOON 5 /* virtio balloon */
+#define VIRTIO_ID_GL 6 /* virtio usermem */
#define VIRTIO_ID_9P 9 /* 9p virtio console */
#endif /* _LINUX_VIRTIO_IDS_H */
^ permalink raw reply related
* Re: [PATCH] x86/pvclock-xen: zero last_value on resume
From: Glauber Costa @ 2010-10-26 17:48 UTC (permalink / raw)
To: Jeremy Fitzhardinge
Cc: Xen-devel@lists.xensource.com, kvm-devel, Eelco Dolstra,
the arch/x86 maintainers, Linux Kernel Mailing List,
Linux Virtualization, Olivier Hanesse, Avi Kivity, H. Peter Anvin
In-Reply-To: <4CC708DE.1070000@goop.org>
On Tue, 2010-10-26 at 09:59 -0700, Jeremy Fitzhardinge wrote:
>
> If the guest domain has been suspend/resumed or migrated, then the
> system clock backing the pvclock clocksource may revert to a smaller
> value (ie, can be non-monotonic across the migration/save-restore).
> Make sure we zero last_value in that case so that the domain
> continues to see clock updates.
>
> [ I don't know if kvm needs an analogous fix or not. ]
After migration, save/restore, etc, we issue an ioctl where we tell
the host the last clock value. That (in theory) guarantees monotonicity.
I am not opposed to this patch in any way, however.
^ permalink raw reply
* [PATCH] x86/pvclock-xen: zero last_value on resume
From: Jeremy Fitzhardinge @ 2010-10-26 16:59 UTC (permalink / raw)
To: H. Peter Anvin
Cc: Xen-devel@lists.xensource.com, kvm-devel, Glauber Costa,
the arch/x86 maintainers, Linux Kernel Mailing List,
Linux Virtualization, Olivier Hanesse, Avi Kivity, Eelco Dolstra
[-- Attachment #1: Type: text/plain, Size: 1958 bytes --]
If the guest domain has been suspend/resumed or migrated, then the
system clock backing the pvclock clocksource may revert to a smaller
value (ie, can be non-monotonic across the migration/save-restore).
Make sure we zero last_value in that case so that the domain
continues to see clock updates.
[ I don't know if kvm needs an analogous fix or not. ]
Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
Cc: Stable Kernel <stable@kernel.org>
Reported-by: Eelco Dolstra <e.dolstra@tudelft.nl>
Reported-by: Olivier Hanesse <olivier.hanesse@gmail.com>
Bisected-by: Cédric Schieli <cschieli@gmail.com>
Tested-by: Cédric Schieli <cschieli@gmail.com>
diff --git a/arch/x86/include/asm/pvclock.h b/arch/x86/include/asm/pvclock.h
index cd02f32..6226870 100644
--- a/arch/x86/include/asm/pvclock.h
+++ b/arch/x86/include/asm/pvclock.h
@@ -11,5 +11,6 @@ unsigned long pvclock_tsc_khz(struct pvclock_vcpu_time_info *src);
void pvclock_read_wallclock(struct pvclock_wall_clock *wall,
struct pvclock_vcpu_time_info *vcpu,
struct timespec *ts);
+void pvclock_resume(void);
#endif /* _ASM_X86_PVCLOCK_H */
diff --git a/arch/x86/kernel/pvclock.c b/arch/x86/kernel/pvclock.c
index 239427c..a4f07c1 100644
--- a/arch/x86/kernel/pvclock.c
+++ b/arch/x86/kernel/pvclock.c
@@ -120,6 +120,11 @@ unsigned long pvclock_tsc_khz(struct pvclock_vcpu_time_info *src)
static atomic64_t last_value = ATOMIC64_INIT(0);
+void pvclock_resume(void)
+{
+ atomic64_set(&last_value, 0);
+}
+
cycle_t pvclock_clocksource_read(struct pvclock_vcpu_time_info *src)
{
struct pvclock_shadow_time shadow;
diff --git a/arch/x86/xen/time.c b/arch/x86/xen/time.c
index b2bb5aa..5da5e53 100644
--- a/arch/x86/xen/time.c
+++ b/arch/x86/xen/time.c
@@ -426,6 +426,8 @@ void xen_timer_resume(void)
{
int cpu;
+ pvclock_resume();
+
if (xen_clockevent != &xen_vcpuop_clockevent)
return;
[-- Attachment #2: x86-pvclock-resume.patch --]
[-- Type: text/plain, Size: 2064 bytes --]
From 29acbb4e1d93e719250648db1ce8c7a24144fd86 Mon Sep 17 00:00:00 2001
From: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
Date: Mon, 25 Oct 2010 16:53:46 -0700
Subject: [PATCH] x86/pvclock: zero last_value on resume
If the guest domain has been suspend/resumed or migrated, then the
system clock backing the pvclock clocksource may revert to a smaller
value (ie, can be non-monotonic across the migration/save-restore).
Make sure we zero last_value in that case so that the domain
continues to see clock updates.
Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@citrix.com>
Cc: Stable Kernel <stable@kernel.org>
Reported-by: Eelco Dolstra <e.dolstra@tudelft.nl>
Reported-by: Olivier Hanesse <olivier.hanesse@gmail.com>
Bisected-by: Cédric Schieli <cschieli@gmail.com>
Tested-by: Cédric Schieli <cschieli@gmail.com>
diff --git a/arch/x86/include/asm/pvclock.h b/arch/x86/include/asm/pvclock.h
index cd02f32..6226870 100644
--- a/arch/x86/include/asm/pvclock.h
+++ b/arch/x86/include/asm/pvclock.h
@@ -11,5 +11,6 @@ unsigned long pvclock_tsc_khz(struct pvclock_vcpu_time_info *src);
void pvclock_read_wallclock(struct pvclock_wall_clock *wall,
struct pvclock_vcpu_time_info *vcpu,
struct timespec *ts);
+void pvclock_resume(void);
#endif /* _ASM_X86_PVCLOCK_H */
diff --git a/arch/x86/kernel/pvclock.c b/arch/x86/kernel/pvclock.c
index 239427c..a4f07c1 100644
--- a/arch/x86/kernel/pvclock.c
+++ b/arch/x86/kernel/pvclock.c
@@ -120,6 +120,11 @@ unsigned long pvclock_tsc_khz(struct pvclock_vcpu_time_info *src)
static atomic64_t last_value = ATOMIC64_INIT(0);
+void pvclock_resume(void)
+{
+ atomic64_set(&last_value, 0);
+}
+
cycle_t pvclock_clocksource_read(struct pvclock_vcpu_time_info *src)
{
struct pvclock_shadow_time shadow;
diff --git a/arch/x86/xen/time.c b/arch/x86/xen/time.c
index b2bb5aa..5da5e53 100644
--- a/arch/x86/xen/time.c
+++ b/arch/x86/xen/time.c
@@ -426,6 +426,8 @@ void xen_timer_resume(void)
{
int cpu;
+ pvclock_resume();
+
if (xen_clockevent != &xen_vcpuop_clockevent)
return;
[-- Attachment #3: Type: text/plain, Size: 184 bytes --]
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization
^ permalink raw reply related
* Re: [PATCH 3/14] drivers/vhost/vhost.c: delete double assignment
From: Michael S. Tsirkin @ 2010-10-26 11:06 UTC (permalink / raw)
To: Julia Lawall; +Cc: virtualization, kernel-janitors, linux-kernel, kvm, netdev
In-Reply-To: <1288088743-3725-4-git-send-email-julia@diku.dk>
On Tue, Oct 26, 2010 at 12:25:32PM +0200, Julia Lawall wrote:
> From: Julia Lawall <julia@diku.dk>
>
> Delete successive assignments to the same location.
>
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
>
> // <smpl>
> @@
> expression i;
> @@
>
> *i = ...;
> i = ...;
> // </smpl>
>
> Signed-off-by: Julia Lawall <julia@diku.dk>
Thanks, applied.
> ---
> drivers/vhost/vhost.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index 94701ff..ed27727 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -157,7 +157,6 @@ static void vhost_vq_reset(struct vhost_dev *dev,
> vq->avail_idx = 0;
> vq->last_used_idx = 0;
> vq->used_flags = 0;
> - vq->used_flags = 0;
> vq->log_used = false;
> vq->log_addr = -1ull;
> vq->vhost_hlen = 0;
^ permalink raw reply
* [PATCH 3/14] drivers/vhost/vhost.c: delete double assignment
From: Julia Lawall @ 2010-10-26 10:25 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: virtualization, kernel-janitors, linux-kernel, kvm, netdev
In-Reply-To: <1288088743-3725-1-git-send-email-julia@diku.dk>
From: Julia Lawall <julia@diku.dk>
Delete successive assignments to the same location.
A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)
// <smpl>
@@
expression i;
@@
*i = ...;
i = ...;
// </smpl>
Signed-off-by: Julia Lawall <julia@diku.dk>
---
drivers/vhost/vhost.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 94701ff..ed27727 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -157,7 +157,6 @@ static void vhost_vq_reset(struct vhost_dev *dev,
vq->avail_idx = 0;
vq->last_used_idx = 0;
vq->used_flags = 0;
- vq->used_flags = 0;
vq->log_used = false;
vq->log_addr = -1ull;
vq->vhost_hlen = 0;
^ permalink raw reply related
* [PATCH 9/9] do not use macv[tap/lan] interfaces as ports
From: Jens Osterkamp @ 2010-10-22 11:27 UTC (permalink / raw)
To: e1000-eedc, virtualization, evb; +Cc: chrisw, Jens Osterkamp
In-Reply-To: <1287746820-6036-1-git-send-email-jens@linux.vnet.ibm.com>
At startup lldpad walks through all network interfaces in the system and
creates internal data structures for them. Some interfaces as e.g. vlan
and wlan are skipped in the walkthrough, some have to be treated special
(e.g. bond devices).
This patch adds macvtap and macvlan interfaces to the list of devices that
are skipped as we do not want to send out EVB/ECP frames on them.
Signed-off-by: Jens Osterkamp <jens@linux.vnet.ibm.com>
---
config.c | 2 +
include/lldp_util.h | 1 +
lldp_util.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 93 insertions(+), 0 deletions(-)
diff --git a/config.c b/config.c
index 26afe3b..fdb63f7 100644
--- a/config.c
+++ b/config.c
@@ -365,6 +365,8 @@ void init_ports(void)
p->if_name);
} else if (is_vlan(p->if_name)) {
;
+ } else if (is_macvtap(p->if_name)) {
+ ;
} else if (is_bridge(p->if_name)) {
; /* ignore bridge device */
} else if (check_link_status(p->if_name)) {
diff --git a/include/lldp_util.h b/include/lldp_util.h
index 3353067..ef2c562 100644
--- a/include/lldp_util.h
+++ b/include/lldp_util.h
@@ -40,6 +40,7 @@ int is_bridge(const char *ifname);
int is_vlan(const char *ifname);
int is_vlan_capable(const char *ifname);
int is_wlan(const char *ifname);
+int is_macvtap(const char *ifname);
int is_valid_mac(const u8 *mac);
int is_san_mac(u8 *addr);
int is_ether(const char *ifname);
diff --git a/lldp_util.c b/lldp_util.c
index f39fe6b..40db741 100644
--- a/lldp_util.c
+++ b/lldp_util.c
@@ -34,6 +34,7 @@
#include <sys/types.h>
#include <sys/socket.h>
#include <net/if_arp.h>
+#include <netlink/msg.h>
#include <arpa/inet.h>
#include <linux/if.h>
#include <linux/if_bonding.h>
@@ -42,6 +43,7 @@
#include <linux/wireless.h>
#include <linux/sockios.h>
#include <linux/ethtool.h>
+#include <linux/rtnetlink.h>
#include <dirent.h>
#include "lldp.h"
#include "lldp_util.h"
@@ -57,6 +59,8 @@ int is_valid_lldp_device(const char *device_name)
return 0;
if (is_bridge(device_name))
return 0;
+ if (is_macvtap(device_name))
+ return 0;
return 1;
}
@@ -534,6 +538,92 @@ int is_wlan(const char *ifname)
return rc;
}
+#define NLMSG_SIZE 1024
+
+static struct nla_policy ifla_info_policy[IFLA_INFO_MAX + 1] =
+{
+ [IFLA_INFO_KIND] = { .type = NLA_STRING},
+ [IFLA_INFO_DATA] = { .type = NLA_NESTED },
+};
+
+int is_macvtap(const char *ifname)
+{
+ int ret, s;
+ struct nlmsghdr *nlh;
+ struct ifinfomsg *ifinfo;
+ struct nlattr *tb[IFLA_MAX+1],
+ *tb2[IFLA_INFO_MAX+1];
+
+ s = socket(PF_NETLINK, SOCK_DGRAM, NETLINK_ROUTE);
+
+ if (s < 0) {
+ goto out;
+ }
+
+ nlh = malloc(NLMSG_SIZE);
+ memset(nlh, 0, NLMSG_SIZE);
+
+ if (!nlh) {
+ goto out;
+ }
+
+ nlh->nlmsg_len = NLMSG_LENGTH(sizeof(struct ifinfomsg));
+ nlh->nlmsg_type = RTM_GETLINK;
+ nlh->nlmsg_flags = NLM_F_REQUEST;
+
+ ifinfo = NLMSG_DATA(nlh);
+ ifinfo->ifi_family = AF_UNSPEC;
+ ifinfo->ifi_index = get_ifidx(ifname);
+
+ ret = send(s, nlh, nlh->nlmsg_len, 0);
+
+ if (ret < 0) {
+ goto out_free;
+ }
+
+ memset(nlh, 0, NLMSG_SIZE);
+
+ do {
+ ret = recv(s, (void *) nlh, NLMSG_SIZE, MSG_DONTWAIT);
+ } while ((ret < 0) && errno == EINTR);
+
+ if (nlmsg_parse(nlh, sizeof(struct ifinfomsg),
+ (struct nlattr **)&tb, IFLA_MAX, NULL)) {
+ goto out_free;
+ }
+
+ if (tb[IFLA_IFNAME]) {
+ ifname = (char *)RTA_DATA(tb[IFLA_IFNAME]);
+ } else {
+ ifinfo = (struct ifinfomsg *)NLMSG_DATA(nlh);
+ }
+
+ if (tb[IFLA_LINKINFO]) {
+ if (nla_parse_nested(tb2, IFLA_INFO_MAX, tb[IFLA_LINKINFO],
+ ifla_info_policy)) {
+ goto out_free;
+ }
+
+ if (tb2[IFLA_INFO_KIND]) {
+ char *kind = (char*)(RTA_DATA(tb2[IFLA_INFO_KIND]));
+ if (!(strcmp("macvtap", kind) && strcmp("macvlan", kind))) {
+ free(nlh);
+ close(s);
+ return true;
+ }
+ }
+
+ } else {
+ goto out_free;
+ }
+
+out_free:
+ free(nlh);
+out:
+ close(s);
+ return false;
+}
+
int is_router(const char *ifname)
{
int rc = 0;
--
1.7.1
^ permalink raw reply related
* [PATCH 8/9] lldpad support for libvirt netlink message
From: Jens Osterkamp @ 2010-10-22 11:26 UTC (permalink / raw)
To: e1000-eedc, virtualization, evb
Cc: chrisw, Gerhard Stenzel, Stefan Berger, Jens Osterkamp
In-Reply-To: <1287746820-6036-1-git-send-email-jens@linux.vnet.ibm.com>
This code receives a IEEE 802.1Qbg virtual station instance from libvirt in
a SETLINK message. The parsed VSI is then handed over to VDP for processing.
The VDP state machine processes the VSI while libvirt polls the result using
GETLINK.
Requires at least Linux kernel 2.6.35-rc1.
Signed-off-by: Jens Osterkamp <jens@linux.vnet.ibm.com>
Signed-off-by: Gerhard Stenzel <gstenzel@linux.vnet.ibm.com>
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
---
event_iface.c | 596 ++++++++++++++++++++++++++++++++++++++++++++++++-
include/event_iface.h | 1 +
include/lldp_vdp.h | 2 +
lldp_vdp.c | 2 +-
lldpad.c | 10 +
5 files changed, 600 insertions(+), 11 deletions(-)
diff --git a/event_iface.c b/event_iface.c
index 439e4d0..e43c187 100644
--- a/event_iface.c
+++ b/event_iface.c
@@ -3,6 +3,13 @@
LLDP Agent Daemon (LLDPAD) Software
Copyright(c) 2007-2010 Intel Corporation.
+ implementation of libvirt netlink interface
+ (c) Copyright IBM Corp. 2010
+
+ Author(s): Jens Osterkamp <jens@linux.vnet.ibm.com>
+ Stefan Berger <stefanb@linux.vnet.ibm.com>
+ Gerhard Stenzel <gstenzel@linux.vnet.ibm.com>
+
This program is free software; you can redistribute it and/or modify it
under the terms and conditions of the GNU General Public License,
version 2, as published by the Free Software Foundation.
@@ -31,12 +38,16 @@
#include <sys/socket.h>
#include <linux/rtnetlink.h>
#include <linux/if.h>
+#include <linux/if_link.h>
#include <linux/if_vlan.h>
#include <linux/sockios.h>
#include <syslog.h>
#include <unistd.h>
+#include <linux/netlink.h>
+#include <netlink/msg.h>
#include "lldpad.h"
#include "lldp_mod.h"
+#include "lldp_vdp.h"
#include "common.h"
#include "eloop.h"
#include "drv_cfg.h"
@@ -47,6 +58,7 @@
#include "lldp/l2_packet.h"
#include "config.h"
#include "lldp/states.h"
+#include "messages.h"
static void event_if_decode_rta(int type, struct rtattr *rta);
@@ -55,6 +67,32 @@ static void event_if_decode_rta(int type, struct rtattr *rta);
static char *device_name = NULL;
static int link_status = 0;
+static struct nla_policy ifla_vf_policy[IFLA_VF_MAX + 1] =
+{
+ [IFLA_VF_MAC] = { .minlen = sizeof(struct ifla_vf_mac),
+ .maxlen = sizeof(struct ifla_vf_mac)},
+ [IFLA_VF_VLAN] = { .minlen = sizeof(struct ifla_vf_vlan),
+ .maxlen = sizeof(struct ifla_vf_vlan)},
+};
+
+static struct nla_policy ifla_vf_ports_policy[IFLA_VF_PORT_MAX + 1] =
+{
+ [IFLA_VF_PORT] = { .type = NLA_NESTED },
+};
+
+static struct nla_policy ifla_port_policy[IFLA_PORT_MAX + 1] =
+{
+ [IFLA_PORT_VF] = { .type = NLA_U32 },
+ [IFLA_PORT_PROFILE] = { .type = NLA_STRING },
+ [IFLA_PORT_VSI_TYPE] = { .minlen = sizeof(struct ifla_port_vsi) },
+ [IFLA_PORT_INSTANCE_UUID] = { .minlen = PORT_UUID_MAX,
+ .maxlen = PORT_UUID_MAX, },
+ [IFLA_PORT_HOST_UUID] = { .minlen = PORT_UUID_MAX,
+ .maxlen = PORT_UUID_MAX, },
+ [IFLA_PORT_REQUEST] = { .type = NLA_U8 },
+ [IFLA_PORT_RESPONSE] = { .type = NLA_U16 },
+};
+
static void event_if_decode_rta(int type, struct rtattr *rta)
{
@@ -220,15 +258,513 @@ static void event_if_decode_nlmsg(int route_type, void *data, int len)
}
}
-
static void event_if_process_recvmsg(struct nlmsghdr *nlmsg)
{
-
- /* print out details */
+ LLDPAD_DBG("%s:%s: nlmsg_type: %d", __FILE__, __FUNCTION__, nlmsg->nlmsg_type);
event_if_decode_nlmsg(nlmsg->nlmsg_type, NLMSG_DATA(nlmsg),
NLMSG_PAYLOAD(nlmsg, 0));
}
+static int event_if_parse_getmsg(struct nlmsghdr *nlh, int *ifindex,
+ char *ifname)
+{
+ struct nlattr *tb[IFLA_MAX+1];
+ struct ifinfomsg *ifinfo;
+
+ if (nlmsg_parse(nlh, sizeof(struct ifinfomsg),
+ (struct nlattr **)&tb, IFLA_MAX, NULL)) {
+ LLDPAD_ERR("Error parsing GETLINK request...");
+ return -EINVAL;
+ }
+
+ if (tb[IFLA_IFNAME]) {
+ ifname = (char *)RTA_DATA(tb[IFLA_IFNAME]);
+ LLDPAD_DBG("IFLA_IFNAME=%s", ifname);
+ } else {
+ ifinfo = (struct ifinfomsg *)NLMSG_DATA(nlh);
+ *ifindex = ifinfo->ifi_index;
+ LLDPAD_DBG("interface index: %d", ifinfo->ifi_index);
+ }
+
+ return 0;
+}
+
+static int event_if_parse_setmsg(struct nlmsghdr *nlh)
+{
+ struct nlattr *tb[IFLA_MAX+1],
+ *tb3[IFLA_PORT_MAX+1],
+ *tb_vfinfo[IFLA_VF_MAX+1],
+ *tb_vfinfo_list;
+ struct vsi_profile *profile, *p;
+ struct ifinfomsg *ifinfo;
+ char *ifname;
+ int rem;
+
+ profile = malloc(sizeof(struct vsi_profile));
+ if (!profile)
+ return -ENOMEM;
+ memset(profile, 0, sizeof(struct vsi_profile));
+
+ if (nlmsg_parse(nlh, sizeof(struct ifinfomsg),
+ (struct nlattr **)&tb, IFLA_MAX, NULL)) {
+ LLDPAD_ERR("Error parsing request...\n");
+ return -EINVAL;
+ }
+
+ LLDPAD_DBG("%s(%d): nlmsg_len %i", __FILE__, __LINE__, nlh->nlmsg_len);
+
+ if (tb[IFLA_IFNAME]) {
+ ifname = (char *)RTA_DATA(tb[IFLA_IFNAME]);
+ LLDPAD_DBG("IFLA_IFNAME=%s", ifname);
+ } else {
+ ifinfo = (struct ifinfomsg *)NLMSG_DATA(nlh);
+ LLDPAD_DBG("interface index: %d", ifinfo->ifi_index);
+ }
+
+ if (!tb[IFLA_VFINFO_LIST]) {
+ LLDPAD_ERR("IFLA_VFINFO_LIST missing.");
+ return -EINVAL;
+ } else {
+ LLDPAD_DBG("FOUND IFLA_VFINFO_LIST!");
+ }
+
+ nla_for_each_nested(tb_vfinfo_list, tb[IFLA_VFINFO_LIST], rem) {
+ if (nla_type(tb_vfinfo_list) != IFLA_VF_INFO) {
+ LLDPAD_ERR("nested parsing of IFLA_VFINFO_LIST failed.");
+ return -EINVAL;
+ }
+
+ if (nla_parse_nested(tb_vfinfo, IFLA_VF_MAX, tb_vfinfo_list,
+ ifla_vf_policy)) {
+ LLDPAD_ERR("nested parsing of IFLA_VF_INFO failed.");
+ return -EINVAL;
+ }
+ }
+
+ if (tb_vfinfo[IFLA_VF_MAC]) {
+ struct ifla_vf_mac *mac = RTA_DATA(tb_vfinfo[IFLA_VF_MAC]);
+ u8 *m = mac->mac;
+ LLDPAD_DBG("IFLA_VF_MAC=%2x:%2x:%2x:%2x:%2x:%2x",
+ m[0], m[1], m[2], m[3], m[4], m[5]);
+ memcpy(&profile->mac, m, ETH_ALEN);
+ }
+
+ if (tb_vfinfo[IFLA_VF_VLAN]) {
+ struct ifla_vf_vlan *vlan = RTA_DATA(tb_vfinfo[IFLA_VF_VLAN]);
+ LLDPAD_DBG("IFLA_VF_VLAN=%d", vlan->vlan);
+ profile->vlan = (u16) vlan->vlan;
+ }
+
+ if (tb[IFLA_VF_PORTS]) {
+ struct nlattr *tb_vf_ports;
+
+ LLDPAD_DBG("FOUND IFLA_VF_PORTS");
+
+ nla_for_each_nested(tb_vf_ports, tb[IFLA_VF_PORTS], rem) {
+
+ LLDPAD_DBG("ITERATING");
+
+ if (nla_type(tb_vf_ports) != IFLA_VF_PORT) {
+ LLDPAD_DBG("not a IFLA_VF_PORT. skipping");
+ continue;
+ }
+
+ if (nla_parse_nested(tb3, IFLA_PORT_MAX, tb_vf_ports,
+ ifla_port_policy)) {
+ LLDPAD_ERR("nested parsing on level 2 failed.");
+ return -EINVAL;
+ }
+
+ if (tb3[IFLA_PORT_VF]) {
+ LLDPAD_DBG("IFLA_PORT_VF=%d", *(uint32_t*)(RTA_DATA(tb3[IFLA_PORT_VF])));
+ }
+
+ if (tb3[IFLA_PORT_PROFILE]) {
+ LLDPAD_DBG("IFLA_PORT_PROFILE=%s", (char *)RTA_DATA(tb3[IFLA_PORT_PROFILE]));
+ }
+
+ if (tb3[IFLA_PORT_VSI_TYPE]) {
+ struct ifla_port_vsi *pvsi;
+ int tid = 0;
+
+ pvsi = (struct ifla_port_vsi*)RTA_DATA(tb3[IFLA_PORT_VSI_TYPE]);
+ tid = pvsi->vsi_type_id[2] << 16 |
+ pvsi->vsi_type_id[1] << 8 |
+ pvsi->vsi_type_id[0];
+
+ LLDPAD_DBG("mgr_id : %d", pvsi->vsi_mgr_id);
+ LLDPAD_DBG("type_id : %d", tid);
+ LLDPAD_DBG("type_version : %d", pvsi->vsi_type_version);
+
+ profile->mgrid = pvsi->vsi_mgr_id;
+ profile->id = tid;
+ profile->version = pvsi->vsi_type_version;
+ }
+
+ if (tb3[IFLA_PORT_INSTANCE_UUID]) {
+ int i;
+ unsigned char *uuid;
+ uuid = (unsigned char *)RTA_DATA(tb3[IFLA_PORT_INSTANCE_UUID]);
+
+ char instance[INSTANCE_STRLEN+2];
+ instance2str(uuid, instance, sizeof(instance));
+ LLDPAD_DBG("IFLA_PORT_INSTANCE_UUID=%s", &instance[0]);
+
+ memcpy(&profile->instance,
+ RTA_DATA(tb3[IFLA_PORT_INSTANCE_UUID]), 16);
+ }
+
+ if (tb3[IFLA_PORT_REQUEST]) {
+ LLDPAD_DBG("IFLA_PORT_REQUEST=%d",
+ *(uint8_t*)RTA_DATA(tb3[IFLA_PORT_REQUEST]));
+ profile->mode = *(uint8_t*)RTA_DATA(tb3[IFLA_PORT_REQUEST]);
+ }
+
+ if (tb3[IFLA_PORT_RESPONSE]) {
+ LLDPAD_DBG("IFLA_PORT_RESPONSE=%d",
+ *(uint16_t*)RTA_DATA(tb3[IFLA_PORT_RESPONSE]));
+ profile->response = *(uint16_t*)RTA_DATA(tb3[IFLA_PORT_RESPONSE]);
+ }
+ }
+ }
+
+ if (ifname) {
+ struct port *port = port_find_by_name(ifname);
+
+ if (port) {
+ profile->port = port;
+ } else {
+ LLDPAD_ERR("%s(%i): Could not find port for %s", __func__,
+ __LINE__, ifname);
+ return -EEXIST;
+ }
+ }
+
+ p = vdp_add_profile(profile);
+
+ if (!p) {
+ free(profile);
+ return -EINVAL;
+ }
+
+ vdp_somethingChangedLocal(profile, VDP_PROFILE_REQ);
+ vdp_vsi_sm_station(p);
+
+ return 0;
+}
+
+static void event_if_parseResponseMsg(struct nlmsghdr *nlh)
+{
+ struct nlattr *tb[IFLA_MAX+1],
+ *tb2[IFLA_VF_PORT_MAX + 1],
+ *tb3[IFLA_PORT_MAX+1];
+
+ if (nlmsg_parse(nlh, sizeof(struct ifinfomsg),
+ (struct nlattr **)&tb, IFLA_MAX, NULL)) {
+ LLDPAD_ERR("Error parsing netlink response...");
+ return;
+ }
+
+ if (tb[IFLA_IFNAME]) {
+ LLDPAD_DBG("IFLA_IFNAME=%s", (char *)RTA_DATA(tb[IFLA_IFNAME]));
+ } else {
+ struct ifinfomsg *ifinfo = (struct ifinfomsg *)NLMSG_DATA(nlh);
+ LLDPAD_DBG("interface index: %d", ifinfo->ifi_index);
+ }
+
+ if (tb[IFLA_VF_PORTS]) {
+ if (nla_parse_nested(tb2, IFLA_VF_PORT_MAX, tb[IFLA_VF_PORTS],
+ ifla_vf_ports_policy)) {
+ LLDPAD_ERR("nested parsing on level 1 failed.");
+ return;
+ }
+
+ if (tb2[IFLA_VF_PORT]) {
+ if (nla_parse_nested(tb3, IFLA_PORT_MAX, tb2[IFLA_VF_PORT],
+ ifla_port_policy)) {
+ LLDPAD_ERR("nested parsing on level 2 failed.");
+ return;
+ }
+
+ if (tb3[IFLA_PORT_VF]) {
+ LLDPAD_DBG("IFLA_PORT_VF=%d", *(uint32_t*)(RTA_DATA(tb3[IFLA_PORT_VF])));
+ }
+
+ if (tb3[IFLA_PORT_PROFILE]) {
+ LLDPAD_DBG("IFLA_PORT_PROFILE=%s", (char *)RTA_DATA(tb3[IFLA_PORT_PROFILE]));
+ }
+
+ if (tb3[IFLA_PORT_VSI_TYPE]) {
+ struct ifla_port_vsi *pvsi;
+ int tid = 0;
+ pvsi = (struct ifla_port_vsi*)RTA_DATA(tb3[IFLA_PORT_VSI_TYPE]);
+ tid = pvsi->vsi_type_id[2] << 16 |
+ pvsi->vsi_type_id[1] << 8 |
+ pvsi->vsi_type_id[0];
+ LLDPAD_DBG("mgr_id : %d"
+ "type_id : %d"
+ "type_version : %d",
+ pvsi->vsi_mgr_id,
+ tid,
+ pvsi->vsi_type_version);
+ }
+
+ if (tb3[IFLA_PORT_INSTANCE_UUID]) {
+ int i;
+ unsigned char *uuid;
+ uuid = (unsigned char *)RTA_DATA(tb3[IFLA_PORT_INSTANCE_UUID]);
+
+ char instance[INSTANCE_STRLEN+2];
+ instance2str(uuid, instance, sizeof(instance));
+ LLDPAD_DBG("IFLA_PORT_INSTANCE_UUID=%s", &instance[0]);
+ }
+
+ if (tb3[IFLA_PORT_REQUEST]) {
+ LLDPAD_DBG("IFLA_PORT_REQUEST=%d",
+ *(uint8_t*)RTA_DATA(tb3[IFLA_PORT_REQUEST]));
+ }
+
+ if (tb3[IFLA_PORT_RESPONSE]) {
+ LLDPAD_DBG("IFLA_PORT_RESPONSE=%d",
+ *(uint16_t*)RTA_DATA(tb3[IFLA_PORT_RESPONSE]));
+ }
+ }
+ }
+}
+
+struct nl_msg *event_if_constructResponse(struct nlmsghdr *nlh, int ifindex)
+{
+ struct nl_msg *nl_msg;
+ struct nlattr *vf_ports = NULL, *vf_port;
+ struct ifinfomsg ifinfo;
+ struct vdp_data *vd;
+ uint32_t pid = nlh->nlmsg_pid;
+ uint32_t seq = nlh->nlmsg_seq;
+ char *ifname = malloc(IFNAMSIZ);
+ struct vsi_profile *p;
+
+ nl_msg = nlmsg_alloc();
+
+ if (!nl_msg) {
+ LLDPAD_ERR("%s(%i): Unable to allocate netlink message !", __func__, __LINE__);
+ return NULL;
+ }
+
+ if (!if_indextoname(ifindex, ifname)) {
+ LLDPAD_ERR("%s(%i): No name found for interface with index %i !", __func__, __LINE__,
+ ifindex);
+ }
+
+ vd = vdp_data(ifname);
+ if (!vd) {
+ LLDPAD_ERR("%s(%i): Could not find vdp_data for %s !", __func__, __LINE__,
+ ifname);
+ return NULL;
+ }
+
+ if (nlmsg_put(nl_msg, pid, seq, NLMSG_DONE, 0, 0) == NULL)
+ goto err_exit;
+
+ ifinfo.ifi_index = ifindex;
+
+ if (nlmsg_append(nl_msg, &ifinfo, sizeof(ifinfo), NLMSG_ALIGNTO) < 0)
+ goto err_exit;
+
+ vf_ports = nla_nest_start(nl_msg, IFLA_VF_PORTS);
+
+ if (!vf_ports)
+ goto err_exit;
+
+ /* loop over all existing profiles on this interface and
+ * put them into the nested IFLA_VF_PORT structure */
+ LIST_FOREACH(p, &vd->profile_head, profile) {
+ if (p) {
+ vdp_print_profile(p);
+
+ vf_port = nla_nest_start(nl_msg, IFLA_VF_PORT);
+
+ if (!vf_port)
+ goto err_exit;
+
+ if (nla_put(nl_msg, IFLA_PORT_INSTANCE_UUID, 16, p->instance) < 0)
+ goto err_exit;
+
+ if (nla_put_u32(nl_msg, IFLA_PORT_VF, PORT_SELF_VF) < 0)
+ goto err_exit;
+
+ if (p->mode == p->state) {
+ if (nla_put_u16(nl_msg, IFLA_PORT_RESPONSE,
+ PORT_VDP_RESPONSE_SUCCESS) < 0)
+ goto err_exit;
+ } else {
+ if (p->state == VSI_EXIT) {
+ if (p->response > PORT_VDP_RESPONSE_SUCCESS) {
+ if (nla_put_u16(nl_msg, IFLA_PORT_RESPONSE, p->response) < 0)
+ goto err_exit;
+ } else {
+ if (nla_put_u16(nl_msg, IFLA_PORT_RESPONSE,
+ PORT_VDP_RESPONSE_INSUFFICIENT_RESOURCES) < 0)
+ goto err_exit;
+ }
+ }
+ }
+
+ nla_nest_end(nl_msg, vf_port);
+ }
+ }
+
+ if (vf_ports)
+ nla_nest_end(nl_msg, vf_ports);
+
+ return nl_msg;
+
+err_exit:
+ nlmsg_free(nl_msg);
+
+ return NULL;
+}
+
+struct nl_msg *event_if_simpleResponse(uint32_t pid, uint32_t seq, int err)
+{
+ struct nl_msg *nl_msg = nlmsg_alloc();
+ struct nlmsgerr nlmsgerr;
+
+ memset(&nlmsgerr, 0x0, sizeof(nlmsgerr));
+
+ nlmsgerr.error = err;
+ LLDPAD_DBG("RESPONSE error code: %d",err);
+
+ if (nlmsg_put(nl_msg, pid, seq, NLMSG_ERROR, 0, 0) == NULL)
+ goto err_exit;
+
+ if (nlmsg_append(nl_msg, &nlmsgerr, sizeof(nlmsgerr), NLMSG_ALIGNTO) < 0)
+ goto err_exit;
+
+ return nl_msg;
+
+err_exit:
+ nlmsg_free(nl_msg);
+
+ return NULL;
+}
+
+static void event_iface_receive_user_space(int sock, void *eloop_ctx, void *sock_ctx)
+{
+ struct nlmsghdr *nlh;
+ struct nl_msg *nl_msg;
+ struct msghdr msg;
+ struct sockaddr_nl dest_addr;
+ struct iovec iov;
+ int result;
+ int err;
+ int ifindex = 0;
+ char *ifname = NULL;
+
+ nlh = (struct nlmsghdr *)calloc(1,
+ NLMSG_SPACE(MAX_PAYLOAD));
+ if (!nlh) {
+ LLDPAD_ERR("%s(%i): could not allocate nlh !", __func__,
+ __LINE__);
+ return;
+ }
+ memset(nlh, 0, NLMSG_SPACE(MAX_PAYLOAD));
+
+ memset(&dest_addr, 0, sizeof(dest_addr));
+ iov.iov_base = (void *)nlh;
+ iov.iov_len = NLMSG_SPACE(MAX_PAYLOAD);
+ msg.msg_name = (void *)&dest_addr;
+ msg.msg_namelen = sizeof(dest_addr);
+ msg.msg_iov = &iov;
+ msg.msg_iovlen = 1;
+ msg.msg_control = NULL;
+ msg.msg_controllen = 0;
+ msg.msg_flags = 0;
+
+ LLDPAD_DBG("Waiting for message");
+ result = recvmsg(sock, &msg, MSG_DONTWAIT);
+
+ LLDPAD_DBG("%s(%i): ", __func__, __LINE__);
+ LLDPAD_DBG("recvmsg received %d bytes", result);
+
+ if(result < 0) {
+ LLDPAD_ERR("Error receiving from netlink socket : %s", strerror(errno));
+ }
+
+ LLDPAD_DBG("dest_addr.nl_pid: %d", dest_addr.nl_pid);
+ LLDPAD_DBG("nlh.nl_pid: %d", nlh->nlmsg_pid);
+ LLDPAD_DBG("nlh_type: %d", nlh->nlmsg_type);
+ LLDPAD_DBG("nlh_seq: 0x%x", nlh->nlmsg_seq);
+ LLDPAD_DBG("nlh_len: 0x%x", nlh->nlmsg_len);
+
+ switch (nlh->nlmsg_type) {
+ case RTM_SETLINK:
+ LLDPAD_DBG("RTM_SETLINK");
+
+ err = event_if_parse_setmsg(nlh);
+
+ /* send simple response wether profile was accepted
+ * or not */
+ nl_msg = event_if_simpleResponse(nlh->nlmsg_pid,
+ nlh->nlmsg_seq,
+ err);
+ nlh = nlmsg_hdr(nl_msg);
+ break;
+ case RTM_GETLINK:
+ LLDPAD_DBG("RTM_GETLINK");
+
+ err = event_if_parse_getmsg(nlh, &ifindex, ifname);
+
+ if (err) {
+ nl_msg = event_if_simpleResponse(nlh->nlmsg_pid,
+ nlh->nlmsg_seq,
+ err);
+ } else if (ifname) {
+ ifindex = if_nametoindex(ifname);
+ LLDPAD_DBG("%s(%i): ifname %s (%d)", __func__,
+ __LINE__, ifname, ifindex);
+ } else {
+ ifname = malloc(IFNAMSIZ);
+ LLDPAD_DBG("%s(%i): ifindex %i", __func__,
+ __LINE__, ifindex);
+ }
+
+ nl_msg = event_if_constructResponse(nlh, ifindex);
+
+ if (!nl_msg) {
+ LLDPAD_ERR("%s(%i): Unable to construct response !",
+ __func__, __LINE__);
+ goto out_err;
+ }
+
+ nlh = nlmsg_hdr(nl_msg);
+
+ LLDPAD_DBG("RESPONSE:");
+
+ event_if_parseResponseMsg(nlh);
+
+ break;
+ }
+
+ iov.iov_base = (void*)nlh;
+ iov.iov_len = nlh->nlmsg_len;
+
+ msg.msg_name = (void *)&dest_addr;
+ msg.msg_namelen = sizeof(dest_addr);
+ msg.msg_iov = &iov;
+ msg.msg_iovlen = 1;
+
+ result = sendmsg(sock, &msg, 0);
+
+ if (result < 0) {
+ LLDPAD_ERR("Error sending on netlink socket (%s) !", strerror(errno));
+ } else {
+ LLDPAD_DBG("Sent %d bytes !",result);
+ }
+
+out_err:
+ free(nlh);
+}
+
static void event_iface_receive(int sock, void *eloop_ctx, void *sock_ctx)
{
struct nlmsghdr *nlh;
@@ -236,12 +772,14 @@ static void event_iface_receive(int sock, void *eloop_ctx, void *sock_ctx)
char buf[MAX_MSG_SIZE];
socklen_t fromlen = sizeof(dest_addr);
int result;
-
+
result = recvfrom(sock, buf, sizeof(buf), MSG_DONTWAIT,
(struct sockaddr *) &dest_addr, &fromlen);
+ LLDPAD_DBG("%s:%s: result from receive: %d", __FILE__, __FUNCTION__, result);
+
if (result < 0) {
- perror("recvfrom(Event interface)");
+ LLDPAD_ERR("recvfrom(event interface): %s", strerror(errno));
if ((errno == ENOBUFS) || (errno == EAGAIN))
eloop_register_timeout(INI_TIMER, 0, scan_port,
NULL, NULL);
@@ -250,10 +788,15 @@ static void event_iface_receive(int sock, void *eloop_ctx, void *sock_ctx)
TRACE("PRINT BUF info.\n")
- device_name = NULL;
- link_status = IF_OPER_UNKNOWN;
- nlh = (struct nlmsghdr *)buf;
- event_if_process_recvmsg(nlh);
+ /* Separate handler for kernel messages from userspace messages*/
+ LLDPAD_DBG("%s:%s: dest_addr.nl_pid: %d", __FILE__, __FUNCTION__, dest_addr.nl_pid);
+
+ if (dest_addr.nl_pid == 0) {
+ device_name = NULL;
+ link_status = IF_OPER_UNKNOWN;
+ nlh = (struct nlmsghdr *)buf;
+ event_if_process_recvmsg(nlh);
+ }
}
int event_iface_init()
@@ -261,7 +804,7 @@ int event_iface_init()
int fd;
int rcv_size = MAX_MSG_SIZE;
struct sockaddr_nl snl;
-
+ fprintf(stderr, "%s:%s:%d\n", __FILE__, __FUNCTION__, __LINE__);
fd = socket(PF_NETLINK, SOCK_RAW, NETLINK_ROUTE);
if (fd < 0)
@@ -282,6 +825,39 @@ int event_iface_init()
}
eloop_register_read_sock(fd, event_iface_receive, NULL, NULL);
+
+ return 0;
+}
+
+int event_iface_init_user_space()
+{
+ int fd;
+ int rcv_size = MAX_MSG_SIZE;
+ struct sockaddr_nl snl;
+
+ fd = socket(PF_NETLINK, SOCK_RAW, NETLINK_ROUTE);
+
+ if (fd < 0)
+ return fd;
+
+ if (setsockopt(fd, SOL_SOCKET, SO_RCVBUF, &rcv_size, sizeof(int)) < 0) {
+ close(fd);
+ return -EIO;
+ }
+
+ memset((void *)&snl, 0, sizeof(struct sockaddr_nl));
+ snl.nl_family = AF_NETLINK;
+ snl.nl_pid = getpid(); /* self pid */
+ snl.nl_groups = 0;
+
+ if (bind(fd, (struct sockaddr *)&snl, sizeof(struct sockaddr_nl)) < 0) {
+ close(fd);
+ LLDPAD_ERR("Error binding to netlink socket (%s) !", strerror(errno));
+ return -EIO;
+ }
+
+ eloop_register_read_sock(fd, event_iface_receive_user_space, NULL, NULL);
+
return 0;
}
diff --git a/include/event_iface.h b/include/event_iface.h
index b2c93f0..d80158d 100644
--- a/include/event_iface.h
+++ b/include/event_iface.h
@@ -29,6 +29,7 @@
#define _EVENT_IFACE_H_
int event_iface_init(void);
+int event_iface_init_user_space(void);
int event_iface_deinit(void);
int oper_add_device(char *device_name);
diff --git a/include/lldp_vdp.h b/include/lldp_vdp.h
index b97d8c0..53f814c 100644
--- a/include/lldp_vdp.h
+++ b/include/lldp_vdp.h
@@ -134,6 +134,8 @@ struct vdp_data *vdp_data(char *ifname);
struct packed_tlv *vdp_gettlv(struct vdp_data *vd, struct vsi_profile *profile);
void vdp_vsi_sm_station(struct vsi_profile *profile);
struct vsi_profile *vdp_add_profile(struct vsi_profile *profile);
+void vdp_somethingChangedLocal(struct vsi_profile *profile, int mode);
+void vdp_print_profile(struct vsi_profile *profile);
#define MAC_ADDR_STRLEN 18
#define INSTANCE_STRLEN 32
diff --git a/lldp_vdp.c b/lldp_vdp.c
index d66be6e..abd56d2 100644
--- a/lldp_vdp.c
+++ b/lldp_vdp.c
@@ -105,7 +105,7 @@ static void vdp_free_data(struct vdp_user_data *ud)
* prints the contents of a profile first to a string using the PRINT_PROFILE
* macro, and then to the screen. Used for debug purposes.
*/
-static inline void vdp_print_profile(struct vsi_profile *profile)
+void vdp_print_profile(struct vsi_profile *profile)
{
LLDPAD_DBG("profile:\n");
diff --git a/lldpad.c b/lldpad.c
index c0938af..69faa29 100644
--- a/lldpad.c
+++ b/lldpad.c
@@ -374,6 +374,16 @@ int main(int argc, char *argv[])
exit(1);
}
+ /* setup event netlink interface for user space processes.
+ * This needs to be setup first to ensure it gets lldpads
+ * pid as netlink address.
+ */
+ if (event_iface_init_user_space() < 0) {
+ log_message(MSG_ERR_SERVICE_START_FAILURE,
+ "%s", "failed to register user space event interface");
+ exit(1);
+ }
+
init_modules("");
--
1.7.1
^ permalink raw reply related
* [PATCH 7/9] use connect instead of bind
From: Jens Osterkamp @ 2010-10-22 11:26 UTC (permalink / raw)
To: e1000-eedc, virtualization, evb; +Cc: chrisw, Gerhard Stenzel, Jens Osterkamp
In-Reply-To: <1287746820-6036-1-git-send-email-jens@linux.vnet.ibm.com>
modifies the setup of the netlink socket in drv_cfg.c to use pid 0 instead
of the processes pid.
Also replaces bind with connect to allow the reception of netlink messages
from libvirt.
Preparation patch for communication between libvirt and lldpad.
Signed-off-by: Gerhard Stenzel <gstenzel@linux.vnet.ibm.com>
---
drv_cfg.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drv_cfg.c b/drv_cfg.c
index 6c02555..23c11f3 100644
--- a/drv_cfg.c
+++ b/drv_cfg.c
@@ -71,9 +71,9 @@ static int init_socket(void)
memset((void *)&snl, 0, sizeof(struct sockaddr_nl));
snl.nl_family = AF_NETLINK;
- snl.nl_pid = getpid();
+ snl.nl_pid = 0;
- if (bind(sd, (struct sockaddr *)&snl, sizeof(struct sockaddr_nl)) < 0) {
+ if (connect(sd, (struct sockaddr *)&snl, sizeof(struct sockaddr_nl)) < 0) {
close(sd);
return -EIO;
}
@@ -195,6 +195,7 @@ static struct nlmsghdr *get_msg(unsigned int seq)
nlh = NULL;
break;
}
+ fprintf(stderr, "%s:%s: nlmsg_type: %d\n", __FILE__, __FUNCTION__, nlh->nlmsg_type);
if ((nlh->nlmsg_type == RTM_GETDCB ||
nlh->nlmsg_type == RTM_SETDCB) &&
nlh->nlmsg_seq == seq) {
--
1.7.1
^ permalink raw reply related
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