Thanks Numan.

regards,
Vladislav Odintsov

> On 16 Aug 2022, at 04:56, Numan Siddique <[email protected]> wrote:
> 
> 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
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to