Subject: Re: [Patch net-next v7 4/6] vxlan: add ipv6 support

From: Bjørn Mork <[email-masked]>
Date: Thu, 02 May 2013 14:45:36 +0200

Cong Wang <[email-masked]> writes:
> On Tue, 2013-04-30 at 12:33 +0200, Bjørn Mork wrote:
>> 
>> Well, you have been told that your proposed new feature breaks IPv4
>> support with specific settings.  I do not think that you are speeding
>> up
>> anything here by ignoring that fact.  On the contrary. 
>
> It is obviously a corner case which is not deserved such a high priority
> (as high as a kernel panic or a compile error). bindv6only is default to
> be 0, I even wasn't aware of such sysctl until David pointed it out.

It's a regression. Regressions trump new features. No need to argue
about the relative importance here.  Fix it.

> So, please give me a reason why we should let such a corner case block
> the inclusion? I think there are still other such corner cases too (and
> maybe many), which neither you nor me are aware of.

Yes, I am pretty sure of that as well.  That's why I ask you to *test*
your code with assorted settings before resubmitting it.  I'm not going
to.

I note that you claim you are not an expert.  Neither am I.  And luckily
I believe that is not required around here. Anyone can contribute, and
all contributions are very welcome.  At least that is the impression
I've got.

But only the experts can get away with limited testing. The less of an
expert you are, the more time you need to spend trying out every small
codepath you added using every possible input combination.  Doing
different combinations of buildtime and runtime IPv6 enable/disable is
an obvious requirement when you add code which has some sort of "if IPv6
is enabled" conditionals.  IMHO.

Bjørn