Netdev List
 help / color / mirror / Atom feed
* [PATCH 2/2] samples: pktgen: allow to specify destination port
From: Daniel T. Lee @ 2019-06-29 13:33 UTC (permalink / raw)
  To: Jesper Dangaard Brouer, David S . Miller; +Cc: netdev
In-Reply-To: <20190629133358.8251-1-danieltimlee@gmail.com>

Currently, kernel pktgen has the feature to specify udp destination port
for sending packet. (e.g. pgset "udp_dst_min 9")

But on samples, each of the scripts doesn't have any option to achieve this.

This commit adds the DST_PORT option to specify the target port(s) in the script.

    -p : ($DST_PORT)  destination PORT range (e.g. 433-444) is also allowed

Signed-off-by: Daniel T. Lee <danieltimlee@gmail.com>
---
 samples/pktgen/README.rst                            |  1 +
 samples/pktgen/parameters.sh                         |  7 ++++++-
 .../pktgen/pktgen_bench_xmit_mode_netif_receive.sh   | 11 +++++++++++
 samples/pktgen/pktgen_bench_xmit_mode_queue_xmit.sh  | 11 +++++++++++
 samples/pktgen/pktgen_sample01_simple.sh             | 11 +++++++++++
 samples/pktgen/pktgen_sample02_multiqueue.sh         | 11 +++++++++++
 samples/pktgen/pktgen_sample03_burst_single_flow.sh  | 11 +++++++++++
 samples/pktgen/pktgen_sample04_many_flows.sh         | 11 +++++++++++
 samples/pktgen/pktgen_sample05_flow_per_thread.sh    | 12 +++++++++++-
 ...pktgen_sample06_numa_awared_queue_irq_affinity.sh | 11 +++++++++++
 10 files changed, 95 insertions(+), 2 deletions(-)

diff --git a/samples/pktgen/README.rst b/samples/pktgen/README.rst
index ff8929da61c5..fd39215db508 100644
--- a/samples/pktgen/README.rst
+++ b/samples/pktgen/README.rst
@@ -20,6 +20,7 @@ across the sample scripts.  Usage example is printed on errors::
   -s : ($PKT_SIZE)  packet size
   -d : ($DEST_IP)   destination IP
   -m : ($DST_MAC)   destination MAC-addr
+  -p : ($DST_PORT)  destination PORT range (e.g. 433-444) is also allowed
   -t : ($THREADS)   threads to start
   -f : ($F_THREAD)  index of first thread (zero indexed CPU number)
   -c : ($SKB_CLONE) SKB clones send before alloc new SKB
diff --git a/samples/pktgen/parameters.sh b/samples/pktgen/parameters.sh
index 72fc562876e2..a06b00a0c7b6 100644
--- a/samples/pktgen/parameters.sh
+++ b/samples/pktgen/parameters.sh
@@ -10,6 +10,7 @@ function usage() {
     echo "  -s : (\$PKT_SIZE)  packet size"
     echo "  -d : (\$DEST_IP)   destination IP"
     echo "  -m : (\$DST_MAC)   destination MAC-addr"
+    echo "  -p : (\$DST_PORT)  destination PORT range (e.g. 433-444) is also allowed"
     echo "  -t : (\$THREADS)   threads to start"
     echo "  -f : (\$F_THREAD)  index of first thread (zero indexed CPU number)"
     echo "  -c : (\$SKB_CLONE) SKB clones send before alloc new SKB"
@@ -23,7 +24,7 @@ function usage() {
 
 ##  --- Parse command line arguments / parameters ---
 ## echo "Commandline options:"
-while getopts "s:i:d:m:f:t:c:n:b:vxh6" option; do
+while getopts "s:i:d:m:p:f:t:c:n:b:vxh6" option; do
     case $option in
         i) # interface
           export DEV=$OPTARG
@@ -41,6 +42,10 @@ while getopts "s:i:d:m:f:t:c:n:b:vxh6" option; do
           export DST_MAC=$OPTARG
 	  info "Destination MAC set to: DST_MAC=$DST_MAC"
           ;;
+        p) # PORT
+          export DST_PORT=$OPTARG
+	  info "Destination PORT set to: DST_PORT=$DST_PORT"
+          ;;
         f)
 	  export F_THREAD=$OPTARG
 	  info "Index of first thread (zero indexed CPU number): $F_THREAD"
diff --git a/samples/pktgen/pktgen_bench_xmit_mode_netif_receive.sh b/samples/pktgen/pktgen_bench_xmit_mode_netif_receive.sh
index 2839f7d315cf..e14b1a9144d9 100755
--- a/samples/pktgen/pktgen_bench_xmit_mode_netif_receive.sh
+++ b/samples/pktgen/pktgen_bench_xmit_mode_netif_receive.sh
@@ -41,6 +41,10 @@ fi
 [ -z "$DST_MAC" ] && DST_MAC="90:e2:ba:ff:ff:ff"
 [ -z "$BURST" ] && BURST=1024
 [ -z "$COUNT" ] && COUNT="10000000" # Zero means indefinitely
+if [ -n "$DST_PORT" ]; then
+    read -r DST_MIN DST_MAX <<< $(parse_ports $DST_PORT)
+    validate_ports $DST_MIN $DST_MAX
+fi
 
 # Base Config
 DELAY="0"        # Zero means max speed
@@ -69,6 +73,13 @@ for ((thread = $F_THREAD; thread <= $L_THREAD; thread++)); do
     pg_set $dev "dst_mac $DST_MAC"
     pg_set $dev "dst$IP6 $DEST_IP"
 
+    if [ -n "$DST_PORT" ]; then
+	# Single destination port or random port range
+	pg_set $dev "flag UDPDST_RND"
+	pg_set $dev "udp_dst_min $DST_MIN"
+	pg_set $dev "udp_dst_max $DST_MAX"
+    fi
+
     # Inject packet into RX path of stack
     pg_set $dev "xmit_mode netif_receive"
 
diff --git a/samples/pktgen/pktgen_bench_xmit_mode_queue_xmit.sh b/samples/pktgen/pktgen_bench_xmit_mode_queue_xmit.sh
index e1ee54465def..82c3e504e056 100755
--- a/samples/pktgen/pktgen_bench_xmit_mode_queue_xmit.sh
+++ b/samples/pktgen/pktgen_bench_xmit_mode_queue_xmit.sh
@@ -24,6 +24,10 @@ if [[ -n "$BURST" ]]; then
     err 1 "Bursting not supported for this mode"
 fi
 [ -z "$COUNT" ] && COUNT="10000000" # Zero means indefinitely
+if [ -n "$DST_PORT" ]; then
+    read -r DST_MIN DST_MAX <<< $(parse_ports $DST_PORT)
+    validate_ports $DST_MIN $DST_MAX
+fi
 
 # Base Config
 DELAY="0"        # Zero means max speed
@@ -52,6 +56,13 @@ for ((thread = $F_THREAD; thread <= $L_THREAD; thread++)); do
     pg_set $dev "dst_mac $DST_MAC"
     pg_set $dev "dst$IP6 $DEST_IP"
 
+    if [ -n "$DST_PORT" ]; then
+	# Single destination port or random port range
+	pg_set $dev "flag UDPDST_RND"
+	pg_set $dev "udp_dst_min $DST_MIN"
+	pg_set $dev "udp_dst_max $DST_MAX"
+    fi
+
     # Inject packet into TX qdisc egress path of stack
     pg_set $dev "xmit_mode queue_xmit"
 done
diff --git a/samples/pktgen/pktgen_sample01_simple.sh b/samples/pktgen/pktgen_sample01_simple.sh
index e9ab4edba2d7..d1702fdde8f3 100755
--- a/samples/pktgen/pktgen_sample01_simple.sh
+++ b/samples/pktgen/pktgen_sample01_simple.sh
@@ -22,6 +22,10 @@ fi
 # Example enforce param "-m" for dst_mac
 [ -z "$DST_MAC" ] && usage && err 2 "Must specify -m dst_mac"
 [ -z "$COUNT" ]   && COUNT="100000" # Zero means indefinitely
+if [ -n "$DST_PORT" ]; then
+    read -r DST_MIN DST_MAX <<< $(parse_ports $DST_PORT)
+    validate_ports $DST_MIN $DST_MAX
+fi
 
 # Base Config
 DELAY="0"        # Zero means max speed
@@ -59,6 +63,13 @@ pg_set $DEV "flag NO_TIMESTAMP"
 pg_set $DEV "dst_mac $DST_MAC"
 pg_set $DEV "dst$IP6 $DEST_IP"
 
+if [ -n "$DST_PORT" ]; then
+    # Single destination port or random port range
+    pg_set $DEV "flag UDPDST_RND"
+    pg_set $DEV "udp_dst_min $DST_MIN"
+    pg_set $DEV "udp_dst_max $DST_MAX"
+fi
+
 # Setup random UDP port src range
 pg_set $DEV "flag UDPSRC_RND"
 pg_set $DEV "udp_src_min $UDP_MIN"
diff --git a/samples/pktgen/pktgen_sample02_multiqueue.sh b/samples/pktgen/pktgen_sample02_multiqueue.sh
index 99f740ae9857..7f7a9a27548f 100755
--- a/samples/pktgen/pktgen_sample02_multiqueue.sh
+++ b/samples/pktgen/pktgen_sample02_multiqueue.sh
@@ -29,6 +29,10 @@ if [ -z "$DEST_IP" ]; then
     [ -z "$IP6" ] && DEST_IP="198.18.0.42" || DEST_IP="FD00::1"
 fi
 [ -z "$DST_MAC" ] && DST_MAC="90:e2:ba:ff:ff:ff"
+if [ -n "$DST_PORT" ]; then
+    read -r DST_MIN DST_MAX <<< $(parse_ports $DST_PORT)
+    validate_ports $DST_MIN $DST_MAX
+fi
 
 # General cleanup everything since last run
 pg_ctrl "reset"
@@ -60,6 +64,13 @@ for ((thread = $F_THREAD; thread <= $L_THREAD; thread++)); do
     pg_set $dev "dst_mac $DST_MAC"
     pg_set $dev "dst$IP6 $DEST_IP"
 
+    if [ -n "$DST_PORT" ]; then
+	# Single destination port or random port range
+	pg_set $dev "flag UDPDST_RND"
+	pg_set $dev "udp_dst_min $DST_MIN"
+	pg_set $dev "udp_dst_max $DST_MAX"
+    fi
+
     # Setup random UDP port src range
     pg_set $dev "flag UDPSRC_RND"
     pg_set $dev "udp_src_min $UDP_MIN"
diff --git a/samples/pktgen/pktgen_sample03_burst_single_flow.sh b/samples/pktgen/pktgen_sample03_burst_single_flow.sh
index 8fdd36722d9e..b520637817ce 100755
--- a/samples/pktgen/pktgen_sample03_burst_single_flow.sh
+++ b/samples/pktgen/pktgen_sample03_burst_single_flow.sh
@@ -33,6 +33,10 @@ fi
 [ -z "$BURST" ]     && BURST=32
 [ -z "$CLONE_SKB" ] && CLONE_SKB="0" # No need for clones when bursting
 [ -z "$COUNT" ]     && COUNT="0" # Zero means indefinitely
+if [ -n "$DST_PORT" ]; then
+    read -r DST_MIN DST_MAX <<< $(parse_ports $DST_PORT)
+    validate_ports $DST_MIN $DST_MAX
+fi
 
 # Base Config
 DELAY="0"  # Zero means max speed
@@ -60,6 +64,13 @@ for ((thread = $F_THREAD; thread <= $L_THREAD; thread++)); do
     pg_set $dev "dst_mac $DST_MAC"
     pg_set $dev "dst$IP6 $DEST_IP"
 
+    if [ -n "$DST_PORT" ]; then
+	# Single destination port or random port range
+	pg_set $dev "flag UDPDST_RND"
+	pg_set $dev "udp_dst_min $DST_MIN"
+	pg_set $dev "udp_dst_max $DST_MAX"
+    fi
+
     # Setup burst, for easy testing -b 0 disable bursting
     # (internally in pktgen default and minimum burst=1)
     if [[ ${BURST} -ne 0 ]]; then
diff --git a/samples/pktgen/pktgen_sample04_many_flows.sh b/samples/pktgen/pktgen_sample04_many_flows.sh
index 4df92b7176da..5b6e9d9cb5b5 100755
--- a/samples/pktgen/pktgen_sample04_many_flows.sh
+++ b/samples/pktgen/pktgen_sample04_many_flows.sh
@@ -17,6 +17,10 @@ source ${basedir}/parameters.sh
 [ -z "$DST_MAC" ]   && DST_MAC="90:e2:ba:ff:ff:ff"
 [ -z "$CLONE_SKB" ] && CLONE_SKB="0"
 [ -z "$COUNT" ]     && COUNT="0" # Zero means indefinitely
+if [ -n "$DST_PORT" ]; then
+    read -r DST_MIN DST_MAX <<< $(parse_ports $DST_PORT)
+    validate_ports $DST_MIN $DST_MAX
+fi
 
 # NOTICE:  Script specific settings
 # =======
@@ -56,6 +60,13 @@ for ((thread = $F_THREAD; thread <= $L_THREAD; thread++)); do
     pg_set $dev "dst_mac $DST_MAC"
     pg_set $dev "dst $DEST_IP"
 
+    if [ -n "$DST_PORT" ]; then
+	# Single destination port or random port range
+	pg_set $dev "flag UDPDST_RND"
+	pg_set $dev "udp_dst_min $DST_MIN"
+	pg_set $dev "udp_dst_max $DST_MAX"
+    fi
+
     # Randomize source IP-addresses
     pg_set $dev "flag IPSRC_RND"
     pg_set $dev "src_min 198.18.0.0"
diff --git a/samples/pktgen/pktgen_sample05_flow_per_thread.sh b/samples/pktgen/pktgen_sample05_flow_per_thread.sh
index 7f8b5e59f01e..0c06e63fbe97 100755
--- a/samples/pktgen/pktgen_sample05_flow_per_thread.sh
+++ b/samples/pktgen/pktgen_sample05_flow_per_thread.sh
@@ -22,7 +22,10 @@ source ${basedir}/parameters.sh
 [ -z "$CLONE_SKB" ] && CLONE_SKB="0"
 [ -z "$BURST" ]     && BURST=32
 [ -z "$COUNT" ]     && COUNT="0" # Zero means indefinitely
-
+if [ -n "$DST_PORT" ]; then
+    read -r DST_MIN DST_MAX <<< $(parse_ports $DST_PORT)
+    validate_ports $DST_MIN $DST_MAX
+fi
 
 # Base Config
 DELAY="0"  # Zero means max speed
@@ -50,6 +53,13 @@ for ((thread = $F_THREAD; thread <= $L_THREAD; thread++)); do
     pg_set $dev "dst_mac $DST_MAC"
     pg_set $dev "dst $DEST_IP"
 
