On Tue, Aug 16, 2022 at 12:19 AM Vladislav Odintsov <[email protected]> wrote:
>
> If LRP's logical switch has attached LSP of vtep type, the
> is_chassis_resident() part is not added to lflow to allow traffic
> originated from logical switch to reach LR services (LBs, NAT).
>
> Signed-off-by: Vladislav Odintsov <[email protected]>

Thanks for v2 and for the test case.
Applied the patch to main.

Numan


> ---
> This is a continuation from [1] as a v2 edition after Numan's review.
> - reworked to use od->has_vtep_lports and removed lrp's confusing option
>   'is_distributed'
> - added related tests
> - updated ovn-northd flows docs
>
> 1: https://mail.openvswitch.org/pipermail/ovs-dev/2022-August/396796.html
> ---
>  northd/northd.c         | 33 +++++++++++++++++---
>  northd/ovn-northd.8.xml |  4 +++
>  tests/ovn-northd.at     | 69 +++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 102 insertions(+), 4 deletions(-)
>
> diff --git a/northd/northd.c b/northd/northd.c
> index facd41a59..b1e9ffc87 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -637,6 +637,7 @@ struct ovn_datapath {
>      bool has_lb_vip;
>      bool has_unknown;
>      bool has_acls;
> +    bool has_vtep_lports;
>
>      /* IPAM data. */
>      struct ipam_info ipam_info;
> @@ -1847,6 +1848,12 @@ lsp_is_localnet(const struct nbrec_logical_switch_port 
> *nbsp)
>      return !strcmp(nbsp->type, "localnet");
>  }
>
> +static bool
> +lsp_is_vtep(const struct nbrec_logical_switch_port *nbsp)
> +{
> +    return !strcmp(nbsp->type, "vtep");
> +}
> +
>  static bool
>  localnet_can_learn_mac(const struct nbrec_logical_switch_port *nbsp)
>  {
> @@ -2655,6 +2662,10 @@ join_logical_ports(struct northd_input *input_data,
>                     od->localnet_ports[od->n_localnet_ports++] = op;
>                  }
>
> +                if (lsp_is_vtep(nbsp)) {
> +                    od->has_vtep_lports = true;
> +                }
> +
>                  op->lsp_addrs
>                      = xmalloc(sizeof *op->lsp_addrs * nbsp->n_addresses);
>                  for (size_t j = 0; j < nbsp->n_addresses; j++) {
> @@ -5518,7 +5529,7 @@ build_lswitch_port_sec_op(struct ovn_port *op, struct 
> hmap *lflows,
>          ds_put_format(actions, "set_queue(%s); ", queue_id);
>      }
>
> -    if (!strcmp(op->nbsp->type, "vtep")) {
> +    if (lsp_is_vtep(op->nbsp)) {
>          ds_put_format(actions, REGBIT_FROM_RAMP" = 1; ");
>          ds_put_format(actions, "next(pipeline=ingress, table=%d);",
>                        ovn_stage_get_table(S_SWITCH_IN_HAIRPIN));
> @@ -10894,6 +10905,22 @@ build_gateway_mtu_flow(struct hmap *lflows, struct 
> ovn_port *op,
>      va_end(extra_actions_args);
>  }
>
> +static bool
> +consider_l3dwg_port_is_centralized(struct ovn_port *op)
> +{
> +    if (op->peer && op->peer->od->has_vtep_lports) {
> +        return false;
> +    }
> +
> +    if (is_l3dgw_port(op)) {
> +        /* Traffic with eth.dst = l3dgw_port->lrp_networks.ea_s
> +         * should only be received on the gateway chassis. */
> +        return true;
> +    }
> +
> +    return false;
> +}
> +
>  /* Logical router ingress Table 0: L2 Admission Control
>   * This table drops packets that the router shouldn’t see at all based
>   * on their Ethernet headers.
> @@ -10930,9 +10957,7 @@ build_adm_ctrl_flows_for_lrouter_port(
>          ds_clear(match);
>          ds_put_format(match, "eth.dst == %s && inport == %s",
>                        op->lrp_networks.ea_s, op->json_key);
> -        if (is_l3dgw_port(op)) {
> -            /* Traffic with eth.dst = l3dgw_port->lrp_networks.ea_s
> -             * should only be received on the gateway chassis. */
> +        if (consider_l3dwg_port_is_centralized(op)) {
>              ds_put_format(match, " && is_chassis_resident(%s)",
>                            op->cr_port->json_key);
>          }
> diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
> index ff21c0737..9b6459d9e 100644
> --- a/northd/ovn-northd.8.xml
> +++ b/northd/ovn-northd.8.xml
> @@ -2114,6 +2114,10 @@ output;
>            gateway chassis), the above flow matching
>            <code>eth.dst == <var>E</var></code> is only programmed on
>            the gateway port instance on the gateway chassis.
> +          If LRP's logical switch has attached LSP of <code>vtep</code> type,
> +          the <code>is_chassis_resident()</code> part is not added to lflow 
> to
> +          allow traffic originated from logical switch to reach LR services
> +          (LBs, NAT).
>          </p>
>
>          <p>
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index 5b5eeb0ee..3ffa39419 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -5747,6 +5747,75 @@ AT_CHECK([grep lr_in_gw_redirect lrflows | grep cr-DR 
> | sed 's/table=../table=??
>  AT_CLEANUP
>  ])
>
> +OVN_FOR_EACH_NORTHD([
> +AT_SETUP([ovn-northd -- lr admission with vtep lports])
> +AT_KEYWORDS([multiple-l3dgw-ports])
> +ovn_start NORTHD_TYPE
> +check ovn-sbctl chassis-add ch1 geneve 127.0.0.2
> +
> +check ovn-nbctl lr-add lr1
> +check ovn-nbctl lrp-add lr1 lrp1 00:00:00:00:00:01 10.0.0.1/24
> +check ovn-nbctl ls-add ls1
> +check ovn-nbctl lsp-add ls1 lsp1 -- \
> +    lsp-set-addresses lsp1 router -- \
> +    lsp-set-type lsp1 router -- \
> +    lsp-set-options lsp1 router-port=lrp1
> +
> +# ensure initial flows are installed without is_chassis_resident match part
> +ovn-nbctl --wait=sb sync
> +ovn-sbctl dump-flows lr1 > lrflows
> +AT_CAPTURE_FILE([lrflows])
> +
> +# Check the flows in lr_in_admission stage
> +AT_CHECK([grep lr_in_admission lrflows | grep lrp1 | sed 
> 's/table=../table=??/' | sort], [0], [dnl
> +  table=??(lr_in_admission    ), priority=50   , match=(eth.dst == 
> 00:00:00:00:00:01 && inport == "lrp1"), action=(xreg0[[0..47]] = 
> 00:00:00:00:00:01; next;)
> +  table=??(lr_in_admission    ), priority=50   , match=(eth.mcast && inport 
> == "lrp1"), action=(xreg0[[0..47]] = 00:00:00:00:00:01; next;)
> +])
> +
> +# make lrp a cr-port and check its flows
> +check ovn-nbctl lrp-set-gateway-chassis lrp1 ch1
> +
> +ovn-nbctl --wait=sb sync
> +ovn-sbctl dump-flows lr1 > lrflows
> +AT_CAPTURE_FILE([lrflows])
> +
> +# Check the flows in lr_in_admission stage
> +AT_CHECK([grep lr_in_admission lrflows | grep lrp1 | sed 
> 's/table=../table=??/' | sort], [0], [dnl
> +  table=??(lr_in_admission    ), priority=50   , match=(eth.dst == 
> 00:00:00:00:00:01 && inport == "lrp1" && is_chassis_resident("cr-lrp1")), 
> action=(xreg0[[0..47]] = 00:00:00:00:00:01; next;)
> +  table=??(lr_in_admission    ), priority=50   , match=(eth.mcast && inport 
> == "lrp1"), action=(xreg0[[0..47]] = 00:00:00:00:00:01; next;)
> +])
> +
> +# attach vtep logical port to logical switch and check flows.
> +# there should not be is_chassis_resident part.
> +check ovn-nbctl lsp-add ls1 lsp-vtep -- lsp-set-type lsp-vtep vtep
> +
> +ovn-nbctl --wait=sb sync
> +ovn-sbctl dump-flows lr1 > lrflows
> +AT_CAPTURE_FILE([lrflows])
> +
> +# Check the flows in lr_in_admission stage
> +AT_CHECK([grep lr_in_admission lrflows | grep lrp1 | sed 
> 's/table=../table=??/' | sort], [0], [dnl
> +  table=??(lr_in_admission    ), priority=50   , match=(eth.dst == 
> 00:00:00:00:00:01 && inport == "lrp1"), action=(xreg0[[0..47]] = 
> 00:00:00:00:00:01; next;)
> +  table=??(lr_in_admission    ), priority=50   , match=(eth.mcast && inport 
> == "lrp1"), action=(xreg0[[0..47]] = 00:00:00:00:00:01; next;)
> +])
> +
> +# delete vtep lport and check lrp has is_chassis_resident match part again.
> +check ovn-nbctl lsp-del lsp-vtep
> +
> +ovn-nbctl --wait=sb sync
> +ovn-sbctl dump-flows lr1 > lrflows
> +AT_CAPTURE_FILE([lrflows])
> +
> +# Check the flows in lr_in_admission stage
> +AT_CHECK([grep lr_in_admission lrflows | grep lrp1 | sed 
> 's/table=../table=??/' | sort], [0], [dnl
> +  table=??(lr_in_admission    ), priority=50   , match=(eth.dst == 
> 00:00:00:00:00:01 && inport == "lrp1" && is_chassis_resident("cr-lrp1")), 
> action=(xreg0[[0..47]] = 00:00:00:00:00:01; next;)
> +  table=??(lr_in_admission    ), priority=50   , match=(eth.mcast && inport 
> == "lrp1"), action=(xreg0[[0..47]] = 00:00:00:00:00:01; next;)
> +])
> +
> +AT_CLEANUP
> +])
> +
> +
>  OVN_FOR_EACH_NORTHD([
>  AT_SETUP([check options:requested-chassis fills requested_chassis col])
>  ovn_start NORTHD_TYPE
> --
> 2.36.1
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to