* [PATCH] ipv4: mark expected switch fall-throughs
From: Gustavo A. R. Silva @ 2017-10-16 20:48 UTC (permalink / raw)
To: David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI,
Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal
Cc: netdev, linux-kernel, netfilter-devel, coreteam,
Gustavo A. R. Silva
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.
Notice that in some cases I placed the "fall through" comment
on its own line, which is what GCC is expecting to find.
Addresses-Coverity-ID: 115108
Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
---
This code was tested by compilation only (GCC 7.2.0 was used).
Please, verify if the actual intention of the code is to fall through.
net/ipv4/af_inet.c | 3 ++-
net/ipv4/arp.c | 1 +
net/ipv4/devinet.c | 1 +
net/ipv4/ipmr.c | 1 +
net/ipv4/netfilter/nf_nat_l3proto_ipv4.c | 3 ++-
net/ipv4/tcp_input.c | 2 ++
net/ipv4/tcp_ipv4.c | 3 ++-
7 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 43a1bbe..ce4aa82 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -827,6 +827,7 @@ int inet_shutdown(struct socket *sock, int how)
err = -ENOTCONN;
/* Hack to wake up other listeners, who can poll for
POLLHUP, even on eg. unconnected UDP sockets -- RR */
+ /* fall through */
default:
sk->sk_shutdown |= how;
if (sk->sk_prot->shutdown)
@@ -840,7 +841,7 @@ int inet_shutdown(struct socket *sock, int how)
case TCP_LISTEN:
if (!(how & RCV_SHUTDOWN))
break;
- /* Fall through */
+ /* fall through */
case TCP_SYN_SENT:
err = sk->sk_prot->disconnect(sk, O_NONBLOCK);
sock->state = err ? SS_DISCONNECTING : SS_UNCONNECTED;
diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
index 7c45b88..a8d7c5a 100644
--- a/net/ipv4/arp.c
+++ b/net/ipv4/arp.c
@@ -1180,6 +1180,7 @@ int arp_ioctl(struct net *net, unsigned int cmd, void __user *arg)
case SIOCSARP:
if (!ns_capable(net->user_ns, CAP_NET_ADMIN))
return -EPERM;
+ /* fall through */
case SIOCGARP:
err = copy_from_user(&r, arg, sizeof(struct arpreq));
if (err)
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index 7ce22a2..d1aee7a 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -1522,6 +1522,7 @@ static int inetdev_event(struct notifier_block *this, unsigned long event,
if (inetdev_valid_mtu(dev->mtu))
break;
/* disable IP when MTU is not enough */
+ /* fall through */
case NETDEV_UNREGISTER:
inetdev_destroy(in_dev);
break;
diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index b3ee01b..40a43ad 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -1528,6 +1528,7 @@ int ip_mroute_setsockopt(struct sock *sk, int optname, char __user *optval,
case MRT_ADD_MFC:
case MRT_DEL_MFC:
parent = -1;
+ /* fall through */
case MRT_ADD_MFC_PROXY:
case MRT_DEL_MFC_PROXY:
if (optlen != sizeof(mfc)) {
diff --git a/net/ipv4/netfilter/nf_nat_l3proto_ipv4.c b/net/ipv4/netfilter/nf_nat_l3proto_ipv4.c
index a0f37b2..0443ca4 100644
--- a/net/ipv4/netfilter/nf_nat_l3proto_ipv4.c
+++ b/net/ipv4/netfilter/nf_nat_l3proto_ipv4.c
@@ -276,7 +276,8 @@ nf_nat_ipv4_fn(void *priv, struct sk_buff *skb,
else
return NF_ACCEPT;
}
- /* Fall thru... (Only ICMPs can be IP_CT_IS_REPLY) */
+ /* Only ICMPs can be IP_CT_IS_REPLY: */
+ /* fall through */
case IP_CT_NEW:
/* Seen it before? This can happen for loopback, retrans,
* or local packets.
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index d0682ce..b2390bf 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -2885,6 +2885,7 @@ static void tcp_fastretrans_alert(struct sock *sk, const int acked,
(*ack_flag & FLAG_LOST_RETRANS)))
return;
/* Change state if cwnd is undone or retransmits are lost */
+ /* fall through */
default:
if (tcp_is_reno(tp)) {
if (flag & FLAG_SND_UNA_ADVANCED)
@@ -6044,6 +6045,7 @@ int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb)
case TCP_LAST_ACK:
if (!before(TCP_SKB_CB(skb)->seq, tp->rcv_nxt))
break;
+ /* fall through */
case TCP_FIN_WAIT1:
case TCP_FIN_WAIT2:
/* RFC 793 says to queue data in these states,
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 5418ecf..ecee4dd 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1779,8 +1779,9 @@ int tcp_v4_rcv(struct sk_buff *skb)
refcounted = false;
goto process;
}
- /* Fall through to ACK */
}
+ /* to ACK */
+ /* fall through */
case TCP_TW_ACK:
tcp_v4_timewait_ack(sk, skb);
break;
--
2.7.4
^ permalink raw reply related
* Re: [PATCH 3/3] bpf: Make sure that ->comm does not change under us.
From: Daniel Borkmann @ 2017-10-16 20:50 UTC (permalink / raw)
To: Richard Weinberger, netdev; +Cc: linux-kernel, ast
In-Reply-To: <20171016181856.12497-3-richard@nod.at>
On 10/16/2017 08:18 PM, Richard Weinberger wrote:
> Sadly we cannot use get_task_comm() since bpf_get_current_comm()
> allows truncation.
>
> Signed-off-by: Richard Weinberger <richard@nod.at>
> ---
> kernel/bpf/helpers.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index 511c9d522cfc..4b042b24524d 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -18,6 +18,7 @@
> #include <linux/sched.h>
> #include <linux/uidgid.h>
> #include <linux/filter.h>
> +#include <linux/sched/task.h>
>
> /* If kernel subsystem is allowing eBPF programs to call this function,
> * inside its own verifier_ops->get_func_proto() callback it should return
> @@ -149,7 +150,9 @@ BPF_CALL_2(bpf_get_current_comm, char *, buf, u32, size)
> {
> struct task_struct *task = current;
>
> + task_lock(task);
> strncpy(buf, task->comm, size);
> + task_unlock(task);
Wouldn't this potentially lead to a deadlock? E.g. you attach yourself
to task_lock() / spin_lock() / etc, and then the BPF prog triggers the
bpf_get_current_comm() taking the lock again ...
> /* Verifier guarantees that size > 0. For task->comm exceeding
> * size, guarantee that buf is %NUL-terminated. Unconditionally
>
^ permalink raw reply
* Re: [PATCH 3/3] bpf: Make sure that ->comm does not change under us.
From: Richard Weinberger @ 2017-10-16 20:55 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: netdev, linux-kernel, ast
In-Reply-To: <59E51BA3.8040106@iogearbox.net>
Am Montag, 16. Oktober 2017, 22:50:43 CEST schrieb Daniel Borkmann:
> > struct task_struct *task = current;
> >
> > + task_lock(task);
> >
> > strncpy(buf, task->comm, size);
> >
> > + task_unlock(task);
>
> Wouldn't this potentially lead to a deadlock? E.g. you attach yourself
> to task_lock() / spin_lock() / etc, and then the BPF prog triggers the
> bpf_get_current_comm() taking the lock again ...
Yes, but doesn't the same apply to the use case when I attach to strncpy()
and run bpf_get_current_comm()?
Thanks,
//richard
^ permalink raw reply
* Re: [PATCH 3/3] bpf: Make sure that ->comm does not change under us.
From: Daniel Borkmann @ 2017-10-16 21:02 UTC (permalink / raw)
To: Richard Weinberger; +Cc: netdev, linux-kernel, ast
In-Reply-To: <3636052.F7cf4ubS1t@blindfold>
On 10/16/2017 10:55 PM, Richard Weinberger wrote:
> Am Montag, 16. Oktober 2017, 22:50:43 CEST schrieb Daniel Borkmann:
>>> struct task_struct *task = current;
>>>
>>> + task_lock(task);
>>>
>>> strncpy(buf, task->comm, size);
>>>
>>> + task_unlock(task);
>>
>> Wouldn't this potentially lead to a deadlock? E.g. you attach yourself
>> to task_lock() / spin_lock() / etc, and then the BPF prog triggers the
>> bpf_get_current_comm() taking the lock again ...
>
> Yes, but doesn't the same apply to the use case when I attach to strncpy()
> and run bpf_get_current_comm()?
You mean due to recursion? In that case trace_call_bpf() would bail out
due to the bpf_prog_active counter.
^ permalink raw reply
* Re: [next-queue PATCH v8 0/6] TSN: Add qdisc based config interface for CBS
From: Guedes, Andre @ 2017-10-16 21:07 UTC (permalink / raw)
To: davem@davemloft.net, Gomes, Vinicius, Kirsher, Jeffrey T
Cc: jiri@resnulli.us, jhs@mojatatu.com, Ong, Boon Leong,
rodney.cummings@ni.com, levipearson@gmail.com,
xiyou.wangcong@gmail.com, Sanchez-Palencia, Jesus,
richardcochran@gmail.com, netdev@vger.kernel.org,
henrik@austad.us, Briano, Ivan, intel-wired-lan@lists.osuosl.org
In-Reply-To: <20171016.205146.1053238236578937188.davem@davemloft.net>
+Jeff
Since this patchset also touches the IGB driver, Jeff suggested we merged it
into his tree and he'd make a pull-request to your net-next tree.
@Jeff, are we moving with this approach or should David apply the patchset to
his net-next tree directly?
Thanks,
Andre
On Mon, 2017-10-16 at 20:51 +0100, David Miller wrote:
> I'm fine with this patch set. I see it's against Jeff's next-queue, so
> where exactly do you want this to be merged? My net-next tree?
>
> Thank you.
^ permalink raw reply
* Re: [PATCH 3/3] bpf: Make sure that ->comm does not change under us.
From: Richard Weinberger @ 2017-10-16 21:10 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: netdev, linux-kernel, ast
In-Reply-To: <59E51E4E.4060009@iogearbox.net>
Am Montag, 16. Oktober 2017, 23:02:06 CEST schrieb Daniel Borkmann:
> On 10/16/2017 10:55 PM, Richard Weinberger wrote:
> > Am Montag, 16. Oktober 2017, 22:50:43 CEST schrieb Daniel Borkmann:
> >>> struct task_struct *task = current;
> >>>
> >>> + task_lock(task);
> >>>
> >>> strncpy(buf, task->comm, size);
> >>>
> >>> + task_unlock(task);
> >>
> >> Wouldn't this potentially lead to a deadlock? E.g. you attach yourself
> >> to task_lock() / spin_lock() / etc, and then the BPF prog triggers the
> >> bpf_get_current_comm() taking the lock again ...
> >
> > Yes, but doesn't the same apply to the use case when I attach to strncpy()
> > and run bpf_get_current_comm()?
>
> You mean due to recursion? In that case trace_call_bpf() would bail out
> due to the bpf_prog_active counter.
Ah, that's true.
So, when someone wants to use bpf_get_current_comm() while tracing task_lock,
we have a problem. I agree.
On the other hand, without locking the function may return wrong results.
Thanks,
//richard
^ permalink raw reply
* [PATCH net-next] tcp: Use pI6c in tcp tracepoint
From: David Ahern @ 2017-10-16 21:24 UTC (permalink / raw)
To: netdev; +Cc: xiyou.wangcong, David Ahern
The compact form for IPv6 addresses is more user friendly than the full
version. For example:
compact: 2001:db8:1::1
full: 2001:0db8:0001:0000:0000:0000:0000:0004i
Update the tcp tracepoint to show the compact form.
Signed-off-by: David Ahern <dsahern@gmail.com>
---
include/trace/events/tcp.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/trace/events/tcp.h b/include/trace/events/tcp.h
index 3d1cbd072b7e..1ffab6d96e94 100644
--- a/include/trace/events/tcp.h
+++ b/include/trace/events/tcp.h
@@ -57,7 +57,7 @@ TRACE_EVENT(tcp_retransmit_skb,
}
),
- TP_printk("sport=%hu dport=%hu saddr=%pI4 daddr=%pI4 saddrv6=%pI6 daddrv6=%pI6",
+ TP_printk("sport=%hu dport=%hu saddr=%pI4 daddr=%pI4 saddrv6=%pI6c daddrv6=%pI6c",
__entry->sport, __entry->dport, __entry->saddr, __entry->daddr,
__entry->saddr_v6, __entry->daddr_v6)
);
--
2.1.4
^ permalink raw reply related
* Re: [iproute2] regression in ss output
From: Stephen Hemminger @ 2017-10-16 21:28 UTC (permalink / raw)
To: Humberto Alves; +Cc: Phil Sutter, netdev@vger.kernel.org
In-Reply-To: <AM4PR0901MB12527C55F04814777A0E6849CC4F0@AM4PR0901MB1252.eurprd09.prod.outlook.com>
On Mon, 16 Oct 2017 20:44:07 +0000
Humberto Alves <hjalves@live.com> wrote:
> Yes, just get rid of this 'if statement'.
> in6addr_any should be represented as '::', not '*'. Otherwise it's
> impossible to distinguish IPv4 listening addresses from IPv6. Thank you :)
But IPv6 accepts IPv4 as well.
^ permalink raw reply
* [PATCH net-next] tcp: Check daddr_cache before use in tracepoint
From: David Ahern @ 2017-10-16 21:29 UTC (permalink / raw)
To: netdev; +Cc: xiyou.wangcong, David Ahern
Running perf in one window to capture tcp_retransmit_skb tracepoint:
$ perf record -e tcp:tcp_retransmit_skb -a
And causing a retransmission on an active TCP session (e.g., dropping
packets in the receiver, changing MTU on the interface to 500 and back
to 1500) triggers a panic:
[ 58.543144] BUG: unable to handle kernel NULL pointer dereference at 0000000000000008
[ 58.545300] IP: perf_trace_tcp_retransmit_skb+0xd0/0x145
[ 58.546770] PGD 0 P4D 0
[ 58.547472] Oops: 0000 [#1] SMP
[ 58.548328] Modules linked in: vrf
[ 58.549262] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.14.0-rc4+ #26
[ 58.551004] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.7.5-20140531_083030-gandalf 04/01/2014
[ 58.554560] task: ffffffff81a0e540 task.stack: ffffffff81a00000
[ 58.555817] RIP: 0010:perf_trace_tcp_retransmit_skb+0xd0/0x145
[ 58.557137] RSP: 0018:ffff88003fc03d68 EFLAGS: 00010282
[ 58.558292] RAX: 0000000000000000 RBX: ffffe8ffffc0ec80 RCX: ffff880038543098
[ 58.559850] RDX: 0400000000000000 RSI: ffff88003fc03d70 RDI: ffff88003fc14b68
[ 58.561099] RBP: ffff88003fc03da8 R08: 0000000000000000 R09: ffffea0000d3224a
[ 58.562005] R10: ffff88003fc03db8 R11: 0000000000000010 R12: ffff8800385428c0
[ 58.562930] R13: ffffe8ffffc0e478 R14: ffffffff81a93a40 R15: ffff88003d4f0c00
[ 58.563845] FS: 0000000000000000(0000) GS:ffff88003fc00000(0000) knlGS:0000000000000000
[ 58.564873] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 58.565613] CR2: 0000000000000008 CR3: 000000003d68f004 CR4: 00000000000606f0
[ 58.566538] Call Trace:
[ 58.566865] <IRQ>
[ 58.567140] __tcp_retransmit_skb+0x4ab/0x4c6
[ 58.567704] ? tcp_set_ca_state+0x22/0x3f
[ 58.568231] tcp_retransmit_skb+0x14/0xa3
[ 58.568754] tcp_retransmit_timer+0x472/0x5e3
[ 58.569324] ? tcp_write_timer_handler+0x1e9/0x1e9
[ 58.569946] tcp_write_timer_handler+0x95/0x1e9
[ 58.570548] tcp_write_timer+0x2a/0x58
Check that daddr_cache is non-NULL before de-referencing.
Fixes: e086101b150a ("tcp: add a tracepoint for tcp retransmission")
Signed-off-by: David Ahern <dsahern@gmail.com>
---
include/trace/events/tcp.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/trace/events/tcp.h b/include/trace/events/tcp.h
index 3d1cbd072b7e..13e8ee8af2c8 100644
--- a/include/trace/events/tcp.h
+++ b/include/trace/events/tcp.h
@@ -48,7 +48,8 @@ TRACE_EVENT(tcp_retransmit_skb,
pin6 = (struct in6_addr *)__entry->saddr_v6;
*pin6 = np->saddr;
pin6 = (struct in6_addr *)__entry->daddr_v6;
- *pin6 = *(np->daddr_cache);
+ if (np->daddr_cache)
+ *pin6 = *(np->daddr_cache);
} else {
pin6 = (struct in6_addr *)__entry->saddr_v6;
ipv6_addr_set_v4mapped(inet->inet_saddr, pin6);
--
2.1.4
^ permalink raw reply related
* Re: [PATCH net-next] tcp: Use pI6c in tcp tracepoint
From: Cong Wang @ 2017-10-16 21:41 UTC (permalink / raw)
To: David Ahern; +Cc: Linux Kernel Network Developers
In-Reply-To: <1508189042-19591-1-git-send-email-dsahern@gmail.com>
On Mon, Oct 16, 2017 at 2:24 PM, David Ahern <dsahern@gmail.com> wrote:
> The compact form for IPv6 addresses is more user friendly than the full
> version. For example:
> compact: 2001:db8:1::1
> full: 2001:0db8:0001:0000:0000:0000:0000:0004i
>
> Update the tcp tracepoint to show the compact form.
>
> Signed-off-by: David Ahern <dsahern@gmail.com>
Acked-by: Cong Wang <xiyou.wangcong@gmail.com>
I was planning to send a same patch based on your
suggestion, you are faster than me. ;)
^ permalink raw reply
* [net-next:master 277/285] include/trace/events/xdp.h:91:17: warning: 'struct bpf_map' declared inside parameter list
From: kbuild test robot @ 2017-10-16 21:42 UTC (permalink / raw)
To: Steven Rostedt (VMware); +Cc: kbuild-all, netdev
[-- Attachment #1: Type: text/plain, Size: 18116 bytes --]
tree: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git master
head: e4467f2e2442e15cf047c2e7b91cb44a643cb149
commit: 9185a610f8f7f1b4e4d28c9de27d1969cf58e0f1 [277/285] tracing: bpf: Hide bpf trace events when they are not used
config: x86_64-randconfig-i0-201742 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
git checkout 9185a610f8f7f1b4e4d28c9de27d1969cf58e0f1
# save the attached .config to linux build tree
make ARCH=x86_64
All warnings (new ones prefixed by >>):
In file included from include/trace/events/xdp.h:9:0,
from include/linux/bpf_trace.h:5,
from drivers/net//ethernet/qlogic/qede/qede_fp.c:35:
>> include/trace/events/xdp.h:91:17: warning: 'struct bpf_map' declared inside parameter list
const struct bpf_map *map, u32 map_index),
^
include/linux/tracepoint.h:256:34: note: in definition of macro '__DECLARE_TRACE'
static inline void trace_##name(proto) \
^
include/linux/tracepoint.h:352:24: note: in expansion of macro 'PARAMS'
__DECLARE_TRACE(name, PARAMS(proto), PARAMS(args), \
^
include/linux/tracepoint.h:477:2: note: in expansion of macro 'DECLARE_TRACE'
DECLARE_TRACE(name, PARAMS(proto), PARAMS(args))
^
include/linux/tracepoint.h:477:22: note: in expansion of macro 'PARAMS'
DECLARE_TRACE(name, PARAMS(proto), PARAMS(args))
^
include/trace/events/xdp.h:87:1: note: in expansion of macro 'DEFINE_EVENT'
DEFINE_EVENT(xdp_redirect_template, xdp_redirect,
^
include/trace/events/xdp.h:88:2: note: in expansion of macro 'TP_PROTO'
TP_PROTO(const struct net_device *dev,
^
>> include/trace/events/xdp.h:91:17: warning: its scope is only this definition or declaration, which is probably not what you want
const struct bpf_map *map, u32 map_index),
^
include/linux/tracepoint.h:256:34: note: in definition of macro '__DECLARE_TRACE'
static inline void trace_##name(proto) \
^
include/linux/tracepoint.h:352:24: note: in expansion of macro 'PARAMS'
__DECLARE_TRACE(name, PARAMS(proto), PARAMS(args), \
^
include/linux/tracepoint.h:477:2: note: in expansion of macro 'DECLARE_TRACE'
DECLARE_TRACE(name, PARAMS(proto), PARAMS(args))
^
include/linux/tracepoint.h:477:22: note: in expansion of macro 'PARAMS'
DECLARE_TRACE(name, PARAMS(proto), PARAMS(args))
^
include/trace/events/xdp.h:87:1: note: in expansion of macro 'DEFINE_EVENT'
DEFINE_EVENT(xdp_redirect_template, xdp_redirect,
^
include/trace/events/xdp.h:88:2: note: in expansion of macro 'TP_PROTO'
TP_PROTO(const struct net_device *dev,
^
>> include/trace/events/xdp.h:91:17: warning: 'struct bpf_map' declared inside parameter list
const struct bpf_map *map, u32 map_index),
^
include/linux/tracepoint.h:258:44: note: in definition of macro '__DECLARE_TRACE'
static inline void trace_##name##_rcuidle(proto) \
^
include/linux/tracepoint.h:352:24: note: in expansion of macro 'PARAMS'
__DECLARE_TRACE(name, PARAMS(proto), PARAMS(args), \
^
include/linux/tracepoint.h:477:2: note: in expansion of macro 'DECLARE_TRACE'
DECLARE_TRACE(name, PARAMS(proto), PARAMS(args))
^
include/linux/tracepoint.h:477:22: note: in expansion of macro 'PARAMS'
DECLARE_TRACE(name, PARAMS(proto), PARAMS(args))
^
include/trace/events/xdp.h:87:1: note: in expansion of macro 'DEFINE_EVENT'
DEFINE_EVENT(xdp_redirect_template, xdp_redirect,
^
include/trace/events/xdp.h:88:2: note: in expansion of macro 'TP_PROTO'
TP_PROTO(const struct net_device *dev,
^
>> include/trace/events/xdp.h:91:17: warning: 'struct bpf_map' declared inside parameter list
const struct bpf_map *map, u32 map_index),
^
include/linux/tracepoint.h:261:38: note: in definition of macro '__DECLARE_TRACE'
register_trace_##name(void (*probe)(data_proto), \
^
include/linux/tracepoint.h:354:4: note: in expansion of macro 'PARAMS'
PARAMS(void *__data, proto), \
^
include/linux/tracepoint.h:477:2: note: in expansion of macro 'DECLARE_TRACE'
DECLARE_TRACE(name, PARAMS(proto), PARAMS(args))
^
include/linux/tracepoint.h:477:22: note: in expansion of macro 'PARAMS'
DECLARE_TRACE(name, PARAMS(proto), PARAMS(args))
^
include/trace/events/xdp.h:87:1: note: in expansion of macro 'DEFINE_EVENT'
DEFINE_EVENT(xdp_redirect_template, xdp_redirect,
^
include/trace/events/xdp.h:88:2: note: in expansion of macro 'TP_PROTO'
TP_PROTO(const struct net_device *dev,
^
>> include/trace/events/xdp.h:91:17: warning: 'struct bpf_map' declared inside parameter list
const struct bpf_map *map, u32 map_index),
^
include/linux/tracepoint.h:267:40: note: in definition of macro '__DECLARE_TRACE'
unregister_trace_##name(void (*probe)(data_proto), \
^
include/linux/tracepoint.h:354:4: note: in expansion of macro 'PARAMS'
PARAMS(void *__data, proto), \
^
include/linux/tracepoint.h:477:2: note: in expansion of macro 'DECLARE_TRACE'
DECLARE_TRACE(name, PARAMS(proto), PARAMS(args))
^
include/linux/tracepoint.h:477:22: note: in expansion of macro 'PARAMS'
DECLARE_TRACE(name, PARAMS(proto), PARAMS(args))
^
include/trace/events/xdp.h:87:1: note: in expansion of macro 'DEFINE_EVENT'
DEFINE_EVENT(xdp_redirect_template, xdp_redirect,
^
include/trace/events/xdp.h:88:2: note: in expansion of macro 'TP_PROTO'
TP_PROTO(const struct net_device *dev,
^
>> include/trace/events/xdp.h:91:17: warning: 'struct bpf_map' declared inside parameter list
const struct bpf_map *map, u32 map_index),
^
include/linux/tracepoint.h:272:65: note: in definition of macro '__DECLARE_TRACE'
static inline void check_trace_callback_type_##name(void (*cb)(data_proto)) \
^
include/linux/tracepoint.h:354:4: note: in expansion of macro 'PARAMS'
PARAMS(void *__data, proto), \
^
include/linux/tracepoint.h:477:2: note: in expansion of macro 'DECLARE_TRACE'
DECLARE_TRACE(name, PARAMS(proto), PARAMS(args))
^
include/linux/tracepoint.h:477:22: note: in expansion of macro 'PARAMS'
DECLARE_TRACE(name, PARAMS(proto), PARAMS(args))
^
include/trace/events/xdp.h:87:1: note: in expansion of macro 'DEFINE_EVENT'
DEFINE_EVENT(xdp_redirect_template, xdp_redirect,
^
include/trace/events/xdp.h:88:2: note: in expansion of macro 'TP_PROTO'
TP_PROTO(const struct net_device *dev,
^
include/trace/events/xdp.h:99:17: warning: 'struct bpf_map' declared inside parameter list
const struct bpf_map *map, u32 map_index),
^
include/linux/tracepoint.h:256:34: note: in definition of macro '__DECLARE_TRACE'
static inline void trace_##name(proto) \
^
include/linux/tracepoint.h:352:24: note: in expansion of macro 'PARAMS'
__DECLARE_TRACE(name, PARAMS(proto), PARAMS(args), \
^
include/linux/tracepoint.h:477:2: note: in expansion of macro 'DECLARE_TRACE'
DECLARE_TRACE(name, PARAMS(proto), PARAMS(args))
^
include/linux/tracepoint.h:477:22: note: in expansion of macro 'PARAMS'
DECLARE_TRACE(name, PARAMS(proto), PARAMS(args))
^
include/trace/events/xdp.h:95:1: note: in expansion of macro 'DEFINE_EVENT'
DEFINE_EVENT(xdp_redirect_template, xdp_redirect_err,
^
include/trace/events/xdp.h:96:2: note: in expansion of macro 'TP_PROTO'
TP_PROTO(const struct net_device *dev,
^
include/trace/events/xdp.h:99:17: warning: 'struct bpf_map' declared inside parameter list
const struct bpf_map *map, u32 map_index),
^
include/linux/tracepoint.h:258:44: note: in definition of macro '__DECLARE_TRACE'
static inline void trace_##name##_rcuidle(proto) \
^
include/linux/tracepoint.h:352:24: note: in expansion of macro 'PARAMS'
__DECLARE_TRACE(name, PARAMS(proto), PARAMS(args), \
^
include/linux/tracepoint.h:477:2: note: in expansion of macro 'DECLARE_TRACE'
DECLARE_TRACE(name, PARAMS(proto), PARAMS(args))
^
include/linux/tracepoint.h:477:22: note: in expansion of macro 'PARAMS'
DECLARE_TRACE(name, PARAMS(proto), PARAMS(args))
^
include/trace/events/xdp.h:95:1: note: in expansion of macro 'DEFINE_EVENT'
DEFINE_EVENT(xdp_redirect_template, xdp_redirect_err,
^
include/trace/events/xdp.h:96:2: note: in expansion of macro 'TP_PROTO'
TP_PROTO(const struct net_device *dev,
^
include/trace/events/xdp.h:99:17: warning: 'struct bpf_map' declared inside parameter list
const struct bpf_map *map, u32 map_index),
^
include/linux/tracepoint.h:261:38: note: in definition of macro '__DECLARE_TRACE'
register_trace_##name(void (*probe)(data_proto), \
^
include/linux/tracepoint.h:354:4: note: in expansion of macro 'PARAMS'
PARAMS(void *__data, proto), \
^
include/linux/tracepoint.h:477:2: note: in expansion of macro 'DECLARE_TRACE'
DECLARE_TRACE(name, PARAMS(proto), PARAMS(args))
^
include/linux/tracepoint.h:477:22: note: in expansion of macro 'PARAMS'
DECLARE_TRACE(name, PARAMS(proto), PARAMS(args))
^
include/trace/events/xdp.h:95:1: note: in expansion of macro 'DEFINE_EVENT'
DEFINE_EVENT(xdp_redirect_template, xdp_redirect_err,
^
include/trace/events/xdp.h:96:2: note: in expansion of macro 'TP_PROTO'
TP_PROTO(const struct net_device *dev,
^
include/trace/events/xdp.h:99:17: warning: 'struct bpf_map' declared inside parameter list
const struct bpf_map *map, u32 map_index),
^
include/linux/tracepoint.h:267:40: note: in definition of macro '__DECLARE_TRACE'
unregister_trace_##name(void (*probe)(data_proto), \
^
include/linux/tracepoint.h:354:4: note: in expansion of macro 'PARAMS'
PARAMS(void *__data, proto), \
^
include/linux/tracepoint.h:477:2: note: in expansion of macro 'DECLARE_TRACE'
DECLARE_TRACE(name, PARAMS(proto), PARAMS(args))
^
include/linux/tracepoint.h:477:22: note: in expansion of macro 'PARAMS'
DECLARE_TRACE(name, PARAMS(proto), PARAMS(args))
^
include/trace/events/xdp.h:95:1: note: in expansion of macro 'DEFINE_EVENT'
DEFINE_EVENT(xdp_redirect_template, xdp_redirect_err,
vim +91 include/trace/events/xdp.h
a67edbf4f Daniel Borkmann 2017-01-25 6
a67edbf4f Daniel Borkmann 2017-01-25 7 #include <linux/netdevice.h>
a67edbf4f Daniel Borkmann 2017-01-25 8 #include <linux/filter.h>
a67edbf4f Daniel Borkmann 2017-01-25 @9 #include <linux/tracepoint.h>
a67edbf4f Daniel Borkmann 2017-01-25 10
a67edbf4f Daniel Borkmann 2017-01-25 11 #define __XDP_ACT_MAP(FN) \
a67edbf4f Daniel Borkmann 2017-01-25 12 FN(ABORTED) \
a67edbf4f Daniel Borkmann 2017-01-25 13 FN(DROP) \
a67edbf4f Daniel Borkmann 2017-01-25 14 FN(PASS) \
5acaee0a8 John Fastabend 2017-07-17 15 FN(TX) \
5acaee0a8 John Fastabend 2017-07-17 16 FN(REDIRECT)
a67edbf4f Daniel Borkmann 2017-01-25 17
a67edbf4f Daniel Borkmann 2017-01-25 18 #define __XDP_ACT_TP_FN(x) \
a67edbf4f Daniel Borkmann 2017-01-25 19 TRACE_DEFINE_ENUM(XDP_##x);
a67edbf4f Daniel Borkmann 2017-01-25 20 #define __XDP_ACT_SYM_FN(x) \
a67edbf4f Daniel Borkmann 2017-01-25 21 { XDP_##x, #x },
a67edbf4f Daniel Borkmann 2017-01-25 22 #define __XDP_ACT_SYM_TAB \
a67edbf4f Daniel Borkmann 2017-01-25 23 __XDP_ACT_MAP(__XDP_ACT_SYM_FN) { -1, 0 }
a67edbf4f Daniel Borkmann 2017-01-25 24 __XDP_ACT_MAP(__XDP_ACT_TP_FN)
a67edbf4f Daniel Borkmann 2017-01-25 25
a67edbf4f Daniel Borkmann 2017-01-25 26 TRACE_EVENT(xdp_exception,
a67edbf4f Daniel Borkmann 2017-01-25 27
a67edbf4f Daniel Borkmann 2017-01-25 28 TP_PROTO(const struct net_device *dev,
a67edbf4f Daniel Borkmann 2017-01-25 29 const struct bpf_prog *xdp, u32 act),
a67edbf4f Daniel Borkmann 2017-01-25 30
a67edbf4f Daniel Borkmann 2017-01-25 31 TP_ARGS(dev, xdp, act),
a67edbf4f Daniel Borkmann 2017-01-25 32
a67edbf4f Daniel Borkmann 2017-01-25 33 TP_STRUCT__entry(
b06337dfd Jesper Dangaard Brouer 2017-08-29 34 __field(int, prog_id)
a67edbf4f Daniel Borkmann 2017-01-25 35 __field(u32, act)
315ec3990 Jesper Dangaard Brouer 2017-08-24 36 __field(int, ifindex)
a67edbf4f Daniel Borkmann 2017-01-25 37 ),
a67edbf4f Daniel Borkmann 2017-01-25 38
a67edbf4f Daniel Borkmann 2017-01-25 39 TP_fast_assign(
b06337dfd Jesper Dangaard Brouer 2017-08-29 40 __entry->prog_id = xdp->aux->id;
a67edbf4f Daniel Borkmann 2017-01-25 41 __entry->act = act;
315ec3990 Jesper Dangaard Brouer 2017-08-24 42 __entry->ifindex = dev->ifindex;
a67edbf4f Daniel Borkmann 2017-01-25 43 ),
a67edbf4f Daniel Borkmann 2017-01-25 44
b06337dfd Jesper Dangaard Brouer 2017-08-29 45 TP_printk("prog_id=%d action=%s ifindex=%d",
b06337dfd Jesper Dangaard Brouer 2017-08-29 46 __entry->prog_id,
315ec3990 Jesper Dangaard Brouer 2017-08-24 47 __print_symbolic(__entry->act, __XDP_ACT_SYM_TAB),
315ec3990 Jesper Dangaard Brouer 2017-08-24 48 __entry->ifindex)
a67edbf4f Daniel Borkmann 2017-01-25 49 );
a67edbf4f Daniel Borkmann 2017-01-25 50
8d3b778ff Jesper Dangaard Brouer 2017-08-29 51 DECLARE_EVENT_CLASS(xdp_redirect_template,
5acaee0a8 John Fastabend 2017-07-17 52
a87358558 Jesper Dangaard Brouer 2017-08-24 53 TP_PROTO(const struct net_device *dev,
c31e5a487 Jesper Dangaard Brouer 2017-08-29 54 const struct bpf_prog *xdp,
8d3b778ff Jesper Dangaard Brouer 2017-08-29 55 int to_ifindex, int err,
8d3b778ff Jesper Dangaard Brouer 2017-08-29 56 const struct bpf_map *map, u32 map_index),
5acaee0a8 John Fastabend 2017-07-17 57
8d3b778ff Jesper Dangaard Brouer 2017-08-29 58 TP_ARGS(dev, xdp, to_ifindex, err, map, map_index),
5acaee0a8 John Fastabend 2017-07-17 59
5acaee0a8 John Fastabend 2017-07-17 60 TP_STRUCT__entry(
b06337dfd Jesper Dangaard Brouer 2017-08-29 61 __field(int, prog_id)
5acaee0a8 John Fastabend 2017-07-17 62 __field(u32, act)
a87358558 Jesper Dangaard Brouer 2017-08-24 63 __field(int, ifindex)
4c03bdd7b Jesper Dangaard Brouer 2017-08-17 64 __field(int, err)
8d3b778ff Jesper Dangaard Brouer 2017-08-29 65 __field(int, to_ifindex)
8d3b778ff Jesper Dangaard Brouer 2017-08-29 66 __field(u32, map_id)
8d3b778ff Jesper Dangaard Brouer 2017-08-29 67 __field(int, map_index)
5acaee0a8 John Fastabend 2017-07-17 68 ),
5acaee0a8 John Fastabend 2017-07-17 69
5acaee0a8 John Fastabend 2017-07-17 70 TP_fast_assign(
b06337dfd Jesper Dangaard Brouer 2017-08-29 71 __entry->prog_id = xdp->aux->id;
c31e5a487 Jesper Dangaard Brouer 2017-08-29 72 __entry->act = XDP_REDIRECT;
a87358558 Jesper Dangaard Brouer 2017-08-24 73 __entry->ifindex = dev->ifindex;
4c03bdd7b Jesper Dangaard Brouer 2017-08-17 74 __entry->err = err;
8d3b778ff Jesper Dangaard Brouer 2017-08-29 75 __entry->to_ifindex = to_ifindex;
8d3b778ff Jesper Dangaard Brouer 2017-08-29 76 __entry->map_id = map ? map->id : 0;
8d3b778ff Jesper Dangaard Brouer 2017-08-29 77 __entry->map_index = map_index;
5acaee0a8 John Fastabend 2017-07-17 78 ),
5acaee0a8 John Fastabend 2017-07-17 79
59a308967 Jesper Dangaard Brouer 2017-08-29 80 TP_printk("prog_id=%d action=%s ifindex=%d to_ifindex=%d err=%d",
b06337dfd Jesper Dangaard Brouer 2017-08-29 81 __entry->prog_id,
4c03bdd7b Jesper Dangaard Brouer 2017-08-17 82 __print_symbolic(__entry->act, __XDP_ACT_SYM_TAB),
8d3b778ff Jesper Dangaard Brouer 2017-08-29 83 __entry->ifindex, __entry->to_ifindex,
59a308967 Jesper Dangaard Brouer 2017-08-29 84 __entry->err)
5acaee0a8 John Fastabend 2017-07-17 85 );
8d3b778ff Jesper Dangaard Brouer 2017-08-29 86
8d3b778ff Jesper Dangaard Brouer 2017-08-29 @87 DEFINE_EVENT(xdp_redirect_template, xdp_redirect,
8d3b778ff Jesper Dangaard Brouer 2017-08-29 88 TP_PROTO(const struct net_device *dev,
8d3b778ff Jesper Dangaard Brouer 2017-08-29 89 const struct bpf_prog *xdp,
8d3b778ff Jesper Dangaard Brouer 2017-08-29 90 int to_ifindex, int err,
8d3b778ff Jesper Dangaard Brouer 2017-08-29 @91 const struct bpf_map *map, u32 map_index),
8d3b778ff Jesper Dangaard Brouer 2017-08-29 92 TP_ARGS(dev, xdp, to_ifindex, err, map, map_index)
8d3b778ff Jesper Dangaard Brouer 2017-08-29 93 );
8d3b778ff Jesper Dangaard Brouer 2017-08-29 94
:::::: The code at line 91 was first introduced by commit
:::::: 8d3b778ff544b369f0847e6c15f3e73057298aa4 xdp: tracepoint xdp_redirect also need a map argument
:::::: TO: Jesper Dangaard Brouer <brouer@redhat.com>
:::::: CC: David S. Miller <davem@davemloft.net>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 32470 bytes --]
^ permalink raw reply
* Re: [PATCH net-next] tcp: Use pI6c in tcp tracepoint
From: David Ahern @ 2017-10-16 21:45 UTC (permalink / raw)
To: Cong Wang; +Cc: Linux Kernel Network Developers
In-Reply-To: <CAM_iQpV5=uSzF3zBDPjrg9uhpLGk0tk07p0RLJgEvgOnSkSjDw@mail.gmail.com>
On 10/16/17 3:41 PM, Cong Wang wrote:
> I was planning to send a same patch based on your
> suggestion, you are faster than me. ;)
>
I would have sent that patch hours ago, but got hung up on the testing.
See the second patch.
^ permalink raw reply
* Re: [PATCH net-next] tcp: Check daddr_cache before use in tracepoint
From: Cong Wang @ 2017-10-16 21:46 UTC (permalink / raw)
To: David Ahern; +Cc: Linux Kernel Network Developers
In-Reply-To: <1508189387-27931-1-git-send-email-dsahern@gmail.com>
On Mon, Oct 16, 2017 at 2:29 PM, David Ahern <dsahern@gmail.com> wrote:
> Running perf in one window to capture tcp_retransmit_skb tracepoint:
> $ perf record -e tcp:tcp_retransmit_skb -a
>
> And causing a retransmission on an active TCP session (e.g., dropping
> packets in the receiver, changing MTU on the interface to 500 and back
> to 1500) triggers a panic:
>
> [ 58.543144] BUG: unable to handle kernel NULL pointer dereference at 0000000000000008
> [ 58.545300] IP: perf_trace_tcp_retransmit_skb+0xd0/0x145
> [ 58.546770] PGD 0 P4D 0
> [ 58.547472] Oops: 0000 [#1] SMP
> [ 58.548328] Modules linked in: vrf
> [ 58.549262] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.14.0-rc4+ #26
> [ 58.551004] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.7.5-20140531_083030-gandalf 04/01/2014
> [ 58.554560] task: ffffffff81a0e540 task.stack: ffffffff81a00000
> [ 58.555817] RIP: 0010:perf_trace_tcp_retransmit_skb+0xd0/0x145
> [ 58.557137] RSP: 0018:ffff88003fc03d68 EFLAGS: 00010282
> [ 58.558292] RAX: 0000000000000000 RBX: ffffe8ffffc0ec80 RCX: ffff880038543098
> [ 58.559850] RDX: 0400000000000000 RSI: ffff88003fc03d70 RDI: ffff88003fc14b68
> [ 58.561099] RBP: ffff88003fc03da8 R08: 0000000000000000 R09: ffffea0000d3224a
> [ 58.562005] R10: ffff88003fc03db8 R11: 0000000000000010 R12: ffff8800385428c0
> [ 58.562930] R13: ffffe8ffffc0e478 R14: ffffffff81a93a40 R15: ffff88003d4f0c00
> [ 58.563845] FS: 0000000000000000(0000) GS:ffff88003fc00000(0000) knlGS:0000000000000000
> [ 58.564873] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 58.565613] CR2: 0000000000000008 CR3: 000000003d68f004 CR4: 00000000000606f0
> [ 58.566538] Call Trace:
> [ 58.566865] <IRQ>
> [ 58.567140] __tcp_retransmit_skb+0x4ab/0x4c6
> [ 58.567704] ? tcp_set_ca_state+0x22/0x3f
> [ 58.568231] tcp_retransmit_skb+0x14/0xa3
> [ 58.568754] tcp_retransmit_timer+0x472/0x5e3
> [ 58.569324] ? tcp_write_timer_handler+0x1e9/0x1e9
> [ 58.569946] tcp_write_timer_handler+0x95/0x1e9
> [ 58.570548] tcp_write_timer+0x2a/0x58
>
> Check that daddr_cache is non-NULL before de-referencing.
Well, for NULL case, the entry->saddr_v6 will not be filled with
your patch.
How about using sk->sk_v6_daddr?
^ permalink raw reply
* Re: [net-next V8 PATCH 1/5] bpf: introduce new bpf cpu map type BPF_MAP_TYPE_CPUMAP
From: Alexei Starovoitov @ 2017-10-16 21:49 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: netdev, jakub.kicinski, Michael S. Tsirkin, pavel.odintsov,
Jason Wang, mchan, John Fastabend, peter.waskiewicz.jr, ast,
Daniel Borkmann, Andy Gospodarek
In-Reply-To: <150814916887.1806.4443991765779135803.stgit@firesoul>
On Mon, Oct 16, 2017 at 12:19:28PM +0200, Jesper Dangaard Brouer wrote:
> The 'cpumap' is primarily used as a backend map for XDP BPF helper
> call bpf_redirect_map() and XDP_REDIRECT action, like 'devmap'.
>
> This patch implement the main part of the map. It is not connected to
> the XDP redirect system yet, and no SKB allocation are done yet.
>
> The main concern in this patch is to ensure the datapath can run
> without any locking. This adds complexity to the setup and tear-down
> procedure, which assumptions are extra carefully documented in the
> code comments.
>
> V2:
> - make sure array isn't larger than NR_CPUS
> - make sure CPUs added is a valid possible CPU
>
> V3: fix nitpicks from Jakub Kicinski <kubakici@wp.pl>
>
> V5:
> - Restrict map allocation to root / CAP_SYS_ADMIN
> - WARN_ON_ONCE if queue is not empty on tear-down
> - Return -EPERM on memlock limit instead of -ENOMEM
> - Error code in __cpu_map_entry_alloc() also handle ptr_ring_cleanup()
> - Moved cpu_map_enqueue() to next patch
>
> V6: all notice by Daniel Borkmann
> - Fix err return code in cpu_map_alloc() introduced in V5
> - Move cpu_possible() check after max_entries boundary check
> - Forbid usage initially in check_map_func_compatibility()
>
> V7:
> - Fix alloc error path spotted by Daniel Borkmann
> - Did stress test adding+removing CPUs from the map concurrently
> - Fixed refcnt issue on cpu_map_entry, kthread started too soon
> - Make sure packets are flushed during tear-down, involved use of
> rcu_barrier() and kthread_run only exit after queue is empty
> - Fix alloc error path in __cpu_map_entry_alloc() for ptr_ring
>
> V8:
> - Nitpicking comments and gramma by Edward Cree
> - Fix missing semi-colon introduced in V7 due to rebasing
> - Move struct bpf_cpu_map_entry members cpu+map_id to tracepoint patch
>
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
> ---
> include/linux/bpf_types.h | 1
> include/uapi/linux/bpf.h | 1
> kernel/bpf/Makefile | 1
> kernel/bpf/cpumap.c | 560 ++++++++++++++++++++++++++++++++++++++++
> kernel/bpf/syscall.c | 8 +
> kernel/bpf/verifier.c | 5
> tools/include/uapi/linux/bpf.h | 1
> 7 files changed, 576 insertions(+), 1 deletion(-)
> create mode 100644 kernel/bpf/cpumap.c
Looks good to me
I like the idea of running networking stack from kthread
and hope adding GRO won't change the api.
Acked-by: Alexei Starovoitov <ast@kernel.org>
^ permalink raw reply
* Re: [PATCH net-next] tools: bpftool: use more common tag format
From: Alexei Starovoitov @ 2017-10-16 21:52 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: netdev, oss-drivers, daniel
In-Reply-To: <20171016171254.26225-1-jakub.kicinski@netronome.com>
On Mon, Oct 16, 2017 at 10:12:54AM -0700, Jakub Kicinski wrote:
> Program tag is usually displayed as string of bytes without
> any separators (e.g. as "aa5520b1090cfeb6" vs MAC addr-like
> format bpftool uses currently: "aa:55:20:b1:09:0c:fe:b6").
> Make bptfool use the more common format both for displaying
> the tag and selecting the program by tag.
>
> This was pointed out in review but I misunderstood the comment.
>
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> Reviewed-by: Simon Horman <simon.horman@netronome.com>
thanks!
Acked-by: Alexei Starovoitov <ast@kernel.org>
^ permalink raw reply
* [PATCH] net: ipx: mark expected switch fall-through
From: Gustavo A. R. Silva @ 2017-10-16 21:53 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, linux-kernel, Gustavo A. R. Silva
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.
Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
---
This code was tested by compilation only (GCC 7.2.0 was used).
Please, verify if the actual intention of the code is to fall through.
net/ipx/af_ipx.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/ipx/af_ipx.c b/net/ipx/af_ipx.c
index ac598ec..d21a9d1 100644
--- a/net/ipx/af_ipx.c
+++ b/net/ipx/af_ipx.c
@@ -1867,6 +1867,7 @@ static int ipx_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
rc = -EPERM;
if (!capable(CAP_NET_ADMIN))
break;
+ /* fall through */
case SIOCGIFADDR:
rc = ipxitf_ioctl(cmd, argp);
break;
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net-next] tcp: Check daddr_cache before use in tracepoint
From: David Ahern @ 2017-10-16 21:55 UTC (permalink / raw)
To: Cong Wang; +Cc: Linux Kernel Network Developers
In-Reply-To: <CAM_iQpUgWJwjaRYw3vX4t20foheOrebU3U2ACZqrAbTYqYc-5A@mail.gmail.com>
On 10/16/17 3:46 PM, Cong Wang wrote:
> Well, for NULL case, the entry->saddr_v6 will not be filled with
> your patch.
I think you meant daddr_v6 and yes it will not get filled in, but 0 is
better than a panic ;-)
>
> How about using sk->sk_v6_daddr?
>
That works, but then both addresses should come from sk and not np:
diff --git a/include/trace/events/tcp.h b/include/trace/events/tcp.h
index 1ffab6d96e94..7989c2dcedf1 100644
--- a/include/trace/events/tcp.h
+++ b/include/trace/events/tcp.h
@@ -46,9 +46,9 @@ TRACE_EVENT(tcp_retransmit_skb,
if (np) {
pin6 = (struct in6_addr *)__entry->saddr_v6;
- *pin6 = np->saddr;
+ *pin6 = sk->sk_v6_rcv_saddr;
pin6 = (struct in6_addr *)__entry->daddr_v6;
- *pin6 = *(np->daddr_cache);
+ *pin6 = sk->sk_v6_daddr;
} else {
pin6 = (struct in6_addr *)__entry->saddr_v6;
ipv6_addr_set_v4mapped(inet->inet_saddr, pin6);
So np is just a trigger for IPv6 versus v4 mapped.
I do have a question about why ipv4 addresses are non-NULL:
$ perf script
swapper 0 [000] 25.025102: tcp:tcp_retransmit_skb:
sport=22 dport=48076 saddr=127.0.0.6 daddr=127.0.0.6
saddrv6=2001:db8:1::4 daddrv6=2001:db8:1::64
swapper 0 [000] 25.231125: tcp:tcp_retransmit_skb:
sport=22 dport=48076 saddr=127.0.0.6 daddr=127.0.0.6
saddrv6=2001:db8:1::4 daddrv6=2001:db8:1::64
^ permalink raw reply related
* Re: [PATCH net v2] bpf: disallow arithmetic operations on context pointer
From: Daniel Borkmann @ 2017-10-16 21:56 UTC (permalink / raw)
To: Jakub Kicinski, netdev; +Cc: oss-drivers, alexei.starovoitov, ecree
In-Reply-To: <20171016181655.16366-1-jakub.kicinski@netronome.com>
On 10/16/2017 08:16 PM, Jakub Kicinski wrote:
> Commit f1174f77b50c ("bpf/verifier: rework value tracking")
> removed the crafty selection of which pointer types are
> allowed to be modified. This is OK for most pointer types
> since adjust_ptr_min_max_vals() will catch operations on
> immutable pointers. One exception is PTR_TO_CTX which is
> now allowed to be offseted freely.
>
> The intent of aforementioned commit was to allow context
> access via modified registers. The offset passed to
> ->is_valid_access() verifier callback has been adjusted
> by the value of the variable offset.
>
> What is missing, however, is taking the variable offset
> into account when the context register is used. Or in terms
> of the code adding the offset to the value passed to the
> ->convert_ctx_access() callback. This leads to the following
> eBPF user code:
>
> r1 += 68
> r0 = *(u32 *)(r1 + 8)
> exit
>
> being translated to this in kernel space:
>
> 0: (07) r1 += 68
> 1: (61) r0 = *(u32 *)(r1 +180)
> 2: (95) exit
>
> Offset 8 is corresponding to 180 in the kernel, but offset
> 76 is valid too. Verifier will "accept" access to offset
> 68+8=76 but then "convert" access to offset 8 as 180.
> Effective access to offset 248 is beyond the kernel context.
> (This is a __sk_buff example on a debug-heavy kernel -
> packet mark is 8 -> 180, 76 would be data.)
>
> Dereferencing the modified context pointer is not as easy
> as dereferencing other types, because we have to translate
> the access to reading a field in kernel structures which is
> usually at a different offset and often of a different size.
> To allow modifying the pointer we would have to make sure
> that given eBPF instruction will always access the same
> field or the fields accessed are "compatible" in terms of
> offset and size...
>
> Disallow dereferencing modified context pointers and add
> to selftests the test case described here.
>
> Fixes: f1174f77b50c ("bpf/verifier: rework value tracking")
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Thanks for the fix!
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
^ permalink raw reply
* [PATCH] ipv6: mark expected switch fall-throughs
From: Gustavo A. R. Silva @ 2017-10-16 21:36 UTC (permalink / raw)
To: Steffen Klassert, Herbert Xu, David S. Miller, Alexey Kuznetsov,
Hideaki YOSHIFUJI, Pablo Neira Ayuso, Jozsef Kadlecsik,
Florian Westphal
Cc: netdev, linux-kernel, netfilter-devel, coreteam,
Gustavo A. R. Silva
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.
Notice that in some cases I placed the "fall through" comment
on its own line, which is what GCC is expecting to find.
Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
---
This code was tested by compilation only (GCC 7.2.0 was used).
Please, verify if the actual intention of the code is to fall through.
net/ipv6/ah6.c | 1 +
net/ipv6/exthdrs.c | 1 +
net/ipv6/icmp.c | 6 ++----
net/ipv6/ip6_fib.c | 4 ++++
net/ipv6/ip6_tunnel.c | 1 +
net/ipv6/ip6mr.c | 1 +
net/ipv6/netfilter/nf_nat_l3proto_ipv6.c | 3 ++-
net/ipv6/raw.c | 4 ++++
net/ipv6/tcp_ipv6.c | 3 ++-
net/ipv6/xfrm6_policy.c | 1 +
10 files changed, 19 insertions(+), 6 deletions(-)
diff --git a/net/ipv6/ah6.c b/net/ipv6/ah6.c
index 7802b72..37bb33f 100644
--- a/net/ipv6/ah6.c
+++ b/net/ipv6/ah6.c
@@ -271,6 +271,7 @@ static int ipv6_clear_mutable_options(struct ipv6hdr *iph, int len, int dir)
case NEXTHDR_DEST:
if (dir == XFRM_POLICY_OUT)
ipv6_rearrange_destopt(iph, exthdr.opth);
+ /* fall through */
case NEXTHDR_HOP:
if (!zero_out_mutable_opts(exthdr.opth)) {
net_dbg_ratelimited("overrun %sopts\n",
diff --git a/net/ipv6/exthdrs.c b/net/ipv6/exthdrs.c
index 9551613..7835dea 100644
--- a/net/ipv6/exthdrs.c
+++ b/net/ipv6/exthdrs.c
@@ -89,6 +89,7 @@ static bool ip6_tlvopt_unknown(struct sk_buff *skb, int optoff)
*/
if (ipv6_addr_is_multicast(&ipv6_hdr(skb)->daddr))
break;
+ /* fall through */
case 2: /* send ICMP PARM PROB regardless and drop packet */
icmpv6_param_prob(skb, ICMPV6_UNK_OPTION, optoff);
return false;
diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
index 4e52d52..6ae5dd3 100644
--- a/net/ipv6/icmp.c
+++ b/net/ipv6/icmp.c
@@ -864,10 +864,8 @@ static int icmpv6_rcv(struct sk_buff *skb)
goto discard_it;
hdr = icmp6_hdr(skb);
- /*
- * Drop through to notify
- */
-
+ /* to notify */
+ /* fall through */
case ICMPV6_DEST_UNREACH:
case ICMPV6_TIME_EXCEED:
case ICMPV6_PARAMPROB:
diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
index c2ecd5e..dcb0649 100644
--- a/net/ipv6/ip6_fib.c
+++ b/net/ipv6/ip6_fib.c
@@ -1777,6 +1777,7 @@ static int fib6_walk_continue(struct fib6_walker *w)
}
w->state = FWS_L;
#endif
+ /* fall through */
case FWS_L:
left = rcu_dereference_protected(fn->left, 1);
if (left) {
@@ -1785,6 +1786,7 @@ static int fib6_walk_continue(struct fib6_walker *w)
continue;
}
w->state = FWS_R;
+ /* fall through */
case FWS_R:
right = rcu_dereference_protected(fn->right, 1);
if (right) {
@@ -1794,6 +1796,7 @@ static int fib6_walk_continue(struct fib6_walker *w)
}
w->state = FWS_C;
w->leaf = rcu_dereference_protected(fn->leaf, 1);
+ /* fall through */
case FWS_C:
if (w->leaf && fn->fn_flags & RTN_RTINFO) {
int err;
@@ -1812,6 +1815,7 @@ static int fib6_walk_continue(struct fib6_walker *w)
}
skip:
w->state = FWS_U;
+ /* fall through */
case FWS_U:
if (fn == w->root)
return 0;
diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
index 8255486..4212879 100644
--- a/net/ipv6/ip6_tunnel.c
+++ b/net/ipv6/ip6_tunnel.c
@@ -593,6 +593,7 @@ ip4ip6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
case NDISC_REDIRECT:
rel_type = ICMP_REDIRECT;
rel_code = ICMP_REDIR_HOST;
+ /* fall through */
default:
return 0;
}
diff --git a/net/ipv6/ip6mr.c b/net/ipv6/ip6mr.c
index f5500f5..59fad81 100644
--- a/net/ipv6/ip6mr.c
+++ b/net/ipv6/ip6mr.c
@@ -1722,6 +1722,7 @@ int ip6_mroute_setsockopt(struct sock *sk, int optname, char __user *optval, uns
case MRT6_ADD_MFC:
case MRT6_DEL_MFC:
parent = -1;
+ /* fall through */
case MRT6_ADD_MFC_PROXY:
case MRT6_DEL_MFC_PROXY:
if (optlen < sizeof(mfc))
diff --git a/net/ipv6/netfilter/nf_nat_l3proto_ipv6.c b/net/ipv6/netfilter/nf_nat_l3proto_ipv6.c
index 46d6dba..1d2fb92 100644
--- a/net/ipv6/netfilter/nf_nat_l3proto_ipv6.c
+++ b/net/ipv6/netfilter/nf_nat_l3proto_ipv6.c
@@ -290,7 +290,8 @@ nf_nat_ipv6_fn(void *priv, struct sk_buff *skb,
else
return NF_ACCEPT;
}
- /* Fall thru... (Only ICMPs can be IP_CT_IS_REPLY) */
+ /* Only ICMPs can be IP_CT_IS_REPLY: */
+ /* fall through */
case IP_CT_NEW:
/* Seen it before? This can happen for loopback, retrans,
* or local packets.
diff --git a/net/ipv6/raw.c b/net/ipv6/raw.c
index 041d1cd..08a85fa 100644
--- a/net/ipv6/raw.c
+++ b/net/ipv6/raw.c
@@ -1055,6 +1055,7 @@ static int rawv6_setsockopt(struct sock *sk, int level, int optname,
if (optname == IPV6_CHECKSUM ||
optname == IPV6_HDRINCL)
break;
+ /* fall through */
default:
return ipv6_setsockopt(sk, level, optname, optval, optlen);
}
@@ -1077,6 +1078,7 @@ static int compat_rawv6_setsockopt(struct sock *sk, int level, int optname,
if (optname == IPV6_CHECKSUM ||
optname == IPV6_HDRINCL)
break;
+ /* fall through */
default:
return compat_ipv6_setsockopt(sk, level, optname,
optval, optlen);
@@ -1138,6 +1140,7 @@ static int rawv6_getsockopt(struct sock *sk, int level, int optname,
if (optname == IPV6_CHECKSUM ||
optname == IPV6_HDRINCL)
break;
+ /* fall through */
default:
return ipv6_getsockopt(sk, level, optname, optval, optlen);
}
@@ -1160,6 +1163,7 @@ static int compat_rawv6_getsockopt(struct sock *sk, int level, int optname,
if (optname == IPV6_CHECKSUM ||
optname == IPV6_HDRINCL)
break;
+ /* fall through */
default:
return compat_ipv6_getsockopt(sk, level, optname,
optval, optlen);
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 64d94af..ae83615 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -1577,8 +1577,9 @@ static int tcp_v6_rcv(struct sk_buff *skb)
refcounted = false;
goto process;
}
- /* Fall through to ACK */
}
+ /* to ACK */
+ /* fall through */
case TCP_TW_ACK:
tcp_v6_timewait_ack(sk, skb);
break;
diff --git a/net/ipv6/xfrm6_policy.c b/net/ipv6/xfrm6_policy.c
index 11d1314..4ed9f8c 100644
--- a/net/ipv6/xfrm6_policy.c
+++ b/net/ipv6/xfrm6_policy.c
@@ -152,6 +152,7 @@ _decode_session6(struct sk_buff *skb, struct flowi *fl, int reverse)
switch (nexthdr) {
case NEXTHDR_FRAGMENT:
onlyproto = 1;
+ /* fall through */
case NEXTHDR_ROUTING:
case NEXTHDR_HOP:
case NEXTHDR_DEST:
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net v2] bpf: disallow arithmetic operations on context pointer
From: Alexei Starovoitov @ 2017-10-16 22:02 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: netdev, oss-drivers, daniel, ecree
In-Reply-To: <20171016181655.16366-1-jakub.kicinski@netronome.com>
On Mon, Oct 16, 2017 at 11:16:55AM -0700, Jakub Kicinski wrote:
> Commit f1174f77b50c ("bpf/verifier: rework value tracking")
> removed the crafty selection of which pointer types are
> allowed to be modified. This is OK for most pointer types
> since adjust_ptr_min_max_vals() will catch operations on
> immutable pointers. One exception is PTR_TO_CTX which is
> now allowed to be offseted freely.
>
> The intent of aforementioned commit was to allow context
> access via modified registers. The offset passed to
> ->is_valid_access() verifier callback has been adjusted
> by the value of the variable offset.
>
> What is missing, however, is taking the variable offset
> into account when the context register is used. Or in terms
> of the code adding the offset to the value passed to the
> ->convert_ctx_access() callback. This leads to the following
> eBPF user code:
>
> r1 += 68
> r0 = *(u32 *)(r1 + 8)
> exit
>
> being translated to this in kernel space:
>
> 0: (07) r1 += 68
> 1: (61) r0 = *(u32 *)(r1 +180)
> 2: (95) exit
>
> Offset 8 is corresponding to 180 in the kernel, but offset
> 76 is valid too. Verifier will "accept" access to offset
> 68+8=76 but then "convert" access to offset 8 as 180.
> Effective access to offset 248 is beyond the kernel context.
> (This is a __sk_buff example on a debug-heavy kernel -
> packet mark is 8 -> 180, 76 would be data.)
>
> Dereferencing the modified context pointer is not as easy
> as dereferencing other types, because we have to translate
> the access to reading a field in kernel structures which is
> usually at a different offset and often of a different size.
> To allow modifying the pointer we would have to make sure
> that given eBPF instruction will always access the same
> field or the fields accessed are "compatible" in terms of
> offset and size...
>
> Disallow dereferencing modified context pointers and add
> to selftests the test case described here.
>
> Fixes: f1174f77b50c ("bpf/verifier: rework value tracking")
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Acked-by: Alexei Starovoitov <ast@kernel.org>
that's a required fix for net and for net-next I like Ed's idea
to teach convert_ctx_access to recognize such complex pattern
and do fancier rewrite.
iirc we were hitting similar ctx rewrite issue from time to time
and managed to change .c and/or llvm codegen in the past
when compiler was producing code that technically was valid, but
didn't satisfy 'ctx+const' requirement.
Back then verifier wasn't that smart. Now we can do better.
^ permalink raw reply
* Re: [PATCH net-next] tcp: Check daddr_cache before use in tracepoint
From: Cong Wang @ 2017-10-16 22:05 UTC (permalink / raw)
To: David Ahern; +Cc: Linux Kernel Network Developers
In-Reply-To: <cfd6b39a-41a6-ccf6-80a7-07089a2d6fea@gmail.com>
On Mon, Oct 16, 2017 at 2:55 PM, David Ahern <dsahern@gmail.com> wrote:
> On 10/16/17 3:46 PM, Cong Wang wrote:
>> Well, for NULL case, the entry->saddr_v6 will not be filled with
>> your patch.
>
> I think you meant daddr_v6 and yes it will not get filled in, but 0 is
> better than a panic ;-)
>
Sure, but we can do even better with sk->sk_v6_daddr.
>>
>> How about using sk->sk_v6_daddr?
>>
>
> That works, but then both addresses should come from sk and not np:
Yes.
>
> diff --git a/include/trace/events/tcp.h b/include/trace/events/tcp.h
> index 1ffab6d96e94..7989c2dcedf1 100644
> --- a/include/trace/events/tcp.h
> +++ b/include/trace/events/tcp.h
> @@ -46,9 +46,9 @@ TRACE_EVENT(tcp_retransmit_skb,
>
> if (np) {
> pin6 = (struct in6_addr *)__entry->saddr_v6;
> - *pin6 = np->saddr;
> + *pin6 = sk->sk_v6_rcv_saddr;
> pin6 = (struct in6_addr *)__entry->daddr_v6;
> - *pin6 = *(np->daddr_cache);
> + *pin6 = sk->sk_v6_daddr;
> } else {
> pin6 = (struct in6_addr *)__entry->saddr_v6;
> ipv6_addr_set_v4mapped(inet->inet_saddr, pin6);
>
>
> So np is just a trigger for IPv6 versus v4 mapped.
>
> I do have a question about why ipv4 addresses are non-NULL:
If you really mean non-NULL, they are addresses, not pointers to
addresses.
Or if you mean non-zero, they are set to LOOPBACK4_IPV6
which is what I expect.
^ permalink raw reply
* Re: [PATCH 1/3] bpf: Don't check for current being NULL
From: Alexei Starovoitov @ 2017-10-16 22:06 UTC (permalink / raw)
To: Richard Weinberger
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
Daniel Borkmann, Alexei Starovoitov
On Mon, Oct 16, 2017 at 11:18 AM, Richard Weinberger <richard@nod.at> wrote:
> current is never NULL.
>
> Signed-off-by: Richard Weinberger <richard@nod.at>
> ---
> kernel/bpf/helpers.c | 12 ------------
> 1 file changed, 12 deletions(-)
>
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index 3d24e238221e..e8845adcd15e 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -120,9 +120,6 @@ BPF_CALL_0(bpf_get_current_pid_tgid)
> {
> struct task_struct *task = current;
>
> - if (unlikely(!task))
> - return -EINVAL;
> -
really? in all context? including irq and nmi?
^ permalink raw reply
* [PATCH] mac80211: aggregation: Convert timers to use timer_setup()
From: Kees Cook @ 2017-10-16 22:08 UTC (permalink / raw)
To: Johannes Berg; +Cc: David S. Miller, netdev
In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly.
This removes the tid mapping array and expands the tid structures to
add a pointer back to the station, along with the tid index itself.
Cc: Johannes Berg <johannes@sipsolutions.net>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: linux-wireless@vger.kernel.org
Cc: netdev@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
This has been the least trivial timer conversion yet. Given the use of
RCU and other things I may not even know about, I'd love to get a close
look at this. I *think* this is correct, as it will re-lookup the tid
entries when firing the timer.
---
net/mac80211/agg-rx.c | 41 +++++++++++++++++------------------------
net/mac80211/agg-tx.c | 42 ++++++++++++++++--------------------------
net/mac80211/sta_info.c | 8 --------
net/mac80211/sta_info.h | 12 ++++++++++--
4 files changed, 43 insertions(+), 60 deletions(-)
diff --git a/net/mac80211/agg-rx.c b/net/mac80211/agg-rx.c
index 88cc1ae935ea..63aba6dbc92a 100644
--- a/net/mac80211/agg-rx.c
+++ b/net/mac80211/agg-rx.c
@@ -151,21 +151,17 @@ EXPORT_SYMBOL(ieee80211_stop_rx_ba_session);
* After accepting the AddBA Request we activated a timer,
* resetting it after each frame that arrives from the originator.
*/
-static void sta_rx_agg_session_timer_expired(unsigned long data)
+static void sta_rx_agg_session_timer_expired(struct timer_list *t)
{
- /* not an elegant detour, but there is no choice as the timer passes
- * only one argument, and various sta_info are needed here, so init
- * flow in sta_info_create gives the TID as data, while the timer_to_id
- * array gives the sta through container_of */
- u8 *ptid = (u8 *)data;
- u8 *timer_to_id = ptid - *ptid;
- struct sta_info *sta = container_of(timer_to_id, struct sta_info,
- timer_to_tid[0]);
+ struct tid_ampdu_rx *tid_rx_timer =
+ from_timer(tid_rx_timer, t, session_timer);
+ struct sta_info *sta = tid_rx_timer->sta;
+ u16 tid = tid_rx_timer->tid;
struct tid_ampdu_rx *tid_rx;
unsigned long timeout;
rcu_read_lock();
- tid_rx = rcu_dereference(sta->ampdu_mlme.tid_rx[*ptid]);
+ tid_rx = rcu_dereference(sta->ampdu_mlme.tid_rx[tid]);
if (!tid_rx) {
rcu_read_unlock();
return;
@@ -180,21 +176,18 @@ static void sta_rx_agg_session_timer_expired(unsigned long data)
rcu_read_unlock();
ht_dbg(sta->sdata, "RX session timer expired on %pM tid %d\n",
- sta->sta.addr, (u16)*ptid);
+ sta->sta.addr, tid);
- set_bit(*ptid, sta->ampdu_mlme.tid_rx_timer_expired);
+ set_bit(tid, sta->ampdu_mlme.tid_rx_timer_expired);
ieee80211_queue_work(&sta->local->hw, &sta->ampdu_mlme.work);
}
-static void sta_rx_agg_reorder_timer_expired(unsigned long data)
+static void sta_rx_agg_reorder_timer_expired(struct timer_list *t)
{
- u8 *ptid = (u8 *)data;
- u8 *timer_to_id = ptid - *ptid;
- struct sta_info *sta = container_of(timer_to_id, struct sta_info,
- timer_to_tid[0]);
+ struct tid_ampdu_rx *tid_rx = from_timer(tid_rx, t, reorder_timer);
rcu_read_lock();
- ieee80211_release_reorder_timeout(sta, *ptid);
+ ieee80211_release_reorder_timeout(tid_rx->sta, tid_rx->tid);
rcu_read_unlock();
}
@@ -356,14 +349,12 @@ void ___ieee80211_start_rx_ba_session(struct sta_info *sta,
spin_lock_init(&tid_agg_rx->reorder_lock);
/* rx timer */
- setup_deferrable_timer(&tid_agg_rx->session_timer,
- sta_rx_agg_session_timer_expired,
- (unsigned long)&sta->timer_to_tid[tid]);
+ timer_setup(&tid_agg_rx->session_timer,
+ sta_rx_agg_session_timer_expired, TIMER_DEFERRABLE);
/* rx reorder timer */
- setup_timer(&tid_agg_rx->reorder_timer,
- sta_rx_agg_reorder_timer_expired,
- (unsigned long)&sta->timer_to_tid[tid]);
+ timer_setup(&tid_agg_rx->reorder_timer,
+ sta_rx_agg_reorder_timer_expired, 0);
/* prepare reordering buffer */
tid_agg_rx->reorder_buf =
@@ -399,6 +390,8 @@ void ___ieee80211_start_rx_ba_session(struct sta_info *sta,
tid_agg_rx->auto_seq = auto_seq;
tid_agg_rx->started = false;
tid_agg_rx->reorder_buf_filtered = 0;
+ tid_agg_rx->tid = tid;
+ tid_agg_rx->sta = sta;
status = WLAN_STATUS_SUCCESS;
/* activate it for RX */
diff --git a/net/mac80211/agg-tx.c b/net/mac80211/agg-tx.c
index bef516ec47f9..dedbb1fb10e7 100644
--- a/net/mac80211/agg-tx.c
+++ b/net/mac80211/agg-tx.c
@@ -422,15 +422,12 @@ int ___ieee80211_stop_tx_ba_session(struct sta_info *sta, u16 tid,
* add Block Ack response will arrive from the recipient.
* If this timer expires sta_addba_resp_timer_expired will be executed.
*/
-static void sta_addba_resp_timer_expired(unsigned long data)
+static void sta_addba_resp_timer_expired(struct timer_list *t)
{
- /* not an elegant detour, but there is no choice as the timer passes
- * only one argument, and both sta_info and TID are needed, so init
- * flow in sta_info_create gives the TID as data, while the timer_to_id
- * array gives the sta through container_of */
- u16 tid = *(u8 *)data;
- struct sta_info *sta = container_of((void *)data,
- struct sta_info, timer_to_tid[tid]);
+ struct tid_ampdu_tx *tid_tx_timer =
+ from_timer(tid_tx_timer, t, addba_resp_timer);
+ struct sta_info *sta = tid_tx_timer->sta;
+ u16 tid = tid_tx_timer->tid;
struct tid_ampdu_tx *tid_tx;
/* check if the TID waits for addBA response */
@@ -525,21 +522,17 @@ void ieee80211_tx_ba_session_handle_start(struct sta_info *sta, int tid)
* After accepting the AddBA Response we activated a timer,
* resetting it after each frame that we send.
*/
-static void sta_tx_agg_session_timer_expired(unsigned long data)
+static void sta_tx_agg_session_timer_expired(struct timer_list *t)
{
- /* not an elegant detour, but there is no choice as the timer passes
- * only one argument, and various sta_info are needed here, so init
- * flow in sta_info_create gives the TID as data, while the timer_to_id
- * array gives the sta through container_of */
- u8 *ptid = (u8 *)data;
- u8 *timer_to_id = ptid - *ptid;
- struct sta_info *sta = container_of(timer_to_id, struct sta_info,
- timer_to_tid[0]);
+ struct tid_ampdu_tx *tid_tx_timer =
+ from_timer(tid_tx_timer, t, session_timer);
+ struct sta_info *sta = tid_tx_timer->sta;
+ u16 tid = tid_tx_timer->tid;
struct tid_ampdu_tx *tid_tx;
unsigned long timeout;
rcu_read_lock();
- tid_tx = rcu_dereference(sta->ampdu_mlme.tid_tx[*ptid]);
+ tid_tx = rcu_dereference(sta->ampdu_mlme.tid_tx[tid]);
if (!tid_tx || test_bit(HT_AGG_STATE_STOPPING, &tid_tx->state)) {
rcu_read_unlock();
return;
@@ -555,9 +548,9 @@ static void sta_tx_agg_session_timer_expired(unsigned long data)
rcu_read_unlock();
ht_dbg(sta->sdata, "tx session timer expired on %pM tid %d\n",
- sta->sta.addr, (u16)*ptid);
+ sta->sta.addr, tid);
- ieee80211_stop_tx_ba_session(&sta->sta, *ptid);
+ ieee80211_stop_tx_ba_session(&sta->sta, tid);
}
int ieee80211_start_tx_ba_session(struct ieee80211_sta *pubsta, u16 tid,
@@ -672,14 +665,11 @@ int ieee80211_start_tx_ba_session(struct ieee80211_sta *pubsta, u16 tid,
tid_tx->timeout = timeout;
/* response timer */
- setup_timer(&tid_tx->addba_resp_timer,
- sta_addba_resp_timer_expired,
- (unsigned long)&sta->timer_to_tid[tid]);
+ timer_setup(&tid_tx->addba_resp_timer, sta_addba_resp_timer_expired, 0);
/* tx timer */
- setup_deferrable_timer(&tid_tx->session_timer,
- sta_tx_agg_session_timer_expired,
- (unsigned long)&sta->timer_to_tid[tid]);
+ timer_setup(&tid_tx->session_timer,
+ sta_tx_agg_session_timer_expired, TIMER_DEFERRABLE);
/* assign a dialog token */
sta->ampdu_mlme.dialog_token_allocator++;
diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index 877d35796776..b5add1464aeb 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -379,14 +379,6 @@ struct sta_info *sta_info_alloc(struct ieee80211_sub_if_data *sdata,
if (sta_prepare_rate_control(local, sta, gfp))
goto free_txq;
- for (i = 0; i < IEEE80211_NUM_TIDS; i++) {
- /*
- * timer_to_tid must be initialized with identity mapping
- * to enable session_timer's data differentiation. See
- * sta_rx_agg_session_timer_expired for usage.
- */
- sta->timer_to_tid[i] = i;
- }
for (i = 0; i < IEEE80211_NUM_ACS; i++) {
skb_queue_head_init(&sta->ps_tx_buf[i]);
skb_queue_head_init(&sta->tx_filtered[i]);
diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
index 5c54acd10562..1b9c1e81495d 100644
--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -126,6 +126,8 @@ enum ieee80211_agg_stop_reason {
AGG_STOP_DESTROY_STA,
};
+struct sta_info;
+
/**
* struct tid_ampdu_tx - TID aggregation information (Tx).
*
@@ -133,8 +135,10 @@ enum ieee80211_agg_stop_reason {
* @session_timer: check if we keep Tx-ing on the TID (by timeout value)
* @addba_resp_timer: timer for peer's response to addba request
* @pending: pending frames queue -- use sta's spinlock to protect
+ * @sta: station we are attached to
* @dialog_token: dialog token for aggregation session
* @timeout: session timeout value to be filled in ADDBA requests
+ * @tid: index in station tid list
* @state: session state (see above)
* @last_tx: jiffies of last tx activity
* @stop_initiator: initiator of a session stop
@@ -158,9 +162,11 @@ struct tid_ampdu_tx {
struct timer_list session_timer;
struct timer_list addba_resp_timer;
struct sk_buff_head pending;
+ struct sta_info *sta;
unsigned long state;
unsigned long last_tx;
u16 timeout;
+ u16 tid;
u8 dialog_token;
u8 stop_initiator;
bool tx_stop;
@@ -181,12 +187,14 @@ struct tid_ampdu_tx {
* @reorder_time: jiffies when skb was added
* @session_timer: check if peer keeps Tx-ing on the TID (by timeout value)
* @reorder_timer: releases expired frames from the reorder buffer.
+ * @sta: station we are attached to
* @last_rx: jiffies of last rx activity
* @head_seq_num: head sequence number in reordering buffer.
* @stored_mpdu_num: number of MPDUs in reordering buffer
* @ssn: Starting Sequence Number expected to be aggregated.
* @buf_size: buffer size for incoming A-MPDUs
* @timeout: reset timer value (in TUs).
+ * @tid: index in station tid list
* @rcu_head: RCU head used for freeing this struct
* @reorder_lock: serializes access to reorder buffer, see below.
* @auto_seq: used for offloaded BA sessions to automatically pick head_seq_and
@@ -208,6 +216,7 @@ struct tid_ampdu_rx {
u64 reorder_buf_filtered;
struct sk_buff_head *reorder_buf;
unsigned long *reorder_time;
+ struct sta_info *sta;
struct timer_list session_timer;
struct timer_list reorder_timer;
unsigned long last_rx;
@@ -216,6 +225,7 @@ struct tid_ampdu_rx {
u16 ssn;
u16 buf_size;
u16 timeout;
+ u16 tid;
u8 auto_seq:1,
removed:1,
started:1;
@@ -447,7 +457,6 @@ struct ieee80211_sta_rx_stats {
* plus one for non-QoS frames)
* @tid_seq: per-TID sequence numbers for sending to this STA
* @ampdu_mlme: A-MPDU state machine state
- * @timer_to_tid: identity mapping to ID timers
* @mesh: mesh STA information
* @debugfs_dir: debug filesystem directory dentry
* @dead: set to true when sta is unlinked
@@ -554,7 +563,6 @@ struct sta_info {
* Aggregation information, locked with lock.
*/
struct sta_ampdu_mlme ampdu_mlme;
- u8 timer_to_tid[IEEE80211_NUM_TIDS];
#ifdef CONFIG_MAC80211_DEBUGFS
struct dentry *debugfs_dir;
--
2.7.4
--
Kees Cook
Pixel Security
^ permalink raw reply related
* Re: [PATCH net-next] tcp: Check daddr_cache before use in tracepoint
From: David Ahern @ 2017-10-16 22:08 UTC (permalink / raw)
To: Cong Wang; +Cc: Linux Kernel Network Developers
In-Reply-To: <CAM_iQpVy-YY+e1R17P_9uV22n5DbXxL8+ACpY+E5yJYdp8doAQ@mail.gmail.com>
On 10/16/17 4:05 PM, Cong Wang wrote:
> Or if you mean non-zero, they are set to LOOPBACK4_IPV6
> which is what I expect.
yes, I meant non-0. thanks for the explanation. I'll send a v2
^ permalink raw reply
* Re: [PATCH 3/3] bpf: Make sure that ->comm does not change under us.
From: Alexei Starovoitov @ 2017-10-16 22:10 UTC (permalink / raw)
To: Richard Weinberger
Cc: Daniel Borkmann, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, Alexei Starovoitov
On Mon, Oct 16, 2017 at 2:10 PM, Richard Weinberger <richard@nod.at> wrote:
> Am Montag, 16. Oktober 2017, 23:02:06 CEST schrieb Daniel Borkmann:
>> On 10/16/2017 10:55 PM, Richard Weinberger wrote:
>> > Am Montag, 16. Oktober 2017, 22:50:43 CEST schrieb Daniel Borkmann:
>> >>> struct task_struct *task = current;
>> >>>
>> >>> + task_lock(task);
>> >>>
>> >>> strncpy(buf, task->comm, size);
>> >>>
>> >>> + task_unlock(task);
>> >>
>> >> Wouldn't this potentially lead to a deadlock? E.g. you attach yourself
>> >> to task_lock() / spin_lock() / etc, and then the BPF prog triggers the
>> >> bpf_get_current_comm() taking the lock again ...
>> >
>> > Yes, but doesn't the same apply to the use case when I attach to strncpy()
>> > and run bpf_get_current_comm()?
>>
>> You mean due to recursion? In that case trace_call_bpf() would bail out
>> due to the bpf_prog_active counter.
>
> Ah, that's true.
> So, when someone wants to use bpf_get_current_comm() while tracing task_lock,
> we have a problem. I agree.
> On the other hand, without locking the function may return wrong results.
it will surely race with somebody else setting task comm and it's fine.
all of bpf tracing is read-only, so locks are only allowed inside bpf core
bits like maps. Taking core locks like task_lock() is quite scary.
bpf scripts rely on bpf_probe_read() of all sorts of kernel fields
so reading comm here w/o lock is fine.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox