Netdev List
 help / color / mirror / Atom feed
* [PATCH bpf-next 9/9] bpf: NRM test script
From: brakmo @ 2019-02-19  5:38 UTC (permalink / raw)
  To: netdev; +Cc: Martin Lau, Alexei Starovoitov, Daniel Borkmann --cc=Kernel Team

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 | 429 +++++++++++++++++++++++++++++++++++++
 1 file changed, 429 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..c068c6e889d9
--- /dev/null
+++ b/samples/bpf/do_nrm_test.sh
@@ -0,0 +1,429 @@
+#!/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.$id.$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
+(while [ $n -lt $m ] ; \
+ do ss -6 -i -t -m -o state established "( dport = :$port )" >> ss.out ; \
+    n=$[n+1] ; usleep 200000 ; done \
+ ) &
+
+( 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
+
+if [ "$dir" == "-i" ] ; then
+  ./load_cg_skb -ri $cg_dir > /dev/null 2>&1
+else
+  ./load_cg_skb -r $cg_dir > /dev/null 2>&1
+fi
+
+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

* [PATCH bpf-next 8/9] bpf: User program for testing NRM
From: brakmo @ 2019-02-19  5:38 UTC (permalink / raw)
  To: netdev; +Cc: Martin Lau, Alexei Starovoitov, Daniel Borkmann --cc=Kernel Team

The program nrm creates a cgroup and attaches a BPF program to the
cgroup for testing NRM for egress traffic. One still needs to create
network traffic. This can be done through netesto, netperf or iperf3.
A follow-up patch contains a script to create traffic.

USAGE: nrm [-d] [-l] [-n <id>] [-r <rate>] [-s] [-t <secs>]
           [-w] [-h] [prog]
  Where:
   -d        Print BPF trace debug buffer
   -l        Also limit flows doing loopback
   -n <#>    To create cgroup "/nrm#" and attach prog. Default is /nrm1
             This is convenient when testing NRM in more than 1 cgroup
   -r <rate> Rate limit in Mbps
   -s        Get NRM stats (marked, dropped, etc.)
   -t <time> Exit after specified seconds (deault is 0)
   -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
             NIC (eth0), but can be extended to support multiple NICs.
             Currrently only supported for egress. Note, this is just
	     a proof of concept.
   -h        Print this info
   prog      BPF program file name. Name defaults to nrm_out_kern.o for
             output, and nrm_in_ker.o for input.

More information about NRM can be found in the paper "BPF Host Resource
Management" presented at the 2018 Linux Plumbers Conference, Networking Track
(http://vger.kernel.org/lpc_net2018_talks/LPC%20BPF%20Network%20Resource%20Paper.pdf)

Signed-off-by: Lawrence Brakmo <brakmo@fb.com>
---
 samples/bpf/Makefile |   3 +
 samples/bpf/nrm.c    | 439 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 442 insertions(+)
 create mode 100644 samples/bpf/nrm.c

diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index 86633af239d4..248a88355a6b 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -54,6 +54,7 @@ hostprogs-y += xdp_fwd
 hostprogs-y += task_fd_query
 hostprogs-y += xdp_sample_pkts
 hostprogs-y += load_cg_skb
+hostprogs-y += nrm
 
 # Libbpf dependencies
 LIBBPF = $(TOOLS_PATH)/lib/bpf/libbpf.a
@@ -111,6 +112,7 @@ xdp_fwd-objs := xdp_fwd_user.o
 task_fd_query-objs := bpf_load.o task_fd_query_user.o $(TRACE_HELPERS)
 xdp_sample_pkts-objs := xdp_sample_pkts_user.o $(TRACE_HELPERS)
 load_cg_skb-objs := bpf_load.o load_cg_skb.o
+nrm-objs := bpf_load.o nrm.o $(CGROUP_HELPERS)
 
 # Tell kbuild to always build the programs
 always := $(hostprogs-y)
@@ -270,6 +272,7 @@ $(src)/*.c: verify_target_bpf $(LIBBPF)
 
 $(obj)/tracex5_kern.o: $(obj)/syscall_nrs.h
 $(obj)/nrm_out_kern.o: $(src)/nrm.h $(src)/nrm_kern.h
+$(obj)/nrm.o: $(src)/nrm.h
 
 # asm/sysreg.h - inline assembly used by it is incompatible with llvm.
 # But, there is no easy way to fix it, so just exclude it since it is
diff --git a/samples/bpf/nrm.c b/samples/bpf/nrm.c
new file mode 100644
index 000000000000..8c30f64e219c
--- /dev/null
+++ b/samples/bpf/nrm.c
@@ -0,0 +1,439 @@
+// 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.
+ *
+ * Example program for Network Resource Managmement
+ *
+ * This program loads a cgroup skb BPF program to enforce cgroup output
+ * (egress) or input (ingress) bandwidth limits.
+ *
+ * USAGE: nrm [-d] [-l] [-n <id>] [-r <rate>] [-s] [-t <secs>] [-w] [-h] [prog]
+ *   Where:
+ *    -d	Print BPF trace debug buffer
+ *    -l	Also limit flows doing loopback
+ *    -n <#>	To create cgroup \"/nrm#\" and attach prog
+ *		Default is /nrm1
+ *    -r <rate>	Rate limit in Mbps
+ *    -s	Get NRM stats (marked, dropped, etc.)
+ *    -t <time>	Exit after specified seconds (deault is 0)
+ *    -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
+ *		NIC (eth0), but can be extended to support multiple NICs.
+ *		Currrently only supported for egress.
+ *    -h	Print this info
+ *    prog	BPF program file name. Name defaults to nrm_out_kern.o
+ */
+
+#define _GNU_SOURCE
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <assert.h>
+#include <sys/resource.h>
+#include <sys/time.h>
+#include <unistd.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <linux/unistd.h>
+
+#include <linux/bpf.h>
+#include <bpf/bpf.h>
+
+#include "bpf_load.h"
+#include "bpf_rlimit.h"
+#include "cgroup_helpers.h"
+#include "nrm.h"
+#include "bpf_util.h"
+#include "bpf/bpf.h"
+#include "bpf/libbpf.h"
+
+bool outFlag = true;
+int minRate = 1000;		/* cgroup rate limit in Mbps */
+int rate = 1000;		/* can grow if rate conserving is enabled */
+int dur = 1;
+bool stats_flag;
+bool loopback_flag;
+bool debugFlag;
+bool work_conserving_flag;
+
+static void Usage(void);
+static void read_trace_pipe2(void);
+static void do_error(char *msg, bool errno_flag);
+
+#define DEBUGFS "/sys/kernel/debug/tracing/"
+
+struct bpf_object *obj;
+int bpfprog_fd;
+int cgroup_storage_fd;
+
+static void read_trace_pipe2(void)
+{
+	int trace_fd;
+	FILE *outf;
+	char *outFname = "nrm_out.log";
+
+	trace_fd = open(DEBUGFS "trace_pipe", O_RDONLY, 0);
+	if (trace_fd < 0) {
+		printf("Error opening trace_pipe\n");
+		return;
+	}
+
+	if (!outFlag)
+		outFname = "nrm_in.log";
+	outf = fopen(outFname, "w");
+
+	if (outf == NULL)
+		printf("Error creating %s\n", outFname);
+
+	while (1) {
+		static char buf[4097];
+		ssize_t sz;
+
+		sz = read(trace_fd, buf, sizeof(buf) - 1);
+		if (sz > 0) {
+			buf[sz] = 0;
+			puts(buf);
+			if (outf != NULL) {
+				fprintf(outf, "%s\n", buf);
+				fflush(outf);
+			}
+		}
+	}
+}
+
+static void do_error(char *msg, bool errno_flag)
+{
+	if (errno_flag)
+		printf("ERROR: %s, errno: %d\n", msg, errno);
+	else
+		printf("ERROR: %s\n", msg);
+	exit(1);
+}
+
+static int prog_load(char *prog)
+{
+	struct bpf_prog_load_attr prog_load_attr = {
+		.prog_type = BPF_PROG_TYPE_CGROUP_SKB,
+		.file = prog,
+	};
+	int map_fd;
+	struct bpf_map *map;
+
+	int ret = 0;
+
+	if (access(prog, O_RDONLY) < 0) {
+		printf("Error accessing file %s: %s\n", prog, strerror(errno));
+		return 1;
+	}
+	if (bpf_prog_load_xattr(&prog_load_attr, &obj, &bpfprog_fd))
+		ret = 1;
+	if (!ret) {
+		map = bpf_object__find_map_by_name(obj, "queue_stats");
+		map_fd = bpf_map__fd(map);
+		if (map_fd < 0) {
+			printf("Map not found: %s\n", strerror(map_fd));
+			ret = 1;
+		}
+	}
+
+	if (ret) {
+		printf("ERROR: load_bpf_file failed for: %s\n", prog);
+		printf("  Output from verifier:\n%s\n------\n", bpf_log_buf);
+		ret = -1;
+	} else {
+		ret = map_fd;
+	}
+
+	return ret;
+}
+
+static int run_bpf_prog(char *prog, int cg_id)
+{
+	int map_fd;
+	int rc = 0;
+	int key = 0;
+	int cg1 = 0;
+	int type = BPF_CGROUP_INET_EGRESS;
+	char cg_dir[100];
+	struct queue_stats qstats = {0};
+
+	sprintf(cg_dir, "/nrm%d", cg_id);
+	map_fd = prog_load(prog);
+	if (map_fd  == -1)
+		return 1;
+
+	if (setup_cgroup_environment()) {
+		printf("ERROR: setting cgroup environment\n");
+		goto err;
+	}
+	cg1 = create_and_get_cgroup(cg_dir);
+	if (!cg1) {
+		printf("ERROR: create_and_get_cgroup\n");
+		goto err;
+	}
+	if (join_cgroup(cg_dir)) {
+		printf("ERROR: join_cgroup\n");
+		goto err;
+	}
+
+	qstats.rate = rate;
+	qstats.stats = stats_flag ? 1 : 0;
+	qstats.loopback = loopback_flag ? 1 : 0;
+	if (bpf_map_update_elem(map_fd, &key, &qstats, BPF_ANY)) {
+		printf("ERROR: Could not update map element\n");
+		goto err;
+	}
+
+	if (!outFlag)
+		type = BPF_CGROUP_INET_INGRESS;
+	if (bpf_prog_attach(bpfprog_fd, cg1, type, 0)) {
+		printf("ERROR: bpf_prog_attach fails!\n");
+		log_err("Attaching prog");
+		goto err;
+	}
+
+	if (work_conserving_flag) {
+		struct timeval t0, t_last, t_new;
+		FILE *fin;
+		unsigned long long last_eth_tx_bytes, new_eth_tx_bytes;
+		signed long long last_cg_tx_bytes, new_cg_tx_bytes;
+		signed long long delta_time, delta_bytes, delta_rate;
+		int delta_ms;
+#define DELTA_RATE_CHECK 10000		/* in us */
+#define RATE_THRESHOLD 9500000000	/* 9.5 Gbps */
+
+		bpf_map_lookup_elem(map_fd, &key, &qstats);
+		if (gettimeofday(&t0, NULL) < 0)
+			do_error("gettimeofday failed", true);
+		t_last = t0;
+		fin = fopen("/sys/class/net/eth0/statistics/tx_bytes", "r");
+		if (fscanf(fin, "%llu", &last_eth_tx_bytes) != 1)
+			do_error("fscanf fails", false);
+		fclose(fin);
+		last_cg_tx_bytes = qstats.bytes_total;
+		while (true) {
+			usleep(DELTA_RATE_CHECK);
+			if (gettimeofday(&t_new, NULL) < 0)
+				do_error("gettimeofday failed", true);
+			delta_ms = (t_new.tv_sec - t0.tv_sec) * 1000 +
+				(t_new.tv_usec - t0.tv_usec)/1000;
+			if (delta_ms > dur * 1000)
+				break;
+			delta_time = (t_new.tv_sec - t_last.tv_sec) * 1000000 +
+				(t_new.tv_usec - t_last.tv_usec);
+			if (delta_time == 0)
+				continue;
+			t_last = t_new;
+			fin = fopen("/sys/class/net/eth0/statistics/tx_bytes",
+				    "r");
+			if (fscanf(fin, "%llu", &new_eth_tx_bytes) != 1)
+				do_error("fscanf fails", false);
+			fclose(fin);
+			printf("  new_eth_tx_bytes:%llu\n",
+			       new_eth_tx_bytes);
+			bpf_map_lookup_elem(map_fd, &key, &qstats);
+			new_cg_tx_bytes = qstats.bytes_total;
+			delta_bytes = new_eth_tx_bytes - last_eth_tx_bytes;
+			last_eth_tx_bytes = new_eth_tx_bytes;
+			delta_rate = (delta_bytes * 8000000) / delta_time;
+			printf("%5d - eth_rate:%.1fGbps cg_rate:%.3fGbps",
+			       delta_ms, delta_rate/1000000000.0,
+			       rate/1000.0);
+			if (delta_rate < RATE_THRESHOLD) {
+				/* can increase cgroup rate limit, but first
+				 * check if we are using the current limit.
+				 * Currently increasing by 6.25%, unknown
+				 * if that is the optimal rate.
+				 */
+				int rate_diff100;
+
+				delta_bytes = new_cg_tx_bytes -
+					last_cg_tx_bytes;
+				last_cg_tx_bytes = new_cg_tx_bytes;
+				delta_rate = (delta_bytes * 8000000) /
+					delta_time;
+				printf(" rate:%.3fGbps",
+				       delta_rate/1000000000.0);
+				rate_diff100 = (((long long)rate)*1000000 -
+						     delta_rate) * 100 /
+					(((long long) rate) * 1000000);
+				printf("  rdiff:%d", rate_diff100);
+				if (rate_diff100  <= 3) {
+					rate += (rate >> 4);
+					if (rate > RATE_THRESHOLD / 1000000)
+						rate = RATE_THRESHOLD / 1000000;
+					qstats.rate = rate;
+					printf(" INC\n");
+				} else {
+					printf("\n");
+				}
+			} else {
+				/* Need to decrease cgroup rate limit.
+				 * Currently decreasing by 12.5%, unknown
+				 * if that is optimal
+				 */
+				printf(" DEC\n");
+				rate -= (rate >> 3);
+				if (rate < minRate)
+					rate = minRate;
+				qstats.rate = rate;
+			}
+			if (bpf_map_update_elem(map_fd, &key, &qstats, BPF_ANY))
+				do_error("update map element fails", false);
+		}
+	} else {
+		sleep(dur);
+	}
+	// Get stats!
+	if (stats_flag && bpf_map_lookup_elem(map_fd, &key, &qstats)) {
+		char fname[100];
+		FILE *fout;
+
+		if (!outFlag)
+			sprintf(fname, "nrm.%d.in", cg_id);
+		else
+			sprintf(fname, "nrm.%d.out", cg_id);
+		fout = fopen(fname, "w");
+		fprintf(fout, "id:%d\n", cg_id);
+		fprintf(fout, "ERROR: Could not lookup queue_stats\n");
+	} else if (stats_flag && qstats.lastPacketTime >
+		   qstats.firstPacketTime) {
+		long long delta_us = (qstats.lastPacketTime -
+				      qstats.firstPacketTime)/1000;
+		unsigned int rate_mbps = ((qstats.bytes_total -
+					   qstats.bytes_dropped) * 8 /
+					  delta_us);
+		double percent_pkts, percent_bytes;
+		char fname[100];
+		FILE *fout;
+
+		if (!outFlag)
+			sprintf(fname, "nrm.%d.in", cg_id);
+		else
+			sprintf(fname, "nrm.%d.out", cg_id);
+		fout = fopen(fname, "w");
+		fprintf(fout, "id:%d\n", cg_id);
+		fprintf(fout, "rate_mbps:%d\n", rate_mbps);
+		fprintf(fout, "duration:%.1f secs\n",
+			(qstats.lastPacketTime - qstats.firstPacketTime) /
+			1000000000.0);
+		fprintf(fout, "packets:%d\n", (int)qstats.pkts_total);
+		fprintf(fout, "bytes_MB:%d\n", (int)(qstats.bytes_total /
+						     1000000));
+		fprintf(fout, "pkts_dropped:%d\n", (int)qstats.pkts_dropped);
+		fprintf(fout, "bytes_dropped_MB:%d\n",
+			(int)(qstats.bytes_dropped /
+						       1000000));
+		// Marked Pkts and Bytes
+		percent_pkts = (qstats.pkts_marked * 100.0) /
+			(qstats.pkts_total + 1);
+		percent_bytes = (qstats.bytes_marked * 100.0) /
+			(qstats.bytes_total + 1);
+		fprintf(fout, "pkts_marked_percent:%6.2f\n", percent_pkts);
+		fprintf(fout, "bytes_marked_percent:%6.2f\n", percent_bytes);
+
+		// Dropped Pkts and Bytes
+		percent_pkts = (qstats.pkts_dropped * 100.0) /
+			(qstats.pkts_total + 1);
+		percent_bytes = (qstats.bytes_dropped * 100.0) /
+			(qstats.bytes_total + 1);
+		fprintf(fout, "pkts_dropped_percent:%6.2f\n", percent_pkts);
+		fprintf(fout, "bytes_dropped_percent:%6.2f\n", percent_bytes);
+		fclose(fout);
+	}
+
+	if (debugFlag)
+		read_trace_pipe2();
+	return rc;
+err:
+	rc = 1;
+
+	if (cg1)
+		close(cg1);
+	cleanup_cgroup_environment();
+
+	return rc;
+}
+
+static void Usage(void)
+{
+	printf("This program loads a cgroup skb BPF program to enforce\n"
+	       "cgroup output (egress) bandwidth limits.\n\n"
+	       "USAGE: nrm [-o] [-d]  [-l] [-n <id>] [-r <rate>] [-s]\n"
+	       "           [-t <secs>] [-w] [-h] [prog]\n"
+	       "  Where:\n"
+	       "    -o         indicates egress direction (default)\n"
+	       "    -d         print BPF trace debug buffer\n"
+	       "    -l         also limit flows using loopback\n"
+	       "    -n <#>     to create cgroup \"/nrm#\" and attach prog\n"
+	       "               Default is /nrm1\n"
+	       "    -r <rate>  Rate in Mbps\n"
+	       "    -s         Update NRM stats\n"
+	       "    -t <time>  Exit after specified seconds (deault is 0)\n"
+	       "    -w	       Work conserving flag. cgroup can increase\n"
+	       "               bandwidth beyond the rate limit specified\n"
+	       "               while there is available bandwidth. Current\n"
+	       "               implementation assumes there is only eth0\n"
+	       "               but can be extended to support multiple NICs\n"
+	       "    -h         print this info\n"
+	       "    prog       BPF program file name. Name defaults to\n"
+	       "                 nrm_out_kern.o for output, and\n"
+	       "                 nrm_in_ker.o for input.\n");
+}
+
+int main(int argc, char **argv)
+{
+	char *prog = "nrm_out_kern.o";
+	int  k;
+	int cg_id = 1;
+	char *optstring = "iodln:r:st:wh";
+
+	while ((k = getopt(argc, argv, optstring)) != -1) {
+		switch (k) {
+		case'o':
+			break;
+		case 'd':
+			debugFlag = true;
+			break;
+		case 'l':
+			loopback_flag = true;
+			break;
+		case 'n':
+			cg_id = atoi(optarg);
+			break;
+		case 'r':
+			minRate = atoi(optarg) * 1.024;
+			rate = minRate;
+			break;
+		case 's':
+			stats_flag = true;
+			break;
+		case 't':
+			dur = atoi(optarg);
+			break;
+		case 'w':
+			work_conserving_flag = true;
+			break;
+		case '?':
+			if (optopt == 'n' || optopt == 'r' || optopt == 't')
+				fprintf(stderr,
+					"Option -%c requires an argument.\n\n",
+					optopt);
+		case 'h':
+			// fallthrough
+		default:
+			Usage();
+			return 0;
+		}
+	}
+
+	if (optind < argc)
+		prog = argv[optind];
+	printf("NRM prog: %s\n", prog != NULL ? prog : "NULL");
+
+	return run_bpf_prog(prog, cg_id);
+}
-- 
2.17.1


^ permalink raw reply related

* [PATCH bpf-next 7/9] bpf: Sample NRM BPF program to limit egress bw
From: brakmo @ 2019-02-19  5:38 UTC (permalink / raw)
  To: netdev; +Cc: Martin Lau, Alexei Starovoitov, Daniel Borkmann --cc=Kernel Team

A cgroup skb BPF program to limit cgroup output bandwidth.
It uses a modified virtual token bucket queue to limit average
egress bandwidth. The implementation uses credits instead of tokens.
Negative credits imply that queueing would have happened (this is
a virtual queue, so no queueing is done by it. However, queueing may
occur at the actual qdisc (which is not used for rate limiting).

This implementation uses 3 thresholds, one to start marking packets and
the other two to drop packets:
                                 CREDIT
       - <--------------------------|------------------------> +
             |    |          |      0
             |  Large pkt    |
             |  drop thresh  |
  Small pkt drop             Mark threshold
      thresh

The effect of marking depends on the type of packet:
a) If the packet is ECN enabled and it is a TCP packet, then the packet
   is ECN marked. The current mark threshold is tuned for DCTCP.
b) If the packet is a TCP packet, then we probabilistically call tcp_cwr
   to reduce the congestion window. The current implementation uses a linear
   distribution (0% probability at marking threshold, 100% probability
   at drop threshold).
c) If the packet is not a TCP packet, then it is dropped.

