| 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: <658c46269aa52_a33e629442@willemb.c.googlers.com.notmuch>
Date: Wed, 27 Dec 2023 10:43:34 -0500
From: Willem de Bruijn <willemdebruijn.kernel@...il.com>
To: Alexander Lobakin <aleksander.lobakin@...el.com>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>
Cc: Alexander Lobakin <aleksander.lobakin@...el.com>,
Maciej Fijalkowski <maciej.fijalkowski@...el.com>,
Michal Kubiak <michal.kubiak@...el.com>,
Larysa Zaremba <larysa.zaremba@...el.com>,
Alexei Starovoitov <ast@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>,
Willem de Bruijn <willemdebruijn.kernel@...il.com>,
intel-wired-lan@...ts.osuosl.org,
netdev@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH RFC net-next 01/34] idpf: reuse libie's definitions of
parsed ptype structures
Alexander Lobakin wrote:
> idpf's in-kernel parsed ptype structure is almost identical to the one
> used in the previous Intel drivers, which means it can be converted to
> use libie's definitions and even helpers. The only difference is that
> it doesn't use a constant table, rather than one obtained from the
> device.
> Remove the driver counterpart and use libie's helpers for hashes and
> checksums. This slightly optimizes skb fields processing due to faster
> checks.
>
> Signed-off-by: Alexander Lobakin <aleksander.lobakin@...el.com>
> ---
> drivers/net/ethernet/intel/Kconfig | 1 +
> drivers/net/ethernet/intel/idpf/idpf.h | 2 +-
> drivers/net/ethernet/intel/idpf/idpf_main.c | 1 +
> .../ethernet/intel/idpf/idpf_singleq_txrx.c | 87 +++++++--------
> drivers/net/ethernet/intel/idpf/idpf_txrx.c | 101 ++++++------------
> drivers/net/ethernet/intel/idpf/idpf_txrx.h | 88 +--------------
> .../net/ethernet/intel/idpf/idpf_virtchnl.c | 54 ++++++----
> 7 files changed, 110 insertions(+), 224 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/Kconfig b/drivers/net/ethernet/intel/Kconfig
> index c7da7d05d93e..0db1aa36866e 100644
> --- a/drivers/net/ethernet/intel/Kconfig
> +++ b/drivers/net/ethernet/intel/Kconfig
> @@ -378,6 +378,7 @@ config IDPF
> tristate "Intel(R) Infrastructure Data Path Function Support"
> depends on PCI_MSI
> select DIMLIB
> + select LIBIE
> select PAGE_POOL
> select PAGE_POOL_STATS
> help
> diff --git a/drivers/net/ethernet/intel/idpf/idpf.h b/drivers/net/ethernet/intel/idpf/idpf.h
> index 0acc125decb3..8342df0f4f3d 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf.h
> +++ b/drivers/net/ethernet/intel/idpf/idpf.h
> @@ -385,7 +385,7 @@ struct idpf_vport {
> u16 num_rxq_grp;
> struct idpf_rxq_group *rxq_grps;
> u32 rxq_model;
> - struct idpf_rx_ptype_decoded rx_ptype_lkup[IDPF_RX_MAX_PTYPE];
> + struct libie_rx_ptype_parsed rx_ptype_lkup[IDPF_RX_MAX_PTYPE];
>
> struct idpf_adapter *adapter;
> struct net_device *netdev;
> diff --git a/drivers/net/ethernet/intel/idpf/idpf_main.c b/drivers/net/ethernet/intel/idpf/idpf_main.c
> index e1febc74cefd..6471158e6f6b 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_main.c
> +++ b/drivers/net/ethernet/intel/idpf/idpf_main.c
> @@ -7,6 +7,7 @@
> #define DRV_SUMMARY "Intel(R) Infrastructure Data Path Function Linux Driver"
>
> MODULE_DESCRIPTION(DRV_SUMMARY);
> +MODULE_IMPORT_NS(LIBIE);
> MODULE_LICENSE("GPL");
>
> /**
> diff --git a/drivers/net/ethernet/intel/idpf/idpf_singleq_txrx.c b/drivers/net/ethernet/intel/idpf/idpf_singleq_txrx.c
> index 8122a0cc97de..e58e08c9997d 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_singleq_txrx.c
> +++ b/drivers/net/ethernet/intel/idpf/idpf_singleq_txrx.c
> @@ -636,75 +636,64 @@ static bool idpf_rx_singleq_is_non_eop(struct idpf_queue *rxq,
> * @rxq: Rx ring being processed
> * @skb: skb currently being received and modified
> * @csum_bits: checksum bits from descriptor
> - * @ptype: the packet type decoded by hardware
> + * @parsed: the packet type parsed by hardware
> *
> * skb->protocol must be set before this function is called
> */
> static void idpf_rx_singleq_csum(struct idpf_queue *rxq, struct sk_buff *skb,
> - struct idpf_rx_csum_decoded *csum_bits,
> - u16 ptype)
> + struct idpf_rx_csum_decoded csum_bits,
> + struct libie_rx_ptype_parsed parsed)
> {
> - struct idpf_rx_ptype_decoded decoded;
> bool ipv4, ipv6;
>
> /* check if Rx checksum is enabled */
> - if (unlikely(!(rxq->vport->netdev->features & NETIF_F_RXCSUM)))
> + if (!libie_has_rx_checksum(rxq->vport->netdev, parsed))
> return;
>
> /* check if HW has decoded the packet and checksum */
> - if (unlikely(!(csum_bits->l3l4p)))
> + if (unlikely(!csum_bits.l3l4p))
> return;
>
> - decoded = rxq->vport->rx_ptype_lkup[ptype];
> - if (unlikely(!(decoded.known && decoded.outer_ip)))
> + if (unlikely(parsed.outer_ip == LIBIE_RX_PTYPE_OUTER_L2))
> return;
>
> - ipv4 = IDPF_RX_PTYPE_TO_IPV(&decoded, IDPF_RX_PTYPE_OUTER_IPV4);
> - ipv6 = IDPF_RX_PTYPE_TO_IPV(&decoded, IDPF_RX_PTYPE_OUTER_IPV6);
> + ipv4 = parsed.outer_ip == LIBIE_RX_PTYPE_OUTER_IPV4;
> + ipv6 = parsed.outer_ip == LIBIE_RX_PTYPE_OUTER_IPV6;
>
> /* Check if there were any checksum errors */
> - if (unlikely(ipv4 && (csum_bits->ipe || csum_bits->eipe)))
> + if (unlikely(ipv4 && (csum_bits.ipe || csum_bits.eipe)))
> goto checksum_fail;
>
> /* Device could not do any checksum offload for certain extension
> * headers as indicated by setting IPV6EXADD bit
> */
> - if (unlikely(ipv6 && csum_bits->ipv6exadd))
> + if (unlikely(ipv6 && csum_bits.ipv6exadd))
> return;
>
> /* check for L4 errors and handle packets that were not able to be
> * checksummed due to arrival speed
> */
> - if (unlikely(csum_bits->l4e))
> + if (unlikely(csum_bits.l4e))
> goto checksum_fail;
>
> - if (unlikely(csum_bits->nat && csum_bits->eudpe))
> + if (unlikely(csum_bits.nat && csum_bits.eudpe))
> goto checksum_fail;
>
> /* Handle packets that were not able to be checksummed due to arrival
> * speed, in this case the stack can compute the csum.
> */
> - if (unlikely(csum_bits->pprs))
> + if (unlikely(csum_bits.pprs))
> return;
>
> /* If there is an outer header present that might contain a checksum
> * we need to bump the checksum level by 1 to reflect the fact that
> * we are indicating we validated the inner checksum.
> */
> - if (decoded.tunnel_type >= IDPF_RX_PTYPE_TUNNEL_IP_GRENAT)
> + if (parsed.tunnel_type >= LIBIE_RX_PTYPE_TUNNEL_IP_GRENAT)
> skb->csum_level = 1;
>
> - /* Only report checksum unnecessary for ICMP, TCP, UDP, or SCTP */
> - switch (decoded.inner_prot) {
> - case IDPF_RX_PTYPE_INNER_PROT_ICMP:
> - case IDPF_RX_PTYPE_INNER_PROT_TCP:
> - case IDPF_RX_PTYPE_INNER_PROT_UDP:
> - case IDPF_RX_PTYPE_INNER_PROT_SCTP:
> - skb->ip_summed = CHECKSUM_UNNECESSARY;
> - return;
> - default:
> - return;
> - }
> + skb->ip_summed = CHECKSUM_UNNECESSARY;
> + return;
Is it intentional to change from CHECKSUM_NONE to CHECKSUM_UNNECESSARY
in the default case?
I suppose so, as idpf_rx_csum (the splitq equivalent) does the same
(bar CHECKSUM_COMPLETE depending on descriptor bit).
Powered by blists - more mailing lists