diff mbox series

[v8,net-next,1/4] net: enetc: add Tx checksum offload for i.MX95 ENETC

Message ID 20241213021731.1157535-2-wei.fang@nxp.com (mailing list archive)
State New
Headers show
Series Add more feautues for ENETC v4 - round 1 | expand

Commit Message

Wei Fang Dec. 13, 2024, 2:17 a.m. UTC 
In addition to supporting Rx checksum offload, i.MX95 ENETC also supports
Tx checksum offload. The transmit checksum offload is implemented through
the Tx BD. To support Tx checksum offload, software needs to fill some
auxiliary information in Tx BD, such as IP version, IP header offset and
size, whether L4 is UDP or TCP, etc.

Same as Rx checksum offload, Tx checksum offload capability isn't defined
in register, so tx_csum bit is added to struct enetc_drvdata to indicate
whether the device supports Tx checksum offload.

Signed-off-by: Wei Fang <wei.fang@nxp.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Reviewed-by: Claudiu Manoil <claudiu.manoil@nxp.com>
---
v2: refine enetc_tx_csum_offload_check().
v3:
1. refine enetc_tx_csum_offload_check() and enetc_skb_is_tcp() through
skb->csum_offset instead of touching skb->data.
2. add enetc_skb_is_ipv6() helper function
v4: no changes
v5:
1. remove 'inline' from enetc_skb_is_ipv6() and enetc_skb_is_tcp().
2. temp_bd.ipcs is no need to be set due to Linux always aclculates
the IPv4 checksum, so remove it.
3. simplify the setting of temp_bd.l3t.
4. remove the error log from the datapath
v6: no changes
v7:
1. Change the layout of enetc_tx_bd to fix the issue on big-endian
hosts.
2. Rebase the patch due to remove the Rx checksum offload patch from
v6.
v8: no changes
---
 drivers/net/ethernet/freescale/enetc/enetc.c  | 54 ++++++++++++++++---
 drivers/net/ethernet/freescale/enetc/enetc.h  |  2 +
 .../net/ethernet/freescale/enetc/enetc_hw.h   | 15 ++++--
 .../freescale/enetc/enetc_pf_common.c         |  3 ++
 4 files changed, 64 insertions(+), 10 deletions(-)

Comments

Alexander Lobakin Dec. 17, 2024, 3:13 p.m. UTC | #1 
From: Wei Fang <wei.fang@nxp.com>
Date: Fri, 13 Dec 2024 10:17:28 +0800

> In addition to supporting Rx checksum offload, i.MX95 ENETC also supports
> Tx checksum offload. The transmit checksum offload is implemented through
> the Tx BD. To support Tx checksum offload, software needs to fill some
> auxiliary information in Tx BD, such as IP version, IP header offset and
> size, whether L4 is UDP or TCP, etc.
> 
> Same as Rx checksum offload, Tx checksum offload capability isn't defined
> in register, so tx_csum bit is added to struct enetc_drvdata to indicate
> whether the device supports Tx checksum offload.

[...]

> @@ -163,6 +184,30 @@ static int enetc_map_tx_buffs(struct enetc_bdr *tx_ring, struct sk_buff *skb)
>  	dma_addr_t dma;
>  	u8 flags = 0;
>  
> +	enetc_clear_tx_bd(&temp_bd);
> +	if (skb->ip_summed == CHECKSUM_PARTIAL) {
> +		/* Can not support TSD and checksum offload at the same time */
> +		if (priv->active_offloads & ENETC_F_TXCSUM &&
> +		    enetc_tx_csum_offload_check(skb) && !tx_ring->tsd_enable) {
> +			temp_bd.l3_aux0 = FIELD_PREP(ENETC_TX_BD_L3_START,
> +						     skb_network_offset(skb));
> +			temp_bd.l3_aux1 = FIELD_PREP(ENETC_TX_BD_L3_HDR_LEN,
> +						     skb_network_header_len(skb) / 4);
> +			temp_bd.l3_aux1 |= FIELD_PREP(ENETC_TX_BD_L3T,
> +						      enetc_skb_is_ipv6(skb));
> +			if (enetc_skb_is_tcp(skb))
> +				temp_bd.l4_aux = FIELD_PREP(ENETC_TX_BD_L4T,
> +							    ENETC_TXBD_L4T_TCP);
> +			else
> +				temp_bd.l4_aux = FIELD_PREP(ENETC_TX_BD_L4T,
> +							    ENETC_TXBD_L4T_UDP);
> +			flags |= ENETC_TXBD_FLAGS_CSUM_LSO | ENETC_TXBD_FLAGS_L4CS;
> +		} else {
> +			if (skb_checksum_help(skb))

Why not

		} else if (skb_checksum_help(skb)) {

?

> +				return 0;
> +		}
> +	}
> +
>  	i = tx_ring->next_to_use;
>  	txbd = ENETC_TXBD(*tx_ring, i);
>  	prefetchw(txbd);
> @@ -173,7 +218,6 @@ static int enetc_map_tx_buffs(struct enetc_bdr *tx_ring, struct sk_buff *skb)
>  
>  	temp_bd.addr = cpu_to_le64(dma);
>  	temp_bd.buf_len = cpu_to_le16(len);
> -	temp_bd.lstatus = 0;

Why is this removed and how is this change related to the checksum offload?

>  
>  	tx_swbd = &tx_ring->tx_swbd[i];
>  	tx_swbd->dma = dma;
> @@ -594,7 +638,7 @@ static netdev_tx_t enetc_start_xmit(struct sk_buff *skb,
>  {
>  	struct enetc_ndev_priv *priv = netdev_priv(ndev);
>  	struct enetc_bdr *tx_ring;
> -	int count, err;
> +	int count;
>  
>  	/* Queue one-step Sync packet if already locked */
>  	if (skb->cb[0] & ENETC_F_TX_ONESTEP_SYNC_TSTAMP) {
> @@ -627,11 +671,6 @@ static netdev_tx_t enetc_start_xmit(struct sk_buff *skb,
>  			return NETDEV_TX_BUSY;
>  		}
>  
> -		if (skb->ip_summed == CHECKSUM_PARTIAL) {
> -			err = skb_checksum_help(skb);
> -			if (err)
> -				goto drop_packet_err;
> -		}
>  		enetc_lock_mdio();
>  		count = enetc_map_tx_buffs(tx_ring, skb);
>  		enetc_unlock_mdio();
> @@ -3274,6 +3313,7 @@ static const struct enetc_drvdata enetc_pf_data = {
>  
>  static const struct enetc_drvdata enetc4_pf_data = {
>  	.sysclk_freq = ENETC_CLK_333M,
> +	.tx_csum = 1,

Maybe make it `bool tx_csum:1` instead of u8 and assign `true` here?

>  	.pmac_offset = ENETC4_PMAC_OFFSET,
>  	.eth_ops = &enetc4_pf_ethtool_ops,
>  };
> diff --git a/drivers/net/ethernet/freescale/enetc/enetc.h b/drivers/net/ethernet/freescale/enetc/enetc.h
> index 72fa03dbc2dd..e82eb9a9137c 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc.h
> +++ b/drivers/net/ethernet/freescale/enetc/enetc.h
> @@ -234,6 +234,7 @@ enum enetc_errata {
>  
>  struct enetc_drvdata {
>  	u32 pmac_offset; /* Only valid for PSI which supports 802.1Qbu */
> +	u8 tx_csum:1;
>  	u64 sysclk_freq;
>  	const struct ethtool_ops *eth_ops;
>  };

Thanks,
Olek
Wei Fang Dec. 18, 2024, 1:53 a.m. UTC | #2 
> 
> From: Wei Fang <wei.fang@nxp.com>
> Date: Fri, 13 Dec 2024 10:17:28 +0800
> 
> > In addition to supporting Rx checksum offload, i.MX95 ENETC also supports
> > Tx checksum offload. The transmit checksum offload is implemented through
> > the Tx BD. To support Tx checksum offload, software needs to fill some
> > auxiliary information in Tx BD, such as IP version, IP header offset and
> > size, whether L4 is UDP or TCP, etc.
> >
> > Same as Rx checksum offload, Tx checksum offload capability isn't defined
> > in register, so tx_csum bit is added to struct enetc_drvdata to indicate
> > whether the device supports Tx checksum offload.
> 
> [...]
> 
> > @@ -163,6 +184,30 @@ static int enetc_map_tx_buffs(struct enetc_bdr
> *tx_ring, struct sk_buff *skb)
> >  	dma_addr_t dma;
> >  	u8 flags = 0;
> >
> > +	enetc_clear_tx_bd(&temp_bd);
> > +	if (skb->ip_summed == CHECKSUM_PARTIAL) {
> > +		/* Can not support TSD and checksum offload at the same time */
> > +		if (priv->active_offloads & ENETC_F_TXCSUM &&
> > +		    enetc_tx_csum_offload_check(skb) && !tx_ring->tsd_enable) {
> > +			temp_bd.l3_aux0 = FIELD_PREP(ENETC_TX_BD_L3_START,
> > +						     skb_network_offset(skb));
> > +			temp_bd.l3_aux1 = FIELD_PREP(ENETC_TX_BD_L3_HDR_LEN,
> > +						     skb_network_header_len(skb) / 4);
> > +			temp_bd.l3_aux1 |= FIELD_PREP(ENETC_TX_BD_L3T,
> > +						      enetc_skb_is_ipv6(skb));
> > +			if (enetc_skb_is_tcp(skb))
> > +				temp_bd.l4_aux = FIELD_PREP(ENETC_TX_BD_L4T,
> > +							    ENETC_TXBD_L4T_TCP);
> > +			else
> > +				temp_bd.l4_aux = FIELD_PREP(ENETC_TX_BD_L4T,
> > +							    ENETC_TXBD_L4T_UDP);
> > +			flags |= ENETC_TXBD_FLAGS_CSUM_LSO |
> ENETC_TXBD_FLAGS_L4CS;
> > +		} else {
> > +			if (skb_checksum_help(skb))
> 
> Why not
> 
> 		} else if (skb_checksum_help(skb)) {
> 
> ?

Okay, accept.
> 
> > +				return 0;
> > +		}
> > +	}
> > +
> >  	i = tx_ring->next_to_use;
> >  	txbd = ENETC_TXBD(*tx_ring, i);
> >  	prefetchw(txbd);
> > @@ -173,7 +218,6 @@ static int enetc_map_tx_buffs(struct enetc_bdr
> *tx_ring, struct sk_buff *skb)
> >
> >  	temp_bd.addr = cpu_to_le64(dma);
> >  	temp_bd.buf_len = cpu_to_le16(len);
> > -	temp_bd.lstatus = 0;
> 
> Why is this removed and how is this change related to the checksum offload?

temp_bd has been cleared at the beginning, so we don't need to clear
lstatus again.
+	enetc_clear_tx_bd(&temp_bd);

And lstatus and aux* fields are in the same union. Clearing the lstatus
field will clear the checksum offload auxiliary information previously set.

union {
	struct {
		u8 l3_aux0;
		u8 l3_aux1;
		u8 l4_aux;
		u8 flags;
	}; /* default layout */
	__le32 txstart;
	__le32 lstatus;
};

> 
> >
> >  	tx_swbd = &tx_ring->tx_swbd[i];
> >  	tx_swbd->dma = dma;
> > @@ -594,7 +638,7 @@ static netdev_tx_t enetc_start_xmit(struct sk_buff
> *skb,
> >  {
> >  	struct enetc_ndev_priv *priv = netdev_priv(ndev);
> >  	struct enetc_bdr *tx_ring;
> > -	int count, err;
> > +	int count;
> >
> >  	/* Queue one-step Sync packet if already locked */
> >  	if (skb->cb[0] & ENETC_F_TX_ONESTEP_SYNC_TSTAMP) {
> > @@ -627,11 +671,6 @@ static netdev_tx_t enetc_start_xmit(struct sk_buff
> *skb,
> >  			return NETDEV_TX_BUSY;
> >  		}
> >
> > -		if (skb->ip_summed == CHECKSUM_PARTIAL) {
> > -			err = skb_checksum_help(skb);
> > -			if (err)
> > -				goto drop_packet_err;
> > -		}
> >  		enetc_lock_mdio();
> >  		count = enetc_map_tx_buffs(tx_ring, skb);
> >  		enetc_unlock_mdio();
> > @@ -3274,6 +3313,7 @@ static const struct enetc_drvdata enetc_pf_data =
> {
> >
> >  static const struct enetc_drvdata enetc4_pf_data = {
> >  	.sysclk_freq = ENETC_CLK_333M,
> > +	.tx_csum = 1,
> 
> Maybe make it `bool tx_csum:1` instead of u8 and assign `true` here?

I think 'u8 tx_csum:1' is fine, we just need to change "1" to "true". After
all, 'u8 xxx:n' fields may be defined later.

> 
> >  	.pmac_offset = ENETC4_PMAC_OFFSET,
> >  	.eth_ops = &enetc4_pf_ethtool_ops,
> >  };
> > diff --git a/drivers/net/ethernet/freescale/enetc/enetc.h
> b/drivers/net/ethernet/freescale/enetc/enetc.h
> > index 72fa03dbc2dd..e82eb9a9137c 100644
> > --- a/drivers/net/ethernet/freescale/enetc/enetc.h
> > +++ b/drivers/net/ethernet/freescale/enetc/enetc.h
> > @@ -234,6 +234,7 @@ enum enetc_errata {
> >
> >  struct enetc_drvdata {
> >  	u32 pmac_offset; /* Only valid for PSI which supports 802.1Qbu */
> > +	u8 tx_csum:1;
> >  	u64 sysclk_freq;
> >  	const struct ethtool_ops *eth_ops;
> >  };
> 
> Thanks,
> Olek
diff mbox series

Patch

diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c b/drivers/net/ethernet/freescale/enetc/enetc.c
index 535969fa0fdb..b8ac680e46bd 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc.c
@@ -146,6 +146,27 @@  static int enetc_ptp_parse(struct sk_buff *skb, u8 *udp,
 	return 0;
 }
 
+static bool enetc_tx_csum_offload_check(struct sk_buff *skb)
+{
+	switch (skb->csum_offset) {
+	case offsetof(struct tcphdr, check):
+	case offsetof(struct udphdr, check):
+		return true;
+	default:
+		return false;
+	}
+}
+
+static bool enetc_skb_is_ipv6(struct sk_buff *skb)
+{
+	return vlan_get_protocol(skb) == htons(ETH_P_IPV6);
+}
+
+static bool enetc_skb_is_tcp(struct sk_buff *skb)
+{
+	return skb->csum_offset == offsetof(struct tcphdr, check);
+}
+
 static int enetc_map_tx_buffs(struct enetc_bdr *tx_ring, struct sk_buff *skb)
 {
 	bool do_vlan, do_onestep_tstamp = false, do_twostep_tstamp = false;
@@ -163,6 +184,30 @@  static int enetc_map_tx_buffs(struct enetc_bdr *tx_ring, struct sk_buff *skb)
 	dma_addr_t dma;
 	u8 flags = 0;
 
+	enetc_clear_tx_bd(&temp_bd);
+	if (skb->ip_summed == CHECKSUM_PARTIAL) {
+		/* Can not support TSD and checksum offload at the same time */
+		if (priv->active_offloads & ENETC_F_TXCSUM &&
+		    enetc_tx_csum_offload_check(skb) && !tx_ring->tsd_enable) {
+			temp_bd.l3_aux0 = FIELD_PREP(ENETC_TX_BD_L3_START,
+						     skb_network_offset(skb));
+			temp_bd.l3_aux1 = FIELD_PREP(ENETC_TX_BD_L3_HDR_LEN,
+						     skb_network_header_len(skb) / 4);
+			temp_bd.l3_aux1 |= FIELD_PREP(ENETC_TX_BD_L3T,
+						      enetc_skb_is_ipv6(skb));
+			if (enetc_skb_is_tcp(skb))
+				temp_bd.l4_aux = FIELD_PREP(ENETC_TX_BD_L4T,
+							    ENETC_TXBD_L4T_TCP);
+			else
+				temp_bd.l4_aux = FIELD_PREP(ENETC_TX_BD_L4T,
+							    ENETC_TXBD_L4T_UDP);
+			flags |= ENETC_TXBD_FLAGS_CSUM_LSO | ENETC_TXBD_FLAGS_L4CS;
+		} else {
+			if (skb_checksum_help(skb))
+				return 0;
+		}
+	}
+
 	i = tx_ring->next_to_use;
 	txbd = ENETC_TXBD(*tx_ring, i);
 	prefetchw(txbd);
@@ -173,7 +218,6 @@  static int enetc_map_tx_buffs(struct enetc_bdr *tx_ring, struct sk_buff *skb)
 
 	temp_bd.addr = cpu_to_le64(dma);
 	temp_bd.buf_len = cpu_to_le16(len);
-	temp_bd.lstatus = 0;
 
 	tx_swbd = &tx_ring->tx_swbd[i];
 	tx_swbd->dma = dma;
@@ -594,7 +638,7 @@  static netdev_tx_t enetc_start_xmit(struct sk_buff *skb,
 {
 	struct enetc_ndev_priv *priv = netdev_priv(ndev);
 	struct enetc_bdr *tx_ring;
-	int count, err;
+	int count;
 
 	/* Queue one-step Sync packet if already locked */
 	if (skb->cb[0] & ENETC_F_TX_ONESTEP_SYNC_TSTAMP) {
@@ -627,11 +671,6 @@  static netdev_tx_t enetc_start_xmit(struct sk_buff *skb,
 			return NETDEV_TX_BUSY;
 		}
 
-		if (skb->ip_summed == CHECKSUM_PARTIAL) {
-			err = skb_checksum_help(skb);
-			if (err)
-				goto drop_packet_err;
-		}
 		enetc_lock_mdio();
 		count = enetc_map_tx_buffs(tx_ring, skb);
 		enetc_unlock_mdio();
@@ -3274,6 +3313,7 @@  static const struct enetc_drvdata enetc_pf_data = {
 
 static const struct enetc_drvdata enetc4_pf_data = {
 	.sysclk_freq = ENETC_CLK_333M,
+	.tx_csum = 1,
 	.pmac_offset = ENETC4_PMAC_OFFSET,
 	.eth_ops = &enetc4_pf_ethtool_ops,
 };
diff --git a/drivers/net/ethernet/freescale/enetc/enetc.h b/drivers/net/ethernet/freescale/enetc/enetc.h
index 72fa03dbc2dd..e82eb9a9137c 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc.h
+++ b/drivers/net/ethernet/freescale/enetc/enetc.h
@@ -234,6 +234,7 @@  enum enetc_errata {
 
 struct enetc_drvdata {
 	u32 pmac_offset; /* Only valid for PSI which supports 802.1Qbu */
+	u8 tx_csum:1;
 	u64 sysclk_freq;
 	const struct ethtool_ops *eth_ops;
 };
@@ -341,6 +342,7 @@  enum enetc_active_offloads {
 	ENETC_F_QBV			= BIT(9),
 	ENETC_F_QCI			= BIT(10),
 	ENETC_F_QBU			= BIT(11),
+	ENETC_F_TXCSUM			= BIT(12),
 };
 
 enum enetc_flags_bit {
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_hw.h b/drivers/net/ethernet/freescale/enetc/enetc_hw.h
index 55ba949230ff..0e259baf36ee 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_hw.h
+++ b/drivers/net/ethernet/freescale/enetc/enetc_hw.h
@@ -558,7 +558,16 @@  union enetc_tx_bd {
 		__le16 frm_len;
 		union {
 			struct {
-				u8 reserved[3];
+				u8 l3_aux0;
+#define ENETC_TX_BD_L3_START	GENMASK(6, 0)
+#define ENETC_TX_BD_IPCS	BIT(7)
+				u8 l3_aux1;
+#define ENETC_TX_BD_L3_HDR_LEN	GENMASK(6, 0)
+#define ENETC_TX_BD_L3T		BIT(7)
+				u8 l4_aux;
+#define ENETC_TX_BD_L4T		GENMASK(7, 5)
+#define ENETC_TXBD_L4T_UDP	1
+#define ENETC_TXBD_L4T_TCP	2
 				u8 flags;
 			}; /* default layout */
 			__le32 txstart;
@@ -582,10 +591,10 @@  union enetc_tx_bd {
 };
 
 enum enetc_txbd_flags {
-	ENETC_TXBD_FLAGS_RES0 = BIT(0), /* reserved */
+	ENETC_TXBD_FLAGS_L4CS = BIT(0), /* For ENETC 4.1 and later */
 	ENETC_TXBD_FLAGS_TSE = BIT(1),
 	ENETC_TXBD_FLAGS_W = BIT(2),
-	ENETC_TXBD_FLAGS_RES3 = BIT(3), /* reserved */
+	ENETC_TXBD_FLAGS_CSUM_LSO = BIT(3), /* For ENETC 4.1 and later */
 	ENETC_TXBD_FLAGS_TXSTART = BIT(4),
 	ENETC_TXBD_FLAGS_EX = BIT(6),
 	ENETC_TXBD_FLAGS_F = BIT(7)
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf_common.c b/drivers/net/ethernet/freescale/enetc/enetc_pf_common.c
index 0eecfc833164..09f2d7ec44eb 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_pf_common.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_pf_common.c
@@ -119,6 +119,9 @@  void enetc_pf_netdev_setup(struct enetc_si *si, struct net_device *ndev,
 
 	ndev->priv_flags |= IFF_UNICAST_FLT;
 
+	if (si->drvdata->tx_csum)
+		priv->active_offloads |= ENETC_F_TXCSUM;
+
 	/* TODO: currently, i.MX95 ENETC driver does not support advanced features */
 	if (!is_enetc_rev1(si)) {
 		ndev->hw_features &= ~(NETIF_F_HW_VLAN_CTAG_FILTER | NETIF_F_LOOPBACK);