Hi Adrian,

Happy New Year.
Please see inline.

Many thanks for your comments,
Pablo.

-----Original Message-----
From: Adrian Farrel <adr...@olddog.co.uk>
Organisation: Old Dog Consulting
Reply to: "adr...@olddog.co.uk" <adr...@olddog.co.uk>
Date: Friday, 27 December 2019 at 20:29
To: "Pablo Camarillo (pcamaril)" <pcama...@cisco.com>
Cc: "spring@ietf.org" <spring@ietf.org>
Subject: RE: WGLC - draft-ietf-spring-srv6-network-programming

    Thanks Pablo and Happy Imminent New Year.
    
    > I have reviewed your suggested changes and incorporated them
    > in the latest revision of the document. 
    
    Great. I will try to pull that and review it soon.
    
    > Please see below inline for more detail.
    
    [snipped]
         
        >> 4.1
        >>
        >> I was surprised by...
        >>
        >>  S01. When an SRH is processed {
        >>  S02.   If (Segments Left == 0) {
        >>  S03.      Send an ICMP Parameter Problem message to the Source 
Address
        >>               Code TBD-SRH (SR Upper-layer Header Error),
        >>               Pointer set to the offset of the upper-layer header,
        >>               interrupt packet processing and discard the packet
        >>  S04.   }
        >>  S05.   If (IPv6 Hop Limit <= 1) {
        >>  S06.      Send an ICMP Time Exceeded message to the Source Address,
        >>               Code 0 (Hop limit exceeded in transit),
        >>               interrupt packet processing and discard the packet
        >>  S07.   }
        >>
        >> Are you sure that Hop Limit processing is left so late? Isn't it one 
of
        >> the first things done with a packet before even the SRH is 
discovered to
        >> be present?
        >
        > PC: Is there any difference on the hop limit check if I do it two 
lines earlier? 
        > I believe that the end result is the same.
         
        It makes a difference which ICMP error is sent. I would argue that you 
shouldn't be looking at the SRH of a packet when the Hop Limit has expired.
    
    PC: I've checked with the SRH draft and the ICMP check is the last check 
happening in the pseudocode. I would tend to agree that this should be the last 
thing done once you have determined that the packet needs to be forwarded. 
    
    AF: So, just to check I understand what you are saying. If a packet arrives 
with Hop Limit already at zero you would still look inside the SRH?

PC: As per RFC8200, if a packet arrives with Hop Limit equal to zero, you 
process the packet normally.
Then, when you attempt to forward it, if Hop Limit is zero, then you discard 
the packet.
This makes a difference when you are the last segment (like End.DX4).
      
        >> 4.1 (and other sections)
        >> 
        >> I spent rather too much time trying to work out
        >> 
        >>  S08.   max_LE = (Hdr Ext Len / 2) - 1
        >>  S09.   If ((Last Entry > max_LE) or (Segments Left > Last Entry+1)) 
{
        >> 
        >> As far as I can see, for an SRH,
        >> 
        >>   Hdr Ext Len = 7 + (16 * (Last Entry + 1) ) + sum(all TLV lengths)
        >> 
        >> So, while I see that you want to check that Last Entry doesn't point 
out
        >> beyond the end of the SRH, I don't see how your computation of 
max_LE 
        >> helps.
        >> 
        >> What have I missed?
        > 
        > PC: How do you propose to check whether Last Entry value exceeds
        > the maximum possible theoretical Last Entry value?
        > The only information you have is the Header Extension Length. Based
        > on this I compute the maximum theoretical Last Entry possible value,
        > and then just compare the values.
        >
        > Maybe I am the one who have missed something. If you have a simpler
        > proposal please let me know.
        
        OK, we agree that the right thing to do is work out the largest 
possible Last Entry value (max_LE) and compare it to the actual Last Entry 
value. That is good.
        
        I also agree that all you have in hand is the Header Extension Length 
(Hdr Ext Len)
        
        I just don't think that you are computing the right value for max_LE 
using max_LE = (Hdr Ext Len / 2) - 1.
        
        I would agree that max_LE < (Hdr Ext Len / 2) - 1  so that comparing 
Last Entry with that value provides a check. I just don't think it is a very 
precise check.
        
        Perhaps my understanding of the actual value of Hdr Ext Len is 
incorrect, but if I am right, then you could probably get closer to a good 
number.
       
    PC: Ok. I rechecked it and I believe that we cannot get any better than 
that.
    [RFC8200]
    Hdr Ext Len         8-bit unsigned integer.  Length of the Routing
                                  header in 8-octet units, not including the
                                  first 8 octets.
    [draft-ietf-6man-segment-routing-header-26]
    o  Last Entry: contains the index (zero based), in the Segment List,
       of the last element of the Segment List.
    
    Let us consider an SRH containing k SIDs and 0 or more TLVs.
    Hdr Ext Len >= k x (16 / 8) = k x 2
    Last Entry = k - 1
    
    Isolating k on one side of each equation:
    k <= Hdr Ext Len / 2
    k = Last Entry + 1
    
    Combining the two equations together:
    Last Entry + 1 <= Hdr Ext Len / 2
    
    Isolating Last Entry on one side of the equation:
    Last Entry <= Hdr Ext Len / 2 - 1
    
    Further note that, if no TLV is present, we have:
    Last Entry = Hdr Ext Len / 2 - 1
    
    PC: If I'm still missing the point let me know!
    
    AF: Very (very) sorry. My bad.
    I entirely missed " Length of the Routing header *in*8-octet*units*, not 
including the first 8 octets"
    Now I feel stupid.
    
        >> 6.1
        >> 
        >> I agree with this section, but I am concerned that you are updating 
        >> draft-ietf-6man-segment-routing-header by adding normative language 
to
        >> describe nodes that process the SRH. Does that mean you should use an
        >> "Updates" clause in the document header?
        >>  
        >> After describing CNT-1 and CNt-2 you say...
        >> 
        >>   Furthermore, an SRv6 capable node maintains an aggregate counter
        >>   CNT-3 tracking the IPv6 traffic that was received with a 
destination
        >>   address matching a local interface address that is not a locally
        >>   instantiated SID and the next-header is SRH with SL>0.
        >> 
        >> That looks like s/maintains/MUST maintain/ unless you have a 
reference
        >> for the assertion that it does maintain CNT-3.
        >
        > PC: I disagree that this is an update to the SRH. The SRH defines a
        > header and one type of segment processing.
        > Defining a counter is not related to the SRH at all.
         
        Well, perhaps you didn't just say what you meant to say because the 
text I quoted most clearly says that the counter is counting events related to 
the SRH.
        
        But to the point: the document says...
           Any SRv6 capable node SHOULD implement the following set of combined
           counters (packets and bytes):
        "Any SRv6 capable node" is clearly in the scope of 
draft-ietf-6man-segment-routing-header
    
    PC: This document talks of the processing and behaviours of SRv6. And hence 
the counters are included in this draft.
    
    AF: Last try before I give up on this one (you may be relieved to know).
    Does every device that supports SRv6 have to support this document? I had 
formed the impression not: that is a simple SRv6 routing node functions fine 
without this document (even if this document also describes it's behaviour). 
But, in the quoted text you are adding description of function for all "SRv6 
capable nodes" that is not described elsewhere. So either:
    - it is not possible to have an SRv6 capable node prior to this draft
    or
    - you are updating some other document that describes SRv6 capable nodes

PC: I think we can address this by changing "Any SRv6 capable node" by "A node 
supporting this document ...".
    
    Many thanks for the work,
    Adrian
    
    

_______________________________________________
spring mailing list
spring@ietf.org
https://www.ietf.org/mailman/listinfo/spring

Reply via email to