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

Refactor ingest to utilize zero copy (can reduce two copy to one) #73

Closed
skliper opened this issue Mar 5, 2021 · 2 comments · Fixed by #84 or #85
Closed

Refactor ingest to utilize zero copy (can reduce two copy to one) #73

skliper opened this issue Mar 5, 2021 · 2 comments · Fixed by #84 or #85
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@skliper
Copy link
Contributor

skliper commented Mar 5, 2021

Is your feature request related to a problem? Please describe.
CI copies the data into a local buffer and then uses CFE_SB_TransmitMsg (which does a second copy into the SB buffer):

ci_lab/fsw/src/ci_lab_app.c

Lines 359 to 374 in 296d12c

void CI_LAB_ReadUpLink(void)
{
int i;
int32 status;
for (i = 0; i <= 10; i++)
{
status = OS_SocketRecvFrom(CI_LAB_Global.SocketID, CI_LAB_Global.IngestBuffer.bytes,
sizeof(CI_LAB_Global.IngestBuffer), &CI_LAB_Global.SocketAddress, OS_CHECK);
if (status >= (int32)sizeof(CFE_MSG_CommandHeader_t) && status <= ((int32)CI_LAB_MAX_INGEST))
{
CFE_ES_PerfLogEntry(CI_LAB_SOCKET_RCV_PERF_ID);
CI_LAB_Global.HkTlm.Payload.IngestPackets++;
status = CFE_SB_TransmitMsg(&CI_LAB_Global.IngestBuffer.Msg, false);
CFE_ES_PerfLogExit(CI_LAB_SOCKET_RCV_PERF_ID);
}

Describe the solution you'd like
Could use zero copy to get a buffer, write directly to the buffer and CFE_SB_TransmitBuffer (single copy)

Describe alternatives you've considered
None

Additional context
We don't have a functional example of zero copy, just documentation. This would support user request for a working example as well as improve performance.

Requester Info
Jacob Hageman - NASA/GSFC (from stakeholder request)

@skliper skliper added the enhancement New feature or request label Mar 5, 2021
@skliper skliper added this to the 2.5.0 milestone Mar 5, 2021
@jphickey
Copy link
Contributor

jphickey commented Mar 5, 2021

I actually like the fact that this copies the packet, because CI/TO should someday take on the role of dealing with the differences between network encoding/padding and native encoding/padding.

That is, the "dumb" memcpy that is done now can be replaced with a "smart" endian- and padding- aware copy. It is the ideal place to do this because its transparent to all apps.

I don't want to re-open culture wars on the subject - even if some users don't want to do it this way for some reason - I'm just saying its a valid and useful option to have, and it works well (speaking from experience).

@jphickey
Copy link
Contributor

jphickey commented Mar 5, 2021

After thinking about this for another minute - it's probably fine either way because even in the "smart" CI/TO copy case it can still use the zero copy method to send the message on once its been fixed up.

However - we do need to coordinate this with nasa/cFE#1155. That one is waiting right behind the reorganization stuff in nasa/cFE#1203.

@skliper skliper assigned jphickey and unassigned zanzaben Mar 23, 2021
jphickey added a commit to jphickey/ci_lab that referenced this issue Mar 23, 2021
Updates CI_LAB to obtain a buffer prior to calling OS_SocketRecvFrom,
and then transmit that same buffer directly rather than copying it.

This demonstrates use of the Zero Copy API.
astrogeco added a commit that referenced this issue Mar 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants