From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.codeaurora.org by pdx-caf-mail.web.codeaurora.org (Dovecot) with LMTP id GgkDMJneHVuQKAAAmS7hNA ; Mon, 11 Jun 2018 02:29:48 +0000 Received: by smtp.codeaurora.org (Postfix, from userid 1000) id 85E7D607BB; Mon, 11 Jun 2018 02:29:48 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on pdx-caf-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.9 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI autolearn=ham autolearn_force=no version=3.4.0 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by smtp.codeaurora.org (Postfix) with ESMTP id E78E7605A5; Mon, 11 Jun 2018 02:29:47 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org E78E7605A5 Authentication-Results: pdx-caf-mail.web.codeaurora.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: pdx-caf-mail.web.codeaurora.org; spf=none smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753923AbeFKC3p (ORCPT + 21 others); Sun, 10 Jun 2018 22:29:45 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:53260 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1753793AbeFKC3o (ORCPT ); Sun, 10 Jun 2018 22:29:44 -0400 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id A8F3EBB40A; Mon, 11 Jun 2018 02:29:43 +0000 (UTC) Received: from [10.72.12.98] (ovpn-12-98.pek2.redhat.com [10.72.12.98]) by smtp.corp.redhat.com (Postfix) with ESMTPS id D602420244E0; Mon, 11 Jun 2018 02:29:39 +0000 (UTC) Subject: Re: [PATCH net] vhost_net: remove VHOST_NET_F_VIRTIO_NET_HDR support To: "Michael S. Tsirkin" Cc: kvm@vger.kernel.org, virtualization@lists.linux-foundation.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org References: <1528429842-22835-1-git-send-email-jasowang@redhat.com> <20180608074115-mutt-send-email-mst@kernel.org> <23efe110-f61a-5aee-c0b4-bd3dc5426438@redhat.com> <20180611050741-mutt-send-email-mst@kernel.org> From: Jason Wang Message-ID: <90e9b3ea-e9d2-d60d-8355-42d447c67452@redhat.com> Date: Mon, 11 Jun 2018 10:29:36 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0 MIME-Version: 1.0 In-Reply-To: <20180611050741-mutt-send-email-mst@kernel.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US X-Scanned-By: MIMEDefang 2.78 on 10.11.54.4 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.1]); Mon, 11 Jun 2018 02:29:43 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.1]); Mon, 11 Jun 2018 02:29:43 +0000 (UTC) for IP:'10.11.54.4' DOMAIN:'int-mx04.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'jasowang@redhat.com' RCPT:'' Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2018年06月11日 10:12, Michael S. Tsirkin wrote: > On Fri, Jun 08, 2018 at 01:07:09PM +0800, Jason Wang wrote: >> >> On 2018年06月08日 12:46, Michael S. Tsirkin wrote: >>> On Fri, Jun 08, 2018 at 11:50:42AM +0800, Jason Wang wrote: >>>> This feature bit is duplicated with VIRTIO_F_ANY_LAYOUT, this means if >>>> a userpsace want to enable VRITIO_F_ANY_LAYOUT, >>>> VHOST_NET_F_VIRTIO_NET_HDR will be implied too. This is wrong and will >>>> break networking. >>> What breaks networking exactly? VHOST_NET supported ANY_LAYOUT >>> from day one. For this reason it does not need to know about >>> VRITIO_F_ANY_LAYOUT and we reused the bit for other purposes. >> It's the knowledge of vhost_net code it self but not userspace. For >> userspace, it should depends on the value of returned by VHOST_GET_FEATURES. >> So when userspace can set_features with ANY_LAYOUT, vhost may think it wants >> VHOST_NET_F_VIRTIO_NET_HDR. > Yes but that's the admittedly ugly API that we have now. > userspace is supposed to know VRITIO_F_ANY_LAYOUT does > not make sense for vhost. Ok. > > > >>> >>> >>>> Fixing this by safely removing >>>> VHOST_NET_F_VIRTIO_NET_HDR support. There should be very few or even >>>> no userspace can use this. >>> Quite possibly, but it is hard to be sure. It seems safer to >>> maintain it unless there's an actual reason something's broken. >> I think not since the feature is negotiated not mandatory? > That doesn't mean much. > >>>> Further cleanups could be done for >>>> -net-next for safety. >>>> >>>> In the future, we need a vhost dedicated feature set/get ioctl() >>>> instead of reusing virtio ones. >>> Not just in the future, we might want to switch iommu >>> to a sane structure without the 64 bit padding bug >>> right now. >> Yes, I hit this bug when introducing V2 of msg IOTLB message. > Sounds good, so if you like, reserve a bit for > VHOST_NET_F_VIRTIO_NET_HDR in the new ioctl mask and > do not enable it there. Ok, and maybe VHOST_F_LOG_ALL. > >>>> Fixes: 4e9fa50c6ccbe ("vhost: move features to core") >>> This tag makes no sense here IMHO. Looks like people are using some tool >>> that just looks at the earliest version where patch won't apply. The >>> commit in question just moved some code around. >> Looks not, before this commit, vhost_net won't return ANY_LAYOUT. >> >> Thanks > Well ANY_LAYOUT just happens to be same as VHOST_NET_F_VIRTIO_NET_HDR > and that has been set since forever. So do you still want this patch? If not we need to document that ANY_LAYOUT could not be passed through SET_FEATURES somewhere. Thanks > >>>> Signed-off-by: Jason Wang >>>> --- >>>> drivers/vhost/net.c | 15 +++++---------- >>>> 1 file changed, 5 insertions(+), 10 deletions(-) >>>> >>>> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c >>>> index 986058a..83eef52 100644 >>>> --- a/drivers/vhost/net.c >>>> +++ b/drivers/vhost/net.c >>>> @@ -69,7 +69,6 @@ MODULE_PARM_DESC(experimental_zcopytx, "Enable Zero Copy TX;" >>>> enum { >>>> VHOST_NET_FEATURES = VHOST_FEATURES | >>>> - (1ULL << VHOST_NET_F_VIRTIO_NET_HDR) | >>>> (1ULL << VIRTIO_NET_F_MRG_RXBUF) | >>>> (1ULL << VIRTIO_F_IOMMU_PLATFORM) >>>> }; >>>> @@ -1255,15 +1254,11 @@ static int vhost_net_set_features(struct vhost_net *n, u64 features) >>>> (1ULL << VIRTIO_F_VERSION_1))) ? >>>> sizeof(struct virtio_net_hdr_mrg_rxbuf) : >>>> sizeof(struct virtio_net_hdr); >>>> - if (features & (1 << VHOST_NET_F_VIRTIO_NET_HDR)) { >>>> - /* vhost provides vnet_hdr */ >>>> - vhost_hlen = hdr_len; >>>> - sock_hlen = 0; >>>> - } else { >>>> - /* socket provides vnet_hdr */ >>>> - vhost_hlen = 0; >>>> - sock_hlen = hdr_len; >>>> - } >>>> + >>>> + /* socket provides vnet_hdr */ >>>> + vhost_hlen = 0; >>>> + sock_hlen = hdr_len; >>>> + >>>> mutex_lock(&n->dev.mutex); >>>> if ((features & (1 << VHOST_F_LOG_ALL)) && >>>> !vhost_log_access_ok(&n->dev)) >>>> -- >>>> 2.7.4 From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jason Wang Subject: Re: [PATCH net] vhost_net: remove VHOST_NET_F_VIRTIO_NET_HDR support Date: Mon, 11 Jun 2018 10:29:36 +0800 Message-ID: <90e9b3ea-e9d2-d60d-8355-42d447c67452@redhat.com> References: <1528429842-22835-1-git-send-email-jasowang@redhat.com> <20180608074115-mutt-send-email-mst@kernel.org> <23efe110-f61a-5aee-c0b4-bd3dc5426438@redhat.com> <20180611050741-mutt-send-email-mst@kernel.org> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; Format="flowed" Content-Transfer-Encoding: base64 Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org, kvm@vger.kernel.org, virtualization@lists.linux-foundation.org To: "Michael S. Tsirkin" Return-path: In-Reply-To: <20180611050741-mutt-send-email-mst@kernel.org> Content-Language: en-US List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: virtualization-bounces@lists.linux-foundation.org Errors-To: virtualization-bounces@lists.linux-foundation.org List-Id: netdev.vger.kernel.org CgpPbiAyMDE45bm0MDbmnIgxMeaXpSAxMDoxMiwgTWljaGFlbCBTLiBUc2lya2luIHdyb3RlOgo+ IE9uIEZyaSwgSnVuIDA4LCAyMDE4IGF0IDAxOjA3OjA5UE0gKzA4MDAsIEphc29uIFdhbmcgd3Jv dGU6Cj4+Cj4+IE9uIDIwMTjlubQwNuaciDA45pelIDEyOjQ2LCBNaWNoYWVsIFMuIFRzaXJraW4g d3JvdGU6Cj4+PiBPbiBGcmksIEp1biAwOCwgMjAxOCBhdCAxMTo1MDo0MkFNICswODAwLCBKYXNv biBXYW5nIHdyb3RlOgo+Pj4+IFRoaXMgZmVhdHVyZSBiaXQgaXMgZHVwbGljYXRlZCB3aXRoIFZJ UlRJT19GX0FOWV9MQVlPVVQsIHRoaXMgbWVhbnMgaWYKPj4+PiBhIHVzZXJwc2FjZSB3YW50IHRv IGVuYWJsZSBWUklUSU9fRl9BTllfTEFZT1VULAo+Pj4+IFZIT1NUX05FVF9GX1ZJUlRJT19ORVRf SERSIHdpbGwgYmUgaW1wbGllZCB0b28uIFRoaXMgaXMgd3JvbmcgYW5kIHdpbGwKPj4+PiBicmVh ayBuZXR3b3JraW5nLgo+Pj4gV2hhdCBicmVha3MgbmV0d29ya2luZyBleGFjdGx5PyBWSE9TVF9O RVQgc3VwcG9ydGVkIEFOWV9MQVlPVVQKPj4+IGZyb20gZGF5IG9uZS4gRm9yIHRoaXMgcmVhc29u IGl0IGRvZXMgbm90IG5lZWQgdG8ga25vdyBhYm91dAo+Pj4gVlJJVElPX0ZfQU5ZX0xBWU9VVCBh bmQgd2UgcmV1c2VkIHRoZSBiaXQgZm9yIG90aGVyIHB1cnBvc2VzLgo+PiBJdCdzIHRoZSBrbm93 bGVkZ2Ugb2Ygdmhvc3RfbmV0IGNvZGUgaXQgc2VsZiBidXQgbm90IHVzZXJzcGFjZS4gRm9yCj4+ IHVzZXJzcGFjZSwgaXQgc2hvdWxkIGRlcGVuZHMgb24gdGhlIHZhbHVlIG9mIHJldHVybmVkIGJ5 IFZIT1NUX0dFVF9GRUFUVVJFUy4KPj4gU28gd2hlbiB1c2Vyc3BhY2UgY2FuIHNldF9mZWF0dXJl cyB3aXRoIEFOWV9MQVlPVVQsIHZob3N0IG1heSB0aGluayBpdCB3YW50cwo+PiBWSE9TVF9ORVRf Rl9WSVJUSU9fTkVUX0hEUi4KPiBZZXMgYnV0IHRoYXQncyB0aGUgYWRtaXR0ZWRseSB1Z2x5IEFQ SSB0aGF0IHdlIGhhdmUgbm93Lgo+IHVzZXJzcGFjZSBpcyBzdXBwb3NlZCB0byBrbm93IFZSSVRJ T19GX0FOWV9MQVlPVVQgZG9lcwo+IG5vdCBtYWtlIHNlbnNlIGZvciB2aG9zdC4KCk9rLgoKPgo+ Cj4KPj4+Cj4+Pgo+Pj4+IEZpeGluZyB0aGlzIGJ5IHNhZmVseSByZW1vdmluZwo+Pj4+IFZIT1NU X05FVF9GX1ZJUlRJT19ORVRfSERSIHN1cHBvcnQuIFRoZXJlIHNob3VsZCBiZSB2ZXJ5IGZldyBv ciBldmVuCj4+Pj4gbm8gdXNlcnNwYWNlIGNhbiB1c2UgdGhpcy4KPj4+IFF1aXRlIHBvc3NpYmx5 LCBidXQgaXQgaXMgaGFyZCB0byBiZSBzdXJlLiBJdCBzZWVtcyBzYWZlciB0bwo+Pj4gbWFpbnRh aW4gaXQgdW5sZXNzIHRoZXJlJ3MgYW4gYWN0dWFsIHJlYXNvbiBzb21ldGhpbmcncyBicm9rZW4u Cj4+IEkgdGhpbmsgbm90IHNpbmNlIHRoZSBmZWF0dXJlIGlzIG5lZ290aWF0ZWQgbm90IG1hbmRh dG9yeT8KPiBUaGF0IGRvZXNuJ3QgbWVhbiBtdWNoLgo+Cj4+Pj4gRnVydGhlciBjbGVhbnVwcyBj b3VsZCBiZSBkb25lIGZvcgo+Pj4+IC1uZXQtbmV4dCBmb3Igc2FmZXR5Lgo+Pj4+Cj4+Pj4gSW4g dGhlIGZ1dHVyZSwgd2UgbmVlZCBhIHZob3N0IGRlZGljYXRlZCBmZWF0dXJlIHNldC9nZXQgaW9j dGwoKQo+Pj4+IGluc3RlYWQgb2YgcmV1c2luZyB2aXJ0aW8gb25lcy4KPj4+IE5vdCBqdXN0IGlu IHRoZSBmdXR1cmUsIHdlIG1pZ2h0IHdhbnQgdG8gc3dpdGNoIGlvbW11Cj4+PiB0byBhIHNhbmUg c3RydWN0dXJlIHdpdGhvdXQgdGhlIDY0IGJpdCBwYWRkaW5nIGJ1Zwo+Pj4gcmlnaHQgbm93Lgo+ PiBZZXMsIEkgaGl0IHRoaXMgYnVnIHdoZW4gaW50cm9kdWNpbmcgVjIgb2YgbXNnIElPVExCIG1l c3NhZ2UuCj4gU291bmRzIGdvb2QsIHNvIGlmIHlvdSBsaWtlLCByZXNlcnZlIGEgYml0IGZvcgo+ IFZIT1NUX05FVF9GX1ZJUlRJT19ORVRfSERSIGluIHRoZSBuZXcgaW9jdGwgbWFzayBhbmQKPiBk byBub3QgZW5hYmxlIGl0IHRoZXJlLgoKT2ssIGFuZCBtYXliZSBWSE9TVF9GX0xPR19BTEwuCgo+ Cj4+Pj4gRml4ZXM6IDRlOWZhNTBjNmNjYmUgKCJ2aG9zdDogbW92ZSBmZWF0dXJlcyB0byBjb3Jl IikKPj4+IFRoaXMgdGFnIG1ha2VzIG5vIHNlbnNlIGhlcmUgSU1ITy4gTG9va3MgbGlrZSBwZW9w bGUgYXJlIHVzaW5nIHNvbWUgdG9vbAo+Pj4gdGhhdCBqdXN0IGxvb2tzIGF0IHRoZSBlYXJsaWVz dCB2ZXJzaW9uIHdoZXJlIHBhdGNoIHdvbid0IGFwcGx5LiBUaGUKPj4+IGNvbW1pdCBpbiBxdWVz dGlvbiBqdXN0IG1vdmVkIHNvbWUgY29kZSBhcm91bmQuCj4+IExvb2tzIG5vdCwgYmVmb3JlIHRo aXMgY29tbWl0LCB2aG9zdF9uZXQgd29uJ3QgcmV0dXJuIEFOWV9MQVlPVVQuCj4+Cj4+IFRoYW5r cwo+IFdlbGwgQU5ZX0xBWU9VVCBqdXN0IGhhcHBlbnMgdG8gYmUgc2FtZSBhcyBWSE9TVF9ORVRf Rl9WSVJUSU9fTkVUX0hEUgo+IGFuZCB0aGF0IGhhcyBiZWVuIHNldCBzaW5jZSBmb3JldmVyLgoK U28gZG8geW91IHN0aWxsIHdhbnQgdGhpcyBwYXRjaD8gSWYgbm90IHdlIG5lZWQgdG8gZG9jdW1l bnQgdGhhdCAKQU5ZX0xBWU9VVCBjb3VsZCBub3QgYmUgcGFzc2VkIHRocm91Z2ggU0VUX0ZFQVRV UkVTIHNvbWV3aGVyZS4KClRoYW5rcwoKPgo+Pj4+IFNpZ25lZC1vZmYtYnk6IEphc29uIFdhbmcg PGphc293YW5nQHJlZGhhdC5jb20+Cj4+Pj4gLS0tCj4+Pj4gICAgZHJpdmVycy92aG9zdC9uZXQu YyB8IDE1ICsrKysrLS0tLS0tLS0tLQo+Pj4+ICAgIDEgZmlsZSBjaGFuZ2VkLCA1IGluc2VydGlv bnMoKyksIDEwIGRlbGV0aW9ucygtKQo+Pj4+Cj4+Pj4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvdmhv c3QvbmV0LmMgYi9kcml2ZXJzL3Zob3N0L25ldC5jCj4+Pj4gaW5kZXggOTg2MDU4YS4uODNlZWY1 MiAxMDA2NDQKPj4+PiAtLS0gYS9kcml2ZXJzL3Zob3N0L25ldC5jCj4+Pj4gKysrIGIvZHJpdmVy cy92aG9zdC9uZXQuYwo+Pj4+IEBAIC02OSw3ICs2OSw2IEBAIE1PRFVMRV9QQVJNX0RFU0MoZXhw ZXJpbWVudGFsX3pjb3B5dHgsICJFbmFibGUgWmVybyBDb3B5IFRYOyIKPj4+PiAgICBlbnVtIHsK Pj4+PiAgICAJVkhPU1RfTkVUX0ZFQVRVUkVTID0gVkhPU1RfRkVBVFVSRVMgfAo+Pj4+IC0JCQkg KDFVTEwgPDwgVkhPU1RfTkVUX0ZfVklSVElPX05FVF9IRFIpIHwKPj4+PiAgICAJCQkgKDFVTEwg PDwgVklSVElPX05FVF9GX01SR19SWEJVRikgfAo+Pj4+ICAgIAkJCSAoMVVMTCA8PCBWSVJUSU9f Rl9JT01NVV9QTEFURk9STSkKPj4+PiAgICB9Owo+Pj4+IEBAIC0xMjU1LDE1ICsxMjU0LDExIEBA IHN0YXRpYyBpbnQgdmhvc3RfbmV0X3NldF9mZWF0dXJlcyhzdHJ1Y3Qgdmhvc3RfbmV0ICpuLCB1 NjQgZmVhdHVyZXMpCj4+Pj4gICAgCQkJICAgICAgICgxVUxMIDw8IFZJUlRJT19GX1ZFUlNJT05f MSkpKSA/Cj4+Pj4gICAgCQkJc2l6ZW9mKHN0cnVjdCB2aXJ0aW9fbmV0X2hkcl9tcmdfcnhidWYp IDoKPj4+PiAgICAJCQlzaXplb2Yoc3RydWN0IHZpcnRpb19uZXRfaGRyKTsKPj4+PiAtCWlmIChm ZWF0dXJlcyAmICgxIDw8IFZIT1NUX05FVF9GX1ZJUlRJT19ORVRfSERSKSkgewo+Pj4+IC0JCS8q IHZob3N0IHByb3ZpZGVzIHZuZXRfaGRyICovCj4+Pj4gLQkJdmhvc3RfaGxlbiA9IGhkcl9sZW47 Cj4+Pj4gLQkJc29ja19obGVuID0gMDsKPj4+PiAtCX0gZWxzZSB7Cj4+Pj4gLQkJLyogc29ja2V0 IHByb3ZpZGVzIHZuZXRfaGRyICovCj4+Pj4gLQkJdmhvc3RfaGxlbiA9IDA7Cj4+Pj4gLQkJc29j a19obGVuID0gaGRyX2xlbjsKPj4+PiAtCX0KPj4+PiArCj4+Pj4gKyAgICAgICAgLyogc29ja2V0 IHByb3ZpZGVzIHZuZXRfaGRyICovCj4+Pj4gKwl2aG9zdF9obGVuID0gMDsKPj4+PiArCXNvY2tf aGxlbiA9IGhkcl9sZW47Cj4+Pj4gKwo+Pj4+ICAgIAltdXRleF9sb2NrKCZuLT5kZXYubXV0ZXgp Owo+Pj4+ICAgIAlpZiAoKGZlYXR1cmVzICYgKDEgPDwgVkhPU1RfRl9MT0dfQUxMKSkgJiYKPj4+ PiAgICAJICAgICF2aG9zdF9sb2dfYWNjZXNzX29rKCZuLT5kZXYpKQo+Pj4+IC0tIAo+Pj4+IDIu Ny40CgpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwpWaXJ0 dWFsaXphdGlvbiBtYWlsaW5nIGxpc3QKVmlydHVhbGl6YXRpb25AbGlzdHMubGludXgtZm91bmRh dGlvbi5vcmcKaHR0cHM6Ly9saXN0cy5saW51eGZvdW5kYXRpb24ub3JnL21haWxtYW4vbGlzdGlu Zm8vdmlydHVhbGl6YXRpb24=