https://redmine.named-data.net/https://redmine.named-data.net/favicon.ico?14759811232016-10-18T14:18:18ZNDN project issue tracking systemNFD - Feature #3818: Readvertise end-host routes into NLSRhttps://redmine.named-data.net/issues/3818?journal_id=173142016-10-18T14:18:18ZJunxiao Shi
<ul><li><strong>Blocked by</strong> <i><a class="issue tracker-2 status-5 priority-2 priority-default closed" href="/issues/3784">Feature #3784</a>: Design readvertise</i> added</li></ul> NFD - Feature #3818: Readvertise end-host routes into NLSRhttps://redmine.named-data.net/issues/3818?journal_id=173152016-10-18T14:27:07ZJunxiao Shi
<ul><li><strong>Subject</strong> changed from <i>Route readvertise</i> to <i>Readvertise end-host routes into NLSR</i></li><li><strong>Description</strong> updated (<a title="View differences" href="/journals/17315/diff?detail_id=15167">diff</a>)</li><li><strong>Assignee</strong> set to <i>Nicholas Gordon</i></li><li><strong>Estimated time</strong> set to <i>15.00 h</i></li></ul><p>I suggest this implementation to be performed in stages as multiple commits:</p>
<ol>
<li>RIB route additional/removal signals, including <code>RibRouteRef</code> struct (1.5 hours)</li>
<li><code>ReadvertiseDestination</code> and <code>NfdRibReadvertiseDestination</code>, including <code>ReadvertisedRoute</code> struct (4.5 hours)</li>
<li><code>ReadvertisePolicy</code> and <code>ClientToNlsrReadvertisePolicy</code> and <code>Readvertise</code> (9 hours)</li>
</ol>
<p>Each change listed above is individually testable. First and second changes can be worked in parallel.</p>
NFD - Feature #3818: Readvertise end-host routes into NLSRhttps://redmine.named-data.net/issues/3818?journal_id=173162016-10-18T14:28:10ZJunxiao Shi
<ul><li><strong>Estimated time</strong> changed from <i>15.00 h</i> to <i>18.00 h</i></li></ul><p>NFD developer guide should be updated as part of this issue, thus adding 3 hours.</p>
NFD - Feature #3818: Readvertise end-host routes into NLSRhttps://redmine.named-data.net/issues/3818?journal_id=173172016-10-18T14:30:57ZJunxiao Shi
<ul><li><strong>Blocks</strong> <i><a class="issue tracker-3 status-5 priority-2 priority-default closed" href="/issues/3819">Task #3819</a>: Use readvertise for auto prefix propagation</i> added</li></ul> NFD - Feature #3818: Readvertise end-host routes into NLSRhttps://redmine.named-data.net/issues/3818?journal_id=173372016-10-20T11:37:57ZNicholas Gordonnmgordon@memphis.edu
<ul></ul><p>I have finished implementing the added Signals to rib.cpp, and added the struct definition to rib.hpp. I am unsure what you mean by "separate commits." Do you intend for me to push this patch to gerrit, and then to move on to step 2?</p>
NFD - Feature #3818: Readvertise end-host routes into NLSRhttps://redmine.named-data.net/issues/3818?journal_id=173382016-10-20T12:55:23ZJunxiao Shi
<ul></ul><blockquote>
<p>Do you intend for me to push this patch to gerrit, and then to move on to step 2?</p>
</blockquote>
<p>Yes, you should upload RIB signals commit to Gerrit. Subsequent steps should be performed in separate commits.</p>
NFD - Feature #3818: Readvertise end-host routes into NLSRhttps://redmine.named-data.net/issues/3818?journal_id=177222016-11-28T10:59:37ZNicholas Gordonnmgordon@memphis.edu
<ul></ul><p>I have been struggling with code review and unit tests for a while now (mostly as a result of general unfamiliarity with the NFD codebase and its design). However today I have had some success that prompts a question:<br>
To make the unit test succeed it was required that I fully specify all of the parameters in the ControlParameters used in the advertise function. I don't understand why I've had to do this, though, because the RibMgmt documentation says that these fields should have defaults applied to them if they are absent. Further, I wasn't able to find a line outside of the unit tests where the functions "applyDefaultsToRequest" is used, which seems to me like they aren't used anywhere at all.</p>
<p>Can someone explain to me what's going on?</p>
NFD - Feature #3818: Readvertise end-host routes into NLSRhttps://redmine.named-data.net/issues/3818?journal_id=177252016-11-28T21:40:56ZJunxiao Shi
<ul></ul><blockquote>
<p>I wasn't able to find a line outside of the unit tests where the functions "applyDefaultsToRequest" is used</p>
</blockquote>
<p>If you grep the entire codebase, you should be able to find the <a href="https://github.com/named-data/NFD/blob/9ddf1b5c26ab3920415e5c272464219f61c63be2/core/manager-base.cpp#L98" class="external">only invocation</a>.</p>
NFD - Feature #3818: Readvertise end-host routes into NLSRhttps://redmine.named-data.net/issues/3818?journal_id=178922016-12-21T14:53:06ZNicholas Gordonnmgordon@memphis.edu
<ul></ul><p>I've managed to muddy the waters for myself again. I'm currently implementing the methods for Readvertise, but I've noticed that ReadvertisedRoute was designed with only a getter for a const version of the vector. The header design is unclear whether on afterAddRoute() a new, distinct ReadvertisedRoute should be created and added to the Readvertise instance, or if the existing one by prefix should be updated with more RibRouteRefs.</p>
<p>It is unclear whether RibRouteRefs for a common name should be kept together in one ReadvertisedRoute instance. If this is the case, then the ReadvertisedRoute needs to be extended to allow for adding a RibRouteRef to the member vector. If that is not the case, then why was it designed with a <em>vector</em> of RibRouteRefs in the first place and not just a singular instance? To be clear, in the case that the <em>are</em> meant to be aggregated, then on .advertise() and .withdraw(), each RibRouteRef in the vector must be used, correct?</p>
<p>Some clarification would be appreciated.</p>
NFD - Feature #3818: Readvertise end-host routes into NLSRhttps://redmine.named-data.net/issues/3818?journal_id=180082017-01-03T09:57:53ZNicholas Gordonnmgordon@memphis.edu
<ul></ul><p>Since I have received no comments on note-9, I have assumed that the ReadvertisedRoute's list of ribRouteRefs should be mutable in some way, and have added mutator methods to it.</p>
<p>I don't understand exactly what is meant by "destination available." Is this some functionality available elsewhere, or is it implemented by each derived ReadvertiseDestination class? I cannot proceed until I understand this, so some help would be much appreciated.</p>
NFD - Feature #3818: Readvertise end-host routes into NLSRhttps://redmine.named-data.net/issues/3818?journal_id=180392017-01-05T13:31:10ZJunxiao Shi
<ul></ul><blockquote>
<p><code>ReadvertisedRoute</code> was designed with only a getter for a const version of the <code>vector<RibRouteRef></code></p>
</blockquote>
<p><a href="https://gist.github.com/yoursunny/bcee67893134d7d49c0868f5a8b9ed41/2e3a6e24042be5f02984e90815fe34567a9f8ace#file-readvertise-hpp-L39-L42">https://gist.github.com/yoursunny/bcee67893134d7d49c0868f5a8b9ed41/2e3a6e24042be5f02984e90815fe34567a9f8ace#file-readvertise-hpp-L39-L42</a><br>
<code>ReadvertisedRoute::getRibRoutes()</code> returns a <strong>mutable</strong> reference of the vector, so that you can modify the returned reference directly.</p>
NFD - Feature #3818: Readvertise end-host routes into NLSRhttps://redmine.named-data.net/issues/3818?journal_id=181282017-01-13T08:43:47ZNicholas Gordonnmgordon@memphis.edu
<ul><li><strong>% Done</strong> changed from <i>0</i> to <i>90</i></li></ul><p>I am probably 90% done with pre-review implementation. However, I'm running into a problem with testing. I have written some fakes to examine Readvertise's methods since it has no public API, including Destination and Policy fakes. However, I can't make unique_ptrs out of these things because their parent class is noncopyable. It appears that the implementation of make_unique calls the copy constructor instead of the move constructor. This results in errors since the parent class is noncopyable...</p>
<p>I don't know enough about the implementation details of our make_unique, but I can say that it would be very cumbersome to test this without fakes/mocks. Is there any way to work around this? I have uploaded my readvertise.t.cpp to gist if it helps. (<a href="https://gist.github.com/gorgonical/4b3be61b1faac093693a484213457b93">https://gist.github.com/gorgonical/4b3be61b1faac093693a484213457b93</a>)</p>
<p>Note that I intend to revise and refactor the tests, as it is clear there is a lot of overlap.</p>
NFD - Feature #3818: Readvertise end-host routes into NLSRhttps://redmine.named-data.net/issues/3818?journal_id=181972017-01-24T07:44:31ZNicholas Gordonnmgordon@memphis.edu
<ul></ul><p>I've been trying to write the unit tests for the NLSR component that listens for Readvertise commands, however I am having some difficulty using the DummyClientFace class to mock receiving interests through the dispatcher. I have posted my code snippets here:</p>
<p><a href="https://gist.github.com/gorgonical/2ab5eff58c81a60e45dbaee99e087298">https://gist.github.com/gorgonical/2ab5eff58c81a60e45dbaee99e087298</a></p>
<p>I understand that this is rather vague, but I don't really even know what I'm doing wrong, much less how to resolve it. At this point, none of my dispatcher callbacks are being called, as verified by GDB, but the ControlCommand handlers appear to be registered successfully in the dispatcher. This suggests to me that I am misunderstanding something about how to simulate Interests on this Face or about how the dispatcher works. I've tried emulating the ndn-cxx and nfd tests that involve the dispatcher, but that hasn't worked either. Any tips or discussion about how to correctly mock up what I'm doing would be very helpful and much appreciated.</p>
NFD - Feature #3818: Readvertise end-host routes into NLSRhttps://redmine.named-data.net/issues/3818?journal_id=181982017-01-24T08:42:47ZNicholas Gordonnmgordon@memphis.edu
<ul><li><strong>Status</strong> changed from <i>New</i> to <i>In Progress</i></li></ul> NFD - Feature #3818: Readvertise end-host routes into NLSRhttps://redmine.named-data.net/issues/3818?journal_id=182022017-01-25T12:10:00ZNicholas Gordonnmgordon@memphis.edu
<ul><li><strong>Blocked by</strong> <i><a class="issue tracker-2 status-5 priority-2 priority-default closed" href="/issues/3932">Feature #3932</a>: Add operator== for SigningInfo</i> added</li></ul> NFD - Feature #3818: Readvertise end-host routes into NLSRhttps://redmine.named-data.net/issues/3818?journal_id=183332017-02-09T09:24:06ZNicholas Gordonnmgordon@memphis.edu
<ul></ul><p>I need a little help understanding how to effectively use this boost::multi_index_container. First, I can't use std::set because it only allows searching based on element identity. This doesn't mimic the previous data structure very well. This multi_index_container is working well, except all of the elements in the container are const. This means that the object has to be copied (or moved) from the container, modified, then placed back into the container.</p>
<p>I can do that, but I have to implement a lot of move mechanics and heavily refactor the code. Is that really worth the cost saving of storing two Names? Names are essentially fancy strings, and I wonder about the tradeoff in simplicity for the small improvement in memory efficiency. </p>
<p>If this refactor is insisted upon, I would appreciate some help about how to use this container effectively.</p>
<p>I have uploaded some code snippets to this gist: <a href="https://gist.github.com/gorgonical/3308d405ccbbc3279c7576e7c7678d7d">https://gist.github.com/gorgonical/3308d405ccbbc3279c7576e7c7678d7d</a></p>
<p>Any comments would be appreciated.</p>
NFD - Feature #3818: Readvertise end-host routes into NLSRhttps://redmine.named-data.net/issues/3818?journal_id=183342017-02-09T09:30:56ZJunxiao Shi
<ul></ul><blockquote>
<p>This multi_index_container is working well, except all of the elements in the container are const. This means that the object has to be copied (or moved) from the container, modified, then placed back into the container.</p>
</blockquote>
<p>Have you looked at the <code>modify</code> method on an index type?</p>
NFD - Feature #3818: Readvertise end-host routes into NLSRhttps://redmine.named-data.net/issues/3818?journal_id=183352017-02-09T12:35:16ZNicholas Gordonnmgordon@memphis.edu
<ul></ul><p><code>modify</code> will work for this situation, however, I have a problem in that the <code>const</code> semantics of the <code>multi_index_container</code> complicated things significantly. I'm running into issues using the container effectively, and in the interest of time, would it be an acceptable alternative to use the string representation the ReadvertisedRoute's prefix as the key in a map? I think that I could work out the issues given enough time, but I don't see it as being worth it; I'm unfamiliar with the construct and the const semantics ask for a large refactor of the patch. However, I see that Name's string representations (should) be unique, so that should be a cheap key.</p>
NFD - Feature #3818: Readvertise end-host routes into NLSRhttps://redmine.named-data.net/issues/3818?journal_id=183362017-02-09T12:40:50ZJunxiao Shi
<ul></ul><blockquote>
<p>would it be an acceptable alternative to use the string representation the ReadvertisedRoute's prefix as the key in a map?</p>
</blockquote>
<p>No, this is unacceptable.</p>
NFD - Feature #3818: Readvertise end-host routes into NLSRhttps://redmine.named-data.net/issues/3818?journal_id=183392017-02-09T17:42:53ZNicholas Gordonnmgordon@memphis.edu
<ul></ul><p>The current patch set fails on the Ubuntu 14.04 slave because the version of the Boost library installed there (1.54) does not have the <code>emplace</code> function.<br>
I see two options:</p>
<ol>
<li>Refactor the patch set to use <code>insert</code>, which I don't think would be too much hassle.</li>
<li>Update the libraries on that host.</li>
</ol>
<p>The only caveat is that <code>insert</code> is slightly less efficient, because the move constructor would be used to move the objects.</p>
NFD - Feature #3818: Readvertise end-host routes into NLSRhttps://redmine.named-data.net/issues/3818?journal_id=183402017-02-09T19:00:39ZJunxiao Shi
<ul></ul><blockquote>
<p>The current patch set fails on the Ubuntu 14.04 slave because the version of the Boost library installed there (1.54) does not have the <code>emplace</code> function.</p>
<ol>
<li>Update the libraries on that host.</li>
</ol>
</blockquote>
<p>You can't. See <a href="https://named-data.net/codebase/platform/documentation/ndn-platform-development-guidelines/#Build_support" class="external">platform policy</a>.</p>
<blockquote>
<p>The only caveat is that <code>insert</code> is slightly less efficient, because the move constructor would be used to move the objects.</p>
</blockquote>
<p>To avoid the performance penalty:</p>
<pre><code>#if BOOST_VERSION >= 105500
m_index.emplace(..);
#else
m_index.insert(..);
#endif
</code></pre> NFD - Feature #3818: Readvertise end-host routes into NLSRhttps://redmine.named-data.net/issues/3818?journal_id=183822017-02-14T15:04:17ZNicholas Gordonnmgordon@memphis.edu
<ul></ul><p>Regarding 02-16-2017's NFD call, I claimed that the Readvertise patch would be 100% feature-complete when this current patch on gerrit cleared, and Junxiao agreed with me. However, I do not think this is true. Nowhere in the patches is a Readvertise instance created that would actually connect to the RIB signals added earlier. This is not a large patch, but it does need to come along.</p>
<p>I propose adding the Readvertise instance to the RibManager. It could be added to the RIB itself, but it would require refactoring the RIB some; <code>NfdRibReadvertiseDestination</code> requires a <code>controller</code> instance, which the RIB does not have, but the RibManager does. Additionally, the AutoPrefixPropagator instance is already located in the RibManager. Further, the design of Readvertise requires that the instance not be created if readvertising is not desired. This is easiest to accomplish when the RibManager processes the config file, if the RibManager had for example a unique_ptr to its Readvertise instance.</p>
NFD - Feature #3818: Readvertise end-host routes into NLSRhttps://redmine.named-data.net/issues/3818?journal_id=183832017-02-14T16:14:23ZJunxiao Shi
<ul></ul><p><code>Readvertise</code> should be owned by <code>nfd::rib::Service</code>.</p>
<blockquote>
<p>I propose adding the Readvertise instance to the RibManager.</p>
</blockquote>
<p>No. <code>RibManager</code> should only be responsible for processing incoming commands.</p>
<blockquote>
<p>the AutoPrefixPropagator instance is already located in the RibManager.</p>
</blockquote>
<p>That was a mistake. It should be moved into <code>nfd::rib::Service</code>.</p>
<blockquote>
<p>Further, the design of Readvertise requires that the instance not be created if readvertising is not desired. This is easiest to accomplish when the RibManager processes the config file, if the RibManager had for example a unique_ptr to its Readvertise instance.</p>
</blockquote>
<p><code>rib</code> config section should be processed in <code>nfd::rib::Service</code>.</p>
<p>You may add two more commits after the commit adding <code>Readvertise</code> itself:</p>
<ul>
<li>rib: process rib config section in rib::Service
(this commit also moves <code>AutoPrefixPropagator</code>)</li>
<li>rib: enable readvertise into NLSR</li>
</ul>
<p>Finally, the config key for readvertising into NLSR should be <code>rib.readvertise_nlsr</code>. It can be a simple yes/no or a section like <code>auto_prefix_propagate</code>.</p>
NFD - Feature #3818: Readvertise end-host routes into NLSRhttps://redmine.named-data.net/issues/3818?journal_id=184412017-02-23T14:37:33ZNicholas Gordonnmgordon@memphis.edu
<ul><li><strong>File</strong> <a href="/attachments/730">readvertise-test.tar.gz</a> <a class="icon-only icon-download" title="Download" href="/attachments/download/730/readvertise-test.tar.gz">readvertise-test.tar.gz</a> added</li></ul><p>I have developed an integration test for mini-ndn using the two patches still on gerrit, as well as a third patch to enable Readvertise in NFD itself. That patchfile is included in the .tar.gz</p>
<p>It is a 5-node, linear topology that does verify that a prefix that is AutoPrefixPropagated to a hub router will be Readvertised into NLSR, and thus show up on remote routers through NLSR. </p>
NFD - Feature #3818: Readvertise end-host routes into NLSRhttps://redmine.named-data.net/issues/3818?journal_id=185172017-03-04T13:30:11ZJunxiao Shi
<ul><li><strong>Assignee</strong> changed from <i>Nicholas Gordon</i> to <i>Junxiao Shi</i></li><li><strong>% Done</strong> changed from <i>90</i> to <i>80</i></li></ul><p>As I promised, after Nick is gone at end of Feb 2017, I'll complete this feature.</p>
<p><a href="https://gerrit.named-data.net/3747">https://gerrit.named-data.net/3747</a> simplifies <code>ReadvertisedRoute</code> as a POD type, so that it can be kept <code>noncopyable</code> and used in a STL container.<br>
<a href="https://gerrit.named-data.net/3573">https://gerrit.named-data.net/3573</a> adds <code>ReadvertisePolicy</code> and implements <code>ClientToNlsrReadvertisePolicy</code>.<br>
<a href="https://gerrit.named-data.net/3748">https://gerrit.named-data.net/3748</a> introduces <code>Readvertise</code> class.</p>
NFD - Feature #3818: Readvertise end-host routes into NLSRhttps://redmine.named-data.net/issues/3818?journal_id=185472017-03-09T20:26:31ZJunxiao Shi
<ul><li><strong>Status</strong> changed from <i>In Progress</i> to <i>Code review</i></li><li><strong>% Done</strong> changed from <i>80</i> to <i>100</i></li></ul><p>After a careful examination on current code, I'm unable to fulfill note-23 design. Thus, I just updated <a href="https://gerrit.named-data.net/3734">https://gerrit.named-data.net/3734</a> to use the recommended config option name.</p>
NFD - Feature #3818: Readvertise end-host routes into NLSRhttps://redmine.named-data.net/issues/3818?journal_id=185512017-03-10T19:51:56ZJunxiao Shi
<ul><li><strong>Status</strong> changed from <i>Code review</i> to <i>In Progress</i></li></ul><p>Code is merged. Need devguide update.</p>
NFD - Feature #3818: Readvertise end-host routes into NLSRhttps://redmine.named-data.net/issues/3818?journal_id=188232017-04-04T03:06:27ZJunxiao Shi
<ul><li><strong>Assignee</strong> changed from <i>Junxiao Shi</i> to <i>Nicholas Gordon</i></li></ul><p>I'd leave NFD devguide work to Nick.</p>
NFD - Feature #3818: Readvertise end-host routes into NLSRhttps://redmine.named-data.net/issues/3818?journal_id=189372017-04-20T08:45:13ZNicholas Gordonnmgordon@memphis.edu
<ul></ul><p>I've pushed a first draft of the Readvertise documentation to the Dev guide repo. Let me know what you think of the additions. I've placed the section just above the AutoPrefixPropagator section.</p>
NFD - Feature #3818: Readvertise end-host routes into NLSRhttps://redmine.named-data.net/issues/3818?journal_id=192472017-05-26T11:41:20ZNicholas Gordonnmgordon@memphis.edu
<ul></ul><p>If no one has any comments, I can close this issue then?</p>
NFD - Feature #3818: Readvertise end-host routes into NLSRhttps://redmine.named-data.net/issues/3818?journal_id=192862017-05-27T10:27:06ZJunxiao Shi
<ul><li><strong>Status</strong> changed from <i>In Progress</i> to <i>Closed</i></li></ul>