David Marchand <[email protected]> writes:

> Hello Aaron,
>
> On Mon, 9 Mar 2026 at 17:19, Aaron Conole via dev
> <[email protected]> wrote:
>>
>> Timothy Redaelli notes that, for example, when building with DPDK - EVERY
>> binary object is linked to DPDK libraries - which includes things like
>> ovs-appctl, ovs-vsctl, etc.
>>
>> This is a first cut at improving things.  Create a separate libovscommon
>> library that for example, the ovsdb-* suite, and some ovs-* utilities can
>> use rather than linking to the large libopenvswitch library.  This improves
>> the situation somewhat:
>>
>>  pre-series binary file usage: 279.37 MB
>>  post-series binary file usage: 185.47 MB
>>
>> There is additional work we can do, but I spent enough time ripping apart
>> dependencies to save almost 100M on disk (on my fedora 43 x86_64 system).
>> There are additional areas that could be refactored, for instance the
>> vconn and ofpbuf code could be extracted to shrink ovs-ofctl/ovs-vsctl
>> to see additional savings.
>>
>> The patch is split up so that all the beginning work is mostly about moving
>> code around, and the final patch introduces what I'm calling the
>> libovscommon library.  I've updated the NEWS file since this is API/ABI
>> breaking changes.
>>
>> Aaron Conole (5):
>>   lib/net-proto: Create a new net-proto module for handling generic
>>     network protocols.
>>   lib/ct-state: Move ct-state macros to their own header.
>>   lib/packets: Remove flow_tnl_equal.
>>   lib/dp-packet: Fold the 'packets' module into dp-packets.
>>   libs: Introduce libovscommon to isolate common routines.
>
> Thanks for this cleanup, I like the direction this series takes.
> Some quick comments/nits.
>
> - I would put the flow_tnl_equal removal (kind of a fix) early in the series.
>
> - The renames and internal movement of code is hard to follow in patch 2.
> For example, two more renames were missed in the commitlog:
> * packet_set_ipv6_flow_label => ip_set_ipv6_flow_label
> * packet_set_ipv6_tc => ip_set_ipv6_tc
> I would add a (exhaustive) note in NEWS about the renames.

ACK - I'll add it in NEWS, and thanks for catching those.

> Because of those renames, indent can be fixed (the renamed functions
> have now a shorter prefix).

I'll go back through things and fix up the whitespace as I move them.
Maybe a good use case for AI to assist.

> If possible, have those renames in a preparation patch before moving
> all code around (and avoid moving helpers between them during this
> move).
> This is just to ease review, this could be squashed when applying, no
> strong opinion.

This is a good idea, I'll do it - doesn't need to be squashed either.

> - In patch 4, flow_tnl_(src|dst) should probably move to lib/flow.c
> rather than lib/dp-packet.c

Oops - I intended to move them there, but probably cut-paste too
aggressively.

> - In patch 4, I would prefix functions moved to dp-packet.c with
> dp_packet_ (but that's a huge rename... so not a strong opinion).

I wanted to avoid that (because as you note it is a huge rename).

> - You probably noticed that FreeBSD compilation is broken, some header
> inclusion is missing/broken.

Yep - getting a FreeBSD VM up and running to actually finish the port.

> - I prefer libopenvswitchutils.a (or -utils.a) but I can live with
> your libovscommon.a :-).

I had it called 'libovsutils' originally, but went with ovscommon
because it seemed "more appropriate."  I don't have a strong opinion, so
since I'm spinning again I'll move to something like libopenvswitch-utils.

> Isolating dp-packet is one important step.
> You listed vconn and ofpbuf, I think that the netdev* stuff also does
> not have to be in the common library.

Yes, baby steps :)  There are quite a bit of things I'd like to break
up to make the linking a bit cleaner.

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

Reply via email to