Netdev List
 help / color / mirror / Atom feed
* [PATCH bpf-next v4 00/10] bpf: document eBPF helpers and add a script to generate man page
From: Quentin Monnet @ 2018-04-25 17:16 UTC (permalink / raw)
  To: daniel, ast; +Cc: netdev, oss-drivers, quentin.monnet, linux-doc, linux-man

eBPF helper functions can be called from within eBPF programs to perform
a variety of tasks that would be otherwise hard or impossible to do with
eBPF itself. There is a growing number of such helper functions in the
kernel, but documentation is scarce. The main user space header file
does contain a short commented description of most helpers, but it is
somewhat outdated and not complete. It is more a "cheat sheet" than a
real documentation accessible to new eBPF developers.

This commit attempts to improve the situation by replacing the existing
overview for the helpers with a more developed description. Furthermore,
a Python script is added to generate a manual page for eBPF helpers. The
workflow is the following, and requires the rst2man utility:

    $ ./scripts/bpf_helpers_doc.py \
            --filename include/uapi/linux/bpf.h > /tmp/bpf-helpers.rst
    $ rst2man /tmp/bpf-helpers.rst > /tmp/bpf-helpers.7
    $ man /tmp/bpf-helpers.7

The objective is to keep all documentation related to the helpers in a
single place, and to be able to generate from here a manual page that
could be packaged in the man-pages repository and shipped with most
distributions.

Additionally, parsing the prototypes of the helper functions could
hopefully be reused, with a different Printer object, to generate
header files needed in some eBPF-related projects.

Regarding the description of each helper, it comprises several items:

- The function prototype.
- A description of the function and of its arguments (except for a
  couple of cases, when there are no arguments and the return value
  makes the function usage really obvious).
- A description of return values (if not void).

Additional items such as the list of compatible eBPF program and map
types for each helper, Linux kernel version that introduced the helper,
GPL-only restriction, and commit hash could be added in the future, but
it was decided on the mailing list to leave them aside for now.

For several helpers, descriptions are inspired (at times, nearly copied)
from the commit logs introducing them in the kernel--Many thanks to
their respective authors! Some sentences were also adapted from comments
from the reviews, thanks to the reviewers as well. Descriptions were
completed as much as possible, the objective being to have something easily
accessible even for people just starting with eBPF. There is probably a bit
more work to do in this direction for some helpers.

Some RST formatting is used in the descriptions (not in function
prototypes, to keep them readable, but the Python script provided in
order to generate the RST for the manual page does add formatting to
prototypes, to produce something pretty) to get "bold" and "italics" in
manual pages. Hopefully, the descriptions in bpf.h file remains
perfectly readable. Note that the few trailing white spaces are
intentional, removing them would break paragraphs for rst2man.

The descriptions should ideally be updated each time someone adds a new
helper, or updates the behaviour (new socket option supported, ...) or
the interface (new flags available, ...) of existing ones.

To ease the review process, the documentation has been split into several
patches.

v3 -> v4:
- Add a patch (#9) for newly added BPF helpers.
- Add a patch (#10) to update UAPI bpf.h version under tools/.
- Use SPDX tag in Python script.
- Several fixes on man page header and footer, and helpers documentation.
  Please refer to individual patches for details.

RFC v2 -> PATCH v3:
Several fixes on man page header and footer, and helpers documentation.
Please refer to individual patches for details.

RFC v1 -> RFC v2:
- Remove "For" (compatible program and map types), "Since" (minimal
  Linux kernel version required), "GPL only" sections and commit hashes
  for the helpers.
- Add comment on top of the description list to explain how this
  documentation is supposed to be processed.
- Update Python script accordingly (remove the same sections, and remove
  paragraphs on program types and GPL restrictions from man page
  header).
- Split series into several patches.

Cc: linux-doc@vger.kernel.org
Cc: linux-man@vger.kernel.org

Quentin Monnet (10):
  bpf: add script and prepare bpf.h for new helpers documentation
  bpf: add documentation for eBPF helpers (01-11)
  bpf: add documentation for eBPF helpers (12-22)
  bpf: add documentation for eBPF helpers (23-32)
  bpf: add documentation for eBPF helpers (33-41)
  bpf: add documentation for eBPF helpers (42-50)
  bpf: add documentation for eBPF helpers (51-57)
  bpf: add documentation for eBPF helpers (58-64)
  bpf: add documentation for eBPF helpers (65-66)
  bpf: update bpf.h uapi header for tools

 include/uapi/linux/bpf.h       | 1776 +++++++++++++++++++++++++++++++---------
 scripts/bpf_helpers_doc.py     |  421 ++++++++++
 tools/include/uapi/linux/bpf.h | 1776 +++++++++++++++++++++++++++++++---------
 3 files changed, 3181 insertions(+), 792 deletions(-)
 create mode 100755 scripts/bpf_helpers_doc.py

-- 
2.14.1

^ permalink raw reply

* RE: [PATCH net-next] lan78xx: Lan7801 Support for Fixed PHY
From: RaghuramChary.Jallipalli @ 2018-04-25 17:04 UTC (permalink / raw)
  To: andrew; +Cc: davem, netdev, UNGLinuxDriver, Woojung.Huh
In-Reply-To: <20180425123906.GA13486@lunn.ch>

Hi Andrew,
> > >
> > dev->fixedphy stores the fixed phydev, which will be passed to the
> > fixed_phy_unregister routine , so I think phy_is_pseudo_fixed_link check
> is not necessary.
> 
> I'm saying you can get rid of dev->fixedphy, and just use
> netdev->phydev, and phy_is_pseudo_fixed_link(netdev->phydev)
> 
After phy_disconnect() , the netdev->phydev becomes null, but the phydev->mdio instances
are still valid. So I'm saving the phydev ptr and passing to unregister the fixed phy.
If I try to unregister first and disconnect, I see panic at sysfs remove link. 
I believe having dev->fixedphy should not cause any problem.

Thanks,
Raghu

^ permalink raw reply

* Re: [PATCH net-next v2] ipv6: addrconf: don't evaluate keep_addr_on_down twice
From: David Miller @ 2018-04-25 17:04 UTC (permalink / raw)
  To: cera; +Cc: netdev, dsahern
In-Reply-To: <20180424175129.6663-1-cera@cera.cz>

From: Ivan Vecera <cera@cera.cz>
Date: Tue, 24 Apr 2018 19:51:29 +0200

> The addrconf_ifdown() evaluates keep_addr_on_down state twice. There
> is no need to do it.
> 
> Cc: David Ahern <dsahern@gmail.com>
> Signed-off-by: Ivan Vecera <cera@cera.cz>

Applied, thanks Ivan.

^ permalink raw reply

* Re: [net-next v3] ipv6: sr: Compute flowlabel for outer IPv6 header of seg6 encap mode
From: David Miller @ 2018-04-25 17:03 UTC (permalink / raw)
  To: amsalam20; +Cc: dav.lebrun, kuznet, yoshfuji, netdev, linux-kernel
In-Reply-To: <1524594196-12383-1-git-send-email-amsalam20@gmail.com>

From: Ahmed Abdelsalam <amsalam20@gmail.com>
Date: Tue, 24 Apr 2018 20:23:16 +0200

> ECMP (equal-cost multipath) hashes are typically computed on the packets'
> 5-tuple(src IP, dst IP, src port, dst port, L4 proto).
> 
> For encapsulated packets, the L4 data is not readily available and ECMP
> hashing will often revert to (src IP, dst IP). This will lead to traffic
> polarization on a single ECMP path, causing congestion and waste of network
> capacity.
> 
> In IPv6, the 20-bit flow label field is also used as part of the ECMP hash.
> In the lack of L4 data, the hashing will be on (src IP, dst IP, flow
> label). Having a non-zero flow label is thus important for proper traffic
> load balancing when L4 data is unavailable (i.e., when packets are
> encapsulated).
> 
> Currently, the seg6_do_srh_encap() function extracts the original packet's
> flow label and set it as the outer IPv6 flow label. There are two issues
> with this behaviour:
> 
> a) There is no guarantee that the inner flow label is set by the source.
> b) If the original packet is not IPv6, the flow label will be set to
> zero (e.g., IPv4 or L2 encap).
> 
> This patch adds a function, named seg6_make_flowlabel(), that computes a
> flow label from a given skb. It supports IPv6, IPv4 and L2 payloads, and
> leverages the per namespace 'seg6_flowlabel" sysctl value.
> 
> The currently support behaviours are as follows:
> -1 set flowlabel to zero.
> 0 copy flowlabel from Inner paceket in case of Inner IPv6
> (Set flowlabel to 0 in case IPv4/L2)
> 1 Compute the flowlabel using seg6_make_flowlabel()
> 
> This patch has been tested for IPv6, IPv4, and L2 traffic.
> 
> Signed-off-by: Ahmed Abdelsalam <amsalam20@gmail.com>

