diff mbox series

[v2,net-next,2/5] net: enetc: add Tx checksum offload for i.MX95 ENETC

Message ID 20241111015216.1804534-3-wei.fang@nxp.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series Add more feautues for ENETC v4 - round 1 | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 3 this patch: 3
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 10 of 10 maintainers
netdev/build_clang success Errors and warnings before: 3 this patch: 3
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 8 this patch: 8
netdev/checkpatch warning WARNING: line length of 83 exceeds 80 columns WARNING: line length of 87 exceeds 80 columns
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
netdev/contest success net-next-2024-11-11--21-00 (tests: 787)

Commit Message

Wei Fang Nov. 11, 2024, 1:52 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>
---
v2: refine enetc_tx_csum_offload_check().
---
 drivers/net/ethernet/freescale/enetc/enetc.c  | 53 ++++++++++++++++---
 drivers/net/ethernet/freescale/enetc/enetc.h  |  2 +
 .../net/ethernet/freescale/enetc/enetc_hw.h   | 14 +++--
 .../freescale/enetc/enetc_pf_common.c         |  3 ++
 4 files changed, 62 insertions(+), 10 deletions(-)

Comments

Claudiu Manoil Nov. 11, 2024, 7:26 a.m. UTC | #1
Hi Wei,

> -----Original Message-----
> From: Wei Fang <wei.fang@nxp.com>
> Sent: Monday, November 11, 2024 3:52 AM
[...]
> Subject: [PATCH v2 net-next 2/5] net: enetc: add Tx checksum offload for
> i.MX95 ENETC
> 
> 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>
> ---
> v2: refine enetc_tx_csum_offload_check().
> ---
>  drivers/net/ethernet/freescale/enetc/enetc.c  | 53 ++++++++++++++++---
>  drivers/net/ethernet/freescale/enetc/enetc.h  |  2 +
>  .../net/ethernet/freescale/enetc/enetc_hw.h   | 14 +++--
>  .../freescale/enetc/enetc_pf_common.c         |  3 ++
>  4 files changed, 62 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c
> b/drivers/net/ethernet/freescale/enetc/enetc.c
> index 3137b6ee62d3..502194317a96 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc.c
> @@ -143,6 +143,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)
> +{
> +	if (ip_hdr(skb)->version == 4)

I would avoid using ip_hdr(), or any form of touching packed data and try
extract this kind of info directly from the skb metadata instead, see also comment below.

i.e., why not:
if (skb->protocol == htons(ETH_P_IPV6)) ..  etc. ?
or
switch (skb->csum_offset) {
case offsetof(struct tcphdr, check):
[...]
case offsetof(struct udphdr, check):
[...]

> +		return ip_hdr(skb)->protocol == IPPROTO_TCP ||
> +		       ip_hdr(skb)->protocol == IPPROTO_UDP;
> +
> +	if (ip_hdr(skb)->version == 6)
> +		return ipv6_hdr(skb)->nexthdr == NEXTHDR_TCP ||
> +		       ipv6_hdr(skb)->nexthdr == NEXTHDR_UDP;
> +
> +	return false;
> +}
> +
> +static bool enetc_skb_is_tcp(struct sk_buff *skb)
> +{

There is a more efficient way of checking if L4 is TCP, without touching
packet data, i.e. through the 'csum_offset' skb field:
return skb->csum_offset == offsetof(struct tcphdr, check);

Pls. have a look at these optimizations, I would expect visible improvements
in throughput. Thanks.

> +	if (ip_hdr(skb)->version == 4)
> +		return ip_hdr(skb)->protocol == IPPROTO_TCP;
> +	else
> +		return ipv6_hdr(skb)->nexthdr == NEXTHDR_TCP;
> +}
> +
Wei Fang Nov. 11, 2024, 9:26 a.m. UTC | #2
> > --- a/drivers/net/ethernet/freescale/enetc/enetc.c
> > +++ b/drivers/net/ethernet/freescale/enetc/enetc.c
> > @@ -143,6 +143,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)
> > +{
> > +	if (ip_hdr(skb)->version == 4)
> 
> I would avoid using ip_hdr(), or any form of touching packed data and try
> extract this kind of info directly from the skb metadata instead, see also
> comment below.
> 
> i.e., why not:
> if (skb->protocol == htons(ETH_P_IPV6)) ..  etc. ?

skb->protocol may be VLAN protocol, such as ETH_P_8021Q, ETH_P_8021AD.
If so, it is impossible to determine whether it is an IPv4 or IPv6 frames through
protocol.

> or
> switch (skb->csum_offset) {
> case offsetof(struct tcphdr, check):
> [...]
> case offsetof(struct udphdr, check):
> [...]

This seems to be able to be used to determine whether it is a UDP or TCP frame.
Thanks.

> 
> > +		return ip_hdr(skb)->protocol == IPPROTO_TCP ||
> > +		       ip_hdr(skb)->protocol == IPPROTO_UDP;
> > +
> > +	if (ip_hdr(skb)->version == 6)
> > +		return ipv6_hdr(skb)->nexthdr == NEXTHDR_TCP ||
> > +		       ipv6_hdr(skb)->nexthdr == NEXTHDR_UDP;
> > +
> > +	return false;
> > +}
> > +
> > +static bool enetc_skb_is_tcp(struct sk_buff *skb)
> > +{
> 
> There is a more efficient way of checking if L4 is TCP, without touching
> packet data, i.e. through the 'csum_offset' skb field:
> return skb->csum_offset == offsetof(struct tcphdr, check);
> 
> Pls. have a look at these optimizations, I would expect visible improvements
> in throughput. Thanks.

For small packets this might improve performance, but I'm not sure if it would be
a significant improvement. :)

> 
> > +	if (ip_hdr(skb)->version == 4)
> > +		return ip_hdr(skb)->protocol == IPPROTO_TCP;
> > +	else
> > +		return ipv6_hdr(skb)->nexthdr == NEXTHDR_TCP;
> > +}
> > +
Wei Fang Nov. 11, 2024, 12:16 p.m. UTC | #3
Hi Claudiu,

