pimd: change default value of rp keep_alive_timer to 185 seconds

[pguibert6WIND]

In order to be compliant with the code.

Signed-off-by: Philippe Guibert [email protected]

[donaldsharp]

There is no code compliance issue. The rp pim keep alive timer is 210 seconds. There is nothing in the code that lists it as 185. This is a straight up NACK from me.

[NetDEF-CI]

Continuous Integration Result: FAILED

Continuous Integration Result: FAILED

See below for issues. CI System Testrun URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4203/

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.

Get source / Pull Request: Successful

Building Stage: Failed

OpenBSD 7 amd64 build: Failed (click for details)

OpenBSD 7 amd64 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4203/artifact/CI011BUILD/frr.xref.xz/frr.xref.xz OpenBSD 7 amd64 build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4203/artifact/CI011BUILD/config.log/config.log.gz OpenBSD 7 amd64 build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-PULLREQ2-4203/artifact/CI011BUILD/config.status/config.status OpenBSD 7 amd64 build: No useful log found

Successful on other platforms/tests

• Debian 10 amd64 build
• FreeBSD 11 amd64 build
• Ubuntu 16.04 arm7 build
• Debian 11 amd64 build
• Ubuntu 16.04 i386 build
• Ubuntu 18.04 amd64 build
• Ubuntu 16.04 arm8 build
• Ubuntu 18.04 i386 build
• Ubuntu 16.04 amd64 build
• CentOS 7 amd64 build
• Redhat 8 amd64 build
• NetBSD 9 amd64 build
• Debian 9 amd64 build
• Ubuntu 20.04 amd64 build
• Ubuntu 18.04 ppc64le build
• FreeBSD 12 amd64 build
• Ubuntu 18.04 arm7 build
• Fedora 29 amd64 build
• Ubuntu 18.04 arm8 build

[NetDEF-CI]

Continuous Integration Result: FAILED

See below for issues. CI System Testrun URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4204/

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.

Get source / Pull Request: Successful

Building Stage: Successful

Basic Tests: Failed

Topotests Ubuntu 18.04 i386 part 9: Failed (click for details)

Topotests Ubuntu 18.04 i386 part 9: Unknown Log URL: https://ci1.netdef.org/browse/FRR-PULLREQ2-4204/artifact/TOPO9U18I386/ErrorLog/ Topotests Ubuntu 18.04 i386 part 9: No useful log found

Successful on other platforms/tests

• Topotests debian 10 amd64 part 9
• Addresssanitizer topotests part 4
• Topotests Ubuntu 18.04 i386 part 7
• Addresssanitizer topotests part 5
• Topotests Ubuntu 18.04 arm8 part 3
• Topotests debian 10 amd64 part 4
• Topotests Ubuntu 18.04 amd64 part 3
• Topotests debian 10 amd64 part 3
• Topotests debian 10 amd64 part 8
• Addresssanitizer topotests part 0
• Topotests Ubuntu 18.04 amd64 part 2
• Topotests Ubuntu 18.04 arm8 part 4
• Ubuntu 20.04 deb pkg check
• Topotests Ubuntu 18.04 arm8 part 9
• Addresssanitizer topotests part 2
• Topotests Ubuntu 18.04 i386 part 6
• Debian 10 deb pkg check
• IPv6 protocols on Ubuntu 18.04
• IPv4 ldp protocol on Ubuntu 18.04
• Ubuntu 16.04 deb pkg check
• Topotests Ubuntu 18.04 amd64 part 5
• Topotests Ubuntu 18.04 amd64 part 4
• IPv4 protocols on Ubuntu 18.04
• Topotests Ubuntu 18.04 i386 part 1
• Addresssanitizer topotests part 9
• Topotests Ubuntu 18.04 i386 part 3
• Topotests Ubuntu 18.04 i386 part 8
• Topotests Ubuntu 18.04 arm8 part 2
• Addresssanitizer topotests part 3
• Topotests Ubuntu 18.04 amd64 part 7
• Topotests Ubuntu 18.04 arm8 part 7
• Topotests debian 10 amd64 part 0
• Topotests Ubuntu 18.04 amd64 part 9
• Addresssanitizer topotests part 7
• Topotests Ubuntu 18.04 i386 part 0
• Addresssanitizer topotests part 6
• Topotests Ubuntu 18.04 arm8 part 5
• CentOS 7 rpm pkg check
• Fedora 29 rpm pkg check
• Topotests debian 10 amd64 part 1
• Topotests debian 10 amd64 part 2
• Topotests debian 10 amd64 part 7
• Topotests Ubuntu 18.04 arm8 part 0
• Topotests Ubuntu 18.04 amd64 part 8
• Topotests Ubuntu 18.04 amd64 part 0
• Static analyzer (clang)
• Topotests Ubuntu 18.04 i386 part 5
• Ubuntu 18.04 deb pkg check
• Debian 9 deb pkg check
• Addresssanitizer topotests part 1
• Topotests Ubuntu 18.04 i386 part 4
• Topotests Ubuntu 18.04 amd64 part 1
• Topotests debian 10 amd64 part 6
• Topotests Ubuntu 18.04 i386 part 2
• Topotests Ubuntu 18.04 arm8 part 1
• Topotests Ubuntu 18.04 arm8 part 6
• Addresssanitizer topotests part 8
• Topotests Ubuntu 18.04 amd64 part 6
• Topotests debian 10 amd64 part 5
• Topotests Ubuntu 18.04 arm8 part 8

[pguibert6WIND]

There is no code compliance issue. The rp pim keep alive timer is 210 seconds. There is nothing in the code that lists it as 185. This is a straight up NACK from me.

@patrasar is right. there is an issue. And because 210 seconds is the value to keep, there will be a separate pull request to perform to modify PIM..

[mobash-rasool]

@pguibert6WIND : The problem lies in the function pim_instance_init:

pim->keep_alive_time = PIM_KEEPALIVE_PERIOD; pim->rp_keep_alive_time = PIM_RP_KEEPALIVE_PERIOD;

#define PIM_RP_KEEPALIVE_PERIOD
(3 * router->register_suppress_time + router->register_probe_time)

PIM_RP_KEEPALIVE_PERIOD calculation is right.

During the initialisation of rp_keep_alive_time, we need to take as per below calculation max(Keepalive_Period, RP_Keepalive_Period)

As per RFC 7761, section 4.11 page 127

The normal keepalive period for the KAT(S,G) defaults to 210 seconds.
   However, at the RP, the keepalive period must be at least the
   Register_Suppression_Time, or the RP may time out the (S,G) state
   before the next Null-Register arrives.  Thus, the KAT(S,G) is set to
   max(Keepalive_Period, RP_Keepalive_Period) when a Register-Stop
   is sent.

Since the current code is not considering the max of the 2 values, therefore rp keep alive timer is getting set to 185.

[github-actions[bot]]

This PR is stale because it has been open 180 days with no activity. Comment or remove the autoclose label in order to avoid having this PR closed.