Version 1.6.0

[Toke Høiland-Jørgensen]

Ondrej Zajicek <santiago at crfreenet.org> writes:

> Some comments to my changes. I originally thought that i would just do
> some minor formatting changes, but i end with some heavy code changes and
> plenty of bugfixes. Hopefully i did not make much more errors.

Thanks for the overview. Some comments below.

> Some issues i found and fixed (and noted down):
>
>  - modified ge_mod64k() still does not work:
>
>    static inline u16 ge_mod64k(u16 a, u16 b)
>    { return (a == b || b - a > 0x8000); }
>
>    Due to C integer promotion rules, b - a is computed in (signed) integers
>    and not in u16s, so if b = 1 and a = 2, you get -1, so > is false.
>    The solution is to use uint as the type for a, b, so it is computed in
>    unsigned integers and result can be safely typecasted to u16:
>
>    static inline int ge_mod64k(uint a, uint b)
>    { return (u16)(a - b) < 0x8000; }

Was wondering about this. Thanks for the explanation.

>  - in some cases (e.g. babel_handle_route_request()), check for plen == 0
>    or check if prefix is IPA_NONE is used where check for AE 0 should be
>    used instead. Note that these are two different cases - there is the
>    default route ::/0 (with AE 2), which is a regular route, and there is
>    'wildcard' AE 0, which represent all routes.

This was actually deliberate on my part: I have considered the different
AEs as solely a matter of encoding on the wire, and not carrying any
significance as to the semantics of the prefix. I.e. and update with AE
0 and no prefix and one with AE 2 encoding ::/0 are identical in my
reading.

If this is not the case, I think the RFC needs to specify what, exactly,
is meant by a "wildcard address". I've always thought of ::/0 as the
wildcard address; and doesn't "default route" also mean "wildcard
route"?

Either way, this probably needs to be specified somewhere.

>  - babel_read_update() - does BABEL_AE_IP6_LL make sense?

Not sure it makes much sense to announce a route for a link-local
address, but I suppose it could be used to set a router ID? (Not sure
why anyone would do that, but it would not technically be out of spec).

> Feel free to comment any of my changes.

No comments on the rest of the fixes, other than 'thanks for the
bugfixes'. Oh, thanks in general for your thorough review of the
different iterations of my patch series; it has definitely improved
things, and I have learned a lot about the idiosyncrasies of C :) 

> I found also some problems that were hard to fix and not severe,
> mainly related to route recalculation and triggered updates. These
> were left in the code. I will comment them later.

Okidoki, please do tell. I do believe there is some more work that can
be done to improve the route selection logic in the future; some sort of
hysteresis would probably be desirable, for instance.


I'll send a patch series fixing all the small issues we have accumulated
once I'm fairly certain that they are actually resolved. :)

-Toke
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 472 bytes
Desc: not available
URL: <http://trubka.network.cz/pipermail/bird-users/attachments/20160501/2d3bec50/attachment.asc>