Netdev List
 help / color / mirror / Atom feed
* Re: eBPF verifier thoughts (Re: [PATCH v15 net-next 00/11] eBPF syscall, verifier, testsuite)
From: Alexei Starovoitov @ 2014-09-26 22:26 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: <CALCETrWcmZVmRZARM_s9S-zZ4Xm_TcxRrzJWGwL0O9ySmVgr5A@mail.gmail.com>

On Fri, Sep 26, 2014 at 3:07 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> On Fri, Sep 26, 2014 at 3:03 PM, Alexei Starovoitov <ast@plumgrid.com> wrote:
>> On Fri, Sep 26, 2014 at 2:47 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>>>
>>> Can't you just disallow the 1-byte write to the stack?
>>
>> of course not.
>> That would be extremely limiting to users.
>> Can you actually see yourself living with stack that only
>> allows 8-byte writes/reads?
>> The stack usage will increase a lot, since all char/short
>> stack variables will become 8-byte...
>
> How about requiring that sub-8-byte stack accesses only be to integer slots?

you mean to reject the sub-8-byte write early if it's going
into space where pointers were stored?
That will limit stack reuse.
gcc/llvm generate code where the same stack location
is used by different variables during life of the function.
So if I reject the write early, it will break otherwise valid
programs.

^ permalink raw reply

* VRFs and the scalability of namespaces
From: David Ahern @ 2014-09-26 22:37 UTC (permalink / raw)
  To: Eric W. Biederman; +Cc: nicolas.dichtel, netdev@vger.kernel.org

Hi Eric:

As you suggested [1] I am starting a new thread to discuss scalability 
problems using namespaces for VRFs.

Background
----------
Consider a single system that wants to provide VRF-based features with 
support for N VRFs. N could easily be 2048 (e.g., 6Wind, [2]), 4000 
(e.g., Cisco, [3]) or even higher.

The single system with support for N VRFs runs M services (e.g., quagga, 
cdp, lldp, stp, strongswan, some homegrown routing protocol) and 
includes standard system services like sshd. Furthermore, a system also 
includes monitoring programs like snmpd and tcollector. In short, M is 
easily 20 processes that need to have a presence across all VRFs.


Network Namespaces for VRFs
---------------------------
For the past 4 years or so the response to VRF questions is a drum beat 
of "use network namespaces". But namespaces are not a good match for VRFs.

1. Network namespaces are a complete separation of the networking stack 
from network devices up. VRFs are an L3 concept. Using namespaces forces 
an L3 separation concept onto L2 apps -- lldp, cdp, etc.

There are use cases when you want device level separation, use cases 
where you want only L3 and up separation, and cases where you want both 
(e.g., divy up the netdevices in a system across some small number of 
namespaces and then provide VRF based features within a namespace).


2. Scalability of apps providing service as namespaces are created. How 
do you create the presence for each service in a network namespace?

a. Spawn a new process for each namespace? brute force approach and 
extremely resource intensive. e.g., the quagga example [4]

b. spawn a thread for each namespace? Better than a full process but 
still a heavyweight solution

c. create a socket per namespace. Better but still this is a resource 
intensive solution -- N listen sockets per service and each service 
needs to be modified for namespace support. For opensource software that 
means each project has to agree that namespace awareness is relevant and 
agree to take the patches.


3. Just creating a network namespace consumes non-negligible amount of 
memory -- ~200kB for the 3.10 kernel. I believe the /proc entries are 
the bulk of that memory usage. 200kB/namespace is again a lot of wasted 
memory and overhead.


4. For a single process to straddle multiple namespaces it has to run 
with full root privileges -- CAP_SYS_ADMIN -- to use setns. Using 
network sockets does not require a process to run as root at all unless 
it wants privileged ports in which case CAP_NET_BIND_SERVICE is 
sufficient, not full root.


The Linux kernel needs proper VRF support -- as an L3 concept. A 
capability to run a process in a "VRF any" context provides a resource 
efficient solution where a single process with a single listen socket 
works across all VRFs in a namespace and then connected sockets have a 
specific VRF context.

Before droning on even more, does the above provide better context on 
the general problem?

Thanks,
David


[1] https://lkml.org/lkml/2014/9/26/840

[2] http://www.6wind.com/6windgate-performance/ip-forwarding

[3] 
http://www.cisco.com/c/en/us/td/docs/switches/datacenter/sw/verified_scalability/b_Cisco_Nexus_7000_Series_NX-OS_Verified_Scalability_Guide.html

[4] 
https://lists.quagga.net/pipermail/quagga-users/2010-February/011351.html

^ permalink raw reply

* Re: eBPF verifier thoughts (Re: [PATCH v15 net-next 00/11] eBPF syscall, verifier, testsuite)
From: Andy Lutomirski @ 2014-09-26 22:41 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@vger.kernel.org
In-Reply-To: <CAMEtUuzQF+3uWFWmj5nSdYoJfo0W2F0wkvoyc4pP--EH3O_jYA@mail.gmail.com>

On Fri, Sep 26, 2014 at 3:26 PM, Alexei Starovoitov <ast@plumgrid.com> wrote:
> On Fri, Sep 26, 2014 at 3:07 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>> On Fri, Sep 26, 2014 at 3:03 PM, Alexei Starovoitov <ast@plumgrid.com> wrote:
>>> On Fri, Sep 26, 2014 at 2:47 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>>>>
>>>> Can't you just disallow the 1-byte write to the stack?
>>>
>>> of course not.
>>> That would be extremely limiting to users.
>>> Can you actually see yourself living with stack that only
>>> allows 8-byte writes/reads?
>>> The stack usage will increase a lot, since all char/short
>>> stack variables will become 8-byte...
>>
>> How about requiring that sub-8-byte stack accesses only be to integer slots?
>
> you mean to reject the sub-8-byte write early if it's going
> into space where pointers were stored?
> That will limit stack reuse.
> gcc/llvm generate code where the same stack location
> is used by different variables during life of the function.
> So if I reject the write early, it will break otherwise valid
> programs.

I think that a sub-8-byte write to an integer slot should leave it as
an integer and a sub-8-byte write to a non-integer slot should turn
that slot into an integer (if conversions to integer are permitted) or
be rejected otherwise.  gcc/llvm could emit an 8-byte write first, as
needed, to make this valid.

Alternatively, an integer stack slot could have a bitmask indicating
which bytes are valid.

--Andy

-- 
Andy Lutomirski
AMA Capital Management, LLC

^ permalink raw reply

* Re: tc rsvp filter show broke
From: Jamal Hadi Salim @ 2014-09-26 22:48 UTC (permalink / raw)
  To: John Fastabend; +Cc: Cong Wang, netdev
In-Reply-To: <5425A545.1040907@gmail.com>

On 09/26/14 13:41, John Fastabend wrote:
> Hi Jamal,
>
> Here is another strange 'tc' result that I think is a
> bug. If it was intended I can't see why. Show filter
> commands on rsvp classifiers only list the first entry?
>

Hrm  - poking inside there and this seems to be one of those
classifiers that hasnt been getting a lot of love. Doesnt
support actions well for example.

It seems to work for me:
----------
/sbin/tc -V
tc utility, iproute2-ss140610

uname -a
Linux jhs-1 3.16.0-rc2+ #4 SMP Fri Aug 29 11:13:53 EDT 2014 x86_64 
x86_64 x86_64 GNU/Linux

sudo /sbin/tc qdisc add dev eth0 root handle 1:0 prio
sudo /sbin/tc filter add dev eth0 pref 10 proto ip parent 1:0 rsvp 
session 10.0.0.1 ipproto icmp classid 1:1  police rate 1kbit burst 90k 
conform-exceed drop/ok
/sbin/tc -s filter show dev eth0 parent 1:0
filter protocol ip pref 10 rsvp
filter protocol ip pref 10 rsvp fh 0x0001100a flowid 1:1 session 
10.0.0.1 ipproto icmp
  Sent 0 bytes 0 pkts (dropped 0, overlimits 0)

Now lets send a fast ping:
sudo ping -f 10.0.0.1 -c 10000
.......
10000 packets transmitted, 386 received, 96% packet loss, time 115495ms
rtt min/avg/max/mdev = 0.300/0.499/1.262/0.164 ms, ipg/ewma 11.550/0.463 ms

/sbin/tc -s filter show dev eth0 parent 1:0
filter protocol ip pref 10 rsvp
filter protocol ip pref 10 rsvp fh 0x0001100a flowid 1:1 session 
10.0.0.1 ipproto icmp
  Sent 980000 bytes 10000 pkts (dropped 9614, overlimits 9614)

cheers,
jamal

^ permalink raw reply

* Re: [PATCH net-next 0/5] udp: Generalize GSO for UDP tunnels
From: Tom Herbert @ 2014-09-26 23:04 UTC (permalink / raw)
  To: Or Gerlitz; +Cc: David Miller, Linux Netdev List
In-Reply-To: <CAJ3xEMjVx6-XK9Eb2akLHei_43pw10rnHM-zXrOD7pzoXuXaRg@mail.gmail.com>

On Fri, Sep 26, 2014 at 1:16 PM, Or Gerlitz <gerlitz.or@gmail.com> wrote:
> On Fri, Sep 26, 2014 at 7:22 PM, Tom Herbert <therbert@google.com> wrote:
> [...]
>> Notes:
>>   - GSO for GRE/UDP where GRE checksum is enabled does not work.
>>     Handling this will require some special case code.
>>   - Software GSO now supports many varieties of encapsulation with
>>     SKB_GSO_UDP_TUNNEL{_CSUM}. We still need a mechanism to query
>>     for device support of particular combinations (I intend to
>>     add ndo_gso_check for that).
>
> Tom,
>
> As I wrote you earlier on another thread/s, fact is that there are
> upstream drivers who advertize SKB_GSO_UDP_TUNNEL and aren't capable @
> this point to issue proper HW segmentation of something which isn't
> VXLAN.
>
> Just to make sure, this series isn't expected to introduce a
> regression, right? we don't expect the stack to attempt and xmit a
> large 64KB UDP packet which isn't vxlan through these devices.
>
I am planning to post ndo_gso_check shortly. These patches should not
cause a regression with currently deployed functionality (VXLAN).

>
>
>>   - MPLS seems to be the only previous user of inner_protocol. I don't
>>     believe these patches can affect that. For supporting GSO with
>>     MPLS over UDP, the inner_protocol should be set using the
>>     helper functions in this patch.
>>   - GSO for L2TP/UDP should also be straightforward now.
>
>> Tested GRE, IPIP, and SIT over fou as well as VLXAN. This was
>> done using 200 TCP_STREAMs in netperf.
> [...]
>>    VXLAN
>>       TCP_STREAM TSO enabled on tun interface
>>         16.42% TX CPU utilization
>>         23.66% RX CPU utilization
>>         9081 Mbps
>>       TCP_STREAM TSO disabled on tun interface
>>         30.32% TX CPU utilization
>>         30.55% RX CPU utilization
>>         9185 Mbps
>
> so TSO disabled has better BW vs TSO enabled?
>
Yes, I've noticed that on occasion, it does seem like TSO disabled
tends to get a little more throughput. I see this with plain GRE, so I
don't think it's directly related to fou or these patches. I suppose
there may be some subtle interactions with BQL or something like that.
I'd probably want to repro this on some other devices at some point to
dig deeper.

>>    Baseline (no encp, TSO and LRO enabled)
>>       TCP_STREAM
>>         11.85% TX CPU utilization
>>         15.13% RX CPU utilization
>>         9452 Mbps
>
> I would strongly recommend to have a far better baseline when
> developing and testing these changes in the stack in the form of 40Gbs
> NICs.
>
The only point of putting the baseline was to show that encapsulation
with GSO/GRO/checksum-unnec-conversion is in the ballpark of
performance with native traffic which was a goal. So I'm pretty happy
with this performance right now, although it probably does mean remote
checksum offload won't show so impressive results with this test (TX
csum with data in case isn't so expensive).

Out of curiosity, why do you think using 40Gbs is far better for a baseline?

> Or.
>
>
>>
>> Tom Herbert (5):
>>   udp: Generalize skb_udp_segment
>>   sit: Set inner IP protocol in sit
>>   ipip: Set inner IP protocol in ipip
>>   gre: Set inner protocol in v4 and v6 GRE transmit
>>   vxlan: Set inner protocol before transmit
>>
>>  drivers/net/vxlan.c    |  4 ++++
>>  include/linux/skbuff.h | 26 +++++++++++++++++++++++--
>>  include/net/udp.h      |  3 ++-
>>  net/core/skbuff.c      |  1 +
>>  net/ipv4/ip_gre.c      |  2 ++
>>  net/ipv4/ipip.c        |  2 ++
>>  net/ipv4/udp_offload.c | 51 +++++++++++++++++++++++++++++++++++++++++++++-----
>>  net/ipv6/ip6_gre.c     |  8 ++++++--
>>  net/ipv6/sit.c         |  4 ++++
>>  net/ipv6/udp_offload.c |  2 +-
>>  10 files changed, 92 insertions(+), 11 deletions(-)
>>
>> --
>> 2.1.0.rc2.206.gedb03e5
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: eBPF verifier thoughts (Re: [PATCH v15 net-next 00/11] eBPF syscall, verifier, testsuite)
From: Alexei Starovoitov @ 2014-09-26 23:13 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: <CALCETrUP=LE2QNzYL8LCukeuWeumOn0bm0eqYscc7GJqq45oYA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Fri, Sep 26, 2014 at 3:41 PM, Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> wrote:
> On Fri, Sep 26, 2014 at 3:26 PM, Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org> wrote:
>> On Fri, Sep 26, 2014 at 3:07 PM, Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> wrote:
>>> On Fri, Sep 26, 2014 at 3:03 PM, Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org> wrote:
>>>> On Fri, Sep 26, 2014 at 2:47 PM, Andy Lutomirski <luto-kltTT9wpgjJwATOyAt5JVQ@public.gmane.org> wrote:
>>>>>
>>>>> Can't you just disallow the 1-byte write to the stack?
>>>>
>>>> of course not.
>>>> That would be extremely limiting to users.
>>>> Can you actually see yourself living with stack that only
>>>> allows 8-byte writes/reads?
>>>> The stack usage will increase a lot, since all char/short
>>>> stack variables will become 8-byte...
>>>
>>> How about requiring that sub-8-byte stack accesses only be to integer slots?
>>
>> you mean to reject the sub-8-byte write early if it's going
>> into space where pointers were stored?
>> That will limit stack reuse.
>> gcc/llvm generate code where the same stack location
>> is used by different variables during life of the function.
>> So if I reject the write early, it will break otherwise valid
>> programs.
>
> I think that a sub-8-byte write to an integer slot should leave it as
> an integer and a sub-8-byte write to a non-integer slot should turn
> that slot into an integer (if conversions to integer are permitted) or
> be rejected otherwise.  gcc/llvm could emit an 8-byte write first, as
> needed, to make this valid.

I don't think the above will work.

> Alternatively, an integer stack slot could have a bitmask indicating
> which bytes are valid.

but this one might. Let me think about it more.
Note verifier, as it stands, is using 12Kbyte of temporary
memory to track stack with byte granularity.
(it's freed as soon as verifier finishes)
This bitmask optimization, best case, will reduce it to 1.5Kbyte
at the cost of extra complexity. I'll play with this idea to
see whether this trade off is actually worth doing.
Also note that there are indirect stack reads
(see check_func_arg() -> check_stack_boundary())
used to verify that N bytes were initialized in stack
before calling into helper function.
Also in the future I was planning to allow
indirect stack write, so that helper function can
store stuff into program stack. This indirect accesses
are crossing 8-byte boundaries, so would need special
care with bitmask optimization.
We need to carefully weight all pros and cons.
imo this memory usage during verification is not a big deal,
but of course we should not waste it. I'll see what can be done.

In any case, we're talking about incremental improvements
on top of current stuff, right?

^ permalink raw reply

* Re: tc rsvp filter show broke
From: John Fastabend @ 2014-09-26 23:25 UTC (permalink / raw)
  To: Jamal Hadi Salim, John Fastabend; +Cc: Cong Wang, netdev
In-Reply-To: <5425ED2D.1000005@mojatatu.com>

On 09/26/2014 03:48 PM, Jamal Hadi Salim wrote:
> On 09/26/14 13:41, John Fastabend wrote:
>> Hi Jamal,
>>
>> Here is another strange 'tc' result that I think is a
>> bug. If it was intended I can't see why. Show filter
>> commands on rsvp classifiers only list the first entry?
>>
> 
> Hrm  - poking inside there and this seems to be one of those
> classifiers that hasnt been getting a lot of love. Doesnt
> support actions well for example.
> 
> It seems to work for me:

This works for me as well try adding two or more policers and
check if they all get printed.

> ----------
> /sbin/tc -V
> tc utility, iproute2-ss140610
> 
> uname -a
> Linux jhs-1 3.16.0-rc2+ #4 SMP Fri Aug 29 11:13:53 EDT 2014 x86_64 x86_64 x86_64 GNU/Linux
> 
> sudo /sbin/tc qdisc add dev eth0 root handle 1:0 prio
> sudo /sbin/tc filter add dev eth0 pref 10 proto ip parent 1:0 rsvp session 10.0.0.1 ipproto icmp classid 1:1  police rate 1kbit burst 90k conform-exceed drop/ok
> /sbin/tc -s filter show dev eth0 parent 1:0
> filter protocol ip pref 10 rsvp
> filter protocol ip pref 10 rsvp fh 0x0001100a flowid 1:1 session 10.0.0.1 ipproto icmp
>  Sent 0 bytes 0 pkts (dropped 0, overlimits 0)
> 
> Now lets send a fast ping:
> sudo ping -f 10.0.0.1 -c 10000
> .......
> 10000 packets transmitted, 386 received, 96% packet loss, time 115495ms
> rtt min/avg/max/mdev = 0.300/0.499/1.262/0.164 ms, ipg/ewma 11.550/0.463 ms
> 
> /sbin/tc -s filter show dev eth0 parent 1:0
> filter protocol ip pref 10 rsvp
> filter protocol ip pref 10 rsvp fh 0x0001100a flowid 1:1 session 10.0.0.1 ipproto icmp
>  Sent 980000 bytes 10000 pkts (dropped 9614, overlimits 9614)
> 
> cheers,
> jamal
> 
> 
> 
> 
> -- 
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: VRFs and the scalability of namespaces
From: Stephen Hemminger @ 2014-09-26 23:52 UTC (permalink / raw)
  To: David Ahern; +Cc: Eric W. Biederman, nicolas.dichtel, netdev@vger.kernel.org
In-Reply-To: <5425EAA6.7040302@gmail.com>

On Fri, 26 Sep 2014 16:37:26 -0600
David Ahern <dsahern@gmail.com> wrote:

> Hi Eric:
> 
> As you suggested [1] I am starting a new thread to discuss scalability 
> problems using namespaces for VRFs.
> 

Whining without suggesting a solution or providing code will probably result
in no action.

^ permalink raw reply

* Re: [PATCH/RFC repost 2/8] netlink: Allow suppression of warnings for duplicate attributes
From: Ben Pfaff @ 2014-09-26 23:55 UTC (permalink / raw)
  To: Simon Horman; +Cc: dev-yBygre7rU0TnMu66kgdUjQ, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1411005311-11752-3-git-send-email-simon.horman-wFxRvT7yatFl57MIdRCFDg@public.gmane.org>

On Thu, Sep 18, 2014 at 10:55:05AM +0900, Simon Horman wrote:
> Add a multiple field to struct nl_policy which if set suppresses
> warning of duplicate attributes in nl_parse_nested().
> 
> As is the case without this patch only the last occurrence of an
> attribute is stored in attrs by nl_parse_nested(). As such
> if the multiple field of struct nl_policy is set then it
> is up to the caller to parse the message to extract all the attributes.
> 
> This is in preparation for allowing multiple OVS_SELECT_GROUP_ATTR_BUCKET
> attributes in a nested OVS_ACTION_ATTR_SELECT_GROUP attribute.
> 
> Signed-off-by: Simon Horman <simon.horman-wFxRvT7yatFl57MIdRCFDg@public.gmane.org>

In the other case where we have duplicate attributes, it doesn't make
sense to process them with the policy functions, because we want to
see all of the instances of the duplicate attributes and policy
doesn't allow us to do that.  I'm a little surprised that the new
attributes work differently.  What's the idea?

^ permalink raw reply

* Re: [PATCH/RFC repost 7/8] ofproto: translate datapath select group action
From: Ben Pfaff @ 2014-09-26 23:57 UTC (permalink / raw)
  To: Simon Horman; +Cc: dev-yBygre7rU0TnMu66kgdUjQ, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1411005311-11752-8-git-send-email-simon.horman-wFxRvT7yatFl57MIdRCFDg@public.gmane.org>

On Thu, Sep 18, 2014 at 10:55:10AM +0900, Simon Horman wrote:
> This patch is a prototype and has several limitations:
> 
> * It assumes that no actions follow a select group action
>   because the resulting packet after a select group action may
>   differ depending on the bucket used. It may be possible
>   to address this problem using recirculation. Or to not use
>   the datapath select group in such situations. In any case
>   this patch does not solve this problem or even prevent it
>   from occurring.

It seems like this limitation in particular is a pretty big one.  Do
you have a good plan in mind for how to resolve it?

^ permalink raw reply

* Re: VRFs and the scalability of namespaces
From: David Ahern @ 2014-09-27  0:00 UTC (permalink / raw)
  To: Stephen Hemminger, David Ahern
  Cc: Eric W. Biederman, nicolas.dichtel, netdev@vger.kernel.org
In-Reply-To: <20140926165241.59cf8a60@urahara>

On 9/26/14, 5:52 PM, Stephen Hemminger wrote:
> On Fri, 26 Sep 2014 16:37:26 -0600
> David Ahern <dsahern@gmail.com> wrote:
>
>> Hi Eric:
>>
>> As you suggested [1] I am starting a new thread to discuss scalability
>> problems using namespaces for VRFs.
>>
>
> Whining without suggesting a solution or providing code will probably result
> in no action.

I have a suggestion; I was trying to set a context on the problem.

David

^ permalink raw reply

* Re: VRFs and the scalability of namespaces
From: Eric W. Biederman @ 2014-09-27  1:25 UTC (permalink / raw)
  To: David Ahern; +Cc: nicolas.dichtel, netdev@vger.kernel.org, Stephen Hemminger
In-Reply-To: <5425EAA6.7040302@gmail.com>

David Ahern <dsahern@gmail.com> writes:

> Hi Eric:
>
> As you suggested [1] I am starting a new thread to discuss scalability
> problems using namespaces for VRFs.

I will accept as a given that using network namespaces at a scale of
1000s and with lots of little applications listening for new connections
(but generally not doing anything) is outside the classic networking
usage of linux and of namespaces and so does not work out of the box and
that very least some fixes are necessary.

However your premise that network namespaces are unsupportable unfixable
and fundamentally unscalable for what you want to do is unsupported.

The most difficult problem for high levels of efficiency is that that of
modifying applications so that they are VRF/namespace aware.  That they
look at the appropriate set of dns resolvers for the namespace, that
when messages are logged the messages report not just the ip address but
the context that ip address came from.  There are no magic solutions to
make that kinds of deep and fundamental code modifications.

I can totally see it being frustrating to use linux as a switch OS
when it doesn't quite do what you want on the hardware you want to use,
and definitely not with the efficiencies you want.

I will tell you what network namespaces get you.  Network namespaces
deliver the full power of the linux network stack, every interesting
feature works, and network namespaces provide a path where you can use
unmodified linux applications.

When you say "proper VRF support" what I hear is that you think
something new needs to be added to the linux network stack (called a
VRF) with a new userspace interface that somehow because it lacks
features is better.

> Background
> ----------
> Consider a single system that wants to provide VRF-based features with
> support for N VRFs. N could easily be 2048 (e.g., 6Wind, [2]), 4000
> (e.g., Cisco, [3]) or even higher.
>
> The single system with support for N VRFs runs M services (e.g.,
> quagga, cdp, lldp, stp, strongswan, some homegrown routing protocol)
> and includes standard system services like sshd. Furthermore, a system
> also includes monitoring programs like snmpd and tcollector. In short,
> M is easily 20 processes that need to have a presence across all VRFs.

And trying to run it all on what would be considered an underpowered in
most contexts.

> Before droning on even more, does the above provide better context on
> the general problem?

It provides a rough context on what you are trying to do.  Use linux as
the OS to run on a switch.

It doesn't actually provide much in the way of context actual problems
that show up when you try to use network namespaces.  Which is what I
was expecting the discussion would be about, and which would I expect be
a productive conversation.

Eric

^ permalink raw reply

* [PATCH] net: llc: check for device before dereferencing
From: Sasha Levin @ 2014-09-27  1:41 UTC (permalink / raw)
  To: acme, davem; +Cc: netdev, linux-kernel, Sasha Levin

llc_ui_sendmsg would not make sure that a device indeed exists before
dereferencing it. This caused a user triggerable NULL ptr deref:

[  430.542391] BUG: unable to handle kernel NULL pointer dereference at 000000000000021e
[  430.551939] IP: llc_ui_sendmsg (net/llc/af_llc.c:912)
[  430.551939] PGD 5edcd067 PUD 5edce067 PMD 0
[  430.551939] Oops: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC KASAN
[  430.551939] Dumping ftrace buffer:
[  430.551939]    (ftrace buffer empty)
[  430.551939] Modules linked in:
[  430.551939] CPU: 2 PID: 9395 Comm: trinity-c261 Not tainted 3.17.0-rc6-next-20140926-sasha-00050-g625a54d-dirty #1239
[  430.551939] task: ffff88005edc0000 ti: ffff88005edc8000 task.ti: ffff88005edc8000
[  430.551939] RIP: llc_ui_sendmsg (net/llc/af_llc.c:912)
[  430.551939] RSP: 0018:ffff88005edcbcd8  EFLAGS: 00010282
[  430.551939] RAX: 0000000000000000 RBX: ffff880239191148 RCX: 0000000000000000
[  430.551939] RDX: dfffe90000000000 RSI: ffffffff814471e1 RDI: ffffffff85f7a77f
[  430.551939] RBP: ffff88005edcbd18 R08: dfffe90000000001 R09: 0000000000000000
[  430.551939] R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
[  430.551939] R13: ffff880237160000 R14: ffff88005edcbeb0 R15: 0000000000000000
[  430.551939] FS:  00007fbcf9f6b700(0000) GS:ffff880111c00000(0000) knlGS:0000000000000000
[  430.551939] CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
[  430.551939] CR2: 000000000000021e CR3: 000000005edcc000 CR4: 00000000000006a0
[  430.551939] DR0: 00000000006ef000 DR1: 0000000000000000 DR2: 0000000000000000
[  430.551939] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000010602
[  430.551939] Stack:
[  430.551939]  0000000000000000 0000000000000100 ffff88005edcbd18 ffff88005edcbd30
[  430.551939]  ffff880237160000 ffff88005edcbe78 0000000000000100 ffffffff86963ec0
[  430.551939]  ffff88005edcbe38 ffffffff85680290 ffff88005edcbd68 0000000000000000
[  430.551939] Call Trace:
[  430.551939] sock_sendmsg (net/socket.c:663)
[  430.551939] ? might_fault (mm/memory.c:3733)
[  430.551939] ? might_fault (./arch/x86/include/asm/current.h:14 mm/memory.c:3732)
[  430.551939] ? __fdget (fs/file.c:698)
[  430.551939] SYSC_sendto (net/socket.c:1818)
[  430.551939] ? do_audit_syscall_entry (include/linux/audit.h:153 arch/x86/kernel/ptrace.c:1448)
[  430.551939] ? syscall_trace_enter_phase2 (arch/x86/kernel/ptrace.c:1598 (discriminator 2))
[  430.551939] SyS_sendto (net/socket.c:1783)
[  430.551939] tracesys_phase2 (arch/x86/kernel/entry_64.S:529)
[ 430.551939] Code: 0f 85 b6 00 00 00 48 8d bb 38 05 00 00 e8 f2 01 d5 fb 4c 8b bb 38 05 00 00 49 8d bf 1e 02 00 00 e8 2f 01 d5 fb 66 41 83 7e 04 00 <45> 0f b7 af 1e 02 00 00 75 1a 48 8d bb d0 02 00 00 e8 53 00 d5

All code
========
   0:	0f 85 b6 00 00 00    	jne    0xbc
   6:	48 8d bb 38 05 00 00 	lea    0x538(%rbx),%rdi
   d:	e8 f2 01 d5 fb       	callq  0xfffffffffbd50204
  12:	4c 8b bb 38 05 00 00 	mov    0x538(%rbx),%r15
  19:	49 8d bf 1e 02 00 00 	lea    0x21e(%r15),%rdi
  20:	e8 2f 01 d5 fb       	callq  0xfffffffffbd50154
  25:	66 41 83 7e 04 00    	cmpw   $0x0,0x4(%r14)
  2b:*	45 0f b7 af 1e 02 00 	movzwl 0x21e(%r15),%r13d		<-- trapping instruction
  32:	00
  33:	75 1a                	jne    0x4f
  35:	48 8d bb d0 02 00 00 	lea    0x2d0(%rbx),%rdi
  3c:	e8 53 00 d5 00       	callq  0xd50094

Code starting with the faulting instruction
===========================================
   0:	45 0f b7 af 1e 02 00 	movzwl 0x21e(%r15),%r13d
   7:	00
   8:	75 1a                	jne    0x24
   a:	48 8d bb d0 02 00 00 	lea    0x2d0(%rbx),%rdi
  11:	e8 53 00 d5 00       	callq  0xd50069
[  430.551939] RIP llc_ui_sendmsg (net/llc/af_llc.c:912)
[  430.551939]  RSP <ffff88005edcbcd8>
[  430.551939] CR2: 000000000000021e

Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
---
 net/llc/af_llc.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/llc/af_llc.c b/net/llc/af_llc.c
index c776ffb..c16e01a 100644
--- a/net/llc/af_llc.c
+++ b/net/llc/af_llc.c
@@ -909,6 +909,9 @@ static int llc_ui_sendmsg(struct kiocb *iocb, struct socket *sock,
 		if (rc)
 			goto release;
 	}
+	rc = -ENODEV;
+	if (!llc->dev)
+		goto release;
 	hdrlen = llc->dev->hard_header_len + llc_ui_header_len(sk, addr);
 	size = hdrlen + len;
 	if (size > llc->dev->mtu)
-- 
1.7.10.4

^ permalink raw reply related

* RE: Silicom bypass driver promote from staging
From: David Hendel @ 2014-09-27  6:54 UTC (permalink / raw)
  To: Francois Romieu
  Cc: netdev@vger.kernel.org, arnd@arndb.de, Anna Lukin,
	gregkh@linuxfoundation.org
In-Reply-To: <20140807215115.GB19520@electric-eye.fr.zoreil.com>

[-- Attachment #1: Type: text/plain, Size: 3160 bytes --]

Hi all,
Sorry, for the late response, we had to go over all the code and update as requested and then test to verify, so here it is.

See below with some responses and attached, hope we can move forward with that. getting the bypass driver into the kernel.

On Thu, 7 Aug 2014 19:56 Stephen Hemminger <stephen@networkplumber.org> wrote:
>The current driver uses a device specific /proc interface.
>That API programming model will not likely be acceptable in a standard network driver.
>Please consider doing something generic with netlink.

Actually, this is not a network driver, many of the users use this interface to
 control the bypass/fail-to-wire to control this function. We can add netlink api as additional interface,
 but  not having  proc interface maybe a problem.

On Thu, 7 Aug 2014 23:20 gregkh@linuxfoundation.org wrote:
>Making your patch an attachment in base64 mode, makes it impossible to quote to review it :(
>
>Anyway, I stopped at the first header file.  The kernel already has BIT definitions, no driver should ever have to redefine these and do it on their own.
>  That leads me to believe that this code really isn't all that "cleaned up" at all.
>
>Should I just look at what is in drivers/staging/silicom/ right now as code to review?  Or have you changed it any by making this patch?

OK, will do that,
drivers/staging/silicom/ driver was created from silicom bypass driver released in December 2012. 
The driver we are providing now is based on this staging driver.
We updated it to our Latest release (adding support for new adapters, some changes in functionality) and fixed bug 
(loading the driver from staging led to kernel panic).


On Fri, 8 Aug 2014 00:51 Francois Romieu <romieu@fr.zoreil.com> wrote:
>Location is the easy part. Getting things reviewed is a different story.
>
>You shouldn't expect reviewers to swallow 300k of code (600k / 2 due to removal). Please split the submitted patch into smaller, self-consistent, logical ones.
>
We will include the patches in the following emails inline each will have smaller size, hope this will be as you expect.

>Notwithstanding Stephen and Jeff's remarks, you should also:
>- remove the (out-)commented code
>- reconsider the use of gorilla class macros vs plain functions
>- stop casting ioremap return into long, then later into void *. It's
>  void __iomem * and should stay so.
>- use a consistent locking style and remove BP_SYNC_FLAG
>- fix the 80 cols limit problem(s) (hardly surprizing after 5 levels of
 > nested "if")
>- avoid redefining stuff from include/uapi/linux/mii.h
> I may be wrong but BPCTLI_MII_CR_POWER_DOWN smells of BMCR_PDOWN and
>  BPCTLI_PHY_CONTROL, well, MII_BMCR ?
>- avoid returning with spinlock held (read_reg, wdt_pulse)
>- start explaining what did change between two submissions

This is what we changed in updated patch:
1. Avoid redefining BIT and MII .
2. Fixing returning with spinlock held
3. Remove BP_SYNC_FLAG
4. Use void __iomem * instead of long long in ioremap return
5. Cleanup with checkpatch.pl

Hope that it is OK now, or at least better.


David Hendel.

[-- Attachment #2: bp_patch3.tar.gz --]
[-- Type: application/x-gzip, Size: 43597 bytes --]

^ permalink raw reply

* Re: [PATCH V2 1/2] brcm80211: use container_of to resolve brcms_phy from brcms_phy_pub
From: Fabian Frederick @ 2014-09-27  8:22 UTC (permalink / raw)
  To: John W. Linville
  Cc: linux-wireless, Arend van Spriel, Hante Meuleman, netdev,
	linux-kernel, Franky (Zhenhui) Lin, brcm80211-dev-list, brudley
In-Reply-To: <20140926174636.GJ22967@tuxdriver.com>



> On 26 September 2014 at 19:46 "John W. Linville" <linville@tuxdriver.com>
> wrote:
>
>
> These two patches don't seem to apply on wireless-next, and I don't
> have time at the moment to fix them up.  Feel free to repost...

Hello John,

        No problem, you already pushed the 2 same patches from the first (too
generic) patchset

        First patchset for drivers/net (see patch 3 and 4)
        http://marc.info/?l=linux-kernel&m=141064073901334&w=2
       
        No updates in V2 anyway:
        ("V2: 2 separate patches for brcm80211")

Wireless-next commits:
a38a9ef1c064
("brcm80211: use container_of to resolve dma_info from dma_pub")

d68e9f5d8549
("brcm80211: use container_of to resolve brcms_phy from brcms_phy_pub")

Regards,
Fabian

^ permalink raw reply

* RE: [PATCH] net: fec: remove memory copy for rx path
From: fugang.duan @ 2014-09-27  9:15 UTC (permalink / raw)
  To: David Miller
  Cc: Frank.Li@freescale.com, netdev@vger.kernel.org,
	shawn.guo@linaro.org, bhutchings@solarflare.com
In-Reply-To: <20140926.170720.2230061639326332503.davem@davemloft.net>

From: David Miller <davem@davemloft.net> Sent: Saturday, September 27, 2014 5:07 AM
>To: Duan Fugang-B38611
>Cc: Li Frank-B20596; netdev@vger.kernel.org; shawn.guo@linaro.org;
>bhutchings@solarflare.com
>Subject: Re: [PATCH] net: fec: remove memory copy for rx path
>
>From: Fugang Duan <b38611@freescale.com>
>Date: Wed, 24 Sep 2014 14:05:30 +0800
>
>> Re-allocate skb instead of memory copy skb data in rx path to improve
>> enet rx performance.
>>
>> Signed-off-by: Fugang Duan <B38611@freescale.com>
>> Signed-off-by: Frank Li <Frank.Li@freescale.com>
>
>For small packet sizes, copying is almost certainly faster and it avoids
>mismatched skb->truesize vs. skb->len which hurts TCP performance.
>
>We call this rx_copybreak, and there are many drivers you can look at to
>see how this works.
>
>Thanks.

Great suggestion. I will change it for next version.

Thanks,
Andy

^ permalink raw reply

* Re: [Xen-devel] [PATCH] xen/xenbus: Use 'void' instead of 'int' for the return of xenbus_switch_state()
From: Chen Gang @ 2014-09-27  9:20 UTC (permalink / raw)
  To: David Vrabel
  Cc: konrad.wilk, ian.campbell, wei.liu2, boris.ostrovsky, bhelgaas,
	jgross, yongjun_wei, mukesh.rathor, xen-devel, linux-pci,
	linux-kernel@vger.kernel.org, linux-scsi, netdev@vger.kernel.org
In-Reply-To: <5425AB57.8040804@citrix.com>

On 09/27/2014 02:07 AM, David Vrabel wrote:
> On 26/09/14 17:36, Chen Gang wrote:
>> When xenbus_switch_state() fails, it will call xenbus_switch_fatal()
>> internally, so need not return any status value, then use 'void' instead
>> of 'int' for xenbus_switch_state() and __xenbus_switch_state().
>>
>> Also need be sure that all callers which check the return value must let
>> 'err' be 0.
> 
> I've rewritten the commit message as:
> 
>     xen/xenbus: don't return errors from xenbus_switch_state()
> 
>     Most users of xenbus_switch_state() weren't handling the failure of
>     xenbus_switch_state() correctly.  They either called
>     xenbus_dev_fatal() (which xenbus_switch_state() has effectively
>     already tried to do), or ignored errors.
> 
>     xenbus_switch_state() may fail because:
> 
>     a) The device is being unplugged by the toolstack. The device will
>     shortly be removed and the error does not need to be handled.
> 
>     b) Xenstore is broken.  There isn't much the driver can do in this
>     case since xenstore is required to signal failure to the toolstack.
> 
>     So, don't report any errors from xenbus_switch_state() which removes
>     some unnecessary error handling in some of the drivers.
> 
> I'd appreciate a review from some of the other front/backend driver
> maintainers on whether this is sensible reasoning.
> 

At least for me, what your changes is necessary, and also necessary to
let other related members to have a look.

And one thing I did not mention: after the patch, it reports a warning,
which I have sent the other followed patch for it (it is for a bug fix,
but also can fix this warning). The other followed patch is

  "xen/xen-scsiback: Need go to fail after xenbus_dev_error()"

Please help check, too.

Thanks.

> David
> 
>>
>> And also need change the related comments for xenbus_switch_state().
>>
>> Signed-off-by: Chen Gang <gang.chen.5i5j@gmail.com>
>> ---
>>  drivers/block/xen-blkback/xenbus.c |  9 ++-------
>>  drivers/net/xen-netback/xenbus.c   |  5 +----
>>  drivers/pci/xen-pcifront.c         | 15 ++++++---------
>>  drivers/xen/xen-pciback/xenbus.c   | 21 ++++-----------------
>>  drivers/xen/xen-scsiback.c         |  5 +----
>>  drivers/xen/xenbus/xenbus_client.c | 16 ++++++++--------
>>  include/xen/xenbus.h               |  3 ++-
>>  7 files changed, 24 insertions(+), 50 deletions(-)
>>
>> diff --git a/drivers/block/xen-blkback/xenbus.c b/drivers/block/xen-blkback/xenbus.c
>> index 3a8b810..fdcc584 100644
>> --- a/drivers/block/xen-blkback/xenbus.c
>> +++ b/drivers/block/xen-blkback/xenbus.c
>> @@ -587,9 +587,7 @@ static int xen_blkbk_probe(struct xenbus_device *dev,
>>  	if (err)
>>  		goto fail;
>>  
>> -	err = xenbus_switch_state(dev, XenbusStateInitWait);
>> -	if (err)
>> -		goto fail;
>> +	xenbus_switch_state(dev, XenbusStateInitWait);
>>  
>>  	return 0;
>>  
>> @@ -837,10 +835,7 @@ again:
>>  	if (err)
>>  		xenbus_dev_fatal(dev, err, "ending transaction");
>>  
>> -	err = xenbus_switch_state(dev, XenbusStateConnected);
>> -	if (err)
>> -		xenbus_dev_fatal(dev, err, "%s: switching to Connected state",
>> -				 dev->nodename);
>> +	xenbus_switch_state(dev, XenbusStateConnected);
>>  
>>  	return;
>>   abort:
>> diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c
>> index 9c47b89..b5c3d47 100644
>> --- a/drivers/net/xen-netback/xenbus.c
>> +++ b/drivers/net/xen-netback/xenbus.c
>> @@ -337,10 +337,7 @@ static int netback_probe(struct xenbus_device *dev,
>>  	if (err)
>>  		pr_debug("Error writing multi-queue-max-queues\n");
>>  
>> -	err = xenbus_switch_state(dev, XenbusStateInitWait);
>> -	if (err)
>> -		goto fail;
>> -
>> +	xenbus_switch_state(dev, XenbusStateInitWait);
>>  	be->state = XenbusStateInitWait;
>>  
>>  	/* This kicks hotplug scripts, so do it immediately. */
>> diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
>> index 53df39a..c1d73b7 100644
>> --- a/drivers/pci/xen-pcifront.c
>> +++ b/drivers/pci/xen-pcifront.c
>> @@ -901,18 +901,16 @@ static int pcifront_try_connect(struct pcifront_device *pdev)
>>  		}
>>  	}
>>  
>> -	err = xenbus_switch_state(pdev->xdev, XenbusStateConnected);
>> -
>> +	xenbus_switch_state(pdev->xdev, XenbusStateConnected);
>> +	return 0;
>>  out:
>>  	return err;
>>  }
>>  
>>  static int pcifront_try_disconnect(struct pcifront_device *pdev)
>>  {
>> -	int err = 0;
>>  	enum xenbus_state prev_state;
>>  
>> -
>>  	prev_state = xenbus_read_driver_state(pdev->xdev->nodename);
>>  
>>  	if (prev_state >= XenbusStateClosing)
>> @@ -923,11 +921,10 @@ static int pcifront_try_disconnect(struct pcifront_device *pdev)
>>  		pcifront_disconnect(pdev);
>>  	}
>>  
>> -	err = xenbus_switch_state(pdev->xdev, XenbusStateClosed);
>> +	xenbus_switch_state(pdev->xdev, XenbusStateClosed);
>>  
>>  out:
>> -
>> -	return err;
>> +	return 0;
>>  }
>>  
>>  static int pcifront_attach_devices(struct pcifront_device *pdev)
>> @@ -1060,8 +1057,8 @@ static int pcifront_detach_devices(struct pcifront_device *pdev)
>>  			domain, bus, slot, func);
>>  	}
>>  
>> -	err = xenbus_switch_state(pdev->xdev, XenbusStateReconfiguring);
>> -
>> +	xenbus_switch_state(pdev->xdev, XenbusStateReconfiguring);
>> +	return 0;
>>  out:
>>  	return err;
>>  }
>> diff --git a/drivers/xen/xen-pciback/xenbus.c b/drivers/xen/xen-pciback/xenbus.c
>> index c214daa..630a215 100644
>> --- a/drivers/xen/xen-pciback/xenbus.c
>> +++ b/drivers/xen/xen-pciback/xenbus.c
>> @@ -184,10 +184,7 @@ static int xen_pcibk_attach(struct xen_pcibk_device *pdev)
>>  
>>  	dev_dbg(&pdev->xdev->dev, "Connecting...\n");
>>  
>> -	err = xenbus_switch_state(pdev->xdev, XenbusStateConnected);
>> -	if (err)
>> -		xenbus_dev_fatal(pdev->xdev, err,
>> -				 "Error switching to connected state!");
>> +	xenbus_switch_state(pdev->xdev, XenbusStateConnected);
>>  
>>  	dev_dbg(&pdev->xdev->dev, "Connected? %d\n", err);
>>  out:
>> @@ -500,12 +497,7 @@ static int xen_pcibk_reconfigure(struct xen_pcibk_device *pdev)
>>  		}
>>  	}
>>  
>> -	err = xenbus_switch_state(pdev->xdev, XenbusStateReconfigured);
>> -	if (err) {
>> -		xenbus_dev_fatal(pdev->xdev, err,
>> -				 "Error switching to reconfigured state!");
>> -		goto out;
>> -	}
>> +	xenbus_switch_state(pdev->xdev, XenbusStateReconfigured);
>>  
>>  out:
>>  	mutex_unlock(&pdev->dev_lock);
>> @@ -640,10 +632,7 @@ static int xen_pcibk_setup_backend(struct xen_pcibk_device *pdev)
>>  		goto out;
>>  	}
>>  
>> -	err = xenbus_switch_state(pdev->xdev, XenbusStateInitialised);
>> -	if (err)
>> -		xenbus_dev_fatal(pdev->xdev, err,
>> -				 "Error switching to initialised state!");
>> +	xenbus_switch_state(pdev->xdev, XenbusStateInitialised);
>>  
>>  out:
>>  	mutex_unlock(&pdev->dev_lock);
>> @@ -683,9 +672,7 @@ static int xen_pcibk_xenbus_probe(struct xenbus_device *dev,
>>  	}
>>  
>>  	/* wait for xend to configure us */
>> -	err = xenbus_switch_state(dev, XenbusStateInitWait);
>> -	if (err)
>> -		goto out;
>> +	xenbus_switch_state(dev, XenbusStateInitWait);
>>  
>>  	/* watch the backend node for backend configuration information */
>>  	err = xenbus_watch_path(dev, dev->nodename, &pdev->be_watch,
>> diff --git a/drivers/xen/xen-scsiback.c b/drivers/xen/xen-scsiback.c
>> index ad11258..847bc9c 100644
>> --- a/drivers/xen/xen-scsiback.c
>> +++ b/drivers/xen/xen-scsiback.c
>> @@ -1225,10 +1225,7 @@ static int scsiback_probe(struct xenbus_device *dev,
>>  	if (err)
>>  		xenbus_dev_error(dev, err, "writing feature-sg-grant");
>>  
>> -	err = xenbus_switch_state(dev, XenbusStateInitWait);
>> -	if (err)
>> -		goto fail;
>> -
>> +	xenbus_switch_state(dev, XenbusStateInitWait);
>>  	return 0;
>>  
>>  fail:
>> diff --git a/drivers/xen/xenbus/xenbus_client.c b/drivers/xen/xenbus/xenbus_client.c
>> index ca74410..e2bcd1d 100644
>> --- a/drivers/xen/xenbus/xenbus_client.c
>> +++ b/drivers/xen/xenbus/xenbus_client.c
>> @@ -166,7 +166,7 @@ EXPORT_SYMBOL_GPL(xenbus_watch_pathfmt);
>>  static void xenbus_switch_fatal(struct xenbus_device *, int, int,
>>  				const char *, ...);
>>  
>> -static int
>> +static void
>>  __xenbus_switch_state(struct xenbus_device *dev,
>>  		      enum xenbus_state state, int depth)
>>  {
>> @@ -188,7 +188,7 @@ __xenbus_switch_state(struct xenbus_device *dev,
>>  	int err, abort;
>>  
>>  	if (state == dev->state)
>> -		return 0;
>> +		return;
>>  
>>  again:
>>  	abort = 1;
>> @@ -196,7 +196,7 @@ again:
>>  	err = xenbus_transaction_start(&xbt);
>>  	if (err) {
>>  		xenbus_switch_fatal(dev, depth, err, "starting transaction");
>> -		return 0;
>> +		return;
>>  	}
>>  
>>  	err = xenbus_scanf(xbt, dev->nodename, "state", "%d", &current_state);
>> @@ -219,7 +219,7 @@ abort:
>>  	} else
>>  		dev->state = state;
>>  
>> -	return 0;
>> +	return;
>>  }
>>  
>>  /**
>> @@ -228,12 +228,12 @@ abort:
>>   * @state: new state
>>   *
>>   * Advertise in the store a change of the given driver to the given new_state.
>> - * Return 0 on success, or -errno on error.  On error, the device will switch
>> - * to XenbusStateClosing, and the error will be saved in the store.
>> + * When failure occurs, it will call xenbus_switch_fatal() internally, so need
>> + * not return value to notify upper caller.
>>   */
>> -int xenbus_switch_state(struct xenbus_device *dev, enum xenbus_state state)
>> +void xenbus_switch_state(struct xenbus_device *dev, enum xenbus_state state)
>>  {
>> -	return __xenbus_switch_state(dev, state, 0);
>> +	__xenbus_switch_state(dev, state, 0);
>>  }
>>  
>>  EXPORT_SYMBOL_GPL(xenbus_switch_state);
>> diff --git a/include/xen/xenbus.h b/include/xen/xenbus.h
>> index 0324c6d..587c53d 100644
>> --- a/include/xen/xenbus.h
>> +++ b/include/xen/xenbus.h
>> @@ -195,7 +195,8 @@ int xenbus_watch_pathfmt(struct xenbus_device *dev, struct xenbus_watch *watch,
>>  					  const char **, unsigned int),
>>  			 const char *pathfmt, ...);
>>  
>> -int xenbus_switch_state(struct xenbus_device *dev, enum xenbus_state new_state);
>> +void xenbus_switch_state(struct xenbus_device *dev,
>> +			 enum xenbus_state new_state);
>>  int xenbus_grant_ring(struct xenbus_device *dev, unsigned long ring_mfn);
>>  int xenbus_map_ring_valloc(struct xenbus_device *dev,
>>  			   int gnt_ref, void **vaddr);
>>
> 


-- 
Chen Gang

Open share and attitude like air water and life which God blessed

^ permalink raw reply

* RE: [PATCH v1 0/4] net: fec: Code cleanup
From: fugang.duan @ 2014-09-27  9:51 UTC (permalink / raw)
  To: David Miller; +Cc: netdev@vger.kernel.org, Frank.Li@freescale.com
In-Reply-To: <20140926.160602.1429816882470314568.davem@davemloft.net>

From: David Miller <davem@davemloft.net> Sent: Saturday, September 27, 2014 4:06 AM
>To: Duan Fugang-B38611
>Cc: netdev@vger.kernel.org; Li Frank-B20596
>Subject: Re: [PATCH v1 0/4] net: fec: Code cleanup
>
>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.

No special requirement, just follow the normal rel flow.

Thanks,
Andy

^ permalink raw reply

* Re: [PATCH] net: fec: remove memory copy for rx path
From: Francois Romieu @ 2014-09-27 11:39 UTC (permalink / raw)
  To: fugang.duan@freescale.com
  Cc: David Miller, Frank.Li@freescale.com, netdev@vger.kernel.org,
	shawn.guo@linaro.org, bhutchings@solarflare.com
In-Reply-To: <057807be43ca4c79ad51ab07c668519d@BLUPR03MB373.namprd03.prod.outlook.com>

fugang.duan@freescale.com <fugang.duan@freescale.com> :
> From: David Miller <davem@davemloft.net> Sent: Saturday, September 27, 2014 5:07 AM
[...]
> >We call this rx_copybreak, and there are many drivers you can look at to
> >see how this works.
> >
> >Thanks.
> 
> Great suggestion. I will change it for next version.

Be aware that copybreak module option should be considered legacy.

There is an ethtool api for it.

-- 
Ueimor

^ permalink raw reply

* Re: VRFs and the scalability of namespaces
From: Hannes Frederic Sowa @ 2014-09-27 13:29 UTC (permalink / raw)
  To: David Ahern, Eric W. Biederman; +Cc: nicolas.dichtel, netdev
In-Reply-To: <5425EAA6.7040302@gmail.com>

Hi,

On Sat, Sep 27, 2014, at 00:37, David Ahern wrote:
> The Linux kernel needs proper VRF support -- as an L3 concept. A 
> capability to run a process in a "VRF any" context provides a resource 
> efficient solution where a single process with a single listen socket 
> works across all VRFs in a namespace and then connected sockets have a 
> specific VRF context.

In case you want to have full blown VRF support as in BGP-MPLS-VRF
setups I agree that namespaces seem to be not the right tool for the
job, even not closely. Although if those namespaces get split up in a
more lightweight routing/forwarding namespace and the normal full blown
network namespace it seems to be too cumbersome for user space to manage
those.

Did you already did an investigation how maybe the rule and table
features could be exploited to suite your needs? Some time back I
suggested something like "ip route table foo exec ....", keep an default
routing lookup indicator in task_struct which gets implicitly propagated
to rtnetlink routing table requests/modification for the requested
table. Tables already can be specified via rtnetlink, so no change would
be needed here.

For sockets something like SO_BINDTOTABLE might work, maybe even we can
by default use the task_struct information to also bind the sockets to
the per-process table. We certainly need to preserve the routing
information on the socket as we need those in icmp error handling (e.g.
where to apply ipv4/ipv6 redirects too). Directing incoming packets to
specific table also works via ip-rule-iif match.

Advantage with the ip route table foo exec... method would be, that
conversion of some unmodified routing management daemons might be
easier, others can either use rtnetlink extended attributes which are
already available, and we only need to have per-process context routing
table control, which seems not too hard to implement in ip-rule
subsystem, but I haven't checked.

The problem I see with rules is that some of those tables already work
hand in hand, they already have a implicit semantics, e.g. local, main,
default and unspec (this is even worse for IPv6, where addrconf already
uses hardcoded tables). Working around this might be very tricky and
even more problematic to do from user space.

I think I am not yet sure what features you want from VRFs, some things
seem to match the rule/table features but others I think are pretty hard
to implement.

I worry a bit about icmp error handling and updates of the routing
table, but this is a detail we can look into later if all needed
features are known.

Bye,
Hannes

^ permalink raw reply

* Re: VRFs and the scalability of namespaces
From: Hannes Frederic Sowa @ 2014-09-27 14:09 UTC (permalink / raw)
  To: David Ahern, Eric W. Biederman; +Cc: nicolas.dichtel, netdev
In-Reply-To: <1411824598.2136890.172383085.705271DD@webmail.messagingengine.com>

Hi,

Addendum:

On Sat, Sep 27, 2014, at 15:29, Hannes Frederic Sowa wrote:
> Did you already did an investigation how maybe the rule and table
> features could be exploited to suite your needs? Some time back I
> suggested something like "ip route table foo exec ....", keep an default
> routing lookup indicator in task_struct which gets implicitly propagated
> to rtnetlink routing table requests/modification for the requested
> table. Tables already can be specified via rtnetlink, so no change would
> be needed here.
>
> For sockets something like SO_BINDTOTABLE might work, maybe even we can
> by default use the task_struct information to also bind the sockets to
> the per-process table. We certainly need to preserve the routing
> information on the socket as we need those in icmp error handling (e.g.
> where to apply ipv4/ipv6 redirects too). Directing incoming packets to
> specific table also works via ip-rule-iif match.

Update and lookup rule ids must be separated, so a process might need to
get a tuple of references which table to update and which tables to
match in ip rules.

Also some data structures on matching might be change, e.g. an ->action
which takes an interface and returns the routing table id in O(1)
instead of walking the rules and executing the actions in order.

> The problem I see with rules is that some of those tables already work
> hand in hand, they already have a implicit semantics, e.g. local, main,
> default and unspec (this is even worse for IPv6, where addrconf already
> uses hardcoded tables). Working around this might be very tricky and
> even more problematic to do from user space.

We might also add an rule reference to net_device so we redirect the
route changes during address addition/deletion to a separate table,
otherwise user space has to move them non-atomically.

Bye,
Hannes

^ permalink raw reply

* Re: [PATCH v3 net-next 3/3] tcp: better TCP_SKB_CB layout to reduce cache line misses
From: Eric Dumazet @ 2014-09-27 16:46 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David S. Miller, netdev, Yuchung Cheng, Neal Cardwell,
	Christoph Paasch
In-Reply-To: <1411676092-16196-4-git-send-email-edumazet@google.com>

On Thu, 2014-09-25 at 13:14 -0700, Eric Dumazet wrote:
> TCP maintains lists of skb in write queue, and in receive queues
> (in order and out of order queues)
> 
> Scanning these lists both in input and output path usually requires
> access to skb->next, TCP_SKB_CB(skb)->seq, and TCP_SKB_CB(skb)->end_seq
> 
> These fields are currently in two different cache lines, meaning we
> waste lot of memory bandwidth when these queues are big and flows
> have either packet drops or packet reorders.
> 
> We can move TCP_SKB_CB(skb)->header at the end of TCP_SKB_CB, because
> this header is not used in fast path. This allows TCP to search much faster
> in the skb lists.
> 
> Even with regular flows, we save one cache line miss in fast path.
> 
> Thanks to Christoph Paasch for noticing we need to cleanup
> skb->cb[] (IPCB/IP6CB) before entering IP stack in tx path,
> and that I forgot IPCB use in tcp_v4_hnd_req() and tcp_v4_save_options().
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> ---

Silly me, I'll send a v4, sorry for the noise

net/ipv4/tcp_ipv4.c: In function 'tcp_v4_save_options':
net/ipv4/tcp_ipv4.c:889:33: warning: initialization from incompatible pointer type [enabled by default]
net/ipv4/tcp_ipv4.c: In function 'tcp_v4_hnd_req':
net/ipv4/tcp_ipv4.c:1432:43: warning: passing argument 3 of 'cookie_v4_check' from incompatible pointer type [enabled by default]
In file included from net/ipv4/tcp_ipv4.c:69:0:
include/net/tcp.h:474:14: note: expected 'struct ip_options *' but argument is of type 'struct inet_skb_parm *'

^ permalink raw reply

* [PATCH v4 net-next 0/3] tcp: better TCP_SKB_CB layout
From: Eric Dumazet @ 2014-09-27 16:50 UTC (permalink / raw)
  To: David S. Miller
  Cc: netdev, Yuchung Cheng, Neal Cardwell, Christoph Paasch,
	Eric Dumazet

TCP had the assumption that IPCB and IP6CB are first members of skb->cb[]

This is fine, except that IPCB/IP6CB are used in TCP for a very short time
in input path.

What really matters for TCP stack is to get skb->next,
TCP_SKB_CB(skb)->seq, and TCP_SKB_CB(skb)->end_seq in the same cache line.

skb that are immediately consumed do not care because whole skb->cb[] is
hot in cpu cache, while skb that sit in wocket write queue or receive queues
do not need TCP_SKB_CB(skb)->header at all.

This patch set implements the prereq for IPv4, IPv6, and TCP to make this
possible. This makes TCP more efficient.


Eric Dumazet (3):
  ipv4: rename ip_options_echo to __ip_options_echo()
  ipv6: add a struct inet6_skb_parm param to ipv6_opt_accepted()
  tcp: better TCP_SKB_CB layout to reduce cache line misses

 include/net/ip.h      | 15 ++++++++++++---
 include/net/ipv6.h    |  3 ++-
 include/net/tcp.h     | 12 ++++++------
 net/dccp/ipv6.c       |  2 +-
 net/ipv4/ip_options.c |  6 ++----
 net/ipv4/ip_output.c  |  8 +++++---
 net/ipv4/tcp_ipv4.c   | 29 ++++++++++++++++++-----------
 net/ipv4/tcp_output.c |  5 +++++
 net/ipv6/af_inet6.c   |  4 ++--
 net/ipv6/syncookies.c |  2 +-
 net/ipv6/tcp_ipv6.c   | 12 ++++++++++--
 11 files changed, 64 insertions(+), 34 deletions(-)

-- 
2.1.0.rc2.206.gedb03e5

^ permalink raw reply

* [PATCH v4 net-next 1/3] ipv4: rename ip_options_echo to __ip_options_echo()
From: Eric Dumazet @ 2014-09-27 16:50 UTC (permalink / raw)
  To: David S. Miller
  Cc: netdev, Yuchung Cheng, Neal Cardwell, Christoph Paasch,
	Eric Dumazet
In-Reply-To: <1411836657-9542-1-git-send-email-edumazet@google.com>

ip_options_echo() assumes struct ip_options is provided in &IPCB(skb)->opt
Lets break this assumption, but provide a helper to not change all call points.

ip_send_unicast_reply() gets a new struct ip_options pointer.

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 include/net/ip.h      | 15 ++++++++++++---
 net/ipv4/ip_options.c |  6 ++----
 net/ipv4/ip_output.c  |  8 +++++---
 net/ipv4/tcp_ipv4.c   | 10 ++++++----
 4 files changed, 25 insertions(+), 14 deletions(-)

diff --git a/include/net/ip.h b/include/net/ip.h
index fcd9068fb8c3..0bb620702929 100644
--- a/include/net/ip.h
+++ b/include/net/ip.h
@@ -180,8 +180,10 @@ static inline __u8 ip_reply_arg_flowi_flags(const struct ip_reply_arg *arg)
 	return (arg->flags & IP_REPLY_ARG_NOSRCCHECK) ? FLOWI_FLAG_ANYSRC : 0;
 }
 
