On 8/22/24 17:17, Lorenzo Bianconi wrote:
>> Hi Lorenzo,
>>
>> I have one complaint below, otherwise the series looks good to me.
>>
> 
> [...]
> 
>>> @@ -742,12 +738,6 @@ void static_routes_destroy(struct static_routes_data 
>>> *);
>>>   void bfd_init(struct bfd_data *);
>>>   void bfd_destroy(struct bfd_data *);
>>> -void build_ecmp_nexthop_table(struct ovsdb_idl_txn *,
>>> -                              struct hmap *, struct simap *,
>>> -                              const struct sbrec_ecmp_nexthop_table *);
>>> -void ecmp_nexthop_init(struct ecmp_nexthop_data *);
>>> -void ecmp_nexthop_destroy(struct ecmp_nexthop_data *);
>>> -
>>>   struct lflow_table;
>>>   struct lr_stateful_tracked_data;
>>>   struct ls_stateful_tracked_data;
>>> diff --git a/ovn-sb.ovsschema b/ovn-sb.ovsschema
>>> index 9d8e0ac46..ec39fdd81 100644
>>> --- a/ovn-sb.ovsschema
>>> +++ b/ovn-sb.ovsschema
>>> @@ -1,7 +1,7 @@
>>>   {
>>>       "name": "OVN_Southbound",
>>> -    "version": "20.36.0",
>>> -    "cksum": "1845967275 32154",
>>> +    "version": "20.35.0",
>>> +    "cksum": "2897301415 31493",
>>
>> Even though the table that bumped the version is being removed, I think it
>> makes more sense to bump the version to 23.37.0 instead of decreasing the
>> version number here.
> 
> ack, I am fine with it. If we have a consensus for it I will post v2.
> 

While I don't think there's anyone (there shouldn't be at least) using
an unreleased version of OVN with this table in the schema it doesn't
hurt to bump the version just to be sure.

On the other hand, if someone was using this version an upgrade to this
new version (without the SB table) will break their deployment anyway.

TL/DR: I'm fine either way. :)

Regards,
Dumitru

> Regards,
> Lorenzo
> 
>>
>>>       "tables": {
>>>           "SB_Global": {
>>>               "columns": {
>>> @@ -610,20 +610,6 @@
>>>                                             "refTable": 
>>> "Datapath_Binding"}}}},
>>>               "indexes": [["logical_port", "ip"]],
>>>               "isRoot": true},
>>> -        "ECMP_Nexthop": {
>>> -            "columns": {
>>> -                "nexthop": {"type": "string"},
>>> -                "id": {"type": {"key": {"type": "integer",
>>> -                                        "minInteger": 0,
>>> -                                        "maxInteger": 65535}}},
>>> -                "external_ids": {
>>> -                    "type": {"key": "string", "value": "string",
>>> -                             "min": 0, "max": "unlimited"}},
>>> -                "options": {
>>> -                    "type": {"key": "string", "value": "string",
>>> -                             "min": 0, "max": "unlimited"}}},
>>> -            "indexes": [["nexthop"]],
>>> -            "isRoot": true},
>>>           "Chassis_Template_Var": {
>>>               "columns": {
>>>                   "chassis": {"type": "string"},
>>> diff --git a/ovn-sb.xml b/ovn-sb.xml
>>> index c11296d7c..746cc6308 100644
>>> --- a/ovn-sb.xml
>>> +++ b/ovn-sb.xml
>>> @@ -5178,35 +5178,4 @@ tcp.flags = RST;
>>>         The set of variable values for a given chassis.
>>>       </column>
>>>     </table>
>>> -
>>> -  <table name="ECMP_Nexthop">
>>> -    <p>
>>> -      Each record in this table represents an active ECMP route committed 
>>> by
>>> -      <code>ovn-northd</code> to <code>ovs</code> connection-tracking 
>>> table.
>>> -      <code>ECMP_Nexthop</code> table is used by 
>>> <code>ovn-controller</code>
>>> -      to track active ct entries and to flush stale ones.
>>> -    </p>
>>> -    <column name="nexthop">
>>> -      <p>
>>> -        Nexthop IP address for this ECMP route. Nexthop IP address should
>>> -        be the IP address of a connected router port or the IP address of
>>> -        an external device used as nexthop for the given destination.
>>> -      </p>
>>> -    </column>
>>> -
>>> -    <column name="id">
>>> -      <p>
>>> -        Nexthop unique identifier. Nexthop ID is used to track active
>>> -        ecmp-symmetric-reply connections and flush stale ones.
>>> -      </p>
>>> -    </column>
>>> -
>>> -    <column name="options">
>>> -      Reserved for future use.
>>> -    </column>
>>> -
>>> -    <column name="external_ids">
>>> -      See <em>External IDs</em> at the beginning of this document.
>>> -    </column>
>>> -  </table>
>>>   </database>
>>> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
>>> index 93ccbce6b..fddf222b3 100644
>>> --- a/tests/ovn-northd.at
>>> +++ b/tests/ovn-northd.at
>>> @@ -6800,7 +6800,6 @@ check ovn-nbctl lsp-set-addresses public-lr0 router
>>>   check ovn-nbctl lsp-set-options public-lr0 router-port=lr0-public
>>>   check ovn-nbctl --wait=sb --ecmp-symmetric-reply lr-route-add lr0 1.0.0.1 
>>> 192.168.0.10
>>> -check_row_count ECMP_Nexthop 1
>>>   ovn-sbctl dump-flows lr0 > lr0flows
>>> @@ -6818,7 +6817,6 @@ AT_CHECK([grep -e "lr_in_ip_routing_ecmp" lr0flows | 
>>> ovn_strip_lflows], [0], [dn
>>>   ])
>>>   check ovn-nbctl --wait=sb --ecmp-symmetric-reply lr-route-add lr0 1.0.0.1 
>>> 192.168.0.20
>>> -check_row_count ECMP_Nexthop 2
>>>   ovn-sbctl dump-flows lr0 > lr0flows
>>>   AT_CHECK([grep -w "lr_in_ip_routing" lr0flows | ovn_strip_lflows], [0], 
>>> [dnl
>>> @@ -6848,7 +6846,6 @@ AT_CHECK([grep -e "lr_in_arp_resolve.*ecmp" lr0flows 
>>> | ovn_strip_lflows], [0], [
>>>   # add ecmp route with wrong nexthop
>>>   check ovn-nbctl --wait=sb --ecmp-symmetric-reply lr-route-add lr0 1.0.0.1 
>>> 192.168.1.20
>>> -check_row_count ECMP_Nexthop 2
>>>   ovn-sbctl dump-flows lr0 > lr0flows
>>>   AT_CHECK([grep -w "lr_in_ip_routing" lr0flows | ovn_strip_lflows], [0], 
>>> [dnl
>>> @@ -6868,7 +6865,6 @@ AT_CHECK([grep -e "lr_in_ip_routing_ecmp" lr0flows | 
>>> sed 's/192\.168\.0\..0/192.
>>>   check ovn-nbctl lr-route-del lr0
>>>   wait_row_count nb:Logical_Router_Static_Route 0
>>> -check_row_count ECMP_Nexthop 0
>>>   check ovn-nbctl --wait=sb lr-route-add lr0 1.0.0.0/24 192.168.0.10
>>>   ovn-sbctl dump-flows lr0 > lr0flows
>>

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to