Applied.

Please submit a patch which adds appropriate documentation for this new sysctl
to Documentation/networking/ip-sysctl.txt

Thank you.

^ permalink raw reply

* Re: [PATCH] net: phy: allow scanning busses with missing phys
From: David Miller @ 2018-04-25 17:01 UTC (permalink / raw)
  To: alexandre.belloni
  Cc: andrew, f.fainelli, Allan.Nielsen, thomas.petazzoni, netdev,
	linux-kernel
In-Reply-To: <20180424160904.32457-1-alexandre.belloni@bootlin.com>

From: Alexandre Belloni <alexandre.belloni@bootlin.com>
Date: Tue, 24 Apr 2018 18:09:04 +0200

> Some MDIO busses will error out when trying to read a phy address with no
> phy present at that address. In that case, probing the bus will fail
> because __mdiobus_register() is scanning the bus for all possible phys
> addresses.
> 
> In case MII_PHYSID1 returns -EIO or -ENODEV, consider there is no phy at
> this address and set the phy ID to 0xffffffff which is then properly
> handled in get_phy_device().
> 
> Suggested-by: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@bootlin.com>

Applied, thank you.

^ permalink raw reply

* Re: [PATCH net-next v3] Add Common Applications Kept Enhanced (cake) qdisc
From: Eric Dumazet @ 2018-04-25 16:59 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen, Eric Dumazet, netdev; +Cc: cake, Dave Taht
In-Reply-To: <87tvrz5ipt.fsf@toke.dk>



On 04/25/2018 09:55 AM, Toke Høiland-Jørgensen wrote:

> Well, as I said, 10Gbit+ links are not really the target audience ;)

Well, 640KB of memory is all we need.

> 
> We did actually have a threshold at some point, but it was removed
> because it didn't work well (I'm not sure of the details, perhaps
> someone else will chime in).
> 
> However, I'm fine with adding a flag, as long as peeling defaults to on,
> at least when the shaper is active (to properly account for packet
> overhead we really need to see every packet that goes out on the wire).
> Would that be acceptable?


Not a flag, a threshold based on bandwidth.

^ permalink raw reply

* Re: [Cake] [PATCH net-next v3] Add Common Applications Kept Enhanced (cake) qdisc
From: Eric Dumazet @ 2018-04-25 16:57 UTC (permalink / raw)
  To: Jonathan Morton, Eric Dumazet
  Cc: Toke Høiland-Jørgensen, netdev, cake
In-Reply-To: <CF30AEA0-96EA-4880-93F0-25BC56030288@gmail.com>



On 04/25/2018 09:52 AM, Jonathan Morton wrote:
>> We can see here the high cost of forcing software GSO :/
>>
>> Really, this should be done only :
>> 1) If requested by the admin ( tc .... gso ....)
>>
>> 2) If packet size is above a threshold.
>>  The threshold could be set by the admin, and/or based on a fraction of the bandwidth parameter.
>>
>> I totally understand why you prefer to segment yourself for < 100 Mbit links.
>>
>> But this makes no sense on 10Gbit+
> 
> It is absolutely necessary, so far as I can see, to segment GSO superpackets when overhead compensation is selected - as it very often should be, even on pure Ethernet links.  Without that, the calculation of link occupancy time will be wrong.  (The actual transmission time of an Ethernet frame is rather more than just 14 bytes longer than the underlying IP packet.)

Just fix the overhead compensation computation in the code.

skb in a qdisc have everything you need.

qdisc_pkt_len_init() has initialized qdisc_skb_cb(skb)->pkt_len  with the exact bytes on the wire,
and you have gso_segs to perform any adjustement you need to do.

Do not kill GSO only because you do not want to deal with it.

> 
> Another reason to apply GSO segmentation is to achieve maximal smoothness of flow isolation.  This should still be achievable within some tolerance at high link rates, but calculating this tolerance is complicated by the triple-isolate algorithm.
> 
> If there's a way to obtain the individual packet sizes without incurring the full segmentation overhead, it may be worth considering (at high link rates only).  I would want to leave it on by default, because some of Cake's demonstrably superior latency performance depends on seeing the real packets, not the aggregates, and the overhead only becomes significant above 100Mbps on weak MIPS CPUs and 1Gbps on vaguely modern x86 stuff.
> 
>  - Jonathan Morton
> 

Again, these arguments are moot on 10Gbit+.

Lets build the future, do not pretend GSO/TSO is not part of it.

Thanks

^ permalink raw reply

* Re: [PATCH net-next v3] Add Common Applications Kept Enhanced (cake) qdisc
From: Toke Høiland-Jørgensen @ 2018-04-25 16:55 UTC (permalink / raw)
  To: Eric Dumazet, netdev; +Cc: cake, Dave Taht
In-Reply-To: <8bae2ee1-efcc-1571-2a30-5b7779de2c88@gmail.com>

Eric Dumazet <eric.dumazet@gmail.com> writes:

