mbox series

[nf-next,0/2] ebtables: add support for ICMP and IGMP type/code matching

Message ID cover.1520150717.git.mschiffer@universe-factory.net
Headers show
Series ebtables: add support for ICMP and IGMP type/code matching | expand

Message

Matthias Schiffer March 4, 2018, 8:28 a.m. UTC
I recently found myself in a situation that required me to filter IGMP
packets of certain types on a bridge. Switching to nftables is
unfortunately not an option at the moment because of hardware constraints,
in particular regarding code size; therefore, this patchset adds support
for such matches to ebtables. For completeness and feature-parity with
IPv6, matches for ICMP are added as well.

Matthias Schiffer (2):
  ebtables: add support for matching ICMP type and code
  ebtables: add support for matching IGMP type

 include/uapi/linux/netfilter_bridge/ebt_ip.h | 15 +++++--
 net/bridge/netfilter/ebt_ip.c                | 58 +++++++++++++++++++++++-----
 2 files changed, 60 insertions(+), 13 deletions(-)

Comments

Florian Westphal March 4, 2018, 9:40 a.m. UTC | #1
Matthias Schiffer <mschiffer@universe-factory.net> wrote:
> I recently found myself in a situation that required me to filter IGMP
> packets of certain types on a bridge. Switching to nftables is
> unfortunately not an option at the moment because of hardware constraints,
> in particular regarding code size;

Is there anything we can do on nftables side to make switch more viable?
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Matthias Schiffer March 4, 2018, 11:16 a.m. UTC | #2
On 03/04/2018 10:40 AM, Florian Westphal wrote:
> Matthias Schiffer <mschiffer@universe-factory.net> wrote:
>> I recently found myself in a situation that required me to filter IGMP
>> packets of certain types on a bridge. Switching to nftables is
>> unfortunately not an option at the moment because of hardware constraints,
>> in particular regarding code size;
> 
> Is there anything we can do on nftables side to make switch more viable?
> 

Our system (the open mesh network framework Gluon) is based on OpenWrt, and
our non-ebtables firewall is implemented as a ruleset for the OpenWrt
firewall fw3, which is based on iptables. Porting fw3 to libnftnl is
planned, but I don't expect it to be happening any time soon.

Switching from ebtables to nftables without also dropping iptables from
Gluon is definitely out of the question, which leaves us with two options
for the switch to nftables: waiting for (or working on) the fw3 port, or
replacing fw3 altogether (which would also force users of Gluon to work
with lower-level rules when making custom changes to the firewall of their
deployment).

Because of the above challenges, I have not looked into the nftables
userspace components in much detail yet. libnftnl is ~45KB (MIPS,
LZMA-compressed), libmnl another ~6KB, which is acceptable if we can throw
out all of iptables and ebtables.

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.

The size of the nft binary is much more problematic (~165KB compressed); I
assume we will have to create our own specialized replacement based on
libnftnl.

Kind regards,
Matthias
Duncan Roe March 14, 2018, 7:24 a.m. UTC | #3
On Sun, Mar 04, 2018 at 12:16:55PM +0100, Matthias Schiffer wrote:
> On 03/04/2018 10:40 AM, Florian Westphal wrote:
> > Matthias Schiffer <mschiffer@universe-factory.net> wrote:
[...]
> Switching from ebtables to nftables without also dropping iptables from
> Gluon is definitely out of the question, which leaves us with two options
> for the switch to nftables: waiting for (or working on) the fw3 port, or
> replacing fw3 altogether (which would also force users of Gluon to work
> with lower-level rules when making custom changes to the firewall of their
> deployment).

Please try this in a test environment / VM that mirrors your production setup:

1. Run Gluon / fw3 (i.e. your normal xtables rule scripts). Then capture what
they are:

> { set -x;for i in filter nat mangle raw;do iptables -t $i -L -v -n --line-numbers;done;set +x; } >iptables.txt 2>&1

repeat for ip6tables if necessary:
> { set -x;for i in filter nat mangle raw;do ip6tables -t $i -L -v -n --line-numbers;done;set +x; } >ip6tables.txt 2>&1

> { set -x;for i in filter nat broute;do ebtables -t $i -L --Ln --Lc --Lmac2;done;set +x; } > ebtables.txt 2>&1

2. Switch over to using the nft compatibility layer. As root, do:

