Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Processing and requirements of APC and Authentication options #21

Closed
tompandadev opened this issue Sep 27, 2023 · 13 comments
Closed

Processing and requirements of APC and Authentication options #21

tompandadev opened this issue Sep 27, 2023 · 13 comments

Comments

@tompandadev
Copy link

The processing and requirements of APC and Authentication options are not clear in the draft, especially in regards to what actions receiver takes if an APC value fails to be verified or authentication fails. To be consistent with other IETF protocols, the behavior should be to drop when CRC or authentication validate fails

@moeller0
Copy link

My take in this is, UDP options clearly are not a protocol in themselves, but building blocks to create protocols running on top pf a UDP carrier abstracting out a number of assumed useful types of information a real protocol might want to use. By that logic the real protocol lives on top of UDP and hence it seems suitable to have that protocol layer make decisions about what to reject and what to keep, so passing packets with incorrect checksums on might still be in-line with how other protocols operate. But the onus to do something reasonable with those packets is on the layer on top of UDP that implements the actual protocol.

@jtouch
Copy link

jtouch commented Nov 7, 2023

The goal of UDP options is to first and foremost be backward compatible with legacy receivers. Because a legacy receiver will, by default, ignore UDP options, the default for APC is to pass the packet - along with the indicator that the checksum has failed - to the user. The user default is to accept such a packet, just as a legacy receiver would. This is IMO already clear in the document.

The default for CRC is to cease option processing but also to pass the packet up to the user. In that case, the CRC indicates only that the option area is incorrect; it is NOT an indication about the UDP user data.

Users are free to make other decisions, but that is why these are the defaults.

@tompandadev
Copy link
Author

tompandadev commented Nov 7, 2023 via email

@jtouch
Copy link

jtouch commented Nov 7, 2023 via email

@tompandadev
Copy link
Author

tompandadev commented Nov 7, 2023 via email

@jtouch
Copy link

jtouch commented Nov 7, 2023

OK, so the doc does not need an update. My response that cited CRC was the only point of confusion - I should have referred to OCS rather than CRC.

The rest of this thread raises no new issues. UDP options were always designed to behave consistently with a legacy endpoint by default - the default never adds UDP options and the default silently accepts all received options UNLESS OCS fails, at which point only the failed OCS is indicated. Just as it is the sender's prerogative what to send, it remains the receivers' prerogative which SAFE options to require or ignore.

@tompandadev
Copy link
Author

tompandadev commented Nov 7, 2023 via email

@jtouch
Copy link

jtouch commented Nov 7, 2023

Note also that UNSAFE options are designed for those that modify user data. APC does not modify user data, so it should not be considered an UNSAFE option. I

As to "typical IETF requirements", UDP options cannot simultaneously support that expectation AND equivalence with legacy receivers by default. That latter is the result of extending a widely deployed protocol with minimal impact until explicitly activated by the user.

And yes, it's an open issue, but no - none of these points are new, so they don't warrant new responses.

@tompandadev
Copy link
Author

tompandadev commented Nov 7, 2023 via email

@jtouch
Copy link

jtouch commented Nov 7, 2023 via email

@jtouch
Copy link

jtouch commented Nov 13, 2023

[not addressed in -25 - I do not see consensus on needed changes]

@jtouch
Copy link

jtouch commented Mar 3, 2024

See response to #23.

@Mike-Heard
Copy link

I think that the substantive issues are satisfactorily addressed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants