diff mbox series

[net] net: mscc: ocelot: fix all IPv6 getting trapped to CPU when PTP timestamping is used

Message ID 20230207183117.1745754-1-vladimir.oltean@nxp.com
State New
Headers
Series [net] net: mscc: ocelot: fix all IPv6 getting trapped to CPU when PTP timestamping is used |

Commit Message

Vladimir Oltean Feb. 7, 2023, 6:31 p.m. UTC 
  While running this selftest which usually passes:

~/selftests/drivers/net/dsa# ./local_termination.sh eno0 swp0
TEST: swp0: Unicast IPv4 to primary MAC address                     [ OK ]
TEST: swp0: Unicast IPv4 to macvlan MAC address                     [ OK ]
TEST: swp0: Unicast IPv4 to unknown MAC address                     [ OK ]
TEST: swp0: Unicast IPv4 to unknown MAC address, promisc            [ OK ]
TEST: swp0: Unicast IPv4 to unknown MAC address, allmulti           [ OK ]
TEST: swp0: Multicast IPv4 to joined group                          [ OK ]
TEST: swp0: Multicast IPv4 to unknown group                         [ OK ]
TEST: swp0: Multicast IPv4 to unknown group, promisc                [ OK ]
TEST: swp0: Multicast IPv4 to unknown group, allmulti               [ OK ]
TEST: swp0: Multicast IPv6 to joined group                          [ OK ]
TEST: swp0: Multicast IPv6 to unknown group                         [ OK ]
TEST: swp0: Multicast IPv6 to unknown group, promisc                [ OK ]
TEST: swp0: Multicast IPv6 to unknown group, allmulti               [ OK ]

if I start PTP timestamping then run it again (debug prints added by me),
the unknown IPv6 MC traffic is seen by the CPU port even when it should
have been dropped:

~/selftests/drivers/net/dsa# ptp4l -i swp0 -2 -P -m
ptp4l[225.410]: selected /dev/ptp1 as PTP clock
[  225.445746] mscc_felix 0000:00:00.5: ocelot_l2_ptp_trap_add: port 0 adding L2 PTP trap
[  225.453815] mscc_felix 0000:00:00.5: ocelot_ipv4_ptp_trap_add: port 0 adding IPv4 PTP event trap
[  225.462703] mscc_felix 0000:00:00.5: ocelot_ipv4_ptp_trap_add: port 0 adding IPv4 PTP general trap
[  225.471768] mscc_felix 0000:00:00.5: ocelot_ipv6_ptp_trap_add: port 0 adding IPv6 PTP event trap
[  225.480651] mscc_felix 0000:00:00.5: ocelot_ipv6_ptp_trap_add: port 0 adding IPv6 PTP general trap
ptp4l[225.488]: port 1: INITIALIZING to LISTENING on INIT_COMPLETE
ptp4l[225.488]: port 0: INITIALIZING to LISTENING on INIT_COMPLETE
^C
~/selftests/drivers/net/dsa# ./local_termination.sh eno0 swp0
TEST: swp0: Unicast IPv4 to primary MAC address                     [ OK ]
TEST: swp0: Unicast IPv4 to macvlan MAC address                     [ OK ]
TEST: swp0: Unicast IPv4 to unknown MAC address                     [ OK ]
TEST: swp0: Unicast IPv4 to unknown MAC address, promisc            [ OK ]
TEST: swp0: Unicast IPv4 to unknown MAC address, allmulti           [ OK ]
TEST: swp0: Multicast IPv4 to joined group                          [ OK ]
TEST: swp0: Multicast IPv4 to unknown group                         [ OK ]
TEST: swp0: Multicast IPv4 to unknown group, promisc                [ OK ]
TEST: swp0: Multicast IPv4 to unknown group, allmulti               [ OK ]
TEST: swp0: Multicast IPv6 to joined group                          [ OK ]
TEST: swp0: Multicast IPv6 to unknown group                         [FAIL]
        reception succeeded, but should have failed
TEST: swp0: Multicast IPv6 to unknown group, promisc                [ OK ]
TEST: swp0: Multicast IPv6 to unknown group, allmulti               [ OK ]

