macsec: Abort MACSec Rx offload datapath when skb is not marked with MACSec metadata

When MACSec is configured on an outer netdev, traffic received directly through the underlying netdev should not be processed by the MACSec Rx datapath. When using MACSec offload on an outer netdev, traffic with no metadata indicator in the skb is mistakenly considered as MACSec traffic and incorrectly handled in the handle_not_macsec function. Treat skbs with no metadata type as non-MACSec packets rather than assuming they are MACSec packets.

---
 drivers/net/macsec.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

index 9663050a852d..102200ce87d3 100644
@@ -996,10 +996,12 @@ static enum rx_handler_result handle_not_macsec(struct sk_buff *skb)
 	struct metadata_dst *md_dst;
 	struct macsec_rxh_data *rxd;
 	struct macsec_dev *macsec;
+	bool is_macsec_md_skb;
 
 	rcu_read_lock();
 	rxd = macsec_data_rcu(skb->dev);
 	md_dst = skb_metadata_dst(skb);
+	is_macsec_md_skb = !md_dst || md_dst->type != METADATA_MACSEC;
 
 	list_for_each_entry_rcu(macsec, &rxd->secys, secys) {
 		struct sk_buff *nskb;

@@ -1012,10 +1014,11 @@ static enum rx_handler_result handle_not_macsec(struct sk_buff *skb)
 		if (macsec_is_offloaded(macsec) && netif_running(ndev)) {
 			struct macsec_rx_sc *rx_sc = NULL;
 
-			if (md_dst && md_dst->type == METADATA_MACSEC)
-				rx_sc = find_rx_sc(&macsec->secy, md_dst->u.macsec_info.sci);
+			if (is_macsec_md_skb)
+				continue;
 
-			if (md_dst && md_dst->type == METADATA_MACSEC && !rx_sc)
+			rx_sc = find_rx_sc(&macsec->secy, md_dst->u.macsec_info.sci);
+			if (!rx_sc)
 				continue;
 
 			if (ether_addr_equal_64bits(hdr->h_dest,

2.40.1

What about the other drivers? mlx5 is the only driver that sets md_dst on its macsec skbs, so their offloaded packets just get dropped now?

Sabrina

After taking a deeper look throughout the tree, I realize there are macsec offloading drivers that do not set md_dst. In this event, we fail to correctly handle the skb and deliver to the port as you mentioned previously. Sorry about this miss on my end.

However, I believe that all macsec offload supporting devices run into the following problem today (including mlx5 devices).

When I configure macsec offload on a device and then vlan on top of the macsec interface, I become unable to send traffic through the underlying device.

(can replace mlx5_1 with any ifname of a device that supports macsec offload)

Side 1

ip link del macsec0
ip address flush mlx5_1
ip address add 1.1.1.1/24 dev mlx5_1
ip link set dev mlx5_1 up
ip link add link mlx5_1 macsec0 type macsec sci 1 encrypt on
ip link set dev macsec0 address 00:11:22:33:44:66
ip macsec offload macsec0 mac
ip macsec add macsec0 tx sa 0 pn 1 on key 00 dffafc8d7b9a43d5b9a3dfbbf6a30c16 ip macsec add macsec0 rx sci 2 on
ip macsec add macsec0 rx sci 2 sa 0 pn 1 on key 00 ead3664f508eb06c40ac7104cdae4ce5 ip address flush macsec0
ip address add 2.2.2.1/24 dev macsec0
ip link set dev macsec0 up
ip link add link macsec0 name macsec_vlan type vlan id 1
ip link set dev macsec_vlan address 00:11:22:33:44:88
ip address flush macsec_vlan
ip address add 3.3.3.1/24 dev macsec_vlan
ip link set dev macsec_vlan up

Side 2

ip link del macsec0
ip address flush mlx5_1
ip address add 1.1.1.2/24 dev mlx5_1
ip link set dev mlx5_1 up
ip link add link mlx5_1 macsec0 type macsec sci 2 encrypt on
ip link set dev macsec0 address 00:11:22:33:44:77
ip macsec offload macsec0 mac
ip macsec add macsec0 tx sa 0 pn 1 on key 00 ead3664f508eb06c40ac7104cdae4ce5 ip macsec add macsec0 rx sci 1 on
ip macsec add macsec0 rx sci 1 sa 0 pn 1 on key 00 dffafc8d7b9a43d5b9a3dfbbf6a30c16 ip address flush macsec0
ip address add 2.2.2.2/24 dev macsec0
ip link set dev macsec0 up
ip link add link macsec0 name macsec_vlan type vlan id 1
ip link set dev macsec_vlan address 00:11:22:33:44:99
ip address flush macsec_vlan
ip address add 3.3.3.2/24 dev macsec_vlan
ip link set dev macsec_vlan up

Side 1

ping -I mlx5_1 1.1.1.2
PING 1.1.1.2 (1.1.1.2) from 1.1.1.1 mlx5_1: 56(84) bytes of data. From 1.1.1.1 icmp_seq=1 Destination Host Unreachable
ping: sendmsg: No route to host
From 1.1.1.1 icmp_seq=2 Destination Host Unreachable From 1.1.1.1 icmp_seq=3 Destination Host Unreachable

I am thinking the solution is a combination of annotating which macsec devices support md_dst and this patch. However, I am not sure this fix would be helpful for devices that support macsec offload without utilizing md_dst information (would still be problematic).

Would be glad to hear any suggestions you may have before sending out a v2.

Sorry for the late reply. Was away from a trip and wanted to validate some things before posting back.

--
Thanks,

Rahul Rameshbabu

If that's the case, we have to fix all of them.

[...]

Which packet gets dropped and why? Where? I don't understand how the vlan makes a difference in a packet targeting the lower device.

Yes, if we know that the offloading device sets md_dst on all its offloaded packets, we can just look up the rx_sc based on the sci and be done, or pass the packet directly to the real device if md_dst wasn't provided. No need to go through the MAC address matching at all.

Yeah, anything relying on md_dst is not going to help the rest of the drivers.

Sabrina

I have an RFC patch series undergoing internal review for supporting devices that advertise md_dst for handling non-MACsec multicast traffic sent to the port. In the cover letter, I describe why I do not believe the same case can be trivially handled for devices that do not support setting md_dst since there is no way of knowing whether the traffic received on the port was MACsec or not.

This is exactly what the RFC patch series I am proposing looks like.

Right, I have an example in the cover letter that elaborates on why I do not think it's possible to support this on devices that do not set md_dst. I think the existing handling for multicast messages and promiscuous mode in handle_not_macsec is the most appropriate for drivers unable to set md_dst to indicate whether traffic received on the port is MACsec traffic that will be handled by the NIC's offloading functionality. Let me try to get that RFC out on the list soon for your review.

--
Thanks,

Rahul Rameshbabu