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

NodePort, LoadBalancer and ClusterIP full support for AntreaProxy on Linux #2599

Merged
merged 1 commit into from
Sep 16, 2021

Conversation

hongliangl
Copy link
Contributor

@hongliangl hongliangl commented Aug 16, 2021

For NodePort support, an ipset is used to store NodePort IP, port and
transparent protocol in an entry IP:port,protocol. Then an iptables
entry is used to match the ipset as destination and perform DNAT with
a virtual IP. For DNAT'd packets, a routing entry is used to route
them to Antrea gateway.

For LoadBalancer support, a routing entry will be created for every
ingress IP to route the packets from remote or localhost to Antrea
gateway.

For ClusterIP support, a routing entry is always used to route the
packets of all ClusterIPs to Antrea gateway. when a new ClusterIP is
created, the destination IP block of the routing entry might be
extended to include the ClusterIP address.

To support the Service traffic of above cases, the main changes of
OVS pipeline include:

  • Change table serviceHairpinTable ID from 29 to 23.
  • Change table hairpinSNATTable ID from 106 to 108.
  • Add table serviceConntrackTable 24 to transform SNAT'd connections.
  • Add table serviceClassifierTable 35 to classify Service traffic.
  • Add table serviceConntrackCommitTable 106 to perform SNAT for Service
    traffic.

@codecov-commenter
Copy link

codecov-commenter commented Aug 17, 2021

Codecov Report

Merging #2599 (c3705b8) into main (dad38e9) will increase coverage by 5.06%.
The diff coverage is 66.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2599      +/-   ##
==========================================
+ Coverage   60.82%   65.88%   +5.06%     
==========================================
  Files         285      284       -1     
  Lines       23041    26909    +3868     
==========================================
+ Hits        14014    17729    +3715     
+ Misses       7526     7499      -27     
- Partials     1501     1681     +180     
Flag Coverage Δ
e2e-tests 55.35% <21.91%> (?)
kind-e2e-tests 49.33% <69.17%> (+1.04%) ⬆️
unit-tests 40.88% <7.79%> (-0.45%) ⬇️

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

Impacted Files Coverage Δ
pkg/agent/config/node_config.go 94.11% <ø> (-5.89%) ⬇️
pkg/agent/util/ipset/ipset.go 74.19% <ø> (+12.65%) ⬆️
pkg/agent/util/iptables/iptables.go 43.55% <ø> (+6.67%) ⬆️
pkg/agent/wireguard/client_linux.go 65.07% <ø> (-0.10%) ⬇️
pkg/agent/proxy/proxier.go 61.55% <41.59%> (-2.55%) ⬇️
pkg/agent/route/route_linux.go 51.51% <56.80%> (+8.01%) ⬆️
pkg/agent/util/net.go 51.77% <58.92%> (+15.95%) ⬆️
pkg/agent/openflow/client.go 71.37% <66.66%> (+13.76%) ⬆️
pkg/agent/openflow/pipeline.go 84.25% <92.30%> (+11.04%) ⬆️
pkg/agent/agent.go 59.32% <95.00%> (+7.39%) ⬆️
... and 278 more

@hongliangl
Copy link
Contributor Author

/test-all

build/yamls/antrea.yml Outdated Show resolved Hide resolved
cmd/antrea-agent/util.go Outdated Show resolved Hide resolved
cmd/antrea-agent/util.go Outdated Show resolved Hide resolved
pkg/agent/config/node_config.go Outdated Show resolved Hide resolved
pkg/agent/openflow/client.go Outdated Show resolved Hide resolved
pkg/agent/openflow/client.go Outdated Show resolved Hide resolved
pkg/agent/openflow/pipeline.go Outdated Show resolved Hide resolved
pkg/agent/openflow/pipeline.go Outdated Show resolved Hide resolved
Copy link
Member

@tnqn tnqn left a comment

Choose a reason for hiding this comment

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

