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/bpf: Fix writing of buffer bigger than PAGESIZE (Bug 205164) #131

Closed
wants to merge 1 commit into from

Conversation

fflorens
Copy link

When allocating the mbuf we used m_get2 which fails
if len is superior to MJUMPAGESIZE, if its the case,
use m_getjcl instead.

This fixes bug 205164.

When allocating the mbuf we used m_get2 which fails
if len is superior to MJUMPAGESIZE, if its the case,
use m_getjcl instead.

This fixes bug 205164.
@kprovost
Copy link
Contributor

This may be useful context:
https://lists.freebsd.org/pipermail/freebsd-net/2017-November/049231.html

The main concern is that allocating large contiguous memory areas (as is required for BPF) is problematic, especially under memory pressure. Changing the BPF code to cope with non-contiguous memory would be difficult, so I'm not sure what the best solution would be here.

@fflorens
Copy link
Author

Well as i am fairly new to the internals of bpf, im someone can help me get up to speed, maybe i could patch this in a 'better' way allowing jumbo writes even under memory pressure.

@kprovost
Copy link
Contributor

After the allocation of the mbuf the data that's supposed to be sent is copied from user space (that's the uiomove() call). That can be adapted to deal with an mbuf chain.
After that the filter program is applied (bpf_filter()), but that function assumes a flat (i.e. contiguous) buffer to operate on. It executes the BPF bytecode on that buffer. Changing that bytecode interpreter is not a realistic option I'm afraid.

@fflorens
Copy link
Author

When using M_NOWAIT to allocate a buffer, shouldn't it fail under memory pressure ? thus avoiding any 'real' issue with big clusters under memory pressure.

@kprovost
Copy link
Contributor

That helps, but holding contiguous memory (larger than a single page) is problematic too.

Ryan might be able to give you more information: https://lists.freebsd.org/pipermail/freebsd-net/2017-November/049258.html

@bsdimp
Copy link
Member

bsdimp commented Jun 23, 2021

Hash f13da24 lands this. I confirmed with kp@ that this was a good chance and that the negative effects noted by Ryan are less bad than this not working.

@bsdimp bsdimp closed this Jun 23, 2021
freebsd-git pushed a commit that referenced this pull request Jun 23, 2021
When allocating the mbuf we used m_get2 which fails
if len is superior to MJUMPAGESIZE, if its the case,
use m_getjcl instead.

Reviewed by:	kp@
PR:		205164
Pull Request:	#131
bsdjhb pushed a commit to bsdjhb/cheribsd that referenced this pull request Dec 11, 2021
When allocating the mbuf we used m_get2 which fails
if len is superior to MJUMPAGESIZE, if its the case,
use m_getjcl instead.

Reviewed by:	kp@
PR:		205164
Pull Request:	freebsd/freebsd-src#131
@emaste emaste added the merged label Jun 12, 2023
freebsd-git pushed a commit that referenced this pull request Sep 17, 2023
When allocating the mbuf we used m_get2 which fails
if len is superior to MJUMPAGESIZE, if its the case,
use m_getjcl instead.

Reviewed by:	kp@
PR:		205164
Pull Request:	#131

(cherry picked from commit f13da24)
fichtner pushed a commit to opnsense/src that referenced this pull request Sep 27, 2023
When allocating the mbuf we used m_get2 which fails
if len is superior to MJUMPAGESIZE, if its the case,
use m_getjcl instead.

Reviewed by:	kp@
PR:		205164
Pull Request:	freebsd/freebsd-src#131

(cherry picked from commit f13da24)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants