* Re: Linux ECN Handling
From: Steve Ibanez @ 2017-10-23 22:15 UTC (permalink / raw)
To: netdev; +Cc: Florian Westphal, Mohammad Alizadeh
In-Reply-To: <20171019124312.GE16796@breakpoint.cc>
Hi All,
I upgraded the kernel on all of our machines to Linux
4.13.8-041308-lowlatency. However, I'm still observing the same
behavior where the source enters a timeout when the CWND=1MSS and it
receives ECN marks.
Here are the measured flow rates:
<https://drive.google.com/file/d/0B-bt9QS-C3ONT0VXMUt6WHhKREE/view?usp=sharing>
Here are snapshots of the packet traces at the sources when they both
enter a timeout at t=1.6sec:
10.0.0.1 timeout event:
<https://drive.google.com/file/d/0B-bt9QS-C3ONcl9WRnRPazg2ems/view?usp=sharing>
10.0.0.3 timeout event:
<https://drive.google.com/file/d/0B-bt9QS-C3ONeDlxRjNXa0VzWm8/view?usp=sharing>
Both still essentially follow the same sequence of events that I
mentioned earlier:
(1) receives an ACK for byte XYZ with the ECN flag set
(2) stops sending for RTO_min=300ms
(3) sends a retransmission for byte XYZ
The cwnd samples reported by tcp_probe still indicate that the sources
are reacting to the ECN marks more than once per window. Here are the
cwnd samples at the same timeout event mentioned above:
<https://drive.google.com/file/d/0B-bt9QS-C3ONdEZQdktpaW5JUm8/view?usp=sharing>
Let me know if there is anything else you think I should try.
Thanks,
-Steve
On Thu, Oct 19, 2017 at 5:43 AM, Florian Westphal <fw@strlen.de> wrote:
>
> [ full-quoting due to Cc fixups, adding netdev ]
>
> Steve Ibanez <sibanez@stanford.edu> wrote:
> > Hi Florian, Neal, and Daniel,
> >
> > I hope this email finds you well. My name is Stephen Ibanez and I'm a PhD
> > Student at Stanford currently working on a project with Mohammad Alizadeh,
> > Nick McKeown, and Lavanya Jose. We have been doing some experiments using
> > the linux DCTCP implementation and are trying to understand some strange
> > behavior that we are encountering. I'm contacting you three because I have
> > seen your names on some of the source files and recent commits in the linux
> > source tree. Hopefully you can help us out or put us in contact with the
> > right people?
> >
> > Here are some details about our servers:
> >
> > - Distribution: Ubuntu 14.04 LTS
> > - Kernel release: 4.4.0-75-generic
>
> Can you re-test with a more recent kernel such as 4.13.8?
>
> > *The experiment:*
> >
> > We use iperf3 to generate two DCTCP flows from different servers to a
> > common server, as shown in the diagram below. We measure the sending rate
> > of each flow, record the tcp_probe output, as well as run tcpdump on the
> > source host interfaces.
> >
> > [image: Inline image 6]
> >
> > *The problem:*
> >
> > Our rate measurements look like the one shown below; the flows often enter
> > timeouts. In this case, both flows hit a timeout at t=0.3.
> > [image: Inline image 2]
> >
> > When looking at the sequence of packets seen at the source host interfaces
> > around this timeout event this is what we see:
> >
> > *10.0.0.1 timeout event:*
> > [image: Inline image 3]
> >
> > *10.0.0.3 timeout event:*
> > [image: Inline image 4]
> >
> > In both cases, the source:
> > (1) receives an ACK for byte XYZ with the ECN flag set
> > (2) stops sending anything for RTO_min=300ms
> > (3) sends a retransmission for byte XYZ
> >
> > I have verified that this behavior is consistent across multiple experiment
> > runs. Here are the CWND samples for the 10.0.0.1 flow provided by tcp_probe
> > at the time of the timeout event:
> >
> > [image: Inline image 5]
> >
> > From what I can tell, tcp_probe logs a sample whenever a packet is
> > received. If this is true, then that means when the source receives the
> > final ECN marked ACK just before the timeout the CWND=1 MSS.
> >
> > *The conclusion:*
> >
> > We believe that there may be an issue with how the linux kernel is handling
> > the ECN echoes. For DCTCP, if the CWND is 1 MSS and the end host is still
> > receiving ECN marks then the CWND should remain at 1 MSS and should *not*
> > enter a timeout. This is because the switch can perform ECN marking very
> > aggressively causing the source end host to receive many redundant ECN
> > echoes over a short period of time.
> >
> > Another potential issue is that from the CWND plot above it looks like the
> > end host may be reacting to congestion signals more than once per window,
> > which should not happen (section 5 of RF3168
> > <https://tools.ietf.org/html/rfc3168>). tcp_probe reports SRTT measurements
> > of about 400-500 us and in the plot above the CWND is reduced 6 times
> > within this amount of time.
> >
> > We have not yet tracked down the code path in the kernel code that is
> > causing the behavior described above. Perhaps this is something that you
> > can help us with? We would love to hear your thoughts on this matter and
> > are happy to try other experiments that you suggest.
> >
> > Here is a link
> > <https://drive.google.com/file/d/0Bw-GEX7h5ufiYmpCV2VpOGEtQWs/view?usp=sharing>
> > to
> > download the packet traces if you would like to take a look.
> > han-1_host.pcap is the trace from 10.0.0.1 and han-3_host.pcap is the trace
> > from 10.0.0.3.
> >
> > Looking forward to hearing from you!
> >
> > Best,
> > -Steve
^ permalink raw reply
* Re: [Patch net 00/15] net_sched: remove RCU callbacks from TC
From: Eric Dumazet @ 2017-10-23 23:16 UTC (permalink / raw)
To: Cong Wang; +Cc: netdev, paulmck, jhs, john.fastabend, Chris Mi
In-Reply-To: <20171023220304.2268-1-xiyou.wangcong@gmail.com>
On Mon, 2017-10-23 at 15:02 -0700, Cong Wang wrote:
> b) As suggested by Paul, we could defer the work to a workqueue and
> gain the permission of holding RTNL again without any performance
> impact, however, this seems impossible too, because as lockdep
> complains we have a deadlock when flushing workqueue while hodling
> RTNL lock, see the rcu_barrier() in tcf_block_put().
>
> Therefore, the simplest solution we have is probably just getting
> rid of these RCU callbacks, because they are not necessary at all,
> callers of these call_rcu() are all on slow paths and have RTNL
> lock, so blocking is allowed in their contexts.
I am against these pessimistic changes, sorry for not following past
discussions last week.
I am asking a talk during upcoming netdev/netconf about this, if we need
to take a decision.
RTNL is already a big problem for many of us, adding synchronize_rcu()
calls while holding RTNL is a no - go, unless we have clear evidence it
can not be avoided.
Thanks !
^ permalink raw reply
* [PATCH v2 net] tcp/dccp: fix other lockdep splats accessing ireq_opt
From: Eric Dumazet @ 2017-10-23 23:15 UTC (permalink / raw)
To: kernel test robot; +Cc: David Miller, netdev
In-Reply-To: <1508770960.30291.83.camel@edumazet-glaptop3.roam.corp.google.com>
From: Eric Dumazet <edumazet@google.com>
In my first attempt to fix the lockdep splat, I forgot we could
enter inet_csk_route_req() with a freshly allocated request socket,
for which refcount has not yet been elevated, due to complex
SLAB_TYPESAFE_BY_RCU rules.
We either are in rcu_read_lock() section _or_ we own a refcount on the
request.
Correct RCU verb to use here is rcu_dereference_check(), although it is
not possible to prove we actually own a reference on a shared
refcount :/
In v2, I added ireq_opt_deref() helper and use in three places, to fix other
possible splats.
[ 49.844590] lockdep_rcu_suspicious+0xea/0xf3
[ 49.846487] inet_csk_route_req+0x53/0x14d
[ 49.848334] tcp_v4_route_req+0xe/0x10
[ 49.850174] tcp_conn_request+0x31c/0x6a0
[ 49.851992] ? __lock_acquire+0x614/0x822
[ 49.854015] tcp_v4_conn_request+0x5a/0x79
[ 49.855957] ? tcp_v4_conn_request+0x5a/0x79
[ 49.858052] tcp_rcv_state_process+0x98/0xdcc
[ 49.859990] ? sk_filter_trim_cap+0x2f6/0x307
[ 49.862085] tcp_v4_do_rcv+0xfc/0x145
[ 49.864055] ? tcp_v4_do_rcv+0xfc/0x145
[ 49.866173] tcp_v4_rcv+0x5ab/0xaf9
[ 49.868029] ip_local_deliver_finish+0x1af/0x2e7
[ 49.870064] ip_local_deliver+0x1b2/0x1c5
[ 49.871775] ? inet_del_offload+0x45/0x45
[ 49.873916] ip_rcv_finish+0x3f7/0x471
[ 49.875476] ip_rcv+0x3f1/0x42f
[ 49.876991] ? ip_local_deliver_finish+0x2e7/0x2e7
[ 49.878791] __netif_receive_skb_core+0x6d3/0x950
[ 49.880701] ? process_backlog+0x7e/0x216
[ 49.882589] __netif_receive_skb+0x1d/0x5e
[ 49.884122] process_backlog+0x10c/0x216
[ 49.885812] net_rx_action+0x147/0x3df
Fixes: a6ca7abe53633 ("tcp/dccp: fix lockdep splat in inet_csk_route_req()")
Fixes: c92e8c02fe66 ("tcp/dccp: fix ireq->opt races")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: kernel test robot <fengguang.wu@intel.com>
Reported-by: Maciej Żenczykowski <maze@google.com>
---
include/net/inet_sock.h | 6 ++++++
net/dccp/ipv4.c | 2 +-
net/ipv4/inet_connection_sock.c | 4 ++--
net/ipv4/tcp_ipv4.c | 2 +-
4 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/include/net/inet_sock.h b/include/net/inet_sock.h
index 425752f768d2f1a0efb13964204e07f27609e9db..b9d59d93ec8f1e763f53a4df25366d1f1d576563 100644
--- a/include/net/inet_sock.h
+++ b/include/net/inet_sock.h
@@ -132,6 +132,12 @@ static inline int inet_request_bound_dev_if(const struct sock *sk,
return sk->sk_bound_dev_if;
}
+static inline struct ip_options_rcu *ireq_opt_deref(const struct inet_request_sock *ireq)
+{
+ return rcu_dereference_check(ireq->ireq_opt,
+ refcount_read(&ireq.req->rsk_refcnt) > 0);
+}
+
struct inet_cork {
unsigned int flags;
__be32 addr;
diff --git a/net/dccp/ipv4.c b/net/dccp/ipv4.c
index 0490916864f93d5466e87f5b97dc524b3ee57a2e..e65fcb45c3f6c1edc70fc9898ebe6404175b102f 100644
--- a/net/dccp/ipv4.c
+++ b/net/dccp/ipv4.c
@@ -495,7 +495,7 @@ static int dccp_v4_send_response(const struct sock *sk, struct request_sock *req
ireq->ir_rmt_addr);
err = ip_build_and_send_pkt(skb, sk, ireq->ir_loc_addr,
ireq->ir_rmt_addr,
- rcu_dereference(ireq->ireq_opt));
+ ireq_opt_deref(ireq));
err = net_xmit_eval(err);
}
diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index 18cd2eae758ff1a9d8a736e143417c7007b99067..b47a59cb3573b3b77aa5cbb9c2739a12ef37a237 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -543,8 +543,8 @@ struct dst_entry *inet_csk_route_req(const struct sock *sk,
struct ip_options_rcu *opt;
struct rtable *rt;
- opt = rcu_dereference_protected(ireq->ireq_opt,
- refcount_read(&req->rsk_refcnt) > 0);
+ opt = ireq_opt_deref(ireq);
+
flowi4_init_output(fl4, ireq->ir_iif, ireq->ir_mark,
RT_CONN_FLAGS(sk), RT_SCOPE_UNIVERSE,
sk->sk_protocol, inet_sk_flowi_flags(sk),
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 4c43365c374c8bf868fc0b862333244ca26d5016..5b027c69cbc540d4e933189f9de5baab5472eadb 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -877,7 +877,7 @@ static int tcp_v4_send_synack(const struct sock *sk, struct dst_entry *dst,
err = ip_build_and_send_pkt(skb, sk, ireq->ir_loc_addr,
ireq->ir_rmt_addr,
- rcu_dereference(ireq->ireq_opt));
+ ireq_opt_deref(ireq));
err = net_xmit_eval(err);
}
^ permalink raw reply related
* Re: [PATCH v2 net-next 2/2] ibmvnic: Fix failover error path for non-fatal resets
From: kbuild test robot @ 2017-10-23 23:02 UTC (permalink / raw)
To: John Allen; +Cc: kbuild-all, netdev, Thomas Falcon, Nathan Fontenot
In-Reply-To: <0d186152-78b3-fc62-3227-788c314448b0@linux.vnet.ibm.com>
[-- Attachment #1: Type: text/plain, Size: 4274 bytes --]
Hi John,
[auto build test WARNING on net-next/master]
url: https://github.com/0day-ci/linux/commits/John-Allen/ibmvnic-Update-reset-infrastructure-to-support-tunable-parameters/20171024-062007
config: powerpc-allmodconfig (attached as .config)
compiler: powerpc64-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=powerpc
Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings
All warnings (new ones prefixed by >>):
drivers/net//ethernet/ibm/ibmvnic.c: In function '__ibmvnic_reset':
>> drivers/net//ethernet/ibm/ibmvnic.c:1620:26: warning: 'rc' may be used uninitialized in this function [-Wmaybe-uninitialized]
adapter->reset_done_rc = rc;
~~~~~~~~~~~~~~~~~~~~~~~^~~~
vim +/rc +1620 drivers/net//ethernet/ibm/ibmvnic.c
ed651a108 Nathan Fontenot 2017-05-03 1592
ed651a108 Nathan Fontenot 2017-05-03 1593 static void __ibmvnic_reset(struct work_struct *work)
ed651a108 Nathan Fontenot 2017-05-03 1594 {
ed651a108 Nathan Fontenot 2017-05-03 1595 struct ibmvnic_rwi *rwi;
ed651a108 Nathan Fontenot 2017-05-03 1596 struct ibmvnic_adapter *adapter;
ed651a108 Nathan Fontenot 2017-05-03 1597 struct net_device *netdev;
ed651a108 Nathan Fontenot 2017-05-03 1598 u32 reset_state;
ed651a108 Nathan Fontenot 2017-05-03 1599 int rc;
ed651a108 Nathan Fontenot 2017-05-03 1600
ed651a108 Nathan Fontenot 2017-05-03 1601 adapter = container_of(work, struct ibmvnic_adapter, ibmvnic_reset);
ed651a108 Nathan Fontenot 2017-05-03 1602 netdev = adapter->netdev;
ed651a108 Nathan Fontenot 2017-05-03 1603
ed651a108 Nathan Fontenot 2017-05-03 1604 mutex_lock(&adapter->reset_lock);
ed651a108 Nathan Fontenot 2017-05-03 1605 adapter->resetting = true;
ed651a108 Nathan Fontenot 2017-05-03 1606 reset_state = adapter->state;
ed651a108 Nathan Fontenot 2017-05-03 1607
ed651a108 Nathan Fontenot 2017-05-03 1608 rwi = get_next_rwi(adapter);
ed651a108 Nathan Fontenot 2017-05-03 1609 while (rwi) {
ed651a108 Nathan Fontenot 2017-05-03 1610 rc = do_reset(adapter, rwi, reset_state);
ed651a108 Nathan Fontenot 2017-05-03 1611 kfree(rwi);
73daa92e4 John Allen 2017-10-23 1612 if (rc && rc != IBMVNIC_INIT_FAILED)
ed651a108 Nathan Fontenot 2017-05-03 1613 break;
ed651a108 Nathan Fontenot 2017-05-03 1614
ed651a108 Nathan Fontenot 2017-05-03 1615 rwi = get_next_rwi(adapter);
ed651a108 Nathan Fontenot 2017-05-03 1616 }
ed651a108 Nathan Fontenot 2017-05-03 1617
ce7a66cc8 John Allen 2017-10-23 1618 if (adapter->wait_for_reset) {
ce7a66cc8 John Allen 2017-10-23 1619 adapter->wait_for_reset = false;
ce7a66cc8 John Allen 2017-10-23 @1620 adapter->reset_done_rc = rc;
ce7a66cc8 John Allen 2017-10-23 1621 complete(&adapter->reset_done);
ce7a66cc8 John Allen 2017-10-23 1622 }
ce7a66cc8 John Allen 2017-10-23 1623
ed651a108 Nathan Fontenot 2017-05-03 1624 if (rc) {
d1cf33d93 Nathan Fontenot 2017-08-08 1625 netdev_dbg(adapter->netdev, "Reset failed\n");
ed651a108 Nathan Fontenot 2017-05-03 1626 free_all_rwi(adapter);
6d0af07d5 Wei Yongjun 2017-05-18 1627 mutex_unlock(&adapter->reset_lock);
ed651a108 Nathan Fontenot 2017-05-03 1628 return;
ed651a108 Nathan Fontenot 2017-05-03 1629 }
ed651a108 Nathan Fontenot 2017-05-03 1630
ed651a108 Nathan Fontenot 2017-05-03 1631 adapter->resetting = false;
ed651a108 Nathan Fontenot 2017-05-03 1632 mutex_unlock(&adapter->reset_lock);
ed651a108 Nathan Fontenot 2017-05-03 1633 }
ed651a108 Nathan Fontenot 2017-05-03 1634
:::::: The code at line 1620 was first introduced by commit
:::::: ce7a66cc8fceb75d2a95e5e68d21c1889821ff08 ibmvnic: Update reset infrastructure to support tunable parameters
:::::: TO: John Allen <jallen@linux.vnet.ibm.com>
:::::: CC: 0day robot <fengguang.wu@intel.com>
---
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: 55120 bytes --]
^ permalink raw reply
* Re: [PATCH net] tcp/dccp: fix again lockdep splat in inet_csk_route_req()
From: Eric Dumazet @ 2017-10-23 22:46 UTC (permalink / raw)
To: kernel test robot; +Cc: David Miller, netdev
In-Reply-To: <1508770960.30291.83.camel@edumazet-glaptop3.roam.corp.google.com>
On Mon, 2017-10-23 at 08:02 -0700, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> In my first attempt to fix the lockdep splat, I forgot we could
> enter inet_csk_route_req() with a freshly allocated request socket,
> for which refcount has not yet been elevated, due to complex
> SLAB_TYPESAFE_BY_RCU rules.
>
> We either are in rcu_read_lock() section _or_ we own a refcount on the
> request.
>
> Correct RCU verb to use here is rcu_dereference_check(), although it is
> not possible to prove we actually own a reference on a shared
> refcount :/
David, please hold on this patch, I will squash another fixes, since we
need the same in two other places (tcp_v4_send_synack() &
dccp_v4_send_response())
^ permalink raw reply
* Re: [PATCH net-next] ipv6: add ip6_null_entry check in rt6_select()
From: Martin KaFai Lau @ 2017-10-23 22:27 UTC (permalink / raw)
To: Wei Wang; +Cc: David Miller, netdev, Eric Dumazet
In-Reply-To: <20171023215935.52136-1-tracywwnj@gmail.com>
On Mon, Oct 23, 2017 at 09:59:35PM +0000, Wei Wang wrote:
> From: Wei Wang <weiwan@google.com>
>
> In rt6_select(), fn->leaf could be pointing to net->ipv6.ip6_null_entry.
> In this case, we should directly return instead of trying to carry on
> with the rest of the process.
> If not, we could crash at:
> spin_lock_bh(&leaf->rt6i_table->rt6_lock);
> because net->ipv6.ip6_null_entry does not have rt6i_table set.
Make sense. leaf->rt6i_table->rt6_lock is only needed after the rcu work.
Thanks for fixing it!
Acked-by: Martin KaFai Lau <kafai@fb.com>
^ permalink raw reply
* Re: [PATCH v3 net-next] tcp: Configure TFO without cookie per socket and/or per route
From: Yuchung Cheng @ 2017-10-23 22:24 UTC (permalink / raw)
To: Christoph Paasch; +Cc: David Miller, Eric Dumazet, netdev
In-Reply-To: <20171023202223.42600-1-cpaasch@apple.com>
On Mon, Oct 23, 2017 at 1:22 PM, Christoph Paasch <cpaasch@apple.com> wrote:
>
> We already allow to enable TFO without a cookie by using the
> fastopen-sysctl and setting it to TFO_SERVER_COOKIE_NOT_REQD (or
> TFO_CLIENT_NO_COOKIE).
> This is safe to do in certain environments where we know that there
> isn't a malicous host (aka., data-centers) or when the
> application-protocol already provides an authentication mechanism in the
> first flight of data.
>
> A server however might be providing multiple services or talking to both
> sides (public Internet and data-center). So, this server would want to
> enable cookie-less TFO for certain services and/or for connections that
> go to the data-center.
>
> This patch exposes a socket-option and a per-route attribute to enable such
> fine-grained configurations.
>
> Signed-off-by: Christoph Paasch <cpaasch@apple.com>
Reviewed-by: Yuchung Cheng <ycheng@google.com>
nice work thanks.
> ---
>
> Notes:
> v3: * Fix/Clarify commit-log
> * Add helper function tcp_fastopen_no_cookie()
> * Set tp->fastopen_no_cookie to "val", not 1 in do_tcp_setsockopt().
> v2: * Rename to fastopen_no_cookie and TCP_FASTOPEN_NO_COOKIE
> * Add per-route attribute for fastopen_no_cookie
> * Get rid of the capability check
>
> include/linux/tcp.h | 3 ++-
> include/net/tcp.h | 3 ++-
> include/uapi/linux/rtnetlink.h | 2 ++
> include/uapi/linux/tcp.h | 1 +
> net/ipv4/tcp.c | 12 ++++++++++++
> net/ipv4/tcp_fastopen.c | 20 +++++++++++++++++---
> net/ipv4/tcp_input.c | 2 +-
> 7 files changed, 37 insertions(+), 6 deletions(-)
>
> diff --git a/include/linux/tcp.h b/include/linux/tcp.h
> index 1d2c44e09e31..173a7c2f9636 100644
> --- a/include/linux/tcp.h
> +++ b/include/linux/tcp.h
> @@ -215,7 +215,8 @@ struct tcp_sock {
> u8 chrono_type:2, /* current chronograph type */
> rate_app_limited:1, /* rate_{delivered,interval_us} limited? */
> fastopen_connect:1, /* FASTOPEN_CONNECT sockopt */
> - unused:4;
> + fastopen_no_cookie:1, /* Allow send/recv SYN+data without a cookie */
> + unused:3;
> u8 nonagle : 4,/* Disable Nagle algorithm? */
> thin_lto : 1,/* Use linear timeouts for thin streams */
> unused1 : 1,
> diff --git a/include/net/tcp.h b/include/net/tcp.h
> index 2c13484704cb..2392f74074e7 100644
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
> @@ -1567,7 +1567,8 @@ int tcp_fastopen_reset_cipher(struct net *net, struct sock *sk,
> void tcp_fastopen_add_skb(struct sock *sk, struct sk_buff *skb);
> struct sock *tcp_try_fastopen(struct sock *sk, struct sk_buff *skb,
> struct request_sock *req,
> - struct tcp_fastopen_cookie *foc);
> + struct tcp_fastopen_cookie *foc,
> + const struct dst_entry *dst);
> void tcp_fastopen_init_key_once(struct net *net);
> bool tcp_fastopen_cookie_check(struct sock *sk, u16 *mss,
> struct tcp_fastopen_cookie *cookie);
> diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h
> index dab7dad9e01a..fe6679268901 100644
> --- a/include/uapi/linux/rtnetlink.h
> +++ b/include/uapi/linux/rtnetlink.h
> @@ -430,6 +430,8 @@ enum {
> #define RTAX_QUICKACK RTAX_QUICKACK
> RTAX_CC_ALGO,
> #define RTAX_CC_ALGO RTAX_CC_ALGO
> + RTAX_FASTOPEN_NO_COOKIE,
> +#define RTAX_FASTOPEN_NO_COOKIE RTAX_FASTOPEN_NO_COOKIE
> __RTAX_MAX
> };
>
> diff --git a/include/uapi/linux/tcp.h b/include/uapi/linux/tcp.h
> index 69c7493e42f8..d67e1d40c6d6 100644
> --- a/include/uapi/linux/tcp.h
> +++ b/include/uapi/linux/tcp.h
> @@ -120,6 +120,7 @@ enum {
> #define TCP_ULP 31 /* Attach a ULP to a TCP connection */
> #define TCP_MD5SIG_EXT 32 /* TCP MD5 Signature with extensions */
> #define TCP_FASTOPEN_KEY 33 /* Set the key for Fast Open (cookie) */
> +#define TCP_FASTOPEN_NO_COOKIE 34 /* Enable TFO without a TFO cookie */
>
> struct tcp_repair_opt {
> __u32 opt_code;
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index 8b1fa4dd4538..02eb2f9cce4f 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -2832,6 +2832,14 @@ static int do_tcp_setsockopt(struct sock *sk, int level,
> err = -EOPNOTSUPP;
> }
> break;
> + case TCP_FASTOPEN_NO_COOKIE:
> + if (val > 1 || val < 0)
> + err = -EINVAL;
> + else if (!((1 << sk->sk_state) & (TCPF_CLOSE | TCPF_LISTEN)))
> + err = -EINVAL;
> + else
> + tp->fastopen_no_cookie = val;
> + break;
> case TCP_TIMESTAMP:
> if (!tp->repair)
> err = -EPERM;
> @@ -3252,6 +3260,10 @@ static int do_tcp_getsockopt(struct sock *sk, int level,
> val = tp->fastopen_connect;
> break;
>
> + case TCP_FASTOPEN_NO_COOKIE:
> + val = tp->fastopen_no_cookie;
> + break;
> +
> case TCP_TIMESTAMP:
> val = tcp_time_stamp_raw() + tp->tsoffset;
> break;
> diff --git a/net/ipv4/tcp_fastopen.c b/net/ipv4/tcp_fastopen.c
> index 21075ce19cb6..e0a4b56644aa 100644
> --- a/net/ipv4/tcp_fastopen.c
> +++ b/net/ipv4/tcp_fastopen.c
> @@ -310,13 +310,23 @@ static bool tcp_fastopen_queue_check(struct sock *sk)
> return true;
> }
>
> +static bool tcp_fastopen_no_cookie(const struct sock *sk,
> + const struct dst_entry *dst,
> + int flag)
> +{
> + return (sock_net(sk)->ipv4.sysctl_tcp_fastopen & flag) ||
> + tcp_sk(sk)->fastopen_no_cookie ||
> + (dst && dst_metric(dst, RTAX_FASTOPEN_NO_COOKIE));
> +}
> +
> /* Returns true if we should perform Fast Open on the SYN. The cookie (foc)
> * may be updated and return the client in the SYN-ACK later. E.g., Fast Open
> * cookie request (foc->len == 0).
> */
> struct sock *tcp_try_fastopen(struct sock *sk, struct sk_buff *skb,
> struct request_sock *req,
> - struct tcp_fastopen_cookie *foc)
> + struct tcp_fastopen_cookie *foc,
> + const struct dst_entry *dst)
> {
> bool syn_data = TCP_SKB_CB(skb)->end_seq != TCP_SKB_CB(skb)->seq + 1;
> int tcp_fastopen = sock_net(sk)->ipv4.sysctl_tcp_fastopen;
> @@ -333,7 +343,8 @@ struct sock *tcp_try_fastopen(struct sock *sk, struct sk_buff *skb,
> return NULL;
> }
>
> - if (syn_data && (tcp_fastopen & TFO_SERVER_COOKIE_NOT_REQD))
> + if (syn_data &&
> + tcp_fastopen_no_cookie(sk, dst, TFO_SERVER_COOKIE_NOT_REQD))
> goto fastopen;
>
> if (foc->len >= 0 && /* Client presents or requests a cookie */
> @@ -370,6 +381,7 @@ bool tcp_fastopen_cookie_check(struct sock *sk, u16 *mss,
> struct tcp_fastopen_cookie *cookie)
> {
> unsigned long last_syn_loss = 0;
> + const struct dst_entry *dst;
> int syn_loss = 0;
>
> tcp_fastopen_cache_get(sk, mss, cookie, &syn_loss, &last_syn_loss);
> @@ -387,7 +399,9 @@ bool tcp_fastopen_cookie_check(struct sock *sk, u16 *mss,
> return false;
> }
>
> - if (sock_net(sk)->ipv4.sysctl_tcp_fastopen & TFO_CLIENT_NO_COOKIE) {
> + dst = __sk_dst_get(sk);
> +
> + if (tcp_fastopen_no_cookie(sk, dst, TFO_CLIENT_NO_COOKIE)) {
> cookie->len = -1;
> return true;
> }
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index ab3f12898245..714ba553e545 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -6329,7 +6329,7 @@ int tcp_conn_request(struct request_sock_ops *rsk_ops,
> tcp_openreq_init_rwin(req, sk, dst);
> if (!want_cookie) {
> tcp_reqsk_record_syn(sk, req, skb);
> - fastopen_sk = tcp_try_fastopen(sk, skb, req, &foc);
> + fastopen_sk = tcp_try_fastopen(sk, skb, req, &foc, dst);
> }
> if (fastopen_sk) {
> af_ops->send_synack(fastopen_sk, dst, &fl, req,
> --
> 2.14.1
>
^ permalink raw reply
* [PATCH net-next v2 2/3] bpf: permit multiple bpf attachments for a single perf event
From: Yonghong Song @ 2017-10-23 22:13 UTC (permalink / raw)
To: peterz, rostedt, ast, daniel, kafai, netdev; +Cc: kernel-team
In-Reply-To: <20171023221331.4131164-1-yhs@fb.com>
This patch enables multiple bpf attachments for a
kprobe/uprobe/tracepoint single trace event.
Each trace_event keeps a list of attached perf events.
When an event happens, all attached bpf programs will
be executed based on the order of attachment.
A global bpf_event_mutex lock is introduced to protect
prog_array attaching and detaching. An alternative will
be introduce a mutex lock in every trace_event_call
structure, but it takes a lot of extra memory.
So a global bpf_event_mutex lock is a good compromise.
The bpf prog detachment involves allocation of memory.
If the allocation fails, a dummy do-nothing program
will replace to-be-detached program in-place.
Signed-off-by: Yonghong Song <yhs@fb.com>
Acked-by: Alexei Starovoitov <ast@kernel.org>
Acked-by: Martin KaFai Lau <kafai@fb.com>
---
include/linux/bpf.h | 30 +++++++++++++---
include/linux/trace_events.h | 33 ++++++++++++++---
include/trace/perf.h | 6 ++--
kernel/bpf/core.c | 81 ++++++++++++++++++++++++++++++++++++++++++
kernel/events/core.c | 26 +++++---------
kernel/trace/bpf_trace.c | 82 ++++++++++++++++++++++++++++++++++++++++---
kernel/trace/trace_kprobe.c | 6 ++--
kernel/trace/trace_syscalls.c | 34 ++++++++++--------
kernel/trace/trace_uprobe.c | 3 +-
9 files changed, 245 insertions(+), 56 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 1e334b2..172be7f 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -273,18 +273,38 @@ int bpf_prog_array_length(struct bpf_prog_array __rcu *progs);
int bpf_prog_array_copy_to_user(struct bpf_prog_array __rcu *progs,
__u32 __user *prog_ids, u32 cnt);
-#define BPF_PROG_RUN_ARRAY(array, ctx, func) \
+void bpf_prog_array_delete_safe(struct bpf_prog_array __rcu *progs,
+ struct bpf_prog *old_prog);
+int bpf_prog_array_copy(struct bpf_prog_array __rcu *old_array,
+ struct bpf_prog *exclude_prog,
+ struct bpf_prog *include_prog,
+ struct bpf_prog_array **new_array);
+
+#define __BPF_PROG_RUN_ARRAY(array, ctx, func, check_non_null) \
({ \
- struct bpf_prog **_prog; \
+ struct bpf_prog **_prog, *__prog; \
+ struct bpf_prog_array *_array; \
u32 _ret = 1; \
rcu_read_lock(); \
- _prog = rcu_dereference(array)->progs; \
- for (; *_prog; _prog++) \
- _ret &= func(*_prog, ctx); \
+ _array = rcu_dereference(array); \
+ if (unlikely(check_non_null && !_array))\
+ goto _out; \
+ _prog = _array->progs; \
+ while ((__prog = READ_ONCE(*_prog))) { \
+ _ret &= func(__prog, ctx); \
+ _prog++; \
+ } \
+_out: \
rcu_read_unlock(); \
_ret; \
})
+#define BPF_PROG_RUN_ARRAY(array, ctx, func) \
+ __BPF_PROG_RUN_ARRAY(array, ctx, func, false)
+
+#define BPF_PROG_RUN_ARRAY_CHECK(array, ctx, func) \
+ __BPF_PROG_RUN_ARRAY(array, ctx, func, true)
+
#ifdef CONFIG_BPF_SYSCALL
DECLARE_PER_CPU(int, bpf_prog_active);
diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
index 2e0f222..c811464 100644
--- a/include/linux/trace_events.h
+++ b/include/linux/trace_events.h
@@ -271,14 +271,37 @@ struct trace_event_call {
#ifdef CONFIG_PERF_EVENTS
int perf_refcount;
struct hlist_head __percpu *perf_events;
- struct bpf_prog *prog;
- struct perf_event *bpf_prog_owner;
+ struct bpf_prog_array __rcu *prog_array;
int (*perf_perm)(struct trace_event_call *,
struct perf_event *);
#endif
};
+#ifdef CONFIG_PERF_EVENTS
+static inline bool bpf_prog_array_valid(struct trace_event_call *call)
+{
+ /*
+ * This inline function checks whether call->prog_array
+ * is valid or not. The function is called in various places,
+ * outside rcu_read_lock/unlock, as a heuristic to speed up execution.
+ *
+ * If this function returns true, and later call->prog_array
+ * becomes false inside rcu_read_lock/unlock region,
+ * we bail out then. If this function return false,
+ * there is a risk that we might miss a few events if the checking
+ * were delayed until inside rcu_read_lock/unlock region and
+ * call->prog_array happened to become non-NULL then.
+ *
+ * Here, READ_ONCE() is used instead of rcu_access_pointer().
+ * rcu_access_pointer() requires the actual definition of
+ * "struct bpf_prog_array" while READ_ONCE() only needs
+ * a declaration of the same type.
+ */
+ return !!READ_ONCE(call->prog_array);
+}
+#endif
+
static inline const char *
trace_event_name(struct trace_event_call *call)
{
@@ -435,9 +458,9 @@ trace_trigger_soft_disabled(struct trace_event_file *file)
}
#ifdef CONFIG_BPF_EVENTS
-unsigned int trace_call_bpf(struct bpf_prog *prog, void *ctx);
+unsigned int trace_call_bpf(struct trace_event_call *call, void *ctx);
#else
-static inline unsigned int trace_call_bpf(struct bpf_prog *prog, void *ctx)
+static inline unsigned int trace_call_bpf(struct trace_event_call *call, void *ctx)
{
return 1;
}
@@ -499,6 +522,8 @@ extern void ftrace_profile_free_filter(struct perf_event *event);
void perf_trace_buf_update(void *record, u16 type);
void *perf_trace_buf_alloc(int size, struct pt_regs **regs, int *rctxp);
+int perf_event_attach_bpf_prog(struct perf_event *event, struct bpf_prog *prog);
+void perf_event_detach_bpf_prog(struct perf_event *event);
void perf_trace_run_bpf_submit(void *raw_data, int size, int rctx,
struct trace_event_call *call, u64 count,
struct pt_regs *regs, struct hlist_head *head,
diff --git a/include/trace/perf.h b/include/trace/perf.h
index 04fe68bb..14f127b6 100644
--- a/include/trace/perf.h
+++ b/include/trace/perf.h
@@ -34,7 +34,6 @@ perf_trace_##call(void *__data, proto) \
struct trace_event_call *event_call = __data; \
struct trace_event_data_offsets_##call __maybe_unused __data_offsets;\
struct trace_event_raw_##call *entry; \
- struct bpf_prog *prog = event_call->prog; \
struct pt_regs *__regs; \
u64 __count = 1; \
struct task_struct *__task = NULL; \
@@ -46,8 +45,9 @@ perf_trace_##call(void *__data, proto) \
__data_size = trace_event_get_offsets_##call(&__data_offsets, args); \
\
head = this_cpu_ptr(event_call->perf_events); \
- if (!prog && __builtin_constant_p(!__task) && !__task && \
- hlist_empty(head)) \
+ if (!bpf_prog_array_valid(event_call) && \
+ __builtin_constant_p(!__task) && !__task && \
+ hlist_empty(head)) \
return; \
\
__entry_size = ALIGN(__data_size + sizeof(*entry) + sizeof(u32),\
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 8e7c8bf..7fe4487 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -1394,6 +1394,20 @@ struct bpf_prog *bpf_prog_select_runtime(struct bpf_prog *fp, int *err)
}
EXPORT_SYMBOL_GPL(bpf_prog_select_runtime);
+static unsigned int __bpf_prog_ret1(const void *ctx,
+ const struct bpf_insn *insn)
+{
+ return 1;
+}
+
+static struct bpf_prog_dummy {
+ struct bpf_prog prog;
+} dummy_bpf_prog = {
+ .prog = {
+ .bpf_func = __bpf_prog_ret1,
+ },
+};
+
/* to avoid allocating empty bpf_prog_array for cgroups that
* don't have bpf program attached use one global 'empty_prog_array'
* It will not be modified the caller of bpf_prog_array_alloc()
@@ -1463,6 +1477,73 @@ int bpf_prog_array_copy_to_user(struct bpf_prog_array __rcu *progs,
return 0;
}
+void bpf_prog_array_delete_safe(struct bpf_prog_array __rcu *progs,
+ struct bpf_prog *old_prog)
+{
+ struct bpf_prog **prog = progs->progs;
+
+ for (; *prog; prog++)
+ if (*prog == old_prog) {
+ WRITE_ONCE(*prog, &dummy_bpf_prog.prog);
+ break;
+ }
+}
+
+int bpf_prog_array_copy(struct bpf_prog_array __rcu *old_array,
+ struct bpf_prog *exclude_prog,
+ struct bpf_prog *include_prog,
+ struct bpf_prog_array **new_array)
+{
+ int new_prog_cnt, carry_prog_cnt = 0;
+ struct bpf_prog **existing_prog;
+ struct bpf_prog_array *array;
+ int new_prog_idx = 0;
+
+ /* Figure out how many existing progs we need to carry over to
+ * the new array.
+ */
+ if (old_array) {
+ existing_prog = old_array->progs;
+ for (; *existing_prog; existing_prog++) {
+ if (*existing_prog != exclude_prog &&
+ *existing_prog != &dummy_bpf_prog.prog)
+ carry_prog_cnt++;
+ if (*existing_prog == include_prog)
+ return -EEXIST;
+ }
+ }
+
+ /* How many progs (not NULL) will be in the new array? */
+ new_prog_cnt = carry_prog_cnt;
+ if (include_prog)
+ new_prog_cnt += 1;
+
+ /* Do we have any prog (not NULL) in the new array? */
+ if (!new_prog_cnt) {
+ *new_array = NULL;
+ return 0;
+ }
+
+ /* +1 as the end of prog_array is marked with NULL */
+ array = bpf_prog_array_alloc(new_prog_cnt + 1, GFP_KERNEL);
+ if (!array)
+ return -ENOMEM;
+
+ /* Fill in the new prog array */
+ if (carry_prog_cnt) {
+ existing_prog = old_array->progs;
+ for (; *existing_prog; existing_prog++)
+ if (*existing_prog != exclude_prog &&
+ *existing_prog != &dummy_bpf_prog.prog)
+ array->progs[new_prog_idx++] = *existing_prog;
+ }
+ if (include_prog)
+ array->progs[new_prog_idx++] = include_prog;
+ array->progs[new_prog_idx] = NULL;
+ *new_array = array;
+ return 0;
+}
+
static void bpf_prog_free_deferred(struct work_struct *work)
{
struct bpf_prog_aux *aux;
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 9f78a682..9660ee6 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -7954,11 +7954,9 @@ void perf_trace_run_bpf_submit(void *raw_data, int size, int rctx,
struct pt_regs *regs, struct hlist_head *head,
struct task_struct *task)
{
- struct bpf_prog *prog = call->prog;
-
- if (prog) {
+ if (bpf_prog_array_valid(call)) {
*(struct pt_regs **)raw_data = regs;
- if (!trace_call_bpf(prog, raw_data) || hlist_empty(head)) {
+ if (!trace_call_bpf(call, raw_data) || hlist_empty(head)) {
perf_swevent_put_recursion_context(rctx);
return;
}
@@ -8147,13 +8145,11 @@ static int perf_event_set_bpf_prog(struct perf_event *event, u32 prog_fd)
{
bool is_kprobe, is_tracepoint, is_syscall_tp;
struct bpf_prog *prog;
+ int ret;
if (event->attr.type != PERF_TYPE_TRACEPOINT)
return perf_event_set_bpf_handler(event, prog_fd);
- if (event->tp_event->prog)
- return -EEXIST;
-
is_kprobe = event->tp_event->flags & TRACE_EVENT_FL_UKPROBE;
is_tracepoint = event->tp_event->flags & TRACE_EVENT_FL_TRACEPOINT;
is_syscall_tp = is_syscall_trace_event(event->tp_event);
@@ -8181,26 +8177,20 @@ static int perf_event_set_bpf_prog(struct perf_event *event, u32 prog_fd)
return -EACCES;
}
}
- event->tp_event->prog = prog;
- event->tp_event->bpf_prog_owner = event;
- return 0;
+ ret = perf_event_attach_bpf_prog(event, prog);
+ if (ret)
+ bpf_prog_put(prog);
+ return ret;
}
static void perf_event_free_bpf_prog(struct perf_event *event)
{
- struct bpf_prog *prog;
-
if (event->attr.type != PERF_TYPE_TRACEPOINT) {
perf_event_free_bpf_handler(event);
return;
}
-
- prog = event->tp_event->prog;
- if (prog && event->tp_event->bpf_prog_owner == event) {
- event->tp_event->prog = NULL;
- bpf_prog_put(prog);
- }
+ perf_event_detach_bpf_prog(event);
}
#else
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 3126da2..b65011d 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -17,7 +17,7 @@
/**
* trace_call_bpf - invoke BPF program
- * @prog: BPF program
+ * @call: tracepoint event
* @ctx: opaque context pointer
*
* kprobe handlers execute BPF programs via this helper.
@@ -29,7 +29,7 @@
* 1 - store kprobe event into ring buffer
* Other values are reserved and currently alias to 1
*/
-unsigned int trace_call_bpf(struct bpf_prog *prog, void *ctx)
+unsigned int trace_call_bpf(struct trace_event_call *call, void *ctx)
{
unsigned int ret;
@@ -49,9 +49,22 @@ unsigned int trace_call_bpf(struct bpf_prog *prog, void *ctx)
goto out;
}
- rcu_read_lock();
- ret = BPF_PROG_RUN(prog, ctx);
- rcu_read_unlock();
+ /*
+ * Instead of moving rcu_read_lock/rcu_dereference/rcu_read_unlock
+ * to all call sites, we did a bpf_prog_array_valid() there to check
+ * whether call->prog_array is empty or not, which is
+ * a heurisitc to speed up execution.
+ *
+ * If bpf_prog_array_valid() fetched prog_array was
+ * non-NULL, we go into trace_call_bpf() and do the actual
+ * proper rcu_dereference() under RCU lock.
+ * If it turns out that prog_array is NULL then, we bail out.
+ * For the opposite, if the bpf_prog_array_valid() fetched pointer
+ * was NULL, you'll skip the prog_array with the risk of missing
+ * out of events when it was updated in between this and the
+ * rcu_dereference() which is accepted risk.
+ */
+ ret = BPF_PROG_RUN_ARRAY_CHECK(call->prog_array, ctx, BPF_PROG_RUN);
out:
__this_cpu_dec(bpf_prog_active);
@@ -741,3 +754,62 @@ const struct bpf_verifier_ops perf_event_verifier_ops = {
const struct bpf_prog_ops perf_event_prog_ops = {
};
+
+static DEFINE_MUTEX(bpf_event_mutex);
+
+int perf_event_attach_bpf_prog(struct perf_event *event,
+ struct bpf_prog *prog)
+{
+ struct bpf_prog_array __rcu *old_array;
+ struct bpf_prog_array *new_array;
+ int ret = -EEXIST;
+
+ mutex_lock(&bpf_event_mutex);
+
+ if (event->prog)
+ goto out;
+
+ old_array = rcu_dereference_protected(event->tp_event->prog_array,
+ lockdep_is_held(&bpf_event_mutex));
+ ret = bpf_prog_array_copy(old_array, NULL, prog, &new_array);
+ if (ret < 0)
+ goto out;
+
+ /* set the new array to event->tp_event and set event->prog */
+ event->prog = prog;
+ rcu_assign_pointer(event->tp_event->prog_array, new_array);
+ bpf_prog_array_free(old_array);
+
+out:
+ mutex_unlock(&bpf_event_mutex);
+ return ret;
+}
+
+void perf_event_detach_bpf_prog(struct perf_event *event)
+{
+ struct bpf_prog_array __rcu *old_array;
+ struct bpf_prog_array *new_array;
+ int ret;
+
+ mutex_lock(&bpf_event_mutex);
+
+ if (!event->prog)
+ goto out;
+
+ old_array = rcu_dereference_protected(event->tp_event->prog_array,
+ lockdep_is_held(&bpf_event_mutex));
+
+ ret = bpf_prog_array_copy(old_array, event->prog, NULL, &new_array);
+ if (ret < 0) {
+ bpf_prog_array_delete_safe(old_array, event->prog);
+ } else {
+ rcu_assign_pointer(event->tp_event->prog_array, new_array);
+ bpf_prog_array_free(old_array);
+ }
+
+ bpf_prog_put(event->prog);
+ event->prog = NULL;
+
+out:
+ mutex_unlock(&bpf_event_mutex);
+}
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 8a907e1..abf92e4 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -1174,13 +1174,12 @@ static void
kprobe_perf_func(struct trace_kprobe *tk, struct pt_regs *regs)
{
struct trace_event_call *call = &tk->tp.call;
- struct bpf_prog *prog = call->prog;
struct kprobe_trace_entry_head *entry;
struct hlist_head *head;
int size, __size, dsize;
int rctx;
- if (prog && !trace_call_bpf(prog, regs))
+ if (bpf_prog_array_valid(call) && !trace_call_bpf(call, regs))
return;
head = this_cpu_ptr(call->perf_events);
@@ -1210,13 +1209,12 @@ kretprobe_perf_func(struct trace_kprobe *tk, struct kretprobe_instance *ri,
struct pt_regs *regs)
{
struct trace_event_call *call = &tk->tp.call;
- struct bpf_prog *prog = call->prog;
struct kretprobe_trace_entry_head *entry;
struct hlist_head *head;
int size, __size, dsize;
int rctx;
- if (prog && !trace_call_bpf(prog, regs))
+ if (bpf_prog_array_valid(call) && !trace_call_bpf(call, regs))
return;
head = this_cpu_ptr(call->perf_events);
diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
index 696afe7..71a6af3 100644
--- a/kernel/trace/trace_syscalls.c
+++ b/kernel/trace/trace_syscalls.c
@@ -559,9 +559,10 @@ static DECLARE_BITMAP(enabled_perf_exit_syscalls, NR_syscalls);
static int sys_perf_refcount_enter;
static int sys_perf_refcount_exit;
-static int perf_call_bpf_enter(struct bpf_prog *prog, struct pt_regs *regs,
- struct syscall_metadata *sys_data,
- struct syscall_trace_enter *rec) {
+static int perf_call_bpf_enter(struct trace_event_call *call, struct pt_regs *regs,
+ struct syscall_metadata *sys_data,
+ struct syscall_trace_enter *rec)
+{
struct syscall_tp_t {
unsigned long long regs;
unsigned long syscall_nr;
@@ -573,7 +574,7 @@ static int perf_call_bpf_enter(struct bpf_prog *prog, struct pt_regs *regs,
param.syscall_nr = rec->nr;
for (i = 0; i < sys_data->nb_args; i++)
param.args[i] = rec->args[i];
- return trace_call_bpf(prog, ¶m);
+ return trace_call_bpf(call, ¶m);
}
static void perf_syscall_enter(void *ignore, struct pt_regs *regs, long id)
@@ -581,7 +582,7 @@ static void perf_syscall_enter(void *ignore, struct pt_regs *regs, long id)
struct syscall_metadata *sys_data;
struct syscall_trace_enter *rec;
struct hlist_head *head;
- struct bpf_prog *prog;
+ bool valid_prog_array;
int syscall_nr;
int rctx;
int size;
@@ -596,9 +597,9 @@ static void perf_syscall_enter(void *ignore, struct pt_regs *regs, long id)
if (!sys_data)
return;
- prog = READ_ONCE(sys_data->enter_event->prog);
head = this_cpu_ptr(sys_data->enter_event->perf_events);
- if (!prog && hlist_empty(head))
+ valid_prog_array = bpf_prog_array_valid(sys_data->enter_event);
+ if (!valid_prog_array && hlist_empty(head))
return;
/* get the size after alignment with the u32 buffer size field */
@@ -614,7 +615,8 @@ static void perf_syscall_enter(void *ignore, struct pt_regs *regs, long id)
syscall_get_arguments(current, regs, 0, sys_data->nb_args,
(unsigned long *)&rec->args);
- if ((prog && !perf_call_bpf_enter(prog, regs, sys_data, rec)) ||
+ if ((valid_prog_array &&
+ !perf_call_bpf_enter(sys_data->enter_event, regs, sys_data, rec)) ||
hlist_empty(head)) {
perf_swevent_put_recursion_context(rctx);
return;
@@ -659,8 +661,9 @@ static void perf_sysenter_disable(struct trace_event_call *call)
mutex_unlock(&syscall_trace_lock);
}
-static int perf_call_bpf_exit(struct bpf_prog *prog, struct pt_regs *regs,
- struct syscall_trace_exit *rec) {
+static int perf_call_bpf_exit(struct trace_event_call *call, struct pt_regs *regs,
+ struct syscall_trace_exit *rec)
+{
struct syscall_tp_t {
unsigned long long regs;
unsigned long syscall_nr;
@@ -670,7 +673,7 @@ static int perf_call_bpf_exit(struct bpf_prog *prog, struct pt_regs *regs,
*(struct pt_regs **)¶m = regs;
param.syscall_nr = rec->nr;
param.ret = rec->ret;
- return trace_call_bpf(prog, ¶m);
+ return trace_call_bpf(call, ¶m);
}
static void perf_syscall_exit(void *ignore, struct pt_regs *regs, long ret)
@@ -678,7 +681,7 @@ static void perf_syscall_exit(void *ignore, struct pt_regs *regs, long ret)
struct syscall_metadata *sys_data;
struct syscall_trace_exit *rec;
struct hlist_head *head;
- struct bpf_prog *prog;
+ bool valid_prog_array;
int syscall_nr;
int rctx;
int size;
@@ -693,9 +696,9 @@ static void perf_syscall_exit(void *ignore, struct pt_regs *regs, long ret)
if (!sys_data)
return;
- prog = READ_ONCE(sys_data->exit_event->prog);
head = this_cpu_ptr(sys_data->exit_event->perf_events);
- if (!prog && hlist_empty(head))
+ valid_prog_array = bpf_prog_array_valid(sys_data->exit_event);
+ if (!valid_prog_array && hlist_empty(head))
return;
/* We can probably do that at build time */
@@ -709,7 +712,8 @@ static void perf_syscall_exit(void *ignore, struct pt_regs *regs, long ret)
rec->nr = syscall_nr;
rec->ret = syscall_get_return_value(current, regs);
- if ((prog && !perf_call_bpf_exit(prog, regs, rec)) ||
+ if ((valid_prog_array &&
+ !perf_call_bpf_exit(sys_data->exit_event, regs, rec)) ||
hlist_empty(head)) {
perf_swevent_put_recursion_context(rctx);
return;
diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index 4525e02..153c0e4 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -1113,13 +1113,12 @@ static void __uprobe_perf_func(struct trace_uprobe *tu,
{
struct trace_event_call *call = &tu->tp.call;
struct uprobe_trace_entry_head *entry;
- struct bpf_prog *prog = call->prog;
struct hlist_head *head;
void *data;
int size, esize;
int rctx;
- if (prog && !trace_call_bpf(prog, regs))
+ if (bpf_prog_array_valid(call) && !trace_call_bpf(call, regs))
return;
esize = SIZEOF_TRACE_ENTRY(is_ret_probe(tu));
--
2.9.5
^ permalink raw reply related
* [PATCH net-next v2 0/3] bpf: permit multiple bpf attachments for a single perf tracepoint event
From: Yonghong Song @ 2017-10-23 22:13 UTC (permalink / raw)
To: peterz, rostedt, ast, daniel, kafai, netdev; +Cc: kernel-team
This patch set adds support to permit multiple bpf prog attachments
for a single perf tracepoint event. Patch 1 does some cleanup such
that perf_event_{set|free}_bpf_handler is called under the
same condition. Patch 2 has the core implementation, and
Patch 3 adds a test case.
Changelogs:
v1 -> v2:
. fix a potential deadlock issue discovered by Daniel.
. fix some coding style issues.
Yonghong Song (3):
bpf: use the same condition in perf event set/free bpf handler
bpf: permit multiple bpf attachments for a single perf event
bpf: add a test case to test single tp multiple bpf attachment
include/linux/bpf.h | 30 +++++++++++++---
include/linux/trace_events.h | 33 ++++++++++++++---
include/trace/perf.h | 6 ++--
kernel/bpf/core.c | 81 ++++++++++++++++++++++++++++++++++++++++++
kernel/events/core.c | 30 ++++++----------
kernel/trace/bpf_trace.c | 82 ++++++++++++++++++++++++++++++++++++++++---
kernel/trace/trace_kprobe.c | 6 ++--
kernel/trace/trace_syscalls.c | 34 ++++++++++--------
kernel/trace/trace_uprobe.c | 3 +-
samples/bpf/syscall_tp_user.c | 66 +++++++++++++++++++++++++++-------
10 files changed, 300 insertions(+), 71 deletions(-)
--
2.9.5
^ permalink raw reply
* [PATCH net-next v2 3/3] bpf: add a test case to test single tp multiple bpf attachment
From: Yonghong Song @ 2017-10-23 22:13 UTC (permalink / raw)
To: peterz, rostedt, ast, daniel, kafai, netdev; +Cc: kernel-team
In-Reply-To: <20171023221331.4131164-1-yhs@fb.com>
The bpf sample program syscall_tp is modified to
show attachment of more than bpf programs
for a particular kernel tracepoint.
Signed-off-by: Yonghong Song <yhs@fb.com>
Acked-by: Alexei Starovoitov <ast@kernel.org>
Acked-by: Martin KaFai Lau <kafai@fb.com>
---
samples/bpf/syscall_tp_user.c | 66 ++++++++++++++++++++++++++++++++++---------
1 file changed, 53 insertions(+), 13 deletions(-)
diff --git a/samples/bpf/syscall_tp_user.c b/samples/bpf/syscall_tp_user.c
index a3cb91e..9169d32 100644
--- a/samples/bpf/syscall_tp_user.c
+++ b/samples/bpf/syscall_tp_user.c
@@ -23,6 +23,13 @@
* This requires kernel CONFIG_FTRACE_SYSCALLS to be set.
*/
+static void usage(const char *cmd)
+{
+ printf("USAGE: %s [-i num_progs] [-h]\n", cmd);
+ printf(" -i num_progs # number of progs of the test\n");
+ printf(" -h # help\n");
+}
+
static void verify_map(int map_id)
{
__u32 key = 0;
@@ -32,22 +39,29 @@ static void verify_map(int map_id)
fprintf(stderr, "map_lookup failed: %s\n", strerror(errno));
return;
}
- if (val == 0)
+ if (val == 0) {
fprintf(stderr, "failed: map #%d returns value 0\n", map_id);
+ return;
+ }
+ val = 0;
+ if (bpf_map_update_elem(map_id, &key, &val, BPF_ANY) != 0) {
+ fprintf(stderr, "map_update failed: %s\n", strerror(errno));
+ return;
+ }
}
-int main(int argc, char **argv)
+static int test(char *filename, int num_progs)
{
- struct rlimit r = {RLIM_INFINITY, RLIM_INFINITY};
- char filename[256];
- int fd;
+ int i, fd, map0_fds[num_progs], map1_fds[num_progs];
- setrlimit(RLIMIT_MEMLOCK, &r);
- snprintf(filename, sizeof(filename), "%s_kern.o", argv[0]);
-
- if (load_bpf_file(filename)) {
- fprintf(stderr, "%s", bpf_log_buf);
- return 1;
+ for (i = 0; i < num_progs; i++) {
+ if (load_bpf_file(filename)) {
+ fprintf(stderr, "%s", bpf_log_buf);
+ return 1;
+ }
+ printf("prog #%d: map ids %d %d\n", i, map_fd[0], map_fd[1]);
+ map0_fds[i] = map_fd[0];
+ map1_fds[i] = map_fd[1];
}
/* current load_bpf_file has perf_event_open default pid = -1
@@ -64,8 +78,34 @@ int main(int argc, char **argv)
close(fd);
/* verify the map */
- verify_map(map_fd[0]);
- verify_map(map_fd[1]);
+ for (i = 0; i < num_progs; i++) {
+ verify_map(map0_fds[i]);
+ verify_map(map1_fds[i]);
+ }
return 0;
}
+
+int main(int argc, char **argv)
+{
+ struct rlimit r = {RLIM_INFINITY, RLIM_INFINITY};
+ int opt, num_progs = 1;
+ char filename[256];
+
+ while ((opt = getopt(argc, argv, "i:h")) != -1) {
+ switch (opt) {
+ case 'i':
+ num_progs = atoi(optarg);
+ break;
+ case 'h':
+ default:
+ usage(argv[0]);
+ return 0;
+ }
+ }
+
+ setrlimit(RLIMIT_MEMLOCK, &r);
+ snprintf(filename, sizeof(filename), "%s_kern.o", argv[0]);
+
+ return test(filename, num_progs);
+}
--
2.9.5
^ permalink raw reply related
* [PATCH net-next v2 1/3] bpf: use the same condition in perf event set/free bpf handler
From: Yonghong Song @ 2017-10-23 22:13 UTC (permalink / raw)
To: peterz, rostedt, ast, daniel, kafai, netdev; +Cc: kernel-team
In-Reply-To: <20171023221331.4131164-1-yhs@fb.com>
This is a cleanup such that doing the same check in
perf_event_free_bpf_prog as we already do in
perf_event_set_bpf_prog step.
Signed-off-by: Yonghong Song <yhs@fb.com>
Acked-by: Alexei Starovoitov <ast@kernel.org>
Acked-by: Martin KaFai Lau <kafai@fb.com>
---
kernel/events/core.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 31ee304..9f78a682 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -8191,10 +8191,10 @@ static void perf_event_free_bpf_prog(struct perf_event *event)
{
struct bpf_prog *prog;
- perf_event_free_bpf_handler(event);
-
- if (!event->tp_event)
+ if (event->attr.type != PERF_TYPE_TRACEPOINT) {
+ perf_event_free_bpf_handler(event);
return;
+ }
prog = event->tp_event->prog;
if (prog && event->tp_event->bpf_prog_owner == event) {
--
2.9.5
^ permalink raw reply related
* Re: [PATCH v8 02/10] dt-bindings: net: dwmac-sun8i: update documentation about integrated PHY
From: Rob Herring @ 2017-10-23 22:06 UTC (permalink / raw)
To: Corentin Labbe
Cc: mark.rutland, maxime.ripard, wens, linux, catalin.marinas,
will.deacon, peppe.cavallaro, alexandre.torgue, andrew,
f.fainelli, netdev, devicetree, linux-arm-kernel, linux-kernel
In-Reply-To: <20171023185626.31793-3-clabbe.montjoie@gmail.com>
On Mon, Oct 23, 2017 at 08:56:18PM +0200, Corentin Labbe wrote:
> This patch add documentation about the MDIO switch used on sun8i-h3-emac
> for integrated PHY.
>
> Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
> ---
> .../devicetree/bindings/net/dwmac-sun8i.txt | 139 +++++++++++++++++++--
> 1 file changed, 127 insertions(+), 12 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
> index 725f3b187886..3e37db10fa02 100644
> --- a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
> +++ b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
> @@ -4,18 +4,18 @@ This device is a platform glue layer for stmmac.
> Please see stmmac.txt for the other unchanged properties.
>
> Required properties:
> -- compatible: should be one of the following string:
> +- compatible: must be one of the following string:
> "allwinner,sun8i-a83t-emac"
> "allwinner,sun8i-h3-emac"
> "allwinner,sun8i-v3s-emac"
> "allwinner,sun50i-a64-emac"
> - reg: address and length of the register for the device.
> - interrupts: interrupt for the device
> -- interrupt-names: should be "macirq"
> +- interrupt-names: must be "macirq"
> - clocks: A phandle to the reference clock for this device
> -- clock-names: should be "stmmaceth"
> +- clock-names: must be "stmmaceth"
> - resets: A phandle to the reset control for this device
> -- reset-names: should be "stmmaceth"
> +- reset-names: must be "stmmaceth"
> - phy-mode: See ethernet.txt
> - phy-handle: See ethernet.txt
> - #address-cells: shall be 1
> @@ -39,23 +39,38 @@ Optional properties for the following compatibles:
> - allwinner,leds-active-low: EPHY LEDs are active low
>
> Required child node of emac:
> -- mdio bus node: should be named mdio
> +- mdio bus node: with compatible "snps,dwmac-mdio"
>
> Required properties of the mdio node:
> - #address-cells: shall be 1
> - #size-cells: shall be 0
>
> -The device node referenced by "phy" or "phy-handle" should be a child node
> +The device node referenced by "phy" or "phy-handle" must be a child node
> of the mdio node. See phy.txt for the generic PHY bindings.
>
> -Required properties of the phy node with the following compatibles:
> +The following compatibles require that the emac node have a mdio-mux child
> +node called "mdio-mux":
> + - "allwinner,sun8i-h3-emac"
> + - "allwinner,sun8i-v3s-emac":
> +Required properties for the mdio-mux node:
> + - compatible = "mdio-mux", "allwinner,sun8i-h3-mdio-mux"
I think you can drop mdio-mux. They are in the wrong order anyway.
> + - one child mdio for the integrated mdio with the compatible
> + "allwinner,sun8i-h3-mdio-internal"
> + - one child mdio for the external mdio if present (V3s have none)
> +Required properties for the mdio-mux children node:
> + - reg: 1 for internal MDIO bus, 2 for external MDIO bus
> +
> +The following compatibles require a PHY node representing the integrated
> +PHY, under the integrated MDIO bus node if an mdio-mux node is used:
> - "allwinner,sun8i-h3-emac",
> - "allwinner,sun8i-v3s-emac":
Please refer to mdio-mux.txt somewhere in here.
> +
> +Required properties of the integrated phy node:
> - clocks: a phandle to the reference clock for the EPHY
> - resets: a phandle to the reset control for the EPHY
> +- Must be a child of the integrated mdio
>
> -Example:
> -
> +Example with integrated PHY:
> emac: ethernet@1c0b000 {
> compatible = "allwinner,sun8i-h3-emac";
> syscon = <&syscon>;
> @@ -72,13 +87,113 @@ emac: ethernet@1c0b000 {
> phy-handle = <&int_mii_phy>;
> phy-mode = "mii";
> allwinner,leds-active-low;
> +
> + mdio0: mdio {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + compatible = "snps,dwmac-mdio";
> + };
> +
> + mdio-mux {
> + compatible = "mdio-mux", "allwinner,sun8i-h3-mdio-mux";
To be compliant with the mdio-mux binding, you need a mdio-parent-bus
property. Though that property is optional, so maybe the mdio-mux
binding just needs to say when it is optional. Presumably when the
parent is the mdio bus controller.
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + int_mdio: mdio@1 {
> + compatible = "allwinner,sun8i-h3-mdio-internal";
> + reg = <1>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + int_mii_phy: ethernet-phy@1 {
> + reg = <1>;
> + clocks = <&ccu CLK_BUS_EPHY>;
> + resets = <&ccu RST_BUS_EPHY>;
> + phy-is-integrated;
> + };
> + };
> + ext_mdio: mdio@2 {
> + reg = <2>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + };
> + };
> +};
> +
> +Example with external PHY:
> +emac: ethernet@1c0b000 {
> + compatible = "allwinner,sun8i-h3-emac";
> + syscon = <&syscon>;
> + reg = <0x01c0b000 0x104>;
> + interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-names = "macirq";
> + resets = <&ccu RST_BUS_EMAC>;
> + reset-names = "stmmaceth";
> + clocks = <&ccu CLK_BUS_EMAC>;
> + clock-names = "stmmaceth";
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + phy-handle = <&ext_rgmii_phy>;
> + phy-mode = "rgmii";
> + allwinner,leds-active-low;
> +
> + mdio0: mdio {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + compatible = "snps,dwmac-mdio";
> + };
> +
> + mdio-mux {
> + compatible = "mdio-mux", "allwinner,sun8i-h3-mdio-mux";
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + int_mdio: mdio@1 {
> + compatible = "allwinner,sun8i-h3-mdio-internal";
> + reg = <1>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + int_mii_phy: ethernet-phy@1 {
> + reg = <1>;
> + clocks = <&ccu CLK_BUS_EPHY>;
> + resets = <&ccu RST_BUS_EPHY>;
> + };
> + };
> + ext_mdio: mdio@2 {
> + reg = <2>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + ext_rgmii_phy: ethernet-phy@1 {
> + reg = <1>;
> + };
> + }:
> + };
> +};
> +
> +Example with SoC without integrated PHY
> +
> +emac: ethernet@1c0b000 {
> + compatible = "allwinner,sun8i-a83t-emac";
> + syscon = <&syscon>;
> + reg = <0x01c0b000 0x104>;
> + interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-names = "macirq";
> + resets = <&ccu RST_BUS_EMAC>;
> + reset-names = "stmmaceth";
> + clocks = <&ccu CLK_BUS_EMAC>;
> + clock-names = "stmmaceth";
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + phy-handle = <&ext_rgmii_phy>;
> + phy-mode = "rgmii";
> +
> mdio: mdio {
> + compatible = "snps,dwmac-mdio";
> #address-cells = <1>;
> #size-cells = <0>;
> - int_mii_phy: ethernet-phy@1 {
> + ext_rgmii_phy: ethernet-phy@1 {
> reg = <1>;
> - clocks = <&ccu CLK_BUS_EPHY>;
> - resets = <&ccu RST_BUS_EPHY>;
> };
> };
> };
> --
> 2.13.6
>
^ permalink raw reply
* [Patch net 15/15] selftests: Introduce a new test case to tc testsuite
From: Cong Wang @ 2017-10-23 22:03 UTC (permalink / raw)
To: netdev; +Cc: paulmck, jhs, john.fastabend, Chris Mi, Cong Wang
In-Reply-To: <20171023220304.2268-1-xiyou.wangcong@gmail.com>
From: Chris Mi <chrism@mellanox.com>
From: Chris Mi <chrism@mellanox.com>
In this patchset, we fixed a tc bug. This patch adds the test case
that reproduces the bug. To run this test case, user should specify
an existing NIC device:
# sudo ./tdc.py -d enp4s0f0
This test case belongs to category "flower". If user doesn't specify
a NIC device, the test cases belong to "flower" will not be run.
In this test case, we create 1M filters and all filters share the same
action. When destroying all filters, kernel should not panic. It takes
about 18s to run it.
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
Acked-by: Lucas Bates <lucasb@mojatatu.com>
Signed-off-by: Chris Mi <chrism@mellanox.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
.../tc-testing/tc-tests/filters/tests.json | 23 +++++++++++++++++++++-
tools/testing/selftests/tc-testing/tdc.py | 20 +++++++++++++++----
tools/testing/selftests/tc-testing/tdc_config.py | 2 ++
3 files changed, 40 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/tc-testing/tc-tests/filters/tests.json b/tools/testing/selftests/tc-testing/tc-tests/filters/tests.json
index c727b96a59b0..5fa02d86b35f 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/filters/tests.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/filters/tests.json
@@ -17,5 +17,26 @@
"teardown": [
"$TC qdisc del dev $DEV1 ingress"
]
+ },
+ {
+ "id": "d052",
+ "name": "Add 1M filters with the same action",
+ "category": [
+ "filter",
+ "flower"
+ ],
+ "setup": [
+ "$TC qdisc add dev $DEV2 ingress",
+ "./tdc_batch.py $DEV2 $BATCH_FILE --share_action -n 1000000"
+ ],
+ "cmdUnderTest": "$TC -b $BATCH_FILE",
+ "expExitCode": "0",
+ "verifyCmd": "$TC actions list action gact",
+ "matchPattern": "action order 0: gact action drop.*index 1 ref 1000000 bind 1000000",
+ "matchCount": "1",
+ "teardown": [
+ "$TC qdisc del dev $DEV2 ingress",
+ "/bin/rm $BATCH_FILE"
+ ]
}
-]
\ No newline at end of file
+]
diff --git a/tools/testing/selftests/tc-testing/tdc.py b/tools/testing/selftests/tc-testing/tdc.py
index cd61b7844c0d..5f11f5d7456e 100755
--- a/tools/testing/selftests/tc-testing/tdc.py
+++ b/tools/testing/selftests/tc-testing/tdc.py
@@ -88,7 +88,7 @@ USE_NS = True
exit(1)
-def test_runner(filtered_tests):
+def test_runner(filtered_tests, args):
"""
Driver function for the unit tests.
@@ -105,6 +105,8 @@ USE_NS = True
for tidx in testlist:
result = True
tresult = ""
+ if "flower" in tidx["category"] and args.device == None:
+ continue
print("Test " + tidx["id"] + ": " + tidx["name"])
prepare_env(tidx["setup"])
(p, procout) = exec_cmd(tidx["cmdUnderTest"])
@@ -152,6 +154,10 @@ USE_NS = True
exec_cmd(cmd, False)
cmd = 'ip -s $NS link set $DEV1 up'
exec_cmd(cmd, False)
+ cmd = 'ip link set $DEV2 netns $NS'
+ exec_cmd(cmd, False)
+ cmd = 'ip -s $NS link set $DEV2 up'
+ exec_cmd(cmd, False)
def ns_destroy():
@@ -211,7 +217,8 @@ USE_NS = True
help='Execute the single test case with specified ID')
parser.add_argument('-i', '--id', action='store_true', dest='gen_id',
help='Generate ID numbers for new test cases')
- return parser
+ parser.add_argument('-d', '--device',
+ help='Execute the test case in flower category')
return parser
@@ -225,6 +232,8 @@ USE_NS = True
if args.path != None:
NAMES['TC'] = args.path
+ if args.device != None:
+ NAMES['DEV2'] = args.device
if not os.path.isfile(NAMES['TC']):
print("The specified tc path " + NAMES['TC'] + " does not exist.")
exit(1)
@@ -381,14 +390,17 @@ USE_NS = True
if (len(alltests) == 0):
print("Cannot find a test case with ID matching " + target_id)
exit(1)
- catresults = test_runner(alltests)
+ catresults = test_runner(alltests, args)
print("All test results: " + "\n\n" + catresults)
elif (len(target_category) > 0):
+ if (target_category == "flower") and args.device == None:
+ print("Please specify a NIC device (-d) to run category flower")
+ exit(1)
if (target_category not in ucat):
print("Specified category is not present in this file.")
exit(1)
else:
- catresults = test_runner(testcases[target_category])
+ catresults = test_runner(testcases[target_category], args)
print("Category " + target_category + "\n\n" + catresults)
ns_destroy()
diff --git a/tools/testing/selftests/tc-testing/tdc_config.py b/tools/testing/selftests/tc-testing/tdc_config.py
index 01087375a7c3..b6352515c1b5 100644
--- a/tools/testing/selftests/tc-testing/tdc_config.py
+++ b/tools/testing/selftests/tc-testing/tdc_config.py
@@ -12,6 +12,8 @@ NAMES = {
# Name of veth devices to be created for the namespace
'DEV0': 'v0p0',
'DEV1': 'v0p1',
+ 'DEV2': '',
+ 'BATCH_FILE': './batch.txt',
# Name of the namespace to use
'NS': 'tcut'
}
--
2.13.0
^ permalink raw reply related
* [Patch net 14/15] selftests: Introduce a new script to generate tc batch file
From: Cong Wang @ 2017-10-23 22:03 UTC (permalink / raw)
To: netdev; +Cc: paulmck, jhs, john.fastabend, Chris Mi, Cong Wang
In-Reply-To: <20171023220304.2268-1-xiyou.wangcong@gmail.com>
From: Chris Mi <chrism@mellanox.com>
From: Chris Mi <chrism@mellanox.com>
# ./tdc_batch.py -h
usage: tdc_batch.py [-h] [-n NUMBER] [-o] [-s] [-p] device file
TC batch file generator
positional arguments:
device device name
file batch file name
optional arguments:
-h, --help show this help message and exit
-n NUMBER, --number NUMBER
how many lines in batch file
-o, --skip_sw skip_sw (offload), by default skip_hw
-s, --share_action all filters share the same action
-p, --prio all filters have different prio
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
Acked-by: Lucas Bates <lucasb@mojatatu.com>
Signed-off-by: Chris Mi <chrism@mellanox.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
tools/testing/selftests/tc-testing/tdc_batch.py | 62 +++++++++++++++++++++++++
1 file changed, 62 insertions(+)
create mode 100644 tools/testing/selftests/tc-testing/tdc_batch.py
diff --git a/tools/testing/selftests/tc-testing/tdc_batch.py b/tools/testing/selftests/tc-testing/tdc_batch.py
new file mode 100644
index 000000000000..707c6bfef689
--- /dev/null
+++ b/tools/testing/selftests/tc-testing/tdc_batch.py
@@ -0,0 +1,62 @@
+#!/usr/bin/python3
+
+"""
+tdc_batch.py - a script to generate TC batch file
+
+Copyright (C) 2017 Chris Mi <chrism@mellanox.com>
+"""
+
+import argparse
+
+parser = argparse.ArgumentParser(description='TC batch file generator')
+parser.add_argument("device", help="device name")
+parser.add_argument("file", help="batch file name")
+parser.add_argument("-n", "--number", type=int,
+ help="how many lines in batch file")
+parser.add_argument("-o", "--skip_sw",
+ help="skip_sw (offload), by default skip_hw",
+ action="store_true")
+parser.add_argument("-s", "--share_action",
+ help="all filters share the same action",
+ action="store_true")
+parser.add_argument("-p", "--prio",
+ help="all filters have different prio",
+ action="store_true")
+args = parser.parse_args()
+
+device = args.device
+file = open(args.file, 'w')
+
+number = 1
+if args.number:
+ number = args.number
+
+skip = "skip_hw"
+if args.skip_sw:
+ skip = "skip_sw"
+
+share_action = ""
+if args.share_action:
+ share_action = "index 1"
+
+prio = "prio 1"
+if args.prio:
+ prio = ""
+ if number > 0x4000:
+ number = 0x4000
+
+index = 0
+for i in range(0x100):
+ for j in range(0x100):
+ for k in range(0x100):
+ mac = ("%02x:%02x:%02x" % (i, j, k))
+ src_mac = "e4:11:00:" + mac
+ dst_mac = "e4:12:00:" + mac
+ cmd = ("filter add dev %s %s protocol ip parent ffff: flower %s "
+ "src_mac %s dst_mac %s action drop %s" %
+ (device, prio, skip, src_mac, dst_mac, share_action))
+ file.write("%s\n" % cmd)
+ index += 1
+ if index >= number:
+ file.close()
+ exit(0)
--
2.13.0
^ permalink raw reply related
* [Patch net 13/15] net_sched: add rtnl assertion to tcf_exts_destroy()
From: Cong Wang @ 2017-10-23 22:03 UTC (permalink / raw)
To: netdev; +Cc: paulmck, jhs, john.fastabend, Chris Mi, Cong Wang, Jiri Pirko
In-Reply-To: <20171023220304.2268-1-xiyou.wangcong@gmail.com>
After previous patches, it is now safe to claim that
tcf_exts_destroy() is always called with RTNL lock.
Cc: Jiri Pirko <jiri@resnulli.us>
Cc: John Fastabend <john.fastabend@gmail.com>
Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
net/sched/cls_api.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 0b2219adf520..c48482942995 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -879,6 +879,7 @@ void tcf_exts_destroy(struct tcf_exts *exts)
#ifdef CONFIG_NET_CLS_ACT
LIST_HEAD(actions);
+ ASSERT_RTNL();
tcf_exts_to_list(exts, &actions);
tcf_action_destroy(&actions, TCA_ACT_UNBIND);
kfree(exts->actions);
--
2.13.0
^ permalink raw reply related
* [Patch net 12/15] net_sched: remove RCU callbacks in sample action
From: Cong Wang @ 2017-10-23 22:03 UTC (permalink / raw)
To: netdev
Cc: paulmck, jhs, john.fastabend, Chris Mi, Cong Wang, Yotam Gigi,
Jiri Pirko
In-Reply-To: <20171023220304.2268-1-xiyou.wangcong@gmail.com>
The ->cleanup() now can block, it is okay to change
it to synchronize_rcu(). This could also fix a use-after-free
in module unloading.
Cc: Yotam Gigi <yotamg@mellanox.com>
Cc: Jiri Pirko <jiri@resnulli.us>
Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
include/net/tc_act/tc_sample.h | 1 -
net/sched/act_sample.c | 12 +++---------
2 files changed, 3 insertions(+), 10 deletions(-)
diff --git a/include/net/tc_act/tc_sample.h b/include/net/tc_act/tc_sample.h
index 89e9305be880..eb44804ff5f0 100644
--- a/include/net/tc_act/tc_sample.h
+++ b/include/net/tc_act/tc_sample.h
@@ -13,7 +13,6 @@ struct tcf_sample {
struct psample_group __rcu *psample_group;
u32 psample_group_num;
struct list_head tcfm_list;
- struct rcu_head rcu;
};
#define to_sample(a) ((struct tcf_sample *)a)
diff --git a/net/sched/act_sample.c b/net/sched/act_sample.c
index ec986ae52808..b8863e3f6776 100644
--- a/net/sched/act_sample.c
+++ b/net/sched/act_sample.c
@@ -96,23 +96,17 @@ static int tcf_sample_init(struct net *net, struct nlattr *nla,
return ret;
}
-static void tcf_sample_cleanup_rcu(struct rcu_head *rcu)
+static void tcf_sample_cleanup(struct tc_action *a, int bind)
{
- struct tcf_sample *s = container_of(rcu, struct tcf_sample, rcu);
+ struct tcf_sample *s = to_sample(a);
struct psample_group *psample_group;
+ synchronize_rcu();
psample_group = rcu_dereference_protected(s->psample_group, 1);
RCU_INIT_POINTER(s->psample_group, NULL);
psample_group_put(psample_group);
}
-static void tcf_sample_cleanup(struct tc_action *a, int bind)
-{
- struct tcf_sample *s = to_sample(a);
-
- call_rcu(&s->rcu, tcf_sample_cleanup_rcu);
-}
-
static bool tcf_sample_dev_ok_push(struct net_device *dev)
{
switch (dev->type) {
--
2.13.0
^ permalink raw reply related
* [Patch net 11/15] net_sched: remove RCU callbacks in route filter
From: Cong Wang @ 2017-10-23 22:03 UTC (permalink / raw)
To: netdev
Cc: paulmck, jhs, john.fastabend, Chris Mi, Cong Wang,
Daniel Borkmann, Jiri Pirko
In-Reply-To: <20171023220304.2268-1-xiyou.wangcong@gmail.com>
Replace call_rcu() with synchronize_rcu().
Similarly, route4_destroy() is special here, because
it deletes all elements from a hashtable, we definitely
don't want to wait for each element, however we can
unpublish the whole hashtable from upper layer first,
so that after one grace period, the whole hashtable is
safe to remove too.
Reported-by: Chris Mi <chrism@mellanox.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Jiri Pirko <jiri@resnulli.us>
Cc: John Fastabend <john.fastabend@gmail.com>
Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
net/sched/cls_route.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/net/sched/cls_route.c b/net/sched/cls_route.c
index 9ddde65915d2..b9b77baac1d2 100644
--- a/net/sched/cls_route.c
+++ b/net/sched/cls_route.c
@@ -38,7 +38,6 @@ struct route4_fastmap {
struct route4_head {
struct route4_fastmap fastmap[16];
struct route4_bucket __rcu *table[256 + 1];
- struct rcu_head rcu;
};
struct route4_bucket {
@@ -57,7 +56,6 @@ struct route4_filter {
u32 handle;
struct route4_bucket *bkt;
struct tcf_proto *tp;
- struct rcu_head rcu;
};
#define ROUTE4_FAILURE ((struct route4_filter *)(-1L))
@@ -254,10 +252,8 @@ static int route4_init(struct tcf_proto *tp)
return 0;
}
-static void route4_delete_filter(struct rcu_head *head)
+static void route4_delete_filter(struct route4_filter *f)
{
- struct route4_filter *f = container_of(head, struct route4_filter, rcu);
-
tcf_exts_destroy(&f->exts);
kfree(f);
}
@@ -270,6 +266,9 @@ static void route4_destroy(struct tcf_proto *tp)
if (head == NULL)
return;
+ RCU_INIT_POINTER(tp->root, NULL);
+ synchronize_rcu();
+
for (h1 = 0; h1 <= 256; h1++) {
struct route4_bucket *b;
@@ -284,14 +283,14 @@ static void route4_destroy(struct tcf_proto *tp)
next = rtnl_dereference(f->next);
RCU_INIT_POINTER(b->ht[h2], next);
tcf_unbind_filter(tp, &f->res);
- call_rcu(&f->rcu, route4_delete_filter);
+ route4_delete_filter(f);
}
}
RCU_INIT_POINTER(head->table[h1], NULL);
- kfree_rcu(b, rcu);
+ kfree(b);
}
}
- kfree_rcu(head, rcu);
+ kfree(head);
}
static int route4_delete(struct tcf_proto *tp, void *arg, bool *last)
@@ -325,7 +324,8 @@ static int route4_delete(struct tcf_proto *tp, void *arg, bool *last)
/* Delete it */
tcf_unbind_filter(tp, &f->res);
- call_rcu(&f->rcu, route4_delete_filter);
+ synchronize_rcu();
+ route4_delete_filter(f);
/* Strip RTNL protected tree */
for (i = 0; i <= 32; i++) {
@@ -528,7 +528,8 @@ static int route4_change(struct net *net, struct sk_buff *in_skb,
*arg = f;
if (fold) {
tcf_unbind_filter(tp, &fold->res);
- call_rcu(&fold->rcu, route4_delete_filter);
+ synchronize_rcu();
+ route4_delete_filter(fold);
}
return 0;
--
2.13.0
^ permalink raw reply related
* [Patch net 10/15] net_sched: remove RCU callbacks in fw filter
From: Cong Wang @ 2017-10-23 22:02 UTC (permalink / raw)
To: netdev
Cc: paulmck, jhs, john.fastabend, Chris Mi, Cong Wang,
Daniel Borkmann, Jiri Pirko
In-Reply-To: <20171023220304.2268-1-xiyou.wangcong@gmail.com>
Replace call_rcu() with synchronize_rcu().
Similarly, fw_destroy() is special here, because
it deletes all elements from a hashtable, we definitely
don't want to wait for each element, however we can
unpublish the whole hashtable from upper layer first,
so that after one grace period, the whole hashtable is
safe to remove too.
Reported-by: Chris Mi <chrism@mellanox.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Jiri Pirko <jiri@resnulli.us>
Cc: John Fastabend <john.fastabend@gmail.com>
Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
net/sched/cls_fw.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/net/sched/cls_fw.c b/net/sched/cls_fw.c
index 941245ad07fd..2e7171d3da6d 100644
--- a/net/sched/cls_fw.c
+++ b/net/sched/cls_fw.c
@@ -46,7 +46,6 @@ struct fw_filter {
#endif /* CONFIG_NET_CLS_IND */
struct tcf_exts exts;
struct tcf_proto *tp;
- struct rcu_head rcu;
};
static u32 fw_hash(u32 handle)
@@ -119,10 +118,8 @@ static int fw_init(struct tcf_proto *tp)
return 0;
}
-static void fw_delete_filter(struct rcu_head *head)
+static void fw_delete_filter(struct fw_filter *f )
{
- struct fw_filter *f = container_of(head, struct fw_filter, rcu);
-
tcf_exts_destroy(&f->exts);
kfree(f);
}
@@ -136,15 +133,18 @@ static void fw_destroy(struct tcf_proto *tp)
if (head == NULL)
return;
+ RCU_INIT_POINTER(tp->root, NULL);
+ synchronize_rcu();
+
for (h = 0; h < HTSIZE; h++) {
while ((f = rtnl_dereference(head->ht[h])) != NULL) {
RCU_INIT_POINTER(head->ht[h],
rtnl_dereference(f->next));
tcf_unbind_filter(tp, &f->res);
- call_rcu(&f->rcu, fw_delete_filter);
+ fw_delete_filter(f);
}
}
- kfree_rcu(head, rcu);
+ kfree(head);
}
static int fw_delete(struct tcf_proto *tp, void *arg, bool *last)
@@ -166,7 +166,8 @@ static int fw_delete(struct tcf_proto *tp, void *arg, bool *last)
if (pfp == f) {
RCU_INIT_POINTER(*fp, rtnl_dereference(f->next));
tcf_unbind_filter(tp, &f->res);
- call_rcu(&f->rcu, fw_delete_filter);
+ synchronize_rcu();
+ fw_delete_filter(f);
ret = 0;
break;
}
@@ -286,7 +287,8 @@ static int fw_change(struct net *net, struct sk_buff *in_skb,
RCU_INIT_POINTER(fnew->next, rtnl_dereference(pfp->next));
rcu_assign_pointer(*fp, fnew);
tcf_unbind_filter(tp, &f->res);
- call_rcu(&f->rcu, fw_delete_filter);
+ synchronize_rcu();
+ fw_delete_filter(f);
*arg = fnew;
return err;
--
2.13.0
^ permalink raw reply related
* [Patch net 09/15] net_sched: remove RCU callbacks in u32 filter
From: Cong Wang @ 2017-10-23 22:02 UTC (permalink / raw)
To: netdev
Cc: paulmck, jhs, john.fastabend, Chris Mi, Cong Wang,
Daniel Borkmann, Jiri Pirko
In-Reply-To: <20171023220304.2268-1-xiyou.wangcong@gmail.com>
Replace call_rcu() with synchronize_rcu().
Note, u32_clear_hnode() is very special here, because
it deletes all elements from a hashtable, we definitely
don't want to wait for each element, however we can
unpublish the whole hashtable from upper layer first,
so that after one grace period, the whole hashtable is
safe to remove too.
Reported-by: Chris Mi <chrism@mellanox.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Jiri Pirko <jiri@resnulli.us>
Cc: John Fastabend <john.fastabend@gmail.com>
Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
net/sched/cls_u32.c | 64 ++++++++++++++---------------------------------------
1 file changed, 17 insertions(+), 47 deletions(-)
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index 10b8d851fc6b..393c0aead78a 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -68,7 +68,6 @@ struct tc_u_knode {
u32 __percpu *pcpu_success;
#endif
struct tcf_proto *tp;
- struct rcu_head rcu;
/* The 'sel' field MUST be the last field in structure to allow for
* tc_u32_keys allocated at end of structure.
*/
@@ -82,7 +81,6 @@ struct tc_u_hnode {
struct tc_u_common *tp_c;
int refcnt;
unsigned int divisor;
- struct rcu_head rcu;
/* The 'ht' field MUST be the last field in structure to allow for
* more entries allocated at end of structure.
*/
@@ -95,7 +93,6 @@ struct tc_u_common {
int refcnt;
u32 hgenerator;
struct hlist_node hnode;
- struct rcu_head rcu;
};
static inline unsigned int u32_hash_fold(__be32 key,
@@ -410,35 +407,6 @@ static int u32_destroy_key(struct tcf_proto *tp, struct tc_u_knode *n,
return 0;
}
-/* u32_delete_key_rcu should be called when free'ing a copied
- * version of a tc_u_knode obtained from u32_init_knode(). When
- * copies are obtained from u32_init_knode() the statistics are
- * shared between the old and new copies to allow readers to
- * continue to update the statistics during the copy. To support
- * this the u32_delete_key_rcu variant does not free the percpu
- * statistics.
- */
-static void u32_delete_key_rcu(struct rcu_head *rcu)
-{
- struct tc_u_knode *key = container_of(rcu, struct tc_u_knode, rcu);
-
- u32_destroy_key(key->tp, key, false);
-}
-
-/* u32_delete_key_freepf_rcu is the rcu callback variant
- * that free's the entire structure including the statistics
- * percpu variables. Only use this if the key is not a copy
- * returned by u32_init_knode(). See u32_delete_key_rcu()
- * for the variant that should be used with keys return from
- * u32_init_knode()
- */
-static void u32_delete_key_freepf_rcu(struct rcu_head *rcu)
-{
- struct tc_u_knode *key = container_of(rcu, struct tc_u_knode, rcu);
-
- u32_destroy_key(key->tp, key, true);
-}
-
static int u32_delete_key(struct tcf_proto *tp, struct tc_u_knode *key)
{
struct tc_u_knode __rcu **kp;
@@ -453,7 +421,8 @@ static int u32_delete_key(struct tcf_proto *tp, struct tc_u_knode *key)
RCU_INIT_POINTER(*kp, key->next);
tcf_unbind_filter(tp, &key->res);
- call_rcu(&key->rcu, u32_delete_key_freepf_rcu);
+ synchronize_rcu();
+ u32_destroy_key(key->tp, key, true);
return 0;
}
}
@@ -565,7 +534,7 @@ static void u32_clear_hnode(struct tcf_proto *tp, struct tc_u_hnode *ht)
rtnl_dereference(n->next));
tcf_unbind_filter(tp, &n->res);
u32_remove_hw_knode(tp, n->handle);
- call_rcu(&n->rcu, u32_delete_key_freepf_rcu);
+ u32_destroy_key(n->tp, n, true);
}
}
}
@@ -578,8 +547,6 @@ static int u32_destroy_hnode(struct tcf_proto *tp, struct tc_u_hnode *ht)
WARN_ON(ht->refcnt);
- u32_clear_hnode(tp, ht);
-
hn = &tp_c->hlist;
for (phn = rtnl_dereference(*hn);
phn;
@@ -587,7 +554,10 @@ static int u32_destroy_hnode(struct tcf_proto *tp, struct tc_u_hnode *ht)
if (phn == ht) {
u32_clear_hw_hnode(tp, ht);
RCU_INIT_POINTER(*hn, ht->next);
- kfree_rcu(ht, rcu);
+ synchronize_rcu();
+
+ u32_clear_hnode(tp, ht);
+ kfree(ht);
return 0;
}
}
@@ -617,20 +587,19 @@ static void u32_destroy(struct tcf_proto *tp)
u32_destroy_hnode(tp, root_ht);
if (--tp_c->refcnt == 0) {
- struct tc_u_hnode *ht;
+ struct tc_u_hnode *ht, *tmp;
hlist_del(&tp_c->hnode);
- for (ht = rtnl_dereference(tp_c->hlist);
- ht;
- ht = rtnl_dereference(ht->next)) {
+ tmp = rtnl_dereference(tp_c->hlist);
+ RCU_INIT_POINTER(tp_c->hlist, NULL);
+ synchronize_rcu();
+
+ while ((ht = tmp) != NULL) {
+ tmp = rtnl_dereference(ht->next);
ht->refcnt--;
u32_clear_hnode(tp, ht);
- }
-
- while ((ht = rtnl_dereference(tp_c->hlist)) != NULL) {
- RCU_INIT_POINTER(tp_c->hlist, ht->next);
- kfree_rcu(ht, rcu);
+ kfree(ht);
}
kfree(tp_c);
@@ -926,7 +895,8 @@ static int u32_change(struct net *net, struct sk_buff *in_skb,
u32_replace_knode(tp, tp_c, new);
tcf_unbind_filter(tp, &n->res);
- call_rcu(&n->rcu, u32_delete_key_rcu);
+ synchronize_rcu();
+ u32_destroy_key(n->tp, n, false);
return 0;
}
--
2.13.0
^ permalink raw reply related
* [Patch net 08/15] net_sched: remove RCU callbacks in tcindex filter
From: Cong Wang @ 2017-10-23 22:02 UTC (permalink / raw)
To: netdev; +Cc: paulmck, jhs, john.fastabend, Chris Mi, Cong Wang,
Daniel Borkmann
In-Reply-To: <20171023220304.2268-1-xiyou.wangcong@gmail.com>
Replace call_rcu() with synchronize_rcu().
Reported-by: Chris Mi <chrism@mellanox.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: John Fastabend <john.fastabend@gmail.com>
Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
net/sched/cls_tcindex.c | 35 +++++++++++++----------------------
1 file changed, 13 insertions(+), 22 deletions(-)
diff --git a/net/sched/cls_tcindex.c b/net/sched/cls_tcindex.c
index 14a7e08b2fa9..32aaa5e459d3 100644
--- a/net/sched/cls_tcindex.c
+++ b/net/sched/cls_tcindex.c
@@ -27,14 +27,12 @@
struct tcindex_filter_result {
struct tcf_exts exts;
struct tcf_result res;
- struct rcu_head rcu;
};
struct tcindex_filter {
u16 key;
struct tcindex_filter_result result;
struct tcindex_filter __rcu *next;
- struct rcu_head rcu;
};
@@ -47,7 +45,6 @@ struct tcindex_data {
u32 hash; /* hash table size; 0 if undefined */
u32 alloc_hash; /* allocated size */
u32 fall_through; /* 0: only classify if explicit match */
- struct rcu_head rcu;
};
static inline int tcindex_filter_is_set(struct tcindex_filter_result *r)
@@ -133,19 +130,13 @@ static int tcindex_init(struct tcf_proto *tp)
return 0;
}
-static void tcindex_destroy_rexts(struct rcu_head *head)
+static void tcindex_destroy_rexts(struct tcindex_filter_result *r)
{
- struct tcindex_filter_result *r;
-
- r = container_of(head, struct tcindex_filter_result, rcu);
tcf_exts_destroy(&r->exts);
}
-static void tcindex_destroy_fexts(struct rcu_head *head)
+static void tcindex_destroy_fexts(struct tcindex_filter *f)
{
- struct tcindex_filter *f = container_of(head, struct tcindex_filter,
- rcu);
-
tcf_exts_destroy(&f->result.exts);
kfree(f);
}
@@ -182,10 +173,11 @@ static int tcindex_delete(struct tcf_proto *tp, void *arg, bool *last)
* grace period, since converted-to-rcu actions are relying on that
* in cleanup() callback
*/
+ synchronize_rcu();
if (f)
- call_rcu(&f->rcu, tcindex_destroy_fexts);
+ tcindex_destroy_fexts(f);
else
- call_rcu(&r->rcu, tcindex_destroy_rexts);
+ tcindex_destroy_rexts(r);
*last = false;
return 0;
@@ -199,10 +191,8 @@ static int tcindex_destroy_element(struct tcf_proto *tp,
return tcindex_delete(tp, arg, &last);
}
-static void __tcindex_destroy(struct rcu_head *head)
+static void __tcindex_destroy(struct tcindex_data *p)
{
- struct tcindex_data *p = container_of(head, struct tcindex_data, rcu);
-
kfree(p->perfect);
kfree(p->h);
kfree(p);
@@ -228,10 +218,8 @@ static int tcindex_filter_result_init(struct tcindex_filter_result *r)
return tcf_exts_init(&r->exts, TCA_TCINDEX_ACT, TCA_TCINDEX_POLICE);
}
-static void __tcindex_partial_destroy(struct rcu_head *head)
+static void __tcindex_partial_destroy(struct tcindex_data *p)
{
- struct tcindex_data *p = container_of(head, struct tcindex_data, rcu);
-
kfree(p->perfect);
kfree(p);
}
@@ -449,8 +437,10 @@ tcindex_set_parms(struct net *net, struct tcf_proto *tp, unsigned long base,
rcu_assign_pointer(*fp, f);
}
- if (oldp)
- call_rcu(&oldp->rcu, __tcindex_partial_destroy);
+ if (oldp) {
+ synchronize_rcu();
+ __tcindex_partial_destroy(oldp);
+ }
return 0;
errout_alloc:
@@ -540,7 +530,8 @@ static void tcindex_destroy(struct tcf_proto *tp)
walker.fn = tcindex_destroy_element;
tcindex_walk(tp, &walker);
- call_rcu(&p->rcu, __tcindex_destroy);
+ synchronize_rcu();
+ __tcindex_destroy(p);
}
--
2.13.0
^ permalink raw reply related
* [Patch net 07/15] net_sched: remove RCU callbacks in flow filter
From: Cong Wang @ 2017-10-23 22:02 UTC (permalink / raw)
To: netdev; +Cc: paulmck, jhs, john.fastabend, Chris Mi, Cong Wang,
Daniel Borkmann
In-Reply-To: <20171023220304.2268-1-xiyou.wangcong@gmail.com>
Replace call_rcu() with synchronize_rcu(), except in
flow_destroy() we have to use list_splice_init_rcu().
Reported-by: Chris Mi <chrism@mellanox.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: John Fastabend <john.fastabend@gmail.com>
Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
net/sched/cls_flow.c | 24 +++++++++++++-----------
1 file changed, 13 insertions(+), 11 deletions(-)
diff --git a/net/sched/cls_flow.c b/net/sched/cls_flow.c
index 2a3a60ec5b86..c33643289fa9 100644
--- a/net/sched/cls_flow.c
+++ b/net/sched/cls_flow.c
@@ -35,7 +35,6 @@
struct flow_head {
struct list_head filters;
- struct rcu_head rcu;
};
struct flow_filter {
@@ -57,7 +56,6 @@ struct flow_filter {
u32 divisor;
u32 baseclass;
u32 hashrnd;
- struct rcu_head rcu;
};
static inline u32 addr_fold(void *addr)
@@ -369,10 +367,8 @@ static const struct nla_policy flow_policy[TCA_FLOW_MAX + 1] = {
[TCA_FLOW_PERTURB] = { .type = NLA_U32 },
};
-static void flow_destroy_filter(struct rcu_head *head)
+static void flow_destroy_filter(struct flow_filter *f)
{
- struct flow_filter *f = container_of(head, struct flow_filter, rcu);
-
del_timer_sync(&f->perturb_timer);
tcf_exts_destroy(&f->exts);
tcf_em_tree_destroy(&f->ematches);
@@ -539,8 +535,10 @@ static int flow_change(struct net *net, struct sk_buff *in_skb,
*arg = fnew;
- if (fold)
- call_rcu(&fold->rcu, flow_destroy_filter);
+ if (fold) {
+ synchronize_rcu();
+ flow_destroy_filter(fold);
+ }
return 0;
err2:
@@ -557,7 +555,8 @@ static int flow_delete(struct tcf_proto *tp, void *arg, bool *last)
struct flow_filter *f = arg;
list_del_rcu(&f->list);
- call_rcu(&f->rcu, flow_destroy_filter);
+ synchronize_rcu();
+ flow_destroy_filter(f);
*last = list_empty(&head->filters);
return 0;
}
@@ -578,12 +577,15 @@ static void flow_destroy(struct tcf_proto *tp)
{
struct flow_head *head = rtnl_dereference(tp->root);
struct flow_filter *f, *next;
+ LIST_HEAD(local);
+
+ list_splice_init_rcu(&head->filters, &local, synchronize_rcu);
- list_for_each_entry_safe(f, next, &head->filters, list) {
+ list_for_each_entry_safe(f, next, &local, list) {
list_del_rcu(&f->list);
- call_rcu(&f->rcu, flow_destroy_filter);
+ flow_destroy_filter(f);
}
- kfree_rcu(head, rcu);
+ kfree(head);
}
static void *flow_get(struct tcf_proto *tp, u32 handle)
--
2.13.0
^ permalink raw reply related
* [Patch net 06/15] net_sched: remove RCU callbacks in rsvp filter
From: Cong Wang @ 2017-10-23 22:02 UTC (permalink / raw)
To: netdev
Cc: paulmck, jhs, john.fastabend, Chris Mi, Cong Wang,
Daniel Borkmann, Jiri Pirko
In-Reply-To: <20171023220304.2268-1-xiyou.wangcong@gmail.com>
Replace call_rcu() with synchronize_rcu(). I don't
touch the rest kfree_rcu() because they are not relevant
here and they are more complicated.
Reported-by: Chris Mi <chrism@mellanox.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Jiri Pirko <jiri@resnulli.us>
Cc: John Fastabend <john.fastabend@gmail.com>
Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
net/sched/cls_rsvp.h | 13 +++----------
1 file changed, 3 insertions(+), 10 deletions(-)
diff --git a/net/sched/cls_rsvp.h b/net/sched/cls_rsvp.h
index b1f6ed48bc72..5013e9a58f30 100644
--- a/net/sched/cls_rsvp.h
+++ b/net/sched/cls_rsvp.h
@@ -97,7 +97,6 @@ struct rsvp_filter {
u32 handle;
struct rsvp_session *sess;
- struct rcu_head rcu;
};
static inline unsigned int hash_dst(__be32 *dst, u8 protocol, u8 tunnelid)
@@ -282,14 +281,6 @@ static int rsvp_init(struct tcf_proto *tp)
return -ENOBUFS;
}
-static void rsvp_delete_filter_rcu(struct rcu_head *head)
-{
- struct rsvp_filter *f = container_of(head, struct rsvp_filter, rcu);
-
- tcf_exts_destroy(&f->exts);
- kfree(f);
-}
-
static void rsvp_delete_filter(struct tcf_proto *tp, struct rsvp_filter *f)
{
tcf_unbind_filter(tp, &f->res);
@@ -297,7 +288,9 @@ static void rsvp_delete_filter(struct tcf_proto *tp, struct rsvp_filter *f)
* grace period, since converted-to-rcu actions are relying on that
* in cleanup() callback
*/
- call_rcu(&f->rcu, rsvp_delete_filter_rcu);
+ synchronize_rcu();
+ tcf_exts_destroy(&f->exts);
+ kfree(f);
}
static void rsvp_destroy(struct tcf_proto *tp)
--
2.13.0
^ permalink raw reply related
* [Patch net 05/15] net_sched: remove RCU callbacks in cgroup filter
From: Cong Wang @ 2017-10-23 22:02 UTC (permalink / raw)
To: netdev
Cc: paulmck, jhs, john.fastabend, Chris Mi, Cong Wang,
Daniel Borkmann, Jiri Pirko
In-Reply-To: <20171023220304.2268-1-xiyou.wangcong@gmail.com>
Replace call_rcu() with synchronize_rcu().
Reported-by: Chris Mi <chrism@mellanox.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Jiri Pirko <jiri@resnulli.us>
Cc: John Fastabend <john.fastabend@gmail.com>
Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
net/sched/cls_cgroup.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/net/sched/cls_cgroup.c b/net/sched/cls_cgroup.c
index d48452f87975..fb001799eca6 100644
--- a/net/sched/cls_cgroup.c
+++ b/net/sched/cls_cgroup.c
@@ -23,7 +23,6 @@ struct cls_cgroup_head {
struct tcf_exts exts;
struct tcf_ematch_tree ematches;
struct tcf_proto *tp;
- struct rcu_head rcu;
};
static int cls_cgroup_classify(struct sk_buff *skb, const struct tcf_proto *tp,
@@ -57,11 +56,8 @@ static const struct nla_policy cgroup_policy[TCA_CGROUP_MAX + 1] = {
[TCA_CGROUP_EMATCHES] = { .type = NLA_NESTED },
};
-static void cls_cgroup_destroy_rcu(struct rcu_head *root)
+static void __cls_cgroup_destroy(struct cls_cgroup_head *head)
{
- struct cls_cgroup_head *head = container_of(root,
- struct cls_cgroup_head,
- rcu);
tcf_exts_destroy(&head->exts);
tcf_em_tree_destroy(&head->ematches);
@@ -110,8 +106,10 @@ static int cls_cgroup_change(struct net *net, struct sk_buff *in_skb,
goto errout;
rcu_assign_pointer(tp->root, new);
- if (head)
- call_rcu(&head->rcu, cls_cgroup_destroy_rcu);
+ if (head) {
+ synchronize_rcu();
+ __cls_cgroup_destroy(head);
+ }
return 0;
errout:
tcf_exts_destroy(&new->exts);
@@ -124,8 +122,10 @@ static void cls_cgroup_destroy(struct tcf_proto *tp)
struct cls_cgroup_head *head = rtnl_dereference(tp->root);
/* Head can still be NULL due to cls_cgroup_init(). */
- if (head)
- call_rcu(&head->rcu, cls_cgroup_destroy_rcu);
+ if (head) {
+ synchronize_rcu();
+ __cls_cgroup_destroy(head);
+ }
}
static int cls_cgroup_delete(struct tcf_proto *tp, void *arg, bool *last)
--
2.13.0
^ permalink raw reply related
* [Patch net 04/15] net_sched: remove RCU callbacks in matchall filter
From: Cong Wang @ 2017-10-23 22:02 UTC (permalink / raw)
To: netdev
Cc: paulmck, jhs, john.fastabend, Chris Mi, Cong Wang,
Daniel Borkmann, Jiri Pirko
In-Reply-To: <20171023220304.2268-1-xiyou.wangcong@gmail.com>
Replace call_rcu() with synchronize_rcu().
Reported-by: Chris Mi <chrism@mellanox.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Jiri Pirko <jiri@resnulli.us>
Cc: John Fastabend <john.fastabend@gmail.com>
Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
net/sched/cls_matchall.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/net/sched/cls_matchall.c b/net/sched/cls_matchall.c
index eeac606c95ab..459614cf4186 100644
--- a/net/sched/cls_matchall.c
+++ b/net/sched/cls_matchall.c
@@ -21,7 +21,6 @@ struct cls_mall_head {
struct tcf_result res;
u32 handle;
u32 flags;
- struct rcu_head rcu;
};
static int mall_classify(struct sk_buff *skb, const struct tcf_proto *tp,
@@ -41,11 +40,8 @@ static int mall_init(struct tcf_proto *tp)
return 0;
}
-static void mall_destroy_rcu(struct rcu_head *rcu)
+static void __mall_destroy(struct cls_mall_head *head)
{
- struct cls_mall_head *head = container_of(rcu, struct cls_mall_head,
- rcu);
-
tcf_exts_destroy(&head->exts);
kfree(head);
}
@@ -96,7 +92,8 @@ static void mall_destroy(struct tcf_proto *tp)
if (tc_should_offload(dev, head->flags))
mall_destroy_hw_filter(tp, head, (unsigned long) head);
- call_rcu(&head->rcu, mall_destroy_rcu);
+ synchronize_rcu();
+ __mall_destroy(head);
}
static void *mall_get(struct tcf_proto *tp, u32 handle)
--
2.13.0
^ permalink raw reply related
* [Patch net 03/15] net_sched: remove RCU callbacks in flower filter
From: Cong Wang @ 2017-10-23 22:02 UTC (permalink / raw)
To: netdev
Cc: paulmck, jhs, john.fastabend, Chris Mi, Cong Wang,
Daniel Borkmann, Jiri Pirko
In-Reply-To: <20171023220304.2268-1-xiyou.wangcong@gmail.com>
Replace call_rcu() with synchronize_rcu(), except
in fl_destroy() we have to use list_splice_init_rcu().
As a bonus, this also drops the ugly code in commit
d936377414fa ("net, sched: respect rcu grace period on cls destruction").
Reported-by: Chris Mi <chrism@mellanox.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Jiri Pirko <jiri@resnulli.us>
Cc: John Fastabend <john.fastabend@gmail.com>
Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
net/sched/cls_flower.c | 46 ++++++++++++++--------------------------------
1 file changed, 14 insertions(+), 32 deletions(-)
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index b480d7c792ba..ad33bd00b4f0 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -61,7 +61,6 @@ struct fl_flow_mask_range {
struct fl_flow_mask {
struct fl_flow_key key;
struct fl_flow_mask_range range;
- struct rcu_head rcu;
};
struct cls_fl_head {
@@ -71,10 +70,6 @@ struct cls_fl_head {
bool mask_assigned;
struct list_head filters;
struct rhashtable_params ht_params;
- union {
- struct work_struct work;
- struct rcu_head rcu;
- };
struct idr handle_idr;
};
@@ -87,7 +82,6 @@ struct cls_fl_filter {
struct list_head list;
u32 handle;
u32 flags;
- struct rcu_head rcu;
struct net_device *hw_dev;
};
@@ -215,10 +209,8 @@ static int fl_init(struct tcf_proto *tp)
return 0;
}
-static void fl_destroy_filter(struct rcu_head *head)
+static void fl_destroy_filter(struct cls_fl_filter *f)
{
- struct cls_fl_filter *f = container_of(head, struct cls_fl_filter, rcu);
-
tcf_exts_destroy(&f->exts);
kfree(f);
}
@@ -305,38 +297,25 @@ static void __fl_delete(struct tcf_proto *tp, struct cls_fl_filter *f)
if (!tc_skip_hw(f->flags))
fl_hw_destroy_filter(tp, f);
tcf_unbind_filter(tp, &f->res);
- call_rcu(&f->rcu, fl_destroy_filter);
-}
-
-static void fl_destroy_sleepable(struct work_struct *work)
-{
- struct cls_fl_head *head = container_of(work, struct cls_fl_head,
- work);
- if (head->mask_assigned)
- rhashtable_destroy(&head->ht);
- kfree(head);
- module_put(THIS_MODULE);
-}
-
-static void fl_destroy_rcu(struct rcu_head *rcu)
-{
- struct cls_fl_head *head = container_of(rcu, struct cls_fl_head, rcu);
-
- INIT_WORK(&head->work, fl_destroy_sleepable);
- schedule_work(&head->work);
}
static void fl_destroy(struct tcf_proto *tp)
{
struct cls_fl_head *head = rtnl_dereference(tp->root);
struct cls_fl_filter *f, *next;
+ LIST_HEAD(local);
+
+ list_splice_init_rcu(&head->filters, &local, synchronize_rcu);
- list_for_each_entry_safe(f, next, &head->filters, list)
+ list_for_each_entry_safe(f, next, &local, list) {
__fl_delete(tp, f);
+ fl_destroy_filter(f);
+ }
idr_destroy(&head->handle_idr);
- __module_get(THIS_MODULE);
- call_rcu(&head->rcu, fl_destroy_rcu);
+ if (head->mask_assigned)
+ rhashtable_destroy(&head->ht);
+ kfree(head);
}
static void *fl_get(struct tcf_proto *tp, u32 handle)
@@ -975,7 +954,8 @@ static int fl_change(struct net *net, struct sk_buff *in_skb,
idr_replace_ext(&head->handle_idr, fnew, fnew->handle);
list_replace_rcu(&fold->list, &fnew->list);
tcf_unbind_filter(tp, &fold->res);
- call_rcu(&fold->rcu, fl_destroy_filter);
+ synchronize_rcu();
+ fl_destroy_filter(fold);
} else {
list_add_tail_rcu(&fnew->list, &head->filters);
}
@@ -1003,6 +983,8 @@ static int fl_delete(struct tcf_proto *tp, void *arg, bool *last)
rhashtable_remove_fast(&head->ht, &f->ht_node,
head->ht_params);
__fl_delete(tp, f);
+ synchronize_rcu();
+ fl_destroy_filter(f);
*last = list_empty(&head->filters);
return 0;
}
--
2.13.0
^ 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