* Re: [bpf PATCH 2/2] bpf: parse and verdict prog attach may race with bpf map update
From: John Fastabend @ 2018-05-17 21:06 UTC (permalink / raw)
To: Daniel Borkmann, ast; +Cc: netdev
In-Reply-To: <ca36c044-2da2-8ced-529a-b5244e9232ee@iogearbox.net>
On 05/17/2018 01:31 PM, Daniel Borkmann wrote:
> On 05/16/2018 11:46 PM, John Fastabend wrote:
>> In the sockmap design BPF programs (SK_SKB_STREAM_PARSER and
>> SK_SKB_STREAM_VERDICT) are attached to the sockmap map type and when
>> a sock is added to the map the programs are used by the socket.
>> However, sockmap updates from both userspace and BPF programs can
>> happen concurrently with the attach and detach of these programs.
>>
>> To resolve this we use the bpf_prog_inc_not_zero and a READ_ONCE()
>> primitive to ensure the program pointer is not refeched and
>> possibly NULL'd before the refcnt increment. This happens inside
>> a RCU critical section so although the pointer reference in the map
>> object may be NULL (by a concurrent detach operation) the reference
>> from READ_ONCE will not be free'd until after grace period. This
>> ensures the object returned by READ_ONCE() is valid through the
>> RCU criticl section and safe to use as long as we "know" it may
>> be free'd shortly.
>>
>> Daniel spotted a case in the sock update API where instead of using
>> the READ_ONCE() program reference we used the pointer from the
>> original map, stab->bpf_{verdict|parse}. The problem with this is
>> the logic checks the object returned from the READ_ONCE() is not
>> NULL and then tries to reference the object again but using the
>> above map pointer, which may have already been NULL'd by a parallel
>> detach operation. If this happened bpf_porg_inc_not_zero could
>> dereference a NULL pointer.
>>
>> Fix this by using variable returned by READ_ONCE() that is checked
>> for NULL.
>>
>> Fixes: 2f857d04601a ("bpf: sockmap, remove STRPARSER map_flags and add multi-map support")
>> Reported-by: Daniel Borkmann <daniel@iogearbox.net>
>> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
>> ---
[...]
> Isn't the same sort of behavior also possible with the bpf_prog_inc_not_zero(stab->bpf_tx_msg)?
> Meaning, we now have verdict and parse covered with the patch, but the original tx_msg we
> fetched earlier via READ_ONCE() where same would apply not (yet)?
>
Yes, will send a v2 and fix both cases in one shot.
^ permalink raw reply
* [bpf PATCH v2 2/2] bpf: parse and verdict prog attach may race with bpf map update
From: John Fastabend @ 2018-05-17 21:06 UTC (permalink / raw)
To: ast, daniel; +Cc: netdev
In-Reply-To: <20180517210635.13283.94472.stgit@john-Precision-Tower-5810>
In the sockmap design BPF programs (SK_SKB_STREAM_PARSER,
SK_SKB_STREAM_VERDICT and SK_MSG_VERDICT) are attached to the sockmap
map type and when a sock is added to the map the programs are used by
the socket. However, sockmap updates from both userspace and BPF
programs can happen concurrently with the attach and detach of these
programs.
To resolve this we use the bpf_prog_inc_not_zero and a READ_ONCE()
primitive to ensure the program pointer is not refeched and
possibly NULL'd before the refcnt increment. This happens inside
a RCU critical section so although the pointer reference in the map
object may be NULL (by a concurrent detach operation) the reference
from READ_ONCE will not be free'd until after grace period. This
ensures the object returned by READ_ONCE() is valid through the
RCU criticl section and safe to use as long as we "know" it may
be free'd shortly.
Daniel spotted a case in the sock update API where instead of using
the READ_ONCE() program reference we used the pointer from the
original map, stab->bpf_{verdict|parse|txmsg}. The problem with this
is the logic checks the object returned from the READ_ONCE() is not
NULL and then tries to reference the object again but using the
above map pointer, which may have already been NULL'd by a parallel
detach operation. If this happened bpf_porg_inc_not_zero could
dereference a NULL pointer.
Fix this by using variable returned by READ_ONCE() that is checked
for NULL.
Fixes: 2f857d04601a ("bpf: sockmap, remove STRPARSER map_flags and add multi-map support")
Reported-by: Daniel Borkmann <daniel@iogearbox.net>
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
Acked-by: Martin KaFai Lau <kafai@fb.com>
---
kernel/bpf/sockmap.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/kernel/bpf/sockmap.c b/kernel/bpf/sockmap.c
index f03aaa8..95a84b2 100644
--- a/kernel/bpf/sockmap.c
+++ b/kernel/bpf/sockmap.c
@@ -1703,11 +1703,11 @@ static int sock_map_ctx_update_elem(struct bpf_sock_ops_kern *skops,
* we increment the refcnt. If this is the case abort with an
* error.
*/
- verdict = bpf_prog_inc_not_zero(stab->bpf_verdict);
+ verdict = bpf_prog_inc_not_zero(verdict);
if (IS_ERR(verdict))
return PTR_ERR(verdict);
- parse = bpf_prog_inc_not_zero(stab->bpf_parse);
+ parse = bpf_prog_inc_not_zero(parse);
if (IS_ERR(parse)) {
bpf_prog_put(verdict);
return PTR_ERR(parse);
@@ -1715,7 +1715,7 @@ static int sock_map_ctx_update_elem(struct bpf_sock_ops_kern *skops,
}
if (tx_msg) {
- tx_msg = bpf_prog_inc_not_zero(stab->bpf_tx_msg);
+ tx_msg = bpf_prog_inc_not_zero(tx_msg);
if (IS_ERR(tx_msg)) {
if (parse && verdict) {
bpf_prog_put(parse);
^ permalink raw reply related
* Re: [PATCH net v2] net: test tailroom before appending to linear skb
From: David Miller @ 2018-05-17 21:07 UTC (permalink / raw)
To: willemdebruijn.kernel; +Cc: netdev, willemb
In-Reply-To: <20180517171329.201710-1-willemdebruijn.kernel@gmail.com>
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Date: Thu, 17 May 2018 13:13:29 -0400
> From: Willem de Bruijn <willemb@google.com>
>
> Device features may change during transmission. In particular with
> corking, a device may toggle scatter-gather in between allocating
> and writing to an skb.
>
> Do not unconditionally assume that !NETIF_F_SG at write time implies
> that the same held at alloc time and thus the skb has sufficient
> tailroom.
>
> This issue predates git history.
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Reported-by: Eric Dumazet <edumazet@google.com>
> Signed-off-by: Willem de Bruijn <willemb@google.com>
>
> ---
>
> v2: fix ipv4 boundary condition
Applied and queued up for -stable, thanks Willem.
^ permalink raw reply
* Re: [PATCH net-next 1/1] qede: Add build_skb() support.
From: David Miller @ 2018-05-17 21:08 UTC (permalink / raw)
To: manish.chopra; +Cc: netdev, ariel.elior, michal.kalderon
In-Reply-To: <20180517190500.24814-1-manish.chopra@cavium.com>
From: Manish Chopra <manish.chopra@cavium.com>
Date: Thu, 17 May 2018 12:05:00 -0700
> This patch makes use of build_skb() throughout in driver's receieve
> data path [HW gro flow and non HW gro flow]. With this, driver can
> build skb directly from the page segments which are already mapped
> to the hardware instead of allocating new SKB via netdev_alloc_skb()
> and memcpy the data which is quite costly.
>
> This really improves performance (keeping same or slight gain in rx
> throughput) in terms of CPU utilization which is significantly reduced
> [almost half] in non HW gro flow where for every incoming MTU sized
> packet driver had to allocate skb, memcpy headers etc. Additionally
> in that flow, it also gets rid of bunch of additional overheads
> [eth_get_headlen() etc.] to split headers and data in the skb.
>
> Tested with:
> system: 2 sockets, 4 cores per socket, hyperthreading, 2x4x2=16 cores
> iperf [server]: iperf -s
> iperf [client]: iperf -c <server_ip> -t 500 -i 10 -P 32
>
> HW GRO off – w/o build_skb(), throughput: 36.8 Gbits/sec
>
> Average: CPU %usr %nice %sys %iowait %irq %soft %steal %guest %idle
> Average: all 0.59 0.00 32.93 0.00 0.00 43.07 0.00 0.00 23.42
>
> HW GRO off - with build_skb(), throughput: 36.9 Gbits/sec
>
> Average: CPU %usr %nice %sys %iowait %irq %soft %steal %guest %idle
> Average: all 0.70 0.00 31.70 0.00 0.00 25.68 0.00 0.00 41.92
^^^^^ ^^^^^
>
> HW GRO on - w/o build_skb(), throughput: 36.9 Gbits/sec
>
> Average: CPU %usr %nice %sys %iowait %irq %soft %steal %guest %idle
> Average: all 0.86 0.00 24.14 0.00 0.00 6.59 0.00 0.00 68.41
>
> HW GRO on - with build_skb(), throughput: 37.5 Gbits/sec
>
> Average: CPU %usr %nice %sys %iowait %irq %soft %steal %guest %idle
> Average: all 0.87 0.00 23.75 0.00 0.00 6.19 0.00 0.00 69.19
>
> Signed-off-by: Ariel Elior <ariel.elior@cavium.com>
> Signed-off-by: Manish Chopra <manish.chopra@cavium.com>
Looks great, applied, thank you.
^ permalink raw reply
* Re: [patch net-next RFC 04/12] dsa: set devlink port attrs for dsa ports
From: Andrew Lunn @ 2018-05-17 21:08 UTC (permalink / raw)
To: Jiri Pirko
Cc: Florian Fainelli, netdev, davem, idosch, jakub.kicinski, mlxsw,
vivien.didelot, michael.chan, ganeshgr, saeedm, simon.horman,
pieter.jansenvanvuuren, john.hurley, dirk.vandermerwe,
alexander.h.duyck, ogerlitz, dsahern, vijaya.guvva,
satananda.burla, raghu.vatsavayi, felix.manlunas, gospo,
sathya.perla, vasundhara-v.volam, tariqt, eranbe,
jeffrey.t.kirsher
In-Reply-To: <20180517204855.GX1972@nanopsycho>
On Thu, May 17, 2018 at 10:48:55PM +0200, Jiri Pirko wrote:
> Thu, May 17, 2018 at 09:14:32PM CEST, f.fainelli@gmail.com wrote:
> >On 05/17/2018 10:39 AM, Jiri Pirko wrote:
> >>>> That is compiled inside "fixed_phy", isn't it?
> >>>
> >>> It matches what CONFIG_FIXED_PHY is, so if it's built-in it also becomes
> >>> built-in, if is modular, it is also modular, this was fixed with
> >>> 40013ff20b1beed31184935fc0aea6a859d4d4ef ("net: dsa: Fix functional
> >>> dsa-loop dependency on FIXED_PHY")
> >>
> >> Now I have it compiled as module, and after modprobe dsa_loop I see:
> >> [ 1168.129202] libphy: Fixed MDIO Bus: probed
> >> [ 1168.222716] dsa-loop fixed-0:1f: DSA mockup driver: 0x1f
> >>
> >> This messages I did not see when I had fixed_phy compiled as buildin.
> >>
> >> But I still see no netdevs :/
> >
> >The platform data assumes there is a network device named "eth0" as the
>
> Oups, I missed, I created dummy device and modprobed again. Now I see:
>
> $ sudo devlink port
> mdio_bus/fixed-0:1f/0: type eth netdev lan1
> mdio_bus/fixed-0:1f/1: type eth netdev lan2
> mdio_bus/fixed-0:1f/2: type eth netdev lan3
> mdio_bus/fixed-0:1f/3: type eth netdev lan4
> mdio_bus/fixed-0:1f/4: type notset
> mdio_bus/fixed-0:1f/5: type notset
> mdio_bus/fixed-0:1f/6: type notset
> mdio_bus/fixed-0:1f/7: type notset
> mdio_bus/fixed-0:1f/8: type notset
> mdio_bus/fixed-0:1f/9: type notset
> mdio_bus/fixed-0:1f/10: type notset
> mdio_bus/fixed-0:1f/11: type notset
>
> I wonder why there are ports 4-11
Hi Jiri
ds = dsa_switch_alloc(&mdiodev->dev, DSA_MAX_PORTS);
It is allocating a switch with 12 ports. However only 4 of them have
names. So the core only creates slave devices for those 4.
This is a useful test. Real hardware often has unused ports. A WiFi AP
with a 7 port switch which only uses 6 ports is often seen.
Andrew
^ permalink raw reply
* Re: [RFC PATCH ghak32 V2 03/13] audit: log container info of syscalls
From: Steve Grubb @ 2018-05-17 21:09 UTC (permalink / raw)
To: Richard Guy Briggs
Cc: cgroups, containers, linux-api, Linux-Audit Mailing List,
linux-fsdevel, LKML, netdev, ebiederm, luto, jlayton, carlos,
dhowells, viro, simo, eparis, serge
In-Reply-To: <6768d20c636df65534f8d325529669bb30a58382.1521179281.git.rgb@redhat.com>
On Fri, 16 Mar 2018 05:00:30 -0400
Richard Guy Briggs <rgb@redhat.com> wrote:
> Create a new audit record AUDIT_CONTAINER_INFO to document the
> container ID of a process if it is present.
As mentioned in a previous email, I think AUDIT_CONTAINER is more
suitable for the container record. One more comment below...
> Called from audit_log_exit(), syscalls are covered.
>
> A sample raw event:
> type=SYSCALL msg=audit(1519924845.499:257): arch=c000003e syscall=257
> success=yes exit=3 a0=ffffff9c a1=56374e1cef30 a2=241 a3=1b6 items=2
> ppid=606 pid=635 auid=0 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0
> sgid=0 fsgid=0 tty=pts0 ses=3 comm="bash" exe="/usr/bin/bash"
> subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
> key="tmpcontainerid" type=CWD msg=audit(1519924845.499:257):
> cwd="/root" type=PATH msg=audit(1519924845.499:257): item=0
> name="/tmp/" inode=13863 dev=00:27 mode=041777 ouid=0 ogid=0
> rdev=00:00 obj=system_u:object_r:tmp_t:s0 nametype= PARENT
> cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0 cap_fver=0
> type=PATH msg=audit(1519924845.499:257): item=1
> name="/tmp/tmpcontainerid" inode=17729 dev=00:27 mode=0100644 ouid=0
> ogid=0 rdev=00:00 obj=unconfined_u:object_r:user_tmp_t:s0
> nametype=CREATE cap_fp=0000000000000000 cap_fi=0000000000000000
> cap_fe=0 cap_fver=0 type=PROCTITLE msg=audit(1519924845.499:257):
> proctitle=62617368002D6300736C65657020313B206563686F2074657374203E202F746D702F746D70636F6E7461696E65726964
> type=CONTAINER_INFO msg=audit(1519924845.499:257): op=task
> contid=123458
>
> See: https://github.com/linux-audit/audit-kernel/issues/32
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> ---
> include/linux/audit.h | 5 +++++
> include/uapi/linux/audit.h | 1 +
> kernel/audit.c | 20 ++++++++++++++++++++
> kernel/auditsc.c | 2 ++
> 4 files changed, 28 insertions(+)
>
> diff --git a/include/linux/audit.h b/include/linux/audit.h
> index fe4ba3f..3acbe9d 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -154,6 +154,8 @@ extern void
> audit_log_link_denied(const char *operation, extern int
> audit_log_task_context(struct audit_buffer *ab); extern void
> audit_log_task_info(struct audit_buffer *ab, struct task_struct *tsk);
> +extern int audit_log_container_info(struct task_struct *tsk,
> + struct audit_context *context);
>
> extern int audit_update_lsm_rules(void);
>
> @@ -205,6 +207,9 @@ static inline int audit_log_task_context(struct
> audit_buffer *ab) static inline void audit_log_task_info(struct
> audit_buffer *ab, struct task_struct *tsk)
> { }
> +static inline int audit_log_container_info(struct task_struct *tsk,
> + struct audit_context
> *context); +{ }
> #define audit_enabled 0
> #endif /* CONFIG_AUDIT */
>
> diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> index 921a71f..e83ccbd 100644
> --- a/include/uapi/linux/audit.h
> +++ b/include/uapi/linux/audit.h
> @@ -115,6 +115,7 @@
> #define AUDIT_REPLACE 1329 /* Replace auditd
> if this packet unanswerd */ #define AUDIT_KERN_MODULE
> 1330 /* Kernel Module events */ #define
> AUDIT_FANOTIFY 1331 /* Fanotify access decision
> */ +#define AUDIT_CONTAINER_INFO 1332 /* Container ID
> information */ #define AUDIT_AVC 1400 /* SE
> Linux avc denial or grant */ #define AUDIT_SELINUX_ERR
> 1401 /* Internal SE Linux Errors */ diff --git
> a/kernel/audit.c b/kernel/audit.c index 3f2f143..a12f21f 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -2049,6 +2049,26 @@ void audit_log_session_info(struct
> audit_buffer *ab) audit_log_format(ab, " auid=%u ses=%u", auid,
> sessionid); }
>
> +/*
> + * audit_log_container_info - report container info
> + * @tsk: task to be recorded
> + * @context: task or local context for record
> + */
> +int audit_log_container_info(struct task_struct *tsk, struct
> audit_context *context) +{
> + struct audit_buffer *ab;
> +
> + if (!audit_containerid_set(tsk))
> + return 0;
> + /* Generate AUDIT_CONTAINER_INFO with container ID */
> + ab = audit_log_start(context, GFP_KERNEL,
> AUDIT_CONTAINER_INFO);
> + if (!ab)
> + return -ENOMEM;
> + audit_log_format(ab, "contid=%llu",
> audit_get_containerid(tsk));
> + audit_log_end(ab);
> + return 0;
> +}
> +
> void audit_log_key(struct audit_buffer *ab, char *key)
> {
> audit_log_format(ab, " key=");
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index a6b0a52..65be110 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -1453,6 +1453,8 @@ static void audit_log_exit(struct audit_context
> *context, struct task_struct *ts
> audit_log_proctitle(tsk, context);
>
> + audit_log_container_info(tsk, context);
Would there be any problem moving audit_log_container_info before
audit_log_proctitle? There are some assumptions that proctitle is the
last record in some situations.
Thanks,
-Steve
> /* Send end of event record to help user space know we are
> finished */ ab = audit_log_start(context, GFP_KERNEL, AUDIT_EOE);
> if (ab)
^ permalink raw reply
* Re: [PATCH net-next] vlan: Add extack messages for link create
From: David Miller @ 2018-05-17 21:09 UTC (permalink / raw)
To: dsahern; +Cc: netdev
In-Reply-To: <20180517192947.11269-1-dsahern@gmail.com>
From: David Ahern <dsahern@gmail.com>
Date: Thu, 17 May 2018 12:29:47 -0700
> Add informative messages for error paths related to adding a
> VLAN to a device.
>
> Signed-off-by: David Ahern <dsahern@gmail.com>
Applied, thanks David.
^ permalink raw reply
* Re: [PATCH net-next v3 0/3] net: Allow more drivers with COMPILE_TEST
From: David Miller @ 2018-05-17 21:14 UTC (permalink / raw)
To: f.fainelli; +Cc: netdev, fugang.duan, andrew, linux-kernel
In-Reply-To: <20180517200745.19142-1-f.fainelli@gmail.com>
From: Florian Fainelli <f.fainelli@gmail.com>
Date: Thu, 17 May 2018 13:07:42 -0700
> Hi David,
>
> This patch series includes more drivers to be build tested with COMPILE_TEST
> enabled. This helps cover some of the issues I just ran into with missing
> a driver *sigh*.
>
> Chanves in v3:
>
> - drop the TI Keystone NETCP driver from the COMPILE_TEST additions
>
> Changes in v2:
>
> - allow FEC to build outside of CONFIG_ARM/ARM64 by defining a layout of
> registers, this is not meant to run, so this is not a real issue if we
> are not matching the correct register layout
Ok, series applied.
Just some printf format string warnings to clear up on 64-bit in TI
driver files davinci_cpdma.c, cpsw.c, and cpts.c.
In file included from ./arch/x86/include/asm/bug.h:83:0,
from ./include/linux/bug.h:5,
from ./include/linux/thread_info.h:12,
from ./arch/x86/include/asm/preempt.h:7,
from ./include/linux/preempt.h:81,
from ./include/linux/spinlock.h:51,
from drivers/net/ethernet/ti/davinci_cpdma.c:16:
drivers/net/ethernet/ti/davinci_cpdma.c: In function ‘cpdma_desc_pool_destroy’:
drivers/net/ethernet/ti/davinci_cpdma.c:194:7: warning: format ‘%d’ expects argument of type ‘int’, but argument 2 has type ‘size_t {aka long unsigned int}’ [-Wformat=]
"cpdma_desc_pool size %d != avail %d",
^
gen_pool_size(pool->gen_pool),
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
./include/asm-generic/bug.h:98:50: note: in definition of macro ‘__WARN_printf’
#define __WARN_printf(arg...) do { __warn_printk(arg); __WARN(); } while (0)
^~~
drivers/net/ethernet/ti/davinci_cpdma.c:193:2: note: in expansion of macro ‘WARN’
WARN(gen_pool_size(pool->gen_pool) != gen_pool_avail(pool->gen_pool),
^~~~
drivers/net/ethernet/ti/davinci_cpdma.c:194:7: warning: format ‘%d’ expects argument of type ‘int’, but argument 3 has type ‘size_t {aka long unsigned int}’ [-Wformat=]
"cpdma_desc_pool size %d != avail %d",
^
drivers/net/ethernet/ti/davinci_cpdma.c:196:7:
gen_pool_avail(pool->gen_pool));
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
./include/asm-generic/bug.h:98:50: note: in definition of macro ‘__WARN_printf’
#define __WARN_printf(arg...) do { __warn_printk(arg); __WARN(); } while (0)
^~~
drivers/net/ethernet/ti/davinci_cpdma.c:193:2: note: in expansion of macro ‘WARN’
WARN(gen_pool_size(pool->gen_pool) != gen_pool_avail(pool->gen_pool),
^~~~
In file included from ./arch/x86/include/asm/realmode.h:15:0,
from ./arch/x86/include/asm/acpi.h:33,
from ./arch/x86/include/asm/fixmap.h:19,
from ./arch/x86/include/asm/apic.h:10,
from ./arch/x86/include/asm/smp.h:13,
from ./arch/x86/include/asm/mmzone_64.h:11,
from ./arch/x86/include/asm/mmzone.h:5,
from ./include/linux/mmzone.h:911,
from ./include/linux/gfp.h:6,
from ./include/linux/idr.h:16,
from ./include/linux/kernfs.h:14,
from ./include/linux/sysfs.h:16,
from ./include/linux/kobject.h:20,
from ./include/linux/device.h:16,
from drivers/net/ethernet/ti/davinci_cpdma.c:17:
drivers/net/ethernet/ti/davinci_cpdma.c: In function ‘cpdma_chan_submit’:
drivers/net/ethernet/ti/davinci_cpdma.c:1083:17: warning: passing argument 1 of ‘__writel’ makes integer from pointer without a cast [-Wint-conversion]
writel_relaxed(token, &desc->sw_token);
^
./arch/x86/include/asm/io.h:88:39: note: in definition of macro ‘writel_relaxed’
#define writel_relaxed(v, a) __writel(v, a)
^
./arch/x86/include/asm/io.h:71:18: note: expected ‘unsigned int’ but argument is of type ‘void *’
build_mmio_write(__writel, "l", unsigned int, "r", )
^
./arch/x86/include/asm/io.h:53:20: note: in definition of macro ‘build_mmio_write’
static inline void name(type val, volatile void __iomem *addr) \
^~~~
drivers/net/ethernet/ti/davinci_cpdma.c: In function ‘__cpdma_chan_free’:
drivers/net/ethernet/ti/davinci_cpdma.c:1126:15: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
token = (void *)desc_read(desc, sw_token);
^
In file included from ./include/linux/kernel.h:14:0,
from ./include/linux/uio.h:12,
from ./include/linux/socket.h:8,
from ./include/uapi/linux/if.h:25,
from drivers/net/ethernet/ti/cpts.c:21:
drivers/net/ethernet/ti/cpts.c: In function ‘cpts_overflow_check’:
drivers/net/ethernet/ti/cpts.c:297:11: warning: format ‘%lld’ expects argument of type ‘long long int’, but argument 3 has type ‘__kernel_time_t {aka long int}’ [-Wformat=]
pr_debug("cpts overflow check at %lld.%09lu\n", ts.tv_sec, ts.tv_nsec);
^
./include/linux/printk.h:288:21: note: in definition of macro ‘pr_fmt’
#define pr_fmt(fmt) fmt
^~~
./include/linux/printk.h:336:2: note: in expansion of macro ‘dynamic_pr_debug’
dynamic_pr_debug(fmt, ##__VA_ARGS__)
^~~~~~~~~~~~~~~~
drivers/net/ethernet/ti/cpts.c:297:2: note: in expansion of macro ‘pr_debug’
pr_debug("cpts overflow check at %lld.%09lu\n", ts.tv_sec, ts.tv_nsec);
^~~~~~~~
drivers/net/ethernet/ti/davinci_emac.c: In function ‘davinci_emac_probe’:
drivers/net/ethernet/ti/davinci_emac.c:1934:7: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
(void *)priv->emac_base_phys, ndev->irq);
^
drivers/net/ethernet/ti/cpsw.c: In function ‘cpsw_add_ch_strings’:
drivers/net/ethernet/ti/cpsw.c:1284:19: warning: format ‘%d’ expects argument of type ‘int’, but argument 5 has type ‘long unsigned int’ [-Wformat=]
"%s DMA chan %d: %s", rx_dir ? "Rx" : "Tx",
~^
%ld
^ permalink raw reply
* Re: [PATCH v3] mlx4_core: allocate ICM memory in page size chunks
From: Eric Dumazet @ 2018-05-17 21:14 UTC (permalink / raw)
To: Qing Huang, tariqt, davem, haakon.bugge, yanjun.zhu
Cc: netdev, linux-rdma, linux-kernel, gi-oh.kim
In-Reply-To: <20180517205343.8401-1-qing.huang@oracle.com>
On 05/17/2018 01:53 PM, Qing Huang wrote:
> When a system is under memory presure (high usage with fragments),
> the original 256KB ICM chunk allocations will likely trigger kernel
> memory management to enter slow path doing memory compact/migration
> ops in order to complete high order memory allocations.
>
> When that happens, user processes calling uverb APIs may get stuck
> for more than 120s easily even though there are a lot of free pages
> in smaller chunks available in the system.
>
> Syslog:
> ...
> Dec 10 09:04:51 slcc03db02 kernel: [397078.572732] INFO: task
> oracle_205573_e:205573 blocked for more than 120 seconds.
> ...
>
NACK on this patch.
You have been asked repeatedly to use kvmalloc()
This is not a minor suggestion.
Take a look at https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=d8c13f2271ec5178c52fbde072ec7b562651ed9d
And you'll understand some people care about this.
Strongly.
Thanks.
^ permalink raw reply
* [bpf-next PATCH v2 0/2] SK_MSG programs: read sock fields
From: John Fastabend @ 2018-05-17 21:16 UTC (permalink / raw)
To: ast, daniel; +Cc: netdev
In this series we add the ability for sk msg programs to read basic
sock information about the sock they are attached to. The second
patch adds the tests to the selftest test_verifier.
One obseration that I had from writing this seriess is lots of the
./net/core/filter.c code is almost duplicated across program types.
I thought about building a template/macro that we could use as a
single block of code to read sock data out for multiple programs,
but I wasn't convinced it was worth it yet. The result was using a
macro saved a couple lines of code per block but made the code
a bit harder to read IMO. We can probably revisit the idea later
if we get more duplication.
v2: add errstr field to negative test_verifier test cases to ensure
we get the expected err string back from the verifier.
---
John Fastabend (2):
bpf: allow sk_msg programs to read sock fields
bpf: add sk_msg prog sk access tests to test_verifier
include/linux/filter.h | 1
include/uapi/linux/bpf.h | 8 ++
kernel/bpf/sockmap.c | 1
net/core/filter.c | 114 ++++++++++++++++++++++++++-
tools/include/uapi/linux/bpf.h | 8 ++
tools/testing/selftests/bpf/test_verifier.c | 115 +++++++++++++++++++++++++++
6 files changed, 244 insertions(+), 3 deletions(-)
^ permalink raw reply
* [bpf-next PATCH v2 1/2] bpf: allow sk_msg programs to read sock fields
From: John Fastabend @ 2018-05-17 21:16 UTC (permalink / raw)
To: ast, daniel; +Cc: netdev
In-Reply-To: <20180517211452.14426.98480.stgit@john-Precision-Tower-5810>
Currently sk_msg programs only have access to the raw data. However,
it is often useful when building policies to have the policies specific
to the socket endpoint. This allows using the socket tuple as input
into filters, etc.
This patch adds ctx access to the sock fields.
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
Acked-by: Martin KaFai Lau <kafai@fb.com>
---
include/linux/filter.h | 1
include/uapi/linux/bpf.h | 8 +++
kernel/bpf/sockmap.c | 1
net/core/filter.c | 114 +++++++++++++++++++++++++++++++++++++++++++++-
4 files changed, 121 insertions(+), 3 deletions(-)
diff --git a/include/linux/filter.h b/include/linux/filter.h
index 9dbcb9d..d358d18 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -517,6 +517,7 @@ struct sk_msg_buff {
bool sg_copy[MAX_SKB_FRAGS];
__u32 flags;
struct sock *sk_redir;
+ struct sock *sk;
struct sk_buff *skb;
struct list_head list;
};
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index d94d333..97446bb 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -2176,6 +2176,14 @@ enum sk_action {
struct sk_msg_md {
void *data;
void *data_end;
+
+ __u32 family;
+ __u32 remote_ip4; /* Stored in network byte order */
+ __u32 local_ip4; /* Stored in network byte order */
+ __u32 remote_ip6[4]; /* Stored in network byte order */
+ __u32 local_ip6[4]; /* Stored in network byte order */
+ __u32 remote_port; /* Stored in network byte order */
+ __u32 local_port; /* stored in host byte order */
};
#define BPF_TAG_SIZE 8
diff --git a/kernel/bpf/sockmap.c b/kernel/bpf/sockmap.c
index c6de139..0ebf157 100644
--- a/kernel/bpf/sockmap.c
+++ b/kernel/bpf/sockmap.c
@@ -523,6 +523,7 @@ static unsigned int smap_do_tx_msg(struct sock *sk,
}
bpf_compute_data_pointers_sg(md);
+ md->sk = sk;
rc = (*prog->bpf_func)(md, prog->insnsi);
psock->apply_bytes = md->apply_bytes;
diff --git a/net/core/filter.c b/net/core/filter.c
index 6d0d156..aec5eba 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -5148,18 +5148,23 @@ static bool sk_msg_is_valid_access(int off, int size,
switch (off) {
case offsetof(struct sk_msg_md, data):
info->reg_type = PTR_TO_PACKET;
+ if (size != sizeof(__u64))
+ return false;
break;
case offsetof(struct sk_msg_md, data_end):
info->reg_type = PTR_TO_PACKET_END;
+ if (size != sizeof(__u64))
+ return false;
break;
+ default:
+ if (size != sizeof(__u32))
+ return false;
}
if (off < 0 || off >= sizeof(struct sk_msg_md))
return false;
if (off % size != 0)
return false;
- if (size != sizeof(__u64))
- return false;
return true;
}
@@ -5835,7 +5840,8 @@ static u32 sock_ops_convert_ctx_access(enum bpf_access_type type,
break;
case offsetof(struct bpf_sock_ops, local_ip4):
- BUILD_BUG_ON(FIELD_SIZEOF(struct sock_common, skc_rcv_saddr) != 4);
+ BUILD_BUG_ON(FIELD_SIZEOF(struct sock_common,
+ skc_rcv_saddr) != 4);
*insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF(
struct bpf_sock_ops_kern, sk),
@@ -6152,6 +6158,7 @@ static u32 sk_msg_convert_ctx_access(enum bpf_access_type type,
struct bpf_prog *prog, u32 *target_size)
{
struct bpf_insn *insn = insn_buf;
+ int off;
switch (si->off) {
case offsetof(struct sk_msg_md, data):
@@ -6164,6 +6171,107 @@ static u32 sk_msg_convert_ctx_access(enum bpf_access_type type,
si->dst_reg, si->src_reg,
offsetof(struct sk_msg_buff, data_end));
break;
+ case offsetof(struct sk_msg_md, family):
+ BUILD_BUG_ON(FIELD_SIZEOF(struct sock_common, skc_family) != 2);
+
+ *insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF(
+ struct sk_msg_buff, sk),
+ si->dst_reg, si->src_reg,
+ offsetof(struct sk_msg_buff, sk));
+ *insn++ = BPF_LDX_MEM(BPF_H, si->dst_reg, si->dst_reg,
+ offsetof(struct sock_common, skc_family));
+ break;
+
+ case offsetof(struct sk_msg_md, remote_ip4):
+ BUILD_BUG_ON(FIELD_SIZEOF(struct sock_common, skc_daddr) != 4);
+
+ *insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF(
+ struct sk_msg_buff, sk),
+ si->dst_reg, si->src_reg,
+ offsetof(struct sk_msg_buff, sk));
+ *insn++ = BPF_LDX_MEM(BPF_W, si->dst_reg, si->dst_reg,
+ offsetof(struct sock_common, skc_daddr));
+ break;
+
+ case offsetof(struct sk_msg_md, local_ip4):
+ BUILD_BUG_ON(FIELD_SIZEOF(struct sock_common,
+ skc_rcv_saddr) != 4);
+
+ *insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF(
+ struct sk_msg_buff, sk),
+ si->dst_reg, si->src_reg,
+ offsetof(struct sk_msg_buff, sk));
+ *insn++ = BPF_LDX_MEM(BPF_W, si->dst_reg, si->dst_reg,
+ offsetof(struct sock_common,
+ skc_rcv_saddr));
+ break;
+
+ case offsetof(struct sk_msg_md, remote_ip6[0]) ...
+ offsetof(struct sk_msg_md, remote_ip6[3]):
+#if IS_ENABLED(CONFIG_IPV6)
+ BUILD_BUG_ON(FIELD_SIZEOF(struct sock_common,
+ skc_v6_daddr.s6_addr32[0]) != 4);
+
+ off = si->off;
+ off -= offsetof(struct sk_msg_md, remote_ip6[0]);
+ *insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF(
+ struct sk_msg_buff, sk),
+ si->dst_reg, si->src_reg,
+ offsetof(struct sk_msg_buff, sk));
+ *insn++ = BPF_LDX_MEM(BPF_W, si->dst_reg, si->dst_reg,
+ offsetof(struct sock_common,
+ skc_v6_daddr.s6_addr32[0]) +
+ off);
+#else
+ *insn++ = BPF_MOV32_IMM(si->dst_reg, 0);
+#endif
+ break;
+
+ case offsetof(struct sk_msg_md, local_ip6[0]) ...
+ offsetof(struct sk_msg_md, local_ip6[3]):
+#if IS_ENABLED(CONFIG_IPV6)
+ BUILD_BUG_ON(FIELD_SIZEOF(struct sock_common,
+ skc_v6_rcv_saddr.s6_addr32[0]) != 4);
+
+ off = si->off;
+ off -= offsetof(struct sk_msg_md, local_ip6[0]);
+ *insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF(
+ struct sk_msg_buff, sk),
+ si->dst_reg, si->src_reg,
+ offsetof(struct sk_msg_buff, sk));
+ *insn++ = BPF_LDX_MEM(BPF_W, si->dst_reg, si->dst_reg,
+ offsetof(struct sock_common,
+ skc_v6_rcv_saddr.s6_addr32[0]) +
+ off);
+#else
+ *insn++ = BPF_MOV32_IMM(si->dst_reg, 0);
+#endif
+ break;
+
+ case offsetof(struct sk_msg_md, remote_port):
+ BUILD_BUG_ON(FIELD_SIZEOF(struct sock_common, skc_dport) != 2);
+
+ *insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF(
+ struct sk_msg_buff, sk),
+ si->dst_reg, si->src_reg,
+ offsetof(struct sk_msg_buff, sk));
+ *insn++ = BPF_LDX_MEM(BPF_H, si->dst_reg, si->dst_reg,
+ offsetof(struct sock_common, skc_dport));
+#ifndef __BIG_ENDIAN_BITFIELD
+ *insn++ = BPF_ALU32_IMM(BPF_LSH, si->dst_reg, 16);
+#endif
+ break;
+
+ case offsetof(struct sk_msg_md, local_port):
+ BUILD_BUG_ON(FIELD_SIZEOF(struct sock_common, skc_num) != 2);
+
+ *insn++ = BPF_LDX_MEM(BPF_FIELD_SIZEOF(
+ struct sk_msg_buff, sk),
+ si->dst_reg, si->src_reg,
+ offsetof(struct sk_msg_buff, sk));
+ *insn++ = BPF_LDX_MEM(BPF_H, si->dst_reg, si->dst_reg,
+ offsetof(struct sock_common, skc_num));
+ break;
}
return insn - insn_buf;
^ permalink raw reply related
* [bpf-next PATCH v2 2/2] bpf: add sk_msg prog sk access tests to test_verifier
From: John Fastabend @ 2018-05-17 21:17 UTC (permalink / raw)
To: ast, daniel; +Cc: netdev
In-Reply-To: <20180517211452.14426.98480.stgit@john-Precision-Tower-5810>
Add tests for BPF_PROG_TYPE_SK_MSG to test_verifier for read access
to new sk fields.
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
Acked-by: Martin KaFai Lau <kafai@fb.com>
---
tools/include/uapi/linux/bpf.h | 8 ++
tools/testing/selftests/bpf/test_verifier.c | 115 +++++++++++++++++++++++++++
2 files changed, 123 insertions(+)
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index d94d333..97446bb 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -2176,6 +2176,14 @@ enum sk_action {
struct sk_msg_md {
void *data;
void *data_end;
+
+ __u32 family;
+ __u32 remote_ip4; /* Stored in network byte order */
+ __u32 local_ip4; /* Stored in network byte order */
+ __u32 remote_ip6[4]; /* Stored in network byte order */
+ __u32 local_ip6[4]; /* Stored in network byte order */
+ __u32 remote_port; /* Stored in network byte order */
+ __u32 local_port; /* stored in host byte order */
};
#define BPF_TAG_SIZE 8
diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
index a877af0..6ec4d9d 100644
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -1686,6 +1686,121 @@ static void bpf_fill_rand_ld_dw(struct bpf_test *self)
.prog_type = BPF_PROG_TYPE_SK_SKB,
},
{
+ "valid access family in SK_MSG",
+ .insns = {
+ BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_1,
+ offsetof(struct sk_msg_md, family)),
+ BPF_EXIT_INSN(),
+ },
+ .result = ACCEPT,
+ .prog_type = BPF_PROG_TYPE_SK_MSG,
+ },
+ {
+ "valid access remote_ip4 in SK_MSG",
+ .insns = {
+ BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_1,
+ offsetof(struct sk_msg_md, remote_ip4)),
+ BPF_EXIT_INSN(),
+ },
+ .result = ACCEPT,
+ .prog_type = BPF_PROG_TYPE_SK_MSG,
+ },
+ {
+ "valid access local_ip4 in SK_MSG",
+ .insns = {
+ BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_1,
+ offsetof(struct sk_msg_md, local_ip4)),
+ BPF_EXIT_INSN(),
+ },
+ .result = ACCEPT,
+ .prog_type = BPF_PROG_TYPE_SK_MSG,
+ },
+ {
+ "valid access remote_port in SK_MSG",
+ .insns = {
+ BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_1,
+ offsetof(struct sk_msg_md, remote_port)),
+ BPF_EXIT_INSN(),
+ },
+ .result = ACCEPT,
+ .prog_type = BPF_PROG_TYPE_SK_MSG,
+ },
+ {
+ "valid access local_port in SK_MSG",
+ .insns = {
+ BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_1,
+ offsetof(struct sk_msg_md, local_port)),
+ BPF_EXIT_INSN(),
+ },
+ .result = ACCEPT,
+ .prog_type = BPF_PROG_TYPE_SK_MSG,
+ },
+ {
+ "valid access remote_ip6 in SK_MSG",
+ .insns = {
+ BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_1,
+ offsetof(struct sk_msg_md, remote_ip6[0])),
+ BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_1,
+ offsetof(struct sk_msg_md, remote_ip6[1])),
+ BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_1,
+ offsetof(struct sk_msg_md, remote_ip6[2])),
+ BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_1,
+ offsetof(struct sk_msg_md, remote_ip6[3])),
+ BPF_EXIT_INSN(),
+ },
+ .result = ACCEPT,
+ .prog_type = BPF_PROG_TYPE_SK_SKB,
+ },
+ {
+ "valid access local_ip6 in SK_MSG",
+ .insns = {
+ BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_1,
+ offsetof(struct sk_msg_md, local_ip6[0])),
+ BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_1,
+ offsetof(struct sk_msg_md, local_ip6[1])),
+ BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_1,
+ offsetof(struct sk_msg_md, local_ip6[2])),
+ BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_1,
+ offsetof(struct sk_msg_md, local_ip6[3])),
+ BPF_EXIT_INSN(),
+ },
+ .result = ACCEPT,
+ .prog_type = BPF_PROG_TYPE_SK_SKB,
+ },
+ {
+ "invalid 64B read of family in SK_MSG",
+ .insns = {
+ BPF_LDX_MEM(BPF_DW, BPF_REG_2, BPF_REG_1,
+ offsetof(struct sk_msg_md, family)),
+ BPF_EXIT_INSN(),
+ },
+ .errstr = "invalid bpf_context access",
+ .result = REJECT,
+ .prog_type = BPF_PROG_TYPE_SK_MSG,
+ },
+ {
+ "invalid read past end of SK_MSG",
+ .insns = {
+ BPF_LDX_MEM(BPF_W, BPF_REG_2, BPF_REG_1,
+ offsetof(struct sk_msg_md, local_port) + 4),
+ BPF_EXIT_INSN(),
+ },
+ .errstr = "R0 !read_ok",
+ .result = REJECT,
+ .prog_type = BPF_PROG_TYPE_SK_MSG,
+ },
+ {
+ "invalid read offset in SK_MSG",
+ .insns = {
+ BPF_LDX_MEM(BPF_W, BPF_REG_2, BPF_REG_1,
+ offsetof(struct sk_msg_md, family) + 1),
+ BPF_EXIT_INSN(),
+ },
+ .errstr = "invalid bpf_context access",
+ .result = REJECT,
+ .prog_type = BPF_PROG_TYPE_SK_MSG,
+ },
+ {
"direct packet read for SK_MSG",
.insns = {
BPF_LDX_MEM(BPF_DW, BPF_REG_2, BPF_REG_1,
^ permalink raw reply related
* Re: [PATCH net 0/7] net: ip6_gre: Fixes in headroom handling
From: David Miller @ 2018-05-17 21:18 UTC (permalink / raw)
To: petrm; +Cc: netdev, kuznet, yoshfuji, u9012063, xeb
In-Reply-To: <wihfu2q2dvl.fsf@dev-r-vrt-156.mtr.labs.mlnx>
From: Petr Machata <petrm@mellanox.com>
Date: Fri, 18 May 2018 00:03:58 +0300
> David Miller <davem@davemloft.net> writes:
>
>> Series applied, thank you.
>
> Hi David, I forgot to add Fixes lines to the individual patches. I
> replied to the e-mails with those. Let me know if you want me to send a
> v2 with that and the Acked-by's.
When something is already in my tree, it can't be changed as it is committed
to the permanent record of my GIT tree and I cannot rebase since so many
people clone my tree.
Luckily for you, your Fixes: tags went out before I pushed, so I could
actually fix up the commit messages and add the tags.
>> Those reproducable test cases in the various commit messages are
>> pretty awesome. Could you please extract them and put them somewhere
>> appropriate under selftests?
>
> The ip6gretap one is covered by the mirror_gre test if you run it
> with veth devices instead of HW ports, but I can make it self-contained
> if you think that would be better.
>
> I'll add the erspan one.
Thank you.
^ permalink raw reply
* Re: [PATCH bpf] bpf: fix truncated jump targets on heavy expansions
From: Martin KaFai Lau @ 2018-05-17 21:20 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: alexei.starovoitov, netdev
In-Reply-To: <20180516234411.18122-1-daniel@iogearbox.net>
On Thu, May 17, 2018 at 01:44:11AM +0200, Daniel Borkmann wrote:
> Recently during testing, I ran into the following panic:
>
> [ 207.892422] Internal error: Accessing user space memory outside uaccess.h routines: 96000004 [#1] SMP
> [ 207.901637] Modules linked in: binfmt_misc [...]
> [ 207.966530] CPU: 45 PID: 2256 Comm: test_verifier Tainted: G W 4.17.0-rc3+ #7
> [ 207.974956] Hardware name: FOXCONN R2-1221R-A4/C2U4N_MB, BIOS G31FB18A 03/31/2017
> [ 207.982428] pstate: 60400005 (nZCv daif +PAN -UAO)
> [ 207.987214] pc : bpf_skb_load_helper_8_no_cache+0x34/0xc0
> [ 207.992603] lr : 0xffff000000bdb754
> [ 207.996080] sp : ffff000013703ca0
> [ 207.999384] x29: ffff000013703ca0 x28: 0000000000000001
> [ 208.004688] x27: 0000000000000001 x26: 0000000000000000
> [ 208.009992] x25: ffff000013703ce0 x24: ffff800fb4afcb00
> [ 208.015295] x23: ffff00007d2f5038 x22: ffff00007d2f5000
> [ 208.020599] x21: fffffffffeff2a6f x20: 000000000000000a
> [ 208.025903] x19: ffff000009578000 x18: 0000000000000a03
> [ 208.031206] x17: 0000000000000000 x16: 0000000000000000
> [ 208.036510] x15: 0000ffff9de83000 x14: 0000000000000000
> [ 208.041813] x13: 0000000000000000 x12: 0000000000000000
> [ 208.047116] x11: 0000000000000001 x10: ffff0000089e7f18
> [ 208.052419] x9 : fffffffffeff2a6f x8 : 0000000000000000
> [ 208.057723] x7 : 000000000000000a x6 : 00280c6160000000
> [ 208.063026] x5 : 0000000000000018 x4 : 0000000000007db6
> [ 208.068329] x3 : 000000000008647a x2 : 19868179b1484500
> [ 208.073632] x1 : 0000000000000000 x0 : ffff000009578c08
> [ 208.078938] Process test_verifier (pid: 2256, stack limit = 0x0000000049ca7974)
> [ 208.086235] Call trace:
> [ 208.088672] bpf_skb_load_helper_8_no_cache+0x34/0xc0
> [ 208.093713] 0xffff000000bdb754
> [ 208.096845] bpf_test_run+0x78/0xf8
> [ 208.100324] bpf_prog_test_run_skb+0x148/0x230
> [ 208.104758] sys_bpf+0x314/0x1198
> [ 208.108064] el0_svc_naked+0x30/0x34
> [ 208.111632] Code: 91302260 f9400001 f9001fa1 d2800001 (29500680)
> [ 208.117717] ---[ end trace 263cb8a59b5bf29f ]---
>
> The program itself which caused this had a long jump over the whole
> instruction sequence where all of the inner instructions required
> heavy expansions into multiple BPF instructions. Additionally, I also
> had BPF hardening enabled which requires once more rewrites of all
> constant values in order to blind them. Each time we rewrite insns,
> bpf_adj_branches() would need to potentially adjust branch targets
> which cross the patchlet boundary to accommodate for the additional
> delta. Eventually that lead to the case where the target offset could
> not fit into insn->off's upper 0x7fff limit anymore where then offset
> wraps around becoming negative (in s16 universe), or vice versa
> depending on the jump direction.
>
> Therefore it becomes necessary to detect and reject any such occasions
> in a generic way for native eBPF and cBPF to eBPF migrations. For
> the latter we can simply check bounds in the bpf_convert_filter()'s
> BPF_EMIT_JMP helper macro and bail out once we surpass limits. The
> bpf_patch_insn_single() for native eBPF (and cBPF to eBPF in case
> of subsequent hardening) is a bit more complex in that we need to
> detect such truncations before hitting the bpf_prog_realloc(). Thus
> the latter is split into an extra pass to probe problematic offsets
> on the original program in order to fail early. With that in place
> and carefully tested I no longer hit the panic and the rewrites are
> rejected properly. The above example panic I've seen on bpf-next,
> though the issue itself is generic in that a guard against this issue
> in bpf seems more appropriate in this case.
>
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Martin KaFai Lau <kafai@fb.com>
^ permalink raw reply
* Re: [RFC PATCH bpf-next 12/12] i40e: implement Tx zero-copy
From: Jesper Dangaard Brouer @ 2018-05-17 21:31 UTC (permalink / raw)
To: Björn Töpel
Cc: magnus.karlsson, magnus.karlsson, alexander.h.duyck,
alexander.duyck, john.fastabend, ast, willemdebruijn.kernel,
daniel, mst, netdev, michael.lundkvist, jesse.brandeburg,
anjali.singhai, qi.z.zhang, intel-wired-lan, brouer
In-Reply-To: <20180515190615.23099-13-bjorn.topel@gmail.com>
On Tue, 15 May 2018 21:06:15 +0200 Björn Töpel <bjorn.topel@gmail.com> wrote:
> From: Magnus Karlsson <magnus.karlsson@intel.com>
>
> Here, the zero-copy ndo is implemented. As a shortcut, the existing
> XDP Tx rings are used for zero-copy. This means that and XDP program
> cannot redirect to an AF_XDP enabled XDP Tx ring.
This "shortcut" is not acceptable, and completely broken. The
XDP_REDIRECT queue_index is based on smp_processor_id(), and can easily
clash with the configured XSK queue_index. Provided a bit more code
context below...
On Tue, 15 May 2018 21:06:15 +0200
Björn Töpel <bjorn.topel@gmail.com> wrote:
int i40e_xdp_xmit(struct net_device *dev, struct xdp_frame *xdpf)
{
struct i40e_netdev_priv *np = netdev_priv(dev);
unsigned int queue_index = smp_processor_id();
struct i40e_vsi *vsi = np->vsi;
int err;
if (test_bit(__I40E_VSI_DOWN, vsi->state))
return -ENETDOWN;
> @@ -4025,6 +4158,9 @@ int i40e_xdp_xmit(struct net_device *dev, struct xdp_frame *xdpf)
> if (!i40e_enabled_xdp_vsi(vsi) || queue_index >= vsi->num_queue_pairs)
> return -ENXIO;
>
> + if (vsi->xdp_rings[queue_index]->xsk_umem)
> + return -ENXIO;
> +
Using the sane errno makes this impossible to debug (via the tracepoints).
> err = i40e_xmit_xdp_ring(xdpf, vsi->xdp_rings[queue_index]);
> if (err != I40E_XDP_TX)
> return -ENOSPC;
> @@ -4048,5 +4184,34 @@ void i40e_xdp_flush(struct net_device *dev)
> if (!i40e_enabled_xdp_vsi(vsi) || queue_index >= vsi->num_queue_pairs)
> return;
>
> + if (vsi->xdp_rings[queue_index]->xsk_umem)
> + return;
> +
> i40e_xdp_ring_update_tail(vsi->xdp_rings[queue_index]);
> }
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: [RFC PATCH ghak32 V2 03/13] audit: log container info of syscalls
From: Richard Guy Briggs @ 2018-05-17 21:41 UTC (permalink / raw)
To: Steve Grubb
Cc: cgroups, containers, linux-api, Linux-Audit Mailing List,
linux-fsdevel, LKML, netdev, ebiederm, luto, jlayton, carlos,
dhowells, viro, simo, eparis, serge
In-Reply-To: <20180517170907.3d9f7c1a@ivy-bridge>
On 2018-05-17 17:09, Steve Grubb wrote:
> On Fri, 16 Mar 2018 05:00:30 -0400
> Richard Guy Briggs <rgb@redhat.com> wrote:
>
> > Create a new audit record AUDIT_CONTAINER_INFO to document the
> > container ID of a process if it is present.
>
> As mentioned in a previous email, I think AUDIT_CONTAINER is more
> suitable for the container record. One more comment below...
>
> > Called from audit_log_exit(), syscalls are covered.
> >
> > A sample raw event:
> > type=SYSCALL msg=audit(1519924845.499:257): arch=c000003e syscall=257
> > success=yes exit=3 a0=ffffff9c a1=56374e1cef30 a2=241 a3=1b6 items=2
> > ppid=606 pid=635 auid=0 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0
> > sgid=0 fsgid=0 tty=pts0 ses=3 comm="bash" exe="/usr/bin/bash"
> > subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
> > key="tmpcontainerid" type=CWD msg=audit(1519924845.499:257):
> > cwd="/root" type=PATH msg=audit(1519924845.499:257): item=0
> > name="/tmp/" inode=13863 dev=00:27 mode=041777 ouid=0 ogid=0
> > rdev=00:00 obj=system_u:object_r:tmp_t:s0 nametype= PARENT
> > cap_fp=0000000000000000 cap_fi=0000000000000000 cap_fe=0 cap_fver=0
> > type=PATH msg=audit(1519924845.499:257): item=1
> > name="/tmp/tmpcontainerid" inode=17729 dev=00:27 mode=0100644 ouid=0
> > ogid=0 rdev=00:00 obj=unconfined_u:object_r:user_tmp_t:s0
> > nametype=CREATE cap_fp=0000000000000000 cap_fi=0000000000000000
> > cap_fe=0 cap_fver=0 type=PROCTITLE msg=audit(1519924845.499:257):
> > proctitle=62617368002D6300736C65657020313B206563686F2074657374203E202F746D702F746D70636F6E7461696E65726964
> > type=CONTAINER_INFO msg=audit(1519924845.499:257): op=task
> > contid=123458
> >
> > See: https://github.com/linux-audit/audit-kernel/issues/32
> > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > ---
> > include/linux/audit.h | 5 +++++
> > include/uapi/linux/audit.h | 1 +
> > kernel/audit.c | 20 ++++++++++++++++++++
> > kernel/auditsc.c | 2 ++
> > 4 files changed, 28 insertions(+)
> >
> > diff --git a/include/linux/audit.h b/include/linux/audit.h
> > index fe4ba3f..3acbe9d 100644
> > --- a/include/linux/audit.h
> > +++ b/include/linux/audit.h
> > @@ -154,6 +154,8 @@ extern void
> > audit_log_link_denied(const char *operation, extern int
> > audit_log_task_context(struct audit_buffer *ab); extern void
> > audit_log_task_info(struct audit_buffer *ab, struct task_struct *tsk);
> > +extern int audit_log_container_info(struct task_struct *tsk,
> > + struct audit_context *context);
> >
> > extern int audit_update_lsm_rules(void);
> >
> > @@ -205,6 +207,9 @@ static inline int audit_log_task_context(struct
> > audit_buffer *ab) static inline void audit_log_task_info(struct
> > audit_buffer *ab, struct task_struct *tsk)
> > { }
> > +static inline int audit_log_container_info(struct task_struct *tsk,
> > + struct audit_context
> > *context); +{ }
> > #define audit_enabled 0
> > #endif /* CONFIG_AUDIT */
> >
> > diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> > index 921a71f..e83ccbd 100644
> > --- a/include/uapi/linux/audit.h
> > +++ b/include/uapi/linux/audit.h
> > @@ -115,6 +115,7 @@
> > #define AUDIT_REPLACE 1329 /* Replace auditd
> > if this packet unanswerd */ #define AUDIT_KERN_MODULE
> > 1330 /* Kernel Module events */ #define
> > AUDIT_FANOTIFY 1331 /* Fanotify access decision
> > */ +#define AUDIT_CONTAINER_INFO 1332 /* Container ID
> > information */ #define AUDIT_AVC 1400 /* SE
> > Linux avc denial or grant */ #define AUDIT_SELINUX_ERR
> > 1401 /* Internal SE Linux Errors */ diff --git
> > a/kernel/audit.c b/kernel/audit.c index 3f2f143..a12f21f 100644
> > --- a/kernel/audit.c
> > +++ b/kernel/audit.c
> > @@ -2049,6 +2049,26 @@ void audit_log_session_info(struct
> > audit_buffer *ab) audit_log_format(ab, " auid=%u ses=%u", auid,
> > sessionid); }
> >
> > +/*
> > + * audit_log_container_info - report container info
> > + * @tsk: task to be recorded
> > + * @context: task or local context for record
> > + */
> > +int audit_log_container_info(struct task_struct *tsk, struct
> > audit_context *context) +{
> > + struct audit_buffer *ab;
> > +
> > + if (!audit_containerid_set(tsk))
> > + return 0;
> > + /* Generate AUDIT_CONTAINER_INFO with container ID */
> > + ab = audit_log_start(context, GFP_KERNEL,
> > AUDIT_CONTAINER_INFO);
> > + if (!ab)
> > + return -ENOMEM;
> > + audit_log_format(ab, "contid=%llu",
> > audit_get_containerid(tsk));
> > + audit_log_end(ab);
> > + return 0;
> > +}
> > +
> > void audit_log_key(struct audit_buffer *ab, char *key)
> > {
> > audit_log_format(ab, " key=");
> > diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> > index a6b0a52..65be110 100644
> > --- a/kernel/auditsc.c
> > +++ b/kernel/auditsc.c
> > @@ -1453,6 +1453,8 @@ static void audit_log_exit(struct audit_context
> > *context, struct task_struct *ts
> > audit_log_proctitle(tsk, context);
> >
> > + audit_log_container_info(tsk, context);
>
> Would there be any problem moving audit_log_container_info before
> audit_log_proctitle? There are some assumptions that proctitle is the
> last record in some situations.
I see no problem doing that.
> Thanks,
> -Steve
>
> > /* Send end of event record to help user space know we are
> > finished */ ab = audit_log_start(context, GFP_KERNEL, AUDIT_EOE);
> > if (ab)
- RGB
--
Richard Guy Briggs <rgb@redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635
^ permalink raw reply
* Re: [PATCH v3] mlx4_core: allocate ICM memory in page size chunks
From: Qing Huang @ 2018-05-17 21:45 UTC (permalink / raw)
To: Eric Dumazet, tariqt, davem, haakon.bugge, yanjun.zhu
Cc: netdev, linux-rdma, linux-kernel, gi-oh.kim
In-Reply-To: <e6c470f0-f2ae-5efd-09ed-0c9562a4a764@gmail.com>
On 5/17/2018 2:14 PM, Eric Dumazet wrote:
> On 05/17/2018 01:53 PM, Qing Huang wrote:
>> When a system is under memory presure (high usage with fragments),
>> the original 256KB ICM chunk allocations will likely trigger kernel
>> memory management to enter slow path doing memory compact/migration
>> ops in order to complete high order memory allocations.
>>
>> When that happens, user processes calling uverb APIs may get stuck
>> for more than 120s easily even though there are a lot of free pages
>> in smaller chunks available in the system.
>>
>> Syslog:
>> ...
>> Dec 10 09:04:51 slcc03db02 kernel: [397078.572732] INFO: task
>> oracle_205573_e:205573 blocked for more than 120 seconds.
>> ...
>>
> NACK on this patch.
>
> You have been asked repeatedly to use kvmalloc()
>
> This is not a minor suggestion.
>
> Take a look athttps://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=d8c13f2271ec5178c52fbde072ec7b562651ed9d
Would you please take a look at how table->icm is being used in the mlx4
driver? It's a meta data used for individual pointer variable referencing,
not as data frag or in/out buffer. It has no need for contiguous phy.
memory.
Thanks.
> And you'll understand some people care about this.
>
> Strongly.
>
> Thanks.
>
^ permalink raw reply
* Re: [PATCH v3 1/2] media: rc: introduce BPF_PROG_RAWIR_EVENT
From: Sean Young @ 2018-05-17 21:45 UTC (permalink / raw)
To: Y Song
Cc: linux-media, linux-kernel, Alexei Starovoitov,
Mauro Carvalho Chehab, Daniel Borkmann, netdev, Matthias Reichl,
Devin Heitmueller
In-Reply-To: <CAH3MdRUqpenhK0hCzMfRyJU5Dz=k83Vxw0HFtyL1qa7aO5vDBA@mail.gmail.com>
Hi,
Again thanks for a thoughtful review. This will definitely will improve
the code.
On Thu, May 17, 2018 at 10:02:52AM -0700, Y Song wrote:
> On Wed, May 16, 2018 at 2:04 PM, Sean Young <sean@mess.org> wrote:
> > Add support for BPF_PROG_RAWIR_EVENT. This type of BPF program can call
> > rc_keydown() to reported decoded IR scancodes, or rc_repeat() to report
> > that the last key should be repeated.
> >
> > The bpf program can be attached to using the bpf(BPF_PROG_ATTACH) syscall;
> > the target_fd must be the /dev/lircN device.
> >
> > Signed-off-by: Sean Young <sean@mess.org>
> > ---
> > drivers/media/rc/Kconfig | 13 ++
> > drivers/media/rc/Makefile | 1 +
> > drivers/media/rc/bpf-rawir-event.c | 363 +++++++++++++++++++++++++++++
> > drivers/media/rc/lirc_dev.c | 24 ++
> > drivers/media/rc/rc-core-priv.h | 24 ++
> > drivers/media/rc/rc-ir-raw.c | 14 +-
> > include/linux/bpf_rcdev.h | 30 +++
> > include/linux/bpf_types.h | 3 +
> > include/uapi/linux/bpf.h | 55 ++++-
> > kernel/bpf/syscall.c | 7 +
> > 10 files changed, 531 insertions(+), 3 deletions(-)
> > create mode 100644 drivers/media/rc/bpf-rawir-event.c
> > create mode 100644 include/linux/bpf_rcdev.h
> >
> > diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig
> > index eb2c3b6eca7f..2172d65b0213 100644
> > --- a/drivers/media/rc/Kconfig
> > +++ b/drivers/media/rc/Kconfig
> > @@ -25,6 +25,19 @@ config LIRC
> > passes raw IR to and from userspace, which is needed for
> > IR transmitting (aka "blasting") and for the lirc daemon.
> >
> > +config BPF_RAWIR_EVENT
> > + bool "Support for eBPF programs attached to lirc devices"
> > + depends on BPF_SYSCALL
> > + depends on RC_CORE=y
> > + depends on LIRC
> > + help
> > + Allow attaching eBPF programs to a lirc device using the bpf(2)
> > + syscall command BPF_PROG_ATTACH. This is supported for raw IR
> > + receivers.
> > +
> > + These eBPF programs can be used to decode IR into scancodes, for
> > + IR protocols not supported by the kernel decoders.
> > +
> > menuconfig RC_DECODERS
> > bool "Remote controller decoders"
> > depends on RC_CORE
> > diff --git a/drivers/media/rc/Makefile b/drivers/media/rc/Makefile
> > index 2e1c87066f6c..74907823bef8 100644
> > --- a/drivers/media/rc/Makefile
> > +++ b/drivers/media/rc/Makefile
> > @@ -5,6 +5,7 @@ obj-y += keymaps/
> > obj-$(CONFIG_RC_CORE) += rc-core.o
> > rc-core-y := rc-main.o rc-ir-raw.o
> > rc-core-$(CONFIG_LIRC) += lirc_dev.o
> > +rc-core-$(CONFIG_BPF_RAWIR_EVENT) += bpf-rawir-event.o
> > obj-$(CONFIG_IR_NEC_DECODER) += ir-nec-decoder.o
> > obj-$(CONFIG_IR_RC5_DECODER) += ir-rc5-decoder.o
> > obj-$(CONFIG_IR_RC6_DECODER) += ir-rc6-decoder.o
> > diff --git a/drivers/media/rc/bpf-rawir-event.c b/drivers/media/rc/bpf-rawir-event.c
> > new file mode 100644
> > index 000000000000..7cb48b8d87b5
> > --- /dev/null
> > +++ b/drivers/media/rc/bpf-rawir-event.c
> > @@ -0,0 +1,363 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +// bpf-rawir-event.c - handles bpf
> > +//
> > +// Copyright (C) 2018 Sean Young <sean@mess.org>
> > +
> > +#include <linux/bpf.h>
> > +#include <linux/filter.h>
> > +#include <linux/bpf_rcdev.h>
> > +#include "rc-core-priv.h"
> > +
> > +/*
> > + * BPF interface for raw IR
> > + */
> > +const struct bpf_prog_ops rawir_event_prog_ops = {
> > +};
> > +
> > +BPF_CALL_1(bpf_rc_repeat, struct bpf_rawir_event*, event)
> > +{
> > + struct ir_raw_event_ctrl *ctrl;
> > +
> > + ctrl = container_of(event, struct ir_raw_event_ctrl, bpf_rawir_event);
> > +
> > + rc_repeat(ctrl->dev);
> > +
> > + return 0;
> > +}
> > +
> > +static const struct bpf_func_proto rc_repeat_proto = {
> > + .func = bpf_rc_repeat,
> > + .gpl_only = true, /* rc_repeat is EXPORT_SYMBOL_GPL */
> > + .ret_type = RET_INTEGER,
> > + .arg1_type = ARG_PTR_TO_CTX,
> > +};
> > +
> > +BPF_CALL_4(bpf_rc_keydown, struct bpf_rawir_event*, event, u32, protocol,
> > + u32, scancode, u32, toggle)
> > +{
> > + struct ir_raw_event_ctrl *ctrl;
> > +
> > + ctrl = container_of(event, struct ir_raw_event_ctrl, bpf_rawir_event);
> > +
> > + rc_keydown(ctrl->dev, protocol, scancode, toggle != 0);
> > +
> > + return 0;
> > +}
> > +
> > +static const struct bpf_func_proto rc_keydown_proto = {
> > + .func = bpf_rc_keydown,
> > + .gpl_only = true, /* rc_keydown is EXPORT_SYMBOL_GPL */
> > + .ret_type = RET_INTEGER,
> > + .arg1_type = ARG_PTR_TO_CTX,
> > + .arg2_type = ARG_ANYTHING,
> > + .arg3_type = ARG_ANYTHING,
> > + .arg4_type = ARG_ANYTHING,
> > +};
> > +
> > +static const struct bpf_func_proto *
> > +rawir_event_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> > +{
> > + switch (func_id) {
> > + case BPF_FUNC_rc_repeat:
> > + return &rc_repeat_proto;
> > + case BPF_FUNC_rc_keydown:
> > + return &rc_keydown_proto;
> > + case BPF_FUNC_map_lookup_elem:
> > + return &bpf_map_lookup_elem_proto;
> > + case BPF_FUNC_map_update_elem:
> > + return &bpf_map_update_elem_proto;
> > + case BPF_FUNC_map_delete_elem:
> > + return &bpf_map_delete_elem_proto;
> > + case BPF_FUNC_ktime_get_ns:
> > + return &bpf_ktime_get_ns_proto;
> > + case BPF_FUNC_tail_call:
> > + return &bpf_tail_call_proto;
> > + case BPF_FUNC_get_prandom_u32:
> > + return &bpf_get_prandom_u32_proto;
> > + case BPF_FUNC_trace_printk:
> > + if (capable(CAP_SYS_ADMIN))
> > + return bpf_get_trace_printk_proto();
> > + /* fall through */
> > + default:
> > + return NULL;
> > + }
> > +}
> > +
> > +static bool rawir_event_is_valid_access(int off, int size,
> > + enum bpf_access_type type,
> > + const struct bpf_prog *prog,
> > + struct bpf_insn_access_aux *info)
> > +{
> > + /* struct bpf_rawir_event has two u32 fields */
> > + if (type == BPF_WRITE)
> > + return false;
> > +
> > + if (size != sizeof(__u32))
> > + return false;
> > +
> > + if (!(off == offsetof(struct bpf_rawir_event, duration) ||
> > + off == offsetof(struct bpf_rawir_event, type)))
> > + return false;
> > +
> > + return true;
> > +}
> > +
> > +const struct bpf_verifier_ops rawir_event_verifier_ops = {
> > + .get_func_proto = rawir_event_func_proto,
> > + .is_valid_access = rawir_event_is_valid_access
> > +};
> > +
> > +#define BPF_MAX_PROGS 64
> > +
> > +static int rc_dev_bpf_attach(struct rc_dev *rcdev, struct bpf_prog *prog)
> > +{
> > + struct ir_raw_event_ctrl *raw;
> > + struct bpf_prog_list *pl;
> > + int ret, size;
> > +
> > + if (rcdev->driver_type != RC_DRIVER_IR_RAW)
> > + return -EINVAL;
> > +
> > + ret = mutex_lock_interruptible(&ir_raw_handler_lock);
> > + if (ret)
> > + return ret;
> > +
> > + raw = rcdev->raw;
> > + if (!raw) {
> > + ret = -ENODEV;
> > + goto out;
> > + }
> > +
> > + size = 0;
> > + list_for_each_entry(pl, &raw->progs, node) {
> > + if (pl->prog == prog) {
> > + ret = -EEXIST;
> > + goto out;
> > + }
> > +
> > + size++;
> > + }
> > +
> > + if (size >= BPF_MAX_PROGS) {
> > + ret = -E2BIG;
> > + goto out;
> > + }
> > +
> > + pl = kmalloc(sizeof(*pl), GFP_KERNEL);
> > + if (!pl) {
> > + ret = -ENOMEM;
> > + goto out;
> > + }
> > +
> > + pl->prog = prog;
> > + list_add(&pl->node, &raw->progs);
> > +out:
> > + mutex_unlock(&ir_raw_handler_lock);
> > + return ret;
> > +}
> > +
> > +static int rc_dev_bpf_detach(struct rc_dev *rcdev, struct bpf_prog *prog)
> > +{
> > + struct ir_raw_event_ctrl *raw;
> > + struct bpf_prog_list *pl, *tmp;
> > + int ret;
> > +
> > + if (rcdev->driver_type != RC_DRIVER_IR_RAW)
> > + return -EINVAL;
> > +
> > + ret = mutex_lock_interruptible(&ir_raw_handler_lock);
> > + if (ret)
> > + return ret;
> > +
> > + raw = rcdev->raw;
> > + if (!raw) {
> > + ret = -ENODEV;
> > + goto out;
> > + }
> > +
> > + ret = -ENOENT;
> > +
> > + list_for_each_entry_safe(pl, tmp, &raw->progs, node) {
> > + if (pl->prog == prog) {
> > + list_del(&pl->node);
> > + kfree(pl);
> > + bpf_prog_put(prog);
> > + ret = 0;
> > + goto out;
> > + }
> > + }
> > +out:
> > + mutex_unlock(&ir_raw_handler_lock);
> > + return ret;
> > +}
> > +
> > +void rc_dev_bpf_init(struct rc_dev *rcdev)
> > +{
> > + INIT_LIST_HEAD(&rcdev->raw->progs);
> > +}
> > +
> > +void rc_dev_bpf_run(struct rc_dev *rcdev, struct ir_raw_event ev)
> > +{
> > + struct ir_raw_event_ctrl *raw = rcdev->raw;
> > + struct bpf_prog_list *pl;
> > +
> > + if (list_empty(&raw->progs))
> > + return;
> > +
> > + if (unlikely(ev.carrier_report)) {
> > + raw->bpf_rawir_event.carrier = ev.carrier;
> > + raw->bpf_rawir_event.type = BPF_RAWIR_EVENT_CARRIER;
> > + } else {
> > + raw->bpf_rawir_event.duration = ev.duration;
> > +
> > + if (ev.pulse)
> > + raw->bpf_rawir_event.type = BPF_RAWIR_EVENT_PULSE;
> > + else if (ev.timeout)
> > + raw->bpf_rawir_event.type = BPF_RAWIR_EVENT_TIMEOUT;
> > + else if (ev.reset)
> > + raw->bpf_rawir_event.type = BPF_RAWIR_EVENT_RESET;
> > + else
> > + raw->bpf_rawir_event.type = BPF_RAWIR_EVENT_SPACE;
> > + }
> > +
> > + list_for_each_entry(pl, &raw->progs, node)
> > + BPF_PROG_RUN(pl->prog, &raw->bpf_rawir_event);
>
> Is the raw->progs protected by locks? It is possible that attaching/detaching
> could manipulate raw->progs at the same time? In perf/cgroup prog array
> case, the prog array run is protected by rcu and the dummy prog idea is
> to avoid the prog is skipped by reshuffling.
Yes, it is. The caller takes the appropriate lock. These functions could do
with some comments.
> Also, the original idea about using prog array is the least overhead since you
> want to BPF programs to execute as soon as possible.
Now, for our purposes the we're not bothered by a few milliseconds delay,
so I would pick whatever uses less cpu/memory overall. There is some overhead
in using rcu but the array is nice since it uses less memory than the
double-linked list, and less cache misses.
So I wanted bpf_prog_detach() to return -ENOENT if the prog was not in the list,
however bpf_prog_array_copy() and bpf_prog_array_delete_safe() do not return
anything useful for that.
Maybe I should look at adding a count-delta return to bpf_prog_array_copy(),
and a bool return to bpf_prog_array_delete_safe(). Or I could scan the array
an extra time to see if it is present. Any other ideas?
Also, BPF_F_OVERRIDE is not relevant for this but possibly BPF_F_ALLOW_MULTI
could invoke the same behaviour as for cgroups. What do you think?
> > +}
> > +
> > +void rc_dev_bpf_free(struct rc_dev *rcdev)
> > +{
> > + struct bpf_prog_list *pl, *tmp;
> > +
> > + list_for_each_entry_safe(pl, tmp, &rcdev->raw->progs, node) {
> > + list_del(&pl->node);
> > + bpf_prog_put(pl->prog);
> > + kfree(pl);
> > + }
> > +}
> > +
> > +int rc_dev_prog_attach(const union bpf_attr *attr)
> > +{
> > + struct bpf_prog *prog;
> > + struct rc_dev *rcdev;
> > + int ret;
> > +
> > + if (attr->attach_flags)
> > + return -EINVAL;
> > +
> > + prog = bpf_prog_get_type(attr->attach_bpf_fd,
> > + BPF_PROG_TYPE_RAWIR_EVENT);
> > + if (IS_ERR(prog))
> > + return PTR_ERR(prog);
> > +
> > + rcdev = rc_dev_get_from_fd(attr->target_fd);
> > + if (IS_ERR(rcdev)) {
> > + bpf_prog_put(prog);
> > + return PTR_ERR(rcdev);
> > + }
> > +
> > + ret = rc_dev_bpf_attach(rcdev, prog);
> > + if (ret)
> > + bpf_prog_put(prog);
> > +
> > + put_device(&rcdev->dev);
> > +
> > + return ret;
> > +}
> > +
> > +int rc_dev_prog_detach(const union bpf_attr *attr)
> > +{
> > + struct bpf_prog *prog;
> > + struct rc_dev *rcdev;
> > + int ret;
> > +
> > + if (attr->attach_flags)
> > + return -EINVAL;
> > +
> > + prog = bpf_prog_get_type(attr->attach_bpf_fd,
> > + BPF_PROG_TYPE_RAWIR_EVENT);
> > + if (IS_ERR(prog))
> > + return PTR_ERR(prog);
> > +
> > + rcdev = rc_dev_get_from_fd(attr->target_fd);
> > + if (IS_ERR(rcdev)) {
> > + bpf_prog_put(prog);
> > + return PTR_ERR(rcdev);
> > + }
> > +
> > + ret = rc_dev_bpf_detach(rcdev, prog);
> > +
> > + bpf_prog_put(prog);
> > + put_device(&rcdev->dev);
> > +
> > + return ret;
> > +}
> > +
> > +int rc_dev_prog_query(const union bpf_attr *attr, union bpf_attr __user *uattr)
> > +{
> > + __u32 __user *prog_ids = u64_to_user_ptr(attr->query.prog_ids);
> > + struct ir_raw_event_ctrl *raw;
> > + struct bpf_prog_list *pl;
> > + struct rc_dev *rcdev;
> > + u32 cnt, flags = 0, *ids, i;
> > + int ret;
> > +
> > + if (attr->query.query_flags)
> > + return -EINVAL;
> > +
> > + rcdev = rc_dev_get_from_fd(attr->query.target_fd);
> > + if (IS_ERR(rcdev))
> > + return PTR_ERR(rcdev);
> > +
> > + if (rcdev->driver_type != RC_DRIVER_IR_RAW) {
> > + ret = -EINVAL;
> > + goto out;
>
> mutex_lock_interruptible() has not been called. You can just return here.
Yep.
> > + }
> > +
> > + ret = mutex_lock_interruptible(&ir_raw_handler_lock);
> > + if (ret)
> > + goto out;
>
> Maybe you can rename label "out" to "unlock" since it
> is really an unlock and out?
Good point.
> > +
> > + raw = rcdev->raw;
> > + if (!raw) {
> > + ret = -ENODEV;
> > + goto out;
> > + }
> > +
> > + cnt = 0;
> > + list_for_each_entry(pl, &raw->progs, node)
> > + cnt++;
> > +
> > + if (copy_to_user(&uattr->query.prog_cnt, &cnt, sizeof(cnt))) {
> > + ret = -EFAULT;
> > + goto out;
> > + }
> > + if (copy_to_user(&uattr->query.attach_flags, &flags, sizeof(flags))) {
> > + ret = -EFAULT;
> > + goto out;
> > + }
> > +
> > + if (attr->query.prog_cnt != 0 && prog_ids && cnt) {
> > + if (attr->query.prog_cnt < cnt)
> > + cnt = attr->query.prog_cnt;
> > +
> > + ids = kmalloc_array(cnt, sizeof(u32), GFP_KERNEL);
> > + if (!ids) {
> > + ret = -ENOMEM;
> > + goto out;
> > + }
> > +
> > + i = 0;
> > + list_for_each_entry(pl, &raw->progs, node) {
> > + ids[i++] = pl->prog->aux->id;
> > + if (i == cnt)
> > + break;
> > + }
> > +
> > + ret = copy_to_user(prog_ids, ids, cnt * sizeof(u32));
>
> Do you want to give user a chance to know that the "cnt" is not big enough
> by return -ENOSPC if cnt is smaller than the number of progs in the array?
True.
> > + }
> > +out:
> > + mutex_unlock(&ir_raw_handler_lock);
> > + put_device(&rcdev->dev);
> > +
> > + return ret;
> > +}
> > diff --git a/drivers/media/rc/lirc_dev.c b/drivers/media/rc/lirc_dev.c
> > index 24e9fbb80e81..193540ded626 100644
> > --- a/drivers/media/rc/lirc_dev.c
> > +++ b/drivers/media/rc/lirc_dev.c
> > @@ -20,6 +20,7 @@
> > #include <linux/module.h>
> > #include <linux/mutex.h>
> > #include <linux/device.h>
> > +#include <linux/file.h>
> > #include <linux/idr.h>
> > #include <linux/poll.h>
> > #include <linux/sched.h>
> > @@ -816,4 +817,27 @@ void __exit lirc_dev_exit(void)
> > unregister_chrdev_region(lirc_base_dev, RC_DEV_MAX);
> > }
> >
> > +struct rc_dev *rc_dev_get_from_fd(int fd)
> > +{
> > + struct fd f = fdget(fd);
> > + struct lirc_fh *fh;
> > + struct rc_dev *dev;
> > +
> > + if (!f.file)
> > + return ERR_PTR(-EBADF);
> > +
> > + if (f.file->f_op != &lirc_fops) {
> > + fdput(f);
> > + return ERR_PTR(-EINVAL);
> > + }
> > +
> > + fh = f.file->private_data;
> > + dev = fh->rc;
> > +
> > + get_device(&dev->dev);
> > + fdput(f);
> > +
> > + return dev;
> > +}
> > +
> > MODULE_ALIAS("lirc_dev");
> > diff --git a/drivers/media/rc/rc-core-priv.h b/drivers/media/rc/rc-core-priv.h
> > index e0e6a17460f6..148db73cfa0c 100644
> > --- a/drivers/media/rc/rc-core-priv.h
> > +++ b/drivers/media/rc/rc-core-priv.h
> > @@ -13,6 +13,7 @@
> > #define MAX_IR_EVENT_SIZE 512
> >
> > #include <linux/slab.h>
> > +#include <uapi/linux/bpf.h>
> > #include <media/rc-core.h>
> >
> > /**
> > @@ -57,6 +58,11 @@ struct ir_raw_event_ctrl {
> > /* raw decoder state follows */
> > struct ir_raw_event prev_ev;
> > struct ir_raw_event this_ev;
> > +
> > +#ifdef CONFIG_BPF_RAWIR_EVENT
> > + struct bpf_rawir_event bpf_rawir_event;
> > + struct list_head progs;
> > +#endif
> > struct nec_dec {
> > int state;
> > unsigned count;
> > @@ -126,6 +132,9 @@ struct ir_raw_event_ctrl {
> > } imon;
> > };
> >
> > +/* Mutex for locking raw IR processing and handler change */
> > +extern struct mutex ir_raw_handler_lock;
> > +
> > /* macros for IR decoders */
> > static inline bool geq_margin(unsigned d1, unsigned d2, unsigned margin)
> > {
> > @@ -288,6 +297,7 @@ void ir_lirc_raw_event(struct rc_dev *dev, struct ir_raw_event ev);
> > void ir_lirc_scancode_event(struct rc_dev *dev, struct lirc_scancode *lsc);
> > int ir_lirc_register(struct rc_dev *dev);
> > void ir_lirc_unregister(struct rc_dev *dev);
> > +struct rc_dev *rc_dev_get_from_fd(int fd);
> > #else
> > static inline int lirc_dev_init(void) { return 0; }
> > static inline void lirc_dev_exit(void) {}
> > @@ -299,4 +309,18 @@ static inline int ir_lirc_register(struct rc_dev *dev) { return 0; }
> > static inline void ir_lirc_unregister(struct rc_dev *dev) { }
> > #endif
> >
> > +/*
> > + * bpf interface
> > + */
> > +#ifdef CONFIG_BPF_RAWIR_EVENT
> > +void rc_dev_bpf_init(struct rc_dev *dev);
> > +void rc_dev_bpf_free(struct rc_dev *dev);
> > +void rc_dev_bpf_run(struct rc_dev *dev, struct ir_raw_event ev);
> > +#else
> > +static inline void rc_dev_bpf_init(struct rc_dev *dev) { }
> > +static inline void rc_dev_bpf_free(struct rc_dev *dev) { }
> > +static inline void rc_dev_bpf_run(struct rc_dev *dev, struct ir_raw_event ev)
> > +{ }
> > +#endif
> > +
> > #endif /* _RC_CORE_PRIV */
> > diff --git a/drivers/media/rc/rc-ir-raw.c b/drivers/media/rc/rc-ir-raw.c
> > index 374f83105a23..e68cdd4fbf5d 100644
> > --- a/drivers/media/rc/rc-ir-raw.c
> > +++ b/drivers/media/rc/rc-ir-raw.c
> > @@ -14,7 +14,7 @@
> > static LIST_HEAD(ir_raw_client_list);
> >
> > /* Used to handle IR raw handler extensions */
> > -static DEFINE_MUTEX(ir_raw_handler_lock);
> > +DEFINE_MUTEX(ir_raw_handler_lock);
> > static LIST_HEAD(ir_raw_handler_list);
> > static atomic64_t available_protocols = ATOMIC64_INIT(0);
> >
> > @@ -32,6 +32,7 @@ static int ir_raw_event_thread(void *data)
> > handler->protocols || !handler->protocols)
> > handler->decode(raw->dev, ev);
> > ir_lirc_raw_event(raw->dev, ev);
> > + rc_dev_bpf_run(raw->dev, ev);
> > raw->prev_ev = ev;
> > }
> > mutex_unlock(&ir_raw_handler_lock);
> > @@ -572,6 +573,7 @@ int ir_raw_event_prepare(struct rc_dev *dev)
> > spin_lock_init(&dev->raw->edge_spinlock);
> > timer_setup(&dev->raw->edge_handle, ir_raw_edge_handle, 0);
> > INIT_KFIFO(dev->raw->kfifo);
> > + rc_dev_bpf_init(dev);
> >
> > return 0;
> > }
> > @@ -621,9 +623,17 @@ void ir_raw_event_unregister(struct rc_dev *dev)
> > list_for_each_entry(handler, &ir_raw_handler_list, list)
> > if (handler->raw_unregister)
> > handler->raw_unregister(dev);
> > - mutex_unlock(&ir_raw_handler_lock);
> > +
> > + rc_dev_bpf_free(dev);
> >
> > ir_raw_event_free(dev);
> > +
> > + /*
> > + * A user can be calling bpf(BPF_PROG_{QUERY|ATTACH|DETACH}), so
> > + * ensure that the raw member is null on unlock; this is how
> > + * "device gone" is checked.
> > + */
> > + mutex_unlock(&ir_raw_handler_lock);
> > }
> >
> > /*
> > diff --git a/include/linux/bpf_rcdev.h b/include/linux/bpf_rcdev.h
> > new file mode 100644
> > index 000000000000..17a30f30436a
> > --- /dev/null
> > +++ b/include/linux/bpf_rcdev.h
> > @@ -0,0 +1,30 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +#ifndef _BPF_RCDEV_H
> > +#define _BPF_RCDEV_H
> > +
> > +#include <linux/bpf.h>
> > +#include <uapi/linux/bpf.h>
> > +
> > +#ifdef CONFIG_BPF_RAWIR_EVENT
> > +int rc_dev_prog_attach(const union bpf_attr *attr);
> > +int rc_dev_prog_detach(const union bpf_attr *attr);
> > +int rc_dev_prog_query(const union bpf_attr *attr, union bpf_attr __user *uattr);
> > +#else
> > +static inline int rc_dev_prog_attach(const union bpf_attr *attr)
> > +{
> > + return -EINVAL;
> > +}
> > +
> > +static inline int rc_dev_prog_detach(const union bpf_attr *attr)
> > +{
> > + return -EINVAL;
> > +}
> > +
> > +static inline int rc_dev_prog_query(const union bpf_attr *attr,
> > + union bpf_attr __user *uattr)
> > +{
> > + return -EINVAL;
> > +}
> > +#endif
> > +
> > +#endif /* _BPF_RCDEV_H */
> > diff --git a/include/linux/bpf_types.h b/include/linux/bpf_types.h
> > index b67f8793de0d..e2b1b12474d4 100644
> > --- a/include/linux/bpf_types.h
> > +++ b/include/linux/bpf_types.h
> > @@ -25,6 +25,9 @@ BPF_PROG_TYPE(BPF_PROG_TYPE_RAW_TRACEPOINT, raw_tracepoint)
> > #ifdef CONFIG_CGROUP_BPF
> > BPF_PROG_TYPE(BPF_PROG_TYPE_CGROUP_DEVICE, cg_dev)
> > #endif
> > +#ifdef CONFIG_BPF_RAWIR_EVENT
> > +BPF_PROG_TYPE(BPF_PROG_TYPE_RAWIR_EVENT, rawir_event)
> > +#endif
> >
> > BPF_MAP_TYPE(BPF_MAP_TYPE_ARRAY, array_map_ops)
> > BPF_MAP_TYPE(BPF_MAP_TYPE_PERCPU_ARRAY, percpu_array_map_ops)
> > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > index d94d333a8225..243e141e8a5b 100644
> > --- a/include/uapi/linux/bpf.h
> > +++ b/include/uapi/linux/bpf.h
> > @@ -141,6 +141,7 @@ enum bpf_prog_type {
> > BPF_PROG_TYPE_SK_MSG,
> > BPF_PROG_TYPE_RAW_TRACEPOINT,
> > BPF_PROG_TYPE_CGROUP_SOCK_ADDR,
> > + BPF_PROG_TYPE_RAWIR_EVENT,
> > };
> >
> > enum bpf_attach_type {
> > @@ -158,6 +159,7 @@ enum bpf_attach_type {
> > BPF_CGROUP_INET6_CONNECT,
> > BPF_CGROUP_INET4_POST_BIND,
> > BPF_CGROUP_INET6_POST_BIND,
> > + BPF_RAWIR_EVENT,
> > __MAX_BPF_ATTACH_TYPE
> > };
> >
> > @@ -1902,6 +1904,35 @@ union bpf_attr {
> > * egress otherwise). This is the only flag supported for now.
> > * Return
> > * **SK_PASS** on success, or **SK_DROP** on error.
> > + *
> > + * int bpf_rc_keydown(void *ctx, u32 protocol, u32 scancode, u32 toggle)
> > + * Description
> > + * Report decoded scancode with toggle value. For use in
> > + * BPF_PROG_TYPE_RAWIR_EVENT, to report a successfully
> > + * decoded scancode. This is will generate a keydown event,
> > + * and a keyup event once the scancode is no longer repeated.
> > + *
> > + * *ctx* pointer to bpf_rawir_event, *protocol* is decoded
> > + * protocol (see RC_PROTO_* enum).
> > + *
> > + * Some protocols include a toggle bit, in case the button
> > + * was released and pressed again between consecutive scancodes,
> > + * copy this bit into *toggle* if it exists, else set to 0.
> > + *
> > + * Return
> > + * Always return 0 (for now)
> > + *
> > + * int bpf_rc_repeat(void *ctx)
> > + * Description
> > + * Repeat the last decoded scancode; some IR protocols like
> > + * NEC have a special IR message for repeat last button,
> > + * in case user is holding a button down; the scancode is
> > + * not repeated.
> > + *
> > + * *ctx* pointer to bpf_rawir_event.
> > + *
> > + * Return
> > + * Always return 0 (for now)
> > */
> > #define __BPF_FUNC_MAPPER(FN) \
> > FN(unspec), \
> > @@ -1976,7 +2007,9 @@ union bpf_attr {
> > FN(fib_lookup), \
> > FN(sock_hash_update), \
> > FN(msg_redirect_hash), \
> > - FN(sk_redirect_hash),
> > + FN(sk_redirect_hash), \
> > + FN(rc_repeat), \
> > + FN(rc_keydown),
> >
> > /* integer value in 'imm' field of BPF_CALL instruction selects which helper
> > * function eBPF program intends to call
> > @@ -2043,6 +2076,26 @@ enum bpf_hdr_start_off {
> > BPF_HDR_START_NET,
> > };
> >
> > +/*
> > + * user accessible mirror of in-kernel ir_raw_event
> > + */
> > +#define BPF_RAWIR_EVENT_SPACE 0
> > +#define BPF_RAWIR_EVENT_PULSE 1
> > +#define BPF_RAWIR_EVENT_TIMEOUT 2
> > +#define BPF_RAWIR_EVENT_RESET 3
> > +#define BPF_RAWIR_EVENT_CARRIER 4
> > +#define BPF_RAWIR_EVENT_DUTY_CYCLE 5
> > +
> > +struct bpf_rawir_event {
> > + union {
> > + __u32 duration;
> > + __u32 carrier;
> > + __u32 duty_cycle;
> > + };
> > +
> > + __u32 type;
> > +};
> > +
> > /* user accessible mirror of in-kernel sk_buff.
> > * new fields can only be added to the end of this structure
> > */
> > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> > index e2aeb5e89f44..75c089f407c8 100644
> > --- a/kernel/bpf/syscall.c
> > +++ b/kernel/bpf/syscall.c
> > @@ -11,6 +11,7 @@
> > */
> > #include <linux/bpf.h>
> > #include <linux/bpf_trace.h>
> > +#include <linux/bpf_rcdev.h>
> > #include <linux/btf.h>
> > #include <linux/syscalls.h>
> > #include <linux/slab.h>
> > @@ -1567,6 +1568,8 @@ static int bpf_prog_attach(const union bpf_attr *attr)
> > case BPF_SK_SKB_STREAM_PARSER:
> > case BPF_SK_SKB_STREAM_VERDICT:
> > return sockmap_get_from_fd(attr, BPF_PROG_TYPE_SK_SKB, true);
> > + case BPF_RAWIR_EVENT:
> > + return rc_dev_prog_attach(attr);
> > default:
> > return -EINVAL;
> > }
> > @@ -1637,6 +1640,8 @@ static int bpf_prog_detach(const union bpf_attr *attr)
> > case BPF_SK_SKB_STREAM_PARSER:
> > case BPF_SK_SKB_STREAM_VERDICT:
> > return sockmap_get_from_fd(attr, BPF_PROG_TYPE_SK_SKB, false);
> > + case BPF_RAWIR_EVENT:
> > + return rc_dev_prog_detach(attr);
> > default:
> > return -EINVAL;
> > }
> > @@ -1684,6 +1689,8 @@ static int bpf_prog_query(const union bpf_attr *attr,
> > case BPF_CGROUP_SOCK_OPS:
> > case BPF_CGROUP_DEVICE:
> > break;
> > + case BPF_RAWIR_EVENT:
> > + return rc_dev_prog_query(attr, uattr);
> > default:
> > return -EINVAL;
> > }
> > --
> > 2.17.0
> >
^ permalink raw reply
* [PATCH v3 net-next 0/6] tcp: implement SACK compression
From: Eric Dumazet @ 2018-05-17 21:47 UTC (permalink / raw)
To: David S . Miller
Cc: netdev, Toke Høiland-Jørgensen, Neal Cardwell,
Yuchung Cheng, Soheil Hassas Yeganeh, Eric Dumazet, Eric Dumazet
When TCP receives an out-of-order packet, it immediately sends
a SACK packet, generating network load but also forcing the
receiver to send 1-MSS pathological packets, increasing its
RTX queue length/depth, and thus processing time.
Wifi networks suffer from this aggressive behavior, but generally
speaking, all these SACK packets add fuel to the fire when networks
are under congestion.
This patch series adds SACK compression, but the infrastructure
could be leveraged to also compress ACK in the future.
v2: Addressed Neal feedback.
Added two sysctls to allow fine tuning, or even disabling the feature.
v3: take rtt = min(srtt, rcv_rtt) as Yuchung suggested, because rcv_rtt
can be over estimated for RPC (or sender limited)
Eric Dumazet (6):
tcp: use __sock_put() instead of sock_put() in tcp_clear_xmit_timers()
tcp: do not force quickack when receiving out-of-order packets
tcp: add SACK compression
tcp: add TCPAckCompressed SNMP counter
tcp: add tcp_comp_sack_delay_ns sysctl
tcp: add tcp_comp_sack_nr sysctl
Documentation/networking/ip-sysctl.txt | 13 +++++++++
include/linux/tcp.h | 2 ++
include/net/netns/ipv4.h | 2 ++
include/net/tcp.h | 5 +++-
include/uapi/linux/snmp.h | 1 +
net/ipv4/proc.c | 1 +
net/ipv4/sysctl_net_ipv4.c | 17 ++++++++++++
net/ipv4/tcp.c | 1 +
net/ipv4/tcp_input.c | 38 ++++++++++++++++++++------
net/ipv4/tcp_ipv4.c | 2 ++
net/ipv4/tcp_output.c | 9 ++++++
net/ipv4/tcp_timer.c | 25 +++++++++++++++++
12 files changed, 107 insertions(+), 9 deletions(-)
--
2.17.0.441.gb46fe60e1d-goog
^ permalink raw reply
* [PATCH v3 net-next 1/6] tcp: use __sock_put() instead of sock_put() in tcp_clear_xmit_timers()
From: Eric Dumazet @ 2018-05-17 21:47 UTC (permalink / raw)
To: David S . Miller
Cc: netdev, Toke Høiland-Jørgensen, Neal Cardwell,
Yuchung Cheng, Soheil Hassas Yeganeh, Eric Dumazet, Eric Dumazet
In-Reply-To: <20180517214729.186094-1-edumazet@google.com>
Socket can not disappear under us.
Signed-off-by: Eric Dumazet <edumazet@google.com>
Acked-by: Neal Cardwell <ncardwell@google.com>
---
include/net/tcp.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 6deb540297ccaa1f05ce633efe313d1ca2c15dd9..511bd0fde1dc1dd842598d083905b0425bcb05f8 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -559,7 +559,7 @@ void tcp_init_xmit_timers(struct sock *);
static inline void tcp_clear_xmit_timers(struct sock *sk)
{
if (hrtimer_try_to_cancel(&tcp_sk(sk)->pacing_timer) == 1)
- sock_put(sk);
+ __sock_put(sk);
inet_csk_clear_xmit_timers(sk);
}
--
2.17.0.441.gb46fe60e1d-goog
^ permalink raw reply related
* [PATCH v3 net-next 2/6] tcp: do not force quickack when receiving out-of-order packets
From: Eric Dumazet @ 2018-05-17 21:47 UTC (permalink / raw)
To: David S . Miller
Cc: netdev, Toke Høiland-Jørgensen, Neal Cardwell,
Yuchung Cheng, Soheil Hassas Yeganeh, Eric Dumazet, Eric Dumazet
In-Reply-To: <20180517214729.186094-1-edumazet@google.com>
As explained in commit 9f9843a751d0 ("tcp: properly handle stretch
acks in slow start"), TCP stacks have to consider how many packets
are acknowledged in one single ACK, because of GRO, but also
because of ACK compression or losses.
We plan to add SACK compression in the following patch, we
must therefore not call tcp_enter_quickack_mode()
Signed-off-by: Eric Dumazet <edumazet@google.com>
Acked-by: Neal Cardwell <ncardwell@google.com>
Acked-by: Soheil Hassas Yeganeh <soheil@google.com>
---
net/ipv4/tcp_input.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 0bf032839548f8dccb7f24a6fb5a7d47ea29208b..f5622b250665178e44460fa2cd4a11af23dfb23d 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -4715,8 +4715,6 @@ static void tcp_data_queue(struct sock *sk, struct sk_buff *skb)
if (!before(TCP_SKB_CB(skb)->seq, tp->rcv_nxt + tcp_receive_window(tp)))
goto out_of_window;
- tcp_enter_quickack_mode(sk);
-
if (before(TCP_SKB_CB(skb)->seq, tp->rcv_nxt)) {
/* Partial packet, seq < rcv_next < end_seq */
SOCK_DEBUG(sk, "partial packet: rcv_next %X seq %X - %X\n",
--
2.17.0.441.gb46fe60e1d-goog
^ permalink raw reply related
* [PATCH v3 net-next 3/6] tcp: add SACK compression
From: Eric Dumazet @ 2018-05-17 21:47 UTC (permalink / raw)
To: David S . Miller
Cc: netdev, Toke Høiland-Jørgensen, Neal Cardwell,
Yuchung Cheng, Soheil Hassas Yeganeh, Eric Dumazet, Eric Dumazet
In-Reply-To: <20180517214729.186094-1-edumazet@google.com>
When TCP receives an out-of-order packet, it immediately sends
a SACK packet, generating network load but also forcing the
receiver to send 1-MSS pathological packets, increasing its
RTX queue length/depth, and thus processing time.
Wifi networks suffer from this aggressive behavior, but generally
speaking, all these SACK packets add fuel to the fire when networks
are under congestion.
This patch adds a high resolution timer and tp->compressed_ack counter.
Instead of sending a SACK, we program this timer with a small delay,
based on RTT and capped to 1 ms :
delay = min ( 5 % of RTT, 1 ms)
If subsequent SACKs need to be sent while the timer has not yet
expired, we simply increment tp->compressed_ack.
When timer expires, a SACK is sent with the latest information.
Whenever an ACK is sent (if data is sent, or if in-order
data is received) timer is canceled.
Note that tcp_sack_new_ofo_skb() is able to force a SACK to be sent
if the sack blocks need to be shuffled, even if the timer has not
expired.
A new SNMP counter is added in the following patch.
Two other patches add sysctls to allow changing the 1,000,000 and 44
values that this commit hard-coded.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/linux/tcp.h | 2 ++
include/net/tcp.h | 3 +++
net/ipv4/tcp.c | 1 +
net/ipv4/tcp_input.c | 35 +++++++++++++++++++++++++++++------
net/ipv4/tcp_output.c | 7 +++++++
net/ipv4/tcp_timer.c | 25 +++++++++++++++++++++++++
6 files changed, 67 insertions(+), 6 deletions(-)
diff --git a/include/linux/tcp.h b/include/linux/tcp.h
index 807776928cb8610fe97121fbc3c600b08d5d2991..72705eaf4b84060a45bf04d5170f389a18010eac 100644
--- a/include/linux/tcp.h
+++ b/include/linux/tcp.h
@@ -218,6 +218,7 @@ struct tcp_sock {
reord:1; /* reordering detected */
} rack;
u16 advmss; /* Advertised MSS */
+ u8 compressed_ack;
u32 chrono_start; /* Start time in jiffies of a TCP chrono */
u32 chrono_stat[3]; /* Time in jiffies for chrono_stat stats */
u8 chrono_type:2, /* current chronograph type */
@@ -297,6 +298,7 @@ struct tcp_sock {
u32 sacked_out; /* SACK'd packets */
struct hrtimer pacing_timer;
+ struct hrtimer compressed_ack_timer;
/* from STCP, retrans queue hinting */
struct sk_buff* lost_skb_hint;
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 511bd0fde1dc1dd842598d083905b0425bcb05f8..952d842a604a3ed79e1bf87a712db20a461c35a9 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -561,6 +561,9 @@ static inline void tcp_clear_xmit_timers(struct sock *sk)
if (hrtimer_try_to_cancel(&tcp_sk(sk)->pacing_timer) == 1)
__sock_put(sk);
+ if (hrtimer_try_to_cancel(&tcp_sk(sk)->compressed_ack_timer) == 1)
+ __sock_put(sk);
+
inet_csk_clear_xmit_timers(sk);
}
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 62b776f9003798eaf06992a4eb0914d17646aa61..0a2ea0bbf867271db05aedd7d48b193677664321 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -2595,6 +2595,7 @@ int tcp_disconnect(struct sock *sk, int flags)
dst_release(sk->sk_rx_dst);
sk->sk_rx_dst = NULL;
tcp_saved_syn_free(tp);
+ tp->compressed_ack = 0;
/* Clean up fastopen related fields */
tcp_free_fastopen_req(tp);
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index f5622b250665178e44460fa2cd4a11af23dfb23d..cc2ac5346b92b968593f919192d543384865bcb8 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -4249,6 +4249,8 @@ static void tcp_sack_new_ofo_skb(struct sock *sk, u32 seq, u32 end_seq)
* If the sack array is full, forget about the last one.
*/
if (this_sack >= TCP_NUM_SACKS) {
+ if (tp->compressed_ack)
+ tcp_send_ack(sk);
this_sack--;
tp->rx_opt.num_sacks--;
sp--;
@@ -5081,6 +5083,7 @@ static inline void tcp_data_snd_check(struct sock *sk)
static void __tcp_ack_snd_check(struct sock *sk, int ofo_possible)
{
struct tcp_sock *tp = tcp_sk(sk);
+ unsigned long rtt, delay;
/* More than one full frame received... */
if (((tp->rcv_nxt - tp->rcv_wup) > inet_csk(sk)->icsk_ack.rcv_mss &&
@@ -5092,15 +5095,35 @@ static void __tcp_ack_snd_check(struct sock *sk, int ofo_possible)
(tp->rcv_nxt - tp->copied_seq < sk->sk_rcvlowat ||
__tcp_select_window(sk) >= tp->rcv_wnd)) ||
/* We ACK each frame or... */
- tcp_in_quickack_mode(sk) ||
- /* We have out of order data. */
- (ofo_possible && !RB_EMPTY_ROOT(&tp->out_of_order_queue))) {
- /* Then ack it now */
+ tcp_in_quickack_mode(sk)) {
+send_now:
tcp_send_ack(sk);
- } else {
- /* Else, send delayed ack. */
+ return;
+ }
+
+ if (!ofo_possible || RB_EMPTY_ROOT(&tp->out_of_order_queue)) {
tcp_send_delayed_ack(sk);
+ return;
}
+
+ if (!tcp_is_sack(tp) || tp->compressed_ack >= 44)
+ goto send_now;
+ tp->compressed_ack++;
+
+ if (hrtimer_is_queued(&tp->compressed_ack_timer))
+ return;
+
+ /* compress ack timer : 5 % of rtt, but no more than 1 ms */
+
+ rtt = tp->rcv_rtt_est.rtt_us;
+ if (tp->srtt_us && tp->srtt_us < rtt)
+ rtt = tp->srtt_us;
+
+ delay = min_t(unsigned long, NSEC_PER_MSEC,
+ rtt * (NSEC_PER_USEC >> 3)/20);
+ sock_hold(sk);
+ hrtimer_start(&tp->compressed_ack_timer, ns_to_ktime(delay),
+ HRTIMER_MODE_REL_PINNED_SOFT);
}
static inline void tcp_ack_snd_check(struct sock *sk)
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 0d8f950a9006598c70dbf51e281a3fe32dfaa234..7ee98aad82b758674ca7f3e90bd3fc165e8fcd45 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -162,6 +162,13 @@ static void tcp_event_data_sent(struct tcp_sock *tp,
/* Account for an ACK we sent. */
static inline void tcp_event_ack_sent(struct sock *sk, unsigned int pkts)
{
+ struct tcp_sock *tp = tcp_sk(sk);
+
+ if (unlikely(tp->compressed_ack)) {
+ tp->compressed_ack = 0;
+ if (hrtimer_try_to_cancel(&tp->compressed_ack_timer) == 1)
+ __sock_put(sk);
+ }
tcp_dec_quickack_mode(sk, pkts);
inet_csk_clear_xmit_timer(sk, ICSK_TIME_DACK);
}
diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index 92bdf64fffae3a5be291ca419eb21276b4c8cbae..3b3611729928f77934e0298bb248e55c7a7c5def 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -708,6 +708,27 @@ static void tcp_keepalive_timer (struct timer_list *t)
sock_put(sk);
}
+static enum hrtimer_restart tcp_compressed_ack_kick(struct hrtimer *timer)
+{
+ struct tcp_sock *tp = container_of(timer, struct tcp_sock, compressed_ack_timer);
+ struct sock *sk = (struct sock *)tp;
+
+ bh_lock_sock(sk);
+ if (!sock_owned_by_user(sk)) {
+ if (tp->compressed_ack)
+ tcp_send_ack(sk);
+ } else {
+ if (!test_and_set_bit(TCP_DELACK_TIMER_DEFERRED,
+ &sk->sk_tsq_flags))
+ sock_hold(sk);
+ }
+ bh_unlock_sock(sk);
+
+ sock_put(sk);
+
+ return HRTIMER_NORESTART;
+}
+
void tcp_init_xmit_timers(struct sock *sk)
{
inet_csk_init_xmit_timers(sk, &tcp_write_timer, &tcp_delack_timer,
@@ -715,4 +736,8 @@ void tcp_init_xmit_timers(struct sock *sk)
hrtimer_init(&tcp_sk(sk)->pacing_timer, CLOCK_MONOTONIC,
HRTIMER_MODE_ABS_PINNED_SOFT);
tcp_sk(sk)->pacing_timer.function = tcp_pace_kick;
+
+ hrtimer_init(&tcp_sk(sk)->compressed_ack_timer, CLOCK_MONOTONIC,
+ HRTIMER_MODE_REL_PINNED_SOFT);
+ tcp_sk(sk)->compressed_ack_timer.function = tcp_compressed_ack_kick;
}
--
2.17.0.441.gb46fe60e1d-goog
^ permalink raw reply related
* [PATCH v3 net-next 4/6] tcp: add TCPAckCompressed SNMP counter
From: Eric Dumazet @ 2018-05-17 21:47 UTC (permalink / raw)
To: David S . Miller
Cc: netdev, Toke Høiland-Jørgensen, Neal Cardwell,
Yuchung Cheng, Soheil Hassas Yeganeh, Eric Dumazet, Eric Dumazet
In-Reply-To: <20180517214729.186094-1-edumazet@google.com>
This counter tracks number of ACK packets that the host has not sent,
thanks to ACK compression.
Sample output :
$ nstat -n;sleep 1;nstat|egrep "IpInReceives|IpOutRequests|TcpInSegs|TcpOutSegs|TcpExtTCPAckCompressed"
IpInReceives 123250 0.0
IpOutRequests 3684 0.0
TcpInSegs 123251 0.0
TcpOutSegs 3684 0.0
TcpExtTCPAckCompressed 119252 0.0
Signed-off-by: Eric Dumazet <edumazet@google.com>
Acked-by: Neal Cardwell <ncardwell@google.com>
---
include/uapi/linux/snmp.h | 1 +
net/ipv4/proc.c | 1 +
net/ipv4/tcp_output.c | 2 ++
3 files changed, 4 insertions(+)
diff --git a/include/uapi/linux/snmp.h b/include/uapi/linux/snmp.h
index d02e859301ff499dd72a1c0e1b56bed10a9397a6..750d89120335eb489f698191edb6c5110969fa8c 100644
--- a/include/uapi/linux/snmp.h
+++ b/include/uapi/linux/snmp.h
@@ -278,6 +278,7 @@ enum
LINUX_MIB_TCPMTUPSUCCESS, /* TCPMTUPSuccess */
LINUX_MIB_TCPDELIVERED, /* TCPDelivered */
LINUX_MIB_TCPDELIVEREDCE, /* TCPDeliveredCE */
+ LINUX_MIB_TCPACKCOMPRESSED, /* TCPAckCompressed */
__LINUX_MIB_MAX
};
diff --git a/net/ipv4/proc.c b/net/ipv4/proc.c
index 261b71d0ccc5c17c6032bf67eb8f842006766e64..6c1ff89a60fa0a3485dcc71fafc799e798d5dc11 100644
--- a/net/ipv4/proc.c
+++ b/net/ipv4/proc.c
@@ -298,6 +298,7 @@ static const struct snmp_mib snmp4_net_list[] = {
SNMP_MIB_ITEM("TCPMTUPSuccess", LINUX_MIB_TCPMTUPSUCCESS),
SNMP_MIB_ITEM("TCPDelivered", LINUX_MIB_TCPDELIVERED),
SNMP_MIB_ITEM("TCPDeliveredCE", LINUX_MIB_TCPDELIVEREDCE),
+ SNMP_MIB_ITEM("TCPAckCompressed", LINUX_MIB_TCPACKCOMPRESSED),
SNMP_MIB_SENTINEL
};
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 7ee98aad82b758674ca7f3e90bd3fc165e8fcd45..437bb7ceba7fd388abac1c12f2920b02be77bad9 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -165,6 +165,8 @@ static inline void tcp_event_ack_sent(struct sock *sk, unsigned int pkts)
struct tcp_sock *tp = tcp_sk(sk);
if (unlikely(tp->compressed_ack)) {
+ NET_ADD_STATS(sock_net(sk), LINUX_MIB_TCPACKCOMPRESSED,
+ tp->compressed_ack);
tp->compressed_ack = 0;
if (hrtimer_try_to_cancel(&tp->compressed_ack_timer) == 1)
__sock_put(sk);
--
2.17.0.441.gb46fe60e1d-goog
^ permalink raw reply related
* [PATCH v3 net-next 5/6] tcp: add tcp_comp_sack_delay_ns sysctl
From: Eric Dumazet @ 2018-05-17 21:47 UTC (permalink / raw)
To: David S . Miller
Cc: netdev, Toke Høiland-Jørgensen, Neal Cardwell,
Yuchung Cheng, Soheil Hassas Yeganeh, Eric Dumazet, Eric Dumazet
In-Reply-To: <20180517214729.186094-1-edumazet@google.com>
This per netns sysctl allows for TCP SACK compression fine-tuning.
Its default value is 1,000,000, or 1 ms to meet TSO autosizing period.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
Documentation/networking/ip-sysctl.txt | 7 +++++++
include/net/netns/ipv4.h | 1 +
net/ipv4/sysctl_net_ipv4.c | 7 +++++++
net/ipv4/tcp_input.c | 4 ++--
net/ipv4/tcp_ipv4.c | 1 +
5 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
index ea304a23c8d72c92a925d0c107bfd2bcfbbb92ec..7ba952959bca0eee4ecf81fb5837e17790db0fde 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt
@@ -525,6 +525,13 @@ tcp_rmem - vector of 3 INTEGERs: min, default, max
tcp_sack - BOOLEAN
Enable select acknowledgments (SACKS).
+tcp_comp_sack_delay_ns - LONG INTEGER
+ TCP tries to reduce number of SACK sent, using a timer
+ based on 5% of SRTT, capped by this sysctl, in nano seconds.
+ The default is 1ms, based on TSO autosizing period.
+
+ Default : 1,000,000 ns (1 ms)
+
tcp_slow_start_after_idle - BOOLEAN
If set, provide RFC2861 behavior and time out the congestion
window after an idle period. An idle period is defined at
diff --git a/include/net/netns/ipv4.h b/include/net/netns/ipv4.h
index 8491bc9c86b1553ab603e4363e8e38ca7ff547e0..927318243cfaa2ddd8eb423c6ba6e66253f771d3 100644
--- a/include/net/netns/ipv4.h
+++ b/include/net/netns/ipv4.h
@@ -160,6 +160,7 @@ struct netns_ipv4 {
int sysctl_tcp_pacing_ca_ratio;
int sysctl_tcp_wmem[3];
int sysctl_tcp_rmem[3];
+ unsigned long sysctl_tcp_comp_sack_delay_ns;
struct inet_timewait_death_row tcp_death_row;
int sysctl_max_syn_backlog;
int sysctl_tcp_fastopen;
diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
index 4b195bac8ac0eefe0a224528ad854338c4f8e6e3..11fbfdc1566eca95f91360522178295318277588 100644
--- a/net/ipv4/sysctl_net_ipv4.c
+++ b/net/ipv4/sysctl_net_ipv4.c
@@ -1151,6 +1151,13 @@ static struct ctl_table ipv4_net_table[] = {
.proc_handler = proc_dointvec_minmax,
.extra1 = &one,
},
+ {
+ .procname = "tcp_comp_sack_delay_ns",
+ .data = &init_net.ipv4.sysctl_tcp_comp_sack_delay_ns,
+ .maxlen = sizeof(unsigned long),
+ .mode = 0644,
+ .proc_handler = proc_doulongvec_minmax,
+ },
{
.procname = "udp_rmem_min",
.data = &init_net.ipv4.sysctl_udp_rmem_min,
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index cc2ac5346b92b968593f919192d543384865bcb8..6a1dae38c9558c7bc9dd31e9f16c4e8ea8c78149 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -5113,13 +5113,13 @@ static void __tcp_ack_snd_check(struct sock *sk, int ofo_possible)
if (hrtimer_is_queued(&tp->compressed_ack_timer))
return;
- /* compress ack timer : 5 % of rtt, but no more than 1 ms */
+ /* compress ack timer : 5 % of rtt, but no more than tcp_comp_sack_delay_ns */
rtt = tp->rcv_rtt_est.rtt_us;
if (tp->srtt_us && tp->srtt_us < rtt)
rtt = tp->srtt_us;
- delay = min_t(unsigned long, NSEC_PER_MSEC,
+ delay = min_t(unsigned long, sock_net(sk)->ipv4.sysctl_tcp_comp_sack_delay_ns,
rtt * (NSEC_PER_USEC >> 3)/20);
sock_hold(sk);
hrtimer_start(&tp->compressed_ack_timer, ns_to_ktime(delay),
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index caf23de88f8a369c2038cecd34ce42c522487e90..a3f4647341db2eb5a63c3e9f1e8b93099aedadab 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -2572,6 +2572,7 @@ static int __net_init tcp_sk_init(struct net *net)
init_net.ipv4.sysctl_tcp_wmem,
sizeof(init_net.ipv4.sysctl_tcp_wmem));
}
+ net->ipv4.sysctl_tcp_comp_sack_delay_ns = NSEC_PER_MSEC;
net->ipv4.sysctl_tcp_fastopen = TFO_CLIENT_ENABLE;
spin_lock_init(&net->ipv4.tcp_fastopen_ctx_lock);
net->ipv4.sysctl_tcp_fastopen_blackhole_timeout = 60 * 60;
--
2.17.0.441.gb46fe60e1d-goog
^ permalink raw reply related
* [PATCH v3 net-next 6/6] tcp: add tcp_comp_sack_nr sysctl
From: Eric Dumazet @ 2018-05-17 21:47 UTC (permalink / raw)
To: David S . Miller
Cc: netdev, Toke Høiland-Jørgensen, Neal Cardwell,
Yuchung Cheng, Soheil Hassas Yeganeh, Eric Dumazet, Eric Dumazet
In-Reply-To: <20180517214729.186094-1-edumazet@google.com>
This per netns sysctl allows for TCP SACK compression fine-tuning.
This limits number of SACK that can be compressed.
Using 0 disables SACK compression.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
Documentation/networking/ip-sysctl.txt | 6 ++++++
include/net/netns/ipv4.h | 1 +
net/ipv4/sysctl_net_ipv4.c | 10 ++++++++++
net/ipv4/tcp_input.c | 3 ++-
net/ipv4/tcp_ipv4.c | 1 +
5 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
index 7ba952959bca0eee4ecf81fb5837e17790db0fde..924bd51327b7a8dff3503d7afccdd54e1eb5c29b 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt
@@ -532,6 +532,12 @@ tcp_comp_sack_delay_ns - LONG INTEGER
Default : 1,000,000 ns (1 ms)
+tcp_comp_sack_nr - INTEGER
+ Max numer of SACK that can be compressed.
+ Using 0 disables SACK compression.
+
+ Detault : 44
+
tcp_slow_start_after_idle - BOOLEAN
If set, provide RFC2861 behavior and time out the congestion
window after an idle period. An idle period is defined at
diff --git a/include/net/netns/ipv4.h b/include/net/netns/ipv4.h
index 927318243cfaa2ddd8eb423c6ba6e66253f771d3..661348f23ea5a3a9320b2cafcd17e23960214771 100644
--- a/include/net/netns/ipv4.h
+++ b/include/net/netns/ipv4.h
@@ -160,6 +160,7 @@ struct netns_ipv4 {
int sysctl_tcp_pacing_ca_ratio;
int sysctl_tcp_wmem[3];
int sysctl_tcp_rmem[3];
+ int sysctl_tcp_comp_sack_nr;
unsigned long sysctl_tcp_comp_sack_delay_ns;
struct inet_timewait_death_row tcp_death_row;
int sysctl_max_syn_backlog;
diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
index 11fbfdc1566eca95f91360522178295318277588..d2eed3ddcb0a1ad9778d96d46c685f6c60b93d8d 100644
--- a/net/ipv4/sysctl_net_ipv4.c
+++ b/net/ipv4/sysctl_net_ipv4.c
@@ -46,6 +46,7 @@ static int tcp_syn_retries_min = 1;
static int tcp_syn_retries_max = MAX_TCP_SYNCNT;
static int ip_ping_group_range_min[] = { 0, 0 };
static int ip_ping_group_range_max[] = { GID_T_MAX, GID_T_MAX };
+static int comp_sack_nr_max = 255;
/* obsolete */
static int sysctl_tcp_low_latency __read_mostly;
@@ -1158,6 +1159,15 @@ static struct ctl_table ipv4_net_table[] = {
.mode = 0644,
.proc_handler = proc_doulongvec_minmax,
},
+ {
+ .procname = "tcp_comp_sack_nr",
+ .data = &init_net.ipv4.sysctl_tcp_comp_sack_nr,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = proc_dointvec_minmax,
+ .extra1 = &zero,
+ .extra2 = &comp_sack_nr_max,
+ },
{
.procname = "udp_rmem_min",
.data = &init_net.ipv4.sysctl_udp_rmem_min,
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 6a1dae38c9558c7bc9dd31e9f16c4e8ea8c78149..aebb29ab2fdf2ceaa182cd11928f145a886149ff 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -5106,7 +5106,8 @@ static void __tcp_ack_snd_check(struct sock *sk, int ofo_possible)
return;
}
- if (!tcp_is_sack(tp) || tp->compressed_ack >= 44)
+ if (!tcp_is_sack(tp) ||
+ tp->compressed_ack >= sock_net(sk)->ipv4.sysctl_tcp_comp_sack_nr)
goto send_now;
tp->compressed_ack++;
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index a3f4647341db2eb5a63c3e9f1e8b93099aedadab..adbdb503db0c983ef4185f83b138aa51bafd15bf 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -2573,6 +2573,7 @@ static int __net_init tcp_sk_init(struct net *net)
sizeof(init_net.ipv4.sysctl_tcp_wmem));
}
net->ipv4.sysctl_tcp_comp_sack_delay_ns = NSEC_PER_MSEC;
+ net->ipv4.sysctl_tcp_comp_sack_nr = 44;
net->ipv4.sysctl_tcp_fastopen = TFO_CLIENT_ENABLE;
spin_lock_init(&net->ipv4.tcp_fastopen_ctx_lock);
net->ipv4.sysctl_tcp_fastopen_blackhole_timeout = 60 * 60;
--
2.17.0.441.gb46fe60e1d-goog
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox