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(_dot_)westerlund(_at_)ericsson(_dot_)com> 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(_at_)ietf(_dot_)org
https://www.ietf.org/mailman/listinfo/trill