[PATCH 8/8] vxlan: Add support for zero UDP6 checksums

[PATCH 8/8] vxlan: Add support for zero UDP6 checksums

[Tom Herbert]

Added module parameters to specify sending of TX checksums and
allow receiving RX checksums. Also added a module parameter and
support for using IPv4 checksums on TX (default is off so zero
checksums are sent).

Signed-off-by: Tom Herbert <therbert@google.com>
---
 drivers/net/vxlan.c | 42 ++++++++++++++++++++++++++----------------
 1 file changed, 26 insertions(+), 16 deletions(-)

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 1dfee9a..4839edb 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -83,6 +83,18 @@ static bool log_ecn_error = true;
 module_param(log_ecn_error, bool, 0644);
 MODULE_PARM_DESC(log_ecn_error, "Log packets received with corrupted ECN");
 
+static bool use_udp_checksums;
+module_param(use_udp_checksums, bool, 0644);
+MODULE_PARM_DESC(use_udp_checksums, "Send UDP checksums (IPv4)");
+
+static bool udp6_tx_zero_checksums;
+module_param(udp6_tx_zero_checksums, bool, 0644);
+MODULE_PARM_DESC(udp6_tx_zero_checksums, "Send zero IPv6 checksums");
+
+static bool udp6_rx_zero_checksums;
+module_param(udp6_rx_zero_checksums, bool, 0644);
+MODULE_PARM_DESC(udp6_rx_zero_checksums, "Receive zero IPv6 checksums");
+
 static int vxlan_net_id;
 
 static const u8 all_zeros_mac[ETH_ALEN];
@@ -1666,27 +1678,13 @@ static int vxlan6_xmit_skb(struct vxlan_sock *vs,
 	uh->source = src_port;
 
 	uh->len = htons(skb->len);
-	uh->check = 0;
 
 	memset(&(IPCB(skb)->opt), 0, sizeof(IPCB(skb)->opt));
 	IPCB(skb)->flags &= ~(IPSKB_XFRM_TUNNEL_SIZE | IPSKB_XFRM_TRANSFORMED |
 			      IPSKB_REROUTED);
 	skb_dst_set(skb, dst);
 
-	if (!skb_is_gso(skb) && !(dst->dev->features & NETIF_F_IPV6_CSUM)) {
-		__wsum csum = skb_checksum(skb, 0, skb->len, 0);
-		skb->ip_summed = CHECKSUM_UNNECESSARY;
-		uh->check = csum_ipv6_magic(saddr, daddr, skb->len,
-					    IPPROTO_UDP, csum);
-		if (uh->check == 0)
-			uh->check = CSUM_MANGLED_0;
-	} else {
-		skb->ip_summed = CHECKSUM_PARTIAL;
-		skb->csum_start = skb_transport_header(skb) - skb->head;
-		skb->csum_offset = offsetof(struct udphdr, check);
-		uh->check = ~csum_ipv6_magic(saddr, daddr,
-					     skb->len, IPPROTO_UDP, 0);
-	}
+	udp6_set_csum(vs->sock->sk, skb, saddr, daddr, skb->len);
 
 	__skb_push(skb, sizeof(*ip6h));
 	skb_reset_network_header(skb);
@@ -1756,7 +1754,8 @@ int vxlan_xmit_skb(struct vxlan_sock *vs,
 	uh->source = src_port;
 
 	uh->len = htons(skb->len);
-	uh->check = 0;
+
+	udp_set_csum(vs->sock->sk, skb, src, dst, skb->len);
 
 	err = handle_offloads(skb);
 	if (err)
@@ -2442,6 +2441,13 @@ static struct socket *create_v6_sock(struct net *net, __be16 port)
 
 	/* Disable multicast loopback */
 	inet_sk(sk)->mc_loop = 0;
+
+	if (udp6_rx_zero_checksums)
+		udp_set_no_check6_rx(sk, true);
+
+	if (udp6_tx_zero_checksums)
+		udp_set_no_check6_tx(sk, true);
+
 	return sock;
 }
 
@@ -2486,6 +2492,10 @@ static struct socket *create_v4_sock(struct net *net, __be16 port)
 
 	/* Disable multicast loopback */
 	inet_sk(sk)->mc_loop = 0;
+
+	if (!use_udp_checksums)
+		sock->sk->sk_no_check_tx = 1;
+
 	return sock;
 }
 
-- 
1.9.1.423.g4596e3a

Re: [PATCH 8/8] vxlan: Add support for zero UDP6 checksums

[Jesse Gross]