Not finished yet, but I feel this PR could have been separated by features: a PR to support NodePort, a PR to support host accessing ClusterIPs, a PR to support LoadBalancer IPs, which are not really relevant to each other and can help identify which change is for which feature. Please consider dividing patch for individual features in the future. If other reviewers have the same feeling as me and the review progress goes slowly, I would suggest scoping this PR to NodePort related change only.

pkg/agent/openflow/pipeline.go Outdated Show resolved Hide resolved
pkg/agent/openflow/pipeline.go Outdated Show resolved Hide resolved
pkg/agent/openflow/pipeline.go Outdated Show resolved Hide resolved
pkg/agent/openflow/pipeline.go Outdated Show resolved Hide resolved
pkg/agent/openflow/pipeline.go Outdated Show resolved Hide resolved
pkg/agent/proxy/proxier.go Show resolved Hide resolved
pkg/agent/proxy/proxier.go Outdated Show resolved Hide resolved
pkg/agent/proxy/proxier.go Show resolved Hide resolved
pkg/agent/route/route_linux.go Outdated Show resolved Hide resolved
pkg/agent/route/route_linux.go Outdated Show resolved Hide resolved
@hongliangl hongliangl force-pushed the feature-antrea-proxy-ipset branch from 25ebfcc to 8d272a7 Compare August 17, 2021 18:01
Copy link
Contributor

@jianjuns jianjuns left a comment

Choose a reason for hiding this comment

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

In the commit message:

localhost -> local host

"There are iptables rules which is used" -> "iptables rules are used"

"virtual Service IP" -> "virtual IP"

"There is a routing entry which is used" -> "A routing entry is added to"

"when new ClusterIP are" -> "When a new ClusterIP Service is"

"the destination IP block of the routing entry will be become large to include the IP address of ClusterIP" -> the destination IP block of the routing entry might be extended to include the ClusterIP address"

"there are main changes of OVS pipeline" -> "main changes of OVS pipeline include:"

