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.

Sign up for GitHub

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

Jump to bottom

Update flow exporter documentation #1280

Merged

srikartati merged 1 commit into antrea-io:master from srikartati:update_flowexp_doc

Sep 24, 2020

Member

srikartati commented Sep 22, 2020

Update the figure in flow exporter documentation

vmwclabot added the cla-not-required label

Collaborator

antrea-bot commented Sep 22, 2020

Thanks for your PR.
Unit tests and code linters are run automatically every time the PR is updated.
E2e, conformance and network policy tests can only be triggered by a member of the vmware-tanzu organization. Regular contributors to the project should join the org.

The following commands are available:

/test-e2e: to trigger e2e tests.
/skip-e2e: to skip e2e tests.
/test-conformance: to trigger conformance tests.
/skip-conformance: to skip conformance tests.
/test-whole-conformance: to trigger all conformance tests on linux.
/skip-whole-conformance: to skip all conformance tests on linux.
/test-networkpolicy: to trigger networkpolicy tests.
/skip-networkpolicy: to skip networkpolicy tests.
/test-windows-conformance: to trigger windows conformance tests.
/skip-windows-conformance: to skip windows conformance tests.
/test-windows-networkpolicy: to trigger windows networkpolicy tests.
/skip-windows-networkpolicy: to skip windows networkpolicy tests.
/test-hw-offload: to trigger ovs hardware offload test.
/skip-hw-offload: to skip ovs hardware offload test.
/test-all: to trigger all tests (except whole conformance).
/skip-all: to skip all tests (except whole conformance).

codecov-commenter commented Sep 22, 2020 •

edited

Loading

Codecov Report

Merging #1280 into master will decrease coverage by 0.70%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1280      +/-   ##
==========================================
- Coverage   55.07%   54.37%   -0.71%     
==========================================
  Files         110      115       +5     
  Lines       10596    10807     +211     
==========================================
+ Hits         5836     5876      +40     
- Misses       4187     4356     +169     
- Partials      573      575       +2

Flag	Coverage Δ
#integration-tests	`44.98% <ø> (+0.04%)`	⬆️
#unit-tests	`41.42% <ø> (-0.51%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
pkg/apiserver/storage/ram/watch.go	`84.61% <0.00%> (-3.85%)`	⬇️
pkg/agent/types/networkpolicy.go	`7.69% <0.00%> (-3.42%)`	⬇️
pkg/apiserver/certificate/cacert_controller.go	`12.76% <0.00%> (-3.03%)`	⬇️
...ntroller/networkpolicy/networkpolicy_controller.go	`70.40% <0.00%> (-2.01%)`	⬇️
pkg/agent/flowexporter/exporter/exporter.go	`35.19% <0.00%> (-1.48%)`	⬇️
pkg/ovs/openflow/ofctrl_bridge.go	`68.77% <0.00%> (-0.80%)`	⬇️
...g/controller/networkpolicy/clusternetworkpolicy.go	`63.41% <0.00%> (ø)`
pkg/controller/networkpolicy/validate.go	`0.00% <0.00%> (ø)`
pkg/controller/networkpolicy/tier.go	`0.00% <0.00%> (ø)`
pkg/apis/controlplane/v1alpha1/register.go	`83.33% <0.00%> (ø)`
... and 7 more

srikartati force-pushed the update_flowexp_doc branch 6 times, most recently from 2a39181 to dd33684 Compare

September 23, 2020 17:15

antoninbas added this to the Antrea v0.10.0 release milestone

antoninbas reviewed

View reviewed changes

docs/network-flow-visibility.md Outdated

+              ### IPFIX Fields in a Flow Record
+              There are 23 IPFIX fields in each flow record. They are from the standard IANA
+               registry, the reverse IANA registry and the Antrea registry for Antrea specific

Contributor

antoninbas Sep 23, 2020

some notes on terminology:

is "Information Element" more accurate than "field"?
"reverse IANA registry" does not seem to really match the official name: https://tools.ietf.org/html/rfc5103#section-6.1. For the registry names, I would maybe suggest "IANA-assigned IE registry", "Reverse IE registry" and "Antrea IE registry"

Member Author

srikartati Sep 23, 2020

I would like to specify IANA in "Reverse IE registry" as this corresponds to only IANA assigned information elements. I am thinking "Reverse IANA IE registry" is ok as "Reverse IANA-assigned IE registry". Any thoughts?

Contributor

antoninbas Sep 23, 2020

"Reverse IANA-assigned IE registry" works for me

docs/network-flow-visibility.md

Comment on lines +108 to +122

+              | sourcePodNamespace        | 55829         | 100      | string      |
+              | sourcePodName             | 55829         | 101      | string      |
+              | destinationPodNamespace   | 55829         | 102      | string      |
+              | destinationPodName        | 55829         | 103      | string      |
+              | sourceNodeName            | 55829         | 104      | string      |
+              | destinationNodeName       | 55829         | 105      | string      |
+              | destinationClusterIP      | 55829         | 106      | ipv4Address |
+              | destinationServicePortName| 55829         | 108      | string      |

Contributor

antoninbas Sep 23, 2020

orthogonal to this PR, but could you request an actual assignment from IANA for Antrea (https://tools.ietf.org/id/draft-pti-pen-registration-02.html#rfc.section.2.1).

It seems that 55829 is taken already:

55829  Arthur18
    Heiko Grill
      heiko.grill&diegrills.org

Since VMware has an assignment already, the correct thing to do may be to request a sub-assigment from the contact person. I have not looked into exactly what it entails. Antrea being an open-source project, it may make more sense to request a new assignment.

Member Author

srikartati Sep 24, 2020 •

edited

Loading

Thanks for bringing this up. Yes, we haven't explored in that direction yet. Few months back, we picked an available ID as Antrea enterprise ID.
I will open an issue in go-ipfix and try contacting VMware enterpriseID contact to get his suggestion. If I do not hear from him, we can apply through this link I guess: https://pen.iana.org/pen/PenApplication.page

Contributor

antoninbas Sep 24, 2020

please open an issue in Antrea repo to track this. If you already have an issue in go-ipfix, I suggest opening an issue in Antrea anyway, and linking the 2 issues.

docs/network-flow-visibility.md

               For visualizing the network flows, Antrea monitors the flows in Linux conntrack module. These flows are converted to flow records
               and are sent to the configured flow controller. High-level design is given below:
-              <img src="https://downloads.antrea.io/static/netviz.png" width="600" alt="Network Flow Visibilty">
+              ![Flow Exporter Design](assets/flow_exporter.svg)

Contributor

antoninbas Sep 23, 2020

Thanks for enabling anti-aliasing in the svg, text looks much better now

Member Author

srikartati Sep 24, 2020

Thanks for the suggestion.

docs/network-flow-visibility.md Outdated

Comment on lines 75 to 78

+              There are 23 IPFIX fields in each flow record. They are from the standard IANA
+               registry, the reverse IANA registry and the Antrea registry for Antrea specific
+                fields. The reverse IANA registry fields are used for flows in the reverse
+                 direction.

Contributor

antoninbas Sep 23, 2020

formatting is weird (white spaces at the beginning of the lines)

plus leave empty line before section header

srikartati force-pushed the update_flowexp_doc branch from dd33684 to 32206ac Compare

September 24, 2020 00:32

srikartati commented

View reviewed changes

Member Author

srikartati left a comment •

edited

Loading

Made some changes to restrict the line length to be 80.

docs/network-flow-visibility.md

               For visualizing the network flows, Antrea monitors the flows in Linux conntrack module. These flows are converted to flow records
               and are sent to the configured flow controller. High-level design is given below:
-              <img src="https://downloads.antrea.io/static/netviz.png" width="600" alt="Network Flow Visibilty">
+              ![Flow Exporter Design](assets/flow_exporter.svg)

Member Author

srikartati Sep 24, 2020

Thanks for the suggestion.

docs/network-flow-visibility.md

Comment on lines +108 to +122

+              | sourcePodNamespace        | 55829         | 100      | string      |
+              | sourcePodName             | 55829         | 101      | string      |
+              | destinationPodNamespace   | 55829         | 102      | string      |
+              | destinationPodName        | 55829         | 103      | string      |
+              | sourceNodeName            | 55829         | 104      | string      |
+              | destinationNodeName       | 55829         | 105      | string      |
+              | destinationClusterIP      | 55829         | 106      | ipv4Address |
+              | destinationServicePortName| 55829         | 108      | string      |

Member Author

srikartati Sep 24, 2020 •

edited

Loading

Thanks for bringing this up. Yes, we haven't explored in that direction yet. Few months back, we picked an available ID as Antrea enterprise ID.
I will open an issue in go-ipfix and try contacting VMware enterpriseID contact to get his suggestion. If I do not hear from him, we can apply through this link I guess: https://pen.iana.org/pen/PenApplication.page

srikartati mentioned this pull request

Antrea IPFIX registry: Create a sub-assignment under VMware enterprise registry or apply for new one vmware/go-ipfix#36

Closed

antoninbas reviewed

View reviewed changes

Contributor

antoninbas left a comment

couple nits, otherwise LGTM

docs/network-flow-visibility.md Outdated

+              parameters are set to 5s and 12, respectively. `flowCollectorAddr` is a required
+              parameter that is necessary for the Flow Exporter feature to work.
+              ### IPFIX Information Elements(IEs) in a Flow Record

Contributor

antoninbas Sep 24, 2020

Suggested change

      
            ### IPFIX Information Elements(IEs) in a Flow Record
          
            ### IPFIX Information Elements (IEs) in a Flow Record

docs/network-flow-visibility.md Outdated

Comment on lines 81 to 84

+              There are 23 IPFIX IEs in each flow record, which are registered the IANA-assigned IE
+              registry, the Reverse IANA-assigned IE registry and the Antrea IE registry. The
+              reverse IEs are used for the flow in reverse direction when representing a
+              connection. All the utilized IEs are given below:

Contributor

antoninbas Sep 24, 2020

Suggestion:

There are 23 IPFIX IEs in each flow record, which are defined in the IANA-assigned IE
registry, the Reverse IANA-assigned IE registry and the Antrea IE registry. The
reverse IEs are used to provide bi-directional information about the flow.
All the IEs used by the Antrea Flow Exporter are listed below:

docs/network-flow-visibility.md

Comment on lines +108 to +122

+              | sourcePodNamespace        | 55829         | 100      | string      |
+              | sourcePodName             | 55829         | 101      | string      |
+              | destinationPodNamespace   | 55829         | 102      | string      |
+              | destinationPodName        | 55829         | 103      | string      |
+              | sourceNodeName            | 55829         | 104      | string      |
+              | destinationNodeName       | 55829         | 105      | string      |
+              | destinationClusterIP      | 55829         | 106      | ipv4Address |
+              | destinationServicePortName| 55829         | 108      | string      |

Contributor

antoninbas Sep 24, 2020

please open an issue in Antrea repo to track this. If you already have an issue in go-ipfix, I suggest opening an issue in Antrea anyway, and linking the 2 issues.


          Update flow exporter documentation

Update the figure in flow exporter documentation

srikartati mentioned this pull request

Antrea IPFIX registry: Create a sub-assignment under VMware enterprise registry or apply for new one #1295

Closed

srikartati force-pushed the update_flowexp_doc branch from 32206ac to 5374162 Compare

September 24, 2020 18:43

Member Author

srikartati commented Sep 24, 2020

/skip-all

antoninbas approved these changes

View reviewed changes

Contributor

antoninbas left a comment

LGTM

srikartati merged commit c46900c into antrea-io:master

srikartati deleted the update_flowexp_doc branch

September 24, 2020 18:59

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet