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

udp: fix ofp_send/ofp_sendto return value on success #251

Merged
merged 2 commits into from
Jun 15, 2020

Conversation

bogdanPricope
Copy link
Contributor

On UDP side, remaining bytes to process were not updated.

Copy link

@gabeblack gabeblack left a comment

Choose a reason for hiding this comment

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

I tested your change out and it works. It might be nice to change example/socket/socket_send_recv_udp.c ofp_sendto calls to check for <= 0 rather than -1 as a sort of unit/regression test for this fix.

@bogdanPricope
Copy link
Contributor Author

Nope. Validation should be done against sent buffer size for UDP packets.

@bogdanPricope
Copy link
Contributor Author

Note: if you want to check with dpdk pktio you need to wait for my next PR. You may use socket based pktio for this test.

@MatiasElo
Copy link
Contributor

You could mention in the commit message that this commit fixes issue #250.

E.g. Fixes: url_to_issue

On UDP side, remaining bytes to process were not updated.
This commit fixes issue OpenFastPath#250.

Signed-off-by: Bogdan Pricope <[email protected]>
Reviewed-by: Matias Elo <[email protected]>
Returned value of ofp_send() and ofp_sendto() is validated
against sent buffer size for UDP.

Signed-off-by: Bogdan Pricope <[email protected]>
Reviewed-by: Matias Elo <[email protected]>
@MatiasElo MatiasElo merged commit f6cc4a2 into OpenFastPath:master Jun 15, 2020
@bogdanPricope bogdanPricope deleted the sendto_udp branch June 20, 2020 06:11
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.

3 participants