Hi All,
Please see the next part of my review of the draft mentioned in the title –
this should be read in conjunction with the other part of this review as posted
in a separate email. This part of the review covers sections 4.2 through to
the end of section 6.2. I will attempt to send the next part as soon as time
permits.
In Section 4.2 it states:
“The C-SID sequence ends with a last C-SID in the last C-SID container that
does not have the REPLACE-C-SID flavor, or with the special C-SID value 0, or
where reaching the end of the segment list, whichever comes first.”
This implies that a container can have parts of it that are not valid C-SID’s
and are not set to zero. I found this confusing, and I’d like to understand
the process for differentiating between a valid C-SID in the container and “a
part of the C-SID container that does not contain the REPLACE-C-SID flavor.” I
can fully understand a case where you had:
Second c-sid|first c-sid|0|0 (since that would terminate on the zero) – the
text however implies that you can have:
Second c-sid|first c-sid|xxxxx – where xxxxxx could be variable length and not
be a c-sid at all.
Further to this – it implies that in the above the xxxxx may not be the end of
the c-sid list – since the text states three different conditions –
1. When the last C-SID in the last C-SID container does not have a
REPLACE-C-SID flavour (OR)
2. The C-SID has special value zero (OR)
3. When reaching the end of the segment list
This implies – that (a) could happen before (c) – so this needs clarity.
As a note – and I realize that this is also syntax used in RFC8986 – the
pseudo-code at points says effectively If(Segments Left > Last Entry) or
equivalent. Now – logically what that means is “Segments Left > Last Entry
Position” – but – what it says is – “Segments Left > Whatever is in the last
entry” – and to a programmer who is implementing based entirely on pseudo-code
without deep understanding of the document (or modifying it) this could
potentially create confusion. I may consider submitting an erratum on this
against other documents where this happens. (Basically, there is a big
difference between a comparison between the value of last entry and the
position of the last entry in code terms – and this is further complicated when
in section 4.1 there is a comparison between a position and a set value on S09)
As another point about the pseudocode detailed in section 4.2.1 on close
examination:
After replacing S02 with the given pseudocode – and replacing S09 – S16 we end
up with this:
When an SRH is processed {
If (Segments Left == 0 and (DA.Arg.Index == 0 or
Segment List[0][DA.Arg.Index-1] == 0)) {
Stop processing the SRH, and proceed to process the next
header in the packet, whose type is identified by
the Next Header field in the routing header.
}
If (IPv6 Hop Limit <= 1) {
Send an ICMP Time Exceeded message to the Source Address
with Code 0 (Hop limit exceeded in transit),
interrupt packet processing, and discard the packet.
}
max_LE = (Hdr Ext Len / 2) - 1
If (DA.Arg.Index != 0) {
If ((Last Entry > max_LE) or (Segments Left > Last Entry)) {
Send an ICMP Parameter Problem to the Source Address,
Code 0 (Erroneous header field encountered),
Pointer set to the Segments Left field,
interrupt packet processing and discard the packet.
}
Decrement DA.Arg.Index by 1.
If (Segment List[Segments Left][DA.Arg.Index] == 0) {
Decrement Segments Left by 1.
Decrement IPv6 Hop Limit by 1.
Update IPv6 DA with Segment List[Segments Left]
Submit the packet to the egress IPv6 FIB lookup for
transmission to the new destination.
}
} Else {
If((Last Entry > max_LE) or (Segments Left > Last Entry+1)){
Send an ICMP Parameter Problem to the Source Address,
Code 0 (Erroneous header field encountered),
Pointer set to the Segments Left field,
interrupt packet processing and discard the packet.
}
Decrement Segments Left by 1.
Set DA.Arg.Index to (128/LNFL - 1).
}
Decrement IPv6 Hop Limit by 1.
Write Segment List[Segments Left][DA.Arg.Index] into the bits
[LBL..LBL+LNFL-1] of the Destination Address of the IPv6
header.
Submit the packet to the egress IPv6 FIB lookup for
transmission to the new destination.
}
if(Segments Left == 0 AND (DA.Arg.Index == 0 or Segment List[0][DA.Arg.Index-1]
== 0)) – this is an AND condition – this states that the condition is valid
when Segments Left is zero in certain cases.
This becomes a problem further down where we get to this:
If (DA.Arg.Index != 0) {
If ((Last Entry > max_LE) or (Segments Left > Last Entry)) {
Send an ICMP Parameter Problem to the Source Address,
Code 0 (Erroneous header field encountered),
Pointer set to the Segments Left field,
interrupt packet processing and discard the packet.
}
Decrement DA.Arg.Index by 1.
If (Segment List[Segments Left][DA.Arg.Index] == 0) {
Decrement Segments Left by 1.
Decrement IPv6 Hop Limit by 1.
Update IPv6 DA with Segment List[Segments Left]
Submit the packet to the egress IPv6 FIB lookup for
transmission to the new destination.
}
Since Segments Left can be zero in the containing IF stanza – you could end up
decrementing segments left while it is zero – which would be -1 (or in standard
coding terms if we assume that segments left is unsigned – this would wrap
setting segments left back to 255 – and causing a significant logic error with
what you are updating the IPv6 DA with) – This comment applies to anywhere this
pseudocode is used and modified (as it is in the subsequent sections)
In Section 4.2.2 it states that the pseudo-code in section 4.2.1 is modified by
replacing S18 and S29 with “S01. Submit the packet to IPv6 module for
transmission to the new destination via a member of J”
Firstly – stating it in this manner would throw out the ordering (S18 is still
S18 and not S01 etc) and secondly – while J is defined in RFC8986 it took some
searching to find it – I believe for clarity that J needs to be clearly defined
in this document to make this more readable.
In Section 4.2.3 the comments about ordering and the text remain the same as
the above comment, in addition, T is defined in section 4.3 of RFC8986 – but
that definition should be carried over into this document for readability.
Same comment about ordering applies to section 4.2.4
In section 4.2.7 it states
“As per section 6.4, when allocating a C-SID value from a Local Identifiers
Block (LIB), an extra prefix of Locator_block:FunctionID::/LBL+FL is required
on the Segment Endpoint node to support LIB matching in forwarding. The new
behaviors with REPLACE-C-SID flavor explicitly require the node to do so in SID
initiation.”
I failed to understand the latter part of this – “explicitly require the node
to do so” – do WHAT exactly – this failed to get through my parser 😊
In Section 4.2.8 it states that S28 should be modified with the pseudocode
listed as S28.1. This is ambiguous – since S28 could then read:
S28.0 Write Segment List[Segments Left][DA.Arg.Index] into the bits
[LBL..LBL+LNFL-1] of the Destination Address of the IPv6
header.
S28.1. If (Segments Left == 0 and (DA.Arg.Index == 0 or
Segment List[0][DA.Arg.Index-1] == 0)) {
S29. Submit the packet to the egress IPv6 FIB lookup for
transmission to the new destination.
S30. }
S31. }
OR S28.0 and S28.1 could be swapped reading:
S28.1 If (Segments Left == 0 and (DA.Arg.Index == 0 or
Segment List[0][DA.Arg.Index-1] == 0)) {
S28.2 Write Segment List[Segments Left][DA.Arg.Index] into the bits
[LBL..LBL+LNFL-1] of the Destination Address of the IPv6
header.
S29. Submit the packet to the egress IPv6 FIB lookup for
transmission to the new destination.
S30. }
S31. }
In Section 5.2 it states:
“If N1 and N2 are two different physical nodes of the SRv6 domain and I is the
local C-SID value, then N1 and N2 may bind two different behaviors to I”
While N1 and N2 here presumably are “Node1 and Node2” – this needs more text to
clarify this is the actual meaning – basically either needs to be written out
or defined directly above the text.
In section 6.2 it states:
“The RECOMMENDED Locator-Block sizes for the NEXT-C-SID flavor are 16, 32, or
48 bits. The smaller the block length, the higher the compression efficiency.”
I presume that “smaller the block length” is an actual literal reference to 16
< 32 < 48 – however – this could probably be re-worded in terms of
shorter/longer – since a /16 is a way larger block than a /32 etc. I also
strongly question the recommendation of a /16 locator block if this refers to
global space – since this is a *vast* amount of space
Internal All Employees
_______________________________________________
spring mailing list
spring@ietf.org
https://www.ietf.org/mailman/listinfo/spring
