* [PATCH v2 bpf-next 9/9] bpf: NRM test script
From: brakmo @ 2019-02-23 1:07 UTC (permalink / raw)
To: netdev
Cc: Martin Lau, Alexei Starovoitov, Daniel Borkmann, Eric Dumazet,
Kernel Team
In-Reply-To: <20190223010703.678070-1-brakmo@fb.com>
Script for testing NRM (Network Resource Manager) framework.
It creates a cgroup to use for testing and load a BPF program to limit
egress bandwidht. It then uses iperf3 or netperf to create
loads. The output is the goodput in Mbps (unless -D is used).
It can work on a single host using loopback or among two hosts (with netperf).
When using loopback, it is recommended to also introduce a delay of at least
1ms (-d=1), otherwise the assigned bandwidth is likely to be underutilized.
USAGE: $name [out] [-b=<prog>|--bpf=<prog>] [-c=<cc>|--cc=<cc>] [-D]
[-d=<delay>|--delay=<delay>] [--debug] [-E]
[-f=<#flows>|--flows=<#flows>] [-h] [-i=<id>|--id=<id >] [-l]
[-N] [-p=<port>|--port=<port>] [-P] [-q=<qdisc>]
[-R] [-s=<server>|--server=<server] [--stats]
[-t=<time>|--time=<time>] [-w] [cubic|dctcp]
Where:
out Egress (default egress)
-b or --bpf BPF program filename to load and attach.
Default is nrm_out_kern.o for egress,
nrm_in_kern.o for ingress
-c or -cc TCP congestion control (cubic or dctcp)
-d or --delay Add a delay in ms using netem
-D In addition to the goodput in Mbps, it also outputs
other detailed information. This information is
test dependent (i.e. iperf3 or netperf).
--debug Print BPF trace buffer
-E Enable ECN (not required for dctcp)
-f or --flows Number of concurrent flows (default=1)
-i or --id cgroup id (an integer, default is 1)
-l Do not limit flows using loopback
-N Use netperf instead of iperf3
-h Help
-p or --port iperf3 port (default is 5201)
-P Use an iperf3 instance for each flow
-q Use the specified qdisc.
-r or --rate Rate in Mbps (default 1s 1Gbps)
-R Use TCP_RR for netperf. 1st flow has req
size of 10KB, rest of 1MB. Reply in all
cases is 1 byte.
More detailed output for each flow can be found
in the files netperf.<cg>.<flow>, where <cg> is the
cgroup id as specified with the -i flag, and <flow>
is the flow id starting at 1 and increasing by 1 for
flow (as specified by -f).
-s or --server hostname of netperf server. Used to create netperf
test traffic between to hosts (default is within host)
netserver must be running on the host.
--stats Get NRM stats (marked, dropped, etc.)
-t or --time duration of iperf3 in seconds (default=5)
-w Work conserving flag. cgroup can increase its
bandwidth beyond the rate limit specified
while there is available bandwidth. Current
implementation assumes there is only one NIC
(eth0), but can be extended to support multiple
NICs. This is just a proof of concept.
cubic or dctcp specify TCP CC to use
Examples:
./do_nrm_test.sh -l -d=1 -D --stats
Runs a 5 second test, using a single iperf3 flow and with the default
rate limit of 1Gbps and a delay of 1ms (using netem) using the default
TCP congestion control on the loopback device (hence we use "-l" to
enforce bandwidth limit on loopback device). Since no direction is
specified, it defaults to egress. Since no TCP CC algorithm is
specified it uses the system default.
With no -D flag, only the value of the AGGREGATE OUTPUT would show.
id refers to the cgroup id and is useful when running multi cgroup
tests (see do_nrm_test_multi.sh script).
Output:
Details for NRM in cgroup 1
id:1
rate_mbps:713
duration:4.9 secs
packets:10072
bytes_MB:468
pkts_dropped:491
bytes_dropped_MB:32
pkts_marked_percent: 28.64
bytes_marked_percent: 29.15
pkts_dropped_percent: 4.87
bytes_dropped_percent: 6.86
PING AVG DELAY:2.072
AGGREGATE_GOODPUT:729
./do_nrm_test.sh -l -d=1 -D --stats dctcp
Same as above but using dctcp. Note that fewer bytes are dropped
(0.13 vs. 6.86%).
Output:
Details for NRM in cgroup 1
id:1
rate_mbps:932
duration:4.9 secs
packets:15514
bytes_MB:570
pkts_dropped:11
bytes_dropped_MB:0
pkts_marked_percent: 40.38
bytes_marked_percent: 46.82
pkts_dropped_percent: 0.07
bytes_dropped_percent: 0.13
PING AVG DELAY:2.069
AGGREGATE_GOODPUT:953
./do_nrm_test.sh -d=1 -D --stats
As first example, but without limiting loopback device (i.e. no
"-l" flag). Since there is no bandwidth limiting, no details for
NRM are printed out.
Output:
Details for NRM in cgroup 1
PING AVG DELAY:2.021
AGGREGATE_GOODPUT:40226
./do_nrm_test.sh -l -d=1 -D --stats -f=2
Uses iper3 and does 2 flows
./do_nrm_test.sh -l -d=1 -D --stats -f=4 -P
Uses iperf3 and does 4 flows, each flow as a separate process.
./do_nrm_test.sh -l -d=1 -D --stats -f=4 -N
Uses netperf, 4 flows
./do_nrm_test.sh -f=1 -r=2000 -t=5 -N -D --stats dctcp -s=<server-name>
Uses netperf between two hosts. The remote host name is specified
with -s= and you need to start the program netserver manually on
the remote host. It will use 1 flow, a rate limit of 2Gbps and dctcp.
./do_nrm_test.sh -f=1 -r=2000 -t=5 -N -D --stats -w dctcp \
-s=<server-name>
As previous, but allows use of extra bandwidth. For this test the
rate is 8Gbps vs. 1Gbps of the previous test.
Signed-off-by: Lawrence Brakmo <brakmo@fb.com>
---
samples/bpf/do_nrm_test.sh | 437 +++++++++++++++++++++++++++++++++++++
1 file changed, 437 insertions(+)
create mode 100755 samples/bpf/do_nrm_test.sh
diff --git a/samples/bpf/do_nrm_test.sh b/samples/bpf/do_nrm_test.sh
new file mode 100755
index 000000000000..91d99237aea5
--- /dev/null
+++ b/samples/bpf/do_nrm_test.sh
@@ -0,0 +1,437 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+#
+# Copyright (c) 2019 Facebook
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of version 2 of the GNU General Public
+# License as published by the Free Software Foundation.
+
+Usage() {
+ echo "Script for testing NRM (Network Resource Manager) framework."
+ echo "It creates a cgroup to use for testing and load a BPF program to limit"
+ echo "egress or ingress bandwidht. It then uses iperf3 or netperf to create"
+ echo "loads. The output is the goodput in Mbps (unless -D was used)."
+ echo ""
+ echo "USAGE: $name [out] [-b=<prog>|--bpf=<prog>] [-c=<cc>|--cc=<cc>] [-D]"
+ echo " [-d=<delay>|--delay=<delay>] [--debug] [-E]"
+ echo " [-f=<#flows>|--flows=<#flows>] [-h] [-i=<id>|--id=<id >]"
+ echo " [-l] [-N] [-p=<port>|--port=<port>] [-P]"
+ echo " [-q=<qdisc>] [-R] [-s=<server>|--server=<server]"
+ echo " [-S|--stats] -t=<time>|--time=<time>] [-w] [cubic|dctcp]"
+ echo " Where:"
+ echo " out egress (default)"
+ echo " -b or --bpf BPF program filename to load and attach."
+ echo " Default is nrm_out_kern.o for egress,"
+ echo " nrm_in_kern.o for ingress"
+ echo " -c or -cc TCP congestion control (cubic or dctcp)"
+ echo " --debug print BPF trace buffer"
+ echo " -d or --delay add a delay in ms using netem"
+ echo " -D In addition to the goodput in Mbps, it also outputs"
+ echo " other detailed information. This information is"
+ echo " test dependent (i.e. iperf3 or netperf)."
+ echo " -E enable ECN (not required for dctcp)"
+ echo " -f or --flows number of concurrent flows (default=1)"
+ echo " -i or --id cgroup id (an integer, default is 1)"
+ echo " -N use netperf instead of iperf3"
+ echo " -l do not limit flows using loopback"
+ echo " -h Help"
+ echo " -p or --port iperf3 port (default is 5201)"
+ echo " -P use an iperf3 instance for each flow"
+ echo " -q use the specified qdisc"
+ echo " -r or --rate rate in Mbps (default 1s 1Gbps)"
+ echo " -R Use TCP_RR for netperf. 1st flow has req"
+ echo " size of 10KB, rest of 1MB. Reply in all"
+ echo " cases is 1 byte."
+ echo " More detailed output for each flow can be found"
+ echo " in the files netperf.<cg>.<flow>, where <cg> is the"
+ echo " cgroup id as specified with the -i flag, and <flow>"
+ echo " is the flow id starting at 1 and increasing by 1 for"
+ echo " flow (as specified by -f)."
+ echo " -s or --server hostname of netperf server. Used to create netperf"
+ echo " test traffic between to hosts (default is within host)"
+ echo " netserver must be running on the host."
+ echo " -S or --stats whether to update nrm stats (default is yes)."
+ echo " -t or --time duration of iperf3 in seconds (default=5)"
+ echo " -w Work conserving flag. cgroup can increase its"
+ echo " bandwidth beyond the rate limit specified"
+ echo " while there is available bandwidth. Current"
+ echo " implementation assumes there is only one NIC"
+ echo " (eth0), but can be extended to support multiple"
+ echo " NICs."
+ echo " cubic or dctcp specify which TCP CC to use"
+ echo " "
+ exit
+}
+
+#set -x
+
+debug_flag=0
+args="$@"
+name="$0"
+netem=0
+cc=x
+dir="-o"
+dir_name="out"
+dur=5
+flows=1
+id=1
+prog=""
+port=5201
+rate=1000
+multi_iperf=0
+flow_cnt=1
+use_netperf=0
+rr=0
+ecn=0
+details=0
+server=""
+qdisc=""
+flags=""
+do_stats=0
+
+function start_nrm () {
+ rm -f nrm.out
+ echo "./nrm $dir -n $id -r $rate -t $dur $flags $dbg $prog" > nrm.out
+ echo " " >> nrm.out
+ ./nrm $dir -n $id -r $rate -t $dur $flags $dbg $prog >> nrm.out 2>&1 &
+ echo $!
+}
+
+processArgs () {
+ for i in $args ; do
+ case $i in
+ # Support for upcomming ingress rate limiting
+ #in) # support for upcoming ingress rate limiting
+ # dir="-i"
+ # dir_name="in"
+ # ;;
+ out)
+ dir="-o"
+ dir_name="out"
+ ;;
+ -b=*|--bpf=*)
+ prog="${i#*=}"
+ ;;
+ -c=*|--cc=*)
+ cc="${i#*=}"
+ ;;
+ --debug)
+ flags="$flags -d"
+ debug_flag=1
+ ;;
+ -d=*|--delay=*)
+ netem="${i#*=}"
+ ;;
+ -D)
+ details=1
+ ;;
+ -E)
+ ecn=1
+ ;;
+ # Support for upcomming fq Early Departure Time egress rate limiting
+ #--edt)
+ # prog="nrm_out_edt_kern.o"
+ # qdisc="fq"
+ # ;;
+ -f=*|--flows=*)
+ flows="${i#*=}"
+ ;;
+ -i=*|--id=*)
+ id="${i#*=}"
+ ;;
+ -l)
+ flags="$flags -l"
+ ;;
+ -N)
+ use_netperf=1
+ ;;
+ -p=*|--port=*)
+ port="${i#*=}"
+ ;;
+ -P)
+ multi_iperf=1
+ ;;
+ -q=*)
+ qdisc="${i#*=}"
+ ;;
+ -r=*|--rate=*)
+ rate="${i#*=}"
+ ;;
+ -R)
+ rr=1
+ ;;
+ -s=*|--server=*)
+ server="${i#*=}"
+ ;;
+ -S|--stats)
+ flags="$flags -s"
+ do_stats=1
+ ;;
+ -t=*|--time=*)
+ dur="${i#*=}"
+ ;;
+ -w)
+ flags="$flags -w"
+ ;;
+ cubic)
+ cc=cubic
+ ;;
+ dctcp)
+ cc=dctcp
+ ;;
+ *)
+ echo "Unknown arg:$i"
+ Usage
+ ;;
+ esac
+ done
+}
+
+processArgs
+
+if [ $debug_flag -eq 1 ] ; then
+ rm -f nrm_out.log
+fi
+
+nrm_pid=$(start_nrm)
+usleep 100000
+
+host=`hostname`
+cg_base_dir=/sys/fs/cgroup
+cg_dir="$cg_base_dir/cgroup-test-work-dir/nrm$id"
+
+echo $$ >> $cg_dir/cgroup.procs
+
+ulimit -l unlimited
+
+rm -f ss.out
+rm -f nrm.[0-9]*.$dir_name
+if [ $ecn -ne 0 ] ; then
+ sysctl -w -q -n net.ipv4.tcp_ecn=1
+fi
+
+if [ $use_netperf -eq 0 ] ; then
+ cur_cc=`sysctl -n net.ipv4.tcp_congestion_control`
+ if [ "$cc" != "x" ] ; then
+ sysctl -w -q -n net.ipv4.tcp_congestion_control=$cc
+ fi
+fi
+
+if [ "$netem" -ne "0" ] ; then
+ if [ "$qdisc" != "" ] ; then
+ echo "WARNING: Ignoring -q options because -d option used"
+ fi
+ tc qdisc del dev lo root > /dev/null 2>&1
+ tc qdisc add dev lo root netem delay $netem\ms > /dev/null 2>&1
+elif [ "$qdisc" != "" ] ; then
+ tc qdisc del dev lo root > /dev/null 2>&1
+ tc qdisc add dev lo root $qdisc > /dev/null 2>&1
+fi
+
+n=0
+m=$[$dur * 5]
+hn="::1"
+if [ $use_netperf -ne 0 ] ; then
+ if [ "$server" != "" ] ; then
+ hn=$server
+ fi
+fi
+
+( ping6 -i 0.2 -c $m $hn > ping.out 2>&1 ) &
+
+if [ $use_netperf -ne 0 ] ; then
+ begNetserverPid=`ps ax | grep netserver | grep --invert-match "grep" | \
+ awk '{ print $1 }'`
+ if [ "$begNetserverPid" == "" ] ; then
+ if [ "$server" == "" ] ; then
+ ( ./netserver > /dev/null 2>&1) &
+ usleep 100000
+ fi
+ fi
+ flow_cnt=1
+ if [ "$server" == "" ] ; then
+ np_server=$host
+ else
+ np_server=$server
+ fi
+ if [ "$cc" == "x" ] ; then
+ np_cc=""
+ else
+ np_cc="-K $cc,$cc"
+ fi
+ replySize=1
+ while [ $flow_cnt -le $flows ] ; do
+ if [ $rr -ne 0 ] ; then
+ reqSize=1M
+ if [ $flow_cnt -eq 1 ] ; then
+ reqSize=10K
+ fi
+ if [ "$dir" == "-i" ] ; then
+ replySize=$reqSize
+ reqSize=1
+ fi
+ ( ./netperf -H $np_server -l $dur -f m -j -t TCP_RR -- -r $reqSize,$replySize $np_cc -k P50_lATENCY,P90_LATENCY,LOCAL_TRANSPORT_RETRANS,REMOTE_TRANSPORT_RETRANS,LOCAL_SEND_THROUGHPUT,LOCAL_RECV_THROUGHPUT,REQUEST_SIZE,RESPONSE_SIZE > netperf.$id.$flow_cnt ) &
+ else
+ if [ "$dir" == "-i" ] ; then
+ ( ./netperf -H $np_server -l $dur -f m -j -t TCP_RR -- -r 1,10M $np_cc -k P50_LATENCY,P90_LATENCY,LOCAL_TRANSPORT_RETRANS,LOCAL_SEND_THROUGHPUT,REMOTE_TRANSPORT_RETRANS,REMOTE_SEND_THROUGHPUT,REQUEST_SIZE,RESPONSE_SIZE > netperf.$id.$flow_cnt ) &
+ else
+ ( ./netperf -H $np_server -l $dur -f m -j -t TCP_STREAM -- $np_cc -k P50_lATENCY,P90_LATENCY,LOCAL_TRANSPORT_RETRANS,LOCAL_SEND_THROUGHPUT,REQUEST_SIZE,RESPONSE_SIZE > netperf.$id.$flow_cnt ) &
+ fi
+ fi
+ flow_cnt=$[flow_cnt+1]
+ done
+
+# sleep for duration of test (plus some buffer)
+ n=$[dur+2]
+ sleep $n
+
+# force graceful termination of netperf
+ pids=`pgrep netperf`
+ for p in $pids ; do
+ kill -SIGALRM $p
+ done
+
+ flow_cnt=1
+ rate=0
+ if [ $details -ne 0 ] ; then
+ echo ""
+ echo "Details for NRM in cgroup $id"
+ if [ $do_stats -eq 1 ] ; then
+ if [ -e nrm.$id.$dir_name ] ; then
+ cat nrm.$id.$dir_name
+ fi
+ fi
+ fi
+ while [ $flow_cnt -le $flows ] ; do
+ if [ "$dir" == "-i" ] ; then
+ r=`cat netperf.$id.$flow_cnt | grep -o "REMOTE_SEND_THROUGHPUT=[0-9]*" | grep -o "[0-9]*"`
+ else
+ r=`cat netperf.$id.$flow_cnt | grep -o "LOCAL_SEND_THROUGHPUT=[0-9]*" | grep -o "[0-9]*"`
+ fi
+ echo "rate for flow $flow_cnt: $r"
+ rate=$[rate+r]
+ if [ $details -ne 0 ] ; then
+ echo "-----"
+ echo "Details for cgroup $id, flow $flow_cnt"
+ cat netperf.$id.$flow_cnt
+ fi
+ flow_cnt=$[flow_cnt+1]
+ done
+ if [ $details -ne 0 ] ; then
+ echo ""
+ delay=`grep "avg" ping.out | grep -o "= [0-9.]*/[0-9.]*" | grep -o "[0-9.]*$"`
+ echo "PING AVG DELAY:$delay"
+ echo "AGGREGATE_GOODPUT:$rate"
+ else
+ echo $rate
+ fi
+elif [ $multi_iperf -eq 0 ] ; then
+ (iperf3 -s -p $port -1 > /dev/null 2>&1) &
+ usleep 100000
+ iperf3 -c $host -p $port -i 0 -P $flows -f m -t $dur > iperf.$id
+ rates=`grep receiver iperf.$id | grep -o "[0-9.]* Mbits" | grep -o "^[0-9]*"`
+ rate=`echo $rates | grep -o "[0-9]*$"`
+
+ if [ $details -ne 0 ] ; then
+ echo ""
+ echo "Details for NRM in cgroup $id"
+ if [ $do_stats -eq 1 ] ; then
+ if [ -e nrm.$id.$dir_name ] ; then
+ cat nrm.$id.$dir_name
+ fi
+ fi
+ delay=`grep "avg" ping.out | grep -o "= [0-9.]*/[0-9.]*" | grep -o "[0-9.]*$"`
+ echo "PING AVG DELAY:$delay"
+ echo "AGGREGATE_GOODPUT:$rate"
+ else
+ echo $rate
+ fi
+else
+ flow_cnt=1
+ while [ $flow_cnt -le $flows ] ; do
+ (iperf3 -s -p $port -1 > /dev/null 2>&1) &
+ ( iperf3 -c $host -p $port -i 0 -P 1 -f m -t $dur | grep receiver | grep -o "[0-9.]* Mbits" | grep -o "^[0-9]*" | grep -o "[0-9]*$" > iperf3.$id.$flow_cnt ) &
+ port=$[port+1]
+ flow_cnt=$[flow_cnt+1]
+ done
+ n=$[dur+1]
+ sleep $n
+ flow_cnt=1
+ rate=0
+ if [ $details -ne 0 ] ; then
+ echo ""
+ echo "Details for NRM in cgroup $id"
+ if [ $do_stats -eq 1 ] ; then
+ if [ -e nrm.$id.$dir_name ] ; then
+ cat nrm.$id.$dir_name
+ fi
+ fi
+ fi
+
+ while [ $flow_cnt -le $flows ] ; do
+ r=`cat iperf3.$id.$flow_cnt`
+# echo "rate for flow $flow_cnt: $r"
+ if [ $details -ne 0 ] ; then
+ echo "Rate for cgroup $id, flow $flow_cnt LOCAL_SEND_THROUGHPUT=$r"
+ fi
+ rate=$[rate+r]
+ flow_cnt=$[flow_cnt+1]
+ done
+ if [ $details -ne 0 ] ; then
+ delay=`grep "avg" ping.out | grep -o "= [0-9.]*/[0-9.]*" | grep -o "[0-9.]*$"`
+ echo "PING AVG DELAY:$delay"
+ echo "AGGREGATE_GOODPUT:$rate"
+ else
+ echo $rate
+ fi
+fi
+
+if [ $use_netperf -eq 0 ] ; then
+ sysctl -w -q -n net.ipv4.tcp_congestion_control=$cur_cc
+fi
+if [ $ecn -ne 0 ] ; then
+ sysctl -w -q -n net.ipv4.tcp_ecn=0
+fi
+if [ "$netem" -ne "0" ] ; then
+ tc qdisc del dev lo root > /dev/null 2>&1
+fi
+
+sleep 2
+
+nrmPid=`ps ax | grep "nrm " | grep --invert-match "grep" | awk '{ print $1 }'`
+if [ "$nrmPid" == "$nrm_pid" ] ; then
+ kill $nrm_pid
+fi
+
+sleep 1
+
+# Detach any BPF programs that may have lingered
+ttx=`bpftool cgroup tree | grep nrm`
+v=2
+for x in $ttx ; do
+ if [ "${x:0:36}" == "/sys/fs/cgroup/cgroup-test-work-dir/" ] ; then
+ cg=$x ; v=0
+ else
+ if [ $v -eq 0 ] ; then
+ id=$x ; v=1
+ else
+ if [ $v -eq 1 ] ; then
+ type=$x ; bpftool cgroup detach $cg $type id $id
+ v=0
+ fi
+ fi
+ fi
+done
+
+if [ $use_netperf -ne 0 ] ; then
+ if [ "$server" == "" ] ; then
+ if [ "$begNetserverPid" == "" ] ; then
+ netserverPid=`ps ax | grep netserver | grep --invert-match "grep" | awk '{ print $1 }'`
+ if [ "$netserverPid" != "" ] ; then
+ kill $netserverPid
+ fi
+ fi
+ fi
+fi
+exit
--
2.17.1
^ permalink raw reply related
* Re: [PATCH v2 bpf-next 4/9] bpf: add bpf helper bpf_skb_ecn_set_ce
From: Daniel Borkmann @ 2019-02-23 1:14 UTC (permalink / raw)
To: brakmo, netdev; +Cc: Martin Lau, Alexei Starovoitov, Eric Dumazet, Kernel Team
In-Reply-To: <20190223010703.678070-5-brakmo@fb.com>
On 02/23/2019 02:06 AM, brakmo wrote:
> This patch adds a new bpf helper BPF_FUNC_skb_ecn_set_ce
> "int bpf_skb_ecn_set_ce(struct sk_buff *skb)". It is added to
> BPF_PROG_TYPE_CGROUP_SKB typed bpf_prog which currently can
> be attached to the ingress and egress path. The helper is needed
> because his type of bpf_prog cannot modify the skb directly.
>
> This helper is used to set the ECN field of ECN capable IP packets to ce
> (congestion encountered) in the IPv6 or IPv4 header of the skb. It can be
> used by a bpf_prog to manage egress or ingress network bandwdith limit
> per cgroupv2 by inducing an ECN response in the TCP sender.
> This works best when using DCTCP.
>
> Signed-off-by: Lawrence Brakmo <brakmo@fb.com>
> ---
> include/uapi/linux/bpf.h | 10 +++++++++-
> net/core/filter.c | 14 ++++++++++++++
> 2 files changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 95b5058fa945..fc646f3eaf9b 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -2365,6 +2365,13 @@ union bpf_attr {
> * Make a tcp_sock enter CWR state.
> * Return
> * 0 on success, or a negative error in case of failure.
> + *
> + * int bpf_skb_ecn_set_ce(struct sk_buf *skb)
> + * Description
> + * Sets ECN of IP header to ce (congestion encountered) if
> + * current value is ect (ECN capable). Works with IPv6 and IPv4.
> + * Return
> + * 1 if set, 0 if not set.
> */
> #define __BPF_FUNC_MAPPER(FN) \
> FN(unspec), \
> @@ -2464,7 +2471,8 @@ union bpf_attr {
> FN(spin_unlock), \
> FN(sk_fullsock), \
> FN(tcp_sock), \
> - FN(tcp_enter_cwr),
> + FN(tcp_enter_cwr), \
> + FN(skb_ecn_set_ce),
>
> /* integer value in 'imm' field of BPF_CALL instruction selects which helper
> * function eBPF program intends to call
> diff --git a/net/core/filter.c b/net/core/filter.c
> index ca57ef25279c..955369c6ed30 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -5444,6 +5444,18 @@ static const struct bpf_func_proto bpf_tcp_enter_cwr_proto = {
> .ret_type = RET_INTEGER,
> .arg1_type = ARG_PTR_TO_TCP_SOCK,
> };
> +
> +BPF_CALL_1(bpf_skb_ecn_set_ce, struct sk_buff *, skb)
> +{
> + return INET_ECN_set_ce(skb);
Hm, but as mentioned last time, don't we have to ensure here that skb
is writable (aka skb->data private to us before writing into it)?
> +}
> +
> +static const struct bpf_func_proto bpf_skb_ecn_set_ce_proto = {
> + .func = bpf_skb_ecn_set_ce,
> + .gpl_only = false,
> + .ret_type = RET_INTEGER,
> + .arg1_type = ARG_PTR_TO_CTX,
> +};
> #endif /* CONFIG_INET */
>
> bool bpf_helper_changes_pkt_data(void *func)
> @@ -5610,6 +5622,8 @@ cg_skb_func_proto(enum bpf_func_id func_id, struct bpf_prog *prog)
> } else {
> return NULL;
> }
> + case BPF_FUNC_skb_ecn_set_ce:
> + return &bpf_skb_ecn_set_ce_proto;
> #endif
> default:
> return sk_filter_func_proto(func_id, prog);
>
Thanks,
Daniel
^ permalink raw reply
* Re: [PATCH v4 net-next 5/5] net: dsa: microchip: add port_cleanup function
From: Florian Fainelli @ 2019-02-23 2:17 UTC (permalink / raw)
To: Tristram.Ha, Sergio Paracuellos, Andrew Lunn, Pavel Machek
Cc: UNGLinuxDriver, netdev
In-Reply-To: <1550882211-10381-6-git-send-email-Tristram.Ha@microchip.com>
Le 2/22/19 à 4:36 PM, Tristram.Ha@microchip.com a écrit :
> From: Tristram Ha <Tristram.Ha@microchip.com>
>
> Add port_cleanup function to reset some device variables when the port is
> disabled. Add a mutex to make sure changing those variables is
> thread-safe.
>
> Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* Re: [PATCH 1/2 v2] kprobe: Do not use uaccess functions to access kernel memory that can fault
From: Alexei Starovoitov @ 2019-02-23 2:28 UTC (permalink / raw)
To: Linus Torvalds
Cc: David Miller, Masami Hiramatsu, Steven Rostedt, Andy Lutomirski,
Linux List Kernel Mailing, Ingo Molnar, Andrew Morton, stable,
Changbin Du, Jann Horn, Kees Cook, Andrew Lutomirski,
Daniel Borkmann, Netdev, bpf
In-Reply-To: <CAHk-=whu_iwv4TQP5Xd62bTfWW-uswVecUbFeiSCuTWnH1xgPg@mail.gmail.com>
On Fri, Feb 22, 2019 at 04:08:59PM -0800, Linus Torvalds wrote:
> On Fri, Feb 22, 2019 at 3:56 PM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> >
> > It will preserve existing bpf_probe_read() behavior on x86.
>
> ... but that's the worst possible situation.
>
> It appears that people haven't understood that kernel and user
> addresses are distinct, and may have written programs that are
> fundamentally buggy.
>
> And we _want_ to make it clear that they are buggy on x86-64, exactly
> because x86-64 is the one that gets the most testing - by far.
>
> So if x86-64 continues working for buggy programs, then that only
> means that those bugs never get fixed.
>
> It would be much better to try to get those things fixed, and make the
> x86-64 implementation stricter, exactly so that people end up
> _realizing_ that they can't just think "a pointer is a pointer, and
> the context doesn't matter".
>
> From a pure functional safety standpoint, I thought bpf already knew
> what kind of a pointer it had?
when bpf verifier knows the type of pointer it allows direct access to it.
That's the case for skb, socket, packet data, hash maps, arrays, stack, etc.
Networking progs cannot call bpf_probe_read().
It's available to tracing progs only and their goal is to walk the kernel and
user memory with addresses that cannot be statically verified
at program load time.
We are working on adding type information (BTF) to vmlinux.
Soon we'll be able to tell the type of every kernel function argument.
Right now arg1 and arg2 in a kprobed function are just u64 pt_regs->di, si.
Soon we'll be able to precisely identify their C type.
I completely agree on the direction that x86 is the architecture that
sets an example and users need to learn the difference in pointers.
I only disagree on timing.
Right now users don't have an alternative.
In our repo I counted ~400 calls to bpf_probe_read and about 10 times more
'indirect' calls. What's happening with 'indirect' calls is BCC toolchain
using clang to automatically replace struct_a->field_foo access with
bpf_probe_read(struct_a + offsetof(typeof(struct_a), field_foo));
If we had __user vs __kernel annotation available to clang we could have
taught BCC to replace this '->' dereference with appropriate kernel vs user
helper. Also we need to teach GCC to recognize __user and store into dwarf,
so we can take it further into BTF and verify later.
Also I think disallowing bpf_probe_read() to read user pointer will not
make a desired teaching effect. It will only cause painful debugging to folks
when their progs will stop working. It's better to remove bpf_probe_read()
completely.
imo the process of teaching the users of kernel vs user pointer difference
needs to be gradual.
First we introduce bpf_probe_kernel_read and bpf_probe_user_read and
introduce clang/gcc tooling to catch the mistakes.
Going over this 400+ places and manually grepping kernel sources
for __user keyword is not a great proposal if we want to keep those users.
Once we have this working we can remove bpf_probe_read() altogether.
Rejecting bpf prog at load time is a clear signal that user has to fix it
(instead of changing run-time behavior).
When the verifier gets even smarter it could potentially replace prob_read
with probe_kernel_read and probe_user_read when it has that type info.
imo this kernel release should finish as-is and in the next cycle we can
change probe_kernel_read() to reject user address, have temporary
workaround in bpf_probe_read() with probe_kernel_read+get_user hack,
introduce new bpf helpers, new tooling and eventually remove
buggy bpf_probe_read.
^ permalink raw reply
* Re: [PATCH bpf-next 1/4] bpf: enable program stats
From: Alexei Starovoitov @ 2019-02-23 2:29 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Alexei Starovoitov, davem, daniel, netdev, bpf, kernel-team
In-Reply-To: <adabf404-7d0e-d801-1952-14f6b3ff8ced@gmail.com>
On Fri, Feb 22, 2019 at 04:05:55PM -0800, Eric Dumazet wrote:
>
>
> On 02/22/2019 03:36 PM, Alexei Starovoitov wrote:
>
> >
> > +static void bpf_prog_get_stats(const struct bpf_prog *prog,
> > + struct bpf_prog_stats *stats)
> > +{
> > + u64 nsecs = 0, cnt = 0;
> > + int cpu;
> > +
> > + for_each_possible_cpu(cpu) {
> > + nsecs += per_cpu(prog->aux->stats->nsecs, cpu);
> > + cnt += per_cpu(prog->aux->stats->cnt, cpu);
>
> This does not work reliably for 32 bit kernels.
right. my mind was in probe_read thread. will fix.
^ permalink raw reply
* Re: [PATCH bpf-next 1/4] bpf: enable program stats
From: Alexei Starovoitov @ 2019-02-23 2:31 UTC (permalink / raw)
To: Roman Gushchin
Cc: Alexei Starovoitov, davem@davemloft.net, daniel@iogearbox.net,
netdev@vger.kernel.org, bpf@vger.kernel.org, Kernel Team
In-Reply-To: <20190223003434.GA17559@tower.DHCP.thefacebook.com>
On Sat, Feb 23, 2019 at 12:34:38AM +0000, Roman Gushchin wrote:
> On Fri, Feb 22, 2019 at 03:36:41PM -0800, Alexei Starovoitov wrote:
> > JITed BPF programs are indistinguishable from kernel functions, but unlike
> > kernel code BPF code can be changed often.
> > Typical approach of "perf record" + "perf report" profiling and tunning of
> > kernel code works just as well for BPF programs, but kernel code doesn't
> > need to be monitored whereas BPF programs do.
> > Users load and run large amount of BPF programs.
> > These BPF stats allow tools monitor the usage of BPF on the server.
> > The monitoring tools will turn sysctl kernel.bpf_stats_enabled
> > on and off for few seconds to sample average cost of the programs.
> > Aggregated data over hours and days will provide an insight into cost of BPF
> > and alarms can trigger in case given program suddenly gets more expensive.
> >
> > The cost of two sched_clock() per program invocation adds ~20 nsec.
> > Fast BPF progs (like selftests/bpf/progs/test_pkt_access.c) will slow down
> > from ~40 nsec to ~60 nsec.
> > static_key minimizes the cost of the stats collection.
> > There is no measurable difference before/after this patch
> > with kernel.bpf_stats_enabled=0
> >
> > Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> > ---
> > include/linux/bpf.h | 7 +++++++
> > include/linux/filter.h | 16 +++++++++++++++-
> > kernel/bpf/core.c | 13 +++++++++++++
> > kernel/bpf/syscall.c | 24 ++++++++++++++++++++++--
> > kernel/bpf/verifier.c | 5 +++++
> > kernel/sysctl.c | 34 ++++++++++++++++++++++++++++++++++
> > 6 files changed, 96 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> > index de18227b3d95..14260674bc57 100644
> > --- a/include/linux/bpf.h
> > +++ b/include/linux/bpf.h
> > @@ -340,6 +340,11 @@ enum bpf_cgroup_storage_type {
> >
> > #define MAX_BPF_CGROUP_STORAGE_TYPE __BPF_CGROUP_STORAGE_MAX
> >
> > +struct bpf_prog_stats {
> > + u64 cnt;
> > + u64 nsecs;
> > +};
> > +
> > struct bpf_prog_aux {
> > atomic_t refcnt;
> > u32 used_map_cnt;
> > @@ -389,6 +394,7 @@ struct bpf_prog_aux {
> > * main prog always has linfo_idx == 0
> > */
> > u32 linfo_idx;
> > + struct bpf_prog_stats __percpu *stats;
> > union {
> > struct work_struct work;
> > struct rcu_head rcu;
> > @@ -559,6 +565,7 @@ void bpf_map_area_free(void *base);
> > void bpf_map_init_from_attr(struct bpf_map *map, union bpf_attr *attr);
> >
> > extern int sysctl_unprivileged_bpf_disabled;
> > +extern int sysctl_bpf_stats_enabled;
> >
> > int bpf_map_new_fd(struct bpf_map *map, int flags);
> > int bpf_prog_new_fd(struct bpf_prog *prog);
> > diff --git a/include/linux/filter.h b/include/linux/filter.h
> > index f32b3eca5a04..7b14235b6f7d 100644
> > --- a/include/linux/filter.h
> > +++ b/include/linux/filter.h
> > @@ -533,7 +533,21 @@ struct sk_filter {
> > struct bpf_prog *prog;
> > };
> >
> > -#define BPF_PROG_RUN(filter, ctx) ({ cant_sleep(); (*(filter)->bpf_func)(ctx, (filter)->insnsi); })
> > +DECLARE_STATIC_KEY_FALSE(bpf_stats_enabled_key);
> > +
> > +#define BPF_PROG_RUN(prog, ctx) ({ \
> > + u32 ret; \
> > + cant_sleep(); \
> > + if (static_branch_unlikely(&bpf_stats_enabled_key)) { \
> > + u64 start = sched_clock(); \
> > + ret = (*(prog)->bpf_func)(ctx, (prog)->insnsi); \
> > + this_cpu_inc(prog->aux->stats->cnt); \
> > + this_cpu_add(prog->aux->stats->nsecs, \
> > + sched_clock() - start); \
> > + } else { \
> > + ret = (*(prog)->bpf_func)(ctx, (prog)->insnsi); \
> > + } \
> > + ret; })
>
> Can we bump "cnt" unconditionally and gate only the "nsecs" calculation?
xdp cannot afford to pay this penalty unconditionally.
It's not only this_cpu_inc. It's prog->aux->stats loads.
^ permalink raw reply
* [PATCH] net: phy: realtek: Dummy IRQ calls for RTL8366RB
From: Linus Walleij @ 2019-02-23 2:36 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli; +Cc: netdev, Linus Walleij, Heiner Kallweit
This fixes a regression introduced by
commit 0d2e778e38e0ddffab4bb2b0e9ed2ad5165c4bf7
"net: phy: replace PHY_HAS_INTERRUPT with a check for
config_intr and ack_interrupt".
This assumes that a PHY cannot trigger interrupt unless
it has .config_intr() or .ack_interrupt() implemented.
A later patch makes the code assume both need to be
implemented for interrupts to be present.
But this PHY (which is inside a DSA) will happily
fire interrupts without either callback.
Implement a dummy callback for .config_intr() and
.ack_interrupt() as a workaround.
Tested on the RTL8366RB on D-Link DIR-685.
Fixes: 0d2e778e38e0 ("net: phy: replace PHY_HAS_INTERRUPT with a check for config_intr and ack_interrupt")
Cc: Heiner Kallweit <hkallweit1@gmail.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/net/phy/realtek.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
index c6010fb1aa0f..31372be44570 100644
--- a/drivers/net/phy/realtek.c
+++ b/drivers/net/phy/realtek.c
@@ -211,6 +211,17 @@ static int rtl8366rb_config_init(struct phy_device *phydev)
return ret;
}
+static int rtl8366rb_ack_interrupt(struct phy_device *phydev)
+{
+ return 0;
+}
+
+static int rtl8366rb_config_intr(struct phy_device *phydev)
+{
+ /* Dummy function to make sure we get interrupts */
+ return 0;
+}
+
static struct phy_driver realtek_drvs[] = {
{
PHY_ID_MATCH_EXACT(0x00008201),
@@ -282,6 +293,8 @@ static struct phy_driver realtek_drvs[] = {
.name = "RTL8366RB Gigabit Ethernet",
.features = PHY_GBIT_FEATURES,
.config_init = &rtl8366rb_config_init,
+ .ack_interrupt = &rtl8366rb_ack_interrupt,
+ .config_intr = &rtl8366rb_config_intr,
.suspend = genphy_suspend,
.resume = genphy_resume,
},
--
2.20.1
^ permalink raw reply related
* Re: [PATCH bpf-next 1/4] bpf: enable program stats
From: Alexei Starovoitov @ 2019-02-23 2:38 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Roman Gushchin, Alexei Starovoitov, davem@davemloft.net,
netdev@vger.kernel.org, bpf@vger.kernel.org, Kernel Team
In-Reply-To: <8de20612-c32a-9370-254d-9ebf1df78efa@iogearbox.net>
On Sat, Feb 23, 2019 at 02:06:56AM +0100, Daniel Borkmann wrote:
>
> In general, having some stats and timing info would be useful, but I
> guess people might want to customize it in future even more specifically
> beyond number of runs + time it takes. One thing that would be super
> useful is to have some notion of __attribute__((constructor)) and
> __attribute__((destructor)) support in BPF which gets then inlined into
> prologue / epilogue of program. E.g. such infrastructure would allow to
> mark an skb and measure time it takes through the BPF prog till it hits
> an exit point somewhere (without having to explicitly code this into the
> program everywhere). Other examples may be histograms or p99 latencies
> that might probably be useful. Feels like for monitoring more ways to
> program would be nice and to move it into the BPF insns sequence (e.g.
> enforced globally or by choice of prog as another option)? Thoughts?
the constructor/destructor you mean to capture the full sequence of tail_calls?
Or full path of skb through the stack with all hook points?
That is likely very useful without any bpf, but I think hw timestamping
already serves that purpose.
I've been thinking about doing this stats per program
(instead of static_key for all).
Like replacing a bpf_prog->bpf_func with a wrapper function
that does stats, but that is more costly in retpoline world
due to extra indirect call.
Another alternative is to patch the stats in via JITs, but
it's much more complex and error prone.
So went with the simplest approach.
With all stats ideas we need to be careful not to reintroduce what
perf is already great at.
These stats are _not_ for performance analysis. That's what perf does.
These stats are for 24/7 monitoring to catch things that not suppose to happen.
^ permalink raw reply
* Re: [PATCH 1/2 v2] kprobe: Do not use uaccess functions to access kernel memory that can fault
From: Linus Torvalds @ 2019-02-23 2:32 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: David Miller, Masami Hiramatsu, Steven Rostedt, Andy Lutomirski,
Linux List Kernel Mailing, Ingo Molnar, Andrew Morton, stable,
Changbin Du, Jann Horn, Kees Cook, Andrew Lutomirski,
Daniel Borkmann, Netdev, bpf
In-Reply-To: <20190223022850.nv4hnweueetprbot@ast-mbp.dhcp.thefacebook.com>
On Fri, Feb 22, 2019 at 6:29 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> imo this kernel release should finish as-is and in the next cycle we can
> change probe_kernel_read() to reject user address [..]
Absolutely. Nothing is going to change right now for 5.0, which is imminent.
It's really a "long-term we really need to fix this", where the only
question is how soon "long-term" is.
Linus
^ permalink raw reply
* Re: [PATCH net-next v3] ip_tunnel: Add dst_cache support in lwtunnel_state of ip tunnel
From: David Ahern @ 2019-02-23 2:45 UTC (permalink / raw)
To: wenxu, davem; +Cc: netdev
In-Reply-To: <1550881979-12589-1-git-send-email-wenxu@ucloud.cn>
On 2/22/19 7:32 PM, wenxu@ucloud.cn wrote:
> diff --git a/net/core/lwtunnel.c b/net/core/lwtunnel.c
> index 0b17175..f6301af 100644
> --- a/net/core/lwtunnel.c
> +++ b/net/core/lwtunnel.c
> @@ -123,17 +123,18 @@ int lwtunnel_build_state(u16 encap_type,
> rcu_read_lock();
> ops = rcu_dereference(lwtun_encaps[encap_type]);
> if (likely(ops && ops->build_state && try_module_get(ops->owner))) {
> + rcu_read_unlock();
> found = true;
> ret = ops->build_state(encap, family, cfg, lws, extack);
> if (ret)
> module_put(ops->owner);
> }
> - rcu_read_unlock();
>
> /* don't rely on -EOPNOTSUPP to detect match as build_state
> * handlers could return it
> */
> if (!found) {
> + rcu_read_unlock();
I was thinking something much simpler ... like:
rcu_read_lock();
ops = rcu_dereference(lwtun_encaps[encap_type]);
if (likely(ops && ops->build_state && try_module_get(ops->owner))) {
found = true;
rcu_read_unlock();
if (found) {
ret = ops->build_state(encap, family, cfg, lws, extack);
if (ret)
module_put(ops->owner);
} else {
NL_SET_ERR_MSG_ATTR(...);
}
>
> NL_SET_ERR_MSG_ATTR(extack, encap,
> "LWT encapsulation type not supported");
> }
> diff --git a/net/ipv4/ip_tunnel_core.c b/net/ipv4/ip_tunnel_core.c
> index 9a0e67b..334e77a 100644
> --- a/net/ipv4/ip_tunnel_core.c
> +++ b/net/ipv4/ip_tunnel_core.c
> @@ -252,6 +252,14 @@ static int ip_tun_build_state(struct nlattr *attr,
>
> tun_info = lwt_tun_info(new_state);
>
> +#ifdef CONFIG_DST_CACHE
> + err = dst_cache_init(&tun_info->dst_cache, GFP_KERNEL);
> + if (err) {
> + lwtstate_free(new_state);
> + return err;
> + }
> +#endif
> +
> if (tb[LWTUNNEL_IP_ID])
> tun_info->key.tun_id = nla_get_be64(tb[LWTUNNEL_IP_ID]);
>
> @@ -278,6 +286,15 @@ static int ip_tun_build_state(struct nlattr *attr,
> return 0;
> }
>
> +#ifdef CONFIG_DST_CACHE
> +static void ip_tun_destroy_state(struct lwtunnel_state *lwtstate)
> +{
> + struct ip_tunnel_info *tun_info = lwt_tun_info(lwtstate);
> +
> + dst_cache_destroy(&tun_info->dst_cache);
> +}> +#endif
> +
> static int ip_tun_fill_encap_info(struct sk_buff *skb,
> struct lwtunnel_state *lwtstate)
> {
> @@ -313,6 +330,9 @@ static int ip_tun_cmp_encap(struct lwtunnel_state *a, struct lwtunnel_state *b)
>
> static const struct lwtunnel_encap_ops ip_tun_lwt_ops = {
> .build_state = ip_tun_build_state,
> +#ifdef CONFIG_DST_CACHE
> + .destroy_state = ip_tun_destroy_state,
> +#endif
Define ip_tun_destroy_state when DST_CACHE is not enabled so you don't
need this ifdef as well. e.g., move the ifdef around dst_cache_destroy
and its local arg to mirror the init.
^ permalink raw reply
* Re: [PATCH 1/2 v2] kprobe: Do not use uaccess functions to access kernel memory that can fault
From: Steven Rostedt @ 2019-02-23 3:02 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Linus Torvalds, David Miller, Masami Hiramatsu, Andy Lutomirski,
Linux List Kernel Mailing, Ingo Molnar, Andrew Morton, stable,
Changbin Du, Jann Horn, Kees Cook, Andrew Lutomirski,
Daniel Borkmann, Netdev, bpf
In-Reply-To: <20190223022850.nv4hnweueetprbot@ast-mbp.dhcp.thefacebook.com>
On Fri, 22 Feb 2019 18:28:53 -0800
Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> First we introduce bpf_probe_kernel_read and bpf_probe_user_read and
> introduce clang/gcc tooling to catch the mistakes.
> Going over this 400+ places and manually grepping kernel sources
> for __user keyword is not a great proposal if we want to keep those users.
> Once we have this working we can remove bpf_probe_read() altogether.
> Rejecting bpf prog at load time is a clear signal that user has to fix it
> (instead of changing run-time behavior).
> When the verifier gets even smarter it could potentially replace prob_read
> with probe_kernel_read and probe_user_read when it has that type info.
I was about to suggest this approach. Document that bpf_probe_read() is
known to be buggy and will be deprecated in the future, and that all
new bpf scripts should start using bpf_probe_kernel/user_read()
instead (after they have been implemented of course). And give time for
people to fix their current scripts.
Perhaps in the near future, trigger some kind of warning for users that
use bpf_probe_read().
-- Steve
^ permalink raw reply
* Re: [PATCH v2 bpf-next 0/9] bpf: Network Resource Manager (NRM)
From: David Ahern @ 2019-02-23 3:03 UTC (permalink / raw)
To: brakmo, netdev
Cc: Martin Lau, Alexei Starovoitov, Daniel Borkmann, Eric Dumazet,
Kernel Team
In-Reply-To: <20190223010703.678070-1-brakmo@fb.com>
On 2/22/19 8:06 PM, brakmo wrote:
> Network Resource Manager is a framework for limiting the bandwidth used
> by v2 cgroups. It consists of 4 BPF helpers and a sample BPF program to
> limit egress bandwdith as well as a sample user program and script to
> simplify NRM testing.
'resource manager' is a really generic name. Since you are referring to
bandwidth, how about renaming to Network Bandwidth Manager?
^ permalink raw reply
* Re: [PATCH net-next] route: Add a new fib_multipath_hash_policy base on cpu id for tunnel packet
From: David Ahern @ 2019-02-23 3:19 UTC (permalink / raw)
To: wenxu, davem; +Cc: netdev, Ido Schimmel
In-Reply-To: <1550807549-22720-1-git-send-email-wenxu@ucloud.cn>
On 2/21/19 10:52 PM, wenxu@ucloud.cn wrote:
> From: wenxu <wenxu@ucloud.cn>
>
> Current fib_multipath_hash_policy can make hash based on the L3 or
> L4. But it only work on the outer IP. So a specific tunnel always
> has the same hash value. But a specific tunnel may contain so many
> inner connection. However there is no good ways for tunnel packet.
> A specific tunnel route based on the percpu dst_cache, It will not
> lookup route table each packet.
>
> This patch provide a based cpu id hash policy. The different
> connection run on differnt cpu and There will differnet hash
> value for percpu dst_cache.
>
> Signed-off-by: wenxu <wenxu@ucloud.cn>
> ---
> net/ipv4/route.c | 6 ++++++
> net/ipv4/sysctl_net_ipv4.c | 2 +-
> 2 files changed, 7 insertions(+), 1 deletion(-)
>
This multipath hash policy is global - for all fib lookups, not just
tunnels.
The suggestion by Nik is worth exploring - an option to add the mark to
the hash (e.g., L3 header + mark) which makes this more generic.
If the policy options are changed, the call to call_netevent_notifiers
needs to be updated to handle a failure. For example, the mlxsw handler
needs to be able to veto an option it does not support.
^ permalink raw reply
* Re: [PATCH bpf-next 4/4] tools/bpftool: recognize bpf_prog_info runtime and runcnt
From: Andrii Nakryiko @ 2019-02-23 4:17 UTC (permalink / raw)
To: Alexei Starovoitov; +Cc: davem, Daniel Borkmann, netdev, bpf, Kernel Team
In-Reply-To: <20190222233644.1487087-5-ast@kernel.org>
On Fri, Feb 22, 2019 at 3:37 PM Alexei Starovoitov <ast@kernel.org> wrote:
>
> $ bpftool p s
> 1: kprobe tag a56587d488d216c9 gpl runtime 79786 runcnt 8
> loaded_at 2019-02-22T12:22:51-0800 uid 0
> xlated 352B not jited memlock 4096B
>
> $ bpftool --json --pretty p s
> [{
> "id": 1,
> "type": "kprobe",
> "tag": "a56587d488d216c9",
> "gpl_compatible": true,
> "runtime": 79786,
> "runcnt": 8,
> "loaded_at": 1550866971,
> "uid": 0,
> "bytes_xlated": 352,
> "jited": false,
> "bytes_memlock": 4096
> }
> ]
>
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> ---
> tools/bpf/bpftool/Documentation/bpftool-prog.rst | 4 +++-
> tools/bpf/bpftool/prog.c | 7 +++++++
> 2 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/tools/bpf/bpftool/Documentation/bpftool-prog.rst b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> index 12bc1e2d4b46..102b180d3add 100644
> --- a/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> +++ b/tools/bpf/bpftool/Documentation/bpftool-prog.rst
> @@ -171,7 +171,7 @@ EXAMPLES
>
> ::
>
> - 10: xdp name some_prog tag 005a3d2123620c8b gpl
> + 10: xdp name some_prog tag 005a3d2123620c8b gpl runtime 81632 runcnt 10
> loaded_at 2017-09-29T20:11:00+0000 uid 0
> xlated 528B jited 370B memlock 4096B map_ids 10
>
> @@ -184,6 +184,8 @@ EXAMPLES
> "type": "xdp",
> "tag": "005a3d2123620c8b",
> "gpl_compatible": true,
> + "runtime": 81632,
> + "runcnt": 10,
Should these fields be called "run_time" and "run_cnt" for consistency
with other fields? I'd even suggest "run_time_ns" so that there is no
question about time units.
> "loaded_at": 1506715860,
> "uid": 0,
> "bytes_xlated": 528,
> diff --git a/tools/bpf/bpftool/prog.c b/tools/bpf/bpftool/prog.c
> index db978c8d76a8..8065ba11b9d5 100644
> --- a/tools/bpf/bpftool/prog.c
> +++ b/tools/bpf/bpftool/prog.c
> @@ -214,6 +214,10 @@ static void print_prog_json(struct bpf_prog_info *info, int fd)
> info->tag[4], info->tag[5], info->tag[6], info->tag[7]);
>
> jsonw_bool_field(json_wtr, "gpl_compatible", info->gpl_compatible);
> + if (info->runtime) {
> + jsonw_uint_field(json_wtr, "runtime", info->runtime);
> + jsonw_uint_field(json_wtr, "runcnt", info->runcnt);
> + }
>
> print_dev_json(info->ifindex, info->netns_dev, info->netns_ino);
>
> @@ -277,6 +281,9 @@ static void print_prog_plain(struct bpf_prog_info *info, int fd)
> fprint_hex(stdout, info->tag, BPF_TAG_SIZE, "");
> print_dev_plain(info->ifindex, info->netns_dev, info->netns_ino);
> printf("%s", info->gpl_compatible ? " gpl" : "");
> + if (info->runtime)
> + printf(" runtime %lld runcnt %lld",
> + info->runtime, info->runcnt);
> printf("\n");
>
> if (info->load_time) {
> --
> 2.20.0
>
^ permalink raw reply
* Re: pull-request: bpf 2019-02-23
From: David Miller @ 2019-02-23 4:45 UTC (permalink / raw)
To: daniel; +Cc: ast, netdev, bpf
In-Reply-To: <20190223003625.32021-1-daniel@iogearbox.net>
From: Daniel Borkmann <daniel@iogearbox.net>
Date: Sat, 23 Feb 2019 01:36:25 +0100
> Please consider pulling these changes from:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git
Pulled, thanks Daniel.
^ permalink raw reply
* Re: [PATCH 1/2 v2] kprobe: Do not use uaccess functions to access kernel memory that can fault
From: Masami Hiramatsu @ 2019-02-23 4:51 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Linus Torvalds, David Miller, Masami Hiramatsu, Steven Rostedt,
Andy Lutomirski, Linux List Kernel Mailing, Ingo Molnar,
Andrew Morton, stable, Changbin Du, Jann Horn, Kees Cook,
Andrew Lutomirski, Daniel Borkmann, Netdev, bpf
In-Reply-To: <20190222235618.dxewmv5dukltaoxl@ast-mbp.dhcp.thefacebook.com>
On Fri, 22 Feb 2019 15:56:20 -0800
Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> On Fri, Feb 22, 2019 at 03:16:35PM -0800, Linus Torvalds wrote:
> >
> > So a kernel pointer value of 0x12345678 could be a value kernel
> > pointer pointing to some random kmalloc'ed kernel memory, and a user
> > pointer value of 0x12345678 could be a valid _user_ pointer pointing
> > to some user mapping.
> >
> > See?
> >
> > If you access a user pointer, you need to use a user accessor function
> > (eg "get_user()"), while if you access a kernel pointer you need to
> > just dereference it directly (unless you can't trust it, in which case
> > you need to use a _different_ accessor function).
>
> that was clear already.
> Reading 0x12345678 via probe_kernel_read can return valid value
> and via get_user() can return another valid value on _some_ architectures.
>
> > The fact that user and kernel pointers happen to be distinct on x86-64
> > (right now) is just a random implementation detail.
>
> yes and my point that people already rely on this implementation detail.
> Say we implement
> int bpf_probe_read(void *val, void *unsafe_ptr)
> {
> if (probe_kernel_read(val, unsafe_ptr) == OK) {
> return 0;
> } else (get_user(val, unsafe_ptr) == OK) {
> return 0;
> } else {
> *val = 0;
> return -EFAULT;
> }
> }
Note that we can not use get_user() form kprobe handler. If you use it,
you have to prepare fault_handler() and make bpf itself can be aborted.
So, maybe you can use probe_user_read().
Hmm, however, it still doesn't work correctly on "some" architecture,
since whether a pointer (address) points user-space or kernel-space
depends on the context. In kprobe/bpf, the context means where you
put the probe and which pointer you record.
I think only "__user" tag tells us which one is user-space. But
unfortunately, that "__user" tag is only for compiler or checker, not
for runtime binary. Such useful attribute goes away when we execute it.
So, even if we introduce "ustring", ftrace/perf users has to decide to use
it by themselves. As far as I know, DWARF(debuginfo) also doesn't have
that attribute. So perf-probe can not help it from debuginfo.
(Maybe if we introduce C parser, it might be detected...)
> It will preserve existing bpf_probe_read() behavior on x86.
> If x86 implementation changes tomorrow then progs that read user
> addresses may start failing randomly because first probe_kernel_read()
> will be returning random values from kernel memory and that's no good,
> but at least we won't be breaking them today, so we have time to
> introduce bpf_user_read and bpf_kernel_read and folks have time to adopt them.
I see. I think bpf also has to introduce new bpf_probe_read_user() and
keep bpf_probe_read() for kernel dataa only.
> Imo that's much better than making current bpf_probe_read() fail
> on user addresses today and not providing a non disruptive path forward.
Agreed.
Thank you,
--
Masami Hiramatsu <mhiramat@kernel.org>
^ permalink raw reply
* [PATCH] ila: Fix uninitialised return value in ila_xlat_nl_cmd_flush
From: Herbert Xu @ 2019-02-23 5:30 UTC (permalink / raw)
To: Dan Carpenter; +Cc: kbuild, kbuild-all, linux-wireless, Johannes Berg, netdev
In-Reply-To: <20190223042747.GK1711@kadam>
On Sat, Feb 23, 2019 at 07:27:47AM +0300, Dan Carpenter wrote:
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git master
> head: db0342b20f32f584aedff27c9c09e0a4bbe5beff
> commit: 6c4128f658571b2dc7e01058ad09a8e947bc0159 [25/27] rhashtable: Remove obsolete rhashtable_walk_init function
>
> smatch warnings:
> net/ipv6/ila/ila_xlat.c:420 ila_xlat_nl_cmd_flush() error: uninitialized symbol 'ret'.
Thanks for catching this.
---8<---
This patch fixes an uninitialised return value error in
ila_xlat_nl_cmd_flush.
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Fixes: 6c4128f65857 ("rhashtable: Remove obsolete...")
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
diff --git a/net/ipv6/ila/ila_xlat.c b/net/ipv6/ila/ila_xlat.c
index ae6cd4cef8db..79d2e43c05c5 100644
--- a/net/ipv6/ila/ila_xlat.c
+++ b/net/ipv6/ila/ila_xlat.c
@@ -383,7 +383,7 @@ int ila_xlat_nl_cmd_flush(struct sk_buff *skb, struct genl_info *info)
struct rhashtable_iter iter;
struct ila_map *ila;
spinlock_t *lock;
- int ret;
+ int ret = 0;
rhashtable_walk_enter(&ilan->xlat.rhash_table, &iter);
rhashtable_walk_start(&iter);
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply related
* Re: [PATCH net-next] net: dsa: add missing of_node_put
From: Himadri Pandya @ 2019-02-23 5:53 UTC (permalink / raw)
To: Andrew Lunn, wen.yang99
Cc: vivien.didelot, f.fainelli, davem, netdev, linux-kernel
In-Reply-To: <20190222143649.GN5653@lunn.ch>
On 22/02/19 8:06 PM, Andrew Lunn wrote:
> On Fri, Feb 22, 2019 at 04:48:18PM +0530, Himadri Pandya wrote:
>> Decrement the reference count on port while returning out of the
>> loop. Issue identified by Coccinelle.
> You and Wen Yang are both fixing the same issue. Maybe you can
> coordinate?
Sure.
- Himadri
>
> Andrew
^ permalink raw reply
* RE: [PATCH 1/3] net: ethernet: add support for PCS and 2.5G speed
From: Parshuram Raju Thombare @ 2019-02-23 6:24 UTC (permalink / raw)
To: Andrew Lunn
Cc: nicolas.ferre@microchip.com, davem@davemloft.net,
netdev@vger.kernel.org, f.fainelli@gmail.com,
hkallweit1@gmail.com, linux-kernel@vger.kernel.org,
Rafal Ciepiela, Piotr Sroka, Jan Kotas
In-Reply-To: <20190222213953.GN5894@lunn.ch>
>> /* mask with MAC supported features */
>> - if (macb_is_gem(bp) && bp->caps &
>MACB_CAPS_GIGABIT_MODE_AVAILABLE)
>> - phy_set_max_speed(phydev, SPEED_1000);
>> - else
>> - phy_set_max_speed(phydev, SPEED_100);
>> + if (macb_is_gem(bp)) {
>> + linkmode_copy(phydev->supported, PHY_GBIT_FEATURES);
>> + if (bp->caps & MACB_CAPS_TWO_PT_FIVE_GIG_SPEED)
>> +
> linkmode_set_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT,
>> + phydev->supported);
>> + } else {
>> + linkmode_copy(phydev->supported, PHY_BASIC_FEATURES);
>> + }
>> +
>> + linkmode_copy(phydev->advertising, phydev->supported);
>
>This is not correct. Just because the MAC can do 2.5G does not mean the PHY
>can. So you should not be adding links modes. Also, somebody might be using a
>PHY that can do 2.5G with a MAC which can only do 1G.
>
>The correct thing to do is call phy_set_max_speed() with the maximum speed the
>MAC can do.
Hi Andrew,
Ok, I think this should have been logical AND. I will modify to use phy_set_max_speed()
instead of directly copying linkmodes.
Regards,
Parshuram Thombare
^ permalink raw reply
* RE: [PATCH 3/3] net: ethernet: add support for high speed mac and usxgmii pcs
From: Parshuram Raju Thombare @ 2019-02-23 6:24 UTC (permalink / raw)
To: Andrew Lunn
Cc: nicolas.ferre@microchip.com, davem@davemloft.net,
netdev@vger.kernel.org, f.fainelli@gmail.com,
hkallweit1@gmail.com, linux-kernel@vger.kernel.org,
Rafal Ciepiela, Piotr Sroka, Jan Kotas
In-Reply-To: <20190222214404.GP5894@lunn.ch>
>> if (macb_is_gem(bp)) {
>> - linkmode_copy(phydev->supported, PHY_GBIT_FEATURES);
>> - if (bp->caps & MACB_CAPS_TWO_PT_FIVE_GIG_SPEED)
>> -
> linkmode_set_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT,
>> - phydev->supported);
>> + if (bp->caps & MACB_CAPS_HIGH_SPEED) {
>> + linkmode_copy(phydev->supported,
>PHY_10GBIT_FEATURES);
>> + } else {
>> + u32 bitmask =
>ETHTOOL_LINK_MODE_2500baseT_Full_BIT;
>> +
>> + linkmode_copy(phydev->supported,
>PHY_GBIT_FEATURES);
>> + if (bp->caps & MACB_CAPS_TWO_PT_FIVE_GIG_SPEED)
>> + linkmode_set_bit(bitmask, phydev->supported);
>> + }
>
>Same issue again. Somebody could be using a 10G MAC with a 2.5G PHY.
>
> Andrew
Hi Andrew,
Ok, I think this should have been logical AND. I will modify to use phy_set_max_speed()
instead of directly copying linkmodes.
Regards,
Parshuram Thombare
^ permalink raw reply
* RE: [PATCH 2/3] net: ethernet: add c45 PHY support in MDIO read/write functions.
From: Parshuram Raju Thombare @ 2019-02-23 6:27 UTC (permalink / raw)
To: Andrew Lunn
Cc: nicolas.ferre@microchip.com, davem@davemloft.net,
netdev@vger.kernel.org, f.fainelli@gmail.com,
hkallweit1@gmail.com, linux-kernel@vger.kernel.org,
Rafal Ciepiela, Piotr Sroka, Jan Kotas
In-Reply-To: <20190222214140.GO5894@lunn.ch>
Regards,
Parshuram Thombare
>-----Original Message-----
>From: Andrew Lunn <andrew@lunn.ch>
>Sent: Saturday, February 23, 2019 3:12 AM
>To: Parshuram Raju Thombare <pthombar@cadence.com>
>Cc: nicolas.ferre@microchip.com; davem@davemloft.net;
>netdev@vger.kernel.org; f.fainelli@gmail.com; hkallweit1@gmail.com; linux-
>kernel@vger.kernel.org; Rafal Ciepiela <rafalc@cadence.com>; Piotr Sroka
><piotrs@cadence.com>; Jan Kotas <jank@cadence.com>
>Subject: Re: [PATCH 2/3] net: ethernet: add c45 PHY support in MDIO read/write
>functions.
>
>EXTERNAL MAIL
>
>
>On Fri, Feb 22, 2019 at 08:12:42PM +0000, Parshuram Thombare wrote:
>> This patch modify MDIO read/write functions to support communication
>> with C45 PHY in Cadence ethernet controller driver.
>
>Hi Parshuram
>
>Are all versions of the MDIO controller capable of doing C45?
>
> Andrew
Now driver support c22 and c45 PHY.
Are you suggesting to add check for C45 PHY using is_c45 in phydev ?
Regards,
Parshuram Thombare
^ permalink raw reply
* [PATCH v2 0/6] RCU fixes for rcu_assign_pointer() usage
From: Joel Fernandes (Google) @ 2019-02-23 6:34 UTC (permalink / raw)
To: linux-kernel
Cc: Joel Fernandes (Google), Alexei Starovoitov, Christian Brauner,
Daniel Borkmann, David Ahern, David S. Miller, Ingo Molnar,
Jakub Kicinski, Jeff Kirsher, Jesper Dangaard Brouer,
John Fastabend, Josh Triplett, keescook, kernel-hardening,
kernel-team, Kirill Tkhai, Lai Jiangshan, Martin KaFai Lau,
Mathieu Desnoyers, netdev, Paul E. McKenney, Peter Zijlstra,
Quentin Perret, rcu, Song Liu, Steven Rostedt, Vincent Guittot,
xdp-newbies, Yonghong Song
These patches fix various sparse errors found as a result of the recent check
to add rcu_check_sparse() to rcu_assign_pointer(). The errors in some cases
seem to either missing API usage, or missing annotations. The annotations added
in the series can also help avoid future incorrect usages and bugs so it is a
good idea to do in any case.
RFC v1 -> Patch v2:
Made changes to various scheduler patches (Peter Zijlstra)
Joel Fernandes (Google) (6):
net: rtnetlink: Fix incorrect RCU API usage
ixgbe: Fix incorrect RCU API usage
sched/cpufreq: Annotate cpufreq_update_util_data pointer with __rcu
sched_domain: Annotate RCU pointers properly
rcuwait: Annotate task_struct with __rcu
sched: Annotate perf_domain pointer with __rcu
drivers/net/ethernet/intel/ixgbe/ixgbe.h | 4 ++--
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 15 ++++++++++-----
include/linux/rcuwait.h | 2 +-
include/linux/sched/topology.h | 4 ++--
kernel/sched/cpufreq.c | 2 +-
kernel/sched/sched.h | 18 +++++++++---------
kernel/sched/topology.c | 10 +++++-----
net/core/rtnetlink.c | 4 ++--
8 files changed, 32 insertions(+), 27 deletions(-)
--
2.21.0.rc0.258.g878e2cd30e-goog
^ permalink raw reply
* [PATCH v2 1/6] net: rtnetlink: Fix incorrect RCU API usage
From: Joel Fernandes (Google) @ 2019-02-23 6:34 UTC (permalink / raw)
To: linux-kernel
Cc: Joel Fernandes (Google), Alexei Starovoitov, Christian Brauner,
Daniel Borkmann, David Ahern, David S. Miller, Ingo Molnar,
Jakub Kicinski, Jeff Kirsher, Jesper Dangaard Brouer,
John Fastabend, Josh Triplett, keescook, kernel-hardening,
kernel-team, Kirill Tkhai, Lai Jiangshan, Martin KaFai Lau,
Mathieu Desnoyers, netdev, Paul E. McKenney, Peter Zijlstra,
Quentin Perret, rcu, Song Liu, Steven Rostedt, Vincent Guittot,
xdp-newbies, Yonghong Song
In-Reply-To: <20190223063434.6793-1-joel@joelfernandes.org>
rtnl_register_internal() and rtnl_unregister_all tries to directly
dereference an RCU protected pointed outside RCU read side section.
While this is Ok to do since a lock is held, let us use the correct
API to avoid programmer bugs in the future.
This also fixes sparse warnings arising from not using RCU API.
net/core/rtnetlink.c:332:13: warning: incorrect type in assignment
(different address spaces) net/core/rtnetlink.c:332:13: expected
struct rtnl_link **tab net/core/rtnetlink.c:332:13: got struct
rtnl_link *[noderef] <asn:4>*<noident>
Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
net/core/rtnetlink.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 5ea1bed08ede..98be4b4818a9 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -188,7 +188,7 @@ static int rtnl_register_internal(struct module *owner,
msgindex = rtm_msgindex(msgtype);
rtnl_lock();
- tab = rtnl_msg_handlers[protocol];
+ tab = rtnl_dereference(rtnl_msg_handlers[protocol]);
if (tab == NULL) {
tab = kcalloc(RTM_NR_MSGTYPES, sizeof(void *), GFP_KERNEL);
if (!tab)
@@ -329,7 +329,7 @@ void rtnl_unregister_all(int protocol)
BUG_ON(protocol < 0 || protocol > RTNL_FAMILY_MAX);
rtnl_lock();
- tab = rtnl_msg_handlers[protocol];
+ tab = rtnl_dereference(rtnl_msg_handlers[protocol]);
if (!tab) {
rtnl_unlock();
return;
--
2.21.0.rc0.258.g878e2cd30e-goog
^ permalink raw reply related
* [PATCH v2 2/6] ixgbe: Fix incorrect RCU API usage
From: Joel Fernandes (Google) @ 2019-02-23 6:34 UTC (permalink / raw)
To: linux-kernel
Cc: Joel Fernandes (Google), Alexei Starovoitov, Christian Brauner,
Daniel Borkmann, David Ahern, David S. Miller, Ingo Molnar,
Jakub Kicinski, Jeff Kirsher, Jesper Dangaard Brouer,
John Fastabend, Josh Triplett, keescook, kernel-hardening,
kernel-team, Kirill Tkhai, Lai Jiangshan, Martin KaFai Lau,
Mathieu Desnoyers, netdev, Paul E. McKenney, Peter Zijlstra,
Quentin Perret, rcu, Song Liu, Steven Rostedt, Vincent Guittot,
xdp-newbies, Yonghong Song
In-Reply-To: <20190223063434.6793-1-joel@joelfernandes.org>
Recently, I added an RCU annotation check in rcu_assign_pointer. This
caused a sparse error to be reported by the ixgbe driver.
Further looking, it seems the adapter->xdp_prog pointer is not annotated
with __rcu. Annonating it fixed the error, but caused a bunch of other
warnings.
This patch tries to fix all warnings by using RCU API properly. This
makes sense to do because not using RCU properly can result in various
hard to find bugs. This is a best effort fix and is only build tested.
The sparse errors and warnings go away with the change. I request
maintainers / developers in this area to review / test it properly.
The sparse error fixed is:
ixgbe_main.c:10256:25: error: incompatible types in comparison expression
Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
drivers/net/ethernet/intel/ixgbe/ixgbe.h | 4 ++--
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 15 ++++++++++-----
2 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
index 08d85e336bd4..3b14daf27516 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
@@ -311,7 +311,7 @@ struct ixgbe_ring {
struct ixgbe_ring *next; /* pointer to next ring in q_vector */
struct ixgbe_q_vector *q_vector; /* backpointer to host q_vector */
struct net_device *netdev; /* netdev ring belongs to */
- struct bpf_prog *xdp_prog;
+ struct bpf_prog __rcu *xdp_prog;
struct device *dev; /* device for DMA mapping */
void *desc; /* descriptor ring memory */
union {
@@ -560,7 +560,7 @@ struct ixgbe_adapter {
unsigned long active_vlans[BITS_TO_LONGS(VLAN_N_VID)];
/* OS defined structs */
struct net_device *netdev;
- struct bpf_prog *xdp_prog;
+ struct bpf_prog __rcu *xdp_prog;
struct pci_dev *pdev;
struct mii_bus *mii_bus;
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index daff8183534b..408a312aa6ba 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -2199,7 +2199,7 @@ static struct sk_buff *ixgbe_run_xdp(struct ixgbe_adapter *adapter,
u32 act;
rcu_read_lock();
- xdp_prog = READ_ONCE(rx_ring->xdp_prog);
+ xdp_prog = rcu_dereference(rx_ring->xdp_prog);
if (!xdp_prog)
goto xdp_out;
@@ -6547,7 +6547,7 @@ int ixgbe_setup_rx_resources(struct ixgbe_adapter *adapter,
rx_ring->queue_index) < 0)
goto err;
- rx_ring->xdp_prog = adapter->xdp_prog;
+ rcu_assign_pointer(rx_ring->xdp_prog, adapter->xdp_prog);
return 0;
err:
@@ -10246,7 +10246,8 @@ static int ixgbe_xdp_setup(struct net_device *dev, struct bpf_prog *prog)
if (nr_cpu_ids > MAX_XDP_QUEUES)
return -ENOMEM;
- old_prog = xchg(&adapter->xdp_prog, prog);
+ old_prog = rcu_access_pointer(adapter->xdp_prog);
+ rcu_assign_pointer(adapter->xdp_prog, prog);
/* If transitioning XDP modes reconfigure rings */
if (!!prog != !!old_prog) {
@@ -10271,13 +10272,17 @@ static int ixgbe_xdp_setup(struct net_device *dev, struct bpf_prog *prog)
static int ixgbe_xdp(struct net_device *dev, struct netdev_bpf *xdp)
{
struct ixgbe_adapter *adapter = netdev_priv(dev);
+ struct bpf_prog *prog;
switch (xdp->command) {
case XDP_SETUP_PROG:
return ixgbe_xdp_setup(dev, xdp->prog);
case XDP_QUERY_PROG:
- xdp->prog_id = adapter->xdp_prog ?
- adapter->xdp_prog->aux->id : 0;
+ rcu_read_lock();
+ prog = rcu_dereference(adapter->xdp_prog);
+ xdp->prog_id = prog ? prog->aux->id : 0;
+ rcu_read_unlock();
+
return 0;
case XDP_QUERY_XSK_UMEM:
return ixgbe_xsk_umem_query(adapter, &xdp->xsk.umem,
--
2.21.0.rc0.258.g878e2cd30e-goog
^ permalink raw reply related
* [PATCH v2 6/6] sched: Annotate perf_domain pointer with __rcu
From: Joel Fernandes (Google) @ 2019-02-23 6:34 UTC (permalink / raw)
To: linux-kernel
Cc: Joel Fernandes (Google), Alexei Starovoitov, Christian Brauner,
Daniel Borkmann, David Ahern, David S. Miller, Ingo Molnar,
Jakub Kicinski, Jeff Kirsher, Jesper Dangaard Brouer,
John Fastabend, Josh Triplett, keescook, kernel-hardening,
kernel-team, Kirill Tkhai, Lai Jiangshan, Martin KaFai Lau,
Mathieu Desnoyers, netdev, Paul E. McKenney, Peter Zijlstra,
Quentin Perret, rcu, Song Liu, Steven Rostedt, Vincent Guittot,
xdp-newbies, Yonghong Song
In-Reply-To: <20190223063434.6793-1-joel@joelfernandes.org>
This fixes the following sparse errors in sched/fair.c:
fair.c:6506:14: error: incompatible types in comparison expression
fair.c:8642:21: error: incompatible types in comparison expression
Using __rcu will also help sparse catch any future bugs.
Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
kernel/sched/sched.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index ca6a79f57e7a..c8e6514433a9 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -780,7 +780,7 @@ struct root_domain {
* NULL-terminated list of performance domains intersecting with the
* CPUs of the rd. Protected by RCU.
*/
- struct perf_domain *pd;
+ struct perf_domain __rcu *pd;
};
extern struct root_domain def_root_domain;
--
2.21.0.rc0.258.g878e2cd30e-goog
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox