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

Update ovs-pipeline.md for Egress flows #2044

Merged
merged 1 commit into from
Apr 9, 2021

Conversation

jianjuns
Copy link
Contributor

@jianjuns jianjuns commented Apr 8, 2021

No description provided.

### L3DecTTLTable (71)
When the Egress feature is enabled, there will be two extra flows added in
[L3ForwardingTable], which send the egress traffic from Pods to external network
to [SNATTable] (rather than sending the traffic the [L2ForwardingCalcTable]
Copy link
Member

Choose a reason for hiding this comment

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

Need an anchor of SNATTable in the end of the file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok. Added.


```text
table=70, priority=190,ip,reg0=0x2/0xffff actions=goto_table:72
table=70, priority=190,ip,reg0=0/0xffff actions=mod_dl_dst:e2:e5:a4:9b:1c:b1,goto_table:72
Copy link
Member

Choose a reason for hiding this comment

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

Should the goto_table be 71?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right. Fixed.

table=71, priority=0 actions=goto_table:80
```

When there is an Egress applies to a Pod on the Node, a flow will be added for
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
When there is an Egress applies to a Pod on the Node, a flow will be added for
When there is an Egress applied to a Pod on the Node, a flow will be added for

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I feel "applies" works too, but changed to "applied"

When the SNAT IP of the Egress is on a remote Node, the flow will tunnel the
packets to the remote Node with the tunnel's destination IP to be the SNAT IP.
The packets will be SNAT'd on the remote Node. The same as a normal tunnel flow
in [L3ForwardingTable], the flow will rewite the packets' source and destination
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
in [L3ForwardingTable], the flow will rewite the packets' source and destination
in [L3ForwardingTable], the flow will rewrite the packets' source and destination

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks.

Copy link
Contributor

@antoninbas antoninbas left a comment

Choose a reason for hiding this comment

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

LGTM
I also inspected the svg and it looks fine, but I am not sure about the connectors?
It seems that these 3 paths should be shown:

  • 70 to 72: "inter-Node Pod traffic"
  • 70 to 71: "to external with egress"
  • 70 to 80: "to local gateway"

In particular, the "to local" label now seems incorrect

@jianjuns
Copy link
Contributor Author

jianjuns commented Apr 8, 2021

You are right - these paths are not correct. Updated. Check. @antoninbas

Copy link
Contributor

@antoninbas antoninbas left a comment

Choose a reason for hiding this comment

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

LGTM
I think we can merge it as is, but the "to local Pod or gateway" label is in a confusing position IMO, since it looks like it also applies to traffic leaving the SNAT table. Perhaps the graph would be too crowded if we were to apply all the appropriate labels.

@jianjuns
Copy link
Contributor Author

jianjuns commented Apr 8, 2021

Yes, but in a sense for SNAT, it is also "to gateway" right? I thought about adding one for "egress at remote Node", and another for "egress at local Node", but not enough space.

@jianjuns
Copy link
Contributor Author

jianjuns commented Apr 9, 2021

/skip-all

@jianjuns jianjuns merged commit 399c616 into antrea-io:main Apr 9, 2021
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.

4 participants