From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Ahern Subject: Re: [patch iproute2 v8 1/2] lib/libnetlink: Add functions rtnl_talk_msg and rtnl_talk_iov Date: Wed, 10 Jan 2018 12:20:36 -0700 Message-ID: References: <20180110032742.8127-1-chrism@mellanox.com> <20180110032742.8127-2-chrism@mellanox.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------1CB574FB67711F97B0EDDEA1" Cc: gerlitz.or@gmail.com, stephen@networkplumber.org, marcelo.leitner@gmail.com, phil@nwl.cc To: Chris Mi , netdev@vger.kernel.org Return-path: Received: from mail-pg0-f65.google.com ([74.125.83.65]:43884 "EHLO mail-pg0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750861AbeAJTUj (ORCPT ); Wed, 10 Jan 2018 14:20:39 -0500 Received: by mail-pg0-f65.google.com with SMTP id f14so79253pga.10 for ; Wed, 10 Jan 2018 11:20:39 -0800 (PST) In-Reply-To: <20180110032742.8127-2-chrism@mellanox.com> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: This is a multi-part message in MIME format. --------------1CB574FB67711F97B0EDDEA1 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit On 1/9/18 8:27 PM, Chris Mi wrote: > rtnl_talk can only send a single message to kernel. Add two functions > rtnl_talk_msg and rtnl_talk_iov that can send multiple messages to kernel. > rtnl_talk_msg takes struct msghdr * as argument. > rtnl_talk_iov takes struct iovec * and iovlen as arguments. > > Signed-off-by: Chris Mi > --- > include/libnetlink.h | 6 ++++ > lib/libnetlink.c | 82 ++++++++++++++++++++++++++++++++++++++++------------ > 2 files changed, 70 insertions(+), 18 deletions(-) > > diff --git a/include/libnetlink.h b/include/libnetlink.h > index a4d83b9e..e9a63dbc 100644 > --- a/include/libnetlink.h > +++ b/include/libnetlink.h > @@ -96,6 +96,12 @@ int rtnl_dump_filter_nc(struct rtnl_handle *rth, > int rtnl_talk(struct rtnl_handle *rtnl, struct nlmsghdr *n, > struct nlmsghdr **answer) > __attribute__((warn_unused_result)); > +int rtnl_talk_msg(struct rtnl_handle *rtnl, struct msghdr *m, > + struct nlmsghdr **answer) > + __attribute__((warn_unused_result)); As mentioned before rtnl_talk_msg is not needed; you only need to add rtnl_talk_iov. The attached fixup on top of your patch removes it and adjusts __rtnl_talk_iov. Please roll that change into your patch. While testing this I noticed 2 other oddities: $ perf trace -s tc -b tc.batch (stddev column removed to shorten line width) Summary of events: tc (780), 1857 events, 97.9% syscall calls total min avg max (msec) (msec) (msec) (msec) --------------- -------- --------- --------- --------- --------- recvmsg 530 6.532 0.008 0.012 0.218 open 269 5.429 0.012 0.020 0.117 sendmsg 4 3.518 0.092 0.879 1.647 1. recvmsg is called twice - once to peek at message size, allocate a buffer and then really receive the message. That is overkill for ACKs. 2. I am using a batch file with drop filters: filter add dev eth2 ingress protocol ip pref 273 flower dst_ip 192.168.253.0/16 action drop and for each command tc is trying to dlopen m_drop.so: open("/usr/lib/tc//m_drop.so", O_RDONLY|O_CLOEXEC) = -1 ENOENT (No such file or directory) With a patch to use a stack buffer for ACKs, the above perf summary becomes: $ perf trace -s tc -b tc.batch Summary of events: tc (777), 1345 events, 97.1% syscall calls total min avg max (msec) (msec) (msec) (msec) --------------- -------- --------- --------- --------- --------- open 269 5.510 0.013 0.020 0.160 recvmsg 274 3.758 0.009 0.014 0.396 sendmsg 4 3.531 0.098 0.883 1.672 Making the open errors now the dominate overhead affecting performance. If tc had some smarts that it already tried that file it would avoid the subsequent open calls. The end result is a significant speed up compared to the current tc: Summary of events: tc (785), 2333 events, 98.3% syscall calls total min avg max (msec) (msec) (msec) (msec) --------------- -------- --------- --------- --------- --------- sendmsg 256 9.832 0.029 0.038 0.181 open 269 5.819 0.013 0.022 0.353 recvmsg 530 5.592 0.009 0.011 0.285 Can you look at a follow on patch (not part of this set) to cache status of dlopen attempts? --------------1CB574FB67711F97B0EDDEA1 Content-Type: text/plain; charset=UTF-8; x-mac-type="0"; x-mac-creator="0"; name="rtnl_talk_fixup.patch" Content-Transfer-Encoding: base64 Content-Disposition: attachment; filename="rtnl_talk_fixup.patch" ZGlmZiAtLWdpdCBhL2luY2x1ZGUvbGlibmV0bGluay5oIGIvaW5jbHVkZS9saWJuZXRsaW5r LmgKaW5kZXggZTlhNjNkYmNhMjU5Li5kNjMyMjE5MGFmYWEgMTAwNjQ0Ci0tLSBhL2luY2x1 ZGUvbGlibmV0bGluay5oCisrKyBiL2luY2x1ZGUvbGlibmV0bGluay5oCkBAIC05Niw5ICs5 Niw2IEBAIGludCBydG5sX2R1bXBfZmlsdGVyX25jKHN0cnVjdCBydG5sX2hhbmRsZSAqcnRo LAogaW50IHJ0bmxfdGFsayhzdHJ1Y3QgcnRubF9oYW5kbGUgKnJ0bmwsIHN0cnVjdCBubG1z Z2hkciAqbiwKIAkgICAgICBzdHJ1Y3Qgbmxtc2doZHIgKiphbnN3ZXIpCiAJX19hdHRyaWJ1 dGVfXygod2Fybl91bnVzZWRfcmVzdWx0KSk7Ci1pbnQgcnRubF90YWxrX21zZyhzdHJ1Y3Qg cnRubF9oYW5kbGUgKnJ0bmwsIHN0cnVjdCBtc2doZHIgKm0sCi0JCSAgc3RydWN0IG5sbXNn aGRyICoqYW5zd2VyKQotCV9fYXR0cmlidXRlX18oKHdhcm5fdW51c2VkX3Jlc3VsdCkpOwog aW50IHJ0bmxfdGFsa19pb3Yoc3RydWN0IHJ0bmxfaGFuZGxlICpydG5sLCBzdHJ1Y3QgaW92 ZWMgKmlvdmVjLCBzaXplX3QgaW92bGVuLAogCQkgIHN0cnVjdCBubG1zZ2hkciAqKmFuc3dl cikKIAlfX2F0dHJpYnV0ZV9fKCh3YXJuX3VudXNlZF9yZXN1bHQpKTsKZGlmZiAtLWdpdCBh L2xpYi9saWJuZXRsaW5rLmMgYi9saWIvbGlibmV0bGluay5jCmluZGV4IDE4MzgyNWM3ZmUw ZS4uY2IxOTkwZmNiZjFjIDEwMDY0NAotLS0gYS9saWIvbGlibmV0bGluay5jCisrKyBiL2xp Yi9saWJuZXRsaW5rLmMKQEAgLTU4MSwzOCArNTgxLDQwIEBAIHN0YXRpYyB2b2lkIHJ0bmxf dGFsa19lcnJvcihzdHJ1Y3Qgbmxtc2doZHIgKmgsIHN0cnVjdCBubG1zZ2VyciAqZXJyLAog CQlzdHJlcnJvcigtZXJyLT5lcnJvcikpOwogfQogCi1zdGF0aWMgaW50IF9fcnRubF90YWxr X21zZyhzdHJ1Y3QgcnRubF9oYW5kbGUgKnJ0bmwsIHN0cnVjdCBtc2doZHIgKm0sCi0JCQkg ICBzdHJ1Y3Qgbmxtc2doZHIgKiphbnN3ZXIsCisKK3N0YXRpYyBpbnQgX19ydG5sX3RhbGtf aW92KHN0cnVjdCBydG5sX2hhbmRsZSAqcnRubCwgc3RydWN0IGlvdmVjICppb3YsCisJCQkg ICBzaXplX3QgaW92bGVuLCBzdHJ1Y3Qgbmxtc2doZHIgKiphbnN3ZXIsCiAJCQkgICBib29s IHNob3dfcnRubF9lcnIsIG5sX2V4dF9hY2tfZm5fdCBlcnJmbikKIHsKIAlzdHJ1Y3Qgc29j a2FkZHJfbmwgbmxhZGRyID0geyAubmxfZmFtaWx5ID0gQUZfTkVUTElOSyB9OwotCWludCBp LCBzdGF0dXMsIGlvdmxlbiA9IG0tPm1zZ19pb3ZsZW47Ci0Jc3RydWN0IGlvdmVjIGlvdjsK KwlpbnQgaSwgc3RhdHVzOworCXN0cnVjdCBpb3ZlYyByaW92OwogCXN0cnVjdCBtc2doZHIg bXNnID0gewogCQkubXNnX25hbWUgPSAmbmxhZGRyLAogCQkubXNnX25hbWVsZW4gPSBzaXpl b2YobmxhZGRyKSwKLQkJLm1zZ19pb3YgPSAmaW92LAotCQkubXNnX2lvdmxlbiA9IDEsCisJ CS5tc2dfaW92ID0gaW92LAorCQkubXNnX2lvdmxlbiA9IGlvdmxlbiwKIAl9OwogCXVuc2ln bmVkIGludCBzZXEgPSAwOwogCXN0cnVjdCBubG1zZ2hkciAqaDsKIAljaGFyICpidWY7CiAK IAlmb3IgKGkgPSAwOyBpIDwgaW92bGVuOyBpKyspIHsKLQkJc3RydWN0IGlvdmVjICp2Owot CQl2ID0gJm0tPm1zZ19pb3ZbaV07Ci0JCWggPSB2LT5pb3ZfYmFzZTsKKwkJaCA9IGlvdltp XS5pb3ZfYmFzZTsKIAkJaC0+bmxtc2dfc2VxID0gc2VxID0gKytydG5sLT5zZXE7CiAJCWlm IChhbnN3ZXIgPT0gTlVMTCkKIAkJCWgtPm5sbXNnX2ZsYWdzIHw9IE5MTV9GX0FDSzsKIAl9 CiAKLQlzdGF0dXMgPSBzZW5kbXNnKHJ0bmwtPmZkLCBtLCAwKTsKKwlzdGF0dXMgPSBzZW5k bXNnKHJ0bmwtPmZkLCAmbXNnLCAwKTsKIAlpZiAoc3RhdHVzIDwgMCkgewogCQlwZXJyb3Io IkNhbm5vdCB0YWxrIHRvIHJ0bmV0bGluayIpOwogCQlyZXR1cm4gLTE7CiAJfQogCisJLyog Y2hhbmdlIG1zZyB0byB1c2UgdGhlIHJlc3BvbnNlIGlvdiAqLworCW1zZy5tc2dfaW92ID0g JnJpb3Y7CisJbXNnLm1zZ19pb3ZsZW4gPSAxOwogCWkgPSAwOwogCXdoaWxlICgxKSB7CiBu ZXh0OgpAQCAtNzA1LDIxICs3MDcsNiBAQCBzdGF0aWMgaW50IF9fcnRubF90YWxrX21zZyhz dHJ1Y3QgcnRubF9oYW5kbGUgKnJ0bmwsIHN0cnVjdCBtc2doZHIgKm0sCiAJfQogfQogCi1z dGF0aWMgaW50IF9fcnRubF90YWxrX2lvdihzdHJ1Y3QgcnRubF9oYW5kbGUgKnJ0bmwsIHN0 cnVjdCBpb3ZlYyAqbXNnX2lvdiwKLQkJCSAgIHNpemVfdCBpb3ZsZW4sIHN0cnVjdCBubG1z Z2hkciAqKmFuc3dlciwKLQkJCSAgIGJvb2wgc2hvd19ydG5sX2VyciwgbmxfZXh0X2Fja19m bl90IGVycmZuKQotewotCXN0cnVjdCBzb2NrYWRkcl9ubCBubGFkZHIgPSB7IC5ubF9mYW1p bHkgPSBBRl9ORVRMSU5LIH07Ci0Jc3RydWN0IG1zZ2hkciBtc2cgPSB7Ci0JCS5tc2dfbmFt ZSA9ICZubGFkZHIsCi0JCS5tc2dfbmFtZWxlbiA9IHNpemVvZihubGFkZHIpLAotCQkubXNn X2lvdiA9IG1zZ19pb3YsCi0JCS5tc2dfaW92bGVuID0gaW92bGVuLAotCX07Ci0KLQlyZXR1 cm4gX19ydG5sX3RhbGtfbXNnKHJ0bmwsICZtc2csIGFuc3dlciwgc2hvd19ydG5sX2Vyciwg ZXJyZm4pOwotfQotCiBzdGF0aWMgaW50IF9fcnRubF90YWxrKHN0cnVjdCBydG5sX2hhbmRs ZSAqcnRubCwgc3RydWN0IG5sbXNnaGRyICpuLAogCQkgICAgICAgc3RydWN0IG5sbXNnaGRy ICoqYW5zd2VyLAogCQkgICAgICAgYm9vbCBzaG93X3J0bmxfZXJyLCBubF9leHRfYWNrX2Zu X3QgZXJyZm4pCkBAIC03MzgsMTIgKzcyNSw2IEBAIGludCBydG5sX3RhbGsoc3RydWN0IHJ0 bmxfaGFuZGxlICpydG5sLCBzdHJ1Y3Qgbmxtc2doZHIgKm4sCiAJcmV0dXJuIF9fcnRubF90 YWxrKHJ0bmwsIG4sIGFuc3dlciwgdHJ1ZSwgTlVMTCk7CiB9CiAKLWludCBydG5sX3RhbGtf bXNnKHN0cnVjdCBydG5sX2hhbmRsZSAqcnRubCwgc3RydWN0IG1zZ2hkciAqbSwKLQkJICBz dHJ1Y3Qgbmxtc2doZHIgKiphbnN3ZXIpCi17Ci0JcmV0dXJuIF9fcnRubF90YWxrX21zZyhy dG5sLCBtLCBhbnN3ZXIsIHRydWUsIE5VTEwpOwotfQotCiBpbnQgcnRubF90YWxrX2lvdihz dHJ1Y3QgcnRubF9oYW5kbGUgKnJ0bmwsIHN0cnVjdCBpb3ZlYyAqaW92ZWMsIHNpemVfdCBp b3ZsZW4sCiAJCSAgc3RydWN0IG5sbXNnaGRyICoqYW5zd2VyKQogewo= --------------1CB574FB67711F97B0EDDEA1--