The PGID_MCIPV6 is configured correctly to not flood to the CPU,
I checked that.

Furthermore, when I disable back PTP RX timestamping (ptp4l doesn't do
that when it exists), packets are RX filtered again as they should be:

~/selftests/drivers/net/dsa# hwstamp_ctl -i swp0 -r 0
[  218.202854] mscc_felix 0000:00:00.5: ocelot_l2_ptp_trap_del: port 0 removing L2 PTP trap
[  218.212656] mscc_felix 0000:00:00.5: ocelot_ipv4_ptp_trap_del: port 0 removing IPv4 PTP event trap
[  218.222975] mscc_felix 0000:00:00.5: ocelot_ipv4_ptp_trap_del: port 0 removing IPv4 PTP general trap
[  218.233133] mscc_felix 0000:00:00.5: ocelot_ipv6_ptp_trap_del: port 0 removing IPv6 PTP event trap
[  218.242251] mscc_felix 0000:00:00.5: ocelot_ipv6_ptp_trap_del: port 0 removing IPv6 PTP general trap
current settings:
tx_type 1
rx_filter 12
new settings:
tx_type 1
rx_filter 0
~/selftests/drivers/net/dsa# ./local_termination.sh eno0 swp0
TEST: swp0: Unicast IPv4 to primary MAC address                     [ OK ]
TEST: swp0: Unicast IPv4 to macvlan MAC address                     [ OK ]
TEST: swp0: Unicast IPv4 to unknown MAC address                     [ OK ]
TEST: swp0: Unicast IPv4 to unknown MAC address, promisc            [ OK ]
TEST: swp0: Unicast IPv4 to unknown MAC address, allmulti           [ OK ]
TEST: swp0: Multicast IPv4 to joined group                          [ OK ]
TEST: swp0: Multicast IPv4 to unknown group                         [ OK ]
TEST: swp0: Multicast IPv4 to unknown group, promisc                [ OK ]
TEST: swp0: Multicast IPv4 to unknown group, allmulti               [ OK ]
TEST: swp0: Multicast IPv6 to joined group                          [ OK ]
TEST: swp0: Multicast IPv6 to unknown group                         [ OK ]
TEST: swp0: Multicast IPv6 to unknown group, promisc                [ OK ]
TEST: swp0: Multicast IPv6 to unknown group, allmulti               [ OK ]

So it's clear that something in the PTP RX trapping logic went wrong.

Looking a bit at the code, I can see that there are 4 typos, which
populate "ipv4" VCAP IS2 key filter fields for IPv6 keys.

VCAP IS2 keys of type OCELOT_VCAP_KEY_IPV4 and OCELOT_VCAP_KEY_IPV6 are
handled by is2_entry_set(). OCELOT_VCAP_KEY_IPV4 looks at
&filter->key.ipv4, and OCELOT_VCAP_KEY_IPV6 at &filter->key.ipv6.
Simply put, when we populate the wrong key field, &filter->key.ipv6
fields "proto.mask" and "proto.value" remain all zeroes (or "don't care").
So is2_entry_set() will enter the "else" of this "if" condition:

	if (msk == 0xff && (val == IPPROTO_TCP || val == IPPROTO_UDP))

and proceed to ignore the "proto" field. The resulting rule will match
on all IPv6 traffic, trapping it to the CPU.

This is the reason why the local_termination.sh selftest sees it,
because control traps are stronger than the PGID_MCIPV6 used for
flooding (from the forwarding data path).

But the problem is in fact much deeper. We trap all IPv6 traffic to the
CPU, but if we're bridged, we set skb->offload_fwd_mark = 1, so software
forwarding will not take place and IPv6 traffic will never reach its
destination.

The fix is simple - correct the typos.

I was intentionally inaccurate in the commit message about the breakage
occurring when any PTP timestamping is enabled. In fact it only happens
when L4 timestamping is requested (HWTSTAMP_FILTER_PTP_V2_EVENT or
HWTSTAMP_FILTER_PTP_V2_L4_EVENT). But ptp4l requests a larger RX
timestamping filter than it needs for "-2": HWTSTAMP_FILTER_PTP_V2_EVENT.
I wanted people skimming through git logs to not think that the bug
doesn't affect them because they only use ptp4l in L2 mode.

Fixes: 96ca08c05838 ("net: mscc: ocelot: set up traps for PTP packets")
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/ethernet/mscc/ocelot_ptp.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Simon Horman Feb. 8, 2023, 11:20 a.m. UTC | #1 
On Tue, Feb 07, 2023 at 08:31:17PM +0200, Vladimir Oltean wrote:
> While running this selftest which usually passes:
> 
> ~/selftests/drivers/net/dsa# ./local_termination.sh eno0 swp0
> TEST: swp0: Unicast IPv4 to primary MAC address                     [ OK ]
> TEST: swp0: Unicast IPv4 to macvlan MAC address                     [ OK ]
> TEST: swp0: Unicast IPv4 to unknown MAC address                     [ OK ]
> TEST: swp0: Unicast IPv4 to unknown MAC address, promisc            [ OK ]
> TEST: swp0: Unicast IPv4 to unknown MAC address, allmulti           [ OK ]
> TEST: swp0: Multicast IPv4 to joined group                          [ OK ]
> TEST: swp0: Multicast IPv4 to unknown group                         [ OK ]
> TEST: swp0: Multicast IPv4 to unknown group, promisc                [ OK ]
> TEST: swp0: Multicast IPv4 to unknown group, allmulti               [ OK ]
> TEST: swp0: Multicast IPv6 to joined group                          [ OK ]
> TEST: swp0: Multicast IPv6 to unknown group                         [ OK ]
> TEST: swp0: Multicast IPv6 to unknown group, promisc                [ OK ]
> TEST: swp0: Multicast IPv6 to unknown group, allmulti               [ OK ]
> 
> if I start PTP timestamping then run it again (debug prints added by me),
> the unknown IPv6 MC traffic is seen by the CPU port even when it should
> have been dropped:
> 
> ~/selftests/drivers/net/dsa# ptp4l -i swp0 -2 -P -m
> ptp4l[225.410]: selected /dev/ptp1 as PTP clock
> [  225.445746] mscc_felix 0000:00:00.5: ocelot_l2_ptp_trap_add: port 0 adding L2 PTP trap
> [  225.453815] mscc_felix 0000:00:00.5: ocelot_ipv4_ptp_trap_add: port 0 adding IPv4 PTP event trap
> [  225.462703] mscc_felix 0000:00:00.5: ocelot_ipv4_ptp_trap_add: port 0 adding IPv4 PTP general trap
> [  225.471768] mscc_felix 0000:00:00.5: ocelot_ipv6_ptp_trap_add: port 0 adding IPv6 PTP event trap
> [  225.480651] mscc_felix 0000:00:00.5: ocelot_ipv6_ptp_trap_add: port 0 adding IPv6 PTP general trap
> ptp4l[225.488]: port 1: INITIALIZING to LISTENING on INIT_COMPLETE
> ptp4l[225.488]: port 0: INITIALIZING to LISTENING on INIT_COMPLETE
> ^C
> ~/selftests/drivers/net/dsa# ./local_termination.sh eno0 swp0
> TEST: swp0: Unicast IPv4 to primary MAC address                     [ OK ]
> TEST: swp0: Unicast IPv4 to macvlan MAC address                     [ OK ]
> TEST: swp0: Unicast IPv4 to unknown MAC address                     [ OK ]
> TEST: swp0: Unicast IPv4 to unknown MAC address, promisc            [ OK ]
> TEST: swp0: Unicast IPv4 to unknown MAC address, allmulti           [ OK ]
> TEST: swp0: Multicast IPv4 to joined group                          [ OK ]
> TEST: swp0: Multicast IPv4 to unknown group                         [ OK ]
> TEST: swp0: Multicast IPv4 to unknown group, promisc                [ OK ]
> TEST: swp0: Multicast IPv4 to unknown group, allmulti               [ OK ]
> TEST: swp0: Multicast IPv6 to joined group                          [ OK ]
> TEST: swp0: Multicast IPv6 to unknown group                         [FAIL]
>         reception succeeded, but should have failed
> TEST: swp0: Multicast IPv6 to unknown group, promisc                [ OK ]
> TEST: swp0: Multicast IPv6 to unknown group, allmulti               [ OK ]
> 
> The PGID_MCIPV6 is configured correctly to not flood to the CPU,
> I checked that.
> 
> Furthermore, when I disable back PTP RX timestamping (ptp4l doesn't do
> that when it exists), packets are RX filtered again as they should be:
> 
> ~/selftests/drivers/net/dsa# hwstamp_ctl -i swp0 -r 0
> [  218.202854] mscc_felix 0000:00:00.5: ocelot_l2_ptp_trap_del: port 0 removing L2 PTP trap
> [  218.212656] mscc_felix 0000:00:00.5: ocelot_ipv4_ptp_trap_del: port 0 removing IPv4 PTP event trap
> [  218.222975] mscc_felix 0000:00:00.5: ocelot_ipv4_ptp_trap_del: port 0 removing IPv4 PTP general trap
> [  218.233133] mscc_felix 0000:00:00.5: ocelot_ipv6_ptp_trap_del: port 0 removing IPv6 PTP event trap
> [  218.242251] mscc_felix 0000:00:00.5: ocelot_ipv6_ptp_trap_del: port 0 removing IPv6 PTP general trap
> current settings:
> tx_type 1
> rx_filter 12
> new settings:
> tx_type 1
> rx_filter 0
> ~/selftests/drivers/net/dsa# ./local_termination.sh eno0 swp0
> TEST: swp0: Unicast IPv4 to primary MAC address                     [ OK ]
> TEST: swp0: Unicast IPv4 to macvlan MAC address                     [ OK ]
> TEST: swp0: Unicast IPv4 to unknown MAC address                     [ OK ]
> TEST: swp0: Unicast IPv4 to unknown MAC address, promisc            [ OK ]
> TEST: swp0: Unicast IPv4 to unknown MAC address, allmulti           [ OK ]
> TEST: swp0: Multicast IPv4 to joined group                          [ OK ]
> TEST: swp0: Multicast IPv4 to unknown group                         [ OK ]
> TEST: swp0: Multicast IPv4 to unknown group, promisc                [ OK ]
> TEST: swp0: Multicast IPv4 to unknown group, allmulti               [ OK ]
> TEST: swp0: Multicast IPv6 to joined group                          [ OK ]
> TEST: swp0: Multicast IPv6 to unknown group                         [ OK ]
> TEST: swp0: Multicast IPv6 to unknown group, promisc                [ OK ]
> TEST: swp0: Multicast IPv6 to unknown group, allmulti               [ OK ]
> 
> So it's clear that something in the PTP RX trapping logic went wrong.
> 
> Looking a bit at the code, I can see that there are 4 typos, which
> populate "ipv4" VCAP IS2 key filter fields for IPv6 keys.
> 
> VCAP IS2 keys of type OCELOT_VCAP_KEY_IPV4 and OCELOT_VCAP_KEY_IPV6 are
> handled by is2_entry_set(). OCELOT_VCAP_KEY_IPV4 looks at
> &filter->key.ipv4, and OCELOT_VCAP_KEY_IPV6 at &filter->key.ipv6.
> Simply put, when we populate the wrong key field, &filter->key.ipv6
> fields "proto.mask" and "proto.value" remain all zeroes (or "don't care").
> So is2_entry_set() will enter the "else" of this "if" condition:
> 
> 	if (msk == 0xff && (val == IPPROTO_TCP || val == IPPROTO_UDP))
> 
> and proceed to ignore the "proto" field. The resulting rule will match
> on all IPv6 traffic, trapping it to the CPU.
> 
> This is the reason why the local_termination.sh selftest sees it,
> because control traps are stronger than the PGID_MCIPV6 used for
> flooding (from the forwarding data path).
> 
> But the problem is in fact much deeper. We trap all IPv6 traffic to the
> CPU, but if we're bridged, we set skb->offload_fwd_mark = 1, so software
> forwarding will not take place and IPv6 traffic will never reach its
> destination.
> 
> The fix is simple - correct the typos.
> 
> I was intentionally inaccurate in the commit message about the breakage
> occurring when any PTP timestamping is enabled. In fact it only happens
> when L4 timestamping is requested (HWTSTAMP_FILTER_PTP_V2_EVENT or
> HWTSTAMP_FILTER_PTP_V2_L4_EVENT). But ptp4l requests a larger RX
> timestamping filter than it needs for "-2": HWTSTAMP_FILTER_PTP_V2_EVENT.
> I wanted people skimming through git logs to not think that the bug
> doesn't affect them because they only use ptp4l in L2 mode.
> 
> Fixes: 96ca08c05838 ("net: mscc: ocelot: set up traps for PTP packets")
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Reviewed-by: Simon Horman <simon.horman@corigine.com>
patchwork-bot+netdevbpf@kernel.org Feb. 9, 2023, 10 a.m. UTC | #2 
Hello:

