On Wed, Jan 18, 2023 at 8:32 PM Mark Michelson <[email protected]> wrote:

> Thanks Dumitru, it looks good to me.
>
> Acked-by: Mark Michelson <[email protected]>
>
> On 1/18/23 07:48, Dumitru Ceara wrote:
> > Otherwise we cannot correctly implement "allow-related" semantics when
> > default_acl_drop is set to "true".
> >
> > Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1947807#c11
> > 74d82e296f80 ("northd: Support the option to apply from-lport ACLs after
> load balancer.")
> > Signed-off-by: Dumitru Ceara <[email protected]>
> > ---
> > V2: rebased on top of latest main branch.
> > ---
> >   northd/northd.c         |  12 +++-
> >   northd/ovn-northd.8.xml |  14 +++-
> >   tests/ovn-northd.at     |  63 ++++++++++--------
> >   tests/system-ovn.at     | 142 ++++++++++++++++++++++++++++++++++++++++
> >   4 files changed, 199 insertions(+), 32 deletions(-)
> >
> > diff --git a/northd/northd.c b/northd/northd.c
> > index d4744a5bc7..b4b11ae26d 100644
> > --- a/northd/northd.c
> > +++ b/northd/northd.c
> > @@ -216,6 +216,7 @@ enum ovn_stage {
> >   #define REGBIT_FROM_RAMP          "reg0[14]"
> >   #define REGBIT_PORT_SEC_DROP      "reg0[15]"
> >   #define REGBIT_ACL_STATELESS      "reg0[16]"
> > +#define REGBIT_ACL_HINT_ALLOW_REL "reg0[17]"
> >
> >   #define REG_ORIG_DIP_IPV4         "reg1"
> >   #define REG_ORIG_DIP_IPV6         "xxreg1"
> > @@ -6770,7 +6771,8 @@ build_acls(struct ovn_datapath *od, const struct
> chassis_features *features,
> >                         ct_blocked_match);
> >           ovn_lflow_add(lflows, od, S_SWITCH_IN_ACL, UINT16_MAX - 3,
> >                         ds_cstr(&match), REGBIT_ACL_HINT_DROP" = 0; "
> > -                      REGBIT_ACL_HINT_BLOCK" = 0; next;");
> > +                      REGBIT_ACL_HINT_BLOCK" = 0; "
> > +                      REGBIT_ACL_HINT_ALLOW_REL" = 1; next;");
> >           ovn_lflow_add(lflows, od, S_SWITCH_OUT_ACL, UINT16_MAX - 3,
> >                         ds_cstr(&match), "next;");
> >
> > @@ -6791,7 +6793,8 @@ build_acls(struct ovn_datapath *od, const struct
> chassis_features *features,
> >                         use_ct_inv_match ? " && !ct.inv" : "",
> >                         ct_blocked_match);
> >           ovn_lflow_add(lflows, od, S_SWITCH_IN_ACL, UINT16_MAX - 3,
> > -                      ds_cstr(&match), "ct_commit_nat;");
> > +                      ds_cstr(&match),
> > +                      REGBIT_ACL_HINT_ALLOW_REL" = 1; ct_commit_nat;");
> >           ovn_lflow_add(lflows, od, S_SWITCH_OUT_ACL, UINT16_MAX - 3,
> >                         ds_cstr(&match), "ct_commit_nat;");
> >
> > @@ -6802,6 +6805,11 @@ build_acls(struct ovn_datapath *od, const struct
> chassis_features *features,
> >                         "nd || nd_ra || nd_rs || mldv1 || mldv2",
> "next;");
> >           ovn_lflow_add(lflows, od, S_SWITCH_OUT_ACL, UINT16_MAX - 3,
> >                         "nd || nd_ra || nd_rs || mldv1 || mldv2",
> "next;");
> > +
> > +        /* Reply and related traffic matched by an "allow-related" ACL
> > +         * should be allowed in the ls_in_acl_after_lb stage too. */
> > +        ovn_lflow_add(lflows, od, S_SWITCH_IN_ACL_AFTER_LB, UINT16_MAX
> - 3,
> > +                      REGBIT_ACL_HINT_ALLOW_REL" == 1", "next;");
> >       }
> >
> >       /* Ingress or Egress ACL Table (Various priorities). */
> > diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
> > index d8e57b7f07..c0b0d16da6 100644
> > --- a/northd/ovn-northd.8.xml
> > +++ b/northd/ovn-northd.8.xml
> > @@ -794,8 +794,9 @@
> >           policy, <code>ct_mark.blocked</code> will get set and packets
> in the
> >           reply direction will no longer be allowed, either. This flow
> also
> >           clears the register bits <code>reg0[9]</code> and
> > -        <code>reg0[10]</code>.  If ACL logging and logging of related
> packets
> > -        is enabled, then a companion priority-65533 flow will be
> installed that
> > +        <code>reg0[10]</code> and sets register bit
> <code>reg0[17]</code>.
> > +        If ACL logging and logging of related packets is enabled, then a
> > +        companion priority-65533 flow will be installed that
> >           accomplishes the same thing but also logs the traffic.
> >         </li>
> >
> > @@ -1174,6 +1175,15 @@
> >         </li>
> >       </ul>
> >
> > +    <ul>
> > +      <li>
> > +        One priority-65532 flow matching packets with
> <code>reg0[17]</code>
> > +        set (either replies to existing sessions or traffic related to
> > +        existing sessions) and allows these by advancing to the next
> > +        table.
> > +      </li>
> > +    </ul>
> > +
> >       <ul>
> >         <li>
> >           One priority-0 fallback flow that matches all packets and
> advances to
> > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> > index 4cff934e5d..c6fa87ef89 100644
> > --- a/tests/ovn-northd.at
> > +++ b/tests/ovn-northd.at
> > @@ -2472,8 +2472,8 @@ AT_CHECK([ovn-sbctl lflow-list ls | grep -e
> ls_in_acl_hint -e ls_out_acl_hint -e
> >     table=7 (ls_in_acl_hint     ), priority=7    , match=(ct.new &&
> !ct.est), action=(reg0[[7]] = 1; reg0[[9]] = 1; next;)
> >     table=8 (ls_in_acl          ), priority=1    , match=(ip &&
> !ct.est), action=(reg0[[1]] = 1; next;)
> >     table=8 (ls_in_acl          ), priority=1    , match=(ip && ct.est
> && ct_mark.blocked == 1), action=(reg0[[1]] = 1; next;)
> > -  table=8 (ls_in_acl          ), priority=65532, match=(!ct.est &&
> ct.rel && !ct.new && !ct.inv && ct_mark.blocked == 0),
> action=(ct_commit_nat;)
> > -  table=8 (ls_in_acl          ), priority=65532, match=(ct.est &&
> !ct.rel && !ct.new && !ct.inv && ct.rpl && ct_mark.blocked == 0),
> action=(reg0[[9]] = 0; reg0[[10]] = 0; next;)
> > +  table=8 (ls_in_acl          ), priority=65532, match=(!ct.est &&
> ct.rel && !ct.new && !ct.inv && ct_mark.blocked == 0), action=(reg0[[17]] =
> 1; ct_commit_nat;)
> > +  table=8 (ls_in_acl          ), priority=65532, match=(ct.est &&
> !ct.rel && !ct.new && !ct.inv && ct.rpl && ct_mark.blocked == 0),
> action=(reg0[[9]] = 0; reg0[[10]] = 0; reg0[[17]] = 1; next;)
> >     table=8 (ls_in_acl          ), priority=65532, match=(ct.inv ||
> (ct.est && ct.rpl && ct_mark.blocked == 1)), action=(drop;)
> >   ])
> >
> > @@ -2486,6 +2486,7 @@ check ovn-nbctl --wait=sb \
> >
> >   AT_CHECK([ovn-sbctl lflow-list ls | grep -e ls_in_acl_hint -e
> ls_out_acl_hint -e ls_in_acl -e ls_out_acl | sort], [0], [dnl
> >     table=17(ls_in_acl_after_lb ), priority=0    , match=(1),
> action=(next;)
> > +  table=17(ls_in_acl_after_lb ), priority=65532, match=(reg0[[17]] ==
> 1), action=(next;)
> >     table=3 (ls_out_acl_hint    ), priority=0    , match=(1),
> action=(next;)
> >     table=3 (ls_out_acl_hint    ), priority=1    , match=(ct.est &&
> ct_mark.blocked == 0), action=(reg0[[10]] = 1; next;)
> >     table=3 (ls_out_acl_hint    ), priority=2    , match=(ct.est &&
> ct_mark.blocked == 1), action=(reg0[[9]] = 1; next;)
> > @@ -2518,8 +2519,8 @@ AT_CHECK([ovn-sbctl lflow-list ls | grep -e
> ls_in_acl_hint -e ls_out_acl_hint -e
> >     table=8 (ls_in_acl          ), priority=1001 , match=(reg0[[7]] == 1
> && (ip)), action=(reg0[[1]] = 1; next;)
> >     table=8 (ls_in_acl          ), priority=1001 , match=(reg0[[8]] == 1
> && (ip)), action=(next;)
> >     table=8 (ls_in_acl          ), priority=34000, match=(eth.dst ==
> $svc_monitor_mac), action=(next;)
> > -  table=8 (ls_in_acl          ), priority=65532, match=(!ct.est &&
> ct.rel && !ct.new && !ct.inv && ct_mark.blocked == 0),
> action=(ct_commit_nat;)
> > -  table=8 (ls_in_acl          ), priority=65532, match=(ct.est &&
> !ct.rel && !ct.new && !ct.inv && ct.rpl && ct_mark.blocked == 0),
> action=(reg0[[9]] = 0; reg0[[10]] = 0; next;)
> > +  table=8 (ls_in_acl          ), priority=65532, match=(!ct.est &&
> ct.rel && !ct.new && !ct.inv && ct_mark.blocked == 0), action=(reg0[[17]] =
> 1; ct_commit_nat;)
> > +  table=8 (ls_in_acl          ), priority=65532, match=(ct.est &&
> !ct.rel && !ct.new && !ct.inv && ct.rpl && ct_mark.blocked == 0),
> action=(reg0[[9]] = 0; reg0[[10]] = 0; reg0[[17]] = 1; next;)
> >     table=8 (ls_in_acl          ), priority=65532, match=(ct.inv ||
> (ct.est && ct.rpl && ct_mark.blocked == 1)), action=(drop;)
> >     table=8 (ls_in_acl          ), priority=65532, match=(nd || nd_ra ||
> nd_rs || mldv1 || mldv2), action=(next;)
> >   ])
> > @@ -4348,8 +4349,8 @@ ovn-sbctl dump-flows sw0 > sw0flows
> >   AT_CAPTURE_FILE([sw0flows])
> >
> >   AT_CHECK([grep -w "ls_in_acl" sw0flows | grep 6553 | sort | sed
> 's/table=./table=?/'], [0], [dnl
> > -  table=? (ls_in_acl          ), priority=65532, match=(!ct.est &&
> ct.rel && !ct.new && !ct.inv && ct_mark.blocked == 0),
> action=(ct_commit_nat;)
> > -  table=? (ls_in_acl          ), priority=65532, match=(ct.est &&
> !ct.rel && !ct.new && !ct.inv && ct.rpl && ct_mark.blocked == 0),
> action=(reg0[[9]] = 0; reg0[[10]] = 0; next;)
> > +  table=? (ls_in_acl          ), priority=65532, match=(!ct.est &&
> ct.rel && !ct.new && !ct.inv && ct_mark.blocked == 0), action=(reg0[[17]] =
> 1; ct_commit_nat;)
> > +  table=? (ls_in_acl          ), priority=65532, match=(ct.est &&
> !ct.rel && !ct.new && !ct.inv && ct.rpl && ct_mark.blocked == 0),
> action=(reg0[[9]] = 0; reg0[[10]] = 0; reg0[[17]] = 1; next;)
> >     table=? (ls_in_acl          ), priority=65532, match=(ct.inv ||
> (ct.est && ct.rpl && ct_mark.blocked == 1)), action=(drop;)
> >     table=? (ls_in_acl          ), priority=65532, match=(nd || nd_ra ||
> nd_rs || mldv1 || mldv2), action=(next;)
> >   ])
> > @@ -4368,9 +4369,9 @@ ovn-sbctl dump-flows sw0 > sw0flows
> >   AT_CAPTURE_FILE([sw0flows])
> >
> >   AT_CHECK([grep -w "ls_in_acl" sw0flows | grep 6553 | sort | sed
> 's/table=./table=?/'], [0], [dnl
> > -  table=? (ls_in_acl          ), priority=65532, match=(!ct.est &&
> ct.rel && !ct.new && ct_mark.blocked == 0), action=(ct_commit_nat;)
> > +  table=? (ls_in_acl          ), priority=65532, match=(!ct.est &&
> ct.rel && !ct.new && ct_mark.blocked == 0), action=(reg0[[17]] = 1;
> ct_commit_nat;)
> >     table=? (ls_in_acl          ), priority=65532, match=((ct.est &&
> ct.rpl && ct_mark.blocked == 1)), action=(drop;)
> > -  table=? (ls_in_acl          ), priority=65532, match=(ct.est &&
> !ct.rel && !ct.new && ct.rpl && ct_mark.blocked == 0), action=(reg0[[9]] =
> 0; reg0[[10]] = 0; next;)
> > +  table=? (ls_in_acl          ), priority=65532, match=(ct.est &&
> !ct.rel && !ct.new && ct.rpl && ct_mark.blocked == 0), action=(reg0[[9]] =
> 0; reg0[[10]] = 0; reg0[[17]] = 1; next;)
> >     table=? (ls_in_acl          ), priority=65532, match=(nd || nd_ra ||
> nd_rs || mldv1 || mldv2), action=(next;)
> >   ])
> >
> > @@ -4392,8 +4393,8 @@ ovn-sbctl dump-flows sw0 > sw0flows
> >   AT_CAPTURE_FILE([sw0flows])
> >
> >   AT_CHECK([grep -w "ls_in_acl" sw0flows | grep 6553 | sort | sed
> 's/table=./table=?/'], [0], [dnl
> > -  table=? (ls_in_acl          ), priority=65532, match=(!ct.est &&
> ct.rel && !ct.new && !ct.inv && ct_mark.blocked == 0),
> action=(ct_commit_nat;)
> > -  table=? (ls_in_acl          ), priority=65532, match=(ct.est &&
> !ct.rel && !ct.new && !ct.inv && ct.rpl && ct_mark.blocked == 0),
> action=(reg0[[9]] = 0; reg0[[10]] = 0; next;)
> > +  table=? (ls_in_acl          ), priority=65532, match=(!ct.est &&
> ct.rel && !ct.new && !ct.inv && ct_mark.blocked == 0), action=(reg0[[17]] =
> 1; ct_commit_nat;)
> > +  table=? (ls_in_acl          ), priority=65532, match=(ct.est &&
> !ct.rel && !ct.new && !ct.inv && ct.rpl && ct_mark.blocked == 0),
> action=(reg0[[9]] = 0; reg0[[10]] = 0; reg0[[17]] = 1; next;)
> >     table=? (ls_in_acl          ), priority=65532, match=(ct.inv ||
> (ct.est && ct.rpl && ct_mark.blocked == 1)), action=(drop;)
> >     table=? (ls_in_acl          ), priority=65532, match=(nd || nd_ra ||
> nd_rs || mldv1 || mldv2), action=(next;)
> >   ])
> > @@ -6673,11 +6674,12 @@ AT_CHECK([grep -e "ls_in_acl" lsflows | sed
> 's/table=../table=??/' | sort], [0],
> >     table=??(ls_in_acl          ), priority=2004 , match=(reg0[[10]] ==
> 1 && (ip4 && ip4.dst == 10.0.0.2)), action=(ct_commit { ct_mark.blocked =
> 1; }; /* drop */)
> >     table=??(ls_in_acl          ), priority=2004 , match=(reg0[[9]] == 1
> && (ip4 && ip4.dst == 10.0.0.2)), action=(/* drop */)
> >     table=??(ls_in_acl          ), priority=34000, match=(eth.dst ==
> $svc_monitor_mac), action=(next;)
> > -  table=??(ls_in_acl          ), priority=65532, match=(!ct.est &&
> ct.rel && !ct.new && !ct.inv && ct_mark.blocked == 0),
> action=(ct_commit_nat;)
> > -  table=??(ls_in_acl          ), priority=65532, match=(ct.est &&
> !ct.rel && !ct.new && !ct.inv && ct.rpl && ct_mark.blocked == 0),
> action=(reg0[[9]] = 0; reg0[[10]] = 0; next;)
> > +  table=??(ls_in_acl          ), priority=65532, match=(!ct.est &&
> ct.rel && !ct.new && !ct.inv && ct_mark.blocked == 0), action=(reg0[[17]] =
> 1; ct_commit_nat;)
> > +  table=??(ls_in_acl          ), priority=65532, match=(ct.est &&
> !ct.rel && !ct.new && !ct.inv && ct.rpl && ct_mark.blocked == 0),
> action=(reg0[[9]] = 0; reg0[[10]] = 0; reg0[[17]] = 1; next;)
> >     table=??(ls_in_acl          ), priority=65532, match=(ct.inv ||
> (ct.est && ct.rpl && ct_mark.blocked == 1)), action=(drop;)
> >     table=??(ls_in_acl          ), priority=65532, match=(nd || nd_ra ||
> nd_rs || mldv1 || mldv2), action=(next;)
> >     table=??(ls_in_acl_after_lb ), priority=0    , match=(1),
> action=(next;)
> > +  table=??(ls_in_acl_after_lb ), priority=65532, match=(reg0[[17]] ==
> 1), action=(next;)
> >     table=??(ls_in_acl_hint     ), priority=0    , match=(1),
> action=(next;)
> >     table=??(ls_in_acl_hint     ), priority=1    , match=(ct.est &&
> ct_mark.blocked == 0), action=(reg0[[10]] = 1; next;)
> >     table=??(ls_in_acl_hint     ), priority=2    , match=(ct.est &&
> ct_mark.blocked == 1), action=(reg0[[9]] = 1; next;)
> > @@ -6718,8 +6720,8 @@ AT_CHECK([grep -e "ls_in_acl" lsflows | sed
> 's/table=../table=??/' | sort], [0],
> >     table=??(ls_in_acl          ), priority=1    , match=(ip &&
> !ct.est), action=(reg0[[1]] = 1; next;)
> >     table=??(ls_in_acl          ), priority=1    , match=(ip && ct.est
> && ct_mark.blocked == 1), action=(reg0[[1]] = 1; next;)
> >     table=??(ls_in_acl          ), priority=34000, match=(eth.dst ==
> $svc_monitor_mac), action=(next;)
> > -  table=??(ls_in_acl          ), priority=65532, match=(!ct.est &&
> ct.rel && !ct.new && !ct.inv && ct_mark.blocked == 0),
> action=(ct_commit_nat;)
> > -  table=??(ls_in_acl          ), priority=65532, match=(ct.est &&
> !ct.rel && !ct.new && !ct.inv && ct.rpl && ct_mark.blocked == 0),
> action=(reg0[[9]] = 0; reg0[[10]] = 0; next;)
> > +  table=??(ls_in_acl          ), priority=65532, match=(!ct.est &&
> ct.rel && !ct.new && !ct.inv && ct_mark.blocked == 0), action=(reg0[[17]] =
> 1; ct_commit_nat;)
> > +  table=??(ls_in_acl          ), priority=65532, match=(ct.est &&
> !ct.rel && !ct.new && !ct.inv && ct.rpl && ct_mark.blocked == 0),
> action=(reg0[[9]] = 0; reg0[[10]] = 0; reg0[[17]] = 1; next;)
> >     table=??(ls_in_acl          ), priority=65532, match=(ct.inv ||
> (ct.est && ct.rpl && ct_mark.blocked == 1)), action=(drop;)
> >     table=??(ls_in_acl          ), priority=65532, match=(nd || nd_ra ||
> nd_rs || mldv1 || mldv2), action=(next;)
> >     table=??(ls_in_acl_after_lb ), priority=0    , match=(1),
> action=(next;)
> > @@ -6731,6 +6733,7 @@ AT_CHECK([grep -e "ls_in_acl" lsflows | sed
> 's/table=../table=??/' | sort], [0],
> >     table=??(ls_in_acl_after_lb ), priority=2003 , match=(reg0[[8]] == 1
> && (ip4 && icmp)), action=(next;)
> >     table=??(ls_in_acl_after_lb ), priority=2004 , match=(reg0[[10]] ==
> 1 && (ip4 && ip4.dst == 10.0.0.2)), action=(ct_commit { ct_mark.blocked =
> 1; }; /* drop */)
> >     table=??(ls_in_acl_after_lb ), priority=2004 , match=(reg0[[9]] == 1
> && (ip4 && ip4.dst == 10.0.0.2)), action=(/* drop */)
> > +  table=??(ls_in_acl_after_lb ), priority=65532, match=(reg0[[17]] ==
> 1), action=(next;)
> >     table=??(ls_in_acl_hint     ), priority=0    , match=(1),
> action=(next;)
> >     table=??(ls_in_acl_hint     ), priority=1    , match=(ct.est &&
> ct_mark.blocked == 0), action=(reg0[[10]] = 1; next;)
> >     table=??(ls_in_acl_hint     ), priority=2    , match=(ct.est &&
> ct_mark.blocked == 1), action=(reg0[[9]] = 1; next;)
> > @@ -6775,8 +6778,8 @@ AT_CHECK([grep -e "ls_in_acl" lsflows | sed
> 's/table=../table=??/' | sort], [0],
> >     table=??(ls_in_acl          ), priority=2003 , match=(reg0[[7]] == 1
> && (ip4 && icmp)), action=(reg0[[1]] = 1; next;)
> >     table=??(ls_in_acl          ), priority=2003 , match=(reg0[[8]] == 1
> && (ip4 && icmp)), action=(next;)
> >     table=??(ls_in_acl          ), priority=34000, match=(eth.dst ==
> $svc_monitor_mac), action=(next;)
> > -  table=??(ls_in_acl          ), priority=65532, match=(!ct.est &&
> ct.rel && !ct.new && !ct.inv && ct_mark.blocked == 0),
> action=(ct_commit_nat;)
> > -  table=??(ls_in_acl          ), priority=65532, match=(ct.est &&
> !ct.rel && !ct.new && !ct.inv && ct.rpl && ct_mark.blocked == 0),
> action=(reg0[[9]] = 0; reg0[[10]] = 0; next;)
> > +  table=??(ls_in_acl          ), priority=65532, match=(!ct.est &&
> ct.rel && !ct.new && !ct.inv && ct_mark.blocked == 0), action=(reg0[[17]] =
> 1; ct_commit_nat;)
> > +  table=??(ls_in_acl          ), priority=65532, match=(ct.est &&
> !ct.rel && !ct.new && !ct.inv && ct.rpl && ct_mark.blocked == 0),
> action=(reg0[[9]] = 0; reg0[[10]] = 0; reg0[[17]] = 1; next;)
> >     table=??(ls_in_acl          ), priority=65532, match=(ct.inv ||
> (ct.est && ct.rpl && ct_mark.blocked == 1)), action=(drop;)
> >     table=??(ls_in_acl          ), priority=65532, match=(nd || nd_ra ||
> nd_rs || mldv1 || mldv2), action=(next;)
> >     table=??(ls_in_acl_after_lb ), priority=0    , match=(1),
> action=(next;)
> > @@ -6784,6 +6787,7 @@ AT_CHECK([grep -e "ls_in_acl" lsflows | sed
> 's/table=../table=??/' | sort], [0],
> >     table=??(ls_in_acl_after_lb ), priority=2001 , match=(reg0[[9]] == 1
> && (ip4)), action=(/* drop */)
> >     table=??(ls_in_acl_after_lb ), priority=2004 , match=(reg0[[10]] ==
> 1 && (ip4 && ip4.dst == 10.0.0.2)), action=(ct_commit { ct_mark.blocked =
> 1; }; /* drop */)
> >     table=??(ls_in_acl_after_lb ), priority=2004 , match=(reg0[[9]] == 1
> && (ip4 && ip4.dst == 10.0.0.2)), action=(/* drop */)
> > +  table=??(ls_in_acl_after_lb ), priority=65532, match=(reg0[[17]] ==
> 1), action=(next;)
> >     table=??(ls_in_acl_hint     ), priority=0    , match=(1),
> action=(next;)
> >     table=??(ls_in_acl_hint     ), priority=1    , match=(ct.est &&
> ct_mark.blocked == 0), action=(reg0[[10]] = 1; next;)
> >     table=??(ls_in_acl_hint     ), priority=2    , match=(ct.est &&
> ct_mark.blocked == 1), action=(reg0[[9]] = 1; next;)
> > @@ -7207,11 +7211,12 @@ AT_CHECK([ovn-sbctl dump-flows | grep -E
> "ls_.*_acl" | sed 's/table=../table=??/
> >     table=??(ls_in_acl          ), priority=1001 , match=(reg0[[7]] == 1
> && (ip4 && tcp)), action=(reg0[[1]] = 1; next;)
> >     table=??(ls_in_acl          ), priority=1001 , match=(reg0[[8]] == 1
> && (ip4 && tcp)), action=(next;)
> >     table=??(ls_in_acl          ), priority=34000, match=(eth.dst ==
> $svc_monitor_mac), action=(next;)
> > -  table=??(ls_in_acl          ), priority=65532, match=(!ct.est &&
> ct.rel && !ct.new && !ct.inv && ct_mark.blocked == 0),
> action=(ct_commit_nat;)
> > -  table=??(ls_in_acl          ), priority=65532, match=(ct.est &&
> !ct.rel && !ct.new && !ct.inv && ct.rpl && ct_mark.blocked == 0),
> action=(reg0[[9]] = 0; reg0[[10]] = 0; next;)
> > +  table=??(ls_in_acl          ), priority=65532, match=(!ct.est &&
> ct.rel && !ct.new && !ct.inv && ct_mark.blocked == 0), action=(reg0[[17]] =
> 1; ct_commit_nat;)
> > +  table=??(ls_in_acl          ), priority=65532, match=(ct.est &&
> !ct.rel && !ct.new && !ct.inv && ct.rpl && ct_mark.blocked == 0),
> action=(reg0[[9]] = 0; reg0[[10]] = 0; reg0[[17]] = 1; next;)
> >     table=??(ls_in_acl          ), priority=65532, match=(ct.inv ||
> (ct.est && ct.rpl && ct_mark.blocked == 1)), action=(drop;)
> >     table=??(ls_in_acl          ), priority=65532, match=(nd || nd_ra ||
> nd_rs || mldv1 || mldv2), action=(next;)
> >     table=??(ls_in_acl_after_lb ), priority=0    , match=(1),
> action=(drop;)
> > +  table=??(ls_in_acl_after_lb ), priority=65532, match=(reg0[[17]] ==
> 1), action=(next;)
> >     table=??(ls_in_acl_hint     ), priority=0    , match=(1),
> action=(next;)
> >     table=??(ls_in_acl_hint     ), priority=1    , match=(ct.est &&
> ct_mark.blocked == 0), action=(reg0[[10]] = 1; next;)
> >     table=??(ls_in_acl_hint     ), priority=2    , match=(ct.est &&
> ct_mark.blocked == 1), action=(reg0[[9]] = 1; next;)
> > @@ -7330,13 +7335,14 @@ AT_CHECK([ovn-sbctl dump-flows | grep -E
> "ls_.*_acl" | sed 's/table=../table=??/
> >     table=??(ls_in_acl          ), priority=1    , match=(ip &&
> !ct.est), action=(drop;)
> >     table=??(ls_in_acl          ), priority=1    , match=(ip && ct.est
> && ct_mark.blocked == 1), action=(reg0[[1]] = 1; next;)
> >     table=??(ls_in_acl          ), priority=34000, match=(eth.dst ==
> $svc_monitor_mac), action=(next;)
> > -  table=??(ls_in_acl          ), priority=65532, match=(!ct.est &&
> ct.rel && !ct.new && !ct.inv && ct_mark.blocked == 0),
> action=(ct_commit_nat;)
> > -  table=??(ls_in_acl          ), priority=65532, match=(ct.est &&
> !ct.rel && !ct.new && !ct.inv && ct.rpl && ct_mark.blocked == 0),
> action=(reg0[[9]] = 0; reg0[[10]] = 0; next;)
> > +  table=??(ls_in_acl          ), priority=65532, match=(!ct.est &&
> ct.rel && !ct.new && !ct.inv && ct_mark.blocked == 0), action=(reg0[[17]] =
> 1; ct_commit_nat;)
> > +  table=??(ls_in_acl          ), priority=65532, match=(ct.est &&
> !ct.rel && !ct.new && !ct.inv && ct.rpl && ct_mark.blocked == 0),
> action=(reg0[[9]] = 0; reg0[[10]] = 0; reg0[[17]] = 1; next;)
> >     table=??(ls_in_acl          ), priority=65532, match=(ct.inv ||
> (ct.est && ct.rpl && ct_mark.blocked == 1)), action=(drop;)
> >     table=??(ls_in_acl          ), priority=65532, match=(nd || nd_ra ||
> nd_rs || mldv1 || mldv2), action=(next;)
> >     table=??(ls_in_acl_after_lb ), priority=0    , match=(1),
> action=(drop;)
> >     table=??(ls_in_acl_after_lb ), priority=1001 , match=(reg0[[7]] == 1
> && (ip4 && tcp)), action=(reg0[[1]] = 1; next;)
> >     table=??(ls_in_acl_after_lb ), priority=1001 , match=(reg0[[8]] == 1
> && (ip4 && tcp)), action=(next;)
> > +  table=??(ls_in_acl_after_lb ), priority=65532, match=(reg0[[17]] ==
> 1), action=(next;)
> >     table=??(ls_in_acl_hint     ), priority=0    , match=(1),
> action=(next;)
> >     table=??(ls_in_acl_hint     ), priority=1    , match=(ct.est &&
> ct_mark.blocked == 0), action=(reg0[[10]] = 1; next;)
> >     table=??(ls_in_acl_hint     ), priority=2    , match=(ct.est &&
> ct_mark.blocked == 1), action=(reg0[[9]] = 1; next;)
> > @@ -7455,11 +7461,12 @@ AT_CHECK([ovn-sbctl dump-flows | grep -E
> "ls_.*_acl" | sed 's/table=../table=??/
> >     table=??(ls_in_acl          ), priority=1    , match=(ip &&
> !ct.est), action=(drop;)
> >     table=??(ls_in_acl          ), priority=1    , match=(ip && ct.est
> && ct_mark.blocked == 1), action=(reg0[[1]] = 1; next;)
> >     table=??(ls_in_acl          ), priority=34000, match=(eth.dst ==
> $svc_monitor_mac), action=(next;)
> > -  table=??(ls_in_acl          ), priority=65532, match=(!ct.est &&
> ct.rel && !ct.new && !ct.inv && ct_mark.blocked == 0),
> action=(ct_commit_nat;)
> > -  table=??(ls_in_acl          ), priority=65532, match=(ct.est &&
> !ct.rel && !ct.new && !ct.inv && ct.rpl && ct_mark.blocked == 0),
> action=(reg0[[9]] = 0; reg0[[10]] = 0; next;)
> > +  table=??(ls_in_acl          ), priority=65532, match=(!ct.est &&
> ct.rel && !ct.new && !ct.inv && ct_mark.blocked == 0), action=(reg0[[17]] =
> 1; ct_commit_nat;)
> > +  table=??(ls_in_acl          ), priority=65532, match=(ct.est &&
> !ct.rel && !ct.new && !ct.inv && ct.rpl && ct_mark.blocked == 0),
> action=(reg0[[9]] = 0; reg0[[10]] = 0; reg0[[17]] = 1; next;)
> >     table=??(ls_in_acl          ), priority=65532, match=(ct.inv ||
> (ct.est && ct.rpl && ct_mark.blocked == 1)), action=(drop;)
> >     table=??(ls_in_acl          ), priority=65532, match=(nd || nd_ra ||
> nd_rs || mldv1 || mldv2), action=(next;)
> >     table=??(ls_in_acl_after_lb ), priority=0    , match=(1),
> action=(drop;)
> > +  table=??(ls_in_acl_after_lb ), priority=65532, match=(reg0[[17]] ==
> 1), action=(next;)
> >     table=??(ls_in_acl_hint     ), priority=0    , match=(1),
> action=(next;)
> >     table=??(ls_in_acl_hint     ), priority=1    , match=(ct.est &&
> ct_mark.blocked == 0), action=(reg0[[10]] = 1; next;)
> >     table=??(ls_in_acl_hint     ), priority=2    , match=(ct.est &&
> ct_mark.blocked == 1), action=(reg0[[9]] = 1; next;)
> > @@ -7856,8 +7863,8 @@ AT_CHECK([ovn-sbctl lflow-list | grep
> 'ls.*acl.*blocked' ], [0], [dnl
> >     table=7 (ls_in_acl_hint     ), priority=4    , match=(!ct.new &&
> ct.est && !ct.rpl && ct_mark.blocked == 0), action=(reg0[[8]] = 1;
> reg0[[10]] = 1; next;)
> >     table=7 (ls_in_acl_hint     ), priority=2    , match=(ct.est &&
> ct_mark.blocked == 1), action=(reg0[[9]] = 1; next;)
> >     table=7 (ls_in_acl_hint     ), priority=1    , match=(ct.est &&
> ct_mark.blocked == 0), action=(reg0[[10]] = 1; next;)
> > -  table=8 (ls_in_acl          ), priority=65532, match=(!ct.est &&
> ct.rel && !ct.new && !ct.inv && ct_mark.blocked == 0),
> action=(ct_commit_nat;)
> > -  table=8 (ls_in_acl          ), priority=65532, match=(ct.est &&
> !ct.rel && !ct.new && !ct.inv && ct.rpl && ct_mark.blocked == 0),
> action=(reg0[[9]] = 0; reg0[[10]] = 0; next;)
> > +  table=8 (ls_in_acl          ), priority=65532, match=(!ct.est &&
> ct.rel && !ct.new && !ct.inv && ct_mark.blocked == 0), action=(reg0[[17]] =
> 1; ct_commit_nat;)
> > +  table=8 (ls_in_acl          ), priority=65532, match=(ct.est &&
> !ct.rel && !ct.new && !ct.inv && ct.rpl && ct_mark.blocked == 0),
> action=(reg0[[9]] = 0; reg0[[10]] = 0; reg0[[17]] = 1; next;)
> >     table=8 (ls_in_acl          ), priority=65532, match=(ct.inv ||
> (ct.est && ct.rpl && ct_mark.blocked == 1)), action=(drop;)
> >     table=8 (ls_in_acl          ), priority=1    , match=(ip && ct.est
> && ct_mark.blocked == 1), action=(reg0[[1]] = 1; next;)
> >     table=3 (ls_out_acl_hint    ), priority=6    , match=(!ct.new &&
> ct.est && !ct.rpl && ct_mark.blocked == 1), action=(reg0[[7]] = 1;
> reg0[[9]] = 1; next;)
> > @@ -7878,8 +7885,8 @@ AT_CHECK([ovn-sbctl lflow-list | grep
> 'ls.*acl.*blocked' ], [0], [dnl
> >     table=7 (ls_in_acl_hint     ), priority=4    , match=(!ct.new &&
> ct.est && !ct.rpl && ct_label.blocked == 0), action=(reg0[[8]] = 1;
> reg0[[10]] = 1; next;)
> >     table=7 (ls_in_acl_hint     ), priority=2    , match=(ct.est &&
> ct_label.blocked == 1), action=(reg0[[9]] = 1; next;)
> >     table=7 (ls_in_acl_hint     ), priority=1    , match=(ct.est &&
> ct_label.blocked == 0), action=(reg0[[10]] = 1; next;)
> > -  table=8 (ls_in_acl          ), priority=65532, match=(!ct.est &&
> ct.rel && !ct.new && !ct.inv && ct_label.blocked == 0),
> action=(ct_commit_nat;)
> > -  table=8 (ls_in_acl          ), priority=65532, match=(ct.est &&
> !ct.rel && !ct.new && !ct.inv && ct.rpl && ct_label.blocked == 0),
> action=(reg0[[9]] = 0; reg0[[10]] = 0; next;)
> > +  table=8 (ls_in_acl          ), priority=65532, match=(!ct.est &&
> ct.rel && !ct.new && !ct.inv && ct_label.blocked == 0), action=(reg0[[17]]
> = 1; ct_commit_nat;)
> > +  table=8 (ls_in_acl          ), priority=65532, match=(ct.est &&
> !ct.rel && !ct.new && !ct.inv && ct.rpl && ct_label.blocked == 0),
> action=(reg0[[9]] = 0; reg0[[10]] = 0; reg0[[17]] = 1; next;)
> >     table=8 (ls_in_acl          ), priority=65532, match=(ct.inv ||
> (ct.est && ct.rpl && ct_label.blocked == 1)), action=(drop;)
> >     table=8 (ls_in_acl          ), priority=1    , match=(ip && ct.est
> && ct_label.blocked == 1), action=(reg0[[1]] = 1; next;)
> >     table=3 (ls_out_acl_hint    ), priority=6    , match=(!ct.new &&
> ct.est && !ct.rpl && ct_label.blocked == 1), action=(reg0[[7]] = 1;
> reg0[[9]] = 1; next;)
> > @@ -7900,8 +7907,8 @@ AT_CHECK([ovn-sbctl lflow-list | grep
> 'ls.*acl.*blocked' ], [0], [dnl
> >     table=7 (ls_in_acl_hint     ), priority=4    , match=(!ct.new &&
> ct.est && !ct.rpl && ct_mark.blocked == 0), action=(reg0[[8]] = 1;
> reg0[[10]] = 1; next;)
> >     table=7 (ls_in_acl_hint     ), priority=2    , match=(ct.est &&
> ct_mark.blocked == 1), action=(reg0[[9]] = 1; next;)
> >     table=7 (ls_in_acl_hint     ), priority=1    , match=(ct.est &&
> ct_mark.blocked == 0), action=(reg0[[10]] = 1; next;)
> > -  table=8 (ls_in_acl          ), priority=65532, match=(!ct.est &&
> ct.rel && !ct.new && !ct.inv && ct_mark.blocked == 0),
> action=(ct_commit_nat;)
> > -  table=8 (ls_in_acl          ), priority=65532, match=(ct.est &&
> !ct.rel && !ct.new && !ct.inv && ct.rpl && ct_mark.blocked == 0),
> action=(reg0[[9]] = 0; reg0[[10]] = 0; next;)
> > +  table=8 (ls_in_acl          ), priority=65532, match=(!ct.est &&
> ct.rel && !ct.new && !ct.inv && ct_mark.blocked == 0), action=(reg0[[17]] =
> 1; ct_commit_nat;)
> > +  table=8 (ls_in_acl          ), priority=65532, match=(ct.est &&
> !ct.rel && !ct.new && !ct.inv && ct.rpl && ct_mark.blocked == 0),
> action=(reg0[[9]] = 0; reg0[[10]] = 0; reg0[[17]] = 1; next;)
> >     table=8 (ls_in_acl          ), priority=65532, match=(ct.inv ||
> (ct.est && ct.rpl && ct_mark.blocked == 1)), action=(drop;)
> >     table=8 (ls_in_acl          ), priority=1    , match=(ip && ct.est
> && ct_mark.blocked == 1), action=(reg0[[1]] = 1; next;)
> >     table=3 (ls_out_acl_hint    ), priority=6    , match=(!ct.new &&
> ct.est && !ct.rpl && ct_mark.blocked == 1), action=(reg0[[7]] = 1;
> reg0[[9]] = 1; next;)
> > diff --git a/tests/system-ovn.at b/tests/system-ovn.at
> > index 8e409c46e5..86cd1789e3 100644
> > --- a/tests/system-ovn.at
> > +++ b/tests/system-ovn.at
> > @@ -10159,3 +10159,145 @@ OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query
> port patch-.*/d
> >   /connection dropped.*/d"])
> >   AT_CLEANUP
> >   ])
> > +
> > +OVN_FOR_EACH_NORTHD([
> > +AT_SETUP([ACL default_acl_drop])
> > +AT_KEYWORDS([acl default_acl_drop])
> > +
> > +CHECK_CONNTRACK()
> > +ovn_start
> > +
> > +OVS_TRAFFIC_VSWITCHD_START()
> > +ADD_BR([br-int])
> > +
> > +# Set external-ids in br-int needed for ovn-controller
> > +ovs-vsctl \
> > +        -- set Open_vSwitch . external-ids:system-id=hv1 \
> > +        -- set Open_vSwitch .
> external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \
> > +        -- set Open_vSwitch . external-ids:ovn-encap-type=geneve \
> > +        -- set Open_vSwitch . external-ids:ovn-encap-ip=169.0.0.1 \
> > +        -- set bridge br-int fail-mode=secure
> other-config:disable-in-band=true
> > +
> > +# Start ovn-controller
> > +start_daemon ovn-controller
> > +
> > +ovn-nbctl ls-add sw
> > +
> > +# Logical port 'vm1' in switch 'sw'.
> > +ADD_NAMESPACES(vm1)
> > +ADD_VETH(vm1, vm1, br-int, "10.0.0.1/24", "f0:00:00:01:02:03", \
> > +         "10.0.0.254")
> > +check ovn-nbctl lsp-add sw vm1 \
> > +-- lsp-set-addresses vm1 "f0:00:00:01:02:03 10.0.0.1"
> > +
> > +# Logical port 'vm2' in switch 'sw'.
> > +ADD_NAMESPACES(vm2)
> > +ADD_VETH(vm2, vm2, br-int, "10.0.0.2/24", "f0:00:00:01:02:05", \
> > +"10.0.0.254")
> > +check ovn-nbctl lsp-add sw vm2 \
> > +-- lsp-set-addresses vm2 "f0:00:00:01:02:05 10.0.0.2"
> > +
> > +# Wait for ovn-controller to catch up.
> > +wait_for_ports_up
> > +check ovn-nbctl --wait=hv sync
> > +
> > +AS_BOX([from-lport acl, default_acl_drop false])
> > +check ovn-nbctl acl-del sw
> > +check ovn-nbctl set NB_Global . options:default_acl_drop=false \
> > +    -- acl-add sw from-lport 20 "ip4 && icmp" allow-related \
> > +    -- acl-add sw from-lport 10 "ip4" drop
> > +check ovn-nbctl --wait=hv sync
> > +
> > +# 'vm1' should be able to ping 'vm2' directly.
> > +NS_CHECK_EXEC([vm1], [ping -q -c 3 -i 0.3 -w 2 10.0.0.2 | FORMAT_PING],
> \
> > +[0], [dnl
> > +3 packets transmitted, 3 received, 0% packet loss, time 0ms
> > +])
> > +
> > +AS_BOX([from-lport acl, default_acl_drop true])
> > +check ovn-nbctl acl-del sw
> > +check ovn-nbctl set NB_Global . options:default_acl_drop=true \
> > +    -- acl-add sw from-lport 20 "ip4 && icmp" allow-related \
> > +    -- acl-add sw from-lport 10 "arp" allow \
> > +    -- --apply-after-lb acl-add sw from-lport 1 1 allow \
> > +    -- acl-add sw to-lport 1 1 allow
> > +check ovn-nbctl --wait=hv sync
> > +
> > +# 'vm1' should be able to ping 'vm2' directly.
> > +NS_CHECK_EXEC([vm1], [ping -q -c 3 -i 0.3 -w 2 10.0.0.2 | FORMAT_PING],
> \
> > +[0], [dnl
> > +3 packets transmitted, 3 received, 0% packet loss, time 0ms
> > +])
> > +
> > +AS_BOX([from-lport acl, after LB, default_acl_drop false])
> > +check ovn-nbctl acl-del sw
> > +check ovn-nbctl set NB_Global . options:default_acl_drop=false \
> > +    -- --apply-after-lb acl-add sw from-lport 20 "ip4 && icmp"
> allow-related \
> > +    -- --apply-after-lb acl-add sw from-lport 10 "ip4" drop
> > +check ovn-nbctl --wait=hv sync
> > +
> > +# 'vm1' should be able to ping 'vm2' directly.
> > +NS_CHECK_EXEC([vm1], [ping -q -c 3 -i 0.3 -w 2 10.0.0.2 | FORMAT_PING],
> \
> > +[0], [dnl
> > +3 packets transmitted, 3 received, 0% packet loss, time 0ms
> > +])
> > +
> > +AS_BOX([from-lport acl, after LB, default_acl_drop true])
> > +check ovn-nbctl acl-del sw
> > +check ovn-nbctl set NB_Global . options:default_acl_drop=true \
> > +    -- acl-add sw from-lport 1 1 allow \
> > +    -- --apply-after-lb acl-add sw from-lport 20 "ip4 && icmp"
> allow-related \
> > +    -- --apply-after-lb acl-add sw from-lport 20 "arp" allow-related \
> > +    -- acl-add sw to-lport 1 1 allow
> > +check ovn-nbctl --wait=hv sync
> > +
> > +# 'vm1' should be able to ping 'vm2' directly.
> > +NS_CHECK_EXEC([vm1], [ping -q -c 3 -i 0.3 -w 2 10.0.0.2 | FORMAT_PING],
> \
> > +[0], [dnl
> > +3 packets transmitted, 3 received, 0% packet loss, time 0ms
> > +])
> > +
> > +AS_BOX([to-lport acl, default_acl_drop false])
> > +check ovn-nbctl acl-del sw
> > +check ovn-nbctl set NB_Global . options:default_acl_drop=false \
> > +    -- acl-add sw to-lport 20 "ip4 && icmp" allow-related \
> > +    -- acl-add sw to-lport 10 "ip4" drop
> > +check ovn-nbctl --wait=hv sync
> > +
> > +# 'vm1' should be able to ping 'vm2' directly.
> > +NS_CHECK_EXEC([vm1], [ping -q -c 3 -i 0.3 -w 2 10.0.0.2 | FORMAT_PING],
> \
> > +[0], [dnl
> > +3 packets transmitted, 3 received, 0% packet loss, time 0ms
> > +])
> > +
> > +AS_BOX([to-lport acl, default_acl_drop true])
> > +check ovn-nbctl acl-del sw
> > +check ovn-nbctl set NB_Global . options:default_acl_drop=true \
> > +    -- acl-add sw from-lport 1 1 allow \
> > +    -- --apply-after-lb acl-add sw from-lport 1 1 allow \
> > +    -- acl-add sw to-lport 20 "ip4 && icmp" allow-related \
> > +    -- acl-add sw to-lport 20 "arp" allow
> > +check ovn-nbctl --wait=hv sync
> > +
> > +# 'vm1' should be able to ping 'vm2' directly.
> > +NS_CHECK_EXEC([vm1], [ping -q -c 3 -i 0.3 -w 2 10.0.0.2 | FORMAT_PING],
> \
> > +[0], [dnl
> > +3 packets transmitted, 3 received, 0% packet loss, time 0ms
> > +])
> > +
> > +OVS_APP_EXIT_AND_WAIT([ovn-controller])
> > +
> > +as ovn-sb
> > +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
> > +
> > +as ovn-nb
> > +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
> > +
> > +as northd
> > +OVS_APP_EXIT_AND_WAIT([NORTHD_TYPE])
> > +
> > +as
> > +OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d
> > +/connection dropped.*/d"])
> > +AT_CLEANUP
> > +])
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
Looks good to me, thanks.
It will need a rebase because of the CT related feature flag.

Acked-by: Ales Musil <[email protected]>

-- 

Ales Musil

Senior Software Engineer - OVN Core

Red Hat EMEA <https://www.redhat.com>

[email protected]    IM: amusil
<https://red.ht/sig>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to