| Message ID | 1595450449-5861-1-git-send-email-pkusunyifeng@gmail.com |
|---|---|
| State | Accepted |
| Commit | b793a65c1667c028d94d91e3cd5e36ca26e38d8b |
| Headers | show |
| Series | [ovs-dev,v4] bfd: Support overlay BFD | expand |
On Wed, Jul 22, 2020 at 1:41 PM Yifeng Sun <pkusunyifeng@gmail.com> wrote: > > Current OVS intercepts and processes all BFD packets, thus VM-2-VM > BFD packets get lost and the recipient VM never sees them. > > This patch fixes it by only intercepting and processing BFD packets > destined to a configured BFD instance, and other BFD packets are made > available to the OVS flow table for forwarding. > > This patch keeps BFD's backward compatibility. > > VMWare-BZ: 2579326 s/VMWare/VMware s/2579326/#2579326/ > Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> > --- > v1->v2: Add test by William's suggestion. > v2->v3: Fix BFD config, thanks William. > v3->v4: Test will fail at second run, fixed it. > > lib/bfd.c | 16 +++++++++++++--- > tests/system-traffic.at | 44 ++++++++++++++++++++++++++++++++++++++++++++ > vswitchd/vswitch.xml | 7 +++++++ > 3 files changed, 64 insertions(+), 3 deletions(-) > > diff --git a/lib/bfd.c b/lib/bfd.c > index cc8c6857afa4..3c965699ace3 100644 > --- a/lib/bfd.c > +++ b/lib/bfd.c > @@ -149,6 +149,9 @@ BUILD_ASSERT_DECL(BFD_PACKET_LEN == sizeof(struct msg)); > #define FLAGS_MASK 0x3f > #define DEFAULT_MULT 3 > > +#define BFD_DEFAULT_SRC_IP 0xA9FE0101 /* 169.254.1.1 */ > +#define BFD_DEFAULT_DST_IP 0xA9FE0100 /* 169.254.1.0 */ > + > struct bfd { > struct hmap_node node; /* In 'all_bfds'. */ > uint32_t disc; /* bfd.LocalDiscr. Key in 'all_bfds' hmap. */ > @@ -457,9 +460,9 @@ bfd_configure(struct bfd *bfd, const char *name, const struct smap *cfg, > &bfd->rmt_eth_dst); > > bfd_lookup_ip(smap_get_def(cfg, "bfd_src_ip", ""), > - htonl(0xA9FE0101) /* 169.254.1.1 */, &bfd->ip_src); > + htonl(BFD_DEFAULT_SRC_IP), &bfd->ip_src); > bfd_lookup_ip(smap_get_def(cfg, "bfd_dst_ip", ""), > - htonl(0xA9FE0100) /* 169.254.1.0 */, &bfd->ip_dst); > + htonl(BFD_DEFAULT_DST_IP), &bfd->ip_dst); > > forwarding_if_rx = smap_get_bool(cfg, "forwarding_if_rx", false); > if (bfd->forwarding_if_rx != forwarding_if_rx) { > @@ -674,7 +677,14 @@ bfd_should_process_flow(const struct bfd *bfd_, const struct flow *flow, > memset(&wc->masks.nw_proto, 0xff, sizeof wc->masks.nw_proto); > if (flow->nw_proto == IPPROTO_UDP > && !(flow->nw_frag & FLOW_NW_FRAG_LATER) > - && tp_dst_equals(flow, BFD_DEST_PORT, wc)) { > + && tp_dst_equals(flow, BFD_DEST_PORT, wc) > + && (bfd->ip_src == htonl(BFD_DEFAULT_SRC_IP) > + || bfd->ip_src == flow->nw_dst)) { > + > + if (bfd->ip_src == flow->nw_dst) { > + memset(&wc->masks.nw_dst, 0xffffffff, sizeof wc->masks.nw_dst); > + } > + > bool check_tnl_key; > > atomic_read_relaxed(&bfd->check_tnl_key, &check_tnl_key); > diff --git a/tests/system-traffic.at b/tests/system-traffic.at > index 2a0fbadff4a1..ea72f155782f 100644 > --- a/tests/system-traffic.at > +++ b/tests/system-traffic.at > @@ -6289,3 +6289,47 @@ OVS_WAIT_UNTIL([cat p2.pcap | egrep "0x0050: *0000 *0000 *5002 *2000 *b85e *0000 > > OVS_TRAFFIC_VSWITCHD_STOP > AT_CLEANUP > + > +AT_SETUP([bfd - BFD overlay]) > +OVS_CHECK_GENEVE() > + > +OVS_TRAFFIC_VSWITCHD_START() > + > +AT_CHECK([ovs-vsctl -- set bridge br0 other-config:hwaddr=\"f2:ff:00:00:00:01\"]) > +ADD_BR([br-underlay], [set bridge br-underlay other-config:hwaddr=\"ee:09:e0:4d:bf:31\"]) > + > +AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"]) > +AT_CHECK([ovs-ofctl add-flow br-underlay "actions=normal"]) > + > +ADD_NAMESPACES(at_ns0) > + > +dnl Set up underlay link from host into the namespace using veth pair. > +ADD_VETH(p0, at_ns0, br-underlay, "172.16.180.105/24", 4e:12:5d:6c:74:3d) > +AT_CHECK([ip addr add dev br-underlay "172.16.180.106/24"]) > +AT_CHECK([ip link set dev br-underlay up]) > + > +dnl Set up tunnel endpoints on OVS outside the namespace. > +ADD_OVS_TUNNEL([geneve], [br0], [at_gnv0], [172.16.180.105], [192.168.10.100/24], > + [options:packet_type=ptap]) > +AT_CHECK([ovs-vsctl -- set Interface at_gnv0 ofport_request=1]) > +AT_CHECK([ovs-vsctl -- set Interface at_gnv0 bfd:enable=true bfd:bfd_src_ip="172.16.180.106"]) > + > +dnl Certain Linux distributions, like CentOS, have default iptable rules > +dnl to reject input traffic from br-underlay. Here we add a rule to walk > +dnl around it. > +iptables -I INPUT 1 -i br-underlay -j ACCEPT > +on_exit 'iptables -D INPUT 1' > + > +dnl Firstly, test normal BFD packet. > +ovs-ofctl -O OpenFlow13 packet-out br-underlay "in_port=1 packet=ee09e04dbf314e125d6c743d080045c00066ea4e400040118e83ac10b469ac10b46a356e17c10052cb5500806558000000000023200000018ad232b919c4080045c0003400000000ff11fa03ac10b469ac10b46ac0000ec80020000020800318035cd00e00000000000f4240000f424000000000 actions=NORMAL" > +dnl Next we test overlay BFD packet. > +ovs-ofctl -O OpenFlow13 packet-out br-underlay "in_port=1 packet=ee09e04dbf314e125d6c743d08004500005b2558400040115445ac10b469ac10b46a6b1017c1004722c1024065580000000001048001001803005254009d0b6d5254000c89840800450000210e22400040119688c0a80a68c0a80a6995cd0ec8000dd342746573740a actions=NORMAL" > + > +ovs-dpctl dump-flows > datapath-flows.txt > +dnl BFD packet was handled by BFD flow. Ipv4.dst is matched. > +AT_FAIL_IF([cat datapath-flows.txt | egrep "tunnel\(.*,src=172.16.180.105,dst=172.16.180.106,.*,eth\(\),eth_type\(0x0800\),ipv4\(dst=172.16.180.106,proto=17,.*\),udp\(dst=3784\), .*, actions:userspace\(pid=.*,slow_path\(bfd\)\)" 2>&1 1>/dev/null]) please run $ make check-kmod TESTSUITEFLAGS='-d -k BFD' the test pass but there is no bfd packet in the datapath-flows.txt $ grep bfd tests/system-kmod-testsuite.dir/131/datapath-flows.txt William
Confirmed that the setup is quite unstable. Sometimes bfd flow shows up in datapath-flows.txt but sometimes not. Let me take a look. Thanks, Yifeng On Thu, Jul 23, 2020 at 6:51 AM William Tu <u9012063@gmail.com> wrote: > On Wed, Jul 22, 2020 at 1:41 PM Yifeng Sun <pkusunyifeng@gmail.com> wrote: > > > > Current OVS intercepts and processes all BFD packets, thus VM-2-VM > > BFD packets get lost and the recipient VM never sees them. > > > > This patch fixes it by only intercepting and processing BFD packets > > destined to a configured BFD instance, and other BFD packets are made > > available to the OVS flow table for forwarding. > > > > This patch keeps BFD's backward compatibility. > > > > VMWare-BZ: 2579326 > s/VMWare/VMware > s/2579326/#2579326/ > > > Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> > > --- > > v1->v2: Add test by William's suggestion. > > v2->v3: Fix BFD config, thanks William. > > v3->v4: Test will fail at second run, fixed it. > > > > lib/bfd.c | 16 +++++++++++++--- > > tests/system-traffic.at | 44 > ++++++++++++++++++++++++++++++++++++++++++++ > > vswitchd/vswitch.xml | 7 +++++++ > > 3 files changed, 64 insertions(+), 3 deletions(-) > > > > diff --git a/lib/bfd.c b/lib/bfd.c > > index cc8c6857afa4..3c965699ace3 100644 > > --- a/lib/bfd.c > > +++ b/lib/bfd.c > > @@ -149,6 +149,9 @@ BUILD_ASSERT_DECL(BFD_PACKET_LEN == sizeof(struct > msg)); > > #define FLAGS_MASK 0x3f > > #define DEFAULT_MULT 3 > > > > +#define BFD_DEFAULT_SRC_IP 0xA9FE0101 /* 169.254.1.1 */ > > +#define BFD_DEFAULT_DST_IP 0xA9FE0100 /* 169.254.1.0 */ > > + > > struct bfd { > > struct hmap_node node; /* In 'all_bfds'. */ > > uint32_t disc; /* bfd.LocalDiscr. Key in 'all_bfds' > hmap. */ > > @@ -457,9 +460,9 @@ bfd_configure(struct bfd *bfd, const char *name, > const struct smap *cfg, > > &bfd->rmt_eth_dst); > > > > bfd_lookup_ip(smap_get_def(cfg, "bfd_src_ip", ""), > > - htonl(0xA9FE0101) /* 169.254.1.1 */, &bfd->ip_src); > > + htonl(BFD_DEFAULT_SRC_IP), &bfd->ip_src); > > bfd_lookup_ip(smap_get_def(cfg, "bfd_dst_ip", ""), > > - htonl(0xA9FE0100) /* 169.254.1.0 */, &bfd->ip_dst); > > + htonl(BFD_DEFAULT_DST_IP), &bfd->ip_dst); > > > > forwarding_if_rx = smap_get_bool(cfg, "forwarding_if_rx", false); > > if (bfd->forwarding_if_rx != forwarding_if_rx) { > > @@ -674,7 +677,14 @@ bfd_should_process_flow(const struct bfd *bfd_, > const struct flow *flow, > > memset(&wc->masks.nw_proto, 0xff, sizeof wc->masks.nw_proto); > > if (flow->nw_proto == IPPROTO_UDP > > && !(flow->nw_frag & FLOW_NW_FRAG_LATER) > > - && tp_dst_equals(flow, BFD_DEST_PORT, wc)) { > > + && tp_dst_equals(flow, BFD_DEST_PORT, wc) > > + && (bfd->ip_src == htonl(BFD_DEFAULT_SRC_IP) > > + || bfd->ip_src == flow->nw_dst)) { > > + > > + if (bfd->ip_src == flow->nw_dst) { > > + memset(&wc->masks.nw_dst, 0xffffffff, sizeof > wc->masks.nw_dst); > > + } > > + > > bool check_tnl_key; > > > > atomic_read_relaxed(&bfd->check_tnl_key, &check_tnl_key); > > diff --git a/tests/system-traffic.at b/tests/system-traffic.at > > index 2a0fbadff4a1..ea72f155782f 100644 > > --- a/tests/system-traffic.at > > +++ b/tests/system-traffic.at > > @@ -6289,3 +6289,47 @@ OVS_WAIT_UNTIL([cat p2.pcap | egrep "0x0050: > *0000 *0000 *5002 *2000 *b85e *0000 > > > > OVS_TRAFFIC_VSWITCHD_STOP > > AT_CLEANUP > > + > > +AT_SETUP([bfd - BFD overlay]) > > +OVS_CHECK_GENEVE() > > + > > +OVS_TRAFFIC_VSWITCHD_START() > > + > > +AT_CHECK([ovs-vsctl -- set bridge br0 > other-config:hwaddr=\"f2:ff:00:00:00:01\"]) > > +ADD_BR([br-underlay], [set bridge br-underlay > other-config:hwaddr=\"ee:09:e0:4d:bf:31\"]) > > + > > +AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"]) > > +AT_CHECK([ovs-ofctl add-flow br-underlay "actions=normal"]) > > + > > +ADD_NAMESPACES(at_ns0) > > + > > +dnl Set up underlay link from host into the namespace using veth pair. > > +ADD_VETH(p0, at_ns0, br-underlay, "172.16.180.105/24", > 4e:12:5d:6c:74:3d) > > +AT_CHECK([ip addr add dev br-underlay "172.16.180.106/24"]) > > +AT_CHECK([ip link set dev br-underlay up]) > > + > > +dnl Set up tunnel endpoints on OVS outside the namespace. > > +ADD_OVS_TUNNEL([geneve], [br0], [at_gnv0], [172.16.180.105], [ > 192.168.10.100/24], > > + [options:packet_type=ptap]) > > +AT_CHECK([ovs-vsctl -- set Interface at_gnv0 ofport_request=1]) > > +AT_CHECK([ovs-vsctl -- set Interface at_gnv0 bfd:enable=true > bfd:bfd_src_ip="172.16.180.106"]) > > + > > +dnl Certain Linux distributions, like CentOS, have default iptable rules > > +dnl to reject input traffic from br-underlay. Here we add a rule to walk > > +dnl around it. > > +iptables -I INPUT 1 -i br-underlay -j ACCEPT > > +on_exit 'iptables -D INPUT 1' > > + > > +dnl Firstly, test normal BFD packet. > > +ovs-ofctl -O OpenFlow13 packet-out br-underlay "in_port=1 > packet=ee09e04dbf314e125d6c743d080045c00066ea4e400040118e83ac10b469ac10b46a356e17c10052cb5500806558000000000023200000018ad232b919c4080045c0003400000000ff11fa03ac10b469ac10b46ac0000ec80020000020800318035cd00e00000000000f4240000f424000000000 > actions=NORMAL" > > +dnl Next we test overlay BFD packet. > > +ovs-ofctl -O OpenFlow13 packet-out br-underlay "in_port=1 > packet=ee09e04dbf314e125d6c743d08004500005b2558400040115445ac10b469ac10b46a6b1017c1004722c1024065580000000001048001001803005254009d0b6d5254000c89840800450000210e22400040119688c0a80a68c0a80a6995cd0ec8000dd342746573740a > actions=NORMAL" > > + > > +ovs-dpctl dump-flows > datapath-flows.txt > > +dnl BFD packet was handled by BFD flow. Ipv4.dst is matched. > > +AT_FAIL_IF([cat datapath-flows.txt | egrep > "tunnel\(.*,src=172.16.180.105,dst=172.16.180.106,.*,eth\(\),eth_type\(0x0800\),ipv4\(dst=172.16.180.106,proto=17,.*\),udp\(dst=3784\), > .*, actions:userspace\(pid=.*,slow_path\(bfd\)\)" 2>&1 1>/dev/null]) > > please run > $ make check-kmod TESTSUITEFLAGS='-d -k BFD' > the test pass but there is no bfd packet in the datapath-flows.txt > $ grep bfd tests/system-kmod-testsuite.dir/131/datapath-flows.txt > > William >
diff --git a/lib/bfd.c b/lib/bfd.c index cc8c6857afa4..3c965699ace3 100644 --- a/lib/bfd.c +++ b/lib/bfd.c @@ -149,6 +149,9 @@ BUILD_ASSERT_DECL(BFD_PACKET_LEN == sizeof(struct msg)); #define FLAGS_MASK 0x3f #define DEFAULT_MULT 3 +#define BFD_DEFAULT_SRC_IP 0xA9FE0101 /* 169.254.1.1 */ +#define BFD_DEFAULT_DST_IP 0xA9FE0100 /* 169.254.1.0 */ + struct bfd { struct hmap_node node; /* In 'all_bfds'. */ uint32_t disc; /* bfd.LocalDiscr. Key in 'all_bfds' hmap. */ @@ -457,9 +460,9 @@ bfd_configure(struct bfd *bfd, const char *name, const struct smap *cfg, &bfd->rmt_eth_dst); bfd_lookup_ip(smap_get_def(cfg, "bfd_src_ip", ""), - htonl(0xA9FE0101) /* 169.254.1.1 */, &bfd->ip_src); + htonl(BFD_DEFAULT_SRC_IP), &bfd->ip_src); bfd_lookup_ip(smap_get_def(cfg, "bfd_dst_ip", ""), - htonl(0xA9FE0100) /* 169.254.1.0 */, &bfd->ip_dst); + htonl(BFD_DEFAULT_DST_IP), &bfd->ip_dst); forwarding_if_rx = smap_get_bool(cfg, "forwarding_if_rx", false); if (bfd->forwarding_if_rx != forwarding_if_rx) { @@ -674,7 +677,14 @@ bfd_should_process_flow(const struct bfd *bfd_, const struct flow *flow, memset(&wc->masks.nw_proto, 0xff, sizeof wc->masks.nw_proto); if (flow->nw_proto == IPPROTO_UDP && !(flow->nw_frag & FLOW_NW_FRAG_LATER) - && tp_dst_equals(flow, BFD_DEST_PORT, wc)) { + && tp_dst_equals(flow, BFD_DEST_PORT, wc) + && (bfd->ip_src == htonl(BFD_DEFAULT_SRC_IP) + || bfd->ip_src == flow->nw_dst)) { + + if (bfd->ip_src == flow->nw_dst) { + memset(&wc->masks.nw_dst, 0xffffffff, sizeof wc->masks.nw_dst); + } + bool check_tnl_key; atomic_read_relaxed(&bfd->check_tnl_key, &check_tnl_key); diff --git a/tests/system-traffic.at b/tests/system-traffic.at index 2a0fbadff4a1..ea72f155782f 100644 --- a/tests/system-traffic.at +++ b/tests/system-traffic.at @@ -6289,3 +6289,47 @@ OVS_WAIT_UNTIL([cat p2.pcap | egrep "0x0050: *0000 *0000 *5002 *2000 *b85e *0000 OVS_TRAFFIC_VSWITCHD_STOP AT_CLEANUP + +AT_SETUP([bfd - BFD overlay]) +OVS_CHECK_GENEVE() + +OVS_TRAFFIC_VSWITCHD_START() + +AT_CHECK([ovs-vsctl -- set bridge br0 other-config:hwaddr=\"f2:ff:00:00:00:01\"]) +ADD_BR([br-underlay], [set bridge br-underlay other-config:hwaddr=\"ee:09:e0:4d:bf:31\"]) + +AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"]) +AT_CHECK([ovs-ofctl add-flow br-underlay "actions=normal"]) + +ADD_NAMESPACES(at_ns0) + +dnl Set up underlay link from host into the namespace using veth pair. +ADD_VETH(p0, at_ns0, br-underlay, "172.16.180.105/24", 4e:12:5d:6c:74:3d) +AT_CHECK([ip addr add dev br-underlay "172.16.180.106/24"]) +AT_CHECK([ip link set dev br-underlay up]) + +dnl Set up tunnel endpoints on OVS outside the namespace. +ADD_OVS_TUNNEL([geneve], [br0], [at_gnv0], [172.16.180.105], [192.168.10.100/24], + [options:packet_type=ptap]) +AT_CHECK([ovs-vsctl -- set Interface at_gnv0 ofport_request=1]) +AT_CHECK([ovs-vsctl -- set Interface at_gnv0 bfd:enable=true bfd:bfd_src_ip="172.16.180.106"]) + +dnl Certain Linux distributions, like CentOS, have default iptable rules +dnl to reject input traffic from br-underlay. Here we add a rule to walk +dnl around it. +iptables -I INPUT 1 -i br-underlay -j ACCEPT +on_exit 'iptables -D INPUT 1' + +dnl Firstly, test normal BFD packet. +ovs-ofctl -O OpenFlow13 packet-out br-underlay "in_port=1 packet=ee09e04dbf314e125d6c743d080045c00066ea4e400040118e83ac10b469ac10b46a356e17c10052cb5500806558000000000023200000018ad232b919c4080045c0003400000000ff11fa03ac10b469ac10b46ac0000ec80020000020800318035cd00e00000000000f4240000f424000000000 actions=NORMAL" +dnl Next we test overlay BFD packet. +ovs-ofctl -O OpenFlow13 packet-out br-underlay "in_port=1 packet=ee09e04dbf314e125d6c743d08004500005b2558400040115445ac10b469ac10b46a6b1017c1004722c1024065580000000001048001001803005254009d0b6d5254000c89840800450000210e22400040119688c0a80a68c0a80a6995cd0ec8000dd342746573740a actions=NORMAL" + +ovs-dpctl dump-flows > datapath-flows.txt +dnl BFD packet was handled by BFD flow. Ipv4.dst is matched. +AT_FAIL_IF([cat datapath-flows.txt | egrep "tunnel\(.*,src=172.16.180.105,dst=172.16.180.106,.*,eth\(\),eth_type\(0x0800\),ipv4\(dst=172.16.180.106,proto=17,.*\),udp\(dst=3784\), .*, actions:userspace\(pid=.*,slow_path\(bfd\)\)" 2>&1 1>/dev/null]) +dnl Overlay BFD packet was handled by non-BFD flows. +AT_FAIL_IF([cat datapath-flows.txt | egrep "tunnel\(.*,src=172.16.180.105,dst=172.16.180.106,.*,eth\(.*\),eth_type\(0x0800\),ipv4\(src=192.168.10.104,dst=192.168.10.105,proto=17,.*\),udp\(.*,dst=3784\), .*, actions:drop" 2>&1 1>/dev/null]) + +OVS_TRAFFIC_VSWITCHD_STOP +AT_CLEANUP diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml index b6acb34ca1b8..e1f895134ff2 100644 --- a/vswitchd/vswitch.xml +++ b/vswitchd/vswitch.xml @@ -3662,6 +3662,13 @@ ovs-vsctl add-port br0 p0 -- set Interface p0 type=patch options:peer=p1 \ optional Authentication or ``Echo Mode'' features. </p> + <p> + OVS 2.13 and earlier intercepted and processed all BFD packets. + OVS 2.14 and later only intercept and process BFD packets destined + to a configured BFD instance, and other BFD packets are made available + to the OVS flow table for forwarding. + </p> + <group title="BFD Configuration"> <p> A controller sets up key-value pairs in the <ref column="bfd"/>
Current OVS intercepts and processes all BFD packets, thus VM-2-VM BFD packets get lost and the recipient VM never sees them. This patch fixes it by only intercepting and processing BFD packets destined to a configured BFD instance, and other BFD packets are made available to the OVS flow table for forwarding. This patch keeps BFD's backward compatibility. VMWare-BZ: 2579326 Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> --- v1->v2: Add test by William's suggestion. v2->v3: Fix BFD config, thanks William. v3->v4: Test will fail at second run, fixed it. lib/bfd.c | 16 +++++++++++++--- tests/system-traffic.at | 44 ++++++++++++++++++++++++++++++++++++++++++++ vswitchd/vswitch.xml | 7 +++++++ 3 files changed, 64 insertions(+), 3 deletions(-)