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

Want built-in field validation mechanisms #2

Open
FelixMcFelix opened this issue Nov 22, 2024 · 1 comment
Open

Want built-in field validation mechanisms #2

FelixMcFelix opened this issue Nov 22, 2024 · 1 comment
Assignees

Comments

@FelixMcFelix
Copy link
Collaborator

Currently, consumers such as OPTE need to manually check certain header validity properties on parsed packets -- mainly IP and Geneve version fields. It would be nice to include these as part of the packet definition, and factor them into (de)parsing.

These should be optional -- ideally parsed packets which pass muster would have this represented as a distinct typestate.

@FelixMcFelix
Copy link
Collaborator Author

FelixMcFelix commented Feb 18, 2025

@zeeshanlakhani and I talked this over earlier -- to some extent this going to be key for IGMP because we want to catch and validate far more application-level facets of input data in keeping with the spec (and to do this with as little manual effort as possible). There are a couple of things to keep in mind.

  • As above, we want these to be optional where possible.
  • The error type Error::IllegalValue I added in anticipation of validation, is a bit anaemic relative to what we want. Ideally, we want to see a few things:
    • At the very least we want to capture the name of the field which violated a constraint, and said value.
    • We want to, in some circumstances, capture every validation failure associated with a packet. Doing this might not happen in OPTE/kernel-space given the design of DError, but for userland daemons this would be excellent.
  • We're not immediately sure whether validation should necessarily happen during parsing, or afterwards as a discrete step. Part of this comes down to not wanting to completely discard a packet which just happens to, say, provide non-multicast addresses in a given IGMP report. We probably want something in the middle such that a packet which fails to validate on all specifications should still be returned to the caller?
  • Validations themselves can/should be specified in a similar syntax to variable length bounds on Vec<u8>s today, except their expression would return a bool instead of a usize.
    • This gives us a convenient way to pick out what label a given validation failure should have (even if it is derived from, e.g., field1 < field2 && field1 > field3) as well as a value we can pin the blame on.
    • If we apply our constraints after extracting a full packet (today's notion of Valid), then we don't need to ensure that referenced fields come only from ancestor chunks (we know they're all present by that point!).
  • Typestate to represent packets which are valid in both senses would be nice -- I'm not sure what this implies for the mutability of the wrapped packet, however (is every setter now fallible, or just outlawed?).
  • Nomenclature is a bit tricky. Valid today is used to mean 'a valid byte layout', whereas the notion of validity here is closer to 'spec-compliance'. Both are key, but from a naming standpoint there can be only one (and naming is hard). If we can come up with a better name for valid byte-slice-backed packets, we should.

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

2 participants