* Re: [PATCH v10 net-next 0/2] load imm64 insn and uapi/linux/bpf.h
From: David Miller @ 2014-09-09 17:30 UTC (permalink / raw)
To: ast-uqk4Ao+rVK5Wk0Htik3J/w
Cc: mingo-DgEjT+Ai2ygdnm+yROfE0A,
torvalds-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
luto-kltTT9wpgjJwATOyAt5JVQ, rostedt-nx8X9YLhiw1AfugRpC6u6w,
dborkman-H+wXaHxf7aLQT0dZR+AlfA,
hannes-tFNcAqjVMyqKXQKiL6tip0B+6BGkLq7r,
chema-hpIqsD4AKlfQT0dZR+AlfA, edumazet-hpIqsD4AKlfQT0dZR+AlfA,
a.p.zijlstra-/NLkJaSkS4VmR6Xm/wNWPw, hpa-YMNOUZJC4hwAvxtiuMwx3w,
akpm-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
keescook-F7+t8E8rja9g9hUCZPvPmw, linux-api-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1409894238-9055-1-git-send-email-ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
From: Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
Date: Thu, 4 Sep 2014 22:17:16 -0700
> V9->V10
> - no changes, added Daniel's ack
>
> Note they're on top of Hannes's patch in the same area [1]
>
> V8 thread with 'why' reasoning and end goal [2]
>
> Original set [3] of ~28 patches I'm planning to present in 4 stages:
>
> I. this 2 patches to fork off llvm upstreaming
> II. bpf syscall with manpage and map implementation
> III. bpf program load/unload with verifier testsuite (1st user of
> instruction macros from bpf.h and 1st user of load imm64 insn)
> IV. tracing, etc
>
> [1] http://patchwork.ozlabs.org/patch/385266/
> [2] https://lkml.org/lkml/2014/8/27/628
> [3] https://lkml.org/lkml/2014/8/26/859
Begrudgingly, I've applied this series.
Although I really wish you had included the mechanism for userland to
use the eBPF instructions alongside exporting them to userspace.
You kept saying "LLVM is the user" but that's a bullshit argument
because you aren't including the patches necessary to actually
propagate native eBPF programs into the kernel.
That's what, 1 or 2 patches, right? Which is not an unreasonable
request.
Anyways, I'm just extremely frustrated with how you operate and work,
you push things way too hard. I hate to say this, but you are the
kind of submitter who gets his way by being persistent rather than
making well formed pleasant submissions that are easy to integrate.
^ permalink raw reply
* Re: [PATCH] locks: Ability to test for flock presence on fd
From: J. Bruce Fields @ 2014-09-09 16:18 UTC (permalink / raw)
To: Pavel Emelyanov
Cc: Jeff Layton, Alexander Viro, linux-fsdevel,
Linux Kernel Mailing List, linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <54061562.4080306-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
On Tue, Sep 02, 2014 at 11:07:14PM +0400, Pavel Emelyanov wrote:
> > Would it make sense to return the lock type held instead, so you could
> > do one flock(fd, LOCK_TEST) instead of flock(fd, LOCK_TEST|LOCK_SH) and
> > flock(fd, LOCK_TEST|LOCK_EX) ?
>
> Well, in our case we parse /proc/locks anyway to see what
> files at least to test for being locked. But what you propose
> looks even better. I'll look what can be done here.
Actually I think I prefer your version. It seems cleaner to define
LOCK_TEST as returning the same result as you'd get if you actually
tried the lock, just without applying the lock. It avoids having a
different return-value convention for this one command. It might avoid
some ambiguity in cases where the flock might be denied for reasons
other than a conflicting flock (e.g. on NFS where flocks and fcntl locks
conflict). It's closer to what GETLK does in the fcntl case.
--b.
^ permalink raw reply
* Re: Request to add linux-kselftest to linux-next
From: Stephen Rothwell @ 2014-09-08 23:14 UTC (permalink / raw)
To: Shuah Khan
Cc: Andrew Morton, linux-next-u79uwXL29TY76Z2rM5mHXA,
greg Kroah-Hartman, fengguang.wu-ral2JQCrhuEAvxtiuMwx3w,
linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <540E36BD.4060607-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1150 bytes --]
Hi Shuah,
On Mon, 08 Sep 2014 17:07:41 -0600 Shuah Khan <shuahkh-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org> wrote:
>
> On 09/08/2014 04:58 PM, Stephen Rothwell wrote:
> > Hi Shuah,
> >
> > On Mon, 08 Sep 2014 08:52:45 -0600 Shuah Khan
> > <shuahkh-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org> wrote:
> >>
> >> Could you please add linux-kselftest tree to linux-next. I plan
> >> to maintain two branches:
> >>
> >> linux-kselftest for Kselftest Framework
> >>
> >> fixes - Contains changes intended for merging before the next
> >> major release, including fixes for regressions introduced during
> >> merge window. next - Contains changes intended for merging
> >> during the next merge window between a major release and -rc1.
> >
> > Umm, a git URL would be helpful :-)
> >
>
> Sorry: here it is:
>
> https://git.kernel.org/cgit/linux/kernel/git/shuah/linux-kselftest.git/
Not a git URL :-(
I assume hat you mean is
git://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git .
I will use that.
--
Cheers,
Stephen Rothwell sfr-3FnU+UHB4dNDw9hX6IcOSA@public.gmane.org
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: Request to add linux-kselftest to linux-next
From: Shuah Khan @ 2014-09-08 23:07 UTC (permalink / raw)
To: Stephen Rothwell
Cc: Andrew Morton, linux-next, greg Kroah-Hartman, fengguang.wu,
linux-api, Shuah Khan
In-Reply-To: <20140909085841.2ec2871e@canb.auug.org.au>
On 09/08/2014 04:58 PM, Stephen Rothwell wrote:
> Hi Shuah,
>
> On Mon, 08 Sep 2014 08:52:45 -0600 Shuah Khan
> <shuahkh@osg.samsung.com> wrote:
>>
>> Could you please add linux-kselftest tree to linux-next. I plan
>> to maintain two branches:
>>
>> linux-kselftest for Kselftest Framework
>>
>> fixes - Contains changes intended for merging before the next
>> major release, including fixes for regressions introduced during
>> merge window. next - Contains changes intended for merging
>> during the next merge window between a major release and -rc1.
>
> Umm, a git URL would be helpful :-)
>
Sorry: here it is:
https://git.kernel.org/cgit/linux/kernel/git/shuah/linux-kselftest.git/
-- Shuah
--
Shuah Khan
Sr. Linux Kernel Developer
Samsung Research America (Silicon Valley)
shuahkh@osg.samsung.com | (970) 217-8978
^ permalink raw reply
* Re: [PATCH v10 net-next 2/2] net: filter: split filter.h and expose eBPF to user space
From: Ingo Molnar @ 2014-09-08 6:37 UTC (permalink / raw)
To: David Miller
Cc: ast-uqk4Ao+rVK5Wk0Htik3J/w, pablo-Cap9r6Oaw4JrovVCs/uTlw,
torvalds-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
luto-kltTT9wpgjJwATOyAt5JVQ, rostedt-nx8X9YLhiw1AfugRpC6u6w,
dborkman-H+wXaHxf7aLQT0dZR+AlfA,
hannes-tFNcAqjVMyqKXQKiL6tip0B+6BGkLq7r,
chema-hpIqsD4AKlfQT0dZR+AlfA, edumazet-hpIqsD4AKlfQT0dZR+AlfA,
a.p.zijlstra-/NLkJaSkS4VmR6Xm/wNWPw, hpa-YMNOUZJC4hwAvxtiuMwx3w,
akpm-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
keescook-F7+t8E8rja9g9hUCZPvPmw, linux-api-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20140907.222841.1933004265241590374.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
* David Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org> wrote:
> From: Ingo Molnar <mingo-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Date: Mon, 8 Sep 2014 07:23:29 +0200
>
> >
> > * Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org> wrote:
> >
> >> > I don't think the speed up of the llvm submission is a
> >> > good argument, this sounds to me similar to the "please
> >> > apply this patch that reserves this new netlink family in
> >> > include/linux/netlink.h, I promise this new subsystem will
> >> > be submitted soon though. Meanwhile this will speed up
> >> > submission of my userspace software to distributions for
> >> > packaging" argument.
> >>
> >> You're not correct here. I'm not saying 'I promise it will
> >> be submitted'. There _were_ already submitted. [...]
> >
> > And this split-up smaller submissions was requested by David
> > Miller, the networking maintainer, so if Pablo wants another
> > submission format, he needs to take it up with David - we
> > can't do both at once obviously.
>
> I think that just because I asked the submission size to be
> smaller, it does not mean that you can submit things before you
> provide the initial user as well.
>
> And how to work that out and keep the submission size
> reasonable is the submitter's problem, not mine.
That's a pretty harsh requirement but might be doable
technically: Alexei, please submit a series that is large enough
to provide self-sufficient functionality as per Pablo's request,
but is also small and minimal, as per David's request.
If that fails then another route would be to decouple from
networking initially and create something new and stand-alone in
kernel/ebpf/ (or any other name really), with tracing and perf
usecases, with networking integration and code deduplication to
be done at the end, when there can be no legitimate argument
about its utility.
Thanks,
Ingo
^ permalink raw reply
* Re: [PATCH v10 net-next 2/2] net: filter: split filter.h and expose eBPF to user space
From: Alexei Starovoitov @ 2014-09-08 6:34 UTC (permalink / raw)
To: David Miller
Cc: Ingo Molnar, Pablo Neira Ayuso, Linus Torvalds, Andy Lutomirski,
Steven Rostedt, Daniel Borkmann, Hannes Frederic Sowa,
Chema Gonzalez, Eric Dumazet, Peter Zijlstra, H. Peter Anvin,
Andrew Morton, Kees Cook, Linux API, Network Development, LKML
In-Reply-To: <20140907.222841.1933004265241590374.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
On Sun, Sep 7, 2014 at 10:28 PM, David Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org> wrote:
> From: Ingo Molnar <mingo-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Date: Mon, 8 Sep 2014 07:23:29 +0200
>
>>
>> * Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org> wrote:
>>
>>> > I don't think the speed up of the llvm submission is a good
>>> > argument, this sounds to me similar to the "please apply this
>>> > patch that reserves this new netlink family in
>>> > include/linux/netlink.h, I promise this new subsystem will be
>>> > submitted soon though. Meanwhile this will speed up
>>> > submission of my userspace software to distributions for
>>> > packaging" argument.
>>>
>>> You're not correct here. I'm not saying 'I promise it will be
>>> submitted'. There _were_ already submitted. [...]
>>
>> And this split-up smaller submissions was requested by David
>> Miller, the networking maintainer, so if Pablo wants another
>> submission format, he needs to take it up with David - we can't
>> do both at once obviously.
>
> I think that just because I asked the submission size to be smaller,
> it does not mean that you can submit things before you provide the
> initial user as well.
>
> And how to work that out and keep the submission size reasonable is
> the submitter's problem, not mine.
imo llvm is more than enough for the first user, no?
I think I've explained that it's practically impossible to have
first in-tree user in the first patch. What do you suggest?
I'm listening, but currently I see no way out.
The patch was large. I broke it down. The first patch is as tiny
as it can get, but Pablo is stalling it, like he did for the last year.
Honestly it doesn't feel fair.
When there were technical arguments (like global vs fd) or
(union attr vs long for syscall) I've listened and rewrote things.
Now it doesn't sound technical.
^ permalink raw reply
* Re: [PATCH v10 net-next 2/2] net: filter: split filter.h and expose eBPF to user space
From: David Miller @ 2014-09-08 5:28 UTC (permalink / raw)
To: mingo-DgEjT+Ai2ygdnm+yROfE0A
Cc: ast-uqk4Ao+rVK5Wk0Htik3J/w, pablo-Cap9r6Oaw4JrovVCs/uTlw,
torvalds-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
luto-kltTT9wpgjJwATOyAt5JVQ, rostedt-nx8X9YLhiw1AfugRpC6u6w,
dborkman-H+wXaHxf7aLQT0dZR+AlfA,
hannes-tFNcAqjVMyqKXQKiL6tip0B+6BGkLq7r,
chema-hpIqsD4AKlfQT0dZR+AlfA, edumazet-hpIqsD4AKlfQT0dZR+AlfA,
a.p.zijlstra-/NLkJaSkS4VmR6Xm/wNWPw, hpa-YMNOUZJC4hwAvxtiuMwx3w,
akpm-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
keescook-F7+t8E8rja9g9hUCZPvPmw, linux-api-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20140908052329.GA30461-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
From: Ingo Molnar <mingo-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Date: Mon, 8 Sep 2014 07:23:29 +0200
>
> * Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org> wrote:
>
>> > I don't think the speed up of the llvm submission is a good
>> > argument, this sounds to me similar to the "please apply this
>> > patch that reserves this new netlink family in
>> > include/linux/netlink.h, I promise this new subsystem will be
>> > submitted soon though. Meanwhile this will speed up
>> > submission of my userspace software to distributions for
>> > packaging" argument.
>>
>> You're not correct here. I'm not saying 'I promise it will be
>> submitted'. There _were_ already submitted. [...]
>
> And this split-up smaller submissions was requested by David
> Miller, the networking maintainer, so if Pablo wants another
> submission format, he needs to take it up with David - we can't
> do both at once obviously.
I think that just because I asked the submission size to be smaller,
it does not mean that you can submit things before you provide the
initial user as well.
And how to work that out and keep the submission size reasonable is
the submitter's problem, not mine.
^ permalink raw reply
* Re: [PATCH v10 net-next 2/2] net: filter: split filter.h and expose eBPF to user space
From: Ingo Molnar @ 2014-09-08 5:23 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Pablo Neira Ayuso, David S. Miller, Linus Torvalds,
Andy Lutomirski, Steven Rostedt, Daniel Borkmann,
Hannes Frederic Sowa, Chema Gonzalez, Eric Dumazet,
Peter Zijlstra, H. Peter Anvin, Andrew Morton, Kees Cook,
Linux API, Network Development, LKML
In-Reply-To: <CAMEtUuxJLDu8AAsZQdtG781vwC8y60Ygdt4ji5tbeyTE8YbWbQ@mail.gmail.com>
* Alexei Starovoitov <ast@plumgrid.com> wrote:
> > I don't think the speed up of the llvm submission is a good
> > argument, this sounds to me similar to the "please apply this
> > patch that reserves this new netlink family in
> > include/linux/netlink.h, I promise this new subsystem will be
> > submitted soon though. Meanwhile this will speed up
> > submission of my userspace software to distributions for
> > packaging" argument.
>
> You're not correct here. I'm not saying 'I promise it will be
> submitted'. There _were_ already submitted. [...]
And this split-up smaller submissions was requested by David
Miller, the networking maintainer, so if Pablo wants another
submission format, he needs to take it up with David - we can't
do both at once obviously.
Thanks,
Ingo
^ permalink raw reply
* Re: [PATCH v10 net-next 2/2] net: filter: split filter.h and expose eBPF to user space
From: Alexei Starovoitov @ 2014-09-07 20:13 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: David S. Miller, Ingo Molnar, Linus Torvalds, Andy Lutomirski,
Steven Rostedt, Daniel Borkmann, Hannes Frederic Sowa,
Chema Gonzalez, Eric Dumazet, Peter Zijlstra, H. Peter Anvin,
Andrew Morton, Kees Cook, Linux API, Network Development, LKML
In-Reply-To: <20140907180737.GA5057@salvia>
On Sun, Sep 7, 2014 at 11:07 AM, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
>
> If the patches that provide the very first user interface don't get in
> time to this merge window round for whatever reason, we'll have the
> layout of this exposed to userspace in the next kernel version with no
> clients at all, that doesn't make sense to me.
eBPF cannot have the first user without verifier and tracing
fully reviewed, so first user cannot be in the first
patch no matter what.
In particular this patch only exposed eBPF as an _instruction set_
to user space. llvm and gcc are only two users.
llvm patches _were_ submitted to the list.
Compilers are not some fictitious users.
eBPF ISA is solid. Two backends is a proof.
For eBPF to be loaded, verifier, syscall and other pieces need to
come in gradually. So I logically split them in series:
stage I - expose instruction set
stage II - bpf syscall for maps and manpage
stage III - programs, verifier and user space testsuite
stage IV - ebpf+tracing (the first user of ebpf isa and syscall)
stage V - ebpf+sockets
stage VI - ebpf+ovs
all of the patches _were_ submitted in the past.
Split is done to make review and integration easier.
After stage III user space will be able to load eBPF programs,
but they will still be useless, because they cannot be attached
to anything. Realistically only stage IV makes first real use of
them in tracing.
You know this, yet, you're saying the first user must be
in the first series. Really, what this 'feedback' is about?
> I don't think the speed up of the llvm submission is a good argument,
> this sounds to me similar to the "please apply this patch that
> reserves this new netlink family in include/linux/netlink.h, I promise
> this new subsystem will be submitted soon though. Meanwhile this will
> speed up submission of my userspace software to distributions for
> packaging" argument.
You're not correct here. I'm not saying 'I promise it will be submitted'.
There _were_ already submitted. I split them in chunks to make
review easier and to follow standard linux philosophy of making
small decisions that can be reverted.
We still have a month until merge window, so if stages II and III
don't make it in time, Dave can revert these small patches just
as easily. You're advocating first_user_must_be_in_first_patch
approach, which is against the linux methodology.
> I think you have to find the way to send a small batch with the very
> essencial stuff that, if merged mainstream, will provide just one new
> feature while leaving the repository in consistent state. Then, send
> follow up patches that enhance your thing and that add new clients of
> it.
As I said above the _minimum_ useful program needs verifier
which is more than Dave's cutoff of 10 patches. Therefore I
split all very_essential_stuff into these stages.
Note I'm not sending radix-tree type of eBPF maps as part
of these stages, neither I send array type of eBPF maps,
though they're needed for my last stage (ebpf+ovs).
I'm not sending pointer leak detector for verifier either,
it will be needed before syscall can be exposed to unprivileged
users, etc. There is a lot of stuff that I need for ebpf+ovs that
is _not_ part of these stages. What you see is really
the minimum to make ebpf+tracing useful.
btw, all of ebpf+ovs was submitted to the list back in Sep 2013.
So there is no practical way to do first set of < 10 patches
that will be usable from user space on its own.
Even if I remove verifier and maps altogether, bpf programs
need to be attached to something like tracing and
that is again >10 patches.
These stages with small patches is only sensible approach.
^ permalink raw reply
* Re: [PATCH v10 net-next 2/2] net: filter: split filter.h and expose eBPF to user space
From: Pablo Neira Ayuso @ 2014-09-07 18:07 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: David S. Miller, Ingo Molnar, Linus Torvalds, Andy Lutomirski,
Steven Rostedt, Daniel Borkmann, Hannes Frederic Sowa,
Chema Gonzalez, Eric Dumazet, Peter Zijlstra, H. Peter Anvin,
Andrew Morton, Kees Cook, Linux API, Network Development, LKML
In-Reply-To: <CAMEtUuxPynQ4wdWDgCU+9_3_GRYn8oA4f-emcF+mqiQdLsqEWg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Sat, Sep 06, 2014 at 09:04:23AM -0700, Alexei Starovoitov wrote:
> On Sat, Sep 6, 2014 at 7:10 AM, Pablo Neira Ayuso <pablo-Cap9r6Oaw4JrovVCs/uTlw@public.gmane.org> wrote:
> > On Thu, Sep 04, 2014 at 10:17:18PM -0700, Alexei Starovoitov wrote:
> >> allow user space to generate eBPF programs
> >>
> >> uapi/linux/bpf.h: eBPF instruction set definition
> >>
> >> linux/filter.h: the rest
> >>
> >> This patch only moves macro definitions, but practically it freezes existing
> >> eBPF instruction set, though new instructions can still be added in the future.
> >>
> >> These eBPF definitions cannot go into uapi/linux/filter.h, since the names
> >> may conflict with existing applications.
> >>
> >> Full eBPF ISA description is in Documentation/networking/filter.txt
> >
> > I think you need to have at least one single interface using this
> > before you can expose it to userspace. So this should come in the
> > small batch that introduces the first interface of your ebpf code in
> > userspace. AFAIK, this has been the policy so far.
>
> That's what I've been doing over the last year.
> My first eBPF patch was in Sep of 2013!
> since then I've been only tweaking and massaging it.
> Nothing fundamentally changed.
> Last few month I've been posting these series with not only
> first user, but with multiple. Many examples, test cases and so on.
> The series became big and Dave asked to split them.
> Please see the cover letter. These two patches is stage I.
> More examples and use cases in stage II, stage III, stage IV, etc
If the patches that provide the very first user interface don't get in
time to this merge window round for whatever reason, we'll have the
layout of this exposed to userspace in the next kernel version with no
clients at all, that doesn't make sense to me.
I don't think the speed up of the llvm submission is a good argument,
this sounds to me similar to the "please apply this patch that
reserves this new netlink family in include/linux/netlink.h, I promise
this new subsystem will be submitted soon though. Meanwhile this will
speed up submission of my userspace software to distributions for
packaging" argument.
I think you have to find the way to send a small batch with the very
essencial stuff that, if merged mainstream, will provide just one new
feature while leaving the repository in consistent state. Then, send
follow up patches that enhance your thing and that add new clients of
it.
^ permalink raw reply
* Re: [PATCH v10 net-next 2/2] net: filter: split filter.h and expose eBPF to user space
From: Alexei Starovoitov @ 2014-09-06 16:04 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: David S. Miller, Ingo Molnar, Linus Torvalds, Andy Lutomirski,
Steven Rostedt, Daniel Borkmann, Hannes Frederic Sowa,
Chema Gonzalez, Eric Dumazet, Peter Zijlstra, H. Peter Anvin,
Andrew Morton, Kees Cook, Linux API, Network Development, LKML
In-Reply-To: <20140906141051.GA4245@salvia>
On Sat, Sep 6, 2014 at 7:10 AM, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Thu, Sep 04, 2014 at 10:17:18PM -0700, Alexei Starovoitov wrote:
>> allow user space to generate eBPF programs
>>
>> uapi/linux/bpf.h: eBPF instruction set definition
>>
>> linux/filter.h: the rest
>>
>> This patch only moves macro definitions, but practically it freezes existing
>> eBPF instruction set, though new instructions can still be added in the future.
>>
>> These eBPF definitions cannot go into uapi/linux/filter.h, since the names
>> may conflict with existing applications.
>>
>> Full eBPF ISA description is in Documentation/networking/filter.txt
>
> I think you need to have at least one single interface using this
> before you can expose it to userspace. So this should come in the
> small batch that introduces the first interface of your ebpf code in
> userspace. AFAIK, this has been the policy so far.
That's what I've been doing over the last year.
My first eBPF patch was in Sep of 2013!
since then I've been only tweaking and massaging it.
Nothing fundamentally changed.
Last few month I've been posting these series with not only
first user, but with multiple. Many examples, test cases and so on.
The series became big and Dave asked to split them.
Please see the cover letter. These two patches is stage I.
More examples and use cases in stage II, stage III, stage IV, etc
All these patches are ready. I'm only submitting them one at
a time to make review easier.
Please see them in my tree if you interested.
Thanks
^ permalink raw reply
* Re: [PATCH v10 net-next 2/2] net: filter: split filter.h and expose eBPF to user space
From: Pablo Neira Ayuso @ 2014-09-06 14:10 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: David S. Miller, Ingo Molnar, Linus Torvalds, Andy Lutomirski,
Steven Rostedt, Daniel Borkmann, Hannes Frederic Sowa,
Chema Gonzalez, Eric Dumazet, Peter Zijlstra, H. Peter Anvin,
Andrew Morton, Kees Cook, linux-api-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1409894238-9055-3-git-send-email-ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
On Thu, Sep 04, 2014 at 10:17:18PM -0700, Alexei Starovoitov wrote:
> allow user space to generate eBPF programs
>
> uapi/linux/bpf.h: eBPF instruction set definition
>
> linux/filter.h: the rest
>
> This patch only moves macro definitions, but practically it freezes existing
> eBPF instruction set, though new instructions can still be added in the future.
>
> These eBPF definitions cannot go into uapi/linux/filter.h, since the names
> may conflict with existing applications.
>
> Full eBPF ISA description is in Documentation/networking/filter.txt
I think you need to have at least one single interface using this
before you can expose it to userspace. So this should come in the
small batch that introduces the first interface of your ebpf code in
userspace. AFAIK, this has been the policy so far.
^ permalink raw reply
* [PATCH v10 net-next 2/2] net: filter: split filter.h and expose eBPF to user space
From: Alexei Starovoitov @ 2014-09-05 5:17 UTC (permalink / raw)
To: David S. Miller
Cc: Ingo Molnar, Linus Torvalds, Andy Lutomirski, Steven Rostedt,
Daniel Borkmann, Hannes Frederic Sowa, Chema Gonzalez,
Eric Dumazet, Peter Zijlstra, H. Peter Anvin, Andrew Morton,
Kees Cook, linux-api-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1409894238-9055-1-git-send-email-ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
allow user space to generate eBPF programs
uapi/linux/bpf.h: eBPF instruction set definition
linux/filter.h: the rest
This patch only moves macro definitions, but practically it freezes existing
eBPF instruction set, though new instructions can still be added in the future.
These eBPF definitions cannot go into uapi/linux/filter.h, since the names
may conflict with existing applications.
Full eBPF ISA description is in Documentation/networking/filter.txt
Signed-off-by: Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
Acked-by: Daniel Borkmann <dborkman-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
include/linux/filter.h | 56 +-------------------------------------
include/uapi/linux/Kbuild | 1 +
include/uapi/linux/bpf.h | 65 +++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 67 insertions(+), 55 deletions(-)
create mode 100644 include/uapi/linux/bpf.h
diff --git a/include/linux/filter.h b/include/linux/filter.h
index bf323da77950..8f82ef3f1cdd 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -10,58 +10,12 @@
#include <linux/workqueue.h>
#include <uapi/linux/filter.h>
#include <asm/cacheflush.h>
+#include <uapi/linux/bpf.h>
struct sk_buff;
struct sock;
struct seccomp_data;
-/* Internally used and optimized filter representation with extended
- * instruction set based on top of classic BPF.
- */
-
-/* instruction classes */
-#define BPF_ALU64 0x07 /* alu mode in double word width */
-
-/* ld/ldx fields */
-#define BPF_DW 0x18 /* double word */
-#define BPF_XADD 0xc0 /* exclusive add */
-
-/* alu/jmp fields */
-#define BPF_MOV 0xb0 /* mov reg to reg */
-#define BPF_ARSH 0xc0 /* sign extending arithmetic shift right */
-
-/* change endianness of a register */
-#define BPF_END 0xd0 /* flags for endianness conversion: */
-#define BPF_TO_LE 0x00 /* convert to little-endian */
-#define BPF_TO_BE 0x08 /* convert to big-endian */
-#define BPF_FROM_LE BPF_TO_LE
-#define BPF_FROM_BE BPF_TO_BE
-
-#define BPF_JNE 0x50 /* jump != */
-#define BPF_JSGT 0x60 /* SGT is signed '>', GT in x86 */
-#define BPF_JSGE 0x70 /* SGE is signed '>=', GE in x86 */
-#define BPF_CALL 0x80 /* function call */
-#define BPF_EXIT 0x90 /* function return */
-
-/* Register numbers */
-enum {
- BPF_REG_0 = 0,
- BPF_REG_1,
- BPF_REG_2,
- BPF_REG_3,
- BPF_REG_4,
- BPF_REG_5,
- BPF_REG_6,
- BPF_REG_7,
- BPF_REG_8,
- BPF_REG_9,
- BPF_REG_10,
- __MAX_BPF_REG,
-};
-
-/* BPF has 10 general purpose 64-bit registers and stack frame. */
-#define MAX_BPF_REG __MAX_BPF_REG
-
/* ArgX, context and stack frame pointer register positions. Note,
* Arg1, Arg2, Arg3, etc are used as argument mappings of function
* calls in BPF_CALL instruction.
@@ -322,14 +276,6 @@ enum {
#define SK_RUN_FILTER(filter, ctx) \
(*filter->prog->bpf_func)(ctx, filter->prog->insnsi)
-struct bpf_insn {
- __u8 code; /* opcode */
- __u8 dst_reg:4; /* dest register */
- __u8 src_reg:4; /* source register */
- __s16 off; /* signed offset */
- __s32 imm; /* signed immediate constant */
-};
-
#ifdef CONFIG_COMPAT
/* A struct sock_filter is architecture independent. */
struct compat_sock_fprog {
diff --git a/include/uapi/linux/Kbuild b/include/uapi/linux/Kbuild
index 24e9033f8b3f..fb3f7b675229 100644
--- a/include/uapi/linux/Kbuild
+++ b/include/uapi/linux/Kbuild
@@ -67,6 +67,7 @@ header-y += bfs_fs.h
header-y += binfmts.h
header-y += blkpg.h
header-y += blktrace_api.h
+header-y += bpf.h
header-y += bpqether.h
header-y += bsg.h
header-y += btrfs.h
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
new file mode 100644
index 000000000000..479ed0b6be16
--- /dev/null
+++ b/include/uapi/linux/bpf.h
@@ -0,0 +1,65 @@
+/* Copyright (c) 2011-2014 PLUMgrid, http://plumgrid.com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of version 2 of the GNU General Public
+ * License as published by the Free Software Foundation.
+ */
+#ifndef _UAPI__LINUX_BPF_H__
+#define _UAPI__LINUX_BPF_H__
+
+#include <linux/types.h>
+
+/* Extended instruction set based on top of classic BPF */
+
+/* instruction classes */
+#define BPF_ALU64 0x07 /* alu mode in double word width */
+
+/* ld/ldx fields */
+#define BPF_DW 0x18 /* double word */
+#define BPF_XADD 0xc0 /* exclusive add */
+
+/* alu/jmp fields */
+#define BPF_MOV 0xb0 /* mov reg to reg */
+#define BPF_ARSH 0xc0 /* sign extending arithmetic shift right */
+
+/* change endianness of a register */
+#define BPF_END 0xd0 /* flags for endianness conversion: */
+#define BPF_TO_LE 0x00 /* convert to little-endian */
+#define BPF_TO_BE 0x08 /* convert to big-endian */
+#define BPF_FROM_LE BPF_TO_LE
+#define BPF_FROM_BE BPF_TO_BE
+
+#define BPF_JNE 0x50 /* jump != */
+#define BPF_JSGT 0x60 /* SGT is signed '>', GT in x86 */
+#define BPF_JSGE 0x70 /* SGE is signed '>=', GE in x86 */
+#define BPF_CALL 0x80 /* function call */
+#define BPF_EXIT 0x90 /* function return */
+
+/* Register numbers */
+enum {
+ BPF_REG_0 = 0,
+ BPF_REG_1,
+ BPF_REG_2,
+ BPF_REG_3,
+ BPF_REG_4,
+ BPF_REG_5,
+ BPF_REG_6,
+ BPF_REG_7,
+ BPF_REG_8,
+ BPF_REG_9,
+ BPF_REG_10,
+ __MAX_BPF_REG,
+};
+
+/* BPF has 10 general purpose 64-bit registers and stack frame. */
+#define MAX_BPF_REG __MAX_BPF_REG
+
+struct bpf_insn {
+ __u8 code; /* opcode */
+ __u8 dst_reg:4; /* dest register */
+ __u8 src_reg:4; /* source register */
+ __s16 off; /* signed offset */
+ __s32 imm; /* signed immediate constant */
+};
+
+#endif /* _UAPI__LINUX_BPF_H__ */
--
1.7.9.5
^ permalink raw reply related
* [PATCH v10 net-next 1/2] net: filter: add "load 64-bit immediate" eBPF instruction
From: Alexei Starovoitov @ 2014-09-05 5:17 UTC (permalink / raw)
To: David S. Miller
Cc: Ingo Molnar, Linus Torvalds, Andy Lutomirski, Steven Rostedt,
Daniel Borkmann, Hannes Frederic Sowa, Chema Gonzalez,
Eric Dumazet, Peter Zijlstra, H. Peter Anvin, Andrew Morton,
Kees Cook, linux-api-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1409894238-9055-1-git-send-email-ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
add BPF_LD_IMM64 instruction to load 64-bit immediate value into a register.
All previous instructions were 8-byte. This is first 16-byte instruction.
Two consecutive 'struct bpf_insn' blocks are interpreted as single instruction:
insn[0].code = BPF_LD | BPF_DW | BPF_IMM
insn[0].dst_reg = destination register
insn[0].imm = lower 32-bit
insn[1].code = 0
insn[1].imm = upper 32-bit
All unused fields must be zero.
Classic BPF has similar instruction: BPF_LD | BPF_W | BPF_IMM
which loads 32-bit immediate value into a register.
x64 JITs it as single 'movabsq %rax, imm64'
arm64 may JIT as sequence of four 'movk x0, #imm16, lsl #shift' insn
Note that old eBPF programs are binary compatible with new interpreter.
It helps eBPF programs load 64-bit constant into a register with one
instruction instead of using two registers and 4 instructions:
BPF_MOV32_IMM(R1, imm32)
BPF_ALU64_IMM(BPF_LSH, R1, 32)
BPF_MOV32_IMM(R2, imm32)
BPF_ALU64_REG(BPF_OR, R1, R2)
User space generated programs will use this instruction to load constants only.
To tell kernel that user space needs a pointer the _pseudo_ variant of
this instruction may be added later, which will use extra bits of encoding
to indicate what type of pointer user space is asking kernel to provide.
For example 'off' or 'src_reg' fields can be used for such purpose.
src_reg = 1 could mean that user space is asking kernel to validate and
load in-kernel map pointer.
src_reg = 2 could mean that user space needs readonly data section pointer
src_reg = 3 could mean that user space needs a pointer to per-cpu local data
All such future pseudo instructions will not be carrying the actual pointer
as part of the instruction, but rather will be treated as a request to kernel
to provide one. The kernel will verify the request_for_a_pointer, then
will drop _pseudo_ marking and will store actual internal pointer inside
the instruction, so the end result is the interpreter and JITs never
see pseudo BPF_LD_IMM64 insns and only operate on generic BPF_LD_IMM64 that
loads 64-bit immediate into a register. User space never operates on direct
pointers and verifier can easily recognize request_for_pointer vs other
instructions.
Signed-off-by: Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
---
Documentation/networking/filter.txt | 8 +++++++-
arch/x86/net/bpf_jit_comp.c | 17 +++++++++++++++++
include/linux/filter.h | 18 ++++++++++++++++++
kernel/bpf/core.c | 5 +++++
lib/test_bpf.c | 21 +++++++++++++++++++++
5 files changed, 68 insertions(+), 1 deletion(-)
diff --git a/Documentation/networking/filter.txt b/Documentation/networking/filter.txt
index c48a9704bda8..81916ab5d96f 100644
--- a/Documentation/networking/filter.txt
+++ b/Documentation/networking/filter.txt
@@ -951,7 +951,7 @@ Size modifier is one of ...
Mode modifier is one of:
- BPF_IMM 0x00 /* classic BPF only, reserved in eBPF */
+ BPF_IMM 0x00 /* used for 32-bit mov in classic BPF and 64-bit in eBPF */
BPF_ABS 0x20
BPF_IND 0x40
BPF_MEM 0x60
@@ -995,6 +995,12 @@ BPF_XADD | BPF_DW | BPF_STX: lock xadd *(u64 *)(dst_reg + off16) += src_reg
Where size is one of: BPF_B or BPF_H or BPF_W or BPF_DW. Note that 1 and
2 byte atomic increments are not supported.
+eBPF has one 16-byte instruction: BPF_LD | BPF_DW | BPF_IMM which consists
+of two consecutive 'struct bpf_insn' 8-byte blocks and interpreted as single
+instruction that loads 64-bit immediate value into a dst_reg.
+Classic BPF has similar instruction: BPF_LD | BPF_W | BPF_IMM which loads
+32-bit immediate value into a register.
+
Testing
-------
diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index 39ccfbb4a723..06f8c17f5484 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -393,6 +393,23 @@ static int do_jit(struct bpf_prog *bpf_prog, int *addrs, u8 *image,
EMIT1_off32(add_1reg(0xB8, dst_reg), imm32);
break;
+ case BPF_LD | BPF_IMM | BPF_DW:
+ if (insn[1].code != 0 || insn[1].src_reg != 0 ||
+ insn[1].dst_reg != 0 || insn[1].off != 0) {
+ /* verifier must catch invalid insns */
+ pr_err("invalid BPF_LD_IMM64 insn\n");
+ return -EINVAL;
+ }
+
+ /* movabsq %rax, imm64 */
+ EMIT2(add_1mod(0x48, dst_reg), add_1reg(0xB8, dst_reg));
+ EMIT(insn[0].imm, 4);
+ EMIT(insn[1].imm, 4);
+
+ insn++;
+ i++;
+ break;
+
/* dst %= src, dst /= src, dst %= imm32, dst /= imm32 */
case BPF_ALU | BPF_MOD | BPF_X:
case BPF_ALU | BPF_DIV | BPF_X:
diff --git a/include/linux/filter.h b/include/linux/filter.h
index c78994593355..bf323da77950 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -166,6 +166,24 @@ enum {
.off = 0, \
.imm = IMM })
+/* BPF_LD_IMM64 macro encodes single 'load 64-bit immediate' insn */
+#define BPF_LD_IMM64(DST, IMM) \
+ BPF_LD_IMM64_RAW(DST, 0, IMM)
+
+#define BPF_LD_IMM64_RAW(DST, SRC, IMM) \
+ ((struct bpf_insn) { \
+ .code = BPF_LD | BPF_DW | BPF_IMM, \
+ .dst_reg = DST, \
+ .src_reg = SRC, \
+ .off = 0, \
+ .imm = (__u32) (IMM) }), \
+ ((struct bpf_insn) { \
+ .code = 0, /* zero is reserved opcode */ \
+ .dst_reg = 0, \
+ .src_reg = 0, \
+ .off = 0, \
+ .imm = ((__u64) (IMM)) >> 32 })
+
/* Short form of mov based on type, BPF_X: dst_reg = src_reg, BPF_K: dst_reg = imm32 */
#define BPF_MOV64_RAW(TYPE, DST, SRC, IMM) \
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index b54bb2c2e494..2c2bfaacce66 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -242,6 +242,7 @@ static unsigned int __bpf_prog_run(void *ctx, const struct bpf_insn *insn)
[BPF_LD | BPF_IND | BPF_W] = &&LD_IND_W,
[BPF_LD | BPF_IND | BPF_H] = &&LD_IND_H,
[BPF_LD | BPF_IND | BPF_B] = &&LD_IND_B,
+ [BPF_LD | BPF_IMM | BPF_DW] = &&LD_IMM_DW,
};
void *ptr;
int off;
@@ -301,6 +302,10 @@ select_insn:
ALU64_MOV_K:
DST = IMM;
CONT;
+ LD_IMM_DW:
+ DST = (u64) (u32) insn[0].imm | ((u64) (u32) insn[1].imm) << 32;
+ insn++;
+ CONT;
ALU64_ARSH_X:
(*(s64 *) &DST) >>= SRC;
CONT;
diff --git a/lib/test_bpf.c b/lib/test_bpf.c
index 9a67456ba29a..413890815d3e 100644
--- a/lib/test_bpf.c
+++ b/lib/test_bpf.c
@@ -1735,6 +1735,27 @@ static struct bpf_test tests[] = {
{ },
{ { 1, 0 } },
},
+ {
+ "load 64-bit immediate",
+ .u.insns_int = {
+ BPF_LD_IMM64(R1, 0x567800001234L),
+ BPF_MOV64_REG(R2, R1),
+ BPF_MOV64_REG(R3, R2),
+ BPF_ALU64_IMM(BPF_RSH, R2, 32),
+ BPF_ALU64_IMM(BPF_LSH, R3, 32),
+ BPF_ALU64_IMM(BPF_RSH, R3, 32),
+ BPF_ALU64_IMM(BPF_MOV, R0, 0),
+ BPF_JMP_IMM(BPF_JEQ, R2, 0x5678, 1),
+ BPF_EXIT_INSN(),
+ BPF_JMP_IMM(BPF_JEQ, R3, 0x1234, 1),
+ BPF_EXIT_INSN(),
+ BPF_ALU64_IMM(BPF_MOV, R0, 1),
+ BPF_EXIT_INSN(),
+ },
+ INTERNAL,
+ { },
+ { { 0, 1 } }
+ },
};
static struct net_device dev;
--
1.7.9.5
^ permalink raw reply related
* [PATCH v10 net-next 0/2] load imm64 insn and uapi/linux/bpf.h
From: Alexei Starovoitov @ 2014-09-05 5:17 UTC (permalink / raw)
To: David S. Miller
Cc: Ingo Molnar, Linus Torvalds, Andy Lutomirski, Steven Rostedt,
Daniel Borkmann, Hannes Frederic Sowa, Chema Gonzalez,
Eric Dumazet, Peter Zijlstra, H. Peter Anvin, Andrew Morton,
Kees Cook, linux-api, netdev, linux-kernel
Hi,
V9->V10
- no changes, added Daniel's ack
Note they're on top of Hannes's patch in the same area [1]
V8 thread with 'why' reasoning and end goal [2]
Original set [3] of ~28 patches I'm planning to present in 4 stages:
I. this 2 patches to fork off llvm upstreaming
II. bpf syscall with manpage and map implementation
III. bpf program load/unload with verifier testsuite (1st user of
instruction macros from bpf.h and 1st user of load imm64 insn)
IV. tracing, etc
[1] http://patchwork.ozlabs.org/patch/385266/
[2] https://lkml.org/lkml/2014/8/27/628
[3] https://lkml.org/lkml/2014/8/26/859
^ permalink raw reply
* Re: [PATCH 1/1] add selftest for virtio-net v1.0
From: Jason Wang @ 2014-09-05 3:31 UTC (permalink / raw)
To: Hengjinxiao, virtualization, netdev, linux-kernel, linux-api; +Cc: famz, mst
In-Reply-To: <1409881866-14780-1-git-send-email-hjxiaohust@gmail.com>
On 09/05/2014 09:51 AM, Hengjinxiao wrote:
> Selftest is an important part of network driver, this patch adds selftest for
> virtio-net, including loopback test, negotiate test and reset test. Loopback
> test checks whether virtio-net can send and receive packets normally. Negotiate test
> executes feature negotiation between virtio-net driver in Guest OS and virtio-net
> device in Host OS. Reset test resets virtio-net.
> Following last patch, this version has deleted some useless codes and fixed bugs
> as you suggest.
> Any corrections are welcome.
>
> Signed-off-by: Hengjinxiao <hjxiaohust@gmail.com>
Some of the lines were indented correctly, some others exceeded 80
characters per line. Please see Documentation/SubmittingPatches for more
guide lines. More important, some of the comments in V1 were still not
addressed.
I suggest split this patch into series:
- patch that introduces virtio core helpers
- patch that introduces new virtio-net helpers
- patch that implements a skeleton of the selftest
- patch that implements loopback test
- patch that implements feature negotation test
Thanks
>
> ---
> drivers/net/virtio_net.c | 241 ++++++++++++++++++++++++++++++++++++++--
> drivers/virtio/virtio.c | 20 +++-
> include/linux/virtio.h | 2 +
> include/uapi/linux/virtio_net.h | 9 ++
> 4 files changed, 256 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 59caa06..22d8228 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -28,6 +28,7 @@
> #include <linux/cpu.h>
> #include <linux/average.h>
> #include <net/busy_poll.h>
> +#include <linux/pci.h>
Why pci.h is needed?
>
> static int napi_weight = NAPI_POLL_WEIGHT;
> module_param(napi_weight, int, 0444);
> @@ -51,6 +52,17 @@ module_param(gso, bool, 0444);
> #define MERGEABLE_BUFFER_ALIGN max(L1_CACHE_BYTES, 256)
>
> #define VIRTNET_DRIVER_VERSION "1.0.0"
> +#define __VIRTNET_TESTING 0
> +
> +static const struct {
> + const char string[ETH_GSTRING_LEN];
> +} virtnet_gstrings_test[] = {
> + { "loopback test (offline)" },
> + { "negotiate test (offline)" },
> + { "reset test (offline)" },
> +};
> +
> +#define VIRTNET_NUM_TEST ARRAY_SIZE(virtnet_gstrings_test)
>
> struct virtnet_stats {
> struct u64_stats_sync tx_syncp;
> @@ -104,6 +116,8 @@ struct virtnet_info {
> struct send_queue *sq;
> struct receive_queue *rq;
> unsigned int status;
> + unsigned long flags;
> + atomic_t lb_count;
>
> /* Max # of queue pairs supported by the device */
> u16 max_queue_pairs;
> @@ -436,6 +450,19 @@ err_buf:
> return NULL;
> }
>
> +void virtnet_check_lb_frame(struct virtnet_info *vi,
> + struct sk_buff *skb)
> +{
> + unsigned int frame_size = skb->len;
> +
> + if (*(skb->data + 3) == 0xFF) {
> + if ((*(skb->data + frame_size / 2 + 10) == 0xBE) &&
> + (*(skb->data + frame_size / 2 + 12) == 0xAF)) {
> + atomic_dec(&vi->lb_count);
> + }
> + }
> +}
> +
> static void receive_buf(struct receive_queue *rq, void *buf, unsigned int len)
> {
> struct virtnet_info *vi = rq->vq->vdev->priv;
> @@ -485,7 +512,12 @@ static void receive_buf(struct receive_queue *rq, void *buf, unsigned int len)
> } else if (hdr->hdr.flags & VIRTIO_NET_HDR_F_DATA_VALID) {
> skb->ip_summed = CHECKSUM_UNNECESSARY;
> }
> -
> + /* loopback self test for ethtool */
> + if (test_bit(__VIRTNET_TESTING, &vi->flags)) {
> + virtnet_check_lb_frame(vi, skb);
> + dev_kfree_skb_any(skb);
> + return;
> + }
Again, we really need a selftest specific handler and don't put anything
in the ordinary fast path.
> skb->protocol = eth_type_trans(skb, dev);
> pr_debug("Receiving skb proto 0x%04x len %i type %i\n",
> ntohs(skb->protocol), skb->len, skb->pkt_type);
> @@ -813,6 +845,9 @@ static int virtnet_open(struct net_device *dev)
> {
> struct virtnet_info *vi = netdev_priv(dev);
> int i;
> + /* disallow open during test */
> + if (test_bit(__VIRTNET_TESTING, &vi->flags))
> + return -EBUSY;
>
> for (i = 0; i < vi->max_queue_pairs; i++) {
> if (i < vi->curr_queue_pairs)
> @@ -1363,12 +1398,166 @@ static void virtnet_get_channels(struct net_device *dev,
> channels->other_count = 0;
> }
>
> +static int virtnet_reset(struct virtnet_info *vi, u64 *data);
> +
> +static void virtnet_create_lb_frame(struct sk_buff *skb,
> + unsigned int frame_size)
> +{
> + memset(skb->data, 0xFF, frame_size);
> + frame_size &= ~1;
> + memset(&skb->data[frame_size / 2], 0xAA, frame_size / 2 - 1);
> + memset(&skb->data[frame_size / 2 + 10], 0xBE, 1);
> + memset(&skb->data[frame_size / 2 + 12], 0xAF, 1);
> +}
> +
> +static int virtnet_start_loopback(struct virtnet_info *vi)
> +{
> + if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_LOOPBACK,
> + VIRTIO_NET_CTRL_LOOPBACK_SET, NULL, NULL)) {
> + dev_warn(&vi->dev->dev, "Failed to set loopback.\n");
> + return -EINVAL;
> + }
> + for (i = 0; i < vi->curr_queue_pairs; i++)
> + napi_disable(&vi->rq[i].napi);
> + return 0;
> +}
> +
> +static int virtnet_run_loopback_test(struct virtnet_info *vi)
> +{
> + int i;
> + struct sk_buff *skb;
> + unsigned int size = GOOD_COPY_LEN;
> +
> + for (i = 0; i < 100; i++) {
> + skb = netdev_alloc_skb(vi->dev, size);
> + if (!skb)
> + return -ENOMEM;
> +
> + skb->queue_mapping = 0;
> + skb_put(skb, size);
> + virtnet_create_lb_frame(skb, size);
> + start_xmit(skb, vi->dev);
> + atomic_inc(&vi->lb_count);
> + }
> + free_old_xmit_skbs(&vi->sq[skb->queue_mapping]);
You could not assume the packets were all sent at this time. It may be
delayed by several reasons e.g host load or zerocopy enabled.
> + /* Give queue time to settle before testing results. */
> + msleep(20);
> + for (i = 0; i < vi->curr_queue_pairs; i++) {
> + void *buf;
> + unsigned int len, received = 0;
> +
> + while ((received < 100) &&
> + (buf = virtqueue_get_buf(vi->rq[i].vq, &len)) != NULL) {
Please use a test specific rx handler for this. And the code here
duplicates the code of virtnet_receive(), you can just pass 100 as
budget to that function.
> + receive_buf(&vi->rq[i], buf, len);
> + --vi->rq[i].num;
> + received++;
> + }
> + if ((vi->rq[i].vq)->num_free <
> + virtqueue_get_vring_size(vi->rq[i].vq) / 2)
> + if (!try_fill_recv(&vi->rq[i], GFP_ATOMIC))
> + schedule_delayed_work(&vi->refill, 0);
You disable NAPI but schedule the refill work, it won't be work until
you enable the NAPI. What's more serious is that if there's a refill
work who want to run at the same time of the beginning of the loopback
test, the virtqueue will never get refilled and the test may fail.
Mixing selftest with real workload brings a lot of complexity. Lots of
thing get easier when you using a dedicated vq handler for selftest:
- Don't bother fast path.
- The rx virtqueue will be empty and completely refilled, so if you're
testing 100 packets which is smaller than 256, no need to care about the
refill, (if you detect a fill need, it was probably a bug)
- You can use polling for rx by just disabling the rx notifiy.
- The tx virtqueue will be empty and you can just detach all skbs at the
end of test.
> + }
> + return atomic_read(&vi->lb_count) ? -EIO : 0;
> +}
> +
> +static int virtnet_stop_loopback(struct virtnet_info *vi)
> +{
> + int i;
> +
> + for (i = 0; i < vi->curr_queue_pairs; i++)
> + virtnet_napi_enable(&vi->rq[i]);
> + if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_LOOPBACK,
> + VIRTIO_NET_CTRL_LOOPBACK_UNSET, NULL, NULL)) {
> + dev_warn(&vi->dev->dev, "Failed to unset loopback.\n");
> + return -EINVAL;
> + }
> + return 0;
> +}
> +
> +static int virtnet_loopback_test(struct virtnet_info *vi, u64 *data)
> +{
> + *data = virtnet_start_loopback(vi);
> + if (*data)
> + goto out;
> + *data = virtnet_run_loopback_test(vi);
> + if (*data)
> + goto out;
Again, you may also need to stop the loopback test in this case?
> + *data = virtnet_stop_loopback(vi);
> +out:
> + return *data;
> +}
> +
> +static int virtnet_feature_neg_test(struct virtnet_info *vi, u64 *data)
> +{
> + struct virtio_device *dev = vi->vdev;
> + u8 status;
> +
> + status = dev->config->get_status(dev);
> + if (status & VIRTIO_CONFIG_S_DRIVER_OK) {
> + u8 test_status = status & ~VIRTIO_CONFIG_S_DRIVER_OK;
> +
> + dev->config->set_status(dev, test_status);
> + }
> + *data = virtio_feature_negotiate(dev);
> + dev->config->set_status(dev, status);
> + return *data;
> +}
> +
> +static int virtnet_get_sset_count(struct net_device *netdev, int sset)
> +{
> + switch (sset) {
> + case ETH_SS_TEST:
> + return VIRTNET_NUM_TEST;
> + default:
> + return -EOPNOTSUPP;
> + }
> +}
> +
> +static void virtnet_get_strings(struct net_device *dev, u32 stringset, u8 *buf)
> +{
> + switch (stringset) {
> + case ETH_SS_TEST:
> + memcpy(buf, &virtnet_gstrings_test,
> + sizeof(virtnet_gstrings_test));
> + break;
> + default:
> + break;
> + }
> +}
> +
> +static void virtnet_self_test(struct net_device *netdev,
> + struct ethtool_test *eth_test, u64 *data)
> +{
> + struct virtnet_info *vi = netdev_priv(netdev);
> +
> + memset(data, 0, sizeof(u64) * VIRTNET_NUM_TEST);
> + if (netif_running(netdev)) {
> + set_bit(__VIRTNET_TESTING, &vi->flags);
> + if (eth_test->flags == ETH_TEST_FL_OFFLINE) {
> + if (virtnet_loopback_test(vi, &data[0]))
> + eth_test->flags |= ETH_TEST_FL_FAILED;
> + if (virtnet_feature_neg_test(vi, &data[1]))
> + eth_test->flags |= ETH_TEST_FL_FAILED;
> + if (virtnet_reset(vi, &data[2]))
> + eth_test->flags |= ETH_TEST_FL_FAILED;
virtnet_reset() sounds like a generic helper, better not use a ehttool
specific parameter.
> + }
> + clear_bit(__VIRTNET_TESTING, &vi->flags);
> + } else {
> + dev_warn(&vi->dev->dev,
> + "%s is down, Loopback test will fail.\n", netdev->name);
> + eth_test->flags |= ETH_TEST_FL_FAILED;
> + }
> +}
> +
> static const struct ethtool_ops virtnet_ethtool_ops = {
> .get_drvinfo = virtnet_get_drvinfo,
> .get_link = ethtool_op_get_link,
> .get_ringparam = virtnet_get_ringparam,
> .set_channels = virtnet_set_channels,
> .get_channels = virtnet_get_channels,
> + .self_test = virtnet_self_test,
> + .get_strings = virtnet_get_strings,
> + .get_sset_count = virtnet_get_sset_count,
> };
>
> #define MIN_MTU 68
> @@ -1890,14 +2079,10 @@ static void virtnet_remove(struct virtio_device *vdev)
> free_netdev(vi->dev);
> }
>
> -#ifdef CONFIG_PM_SLEEP
> -static int virtnet_freeze(struct virtio_device *vdev)
> +static void virtnet_stop(struct virtnet_info *vi)
Please split those introduction of new helpers into separate patches.
> {
> - struct virtnet_info *vi = vdev->priv;
> int i;
>
> - unregister_hotcpu_notifier(&vi->nb);
> -
> /* Prevent config work handler from accessing the device */
> mutex_lock(&vi->config_lock);
> vi->config_enable = false;
> @@ -1906,24 +2091,20 @@ static int virtnet_freeze(struct virtio_device *vdev)
> netif_device_detach(vi->dev);
> cancel_delayed_work_sync(&vi->refill);
>
> - if (netif_running(vi->dev)) {
> + if (netif_running(vi->dev))
> for (i = 0; i < vi->max_queue_pairs; i++) {
> napi_disable(&vi->rq[i].napi);
> napi_hash_del(&vi->rq[i].napi);
> netif_napi_del(&vi->rq[i].napi);
> }
> - }
Those removing of braces seems unrelated to the topic, please drop them.
>
> remove_vq_common(vi);
>
> flush_work(&vi->config_work);
> -
> - return 0;
> }
>
> -static int virtnet_restore(struct virtio_device *vdev)
> +static int virtnet_start(struct virtnet_info *vi)
> {
> - struct virtnet_info *vi = vdev->priv;
> int err, i;
>
> err = init_vqs(vi);
> @@ -1944,7 +2125,27 @@ static int virtnet_restore(struct virtio_device *vdev)
> mutex_lock(&vi->config_lock);
> vi->config_enable = true;
> mutex_unlock(&vi->config_lock);
> + return 0;
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int virtnet_freeze(struct virtio_device *vdev)
> +{
> + struct virtnet_info *vi = vdev->priv;
>
> + unregister_hotcpu_notifier(&vi->nb);
> + virtnet_stop(vi);
> + return 0;
> +}
> +
> +static int virtnet_restore(struct virtio_device *vdev)
> +{
> + struct virtnet_info *vi = vdev->priv;
> + int err;
> +
> + err = virtnet_start(vi);
> + if (err)
> + return err;
> rtnl_lock();
> virtnet_set_queues(vi, vi->curr_queue_pairs);
> rtnl_unlock();
> @@ -1957,6 +2158,22 @@ static int virtnet_restore(struct virtio_device *vdev)
> }
> #endif
>
> +static int virtnet_reset(struct virtnet_info *vi, u64 *data)
> +{
> + struct virtio_device *vdev = vi->vdev;
> + u8 status;
> +
> + virtnet_stop(vi);
> + virtio_feature_negotiate(vdev);
> + *data = virtnet_start(vi);
> + if (*data)
> + return *data;
> + virtnet_set_queues(vi, vi->curr_queue_pairs);
> + status = vdev->config->get_status(vdev);
> + vdev->config->set_status(vdev, status | VIRTIO_CONFIG_S_DRIVER_OK);
> + return 0;
> +}
> +
> static struct virtio_device_id id_table[] = {
> { VIRTIO_ID_NET, VIRTIO_DEV_ANY_ID },
> { 0 },
> diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
> index fed0ce1..2fc396c 100644
> --- a/drivers/virtio/virtio.c
> +++ b/drivers/virtio/virtio.c
> @@ -117,11 +117,10 @@ void virtio_check_driver_offered_feature(const struct virtio_device *vdev,
> }
> EXPORT_SYMBOL_GPL(virtio_check_driver_offered_feature);
>
> -static int virtio_dev_probe(struct device *_d)
> +int virtio_feature_negotiate(struct virtio_device *dev)
> {
> - int err, i;
> - struct virtio_device *dev = dev_to_virtio(_d);
> struct virtio_driver *drv = drv_to_virtio(dev->dev.driver);
> + int i;
> u32 device_features;
>
> /* We have a driver! */
> @@ -134,7 +133,8 @@ static int virtio_dev_probe(struct device *_d)
> memset(dev->features, 0, sizeof(dev->features));
> for (i = 0; i < drv->feature_table_size; i++) {
> unsigned int f = drv->feature_table[i];
> - BUG_ON(f >= 32);
> + if (f >= 32)
> + return -EINVAL;
> if (device_features & (1 << f))
> set_bit(f, dev->features);
> }
> @@ -145,7 +145,19 @@ static int virtio_dev_probe(struct device *_d)
> set_bit(i, dev->features);
>
> dev->config->finalize_features(dev);
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(virtio_feature_negotiate);
>
> +static int virtio_dev_probe(struct device *_d)
Need another patch for this.
> +{
> + int err;
> + struct virtio_device *dev = dev_to_virtio(_d);
> + struct virtio_driver *drv = drv_to_virtio(dev->dev.driver);
> +
> + err = virtio_feature_negotiate(dev);
> + if (err)
> + return err;
> err = drv->probe(dev);
> if (err)
> add_status(dev, VIRTIO_CONFIG_S_FAILED);
> diff --git a/include/linux/virtio.h b/include/linux/virtio.h
> index b46671e..49d8ab4 100644
> --- a/include/linux/virtio.h
> +++ b/include/linux/virtio.h
> @@ -98,6 +98,8 @@ struct virtio_device {
> void *priv;
> };
>
> +int virtio_feature_negotiate(struct virtio_device *dev);
> +
> static inline struct virtio_device *dev_to_virtio(struct device *_dev)
> {
> return container_of(_dev, struct virtio_device, dev);
> diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
> index 172a7f0..1f31f90 100644
> --- a/include/uapi/linux/virtio_net.h
> +++ b/include/uapi/linux/virtio_net.h
> @@ -201,4 +201,13 @@ struct virtio_net_ctrl_mq {
> #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN 1
> #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MAX 0x8000
>
> + /*
> + * Control Loopback(5 is used by VIRTIO_NET_CTRL_GUEST_OFFLOADS in latest qemu)
> + *
> + * The command VIRTIO_NET_CTRL_LOOPBACK_SET is used to require the device come
> + * into loopback state.
> + */
> +#define VIRTIO_NET_CTRL_LOOPBACK 6
> + #define VIRTIO_NET_CTRL_LOOPBACK_SET 0
> + #define VIRTIO_NET_CTRL_LOOPBACK_UNSET 1
> #endif /* _LINUX_VIRTIO_NET_H */
^ permalink raw reply
* [PATCH 1/1] add selftest for virtio-net v1.0
From: Hengjinxiao @ 2014-09-05 1:51 UTC (permalink / raw)
To: virtualization, netdev, linux-kernel, linux-api; +Cc: famz, mst
Selftest is an important part of network driver, this patch adds selftest for
virtio-net, including loopback test, negotiate test and reset test. Loopback
test checks whether virtio-net can send and receive packets normally. Negotiate test
executes feature negotiation between virtio-net driver in Guest OS and virtio-net
device in Host OS. Reset test resets virtio-net.
Following last patch, this version has deleted some useless codes and fixed bugs
as you suggest.
Any corrections are welcome.
Signed-off-by: Hengjinxiao <hjxiaohust@gmail.com>
---
drivers/net/virtio_net.c | 241 ++++++++++++++++++++++++++++++++++++++--
drivers/virtio/virtio.c | 20 +++-
include/linux/virtio.h | 2 +
include/uapi/linux/virtio_net.h | 9 ++
4 files changed, 256 insertions(+), 16 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 59caa06..22d8228 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -28,6 +28,7 @@
#include <linux/cpu.h>
#include <linux/average.h>
#include <net/busy_poll.h>
+#include <linux/pci.h>
static int napi_weight = NAPI_POLL_WEIGHT;
module_param(napi_weight, int, 0444);
@@ -51,6 +52,17 @@ module_param(gso, bool, 0444);
#define MERGEABLE_BUFFER_ALIGN max(L1_CACHE_BYTES, 256)
#define VIRTNET_DRIVER_VERSION "1.0.0"
+#define __VIRTNET_TESTING 0
+
+static const struct {
+ const char string[ETH_GSTRING_LEN];
+} virtnet_gstrings_test[] = {
+ { "loopback test (offline)" },
+ { "negotiate test (offline)" },
+ { "reset test (offline)" },
+};
+
+#define VIRTNET_NUM_TEST ARRAY_SIZE(virtnet_gstrings_test)
struct virtnet_stats {
struct u64_stats_sync tx_syncp;
@@ -104,6 +116,8 @@ struct virtnet_info {
struct send_queue *sq;
struct receive_queue *rq;
unsigned int status;
+ unsigned long flags;
+ atomic_t lb_count;
/* Max # of queue pairs supported by the device */
u16 max_queue_pairs;
@@ -436,6 +450,19 @@ err_buf:
return NULL;
}
+void virtnet_check_lb_frame(struct virtnet_info *vi,
+ struct sk_buff *skb)
+{
+ unsigned int frame_size = skb->len;
+
+ if (*(skb->data + 3) == 0xFF) {
+ if ((*(skb->data + frame_size / 2 + 10) == 0xBE) &&
+ (*(skb->data + frame_size / 2 + 12) == 0xAF)) {
+ atomic_dec(&vi->lb_count);
+ }
+ }
+}
+
static void receive_buf(struct receive_queue *rq, void *buf, unsigned int len)
{
struct virtnet_info *vi = rq->vq->vdev->priv;
@@ -485,7 +512,12 @@ static void receive_buf(struct receive_queue *rq, void *buf, unsigned int len)
} else if (hdr->hdr.flags & VIRTIO_NET_HDR_F_DATA_VALID) {
skb->ip_summed = CHECKSUM_UNNECESSARY;
}
-
+ /* loopback self test for ethtool */
+ if (test_bit(__VIRTNET_TESTING, &vi->flags)) {
+ virtnet_check_lb_frame(vi, skb);
+ dev_kfree_skb_any(skb);
+ return;
+ }
skb->protocol = eth_type_trans(skb, dev);
pr_debug("Receiving skb proto 0x%04x len %i type %i\n",
ntohs(skb->protocol), skb->len, skb->pkt_type);
@@ -813,6 +845,9 @@ static int virtnet_open(struct net_device *dev)
{
struct virtnet_info *vi = netdev_priv(dev);
int i;
+ /* disallow open during test */
+ if (test_bit(__VIRTNET_TESTING, &vi->flags))
+ return -EBUSY;
for (i = 0; i < vi->max_queue_pairs; i++) {
if (i < vi->curr_queue_pairs)
@@ -1363,12 +1398,166 @@ static void virtnet_get_channels(struct net_device *dev,
channels->other_count = 0;
}
+static int virtnet_reset(struct virtnet_info *vi, u64 *data);
+
+static void virtnet_create_lb_frame(struct sk_buff *skb,
+ unsigned int frame_size)
+{
+ memset(skb->data, 0xFF, frame_size);
+ frame_size &= ~1;
+ memset(&skb->data[frame_size / 2], 0xAA, frame_size / 2 - 1);
+ memset(&skb->data[frame_size / 2 + 10], 0xBE, 1);
+ memset(&skb->data[frame_size / 2 + 12], 0xAF, 1);
+}
+
+static int virtnet_start_loopback(struct virtnet_info *vi)
+{
+ if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_LOOPBACK,
+ VIRTIO_NET_CTRL_LOOPBACK_SET, NULL, NULL)) {
+ dev_warn(&vi->dev->dev, "Failed to set loopback.\n");
+ return -EINVAL;
+ }
+ for (i = 0; i < vi->curr_queue_pairs; i++)
+ napi_disable(&vi->rq[i].napi);
+ return 0;
+}
+
+static int virtnet_run_loopback_test(struct virtnet_info *vi)
+{
+ int i;
+ struct sk_buff *skb;
+ unsigned int size = GOOD_COPY_LEN;
+
+ for (i = 0; i < 100; i++) {
+ skb = netdev_alloc_skb(vi->dev, size);
+ if (!skb)
+ return -ENOMEM;
+
+ skb->queue_mapping = 0;
+ skb_put(skb, size);
+ virtnet_create_lb_frame(skb, size);
+ start_xmit(skb, vi->dev);
+ atomic_inc(&vi->lb_count);
+ }
+ free_old_xmit_skbs(&vi->sq[skb->queue_mapping]);
+ /* Give queue time to settle before testing results. */
+ msleep(20);
+ for (i = 0; i < vi->curr_queue_pairs; i++) {
+ void *buf;
+ unsigned int len, received = 0;
+
+ while ((received < 100) &&
+ (buf = virtqueue_get_buf(vi->rq[i].vq, &len)) != NULL) {
+ receive_buf(&vi->rq[i], buf, len);
+ --vi->rq[i].num;
+ received++;
+ }
+ if ((vi->rq[i].vq)->num_free <
+ virtqueue_get_vring_size(vi->rq[i].vq) / 2)
+ if (!try_fill_recv(&vi->rq[i], GFP_ATOMIC))
+ schedule_delayed_work(&vi->refill, 0);
+ }
+ return atomic_read(&vi->lb_count) ? -EIO : 0;
+}
+
+static int virtnet_stop_loopback(struct virtnet_info *vi)
+{
+ int i;
+
+ for (i = 0; i < vi->curr_queue_pairs; i++)
+ virtnet_napi_enable(&vi->rq[i]);
+ if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_LOOPBACK,
+ VIRTIO_NET_CTRL_LOOPBACK_UNSET, NULL, NULL)) {
+ dev_warn(&vi->dev->dev, "Failed to unset loopback.\n");
+ return -EINVAL;
+ }
+ return 0;
+}
+
+static int virtnet_loopback_test(struct virtnet_info *vi, u64 *data)
+{
+ *data = virtnet_start_loopback(vi);
+ if (*data)
+ goto out;
+ *data = virtnet_run_loopback_test(vi);
+ if (*data)
+ goto out;
+ *data = virtnet_stop_loopback(vi);
+out:
+ return *data;
+}
+
+static int virtnet_feature_neg_test(struct virtnet_info *vi, u64 *data)
+{
+ struct virtio_device *dev = vi->vdev;
+ u8 status;
+
+ status = dev->config->get_status(dev);
+ if (status & VIRTIO_CONFIG_S_DRIVER_OK) {
+ u8 test_status = status & ~VIRTIO_CONFIG_S_DRIVER_OK;
+
+ dev->config->set_status(dev, test_status);
+ }
+ *data = virtio_feature_negotiate(dev);
+ dev->config->set_status(dev, status);
+ return *data;
+}
+
+static int virtnet_get_sset_count(struct net_device *netdev, int sset)
+{
+ switch (sset) {
+ case ETH_SS_TEST:
+ return VIRTNET_NUM_TEST;
+ default:
+ return -EOPNOTSUPP;
+ }
+}
+
+static void virtnet_get_strings(struct net_device *dev, u32 stringset, u8 *buf)
+{
+ switch (stringset) {
+ case ETH_SS_TEST:
+ memcpy(buf, &virtnet_gstrings_test,
+ sizeof(virtnet_gstrings_test));
+ break;
+ default:
+ break;
+ }
+}
+
+static void virtnet_self_test(struct net_device *netdev,
+ struct ethtool_test *eth_test, u64 *data)
+{
+ struct virtnet_info *vi = netdev_priv(netdev);
+
+ memset(data, 0, sizeof(u64) * VIRTNET_NUM_TEST);
+ if (netif_running(netdev)) {
+ set_bit(__VIRTNET_TESTING, &vi->flags);
+ if (eth_test->flags == ETH_TEST_FL_OFFLINE) {
+ if (virtnet_loopback_test(vi, &data[0]))
+ eth_test->flags |= ETH_TEST_FL_FAILED;
+ if (virtnet_feature_neg_test(vi, &data[1]))
+ eth_test->flags |= ETH_TEST_FL_FAILED;
+ if (virtnet_reset(vi, &data[2]))
+ eth_test->flags |= ETH_TEST_FL_FAILED;
+ }
+ clear_bit(__VIRTNET_TESTING, &vi->flags);
+ } else {
+ dev_warn(&vi->dev->dev,
+ "%s is down, Loopback test will fail.\n", netdev->name);
+ eth_test->flags |= ETH_TEST_FL_FAILED;
+ }
+}
+
static const struct ethtool_ops virtnet_ethtool_ops = {
.get_drvinfo = virtnet_get_drvinfo,
.get_link = ethtool_op_get_link,
.get_ringparam = virtnet_get_ringparam,
.set_channels = virtnet_set_channels,
.get_channels = virtnet_get_channels,
+ .self_test = virtnet_self_test,
+ .get_strings = virtnet_get_strings,
+ .get_sset_count = virtnet_get_sset_count,
};
#define MIN_MTU 68
@@ -1890,14 +2079,10 @@ static void virtnet_remove(struct virtio_device *vdev)
free_netdev(vi->dev);
}
-#ifdef CONFIG_PM_SLEEP
-static int virtnet_freeze(struct virtio_device *vdev)
+static void virtnet_stop(struct virtnet_info *vi)
{
- struct virtnet_info *vi = vdev->priv;
int i;
- unregister_hotcpu_notifier(&vi->nb);
-
/* Prevent config work handler from accessing the device */
mutex_lock(&vi->config_lock);
vi->config_enable = false;
@@ -1906,24 +2091,20 @@ static int virtnet_freeze(struct virtio_device *vdev)
netif_device_detach(vi->dev);
cancel_delayed_work_sync(&vi->refill);
- if (netif_running(vi->dev)) {
+ if (netif_running(vi->dev))
for (i = 0; i < vi->max_queue_pairs; i++) {
napi_disable(&vi->rq[i].napi);
napi_hash_del(&vi->rq[i].napi);
netif_napi_del(&vi->rq[i].napi);
}
- }
remove_vq_common(vi);
flush_work(&vi->config_work);
-
- return 0;
}
-static int virtnet_restore(struct virtio_device *vdev)
+static int virtnet_start(struct virtnet_info *vi)
{
- struct virtnet_info *vi = vdev->priv;
int err, i;
err = init_vqs(vi);
@@ -1944,7 +2125,27 @@ static int virtnet_restore(struct virtio_device *vdev)
mutex_lock(&vi->config_lock);
vi->config_enable = true;
mutex_unlock(&vi->config_lock);
+ return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int virtnet_freeze(struct virtio_device *vdev)
+{
+ struct virtnet_info *vi = vdev->priv;
+ unregister_hotcpu_notifier(&vi->nb);
+ virtnet_stop(vi);
+ return 0;
+}
+
+static int virtnet_restore(struct virtio_device *vdev)
+{
+ struct virtnet_info *vi = vdev->priv;
+ int err;
+
+ err = virtnet_start(vi);
+ if (err)
+ return err;
rtnl_lock();
virtnet_set_queues(vi, vi->curr_queue_pairs);
rtnl_unlock();
@@ -1957,6 +2158,22 @@ static int virtnet_restore(struct virtio_device *vdev)
}
#endif
+static int virtnet_reset(struct virtnet_info *vi, u64 *data)
+{
+ struct virtio_device *vdev = vi->vdev;
+ u8 status;
+
+ virtnet_stop(vi);
+ virtio_feature_negotiate(vdev);
+ *data = virtnet_start(vi);
+ if (*data)
+ return *data;
+ virtnet_set_queues(vi, vi->curr_queue_pairs);
+ status = vdev->config->get_status(vdev);
+ vdev->config->set_status(vdev, status | VIRTIO_CONFIG_S_DRIVER_OK);
+ return 0;
+}
+
static struct virtio_device_id id_table[] = {
{ VIRTIO_ID_NET, VIRTIO_DEV_ANY_ID },
{ 0 },
diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index fed0ce1..2fc396c 100644
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -117,11 +117,10 @@ void virtio_check_driver_offered_feature(const struct virtio_device *vdev,
}
EXPORT_SYMBOL_GPL(virtio_check_driver_offered_feature);
-static int virtio_dev_probe(struct device *_d)
+int virtio_feature_negotiate(struct virtio_device *dev)
{
- int err, i;
- struct virtio_device *dev = dev_to_virtio(_d);
struct virtio_driver *drv = drv_to_virtio(dev->dev.driver);
+ int i;
u32 device_features;
/* We have a driver! */
@@ -134,7 +133,8 @@ static int virtio_dev_probe(struct device *_d)
memset(dev->features, 0, sizeof(dev->features));
for (i = 0; i < drv->feature_table_size; i++) {
unsigned int f = drv->feature_table[i];
- BUG_ON(f >= 32);
+ if (f >= 32)
+ return -EINVAL;
if (device_features & (1 << f))
set_bit(f, dev->features);
}
@@ -145,7 +145,19 @@ static int virtio_dev_probe(struct device *_d)
set_bit(i, dev->features);
dev->config->finalize_features(dev);
+ return 0;
+}
+EXPORT_SYMBOL_GPL(virtio_feature_negotiate);
+static int virtio_dev_probe(struct device *_d)
+{
+ int err;
+ struct virtio_device *dev = dev_to_virtio(_d);
+ struct virtio_driver *drv = drv_to_virtio(dev->dev.driver);
+
+ err = virtio_feature_negotiate(dev);
+ if (err)
+ return err;
err = drv->probe(dev);
if (err)
add_status(dev, VIRTIO_CONFIG_S_FAILED);
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index b46671e..49d8ab4 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -98,6 +98,8 @@ struct virtio_device {
void *priv;
};
+int virtio_feature_negotiate(struct virtio_device *dev);
+
static inline struct virtio_device *dev_to_virtio(struct device *_dev)
{
return container_of(_dev, struct virtio_device, dev);
diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
index 172a7f0..1f31f90 100644
--- a/include/uapi/linux/virtio_net.h
+++ b/include/uapi/linux/virtio_net.h
@@ -201,4 +201,13 @@ struct virtio_net_ctrl_mq {
#define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN 1
#define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MAX 0x8000
+ /*
+ * Control Loopback(5 is used by VIRTIO_NET_CTRL_GUEST_OFFLOADS in latest qemu)
+ *
+ * The command VIRTIO_NET_CTRL_LOOPBACK_SET is used to require the device come
+ * into loopback state.
+ */
+#define VIRTIO_NET_CTRL_LOOPBACK 6
+ #define VIRTIO_NET_CTRL_LOOPBACK_SET 0
+ #define VIRTIO_NET_CTRL_LOOPBACK_UNSET 1
#endif /* _LINUX_VIRTIO_NET_H */
--
1.8.3.2
^ permalink raw reply related
* Re: [PATCH v9 net-next 2/4] net: filter: split filter.h and expose eBPF to user space
From: David Miller @ 2014-09-04 21:49 UTC (permalink / raw)
To: ast-uqk4Ao+rVK5Wk0Htik3J/w
Cc: mingo-DgEjT+Ai2ygdnm+yROfE0A,
torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
luto-kltTT9wpgjJwATOyAt5JVQ, rostedt-nx8X9YLhiw1AfugRpC6u6w,
hannes-tFNcAqjVMyqKXQKiL6tip0B+6BGkLq7r,
chema-hpIqsD4AKlfQT0dZR+AlfA, edumazet-hpIqsD4AKlfQT0dZR+AlfA,
a.p.zijlstra-/NLkJaSkS4VmR6Xm/wNWPw, hpa-YMNOUZJC4hwAvxtiuMwx3w,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
keescook-F7+t8E8rja9g9hUCZPvPmw, linux-api-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
dborkman-H+wXaHxf7aLQT0dZR+AlfA
In-Reply-To: <CAMEtUuz6zraph_8xXW43bNbh5uPRki0yOufMfV5pKMRLhDrQQA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
From: Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
Date: Wed, 3 Sep 2014 22:41:48 -0700
> Do you want me to resubmit just first two?
Yes, you can't submit hodge-podge path series, either it's all to
be applied or it's all RFC material.
^ permalink raw reply
* Re: [PATCH Resend] memfd_test: Add missing argument to printf()
From: Shuah Khan @ 2014-09-04 18:31 UTC (permalink / raw)
To: Pranith Kumar, Andrew Morton, Hugh Dickins, David Herrmann,
open list:KERNEL SELFTEST F..., open list, Shuah Khan
In-Reply-To: <1409846302-3513-1-git-send-email-bobby.prani-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On 09/04/2014 09:58 AM, Pranith Kumar wrote:
> Add a missing path argument buf to printf()
>
> Signed-off-by: Pranith Kumar <bobby.prani-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Reviewed-by: David Herrmann <dh.herrmann-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> tools/testing/selftests/memfd/memfd_test.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/memfd/memfd_test.c b/tools/testing/selftests/memfd/memfd_test.c
> index 3634c90..c343df8 100644
> --- a/tools/testing/selftests/memfd/memfd_test.c
> +++ b/tools/testing/selftests/memfd/memfd_test.c
> @@ -205,7 +205,7 @@ static void mfd_fail_open(int fd, int flags, mode_t mode)
> sprintf(buf, "/proc/self/fd/%d", fd);
> r = open(buf, flags, mode);
> if (r >= 0) {
> - printf("open(%s) didn't fail as expected\n");
> + printf("open(%s) didn't fail as expected\n", buf);
> abort();
> }
> }
>
Thanks. I queued it for 3.17 fixes.
-- Shuah
--
Shuah Khan
Sr. Linux Kernel Developer
Samsung Research America (Silicon Valley)
shuahkh-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org | (970) 217-8978
^ permalink raw reply
* [PATCH Resend] memfd_test: Add missing argument to printf()
From: Pranith Kumar @ 2014-09-04 15:58 UTC (permalink / raw)
To: Shuah Khan, Andrew Morton, Hugh Dickins, David Herrmann,
open list:KERNEL SELFTEST F..., open list
Add a missing path argument buf to printf()
Signed-off-by: Pranith Kumar <bobby.prani-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Reviewed-by: David Herrmann <dh.herrmann-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
tools/testing/selftests/memfd/memfd_test.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/memfd/memfd_test.c b/tools/testing/selftests/memfd/memfd_test.c
index 3634c90..c343df8 100644
--- a/tools/testing/selftests/memfd/memfd_test.c
+++ b/tools/testing/selftests/memfd/memfd_test.c
@@ -205,7 +205,7 @@ static void mfd_fail_open(int fd, int flags, mode_t mode)
sprintf(buf, "/proc/self/fd/%d", fd);
r = open(buf, flags, mode);
if (r >= 0) {
- printf("open(%s) didn't fail as expected\n");
+ printf("open(%s) didn't fail as expected\n", buf);
abort();
}
}
--
2.1.0
^ permalink raw reply related
* Re: [PATCH] selftests/memfd: fix mfd_fail_open() to pass in buf to printf
From: Shuah Khan @ 2014-09-04 15:21 UTC (permalink / raw)
To: Pranith Kumar
Cc: David Herrmann, Andrew Morton, Hugh Dickins,
open list:KERNEL SELFTEST F..., LKML
In-Reply-To: <CAJhHMCCuyC3HUfUktTpkQcxnqJDpA2O0Gqc8wZMq0TwB5PeY6w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On 09/04/2014 09:16 AM, Pranith Kumar wrote:
> On Thu, Sep 4, 2014 at 11:07 AM, Shuah Khan <shuahkh-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org> wrote:
>> mfd_fail_open() doesn't pass in the buffer to printf resulting
>> in the following warning:
>>
>> memfd_test.c: In function ‘mfd_fail_open’:
>> memfd_test.c:208:3: warning: format ‘%s’ expects a matching ‘char *’ argument [-Wformat=]
>> printf("open(%s) didn't fail as expected\n");
>> ^
>>
>> This change fixes the problem.
>>
>> Signed-off-by: Shuah Khan <shuahkh-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
>
> Hi Shuah,
>
> I already sent in a patch to fix this:
>
> https://lkml.org/lkml/2014/8/31/85
>
> Can you please pick that up?
Hi Pranith,
Great. Could you please resend it to me cc'ing linux-api as well.
thanks,
-- Shuah
--
Shuah Khan
Sr. Linux Kernel Developer
Samsung Research America (Silicon Valley)
shuahkh-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org | (970) 217-8978
^ permalink raw reply
* Re: [PATCH] selftests/memfd: fix mfd_fail_open() to pass in buf to printf
From: Pranith Kumar @ 2014-09-04 15:16 UTC (permalink / raw)
To: Shuah Khan
Cc: David Herrmann, Andrew Morton, Hugh Dickins,
open list:KERNEL SELFTEST F..., LKML
In-Reply-To: <1409843242-6596-1-git-send-email-shuahkh-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
On Thu, Sep 4, 2014 at 11:07 AM, Shuah Khan <shuahkh-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org> wrote:
> mfd_fail_open() doesn't pass in the buffer to printf resulting
> in the following warning:
>
> memfd_test.c: In function ‘mfd_fail_open’:
> memfd_test.c:208:3: warning: format ‘%s’ expects a matching ‘char *’ argument [-Wformat=]
> printf("open(%s) didn't fail as expected\n");
> ^
>
> This change fixes the problem.
>
> Signed-off-by: Shuah Khan <shuahkh-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
Hi Shuah,
I already sent in a patch to fix this:
https://lkml.org/lkml/2014/8/31/85
Can you please pick that up?
Thanks!
--
Pranith
^ permalink raw reply
* [PATCH] selftests/memfd: fix mfd_fail_open() to pass in buf to printf
From: Shuah Khan @ 2014-09-04 15:07 UTC (permalink / raw)
To: dh.herrmann-Re5JQEeQqe8AvxtiuMwx3w,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
bobby.prani-Re5JQEeQqe8AvxtiuMwx3w, hughd-hpIqsD4AKlfQT0dZR+AlfA
Cc: Shuah Khan, linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
mfd_fail_open() doesn't pass in the buffer to printf resulting
in the following warning:
memfd_test.c: In function ‘mfd_fail_open’:
memfd_test.c:208:3: warning: format ‘%s’ expects a matching ‘char *’ argument [-Wformat=]
printf("open(%s) didn't fail as expected\n");
^
This change fixes the problem.
Signed-off-by: Shuah Khan <shuahkh-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
---
tools/testing/selftests/memfd/memfd_test.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/memfd/memfd_test.c b/tools/testing/selftests/memfd/memfd_test.c
index cb5001b..0b9eafb 100644
--- a/tools/testing/selftests/memfd/memfd_test.c
+++ b/tools/testing/selftests/memfd/memfd_test.c
@@ -203,7 +203,7 @@ static void mfd_fail_open(int fd, int flags, mode_t mode)
sprintf(buf, "/proc/self/fd/%d", fd);
r = open(buf, flags, mode);
if (r >= 0) {
- printf("open(%s) didn't fail as expected\n");
+ printf("open(%s) didn't fail as expected\n", buf);
abort();
}
}
--
1.9.1
^ permalink raw reply related
* Re: [PATCH v9 net-next 2/4] net: filter: split filter.h and expose eBPF to user space
From: Alexei Starovoitov @ 2014-09-04 5:41 UTC (permalink / raw)
To: David S. Miller
Cc: Ingo Molnar, Linus Torvalds, Andy Lutomirski, Steven Rostedt,
Hannes Frederic Sowa, Chema Gonzalez, Eric Dumazet,
Peter Zijlstra, H. Peter Anvin, Andrew Morton, Kees Cook,
Linux API, Network Development, LKML, Daniel Borkmann
On Wed, Sep 3, 2014 at 8:46 AM, Daniel Borkmann <dborkman@redhat.com> wrote:
> On 09/03/2014 05:17 AM, Alexei Starovoitov wrote:
>>
>> allow user space to generate eBPF programs
>>
>> uapi/linux/bpf.h: eBPF instruction set definition
>>
>> linux/filter.h: the rest
>>
>> This patch only moves macro definitions, but practically it freezes
>> existing
>> eBPF instruction set, though new instructions can still be added in the
>> future.
>>
>> These eBPF definitions cannot go into uapi/linux/filter.h, since the names
>> may conflict with existing applications.
>>
>> Full eBPF ISA description is in Documentation/networking/filter.txt
>>
>> Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
>
>
> Ok, given you post the remaining two RFCs later on this window as
> you indicate, I have no objections:
>
> Acked-by: Daniel Borkmann <dborkman@redhat.com>
David,
I see that all 4 patches are marked as rfc, whereas I tagged
only 3 and 4 as 'rfc' and mentioned in cover letter that they
will be coming for real in stage III. Here they support patch 2
and show the future changes to bpf.h file.
Patches 1 and 2 are good to go.
Note that they're on top of Hannes's patch:
http://patchwork.ozlabs.org/patch/385266/
Do you want me to resubmit just first two?
Thanks
^ permalink raw reply
* Re: [PATCH v3] memfd_test: Make it work on 32-bit systems
From: Shuah Khan @ 2014-09-03 21:41 UTC (permalink / raw)
To: Pranith Kumar, David Herrmann, Hugh Dickins, Andrew Morton,
open list:KERNEL SELFTEST F..., open list
Cc: Shuah Khan
In-Reply-To: <1409754679-22667-1-git-send-email-bobby.prani@gmail.com>
On 09/03/2014 08:31 AM, Pranith Kumar wrote:
> This test currently fails on 32-bit systems since we use u64 type to pass the
> flags to fcntl.
>
> This commit changes this to use 'unsigned int' type for flags to fcntl making it
> work on 32-bit systems.
>
> Signed-off-by: Pranith Kumar <bobby.prani@gmail.com>
> Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
> ---
I will queue this up for 3.17-fixes. I am in the process of
setting up kselftest git for fixes.
thanks,
-- Shuah
--
Shuah Khan
Sr. Linux Kernel Developer
Samsung Research America (Silicon Valley)
shuahkh@osg.samsung.com | (970) 217-8978
^ 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