kmesh-net / kmesh

High Performance ServiceMesh Data Plane Based on Programmable Kernel
https://kmesh.net
Apache License 2.0
362 stars 46 forks source link

unified handing logic for skops->remote_port/local_port #395

Closed nlgwcy closed 1 month ago

nlgwcy commented 1 month ago

What type of PR is this?

/kind enhancement

What this PR does / why we need it: Current Situation:

bpf_sock_addr:user_port is stored in u16 network order. bpf_sock_ops:local_port is stored in u32 host order. bpf_sock_ops:remote_port oe23.03 is stored in u16 network order, while non-oe23.03 is stored in u32 network order. Currently, the port conversion is scattered throughout the code and not handled consistently.

Proposed Solution:

Treat all ports as u16. Maintain the byte order as defined originally. Provide dedicated macros to retrieve the remote_port and local_port of skops.

Which issue(s) this PR fixes: Fixes https://github.com/kmesh-net/kmesh/issues/374

Special notes for your reviewer:

self test:

  1. fortio benchmark
    
    [root@localhost images]# kubectl get svc
    NAME            TYPE        CLUSTER-IP     EXTERNAL-IP   PORT(S)   AGE
    fortio-server   ClusterIP   10.96.52.254   <none>        80/TCP    3m24s
    [root@localhost images]# kubectl get endpoints
    NAME            ENDPOINTS         AGE
    fortio-server   10.244.1.8:9090   4m51s

[root@localhost images]# kubectl exec -ti fortio-client-deployment-6c86f967bf-p89pm -- fortio load -c 1 -t 30s http://10.96.52.254:80

dnat to fortio-server pod

[root@localhost ~]# nsenter -t 1321071 -n ss -antp State Recv-Q Send-Q Local Address:Port Peer Address:Port Process ESTAB 0 0 10.244.1.9:39254 10.244.1.8:9090 users:(("fortio",pid=1321071,fd=3))

2. fortio benchmark over waypoint
```sh
[root@localhost images]# kubectl get pods -owide
NAME                                        READY   STATUS    RESTARTS   AGE     IP            NODE             NOMINATED NODE   READINESS GATES
default-istio-waypoint-854d5c77bb-djm8l     1/1     Running   0          9s      10.244.1.11   ambient-worker   <none>           <none>

# dnat to waypoint
[root@localhost ~]# nsenter -t 1324267 -n ss -antp
State                Recv-Q                Send-Q                               Local Address:Port                                Peer Address:Port                Process
ESTAB                0                     0                                       10.244.1.9:42308                                10.244.1.11:15019                users:(("fortio",pid=1324267,fd=3))
  1. authorization allow
    [root@localhost kmesh]# kubectl get pods -owide
    NAME                                        READY   STATUS    RESTARTS   AGE    IP            NODE             NOMINATED NODE   READINESS GATES
    default-istio-waypoint-854d5c77bb-djm8l     1/1     Running   0          119m   10.244.1.11   ambient-worker   <none>           <none>
    fortio-client-deployment-6c86f967bf-p89pm   1/1     Running   0          127m   10.244.1.9    ambient-worker   <none>           <none>
    fortio-server-deployment-7fcf96565c-sn7sf   1/1     Running   0          127m   10.244.1.8    ambient-worker   <none>           <none>
    httpbin-f85dc8498-4mqc9                     1/1     Running   0          11m    10.244.1.19   ambient-worker   <none>           <none>
    sleep-78ff5975c6-5svwl                      1/1     Running   0          112m   10.244.1.14   ambient-worker   <none>           <none>
    sleep-78ff5975c6-xfgh6                      1/1     Running   0          112m   10.244.1.15   ambient-worker   <none>           <none>
    [root@localhost kmesh]# kubectl apply -f - <<EOF
    > apiVersion: security.istio.io/v1beta1
    > kind: AuthorizationPolicy
    > metadata:
    >  name: allow-by-srcip
    >  namespace: default
    > spec:
    >  selector:
    >    matchLabels:
    >      app: httpbin
    >  action: ALLOW
    >  rules:
    >  - from:
    >    - source:
    >        ipBlocks:
    >        - 10.244.1.14/32
    > EOF
    authorizationpolicy.security.istio.io/allow-by-srcip created
    [root@localhost kmesh]# kubectl exec -ti sleep-78ff5975c6-xfgh6 -- curl http://10.96.23.33:8000/headers
    ^Ccommand terminated with exit code 130
    [root@localhost kmesh]#  kubectl exec -ti sleep-78ff5975c6-5svwl -- curl http://10.96.23.33:8000/headers
    {
    "headers": {
    "Accept": "*/*",
    "Host": "10.96.23.33:8000",
    "User-Agent": "curl/8.8.0"
    }
    }
    [root@localhost kmesh]# kubectl exec -ti sleep-78ff5975c6-xfgh6 -- curl http://10.96.23.33:8000/headers
    curl: (56) Recv failure: Connection reset by peer
    command terminated with exit code 56
    [root@localhost kmesh]#
  2. authorization deny

[root@localhost kmesh]# kubectl apply -f - <<EOF

apiVersion: security.istio.io/v1beta1 kind: AuthorizationPolicy metadata: name: deny-by-srcip namespace: default spec: selector: matchLabels: app: httpbin action: DENY rules:

  • from:
    • source: ipBlocks:
      • 10.244.1.15/32 EOF authorizationpolicy.security.istio.io/deny-by-srcip created [root@localhost kmesh]# kubectl exec -ti sleep-78ff5975c6-5svwl -- curl http://10.96.23.33:8000/headers { "headers": { "Accept": "/", "Host": "10.96.23.33:8000", "User-Agent": "curl/8.8.0" } } [root@localhost kmesh]# kubectl exec -ti sleep-78ff5975c6-xfgh6 -- curl http://10.96.23.33:8000/headers curl: (56) Recv failure: Connection reset by peer command terminated with exit code 56 [root@localhost kmesh]#
        
        **Does this PR introduce a user-facing change?**:
        <!--
        If no, just write "NONE" in the release-note block below.
        If yes, a release note is required.
        -->
        ```release-note
codecov-commenter commented 1 month ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Flag Coverage Δ
unittests 31.15% <ø> (?)

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

nlgwcy commented 1 month ago

/reopen

kmesh-bot commented 1 month ago

@nlgwcy: Reopened this PR.

In response to [this](https://github.com/kmesh-net/kmesh/pull/395#issuecomment-2138710177): >/reopen Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
hzxuzhonghu commented 1 month ago

/lgtm /approve

kmesh-bot commented 1 month ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hzxuzhonghu

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/kmesh-net/kmesh/blob/main/OWNERS)~~ [hzxuzhonghu] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment