lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <ZDbHI/VLKkGib3kQ@hog> Date: Wed, 12 Apr 2023 16:58:43 +0200 From: Sabrina Dubroca <sd@...asysnail.net> To: Emeel Hakim <ehakim@...dia.com> Cc: davem@...emloft.net, kuba@...nel.org, pabeni@...hat.com, edumazet@...gle.com, netdev@...r.kernel.org, leon@...nel.org Subject: Re: [PATCH net-next v4 5/5] macsec: Add MACsec rx_handler change support 2023-04-08, 13:57:35 +0300, Emeel Hakim wrote: > Offloading device drivers will mark offloaded MACsec SKBs with the > corresponding SCI in the skb_metadata_dst so the macsec rx handler will > know to which interface to divert those skbs, in case of a marked skb > and a mismatch on the dst MAC address, divert the skb to the macsec > net_device where the macsec rx_handler will be called. Quoting my reply to v2: ======== Sorry, I don't understand what you're trying to say here and in the subject line. To me, "Add MACsec rx_handler change support" sounds like you're changing what function is used as ->rx_handler, which is not what this patch is doing. ======== > Example of such a case is having a MACsec with VLAN as an inner header > ETHERNET | SECTAG | VLAN packet. > > Signed-off-by: Emeel Hakim <ehakim@...dia.com> > --- > drivers/net/macsec.c | 16 ++++++++++++++-- > 1 file changed, 14 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c > index 25616247d7a5..4e58d2b4f0e1 100644 > --- a/drivers/net/macsec.c > +++ b/drivers/net/macsec.c > @@ -1016,14 +1016,18 @@ static enum rx_handler_result handle_not_macsec(struct sk_buff *skb) > struct sk_buff *nskb; > struct pcpu_secy_stats *secy_stats = this_cpu_ptr(macsec->stats); > struct net_device *ndev = macsec->secy.netdev; > + struct macsec_rx_sc *rx_sc_found = NULL; I don't think "_found" is adding any information. "rx_sc" is enough. And since it's only used in the if block below, it could be defined down there. And btw I don't think we even need to check "&& rx_sc_found" in the code you're adding, but maybe I need more coffee. Anyway, I'd be fine with saving the result of find_rx_sc and reusing it. if (A && !rx_sc) continue; [...] if (A) // here we know rx_sc can't be NULL, otherwise we would have hit the continue earlier packet_host etc > /* If h/w offloading is enabled, HW decodes frames and strips > * the SecTAG, so we have to deduce which port to deliver to. > */ > if (macsec_is_offloaded(macsec) && netif_running(ndev)) { > - if (md_dst && md_dst->type == METADATA_MACSEC && > - (!find_rx_sc(&macsec->secy, md_dst->u.macsec_info.sci))) > + rx_sc_found = (md_dst && md_dst->type == METADATA_MACSEC) ? > + find_rx_sc(&macsec->secy, md_dst->u.macsec_info.sci) : NULL; > + > + if (md_dst && md_dst->type == METADATA_MACSEC && !rx_sc_found) { > continue; > + } {} not needed around a single line. > > if (ether_addr_equal_64bits(hdr->h_dest, > ndev->dev_addr)) { > @@ -1048,6 +1052,14 @@ static enum rx_handler_result handle_not_macsec(struct sk_buff *skb) > > __netif_rx(nskb); > } > + > + if (md_dst && md_dst->type == METADATA_MACSEC && rx_sc_found) { > + skb->dev = ndev; > + skb->pkt_type = PACKET_HOST; > + ret = RX_HANDLER_ANOTHER; > + goto out; > + } > + > continue; > } > > -- > 2.21.3 > -- Sabrina
Powered by blists - more mailing lists