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 KbivF4raHVuUdgAAmS7hNA ; Mon, 11 Jun 2018 02:12:26 +0000 Received: by smtp.codeaurora.org (Postfix, from userid 1000) id 51365607A4; Mon, 11 Jun 2018 02:12:26 +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 B9609601D2; Mon, 11 Jun 2018 02:12:25 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org B9609601D2 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 S1753918AbeFKCMX (ORCPT + 21 others); Sun, 10 Jun 2018 22:12:23 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:48852 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1753886AbeFKCMV (ORCPT ); Sun, 10 Jun 2018 22:12:21 -0400 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.rdu2.redhat.com [10.11.54.5]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id ECC157D65F; Mon, 11 Jun 2018 02:12:20 +0000 (UTC) Received: from redhat.com (ovpn-120-77.rdu2.redhat.com [10.10.120.77]) by smtp.corp.redhat.com (Postfix) with ESMTP id 94096ED155; Mon, 11 Jun 2018 02:12:20 +0000 (UTC) Date: Mon, 11 Jun 2018 05:12:20 +0300 From: "Michael S. Tsirkin" To: Jason Wang Cc: kvm@vger.kernel.org, virtualization@lists.linux-foundation.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH net] vhost_net: remove VHOST_NET_F_VIRTIO_NET_HDR support Message-ID: <20180611050741-mutt-send-email-mst@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> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <23efe110-f61a-5aee-c0b4-bd3dc5426438@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.11.54.5 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.2]); Mon, 11 Jun 2018 02:12:21 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.2]); Mon, 11 Jun 2018 02:12:21 +0000 (UTC) for IP:'10.11.54.5' DOMAIN:'int-mx05.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'mst@redhat.com' RCPT:'' Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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. > > > > > > > > > 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. > > > > > 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. > > > > > 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: "Michael S. Tsirkin" Subject: Re: [PATCH net] vhost_net: remove VHOST_NET_F_VIRTIO_NET_HDR support Date: Mon, 11 Jun 2018 05:12:20 +0300 Message-ID: <20180611050741-mutt-send-email-mst@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> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org, kvm@vger.kernel.org, virtualization@lists.linux-foundation.org To: Jason Wang Return-path: Content-Disposition: inline In-Reply-To: <23efe110-f61a-5aee-c0b4-bd3dc5426438@redhat.com> 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 T24gRnJpLCBKdW4gMDgsIDIwMTggYXQgMDE6MDc6MDlQTSArMDgwMCwgSmFzb24gV2FuZyB3cm90 ZToKPiAKPiAKPiBPbiAyMDE45bm0MDbmnIgwOOaXpSAxMjo0NiwgTWljaGFlbCBTLiBUc2lya2lu IHdyb3RlOgo+ID4gT24gRnJpLCBKdW4gMDgsIDIwMTggYXQgMTE6NTA6NDJBTSArMDgwMCwgSmFz b24gV2FuZyB3cm90ZToKPiA+ID4gVGhpcyBmZWF0dXJlIGJpdCBpcyBkdXBsaWNhdGVkIHdpdGgg VklSVElPX0ZfQU5ZX0xBWU9VVCwgdGhpcyBtZWFucyBpZgo+ID4gPiBhIHVzZXJwc2FjZSB3YW50 IHRvIGVuYWJsZSBWUklUSU9fRl9BTllfTEFZT1VULAo+ID4gPiBWSE9TVF9ORVRfRl9WSVJUSU9f TkVUX0hEUiB3aWxsIGJlIGltcGxpZWQgdG9vLiBUaGlzIGlzIHdyb25nIGFuZCB3aWxsCj4gPiA+ IGJyZWFrIG5ldHdvcmtpbmcuCj4gPiBXaGF0IGJyZWFrcyBuZXR3b3JraW5nIGV4YWN0bHk/IFZI T1NUX05FVCBzdXBwb3J0ZWQgQU5ZX0xBWU9VVAo+ID4gZnJvbSBkYXkgb25lLiBGb3IgdGhpcyBy ZWFzb24gaXQgZG9lcyBub3QgbmVlZCB0byBrbm93IGFib3V0Cj4gPiBWUklUSU9fRl9BTllfTEFZ T1VUIGFuZCB3ZSByZXVzZWQgdGhlIGJpdCBmb3Igb3RoZXIgcHVycG9zZXMuCj4gCj4gSXQncyB0 aGUga25vd2xlZGdlIG9mIHZob3N0X25ldCBjb2RlIGl0IHNlbGYgYnV0IG5vdCB1c2Vyc3BhY2Uu IEZvcgo+IHVzZXJzcGFjZSwgaXQgc2hvdWxkIGRlcGVuZHMgb24gdGhlIHZhbHVlIG9mIHJldHVy bmVkIGJ5IFZIT1NUX0dFVF9GRUFUVVJFUy4KPiBTbyB3aGVuIHVzZXJzcGFjZSBjYW4gc2V0X2Zl YXR1cmVzIHdpdGggQU5ZX0xBWU9VVCwgdmhvc3QgbWF5IHRoaW5rIGl0IHdhbnRzCj4gVkhPU1Rf TkVUX0ZfVklSVElPX05FVF9IRFIuCgpZZXMgYnV0IHRoYXQncyB0aGUgYWRtaXR0ZWRseSB1Z2x5 IEFQSSB0aGF0IHdlIGhhdmUgbm93Lgp1c2Vyc3BhY2UgaXMgc3VwcG9zZWQgdG8ga25vdyBWUklU SU9fRl9BTllfTEFZT1VUIGRvZXMKbm90IG1ha2Ugc2Vuc2UgZm9yIHZob3N0LgoKCgo+ID4gCj4g PiAKPiA+IAo+ID4gPiBGaXhpbmcgdGhpcyBieSBzYWZlbHkgcmVtb3ZpbmcKPiA+ID4gVkhPU1Rf TkVUX0ZfVklSVElPX05FVF9IRFIgc3VwcG9ydC4gVGhlcmUgc2hvdWxkIGJlIHZlcnkgZmV3IG9y IGV2ZW4KPiA+ID4gbm8gdXNlcnNwYWNlIGNhbiB1c2UgdGhpcy4KPiA+IFF1aXRlIHBvc3NpYmx5 LCBidXQgaXQgaXMgaGFyZCB0byBiZSBzdXJlLiBJdCBzZWVtcyBzYWZlciB0bwo+ID4gbWFpbnRh aW4gaXQgdW5sZXNzIHRoZXJlJ3MgYW4gYWN0dWFsIHJlYXNvbiBzb21ldGhpbmcncyBicm9rZW4u Cj4gCj4gSSB0aGluayBub3Qgc2luY2UgdGhlIGZlYXR1cmUgaXMgbmVnb3RpYXRlZCBub3QgbWFu ZGF0b3J5PwoKVGhhdCBkb2Vzbid0IG1lYW4gbXVjaC4KCj4gPiAKPiA+ID4gRnVydGhlciBjbGVh bnVwcyBjb3VsZCBiZSBkb25lIGZvcgo+ID4gPiAtbmV0LW5leHQgZm9yIHNhZmV0eS4KPiA+ID4g Cj4gPiA+IEluIHRoZSBmdXR1cmUsIHdlIG5lZWQgYSB2aG9zdCBkZWRpY2F0ZWQgZmVhdHVyZSBz ZXQvZ2V0IGlvY3RsKCkKPiA+ID4gaW5zdGVhZCBvZiByZXVzaW5nIHZpcnRpbyBvbmVzLgo+ID4g Tm90IGp1c3QgaW4gdGhlIGZ1dHVyZSwgd2UgbWlnaHQgd2FudCB0byBzd2l0Y2ggaW9tbXUKPiA+ IHRvIGEgc2FuZSBzdHJ1Y3R1cmUgd2l0aG91dCB0aGUgNjQgYml0IHBhZGRpbmcgYnVnCj4gPiBy aWdodCBub3cuCj4gCj4gWWVzLCBJIGhpdCB0aGlzIGJ1ZyB3aGVuIGludHJvZHVjaW5nIFYyIG9m IG1zZyBJT1RMQiBtZXNzYWdlLgoKU291bmRzIGdvb2QsIHNvIGlmIHlvdSBsaWtlLCByZXNlcnZl IGEgYml0IGZvcgpWSE9TVF9ORVRfRl9WSVJUSU9fTkVUX0hEUiBpbiB0aGUgbmV3IGlvY3RsIG1h c2sgYW5kCmRvIG5vdCBlbmFibGUgaXQgdGhlcmUuCgo+ID4gCj4gPiA+IEZpeGVzOiA0ZTlmYTUw YzZjY2JlICgidmhvc3Q6IG1vdmUgZmVhdHVyZXMgdG8gY29yZSIpCj4gPiBUaGlzIHRhZyBtYWtl cyBubyBzZW5zZSBoZXJlIElNSE8uIExvb2tzIGxpa2UgcGVvcGxlIGFyZSB1c2luZyBzb21lIHRv b2wKPiA+IHRoYXQganVzdCBsb29rcyBhdCB0aGUgZWFybGllc3QgdmVyc2lvbiB3aGVyZSBwYXRj aCB3b24ndCBhcHBseS4gVGhlCj4gPiBjb21taXQgaW4gcXVlc3Rpb24ganVzdCBtb3ZlZCBzb21l IGNvZGUgYXJvdW5kLgo+IAo+IExvb2tzIG5vdCwgYmVmb3JlIHRoaXMgY29tbWl0LCB2aG9zdF9u ZXQgd29uJ3QgcmV0dXJuIEFOWV9MQVlPVVQuCj4gCj4gVGhhbmtzCgpXZWxsIEFOWV9MQVlPVVQg anVzdCBoYXBwZW5zIHRvIGJlIHNhbWUgYXMgVkhPU1RfTkVUX0ZfVklSVElPX05FVF9IRFIKYW5k IHRoYXQgaGFzIGJlZW4gc2V0IHNpbmNlIGZvcmV2ZXIuCgo+ID4gCj4gPiA+IFNpZ25lZC1vZmYt Ynk6IEphc29uIFdhbmcgPGphc293YW5nQHJlZGhhdC5jb20+Cj4gPiA+IC0tLQo+ID4gPiAgIGRy aXZlcnMvdmhvc3QvbmV0LmMgfCAxNSArKysrKy0tLS0tLS0tLS0KPiA+ID4gICAxIGZpbGUgY2hh bmdlZCwgNSBpbnNlcnRpb25zKCspLCAxMCBkZWxldGlvbnMoLSkKPiA+ID4gCj4gPiA+IGRpZmYg LS1naXQgYS9kcml2ZXJzL3Zob3N0L25ldC5jIGIvZHJpdmVycy92aG9zdC9uZXQuYwo+ID4gPiBp bmRleCA5ODYwNThhLi44M2VlZjUyIDEwMDY0NAo+ID4gPiAtLS0gYS9kcml2ZXJzL3Zob3N0L25l dC5jCj4gPiA+ICsrKyBiL2RyaXZlcnMvdmhvc3QvbmV0LmMKPiA+ID4gQEAgLTY5LDcgKzY5LDYg QEAgTU9EVUxFX1BBUk1fREVTQyhleHBlcmltZW50YWxfemNvcHl0eCwgIkVuYWJsZSBaZXJvIENv cHkgVFg7Igo+ID4gPiAgIGVudW0gewo+ID4gPiAgIAlWSE9TVF9ORVRfRkVBVFVSRVMgPSBWSE9T VF9GRUFUVVJFUyB8Cj4gPiA+IC0JCQkgKDFVTEwgPDwgVkhPU1RfTkVUX0ZfVklSVElPX05FVF9I RFIpIHwKPiA+ID4gICAJCQkgKDFVTEwgPDwgVklSVElPX05FVF9GX01SR19SWEJVRikgfAo+ID4g PiAgIAkJCSAoMVVMTCA8PCBWSVJUSU9fRl9JT01NVV9QTEFURk9STSkKPiA+ID4gICB9Owo+ID4g PiBAQCAtMTI1NSwxNSArMTI1NCwxMSBAQCBzdGF0aWMgaW50IHZob3N0X25ldF9zZXRfZmVhdHVy ZXMoc3RydWN0IHZob3N0X25ldCAqbiwgdTY0IGZlYXR1cmVzKQo+ID4gPiAgIAkJCSAgICAgICAo MVVMTCA8PCBWSVJUSU9fRl9WRVJTSU9OXzEpKSkgPwo+ID4gPiAgIAkJCXNpemVvZihzdHJ1Y3Qg dmlydGlvX25ldF9oZHJfbXJnX3J4YnVmKSA6Cj4gPiA+ICAgCQkJc2l6ZW9mKHN0cnVjdCB2aXJ0 aW9fbmV0X2hkcik7Cj4gPiA+IC0JaWYgKGZlYXR1cmVzICYgKDEgPDwgVkhPU1RfTkVUX0ZfVklS VElPX05FVF9IRFIpKSB7Cj4gPiA+IC0JCS8qIHZob3N0IHByb3ZpZGVzIHZuZXRfaGRyICovCj4g PiA+IC0JCXZob3N0X2hsZW4gPSBoZHJfbGVuOwo+ID4gPiAtCQlzb2NrX2hsZW4gPSAwOwo+ID4g PiAtCX0gZWxzZSB7Cj4gPiA+IC0JCS8qIHNvY2tldCBwcm92aWRlcyB2bmV0X2hkciAqLwo+ID4g PiAtCQl2aG9zdF9obGVuID0gMDsKPiA+ID4gLQkJc29ja19obGVuID0gaGRyX2xlbjsKPiA+ID4g LQl9Cj4gPiA+ICsKPiA+ID4gKyAgICAgICAgLyogc29ja2V0IHByb3ZpZGVzIHZuZXRfaGRyICov Cj4gPiA+ICsJdmhvc3RfaGxlbiA9IDA7Cj4gPiA+ICsJc29ja19obGVuID0gaGRyX2xlbjsKPiA+ ID4gKwo+ID4gPiAgIAltdXRleF9sb2NrKCZuLT5kZXYubXV0ZXgpOwo+ID4gPiAgIAlpZiAoKGZl YXR1cmVzICYgKDEgPDwgVkhPU1RfRl9MT0dfQUxMKSkgJiYKPiA+ID4gICAJICAgICF2aG9zdF9s b2dfYWNjZXNzX29rKCZuLT5kZXYpKQo+ID4gPiAtLSAKPiA+ID4gMi43LjQKX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KVmlydHVhbGl6YXRpb24gbWFpbGlu ZyBsaXN0ClZpcnR1YWxpemF0aW9uQGxpc3RzLmxpbnV4LWZvdW5kYXRpb24ub3JnCmh0dHBzOi8v bGlzdHMubGludXhmb3VuZGF0aW9uLm9yZy9tYWlsbWFuL2xpc3RpbmZvL3ZpcnR1YWxpemF0aW9u