Den lör 21 juli 2018 kl 14:58 skrev Taehee Yoo <ap420...@gmail.com>:
>
> 2018-07-21 2:18 GMT+09:00 Martin KaFai Lau <ka...@fb.com>:
> > On Sat, Jul 21, 2018 at 01:04:45AM +0900, Taehee Yoo wrote:
> >> rhashtable_lookup() can return NULL. so that NULL pointer
> >> check routine should be added.
> >>
> >> Fixes: 02b55e5657c3 ("xdp: add MEM_TYPE_ZERO_COPY")
> >> Signed-off-by: Taehee Yoo <ap420...@gmail.com>
> >> ---
> >>  net/core/xdp.c | 3 ++-
> >>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/net/core/xdp.c b/net/core/xdp.c
> >> index 9d1f220..1c12bc7 100644
> >> --- a/net/core/xdp.c
> >> +++ b/net/core/xdp.c
> >> @@ -345,7 +345,8 @@ 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);
> >> -             xa->zc_alloc->free(xa->zc_alloc, handle);
> >> +             if (xa)
> >> +                     xa->zc_alloc->free(xa->zc_alloc, handle);
> > hmm...It is not clear to me the "!xa" case don't have to be handled?
>
> Thank you for reviewing!
>
> Returning NULL pointer is bug case such as calling after use
> xdp_rxq_info_unreg().
> so that, I think it can't handle at that moment.
> we can make __xdp_return to add WARN_ON_ONCE() or
> add return error code to driver.
> But I'm not sure if these is useful information.
>
> I might have misunderstood scenario of MEM_TYPE_ZERO_COPY
> because there is no use case of MEM_TYPE_ZERO_COPY yet.
>

Taehee, again, sorry for the slow response and thanks for patch!

If xa is NULL, the driver has a buggy/broken implementation. What
would be a proper way of dealing with this? BUG?


Björn

> Thanks!
>
> >
> >>               rcu_read_unlock();
> >>       default:
> >>               /* Not possible, checked in xdp_rxq_info_reg_mem_model() */
> >> --
> >> 2.9.3
> >>

Reply via email to