Hi Felix,
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?
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 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.
 
> +                "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.

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

Reply via email to