Thomas Huth, on Fri 11 Dec 2015 14:38:44 +0100, wrote:
> > +switch (iph->ip_v) {
> > +case IPVERSION:
> > if (iph->ip_dst.s_addr == 0) {
> > /* 0.0.0.0 can not be a destination address, something went wrong,
> > * avoid making it worse */
>
> That indentation looks
Thomas Huth, on Fri 11 Dec 2015 14:43:42 +0100, wrote:
> Anyway, it's IMHO a somewhat strange way to structure a patch ... maybe
> it would be nicer to do it in one go instead (after splitting off the
> arp_requested renaming)?
Please discuss about it with the people who asked for it. I won't
On 11/12/15 15:01, Samuel Thibault wrote:
> Just to explain.
>
> I'm fine with revamping the patches, provided that we eventually
> converge somewhere.
>
> What I'm afraid of is that splitting patches in yet more many pieces,
> revamping the code yet more (moving code into their own functions,
On 11/12/15 14:38, Thomas Huth wrote:
> On 11/12/15 01:15, Samuel Thibault wrote:
>> From: Guillaume Subiron
>>
>> Basically, this patch replaces "arp" by "resolution" every time "arp"
>> means "mac resolution" and not specifically ARP.
>>
>> Some indentation problems are
Just to explain.
I'm fine with revamping the patches, provided that we eventually
converge somewhere.
What I'm afraid of is that splitting patches in yet more many pieces,
revamping the code yet more (moving code into their own functions, etc.)
will only make the patch series look even bigger
Thomas Huth, on Fri 11 Dec 2015 15:32:48 +0100, wrote:
> So maybe it's better to do smaller steps instead: Would it for example
> make sense to split the whole series into two parts - first a series
> that does all the preparation and cleanup patches. And then once that
> has been reviewed and
On 11/12/15 01:15, Samuel Thibault wrote:
> From: Guillaume Subiron
>
> Basically, this patch replaces "arp" by "resolution" every time "arp"
> means "mac resolution" and not specifically ARP.
>
> Some indentation problems are solved in functions that will be modified
> in
On 11/12/15 14:43, Thomas Huth wrote:
> On 11/12/15 14:38, Thomas Huth wrote:
>> On 11/12/15 01:15, Samuel Thibault wrote:
>>> From: Guillaume Subiron
>>>
>>> Basically, this patch replaces "arp" by "resolution" every time "arp"
>>> means "mac resolution" and not specifically
On 11/12/15 15:55, Samuel Thibault wrote:
> Thomas Huth, on Fri 11 Dec 2015 15:32:48 +0100, wrote:
>> So maybe it's better to do smaller steps instead: Would it for example
>> make sense to split the whole series into two parts - first a series
>> that does all the preparation and cleanup patches.
Laszlo Ersek, on Fri 11 Dec 2015 16:40:32 +0100, wrote:
> > Sounds good, ... then I'd suggest to sent the preparation patches
> > separately next time and get them accepted first.
>
> And then the next reviewer will say, "nice, but it would be even nicer
> to see what *motivates* these
meta
On 12/11/15 16:09, Thomas Huth wrote:
> On 11/12/15 15:55, Samuel Thibault wrote:
>> Thomas Huth, on Fri 11 Dec 2015 15:32:48 +0100, wrote:
>>> So maybe it's better to do smaller steps instead: Would it for example
>>> make sense to split the whole series into two parts - first a series
>>>
On 12/11/2015 08:40 AM, Laszlo Ersek wrote:
> meta
>
> On 12/11/15 16:09, Thomas Huth wrote:
>> On 11/12/15 15:55, Samuel Thibault wrote:
>>> Thomas Huth, on Fri 11 Dec 2015 15:32:48 +0100, wrote:
So maybe it's better to do smaller steps instead: Would it for example
make sense to split
On 12/11/15 17:17, Eric Blake wrote:
> On 12/11/2015 08:40 AM, Laszlo Ersek wrote:
>> meta
>>
>> On 12/11/15 16:09, Thomas Huth wrote:
>>> On 11/12/15 15:55, Samuel Thibault wrote:
Thomas Huth, on Fri 11 Dec 2015 15:32:48 +0100, wrote:
> So maybe it's better to do smaller steps instead:
On 11/12/15 14:47, Samuel Thibault wrote:
> Thomas Huth, on Fri 11 Dec 2015 14:43:42 +0100, wrote:
>> Anyway, it's IMHO a somewhat strange way to structure a patch ... maybe
>> it would be nicer to do it in one go instead (after splitting off the
>> arp_requested renaming)?
>
> Please discuss
Thomas Huth, on Fri 11 Dec 2015 14:38:44 +0100, wrote:
> Also, I'd prefer if you could split this patch into two: One for
> renaming the "arp_requested" field, and one for adding the additional
> logic with the switch statement (since there are two different kind of
> changes).
Ok, at least it's
From: Guillaume Subiron
Basically, this patch replaces "arp" by "resolution" every time "arp"
means "mac resolution" and not specifically ARP.
Some indentation problems are solved in functions that will be modified
in the next patches (ip_input…).
In if_encap, a switch is
Basically, this patch replaces arp by resolution every time arp
means mac resolution and not specifically ARP.
Some indentation problems are solved in functions that will be modified
in the next patches (ip_input…).
In if_encap, a switch is added to prepare for the IPv6 case. Some code
is
Basically, this patch replaces arp by resolution every time arp
means mac resolution and not specifically ARP.
Some indentation problems are solved in functions that will be modified
in the next patches (ip_input…).
In if_encap, a switch is added to prepare for the IPv6 case. Some code
is
18 matches
Mail list logo