[katran]: adding iobuf writer #51

tehnerd · 2019-09-19T00:06:12Z

today KatranMonitor storing packets on a disk in pcap format.
sometime it is more usefull to store em in memory and allow for remote client
get em (e.g. through thrift/grpc call). in this diff i'm adding such ability
by implementing IOBufWriter

Tested By:
modified katran_tester. making sure that iobuf contains proper pcap
data (by writing this data in a file in tester, and then reading
w/ tcpdump)

facebook-github-bot

@udippant has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

udippant · 2019-09-30T18:42:03Z

katran/lib/IOBufWriter.h

+  /**
+   * @param unique_ptr<IOBuf> iobuf for packets to written into
+   */
+  explicit IOBufWriter(folly::IOBuf* iobuf);


Why not give ownership of IOBuf to IOBufWriter?

because KatranMonitor needs to have a reference to em as well (so getEventBuffer helper could be called. somehow we need to wire all of this IOBuf all the way back to KatranLb)

udippant · 2019-09-30T18:46:09Z

katran/lib/KatranMonitor.cpp

+      data_writers.push_back(std::make_shared<FileWriter>(fname));
+    } else {
+      // PcapStorageFormat::IOBuf
+      buffers_.emplace_back(folly::IOBuf::create(config_.bufferSize));


Here buffer is emplaced back to the last index. Later (in getEventBuffer) you try to access buffer with index of 'event'. What associates a buffer with an event-type?

line 54 above. events are enumerated from 0 to up to maxEvents. eventId is an index inside data_writers and buffers (if IOBuf storage is being used)

udippant · 2019-09-30T18:46:28Z

katran/lib/KatranLbStructs.h

@@ -188,6 +198,7 @@ struct KatranConfig {
 struct KatranMonitorStats {
  uint32_t limit{0};
  uint32_t amount{0};
+  uint32_t bufferFull{0};


nit: bufferExceedCount?

udippant · 2019-09-30T18:52:19Z

katran/lib/PcapWriter.cpp

  if (headerExists_[writerId]) {
    VLOG(4) << "header already exists";
    return true;
  }
+  if (!dataWriters_[writerId]->available(sizeof(struct pcap_hdr_s))) {
+    LOG(ERROR) << "DataWriter failed to write a header. Too few space.";


LOG(ERROR) << "DataWriter failed to write a header. Not enough space.";

udippant · 2019-09-30T18:57:40Z

katran/lib/KatranMonitor.cpp

+    } else {
+      // PcapStorageFormat::IOBuf
+      buffers_.emplace_back(folly::IOBuf::create(config_.bufferSize));
+      data_writers.push_back(


Likewise, data_writers are emplaced on the last index. Later we try to pick appropriate writer using eventId.
One idea that can make it simpler - if we start with a struct that is associated with an event and owns both the buffer and data_writers. Instead of bufferes_ and data_writers it maintains list of events.

eventId is returned from XDP forwarding plane as index. there is no way to return "event" (as object) from there.

today KatranMonitor storing packets on a disk in pcap format. sometime it is more usefull to store em in memory and allow for remote client get em (e.g. through thrift/grpc call). in this diff i'm adding such ability by implementing IOBufWriter Tested By: modified katran_tester. making sure that iobuf contains proper pcap data (by writing this data in a file in tester, and then reading w/ tcpdump)

facebook-github-bot · 2019-09-30T20:51:52Z

@tehnerd has updated the pull request. Re-import the pull request

tehnerd · 2019-09-30T20:52:19Z

updated w/ renaming struct field and fixing log's comment

facebook-github-bot

@udippant has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

udippant · 2019-10-02T16:05:58Z

katran/lib/PcapWriter.cpp

  if (headerExists_[writerId]) {
    VLOG(4) << "header already exists";
    return true;
  }
+  if (!dataWriters_[writerId]->available(sizeof(struct pcap_hdr_s))) {


One last thing - why don't we check available(..) beforedataWriters_[writerId]->writeData in in line # 75?

line 117. we check that we have enough available data before calling writePacket

facebook-github-bot · 2019-10-02T18:04:38Z

@udippant merged this pull request in f1b5135.

Summary: Typo in python shebang introduced by 0d19e27, probably by accident. Found while skimming the code. Pull Request resolved: facebook/openr#51 Reviewed By: steven1327 Differential Revision: D21865922 Pulled By: saifhhasan fbshipit-source-id: 5f2c2c2fac82078070920915812139f5fef1c7fe

…deps and use them in tests (#51) Summary: Pull Request resolved: facebook/sapling#51 This diff extends capabilities of CargoBuilder in getdeps so that individual manifests can be build even without workspaces. Thanks to that a build for edenapi/tools can be made and its artifacts can be used in mononoke integration tests. Reviewed By: StanislavGlebik Differential Revision: D23574887 fbshipit-source-id: 8a974a6b5235d36a44fe082aad55cd380d84dd09

Summary: After diving in all the build system I found that the first error mentioned in #219 and #220 ``` 1597 | static_assert(formattable_char, "Mixing character types is disallowed."); ``` Was basically happening while compiling folly, after compiling it by itself I noticed this didn't happened, so I found that there was an issue with the fmt dependencies, removing the one that was downloaded by katran The issue mentioned in: #221 Was because katran was configured to use C++14 by default, and some of the libraries of folly require C++17, updated our requirements. Test Plan: TEST Output: ``` Test project /home/ubuntu/ivanmorett/katran/_build/build/katran/lib/tests Start 1: IpHelpersTests.testV4ParsingBe 1/56 Test #1: IpHelpersTests.testV4ParsingBe ....................... Passed 0.01 sec Start 2: IpHelpersTests.testV4ParsingInt 2/56 Test #2: IpHelpersTests.testV4ParsingInt ...................... Passed 0.01 sec Start 3: IpHelpersTests.testV6ParsingBe 3/56 Test #3: IpHelpersTests.testV6ParsingBe ....................... Passed 0.01 sec Start 4: IpHelpersTests.testV6ParsingInt 4/56 Test #4: IpHelpersTests.testV6ParsingInt ...................... Passed 0.01 sec Start 5: IpHelpersTests.testIncorrectAddr 5/56 Test #5: IpHelpersTests.testIncorrectAddr ..................... Passed 0.01 sec Start 6: CHHelpersTest.testMaglevCHSameWeight 6/56 Test #6: CHHelpersTest.testMaglevCHSameWeight ................. Passed 0.01 sec Start 7: CHHelpersTest.testMaglevV2CHSameWeight 7/56 Test #7: CHHelpersTest.testMaglevV2CHSameWeight ............... Passed 0.01 sec Start 8: CHHelpersTest.testMaglevCHDiffWeight 8/56 Test #8: CHHelpersTest.testMaglevCHDiffWeight ................. Passed 0.01 sec Start 9: CHHelpersTest.testMaglevV2CHDiffWeight 9/56 Test #9: CHHelpersTest.testMaglevV2CHDiffWeight ............... Passed 0.01 sec Start 10: CHHelpersTest.testMaglevWeightsSumLargerThanRing 10/56 Test #10: CHHelpersTest.testMaglevWeightsSumLargerThanRing ..... Passed 0.01 sec Start 11: CHHelpersTest.testMaglevWeightsSumBelowRingSize 11/56 Test #11: CHHelpersTest.testMaglevWeightsSumBelowRingSize ...... Passed 0.01 sec Start 12: KatranLbTest.testChangeMac 12/56 Test #12: KatranLbTest.testChangeMac ........................... Passed 0.01 sec Start 13: KatranLbTest.testIfIndex 13/56 Test #13: KatranLbTest.testIfIndex ............................. Passed 0.01 sec Start 14: KatranLbTest.testVipHelpers 14/56 Test #14: KatranLbTest.testVipHelpers .......................... Passed 0.14 sec Start 15: KatranLbTest.testAddingInvalidVip 15/56 Test #15: KatranLbTest.testAddingInvalidVip .................... Passed 0.01 sec Start 16: KatranLbTest.testRealHelpers 16/56 Test #16: KatranLbTest.testRealHelpers ......................... Passed 0.01 sec Start 17: KatranLbTest.testRealFlags 17/56 Test #17: KatranLbTest.testRealFlags ........................... Passed 0.01 sec Start 18: KatranLbTest.testVipStatsHelper 18/56 Test #18: KatranLbTest.testVipStatsHelper ...................... Passed 0.01 sec Start 19: KatranLbTest.testLruStatsHelper 19/56 Test #19: KatranLbTest.testLruStatsHelper ...................... Passed 0.01 sec Start 20: KatranLbTest.testLruMissStatsHelper 20/56 Test #20: KatranLbTest.testLruMissStatsHelper .................. Passed 0.01 sec Start 21: KatranLbTest.testHcHelpers 21/56 Test #21: KatranLbTest.testHcHelpers ........................... Passed 0.01 sec Start 22: KatranLbTest.getVipFlags 22/56 Test #22: KatranLbTest.getVipFlags ............................. Passed 0.01 sec Start 23: KatranLbTest.getAllVips 23/56 Test #23: KatranLbTest.getAllVips .............................. Passed 0.01 sec Start 24: KatranLbTest.testUpdateRealsHelper 24/56 Test #24: KatranLbTest.testUpdateRealsHelper ................... Passed 0.07 sec Start 25: KatranLbTest.testUpdateQuicRealsHelper 25/56 Test #25: KatranLbTest.testUpdateQuicRealsHelper ............... Passed 0.06 sec Start 26: KatranLbTest.testUpdateQuicReal 26/56 Test #26: KatranLbTest.testUpdateQuicReal ...................... Passed 0.01 sec Start 27: KatranLbTest.getRealsForVip 27/56 Test #27: KatranLbTest.getRealsForVip .......................... Passed 0.01 sec Start 28: KatranLbTest.getHealthcheckersDst 28/56 Test #28: KatranLbTest.getHealthcheckersDst .................... Passed 0.01 sec Start 29: KatranLbTest.invalidAddressHandling 29/56 Test #29: KatranLbTest.invalidAddressHandling .................. Passed 0.01 sec Start 30: KatranLbTest.addInvalidSrcRoutingRule 30/56 Test #30: KatranLbTest.addInvalidSrcRoutingRule ................ Passed 0.01 sec Start 31: KatranLbTest.addValidSrcRoutingRuleV4 31/56 Test #31: KatranLbTest.addValidSrcRoutingRuleV4 ................ Passed 0.01 sec Start 32: KatranLbTest.addValidSrcRoutingRuleV6 32/56 Test #32: KatranLbTest.addValidSrcRoutingRuleV6 ................ Passed 0.01 sec Start 33: KatranLbTest.addMaxSrcRules 33/56 Test #33: KatranLbTest.addMaxSrcRules .......................... Passed 0.01 sec Start 34: KatranLbTest.delSrcRules 34/56 Test #34: KatranLbTest.delSrcRules ............................. Passed 0.01 sec Start 35: KatranLbTest.clearSrcRules 35/56 Test #35: KatranLbTest.clearSrcRules ........................... Passed 0.01 sec Start 36: KatranLbTest.addFewInvalidNets 36/56 Test #36: KatranLbTest.addFewInvalidNets ....................... Passed 0.01 sec Start 37: KatranLbTest.addInvalidDecapDst 37/56 Test #37: KatranLbTest.addInvalidDecapDst ...................... Passed 0.01 sec Start 38: KatranLbTest.addInvalidDecapDstNet 38/56 Test #38: KatranLbTest.addInvalidDecapDstNet ................... Passed 0.01 sec Start 39: KatranLbTest.addValidDecapDst 39/56 Test #39: KatranLbTest.addValidDecapDst ........................ Passed 0.01 sec Start 40: KatranLbTest.delValidDecapDst 40/56 Test #40: KatranLbTest.delValidDecapDst ........................ Passed 0.01 sec Start 41: KatranLbTest.delInvalidDecapDst 41/56 Test #41: KatranLbTest.delInvalidDecapDst ...................... Passed 0.01 sec Start 42: KatranLbTest.addMaxDecapDst 42/56 Test #42: KatranLbTest.addMaxDecapDst .......................... Passed 0.01 sec Start 43: VipTestF.testBatchUpdateReals 43/56 Test #43: VipTestF.testBatchUpdateReals ........................ Passed 0.04 sec Start 44: VipTestF.testBatchUpdateRealsWeight 44/56 Test #44: VipTestF.testBatchUpdateRealsWeight .................. Passed 0.05 sec Start 45: VipTestF.testGetRealsAndWeight 45/56 Test #45: VipTestF.testGetRealsAndWeight ....................... Passed 0.01 sec Start 46: VipTestF.testGetReals 46/56 Test #46: VipTestF.testGetReals ................................ Passed 0.02 sec Start 47: VipTest.testAddRemoveReal 47/56 Test #47: VipTest.testAddRemoveReal ............................ Passed 0.01 sec Start 48: EventPipeCallbackTest.SimpleCallbackTest 48/56 Test #48: EventPipeCallbackTest.SimpleCallbackTest ............. Passed 0.01 sec Start 49: EventPipeCallbackTest.LargeWriteTest 49/56 Test #49: EventPipeCallbackTest.LargeWriteTest ................. Passed 0.15 sec Start 50: TestMonitoringServiceCore.SimpleAcceptSubscription 50/56 Test #50: TestMonitoringServiceCore.SimpleAcceptSubscription ... Passed 0.01 sec Start 51: TestMonitoringServiceCore.SimpleErrors 51/56 Test #51: TestMonitoringServiceCore.SimpleErrors ............... Passed 0.01 sec Start 52: TestMonitoringServiceCore.EventIntersection 52/56 Test #52: TestMonitoringServiceCore.EventIntersection .......... Passed 0.01 sec Start 53: TestMonitoringServiceCore.RacingClients 53/56 Test #53: TestMonitoringServiceCore.RacingClients .............. Passed 0.01 sec Start 54: TestMonitoringServiceCore.SubscribeAndCancel 54/56 Test #54: TestMonitoringServiceCore.SubscribeAndCancel ......... Passed 0.01 sec Start 55: PcapWriterTest.SingleWriter 55/56 Test #55: PcapWriterTest.SingleWriter .......................... Passed 0.02 sec Start 56: PcapWriterTest.MultiWriter 56/56 Test #56: PcapWriterTest.MultiWriter ........................... Passed 0.01 sec 100% tests passed, 0 tests failed out of 56 Total Test time (real) = 1.04 sec + cd ../testing/ + ctest -v ./CMakeFiles ./CTestTestfile.cmake ./Makefile ./base64helpers-tests './base64helpers-tests[1]_include.cmake' './base64helpers-tests[1]_tests.cmake' ./cmake_install.cmake ./katran_tester ./libbase64_helpers.a ./libbpftester.a ./libkatran_test_provision.a ./libkatran_test_util.a ./libpcap_parser.a ctest: /usr/local/lib/libcurl.so.4: no version information available (required by ctest) Test project /home/ubuntu/ivanmorett/katran/_build/build/katran/lib/testing Start 1: Base64Tests.testEncode 1/2 Test #1: Base64Tests.testEncode ........... Passed 0.01 sec Start 2: Base64Tests.testDecode 2/2 Test #2: Base64Tests.testDecode ........... Passed 0.01 sec 100% tests passed, 0 tests failed out of 2 Total Test time (real) = 0.01 sec + popd ~/ivanmorett/katran/_build ``` Differential Revision: D55108012 Pulled By: lima1756

facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Sep 19, 2019

facebook-github-bot reviewed Sep 19, 2019

View reviewed changes

udippant suggested changes Sep 30, 2019

View reviewed changes

tehnerd force-pushed the iobuf_writer branch from 29a3a48 to 4b53616 Compare September 30, 2019 20:51

facebook-github-bot reviewed Sep 30, 2019

View reviewed changes

udippant reviewed Oct 2, 2019

View reviewed changes

facebook-github-bot closed this in f1b5135 Oct 2, 2019

facebook-github-bot added the Merged label Oct 2, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[katran]: adding iobuf writer #51

[katran]: adding iobuf writer #51

tehnerd commented Sep 19, 2019

facebook-github-bot left a comment

udippant Sep 30, 2019

tehnerd Sep 30, 2019

udippant Sep 30, 2019

tehnerd Sep 30, 2019

udippant Sep 30, 2019

tehnerd Sep 30, 2019

udippant Sep 30, 2019

tehnerd Sep 30, 2019

udippant Sep 30, 2019

tehnerd Sep 30, 2019

facebook-github-bot commented Sep 30, 2019

tehnerd commented Sep 30, 2019

facebook-github-bot left a comment

udippant Oct 2, 2019

tehnerd Oct 2, 2019

facebook-github-bot commented Oct 2, 2019

[katran]: adding iobuf writer #51

[katran]: adding iobuf writer #51

Conversation

tehnerd commented Sep 19, 2019

facebook-github-bot left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

facebook-github-bot commented Sep 30, 2019

tehnerd commented Sep 30, 2019

facebook-github-bot left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

facebook-github-bot commented Oct 2, 2019