[PATCH] net: l2tp: fix reversed udp6 checksum flags

[PATCH] net: l2tp: fix reversed udp6 checksum flags

[Wang Shanker]

[-- Attachment #1: Type: text/plain, Size: 1554 bytes --]

This patch fixes a bug which causes the behavior of whether to ignore
udp6 checksum of udp6 encapsulated l2tp tunnel contrary to what
userspace program requests.

When the flag `L2TP_ATTR_UDP_ZERO_CSUM6_RX` is set by userspace, it is
expected that udp6 checksums of received packets of the l2tp tunnel
to create should be ignored. In `l2tp_netlink.c`:
`l2tp_nl_cmd_tunnel_create()`, `cfg.udp6_zero_rx_checksums` is set
according to the flag, and then passed to `l2tp_core.c`:
`l2tp_tunnel_create()` and then `l2tp_tunnel_sock_create()`. In
`l2tp_tunnel_sock_create()`, `udp_conf.use_udp6_rx_checksums` is set
the same to `cfg.udp6_zero_rx_checksums`. However, if we want the
checksum to be ignored, `udp_conf.use_udp6_rx_checksums` should be set
to `false`, i.e. be set to the contrary. Similarly, the same should be
done to `udp_conf.use_udp6_tx_checksums`.

Signed-off-by: Miao Wang <shankerwangmiao@gmail.com>
---
 net/l2tp/l2tp_core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
index afca2eb..6edfa99 100644
--- a/net/l2tp/l2tp_core.c
+++ b/net/l2tp/l2tp_core.c
@@ -1376,9 +1376,9 @@ static int l2tp_tunnel_sock_create(struct net *net,
 			memcpy(&udp_conf.peer_ip6, cfg->peer_ip6,
 			       sizeof(udp_conf.peer_ip6));
 			udp_conf.use_udp6_tx_checksums =
-			    cfg->udp6_zero_tx_checksums;
+			  ! cfg->udp6_zero_tx_checksums;
 			udp_conf.use_udp6_rx_checksums =
-			    cfg->udp6_zero_rx_checksums;
+			  ! cfg->udp6_zero_rx_checksums;
 		} else
 #endif
 		{
-- 
2.5.2


[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 4130 bytes --]

Re: [PATCH] net: l2tp: fix reversed udp6 checksum flags

[James Chapman]

Some additional background on this: Wang found this when configuring
l2tp tunnels using "ip l2tp" between two systems and then one system
was upgraded. The tunnel failed to pass data because one side had UDP
checksums enabled and the other now had them disabled. It seems kernel
changes related to UDP checksums resulted in a change to the default
UDP checksum setting for L2TP tunnels when using IPv6. Unfortunately,
iproute2 doesn't let the user configure L2TP UDP checksum settings, so
without this fix, some users may see problems depending on the kernel
version differences on the L2TP peers. One for stable?

Acked-by: James Chapman <jchapman@katalix.com>

On 28 April 2016 at 18:29, Wang Shanker <shankerwangmiao@gmail.com> wrote:
> This patch fixes a bug which causes the behavior of whether to ignore
> udp6 checksum of udp6 encapsulated l2tp tunnel contrary to what
> userspace program requests.
>
> When the flag `L2TP_ATTR_UDP_ZERO_CSUM6_RX` is set by userspace, it is
> expected that udp6 checksums of received packets of the l2tp tunnel
> to create should be ignored. In `l2tp_netlink.c`:
> `l2tp_nl_cmd_tunnel_create()`, `cfg.udp6_zero_rx_checksums` is set
> according to the flag, and then passed to `l2tp_core.c`:
> `l2tp_tunnel_create()` and then `l2tp_tunnel_sock_create()`. In
> `l2tp_tunnel_sock_create()`, `udp_conf.use_udp6_rx_checksums` is set
> the same to `cfg.udp6_zero_rx_checksums`. However, if we want the
> checksum to be ignored, `udp_conf.use_udp6_rx_checksums` should be set
> to `false`, i.e. be set to the contrary. Similarly, the same should be
> done to `udp_conf.use_udp6_tx_checksums`.
>
> Signed-off-by: Miao Wang <shankerwangmiao@gmail.com>
> ---
>  net/l2tp/l2tp_core.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
> index afca2eb..6edfa99 100644
> --- a/net/l2tp/l2tp_core.c
> +++ b/net/l2tp/l2tp_core.c
> @@ -1376,9 +1376,9 @@ static int l2tp_tunnel_sock_create(struct net *net,
>                         memcpy(&udp_conf.peer_ip6, cfg->peer_ip6,
>                                sizeof(udp_conf.peer_ip6));
>                         udp_conf.use_udp6_tx_checksums =
> -                           cfg->udp6_zero_tx_checksums;
> +                         ! cfg->udp6_zero_tx_checksums;
>                         udp_conf.use_udp6_rx_checksums =
> -                           cfg->udp6_zero_rx_checksums;
> +                         ! cfg->udp6_zero_rx_checksums;
>                 } else
>  #endif
>                 {
> --
> 2.5.2
>

Re: [PATCH] net: l2tp: fix reversed udp6 checksum flags

[Wang Shanker]

I think this is a logic error, rather than a change to the default
UDP checksum setting. As expected, take rx for example, the flag 
`L2TP_ATTR_UDP_ZERO_CSUM6_RX` is not set by default, and udp6 
checksum will be checked by default. The fact is that, not setting
`L2TP_ATTR_UDP_ZERO_CSUM6_RX` leads to ignoring udp6 checksum. Such
a behavior does not correspond to the name 
“L2TP_ATTR_UDP_ZERO_CSUM6_RX”. As a result, I call it a logic error.

> 在 2016年4月29日，02:46，James Chapman <jchapman@katalix.com> 写道：
> 
> Some additional background on this: Wang found this when configuring
> l2tp tunnels using "ip l2tp" between two systems and then one system
> was upgraded. The tunnel failed to pass data because one side had UDP
> checksums enabled and the other now had them disabled. It seems kernel
> changes related to UDP checksums resulted in a change to the default
> UDP checksum setting for L2TP tunnels when using IPv6. Unfortunately,
> iproute2 doesn't let the user configure L2TP UDP checksum settings, so
> without this fix, some users may see problems depending on the kernel
> version differences on the L2TP peers. One for stable?
> 
> Acked-by: James Chapman <jchapman@katalix.com>
> 
> On 28 April 2016 at 18:29, Wang Shanker <shankerwangmiao@gmail.com> wrote:
>> This patch fixes a bug which causes the behavior of whether to ignore
>> udp6 checksum of udp6 encapsulated l2tp tunnel contrary to what
>> userspace program requests.
>> 
>> When the flag `L2TP_ATTR_UDP_ZERO_CSUM6_RX` is set by userspace, it is
>> expected that udp6 checksums of received packets of the l2tp tunnel
>> to create should be ignored. In `l2tp_netlink.c`:
>> `l2tp_nl_cmd_tunnel_create()`, `cfg.udp6_zero_rx_checksums` is set
>> according to the flag, and then passed to `l2tp_core.c`:
>> `l2tp_tunnel_create()` and then `l2tp_tunnel_sock_create()`. In
>> `l2tp_tunnel_sock_create()`, `udp_conf.use_udp6_rx_checksums` is set
>> the same to `cfg.udp6_zero_rx_checksums`. However, if we want the
>> checksum to be ignored, `udp_conf.use_udp6_rx_checksums` should be set
>> to `false`, i.e. be set to the contrary. Similarly, the same should be
>> done to `udp_conf.use_udp6_tx_checksums`.
>> 
>> Signed-off-by: Miao Wang <shankerwangmiao@gmail.com>
>> ---
>> net/l2tp/l2tp_core.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>> 
>> diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
>> index afca2eb..6edfa99 100644
>> --- a/net/l2tp/l2tp_core.c
>> +++ b/net/l2tp/l2tp_core.c
>> @@ -1376,9 +1376,9 @@ static int l2tp_tunnel_sock_create(struct net *net,
>>                        memcpy(&udp_conf.peer_ip6, cfg->peer_ip6,
>>                               sizeof(udp_conf.peer_ip6));
>>                        udp_conf.use_udp6_tx_checksums =
>> -                           cfg->udp6_zero_tx_checksums;
>> +                         ! cfg->udp6_zero_tx_checksums;
>>                        udp_conf.use_udp6_rx_checksums =
>> -                           cfg->udp6_zero_rx_checksums;
>> +                         ! cfg->udp6_zero_rx_checksums;
>>                } else
>> #endif
>>                {
>> --
>> 2.5.2
>> 

Re: [PATCH] net: l2tp: fix reversed udp6 checksum flags

[Wang Shanker]

Is there any further suggestion or review?

> 在 2016年4月29日，03:25，Wang Shanker <shankerwangmiao@gmail.com> 写道：
> 
> I think this is a logic error, rather than a change to the default
> UDP checksum setting. As expected, take rx for example, the flag 
> `L2TP_ATTR_UDP_ZERO_CSUM6_RX` is not set by default, and udp6 
> checksum will be checked by default. The fact is that, not setting
> `L2TP_ATTR_UDP_ZERO_CSUM6_RX` leads to ignoring udp6 checksum. Such
> a behavior does not correspond to the name 
> “L2TP_ATTR_UDP_ZERO_CSUM6_RX”. As a result, I call it a logic error.
> 
>> 在 2016年4月29日，02:46，James Chapman <jchapman@katalix.com> 写道：
>> 
>> Some additional background on this: Wang found this when configuring
>> l2tp tunnels using "ip l2tp" between two systems and then one system
>> was upgraded. The tunnel failed to pass data because one side had UDP
>> checksums enabled and the other now had them disabled. It seems kernel
>> changes related to UDP checksums resulted in a change to the default
>> UDP checksum setting for L2TP tunnels when using IPv6. Unfortunately,
>> iproute2 doesn't let the user configure L2TP UDP checksum settings, so
>> without this fix, some users may see problems depending on the kernel
>> version differences on the L2TP peers. One for stable?
>> 
>> Acked-by: James Chapman <jchapman@katalix.com>
>> 
>> On 28 April 2016 at 18:29, Wang Shanker <shankerwangmiao@gmail.com> wrote:
>>> This patch fixes a bug which causes the behavior of whether to ignore
>>> udp6 checksum of udp6 encapsulated l2tp tunnel contrary to what
>>> userspace program requests.
>>> 
>>> When the flag `L2TP_ATTR_UDP_ZERO_CSUM6_RX` is set by userspace, it is
>>> expected that udp6 checksums of received packets of the l2tp tunnel
>>> to create should be ignored. In `l2tp_netlink.c`:
>>> `l2tp_nl_cmd_tunnel_create()`, `cfg.udp6_zero_rx_checksums` is set
>>> according to the flag, and then passed to `l2tp_core.c`:
>>> `l2tp_tunnel_create()` and then `l2tp_tunnel_sock_create()`. In
>>> `l2tp_tunnel_sock_create()`, `udp_conf.use_udp6_rx_checksums` is set
>>> the same to `cfg.udp6_zero_rx_checksums`. However, if we want the
>>> checksum to be ignored, `udp_conf.use_udp6_rx_checksums` should be set
>>> to `false`, i.e. be set to the contrary. Similarly, the same should be
>>> done to `udp_conf.use_udp6_tx_checksums`.
>>> 
>>> Signed-off-by: Miao Wang <shankerwangmiao@gmail.com>
>>> ---
>>> net/l2tp/l2tp_core.c | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>> 
>>> diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
>>> index afca2eb..6edfa99 100644
>>> --- a/net/l2tp/l2tp_core.c
>>> +++ b/net/l2tp/l2tp_core.c
>>> @@ -1376,9 +1376,9 @@ static int l2tp_tunnel_sock_create(struct net *net,
>>>                       memcpy(&udp_conf.peer_ip6, cfg->peer_ip6,
>>>                              sizeof(udp_conf.peer_ip6));
>>>                       udp_conf.use_udp6_tx_checksums =
>>> -                           cfg->udp6_zero_tx_checksums;
>>> +                         ! cfg->udp6_zero_tx_checksums;
>>>                       udp_conf.use_udp6_rx_checksums =
>>> -                           cfg->udp6_zero_rx_checksums;
>>> +                         ! cfg->udp6_zero_rx_checksums;
>>>               } else
>>> #endif
>>>               {
>>> --
>>> 2.5.2
>>> 
> 

Re: [PATCH] net: l2tp: fix reversed udp6 checksum flags

[David Miller]

From: Wang Shanker <shankerwangmiao@gmail.com>
Date: Fri, 29 Apr 2016 01:29:43 +0800

> This patch fixes a bug which causes the behavior of whether to ignore
> udp6 checksum of udp6 encapsulated l2tp tunnel contrary to what
> userspace program requests.
> 
> When the flag `L2TP_ATTR_UDP_ZERO_CSUM6_RX` is set by userspace, it is
> expected that udp6 checksums of received packets of the l2tp tunnel
> to create should be ignored. In `l2tp_netlink.c`:
> `l2tp_nl_cmd_tunnel_create()`, `cfg.udp6_zero_rx_checksums` is set
> according to the flag, and then passed to `l2tp_core.c`:
> `l2tp_tunnel_create()` and then `l2tp_tunnel_sock_create()`. In
> `l2tp_tunnel_sock_create()`, `udp_conf.use_udp6_rx_checksums` is set
> the same to `cfg.udp6_zero_rx_checksums`. However, if we want the
> checksum to be ignored, `udp_conf.use_udp6_rx_checksums` should be set
> to `false`, i.e. be set to the contrary. Similarly, the same should be
> done to `udp_conf.use_udp6_tx_checksums`.
> 
> Signed-off-by: Miao Wang <shankerwangmiao@gmail.com>

Applied, thanks.

[PATCH] net: l2tp: fix reversed udp6 checksum flags

[Wang Shanker]

This patch fixes a bug which causes the behavior of whether to ignore
udp6 checksum of udp6 encapsulated l2tp tunnel contrary to what
userspace program requests.

When the flag `L2TP_ATTR_UDP_ZERO_CSUM6_RX` is set by userspace, it is
expected that udp6 checksums of received packets of the l2tp tunnel
to create should be ignored. In `l2tp_netlink.c`:
`l2tp_nl_cmd_tunnel_create()`, `cfg.udp6_zero_rx_checksums` is set
according to the flag, and then passed to `l2tp_core.c`:
`l2tp_tunnel_create()` and then `l2tp_tunnel_sock_create()`. In
`l2tp_tunnel_sock_create()`, `udp_conf.use_udp6_rx_checksums` is set
the same to `cfg.udp6_zero_rx_checksums`. However, if we want the
checksum to be ignored, `udp_conf.use_udp6_rx_checksums` should be set
to `false`, i.e. be set to the contrary. Similarly, the same should be
done to `udp_conf.use_udp6_tx_checksums`.

Signed-off-by: Miao Wang <shankerwangmiao@gmail.com>
---
net/l2tp/l2tp_core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/l2tp/l2tp_core.c b/net/l2tp/l2tp_core.c
index afca2eb..6edfa99 100644
--- a/net/l2tp/l2tp_core.c
+++ b/net/l2tp/l2tp_core.c
@@ -1376,9 +1376,9 @@ static int l2tp_tunnel_sock_create(struct net *net,
			memcpy(&udp_conf.peer_ip6, cfg->peer_ip6,
			       sizeof(udp_conf.peer_ip6));
			udp_conf.use_udp6_tx_checksums =
-			    cfg->udp6_zero_tx_checksums;
+			  ! cfg->udp6_zero_tx_checksums;
			udp_conf.use_udp6_rx_checksums =
-			    cfg->udp6_zero_rx_checksums;
+			  ! cfg->udp6_zero_rx_checksums;
		} else
#endif
		{
-- 
2.5.2