spiderpool icon indicating copy to clipboard operation
spiderpool copied to clipboard

coordinator: add a from policy route for pod's eth0

Open cyclinder opened this issue 1 year ago • 4 comments

Add a from policy route for pod's eth0, which make sure that packets received from eth0 are forwarded out of eth0. fix to the problem of inconsistent routes.

Thanks for contributing!

What type of PR is this?

  • release/bug

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #3683 Special notes for your reviewer:

root@demo1-7fbdd6cc66-bwgtn:/# ip r
default via 169.254.1.1 dev eth0
10.7.0.0/16 dev net1 proto kernel scope link src 10.7.168.202
10.7.168.71 dev eth0 scope link src 10.233.74.111
10.233.0.0/18 via 10.7.168.71 dev eth0 src 10.233.74.111
10.233.64.0/18 via 10.7.168.71 dev eth0 src 10.233.74.111
10.233.74.64 dev eth0 scope link src 10.233.74.111
169.254.0.0/16 via 10.7.168.71 dev eth0 src 10.233.74.111
169.254.1.1 dev eth0 scope link
172.224.168.71 dev eth0 scope link src 10.233.74.111
root@demo1-7fbdd6cc66-bwgtn:/# ip rule
0:	from all lookup local
32764:	from 10.233.74.111 lookup 500.    # ===>> new added
32765:	from 10.7.168.202 lookup 100
32766:	from all lookup main
32767:	from all lookup default
root@demo1-7fbdd6cc66-bwgtn:/# ip r show table 500
default via 169.254.1.1 dev eth0 # ===>> new added

cyclinder avatar Jul 02 '24 07:07 cyclinder

Codecov Report

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

Project coverage is 81.16%. Comparing base (0cdac43) to head (4b166d9). Report is 4 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #3686   +/-   ##
=======================================
  Coverage   81.16%   81.16%           
=======================================
  Files          50       50           
  Lines        4391     4391           
=======================================
  Hits         3564     3564           
  Misses        670      670           
  Partials      157      157           
Flag Coverage Δ
unittests 81.16% <ø> (ø)

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

codecov[bot] avatar Jul 02 '24 07:07 codecov[bot]

PR 描述中,应该给出详细的 路由变更例子,以方便 PR review 和维护

weizhoublue avatar Jul 02 '24 09:07 weizhoublue

这个代码变更 比较多,需要详细 评审下 用例的覆盖 情况 ,避免 按下葫芦浮起瓢

weizhoublue avatar Jul 02 '24 09:07 weizhoublue

看起来代码变动多,但其实只加了一个东西。经过评估,目前e2e 已经覆盖大部分case,之前没测出问题,是因为环境配置问题。再加一个指定默认网卡后的联通性case 就可以了

cyclinder avatar Jul 02 '24 10:07 cyclinder

pr 的 title 最好 主要体现 修复了什么现象或问题,不能说是 做了什么技术修改 。 否则 未来 在 release note 中 没法 review,使用者也没法知道这个pr 对他有什么帮助

weizhoublue avatar Jul 08 '24 02:07 weizhoublue

have it checked when the eth0 is from macvlan

weizhoublue avatar Jul 08 '24 02:07 weizhoublue

does it need any document modification ?

weizhoublue avatar Jul 08 '24 03:07 weizhoublue

have it checked when the eth0 is from macvlan

these changes doesn't related to the case that when the eth0 is from macvlan, only the eth0 is from calico.

cyclinder avatar Jul 08 '24 05:07 cyclinder

have it checked when the eth0 is from macvlan

these changes doesn't related to the case that when the eth0 is from macvlan, only the eth0 is from calico.

does it really not correlate with mutiple macvlan interfaces ? maybe I do not think that way

weizhoublue avatar Jul 08 '24 06:07 weizhoublue

Yes, In the case of multiple macvlan-nics accessing the nodeport, packets will come in from veth0, and then we have some iptables rules and policy-based routing to ensure that reply packets are still sent from veth0. So it's not relevant to the content of this pr

cyclinder avatar Jul 08 '24 07:07 cyclinder

Yes, In the case of multiple macvlan-nics accessing the nodeport, packets will come in from veth0, and then we have some iptables rules and policy-based routing to ensure that reply packets are still sent from veth0. So it's not relevant to the content of this pr

actually, I mean, there is a pod owns eth0 172.16.2.11 and eth1 172.16.3 .11. when a remote host 172.16.3.10 visits the address 172.16.2.11 of the Pod, the request packet ingress in eth0 , and the reply packet egress in eth1. May that happen

weizhoublue avatar Jul 09 '24 05:07 weizhoublue

If we need to forward through the host stack then we need to make sure that the forwarding path is consistent, otherwise for multiple macvlan underlay interfaces this forwarding is works well, I did some testing and this is fine.

➜  ~ ip netns exec net1 ip a
1: lo: <LOOPBACK> mtu 65536 qdisc noop state DOWN group default qlen 1000
    link/loopback 00:00:00:00:00:00 brd 00:00:00:00:00:00
2: tunl0@NONE: <NOARP> mtu 1480 qdisc noop state DOWN group default qlen 1000
    link/ipip 0.0.0.0 brd 0.0.0.0
1999: macvlan0@if2: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP group default qlen 1000
    link/ether ce:3a:92:4f:d0:1b brd ff:ff:ff:ff:ff:ff link-netnsid 0
    inet 10.6.212.188/16 scope global macvlan0
       valid_lft forever preferred_lft forever
    inet6 fe80::cc3a:92ff:fe4f:d01b/64 scope link
       valid_lft forever preferred_lft forever
➜  ~ ip netns exec net1 ping 10.7.212.207 -c 2
PING 10.7.212.207 (10.7.212.207) 56(84) bytes of data.
64 bytes from 10.7.212.207: icmp_seq=1 ttl=64 time=0.465 ms
64 bytes from 10.7.212.207: icmp_seq=2 ttl=64 time=0.342 ms

--- 10.7.212.207 ping statistics ---
2 packets transmitted, 2 received, 0% packet loss, time 1000ms
rtt min/avg/max/mdev = 0.342/0.403/0.465/0.061 ms

cyclinder avatar Jul 10 '24 06:07 cyclinder

10.7.212.207

I does not mean the local node of pod, I mean a remote node. So, when a pod owns 2 macvlan interface, is there a policy rule for the source IP of eth1 ? like what does in calico case

weizhoublue avatar Jul 11 '24 12:07 weizhoublue

yes, I know this case, there are a remote client(10.6.212.188) and a macvlan pod with two macvlan interface(eth0: 10.7.212.201, eth1: 10.6.212.228), the client access to eth0(10.7.212.201), and the response is sent fron eth1(10.6.212.228), it works well.

cyclinder avatar Jul 11 '24 14:07 cyclinder