zebra: be more careful removing 'installed' flag from nhgs

[mjstapp]

When interface addresses change, we examine nhgs associated with the interface in case they need to be reinstalled. As part of that, we may need to reinstall ecmp nhgs that use the interface being examined - but not always. Currently, changes to addresses on an interface may incorrectly mark nexthop-groups as 'uninstalled' incorrectly, and zebra won't correct that by itself. For example, run zebra and sharpd, and configure a nexthop-group with two nexthops:

mjs-ubu22(config)# nexthop-group nh1
mjs-ubu22(config-nh-group)# nexthop 2.2.2.22 ANNIE onlink 
mjs-ubu22(config-nh-group)# nexthop 2.2.2.23 ANNIE onlink 
mjs-ubu22(config-nh-group)# end

And the output of the nhg will be as expected:

ID: 68 (sharp)
     RefCnt: 2
     Uptime: 00:00:05
     VRF: default
     Valid, Installed
     Interface Index: 3
           via 2.2.2.23, ANNIE (vrf default) onlink, weight 1
     Dependents: (185483868)
ID: 69 (sharp)
     RefCnt: 2
     Uptime: 00:00:05
     VRF: default
     Valid, Installed
     Interface Index: 3
           via 2.2.2.22, ANNIE (vrf default) onlink, weight 1
     Dependents: (185483868)
[...]
ID: 185483868 (sharp)
     RefCnt: 1
     Uptime: 00:00:05
     VRF: default
     Valid, Installed
     Depends: (68) (69)
           via 2.2.2.22, ANNIE (vrf default) onlink, weight 1
           via 2.2.2.23, ANNIE (vrf default) onlink, weight 1

Then add an ipv6 address to the interface - this will trigger change processing that marks sharpd's NHG "uninstalled":

mjs-ubu22(config)# int ANNIE 
mjs-ubu22(config-if)# ip address 2222::2222/64
mjs-ubu22(config-if)# do sho nexthop-group rib 
[...]
ID: 68 (sharp)
     RefCnt: 2
     Uptime: 00:00:44
     VRF: default
     Valid, Installed
     Interface Index: 3
           via 2.2.2.23, ANNIE (vrf default) onlink, weight 1
     Dependents: (185483868)
ID: 69 (sharp)
     RefCnt: 2
     Uptime: 00:00:44
     VRF: default
     Valid, Installed
     Interface Index: 3
           via 2.2.2.22, ANNIE (vrf default) onlink, weight 1
     Dependents: (185483868)
[...]
ID: 185483868 (sharp)
     RefCnt: 1
     Uptime: 00:00:44
     VRF: default
     Valid
     Depends: (68) (69)
           via 2.2.2.22, ANNIE (vrf default) onlink, weight 1
           via 2.2.2.23, ANNIE (vrf default) onlink, weight 1

[NetDEF-CI]

Continuous Integration Result: SUCCESSFUL

Congratulations, this patch passed basic tests

Tested-by: NetDEF / OpenSourceRouting.org CI System

CI System Testrun URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-14412/

This is a comment from an automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.

[donaldsharp]

Can I get an example of when this is wrong? And how this fixes this?

[mjstapp]

Heh, yeah, sorry: hate it when people open mystery PRs with no context!
I've updated the description with a config example and show output.

Can I get an example of when this is wrong? And how this fixes this?

[riw777]

looks good

[riw777]

Is this possibly also related to #14102?

[donaldsharp]

I hate to be this way but can we add this analysis to the commit message itself?

[mjstapp]

heh - sure, of course - but I do think (after our discussion during the weekly meeting on Tuesday this week) that we should wait a bit to hear from Chirag who has some insight into the event-handling that led to the current version of the code. it may be that we need a more complete solution to the problem of lost/elided OS events...

I hate to be this way but can we add this analysis to the commit message itself?