build/yamls/antrea.yml Outdated Show resolved Hide resolved
pkg/agent/config/node_config.go Outdated Show resolved Hide resolved
pkg/agent/route/interfaces.go Outdated Show resolved Hide resolved
pkg/agent/openflow/pipeline.go Outdated Show resolved Hide resolved
pkg/agent/openflow/pipeline.go Outdated Show resolved Hide resolved
pkg/agent/openflow/pipeline.go Outdated Show resolved Hide resolved
pkg/agent/openflow/pipeline.go Outdated Show resolved Hide resolved
pkg/agent/openflow/client.go Outdated Show resolved Hide resolved
pkg/agent/openflow/client.go Show resolved Hide resolved
pkg/agent/route/route_linux.go Outdated Show resolved Hide resolved
pkg/agent/openflow/client.go Outdated Show resolved Hide resolved
pkg/agent/openflow/client.go Outdated Show resolved Hide resolved
pkg/agent/openflow/client.go Show resolved Hide resolved
pkg/agent/openflow/client.go Outdated Show resolved Hide resolved
pkg/agent/openflow/client.go Outdated Show resolved Hide resolved
pkg/agent/openflow/client.go Outdated Show resolved Hide resolved
pkg/agent/openflow/pipeline.go Outdated Show resolved Hide resolved
@hongliangl hongliangl force-pushed the feature-antrea-proxy-ipset branch from 984b657 to 3d67f4d Compare August 19, 2021 09:27
@hongliangl hongliangl requested review from tnqn and jianjuns August 19, 2021 09:28
@hongliangl hongliangl force-pushed the feature-antrea-proxy-ipset branch 5 times, most recently from 6d9fc27 to e7c2b4c Compare August 22, 2021 10:40
cmd/antrea-agent/util.go Show resolved Hide resolved
hack/generate-manifest.sh Outdated Show resolved Hide resolved
hack/generate-manifest.sh Outdated Show resolved Hide resolved
pkg/agent/config/node_config.go Outdated Show resolved Hide resolved
pkg/agent/config/node_config.go Outdated Show resolved Hide resolved
pkg/agent/route/route_linux.go Outdated Show resolved Hide resolved
pkg/agent/route/route_linux.go Outdated Show resolved Hide resolved
pkg/agent/util/net.go Outdated Show resolved Hide resolved
pkg/agent/util/net.go Outdated Show resolved Hide resolved
pkg/agent/util/net_linux.go Outdated Show resolved Hide resolved
@hongliangl hongliangl requested a review from tnqn August 25, 2021 10:27
@hongliangl hongliangl force-pushed the feature-antrea-proxy-ipset branch from 0190692 to f6c0c15 Compare August 25, 2021 10:46
pkg/agent/util/net.go Outdated Show resolved Hide resolved
@hongliangl hongliangl requested a review from jianjuns August 26, 2021 02:15
cmd/antrea-agent/config.go Outdated Show resolved Hide resolved
pkg/agent/agent.go Outdated Show resolved Hide resolved
pkg/agent/openflow/pipeline.go Show resolved Hide resolved
pkg/agent/openflow/pipeline.go Outdated Show resolved Hide resolved
pkg/agent/proxy/proxier.go Outdated Show resolved Hide resolved
pkg/agent/route/route_linux.go Outdated Show resolved Hide resolved
pkg/agent/route/route_linux.go Outdated Show resolved Hide resolved
pkg/agent/route/route_linux.go Outdated Show resolved Hide resolved
pkg/agent/route/route_linux.go Outdated Show resolved Hide resolved
pkg/agent/route/route_linux.go Outdated Show resolved Hide resolved
@hongliangl hongliangl requested a review from tnqn August 26, 2021 08:04
cmd/antrea-agent/config.go Outdated Show resolved Hide resolved
tnqn
tnqn previously approved these changes Sep 15, 2021
Copy link
Member

@tnqn tnqn left a comment

Choose a reason for hiding this comment

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

LGTM, thanks

@hongliangl
Copy link
Contributor Author

/test-all
/test-conformance
/test-ipv6-only-e2e
/test-ipv6-ds-e2e

@tnqn
Copy link
Member

tnqn commented Sep 15, 2021

Could we change the title to "NodePort, LoadBalancer and ClusterIP full support for AntreaProxy on Linux" as it's not only for traffic from K8s Node, and a shorter title will look neater in git logs.

@hongliangl hongliangl force-pushed the feature-antrea-proxy-ipset branch from 5b6ce39 to b3f7331 Compare September 15, 2021 15:39
@hongliangl hongliangl changed the title NodePort, LoadBalancer and ClusterIP from K8s Node support for AntreaProxy on Linux NodePort, LoadBalancer and ClusterIP full support for AntreaProxy on Linux Sep 15, 2021
@hongliangl
Copy link
Contributor Author

Could we change the title to "NodePort, LoadBalancer and ClusterIP full support for AntreaProxy on Linux" as it's not only for traffic from K8s Node, and a shorter title will look neater in git logs.

Updated.

tnqn
tnqn previously approved these changes Sep 15, 2021
Copy link
Member

@tnqn tnqn left a comment

Choose a reason for hiding this comment

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

LGTM

@tnqn
Copy link
Member

tnqn commented Sep 15, 2021

@jianjuns will you take another look?

@jianjuns
Copy link
Contributor

@jianjuns will you take another look?

No, no extra comment from me.

…Linux

For NodePort support, an ipset is used to store NodePort IP, port and
transparent protocol in an entry IP:port,protocol. Then an iptables
entry is used to match the ipset as destination and perform DNAT with
a virtual IP. For DNAT'd packets, a routing entry is used to route
them to Antrea gateway.

For LoadBalancer support, a routing entry will be created for every
ingress IP to route the packets from remote or localhost to Antrea
gateway.

For ClusterIP support, a routing entry is always used to route the
packets of all ClusterIPs to Antrea gateway. when a new ClusterIP is
created, the destination IP block of the routing entry might be
extended to include the ClusterIP address.

To support the Service traffic of above cases, the main changes of
OVS pipeline include:
- Change table serviceHairpinTable ID from 29 to 23.
- Change table hairpinSNATTable ID from 106 to 108.
- Add table serviceConntrackTable 24 to transform SNAT'd connections.
- Add table serviceClassifierTable 35 to classify Service traffic.
- Add table serviceConntrackCommitTable 106 to perform SNAT for Service
  traffic.

Signed-off-by: Hongliang Liu <lhongliang@vmware.com>
Co-authored-by: Weiqiang Tang <tangweiqiang@hotmail.com>
@hongliangl
Copy link
Contributor Author

LGTM

Thanks for reviewing, @tnqn @jianjuns. I have just updated the commit message.

@hongliangl hongliangl requested a review from tnqn September 15, 2021 15:59
@hongliangl
Copy link
Contributor Author

/test-all
/test-conformance
/test-ipv6-only-e2e
/test-ipv6-ds-e2e

@tnqn
Copy link
Member

tnqn commented Sep 16, 2021

/test-ipv6-conformance
/test-ipv6-e2e
/test-ipv6-networkpolicy
/test-ipv6-only-conformance
/test-ipv6-only-networkpolicy
/test-windows-conformance
/test-windows-e2e
/test-windows-networkpolicy

@tnqn tnqn merged commit 88885ab into antrea-io:main Sep 16, 2021
@GraysonWu
Copy link
Contributor

Quick question: I noticed that in serviceClassifierTable(35), only the flow with IP related to the current Node will be added. I think all Nodes which expose the NodePort should be considered?
For example, I have three Nodes.

NAME                     STATUS   ROLES                  AGE     VERSION   INTERNAL-IP      EXTERNAL-IP   OS-IMAGE             KERNEL-VERSION     CONTAINER-RUNTIME
k8s-node-control-plane   Ready    control-plane,master   2d23h   v1.22.1   192.168.77.100   <none>        Ubuntu 20.04.2 LTS   5.4.0-65-generic   containerd://1.4.9
k8s-node-worker-1        Ready    <none>                 2d23h   v1.22.1   192.168.77.101   <none>        Ubuntu 20.04.2 LTS   5.4.0-65-generic   containerd://1.4.9
k8s-node-worker-2        Ready    <none>                 2d23h   v1.22.1   192.168.77.102   <none>        Ubuntu 20.04.2 LTS   5.4.0-65-generic   containerd://1.4.9

And on k8s-node-worker-1, flows in serviceClassifierTable are shown below, which only include worker-1's IP.

 cookie=0x1c040000000000, duration=1586.613s, table=35, n_packets=0, n_bytes=0, priority=200,ip,nw_dst=127.0.0.1 actions=load:0x1->NXM_NX_REG4[19]
 cookie=0x1c040000000000, duration=1586.613s, table=35, n_packets=0, n_bytes=0, priority=200,ip,nw_dst=10.0.2.15 actions=load:0x1->NXM_NX_REG4[19]
 cookie=0x1c040000000000, duration=1586.613s, table=35, n_packets=0, n_bytes=0, priority=200,ip,nw_dst=192.168.77.101 actions=load:0x1->NXM_NX_REG4[19]
 cookie=0x1c040000000000, duration=1586.613s, table=35, n_packets=0, n_bytes=0, priority=200,ip,nw_dst=169.254.169.253 actions=load:0x1->NXM_NX_REG4[19]

Should flows with another two IPs below also need to be installed?

 cookie=0x1c040000000000, duration=1586.613s, table=35, n_packets=0, n_bytes=0, priority=200,ip,nw_dst=192.168.77.100 actions=load:0x1->NXM_NX_REG4[19]
 cookie=0x1c040000000000, duration=1586.613s, table=35, n_packets=0, n_bytes=0, priority=200,ip,nw_dst=192.168.77.102 actions=load:0x1->NXM_NX_REG4[19]

@hongliangl
Copy link
Contributor Author

@GraysonWu , it may be a little complicated.

We can see the flow
cookie=0x1c040000000000, duration=1586.613s, table=35, n_packets=0, n_bytes=0, priority=200,ip,nw_dst=169.254.169.253 actions=load:0x1->NXM_NX_REG4[19]

The above flow is used to match the first packet of NodePort connections from Antrea gateway (the first packet of NodePort has been DNAT'd with the virtual IP 169.254.169.253.

For other flows:

cookie=0x1c040000000000, duration=1586.613s, table=35, n_packets=0, n_bytes=0, priority=200,ip,nw_dst=127.0.0.1 actions=load:0x1->NXM_NX_REG4[19]
 cookie=0x1c040000000000, duration=1586.613s, table=35, n_packets=0, n_bytes=0, priority=200,ip,nw_dst=10.0.2.15 actions=load:0x1->NXM_NX_REG4[19]
 cookie=0x1c040000000000, duration=1586.613s, table=35, n_packets=0, n_bytes=0, priority=200,ip,nw_dst=192.168.77.101 actions=load:0x1->NXM_NX_REG4[19]

The above flows are used to match the first packet of NodePort connections from Pods on local Node.

For example, Pod A on Node whose internal IP is 192.168.77.101 accesses a Service NodePort (assumed that the NodePort port is 30000, then the URL should be 192.168.77.101:30000). The first packet of NodePort connections will hit the flow cookie=0x1c040000000000, duration=1586.613s, table=35, n_packets=0, n_bytes=0, priority=200,ip,nw_dst=192.168.77.101 actions=load:0x1->NXM_NX_REG4[19], then the packet will be matched by a flow in table serviceLBTable 41 like the following:

 cookie=0x6040000000000, duration=7.250s, table=41, n_packets=0, n_bytes=0, idle_age=7, priority=200,tcp,reg4=0x90000/0xf0000,tp_dst=30000 actions=load:0x2->NXM_NX_REG4[16..18],load:0x1->NXM_NX_REG0[19],group:8

In sum, NodePort connections from Pods on local Node to NodePort with IPs of the local Node can be done via a quick path within OVS pipeline. Flows in table serviceClassifierTable are used to mark the first packet of NodePort. For NodePort connections from Pods to remote Node, we don't set up a quick path. In the view of Pods, NodePort on remote Node is not special, just like connecting to any URLs on Internet.

hongliangl added a commit to hongliangl/antrea that referenced this pull request Sep 29, 2021
Since a SNAT ct zone is added in PR antrea-io#2599, hairpin Service
traffic can make use of the SNAT ct zone instead of current
stateless SNAT by modifying source and destination IPs.
By removing hairpin table ServiceHairpinTable and
HairpinSNATTable, the OVS pipeline can be simpler.

Pipeline modifications:
- Remove table serviceHairpinTable #23.
- Remove table HairpinSNATTable #108.
- Add table hairpinMarkTable #81 after table
  l2ForwardingCalcTable #80.

When a local Endpoint is referenced by a Service, then a
flow that matches the packet whose input and output
interfaces are the same and makes mark HairpinRegMark,
will be installed on table hairpinMarkTable. Packets with
mark HairpinRegMark will be performed SNAT with Antrea
gateway IP on table snatConntrackCommitTable.

Signed-off-by: Hongliang Liu <lhongliang@vmware.com>
hongliangl added a commit to hongliangl/antrea that referenced this pull request Sep 29, 2021
Since a SNAT ct zone is added in PR antrea-io#2599, hairpin Service
traffic can make use of the SNAT ct zone instead of current
stateless SNAT by modifying source and destination IPs.
By removing hairpin table ServiceHairpinTable and
HairpinSNATTable, the OVS pipeline can be simpler.

Pipeline modifications:
- Remove table serviceHairpinTable #23.
- Remove table HairpinSNATTable #108.
- Add table hairpinMarkTable #81 after table
  l2ForwardingCalcTable #80.

When a local Endpoint is referenced by a Service, then a
flow that matches the packet whose input and output
interfaces are the same and makes mark HairpinRegMark,
will be installed on table hairpinMarkTable. Packets with
mark HairpinRegMark will be performed SNAT with Antrea
gateway IP on table snatConntrackCommitTable.

Signed-off-by: Hongliang Liu <lhongliang@vmware.com>
hongliangl added a commit to hongliangl/antrea that referenced this pull request Sep 29, 2021
Since a SNAT ct zone is added in PR antrea-io#2599, hairpin Service
traffic can make use of the SNAT ct zone instead of current
stateless SNAT by modifying source and destination IPs.
By removing hairpin table ServiceHairpinTable and
HairpinSNATTable, the OVS pipeline can be simpler.

Pipeline modifications:
- Remove table serviceHairpinTable #23.
- Remove table HairpinSNATTable #108.
- Add table hairpinMarkTable #81 after table
  l2ForwardingCalcTable #80.

When a local Endpoint is referenced by a Service, then a
flow that matches the packet whose input and output
interfaces are the same and makes mark HairpinRegMark,
will be installed on table hairpinMarkTable. Packets with
mark HairpinRegMark will be performed SNAT with Antrea
gateway IP on table snatConntrackCommitTable.

Signed-off-by: Hongliang Liu <lhongliang@vmware.com>
hongliangl added a commit to hongliangl/antrea that referenced this pull request Sep 29, 2021
Since a SNAT ct zone is added in PR antrea-io#2599, hairpin Service
traffic can make use of the SNAT ct zone instead of current
stateless SNAT by modifying source and destination IPs.
By removing hairpin table ServiceHairpinTable and
HairpinSNATTable, the OVS pipeline can be simpler.

Pipeline modifications:
- Remove table serviceHairpinTable #23.
- Remove table HairpinSNATTable #108.
- Add table hairpinMarkTable #81 after table
  l2ForwardingCalcTable #80.

When a local Endpoint is referenced by a Service, then a
flow that matches the packet whose input and output
interfaces are the same and makes mark HairpinRegMark,
will be installed on table hairpinMarkTable. Packets with
mark HairpinRegMark will be performed SNAT with Antrea
gateway IP on table snatConntrackCommitTable.

Signed-off-by: Hongliang Liu <lhongliang@vmware.com>
hongliangl added a commit to hongliangl/antrea that referenced this pull request Sep 29, 2021
Since a SNAT ct zone is added in PR antrea-io#2599, hairpin Service
traffic can make use of the SNAT ct zone instead of current
stateless SNAT by modifying source and destination IPs.
By removing hairpin table ServiceHairpinTable and
HairpinSNATTable, the OVS pipeline can be simpler.

Pipeline modifications:
- Remove table serviceHairpinTable #23.
- Remove table HairpinSNATTable #108.
- Add table hairpinMarkTable #81 after table
  l2ForwardingCalcTable #80.

When a local Endpoint is referenced by a Service, then a
flow that matches the packet whose input and output
interfaces are the same and makes mark HairpinRegMark,
will be installed on table hairpinMarkTable. Packets with
mark HairpinRegMark will be performed SNAT with Antrea
gateway IP on table snatConntrackCommitTable.

Signed-off-by: Hongliang Liu <lhongliang@vmware.com>
@hongliangl hongliangl deleted the feature-antrea-proxy-ipset branch April 28, 2022 11:57
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.

6 participants