Last Hop Check Issue

[wgc1212]

pimd/src/vif.h

Line 82 in 3aefd81

inline static int PIMD_VIFM_LASTHOP_ROUTER(uint8_t *leaves, uint8_t *oifs)

Comparing PIMD_VIFM_LASTHOP_ROUTER() to the released comparison for last hop check:

#define VIFM_LASTHOP_ROUTER(leaves, oifs) ((leaves) & (oifs))

The released software comparison will accept any interface that has both a leave and an oif, while the new comparison requires all interfaces to have both set in order to qualify as a Lasthop router. Is this assessment correct and was this change intentional?

Thanks.

[troglobit]

Good catch! No, that change was not intentional. The commit that introduced this was only supposed to change from a uint32_t based bitmask to use a variable length array.

Do you think you could help out with a fix and submit as a pull request?

[wgc1212]

Sure! I'm working the code now, I just need a good way to test it. Bill

…

On Thu, Nov 15, 2018 at 2:49 AM Joachim Nilsson ***@***.***> wrote: Good catch! No, that change was not intentional. The commit that introduced this was only supposed to change from a uint32_t based bitmask to use a variable length array. Do you think you could help out with a fix and submit as a pull request? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#131 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/Aq4LbnJeRWqUg70Vv6g5fkADmfxDe12Cks5uvRySgaJpZM4YfNx-> .

[wgc1212]

I wasn't sure how updates to the project were supposed to work. Should I enter a pull request for the code first? The routine I was thinking of would be as follows. This would check for any non-zero leaves and oifs in the arrays:

inline static int PIMD_VIFM_LASTHOP_ROUTER(uint8_t *leaves, uint8_t *oifs)
{
    int i=0;
    for (; ((i<MAXVIFS) && (!leaves[i] || !oifs[i])); ++i);
    return((i<MAXVIFS) ? 1 : 0);
}

[troglobit]

A pull request is always preferable, they work well also for prototyping code together or getting feedback.

The code looks OK, but I'd like to propose the following instead, which I hope is more readable and yet still correct:

inline static int PIMD_VIFM_LASTHOP_ROUTER(uint8_t *leaves, uint8_t *oifs)
{
    int i;

    for (i = 0; i < MAXVIFS; i++) {
        if (leaves[i] && oifs[i])
            return 1;
    }

    return 0;
}

What do you think?

[wgc1212]

That looks fine. I tried to do a pull request, and I didn't seem to be allowed to.

[troglobit]

Anybody can do a pull request against pimd. There are no limitations in place. Have you read up on how to do it? https://help.github.com/articles/creating-a-pull-request-from-a-fork/

Briefly, the process is as follows:

• Fork the repo
• Create a new branch (with a useful name) in your fork
• Commit the change to that branch
• Push change branch to your fork at GitHub
• Check the GitHub page for your fork, a pull-request button should now appear next to your pushed branch

[wgc1212]

Thanks. I have gone through the GitHub process. I had gotten a couple of steps out of order. I have updated the code and have submitted the Pull request.

[troglobit]

Good work! Small final comment from me.