> set -e; cd /usr/sbin; for i in xtables-multi ebtables; do if [ -x $i.orig ]; then rm -v $i; mv -iv $i.orig $i; else mv -iv $i $i.orig; ln -sv xtables-compat-multi $i; fi; done; cd -; set +e

For this to work, you will need iptables newer or equal "Sun Feb 25 18:14:00
2018 +1100" (commit 632ace7c2947dbb70f74ea263b86ff68de391622).

3. Verify that the output from "ls -lct /usr/sbin | tail -n+2 | head -n4" looks
like:

> lrwxrwxrwx 1 root root       20 Mar  8 10:13 ebtables -> xtables-compat-multi
> -rwxr-xr-x 1 root root    75176 Mar  8 10:13 ebtables.orig
> lrwxrwxrwx 1 root root       20 Mar  8 10:13 xtables-multi -> xtables-compat-multi
> -rwxr-xr-x 1 root root   341928 Mar  8 10:13 xtables-multi.orig

4. Reboot and run your normal Gluon / fw3 (xtables) rule scripts.

This is a test envrironment right? Of course you *could* avoid a reboot and
instead remove Linux Kernel Modules and xtables rules, but reboot is easier.

Repeat step 1 except first move iptables.txt , ip6tables.txt & ebtables.txt to
=.old (e.g. iptables.txt -> iptables.txt.old).

diff iptables.txt.old iptables.txt (and so on).

Report any differences to this list. If there are no differences, you have
successfully converted to the nft compatibility layer.

Assuming there are no differences, try

nft list ruleset

Carefully inspect the output for missing detail. For example, look for "reject"
with no qualifyinmg  "with icmp type". Report any differences to this list.

If all looks ok, you have done a complete conversion to nftables.

5. If all is not OK, you can revert to the old xtables commands by running the
command in step 2 again.

HTH,

Cheers ... Duncan.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pablo Neira Ayuso March 20, 2018, 4:24 p.m. UTC | #4
On Sun, Mar 04, 2018 at 09:28:52AM +0100, Matthias Schiffer wrote:
> I recently found myself in a situation that required me to filter IGMP
> packets of certain types on a bridge. Switching to nftables is
> unfortunately not an option at the moment because of hardware constraints,
> in particular regarding code size; therefore, this patchset adds support
> for such matches to ebtables. For completeness and feature-parity with
> IPv6, matches for ICMP are added as well.

Series applied, thanks.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Matthias Schiffer April 11, 2018, 4:40 a.m. UTC | #5
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?

Kind regards,
Matthias
Florian Westphal April 11, 2018, 6:59 a.m. UTC | #6
Matthias Schiffer <mschiffer@universe-factory.net> wrote:
> 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%.

[..]

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

Seems like a good idea to split this up.

I think as first step you could even send a patch that
just excludes all unneeded snprintf etc. code from the build.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pablo Neira Ayuso April 11, 2018, 8:03 a.m. UTC | #7
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 :-)
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Matthias Schiffer April 11, 2018, 8:45 a.m. UTC | #8
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.

Kind regards,
Matthias
Pablo Neira Ayuso April 11, 2018, 8:47 a.m. UTC | #9
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.

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.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Matthias Schiffer April 11, 2018, 9:10 a.m. UTC | #10
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.
Phil Sutter April 11, 2018, 11:53 a.m. UTC | #11
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'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
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Matthias Schiffer April 11, 2018, 12:32 p.m. UTC | #12
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
>
Phil Sutter April 12, 2018, 7:21 a.m. UTC | #13
Hi,

On Wed, Apr 11, 2018 at 02:32:29PM +0200, Matthias Schiffer wrote:
[...]
> 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.

Assuming that libnftables is the only user of libnftnl we do care about,
I guess if it can live without the printing routines from libnftnl there
shouldn't be a problem in dropping them.

So I just had a look at where libnftables prints nftnl objects:

* Netlink debug output.
* Monitor output if JSON output is requested. (Actually also for XML
  output, but libnftnl doesn't support that.)
* Export command.

My implementation should replace the latter two completely, so
everything boils down to finding an alternative for netlink debug
output. Maybe it is possible to go with a more generic approach
(something like a semi-intelligent hexdump or so) so we don't have to
keep custom printing functions for every possible nftnl type?

Ideas anyone?

Cheers, Phil
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html