* Re: [RFC PATCH net-next v2 0/5] netns: allow to identify peer netns
From: David Ahern @ 2014-09-26 20:56 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Andrew Morton, Cong Wang,
containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Andy Lutomirski, Stephen Hemminger, netdev,
linux-api-u79uwXL29TY76Z2rM5mHXA,
nicolas.dichtel-pdR9zngts4EAvxtiuMwx3w, David Miller
In-Reply-To: <87tx3uun4q.fsf-JOvCrm2gF+uungPnsOpG7nhyD016LWXt@public.gmane.org>
On 9/26/14, 2:45 PM, Eric W. Biederman wrote:
> Ah. You are talking about a system with 2k namespaces and 20-50
> services providing services in all 2k namespaces. Something completely
> different than the case of quagga you mentioned earlier.
Not at all. The earlier quagga example was a starting point on the
bigger topic -- inefficiencies of namespaces as VRFs. In all of the
products I have worked on there is always more than 1 service running on
the system.
> But again this has nothing do with the peer netns work. So if you have
> something practical to contribute please start a new thread.
Sure, I'll start a new thread.
Thanks,
David
^ permalink raw reply
* Re: [RFC PATCH net-next v2 0/5] netns: allow to identify peer netns
From: Eric W. Biederman @ 2014-09-26 20:45 UTC (permalink / raw)
To: David Ahern
Cc: nicolas.dichtel-pdR9zngts4EAvxtiuMwx3w, Cong Wang, netdev,
containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
linux-kernel@vger.kernel.org, linux-api-u79uwXL29TY76Z2rM5mHXA,
David Miller, Stephen Hemminger, Andrew Morton, Andy Lutomirski
In-Reply-To: <5425C22F.7050301-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
David Ahern <lxhacker68-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> writes:
> On 9/26/14, 1:34 PM, Eric W. Biederman wrote:
>> When I wrote the "ip netns" support I never expected that all
>> applications would want to run in a specific network namespace. All
>> that is needed is one socket per network namespace.
>
> Sure that is another option. But for a process to create a socket or
> thread in a second namespace it has to run as root -- CAP_SYS_ADMIN is
> needed for setns (or perhaps there is another way to create the socket
> or thread in the namespace).
To do anything other than simply listen on a netlink socket you also
have to be root. So this is most cases that I am aware of this is a
don't care. Especially for routing daemons.
If it becomes a common pain in writing network namespace aware
applications that the you have to be root just to open your listening
socket then that probably would be sufficient justification for the
socketat system call that I have I prototyped and then never did
anything with because at the time it was insufficiently interesting.
> Second, it still does not address the scalability problem. For example
> a single daemon providing service across 2k namespaces means it needs
> 2k listen sockets. From there a system could have 20, 30 or 50
> services running. Certainly lighter than a process per namespace, but
> not even close to ideal when talking about something like VRFs.
Ah. You are talking about a system with 2k namespaces and 20-50
services providing services in all 2k namespaces. Something completely
different than the case of quagga you mentioned earlier.
I expect quagga would need one netlink control socket and one socket
listening to netlink events, and a tcp connection or two to remote bgp
servers in each network namespace. In that case I don't see anything
except a small constant difference in ways it can be handled.
For your new example of a crazy number of servers running on a box each
of which is had one listening socket in each network namespace maybe
they will be idle most of the time in most network namespaces and the
overhead will be significant. Shrug those applications don't appear to
exist so I can't say what would make a good design.
If someone writes them and describes what is going on we can see if the
current set of interfaces is ideal or problematics. If there are
signifcantly better interfaces that can be provided in a maintainable
way I imagine the patches would be easily accepted.
But again this has nothing do with the peer netns work. So if you have
something practical to contribute please start a new thread.
Eric
^ permalink raw reply
* Re: eBPF verifier thoughts (Re: [PATCH v15 net-next 00/11] eBPF syscall, verifier, testsuite)
From: Andy Lutomirski @ 2014-09-26 20:42 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: David Miller, Ingo Molnar, Linus Torvalds, Daniel Borkmann,
Hannes Frederic Sowa, Chema Gonzalez, Eric Dumazet,
Peter Zijlstra, Pablo Neira Ayuso, H. Peter Anvin, Andrew Morton,
Kees Cook, Linux API, Network Development,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CAMEtUuzAuX=PVDtfQiRKgO7h5wx3gsTJUajr8VKsF4FR-d=JDw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Fri, Sep 26, 2014 at 1:09 PM, Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org> wrote:
> On Fri, Sep 26, 2014 at 12:51 PM, Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> wrote:
>> On Fri, Sep 26, 2014 at 12:34 PM, Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> wrote:
>>
>> To add one more point:
>>
>> With the current verifier design, it's impossible to write a userspace
>> tool that can take an eBPF program and check it. The verification is
>> far too context-dependent for that to be possible. I won't go so far
>> as to say that a userspace tool needs to *exist*, but I strongly
>> object to exposing a verification algorithm that *precludes* writing
>> such a tool.
>
> that's just not true.
> why is it not possible?
Because the types of referenced objects aren't encoded in the blob
that a user program loads, unless I'm missing something.
>
>> I think that the eBPF program format needs to encode all context
>> needed for verification. Then verification should check that the
>> program is compliant with the context and that the context is correct.
>> The former could, in principle, be done in userspace, too.
>
> one can have maps and other future objects equally
> represented in user space. Nothing stops doing exactly the same
> logic there.
But the eBPF binary doesn't encode this information. In fact, the
caller of an ebpf syscall may not even have access to this
information.
I really think that the information needed to check type safety should
be encapsulated in the program.
>
>> Here, "context" includes the program type (i.e. what type R1 hasis),
>> the key and value sizes of all referenced maps, the fact that the maps
>> are maps (damnit, "every object implements exactly the same interface
>> and is called a 'map'" is a bad type system*), and possible also
>> things like the intended stack size and any other relevant details
>> about the entry calling convention.
>
> Andy, I'm not sure where you're going with this.
> Sounds like you want to redesign the whole thing?
> How long it will take?
> Did you consider all the cases I did?
> I think I understand your concerns. What I don't understand
> why you think we cannot address them step by step.
> imo what this does covers a ton of use cases.
> Some futuristic stuff may be better and may be not.
> But here I have it working, tested and proven over many
> use cases, whereas some future unclear stuff will take
> unknown amount of time to redesign...
I think this is addressable as a smallish change on top of your code.
Rather than looking up a map when you need to learn its key and value
size, I think that all you need to do is to look in a program section
for the key and value size (and the fact that it's a map) and confirm
that the referenced map *matches* the stored values.
--Andy
--
Andy Lutomirski
AMA Capital Management, LLC
^ permalink raw reply
* Re: eBPF verifier thoughts (Re: [PATCH v15 net-next 00/11] eBPF syscall, verifier, testsuite)
From: Andy Lutomirski @ 2014-09-26 20:39 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: David Miller, Ingo Molnar, Linus Torvalds, Daniel Borkmann,
Hannes Frederic Sowa, Chema Gonzalez, Eric Dumazet,
Peter Zijlstra, Pablo Neira Ayuso, H. Peter Anvin, Andrew Morton,
Kees Cook, Linux API, Network Development,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CAMEtUuyt9RASKZ4NeqK_OUYNN8QT6P20+kjcwywchur6QXSt0A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Fri, Sep 26, 2014 at 1:00 PM, Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org> wrote:
> On Fri, Sep 26, 2014 at 12:34 PM, Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> wrote:
>> On Fri, Sep 26, 2014 at 12:06 PM, David Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org> wrote:
>>> From: Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
>>> Date: Fri, 26 Sep 2014 00:16:56 -0700
>>>
>>>> v14 -> v15:
>>>> - got rid of macros with hidden control flow (suggested by David)
>>>> replaced macro with explicit goto or return and simplified
>>>> where possible (affected patches #9 and #10)
>>>> - rebased, retested
>>>
>>> Series applied to net-next, thanks.
>>
>> Hi all-
>>
>> I read through the verifier. Sorry this email is a little bit late.
>> Here are some thoughts, in no particular order.
>>
>> Programs should IMO have feature flags. One such feature would be
>> "can use pointers as integers", for example.
>
> of course.
> I believe we discussed it in the past.
> programs will have flags. I'm not sure why you insist on this in
> the first version.
>
>> UNKNOWN_VALUE is IMO a crappy type. I think that it should renamed to
>> INTEGER and that a bunch of the things that generate it should be
>> errors.
>
> it's an internal type.
> Sure, I'm ok renaming it, but 'integer' name doesn't fit.
> It's not integer type.
> When I post a patch for "can use pointers as integers" flag
> you'll see that this 'unknown_value' will be broken down
> into more precise types.
>
>> ALU ops like adding two pointers should IMO be failures. I don't
>> think that they should result in UNKNOWN_VALUE.
>
> they will be failures for unprivileged programs.
> In this patch the whole thing is for root only and root programs need
> arithmetic on pointers to compute hashes and so on.
> I wish we spent more time chatting, so I could explain this better.
>
>> I don't like PTR_TO_MAP_VALUE as a type. I think that there should
>> instead be "pointer to (const?) buffer of N bytes".
>>
>> ARG_PTR_TO_MAP_KEY is odd. Why do we care what a function does with a
>> key? It just needs to be a big enough buffer, right?
>
> not quite. there is a distinction between key and value.
> They both come from map definition and correspond to key_size
> and value_size, so they have to have two different corresponding
> _internal_ types 'ptr_to_map_key' and 'ptr_to_map_value'
> This distinction is needed to properly describe function
> arguments constraints.
But they're still just pointers to buffers of some size known to the
verifier, right? By calling them "pointer to map key" and "pointer to
map value" you're tying them to map objects in a way that makes little
sense to me.
>
>> All of the stack spill stuff seems overcomplicated. Can you just
>> disallow unaligned stack access? Right off the bat, that will delete
>> a bunch of code and cut down runtime memory use by nearly a factor of
>> eight.
>
> not quite. stack spill/fill is not for unaligned access.
> unaligned access is disallowed first.
> See line 663 in check_mem_access().
> Often enough gcc/llvm ran out of registers and have to spill
> them into stack. This spill/fill tracking mechanism keeps
> track of what is stored into stack. Otherwise type violation will be
> possible. We cannot get rid of it. It essential for safety.
So what's "spill part"? Unless I misunderstood the stack tracking
code, you're tracking each byte separately.
You're also tracking the type for each stack slot separately for each
instruction. That looks like it'll account for the considerable
majority of total memory usage.
>
>> Also, there are no calls to functions written eBPF, right? If so, why
>> not just give each stack slot a *fixed* type for the lifetime of the
>> program? That would be a huge time complexity win, not to mention
>> being more comprehensible. When function calls get added, they'll
>> need a whole pile of new infrastructure anyway.
>
> you cannot. stack cannot be fixed. It's very small and valuable
> resources. Programs will be using differently.
>
>> check_call is a mess. I predict that it will be unmaintainable.
>
> why? care to provide details?
I may have misread my notes.
I don't like the fact that the function proto comes from the
environment instead of from the program.
>
>> check_ld_abs is messy, IMO. Can't there be a real type "pointer to
>> object of type [type]", where skb would be a type? Then you could use
>> the normal function call logic for skb pointers instead of hardcoded
>> crud.
>>
>> You're doing a depth-first search. I don't like it. You need complex
>> pruning logic to avoid exponential behavior, and it's not obvious to
>
> there was in patch in previous series that did the prunning.
> I dropped out of this set to simplify things.
>
>> me that pathological programs that cause exponential blowups in the
>> verifier don't exist. Wouldn't it make more sense to do a
>> breadth-first search instead and to give each reg/stack slot a
>> definite type at each point in the control flow? You'd need a
>> function to find the intersection of two types, but I think that would
>> be preferable to the current code that tests whether one type is a
>> subset of another.
>
> nope. breadth-first just doesn't work at all.
Sorry, I didn't actually mean BFS. I meant to order the search such
that all incoming control flow edges to an insn are visited before any
of the outgoing edges are visited.
>
>> At some point someone will want loops, and that will open a huge can
>> of worms involving explicit typing or type inference. It might pay to
>> add some kind of stack slot (and maybe even register slot) type
>> declarations now to get things ready for that.
>
> sure. As discussed we may allow loops in the future.
>
>> NB: If this code is actually invoked in net-next when loading a
>> filter, please don't push to Linus until there's a convincing argument
>> that the verifier has acceptable asymptotic complexity.
>
> complexity is actually described in the doc.
> There are several limits. Verifier will be aborted if it walks
> more then 32k instructions or more then 1k branches.
> So the very worst case takes micro seconds to reject
> the program. So I don't see your concern.
That this will randomly fail, then. For all I know, there are
existing valid BPF programs with vastly more than 32k "instructions"
as counted by the verifier.
--Andy
^ permalink raw reply
* Re: [PATCHv7 10/26] vfio: platform: probe to devices on the platform bus
From: Alex Williamson @ 2014-09-26 20:18 UTC (permalink / raw)
To: Antonios Motakis
Cc: KVM devel mailing list, Eric Auger, Marc Zyngier,
open list:ABI/API, Will Deacon, open list, Linux IOMMU,
VirtualOpenSystems Technical Team, kvm-arm, Christoffer Dall
In-Reply-To: <CAG8rG2wECWL2w9Xyvs7tZx8bqdMqk2SP8gfYpq=ZY0_Jk0Q+-g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Fri, 2014-09-26 at 17:30 +0200, Antonios Motakis wrote:
> On Wed, Sep 24, 2014 at 1:01 AM, Alex Williamson
> <alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> >
> > On Tue, 2014-09-23 at 16:46 +0200, Antonios Motakis wrote:
> > > Driver to bind to Linux platform devices, and callbacks to discover their
> > > resources to be used by the main VFIO PLATFORM code.
> > >
> > > Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>
> > > ---
> > > drivers/vfio/platform/vfio_platform.c | 96 +++++++++++++++++++++++++++++++++++
> > > include/uapi/linux/vfio.h | 1 +
> > > 2 files changed, 97 insertions(+)
> > > create mode 100644 drivers/vfio/platform/vfio_platform.c
> > >
> > > diff --git a/drivers/vfio/platform/vfio_platform.c b/drivers/vfio/platform/vfio_platform.c
> > > new file mode 100644
> > > index 0000000..024c026
> > > --- /dev/null
> > > +++ b/drivers/vfio/platform/vfio_platform.c
> > > @@ -0,0 +1,96 @@
> > > +/*
> > > + * Copyright (C) 2013 - Virtual Open Systems
> > > + * Author: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>
> > > + *
> > > + * This program is free software; you can redistribute it and/or modify
> > > + * it under the terms of the GNU General Public License, version 2, as
> > > + * published by the Free Software Foundation.
> > > + *
> > > + * 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.
> > > + */
> > > +
> > > +#include <linux/device.h>
> > > +#include <linux/eventfd.h>
> > > +#include <linux/interrupt.h>
> > > +#include <linux/iommu.h>
> > > +#include <linux/module.h>
> > > +#include <linux/mutex.h>
> > > +#include <linux/notifier.h>
> > > +#include <linux/pm_runtime.h>
> > > +#include <linux/slab.h>
> > > +#include <linux/types.h>
> > > +#include <linux/uaccess.h>
> > > +#include <linux/vfio.h>
> > > +#include <linux/io.h>
> > > +#include <linux/platform_device.h>
> > > +#include <linux/irq.h>
> > > +
> > > +#include "vfio_platform_private.h"
> > > +
> > > +#define DRIVER_VERSION "0.7"
> > > +#define DRIVER_AUTHOR "Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>"
> > > +#define DRIVER_DESC "VFIO for platform devices - User Level meta-driver"
> > > +
> > > +/* probing devices from the linux platform bus */
> > > +
> > > +static struct resource *get_platform_resource(struct vfio_platform_device *vdev,
> > > + int i)
> > > +{
> > > + struct platform_device *pdev = (struct platform_device *) vdev->opaque;
> > > +
> > > + return platform_get_resource(pdev, IORESOURCE_MEM, i);
> >
> > ARM may only support IORESOURCE_MEM, but I don't think platform devices
> > are limited to MMIO, right? vfio-platform shouldn't be either.
> >
>
> Indeed. Should we however implement this lacking a target to verify it
> is working correctly?
>
> Leaving it out would mean PIO resources for those devices would not be
> exposed before an update to VFIO, but we wouldn't have to break
> backward compatibility I think.
>
> Would you prefer to have it implemented regardless?
I think we need to have PIO figured out at least enough to have stubbed
read/write handlers that could be filled in by someone with test
hardware. I'm not sure I fully understand how a user associates a
region index to a device tree description, whether it's ordering or
something more complicated, so I'm not sure if simply listing all the
PIO resources after the MMIO resources is sufficient and compatible.
Maybe you have some thoughts on that. Thanks,
Alex
> > > +}
> > > +
> > > +static int get_platform_irq(struct vfio_platform_device *vdev, int i)
> > > +{
> > > + struct platform_device *pdev = (struct platform_device *) vdev->opaque;
> > > +
> > > + return platform_get_irq(pdev, i);
> > > +}
> > > +
> > > +
> > > +static int vfio_platform_probe(struct platform_device *pdev)
> > > +{
> > > + struct vfio_platform_device *vdev;
> > > + int ret;
> > > +
> > > + vdev = kzalloc(sizeof(*vdev), GFP_KERNEL);
> > > + if (!vdev)
> > > + return -ENOMEM;
> > > +
> > > + vdev->opaque = (void *) pdev;
> > > + vdev->name = pdev->name;
> > > + vdev->flags = VFIO_DEVICE_FLAGS_PLATFORM;
> > > + vdev->get_resource = get_platform_resource;
> > > + vdev->get_irq = get_platform_irq;
> > > +
> > > + ret = vfio_platform_probe_common(vdev, &pdev->dev);
> > > + if (ret)
> > > + kfree(vdev);
> > > +
> > > + return ret;
> > > +}
> > > +
> > > +static int vfio_platform_remove(struct platform_device *pdev)
> > > +{
> > > + return vfio_platform_remove_common(&pdev->dev);
> > > +}
> > > +
> > > +static struct platform_driver vfio_platform_driver = {
> > > + .probe = vfio_platform_probe,
> > > + .remove = vfio_platform_remove,
> > > + .driver = {
> > > + .name = "vfio-platform",
> > > + .owner = THIS_MODULE,
> > > + },
> > > +};
> > > +
> > > +module_platform_driver(vfio_platform_driver);
> > > +
> > > +MODULE_VERSION(DRIVER_VERSION);
> > > +MODULE_LICENSE("GPL v2");
> > > +MODULE_AUTHOR(DRIVER_AUTHOR);
> > > +MODULE_DESCRIPTION(DRIVER_DESC);
> > > diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> > > index 30f630c..b022a25 100644
> > > --- a/include/uapi/linux/vfio.h
> > > +++ b/include/uapi/linux/vfio.h
> > > @@ -158,6 +158,7 @@ struct vfio_device_info {
> > > __u32 flags;
> > > #define VFIO_DEVICE_FLAGS_RESET (1 << 0) /* Device supports reset */
> > > #define VFIO_DEVICE_FLAGS_PCI (1 << 1) /* vfio-pci device */
> > > +#define VFIO_DEVICE_FLAGS_PLATFORM (1 << 2) /* vfio-platform device */
> > > __u32 num_regions; /* Max region index + 1 */
> > > __u32 num_irqs; /* Max IRQ index + 1 */
> > > };
> >
> >
> >
>
>
>
^ permalink raw reply
* Re: eBPF verifier thoughts (Re: [PATCH v15 net-next 00/11] eBPF syscall, verifier, testsuite)
From: Alexei Starovoitov @ 2014-09-26 20:09 UTC (permalink / raw)
To: Andy Lutomirski
Cc: David Miller, Ingo Molnar, Linus Torvalds, Daniel Borkmann,
Hannes Frederic Sowa, Chema Gonzalez, Eric Dumazet,
Peter Zijlstra, Pablo Neira Ayuso, H. Peter Anvin, Andrew Morton,
Kees Cook, Linux API, Network Development,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CALCETrXFb=etdrabEjbKdxKg51ViPXptOC=6CkKwcGS7Jau8nA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Fri, Sep 26, 2014 at 12:51 PM, Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> wrote:
> On Fri, Sep 26, 2014 at 12:34 PM, Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> wrote:
>
> To add one more point:
>
> With the current verifier design, it's impossible to write a userspace
> tool that can take an eBPF program and check it. The verification is
> far too context-dependent for that to be possible. I won't go so far
> as to say that a userspace tool needs to *exist*, but I strongly
> object to exposing a verification algorithm that *precludes* writing
> such a tool.
that's just not true.
why is it not possible?
> I think that the eBPF program format needs to encode all context
> needed for verification. Then verification should check that the
> program is compliant with the context and that the context is correct.
> The former could, in principle, be done in userspace, too.
one can have maps and other future objects equally
represented in user space. Nothing stops doing exactly the same
logic there.
> Here, "context" includes the program type (i.e. what type R1 hasis),
> the key and value sizes of all referenced maps, the fact that the maps
> are maps (damnit, "every object implements exactly the same interface
> and is called a 'map'" is a bad type system*), and possible also
> things like the intended stack size and any other relevant details
> about the entry calling convention.
Andy, I'm not sure where you're going with this.
Sounds like you want to redesign the whole thing?
How long it will take?
Did you consider all the cases I did?
I think I understand your concerns. What I don't understand
why you think we cannot address them step by step.
imo what this does covers a ton of use cases.
Some futuristic stuff may be better and may be not.
But here I have it working, tested and proven over many
use cases, whereas some future unclear stuff will take
unknown amount of time to redesign...
^ permalink raw reply
* Re: eBPF verifier thoughts (Re: [PATCH v15 net-next 00/11] eBPF syscall, verifier, testsuite)
From: Alexei Starovoitov @ 2014-09-26 20:00 UTC (permalink / raw)
To: Andy Lutomirski
Cc: David Miller, Ingo Molnar, Linus Torvalds, Daniel Borkmann,
Hannes Frederic Sowa, Chema Gonzalez, Eric Dumazet,
Peter Zijlstra, Pablo Neira Ayuso, H. Peter Anvin, Andrew Morton,
Kees Cook, Linux API, Network Development,
linux-kernel@vger.kernel.org
In-Reply-To: <CALCETrVrHHxGo1RkPu+9LVnPo9XY9yYnJzg2ot0zOgoBspzbOQ@mail.gmail.com>
On Fri, Sep 26, 2014 at 12:34 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> On Fri, Sep 26, 2014 at 12:06 PM, David Miller <davem@davemloft.net> wrote:
>> From: Alexei Starovoitov <ast@plumgrid.com>
>> Date: Fri, 26 Sep 2014 00:16:56 -0700
>>
>>> v14 -> v15:
>>> - got rid of macros with hidden control flow (suggested by David)
>>> replaced macro with explicit goto or return and simplified
>>> where possible (affected patches #9 and #10)
>>> - rebased, retested
>>
>> Series applied to net-next, thanks.
>
> Hi all-
>
> I read through the verifier. Sorry this email is a little bit late.
> Here are some thoughts, in no particular order.
>
> Programs should IMO have feature flags. One such feature would be
> "can use pointers as integers", for example.
of course.
I believe we discussed it in the past.
programs will have flags. I'm not sure why you insist on this in
the first version.
> UNKNOWN_VALUE is IMO a crappy type. I think that it should renamed to
> INTEGER and that a bunch of the things that generate it should be
> errors.
it's an internal type.
Sure, I'm ok renaming it, but 'integer' name doesn't fit.
It's not integer type.
When I post a patch for "can use pointers as integers" flag
you'll see that this 'unknown_value' will be broken down
into more precise types.
> ALU ops like adding two pointers should IMO be failures. I don't
> think that they should result in UNKNOWN_VALUE.
they will be failures for unprivileged programs.
In this patch the whole thing is for root only and root programs need
arithmetic on pointers to compute hashes and so on.
I wish we spent more time chatting, so I could explain this better.
> I don't like PTR_TO_MAP_VALUE as a type. I think that there should
> instead be "pointer to (const?) buffer of N bytes".
>
> ARG_PTR_TO_MAP_KEY is odd. Why do we care what a function does with a
> key? It just needs to be a big enough buffer, right?
not quite. there is a distinction between key and value.
They both come from map definition and correspond to key_size
and value_size, so they have to have two different corresponding
_internal_ types 'ptr_to_map_key' and 'ptr_to_map_value'
This distinction is needed to properly describe function
arguments constraints.
> All of the stack spill stuff seems overcomplicated. Can you just
> disallow unaligned stack access? Right off the bat, that will delete
> a bunch of code and cut down runtime memory use by nearly a factor of
> eight.
not quite. stack spill/fill is not for unaligned access.
unaligned access is disallowed first.
See line 663 in check_mem_access().
Often enough gcc/llvm ran out of registers and have to spill
them into stack. This spill/fill tracking mechanism keeps
track of what is stored into stack. Otherwise type violation will be
possible. We cannot get rid of it. It essential for safety.
> Also, there are no calls to functions written eBPF, right? If so, why
> not just give each stack slot a *fixed* type for the lifetime of the
> program? That would be a huge time complexity win, not to mention
> being more comprehensible. When function calls get added, they'll
> need a whole pile of new infrastructure anyway.
you cannot. stack cannot be fixed. It's very small and valuable
resources. Programs will be using differently.
> check_call is a mess. I predict that it will be unmaintainable.
why? care to provide details?
> check_ld_abs is messy, IMO. Can't there be a real type "pointer to
> object of type [type]", where skb would be a type? Then you could use
> the normal function call logic for skb pointers instead of hardcoded
> crud.
>
> You're doing a depth-first search. I don't like it. You need complex
> pruning logic to avoid exponential behavior, and it's not obvious to
there was in patch in previous series that did the prunning.
I dropped out of this set to simplify things.
> me that pathological programs that cause exponential blowups in the
> verifier don't exist. Wouldn't it make more sense to do a
> breadth-first search instead and to give each reg/stack slot a
> definite type at each point in the control flow? You'd need a
> function to find the intersection of two types, but I think that would
> be preferable to the current code that tests whether one type is a
> subset of another.
nope. breadth-first just doesn't work at all.
> At some point someone will want loops, and that will open a huge can
> of worms involving explicit typing or type inference. It might pay to
> add some kind of stack slot (and maybe even register slot) type
> declarations now to get things ready for that.
sure. As discussed we may allow loops in the future.
> NB: If this code is actually invoked in net-next when loading a
> filter, please don't push to Linus until there's a convincing argument
> that the verifier has acceptable asymptotic complexity.
complexity is actually described in the doc.
There are several limits. Verifier will be aborted if it walks
more then 32k instructions or more then 1k branches.
So the very worst case takes micro seconds to reject
the program. So I don't see your concern.
^ permalink raw reply
* Re: eBPF verifier thoughts (Re: [PATCH v15 net-next 00/11] eBPF syscall, verifier, testsuite)
From: Andy Lutomirski @ 2014-09-26 19:51 UTC (permalink / raw)
To: David Miller
Cc: Alexei Starovoitov, Ingo Molnar, Linus Torvalds, Daniel Borkmann,
Hannes Frederic Sowa, Chema Gonzalez, Eric Dumazet,
Peter Zijlstra, Pablo Neira Ayuso, H. Peter Anvin, Andrew Morton,
Kees Cook, Linux API, Network Development,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CALCETrVrHHxGo1RkPu+9LVnPo9XY9yYnJzg2ot0zOgoBspzbOQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Fri, Sep 26, 2014 at 12:34 PM, Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> wrote:
> On Fri, Sep 26, 2014 at 12:06 PM, David Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org> wrote:
>> From: Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
>> Date: Fri, 26 Sep 2014 00:16:56 -0700
>>
>>> v14 -> v15:
>>> - got rid of macros with hidden control flow (suggested by David)
>>> replaced macro with explicit goto or return and simplified
>>> where possible (affected patches #9 and #10)
>>> - rebased, retested
>>
>> Series applied to net-next, thanks.
>
> Hi all-
>
> I read through the verifier. Sorry this email is a little bit late.
> Here are some thoughts, in no particular order.
>
> Programs should IMO have feature flags. One such feature would be
> "can use pointers as integers", for example.
>
> UNKNOWN_VALUE is IMO a crappy type. I think that it should renamed to
> INTEGER and that a bunch of the things that generate it should be
> errors.
>
> ALU ops like adding two pointers should IMO be failures. I don't
> think that they should result in UNKNOWN_VALUE.
>
> I don't like PTR_TO_MAP_VALUE as a type. I think that there should
> instead be "pointer to (const?) buffer of N bytes".
>
> ARG_PTR_TO_MAP_KEY is odd. Why do we care what a function does with a
> key? It just needs to be a big enough buffer, right?
>
> All of the stack spill stuff seems overcomplicated. Can you just
> disallow unaligned stack access? Right off the bat, that will delete
> a bunch of code and cut down runtime memory use by nearly a factor of
> eight.
>
> Also, there are no calls to functions written eBPF, right? If so, why
> not just give each stack slot a *fixed* type for the lifetime of the
> program? That would be a huge time complexity win, not to mention
> being more comprehensible. When function calls get added, they'll
> need a whole pile of new infrastructure anyway.
>
> check_call is a mess. I predict that it will be unmaintainable.
>
> check_ld_abs is messy, IMO. Can't there be a real type "pointer to
> object of type [type]", where skb would be a type? Then you could use
> the normal function call logic for skb pointers instead of hardcoded
> crud.
>
> You're doing a depth-first search. I don't like it. You need complex
> pruning logic to avoid exponential behavior, and it's not obvious to
> me that pathological programs that cause exponential blowups in the
> verifier don't exist. Wouldn't it make more sense to do a
> breadth-first search instead and to give each reg/stack slot a
> definite type at each point in the control flow? You'd need a
> function to find the intersection of two types, but I think that would
> be preferable to the current code that tests whether one type is a
> subset of another.
>
> At some point someone will want loops, and that will open a huge can
> of worms involving explicit typing or type inference. It might pay to
> add some kind of stack slot (and maybe even register slot) type
> declarations now to get things ready for that.
>
> NB: If this code is actually invoked in net-next when loading a
> filter, please don't push to Linus until there's a convincing argument
> that the verifier has acceptable asymptotic complexity.
>
To add one more point:
With the current verifier design, it's impossible to write a userspace
tool that can take an eBPF program and check it. The verification is
far too context-dependent for that to be possible. I won't go so far
as to say that a userspace tool needs to *exist*, but I strongly
object to exposing a verification algorithm that *precludes* writing
such a tool.
I think that the eBPF program format needs to encode all context
needed for verification. Then verification should check that the
program is compliant with the context and that the context is correct.
The former could, in principle, be done in userspace, too.
Here, "context" includes the program type (i.e. what type R1 hasis),
the key and value sizes of all referenced maps, the fact that the maps
are maps (damnit, "every object implements exactly the same interface
and is called a 'map'" is a bad type system*), and possible also
things like the intended stack size and any other relevant details
about the entry calling convention.
* It's especially bad because you already have a type (skb) that
doesn't conform.
--Andy
^ permalink raw reply
* Re: [RFC PATCH net-next v2 0/5] netns: allow to identify peer netns
From: David Ahern @ 2014-09-26 19:44 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Andrew Morton, Cong Wang,
containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Andy Lutomirski, Stephen Hemminger, netdev,
linux-api-u79uwXL29TY76Z2rM5mHXA,
nicolas.dichtel-pdR9zngts4EAvxtiuMwx3w, David Miller
In-Reply-To: <87mw9myy4n.fsf-JOvCrm2gF+uungPnsOpG7nhyD016LWXt@public.gmane.org>
On 9/26/14, 1:34 PM, Eric W. Biederman wrote:
> When I wrote the "ip netns" support I never expected that all
> applications would want to run in a specific network namespace. All
> that is needed is one socket per network namespace.
Sure that is another option. But for a process to create a socket or
thread in a second namespace it has to run as root -- CAP_SYS_ADMIN is
needed for setns (or perhaps there is another way to create the socket
or thread in the namespace).
Second, it still does not address the scalability problem. For example a
single daemon providing service across 2k namespaces means it needs 2k
listen sockets. From there a system could have 20, 30 or 50 services
running. Certainly lighter than a process per namespace, but not even
close to ideal when talking about something like VRFs.
David
^ permalink raw reply
* eBPF verifier thoughts (Re: [PATCH v15 net-next 00/11] eBPF syscall, verifier, testsuite)
From: Andy Lutomirski @ 2014-09-26 19:34 UTC (permalink / raw)
To: David Miller
Cc: Alexei Starovoitov, Ingo Molnar, Linus Torvalds, Daniel Borkmann,
Hannes Frederic Sowa, Chema Gonzalez, Eric Dumazet,
Peter Zijlstra, Pablo Neira Ayuso, H. Peter Anvin, Andrew Morton,
Kees Cook, Linux API, Network Development,
linux-kernel@vger.kernel.org
On Fri, Sep 26, 2014 at 12:06 PM, David Miller <davem@davemloft.net> wrote:
> From: Alexei Starovoitov <ast@plumgrid.com>
> Date: Fri, 26 Sep 2014 00:16:56 -0700
>
>> v14 -> v15:
>> - got rid of macros with hidden control flow (suggested by David)
>> replaced macro with explicit goto or return and simplified
>> where possible (affected patches #9 and #10)
>> - rebased, retested
>
> Series applied to net-next, thanks.
Hi all-
I read through the verifier. Sorry this email is a little bit late.
Here are some thoughts, in no particular order.
Programs should IMO have feature flags. One such feature would be
"can use pointers as integers", for example.
UNKNOWN_VALUE is IMO a crappy type. I think that it should renamed to
INTEGER and that a bunch of the things that generate it should be
errors.
ALU ops like adding two pointers should IMO be failures. I don't
think that they should result in UNKNOWN_VALUE.
I don't like PTR_TO_MAP_VALUE as a type. I think that there should
instead be "pointer to (const?) buffer of N bytes".
ARG_PTR_TO_MAP_KEY is odd. Why do we care what a function does with a
key? It just needs to be a big enough buffer, right?
All of the stack spill stuff seems overcomplicated. Can you just
disallow unaligned stack access? Right off the bat, that will delete
a bunch of code and cut down runtime memory use by nearly a factor of
eight.
Also, there are no calls to functions written eBPF, right? If so, why
not just give each stack slot a *fixed* type for the lifetime of the
program? That would be a huge time complexity win, not to mention
being more comprehensible. When function calls get added, they'll
need a whole pile of new infrastructure anyway.
check_call is a mess. I predict that it will be unmaintainable.
check_ld_abs is messy, IMO. Can't there be a real type "pointer to
object of type [type]", where skb would be a type? Then you could use
the normal function call logic for skb pointers instead of hardcoded
crud.
You're doing a depth-first search. I don't like it. You need complex
pruning logic to avoid exponential behavior, and it's not obvious to
me that pathological programs that cause exponential blowups in the
verifier don't exist. Wouldn't it make more sense to do a
breadth-first search instead and to give each reg/stack slot a
definite type at each point in the control flow? You'd need a
function to find the intersection of two types, but I think that would
be preferable to the current code that tests whether one type is a
subset of another.
At some point someone will want loops, and that will open a huge can
of worms involving explicit typing or type inference. It might pay to
add some kind of stack slot (and maybe even register slot) type
declarations now to get things ready for that.
NB: If this code is actually invoked in net-next when loading a
filter, please don't push to Linus until there's a convincing argument
that the verifier has acceptable asymptotic complexity.
--Andy
^ permalink raw reply
* Re: [RFC PATCH net-next v2 0/5] netns: allow to identify peer netns
From: Eric W. Biederman @ 2014-09-26 19:34 UTC (permalink / raw)
To: David Ahern
Cc: nicolas.dichtel-pdR9zngts4EAvxtiuMwx3w, Cong Wang, netdev,
containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
linux-kernel@vger.kernel.org, linux-api-u79uwXL29TY76Z2rM5mHXA,
David Miller, Stephen Hemminger, Andrew Morton, Andy Lutomirski
In-Reply-To: <5425BB3E.10700-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
David Ahern <lxhacker68-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> writes:
> On 9/26/14, 7:40 AM, Nicolas Dichtel wrote:
>>>
>>>
>>> No, I don't want to monitor anything. Even if I wanted, I would just
>>> start one
>>> daemon in each netns instead of one for all.
>> Ok you don't want, but some other people (not only me) want it! And
>> having one
>> daemon per netns does not scale: there are scenarii with thousand netns
>> which
>> are dynamically created and deleted.
>
> An example of the scaling problem using quagga (old but still seems to be a
> relevant data point):
>
>
> https://lists.quagga.net/pipermail/quagga-users/2010-February/011351.html
>
> "2k VRFs that would be 2.6G"
>
> And that does not include the overhead of each namespace -- roughly
> 200kB/namespace on one kernel I checked (v3.10). So that's a ballpark of 3G of
> memory.
Resetting the conversation just a little bit.
When I wrote the "ip netns" support I never expected that all
applications would want to run in a specific network namespace. All
that is needed is one socket per network namespace.
Furthermore one socket or one procesess per network namespaces is
completely orthogonal to the patches presented. I do not see a
identifying where the far end of a veth pair or similar set of
networking objects as anything that even closely resembles a path to a
using only a single socket.
So I think this whole subthread is quite silly and grossly off track.
Eric
^ permalink raw reply
* Re: [PATCH v2 1/2] perf: Add sampling of the raw monotonic clock
From: David Ahern @ 2014-09-26 19:25 UTC (permalink / raw)
To: Pawel Moll, Namhyung Kim
Cc: Richard Cochran, Steven Rostedt, Ingo Molnar, Peter Zijlstra,
Paul Mackerras, Arnaldo Carvalho de Melo, John Stultz,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <1411743959.3852.45.camel@hornet>
On 9/26/14, 9:05 AM, Pawel Moll wrote:
> To do the correlation you need both timestamps to be "taken"
> simultaneously:
>
> perf event user event
> -----O--------------+-------------O------> t_mono
> : | :
> : V :
> -----O----------------------------O------> t_perf
>
> Of course it's not possible get both values literally at the same time,
> but placing them in a atomic context a couple of instructions from each
> other still gives pretty good results. The larger this distance is, the
An early patchset on this topic added the realtime clock as an event and
an ioctl was used to push a sample into the event stream. In that case
you have wall clock and perf-clock samples taken in the same kernel
context and about as close together as you can get.
https://lkml.org/lkml/2011/2/27/158
https://lkml.org/lkml/2011/2/27/159
David
^ permalink raw reply
* Re: [RFC PATCH net-next v2 0/5] netns: allow to identify peer netns
From: David Ahern @ 2014-09-26 19:15 UTC (permalink / raw)
To: nicolas.dichtel-pdR9zngts4EAvxtiuMwx3w, Cong Wang
Cc: netdev, containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Andy Lutomirski, Stephen Hemminger, Eric W. Biederman,
linux-api-u79uwXL29TY76Z2rM5mHXA, Andrew Morton, David Miller
In-Reply-To: <54256CCB.4000709-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>
On 9/26/14, 7:40 AM, Nicolas Dichtel wrote:
>>
>>
>> No, I don't want to monitor anything. Even if I wanted, I would just
>> start one
>> daemon in each netns instead of one for all.
> Ok you don't want, but some other people (not only me) want it! And
> having one
> daemon per netns does not scale: there are scenarii with thousand netns
> which
> are dynamically created and deleted.
An example of the scaling problem using quagga (old but still seems to
be a relevant data point):
https://lists.quagga.net/pipermail/quagga-users/2010-February/011351.html
"2k VRFs that would be 2.6G"
And that does not include the overhead of each namespace -- roughly
200kB/namespace on one kernel I checked (v3.10). So that's a ballpark of
3G of memory.
David
^ permalink raw reply
* Re: [PATCH v15 net-next 00/11] eBPF syscall, verifier, testsuite
From: David Miller @ 2014-09-26 19:06 UTC (permalink / raw)
To: ast-uqk4Ao+rVK5Wk0Htik3J/w
Cc: mingo-DgEjT+Ai2ygdnm+yROfE0A,
torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
luto-kltTT9wpgjJwATOyAt5JVQ, dborkman-H+wXaHxf7aLQT0dZR+AlfA,
hannes-tFNcAqjVMyqKXQKiL6tip0B+6BGkLq7r,
chema-hpIqsD4AKlfQT0dZR+AlfA, edumazet-hpIqsD4AKlfQT0dZR+AlfA,
a.p.zijlstra-/NLkJaSkS4VmR6Xm/wNWPw, pablo-Cap9r6Oaw4JrovVCs/uTlw,
hpa-YMNOUZJC4hwAvxtiuMwx3w, akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
keescook-F7+t8E8rja9g9hUCZPvPmw, linux-api-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1411715827-10430-1-git-send-email-ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
From: Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
Date: Fri, 26 Sep 2014 00:16:56 -0700
> v14 -> v15:
> - got rid of macros with hidden control flow (suggested by David)
> replaced macro with explicit goto or return and simplified
> where possible (affected patches #9 and #10)
> - rebased, retested
Series applied to net-next, thanks.
^ permalink raw reply
* Re: [RFC PATCH net-next v2 0/5] netns: allow to identify peer netns
From: Eric W. Biederman @ 2014-09-26 18:57 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Andrew Morton, Network Development, Linux Containers,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Stephen Hemminger, Cong Wang, Linux API, Nicolas Dichtel,
David S. Miller
In-Reply-To: <CALCETrX5e0cp4QFCv1eAqR1hjoROU9Rh=cRos9U35DaR-py3Eg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> writes:
> On Fri, Sep 26, 2014 at 11:10 AM, Eric W. Biederman
> <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org> wrote:
>> Nicolas Dichtel <nicolas.dichtel-pdR9zngts4EAvxtiuMwx3w@public.gmane.org> writes:
>>
>>> The goal of this serie is to be able to multicast netlink messages with an
>>> attribute that identify a peer netns.
>>> This is needed by the userland to interpret some informations contained in
>>> netlink messages (like IFLA_LINK value, but also some other attributes in case
>>> of x-netns netdevice (see also
>>> http://thread.gmane.org/gmane.linux.network/315933/focus=316064 and
>>> http://thread.gmane.org/gmane.linux.kernel.containers/28301/focus=4239)).
>>
>> I want say that the problem addressed by patch 3/5 of this series is a
>> fundamentally valid problem. We have network objects spanning network
>> namespaces and it would be very nice to be able to talk about them in
>> netlink, and file descriptors are too local and argubably too heavy
>> weight for netlink quires and especially for netlink broadcast messages.
>>
>> Furthermore the concept of ineternal concept of peernet2id seems valid.
>>
>> However what you do not address is a way for CRIU (aka process
>> migration) to be able to restore these ids after process migration.
>> Going farther it looks like you are actively breaking process migration
>> at this time, making this set of patches a no-go.
>>
>> When adding a new form of namespace id CRIU patches are just about
>> as necessary as iproute patches.
>>
>>> Ids are stored in the parent user namespace. These ids are valid only inside
>>> this user namespace. The user can retrieve these ids via a new netlink messages,
>>> but only if peer netns are in the same user namespace.
>>
>> That does not describe what you have actually implemented in the
>> patches.
>>
>> I see two ways to go with this.
>>
>> - A per network namespace table to that you can store ids for ``peer''
>> network namespaces. The table would need to be populated manually by
>> the likes of ip netns add.
>>
>> That flips the order of assignment and makes this idea solid.
>>
>> Unfortunately in the case of a fully referencing mesh of N network
>> namespaces such a mesh winds up taking O(N^2) space, which seems
>> undesirable.
>>
>> - Add a netlink attribute that says this network element is in a peer
>> network namespace.
>>
>> Add a unicast query message that let's you ask if the remote
>> end of a tunnel is in a network namespace specified by file
>> descriptor.
>>
>> I personally lean towards the second version as it is fundamentally
>> simpler, and generally scales better, and the visibility controls are
>> the existing visibility controls. The only downside is it requires
>> a query after receiving a netlink broadcast message for the times that
>> we care.
>
> The downside of that approach, and all the similar kcmp stuff, is that
> it scales poorly for applications using it. This is probably not the
> end of the world, but it's not ideal.
Agreed, the efficiency is not ideal and there is plenty of room for
optimization. We could certainly adopt some of kcmps ordering
infrastructure to make it suck less, or even potentially work out how
to return a file descriptor to the network namespace in question.
The key insight of my second proposal is that we can get out of the
broadcast message business, and only care about the remote namespace for
unicast messages. Putting the work in an infrequently used slow path
instead of a comparitively common path gives us much more freedom in
the implementation.
Eric
^ permalink raw reply
* Re: [RFC PATCH net-next v2 0/5] netns: allow to identify peer netns
From: Andy Lutomirski @ 2014-09-26 18:26 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Andrew Morton, Network Development, Linux Containers,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Stephen Hemminger, Cong Wang, Linux API, Nicolas Dichtel,
David S. Miller
In-Reply-To: <87ppei45ig.fsf-JOvCrm2gF+uungPnsOpG7nhyD016LWXt@public.gmane.org>
On Fri, Sep 26, 2014 at 11:10 AM, Eric W. Biederman
<ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org> wrote:
> Nicolas Dichtel <nicolas.dichtel-pdR9zngts4EAvxtiuMwx3w@public.gmane.org> writes:
>
>> The goal of this serie is to be able to multicast netlink messages with an
>> attribute that identify a peer netns.
>> This is needed by the userland to interpret some informations contained in
>> netlink messages (like IFLA_LINK value, but also some other attributes in case
>> of x-netns netdevice (see also
>> http://thread.gmane.org/gmane.linux.network/315933/focus=316064 and
>> http://thread.gmane.org/gmane.linux.kernel.containers/28301/focus=4239)).
>
> I want say that the problem addressed by patch 3/5 of this series is a
> fundamentally valid problem. We have network objects spanning network
> namespaces and it would be very nice to be able to talk about them in
> netlink, and file descriptors are too local and argubably too heavy
> weight for netlink quires and especially for netlink broadcast messages.
>
> Furthermore the concept of ineternal concept of peernet2id seems valid.
>
> However what you do not address is a way for CRIU (aka process
> migration) to be able to restore these ids after process migration.
> Going farther it looks like you are actively breaking process migration
> at this time, making this set of patches a no-go.
>
> When adding a new form of namespace id CRIU patches are just about
> as necessary as iproute patches.
>
>> Ids are stored in the parent user namespace. These ids are valid only inside
>> this user namespace. The user can retrieve these ids via a new netlink messages,
>> but only if peer netns are in the same user namespace.
>
> That does not describe what you have actually implemented in the
> patches.
>
> I see two ways to go with this.
>
> - A per network namespace table to that you can store ids for ``peer''
> network namespaces. The table would need to be populated manually by
> the likes of ip netns add.
>
> That flips the order of assignment and makes this idea solid.
>
> Unfortunately in the case of a fully referencing mesh of N network
> namespaces such a mesh winds up taking O(N^2) space, which seems
> undesirable.
>
> - Add a netlink attribute that says this network element is in a peer
> network namespace.
>
> Add a unicast query message that let's you ask if the remote
> end of a tunnel is in a network namespace specified by file
> descriptor.
>
> I personally lean towards the second version as it is fundamentally
> simpler, and generally scales better, and the visibility controls are
> the existing visibility controls. The only downside is it requires
> a query after receiving a netlink broadcast message for the times that
> we care.
The downside of that approach, and all the similar kcmp stuff, is that
it scales poorly for applications using it. This is probably not the
end of the world, but it's not ideal.
--Andy
^ permalink raw reply
* Re: [PATCH v2] next: openrisc: Fix build
From: Jonas Bonn @ 2014-09-26 18:18 UTC (permalink / raw)
To: Guenter Roeck, Eric Paris
Cc: Stephen Rothwell, linux-api, linux, linux-kernel, linux-next
In-Reply-To: <20140926180813.GA19291@roeck-us.net>
--
Sent from my Android phone with K-9 Mail. Please excuse my brevity.
On September 26, 2014 8:08:13 PM CEST, Guenter Roeck <linux@roeck-us.net> wrote:
>On Fri, Sep 26, 2014 at 01:53:01PM -0400, Eric Paris wrote:
>> Would you like me to carry this in the audit tree, since I'm the one
>who
>> broke it?
>>
>Hi Eric,
>
>guess you are asking Jonas, but it would make sense to me.
Hi Eric,
Yes, please take this via the audit tree.
/Jonas
>
>Guenter
>
>> -Eric
>>
>> On Fri, 2014-09-26 at 09:05 -0700, Guenter Roeck wrote:
>> > openrisc:defconfig fails to build in next-20140926 with the
>following error.
>> >
>> > In file included from arch/openrisc/kernel/signal.c:31:0:
>> > ./arch/openrisc/include/asm/syscall.h: In function
>'syscall_get_arch':
>> > ./arch/openrisc/include/asm/syscall.h:77:9: error: 'EM_OPENRISC'
>undeclared
>> >
>> > Fix by moving EM_OPENRISC to include/uapi/linux/elf-em.h.
>> >
>> > Fixes: ce5d112827e5 ("ARCH: AUDIT: implement syscall_get_arch for
>all arches")
>> > Cc: Eric Paris <eparis@redhat.com>
>> > Cc: Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>
>> > Cc: Geert Uytterhoeven <geert@linux-m68k.org>
>> > Cc: Stephen Rothwell <sfr@canb.auug.org.au>
>> > Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>> > ---
>> > v2: Only move EM_OPENRISC.
>> >
>> > Another possible solution for the problem would be to include
>asm/elf.h
>> > in arch/openrisc/kernel/signal.c. I had actually submitted a patch
>with
>> > that fix back in August (maybe that is where I remembered the
>problem from).
>> > Wonder what happened with that patch.
>> >
>> > Would it make sense to drop EM_OR32 and replace it with EM_OPENRISC
>where
>> > it is used ? binutils seems to suggest that EM_OPENRISC is the
>"official"
>> > definition.
>> >
>> > arch/openrisc/include/uapi/asm/elf.h | 3 +--
>> > include/uapi/linux/elf-em.h | 1 +
>> > 2 files changed, 2 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/arch/openrisc/include/uapi/asm/elf.h
>b/arch/openrisc/include/uapi/asm/elf.h
>> > index f02ea58..8884276 100644
>> > --- a/arch/openrisc/include/uapi/asm/elf.h
>> > +++ b/arch/openrisc/include/uapi/asm/elf.h
>> > @@ -55,9 +55,8 @@ typedef elf_greg_t elf_gregset_t[ELF_NGREG];
>> > /* A placeholder; OR32 does not have fp support yes, so no fp regs
>for now. */
>> > typedef unsigned long elf_fpregset_t;
>> >
>> > -/* This should be moved to include/linux/elf.h */
>> > +/* EM_OPENRISC is defined in linux/elf-em.h */
>> > #define EM_OR32 0x8472
>> > -#define EM_OPENRISC 92 /* OpenRISC 32-bit embedded
>processor */
>> >
>> > /*
>> > * These are used to set parameters in the core dumps.
>> > diff --git a/include/uapi/linux/elf-em.h
>b/include/uapi/linux/elf-em.h
>> > index 01529bd..aa90bc9 100644
>> > --- a/include/uapi/linux/elf-em.h
>> > +++ b/include/uapi/linux/elf-em.h
>> > @@ -32,6 +32,7 @@
>> > #define EM_V850 87 /* NEC v850 */
>> > #define EM_M32R 88 /* Renesas M32R */
>> > #define EM_MN10300 89 /* Panasonic/MEI MN10300, AM33 */
>> > +#define EM_OPENRISC 92 /* OpenRISC 32-bit embedded
>processor */
>> > #define EM_BLACKFIN 106 /* ADI Blackfin Processor */
>> > #define EM_TI_C6000 140 /* TI C6X DSPs */
>> > #define EM_AARCH64 183 /* ARM 64 bit */
>>
>>
^ permalink raw reply
* Re: [RFC PATCH net-next v2 0/5] netns: allow to identify peer netns
From: Eric W. Biederman @ 2014-09-26 18:10 UTC (permalink / raw)
To: Nicolas Dichtel
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, luto-kltTT9wpgjJwATOyAt5JVQ,
stephen-OTpzqLSitTUnbdJkjeBofR2eb7JE58TQ, Cong Wang,
linux-api-u79uwXL29TY76Z2rM5mHXA,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <1411478430-4989-1-git-send-email-nicolas.dichtel-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>
Nicolas Dichtel <nicolas.dichtel-pdR9zngts4EAvxtiuMwx3w@public.gmane.org> writes:
> The goal of this serie is to be able to multicast netlink messages with an
> attribute that identify a peer netns.
> This is needed by the userland to interpret some informations contained in
> netlink messages (like IFLA_LINK value, but also some other attributes in case
> of x-netns netdevice (see also
> http://thread.gmane.org/gmane.linux.network/315933/focus=316064 and
> http://thread.gmane.org/gmane.linux.kernel.containers/28301/focus=4239)).
I want say that the problem addressed by patch 3/5 of this series is a
fundamentally valid problem. We have network objects spanning network
namespaces and it would be very nice to be able to talk about them in
netlink, and file descriptors are too local and argubably too heavy
weight for netlink quires and especially for netlink broadcast messages.
Furthermore the concept of ineternal concept of peernet2id seems valid.
However what you do not address is a way for CRIU (aka process
migration) to be able to restore these ids after process migration.
Going farther it looks like you are actively breaking process migration
at this time, making this set of patches a no-go.
When adding a new form of namespace id CRIU patches are just about
as necessary as iproute patches.
> Ids are stored in the parent user namespace. These ids are valid only inside
> this user namespace. The user can retrieve these ids via a new netlink messages,
> but only if peer netns are in the same user namespace.
That does not describe what you have actually implemented in the
patches.
I see two ways to go with this.
- A per network namespace table to that you can store ids for ``peer''
network namespaces. The table would need to be populated manually by
the likes of ip netns add.
That flips the order of assignment and makes this idea solid.
Unfortunately in the case of a fully referencing mesh of N network
namespaces such a mesh winds up taking O(N^2) space, which seems
undesirable.
- Add a netlink attribute that says this network element is in a peer
network namespace.
Add a unicast query message that let's you ask if the remote
end of a tunnel is in a network namespace specified by file
descriptor.
I personally lean towards the second version as it is fundamentally
simpler, and generally scales better, and the visibility controls are
the existing visibility controls. The only downside is it requires
a query after receiving a netlink broadcast message for the times that
we care.
Eric
^ permalink raw reply
* Re: [PATCH v2] next: openrisc: Fix build
From: Guenter Roeck @ 2014-09-26 18:08 UTC (permalink / raw)
To: Eric Paris
Cc: Jonas Bonn, linux-73nT9ZVIoz3k1uMJSBkQmQ,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-next-u79uwXL29TY76Z2rM5mHXA, Stefan Kristiansson,
Geert Uytterhoeven, Stephen Rothwell
In-Reply-To: <1411753981.2739.16.camel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
On Fri, Sep 26, 2014 at 01:53:01PM -0400, Eric Paris wrote:
> Would you like me to carry this in the audit tree, since I'm the one who
> broke it?
>
Hi Eric,
guess you are asking Jonas, but it would make sense to me.
Guenter
> -Eric
>
> On Fri, 2014-09-26 at 09:05 -0700, Guenter Roeck wrote:
> > openrisc:defconfig fails to build in next-20140926 with the following error.
> >
> > In file included from arch/openrisc/kernel/signal.c:31:0:
> > ./arch/openrisc/include/asm/syscall.h: In function 'syscall_get_arch':
> > ./arch/openrisc/include/asm/syscall.h:77:9: error: 'EM_OPENRISC' undeclared
> >
> > Fix by moving EM_OPENRISC to include/uapi/linux/elf-em.h.
> >
> > Fixes: ce5d112827e5 ("ARCH: AUDIT: implement syscall_get_arch for all arches")
> > Cc: Eric Paris <eparis-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > Cc: Stefan Kristiansson <stefan.kristiansson-MbMCFXIvDHJFcC0YU169RA@public.gmane.org>
> > Cc: Geert Uytterhoeven <geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>
> > Cc: Stephen Rothwell <sfr-3FnU+UHB4dNDw9hX6IcOSA@public.gmane.org>
> > Signed-off-by: Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>
> > ---
> > v2: Only move EM_OPENRISC.
> >
> > Another possible solution for the problem would be to include asm/elf.h
> > in arch/openrisc/kernel/signal.c. I had actually submitted a patch with
> > that fix back in August (maybe that is where I remembered the problem from).
> > Wonder what happened with that patch.
> >
> > Would it make sense to drop EM_OR32 and replace it with EM_OPENRISC where
> > it is used ? binutils seems to suggest that EM_OPENRISC is the "official"
> > definition.
> >
> > arch/openrisc/include/uapi/asm/elf.h | 3 +--
> > include/uapi/linux/elf-em.h | 1 +
> > 2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/openrisc/include/uapi/asm/elf.h b/arch/openrisc/include/uapi/asm/elf.h
> > index f02ea58..8884276 100644
> > --- a/arch/openrisc/include/uapi/asm/elf.h
> > +++ b/arch/openrisc/include/uapi/asm/elf.h
> > @@ -55,9 +55,8 @@ typedef elf_greg_t elf_gregset_t[ELF_NGREG];
> > /* A placeholder; OR32 does not have fp support yes, so no fp regs for now. */
> > typedef unsigned long elf_fpregset_t;
> >
> > -/* This should be moved to include/linux/elf.h */
> > +/* EM_OPENRISC is defined in linux/elf-em.h */
> > #define EM_OR32 0x8472
> > -#define EM_OPENRISC 92 /* OpenRISC 32-bit embedded processor */
> >
> > /*
> > * These are used to set parameters in the core dumps.
> > diff --git a/include/uapi/linux/elf-em.h b/include/uapi/linux/elf-em.h
> > index 01529bd..aa90bc9 100644
> > --- a/include/uapi/linux/elf-em.h
> > +++ b/include/uapi/linux/elf-em.h
> > @@ -32,6 +32,7 @@
> > #define EM_V850 87 /* NEC v850 */
> > #define EM_M32R 88 /* Renesas M32R */
> > #define EM_MN10300 89 /* Panasonic/MEI MN10300, AM33 */
> > +#define EM_OPENRISC 92 /* OpenRISC 32-bit embedded processor */
> > #define EM_BLACKFIN 106 /* ADI Blackfin Processor */
> > #define EM_TI_C6000 140 /* TI C6X DSPs */
> > #define EM_AARCH64 183 /* ARM 64 bit */
>
>
^ permalink raw reply
* Re: [PATCH v2] next: openrisc: Fix build
From: Guenter Roeck @ 2014-09-26 18:06 UTC (permalink / raw)
To: Jonas Bonn
Cc: linux-ZwoEplunGu2h2975yA52AaxOck334EZe,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-next-u79uwXL29TY76Z2rM5mHXA, Eric Paris,
Stefan Kristiansson, Geert Uytterhoeven, Stephen Rothwell
In-Reply-To: <5425A7B5.10209-A9uVI2HLR7kOP4wsBPIw7w@public.gmane.org>
On Fri, Sep 26, 2014 at 07:51:49PM +0200, Jonas Bonn wrote:
> On 09/26/2014 06:05 PM, Guenter Roeck wrote:
> > openrisc:defconfig fails to build in next-20140926 with the following error.
> >
> > In file included from arch/openrisc/kernel/signal.c:31:0:
> > ./arch/openrisc/include/asm/syscall.h: In function 'syscall_get_arch':
> > ./arch/openrisc/include/asm/syscall.h:77:9: error: 'EM_OPENRISC' undeclared
> >
> > Fix by moving EM_OPENRISC to include/uapi/linux/elf-em.h.
> >
> > Fixes: ce5d112827e5 ("ARCH: AUDIT: implement syscall_get_arch for all arches")
> > Cc: Eric Paris <eparis-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > Cc: Stefan Kristiansson <stefan.kristiansson-MbMCFXIvDHJFcC0YU169RA@public.gmane.org>
> > Cc: Geert Uytterhoeven <geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>
> > Cc: Stephen Rothwell <sfr-3FnU+UHB4dNDw9hX6IcOSA@public.gmane.org>
> > Signed-off-by: Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>
> > ---
> > v2: Only move EM_OPENRISC.
> >
> > Another possible solution for the problem would be to include asm/elf.h
> > in arch/openrisc/kernel/signal.c. I had actually submitted a patch with
> > that fix back in August (maybe that is where I remembered the problem from).
> > Wonder what happened with that patch.
> >
> > Would it make sense to drop EM_OR32 and replace it with EM_OPENRISC where
> > it is used ? binutils seems to suggest that EM_OPENRISC is the "official"
> > definition.
>
> Do we even use EM_OR32? Will the kernel build with the old toolchain if
> we drop it? If yes, drop it altogether... I don't recall the details as
> to why we kept that define around at all. And really, why bother
> supporting the old toolchain at all... it's been at least two or three
> years since EM_OPENRISC was added, hopefully people have moved on. If
> users want to upgrade their kernel, they can update the toolchain, too,
> at this point.
>
EM_OR32 is used to define ELF_ARCH, which per its comment is used in core
dumps. No idea if that is important/relevant. You tell me ;-).
The kernel builds with both the old (or32) and new (or1k) toolchain after
removing EM_OR32 and defining ELF_ARCH as EM_OPENRISC. I can not test the
kernel built with the or32 toolchain, but qemu is happy with the image built
using the or1k toolchain.
I'll be happy to submit a patch to remove EM_OR32 if it is not needed anymore.
Thanks,
Guenter
^ permalink raw reply
* Re: [PATCH v2] next: openrisc: Fix build
From: Eric Paris @ 2014-09-26 17:53 UTC (permalink / raw)
To: Guenter Roeck
Cc: Jonas Bonn, linux, linux-kernel, linux-api, linux-next,
Stefan Kristiansson, Geert Uytterhoeven, Stephen Rothwell
In-Reply-To: <1411747539-28308-1-git-send-email-linux@roeck-us.net>
Would you like me to carry this in the audit tree, since I'm the one who
broke it?
-Eric
On Fri, 2014-09-26 at 09:05 -0700, Guenter Roeck wrote:
> openrisc:defconfig fails to build in next-20140926 with the following error.
>
> In file included from arch/openrisc/kernel/signal.c:31:0:
> ./arch/openrisc/include/asm/syscall.h: In function 'syscall_get_arch':
> ./arch/openrisc/include/asm/syscall.h:77:9: error: 'EM_OPENRISC' undeclared
>
> Fix by moving EM_OPENRISC to include/uapi/linux/elf-em.h.
>
> Fixes: ce5d112827e5 ("ARCH: AUDIT: implement syscall_get_arch for all arches")
> Cc: Eric Paris <eparis@redhat.com>
> Cc: Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>
> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> Cc: Stephen Rothwell <sfr@canb.auug.org.au>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> v2: Only move EM_OPENRISC.
>
> Another possible solution for the problem would be to include asm/elf.h
> in arch/openrisc/kernel/signal.c. I had actually submitted a patch with
> that fix back in August (maybe that is where I remembered the problem from).
> Wonder what happened with that patch.
>
> Would it make sense to drop EM_OR32 and replace it with EM_OPENRISC where
> it is used ? binutils seems to suggest that EM_OPENRISC is the "official"
> definition.
>
> arch/openrisc/include/uapi/asm/elf.h | 3 +--
> include/uapi/linux/elf-em.h | 1 +
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/openrisc/include/uapi/asm/elf.h b/arch/openrisc/include/uapi/asm/elf.h
> index f02ea58..8884276 100644
> --- a/arch/openrisc/include/uapi/asm/elf.h
> +++ b/arch/openrisc/include/uapi/asm/elf.h
> @@ -55,9 +55,8 @@ typedef elf_greg_t elf_gregset_t[ELF_NGREG];
> /* A placeholder; OR32 does not have fp support yes, so no fp regs for now. */
> typedef unsigned long elf_fpregset_t;
>
> -/* This should be moved to include/linux/elf.h */
> +/* EM_OPENRISC is defined in linux/elf-em.h */
> #define EM_OR32 0x8472
> -#define EM_OPENRISC 92 /* OpenRISC 32-bit embedded processor */
>
> /*
> * These are used to set parameters in the core dumps.
> diff --git a/include/uapi/linux/elf-em.h b/include/uapi/linux/elf-em.h
> index 01529bd..aa90bc9 100644
> --- a/include/uapi/linux/elf-em.h
> +++ b/include/uapi/linux/elf-em.h
> @@ -32,6 +32,7 @@
> #define EM_V850 87 /* NEC v850 */
> #define EM_M32R 88 /* Renesas M32R */
> #define EM_MN10300 89 /* Panasonic/MEI MN10300, AM33 */
> +#define EM_OPENRISC 92 /* OpenRISC 32-bit embedded processor */
> #define EM_BLACKFIN 106 /* ADI Blackfin Processor */
> #define EM_TI_C6000 140 /* TI C6X DSPs */
> #define EM_AARCH64 183 /* ARM 64 bit */
^ permalink raw reply
* Re: [PATCH v2] next: openrisc: Fix build
From: Jonas Bonn @ 2014-09-26 17:51 UTC (permalink / raw)
To: Guenter Roeck
Cc: Stephen Rothwell, linux-api, linux-kernel, Eric Paris, linux-next,
linux
In-Reply-To: <1411747539-28308-1-git-send-email-linux@roeck-us.net>
On 09/26/2014 06:05 PM, Guenter Roeck wrote:
> openrisc:defconfig fails to build in next-20140926 with the following error.
>
> In file included from arch/openrisc/kernel/signal.c:31:0:
> ./arch/openrisc/include/asm/syscall.h: In function 'syscall_get_arch':
> ./arch/openrisc/include/asm/syscall.h:77:9: error: 'EM_OPENRISC' undeclared
>
> Fix by moving EM_OPENRISC to include/uapi/linux/elf-em.h.
>
> Fixes: ce5d112827e5 ("ARCH: AUDIT: implement syscall_get_arch for all arches")
> Cc: Eric Paris <eparis@redhat.com>
> Cc: Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>
> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> Cc: Stephen Rothwell <sfr@canb.auug.org.au>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> v2: Only move EM_OPENRISC.
>
> Another possible solution for the problem would be to include asm/elf.h
> in arch/openrisc/kernel/signal.c. I had actually submitted a patch with
> that fix back in August (maybe that is where I remembered the problem from).
> Wonder what happened with that patch.
>
> Would it make sense to drop EM_OR32 and replace it with EM_OPENRISC where
> it is used ? binutils seems to suggest that EM_OPENRISC is the "official"
> definition.
Do we even use EM_OR32? Will the kernel build with the old toolchain if
we drop it? If yes, drop it altogether... I don't recall the details as
to why we kept that define around at all. And really, why bother
supporting the old toolchain at all... it's been at least two or three
years since EM_OPENRISC was added, hopefully people have moved on. If
users want to upgrade their kernel, they can update the toolchain, too,
at this point.
/Jonas
>
> arch/openrisc/include/uapi/asm/elf.h | 3 +--
> include/uapi/linux/elf-em.h | 1 +
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/openrisc/include/uapi/asm/elf.h b/arch/openrisc/include/uapi/asm/elf.h
> index f02ea58..8884276 100644
> --- a/arch/openrisc/include/uapi/asm/elf.h
> +++ b/arch/openrisc/include/uapi/asm/elf.h
> @@ -55,9 +55,8 @@ typedef elf_greg_t elf_gregset_t[ELF_NGREG];
> /* A placeholder; OR32 does not have fp support yes, so no fp regs for now. */
> typedef unsigned long elf_fpregset_t;
>
> -/* This should be moved to include/linux/elf.h */
> +/* EM_OPENRISC is defined in linux/elf-em.h */
> #define EM_OR32 0x8472
> -#define EM_OPENRISC 92 /* OpenRISC 32-bit embedded processor */
>
> /*
> * These are used to set parameters in the core dumps.
> diff --git a/include/uapi/linux/elf-em.h b/include/uapi/linux/elf-em.h
> index 01529bd..aa90bc9 100644
> --- a/include/uapi/linux/elf-em.h
> +++ b/include/uapi/linux/elf-em.h
> @@ -32,6 +32,7 @@
> #define EM_V850 87 /* NEC v850 */
> #define EM_M32R 88 /* Renesas M32R */
> #define EM_MN10300 89 /* Panasonic/MEI MN10300, AM33 */
> +#define EM_OPENRISC 92 /* OpenRISC 32-bit embedded processor */
> #define EM_BLACKFIN 106 /* ADI Blackfin Processor */
> #define EM_TI_C6000 140 /* TI C6X DSPs */
> #define EM_AARCH64 183 /* ARM 64 bit */
^ permalink raw reply
* [PATCH v2] next: openrisc: Fix build
From: Guenter Roeck @ 2014-09-26 16:05 UTC (permalink / raw)
To: Jonas Bonn
Cc: linux-ZwoEplunGu2h2975yA52AaxOck334EZe,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-next-u79uwXL29TY76Z2rM5mHXA, Guenter Roeck, Eric Paris,
Stefan Kristiansson, Geert Uytterhoeven, Stephen Rothwell
openrisc:defconfig fails to build in next-20140926 with the following error.
In file included from arch/openrisc/kernel/signal.c:31:0:
./arch/openrisc/include/asm/syscall.h: In function 'syscall_get_arch':
./arch/openrisc/include/asm/syscall.h:77:9: error: 'EM_OPENRISC' undeclared
Fix by moving EM_OPENRISC to include/uapi/linux/elf-em.h.
Fixes: ce5d112827e5 ("ARCH: AUDIT: implement syscall_get_arch for all arches")
Cc: Eric Paris <eparis-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Cc: Stefan Kristiansson <stefan.kristiansson-MbMCFXIvDHJFcC0YU169RA@public.gmane.org>
Cc: Geert Uytterhoeven <geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>
Cc: Stephen Rothwell <sfr-3FnU+UHB4dNDw9hX6IcOSA@public.gmane.org>
Signed-off-by: Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>
---
v2: Only move EM_OPENRISC.
Another possible solution for the problem would be to include asm/elf.h
in arch/openrisc/kernel/signal.c. I had actually submitted a patch with
that fix back in August (maybe that is where I remembered the problem from).
Wonder what happened with that patch.
Would it make sense to drop EM_OR32 and replace it with EM_OPENRISC where
it is used ? binutils seems to suggest that EM_OPENRISC is the "official"
definition.
arch/openrisc/include/uapi/asm/elf.h | 3 +--
include/uapi/linux/elf-em.h | 1 +
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/openrisc/include/uapi/asm/elf.h b/arch/openrisc/include/uapi/asm/elf.h
index f02ea58..8884276 100644
--- a/arch/openrisc/include/uapi/asm/elf.h
+++ b/arch/openrisc/include/uapi/asm/elf.h
@@ -55,9 +55,8 @@ typedef elf_greg_t elf_gregset_t[ELF_NGREG];
/* A placeholder; OR32 does not have fp support yes, so no fp regs for now. */
typedef unsigned long elf_fpregset_t;
-/* This should be moved to include/linux/elf.h */
+/* EM_OPENRISC is defined in linux/elf-em.h */
#define EM_OR32 0x8472
-#define EM_OPENRISC 92 /* OpenRISC 32-bit embedded processor */
/*
* These are used to set parameters in the core dumps.
diff --git a/include/uapi/linux/elf-em.h b/include/uapi/linux/elf-em.h
index 01529bd..aa90bc9 100644
--- a/include/uapi/linux/elf-em.h
+++ b/include/uapi/linux/elf-em.h
@@ -32,6 +32,7 @@
#define EM_V850 87 /* NEC v850 */
#define EM_M32R 88 /* Renesas M32R */
#define EM_MN10300 89 /* Panasonic/MEI MN10300, AM33 */
+#define EM_OPENRISC 92 /* OpenRISC 32-bit embedded processor */
#define EM_BLACKFIN 106 /* ADI Blackfin Processor */
#define EM_TI_C6000 140 /* TI C6X DSPs */
#define EM_AARCH64 183 /* ARM 64 bit */
--
1.9.1
^ permalink raw reply related
* Re: [PATCH] next: openrisc: Fix build
From: Guenter Roeck @ 2014-09-26 15:48 UTC (permalink / raw)
To: Jonas Bonn
Cc: linux-ZwoEplunGu2h2975yA52AaxOck334EZe,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA, Eric Paris, Stefan Kristiansson,
Geert Uytterhoeven, Stephen Rothwell
In-Reply-To: <542573DF.3040405-A9uVI2HLR7kOP4wsBPIw7w@public.gmane.org>
On Fri, Sep 26, 2014 at 04:10:39PM +0200, Jonas Bonn wrote:
> On 09/26/2014 03:54 PM, Guenter Roeck wrote:
> > openrisc:defconfig fails to build in next-20140926 with the following error.
> >
> > In file included from arch/openrisc/kernel/signal.c:31:0:
> > ./arch/openrisc/include/asm/syscall.h: In function 'syscall_get_arch':
> > ./arch/openrisc/include/asm/syscall.h:77:9: error: 'EM_OPENRISC' undeclared
> >
> > Fix by moving EM_OPENRISC and EM_OR32 to include/uapi/linux/elf-em.h.
> >
> > Fixes: ce5d112827e5 ("ARCH: AUDIT: implement syscall_get_arch for all arches")
> > Cc: Eric Paris <eparis-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > Cc: Stefan Kristiansson <stefan.kristiansson-MbMCFXIvDHJFcC0YU169RA@public.gmane.org>
> > Cc: Geert Uytterhoeven <geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>
> > Cc: Stephen Rothwell <sfr-3FnU+UHB4dNDw9hX6IcOSA@public.gmane.org>
> > Signed-off-by: Guenter Roeck <linux-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>
> > ---
> > Based Stefan's note. No idea if it is the correct fix.
> >
> > arch/openrisc/include/uapi/asm/elf.h | 4 +---
> > include/uapi/linux/elf-em.h | 2 ++
> > 2 files changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/openrisc/include/uapi/asm/elf.h b/arch/openrisc/include/uapi/asm/elf.h
> > index f02ea58..e75fdce 100644
> > --- a/arch/openrisc/include/uapi/asm/elf.h
> > +++ b/arch/openrisc/include/uapi/asm/elf.h
> > @@ -55,9 +55,7 @@ typedef elf_greg_t elf_gregset_t[ELF_NGREG];
> > /* A placeholder; OR32 does not have fp support yes, so no fp regs for now. */
> > typedef unsigned long elf_fpregset_t;
> >
> > -/* This should be moved to include/linux/elf.h */
> > -#define EM_OR32 0x8472
> > -#define EM_OPENRISC 92 /* OpenRISC 32-bit embedded processor */
> > +/* EM_OPENRISC and EM_OR32 defined in linux/elf-em.h */
> >
> > /*
> > * These are used to set parameters in the core dumps.
> > diff --git a/include/uapi/linux/elf-em.h b/include/uapi/linux/elf-em.h
> > index 01529bd..c5d3948 100644
> > --- a/include/uapi/linux/elf-em.h
> > +++ b/include/uapi/linux/elf-em.h
> > @@ -32,11 +32,13 @@
> > #define EM_V850 87 /* NEC v850 */
> > #define EM_M32R 88 /* Renesas M32R */
> > #define EM_MN10300 89 /* Panasonic/MEI MN10300, AM33 */
> > +#define EM_OPENRISC 92 /* OpenRISC 32-bit embedded processor */
> > #define EM_BLACKFIN 106 /* ADI Blackfin Processor */
> > #define EM_TI_C6000 140 /* TI C6X DSPs */
> > #define EM_AARCH64 183 /* ARM 64 bit */
> > #define EM_FRV 0x5441 /* Fujitsu FR-V */
> > #define EM_AVR32 0x18ad /* Atmel AVR32 */
> > +#define EM_OR32 0x8472 /* OpenRISC, alternate */
> Have these codes hit upstream binutils yet? Did they both go upstream
> of only one of them? Is there any chance they are going to change
> again? I think that's the reason these have been lingering in
> arch/openrisc instead of the generic header.
>
EM_OPENRISC is there. EM_OR32 isn't.
I'll redo the patch and only move EM_OPENRISC to the common file.
Thanks,
Guenter
^ permalink raw reply
* Re: [PATCHv7 05/26] vfio: introduce the VFIO_DMA_MAP_FLAG_NOEXEC flag
From: Antonios Motakis @ 2014-09-26 15:45 UTC (permalink / raw)
To: Alex Williamson
Cc: KVM devel mailing list, Eric Auger, Marc Zyngier,
open list:ABI/API, Will Deacon, open list, Linux IOMMU,
VirtualOpenSystems Technical Team, kvm-arm, Christoffer Dall
In-Reply-To: <1411511426.24563.52.camel-85EaTFmN5p//9pzu0YdTqQ@public.gmane.org>
On Wed, Sep 24, 2014 at 12:30 AM, Alex Williamson
<alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> On Tue, 2014-09-23 at 16:21 -0600, Alex Williamson wrote:
>> On Tue, 2014-09-23 at 16:46 +0200, Antonios Motakis wrote:
>> > We introduce the VFIO_DMA_MAP_FLAG_NOEXEC flag to the VFIO dma map call,
>> > and expose its availability via the capability VFIO_IOMMU_PROT_NOEXEC.
>> > This way the user can control whether the XN flag will be set on the
>> > requested mappings. The IOMMU_NOEXEC flag needs to be available for all
>> > the IOMMUs of the container used.
>> >
>> > Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>
>> > ---
>> > include/uapi/linux/vfio.h | 2 ++
>> > 1 file changed, 2 insertions(+)
>> >
>> > diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
>> > index 6612974..30f630c 100644
>> > --- a/include/uapi/linux/vfio.h
>> > +++ b/include/uapi/linux/vfio.h
>> > @@ -29,6 +29,7 @@
>> > * capability is subject to change as groups are added or removed.
>> > */
>> > #define VFIO_DMA_CC_IOMMU 4
>> > +#define VFIO_IOMMU_PROT_NOEXEC 5
>>
>> Can't we advertise this as a flag bit in vfio_iommu_type1_info instead?
>
> Ok, I see in the next patch that it's pretty similar to
> VFIO_DMA_CC_IOMMU, so the check extension is probably correct for
> determining the current state. Maybe we could name it more similarly,
> VFIO_DMA_NOEXEC_IOMMU. I guess the intended usage is that once a user
> attaches a group to the container they can query whether the
> VFIO_DMA_MAP_FLAG_NOEXEC is valid. Ok. Thanks,
>
Sorry for the numbering, ack and ack :)
> Alex
>
>> Also, EEH already took 5 as seen immediately below.
>>
>> >
>> > /* Check if EEH is supported */
>> > #define VFIO_EEH 5
>> > @@ -401,6 +402,7 @@ struct vfio_iommu_type1_dma_map {
>> > __u32 flags;
>> > #define VFIO_DMA_MAP_FLAG_READ (1 << 0) /* readable from device */
>> > #define VFIO_DMA_MAP_FLAG_WRITE (1 << 1) /* writable from device */
>> > +#define VFIO_DMA_MAP_FLAG_NOEXEC (1 << 2) /* not executable from device */
>> > __u64 vaddr; /* Process virtual address */
>> > __u64 iova; /* IO virtual address */
>> > __u64 size; /* Size of mapping (bytes) */
>>
>>
>
>
>
--
Antonios Motakis
Virtual Open Systems
^ 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