This patch was applied to netdev/net.git (master)
by Paolo Abeni <pabeni@redhat.com>:

On Tue,  7 Feb 2023 20:31:17 +0200 you wrote:
> While running this selftest which usually passes:
> 
> ~/selftests/drivers/net/dsa# ./local_termination.sh eno0 swp0
> TEST: swp0: Unicast IPv4 to primary MAC address                     [ OK ]
> TEST: swp0: Unicast IPv4 to macvlan MAC address                     [ OK ]
> TEST: swp0: Unicast IPv4 to unknown MAC address                     [ OK ]
> TEST: swp0: Unicast IPv4 to unknown MAC address, promisc            [ OK ]
> TEST: swp0: Unicast IPv4 to unknown MAC address, allmulti           [ OK ]
> TEST: swp0: Multicast IPv4 to joined group                          [ OK ]
> TEST: swp0: Multicast IPv4 to unknown group                         [ OK ]
> TEST: swp0: Multicast IPv4 to unknown group, promisc                [ OK ]
> TEST: swp0: Multicast IPv4 to unknown group, allmulti               [ OK ]
> TEST: swp0: Multicast IPv6 to joined group                          [ OK ]
> TEST: swp0: Multicast IPv6 to unknown group                         [ OK ]
> TEST: swp0: Multicast IPv6 to unknown group, promisc                [ OK ]
> TEST: swp0: Multicast IPv6 to unknown group, allmulti               [ OK ]
> 
> [...]

