| Message ID | 1379474877-3097-1-git-send-email-duanj.fnst@cn.fujitsu.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
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(). -- 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
于 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(). -- 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
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));
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(-)