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

net-test: packetdrill: fix compilation/runtime error for linux 3.x #13

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

particle128
Copy link

Hi,

I found this little issue when using packetdrill under linux 3.10. Would you kindly help review the following pull request?

For linux 3.10, TCP_CC_INFO and SO_MEMINFO are not supported. When we define HAVE_SO_MEMINFO / HAVE_TCP_CC_INFO as 0, functions like write_tcp_bbr_cc_info and write_so_meminfo will not be accessed. So wrap them with #ifdef and #endif.

Also, for lower linux version, tcp_info option value has less fields(e.g. 120 bytes). So remove the size assert.

Thanks,
Particle128

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

@particle128
Copy link
Author

I signed it!

@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again. If the bot doesn't comment, it means it doesn't think anything has changed.

For linux 3.10, TCP_CC_INFO and SO_MEMINFO are not supported. When we define HAVE_SO_MEMINFO / HAVE_TCP_CC_INFO as 0, functions like write_tcp_bbr_cc_info and write_so_meminfo will not be accessed. So wrap them with #ifdef and #endif.

Also, for lower linux version, tcp_info option value has less fields(e.g. 120 bytes). So remove the size assert.

Change-Id: Ibb4458067becdb857ff82f680bdd37727b5c2378
@googlebot
Copy link

CLAs look good, thanks!

@particle128
Copy link
Author

Could you kindly help review the PR ?

Thanks,
Particle128

@HughLu
Copy link

HughLu commented Jul 9, 2019

do ./configure and make in packetdrill project , have error info : make: Makefile: No such file or directory
make: *** No rule to make target `Makefile'. Stop.
I build it in imac computer
Can you tell what can I do? Thanks

nealcardwell pushed a commit to nealcardwell/packetdrill that referenced this pull request Feb 18, 2020
For linux 3.10, TCP_CC_INFO and SO_MEMINFO are not supported. When we
define HAVE_SO_MEMINFO / HAVE_TCP_CC_INFO as 0, functions like
write_tcp_bbr_cc_info and write_so_meminfo will not be accessed. So
wrap them with #ifdef and #endif.

Also, for lower linux version, tcp_info option value has less
fields(e.g. 120 bytes). So remove the size assert.

This commit is based on the following packetdrill upstream pull
request, with a slightly tweaked commit description:
  google#13

Signed-off-by: Neal Cardwell <ncardwell@google.com>
Change-Id: I2f4f8cea7357356f2041eabed044b6f7616d4e91
nealcardwell pushed a commit to nealcardwell/packetdrill that referenced this pull request Feb 18, 2020
For linux 3.10, TCP_CC_INFO and SO_MEMINFO are not supported. When we
define HAVE_SO_MEMINFO / HAVE_TCP_CC_INFO as 0, functions like
write_tcp_bbr_cc_info and write_so_meminfo will not be accessed. So
wrap them with #ifdef and #endif.

Also, for lower linux version, tcp_info option value has less
fields(e.g. 120 bytes). So remove the size assert.

This commit is based on the following packetdrill upstream pull
request, with a slightly tweaked commit description:
  google#13

Signed-off-by: Neal Cardwell <ncardwell@google.com>
@nealcardwell
Copy link
Collaborator

Hi. Thank you for your contribution! Sorry for the delay in reviewing this.

I have proposed a version of this pull request with a slightly tweaked formatting of the commit description:
#35

Can you please either:

(A) update your pull request #13 to match the formatting of #35 with an added Signed-off-by: footer containing your name and preferred email.

or

(B) post a message in #35 to confirm that you are OK with the edits I made by leaving a comment that contains only "@googlebot I consent." in the pull request #35

Thanks!
neal

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.

4 participants