> On 21 Oct 2022, at 10:58, Dumitru Ceara <dce...@redhat.com> wrote:
> 
> On 10/20/22 17:34, Abhiram Sangana wrote:
>> Hi Dumitru,
>> 
>> Can you please check if the implementation for the proposal looks ok?
>> Will send out v1 with the review comments and tests.
>> Also, any ideas how we can selectively create new drop zones for ports.
>> Currently, I am assigning a new zone for dropped connections to every
>> port even if its parent LS doesn’t have drop ACLs with labels.
>> 
> 
> Within a logical switch do we really need a drop zone per port?  Isn't
> it actually enough if we add a "from-lport-drop" and "to-lport-drop"
> zone for the whole logical switch?  That should simplify zone allocation.
> 
Given that we are committing dropped connections to CT table, a DDOS
attack can potentially fill up the CT table. We are planning to send
another patch that limits the number of entries for a given CT zone.
It is easier to manage the size of CT zones when there is a CT zone
per port rather than per Logical_Switch.

>> Thanks,
>> Abhiram Sangana
>> 
>>> On 20 Oct 2022, at 15:18, Dumitru Ceara <dce...@redhat.com> wrote:
>>> 
>>> On 10/20/22 15:49, Abhiram Sangana wrote:
>>>> Hi Dumitru,
>>>> 
>>>> Thanks for reviewing the patch.
>>>> 
>>>>> On 19 Oct 2022, at 14:09, Dumitru Ceara <dce...@redhat.com> wrote:
>>>>> 
>>>>> Hi Abhiram,
>>>>> 
>>>>> Thanks for the patch!  I only skimmed the changes so this is not a full
>>>>> review but more of a discussion starter.
>>>>> 
>>>>> On 10/18/22 17:33, Abhiram Sangana wrote:
>>>>>> To identify connections dropped by ACLs, users can enable logging for 
>>>>>> ACLs
>>>>>> but this approach does not scale. ACL logging uses "controller" action
>>>>>> which causes a significant spike in the CPU usage of ovs-vswitchd (and
>>>>>> ovn-controller to a lesser extent) even with metering enabled (observed
>>>>>> 65% ovs-vswitchd CPU usage for logging 1000 packets per second). Another
>>>>>> approach is to use drop sampling (patch by Adrian Moreno currently in
>>>>>> review) but we might miss specific connections of interest with this
>>>>>> approach.
>>>>>> 
>>>>>> This patch commits connections dropped by ACLs to the connection tracking
>>>>>> table with a specific ACL label that was introduced in 0e0228be (
>>>>>> northd: Add ACL label). The dropped connections are committed in a 
>>>>>> separate
>>>>>> CT zone so that they can be managed independently.
>>>>>> 
>>>>> 
>>>>> I'm not sure I understand how the CMS can manage this.  How is this
>>>>> better than sampling?  Committed connections are going to time out at
>>>>> some point (30 sec by default for udp/icmp with the kernel datapath).
>>>>> So the CMS will have to continuously monitor the contents of the
>>>>> conntrack zone?  Aren't we just moving the CPU load somewhere else with
>>>>> this?  Even so, there's a chance an entry is missed.
>>>> 
>>>> Linux nf_conntrack module supports sending connection tracking events
>>>> to userspace via ctnetlink ("net.netfilter.nf_conntrack_events" kernel
>>>> parameter). So, CMS can parse the stream of new connection events from
>>>> conntrack and log the packets based on CT label.
>>>> 
>>> 
>>> Ack.
>>> 
>>>> An issue with sampling is that if there are a large number of packets
>>>> for a particular connection(s), packets of other connections might not
>>>> get sampled and we miss information about these connections. With the
>>>> conntrack approach, we get a single event for each connection (until
>>>> they time out), so there is lesser load on the CMS/collector and lesser
>>>> likelihood of missing connections.
>>>> 
>>> 
>>> I hadn't considered this advantage, thanks for pointing it out!
>>> 
>>>>> 
>>>>>> Each logical port is assigned a new zone for committing dropped flows.
>>>>>> The zone is loaded into register MFF_LOG_ACL_DROP_ZONE.
>>>>>> 
>>>>>> A new lflow action "ct_commit_drop" is introduced that commits flows
>>>>>> to connection tracking table in a zone identified by
>>>>>> MFF_LOG_ACL_DROP_ZONE register.
>>>>>> 
>>>>>> An ACL with "drop" action and non-empty label is translated to 
>>>>>> "ct_commit_drop"
>>>>>> instead of silently dropping the packet.
>>>>>> 
>>>>>> Signed-off-by: Abhiram Sangana <sangana.abhi...@nutanix.com>
>>>>>> ---
>>>>>> controller/ovn-controller.c  | 23 ++++++++++++++---
>>>>>> controller/physical.c        | 32 +++++++++++++++++++++--
>>>>>> include/ovn/actions.h        |  1 +
>>>>>> include/ovn/logical-fields.h |  1 +
>>>>>> lib/actions.c                | 50 ++++++++++++++++++++++++++++++++++++
>>>>>> lib/ovn-util.c               |  4 +--
>>>>>> lib/ovn-util.h               |  2 +-
>>>>>> northd/northd.c              | 14 ++++++++--
>>>>>> northd/ovn-northd.8.xml      | 14 ++++++++--
>>>>>> ovn-sb.xml                   | 17 ++++++++++++
>>>>>> ovs                          |  2 +-
>>>>> 
>>>>> This shouldn't need to change the OVS submodule.
>>>>> 
>>>> My bad. Will fix this.
>>>> 
>>> 
>>> Cool, thanks, looking forward for v1!
>>> 
>>> Regards,
>>> Dumitru
>>> 
>>>> Thanks,
>>>> Abhiram Sangana
>>>> 
>>>>> Thanks,
>>>>> Dumitru
>>>>> 
>>>>>> utilities/ovn-nbctl.c        |  7 ++---
>>>>>> 12 files changed, 151 insertions(+), 16 deletions(-)
>>>>>> 
>>>>>> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
>>>>>> index c97744d57..1ad20fe55 100644
>>>>>> --- a/controller/ovn-controller.c
>>>>>> +++ b/controller/ovn-controller.c
>>>>>> @@ -660,8 +660,15 @@ update_ct_zones(const struct shash *binding_lports,
>>>>>>   unsigned long unreq_snat_zones[BITMAP_N_LONGS(MAX_CT_ZONES)];
>>>>>> 
>>>>>>   struct shash_node *shash_node;
>>>>>> +    const struct binding_lport *lport;
>>>>>>   SHASH_FOR_EACH (shash_node, binding_lports) {
>>>>>>       sset_add(&all_users, shash_node->name);
>>>>>> +
>>>>>> +        /* Zone for committing dropped connections of a vNIC. */
>>>>>> +        lport = shash_node->data;
>>>>>> +        char *drop_zone = alloc_ct_zone_key(&lport->pb->header_.uuid, 
>>>>>> "drop");
>>>>>> +        sset_add(&all_users, drop_zone);
>>>>>> +        free(drop_zone);
>>>>>>   }
>>>>>> 
>>>>>>   /* Local patched datapath (gateway routers) need zones assigned. */
>>>>>> @@ -670,8 +677,8 @@ update_ct_zones(const struct shash *binding_lports,
>>>>>>   HMAP_FOR_EACH (ld, hmap_node, local_datapaths) {
>>>>>>       /* XXX Add method to limit zone assignment to logical router
>>>>>>        * datapaths with NAT */
>>>>>> -        char *dnat = alloc_nat_zone_key(&ld->datapath->header_.uuid, 
>>>>>> "dnat");
>>>>>> -        char *snat = alloc_nat_zone_key(&ld->datapath->header_.uuid, 
>>>>>> "snat");
>>>>>> +        char *dnat = alloc_ct_zone_key(&ld->datapath->header_.uuid, 
>>>>>> "dnat");
>>>>>> +        char *snat = alloc_ct_zone_key(&ld->datapath->header_.uuid, 
>>>>>> "snat");
>>>>>>       sset_add(&all_users, dnat);
>>>>>>       sset_add(&all_users, snat);
>>>>>>       shash_add(&all_lds, dnat, ld);
>>>>>> @@ -2090,7 +2097,7 @@ ct_zones_datapath_binding_handler(struct 
>>>>>> engine_node *node, void *data)
>>>>>>       /* Check if the requested snat zone has changed for the datapath
>>>>>>        * or not.  If so, then fall back to full recompute of
>>>>>>        * ct_zone engine. */
>>>>>> -        char *snat_dp_zone_key = alloc_nat_zone_key(&dp->header_.uuid, 
>>>>>> "snat");
>>>>>> +        char *snat_dp_zone_key = alloc_ct_zone_key(&dp->header_.uuid, 
>>>>>> "snat");
>>>>>>       struct simap_node *simap_node = simap_find(&ct_zones_data->current,
>>>>>>                                                  snat_dp_zone_key);
>>>>>>       free(snat_dp_zone_key);
>>>>>> @@ -2148,6 +2155,16 @@ ct_zones_runtime_data_handler(struct engine_node 
>>>>>> *node, void *data)
>>>>>>                                       &ct_zones_data->pending);
>>>>>>                   updated = true;
>>>>>>               }
>>>>>> +                char *drop_zone = alloc_ct_zone_key(
>>>>>> +                    &t_lport->pb->header_.uuid, "drop");
>>>>>> +                if (!simap_contains(&ct_zones_data->current, 
>>>>>> drop_zone)) {
>>>>>> +                    alloc_id_to_ct_zone(drop_zone,
>>>>>> +                                        &ct_zones_data->current,
>>>>>> +                                        ct_zones_data->bitmap, 
>>>>>> &scan_start,
>>>>>> +                                        &ct_zones_data->pending);
>>>>>> +                    updated = true;
>>>>>> +                }
>>>>>> +                free(drop_zone);
>>>>>>           } else if (t_lport->tracked_type == TRACKED_RESOURCE_REMOVED) {
>>>>>>               struct simap_node *ct_zone =
>>>>>>                   simap_find(&ct_zones_data->current,
>>>>>> diff --git a/controller/physical.c b/controller/physical.c
>>>>>> index f3c8bddce..fc46669c1 100644
>>>>>> --- a/controller/physical.c
>>>>>> +++ b/controller/physical.c
>>>>>> @@ -60,6 +60,7 @@ struct zone_ids {
>>>>>>   int ct;                     /* MFF_LOG_CT_ZONE. */
>>>>>>   int dnat;                   /* MFF_LOG_DNAT_ZONE. */
>>>>>>   int snat;                   /* MFF_LOG_SNAT_ZONE. */
>>>>>> +    int drop;                   /* MFF_LOG_ACL_DROP_ZONE. */
>>>>>> };
>>>>>> 
>>>>>> struct tunnel {
>>>>>> @@ -204,14 +205,18 @@ get_zone_ids(const struct sbrec_port_binding 
>>>>>> *binding,
>>>>>> 
>>>>>>   const struct uuid *key = &binding->datapath->header_.uuid;
>>>>>> 
>>>>>> -    char *dnat = alloc_nat_zone_key(key, "dnat");
>>>>>> +    char *dnat = alloc_ct_zone_key(key, "dnat");
>>>>>>   zone_ids.dnat = simap_get(ct_zones, dnat);
>>>>>>   free(dnat);
>>>>>> 
>>>>>> -    char *snat = alloc_nat_zone_key(key, "snat");
>>>>>> +    char *snat = alloc_ct_zone_key(key, "snat");
>>>>>>   zone_ids.snat = simap_get(ct_zones, snat);
>>>>>>   free(snat);
>>>>>> 
>>>>>> +    char *drop_zone = alloc_ct_zone_key(&binding->header_.uuid, "drop");
>>>>>> +    zone_ids.drop = simap_get(ct_zones, drop_zone);
>>>>>> +    free(drop_zone);
>>>>>> +
>>>>>>   return zone_ids;
>>>>>> }
>>>>>> 
>>>>>> @@ -822,6 +827,9 @@ put_zones_ofpacts(const struct zone_ids *zone_ids, 
>>>>>> struct ofpbuf *ofpacts_p)
>>>>>>       if (zone_ids->snat) {
>>>>>>           put_load(zone_ids->snat, MFF_LOG_SNAT_ZONE, 0, 32, ofpacts_p);
>>>>>>       }
>>>>>> +        if (zone_ids->drop) {
>>>>>> +            put_load(zone_ids->drop, MFF_LOG_ACL_DROP_ZONE, 0, 32, 
>>>>>> ofpacts_p);
>>>>>> +        }
>>>>>>   }
>>>>>> }
>>>>>> 
>>>>>> @@ -858,6 +866,26 @@ put_local_common_flows(uint32_t dp_key,
>>>>>>                   pb->header_.uuid.parts[0], &match, ofpacts_p,
>>>>>>                   &pb->header_.uuid);
>>>>>> 
>>>>>> +    if (zone_ids->drop) {
>>>>>> +        /* Table 39, Priority 1.
>>>>>> +         * =======================
>>>>>> +         *
>>>>>> +         * Clear the logical registers (for consistent behavior with 
>>>>>> packets
>>>>>> +         * that get tunneled) except MFF_LOG_ACL_DROP_ZONE. */
>>>>>> +        match_init_catchall(&match);
>>>>>> +        ofpbuf_clear(ofpacts_p);
>>>>>> +        match_set_metadata(&match, htonll(dp_key));
>>>>>> +        for (int i = 0; i < MFF_N_LOG_REGS; i++) {
>>>>>> +            if ((MFF_REG0 + i) != MFF_LOG_ACL_DROP_ZONE) {
>>>>>> +                put_load(0, MFF_REG0 + i, 0, 32, ofpacts_p);
>>>>>> +            }
>>>>>> +        }
>>>>>> +        put_resubmit(OFTABLE_LOG_EGRESS_PIPELINE, ofpacts_p);
>>>>>> +        ofctrl_add_flow(flow_table, OFTABLE_CHECK_LOOPBACK, 1,
>>>>>> +                        pb->datapath->header_.uuid.parts[0], &match,
>>>>>> +                        ofpacts_p, &pb->datapath->header_.uuid);
>>>>>> +    }
>>>>>> +
>>>>>>   /* Table 39, Priority 100.
>>>>>>    * =======================
>>>>>>    *
>>>>>> diff --git a/include/ovn/actions.h b/include/ovn/actions.h
>>>>>> index d7ee84dac..6424250a6 100644
>>>>>> --- a/include/ovn/actions.h
>>>>>> +++ b/include/ovn/actions.h
>>>>>> @@ -121,6 +121,7 @@ struct ovn_extend_table;
>>>>>>   OVNACT(COMMIT_ECMP_NH,    ovnact_commit_ecmp_nh)  \
>>>>>>   OVNACT(CHK_ECMP_NH_MAC,   ovnact_result)          \
>>>>>>   OVNACT(CHK_ECMP_NH,       ovnact_result)          \
>>>>>> +    OVNACT(CT_COMMIT_DROP,    ovnact_nest)            \
>>>>>> 
>>>>>> /* enum ovnact_type, with a member OVNACT_<ENUM> for each action. */
>>>>>> enum OVS_PACKED_ENUM ovnact_type {
>>>>>> diff --git a/include/ovn/logical-fields.h b/include/ovn/logical-fields.h
>>>>>> index 3db7265e4..889f5f9e3 100644
>>>>>> --- a/include/ovn/logical-fields.h
>>>>>> +++ b/include/ovn/logical-fields.h
>>>>>> @@ -47,6 +47,7 @@ enum ovn_controller_event {
>>>>>> #define MFF_LOG_REG0             MFF_REG0
>>>>>> #define MFF_LOG_LB_ORIG_DIP_IPV4 MFF_REG1
>>>>>> #define MFF_LOG_LB_ORIG_TP_DPORT MFF_REG2
>>>>>> +#define MFF_LOG_ACL_DROP_ZONE    MFF_REG8
>>>>>> 
>>>>>> #define MFF_LOG_XXREG0           MFF_XXREG0
>>>>>> #define MFF_LOG_LB_ORIG_DIP_IPV6 MFF_XXREG1
>>>>>> diff --git a/lib/actions.c b/lib/actions.c
>>>>>> index adbb42db4..dfff3e793 100644
>>>>>> --- a/lib/actions.c
>>>>>> +++ b/lib/actions.c
>>>>>> @@ -4600,6 +4600,54 @@ encode_CHK_ECMP_NH(const struct ovnact_result 
>>>>>> *res,
>>>>>>                          MLF_LOOKUP_COMMIT_ECMP_NH_BIT, ofpacts);
>>>>>> }
>>>>>> 
>>>>>> +static void
>>>>>> +parse_ct_commit_drop(struct action_context *ctx)
>>>>>> +{
>>>>>> +    parse_nested_action(ctx, OVNACT_CT_COMMIT_DROP, "ip", WR_CT_COMMIT);
>>>>>> +}
>>>>>> +
>>>>>> +static void
>>>>>> +format_CT_COMMIT_DROP(const struct ovnact_nest *on, struct ds *s)
>>>>>> +{
>>>>>> +    format_nested_action(on, "ct_commit_drop", s);
>>>>>> +}
>>>>>> +
>>>>>> +static void
>>>>>> +encode_CT_COMMIT_DROP(const struct ovnact_nest *on,
>>>>>> +                      const struct ovnact_encode_params *ep OVS_UNUSED,
>>>>>> +                      struct ofpbuf *ofpacts)
>>>>>> +{
>>>>>> +    struct ofpact_conntrack *ct = ofpact_put_CT(ofpacts);
>>>>>> +    ct->flags = NX_CT_F_COMMIT;
>>>>>> +    ct->recirc_table = NX_CT_RECIRC_NONE;
>>>>>> +    ct->zone_src.field = mf_from_id(MFF_LOG_ACL_DROP_ZONE);
>>>>>> +    ct->zone_src.ofs = 0;
>>>>>> +    ct->zone_src.n_bits = 16;
>>>>>> +
>>>>>> +    /* If the datapath supports all-zero SNAT then use it to avoid tuple
>>>>>> +     * collisions at commit time between NATed and firewalled-only 
>>>>>> sessions.
>>>>>> +     */
>>>>>> +    if (ovs_feature_is_supported(OVS_CT_ZERO_SNAT_SUPPORT)) {
>>>>>> +        size_t nat_offset = ofpacts->size;
>>>>>> +        ofpbuf_pull(ofpacts, nat_offset);
>>>>>> +
>>>>>> +        struct ofpact_nat *nat = ofpact_put_NAT(ofpacts);
>>>>>> +        nat->flags = 0;
>>>>>> +        nat->range_af = AF_UNSPEC;
>>>>>> +        nat->flags |= NX_NAT_F_SRC;
>>>>>> +        ofpacts->header = ofpbuf_push_uninit(ofpacts, nat_offset);
>>>>>> +        ct = ofpacts->header;
>>>>>> +    }
>>>>>> +
>>>>>> +    size_t set_field_offset = ofpacts->size;
>>>>>> +    ofpbuf_pull(ofpacts, set_field_offset);
>>>>>> +
>>>>>> +    ovnacts_encode(on->nested, on->nested_len, ep, ofpacts);
>>>>>> +    ofpacts->header = ofpbuf_push_uninit(ofpacts, set_field_offset);
>>>>>> +    ct = ofpacts->header;
>>>>>> +    ofpact_finish(ofpacts, &ct->ofpact);
>>>>>> +}
>>>>>> +
>>>>>> /* Parses an assignment or exchange or put_dhcp_opts action. */
>>>>>> static void
>>>>>> parse_set_action(struct action_context *ctx)
>>>>>> @@ -4790,6 +4838,8 @@ parse_action(struct action_context *ctx)
>>>>>>       parse_put_fdb(ctx, ovnact_put_PUT_FDB(ctx->ovnacts));
>>>>>>   } else if (lexer_match_id(ctx->lexer, "commit_ecmp_nh")) {
>>>>>>       parse_commit_ecmp_nh(ctx, ovnact_put_COMMIT_ECMP_NH(ctx->ovnacts));
>>>>>> +    } else if (lexer_match_id(ctx->lexer, "ct_commit_drop")) {
>>>>>> +        parse_ct_commit_drop(ctx);
>>>>>>   } else {
>>>>>>       lexer_syntax_error(ctx->lexer, "expecting action");
>>>>>>   }
>>>>>> diff --git a/lib/ovn-util.c b/lib/ovn-util.c
>>>>>> index 616999eab..a72533a9e 100644
>>>>>> --- a/lib/ovn-util.c
>>>>>> +++ b/lib/ovn-util.c
>>>>>> @@ -443,12 +443,12 @@ split_addresses(const char *addresses, struct svec 
>>>>>> *ipv4_addrs,
>>>>>>   destroy_lport_addresses(&laddrs);
>>>>>> }
>>>>>> 
>>>>>> -/* Allocates a key for NAT conntrack zone allocation for a provided
>>>>>> +/* Allocates a key for conntrack zone allocation for a provided
>>>>>> * 'key' record and a 'type'.
>>>>>> *
>>>>>> * It is the caller's responsibility to free the allocated memory. */
>>>>>> char *
>>>>>> -alloc_nat_zone_key(const struct uuid *key, const char *type)
>>>>>> +alloc_ct_zone_key(const struct uuid *key, const char *type)
>>>>>> {
>>>>>>   return xasprintf(UUID_FMT"_%s", UUID_ARGS(key), type);
>>>>>> }
>>>>>> diff --git a/lib/ovn-util.h b/lib/ovn-util.h
>>>>>> index b3905ef7b..e0855f19e 100644
>>>>>> --- a/lib/ovn-util.h
>>>>>> +++ b/lib/ovn-util.h
>>>>>> @@ -92,7 +92,7 @@ const char *find_lport_address(const struct 
>>>>>> lport_addresses *laddrs,
>>>>>> void split_addresses(const char *addresses, struct svec *ipv4_addrs,
>>>>>>                    struct svec *ipv6_addrs);
>>>>>> 
>>>>>> -char *alloc_nat_zone_key(const struct uuid *key, const char *type);
>>>>>> +char *alloc_ct_zone_key(const struct uuid *key, const char *type);
>>>>>> 
>>>>>> const char *default_nb_db(void);
>>>>>> const char *default_sb_db(void);
>>>>>> diff --git a/northd/northd.c b/northd/northd.c
>>>>>> index 7e2681865..2aff3458c 100644
>>>>>> --- a/northd/northd.c
>>>>>> +++ b/northd/northd.c
>>>>>> @@ -287,7 +287,7 @@ enum ovn_stage {
>>>>>> * +----+----------------------------------------------+ G |              
>>>>>>     |
>>>>>> * | R7 |                   UNUSED                     | 1 |              
>>>>>>     |
>>>>>> * 
>>>>>> +----+----------------------------------------------+---+------------------+
>>>>>> - * | R8 |                   UNUSED                     |
>>>>>> + * | R8 |        DROP_CT_ZONE (<= IN(/OUT)_ACL         |
>>>>>> * +----+----------------------------------------------+
>>>>>> * | R9 |                   UNUSED                     |
>>>>>> * +----+----------------------------------------------+
>>>>>> @@ -6343,6 +6343,11 @@ consider_acl(struct hmap *lflows, struct 
>>>>>> ovn_datapath *od,
>>>>>>           } else {
>>>>>>               ds_put_format(match, " && (%s)", acl->match);
>>>>>>               build_acl_log(actions, acl, meter_groups);
>>>>>> +                if (acl->label) {
>>>>>> +                    ds_put_format(actions, "ct_commit_drop { "
>>>>>> +                                  "ct_label.label = %"PRId64"; }; ",
>>>>>> +                                  acl->label);
>>>>>> +                }
>>>>>>               ds_put_cstr(actions, "/* drop */");
>>>>>>               ovn_lflow_add_with_hint(lflows, od, stage,
>>>>>>                                       acl->priority + OVN_ACL_PRI_OFFSET,
>>>>>> @@ -6363,8 +6368,13 @@ consider_acl(struct hmap *lflows, struct 
>>>>>> ovn_datapath *od,
>>>>>>           ds_clear(match);
>>>>>>           ds_clear(actions);
>>>>>>           ds_put_cstr(match, REGBIT_ACL_HINT_BLOCK " == 1");
>>>>>> -            ds_put_format(actions, "ct_commit { %s = 1; }; ",
>>>>>> +            ds_put_format(actions, "ct_commit { %s = 1; ",
>>>>>>                         ct_blocked_match);
>>>>>> +            if (acl->label) {
>>>>>> +                ds_put_format(actions, "ct_label.label = %"PRId64"; ",
>>>>>> +                              acl->label);
>>>>>> +            }
>>>>>> +            ds_put_cstr(actions, "}; ");
>>>>>>           if (!strcmp(acl->action, "reject")) {
>>>>>>               build_reject_acl_rules(od, lflows, stage, acl, match,
>>>>>>                                      actions, &acl->header_, 
>>>>>> meter_groups);
>>>>>> diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
>>>>>> index b8f871394..af653be4d 100644
>>>>>> --- a/northd/ovn-northd.8.xml
>>>>>> +++ b/northd/ovn-northd.8.xml
>>>>>> @@ -699,7 +699,12 @@
>>>>>>       connections and <code>ct_commit(ct_label=1/1);</code> for known
>>>>>>       connections.  Setting <code>ct_label</code> marks a connection
>>>>>>       as one that was previously allowed, but should no longer be
>>>>>> -        allowed due to a policy change.
>>>>>> +        allowed due to a policy change. If the ACL has a 
>>>>>> <code>label</code>,
>>>>>> +        then it translates to
>>>>>> +        <code>ct_commit_drop(ct_label.label=label)</code> for new and
>>>>>> +        untracked connections and
>>>>>> +        <code>ct_commit(ct_label.blocked=1; 
>>>>>> ct_label.label=label);</code>
>>>>>> +        for known connections.
>>>>>>     </li>
>>>>>>   </ul>
>>>>>> 
>>>>>> @@ -969,7 +974,12 @@
>>>>>>       or untracked connections and <code>ct_commit(ct_label=1/1);</code> 
>>>>>> for
>>>>>>       known connections.  Setting <code>ct_label</code> marks a 
>>>>>> connection
>>>>>>       as one that was previously allowed, but should no longer be
>>>>>> -        allowed due to a policy change.
>>>>>> +        allowed due to a policy change. If the ACL has a 
>>>>>> <code>label</code>,
>>>>>> +        then it translates to
>>>>>> +        <code>ct_commit_drop(ct_label.label=label)</code> for new and
>>>>>> +        untracked connections and
>>>>>> +        <code>ct_commit(ct_label.blocked=1; 
>>>>>> ct_label.label=label);</code>
>>>>>> +        for known connections.
>>>>>>     </li>
>>>>>>   </ul>
>>>>>> 
>>>>>> diff --git a/ovn-sb.xml b/ovn-sb.xml
>>>>>> index 37a709f83..4e294a212 100644
>>>>>> --- a/ovn-sb.xml
>>>>>> +++ b/ovn-sb.xml
>>>>>> @@ -1377,6 +1377,23 @@
>>>>>>         </p>
>>>>>>       </dd>
>>>>>> 
>>>>>> +        <dt><code>ct_commit_drop { };</code></dt>
>>>>>> +        <dt><code>ct_commit_drop { ct_mark=<var>value[/mask]</var>; 
>>>>>> };</code></dt>
>>>>>> +        <dt><code>ct_commit_drop { ct_label=<var>value[/mask]</var>; 
>>>>>> };</code></dt>
>>>>>> +        <dt><code>ct_commit_drop { ct_mark=<var>value[/mask]</var>; 
>>>>>> ct_label=<var>value[/mask]</var>; };</code></dt>
>>>>>> +        <dd>
>>>>>> +          <p>
>>>>>> +            This action is identical to <code>ct_commit</code> except 
>>>>>> that the
>>>>>> +            connection tracking entry is committed in a different zone.
>>>>>> +          </p>
>>>>>> +
>>>>>> +          <p>
>>>>>> +            This action was added to store connections dropped by ACLs 
>>>>>> in a
>>>>>> +            separate zone that is managed independently of the
>>>>>> +            <code>ct_commit</code> zone, for debugging.
>>>>>> +          </p>
>>>>>> +        </dd>
>>>>>> +
>>>>>>       <dt><code>ct_dnat;</code></dt>
>>>>>>       <dt><code>ct_dnat(<var>IP</var>);</code></dt>
>>>>>>       <dd>
>>>>>> diff --git a/ovs b/ovs
>>>>>> index 6f24c2bc7..d94cd0d3e 160000
>>>>>> --- a/ovs
>>>>>> +++ b/ovs
>>>>>> @@ -1 +1 @@
>>>>>> -Subproject commit 6f24c2bc769afde0a390ce344de1a7d9c592e5a6
>>>>>> +Subproject commit d94cd0d3eec33e4290d7ca81918f5ac61444886e
>>>>>> diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
>>>>>> index 3bbdbd998..07509f488 100644
>>>>>> --- a/utilities/ovn-nbctl.c
>>>>>> +++ b/utilities/ovn-nbctl.c
>>>>>> @@ -2225,10 +2225,11 @@ nbctl_acl_add(struct ctl_context *ctx)
>>>>>>   /* Set the ACL label */
>>>>>>   const char *label = shash_find_data(&ctx->options, "--label");
>>>>>>   if (label) {
>>>>>> -      /* Ensure that the action is either allow or allow-related */
>>>>>> -      if (strcmp(action, "allow") && strcmp(action, "allow-related")) {
>>>>>> +      /* Ensure that the action is either allow or allow-related or 
>>>>>> drop */
>>>>>> +      if (strcmp(action, "allow") && strcmp(action, "allow-related") &&
>>>>>> +          strcmp(action, "drop")) {
>>>>>>       ctl_error(ctx, "label can only be set with actions \"allow\" or "
>>>>>> -                  "\"allow-related\"");
>>>>>> +                  "\"allow-related\" or \"drop\"");
>>>>>>       return;
>>>>>>     }
>> 
> 

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to