Please check out the v2:
https://patchwork.ozlabs.org/project/ovn/patch/20230121164609.3625347-1-odiv...@gmail.com/
Regards,
Vladislav Odintsov
> On 21 Jan 2023, at 15:47, Vladislav Odintsov <odiv...@gmail.com> wrote:
>
> Hi Simon,
>
> thanks for the review.
> I agree with all your comments and I’ll address them in v2.
>
> Regards,
> Vladislav Odintsov
>
>> On 18 Jan 2023, at 16:08, Simon Horman <simon.hor...@corigine.com> wrote:
>>
>> On Thu, Dec 22, 2022 at 08:43:08PM +0300, Vladislav Odintsov wrote:
>>> There were two issues prior to this patch:
>>> 1. It was unable to have connectivity to networks over a router in
>>> physical network connected through VTEP (ramp) gateway.
>>> Consider next topology:
>>>
>>> ovn-nbctl lr-add lr1
>>> ovn-nbctl lrp-add lr1 lrp1 00:00:00:00:00:01 10.0.0.1/24
>>> ovn-nbctl ls-add ls1
>>> ovn-nbctl lrp-add ls1 lsp1 -- \
>>> lsp-set-addresses lsp1 router -- \
>>> lsp-set-type lsp1 router -- \
>>> lsp-set-options lsp1 router-port=lrp1
>>> ovn-nbctl lsp-add ls1 lsp-vtep -- \
>>> lsp-set-type lsp-vtep vtep -- \
>>> lsp-set-addresses lsp-vtep unknown -- \
>>> lsp-set-options lsp-vtep vtep-physical-switch=<..>
>>> vtep-logical-switch=<..>
>>> ovn-nbctl lr-route-add lr1 192.168.0.0/24 10.0.0.100
>>>
>>> If one issues ping from lsp1 to some address from 192.168.0.0/24 (via
>>> vtep lsp), to enable routing support with vtep it is required to set
>>> redirect chassis or ha chassis group on lrp1. This topology didn't
>>> provide connectivity. Now such traffic flow will work properly.
>>>
>>> 2. Traffic from lport in one subnet to vtep lport in another subnet of
>>> same LR previously traversed via l3gw chassis, now in 'to vtep lport'
>>> direction goes directly from hypervisor handling lport to VTEP (RAMP)
>>> switch. In the opposite direction traffic still goes from VTEP (RAMP)
>>> switch through l3gw chassis and then to hypervisor.
>>
>> I think it would be good to explain how the logic changes
>> addresses these problems. And breifly describe which tests have been added.
>>
>>>
>>> Signed-off-by: Vladislav Odintsov <odiv...@gmail.com>
>>> ---
>>> northd/northd.c | 16 +++++++++++++++-
>>> tests/ovn-northd.at | 26 +++++++++++++++++++++++++-
>>> 2 files changed, 40 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/northd/northd.c b/northd/northd.c
>>> index 4751feab4..07fb0ab9a 100644
>>> --- a/northd/northd.c
>>> +++ b/northd/northd.c
>>> @@ -3362,7 +3362,12 @@ ovn_port_update_sbrec(struct northd_input
>>> *input_data,
>>> }
>>> smap_add(&new, "distributed-port", op->nbrp->name);
>>>
>>> - bool always_redirect = !op->od->has_distributed_nat;
>>> + bool always_redirect = !(
>>> + op->od->has_distributed_nat ||
>>> + (op->l3dgw_port->peer &&
>>> + op->l3dgw_port->peer->od->has_vtep_lports)
>>> + );
>>
>> This seems slightly easier on my brain:
>>
>> bool always_redirect =
>> !op->od->has_distributed_nat &&
>> !(op->l3dgw_port->peer &&
>> op->l3dgw_port->peer->od->has_vtep_lports)
>>
>>> +
>>> if (redirect_type) {
>>> smap_add(&new, "redirect-type", redirect_type);
>>> /* XXX Why can't we enable always-redirect when
>>> redirect-type
>>> @@ -12815,6 +12820,15 @@ build_gateway_redirect_flows_for_lrouter(
>>> return;
>>> }
>>> for (size_t i = 0; i < od->n_l3dgw_ports; i++) {
>>> + if (od->l3dgw_ports[i]->peer &&
>>> + od->l3dgw_ports[i]->peer->od->has_vtep_lports) {
>>
>> This condition appears twice in this patch.
>> Perhaps a helper would make its meaning more obvious.
>>
>>> + /* Skip adding redirect rule for vtep-enabled l3dgw ports.
>>> + Traffic from hypervisor to VTEP (ramp) switch should go in
>>> + distributed manner. Only returning routed traffic must go
>>> + through centralized gateway (or ha-chassis-group). */
>>
>> nit:
>> /* Multi-line comments
>> * look like this. */
>>
>>> + continue;
>>> + }
>>> +
>>> const struct ovsdb_idl_row *stage_hint = NULL;
>>> bool add_def_flow = true;
>>>
>>
>> ...
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
