Skip to content

Fix #37, Offset the UDP base port per-CPU#41

Merged
astrogeco merged 1 commit into
nasa:integration-candidatefrom
jphickey:fix-37-port-offset
Mar 23, 2020
Merged

Fix #37, Offset the UDP base port per-CPU#41
astrogeco merged 1 commit into
nasa:integration-candidatefrom
jphickey:fix-37-port-offset

Conversation

@jphickey

Copy link
Copy Markdown
Contributor

Describe the contribution
Avoid port conflicts when having more than one CPU within the same mission load CI_LAB app. Instead of binding to the port directly, add the CPU number as an offset. Subtract 1 so that CPU1 remains at the same port (1234 by default) and cmdUtil is not affected.

NOTE: the "cpu number" can be overridden on the command line when using the pc-linux PSP, thereby also permitting the listen point to be tuned for each launch config, if necessary.

Testing performed
Build using default config, ensure no errors.
Run FSW and test sending commands with cmdUtil - confirm no impact when using defaults, still listens on port 1234 as before and can send commands.
Run FSW using --cpuid 2 switch, confirm that listening port has changed, observing syslog trace:
1980-012-14:03:20.25227 CI_LAB listening on UDP port: 1235
Confirmed that cmdUtil still works to send commands if the --port 1235 option is given.

Expected behavior changes

  • No behavior changes in the default/example config when also running with all defaults.
  • CI_LAB changes its listening port when either using a config with multiple CPUs or using the --cpuid option to override (in pc-linux at least).

System(s) tested on

  • Ubuntu 18.04 LTS, 64 bit

Contributor Info - All information REQUIRED for consideration of pull request
Joseph Hickey, Vantage Systems, Inc.

Avoid port conflicts when having more than one CPU within the same
mission load CI_LAB app.  Instead of binding to the port directly,
add the CPU number as an offset.  Subtract 1 so that CPU1 remains
at the same port (1234 by default) and cmdUtil is not affected.

NOTE: the "cpu number" can be overridden on the command line when
using the pc-linux PSP, thereby also permitting the listen point
to be tuned for each launch config, if necessary.
@jphickey

Copy link
Copy Markdown
Contributor Author

Possible topic for CCB discussion:
This change leaves the default port at "1234", which is the old value, for compatibility but this fairly random.
It would be a little more user-friendly to make this e.g. "10000" or something like that, so that the numbers are a little easier to work with for humans.

@jphickey jphickey added the CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) label Mar 12, 2020
@skliper

skliper commented Mar 16, 2020

Copy link
Copy Markdown
Contributor

My only concern related to this change was if the community was using CPU number for something else this association could cause breakage. That said it gets reported at startup, it's a lab app, and this solves a port collision issue and doesn't cause me any issues so I'm good with it.

@astrogeco astrogeco linked an issue Mar 18, 2020 that may be closed by this pull request
@astrogeco astrogeco added this to the 2.4.0 milestone Mar 18, 2020
@astrogeco

Copy link
Copy Markdown
Contributor

CCB 20200318 - Approved

1 similar comment
@astrogeco

astrogeco commented Mar 18, 2020

Copy link
Copy Markdown
Contributor

CCB 20200318 - Approved

@astrogeco astrogeco added CCB - 20200318 CCB:Approved Indicates code approval by CCB and removed CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) labels Mar 18, 2020
@astrogeco astrogeco changed the base branch from master to integration-candidate March 23, 2020 16:59
@astrogeco astrogeco merged commit 8620c62 into nasa:integration-candidate Mar 23, 2020
@jphickey jphickey deleted the fix-37-port-offset branch June 8, 2021 18:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CCB:Approved Indicates code approval by CCB

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CI LAB fails when running more than one instance on the same host

3 participants