I was quite overwhelmed by the whole thing, so I might not have made
my point clear.
While I find e.g. `tcp flags fin,ack,rst` being not the same as `tcp
flags { fin, ack, rst }` confusing, in this case it is still
"reasonable", as we can say that in the former it is a
"comma-separated list" (hence no space), while in the latter it is a
set (hence the spaces).
The real issue here is that a comma-separated list itself can have
totally different meanings. For example,
1. If (just) `fin,rst,ack`, it means "any of the bits set".
2. If `== fin,rst,ack`, it means (fin | rst | ack)
3. If `fin,rst,ack / fin,rst,ack`, it means (fin | rst | ack). *(For
both the value and the mask)*
4. If `== fin,rst,ack / fin,rst,ack`, it means (fin | rst | ack).
*(For both the value and the mask)*
So how could anyone have thought in the first case `fin,rst,ack` does
not have the same meaning as it does in the other cases? That's what I
would call "unreasonable". Also, if in any case e.g. (just) `syn` is
not considered a (single-value) "comma-separated list", it's
"unreasonable" as well.
Or in other words, I don't find a behavior/shortcut like, "if a mask
is not specified explicitly, a mask that is equal to the value is
implied, yet not when we compare the value (e.g. ==)", sensible /
sensical. (Would anyone?)
And you know what, comparing this with the `ct state` is unfair. The
fact that you did so sort of explains why we end up in this...mess.
(Not trying to say it's your fault but rather, how the issue could
have happened.)
In the case of `ct state`, while we use the different bits for the
states, the states themselves are mutually exclusive (AFAIK, e.g. a
packet can't be new and established at the same time). People assume
e.g. `related,established` to be *practically* equivalent to `{
related, established }` not because they would think like, "generally
a comma-separated list should mean any of states", but either because
they know in advance a packet can only be of either, or, they assume
such similar syntax should mean the same thing. (The truth is, `ct
state related,established` is NOT *logically* the same as `ct state {
related, established }`; the former will be true for a packet if its
state can be / is related | established.)
On Thu, 29 Jul 2021 at 15:03, Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote:
>
> On Thu, Jul 29, 2021 at 09:48:21AM +0800, Tom Yan wrote:
> > I'm not sure it's just me or you that are missing something here.
> >
> > On Wed, 28 Jul 2021 at 05:05, Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote:
> > >
> > > On Wed, Jul 28, 2021 at 02:36:18AM +0800, Tom Yan wrote:
> > > > Hmm, that means `tcp flags & (fin | syn | rst | ack) syn` is now
> > > > equivalent to 'tcp flags & (fin | syn | rst | ack) == syn'.
> > >
> > > Yes, those two are equivalent.
> > >
> > > > Does that mean `tcp flags syn` (was supposed to be and) is now
> > > > equivalent to `tcp flags == syn`
> > >
> > > tcp flag syn
> > >
> > > is a shortcut to match on the syn bit regarless other bit values, it's
> > > a property of the bitmask datatypes.
> >
> > Don't you think the syntax will be inconsistent then? As a user, it
> > looks pretty irrational to me: with a mask, just `syn` checks the
> > exact value of the flags (combined); without a mask, it checks and
> > checks only whether the specific bit is on.
> >
> > At least to me I would then expect `tcp flags syn` should be
> > equivalent / is a shortcut to `tcp flags & (fin | syn | rst | psh |
> > ack | urg | ecn | cwr) syn` hence `tcp flags & (fin | syn | rst | psh
> > | ack | urg | ecn | cwr) == syn` hence `tcp flags == syn`.
>
> As I said, think of a different use-case:
>
> ct state new,established
>
> people are _not_ expecting to match on both flags to be set on (that
> will actually never happen).
>
> Should ct state and tcp flags use the same syntax (comma-separated
> values) but intepret things in a different way? I don't think so.
>
> You can use:
>
> nft describe ct state
>
> to check for the real datatype behind this selector: it's a bitmask.
> For this datatype the implicit operation is not ==.
>
> > > tcp flags == syn
> > >
> > > is an exact match, it checks that the syn bit is set on.
> > >
> > > > instead of `tcp flags & syn == syn` / `tcp flags & syn != 0`?
> > >
> > > these two above are equivalent, I just sent a patch to fix the
> > > tcp flags & syn == syn case.
> > >
> > > > Suppose `tcp flags & syn != 0` should then be translated to `tcp flags
> > > > syn / syn` instead, please note that while nft translates `tcp flags &
> > > > syn == syn` to `tcp flags syn / syn`, it does not accept the
> > > > translation as input (when the mask is not a comma-separated list):
> > > >
> > > > # nft --debug=netlink add rule meh tcp_flags 'tcp flags syn / syn'
> > > > Error: syntax error, unexpected newline, expecting comma
> > > > add rule meh tcp_flags tcp flags syn / syn
> > > > ^
> > >
> > > The most simple way to express this is: tcp flags == syn.
> >
> > That does not sound right to me at all. Doesn't `syn / syn` means
> > "with the mask (the second/"denominator" `syn`) applied on the flags,
> > we get the value (the first/"nominator" `syn`), which means `tcp flags
> > & syn == syn` instead of `tcp flags == syn` (which in turn means all
> > bits but syn are cleared).
>
> tcp flags syn / syn makes no sense, it's actually: tcp flags syn.
>
> The goal is to provide a compact syntax for most common operations, so
> users do not need to be mingling with explicit binary expressions
> (which is considered to be a "more advanced" operation).