> On 04/25/2018 09:06 AM, Toke Høiland-Jørgensen wrote:
>> Eric Dumazet <eric.dumazet@gmail.com> writes:
>> 
>>> On 04/25/2018 08:22 AM, Toke Høiland-Jørgensen wrote:
>>>> Eric Dumazet <eric.dumazet@gmail.com> writes:
>>>
>>>>> What performance number do you get on a 10Gbit NIC for example ?
>>>>
>>>> Single-flow throughput through 2 hops on a 40Gbit connection (with CAKE
>>>> in unlimited mode vs pfifo_fast on the router):
>>>>
>>>> MIGRATED TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to testbed-40g-2 () port 0 AF_INET : demo
>>>> Recv   Send    Send                          
>>>> Socket Socket  Message  Elapsed              
>>>> Size   Size    Size     Time     Throughput  
>>>> bytes  bytes   bytes    secs.    10^6bits/sec  
>>>>
>>>>  87380  16384  16384    10.00    18840.40   
>>>>
>>>> MIGRATED TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to testbed-40g-2 () port 0 AF_INET : demo
>>>> Recv   Send    Send                          
>>>> Socket Socket  Message  Elapsed              
>>>> Size   Size    Size     Time     Throughput  
>>>> bytes  bytes   bytes    secs.    10^6bits/sec  
>>>>
>>>>  87380  16384  16384    10.00    24804.77   
>>>
>>> CPU performance would be interesting here.  (netperf -Cc)
>> 
>> 
>> $ sudo tc qdisc replace dev ens2 root cake
>> $ netperf -cC -H 10.70.2.2
>> MIGRATED TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to 10.70.2.2 () port 0 AF_INET : demo
>> Recv   Send    Send                          Utilization       Service Demand
>> Socket Socket  Message  Elapsed              Send     Recv     Send    Recv
>> Size   Size    Size     Time     Throughput  local    remote   local   remote
>> bytes  bytes   bytes    secs.    10^6bits/s  % S      % S      us/KB   us/KB
>> 
>>  87380  16384  16384    10.00      15450.35   13.35    6.68     0.849   0.283  
>> 
>> $ sudo tc qdisc del dev ens2 root 
>> $ netperf -cC -H 10.70.2.2
>> MIGRATED TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to 10.70.2.2 () port 0 AF_INET : demo
>> Recv   Send    Send                          Utilization       Service Demand
>> Socket Socket  Message  Elapsed              Send     Recv     Send    Recv
>> Size   Size    Size     Time     Throughput  local    remote   local   remote
>> bytes  bytes   bytes    secs.    10^6bits/s  % S      % S      us/KB   us/KB
>> 
>>  87380  16384  16384    10.00      36414.23   8.20     14.30    0.221   0.257  
>> 
>> 
>> (In this test I'm running netperf on the machine that was a router
>> before, which is why the base throughput is higher; the other machine
>> runs out of CPU on the sender side).
>
> We can see here the high cost of forcing software GSO :/
>
> Really, this should be done only :
> 1) If requested by the admin ( tc .... gso ....)
>
> 2) If packet size is above a threshold.
>   The threshold could be set by the admin, and/or based on a fraction of the bandwidth parameter.
>
> I totally understand why you prefer to segment yourself for < 100 Mbit links.
>
> But this makes no sense on 10Gbit+

Well, as I said, 10Gbit+ links are not really the target audience ;)

We did actually have a threshold at some point, but it was removed
because it didn't work well (I'm not sure of the details, perhaps
someone else will chime in).

However, I'm fine with adding a flag, as long as peeling defaults to on,
at least when the shaper is active (to properly account for packet
overhead we really need to see every packet that goes out on the wire).
Would that be acceptable?

-Toke

^ permalink raw reply

* Re: [PATCH] rds: ib: Fix missing call to rds_ib_dev_put in rds_ib_setup_qp
From: Santosh Shilimkar @ 2018-04-25 16:53 UTC (permalink / raw)
  To: Dag Moxnes, David S. Miller, netdev, linux-rdma, rds-devel,
	linux-kernel
In-Reply-To: <1524655321-8379-1-git-send-email-dag.moxnes@oracle.com>

On 4/25/2018 4:22 AM, Dag Moxnes wrote:
> The function rds_ib_setup_qp is calling rds_ib_get_client_data and
> should correspondingly call rds_ib_dev_put. This call was lost in
> the non-error path with the introduction of error handling done in
> commit 3b12f73a5c29 ("rds: ib: add error handle")
> 
> Signed-off-by: Dag Moxnes <dag.moxnes@oracle.com>
> ---
Thanks Dag for following it up.

Acked-by: Santosh Shilimkar <santosh.shilimkar@oracle.com>

^ permalink raw reply

* Re: [Cake] [PATCH net-next v3] Add Common Applications Kept Enhanced (cake) qdisc
From: Jonathan Morton @ 2018-04-25 16:52 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Toke Høiland-Jørgensen, netdev, cake
In-Reply-To: <8bae2ee1-efcc-1571-2a30-5b7779de2c88@gmail.com>

> We can see here the high cost of forcing software GSO :/
> 
> Really, this should be done only :
> 1) If requested by the admin ( tc .... gso ....)
> 
> 2) If packet size is above a threshold.
>  The threshold could be set by the admin, and/or based on a fraction of the bandwidth parameter.
> 
> I totally understand why you prefer to segment yourself for < 100 Mbit links.
> 
> But this makes no sense on 10Gbit+

It is absolutely necessary, so far as I can see, to segment GSO superpackets when overhead compensation is selected - as it very often should be, even on pure Ethernet links.  Without that, the calculation of link occupancy time will be wrong.  (The actual transmission time of an Ethernet frame is rather more than just 14 bytes longer than the underlying IP packet.)

Another reason to apply GSO segmentation is to achieve maximal smoothness of flow isolation.  This should still be achievable within some tolerance at high link rates, but calculating this tolerance is complicated by the triple-isolate algorithm.

If there's a way to obtain the individual packet sizes without incurring the full segmentation overhead, it may be worth considering (at high link rates only).  I would want to leave it on by default, because some of Cake's demonstrably superior latency performance depends on seeing the real packets, not the aggregates, and the overhead only becomes significant above 100Mbps on weak MIPS CPUs and 1Gbps on vaguely modern x86 stuff.

 - Jonathan Morton

^ permalink raw reply

* Re: [PATCH bpf-next v6 02/10] bpf: add bpf_get_stack helper
From: Yonghong Song @ 2018-04-25 16:44 UTC (permalink / raw)
  To: Daniel Borkmann, ast, netdev, ecree; +Cc: kernel-team
In-Reply-To: <89ee3446-8119-8bf4-ee69-bd43e4167a2e@iogearbox.net>



