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

adds the list of open and currently unsupported issues. #641

Merged
merged 3 commits into from
May 17, 2017
Merged

Conversation

cc10512
Copy link
Contributor

@cc10512 cc10512 commented May 16, 2017

No description provided.

README.md Outdated
## Backends

### Bmv2
* All checksum verification happens in the parser.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know what this means. Checksum verification has a separate stage in v1model.p4.
Checksum verification/update is never conditional - when it was implemented bmv2 didn't support conditional checksums.

README.md Outdated

### Bmv2
* All checksum verification happens in the parser.
* Range match types in tables
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The compiler supports range matches, perhaps BMv2 doesn't.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

supported by bmv2

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mbudiu-vmw see this issue: #357

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes the issue is not with tables, it is with select.

README.md Outdated
### Bmv2
* All checksum verification happens in the parser.
* Range match types in tables
* Tables with multiple apply calls
Copy link
Contributor

@mihaibudiu mihaibudiu May 16, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The semantics of such calls in P4-14 is not defined.
The bmv2 back-end only supports limited classes of programs where a table can be invoked multiple times.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mbudiu-vmw What do you see lacking in terms of missing semantics in P4-14 for tables with multiple apply calls? In the absence of direct counters and meters, at least, the desired behavior seems fairly clear to me -- repeat construction of a search key, the match operation, and repeat whatever action is found (which could be a different action than happened in a previous apply on the same table, because the search key contents could be different).

Even in the presence of side effects like counters and meters, repeating those sounds like the straightforward desired thing to do.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The question is whether it is the same table or another table with the same type (in P4-16 parlance, it is the same instance or another one?)

The spec was not clear originally, I have not looked lately.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may be a slip on precisely defining the semantics, however, it turns out that when folks call a table multiple times in what they potentially interpret as mutually exclusive paths, they do not go to check the spec, they complain about the compiler ... Since implementing this in bmv2 requires work that we're not currently planning to do, I think it is safer to call it out.

README.md Outdated

## Unsupported P4_16 language features

These are some of the unsupported features we are aware of. We will
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Frankly, all unsupported features that we are aware of already have issues filed. So this list is a duplicate.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do think it is nice to have an exhaustive list in one place

README.md Outdated

## Backends

### Bmv2
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Antonin already created such a list in the BMv2 backend readme file.
Maybe we should have only one such list.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree with @mbudiu-vmw. Can you remove the list in the bmv2 backend readme if we are going to be using the top-level README? Or maybe replace it with a pointer to this new list?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Antonin's list is specific to bmv2. We're trying here to capture what is the delta between what are the gaps in the compiler's bmv2 backend.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But the two lists overlap. It would be much easier for people to read only one list.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My list was specific to the bmv2 backend, not bmv2 itself. I am not sure I understand your remark @cc10512. A list of bmv2 limitations would hardly belong in this repo.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, my bad. I didn't realize that the list was for the bmv2 backend. I will link to that file.

README.md Outdated

- header_union support is incomplete

- nested structs in structs used as block constructor parameters
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an issue only if using v1model.

README.md Outdated
}
```

- explicit transition to reject in parse state
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is only a problem with bmv2.

README.md Outdated

- compound action parameters (can only be `bit<>` or `int<>`)

- functions or methods with a compound return type
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is only a problem with bmv2. The compiler supports this with no problem. But there are no such functions in v1model, so this should not be an issue, unless one wants to provide additional extern blocks.

README.md Outdated
}
```

- user-defined extern types / methods which are not defined in `v1model.p4`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To support these we first have to define how the calling conventions in P4 are mapped to BMv2 C++ code.

@cc10512
Copy link
Contributor Author

cc10512 commented May 16, 2017

@mbudiu-vmw, I think it is good for someone who downloads the compiler to have a place to check for unsupported features. Even if the compiler frontend can parse all the P4 codes, it doesn't mean that all backends generate code that runs. Yes, there are opened issues for most of the items listed in this document, however, we often get questions on the mailing list asking about one issue or another. Having them in a file that is distributed hopefully will help people quickly understand what are some of the issues.

@mihaibudiu
Copy link
Contributor

The point I am trying to make is that we will have to be careful to update the README as issues are fixed or filed.

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

Successfully merging this pull request may close these issues.

5 participants