Re: [ovs-dev] [PATCH v4] bfd: Support overlay BFD
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 wrote:
> On Wed, Jul 22, 2020 at 1:41 PM 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 keeps BFD's backward compatibility.
> >
> > VMWare-BZ: 2579326
> s/VMWare/VMware
> s/2579326/#2579326/
>
> > Signed-off-by: Yifeng Sun
> > ---
> > 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,
> > >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..ea72f155782f 100644
> > --- a/tests/system-traffic.at
> > +++ b/tests/system-traffic.at
> > @@ -6289,3 +6289,47 @@ 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
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.1
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
>> > @@
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 Destinat
[ovs-dev] [PATCH v3] bfd: Support overlay 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
---
v1->v2: Add test by William's suggestion.
v2->v3: Fix BFD config, thanks William.
lib/bfd.c | 16 +---
tests/system-traffic.at | 43 +++
vswitchd/vswitch.xml| 7 +++
3 files changed, 63 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..5c1aee49aba2 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -6289,3 +6289,46 @@ 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])
+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=ee09e04dbf314e125d6c743d080045c0006624d94000401153f9ac10b469ac10b46a739f17c1005247f900806558002320016a0f5a9ed376080045c00034ff11fa03ac10b469ac10b46ac0070ec8002021c003187b3c96ebbc96b962000186af4240
actions=NORMAL"
+dnl Next we test overlay BFD packet.
+ovs-ofctl -O OpenFlow13 packet-out br-underlay "in_port=1
packet=ee09e04dbf314e125d6c743d0800455b2558400040115445ac10b469ac10b46a6b1017c1004722c10240655801048001001803005254009d0b6d5254000c8984080045210e22400040119688c0a80a68c0a80a6995cd0ec8000dd342746573740a
actions=NORMAL"
+
+ovs-dpctl dump-flows > da
[ovs-dev] [PATCH v4] bfd: Support overlay 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
---
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,
>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..ea72f155782f 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -6289,3 +6289,47 @@ 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])
+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=ee09e04dbf314e125d6c743d080045c00066ea4e400040118e83ac10b469ac10b46a356e17c10052cb5500806558002320018ad232b919c4080045c00034ff11fa03ac10b469ac10b46acec8002020800318035cd00e000f424f4240
actions=NORMAL"
+dnl Next we test overlay BFD packet.
+ovs-ofctl -O OpenFlow13 packet-out br-underlay "in_port=1
packet=ee09e04dbf314e125d6c743d0800455b2558400040115445ac10b469ac10b46a6b1017c1004722c10240655801048001001803005254009d0b6d525
Re: [ovs-dev] [PATCH V2] Update scripts to support RHEL 7.9
LGTM.
Reviewed-by: Yifeng Sun
On Tue, Nov 17, 2020 at 3:26 PM Greg Rose wrote:
> Add support for RHEL7.9 GA release with kernel 3.10.0-1160
>
> Signed-off-by: Greg Rose
>
> ---
> V2 - Correct the author
> ---
> rhel/openvswitch-kmod-fedora.spec.in | 6 --
> rhel/usr_share_openvswitch_scripts_ovs-kmod-manage.sh | 6 ++
> 2 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/rhel/openvswitch-kmod-fedora.spec.in b/rhel/
> openvswitch-kmod-fedora.spec.in
> index 15eec6d4c..ff190064f 100644
> --- a/rhel/openvswitch-kmod-fedora.spec.in
> +++ b/rhel/openvswitch-kmod-fedora.spec.in
> @@ -19,6 +19,7 @@
> # - 3.10.0 major revision 1062 (RHEL 7.7)
> # - 3.10.0 major revision 1101 (RHEL 7.8 Beta)
> # - 3.10.0 major revision 1127 (RHEL 7.8 GA)
> +# - 3.10.0 major revision 1160 (RHEL 7.9 GA)
> # By default, build against the current running kernel version
> #%define kernel 3.1.5-1.fc16.x86_64
> #define kernel %{kernel_source}
> @@ -98,8 +99,9 @@ if grep -qs "suse" /etc/os-release; then
> elif [ "$mainline_major" = "3" ] && [ "$mainline_minor" = "10" ] &&
> { [ "$major_rev" = "327" ] || [ "$major_rev" = "693" ] || \
> [ "$major_rev" = "957" ] || [ "$major_rev" == "1062" ] || \
> - [ "$major_rev" = "1101" ] || [ "$major_rev" = "1127" ] ; }; then
> -# For RHEL 7.2, 7.4, 7.6, 7.7, and 7.8
> + [ "$major_rev" = "1101" ] || [ "$major_rev" = "1127" ] || \
> + [ "$major_rev" = "1160" ] ; }; then
> +# For RHEL 7.2, 7.4, 7.6, 7.7, 7.8 and 7.9
> if [ -x "%{_datadir}/openvswitch/scripts/ovs-kmod-manage.sh" ]; then
> %{_datadir}/openvswitch/scripts/ovs-kmod-manage.sh
> fi
> diff --git a/rhel/usr_share_openvswitch_scripts_ovs-kmod-manage.sh
> b/rhel/usr_share_openvswitch_scripts_ovs-kmod-manage.sh
> index c70e135cd..9bf25a46b 100644
> --- a/rhel/usr_share_openvswitch_scripts_ovs-kmod-manage.sh
> +++ b/rhel/usr_share_openvswitch_scripts_ovs-kmod-manage.sh
> @@ -21,6 +21,7 @@
> # - 3.10.0 major revision 1062 (RHEL 7.7)
> # - 3.10.0 major revision 1101 (RHEL 7.8 Beta)
> # - 3.10.0 major revision 1127 (RHEL 7.8 GA)
> +# - 3.10.0 major revision 1160 (RHEL 7.9)
> # - 4.4.x, x >= 73 (SLES 12 SP3)
> # - 4.12.x, x >= 14 (SLES 12 SP4).
> # It is packaged in the openvswitch kmod RPM and run in the post-install
> @@ -118,6 +119,11 @@ if [ "$mainline_major" = "3" ] && [ "$mainline_minor"
> = "10" ]; then
> comp_ver=10
> ver_offset=4
> installed_ver="$minor_rev"
> +elif [ "$major_rev" = "1160" ]; then
> +#echo "rhel79"
> +comp_ver=10
> +ver_offset=4
> +installed_ver="$minor_rev"
> fi
> elif [ "$mainline_major" = "4" ] && [ "$mainline_minor" = "4" ]; then
> if [ "$mainline_patch" -ge "73" ]; then
> --
> 2.17.1
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] rhel: Add option to enable AF_XDP on rpm package
LGTM, thanks.
Tested-by: Yifeng Sun
Reviewed-by: Yifeng Sun
On Thu, Jan 21, 2021 at 10:57 AM Yi-Hung Wei wrote:
> This patch adds an RPMBUILD_OPT so that user can enable
> AF_XDP support in the rpm package by:
>
> $ make rpm-fedora RPMBUILD_OPT="--with afxdp"
>
> Signed-off-by: Yi-Hung Wei
> ---
> rhel/openvswitch-fedora.spec.in | 8
> 1 file changed, 8 insertions(+)
>
> diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/
> openvswitch-fedora.spec.in
> index 2c0c4fa186a3..e03b26b6af34 100644
> --- a/rhel/openvswitch-fedora.spec.in
> +++ b/rhel/openvswitch-fedora.spec.in
> @@ -28,6 +28,8 @@
> %bcond_without libcapng
> # To enable DPDK support, specify '--with dpdk' when building
> %bcond_with dpdk
> +# To enable AF_XDP support, specify '--with afxdp' when building
> +%bcond_with afxdp
>
> # If there is a need to automatically enable the package after
> installation,
> # specify the "--with autoenable"
> @@ -73,6 +75,9 @@ BuildRequires: libpcap-devel numactl-devel
> BuildRequires: dpdk-devel >= 17.05.1
> Provides: %{name}-dpdk = %{version}-%{release}
> %endif
> +%if %{with afxdp}
> +BuildRequires: libbpf-devel numactl-devel
> +%endif
> BuildRequires: unbound unbound-devel
>
> Requires: openssl hostname iproute module-init-tools unbound
> @@ -164,6 +169,9 @@ This package provides IPsec tunneling support for OVS
> tunnels.
> %if %{with dpdk}
> --with-dpdk=$(dirname %{_datadir}/dpdk/*/.config) \
> %endif
> +%if %{with afxdp}
> +--enable-afxdp \
> +%endif
> --enable-ssl \
> --disable-static \
> --enable-shared \
> --
> 2.7.4
>
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] connmgr: Check nullptr inside ofmonitor_report()
ovs-vswitchd could crash under these circumstances:
1. When one bridge is being destroyed, ofproto_destroy() is called and
connmgr pointer of its ofproto struct is nullified. This ofproto struct is
deallocated through 'ovsrcu_postpone(ofproto_destroy_defer__, p);'.
2. Before RCU enters quiesce state to actually free this ofproto struct,
revalidator thread calls udpif_revalidator(), which could handle
a learn flow and calls ofproto_flow_mod_learn(), it later calls
ofmonitor_report() and ofproto struct's connmgr pointer is accessed.
The crash stack trace is shown below:
0 ofmonitor_report (mgr=0x0, rule=rule@entry=0x7fa4ac067c30,
event=event@entry=NXFME_ADDED,
reason=reason@entry=OFPRR_IDLE_TIMEOUT, abbrev_ofconn=0x0, abbrev_xid=0,
old_actions=old_actions@entry=0x0)
at ofproto/connmgr.c:2160
1 0x7fa4d6803495 in add_flow_finish (ofproto=0x55d9075d4ab0,
ofm=, req=req@entry=0x0)
at ofproto/ofproto.c:5221
2 0x7fa4d68036af in modify_flows_finish (req=0x0, ofm=0x7fa4980753f0,
ofproto=0x55d9075d4ab0)
at ofproto/ofproto.c:5823
3 ofproto_flow_mod_finish (ofproto=0x55d9075d4ab0,
ofm=ofm@entry=0x7fa4980753f0, req=req@entry=0x0)
at ofproto/ofproto.c:8088
4 0x7fa4d680372d in ofproto_flow_mod_learn_finish
(ofm=ofm@entry=0x7fa4980753f0,
orig_ofproto=orig_ofproto@entry=0x0) at ofproto/ofproto.c:5439
5 0x7fa4d68072f9 in ofproto_flow_mod_learn (ofm=0x7fa4980753f0,
keep_ref=keep_ref@entry=true,
limit=, below_limitp=below_limitp@entry=0x0) at
ofproto/ofproto.c:5499
6 0x7fa4d6835d33 in xlate_push_stats_entry (entry=0x7fa498012448,
stats=stats@entry=0x7fa4d2701a10,
offloaded=offloaded@entry=false) at ofproto/ofproto-dpif-xlate-cache.c:127
7 0x7fa4d6835e3a in xlate_push_stats (xcache=,
stats=stats@entry=0x7fa4d2701a10,
offloaded=offloaded@entry=false) at ofproto/ofproto-dpif-xlate-cache.c:181
8 0x7fa4d6822046 in revalidate_ukey (udpif=udpif@entry=0x55d90760b240,
ukey=ukey@entry=0x7fa4b0191660,
stats=stats@entry=0x7fa4d2705118,
odp_actions=odp_actions@entry=0x7fa4d2701b50,
reval_seq=reval_seq@entry=5655486242, recircs=recircs@entry=0x7fa4d2701b40,
offloaded=false)
at ofproto/ofproto-dpif-upcall.c:2294
9 0x7fa4d6825aee in revalidate (revalidator=0x55d90769dd00) at
ofproto/ofproto-dpif-upcall.c:2683
10 0x7fa4d6825cf3 in udpif_revalidator (arg=0x55d90769dd00) at
ofproto/ofproto-dpif-upcall.c:936
11 0x7fa4d6259c9f in ovsthread_wrapper (aux_=) at
lib/ovs-thread.c:423
12 0x7fa4d582cea5 in start_thread () from /usr/lib64/libpthread.so.0
13 0x7fa4d504b96d in clone () from /usr/lib64/libc.so.6
At the time of crash, the involved ofproto was already deallocated:
(gdb) print *ofproto
$1 = ..., name = 0x55d907602820 "nsx-managed", ..., ports = {...,
one = 0x0, mask = 63, n = 0}, ..., connmgr = 0x0, ...
This patch fixes it.
VMware-BZ: #2700626
Signed-off-by: Yifeng Sun
---
ofproto/connmgr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c
index 9c5c633b4171..ee07df7a8bc0 100644
--- a/ofproto/connmgr.c
+++ b/ofproto/connmgr.c
@@ -2140,7 +2140,7 @@ ofmonitor_report(struct connmgr *mgr, struct rule *rule,
const struct rule_actions *old_actions)
OVS_REQUIRES(ofproto_mutex)
{
-if (rule_is_hidden(rule)) {
+if (!mgr || rule_is_hidden(rule)) {
return;
}
--
2.7.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] connmgr: Check nullptr inside ofmonitor_report()
Thanks William and YiHung for review, I sent a new version.
On Tue, Feb 16, 2021 at 8:45 PM William Tu wrote:
> On Tue, Feb 16, 2021 at 1:40 PM Yi-Hung Wei wrote:
> >
> > On Tue, Feb 16, 2021 at 1:06 PM Yifeng Sun
> wrote:
> > >
> > > ovs-vswitchd could crash under these circumstances:
> > > 1. When one bridge is being destroyed, ofproto_destroy() is called and
> > > connmgr pointer of its ofproto struct is nullified. This ofproto
> struct is
> > > deallocated through 'ovsrcu_postpone(ofproto_destroy_defer__, p);'.
> > > 2. Before RCU enters quiesce state to actually free this ofproto
> struct,
> > > revalidator thread calls udpif_revalidator(), which could handle
> > > a learn flow and calls ofproto_flow_mod_learn(), it later calls
> > > ofmonitor_report() and ofproto struct's connmgr pointer is accessed.
> > >
>
> LGTM, thanks. I guess this is hard to reproduce or create a test.
> Do we need to worry about other places that use 'ofproto->connmgr'?
> ex: there are a couple of places calling ofmonitor_flush(ofproto->connmgr);
>
> > > The crash stack trace is shown below:
> > >
> > > 0 ofmonitor_report (mgr=0x0, rule=rule@entry=0x7fa4ac067c30,
> event=event@entry=NXFME_ADDED,
> > > reason=reason@entry=OFPRR_IDLE_TIMEOUT, abbrev_ofconn=0x0,
> abbrev_xid=0, old_actions=old_actions@entry=0x0)
> > > at ofproto/connmgr.c:2160
> > > 1 0x7fa4d6803495 in add_flow_finish (ofproto=0x55d9075d4ab0,
> ofm=, req=req@entry=0x0)
> > > at ofproto/ofproto.c:5221
> > > 2 0x7fa4d68036af in modify_flows_finish (req=0x0,
> ofm=0x7fa4980753f0, ofproto=0x55d9075d4ab0)
> > > at ofproto/ofproto.c:5823
> > > 3 ofproto_flow_mod_finish (ofproto=0x55d9075d4ab0,
> > > ofm=ofm@entry=0x7fa4980753f0,
> req=req@entry=0x0)
> > > at ofproto/ofproto.c:8088
> > > 4 0x7fa4d680372d in ofproto_flow_mod_learn_finish (ofm=ofm@entry
> =0x7fa4980753f0,
> > > orig_ofproto=orig_ofproto@entry=0x0) at ofproto/ofproto.c:5439
> > > 5 0x7fa4d68072f9 in ofproto_flow_mod_learn (ofm=0x7fa4980753f0,
> keep_ref=keep_ref@entry=true,
> > > limit=, below_limitp=below_limitp@entry=0x0) at
> ofproto/ofproto.c:5499
> > > 6 0x7fa4d6835d33 in xlate_push_stats_entry (entry=0x7fa498012448,
> stats=stats@entry=0x7fa4d2701a10,
> > > offloaded=offloaded@entry=false) at
> ofproto/ofproto-dpif-xlate-cache.c:127
> > > 7 0x7fa4d6835e3a in xlate_push_stats (xcache=,
> stats=stats@entry=0x7fa4d2701a10,
> > > offloaded=offloaded@entry=false) at
> ofproto/ofproto-dpif-xlate-cache.c:181
> > > 8 0x7fa4d6822046 in revalidate_ukey
> > > (udpif=udpif@entry=0x55d90760b240,
> ukey=ukey@entry=0x7fa4b0191660,
> > > stats=stats@entry=0x7fa4d2705118, odp_actions=odp_actions@entry
> =0x7fa4d2701b50,
> > > reval_seq=reval_seq@entry=5655486242,
> > > recircs=recircs@entry=0x7fa4d2701b40,
> offloaded=false)
> > > at ofproto/ofproto-dpif-upcall.c:2294
> > > 9 0x7fa4d6825aee in revalidate (revalidator=0x55d90769dd00) at
> ofproto/ofproto-dpif-upcall.c:2683
> > > 10 0x7fa4d6825cf3 in udpif_revalidator (arg=0x55d90769dd00) at
> ofproto/ofproto-dpif-upcall.c:936
> > > 11 0x7fa4d6259c9f in ovsthread_wrapper (aux_=) at
> lib/ovs-thread.c:423
> > > 12 0x7fa4d582cea5 in start_thread () from
> /usr/lib64/libpthread.so.0
> > > 13 0x7fa4d504b96d in clone () from /usr/lib64/libc.so.6
> > >
> > > At the time of crash, the involved ofproto was already deallocated:
> > >
> > > (gdb) print *ofproto
> > > $1 = ..., name = 0x55d907602820 "nsx-managed", ..., ports = {...,
> > > one = 0x0, mask = 63, n = 0}, ..., connmgr = 0x0, ...
> > >
> > > This patch fixes it.
> > >
> > > VMware-BZ: #2700626
> > > Signed-off-by: Yifeng Sun
> > > ---
> >
> > LGTM.
> >
> > Acked-by: Yi-Hung Wei
> > ___
> > dev mailing list
> > d...@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCHv2] connmgr: Check nullptr inside ofmonitor_report()
ovs-vswitchd could crash under these circumstances:
1. When one bridge is being destroyed, ofproto_destroy() is called and
connmgr pointer of its ofproto struct is nullified. This ofproto struct is
deallocated through 'ovsrcu_postpone(ofproto_destroy_defer__, p);'.
2. Before RCU enters quiesce state to actually free this ofproto struct,
revalidator thread calls udpif_revalidator(), which could handle
a learn flow and calls ofproto_flow_mod_learn(), it later calls
ofmonitor_report() and ofproto struct's connmgr pointer is accessed.
The crash stack trace is shown below:
0 ofmonitor_report (mgr=0x0, rule=rule@entry=0x7fa4ac067c30,
event=event@entry=NXFME_ADDED,
reason=reason@entry=OFPRR_IDLE_TIMEOUT, abbrev_ofconn=0x0, abbrev_xid=0,
old_actions=old_actions@entry=0x0)
at ofproto/connmgr.c:2160
1 0x7fa4d6803495 in add_flow_finish (ofproto=0x55d9075d4ab0,
ofm=, req=req@entry=0x0)
at ofproto/ofproto.c:5221
2 0x7fa4d68036af in modify_flows_finish (req=0x0, ofm=0x7fa4980753f0,
ofproto=0x55d9075d4ab0)
at ofproto/ofproto.c:5823
3 ofproto_flow_mod_finish (ofproto=0x55d9075d4ab0,
ofm=ofm@entry=0x7fa4980753f0, req=req@entry=0x0)
at ofproto/ofproto.c:8088
4 0x7fa4d680372d in ofproto_flow_mod_learn_finish
(ofm=ofm@entry=0x7fa4980753f0,
orig_ofproto=orig_ofproto@entry=0x0) at ofproto/ofproto.c:5439
5 0x7fa4d68072f9 in ofproto_flow_mod_learn (ofm=0x7fa4980753f0,
keep_ref=keep_ref@entry=true,
limit=, below_limitp=below_limitp@entry=0x0) at
ofproto/ofproto.c:5499
6 0x7fa4d6835d33 in xlate_push_stats_entry (entry=0x7fa498012448,
stats=stats@entry=0x7fa4d2701a10,
offloaded=offloaded@entry=false) at ofproto/ofproto-dpif-xlate-cache.c:127
7 0x7fa4d6835e3a in xlate_push_stats (xcache=,
stats=stats@entry=0x7fa4d2701a10,
offloaded=offloaded@entry=false) at ofproto/ofproto-dpif-xlate-cache.c:181
8 0x7fa4d6822046 in revalidate_ukey (udpif=udpif@entry=0x55d90760b240,
ukey=ukey@entry=0x7fa4b0191660,
stats=stats@entry=0x7fa4d2705118,
odp_actions=odp_actions@entry=0x7fa4d2701b50,
reval_seq=reval_seq@entry=5655486242, recircs=recircs@entry=0x7fa4d2701b40,
offloaded=false)
at ofproto/ofproto-dpif-upcall.c:2294
9 0x7fa4d6825aee in revalidate (revalidator=0x55d90769dd00) at
ofproto/ofproto-dpif-upcall.c:2683
10 0x7fa4d6825cf3 in udpif_revalidator (arg=0x55d90769dd00) at
ofproto/ofproto-dpif-upcall.c:936
11 0x7fa4d6259c9f in ovsthread_wrapper (aux_=) at
lib/ovs-thread.c:423
12 0x7fa4d582cea5 in start_thread () from /usr/lib64/libpthread.so.0
13 0x7fa4d504b96d in clone () from /usr/lib64/libc.so.6
At the time of crash, the involved ofproto was already deallocated:
(gdb) print *ofproto
$1 = ..., name = 0x55d907602820 "nsx-managed", ..., ports = {...,
one = 0x0, mask = 63, n = 0}, ..., connmgr = 0x0, ...
This patch fixes it.
VMware-BZ: #2700626
Signed-off-by: Yifeng Sun
---
v1->v2: Add check for ofmonitor_flush, thanks William.
ofproto/connmgr.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c
index 9c5c633b4171..fa8f6cd0e83a 100644
--- a/ofproto/connmgr.c
+++ b/ofproto/connmgr.c
@@ -2140,7 +2140,7 @@ ofmonitor_report(struct connmgr *mgr, struct rule *rule,
const struct rule_actions *old_actions)
OVS_REQUIRES(ofproto_mutex)
{
-if (rule_is_hidden(rule)) {
+if (!mgr || rule_is_hidden(rule)) {
return;
}
@@ -2244,6 +2244,10 @@ ofmonitor_flush(struct connmgr *mgr)
{
struct ofconn *ofconn;
+if (!mgr) {
+return;
+}
+
LIST_FOR_EACH (ofconn, connmgr_node, >conns) {
struct rconn_packet_counter *counter = ofconn->monitor_counter;
--
2.7.4
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH] rhel: Fix dual kernel rpm install for RHEL 8.4
LGTM, thanks Greg. Reviewed-by: Yifeng Sun On Mon, Aug 23, 2021 at 9:33 AM Greg Rose wrote: > RHEL 8.4 is the first of the RHEL 8.x kernels that has broken ABI so > it requires the same sort of fix as we did for several RHEL 7.x kernel > that needed two kernel rpms to work for all minor revisions of the > baseline kernel module. > > Signed-off-by: Greg Rose > --- > rhel/usr_share_openvswitch_scripts_ovs-kmod-manage.sh | 8 > 1 file changed, 8 insertions(+) > > diff --git a/rhel/usr_share_openvswitch_scripts_ovs-kmod-manage.sh > b/rhel/usr_share_openvswitch_scripts_ovs-kmod-manage.sh > index 22bebaa58..01d31a216 100644 > --- a/rhel/usr_share_openvswitch_scripts_ovs-kmod-manage.sh > +++ b/rhel/usr_share_openvswitch_scripts_ovs-kmod-manage.sh > @@ -24,6 +24,7 @@ > # - 3.10.0 major revision 1160 (RHEL 7.9) > # - 4.4.x, x >= 73 (SLES 12 SP3) > # - 4.12.x, x >= 14 (SLES 12 SP4). > +# - 4.18.x major revision 305 (RHEL 8.4) > # It is packaged in the openvswitch kmod RPM and run in the post-install > # scripts. > # > @@ -139,6 +140,13 @@ elif [ "$mainline_major" = "4" ] && [ > "$mainline_minor" = "12" ]; then > ver_offset=2 > installed_ver="$mainline_patch" > fi > +elif [ "$mainline_major" = "4" ] && [ "$mainline_minor" = "18" ]; then > +if [ "$major_rev" = "305" ]; then > +echo "rhel84" > +comp_ver=9 > +ver_offset=4 > +installed_ver="$minor_rev" > +fi > fi > > if [ X"$ver_offset" = X ]; then > -- > 2.17.1 > > ___ > dev mailing list > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH] conntrack: Support packets/bytes stats
Userspace conntrack doesn't support conntrack stats for packets and
bytes. This patch implements it.
Signed-off-by: Yifeng Sun
---
lib/conntrack-private.h | 9 +
lib/conntrack.c | 28
tests/system-common-macros.at | 2 +-
tests/system-traffic.at | 30 ++
4 files changed, 68 insertions(+), 1 deletion(-)
diff --git a/lib/conntrack-private.h b/lib/conntrack-private.h
index dfdf4e676..7f21d3772 100644
--- a/lib/conntrack-private.h
+++ b/lib/conntrack-private.h
@@ -91,6 +91,11 @@ enum OVS_PACKED_ENUM ct_conn_type {
CT_CONN_TYPE_UN_NAT,
};
+struct conn_counter {
+atomic_uint64_t packets;
+atomic_uint64_t bytes;
+};
+
struct conn {
/* Immutable data. */
struct conn_key key;
@@ -123,6 +128,10 @@ struct conn {
enum ct_conn_type conn_type;
uint32_t tp_id; /* Timeout policy ID. */
+
+/* Counters. */
+struct conn_counter counters_orig;
+struct conn_counter counters_reply;
};
enum ct_update_res {
diff --git a/lib/conntrack.c b/lib/conntrack.c
index 33a1a9295..177154cd8 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -1245,6 +1245,21 @@ conn_update_state_alg(struct conntrack *ct, struct
dp_packet *pkt,
return false;
}
+static void
+conn_update_counters(struct conn *conn,
+ const struct dp_packet *pkt, bool reply)
+{
+if (conn) {
+struct conn_counter *counter = (reply
+ ? >counters_reply
+ : >counters_orig);
+uint64_t old;
+
+atomic_count_inc64(>packets);
+atomic_add(>bytes, dp_packet_size(pkt), );
+}
+}
+
static void
set_cached_conn(const struct nat_action_info_t *nat_action_info,
const struct conn_lookup_ctx *ctx, struct conn *conn,
@@ -1283,6 +1298,8 @@ process_one_fast(uint16_t zone, const uint32_t *setmark,
if (setlabel) {
set_label(pkt, conn, [0], [1]);
}
+
+conn_update_counters(conn, pkt, pkt->md.reply);
}
static void
@@ -1420,6 +1437,8 @@ process_one(struct conntrack *ct, struct dp_packet *pkt,
set_label(pkt, conn, [0], [1]);
}
+conn_update_counters(conn, pkt, ctx->reply);
+
handle_alg_ctl(ct, ctx, pkt, ct_alg_ctl, conn, now, !!nat_action_info);
set_cached_conn(nat_action_info, ctx, conn, pkt);
@@ -2641,6 +2660,15 @@ conn_to_ct_dpif_entry(const struct conn *conn, struct
ct_dpif_entry *entry,
}
ovs_mutex_unlock(>lock);
+entry->counters_orig.packets = atomic_count_get64(
+(atomic_uint64_t *)>counters_orig.packets);
+entry->counters_orig.bytes = atomic_count_get64(
+(atomic_uint64_t *)>counters_orig.bytes);
+entry->counters_reply.packets = atomic_count_get64(
+(atomic_uint64_t *)>counters_reply.packets);
+entry->counters_reply.bytes = atomic_count_get64(
+(atomic_uint64_t *)>counters_reply.bytes);
+
entry->timeout = (expiration > 0) ? expiration / 1000 : 0;
if (conn->alg) {
diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at
index 19a0b125b..89cd7b83c 100644
--- a/tests/system-common-macros.at
+++ b/tests/system-common-macros.at
@@ -240,7 +240,7 @@ m4_define([STRIP_MONITOR_CSUM], [grep "csum:" | sed
's/csum:.*/csum: /'])
# and limit the output to the rows containing 'ip-addr'.
#
m4_define([FORMAT_CT],
-[[grep "dst=$1" | sed -e 's/port=[0-9]*/port=/g' -e
's/id=[0-9]*/id=/g' -e 's/state=[0-9_A-Z]*/state=/g' | sort |
uniq]])
+[[grep "dst=$1" | sed -e 's/port=[0-9]*/port=/g' -e
's/id=[0-9]*/id=/g' -e 's/state=[0-9_A-Z]*/state=/g' -e
's/timeout=[0-9]*/timeout=/g' | sort | uniq]])
# NETNS_DAEMONIZE([namespace], [command], [pidfile])
#
diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index f22d86e46..15b2c288c 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -6743,6 +6743,36 @@ AT_CHECK([ovs-ofctl dump-flows br0 | grep table=2, |
OFPROTO_CLEAR_DURATION_IDLE
OVS_TRAFFIC_VSWITCHD_STOP
AT_CLEANUP
+AT_SETUP([conntrack - stats])
+CHECK_CONNTRACK()
+OVS_TRAFFIC_VSWITCHD_START()
+
+ADD_NAMESPACES(at_ns0, at_ns1)
+
+ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24")
+ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24")
+
+AT_DATA([flows.txt], [dnl
+priority=1,action=drop
+priority=10,arp,action=normal
+priority=100,in_port=1,icmp,action=ct(commit),2
+priority=100,in_port=2,ct_state=-trk,icmp,action=ct(table=0)
+priority=100,in_port=2,ct_state=+trk+est-new,icmp,action=1
+])
+
+AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt])
+
+NS_CHECK_EXEC([at_ns0], [ping -s 500 -q -c 3 -i 0.3 -w 2 10.1.1.2 |
FORMAT_PING], [0], [dnl
+3 packets transmitted, 3 received, 0% packet loss, time 0ms
+])
+
+AT_CHECK([ovs-appctl dpctl/dump-conntrack -s | FORMAT_CT(10.1.1.2)], [0], [dnl
+icmp,orig=(src=10.1.1.1,dst=10.1.
