Ilya Maximets <[email protected]> writes: > 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?
I started by just doing that, but it became a bit strange to read for me as packets.h/.c in lib had nothing to do with 'packets' and much to do with net/inet protocol work. > 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. It makes sense to me. The 'naming' of include/openvswitch/packets.h seems too generic for what is there. >> >> - 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. You would prefer them to be in this common library instead? >> 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. Yes. The idea is to reduce that surface. This is a first step towards that goal, though. > Best regards, Ilya Maximets. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
