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

TCP: Add Linux kernel-based random number generator #7

Merged
merged 4 commits into from Apr 24, 2019

Conversation

bauuuu1021
Copy link
Contributor

Previous generator may not work correctly, so replaced
it with kernel random number source devices

Previous generator may not work correctly, so replaced
it with kernel random number source devices
@jserv
Copy link
Owner

jserv commented Apr 24, 2019

Since it is specific to Linux, you shall use conditional build for such modifications. That is, add #if defined(__linux__) and #endif pair.

Because the modification is only availible on Linux
@@ -1,7 +1,9 @@
#include <errno.h>
#include <fcntl.h>
#include <linux/random.h>
Copy link
Owner

Choose a reason for hiding this comment

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

Conditional build here as well.

Copy link
Owner

Choose a reason for hiding this comment

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

Also, use clang-format -i to indent.

src/tcp.c Outdated
@@ -246,8 +251,14 @@ static int tcp_fsm(struct tcp_conn_tcb *conn,
}
rs->tcp_flags |= TCP_ACK;
rs->tcp_ack_num = rs->tcp_seqno + 1;
srand(time(NULL));
Copy link
Owner

Choose a reason for hiding this comment

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

Why did you remove srand?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

srand(time(NULL)); was the seed of rand()%100;, and I replaced both lines with kernel random number source device.
So srand is no longer needed.

Copy link
Owner

Choose a reason for hiding this comment

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

You have to take care of both POSIX (generic) and Linux kernel random number generator.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mean that using srand (previous generator) while the environment is POSIX?

Copy link
Owner

Choose a reason for hiding this comment

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

Exactly. Only the files in linux/ directory can introduce Linux-specific implementations. Always keep POSIX compatibility in mind.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it!

src/tcp.c Outdated
@@ -1,7 +1,12 @@
#if defined __linux__
Copy link
Owner

Choose a reason for hiding this comment

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

Rewrite as #if defined (__linux__)

src/tcp.c Outdated
srand(time(NULL));
rs->tcp_seqno = rand() % 100;

#if defined __linux__
Copy link
Owner

Choose a reason for hiding this comment

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

Ditto.

@jserv jserv changed the title Use a more proper random number generator TCP: Add Linux kernel-based random number generator Apr 24, 2019
@jserv jserv merged commit f4d6053 into jserv:master Apr 24, 2019
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.

None yet

2 participants