On 04/11/2018 01:53 PM, Phil Sutter wrote: > Hi, > > On Wed, Apr 11, 2018 at 11:10:36AM +0200, Matthias Schiffer wrote: >> 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? > > I'm currently working on an alternative to the standard syntax in nft > (or precisely libnftables) which is JSON based. Since with my patches in > place, people will be able to call 'nft -j list ruleset' and use its > output as full replacement to 'nft export vm json', we may get rid of > all JSON export functionality in libnftnl (which the latter command just > wraps). > > OTOH I don't expect any users for (any) JSON interface on an embedded > system at all, so I guess with JSON support in libnftnl wrapping related > functions in #ifdef's should be the right approach. > >>> 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 wonder how you plan to interface with nftables if you consider > disabling human readable output altogether. Since libnftables is > completely based on the human readable representation, it is pretty much > useless without text output (unless you're fine with write-only > interface ;). > > Then OTOH there's libnftnl, but interfacing with that directly is pretty > painful. Is this amongst your options to choose from? I plan to use libnftnl directly. If removing parts of the API from libnftnl is acceptable at this point, moving all parts in libnftnl dealing with human-readable or JSON input/output to libnftables would be an alternative to splitting another library out of libnftnl. I have not fully decided on my approach yet, but I will probably end up doing one or both of 1) replacing the iptables backend of the OpenWrt firewall "fw3" with libnftnl 2) creating a Lua binding for libnftnl tailored to our needs > >> 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. > > I'm looking forward to that, thanks! > > Cheers, Phil >
Attachment:
signature.asc
Description: OpenPGP digital signature