+    if [ -n "$DST_PORT" ]; then
+	# Single destination port or random port range
+	pg_set $dev "flag UDPDST_RND"
+	pg_set $dev "udp_dst_min $DST_MIN"
+	pg_set $dev "udp_dst_max $DST_MAX"
+    fi
+
     # Setup source IP-addresses based on thread number
     pg_set $dev "src_min 198.18.$((thread+1)).1"
     pg_set $dev "src_max 198.18.$((thread+1)).1"
diff --git a/samples/pktgen/pktgen_sample06_numa_awared_queue_irq_affinity.sh b/samples/pktgen/pktgen_sample06_numa_awared_queue_irq_affinity.sh
index 353adc17205e..97f0266c0356 100755
--- a/samples/pktgen/pktgen_sample06_numa_awared_queue_irq_affinity.sh
+++ b/samples/pktgen/pktgen_sample06_numa_awared_queue_irq_affinity.sh
@@ -35,6 +35,10 @@ if [ -z "$DEST_IP" ]; then
     [ -z "$IP6" ] && DEST_IP="198.18.0.42" || DEST_IP="FD00::1"
 fi
 [ -z "$DST_MAC" ] && DST_MAC="90:e2:ba:ff:ff:ff"
+if [ -n "$DST_PORT" ]; then
+    read -r DST_MIN DST_MAX <<< $(parse_ports $DST_PORT)
+    validate_ports $DST_MIN $DST_MAX
+fi
 
 # General cleanup everything since last run
 pg_ctrl "reset"
@@ -77,6 +81,13 @@ for ((i = 0; i < $THREADS; i++)); do
     pg_set $dev "dst_mac $DST_MAC"
     pg_set $dev "dst$IP6 $DEST_IP"
 
+    if [ -n "$DST_PORT" ]; then
+	# Single destination port or random port range
+	pg_set $dev "flag UDPDST_RND"
+	pg_set $dev "udp_dst_min $DST_MIN"
+	pg_set $dev "udp_dst_max $DST_MAX"
+    fi
+
     # Setup random UDP port src range
     pg_set $dev "flag UDPSRC_RND"
     pg_set $dev "udp_src_min $UDP_MIN"
-- 
2.17.1


^ permalink raw reply related

* [PATCH 1/2] samples: pktgen: add some helper functions for port parsing
From: Daniel T. Lee @ 2019-06-29 13:33 UTC (permalink / raw)
  To: Jesper Dangaard Brouer, David S . Miller; +Cc: netdev

This commit adds port parsing and port validate helper function to parse
single or range of port(s) from a given string. (e.g. 1234, 443-444)

Helpers will be used in prior to set target port(s) in samples/pktgen.

Signed-off-by: Daniel T. Lee <danieltimlee@gmail.com>
---
 samples/pktgen/functions.sh | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/samples/pktgen/functions.sh b/samples/pktgen/functions.sh
index f8bb3cd0f4ce..4af4046d71be 100644
--- a/samples/pktgen/functions.sh
+++ b/samples/pktgen/functions.sh
@@ -162,3 +162,37 @@ function get_node_cpus()
 
 	echo $node_cpu_list
 }
+
+# Given a single or range of port(s), return minimum and maximum port number.
+function parse_ports()
+{
+    local port_str=$1
+    local port_list
+    local min_port
+    local max_port
+
+    IFS="-" read -ra port_list <<< $port_str
+
+    min_port=${port_list[0]}
+    max_port=${port_list[1]:-$min_port}
+
+    echo $min_port $max_port
+}
+
+# Given a minimum and maximum port, verify port number.
+function validate_ports()
+{
+    local min_port=$1
+    local max_port=$2
+
+    # 0 < port < 65536
+    if [[ $min_port -gt 0 && $min_port -lt 65536 ]]; then
+	if [[ $max_port -gt 0 && $max_port -lt 65536 ]]; then
+	    if [[ $min_port -le $max_port ]]; then
+		return 0
+	    fi
+	fi
+    fi
+
+    err 5 "Invalid port(s): $min_port-$max_port"
+}
-- 
2.17.1


^ permalink raw reply related

* Re: [PATCH] let proc net directory inodes reflect to active net namespace
From: Alexey Dobriyan @ 2019-06-29 13:29 UTC (permalink / raw)
  To: Hallsmark, Per
  Cc: David S. Miller, linux-kernel@vger.kernel.org,
	netdev@vger.kernel.org
In-Reply-To: <B7B4BB465792624BAF51F33077E99065DC5D7225@ALA-MBD.corp.ad.wrs.com>

On Tue, Jun 25, 2019 at 10:36:06AM +0000, Hallsmark, Per wrote:
> +struct proc_dir_entry *proc_net_mkdir(struct net *net, const char *name,
> +				      struct proc_dir_entry *parent)
> +{
> +	struct proc_dir_entry *pde;
> +
> +	pde = proc_mkdir_data(name, 0, parent, net);
> +	pde->proc_dops = &proc_net_dentry_ops;
> +
> +	return pde;
> +}

This requires NULL check at least.

^ permalink raw reply

* Re: Use-after-free in br_multicast_rcv
From: Martin Weinelt @ 2019-06-29 13:23 UTC (permalink / raw)
  To: nikolay, Roopa Prabhu, bridge; +Cc: netdev
In-Reply-To: <E0170D52-C181-4F0F-B5F8-F1801C2A8F5A@cumulusnetworks.com>

On 6/29/19 3:11 PM, nikolay@cumulusnetworks.com wrote:
> On 29 June 2019 14:54:44 EEST, Martin Weinelt <martin@linuxlounge.net> wrote:
>> Hello,
>>
>> we've recently been experiencing memory leaks on our Linux-based
>> routers,
>> at least as far back as v4.19.16.
>>
>> After rebuilding with KASAN it found a use-after-free in 
>> br_multicast_rcv which I could reproduce on v5.2.0-rc6. 
>>
>> Please find the KASAN report below, I'm anot sure what else to provide
>> so
>> feel free to ask.
>>
>> Best,
>>  Martin
>>
>>
> 
> Hi Martin, 
> I'll look into this, are there any specific steps to reproduce it? 
> 
> Thanks, 
>    Nik

Hi Nik,

thanks for taking an interest. The setup is a public L2 mesh network
based on the batman-adv kmod. The bridges see 100 Mbps+ of traffic most
of the day and I unfortunately do not yet know what triggers this.


Best,
  Martin

> 
>> ==================================================================
>> BUG: KASAN: use-after-free in br_multicast_rcv+0x480c/0x4ad0 [bridge]
>> Read of size 2 at addr ffff8880421302b4 by task ksoftirqd/1/16
>>
>> CPU: 1 PID: 16 Comm: ksoftirqd/1 Tainted: G           OE     5.2.0-rc6+
>> #1
>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1
>> 04/01/2014
>> Call Trace:
>> dump_stack+0x71/0xab
>> print_address_description+0x6a/0x280
>> ? br_multicast_rcv+0x480c/0x4ad0 [bridge]
>> __kasan_report+0x152/0x1aa
>> ? br_multicast_rcv+0x480c/0x4ad0 [bridge]
>> ? br_multicast_rcv+0x480c/0x4ad0 [bridge]
>> kasan_report+0xe/0x20
>> br_multicast_rcv+0x480c/0x4ad0 [bridge]
>> ? br_multicast_disable_port+0x150/0x150 [bridge]
>> ? ktime_get_with_offset+0xb4/0x150
>> ? __kasan_kmalloc.constprop.6+0xa6/0xf0
>> ? __netif_receive_skb+0x1b0/0x1b0
>> ? br_fdb_update+0x10e/0x6e0 [bridge]
>> ? br_handle_frame_finish+0x3c6/0x11d0 [bridge]
>> br_handle_frame_finish+0x3c6/0x11d0 [bridge]
>> ? br_pass_frame_up+0x3a0/0x3a0 [bridge]
>> ? virtnet_probe+0x1c80/0x1c80 [virtio_net]
>> br_handle_frame+0x731/0xd90 [bridge]
>> ? select_idle_sibling+0x25/0x7d0
>> ? br_handle_frame_finish+0x11d0/0x11d0 [bridge]
>> __netif_receive_skb_core+0xced/0x2d70
>> ? virtqueue_get_buf_ctx+0x230/0x1130 [virtio_ring]
>> ? do_xdp_generic+0x20/0x20
>> ? virtqueue_napi_complete+0x39/0x70 [virtio_net]
>> ? virtnet_poll+0x94d/0xc78 [virtio_net]
>> ? receive_buf+0x5120/0x5120 [virtio_net]
>> ? __netif_receive_skb_one_core+0x97/0x1d0
>> __netif_receive_skb_one_core+0x97/0x1d0
>> ? __netif_receive_skb_core+0x2d70/0x2d70
>> ? _raw_write_trylock+0x100/0x100
>> ? __queue_work+0x41e/0xbe0
>> process_backlog+0x19c/0x650
>> ? _raw_read_lock_irq+0x40/0x40
>> net_rx_action+0x71e/0xbc0
>> ? __switch_to_asm+0x40/0x70
>> ? napi_complete_done+0x360/0x360
>> ? __switch_to_asm+0x34/0x70
>> ? __switch_to_asm+0x40/0x70
>> ? __schedule+0x85e/0x14d0
>> __do_softirq+0x1db/0x5f9
>> ? takeover_tasklets+0x5f0/0x5f0
>> run_ksoftirqd+0x26/0x40
>> smpboot_thread_fn+0x443/0x680
>> ? sort_range+0x20/0x20
>> ? schedule+0x94/0x210
>> ? __kthread_parkme+0x78/0xf0
>> ? sort_range+0x20/0x20
>> kthread+0x2ae/0x3a0
>> ? kthread_create_worker_on_cpu+0xc0/0xc0
>> ret_from_fork+0x35/0x40
>>
>> The buggy address belongs to the page:
>> page:ffffea0001084c00 refcount:0 mapcount:-128 mapping:0000000000000000
>> index:0x0
>> flags: 0xffffc000000000()
>> raw: 00ffffc000000000 ffffea0000cfca08 ffffea0001098608
>> 0000000000000000
>> raw: 0000000000000000 0000000000000003 00000000ffffff7f
>> 0000000000000000
>> page dumped because: kasan: bad access detected
>>
>> Memory state around the buggy address:
>> ffff888042130180: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
>> ffff888042130200: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
>>> ffff888042130280: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
>>                                     ^
>> ffff888042130300: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
>> ffff888042130380: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
>> ==================================================================
>> Disabling lock debugging due to kernel taint
> 
> 

^ permalink raw reply

* [PATCH] net: ethernet: mediatek: Allow non TRGMII mode with MT7621 DDR2 devices
From: René van Dorst @ 2019-06-29 12:24 UTC (permalink / raw)
  To: sean.wang, f.fainelli, linux, davem, matthias.bgg, andrew,
	vivien.didelot
  Cc: frank-w, netdev, linux-mediatek, linux-mips, René van Dorst

No reason to error out on a MT7621 device with DDR2 memory when non
TRGMII mode is selected.
Only MT7621 DDR2 clock setup is not supported for TRGMII mode.
But non TRGMII mode doesn't need any special clock setup.

