Re: [dpdk-dev] [PATCH v11 4/4] kni: add IOVA=VA support in kernel module

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 10/21/2019 9:03 AM, vattunuru@marvell.com wrote:
> From: Vamsi Attunuru <vattunuru@marvell.com>
> 
> Patch adds support for kernel module to work in
> IOVA = VA mode by providing address translation
> routines to convert IOVA aka user space VA to
> kernel virtual addresses.
> 
> Signed-off-by: Vamsi Attunuru <vattunuru@marvell.com>
> Signed-off-by: Kiran Kumar K <kirankumark@marvell.com>

<...>

> +static inline phys_addr_t iova_to_phys(struct task_struct *tsk,
> +				       unsigned long iova)
> +{
> +	unsigned int flags = FOLL_TOUCH;
> +	phys_addr_t offset, phys_addr;
> +	struct page *page = NULL;
> +	int ret;
> +
> +	offset = iova & (PAGE_SIZE - 1);
> +
> +	/* Read one page struct info */
> +	ret =  get_user_pages_remote(tsk, tsk->mm, iova, 1,
> +				     flags, &page, 0, 0);
> +	if (ret < 0)
> +		return 0;
> +
> +	phys_addr = page_to_phys(page) | offset;
> +	put_page(page);
> +
> +	return phys_addr;
> +}
> +
> +static inline void *iova_to_kva(struct task_struct *tsk, unsigned long iova)
> +{
> +	return phys_to_virt(iova_to_phys(tsk, iova));
> +}

Do you have any measurement for the performance affect of this change?

> +
>  void kni_net_release_fifo_phy(struct kni_dev *kni);
>  void kni_net_rx(struct kni_dev *kni);
>  void kni_net_init(struct net_device *dev);
> diff --git a/kernel/linux/kni/kni_misc.c b/kernel/linux/kni/kni_misc.c
> index 2b75502..7af7ab4 100644
> --- a/kernel/linux/kni/kni_misc.c
> +++ b/kernel/linux/kni/kni_misc.c
> @@ -348,15 +348,36 @@ kni_ioctl_create(struct net *net, uint32_t ioctl_num,
>  	strncpy(kni->name, dev_info.name, RTE_KNI_NAMESIZE);
>  
>  	/* Translate user space info into kernel space info */
> -	kni->tx_q = phys_to_virt(dev_info.tx_phys);
> -	kni->rx_q = phys_to_virt(dev_info.rx_phys);
> -	kni->alloc_q = phys_to_virt(dev_info.alloc_phys);
> -	kni->free_q = phys_to_virt(dev_info.free_phys);
> -
> -	kni->req_q = phys_to_virt(dev_info.req_phys);
> -	kni->resp_q = phys_to_virt(dev_info.resp_phys);
> -	kni->sync_va = dev_info.sync_va;
> -	kni->sync_kva = phys_to_virt(dev_info.sync_phys);
> +	if (dev_info.iova_mode) {
> +#ifdef HAVE_IOVA_TO_KVA_MAPPING_SUPPORT

Do you think is a runtime check required, for the case code has been compiled on
a box that has newer kernel but run in a box with older kernel?
Not sure about it myself either...

<...>

> @@ -62,6 +76,24 @@ kva2data_kva(struct rte_kni_mbuf *m)
>  	return phys_to_virt(m->buf_physaddr + m->data_off);
>  }
>  
> +static inline void *
> +get_kva(struct kni_dev *kni, void *pa)
> +{
> +	if (kni->iova_mode == 1)

This check done multiple times per packet, I wonder if this can be prevented,
again even with the original mode do you have any numbers related to the
performance affect of this check?

Thanks,
ferruh