If the credit is below the drop threshold, the packet is dropped. If it
is a TCP packet, then it also calls tcp_cwr since packets dropped by
by a cgroup skb BPF program do not automatically trigger a call to
tcp_cwr in the current kernel code.

This BPF program actually uses 2 drop thresholds, one threshold
for larger packets (>= 120 bytes) and another for smaller packets. This
protects smaller packets such as SYNs, ACKs, etc.

The default bandwidth limit is set at 1Gbps but this can be changed by
a user program through a shared BPF map. In addition, by default this BPF
program does not limit connections using loopback. This behavior can be
overwritten by the user program. There is also an option to calculate
some statistics, such as percent of packets marked or dropped, which
the user program can access.

A latter patch provides such a program (nrm.c)

Signed-off-by: Lawrence Brakmo <brakmo@fb.com>
---
 samples/bpf/Makefile       |   2 +
 samples/bpf/nrm.h          |  31 ++++++
 samples/bpf/nrm_kern.h     | 109 +++++++++++++++++++
 samples/bpf/nrm_out_kern.c | 213 +++++++++++++++++++++++++++++++++++++
 4 files changed, 355 insertions(+)
 create mode 100644 samples/bpf/nrm.h
 create mode 100644 samples/bpf/nrm_kern.h
 create mode 100644 samples/bpf/nrm_out_kern.c

diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index 0cf3347c7443..86633af239d4 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -169,6 +169,7 @@ always += xdpsock_kern.o
 always += xdp_fwd_kern.o
 always += task_fd_query_kern.o
 always += xdp_sample_pkts_kern.o
+always += nrm_out_kern.o
 
 KBUILD_HOSTCFLAGS += -I$(objtree)/usr/include
 KBUILD_HOSTCFLAGS += -I$(srctree)/tools/lib/
@@ -268,6 +269,7 @@ $(BPF_SAMPLES_PATH)/*.c: verify_target_bpf $(LIBBPF)
 $(src)/*.c: verify_target_bpf $(LIBBPF)
 
 $(obj)/tracex5_kern.o: $(obj)/syscall_nrs.h
+$(obj)/nrm_out_kern.o: $(src)/nrm.h $(src)/nrm_kern.h
 
 # asm/sysreg.h - inline assembly used by it is incompatible with llvm.
 # But, there is no easy way to fix it, so just exclude it since it is
diff --git a/samples/bpf/nrm.h b/samples/bpf/nrm.h
new file mode 100644
index 000000000000..7aa113f8b9df
--- /dev/null
+++ b/samples/bpf/nrm.h
@@ -0,0 +1,31 @@
+/* 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.
+ *
+ * Include file for NRM programs
+ */
+struct bpf_vqueue {
+	struct bpf_spin_lock lock;
+	/* 4 byte hole */
+	unsigned long long lasttime;	/* In ns */
+	int credit;			/* In bytes */
+	unsigned int rate;		/* In bytes per NS << 20 */
+};
+
+struct queue_stats {
+	unsigned long rate;		/* in Mbps*/
+	unsigned long stats:1,		/* get NRM stats (marked, dropped,..) */
+		loopback:1;		/* also limit flows using loopback */
+	unsigned long long pkts_marked;
+	unsigned long long bytes_marked;
+	unsigned long long pkts_dropped;
+	unsigned long long bytes_dropped;
+	unsigned long long pkts_total;
+	unsigned long long bytes_total;
+	unsigned long long firstPacketTime;
+	unsigned long long lastPacketTime;
+};
diff --git a/samples/bpf/nrm_kern.h b/samples/bpf/nrm_kern.h
new file mode 100644
index 000000000000..438a14e2ddba
--- /dev/null
+++ b/samples/bpf/nrm_kern.h
@@ -0,0 +1,109 @@
+/* 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.
+ *
+ * Include file for sample NRM BPF programs
+ */
+#define KBUILD_MODNAME "foo"
+#include <stddef.h>
+#include <stdbool.h>
+#include <uapi/linux/bpf.h>
+#include <uapi/linux/if_ether.h>
+#include <uapi/linux/if_packet.h>
+#include <uapi/linux/ip.h>
+#include <uapi/linux/ipv6.h>
+#include <uapi/linux/in.h>
+#include <uapi/linux/tcp.h>
+#include <uapi/linux/filter.h>
+#include <uapi/linux/pkt_cls.h>
+#include <net/ipv6.h>
+#include "bpf_endian.h"
+#include "bpf_helpers.h"
+#include "nrm.h"
+
+#define DROP_PKT	0
+#define ALLOW_PKT	1
+#define TCP_ECN_OK	1
+
+#define NRM_DEBUG 0  // Set to 1 to enable debugging
+#if NRM_DEBUG
+#define bpf_printk(fmt, ...)					\
+({								\
+	char ____fmt[] = fmt;					\
+	bpf_trace_printk(____fmt, sizeof(____fmt),		\
+			 ##__VA_ARGS__);			\
+})
+#else
+#define bpf_printk(fmt, ...)
+#endif
+
+#define INITIAL_CREDIT_PACKETS	100
+#define MAX_BYTES_PER_PACKET	1500
+#define MARK_THRESH		(80 * MAX_BYTES_PER_PACKET)
+#define DROP_THRESH		(80 * 5 * MAX_BYTES_PER_PACKET)
+#define LARGE_PKT_DROP_THRESH	(DROP_THRESH - (15 * MAX_BYTES_PER_PACKET))
+#define MARK_REGION_SIZE	(LARGE_PKT_DROP_THRESH - MARK_THRESH)
+#define LARGE_PKT_THRESH	120
+#define MAX_CREDIT		(100 * MAX_BYTES_PER_PACKET)
+#define INIT_CREDIT		(INITIAL_CREDIT_PACKETS * MAX_BYTES_PER_PACKET)
+
+// rate in bytes per ns << 20
+#define CREDIT_PER_NS(delta, rate) ((((u64)(delta)) * (rate)) >> 20)
+
+struct bpf_map_def SEC("maps") queue_state = {
+	.type = BPF_MAP_TYPE_CGROUP_STORAGE,
+	.key_size = sizeof(struct bpf_cgroup_storage_key),
+	.value_size = sizeof(struct bpf_vqueue),
+};
+BPF_ANNOTATE_KV_PAIR(queue_state, struct bpf_cgroup_storage_key,
+		     struct bpf_vqueue);
+
+struct bpf_map_def SEC("maps") queue_stats = {
+	.type = BPF_MAP_TYPE_ARRAY,
+	.key_size = sizeof(u32),
+	.value_size = sizeof(struct queue_stats),
+	.max_entries = 1,
+};
+BPF_ANNOTATE_KV_PAIR(queue_stats, int, struct queue_stats);
+
+struct nrm_pkt_info {
+	bool	is_ip;
+	bool	is_tcp;
+	short	ecn;
+};
+
+static __always_inline void get_nrm_pkt_info(struct bpf_sock *sk,
+					     struct nrm_pkt_info *pkti)
+{
+	if (sk->family == AF_INET6 || sk->family == AF_INET) {
+		pkti->is_ip = true;
+		pkti->is_tcp = (sk->protocol == IPPROTO_TCP);
+		if (pkti->is_tcp) {
+			struct bpf_tcp_sock *tp;
+
+			tp = bpf_tcp_sock(sk);
+			if (tp)
+				pkti->ecn = tp->ecn_flags & TCP_ECN_OK;
+			else
+				pkti->ecn = 0;
+		} else {
+			pkti->ecn = 0;
+		}
+	} else {
+		pkti->is_ip = false;
+		pkti->is_tcp = false;
+		pkti->ecn = 0;
+	}
+}
+
+static __always_inline void init_bpf_vqueue(struct bpf_vqueue *qdp, int rate)
+{
+		bpf_printk("Initializing queue_state, rate:%d\n", rate * 128);
+		qdp->lasttime = bpf_ktime_get_ns();
+		qdp->credit = INIT_CREDIT;
+		qdp->rate = rate * 128;
+}
diff --git a/samples/bpf/nrm_out_kern.c b/samples/bpf/nrm_out_kern.c
new file mode 100644
index 000000000000..591a30a18a22
--- /dev/null
+++ b/samples/bpf/nrm_out_kern.c
@@ -0,0 +1,213 @@
+// 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.
+ *
+ * Sample Network Resource Manager (NRM) BPF program.
+ *
+ * A cgroup skb BPF egress program to limit cgroup output bandwidth.
+ * It uses a modified virtual token bucket queue to limit average
+ * egress bandwidth. The implementation uses credits instead of tokens.
+ * Negative credits imply that queueing would have happened (this is
+ * a virtual queue, so no queueing is done by it. However, queueing may
+ * occur at the actual qdisc (which is not used for rate limiting).
+ *
+ * This implementation uses 3 thresholds, one to start marking packets and
+ * the other two to drop packets:
+ *                                  CREDIT
+ *        - <--------------------------|------------------------> +
+ *              |    |          |      0
+ *              |  Large pkt    |
+ *              |  drop thresh  |
+ *   Small pkt drop             Mark threshold
+ *       thresh
+ *
+ * The effect of marking depends on the type of packet:
+ * a) If the packet is ECN enabled and it is a TCP packet, then the packet
+ *    is ECN marked.
+ * b) If the packet is a TCP packet, then we probabilistically call tcp_cwr
+ *    to reduce the congestion window. The current implementation uses a linear
+ *    distribution (0% probability at marking threshold, 100% probability
+ *    at drop threshold).
+ * c) If the packet is not a TCP packet, then it is dropped.
+ *
+ * If the credit is below the drop threshold, the packet is dropped. If it
+ * is a TCP packet, then it also calls tcp_cwr since packets dropped by
+ * by a cgroup skb BPF program do not automatically trigger a call to
+ * tcp_cwr in the current kernel code.
+ *
+ * This BPF program actually uses 2 drop thresholds, one threshold
+ * for larger packets (>= 120 bytes) and another for smaller packets. This
+ * protects smaller packets such as SYNs, ACKs, etc.
+ *
+ * The default bandwidth limit is set at 1Gbps but this can be changed by
+ * a user program through a shared BPF map. In addition, by default this BPF
+ * program does not limit connections using loopback. This behavior can be
+ * overwritten by the user program. There is also an option to calculate
+ * some statistics, such as percent of packets marked or dropped, which
+ * the user program can access.
+ *
+ * A latter patch provides such a program (nrm.c)
+ */
+
+#include "nrm_kern.h"
+
+SEC("cgroup/skb")
+int _nrm_out_cg(struct __sk_buff *skb)
+{
+	struct bpf_sock *sk;
+	struct nrm_pkt_info pkti;
+	int len = skb->len;
+	unsigned int queue_index = 0;
+	unsigned long long curtime;
+	int credit;
+	signed long long delta = 0, zero = 0;
+	int max_credit = MAX_CREDIT;
+	bool mark_flag = false;
+	bool drop_flag = false;
+	bool cwr_flag = false;
+	struct bpf_vqueue *qdp;
+	struct queue_stats *qsp = NULL;
+	int rv = ALLOW_PKT;
+	unsigned int payload;
+	unsigned int pkts;
+
+	qsp = bpf_map_lookup_elem(&queue_stats, &queue_index);
+	if (qsp != NULL && !qsp->loopback && (skb->ifindex == 1))
+		return ALLOW_PKT;
+
+	sk = skb->sk;
+	if (!sk)
+		return ALLOW_PKT;
+	sk = bpf_sk_fullsock(sk);
+	if (!sk)
+		return ALLOW_PKT;
+
+	get_nrm_pkt_info(sk, &pkti);
+
+	// We may want to account for the length of headers in len
+	// calculation, like ETH header + overhead, specially if it
+	// is a gso packet. But I am not doing it right now.
+
+	qdp = bpf_get_local_storage(&queue_state, 0);
+	if (!qdp)
+		return ALLOW_PKT;
+	else if (qdp->lasttime == 0)
+		init_bpf_vqueue(qdp, 1024);
+
+	curtime = bpf_ktime_get_ns();
+
+	// Begin critical section
+	bpf_spin_lock(&qdp->lock);
+	credit = qdp->credit;
+	delta = curtime - qdp->lasttime;
+	/* delta < 0 implies that another process with a curtime greater
+	 * than ours beat us to the critical section and already added
+	 * the new credit, so we should not add it ourselves
+	 */
+	if (delta > 0) {
+		qdp->lasttime = curtime;
+		credit += CREDIT_PER_NS(delta, qdp->rate);
+		if (credit > MAX_CREDIT)
+			credit = MAX_CREDIT;
+	}
+	credit -= len;
+	qdp->credit = credit;
+	bpf_spin_unlock(&qdp->lock);
+	// End critical section
+
+	// Check if we should update rate
+	if (qsp != NULL && (qsp->rate * 128) != qdp->rate) {
+		qdp->rate = qsp->rate * 128;
+		bpf_printk("Updating rate: %d (1sec:%llu bits)\n",
+			   (int)qdp->rate,
+			   CREDIT_PER_NS(1000000000, qdp->rate) * 8);
+	}
+
+	// Set flags (drop, mark, cwr)
+	if (pkti.is_ip) {
+		if (credit < -DROP_THRESH ||
+		    (len > LARGE_PKT_THRESH &&
+		     credit < -LARGE_PKT_DROP_THRESH)) {
+			drop_flag = true;
+			mark_flag = false;
+			if (pkti.is_tcp) {
+				if (pkti.ecn == 0)
+					cwr_flag = true;
+			}
+		} else if (credit < 0) {
+			if (pkti.is_tcp) {
+				if (credit < -MARK_THRESH)
+					mark_flag = true;
+				else
+					mark_flag = false;
+			} else {
+				mark_flag = true;
+			}
+		} else {
+			mark_flag = false;
+		}
+
+		if (mark_flag) {
+			if (pkti.is_tcp && pkti.ecn > 0) {
+				bpf_skb_set_ecn(skb, 3);
+			} else if (pkti.is_tcp) {
+				unsigned int rand = bpf_get_prandom_u32();
+
+				if (-credit >= MARK_THRESH +
+				    (rand % MARK_REGION_SIZE)) {
+					// Do cong avoidance
+					cwr_flag = true;
+				}
+			} else if (len > LARGE_PKT_THRESH) {
+				// Problem if too many small packets?
+				drop_flag = true;
+				mark_flag = false;
+			}
+		}
+
+		if (pkti.is_tcp) {
+			struct bpf_tcp_sock *tp;
+
+			tp = bpf_tcp_sock(sk);
+			if (tp && drop_flag)
+				bpf_tcp_check_probe_timer(tp, 20000);
+			if (tp && cwr_flag)
+				bpf_tcp_enter_cwr(tp);
+		}
+		if (drop_flag)
+			rv = DROP_PKT;
+	} else if (credit < -MARK_THRESH) {
+		drop_flag = true;
+		rv =  DROP_PKT;
+	}
+
+	if (qsp != NULL) {
+		// Following is needed for work conserving
+		__sync_add_and_fetch(&(qsp->bytes_total), len);
+		if (qsp->stats) {
+			// Optionally update statistics
+			if (qsp->firstPacketTime == 0)
+				qsp->firstPacketTime = curtime;
+			qsp->lastPacketTime = curtime;
+			__sync_add_and_fetch(&(qsp->pkts_total), 1);
+			if (mark_flag) {
+				__sync_add_and_fetch(&(qsp->pkts_marked), 1);
+				__sync_add_and_fetch(&(qsp->bytes_marked), len);
+			}
+			if (drop_flag) {
+				__sync_add_and_fetch(&(qsp->pkts_dropped), 1);
+				__sync_add_and_fetch(&(qsp->bytes_dropped),
+						     len);
+			}
+		}
+	}
+
+	if (rv == DROP_PKT)
+		__sync_add_and_fetch(&(qdp->credit), len);
+
+	return rv;
+}
+char _license[] SEC("license") = "GPL";
-- 
2.17.1


