[Olsr-dev] [PATCH v1 1/1] lq_packet: only report a neighbour once in a hello message

[Ferry Huberts]

On 19/01/17 16:36, Teco Boot wrote:
> I'm still testing. I want to see a 100% fix on both TX (eliminate duplicates that are not correct or relevant) and on RX (don't use LQ where link-type=UNSPEC. The RFC is my guide.

Eliminating (these kinds of) duplicates is NOT ALLOWED per the RFC.
The RFC very clearly states that the ENTIRE neighbourhood must be sent, 
and neighbours not reachable over the sending interface must be UNSPEC.
In the example setup that results in one SYM neighbour and one UNSPEC 
neighbour, which is entirely correct.

There is not enough information in the messages to fully fix this, 
accept it. This can only be fixed if the hellos contain the interface 
indexes with every record; then these duplicates will cease to be 
duplicates. That would require an RFC change, and we will not go there.


>
> I'm facing lots of SPFs where topology is stable. The glitches bite.
>
> I tested a bit with fragments. I want to make sure that the RX patch doesn't break something.


There is no RX patch (yet), so what are your talking about?


>
> I don't see a reason to hurry.

There is VERY good reason to hurry: this has been broken since 
3d2fd73a5528a9c7cdccd088f2dcca80b37e66b9, which is every release since - 
and including - v0.6.7.

The oneliner patch solves it, or at least mitigates it.

We need to make a patch on the RX side as well, as we already agreed and 
as Henning indicated earlier today.


I am going to push my TX fix tomorrow and I will work on the RX patch 
after next week.


I will also put the fix on our release branch, and coordinate with 
Henning to do a release of that branch.


F

>
> Teco
>
>
>> Op 19 jan. 2017, om 15:00 heeft Henning Rogge <(spam-protected)> het volgende geschreven:
>>
>> On Thu, Jan 19, 2017 at 2:57 PM, Ferry Huberts <(spam-protected)> wrote:
>>
>>
>> On 19/01/17 13:33, Henning Rogge wrote:
>> Hi,
>>
>> back from (two) business trips...
>>
>> On Mon, Jan 16, 2017 at 6:14 PM, Ferry Huberts <(spam-protected)
>> <mailto:(spam-protected)>> wrote:
>>
>>     I think I may have stumbled onto a MUCH easier fix to this problem.
>>
>>
>>     The actual problem is quite involved (with fragmented hello messages
>>     etc) but it all boils down to a later UNSPEC link overwriting a
>>     previous SYM/ASYM/other link on the receiving end.
>>
>>     Since the neighbours in a hello message are already ordered when the
>>     hello message is sent out, the following patch also solves it.
>>
>>     Opinions?
>>
>>
>> I like the idea of solve the problem by sorting, but I worry that doing
>> so on the TX side is a bit "brittle"... its easy to break because there
>> is nothing on the TX side that needs this order. And there will always
>> be older Olsrd installations on many community networks.
>>
>> How difficult would it be to do the sorting on the RX side
>> in deserialize_hello() (process_package.c: 318..)?
>>
>> Instead of building them into a single linked list, have an array with
>> MAX_LINK lists, one for each link type... and then concatenate this
>> lists before parsing in the right order.
>>
>> This way we also fix the problem appearing again because of an outdated
>> Olsrd sending bad data, right?
>>
>>
>>
>> If we do that as a follow-up patch? Would that be ok?
>>
>> Sounds good... quick fix now to solve the problem for the current code, cleanup later to make it future proof.
>>
>> Henning
>

-- 
Ferry Huberts