Subject: Re: comments on draft-balakrishnan-cm-02.txt

From: Hari Balakrishnan <[email-masked]>
Date: Sun, 02 Jul 2000 12:26:27 -0400

Vern,

Thanks very much for these detailed and useful comments; almost all of these 
have now been incorporated.

> >    FLOW 
> > 	A stream of packets that all share the same source and
> >         destination IP address, IP type-of-service, transport
> >         protocol, and source and destination transport port numbers.
> 
> This term is too easily confused with "macroflow" (there are places in the
> document where this happens).  The document should just use "stream", which
> in its definition is noted as being in one-to-one correspondence with the
> notion of "flow", anyway.

Good suggestion.  Done.

> >    CONGESTION WINDOW (cwnd)
> >         A CM state variable that modulates the amount of outstanding
> >         data between sender and receiver.
> 
> Should there also be a definition for ssthresh?

Well, we didn't use it because we just alluded to RFC 2581 when necessary.

> Citation to Stevens97 should be updated to RFC 2581.  (This occurs
> elsewhere, too.)

Done.

> It's important that all of the discussion of APIs in this document
> emphasizes that we're referring to *abstract* APIs.

Explicitly mentioned in two separate places now.

> 
> >    1. Open: All applications MUST call cm_open(u32 dst) before using
> >       the CM API.  dst is the 32-bit IPv4 address.  This returns an
> 
> There needs to be an IPv6 interface, too.

Replaced it with an abstract "stream_info" argument.

> Shouldn't doing PMTU-D be a SHOULD?

Yes.

> >    does not transmit any data upon a cmapp_send() callback invocation,
> >    it SHOULD call cm_notify(dst, 0) to allow the CM to permit other
> >    flows in the macroflow to transmit data.
> 
> Why give it any sort of leeway here?  If the the app doesn't call
> cm_notify(), what ensures that the bookkeeping will stay consistent?
> (Or is the SHOULD to emphasize that correct operation of the controller
> shouldn't rely on the didn't-send notification, because it's an easy
> mistake for the app to make?)

Notice that this is only for a non-transmission.  If the app doesn't call 
cm_notify(), it isn't the end of the world.  All other cm_notify()'s are done 
from the IP output routine.

> >    It uses application
> >    notifications (Section 4.3) from concurrent streams on the same
> >    macroflow to build up information about the congestion state of the
> >    different network paths.
> 
> It's not clear what's meant by "different network paths" here.  Different
> paths taken by the elements of the same macroflow?  Or is this simply a
> comment that there will be multiple macroflows?  If the latter, then I suggest
> deleting it.

Fixed.

> 
> >    The congestion control module MUST implement the following
> >    interfaces (these are not directly visible to applications; they
> >    are within the context of a macroflow):
> 
> If they're not directly visible, then why *must* the congestion control
> module implement them?  If it's to provide for modular interoperability
> with different, separately-developed schedulers, then that should be stated.

This point is made explicit now.

> >    - void schedule(u32 num_bytes): When the congestion control module
> >      determines that data can be sent, the schedule() routine MUST be
> >      called with the number of bytes that can be sent.
> 
> This MUST reads a little funny to me.  Is the point that it shouldn't be
> calle with *no more* bytes than the number that can be sent?  If so,
> then phrasing it "with no more than the number ..." would make it read
> a little better.

Done.

> "control" should be "controller"
> 
> >    - void notify(u32 nsent): This interface is used to notify the
> >      scheduler module whenever data is sent by a CM application. The
> >      nsent parameter indicates the number of bytes just sent by the
> >      application.
> 
> Shouldn't the call include the flowid (streamid)?  That would simplify the
> scheduler's state management.

Yes, indeed!

> >    The IP output routines on the host calls cm_notify() when the data
> >    is actually sent out.
> 
> How does it know which flowid/streamid is responsible?

It doesn't.  It calls cm_notify() with the stream_info (which includes src and 
dst addrs and ports and IP proto).

> >      Therefore, it calls
> >      cm_update(flowid, 4 * avg_pkt_size, 3 * avg_pkt_size,
> >      CM_TRANSIENT, rtt).
> 
> This needs careful explanation.  Also, why is it okay to use avg_pkt_size
> instead of computing the actual sizes?  If this is a general principle,
> then there needs to be a section explaining it as such.

We've addressed this.

> >      5. Upon the arrival of a partial acknowledgment (one that does
> >      not exceed that last segment transmitted at the time the loss
> 
> "that last segment" should be "the last segment".  Or should this 
> be "the highest segment"?
> 
> >    When the source first starts, it uses the cm_query() call to get an
> >    initial estimate of network bandwidth and delay.
> 
> But there won't actually be an initial estimate available, will there?
> (unless some other streams have already been active)

Fixed.  If there is, it is used, else not.  cm_query() returns a rate, rtt, 
rttdev of -1 if no info is available.  We changed the abstract API to make 
these signed values.

Thanks again for your comments!

Hari