^ permalink raw reply related

* [PATCH bpf-next 0/9] bpf: Network Resource Manager (NRM)
From: brakmo @ 2019-02-19  5:38 UTC (permalink / raw)
  To: netdev; +Cc: Martin Lau, Alexei Starovoitov, Daniel Borkmann --cc=Kernel Team

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.

The sample NRM BPF program is not meant to be production quality, it is
provided as proof of concept. A lot more information, including sample
runs in some cases, are provided in the commit messages of the individual
patches.

Two more BPF programs, one to limit ingress and one that limits egress
and uses fq's Earliest Departure Time feature, will be provided in an
upcomming patchset.

brakmo (9):
  bpf: Add bpf helper bpf_tcp_enter_cwr
  bpf: Test bpf_tcp_enter_cwr in test_verifier
  bpf: add bpf helper bpf_skb_set_ecn
  bpf: Add bpf helper bpf_tcp_check_probe_timer
  bpf: sync bpf.h to tools and update bpf_helpers.h
  bpf: Sample program to load cg skb BPF programs
  bpf: Sample NRM BPF program to limit egress bw
  bpf: User program for testing NRM
  bpf: NRM test script

 include/linux/bpf.h                         |   1 +
 include/uapi/linux/bpf.h                    |  27 +-
 kernel/bpf/verifier.c                       |   4 +
 net/core/filter.c                           |  70 ++++
 samples/bpf/Makefile                        |   7 +
 samples/bpf/do_nrm_test.sh                  | 429 +++++++++++++++++++
 samples/bpf/load_cg_skb.c                   | 109 +++++
 samples/bpf/nrm.c                           | 439 ++++++++++++++++++++
 samples/bpf/nrm.h                           |  31 ++
 samples/bpf/nrm_kern.h                      | 109 +++++
 samples/bpf/nrm_out_kern.c                  | 213 ++++++++++
 tools/include/uapi/linux/bpf.h              |  27 +-
 tools/testing/selftests/bpf/bpf_helpers.h   |   6 +
 tools/testing/selftests/bpf/verifier/sock.c |  33 ++
 14 files changed, 1503 insertions(+), 2 deletions(-)
 create mode 100755 samples/bpf/do_nrm_test.sh
 create mode 100644 samples/bpf/load_cg_skb.c
 create mode 100644 samples/bpf/nrm.c
 create mode 100644 samples/bpf/nrm.h
 create mode 100644 samples/bpf/nrm_kern.h
 create mode 100644 samples/bpf/nrm_out_kern.c

-- 
2.17.1


^ permalink raw reply

* [PATCH bpf-next 3/9] bpf: add bpf helper bpf_skb_set_ecn
From: brakmo @ 2019-02-19  5:38 UTC (permalink / raw)
  To: netdev; +Cc: Martin Lau, Alexei Starovoitov, Daniel Borkmann --cc=Kernel Team

This patch adds a new bpf helper BPF_FUNC_skb_set_ecn
"int bpf_skb_set_Ecn(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. This type of
bpf_prog cannot modify the skb directly.

This helper is used to set the ECN bits (2) of the IPv6 or IPv4
header in skb. It can be used by a bpf_prog to manage egress
network bandwdith limit per cgroupv2 by inducing an ECN
response in the TCP sender (when the packet is ECN enabled).
This works best when using DCTCP.

Signed-off-by: Lawrence Brakmo <brakmo@fb.com>
---
 include/uapi/linux/bpf.h | 10 +++++++++-
 net/core/filter.c        | 29 +++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 9e9f4f1a0370..5daf404511f7 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
+ *
+ * int bpf_skb_set_ecn(struct sk_buf *skb, int val)
+ *	Description
+ *		Sets ECN bits (2) of IP header. Works with IPv6 and IPv4.
+ *		val should be one of 0, 1, 2, 3.
+ *	Return
+ *		-EINVAL on error (e.g. val > 3), 0 otherwise.
  */
 #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_set_ecn),
 
 /* 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 f51c4a781844..275acfb2117d 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -5438,6 +5438,33 @@ static const struct bpf_func_proto bpf_tcp_enter_cwr_proto = {
 	.ret_type    = RET_INTEGER,
 	.arg1_type    = ARG_PTR_TO_TCP_SOCK,
 };
+
+BPF_CALL_2(bpf_skb_set_ecn, struct sk_buff *, skb, u32, val)
+{
+	struct ipv6hdr *ip6h = ipv6_hdr(skb);
+
+	if ((val & ~0x3) != 0)
+		return -EINVAL;
+
+	if (ip6h->version == 6) {
+		ip6h->flow_lbl[0] = (ip6h->flow_lbl[0] & ~0x30) | (val << 4);
+		return 0;
+	} else if (ip6h->version == 4) {
+		struct iphdr *ip4h = (struct iphdr *)ip6h;
+
+		ip4h->tos = (ip4h->tos & ~0x3) | val;
+		return 0;
+	}
+	return -EINVAL;
+}
+
+static const struct bpf_func_proto bpf_skb_set_ecn_proto = {
+	.func		= bpf_skb_set_ecn,
+	.gpl_only	= false,
+	.ret_type	= RET_INTEGER,
+	.arg1_type	= ARG_PTR_TO_CTX,
+	.arg2_type	= ARG_ANYTHING,
+};
 #endif /* CONFIG_INET */
 
 bool bpf_helper_changes_pkt_data(void *func)
