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

Add Flow API Abstraction Implementation #524

Merged
merged 51 commits into from
Jul 12, 2024
Merged

Add Flow API Abstraction Implementation #524

merged 51 commits into from
Jul 12, 2024

Conversation

zhixiongniu
Copy link
Contributor

@zhixiongniu zhixiongniu commented Feb 26, 2024

So sorry, I accidentally closed #507.
This PR achieves same thing as #507 and I am adopting all comments from that PR.

This PR introduces the abstraction of the DASH Flow API, encompassing both the flow table and flow entry, within the bmv2 environment.

For a preview of the SAI introduced by this PR, please refer to the following branch:
SAI Flow API Preview Branch.

Signed-off-by: Zhixiong Niu <niu@zhixiong.org>
zhixiongniu and others added 24 commits March 4, 2024 09:49
Signed-off-by: Zhixiong Niu <niu@zhixiong.org>
Signed-off-by: Zhixiong Niu <niu@zhixiong.org>
Signed-off-by: Zhixiong Niu <niu@zhixiong.org>
Signed-off-by: Zhixiong Niu <niu@zhixiong.org>
Signed-off-by: Zhixiong Niu <niu@zhixiong.org>
Signed-off-by: Zhixiong Niu <niu@zhixiong.org>
Signed-off-by: Zhixiong Niu <niu@zhixiong.org>
Signed-off-by: Zhixiong Niu <niu@zhixiong.org>
Signed-off-by: Zhixiong Niu <niu@zhixiong.org>
@r12f r12f self-requested a review July 2, 2024 00:56
Signed-off-by: Zhixiong Niu <niu@zhixiong.org>
Signed-off-by: Zhixiong Niu <niu@zhixiong.org>
@KrisNey-MSFT
Copy link
Collaborator

Did you need some help with this @zhixiongniu ? Let us know!

@chrispsommers
Copy link
Collaborator

Pipeline error is here: https://github.com/sonic-net/DASH/actions/runs/9779327816/job/26998305213#step:9:15

SAI specs have changed, please run "make sai-headers" locally and commit the changes for review.

@KrisNey-MSFT
Copy link
Collaborator

KrisNey-MSFT commented Jul 10, 2024 via email

Signed-off-by: Zhixiong Niu <niu@zhixiong.org>
Signed-off-by: Zhixiong Niu <niu@zhixiong.org>
@zhixiongniu
Copy link
Contributor Author

Did you need some help with this @zhixiongniu ? Let us know!

Thanks, I have fixed the issue.

@zhixiongniu
Copy link
Contributor Author

Pipeline error is here: https://github.com/sonic-net/DASH/actions/runs/9779327816/job/26998305213#step:9:15

SAI specs have changed, please run "make sai-headers" locally and commit the changes for review.

Thanks, I have fixed the issue.

Signed-off-by: Zhixiong Niu <niu@zhixiong.org>
is_vlan: false
deprecated: false
Copy link
Collaborator

Choose a reason for hiding this comment

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

this change is odd, maybe it is coming from code merge, do you mind to help revert these changes?

dash-pipeline/bmv2/stages/conntrack_lookup.p4 Show resolved Hide resolved
dash-pipeline/bmv2/stages/conntrack_lookup.p4 Show resolved Hide resolved
action set_flow_entry_bulk_get_session_attr(
/* GRPC Session server IP and port */
IPv4ORv6Address bulk_get_session_ip,
bit<16> bulk_get_session_port,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like the mode, limit are missing. Also the name doesn't match with the spec.

Signed-off-by: Zhixiong Niu <niu@zhixiong.org>
Signed-off-by: Zhixiong Niu <niu@zhixiong.org>
Signed-off-by: Zhixiong Niu <niu@zhixiong.org>
@zhixiongniu zhixiongniu changed the title Add flow API Abstraction Implementation Add Flow API Abstraction Implementation Jul 11, 2024
@KrisNey-MSFT
Copy link
Collaborator

KrisNey-MSFT commented Jul 11, 2024 via email

Signed-off-by: Zhixiong Niu <niu@zhixiong.org>
dash-pipeline/SAI/specs/sai_spec.yaml Outdated Show resolved Hide resolved
}

enum bit<16> dash_flow_entry_bulk_get_session_mode_t

Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: empty line.

zhixiongniu and others added 3 commits July 12, 2024 05:43
Signed-off-by: Zhixiong Niu <niu@zhixiong.org>
Signed-off-by: Zhixiong Niu <niu@zhixiong.org>
@KrisNey-MSFT
Copy link
Collaborator

@r12f looks like all checks have passed - is it ready for a merge?

@r12f
Copy link
Collaborator

r12f commented Jul 12, 2024

Yep! I think so, it looks aligned with the HLD design now. Is it ok if I approved and merge it?

I checked with Marian offline, and he has no problem on this PR as well, but he is OOF recently, so maybe late on the sign-off.

@KrisNey-MSFT
Copy link
Collaborator

Yep! I think so, it looks aligned with the HLD design now. Is it ok if I approved and merge it?

I checked with Marian offline, and he has no problem on this PR as well, but he is OOF recently, so maybe late on the sign-off.

Sounds good to me, go ahead

@r12f
Copy link
Collaborator

r12f commented Jul 12, 2024

Thanks Kristina!

Copy link
Collaborator

@r12f r12f left a comment

Choose a reason for hiding this comment

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

lgtm!

@r12f r12f merged commit 2c49ba4 into sonic-net:main Jul 12, 2024
5 checks passed
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