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. > > > + "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. 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
