stratum / fabric-tna

The SD-Fabric data plane
https://docs.sd-fabric.org/
31 stars 15 forks source link

[SDFAB-658] Port Fabric-int ingress drop report to v1model #437

Closed EmanueleGallone closed 2 years ago

EmanueleGallone commented 2 years ago

This PR contains a partial porting of fabric-int, for v1model. Currently, only INT ingress drop reports are supported

PTF Tests correctly executed:

Egress drop reports and flow reports are still not supported. They require metadata preservation after mirroring the packet(Clone E2E). They will be implemented in a subsequent PR.

codecov[bot] commented 2 years ago

Codecov Report

Merging #437 (8f56c4c) into main (5827eaf) will increase coverage by 0.03%. The diff coverage is 71.66%.

:exclamation: Current head 8f56c4c differs from pull request most recent head 1cfb0ad. Consider uploading reports for the commit 1cfb0ad to get more accurate results Impacted file tree graph

@@             Coverage Diff              @@
##               main     #437      +/-   ##
============================================
+ Coverage     66.92%   66.95%   +0.03%     
- Complexity      619      621       +2     
============================================
  Files            63       63              
  Lines          4444     4452       +8     
  Branches        469      472       +3     
============================================
+ Hits           2974     2981       +7     
  Misses         1235     1235              
- Partials        235      236       +1     
Impacted Files Coverage Δ
...ct/fabric/tna/behaviour/FabricIntProgrammable.java 86.01% <68.42%> (-0.01%) :arrow_down:
...abric/tna/behaviour/pipeliner/FabricPipeliner.java 30.83% <72.50%> (ø)
...stratumproject/fabric/tna/behaviour/Constants.java 100.00% <100.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 5827eaf...1cfb0ad. Read the comment docs.

EmanueleGallone commented 2 years ago

I just realized that I force pushed the new commits breaking the review. Sorry about that.

I think that we could clean up and let this PR focus only on ingress drop reports. Do you see any options to implement egress drop reports and flow reports without the _preserve primitives?

There must be a preservation of the metadata report_type and the egress_spec/port of the original packet, otherwise we lose the information about the report type (if any) and the original egress port.

Check this PR: #450 I found a way to overcome the bmv2 crash while preserving the needed information. Still WIP, though.

daniele-moro commented 2 years ago

test this please

daniele-moro commented 2 years ago

retest this please