Skip to content

Implement SSH keepalives, default 30-second interval, for NETCONF over SSH sessions.#668

Open
stacywsmith wants to merge 6 commits into
Juniper:masterfrom
stacywsmith:ssh-keepalives
Open

Implement SSH keepalives, default 30-second interval, for NETCONF over SSH sessions.#668
stacywsmith wants to merge 6 commits into
Juniper:masterfrom
stacywsmith:ssh-keepalives

Conversation

@stacywsmith

Copy link
Copy Markdown
Contributor

Without SSH keepalives, a NAT or stateful firewall along the network
path between the PyEZ host and the target Junos device,
may timeout an inactive TCP flow and cause the NETCONF over SSH
session to hang. Sending SSH keepalives avoids this situation. The
default value is 30 seconds. Setting this parameter to a value of 0
disables SSH keepalives.

Note: This is a different situation than Issue #663 in which the
target Junos device is timing out the NETCONF over SSH session
due to a configured idle-timeout on the system login class.

…ONF over SSH sessions.

Without SSH keepalives, a NAT or stateful firewall along the network
path between the PyEZ host and the target Junos device,
may timeout an inactive TCP flow and cause the NETCONF over SSH
session to hang. Sending SSH keepalives avoids this situation. The
default value is 30 seconds. Setting this parameter to a value of 0
disables SSH keepalives.

Note: This is a different situation than Issue Juniper#663 in which the
target Junos device is timing out the NETCONF over SSH session
due to a configured idle-timeout on the system login class.
@stacywsmith stacywsmith requested a review from vnitinv February 14, 2017 03:02
@spidercensus

Copy link
Copy Markdown
Contributor

There should be a check to make sure that this value in the ssh config file is not overridden.

Comment thread lib/jnpr/junos/device.py Outdated
ssh_config=self._sshconf_lkup(),
device_params={'name': 'junos', 'local': False})

self._conn._session.transport.set_keepalive(

@vnitinv vnitinv Feb 15, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line 910 we are setting it to None for on-box if condition. So on on-box are not reaching line 924, hence converting None to int at line 1024 will throw exception.

@vnitinv vnitinv left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got 1 comment to be addressed.

@vnitinv

vnitinv commented Mar 24, 2017

Copy link
Copy Markdown
Contributor

Can one of the admins verify this patch?

@spidercensus spidercensus left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the docs for the Transport module:
| set_keepalive(self, interval)
| Turn on/off keepalive packets (default is off). If this is set, after
| interval seconds without sending any data over the connection, a
| "keepalive" packet will be sent (and ignored by the remote host). This
| can be useful to keep connections alive over a NAT, for example.
|
| :param int interval:
| seconds to wait before sending a keepalive packet (or
| 0 to disable keepalives).

So this means that at line 910 we should be setting keepalives to 0 by default, not None.

Comment thread lib/jnpr/junos/device.py
self._hostname = 'localhost'
self._ssh_private_key_file = None
self._ssh_config = None
self._ssh_keepalives = None

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Setting this to None does not make sense. This value is initialized to zero in the underlying module.

Help on method set_keepalive in module paramiko.transport:

set_keepalive(self, interval) method of paramiko.transport.Transport instance
Turn on/off keepalive packets (default is off). If this is set, after
interval seconds without sending any data over the connection, a
"keepalive" packet will be sent (and ignored by the remote host). This
can be useful to keep connections alive over a NAT, for example.

:param int interval:
    seconds to wait before sending a keepalive packet (or
    0 to disable keepalives).  

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

None is set for on-box python, where it won't use ssh connection. So this should be OK

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants