On 10/10/22 17:55, Adrian Moreno wrote:
> 
> 
> On 10/10/22 13:56, Dumitru Ceara wrote:
>> On 9/27/22 09:31, Adrian Moreno wrote:
>>> Add a new config flag called "drop-debug-mode" that makes northd add an
>>> explicit default drop to all tables that currently do not have a default
>>> (prio=0, match=1) lflow.
>>>
>>> In the controller side, also add explicit default drop rules on physical
>>> tables that need it.
>>>
>>> When this mode is enabled the explicit drop actions  make it easier to
>>> debug when OVN is dropping a packet.
>>>
>>> Signed-off-by: Adrian Moreno <[email protected]>
>>> ---
>>
>> Hi Adrian,
>>
>>>   controller/ovn-controller.c |  33 ++++++++
>>>   controller/physical.c       |  48 +++++++++++
>>>   controller/physical.h       |   1 +
>>>   northd/automake.mk          |   2 +
>>>   northd/debug.c              |  23 ++++++
>>>   northd/debug.h              |  31 +++++++
>>>   northd/northd.c             |  44 +++++++++-
>>>   ovn-nb.xml                  |   8 ++
>>>   tests/ovn-northd.at         |  75 +++++++++++++++++
>>>   tests/ovn.at                | 158 +++++++++++++++++++++++++++++++++++-
>>>   10 files changed, 418 insertions(+), 5 deletions(-)
>>>   create mode 100644 northd/debug.c
>>>   create mode 100644 northd/debug.h
>>>
>>> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
>>> index 43fbf2ba3..cc3bea64b 100644
>>> --- a/controller/ovn-controller.c
>>> +++ b/controller/ovn-controller.c
>>> @@ -3118,6 +3118,8 @@ lflow_output_sb_meter_handler(struct
>>> engine_node *node, void *data)
>>>   struct ed_type_pflow_output {
>>>       /* Desired physical flows. */
>>>       struct ovn_desired_flow_table flow_table;
>>> +    /* Drop debugging options. */
>>> +    bool debug_drop;
>>
>> I might have missed some of the discussion on the initial RFC but I
>> don't really see the reason why we only add the explicit drop rules if
>> debug_drop is set to 'true'.  I have the same comment for the northd
>> side.
>>
>> In general we add:
>> - one openflow for every table explicitly maintained by ovn-controller
>> (e.g., in physical.c).
>> - one logical flow with a generic match and action for every logical
>> datapath for every stage in northd.
>>
>> These last logical flows actually get combined into a single logical
>> flow per stage (applied on a datapath group).  So there's no potential
>> performance impact.  So I would keep it simple and always add these drop
>> flows.
> 
> I don't know much about the datapath group optimization, but as per
> Mark's comment on v1
> (https://patchwork.ozlabs.org/project/ovn/patch/[email protected]/#2914552)
>  I was under the impression that there would be one logical flow per datapath.
> 
> Did a quick test with ovn-k8s kind setup:
> #nodes : [lflows before] -> [lflows after enabling debug-mode]
> 2:  1123 -> 1151
> 3:  1449 -> 1484
> 4:  1775 -> 1817
> 5:  2101 -> 2150
> 

Did you use "ovn-sbctl lflow-list" or "ovn-sbctl list logical_flow"?
The former expands logical datapath groups.

In an ovn sandbox I did:

$ ovn-nbctl lr-add lr1 -- lr-add lr2
$ ovn-nbctl --wait=sb sync
$ ovn-sbctl list logical_flow | grep uuid -c
60
$ ovn-sbctl lflow-list | wc -l
96

$ ovn-nbctl set NB_Global . options:debug_drop_mode="true"
$ ovn-sbctl list logical_flow | grep uuid -c
52
$ ovn-sbctl lflow-list | wc -l
108

# Add a third router, the actual number of lflows stays the same (as
# reported by "ovn-sbctl list logical_flow"):
$ ovn-nbctl lr-add lr3
$ ovn-sbctl list logical_flow | grep uuid -c
52
$ ovn-sbctl lflow-list | wc -l
162

Also, since 22.09.0 datapath groups are always on and can't be disabled:
https://github.com/ovn-org/ovn/commit/9dea0c090e91

Regards,
Dumitru

> ...sorry ovn-k8s kind setup does not work for me for larger clusters :(
> 
> So there is some difference that seems to be proportional to the number
> of datapaths.
> 
> You are the experts so I'd leave it at your discretion.
> 
>>
>> Or am I missing something?
>>
>>>   };
>>>     static void init_physical_ctx(struct engine_node *node,
>>> @@ -3167,6 +3169,12 @@ static void init_physical_ctx(struct
>>> engine_node *node,
>>>           chassis = chassis_lookup_by_name(sbrec_chassis_by_name,
>>> chassis_id);
>>>       }
>>>   +    struct sbrec_sb_global_table *sb_global_table =
>>> +        (struct sbrec_sb_global_table *)EN_OVSDB_GET(
>>> +            engine_get_input("SB_sb_global", node));
>>> +    const struct sbrec_sb_global *sb_global =
>>> +        sbrec_sb_global_table_first(sb_global_table);
>>> +
>>>       ovs_assert(br_int && chassis);
>>>         struct ed_type_ct_zones *ct_zones_data =
>>> @@ -3188,6 +3196,8 @@ static void init_physical_ctx(struct
>>> engine_node *node,
>>>       p_ctx->local_bindings = &rt_data->lbinding_data.bindings;
>>>       p_ctx->patch_ofports = &non_vif_data->patch_ofports;
>>>       p_ctx->chassis_tunnels = &non_vif_data->chassis_tunnels;
>>> +    p_ctx->debug_drop = smap_get_bool(&sb_global->options,
>>> +                                      "debug_drop_mode", false);
>>>   }
>>>     static void *
>>> @@ -3390,6 +3400,27 @@ pflow_output_activated_ports_handler(struct
>>> engine_node *node, void *data)
>>>       return true;
>>>   }
>>>   +static bool
>>> +pflow_output_sb_sb_global_handler(struct engine_node *node, void *data)
>>> +{
>>> +    struct sbrec_sb_global_table *sb_global_table =
>>> +        (struct sbrec_sb_global_table *)EN_OVSDB_GET(
>>> +            engine_get_input("SB_sb_global", node));
>>> +    const struct sbrec_sb_global *sb_global =
>>> +        sbrec_sb_global_table_first(sb_global_table);
>>> +
>>> +    struct ed_type_pflow_output *pfo = data;
>>> +
>>> +    bool debug_drop = smap_get_bool(&sb_global->options,
>>> +                                    "debug_drop_mode", false);
>>> +
>>> +    if (pfo->debug_drop != debug_drop) {
>>> +        engine_set_node_state(node, EN_UPDATED);
>>> +        pfo->debug_drop = debug_drop;
>>> +    }
>>> +    return true;
>>> +}
>>> +
>>>   static void *
>>>   en_flow_output_init(struct engine_node *node OVS_UNUSED,
>>>                       struct engine_arg *arg OVS_UNUSED)
>>> @@ -3732,6 +3763,8 @@ main(int argc, char *argv[])
>>>       engine_add_input(&en_pflow_output, &en_mff_ovn_geneve, NULL);
>>>       engine_add_input(&en_pflow_output, &en_ovs_open_vswitch, NULL);
>>>       engine_add_input(&en_pflow_output, &en_ovs_bridge, NULL);
>>> +    engine_add_input(&en_pflow_output, &en_sb_sb_global,
>>> +                     pflow_output_sb_sb_global_handler);
>>>         engine_add_input(&en_northd_options, &en_sb_sb_global,
>>>                        en_northd_options_sb_sb_global_handler);
>>> diff --git a/controller/physical.c b/controller/physical.c
>>> index f3c8bddce..e86d0297c 100644
>>> --- a/controller/physical.c
>>> +++ b/controller/physical.c
>>> @@ -825,6 +825,20 @@ put_zones_ofpacts(const struct zone_ids
>>> *zone_ids, struct ofpbuf *ofpacts_p)
>>>       }
>>>   }
>>>   +static void
>>> +add_default_drop_flow(const struct physical_ctx *p_ctx,
>>> +                      uint8_t table_id,
>>> +                      struct ovn_desired_flow_table *flow_table)
>>> +{
>>> +    if (p_ctx->debug_drop) {
>>> +        struct match match = MATCH_CATCHALL_INITIALIZER;
>>> +        struct ofpbuf ofpacts;
>>> +        ofpbuf_init(&ofpacts, 0);
>>> +        ofctrl_add_flow(flow_table, table_id, 0, 0, &match,
>>> +                        &ofpacts, hc_uuid);
>>> +    }
>>> +}
>>> +
>>>   static void
>>>   put_local_common_flows(uint32_t dp_key,
>>>                          const struct sbrec_port_binding *pb,
>>> @@ -2106,6 +2120,13 @@ physical_run(struct physical_ctx *p_ctx,
>>>           }
>>>       }
>>>   +    /* Table 0, priority 0.
>>> +     * ======================
>>> +     *
>>> +     * Drop packets tha do not match any tunnel in_port.
>>> +     */
>>> +    add_default_drop_flow(p_ctx, OFTABLE_PHY_TO_LOG, flow_table);
>>> +
>>>       /* Table 37, priority 150.
>>>        * =======================
>>>        *
>>> @@ -2151,6 +2172,13 @@ physical_run(struct physical_ctx *p_ctx,
>>>       ofctrl_add_flow(flow_table, OFTABLE_REMOTE_OUTPUT, 0, 0, &match,
>>>                       &ofpacts, hc_uuid);
>>>   +    /* Table 38, priority 0.
>>> +     * ======================
>>> +     *
>>> +     * Drop packets that do not match previous flows.
>>> +     */
>>> +    add_default_drop_flow(p_ctx, OFTABLE_LOCAL_OUTPUT, flow_table);
>>> +
>>>       /* Table 39, Priority 0.
>>>        * =======================
>>>        *
>>> @@ -2177,5 +2205,25 @@ physical_run(struct physical_ctx *p_ctx,
>>>       ofctrl_add_flow(flow_table, OFTABLE_SAVE_INPORT, 0, 0, &match,
>>>                       &ofpacts, hc_uuid);
>>>   +    /* Table 65, priority 0.
>>> +     * ======================
>>> +     *
>>> +     * Drop packets that do not match previous flows.
>>> +     */
>>> +    add_default_drop_flow(p_ctx, OFTABLE_LOG_TO_PHY, flow_table);
>>> +
>>> +    /* Table 68, priority 0.
>>> +     * ======================
>>> +     *
>>> +     * Drop packets that do not match previous flows.
>>> +     */
>>> +    add_default_drop_flow(p_ctx, OFTABLE_CHK_LB_HAIRPIN, flow_table);
>>> +
>>> +    /* Table 70, priority 0.
>>> +     * ======================
>>> +     *
>>> +     * Drop packets that do not match previous flows.
>>> +     */
>>> +    add_default_drop_flow(p_ctx, OFTABLE_CT_SNAT_HAIRPIN, flow_table);
>>>       ofpbuf_uninit(&ofpacts);
>>>   }
>>> diff --git a/controller/physical.h b/controller/physical.h
>>> index 1b8f1ea55..3947099b4 100644
>>> --- a/controller/physical.h
>>> +++ b/controller/physical.h
>>> @@ -59,6 +59,7 @@ struct physical_ctx {
>>>       struct shash *local_bindings;
>>>       struct simap *patch_ofports;
>>>       struct hmap *chassis_tunnels;
>>> +    bool debug_drop;
>>>   };
>>>     void physical_register_ovs_idl(struct ovsdb_idl *);
>>> diff --git a/northd/automake.mk b/northd/automake.mk
>>> index 81582867d..14cf525d8 100644
>>> --- a/northd/automake.mk
>>> +++ b/northd/automake.mk
>>> @@ -1,6 +1,8 @@
>>>   # ovn-northd
>>>   bin_PROGRAMS += northd/ovn-northd
>>>   northd_ovn_northd_SOURCES = \
>>> +    northd/debug.c \
>>> +    northd/debug.h \
>>>       northd/mac-binding-aging.c \
>>>       northd/mac-binding-aging.h \
>>>       northd/northd.c \
>>> diff --git a/northd/debug.c b/northd/debug.c
>>> new file mode 100644
>>> index 000000000..3db0a3c4f
>>> --- /dev/null
>>> +++ b/northd/debug.c
>>> @@ -0,0 +1,23 @@
>>> +#include <config.h>
>>> +
>>> +#include <string.h>
>>> +
>>> +#include "debug.h"
>>> +
>>> +#include "smap.h"
>>> +
>>> +static struct debug_config config;
>>> +
>>> +void
>>> +init_debug_config(const struct nbrec_nb_global *nb)
>>> +{
>>> +
>>> +    const struct smap *options = &nb->options;
>>> +    config.enabled = smap_get_bool(options, "debug_drop_mode", false);
>>> +}
>>> +
>>> +bool
>>> +debug_enabled(void)
>>> +{
>>> +    return config.enabled;
>>> +}
>>> diff --git a/northd/debug.h b/northd/debug.h
>>> new file mode 100644
>>> index 000000000..69d4da171
>>> --- /dev/null
>>> +++ b/northd/debug.h
>>> @@ -0,0 +1,31 @@
>>> +/*
>>> + * Licensed under the Apache License, Version 2.0 (the "License");
>>> + * you may not use this file except in compliance with the License.
>>> + * You may obtain a copy of the License at:
>>> + *
>>> + *     http://www.apache.org/licenses/LICENSE-2.0
>>> + *
>>> + * Unless required by applicable law or agreed to in writing, software
>>> + * distributed under the License is distributed on an "AS IS" BASIS,
>>> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
>>> implied.
>>> + * See the License for the specific language governing permissions and
>>> + * limitations under the License.
>>> + */
>>> +
>>> +#ifndef NORTHD_DEBUG_H
>>> +#define NORTHD_DEBUG_H 1
>>> +
>>> +#include <stdint.h>
>>> +#include <stdbool.h>
>>> +
>>> +#include "lib/ovn-nb-idl.h"
>>> +
>>> +struct debug_config {
>>> +    bool enabled;
>>> +};
>>> +
>>> +void init_debug_config(const struct nbrec_nb_global *nb);
>>> +
>>> +bool debug_enabled(void);
>>> +
>>> +#endif /* NORTHD_DEBUG_H */
>>> diff --git a/northd/northd.c b/northd/northd.c
>>> index 84440a47f..2cd0d0b11 100644
>>> --- a/northd/northd.c
>>> +++ b/northd/northd.c
>>> @@ -17,6 +17,7 @@
>>>   #include <stdlib.h>
>>>   #include <stdio.h>
>>>   +#include "debug.h"
>>>   #include "bitmap.h"
>>>   #include "dirs.h"
>>>   #include "ipam.h"
>>> @@ -5148,6 +5149,18 @@ ovn_lflow_add_at(struct hmap *lflow_map,
>>> struct ovn_datapath *od,
>>>                                  io_port, ctrl_meter, stage_hint,
>>> where, hash);
>>>   }
>>>   +static void
>>> +__ovn_lflow_add_default_drop(struct hmap *lflow_map,
>>> +                             struct ovn_datapath *od,
>>> +                             enum ovn_stage stage,
>>> +                             const char *where)
>>> +{
>>> +    if (OVS_UNLIKELY(debug_enabled())) {
>>> +        ovn_lflow_add_at(lflow_map, od, stage, 0, "1", "drop;",
>>> +                         NULL, NULL, NULL, where );
>>> +    }
>>> +}
>>> +
>>>   /* Adds a row with the specified contents to the Logical_Flow
>>> table. */
>>>   #define ovn_lflow_add_with_hint__(LFLOW_MAP, OD, STAGE, PRIORITY,
>>> MATCH, \
>>>                                     ACTIONS, IN_OUT_PORT, CTRL_METER, \
>>> @@ -5160,6 +5173,10 @@ ovn_lflow_add_at(struct hmap *lflow_map,
>>> struct ovn_datapath *od,
>>>       ovn_lflow_add_at(LFLOW_MAP, OD, STAGE, PRIORITY, MATCH, ACTIONS, \
>>>                        NULL, NULL, STAGE_HINT, OVS_SOURCE_LOCATOR)
>>>   +#define ovn_lflow_add_default_drop(LFLOW_MAP, OD,
>>> STAGE)                    \
>>> +    __ovn_lflow_add_default_drop(LFLOW_MAP, OD, STAGE,
>>> OVS_SOURCE_LOCATOR)
>>> +
>>> +
>>>   /* This macro is similar to ovn_lflow_add_with_hint, except that it
>>> requires
>>>    * the IN_OUT_PORT argument, which tells the lport name that
>>> appears in the
>>>    * MATCH, which helps ovn-controller to bypass lflows parsing when
>>> the lport is
>>> @@ -7879,6 +7896,9 @@ build_lswitch_lflows_admission_control(struct
>>> ovn_datapath *od,
>>>                         REGBIT_PORT_SEC_DROP" == 1", "drop;");
>>>             ovn_lflow_add(lflows, od, S_SWITCH_IN_APPLY_PORT_SEC, 0,
>>> "1", "next;");
>>> +        /* Port security flows have priority 50
>>> +         * (see build_lswitch_input_port_sec()) and will continue
>>> +         * to the next table if packet source is acceptable. */
>>>       }
>>>   }
>>>   @@ -9807,6 +9827,7 @@ add_route(struct hmap *lflows, struct
>>> ovn_datapath *od,
>>>                                   priority + 1, ds_cstr(&match),
>>>                                   ds_cstr(&common_actions), stage_hint);
>>>       }
>>> +
>>
>> Nit: unrelated.
>>
>> Thanks,
>> Dumitru
>>
>>>       ds_destroy(&match);
>>>       ds_destroy(&common_actions);
>>>       ds_destroy(&actions);
>>> @@ -10963,6 +10984,9 @@ build_adm_ctrl_flows_for_lrouter(
>>>            * Broadcast/multicast source address is invalid. */
>>>           ovn_lflow_add(lflows, od, S_ROUTER_IN_ADMISSION, 100,
>>>                         "vlan.present || eth.src[40]", "drop;");
>>> +
>>> +        /* Default action for L2 security is to drop. */
>>> +        ovn_lflow_add_default_drop(lflows, od, S_ROUTER_IN_ADMISSION);
>>>       }
>>>   }
>>>   @@ -11204,6 +11228,8 @@ build_neigh_learning_flows_for_lrouter(
>>>                             "nd_ns", "put_nd(inport, ip6.src,
>>> nd.sll); next;",
>>>                             copp_meter_get(COPP_ND_NS, od->nbr->copp,
>>>                                            meter_groups));
>>> +
>>> +        ovn_lflow_add_default_drop(lflows, od,
>>> S_ROUTER_IN_LEARN_NEIGHBOR);
>>>       }
>>>     }
>>> @@ -11480,6 +11506,8 @@ build_static_route_flows_for_lrouter(
>>>           const struct hmap *bfd_connections)
>>>   {
>>>       if (od->nbr) {
>>> +        ovn_lflow_add_default_drop(lflows, od,
>>> S_ROUTER_IN_IP_ROUTING_ECMP);
>>> +        ovn_lflow_add_default_drop(lflows, od, S_ROUTER_IN_IP_ROUTING);
>>>           ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_ROUTING_ECMP, 150,
>>>                         REG_ECMP_GROUP_ID" == 0", "next;");
>>>   @@ -11651,6 +11679,7 @@ build_ingress_policy_flows_for_lrouter(
>>>                         REG_ECMP_GROUP_ID" = 0; next;");
>>>           ovn_lflow_add(lflows, od, S_ROUTER_IN_POLICY_ECMP, 150,
>>>                         REG_ECMP_GROUP_ID" == 0", "next;");
>>> +        ovn_lflow_add_default_drop(lflows, od,
>>> S_ROUTER_IN_POLICY_ECMP);
>>>             /* Convert routing policies to flows. */
>>>           uint16_t ecmp_group_id = 1;
>>> @@ -11683,11 +11712,13 @@ build_arp_resolve_flows_for_lrouter(
>>>           ovn_lflow_add(lflows, od, S_ROUTER_IN_ARP_RESOLVE, 500,
>>>                         "ip4.mcast || ip6.mcast", "next;");
>>>   -        ovn_lflow_add(lflows, od, S_ROUTER_IN_ARP_RESOLVE, 0, "ip4",
>>> +        ovn_lflow_add(lflows, od, S_ROUTER_IN_ARP_RESOLVE, 1, "ip4",
>>>                         "get_arp(outport, " REG_NEXT_HOP_IPV4 ");
>>> next;");
>>>   -        ovn_lflow_add(lflows, od, S_ROUTER_IN_ARP_RESOLVE, 0, "ip6",
>>> +        ovn_lflow_add(lflows, od, S_ROUTER_IN_ARP_RESOLVE, 1, "ip6",
>>>                         "get_nd(outport, " REG_NEXT_HOP_IPV6 ");
>>> next;");
>>> +
>>> +        ovn_lflow_add_default_drop(lflows, od,
>>> S_ROUTER_IN_ARP_RESOLVE);
>>>       }
>>>   }
>>>   @@ -11813,9 +11844,9 @@ build_arp_resolve_flows_for_lrouter_port(
>>>            * in stage "lr_in_ip_input" but traffic that could have
>>> been unSNATed
>>>            * but didn't match any existing session might still end up
>>> here.
>>>            *
>>> -         * Priority 1.
>>> +         * Priority 2.
>>>            */
>>> -        build_lrouter_drop_own_dest(op, S_ROUTER_IN_ARP_RESOLVE, 1,
>>> true,
>>> +        build_lrouter_drop_own_dest(op, S_ROUTER_IN_ARP_RESOLVE, 2,
>>> true,
>>>                                       lflows);
>>>       } else if (op->od->n_router_ports && !lsp_is_router(op->nbsp)
>>>                  && strcmp(op->nbsp->type, "virtual")) {
>>> @@ -12417,6 +12448,8 @@ build_egress_delivery_flows_for_lrouter_port(
>>>           ds_put_format(match, "outport == %s", op->json_key);
>>>           ovn_lflow_add(lflows, op->od, S_ROUTER_OUT_DELIVERY, 100,
>>>                         ds_cstr(match), "output;");
>>> +
>>> +        ovn_lflow_add_default_drop(lflows, op->od,
>>> S_ROUTER_OUT_DELIVERY);
>>>       }
>>>     }
>>> @@ -15561,6 +15594,9 @@ ovnnb_db_run(struct northd_input *input_data,
>>>                                                 false);
>>>         build_chassis_features(input_data, &data->features);
>>> +
>>> +    init_debug_config(nb);
>>> +
>>>       build_datapaths(input_data, ovnsb_txn, &data->datapaths,
>>> &data->lr_list);
>>>       build_lbs(input_data, &data->datapaths, &data->lbs,
>>> &data->lb_groups);
>>>       build_ports(input_data, ovnsb_txn, sbrec_chassis_by_name,
>>> diff --git a/ovn-nb.xml b/ovn-nb.xml
>>> index 7fe88af27..813ccf6e6 100644
>>> --- a/ovn-nb.xml
>>> +++ b/ovn-nb.xml
>>> @@ -264,6 +264,14 @@
>>>           </p>
>>>         </column>
>>>   +      <column name="options" key="debug_drop_mode">
>>> +        <p>
>>> +          If set to true, <code>ovn-northd</code> will add an
>>> explicit 'drop'
>>> +          logical flow when possible instead of relying on the OVS
>>> implicitly
>>> +          dropping packets that do not match any flow.
>>> +        </p>
>>> +      </column>
>>> +
>>>         <group title="Options for configuring interconnection route
>>> advertisement">
>>>           <p>
>>>             These options control how routes are advertised between OVN
>>> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
>>> index 7c3c84007..f33609cdb 100644
>>> --- a/tests/ovn-northd.at
>>> +++ b/tests/ovn-northd.at
>>> @@ -7852,3 +7852,78 @@ check_column "" sb:load_balancer datapaths
>>> name=lb0
>>>     AT_CLEANUP
>>>   ])
>>> +
>>> +OVN_FOR_EACH_NORTHD([
>>> +AT_SETUP([Check drop-debug-mode])
>>> +AT_KEYWORDS([debug drop])
>>> +
>>> +check_default_lflow() {
>>> +    dps=$(ovn-sbctl --bare  --columns=_uuid list Datapath_Binding |
>>> xargs)
>>> +    for dp in $dps; do
>>> +        for pipeline in ingress egress; do
>>> +            for table in $(ovn-sbctl --bare --columns=table_id find
>>> Logical_Flow logical_datapath="$dp" pipeline="$pipeline" | xargs |
>>> sort | uniq); do
>>> +               echo "Checking if datapath $dp pipeline $pipeline
>>> table $table has a default action"
>>> +               AT_CHECK([ovn-sbctl --columns=_uuid find Logical_Flow
>>> logical_datapath="$dp" pipeline="$pipeline" table_id=$table match="1"
>>> priority">="0 | wc -l | tr -d "\n\r" ], [0], [1], [ignore],
>>> +               [echo "Datapath $dp pipeline $pipeline table $table
>>> does not have a default action"])
>>> +            done
>>> +        done
>>> +    done
>>> +}
>>> +
>>> +ovn_start
>>> +
>>> +check ovn-nbctl set NB_Global . options:debug_drop_mode="true"
>>> +
>>> +# Create LS + LR
>>> +check ovn-nbctl --wait=sb \
>>> +                -- lr-add R1 \
>>> +                -- lrp-add R1 R1-S1 02:ac:10:01:00:01 172.16.1.1/24 \
>>> +                -- ls-add S1 \
>>> +                -- lsp-add S1 S1-R1 \
>>> +                -- lsp-set-type S1-R1 router \
>>> +                -- lsp-set-addresses S1-R1 02:ac:10:01:00:01 \
>>> +                -- lsp-set-options S1-R1 router-port=R1-S1 \
>>> +                -- lsp-add S1 p1 \
>>> +                -- lsp-set-addresses p1 "02:ac:10:01:00:0a
>>> 172.16.1.100"
>>> +
>>> +check_default_lflow
>>> +
>>> +# Add stateless ACL
>>> +check ovn-nbctl --wait=sb \
>>> +                -- acl-add S1 from-lport 100 'inport=p1 && ip4'
>>> allow-stateless
>>> +
>>> +check_default_lflow
>>> +
>>> +check ovn-nbctl --wait=sb acl-del S1
>>> +
>>> +
>>> +# Add stateful ACL
>>> +check ovn-nbctl --wait=sb \
>>> +                -- acl-add S1 from-lport 2 "udp" allow-related
>>> +
>>> +check_default_lflow
>>> +
>>> +check ovn-nbctl --wait=sb acl-del S1
>>> +
>>> +# Add LB
>>> +check ovn-nbctl --wait=sb \
>>> +    -- lb-add lb "10.0.0.1" "10.0.0.2" \
>>> +    -- ls-lb-add S1 lb
>>> +
>>> +check_default_lflow
>>> +
>>> +# Check LB + stateless ACL
>>> +check ovn-nbctl --wait=sb \
>>> +                -- acl-add S1 from-lport 100 'inport=p1 && ip4'
>>> allow-stateless
>>> +check_default_lflow
>>> +
>>> +check ovn-nbctl --wait=sb acl-del S1
>>> +
>>> +# Check LB + statelful ACL
>>> +check ovn-nbctl --wait=sb \
>>> +                -- acl-add S1 from-lport 2 "udp" allow-related
>>> +
>>> +check_default_lflow
>>> +
>>> +AT_CLEANUP
>>> +])
>>> diff --git a/tests/ovn.at b/tests/ovn.at
>>> index d54bd5a14..1ab7a49d4 100644
>>> --- a/tests/ovn.at
>>> +++ b/tests/ovn.at
>>> @@ -27773,7 +27773,7 @@ AT_CHECK([as hv1 ovs-ofctl dump-flows br-int
>>> | grep "actions=controller" | grep
>>>   ])
>>>     # The packet should've been dropped in the lr_in_arp_resolve stage.
>>> -AT_CHECK([as hv1 ovs-ofctl dump-flows br-int | grep -E "table=23,
>>> n_packets=1,.* priority=1,ip,metadata=0x${sw_key},nw_dst=10.0.1.1
>>> actions=drop" -c], [0], [dnl
>>> +AT_CHECK([as hv1 ovs-ofctl dump-flows br-int | grep -E "table=23,
>>> n_packets=1,.* priority=2,ip,metadata=0x${sw_key},nw_dst=10.0.1.1
>>> actions=drop" -c], [0], [dnl
>>>   1
>>>   ])
>>>   @@ -32914,3 +32914,159 @@ check ovn-nbctl --wait=hv sync
>>>   OVN_CLEANUP([hv1])
>>>   AT_CLEANUP
>>>   ])
>>> +
>>> +OVN_FOR_EACH_NORTHD([
>>> +AT_SETUP([Check drop-debug-mode openflow flows])
>>> +AT_KEYWORDS([debug drop])
>>> +ovn_start
>>> +
>>> +check_default_flows() {
>>> +    ovs-ofctl dump-flows br-int > oflows
>>> +    AT_CAPTURE_FILE([oflows])
>>> +    for table in $(grep -oP "table=\K\d*, " oflows | sort -n |
>>> uniq); do
>>> +        AT_CHECK([grep -qe "table=$table.*
>>> priority=0\(,metadata=0x\w*\)\? actions" oflows], [0], [ignore],
>>> [ignore], [echo "Table $table does not contain a default action"])
>>> +    done
>>> +}
>>> +
>>> +check ovn-nbctl -- set NB_Global . options:debug_drop_mode="true"
>>> +
>>> +# Logical network:
>>> +# Two LRs - R1 and R2 that are connected to each other as peers in
>>> 20.0.0.0/24
>>> +# network. R1 has a switchs ls1 (191.168.1.0/24) connected to it.
>>> +# R2 has ls2 (172.16.1.0/24) connected to it.
>>> +
>>> +ls1_lp1_mac="f0:00:00:01:02:03"
>>> +rp_ls1_mac="00:00:00:01:02:03"
>>> +rp_ls2_mac="00:00:00:01:02:04"
>>> +ls2_lp1_mac="f0:00:00:01:02:04"
>>> +
>>> +ls1_lp1_ip="192.168.1.2"
>>> +ls2_lp1_ip="172.16.1.2"
>>> +
>>> +ovn-nbctl lr-add R1
>>> +ovn-nbctl ls-add ls1
>>> +ovn-nbctl ls-add ls2
>>> +
>>> +# Connect ls1 to R1
>>> +ovn-nbctl lrp-add R1 ls1 $rp_ls1_mac 192.168.1.1/24
>>> +
>>> +ovn-nbctl lsp-add ls1 rp-ls1 -- set Logical_Switch_Port rp-ls1
>>> type=router \
>>> +  options:router-port=ls1 addresses=\"$rp_ls1_mac\"
>>> +
>>> +# Connect ls2 to R1
>>> +ovn-nbctl lrp-add R1 ls2 $rp_ls2_mac 172.16.1.1/24
>>> +
>>> +ovn-nbctl lsp-add ls2 rp-ls2 -- set Logical_Switch_Port rp-ls2
>>> type=router \
>>> +  options:router-port=ls2 addresses=\"$rp_ls2_mac\"
>>> +
>>> +# Create logical port ls1-lp1 in ls1
>>> +ovn-nbctl lsp-add ls1 ls1-lp1 \
>>> +-- lsp-set-addresses ls1-lp1 "$ls1_lp1_mac $ls1_lp1_ip"
>>> +
>>> +# Create logical port ls2-lp1 in ls2
>>> +ovn-nbctl lsp-add ls2 ls2-lp1 \
>>> +-- lsp-set-addresses ls2-lp1 "$ls2_lp1_mac $ls2_lp1_ip"
>>> +
>>> +# Create two hypervisor and create OVS ports corresponding to
>>> logical ports.
>>> +net_add n1
>>> +
>>> +sim_add hv1
>>> +as hv1
>>> +ovs-vsctl add-br br-phys
>>> +ovn_attach n1 br-phys 192.168.0.1
>>> +ovs-vsctl -- add-port br-int hv1-vif1 -- \
>>> +    set interface hv1-vif1 external-ids:iface-id=ls1-lp1 \
>>> +    options:tx_pcap=hv1/vif1-tx.pcap \
>>> +    options:rxq_pcap=hv1/vif1-rx.pcap \
>>> +    ofport-request=1
>>> +
>>> +sim_add hv2
>>> +as hv2
>>> +ovs-vsctl add-br br-phys
>>> +ovn_attach n1 br-phys 192.168.0.2
>>> +ovs-vsctl -- add-port br-int hv2-vif1 -- \
>>> +    set interface hv2-vif1 external-ids:iface-id=ls2-lp1 \
>>> +    options:tx_pcap=hv2/vif1-tx.pcap \
>>> +    options:rxq_pcap=hv2/vif1-rx.pcap \
>>> +    ofport-request=1
>>> +
>>> +
>>> +# Allow some time for ovn-northd and ovn-controller to catch up.
>>> +wait_for_ports_up
>>> +check ovn-nbctl --wait=hv sync
>>> +
>>> +as hv1
>>> +check_default_flows
>>> +as hv2
>>> +check_default_flows
>>> +
>>> +# Add stateless ACL
>>> +check ovn-nbctl --wait=sb \
>>> +                -- acl-add ls1 from-lport 100 'ip4' allow-stateless
>>> +check ovn-nbctl --wait=sb \
>>> +                -- acl-add ls2 from-lport 100 'ip4' allow-stateless
>>> +
>>> +as hv1
>>> +check_default_flows
>>> +as hv2
>>> +check_default_flows
>>> +
>>> +check ovn-nbctl --wait=sb acl-del ls1
>>> +check ovn-nbctl --wait=sb acl-del ls2
>>> +
>>> +# Add stateful ACL
>>> +check ovn-nbctl --wait=sb \
>>> +                -- acl-add ls1 from-lport 100 "udp" allow-related
>>> +check ovn-nbctl --wait=sb \
>>> +                -- acl-add ls2 from-lport 100 "udp" allow-related
>>> +
>>> +as hv1
>>> +check_default_flows
>>> +as hv2
>>> +check_default_flows
>>> +
>>> +check ovn-nbctl --wait=sb acl-del ls1
>>> +check ovn-nbctl --wait=sb acl-del ls2
>>> +
>>> +# Add LB
>>> +check ovn-nbctl --wait=sb \
>>> +    -- lb-add lb1 "10.0.0.1" "10.0.0.2" \
>>> +    -- ls-lb-add ls1 lb1
>>> +
>>> +check ovn-nbctl --wait=sb \
>>> +    -- lb-add lb2 "10.0.1.1" "10.0.1.2" \
>>> +    -- ls-lb-add ls2 lb2
>>> +
>>> +as hv1
>>> +check_default_flows
>>> +as hv2
>>> +check_default_flows
>>> +
>>> +# LB + stateless ACL
>>> +check ovn-nbctl --wait=sb \
>>> +                -- acl-add ls1 from-lport 100 'ip4' allow-stateless
>>> +check ovn-nbctl --wait=sb \
>>> +                -- acl-add ls2 from-lport 100 'ip4' allow-stateless
>>> +
>>> +as hv1
>>> +check_default_flows
>>> +as hv2
>>> +check_default_flows
>>> +
>>> +check ovn-nbctl --wait=sb acl-del ls1
>>> +check ovn-nbctl --wait=sb acl-del ls2
>>> +
>>> +# LB + stateful ACL
>>> +check ovn-nbctl --wait=sb \
>>> +                -- acl-add ls1 from-lport 100 "udp" allow-related
>>> +check ovn-nbctl --wait=sb \
>>> +                -- acl-add ls2 from-lport 100 "udp" allow-related
>>> +
>>> +as hv1
>>> +check_default_flows
>>> +as hv2
>>> +check_default_flows
>>> +
>>> +OVN_CLEANUP([hv1],[hv2])
>>> +AT_CLEANUP
>>> +])
>>
> 

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to