* [PATCH net] vrf: packets with lladdr src needs dst at input with orig_iif when needs strict
@ 2020-12-04 3:06 Stephen Suryaputra
2020-12-04 16:32 ` David Ahern
0 siblings, 1 reply; 5+ messages in thread
From: Stephen Suryaputra @ 2020-12-04 3:06 UTC (permalink / raw)
To: netdev; +Cc: dsahern, Stephen Suryaputra
Depending on the order of the routes to fe80::/64 are installed on the
VRF table, the NS for the source link-local address of the originator
might be sent to the wrong interface.
This patch ensures that packets with link-local addr source is doing a
lookup with the orig_iif when the destination addr indicates that it
is strict.
Add the reproducer as a use case in self test script fcnal-test.sh.
Signed-off-by: Stephen Suryaputra <ssuryaextr@gmail.com>
---
drivers/net/vrf.c | 10 ++-
tools/testing/selftests/net/fcnal-test.sh | 95 +++++++++++++++++++++++
2 files changed, 103 insertions(+), 2 deletions(-)
diff --git a/drivers/net/vrf.c b/drivers/net/vrf.c
index f2793ffde191..b9b7e00b72a8 100644
--- a/drivers/net/vrf.c
+++ b/drivers/net/vrf.c
@@ -1315,11 +1315,17 @@ static struct sk_buff *vrf_ip6_rcv(struct net_device *vrf_dev,
int orig_iif = skb->skb_iif;
bool need_strict = rt6_need_strict(&ipv6_hdr(skb)->daddr);
bool is_ndisc = ipv6_ndisc_frame(skb);
+ bool is_ll_src;
/* loopback, multicast & non-ND link-local traffic; do not push through
- * packet taps again. Reset pkt_type for upper layers to process skb
+ * packet taps again. Reset pkt_type for upper layers to process skb.
+ * for packets with lladdr src, however, skip so that the dst can be
+ * determine at input using original ifindex in the case that daddr
+ * needs strict
*/
- if (skb->pkt_type == PACKET_LOOPBACK || (need_strict && !is_ndisc)) {
+ is_ll_src = ipv6_addr_type(&ipv6_hdr(skb)->saddr) & IPV6_ADDR_LINKLOCAL;
+ if (skb->pkt_type == PACKET_LOOPBACK ||
+ (need_strict && !is_ndisc && !is_ll_src)) {
skb->dev = vrf_dev;
skb->skb_iif = vrf_dev->ifindex;
IP6CB(skb)->flags |= IP6SKB_L3SLAVE;
diff --git a/tools/testing/selftests/net/fcnal-test.sh b/tools/testing/selftests/net/fcnal-test.sh
index fb5c55dd6df8..02b0b9ead40b 100755
--- a/tools/testing/selftests/net/fcnal-test.sh
+++ b/tools/testing/selftests/net/fcnal-test.sh
@@ -256,6 +256,28 @@ setup_cmd_nsb()
fi
}
+setup_cmd_nsc()
+{
+ local cmd="$*"
+ local rc
+
+ run_cmd_nsc ${cmd}
+ rc=$?
+ if [ $rc -ne 0 ]; then
+ # show user the command if not done so already
+ if [ "$VERBOSE" = "0" ]; then
+ echo "setup command: $cmd"
+ fi
+ echo "failed. stopping tests"
+ if [ "${PAUSE_ON_FAIL}" = "yes" ]; then
+ echo
+ echo "hit enter to continue"
+ read a
+ fi
+ exit $rc
+ fi
+}
+
# set sysctl values in NS-A
set_sysctl()
{
@@ -471,6 +493,36 @@ setup()
sleep 1
}
+setup_lla_only()
+{
+ # make sure we are starting with a clean slate
+ kill_procs
+ cleanup 2>/dev/null
+
+ log_debug "Configuring network namespaces"
+ set -e
+
+ create_ns ${NSA} "-" "-"
+ create_ns ${NSB} "-" "-"
+ create_ns ${NSC} "-" "-"
+ connect_ns ${NSA} ${NSA_DEV} "-" "-" \
+ ${NSB} ${NSB_DEV} "-" "-"
+ connect_ns ${NSA} ${NSA_DEV2} "-" "-" \
+ ${NSC} ${NSC_DEV} "-" "-"
+
+ NSA_LINKIP6=$(get_linklocal ${NSA} ${NSA_DEV})
+ NSB_LINKIP6=$(get_linklocal ${NSB} ${NSB_DEV})
+ NSC_LINKIP6=$(get_linklocal ${NSC} ${NSC_DEV})
+
+ create_vrf ${NSA} ${VRF} ${VRF_TABLE} "-" "-"
+ ip -netns ${NSA} link set dev ${NSA_DEV} vrf ${VRF}
+ ip -netns ${NSA} link set dev ${NSA_DEV2} vrf ${VRF}
+
+ set +e
+
+ sleep 1
+}
+
################################################################################
# IPv4
@@ -3787,10 +3839,53 @@ use_case_br()
setup_cmd_nsb ip li del vlan100 2>/dev/null
}
+# VRF only.
+# ns-A device is connected to both ns-B and ns-C on a single VRF but only has
+# LLA on the interfaces
+use_case_ping_lla_multi()
+{
+ setup_lla_only
+ # only want reply from ns-A
+ setup_cmd_nsb sysctl -qw net.ipv6.icmp.echo_ignore_multicast=1
+ setup_cmd_nsc sysctl -qw net.ipv6.icmp.echo_ignore_multicast=1
+
+ log_start
+ run_cmd_nsb ping -c1 -w1 ${MCAST}%${NSB_DEV}
+ log_test_addr ${MCAST}%${NSB_DEV} $? 0 "Pre cycle, ping out ns-B"
+
+ run_cmd_nsc ping -c1 -w1 ${MCAST}%${NSC_DEV}
+ log_test_addr ${MCAST}%${NSC_DEV} $? 0 "Pre cycle, ping out ns-C"
+
+ # cycle/flap the first ns-A interface
+ setup_cmd ip link set ${NSA_DEV} down
+ setup_cmd ip link set ${NSA_DEV} up
+ sleep 1
+
+ log_start
+ run_cmd_nsb ping -c1 -w1 ${MCAST}%${NSB_DEV}
+ log_test_addr ${MCAST}%${NSB_DEV} $? 0 "Post cycle ${NSA} ${NSA_DEV}, ping out ns-B"
+ run_cmd_nsc ping -c1 -w1 ${MCAST}%${NSC_DEV}
+ log_test_addr ${MCAST}%${NSC_DEV} $? 0 "Post cycle ${NSA} ${NSA_DEV}, ping out ns-C"
+
+ # cycle/flap the second ns-A interface
+ setup_cmd ip link set ${NSA_DEV2} down
+ setup_cmd ip link set ${NSA_DEV2} up
+ sleep 1
+
+ log_start
+ run_cmd_nsb ping -c1 -w1 ${MCAST}%${NSB_DEV}
+ log_test_addr ${MCAST}%${NSB_DEV} $? 0 "Post cycle ${NSA} ${NSA_DEV2}, ping out ns-B"
+ run_cmd_nsc ping -c1 -w1 ${MCAST}%${NSC_DEV}
+ log_test_addr ${MCAST}%${NSC_DEV} $? 0 "Post cycle ${NSA} ${NSA_DEV2}, ping out ns-C"
+}
+
use_cases()
{
log_section "Use cases"
+ log_subsection "Device enslaved to bridge"
use_case_br
+ log_subsection "Ping LLA with multiple interfaces"
+ use_case_ping_lla_multi
}
################################################################################
--
2.20.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH net] vrf: packets with lladdr src needs dst at input with orig_iif when needs strict
2020-12-04 3:06 [PATCH net] vrf: packets with lladdr src needs dst at input with orig_iif when needs strict Stephen Suryaputra
@ 2020-12-04 16:32 ` David Ahern
2020-12-04 23:37 ` Jakub Kicinski
0 siblings, 1 reply; 5+ messages in thread
From: David Ahern @ 2020-12-04 16:32 UTC (permalink / raw)
To: Stephen Suryaputra, netdev
On 12/3/20 8:06 PM, Stephen Suryaputra wrote:
> Depending on the order of the routes to fe80::/64 are installed on the
> VRF table, the NS for the source link-local address of the originator
> might be sent to the wrong interface.
>
> This patch ensures that packets with link-local addr source is doing a
> lookup with the orig_iif when the destination addr indicates that it
> is strict.
>
> Add the reproducer as a use case in self test script fcnal-test.sh.
>
> Signed-off-by: Stephen Suryaputra <ssuryaextr@gmail.com>
> ---
> drivers/net/vrf.c | 10 ++-
> tools/testing/selftests/net/fcnal-test.sh | 95 +++++++++++++++++++++++
> 2 files changed, 103 insertions(+), 2 deletions(-)
>
Reviewed-by: David Ahern <dsahern@kernel.org>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net] vrf: packets with lladdr src needs dst at input with orig_iif when needs strict
2020-12-04 16:32 ` David Ahern
@ 2020-12-04 23:37 ` Jakub Kicinski
2020-12-05 0:22 ` Stephen Suryaputra
0 siblings, 1 reply; 5+ messages in thread
From: Jakub Kicinski @ 2020-12-04 23:37 UTC (permalink / raw)
To: David Ahern; +Cc: Stephen Suryaputra, netdev
On Fri, 4 Dec 2020 09:32:04 -0700 David Ahern wrote:
> On 12/3/20 8:06 PM, Stephen Suryaputra wrote:
> > Depending on the order of the routes to fe80::/64 are installed on the
> > VRF table, the NS for the source link-local address of the originator
> > might be sent to the wrong interface.
> >
> > This patch ensures that packets with link-local addr source is doing a
> > lookup with the orig_iif when the destination addr indicates that it
> > is strict.
> >
> > Add the reproducer as a use case in self test script fcnal-test.sh.
> >
> > Signed-off-by: Stephen Suryaputra <ssuryaextr@gmail.com>
> > ---
> > drivers/net/vrf.c | 10 ++-
> > tools/testing/selftests/net/fcnal-test.sh | 95 +++++++++++++++++++++++
> > 2 files changed, 103 insertions(+), 2 deletions(-)
>
> Reviewed-by: David Ahern <dsahern@kernel.org>
Should I put something like:
Fixes: b4869aa2f881 ("net: vrf: ipv6 support for local traffic to local addresses")
on this?
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net] vrf: packets with lladdr src needs dst at input with orig_iif when needs strict
2020-12-04 23:37 ` Jakub Kicinski
@ 2020-12-05 0:22 ` Stephen Suryaputra
2020-12-05 21:49 ` Jakub Kicinski
0 siblings, 1 reply; 5+ messages in thread
From: Stephen Suryaputra @ 2020-12-05 0:22 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: David Ahern, netdev
On Fri, Dec 04, 2020 at 03:37:48PM -0800, Jakub Kicinski wrote:
> On Fri, 4 Dec 2020 09:32:04 -0700 David Ahern wrote:
> > On 12/3/20 8:06 PM, Stephen Suryaputra wrote:
> > > Depending on the order of the routes to fe80::/64 are installed on the
> > > VRF table, the NS for the source link-local address of the originator
> > > might be sent to the wrong interface.
> > >
> > > This patch ensures that packets with link-local addr source is doing a
> > > lookup with the orig_iif when the destination addr indicates that it
> > > is strict.
> > >
> > > Add the reproducer as a use case in self test script fcnal-test.sh.
> > >
> > > Signed-off-by: Stephen Suryaputra <ssuryaextr@gmail.com>
> > > ---
> > > drivers/net/vrf.c | 10 ++-
> > > tools/testing/selftests/net/fcnal-test.sh | 95 +++++++++++++++++++++++
> > > 2 files changed, 103 insertions(+), 2 deletions(-)
> >
> > Reviewed-by: David Ahern <dsahern@kernel.org>
>
> Should I put something like:
>
> Fixes: b4869aa2f881 ("net: vrf: ipv6 support for local traffic to local addresses")
>
> on this?
I was conflicted when I was about to put Fixes tag on this patch because
it could either be b4869aa2f881 that you mentioned above, or 6f12fa7755301
("vrf: mark skb for multicast or link-local as enslaved to VRF"). So, I
decided not to put it, but may be I should so that this is qualified to
be queued to stable?
Thanks,
Stephen.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net] vrf: packets with lladdr src needs dst at input with orig_iif when needs strict
2020-12-05 0:22 ` Stephen Suryaputra
@ 2020-12-05 21:49 ` Jakub Kicinski
0 siblings, 0 replies; 5+ messages in thread
From: Jakub Kicinski @ 2020-12-05 21:49 UTC (permalink / raw)
To: Stephen Suryaputra; +Cc: David Ahern, netdev
On Fri, 4 Dec 2020 19:22:27 -0500 Stephen Suryaputra wrote:
> On Fri, Dec 04, 2020 at 03:37:48PM -0800, Jakub Kicinski wrote:
> > On Fri, 4 Dec 2020 09:32:04 -0700 David Ahern wrote:
> > > On 12/3/20 8:06 PM, Stephen Suryaputra wrote:
> > > > Depending on the order of the routes to fe80::/64 are installed on the
> > > > VRF table, the NS for the source link-local address of the originator
> > > > might be sent to the wrong interface.
> > > >
> > > > This patch ensures that packets with link-local addr source is doing a
> > > > lookup with the orig_iif when the destination addr indicates that it
> > > > is strict.
> > > >
> > > > Add the reproducer as a use case in self test script fcnal-test.sh.
> > > >
> > > > Signed-off-by: Stephen Suryaputra <ssuryaextr@gmail.com>
> > > > ---
> > > > drivers/net/vrf.c | 10 ++-
> > > > tools/testing/selftests/net/fcnal-test.sh | 95 +++++++++++++++++++++++
> > > > 2 files changed, 103 insertions(+), 2 deletions(-)
> > >
> > > Reviewed-by: David Ahern <dsahern@kernel.org>
> >
> > Should I put something like:
> >
> > Fixes: b4869aa2f881 ("net: vrf: ipv6 support for local traffic to local addresses")
> >
> > on this?
>
> I was conflicted when I was about to put Fixes tag on this patch because
> it could either be b4869aa2f881 that you mentioned above, or 6f12fa7755301
> ("vrf: mark skb for multicast or link-local as enslaved to VRF"). So, I
> decided not to put it, but may be I should so that this is qualified to
> be queued to stable?
Yeah, probably doesn't matter that much in practice. Either one would
work, since the patch won't apply without 6f12fa7755301.
I added the one I mentioned and applied to net.
Thanks!
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-12-05 21:50 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-04 3:06 [PATCH net] vrf: packets with lladdr src needs dst at input with orig_iif when needs strict Stephen Suryaputra
2020-12-04 16:32 ` David Ahern
2020-12-04 23:37 ` Jakub Kicinski
2020-12-05 0:22 ` Stephen Suryaputra
2020-12-05 21:49 ` Jakub Kicinski
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.