| Message ID | 20231101200217.121789-1-rrameshbabu@nvidia.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] macsec: Abort MACSec Rx offload datapath when skb is not marked with MACSec metadata | expand |
2023-11-01, 13:02:17 -0700, Rahul Rameshbabu wrote: > 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. 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?
On Wed, 01 Nov, 2023 23:31:33 +0100 Sabrina Dubroca <sd@queasysnail.net> wrote: > 2023-11-01, 13:02:17 -0700, Rahul Rameshbabu wrote: >> 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. > > 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? 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
2023-11-06, 14:15:11 -0800, Rahul Rameshbabu wrote: > However, I believe that all macsec offload supporting devices run into > the following problem today (including mlx5 devices). If that's the case, we have to fix all of them. > 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. [...] > 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 Which packet gets dropped and why? Where? I don't understand how the vlan makes a difference in a packet targeting the lower device. > I am thinking the solution is a combination of annotating which macsec > devices support md_dst and this patch. 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. > 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). Yeah, anything relying on md_dst is not going to help the rest of the drivers.
On Wed, 15 Nov, 2023 16:19:27 +0100 Sabrina Dubroca <sd@queasysnail.net> wrote: > 2023-11-06, 14:15:11 -0800, Rahul Rameshbabu wrote: >> However, I believe that all macsec offload supporting devices run into >> the following problem today (including mlx5 devices). > > If that's the case, we have to fix all of them. 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. > >> 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. > [...] >> 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 > > Which packet gets dropped and why? Where? I don't understand how the > vlan makes a difference in a packet targeting the lower device. > >> I am thinking the solution is a combination of annotating which macsec >> devices support md_dst and this patch. > > 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. > This is exactly what the RFC patch series I am proposing looks like. >> 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). > > Yeah, anything relying on md_dst is not going to help the rest of the > drivers. 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
diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c index 9663050a852d..102200ce87d3 100644 --- a/drivers/net/macsec.c +++ b/drivers/net/macsec.c @@ -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,
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. Fixes: 860ead89b851 ("net/macsec: Add MACsec skb_metadata_dst Rx Data path support") Cc: Sabrina Dubroca <sd@queasysnail.net> Signed-off-by: Rahul Rameshbabu <rrameshbabu@nvidia.com> Reviewed-by: Saeed Mahameed <saeed@kernel.org> --- drivers/net/macsec.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)