Message ID | 0abeec42-a11d-3a51-453b-6acf76604f2e@sberdevices.ru |
---|---|
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:5915:0:0:0:0:0 with SMTP id v21csp210579wrd; Thu, 9 Mar 2023 02:27:15 -0800 (PST) X-Google-Smtp-Source: AK7set85qM9ukvir9xiglqkS5O4o5kRh00LBTpI+3lhCBLl+ktv9421/Zryp9HbUL0VRCVcL/vW2 X-Received: by 2002:a17:902:ce8a:b0:19c:f698:8564 with SMTP id f10-20020a170902ce8a00b0019cf6988564mr27405136plg.17.1678357634839; Thu, 09 Mar 2023 02:27:14 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1678357634; cv=none; d=google.com; s=arc-20160816; b=EyKeEUrcBIZae9FogG3zeZindAMC6cs6MenUuN6sgL4XyT6CxQkzrt+ISjboOPYVyV LJ/c1BBPowddL0qcFpSXmjQLAwgmsKzqwqPBrbbiMEazVCYLUMRmcD0kHwtw+q8vXa+p 7pP6k/CU35L587fj1x1fG/3qsqRJXsL0H2U2/diw34WB/FiYBFiC2UvvAI4nz86Zec1c s0fIy1nozGbU5GV5VhE34edlJQpBK0E0Bh3lrfqA45vqYW1S3VKhvF+2yizol/Sfvo4B Gf4QCWuFepYcOZmTykER4MOoKbT1aEla6zRM+KCejFHZyqQH6jabOdn+Vi+pvreCiOZj b7Qg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:subject:from:cc:to :content-language:user-agent:mime-version:date:message-id :dkim-signature; bh=Ar8EpR1QVdZc2qgf8SPgt+ILQTHYEu4nfHN9aAAB8Qk=; b=WvBuUwznekhhAwX2RYmtGv50I/vq0ThbaqgvncYw8+vnsE8LbKGazfN30LNoS2okf6 eWZ6d0Zgk3mRT9DSCHxWAsJNs90xghJHQoVjmDSTZb/eyARljgsKSqBxwhJbjSXgmWQL fOFAq57c+2HFDRGU/Pw9IQpKiY2aTOYEnjm+d7YqC8sbQAz7yysXkPWqM+e8zTDiQqeG zkssV7+1KRSdzm2ZH5Vz9gH5gN+ko+6OUybeBdvS9wSlsxz/Ukeb+3lJ3mWMBsz6Pyul e7Eo1Lxay95GBCp3fA3Qmwljeo8Eqpr79Hhh+6tTIVQndh9unvnuRJ7mt0mXzr/kaH+p /Zmg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@sberdevices.ru header.s=mail header.b=XHcmk+Ak; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=sberdevices.ru Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id kd13-20020a17090313cd00b0019aa6c46413si16359621plb.345.2023.03.09.02.26.59; Thu, 09 Mar 2023 02:27:14 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@sberdevices.ru header.s=mail header.b=XHcmk+Ak; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=sberdevices.ru Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230266AbjCIKPG (ORCPT <rfc822;carlos.wei.hk@gmail.com> + 99 others); Thu, 9 Mar 2023 05:15:06 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:49680 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230473AbjCIKOw (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Thu, 9 Mar 2023 05:14:52 -0500 Received: from mx.sberdevices.ru (mx.sberdevices.ru [45.89.227.171]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 6DE9D515EF; Thu, 9 Mar 2023 02:14:19 -0800 (PST) Received: from s-lin-edge02.sberdevices.ru (localhost [127.0.0.1]) by mx.sberdevices.ru (Postfix) with ESMTP id 703E05FD38; Thu, 9 Mar 2023 13:13:40 +0300 (MSK) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sberdevices.ru; s=mail; t=1678356820; bh=Ar8EpR1QVdZc2qgf8SPgt+ILQTHYEu4nfHN9aAAB8Qk=; h=Message-ID:Date:MIME-Version:To:From:Subject:Content-Type; b=XHcmk+AkXXWXBp/pGJlndL3ZEJKUVwwA2YVgkru62e9Ng6rK7I7njCyq4BS3YX7LU mPl5Gq7NDsXsFB8+z+gY52NsqI2F1PxcBifJx3pdNJoSZb3R1ThbQiubiMc9J4Fbyt qyO6CTYq7QZ1xQNMg6v2bU4r9LdfagUijseY8AOz8ucigT2l1MfY7DKJID814KeGcm /AwdNGzc+ahU4bMGJYCaVdGnki0Zlky1u52/DDVZXwmeVXIoCHrYw6sehNHCHZ4kja y9gCXm45h7Rqwa5OjuibME7CvMUPlxhVOA/HIK1p4pfgKU2HaYarW6Tj96JY5iRnh8 I7f2xpyjNB0fA== Received: from S-MS-EXCH01.sberdevices.ru (S-MS-EXCH01.sberdevices.ru [172.16.1.4]) by mx.sberdevices.ru (Postfix) with ESMTP; Thu, 9 Mar 2023 13:13:36 +0300 (MSK) Message-ID: <0abeec42-a11d-3a51-453b-6acf76604f2e@sberdevices.ru> Date: Thu, 9 Mar 2023 13:10:36 +0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.7.1 Content-Language: en-US To: Stefan Hajnoczi <stefanha@redhat.com>, Stefano Garzarella <sgarzare@redhat.com>, "David S. Miller" <davem@davemloft.net>, Eric Dumazet <edumazet@google.com>, Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>, Bobby Eshleman <bobby.eshleman@bytedance.com> CC: <kvm@vger.kernel.org>, <virtualization@lists.linux-foundation.org>, <netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org>, <kernel@sberdevices.ru>, <oxffffaa@gmail.com>, <avkrasnov@sberdevices.ru> From: Arseniy Krasnov <avkrasnov@sberdevices.ru> Subject: [RFC PATCH v3 0/4] several updates to virtio/vsock Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit X-Originating-IP: [172.16.1.6] X-ClientProxiedBy: S-MS-EXCH01.sberdevices.ru (172.16.1.4) To S-MS-EXCH01.sberdevices.ru (172.16.1.4) X-KSMG-Rule-ID: 4 X-KSMG-Message-Action: clean X-KSMG-AntiSpam-Status: not scanned, disabled by settings X-KSMG-AntiSpam-Interceptor-Info: not scanned X-KSMG-AntiPhishing: not scanned, disabled by settings X-KSMG-AntiVirus: Kaspersky Secure Mail Gateway, version 1.1.2.30, bases: 2023/03/09 05:43:00 #20927523 X-KSMG-AntiVirus-Status: Clean, skipped X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1759885535159820823?= X-GMAIL-MSGID: =?utf-8?q?1759885535159820823?= |
Series | several updates to virtio/vsock | |
Message
Arseniy Krasnov
March 9, 2023, 10:10 a.m. UTC
Hello, this patchset evolved from previous v2 version (see link below). It does several updates to virtio/vsock: 1) Changes 'virtio_transport_inc/dec_rx_pkt()' interface. Now instead of using skbuff state ('head' and 'data' pointers) to update 'fwd_cnt' and 'rx_bytes', integer value is passed as an input argument. This makes code more simple, because in this case we don't need to udpate skbuff state before calling 'virtio_transport_inc/dec_rx_pkt()'. In more common words - we don't need to change skbuff state to update 'rx_bytes' and 'fwd_cnt' correctly. 2) For SOCK_STREAM, when copying data to user fails, current skbuff is not dropped. Next read attempt will use same skbuff and last offset. Instead of 'skb_dequeue()', 'skb_peek()' + '__skb_unlink()' are used. This behaviour was implemented before skbuff support. 3) For SOCK_SEQPACKET it removes unneeded 'skb_pull()' call, because for this type of socket each skbuff is used only once: after removing it from socket's queue, it will be freed anyway. Test for 2) also added: Test tries to 'recv()' data to NULL buffer, then does 'recv()' with valid buffer. For SOCK_STREAM second 'recv()' must return data, because skbuff must not be dropped, but for SOCK_SEQPACKET skbuff will be dropped by kernel, and 'recv()' will return EAGAIN. Link to v1 on lore: https://lore.kernel.org/netdev/c2d3e204-89d9-88e9-8a15-3fe027e56b4b@sberdevices.ru/ Link to v2 on lore: https://lore.kernel.org/netdev/a7ab414b-5e41-c7b6-250b-e8401f335859@sberdevices.ru/ Change log: v1 -> v2: - For SOCK_SEQPACKET call 'skb_pull()' also in case of copy failure or dropping skbuff (when we just waiting message end). - Handle copy failure for SOCK_STREAM in the same manner (plus free current skbuff). - Replace bug repdroducer with new test in vsock_test.c v2 -> v3: - Replace patch which removes 'skb->len' subtraction from function 'virtio_transport_dec_rx_pkt()' with patch which updates functions 'virtio_transport_inc/dec_rx_pkt()' by passing integer argument instead of skbuff pointer. - Replace patch which drops skbuff when copying to user fails with patch which changes this behaviour by keeping skbuff in queue until it has no data. - Add patch for SOCK_SEQPACKET which removes redundant 'skb_pull()' call on read. - I remove "Fixes" tag from all patches, because all of them now change code logic, not only fix something. Arseniy Krasnov (4): virtio/vsock: don't use skbuff state to account credit virtio/vsock: remove redundant 'skb_pull()' call virtio/vsock: don't drop skbuff on copy failure test/vsock: copy to user failure test net/vmw_vsock/virtio_transport_common.c | 29 +++--- tools/testing/vsock/vsock_test.c | 118 ++++++++++++++++++++++++ 2 files changed, 131 insertions(+), 16 deletions(-)
Comments
On 09.03.2023 19:21, Stefano Garzarella wrote: > On Thu, Mar 09, 2023 at 01:10:36PM +0300, Arseniy Krasnov wrote: >> Hello, >> >> this patchset evolved from previous v2 version (see link below). It does >> several updates to virtio/vsock: >> 1) Changes 'virtio_transport_inc/dec_rx_pkt()' interface. Now instead of >> using skbuff state ('head' and 'data' pointers) to update 'fwd_cnt' >> and 'rx_bytes', integer value is passed as an input argument. This >> makes code more simple, because in this case we don't need to udpate >> skbuff state before calling 'virtio_transport_inc/dec_rx_pkt()'. In >> more common words - we don't need to change skbuff state to update >> 'rx_bytes' and 'fwd_cnt' correctly. >> 2) For SOCK_STREAM, when copying data to user fails, current skbuff is >> not dropped. Next read attempt will use same skbuff and last offset. >> Instead of 'skb_dequeue()', 'skb_peek()' + '__skb_unlink()' are used. >> This behaviour was implemented before skbuff support. >> 3) For SOCK_SEQPACKET it removes unneeded 'skb_pull()' call, because for >> this type of socket each skbuff is used only once: after removing it >> from socket's queue, it will be freed anyway. >> >> Test for 2) also added: >> Test tries to 'recv()' data to NULL buffer, then does 'recv()' with valid >> buffer. For SOCK_STREAM second 'recv()' must return data, because skbuff >> must not be dropped, but for SOCK_SEQPACKET skbuff will be dropped by >> kernel, and 'recv()' will return EAGAIN. >> >> Link to v1 on lore: >> https://lore.kernel.org/netdev/c2d3e204-89d9-88e9-8a15-3fe027e56b4b@sberdevices.ru/ >> >> Link to v2 on lore: >> https://lore.kernel.org/netdev/a7ab414b-5e41-c7b6-250b-e8401f335859@sberdevices.ru/ >> >> Change log: >> >> v1 -> v2: >> - For SOCK_SEQPACKET call 'skb_pull()' also in case of copy failure or >> dropping skbuff (when we just waiting message end). >> - Handle copy failure for SOCK_STREAM in the same manner (plus free >> current skbuff). >> - Replace bug repdroducer with new test in vsock_test.c >> >> v2 -> v3: >> - Replace patch which removes 'skb->len' subtraction from function >> 'virtio_transport_dec_rx_pkt()' with patch which updates functions >> 'virtio_transport_inc/dec_rx_pkt()' by passing integer argument >> instead of skbuff pointer. >> - Replace patch which drops skbuff when copying to user fails with >> patch which changes this behaviour by keeping skbuff in queue until >> it has no data. >> - Add patch for SOCK_SEQPACKET which removes redundant 'skb_pull()' >> call on read. >> - I remove "Fixes" tag from all patches, because all of them now change >> code logic, not only fix something. > > Yes, but they solve the problem, so we should use the tag (I think at > least in patch 1 and 3). > > We usually use the tag when we are fixing a problem introduced by a > previous change. So we need to backport the patch to the stable branches > as well, and we need the tag to figure out which branches have the patch > or not. Ahh, sorry. Ok. I see now :) Thanks, Arseniy > > Thanks, > Stefano > >> >> Arseniy Krasnov (4): >> virtio/vsock: don't use skbuff state to account credit >> virtio/vsock: remove redundant 'skb_pull()' call >> virtio/vsock: don't drop skbuff on copy failure >> test/vsock: copy to user failure test >> >> net/vmw_vsock/virtio_transport_common.c | 29 +++--- >> tools/testing/vsock/vsock_test.c | 118 ++++++++++++++++++++++++ >> 2 files changed, 131 insertions(+), 16 deletions(-) >> >> -- >> 2.25.1 >> >
On Thu, Mar 09, 2023 at 01:10:36PM +0300, Arseniy Krasnov wrote: >Hello, > >this patchset evolved from previous v2 version (see link below). It does >several updates to virtio/vsock: >1) Changes 'virtio_transport_inc/dec_rx_pkt()' interface. Now instead of > using skbuff state ('head' and 'data' pointers) to update 'fwd_cnt' > and 'rx_bytes', integer value is passed as an input argument. This > makes code more simple, because in this case we don't need to udpate > skbuff state before calling 'virtio_transport_inc/dec_rx_pkt()'. In > more common words - we don't need to change skbuff state to update > 'rx_bytes' and 'fwd_cnt' correctly. >2) For SOCK_STREAM, when copying data to user fails, current skbuff is > not dropped. Next read attempt will use same skbuff and last offset. > Instead of 'skb_dequeue()', 'skb_peek()' + '__skb_unlink()' are used. > This behaviour was implemented before skbuff support. >3) For SOCK_SEQPACKET it removes unneeded 'skb_pull()' call, because for > this type of socket each skbuff is used only once: after removing it > from socket's queue, it will be freed anyway. > >Test for 2) also added: >Test tries to 'recv()' data to NULL buffer, then does 'recv()' with valid >buffer. For SOCK_STREAM second 'recv()' must return data, because skbuff >must not be dropped, but for SOCK_SEQPACKET skbuff will be dropped by >kernel, and 'recv()' will return EAGAIN. > >Link to v1 on lore: >https://lore.kernel.org/netdev/c2d3e204-89d9-88e9-8a15-3fe027e56b4b@sberdevices.ru/ > >Link to v2 on lore: >https://lore.kernel.org/netdev/a7ab414b-5e41-c7b6-250b-e8401f335859@sberdevices.ru/ > >Change log: > >v1 -> v2: > - For SOCK_SEQPACKET call 'skb_pull()' also in case of copy failure or > dropping skbuff (when we just waiting message end). > - Handle copy failure for SOCK_STREAM in the same manner (plus free > current skbuff). > - Replace bug repdroducer with new test in vsock_test.c > >v2 -> v3: > - Replace patch which removes 'skb->len' subtraction from function > 'virtio_transport_dec_rx_pkt()' with patch which updates functions > 'virtio_transport_inc/dec_rx_pkt()' by passing integer argument > instead of skbuff pointer. > - Replace patch which drops skbuff when copying to user fails with > patch which changes this behaviour by keeping skbuff in queue until > it has no data. > - Add patch for SOCK_SEQPACKET which removes redundant 'skb_pull()' > call on read. > - I remove "Fixes" tag from all patches, because all of them now change > code logic, not only fix something. Yes, but they solve the problem, so we should use the tag (I think at least in patch 1 and 3). We usually use the tag when we are fixing a problem introduced by a previous change. So we need to backport the patch to the stable branches as well, and we need the tag to figure out which branches have the patch or not. Thanks, Stefano > >Arseniy Krasnov (4): > virtio/vsock: don't use skbuff state to account credit > virtio/vsock: remove redundant 'skb_pull()' call > virtio/vsock: don't drop skbuff on copy failure > test/vsock: copy to user failure test > > net/vmw_vsock/virtio_transport_common.c | 29 +++--- > tools/testing/vsock/vsock_test.c | 118 ++++++++++++++++++++++++ > 2 files changed, 131 insertions(+), 16 deletions(-) > >-- >2.25.1 >
On Thu, Mar 09, 2023 at 07:20:20PM +0300, Arseniy Krasnov wrote: > > >On 09.03.2023 19:21, Stefano Garzarella wrote: >> On Thu, Mar 09, 2023 at 01:10:36PM +0300, Arseniy Krasnov wrote: >>> Hello, >>> >>> this patchset evolved from previous v2 version (see link below). It does >>> several updates to virtio/vsock: >>> 1) Changes 'virtio_transport_inc/dec_rx_pkt()' interface. Now instead of >>> using skbuff state ('head' and 'data' pointers) to update 'fwd_cnt' >>> and 'rx_bytes', integer value is passed as an input argument. This >>> makes code more simple, because in this case we don't need to udpate >>> skbuff state before calling 'virtio_transport_inc/dec_rx_pkt()'. In >>> more common words - we don't need to change skbuff state to update >>> 'rx_bytes' and 'fwd_cnt' correctly. >>> 2) For SOCK_STREAM, when copying data to user fails, current skbuff is >>> not dropped. Next read attempt will use same skbuff and last offset. >>> Instead of 'skb_dequeue()', 'skb_peek()' + '__skb_unlink()' are used. >>> This behaviour was implemented before skbuff support. >>> 3) For SOCK_SEQPACKET it removes unneeded 'skb_pull()' call, because for >>> this type of socket each skbuff is used only once: after removing it >>> from socket's queue, it will be freed anyway. >>> >>> Test for 2) also added: >>> Test tries to 'recv()' data to NULL buffer, then does 'recv()' with valid >>> buffer. For SOCK_STREAM second 'recv()' must return data, because skbuff >>> must not be dropped, but for SOCK_SEQPACKET skbuff will be dropped by >>> kernel, and 'recv()' will return EAGAIN. >>> >>> Link to v1 on lore: >>> https://lore.kernel.org/netdev/c2d3e204-89d9-88e9-8a15-3fe027e56b4b@sberdevices.ru/ >>> >>> Link to v2 on lore: >>> https://lore.kernel.org/netdev/a7ab414b-5e41-c7b6-250b-e8401f335859@sberdevices.ru/ >>> >>> Change log: >>> >>> v1 -> v2: >>> - For SOCK_SEQPACKET call 'skb_pull()' also in case of copy failure or >>> dropping skbuff (when we just waiting message end). >>> - Handle copy failure for SOCK_STREAM in the same manner (plus free >>> current skbuff). >>> - Replace bug repdroducer with new test in vsock_test.c >>> >>> v2 -> v3: >>> - Replace patch which removes 'skb->len' subtraction from function >>> 'virtio_transport_dec_rx_pkt()' with patch which updates functions >>> 'virtio_transport_inc/dec_rx_pkt()' by passing integer argument >>> instead of skbuff pointer. >>> - Replace patch which drops skbuff when copying to user fails with >>> patch which changes this behaviour by keeping skbuff in queue until >>> it has no data. >>> - Add patch for SOCK_SEQPACKET which removes redundant 'skb_pull()' >>> call on read. >>> - I remove "Fixes" tag from all patches, because all of them now change >>> code logic, not only fix something. >> >> Yes, but they solve the problem, so we should use the tag (I think at >> least in patch 1 and 3). >> >> We usually use the tag when we are fixing a problem introduced by a >> previous change. So we need to backport the patch to the stable branches >> as well, and we need the tag to figure out which branches have the patch >> or not. >Ahh, sorry. Ok. I see now :) No problem at all :-) I think also patch 2 can have the Fixes tag. Thanks, Stefano > >Thanks, Arseniy >> >> Thanks, >> Stefano >> >>> >>> Arseniy Krasnov (4): >>> virtio/vsock: don't use skbuff state to account credit >>> virtio/vsock: remove redundant 'skb_pull()' call >>> virtio/vsock: don't drop skbuff on copy failure >>> test/vsock: copy to user failure test >>> >>> net/vmw_vsock/virtio_transport_common.c | 29 +++--- >>> tools/testing/vsock/vsock_test.c | 118 ++++++++++++++++++++++++ >>> 2 files changed, 131 insertions(+), 16 deletions(-) >>> >>> -- >>> 2.25.1 >>> >> >
On 09.03.2023 19:32, Stefano Garzarella wrote: > On Thu, Mar 09, 2023 at 07:20:20PM +0300, Arseniy Krasnov wrote: >> >> >> On 09.03.2023 19:21, Stefano Garzarella wrote: >>> On Thu, Mar 09, 2023 at 01:10:36PM +0300, Arseniy Krasnov wrote: >>>> Hello, >>>> >>>> this patchset evolved from previous v2 version (see link below). It does >>>> several updates to virtio/vsock: >>>> 1) Changes 'virtio_transport_inc/dec_rx_pkt()' interface. Now instead of >>>> using skbuff state ('head' and 'data' pointers) to update 'fwd_cnt' >>>> and 'rx_bytes', integer value is passed as an input argument. This >>>> makes code more simple, because in this case we don't need to udpate >>>> skbuff state before calling 'virtio_transport_inc/dec_rx_pkt()'. In >>>> more common words - we don't need to change skbuff state to update >>>> 'rx_bytes' and 'fwd_cnt' correctly. >>>> 2) For SOCK_STREAM, when copying data to user fails, current skbuff is >>>> not dropped. Next read attempt will use same skbuff and last offset. >>>> Instead of 'skb_dequeue()', 'skb_peek()' + '__skb_unlink()' are used. >>>> This behaviour was implemented before skbuff support. >>>> 3) For SOCK_SEQPACKET it removes unneeded 'skb_pull()' call, because for >>>> this type of socket each skbuff is used only once: after removing it >>>> from socket's queue, it will be freed anyway. >>>> >>>> Test for 2) also added: >>>> Test tries to 'recv()' data to NULL buffer, then does 'recv()' with valid >>>> buffer. For SOCK_STREAM second 'recv()' must return data, because skbuff >>>> must not be dropped, but for SOCK_SEQPACKET skbuff will be dropped by >>>> kernel, and 'recv()' will return EAGAIN. >>>> >>>> Link to v1 on lore: >>>> https://lore.kernel.org/netdev/c2d3e204-89d9-88e9-8a15-3fe027e56b4b@sberdevices.ru/ >>>> >>>> Link to v2 on lore: >>>> https://lore.kernel.org/netdev/a7ab414b-5e41-c7b6-250b-e8401f335859@sberdevices.ru/ >>>> >>>> Change log: >>>> >>>> v1 -> v2: >>>> - For SOCK_SEQPACKET call 'skb_pull()' also in case of copy failure or >>>> dropping skbuff (when we just waiting message end). >>>> - Handle copy failure for SOCK_STREAM in the same manner (plus free >>>> current skbuff). >>>> - Replace bug repdroducer with new test in vsock_test.c >>>> >>>> v2 -> v3: >>>> - Replace patch which removes 'skb->len' subtraction from function >>>> 'virtio_transport_dec_rx_pkt()' with patch which updates functions >>>> 'virtio_transport_inc/dec_rx_pkt()' by passing integer argument >>>> instead of skbuff pointer. >>>> - Replace patch which drops skbuff when copying to user fails with >>>> patch which changes this behaviour by keeping skbuff in queue until >>>> it has no data. >>>> - Add patch for SOCK_SEQPACKET which removes redundant 'skb_pull()' >>>> call on read. >>>> - I remove "Fixes" tag from all patches, because all of them now change >>>> code logic, not only fix something. >>> >>> Yes, but they solve the problem, so we should use the tag (I think at >>> least in patch 1 and 3). >>> >>> We usually use the tag when we are fixing a problem introduced by a >>> previous change. So we need to backport the patch to the stable branches >>> as well, and we need the tag to figure out which branches have the patch >>> or not. >> Ahh, sorry. Ok. I see now :) > > No problem at all :-) > > I think also patch 2 can have the Fixes tag. > Done, fixed everything in v4. Thanks, Arseniy > Thanks, > Stefano > >> >> Thanks, Arseniy >>> >>> Thanks, >>> Stefano >>> >>>> >>>> Arseniy Krasnov (4): >>>> virtio/vsock: don't use skbuff state to account credit >>>> virtio/vsock: remove redundant 'skb_pull()' call >>>> virtio/vsock: don't drop skbuff on copy failure >>>> test/vsock: copy to user failure test >>>> >>>> net/vmw_vsock/virtio_transport_common.c | 29 +++--- >>>> tools/testing/vsock/vsock_test.c | 118 ++++++++++++++++++++++++ >>>> 2 files changed, 131 insertions(+), 16 deletions(-) >>>> >>>> -- >>>> 2.25.1 >>>> >>> >> >