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

Minimal changes to support off-device buffering #81

Merged
merged 13 commits into from
Oct 1, 2020
Merged

Conversation

robertmacdavid
Copy link
Contributor

@robertmacdavid robertmacdavid commented Sep 25, 2020

This PR makes several additions to the pipeline to support off-device buffering. These changes are fully backwards-compatible with the old control plane. Actions and tables have only been added, not modified.

  • Added an action to the interface table for decapsulating packets returning from dbuf, so they may traverse the normal PDR and FAR pipeline
  • Added an action to the FAR table for redirecting packets to a buffering device
  • Added gateways to the ingress and egress counters so buffered packets are not double counted, since they traverse the switch twice.
  • Made some tweaks to reduce the pipeline depth.

@onf-bot
Copy link
Collaborator

onf-bot commented Sep 25, 2020

Can one of the admins verify this patch? For help please reach out to support@opennetworking.org. The list of admins can be updated in the Jenkins configuration.

@ccascone
Copy link
Member

add to allowlist

p4src/include/control/spgw.p4 Show resolved Hide resolved
p4src/include/control/spgw.p4 Outdated Show resolved Hide resolved
Copy link
Member

@ccascone ccascone left a comment

Choose a reason for hiding this comment

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

We should add PTF tests for packets from dbuf. Sending packets to dbuf is equivalent to downlink, right?

p4src/include/control/spgw.p4 Outdated Show resolved Hide resolved
p4src/include/control/spgw.p4 Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Sep 25, 2020

Codecov Report

Merging #81 into master will decrease coverage by 0.27%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master      #81      +/-   ##
============================================
- Coverage     66.83%   66.56%   -0.28%     
- Complexity      200      203       +3     
============================================
  Files            17       17              
  Lines          1598     1585      -13     
  Branches        122      123       +1     
============================================
- Hits           1068     1055      -13     
  Misses          448      448              
  Partials         82       82              
Impacted Files Coverage Δ Complexity Δ
...mproject/fabric/tna/behaviour/P4InfoConstants.java 100.00% <100.00%> (ø) 1.00 <0.00> (ø)
...ct/fabric/tna/behaviour/FabricIntProgrammable.java 80.27% <0.00%> (+0.84%) 35.00% <0.00%> (+3.00%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2a0a83b...10c8c53. Read the comment docs.

Copy link
Member

@ccascone ccascone left a comment

Choose a reason for hiding this comment

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

P4 changes look good. I left comments on the PTF tests.

ptf/tests/ptf/fabric.ptf/test.py Show resolved Hide resolved
ptf/tests/ptf/fabric.ptf/test.py Outdated Show resolved Hide resolved
ptf/tests/ptf/fabric_test.py Outdated Show resolved Hide resolved
ptf/tests/ptf/fabric_test.py Show resolved Hide resolved
ptf/tests/ptf/fabric_test.py Outdated Show resolved Hide resolved
ptf/tests/ptf/fabric_test.py Outdated Show resolved Hide resolved
ptf/tests/ptf/fabric_test.py Outdated Show resolved Hide resolved

exp_pkt = pkt.copy()

exp_pkt[Ether].src = exp_pkt[Ether].dst
Copy link
Member

Choose a reason for hiding this comment

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

We should have a utility method to swap mac addresses, something like route_pkt, which greatly improves readability

ptf/tests/ptf/fabric_test.py Outdated Show resolved Hide resolved
ptf/tests/ptf/fabric.ptf/test.py Outdated Show resolved Hide resolved
ptf/tests/ptf/fabric.ptf/test.py Outdated Show resolved Hide resolved
ptf/tests/ptf/fabric.ptf/test.py Outdated Show resolved Hide resolved
@ccascone
Copy link
Member

retest this please, codecov upload failed

@ccascone
Copy link
Member

Waiting for Jenkins to ✅, will merge after that. @robertmacdavid can you please update the PR description, I believe this no longer applies:

interface table is now TCAM and thus requires a priority

@ccascone
Copy link
Member

ccascone commented Oct 1, 2020

retest this please, the previous run timed out (30 min)

@ccascone
Copy link
Member

ccascone commented Oct 1, 2020

retest this please

@ccascone ccascone merged commit 58e4dec into master Oct 1, 2020
@ccascone ccascone deleted the simpleBuffering branch October 1, 2020 18:01
ccascone added a commit that referenced this pull request Oct 5, 2020
ccascone added a commit that referenced this pull request Oct 5, 2020
@ccascone ccascone restored the simpleBuffering branch October 5, 2020 19:27
robertmacdavid added a commit that referenced this pull request Oct 21, 2020
This is a restoration of #81 that was reverted due to introducing a table read error on hardware. There are a few new changes aside from fixing said bug, include a reworking of the spgw entity namespaces and minor action signature simplification.

* Minimal changes to support off-device buffering

* Addressed review comments

* more review comments

* saving work to merge with master

* added ptf test for dbuf release

* better counter skip fix that I came up with by myself

* some leftover crap

* addressed some review comments, added another test

* more review comments

* addressed final review comments

* missed max's comments

* some pipe cleanup

* added temporary read-write symmetry test

* review comment

* added tv generation skip for symmetry test

* review comments

* Updated SPGW tests to verify byte counts

Co-authored-by: Robert MacDavid <robert@opennetworking.org>
bocon13 pushed a commit that referenced this pull request Dec 7, 2020
This PR makes several additions to the pipeline to support off-device buffering.
These changes are fully backwards-compatible with the old control plane. Actions
and tables have only been added, not modified.

* Added an action to the interface table for decapsulating packets returning
  from dbuf, so they may traverse the normal PDR and FAR pipeline
* Added an action to the FAR table for redirecting packets to a buffering device
* Added gateways to the ingress and egress counters so buffered packets are not
  double counted, since they traverse the switch twice.
* Made some tweaks to reduce the pipeline depth.
bocon13 pushed a commit that referenced this pull request Dec 7, 2020
bocon13 pushed a commit that referenced this pull request Dec 7, 2020
This is a restoration of #81 that was reverted due to introducing a table read error on hardware. There are a few new changes aside from fixing said bug, include a reworking of the spgw entity namespaces and minor action signature simplification.

* Minimal changes to support off-device buffering

* Addressed review comments

* more review comments

* saving work to merge with master

* added ptf test for dbuf release

* better counter skip fix that I came up with by myself

* some leftover crap

* addressed some review comments, added another test

* more review comments

* addressed final review comments

* missed max's comments

* some pipe cleanup

* added temporary read-write symmetry test

* review comment

* added tv generation skip for symmetry test

* review comments

* Updated SPGW tests to verify byte counts

Co-authored-by: Robert MacDavid <robert@opennetworking.org>
bocon13 pushed a commit that referenced this pull request Dec 7, 2020
This PR makes several additions to the pipeline to support off-device buffering.
These changes are fully backwards-compatible with the old control plane. Actions
and tables have only been added, not modified.

* Added an action to the interface table for decapsulating packets returning
  from dbuf, so they may traverse the normal PDR and FAR pipeline
* Added an action to the FAR table for redirecting packets to a buffering device
* Added gateways to the ingress and egress counters so buffered packets are not
  double counted, since they traverse the switch twice.
* Made some tweaks to reduce the pipeline depth.
bocon13 pushed a commit that referenced this pull request Dec 7, 2020
bocon13 pushed a commit that referenced this pull request Dec 7, 2020
This is a restoration of #81 that was reverted due to introducing a table read error on hardware. There are a few new changes aside from fixing said bug, include a reworking of the spgw entity namespaces and minor action signature simplification.

* Minimal changes to support off-device buffering

* Addressed review comments

* more review comments

* saving work to merge with master

* added ptf test for dbuf release

* better counter skip fix that I came up with by myself

* some leftover crap

* addressed some review comments, added another test

* more review comments

* addressed final review comments

* missed max's comments

* some pipe cleanup

* added temporary read-write symmetry test

* review comment

* added tv generation skip for symmetry test

* review comments

* Updated SPGW tests to verify byte counts

Co-authored-by: Robert MacDavid <robert@opennetworking.org>
bocon13 pushed a commit that referenced this pull request Dec 8, 2020
This PR makes several additions to the pipeline to support off-device buffering.
These changes are fully backwards-compatible with the old control plane. Actions
and tables have only been added, not modified.

* Added an action to the interface table for decapsulating packets returning
  from dbuf, so they may traverse the normal PDR and FAR pipeline
* Added an action to the FAR table for redirecting packets to a buffering device
* Added gateways to the ingress and egress counters so buffered packets are not
  double counted, since they traverse the switch twice.
* Made some tweaks to reduce the pipeline depth.
bocon13 pushed a commit that referenced this pull request Dec 8, 2020
bocon13 pushed a commit that referenced this pull request Dec 8, 2020
This is a restoration of #81 that was reverted due to introducing a table read error on hardware. There are a few new changes aside from fixing said bug, include a reworking of the spgw entity namespaces and minor action signature simplification.

* Minimal changes to support off-device buffering

* Addressed review comments

* more review comments

* saving work to merge with master

* added ptf test for dbuf release

* better counter skip fix that I came up with by myself

* some leftover crap

* addressed some review comments, added another test

* more review comments

* addressed final review comments

* missed max's comments

* some pipe cleanup

* added temporary read-write symmetry test

* review comment

* added tv generation skip for symmetry test

* review comments

* Updated SPGW tests to verify byte counts

Co-authored-by: Robert MacDavid <robert@opennetworking.org>
bocon13 pushed a commit that referenced this pull request Dec 8, 2020
This PR makes several additions to the pipeline to support off-device buffering.
These changes are fully backwards-compatible with the old control plane. Actions
and tables have only been added, not modified.

* Added an action to the interface table for decapsulating packets returning
  from dbuf, so they may traverse the normal PDR and FAR pipeline
* Added an action to the FAR table for redirecting packets to a buffering device
* Added gateways to the ingress and egress counters so buffered packets are not
  double counted, since they traverse the switch twice.
* Made some tweaks to reduce the pipeline depth.
bocon13 pushed a commit that referenced this pull request Dec 8, 2020
bocon13 pushed a commit that referenced this pull request Dec 8, 2020
This is a restoration of #81 that was reverted due to introducing a table read error on hardware. There are a few new changes aside from fixing said bug, include a reworking of the spgw entity namespaces and minor action signature simplification.

* Minimal changes to support off-device buffering

* Addressed review comments

* more review comments

* saving work to merge with master

* added ptf test for dbuf release

* better counter skip fix that I came up with by myself

* some leftover crap

* addressed some review comments, added another test

* more review comments

* addressed final review comments

* missed max's comments

* some pipe cleanup

* added temporary read-write symmetry test

* review comment

* added tv generation skip for symmetry test

* review comments

* Updated SPGW tests to verify byte counts

Co-authored-by: Robert MacDavid <robert@opennetworking.org>
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