| Message ID | 20211014130845.410602-1-ssuryaextr@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] ipv6: When forwarding count rx stats on the orig netdev | expand |
[ added Ido for the forwarding tests ] On 10/14/21 7:08 AM, Stephen Suryaputra wrote: > Commit bdb7cc643fc9 ("ipv6: Count interface receive statistics on the > ingress netdev") does not work when ip6_forward() executes on the skbs > with vrf-enslaved netdev. Use IP6CB(skb)->iif to get to the right one. > > Add a selftest script to verify. > > Fixes: bdb7cc643fc9 ("ipv6: Count interface receive statistics on the ingress netdev") > Signed-off-by: Stephen Suryaputra <ssuryaextr@gmail.com> > --- > net/ipv6/ip6_output.c | 3 +- > .../testing/selftests/net/forwarding/Makefile | 1 + > .../net/forwarding/forwarding.config.sample | 2 + > .../net/forwarding/ip6_forward_instats_vrf.sh | 172 ++++++++++++++++++ > tools/testing/selftests/net/forwarding/lib.sh | 8 + > 5 files changed, 185 insertions(+), 1 deletion(-) > create mode 100755 tools/testing/selftests/net/forwarding/ip6_forward_instats_vrf.sh > > diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c > index 12f985f43bcc..2f044a49afa8 100644 > --- a/net/ipv6/ip6_output.c > +++ b/net/ipv6/ip6_output.c > @@ -464,13 +464,14 @@ static bool ip6_pkt_too_big(const struct sk_buff *skb, unsigned int mtu) > > int ip6_forward(struct sk_buff *skb) > { > - struct inet6_dev *idev = __in6_dev_get_safely(skb->dev); > struct dst_entry *dst = skb_dst(skb); > struct ipv6hdr *hdr = ipv6_hdr(skb); > struct inet6_skb_parm *opt = IP6CB(skb); > struct net *net = dev_net(dst->dev); > + struct inet6_dev *idev; > u32 mtu; > > + idev = __in6_dev_get_safely(dev_get_by_index_rcu(net, IP6CB(skb)->iif)); > if (net->ipv6.devconf_all->forwarding == 0) > goto error; > This seems fine to me, but IPv4 and IPv6 should work the same.
On Thu, Oct 14, 2021 at 08:15:34PM -0600, David Ahern wrote: > [ added Ido for the forwarding tests ] > [snip] > > This seems fine to me, but IPv4 and IPv6 should work the same. But we don't have per if ipv4 stats. Remember that I tried to get something going but wasn't getting any traction?
On 10/14/21 8:22 PM, Stephen Suryaputra wrote: > On Thu, Oct 14, 2021 at 08:15:34PM -0600, David Ahern wrote: >> [ added Ido for the forwarding tests ] >> > [snip] >> >> This seems fine to me, but IPv4 and IPv6 should work the same. > > But we don't have per if ipv4 stats. Remember that I tried to get > something going but wasn't getting any traction? > oh right, ipv4 is per net-namespace.
On Thu, 14 Oct 2021 20:27:38 -0600 David Ahern wrote: > On 10/14/21 8:22 PM, Stephen Suryaputra wrote: > > On Thu, Oct 14, 2021 at 08:15:34PM -0600, David Ahern wrote: > >> [ added Ido for the forwarding tests ] > >> > > [snip] > >> > >> This seems fine to me, but IPv4 and IPv6 should work the same. > > > > But we don't have per if ipv4 stats. Remember that I tried to get > > something going but wasn't getting any traction? > > > oh right, ipv4 is per net-namespace. Is that an ack? :)
On 10/15/21 2:01 PM, Jakub Kicinski wrote: > On Thu, 14 Oct 2021 20:27:38 -0600 David Ahern wrote: >> On 10/14/21 8:22 PM, Stephen Suryaputra wrote: >>> On Thu, Oct 14, 2021 at 08:15:34PM -0600, David Ahern wrote: >>>> [ added Ido for the forwarding tests ] >>>> >>> [snip] >>>> >>>> This seems fine to me, but IPv4 and IPv6 should work the same. >>> >>> But we don't have per if ipv4 stats. Remember that I tried to get >>> something going but wasn't getting any traction? >>> >> oh right, ipv4 is per net-namespace. > > Is that an ack? :) > Reviewed-by: David Ahern <dsahern@kernel.org>
On Fri, 15 Oct 2021 15:28:00 -0600 David Ahern wrote: > >> oh right, ipv4 is per net-namespace. > > > > Is that an ack? :) > > Reviewed-by: David Ahern <dsahern@kernel.org> Thanks, applied!
Could this patch be queued for -stable? Thank you. On Fri, Oct 15, 2021 at 7:29 PM Jakub Kicinski <kuba@kernel.org> wrote: > > On Fri, 15 Oct 2021 15:28:00 -0600 David Ahern wrote: > > >> oh right, ipv4 is per net-namespace. > > > > > > Is that an ack? :) > > > > Reviewed-by: David Ahern <dsahern@kernel.org> > > Thanks, applied!
On Mon, 18 Oct 2021 09:24:21 -0400 Stephen Suryaputra wrote:
> Could this patch be queued for -stable?
It will happen automatically based on the Fixes tag.
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c index 12f985f43bcc..2f044a49afa8 100644 --- a/net/ipv6/ip6_output.c +++ b/net/ipv6/ip6_output.c @@ -464,13 +464,14 @@ static bool ip6_pkt_too_big(const struct sk_buff *skb, unsigned int mtu) int ip6_forward(struct sk_buff *skb) { - struct inet6_dev *idev = __in6_dev_get_safely(skb->dev); struct dst_entry *dst = skb_dst(skb); struct ipv6hdr *hdr = ipv6_hdr(skb); struct inet6_skb_parm *opt = IP6CB(skb); struct net *net = dev_net(dst->dev); + struct inet6_dev *idev; u32 mtu; + idev = __in6_dev_get_safely(dev_get_by_index_rcu(net, IP6CB(skb)->iif)); if (net->ipv6.devconf_all->forwarding == 0) goto error; diff --git a/tools/testing/selftests/net/forwarding/Makefile b/tools/testing/selftests/net/forwarding/Makefile index d97bd6889446..72ee644d47bf 100644 --- a/tools/testing/selftests/net/forwarding/Makefile +++ b/tools/testing/selftests/net/forwarding/Makefile @@ -9,6 +9,7 @@ TEST_PROGS = bridge_igmp.sh \ gre_inner_v4_multipath.sh \ gre_inner_v6_multipath.sh \ gre_multipath.sh \ + ip6_forward_instats_vrf.sh \ ip6gre_inner_v4_multipath.sh \ ip6gre_inner_v6_multipath.sh \ ipip_flat_gre_key.sh \ diff --git a/tools/testing/selftests/net/forwarding/forwarding.config.sample b/tools/testing/selftests/net/forwarding/forwarding.config.sample index b802c14d2950..e5e2fbeca22e 100644 --- a/tools/testing/selftests/net/forwarding/forwarding.config.sample +++ b/tools/testing/selftests/net/forwarding/forwarding.config.sample @@ -39,3 +39,5 @@ NETIF_CREATE=yes # Timeout (in seconds) before ping exits regardless of how many packets have # been sent or received PING_TIMEOUT=5 +# IPv6 traceroute utility name. +TROUTE6=traceroute6 diff --git a/tools/testing/selftests/net/forwarding/ip6_forward_instats_vrf.sh b/tools/testing/selftests/net/forwarding/ip6_forward_instats_vrf.sh new file mode 100755 index 000000000000..9f5b3e2e5e95 --- /dev/null +++ b/tools/testing/selftests/net/forwarding/ip6_forward_instats_vrf.sh @@ -0,0 +1,172 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0 + +# Test ipv6 stats on the incoming if when forwarding with VRF + +ALL_TESTS=" + ipv6_ping + ipv6_in_too_big_err + ipv6_in_hdr_err + ipv6_in_addr_err + ipv6_in_discard +" + +NUM_NETIFS=4 +source lib.sh + +h1_create() +{ + simple_if_init $h1 2001:1:1::2/64 + ip -6 route add vrf v$h1 2001:1:2::/64 via 2001:1:1::1 +} + +h1_destroy() +{ + ip -6 route del vrf v$h1 2001:1:2::/64 via 2001:1:1::1 + simple_if_fini $h1 2001:1:1::2/64 +} + +router_create() +{ + vrf_create router + __simple_if_init $rtr1 router 2001:1:1::1/64 + __simple_if_init $rtr2 router 2001:1:2::1/64 + mtu_set $rtr2 1280 +} + +router_destroy() +{ + mtu_restore $rtr2 + __simple_if_fini $rtr2 2001:1:2::1/64 + __simple_if_fini $rtr1 2001:1:1::1/64 + vrf_destroy router +} + +h2_create() +{ + simple_if_init $h2 2001:1:2::2/64 + ip -6 route add vrf v$h2 2001:1:1::/64 via 2001:1:2::1 + mtu_set $h2 1280 +} + +h2_destroy() +{ + mtu_restore $h2 + ip -6 route del vrf v$h2 2001:1:1::/64 via 2001:1:2::1 + simple_if_fini $h2 2001:1:2::2/64 +} + +setup_prepare() +{ + h1=${NETIFS[p1]} + rtr1=${NETIFS[p2]} + + rtr2=${NETIFS[p3]} + h2=${NETIFS[p4]} + + vrf_prepare + h1_create + router_create + h2_create + + forwarding_enable +} + +cleanup() +{ + pre_cleanup + + forwarding_restore + + h2_destroy + router_destroy + h1_destroy + vrf_cleanup +} + +ipv6_in_too_big_err() +{ + RET=0 + + local t0=$(ipv6_stats_get $rtr1 Ip6InTooBigErrors) + local vrf_name=$(master_name_get $h1) + + # Send too big packets + ip vrf exec $vrf_name \ + $PING6 -s 1300 2001:1:2::2 -c 1 -w $PING_TIMEOUT &> /dev/null + + local t1=$(ipv6_stats_get $rtr1 Ip6InTooBigErrors) + test "$((t1 - t0))" -ne 0 + check_err $? + log_test "Ip6InTooBigErrors" +} + +ipv6_in_hdr_err() +{ + RET=0 + + local t0=$(ipv6_stats_get $rtr1 Ip6InHdrErrors) + local vrf_name=$(master_name_get $h1) + + # Send packets with hop limit 1, easiest with traceroute6 as some ping6 + # doesn't allow hop limit to be specified + ip vrf exec $vrf_name \ + $TROUTE6 2001:1:2::2 &> /dev/null + + local t1=$(ipv6_stats_get $rtr1 Ip6InHdrErrors) + test "$((t1 - t0))" -ne 0 + check_err $? + log_test "Ip6InHdrErrors" +} + +ipv6_in_addr_err() +{ + RET=0 + + local t0=$(ipv6_stats_get $rtr1 Ip6InAddrErrors) + local vrf_name=$(master_name_get $h1) + + # Disable forwarding temporary while sending the packet + sysctl -qw net.ipv6.conf.all.forwarding=0 + ip vrf exec $vrf_name \ + $PING6 2001:1:2::2 -c 1 -w $PING_TIMEOUT &> /dev/null + sysctl -qw net.ipv6.conf.all.forwarding=1 + + local t1=$(ipv6_stats_get $rtr1 Ip6InAddrErrors) + test "$((t1 - t0))" -ne 0 + check_err $? + log_test "Ip6InAddrErrors" +} + +ipv6_in_discard() +{ + RET=0 + + local t0=$(ipv6_stats_get $rtr1 Ip6InDiscards) + local vrf_name=$(master_name_get $h1) + + # Add a policy to discard + ip xfrm policy add dst 2001:1:2::2/128 dir fwd action block + ip vrf exec $vrf_name \ + $PING6 2001:1:2::2 -c 1 -w $PING_TIMEOUT &> /dev/null + ip xfrm policy del dst 2001:1:2::2/128 dir fwd + + local t1=$(ipv6_stats_get $rtr1 Ip6InDiscards) + test "$((t1 - t0))" -ne 0 + check_err $? + log_test "Ip6InDiscards" +} +ipv6_ping() +{ + RET=0 + + ping6_test $h1 2001:1:2::2 +} + +trap cleanup EXIT + +setup_prepare +setup_wait +tests_run + +exit $EXIT_STATUS diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh index e7fc5c35b569..92087d423bcf 100644 --- a/tools/testing/selftests/net/forwarding/lib.sh +++ b/tools/testing/selftests/net/forwarding/lib.sh @@ -751,6 +751,14 @@ qdisc_parent_stats_get() | jq '.[] | select(.parent == "'"$parent"'") | '"$selector" } +ipv6_stats_get() +{ + local dev=$1; shift + local stat=$1; shift + + cat /proc/net/dev_snmp6/$dev | grep "^$stat" | cut -f2 +} + humanize() { local speed=$1; shift
Commit bdb7cc643fc9 ("ipv6: Count interface receive statistics on the ingress netdev") does not work when ip6_forward() executes on the skbs with vrf-enslaved netdev. Use IP6CB(skb)->iif to get to the right one. Add a selftest script to verify. Fixes: bdb7cc643fc9 ("ipv6: Count interface receive statistics on the ingress netdev") Signed-off-by: Stephen Suryaputra <ssuryaextr@gmail.com> --- net/ipv6/ip6_output.c | 3 +- .../testing/selftests/net/forwarding/Makefile | 1 + .../net/forwarding/forwarding.config.sample | 2 + .../net/forwarding/ip6_forward_instats_vrf.sh | 172 ++++++++++++++++++ tools/testing/selftests/net/forwarding/lib.sh | 8 + 5 files changed, 185 insertions(+), 1 deletion(-) create mode 100755 tools/testing/selftests/net/forwarding/ip6_forward_instats_vrf.sh