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. > >> >>>> 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