> -----Original Message-----
> From: Wei Fang
> Sent: 2024年11月11日 17:26
> To: Claudiu Manoil <claudiu.manoil@nxp.com>
> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org; imx@lists.linux.dev;
> Vladimir Oltean <vladimir.oltean@nxp.com>; Clark Wang
> <xiaoning.wang@nxp.com>; andrew+netdev@lunn.ch; davem@davemloft.net;
> edumazet@google.com; kuba@kernel.org; pabeni@redhat.com; Frank Li
> <frank.li@nxp.com>
> Subject: RE: [PATCH v2 net-next 2/5] net: enetc: add Tx checksum offload for
> i.MX95 ENETC
> 
> > > --- a/drivers/net/ethernet/freescale/enetc/enetc.c
> > > +++ b/drivers/net/ethernet/freescale/enetc/enetc.c
> > > @@ -143,6 +143,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)
> > > +{
> > > +	if (ip_hdr(skb)->version == 4)
> >
> > I would avoid using ip_hdr(), or any form of touching packed data and try
> > extract this kind of info directly from the skb metadata instead, see also
> > comment below.
> >
> > i.e., why not:
> > if (skb->protocol == htons(ETH_P_IPV6)) ..  etc. ?
> 
> skb->protocol may be VLAN protocol, such as ETH_P_8021Q, ETH_P_8021AD.
> If so, it is impossible to determine whether it is an IPv4 or IPv6 frames through
> protocol.
> 
> > or
> > switch (skb->csum_offset) {
> > case offsetof(struct tcphdr, check):
> > [...]
> > case offsetof(struct udphdr, check):
> > [...]
> 
> This seems to be able to be used to determine whether it is a UDP or TCP
> frame.
> Thanks.
> 
> >
> > > +		return ip_hdr(skb)->protocol == IPPROTO_TCP ||
> > > +		       ip_hdr(skb)->protocol == IPPROTO_UDP;
> > > +
> > > +	if (ip_hdr(skb)->version == 6)
> > > +		return ipv6_hdr(skb)->nexthdr == NEXTHDR_TCP ||
> > > +		       ipv6_hdr(skb)->nexthdr == NEXTHDR_UDP;
> > > +
> > > +	return false;
> > > +}
> > > +
> > > +static bool enetc_skb_is_tcp(struct sk_buff *skb)
> > > +{
> >
> > There is a more efficient way of checking if L4 is TCP, without touching
> > packet data, i.e. through the 'csum_offset' skb field:
> > return skb->csum_offset == offsetof(struct tcphdr, check);
> >
> > Pls. have a look at these optimizations, I would expect visible improvements
> > in throughput. Thanks.
> 
> For small packets this might improve performance, but I'm not sure if it would
> be
> a significant improvement. :)
> 

I didn't see any visible improvements in performance after using csum_offset.
For example, when using pktgen to send 10,000,000 packets, the time taken is
almost the same regardless of whether they are large or small packets, and the
CPU idle ratio seen through the top command is also basically the same. Also,
the UDP performance tested by iperf3 is basically the same.

> >
> > > +	if (ip_hdr(skb)->version == 4)
> > > +		return ip_hdr(skb)->protocol == IPPROTO_TCP;
> > > +	else
> > > +		return ipv6_hdr(skb)->nexthdr == NEXTHDR_TCP;
> > > +}
> > > +
Claudiu Manoil Nov. 11, 2024, 3:14 p.m. UTC | #4
> -----Original Message-----
> From: Wei Fang <wei.fang@nxp.com>
> Sent: Monday, November 11, 2024 2:16 PM
> To: Claudiu Manoil <claudiu.manoil@nxp.com>
> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org; imx@lists.linux.dev;
> Vladimir Oltean <vladimir.oltean@nxp.com>; Clark Wang
> <xiaoning.wang@nxp.com>; andrew+netdev@lunn.ch;
> davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
> pabeni@redhat.com; Frank Li <frank.li@nxp.com>
> Subject: RE: [PATCH v2 net-next 2/5] net: enetc: add Tx checksum offload for
> i.MX95 ENETC
> 
> Hi Claudiu,
> 
> > -----Original Message-----
> > From: Wei Fang
> > Sent: 2024年11月11日 17:26
> > To: Claudiu Manoil <claudiu.manoil@nxp.com>
> > Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> > imx@lists.linux.dev; Vladimir Oltean <vladimir.oltean@nxp.com>; Clark
> > Wang <xiaoning.wang@nxp.com>; andrew+netdev@lunn.ch;
> > davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
> > pabeni@redhat.com; Frank Li <frank.li@nxp.com>
> > Subject: RE: [PATCH v2 net-next 2/5] net: enetc: add Tx checksum
> > offload for
> > i.MX95 ENETC
> >
> > > > --- a/drivers/net/ethernet/freescale/enetc/enetc.c
> > > > +++ b/drivers/net/ethernet/freescale/enetc/enetc.c
> > > > @@ -143,6 +143,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) {
> > > > +	if (ip_hdr(skb)->version == 4)
> > >
> > > I would avoid using ip_hdr(), or any form of touching packed data
> > > and try extract this kind of info directly from the skb metadata
> > > instead, see also comment below.
> > >
> > > i.e., why not:
> > > if (skb->protocol == htons(ETH_P_IPV6)) ..  etc. ?
> >
> > skb->protocol may be VLAN protocol, such as ETH_P_8021Q,
> ETH_P_8021AD.
> > If so, it is impossible to determine whether it is an IPv4 or IPv6
> > frames through protocol.
> >
> > > or
> > > switch (skb->csum_offset) {
> > > case offsetof(struct tcphdr, check):
> > > [...]
> > > case offsetof(struct udphdr, check):
> > > [...]
> >
> > This seems to be able to be used to determine whether it is a UDP or
> > TCP frame.
> > Thanks.
> >
> > >
> > > > +		return ip_hdr(skb)->protocol == IPPROTO_TCP ||
> > > > +		       ip_hdr(skb)->protocol == IPPROTO_UDP;
> > > > +
> > > > +	if (ip_hdr(skb)->version == 6)
> > > > +		return ipv6_hdr(skb)->nexthdr == NEXTHDR_TCP ||
> > > > +		       ipv6_hdr(skb)->nexthdr == NEXTHDR_UDP;
> > > > +
> > > > +	return false;
> > > > +}
> > > > +
> > > > +static bool enetc_skb_is_tcp(struct sk_buff *skb) {
> > >
> > > There is a more efficient way of checking if L4 is TCP, without
> > > touching packet data, i.e. through the 'csum_offset' skb field:
> > > return skb->csum_offset == offsetof(struct tcphdr, check);
> > >
> > > Pls. have a look at these optimizations, I would expect visible
> > > improvements in throughput. Thanks.
> >
> > For small packets this might improve performance, but I'm not sure if
> > it would be a significant improvement. :)
> >
> 
> I didn't see any visible improvements in performance after using csum_offset.
> For example, when using pktgen to send 10,000,000 packets, the time taken is
> almost the same regardless of whether they are large or small packets, and the
> CPU idle ratio seen through the top command is also basically the same. Also,
> the UDP performance tested by iperf3 is basically the same.
> 

Maybe there's a bigger bottleneck somewhere else. I would change enetc_skb_is_tcp()
regardless, instead of 'if (ip_hdr() == 4) ... else ...', you can have the one line return statement
above.

As commented before, I would try to get rid of any access to packet content if skb metadata
fields could be used instead, but I don have a solution now on how to retrieve the IP protocol
version this way.

Thanks for testing anyway.
Claudiu Manoil Nov. 11, 2024, 4:10 p.m. UTC | #5
> -----Original Message-----
> From: Wei Fang <wei.fang@nxp.com>
> Sent: Monday, November 11, 2024 11:26 AM
[...]
> Subject: RE: [PATCH v2 net-next 2/5] net: enetc: add Tx checksum offload for
> i.MX95 ENETC
> 
> > > --- a/drivers/net/ethernet/freescale/enetc/enetc.c
> > > +++ b/drivers/net/ethernet/freescale/enetc/enetc.c
> > > @@ -143,6 +143,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) {
> > > +	if (ip_hdr(skb)->version == 4)
> >
> > I would avoid using ip_hdr(), or any form of touching packed data and
> > try extract this kind of info directly from the skb metadata instead,
> > see also comment below.
> >
> > i.e., why not:
> > if (skb->protocol == htons(ETH_P_IPV6)) ..  etc. ?
> 
> skb->protocol may be VLAN protocol, such as ETH_P_8021Q, ETH_P_8021AD.
> If so, it is impossible to determine whether it is an IPv4 or IPv6 frames through
> protocol.
> 

vlan_get_protocol() then?
I see you're using this helper in the LSO patch, so let's be consistent then. :)
I still think it's better than the ip_hdr(skb) approach.
Wei Fang Nov. 12, 2024, 1:24 a.m. UTC | #6
> > > -----Original Message-----
> > > From: Wei Fang
> > > Sent: 2024年11月11日 17:26
> > > To: Claudiu Manoil <claudiu.manoil@nxp.com>
> > > Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org;
> > > imx@lists.linux.dev; Vladimir Oltean <vladimir.oltean@nxp.com>; Clark
> > > Wang <xiaoning.wang@nxp.com>; andrew+netdev@lunn.ch;
> > > davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
> > > pabeni@redhat.com; Frank Li <frank.li@nxp.com>
> > > Subject: RE: [PATCH v2 net-next 2/5] net: enetc: add Tx checksum
> > > offload for
> > > i.MX95 ENETC
> > >
> > > > > --- a/drivers/net/ethernet/freescale/enetc/enetc.c
> > > > > +++ b/drivers/net/ethernet/freescale/enetc/enetc.c
> > > > > @@ -143,6 +143,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) {
> > > > > +	if (ip_hdr(skb)->version == 4)
> > > >
> > > > I would avoid using ip_hdr(), or any form of touching packed data
> > > > and try extract this kind of info directly from the skb metadata
> > > > instead, see also comment below.
> > > >
> > > > i.e., why not:
> > > > if (skb->protocol == htons(ETH_P_IPV6)) ..  etc. ?
> > >
> > > skb->protocol may be VLAN protocol, such as ETH_P_8021Q,
> > ETH_P_8021AD.
> > > If so, it is impossible to determine whether it is an IPv4 or IPv6
> > > frames through protocol.
> > >
> > > > or
> > > > switch (skb->csum_offset) {
> > > > case offsetof(struct tcphdr, check):
> > > > [...]
> > > > case offsetof(struct udphdr, check):
> > > > [...]
> > >
> > > This seems to be able to be used to determine whether it is a UDP or
> > > TCP frame.
> > > Thanks.
> > >
> > > >
> > > > > +		return ip_hdr(skb)->protocol == IPPROTO_TCP ||
> > > > > +		       ip_hdr(skb)->protocol == IPPROTO_UDP;
> > > > > +
> > > > > +	if (ip_hdr(skb)->version == 6)
> > > > > +		return ipv6_hdr(skb)->nexthdr == NEXTHDR_TCP ||
> > > > > +		       ipv6_hdr(skb)->nexthdr == NEXTHDR_UDP;
> > > > > +
> > > > > +	return false;
> > > > > +}
> > > > > +
> > > > > +static bool enetc_skb_is_tcp(struct sk_buff *skb) {
> > > >
> > > > There is a more efficient way of checking if L4 is TCP, without
> > > > touching packet data, i.e. through the 'csum_offset' skb field:
> > > > return skb->csum_offset == offsetof(struct tcphdr, check);
> > > >
> > > > Pls. have a look at these optimizations, I would expect visible
> > > > improvements in throughput. Thanks.
> > >
> > > For small packets this might improve performance, but I'm not sure if
> > > it would be a significant improvement. :)
> > >
> >
> > I didn't see any visible improvements in performance after using csum_offset.
> > For example, when using pktgen to send 10,000,000 packets, the time taken
> is
> > almost the same regardless of whether they are large or small packets, and
> the
> > CPU idle ratio seen through the top command is also basically the same.
> Also,
> > the UDP performance tested by iperf3 is basically the same.
> >
> 
> Maybe there's a bigger bottleneck somewhere else. I would change
> enetc_skb_is_tcp()
> regardless, instead of 'if (ip_hdr() == 4) ... else ...', you can have the one line
> return statement
> above.
> 

Yeah, I can refine enetc_skb_is_tcp() and enetc_tx_csum_offload_check()
through csum_offset, this is logically simpler.

> As commented before, I would try to get rid of any access to packet content if
> skb metadata
> fields could be used instead, but I don have a solution now on how to retrieve
> the IP protocol
> version this way.
> 
> Thanks for testing anyway.
>
Wei Fang Nov. 12, 2024, 1:30 a.m. UTC | #7
> -----Original Message-----
> From: Claudiu Manoil <claudiu.manoil@nxp.com>
> Sent: 2024年11月12日 0:11
> To: Wei Fang <wei.fang@nxp.com>
> Cc: netdev@vger.kernel.org; linux-kernel@vger.kernel.org; imx@lists.linux.dev;
> Vladimir Oltean <vladimir.oltean@nxp.com>; Clark Wang
> <xiaoning.wang@nxp.com>; andrew+netdev@lunn.ch; davem@davemloft.net;
> edumazet@google.com; kuba@kernel.org; pabeni@redhat.com; Frank Li
> <frank.li@nxp.com>
> Subject: RE: [PATCH v2 net-next 2/5] net: enetc: add Tx checksum offload for
> i.MX95 ENETC
> 
> > -----Original Message-----
> > From: Wei Fang <wei.fang@nxp.com>
> > Sent: Monday, November 11, 2024 11:26 AM
> [...]
> > Subject: RE: [PATCH v2 net-next 2/5] net: enetc: add Tx checksum offload for
> > i.MX95 ENETC
> >
> > > > --- a/drivers/net/ethernet/freescale/enetc/enetc.c
> > > > +++ b/drivers/net/ethernet/freescale/enetc/enetc.c
> > > > @@ -143,6 +143,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) {
> > > > +	if (ip_hdr(skb)->version == 4)
> > >
> > > I would avoid using ip_hdr(), or any form of touching packed data and
> > > try extract this kind of info directly from the skb metadata instead,
> > > see also comment below.
> > >
> > > i.e., why not:
> > > if (skb->protocol == htons(ETH_P_IPV6)) ..  etc. ?
> >
> > skb->protocol may be VLAN protocol, such as ETH_P_8021Q,
> ETH_P_8021AD.
> > If so, it is impossible to determine whether it is an IPv4 or IPv6 frames
> through
> > protocol.
> >
> 
> vlan_get_protocol() then?
> I see you're using this helper in the LSO patch, so let's be consistent then. :)
> I still think it's better than the ip_hdr(skb) approach.

Yes, vlan_get_protocol() can also help us get the IP version, so let's be
consistent. Thanks.
diff mbox series

Patch

diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c b/drivers/net/ethernet/freescale/enetc/enetc.c
index 3137b6ee62d3..502194317a96 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc.c
@@ -143,6 +143,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)
+{
+	if (ip_hdr(skb)->version == 4)
+		return ip_hdr(skb)->protocol == IPPROTO_TCP ||
+		       ip_hdr(skb)->protocol == IPPROTO_UDP;
+
+	if (ip_hdr(skb)->version == 6)
+		return ipv6_hdr(skb)->nexthdr == NEXTHDR_TCP ||
+		       ipv6_hdr(skb)->nexthdr == NEXTHDR_UDP;
+
+	return false;
+}
+
+static bool enetc_skb_is_tcp(struct sk_buff *skb)
+{
+	if (ip_hdr(skb)->version == 4)
+		return ip_hdr(skb)->protocol == IPPROTO_TCP;
+	else
+		return ipv6_hdr(skb)->nexthdr == NEXTHDR_TCP;
+}
+
 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;
@@ -160,6 +181,29 @@  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) {
+			bool is_ipv6 = ip_hdr(skb)->version == 6;
+			bool is_tcp = enetc_skb_is_tcp(skb);
+
+			temp_bd.l3_start = skb_network_offset(skb);
+			temp_bd.ipcs = is_ipv6 ? 0 : 1;
+			temp_bd.l3_hdr_size = skb_network_header_len(skb) / 4;
+			temp_bd.l3t = is_ipv6 ? 1 : 0;
+			temp_bd.l4t = is_tcp ? ENETC_TXBD_L4T_TCP : ENETC_TXBD_L4T_UDP;
+			flags |= ENETC_TXBD_FLAGS_CSUM_LSO | ENETC_TXBD_FLAGS_L4CS;
+		} else {
+			if (skb_checksum_help(skb)) {
+				dev_err(tx_ring->dev, "skb_checksum_help() error\n");
+
+				return 0;
+			}
+		}
+	}
+
 	i = tx_ring->next_to_use;
 	txbd = ENETC_TXBD(*tx_ring, i);
 	prefetchw(txbd);
