| Message ID | 1519280658-4918-2-git-send-email-jasowang@redhat.com |
|---|---|
| State | Superseded, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | [net,v2,1/2] Revert "tuntap: add missing xdp flush" | expand |
Hello! On 2/22/2018 9:24 AM, Jason Wang wrote: > Commit 762c330d670e ("tuntap: add missing xdp flush") tries to fix the > devmap stall caused by missed xdp flush by counting the pending xdp > redirected packets and flush when it exceeds NAPI_POLL_WEIGHT or > MSG_MORE is clear. This may lead BUG() since xdp_do_flush() was Lead to BUG(). > called under process context with preemption enabled. Simply disable s/under/in the/? > preemption may silent the warning but be not enough since process may Silence. > move between different CPUS during a batch which cause xdp_do_flush() > misses some CPU where the process run previously. Consider the several > fallouts, that commit was reverted. To fix the issue correctly, we can > simply calling xdp_do_flush() immediately after xdp_do_redirect(), Call. > a side effect is that this removes any possibility of batching which > could be addressed in the future. > > Reported-by: Christoffer Dall <christoffer.dall@linaro.org> > Fixes: 762c330d670e ("tuntap: add missing xdp flush") > Signed-off-by: Jason Wang <jasowang@redhat.com> [...] MBR, Sergei
On 2018年02月22日 15:54, Sergei Shtylyov wrote: > Hello! > > On 2/22/2018 9:24 AM, Jason Wang wrote: > >> Commit 762c330d670e ("tuntap: add missing xdp flush") tries to fix the >> devmap stall caused by missed xdp flush by counting the pending xdp >> redirected packets and flush when it exceeds NAPI_POLL_WEIGHT or >> MSG_MORE is clear. This may lead BUG() since xdp_do_flush() was > > Lead to BUG(). > >> called under process context with preemption enabled. Simply disable > > s/under/in the/? > >> preemption may silent the warning but be not enough since process may > > Silence. > >> move between different CPUS during a batch which cause xdp_do_flush() >> misses some CPU where the process run previously. Consider the several >> fallouts, that commit was reverted. To fix the issue correctly, we can >> simply calling xdp_do_flush() immediately after xdp_do_redirect(), > > Call. > >> a side effect is that this removes any possibility of batching which >> could be addressed in the future. >> >> Reported-by: Christoffer Dall <christoffer.dall@linaro.org> >> Fixes: 762c330d670e ("tuntap: add missing xdp flush") >> Signed-off-by: Jason Wang <jasowang@redhat.com> > [...] > > MBR, Sergei My bad, let me post v3. Thanks
diff --git a/drivers/net/tun.c b/drivers/net/tun.c index 2823a4a..a363ea2 100644 --- a/drivers/net/tun.c +++ b/drivers/net/tun.c @@ -1662,6 +1662,7 @@ static struct sk_buff *tun_build_skb(struct tun_struct *tun, get_page(alloc_frag->page); alloc_frag->offset += buflen; err = xdp_do_redirect(tun->dev, &xdp, xdp_prog); + xdp_do_flush_map(); if (err) goto err_redirect; rcu_read_unlock();
Commit 762c330d670e ("tuntap: add missing xdp flush") tries to fix the devmap stall caused by missed xdp flush by counting the pending xdp redirected packets and flush when it exceeds NAPI_POLL_WEIGHT or MSG_MORE is clear. This may lead BUG() since xdp_do_flush() was called under process context with preemption enabled. Simply disable preemption may silent the warning but be not enough since process may move between different CPUS during a batch which cause xdp_do_flush() misses some CPU where the process run previously. Consider the several fallouts, that commit was reverted. To fix the issue correctly, we can simply calling xdp_do_flush() immediately after xdp_do_redirect(), a side effect is that this removes any possibility of batching which could be addressed in the future. Reported-by: Christoffer Dall <christoffer.dall@linaro.org> Fixes: 762c330d670e ("tuntap: add missing xdp flush") Signed-off-by: Jason Wang <jasowang@redhat.com> --- drivers/net/tun.c | 1 + 1 file changed, 1 insertion(+)