[PATCH ] ipv6: handle the update of the NDISC_REDIRECT error code in icmpv6_err_convert

[PATCH ] ipv6: handle the update of the NDISC_REDIRECT error code in icmpv6_err_convert

[Duan Jiong]

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>
---
 net/ipv6/icmp.c | 3 +++
 1 file changed, 3 insertions(+)

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;
-- 
1.8.3.1

Re: [PATCH ] ipv6: handle the update of the NDISC_REDIRECT error code in icmpv6_err_convert

[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.

Clearing the socket error to zero is not correct, it means you will
lose any existing error setting.

Re: [PATCH ] ipv6: handle the update of the NDISC_REDIRECT error code in icmpv6_err_convert

[Hannes Frederic Sowa]

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

Re: [PATCH ] ipv6: handle the update of the NDISC_REDIRECT error code in icmpv6_err_convert

[Duan Jiong]

于 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.

Re: [PATCH ] ipv6: handle the update of the NDISC_REDIRECT error code in icmpv6_err_convert

[David Miller]

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.