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

Added shared memory multi-server feature #917

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jkuro-tii
Copy link
Contributor

Description of changes

The memsocket application has been enhanced with the following new features and updates:

Host Support:
    The application can now run on the host in addition to virtual machines (VMs).

Multi-Server Capability:
    Multiple memsocket instances operating in server mode are now supported.
    Each server instance is configured with a list of specific client IDs it serves.
    Multiple servers can run simultaneously on a single VM or the host.

Client ID Exclusivity:
    Each client ID must be managed by only one server across the entire system.

Configuration Updates:
    Renamed some Nix configuration options for clarity.

Option Changes:
    The -c and -s options have been swapped:
        -c: Now used for running in server mode.
        -s: Now used for running in client mode.
    A new -l option has been introduced for server mode to specify the list of client IDs to be serviced.   
    A new -h option has been introduced for indicating run on the host

Documentation: https://confluence.tii.ae/x/uEPOAg

Checklist for things done

  • [ X] Summary of the proposed changes in the PR description
  • [ X] More detailed description in the commit message(s)
  • [ X] Commits are squashed into relevant entities - avoid a lot of minimal dev time commits in the PR
  • Contribution guidelines followed
  • Ghaf documentation updated with the commit - https://tiiuae.github.io/ghaf/
  • PR linked to architecture documentation and requirement(s) (ticket id)
  • Test procedure described (or includes tests). Select one or more:
    • Tested on Lenovo X1 x86_64
    • Tested on Jetson Orin NX or AGX aarch64
    • Tested on Polarfire riscv64
  • Author has run make-checks and it passes
  • All automatic Github Action checks pass - see actions
  • Author has added reviewers and removed PR draft status
  • Change requires full re-installation
  • Change can be updated with nixos-rebuild ... switch

Instructions for Testing

  • List all targets that this applies to:
  • Is this a new feature
    • List the test steps to verify:
  • If it is an improvement how does it impact existing functionality?

Enabled memsocket to run on the host in addition to VMs
Added support for multiple server instances, each managing a unique set of client IDs
Enforced exclusivity of client IDs across all servers
Renamed some Nix configuration options for clarity
Swapped -c (server mode) and -s (client mode) options
Added -l option in server mode to define the list of managed client IDs

Signed-off-by: Jaroslaw Kurowski <[email protected]>
@jkuro-tii jkuro-tii temporarily deployed to internal-build-workflow November 28, 2024 07:53 — with GitHub Actions Inactive
@jkuro-tii jkuro-tii marked this pull request as ready for review November 28, 2024 09:00
@josa41
Copy link
Contributor

josa41 commented Nov 28, 2024

Nothing too obvious to comment in the code.
I did test this also with pulseaudio and got everything to work just fine.
During my testing i found two issues.

  1. If the application socket (pulseaudio in my tests) is not available when memsocket starts then data does not reach the socket. Restarting memsocket helps for this or adding a delay for memsocket to start after the application.
  2. Memsocket/kernel crashes when a second memsocket server is started from command line on the same VM with another id

I passed more details to Jarek in slack

@mbssrc
Copy link
Collaborator

mbssrc commented Dec 4, 2024

Any idea how this can or should be tested?

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