On 12/11/24 1:19 PM, Felix Huettner via dev wrote:
> On Wed, Dec 11, 2024 at 10:23:40AM +0100, [email protected] wrote:
>> Hi Felix,
>
> Hi Martin,
> thanks for the comments.
>
>> thanks for the update. I was testing this version yesterday and I have
>> some thoughts regarding the referencing.
>> Wouldn't it be better if the Advertised_Route was a non-root table and
>> instead of having strong references to Port_Binding via "logical_port",
>> it would be the Port_Binding that would have strong references to the
>> Advertised_Route entries?
>
> We found out here
> https://mail.openvswitch.org/pipermail/ovs-dev/2024-December/418890.html
> that we can not have strong references from a non-root table to a root
> table. This interferes with the correct deletion.
>
> So we could only do this if we remove any kind of outbound references
> from these Tables and i don't think that is reasonable.
> We would need to get rid of the "datapath" column (see below).
>
> Additionally tracked_port would need to be reworked. I don't know how
> we could reasonably model this without a outbound reference.
>
>> I haven't tried my proposed setup yet (currently working on it), but my
>> intention is for the Advertised_Route entry getting automatically
>> cleaned up when the "logical_port" Port_Binding is removed. With the
>> current proposal, Port_Binding removal is blocked due to the integrity
>> violation (Advertised_Route still has strong ref to Port_Binding) and
>> requires additional logic in northd to manually remove associated
>> Advertised_Routes we want to remove Port_Binding.
>
> I would agree that this might be easier for incremental processing.
>
> However for the normal runs i don't think there is too much difference.
> I have already logic that syncs the state of parsed_routes to the
> southbound database. This also needs to handle changed ips and so on.
> So deleting the routes when the logical_port no longer exists is at
> least there already covered.
>
>>
>> I have also couple questions below.
>>
>> On Tue, 2024-12-10 at 14:36 +0100, Felix Huettner via dev wrote:
>>> These two tables will be used in the future to coordinate routing
>>> information between northd and ovn-controller.
>>> Northd will insert routes that should be advertised to the outside
>>> fabric to Advertised_Route.
>>> Ovn-controller will insert routes that have been learned from the
>>> outside fabric to Learned_Route.
>>>
>>> Signed-off-by: Felix Huettner <[email protected]>
>>> ---
>>> v3->v4:
>>> * split Route to Advertised_Route and Learned_Route
>>> v2->v3:
>>> * refType is now strong
>>> * fixed typos
>>>
>>> ovn-sb.ovsschema | 38 ++++++++++++++++++++++++--
>>> ovn-sb.xml | 70
>>> ++++++++++++++++++++++++++++++++++++++++++++++++
>>> 2 files changed, 106 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/ovn-sb.ovsschema b/ovn-sb.ovsschema
>>> index 73abf2c8d..61bc26457 100644
>>> --- a/ovn-sb.ovsschema
>>> +++ b/ovn-sb.ovsschema
>>> @@ -1,7 +1,7 @@
>>> {
>>> "name": "OVN_Southbound",
>>> - "version": "20.37.0",
>>> - "cksum": "1950136776 31493",
>>> + "version": "20.38.0",
>>> + "cksum": "2175279464 33450",
>>> "tables": {
>>> "SB_Global": {
>>> "columns": {
>>> @@ -617,6 +617,40 @@
>>> "type": {"key": "string", "value": "string",
>>> "min": 0, "max": "unlimited"}}},
>>> "indexes": [["chassis"]],
>>> + "isRoot": true},
>>> + "Advertised_Route": {
>>> + "columns": {
>>> + "datapath": {"type": {"key": {"type": "uuid",
>>> + "refTable":
>>> "Datapath_Binding",
>>> + "refType":
>>> "strong"}}},
>>
>> Is the ref. to Datapath_Binding necessary? Even with the current
>> version where this table is Root, we already have reference to
>> Port_Binding and if necessary, we can infer related Datapath_Binding by
>> looking up Advertised_Route->logical_port->datapath.
>
> I think we could.
> The main idea behind the direct connect between Datapath_Binding and
> Advertised_Route or Learned_Route is that we generally use this
> connection on the northd and ovn-controller side to process the entries.
>
> Basically having the datapath in there allows us to skip an iteration
> over all Port_Bindings in a Datapath whenever we need all routes for it.
>
Yes, that's probably the main advantage, we can use an ovsdb idl index
that filters by datapath.
>>
>>> + "logical_port": {"type": {"key": {"type": "uuid",
>>> + "refTable":
>>> "Port_Binding",
>>> + "refType":
>>> "strong"}}},
>>> + "ip_prefix": {"type": "string"},
>>> + "tracked_port": {"type": {"key": {"type": "uuid",
>>> + "refTable":
>>> "Port_Binding",
>>> + "refType":
>>> "strong"},
>>
>> Given that tracked_port is optional, wouldn't it be better for this
>> reference to be weak? Otherwise we run into the same issue of needing
>> manual northd logic when removing Port_Binding referenced here.
>
> The goal for the tracked_port is to specify the "nexthop" Port_Binding
> of a Route. If the Port_Binding would be removed then this Route is no
> longer valid and needs to be removed anyway. So i think because this is
> directly related we don't need to be weak.
>
I agree.
Regards,
Dumitru
> Thanks a lot
> Felix
>
>>
>> Thanks,
>> Martin.
>>
>>> + "min": 0,
>>> + "max": 1}},
>>> + "external_ids": {
>>> + "type": {"key": "string", "value": "string",
>>> + "min": 0, "max": "unlimited"}}},
>>> + "indexes": [["datapath", "logical_port", "ip_prefix"]],
>>> + "isRoot": true},
>>> + "Learned_Route": {
>>> + "columns": {
>>> + "datapath": {"type": {"key": {"type": "uuid",
>>> + "refTable":
>>> "Datapath_Binding",
>>> + "refType":
>>> "strong"}}},
>>> + "logical_port": {"type": {"key": {"type": "uuid",
>>> + "refTable":
>>> "Port_Binding",
>>> + "refType":
>>> "strong"}}},
>>> + "ip_prefix": {"type": "string"},
>>> + "nexthop": {"type": "string"},
>>> + "external_ids": {
>>> + "type": {"key": "string", "value": "string",
>>> + "min": 0, "max": "unlimited"}}},
>>> + "indexes": [["datapath", "logical_port", "ip_prefix",
>>> "nexthop"]],
>>> "isRoot": true}
>>> }
>>> }
>>> diff --git a/ovn-sb.xml b/ovn-sb.xml
>>> index ea4adc1c3..6fbc8b6df 100644
>>> --- a/ovn-sb.xml
>>> +++ b/ovn-sb.xml
>>> @@ -5217,4 +5217,74 @@ tcp.flags = RST;
>>> The set of variable values for a given chassis.
>>> </column>
>>> </table>
>>> +
>>> + <table name="Advertised_Route">
>>> + <p>
>>> + Each record represents a route that should be exported from
>>> ovn to the
>>> + outside network fabric.
>>> + It is populated by <code>ovn-northd</code> based on the
>>> addresses,
>>> + routes and NAT Entries of a
>>> + <code>OVN_Northbound.Logical_Router_Port</code>.
>>> + </p>
>>> +
>>> + <column name="datapath">
>>> + The datapath belonging to the
>>> + <code>OVN_Northbound.Logical_Router</code> that this route is
>>> valid
>>> + for.
>>> + </column>
>>> +
>>> + <column name="logical_port">
>>> + This is the <ref table="Port_Binding"/> that the router will
>>> send packets
>>> + out that are received for the below prefix.
>>> + </column>
>>> +
>>> + <column name="ip_prefix">
>>> + IP prefix of this route (e.g. 192.168.100.0/24).
>>> + </column>
>>> +
>>> + <column name="tracked_port">
>>> + In combination with a host <code>ip_prefix</code> this tracks
>>> the port
>>> + OVN will forward the packets for this destination to.
>>> + If set the <ref table="Advertised_Route" column="ip_prefix"/>
>>> will always
>>> + contain a /32 (for ipv4) or /128 (for ipv6) prefix.
>>> +
>>> + An announcing chassis can use this information to check if
>>> this
>>> + destination is local and adjust the route priorities based on
>>> that.
>>> + </column>
>>> +
>>> + <column name="external_ids">
>>> + See <em>External IDs</em> at the beginning of this document.
>>> + </column>
>>> + </table>
>>> +
>>> + <table name="Learned_Route">
>>> + <p>
>>> + Each record represents a route that learned by ovn using some
>>> dynamic
>>> + routing logic outside of ovn.
>>> + It is populated by <code>ovn-controller</code> with routes it
>>> learns
>>> + locally.
>>> + </p>
>>> +
>>> + <column name="datapath">
>>> + The datapath belonging to the
>>> + <code>OVN_Northbound.Logical_Router</code> that this route is
>>> valid
>>> + for.
>>> + </column>
>>> +
>>> + <column name="logical_port">
>>> + This is the <ref table="Port_Binding"/> that the route was
>>> learned on.
>>> + </column>
>>> +
>>> + <column name="ip_prefix">
>>> + IP prefix of this route (e.g. 192.168.100.0/24).
>>> + </column>
>>> +
>>> + <column name="nexthop">
>>> + This is the nexthop ip we learned from outside of OVN.
>>> + </column>
>>> +
>>> + <column name="external_ids">
>>> + See <em>External IDs</em> at the beginning of this document.
>>> + </column>
>>> + </table>
>>> </database>
>>
> _______________________________________________
> 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
