[dpdk-dev] [PATCH] vhost/crypto: fix missing head correction

[dpdk-dev] [PATCH] vhost/crypto: fix missing head correction

[Fan Zhang]

Fixes: 473387aaae4a ("vhost/crypto: move to safe GPA translation API")

This patch fixes the missing head descriptor correction for
indirect descriptors.

Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
---
 lib/librte_vhost/vhost_crypto.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/librte_vhost/vhost_crypto.c b/lib/librte_vhost/vhost_crypto.c
index c38eb3bb5..81fab7e94 100644
--- a/lib/librte_vhost/vhost_crypto.c
+++ b/lib/librte_vhost/vhost_crypto.c
@@ -919,6 +919,7 @@ vhost_crypto_process_one_req(struct vhost_crypto *vcrypto,
 		if (unlikely(!desc || dlen != head->len))
 			return -1;
 		desc_idx = 0;
+		head = desc;
 	} else {
 		desc = head;
 	}
-- 
2.13.6

Re: [dpdk-dev] [PATCH] vhost/crypto: fix missing head correction

[Maxime Coquelin]

Hi Fan,

On 04/27/2018 03:52 PM, Fan Zhang wrote:
> Fixes: 473387aaae4a ("vhost/crypto: move to safe GPA translation API")
> 
> This patch fixes the missing head descriptor correction for
> indirect descriptors.
> 
> Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
> ---
>   lib/librte_vhost/vhost_crypto.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/lib/librte_vhost/vhost_crypto.c b/lib/librte_vhost/vhost_crypto.c
> index c38eb3bb5..81fab7e94 100644
> --- a/lib/librte_vhost/vhost_crypto.c
> +++ b/lib/librte_vhost/vhost_crypto.c
> @@ -919,6 +919,7 @@ vhost_crypto_process_one_req(struct vhost_crypto *vcrypto,
>   		if (unlikely(!desc || dlen != head->len))
>   			return -1;
>   		desc_idx = 0;
> +		head = desc;

Thanks for fixing this, and sorry for having introduced the regression.

BTW, my patch is half-complete. Indeed, it ensures no out-of-bound
accesses will be done, but it does not handle legit buffers that are
contiguous in guest physical address space, but discontiguous in the
host VA space. Such case can for example happen when having buffers
larger than the huge page size.

I have implemented the handling of such buffers in virtio-net.c, for 
example:

fb3815cc6 vhost: handle virtually non-contiguous buffers in Rx-mrg
6727f5a73 vhost: handle virtually non-contiguous buffers in Rx
91b7b4080 vhost: handle virtually non-contiguous buffers in Tx
d0c24508e vhost: add support for non-contiguous indirect descs tables

Can you try to work on such handling for next -rc release?


For this patch:
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks,
Maxime

>   	} else {
>   		desc = head;
>   	}
> 

Re: [dpdk-dev] [PATCH] vhost/crypto: fix missing head correction

[Zhang, Roy Fan]

Hi Maxime,

No problem. I will work on it.

Regards,
Fan

> -----Original Message-----
> From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com]
> Sent: Friday, April 27, 2018 3:19 PM
> To: Zhang, Roy Fan <roy.fan.zhang@intel.com>; dev@dpdk.org
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>
> Subject: Re: [PATCH] vhost/crypto: fix missing head correction
> 
> Hi Fan,
> 
> On 04/27/2018 03:52 PM, Fan Zhang wrote:
> > Fixes: 473387aaae4a ("vhost/crypto: move to safe GPA translation API")
> >
> > This patch fixes the missing head descriptor correction for indirect
> > descriptors.
> >
> > Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
> > ---
> >   lib/librte_vhost/vhost_crypto.c | 1 +
> >   1 file changed, 1 insertion(+)
> >
> > diff --git a/lib/librte_vhost/vhost_crypto.c
> > b/lib/librte_vhost/vhost_crypto.c index c38eb3bb5..81fab7e94 100644
> > --- a/lib/librte_vhost/vhost_crypto.c
> > +++ b/lib/librte_vhost/vhost_crypto.c
> > @@ -919,6 +919,7 @@ vhost_crypto_process_one_req(struct
> vhost_crypto *vcrypto,
> >   		if (unlikely(!desc || dlen != head->len))
> >   			return -1;
> >   		desc_idx = 0;
> > +		head = desc;
> 
> Thanks for fixing this, and sorry for having introduced the regression.
> 
> BTW, my patch is half-complete. Indeed, it ensures no out-of-bound
> accesses will be done, but it does not handle legit buffers that are contiguous
> in guest physical address space, but discontiguous in the host VA space. Such
> case can for example happen when having buffers larger than the huge page
> size.
> 
> I have implemented the handling of such buffers in virtio-net.c, for
> example:
> 
> fb3815cc6 vhost: handle virtually non-contiguous buffers in Rx-mrg
> 6727f5a73 vhost: handle virtually non-contiguous buffers in Rx
> 91b7b4080 vhost: handle virtually non-contiguous buffers in Tx d0c24508e
> vhost: add support for non-contiguous indirect descs tables
> 
> Can you try to work on such handling for next -rc release?
> 
> 
> For this patch:
> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> 
> Thanks,
> Maxime
> 
> >   	} else {
> >   		desc = head;
> >   	}
> >

Re: [dpdk-dev] [PATCH] vhost/crypto: fix missing head correction

[Thomas Monjalon]

27/04/2018 16:19, Maxime Coquelin:
> Hi Fan,
> 
> On 04/27/2018 03:52 PM, Fan Zhang wrote:
> > Fixes: 473387aaae4a ("vhost/crypto: move to safe GPA translation API")
> > 
> > This patch fixes the missing head descriptor correction for
> > indirect descriptors.
> > 
> > Signed-off-by: Fan Zhang <roy.fan.zhang@intel.com>
> > ---
> >   lib/librte_vhost/vhost_crypto.c | 1 +
> >   1 file changed, 1 insertion(+)
> > 
> > diff --git a/lib/librte_vhost/vhost_crypto.c b/lib/librte_vhost/vhost_crypto.c
> > index c38eb3bb5..81fab7e94 100644
> > --- a/lib/librte_vhost/vhost_crypto.c
> > +++ b/lib/librte_vhost/vhost_crypto.c
> > @@ -919,6 +919,7 @@ vhost_crypto_process_one_req(struct vhost_crypto *vcrypto,
> >   		if (unlikely(!desc || dlen != head->len))
> >   			return -1;
> >   		desc_idx = 0;
> > +		head = desc;
> 
> Thanks for fixing this, and sorry for having introduced the regression.
> 
> BTW, my patch is half-complete. Indeed, it ensures no out-of-bound
> accesses will be done, but it does not handle legit buffers that are
> contiguous in guest physical address space, but discontiguous in the
> host VA space. Such case can for example happen when having buffers
> larger than the huge page size.
> 
> I have implemented the handling of such buffers in virtio-net.c, for 
> example:
> 
> fb3815cc6 vhost: handle virtually non-contiguous buffers in Rx-mrg
> 6727f5a73 vhost: handle virtually non-contiguous buffers in Rx
> 91b7b4080 vhost: handle virtually non-contiguous buffers in Tx
> d0c24508e vhost: add support for non-contiguous indirect descs tables
> 
> Can you try to work on such handling for next -rc release?
> 
> 
> For this patch:
> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Applied as hot patch for RC1, thanks