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

Add session eviction logic to CASE session management #17568

Closed
Tracked by #17298
mrjerryjohns opened this issue Apr 20, 2022 · 0 comments · Fixed by #19903
Closed
Tracked by #17298

Add session eviction logic to CASE session management #17568

mrjerryjohns opened this issue Apr 20, 2022 · 0 comments · Fixed by #19903
Assignees
Labels
p1 priority 1 work secure channel spec Mismatch between spec and implementation V1.0

Comments

@mrjerryjohns
Copy link
Contributor

As part of the tenets outlined in #17298, support needs to be added to the management of sessions in the SecureSessionTable to properly do the following

  1. Instead of CreateNewSecureSession failing if there are no slots available, it should evict the oldest session in the table with the following sorting criteria:
    i. A primary sort of all sessions based on time at which the session was established.
    ii. A secondary sort of the resultant set based on fabrics that over their specified minimas for active CASE sessions.
    iii. A tertiary sort of the resultant set based on nodes that have more than 1 active session to them.

The candidate at the top of the sorted list should be selected.

  1. Remove the ExpireInactiveSessions and associated logic. Sessions should not expire on a timer.
  2. FindSecureSessionForNode should return the most recent session established to a given peer.
tcarmelveilleux added a commit to tcarmelveilleux/connectedhomeip that referenced this issue Apr 26, 2022
- From project-chip#17422 onwards, "previous" CASE sessions were not expired
  in terms of the session cache if sessions were constantly
  re-established.
- A root cause solution will be to solve project-chip#17568, but in the meantime,
  cert testing started failing after N (where N is session manager
  pool size) runs.

This PR cleans-up the previous sessions from same node (duplicate sessions
that should not happen). This is a bit unclean but will do to unblock
testing since it was good enough before, and the better method is not
yet implemented.

This PR also adds logging when this happens.

Fixes project-chip#17698

Testing done:
- Unit tests pass
- Cert tests pass
- Manual run of N > 16
  `for i in {1..18}; do ./out/debug/standalone/chip-tool basic read vendor-name 115566 0 ; done`
  does not fail as before.
kghost pushed a commit that referenced this issue Apr 27, 2022
* Quick-fix running out of CASE sessions

- From #17422 onwards, "previous" CASE sessions were not expired
  in terms of the session cache if sessions were constantly
  re-established.
- A root cause solution will be to solve #17568, but in the meantime,
  cert testing started failing after N (where N is session manager
  pool size) runs.

This PR cleans-up the previous sessions from same node (duplicate sessions
that should not happen). This is a bit unclean but will do to unblock
testing since it was good enough before, and the better method is not
yet implemented.

This PR also adds logging when this happens.

Fixes #17698

Testing done:
- Unit tests pass
- Cert tests pass
- Manual run of N > 16
  `for i in {1..18}; do ./out/debug/standalone/chip-tool basic read vendor-name 115566 0 ; done`
  does not fail as before.

* Restyled by clang-format

* Fix CI

Co-authored-by: Restyled.io <[email protected]>
@mrjerryjohns mrjerryjohns added the spec Mismatch between spec and implementation label Jun 14, 2022
@mrjerryjohns mrjerryjohns added the p1 priority 1 work label Jun 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p1 priority 1 work secure channel spec Mismatch between spec and implementation V1.0
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants