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

Support PISC with pipelines(tna_simple_router, tiny_router_v2 without vlan module) #2

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

PFtyrant
Copy link
Collaborator

@PFtyrant PFtyrant commented Aug 19, 2020

Supporting PISC with (tna_simple_router, tiny_router_v2)
-Fix Makefile
-Fix import path of main.go
-Fix go-bfrt to pisc, bfcli to pisc-cli
-Add set-flow, del-flow, dump, info, table, version features.
-Add pre, port, vlan features.

README.md

@PFtyrant PFtyrant added the enhancement New feature or request label Aug 19, 2020
@PFtyrant PFtyrant requested a review from cmingou August 19, 2020 10:18
-Modify info, dump message format
-Add all of the table flag in dump.go
-Add PISC logo into version.go
-Fix Makefile
-Fix import path of main.go
-Fix go-bfrt to pisc, bfcli to pisc-cli
-Fix dump error of out of range case
@PFtyrant PFtyrant changed the title Modify info, dump message format [Review wanted]Modify info, dump message format Aug 22, 2020
Copy link
Collaborator

@cmingou cmingou left a comment

Choose a reason for hiding this comment

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

Can you have more comment about function in utils.go. The format can reference in https://blog.golang.org/godoc

- Add README.md
- Add description of util's function
- Remove unused variable
- Fix info that error doesn't occur when table Id is not found
Copy link
Collaborator

@cmingou cmingou left a comment

Choose a reason for hiding this comment

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

LGTM, just need to remove unused code.

cmd/utils.go Outdated
Comment on lines 72 to 75
//name, ok = p4Info.SearchActionParameterNameById(id)
//if ok == true {
// return name, found
//}
Copy link
Collaborator

Choose a reason for hiding this comment

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

If this code is not used, remove please

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done.

README.md Outdated
```
2. all : delete all entries of the table
```
//Do not confuse with set-flow's "-a" flag. It's totally diffrent.
Copy link
Collaborator

Choose a reason for hiding this comment

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

/ 和 D 之間少一個空格

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed it.

-Fix README blank in message
-Remove unused code in utils.go
@PFtyrant PFtyrant changed the title [Review wanted]Modify info, dump message format Modify info, dump message format Aug 25, 2020
@PFtyrant PFtyrant added the Review Wanted Please review this PR label Aug 25, 2020
@PFtyrant PFtyrant requested a review from cmingou August 25, 2020 05:13
@PFtyrant PFtyrant mentioned this pull request Aug 26, 2020
@PFtyrant PFtyrant added Detached Do not use this branch. Out dated. and removed Review Wanted Please review this PR enhancement New feature or request labels Sep 7, 2020
@PFtyrant PFtyrant removed the Detached Do not use this branch. Out dated. label Dec 15, 2020
PISC-CLI
-Add set-flow, del-flow, info, table, version function.

* Fixed or removed issues
- Fix table completion to remove the Parser table showing up.
- Fix del-flow out of index problem.
- Remove feature that deleting entry by entry number

* New feature
- Add deleting entry by match keys
- Modify the write request(delete) to delete multiple entities at once.
- Modify the dump function(print format)
- Add go.mod
- Add table name suggestion
- Add dump counts function
- Add the dump count flag -c to take out a number of entries

* Document related
- Modify README for the new version of the flow setup method
- Modify flow cheatsheet
@PFtyrant PFtyrant changed the title Modify info, dump message format Support () Dec 15, 2020
@PFtyrant PFtyrant changed the title Support () Support PISC with () Dec 15, 2020
@PFtyrant PFtyrant changed the title Support PISC with () Support PISC with pipelines(tna_simple_router, tiny_router_v2 without vlan module) Dec 15, 2020
- Make pisc-cli compatible with tiny_router_v2 pipeline.
- Modify dump command print format. Using tablewriter package.
- Modify info command to use string concatenation instead io buffer.
- Add read file feature to set-flow function

*W.I.P
- Show the progress using progress bar when large number of entries are come.
- make the code more compact
- Fix the counting error of the readfile feature
- Add progress bar to del-flow command
- Add progress bar to set-flow command(readfile only)
- Fix del-flow bugs
- Tested set-flow/del-flow performance(del-flow has performance issue,
Readtable function of PISC is too slow.)

W.I.P
- Fix suggestion function
- Adding PRE related function(non p4 Object),
-Tab completion provides table,action name when use set/del-flow, dump and info command
-Adding PRE related features(MGID, NODE, LAG, PRUNE)
-Adding Port feature. Port command only support add port config now. It can't modify or delete port config.
-Added the VLAN "create" and "add" features
-Fixed the flag shorthand collision problem. Now, the Server flag uses the capital charactor
-Modified PRE command's tab completion to suggest the table name with "pre." suffix
-Modified dump function bug when pipeline table key field counts don't matched to the bfrt profile
-Add delete feature. But not fully implemet. It just deletes one vlan group now.
…esponse message

-Now, the vlan add feature support range config
-read the response message from server
-Now, the "show" of VLAN cmd is available. vlan show has two options, one is show vlan by vlanId, the other is show vlan by portId.
the usage of the "show" command please refer to the document in P4Networking/doc repo.
-use JSON.marshal to decode gRPC response message.
-add show all vlans features. "pisc-cli vlan show" command will show all of the vlans
-add range delete feature.
-Delete the vlan modify feature.
-Modifed the set-flow error messages
-Fixed that the set-flow tab completion suggests abnomaly lists.
-Fixed that the del-flow can't delete the flow when using -m flag.
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.

None yet

2 participants