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: Wed, 20 Nov 2013 08:54:02 -0500 (EST) Message-ID: <125586345.27477452.1384955642143.JavaMail.root@redhat.com> References: <1384938447-3775-1-git-send-email-jasowang@redhat.com> <20131120132657.GA8455@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <20131120132657.GA8455@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 CgotLS0tLSDljp/lp4vpgq7ku7YgLS0tLS0KPiBPbiBXZWQsIE5vdiAyMCwgMjAxMyBhdCAwNTow NzoyNVBNICswODAwLCBKYXNvbiBXYW5nIHdyb3RlOgo+ID4gV2hlbiBtZXJnZWFibGUgYnVmZmVy IHdlcmUgdXNlZCwgd2Ugb25seSBwdXQgdGhlIGZpcnN0IHBhZ2UgYnVmIGxlYXZlIHRoZQo+ID4g cmVzdAo+ID4gb2YgYnVmZmVycyBpbiB0aGUgdmlydCBxdWV1ZS4gVGhpcyB3aWxsIGNhdXNlIHRo ZSBkcml2ZXIgY291bGQgbm90IGdldCB0aGUKPiA+IGNvcnJlY3QgaGVhZCBidWZmZXIgYW55IG1v cmUuIEZpeCB0aGlzIGJ5IGRyb3BwaW5nIHRoZSByZXN0IG9mIGJ1ZmZlcnMgZm9yCj4gPiB0aGlz Cj4gPiBwYWNrZXQuCj4gPiAKPiA+IFRoZSBidWcgd2FzIGludHJvZHVjZWQgYnkgY29tbWl0IDlh Yjg2YmJjZjhiZTc1NTI1NmYwYTVlOTk0ZTBiMzhhZjZiNGQzOTkKPiA+ICh2aXJ0aW9fbmV0OiBE ZWZlciBza2IgYWxsb2NhdGlvbiBpbiByZWNlaXZlIHBhdGgpLgo+ID4gCj4gPiBDYzogUnVzdHkg UnVzc2VsbCA8cnVzdHlAcnVzdGNvcnAuY29tLmF1Pgo+ID4gQ2M6IE1pY2hhZWwgUy4gVHNpcmtp biA8bXN0QHJlZGhhdC5jb20+Cj4gPiBDYzogTWljaGFlbCBEYWx0b24gPG13ZGFsdG9uQGdvb2ds ZS5jb20+Cj4gPiBDYzogRXJpYyBEdW1hemV0IDxlZHVtYXpldEBnb29nbGUuY29tPgo+ID4gQ2M6 IFNoaXJsZXkgTWEgPHhtYUB1cy5pYm0uY29tPgo+ID4gU2lnbmVkLW9mZi1ieTogSmFzb24gV2Fu ZyA8amFzb3dhbmdAcmVkaGF0LmNvbT4KPiAKPiBKdXN0IHRvIGNsYXJpZnkgbXkgcHJldmlvdXMg Y29tbWVudDogaXQgd2FzIG5vdCBhYm91dCB0aGUKPiBpZGVhIG9mIGFkZGluZyBkcm9wX21lcmdl YWJsZV9idWZmZXIgLSByYXRoZXIsIEkgdGhpbmsgdGhhdAo+IGFkZGluZyBrbm93bGVkZ2UgYWJv dXQgbWVyZ2VhYmxlIGJ1ZmZlcnMgaW50byBwYWdlX3RvX3NrYiBjcmVhdGVzIGFuCj4gdWdseSBp bnRlcm5hbCBBUEkuCj4gCj4gTGV0J3MgbW92ZSB0aGUgY2FsbCB0byBwYWdlX3RvX3NrYiB3aXRo aW4gcmVjZWl2ZV9tZXJnZWFibGUgaW5zdGVhZDoKPiBpdCdzIGFsc28gbmljZSB0aGF0IGludCBv ZmZzZXQgPSBidWYgLSBwYWdlX2FkZHJlc3MocGFnZSkgbG9naWMKPiBpcyBub3Qgc3ByZWFkIGFy b3VuZCBsaWtlIGl0IHdhcy4KPiAKPiBBbHNvLCBpdCdzIG5vdCBuaWNlIHRoYXQgd2UgaWdub3Jl IGxlbmd0aCBlcnJvcnMgd2hlbiB3ZSBkcm9wCj4gcGFja2V0cyBiZWNhdXNlIG9mIE9PTS4KPiAK PiBTbyBJIGNhbWUgdXAgd2l0aCB0aGUgZm9sbG93aW5nIC0gaXQgc2VlbXMgdG8gd29yayBidXQg SSBkaWRuJ3QKPiBzdHJlc3MgdGVzdCB5ZXQuCgpJJ3ZlIG5vIG9iamVjdGlvbiBvbiB0aGlzLiBC dXQgSSd2ZSByYXRoZXIgbGlrZSBteSBzbWFsbCBhbmQgZGlyZWN0IHBhdGNoIAp0byBiZSBhcHBs aWVkIHRvIC1uZXQgZmlyc3QuIEl0IGhhcyBsb3dlciByaXNrIGFuZCB3YXMgbXVjaCBtb3JlIGVh c2llciB0byAKYmUgYmFja3BvcnRlZCB0byBzdGFibGUgdHJlZXMuIFRoZW4gd2UgY2FuIGRvIHRo ZSByZS1mYWN0b3IgbGlrZSB0aGlzIGluIApuZXQtbmV4dC4gCj4gCj4gY29tbWl0IGViZmZiM2Zl NDMzNWZmZTA3MTI0ZTQ1MThlNzZkNmUwNTg0NGZhMTgKPiBBdXRob3I6IE1pY2hhZWwgUy4gVHNp cmtpbiA8bXN0QHJlZGhhdC5jb20+Cj4gRGF0ZTogICBXZWQgTm92IDIwIDE0OjQxOjI5IDIwMTMg KzAyMDAKPiAKPiAgICAgdmlydGlvX25ldDogZml4IGVycm9yIGhhbmRsaW5nIGZvciBtZXJnZWFi bGUgYnVmZmVycwo+ICAgICAKPiAgICAgRXJpYyBEdW1hemV0IG5vdGljZWQgdGhhdCBpZiB3ZSBl bmNvdW50ZXIgYW4gZXJyb3IKPiAgICAgd2hlbiBwcm9jZXNzaW5nIGEgbWVyZ2VhYmxlIGJ1ZmZl ciwgd2UgZG9uJ3QKPiAgICAgZGVxdWV1ZSBhbGwgb2YgdGhlIGJ1ZmZlcnMgZnJvbSB0aGlzIHBh Y2tldCwKPiAgICAgdGhlIHJlc3VsdCBpcyBhbG1vc3Qgc3VyZSB0byBiZSBsb3NzIG9mIG5ldHdv cmtpbmcuCj4gICAgIAo+ICAgICBKYXNvbiBXYW5nIG5vdGljZWQgdGhhdCB3ZSBhbHNvIGxlYWsg YSBwYWdlIGFuZCB0aGF0IHdlIGRvbid0IGRlY3JlbWVudAo+ICAgICB0aGUgcnEgYnVmIGNvdW50 LCBzbyB3ZSB3b24ndCByZXBvc3QgYnVmZmVycyAoYSByZXNvdXJjZSBsZWFrKS4KPiAgICAgCj4g ICAgIENjOiBSdXN0eSBSdXNzZWxsIDxydXN0eUBydXN0Y29ycC5jb20uYXU+Cj4gICAgIENjOiBN aWNoYWVsIERhbHRvbiA8bXdkYWx0b25AZ29vZ2xlLmNvbT4KPiAgICAgUmVwb3J0ZWQtYnk6IEVy aWMgRHVtYXpldCA8ZWR1bWF6ZXRAZ29vZ2xlLmNvbT4KPiAgICAgUmVwb3J0ZWQtYnk6IEphc29u IFdhbmcgPGphc293YW5nQHJlZGhhdC5jb20+Cj4gICAgIFNpZ25lZC1vZmYtYnk6IE1pY2hhZWwg Uy4gVHNpcmtpbiA8bXN0QHJlZGhhdC5jb20+Cj4gCj4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvbmV0 L3ZpcnRpb19uZXQuYyBiL2RyaXZlcnMvbmV0L3ZpcnRpb19uZXQuYwo+IGluZGV4IDAxZjRlYjUu LjQyZjZhMWUgMTAwNjQ0Cj4gLS0tIGEvZHJpdmVycy9uZXQvdmlydGlvX25ldC5jCj4gKysrIGIv ZHJpdmVycy9uZXQvdmlydGlvX25ldC5jCj4gQEAgLTI5Niw0MSArMjk2LDUzIEBAIHN0YXRpYyBz dHJ1Y3Qgc2tfYnVmZiAqcGFnZV90b19za2Ioc3RydWN0IHJlY2VpdmVfcXVldWUKPiAqcnEsCj4g IAlyZXR1cm4gc2tiOwo+ICB9Cj4gIAo+IC1zdGF0aWMgaW50IHJlY2VpdmVfbWVyZ2VhYmxlKHN0 cnVjdCByZWNlaXZlX3F1ZXVlICpycSwgc3RydWN0IHNrX2J1ZmYKPiAqaGVhZF9za2IpCj4gK3N0 YXRpYyBzdHJ1Y3Qgc2tfYnVmZiAqcmVjZWl2ZV9tZXJnZWFibGUoc3RydWN0IG5ldF9kZXZpY2Ug KmRldiwKPiArCQkJCQkgc3RydWN0IHJlY2VpdmVfcXVldWUgKnJxLAo+ICsJCQkJCSB2b2lkICpi dWYsCj4gKwkJCQkJIHVuc2lnbmVkIGludCBsZW4pCj4gIHsKPiAtCXN0cnVjdCBza2Jfdm5ldF9o ZHIgKmhkciA9IHNrYl92bmV0X2hkcihoZWFkX3NrYik7Cj4gKwlzdHJ1Y3Qgc2tiX3ZuZXRfaGRy ICpoZHIgPSBidWY7Cj4gKwlpbnQgbnVtX2J1ZiA9IGhkci0+bWhkci5udW1fYnVmZmVyczsKPiAr CXN0cnVjdCBwYWdlICpwYWdlID0gdmlydF90b19oZWFkX3BhZ2UoYnVmKTsKPiArCWludCBvZmZz ZXQgPSBidWYgLSBwYWdlX2FkZHJlc3MocGFnZSk7Cj4gKwlzdHJ1Y3Qgc2tfYnVmZiAqaGVhZF9z a2IgPSBwYWdlX3RvX3NrYihycSwgcGFnZSwgb2Zmc2V0LCBsZW4sCj4gKwkJCQkJICAgICAgIE1B WF9QQUNLRVRfTEVOKTsKPiAgCXN0cnVjdCBza19idWZmICpjdXJyX3NrYiA9IGhlYWRfc2tiOwo+ IC0JY2hhciAqYnVmOwo+IC0Jc3RydWN0IHBhZ2UgKnBhZ2U7Cj4gLQlpbnQgbnVtX2J1ZiwgbGVu LCBvZmZzZXQ7Cj4gIAo+IC0JbnVtX2J1ZiA9IGhkci0+bWhkci5udW1fYnVmZmVyczsKPiAtCXdo aWxlICgtLW51bV9idWYpIHsKPiAtCQlpbnQgbnVtX3NrYl9mcmFncyA9IHNrYl9zaGluZm8oY3Vy cl9za2IpLT5ucl9mcmFnczsKPiArCWlmICh1bmxpa2VseSghY3Vycl9za2IpKQo+ICsJCWdvdG8g ZXJyX3NrYjsKPiArCj4gKwl3aGlsZSAoLS1udW1fYnVmKSB7Cj4gKwkJaW50IG51bV9za2JfZnJh Z3M7Cj4gKwo+ICAJCWJ1ZiA9IHZpcnRxdWV1ZV9nZXRfYnVmKHJxLT52cSwgJmxlbik7Cj4gIAkJ aWYgKHVubGlrZWx5KCFidWYpKSB7Cj4gLQkJCXByX2RlYnVnKCIlczogcnggZXJyb3I6ICVkIGJ1 ZmZlcnMgbWlzc2luZ1xuIiwKPiAtCQkJCSBoZWFkX3NrYi0+ZGV2LT5uYW1lLCBoZHItPm1oZHIu bnVtX2J1ZmZlcnMpOwo+IC0JCQloZWFkX3NrYi0+ZGV2LT5zdGF0cy5yeF9sZW5ndGhfZXJyb3Jz Kys7Cj4gLQkJCXJldHVybiAtRUlOVkFMOwo+ICsJCQlwcl9kZWJ1ZygiJXM6IHJ4IGVycm9yOiAl ZCBidWZmZXJzIG91dCBvZiAlZCBtaXNzaW5nXG4iLAo+ICsJCQkJIGRldi0+bmFtZSwgbnVtX2J1 ZiwgaGRyLT5taGRyLm51bV9idWZmZXJzKTsKPiArCQkJZGV2LT5zdGF0cy5yeF9sZW5ndGhfZXJy b3JzKys7Cj4gKwkJCWdvdG8gZXJyX2J1ZjsKPiAgCQl9Cj4gIAkJaWYgKHVubGlrZWx5KGxlbiA+ IE1BWF9QQUNLRVRfTEVOKSkgewo+ICAJCQlwcl9kZWJ1ZygiJXM6IHJ4IGVycm9yOiBtZXJnZSBi dWZmZXIgdG9vIGxvbmdcbiIsCj4gLQkJCQkgaGVhZF9za2ItPmRldi0+bmFtZSk7Cj4gKwkJCQkg ZGV2LT5uYW1lKTsKPiAgCQkJbGVuID0gTUFYX1BBQ0tFVF9MRU47Cj4gIAkJfQo+ICsKPiArCQlw YWdlID0gdmlydF90b19oZWFkX3BhZ2UoYnVmKTsKPiArCQktLXJxLT5udW07Cj4gKwo+ICsJCW51 bV9za2JfZnJhZ3MgPSBza2Jfc2hpbmZvKGN1cnJfc2tiKS0+bnJfZnJhZ3M7Cj4gIAkJaWYgKHVu bGlrZWx5KG51bV9za2JfZnJhZ3MgPT0gTUFYX1NLQl9GUkFHUykpIHsKPiAgCQkJc3RydWN0IHNr X2J1ZmYgKm5za2IgPSBhbGxvY19za2IoMCwgR0ZQX0FUT01JQyk7Cj4gLQkJCWlmICh1bmxpa2Vs eSghbnNrYikpIHsKPiAtCQkJCWhlYWRfc2tiLT5kZXYtPnN0YXRzLnJ4X2Ryb3BwZWQrKzsKPiAt CQkJCXJldHVybiAtRU5PTUVNOwo+IC0JCQl9Cj4gKwo+ICsJCQlpZiAodW5saWtlbHkoIW5za2Ip KQo+ICsJCQkJZ290byBlcnJfc2tiOwo+ICAJCQlpZiAoY3Vycl9za2IgPT0gaGVhZF9za2IpCj4g IAkJCQlza2Jfc2hpbmZvKGN1cnJfc2tiKS0+ZnJhZ19saXN0ID0gbnNrYjsKPiAgCQkJZWxzZQo+ ICAJCQkJY3Vycl9za2ItPm5leHQgPSBuc2tiOwo+IC0JCQljdXJyX3NrYiA9IG5za2I7Cj4gIAkJ CWhlYWRfc2tiLT50cnVlc2l6ZSArPSBuc2tiLT50cnVlc2l6ZTsKPiArCQkJY3Vycl9za2IgPSBu c2tiOwo+ICAJCQludW1fc2tiX2ZyYWdzID0gMDsKPiAgCQl9Cj4gIAkJaWYgKGN1cnJfc2tiICE9 IGhlYWRfc2tiKSB7Cj4gQEAgLTMzOCw4ICszNTAsNyBAQCBzdGF0aWMgaW50IHJlY2VpdmVfbWVy Z2VhYmxlKHN0cnVjdCByZWNlaXZlX3F1ZXVlICpycSwKPiBzdHJ1Y3Qgc2tfYnVmZiAqaGVhZF9z a2IpCj4gIAkJCWhlYWRfc2tiLT5sZW4gKz0gbGVuOwo+ICAJCQloZWFkX3NrYi0+dHJ1ZXNpemUg Kz0gTUFYX1BBQ0tFVF9MRU47Cj4gIAkJfQo+IC0JCXBhZ2UgPSB2aXJ0X3RvX2hlYWRfcGFnZShi dWYpOwo+IC0JCW9mZnNldCA9IGJ1ZiAtIChjaGFyICopcGFnZV9hZGRyZXNzKHBhZ2UpOwo+ICsJ CW9mZnNldCA9IGJ1ZiAtIHBhZ2VfYWRkcmVzcyhwYWdlKTsKPiAgCQlpZiAoc2tiX2Nhbl9jb2Fs ZXNjZShjdXJyX3NrYiwgbnVtX3NrYl9mcmFncywgcGFnZSwgb2Zmc2V0KSkgewo+ICAJCQlwdXRf cGFnZShwYWdlKTsKPiAgCQkJc2tiX2NvYWxlc2NlX3J4X2ZyYWcoY3Vycl9za2IsIG51bV9za2Jf ZnJhZ3MgLSAxLAo+IEBAIC0zNDksOSArMzYwLDI4IEBAIHN0YXRpYyBpbnQgcmVjZWl2ZV9tZXJn ZWFibGUoc3RydWN0IHJlY2VpdmVfcXVldWUgKnJxLAo+IHN0cnVjdCBza19idWZmICpoZWFkX3Nr YikKPiAgCQkJCQlvZmZzZXQsIGxlbiwKPiAgCQkJCQlNQVhfUEFDS0VUX0xFTik7Cj4gIAkJfQo+ ICsJfQo+ICsKPiArCXJldHVybiBoZWFkX3NrYjsKPiArCj4gK2Vycl9za2I6Cj4gKwlwdXRfcGFn ZShwYWdlKTsKPiArZXJyX2J1ZjoKPiArCWRldi0+c3RhdHMucnhfZHJvcHBlZCsrOwo+ICsJZGV2 X2tmcmVlX3NrYihoZWFkX3NrYik7Cj4gKwl3aGlsZSAoLS1udW1fYnVmKSB7Cj4gKwkJYnVmID0g dmlydHF1ZXVlX2dldF9idWYocnEtPnZxLCAmbGVuKTsKPiArCQlpZiAodW5saWtlbHkoIWJ1Zikp IHsKPiArCQkJcHJfZGVidWcoIiVzOiByeCBlcnJvcjogJWQgYnVmZmVycyBtaXNzaW5nXG4iLAo+ ICsJCQkJIGRldi0+bmFtZSwgbnVtX2J1Zik7Cj4gKwkJCWRldi0+c3RhdHMucnhfbGVuZ3RoX2Vy cm9ycysrOwo+ICsJCQlicmVhazsKPiArCQl9Cj4gKwkJcGFnZSA9IHZpcnRfdG9faGVhZF9wYWdl KGJ1Zik7Cj4gKwkJcHV0X3BhZ2UocGFnZSk7Cj4gIAkJLS1ycS0+bnVtOwo+ICAJfQo+IC0JcmV0 dXJuIDA7Cj4gKwlyZXR1cm4gTlVMTDsKPiAgfQo+ICAKPiAgc3RhdGljIHZvaWQgcmVjZWl2ZV9i dWYoc3RydWN0IHJlY2VpdmVfcXVldWUgKnJxLCB2b2lkICpidWYsIHVuc2lnbmVkIGludAo+ICBs ZW4pCj4gQEAgLTM4MCwxOSArNDEwLDkgQEAgc3RhdGljIHZvaWQgcmVjZWl2ZV9idWYoc3RydWN0 IHJlY2VpdmVfcXVldWUgKnJxLCB2b2lkCj4gKmJ1ZiwgdW5zaWduZWQgaW50IGxlbikKPiAgCQls ZW4gLT0gc2l6ZW9mKHN0cnVjdCB2aXJ0aW9fbmV0X2hkcik7Cj4gIAkJc2tiX3RyaW0oc2tiLCBs ZW4pOwo+ICAJfSBlbHNlIGlmICh2aS0+bWVyZ2VhYmxlX3J4X2J1ZnMpIHsKPiAtCQlzdHJ1Y3Qg cGFnZSAqcGFnZSA9IHZpcnRfdG9faGVhZF9wYWdlKGJ1Zik7Cj4gLQkJc2tiID0gcGFnZV90b19z a2IocnEsIHBhZ2UsCj4gLQkJCQkgIChjaGFyICopYnVmIC0gKGNoYXIgKilwYWdlX2FkZHJlc3Mo cGFnZSksCj4gLQkJCQkgIGxlbiwgTUFYX1BBQ0tFVF9MRU4pOwo+IC0JCWlmICh1bmxpa2VseSgh c2tiKSkgewo+IC0JCQlkZXYtPnN0YXRzLnJ4X2Ryb3BwZWQrKzsKPiAtCQkJcHV0X3BhZ2UocGFn ZSk7Cj4gKwkJc2tiID0gcmVjZWl2ZV9tZXJnZWFibGUoZGV2LCBycSwgYnVmLCBsZW4pOwo+ICsJ CWlmICh1bmxpa2VseSghc2tiKSkKPiAgCQkJcmV0dXJuOwo+IC0JCX0KPiAtCQlpZiAocmVjZWl2 ZV9tZXJnZWFibGUocnEsIHNrYikpIHsKPiAtCQkJZGV2X2tmcmVlX3NrYihza2IpOwo+IC0JCQly ZXR1cm47Cj4gLQkJfQo+ICAJfSBlbHNlIHsKPiAgCQlwYWdlID0gYnVmOwo+ICAJCXNrYiA9IHBh Z2VfdG9fc2tiKHJxLCBwYWdlLCAwLCBsZW4sIFBBR0VfU0laRSk7Cj4gLS0KPiBUbyB1bnN1YnNj cmliZSBmcm9tIHRoaXMgbGlzdDogc2VuZCB0aGUgbGluZSAidW5zdWJzY3JpYmUgbmV0ZGV2IiBp bgo+IHRoZSBib2R5IG9mIGEgbWVzc2FnZSB0byBtYWpvcmRvbW9Admdlci5rZXJuZWwub3JnCj4g TW9yZSBtYWpvcmRvbW8gaW5mbyBhdCAgaHR0cDovL3ZnZXIua2VybmVsLm9yZy9tYWpvcmRvbW8t aW5mby5odG1sCj4KX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X18KVmlydHVhbGl6YXRpb24gbWFpbGluZyBsaXN0ClZpcnR1YWxpemF0aW9uQGxpc3RzLmxpbnV4 LWZvdW5kYXRpb24ub3JnCmh0dHBzOi8vbGlzdHMubGludXhmb3VuZGF0aW9uLm9yZy9tYWlsbWFu L2xpc3RpbmZvL3ZpcnR1YWxpemF0aW9u From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751900Ab3KTNyS (ORCPT ); Wed, 20 Nov 2013 08:54:18 -0500 Received: from mx3-phx2.redhat.com ([209.132.183.24]:53619 "EHLO mx3-phx2.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750976Ab3KTNyP convert rfc822-to-8bit (ORCPT ); Wed, 20 Nov 2013 08:54:15 -0500 Date: Wed, 20 Nov 2013 08:54:02 -0500 (EST) From: Jason Wang 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 Message-ID: <125586345.27477452.1384955642143.JavaMail.root@redhat.com> In-Reply-To: <20131120132657.GA8455@redhat.com> References: <1384938447-3775-1-git-send-email-jasowang@redhat.com> <20131120132657.GA8455@redhat.com> Subject: Re: [PATCH RFC] virtio_net: fix error handling for mergeable buffers MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8BIT X-Originating-IP: [10.5.82.11] X-Mailer: Zimbra 8.0.3_GA_5664 (ZimbraWebClient - GC31 (Win)/8.0.3_GA_5664) Thread-Topic: virtio_net: fix error handling for mergeable buffers Thread-Index: Q/EvnJnAgGDdgZfXTPGVS7+N4S7ZxA== Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org ----- 原始邮件 ----- > 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. > > commit ebffb3fe4335ffe07124e4518e76d6e05844fa18 > Author: Michael S. Tsirkin > Date: Wed Nov 20 14:41:29 2013 +0200 > > virtio_net: fix error handling for mergeable buffers > > Eric Dumazet noticed that if we encounter an error > when processing a mergeable buffer, we don't > dequeue all of the buffers from this packet, > the result is almost sure to be loss of networking. > > Jason Wang noticed that we also leak a page and that we don't decrement > the rq buf count, so we won't repost buffers (a resource leak). > > Cc: Rusty Russell > Cc: Michael Dalton > Reported-by: Eric Dumazet > Reported-by: Jason Wang > Signed-off-by: Michael S. Tsirkin > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > index 01f4eb5..42f6a1e 100644 > --- a/drivers/net/virtio_net.c > +++ b/drivers/net/virtio_net.c > @@ -296,41 +296,53 @@ static struct sk_buff *page_to_skb(struct receive_queue > *rq, > return skb; > } > > -static int receive_mergeable(struct receive_queue *rq, struct sk_buff > *head_skb) > +static struct sk_buff *receive_mergeable(struct net_device *dev, > + struct receive_queue *rq, > + void *buf, > + unsigned int len) > { > - struct skb_vnet_hdr *hdr = skb_vnet_hdr(head_skb); > + struct skb_vnet_hdr *hdr = buf; > + int num_buf = hdr->mhdr.num_buffers; > + struct page *page = virt_to_head_page(buf); > + int offset = buf - page_address(page); > + struct sk_buff *head_skb = page_to_skb(rq, page, offset, len, > + MAX_PACKET_LEN); > struct sk_buff *curr_skb = head_skb; > - char *buf; > - struct page *page; > - int num_buf, len, offset; > > - num_buf = hdr->mhdr.num_buffers; > - while (--num_buf) { > - int num_skb_frags = skb_shinfo(curr_skb)->nr_frags; > + if (unlikely(!curr_skb)) > + goto err_skb; > + > + while (--num_buf) { > + int num_skb_frags; > + > buf = virtqueue_get_buf(rq->vq, &len); > if (unlikely(!buf)) { > - pr_debug("%s: rx error: %d buffers missing\n", > - head_skb->dev->name, hdr->mhdr.num_buffers); > - head_skb->dev->stats.rx_length_errors++; > - return -EINVAL; > + pr_debug("%s: rx error: %d buffers out of %d missing\n", > + dev->name, num_buf, hdr->mhdr.num_buffers); > + dev->stats.rx_length_errors++; > + goto err_buf; > } > if (unlikely(len > MAX_PACKET_LEN)) { > pr_debug("%s: rx error: merge buffer too long\n", > - head_skb->dev->name); > + dev->name); > len = MAX_PACKET_LEN; > } > + > + page = virt_to_head_page(buf); > + --rq->num; > + > + num_skb_frags = skb_shinfo(curr_skb)->nr_frags; > if (unlikely(num_skb_frags == MAX_SKB_FRAGS)) { > struct sk_buff *nskb = alloc_skb(0, GFP_ATOMIC); > - if (unlikely(!nskb)) { > - head_skb->dev->stats.rx_dropped++; > - return -ENOMEM; > - } > + > + if (unlikely(!nskb)) > + goto err_skb; > if (curr_skb == head_skb) > skb_shinfo(curr_skb)->frag_list = nskb; > else > curr_skb->next = nskb; > - curr_skb = nskb; > head_skb->truesize += nskb->truesize; > + curr_skb = nskb; > num_skb_frags = 0; > } > if (curr_skb != head_skb) { > @@ -338,8 +350,7 @@ static int receive_mergeable(struct receive_queue *rq, > struct sk_buff *head_skb) > head_skb->len += len; > head_skb->truesize += MAX_PACKET_LEN; > } > - page = virt_to_head_page(buf); > - offset = buf - (char *)page_address(page); > + offset = buf - page_address(page); > if (skb_can_coalesce(curr_skb, num_skb_frags, page, offset)) { > put_page(page); > skb_coalesce_rx_frag(curr_skb, num_skb_frags - 1, > @@ -349,9 +360,28 @@ static int receive_mergeable(struct receive_queue *rq, > struct sk_buff *head_skb) > offset, len, > MAX_PACKET_LEN); > } > + } > + > + return head_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 = virt_to_head_page(buf); > + put_page(page); > --rq->num; > } > - return 0; > + return NULL; > } > > static void receive_buf(struct receive_queue *rq, void *buf, unsigned int > len) > @@ -380,19 +410,9 @@ static void receive_buf(struct receive_queue *rq, void > *buf, unsigned int len) > len -= sizeof(struct virtio_net_hdr); > skb_trim(skb, len); > } else if (vi->mergeable_rx_bufs) { > - struct page *page = virt_to_head_page(buf); > - skb = page_to_skb(rq, page, > - (char *)buf - (char *)page_address(page), > - len, MAX_PACKET_LEN); > - if (unlikely(!skb)) { > - dev->stats.rx_dropped++; > - put_page(page); > + skb = receive_mergeable(dev, rq, buf, len); > + if (unlikely(!skb)) > return; > - } > - if (receive_mergeable(rq, skb)) { > - dev_kfree_skb(skb); > - return; > - } > } else { > page = buf; > skb = page_to_skb(rq, page, 0, len, PAGE_SIZE); > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >