Skip to content

SVS-PS v3#22

Open
yoursunny wants to merge 1 commit into
named-data:mainfrom
yoursunny:ps3
Open

SVS-PS v3#22
yoursunny wants to merge 1 commit into
named-data:mainfrom
yoursunny:ps3

Conversation

@yoursunny

@yoursunny yoursunny commented Jul 17, 2026

Copy link
Copy Markdown
Member

fixes #20

The general structure has been approved in 2026-07-17 NFD call.

However, please review the adoption of NDN Naming Convention rev3 in the sequence number component.
In previous version, it was using GenericNameComponent with NonNegativeInteger in the TLV-VALUE.
In this PR, it has been changed to SequenceNumNameComponent.

@yoursunny
yoursunny force-pushed the ps3 branch 2 times, most recently from e685b4c to ce5a945 Compare July 17, 2026 21:30
@Pesa

Pesa commented Jul 18, 2026

Copy link
Copy Markdown
Member

However, please review the adoption of NDN Naming Convention rev3 in the sequence number component. In previous version, it was using GenericNameComponent with NonNegativeInteger in the TLV-VALUE. In this PR, it has been changed to SequenceNumNameComponent.

I'm not against the idea in principle. However, I'm not sure it makes sense to use SequenceNumNameComponent in the name, but then use a custom SeqNo TLV inside MappingData. It may not be wrong per se, but I don't love the mismatch/inconsistency.

That said, the sequence number change goes beyond what was approved at today's call, so I'm not comfortable merging this without the group's approval.

@yoursunny

Copy link
Copy Markdown
Member Author

The spec is written according to what @matianxing1992 implemented in this PR:

My current SVS-PS implementation follows the segmented naming convention:
Segment (outer): /<node-prefix>/<group-prefix>/t=<bootstrap-time>/seq=<seq>/v=0/seg=<seg>

I don’t see anything wrong so I just followed his lead.

@Pesa

Pesa commented Jul 18, 2026

Copy link
Copy Markdown
Member

Yeah but what about the definition of MappingEntry/SeqNo?

MappingEntry = MAPPING-ENTRY-TYPE TLV-LENGTH
               SeqNo
               ApplicationName

SeqNo = SEQ-NO-TYPE TLV-LENGTH NonNegativeInteger

Should SeqNo become an alias of SequenceNumNameComponent as well?
As I said, I'm not a fan of the inconsistency. It seems better to me to either keep it as is or change it everywhere.

More importantly, the discussion and decision at today's call centered around adding the bootstrap time component to the name. I didn't realize the sequence number encoding was also different (and probably others didn't either). And we didn't discuss changing the .../MAPPING/<low-seq>/<high-seq> Interest name at all.

@matianxing1992 @tianyuan129 What do you think?

@Pesa

Pesa commented Jul 18, 2026

Copy link
Copy Markdown
Member

Should SeqNo become an alias of SequenceNumNameComponent as well?
As I said, I'm not a fan of the inconsistency. It seems better to me to either keep it as is or change it everywhere.

Although, on second thought, the same can be said for SeqNo in StateVectorEntry in the base SVS protocol... And I'm guessing we do not want to make breaking changes to the base SVSv3.

So I suppose I'm fine either way. Just want to make sure the others are on board (and know about this additional change).

@yoursunny

Copy link
Copy Markdown
Member Author

I realized that the switch to SequenceNumNameComponent is a recommendation in SVS v3 protocol:

Data Interest Name: /<node-prefix>/<group-prefix>/t=<bootstrap-time>/seq=<seq>

The same line in SVS v2 was:

Data Interest Name: /<node-prefix>/<group-prefix>/<seq-num>

Hence, the change is already part of SVS v3 and not a sudden choice by C++ library developer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SVS-PS is incompatible with SVS v3

2 participants