Re: [ovs-dev] [PATCH ovn 1/2] northd: make traffic routed to vtep lport distributed
On Sat, Jan 21, 2023 at 07:49:11PM +0300, Vladislav Odintsov wrote: > Please check out the v2: > https://patchwork.ozlabs.org/project/ovn/patch/20230121164609.3625347-1-odiv...@gmail.com/ Thanks, done :) ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH ovn 1/2] northd: make traffic routed to vtep lport distributed
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 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 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
>>> ---
>>> 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(, "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(, "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
Re: [ovs-dev] [PATCH ovn 1/2] northd: make traffic routed to vtep lport distributed
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 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
>> ---
>> 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(, "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(, "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
Re: [ovs-dev] [PATCH ovn 1/2] northd: make traffic routed to vtep lport distributed
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
> ---
> 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(, "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(, "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
Re: [ovs-dev] [PATCH ovn 1/2] northd: make traffic routed to vtep lport distributed
Bleep bloop. Greetings Vladislav Odintsov, I am a robot and I have tried out your patch. Thanks for your contribution. I encountered some error that I wasn't expecting. See the details below. checkpatch: WARNING: Line lacks whitespace around operator #68 FILE: northd/northd.c:12828: through centralized gateway (or ha-chassis-group). */ Lines checked: 143, Warnings: 1, Errors: 0 Please check this out. If you feel there has been an error, please email acon...@redhat.com Thanks, 0-day Robot ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
[ovs-dev] [PATCH ovn 1/2] northd: make traffic routed to vtep lport distributed
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.
Signed-off-by: Vladislav Odintsov
---
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(, "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)
+);
+
if (redirect_type) {
smap_add(, "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) {
+/* 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). */
+continue;
+}
+
const struct ovsdb_idl_row *stage_hint = NULL;
bool add_def_flow = true;
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 56c1e6c2e..72f7c3e2d 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -6074,7 +6074,7 @@ AT_CLEANUP
])
OVN_FOR_EACH_NORTHD_NO_HV([
-AT_SETUP([ovn-northd -- lr admission with vtep lports])
+AT_SETUP([ovn-northd -- lrp with chassis-redirect and ls with vtep lport])
AT_KEYWORDS([multiple-l3dgw-ports])
ovn_start NORTHD_TYPE
check ovn-sbctl chassis-add ch1 geneve 127.0.0.2
@@ -6098,6 +6098,11 @@ AT_CHECK([grep lr_in_admission lrflows | grep lrp1 | sed
's/table=../table=??/'
table=??(lr_in_admission), priority=50 , match=(eth.mcast && inport ==
"lrp1"), action=(xreg0[[0..47]] = 00:00:00:00:00:01; next;)
])
+# Check the flows in lr_in_gw_redirect stage
+AT_CHECK([grep lr_in_gw_redirect lrflows | grep lrp1 | sed
's/table=../table=??/' | sort], [0], [])
+
+wait_row_count Port_Binding 0 logical_port=cr-lrp1
options:always-redirect="true"
+
# make lrp a cr-port and check its flows
check ovn-nbctl lrp-set-gateway-chassis lrp1 ch1
@@ -6111,6 +6116,13 @@ AT_CHECK([grep lr_in_admission lrflows | grep lrp1 | sed
's/table=../table=??/'
table=??(lr_in_admission), priority=50 , match=(eth.mcast && inport ==
"lrp1"), action=(xreg0[[0..47]] = 00:00:00:00:00:01; next;)
])
+# Check the flows in lr_in_gw_redirect stage
+AT_CHECK([grep lr_in_gw_redirect lrflows | grep lrp1 | sed
's/table=../table=??/' | sort], [0], [dnl
+ table=??(lr_in_gw_redirect ), priority=50 , match=(outport == "lrp1"),
action=(outport = "cr-lrp1"; next;)
+])
+
+wait_row_count Port_Binding 1 logical_port=cr-lrp1
options:always-redirect="true"
+
# 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
@@ -6125,6 +6137,11 @@ AT_CHECK([grep lr_in_admission lrflows | grep lrp1 | sed
's/table=../table=??/'
table=??(lr_in_admission), priority=50 , match=(eth.mcast &&