Here is the summary with links:
  - [net] net: mscc: ocelot: fix all IPv6 getting trapped to CPU when PTP timestamping is used
    https://git.kernel.org/netdev/net/c/2fcde9fe258e

You are awesome, thank you!
diff mbox series

Patch

diff --git a/drivers/net/ethernet/mscc/ocelot_ptp.c b/drivers/net/ethernet/mscc/ocelot_ptp.c
index 1a82f10c8853..2180ae94c744 100644
--- a/drivers/net/ethernet/mscc/ocelot_ptp.c
+++ b/drivers/net/ethernet/mscc/ocelot_ptp.c
@@ -335,8 +335,8 @@  static void
 ocelot_populate_ipv6_ptp_event_trap_key(struct ocelot_vcap_filter *trap)
 {
 	trap->key_type = OCELOT_VCAP_KEY_IPV6;
-	trap->key.ipv4.proto.value[0] = IPPROTO_UDP;
-	trap->key.ipv4.proto.mask[0] = 0xff;
+	trap->key.ipv6.proto.value[0] = IPPROTO_UDP;
+	trap->key.ipv6.proto.mask[0] = 0xff;
 	trap->key.ipv6.dport.value = PTP_EV_PORT;
 	trap->key.ipv6.dport.mask = 0xffff;
 }
@@ -355,8 +355,8 @@  static void
 ocelot_populate_ipv6_ptp_general_trap_key(struct ocelot_vcap_filter *trap)
 {
 	trap->key_type = OCELOT_VCAP_KEY_IPV6;
-	trap->key.ipv4.proto.value[0] = IPPROTO_UDP;
-	trap->key.ipv4.proto.mask[0] = 0xff;
+	trap->key.ipv6.proto.value[0] = IPPROTO_UDP;
+	trap->key.ipv6.proto.mask[0] = 0xff;
 	trap->key.ipv6.dport.value = PTP_GEN_PORT;
 	trap->key.ipv6.dport.mask = 0xffff;
 }