Re: [iptables PATCH 4/5] xtables-monitor: Support ARP and bridge families

[Phil Sutter <phil@nwl.cc>]

On Thu, Aug 01, 2019 at 03:03:03PM +0200, Pablo Neira Ayuso wrote:
> On Thu, Aug 01, 2019 at 02:58:00PM +0200, Phil Sutter wrote:
> > On Thu, Aug 01, 2019 at 02:47:38PM +0200, Pablo Neira Ayuso wrote:
> > > On Thu, Aug 01, 2019 at 02:41:07PM +0200, Phil Sutter wrote:
> > > > Hi,
> > > > 
> > > > On Thu, Aug 01, 2019 at 02:30:40PM +0200, Pablo Neira Ayuso wrote:
> > > > > On Thu, Aug 01, 2019 at 02:00:48PM +0200, Phil Sutter wrote:
> > > [...]
> > > > > I think users will end up using --arp and --bridge for this. I myself
> > > > > will not remember this -0 and -1 thing.
> > > > 
> > > > That's correct. So I guess changing cmdline flags to -a/-b makes sense
> > > > either way.
> > > 
> > > In the rule side, getopt_long() is already pretty overloaded, just
> > > double check these are spare.
> > 
> > This is only about xtables-monitor cmdline, or am I missing something?
> 
> I was referring to the iptables rule command. Not sure it's worth
> there the alias. I think you mentioned that there's already -0 and -1
> in the rule command line, hence the -0 and -1 for xtables-monitor.

Why should xtables-monitor print something that can be used as input to
iptables?

> > > > > Feel free to explore any possibility, probably leaving the existing -0
> > > > > and -1 in place if you're afraid of breaking anything, add aliases and
> > > > > only document the more intuitive one. If you think this is worth
> > > > > exploring, of course.
> > > > 
> > > > I would omit the prefix from output if a family was selected. For
> > > > unfiltered xtables-monitor output, I would change the prefix to
> > > > something more readable, e.g.:
> > > > 
> > > > 'ip:  ',
> > > > 'ip6: ',
> > > > 'arp: ',
> > > > 'eb:  '
> > > > 
> > > > What do you think?
> > > 
> > > Probably use the long option name, which seems more readable to me:
> > > 
> > > EVENT: --ipv4 -t filter -A INPUT -j ACCEPT
> > 
> > Ah, good idea!
> > 
> > > I like that the event is printed using the {ip,...}tables syntax.
> > 
> > OK. --arp/--bridge won't work there, obviously. We could of course try
> > to change that, but I guess it's not feasible.
> 
> I think we would need a common parser, and that's not feasible. Unless
> there is some preparsing, just to check if the family option is in
> place, ie. -4, -6, --arp and --bridge, then route the parsing to the
> corresponding parser. It's a bit of extra glue code, not sure it's
> worth, just an idea / future work if helping all these tooling
> converge might be of interest.

Given the large differences in ebtables cmdline syntax to the other
tools, I consider it a plus to have different commands (and hence
separate "main" functions).

> > Also, IIRC 'iptables -6' was buggy in that it should fail but does
> > not. This is a compatibility issue I didn't get to fix yet.
> 
> Noted. I have seen the recent patch to fix this.

That was only for iptables-nft-restore. I am talking about plain
iptables:

| % iptables-legacy -6 -A FORWARD -j ACCEPT
| This is the IPv4 version of iptables.
| Try `iptables -h' or 'iptables --help' for more information.

iptables-nft accepts this but the result seems to be identical to just
omitting '-6'.

Cheers, Phil