> On Dec 19, 2020, at 1:17 AM, Christian Hopps <cho...@chopps.org> wrote:
>
> Changes are underway. One comments inline.
>
>> On Dec 4, 2020, at 11:42 AM, Joseph Touch <to...@strayalpha.com
>> <mailto:to...@strayalpha.com>> wrote:
>>
>> Hi, Christian,
>
> [...]
>
>>>> There is no clear utility in having the blockoffset point past the end of
>>>> the
>>>> current packet. It serves – at best – as only a partially useful check on
>>>> the
>>>> next packet. I.e., if the two blockoffsets disagree, presumably a packet is
>>>> lost – but if they agree, it cannot be concluded that a packet is not
>>>> lost. It
>>>> is sufficient that it points to the end of the tunnel packet.
>>>
>>> No harm either though, right? Having implemented this, I can tell you that
>>> it does help detect bugs in ones code while in development. :)
>>
>> It’s useful as a check, but it’s important to explain what the check means.
>>
>> There are four cases (BO = blockoffsets, seq = ESP sequence number)
>>
>> ESP - no gap ESP - gap
>> ---------------------------------------------------------------------
>> BO - aligned OK BO-FP-err
>> BO - misaligned SN-FP-err Typ-err
>>
>> Typ-err is the typical error when a packet is lost, because that should
>> result in both an ESP seqno gap and blockoffset misalignment.
>>
>> SN-FP-err is when there’s no ESP seq no gap but the blockoffsets are
>> misaligned. This should only ever happen if the seqno rolls around and you
>> missed it, which you should have some other mechanism to prevent (i.e., drop
>> old packets when they’re half the sequence space old - which is easy to
>> predict because you know the tunnel packet generation rate).
>>
>> BO-FP-err is when there’s an ESP seq no gap but the blockoffsets are
>> aligned. This is just a false positive (thus “FP” in my notation). S
>>
>> In any of the error cases, you do not reassemble.
>>
>> So checking blockoffset alignment only helps you know whether your sequence
>> number check and timeout discard is working correctly.
>>
>> Protocol implementation correctness should be checked by test vectors, not
>> during normal code operation IMO. At best, it’s unnecessary complexity. At
>> worst, it gives you a false sense of wanting to reassemble when you
>> shouldn’t.
>
> Conversely, test-vectors are harder to write w/o this and must be external,
> whereas this is a simple sanity check that can be added inline (perhaps
> conditionally) to code which has been shown to catch bugs. This will help
> implementations be more robust.
Test vectors run only during testing. By putting this test in regular
operation, you’re adding overhead to check that the code is working every time
it runs.
It’s your call; I was just pointing out that this is very inefficient.
> Regarding complexity, having implemented this I can say there really is no
> complexity :)
If the LOC > 0, then it’s not “no complexity”.
> You set the block offset to the remaining inner packet length to be sent (or
> 0 if no packet fragmentation is in progress). This value is sitting right
> there in the code so you can either plug it in or use some marker value (you
> suggest the length to the end of the payload). On receive the code is
> identical: does it exceed the length of the payload or not.
Agreed, it’s simple - but it’s nonzero effort and code. Again, at run-time. For
a check that basically keeps saying the code is working, rather than checking
an actual packet error.
Again, recommending inefficient code is up to you.
Joe
_______________________________________________
IPsec mailing list
IPsec@ietf.org
https://www.ietf.org/mailman/listinfo/ipsec
