* Re: [PATCH] net: optimise csum_replace4()
From: David Miller @ 2014-09-26 20:14 UTC (permalink / raw)
To: christophe.leroy; +Cc: linux-kernel, edumazet, netdev
In-Reply-To: <20140923085433.09C5A1AB26A@localhost.localdomain>
From: Christophe Leroy <christophe.leroy@c-s.fr>
Date: Tue, 23 Sep 2014 10:54:32 +0200 (CEST)
> csum_partial() is a generic function which is not optimised for small fixed
> length calculations, and its use requires to store "from" and "to" values in
> memory while we already have them available in registers. This also has impact,
> especially on RISC processors. In the same spirit as the change done by
> Eric Dumazet on csum_replace2(), this patch rewrites inet_proto_csum_replace4()
> taking into account RFC1624.
>
> I spotted during a NATted tcp transfert that csum_partial() is one of top 5
> consuming functions (around 8%), and the second user of csum_partial() is
> inet_proto_csum_replace4().
>
> I have proposed the same modification to inet_proto_csum_replace4() in another
> patch.
>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
Applied.
^ permalink raw reply
* Re: [PATCH net] r8152: fix the carrier off when autoresuming
From: David Miller @ 2014-09-26 20:13 UTC (permalink / raw)
To: hayeswang; +Cc: netdev, nic_swsd, linux-kernel, linux-usb
In-Reply-To: <1394712342-15778-45-Taiwan-albertk@realtek.com>
From: Hayes Wang <hayeswang@realtek.com>
Date: Tue, 23 Sep 2014 16:31:47 +0800
> netif_carrier_off would be called when autoresuming, even though
> the cable is plugged. This causes some applications do relative
> actions when detecting the carrier off. Keep the status of the
> carrier, and let it be modified when the linking change occurs.
>
> Signed-off-by: Hayes Wang <hayeswang@realtek.com>
Applied, thanks.
^ 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: [PATCH] Revert "net/macb: add pinctrl consumer support"
From: Sören Brinkmann @ 2014-09-26 20:06 UTC (permalink / raw)
To: David Miller; +Cc: nicolas.ferre, linux-kernel, netdev
In-Reply-To: <20140926.154930.1459165439870528648.davem@davemloft.net>
On Fri, 2014-09-26 at 03:49PM -0400, David Miller wrote:
> From: Soren Brinkmann <soren.brinkmann@xilinx.com>
> Date: Mon, 22 Sep 2014 16:49:08 -0700
>
> > This reverts commit 8ef29f8aae524bd51298fb10ac6a5ce6c4c5a3d8.
> > The driver core already calls pinctrl_get() and claims the default
> > state. There is no need to replicate this in the driver.
> > ---
> > Hi,
> >
> > I might be mistaken, but I think the driver core does already take care of
> > calling into the pinctrl framework and the driver does not need to do it on its
> > own (drivers/base/dd.c:really_probe() calls 'pinctrl_bind_pins() which takes
> > care of the pinctrl setup).
> >
>
> Applied and queued up for -stable, thanks.
thx
> Next time please provide a proper "Signed-off-by: " in the commit message
> of your submission.
Yeah, sorry for that. I overlooked that one. My alias for commit takes
care of this, but I don't have that for revert.
In case you need it:
Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
Sören
^ permalink raw reply
* Re: [PATCH v1 0/4] net: fec: Code cleanup
From: David Miller @ 2014-09-26 20:06 UTC (permalink / raw)
To: b38611; +Cc: netdev, b20596
In-Reply-To: <1411458058-18934-1-git-send-email-b38611@freescale.com>
From: Fugang Duan <b38611@freescale.com>
Date: Tue, 23 Sep 2014 15:40:54 +0800
> This patches does several things:
> - Fixing multiqueue issue.
> - Removing the unnecessary errata workaround.
> - Aligning the data buffer dma map/unmap size.
> - Freeing resource after probe failed.
Series applied to net-next, thanks.
Please always explicitly state what tree you expect your patches to be
applied to in the future.
^ permalink raw reply
* Re: [patch net-next v2 8/9] switchdev: introduce Netlink API
From: Or Gerlitz @ 2014-09-26 20:03 UTC (permalink / raw)
To: Thomas Graf
Cc: Or Gerlitz, Andy Gospodarek, Tom Herbert, Alexei Starovoitov,
Jiri Pirko, John Fastabend, Jamal Hadi Salim,
netdev@vger.kernel.org, David S. Miller, Neil Horman,
Andy Gospodarek, Daniel Borkmann, Jesse Gross, Pravin Shelar,
Andy Zhou, Ben Hutchings, Stephen Hemminger, Jeff Kirsher,
Vladislav Yasevich, Cong Wang, Eric Dumazet, Scott Feldman
In-Reply-To: <20140924133246.GB4966@casper.infradead.org>
On Wed, Sep 24, 2014 at 4:32 PM, Thomas Graf <tgraf@suug.ch> wrote:
> On 09/23/14 at 06:32pm, Or Gerlitz wrote:
>> Indeed.
>>
>> The idea is to leverage OVS to manage eSwitch (embedded NIC switch) as well
>> (NOT to offload OVS).
>>
>> We envision a seamless integration of user environment which is based on OVS
>> with SRIOV eSwitch and the grounds for that were very well supported in
>> Jiri’s V1.
>
> Please consider comparing your model with what is described here [0].
> I'm trying to write down an architecture document that we can finalize
> in Düsseldorf.
> [0] http://goo.gl/qkzW5y
Yep, this can serve us for the architecture discussion @ LPC. Re the
SRIOV case, you referred to the case where guest VF traffic goes
through HW (say) VXLAN encap/decap -- just to make sure, we need also
to support the simpler case, where guest traffic just goes through
vlan tag/strip.
>> The eSwitch hardware does not need to have multiple tables and ‘enjoys’ the
>> flat rule of OVS. The kernel datapath does not need to be aware of the
>> existence of HW nor its capabilities, it just pushes the flow also to the
>> switchdev which represents the eSwitch.
> I think you are saying that the kernel should not be required to make
> the offload decision which is fair. We definitely don't want to force
> the decision to be outside though, there are several legit reasons to
> support transparent offloads within the kernel as well outside of OVS.
>
>> Yep. LPC is the time and place to go over the multiple use-cases (phyiscal
>> switch, eSwitch, eBPF, etc) that could (should) be supported by the basic
>> framework.
>
> For reference:
> http://www.linuxplumbersconf.org/2014/ocw/proposals/2463
The SRIOV case is only mentioned here in the "Compatibility with
existing FDB ioctls for SR-IOV" bullet, so I'm a bit nervous... we
need to have it clear in the agenda. Also, this BoF needs to be
double-len, two hours, can you act to get that done?
thanks,
Or.
^ 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: [PATCH net-next 1/5] udp: Generalize skb_udp_segment
From: Tom Herbert @ 2014-09-26 19:56 UTC (permalink / raw)
To: Alexei Starovoitov; +Cc: David S. Miller, netdev@vger.kernel.org
In-Reply-To: <CAADnVQJgoW1SbkK6hoJcRtTwyKxgNBtFYqBF-5aW9D4=Zi5ofg@mail.gmail.com>
On Fri, Sep 26, 2014 at 12:39 PM, Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
> On Fri, Sep 26, 2014 at 9:22 AM, Tom Herbert <therbert@google.com> wrote:
>> skb_udp_segment is the function called from udp4_ufo_fragment to
>> segment a UDP tunnel packet. This function currently assumes
>> segmentation is transparent Ethernet bridging (i.e. VXLAN
>> encapsulation). This patch generalizes the function to
>> operate on either Ethertype or IP protocol.
>
> this is great generalization.
> Do you envision that type_ether will cover vlan and mpls, right?
This design should allow providing GSO for any encapsulation of IP
protocol or Ethertype of UDP (as long as encapsulation headers don't
need to be modified for each segment). This actually covers quite a
lot of possibilities.
> What if somebody crazy adds a new thing after udp encap
> that is neither ethernet nor ip header?
> Like encryption header?
We could add an internal offloads table for that. This would entail a
new inner protocol type and then use inner_protocol to set offload
flavor which we can define. This also might be the path to some sort
of GSO for application layer protocols (like QUIC maybe...?)
> I think it would fit this model well...
^ permalink raw reply
* Re: [PATCH net-next] tcp: abort orphan sockets stalling on zero window probes
From: David Miller @ 2014-09-26 19:56 UTC (permalink / raw)
To: ycheng; +Cc: edumazet, andrey.dmitrov, ncardwell, netdev
In-Reply-To: <1411444333-6013-1-git-send-email-ycheng@google.com>
From: Yuchung Cheng <ycheng@google.com>
Date: Mon, 22 Sep 2014 20:52:13 -0700
> -
> + if (!alive && icsk->icsk_backoff >= max_probes)
> + goto abort;
> if (tcp_out_of_resources(sk, alive || icsk->icsk_probes_out <= max_probes))
> return;
This new logic means that the second argument to tcp_out_of_resources()
will always be 'true'.
Please make that explicit, because the code is confusing otherwise.
Also perhaps target this for the 'net' tree?
Thanks.
^ 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: [PATCH] Revert "net/macb: add pinctrl consumer support"
From: David Miller @ 2014-09-26 19:49 UTC (permalink / raw)
To: soren.brinkmann; +Cc: nicolas.ferre, linux-kernel, netdev
In-Reply-To: <1411429748-31444-1-git-send-email-soren.brinkmann@xilinx.com>
From: Soren Brinkmann <soren.brinkmann@xilinx.com>
Date: Mon, 22 Sep 2014 16:49:08 -0700
> This reverts commit 8ef29f8aae524bd51298fb10ac6a5ce6c4c5a3d8.
> The driver core already calls pinctrl_get() and claims the default
> state. There is no need to replicate this in the driver.
> ---
> Hi,
>
> I might be mistaken, but I think the driver core does already take care of
> calling into the pinctrl framework and the driver does not need to do it on its
> own (drivers/base/dd.c:really_probe() calls 'pinctrl_bind_pins() which takes
> care of the pinctrl setup).
>
Applied and queued up for -stable, thanks.
Next time please provide a proper "Signed-off-by: " in the commit message
of your submission.
^ 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
* Re: [PATCH net-next] net: introduce __skb_header_release()
From: David Miller @ 2014-09-26 19:40 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <1411428572.26859.165.camel@edumazet-glaptop2.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 22 Sep 2014 16:29:32 -0700
> From: Eric Dumazet <edumazet@google.com>
>
> While profiling TCP stack, I noticed one useless atomic operation
> in tcp_sendmsg(), caused by skb_header_release().
>
> It turns out all current skb_header_release() users have a fresh skb,
> that no other user can see, so we can avoid one atomic operation.
>
> Introduce __skb_header_release() to clearly document this.
>
> This gave me a 1.5 % improvement on TCP_RR workload.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Looks great, applied, thanks!
^ permalink raw reply
* Re: [PATCH net-next 1/5] udp: Generalize skb_udp_segment
From: Alexei Starovoitov @ 2014-09-26 19:39 UTC (permalink / raw)
To: Tom Herbert; +Cc: David S. Miller, netdev@vger.kernel.org
On Fri, Sep 26, 2014 at 9:22 AM, Tom Herbert <therbert@google.com> wrote:
> skb_udp_segment is the function called from udp4_ufo_fragment to
> segment a UDP tunnel packet. This function currently assumes
> segmentation is transparent Ethernet bridging (i.e. VXLAN
> encapsulation). This patch generalizes the function to
> operate on either Ethertype or IP protocol.
this is great generalization.
Do you envision that type_ether will cover vlan and mpls, right?
What if somebody crazy adds a new thing after udp encap
that is neither ethernet nor ip header?
Like encryption header?
I think it would fit this model well...
^ 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 net-next v3] fec: Remove fec_enet_select_queue()
From: David Miller @ 2014-09-26 19:22 UTC (permalink / raw)
To: festevam; +Cc: Frank.Li, netdev, fabio.estevam
In-Reply-To: <1411419329-11957-1-git-send-email-festevam@gmail.com>
From: Fabio Estevam <festevam@gmail.com>
Date: Mon, 22 Sep 2014 17:55:29 -0300
> From: Fabio Estevam <fabio.estevam@freescale.com>
>
> Sparse complains about fec_enet_select_queue() not being static.
>
> Feedback from David Miller [1] was to remove this function instead of making it
> static:
>
> "Please just delete this function.
>
> It's overriding code which does exactly the same thing.
>
> Actually, more precisely, this code is duplicating code in a way that
> bypasses many core facilitites of the networking. For example, this
> override means that socket based flow steering, XPS, etc. are all
> not happening on these devices.
>
> Without ->ndo_select_queue(), the flow dissector does __netdev_pick_tx
> which is exactly what you want to happen."
>
> [1] http://www.spinics.net/lists/netdev/msg297653.html
>
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
Looks a lot better, applied, thanks!
^ permalink raw reply
* Re: [PATCH] macvtap: Fix race between device delete and open.
From: David Miller @ 2014-09-26 19:21 UTC (permalink / raw)
To: vyasevich; +Cc: netdev, vyasevic, mst, jasowang
In-Reply-To: <1411418057-18937-1-git-send-email-vyasevic@redhat.com>
From: Vladislav Yasevich <vyasevich@gmail.com>
Date: Mon, 22 Sep 2014 16:34:17 -0400
> In macvtap device delete and open calls can race and
> this causes a list curruption of the vlan queue_list.
>
> The race intself is triggered by the idr accessors
> that located the vlan device. The device is stored
> into and removed from the idr under both an rtnl and
> a mutex. However, when attempting to locate the device
> in idr, only a mutex is taken. As a result, once cpu
> perfoming a delete may take an rtnl and wait for the mutex,
> while another cput doing an open() will take the idr
> mutex first to fetch the device pointer and later take
> an rtnl to add a queue for the device which may have
> just gotten deleted.
>
> With this patch, we now hold the rtnl for the duration
> of the macvtap_open() call thus making sure that
> open will not race with delete.
>
> CC: Michael S. Tsirkin <mst@redhat.com>
> CC: Jason Wang <jasowang@redhat.com>
> Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
Applied and queued up for -stable, thanks!
^ permalink raw reply
* Re: pull request: wireless-next 2014-09-22
From: David Miller @ 2014-09-26 19:19 UTC (permalink / raw)
To: linville; +Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <20140922194512.GD23806@tuxdriver.com>
From: "John W. Linville" <linville@tuxdriver.com>
Date: Mon, 22 Sep 2014 15:45:13 -0400
> Please pull this batch of updates intended for the 3.18 stream...
Pulled, thanks so much John.
^ permalink raw reply
* Re: [PATCH net-next] net: Change netdev_<level> logging functions to return void
From: David Miller @ 2014-09-26 19:17 UTC (permalink / raw)
To: joe; +Cc: netdev, linux-kernel
In-Reply-To: <1411409450.2952.55.camel@joe-AO725>
From: Joe Perches <joe@perches.com>
Date: Mon, 22 Sep 2014 11:10:50 -0700
> No caller or macro uses the return value so make all
> the functions return void.
>
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
> This change is associated to a desire to eventually
> change printk to return void.
Also applied, thanks Joe.
^ permalink raw reply
* Re: [PATCH net-next] mellanox: Change en_print to return void
From: David Miller @ 2014-09-26 19:17 UTC (permalink / raw)
To: joe; +Cc: amirv, netdev, linux-kernel
In-Reply-To: <1411407622.2952.49.camel@joe-AO725>
From: Joe Perches <joe@perches.com>
Date: Mon, 22 Sep 2014 10:40:22 -0700
> No caller or macro uses the return value so make it void.
>
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
> This change is associated to a desire to eventually
> change printk to return void.
Applied, thanks.
^ 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 net 0/4] qlcnic: Bug fixes.
From: David Miller @ 2014-09-26 19:12 UTC (permalink / raw)
To: manish.chopra; +Cc: netdev, Dept-GELinuxNICDev
In-Reply-To: <1411379513-26902-1-git-send-email-manish.chopra@qlogic.com>
From: Manish Chopra <manish.chopra@qlogic.com>
Date: Mon, 22 Sep 2014 05:51:49 -0400
> This patch series contains following bug fixes:
>
> * Fixes related to ethtool statistics.
> * Fix for flash read related API.
>
> Please apply this series to 'net'.
Series applied, thanks.
^ 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
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