From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jason Wang Subject: Re: [PATCH RFC] virtio_net: fix error handling for mergeable buffers Date: Thu, 21 Nov 2013 11:27:44 +0800 Message-ID: <528D7DB0.8030101@redhat.com> References: <1384938447-3775-1-git-send-email-jasowang@redhat.com> <20131120132657.GA8455@redhat.com> <125586345.27477452.1384955642143.JavaMail.root@redhat.com> <20131120142032.GA13492@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <20131120142032.GA13492@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 To: "Michael S. Tsirkin" Cc: Michael Dalton , netdev@vger.kernel.org, linux-kernel@vger.kernel.org, virtualization@lists.linux-foundation.org, Eric Dumazet , Shirley Ma List-Id: virtualization@lists.linuxfoundation.org T24gMTEvMjAvMjAxMyAxMDoyMCBQTSwgTWljaGFlbCBTLiBUc2lya2luIHdyb3RlOgo+IE9uIFdl ZCwgTm92IDIwLCAyMDEzIGF0IDA4OjU0OjAyQU0gLTA1MDAsIEphc29uIFdhbmcgd3JvdGU6Cj4+ Cj4+IC0tLS0tIOWOn+Wni+mCruS7tiAtLS0tLQo+Pj4gT24gV2VkLCBOb3YgMjAsIDIwMTMgYXQg MDU6MDc6MjVQTSArMDgwMCwgSmFzb24gV2FuZyB3cm90ZToKPj4+PiBXaGVuIG1lcmdlYWJsZSBi dWZmZXIgd2VyZSB1c2VkLCB3ZSBvbmx5IHB1dCB0aGUgZmlyc3QgcGFnZSBidWYgbGVhdmUgdGhl Cj4+Pj4gcmVzdAo+Pj4+IG9mIGJ1ZmZlcnMgaW4gdGhlIHZpcnQgcXVldWUuIFRoaXMgd2lsbCBj YXVzZSB0aGUgZHJpdmVyIGNvdWxkIG5vdCBnZXQgdGhlCj4+Pj4gY29ycmVjdCBoZWFkIGJ1ZmZl ciBhbnkgbW9yZS4gRml4IHRoaXMgYnkgZHJvcHBpbmcgdGhlIHJlc3Qgb2YgYnVmZmVycyBmb3IK Pj4+PiB0aGlzCj4+Pj4gcGFja2V0Lgo+Pj4+Cj4+Pj4gVGhlIGJ1ZyB3YXMgaW50cm9kdWNlZCBi eSBjb21taXQgOWFiODZiYmNmOGJlNzU1MjU2ZjBhNWU5OTRlMGIzOGFmNmI0ZDM5OQo+Pj4+ICh2 aXJ0aW9fbmV0OiBEZWZlciBza2IgYWxsb2NhdGlvbiBpbiByZWNlaXZlIHBhdGgpLgo+Pj4+Cj4+ Pj4gQ2M6IFJ1c3R5IFJ1c3NlbGwgPHJ1c3R5QHJ1c3Rjb3JwLmNvbS5hdT4KPj4+PiBDYzogTWlj aGFlbCBTLiBUc2lya2luIDxtc3RAcmVkaGF0LmNvbT4KPj4+PiBDYzogTWljaGFlbCBEYWx0b24g PG13ZGFsdG9uQGdvb2dsZS5jb20+Cj4+Pj4gQ2M6IEVyaWMgRHVtYXpldCA8ZWR1bWF6ZXRAZ29v Z2xlLmNvbT4KPj4+PiBDYzogU2hpcmxleSBNYSA8eG1hQHVzLmlibS5jb20+Cj4+Pj4gU2lnbmVk LW9mZi1ieTogSmFzb24gV2FuZyA8amFzb3dhbmdAcmVkaGF0LmNvbT4KPj4+IEp1c3QgdG8gY2xh cmlmeSBteSBwcmV2aW91cyBjb21tZW50OiBpdCB3YXMgbm90IGFib3V0IHRoZQo+Pj4gaWRlYSBv ZiBhZGRpbmcgZHJvcF9tZXJnZWFibGVfYnVmZmVyIC0gcmF0aGVyLCBJIHRoaW5rIHRoYXQKPj4+ IGFkZGluZyBrbm93bGVkZ2UgYWJvdXQgbWVyZ2VhYmxlIGJ1ZmZlcnMgaW50byBwYWdlX3RvX3Nr YiBjcmVhdGVzIGFuCj4+PiB1Z2x5IGludGVybmFsIEFQSS4KPj4+Cj4+PiBMZXQncyBtb3ZlIHRo ZSBjYWxsIHRvIHBhZ2VfdG9fc2tiIHdpdGhpbiByZWNlaXZlX21lcmdlYWJsZSBpbnN0ZWFkOgo+ Pj4gaXQncyBhbHNvIG5pY2UgdGhhdCBpbnQgb2Zmc2V0ID0gYnVmIC0gcGFnZV9hZGRyZXNzKHBh Z2UpIGxvZ2ljCj4+PiBpcyBub3Qgc3ByZWFkIGFyb3VuZCBsaWtlIGl0IHdhcy4KPj4+Cj4+PiBB bHNvLCBpdCdzIG5vdCBuaWNlIHRoYXQgd2UgaWdub3JlIGxlbmd0aCBlcnJvcnMgd2hlbiB3ZSBk cm9wCj4+PiBwYWNrZXRzIGJlY2F1c2Ugb2YgT09NLgo+Pj4KPj4+IFNvIEkgY2FtZSB1cCB3aXRo IHRoZSBmb2xsb3dpbmcgLSBpdCBzZWVtcyB0byB3b3JrIGJ1dCBJIGRpZG4ndAo+Pj4gc3RyZXNz IHRlc3QgeWV0Lgo+PiBJJ3ZlIG5vIG9iamVjdGlvbiBvbiB0aGlzLiBCdXQgSSd2ZSByYXRoZXIg bGlrZSBteSBzbWFsbCBhbmQgZGlyZWN0IHBhdGNoIAo+PiB0byBiZSBhcHBsaWVkIHRvIC1uZXQg Zmlyc3QuIEl0IGhhcyBsb3dlciByaXNrIGFuZCB3YXMgbXVjaCBtb3JlIGVhc2llciB0byAKPj4g YmUgYmFja3BvcnRlZCB0byBzdGFibGUgdHJlZXMuIFRoZW4gd2UgY2FuIGRvIHRoZSByZS1mYWN0 b3IgbGlrZSB0aGlzIGluIAo+PiBuZXQtbmV4dC4gCj4gSXQgbWFrZXMgdGhlIGludGVyZmFjZXMg dG9vIG1lc3N5LiAKCkRvIHlvdSBtZWFuIHJlY2VpdmVfbWVyZ2VhYmxlKCkgc2hvdWxkIGNhbGwg cGFnZV90b19za2IoKT8gSXQgaGFzIGJlZW4KdGhlcmUgc2V2ZXJhbCB5ZWFycy4gQW5kIGV2ZW4g d2l0aCB5b3VyIGNoYW5nZXMsIGZvciBiaWcgYW5kIHNtYWxsCnBhY2tldHMsIHBhZ2VfdG9fc2ti KCkgd2VyZSBzdGlsbCBjYWxsZWQgZGlyZWN0bHkgcmVjZWl2ZV9idWYoKSBhbmQgdGhlCmVycm9y IGhhbmRsaW5nIHdlcmUgZG9uZSB0aGVyZS4KPiBXZSBhcmUgbm90IGluIGNvZGUgZnJlZXplIGlu IC1uZXQgLQo+IG9ubHkgZmVhdHVyZSBmcmVlemUsIHNvIG5vIHJlYXNvbiB0byBtYWtlIGNvZGUg bGlrZSBzcGFnZXR0eSwKPiBhbmQgaXQncyBvbmx5IDI1IGxpbmVzIGNoYW5nZWQgYXMgY29tcGFy ZWQgdG8gNDAuCj4gSXQncyBub3QgYSBodWdlIHJlZmFjdG9yaW5nLgoKVGhlIHByb2JsZW0gaXMg eW91ciBwYXRjaCBtaXhlcyBzZXZlcmFsIGJ1Z3MgZml4aW5nIHdpdGggcmUtZmFjdG9yaW5nLApv bmx5IG9uZSBvZiB0aGUgYnVnIGZpeGluZyB3YXMgcmVhbGx5IG5lZWRlZCBmb3Igc3RhYmxlLiBB dCBsZWFzdCB3ZQpzaG91bGQgbWFrZSBpbmNyZW1lbnRhbCBwYXRjaGVzIG9uIHRoaXMuCj4KPiBJ dCdzIGp1c3QgYXMgZWFzeSB0byBiYWNrcG9ydCBteSBwYXRjaCB0b28uCj4gWW91IGp1c3QgZHJv cCB0aGUgZ290byBpbiB0aGUgbmV3IGNvZGUgcGF0aCB3ZSBhZGRlZC4KCkl0IG1heSBub3QgYmUg c28gZWFzeSBmb3IgdGhlIHBlb3BsZSB3aG8gYXJlIG5vdCBmYW1pbGlhciB3aXRoCnZpcnRpby1u ZXQgYW5kIHRoZSBzL3NrYi0+ZGV2L2Rldi9nIGNoYW5nZXMgbG9va3MgaXJyZWxldmFudCBoZXJl Lgo+IExldCBtZSBzaG93IHlvdSAodW50ZXN0ZWQpIC0geW91ciBwYXRjaCBpcyBub3Qgc21hbGxl ci4KPgo+IFNpZ25lZC1vZmYtYnk6IE1pY2hhZWwgUy4gVHNpcmtpbiA8bXN0QHJlZGhhdC5jb20+ Cj4KPgo+Cj4gY29tbWl0IDliNDQyZmU5NzBkNWM3MTMxMWQ0MzE0ZWRlZjI2ZWUyZWIxNmU3ZmIK PiBBdXRob3I6IE1pY2hhZWwgUy4gVHNpcmtpbiA8bXN0QHJlZGhhdC5jb20+Cj4gRGF0ZTogICBX ZWQgTm92IDIwIDEyOjQ0OjE0IDIwMTMgKzAyMDAKPgo+ICAgICB2aXJ0aW9fbmV0OiBmaXggcmVz b3VyY2UgbGVhayBvbiBhbGxvYyBmYWlsdXJlCj4gICAgIAo+ICAgICB2aXJ0aW8gbmV0IGdvdCBj b25mdXNlZCwgc3RhcnRlZCBkcm9wcGluZyBwYWNrZXRzLgo+ICAgICAKPiAgICAgU2lnbmVkLW9m Zi1ieTogTWljaGFlbCBTLiBUc2lya2luIDxtc3RAcmVkaGF0LmNvbT4KPgo+IGRpZmYgLS1naXQg YS9kcml2ZXJzL25ldC92aXJ0aW9fbmV0LmMgYi9kcml2ZXJzL25ldC92aXJ0aW9fbmV0LmMKPiBp bmRleCA5ZmJkZmNkLi5kZjRiOWQwIDEwMDY0NAo+IC0tLSBhL2RyaXZlcnMvbmV0L3ZpcnRpb19u ZXQuYwo+ICsrKyBiL2RyaXZlcnMvbmV0L3ZpcnRpb19uZXQuYwo+IEBAIC0yOTcsMTMgKzI5Nywy MiBAQCBzdGF0aWMgc3RydWN0IHNrX2J1ZmYgKnBhZ2VfdG9fc2tiKHN0cnVjdCByZWNlaXZlX3F1 ZXVlICpycSwKPiAgCXJldHVybiBza2I7Cj4gIH0KPiAgCj4gLXN0YXRpYyBpbnQgcmVjZWl2ZV9t ZXJnZWFibGUoc3RydWN0IHJlY2VpdmVfcXVldWUgKnJxLCBzdHJ1Y3Qgc2tfYnVmZiAqc2tiKQo+ ICtzdGF0aWMgc3RydWN0IHNrX2J1ZmYgKnJlY2VpdmVfbWVyZ2VhYmxlKHN0cnVjdCBuZXRfZGV2 aWNlICpkZXYsCj4gKwkJCQkJIHN0cnVjdCByZWNlaXZlX3F1ZXVlICpycSwKPiArCQkJCQkgc3Ry dWN0IHBhZ2UgKnBhZ2UsCj4gKwkJCQkJIHVuc2lnbmVkIGludCBsZW4pCj4gIHsKPiAtCXN0cnVj dCBza2Jfdm5ldF9oZHIgKmhkciA9IHNrYl92bmV0X2hkcihza2IpOwo+IC0Jc3RydWN0IHBhZ2Ug KnBhZ2U7Cj4gLQlpbnQgbnVtX2J1ZiwgaSwgbGVuOwo+ICsJc3RydWN0IHNrYl92bmV0X2hkciAq aGRyID0gcGFnZV9hZGRyZXNzKGJ1Zik7Cj4gKwlpbnQgbnVtX2J1ZiA9IGhkci0+bWhkci5udW1f YnVmZmVyczsKPiArCXN0cnVjdCBza19idWZmICpza2IgPSBwYWdlX3RvX3NrYihycSwgcGFnZSwg bGVuKTsKPiArCWludCBpOwo+ICsKPiArCXNrYiA9IHBhZ2VfdG9fc2tiKHJxLCBwYWdlLCBsZW4p Owo+ICsKPiArCWlmICh1bmxpa2VseSghc2tiKSkKPiArCQlnb3RvIGVycl9za2I7Cj4gKwo+ICAK PiAtCW51bV9idWYgPSBoZHItPm1oZHIubnVtX2J1ZmZlcnM7Cj4gIAl3aGlsZSAoLS1udW1fYnVm KSB7Cj4gIAkJaSA9IHNrYl9zaGluZm8oc2tiKS0+bnJfZnJhZ3M7Cj4gIAkJaWYgKGkgPj0gTUFY X1NLQl9GUkFHUykgewo+IEBAIC0zMTMsMTAgKzMyMiwxMCBAQCBzdGF0aWMgaW50IHJlY2VpdmVf bWVyZ2VhYmxlKHN0cnVjdCByZWNlaXZlX3F1ZXVlICpycSwgc3RydWN0IHNrX2J1ZmYgKnNrYikK PiAgCQl9Cj4gIAkJcGFnZSA9IHZpcnRxdWV1ZV9nZXRfYnVmKHJxLT52cSwgJmxlbik7Cj4gIAkJ aWYgKCFwYWdlKSB7Cj4gLQkJCXByX2RlYnVnKCIlczogcnggZXJyb3I6ICVkIGJ1ZmZlcnMgbWlz c2luZ1xuIiwKPiAtCQkJCSBza2ItPmRldi0+bmFtZSwgaGRyLT5taGRyLm51bV9idWZmZXJzKTsK PiAtCQkJc2tiLT5kZXYtPnN0YXRzLnJ4X2xlbmd0aF9lcnJvcnMrKzsKPiAtCQkJcmV0dXJuIC1F SU5WQUw7Cj4gKwkJCXByX2RlYnVnKCIlczogcnggZXJyb3I6ICVkIGJ1ZmZlcnMgJWQgbWlzc2lu Z1xuIiwKPiArCQkJCSBkZXYtPm5hbWUsIGhkci0+bWhkci5udW1fYnVmZmVycywgbnVtX2J1Zik7 Cj4gKwkJCWRldi0+c3RhdHMucnhfbGVuZ3RoX2Vycm9ycysrOwo+ICsJCQlnb3RvIGVycl9idWY7 Cj4gIAkJfQo+ICAKPiAgCQlpZiAobGVuID4gUEFHRV9TSVpFKQo+IEBAIC0zMjYsNyArMzM1LDI1 IEBAIHN0YXRpYyBpbnQgcmVjZWl2ZV9tZXJnZWFibGUoc3RydWN0IHJlY2VpdmVfcXVldWUgKnJx LCBzdHJ1Y3Qgc2tfYnVmZiAqc2tiKQo+ICAKPiAgCQktLXJxLT5udW07Cj4gIAl9Cj4gLQlyZXR1 cm4gMDsKPiArCXJldHVybiBza2I7Cj4gK2Vycl9za2I6Cj4gKwlwdXRfcGFnZShwYWdlKTsKPiAr ZXJyX2J1ZjoKPiArCWRldi0+c3RhdHMucnhfZHJvcHBlZCsrOwo+ICsJZGV2X2tmcmVlX3NrYiho ZWFkX3NrYik7Cj4gKwl3aGlsZSAoLS1udW1fYnVmKSB7Cj4gKwkJYnVmID0gdmlydHF1ZXVlX2dl dF9idWYocnEtPnZxLCAmbGVuKTsKPiArCQlpZiAodW5saWtlbHkoIWJ1ZikpIHsKPiArCQkJcHJf ZGVidWcoIiVzOiByeCBlcnJvcjogJWQgYnVmZmVycyBtaXNzaW5nXG4iLAo+ICsJCQkJIGRldi0+ bmFtZSwgbnVtX2J1Zik7Cj4gKwkJCWRldi0+c3RhdHMucnhfbGVuZ3RoX2Vycm9ycysrOwo+ICsJ CQlicmVhazsKPiArCQl9Cj4gKwkJcGFnZSA9IGJ1ZjsKPiArCQlnaXZlX3BhZ2VzKHJxLCBwYWdl KTsKPiArCQktLXJxLT5udW07Cj4gKwl9Cj4gKwlyZXR1cm4gTlVMTDsKPiAgfQo+ICAKPiAgc3Rh dGljIHZvaWQgcmVjZWl2ZV9idWYoc3RydWN0IHJlY2VpdmVfcXVldWUgKnJxLCB2b2lkICpidWYs IHVuc2lnbmVkIGludCBsZW4pCj4gQEAgLTM1NCwxNyArMzgxLDE4IEBAIHN0YXRpYyB2b2lkIHJl Y2VpdmVfYnVmKHN0cnVjdCByZWNlaXZlX3F1ZXVlICpycSwgdm9pZCAqYnVmLCB1bnNpZ25lZCBp bnQgbGVuKQo+ICAJCXNrYl90cmltKHNrYiwgbGVuKTsKPiAgCX0gZWxzZSB7Cj4gIAkJcGFnZSA9 IGJ1ZjsKPiAtCQlza2IgPSBwYWdlX3RvX3NrYihycSwgcGFnZSwgbGVuKTsKPiAtCQlpZiAodW5s aWtlbHkoIXNrYikpIHsKPiAtCQkJZGV2LT5zdGF0cy5yeF9kcm9wcGVkKys7Cj4gLQkJCWdpdmVf cGFnZXMocnEsIHBhZ2UpOwo+IC0JCQlyZXR1cm47Cj4gLQkJfQo+IC0JCWlmICh2aS0+bWVyZ2Vh YmxlX3J4X2J1ZnMpCj4gLQkJCWlmIChyZWNlaXZlX21lcmdlYWJsZShycSwgc2tiKSkgewo+IC0J CQkJZGV2X2tmcmVlX3NrYihza2IpOwo+ICsJCWlmICh2aS0+bWVyZ2VhYmxlX3J4X2J1ZnMpIHsK PiArCQkJc2tiID0gcmVjZWl2ZV9tZXJnZWFibGUoZGV2LCBycSwgcGFnZSwgbGVuKTsKPiArCQkJ aWYgKHVubGlrZWx5KCFza2IpKQo+ICsJCQkJcmV0dXJuOwo+ICsJCX0gZWxzZSB7Cj4gKwkJCXNr YiA9IHBhZ2VfdG9fc2tiKHJxLCBwYWdlLCBsZW4pOwo+ICsJCQlpZiAodW5saWtlbHkoIXNrYikp IHsKPiArCQkJCWRldi0+c3RhdHMucnhfZHJvcHBlZCsrOwo+ICsJCQkJZ2l2ZV9wYWdlcyhycSwg cGFnZSk7Cj4gIAkJCQlyZXR1cm47Cj4gIAkJCX0KPiArCQl9Cj4gIAl9Cj4gIAo+ICAJaGRyID0g c2tiX3ZuZXRfaGRyKHNrYik7CgpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fXwpWaXJ0dWFsaXphdGlvbiBtYWlsaW5nIGxpc3QKVmlydHVhbGl6YXRpb25AbGlz dHMubGludXgtZm91bmRhdGlvbi5vcmcKaHR0cHM6Ly9saXN0cy5saW51eGZvdW5kYXRpb24ub3Jn L21haWxtYW4vbGlzdGluZm8vdmlydHVhbGl6YXRpb24= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753223Ab3KUD2M (ORCPT ); Wed, 20 Nov 2013 22:28:12 -0500 Received: from mx1.redhat.com ([209.132.183.28]:37581 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750860Ab3KUD2J (ORCPT ); Wed, 20 Nov 2013 22:28:09 -0500 Message-ID: <528D7DB0.8030101@redhat.com> Date: Thu, 21 Nov 2013 11:27:44 +0800 From: Jason Wang User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.1.0 MIME-Version: 1.0 To: "Michael S. Tsirkin" CC: rusty@rustcorp.com.au, virtualization@lists.linux-foundation.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, Michael Dalton , Eric Dumazet , Shirley Ma Subject: Re: [PATCH RFC] virtio_net: fix error handling for mergeable buffers References: <1384938447-3775-1-git-send-email-jasowang@redhat.com> <20131120132657.GA8455@redhat.com> <125586345.27477452.1384955642143.JavaMail.root@redhat.com> <20131120142032.GA13492@redhat.com> In-Reply-To: <20131120142032.GA13492@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 11/20/2013 10:20 PM, Michael S. Tsirkin wrote: > On Wed, Nov 20, 2013 at 08:54:02AM -0500, Jason Wang wrote: >> >> ----- 原始邮件 ----- >>> On Wed, Nov 20, 2013 at 05:07:25PM +0800, Jason Wang wrote: >>>> When mergeable buffer were used, we only put the first page buf leave the >>>> rest >>>> of buffers in the virt queue. This will cause the driver could not get the >>>> correct head buffer any more. Fix this by dropping the rest of buffers for >>>> this >>>> packet. >>>> >>>> The bug was introduced by commit 9ab86bbcf8be755256f0a5e994e0b38af6b4d399 >>>> (virtio_net: Defer skb allocation in receive path). >>>> >>>> Cc: Rusty Russell >>>> Cc: Michael S. Tsirkin >>>> Cc: Michael Dalton >>>> Cc: Eric Dumazet >>>> Cc: Shirley Ma >>>> Signed-off-by: Jason Wang >>> Just to clarify my previous comment: it was not about the >>> idea of adding drop_mergeable_buffer - rather, I think that >>> adding knowledge about mergeable buffers into page_to_skb creates an >>> ugly internal API. >>> >>> Let's move the call to page_to_skb within receive_mergeable instead: >>> it's also nice that int offset = buf - page_address(page) logic >>> is not spread around like it was. >>> >>> Also, it's not nice that we ignore length errors when we drop >>> packets because of OOM. >>> >>> So I came up with the following - it seems to work but I didn't >>> stress test yet. >> I've no objection on this. But I've rather like my small and direct patch >> to be applied to -net first. It has lower risk and was much more easier to >> be backported to stable trees. Then we can do the re-factor like this in >> net-next. > It makes the interfaces too messy. Do you mean receive_mergeable() should call page_to_skb()? It has been there several years. And even with your changes, for big and small packets, page_to_skb() were still called directly receive_buf() and the error handling were done there. > We are not in code freeze in -net - > only feature freeze, so no reason to make code like spagetty, > and it's only 25 lines changed as compared to 40. > It's not a huge refactoring. The problem is your patch mixes several bugs fixing with re-factoring, only one of the bug fixing was really needed for stable. At least we should make incremental patches on this. > > It's just as easy to backport my patch too. > You just drop the goto in the new code path we added. It may not be so easy for the people who are not familiar with virtio-net and the s/skb->dev/dev/g changes looks irrelevant here. > Let me show you (untested) - your patch is not smaller. > > Signed-off-by: Michael S. Tsirkin > > > > commit 9b442fe970d5c71311d4314edef26ee2eb16e7fb > Author: Michael S. Tsirkin > Date: Wed Nov 20 12:44:14 2013 +0200 > > virtio_net: fix resource leak on alloc failure > > virtio net got confused, started dropping packets. > > Signed-off-by: Michael S. Tsirkin > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > index 9fbdfcd..df4b9d0 100644 > --- a/drivers/net/virtio_net.c > +++ b/drivers/net/virtio_net.c > @@ -297,13 +297,22 @@ static struct sk_buff *page_to_skb(struct receive_queue *rq, > return skb; > } > > -static int receive_mergeable(struct receive_queue *rq, struct sk_buff *skb) > +static struct sk_buff *receive_mergeable(struct net_device *dev, > + struct receive_queue *rq, > + struct page *page, > + unsigned int len) > { > - struct skb_vnet_hdr *hdr = skb_vnet_hdr(skb); > - struct page *page; > - int num_buf, i, len; > + struct skb_vnet_hdr *hdr = page_address(buf); > + int num_buf = hdr->mhdr.num_buffers; > + struct sk_buff *skb = page_to_skb(rq, page, len); > + int i; > + > + skb = page_to_skb(rq, page, len); > + > + if (unlikely(!skb)) > + goto err_skb; > + > > - num_buf = hdr->mhdr.num_buffers; > while (--num_buf) { > i = skb_shinfo(skb)->nr_frags; > if (i >= MAX_SKB_FRAGS) { > @@ -313,10 +322,10 @@ static int receive_mergeable(struct receive_queue *rq, struct sk_buff *skb) > } > page = virtqueue_get_buf(rq->vq, &len); > if (!page) { > - pr_debug("%s: rx error: %d buffers missing\n", > - skb->dev->name, hdr->mhdr.num_buffers); > - skb->dev->stats.rx_length_errors++; > - return -EINVAL; > + pr_debug("%s: rx error: %d buffers %d missing\n", > + dev->name, hdr->mhdr.num_buffers, num_buf); > + dev->stats.rx_length_errors++; > + goto err_buf; > } > > if (len > PAGE_SIZE) > @@ -326,7 +335,25 @@ static int receive_mergeable(struct receive_queue *rq, struct sk_buff *skb) > > --rq->num; > } > - return 0; > + return skb; > +err_skb: > + put_page(page); > +err_buf: > + dev->stats.rx_dropped++; > + dev_kfree_skb(head_skb); > + while (--num_buf) { > + buf = virtqueue_get_buf(rq->vq, &len); > + if (unlikely(!buf)) { > + pr_debug("%s: rx error: %d buffers missing\n", > + dev->name, num_buf); > + dev->stats.rx_length_errors++; > + break; > + } > + page = buf; > + give_pages(rq, page); > + --rq->num; > + } > + return NULL; > } > > static void receive_buf(struct receive_queue *rq, void *buf, unsigned int len) > @@ -354,17 +381,18 @@ static void receive_buf(struct receive_queue *rq, void *buf, unsigned int len) > skb_trim(skb, len); > } else { > page = buf; > - skb = page_to_skb(rq, page, len); > - if (unlikely(!skb)) { > - dev->stats.rx_dropped++; > - give_pages(rq, page); > - return; > - } > - if (vi->mergeable_rx_bufs) > - if (receive_mergeable(rq, skb)) { > - dev_kfree_skb(skb); > + if (vi->mergeable_rx_bufs) { > + skb = receive_mergeable(dev, rq, page, len); > + if (unlikely(!skb)) > + return; > + } else { > + skb = page_to_skb(rq, page, len); > + if (unlikely(!skb)) { > + dev->stats.rx_dropped++; > + give_pages(rq, page); > return; > } > + } > } > > hdr = skb_vnet_hdr(skb);