| Message ID | 20240826173916.1394617-1-paul.greenwalt@intel.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [iwl-next,v1] ice: Add E830 checksum support | expand |
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@intel.com> > Signed-off-by: Alice Michael <alice.michael@intel.com> > Co-developed-by: Eric Joyner <eric.joyner@intel.com> > Signed-off-by: Eric Joyner <eric.joyner@intel.com> > Signed-off-by: Paul Greenwalt <paul.greenwalt@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
From: Paul Greenwalt <paul.greenwalt@intel.com> Date: Mon, 26 Aug 2024 13:39:16 -0400 > 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 Why is it so? > (TSO) inner packet modification, so TSO and HW_CSUM are mutually exclusive. What is HW_CSUM in your opinion here? > 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 "the provided"? > 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@intel.com> > Signed-off-by: Alice Michael <alice.michael@intel.com> > Co-developed-by: Eric Joyner <eric.joyner@intel.com> > Signed-off-by: Eric Joyner <eric.joyner@intel.com> > Signed-off-by: Paul Greenwalt <paul.greenwalt@intel.com> [...] > 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; An empty newline here, since WRITE_ONCE() is not related to this one? > 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; Same here. > 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; > + > + changed = netdev->features ^ features; > + > + /* HW checksum feature is supported and set, so enforce mutual > + * exclusivity of TSO and GCS. > + */ > + if (features & NETIF_F_ALL_TSO) { if (!(features & ALL_TSO)) return features; to reduce indent level and avoid huge `if` blocks. > + 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 must be somewhere where the offload params or descriptor values are described, i.e. next to the related definitions. > + if (tx_ring->netdev->features & NETIF_F_HW_CSUM && Please give bitops a separate set of parenthesis if ((features & HW_CSUM) && etc., below as well. > + 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; This must be gcs_params = FIELD_PREP(GCS_DESC_MASK, (skb...) / 2) 2 places below as well. > + 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; > + > + /* Unlike legacy HW checksums, GCS requires a context > + * descriptor. > + */ > + off->cd_qw1 |= (u64)(ICE_TX_DESC_DTYPE_CTX); Redundant cast. > + off->cd_gcs_params = gcs_params; > + /* Fill out CSO info in data descriptors */ > + off->td_offset |= offset; > + off->td_cmd |= cmd; > + return 1; > + } [...] > @@ -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) As Tony said, RX and TX features are now separated, just like Rx and Tx ring structures. Please define them separately for Rx and Tx. > u8 flags; > /* CL5 - 5th cacheline starts here */ > struct xdp_rxq_info xdp_rxq; > diff --git a/drivers/net/ethernet/intel/ice/ice_txrx_lib.c b/drivers/net/ethernet/intel/ice/ice_txrx_lib.c > index 2719f0e20933..a344886d2129 100644 > --- a/drivers/net/ethernet/intel/ice/ice_txrx_lib.c > +++ b/drivers/net/ethernet/intel/ice/ice_txrx_lib.c > @@ -80,6 +80,24 @@ ice_rx_hash_to_skb(const struct ice_rx_ring *rx_ring, > libeth_rx_pt_set_hash(skb, hash, decoded); > } > > +/** > + * ice_rx_gcs - Set generic checksum in skd "skb" > + * @skb: skb currently being received and modified > + * @rx_desc: Receive descriptor Lowercase for "Receive" I'd say. > + * Returns hash, if present, 0 otherwise. The function returns void. > + */ > +static void ice_rx_gcs(struct sk_buff *skb, union ice_32b_rx_flex_desc *rx_desc) const union. > +{ > + struct ice_32b_rx_flex_desc_nic *nic_csum; It's a descriptor, not a csum. + also const. > + > + if (rx_desc->wb.rxdid != ICE_RXDID_FLEX_NIC) > + return; Redundant check since you're checking this below. > + > + nic_csum = (struct ice_32b_rx_flex_desc_nic *)rx_desc; > + skb->ip_summed = CHECKSUM_COMPLETE; > + skb->csum = (__force __wsum)htons(le16_to_cpu(nic_csum->raw_csum)); htons(le16_to_cpu(x)) is the same as swab16(x), have you tried it? > +} > + > /** > * ice_rx_csum - Indicate in skb if checksum is good > * @ring: the ring we care about > @@ -107,6 +125,21 @@ ice_rx_csum(struct ice_rx_ring *ring, struct sk_buff *skb, > rx_status0 = le16_to_cpu(rx_desc->wb.status_error0); > rx_status1 = le16_to_cpu(rx_desc->wb.status_error1); > > + if (rx_desc->wb.rxdid == ICE_RXDID_FLEX_NIC && > + ring->flags & ICE_TXRX_FLAGS_GCS_ENA && I'd reorder these. 1 flags, 2 flex. > + (decoded.inner_prot == LIBETH_RX_PT_INNER_TCP || > + decoded.inner_prot == LIBETH_RX_PT_INNER_UDP || > + decoded.inner_prot == LIBETH_RX_PT_INNER_ICMP)) { > + /* ICE_RX_FLEX_DESC_STATUS1_L2TAG2P_S is overloaded in the > + * rx_status1 layout to indicate that the extracted data from > + * the packet is valid. > + */ > + if (rx_status1 & BIT(ICE_RX_FLEX_DESC_STATUS1_L2TAG2P_S)) > + return ice_rx_gcs(skb, rx_desc); ice_rx_csum() is void. Please separate the call and the return. > + > + goto checksum_fail; Why fail? If there's no STATUS1_L2TAG2P_S bit, can't there be a usual checksum status? > + } > + > /* check if HW has decoded the packet and checksum */ > if (!(rx_status0 & BIT(ICE_RX_FLEX_DESC_STATUS0_L3L4P_S))) > return; Thanks, Olek
On 8/30/2024 6:30 AM, Alexander Lobakin wrote: > From: Paul Greenwalt <paul.greenwalt@intel.com> > Date: Mon, 26 Aug 2024 13:39:16 -0400 > >> 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 > > Why is it so? > >> (TSO) inner packet modification, so TSO and HW_CSUM are mutually exclusive. > > What is HW_CSUM in your opinion here? > >> 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 > > "the provided"? > >> 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@intel.com> >> Signed-off-by: Alice Michael <alice.michael@intel.com> >> Co-developed-by: Eric Joyner <eric.joyner@intel.com> >> Signed-off-by: Eric Joyner <eric.joyner@intel.com> >> Signed-off-by: Paul Greenwalt <paul.greenwalt@intel.com> > > [...] > >> 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; > > An empty newline here, since WRITE_ONCE() is not related to this one? > >> 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; > > Same here. > >> 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; >> + >> + changed = netdev->features ^ features; >> + >> + /* HW checksum feature is supported and set, so enforce mutual >> + * exclusivity of TSO and GCS. >> + */ >> + if (features & NETIF_F_ALL_TSO) { > > if (!(features & ALL_TSO)) > return features; > > to reduce indent level and avoid huge `if` blocks. > >> + 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 must be somewhere where the offload params or descriptor values are > described, i.e. next to the related definitions. > >> + if (tx_ring->netdev->features & NETIF_F_HW_CSUM && > > Please give bitops a separate set of parenthesis > > if ((features & HW_CSUM) && > > etc., below as well. > >> + 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; > > This must be > > gcs_params = FIELD_PREP(GCS_DESC_MASK, (skb...) / 2) > > 2 places below as well. > >> + 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; >> + >> + /* Unlike legacy HW checksums, GCS requires a context >> + * descriptor. >> + */ >> + off->cd_qw1 |= (u64)(ICE_TX_DESC_DTYPE_CTX); > > Redundant cast. > >> + off->cd_gcs_params = gcs_params; >> + /* Fill out CSO info in data descriptors */ >> + off->td_offset |= offset; >> + off->td_cmd |= cmd; >> + return 1; >> + } > > [...] > >> @@ -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) > > As Tony said, RX and TX features are now separated, just like Rx and Tx > ring structures. Please define them separately for Rx and Tx. > >> u8 flags; >> /* CL5 - 5th cacheline starts here */ >> struct xdp_rxq_info xdp_rxq; >> diff --git a/drivers/net/ethernet/intel/ice/ice_txrx_lib.c b/drivers/net/ethernet/intel/ice/ice_txrx_lib.c >> index 2719f0e20933..a344886d2129 100644 >> --- a/drivers/net/ethernet/intel/ice/ice_txrx_lib.c >> +++ b/drivers/net/ethernet/intel/ice/ice_txrx_lib.c >> @@ -80,6 +80,24 @@ ice_rx_hash_to_skb(const struct ice_rx_ring *rx_ring, >> libeth_rx_pt_set_hash(skb, hash, decoded); >> } >> >> +/** >> + * ice_rx_gcs - Set generic checksum in skd > > "skb" > >> + * @skb: skb currently being received and modified >> + * @rx_desc: Receive descriptor > > Lowercase for "Receive" I'd say. > >> + * Returns hash, if present, 0 otherwise. > > The function returns void. > >> + */ >> +static void ice_rx_gcs(struct sk_buff *skb, union ice_32b_rx_flex_desc *rx_desc) > > const union. > >> +{ >> + struct ice_32b_rx_flex_desc_nic *nic_csum; > > It's a descriptor, not a csum. + also const. > >> + >> + if (rx_desc->wb.rxdid != ICE_RXDID_FLEX_NIC) >> + return; > > Redundant check since you're checking this below. > >> + >> + nic_csum = (struct ice_32b_rx_flex_desc_nic *)rx_desc; >> + skb->ip_summed = CHECKSUM_COMPLETE; >> + skb->csum = (__force __wsum)htons(le16_to_cpu(nic_csum->raw_csum)); > > htons(le16_to_cpu(x)) is the same as swab16(x), have you tried it? > >> +} >> + >> /** >> * ice_rx_csum - Indicate in skb if checksum is good >> * @ring: the ring we care about >> @@ -107,6 +125,21 @@ ice_rx_csum(struct ice_rx_ring *ring, struct sk_buff *skb, >> rx_status0 = le16_to_cpu(rx_desc->wb.status_error0); >> rx_status1 = le16_to_cpu(rx_desc->wb.status_error1); >> >> + if (rx_desc->wb.rxdid == ICE_RXDID_FLEX_NIC && >> + ring->flags & ICE_TXRX_FLAGS_GCS_ENA && > > I'd reorder these. 1 flags, 2 flex. > >> + (decoded.inner_prot == LIBETH_RX_PT_INNER_TCP || >> + decoded.inner_prot == LIBETH_RX_PT_INNER_UDP || >> + decoded.inner_prot == LIBETH_RX_PT_INNER_ICMP)) { >> + /* ICE_RX_FLEX_DESC_STATUS1_L2TAG2P_S is overloaded in the >> + * rx_status1 layout to indicate that the extracted data from >> + * the packet is valid. >> + */ >> + if (rx_status1 & BIT(ICE_RX_FLEX_DESC_STATUS1_L2TAG2P_S)) >> + return ice_rx_gcs(skb, rx_desc); > > ice_rx_csum() is void. Please separate the call and the return. > >> + >> + goto checksum_fail; > > Why fail? If there's no STATUS1_L2TAG2P_S bit, can't there be a usual > checksum status? > Hi Olek, I'm preparing a v2 based on yours and Tony's feedback. I will need to check into the conditions that can lead to Rx flex descriptor valid bit for Rx checksum not being set (ICE_RX_FLEX_DESC_STATUS1_L2TAG2P_S), and if it is valid to check the protocal checksum. Thanks, Paul >> + } >> + >> /* check if HW has decoded the packet and checksum */ >> if (!(rx_status0 & BIT(ICE_RX_FLEX_DESC_STATUS0_L3L4P_S))) >> return; > > Thanks, > Olek
On 9/3/2024 8:29 PM, Greenwalt, Paul wrote: >>> >>> @@ -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; >>> + >>> + changed = netdev->features ^ features; >>> + >>> + /* HW checksum feature is supported and set, so enforce mutual >>> + * exclusivity of TSO and GCS. >>> + */ >>> + if (features & NETIF_F_ALL_TSO) { >> >> if (!(features & ALL_TSO)) >> return features; >> >> to reduce indent level and avoid huge `if` blocks. >> Hi Olek and Tony, you both had feedback related to the change to ice_fix_features(). Olek: to reduce indent level and avoid huge `if` blocks. Tony: 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. Would using two helper functions for fixing DVM, and another for GCS and TSO address your feedback? By adding feature checks are the top of the helper functions and returning early, this could reduce the indent level and avoid the huge 'if' block, while removing the restriction of adding feature checks below these in the future. Thanks, Paul >>> + 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);
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; + + changed = netdev->features ^ features; + + /* 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 + 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; + 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; + + /* Unlike legacy HW checksums, GCS requires a context + * descriptor. + */ + off->cd_qw1 |= (u64)(ICE_TX_DESC_DTYPE_CTX); + 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) u8 flags; /* CL5 - 5th cacheline starts here */ struct xdp_rxq_info xdp_rxq; diff --git a/drivers/net/ethernet/intel/ice/ice_txrx_lib.c b/drivers/net/ethernet/intel/ice/ice_txrx_lib.c index 2719f0e20933..a344886d2129 100644 --- a/drivers/net/ethernet/intel/ice/ice_txrx_lib.c +++ b/drivers/net/ethernet/intel/ice/ice_txrx_lib.c @@ -80,6 +80,24 @@ ice_rx_hash_to_skb(const struct ice_rx_ring *rx_ring, libeth_rx_pt_set_hash(skb, hash, decoded); } +/** + * ice_rx_gcs - Set generic checksum in skd + * @skb: skb currently being received and modified + * @rx_desc: Receive descriptor + * Returns hash, if present, 0 otherwise. + */ +static void ice_rx_gcs(struct sk_buff *skb, union ice_32b_rx_flex_desc *rx_desc) +{ + struct ice_32b_rx_flex_desc_nic *nic_csum; + + if (rx_desc->wb.rxdid != ICE_RXDID_FLEX_NIC) + return; + + nic_csum = (struct ice_32b_rx_flex_desc_nic *)rx_desc; + skb->ip_summed = CHECKSUM_COMPLETE; + skb->csum = (__force __wsum)htons(le16_to_cpu(nic_csum->raw_csum)); +} + /** * ice_rx_csum - Indicate in skb if checksum is good * @ring: the ring we care about @@ -107,6 +125,21 @@ ice_rx_csum(struct ice_rx_ring *ring, struct sk_buff *skb, rx_status0 = le16_to_cpu(rx_desc->wb.status_error0); rx_status1 = le16_to_cpu(rx_desc->wb.status_error1); + if (rx_desc->wb.rxdid == ICE_RXDID_FLEX_NIC && + ring->flags & ICE_TXRX_FLAGS_GCS_ENA && + (decoded.inner_prot == LIBETH_RX_PT_INNER_TCP || + decoded.inner_prot == LIBETH_RX_PT_INNER_UDP || + decoded.inner_prot == LIBETH_RX_PT_INNER_ICMP)) { + /* ICE_RX_FLEX_DESC_STATUS1_L2TAG2P_S is overloaded in the + * rx_status1 layout to indicate that the extracted data from + * the packet is valid. + */ + if (rx_status1 & BIT(ICE_RX_FLEX_DESC_STATUS1_L2TAG2P_S)) + return ice_rx_gcs(skb, rx_desc); + + goto checksum_fail; + } + /* check if HW has decoded the packet and checksum */ if (!(rx_status0 & BIT(ICE_RX_FLEX_DESC_STATUS0_L3L4P_S))) return;