On Wed, Aug 07, 2024 at 11:04:23AM GMT, Dumitru Ceara wrote:
> On 8/2/24 09:22, Adrián Moreno wrote:
> > On Thu, Aug 01, 2024 at 11:11:25AM GMT, Jacob Tanenbaum wrote:
> >> Created a new column in the southbound database to hardcode a human
> >> readable
> >> description for flows. This first use is describing why the flow is
> >> dropping packets.
> >> The new column is called flow_desc and will create southbound database
> >> entries like this
> >>
> >> _uuid : 20f1897b-477e-47ae-a32c-c546d83ec097
> >> actions :
> >> "sample(probability=65535,collector_set=123,obs_domain=1,obs_point=$cookie);
> >> /* drop */"
> >> controller_meter : []
> >> external_ids : {source="northd.c:8721", stage-name=ls_in_l2_unknown}
> >> flow_desc : "No L2 destination"
> >> logical_datapath : []
> >> logical_dp_group : ee3c3db5-98a2-4f34-8a84-409deae140a7
> >> match : "outport == \"none\""
> >> pipeline : ingress
> >> priority : 50
> >> table_id : 27
> >> tags : {}
> >> hash : 0
> >>
> >> future work includes entering more flow_desc for more flows and adding
> >> flow_desc to the actions as a comment.
> >
> > Hi Jacob, apart from some nits below, I have a question: I still see a
>
> Hi Jacob, Adrian,
>
> Some more comments from my side too.
>
> > lot of drops without text representation, is this future work planned
> > for this release?
> >
> >>
> >> Signed-off-by: Jacob Tanenbaum <[email protected]>
> >> Suggested-by: Dumitru Ceara <[email protected]>
> >> Reported-at: https://issues.redhat.com/browse/FDP-307
> >> Acked-by: Ales Musil <[email protected]>
> >>
> >
> >
> >> ---
> >>
> >> v1: initial version
> >> v2: correct commit message
> >> make the flow_desc a char*
> >> correct a typo in the ovn-sb.xml
> >> correct the test
> >> v3: rebase issue with NEWS file
> >> v4: remove options:debug_drop_domain_id="1" from testing as we
> >> do not depend on it
> >> v5: introduce string wrapper
> >> increment ovs-sb.ovsschema version
> >> correct the testing
> >> added descriptions to more dropped packets
> >> v6: v5 was not the correct branch, this is...
> >> added descriptions to more dropped packets
> >> changed the names of the macros to be more descriptive
> >> v7: rebased, corrected a few typos, updated the ovs-sb.ovsschema
> >> checksum, and added Ales ACK
> >> ---
> >> NEWS | 2 ++
> >> lib/ovn-util.h | 26 +++++++++++++++++++++++
> >> northd/lflow-mgr.c | 25 +++++++++++++++--------
> >> northd/lflow-mgr.h | 27 +++++++++++++++++++-----
> >> northd/northd.c | 50 ++++++++++++++++++++++++---------------------
> >> ovn-sb.ovsschema | 10 +++++----
> >> ovn-sb.xml | 5 +++++
> >> tests/ovn-northd.at | 15 ++++++++++++++
> >> 8 files changed, 120 insertions(+), 40 deletions(-)
> >>
> >> diff --git a/NEWS b/NEWS
> >> index 136f890f5..246cc9cfd 100644
> >> --- a/NEWS
> >> +++ b/NEWS
> >> @@ -50,6 +50,8 @@ Post v24.03.0
> >> - A new LSP option "disable_garp_rarp" has been added to prevent OVN
> >> from
> >> sending GARP or RARP announcements when a VIF is created on a bridged
> >> logical switch.
> >> + - Added a new column in the southbound database "flow_desc" to provide
> >> + human readable context to flows.
> >>
> >> OVN v24.03.0 - 01 Mar 2024
> >> --------------------------
> >> diff --git a/lib/ovn-util.h b/lib/ovn-util.h
> >> index ae971ce5a..02bf92e62 100644
> >> --- a/lib/ovn-util.h
> >> +++ b/lib/ovn-util.h
> >> @@ -469,6 +469,32 @@ void sorted_array_apply_diff(const struct
> >> sorted_array *a1,
> >> bool add),
> >> const void *arg);
> >>
> >> +/* A wrapper that holds strings */
> >> +struct string_wrapper
> >> +{
>
> The curly brace should be on the previous line.
>
> >> + char *str;
> >> + bool owns_string;
> >> +};
> >> +
> >> +#define EMPTY_STRING_WRAPPER (struct string_wrapper){NULL, false}
>
> Please add a space before the curly brace.
>
> Also, following the (undocumented?) convention from OVS maybe this
> should be:
>
> #define EMPTY_STRING_WRAPPER_INITIALIZER \
> (struct string_wrapper) { NULL, false }
>
> However, I guess this was added based on the discussion in v4:
> https://patchwork.ozlabs.org/project/ovn/patch/[email protected]/#3323815
>
> I'm not sure what extra benefit we get from the new 'struct
> string_wrapper' that we don't already have with OVS' dynamic strings.
> 'struct ds' supports ds_steal_cstr() which essentially changes ownership
> of the internal string.
>
> Maybe it's good if for now if we just pass raw 'const char *' and if we
> ever need to make this dynamic in the future we can consider the change
> then. Sorry for the back and forth on this one..
>
> What do you think of the following incremental (I didn't address any of
> the other review comments):
>
> https://github.com/dceara/ovn/commit/20b2b0261a9241dbd6a254aae180bd7ee6e71da1
+1 for using "struct ds" if we need dynamic strings and "const char *"
if we don't (or until we do).
>
> Regards,
> Dumitru
>
> >> +
> >> +static inline struct string_wrapper
> >> +string_wrapper_create(char *str, bool take_ownership)
> >> +{
> >> + return (struct string_wrapper) {
> >> + .str = str,
> >> + .owns_string = take_ownership,
> >> + };
> >> +}
> >> +
> >> +static inline void
> >> +string_wrapper_destroy(struct string_wrapper *s)
> >> +{
> >> + if (s->owns_string) {
> >> + free(s->str);
> >> + }
> >> +}
> >> +
> >> /* Utilities around properly handling exit command. */
> >> struct ovn_exit_args {
> >> struct unixctl_conn **conns;
> >> diff --git a/northd/lflow-mgr.c b/northd/lflow-mgr.c
> >> index b2c60b5de..6157d4dfe 100644
> >> --- a/northd/lflow-mgr.c
> >> +++ b/northd/lflow-mgr.c
> >> @@ -25,6 +25,7 @@
> >> #include "debug.h"
> >> #include "lflow-mgr.h"
> >> #include "lib/ovn-parallel-hmap.h"
> >> +#include "lib/ovn-util.h"
> >>
> >> VLOG_DEFINE_THIS_MODULE(lflow_mgr);
> >>
> >> @@ -36,7 +37,7 @@ static void ovn_lflow_init(struct ovn_lflow *, struct
> >> ovn_datapath *od,
> >> uint16_t priority, char *match,
> >> char *actions, char *io_port,
> >> char *ctrl_meter, char *stage_hint,
> >> - const char *where);
> >> + const char *where, struct string_wrapper
> >> flow_desc);
> >> static struct ovn_lflow *ovn_lflow_find(const struct hmap *lflows,
> >> enum ovn_stage stage,
> >> uint16_t priority, const char
> >> *match,
> >> @@ -52,7 +53,7 @@ static struct ovn_lflow *do_ovn_lflow_add(
> >> const char *actions, const char *io_port,
> >> const char *ctrl_meter,
> >> const struct ovsdb_idl_row *stage_hint,
> >> - const char *where);
> >> + const char *where, struct string_wrapper flow_desc);
> >>
> >>
> >> static struct ovs_mutex *lflow_hash_lock(const struct hmap *lflow_table,
> >> @@ -173,6 +174,7 @@ struct ovn_lflow {
> >> * 'dpg_bitmap'. */
> >> struct ovn_dp_group *dpg; /* Link to unique Sb datapath group. */
> >> const char *where;
> >> + struct string_wrapper flow_desc;
> >>
> >> struct uuid sb_uuid; /* SB DB row uuid, specified by northd.
> >> */
> >> struct ovs_list referenced_by; /* List of struct lflow_ref_node. */
> >> @@ -659,7 +661,7 @@ lflow_table_add_lflow(struct lflow_table *lflow_table,
> >> const char *match, const char *actions,
> >> const char *io_port, const char *ctrl_meter,
> >> const struct ovsdb_idl_row *stage_hint,
> >> - const char *where,
> >> + const char *where, struct string_wrapper flow_desc,
> >> struct lflow_ref *lflow_ref)
> >> OVS_EXCLUDED(fake_hash_mutex)
> >> {
> >> @@ -679,7 +681,7 @@ lflow_table_add_lflow(struct lflow_table *lflow_table,
> >> do_ovn_lflow_add(lflow_table,
> >> od ? ods_size(od->datapaths) : dp_bitmap_len,
> >> hash, stage, priority, match, actions,
> >> - io_port, ctrl_meter, stage_hint, where);
> >> + io_port, ctrl_meter, stage_hint, where,
> >> flow_desc);
> >>
> >> if (lflow_ref) {
> >> struct lflow_ref_node *lrn =
> >> @@ -733,7 +735,7 @@ lflow_table_add_lflow_default_drop(struct lflow_table
> >> *lflow_table,
> >> {
> >> lflow_table_add_lflow(lflow_table, od, NULL, 0, stage, 0, "1",
> >> debug_drop_action(), NULL, NULL, NULL,
> >> - where, lflow_ref);
> >> + where, EMPTY_STRING_WRAPPER, lflow_ref);
> >> }
> >>
> >> struct ovn_dp_group *
> >> @@ -856,7 +858,8 @@ static void
> >> ovn_lflow_init(struct ovn_lflow *lflow, struct ovn_datapath *od,
> >> size_t dp_bitmap_len, enum ovn_stage stage, uint16_t
> >> priority,
> >> char *match, char *actions, char *io_port, char
> >> *ctrl_meter,
> >> - char *stage_hint, const char *where)
> >> + char *stage_hint, const char *where,
> >> + struct string_wrapper flow_desc)
> >> {
> >> lflow->dpg_bitmap = bitmap_allocate(dp_bitmap_len);
> >> lflow->od = od;
> >> @@ -867,6 +870,7 @@ ovn_lflow_init(struct ovn_lflow *lflow, struct
> >> ovn_datapath *od,
> >> lflow->io_port = io_port;
> >> lflow->stage_hint = stage_hint;
> >> lflow->ctrl_meter = ctrl_meter;
> >> + lflow->flow_desc = flow_desc;
> >> lflow->dpg = NULL;
> >> lflow->where = where;
> >> lflow->sb_uuid = UUID_ZERO;
> >> @@ -946,6 +950,7 @@ ovn_lflow_destroy(struct lflow_table *lflow_table,
> >> struct ovn_lflow *lflow)
> >> free(lflow->io_port);
> >> free(lflow->stage_hint);
> >> free(lflow->ctrl_meter);
> >> + string_wrapper_destroy(&lflow->flow_desc);
> >> ovn_lflow_clear_dp_refcnts_map(lflow);
> >> struct lflow_ref_node *lrn;
> >> LIST_FOR_EACH_SAFE (lrn, ref_list_node, &lflow->referenced_by) {
> >> @@ -960,7 +965,7 @@ do_ovn_lflow_add(struct lflow_table *lflow_table,
> >> size_t dp_bitmap_len,
> >> const char *match, const char *actions,
> >> const char *io_port, const char *ctrl_meter,
> >> const struct ovsdb_idl_row *stage_hint,
> >> - const char *where)
> >> + const char *where, struct string_wrapper flow_desc)
> >> OVS_REQUIRES(fake_hash_mutex)
> >> {
> >> struct ovn_lflow *old_lflow;
> >> @@ -982,7 +987,8 @@ do_ovn_lflow_add(struct lflow_table *lflow_table,
> >> size_t dp_bitmap_len,
> >> xstrdup(match), xstrdup(actions),
> >> io_port ? xstrdup(io_port) : NULL,
> >> nullable_xstrdup(ctrl_meter),
> >> - ovn_lflow_hint(stage_hint), where);
> >> + ovn_lflow_hint(stage_hint), where,
> >> + flow_desc);
> >>
> >> if (parallelization_state != STATE_USE_PARALLELIZATION) {
> >> hmap_insert(&lflow_table->entries, &lflow->hmap_node, hash);
> >> @@ -1050,6 +1056,9 @@ sync_lflow_to_sb(struct ovn_lflow *lflow,
> >> sbrec_logical_flow_set_priority(sbflow, lflow->priority);
> >> sbrec_logical_flow_set_match(sbflow, lflow->match);
> >> sbrec_logical_flow_set_actions(sbflow, lflow->actions);
> >> + if (lflow->flow_desc.str) {
> >> + sbrec_logical_flow_set_flow_desc(sbflow,
> >> lflow->flow_desc.str);
> >> + }
> >> if (lflow->io_port) {
> >> struct smap tags = SMAP_INITIALIZER(&tags);
> >> smap_add(&tags, "in_out_port", lflow->io_port);
> >> diff --git a/northd/lflow-mgr.h b/northd/lflow-mgr.h
> >> index 83b087f47..56cda8507 100644
> >> --- a/northd/lflow-mgr.h
> >> +++ b/northd/lflow-mgr.h
> >> @@ -78,7 +78,8 @@ void lflow_table_add_lflow(struct lflow_table *, const
> >> struct ovn_datapath *,
> >> const char *actions, const char *io_port,
> >> const char *ctrl_meter,
> >> const struct ovsdb_idl_row *stage_hint,
> >> - const char *where, struct lflow_ref *);
> >> + const char *where, struct string_wrapper
> >> flow_desc,
> >> + struct lflow_ref *);
> >> void lflow_table_add_lflow_default_drop(struct lflow_table *,
> >> const struct ovn_datapath *,
> >> enum ovn_stage stage,
> >> @@ -91,20 +92,20 @@ void lflow_table_add_lflow_default_drop(struct
> >> lflow_table *,
> >> STAGE_HINT, LFLOW_REF) \
> >> lflow_table_add_lflow(LFLOW_TABLE, OD, NULL, 0, STAGE, PRIORITY,
> >> MATCH, \
> >> ACTIONS, IN_OUT_PORT, CTRL_METER, STAGE_HINT, \
> >> - OVS_SOURCE_LOCATOR, LFLOW_REF)
> >> + OVS_SOURCE_LOCATOR, EMPTY_STRING_WRAPPER,
> >> LFLOW_REF)
> >>
> >> #define ovn_lflow_add_with_hint(LFLOW_TABLE, OD, STAGE, PRIORITY, MATCH, \
> >> ACTIONS, STAGE_HINT, LFLOW_REF) \
> >> lflow_table_add_lflow(LFLOW_TABLE, OD, NULL, 0, STAGE, PRIORITY,
> >> MATCH, \
> >> ACTIONS, NULL, NULL, STAGE_HINT, \
> >> - OVS_SOURCE_LOCATOR, LFLOW_REF)
> >> + OVS_SOURCE_LOCATOR, EMPTY_STRING_WRAPPER,
> >> LFLOW_REF)
> >>
> >> #define ovn_lflow_add_with_dp_group(LFLOW_TABLE, DP_BITMAP,
> >> DP_BITMAP_LEN, \
> >> STAGE, PRIORITY, MATCH, ACTIONS, \
> >> STAGE_HINT, LFLOW_REF) \
> >> lflow_table_add_lflow(LFLOW_TABLE, NULL, DP_BITMAP, DP_BITMAP_LEN,
> >> STAGE, \
> >> PRIORITY, MATCH, ACTIONS, NULL, NULL,
> >> STAGE_HINT, \
> >> - OVS_SOURCE_LOCATOR, LFLOW_REF)
> >> + OVS_SOURCE_LOCATOR, EMPTY_STRING_WRAPPER,
> >> LFLOW_REF)
> >>
> >> #define ovn_lflow_add_default_drop(LFLOW_TABLE, OD, STAGE, LFLOW_REF) \
> >> lflow_table_add_lflow_default_drop(LFLOW_TABLE, OD, STAGE, \
> >> @@ -126,12 +127,28 @@ void lflow_table_add_lflow_default_drop(struct
> >> lflow_table *,
> >> STAGE_HINT, LFLOW_REF) \
> >> lflow_table_add_lflow(LFLOW_TABLE, OD, NULL, 0, STAGE, PRIORITY,
> >> MATCH, \
> >> ACTIONS, IN_OUT_PORT, NULL, STAGE_HINT, \
> >> - OVS_SOURCE_LOCATOR, LFLOW_REF)
> >> + OVS_SOURCE_LOCATOR, EMPTY_STRING_WRAPPER,
> >> LFLOW_REF)
> >>
> >> #define ovn_lflow_add(LFLOW_TABLE, OD, STAGE, PRIORITY, MATCH, ACTIONS, \
> >> LFLOW_REF) \
> >> lflow_table_add_lflow(LFLOW_TABLE, OD, NULL, 0, STAGE, PRIORITY,
> >> MATCH, \
> >> ACTIONS, NULL, NULL, NULL, OVS_SOURCE_LOCATOR, \
> >> + EMPTY_STRING_WRAPPER, LFLOW_REF)
> >> +
> >> +#define ovn_lflow_add_drop_with_desc(LFLOW_TABLE, OD, STAGE, PRIORITY,
> >> MATCH, \
> >> + DESCRIPTION, LFLOW_REF) \
> >> + lflow_table_add_lflow(LFLOW_TABLE, OD, NULL, 0, STAGE, PRIORITY,
> >> MATCH, \
> >> + debug_drop_action(), NULL, NULL, NULL, \
> >> + OVS_SOURCE_LOCATOR, \
> >> + string_wrapper_create(DESCRIPTION, false),
> >> LFLOW_REF)
> >> +
> >> +#define ovn_lflow_add_drop_with_lport_hint_and_desc(LFLOW_TABLE, OD,
> >> STAGE, \
> >> + PRIORITY, MATCH, IN_OUT_PORT, \
> >> + STAGE_HINT, DESCRIPTION,
> >> LFLOW_REF) \
> >> + lflow_table_add_lflow(LFLOW_TABLE, OD, NULL, 0, STAGE, PRIORITY,
> >> MATCH, \
> >> + debug_drop_action(), IN_OUT_PORT, NULL,
> >> STAGE_HINT, \
> >> + OVS_SOURCE_LOCATOR, \
> >> + string_wrapper_create(DESCRIPTION, false), \
> >> LFLOW_REF)
> >>
> >> #define ovn_lflow_metered(LFLOW_TABLE, OD, STAGE, PRIORITY, MATCH,
> >> ACTIONS, \
> >> diff --git a/northd/northd.c b/northd/northd.c
> >> index a8a0b6f94..1cbb38fa6 100644
> >> --- a/northd/northd.c
> >> +++ b/northd/northd.c
> >> @@ -5883,9 +5883,9 @@ build_lswitch_output_port_sec_od(struct ovn_datapath
> >> *od,
> >> REGBIT_PORT_SEC_DROP" = check_out_port_sec(); next;",
> >> lflow_ref);
> >>
> >> - ovn_lflow_add(lflows, od, S_SWITCH_OUT_APPLY_PORT_SEC, 50,
> >> - REGBIT_PORT_SEC_DROP" == 1", debug_drop_action(),
> >> - lflow_ref);
> >> + ovn_lflow_add_drop_with_desc(lflows, od, S_SWITCH_OUT_APPLY_PORT_SEC,
> >> 50,
> >> + REGBIT_PORT_SEC_DROP" == 1",
> >> + "Packet does not follow port security rules",
> >> lflow_ref);
> >> ovn_lflow_add(lflows, od, S_SWITCH_OUT_APPLY_PORT_SEC, 0,
> >> "1", "output;", lflow_ref);
> >> }
> >> @@ -8672,10 +8672,11 @@
> >> build_drop_arp_nd_flows_for_unbound_router_ports(struct ovn_port *op,
> >> port->json_key,
> >> op->lsp_addrs[i].ea_s, op->json_key,
> >> rp->lsp_addrs[k].ipv4_addrs[l].addr_s);
> >> - ovn_lflow_add_with_lport_and_hint(
> >> + ovn_lflow_add_drop_with_lport_hint_and_desc(
> >> lflows, op->od, S_SWITCH_IN_EXTERNAL_PORT, 100,
> >> - ds_cstr(&match), debug_drop_action(), port->key,
> >> - &op->nbsp->header_, lflow_ref);
> >> + ds_cstr(&match), port->key,
> >> + &op->nbsp->header_,
> >> + "Drop arp for unknown router ports", lflow_ref);
> >
> > nit: I'd write protocols in capital leters: ARP.
> >
> >> }
> >> for (size_t l = 0; l < rp->lsp_addrs[k].n_ipv6_addrs;
> >> l++) {
> >> ds_clear(&match);
> >> @@ -8688,10 +8689,11 @@
> >> build_drop_arp_nd_flows_for_unbound_router_ports(struct ovn_port *op,
> >> rp->lsp_addrs[k].ipv6_addrs[l].addr_s,
> >> rp->lsp_addrs[k].ipv6_addrs[l].sn_addr_s,
> >> rp->lsp_addrs[k].ipv6_addrs[l].addr_s);
> >> - ovn_lflow_add_with_lport_and_hint(
> >> + ovn_lflow_add_drop_with_lport_hint_and_desc(
> >> lflows, op->od, S_SWITCH_IN_EXTERNAL_PORT, 100,
> >> - ds_cstr(&match), debug_drop_action(), port->key,
> >> - &op->nbsp->header_, lflow_ref);
> >> + ds_cstr(&match), port->key,
> >> + &op->nbsp->header_, "Drop ND for unbound router
> >> ports",
> >> + lflow_ref);
> >> }
> >>
> >> ds_clear(&match);
> >> @@ -8702,12 +8704,13 @@
> >> build_drop_arp_nd_flows_for_unbound_router_ports(struct ovn_port *op,
> >> port->json_key,
> >> op->lsp_addrs[i].ea_s, rp->lsp_addrs[k].ea_s,
> >> op->json_key);
> >> - ovn_lflow_add_with_lport_and_hint(lflows, op->od,
> >> + ovn_lflow_add_drop_with_lport_hint_and_desc(lflows,
> >> op->od,
> >>
> >> S_SWITCH_IN_EXTERNAL_PORT,
> >> 100, ds_cstr(&match),
> >> - debug_drop_action(),
> >> port->key,
> >> &op->nbsp->header_,
> >> + "Packet does not come
> >> from" \
> >> + " a chassis resident",
> >> lflow_ref);
> >> }
> >> }
> >> @@ -8733,8 +8736,9 @@ build_lswitch_lflows_l2_unknown(struct ovn_datapath
> >> *od,
> >> "outport = \""MC_UNKNOWN "\"; output;",
> >> lflow_ref);
> >> } else {
> >> - ovn_lflow_add(lflows, od, S_SWITCH_IN_L2_UNKNOWN, 50,
> >> - "outport == \"none\"", debug_drop_action(),
> >> + ovn_lflow_add_drop_with_desc(lflows, od, S_SWITCH_IN_L2_UNKNOWN,
> >> 50,
> >> + "outport == \"none\"",
> >> + "No L2 destination",
> >> lflow_ref);
> >> }
> >> ovn_lflow_add(lflows, od, S_SWITCH_IN_L2_UNKNOWN, 0, "1",
> >> @@ -8769,31 +8773,31 @@ build_lswitch_lflows_admission_control(struct
> >> ovn_datapath *od,
> >> ovs_assert(od->nbs);
> >>
> >> /* Default action for recirculated ICMP error 'packet too big'. */
> >> - ovn_lflow_add(lflows, od, S_SWITCH_IN_CHECK_PORT_SEC, 105,
> >> + ovn_lflow_add_drop_with_desc(lflows, od, S_SWITCH_IN_CHECK_PORT_SEC,
> >> 105,
> >> "((ip4 && icmp4.type == 3 && icmp4.code == 4) ||"
> >> " (ip6 && icmp6.type == 2 && icmp6.code == 0)) &&"
> >> - " flags.tunnel_rx == 1", debug_drop_action(),
> >> lflow_ref);
> >> + " flags.tunnel_rx == 1", "ICMP: packet too big",
> >> lflow_ref);
> >>
> >> /* Logical VLANs not supported. */
> >> if (!is_vlan_transparent(od)) {
> >> /* Block logical VLANs. */
> >> - ovn_lflow_add(lflows, od, S_SWITCH_IN_CHECK_PORT_SEC, 100,
> >> - "vlan.present", debug_drop_action(),
> >> + ovn_lflow_add_drop_with_desc(lflows, od,
> >> S_SWITCH_IN_CHECK_PORT_SEC,
> >> + 100, "vlan.present", "VLANs blocked",
> >
> > nit: For someone not knowing much about OVN internals (such as myself),
> > adding why VLANs were blocked would be very useful, e.g: "Incoming VLAN
> > traffic
> > dropped due to 'vlan-pasthru' option".
> >
> >> lflow_ref);
> >> }
> >>
> >> /* Broadcast/multicast source address is invalid. */
> >> - ovn_lflow_add(lflows, od, S_SWITCH_IN_CHECK_PORT_SEC, 100,
> >> - "eth.src[40]", debug_drop_action(),
> >> - lflow_ref);
> >> + ovn_lflow_add_drop_with_desc(lflows, od, S_SWITCH_IN_CHECK_PORT_SEC,
> >> 100,
> >> + "eth.src[40]", "incoming Broadcast/multicast source" \
> >> + " address is invalid", lflow_ref);
> >
> > Other debug lines start with a capital letter: "Incoming"?
> >
> >>
> >> ovn_lflow_add(lflows, od, S_SWITCH_IN_CHECK_PORT_SEC, 50, "1",
> >> REGBIT_PORT_SEC_DROP" = check_in_port_sec(); next;",
> >> lflow_ref);
> >>
> >> - ovn_lflow_add(lflows, od, S_SWITCH_IN_APPLY_PORT_SEC, 50,
> >> - REGBIT_PORT_SEC_DROP" == 1", debug_drop_action(),
> >> - lflow_ref);
> >> + ovn_lflow_add_drop_with_desc(lflows, od, S_SWITCH_IN_APPLY_PORT_SEC,
> >> 50,
> >> + REGBIT_PORT_SEC_DROP" == 1",
> >> + "Broadcast/multicast port security invalid", lflow_ref);
> >>
> >> ovn_lflow_add(lflows, od, S_SWITCH_IN_APPLY_PORT_SEC, 0, "1", "next;",
> >> lflow_ref);
> >> diff --git a/ovn-sb.ovsschema b/ovn-sb.ovsschema
> >> index b6c051ae6..7d4d024d6 100644
> >> --- a/ovn-sb.ovsschema
> >> +++ b/ovn-sb.ovsschema
> >> @@ -1,7 +1,7 @@
> >> {
> >> "name": "OVN_Southbound",
> >> - "version": "20.34.0",
> >> - "cksum": "2786607656 31376",
> >> + "version": "20.35.0",
> >> + "cksum": "3665804961 31485",
> >> "tables": {
> >> "SB_Global": {
> >> "columns": {
> >> @@ -113,10 +113,12 @@
> >> "type": {"key": "string", "value": "string",
> >> "min": 0, "max": "unlimited"}},
> >> "controller_meter": {"type": {"key": {"type": "string"},
> >> - "min": 0, "max": 1}},
> >> + "min": 0, "max": 1}},
> >> "external_ids": {
> >> "type": {"key": "string", "value": "string",
> >> - "min": 0, "max": "unlimited"}}},
> >> + "min": 0, "max": "unlimited"}},
> >> + "flow_desc": {"type": {"key": {"type": "string"},
> >> + "min": 0, "max": 1}}},
> >> "isRoot": true},
> >> "Logical_DP_Group": {
> >> "columns": {
> >> diff --git a/ovn-sb.xml b/ovn-sb.xml
> >> index 90f113afd..96a5e28d0 100644
> >> --- a/ovn-sb.xml
> >> +++ b/ovn-sb.xml
> >> @@ -2894,6 +2894,11 @@ tcp.flags = RST;
> >> <code>ovn-controller</code>.
> >> </column>
> >>
> >> + <column name="flow_desc">
> >> + Human-readable explanation of the flow, this is optional and used
> >> + to provide context for the given flow.
> >> + </column>
> >> +
> >> <column name="external_ids" key="stage-name">
> >> Human-readable name for this flow's stage in the pipeline.
> >> </column>
> >> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> >> index 199197f09..f32f3eb89 100644
> >> --- a/tests/ovn-northd.at
> >> +++ b/tests/ovn-northd.at
> >> @@ -12256,6 +12256,21 @@ AT_CHECK([grep -e "DHCP_RELAY_" lflows | sed
> >> 's/table=../table=??/'], [0], [dnl
> >> AT_CLEANUP
> >> ])
> >>
> >> +OVN_FOR_EACH_NORTHD_NO_HV([
> >> +AT_SETUP([check for flow_desc])
> >> +ovn_start
> >> +
> >> +check ovn-nbctl -- set NB_Global . options:debug_drop_collector_set="123"
> >> +check ovn-nbctl ls-add ls1
> >> +
> >> +check ovn-nbctl --wait=hv sync
> >> +
> >> +flow_desc=$(fetch_column Logical_flow flow_desc match='"outport ==
> >> \"none\""')
> >> +AT_CHECK([test "$flow_desc" != ""])
> >> +
> >> +AT_CLEANUP
> >> +])
> >> +
> >> AT_SETUP([NB_Global and SB_Global incremental processing])
> >>
> >> ovn_start
> >> --
> >> 2.45.2
> >>
> >> _______________________________________________
> >> dev mailing list
> >> [email protected]
> >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >>
> >
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
