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

Wildcard read vs "Don't care" entry read #462

Open
saynb opened this issue Oct 13, 2023 · 8 comments
Open

Wildcard read vs "Don't care" entry read #462

saynb opened this issue Oct 13, 2023 · 8 comments

Comments

@saynb
Copy link
Contributor

saynb commented Oct 13, 2023

The spec has guidelines for wildcard reads here.
But if there exists a table with just ternary fields and there is one entry which has all don't care bits, there is no way to specify whether user wants to execute wildcard read or reading that specific don't care read.

@saynb
Copy link
Contributor Author

saynb commented Oct 13, 2023

This issue came up in the GenericTable discussion and we found out that this issue was present in p4runtime outside GenericTables too

@antoninbas
Copy link
Member

That's a real issue, and I wished that we had wrapped the repeated FieldMatch match field into another Protobuf message.

Even if one feels that in order to read "one entry which has all don't care bits", one has to provide the correct priority value (which is how I feel), there is still ambiguity, given that the spec supports priority filtering for wildcard reads.

@saynb
Copy link
Contributor Author

saynb commented Oct 15, 2023

That's a real issue, and I wished that we had wrapped the repeated FieldMatch match field into another Protobuf message.

Even if one feels that in order to read "one entry which has all don't care bits", one has to provide the correct priority value (which is how I feel), there is still ambiguity, given that the spec supports priority filtering for wildcard reads.

Do you think it makes sense to wrap both priority and repeated FieldMatch match in another protofbuf message? We can rectify that mistake in GenericTable at least while we figure out an alternative way for TableEntry.

@antoninbas
Copy link
Member

I thought of that too:

message Key {
    int32 priority = 1
    repeated FieldMatch match = 2
}

Unfortunately, it doesn't seem to work for all the cases we currently support or want to support. For example, if I provide { priority = 12 } (match is not set) in a table entry read, which one of these am I trying to do:

  1. read all match entries with priority 12
  2. read the match entry (at most one) which has all don't care bits and priority 12

@jafingerhut
Copy link
Contributor

I would suggest that if you are designing new message formats for GenericTable, and you want to support wildcard reads that can match 0 or more entries installed in the device and return all matching ones, that you strongly consider an approach where there is a completely separate field or fields in a read request message, e.g. a new boolean field wildcardRead, that when false means that the read is always targeting at most one entry, and when true means that the read is definitely targeting all matching entries (and that you then precisely define what a matching entry means, vs. what is in the message).

Anything else seems to me to lead to very subtle edge cases.

@jonathan-dilorenzo
Copy link
Contributor

That still wouldn't let you read all entries where a particular field was a wild card or something like that though.

One possible scheme that I think allows everything we want is to make the meaning of omitting a match field in a write request and read request a bit different:

  • If you omit the match field in the read request, then it is treated as a wild card read for that field.
  • If you add the field, but with the mask set to 0, then it only reads entries that do a wild card match on that field.

Or something like that.

@chrispsommers
Copy link
Collaborator

@jonathan-dilorenzo
Aren't omitting the field, or setting it to zero, indistinguishable in the wire protocol (which elides zero fields), hence this very problem we're debating?

@jonathan-dilorenzo
Copy link
Contributor

Ah, sorry. When I say 'with the mask set to 0' I suppose I mean just an empty match field message as supposed to an omitted one (which are distinguishable in the wire protocol, and the former isn't allowed in write requests).

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