On 04/11/2018 10:47 AM, Pablo Neira Ayuso wrote: > On Wed, Apr 11, 2018 at 10:45:27AM +0200, Matthias Schiffer wrote: >> On 04/11/2018 10:03 AM, Pablo Neira Ayuso wrote: >>> On Wed, Apr 11, 2018 at 06:40:34AM +0200, Matthias Schiffer wrote: >>>> On 03/04/2018 12:16 PM, Matthias Schiffer wrote: >>>>> I noticed that more than 25% of binary size of libnftnl are made up of >>>>> snprintf functions. Having these in a library with the goal to abstract the >>>>> netlink interface of nftables seems questionable to me, but I have no idea >>>>> if it would be viable to move these functions to nft or to a separate library. >>>> >>>> As an experiment, I created a reduced version of libnftnl by ripping out >>>> all import/export functions and related code like buffer handling. This >>>> reduced the size of libnftnl.so from 155KB to 110KB (on x86-64, -Os, >>>> stripped, uncompressed), a reduction of roughly 30%. >>>> >>>> I would like to look into splitting libnftnl into two parts, which could be >>>> called libnftnl-core and libnftnl, to make nftables more suited for tiny >>>> embedded systems. All basic functions that do not deal with textual >>>> representations of rules (i.e. the reduced libnftnl I built) would be moved >>>> into libnftnl-core. >>>> >>>> Does this sound like a good idea, and would such a drastic change be >>>> acceptable for upstream inclusion, given the current libnftnl API can be >>>> preserved? >>> >>> Could you wrap the import/export code around the --with-json-parsing? >>> I mean, turn this into --with-json or such, so you can just compile >>> out that code, which is what is giving you the savings in terms of >>> size, right? >>> >>> I'm trying to keep it simple here :-) >>> >> >> If possible, I'd not only like to get rid of the JSON export support, but >> also the snprintf_default code; in short, anything dealing with >> human-readable rules, as this code is simply not necessary for a firewall >> application that just wants to configure rulesets. >> >> A libnftables without any printf support (i.e. without >> nftnl_ruleset_fprintf() etc.) would not be sufficient to run the nft CLI >> utility. In consequence, we (OpenWrt/Gluon) would need to ship two >> different flavous of libnftables: A "tiny" version for small devices, and a >> "full" version for users that want to use the nft CLI to read/write the >> low-level rules. Separating libnftnl into a core library and an >> import/export library used by nft seems like better software design to me >> than adding more configuration switches. > > I understand, but probably that json support will be deprecated soon > because IIRC Phil is working on something better. Any details on this replacement? Will it be some kind of binary import/export, or another textual representation? > So I'm not sure it's worth to split this library in two, then to > deprecate the non-core part just weeks later. That's why I think > configure time option will be less work for you and will allow us to > make an step to let this code go away. No problem, we don't have a clear plan for the migration to nftables in OpenWrt or Gluon yet, so if this replacement is likely to be finished (or at least under review) in a few weeks, I can just wait that long and reevaluate my idea before doing unnecessary work :) I'll also gather a few more numbers (savings in code size for compressed MIPS16 binaries, which is my platform of interest) and compare the sizes of completely removing printf support vs. only removing JSON printing and keeping snprintf_default intact.
Attachment:
signature.asc
Description: OpenPGP digital signature