@@ -5599,6 +5626,8 @@ cg_skb_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 		return &bpf_tcp_sock_proto;
 	case BPF_FUNC_tcp_enter_cwr:
 		return &bpf_tcp_enter_cwr_proto;
+	case BPF_FUNC_skb_set_ecn:
+		return &bpf_skb_set_ecn_proto;
 #endif
 	default:
 		return sk_filter_func_proto(func_id, prog);
-- 
2.17.1


^ permalink raw reply related

* [PATCH bpf-next 1/9] bpf: Add bpf helper bpf_tcp_enter_cwr
From: brakmo @ 2019-02-19  5:38 UTC (permalink / raw)
  To: netdev; +Cc: Martin Lau, Alexei Starovoitov, Daniel Borkmann --cc=Kernel Team

This patch adds a new bpf helper BPF_FUNC_tcp_enter_cwr
"int bpf_tcp_enter_cwr(struct bpf_tcp_sock *tp)".
It is added to BPF_PROG_TYPE_CGROUP_SKB typed bpf_prog
which currently can be attached to the ingress and egress
path.

This helper makes a tcp_sock enter CWR state.  It can be used
by a bpf_prog to manage egress network bandwidth limit per
cgroupv2.  A later patch will have a sample program to
show how it can be used to limit bandwidth usage per cgroupv2.

Signed-off-by: Lawrence Brakmo <brakmo@fb.com>
Signed-off-by: Martin KaFai Lau <kafai@fb.com>
---
 include/linux/bpf.h      |  1 +
 include/uapi/linux/bpf.h |  9 ++++++++-
 kernel/bpf/verifier.c    |  4 ++++
 net/core/filter.c        | 14 ++++++++++++++
 4 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index de18227b3d95..525628c913c9 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -195,6 +195,7 @@ enum bpf_arg_type {
 	ARG_PTR_TO_SOCKET,	/* pointer to bpf_sock */
 	ARG_PTR_TO_SPIN_LOCK,	/* pointer to bpf_spin_lock */
 	ARG_PTR_TO_SOCK_COMMON,	/* pointer to sock_common */
+	ARG_PTR_TO_TCP_SOCK,    /* pointer to tcp_sock */
 };
 
 /* type of values returned from helper functions */
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index bcdd2474eee7..9e9f4f1a0370 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -2359,6 +2359,12 @@ union bpf_attr {
  *	Return
  *		A **struct bpf_tcp_sock** pointer on success, or NULL in
  *		case of failure.
+ *
+ * int bpf_tcp_enter_cwr(struct bpf_tcp_sock *tp)
+ *    Description
+ *        Make a tcp_sock enter CWR state.
+ *    Return
+ *        0
  */
 #define __BPF_FUNC_MAPPER(FN)		\
 	FN(unspec),			\
@@ -2457,7 +2463,8 @@ union bpf_attr {
 	FN(spin_lock),			\
 	FN(spin_unlock),		\
 	FN(sk_fullsock),		\
-	FN(tcp_sock),
+	FN(tcp_sock),			\
+	FN(tcp_enter_cwr),
 
 /* integer value in 'imm' field of BPF_CALL instruction selects which helper
  * function eBPF program intends to call
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 1b9496c41383..95fb385c6f3c 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -2424,6 +2424,10 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 regno,
 			return -EFAULT;
 		}
 		meta->ptr_id = reg->id;
+	} else if (arg_type == ARG_PTR_TO_TCP_SOCK) {
+		expected_type = PTR_TO_TCP_SOCK;
+		if (type != expected_type)
+			goto err_type;
 	} else if (arg_type == ARG_PTR_TO_SPIN_LOCK) {
 		if (meta->func_id == BPF_FUNC_spin_lock) {
 			if (process_spin_lock(env, regno, true))
diff --git a/net/core/filter.c b/net/core/filter.c
index b584cb42a803..f51c4a781844 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -5426,6 +5426,18 @@ static const struct bpf_func_proto bpf_tcp_sock_proto = {
 	.arg1_type	= ARG_PTR_TO_SOCK_COMMON,
 };
 
+BPF_CALL_1(bpf_tcp_enter_cwr, struct tcp_sock *, tp)
+{
+	tcp_enter_cwr((struct sock *)tp);
+	return 0;
+}
+
+static const struct bpf_func_proto bpf_tcp_enter_cwr_proto = {
+	.func        = bpf_tcp_enter_cwr,
+	.gpl_only    = false,
+	.ret_type    = RET_INTEGER,
+	.arg1_type    = ARG_PTR_TO_TCP_SOCK,
+};
 #endif /* CONFIG_INET */
 
 bool bpf_helper_changes_pkt_data(void *func)
@@ -5585,6 +5597,8 @@ cg_skb_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 #ifdef CONFIG_INET
 	case BPF_FUNC_tcp_sock:
 		return &bpf_tcp_sock_proto;
+	case BPF_FUNC_tcp_enter_cwr:
+		return &bpf_tcp_enter_cwr_proto;
 #endif
 	default:
 		return sk_filter_func_proto(func_id, prog);
-- 
2.17.1


^ permalink raw reply related

* [PATCH bpf-next 2/9] bpf: Test bpf_tcp_enter_cwr in test_verifier
From: brakmo @ 2019-02-19  5:38 UTC (permalink / raw)
  To: netdev; +Cc: Martin Lau, Alexei Starovoitov, Daniel Borkmann --cc=Kernel Team

This test ensures the verifier has checked the arg1 of
BPF_FUNC_tcp_enter_cwr is of ARG_PTR_TO_TCP_SOCK type.

Signed-off-by: Martin KaFai Lau <kafai@fb.com>
Signed-off-by: Lawrence Brakmo <brakmo@fb.com>
---
 tools/testing/selftests/bpf/verifier/sock.c | 33 +++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/tools/testing/selftests/bpf/verifier/sock.c b/tools/testing/selftests/bpf/verifier/sock.c
index 0ddfdf76aba5..b07a083eeb59 100644
--- a/tools/testing/selftests/bpf/verifier/sock.c
+++ b/tools/testing/selftests/bpf/verifier/sock.c
@@ -382,3 +382,36 @@
 	.result = REJECT,
 	.errstr = "type=tcp_sock expected=sock",
 },
+{
+	"bpf_tcp_enter_cwr(skb->sk)",
+	.insns = {
+	BPF_LDX_MEM(BPF_DW, BPF_REG_1, BPF_REG_1, offsetof(struct __sk_buff, sk)),
+	BPF_JMP_IMM(BPF_JNE, BPF_REG_1, 0, 2),
+	BPF_MOV64_IMM(BPF_REG_0, 0),
+	BPF_EXIT_INSN(),
+	BPF_EMIT_CALL(BPF_FUNC_tcp_enter_cwr),
+	BPF_MOV64_IMM(BPF_REG_0, 0),
+	BPF_EXIT_INSN(),
+	},
+	.prog_type = BPF_PROG_TYPE_CGROUP_SKB,
+	.result = REJECT,
+	.errstr = "type=sock_common expected=tcp_sock",
+},
+{
+	"bpf_tcp_enter_cwr(bpf_tcp_sock(skb->sk))",
+	.insns = {
+	BPF_LDX_MEM(BPF_DW, BPF_REG_1, BPF_REG_1, offsetof(struct __sk_buff, sk)),
+	BPF_JMP_IMM(BPF_JNE, BPF_REG_1, 0, 2),
+	BPF_MOV64_IMM(BPF_REG_0, 0),
+	BPF_EXIT_INSN(),
+	BPF_EMIT_CALL(BPF_FUNC_tcp_sock),
+	BPF_JMP_IMM(BPF_JNE, BPF_REG_0, 0, 1),
+	BPF_EXIT_INSN(),
+	BPF_MOV64_REG(BPF_REG_1, BPF_REG_0),
+	BPF_EMIT_CALL(BPF_FUNC_tcp_enter_cwr),
+	BPF_MOV64_IMM(BPF_REG_0, 0),
+	BPF_EXIT_INSN(),
+	},
+	.prog_type = BPF_PROG_TYPE_CGROUP_SKB,
+	.result = ACCEPT,
+},
-- 
2.17.1


^ permalink raw reply related

* Re: [PATCH v2 1/2] net: phy: at803x: dont inline helpers
From: Vinod Koul @ 2019-02-19  5:39 UTC (permalink / raw)
  To: Sekhar Nori
  Cc: David Miller, linux-arm-msm, bjorn.andersson, netdev,
	niklas.cassel, andrew, f.fainelli, peter.ujfalusi,
	marc.w.gonzalez
In-Reply-To: <26c3046d-8b1c-e601-7cad-c9a7bc267033@ti.com>

On 19-02-19, 11:05, Sekhar Nori wrote:
> Hi Vinod,
> 
> On 19/02/19 10:32 AM, Vinod Koul wrote:
> > Hello Dave,
> > 
> > On 18-02-19, 16:28, David Miller wrote:
> >> From: Vinod Koul <vkoul@kernel.org>
> >> Date: Mon, 18 Feb 2019 15:48:52 +0530
> >>
> >>> Some helpers were inlined, but makes more sense to allow compiler
> >>> to do the right optiomazations instead, so remove inline for
> >>> at803x_disable_rx_delay() and at803x_disable_tx_delay()
> >>>
> >>> Signed-off-by: Vinod Koul <vkoul@kernel.org>
> >>
> >> Two problem with this patch series.
> >>
> >> 1) It fixes a bug in 'net' but the patches only apply cleanly to
> >>    'net-next'.  Please respin this against 'net'.
> > 
> > I assume the 'net' is fixes going into current -rcX. This fixes issues
> > reported on linux-next due to patches in net-next, so these fixes should
> > go to net-next as well.
> 
> net-next should be part of subject prefix in this case. See
> Documentation/networking/netdev-FAQ.rst

Thanks for the pointer Sekhar, will do :)

-- 
~Vinod

^ permalink raw reply

* Re: [PATCH v3 perf,bpf 08/11] perf, bpf: save btf information as headers to perf.data
From: Song Liu @ 2019-02-19  5:48 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Netdev, linux-kernel, ast@kernel.org, daniel@iogearbox.net,
	Kernel Team, peterz@infradead.org, acme@redhat.com,
	jolsa@kernel.org, namhyung@kernel.org
In-Reply-To: <20190217230637.GK7443@krava>



> On Feb 17, 2019, at 3:06 PM, Jiri Olsa <jolsa@redhat.com> wrote:
> 
> On Fri, Feb 15, 2019 at 01:53:51PM -0800, Song Liu wrote:
> 
> SNIP
> 
>> +static int process_btf(struct feat_fd *ff, void *data __maybe_unused)
>> +{
>> +	struct perf_env *env = &ff->ph->env;
>> +	u32 count, i;
>> +
>> +	if (do_read_u32(ff, &count))
>> +		return -1;
>> +
>> +	down_write(&env->bpf_progs.bpf_info_lock);
>> +
>> +	for (i = 0; i < count; ++i) {
>> +		struct btf_node btf_node;
>> +		struct btf_node *node;
>> +
>> +		if (__do_read(ff, &btf_node, sizeof(struct btf_node)))
>> +			return -1;
>> +
>> +		node = malloc(sizeof(struct btf_node) + btf_node.data_size);
>> +		if (!node)
>> +			return -1;
>> +
>> +		node->id = btf_node.id;
>> +		node->data_size = btf_node.data_size;
>> +
>> +		if (__do_read(ff, node->data, btf_node.data_size)) {
>> +			free(node);
>> +			return -1;
>> +		}
> 
> hows endianity swap handled got struct btf_node and data in here?
> 
> thanks,
> jirka

I thought about endianity at some point, but forgot about it. Let 
me see how to fix it. 

Thanks,
Song 



^ permalink raw reply

* Re: [PATCH v3 perf,bpf 05/11] perf, bpf: save bpf_prog_info in a rbtree in perf_env
From: Song Liu @ 2019-02-19  5:52 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Netdev, linux-kernel, ast@kernel.org, daniel@iogearbox.net,
	Kernel Team, peterz@infradead.org, acme@redhat.com,
	jolsa@kernel.org, namhyung@kernel.org
In-Reply-To: <20190217230516.GA7443@krava>



> On Feb 17, 2019, at 3:05 PM, Jiri Olsa <jolsa@redhat.com> wrote:
> 
> On Fri, Feb 15, 2019 at 01:53:48PM -0800, Song Liu wrote:
> 
> SNIP
> 
>> 	info_linear = bpf_program__get_prog_info_linear(fd, arrays);
>> 	if (IS_ERR_OR_NULL(info_linear)) {
>> @@ -151,8 +165,8 @@ static int perf_event__synthesize_one_bpf_prog(struct perf_tool *tool,
>> 						     machine, process);
>> 	}
>> 
>> -	/* Synthesize PERF_RECORD_BPF_EVENT */
>> 	if (opts->bpf_event) {
>> +		/* Synthesize PERF_RECORD_BPF_EVENT */
>> 		*bpf_event = (struct bpf_event){
>> 			.header = {
>> 				.type = PERF_RECORD_BPF_EVENT,
>> @@ -165,6 +179,19 @@ static int perf_event__synthesize_one_bpf_prog(struct perf_tool *tool,
>> 		memcpy(bpf_event->tag, info->tag, BPF_TAG_SIZE);
>> 		memset((void *)event + event->header.size, 0, machine->id_hdr_size);
>> 		event->header.size += machine->id_hdr_size;
>> +
>> +		/* save bpf_prog_info to env */
>> +		info_node = malloc(sizeof(struct bpf_prog_info_node));
>> +		if (info_node) {
>> +			info_node->info_linear = info_linear;
>> +			perf_env__insert_bpf_prog_info(env, info_node);
>> +			info_linear = NULL;
>> +		}
> 
> what if the allocation fails? we don't care?
> 
> jirka

My original plan is to just ignore it and accept that this program
doesn't have annotation. Any suggestion on what would be a better 
approach?

Thanks,
Song


^ permalink raw reply

* Re: [PATCH ipsec] xfrm: Fix inbound traffic via XFRM interfaces across network namespaces
From: Steffen Klassert @ 2019-02-19  6:04 UTC (permalink / raw)
  To: Tobias Brunner; +Cc: netdev
In-Reply-To: <862cfc1b-34ad-1755-63c5-af87cc12ff63@strongswan.org>

On Mon, Feb 18, 2019 at 10:49:39AM +0100, Tobias Brunner wrote:
> After moving an XFRM interface to another namespace it stays associated
> with the original namespace (net in `struct xfrm_if` and the list keyed
> with `xfrmi_net_id`), allowing processes in the new namespace to use
> SAs/policies that were created in the original namespace.  For instance,
> this allows a keying daemon in one namespace to establish IPsec SAs for
> other namespaces without processes there having access to the keys or IKE
> credentials.
> 
> This worked fine for outbound traffic, however, for inbound traffic the
> lookup for the interfaces and the policies used the incorrect namespace
> (the one the XFRM interface was moved to).
> 
> Fixes: f203b76d7809 ("xfrm: Add virtual xfrm interfaces")
> Signed-off-by: Tobias Brunner <tobias@strongswan.org>

Applied, thanks a lot Tobias!

^ permalink raw reply

* [PATCH net-next v3 0/2] net: phy: at803x: Update delays for RGMII modes
From: Vinod Koul @ 2019-02-19  6:17 UTC (permalink / raw)
  To: David S Miller
  Cc: linux-arm-msm, Bjorn Andersson, Vinod Koul, netdev, Niklas Cassel,
	Andrew Lunn, Florian Fainelli, Nori, Sekhar, Peter Ujfalusi,
	Marc Gonzalez

Peter[1] reported that patch cd28d1d6e52e: ("net: phy: at803x: Disable
phy delay for RGMII mode") caused regression on am335x-evmsk board.
This board expects the Phy delay to be enabled but specified RGMII mode
which refers to delays being disabled. So fix this by disabling delay only
for RGMII mode and enable for RGMII_ID and RGMII_TXID/RXID modes.

While at it, as pointed by Dave, don't inline the helpers.

[1]: https://www.spinics.net/lists/netdev/msg550749.html

Vinod Koul (2):
  net: phy: at803x: don't inline helpers
  net: phy: at803x: disable delay only for RGMII mode

 drivers/net/phy/at803x.c | 51 ++++++++++++++++++++++++++++++----------
 1 file changed, 38 insertions(+), 13 deletions(-)

-- 
2.20.1


^ permalink raw reply

* [PATCH net-next v3 1/2] net: phy: at803x: don't inline helpers
From: Vinod Koul @ 2019-02-19  6:17 UTC (permalink / raw)
  To: David S Miller
  Cc: linux-arm-msm, Bjorn Andersson, Vinod Koul, netdev, Niklas Cassel,
	Andrew Lunn, Florian Fainelli, Nori, Sekhar, Peter Ujfalusi,
	Marc Gonzalez
In-Reply-To: <20190219061800.31025-1-vkoul@kernel.org>

Some helpers were inlined, but makes more sense to allow compiler
to do the right optimizations instead, so remove inline for
at803x_disable_rx_delay() and at803x_disable_tx_delay()

Reviewed-by: Niklas Cassel <niklas.cassel@linaro.org>
Signed-off-by: Vinod Koul <vkoul@kernel.org>
---
 drivers/net/phy/at803x.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index 8ff12938ab47..c6e7d800fd7a 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -110,13 +110,13 @@ static int at803x_debug_reg_mask(struct phy_device *phydev, u16 reg,
 	return phy_write(phydev, AT803X_DEBUG_DATA, val);
 }
 
-static inline int at803x_disable_rx_delay(struct phy_device *phydev)
+static int at803x_disable_rx_delay(struct phy_device *phydev)
 {
 	return at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_0,
 				     AT803X_DEBUG_RX_CLK_DLY_EN, 0);
 }
 
-static inline int at803x_disable_tx_delay(struct phy_device *phydev)
+static int at803x_disable_tx_delay(struct phy_device *phydev)
 {
 	return at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_5,
 				     AT803X_DEBUG_TX_CLK_DLY_EN, 0);
-- 
2.20.1


^ permalink raw reply related

* [PATCH net-next v3 2/2] net: phy: at803x: disable delay only for RGMII mode
From: Vinod Koul @ 2019-02-19  6:18 UTC (permalink / raw)
  To: David S Miller
  Cc: linux-arm-msm, Bjorn Andersson, Vinod Koul, netdev, Niklas Cassel,
	Andrew Lunn, Florian Fainelli, Nori, Sekhar, Peter Ujfalusi,
	Marc Gonzalez, Peter Ujfalusi
In-Reply-To: <20190219061800.31025-1-vkoul@kernel.org>

Per "Documentation/devicetree/bindings/net/ethernet.txt" RGMII mode
should not have delay in PHY whereas RGMII_ID and RGMII_RXID/RGMII_TXID
can have delay in phy.

So disable the delay only for RGMII mode and enable for other modes.
Also treat the default case as disabled delays.

Fixes: cd28d1d6e52e: ("net: phy: at803x: Disable phy delay for RGMII mode")
Reported-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
Reviewed-by: Niklas Cassel <niklas.cassel@linaro.org>
Tested-by: Peter Ujfalusi <peter.ujflausi@ti.com>
Signed-off-by: Vinod Koul <vkoul@kernel.org>
---
 drivers/net/phy/at803x.c | 47 ++++++++++++++++++++++++++++++----------
 1 file changed, 36 insertions(+), 11 deletions(-)

diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index c6e7d800fd7a..dc1b13f7fc12 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -110,6 +110,18 @@ static int at803x_debug_reg_mask(struct phy_device *phydev, u16 reg,
 	return phy_write(phydev, AT803X_DEBUG_DATA, val);
 }
 
+static int at803x_enable_rx_delay(struct phy_device *phydev)
+{
+	return at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_0, 0,
+				     AT803X_DEBUG_RX_CLK_DLY_EN);
+}
+
+static int at803x_enable_tx_delay(struct phy_device *phydev)
+{
+	return at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_5, 0,
+				     AT803X_DEBUG_TX_CLK_DLY_EN);
+}
+
 static int at803x_disable_rx_delay(struct phy_device *phydev)
 {
 	return at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_0,
@@ -255,23 +267,36 @@ static int at803x_config_init(struct phy_device *phydev)
 	if (ret < 0)
 		return ret;
 
-	if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID ||
-			phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
-			phydev->interface == PHY_INTERFACE_MODE_RGMII) {
-		ret = at803x_disable_rx_delay(phydev);
+	/* The hardware register default is RX and TX delay enabled, so lets
+	 * first disable the RX and TX delays in phy and enable them based
+	 * on the mode selected
+	 */
+	ret = at803x_disable_rx_delay(phydev);
+	if (ret < 0)
+		return ret;
+	ret = at803x_disable_tx_delay(phydev);
+	if (ret < 0)
+		return ret;
+
+	if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
+	    phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID) {
+		/* If RGMII_ID or RGMII_RXID are specified enable RX delay,
+		 * otherwise keep it disabled
+		 */
+		ret = at803x_enable_rx_delay(phydev);
 		if (ret < 0)
 			return ret;
 	}
 
-	if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID ||
-			phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
-			phydev->interface == PHY_INTERFACE_MODE_RGMII) {
-		ret = at803x_disable_tx_delay(phydev);
-		if (ret < 0)
-			return ret;
+	if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
+	    phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID) {
+		/* If RGMII_ID or RGMII_TXID are specified enable TX delay,
+		 * otherwise keep it disabled
+		 */
+		ret = at803x_enable_tx_delay(phydev);
 	}
 
-	return 0;
+	return ret;
 }
 
 static int at803x_ack_interrupt(struct phy_device *phydev)
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH RFC 0/5] net/sched: validate the control action with all the other parameters
From: Cong Wang @ 2019-02-19  6:42 UTC (permalink / raw)
  To: Davide Caratti
  Cc: Jamal Hadi Salim, Jiri Pirko, David S. Miller, Vlad Buslov,
	Paolo Abeni, Linux Kernel Network Developers
In-Reply-To: <cover.1550271080.git.dcaratti@redhat.com>

On Fri, Feb 15, 2019 at 3:06 PM Davide Caratti <dcaratti@redhat.com> wrote:
>
> currently, the kernel checks for bad values of the control action in
> tcf_action_init_1(), after a successful call to the action's init()
> function. This causes three bad behaviors:

Yeah, I have been complaining about this for a long time,
although slightly differently. The problem here is the lack of
"copy" in RCU mechanism, which makes it nearly impossible
to rollback to the previous state of an action on failure path
of an update, which also makes RCU readers reading a partially
updated action too.

Before I fix the "copy" part, your fixes make sense to me. There
might be some other way to expose the action-specific tcfa_action
opcode, but it might not be better than yours.

BTW, please fold these bad behaviors into each appropriate
patch, and keep the cover letter as an overview of the whole
patchset rather than showing any details.

[...]

> all these three problems can be fixed if we validate the control action
> in the init() function, in the same way as we are already doing for all
> the other parameters.
>
> - patch 1 is a temporary fix for problem 2), but it's reverted at the
>   end of the series