On Mon, May 12, 2014 at 9:59 AM, Tom Herbert <therbert@google.com> wrote:
> @@ -1756,7 +1754,8 @@ int vxlan_xmit_skb(struct vxlan_sock *vs,
>         uh->source = src_port;
>
>         uh->len = htons(skb->len);
> -       uh->check = 0;
> +
> +       udp_set_csum(vs->sock->sk, skb, src, dst, skb->len);

How does this interact with inner checksum offloading? I don't think
that we have any infrastructure to support computing two checksums on
a packet yet.

Re: [PATCH 8/8] vxlan: Add support for zero UDP6 checksums

[Tom Herbert]

On Mon, May 12, 2014 at 2:41 PM, Jesse Gross <jesse@nicira.com> wrote:
> On Mon, May 12, 2014 at 9:59 AM, Tom Herbert <therbert@google.com> wrote:
>> @@ -1756,7 +1754,8 @@ int vxlan_xmit_skb(struct vxlan_sock *vs,
>>         uh->source = src_port;
>>
>>         uh->len = htons(skb->len);
>> -       uh->check = 0;
>> +
>> +       udp_set_csum(vs->sock->sk, skb, src, dst, skb->len);
>
> How does this interact with inner checksum offloading? I don't think
> that we have any infrastructure to support computing two checksums on
> a packet yet.

Right, as it stands now we could overwrite the csum offload parameters
for the inner header which would be a problem. I will fix that.

The ability to offload two csums in TX could be quite useful for
encapsulations like vxlan. I don't know if any HW is planning to
support that...

Re: [PATCH 8/8] vxlan: Add support for zero UDP6 checksums

[Jesse Gross]

On Mon, May 12, 2014 at 4:35 PM, Tom Herbert <therbert@google.com> wrote:
> On Mon, May 12, 2014 at 2:41 PM, Jesse Gross <jesse@nicira.com> wrote:
>> On Mon, May 12, 2014 at 9:59 AM, Tom Herbert <therbert@google.com> wrote:
>>> @@ -1756,7 +1754,8 @@ int vxlan_xmit_skb(struct vxlan_sock *vs,
>>>         uh->source = src_port;
>>>
>>>         uh->len = htons(skb->len);
>>> -       uh->check = 0;
>>> +
>>> +       udp_set_csum(vs->sock->sk, skb, src, dst, skb->len);
>>
>> How does this interact with inner checksum offloading? I don't think
>> that we have any infrastructure to support computing two checksums on
>> a packet yet.
>
> Right, as it stands now we could overwrite the csum offload parameters
> for the inner header which would be a problem. I will fix that.
>
> The ability to offload two csums in TX could be quite useful for
> encapsulations like vxlan. I don't know if any HW is planning to
> support that...

I think it would be good to assume that we might want to do that as
something might appear in the not too distant future...

Re: [PATCH 8/8] vxlan: Add support for zero UDP6 checksums

[Or Gerlitz]

On Mon, May 12, 2014 at 7:59 PM, Tom Herbert <therbert@google.com> wrote:
> Added module parameters to specify sending of TX checksums and
> allow receiving RX checksums. Also added a module parameter and
> support for using IPv4 checksums on TX

Hi Tom,

So... module params have their limited (which could turn to be nasty)
nature, do we really want to have them here (which practically also
means supporting them forever) or maybe/better use some more
programmable method to control that?

> (default is off so zero checksums are sent).

Or.




>
> Signed-off-by: Tom Herbert <therbert@google.com>
> ---
>  drivers/net/vxlan.c | 42 ++++++++++++++++++++++++++----------------
>  1 file changed, 26 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
> index 1dfee9a..4839edb 100644
> --- a/drivers/net/vxlan.c
> +++ b/drivers/net/vxlan.c
> @@ -83,6 +83,18 @@ static bool log_ecn_error = true;
>  module_param(log_ecn_error, bool, 0644);
>  MODULE_PARM_DESC(log_ecn_error, "Log packets received with corrupted ECN");
>
> +static bool use_udp_checksums;
> +module_param(use_udp_checksums, bool, 0644);
> +MODULE_PARM_DESC(use_udp_checksums, "Send UDP checksums (IPv4)");
> +
> +static bool udp6_tx_zero_checksums;
> +module_param(udp6_tx_zero_checksums, bool, 0644);
> +MODULE_PARM_DESC(udp6_tx_zero_checksums, "Send zero IPv6 checksums");
> +
> +static bool udp6_rx_zero_checksums;
> +module_param(udp6_rx_zero_checksums, bool, 0644);
> +MODULE_PARM_DESC(udp6_rx_zero_checksums, "Receive zero IPv6 checksums");
> +
>  static int vxlan_net_id;
>
>  static const u8 all_zeros_mac[ETH_ALEN];
> @@ -1666,27 +1678,13 @@ static int vxlan6_xmit_skb(struct vxlan_sock *vs,
>         uh->source = src_port;
>
>         uh->len = htons(skb->len);
> -       uh->check = 0;
>
>         memset(&(IPCB(skb)->opt), 0, sizeof(IPCB(skb)->opt));
>         IPCB(skb)->flags &= ~(IPSKB_XFRM_TUNNEL_SIZE | IPSKB_XFRM_TRANSFORMED |
>                               IPSKB_REROUTED);
>         skb_dst_set(skb, dst);
>
> -       if (!skb_is_gso(skb) && !(dst->dev->features & NETIF_F_IPV6_CSUM)) {
> -               __wsum csum = skb_checksum(skb, 0, skb->len, 0);
> -               skb->ip_summed = CHECKSUM_UNNECESSARY;
> -               uh->check = csum_ipv6_magic(saddr, daddr, skb->len,
> -                                           IPPROTO_UDP, csum);
> -               if (uh->check == 0)
> -                       uh->check = CSUM_MANGLED_0;
> -       } else {
> -               skb->ip_summed = CHECKSUM_PARTIAL;
> -               skb->csum_start = skb_transport_header(skb) - skb->head;
> -               skb->csum_offset = offsetof(struct udphdr, check);
> -               uh->check = ~csum_ipv6_magic(saddr, daddr,
> -                                            skb->len, IPPROTO_UDP, 0);
> -       }
> +       udp6_set_csum(vs->sock->sk, skb, saddr, daddr, skb->len);
>
>         __skb_push(skb, sizeof(*ip6h));
>         skb_reset_network_header(skb);
> @@ -1756,7 +1754,8 @@ int vxlan_xmit_skb(struct vxlan_sock *vs,
>         uh->source = src_port;
>
>         uh->len = htons(skb->len);
> -       uh->check = 0;
> +
> +       udp_set_csum(vs->sock->sk, skb, src, dst, skb->len);
>
>         err = handle_offloads(skb);
>         if (err)
> @@ -2442,6 +2441,13 @@ static struct socket *create_v6_sock(struct net *net, __be16 port)
>
>         /* Disable multicast loopback */
>         inet_sk(sk)->mc_loop = 0;
> +
> +       if (udp6_rx_zero_checksums)
> +               udp_set_no_check6_rx(sk, true);
> +
> +       if (udp6_tx_zero_checksums)
> +               udp_set_no_check6_tx(sk, true);
> +
>         return sock;
>  }
>
> @@ -2486,6 +2492,10 @@ static struct socket *create_v4_sock(struct net *net, __be16 port)
>
>         /* Disable multicast loopback */
>         inet_sk(sk)->mc_loop = 0;
> +
> +       if (!use_udp_checksums)
> +               sock->sk->sk_no_check_tx = 1;
> +
>         return sock;
>  }
>
> --
> 1.9.1.423.g4596e3a
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 8/8] vxlan: Add support for zero UDP6 checksums

[David Miller]

From: Tom Herbert <therbert@google.com>
Date: Mon, 12 May 2014 09:59:25 -0700 (PDT)

> Added module parameters to specify sending of TX checksums and
> allow receiving RX checksums. Also added a module parameter and
> support for using IPv4 checksums on TX (default is off so zero
> checksums are sent).
> 
> Signed-off-by: Tom Herbert <therbert@google.com>

Tom, definitely do not do this using module parameters.

Extend the vxlan configuration to suppose this switch using netlink.

Thanks.

Re: [PATCH 8/8] vxlan: Add support for zero UDP6 checksums

[Tom Herbert]

On Tue, May 13, 2014 at 3:48 PM, David Miller <davem@davemloft.net> wrote:
> From: Tom Herbert <therbert@google.com>
> Date: Mon, 12 May 2014 09:59:25 -0700 (PDT)
>
>> Added module parameters to specify sending of TX checksums and
>> allow receiving RX checksums. Also added a module parameter and
>> support for using IPv4 checksums on TX (default is off so zero
>> checksums are sent).
>>
>> Signed-off-by: Tom Herbert <therbert@google.com>
>
> Tom, definitely do not do this using module parameters.
>
> Extend the vxlan configuration to suppose this switch using netlink.
>
Acknowledged.

> Thanks.

Re: [PATCH 8/8] vxlan: Add support for zero UDP6 checksums

[Or Gerlitz]

On Wed, May 14, 2014 at 1:48 AM, David Miller <davem@davemloft.net> wrote:
>
> From: Tom Herbert <therbert@google.com>
> Date: Mon, 12 May 2014 09:59:25 -0700 (PDT)
>
> > Added module parameters to specify sending of TX checksums and
> > allow receiving RX checksums. Also added a module parameter and
> > support for using IPv4 checksums on TX (default is off so zero
> > checksums are sent).
>
> Tom, definitely do not do this using module parameters.
> Extend the vxlan configuration to suppose this switch using netlink.
>

Tom,

FWIW, note that the netlink code which relates to vxlan netdevice
creation through rtnl link ops itself doesn't come
into play for the openvswitch vxlan use case, the shared section
between OVS vxlan ports and the rtnl link ops one
vxlan_sock_add() - so you might choose to propagate  the new values
through the ovs channel as a use case.

Or.