All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net] tun: fix return value when the number of iovs exceeds MAX_SKB_FRAGS
@ 2020-12-24 11:48 wangyunjian
  2020-12-24 15:56   ` Willem de Bruijn
  2020-12-27  9:39   ` Michael S. Tsirkin
  0 siblings, 2 replies; 7+ messages in thread
From: wangyunjian @ 2020-12-24 11:48 UTC (permalink / raw)
  To: netdev, mst, jasowang, willemdebruijn.kernel
  Cc: virtualization, jerry.lilijun, chenchanghu, xudingke,
	brian.huangbin, Yunjian Wang

From: Yunjian Wang <wangyunjian@huawei.com>

Currently the tun_napi_alloc_frags() function returns -ENOMEM when the
number of iovs exceeds MAX_SKB_FRAGS + 1. However this is inappropriate,
we should use -EMSGSIZE instead of -ENOMEM.

Fixes: 90e33d459407 ("tun: enable napi_gro_frags() for TUN/TAP driver")
Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>
---
 drivers/net/tun.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 2dc1988a8973..15c6dd7fb04c 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -1365,7 +1365,7 @@ static struct sk_buff *tun_napi_alloc_frags(struct tun_file *tfile,
 	int i;
 
 	if (it->nr_segs > MAX_SKB_FRAGS + 1)
-		return ERR_PTR(-ENOMEM);
+		return ERR_PTR(-EMSGSIZE);
 
 	local_bh_disable();
 	skb = napi_get_frags(&tfile->napi);
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH net] tun: fix return value when the number of iovs exceeds MAX_SKB_FRAGS
  2020-12-24 11:48 [PATCH net] tun: fix return value when the number of iovs exceeds MAX_SKB_FRAGS wangyunjian
@ 2020-12-24 15:56   ` Willem de Bruijn
  2020-12-27  9:39   ` Michael S. Tsirkin
  1 sibling, 0 replies; 7+ messages in thread
From: Willem de Bruijn @ 2020-12-24 15:56 UTC (permalink / raw)
  To: wangyunjian
  Cc: Network Development, Michael S. Tsirkin, Jason Wang,
	Willem de Bruijn, virtualization, Lilijun (Jerry),
	chenchanghu, xudingke, huangbin (J)

On Thu, Dec 24, 2020 at 6:51 AM wangyunjian <wangyunjian@huawei.com> wrote:
>
> From: Yunjian Wang <wangyunjian@huawei.com>
>
> Currently the tun_napi_alloc_frags() function returns -ENOMEM when the
> number of iovs exceeds MAX_SKB_FRAGS + 1. However this is inappropriate,
> we should use -EMSGSIZE instead of -ENOMEM.
>
> Fixes: 90e33d459407 ("tun: enable napi_gro_frags() for TUN/TAP driver")
> Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>

Acked-by: Willem de Bruijn <willemb@google.com>

It might be good to explain why the distinction matters: one denotes a
transient failure that the caller (specifically vhost_net) can retry,
the other a persistent failure due to bad packet geometry that should
be dropped.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net] tun: fix return value when the number of iovs exceeds MAX_SKB_FRAGS
@ 2020-12-24 15:56   ` Willem de Bruijn
  0 siblings, 0 replies; 7+ messages in thread
From: Willem de Bruijn @ 2020-12-24 15:56 UTC (permalink / raw)
  To: wangyunjian
  Cc: Willem de Bruijn, Michael S. Tsirkin, Network Development,
	Lilijun (Jerry), virtualization, xudingke, huangbin (J),
	chenchanghu

On Thu, Dec 24, 2020 at 6:51 AM wangyunjian <wangyunjian@huawei.com> wrote:
>
> From: Yunjian Wang <wangyunjian@huawei.com>
>
> Currently the tun_napi_alloc_frags() function returns -ENOMEM when the
> number of iovs exceeds MAX_SKB_FRAGS + 1. However this is inappropriate,
> we should use -EMSGSIZE instead of -ENOMEM.
>
> Fixes: 90e33d459407 ("tun: enable napi_gro_frags() for TUN/TAP driver")
> Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>

Acked-by: Willem de Bruijn <willemb@google.com>

It might be good to explain why the distinction matters: one denotes a
transient failure that the caller (specifically vhost_net) can retry,
the other a persistent failure due to bad packet geometry that should
be dropped.
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net] tun: fix return value when the number of iovs exceeds MAX_SKB_FRAGS
  2020-12-24 11:48 [PATCH net] tun: fix return value when the number of iovs exceeds MAX_SKB_FRAGS wangyunjian
@ 2020-12-27  9:39   ` Michael S. Tsirkin
  2020-12-27  9:39   ` Michael S. Tsirkin
  1 sibling, 0 replies; 7+ messages in thread
From: Michael S. Tsirkin @ 2020-12-27  9:39 UTC (permalink / raw)
  To: wangyunjian
  Cc: netdev, jasowang, willemdebruijn.kernel, virtualization,
	jerry.lilijun, chenchanghu, xudingke, brian.huangbin

On Thu, Dec 24, 2020 at 07:48:53PM +0800, wangyunjian wrote:
> From: Yunjian Wang <wangyunjian@huawei.com>
> 
> Currently the tun_napi_alloc_frags() function returns -ENOMEM when the
> number of iovs exceeds MAX_SKB_FRAGS + 1. However this is inappropriate,
> we should use -EMSGSIZE instead of -ENOMEM.
> 
> Fixes: 90e33d459407 ("tun: enable napi_gro_frags() for TUN/TAP driver")
> Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>

Acked-by: Michael S. Tsirkin <mst@redhat.com>

> ---
>  drivers/net/tun.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index 2dc1988a8973..15c6dd7fb04c 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -1365,7 +1365,7 @@ static struct sk_buff *tun_napi_alloc_frags(struct tun_file *tfile,
>  	int i;
>  
>  	if (it->nr_segs > MAX_SKB_FRAGS + 1)
> -		return ERR_PTR(-ENOMEM);
> +		return ERR_PTR(-EMSGSIZE);
>  
>  	local_bh_disable();
>  	skb = napi_get_frags(&tfile->napi);
> -- 
> 2.23.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net] tun: fix return value when the number of iovs exceeds MAX_SKB_FRAGS
@ 2020-12-27  9:39   ` Michael S. Tsirkin
  0 siblings, 0 replies; 7+ messages in thread
From: Michael S. Tsirkin @ 2020-12-27  9:39 UTC (permalink / raw)
  To: wangyunjian
  Cc: willemdebruijn.kernel, netdev, jerry.lilijun, virtualization,
	xudingke, brian.huangbin, chenchanghu

On Thu, Dec 24, 2020 at 07:48:53PM +0800, wangyunjian wrote:
> From: Yunjian Wang <wangyunjian@huawei.com>
> 
> Currently the tun_napi_alloc_frags() function returns -ENOMEM when the
> number of iovs exceeds MAX_SKB_FRAGS + 1. However this is inappropriate,
> we should use -EMSGSIZE instead of -ENOMEM.
> 
> Fixes: 90e33d459407 ("tun: enable napi_gro_frags() for TUN/TAP driver")
> Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>

Acked-by: Michael S. Tsirkin <mst@redhat.com>

> ---
>  drivers/net/tun.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index 2dc1988a8973..15c6dd7fb04c 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -1365,7 +1365,7 @@ static struct sk_buff *tun_napi_alloc_frags(struct tun_file *tfile,
>  	int i;
>  
>  	if (it->nr_segs > MAX_SKB_FRAGS + 1)
> -		return ERR_PTR(-ENOMEM);
> +		return ERR_PTR(-EMSGSIZE);
>  
>  	local_bh_disable();
>  	skb = napi_get_frags(&tfile->napi);
> -- 
> 2.23.0

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net] tun: fix return value when the number of iovs exceeds MAX_SKB_FRAGS
  2020-12-24 15:56   ` Willem de Bruijn
@ 2020-12-27 11:18     ` Michael S. Tsirkin
  -1 siblings, 0 replies; 7+ messages in thread
From: Michael S. Tsirkin @ 2020-12-27 11:18 UTC (permalink / raw)
  To: Willem de Bruijn
  Cc: wangyunjian, Network Development, Jason Wang, virtualization,
	Lilijun (Jerry), chenchanghu, xudingke, huangbin (J)

On Thu, Dec 24, 2020 at 10:56:16AM -0500, Willem de Bruijn wrote:
> On Thu, Dec 24, 2020 at 6:51 AM wangyunjian <wangyunjian@huawei.com> wrote:
> >
> > From: Yunjian Wang <wangyunjian@huawei.com>
> >
> > Currently the tun_napi_alloc_frags() function returns -ENOMEM when the
> > number of iovs exceeds MAX_SKB_FRAGS + 1. However this is inappropriate,
> > we should use -EMSGSIZE instead of -ENOMEM.
> >
> > Fixes: 90e33d459407 ("tun: enable napi_gro_frags() for TUN/TAP driver")
> > Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>
> 
> Acked-by: Willem de Bruijn <willemb@google.com>
> 
> It might be good to explain why the distinction matters: one denotes a
> transient failure that the caller (specifically vhost_net) can retry,
> the other a persistent failure due to bad packet geometry that should
> be dropped.

It would be good to have a definition of what constitutes a transient
failure. At the moment there's a proposed patch to vhost that
tests for 
	err == -EAGAIN || err == -ENOMEM || err == -ENOBUFS
and we'll likely add EIO to that.

Alternatively, I'm inclined to say any invalid input should
just return EINVAL except maybe for memory access errors
which traditionally are EFAULT.
Then vhost can handle any failure except EINVAL and EFAULT
as transient.

-- 
MST


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net] tun: fix return value when the number of iovs exceeds MAX_SKB_FRAGS
@ 2020-12-27 11:18     ` Michael S. Tsirkin
  0 siblings, 0 replies; 7+ messages in thread
From: Michael S. Tsirkin @ 2020-12-27 11:18 UTC (permalink / raw)
  To: Willem de Bruijn
  Cc: Network Development, wangyunjian, Lilijun (Jerry),
	virtualization, xudingke, huangbin (J),
	chenchanghu

On Thu, Dec 24, 2020 at 10:56:16AM -0500, Willem de Bruijn wrote:
> On Thu, Dec 24, 2020 at 6:51 AM wangyunjian <wangyunjian@huawei.com> wrote:
> >
> > From: Yunjian Wang <wangyunjian@huawei.com>
> >
> > Currently the tun_napi_alloc_frags() function returns -ENOMEM when the
> > number of iovs exceeds MAX_SKB_FRAGS + 1. However this is inappropriate,
> > we should use -EMSGSIZE instead of -ENOMEM.
> >
> > Fixes: 90e33d459407 ("tun: enable napi_gro_frags() for TUN/TAP driver")
> > Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>
> 
> Acked-by: Willem de Bruijn <willemb@google.com>
> 
> It might be good to explain why the distinction matters: one denotes a
> transient failure that the caller (specifically vhost_net) can retry,
> the other a persistent failure due to bad packet geometry that should
> be dropped.

It would be good to have a definition of what constitutes a transient
failure. At the moment there's a proposed patch to vhost that
tests for 
	err == -EAGAIN || err == -ENOMEM || err == -ENOBUFS
and we'll likely add EIO to that.

Alternatively, I'm inclined to say any invalid input should
just return EINVAL except maybe for memory access errors
which traditionally are EFAULT.
Then vhost can handle any failure except EINVAL and EFAULT
as transient.

-- 
MST

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2020-12-27 11:20 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-24 11:48 [PATCH net] tun: fix return value when the number of iovs exceeds MAX_SKB_FRAGS wangyunjian
2020-12-24 15:56 ` Willem de Bruijn
2020-12-24 15:56   ` Willem de Bruijn
2020-12-27 11:18   ` Michael S. Tsirkin
2020-12-27 11:18     ` Michael S. Tsirkin
2020-12-27  9:39 ` Michael S. Tsirkin
2020-12-27  9:39   ` Michael S. Tsirkin

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.