Re: [ovs-dev] [PATCH v2] bfd: Support overlay BFD
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 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: > > > * * *5002 *2000 *b85e * > > > > > > > > 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=ee09e04dbf314e125d6c743d080045c0006624d94000401153f9ac10b469ac10b46a739f17c1005247f900806558002320016a0f5a9ed376080045c00034ff11fa03ac10b469ac10b46ac0070ec8002021c003187b3c96ebbc96b962000186af4240 > > > 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=ee09e04dbf314e125d6c743d0800455b2558400040115445ac10b469ac10b46a6b1017c1004722c10240655801048001001803005254009d0b6d5254000c8984080045210e22400040119688c0a80a68c0a80a6995cd0ec8000dd342746573740a > > > 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 > > > > > > > ___
Re: [ovs-dev] [PATCH v2] bfd: Support overlay BFD
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: > > * * *5002 *2000 *b85e * > > > > > > 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=ee09e04dbf314e125d6c743d080045c0006624d94000401153f9ac10b469ac10b46a739f17c1005247f900806558002320016a0f5a9ed376080045c00034ff11fa03ac10b469ac10b46ac0070ec8002021c003187b3c96ebbc96b962000186af4240 > > 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=ee09e04dbf314e125d6c743d0800455b2558400040115445ac10b469ac10b46a6b1017c1004722c10240655801048001001803005254009d0b6d5254000c8984080045210e22400040119688c0a80a68c0a80a6995cd0ec8000dd342746573740a > > 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 > > > > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2] bfd: Support overlay BFD
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 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 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
>> > ---
>> > 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,
>> > >rmt_eth_dst);
>> >
>> > bfd_lookup_ip(smap_get_def(cfg, "bfd_src_ip", ""),
>> > - htonl(0xA9FE0101) /* 169.254.1.1 */, >ip_src);
>> > + htonl(BFD_DEFAULT_SRC_IP), >ip_src);
>> > bfd_lookup_ip(smap_get_def(cfg, "bfd_dst_ip", ""),
>> > - htonl(0xA9FE0100) /* 169.254.1.0 */, >ip_dst);
>> > + htonl(BFD_DEFAULT_DST_IP), >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(>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(>masks.nw_dst, 0x, sizeof
>> wc->masks.nw_dst);
>> > +}
>> > +
>> > bool check_tnl_key;
>> >
>> > atomic_read_relaxed(>check_tnl_key, _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:
>> * * *5002 *2000 *b85e *
>> >
>> > 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
Re: [ovs-dev] [PATCH v2] bfd: Support overlay BFD
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 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
> > ---
> > 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,
> > >rmt_eth_dst);
> >
> > bfd_lookup_ip(smap_get_def(cfg, "bfd_src_ip", ""),
> > - htonl(0xA9FE0101) /* 169.254.1.1 */, >ip_src);
> > + htonl(BFD_DEFAULT_SRC_IP), >ip_src);
> > bfd_lookup_ip(smap_get_def(cfg, "bfd_dst_ip", ""),
> > - htonl(0xA9FE0100) /* 169.254.1.0 */, >ip_dst);
> > + htonl(BFD_DEFAULT_DST_IP), >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(>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(>masks.nw_dst, 0x, sizeof
> wc->masks.nw_dst);
> > +}
> > +
> > bool check_tnl_key;
> >
> > atomic_read_relaxed(>check_tnl_key, _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:
> * * *5002 *2000 *b85e *
> >
> > 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
>
Re: [ovs-dev] [PATCH v2] bfd: Support overlay BFD
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
> ---
> 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,
> >rmt_eth_dst);
>
> bfd_lookup_ip(smap_get_def(cfg, "bfd_src_ip", ""),
> - htonl(0xA9FE0101) /* 169.254.1.1 */, >ip_src);
> + htonl(BFD_DEFAULT_SRC_IP), >ip_src);
> bfd_lookup_ip(smap_get_def(cfg, "bfd_dst_ip", ""),
> - htonl(0xA9FE0100) /* 169.254.1.0 */, >ip_dst);
> + htonl(BFD_DEFAULT_DST_IP), >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(>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(>masks.nw_dst, 0x, sizeof
> wc->masks.nw_dst);
> +}
> +
> bool check_tnl_key;
>
> atomic_read_relaxed(>check_tnl_key, _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: *
> * *5002 *2000 *b85e *
>
> 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=ee09e04dbf314e125d6c743d080045c0006624d94000401153f9ac10b469ac10b46a739f17c1005247f900806558002320016a0f5a9ed376080045c00034ff11fa03ac10b469ac10b46ac0070ec8002021c003187b3c96ebbc96b962000186af4240
> 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
Re: [ovs-dev] [PATCH v2] bfd: Support overlay BFD
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
> ---
> 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,
> >rmt_eth_dst);
>
> bfd_lookup_ip(smap_get_def(cfg, "bfd_src_ip", ""),
> - htonl(0xA9FE0101) /* 169.254.1.1 */, >ip_src);
> + htonl(BFD_DEFAULT_SRC_IP), >ip_src);
> bfd_lookup_ip(smap_get_def(cfg, "bfd_dst_ip", ""),
> - htonl(0xA9FE0100) /* 169.254.1.0 */, >ip_dst);
> + htonl(BFD_DEFAULT_DST_IP), >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(>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(>masks.nw_dst, 0x, sizeof
> wc->masks.nw_dst);
> +}
> +
> bool check_tnl_key;
>
> atomic_read_relaxed(>check_tnl_key, _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: *
> * *5002 *2000 *b85e *
>
> 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=ee09e04dbf314e125d6c743d080045c0006624d94000401153f9ac10b469ac10b46a739f17c1005247f900806558002320016a0f5a9ed376080045c00034ff11fa03ac10b469ac10b46ac0070ec8002021c003187b3c96ebbc96b962000186af4240
> actions=NORMAL"
> +dnl Next we test overlay BFD packet.
> +ovs-ofctl -O OpenFlow13 packet-out br-underlay "in_port=1
> packet=ee09e04dbf314e125d6c743d0800455b2558400040115445ac10b469ac10b46a6b1017c1004722c10240655801048001001803005254009d0b6d5254000c8984080045210e22400040119688c0a80a68c0a80a6995cd0ec8000dd342746573740a
>
