Thanks for the comments Florian.
(Sorry I missed the mailing list form my first response.)
Florian Westphal <fw@xxxxxxxxx> ezt írta (időpont: 2018. máj. 7., H, 23:23):
> Máté Eckl <ecklm94@xxxxxxxxx> wrote:
> > I have been working on a skeleton for socket matching which is required
by
> > tproxy support.
> >
> > See the WIP patch below and please comment if you have something to
note.
> Excellent, thanks for working on this.
> > My thoughts:
> > * The parser is fine with this version of socket matching, matching the
flags is
> > still to be implemented.
> > * We could treat this tproxy specifically all the way (eg. `tproxy
socket`), but I
> > think this solution is more extensible and flexible. Matching flags
with this
> > syntax makes other socket flags matchable and thus usable outside of
the
> > use-case of transparent proxying.
> > * `isset` is probably not the best keyword to describe this, I have
also thought
> > of `present` but maybe you also have some suggestions. If we want to
match
> > socket flags this way, we need a keyword here.
> >
> > Regards,
> > Máté
> >
> > -- 8< --
> > === Basic matching ===
> >
> > eg.: `meta socket isset 1`
> >
> > This matches when there is a socket with the destination ip address
> > assigned to it as local address.
> I like this, but see below.
> > The new keyword `isset` represents a boolean, and it can later be reused
> > for the pointer type meta attributes, where the attribute is not
> > necessarily present at the time these rules are evaluated. For example
> > sk_user_data, sk_security, etc.
> What about re-using the 'exists' keyword for this?
> "meta socket exists"?
This is a good idea, I didn't notice this one.
> Altough *I* would expect that this is eqivalent of
> skb->sk != NULL, not result of a lookup in the socket hash tables, so i
> would prefer something that makes this more obvious.
Actually I think isset is more of what you say. That's why I didn't think
it to be a good solution.
Exists seems to be more global, so it does not have to be set, it is enough
that it exists and can be found, so a lookup can be part of the decision.
But I will think about this.
> > === Socket specific matching ===
> >
> > `meta socket flags <flags>`
> >
> > This would match when `meta socket isset` matches AND the given flags
> > are set on the socket.
> Where is the 'isset' keyword here?
> meta socket exists flags ...
> won't work as above is really just a short-version of:
> meta socket eq 1 flags ..
I thought of this as a totally independent keyword.
In my plan the command line description would look like this:
meta socket < exists <val> | flags <flags> >
So I would not match flags with the `exists` keyword. This solves
ambiguity, and it seemed to be intuitive for me. Matching flags on NULL is
0 or false anyways, so I didn't think, exists keyword is necessary here.
The lookup should be made in both cases.
> Which will match
> meta_expr relational_expr integer_expr flags), so we don't know what
> 'flags' is here, and where it refers to.
> Perhaps its better to use
> tproxy socket exists [ flags ], as that would allow
> to create a new statement where we know the context/structure
> and where the flags apply to.
> We can also re-purpose such a statement later to get TPROXY working:
> tproxy socket to 1.2.3.4:8080 flags ...
> Just some suggestions, I'm by no means good at defining ui things :-/
My problem with this is that using the tproxy "target" does not require the
socket matching to work in every case. Although it is true that those cases
are rare but can possibly be useful for someone.
So this syntax does not seem to be appropriate for socket matching, as it
can be useful in other use-cases and is not *necessarily* part of a tproxy
setup.
So if you accept the description I gave on my plan above, I would prefer
that.
Does anyone have any other suggestion regarding this?
> > --- a/src/meta.c
> > +++ b/src/meta.c
> > @@ -439,6 +439,8 @@ static const struct meta_template meta_templates[]
= {
> > BYTEORDER_BIG_ENDIAN), /*
avoid conversion; doesn't have endianess */
> > [NFT_META_SECPATH] = META_TEMPLATE("secpath", &boolean_type,
> > BITS_PER_BYTE,
BYTEORDER_HOST_ENDIAN),
> > + [NFT_META_SUBKEY_ISSET] = META_TEMPLATE("isset", &boolean_type,
> > + 1 ,
BYTEORDER_HOST_ENDIAN),
> The NFT_META_XXXXX define the elements that can be accessed, by the
> meta expression, such as skb->secpath or skb->mark.
> E.g. when user says 'meta mark 42' then meta expression is created with
> NFT_META_MARK key attribute, so kernel will know it has to fetch
> skb->mark.
> So, this would be something like
> [NFT_META_SK] = META_TEMPLATE("sk", &boolean_type,
> BITS_PER_BYTE,
BYTEORDER_HOST_ENDIAN),
> I suggest to use BITS_PER_BYTE rather than 'bit' as the nft vm can
> access bytes but needs to generate shifts/masks to access bits.
Ok, I will consider this.
--
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