Please drop patch 1, it is very unlikely only patch 1 will be backported,
I think the whole patchset should be, therefore we have no reason
to carry a temporary fix here.

Thanks.

^ permalink raw reply

* Re: [PATCH net] vhost: correctly check the return value of translate_desc() in log_used()
From: Jason Wang @ 2019-02-19  6:50 UTC (permalink / raw)
  To: David Miller; +Cc: mst, kvm, virtualization, netdev, linux-kernel, stephen
In-Reply-To: <20190215.100356.251698752340585300.davem@davemloft.net>


On 2019/2/16 上午2:03, David Miller wrote:
> From: Jason Wang<jasowang@redhat.com>
> Date: Fri, 15 Feb 2019 15:53:24 +0800
>
>> When fail, translate_desc() returns negative value, otherwise the
>> number of iovs. So we should fail when the return value is negative
>> instead of a blindly check against zero.
>>
>> Reported-by: Stephen Hemminger<stephen@networkplumber.org>
>> Fixes: cc5e71075947 ("vhost: log dirty page correctly")
>> Signed-off-by: Jason Wang<jasowang@redhat.com>
> Jason, please put the Fixes tag first.
>
> Thank you.


Ok. Will post V2.

Thanks


^ permalink raw reply

* Re: [PATCH net] vhost: correctly check the return value of translate_desc() in log_used()
From: Jason Wang @ 2019-02-19  6:51 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: mst, kvm, virtualization, netdev, linux-kernel
In-Reply-To: <20190215084502.23feed0e@shemminger-XPS-13-9360>


On 2019/2/16 上午12:45, Stephen Hemminger wrote:
> On Fri, 15 Feb 2019 15:53:24 +0800
> Jason Wang <jasowang@redhat.com> wrote:
>
>> When fail, translate_desc() returns negative value, otherwise the
>> number of iovs. So we should fail when the return value is negative
>> instead of a blindly check against zero.
>>
>> Reported-by: Stephen Hemminger <stephen@networkplumber.org>
>> Fixes: cc5e71075947 ("vhost: log dirty page correctly")
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
> Looks good. It is best to put the Addresses-Coverity-Id tag on these kind
> of bug fixes so that the automated tools see it.


Ok. Will do this in V2.

Thanks



^ permalink raw reply

* [PATCH net V2] vhost: correctly check the return value of translate_desc() in log_used()
From: Jason Wang @ 2019-02-19  6:53 UTC (permalink / raw)
  To: mst, jasowang; +Cc: kvm, virtualization, netdev, linux-kernel

When fail, translate_desc() returns negative value, otherwise the
number of iovs. So we should fail when the return value is negative
instead of a blindly check against zero.

Detected by CoverityScan, CID# 1442593:  Control flow issues  (DEADCODE)

Fixes: cc5e71075947 ("vhost: log dirty page correctly")
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Reported-by: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 drivers/vhost/vhost.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 24a129fcdd61..a2e5dc7716e2 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -1788,7 +1788,7 @@ static int log_used(struct vhost_virtqueue *vq, u64 used_offset, u64 len)
 
 	ret = translate_desc(vq, (uintptr_t)vq->used + used_offset,
 			     len, iov, 64, VHOST_ACCESS_WO);
