diff mbox series

[ovs-dev,1/3] Revert "northd: support HW VTEP with stateful datapath"

Message ID 20211201125608.36918-2-odivlad@gmail.com
State Accepted
Headers show
Series Support mixing stateless and stateful ACLs regardless of their priority | expand

Checks

Context Check Description
ovsrobot/apply-robot warning apply and check: warning
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/github-robot-_ovn-kubernetes success github build: passed

Commit Message

Vladislav Odintsov Dec. 1, 2021, 12:56 p.m. UTC
This reverts commit 62ca8b9620cc1168ace6905575b7d36438363aed.
---
 northd/northd.c         | 14 --------------
 northd/ovn-northd.8.xml | 28 ----------------------------
 northd/ovn_northd.dl    | 33 ++-------------------------------
 tests/ovn-northd.at     |  2 --
 4 files changed, 2 insertions(+), 75 deletions(-)

Comments

0-day Robot Dec. 1, 2021, 1 p.m. UTC | #1
Bleep bloop.  Greetings Vladislav Odintsov, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
ERROR: Author Vladislav Odintsov <odivlad@gmail.com> needs to sign off.
Lines checked: 207, Warnings: 0, Errors: 1


Please check this out.  If you feel there has been an error, please email aconole@redhat.com

Thanks,
0-day Robot
Vladislav Odintsov Dec. 1, 2021, 1:04 p.m. UTC | #2
Signed-off-by: Vladislav Odintsov <odivlad@gmail.com>

Regards,
Vladislav Odintsov

> On 1 Dec 2021, at 16:00, 0-day Robot <robot@bytheb.org> wrote:
> 
> Bleep bloop.  Greetings Vladislav Odintsov, I am a robot and I have tried out your patch.
> Thanks for your contribution.
> 
> I encountered some error that I wasn't expecting.  See the details below.
> 
> 
> checkpatch:
> ERROR: Author Vladislav Odintsov <odivlad@gmail.com> needs to sign off.
> Lines checked: 207, Warnings: 0, Errors: 1
> 
> 
> Please check this out.  If you feel there has been an error, please email aconole@redhat.com
> 
> Thanks,
> 0-day Robot
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
diff mbox series

Patch

diff --git a/northd/northd.c b/northd/northd.c
index e784ae192..4c1a2a382 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -197,7 +197,6 @@  enum ovn_stage {
 #define REGBIT_LKUP_FDB           "reg0[11]"
 #define REGBIT_HAIRPIN_REPLY      "reg0[12]"
 #define REGBIT_ACL_LABEL          "reg0[13]"
-#define REGBIT_FROM_RAMP          "reg0[14]"
 
 #define REG_ORIG_DIP_IPV4         "reg1"
 #define REG_ORIG_DIP_IPV6         "xxreg1"
@@ -5477,15 +5476,10 @@  build_lswitch_input_port_sec_op(
     build_port_security_l2("eth.src", op->ps_addrs, op->n_ps_addrs,
                            match);
 
-    if (!strcmp(op->nbsp->type, "vtep")) {
-        ds_put_format(actions, REGBIT_FROM_RAMP" = 1; ");
-    }
-
     const char *queue_id = smap_get(&op->sb->options, "qdisc_queue_id");
     if (queue_id) {
         ds_put_format(actions, "set_queue(%s); ", queue_id);
     }
-
     ds_put_cstr(actions, "next;");
     ovn_lflow_add_with_lport_and_hint(lflows, op->od, S_SWITCH_IN_PORT_SEC_L2,
                                       50, ds_cstr(match), ds_cstr(actions),
@@ -5734,10 +5728,6 @@  build_pre_acls(struct ovn_datapath *od, const struct hmap *port_groups,
                       "nd || nd_rs || nd_ra || mldv1 || mldv2 || "
                       "(udp && udp.src == 546 && udp.dst == 547)", "next;");
 
-        /* Do not send coming from RAMP switch packets to conntrack. */
-        ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 110,
-                      REGBIT_FROM_RAMP" == 1", "next;");
-
         /* Ingress and Egress Pre-ACL Table (Priority 100).
          *
          * Regardless of whether the ACL is "from-lport" or "to-lport",
@@ -5828,10 +5818,6 @@  build_pre_lb(struct ovn_datapath *od, struct hmap *lflows)
     ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_LB, 110,
                   "eth.src == $svc_monitor_mac", "next;");
 
-    /* Do not send coming from RAMP switch packets to conntrack. */
-    ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_LB, 110,
-                  REGBIT_FROM_RAMP" == 1", "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;");
diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
index 5d06ac6a7..00fb925f8 100644
--- a/northd/ovn-northd.8.xml
+++ b/northd/ovn-northd.8.xml
@@ -262,16 +262,6 @@ 
         logical ports on which port security is not enabled, these advance all
         packets that match the <code>inport</code>.
       </li>
-      <li>
-        For logical ports of type <code>vtep</code>, the above logical flow
-        will also apply the action <code>REGBIT_FROM_RAMP = 1;</code> to
-        indicate that the packet is coming from a RAMP (controller-vtep)
-        device.  Later pipelines will use this information to skip
-        sending the packet to the conntrack.  Packets from <code>vtep</code>
-        logical ports should go though ingress pipeline only to determine
-        the output port and they should not be subjected to any ACL checks.
-        Egress pipeline will do the ACL checks.
-      </li>
     </ul>
 
     <p>
@@ -463,15 +453,6 @@ 
       processing.
     </p>
 
-    <p>
-      This table has a priority-110 flow with the match
-      <code>REGBIT_FROM_RAMP == 1</code> for all logical switch datapaths to
-      resubmit traffic to the next table. <code>REGBIT_FROM_RAMP</code>
-      indicates that packet was received from <code>vtep</code> logical ports
-      and it can be skipped from the stateful ACL processing in the ingress
-      pipeline.
-    </p>
-
     <p>
       This table also has a priority-110 flow with the match
       <code>eth.dst == <var>E</var></code> for all logical switch
@@ -531,15 +512,6 @@ 
       configured. We can now add a lflow to drop ct.inv packets.
     </p>
 
-    <p>
-      This table has a priority-110 flow with the match
-      <code>REGBIT_FROM_RAMP == 1</code> for all logical switch datapaths to
-      resubmit traffic to the next table. <code>REGBIT_FROM_RAMP</code>
-      indicates that packet was received from <code>vtep</code> logical ports
-      and it can be skipped from the load balancer processing in the ingress
-      pipeline.
-    </p>
-
     <p>
       This table also has a priority-110 flow with the match
       <code>eth.dst == <var>E</var></code> for all logical switch
diff --git a/northd/ovn_northd.dl b/northd/ovn_northd.dl
index 817b11bdc..ffa2e06db 100644
--- a/northd/ovn_northd.dl
+++ b/northd/ovn_northd.dl
@@ -1738,7 +1738,6 @@  function rEGBIT_ACL_HINT_BLOCK()   : istring = i"reg0[10]"
 function rEGBIT_LKUP_FDB()         : istring = i"reg0[11]"
 function rEGBIT_HAIRPIN_REPLY()    : istring = i"reg0[12]"
 function rEGBIT_ACL_LABEL()        : istring = i"reg0[13]"
-function rEGBIT_FROM_RAMP()        : istring = i"reg0[14]"
 
 function rEG_ORIG_DIP_IPV4()       : istring = i"reg1"
 function rEG_ORIG_DIP_IPV6()       : istring = i"xxreg1"
@@ -2178,16 +2177,6 @@  for (&Switch(._uuid = ls_uuid, .has_stateful_acl = true)) {
          .io_port          = None,
          .controller_meter = None);
 
-    /* Do not send coming from RAMP switch packets to conntrack. */
-    Flow(.logical_datapath = ls_uuid,
-         .stage            = s_SWITCH_IN_PRE_ACL(),
-         .priority         = 110,
-         .__match          = i"${rEGBIT_FROM_RAMP()} == 1",
-         .actions          = i"next;",
-         .stage_hint       = 0,
-         .io_port          = None,
-         .controller_meter = None);
-
     /* Ingress and Egress Pre-ACL Table (Priority 100).
      *
      * Regardless of whether the ACL is "from-lport" or "to-lport",
@@ -2254,16 +2243,6 @@  for (&Switch(._uuid = ls_uuid)) {
          .io_port          = None,
          .controller_meter = None);
 
-    /* Do not send coming from RAMP switch packets to conntrack. */
-    Flow(.logical_datapath = ls_uuid,
-         .stage            = s_SWITCH_IN_PRE_LB(),
-         .priority         = 110,
-         .__match          = i"${rEGBIT_FROM_RAMP()} == 1",
-         .actions          = i"next;",
-         .stage_hint       = 0,
-         .io_port          = None,
-         .controller_meter = None);
-
     /* Allow all packets to go to next tables by default. */
     Flow(.logical_datapath = ls_uuid,
          .stage            = s_SWITCH_IN_PRE_LB(),
@@ -3489,18 +3468,10 @@  for (&SwitchPort(.lsp = lsp, .sw = sw, .json_name = json_name, .ps_eth_addresses
             } else {
                 i"inport == ${json_name} && eth.src == {${ps_eth_addresses.join(\" \")}}"
             } in
-        var actions = {
-            var ramp = if (lsp.__type == i"vtep") {
-                i"${rEGBIT_FROM_RAMP()} = 1; "
-            } else {
-                i""
-            };
-            var queue = match (pbinding.options.get(i"qdisc_queue_id")) {
+        var actions = match (pbinding.options.get(i"qdisc_queue_id")) {
                 None -> i"next;",
                 Some{id} -> i"set_queue(${id}); next;"
-            };
-            i"${ramp}${queue}"
-        } in
+            } in
         Flow(.logical_datapath = sw._uuid,
              .stage            = s_SWITCH_IN_PORT_SEC_L2(),
              .priority         = 50,
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index f03d14082..c4424ab14 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -3834,7 +3834,6 @@  check_stateful_flows() {
   table=6 (ls_in_pre_lb       ), priority=110  , match=(eth.dst == $svc_monitor_mac), action=(next;)
   table=6 (ls_in_pre_lb       ), priority=110  , match=(ip && inport == "sw0-lr0"), action=(next;)
   table=6 (ls_in_pre_lb       ), priority=110  , match=(nd || nd_rs || nd_ra || mldv1 || mldv2), action=(next;)
-  table=6 (ls_in_pre_lb       ), priority=110  , match=(reg0[[14]] == 1), action=(next;)
 ])
 
     AT_CHECK([grep "ls_in_pre_stateful" sw0flows | sort], [0], [dnl
@@ -3901,7 +3900,6 @@  AT_CHECK([grep "ls_in_pre_lb" sw0flows | sort], [0], [dnl
   table=6 (ls_in_pre_lb       ), priority=110  , match=(eth.dst == $svc_monitor_mac), action=(next;)
   table=6 (ls_in_pre_lb       ), priority=110  , match=(ip && inport == "sw0-lr0"), action=(next;)
   table=6 (ls_in_pre_lb       ), priority=110  , match=(nd || nd_rs || nd_ra || mldv1 || mldv2), action=(next;)
-  table=6 (ls_in_pre_lb       ), priority=110  , match=(reg0[[14]] == 1), action=(next;)
 ])
 
 AT_CHECK([grep "ls_in_pre_stateful" sw0flows | sort], [0], [dnl