Re: [Gen-art] Gen-ART Last Call review of draft-ietf-manet-olsrv2-dat-metric-08

[Paul Kyzivat <[e-mail]>]

The remedies below seem ok to me. One comment inline.

On 11/24/15 8:16 AM, Henning Rogge wrote:
> Am 20.11.2015 um 21:41 schrieb Paul Kyzivat:
>> * Section 1:
>>
>> "Appendix A contains a few possible steps to improve the DAT metric."
>>
>> This is the first occurrence of the "DAT" acronym. It took me a bit to
>> realize this must be referring to "Directional AirTime". Could you
>> please define the acronym *before* the first use? (Perhaps in the prior
>> paragraph where "Directional Airtime" is first used.)
>
> DAT was explained in the Terminology section... I moved it to the first
> place in this section.
>
>> * Section 2:
>>
>> "These networks employ link layer retransmission to increase the
>> delivery probability and multiple unicast data rates."
>>
>> I'm not sure how to parse this sentence. Is it intended to be:
>>
>> "These networks employ link layer retransmission to increase (the
>> delivery probability) and (multiple unicast data rates)."
>>
>> OR "These networks employ link layer retransmission to (increase the
>> delivery probability) and (multiple unicast data rates)."
>
> I will split the sentence apart into two parts to make this more clear.
>
>> Either way I don't understand what "multiple unicast data rates" means.
>> Can you clarify this?
>
> Wifi (802.11) provides multiple datarates for unicast traffic. I will
> change the sentence to make it easier to understand.
>
>> * Section 7:
>>
>> You call these *constants*, in contrast to the *parameters* defined in
>> section 6. But you then suggest conditions under which they could be
>> changed. Perhaps they should simply be included with the parameters, but
>> with strong warnings about diverging from the recommended values.
>
> Parameters can be chosen by a local implementation/installation... but
> constants need to be the same over the whole Mesh.
>
>> Else, it would be good to define these *before* the parameters, because
>> that would avoid the forward reference to DAT_MAXIMUM_LOSS.
>
> I will switch the Parameter and the Constant section.
>
>> * Section 9.3/9.4:
>>
>> Are you considering these to be mutually exclusive? Or is a HELLO
>> message processed first by 9.3, and then by 9.4?
>
> the RFC5444 packet (which contains the Hello Message, but that is
> unimportant for this processing part) is processed by 9.3
>
> the Hello Message is processed by 9.4
>
>> Since there is considerable overlap in processing between the two, and
>> it would presumably be wrong to do that twice, I guess you must assume
>> them to be mutually exclusive. But I presume HELLO messages arrive in
>> incoming packets, so 9.3 sounds like it ought to apply to them.

So you are saying they are mutually exclusive: HELLO message by 9.4, all 
other packets by 9.3. Can you make the text clearer about this. As 
written it still could be interpreted as applying 9.3 to all packets, 
including HELLO, and then applying 9.4 to HELLO packets. (Even though 
this is nonsense if you think about it.)

	Thanks,
	Paul

> The trouble is (thank you to Thomas Clausen for pointing this out) that
> some RFC5444 implementations don't have access to the packet sequence
> number. So this metric (which needs the validity time value of the Hello
> Message anyways) contains a fallback that provides a loss estimate based
> on Hello Intervals if a node does not provide a Packet sequence number.
>
>> * Section 10.2:
>>
>> IIUC steps 5.3 & 5.4 are just the hard way of saying:
>>
>>    bitrate := MAX(L_DAT_rx_bitrate, DAT_MINIMUM_BITRATE)
>
> I transformed both IF conditions into a MIN() and a MAX() statement.
>
> Henning Rogge
>