On 3/10/26 10:15 AM, David Marchand wrote: > 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. > > Because of those renames, indent can be fixed (the renamed functions > have now a shorter prefix). > > 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.
I'm also a little unsure if we even need to rename the packet_* functions, as I'm not sure if we need to move them. They are moved to a new net-proto, but the packets module is deleted. I don't see a big difference between the net-proto and the packets naming, so I'm not sure why this move is done. We could just keep the functions in the packets.c. Or am I missing something? Also there is the include/openvswitch/packets.h public counterpart to packets.[ch]. It has to be taken into account and renamed as well if we're going to rename the internal module. > > - In patch 4, flow_tnl_(src|dst) should probably move to lib/flow.c > rather than lib/dp-packet.c > > - 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). > > - You probably noticed that FreeBSD compilation is broken, some header > inclusion is missing/broken. > > - I prefer libopenvswitchutils.a (or -utils.a) but I can live with > your libovscommon.a :-). > > Isolating dp-packet is one important step. > You listed vconn and ofpbuf, ofpbuf and vconn are used by the tools and a lot of other code, I'm not sure if we need to move them to a separate library. A lot of things rely on them. > I think that the netdev* stuff also does > not have to be in the common library. Note: FWIW, on a brief look at what is being separated, OVN will have to link both libraries anyway as it uses dp-packet and netlink notifiers. Best regards, Ilya Maximets. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
