Hi, On 12/6/2024 1:06 AM, Alexei Starovoitov wrote: > On Thu, Dec 5, 2024 at 12:53 AM Hou Tao <houtao@xxxxxxxxxxxxxxx> wrote: >> Hi, >> >> On 12/3/2024 9:42 AM, Alexei Starovoitov wrote: >>> On Fri, Nov 29, 2024 at 4:18 AM Toke Høiland-Jørgensen <toke@xxxxxxxxxx> wrote: >>>> Hou Tao <houtao@xxxxxxxxxxxxxxx> writes: >>>> >>>>> From: Hou Tao <houtao1@xxxxxxxxxx> >>>>> >>>>> After switching from kmalloc() to the bpf memory allocator, there will be >>>>> no blocking operation during the update of LPM trie. Therefore, change >>>>> trie->lock from spinlock_t to raw_spinlock_t to make LPM trie usable in >>>>> atomic context, even on RT kernels. >>>>> >>>>> The max value of prefixlen is 2048. Therefore, update or deletion >>>>> operations will find the target after at most 2048 comparisons. >>>>> Constructing a test case which updates an element after 2048 comparisons >>>>> under a 8 CPU VM, and the average time and the maximal time for such >>>>> update operation is about 210us and 900us. >>>> That is... quite a long time? I'm not sure we have any guidance on what >>>> the maximum acceptable time is (perhaps the RT folks can weigh in >>>> here?), but stalling for almost a millisecond seems long. >>>> >>>> Especially doing this unconditionally seems a bit risky; this means that >>>> even a networking program using the lpm map in the data path can stall >>>> the system for that long, even if it would have been perfectly happy to >>>> be preempted. >>> I don't share this concern. >>> 2048 comparisons is an extreme case. >>> I'm sure there are a million other ways to stall bpf prog for that long. >> 2048 is indeed an extreme case. I would do some test to check how much >> time is used for the normal cases with prefixlen=32 or prefixlen=128. > Before you do that please respin with comments addressed, so we can > land the fixes asap. OK. Original I thought there was no need for respin. Before posting the v3, I want to confirm the comments which need to be addressed in the new revision: 1) [PATCH bpf v2 6/9] bpf: Switch to bpf mem allocator for LPM trie Move bpf_mem_cache_free_rcu outside of the locked scope (From Alexei) Move the first lpm_trie_node_alloc() outside of the locked scope (There will be no refill under irq disabled region) 2) [PATCH bpf v2 2/9] bpf: Remove unnecessary kfree(im_node) in lpm_trie_update_elem Remove the NULL init of im_node (From Daniel)
