On 1/16/25 22:13, Ilya Maximets wrote:
> On 1/13/25 21:45, Frode Nordahl wrote:
>> Hello,
>>
>> This is a continuation of the work started in [0], which has gone
>> through a couple of iterations by Felix Huettner [1][2].
>>
>> As agreed in the OVN A/V Community meeting December 2nd 2024, I took
>> over herding these patches again.
>>
>> The main rationale for this is that I have done a fair bit of work on
>> Netlink related code before, so it would be a good way to spread the
>> workload of this epic, with the goal of getting the most out of this
>> community effort prior to looming freeze deadlines.
>>
>> Main change from previous iterations is:
>> * Series has been properly separated into logical units of change.
>> * Nexthops stored as a linked list as opposed to fixed size array.
>> * Support for parsing IPv4 routes over IPv6 next hops through the use
>>   of the RTA_VIA attribute.
>> * As agreed the namespaces code proposed by Felix Huettner has been
>>   dropped.
>> * More complete tests have been provided.
>>
>> v3 -> v4:
>> * Use static buffer for the common case of one nexthop to avoid
>>   unnecessary memory allocations.
>> * Fix memory leak due to missing destruction of dynamically allocated
>>   nexthops on callback from the netlink-notifier module.
>> * Improve bounds checking for handling of RTA_VIA attribute.
>> * Simplify handling of RTA_MULTIPATH attribute.
>> * Add lib route-table test program and re-order patches so that
>>   RTA_MULTIPATH comes last to accommodate for expanded test coverage.
>>
>> v4 -> v5:
>> * Rename static nln callback buffer, this confused me alot.
>> * Add min/max size for RTA_VIA attribute, allowing the use of netlink
>>   policy for validation.
>> * Make use of RTA_TABLE in OVS -> KERNEL direction backwards compatible.
>>   Although we agreed it has been supported since forever, it makes the
>>   code consistent for both directions.
>> * Use proper typedef for route_table_handle_msg callback.
>> * Store original value for rtm_dst_len.
>> * Make naming of exported struct elements consistent:
>>   * local -> rtn_local.
>>   * mark  -> rta_mark.
>>   * uint16_t for nlmsg_type.
>> * Expand tests in tip of series to cover the route attributes added at
>>   the beginning of the series.
>> * Improve quailty of the test program code.
>> * Address various review feedback.
>>
>> v5 -> v6:
>> * Address review feedback.
>>
>> 0: https://mail.openvswitch.org/pipermail/ovs-dev/2024-July/416042.html.
>> 1: https://mail.openvswitch.org/pipermail/ovs-dev/2024-October/417872.html
>> 2: https://mail.openvswitch.org/pipermail/ovs-dev/2024-November/418547.html
>>
>> Frode Nordahl (16):
>>   route-table: Store route table ID.
>>   route-table: Store route priority.
>>   route-table: Store route protocol.
>>   route-table: Split header and attribute parsing.
>>   route-table: Rename static nln callback buffer.
>>   route-table: Harmonize log msgs with code base.
>>   route-table: Store nexthops in linked list.
>>   route-table: Support parsing RTA_VIA attribute.
>>   route-table: Use RTA_TABLE for route table filter.
>>   route-table: Use callback for handling route msgs.
>>   route-table: Store orignal value for rtm_dst_len.
>>   route-table: Rename route_data local to rtn_local.
>>   route-table: Rename route_data mark to rta_mark.
>>   route-table: Use correct type for nlmsg_type.
>>   route-table: Export route table sync functions.
>>   route-table: Support parsing multipath routes.
>>
>>  Makefile.am                  |   5 +-
>>  lib/netlink.c                |  12 ++
>>  lib/netlink.h                |   1 +
>>  lib/route-table.c            | 288 ++++++++++++++++++++++++++---------
>>  lib/route-table.h            | 134 ++++++++++++++++
>>  tests/automake.mk            |   1 +
>>  tests/system-route.at        | 204 +++++++++++++++++++++++++
>>  tests/test-lib-route-table.c | 149 ++++++++++++++++++
>>  8 files changed, 717 insertions(+), 77 deletions(-)
>>  create mode 100644 tests/test-lib-route-table.c
>>
> 
> Thanks, Frode, Felix and Eelco!
> 
> I went through the patch set and it generally looks good to me.
> 
> I fixed a few style issues and typos and applied the set.
> For the record, the largest thing I fixed is I renamed the _primary_next_hop
> into primary_next_hop__, as per our coding style document.

* I also folded in the suggested code for the patch 16.

> 
> Best regards, Ilya Maximets.

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to