| Message ID | 1595408344-6121-1-git-send-email-pkusunyifeng@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ovs-dev,v2] bfd: Support overlay BFD | expand |
On Wed, Jul 22, 2020 at 01:59:04AM -0700, Yifeng Sun 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 adds new test to validate BFD overlay. > > This patch keeps BFD's backward compatibility. > is there a VMware Bug id? > Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> > --- > v1->v2: Add test by William's suggestion. > > lib/bfd.c | 16 +++++++++++++--- > tests/system-traffic.at | 42 ++++++++++++++++++++++++++++++++++++++++++ > vswitchd/vswitch.xml | 7 +++++++ > 3 files changed, 62 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..80b58996d530 100644 > --- a/tests/system-traffic.at > +++ b/tests/system-traffic.at > @@ -6289,3 +6289,45 @@ 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]) > + > +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' Is this only happened to this bfd test, or others also fail? > + > +dnl Firstly, test normal BFD packet. > +ovs-ofctl -O OpenFlow13 packet-out br-underlay "in_port=1 packet=ee09e04dbf314e125d6c743d080045c0006624d94000401153f9ac10b469ac10b46a739f17c1005247f900806558000000000023200000016a0f5a9ed376080045c0003400000000ff11fa03ac10b469ac10b46ac0070ec80020000021c003187b3c96ebbc96b962000186a0000f424000000000 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. > +AT_FAIL_IF([cat datapath-flows.txt | egrep "tunnel\(.*,src=172.16.180.105,dst=172.16.180.106,.*,eth\(\),eth_type\(0x0800\),ipv4\(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 The rest looks good to me. William
On Wed, Jul 22, 2020 at 01:59:04AM -0700, Yifeng Sun 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 adds new test to validate BFD overlay. > > This patch keeps BFD's backward compatibility. > > Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> > --- > v1->v2: Add test by William's suggestion. > > lib/bfd.c | 16 +++++++++++++--- > tests/system-traffic.at | 42 ++++++++++++++++++++++++++++++++++++++++++ > vswitchd/vswitch.xml | 7 +++++++ > 3 files changed, 62 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..80b58996d530 100644 > --- a/tests/system-traffic.at > +++ b/tests/system-traffic.at > @@ -6289,3 +6289,45 @@ 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]) > + > +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=ee09e04dbf314e125d6c743d080045c0006624d94000401153f9ac10b469ac10b46a739f17c1005247f900806558000000000023200000016a0f5a9ed376080045c0003400000000ff11fa03ac10b469ac10b46ac0070ec80020000021c003187b3c96ebbc96b962000186a0000f424000000000 actions=NORMAL" The packet above is dnl outer IP: Source: 172.16.180.105 Destination: 172.16.180.106 dnl inner IP: Source: 172.16.180.105 Destination: 172.16.180.106 And why does this trigger ovs to process it ex: bfd_should_process_flow() return true? In your patch, you're adding extra check bfd->ip_src == flow->nw_dst and here bfd->ip_src is default 169.254.1.1 flow->nw_dst is 172.16.180.106 > +dnl Next we test overlay BFD packet. > +ovs-ofctl -O OpenFlow13 packet-out br-underlay "in_port=1 packet=ee09e04dbf314e125d6c743d08004500005b2558400040115445ac10b469ac10b46a6b1017c1004722c1024065580000000001048001001803005254009d0b6d5254000c89840800450000210e22400040119688c0a80a68c0a80a6995cd0ec8000dd342746573740a actions=NORMAL" The packet above is dnl outer IP: Source: 172.16.180.105 Destination: 172.16.180.106 dnl inner IP: Source: 192.168.10.104 Destination: 192.168.10.105 So the bfd_should_process_flow returns false. > + > +ovs-dpctl dump-flows > datapath-flows.txt > +dnl BFD packet was handled by BFD flow. > +AT_FAIL_IF([cat datapath-flows.txt | egrep "tunnel\(.*,src=172.16.180.105,dst=172.16.180.106,.*,eth\(\),eth_type\(0x0800\),ipv4\(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
Thanks for reviewing.
For these two packets:
dnl outer IP: Source: 172.16.180.105 Destination: 172.16.180.106
This one is normal BFD packet, bfd_should_process_flow should return
true, as used to.
dnl inner IP: Source: 192.168.10.104 Destination: 192.168.10.105
This one is overlay BFD packet, bfd_should_process_flow should return
false so this packet won't be intercepted by OVS's BFD engine.
Thanks,
Yifeng
On Wed, Jul 22, 2020 at 10:54 AM William Tu <u9012063@gmail.com> wrote:
> On Wed, Jul 22, 2020 at 01:59:04AM -0700, Yifeng Sun 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 adds new test to validate BFD overlay.
> >
> > This patch keeps BFD's backward compatibility.
> >
> > Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com>
> > ---
> > v1->v2: Add test by William's suggestion.
> >
> > lib/bfd.c | 16 +++++++++++++---
> > tests/system-traffic.at | 42 ++++++++++++++++++++++++++++++++++++++++++
> > vswitchd/vswitch.xml | 7 +++++++
> > 3 files changed, 62 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..80b58996d530 100644
> > --- a/tests/system-traffic.at
> > +++ b/tests/system-traffic.at
> > @@ -6289,3 +6289,45 @@ 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])
> > +
> > +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=ee09e04dbf314e125d6c743d080045c0006624d94000401153f9ac10b469ac10b46a739f17c1005247f900806558000000000023200000016a0f5a9ed376080045c0003400000000ff11fa03ac10b469ac10b46ac0070ec80020000021c003187b3c96ebbc96b962000186a0000f424000000000
> actions=NORMAL"
> The packet above is
> dnl outer IP: Source: 172.16.180.105 Destination: 172.16.180.106
> dnl inner IP: Source: 172.16.180.105 Destination: 172.16.180.106
>
> And why does this trigger ovs to process it ex: bfd_should_process_flow()
> return true?
> In your patch, you're adding extra check
> bfd->ip_src == flow->nw_dst
> and here
> bfd->ip_src is default 169.254.1.1
> flow->nw_dst is 172.16.180.106
>
>
> > +dnl Next we test overlay BFD packet.
> > +ovs-ofctl -O OpenFlow13 packet-out br-underlay "in_port=1
> packet=ee09e04dbf314e125d6c743d08004500005b2558400040115445ac10b469ac10b46a6b1017c1004722c1024065580000000001048001001803005254009d0b6d5254000c89840800450000210e22400040119688c0a80a68c0a80a6995cd0ec8000dd342746573740a
> actions=NORMAL"
>
> The packet above is
> dnl outer IP: Source: 172.16.180.105 Destination: 172.16.180.106
> dnl inner IP: Source: 192.168.10.104 Destination: 192.168.10.105
> So the bfd_should_process_flow returns false.
>
> > +
> > +ovs-dpctl dump-flows > datapath-flows.txt
> > +dnl BFD packet was handled by BFD flow.
> > +AT_FAIL_IF([cat datapath-flows.txt | egrep
> "tunnel\(.*,src=172.16.180.105,dst=172.16.180.106,.*,eth\(\),eth_type\(0x0800\),ipv4\(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
>
>
Please discard my previous email, I misunderstood your question. The packet above is dnl outer IP: Source: 172.16.180.105 Destination: 172.16.180.106 dnl inner IP: Source: 192.168.10.104 Destination: 192.168.10.105 So the bfd_should_process_flow returns false. Yes, you are correct and this patch returns false in this case. For the above packet, outer IP is extracted as tunnel info, flow->nw_dst is 192.168.10.105. So bfd_should_process_flow returns false. Thanks, Yifeng On Wed, Jul 22, 2020 at 11:02 AM Yifeng Sun <pkusunyifeng@gmail.com> wrote: > Thanks for reviewing. > > For these two packets: > > dnl outer IP: Source: 172.16.180.105 Destination: 172.16.180.106 > This one is normal BFD packet, bfd_should_process_flow should return > true, as used to. > > dnl inner IP: Source: 192.168.10.104 Destination: 192.168.10.105 > This one is overlay BFD packet, bfd_should_process_flow should return > false so this packet won't be intercepted by OVS's BFD engine. > > Thanks, > Yifeng > > On Wed, Jul 22, 2020 at 10:54 AM William Tu <u9012063@gmail.com> wrote: > >> On Wed, Jul 22, 2020 at 01:59:04AM -0700, Yifeng Sun 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 adds new test to validate BFD overlay. >> > >> > This patch keeps BFD's backward compatibility. >> > >> > Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> >> > --- >> > v1->v2: Add test by William's suggestion. >> > >> > lib/bfd.c | 16 +++++++++++++--- >> > tests/system-traffic.at | 42 >> ++++++++++++++++++++++++++++++++++++++++++ >> > vswitchd/vswitch.xml | 7 +++++++ >> > 3 files changed, 62 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..80b58996d530 100644 >> > --- a/tests/system-traffic.at >> > +++ b/tests/system-traffic.at >> > @@ -6289,3 +6289,45 @@ 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]) >> > + >> > +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=ee09e04dbf314e125d6c743d080045c0006624d94000401153f9ac10b469ac10b46a739f17c1005247f900806558000000000023200000016a0f5a9ed376080045c0003400000000ff11fa03ac10b469ac10b46ac0070ec80020000021c003187b3c96ebbc96b962000186a0000f424000000000 >> actions=NORMAL" >> The packet above is >> dnl outer IP: Source: 172.16.180.105 Destination: 172.16.180.106 >> dnl inner IP: Source: 172.16.180.105 Destination: 172.16.180.106 >> >> And why does this trigger ovs to process it ex: bfd_should_process_flow() >> return true? >> In your patch, you're adding extra check >> bfd->ip_src == flow->nw_dst >> and here >> bfd->ip_src is default 169.254.1.1 >> flow->nw_dst is 172.16.180.106 >> >> >> > +dnl Next we test overlay BFD packet. >> > +ovs-ofctl -O OpenFlow13 packet-out br-underlay "in_port=1 >> packet=ee09e04dbf314e125d6c743d08004500005b2558400040115445ac10b469ac10b46a6b1017c1004722c1024065580000000001048001001803005254009d0b6d5254000c89840800450000210e22400040119688c0a80a68c0a80a6995cd0ec8000dd342746573740a >> actions=NORMAL" >> >> The packet above is >> dnl outer IP: Source: 172.16.180.105 Destination: 172.16.180.106 >> dnl inner IP: Source: 192.168.10.104 Destination: 192.168.10.105 >> So the bfd_should_process_flow returns false. >> >> > + >> > +ovs-dpctl dump-flows > datapath-flows.txt >> > +dnl BFD packet was handled by BFD flow. >> > +AT_FAIL_IF([cat datapath-flows.txt | egrep >> "tunnel\(.*,src=172.16.180.105,dst=172.16.180.106,.*,eth\(\),eth_type\(0x0800\),ipv4\(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 >> >>
On Wed, Jul 22, 2020 at 11:02:32AM -0700, Yifeng Sun wrote: > Thanks for reviewing. > > For these two packets: > > dnl outer IP: Source: 172.16.180.105 Destination: 172.16.180.106 > This one is normal BFD packet, bfd_should_process_flow should return > true, as used to. > > dnl inner IP: Source: 192.168.10.104 Destination: 192.168.10.105 > This one is overlay BFD packet, bfd_should_process_flow should return > false so this packet won't be intercepted by OVS's BFD engine. So you add an additional condition here: (bfd->ip_src == htonl(BFD_DEFAULT_SRC_IP) || bfd->ip_src == flow->nw_dst)) How come the first packt is true, and the second packet is false in the above condition? you didn't set bfd_src_ip in the test, so what's the value of bfd->ip_src? another question below > > > --- a/tests/system-traffic.at > > > +++ b/tests/system-traffic.at > > > @@ -6289,3 +6289,45 @@ 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]) > > > + > > > +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=ee09e04dbf314e125d6c743d080045c0006624d94000401153f9ac10b469ac10b46a739f17c1005247f900806558000000000023200000016a0f5a9ed376080045c0003400000000ff11fa03ac10b469ac10b46ac0070ec80020000021c003187b3c96ebbc96b962000186a0000f424000000000 > > actions=NORMAL" > > The packet above is > > dnl outer IP: Source: 172.16.180.105 Destination: 172.16.180.106 > > dnl inner IP: Source: 172.16.180.105 Destination: 172.16.180.106 > > > > And why does this trigger ovs to process it ex: bfd_should_process_flow() > > return true? > > In your patch, you're adding extra check > > bfd->ip_src == flow->nw_dst > > and here > > bfd->ip_src is default 169.254.1.1 > > flow->nw_dst is 172.16.180.106 > > > > > > > +dnl Next we test overlay BFD packet. > > > +ovs-ofctl -O OpenFlow13 packet-out br-underlay "in_port=1 > > packet=ee09e04dbf314e125d6c743d08004500005b2558400040115445ac10b469ac10b46a6b1017c1004722c1024065580000000001048001001803005254009d0b6d5254000c89840800450000210e22400040119688c0a80a68c0a80a6995cd0ec8000dd342746573740a > > actions=NORMAL" the 2nd packet is NORMAL > > > > The packet above is > > dnl outer IP: Source: 172.16.180.105 Destination: 172.16.180.106 > > dnl inner IP: Source: 192.168.10.104 Destination: 192.168.10.105 > > So the bfd_should_process_flow returns false. > > > > > + > > > +ovs-dpctl dump-flows > datapath-flows.txt > > > +dnl BFD packet was handled by BFD flow. > > > +AT_FAIL_IF([cat datapath-flows.txt | egrep > > "tunnel\(.*,src=172.16.180.105,dst=172.16.180.106,.*,eth\(\),eth_type\(0x0800\),ipv4\(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]) But why it is drop here? > > > + > > > +OVS_TRAFFIC_VSWITCHD_STOP > > > +AT_CLEANUP > > > >
You are correct, I will fix BFD config in v3. For the overlay BFD packet, we don't set up a port to handle packets targeted at 192.168.10.105. So ovs simply drops them. On Wed, Jul 22, 2020 at 11:26 AM William Tu <u9012063@gmail.com> wrote: > On Wed, Jul 22, 2020 at 11:02:32AM -0700, Yifeng Sun wrote: > > Thanks for reviewing. > > > > For these two packets: > > > > dnl outer IP: Source: 172.16.180.105 Destination: 172.16.180.106 > > This one is normal BFD packet, bfd_should_process_flow should return > > true, as used to. > > > > dnl inner IP: Source: 192.168.10.104 Destination: 192.168.10.105 > > This one is overlay BFD packet, bfd_should_process_flow should return > > false so this packet won't be intercepted by OVS's BFD engine. > > So you add an additional condition here: > (bfd->ip_src == htonl(BFD_DEFAULT_SRC_IP) > || bfd->ip_src == flow->nw_dst)) > > How come the first packt is true, and the second packet is false in > the above condition? > > you didn't set bfd_src_ip in the test, so what's the value of bfd->ip_src? > > another question below > > > > > --- a/tests/system-traffic.at > > > > +++ b/tests/system-traffic.at > > > > @@ -6289,3 +6289,45 @@ 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]) > > > > + > > > > +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=ee09e04dbf314e125d6c743d080045c0006624d94000401153f9ac10b469ac10b46a739f17c1005247f900806558000000000023200000016a0f5a9ed376080045c0003400000000ff11fa03ac10b469ac10b46ac0070ec80020000021c003187b3c96ebbc96b962000186a0000f424000000000 > > > actions=NORMAL" > > > The packet above is > > > dnl outer IP: Source: 172.16.180.105 Destination: 172.16.180.106 > > > dnl inner IP: Source: 172.16.180.105 Destination: 172.16.180.106 > > > > > > And why does this trigger ovs to process it ex: > bfd_should_process_flow() > > > return true? > > > In your patch, you're adding extra check > > > bfd->ip_src == flow->nw_dst > > > and here > > > bfd->ip_src is default 169.254.1.1 > > > flow->nw_dst is 172.16.180.106 > > > > > > > > > > +dnl Next we test overlay BFD packet. > > > > +ovs-ofctl -O OpenFlow13 packet-out br-underlay "in_port=1 > > > > packet=ee09e04dbf314e125d6c743d08004500005b2558400040115445ac10b469ac10b46a6b1017c1004722c1024065580000000001048001001803005254009d0b6d5254000c89840800450000210e22400040119688c0a80a68c0a80a6995cd0ec8000dd342746573740a > > > actions=NORMAL" > > the 2nd packet is NORMAL > > > > > > The packet above is > > > dnl outer IP: Source: 172.16.180.105 Destination: 172.16.180.106 > > > dnl inner IP: Source: 192.168.10.104 Destination: 192.168.10.105 > > > So the bfd_should_process_flow returns false. > > > > > > > + > > > > +ovs-dpctl dump-flows > datapath-flows.txt > > > > +dnl BFD packet was handled by BFD flow. > > > > +AT_FAIL_IF([cat datapath-flows.txt | egrep > > > > "tunnel\(.*,src=172.16.180.105,dst=172.16.180.106,.*,eth\(\),eth_type\(0x0800\),ipv4\(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]) > > But why it is drop here? > > > > > + > > > > +OVS_TRAFFIC_VSWITCHD_STOP > > > > +AT_CLEANUP > > > > > > >
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..80b58996d530 100644 --- a/tests/system-traffic.at +++ b/tests/system-traffic.at @@ -6289,3 +6289,45 @@ 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]) + +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=ee09e04dbf314e125d6c743d080045c0006624d94000401153f9ac10b469ac10b46a739f17c1005247f900806558000000000023200000016a0f5a9ed376080045c0003400000000ff11fa03ac10b469ac10b46ac0070ec80020000021c003187b3c96ebbc96b962000186a0000f424000000000 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. +AT_FAIL_IF([cat datapath-flows.txt | egrep "tunnel\(.*,src=172.16.180.105,dst=172.16.180.106,.*,eth\(\),eth_type\(0x0800\),ipv4\(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 adds new test to validate BFD overlay. This patch keeps BFD's backward compatibility. Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com> --- v1->v2: Add test by William's suggestion. lib/bfd.c | 16 +++++++++++++--- tests/system-traffic.at | 42 ++++++++++++++++++++++++++++++++++++++++++ vswitchd/vswitch.xml | 7 +++++++ 3 files changed, 62 insertions(+), 3 deletions(-)