* [PATCH v2] ip6tnl: do route updating for redirect in ip6_tnl_err()
@ 2013-09-18 3:27 Duan Jiong
2013-09-19 18:02 ` David Miller
0 siblings, 1 reply; 3+ messages in thread
From: Duan Jiong @ 2013-09-18 3:27 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Duan Jiong
After the ip6_tnl_err() is called, the rel_msg is assigned to
0, and the ip4ip6_err() will return directly, the instruction
below will not be executed.
In rfc2473, we can know that the tunnel ICMP redirect message
should not be reported to the source of the original packet,
so we can handle it in ip6_tnl_err().
Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com>
---
net/ipv6/ip6_tunnel.c | 8 +++-----
1 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
index 61355f7..48034d8 100644
--- a/net/ipv6/ip6_tunnel.c
+++ b/net/ipv6/ip6_tunnel.c
@@ -529,6 +529,9 @@ ip6_tnl_err(struct sk_buff *skb, __u8 ipproto, struct inet6_skb_parm *opt,
rel_msg = 1;
}
break;
+ case NDISC_REDIRECT:
+ ip6_redirect(skb, dev_net(skb->dev), skb->dev->ifindex, 0);
+ break;
}
*type = rel_type;
@@ -576,9 +579,6 @@ ip4ip6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
rel_type = ICMP_DEST_UNREACH;
rel_code = ICMP_FRAG_NEEDED;
break;
- case NDISC_REDIRECT:
- rel_type = ICMP_REDIRECT;
- rel_code = ICMP_REDIR_HOST;
default:
return 0;
}
@@ -637,8 +637,6 @@ ip4ip6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
skb_dst(skb2)->ops->update_pmtu(skb_dst(skb2), NULL, skb2, rel_info);
}
- if (rel_type == ICMP_REDIRECT)
- skb_dst(skb2)->ops->redirect(skb_dst(skb2), NULL, skb2);
icmp_send(skb2, rel_type, rel_code, htonl(rel_info));
--
1.7.7.6
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2] ip6tnl: do route updating for redirect in ip6_tnl_err()
2013-09-18 3:27 [PATCH v2] ip6tnl: do route updating for redirect in ip6_tnl_err() Duan Jiong
@ 2013-09-19 18:02 ` David Miller
2013-09-20 10:43 ` Duan Jiong
0 siblings, 1 reply; 3+ messages in thread
From: David Miller @ 2013-09-19 18:02 UTC (permalink / raw)
To: djduanjiong; +Cc: netdev, duanj.fnst
From: Duan Jiong <djduanjiong@gmail.com>
Date: Tue, 17 Sep 2013 20:27:57 -0700
> After the ip6_tnl_err() is called, the rel_msg is assigned to
> 0, and the ip4ip6_err() will return directly, the instruction
> below will not be executed.
>
> In rfc2473, we can know that the tunnel ICMP redirect message
> should not be reported to the source of the original packet,
> so we can handle it in ip6_tnl_err().
>
> Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com>
I do not think this change is correct.
> @@ -576,9 +579,6 @@ ip4ip6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
> rel_type = ICMP_DEST_UNREACH;
> rel_code = ICMP_FRAG_NEEDED;
> break;
> - case NDISC_REDIRECT:
> - rel_type = ICMP_REDIRECT;
> - rel_code = ICMP_REDIR_HOST;
> default:
> return 0;
> }
The bug is a missing "break;" statement for this case, and:
> @@ -637,8 +637,6 @@ ip4ip6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
>
> skb_dst(skb2)->ops->update_pmtu(skb_dst(skb2), NULL, skb2, rel_info);
> }
> - if (rel_type == ICMP_REDIRECT)
> - skb_dst(skb2)->ops->redirect(skb_dst(skb2), NULL, skb2);
>
We absolutely must run the ipv4 dst_ops redirect handler here so you must
keep this code around as well.
The only change you need to make is to add the missing break;
statement to ip4ip6_err() and then also add appropriate NDISC_REDIRECT
handling to ip6ip6_err().
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] ip6tnl: do route updating for redirect in ip6_tnl_err()
2013-09-19 18:02 ` David Miller
@ 2013-09-20 10:43 ` Duan Jiong
0 siblings, 0 replies; 3+ messages in thread
From: Duan Jiong @ 2013-09-20 10:43 UTC (permalink / raw)
To: David Miller; +Cc: djduanjiong, netdev
于 2013年09月20日 02:02, David Miller 写道:
> From: Duan Jiong <djduanjiong@gmail.com>
> Date: Tue, 17 Sep 2013 20:27:57 -0700
>
>> After the ip6_tnl_err() is called, the rel_msg is assigned to
>> 0, and the ip4ip6_err() will return directly, the instruction
>> below will not be executed.
>>
>> In rfc2473, we can know that the tunnel ICMP redirect message
>> should not be reported to the source of the original packet,
>> so we can handle it in ip6_tnl_err().
>>
>> Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com>
>
> I do not think this change is correct.
>
>> @@ -576,9 +579,6 @@ ip4ip6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
>> rel_type = ICMP_DEST_UNREACH;
>> rel_code = ICMP_FRAG_NEEDED;
>> break;
>> - case NDISC_REDIRECT:
>> - rel_type = ICMP_REDIRECT;
>> - rel_code = ICMP_REDIR_HOST;
>> default:
>> return 0;
>> }
>
> The bug is a missing "break;" statement for this case, and:
You can read the ip6_tnl_err(). when the message is NDISC_REDIRECT, the
rel_msg is assigned to 0, then
561 if (rel_msg == 0)
562 return 0;
the function ip4ip6_err() return directly. so even though you add "break;"
statement for this case, the redirect message will not be handled.
>
>> @@ -637,8 +637,6 @@ ip4ip6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
>>
>> skb_dst(skb2)->ops->update_pmtu(skb_dst(skb2), NULL, skb2, rel_info);
>> }
>> - if (rel_type == ICMP_REDIRECT)
>> - skb_dst(skb2)->ops->redirect(skb_dst(skb2), NULL, skb2);
>>
>
> We absolutely must run the ipv4 dst_ops redirect handler here so you must
> keep this code around as well.
>
Because the Redirect Message is sent to tunnel entry-point, so the target address in
Redirect ICMPv6 Message is an ipv6 address, then we could not run the ipv4 dst_ops
redirect handler here.
Thanks,
Duan
> The only change you need to make is to add the missing break;
> statement to ip4ip6_err() and then also add appropriate NDISC_REDIRECT
> handling to ip6ip6_err().
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-09-20 10:44 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-18 3:27 [PATCH v2] ip6tnl: do route updating for redirect in ip6_tnl_err() Duan Jiong
2013-09-19 18:02 ` David Miller
2013-09-20 10:43 ` Duan Jiong
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).