Allow resize of driverlink_filter table via module parameter - #340
Open
zhuyifei1999 wants to merge 1 commit into
Open
zhuyifei1999 wants to merge 1 commit into
zhuyifei1999 wants to merge 1 commit into
Conversation
The steering of ICMP packets towards the right onload stack is handled via a global hash table efab_tcp_driver.dlfilter->table, whose size is hardcoded to EFAB_DLFILT_ENTRY_COUNT = 16384. Listening on thousands of ports with many IPs run into the hash table exhaustion, causing dlfilter_insert to return -ELOOP and console logspam, not to mention potential loss of ICMP packets. There is a "FIXME: This really should not be defined here" for the size, so this patch makes the size user-defined via module parameter. To increase the size, the "state" attribute in efx_dlfilt_entry must be increased. The upper two bits are used to store the bits for EFAB_DLFILT_TOMBSTONE & EFAB_DLFILT_STATE_MASK. With original u16, the size could be maximum of 2^14 = 16384. This patch increases the max to u32, allowing 2^30 = 1073741824 table size, which should be sufficient for any use case. The default size is left as the original 16384 unless the module parameter is modified. With a resizable table, the table array is no longer a part of efx_dlfilt_cb, but rather efx_dlfilt_cb points to a vmalloc-ed array for the table. This adds one dereference from efx_dlfilt_cb to the table. However, since efx_dlfilt_cb is now a small struct, it is flattened into efab_tcp_driver to avoid the extra dereference. To avoid concurrency & use-after-free issues while preserving performance, the table size becomes read only once the first filter is added to the table. Many functions that use the table will check if the table was used at all before accessing the table, i.e. the table pointer is only accessed if it cannot be mutated. efx_dlfilt_cb points to a tagged pointer for the table, with the LSB representing whether it is used. The fast path is that it is used & untagged. Most table accesses can rely on address dependency to guarantee memory ordering, while the accesses that mutates the tagged pointer (either the tag or the pointer) have explicit acquire / release ordering for guarantees. Signed-off-by: YiFei Zhu <zhuyifei@google.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The steering of ICMP packets towards the right onload stack is handled via a global hash table efab_tcp_driver.dlfilter->table, whose size is hardcoded to EFAB_DLFILT_ENTRY_COUNT = 16384. Listening on thousands of ports with many IPs run into the hash table exhaustion, causing dlfilter_insert to return -ELOOP and console logspam, not to mention potential loss of ICMP packets. There is a "FIXME: This really should not be defined here" for the size, so this patch makes the size user-defined via module parameter.
To increase the size, the "state" attribute in efx_dlfilt_entry must be increased. The upper two bits are used to store the bits for EFAB_DLFILT_TOMBSTONE & EFAB_DLFILT_STATE_MASK. With original u16, the size could be maximum of 2^14 = 16384. This patch increases the max to u32, allowing 2^30 = 1073741824 table size, which should be sufficient for any use case. The default size is left as the original 16384 unless the module parameter is modified.
With a resizable table, the table array is no longer a part of efx_dlfilt_cb, but rather efx_dlfilt_cb points to a vmalloc-ed array for the table. This adds one dereference from efx_dlfilt_cb to the table. However, since efx_dlfilt_cb is now a small struct, it is flattened into efab_tcp_driver to avoid the extra dereference.
To avoid concurrency & use-after-free issues while preserving performance, the table size becomes read only once the first filter is added to the table. Many functions that use the table will check if the table was used at all before accessing the table, i.e. the table pointer is only accessed if it cannot be mutated. efx_dlfilt_cb points to a tagged pointer for the table, with the LSB representing whether it is used. The fast path is that it is used & untagged. Most table accesses can rely on address dependency to guarantee memory ordering, while the accesses that mutates the tagged pointer (either the tag or the pointer) have explicit acquire / release ordering for guarantees.