Hi, On 2/6/2025 11:01 AM, Yan Zhai wrote: > On Wed, Feb 5, 2025 at 6:46 PM Hou Tao <houtao@xxxxxxxxxxxxxxx> wrote: >> >> Hi, >> >> On 2/6/2025 12:27 AM, Yan Zhai wrote: >>> On Wed, Feb 5, 2025 at 3:56 AM Alexei Starovoitov >>> <alexei.starovoitov@xxxxxxxxx> wrote: >>>> Let's not invent new magic return values. >>>> >>>> But stepping back... why do we have this EINTR case at all? >>>> Can we always goto next_key for all map types? >>>> The command returns and a set of (key, value) pairs. >>>> It's always better to skip then get stuck in EINTR, >>>> since EINTR implies that the user space should retry and it >>>> might be successful next time. >>>> While here it's not the case. >>>> I don't see any selftests for EINTR, so I suspect it was added >>>> as escape path in case retry count exceeds 3 and author assumed >>>> that it should never happen in practice, so EINTR was expected >>>> to be 'never happens'. Clearly that's not the case. >>> It makes more sense to me if we just goto the next key for all types. >>> At least for current users of generic batch lookup, arrays and >>> lpm_trie, I didn't notice in any case retry would help. >> I think it will break lpm_trie. In lpm_trie, if tries to find the next >> key of a non-existent key, it will restart from the left-mode node. > I am not sure how lpm trie would break if we always skip to the next > key. Current retry logic does not change prev_key, so the lookup key > will always be the same. It would make sense if searching with the > same key could temporarily fail, but it does not seem so for both > lpm_tire and array based maps. Retry logic does change prev_key, please see "swap(prev_key, key);" below the next_key tag, otherwise the lookup_batch procedure will loop forever for array map. > > Yan > >>> best >>> Yan > .