@@ -170,7 +214,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;
@@ -591,7 +634,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) {
@@ -624,11 +667,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();
@@ -3287,6 +3325,7 @@  static const struct enetc_drvdata enetc4_pf_data = {
 	.sysclk_freq = ENETC_CLK_333M,
 	.pmac_offset = ENETC4_PMAC_OFFSET,
 	.rx_csum = 1,
+	.tx_csum = 1,
 	.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 5b65f79e05be..ee11ff97e9ed 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc.h
+++ b/drivers/net/ethernet/freescale/enetc/enetc.h
@@ -235,6 +235,7 @@  enum enetc_errata {
 struct enetc_drvdata {
 	u32 pmac_offset; /* Only valid for PSI which supports 802.1Qbu */
 	u8 rx_csum:1;
+	u8 tx_csum:1;
 	u64 sysclk_freq;
 	const struct ethtool_ops *eth_ops;
 };
@@ -343,6 +344,7 @@  enum enetc_active_offloads {
 	ENETC_F_QCI			= BIT(10),
 	ENETC_F_QBU			= BIT(11),
 	ENETC_F_RXCSUM			= BIT(12),
+	ENETC_F_TXCSUM			= BIT(13),
 };
 
 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 4b8fd1879005..590b1412fadf 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_hw.h
+++ b/drivers/net/ethernet/freescale/enetc/enetc_hw.h
@@ -558,7 +558,12 @@  union enetc_tx_bd {
 		__le16 frm_len;
 		union {
 			struct {
-				u8 reserved[3];
+				u8 l3_start:7;
+				u8 ipcs:1;
+				u8 l3_hdr_size:7;
+				u8 l3t:1;
+				u8 resv:5;
+				u8 l4t:3;
 				u8 flags;
 			}; /* default layout */
 			__le32 txstart;
@@ -582,10 +587,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)
@@ -594,6 +599,9 @@  enum enetc_txbd_flags {
 #define ENETC_TXBD_TXSTART_MASK GENMASK(24, 0)
 #define ENETC_TXBD_FLAGS_OFFSET 24
 
+#define ENETC_TXBD_L4T_UDP	BIT(0)
+#define ENETC_TXBD_L4T_TCP	BIT(1)
+
 static inline __le32 enetc_txbd_set_tx_start(u64 tx_start, u8 flags)
 {
 	u32 temp;
diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf_common.c b/drivers/net/ethernet/freescale/enetc/enetc_pf_common.c
index 91e79582a541..3a8a5b6d8c26 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_pf_common.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_pf_common.c
@@ -122,6 +122,9 @@  void enetc_pf_netdev_setup(struct enetc_si *si, struct net_device *ndev,
 	if (si->drvdata->rx_csum)
 		priv->active_offloads |= ENETC_F_RXCSUM;
 
+	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);