2024-04-18, 18:17:16 -0700, Rahul Rameshbabu wrote:
diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c index 0206b84284ab..679302ef1cd9 100644 --- a/drivers/net/macsec.c +++ b/drivers/net/macsec.c @@ -991,6 +991,19 @@ static struct macsec_rx_sc *find_rx_sc_rtnl(struct macsec_secy *secy, sci_t sci) return NULL; } +static __u8 macsec_offload_pkt_type(const u8 *h_dest, const u8 *ndev_broadcast)
nit: empty line shouldn't be here
+{
- if (is_multicast_ether_addr_64bits(h_dest)) {
if (ether_addr_equal_64bits(h_dest, ndev_broadcast))return PACKET_BROADCAST;elsereturn PACKET_MULTICAST;- }
- return PACKET_HOST;
+}
static enum rx_handler_result handle_not_macsec(struct sk_buff *skb) { /* Deliver to the uncontrolled port by default */ @@ -999,10 +1012,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_dst;
rcu_read_lock(); rxd = macsec_data_rcu(skb->dev); md_dst = skb_metadata_dst(skb);
- is_macsec_md_dst = md_dst && md_dst->type == METADATA_MACSEC;
list_for_each_entry_rcu(macsec, &rxd->secys, secys) { struct sk_buff *nskb; @@ -1014,13 +1029,40 @@ 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;
Please move this into the "if (is_macsec_md_dst)" block below, since it's no longer used outside.
const struct macsec_ops *ops;
if (md_dst && md_dst->type == METADATA_MACSEC)rx_sc = find_rx_sc(&macsec->secy, md_dst->u.macsec_info.sci);
ops = macsec_get_ops(macsec, NULL);
if (md_dst && md_dst->type == METADATA_MACSEC && !rx_sc)
if (ops->rx_uses_md_dst && !is_macsec_md_dst) continue;
if (is_macsec_md_dst) {/* All drivers that implement MACsec offload* support using skb metadata destinations must* indicate that they do so.*/DEBUG_NET_WARN_ON_ONCE(!ops->rx_uses_md_dst);rx_sc = find_rx_sc(&macsec->secy, md_dst->u.macsec_info.sci);if (!rx_sc)continue;/* device indicated macsec offload occurred */skb->dev = ndev;skb->pkt_type = macsec_offload_pkt_type(hdr->h_dest, ndev->broadcast);ret = RX_HANDLER_ANOTHER;goto out;}/* This datapath is insecure because it is unable to* enforce isolation of broadcast/multicast traffic and* unicast traffic with promiscuous mode on the macsec* netdev. Since the core stack has no mechanism to* check that the hardware did indeed receive MACsec* traffic, it is possible that the response handling* done by the MACsec port was to a plaintext packet.* This violates the MACsec protocol standard.*/DEBUG_NET_WARN_ON_ONCE(true);
If you insist on this warning (and I'm not convinced it's useful, since if the HW is already built and cannot inform the driver, there's nothing the driver implementer can do), I would move it somewhere into the config path. macsec_update_offload would be a better location for this kind of warning (maybe with a pr_warn (not limited to debug configs) saying something like "MACsec offload on devices that don't support md_dst are insecure: they do not provide proper isolation of traffic"). The comment can stay here.
if (ether_addr_equal_64bits(hdr->h_dest, ndev->dev_addr)) { /* exact match, divert skb to this port */