Ilya Maximets <[email protected]> writes: > On 3/11/26 2:33 PM, Ilya Maximets wrote: >> On 3/11/26 2:01 PM, Aaron Conole wrote: >>> 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? >> >> What we could do if to put all the OpenFlow handling modules like all the >> lib/ofp* code and rconn/vconn into something like libopenflow. But I'm >> not really sure if we'll gain anything from doing that, as it will depend >> on the libovscommon. And by itself will unlikley to be large enough to >> be important to split up from the libovscommon. We could do that anyway >> for the internal separation, but it's definitely a low priority thing. >> >> The other way around might be to have libopenflow to only include the >> lib/ofp* stuff, and actually export this one, as all that is a public API >> anyways. This way external applications could use all the OpenFlow >> functions from this library without needing the libovscommon. But then >> libovscommon will need to link libopenflow. This is also an interesting >> way forward, but for entirely different reasons. And we'll have to make >> sure that all the OpenFlow functions actually produce messages with correct >> length fields in this case, as users will not be able to rely on vconn >> fixing them. > > Hrm, lib/ofp* stuff also relies on a lot of common data structures and > libraries, so it wouldn't be easy to actually properly export these > without requiring linking to libovscommon.
Yeah, maybe it doesn't make sense to split it out separately. I was thinking that by having them separate, we reduce the surface area of any one library and can start making meaningful decisions about which libraries we may want to export. BUT as you note, ofp-* is also requiring lots of the libovscommon functionality anyway. I'll see what happens if I rip out the rconn, vconn, and ofp-* stuff into ovscommon (or libopenvswitch-utils that I think David suggested). I also am trying to keep things modular on the utilities side as well. For example, if there was a way to separate some of these so that ovs-ofctl and ovs-vsctl don't need to always include the same libraries but that might just be a too lofty goal that isn't practical (or worth it). The exercise has been pretty good at reducing the number of includes each C file pulls in, anyway. :) >> >>> >>>>> 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 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
