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

New element: SetTCPChecksumIP6 - Calculates TCP/IPv6 checksum and sets the TCP header field. #332

Merged
merged 4 commits into from
Apr 19, 2021

Conversation

kkovacs
Copy link
Contributor

@kkovacs kkovacs commented Apr 16, 2021

The existing SetTCPChecksum element only supports TCP over IPv4. This is a new element that does the same for TCP over IPv6.

The new element's naming convention follows FastUDPSource and FastUDPSourceIP6, and is also the reason I've put this in tcpudp/ instead of ip6/.

The code is based mostly on the original SetTCPChecksum and the way FastUDPSourceIP6 calculates checksum for its packets.

@kkovacs
Copy link
Contributor Author

kkovacs commented Apr 17, 2021

I do see that a "CodeQL" build step is failing on the PR (but all builds are successful), but this one does not seem related to my code, it says:

"Semmle autobuild: no supported build system detected. We were unable to automatically build your code. Please replace the call to the autobuild action with your custom build steps."

Copy link
Owner

@tbarbette tbarbette left a comment

Choose a reason for hiding this comment

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

Thank you very much! Could you just set the copyright correctly?

@@ -0,0 +1,71 @@
/*
* settcpchecksumip6.{cc,hh} -- sets the TCP header checksum for IPv6 packets
* Robert Morris
Copy link
Owner

Choose a reason for hiding this comment

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

Can you add your name and copyright institution? Maybe add "based on settcpchecksum.{cc,hh}"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, of course. I didn't want to claim too much credit, but the way you suggest it is a good way, referencing the others. Pushed!

@tbarbette
Copy link
Owner

Yes the codeql seemed to be a false positive, I'll check after it re-ran.

@kkovacs
Copy link
Contributor Author

kkovacs commented Apr 19, 2021

Copyright was changed as asked (thanks!), now I think only CodeQL is acting up again.

Elment needs ip6 to be enabled
@tbarbette
Copy link
Owner

I found the problem, codeql does not compile with --enable-ip6 and therefore the element is missing a symbol. It's actually legit Should be fixed with the last commit. I'll merge when it passes. Thanks!

@tbarbette
Copy link
Owner

Bad luck... Github has an issue with name resolution right now and cannot resolve fast.dpdk.org... Will re-launch the CI in a bit.

@tbarbette tbarbette merged commit 828d726 into tbarbette:master Apr 19, 2021
@kkovacs
Copy link
Contributor Author

kkovacs commented Apr 19, 2021

Thanks for the quick merge, Tom! And thank you for doing fastclick!

@kkovacs kkovacs deleted the feature/ip6-tcpchecksum branch April 19, 2021 16:18
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.

2 participants