Signed-off-by: René van Dorst <opensource@vdorst.com>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 066712f2e985..b20b3a5a1ebb 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -139,9 +139,12 @@ static int mt7621_gmac0_rgmii_adjust(struct mtk_eth *eth,
 {
 	u32 val;
 
-	/* Check DDR memory type. Currently DDR2 is not supported. */
+	/* Check DDR memory type.
+	 * Currently TRGMII mode with DDR2 memory is not supported.
+	 */
 	regmap_read(eth->ethsys, ETHSYS_SYSCFG, &val);
-	if (val & SYSCFG_DRAM_TYPE_DDR2) {
+	if (interface == PHY_INTERFACE_MODE_TRGMII &&
+	    val & SYSCFG_DRAM_TYPE_DDR2) {
 		dev_err(eth->dev,
 			"TRGMII mode with DDR2 memory is not supported!\n");
 		return -EOPNOTSUPP;
-- 
2.20.1


^ permalink raw reply related

* [PATCH] net: ethernet: mediatek: Fix overlapping capability bits.
From: René van Dorst @ 2019-06-29 12:24 UTC (permalink / raw)
  To: sean.wang, f.fainelli, linux, davem, matthias.bgg, andrew,
	vivien.didelot
  Cc: frank-w, netdev, linux-mediatek, linux-mips, René van Dorst

Both MTK_TRGMII_MT7621_CLK and MTK_PATH_BIT are defined as bit 10.

This causes issues on non-MT7621 devices which has the
MTK_PATH_BIT(MTK_ETH_PATH_GMAC1_RGMII) capability set.
The wrong TRGMII setup code is executed.

Moving the MTK_PATH_BIT to bit 11 fixes the issue.

Fixes: 8efaa653a8a5 ("net: ethernet: mediatek: Add MT7621 TRGMII mode
support")
Signed-off-by: René van Dorst <opensource@vdorst.com>
---
 drivers/net/ethernet/mediatek/mtk_eth_soc.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
index 876ce6798709..2cb8a915731c 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
@@ -626,7 +626,7 @@ enum mtk_eth_path {
 #define MTK_TRGMII_MT7621_CLK		BIT(10)
 
 /* Supported path present on SoCs */
-#define MTK_PATH_BIT(x)         BIT((x) + 10)
+#define MTK_PATH_BIT(x)         BIT((x) + 11)
 
 #define MTK_GMAC1_RGMII \
 	(MTK_PATH_BIT(MTK_ETH_PATH_GMAC1_RGMII) | MTK_RGMII)
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH v4] net: netfilter: Fix rpfilter dropping vrf packets by mistake
From: David Ahern @ 2019-06-29 12:19 UTC (permalink / raw)
  To: linmiaohe, pablo@netfilter.org
  Cc: kadlec@blackhole.kfki.hu, fw@strlen.de, davem@davemloft.net,
	kuznet@ms2.inr.ac.ru, yoshfuji@linux-ipv6.org,
	netfilter-devel@vger.kernel.org, coreteam@netfilter.org,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org, Mingfangsen
In-Reply-To: <2213b3e722a14ee48768ecc7118efc46@huawei.com>

On 6/28/19 8:13 PM, linmiaohe wrote:
> You're right. Fib rules code would set FLOWI_FLAG_SKIP_NH_OIF flag.  But I set
> it here for distinguish with the flags & XT_RPFILTER_LOOSE branch. Without
> this, they do the same work and maybe should be  combined. I don't want to
> do that as that makes code confusing.
> Is this code snipet below ok ? If so, I would delete this flag setting.
>  
>        } else if (netif_is_l3_master(dev) || netif_is_l3_slave(dev)) {
>                fl6.flowi6_oif = dev->ifindex;
>         } else if ((flags & XT_RPFILTER_LOOSE) == 0)
>                 fl6.flowi6_oif = dev->ifindex;

that looks fine to me, but it is up to Pablo.

^ permalink raw reply

* Re: [net-next 2/4] gve: Add transmit and receive support
From: kbuild test robot @ 2019-06-29 11:30 UTC (permalink / raw)
  To: Catherine Sullivan
  Cc: kbuild-all, netdev, Catherine Sullivan, Sagi Shahar, Jon Olson,
	Willem de Bruijn, Luigi Rizzo
In-Reply-To: <20190626185251.205687-3-csully@google.com>

[-- Attachment #1: Type: text/plain, Size: 2760 bytes --]

Hi Catherine,

I love your patch! Yet something to improve:

[auto build test ERROR on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Catherine-Sullivan/Add-gve-driver/20190629-070444
config: i386-allyesconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from drivers/net/ethernet/google/gve/gve_main.c:16:0:
   drivers/net/ethernet/google/gve/gve_adminq.h: In function 'gve_assert_size_gve_adminq_create_rx_queue':
>> drivers/net/ethernet/google/gve/gve_adminq.h:134:469: error: call to '__compiletime_assert_134' declared with attribute error: BUILD_BUG_ON failed: sizeof(struct gve_adminq_create_rx_queue) != (48)
    GVE_ASSERT_SIZE(struct, gve_adminq_create_rx_queue, 48);
                                                                                                                                                                                                                                                                                                                                                                                                                                                                                        ^                         
   In file included from drivers/net/ethernet/google/gve/gve_main.c:16:0:
   drivers/net/ethernet/google/gve/gve_adminq.h: In function 'gve_assert_size_gve_adminq_set_driver_parameter':
   drivers/net/ethernet/google/gve/gve_adminq.h:171:489: error: call to '__compiletime_assert_171' declared with attribute error: BUILD_BUG_ON failed: sizeof(struct gve_adminq_set_driver_parameter) != (16)
    GVE_ASSERT_SIZE(struct, gve_adminq_set_driver_parameter, 16);
                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                            ^                         

vim +/__compiletime_assert_134 +134 drivers/net/ethernet/google/gve/gve_adminq.h

   133	
 > 134	GVE_ASSERT_SIZE(struct, gve_adminq_create_rx_queue, 48);
   135	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 67789 bytes --]

^ permalink raw reply

* Re: [net-next 1/1] tipc: embed jiffies in macro TIPC_BC_RETR_LIM
From: Ying Xue @ 2019-06-29 11:07 UTC (permalink / raw)
  To: Jon Maloy, davem, netdev
  Cc: gordan.mihaljevic, tung.q.nguyen, hoang.h.le, canh.d.luu,
	tipc-discussion
In-Reply-To: <1561734380-26868-1-git-send-email-jon.maloy@ericsson.com>

On 6/28/19 11:06 PM, Jon Maloy wrote:
> The macro TIPC_BC_RETR_LIM is always used in combination with 'jiffies',
> so we can just as well perform the addition in the macro itself. This
> way, we get a few shorter code lines and one less line break.
> 
> Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>

Acked-by: Ying Xue <ying.xue@windriver.com>

> ---
>  net/tipc/link.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/net/tipc/link.c b/net/tipc/link.c
> index f8bf63b..66d3a07 100644
> --- a/net/tipc/link.c
> +++ b/net/tipc/link.c
> @@ -207,7 +207,7 @@ enum {
>  	BC_NACK_SND_SUPPRESS,
>  };
>  
> -#define TIPC_BC_RETR_LIM msecs_to_jiffies(10)   /* [ms] */
> +#define TIPC_BC_RETR_LIM  (jiffies + msecs_to_jiffies(10))
>  #define TIPC_UC_RETR_TIME (jiffies + msecs_to_jiffies(1))
>  
>  /*
> @@ -976,8 +976,7 @@ int tipc_link_xmit(struct tipc_link *l, struct sk_buff_head *list,
>  			__skb_queue_tail(transmq, skb);
>  			/* next retransmit attempt */
>  			if (link_is_bc_sndlink(l))
> -				TIPC_SKB_CB(skb)->nxt_retr =
> -					jiffies + TIPC_BC_RETR_LIM;
> +				TIPC_SKB_CB(skb)->nxt_retr = TIPC_BC_RETR_LIM;
>  			__skb_queue_tail(xmitq, _skb);
>  			TIPC_SKB_CB(skb)->ackers = l->ackers;
>  			l->rcv_unacked = 0;
> @@ -1027,7 +1026,7 @@ static void tipc_link_advance_backlog(struct tipc_link *l,
>  		__skb_queue_tail(&l->transmq, skb);
>  		/* next retransmit attempt */
>  		if (link_is_bc_sndlink(l))
> -			TIPC_SKB_CB(skb)->nxt_retr = jiffies + TIPC_BC_RETR_LIM;
> +			TIPC_SKB_CB(skb)->nxt_retr = TIPC_BC_RETR_LIM;
>  
>  		__skb_queue_tail(xmitq, _skb);
>  		TIPC_SKB_CB(skb)->ackers = l->ackers;
> @@ -1123,7 +1122,7 @@ static int tipc_link_bc_retrans(struct tipc_link *l, struct tipc_link *r,
>  		if (link_is_bc_sndlink(l)) {
>  			if (time_before(jiffies, TIPC_SKB_CB(skb)->nxt_retr))
>  				continue;
> -			TIPC_SKB_CB(skb)->nxt_retr = jiffies + TIPC_BC_RETR_LIM;
> +			TIPC_SKB_CB(skb)->nxt_retr = TIPC_BC_RETR_LIM;
>  		}
>  		_skb = __pskb_copy(skb, LL_MAX_HEADER + MIN_H_SIZE, GFP_ATOMIC);
>  		if (!_skb)
> 

^ permalink raw reply

* Re: [PATCH] docs: packing: move it to core-api book and adjust markups
From: Mike Rapoport @ 2019-06-29 10:42 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Linux Doc Mailing List, Mauro Carvalho Chehab, linux-kernel,
	Jonathan Corbet, Vladimir Oltean, Mike Rapoport, Andrew Morton,
	Randy Dunlap, Matthew Wilcox, Kent Overstreet, Arnd Bergmann,
	Jonathan Neuschäfer, David S. Miller, netdev
In-Reply-To: <46cb79dbc4bbff3e5a4e77b548df1e92c105ed0f.1561804613.git.mchehab+samsung@kernel.org>

On Sat, Jun 29, 2019 at 07:37:02AM -0300, Mauro Carvalho Chehab wrote:
> The packing.txt file was misplaced, as docs should be part of
> a documentation book, and not at the root dir.
> 
> So, move it to the core-api directory and add to its index.
> 
> Also, ensure that the file will be properly parsed and the bitmap
> ascii artwork will use a monotonic font.
> 
> Fixes: 554aae35007e ("lib: Add support for generic packing operations")
> Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>

Reviewed-by: Mike Rapoport <rppt@linux.ibm.com>

> ---
>  Documentation/core-api/index.rst              |  1 +
>  .../{packing.txt => core-api/packing.rst}     | 81 +++++++++++--------
>  2 files changed, 50 insertions(+), 32 deletions(-)
>  rename Documentation/{packing.txt => core-api/packing.rst} (61%)
> 
> diff --git a/Documentation/core-api/index.rst b/Documentation/core-api/index.rst
> index d1e5b95bf86d..aebb16d7771f 100644
> --- a/Documentation/core-api/index.rst
> +++ b/Documentation/core-api/index.rst
> @@ -25,6 +25,7 @@ Core utilities
>     librs
>     genalloc
>     errseq
> +   packing
>     printk-formats
>     circular-buffers
>     generic-radix-tree
> diff --git a/Documentation/packing.txt b/Documentation/core-api/packing.rst
> similarity index 61%
> rename from Documentation/packing.txt
> rename to Documentation/core-api/packing.rst
> index f830c98645f1..d8c341fe383e 100644
> --- a/Documentation/packing.txt
> +++ b/Documentation/core-api/packing.rst
> @@ -30,6 +30,7 @@ The solution
>  ------------
> 
>  This API deals with 2 basic operations:
> +
>    - Packing a CPU-usable number into a memory buffer (with hardware
>      constraints/quirks)
>    - Unpacking a memory buffer (which has hardware constraints/quirks)
> @@ -49,10 +50,12 @@ What the examples show is where the logical bytes and bits sit.
> 
>  1. Normally (no quirks), we would do it like this:
> 
> -63 62 61 60 59 58 57 56 55 54 53 52 51 50 49 48 47 46 45 44 43 42 41 40 39 38 37 36 35 34 33 32
> -7                       6                       5                        4
> -31 30 29 28 27 26 25 24 23 22 21 20 19 18 17 16 15 14 13 12 11 10  9  8  7  6  5  4  3  2  1  0
> -3                       2                       1                        0
> +::
> +
> +  63 62 61 60 59 58 57 56 55 54 53 52 51 50 49 48 47 46 45 44 43 42 41 40 39 38 37 36 35 34 33 32
> +  7                       6                       5                        4
> +  31 30 29 28 27 26 25 24 23 22 21 20 19 18 17 16 15 14 13 12 11 10  9  8  7  6  5  4  3  2  1  0
> +  3                       2                       1                        0
> 
>  That is, the MSByte (7) of the CPU-usable u64 sits at memory offset 0, and the
>  LSByte (0) of the u64 sits at memory offset 7.
> @@ -63,10 +66,12 @@ comments as "logical" notation.
> 
>  2. If QUIRK_MSB_ON_THE_RIGHT is set, we do it like this:
> 
> -56 57 58 59 60 61 62 63 48 49 50 51 52 53 54 55 40 41 42 43 44 45 46 47 32 33 34 35 36 37 38 39
> -7                       6                        5                       4
> -24 25 26 27 28 29 30 31 16 17 18 19 20 21 22 23  8  9 10 11 12 13 14 15  0  1  2  3  4  5  6  7
> -3                       2                        1                       0
> +::
> +
> +  56 57 58 59 60 61 62 63 48 49 50 51 52 53 54 55 40 41 42 43 44 45 46 47 32 33 34 35 36 37 38 39
> +  7                       6                        5                       4
> +  24 25 26 27 28 29 30 31 16 17 18 19 20 21 22 23  8  9 10 11 12 13 14 15  0  1  2  3  4  5  6  7
> +  3                       2                        1                       0
> 
>  That is, QUIRK_MSB_ON_THE_RIGHT does not affect byte positioning, but
>  inverts bit offsets inside a byte.
> @@ -74,10 +79,12 @@ inverts bit offsets inside a byte.
> 
>  3. If QUIRK_LITTLE_ENDIAN is set, we do it like this:
> 
> -39 38 37 36 35 34 33 32 47 46 45 44 43 42 41 40 55 54 53 52 51 50 49 48 63 62 61 60 59 58 57 56
> -4                       5                       6                       7
> -7  6  5  4  3  2  1  0  15 14 13 12 11 10  9  8 23 22 21 20 19 18 17 16 31 30 29 28 27 26 25 24
> -0                       1                       2                       3
> +::
> +
> +  39 38 37 36 35 34 33 32 47 46 45 44 43 42 41 40 55 54 53 52 51 50 49 48 63 62 61 60 59 58 57 56
> +  4                       5                       6                       7
> +  7  6  5  4  3  2  1  0  15 14 13 12 11 10  9  8 23 22 21 20 19 18 17 16 31 30 29 28 27 26 25 24
> +  0                       1                       2                       3
> 
>  Therefore, QUIRK_LITTLE_ENDIAN means that inside the memory region, every
>  byte from each 4-byte word is placed at its mirrored position compared to
> @@ -86,18 +93,22 @@ the boundary of that word.
>  4. If QUIRK_MSB_ON_THE_RIGHT and QUIRK_LITTLE_ENDIAN are both set, we do it
>     like this:
> 
> -32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63
> -4                       5                       6                       7
> -0  1  2  3  4  5  6  7  8   9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31
> -0                       1                       2                       3
> +::
> +
> +  32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63
> +  4                       5                       6                       7
> +  0  1  2  3  4  5  6  7  8   9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31
> +  0                       1                       2                       3
> 
> 
>  5. If just QUIRK_LSW32_IS_FIRST is set, we do it like this:
> 
> -31 30 29 28 27 26 25 24 23 22 21 20 19 18 17 16 15 14 13 12 11 10  9  8  7  6  5  4  3  2  1  0
> -3                       2                       1                        0
> -63 62 61 60 59 58 57 56 55 54 53 52 51 50 49 48 47 46 45 44 43 42 41 40 39 38 37 36 35 34 33 32
> -7                       6                       5                        4
> +::
> +
> +  31 30 29 28 27 26 25 24 23 22 21 20 19 18 17 16 15 14 13 12 11 10  9  8  7  6  5  4  3  2  1  0
> +  3                       2                       1                        0
> +  63 62 61 60 59 58 57 56 55 54 53 52 51 50 49 48 47 46 45 44 43 42 41 40 39 38 37 36 35 34 33 32
> +  7                       6                       5                        4
> 
>  In this case the 8 byte memory region is interpreted as follows: first
>  4 bytes correspond to the least significant 4-byte word, next 4 bytes to
> @@ -107,28 +118,34 @@ the more significant 4-byte word.
>  6. If QUIRK_LSW32_IS_FIRST and QUIRK_MSB_ON_THE_RIGHT are set, we do it like
>     this:
> 
> -24 25 26 27 28 29 30 31 16 17 18 19 20 21 22 23  8  9 10 11 12 13 14 15  0  1  2  3  4  5  6  7
> -3                       2                        1                       0
> -56 57 58 59 60 61 62 63 48 49 50 51 52 53 54 55 40 41 42 43 44 45 46 47 32 33 34 35 36 37 38 39
> -7                       6                        5                       4
> +::
> +
> +  24 25 26 27 28 29 30 31 16 17 18 19 20 21 22 23  8  9 10 11 12 13 14 15  0  1  2  3  4  5  6  7
> +  3                       2                        1                       0
> +  56 57 58 59 60 61 62 63 48 49 50 51 52 53 54 55 40 41 42 43 44 45 46 47 32 33 34 35 36 37 38 39
> +  7                       6                        5                       4
> 
> 
>  7. If QUIRK_LSW32_IS_FIRST and QUIRK_LITTLE_ENDIAN are set, it looks like
>     this:
> 
> -7  6  5  4  3  2  1  0  15 14 13 12 11 10  9  8 23 22 21 20 19 18 17 16 31 30 29 28 27 26 25 24
> -0                       1                       2                       3
> -39 38 37 36 35 34 33 32 47 46 45 44 43 42 41 40 55 54 53 52 51 50 49 48 63 62 61 60 59 58 57 56
> -4                       5                       6                       7
> +::
> +
> +  7  6  5  4  3  2  1  0  15 14 13 12 11 10  9  8 23 22 21 20 19 18 17 16 31 30 29 28 27 26 25 24
> +  0                       1                       2                       3
> +  39 38 37 36 35 34 33 32 47 46 45 44 43 42 41 40 55 54 53 52 51 50 49 48 63 62 61 60 59 58 57 56
> +  4                       5                       6                       7
> 
> 
>  8. If QUIRK_LSW32_IS_FIRST, QUIRK_LITTLE_ENDIAN and QUIRK_MSB_ON_THE_RIGHT
>     are set, it looks like this:
> 
> -0  1  2  3  4  5  6  7  8   9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31
> -0                       1                       2                       3
> -32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63
> -4                       5                       6                       7
> +::
> +
> +  0  1  2  3  4  5  6  7  8   9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31
> +  0                       1                       2                       3
> +  32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63
> +  4                       5                       6                       7
> 
> 
>  We always think of our offsets as if there were no quirk, and we translate
> -- 
> 2.21.0
> 

-- 
Sincerely yours,
Mike.


^ permalink raw reply

* [PATCH] docs: packing: move it to core-api book and adjust markups
From: Mauro Carvalho Chehab @ 2019-06-29 10:37 UTC (permalink / raw)
  To: Linux Doc Mailing List
  Cc: Mauro Carvalho Chehab, Mauro Carvalho Chehab, linux-kernel,
	Jonathan Corbet, Vladimir Oltean, Mike Rapoport, Andrew Morton,
	Randy Dunlap, Matthew Wilcox, Kent Overstreet, Arnd Bergmann,
	Jonathan Neuschäfer, David S. Miller, netdev

The packing.txt file was misplaced, as docs should be part of
a documentation book, and not at the root dir.

So, move it to the core-api directory and add to its index.

Also, ensure that the file will be properly parsed and the bitmap
ascii artwork will use a monotonic font.

Fixes: 554aae35007e ("lib: Add support for generic packing operations")
Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
---
 Documentation/core-api/index.rst              |  1 +
 .../{packing.txt => core-api/packing.rst}     | 81 +++++++++++--------
 2 files changed, 50 insertions(+), 32 deletions(-)
 rename Documentation/{packing.txt => core-api/packing.rst} (61%)

diff --git a/Documentation/core-api/index.rst b/Documentation/core-api/index.rst
index d1e5b95bf86d..aebb16d7771f 100644
--- a/Documentation/core-api/index.rst
+++ b/Documentation/core-api/index.rst
@@ -25,6 +25,7 @@ Core utilities
    librs
    genalloc
    errseq
+   packing
    printk-formats
    circular-buffers
    generic-radix-tree
diff --git a/Documentation/packing.txt b/Documentation/core-api/packing.rst
similarity index 61%
rename from Documentation/packing.txt
rename to Documentation/core-api/packing.rst
index f830c98645f1..d8c341fe383e 100644
--- a/Documentation/packing.txt
+++ b/Documentation/core-api/packing.rst
@@ -30,6 +30,7 @@ The solution
 ------------
 
 This API deals with 2 basic operations:
+
   - Packing a CPU-usable number into a memory buffer (with hardware
     constraints/quirks)
   - Unpacking a memory buffer (which has hardware constraints/quirks)
@@ -49,10 +50,12 @@ What the examples show is where the logical bytes and bits sit.
 
 1. Normally (no quirks), we would do it like this:
 
-63 62 61 60 59 58 57 56 55 54 53 52 51 50 49 48 47 46 45 44 43 42 41 40 39 38 37 36 35 34 33 32
-7                       6                       5                        4
-31 30 29 28 27 26 25 24 23 22 21 20 19 18 17 16 15 14 13 12 11 10  9  8  7  6  5  4  3  2  1  0
-3                       2                       1                        0
+::
+
+  63 62 61 60 59 58 57 56 55 54 53 52 51 50 49 48 47 46 45 44 43 42 41 40 39 38 37 36 35 34 33 32
+  7                       6                       5                        4
+  31 30 29 28 27 26 25 24 23 22 21 20 19 18 17 16 15 14 13 12 11 10  9  8  7  6  5  4  3  2  1  0
+  3                       2                       1                        0
 
 That is, the MSByte (7) of the CPU-usable u64 sits at memory offset 0, and the
 LSByte (0) of the u64 sits at memory offset 7.
@@ -63,10 +66,12 @@ comments as "logical" notation.
 
 2. If QUIRK_MSB_ON_THE_RIGHT is set, we do it like this:
 
-56 57 58 59 60 61 62 63 48 49 50 51 52 53 54 55 40 41 42 43 44 45 46 47 32 33 34 35 36 37 38 39
-7                       6                        5                       4
-24 25 26 27 28 29 30 31 16 17 18 19 20 21 22 23  8  9 10 11 12 13 14 15  0  1  2  3  4  5  6  7
-3                       2                        1                       0
+::
+
+  56 57 58 59 60 61 62 63 48 49 50 51 52 53 54 55 40 41 42 43 44 45 46 47 32 33 34 35 36 37 38 39
+  7                       6                        5                       4
+  24 25 26 27 28 29 30 31 16 17 18 19 20 21 22 23  8  9 10 11 12 13 14 15  0  1  2  3  4  5  6  7
+  3                       2                        1                       0
 
 That is, QUIRK_MSB_ON_THE_RIGHT does not affect byte positioning, but
 inverts bit offsets inside a byte.
@@ -74,10 +79,12 @@ inverts bit offsets inside a byte.
 
 3. If QUIRK_LITTLE_ENDIAN is set, we do it like this:
 
-39 38 37 36 35 34 33 32 47 46 45 44 43 42 41 40 55 54 53 52 51 50 49 48 63 62 61 60 59 58 57 56
-4                       5                       6                       7
-7  6  5  4  3  2  1  0  15 14 13 12 11 10  9  8 23 22 21 20 19 18 17 16 31 30 29 28 27 26 25 24
-0                       1                       2                       3
+::
+
+  39 38 37 36 35 34 33 32 47 46 45 44 43 42 41 40 55 54 53 52 51 50 49 48 63 62 61 60 59 58 57 56
+  4                       5                       6                       7
+  7  6  5  4  3  2  1  0  15 14 13 12 11 10  9  8 23 22 21 20 19 18 17 16 31 30 29 28 27 26 25 24
+  0                       1                       2                       3
 
 Therefore, QUIRK_LITTLE_ENDIAN means that inside the memory region, every
 byte from each 4-byte word is placed at its mirrored position compared to
@@ -86,18 +93,22 @@ the boundary of that word.
 4. If QUIRK_MSB_ON_THE_RIGHT and QUIRK_LITTLE_ENDIAN are both set, we do it
    like this:
 
-32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63
-4                       5                       6                       7
-0  1  2  3  4  5  6  7  8   9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31
-0                       1                       2                       3
+::
+
+  32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63
+  4                       5                       6                       7
+  0  1  2  3  4  5  6  7  8   9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31
+  0                       1                       2                       3
 
 
 5. If just QUIRK_LSW32_IS_FIRST is set, we do it like this:
 
-31 30 29 28 27 26 25 24 23 22 21 20 19 18 17 16 15 14 13 12 11 10  9  8  7  6  5  4  3  2  1  0
-3                       2                       1                        0
-63 62 61 60 59 58 57 56 55 54 53 52 51 50 49 48 47 46 45 44 43 42 41 40 39 38 37 36 35 34 33 32
-7                       6                       5                        4
+::
+
+  31 30 29 28 27 26 25 24 23 22 21 20 19 18 17 16 15 14 13 12 11 10  9  8  7  6  5  4  3  2  1  0
+  3                       2                       1                        0
+  63 62 61 60 59 58 57 56 55 54 53 52 51 50 49 48 47 46 45 44 43 42 41 40 39 38 37 36 35 34 33 32
+  7                       6                       5                        4
 
 In this case the 8 byte memory region is interpreted as follows: first
 4 bytes correspond to the least significant 4-byte word, next 4 bytes to
@@ -107,28 +118,34 @@ the more significant 4-byte word.
 6. If QUIRK_LSW32_IS_FIRST and QUIRK_MSB_ON_THE_RIGHT are set, we do it like
    this:
 
-24 25 26 27 28 29 30 31 16 17 18 19 20 21 22 23  8  9 10 11 12 13 14 15  0  1  2  3  4  5  6  7
-3                       2                        1                       0
-56 57 58 59 60 61 62 63 48 49 50 51 52 53 54 55 40 41 42 43 44 45 46 47 32 33 34 35 36 37 38 39
-7                       6                        5                       4
+::
+
+  24 25 26 27 28 29 30 31 16 17 18 19 20 21 22 23  8  9 10 11 12 13 14 15  0  1  2  3  4  5  6  7
+  3                       2                        1                       0
+  56 57 58 59 60 61 62 63 48 49 50 51 52 53 54 55 40 41 42 43 44 45 46 47 32 33 34 35 36 37 38 39
+  7                       6                        5                       4
 
 
 7. If QUIRK_LSW32_IS_FIRST and QUIRK_LITTLE_ENDIAN are set, it looks like
    this:
 
-7  6  5  4  3  2  1  0  15 14 13 12 11 10  9  8 23 22 21 20 19 18 17 16 31 30 29 28 27 26 25 24
-0                       1                       2                       3
-39 38 37 36 35 34 33 32 47 46 45 44 43 42 41 40 55 54 53 52 51 50 49 48 63 62 61 60 59 58 57 56
-4                       5                       6                       7
+::
+
+  7  6  5  4  3  2  1  0  15 14 13 12 11 10  9  8 23 22 21 20 19 18 17 16 31 30 29 28 27 26 25 24
+  0                       1                       2                       3
+  39 38 37 36 35 34 33 32 47 46 45 44 43 42 41 40 55 54 53 52 51 50 49 48 63 62 61 60 59 58 57 56
+  4                       5                       6                       7
 
 
 8. If QUIRK_LSW32_IS_FIRST, QUIRK_LITTLE_ENDIAN and QUIRK_MSB_ON_THE_RIGHT
    are set, it looks like this:
 
-0  1  2  3  4  5  6  7  8   9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31
-0                       1                       2                       3
-32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63
-4                       5                       6                       7
+::
+
+  0  1  2  3  4  5  6  7  8   9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31
+  0                       1                       2                       3
+  32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63
+  4                       5                       6                       7
 
 
 We always think of our offsets as if there were no quirk, and we translate
-- 
2.21.0


^ permalink raw reply related

* Re: [net-next, PATCH 3/3, v2] net: netsec: add XDP support
From: Jesper Dangaard Brouer @ 2019-06-29 10:36 UTC (permalink / raw)
  To: Ilias Apalodimas
  Cc: netdev, jaswinder.singh, ard.biesheuvel, bjorn.topel,
	magnus.karlsson, daniel, ast, makita.toshiaki, jakub.kicinski,
	john.fastabend, davem, maciejromanfijalkowski, brouer
In-Reply-To: <1561785805-21647-4-git-send-email-ilias.apalodimas@linaro.org>

On Sat, 29 Jun 2019 08:23:25 +0300
Ilias Apalodimas <ilias.apalodimas@linaro.org> wrote:

> The interface only supports 1 Tx queue so locking is introduced on
> the Tx queue if XDP is enabled to make sure .ndo_start_xmit and
> .ndo_xdp_xmit won't corrupt Tx ring
> 
> - Performance (SMMU off)
> 
> Benchmark   XDP_SKB     XDP_DRV
> xdp1        291kpps     344kpps
> rxdrop      282kpps     342kpps
> 
> - Performance (SMMU on)
> Benchmark   XDP_SKB     XDP_DRV
> xdp1        167kpps     324kpps
> rxdrop      164kpps     323kpps
> 
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>

This version looks okay to me.  I don't have the hardware, so I've not
tested it...

Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

^ permalink raw reply

* Re: [net-next 1/4] gve: Add basic driver framework for Compute Engine Virtual NIC
From: kbuild test robot @ 2019-06-29 10:25 UTC (permalink / raw)
  To: Catherine Sullivan
  Cc: kbuild-all, netdev, Catherine Sullivan, Sagi Shahar, Jon Olson,
	Willem de Bruijn, Luigi Rizzo
In-Reply-To: <20190626185251.205687-2-csully@google.com>

[-- Attachment #1: Type: text/plain, Size: 1823 bytes --]

Hi Catherine,

I love your patch! Yet something to improve:

[auto build test ERROR on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Catherine-Sullivan/Add-gve-driver/20190629-070444
config: i386-allyesconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from drivers/net/ethernet/google/gve/gve_main.c:16:0:
   drivers/net/ethernet/google/gve/gve_adminq.h: In function 'gve_assert_size_gve_adminq_set_driver_parameter':
>> drivers/net/ethernet/google/gve/gve_adminq.h:102:489: error: call to '__compiletime_assert_102' declared with attribute error: BUILD_BUG_ON failed: sizeof(struct gve_adminq_set_driver_parameter) != (16)
    GVE_ASSERT_SIZE(struct, gve_adminq_set_driver_parameter, 16);
                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                            ^                         

vim +/__compiletime_assert_102 +102 drivers/net/ethernet/google/gve/gve_adminq.h

   101	
 > 102	GVE_ASSERT_SIZE(struct, gve_adminq_set_driver_parameter, 16);
   103	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 67789 bytes --]

^ permalink raw reply

* [PATCH net-next] hinic: add vlan offload support
From: Xue Chaojing @ 2019-06-29  2:26 UTC (permalink / raw)
  To: davem
  Cc: linux-kernel, netdev, luoshaokai, cloud.wangxiaoyun, xuechaojing,
	chiqijun, wulike1

This patch adds vlan offload support for the HINIC driver.

Signed-off-by: Xue Chaojing <xuechaojing@huawei.com>
---
 .../net/ethernet/huawei/hinic/hinic_hw_dev.h  |  2 ++
 .../net/ethernet/huawei/hinic/hinic_hw_wqe.h  | 15 ++++++++++
 .../net/ethernet/huawei/hinic/hinic_main.c    |  9 ++++--
 .../net/ethernet/huawei/hinic/hinic_port.c    | 30 +++++++++++++++++++
 .../net/ethernet/huawei/hinic/hinic_port.h    | 13 ++++++++
 drivers/net/ethernet/huawei/hinic/hinic_rx.c  | 13 ++++++++
 drivers/net/ethernet/huawei/hinic/hinic_tx.c  | 17 +++++++++++
 7 files changed, 97 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/huawei/hinic/hinic_hw_dev.h b/drivers/net/ethernet/huawei/hinic/hinic_hw_dev.h
index e83e3bf850d5..984c98f33258 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_hw_dev.h
+++ b/drivers/net/ethernet/huawei/hinic/hinic_hw_dev.h
@@ -45,6 +45,8 @@ enum hinic_port_cmd {
 
 	HINIC_PORT_CMD_SET_RX_CSUM	= 26,
 
+	HINIC_PORT_CMD_SET_RX_VLAN_OFFLOAD = 27,
+
 	HINIC_PORT_CMD_GET_PORT_STATISTICS = 28,
 
 	HINIC_PORT_CMD_CLEAR_PORT_STATISTICS = 29,
diff --git a/drivers/net/ethernet/huawei/hinic/hinic_hw_wqe.h b/drivers/net/ethernet/huawei/hinic/hinic_hw_wqe.h
index c6b809e24983..f4b6d2c1061f 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_hw_wqe.h
+++ b/drivers/net/ethernet/huawei/hinic/hinic_hw_wqe.h
@@ -222,6 +222,8 @@
 
 #define RQ_CQE_OFFOLAD_TYPE_PKT_TYPE_SHIFT		0
 #define RQ_CQE_OFFOLAD_TYPE_PKT_TYPE_MASK		0xFFFU
+#define RQ_CQE_OFFOLAD_TYPE_VLAN_EN_SHIFT		21
+#define RQ_CQE_OFFOLAD_TYPE_VLAN_EN_MASK		0x1U
 
 #define RQ_CQE_OFFOLAD_TYPE_GET(val, member)		(((val) >> \
 				RQ_CQE_OFFOLAD_TYPE_##member##_SHIFT) & \
@@ -230,6 +232,19 @@
 #define HINIC_GET_RX_PKT_TYPE(offload_type)	\
 			RQ_CQE_OFFOLAD_TYPE_GET(offload_type, PKT_TYPE)
 
+#define HINIC_GET_RX_VLAN_OFFLOAD_EN(offload_type)	\
+			RQ_CQE_OFFOLAD_TYPE_GET(offload_type, VLAN_EN)
+
+#define RQ_CQE_SGE_VLAN_MASK				0xFFFFU
+#define RQ_CQE_SGE_VLAN_SHIFT				0
+
+#define RQ_CQE_SGE_GET(val, member)			(((val) >> \
+					RQ_CQE_SGE_##member##_SHIFT) & \
+					RQ_CQE_SGE_##member##_MASK)
+
+#define HINIC_GET_RX_VLAN_TAG(vlan_len)	\
+		RQ_CQE_SGE_GET(vlan_len, VLAN)
+
 #define HINIC_RSS_TYPE_VALID_SHIFT			23
 #define HINIC_RSS_TYPE_TCP_IPV6_EXT_SHIFT		24
 #define HINIC_RSS_TYPE_IPV6_EXT_SHIFT			25
diff --git a/drivers/net/ethernet/huawei/hinic/hinic_main.c b/drivers/net/ethernet/huawei/hinic/hinic_main.c
index 1b917543feac..0eb83d529eac 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_main.c
+++ b/drivers/net/ethernet/huawei/hinic/hinic_main.c
@@ -830,14 +830,14 @@ static const struct net_device_ops hinic_netdev_ops = {
 	.ndo_get_stats64 = hinic_get_stats64,
 	.ndo_fix_features = hinic_fix_features,
 	.ndo_set_features = hinic_set_features,
-
 };
 
 static void netdev_features_init(struct net_device *netdev)
 {
 	netdev->hw_features = NETIF_F_SG | NETIF_F_HIGHDMA | NETIF_F_IP_CSUM |
 			      NETIF_F_IPV6_CSUM | NETIF_F_TSO | NETIF_F_TSO6 |
-			      NETIF_F_RXCSUM | NETIF_F_LRO;
+			      NETIF_F_RXCSUM | NETIF_F_LRO |
+			      NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_HW_VLAN_CTAG_RX;
 
 	netdev->vlan_features = netdev->hw_features;
 
@@ -917,6 +917,11 @@ static int set_features(struct hinic_dev *nic_dev,
 					     HINIC_LRO_MAX_WQE_NUM_DEFAULT);
 	}
 
+	if (changed & NETIF_F_HW_VLAN_CTAG_RX)
+		err = hinic_set_rx_vlan_offload(nic_dev,
+						!!(features &
+						   NETIF_F_HW_VLAN_CTAG_RX));
+
 	return err;
 }
 
diff --git a/drivers/net/ethernet/huawei/hinic/hinic_port.c b/drivers/net/ethernet/huawei/hinic/hinic_port.c
index c07adf793215..1bbeb91be808 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_port.c
+++ b/drivers/net/ethernet/huawei/hinic/hinic_port.c
@@ -431,6 +431,36 @@ int hinic_set_rx_csum_offload(struct hinic_dev *nic_dev, u32 en)
 	return 0;
 }
 
+int hinic_set_rx_vlan_offload(struct hinic_dev *nic_dev, u8 en)
+{
+	struct hinic_hwdev *hwdev = nic_dev->hwdev;
+	struct hinic_vlan_cfg vlan_cfg;
+	struct hinic_hwif *hwif;
+	struct pci_dev *pdev;
+	u16 out_size;
+	int err;
+
+	if (!hwdev)
+		return -EINVAL;
+
+	hwif = hwdev->hwif;
+	pdev = hwif->pdev;
+	vlan_cfg.func_id = HINIC_HWIF_FUNC_IDX(hwif);
+	vlan_cfg.vlan_rx_offload = en;
+
+	err = hinic_port_msg_cmd(hwdev, HINIC_PORT_CMD_SET_RX_VLAN_OFFLOAD,
+				 &vlan_cfg, sizeof(vlan_cfg),
+				 &vlan_cfg, &out_size);
+	if (err || !out_size || vlan_cfg.status) {
+		dev_err(&pdev->dev,
+			"Failed to set rx vlan offload, err: %d, status: 0x%x, out size: 0x%x\n",
+			err, vlan_cfg.status, out_size);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 int hinic_set_max_qnum(struct hinic_dev *nic_dev, u8 num_rqs)
 {
 	struct hinic_hwdev *hwdev = nic_dev->hwdev;
diff --git a/drivers/net/ethernet/huawei/hinic/hinic_port.h b/drivers/net/ethernet/huawei/hinic/hinic_port.h
index 16140a13000b..1bc47c7a5c00 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_port.h
+++ b/drivers/net/ethernet/huawei/hinic/hinic_port.h
@@ -223,6 +223,16 @@ struct hinic_lro_timer {
 	u32	timer;
 };
 
+struct hinic_vlan_cfg {
+	u8      status;
+	u8      version;
+	u8      rsvd0[6];
+
+	u16     func_id;
+	u8      vlan_rx_offload;
+	u8      rsvd1[5];
+};
+
 struct hinic_rss_template_mgmt {
 	u8	status;
 	u8	version;
@@ -558,4 +568,7 @@ int hinic_get_phy_port_stats(struct hinic_dev *nic_dev,
 
 int hinic_get_vport_stats(struct hinic_dev *nic_dev,
 			  struct hinic_vport_stats *stats);
+
+int hinic_set_rx_vlan_offload(struct hinic_dev *nic_dev, u8 en);
+
 #endif
diff --git a/drivers/net/ethernet/huawei/hinic/hinic_rx.c b/drivers/net/ethernet/huawei/hinic/hinic_rx.c
index 609ad4333cdd..56ea6d692f1c 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_rx.c
+++ b/drivers/net/ethernet/huawei/hinic/hinic_rx.c
@@ -18,6 +18,7 @@
 #include <linux/dma-mapping.h>
 #include <linux/prefetch.h>
 #include <linux/cpumask.h>
+#include <linux/if_vlan.h>
 #include <asm/barrier.h>
 
 #include "hinic_common.h"
@@ -325,6 +326,7 @@ static int rx_recv_jumbo_pkt(struct hinic_rxq *rxq, struct sk_buff *head_skb,
 static int rxq_recv(struct hinic_rxq *rxq, int budget)
 {
 	struct hinic_qp *qp = container_of(rxq->rq, struct hinic_qp, rq);
+	struct net_device *netdev = rxq->netdev;
 	u64 pkt_len = 0, rx_bytes = 0;
 	struct hinic_rq *rq = rxq->rq;
 	struct hinic_rq_wqe *rq_wqe;
@@ -334,8 +336,11 @@ static int rxq_recv(struct hinic_rxq *rxq, int budget)
 	struct hinic_sge sge;
 	unsigned int status;
 	struct sk_buff *skb;
+	u32 offload_type;
 	u16 ci, num_lro;
 	u16 num_wqe = 0;
+	u32 vlan_len;
+	u16 vid;
 
 	while (pkts < budget) {
 		num_wqes = 0;
@@ -368,6 +373,14 @@ static int rxq_recv(struct hinic_rxq *rxq, int budget)
 		hinic_rq_put_wqe(rq, ci,
 				 (num_wqes + 1) * HINIC_RQ_WQE_SIZE);
 
+		offload_type = be32_to_cpu(cqe->offload_type);
+		vlan_len = be32_to_cpu(cqe->len);
+		if ((netdev->features & NETIF_F_HW_VLAN_CTAG_RX) &&
+		    HINIC_GET_RX_VLAN_OFFLOAD_EN(offload_type)) {
+			vid = HINIC_GET_RX_VLAN_TAG(vlan_len);
+			__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), vid);
+		}
+
 		skb_record_rx_queue(skb, qp->q_id);
 		skb->protocol = eth_type_trans(skb, rxq->netdev);
 
diff --git a/drivers/net/ethernet/huawei/hinic/hinic_tx.c b/drivers/net/ethernet/huawei/hinic/hinic_tx.c
index f4f76370cd65..9c78251f9c39 100644
--- a/drivers/net/ethernet/huawei/hinic/hinic_tx.c
+++ b/drivers/net/ethernet/huawei/hinic/hinic_tx.c
@@ -407,10 +407,20 @@ static int offload_csum(struct hinic_sq_task *task, u32 *queue_info,
 	return 1;
 }
 
+static void offload_vlan(struct hinic_sq_task *task, u32 *queue_info,
+			 u16 vlan_tag, u16 vlan_pri)
+{
+	task->pkt_info0 |= HINIC_SQ_TASK_INFO0_SET(vlan_tag, VLAN_TAG) |
+				HINIC_SQ_TASK_INFO0_SET(1U, VLAN_OFFLOAD);
+
+	*queue_info |= HINIC_SQ_CTRL_SET(vlan_pri, QUEUE_INFO_PRI);
+}
+
 static int hinic_tx_offload(struct sk_buff *skb, struct hinic_sq_task *task,
 			    u32 *queue_info)
 {
 	enum hinic_offload_type offload = 0;
+	u16 vlan_tag;
 	int enabled;
 
 	enabled = offload_tso(task, queue_info, skb);
@@ -424,6 +434,13 @@ static int hinic_tx_offload(struct sk_buff *skb, struct hinic_sq_task *task,
 		return -EPROTONOSUPPORT;
 	}
 
+	if (unlikely(skb_vlan_tag_present(skb))) {
+		vlan_tag = skb_vlan_tag_get(skb);
+		offload_vlan(task, queue_info, vlan_tag,
+			     vlan_tag >> VLAN_PRIO_SHIFT);
+		offload |= TX_OFFLOAD_VLAN;
+	}
+
 	if (offload)
 		hinic_task_set_l2hdr(task, skb_network_offset(skb));
 
-- 
2.17.1


^ permalink raw reply related

* Re: [PATCH v3 bpf-next 2/4] libbpf: auto-set PERF_EVENT_ARRAY size to number of CPUs
From: Song Liu @ 2019-06-29  8:55 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Andrii Nakryiko, Alexei Starovoitov, Daniel Borkmann, bpf,
	Networking, Kernel Team, Song Liu
In-Reply-To: <20190629055309.1594755-3-andriin@fb.com>

On Fri, Jun 28, 2019 at 10:55 PM Andrii Nakryiko <andriin@fb.com> wrote:
>
> For BPF_MAP_TYPE_PERF_EVENT_ARRAY typically correct size is number of
> possible CPUs. This is impossible to specify at compilation time. This
> change adds automatic setting of PERF_EVENT_ARRAY size to number of
> system CPUs, unless non-zero size is specified explicitly. This allows
> to adjust size for advanced specific cases, while providing convenient
> and logical defaults.
>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>

Acked-by: Song Liu <songliubraving@fb.com>

^ permalink raw reply

* Re: [PATCH] rt2x00: fix rx queue hang
From: Stanislaw Gruszka @ 2019-06-29  8:50 UTC (permalink / raw)
  To: Soeren Moch
  Cc: Helmut Schaa, Kalle Valo, David S. Miller, linux-wireless, netdev,
	linux-kernel, stable
In-Reply-To: <8d7da251-8218-ff4b-2cf3-8ed69c97275e@web.de>

Hello

On Wed, Jun 26, 2019 at 03:28:00PM +0200, Soeren Moch wrote:
> Hi Stanislaw,
> 
> the good news is: your patch below also solves the issue for me. But
> removing the ENTRY_DATA_STATUS_PENDING check in
> rt2x00usb_kick_rx_entry() alone does not help, while removing this check
> in rt2x00usb_work_rxdone() alone does the trick.
> 
> So the real race seems to be that the flags set in the completion
> handler are not yet visible on the cpu core running the workqueue. And
> because the worker is not rescheduled when aborted, the entry can just
> wait forever.
> Do you think this could make sense?

Yes.

> > I'm somewhat reluctant to change the order, because TX processing
> > might relay on it (we first mark we wait for TX status and
> > then mark entry is no longer owned by hardware).
> OK, maybe it's just good luck that changing the order solves the rx
> problem. Or can memory barriers associated with the spinlock in
> rt2x00lib_dmadone() be responsible for that?
> (I'm testing on a armv7 system, Cortex-A9 quadcore.)

I'm not sure, rt2x00queue_index_inc() also disable/enable interrupts,
so maybe that make race not reproducible. 

> While looking at it, why we double-clear ENTRY_OWNER_DEVICE_DATA in
> rt2x00usb_interrupt_rxdone() directly and in rt2x00lib_dmadone() again,

rt2x00lib_dmadone() is called also on other palaces (error paths)
when we have to clear flags.

> while not doing the same for tx? 

If I remember correctly we have some races on rx (not happened on tx)
that was solved by using test_and_clear_bit(ENTRY_OWNER_DEVICE_DATA).

> Would it make more sense to possibly
> set ENTRY_DATA_IO_FAILED before clearing ENTRY_OWNER_DEVICE_DATA in
> rt2x00usb_interrupt_rxdone() as for tx?

I don't think so, ENTRY_DATA_IO_FAILED should be only set on error
case.

> >  However on RX
> > side ENTRY_DATA_STATUS_PENDING bit make no sense as we do not
> > wait for status. We should remove ENTRY_DATA_STATUS_PENDING on
> > RX side and perhaps this also will solve issue you observe.
> I agree that removing the unnecessary checks is a good idea in any case.
> > Could you please check below patch, if it fixes the problem as well?
> At least I could not trigger the problem within transferring 10GB of
> data. But maybe the probability for triggering this bug is just lower
> because ENTRY_OWNER_DEVICE_DATA is cleared some time before
> ENTRY_DATA_STATUS_PENDING is set?

Not sure. Anyway, could you post patch removing not needed checks
with proper description/changelog ?

Stanislaw

^ permalink raw reply

* Re: [PATCH v4 bpf-next 4/9] libbpf: add kprobe/uprobe attach API
From: Song Liu @ 2019-06-29  8:50 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Andrii Nakryiko, bpf, Networking, Alexei Starovoitov,
	Daniel Borkmann, Kernel Team, Stanislav Fomichev, Song Liu
In-Reply-To: <20190629034906.1209916-5-andriin@fb.com>

On Fri, Jun 28, 2019 at 8:49 PM Andrii Nakryiko <andriin@fb.com> wrote:
>
> Add ability to attach to kernel and user probes and retprobes.
> Implementation depends on perf event support for kprobes/uprobes.
>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>

Acked-by: Song Liu <songliubraving@fb.com>

^ permalink raw reply

* Re: [PATCH v4 bpf-next 3/9] libbpf: add ability to attach/detach BPF program to perf event
From: Song Liu @ 2019-06-29  8:48 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Andrii Nakryiko, bpf, Networking, Alexei Starovoitov,
	Daniel Borkmann, Kernel Team, Stanislav Fomichev, Song Liu
In-Reply-To: <20190629034906.1209916-4-andriin@fb.com>

On Fri, Jun 28, 2019 at 8:49 PM Andrii Nakryiko <andriin@fb.com> wrote:
>
> bpf_program__attach_perf_event allows to attach BPF program to existing
> perf event hook, providing most generic and most low-level way to attach BPF
> programs. It returns struct bpf_link, which should be passed to
> bpf_link__destroy to detach and free resources, associated with a link.
>
> Signed-off-by: Andrii Nakryiko <andriin@fb.com>

Acked-by: Song Liu <songliubraving@fb.com>

With one nit below.

> ---
>  tools/lib/bpf/libbpf.c   | 61 ++++++++++++++++++++++++++++++++++++++++
>  tools/lib/bpf/libbpf.h   |  3 ++
>  tools/lib/bpf/libbpf.map |  1 +
>  3 files changed, 65 insertions(+)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 455795e6f8af..98c155ec3bfa 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -32,6 +32,7 @@
>  #include <linux/limits.h>
>  #include <linux/perf_event.h>
>  #include <linux/ring_buffer.h>
> +#include <sys/ioctl.h>
>  #include <sys/stat.h>
>  #include <sys/types.h>
>  #include <sys/vfs.h>
> @@ -3958,6 +3959,66 @@ int bpf_link__destroy(struct bpf_link *link)
>         return err;
>  }
>
> +struct bpf_link_fd {
> +       struct bpf_link link; /* has to be at the top of struct */
> +       int fd; /* hook FD */
> +};
> +
> +static int bpf_link__destroy_perf_event(struct bpf_link *link)
> +{
> +       struct bpf_link_fd *l = (void *)link;
> +       int err;
> +
> +       if (l->fd < 0)
> +               return 0;
> +
> +       err = ioctl(l->fd, PERF_EVENT_IOC_DISABLE, 0);
> +       if (err)
> +               err = -errno;
> +
> +       close(l->fd);
> +       return err;
> +}
> +
> +struct bpf_link *bpf_program__attach_perf_event(struct bpf_program *prog,
> +                                               int pfd)
> +{
> +       char errmsg[STRERR_BUFSIZE];
> +       struct bpf_link_fd *link;
> +       int prog_fd, err;
> +
> +       prog_fd = bpf_program__fd(prog);
> +       if (prog_fd < 0) {
> +               pr_warning("program '%s': can't attach before loaded\n",
> +                          bpf_program__title(prog, false));

This warning message is not very easy to follow.

> +               return ERR_PTR(-EINVAL);
> +       }
> +
> +       link = malloc(sizeof(*link));
> +       if (!link)
> +               return ERR_PTR(-ENOMEM);
> +       link->link.destroy = &bpf_link__destroy_perf_event;
> +       link->fd = pfd;
> +
> +       if (ioctl(pfd, PERF_EVENT_IOC_SET_BPF, prog_fd) < 0) {
> +               err = -errno;
> +               free(link);
> +               pr_warning("program '%s': failed to attach to pfd %d: %s\n",
> +                          bpf_program__title(prog, false), pfd,
> +                          libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
> +               return ERR_PTR(err);
> +       }
> +       if (ioctl(pfd, PERF_EVENT_IOC_ENABLE, 0) < 0) {
> +               err = -errno;
> +               free(link);
> +               pr_warning("program '%s': failed to enable pfd %d: %s\n",
> +                          bpf_program__title(prog, false), pfd,
> +                          libbpf_strerror_r(err, errmsg, sizeof(errmsg)));
> +               return ERR_PTR(err);
> +       }
> +       return (struct bpf_link *)link;
> +}
> +
>  enum bpf_perf_event_ret
>  bpf_perf_event_read_simple(void *mmap_mem, size_t mmap_size, size_t page_size,
>                            void **copy_mem, size_t *copy_size,
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index 5082a5ebb0c2..1bf66c4a9330 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -169,6 +169,9 @@ struct bpf_link;
>
>  LIBBPF_API int bpf_link__destroy(struct bpf_link *link);
>
> +LIBBPF_API struct bpf_link *
> +bpf_program__attach_perf_event(struct bpf_program *prog, int pfd);
> +
>  struct bpf_insn;
>
>  /*
> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> index 3cde850fc8da..756f5aa802e9 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map
> @@ -169,6 +169,7 @@ LIBBPF_0.0.4 {
>         global:
>                 bpf_link__destroy;
>                 bpf_object__load_xattr;
> +               bpf_program__attach_perf_event;
>                 btf_dump__dump_type;
>                 btf_dump__free;
>                 btf_dump__new;
> --
> 2.17.1
>

^ permalink raw reply

* Re: net: check before dereferencing netdev_ops during busy poll
From: Greg Kroah-Hartman @ 2019-06-29  7:45 UTC (permalink / raw)
  To: Josh Elsasser
  Cc: Sasha Levin, Matteo Croce, stable, netdev, LKML, David Miller
In-Reply-To: <1560226F-F2C0-440D-9C58-D664DE3C7322@appneta.com>

On Fri, Jun 28, 2019 at 07:03:01PM -0700, Josh Elsasser wrote:
> On Jun 28, 2019, at 3:55 PM, Sasha Levin <sashal@kernel.org> wrote:
> 
> > What's the upstream commit id?
> 
> The commit wasn't needed upstream, as I only sent the original patch after
> 79e7fff47b7b ("net: remove support for per driver ndo_busy_poll()") had
> made the fix unnecessary in Linus' tree.
> 
> May've gotten lost in the shuffle due to my poor Fixes tags. The patch in
> question applied only on top of the 4.9 stable release at the time, but the
> actual NPE had been around in some form since 3.11 / 0602129286705 ("net: add
> low latency socket poll").

Ok, can people then resend this and be very explicit as to why this is
needed only in a stable kernel tree and get reviews from people agreeing
that this really is the correct fix?

thanks,

greg k-h

^ permalink raw reply

* [RFC PATCH] gve: gve_get_channels() can be static
From: kbuild test robot @ 2019-06-29  6:40 UTC (permalink / raw)
  To: Catherine Sullivan
  Cc: kbuild-all, netdev, Catherine Sullivan, Sagi Shahar, Jon Olson,
	Willem de Bruijn, Luigi Rizzo
In-Reply-To: <20190626185251.205687-5-csully@google.com>


Fixes: ac0744578517 ("gve: Add ethtool support")
Signed-off-by: kbuild test robot <lkp@intel.com>
---
 gve_ethtool.c |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/google/gve/gve_ethtool.c b/drivers/net/ethernet/google/gve/gve_ethtool.c
index 8e6863c..036389c 100644
--- a/drivers/net/ethernet/google/gve/gve_ethtool.c
+++ b/drivers/net/ethernet/google/gve/gve_ethtool.c
@@ -144,7 +144,7 @@ gve_get_ethtool_stats(struct net_device *netdev,
 	}
 }
 
-void gve_get_channels(struct net_device *netdev, struct ethtool_channels *cmd)
+static void gve_get_channels(struct net_device *netdev, struct ethtool_channels *cmd)
 {
 	struct gve_priv *priv = netdev_priv(netdev);
 
@@ -158,7 +158,7 @@ void gve_get_channels(struct net_device *netdev, struct ethtool_channels *cmd)
 	cmd->combined_count = 0;
 }
 
-int gve_set_channels(struct net_device *netdev, struct ethtool_channels *cmd)
+static int gve_set_channels(struct net_device *netdev, struct ethtool_channels *cmd)
 {
 	struct gve_priv *priv = netdev_priv(netdev);
 	struct gve_queue_config new_tx_cfg = priv->tx_cfg;
@@ -188,8 +188,8 @@ int gve_set_channels(struct net_device *netdev, struct ethtool_channels *cmd)
 	return gve_adjust_queues(priv, new_rx_cfg, new_tx_cfg);
 }
 
-void gve_get_ringparam(struct net_device *netdev,
-		       struct ethtool_ringparam *cmd)
+static void gve_get_ringparam(struct net_device *netdev,
+			      struct ethtool_ringparam *cmd)
 {
 	struct gve_priv *priv = netdev_priv(netdev);
 
@@ -199,7 +199,7 @@ void gve_get_ringparam(struct net_device *netdev,
 	cmd->tx_pending = priv->tx_desc_cnt;
 }
 
-int gve_user_reset(struct net_device *netdev, u32 *flags)
+static int gve_user_reset(struct net_device *netdev, u32 *flags)
 {
 	struct gve_priv *priv = netdev_priv(netdev);
 

^ permalink raw reply related

* Re: [net-next 4/4] gve: Add ethtool support
From: kbuild test robot @ 2019-06-29  6:40 UTC (permalink / raw)
  To: Catherine Sullivan
  Cc: kbuild-all, netdev, Catherine Sullivan, Sagi Shahar, Jon Olson,
	Willem de Bruijn, Luigi Rizzo
In-Reply-To: <20190626185251.205687-5-csully@google.com>

Hi Catherine,

I love your patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Catherine-Sullivan/Add-gve-driver/20190629-070444
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.1-rc1-7-g2b96cd8-dirty
        make ARCH=x86_64 allmodconfig
        make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)

>> drivers/net/ethernet/google/gve/gve_ethtool.c:147:6: sparse: sparse: symbol 'gve_get_channels' was not declared. Should it be static?
>> drivers/net/ethernet/google/gve/gve_ethtool.c:161:5: sparse: sparse: symbol 'gve_set_channels' was not declared. Should it be static?
>> drivers/net/ethernet/google/gve/gve_ethtool.c:191:6: sparse: sparse: symbol 'gve_get_ringparam' was not declared. Should it be static?
>> drivers/net/ethernet/google/gve/gve_ethtool.c:202:5: sparse: sparse: symbol 'gve_user_reset' was not declared. Should it be static?

Please review and possibly fold the followup patch.

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

^ permalink raw reply

* [PATCH bpf 3/3] selftests: bpf: add tests for shifts by zero
From: Luke Nelson @ 2019-06-29  5:57 UTC (permalink / raw)
  To: linux-kernel
  Cc: Luke Nelson, Xi Wang, David S. Miller, Alexey Kuznetsov,
	Hideaki YOSHIFUJI, Wang YanQing, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, H. Peter Anvin, x86, Alexei Starovoitov,
	Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song,
	Shuah Khan, Jiong Wang, Jakub Kicinski, Björn Töpel,
	netdev, bpf, linux-kselftest
In-Reply-To: <20190629055759.28365-1-luke.r.nels@gmail.com>

There are currently no tests for ALU64 shift operations when the shift
amount is 0. This adds 6 new tests to make sure they are equivalent
to a no-op. The x32 JIT had such bugs that could have been caught by
these tests.

Cc: Xi Wang <xi.wang@gmail.com>
Signed-off-by: Luke Nelson <luke.r.nels@gmail.com>
---
 .../selftests/bpf/verifier/basic_instr.c      | 85 +++++++++++++++++++
 1 file changed, 85 insertions(+)

diff --git a/tools/testing/selftests/bpf/verifier/basic_instr.c b/tools/testing/selftests/bpf/verifier/basic_instr.c
index ed91a7b9a456..071dbc889e8c 100644
--- a/tools/testing/selftests/bpf/verifier/basic_instr.c
+++ b/tools/testing/selftests/bpf/verifier/basic_instr.c
@@ -90,6 +90,91 @@
 	},
 	.result = ACCEPT,
 },
+{
+	"lsh64 by 0 imm",
+	.insns = {
+	BPF_LD_IMM64(BPF_REG_0, 1),
+	BPF_LD_IMM64(BPF_REG_1, 1),
+	BPF_ALU64_IMM(BPF_LSH, BPF_REG_1, 0),
+	BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, 1, 1),
+	BPF_MOV64_IMM(BPF_REG_0, 2),
+	BPF_EXIT_INSN(),
+	},
+	.result = ACCEPT,
+	.retval = 1,
+},
+{
+	"rsh64 by 0 imm",
+	.insns = {
+	BPF_LD_IMM64(BPF_REG_0, 1),
+	BPF_LD_IMM64(BPF_REG_1, 0x100000000LL),
+	BPF_ALU64_REG(BPF_MOV, BPF_REG_2, BPF_REG_1),
+	BPF_ALU64_IMM(BPF_RSH, BPF_REG_1, 0),
+	BPF_JMP_REG(BPF_JEQ, BPF_REG_1, BPF_REG_2, 1),
+	BPF_MOV64_IMM(BPF_REG_0, 2),
+	BPF_EXIT_INSN(),
+	},
+	.result = ACCEPT,
+	.retval = 1,
+},
+{
+	"arsh64 by 0 imm",
+	.insns = {
+	BPF_LD_IMM64(BPF_REG_0, 1),
+	BPF_LD_IMM64(BPF_REG_1, 0x100000000LL),
+	BPF_ALU64_REG(BPF_MOV, BPF_REG_2, BPF_REG_1),
+	BPF_ALU64_IMM(BPF_ARSH, BPF_REG_1, 0),
+	BPF_JMP_REG(BPF_JEQ, BPF_REG_1, BPF_REG_2, 1),
+	BPF_MOV64_IMM(BPF_REG_0, 2),
+	BPF_EXIT_INSN(),
+	},
+	.result = ACCEPT,
+	.retval = 1,
+},
+{
+	"lsh64 by 0 reg",
+	.insns = {
+	BPF_LD_IMM64(BPF_REG_0, 1),
+	BPF_LD_IMM64(BPF_REG_1, 1),
+	BPF_LD_IMM64(BPF_REG_2, 0),
+	BPF_ALU64_REG(BPF_LSH, BPF_REG_1, BPF_REG_2),
+	BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, 1, 1),
+	BPF_MOV64_IMM(BPF_REG_0, 2),
+	BPF_EXIT_INSN(),
+	},
+	.result = ACCEPT,
+	.retval = 1,
+},
+{
+	"rsh64 by 0 reg",
+	.insns = {
+	BPF_LD_IMM64(BPF_REG_0, 1),
+	BPF_LD_IMM64(BPF_REG_1, 0x100000000LL),
+	BPF_ALU64_REG(BPF_MOV, BPF_REG_2, BPF_REG_1),
+	BPF_LD_IMM64(BPF_REG_3, 0),
+	BPF_ALU64_REG(BPF_RSH, BPF_REG_1, BPF_REG_3),
+	BPF_JMP_REG(BPF_JEQ, BPF_REG_1, BPF_REG_2, 1),
+	BPF_MOV64_IMM(BPF_REG_0, 2),
+	BPF_EXIT_INSN(),
+	},
+	.result = ACCEPT,
+	.retval = 1,
+},
+{
+	"arsh64 by 0 reg",
+	.insns = {
+	BPF_LD_IMM64(BPF_REG_0, 1),
+	BPF_LD_IMM64(BPF_REG_1, 0x100000000LL),
+	BPF_ALU64_REG(BPF_MOV, BPF_REG_2, BPF_REG_1),
+	BPF_LD_IMM64(BPF_REG_3, 0),
+	BPF_ALU64_REG(BPF_ARSH, BPF_REG_1, BPF_REG_3),
+	BPF_JMP_REG(BPF_JEQ, BPF_REG_1, BPF_REG_2, 1),
+	BPF_MOV64_IMM(BPF_REG_0, 2),
+	BPF_EXIT_INSN(),
+	},
+	.result = ACCEPT,
+	.retval = 1,
+},
 {
 	"invalid 64-bit BPF_END",
 	.insns = {
-- 
2.20.1


^ permalink raw reply related

* [PATCH bpf 2/3] bpf, x32: Fix bug with ALU64 {LSH,RSH,ARSH} BPF_K shift by 0
From: Luke Nelson @ 2019-06-29  5:57 UTC (permalink / raw)
  To: linux-kernel
  Cc: Luke Nelson, Xi Wang, David S. Miller, Alexey Kuznetsov,
	Hideaki YOSHIFUJI, Wang YanQing, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, H. Peter Anvin, x86, Alexei Starovoitov,
	Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song,
	Shuah Khan, Jiong Wang, Jakub Kicinski, Björn Töpel,
	netdev, bpf, linux-kselftest
In-Reply-To: <20190629055759.28365-1-luke.r.nels@gmail.com>

The current x32 BPF JIT does not correctly compile shift operations when
the immediate shift amount is 0. The expected behavior is for this to
be a no-op.

The following program demonstrates the bug. The expexceted result is 1,
but the current JITed code returns 2.

  r0 = 1
  r1 = 1
  r1 <<= 0
  if r1 == 1 goto end
  r0 = 2
end:
  exit

This patch simplifies the code and fixes the bug.

Fixes: 03f5781be2c7 ("bpf, x86_32: add eBPF JIT compiler for ia32")
Co-developed-by: Xi Wang <xi.wang@gmail.com>
Signed-off-by: Xi Wang <xi.wang@gmail.com>
Signed-off-by: Luke Nelson <luke.r.nels@gmail.com>
---
 arch/x86/net/bpf_jit_comp32.c | 63 ++++-------------------------------
 1 file changed, 6 insertions(+), 57 deletions(-)

diff --git a/arch/x86/net/bpf_jit_comp32.c b/arch/x86/net/bpf_jit_comp32.c
index f34ef513f4f9..1d12d2174085 100644
--- a/arch/x86/net/bpf_jit_comp32.c
+++ b/arch/x86/net/bpf_jit_comp32.c
@@ -894,27 +894,10 @@ static inline void emit_ia32_lsh_i64(const u8 dst[], const u32 val,
 	}
 	/* Do LSH operation */
 	if (val < 32) {
-		/* shl dreg_hi,imm8 */
-		EMIT3(0xC1, add_1reg(0xE0, dreg_hi), val);
-		/* mov ebx,dreg_lo */
-		EMIT2(0x8B, add_2reg(0xC0, dreg_lo, IA32_EBX));
+		/* shld dreg_hi,dreg_lo,imm8 */
+		EMIT4(0x0F, 0xA4, add_2reg(0xC0, dreg_hi, dreg_lo), val);
 		/* shl dreg_lo,imm8 */
 		EMIT3(0xC1, add_1reg(0xE0, dreg_lo), val);
-
-		/* IA32_ECX = 32 - val */
-		/* mov ecx,val */
-		EMIT2(0xB1, val);
-		/* movzx ecx,ecx */
-		EMIT3(0x0F, 0xB6, add_2reg(0xC0, IA32_ECX, IA32_ECX));
-		/* neg ecx */
-		EMIT2(0xF7, add_1reg(0xD8, IA32_ECX));
-		/* add ecx,32 */
-		EMIT3(0x83, add_1reg(0xC0, IA32_ECX), 32);
-
-		/* shr ebx,cl */
-		EMIT2(0xD3, add_1reg(0xE8, IA32_EBX));
-		/* or dreg_hi,ebx */
-		EMIT2(0x09, add_2reg(0xC0, dreg_hi, IA32_EBX));
 	} else if (val >= 32 && val < 64) {
 		u32 value = val - 32;
 
@@ -960,27 +943,10 @@ static inline void emit_ia32_rsh_i64(const u8 dst[], const u32 val,
 
 	/* Do RSH operation */
 	if (val < 32) {
-		/* shr dreg_lo,imm8 */
-		EMIT3(0xC1, add_1reg(0xE8, dreg_lo), val);
-		/* mov ebx,dreg_hi */
-		EMIT2(0x8B, add_2reg(0xC0, dreg_hi, IA32_EBX));
+		/* shrd dreg_lo,dreg_hi,imm8 */
+		EMIT4(0x0F, 0xAC, add_2reg(0xC0, dreg_lo, dreg_hi), val);
 		/* shr dreg_hi,imm8 */
 		EMIT3(0xC1, add_1reg(0xE8, dreg_hi), val);
-
-		/* IA32_ECX = 32 - val */
-		/* mov ecx,val */
-		EMIT2(0xB1, val);
-		/* movzx ecx,ecx */
-		EMIT3(0x0F, 0xB6, add_2reg(0xC0, IA32_ECX, IA32_ECX));
-		/* neg ecx */
-		EMIT2(0xF7, add_1reg(0xD8, IA32_ECX));
-		/* add ecx,32 */
-		EMIT3(0x83, add_1reg(0xC0, IA32_ECX), 32);
-
-		/* shl ebx,cl */
-		EMIT2(0xD3, add_1reg(0xE0, IA32_EBX));
-		/* or dreg_lo,ebx */
-		EMIT2(0x09, add_2reg(0xC0, dreg_lo, IA32_EBX));
 	} else if (val >= 32 && val < 64) {
 		u32 value = val - 32;
 
@@ -1025,27 +991,10 @@ static inline void emit_ia32_arsh_i64(const u8 dst[], const u32 val,
 	}
 	/* Do RSH operation */
 	if (val < 32) {
-		/* shr dreg_lo,imm8 */
-		EMIT3(0xC1, add_1reg(0xE8, dreg_lo), val);
-		/* mov ebx,dreg_hi */
-		EMIT2(0x8B, add_2reg(0xC0, dreg_hi, IA32_EBX));
+		/* shrd dreg_lo,dreg_hi,imm8 */
+		EMIT4(0x0F, 0xAC, add_2reg(0xC0, dreg_lo, dreg_hi), val);
 		/* ashr dreg_hi,imm8 */
 		EMIT3(0xC1, add_1reg(0xF8, dreg_hi), val);
-
-		/* IA32_ECX = 32 - val */
-		/* mov ecx,val */
-		EMIT2(0xB1, val);
-		/* movzx ecx,ecx */
-		EMIT3(0x0F, 0xB6, add_2reg(0xC0, IA32_ECX, IA32_ECX));
-		/* neg ecx */
-		EMIT2(0xF7, add_1reg(0xD8, IA32_ECX));
-		/* add ecx,32 */
-		EMIT3(0x83, add_1reg(0xC0, IA32_ECX), 32);
-
-		/* shl ebx,cl */
-		EMIT2(0xD3, add_1reg(0xE0, IA32_EBX));
-		/* or dreg_lo,ebx */
-		EMIT2(0x09, add_2reg(0xC0, dreg_lo, IA32_EBX));
 	} else if (val >= 32 && val < 64) {
 		u32 value = val - 32;
 
-- 
2.20.1


^ permalink raw reply related

* [PATCH bpf 1/3] bpf, x32: Fix bug with ALU64 {LSH,RSH,ARSH} BPF_X shift by 0
From: Luke Nelson @ 2019-06-29  5:57 UTC (permalink / raw)
  To: linux-kernel
  Cc: Luke Nelson, Xi Wang, Wang YanQing, David S. Miller,
	Alexey Kuznetsov, Hideaki YOSHIFUJI, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, H. Peter Anvin, x86, Alexei Starovoitov,
	Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song,
	Shuah Khan, Jakub Kicinski, Jiong Wang, Björn Töpel,
	netdev, bpf, linux-kselftest

The current x32 BPF JIT for shift operations is not correct when the
shift amount in a register is 0. The expected behavior is a no-op, whereas
the current implementation changes bits in the destination register.

The following example demonstrates the bug. The expected result of this
program is 1, but the current JITed code returns 2.

  r0 = 1
  r1 = 1
  r2 = 0
  r1 <<= r2
  if r1 == 1 goto end
  r0 = 2
end:
  exit

The bug is caused by an incorrect assumption by the JIT that a shift by
32 clear the register. On x32 however, shifts use the lower 5 bits of
the source, making a shift by 32 equivalent to a shift by 0.

This patch fixes the bug using double-precision shifts, which also
simplifies the code.

Fixes: 03f5781be2c7 ("bpf, x86_32: add eBPF JIT compiler for ia32")
Co-developed-by: Xi Wang <xi.wang@gmail.com>
Signed-off-by: Xi Wang <xi.wang@gmail.com>
Signed-off-by: Luke Nelson <luke.r.nels@gmail.com>
---
 arch/x86/net/bpf_jit_comp32.c | 221 ++++------------------------------
 1 file changed, 23 insertions(+), 198 deletions(-)

diff --git a/arch/x86/net/bpf_jit_comp32.c b/arch/x86/net/bpf_jit_comp32.c
index b29e82f190c7..f34ef513f4f9 100644
--- a/arch/x86/net/bpf_jit_comp32.c
+++ b/arch/x86/net/bpf_jit_comp32.c
@@ -724,9 +724,6 @@ static inline void emit_ia32_lsh_r64(const u8 dst[], const u8 src[],
 {
 	u8 *prog = *pprog;
 	int cnt = 0;
-	static int jmp_label1 = -1;
-	static int jmp_label2 = -1;
-	static int jmp_label3 = -1;
 	u8 dreg_lo = dstk ? IA32_EAX : dst_lo;
 	u8 dreg_hi = dstk ? IA32_EDX : dst_hi;
 
@@ -745,79 +742,23 @@ static inline void emit_ia32_lsh_r64(const u8 dst[], const u8 src[],
 		/* mov ecx,src_lo */
 		EMIT2(0x8B, add_2reg(0xC0, src_lo, IA32_ECX));
 
-	/* cmp ecx,32 */
-	EMIT3(0x83, add_1reg(0xF8, IA32_ECX), 32);
-	/* Jumps when >= 32 */
-	if (is_imm8(jmp_label(jmp_label1, 2)))
-		EMIT2(IA32_JAE, jmp_label(jmp_label1, 2));
-	else
-		EMIT2_off32(0x0F, IA32_JAE + 0x10, jmp_label(jmp_label1, 6));
-
-	/* < 32 */
-	/* shl dreg_hi,cl */
-	EMIT2(0xD3, add_1reg(0xE0, dreg_hi));
-	/* mov ebx,dreg_lo */
-	EMIT2(0x8B, add_2reg(0xC0, dreg_lo, IA32_EBX));
+	/* shld dreg_hi,dreg_lo,cl */
+	EMIT3(0x0F, 0xA5, add_2reg(0xC0, dreg_hi, dreg_lo));
 	/* shl dreg_lo,cl */
 	EMIT2(0xD3, add_1reg(0xE0, dreg_lo));
 
-	/* IA32_ECX = -IA32_ECX + 32 */
-	/* neg ecx */
-	EMIT2(0xF7, add_1reg(0xD8, IA32_ECX));
-	/* add ecx,32 */
-	EMIT3(0x83, add_1reg(0xC0, IA32_ECX), 32);
-
-	/* shr ebx,cl */
-	EMIT2(0xD3, add_1reg(0xE8, IA32_EBX));
-	/* or dreg_hi,ebx */
-	EMIT2(0x09, add_2reg(0xC0, dreg_hi, IA32_EBX));
-
-	/* goto out; */
-	if (is_imm8(jmp_label(jmp_label3, 2)))
-		EMIT2(0xEB, jmp_label(jmp_label3, 2));
-	else
-		EMIT1_off32(0xE9, jmp_label(jmp_label3, 5));
-
-	/* >= 32 */
-	if (jmp_label1 == -1)
-		jmp_label1 = cnt;
+	/* if ecx >= 32, mov dreg_lo into dreg_hi and clear dreg_lo */
 
-	/* cmp ecx,64 */
-	EMIT3(0x83, add_1reg(0xF8, IA32_ECX), 64);
-	/* Jumps when >= 64 */
-	if (is_imm8(jmp_label(jmp_label2, 2)))
-		EMIT2(IA32_JAE, jmp_label(jmp_label2, 2));
-	else
-		EMIT2_off32(0x0F, IA32_JAE + 0x10, jmp_label(jmp_label2, 6));
+	/* cmp ecx,32 */
+	EMIT3(0x83, add_1reg(0xF8, IA32_ECX), 32);
+	/* skip the next two instructions (4 bytes) when < 32 */
+	EMIT2(IA32_JB, 4);
 
-	/* >= 32 && < 64 */
-	/* sub ecx,32 */
-	EMIT3(0x83, add_1reg(0xE8, IA32_ECX), 32);
-	/* shl dreg_lo,cl */
-	EMIT2(0xD3, add_1reg(0xE0, dreg_lo));
 	/* mov dreg_hi,dreg_lo */
 	EMIT2(0x89, add_2reg(0xC0, dreg_hi, dreg_lo));
-
 	/* xor dreg_lo,dreg_lo */
 	EMIT2(0x33, add_2reg(0xC0, dreg_lo, dreg_lo));
 
-	/* goto out; */
-	if (is_imm8(jmp_label(jmp_label3, 2)))
-		EMIT2(0xEB, jmp_label(jmp_label3, 2));
-	else
-		EMIT1_off32(0xE9, jmp_label(jmp_label3, 5));
-
-	/* >= 64 */
-	if (jmp_label2 == -1)
-		jmp_label2 = cnt;
-	/* xor dreg_lo,dreg_lo */
-	EMIT2(0x33, add_2reg(0xC0, dreg_lo, dreg_lo));
-	/* xor dreg_hi,dreg_hi */
-	EMIT2(0x33, add_2reg(0xC0, dreg_hi, dreg_hi));
-
-	if (jmp_label3 == -1)
-		jmp_label3 = cnt;
-
 	if (dstk) {
 		/* mov dword ptr [ebp+off],dreg_lo */
 		EMIT3(0x89, add_2reg(0x40, IA32_EBP, dreg_lo),
@@ -836,9 +777,6 @@ static inline void emit_ia32_arsh_r64(const u8 dst[], const u8 src[],
 {
 	u8 *prog = *pprog;
 	int cnt = 0;
-	static int jmp_label1 = -1;
-	static int jmp_label2 = -1;
-	static int jmp_label3 = -1;
 	u8 dreg_lo = dstk ? IA32_EAX : dst_lo;
 	u8 dreg_hi = dstk ? IA32_EDX : dst_hi;
 
@@ -857,78 +795,22 @@ static inline void emit_ia32_arsh_r64(const u8 dst[], const u8 src[],
 		/* mov ecx,src_lo */
 		EMIT2(0x8B, add_2reg(0xC0, src_lo, IA32_ECX));
 
-	/* cmp ecx,32 */
-	EMIT3(0x83, add_1reg(0xF8, IA32_ECX), 32);
-	/* Jumps when >= 32 */
-	if (is_imm8(jmp_label(jmp_label1, 2)))
-		EMIT2(IA32_JAE, jmp_label(jmp_label1, 2));
-	else
-		EMIT2_off32(0x0F, IA32_JAE + 0x10, jmp_label(jmp_label1, 6));
-
-	/* < 32 */
-	/* lshr dreg_lo,cl */
-	EMIT2(0xD3, add_1reg(0xE8, dreg_lo));
-	/* mov ebx,dreg_hi */
-	EMIT2(0x8B, add_2reg(0xC0, dreg_hi, IA32_EBX));
-	/* ashr dreg_hi,cl */
+	/* shrd dreg_lo,dreg_hi,cl */
+	EMIT3(0x0F, 0xAD, add_2reg(0xC0, dreg_lo, dreg_hi));
+	/* sar dreg_hi,cl */
 	EMIT2(0xD3, add_1reg(0xF8, dreg_hi));
 
-	/* IA32_ECX = -IA32_ECX + 32 */
-	/* neg ecx */
-	EMIT2(0xF7, add_1reg(0xD8, IA32_ECX));
-	/* add ecx,32 */
-	EMIT3(0x83, add_1reg(0xC0, IA32_ECX), 32);
-
-	/* shl ebx,cl */
-	EMIT2(0xD3, add_1reg(0xE0, IA32_EBX));
-	/* or dreg_lo,ebx */
-	EMIT2(0x09, add_2reg(0xC0, dreg_lo, IA32_EBX));
-
-	/* goto out; */
-	if (is_imm8(jmp_label(jmp_label3, 2)))
-		EMIT2(0xEB, jmp_label(jmp_label3, 2));
-	else
-		EMIT1_off32(0xE9, jmp_label(jmp_label3, 5));
-
-	/* >= 32 */
-	if (jmp_label1 == -1)
-		jmp_label1 = cnt;
+	/* if ecx >= 32, mov dreg_hi to dreg_lo and set/clear dreg_hi depending on sign */
 
-	/* cmp ecx,64 */
-	EMIT3(0x83, add_1reg(0xF8, IA32_ECX), 64);
-	/* Jumps when >= 64 */
-	if (is_imm8(jmp_label(jmp_label2, 2)))
-		EMIT2(IA32_JAE, jmp_label(jmp_label2, 2));
-	else
-		EMIT2_off32(0x0F, IA32_JAE + 0x10, jmp_label(jmp_label2, 6));
+	/* cmp ecx,32 */
+	EMIT3(0x83, add_1reg(0xF8, IA32_ECX), 32);
+	/* skip the next two instructions (5 bytes) when < 32 */
+	EMIT2(IA32_JB, 5);
 
-	/* >= 32 && < 64 */
-	/* sub ecx,32 */
-	EMIT3(0x83, add_1reg(0xE8, IA32_ECX), 32);
-	/* ashr dreg_hi,cl */
-	EMIT2(0xD3, add_1reg(0xF8, dreg_hi));
 	/* mov dreg_lo,dreg_hi */
 	EMIT2(0x89, add_2reg(0xC0, dreg_lo, dreg_hi));
-
-	/* ashr dreg_hi,imm8 */
-	EMIT3(0xC1, add_1reg(0xF8, dreg_hi), 31);
-
-	/* goto out; */
-	if (is_imm8(jmp_label(jmp_label3, 2)))
-		EMIT2(0xEB, jmp_label(jmp_label3, 2));
-	else
-		EMIT1_off32(0xE9, jmp_label(jmp_label3, 5));
-
-	/* >= 64 */
-	if (jmp_label2 == -1)
-		jmp_label2 = cnt;
-	/* ashr dreg_hi,imm8 */
+	/* sar dreg_hi,31 */
 	EMIT3(0xC1, add_1reg(0xF8, dreg_hi), 31);
-	/* mov dreg_lo,dreg_hi */
-	EMIT2(0x89, add_2reg(0xC0, dreg_lo, dreg_hi));
-
-	if (jmp_label3 == -1)
-		jmp_label3 = cnt;
 
 	if (dstk) {
 		/* mov dword ptr [ebp+off],dreg_lo */
@@ -948,9 +830,6 @@ static inline void emit_ia32_rsh_r64(const u8 dst[], const u8 src[], bool dstk,
 {
 	u8 *prog = *pprog;
 	int cnt = 0;
-	static int jmp_label1 = -1;
-	static int jmp_label2 = -1;
-	static int jmp_label3 = -1;
 	u8 dreg_lo = dstk ? IA32_EAX : dst_lo;
 	u8 dreg_hi = dstk ? IA32_EDX : dst_hi;
 
@@ -969,77 +848,23 @@ static inline void emit_ia32_rsh_r64(const u8 dst[], const u8 src[], bool dstk,
 		/* mov ecx,src_lo */
 		EMIT2(0x8B, add_2reg(0xC0, src_lo, IA32_ECX));
 
-	/* cmp ecx,32 */
-	EMIT3(0x83, add_1reg(0xF8, IA32_ECX), 32);
-	/* Jumps when >= 32 */
-	if (is_imm8(jmp_label(jmp_label1, 2)))
-		EMIT2(IA32_JAE, jmp_label(jmp_label1, 2));
-	else
-		EMIT2_off32(0x0F, IA32_JAE + 0x10, jmp_label(jmp_label1, 6));
-
-	/* < 32 */
-	/* lshr dreg_lo,cl */
-	EMIT2(0xD3, add_1reg(0xE8, dreg_lo));
-	/* mov ebx,dreg_hi */
-	EMIT2(0x8B, add_2reg(0xC0, dreg_hi, IA32_EBX));
+	/* shrd dreg_lo,dreg_hi,cl */
+	EMIT3(0x0F, 0xAD, add_2reg(0xC0, dreg_lo, dreg_hi));
 	/* shr dreg_hi,cl */
 	EMIT2(0xD3, add_1reg(0xE8, dreg_hi));
 
-	/* IA32_ECX = -IA32_ECX + 32 */
-	/* neg ecx */
-	EMIT2(0xF7, add_1reg(0xD8, IA32_ECX));
-	/* add ecx,32 */
-	EMIT3(0x83, add_1reg(0xC0, IA32_ECX), 32);
-
-	/* shl ebx,cl */
-	EMIT2(0xD3, add_1reg(0xE0, IA32_EBX));
-	/* or dreg_lo,ebx */
-	EMIT2(0x09, add_2reg(0xC0, dreg_lo, IA32_EBX));
+	/* if ecx >= 32, mov dreg_hi to dreg_lo and clear dreg_hi */
 
-	/* goto out; */
-	if (is_imm8(jmp_label(jmp_label3, 2)))
-		EMIT2(0xEB, jmp_label(jmp_label3, 2));
-	else
-		EMIT1_off32(0xE9, jmp_label(jmp_label3, 5));
-
-	/* >= 32 */
-	if (jmp_label1 == -1)
-		jmp_label1 = cnt;
-	/* cmp ecx,64 */
-	EMIT3(0x83, add_1reg(0xF8, IA32_ECX), 64);
-	/* Jumps when >= 64 */
-	if (is_imm8(jmp_label(jmp_label2, 2)))
-		EMIT2(IA32_JAE, jmp_label(jmp_label2, 2));
-	else
-		EMIT2_off32(0x0F, IA32_JAE + 0x10, jmp_label(jmp_label2, 6));
+	/* cmp ecx,32 */
+	EMIT3(0x83, add_1reg(0xF8, IA32_ECX), 32);
+	/* skip the next two instructions (4 bytes) when < 32 */
+	EMIT2(IA32_JB, 4);
 
-	/* >= 32 && < 64 */
-	/* sub ecx,32 */
-	EMIT3(0x83, add_1reg(0xE8, IA32_ECX), 32);
-	/* shr dreg_hi,cl */
-	EMIT2(0xD3, add_1reg(0xE8, dreg_hi));
 	/* mov dreg_lo,dreg_hi */
 	EMIT2(0x89, add_2reg(0xC0, dreg_lo, dreg_hi));
 	/* xor dreg_hi,dreg_hi */
 	EMIT2(0x33, add_2reg(0xC0, dreg_hi, dreg_hi));
 
-	/* goto out; */
-	if (is_imm8(jmp_label(jmp_label3, 2)))
-		EMIT2(0xEB, jmp_label(jmp_label3, 2));
-	else
-		EMIT1_off32(0xE9, jmp_label(jmp_label3, 5));
-
-	/* >= 64 */
-	if (jmp_label2 == -1)
-		jmp_label2 = cnt;
-	/* xor dreg_lo,dreg_lo */
-	EMIT2(0x33, add_2reg(0xC0, dreg_lo, dreg_lo));
-	/* xor dreg_hi,dreg_hi */
-	EMIT2(0x33, add_2reg(0xC0, dreg_hi, dreg_hi));
-
-	if (jmp_label3 == -1)
-		jmp_label3 = cnt;
-
 	if (dstk) {
 		/* mov dword ptr [ebp+off],dreg_lo */
 		EMIT3(0x89, add_2reg(0x40, IA32_EBP, dreg_lo),
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH v2 bpf-next 1/3] libbpf: add perf buffer API
From: Andrii Nakryiko @ 2019-06-29  5:56 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: Andrii Nakryiko, Alexei Starovoitov, bpf, Networking, Kernel Team
In-Reply-To: <CAEf4Bzb4U73jb80eCv+JoFGFd2ACXK4j6d=ZeVOoRH1d0f-dPg@mail.gmail.com>

On Thu, Jun 27, 2019 at 2:45 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Thu, Jun 27, 2019 at 2:04 PM Daniel Borkmann <daniel@iogearbox.net> wrote:
> >
> > On 06/26/2019 08:12 AM, Andrii Nakryiko wrote:
> > > BPF_MAP_TYPE_PERF_EVENT_ARRAY map is often used to send data from BPF program
> > > to user space for additional processing. libbpf already has very low-level API
> > > to read single CPU perf buffer, bpf_perf_event_read_simple(), but it's hard to
> > > use and requires a lot of code to set everything up. This patch adds
> > > perf_buffer abstraction on top of it, abstracting setting up and polling
> > > per-CPU logic into simple and convenient API, similar to what BCC provides.
> > >
> > > perf_buffer__new() sets up per-CPU ring buffers and updates corresponding BPF
> > > map entries. It accepts two user-provided callbacks: one for handling raw
> > > samples and one for get notifications of lost samples due to buffer overflow.
> > >
> > > perf_buffer__poll() is used to fetch ring buffer data across all CPUs,
> > > utilizing epoll instance.
> > >
> > > perf_buffer__free() does corresponding clean up and unsets FDs from BPF map.
> > >
> > > All APIs are not thread-safe. User should ensure proper locking/coordination if
> > > used in multi-threaded set up.
> > >
> > > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> >
> > Aside from current feedback, this series generally looks great! Two small things:
> >
> > > diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> > > index 2382fbda4cbb..10f48103110a 100644
> > > --- a/tools/lib/bpf/libbpf.map
> > > +++ b/tools/lib/bpf/libbpf.map
> > > @@ -170,13 +170,16 @@ LIBBPF_0.0.4 {
> > >               btf_dump__dump_type;
> > >               btf_dump__free;
> > >               btf_dump__new;
> > > -             btf__parse_elf;
> > >               bpf_object__load_xattr;
> > >               bpf_program__attach_kprobe;
> > >               bpf_program__attach_perf_event;
> > >               bpf_program__attach_raw_tracepoint;
> > >               bpf_program__attach_tracepoint;
> > >               bpf_program__attach_uprobe;
> > > +             btf__parse_elf;
> > >               libbpf_num_possible_cpus;
> > >               libbpf_perf_event_disable_and_close;
> > > +             perf_buffer__free;
> > > +             perf_buffer__new;
> > > +             perf_buffer__poll;
> >
> > We should prefix with libbpf_* given it's not strictly BPF-only and rather
> > helper function.
>
> Well, perf_buffer is an object similar to `struct btf`, `struct
> bpf_program`, etc. So it seems appropriate to follow this
> "<object>__<method>" convention. Also, `struct libbpf_perf_buffer` and
> `libbpf_perf_buffer__new` looks verbose and pretty ugly, IMO.
>
> >
> > Also, we should convert bpftool (tools/bpf/bpftool/map_perf_ring.c) to make
> > use of these new helpers instead of open-coding there.
>
> Yep, absolutely, will do that as well, thanks for pointing me there!

This turned out to require much bigger changes, as bpftool needed way
more low-level control over structure of events and attachment
policies (custom cpu index and map key). So I ended up having two
APIs:
1. simple common-case perf_buffer__new with 2 callbacks, that attaches
to all CPUs (up to max_elements of map)
2. perf_buffer__new_raw, that allows to provide custom
perf_event_attr, callback that accepts pointer to raw perf event and
allows to specify any set of cpu/map keys.

bpftool uses the latter one. Please see v3.

>
> >
> > Thanks,
> > Daniel

^ 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