-	if (ret)
+	if (ret < 0)
 		return ret;
 
 	for (i = 0; i < ret; i++) {
-- 
2.17.1


^ permalink raw reply related

* [PATCH net-next v2] net: dsa: qca8k: Enable delay for RGMII_ID mode
From: Vinod Koul @ 2019-02-19  6:59 UTC (permalink / raw)
  To: David S Miller
  Cc: linux-arm-msm, Bjorn Andersson, Vinod Koul, netdev, Niklas Cassel,
	Andrew Lunn, Florian Fainelli, Michal Vokáč

RGMII_ID specifies that we should have internal delay, so resurrect the
delay addition routine but under the RGMII_ID mode.

Fixes: 40269aa9f40a ("net: dsa: qca8k: disable delay for RGMII mode")
Tested-by: Michal Vokáč <michal.vokac@ysoft.com>
Signed-off-by: Vinod Koul <vkoul@kernel.org>
---
 drivers/net/dsa/qca8k.c | 12 ++++++++++++
 drivers/net/dsa/qca8k.h |  1 +
 2 files changed, 13 insertions(+)

diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
index a4b6cda38016..195a8a87b984 100644
--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -443,6 +443,18 @@ qca8k_set_pad_ctrl(struct qca8k_priv *priv, int port, int mode)
 		val = QCA8K_PORT_PAD_RGMII_EN;
 		qca8k_write(priv, reg, val);
 		break;
+	case PHY_INTERFACE_MODE_RGMII_ID:
+		/* RGMII_ID needs internal delay. This is enabled through
+		 * PORT5_PAD_CTRL for all ports, rather than individual port
+		 * registers
+		 */
+		qca8k_write(priv, reg,
+			    QCA8K_PORT_PAD_RGMII_EN |
+			    QCA8K_PORT_PAD_RGMII_TX_DELAY(QCA8K_MAX_DELAY) |
+			    QCA8K_PORT_PAD_RGMII_RX_DELAY(QCA8K_MAX_DELAY));
+		qca8k_write(priv, QCA8K_REG_PORT5_PAD_CTRL,
+			    QCA8K_PORT_PAD_RGMII_RX_DELAY_EN);
+		break;
 	case PHY_INTERFACE_MODE_SGMII:
 		qca8k_write(priv, reg, QCA8K_PORT_PAD_SGMII_EN);
 		break;
diff --git a/drivers/net/dsa/qca8k.h b/drivers/net/dsa/qca8k.h
index 613fe5c50236..d146e54c8a6c 100644
--- a/drivers/net/dsa/qca8k.h
+++ b/drivers/net/dsa/qca8k.h
@@ -40,6 +40,7 @@
 						((0x8 + (x & 0x3)) << 22)
 #define   QCA8K_PORT_PAD_RGMII_RX_DELAY(x)		\
 						((0x10 + (x & 0x3)) << 20)
+#define   QCA8K_MAX_DELAY				3
 #define   QCA8K_PORT_PAD_RGMII_RX_DELAY_EN		BIT(24)
 #define   QCA8K_PORT_PAD_SGMII_EN			BIT(7)
 #define QCA8K_REG_MODULE_EN				0x030
-- 
2.20.1


^ permalink raw reply related

* Re: [RFC PATCH net-next v3 07/21] ethtool: implement EVENT notifications
From: Michal Kubecek @ 2019-02-19  7:02 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: netdev, David Miller, Jakub Kicinski, Jiri Pirko, linux-kernel
In-Reply-To: <20190218234608.GI14879@lunn.ch>

On Tue, Feb 19, 2019 at 12:46:08AM +0100, Andrew Lunn wrote:
> On Mon, Feb 18, 2019 at 07:21:59PM +0100, Michal Kubecek wrote:
> > Three types of netlink notifications are introduced:
> > 
> >   - ETHA_EVENT_NEWDEV to notify about newly registered network devices
> >   - ETHA_EVENT_DELDEV to notify about unregistered network devices
> >   - ETHA_EVENT_RENAMEDEV to notify about renamed network device
> > 
> > The notifications are triggered by NETDEV_REGISTER, NETDEV_UNREGISTER and
> > NETDEV_CHANGENAME notifiers.
> > 
> > These notifications are intended for applications and daemons monitoring
> > ethtool events to allow updating the list of existing devices without
> > having to open another socket for rtnetlink.
> 
> Hi Michal
> 
> Does ETHA_EVENT_RENAMEDEV actually contain enough information to avoid
> needing a rtnetlink lookup? If i understand the code correctly, all
> you have is the new name. You don't know the old name?
> 
> Having said that, i don't see an easy way to get access to the old
> name when handling the NETDEV_CHANGENAME.

We don't have the old name and without modifying the NETDEV_CHANGENAME
notifier to pass it, there is probably no way to get it. But the ethtool
notification also contains ifindex so that userspace applications which
want to track the configuration can remember both and identify the
renamed device by ifindex.

Michal

^ permalink raw reply

* [tipc-discussion][net 0/1] tipc: fix race condition between sleep/wakeup
From: Tung Nguyen @ 2019-02-19  7:03 UTC (permalink / raw)
  To: davem, netdev; +Cc: tipc-discussion

Fix issue of hung sendto() in user space.

Tung Nguyen (1):
  tipc: fix race condition causing hung sendto

 net/tipc/socket.c | 4 ++++
 1 file changed, 4 insertions(+)
 mode change 100644 => 100755 net/tipc/socket.c

-- 
2.17.1


^ permalink raw reply

* [tipc-discussion][net 1/1] tipc: fix race condition causing hung sendto
From: Tung Nguyen @ 2019-02-19  7:03 UTC (permalink / raw)
  To: davem, netdev; +Cc: tipc-discussion
In-Reply-To: <20190219070310.23888-1-tung.q.nguyen@dektech.com.au>

When sending multicast messages via blocking socket,
if sending link is congested (tsk->cong_link_cnt is set to 1),
the sending thread will be put into sleeping state. However,
tipc_sk_filter_rcv() is called under socket spin lock but
tipc_wait_for_cond() is not. So, there is no guarantee that
the setting of tsk->cong_link_cnt to 0 in tipc_sk_proto_rcv() in
CPU-1 will be perceived by CPU-0. If that is the case, the sending
thread in CPU-0 after being waken up, will continue to see
tsk->cong_link_cnt as 1 and put the sending thread into sleeping
state again. The sending thread will sleep forever.

CPU-0                                | CPU-1
tipc_wait_for_cond()                 |
{                                    |
 // condition_ = !tsk->cong_link_cnt |
 while ((rc_ = !(condition_))) {     |
  ...                                |
  release_sock(sk_);                 |
  wait_woken();                      |
                                     | if (!sock_owned_by_user(sk))
                                     |  tipc_sk_filter_rcv()
                                     |  {
                                     |   ...
                                     |   tipc_sk_proto_rcv()
                                     |   {
                                     |    ...
                                     |    tsk->cong_link_cnt--;
                                     |    ...
                                     |    sk->sk_write_space(sk);
                                     |    ...
                                     |   }
                                     |   ...
                                     |  }
  sched_annotate_sleep();            |
  lock_sock(sk_);                    |
  remove_wait_queue();               |
 }                                   |
}                                    |

This commit fixes it by adding memory barrier to tipc_sk_proto_rcv()
and tipc_wait_for_cond().

Acked-by: Jon Maloy <jon.maloy@ericsson.com>
Signed-off-by: Tung Nguyen <tung.q.nguyen@dektech.com.au>
---
 net/tipc/socket.c | 4 ++++
 1 file changed, 4 insertions(+)
 mode change 100644 => 100755 net/tipc/socket.c

diff --git a/net/tipc/socket.c b/net/tipc/socket.c
old mode 100644
new mode 100755
index 1217c90a363b..d8f054d45941
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -383,6 +383,8 @@ static int tipc_sk_sock_err(struct socket *sock, long *timeout)
 	int rc_;							       \
 									       \
 	while ((rc_ = !(condition_))) {					       \
+		/* coupled with smp_wmb() in tipc_sk_proto_rcv() */            \
+		smp_rmb();                                                     \
 		DEFINE_WAIT_FUNC(wait_, woken_wake_function);	               \
 		sk_ = (sock_)->sk;					       \
 		rc_ = tipc_sk_sock_err((sock_), timeo_);		       \
@@ -1982,6 +1984,8 @@ static void tipc_sk_proto_rcv(struct sock *sk,
 		return;
 	case SOCK_WAKEUP:
 		tipc_dest_del(&tsk->cong_links, msg_orignode(hdr), 0);
+		/* coupled with smp_rmb() in tipc_wait_for_cond() */
+		smp_wmb();
 		tsk->cong_link_cnt--;
 		wakeup = true;
 		break;
-- 
2.17.1


^ permalink raw reply related

* [PATCH] phy/marvell: prevent unneeded page switching
From: Frank de Brabander @ 2019-02-19  7:21 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, Heiner Kallweit; +Cc: Frank de Brabander, netdev

marvell_read_status() no longer first switches to the fiber page, instead
looks at the already active register page first. In case the link is down,
switches to the other (fiber/copper) register page.

Unneeded register page switching of the Marvell 88E1510 PHY can cause
the ethernet driver to register a link down & up event.

Signed-off-by: Frank de Brabander <debrabander@gmail.com>
---
 drivers/net/phy/marvell.c | 59 ++++++++++++++++++++++++++++++-----------------
 1 file changed, 38 insertions(+), 21 deletions(-)

diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index abb7876..31de2db 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -194,6 +194,11 @@ static int marvell_read_page(struct phy_device *phydev)
 	return __phy_read(phydev, MII_MARVELL_PHY_PAGE);
 }
 
+static int marvell_get_page(struct phy_device *phydev)
+{
+	return phy_read(phydev, MII_MARVELL_PHY_PAGE);
+}
+
 static int marvell_write_page(struct phy_device *phydev, int page)
 {
 	return __phy_write(phydev, MII_MARVELL_PHY_PAGE, page);
@@ -1245,47 +1250,59 @@ static int marvell_read_status_page(struct phy_device *phydev, int page)
  * Some Marvell's phys have two modes: fiber and copper.
  * Both need status checked.
  * Description:
- *   First, check the fiber link and status.
- *   If the fiber link is down, check the copper link and status which
- *   will be the default value if both link are down.
+ *   First, check the link of the currently selected page. Unneeded page
+ *   switches should be prevented, they can trigger the ethernet driver to
+ *   register a link down event.
+ *   If the first checked link type is down, continue with checking the other
+ *   link type. This is needed for platforms that have a fiber and copper link
+ *   connected to the same phy.
  */
 static int marvell_read_status(struct phy_device *phydev)
 {
 	int err;
+	int page;
 
-	/* Check the fiber mode first */
 	if (linkmode_test_bit(ETHTOOL_LINK_MODE_FIBRE_BIT,
 			      phydev->supported) &&
 	    phydev->interface != PHY_INTERFACE_MODE_SGMII) {
-		err = marvell_set_page(phydev, MII_MARVELL_FIBER_PAGE);
-		if (err < 0)
-			goto error;
+		page = marvell_get_page(phydev);
+		if (page < 0)
+			return page;
+
+		if (page != MII_MARVELL_FIBER_PAGE &&
+		    page != MII_MARVELL_COPPER_PAGE) {
+			page = MII_MARVELL_FIBER_PAGE;
 
-		err = marvell_read_status_page(phydev, MII_MARVELL_FIBER_PAGE);
+			err = marvell_set_page(phydev, page);
+			if (err < 0)
+				return err;
+		}
+
+		err = marvell_read_status_page(phydev, page);
 		if (err < 0)
-			goto error;
+			return err;
 
-		/* If the fiber link is up, it is the selected and
-		 * used link. In this case, we need to stay in the
-		 * fiber page. Please to be careful about that, avoid
-		 * to restore Copper page in other functions which
-		 * could break the behaviour for some fiber phy like
+		/* If the link is up, it is the selected and
+		 * used link. In this case, we need to stay in that
+		 * page. Please to be careful about that, avoid
+		 * to restore the page in other functions which
+		 * could break the behaviour for some phy's like
 		 * 88E1512.
 		 */
 		if (phydev->link)
 			return 0;
 
-		/* If fiber link is down, check and save copper mode state */
-		err = marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE);
+		page = (page == MII_MARVELL_FIBER_PAGE) ?
+			MII_MARVELL_COPPER_PAGE : MII_MARVELL_FIBER_PAGE;
+
+		err = marvell_set_page(phydev, page);
 		if (err < 0)
-			goto error;
+			return err;
+
+		return marvell_read_status_page(phydev, page);
 	}
 
 	return marvell_read_status_page(phydev, MII_MARVELL_COPPER_PAGE);
-
-error:
-	marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE);
-	return err;
 }
 
 /* marvell_suspend
-- 
2.7.4


^ permalink raw reply related

* [PATCH] net: ns83820: drop pointless static qualifier in ns83820_probe_phy()
From: Mao Wenan @ 2019-02-19  8:06 UTC (permalink / raw)
  To: kernel-janitors, netdev, john.fastabend, hawk, jakub.kicinski,
	daniel, ast

There is no need to have the 'int first' static
since new value always be assigned before use it.
The old codes of first dereferencing this variable have
been commented out. So it is useless with 'static int first'.
/*
if (!first) {
 ...
}
*/
first = 1;

Signed-off-by: Mao Wenan <maowenan@huawei.com>
---
 drivers/net/ethernet/natsemi/ns83820.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/natsemi/ns83820.c b/drivers/net/ethernet/natsemi/ns83820.c
index 958fced4dacf..fb064df3a1a6 100644
--- a/drivers/net/ethernet/natsemi/ns83820.c
+++ b/drivers/net/ethernet/natsemi/ns83820.c
@@ -1869,7 +1869,7 @@ static unsigned ns83820_mii_write_reg(struct ns83820 *dev, unsigned phy, unsigne
 static void ns83820_probe_phy(struct net_device *ndev)
 {
 	struct ns83820 *dev = PRIV(ndev);
-	static int first;
+	int first;
 	int i;
 #define MII_PHYIDR1	0x02
 #define MII_PHYIDR2	0x03
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH] net: ns83820: drop pointless static qualifier in ns83820_probe_phy()
From: Julia Lawall @ 2019-02-19  8:01 UTC (permalink / raw)
  To: Mao Wenan
  Cc: kernel-janitors, netdev, john.fastabend, hawk, jakub.kicinski,
	daniel, ast
In-Reply-To: <20190219080659.89732-1-maowenan@huawei.com>



On Tue, 19 Feb 2019, Mao Wenan wrote:

> There is no need to have the 'int first' static
> since new value always be assigned before use it.
> The old codes of first dereferencing this variable have
> been commented out. So it is useless with 'static int first'.
> /*
> if (!first) {
>  ...
> }
> */
> first = 1;

Do you need the variable at all?

In the code, the commenting out is actually done with #if 0.  It could be
good for the commit log to reflect the code more accurately.

Can the #if 0 and code be removed?  Apparently it predates git.

A commented out printk below also predates git.

julia

>
> Signed-off-by: Mao Wenan <maowenan@huawei.com>
> ---
>  drivers/net/ethernet/natsemi/ns83820.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/natsemi/ns83820.c b/drivers/net/ethernet/natsemi/ns83820.c
> index 958fced4dacf..fb064df3a1a6 100644
> --- a/drivers/net/ethernet/natsemi/ns83820.c
> +++ b/drivers/net/ethernet/natsemi/ns83820.c
> @@ -1869,7 +1869,7 @@ static unsigned ns83820_mii_write_reg(struct ns83820 *dev, unsigned phy, unsigne
>  static void ns83820_probe_phy(struct net_device *ndev)
>  {
>  	struct ns83820 *dev = PRIV(ndev);
> -	static int first;
> +	int first;
>  	int i;
>  #define MII_PHYIDR1	0x02
>  #define MII_PHYIDR2	0x03
> --
> 2.20.1
>
>

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox