* [PATCH bpf] Revert "xdp: add NULL pointer check in __xdp_return()"
@ 2018-08-10 9:28 Björn Töpel
2018-08-10 10:18 ` Jesper Dangaard Brouer
2018-08-10 14:10 ` Daniel Borkmann
0 siblings, 2 replies; 8+ messages in thread
From: Björn Töpel @ 2018-08-10 9:28 UTC (permalink / raw)
To: ast, daniel, netdev, ap420073, brouer
Cc: Björn Töpel, magnus.karlsson, magnus.karlsson, kafai
From: Björn Töpel <bjorn.topel@intel.com>
This reverts commit 36e0f12bbfd3016f495904b35e41c5711707509f.
The reverted commit adds a WARN to check against NULL entries in the
mem_id_ht rhashtable. Any kernel path implementing the XDP (generic or
driver) fast path is required to make a paired
xdp_rxq_info_reg/xdp_rxq_info_unreg call for proper function. In
addition, a driver using a different allocation scheme than the
default MEM_TYPE_PAGE_SHARED is required to additionally call
xdp_rxq_info_reg_mem_model.
For MEM_TYPE_ZERO_COPY, an xdp_rxq_info_reg_mem_model call ensures
that the mem_id_ht rhashtable has a properly inserted allocator id. If
not, this would be a driver bug. A NULL pointer kernel OOPS is
preferred to the WARN.
Suggested-by: Jesper Dangaard Brouer <brouer@redhat.com>
Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
---
net/core/xdp.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/net/core/xdp.c b/net/core/xdp.c
index 6771f1855b96..9d1f22072d5d 100644
--- a/net/core/xdp.c
+++ b/net/core/xdp.c
@@ -345,8 +345,7 @@ static void __xdp_return(void *data, struct xdp_mem_info *mem, bool napi_direct,
rcu_read_lock();
/* mem->id is valid, checked in xdp_rxq_info_reg_mem_model() */
xa = rhashtable_lookup(mem_id_ht, &mem->id, mem_id_rht_params);
- if (!WARN_ON_ONCE(!xa))
- xa->zc_alloc->free(xa->zc_alloc, handle);
+ xa->zc_alloc->free(xa->zc_alloc, handle);
rcu_read_unlock();
default:
/* Not possible, checked in xdp_rxq_info_reg_mem_model() */
--
2.17.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH bpf] Revert "xdp: add NULL pointer check in __xdp_return()"
2018-08-10 9:28 [PATCH bpf] Revert "xdp: add NULL pointer check in __xdp_return()" Björn Töpel
@ 2018-08-10 10:18 ` Jesper Dangaard Brouer
2018-08-10 15:15 ` Björn Töpel
2018-08-10 14:10 ` Daniel Borkmann
1 sibling, 1 reply; 8+ messages in thread
From: Jesper Dangaard Brouer @ 2018-08-10 10:18 UTC (permalink / raw)
To: Björn Töpel
Cc: ast, daniel, netdev, ap420073, Björn Töpel,
magnus.karlsson, magnus.karlsson, kafai, brouer
On Fri, 10 Aug 2018 11:28:02 +0200
Björn Töpel <bjorn.topel@gmail.com> wrote:
> From: Björn Töpel <bjorn.topel@intel.com>
>
> This reverts commit 36e0f12bbfd3016f495904b35e41c5711707509f.
>
> The reverted commit adds a WARN to check against NULL entries in the
> mem_id_ht rhashtable. Any kernel path implementing the XDP (generic or
> driver) fast path is required to make a paired
> xdp_rxq_info_reg/xdp_rxq_info_unreg call for proper function. In
> addition, a driver using a different allocation scheme than the
> default MEM_TYPE_PAGE_SHARED is required to additionally call
> xdp_rxq_info_reg_mem_model.
>
> For MEM_TYPE_ZERO_COPY, an xdp_rxq_info_reg_mem_model call ensures
> that the mem_id_ht rhashtable has a properly inserted allocator id. If
> not, this would be a driver bug. A NULL pointer kernel OOPS is
> preferred to the WARN.
Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>
As a comment says in the code: /* NB! Only valid from an xdp_buff! */
Which is (currently) guarded by the return/exit in convert_to_xdp_frame().
This means that this code path can only be invoked while the driver is
still running under the RX NAPI process. Thus, there is no chance that
the allocator-id is gone (via calling xdp_rxq_info_unreg) for this code
path.
But I really hope we at somepoint can convert a MEM_TYPE_ZERO_COPY into
a form of xdp_frame, that can travel further into the redirect-core.
In which case, we likely need to handle the NULL case (but also need
other code to handle what to do with the memory backing the frame)
(I'm my vision here:)
I really dislike that the current Zero-Copy mode steal ALL packets,
when ZC is enabled on a RX-queue. This is not better than the existing
bypass solutions, which have ugly ways of re-injecting packet back into
the network stack. With the integration with XDP, we have the
flexibility of selecting frames, that we don't want to be "bypassed"
into AF_XDP, and want the kernel process these. (The most common
use-case is letting the kernel handle the arptable). IHMO this is what
will/would make AF_XDP superior to other bypass solutions.
> Suggested-by: Jesper Dangaard Brouer <brouer@redhat.com>
> Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
> ---
> net/core/xdp.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/net/core/xdp.c b/net/core/xdp.c
> index 6771f1855b96..9d1f22072d5d 100644
> --- a/net/core/xdp.c
> +++ b/net/core/xdp.c
> @@ -345,8 +345,7 @@ static void __xdp_return(void *data, struct xdp_mem_info *mem, bool napi_direct,
> rcu_read_lock();
> /* mem->id is valid, checked in xdp_rxq_info_reg_mem_model() */
> xa = rhashtable_lookup(mem_id_ht, &mem->id, mem_id_rht_params);
> - if (!WARN_ON_ONCE(!xa))
> - xa->zc_alloc->free(xa->zc_alloc, handle);
> + xa->zc_alloc->free(xa->zc_alloc, handle);
> rcu_read_unlock();
> default:
> /* Not possible, checked in xdp_rxq_info_reg_mem_model() */
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH bpf] Revert "xdp: add NULL pointer check in __xdp_return()"
2018-08-10 9:28 [PATCH bpf] Revert "xdp: add NULL pointer check in __xdp_return()" Björn Töpel
2018-08-10 10:18 ` Jesper Dangaard Brouer
@ 2018-08-10 14:10 ` Daniel Borkmann
2018-08-10 15:16 ` Björn Töpel
1 sibling, 1 reply; 8+ messages in thread
From: Daniel Borkmann @ 2018-08-10 14:10 UTC (permalink / raw)
To: Björn Töpel, ast, netdev, ap420073, brouer
Cc: Björn Töpel, magnus.karlsson, magnus.karlsson, kafai
On 08/10/2018 11:28 AM, Björn Töpel wrote:
> From: Björn Töpel <bjorn.topel@intel.com>
>
> This reverts commit 36e0f12bbfd3016f495904b35e41c5711707509f.
>
> The reverted commit adds a WARN to check against NULL entries in the
> mem_id_ht rhashtable. Any kernel path implementing the XDP (generic or
> driver) fast path is required to make a paired
> xdp_rxq_info_reg/xdp_rxq_info_unreg call for proper function. In
> addition, a driver using a different allocation scheme than the
> default MEM_TYPE_PAGE_SHARED is required to additionally call
> xdp_rxq_info_reg_mem_model.
>
> For MEM_TYPE_ZERO_COPY, an xdp_rxq_info_reg_mem_model call ensures
> that the mem_id_ht rhashtable has a properly inserted allocator id. If
> not, this would be a driver bug. A NULL pointer kernel OOPS is
> preferred to the WARN.
>
> Suggested-by: Jesper Dangaard Brouer <brouer@redhat.com>
> Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
Given the last bpf pr went out yesterday night, I've applied this to
bpf-next (worst case we can just route it via stable), thanks!
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH bpf] Revert "xdp: add NULL pointer check in __xdp_return()"
2018-08-10 10:18 ` Jesper Dangaard Brouer
@ 2018-08-10 15:15 ` Björn Töpel
2018-08-10 16:23 ` Jakub Kicinski
0 siblings, 1 reply; 8+ messages in thread
From: Björn Töpel @ 2018-08-10 15:15 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Alexei Starovoitov, Daniel Borkmann, Netdev, Taehee Yoo,
Björn Töpel, Karlsson, Magnus, Magnus Karlsson, kafai,
Jakub Kicinski
Den fre 10 aug. 2018 kl 12:18 skrev Jesper Dangaard Brouer <brouer@redhat.com>:
>
> On Fri, 10 Aug 2018 11:28:02 +0200
> Björn Töpel <bjorn.topel@gmail.com> wrote:
>
> > From: Björn Töpel <bjorn.topel@intel.com>
> >
> > This reverts commit 36e0f12bbfd3016f495904b35e41c5711707509f.
> >
> > The reverted commit adds a WARN to check against NULL entries in the
> > mem_id_ht rhashtable. Any kernel path implementing the XDP (generic or
> > driver) fast path is required to make a paired
> > xdp_rxq_info_reg/xdp_rxq_info_unreg call for proper function. In
> > addition, a driver using a different allocation scheme than the
> > default MEM_TYPE_PAGE_SHARED is required to additionally call
> > xdp_rxq_info_reg_mem_model.
> >
> > For MEM_TYPE_ZERO_COPY, an xdp_rxq_info_reg_mem_model call ensures
> > that the mem_id_ht rhashtable has a properly inserted allocator id. If
> > not, this would be a driver bug. A NULL pointer kernel OOPS is
> > preferred to the WARN.
>
> Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>
>
> As a comment says in the code: /* NB! Only valid from an xdp_buff! */
> Which is (currently) guarded by the return/exit in convert_to_xdp_frame().
>
> This means that this code path can only be invoked while the driver is
> still running under the RX NAPI process. Thus, there is no chance that
> the allocator-id is gone (via calling xdp_rxq_info_unreg) for this code
> path.
>
> But I really hope we at somepoint can convert a MEM_TYPE_ZERO_COPY into
> a form of xdp_frame, that can travel further into the redirect-core.
> In which case, we likely need to handle the NULL case (but also need
> other code to handle what to do with the memory backing the frame)
>
> (I'm my vision here:)
>
> I really dislike that the current Zero-Copy mode steal ALL packets,
> when ZC is enabled on a RX-queue. This is not better than the existing
> bypass solutions, which have ugly ways of re-injecting packet back into
> the network stack. With the integration with XDP, we have the
> flexibility of selecting frames, that we don't want to be "bypassed"
> into AF_XDP, and want the kernel process these. (The most common
> use-case is letting the kernel handle the arptable). IHMO this is what
> will/would make AF_XDP superior to other bypass solutions.
>
>
Thanks for putting your visions/ideas here! I agree with both of your
last sections, and this is what we're working towards. AF_XDP ZC has
to play nicer with XDP. The current (well, the soon-to-be-published
[1] ;-)) ZC scheme is just a first step, and should be seen as a
starting point so people can start playing using AF_XDP. Jakub also
mentioned these issues a couple of threads ago, so there are
definitely more people feeling the ZC allocator pains. Mid-term a
sophisticated/proper and generic (for inter-driver reuse) ZC allocator
is needed; Converting xdp_buffs to xdp_frames cheaply for multi-CPU
completion, and hopefully dito for the XDP_PASS/kernel stack path. But
let's start with something simple that works, and take it from there.
Björn
[1] WIP: https://github.com/bjoto/linux/tree/af-xdp-i40e-zc
> > Suggested-by: Jesper Dangaard Brouer <brouer@redhat.com>
> > Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
> > ---
> > net/core/xdp.c | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/net/core/xdp.c b/net/core/xdp.c
> > index 6771f1855b96..9d1f22072d5d 100644
> > --- a/net/core/xdp.c
> > +++ b/net/core/xdp.c
> > @@ -345,8 +345,7 @@ static void __xdp_return(void *data, struct xdp_mem_info *mem, bool napi_direct,
> > rcu_read_lock();
> > /* mem->id is valid, checked in xdp_rxq_info_reg_mem_model() */
> > xa = rhashtable_lookup(mem_id_ht, &mem->id, mem_id_rht_params);
> > - if (!WARN_ON_ONCE(!xa))
> > - xa->zc_alloc->free(xa->zc_alloc, handle);
> > + xa->zc_alloc->free(xa->zc_alloc, handle);
> > rcu_read_unlock();
> > default:
> > /* Not possible, checked in xdp_rxq_info_reg_mem_model() */
>
>
>
> --
> Best regards,
> Jesper Dangaard Brouer
> MSc.CS, Principal Kernel Engineer at Red Hat
> LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH bpf] Revert "xdp: add NULL pointer check in __xdp_return()"
2018-08-10 14:10 ` Daniel Borkmann
@ 2018-08-10 15:16 ` Björn Töpel
2018-08-10 16:26 ` Jakub Kicinski
0 siblings, 1 reply; 8+ messages in thread
From: Björn Töpel @ 2018-08-10 15:16 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Alexei Starovoitov, Netdev, Taehee Yoo, Jesper Dangaard Brouer,
Björn Töpel, Karlsson, Magnus, Magnus Karlsson, kafai
Den fre 10 aug. 2018 kl 16:10 skrev Daniel Borkmann <daniel@iogearbox.net>:
>
> On 08/10/2018 11:28 AM, Björn Töpel wrote:
> > From: Björn Töpel <bjorn.topel@intel.com>
> >
> > This reverts commit 36e0f12bbfd3016f495904b35e41c5711707509f.
> >
> > The reverted commit adds a WARN to check against NULL entries in the
> > mem_id_ht rhashtable. Any kernel path implementing the XDP (generic or
> > driver) fast path is required to make a paired
> > xdp_rxq_info_reg/xdp_rxq_info_unreg call for proper function. In
> > addition, a driver using a different allocation scheme than the
> > default MEM_TYPE_PAGE_SHARED is required to additionally call
> > xdp_rxq_info_reg_mem_model.
> >
> > For MEM_TYPE_ZERO_COPY, an xdp_rxq_info_reg_mem_model call ensures
> > that the mem_id_ht rhashtable has a properly inserted allocator id. If
> > not, this would be a driver bug. A NULL pointer kernel OOPS is
> > preferred to the WARN.
> >
> > Suggested-by: Jesper Dangaard Brouer <brouer@redhat.com>
> > Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
>
> Given the last bpf pr went out yesterday night, I've applied this to
> bpf-next (worst case we can just route it via stable), thanks!
Ah, right! Thanks!
bpf-next is OK. (Since this path is currently not used yet by any driver... :-()
Björn
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH bpf] Revert "xdp: add NULL pointer check in __xdp_return()"
2018-08-10 15:15 ` Björn Töpel
@ 2018-08-10 16:23 ` Jakub Kicinski
0 siblings, 0 replies; 8+ messages in thread
From: Jakub Kicinski @ 2018-08-10 16:23 UTC (permalink / raw)
To: Björn Töpel
Cc: Jesper Dangaard Brouer, Alexei Starovoitov, Daniel Borkmann,
Netdev, Taehee Yoo, Björn Töpel, Karlsson, Magnus,
Magnus Karlsson, kafai
On Fri, 10 Aug 2018 17:15:07 +0200, Björn Töpel wrote:
> Den fre 10 aug. 2018 kl 12:18 skrev Jesper Dangaard Brouer <brouer@redhat.com>:
> >
> > On Fri, 10 Aug 2018 11:28:02 +0200
> > Björn Töpel <bjorn.topel@gmail.com> wrote:
> >
> > > From: Björn Töpel <bjorn.topel@intel.com>
> > >
> > > This reverts commit 36e0f12bbfd3016f495904b35e41c5711707509f.
> > >
> > > The reverted commit adds a WARN to check against NULL entries in the
> > > mem_id_ht rhashtable. Any kernel path implementing the XDP (generic or
> > > driver) fast path is required to make a paired
> > > xdp_rxq_info_reg/xdp_rxq_info_unreg call for proper function. In
> > > addition, a driver using a different allocation scheme than the
> > > default MEM_TYPE_PAGE_SHARED is required to additionally call
> > > xdp_rxq_info_reg_mem_model.
> > >
> > > For MEM_TYPE_ZERO_COPY, an xdp_rxq_info_reg_mem_model call ensures
> > > that the mem_id_ht rhashtable has a properly inserted allocator id. If
> > > not, this would be a driver bug. A NULL pointer kernel OOPS is
> > > preferred to the WARN.
> >
> > Acked-by: Jesper Dangaard Brouer <brouer@redhat.com>
> >
> > As a comment says in the code: /* NB! Only valid from an xdp_buff! */
> > Which is (currently) guarded by the return/exit in convert_to_xdp_frame().
> >
> > This means that this code path can only be invoked while the driver is
> > still running under the RX NAPI process. Thus, there is no chance that
> > the allocator-id is gone (via calling xdp_rxq_info_unreg) for this code
> > path.
> >
> > But I really hope we at somepoint can convert a MEM_TYPE_ZERO_COPY into
> > a form of xdp_frame, that can travel further into the redirect-core.
> > In which case, we likely need to handle the NULL case (but also need
> > other code to handle what to do with the memory backing the frame)
> >
> > (I'm my vision here:)
> >
> > I really dislike that the current Zero-Copy mode steal ALL packets,
> > when ZC is enabled on a RX-queue. This is not better than the existing
> > bypass solutions, which have ugly ways of re-injecting packet back into
> > the network stack. With the integration with XDP, we have the
> > flexibility of selecting frames, that we don't want to be "bypassed"
> > into AF_XDP, and want the kernel process these. (The most common
> > use-case is letting the kernel handle the arptable). IHMO this is what
> > will/would make AF_XDP superior to other bypass solutions.
Perhaps I'm misunderstanding, but I don't think that's necessarily
true. AFAIU on XDP_PASS drivers should copy the frame into a skb and
pass it up the stack. Granted that's fairly slow but *semantically*
AF_XDP doesn't necessarily steal all the packets :)
> Thanks for putting your visions/ideas here! I agree with both of your
> last sections, and this is what we're working towards. AF_XDP ZC has
> to play nicer with XDP. The current (well, the soon-to-be-published
> [1] ;-)) ZC scheme is just a first step, and should be seen as a
> starting point so people can start playing using AF_XDP. Jakub also
> mentioned these issues a couple of threads ago, so there are
> definitely more people feeling the ZC allocator pains. Mid-term a
> sophisticated/proper and generic (for inter-driver reuse) ZC allocator
> is needed; Converting xdp_buffs to xdp_frames cheaply for multi-CPU
> completion, and hopefully dito for the XDP_PASS/kernel stack path. But
> let's start with something simple that works, and take it from there.
>
> Björn
>
> [1] WIP: https://github.com/bjoto/linux/tree/af-xdp-i40e-zc
Nice, looking forward to a refresh of i40e patches! :)
> > > Suggested-by: Jesper Dangaard Brouer <brouer@redhat.com>
> > > Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
> > > ---
> > > net/core/xdp.c | 3 +--
> > > 1 file changed, 1 insertion(+), 2 deletions(-)
> > >
> > > diff --git a/net/core/xdp.c b/net/core/xdp.c
> > > index 6771f1855b96..9d1f22072d5d 100644
> > > --- a/net/core/xdp.c
> > > +++ b/net/core/xdp.c
> > > @@ -345,8 +345,7 @@ static void __xdp_return(void *data, struct xdp_mem_info *mem, bool napi_direct,
> > > rcu_read_lock();
> > > /* mem->id is valid, checked in xdp_rxq_info_reg_mem_model() */
> > > xa = rhashtable_lookup(mem_id_ht, &mem->id, mem_id_rht_params);
> > > - if (!WARN_ON_ONCE(!xa))
> > > - xa->zc_alloc->free(xa->zc_alloc, handle);
> > > + xa->zc_alloc->free(xa->zc_alloc, handle);
> > > rcu_read_unlock();
> > > default:
> > > /* Not possible, checked in xdp_rxq_info_reg_mem_model() */
> >
> >
> >
> > --
> > Best regards,
> > Jesper Dangaard Brouer
> > MSc.CS, Principal Kernel Engineer at Red Hat
> > LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH bpf] Revert "xdp: add NULL pointer check in __xdp_return()"
2018-08-10 15:16 ` Björn Töpel
@ 2018-08-10 16:26 ` Jakub Kicinski
2018-08-14 12:02 ` Björn Töpel
0 siblings, 1 reply; 8+ messages in thread
From: Jakub Kicinski @ 2018-08-10 16:26 UTC (permalink / raw)
To: Björn Töpel
Cc: Daniel Borkmann, Alexei Starovoitov, Netdev, Taehee Yoo,
Jesper Dangaard Brouer, Björn Töpel, Karlsson, Magnus,
Magnus Karlsson, kafai
On Fri, 10 Aug 2018 17:16:45 +0200, Björn Töpel wrote:
> Den fre 10 aug. 2018 kl 16:10 skrev Daniel Borkmann <daniel@iogearbox.net>:
> >
> > On 08/10/2018 11:28 AM, Björn Töpel wrote:
> > > From: Björn Töpel <bjorn.topel@intel.com>
> > >
> > > This reverts commit 36e0f12bbfd3016f495904b35e41c5711707509f.
> > >
> > > The reverted commit adds a WARN to check against NULL entries in the
> > > mem_id_ht rhashtable. Any kernel path implementing the XDP (generic or
> > > driver) fast path is required to make a paired
> > > xdp_rxq_info_reg/xdp_rxq_info_unreg call for proper function. In
> > > addition, a driver using a different allocation scheme than the
> > > default MEM_TYPE_PAGE_SHARED is required to additionally call
> > > xdp_rxq_info_reg_mem_model.
> > >
> > > For MEM_TYPE_ZERO_COPY, an xdp_rxq_info_reg_mem_model call ensures
> > > that the mem_id_ht rhashtable has a properly inserted allocator id. If
> > > not, this would be a driver bug. A NULL pointer kernel OOPS is
> > > preferred to the WARN.
> > >
> > > Suggested-by: Jesper Dangaard Brouer <brouer@redhat.com>
> > > Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
> >
> > Given the last bpf pr went out yesterday night, I've applied this to
> > bpf-next (worst case we can just route it via stable), thanks!
>
> Ah, right! Thanks!
>
> bpf-next is OK. (Since this path is currently not used yet by any driver... :-()
Wasn't this dead code, anyway? The frame return path is for redirects,
and one can't convert_to_xdp_frame presently?
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH bpf] Revert "xdp: add NULL pointer check in __xdp_return()"
2018-08-10 16:26 ` Jakub Kicinski
@ 2018-08-14 12:02 ` Björn Töpel
0 siblings, 0 replies; 8+ messages in thread
From: Björn Töpel @ 2018-08-14 12:02 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Daniel Borkmann, Alexei Starovoitov, Netdev, Taehee Yoo,
Jesper Dangaard Brouer, Björn Töpel, Karlsson, Magnus,
Magnus Karlsson, kafai
Den fre 10 aug. 2018 kl 18:26 skrev Jakub Kicinski
<jakub.kicinski@netronome.com>:
>
> On Fri, 10 Aug 2018 17:16:45 +0200, Björn Töpel wrote:
> > Den fre 10 aug. 2018 kl 16:10 skrev Daniel Borkmann <daniel@iogearbox.net>:
> > >
> > > On 08/10/2018 11:28 AM, Björn Töpel wrote:
> > > > From: Björn Töpel <bjorn.topel@intel.com>
> > > >
> > > > This reverts commit 36e0f12bbfd3016f495904b35e41c5711707509f.
> > > >
> > > > The reverted commit adds a WARN to check against NULL entries in the
> > > > mem_id_ht rhashtable. Any kernel path implementing the XDP (generic or
> > > > driver) fast path is required to make a paired
> > > > xdp_rxq_info_reg/xdp_rxq_info_unreg call for proper function. In
> > > > addition, a driver using a different allocation scheme than the
> > > > default MEM_TYPE_PAGE_SHARED is required to additionally call
> > > > xdp_rxq_info_reg_mem_model.
> > > >
> > > > For MEM_TYPE_ZERO_COPY, an xdp_rxq_info_reg_mem_model call ensures
> > > > that the mem_id_ht rhashtable has a properly inserted allocator id. If
> > > > not, this would be a driver bug. A NULL pointer kernel OOPS is
> > > > preferred to the WARN.
> > > >
> > > > Suggested-by: Jesper Dangaard Brouer <brouer@redhat.com>
> > > > Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
> > >
> > > Given the last bpf pr went out yesterday night, I've applied this to
> > > bpf-next (worst case we can just route it via stable), thanks!
> >
> > Ah, right! Thanks!
> >
> > bpf-next is OK. (Since this path is currently not used yet by any driver... :-()
>
> Wasn't this dead code, anyway? The frame return path is for redirects,
> and one can't convert_to_xdp_frame presently?
Indeed, dead it is. Hmm, I'll remove it as part of the i40e zc submission.
Björn
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-08-14 14:49 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-10 9:28 [PATCH bpf] Revert "xdp: add NULL pointer check in __xdp_return()" Björn Töpel
2018-08-10 10:18 ` Jesper Dangaard Brouer
2018-08-10 15:15 ` Björn Töpel
2018-08-10 16:23 ` Jakub Kicinski
2018-08-10 14:10 ` Daniel Borkmann
2018-08-10 15:16 ` Björn Töpel
2018-08-10 16:26 ` Jakub Kicinski
2018-08-14 12:02 ` Björn Töpel
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.