PS - the idea that TCP segments within a single connection should ever have different DSCPs is a good example of why it's a bad idea to even 'think' of TRILL over TCP as direct encapsulation. I.e., that concept is inherently hazardous and should be avoided. Joe On 6/26/2017 10:15 AM, Joe Touch wrote: > Hi, Donald, > > > On 6/25/2017 5:07 PM, Donald Eastlake wrote: >> Hi Magnus, >> >> Thanks for the extensive review. See my responses below. >> >> On Thu, Jun 15, 2017 at 1:32 PM, Magnus Westerlund >> <magnus.westerlund@xxxxxxxxxxxx> wrote: >>> Reviewer: Magnus Westerlund >>> Review result: Not Ready >>> >>> Early review of draft-ietf-trill-over-ip-10 >>> Reviewer: Magnus Westerlund >>> Review result: Not Ready >>> >>> TSV-ART review comments: >>> >>> I have set this to not ready as there are several issues, some significant that >>> could affect the protocol realization significantly. Some may be me missing >>> things in TRILL, I was not that familiar with it before this review and I have >>> only tried looking up things, not reading the whole earlier specifications. So >>> don't hesitate to push back and provide pointers to things that can resolve >>> issues. The authors and the WG clearly have thought about a lot of issues and >>> dealt with much already. >> OK. Hopefully we can resolve these one way or the other. >> >> ... >>> TCP Encapsulation issue >>> ----------------------- >>> >>> Section 5.6: >>> >>> The TCP encapsulation appear to be missing an delimiter format allowing each >>> individual TRILL packet/payload to be read out of the TCP's byte stream. In >>> other words, a normal implementation has no way of ensuring that the TCP >>> payload starts with the start of a new TRILL payload. Multiple small TRILL >>> payloads may be included in the same TCP payload, and also only parts as TCP is >>> one way of dealing with TRILL packets that are larger than the IP+Encapsulation >>> MTU that actually will work. >>> >>> This comment is based on that there appear to be no length fields included in >>> the TRILL header. The most straight forward delimiter is a 2-byte length field >>> for the TRILL payload to be encapsulated. >> Right. It might also be useful to include some sort of check field, as >> is done in BGP, to detect if you are out of sync in parsing the TCP >> stream. > There is nothing in BGP that ever assumes that TCP write boundaries are > preserved. BGP uses markers and length fields to create message > boundaries in TCP's bytestream. The same is needed here. > > Note that BGP also never claims to craft TCP packets by 'encapsulating' > a BGP message in a TCP segment. That part of this document needs to be > removed - it not how TCP is ever used. > >> Another point is that, while with UDP it seems fine to send packets >> with assorted QoS, you don't want to encourage re-ordering of TCP >> packets in a stream. So if TCP encapsulation is being used, > Again - please, NO. NEVER use this term. > >> you want >> to use the same DSCP value for the packets in a particular TCP stream. > Again, this is nonsensical. TCP would set a DSCP for the connection, > never in different ways for individual segments of a connection. > > >> So, generally, you need to have a TCP connection per priority handling >> category. Mapping the 8 priority levels into a smaller number of >> handling categories is a normal thing to do so you certainly don't >> necessarily need 8 TCP connections. Adding material on this should not >> be too hard. > Perhaps, but please - again, please - omit any mention or implication > that this occurs via encapsulation. > > If you want to use TCP, please use it properly. > >>> Section 5.6: >>> >>> TCP endpoint requirements. I do wonder if an application like TRILL actual >>> would need to discuss performance impacting implementation choices or >>> limitations. For example use of NAGLE, the requirements on buffer sizes in >>> relation to Bandwidth delay products, as buffer memory in a RBridge will impact >>> performance. >> Well, I'm not sure how deeply this document should get into such >> performance issues. What about just saying something about >> consideration being given to tuning TCP for performance and pointing >> to one or a few other RFCs that talk about this? > Because your use of TCP (even if changed to describe it correctly) isn't > listed in those TCP RFCs. > > And it's not so simple - NAGLE helps performance for interactive systems > that use single-byte messages (e.g., telnet) and reduces the number of > outstanding "less than full" segments. When used for encapsulation, > turning NAGLE off is the right thing for multibyte messages (e.g., TRILL > messages) and can avoid the "gathering" delay (200 ms stalls when there > isn't enough source data - i.e., incoming TRILL packets - to keep up > with the outgoing segments), but could also generate a large number of > small segments (which can interfere with segment-based congestion > control, vs. ABC). > > Unless you want a very poorly performing result, *THIS* is what you need > to drill down into. > > Joe > > _______________________________________________ > trill mailing list > trill@xxxxxxxx > https://www.ietf.org/mailman/listinfo/trill