On 02/25/2019 08:08 PM, Hangbin Liu wrote:
> Hi David,
> On Mon, Feb 25, 2019 at 07:15:26PM -0700, David Ahern wrote:
>> On 2/25/19 6:55 PM, Hangbin Liu wrote:
>>> Just as I said, this issue only occurs when IPv6 is disabled at boot time
>>> as there is no IPv6 route entry. Disable ipv6 on specific interface should
>>> not be affected(IPv6 route/fib has inited). So I think use
>>> ipv6_mod_enabled()
>>> would be more suitable in this scenario. Did I miss something?
>>
>> From a readability perspective the code path depends on whether ipv6 is
>> enabled on the device. I think it is better to leave that as it is.
>
> When I posted 173656accaf5 ("sit: check if IPv6 enabled before calling
> ip6_err_gen_icmpv6_unreach()"), my purpose is to check if IPv6 disabled
> at boot time. I didn't know we have ipv6_mod_enabled() at that time, so I
> just used __in6_dev_get() as a trick way/work around.
>
> A few days later I saw your commit e434863718d4 ("net: vrf: Fix crash when
> IPv6 is disabled at boot time") and I thought this would be a more clear way
> to tell people that we are checking if IPv6 disabled at boot time. So I posted
> these two follow up fixes.
>
> I don't know why you thought check IPv6 is enbled on the device is better.
> Because it's more strict? Maybe I missed something here. But if you feel it
> is better to leave as it it, then let's keep it.
>
I do not get it.
We really do not care if IPv6 is enabled on some device on the host.
Here the correct test is checking if IPv6 is enabled for _this_ device.
It is not about fixing a crash (it is already fixed), but not having to call
ip6_err_gen_icmpv6_unreach() knowing it will fail anyway.
So the current code is better.
Thank you.
