[Intel-wired-lan] [PATCH iwl-next v1] ice: Add E830 checksum support
Tony Nguyen
anthony.l.nguyen at intel.com
Thu Aug 29 21:03:44 UTC 2024
On 8/26/2024 10:39 AM, Paul Greenwalt wrote:
> E830 supports generic receive and HW_CSUM transmit checksumming.
>
> Generic receive checksum support is provided by hardware calculating the
> checksum over the whole packet and providing it to the driver in the Rx
> flex descriptor. Then the driver assigns the checksum to skb-->csum and
> sets skb->ip_summed to CHECKSUM_COMPLETE.
>
> E830 HW_CSUM transmit checksum does not support TCP Segmentation Offload
> (TSO) inner packet modification, so TSO and HW_CSUM are mutually exclusive.
> Therefore NETIF_F_HW_CSUM hardware feature support is indicated but is not
> enabled by default. Instead, IP checksum and TSO are the defaults.
> Enforcement of mutual exclusivity of TSO and HW_CSUM is done in
> ice_fix_features(). Mutual exclusivity of IP checksum and HW_CSUM is
> handled by netdev_fix_features().
>
> When NETIF_F_HW_CSUM is requested the provide skb->csum_start and
> skb->csum_offset are passed to hardware in the Tx context descriptor
> generic checksum (GCS) parameters. Hardware calculates the 1's complement
> from skb->csum_start to the end of the packet, and inserts the result in
> the packet at skb->csum_offset.
>
> Co-developed-by: Alice Michael <alice.michael at intel.com>
> Signed-off-by: Alice Michael <alice.michael at intel.com>
> Co-developed-by: Eric Joyner <eric.joyner at intel.com>
> Signed-off-by: Eric Joyner <eric.joyner at intel.com>
> Signed-off-by: Paul Greenwalt <paul.greenwalt at intel.com>
> ---
> drivers/net/ethernet/intel/ice/ice.h | 1 +
> .../net/ethernet/intel/ice/ice_hw_autogen.h | 1 +
> .../net/ethernet/intel/ice/ice_lan_tx_rx.h | 8 +++--
> drivers/net/ethernet/intel/ice/ice_lib.c | 9 +++++
> drivers/net/ethernet/intel/ice/ice_main.c | 30 +++++++++++++++++
> drivers/net/ethernet/intel/ice/ice_txrx.c | 26 ++++++++++++++-
> drivers/net/ethernet/intel/ice/ice_txrx.h | 2 ++
> drivers/net/ethernet/intel/ice/ice_txrx_lib.c | 33 +++++++++++++++++++
> 8 files changed, 107 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ice/ice.h b/drivers/net/ethernet/intel/ice/ice.h
> index caaa10157909..70e2cf8825cc 100644
> --- a/drivers/net/ethernet/intel/ice/ice.h
> +++ b/drivers/net/ethernet/intel/ice/ice.h
> @@ -205,6 +205,7 @@ enum ice_feature {
> ICE_F_SMA_CTRL,
> ICE_F_CGU,
> ICE_F_GNSS,
> + ICE_F_GCS,
> ICE_F_ROCE_LAG,
> ICE_F_SRIOV_LAG,
> ICE_F_MAX
> diff --git a/drivers/net/ethernet/intel/ice/ice_hw_autogen.h b/drivers/net/ethernet/intel/ice/ice_hw_autogen.h
> index 91cbae1eec89..3128806e1cc6 100644
> --- a/drivers/net/ethernet/intel/ice/ice_hw_autogen.h
> +++ b/drivers/net/ethernet/intel/ice/ice_hw_autogen.h
> @@ -110,6 +110,7 @@
> #define PRTDCB_TUP2TC 0x001D26C0
> #define GL_PREEXT_L2_PMASK0(_i) (0x0020F0FC + ((_i) * 4))
> #define GL_PREEXT_L2_PMASK1(_i) (0x0020F108 + ((_i) * 4))
> +#define GL_RDPU_CNTRL 0x00052054
> #define GLFLXP_RXDID_FLAGS(_i, _j) (0x0045D000 + ((_i) * 4 + (_j) * 256))
> #define GLFLXP_RXDID_FLAGS_FLEXIFLAG_4N_S 0
> #define GLFLXP_RXDID_FLAGS_FLEXIFLAG_4N_M ICE_M(0x3F, 0)
> diff --git a/drivers/net/ethernet/intel/ice/ice_lan_tx_rx.h b/drivers/net/ethernet/intel/ice/ice_lan_tx_rx.h
> index 611577ebc29d..759a7c6f72bd 100644
> --- a/drivers/net/ethernet/intel/ice/ice_lan_tx_rx.h
> +++ b/drivers/net/ethernet/intel/ice/ice_lan_tx_rx.h
> @@ -229,7 +229,7 @@ struct ice_32b_rx_flex_desc_nic {
> __le16 status_error1;
> u8 flexi_flags2;
> u8 ts_low;
> - __le16 l2tag2_1st;
> + __le16 raw_csum;
> __le16 l2tag2_2nd;
>
> /* Qword 3 */
> @@ -500,10 +500,14 @@ enum ice_tx_desc_len_fields {
> struct ice_tx_ctx_desc {
> __le32 tunneling_params;
> __le16 l2tag2;
> - __le16 rsvd;
> + __le16 gcs;
> __le64 qw1;
> };
>
> +#define ICE_TX_GCS_DESC_START 0 /* 8 BITS */
> +#define ICE_TX_GCS_DESC_OFFSET 8 /* 4 BITS */
> +#define ICE_TX_GCS_DESC_TYPE 12 /* 3 BITS */
> +
> #define ICE_TXD_CTX_QW1_CMD_S 4
> #define ICE_TXD_CTX_QW1_CMD_M (0x7FUL << ICE_TXD_CTX_QW1_CMD_S)
>
> diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c
> index f559e60992fa..118ac34f89e9 100644
> --- a/drivers/net/ethernet/intel/ice/ice_lib.c
> +++ b/drivers/net/ethernet/intel/ice/ice_lib.c
> @@ -1380,6 +1380,9 @@ static int ice_vsi_alloc_rings(struct ice_vsi *vsi)
> ring->flags |= ICE_TX_FLAGS_RING_VLAN_L2TAG2;
> else
> ring->flags |= ICE_TX_FLAGS_RING_VLAN_L2TAG1;
> +
> + if (ice_is_feature_supported(pf, ICE_F_GCS))
> + ring->flags |= ICE_TXRX_FLAGS_GCS_ENA;
> WRITE_ONCE(vsi->tx_rings[i], ring);
> }
>
> @@ -1399,6 +1402,9 @@ static int ice_vsi_alloc_rings(struct ice_vsi *vsi)
> ring->dev = dev;
> ring->count = vsi->num_rx_desc;
> ring->cached_phctime = pf->ptp.cached_phc_time;
> +
> + if (ice_is_feature_supported(pf, ICE_F_GCS))
> + ring->flags |= ICE_TXRX_FLAGS_GCS_ENA;
> WRITE_ONCE(vsi->rx_rings[i], ring);
> }
>
> @@ -3896,6 +3902,9 @@ void ice_init_feature_support(struct ice_pf *pf)
> default:
> break;
> }
> +
> + if (pf->hw.mac_type == ICE_MAC_E830)
> + ice_set_feature_support(pf, ICE_F_GCS);
> }
>
> /**
> diff --git a/drivers/net/ethernet/intel/ice/ice_main.c b/drivers/net/ethernet/intel/ice/ice_main.c
> index 6f97ed471fe9..67e32ac962df 100644
> --- a/drivers/net/ethernet/intel/ice/ice_main.c
> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
> @@ -3678,6 +3678,12 @@ static void ice_set_netdev_features(struct net_device *netdev)
> */
> netdev->hw_features |= NETIF_F_RXFCS;
>
> + /* Mutual exclusivity for TSO and GCS is enforced by the
> + * ice_fix_features() ndo callback.
> + */
> + if (ice_is_feature_supported(pf, ICE_F_GCS))
> + netdev->hw_features |= NETIF_F_HW_CSUM;
> +
> netif_set_tso_max_size(netdev, ICE_MAX_TSO_SIZE);
> }
>
> @@ -6237,6 +6243,7 @@ ice_fix_features(struct net_device *netdev, netdev_features_t features)
> struct ice_netdev_priv *np = netdev_priv(netdev);
> netdev_features_t req_vlan_fltr, cur_vlan_fltr;
> bool cur_ctag, cur_stag, req_ctag, req_stag;
> + netdev_features_t changed;
>
> cur_vlan_fltr = netdev->features & NETIF_VLAN_FILTERING_FEATURES;
> cur_ctag = cur_vlan_fltr & NETIF_F_HW_VLAN_CTAG_FILTER;
> @@ -6285,6 +6292,29 @@ ice_fix_features(struct net_device *netdev, netdev_features_t features)
> features &= ~NETIF_VLAN_STRIPPING_FEATURES;
> }
>
> + if (!ice_is_feature_supported(np->vsi->back, ICE_F_GCS) ||
> + !(features & NETIF_F_HW_CSUM))
> + return features;
This prevents adding any other feature checks below this in the future.
Seems like we should rework this into the feature being checked so that
we don't have this restriction.
> +
> + changed = netdev->features ^ features;
Scope of this could be reduced, but I guess the depends on what the
re-work looks like.
> +
> + /* HW checksum feature is supported and set, so enforce mutual
> + * exclusivity of TSO and GCS.
> + */
> + if (features & NETIF_F_ALL_TSO) {
> + if (changed & NETIF_F_HW_CSUM && changed & NETIF_F_ALL_TSO) {
> + netdev_warn(netdev, "Dropping TSO and HW checksum. TSO and HW checksum are mutually exclusive.\n");
> + features &= ~NETIF_F_HW_CSUM;
> + features &= ~((features & changed) & NETIF_F_ALL_TSO);
> + } else if (changed & NETIF_F_HW_CSUM) {
> + netdev_warn(netdev, "Dropping HW checksum. TSO and HW checksum are mutually exclusive.\n");
> + features &= ~NETIF_F_HW_CSUM;
> + } else {
> + netdev_warn(netdev, "Dropping TSO. TSO and HW checksum are mutually exclusive.\n");
> + features &= ~NETIF_F_ALL_TSO;
> + }
> + }
> +
> return features;
> }
>
> diff --git a/drivers/net/ethernet/intel/ice/ice_txrx.c b/drivers/net/ethernet/intel/ice/ice_txrx.c
> index 8d25b6981269..bfcce1eab243 100644
> --- a/drivers/net/ethernet/intel/ice/ice_txrx.c
> +++ b/drivers/net/ethernet/intel/ice/ice_txrx.c
> @@ -1792,6 +1792,7 @@ ice_tx_map(struct ice_tx_ring *tx_ring, struct ice_tx_buf *first,
> static
> int ice_tx_csum(struct ice_tx_buf *first, struct ice_tx_offload_params *off)
> {
> + const struct ice_tx_ring *tx_ring = off->tx_ring;
> u32 l4_len = 0, l3_len = 0, l2_len = 0;
> struct sk_buff *skb = first->skb;
> union {
> @@ -1941,6 +1942,29 @@ int ice_tx_csum(struct ice_tx_buf *first, struct ice_tx_offload_params *off)
> l3_len = l4.hdr - ip.hdr;
> offset |= (l3_len / 4) << ICE_TX_DESC_LEN_IPLEN_S;
>
> +#define TX_GCS_ENABLED 1
This should probably go with the other GCS defines.
> + if (tx_ring->netdev->features & NETIF_F_HW_CSUM &&
> + tx_ring->flags & ICE_TXRX_FLAGS_GCS_ENA &&
> + !(first->tx_flags & ICE_TX_FLAGS_TSO) &&
> + !skb_csum_is_sctp(skb)) {
> + /* Set GCS */
> + u16 gcs_params = ((skb->csum_start - skb->mac_header) / 2) <<
> + ICE_TX_GCS_DESC_START;
Missing newline here.
> + gcs_params |= (skb->csum_offset / 2) << ICE_TX_GCS_DESC_OFFSET;
> + /* Type is 1 for 16-bit TCP/UDP checksums w/ pseudo */
> + gcs_params |= TX_GCS_ENABLED << ICE_TX_GCS_DESC_TYPE
Could we define TX_GCS_ENABLED to have the proper bit set? I don't see
ICE_TX_GCS_DESC_TYPE used anywhere else so it'd eliminate the need for it.
> +
> + /* Unlike legacy HW checksums, GCS requires a context
> + * descriptor.
> + */
> + off->cd_qw1 |= (u64)(ICE_TX_DESC_DTYPE_CTX);
Are these latter parentheses needed?
> + off->cd_gcs_params = gcs_params;
> + /* Fill out CSO info in data descriptors */
> + off->td_offset |= offset;
> + off->td_cmd |= cmd;
> + return 1;
> + }
> +
> /* Enable L4 checksum offloads */
> switch (l4_proto) {
> case IPPROTO_TCP:
> @@ -2422,7 +2446,7 @@ ice_xmit_frame_ring(struct sk_buff *skb, struct ice_tx_ring *tx_ring)
> /* setup context descriptor */
> cdesc->tunneling_params = cpu_to_le32(offload.cd_tunnel_params);
> cdesc->l2tag2 = cpu_to_le16(offload.cd_l2tag2);
> - cdesc->rsvd = cpu_to_le16(0);
> + cdesc->gcs = cpu_to_le16(offload.cd_gcs_params);
> cdesc->qw1 = cpu_to_le64(offload.cd_qw1);
> }
>
> diff --git a/drivers/net/ethernet/intel/ice/ice_txrx.h b/drivers/net/ethernet/intel/ice/ice_txrx.h
> index feba314a3fe4..11b6af7a7414 100644
> --- a/drivers/net/ethernet/intel/ice/ice_txrx.h
> +++ b/drivers/net/ethernet/intel/ice/ice_txrx.h
> @@ -193,6 +193,7 @@ struct ice_tx_offload_params {
> u32 td_l2tag1;
> u32 cd_tunnel_params;
> u16 cd_l2tag2;
> + u16 cd_gcs_params;
> u8 header_len;
> };
>
> @@ -366,6 +367,7 @@ struct ice_rx_ring {
> #define ICE_RX_FLAGS_RING_BUILD_SKB BIT(1)
> #define ICE_RX_FLAGS_CRC_STRIP_DIS BIT(2)
> #define ICE_RX_FLAGS_MULTIDEV BIT(3)
> +#define ICE_TXRX_FLAGS_GCS_ENA BIT(4)
The RX flags [1] and TX flags [2] are seperate, please keep them
seperated and define them individually.
Thanks,
Tony
[1]
https://elixir.bootlin.com/linux/v6.10.6/source/drivers/net/ethernet/intel/ice/ice_txrx.h#L366
[2]
https://elixir.bootlin.com/linux/v6.10.6/source/drivers/net/ethernet/intel/ice/ice_txrx.h#L404
More information about the Intel-wired-lan
mailing list