| Message ID | 523996E0.9080702@cn.fujitsu.com |
|---|---|
| State | Rejected, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Duan Jiong <duanj.fnst@cn.fujitsu.com> Date: Wed, 18 Sep 2013 20:04:48 +0800 > From: Duan Jiong <duanj.fnst@cn.fujitsu.com> > > when dealing with redirect message in udpv6_err() and > rawv6_err() the err shoud be assigned to 0, not EPROTO. > > Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com> No, you should fix this the same way you handled DCCP, but skipping the whole socket error setting path for NDISC_REDIRECT. Clearing the socket error to zero is not correct, it means you will lose any existing error setting. -- 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
On Wed, Sep 18, 2013 at 12:36:13PM -0400, David Miller wrote: > From: Duan Jiong <duanj.fnst@cn.fujitsu.com> > Date: Wed, 18 Sep 2013 20:04:48 +0800 > > > From: Duan Jiong <duanj.fnst@cn.fujitsu.com> > > > > when dealing with redirect message in udpv6_err() and > > rawv6_err() the err shoud be assigned to 0, not EPROTO. > > > > Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com> > > No, you should fix this the same way you handled DCCP, but skipping > the whole socket error setting path for NDISC_REDIRECT. > > Clearing the socket error to zero is not correct, it means you will > lose any existing error setting. My reasoning when I proposed this change was that we should not stop appending ipv6 redirects to the raw and udp sockets error queues because userland could want to deal with them (and probably already does). Also, we need to notify poll of the appended error, hence we still needed a call to sk_err_report to fire up a POLL_ERR event. Because we don't know if the router sending the redirect actually dropped the packet I thought taht '0' would be the only sane choice. We currently report back EPROTO, IPv4 does report (rightfully) EHOSTUNREACH. Perhaps we should stay with EPROTO? I agree, all other error functions should return early after updating the routing table without reporting any error. Either they should not call into this function or ignore err == 0 (I prefer the first). I know, it is a bit hacky but this was the most sane idea I came up with. (I still have not reviewed this patch, just wanted to provide a bit of background that lead to this change.) Greetings, Hannes -- 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/9/19 0:36, David Miller 写道: > From: Duan Jiong <duanj.fnst@cn.fujitsu.com> > Date: Wed, 18 Sep 2013 20:04:48 +0800 > >> From: Duan Jiong <duanj.fnst@cn.fujitsu.com> >> >> when dealing with redirect message in udpv6_err() and >> rawv6_err() the err shoud be assigned to 0, not EPROTO. >> >> Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com> > No, you should fix this the same way you handled DCCP, but skipping > the whole socket error setting path for NDISC_REDIRECT. Your meaning is that we should return directly after calling ip6_sk_redirect(), is that it? Thanks, Duan > Clearing the socket error to zero is not correct, it means you will > lose any existing error setting. -- 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
From: Duan Jiong <djduanjiong@gmail.com> Date: Thu, 19 Sep 2013 13:24:04 +0800 > 于 2013/9/19 0:36, David Miller 写道: >> From: Duan Jiong <duanj.fnst@cn.fujitsu.com> >> Date: Wed, 18 Sep 2013 20:04:48 +0800 >> >>> From: Duan Jiong <duanj.fnst@cn.fujitsu.com> >>> >>> when dealing with redirect message in udpv6_err() and >>> rawv6_err() the err shoud be assigned to 0, not EPROTO. >>> >>> Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com> >> No, you should fix this the same way you handled DCCP, but skipping >> the whole socket error setting path for NDISC_REDIRECT. > > Your meaning is that we should return directly after calling > ip6_sk_redirect(), is that it? It depends upon whether udp/raw sockets are expected to receive redirect notifications this way, are they? You seem to believe so. -- 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/icmp.c b/net/ipv6/icmp.c index eef8d94..795f348 100644 --- a/net/ipv6/icmp.c +++ b/net/ipv6/icmp.c @@ -977,6 +977,9 @@ int icmpv6_err_convert(u8 type, u8 code, int *err) case ICMPV6_TIME_EXCEED: *err = EHOSTUNREACH; break; + case NDISC_REDIRECT: + *err = 0; + break; } return fatal;