tsvwg / draft-ietf-tsvwg-udp-options

0 stars 0 forks source link

* Section 12: The inner if clauses in the pseudo-code seem to be inconsistent #10

Closed gorryfair closed 7 months ago

gorryfair commented 1 year ago

Erik: The inner if clauses in the pseudo-code seem to be

inconsistent:

When (OCS == 0 and UDP CS != 0), both sub-clauses of the second outer if clause are true, and thus the options need to be both ignored (first sub-claused) and processed (second sub-clause).

This issue was raised in https://mailarchive.ietf.org/arch/msg/tsvwg/ocDWz4jWZdqVP6CTuAhsBXer2Bc/

and answered (but not resolved) in https://mailarchive.ietf.org/arch/msg/tsvwg/CnPYSXjPPmLtNkBtjJ4BSgMX2Ck/

See also https://mailarchive.ietf.org/arch/msg/tsvwg/7nC8VbdxdcXiMjVBpnkZn7Dw6ZU/

jtouch commented 9 months ago

Agreed with Gorry; I checked this and updated the text to catch all the pending changes, but the pseudo-code is correct in the test above.

NO CHANGES to -23. Please advise if further discussion is needed.

auerswal commented 9 months ago

Well, the pseudo code gives two logical expressions for correct or zero UDP checksum:

  1. ((OCS != 0 and fails or OCS == 0) and UDP CS != 0) or ((OCS != 0 and passes) and UDP CS == 0)
  2. OCS != 0 and passes or OCS == 0 when UDP CS != 0

The case with OCS == 0 and UDP CS != 0 (and passes) makes both expressions true. But they seem to be intended to be mutually exclusive, since they specify mutually exclusive behavior. I call this inconsistent.

The problem is "UDP CS != 0" in the second expression. I'd say that this should be "==".

I think the following version with a modification of the only second expression might capture the intent:

  1. ((OCS != 0 and fails or OCS == 0) and UDP CS != 0) or ((OCS != 0 and passes) and UDP CS == 0)
  2. (OCS != 0 and passes) or (OCS == 0 and UDP CS == 0)

Changing "!=" to "==" is necessary. Using parentheses helps readability. Using "and" instead of "when" is more consistent.

jtouch commented 9 months ago

Ah - thanks, now I see what you're getting at, and yes, the second should be CS==0 and it's better to replace "when" with "and". Will do in next rev.

jtouch commented 7 months ago

Fixed in rev -24

Mike-Heard commented 7 months ago

Still needs work; the clause "or ((OCS != 0 and passes) and UDP CS == 0)" before the code that says to deliver the data but ignore the options should not be there.

auerswal commented 7 months ago

Indeed, that is inconsistent with section 8 which states:

   >> When the UDP checksum is zero, the OCS MAY be unused, and is then
   indicated by a zero OCS value.

The pseudo-code in section 13 requires OCS==0 when CS==0, while section 8 allows both OCS!=0 and OCS==0 when CS==0.

jtouch commented 7 months ago

OK - please confirm whether fixed in -25

auerswal commented 7 months ago

Looks good to me. :-)

Mike-Heard commented 7 months ago

Looks good to me. :-)

I concur and recommend closing this issue.

gorryfair commented 7 months ago

Resolved