Bug #4538
closed
Congestion mark related elements missing in ControlParameters
Added by Junxiao Shi over 6 years ago.
Updated over 6 years ago.
Description
ndn-cxx:commit:07d05c97b435a8de5ff76986df6a0cb3a50a6484 added several congestion mark related elements to ControlParameters class without adding them to ControlCommand protocol page. In particular, BaseCongestionMarkingInterval has a conflicting TLV-TYPE number with another element that has an assignment on this page, and causes NFD crash (observed in #4318-13).
Please add the congestion marking related elements and their TLV-TYPE assignments to ControlCommand ASAP, to decrease the likelihood of conflicts in the future.
Incidentally, we need a better, more foolproof, way of managing these assignments. But that's a separate issue.
- Related to Feature #4318: Content Store flush/erase command added
- Assignee set to Eric Newberry
- Estimated time set to 2.00 h
- Priority changed from Normal to Immediate
- Description updated (diff)
- Estimated time changed from 2.00 h to 0.10 h
Renumbering of BaseCongestionMarkingInterval is not going to happen. We have a full implementation based on it and we already shipped that code (and that TLV assignment) in v0.6.1. It is way simpler to just change the assignment for NCsEntries, since it was just merged and is still unimplemented in NFD.
- Estimated time changed from 0.10 h to 1.00 h
The spec has to be updated first. Completing that in 6 minutes is impossible.
- Blocks Task #4539: Management: collect all TLV-TYPE number assignments on the same page added
- Status changed from New to In Progress
- % Done changed from 0 to 30
The TLV-TYPE for NCsEntries has been changed to 137 on the ControlCommand, CsMgmt, and ForwarderStatus wiki pages. I also added BaseCongestionMarkingInterval and DefaultCongestionThreshold to the TLV-TYPE list on the ControlCommand page.
- Status changed from In Progress to Code review
- % Done changed from 30 to 100
Update to ndn-cxx has been pushed to Gerrit.
Changes to the NCsEntries TLV-TYPE on the wiki have been reverted and the ndn-cxx change has been abandoned.
- Status changed from Code review to Resolved
Eric Newberry wrote:
The TLV-TYPE for NCsEntries has been changed to 137 on the ControlCommand, CsMgmt, and ForwarderStatus wiki pages.
You were not supposed to do this. Changing CsInfo and FwStatus datasets is just wrong because it changes the protocol in incompatible ways. ControlCommand updates for NCsEntries will be done by Junxiao in #4318 (I guess).
I also added BaseCongestionMarkingInterval and DefaultCongestionThreshold to the TLV-TYPE list on the ControlCommand page.
Ok, I think this issue is resolved then.
Changing CsInfo and FwStatus datasets is just wrong because it changes the protocol in incompatible ways.
NFD Management never guaranteed backwards compatibility.
ControlCommand updates for NCsEntries will be done by Junxiao in #4318
I won’t make this change, but neither would I object to such change.
Renumbering of BaseCongestionMarkingInterval is not going to happen. We have a full implementation based on it and we already shipped that code (and that TLV assignment) in v0.6.1.
It can happen, given the assignment was not published in protocol spec and thus is internal.
- Status changed from Resolved to Closed
Also available in: Atom
PDF
Updated by 
Updated by 
Updated by