diff mbox series

[net-next,1/3] net: core: add helper tcp_v6_gso_csum_prep

Message ID 9fdc5f0c-fdf0-122e-48a5-43ff029cf8d9@gmail.com (mailing list archive)
State New, archived
Headers show
Series net: core: add helper tcp_v6_gso_csum_prep | expand

Commit Message

Heiner Kallweit Feb. 17, 2020, 9:40 p.m. UTC
Several network drivers for chips that support TSO6 share the same code
for preparing the TCP header. A difference is that some reset the
payload_len whilst others don't do this. Let's factor out this common
code to a new helper.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 include/net/ip6_checksum.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

Comments

Alexander Duyck Feb. 18, 2020, 6:25 p.m. UTC | #1
On Mon, Feb 17, 2020 at 1:41 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>
> Several network drivers for chips that support TSO6 share the same code
> for preparing the TCP header. A difference is that some reset the
> payload_len whilst others don't do this. Let's factor out this common
> code to a new helper.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
>  include/net/ip6_checksum.h | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/include/net/ip6_checksum.h b/include/net/ip6_checksum.h
> index 7bec95df4..ef0130023 100644
> --- a/include/net/ip6_checksum.h
> +++ b/include/net/ip6_checksum.h
> @@ -76,6 +76,18 @@ static inline void __tcp_v6_send_check(struct sk_buff *skb,
>         }
>  }
>
> +static inline void tcp_v6_gso_csum_prep(struct sk_buff *skb,
> +                                       bool clear_payload_len)
> +{
> +       struct ipv6hdr *ipv6h = ipv6_hdr(skb);
> +       struct tcphdr *th = tcp_hdr(skb);
> +
> +       if (clear_payload_len)
> +               ipv6h->payload_len = 0;
> +
> +       th->check = ~tcp_v6_check(0, &ipv6h->saddr, &ipv6h->daddr, 0);
> +}
> +
>  #if IS_ENABLED(CONFIG_IPV6)
>  static inline void tcp_v6_send_check(struct sock *sk, struct sk_buff *skb)
>  {

So functionally I believe this is correct. The only piece I have a
question about is if we should just force the clear_payload_len as
always being the case since the value should either be
ignored/overwritten in any GSO case anyway.

Reviewed-by: Alexander Duyck <alexander.h.duyck@linux.intel.com>
Heiner Kallweit Feb. 18, 2020, 6:53 p.m. UTC | #2
On 18.02.2020 19:25, Alexander Duyck wrote:
> On Mon, Feb 17, 2020 at 1:41 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
>>
>> Several network drivers for chips that support TSO6 share the same code
>> for preparing the TCP header. A difference is that some reset the
>> payload_len whilst others don't do this. Let's factor out this common
>> code to a new helper.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>> ---
>>  include/net/ip6_checksum.h | 12 ++++++++++++
>>  1 file changed, 12 insertions(+)
>>
>> diff --git a/include/net/ip6_checksum.h b/include/net/ip6_checksum.h
>> index 7bec95df4..ef0130023 100644
>> --- a/include/net/ip6_checksum.h
>> +++ b/include/net/ip6_checksum.h
>> @@ -76,6 +76,18 @@ static inline void __tcp_v6_send_check(struct sk_buff *skb,
>>         }
>>  }
>>
>> +static inline void tcp_v6_gso_csum_prep(struct sk_buff *skb,
>> +                                       bool clear_payload_len)
>> +{
>> +       struct ipv6hdr *ipv6h = ipv6_hdr(skb);
>> +       struct tcphdr *th = tcp_hdr(skb);
>> +
>> +       if (clear_payload_len)
>> +               ipv6h->payload_len = 0;
>> +
>> +       th->check = ~tcp_v6_check(0, &ipv6h->saddr, &ipv6h->daddr, 0);
>> +}
>> +
>>  #if IS_ENABLED(CONFIG_IPV6)
>>  static inline void tcp_v6_send_check(struct sock *sk, struct sk_buff *skb)
>>  {
> 
> So functionally I believe this is correct. The only piece I have a
> question about is if we should just force the clear_payload_len as
> always being the case since the value should either be
> ignored/overwritten in any GSO case anyway.
> 
I also thought about this and just wasn't sure whether this functional
change may break any driver. But yes, then let's change it this way.
Breaking down the series into smaller patches makes it easier to
revert an individual patch in case of a problem.

> Reviewed-by: Alexander Duyck <alexander.h.duyck@linux.intel.com>
> 
Thanks for the review.
diff mbox series

Patch

diff --git a/include/net/ip6_checksum.h b/include/net/ip6_checksum.h
index 7bec95df4..ef0130023 100644
--- a/include/net/ip6_checksum.h
+++ b/include/net/ip6_checksum.h
@@ -76,6 +76,18 @@  static inline void __tcp_v6_send_check(struct sk_buff *skb,
 	}
 }
 
+static inline void tcp_v6_gso_csum_prep(struct sk_buff *skb,
+					bool clear_payload_len)
+{
+	struct ipv6hdr *ipv6h = ipv6_hdr(skb);
+	struct tcphdr *th = tcp_hdr(skb);
+
+	if (clear_payload_len)
+		ipv6h->payload_len = 0;
+
+	th->check = ~tcp_v6_check(0, &ipv6h->saddr, &ipv6h->daddr, 0);
+}
+
 #if IS_ENABLED(CONFIG_IPV6)
 static inline void tcp_v6_send_check(struct sock *sk, struct sk_buff *skb)
 {