On 10/12/23 23:39, Mark Michelson wrote:
> On 10/9/23 11:15, Dumitru Ceara wrote:
>> On 9/18/23 18:46, Xavier Simonart wrote:
>>> When l3gateway ports get added, ct_zones were assigned during
>>> (ct_zones) recomputes, but not by I+P.
>>> Before this patch, test "Migration of CT zone from UUID to name"
>>> was randomly failing, as ct_zone was not assigned by I+P but
>>> a ct_zone recompute happened most of the time (and hence test
>>> succeeded).
>>> Test case has been adapted so that ct_zone recompute usually happens
>>> before adding the sw0-lr0 port.
>>>
>>> Signed-off-by: Xavier Simonart <xsimo...@redhat.com>
>>> ---
>>> controller/ovn-controller.c | 1 +
>>> tests/ovn.at | 6 +++---
>>> 2 files changed, 4 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
>>> index 859d9cab9..e5067ed1b 100644
>>> --- a/controller/ovn-controller.c
>>> +++ b/controller/ovn-controller.c
>>> @@ -2637,6 +2637,7 @@ ct_zones_runtime_data_handler(struct
>>> engine_node *node, void *data)
>>> struct tracked_lport *t_lport = shash_node->data;
>>> if (strcmp(t_lport->pb->type, "")
>>> && strcmp(t_lport->pb->type, "localport")
>>> + && strcmp(t_lport->pb->type, "l3gateway")
>>> && strcmp(t_lport->pb->type, "localnet")) {
>>> /* We allocate zone-id's only to VIF, localport,
>>> and localnet
>>> * lports. */
>>
>> Hi Xavier,
>>
>> Why do we need per-port zones for routers? Isn't it a bug that one gets
>> assigned in the "recompute" stage?
>>
>> Unlike switches, routers use 2 conntrack zones, SNAT and DNAT (sometimes
>> merged). The comment here also suggests the same thing.
>>
>> Am I missing something?
>
> Hi Dumitru,
>
Hi Mark,
> Ports of type "l3gateway" are on logical switches, not logical routers.
> Ports of this type connect to logical routers. This patch isn't adding
> per-port zones for routers. Instead the patch ensures that all "local"
> logical switch ports are handled in the I-P case, just like they are in
> the recompute case.
>
You're right, I mixed things up, thanks for the clarification!
We should probably also update the comment to include l3gateway ports
too. Otherwise, the patch looks good to me now.
Regards,
Dumitru
>>
>> Thanks,
>> Dumitru
>>
>>> diff --git a/tests/ovn.at b/tests/ovn.at
>>> index ba5ce298a..acf00a335 100644
>>> --- a/tests/ovn.at
>>> +++ b/tests/ovn.at
>>> @@ -36860,14 +36860,14 @@ check ovn-nbctl ls-add sw0
>>> check ovn-nbctl lsp-add sw0 sw0-port1
>>> check ovn-nbctl lsp-set-addresses sw0-port1 "50:54:00:00:00:01
>>> 192.168.0.2"
>>> +ovs-vsctl add-port br-int p1 -- \
>>> + set Interface p1 external_ids:iface-id=sw0-port1
>>> +
>>> check ovn-nbctl lsp-add sw0 sw0-lr0
>>> check ovn-nbctl lsp-set-type sw0-lr0 router
>>> check ovn-nbctl lsp-set-addresses sw0-lr0 00:00:00:00:ff:01
>>> check ovn-nbctl lsp-set-options sw0-lr0 router-port=lr0-sw0
>>> -ovs-vsctl add-port br-int p1 -- \
>>> - set Interface p1 external_ids:iface-id=sw0-port1
>>> -
>>> check ovn-appctl -t ovn-controller vlog/set dbg:main
>>> wait_for_ports_up
>>
>> _______________________________________________
>> 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
