On Fri, Sep 17, 2021 at 11:01 AM Vladislav Odintsov <odiv...@gmail.com> wrote:
>
> A packet going from HW VTEP device to VIF port when arrives to
> hypervisor chassis should go through LS ingress pipeline to l2_lkp
> stage without any match. In l2_lkp stage an output port is
> determined and then packet passed to LS egress pipeline for futher
> processing and to VIF port delivery.
>
> Prior to this commit a packet, which was received from HW VTEP
> device was dropped in an LS ingress datapath, where stateful services
> were defined (ACLs, LBs).
>
> To fix this issue we add a special flag-bit which can be used in LS
> pipelines, to check whether the packet came from HW VTEP devices.
> In ls_in_pre_acl and ls_in_pre_lb we add new flow with priority 110
> to skip such packets.
>
> Signed-off-by: Vladislav Odintsov <odiv...@gmail.com>

Hi Vladislav,

The documentation needs to be updated for the newly added logical flows
and also for the updated flow in "ls_in_port_sec_l2" stage.

I didn't review your p1 earlier and hence missed it.

Regarding the patch,  I don't have much idea on how the ramp/vtep stuff works.
My question is why the traffic received from HW VTEP devices should be skipped
from conntrack ?  ACLs and policies don't apply to vtep logical ports ?

Thanks
Numan

