Support Service loadBalancerSourceRanges in AntreaProxy #6181

hongliangl · 2024-04-02T12:55:20Z

This commit introduces support for loadBalancerSourceRanges for LoadBalancer
Services.

Here is an example of a LoadBalancer Service configuration allowing access
from specific CIDRs:

apiVersion: v1
kind: Service
metadata:
  name: sample-loadbalancer-source-ranges
spec:
  selector:
    app: web
  ports:
    - protocol: TCP
      port: 80
      targetPort: 80
  type: LoadBalancer
  loadBalancerSourceRanges:
    - "192.168.77.0/24"
    - "192.168.78.0/24"
status:
  loadBalancer:
    ingress:
      - ip: 192.168.77.150

[New] Here are the corresponding flows:

1. table=ServiceMark, priority=200,tcp,nw_src=192.168.77.0/24,nw_dst=192.168.77.150,tp_dst=80 actions=set_field:0x20000000/0x60000000->reg4",
2. table=ServiceMark, priority=200,tcp,nw_src=192.168.78.0/24,nw_dst=192.168.77.150,tp_dst=80 actions=set_field:0x20000000/0x60000000->reg4",
3. table=ServiceMark, priority=190,tcp,nw_dst=192.168.77.150,tp_dst=80 actions=set_field:0x40000000/0x60000000->reg4",
4. table=ServiceLB, priority=200,tcp,reg4=0x0x20010000/0x0x60070000,nw_dst=192.168.77.150,tp_dst=80 actions=set_field:0x200/0x200->reg0,set_field:0x20000/0x70000->reg4,set_field:0xe->reg7,group:14
5. table=ServiceLB, priority=190,reg4=0x40000000/0x60000000 actions=drop

Flow 1 is to match packets from allowed CIDR 192.168.77.0/24, marking them with
LoadBalancerSourceRangesAllowRegMark.
Flow 2 is similar to flow 1 but for CIDR 192.168.78.0/24.
Flow 3 is to match packets not from allowed CIDRs, marking with
LoadBalancerSourceRangesDropRegMark.
Flow 4 is to match allowed packets with LoadBalancerSourceRangesAllowRegMark and
perform load balancing.
Flow 5 is to match not allowed packets with LoadBalancerSourceRangesDropRegMark and
drop.

github-actions · 2024-07-29T00:04:19Z

This PR is stale because it has been open 90 days with no activity. Remove stale label or comment, or this will be closed in 90 days

hongliangl · 2024-09-03T07:48:34Z

@tnqn @wenyingd @antoninbas Could you help have a look at this PR? Thanks a lot

antoninbas

I did a quick review, but @tnqn and @wenyingd should definitely take a look if possible.

I wonder if we should have some e2e test coverage for this?
One edge case I can think of would be a NodePort Service with loadBalancerSourceRanges; according to the kube-proxy code base, loadBalancerSourceRanges should not apply to NodePort traffic.

antoninbas · 2024-09-04T22:51:38Z

docs/design/ovs-pipeline.md

+3. table=ServiceMark, priority=200,tcp,nw_src=192.168.77.0/24,nw_dst=192.168.77.152,tp_dst=80 actions=set_field:0x20000000/0x20000000->reg4",
+4. table=ServiceMark, priority=200,tcp,nw_src=192.168.78.0/24,nw_dst=192.168.77.152,tp_dst=80 actions=set_field:0x20000000/0x20000000->reg4",
+5. table=ServiceMark, priority=190,tcp,nw_dst=192.168.77.152,tp_dst=80 actions=set_field:0x4000/0x4000->reg0",


I am probably missing something obvious, but what is the point of setting LoadBalancerSourceRangesRegMark in flows 3 and 4? If the source IP doesn't fall in the correct range, we mark the packet for rejection in flow 5, so it doesn't seem that we need to remember that the source IP was in the correct range?

I have updated the commit message. Maybe it can help.

antoninbas · 2024-09-04T22:56:12Z

docs/design/ovs-pipeline.md

+Service `loadBalancerSourceRanges`. It's worth noting that `loadBalancerSourceRanges` only exclusively influences the
+traffic from the external network as `LoadBalancerSourceRangesRegMark` is not used in flow 10.


Does this match the kube-proxy behavior? I took a quick look and it seems that even when traffic comes from local Pods, loadBalancerSourceRanges is enforced

https://github.com/kubernetes/kubernetes/blob/8119e57c070d21f2a7ff37fa7b02f7f74331e393/pkg/proxy/iptables/proxier_test.go#L2192-L2205

I may be misunderstanding the comment / implementation

You are right. Since K8s 1.25, the Pod CIDR is not allowed by default. I have updated the implementation.

hongliangl · 2024-09-13T11:08:15Z

I wonder if we should have some e2e test coverage for this?

Added

One edge case I can think of would be a NodePort Service with loadBalancerSourceRanges; according to the kube-proxy code base, loadBalancerSourceRanges should not apply to NodePort traffic.

In the current implementation, loadBalancerSourceRanges only applies to LoadBalancer ingress IPs.

docs/design/ovs-pipeline.md

wenyingd · 2024-09-27T07:56:34Z

docs/design/ovs-pipeline.md

+If you dump the flows of this table, you may see the following:
+
+```text
+1. table=LoadBalancerSourceRanges, priority=200,tcp,nw_src=192.168.77.0/24,nw_dst=192.168.77.152,tp_dst=80 actions=goto_table:SessionAffinity",


When reading the CR example, the thought in my mind is to use OVS conjunction, do you think that works?

In current design, we can leverage most code of pkg/agent/proxier.go and pkg/agent/openflow. For example, like InstallServiceFlows in pkg/agent/openflow/client.go, installing a single ClusterIP, LoadBalancerIP, externalIP or NodePort of a Service. If we use conjunction, at least we may add a new function or enhance InstallServiceFlows to install multiple LoadBalancerIPs. Besides, we may also need a conjunction id allocator.

wenyingd · 2024-09-27T07:58:33Z

docs/design/ovs-pipeline.md

+```text
+1. table=LoadBalancerSourceRanges, priority=200,tcp,nw_src=192.168.77.0/24,nw_dst=192.168.77.152,tp_dst=80 actions=goto_table:SessionAffinity",
+2. table=LoadBalancerSourceRanges, priority=200,tcp,nw_src=192.168.78.0/24,nw_dst=192.168.77.152,tp_dst=80 actions=goto_table:SessionAffinity",
+3. table=LoadBalancerSourceRanges, priority=190,tcp,nw_dst=192.168.77.152,tp_dst=80 actions=drop",


For the packets that don't locate in the source range, is the "Drop" action or "Reject" action expected?

Drop is expected, as kube-proxy's behavior.

pkg/agent/openflow/framework.go

pkg/agent/openflow/pipeline.go

wenyingd · 2024-09-27T08:24:48Z

pkg/agent/openflow/client.go

@@ -799,6 +799,9 @@ func (c *client) InstallServiceFlows(config *types.ServiceConfig) error {
 	if config.IsDSR {
 		flows = append(flows, c.featureService.dsrServiceMarkFlow(config))
 	}
+	if len(config.LoadBalancerSourceRanges) != 0 {


ditto, does it need to be scope with condition proxyLoadBalancerIPs==true?

No, the function will be call in pkg/agent/proxier.go when proxyLoadBalancerIPs==true.

wenyingd · 2024-09-27T08:27:14Z

pkg/agent/openflow/pipeline.go

@@ -3106,6 +3107,34 @@ func (f *featureService) gatewaySNATFlows() []binding.Flow {
 	return flows
 }

+func (f *featureService) loadBalancerSourceRangesMarkFlows(config *types.ServiceConfig) []binding.Flow {


We didn't introduce any register mark for LoadBalancer traffic with the check on the source range, and we don't add mark on the packet via the flows built in this function, so maybe remove word "Mark" from the function name? Or rename it as loadBalancerSourceRangesValidateFlows?

I have changed the designed back because I found the drop action doesn't take effect. For example:

table=1, priority=200,ip actions=resubmit(,2),resubmit(,3),resubmit(,4),resubmit(,5) table=2, priority=0, actions=goto_tables:3 table=3, priority=190,tcp,nw_dst=169.254.169.1,tp_dst=8080 actions=drop table=4, priority=0,actions=load:0x1->NXM_NX_REG4[19] table=5, ......

In above flows, the packet destined for 169.254.169.1 matched by the flow in table 3 will not be dropped when I made some tests.

antrea-bot · 2024-09-30T07:30:53Z

Can one of the admins verify this patch?

For antrea-io#5493 This commit introduces support for loadBalancerSourceRanges for LoadBalancer Services. Here is an example of a LoadBalancer Service configuration allowing access from specific CIDRs: ```yaml apiVersion: v1 kind: Service metadata: name: sample-loadbalancer-source-ranges spec: selector: app: web ports: - protocol: TCP port: 80 targetPort: 80 type: LoadBalancer loadBalancerSourceRanges: - "192.168.77.0/24" - "192.168.78.0/24" status: loadBalancer: ingress: - ip: 192.168.77.150 ``` [New] Here are the corresponding flows: ```text 1. table=ServiceMark, priority=200,tcp,nw_src=192.168.77.0/24,nw_dst=192.168.77.150,tp_dst=80 actions=set_field:0x20000000/0x60000000->reg4", 2. table=ServiceMark, priority=200,tcp,nw_src=192.168.78.0/24,nw_dst=192.168.77.150,tp_dst=80 actions=set_field:0x20000000/0x60000000->reg4", 3. table=ServiceMark, priority=190,tcp,nw_dst=192.168.77.150,tp_dst=80 actions=set_field:0x40000000/0x60000000->reg4", 4. table=ServiceLB, priority=200,tcp,reg4=0x0x20010000/0x0x60070000,nw_dst=192.168.77.150,tp_dst=80 actions=set_field:0x200/0x200->reg0,set_field:0x20000/0x70000->reg4,set_field:0xe->reg7,group:14 5. table=ServiceLB, priority=190,reg4=0x40000000/0x60000000 actions=drop ``` - Flow 1 is to match packets from allowed CIDR `192.168.77.0/24`, marking them with `LoadBalancerSourceRangesAllowRegMark`. - Flow 2 is similar to flow 1 but for CIDR `192.168.78.0/24`. - Flow 3 is to match packets not from allowed CIDRs, marking with `LoadBalancerSourceRangesDropRegMark`. - Flow 4 is to match allowed packets with `LoadBalancerSourceRangesAllowRegMark` and perform load balancing. - Flow 5 is to match not allowed packets with `LoadBalancerSourceRangesDropRegMark` and drop. Signed-off-by: Hongliang Liu <hongliang.liu@broadcom.com>

hongliangl added area/proxy Issues or PRs related to proxy functions in Antrea action/release-note Indicates a PR that should be included in release notes. labels Apr 2, 2024

hongliangl requested review from tnqn and wenyingd April 7, 2024 03:14

hongliangl force-pushed the 20240402-support-lb-source-range branch 2 times, most recently from c1e1141 to 779e504 Compare April 29, 2024 02:33

hongliangl requested a review from antoninbas April 29, 2024 04:38

github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jul 29, 2024

tnqn removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jul 29, 2024

hongliangl force-pushed the 20240402-support-lb-source-range branch from 779e504 to 7387fd3 Compare September 3, 2024 07:20

hongliangl added this to the Antrea v2.2 release milestone Sep 3, 2024

hongliangl force-pushed the 20240402-support-lb-source-range branch from 7387fd3 to 78ec531 Compare September 3, 2024 07:43

antoninbas reviewed Sep 4, 2024

View reviewed changes

hongliangl force-pushed the 20240402-support-lb-source-range branch 4 times, most recently from e8dd1ba to edba464 Compare September 13, 2024 10:59

hongliangl requested a review from antoninbas September 13, 2024 11:08

hongliangl force-pushed the 20240402-support-lb-source-range branch from edba464 to f54ac3a Compare September 25, 2024 10:25

wenyingd reviewed Sep 27, 2024

View reviewed changes

hongliangl force-pushed the 20240402-support-lb-source-range branch 2 times, most recently from 647e567 to f4f330a Compare September 30, 2024 07:30

hongliangl force-pushed the 20240402-support-lb-source-range branch 2 times, most recently from 4547344 to c76dd8e Compare September 30, 2024 08:42

hongliangl force-pushed the 20240402-support-lb-source-range branch from c76dd8e to 695df4b Compare October 8, 2024 09:58

hongliangl mentioned this pull request Oct 16, 2024

loadBalancerSourceRanges not supported in AntreaProxy #5493

Open

luolanzone modified the milestones: Antrea v2.2 release, Antrea v2.3 release Oct 23, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support Service loadBalancerSourceRanges in AntreaProxy #6181

Support Service loadBalancerSourceRanges in AntreaProxy #6181

hongliangl commented Apr 2, 2024 •

edited

Loading

github-actions bot commented Jul 29, 2024

hongliangl commented Sep 3, 2024

antoninbas left a comment

antoninbas Sep 4, 2024

hongliangl Sep 13, 2024

antoninbas Sep 4, 2024

hongliangl Sep 13, 2024

hongliangl commented Sep 13, 2024

wenyingd Sep 27, 2024

hongliangl Sep 30, 2024

wenyingd Sep 27, 2024

hongliangl Sep 30, 2024

wenyingd Sep 27, 2024

hongliangl Sep 30, 2024

wenyingd Sep 27, 2024

hongliangl Sep 30, 2024 •

edited

Loading

antrea-bot commented Sep 30, 2024

		Service `loadBalancerSourceRanges`. It's worth noting that `loadBalancerSourceRanges` only exclusively influences the
		traffic from the external network as `LoadBalancerSourceRangesRegMark` is not used in flow 10.

Support Service loadBalancerSourceRanges in AntreaProxy #6181

Are you sure you want to change the base?

Support Service loadBalancerSourceRanges in AntreaProxy #6181

Conversation

hongliangl commented Apr 2, 2024 • edited Loading

github-actions bot commented Jul 29, 2024

hongliangl commented Sep 3, 2024

antoninbas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hongliangl commented Sep 13, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hongliangl Sep 30, 2024 • edited Loading

Choose a reason for hiding this comment

antrea-bot commented Sep 30, 2024

hongliangl commented Apr 2, 2024 •

edited

Loading

hongliangl Sep 30, 2024 •

edited

Loading