Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TACACS] Improve nss-tacplus TACACS connect timeout #17460

Merged
merged 8 commits into from
Mar 6, 2024

Conversation

liuh-80
Copy link
Contributor

@liuh-80 liuh-80 commented Dec 8, 2023

Improve nss-tacplus TACACS connect timeout

Why I did it

TACACS login usually config multiple server, when a high priority server not reachable or high lantency, remote user login will slowly, also run SUDO command will slowly.
To improve this issue, enable read timeout in nss-tacplus.

Work item tracking
  • Microsoft ADO: 25697281
  • Microsoft ADO: 28738251

How I did it

Enable read timeout in nss-tacplus.

How to verify it

Pass all UT.

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111
  • 202205
  • 202211
  • 202305

Tested branch (Please provide the tested image version)

  • SONiC.master.448381-50571ad34
  • SONiC.202311-19944.622805-494cfbc48
  • SONiC.202305-19945.622806-46bffa75e

Description for the changelog

Improve nss-tacplus TACACS connect timeout.

Link to config_db schema for YANG module changes

A picture of a cute animal (not mandatory but encouraged)

@liuh-80
Copy link
Contributor Author

liuh-80 commented Dec 8, 2023

With this change following UT will be failed, because we make parallel connection and generate multiple reject result:

tacacs/test_authorization.py::test_stop_request_next_server_after_reject[vlab-01]

    # Server side should only have 1 login request log:
    #       After first tacacs server reject user login, tacacs will not try to connect to second server.
    res = ptfhost.command(r"sed -n 's/\(exec authorization request for invalid_user\)/\1/p'  /var/log/tac_plus.log")
    logger.warning(res["stdout_lines"])
  pytest_assert(len(res["stdout_lines"]) == 1)

@liuh-80
Copy link
Contributor Author

liuh-80 commented Jan 8, 2024

Please use this PR for review diff file change:
https://github.com/liuh-80/libnss-tacplus/pull/3/files

@liuh-80 liuh-80 changed the title [POC] [TACACS] Connect all TACACS server in parallel [TACACS] Connect all TACACS server in parallel Jan 8, 2024
@liuh-80 liuh-80 marked this pull request as ready for review January 8, 2024 05:25
@liuh-80 liuh-80 requested a review from qiluo-msft January 8, 2024 05:25
@liuh-80 liuh-80 changed the title [TACACS] Connect all TACACS server in parallel [TACACS] Improve nss-tacplus TACACS connect timeout Feb 20, 2024
+ if(tac_srv[tac_srv_no].timeout < 0) {
+ tac_srv[tac_srv_no].timeout = 0;
+ }
+ else {
Copy link
Collaborator

Choose a reason for hiding this comment

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

else

if tac_srv[tac_srv_no].timeout == 0, do you want to tac_readtimeout_enable or not?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed, when config file has a negative config value, timeout will set to 0 and tac_readtimeout_enable will disabled.

@qiluo-msft qiluo-msft merged commit 1cc78c9 into sonic-net:master Mar 6, 2024
18 checks passed
sonic-otn pushed a commit to Weitang-Zheng/sonic-buildimage that referenced this pull request Mar 11, 2024
Improve nss-tacplus TACACS connect timeout

#### Why I did it
TACACS login usually config multiple server, when a high priority server not reachable or high lantency, remote user login will slowly, also run SUDO command will slowly.
To improve this issue, enable read timeout in nss-tacplus.

### How I did it
Enable read timeout in nss-tacplus.

#### How to verify it
Pass all UT.

### Description for the changelog
Improve nss-tacplus TACACS connect timeout.
saksarav-nokia pushed a commit to saksarav-nokia/sonic-buildimage that referenced this pull request Mar 12, 2024
Improve nss-tacplus TACACS connect timeout

#### Why I did it
TACACS login usually config multiple server, when a high priority server not reachable or high lantency, remote user login will slowly, also run SUDO command will slowly.
To improve this issue, enable read timeout in nss-tacplus.

### How I did it
Enable read timeout in nss-tacplus.

#### How to verify it
Pass all UT.

### Description for the changelog
Improve nss-tacplus TACACS connect timeout.
liuh-80 added a commit to liuh-80/sonic-buildimage that referenced this pull request Aug 19, 2024
Improve nss-tacplus TACACS connect timeout

#### Why I did it
TACACS login usually config multiple server, when a high priority server not reachable or high lantency, remote user login will slowly, also run SUDO command will slowly.
To improve this issue, enable read timeout in nss-tacplus.

### How I did it
Enable read timeout in nss-tacplus.

#### How to verify it
Pass all UT.

### Description for the changelog
Improve nss-tacplus TACACS connect timeout.
liuh-80 added a commit to liuh-80/sonic-buildimage that referenced this pull request Aug 19, 2024
Improve nss-tacplus TACACS connect timeout

#### Why I did it
TACACS login usually config multiple server, when a high priority server not reachable or high lantency, remote user login will slowly, also run SUDO command will slowly.
To improve this issue, enable read timeout in nss-tacplus.

### How I did it
Enable read timeout in nss-tacplus.

#### How to verify it
Pass all UT.

### Description for the changelog
Improve nss-tacplus TACACS connect timeout.
mssonicbld pushed a commit to mssonicbld/sonic-buildimage that referenced this pull request Aug 22, 2024
Improve nss-tacplus TACACS connect timeout

#### Why I did it
TACACS login usually config multiple server, when a high priority server not reachable or high lantency, remote user login will slowly, also run SUDO command will slowly.
To improve this issue, enable read timeout in nss-tacplus.

### How I did it
Enable read timeout in nss-tacplus.

#### How to verify it
Pass all UT.

### Description for the changelog
Improve nss-tacplus TACACS connect timeout.
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202311: #19981

mssonicbld pushed a commit that referenced this pull request Aug 22, 2024
Improve nss-tacplus TACACS connect timeout

#### Why I did it
TACACS login usually config multiple server, when a high priority server not reachable or high lantency, remote user login will slowly, also run SUDO command will slowly.
To improve this issue, enable read timeout in nss-tacplus.

### How I did it
Enable read timeout in nss-tacplus.

#### How to verify it
Pass all UT.

### Description for the changelog
Improve nss-tacplus TACACS connect timeout.
mssonicbld pushed a commit to mssonicbld/sonic-buildimage that referenced this pull request Aug 23, 2024
Improve nss-tacplus TACACS connect timeout

#### Why I did it
TACACS login usually config multiple server, when a high priority server not reachable or high lantency, remote user login will slowly, also run SUDO command will slowly.
To improve this issue, enable read timeout in nss-tacplus.

### How I did it
Enable read timeout in nss-tacplus.

#### How to verify it
Pass all UT.

### Description for the changelog
Improve nss-tacplus TACACS connect timeout.
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202305: #19989

mssonicbld pushed a commit that referenced this pull request Aug 27, 2024
Improve nss-tacplus TACACS connect timeout

#### Why I did it
TACACS login usually config multiple server, when a high priority server not reachable or high lantency, remote user login will slowly, also run SUDO command will slowly.
To improve this issue, enable read timeout in nss-tacplus.

### How I did it
Enable read timeout in nss-tacplus.

#### How to verify it
Pass all UT.

### Description for the changelog
Improve nss-tacplus TACACS connect timeout.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants