This commit allows vsock implementations to return errors
to the socket layer other than -ENOMEM. One immediate effect
of this is that upon the sk_sndbuf threshold being reached -EAGAIN
will be returned and userspace may throttle appropriately.
Resultingly, a known issue with uperf is resolved[1].
Additionally, to preserve legacy behavior for non-virtio
implementations, hyperv/vmci force errors to be -ENOMEM so that behavior
is unchanged.
[1]: https://gitlab.com/vsock/vsock/-/issues/1
Signed-off-by: Bobby Eshleman <[email protected]>
---
include/linux/virtio_vsock.h | 3 +++
net/vmw_vsock/af_vsock.c | 3 ++-
net/vmw_vsock/hyperv_transport.c | 2 +-
net/vmw_vsock/virtio_transport_common.c | 3 ---
net/vmw_vsock/vmci_transport.c | 9 ++++++++-
5 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
index 17ed01466875..9a37eddbb87a 100644
--- a/include/linux/virtio_vsock.h
+++ b/include/linux/virtio_vsock.h
@@ -8,6 +8,9 @@
#include <net/sock.h>
#include <net/af_vsock.h>
+/* Threshold for detecting small packets to copy */
+#define GOOD_COPY_LEN 128
+
enum virtio_vsock_metadata_flags {
VIRTIO_VSOCK_METADATA_FLAGS_REPLY = BIT(0),
VIRTIO_VSOCK_METADATA_FLAGS_TAP_DELIVERED = BIT(1),
diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
index e348b2d09eac..1893f8aafa48 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -1844,8 +1844,9 @@ static int vsock_connectible_sendmsg(struct socket *sock, struct msghdr *msg,
written = transport->stream_enqueue(vsk,
msg, len - total_written);
}
+
if (written < 0) {
- err = -ENOMEM;
+ err = written;
goto out_err;
}
diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
index fd98229e3db3..e99aea571f6f 100644
--- a/net/vmw_vsock/hyperv_transport.c
+++ b/net/vmw_vsock/hyperv_transport.c
@@ -687,7 +687,7 @@ static ssize_t hvs_stream_enqueue(struct vsock_sock *vsk, struct msghdr *msg,
if (bytes_written)
ret = bytes_written;
kfree(send_buf);
- return ret;
+ return ret < 0 ? -ENOMEM : ret;
}
static s64 hvs_stream_has_data(struct vsock_sock *vsk)
diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index 920578597bb9..d5780599fe93 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -23,9 +23,6 @@
/* How long to wait for graceful shutdown of a connection */
#define VSOCK_CLOSE_TIMEOUT (8 * HZ)
-/* Threshold for detecting small packets to copy */
-#define GOOD_COPY_LEN 128
-
static const struct virtio_transport *
virtio_transport_get_ops(struct vsock_sock *vsk)
{
diff --git a/net/vmw_vsock/vmci_transport.c b/net/vmw_vsock/vmci_transport.c
index b14f0ed7427b..c927a90dc859 100644
--- a/net/vmw_vsock/vmci_transport.c
+++ b/net/vmw_vsock/vmci_transport.c
@@ -1838,7 +1838,14 @@ static ssize_t vmci_transport_stream_enqueue(
struct msghdr *msg,
size_t len)
{
- return vmci_qpair_enquev(vmci_trans(vsk)->qpair, msg, len, 0);
+ int err;
+
+ err = vmci_qpair_enquev(vmci_trans(vsk)->qpair, msg, len, 0);
+
+ if (err < 0)
+ err = -ENOMEM;
+
+ return err;
}
static s64 vmci_transport_stream_has_data(struct vsock_sock *vsk)
--
2.35.1
Hi Bobby,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on mst-vhost/linux-next]
[also build test WARNING on linus/master v6.0-rc1 next-20220815]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Bobby-Eshleman/virtio-vsock-introduce-dgrams-sk_buff-and-qdisc/20220816-015812
base: https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git linux-next
config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20220816/[email protected]/config)
compiler: m68k-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/68c9c8216a573cdfe2170cad677854e2f4a34634
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Bobby-Eshleman/virtio-vsock-introduce-dgrams-sk_buff-and-qdisc/20220816-015812
git checkout 68c9c8216a573cdfe2170cad677854e2f4a34634
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=m68k SHELL=/bin/bash net/vmw_vsock/
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <[email protected]>
All warnings (new ones prefixed by >>):
>> net/vmw_vsock/virtio_transport.c:178: warning: This comment starts with '/**', but isn't a kernel-doc comment. Refer Documentation/doc-guide/kernel-doc.rst
* Merge the two most recent skbs together if possible.
vim +178 net/vmw_vsock/virtio_transport.c
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 176
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 177 /**
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 @178 * Merge the two most recent skbs together if possible.
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 179 *
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 180 * Caller must hold the queue lock.
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 181 */
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 182 static void
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 183 virtio_transport_add_to_queue(struct sk_buff_head *queue, struct sk_buff *new)
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 184 {
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 185 struct sk_buff *old;
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 186
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 187 spin_lock_bh(&queue->lock);
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 188 /* In order to reduce skb memory overhead, we merge new packets with
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 189 * older packets if they pass virtio_transport_skbs_can_merge().
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 190 */
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 191 if (skb_queue_empty_lockless(queue)) {
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 192 __skb_queue_tail(queue, new);
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 193 goto out;
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 194 }
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 195
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 196 old = skb_peek_tail(queue);
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 197
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 198 if (!virtio_transport_skbs_can_merge(old, new)) {
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 199 __skb_queue_tail(queue, new);
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 200 goto out;
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 201 }
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 202
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 203 memcpy(skb_put(old, new->len), new->data, new->len);
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 204 vsock_hdr(old)->len = cpu_to_le32(old->len);
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 205 vsock_hdr(old)->buf_alloc = vsock_hdr(new)->buf_alloc;
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 206 vsock_hdr(old)->fwd_cnt = vsock_hdr(new)->fwd_cnt;
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 207 dev_kfree_skb_any(new);
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 208
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 209 out:
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 210 spin_unlock_bh(&queue->lock);
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 211 }
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 212
--
0-DAY CI Kernel Test Service
https://01.org/lkp
Hi Bobby,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on mst-vhost/linux-next]
[also build test WARNING on linus/master v6.0-rc1]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Bobby-Eshleman/virtio-vsock-introduce-dgrams-sk_buff-and-qdisc/20220816-015812
base: https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git linux-next
config: i386-randconfig-a014-20220815 (https://download.01.org/0day-ci/archive/20220816/[email protected]/config)
compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 6afcc4a459ead8809a0d6d9b4bf7b64bcc13582b)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/68c9c8216a573cdfe2170cad677854e2f4a34634
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Bobby-Eshleman/virtio-vsock-introduce-dgrams-sk_buff-and-qdisc/20220816-015812
git checkout 68c9c8216a573cdfe2170cad677854e2f4a34634
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash net/vmw_vsock/
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <[email protected]>
All warnings (new ones prefixed by >>):
>> net/vmw_vsock/virtio_transport.c:178: warning: This comment starts with '/**', but isn't a kernel-doc comment. Refer Documentation/doc-guide/kernel-doc.rst
* Merge the two most recent skbs together if possible.
vim +178 net/vmw_vsock/virtio_transport.c
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 176
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 177 /**
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 @178 * Merge the two most recent skbs together if possible.
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 179 *
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 180 * Caller must hold the queue lock.
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 181 */
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 182 static void
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 183 virtio_transport_add_to_queue(struct sk_buff_head *queue, struct sk_buff *new)
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 184 {
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 185 struct sk_buff *old;
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 186
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 187 spin_lock_bh(&queue->lock);
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 188 /* In order to reduce skb memory overhead, we merge new packets with
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 189 * older packets if they pass virtio_transport_skbs_can_merge().
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 190 */
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 191 if (skb_queue_empty_lockless(queue)) {
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 192 __skb_queue_tail(queue, new);
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 193 goto out;
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 194 }
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 195
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 196 old = skb_peek_tail(queue);
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 197
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 198 if (!virtio_transport_skbs_can_merge(old, new)) {
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 199 __skb_queue_tail(queue, new);
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 200 goto out;
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 201 }
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 202
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 203 memcpy(skb_put(old, new->len), new->data, new->len);
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 204 vsock_hdr(old)->len = cpu_to_le32(old->len);
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 205 vsock_hdr(old)->buf_alloc = vsock_hdr(new)->buf_alloc;
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 206 vsock_hdr(old)->fwd_cnt = vsock_hdr(new)->fwd_cnt;
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 207 dev_kfree_skb_any(new);
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 208
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 209 out:
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 210 spin_unlock_bh(&queue->lock);
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 211 }
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 212
--
0-DAY CI Kernel Test Service
https://01.org/lkp
Hi Bobby,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on mst-vhost/linux-next]
[also build test WARNING on linus/master v6.0-rc1 next-20220815]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Bobby-Eshleman/virtio-vsock-introduce-dgrams-sk_buff-and-qdisc/20220816-015812
base: https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git linux-next
config: i386-randconfig-s001-20220815 (https://download.01.org/0day-ci/archive/20220816/[email protected]/config)
compiler: gcc-11 (Debian 11.3.0-5) 11.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.4-39-gce1a6720-dirty
# https://github.com/intel-lab-lkp/linux/commit/68c9c8216a573cdfe2170cad677854e2f4a34634
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Bobby-Eshleman/virtio-vsock-introduce-dgrams-sk_buff-and-qdisc/20220816-015812
git checkout 68c9c8216a573cdfe2170cad677854e2f4a34634
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=i386 SHELL=/bin/bash fs/nilfs2/ net/vmw_vsock/
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <[email protected]>
sparse warnings: (new ones prefixed by >>)
>> net/vmw_vsock/virtio_transport.c:173:31: sparse: sparse: restricted __le16 degrades to integer
net/vmw_vsock/virtio_transport.c:174:31: sparse: sparse: restricted __le16 degrades to integer
vim +173 net/vmw_vsock/virtio_transport.c
0ea9e1d3a9e3ef Asias He 2016-07-28 161
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 162 static inline bool
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 163 virtio_transport_skbs_can_merge(struct sk_buff *old, struct sk_buff *new)
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 164 {
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 165 return (new->len < GOOD_COPY_LEN &&
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 166 skb_tailroom(old) >= new->len &&
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 167 vsock_hdr(new)->src_cid == vsock_hdr(old)->src_cid &&
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 168 vsock_hdr(new)->dst_cid == vsock_hdr(old)->dst_cid &&
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 169 vsock_hdr(new)->src_port == vsock_hdr(old)->src_port &&
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 170 vsock_hdr(new)->dst_port == vsock_hdr(old)->dst_port &&
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 171 vsock_hdr(new)->type == vsock_hdr(old)->type &&
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 172 vsock_hdr(new)->flags == vsock_hdr(old)->flags &&
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 @173 vsock_hdr(old)->op == VIRTIO_VSOCK_OP_RW &&
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 174 vsock_hdr(new)->op == VIRTIO_VSOCK_OP_RW);
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 175 }
93afaf2cdefaa9 Bobby Eshleman 2022-08-15 176
--
0-DAY CI Kernel Test Service
https://01.org/lkp
CC'ing [email protected]
On Mon, Aug 15, 2022 at 10:56:05AM -0700, Bobby Eshleman wrote:
> This commit allows vsock implementations to return errors
> to the socket layer other than -ENOMEM. One immediate effect
> of this is that upon the sk_sndbuf threshold being reached -EAGAIN
> will be returned and userspace may throttle appropriately.
>
> Resultingly, a known issue with uperf is resolved[1].
>
> Additionally, to preserve legacy behavior for non-virtio
> implementations, hyperv/vmci force errors to be -ENOMEM so that behavior
> is unchanged.
>
> [1]: https://gitlab.com/vsock/vsock/-/issues/1
>
> Signed-off-by: Bobby Eshleman <[email protected]>
> ---
> include/linux/virtio_vsock.h | 3 +++
> net/vmw_vsock/af_vsock.c | 3 ++-
> net/vmw_vsock/hyperv_transport.c | 2 +-
> net/vmw_vsock/virtio_transport_common.c | 3 ---
> net/vmw_vsock/vmci_transport.c | 9 ++++++++-
> 5 files changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
> index 17ed01466875..9a37eddbb87a 100644
> --- a/include/linux/virtio_vsock.h
> +++ b/include/linux/virtio_vsock.h
> @@ -8,6 +8,9 @@
> #include <net/sock.h>
> #include <net/af_vsock.h>
>
> +/* Threshold for detecting small packets to copy */
> +#define GOOD_COPY_LEN 128
> +
> enum virtio_vsock_metadata_flags {
> VIRTIO_VSOCK_METADATA_FLAGS_REPLY = BIT(0),
> VIRTIO_VSOCK_METADATA_FLAGS_TAP_DELIVERED = BIT(1),
> diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
> index e348b2d09eac..1893f8aafa48 100644
> --- a/net/vmw_vsock/af_vsock.c
> +++ b/net/vmw_vsock/af_vsock.c
> @@ -1844,8 +1844,9 @@ static int vsock_connectible_sendmsg(struct socket *sock, struct msghdr *msg,
> written = transport->stream_enqueue(vsk,
> msg, len - total_written);
> }
> +
> if (written < 0) {
> - err = -ENOMEM;
> + err = written;
> goto out_err;
> }
>
> diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
> index fd98229e3db3..e99aea571f6f 100644
> --- a/net/vmw_vsock/hyperv_transport.c
> +++ b/net/vmw_vsock/hyperv_transport.c
> @@ -687,7 +687,7 @@ static ssize_t hvs_stream_enqueue(struct vsock_sock *vsk, struct msghdr *msg,
> if (bytes_written)
> ret = bytes_written;
> kfree(send_buf);
> - return ret;
> + return ret < 0 ? -ENOMEM : ret;
> }
>
> static s64 hvs_stream_has_data(struct vsock_sock *vsk)
> diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> index 920578597bb9..d5780599fe93 100644
> --- a/net/vmw_vsock/virtio_transport_common.c
> +++ b/net/vmw_vsock/virtio_transport_common.c
> @@ -23,9 +23,6 @@
> /* How long to wait for graceful shutdown of a connection */
> #define VSOCK_CLOSE_TIMEOUT (8 * HZ)
>
> -/* Threshold for detecting small packets to copy */
> -#define GOOD_COPY_LEN 128
> -
> static const struct virtio_transport *
> virtio_transport_get_ops(struct vsock_sock *vsk)
> {
> diff --git a/net/vmw_vsock/vmci_transport.c b/net/vmw_vsock/vmci_transport.c
> index b14f0ed7427b..c927a90dc859 100644
> --- a/net/vmw_vsock/vmci_transport.c
> +++ b/net/vmw_vsock/vmci_transport.c
> @@ -1838,7 +1838,14 @@ static ssize_t vmci_transport_stream_enqueue(
> struct msghdr *msg,
> size_t len)
> {
> - return vmci_qpair_enquev(vmci_trans(vsk)->qpair, msg, len, 0);
> + int err;
> +
> + err = vmci_qpair_enquev(vmci_trans(vsk)->qpair, msg, len, 0);
> +
> + if (err < 0)
> + err = -ENOMEM;
> +
> + return err;
> }
>
> static s64 vmci_transport_stream_has_data(struct vsock_sock *vsk)
> --
> 2.35.1
>
On 16.08.2022 05:30, Bobby Eshleman wrote:
> CC'ing [email protected]
>
> On Mon, Aug 15, 2022 at 10:56:05AM -0700, Bobby Eshleman wrote:
>> This commit allows vsock implementations to return errors
>> to the socket layer other than -ENOMEM. One immediate effect
>> of this is that upon the sk_sndbuf threshold being reached -EAGAIN
>> will be returned and userspace may throttle appropriately.
>>
>> Resultingly, a known issue with uperf is resolved[1].
>>
>> Additionally, to preserve legacy behavior for non-virtio
>> implementations, hyperv/vmci force errors to be -ENOMEM so that behavior
>> is unchanged.
>>
>> [1]: https://gitlab.com/vsock/vsock/-/issues/1
>>
>> Signed-off-by: Bobby Eshleman <[email protected]>
>> ---
>> include/linux/virtio_vsock.h | 3 +++
>> net/vmw_vsock/af_vsock.c | 3 ++-
>> net/vmw_vsock/hyperv_transport.c | 2 +-
>> net/vmw_vsock/virtio_transport_common.c | 3 ---
>> net/vmw_vsock/vmci_transport.c | 9 ++++++++-
>> 5 files changed, 14 insertions(+), 6 deletions(-)
>>
>> diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
>> index 17ed01466875..9a37eddbb87a 100644
>> --- a/include/linux/virtio_vsock.h
>> +++ b/include/linux/virtio_vsock.h
>> @@ -8,6 +8,9 @@
>> #include <net/sock.h>
>> #include <net/af_vsock.h>
>>
>> +/* Threshold for detecting small packets to copy */
>> +#define GOOD_COPY_LEN 128
>> +
>> enum virtio_vsock_metadata_flags {
>> VIRTIO_VSOCK_METADATA_FLAGS_REPLY = BIT(0),
>> VIRTIO_VSOCK_METADATA_FLAGS_TAP_DELIVERED = BIT(1),
>> diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
>> index e348b2d09eac..1893f8aafa48 100644
>> --- a/net/vmw_vsock/af_vsock.c
>> +++ b/net/vmw_vsock/af_vsock.c
>> @@ -1844,8 +1844,9 @@ static int vsock_connectible_sendmsg(struct socket *sock, struct msghdr *msg,
>> written = transport->stream_enqueue(vsk,
>> msg, len - total_written);
>> }
>> +
>> if (written < 0) {
>> - err = -ENOMEM;
>> + err = written;
>> goto out_err;
>> }
IIUC, for stream, this thing will have effect, only one first transport access fails. In this
case 'total_written' will be 0, so 'err' == 'written' will be returned. But when 'total_written > 0',
'err' will be overwritten by 'total_written' below, preserving current behaviour. Is it what You
supposed?
Thanks
>>
>> diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
>> index fd98229e3db3..e99aea571f6f 100644
>> --- a/net/vmw_vsock/hyperv_transport.c
>> +++ b/net/vmw_vsock/hyperv_transport.c
>> @@ -687,7 +687,7 @@ static ssize_t hvs_stream_enqueue(struct vsock_sock *vsk, struct msghdr *msg,
>> if (bytes_written)
>> ret = bytes_written;
>> kfree(send_buf);
>> - return ret;
>> + return ret < 0 ? -ENOMEM : ret;
>> }
>>
>> static s64 hvs_stream_has_data(struct vsock_sock *vsk)
>> diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
>> index 920578597bb9..d5780599fe93 100644
>> --- a/net/vmw_vsock/virtio_transport_common.c
>> +++ b/net/vmw_vsock/virtio_transport_common.c
>> @@ -23,9 +23,6 @@
>> /* How long to wait for graceful shutdown of a connection */
>> #define VSOCK_CLOSE_TIMEOUT (8 * HZ)
>>
>> -/* Threshold for detecting small packets to copy */
>> -#define GOOD_COPY_LEN 128
>> -
>> static const struct virtio_transport *
>> virtio_transport_get_ops(struct vsock_sock *vsk)
>> {
>> diff --git a/net/vmw_vsock/vmci_transport.c b/net/vmw_vsock/vmci_transport.c
>> index b14f0ed7427b..c927a90dc859 100644
>> --- a/net/vmw_vsock/vmci_transport.c
>> +++ b/net/vmw_vsock/vmci_transport.c
>> @@ -1838,7 +1838,14 @@ static ssize_t vmci_transport_stream_enqueue(
>> struct msghdr *msg,
>> size_t len)
>> {
>> - return vmci_qpair_enquev(vmci_trans(vsk)->qpair, msg, len, 0);
>> + int err;
>> +
>> + err = vmci_qpair_enquev(vmci_trans(vsk)->qpair, msg, len, 0);
>> +
>> + if (err < 0)
>> + err = -ENOMEM;
>> +
>> + return err;
>> }
>>
>> static s64 vmci_transport_stream_has_data(struct vsock_sock *vsk)
>> --
>> 2.35.1
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [email protected]
> For additional commands, e-mail: [email protected]
>
On Mon, Aug 15, 2022 at 10:56:05AM -0700, Bobby Eshleman wrote:
>This commit allows vsock implementations to return errors
>to the socket layer other than -ENOMEM. One immediate effect
>of this is that upon the sk_sndbuf threshold being reached -EAGAIN
>will be returned and userspace may throttle appropriately.
>
>Resultingly, a known issue with uperf is resolved[1].
>
>Additionally, to preserve legacy behavior for non-virtio
>implementations, hyperv/vmci force errors to be -ENOMEM so that behavior
>is unchanged.
>
>[1]: https://gitlab.com/vsock/vsock/-/issues/1
>
>Signed-off-by: Bobby Eshleman <[email protected]>
>---
> include/linux/virtio_vsock.h | 3 +++
> net/vmw_vsock/af_vsock.c | 3 ++-
> net/vmw_vsock/hyperv_transport.c | 2 +-
> net/vmw_vsock/virtio_transport_common.c | 3 ---
> net/vmw_vsock/vmci_transport.c | 9 ++++++++-
> 5 files changed, 14 insertions(+), 6 deletions(-)
>
>diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
>index 17ed01466875..9a37eddbb87a 100644
>--- a/include/linux/virtio_vsock.h
>+++ b/include/linux/virtio_vsock.h
>@@ -8,6 +8,9 @@
> #include <net/sock.h>
> #include <net/af_vsock.h>
>
>+/* Threshold for detecting small packets to copy */
>+#define GOOD_COPY_LEN 128
>+
This change seems unrelated.
Please move it in the patch where you need this.
Maybe it's better to add a prefix if we move it in an header file (e.g.
VIRTIO_VSOCK_...).
Thanks,
Stefano
> enum virtio_vsock_metadata_flags {
> VIRTIO_VSOCK_METADATA_FLAGS_REPLY = BIT(0),
> VIRTIO_VSOCK_METADATA_FLAGS_TAP_DELIVERED = BIT(1),
>diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
>index e348b2d09eac..1893f8aafa48 100644
>--- a/net/vmw_vsock/af_vsock.c
>+++ b/net/vmw_vsock/af_vsock.c
>@@ -1844,8 +1844,9 @@ static int vsock_connectible_sendmsg(struct socket *sock, struct msghdr *msg,
> written = transport->stream_enqueue(vsk,
> msg, len - total_written);
> }
>+
> if (written < 0) {
>- err = -ENOMEM;
>+ err = written;
> goto out_err;
> }
>
>diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
>index fd98229e3db3..e99aea571f6f 100644
>--- a/net/vmw_vsock/hyperv_transport.c
>+++ b/net/vmw_vsock/hyperv_transport.c
>@@ -687,7 +687,7 @@ static ssize_t hvs_stream_enqueue(struct vsock_sock *vsk, struct msghdr *msg,
> if (bytes_written)
> ret = bytes_written;
> kfree(send_buf);
>- return ret;
>+ return ret < 0 ? -ENOMEM : ret;
> }
>
> static s64 hvs_stream_has_data(struct vsock_sock *vsk)
>diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
>index 920578597bb9..d5780599fe93 100644
>--- a/net/vmw_vsock/virtio_transport_common.c
>+++ b/net/vmw_vsock/virtio_transport_common.c
>@@ -23,9 +23,6 @@
> /* How long to wait for graceful shutdown of a connection */
> #define VSOCK_CLOSE_TIMEOUT (8 * HZ)
>
>-/* Threshold for detecting small packets to copy */
>-#define GOOD_COPY_LEN 128
>-
> static const struct virtio_transport *
> virtio_transport_get_ops(struct vsock_sock *vsk)
> {
>diff --git a/net/vmw_vsock/vmci_transport.c b/net/vmw_vsock/vmci_transport.c
>index b14f0ed7427b..c927a90dc859 100644
>--- a/net/vmw_vsock/vmci_transport.c
>+++ b/net/vmw_vsock/vmci_transport.c
>@@ -1838,7 +1838,14 @@ static ssize_t vmci_transport_stream_enqueue(
> struct msghdr *msg,
> size_t len)
> {
>- return vmci_qpair_enquev(vmci_trans(vsk)->qpair, msg, len, 0);
>+ int err;
>+
>+ err = vmci_qpair_enquev(vmci_trans(vsk)->qpair, msg, len, 0);
>+
>+ if (err < 0)
>+ err = -ENOMEM;
>+
>+ return err;
> }
>
> static s64 vmci_transport_stream_has_data(struct vsock_sock *vsk)
>--
>2.35.1
>
On Mon, Sep 26, 2022 at 03:21:45PM +0200, Stefano Garzarella wrote:
> On Mon, Aug 15, 2022 at 10:56:05AM -0700, Bobby Eshleman wrote:
> > This commit allows vsock implementations to return errors
> > to the socket layer other than -ENOMEM. One immediate effect
> > of this is that upon the sk_sndbuf threshold being reached -EAGAIN
> > will be returned and userspace may throttle appropriately.
> >
> > Resultingly, a known issue with uperf is resolved[1].
> >
> > Additionally, to preserve legacy behavior for non-virtio
> > implementations, hyperv/vmci force errors to be -ENOMEM so that behavior
> > is unchanged.
> >
> > [1]: https://gitlab.com/vsock/vsock/-/issues/1
> >
> > Signed-off-by: Bobby Eshleman <[email protected]>
> > ---
> > include/linux/virtio_vsock.h | 3 +++
> > net/vmw_vsock/af_vsock.c | 3 ++-
> > net/vmw_vsock/hyperv_transport.c | 2 +-
> > net/vmw_vsock/virtio_transport_common.c | 3 ---
> > net/vmw_vsock/vmci_transport.c | 9 ++++++++-
> > 5 files changed, 14 insertions(+), 6 deletions(-)
> >
> > diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
> > index 17ed01466875..9a37eddbb87a 100644
> > --- a/include/linux/virtio_vsock.h
> > +++ b/include/linux/virtio_vsock.h
> > @@ -8,6 +8,9 @@
> > #include <net/sock.h>
> > #include <net/af_vsock.h>
> >
> > +/* Threshold for detecting small packets to copy */
> > +#define GOOD_COPY_LEN 128
> > +
>
> This change seems unrelated.
>
> Please move it in the patch where you need this.
> Maybe it's better to add a prefix if we move it in an header file (e.g.
> VIRTIO_VSOCK_...).
>
> Thanks,
> Stefano
>
Oh yes, definitely.
Thanks,
Bobby
> > enum virtio_vsock_metadata_flags {
> > VIRTIO_VSOCK_METADATA_FLAGS_REPLY = BIT(0),
> > VIRTIO_VSOCK_METADATA_FLAGS_TAP_DELIVERED = BIT(1),
> > diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
> > index e348b2d09eac..1893f8aafa48 100644
> > --- a/net/vmw_vsock/af_vsock.c
> > +++ b/net/vmw_vsock/af_vsock.c
> > @@ -1844,8 +1844,9 @@ static int vsock_connectible_sendmsg(struct socket *sock, struct msghdr *msg,
> > written = transport->stream_enqueue(vsk,
> > msg, len - total_written);
> > }
> > +
> > if (written < 0) {
> > - err = -ENOMEM;
> > + err = written;
> > goto out_err;
> > }
> >
> > diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
> > index fd98229e3db3..e99aea571f6f 100644
> > --- a/net/vmw_vsock/hyperv_transport.c
> > +++ b/net/vmw_vsock/hyperv_transport.c
> > @@ -687,7 +687,7 @@ static ssize_t hvs_stream_enqueue(struct vsock_sock *vsk, struct msghdr *msg,
> > if (bytes_written)
> > ret = bytes_written;
> > kfree(send_buf);
> > - return ret;
> > + return ret < 0 ? -ENOMEM : ret;
> > }
> >
> > static s64 hvs_stream_has_data(struct vsock_sock *vsk)
> > diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> > index 920578597bb9..d5780599fe93 100644
> > --- a/net/vmw_vsock/virtio_transport_common.c
> > +++ b/net/vmw_vsock/virtio_transport_common.c
> > @@ -23,9 +23,6 @@
> > /* How long to wait for graceful shutdown of a connection */
> > #define VSOCK_CLOSE_TIMEOUT (8 * HZ)
> >
> > -/* Threshold for detecting small packets to copy */
> > -#define GOOD_COPY_LEN 128
> > -
> > static const struct virtio_transport *
> > virtio_transport_get_ops(struct vsock_sock *vsk)
> > {
> > diff --git a/net/vmw_vsock/vmci_transport.c b/net/vmw_vsock/vmci_transport.c
> > index b14f0ed7427b..c927a90dc859 100644
> > --- a/net/vmw_vsock/vmci_transport.c
> > +++ b/net/vmw_vsock/vmci_transport.c
> > @@ -1838,7 +1838,14 @@ static ssize_t vmci_transport_stream_enqueue(
> > struct msghdr *msg,
> > size_t len)
> > {
> > - return vmci_qpair_enquev(vmci_trans(vsk)->qpair, msg, len, 0);
> > + int err;
> > +
> > + err = vmci_qpair_enquev(vmci_trans(vsk)->qpair, msg, len, 0);
> > +
> > + if (err < 0)
> > + err = -ENOMEM;
> > +
> > + return err;
> > }
> >
> > static s64 vmci_transport_stream_has_data(struct vsock_sock *vsk)
> > --
> > 2.35.1
> >
>
> _______________________________________________
> Virtualization mailing list
> [email protected]
> https://lists.linuxfoundation.org/mailman/listinfo/virtualization