> On Dec 19, 2020, at 7:41 PM, Joseph Touch <to...@strayalpha.com> wrote: > > > >> On Dec 19, 2020, at 1:17 AM, Christian Hopps <cho...@chopps.org >> <mailto: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.
The test is already performed to determine if the fragment is complete or not, regardless of whether you use the real value or some marker value. > 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”. There really is no complexity added. copylen = min(inner_remain, outer_remain) ohdr->block_offset = inner_remain; /* Only Diff */ memcpy(outer_buf, inner_buf, copylen); if (inner_remain > copylen) more to go... vs. copylen = min(inner_remain, outer_remain) ohdr->block_offset = copylen; /* Only Diff */ memcpy(outer_buf, inner_buf, copylen); if (inner_remain > copylen) more to go... Except the former allows for creating more robust implementations, and also differentiates the case where a fragment just fits or not for the receiver. The latter ends up stripping away useful information. Both ways point the block offset past the end of payload. I see the loss for stripping the information here, I don't see the gain. Thanks, Chris. > >> 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
signature.asc
Description: Message signed with OpenPGP
_______________________________________________ IPsec mailing list IPsec@ietf.org https://www.ietf.org/mailman/listinfo/ipsec
