[ovs-dev] [PATCH ovn v6] ofctrl.c: Add a predictable resolution for conflicting flow actions.

Dumitru Ceara dceara at redhat.com
Thu Oct 15 07:09:39 UTC 2020


On 10/14/20 6:11 PM, Han Zhou wrote:
> 
> 
> On Wed, Oct 14, 2020 at 12:21 AM Dumitru Ceara <dceara at redhat.com
> <mailto:dceara at redhat.com>> wrote:
>>
>> On 10/13/20 8:06 PM, Han Zhou wrote:
>> >
>> >
>> > On Tue, Oct 13, 2020 at 1:52 AM Dumitru Ceara <dceara at redhat.com
> <mailto:dceara at redhat.com>
>> > <mailto:dceara at redhat.com <mailto:dceara at redhat.com>>> wrote:
>> >>
>> >> Until now, in case the ACL configuration generates openflows that have
>> >> the same match but different actions, ovn-controller was using the
>> >> following approach:
>> >> 1. If the flow being added contains conjunctive actions, merge its
>> >>    actions with the already existing flow.
>> >> 2. Otherwise, if the flow is being added incrementally
>> >>    (update_installed_flows_by_track), don't install the new flow but
>> >>    instead keep the old one.
>> >> 3. Otherwise, (update_installed_flows_by_compare), don't install the
>> >>    new flow but instead keep the old one.
>> >>
>> >> Even though one can argue that having an ACL with a match that includes
>> >> the match of another ACL is a misconfiguration, it can happen that the
>> >> users provision OVN like this.  Depending on the order of reading and
>> >> installing the logical flows, the above operations can yield
>> >> unpredictable results, e.g., allow specific traffic but then after
>> >> ovn-controller is restarted (or a recompute happens) that specific
>> >> traffic starts being dropped.
>> >>
>> >> A simple example of ACL configuration is:
>> >> ovn-nbctl acl-add ls to-lport 3 '(ip4.src==10.0.0.1 ||
>> >> ip4.src==10.0.0.2) && (ip4.dst == 10.0.0.3 || ip4.dst == 10.0.0.4)' allow
>> >> ovn-nbctl acl-add ls to-lport 3 'ip4.src==10.0.0.1' allow
>> >> ovn-nbctl acl-add ls to-lport 2 'arp' allow
>> >> ovn-nbctl acl-add ls to-lport 1 'ip4' drop
>> >>
>> >> This is a pattern used by most CMSs:
>> >> - define a default deny policy.
>> >> - punch holes in the default deny policy based on user specific
>> >>   configurations.
>> >>
>> >> Without this commit the behavior for traffic from 10.0.0.1 to 10.0.0.5
>> >> is unpredictable.  Depending on the order of operations traffic might be
>> >> dropped or allowed.
>> >>
>> >> It's also quite hard to force the CMS to ensure that such match overlaps
>> >> never occur.
>> >>
>> >> To address this issue we now ensure that when selecting the active flow
>> >> from all desired flows refering the same installed flow we give
>> >> precedence to "allow" over "drop" over "conjunction" flows.
>> >>
>> >> Essentially, less restrictive flows are always preferred over more
>> >> restrictive flows whenever a match conflict happens.
>> >>
>> >> CC: Daniel Alvarez <dalvarez at redhat.com <mailto:dalvarez at redhat.com>
> <mailto:dalvarez at redhat.com <mailto:dalvarez at redhat.com>>>
>> >> Reported-at: https://bugzilla.redhat.com/1871931
>> >> Signed-off-by: Dumitru Ceara <dceara at redhat.com <mailto:dceara at redhat.com>
> <mailto:dceara at redhat.com <mailto:dceara at redhat.com>>>
>> >> ---
>> >> v6:
>> >> - First 3 patches of the v5 series were already applied.
>> >> - Send patch 4 as v6 addressing Han's comments:
>> >>   - Instead of maintaining a sorted list, select active flow by walking
>> >>     the list of desired flows.
>> >>   - Add missing ovn_flow_log() call.
>> >> v5:
>> >> - Turn the patch into a series.
>> >> - Address Han's comments.
>> >> - Ensure predictable flow ordering in all cases (during incremental
>> >>   procesing
>> >>   or full recomputation).
>> >> v4:
>> >> - Address Han's comments:
>> >>   - make sure only flows with action conjunction are combined.
>> >> v3:
>> >> - Add Mark's ack.
>> >> - Add last missing pcap check in the test.
>> >> v2:
>> >> - Address Han's comments:
>> >>   - Do not delete desired flow that cannot be merged, it might be
>> >>     installed later.
>> >>   - Fix typos in the commit log.
>> >> - Update the test to check the OVS flows.
>> >> ---
>> >>  controller/ofctrl.c |  96 +++++++++++++++++++++--
>> >>  tests/ovn.at <http://ovn.at> <http://ovn.at>        | 214
>> > ++++++++++++++++++++++++++++++++++++++++++++++++++++
>> >>  2 files changed, 305 insertions(+), 5 deletions(-)
>> >>
>> >> diff --git a/controller/ofctrl.c b/controller/ofctrl.c
>> >> index ba0c61c..0edb2b9 100644
>> >> --- a/controller/ofctrl.c
>> >> +++ b/controller/ofctrl.c
>> >> @@ -229,6 +229,8 @@ static struct installed_flow *installed_flow_lookup(
>> >>  static void installed_flow_destroy(struct installed_flow *);
>> >>  static struct installed_flow *installed_flow_dup(struct desired_flow *);
>> >>  static struct desired_flow *installed_flow_get_active(struct
> installed_flow *);
>> >> +static struct desired_flow *installed_flow_select_active(
>> >> +    struct installed_flow *);
>> >>
>> >>  static uint32_t ovn_flow_match_hash(const struct ovn_flow *);
>> >>  static char *ovn_flow_to_string(const struct ovn_flow *);
>> >> @@ -796,6 +798,12 @@ ofctrl_recv(const struct ofp_header *oh, enum ofptype
> type)
>> >>  }
>> >>
>> >>  static bool
>> >> +flow_action_has_drop(const struct ovn_flow *f)
>> >> +{
>> >> +    return f->ofpacts_len == 0;
>> >> +}
>> >> +
>> >> +static bool
>> >>  flow_action_has_conj(const struct ovn_flow *f)
>> >>  {
>> >>      const struct ofpact *a = NULL;
>> >> @@ -822,7 +830,7 @@ link_installed_to_desired(struct installed_flow *i,
>> > struct desired_flow *d)
>> >>  {
>> >>      d->installed_flow = i;
>> >>      ovs_list_push_back(&i->desired_refs, &d->installed_ref_list_node);
>> >> -    return installed_flow_get_active(i) == d;
>> >> +    return installed_flow_select_active(i) == d;
>> >>  }
>> >>
>> > Hi Dumitru, I am really sorry that I didn't make it clear enough in my last
>> > comments. I meant that I prefer maintaining the order in the desired_refs list
>> > in the function link_installed_to_desired() when the item is inserted, which
>> > makes the "first item is active" more useful. I didn't want to you drop the
>> > ordering (but just wanted to avoid the array). Otherwise I would not merge the
>> > patch 2 & 3 but directly use the temporary patch you created. To be more clear,
>> > it could be something like below:
>> >
>>
>> Hi Han,
>>
>> I guess I misunderstood your comments indeed. I thought you didn't want to keep
>> the list partially sorted.
>>
>> However, I'm now curious what your concerns were regarding my original
>> suggestion, apart from the missing ovn_flow_log():
>>
>>
> http://patchwork.ozlabs.org/project/ovn/patch/20201011120607.3374.49659.stgit@dceara.remote.csb/
>>
> 
> My last comment only suggested avoiding the array of lists for patch 4. It is a
> little overkill to store such data that in most cases only 1 list item exists
> in only one of the array items.
> 

Ok, thanks for the explanation.

I'll send a new revision soon.

Regards,
Dumitru

>> > /* Returns true if flow a is prefered over flow b. */
>> > static bool
>> > flow_compare(struct ovn_flow *a, struct ovn_flow *b)
>> > {
>> >     ...
>> >     if (b_has_allow) {
>> >         return false;
>> >     }
>> >     if (a_has_allow) {
>> >         return true;
>> >     }
>> >     if (b_has_drop) {
>> >         return false;
>> >     }
>> >     if (a_has_drop) {
>> >         return true;
>> >     }
>> >     /* both has conjunction only, assert failure */
>> >     OVS_NOT_REACHED();
>> > }
>> >
>> > static bool
>> > link_installed_to_desired(struct installed_flow *i, struct desired_flow *d)
>> > {
>> >     d->installed = i;
>> >     bool inserted = false;
>> >     LIST_FOR_EACH(f, &i->desired_refs) {
>> >         if (flow_compare(&d->flow, &f->flow)) {
>> >             /* Insert d before f */
>> >             ovs_list_insert(&f->installed_ref_list_node,
>> > &d->installed_ref_list_node);
>> >             inserted = true;
>> >             break;
>> >         }
>> >     }
>> >     if (!inserted) {
>> >         ovs_list_push_back(&i->desired_refs, &d->installed_ref_list_node);
>> >     }
>> > }
>> >
>> >>  /* Replaces 'old_desired' with 'new_desired' in the list of desired flows
>> >> @@ -852,6 +860,7 @@ unlink_installed_to_desired(struct installed_flow *i,
>> > struct desired_flow *d)
>> >>
>> >>      ovs_assert(d && d->installed_flow == i);
>> >>      ovs_list_remove(&d->installed_ref_list_node);
>> >> +    installed_flow_select_active(i);
>> >
>> > This is not needed if the order is always maintained in
>> > link_installed_to_desired like above example.
>> >
>> >>      d->installed_flow = NULL;
>> >>      return old_active == d;
>> >>  }
>> >> @@ -1274,6 +1283,70 @@ installed_flow_get_active(struct installed_flow *f)
>> >>      return NULL;
>> >>  }
>> >>
>> >> +/* Walks the list of desired flows that refer 'f' and selects the least
>> >> + * restrictive one to be used as active flow, moving it to the front of
>> >> + * the list.
>> >> + *
>> >> + * Returns the new active flow.
>> >> + */
>> >> +static struct desired_flow *
>> >> +installed_flow_select_active(struct installed_flow *f)
>> >> +{
>> >> +    struct desired_flow *old_active = installed_flow_get_active(f);
>> >> +    struct desired_flow *active = NULL;
>> >> +
>> >> +    /* If there is at most one desired_flow, no selection needed, return
>> >> +     * early.
>> >> +     */
>> >> +    if (!old_active || ovs_list_is_short(&f->desired_refs)) {
>> >> +        return old_active;
>> >> +    }
>> >> +
>> >> +    struct desired_flow *d_allow = NULL;
>> >> +    struct desired_flow *d_drop = NULL;
>> >> +    struct desired_flow *d_conj = NULL;
>> >> +    struct desired_flow *d;
>> >> +
>> >> +    LIST_FOR_EACH (d, installed_ref_list_node, &f->desired_refs) {
>> >> +        if (flow_action_has_drop(&d->flow)) {
>> >> +            if (!d_drop) {
>> >> +                d_drop = d;
>> >> +            }
>> >> +        } else if (flow_action_has_conj(&d->flow)) {
>> >> +            /* Conjunction flows are always merged together into a single
>> >> +             * flow.  There should never be more than one referring the same
>> >> +             * installed flow.
>> >> +             */
>> >> +            ovs_assert(!d_conj);
>> >> +            d_conj = d;
>> >> +        } else {
>> >> +            if (!d_allow) {
>> >> +                d_allow = d;
>> >> +            }
>> >> +        }
>> >> +    }
>> >> +
>> >> +    /* Give precedence to "allow" over "drop" over "conjunction" action. */
>> >> +    if (d_allow) {
>> >> +        active = d_allow;
>> >> +    } else if (d_drop) {
>> >> +        active = d_drop;
>> >> +    } else {
>> >> +        active = d_conj;
>> >> +    }
>> >
>> > If there are multiple flows in the same category (e.g. allow) in the list, this
>> > function will select a different flow everytime, and move it to the front,
>>
>> No. The flows are always pushed to the back of the list. So if there already
>> was a flow in the same category this function will still find it as "active"
>> and won't replace it.
> 
> Sorry, I misread it.
>>
>> > resulting in an unnecessary flow-mod. (But never mind, because this function is
>> > not needed with the above suggested change)
>> >
>>
>> Right, this is not needed if we maintain a sorted list.
>>
>> Thanks,
>> Dumitru
>>
>> >> +
>> >> +    /* There must be an active flow otherwise we should have returned
> early. */
>> >> +    ovs_assert(active);
>> >> +
>> >> +    /* Move the newly selected active flow to the front of the list. */
>> >> +    if (active != old_active) {
>> >> +        ovs_list_remove(&active->installed_ref_list_node);
>> >> +        ovs_list_push_front(&f->desired_refs,
>> >> +                            &active->installed_ref_list_node);
>> >> +    }
>> >> +    return active;
>> >> +}
>> >> +
>> >>  static struct desired_flow *
>> >>  desired_flow_lookup__(struct ovn_desired_flow_table *flow_table,
>> >>                        const struct ovn_flow *target,
>> >> @@ -1789,8 +1862,14 @@ update_installed_flows_by_compare(struct
>> > ovn_desired_flow_table *flow_table,
>> >>              link_installed_to_desired(i, d);
>> >>          } else if (!d->installed_flow) {
>> >>              /* This is a desired_flow that conflicts with one installed
>> >> -             * previously but not linked yet. */
>> >> -            link_installed_to_desired(i, d);
>> >> +             * previously but not linked yet.  However, if this flow becomes
>> >> +             * active, e.g., it is less restrictive than the previous active
>> >> +             * flow then modify the installed flow.
>> >> +             */
>> >> +            if (link_installed_to_desired(i, d)) {
>> >> +                installed_flow_mod(&i->flow, &d->flow, msgs);
>> >> +                ovn_flow_log(&i->flow, "updating installed (conflict)");
>> >> +            }
>> >>          }
>> >>      }
>> >>  }
>> >> @@ -1919,8 +1998,15 @@ update_installed_flows_by_track(struct
>> > ovn_desired_flow_table *flow_table,
>> >>                  installed_flow_mod(&i->flow, &f->flow, msgs);
>> >>              } else {
>> >>                  /* Adding a new flow that conflicts with an existing
> installed
>> >> -                 * flow, so just add it to the link. */
>> >> -                link_installed_to_desired(i, f);
>> >> +                 * flow, so add it to the link.  If this flow becomes active,
>> >> +                 * e.g., it is less restrictive than the previous active flow
>> >> +                 * then modify the installed flow.
>> >> +                 */
>> >> +                if (link_installed_to_desired(i, f)) {
>> >> +                    installed_flow_mod(&i->flow, &f->flow, msgs);
>> >> +                    ovn_flow_log(&i->flow,
>> >> +                                 "updating installed (tracked conflict)");
>> >> +                }
>> >>              }
>> >>              /* The track_list_node emptyness is used to check if the node is
>> >>               * already added to track list, so initialize it again here. */
>> >> diff --git a/tests/ovn.at <http://ovn.at> <http://ovn.at> b/tests/ovn.at
> <http://ovn.at> <http://ovn.at>
>> >> index 488fd11..9420b1e 100644
>> >> --- a/tests/ovn.at <http://ovn.at> <http://ovn.at>
>> >> +++ b/tests/ovn.at <http://ovn.at> <http://ovn.at>
>> >> @@ -13727,6 +13727,220 @@ grep conjunction.*conjunction.*conjunction | wc
> -l`])
>> >>  OVN_CLEANUP([hv1])
>> >>  AT_CLEANUP
>> >>
>> >> +AT_SETUP([ovn -- Superseeding ACLs with conjunction])
>> >> +ovn_start
>> >> +
>> >> +ovn-nbctl ls-add ls1
>> >> +
>> >> +ovn-nbctl lsp-add ls1 ls1-lp1 \
>> >> +-- lsp-set-addresses ls1-lp1 "f0:00:00:00:00:01"
>> >> +
>> >> +ovn-nbctl lsp-add ls1 ls1-lp2 \
>> >> +-- lsp-set-addresses ls1-lp2 "f0:00:00:00:00:02"
>> >> +
>> >> +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
>> >> +
>> >> +ovs-vsctl -- add-port br-int hv1-vif2 -- \
>> >> +    set interface hv1-vif2 external-ids:iface-id=ls1-lp2 \
>> >> +    options:tx_pcap=hv1/vif2-tx.pcap \
>> >> +    options:rxq_pcap=hv1/vif2-rx.pcap \
>> >> +    ofport-request=2
>> >> +
>> >> +# test_ip INPORT SRC_MAC DST_MAC SRC_IP DST_IP OUTPORT...
>> >> +#
>> >> +# This shell function causes an ip packet to be received on INPORT.
>> >> +# The packet's content has Ethernet destination DST and source SRC
>> >> +# (each exactly 12 hex digits) and Ethernet type ETHTYPE (4 hex digits).
>> >> +# The OUTPORTs (zero or more) list the VIFs on which the packet should
>> >> +# be received.  INPORT and the OUTPORTs are specified as logical switch
>> >> +# port numbers, e.g. 11 for vif11.
>> >> +test_ip() {
>> >> +    # This packet has bad checksums but logical L3 routing doesn't check.
>> >> +    local inport=$1 src_mac=$2 dst_mac=$3 src_ip=$4 dst_ip=$5
>> >> +    local packet=${dst_mac}${src_mac}08004500001c0000000040110000${src_ip}\
>> >> +${dst_ip}0035111100080000
>> >> +    shift; shift; shift; shift; shift
>> >> +    as hv1 ovs-appctl netdev-dummy/receive hv1-vif1 $packet
>> >> +    for outport; do
>> >> +        echo $packet >> $outport.expected
>> >> +    done
>> >> +}
>> >> +
>> >> +ip_to_hex() {
>> >> +    printf "%02x%02x%02x%02x" "$@"
>> >> +}
>> >> +
>> >> +reset_pcap_file() {
>> >> +    local iface=$1
>> >> +    local pcap_file=$2
>> >> +    ovs-vsctl -- set Interface $iface options:tx_pcap=dummy-tx.pcap \
>> >> +options:rxq_pcap=dummy-rx.pcap
>> >> +    rm -f ${pcap_file}*.pcap
>> >> +    ovs-vsctl -- set Interface $iface options:tx_pcap=${pcap_file}-tx.pcap \
>> >> +options:rxq_pcap=${pcap_file}-rx.pcap
>> >> +}
>> >> +
>> >> +# Add a default deny ACL and an allow ACL for specific IP traffic.
>> >> +ovn-nbctl acl-add ls1 to-lport 2 'arp' allow
>> >> +ovn-nbctl acl-add ls1 to-lport 1 'ip4' drop
>> >> +ovn-nbctl acl-add ls1 to-lport 3 '(ip4.src==10.0.0.1 || ip4.src==10.0.0.2)
>> > && (ip4.dst == 10.0.0.3 || ip4.dst == 10.0.0.4)' allow
>> >> +ovn-nbctl acl-add ls1 to-lport 3 '(ip4.src==10.0.0.1 || ip4.src==10.0.0.42)
>> > && (ip4.dst == 10.0.0.3 || ip4.dst == 10.0.0.4)' allow
>> >> +ovn-nbctl --wait=hv sync
>> >> +
>> >> +# Traffic 10.0.0.1, 10.0.0.2 -> 10.0.0.3, 10.0.0.4 should be allowed.
>> >> +for src in `seq 1 2`; do
>> >> +    for dst in `seq 3 4`; do
>> >> +        sip=`ip_to_hex 10 0 0 $src`
>> >> +        dip=`ip_to_hex 10 0 0 $dst`
>> >> +
>> >> +        test_ip 1 f00000000001 f00000000002 $sip $dip 2
>> >> +    done
>> >> +done
>> >> +
>> >> +# Traffic 10.0.0.1, 10.0.0.2 -> 10.0.0.5 should be dropped.
>> >> +dip=`ip_to_hex 10 0 0 5`
>> >> +for src in `seq 1 2`; do
>> >> +    sip=`ip_to_hex 10 0 0 $src`
>> >> +
>> >> +    test_ip 1 f00000000001 f00000000002 $sip $dip
>> >> +done
>> >> +
>> >> +cat 2.expected > expout
>> >> +$PYTHON "$ovs_srcdir/utilities/ovs-pcap.in <http://ovs-pcap.in>
> <http://ovs-pcap.in>"
>> > hv1/vif2-tx.pcap > 2.packets
>> >> +AT_CHECK([cat 2.packets], [0], [expout])
>> >> +reset_pcap_file hv1-vif2 hv1/vif2
>> >> +rm -f 2.packets
>> >> +> 2.expected
>> >> +
>> >> +# Add two less restrictive allow ACLs for src IP 10.0.0.1.
>> >> +ovn-nbctl acl-add ls1 to-lport 3 'ip4.src==10.0.0.1 || ip4.src==10.0.0.1'
> allow
>> >> +ovn-nbctl acl-add ls1 to-lport 3 'ip4.src==10.0.0.1' allow
>> >> +ovn-nbctl --wait=hv sync
>> >> +
>> >> +# Check OVS flows, the less restrictive flows should have been installed.
>> >> +AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=45 | \
>> >> +    grep "priority=1003" | awk '{print $7 " " $8}' | sort], [0], [dnl
>> >> +priority=1003,conj_id=2,ip,metadata=0x1 actions=resubmit(,46)
>> >> +priority=1003,conj_id=3,ip,metadata=0x1 actions=resubmit(,46)
>> >> +priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3
>> > actions=conjunction(2,1/2),conjunction(3,1/2)
>> >> +priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4
>> > actions=conjunction(2,1/2),conjunction(3,1/2)
>> >> +priority=1003,ip,metadata=0x1,nw_src=10.0.0.1 actions=resubmit(,46)
>> >> +priority=1003,ip,metadata=0x1,nw_src=10.0.0.2 actions=conjunction(2,2/2)
>> >> +priority=1003,ip,metadata=0x1,nw_src=10.0.0.42 actions=conjunction(3,2/2)
>> >> +])
>> >> +
>> >> +# Traffic 10.0.0.1, 10.0.0.2 -> 10.0.0.3, 10.0.0.4 should be allowed.
>> >> +for src in `seq 1 2`; do
>> >> +    for dst in `seq 3 4`; do
>> >> +        sip=`ip_to_hex 10 0 0 $src`
>> >> +        dip=`ip_to_hex 10 0 0 $dst`
>> >> +
>> >> +        test_ip 1 f00000000001 f00000000002 $sip $dip 2
>> >> +    done
>> >> +done
>> >> +
>> >> +# Traffic 10.0.0.2 -> 10.0.0.5 should be dropped.
>> >> +sip=`ip_to_hex 10 0 0 2`
>> >> +dip=`ip_to_hex 10 0 0 5`
>> >> +test_ip 1 f00000000001 f00000000002 $sip $dip
>> >> +
>> >> +# Traffic 10.0.0.1 -> 10.0.0.5 should be allowed.
>> >> +sip=`ip_to_hex 10 0 0 1`
>> >> +dip=`ip_to_hex 10 0 0 5`
>> >> +test_ip 1 f00000000001 f00000000002 $sip $dip 2
>> >> +
>> >> +cat 2.expected > expout
>> >> +$PYTHON "$ovs_srcdir/utilities/ovs-pcap.in <http://ovs-pcap.in>
> <http://ovs-pcap.in>"
>> > hv1/vif2-tx.pcap > 2.packets
>> >> +AT_CHECK([cat 2.packets], [0], [expout])
>> >> +reset_pcap_file hv1-vif2 hv1/vif2
>> >> +rm -f 2.packets
>> >> +> 2.expected
>> >> +
>> >> +#sleep infinity
>> >> +
>> >> +# Remove the first less restrictive allow ACL.
>> >> +ovn-nbctl acl-del ls1 to-lport 3 'ip4.src==10.0.0.1 || ip4.src==10.0.0.1'
>> >> +ovn-nbctl --wait=hv sync
>> >> +
>> >> +# Check OVS flows, the second less restrictive allow ACL should have been
>> > installed.
>> >> +AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=45 | \
>> >> +    grep "priority=1003" | awk '{print $7 " " $8}' | sort], [0], [dnl
>> >> +priority=1003,conj_id=2,ip,metadata=0x1 actions=resubmit(,46)
>> >> +priority=1003,conj_id=3,ip,metadata=0x1 actions=resubmit(,46)
>> >> +priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3
>> > actions=conjunction(2,1/2),conjunction(3,1/2)
>> >> +priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4
>> > actions=conjunction(2,1/2),conjunction(3,1/2)
>> >> +priority=1003,ip,metadata=0x1,nw_src=10.0.0.1 actions=resubmit(,46)
>> >> +priority=1003,ip,metadata=0x1,nw_src=10.0.0.2 actions=conjunction(2,2/2)
>> >> +priority=1003,ip,metadata=0x1,nw_src=10.0.0.42 actions=conjunction(3,2/2)
>> >> +])
>> >> +
>> >> +# Remove the less restrictive allow ACL.
>> >> +ovn-nbctl acl-del ls1 to-lport 3 'ip4.src==10.0.0.1'
>> >> +ovn-nbctl --wait=hv sync
>> >> +
>> >> +# Check OVS flows, the 10.0.0.1 conjunction should have been reinstalled.
>> >> +AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=45 | \
>> >> +    grep "priority=1003" | awk '{print $7 " " $8}' | sort], [0], [dnl
>> >> +priority=1003,conj_id=2,ip,metadata=0x1 actions=resubmit(,46)
>> >> +priority=1003,conj_id=3,ip,metadata=0x1 actions=resubmit(,46)
>> >> +priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3
>> > actions=conjunction(2,1/2),conjunction(3,1/2)
>> >> +priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4
>> > actions=conjunction(2,1/2),conjunction(3,1/2)
>> >> +priority=1003,ip,metadata=0x1,nw_src=10.0.0.1
>> > actions=conjunction(2,2/2),conjunction(3,2/2)
>> >> +priority=1003,ip,metadata=0x1,nw_src=10.0.0.2 actions=conjunction(2,2/2)
>> >> +priority=1003,ip,metadata=0x1,nw_src=10.0.0.42 actions=conjunction(3,2/2)
>> >> +])
>> >> +
>> >> +# Traffic 10.0.0.1, 10.0.0.2 -> 10.0.0.3, 10.0.0.4 should be allowed.
>> >> +for src in `seq 1 2`; do
>> >> +    for dst in `seq 3 4`; do
>> >> +        sip=`ip_to_hex 10 0 0 $src`
>> >> +        dip=`ip_to_hex 10 0 0 $dst`
>> >> +
>> >> +        test_ip 1 f00000000001 f00000000002 $sip $dip 2
>> >> +    done
>> >> +done
>> >> +
>> >> +# Traffic 10.0.0.1, 10.0.0.2 -> 10.0.0.5 should be dropped.
>> >> +dip=`ip_to_hex 10 0 0 5`
>> >> +for src in `seq 1 2`; do
>> >> +    sip=`ip_to_hex 10 0 0 $src`
>> >> +
>> >> +    test_ip 1 f00000000001 f00000000002 $sip $dip
>> >> +done
>> >> +
>> >> +cat 2.expected > expout
>> >> +$PYTHON "$ovs_srcdir/utilities/ovs-pcap.in <http://ovs-pcap.in>
> <http://ovs-pcap.in>"
>> > hv1/vif2-tx.pcap > 2.packets
>> >> +AT_CHECK([cat 2.packets], [0], [expout])
>> >> +
>> >> +# Re-add the less restrictive allow ACL for src IP 10.0.0.1
>> >> +ovn-nbctl acl-add ls1 to-lport 3 'ip4.src==10.0.0.1' allow
>> >> +ovn-nbctl --wait=hv sync
>> >> +
>> >> +# Check OVS flows, the less restrictive flows should have been installed.
>> >> +AT_CHECK([as hv1 ovs-ofctl dump-flows br-int table=45 | \
>> >> +   grep "priority=1003" | awk '{print $7 " " $8}' | sort], [0], [dnl
>> >> +priority=1003,conj_id=2,ip,metadata=0x1 actions=resubmit(,46)
>> >> +priority=1003,conj_id=3,ip,metadata=0x1 actions=resubmit(,46)
>> >> +priority=1003,ip,metadata=0x1,nw_dst=10.0.0.3
>> > actions=conjunction(2,1/2),conjunction(3,1/2)
>> >> +priority=1003,ip,metadata=0x1,nw_dst=10.0.0.4
>> > actions=conjunction(2,1/2),conjunction(3,1/2)
>> >> +priority=1003,ip,metadata=0x1,nw_src=10.0.0.1 actions=resubmit(,46)
>> >> +priority=1003,ip,metadata=0x1,nw_src=10.0.0.2 actions=conjunction(2,2/2)
>> >> +priority=1003,ip,metadata=0x1,nw_src=10.0.0.42 actions=conjunction(3,2/2)
>> >> +])
>> >> +
>> >> +OVN_CLEANUP([hv1])
>> >> +AT_CLEANUP
>> >> +
>> >>  # 3 hypervisors, one logical switch, 3 logical ports per hypervisor
>> >>  AT_SETUP([ovn -- L2 Drop and Allow ACL w/ Stateful ACL])
>> >>  ovn_start
>> >> --
>> >> 1.8.3.1
>> >>
>>



More information about the dev mailing list