> ---
> v1 -> v2:
>    - Patch rebased on upstream changes.
>
> Please note: I've got no experience in DDLog and have no ability to 
> extensively
>              test these changes.
>              Just local ./configure --with-ddlog=...; make; make check was run
>              It seems, that only irrelevant to these changes tests were 
> failed.
> ---
>  northd/northd.c      | 14 ++++++++++++++
>  northd/ovn_northd.dl | 33 +++++++++++++++++++++++++++++++--
>  tests/ovn-northd.at  |  2 ++
>  3 files changed, 47 insertions(+), 2 deletions(-)
>
> diff --git a/northd/northd.c b/northd/northd.c
> index 688a6e4ef..1b84874a7 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -196,6 +196,7 @@ enum ovn_stage {
>  #define REGBIT_LKUP_FDB           "reg0[11]"
>  #define REGBIT_HAIRPIN_REPLY      "reg0[12]"
>  #define REGBIT_ACL_LABEL          "reg0[13]"
> +#define REGBIT_FROM_RAMP          "reg0[14]"
>
>  #define REG_ORIG_DIP_IPV4         "reg1"
>  #define REG_ORIG_DIP_IPV6         "xxreg1"
> @@ -5112,6 +5113,11 @@ build_lswitch_input_port_sec_op(
>      if (queue_id) {
>          ds_put_format(actions, "set_queue(%s); ", queue_id);
>      }
> +
> +    if (!strcmp(op->nbsp->type, "vtep")) {
> +        ds_put_format(actions, REGBIT_FROM_RAMP" = 1; ");
> +    }
> +
>      ds_put_cstr(actions, "next;");
>      ovn_lflow_add_with_lport_and_hint(lflows, op->od, 
> S_SWITCH_IN_PORT_SEC_L2,
>                                        50, ds_cstr(match), ds_cstr(actions),
> @@ -5359,6 +5365,10 @@ build_pre_acls(struct ovn_datapath *od, struct hmap 
> *port_groups,
>                        "nd || nd_rs || nd_ra || mldv1 || mldv2 || "
>                        "(udp && udp.src == 546 && udp.dst == 547)", "next;");
>
> +        /* Do not send coming from RAMP switch packets to conntrack. */
> +        ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_ACL, 110,
> +                      REGBIT_FROM_RAMP" == 1", "next;");
> +
>          /* Ingress and Egress Pre-ACL Table (Priority 100).
>           *
>           * Regardless of whether the ACL is "from-lport" or "to-lport",
> @@ -5463,6 +5473,10 @@ build_pre_lb(struct ovn_datapath *od, struct hmap 
> *lflows,
>      ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_LB, 110,
>                    "eth.src == $svc_monitor_mac", "next;");
>
> +    /* Do not send coming from RAMP switch packets to conntrack. */
> +    ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_LB, 110,
> +                  REGBIT_FROM_RAMP" == 1", "next;");
> +
>      /* Allow all packets to go to next tables by default. */
>      ovn_lflow_add(lflows, od, S_SWITCH_IN_PRE_LB, 0, "1", "next;");
>      ovn_lflow_add(lflows, od, S_SWITCH_OUT_PRE_LB, 0, "1", "next;");
> diff --git a/northd/ovn_northd.dl b/northd/ovn_northd.dl
> index 669728497..0202af5dc 100644
> --- a/northd/ovn_northd.dl
> +++ b/northd/ovn_northd.dl
> @@ -1631,6 +1631,7 @@ function rEGBIT_ACL_HINT_BLOCK()   : istring = 
> i"reg0[10]"
>  function rEGBIT_LKUP_FDB()         : istring = i"reg0[11]"
>  function rEGBIT_HAIRPIN_REPLY()    : istring = i"reg0[12]"
>  function rEGBIT_ACL_LABEL()        : istring = i"reg0[13]"
> +function rEGBIT_FROM_RAMP()        : istring = i"reg0[14]"
>
>  function rEG_ORIG_DIP_IPV4()       : istring = i"reg1"
>  function rEG_ORIG_DIP_IPV6()       : istring = i"xxreg1"
> @@ -2070,6 +2071,16 @@ for (&Switch(._uuid = ls_uuid, .has_stateful_acl = 
> true)) {
>           .io_port          = None,
>           .controller_meter = None);
>
> +    /* Do not send coming from RAMP switch packets to conntrack. */
> +    Flow(.logical_datapath = ls_uuid,
> +         .stage            = s_SWITCH_IN_PRE_ACL(),
> +         .priority         = 110,
> +         .__match          = i"${rEGBIT_FROM_RAMP()} == 1",
> +         .actions          = i"next;",
> +         .stage_hint       = 0,
> +         .io_port          = None,
> +         .controller_meter = None);
> +
>      /* Ingress and Egress Pre-ACL Table (Priority 100).
>       *
>       * Regardless of whether the ACL is "from-lport" or "to-lport",
> @@ -2136,6 +2147,16 @@ for (&Switch(._uuid = ls_uuid)) {
>           .io_port          = None,
>           .controller_meter = None);
>
> +    /* Do not send coming from RAMP switch packets to conntrack. */
> +    Flow(.logical_datapath = ls_uuid,
> +         .stage            = s_SWITCH_IN_PRE_LB(),
> +         .priority         = 110,
> +         .__match          = i"${rEGBIT_FROM_RAMP()} == 1",
> +         .actions          = i"next;",
> +         .stage_hint       = 0,
> +         .io_port          = None,
> +         .controller_meter = None);
> +
>      /* Allow all packets to go to next tables by default. */
>      Flow(.logical_datapath = ls_uuid,
>           .stage            = s_SWITCH_IN_PRE_LB(),
> @@ -3361,10 +3382,18 @@ for (&SwitchPort(.lsp = lsp, .sw = sw, .json_name = 
> json_name, .ps_eth_addresses
>              } else {
>                  i"inport == ${json_name} && eth.src == 
> {${ps_eth_addresses.join(\" \")}}"
>              } in
> -        var actions = match (pbinding.options.get(i"qdisc_queue_id")) {
> +        var actions = {
> +            var ramp = if (lsp.__type == i"vtep") {
> +                i"${rEGBIT_FROM_RAMP()} = 1; "
> +            } else {
> +                i""
> +            };
> +            var queue = match (pbinding.options.get(i"qdisc_queue_id")) {
>                  None -> i"next;",
>                  Some{id} -> i"set_queue(${id}); next;"
> -            } in
> +            };
> +            i"${ramp}${queue}"
> +        } in
>          Flow(.logical_datapath = sw._uuid,
>               .stage            = s_SWITCH_IN_PORT_SEC_L2(),
>               .priority         = 50,
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index 2af3f2096..5de554455 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -3597,6 +3597,7 @@ check_stateful_flows() {
>    table=6 (ls_in_pre_lb       ), priority=110  , match=(eth.dst == 
> $svc_monitor_mac), action=(next;)
>    table=6 (ls_in_pre_lb       ), priority=110  , match=(ip && inport == 
> "sw0-lr0"), action=(next;)
>    table=6 (ls_in_pre_lb       ), priority=110  , match=(nd || nd_rs || nd_ra 
> || mldv1 || mldv2), action=(next;)
> +  table=6 (ls_in_pre_lb       ), priority=110  , match=(reg0[[14]] == 1), 
> action=(next;)
>  ])
>
>      AT_CHECK([grep "ls_in_pre_stateful" sw0flows | sort], [0], [dnl
> @@ -3660,6 +3661,7 @@ AT_CHECK([grep "ls_in_pre_lb" sw0flows | sort], [0], 
> [dnl
>    table=6 (ls_in_pre_lb       ), priority=110  , match=(eth.dst == 
> $svc_monitor_mac), action=(next;)
>    table=6 (ls_in_pre_lb       ), priority=110  , match=(ip && inport == 
> "sw0-lr0"), action=(next;)
>    table=6 (ls_in_pre_lb       ), priority=110  , match=(nd || nd_rs || nd_ra 
> || mldv1 || mldv2), action=(next;)
> +  table=6 (ls_in_pre_lb       ), priority=110  , match=(reg0[[14]] == 1), 
> action=(next;)
>  ])
>
>  AT_CHECK([grep "ls_in_pre_stateful" sw0flows | sort], [0], [dnl
> --
> 2.30.0
>
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to