Skip to content

Commit

Permalink
Merge branch 'net-fix-regressions-for-generic-XDP'
Browse files Browse the repository at this point in the history
Jesper Dangaard Brouer says:

====================
net: fix regressions for generic-XDP

Thanks to Brandon Cazander, who wrote a very detailed bug report that
even used perf probe's on xdp-newbies mailing list, we discovered that
generic-XDP contains some regressions when using bpf_xdp_adjust_head().

First issue were that my selftests script, that use bpf_xdp_adjust_head(),
by mistake didn't use generic-XDP any-longer. That selftest should have
caught the real regression introduced in commit 458bf2f ("net: core:
support XDP generic on stacked devices.").

To verify this patchset fix the regressions, you can invoked manually via:

  cd tools/testing/selftests/bpf/
  sudo ./test_xdp_vlan_mode_generic.sh
  sudo ./test_xdp_vlan_mode_native.sh
====================

Link: https://www.spinics.net/lists/xdp-newbies/msg01231.html
Fixes: 458bf2f ("net: core: support XDP generic on stacked devices.")
Reported by: Brandon Cazander <brandon.cazander@multapplied.net>
Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
David S. Miller committed Aug 5, 2019
2 parents 60d60c8 + 065af35 commit 4130741
Show file tree
Hide file tree
Showing 5 changed files with 75 additions and 18 deletions.
15 changes: 10 additions & 5 deletions net/core/dev.c
Original file line number Diff line number Diff line change
Expand Up @@ -4374,12 +4374,17 @@ static u32 netif_receive_generic_xdp(struct sk_buff *skb,

act = bpf_prog_run_xdp(xdp_prog, xdp);

/* check if bpf_xdp_adjust_head was used */
off = xdp->data - orig_data;
if (off > 0)
__skb_pull(skb, off);
else if (off < 0)
__skb_push(skb, -off);
skb->mac_header += off;
if (off) {
if (off > 0)
__skb_pull(skb, off);
else if (off < 0)
__skb_push(skb, -off);

skb->mac_header += off;
skb_reset_network_header(skb);
}

/* check if bpf_xdp_adjust_tail was used. it can only "shrink"
* pckt.
Expand Down
3 changes: 2 additions & 1 deletion tools/testing/selftests/bpf/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,8 @@ TEST_PROGS := test_kmod.sh \
test_lirc_mode2.sh \
test_skb_cgroup_id.sh \
test_flow_dissector.sh \
test_xdp_vlan.sh \
test_xdp_vlan_mode_generic.sh \
test_xdp_vlan_mode_native.sh \
test_lwt_ip_encap.sh \
test_tcp_check_syncookie.sh \
test_tc_tunnel.sh \
Expand Down
57 changes: 45 additions & 12 deletions tools/testing/selftests/bpf/test_xdp_vlan.sh
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
#!/bin/bash
# SPDX-License-Identifier: GPL-2.0
# Author: Jesper Dangaard Brouer <hawk@kernel.org>

TESTNAME=xdp_vlan
# Allow wrapper scripts to name test
if [ -z "$TESTNAME" ]; then
TESTNAME=xdp_vlan
fi

# Default XDP mode
XDP_MODE=xdpgeneric

usage() {
echo "Testing XDP + TC eBPF VLAN manipulations: $TESTNAME"
Expand All @@ -9,9 +17,23 @@ usage() {
echo " -v | --verbose : Verbose"
echo " --flush : Flush before starting (e.g. after --interactive)"
echo " --interactive : Keep netns setup running after test-run"
echo " --mode=XXX : Choose XDP mode (xdp | xdpgeneric | xdpdrv)"
echo ""
}

valid_xdp_mode()
{
local mode=$1

case "$mode" in
xdpgeneric | xdpdrv | xdp)
return 0
;;
*)
return 1
esac
}

cleanup()
{
local status=$?
Expand All @@ -37,7 +59,7 @@ cleanup()

# Using external program "getopt" to get --long-options
OPTIONS=$(getopt -o hvfi: \
--long verbose,flush,help,interactive,debug -- "$@")
--long verbose,flush,help,interactive,debug,mode: -- "$@")
if (( $? != 0 )); then
usage
echo "selftests: $TESTNAME [FAILED] Error calling getopt, unknown option?"
Expand All @@ -60,6 +82,11 @@ while true; do
cleanup
shift
;;
--mode )
shift
XDP_MODE=$1
shift
;;
-- )
shift
break
Expand All @@ -81,8 +108,14 @@ if [ "$EUID" -ne 0 ]; then
exit 1
fi

ip link set dev lo xdp off 2>/dev/null > /dev/null
if [ $? -ne 0 ];then
valid_xdp_mode $XDP_MODE
if [ $? -ne 0 ]; then
echo "selftests: $TESTNAME [FAILED] unknown XDP mode ($XDP_MODE)"
exit 1
fi

ip link set dev lo xdpgeneric off 2>/dev/null > /dev/null
if [ $? -ne 0 ]; then
echo "selftests: $TESTNAME [SKIP] need ip xdp support"
exit 0
fi
Expand Down Expand Up @@ -155,7 +188,7 @@ ip netns exec ns2 ip link set lo up
# At this point, the hosts cannot reach each-other,
# because ns2 are using VLAN tags on the packets.

ip netns exec ns2 sh -c 'ping -W 1 -c 1 100.64.41.1 || echo "Okay ping fails"'
ip netns exec ns2 sh -c 'ping -W 1 -c 1 100.64.41.1 || echo "Success: First ping must fail"'


# Now we can use the test_xdp_vlan.c program to pop/push these VLAN tags
Expand All @@ -166,7 +199,7 @@ export FILE=test_xdp_vlan.o

# First test: Remove VLAN by setting VLAN ID 0, using "xdp_vlan_change"
export XDP_PROG=xdp_vlan_change
ip netns exec ns1 ip link set $DEVNS1 xdp object $FILE section $XDP_PROG
ip netns exec ns1 ip link set $DEVNS1 $XDP_MODE object $FILE section $XDP_PROG

# In ns1: egress use TC to add back VLAN tag 4011
# (del cmd)
Expand All @@ -177,8 +210,8 @@ ip netns exec ns1 tc filter add dev $DEVNS1 egress \
prio 1 handle 1 bpf da obj $FILE sec tc_vlan_push

# Now the namespaces can reach each-other, test with ping:
ip netns exec ns2 ping -W 2 -c 3 $IPADDR1
ip netns exec ns1 ping -W 2 -c 3 $IPADDR2
ip netns exec ns2 ping -i 0.2 -W 2 -c 2 $IPADDR1
ip netns exec ns1 ping -i 0.2 -W 2 -c 2 $IPADDR2

# Second test: Replace xdp prog, that fully remove vlan header
#
Expand All @@ -187,9 +220,9 @@ ip netns exec ns1 ping -W 2 -c 3 $IPADDR2
# ETH_P_8021Q indication, and this cause overwriting of our changes.
#
export XDP_PROG=xdp_vlan_remove_outer2
ip netns exec ns1 ip link set $DEVNS1 xdp off
ip netns exec ns1 ip link set $DEVNS1 xdp object $FILE section $XDP_PROG
ip netns exec ns1 ip link set $DEVNS1 $XDP_MODE off
ip netns exec ns1 ip link set $DEVNS1 $XDP_MODE object $FILE section $XDP_PROG

# Now the namespaces should still be able reach each-other, test with ping:
ip netns exec ns2 ping -W 2 -c 3 $IPADDR1
ip netns exec ns1 ping -W 2 -c 3 $IPADDR2
ip netns exec ns2 ping -i 0.2 -W 2 -c 2 $IPADDR1
ip netns exec ns1 ping -i 0.2 -W 2 -c 2 $IPADDR2
9 changes: 9 additions & 0 deletions tools/testing/selftests/bpf/test_xdp_vlan_mode_generic.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
#!/bin/bash
# SPDX-License-Identifier: GPL-2.0

# Exit on failure
set -e

# Wrapper script to test generic-XDP
export TESTNAME=xdp_vlan_mode_generic
./test_xdp_vlan.sh --mode=xdpgeneric
9 changes: 9 additions & 0 deletions tools/testing/selftests/bpf/test_xdp_vlan_mode_native.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
#!/bin/bash
# SPDX-License-Identifier: GPL-2.0

# Exit on failure
set -e

# Wrapper script to test native-XDP
export TESTNAME=xdp_vlan_mode_native
./test_xdp_vlan.sh --mode=xdpdrv

0 comments on commit 4130741

Please sign in to comment.