-void ip_send_unicast_reply(struct net *net, struct sk_buff *skb, __be32 daddr,
-			   __be32 saddr, const struct ip_reply_arg *arg,
+void ip_send_unicast_reply(struct net *net, struct sk_buff *skb,
+			   const struct ip_options *sopt,
+			   __be32 daddr, __be32 saddr,
+			   const struct ip_reply_arg *arg,
 			   unsigned int len);
 
 #define IP_INC_STATS(net, field)	SNMP_INC_STATS64((net)->mib.ip_statistics, field)
@@ -511,7 +513,14 @@ int ip_forward(struct sk_buff *skb);
  
 void ip_options_build(struct sk_buff *skb, struct ip_options *opt,
 		      __be32 daddr, struct rtable *rt, int is_frag);
-int ip_options_echo(struct ip_options *dopt, struct sk_buff *skb);
+
+int __ip_options_echo(struct ip_options *dopt, struct sk_buff *skb,
+		      const struct ip_options *sopt);
+static inline int ip_options_echo(struct ip_options *dopt, struct sk_buff *skb)
+{
+	return __ip_options_echo(dopt, skb, &IPCB(skb)->opt);
+}
+
 void ip_options_fragment(struct sk_buff *skb);
 int ip_options_compile(struct net *net, struct ip_options *opt,
 		       struct sk_buff *skb);
diff --git a/net/ipv4/ip_options.c b/net/ipv4/ip_options.c
index ad382499bace..5b3d91be2db0 100644
--- a/net/ipv4/ip_options.c
+++ b/net/ipv4/ip_options.c
@@ -87,17 +87,15 @@ void ip_options_build(struct sk_buff *skb, struct ip_options *opt,
  * NOTE: dopt cannot point to skb.
  */
 
-int ip_options_echo(struct ip_options *dopt, struct sk_buff *skb)
+int __ip_options_echo(struct ip_options *dopt, struct sk_buff *skb,
+		      const struct ip_options *sopt)
 {
-	const struct ip_options *sopt;
 	unsigned char *sptr, *dptr;
 	int soffset, doffset;
 	int	optlen;
 
 	memset(dopt, 0, sizeof(struct ip_options));
 
-	sopt = &(IPCB(skb)->opt);
-
 	if (sopt->optlen == 0)
 		return 0;
 
diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index 215af2b155cb..c8fa62476461 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -1522,8 +1522,10 @@ static DEFINE_PER_CPU(struct inet_sock, unicast_sock) = {
 	.uc_ttl		= -1,
 };
 
-void ip_send_unicast_reply(struct net *net, struct sk_buff *skb, __be32 daddr,
-			   __be32 saddr, const struct ip_reply_arg *arg,
+void ip_send_unicast_reply(struct net *net, struct sk_buff *skb,
+			   const struct ip_options *sopt,
+			   __be32 daddr, __be32 saddr,
+			   const struct ip_reply_arg *arg,
 			   unsigned int len)
 {
 	struct ip_options_data replyopts;
@@ -1534,7 +1536,7 @@ void ip_send_unicast_reply(struct net *net, struct sk_buff *skb, __be32 daddr,
 	struct sock *sk;
 	struct inet_sock *inet;
 
-	if (ip_options_echo(&replyopts.opt.opt, skb))
+	if (__ip_options_echo(&replyopts.opt.opt, skb, sopt))
 		return;
 
 	ipc.addr = daddr;
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 3b2e49cb2b61..28ab90382c01 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -681,8 +681,9 @@ static void tcp_v4_send_reset(struct sock *sk, struct sk_buff *skb)
 
 	net = dev_net(skb_dst(skb)->dev);
 	arg.tos = ip_hdr(skb)->tos;
-	ip_send_unicast_reply(net, skb, ip_hdr(skb)->saddr,
-			      ip_hdr(skb)->daddr, &arg, arg.iov[0].iov_len);
+	ip_send_unicast_reply(net, skb, &TCP_SKB_CB(skb)->header.h4.opt,
+			      ip_hdr(skb)->saddr, ip_hdr(skb)->daddr,
+			      &arg, arg.iov[0].iov_len);
 
 	TCP_INC_STATS_BH(net, TCP_MIB_OUTSEGS);
 	TCP_INC_STATS_BH(net, TCP_MIB_OUTRSTS);
@@ -764,8 +765,9 @@ static void tcp_v4_send_ack(struct sk_buff *skb, u32 seq, u32 ack,
 	if (oif)
 		arg.bound_dev_if = oif;
 	arg.tos = tos;
-	ip_send_unicast_reply(net, skb, ip_hdr(skb)->saddr,
-			      ip_hdr(skb)->daddr, &arg, arg.iov[0].iov_len);
+	ip_send_unicast_reply(net, skb, &TCP_SKB_CB(skb)->header.h4.opt,
+			      ip_hdr(skb)->saddr, ip_hdr(skb)->daddr,
+			      &arg, arg.iov[0].iov_len);
 
 	TCP_INC_STATS_BH(net, TCP_MIB_OUTSEGS);
 }
-- 
2.1.0.rc2.206.gedb03e5

^ permalink raw reply related

* [PATCH v4 net-next 3/3] tcp: better TCP_SKB_CB layout to reduce cache line misses
From: Eric Dumazet @ 2014-09-27 16:50 UTC (permalink / raw)
  To: David S. Miller
  Cc: netdev, Yuchung Cheng, Neal Cardwell, Christoph Paasch,
	Eric Dumazet
In-Reply-To: <1411836657-9542-1-git-send-email-edumazet@google.com>

TCP maintains lists of skb in write queue, and in receive queues
(in order and out of order queues)

Scanning these lists both in input and output path usually requires
access to skb->next, TCP_SKB_CB(skb)->seq, and TCP_SKB_CB(skb)->end_seq

These fields are currently in two different cache lines, meaning we
waste lot of memory bandwidth when these queues are big and flows
have either packet drops or packet reorders.

We can move TCP_SKB_CB(skb)->header at the end of TCP_SKB_CB, because
this header is not used in fast path. This allows TCP to search much faster
in the skb lists.

Even with regular flows, we save one cache line miss in fast path.

Thanks to Christoph Paasch for noticing we need to cleanup
skb->cb[] (IPCB/IP6CB) before entering IP stack in tx path,
and that I forgot IPCB use in tcp_v4_hnd_req() and tcp_v4_save_options().

Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 include/net/tcp.h     | 12 ++++++------
 net/ipv4/tcp_ipv4.c   | 19 ++++++++++++-------
 net/ipv4/tcp_output.c |  5 +++++
 net/ipv6/tcp_ipv6.c   |  7 +++++++
 4 files changed, 30 insertions(+), 13 deletions(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index a4201ef216e8..4dc6641ee990 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -696,12 +696,6 @@ static inline u32 tcp_skb_timestamp(const struct sk_buff *skb)
  * If this grows please adjust skbuff.h:skbuff->cb[xxx] size appropriately.
  */
 struct tcp_skb_cb {
-	union {
-		struct inet_skb_parm	h4;
-#if IS_ENABLED(CONFIG_IPV6)
-		struct inet6_skb_parm	h6;
-#endif
-	} header;	/* For incoming frames		*/
 	__u32		seq;		/* Starting sequence number	*/
 	__u32		end_seq;	/* SEQ + FIN + SYN + datalen	*/
 	__u32		tcp_tw_isn;	/* isn chosen by tcp_timewait_state_process() */
@@ -720,6 +714,12 @@ struct tcp_skb_cb {
 	__u8		ip_dsfield;	/* IPv4 tos or IPv6 dsfield	*/
 	/* 1 byte hole */
 	__u32		ack_seq;	/* Sequence number ACK'd	*/
+	union {
+		struct inet_skb_parm	h4;
+#if IS_ENABLED(CONFIG_IPV6)
+		struct inet6_skb_parm	h6;
+#endif
+	} header;	/* For incoming frames		*/
 };
 
 #define TCP_SKB_CB(__skb)	((struct tcp_skb_cb *)&((__skb)->cb[0]))
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 28ab90382c01..9ce3eac02957 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -886,18 +886,16 @@ EXPORT_SYMBOL(tcp_syn_flood_action);
  */
 static struct ip_options_rcu *tcp_v4_save_options(struct sk_buff *skb)
 {
-	const struct ip_options *opt = &(IPCB(skb)->opt);
+	const struct ip_options *opt = &TCP_SKB_CB(skb)->header.h4.opt;
 	struct ip_options_rcu *dopt = NULL;
 
 	if (opt && opt->optlen) {
 		int opt_size = sizeof(*dopt) + opt->optlen;
 
 		dopt = kmalloc(opt_size, GFP_ATOMIC);
-		if (dopt) {
-			if (ip_options_echo(&dopt->opt, skb)) {
-				kfree(dopt);
-				dopt = NULL;
-			}
+		if (dopt && __ip_options_echo(&dopt->opt, skb, opt)) {
+			kfree(dopt);
+			dopt = NULL;
 		}
 	}
 	return dopt;
@@ -1431,7 +1429,7 @@ static struct sock *tcp_v4_hnd_req(struct sock *sk, struct sk_buff *skb)
 
 #ifdef CONFIG_SYN_COOKIES
 	if (!th->syn)
-		sk = cookie_v4_check(sk, skb, &(IPCB(skb)->opt));
+		sk = cookie_v4_check(sk, skb, &TCP_SKB_CB(skb)->header.h4.opt);
 #endif
 	return sk;
 }
@@ -1636,6 +1634,13 @@ int tcp_v4_rcv(struct sk_buff *skb)
 
 	th = tcp_hdr(skb);
 	iph = ip_hdr(skb);
+	/* This is tricky : We move IPCB at its correct location into TCP_SKB_CB()
+	 * barrier() makes sure compiler wont play fool^Waliasing games.
+	 */
+	memmove(&TCP_SKB_CB(skb)->header.h4, IPCB(skb),
+		sizeof(struct inet_skb_parm));
+	barrier();
+
 	TCP_SKB_CB(skb)->seq = ntohl(th->seq);
 	TCP_SKB_CB(skb)->end_seq = (TCP_SKB_CB(skb)->seq + th->syn + th->fin +
 				    skb->len - th->doff * 4);
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index f173b1c4f815..a462fb1db896 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -974,6 +974,11 @@ static int tcp_transmit_skb(struct sock *sk, struct sk_buff *skb, int clone_it,
 
 	/* Our usage of tstamp should remain private */
 	skb->tstamp.tv64 = 0;
+
+	/* Cleanup our debris for IP stacks */
+	memset(skb->cb, 0, max(sizeof(struct inet_skb_parm),
+			       sizeof(struct inet6_skb_parm)));
+
 	err = icsk->icsk_af_ops->queue_xmit(sk, skb, &inet->cork.fl);
 
 	if (likely(err <= 0))
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 9400b4326f22..132bac137aed 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -1412,6 +1412,13 @@ static int tcp_v6_rcv(struct sk_buff *skb)
 
 	th = tcp_hdr(skb);
 	hdr = ipv6_hdr(skb);
+	/* This is tricky : We move IPCB at its correct location into TCP_SKB_CB()
+	 * barrier() makes sure compiler wont play fool^Waliasing games.
+	 */
+	memmove(&TCP_SKB_CB(skb)->header.h6, IP6CB(skb),
+		sizeof(struct inet6_skb_parm));
+	barrier();
+
 	TCP_SKB_CB(skb)->seq = ntohl(th->seq);
 	TCP_SKB_CB(skb)->end_seq = (TCP_SKB_CB(skb)->seq + th->syn + th->fin +
 				    skb->len - th->doff*4);
-- 
2.1.0.rc2.206.gedb03e5

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox