[ovs-dev,v2] OVN: do not mark ND packets for conntrack in PRE_LB stage

Message ID d99e965505ae6a5167ff5aeba48c8ead83cade56.1527850610.git.lorenzo.bianconi@redhat.com
State Accepted
Delegated to: Guru Shetty
Headers show
Series
  • [ovs-dev,v2] OVN: do not mark ND packets for conntrack in PRE_LB stage
Related show

Commit Message

Lorenzo Bianconi June 1, 2018, 11:05 a.m.
Do not send Neighbor Discovery packets to conntrack module if
load balancing rules have been added to NB db since otherwise
Neighbor Advertisement frames will be discarded by OVN.
In order to reproduce the issue it is enough to add 2 logical ports
to a single logical switch, assign an IPv6 address to each VIF, and
define a load balance rule on the logical switch. After a while the
ping6 from VIF1 to VIF2 will stop since the vm will not receive any NA
packet

Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
---
Changes since v1:
- updated ovn-northd manpage
---
 ovn/northd/ovn-northd.8.xml | 34 +++++++++++++++++++---------------
 ovn/northd/ovn-northd.c     |  6 ++++++
 2 files changed, 25 insertions(+), 15 deletions(-)

Comments

Guru Shetty June 29, 2018, 4:03 p.m. | #1
On 1 June 2018 at 04:05, Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
wrote:

> Do not send Neighbor Discovery packets to conntrack module if
> load balancing rules have been added to NB db since otherwise
> Neighbor Advertisement frames will be discarded by OVN.
> In order to reproduce the issue it is enough to add 2 logical ports
> to a single logical switch, assign an IPv6 address to each VIF, and
> define a load balance rule on the logical switch. After a while the
> ping6 from VIF1 to VIF2 will stop since the vm will not receive any NA
> packet
>
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
>
I applied this to master.


> ---
> Changes since v1:
> - updated ovn-northd manpage
> ---
>  ovn/northd/ovn-northd.8.xml | 34 +++++++++++++++++++---------------
>  ovn/northd/ovn-northd.c     |  6 ++++++
>  2 files changed, 25 insertions(+), 15 deletions(-)
>
> diff --git a/ovn/northd/ovn-northd.8.xml b/ovn/northd/ovn-northd.8.xml
> index 1d68f1aab..4f897bdbe 100644
> --- a/ovn/northd/ovn-northd.8.xml
> +++ b/ovn/northd/ovn-northd.8.xml
> @@ -240,17 +240,19 @@
>      <p>
>        This table prepares flows for possible stateful load balancing
> processing
>        in ingress table <code>LB</code> and <code>Stateful</code>.  It
> contains
> -      a priority-0 flow that simply moves traffic to the next table.  If
> load
> -      balancing rules with virtual IP addresses (and ports) are
> configured in
> -      <code>OVN_Northbound</code> database for a logical switch datapath,
> a
> -      priority-100 flow is added for each configured virtual IP address
> -      <var>VIP</var>. For IPv4 <var>VIPs</var>, the match is <code>ip
> -      &amp;&amp; ip4.dst == <var>VIP</var></code>.  For IPv6
> <var>VIPs</var>,
> -      the match is <code>ip &amp;&amp; ip6.dst == <var>VIP</var></code>.
> The
> -      flow sets an action <code>reg0[0] = 1; next;</code> to act as a
> -      hint for table <code>Pre-stateful</code> to send IP packets to the
> -      connection tracker for packet de-fragmentation before eventually
> -      advancing to ingress table <code>LB</code>.
> +      a priority-0 flow that simply moves traffic to the next table.
> Moreover
> +      it contains a priority-110 flow to move IPv6 Neighbor Discovery
> traffic
> +      to the next table. If load balancing rules with virtual IP addresses
> +      (and ports) are configured in <code>OVN_Northbound</code> database
> for a
> +      logical switch datapath, a priority-100 flow is added for each
> configured
> +      virtual IP address <var>VIP</var>. For IPv4 <var>VIPs</var>, the
> match is
> +      <code>ip &amp;&amp; ip4.dst == <var>VIP</var></code>. For IPv6
> +      <var>VIPs</var>, the match is <code>ip &amp;&amp;
> +      ip6.dst == <var>VIP</var></code>. The flow sets an action
> +      <code>reg0[0] = 1; next;</code> to act as a hint for table
> +      <code>Pre-stateful</code> to send IP packets to the connection
> tracker
> +      for packet de-fragmentation before eventually advancing to ingress
> table
> +      <code>LB</code>.
>      </p>
>
>      <h3>Ingress Table 5: Pre-stateful</h3>
> @@ -866,10 +868,12 @@ output;
>      <p>
>        This table is similar to ingress table <code>Pre-LB</code>.  It
>        contains a priority-0 flow that simply moves traffic to the next
> table.
> -      If any load balancing rules exist for the datapath, a priority-100
> flow
> -      is added with a match of <code>ip</code> and action of
> <code>reg0[0] = 1;
> -       next;</code> to act as a hint for table <code>Pre-stateful</code>
> to
> -      send IP packets to the connection tracker for packet
> de-fragmentation.
> +      Moreover it contains a priority-110 flow to move IPv6 Neighbor
> Discovery
> +      traffic to the next table. If any load balancing rules exist for the
> +      datapath, a priority-100 flow is added with a match of
> <code>ip</code>
> +      and action of <code>reg0[0] = 1; next;</code> to act as a hint for
> +      table <code>Pre-stateful</code> to send IP packets to the connection
> +      tracker for packet de-fragmentation.
>      </p>
>
>      <h3>Egress Table 1: <code>to-lport</code> Pre-ACLs</h3>
> diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
> index 0e06776ad..aa9298d3b 100644
> --- a/ovn/northd/ovn-northd.c
> +++ b/ovn/northd/ovn-northd.c
> @@ -2977,6 +2977,12 @@ ls_has_dns_records(const struct
> nbrec_logical_switch *nbs)
>  static void
>  build_pre_lb(struct ovn_datapath *od, struct hmap *lflows)
>  {
> +    /* Do not send ND packets to conntrack */
> +    ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_LB, 110,
> +                  "nd || nd_rs || nd_ra", "next;");
> +    ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_LB, 110,
> +                  "nd || nd_rs || nd_ra", "next;");
> +
>      /* Allow all packets to go to next tables by default. */
>      ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_LB, 0, "1", "next;");
>      ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_LB, 0, "1", "next;");
> --
> 2.14.3
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>

Patch

diff --git a/ovn/northd/ovn-northd.8.xml b/ovn/northd/ovn-northd.8.xml
index 1d68f1aab..4f897bdbe 100644
--- a/ovn/northd/ovn-northd.8.xml
+++ b/ovn/northd/ovn-northd.8.xml
@@ -240,17 +240,19 @@ 
     <p>
       This table prepares flows for possible stateful load balancing processing
       in ingress table <code>LB</code> and <code>Stateful</code>.  It contains
-      a priority-0 flow that simply moves traffic to the next table.  If load
-      balancing rules with virtual IP addresses (and ports) are configured in
-      <code>OVN_Northbound</code> database for a logical switch datapath, a
-      priority-100 flow is added for each configured virtual IP address
-      <var>VIP</var>. For IPv4 <var>VIPs</var>, the match is <code>ip
-      &amp;&amp; ip4.dst == <var>VIP</var></code>.  For IPv6 <var>VIPs</var>,
-      the match is <code>ip &amp;&amp; ip6.dst == <var>VIP</var></code>. The
-      flow sets an action <code>reg0[0] = 1; next;</code> to act as a
-      hint for table <code>Pre-stateful</code> to send IP packets to the
-      connection tracker for packet de-fragmentation before eventually
-      advancing to ingress table <code>LB</code>.
+      a priority-0 flow that simply moves traffic to the next table. Moreover
+      it contains a priority-110 flow to move IPv6 Neighbor Discovery traffic
+      to the next table. If load balancing rules with virtual IP addresses
+      (and ports) are configured in <code>OVN_Northbound</code> database for a
+      logical switch datapath, a priority-100 flow is added for each configured
+      virtual IP address <var>VIP</var>. For IPv4 <var>VIPs</var>, the match is
+      <code>ip &amp;&amp; ip4.dst == <var>VIP</var></code>. For IPv6
+      <var>VIPs</var>, the match is <code>ip &amp;&amp;
+      ip6.dst == <var>VIP</var></code>. The flow sets an action
+      <code>reg0[0] = 1; next;</code> to act as a hint for table
+      <code>Pre-stateful</code> to send IP packets to the connection tracker
+      for packet de-fragmentation before eventually advancing to ingress table
+      <code>LB</code>.
     </p>
 
     <h3>Ingress Table 5: Pre-stateful</h3>
@@ -866,10 +868,12 @@  output;
     <p>
       This table is similar to ingress table <code>Pre-LB</code>.  It
       contains a priority-0 flow that simply moves traffic to the next table.
-      If any load balancing rules exist for the datapath, a priority-100 flow
-      is added with a match of <code>ip</code> and action of <code>reg0[0] = 1;
-       next;</code> to act as a hint for table <code>Pre-stateful</code> to
-      send IP packets to the connection tracker for packet de-fragmentation.
+      Moreover it contains a priority-110 flow to move IPv6 Neighbor Discovery
+      traffic to the next table. If any load balancing rules exist for the
+      datapath, a priority-100 flow is added with a match of <code>ip</code>
+      and action of <code>reg0[0] = 1; next;</code> to act as a hint for
+      table <code>Pre-stateful</code> to send IP packets to the connection
+      tracker for packet de-fragmentation.
     </p>
 
     <h3>Egress Table 1: <code>to-lport</code> Pre-ACLs</h3>
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 0e06776ad..aa9298d3b 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -2977,6 +2977,12 @@  ls_has_dns_records(const struct nbrec_logical_switch *nbs)
 static void
 build_pre_lb(struct ovn_datapath *od, struct hmap *lflows)
 {
+    /* Do not send ND packets to conntrack */
+    ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_LB, 110,
+                  "nd || nd_rs || nd_ra", "next;");
+    ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_LB, 110,
+                  "nd || nd_rs || nd_ra", "next;");
+
     /* Allow all packets to go to next tables by default. */
     ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_LB, 0, "1", "next;");
     ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_LB, 0, "1", "next;");