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-tcp_bbr: v3: use correct 64-bit division [bbr-v3-upstream-prep-2024-02-19-01] #11

Open
wants to merge 1 commit into
base: bbr-v3-upstream-prep-2024-02-19-01
Choose a base branch
from

Conversation

ptpt52
Copy link

@ptpt52 ptpt52 commented Sep 21, 2024

This commit addresses an issue with integer division on 32-bit system builds, which resulted in undefined symbol errors during the build process.

Errors observed:

arm:
ERROR: modpost: "__aeabi_uldivmod" [net/ipv4/tcp_bbr.ko] undefined! ERROR: modpost: "__aeabi_ldivmod" [net/ipv4/tcp_bbr.ko] undefined!

x86, mips, ppc:
ERROR: modpost: "__udivdi3" [net/ipv4/tcp_bbr.ko] undefined! ERROR: modpost: "__divdi3" [net/ipv4/tcp_bbr.ko] undefined!

The fix ensures proper 64-bit division on affected architectures.

Copy link

google-cla bot commented Sep 21, 2024

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

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@ptpt52 ptpt52 force-pushed the bbr-v3-upstream-prep-2024-02-19-01 branch from 85c4407 to 0bb5b56 Compare September 21, 2024 15:53
@nealcardwell
Copy link
Collaborator

nealcardwell commented Sep 23, 2024

Thanks for the patch! Let's try to converge with the upstream tcp_tso_autosize():
https://elixir.bootlin.com/linux/v6.10/source/net/ipv4/tcp_output.c#L2013

The motivation here: we will likely ultimately just switch BBR to using that. So instead of this patch, can you please change the declaration of the "bytes" variable from:

 u64 bytes;

to:

 unsigned long bytes;

I imagine that will fix it? If so, let's go with that.

Thanks!

…tso_segs_generic`

This change addresses a build failure on 32-bit systems due to undefined division symbols:

arm:
ERROR: modpost: "__aeabi_uldivmod" [net/ipv4/tcp_bbr.ko] undefined!
ERROR: modpost: "__aeabi_ldivmod" [net/ipv4/tcp_bbr.ko] undefined!

x86, mips, ppc:
ERROR: modpost: "__udivdi3" [net/ipv4/tcp_bbr.ko] undefined!
ERROR: modpost: "__divdi3" [net/ipv4/tcp_bbr.ko] undefined!

Since `sk->sk_pacing_rate` is already an `unsigned long`, the `bytes` variable is
updated to `unsigned long` to resolve these division issues and ensure compatibility
across both 32-bit and 64-bit platforms.

Signed-off-by: Chen Minqiang <ptpt52@gmail.com>
@ptpt52 ptpt52 force-pushed the bbr-v3-upstream-prep-2024-02-19-01 branch from 0bb5b56 to dcc8b20 Compare September 27, 2024 02:09
@ptpt52
Copy link
Author

ptpt52 commented Sep 27, 2024

@nealcardwell
updated.
tested build ok for mips.

This change addresses a build failure on 32-bit systems due to undefined division symbols:

arm:
ERROR: modpost: "__aeabi_uldivmod" [net/ipv4/tcp_bbr.ko] undefined!
ERROR: modpost: "__aeabi_ldivmod" [net/ipv4/tcp_bbr.ko] undefined!

x86, mips, ppc:
ERROR: modpost: "__udivdi3" [net/ipv4/tcp_bbr.ko] undefined!
ERROR: modpost: "__divdi3" [net/ipv4/tcp_bbr.ko] undefined!

Since `sk->sk_pacing_rate` is already an `unsigned long`, the `bytes` variable is
updated to `unsigned long` to resolve these division issues and ensure compatibility
across both 32-bit and 64-bit platforms.

@nealcardwell
Copy link
Collaborator

Hi. I just noticed this is proposing to add this commit to bbr-v3-upstream-prep-2024-02-19-01. Any reason you are proposing to use that branch instead of v3?

@ptpt52
Copy link
Author

ptpt52 commented Sep 27, 2024

Hi. I just noticed this is proposing to add this commit to bbr-v3-upstream-prep-2024-02-19-01. Any reason you are proposing to use that branch instead of v3?

I happened to use the bbr-v3-upstream-prep-2024-02-19-01 branch, but there’s no specific reason for it. You can easily cherry-pick this commit onto the v3 branch, or would you prefer me to submit a new PR directly to v3?

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