Am 09.04.2015 um 13:07 schrieb Patrick McHardy:
On 09.04, Alexander Holler wrote:
E.g. the ICMPv6 parameter-problem is good example. In the linux headers it
is called ICMPV6_PARAMPROB, nft named it param-problem and in documentations
it is often named as parameter-problem.
So if nft would use icmpv6_paramprob, the documentation could just refer to
the c-headers.
No, an icmpv6_ prefix is redundant and I don't see any benefit in doing
that. "Documentations", whatever that is, don't matter, what matters is
our documentation. And whether we point people to that or some header
file really doesn't matter, except that we don't expect people to read
header files to use nft. Its a tool for admins, not programmers.
Even some admins can code.
Because nft is just at 0.4 and not widely used (or usable), I decided it's
worse to write another follow up in order to try to fix things before they
can't be fixed anymore.
1. I don't think that a brutforce-approach like "if error try something
else" is the right way to fix a problem in the parser.
Its perfectly fine in this case since the error is only caught in spots
where we expect symbolic constants. So basically its treating the next
keyword as symbolic constant no matter what.
I wonder how the error routine in that patch knowns that a symbolic
constant is expected. Where does it get that information from? And if
so, why is in such a case error called anyways? And how are problems in
the grammar identified in which a token or string might match? Of
course, my bison knowledge got very rusty as I haven't fiddled with it
for a long time, but I still think it's brute-force or trial-and-error
approach.
2. "icmpv6_paramprob" (or even something like "icmpv6_parameter-problem")
doesn't have something redundant. It's a name for constant and e.g.
"icmp_redirect" is a much more descriptive name than just "redirect" because
"icmp_redirect" describes the context too and thus the name is much more
verbose and readable (besides that it would fix the problem the parser
currently has).
It is redundant since it can only occur in the context of ICMP expressions.
icmp type icmp_redirect is obviously redundant.
It's just a name as such it can't be redundant if you want to allow
names. Of course, there might be people which are unaware that "icmp_"
in "icmp_redirect" is just part of the name for a constant, but those
people shouldn't write firewall rules anyway.
3. I don't see why admins have to use another set of names for constants
than developers. Inventing a new set of names for a list of constants for
which there already exist a very widely used set of names just leads to more
confusion. And if it's ok to invent new names, why does nft use
"param-problem" and not "parameter-problem"? Of course, I would suggest to
use the existing name icmp_parameterprob (like it's used in every
c/c++-source).
In case of ICMP we use the same names that iptables used, so this actually
spares admins from getting used to new constants. We're not going to use
source code identifiers, there's no benefit at all, especially if you
consider that Linux headers use different identifiers than the BSD headers.
nft isn't in use on BSD and if you think taking BSD out of a corner
makes sense, I wonder how compatible the names, nft uses, are, with what
is used by ipf or one of the other BSD firewall packages. As the answer
is the names are incompatible, arguing with BSD is nonsense here.
But I agree, ICMPv6 shouldn't use param-problem but parameter-problem for
consistency with both ICMP and ip6tables.
Anyway, I've tried it a second time. Can't do more. So I will now
entering the popcorn-state.
Alexander Holler
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