On 4/25/18 2:00 AM, Daniel Borkmann wrote:
> On 04/23/2018 11:27 PM, Yonghong Song wrote:
>> Currently, stackmap and bpf_get_stackid helper are provided
>> for bpf program to get the stack trace. This approach has
>> a limitation though. If two stack traces have the same hash,
>> only one will get stored in the stackmap table,
>> so some stack traces are missing from user perspective.
>>
>> This patch implements a new helper, bpf_get_stack, will
>> send stack traces directly to bpf program. The bpf program
>> is able to see all stack traces, and then can do in-kernel
>> processing or send stack traces to user space through
>> shared map or bpf_perf_event_output.
>>
>> Acked-by: Alexei Starovoitov <ast@fb.com>
>> Signed-off-by: Yonghong Song <yhs@fb.com>
> [...]
>> diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
>> index d315b39..bf22eca 100644
>> --- a/kernel/bpf/core.c
>> +++ b/kernel/bpf/core.c
>> @@ -31,6 +31,7 @@
>>   #include <linux/rbtree_latch.h>
>>   #include <linux/kallsyms.h>
>>   #include <linux/rcupdate.h>
>> +#include <linux/perf_event.h>
>>   
>>   #include <asm/unaligned.h>
>>   
>> @@ -1709,6 +1710,10 @@ static void bpf_prog_free_deferred(struct work_struct *work)
>>   	aux = container_of(work, struct bpf_prog_aux, work);
>>   	if (bpf_prog_is_dev_bound(aux))
>>   		bpf_prog_offload_destroy(aux->prog);
>> +#ifdef CONFIG_PERF_EVENTS
>> +	if (aux->prog->need_callchain_buf)
>> +		put_callchain_buffers();
>> +#endif
>>   	for (i = 0; i < aux->func_cnt; i++)
>>   		bpf_jit_free(aux->func[i]);
>>   	if (aux->func_cnt) {
> [...]
>> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
>> index fe23dc5a..1ee71f6 100644
>> --- a/kernel/bpf/syscall.c
>> +++ b/kernel/bpf/syscall.c
>> @@ -1360,6 +1360,16 @@ static int bpf_prog_load(union bpf_attr *attr)
>>   	if (err)
>>   		goto free_used_maps;
>>   
>> +	if (prog->need_callchain_buf) {
>> +#ifdef CONFIG_PERF_EVENTS
>> +		err = get_callchain_buffers(sysctl_perf_event_max_stack);
>> +#else
>> +		err = -ENOTSUPP;
>> +#endif
>> +		if (err)
>> +			goto free_used_maps;
>> +	}
>> +
>>   	err = bpf_prog_new_fd(prog);
>>   	if (err < 0) {
>>   		/* failed to allocate fd.
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index 5dd1dcb..aba9425 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -2460,6 +2460,9 @@ static int check_helper_call(struct bpf_verifier_env *env, int func_id, int insn
>>   	if (err)
>>   		return err;
>>   
>> +	if (func_id == BPF_FUNC_get_stack)
>> +		env->prog->need_callchain_buf = true;
>> +
>>   	if (changes_data)
>>   		clear_all_pkt_pointers(env);
>>   	return 0;
> 
> The above three hunks will cause a use-after-free on the perf callchain buffers.
> 
> In check_helper_call() you mark the prog with need_callchain_buf, where the
> program hasn't fully completed verification phase yet, meaning some buggy prog
> will still bail out.
> 
> However, you do the get_callchain_buffers() at a much later phase, so when you
> bail out with error from bpf_check(), you take the free_used_maps error path
> which calls bpf_prog_free().
> 
> The latter calls into bpf_prog_free_deferred() where you do the put_callchain_buffers()
> since the need_callchain_buf is marked, but without prior get_callchain_buffers().

Thanks for catching this! Yes, this is a real issue. I will fix it and 
respin.

^ permalink raw reply

* Re: [PATCH net-next 1/2] tcp: add TCP_ZEROCOPY_RECEIVE support for zerocopy receive
From: Eric Dumazet @ 2018-04-25 16:44 UTC (permalink / raw)
  To: Andy Lutomirski, Matthew Wilcox
  Cc: Christoph Hellwig, Eric Dumazet, David S . Miller, netdev,
	linux-kernel, linux-mm, Soheil Hassas Yeganeh
In-Reply-To: <155a86d5-a910-c366-f521-216a0582bad8@gmail.com>



On 04/25/2018 09:35 AM, Eric Dumazet wrote:
> 
> 
> On 04/25/2018 09:22 AM, Andy Lutomirski wrote:
> 
>> In general, I suspect that the zerocopy receive mechanism will only
>> really be a win in single-threaded applications that consume large
>> amounts of receive bandwidth on a single TCP socket using lots of
>> memory and don't do all that much else.
> 
> This was dully noted in the original patch submission.
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git/commit/?id=309c446cb45f6663932c8e6d0754f4ac81d1b5cd
> 
> Our intent at Google is to use it for some specific 1MB+ receives, not as a generic and universal mechanism.
> 
> The major benefit is really the 4KB+ MTU, allowing to pack exactly 4096 bytes of payload per page.
> 

Some perf numbers with 10 concurrent threads in tcp_mmap with zero copy enabled :
(tcp_mmap uses 512 KB chunks, not 1MB ones)

received 32768 MB (100 % mmap'ed) in 28.3054 s, 9.71116 Gbit
  cpu usage user:0.039 sys:1.946, 60.5774 usec per MB, 65536 c-switches
received 32768 MB (100 % mmap'ed) in 28.2504 s, 9.73004 Gbit
  cpu usage user:0.052 sys:1.941, 60.8215 usec per MB, 65536 c-switches
received 32768 MB (99.9998 % mmap'ed) in 28.2508 s, 9.72993 Gbit
  cpu usage user:0.056 sys:1.915, 60.1501 usec per MB, 65539 c-switches
received 32768 MB (100 % mmap'ed) in 28.2544 s, 9.72866 Gbit
  cpu usage user:0.053 sys:1.966, 61.615 usec per MB, 65536 c-switches
received 32768 MB (100 % mmap'ed) in 115.985 s, 2.36995 Gbit
  cpu usage user:0.057 sys:2.492, 77.7893 usec per MB, 65536 c-switches
received 32768 MB (100 % mmap'ed) in 62.633 s, 4.38871 Gbit
  cpu usage user:0.048 sys:2.076, 64.8193 usec per MB, 65536 c-switches
received 32768 MB (100 % mmap'ed) in 59.4608 s, 4.62285 Gbit
  cpu usage user:0.047 sys:1.965, 61.4014 usec per MB, 65536 c-switches
received 32768 MB (100 % mmap'ed) in 119.364 s, 2.30285 Gbit
  cpu usage user:0.057 sys:2.757, 85.8765 usec per MB, 65536 c-switches
received 32768 MB (100 % mmap'ed) in 121.37 s, 2.2648 Gbit
  cpu usage user:0.05 sys:2.224, 69.397 usec per MB, 65536 c-switches
received 32768 MB (100 % mmap'ed) in 121.382 s, 2.26457 Gbit
  cpu usage user:0.049 sys:2.163, 67.5049 usec per MB, 65538 c-switches
received 32768 MB (100 % mmap'ed) in 39.7636 s, 6.91281 Gbit
  cpu usage user:0.055 sys:2.053, 64.3311 usec per MB, 65536 c-switches
received 32768 MB (100 % mmap'ed) in 21.2803 s, 12.917 Gbit
  cpu usage user:0.043 sys:2.057, 64.0869 usec per MB, 65537 c-switches

When zero copy is not enabled :

received 32768 MB (0 % mmap'ed) in 49.4301 s, 5.56094 Gbit
  cpu usage user:0.036 sys:6.747, 207.001 usec per MB, 65546 c-switches
received 32768 MB (0 % mmap'ed) in 49.431 s, 5.56084 Gbit
  cpu usage user:0.042 sys:5.262, 161.865 usec per MB, 65540 c-switches
received 32768 MB (0 % mmap'ed) in 84.7254 s, 3.24434 Gbit
  cpu usage user:0.045 sys:5.154, 158.661 usec per MB, 65548 c-switches
received 32768 MB (0 % mmap'ed) in 84.7274 s, 3.24426 Gbit
  cpu usage user:0.043 sys:6.528, 200.531 usec per MB, 65542 c-switches
received 32768 MB (0 % mmap'ed) in 35.3133 s, 7.78398 Gbit
  cpu usage user:0.032 sys:5.066, 155.579 usec per MB, 65540 c-switches
received 32768 MB (0 % mmap'ed) in 35.3137 s, 7.78389 Gbit
  cpu usage user:0.034 sys:6.358, 195.068 usec per MB, 65536 c-switches
received 32768 MB (0 % mmap'ed) in 98.8568 s, 2.78057 Gbit
  cpu usage user:0.042 sys:6.519, 200.226 usec per MB, 65550 c-switches
received 32768 MB (0 % mmap'ed) in 98.8638 s, 2.78037 Gbit
  cpu usage user:0.042 sys:5.243, 161.285 usec per MB, 65545 c-switches
received 32768 MB (0 % mmap'ed) in 108.282 s, 2.53853 Gbit
  cpu usage user:0.059 sys:5.938, 183.014 usec per MB, 65538 c-switches
received 32768 MB (0 % mmap'ed) in 108.314 s, 2.53778 Gbit
  cpu usage user:0.04 sys:6.096, 187.256 usec per MB, 65548 c-switches
received 32768 MB (0 % mmap'ed) in 29.4351 s, 9.33845 Gbit
  cpu usage user:0.041 sys:6.03, 185.272 usec per MB, 65536 c-switches
received 32768 MB (0 % mmap'ed) in 44.3993 s, 6.19104 Gbit
  cpu usage user:0.034 sys:5.115, 157.135 usec per MB, 65535 c-switches
received 32768 MB (0 % mmap'ed) in 79.7203 s, 3.44803 Gbit
  cpu usage user:0.046 sys:5.214, 160.522 usec per MB, 65540 c-switches

^ permalink raw reply

* Re: [PATCH net-next 1/2] tcp: add TCP_ZEROCOPY_RECEIVE support for zerocopy receive
From: Eric Dumazet @ 2018-04-25 16:35 UTC (permalink / raw)
  To: Andy Lutomirski, Matthew Wilcox
  Cc: Eric Dumazet, Christoph Hellwig, Eric Dumazet, David S . Miller,
	netdev, linux-kernel, linux-mm, Soheil Hassas Yeganeh
In-Reply-To: <CALCETrWaekirEe+rKiPB-Zim6ZHKL-n7nfk9wrsHra_FtrS=DA@mail.gmail.com>



On 04/25/2018 09:22 AM, Andy Lutomirski wrote:

> In general, I suspect that the zerocopy receive mechanism will only
> really be a win in single-threaded applications that consume large
> amounts of receive bandwidth on a single TCP socket using lots of
> memory and don't do all that much else.

This was dully noted in the original patch submission.

https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git/commit/?id=309c446cb45f6663932c8e6d0754f4ac81d1b5cd

Our intent at Google is to use it for some specific 1MB+ receives, not as a generic and universal mechanism.

The major benefit is really the 4KB+ MTU, allowing to pack exactly 4096 bytes of payload per page.

^ permalink raw reply

* Re: [PATCH net-next 1/2] tcp: add TCP_ZEROCOPY_RECEIVE support for zerocopy receive
From: Matthew Wilcox @ 2018-04-25 16:30 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Christoph Hellwig, Eric Dumazet, David S . Miller, netdev,
	Andy Lutomirski, linux-kernel, linux-mm, Soheil Hassas Yeganeh
In-Reply-To: <8ce78bd6-8142-2937-11fd-2e4a2b22d90c@gmail.com>

On Wed, Apr 25, 2018 at 09:20:55AM -0700, Eric Dumazet wrote:
> On 04/25/2018 09:04 AM, Matthew Wilcox wrote:
> > If you don't zap the page range, any of the CPUs in the system where
> > any thread in this task have ever run may have a TLB entry pointing to
> > this page ... if the page is being recycled into the page allocator,
> > then that page might end up as a slab page or page table or page cache
> > while the other CPU still have access to it.
> 
> Yes, this makes sense.
> 
> > 
> > You could hang onto the page until you've built up a sufficiently large
> > batch, then bulk-invalidate all of the TLB entries, but we start to get
> > into weirdnesses on different CPU architectures.
> > 
> 
> zap_page_range() is already doing a bulk-invalidate,
> so maybe vm_replace_page() wont bring serious improvement if we end-up doing same dance.

Sorry, I was unclear.  zap_page_range() bulk-invalidates all pages that
were torn down as part of this call.  What I was trying to say was that
we could have a whole new API which put page after page into the same
address, and bumped the refcount on them to prevent them from actually
being freed.  Once we get to a batch limit, we invalidate all of the
pages which were mapped at those addresses and can then free the pages
back to the allocator.

I don't think you can implement this scheme on s390 because it requires
the userspace address to still be mapped to that page on shootdown
(?) but I think we could implement it on x86.

Another possibility is if we had some way to insert the TLB entry into
the local CPU's page tables only, we wouldn't need to broadcast-invalidate
the TLB entry; we could just do it locally which is relatively quick.

^ permalink raw reply

* [net  1/1] tipc: fix bug in function tipc_nl_node_dump_monitor
From: Jon Maloy @ 2018-04-25 16:29 UTC (permalink / raw)
  To: davem, netdev
  Cc: mohan.krishna.ghanta.krishnamurthy, tung.q.nguyen, hoang.h.le,
	jon.maloy, canh.d.luu, ying.xue, tipc-discussion

Commit 36a50a989ee8 ("tipc: fix infinite loop when dumping link monitor
summary") intended to fix a problem with user tool looping when max
number of bearers are enabled.

Unfortunately, the wrong version of the commit was posted, so the
problem was not solved at all.

This commit adds the missing part.

Fixes: 36a50a989ee8 ("tipc: fix infinite loop when dumping link monitor summary")
Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
---
 net/tipc/node.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/tipc/node.c b/net/tipc/node.c
index 6f98b56..baaf93f 100644
--- a/net/tipc/node.c
+++ b/net/tipc/node.c
@@ -2244,7 +2244,7 @@ int tipc_nl_node_dump_monitor(struct sk_buff *skb, struct netlink_callback *cb)
 
 	rtnl_lock();
 	for (bearer_id = prev_bearer; bearer_id < MAX_BEARERS; bearer_id++) {
-		err = __tipc_nl_add_monitor(net, &msg, prev_bearer);
+		err = __tipc_nl_add_monitor(net, &msg, bearer_id);
 		if (err)
 			break;
 	}
-- 
2.1.4

^ permalink raw reply related

* Re: [PATCH net-next v3] Add Common Applications Kept Enhanced (cake) qdisc
From: Eric Dumazet @ 2018-04-25 16:29 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen, Eric Dumazet, netdev; +Cc: cake, Dave Taht
In-Reply-To: <8736zj6zj2.fsf@toke.dk>



On 04/25/2018 09:06 AM, Toke Høiland-Jørgensen wrote:
> Eric Dumazet <eric.dumazet@gmail.com> writes:
> 
>> On 04/25/2018 08:22 AM, Toke Høiland-Jørgensen wrote:
>>> Eric Dumazet <eric.dumazet@gmail.com> writes:
>>
>>>> What performance number do you get on a 10Gbit NIC for example ?
>>>
>>> Single-flow throughput through 2 hops on a 40Gbit connection (with CAKE
>>> in unlimited mode vs pfifo_fast on the router):
>>>
>>> MIGRATED TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to testbed-40g-2 () port 0 AF_INET : demo
>>> Recv   Send    Send                          
>>> Socket Socket  Message  Elapsed              
>>> Size   Size    Size     Time     Throughput  
>>> bytes  bytes   bytes    secs.    10^6bits/sec  
>>>
>>>  87380  16384  16384    10.00    18840.40   
>>>
>>> MIGRATED TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to testbed-40g-2 () port 0 AF_INET : demo
>>> Recv   Send    Send                          
>>> Socket Socket  Message  Elapsed              
>>> Size   Size    Size     Time     Throughput  
>>> bytes  bytes   bytes    secs.    10^6bits/sec  
>>>
>>>  87380  16384  16384    10.00    24804.77   
>>
>> CPU performance would be interesting here.  (netperf -Cc)
> 
> 
> $ sudo tc qdisc replace dev ens2 root cake
> $ netperf -cC -H 10.70.2.2
> MIGRATED TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to 10.70.2.2 () port 0 AF_INET : demo
> Recv   Send    Send                          Utilization       Service Demand
> Socket Socket  Message  Elapsed              Send     Recv     Send    Recv
> Size   Size    Size     Time     Throughput  local    remote   local   remote
> bytes  bytes   bytes    secs.    10^6bits/s  % S      % S      us/KB   us/KB
> 
>  87380  16384  16384    10.00      15450.35   13.35    6.68     0.849   0.283  
> 
> $ sudo tc qdisc del dev ens2 root 
> $ netperf -cC -H 10.70.2.2
> MIGRATED TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to 10.70.2.2 () port 0 AF_INET : demo
> Recv   Send    Send                          Utilization       Service Demand
> Socket Socket  Message  Elapsed              Send     Recv     Send    Recv
> Size   Size    Size     Time     Throughput  local    remote   local   remote
> bytes  bytes   bytes    secs.    10^6bits/s  % S      % S      us/KB   us/KB
> 
>  87380  16384  16384    10.00      36414.23   8.20     14.30    0.221   0.257  
> 
> 
> (In this test I'm running netperf on the machine that was a router
> before, which is why the base throughput is higher; the other machine
> runs out of CPU on the sender side).

We can see here the high cost of forcing software GSO :/

Really, this should be done only :
1) If requested by the admin ( tc .... gso ....)

2) If packet size is above a threshold.
  The threshold could be set by the admin, and/or based on a fraction of the bandwidth parameter.

I totally understand why you prefer to segment yourself for < 100 Mbit links.

But this makes no sense on 10Gbit+

^ permalink raw reply

* Re: [PATCH 3/8] ARM: dts: stm32: add ethernet pins to stm32mp157c
From: Alexandre Torgue @ 2018-04-25 16:22 UTC (permalink / raw)
  To: Rob Herring, Christophe Roullier
  Cc: Mark Rutland, Maxime Coquelin, Giuseppe CAVALLARO, devicetree,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE, netdev
In-Reply-To: <CAL_JsqL7bY=OSuzeVHH2UxxaBmCMzpFc8djnuZAY6VWcTgUmZw@mail.gmail.com>

Hi Rob,

On 04/25/2018 05:09 PM, Rob Herring wrote:
> On Tue, Apr 24, 2018 at 10:01 AM, Christophe Roullier
> <christophe.roullier@st.com> wrote:
>> Add ethernet pins on stm32mp157c.
>>
>> Signed-off-by: Christophe Roullier <christophe.roullier@st.com>
>> ---
>>   arch/arm/boot/dts/stm32mp157-pinctrl.dtsi | 46 +++++++++++++++++++++++++++++++
>>   1 file changed, 46 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/stm32mp157-pinctrl.dtsi b/arch/arm/boot/dts/stm32mp157-pinctrl.dtsi
>> index 6f044100..86720a5 100644
>> --- a/arch/arm/boot/dts/stm32mp157-pinctrl.dtsi
>> +++ b/arch/arm/boot/dts/stm32mp157-pinctrl.dtsi
>> @@ -158,6 +158,52 @@
>>                                          bias-disable;
>>                                  };
>>                          };
>> +
>> +                       ethernet0_rgmii_pins_a: rgmii@0 {
> 
> A unit-address without 'reg' property is not valid, so drop the '@0'.
>
Thanks for the highlights. We could replace rgmii@0 by rgmii-0.
If no objections, I will send a series to update all STM32 device tree 
files (MCUs legacy). I will take care about it for future STM32 DT review.

Thanks
Alex

> Please build your dtb with W=1 or W=12 which will tell you this and
> other errors.
> 

> Rob
> 

^ permalink raw reply

* Re: [PATCH net-next 1/2] tcp: add TCP_ZEROCOPY_RECEIVE support for zerocopy receive
From: Andy Lutomirski @ 2018-04-25 16:22 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Eric Dumazet, Christoph Hellwig, Eric Dumazet, David S . Miller,
	netdev, Andy Lutomirski, linux-kernel, linux-mm,
	Soheil Hassas Yeganeh
In-Reply-To: <20180425160413.GC8546@bombadil.infradead.org>

On Wed, Apr 25, 2018 at 9:04 AM, Matthew Wilcox <willy@infradead.org> wrote:
> On Wed, Apr 25, 2018 at 06:01:02AM -0700, Eric Dumazet wrote:
>> On 04/24/2018 11:28 PM, Christoph Hellwig wrote:
>> > On Tue, Apr 24, 2018 at 10:27:21PM -0700, Eric Dumazet wrote:
>> >> When adding tcp mmap() implementation, I forgot that socket lock
>> >> had to be taken before current->mm->mmap_sem. syzbot eventually caught
>> >> the bug.
>> >>
>> >> Since we can not lock the socket in tcp mmap() handler we have to
>> >> split the operation in two phases.
>> >>
>> >> 1) mmap() on a tcp socket simply reserves VMA space, and nothing else.
>> >>   This operation does not involve any TCP locking.
>> >>
>> >> 2) setsockopt(fd, IPPROTO_TCP, TCP_ZEROCOPY_RECEIVE, ...) implements
>> >>  the transfert of pages from skbs to one VMA.
>> >>   This operation only uses down_read(&current->mm->mmap_sem) after
>> >>   holding TCP lock, thus solving the lockdep issue.
>> >>
>> >> This new implementation was suggested by Andy Lutomirski with great details.
>> >
>> > Thanks, this looks much more sensible to me.
>> >
>>
>> Thanks Christoph
>>
>> Note the high cost of zap_page_range(), needed to avoid -EBUSY being returned
>> from vm_insert_page() the second time TCP_ZEROCOPY_RECEIVE is used on one VMA.
>>
>> Ideally a vm_replace_page() would avoid this cost ?
>
> If you don't zap the page range, any of the CPUs in the system where
> any thread in this task have ever run may have a TLB entry pointing to
> this page ... if the page is being recycled into the page allocator,
> then that page might end up as a slab page or page table or page cache
> while the other CPU still have access to it.

Indeed.  This is one of the reasons that Linus has generally been
quite vocal that he doesn't like MMU-based zerocopy schemes.

>
> You could hang onto the page until you've built up a sufficiently large
> batch, then bulk-invalidate all of the TLB entries, but we start to get
> into weirdnesses on different CPU architectures.

The existing mmu_gather code should already handle this at least
moderately well.  If it's not, then it should be fixed.

On x86, there is no operation to flush a range of addresses.  You can
flush one address or you can flush all of them.  If you flush one page
at a time, then you might never recover the performance of a plain old
memcpy().  If you flush all of them, then you're hurting the
performance of everything else in the task.

In general, I suspect that the zerocopy receive mechanism will only
really be a win in single-threaded applications that consume large
amounts of receive bandwidth on a single TCP socket using lots of
memory and don't do all that much else.

^ permalink raw reply

* Re: [PATCH net-next 1/2] tcp: add TCP_ZEROCOPY_RECEIVE support for zerocopy receive
From: Eric Dumazet @ 2018-04-25 16:20 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Christoph Hellwig, Eric Dumazet, David S . Miller, netdev,
	Andy Lutomirski, linux-kernel, linux-mm, Soheil Hassas Yeganeh
In-Reply-To: <20180425160413.GC8546@bombadil.infradead.org>



On 04/25/2018 09:04 AM, Matthew Wilcox wrote:

> If you don't zap the page range, any of the CPUs in the system where
> any thread in this task have ever run may have a TLB entry pointing to
> this page ... if the page is being recycled into the page allocator,
> then that page might end up as a slab page or page table or page cache
> while the other CPU still have access to it.

Yes, this makes sense.

> 
> You could hang onto the page until you've built up a sufficiently large
> batch, then bulk-invalidate all of the TLB entries, but we start to get
> into weirdnesses on different CPU architectures.
> 

zap_page_range() is already doing a bulk-invalidate,
so maybe vm_replace_page() wont bring serious improvement if we end-up doing same dance.

Thanks.

^ permalink raw reply

* Re: [PATCH 0/3] bpf: Store/dump license string for loaded program
From: Jiri Olsa @ 2018-04-25 16:20 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Jiri Olsa, Alexei Starovoitov, Daniel Borkmann, lkml, netdev,
	Quentin Monnet
In-Reply-To: <20180425161527.w7xgpg75npdx2zmb@ast-mbp>

On Wed, Apr 25, 2018 at 10:15:29AM -0600, Alexei Starovoitov wrote:
> On Wed, Apr 25, 2018 at 12:17:13PM +0200, Jiri Olsa wrote:
> > On Mon, Apr 23, 2018 at 02:11:36PM -0600, Alexei Starovoitov wrote:
> > > On Mon, Apr 23, 2018 at 08:59:24AM +0200, Jiri Olsa wrote:
> > > > hi,
> > > > sending the change to store and dump the license
> > > > info for loaded BPF programs. It's important for
> > > > us get the license info, when investigating on
> > > > screwed up machine.
> > > 
> > > hmm. boolean flag whether bpf prog is gpl or not
> > > is already exposed via bpf_prog_info.
> > 
> > hum, I can't see that (on bpf-next/master)
> > would the attached change be ok with you?
> > 
> > jirka
> > 
> > 
> > ---
> > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > index e6679393b687..2ce9c9d41c2b 100644
> > --- a/include/uapi/linux/bpf.h
> > +++ b/include/uapi/linux/bpf.h
> > @@ -1062,6 +1062,7 @@ struct bpf_prog_info {
> >  	__u32 ifindex;
> >  	__u64 netns_dev;
> >  	__u64 netns_ino;
> > +	__u16 gpl_compatible:1;
> >  } __attribute__((aligned(8)));
> 
> ahh. I swear there were patches to add it and I thought we accepted them.
> Also just noticed that commit 675fc275a3a2d added 4-byte hole in there.
> So I'm thinking we can fill the hole with 
>  	__u32 ifindex;
> +	__u32 gpl_compatible:1;
>  	__u64 netns_dev;
>  	__u64 netns_ino;
> 
> and keep adding bit fields in there without breaking user space.
> Such patch would need to go to bpf tree.
> 

ok, will post v2 with that

thanks,
jirka

^ permalink raw reply

* Re: [PATCH net-next v3] Add Common Applications Kept Enhanced (cake) qdisc
From: Toke Høiland-Jørgensen @ 2018-04-25 16:17 UTC (permalink / raw)
  To: Eric Dumazet, netdev; +Cc: cake, Dave Taht
In-Reply-To: <fd92ddfd-e2a6-6ea9-ccf6-d7eef3a5f207@gmail.com>

Eric Dumazet <eric.dumazet@gmail.com> writes:

> On 04/25/2018 08:22 AM, Toke Høiland-Jørgensen wrote:
>> Eric Dumazet <eric.dumazet@gmail.com> writes:
>
>>> Lack of any pskb_may_pull() is really concerning.
>> 
>> By this you mean "check that the packet is long enough to contain the
>> header we are looking for before trying to do ACK filtering", right?
>
>
> skb->len is not enough, you also have skb->data_len that matters.
>
> A qdisc can be fed with skbs that are not linear, or pretend to be
> TCP, but they be truncated by malicious sender.
>
> skb might have headers or payload in the page fragments, thus we
> generally have to call pskb_may_pull() to bring headers in skb->head

Right, gotcha; was just making sure I understood you correctly.

> Quite frankly , an ack-filter does not belong to a packet scheduler.
>
> It might be added to tcp conntrack module _if_ someone really cares.

Well, integrating it with the queue does have the advantage of only
filtering ACKs if the flow is actually bottlenecked...

That being said, I'm not personally hugely attached to the ACK filter;
let's see if someone one the cake list speaks up in favour of keeping
it.

Or am I to interpret that as a hard NAK on having this feature in CAKE
(even if we fix the issues you pointed out)?

-Toke

^ permalink raw reply

* Re: [PATCH 0/3] bpf: Store/dump license string for loaded program
From: Alexei Starovoitov @ 2018-04-25 16:15 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Jiri Olsa, Alexei Starovoitov, Daniel Borkmann, lkml, netdev,
	Quentin Monnet
In-Reply-To: <20180425101713.GB3396@krava>

On Wed, Apr 25, 2018 at 12:17:13PM +0200, Jiri Olsa wrote:
> On Mon, Apr 23, 2018 at 02:11:36PM -0600, Alexei Starovoitov wrote:
> > On Mon, Apr 23, 2018 at 08:59:24AM +0200, Jiri Olsa wrote:
> > > hi,
> > > sending the change to store and dump the license
> > > info for loaded BPF programs. It's important for
> > > us get the license info, when investigating on
> > > screwed up machine.
> > 
> > hmm. boolean flag whether bpf prog is gpl or not
> > is already exposed via bpf_prog_info.
> 
> hum, I can't see that (on bpf-next/master)
> would the attached change be ok with you?
> 
> jirka
> 
> 
> ---
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index e6679393b687..2ce9c9d41c2b 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -1062,6 +1062,7 @@ struct bpf_prog_info {
>  	__u32 ifindex;
>  	__u64 netns_dev;
>  	__u64 netns_ino;
> +	__u16 gpl_compatible:1;
>  } __attribute__((aligned(8)));

ahh. I swear there were patches to add it and I thought we accepted them.
Also just noticed that commit 675fc275a3a2d added 4-byte hole in there.
So I'm thinking we can fill the hole with 
 	__u32 ifindex;
+	__u32 gpl_compatible:1;
 	__u64 netns_dev;
 	__u64 netns_ino;

and keep adding bit fields in there without breaking user space.
Such patch would need to go to bpf tree.

^ permalink raw reply

* Re: [PATCH net-next v3] Add Common Applications Kept Enhanced (cake) qdisc
From: Toke Høiland-Jørgensen @ 2018-04-25 16:06 UTC (permalink / raw)
  To: Eric Dumazet, netdev; +Cc: cake, Dave Taht
In-Reply-To: <6bc11ded-028f-6c8f-964e-a569b4e10813@gmail.com>

Eric Dumazet <eric.dumazet@gmail.com> writes:

> On 04/25/2018 08:22 AM, Toke Høiland-Jørgensen wrote:
>> Eric Dumazet <eric.dumazet@gmail.com> writes:
>
>>> What performance number do you get on a 10Gbit NIC for example ?
>> 
>> Single-flow throughput through 2 hops on a 40Gbit connection (with CAKE
>> in unlimited mode vs pfifo_fast on the router):
>> 
>> MIGRATED TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to testbed-40g-2 () port 0 AF_INET : demo
>> Recv   Send    Send                          
>> Socket Socket  Message  Elapsed              
>> Size   Size    Size     Time     Throughput  
>> bytes  bytes   bytes    secs.    10^6bits/sec  
>> 
>>  87380  16384  16384    10.00    18840.40   
>> 
>> MIGRATED TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to testbed-40g-2 () port 0 AF_INET : demo
>> Recv   Send    Send                          
>> Socket Socket  Message  Elapsed              
>> Size   Size    Size     Time     Throughput  
>> bytes  bytes   bytes    secs.    10^6bits/sec  
>> 
>>  87380  16384  16384    10.00    24804.77   
>
> CPU performance would be interesting here.  (netperf -Cc)


$ sudo tc qdisc replace dev ens2 root cake
$ netperf -cC -H 10.70.2.2
MIGRATED TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to 10.70.2.2 () port 0 AF_INET : demo
Recv   Send    Send                          Utilization       Service Demand
Socket Socket  Message  Elapsed              Send     Recv     Send    Recv
Size   Size    Size     Time     Throughput  local    remote   local   remote
bytes  bytes   bytes    secs.    10^6bits/s  % S      % S      us/KB   us/KB

 87380  16384  16384    10.00      15450.35   13.35    6.68     0.849   0.283  

$ sudo tc qdisc del dev ens2 root 
$ netperf -cC -H 10.70.2.2
MIGRATED TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to 10.70.2.2 () port 0 AF_INET : demo
Recv   Send    Send                          Utilization       Service Demand
Socket Socket  Message  Elapsed              Send     Recv     Send    Recv
Size   Size    Size     Time     Throughput  local    remote   local   remote
bytes  bytes   bytes    secs.    10^6bits/s  % S      % S      us/KB   us/KB

 87380  16384  16384    10.00      36414.23   8.20     14.30    0.221   0.257  


(In this test I'm running netperf on the machine that was a router
before, which is why the base throughput is higher; the other machine
runs out of CPU on the sender side).

-Toke

^ permalink raw reply

* [PATCH 12/17] y2038: aio: Prepare sys_io_getevents for __kernel_timespec
From: Arnd Bergmann @ 2018-04-25 16:03 UTC (permalink / raw)
  To: y2038, linux-kernel
  Cc: Arnd Bergmann, linux-api, linux-arch, libc-alpha, tglx, netdev,
	deepa.kernel, viro, albert.aribaud, Peter Zijlstra, Darren Hart,
	Eric W. Biederman, Dominik Brodowski
In-Reply-To: <20180425160311.2718314-1-arnd@arndb.de>

This is a preparation patch for converting sys_io_getevents to
work with 64-bit time_t on 32-bit architectures. The 'timeout' argument
is changed to struct __kernel_timespec, which will be redefined using
64-bit time_t in the future. The compat version of the system call in
turn is enabled for compilation with CONFIG_COMPAT_32BIT_TIME so
the individual 32-bit architectures can share the handling of the
traditional argument with 64-bit architectures providing it for their
compat mode.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 fs/aio.c                 | 4 ++--
 include/linux/syscalls.h | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/aio.c b/fs/aio.c
index 88d7927ffbc6..64fea8c27b16 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -1858,7 +1858,7 @@ SYSCALL_DEFINE5(io_getevents, aio_context_t, ctx_id,
 		long, min_nr,
 		long, nr,
 		struct io_event __user *, events,
-		struct timespec __user *, timeout)
+		struct __kernel_timespec __user *, timeout)
 {
 	struct timespec64	ts;
 
@@ -1870,7 +1870,7 @@ SYSCALL_DEFINE5(io_getevents, aio_context_t, ctx_id,
 	return do_io_getevents(ctx_id, min_nr, nr, events, timeout ? &ts : NULL);
 }
 
-#ifdef CONFIG_COMPAT
+#ifdef CONFIG_COMPAT_32BIT_TIME
 COMPAT_SYSCALL_DEFINE5(io_getevents, compat_aio_context_t, ctx_id,
 		       compat_long_t, min_nr,
 		       compat_long_t, nr,
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 2619678359ee..ffd8674e9df7 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -289,7 +289,7 @@ asmlinkage long sys_io_getevents(aio_context_t ctx_id,
 				long min_nr,
 				long nr,
 				struct io_event __user *events,
-				struct timespec __user *timeout);
+				struct __kernel_timespec __user *timeout);
 
 /* fs/xattr.c */
 asmlinkage long sys_setxattr(const char __user *path, const char __user *name,
-- 
2.9.0

^ permalink raw reply related

* [PATCH 17/17] y2038: signal: Add compat_sys_rt_sigtimedwait_time64
From: Arnd Bergmann @ 2018-04-25 16:03 UTC (permalink / raw)
  To: y2038, linux-kernel
  Cc: Arnd Bergmann, linux-api, linux-arch, libc-alpha, tglx, netdev,
	deepa.kernel, viro, albert.aribaud, Peter Zijlstra, Darren Hart,
	Eric W. Biederman, Dominik Brodowski
In-Reply-To: <20180425160311.2718314-1-arnd@arndb.de>

Now that 32-bit architectures have two variants of
sys_rt_sigtimedwaid() for 32-bit and 64-bit time_t, we also
need to have a second compat system call entry point on the
corresponding 64-bit architectures.

The traditional system call keeps getting handled
by compat_sys_rt_sigtimedwait(), and this adds a new
compat_sys_rt_sigtimedwait_time64() that differs only in
the timeout argument type.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 kernel/signal.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/kernel/signal.c b/kernel/signal.c
index 72609c6835fd..1927fcfa7077 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -3249,6 +3249,38 @@ COMPAT_SYSCALL_DEFINE4(rt_sigtimedwait, compat_sigset_t __user *, uthese,
 }
 #endif
 
+#ifdef CONFIG_COMPAT
+COMPAT_SYSCALL_DEFINE4(rt_sigtimedwait_time64, compat_sigset_t __user *, uthese,
+		struct compat_siginfo __user *, uinfo,
+		struct __kernel_timespec __user *, uts, compat_size_t, sigsetsize)
+{
+	sigset_t s;
+	struct timespec64 t;
+	siginfo_t info;
+	long ret;
+
+	if (sigsetsize != sizeof(sigset_t))
+		return -EINVAL;
+
+	if (get_compat_sigset(&s, uthese))
+		return -EFAULT;
+
+	if (uts) {
+		if (get_timespec64(&t, uts))
+			return -EFAULT;
+	}
+
+	ret = do_sigtimedwait(&s, &info, uts ? &t : NULL);
+
+	if (ret > 0 && uinfo) {
+		if (copy_siginfo_to_user32(uinfo, &info))
+			ret = -EFAULT;
+	}
+
+	return ret;
+}
+#endif
+
 /**
  *  sys_kill - send a signal to a process
  *  @pid: the PID of the process
-- 
2.9.0

^ 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