On 6/25/2019 4:04 PM, Yangchao Zhou wrote:
> va2pa depends on the physical address and virtual address offset of
> current mbuf. It may get the wrong physical address of next mbuf which
> allocated in another hugepage segment.
> 
> In rte_mempool_populate_default(), trying to allocate whole block of
> contiguous memory could be failed. Then, it would reserve memory in
> several memzones that have different physical address and virtual address
> offsets. The rte_mempool_populate_default() is used by
> rte_pktmbuf_pool_create().
> 
> Fixes: 8451269e6d7b ("kni: remove continuous memory restriction")
> 
> Signed-off-by: Yangchao Zhou <zhouya...@gmail.com>

Overall looks good to me, not from this patch but can you please check below
comment too.
Also there is a comment from Junxiao, lets clear it before the ack.

<...>

> @@ -396,7 +401,7 @@ kni_net_rx_lo_fifo(struct kni_dev *kni)
>       uint32_t ret;
>       uint32_t len;
>       uint32_t i, num, num_rq, num_tq, num_aq, num_fq;
> -     struct rte_kni_mbuf *kva;
> +     struct rte_kni_mbuf *kva, *next_kva;
>       void *data_kva;
>       struct rte_kni_mbuf *alloc_kva;
>       void *alloc_data_kva;
> @@ -439,6 +444,13 @@ kni_net_rx_lo_fifo(struct kni_dev *kni)
>                       data_kva = kva2data_kva(kva);
>                       kni->va[i] = pa2va(kni->pa[i], kva);
>  
> +                     while (kva->next) {
> +                             next_kva = pa2kva(kva->next);
> +                             /* Convert physical address to virtual address 
> */
> +                             kva->next = pa2va(kva->next, next_kva);
> +                             kva = next_kva;
> +                     }

Not done in this patch, but in 'kni_net_rx_lo_fifo()' the len calculated as
'len = kva->pkt_len;'

But while copying 'data' to 'alloc_data' the segmentation is not taken into
account and 'len' is used:
memcpy(alloc_data_kva, data_kva, len);

This may lead overflow 'alloc_data_kva' for some 'pkt_len' values.

Reply via email to