netdev - Re: [PATCH net-next 03/14] udp: Don't set skb->encapsulation with RCO

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 4 May 2016 19:42:31 -0700
From:	Alexander Duyck <alexander.duyck@...il.com>
To:	Tom Herbert <tom@...bertland.com>
Cc:	David Miller <davem@...emloft.net>,
	Netdev <netdev@...r.kernel.org>, Kernel Team <kernel-team@...com>
Subject: Re: [PATCH net-next 03/14] udp: Don't set skb->encapsulation with RCO

On Wed, May 4, 2016 at 6:02 PM, Tom Herbert <tom@...bertland.com> wrote:
> When RCO is in effect we want to ensure that the outer checksum is
> properly offloaded. Don't set skb->encapsulation in this case to
> ensure that checksum offload is later considered for hw_features
> instead of hw_enc_features.
>
> Signed-off-by: Tom Herbert <tom@...bertland.com>
> ---
>  net/ipv4/udp_offload.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
> index b556ef6..4eedec6 100644
> --- a/net/ipv4/udp_offload.c
> +++ b/net/ipv4/udp_offload.c
> @@ -94,11 +94,13 @@ static struct sk_buff *__skb_udp_tunnel_segment(struct sk_buff *skb,
>         do {
>                 unsigned int len;
>
> -               if (remcsum)
> +               if (remcsum) {
>                         skb->ip_summed = CHECKSUM_NONE;
> -
> -               /* Set up inner headers if we are offloading inner checksum */
> -               if (skb->ip_summed == CHECKSUM_PARTIAL) {
> +                       skb->encapsulation = 0;
> +               } else if (skb->ip_summed == CHECKSUM_PARTIAL) {
> +                       /* Set up inner headers if we are offloading inner
> +                        * checksum
> +                        */
>                         skb_reset_inner_headers(skb);
>                         skb->encapsulation = 1;
>                 }


Why are you wasting cycles clearing a value that should have already
been cleared?

We set skb->encapsulation to 0 for the incoming skb before we segment
it.  As such all of the segments we get should also not have it set.
It seems like you are just wasting cycles writing it again even though
it isn't written.

- Alex