Re: [PATCH nft] netlink: Print value sizes on Relational expression size mismatch
Máté Eckl wrote:
> On Thu, May 31, 2018 at 08:39:35PM +0200, Florian Westphal wrote:
> > Máté Eckl wrote:
> > > On Thu, May 31, 2018 at 04:48:58PM +0200, Pablo Neira Ayuso wrote:
> > > > On Thu, May 31, 2018 at 01:42:17PM +0200, Máté Eckl wrote:
> > > > > On Thu, May 31, 2018 at 10:57:49AM +0200, Pablo Neira Ayuso wrote:
> > > > > > > I just wanted to make sure that the only accepted values are 0
> > > > > > > and 1 and I
> > > > > > > didn't find other way to provide this check.
> > > > > >
> > > > > > You can reject this from the evaluation phase.
> > > > >
> > > > > Oh, earlier I didn't find how to do it, but now I think I did.
> > > > >
> > > > > Would you accept a new version of the patch with this?
> > > >
> > > > That looks good.
> > > >
> > > > Please tests if this will that work with maps too? eg.
> > > >
> > > > socket transparent ip saddr map { 1.1.1.1 : 1,
> > > > 2.2.2.2 : 0 }
> >
> > Pablo, this is to test if transparent flag is set, not to set it.
> >
> > There is no dreg. I'm not sure what the above should even do :-)
I meant 'sreg'.
> I think this should mean something like, match transparent eq 1 if saddr is
> 1.1.1.1 and transparent eq 0 if saddr is 2.2.2.2. But this is also just a
> guess.
That would be
socket transparent . ip saddr { 0 . 1.1.1.1, 1 . 2.2.2.2 }
"map" is used to obtain an input value, e.g.
mark set map socket transparent map { 1 : 42, 0 : 23 }
would set skb->mark to 42 or 23, depending on 'socket transparent'
state.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH nft] netlink: Print value sizes on Relational expression size mismatch
On Thu, May 31, 2018 at 08:39:35PM +0200, Florian Westphal wrote:
> Máté Eckl wrote:
> > On Thu, May 31, 2018 at 04:48:58PM +0200, Pablo Neira Ayuso wrote:
> > > On Thu, May 31, 2018 at 01:42:17PM +0200, Máté Eckl wrote:
> > > > On Thu, May 31, 2018 at 10:57:49AM +0200, Pablo Neira Ayuso wrote:
> > > > > > I just wanted to make sure that the only accepted values are 0 and
> > > > > > 1 and I
> > > > > > didn't find other way to provide this check.
> > > > >
> > > > > You can reject this from the evaluation phase.
> > > >
> > > > Oh, earlier I didn't find how to do it, but now I think I did.
> > > >
> > > > Would you accept a new version of the patch with this?
> > >
> > > That looks good.
> > >
> > > Please tests if this will that work with maps too? eg.
> > >
> > > socket transparent ip saddr map { 1.1.1.1 : 1,
> > > 2.2.2.2 : 0 }
>
> Pablo, this is to test if transparent flag is set, not to set it.
>
> There is no dreg. I'm not sure what the above should even do :-)
I think this should mean something like, match transparent eq 1 if saddr is
1.1.1.1 and transparent eq 0 if saddr is 2.2.2.2. But this is also just a guess.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH nft] netlink: Print value sizes on Relational expression size mismatch
Máté Eckl wrote:
> On Thu, May 31, 2018 at 04:48:58PM +0200, Pablo Neira Ayuso wrote:
> > On Thu, May 31, 2018 at 01:42:17PM +0200, Máté Eckl wrote:
> > > On Thu, May 31, 2018 at 10:57:49AM +0200, Pablo Neira Ayuso wrote:
> > > > > I just wanted to make sure that the only accepted values are 0 and 1
> > > > > and I
> > > > > didn't find other way to provide this check.
> > > >
> > > > You can reject this from the evaluation phase.
> > >
> > > Oh, earlier I didn't find how to do it, but now I think I did.
> > >
> > > Would you accept a new version of the patch with this?
> >
> > That looks good.
> >
> > Please tests if this will that work with maps too? eg.
> >
> > socket transparent ip saddr map { 1.1.1.1 : 1,
> > 2.2.2.2 : 0 }
Pablo, this is to test if transparent flag is set, not to set it.
There is no dreg. I'm not sure what the above should even do :-)
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH nft] netlink: Print value sizes on Relational expression size mismatch
On Thu, May 31, 2018 at 04:48:58PM +0200, Pablo Neira Ayuso wrote:
> On Thu, May 31, 2018 at 01:42:17PM +0200, Máté Eckl wrote:
> > On Thu, May 31, 2018 at 10:57:49AM +0200, Pablo Neira Ayuso wrote:
> > > > I just wanted to make sure that the only accepted values are 0 and 1
> > > > and I
> > > > didn't find other way to provide this check.
> > >
> > > You can reject this from the evaluation phase.
> >
> > Oh, earlier I didn't find how to do it, but now I think I did.
> >
> > Would you accept a new version of the patch with this?
>
> That looks good.
>
> Please tests if this will that work with maps too? eg.
>
> socket transparent ip saddr map { 1.1.1.1 : 1,
> 2.2.2.2 : 0 }
>
It does not, but neither it does with the former version :).
# nft add rule inet sockin sockchain socket transparent ip
saddr map { 1.1.1.1 : 1 , 2.2.2.2 : 0 }
Error: syntax error, unexpected saddr, expecting end of file or
newline or semicolon
add rule inet sockin sockchain socket transparent ip saddr map
{ 1.1.1.1 : 1 , 2.2.2.2 : 0 }
^
I'm not sure what can be the problem.
> > diff --git a/src/evaluate.c b/src/evaluate.c
> > index 56fea26..70d6b23 100644
> > --- a/src/evaluate.c
> > +++ b/src/evaluate.c
> > @@ -1712,9 +1712,11 @@ static int expr_evaluate_meta(struct eval_ctx *ctx,
> > struct expr **exprp)
> > return expr_evaluate_primary(ctx, exprp);
> > }
> >
> > -static int expr_evaluate_socket(struct eval_ctx *ctx, struct expr **exprp)
> > +static int expr_evaluate_socket(struct eval_ctx *ctx, struct expr **expr)
> > {
> > - return expr_evaluate_primary(ctx, exprp);
> > + __expr_set_context(>ectx, (*expr)->dtype, (*expr)->byteorder,
> > + (*expr)->len, 1);
> > + return 0;
> > }
> >
> > static int expr_evaluate_variable(struct eval_ctx *ctx, struct expr
> > **exprp)
> >
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH nft] netlink: Print value sizes on Relational expression size mismatch
On Thu, May 31, 2018 at 04:48:58PM +0200, Pablo Neira Ayuso wrote:
> On Thu, May 31, 2018 at 01:42:17PM +0200, Máté Eckl wrote:
> > On Thu, May 31, 2018 at 10:57:49AM +0200, Pablo Neira Ayuso wrote:
> > > > I just wanted to make sure that the only accepted values are 0 and 1
> > > > and I
> > > > didn't find other way to provide this check.
> > >
> > > You can reject this from the evaluation phase.
> >
> > Oh, earlier I didn't find how to do it, but now I think I did.
> >
> > Would you accept a new version of the patch with this?
>
> That looks good.
>
> Please tests if this will that work with maps too? eg.
>
> socket transparent ip saddr map { 1.1.1.1 : 1,
> 2.2.2.2 : 0 }
And send v2, this patch is not yet upstream :-)
> > diff --git a/src/evaluate.c b/src/evaluate.c
> > index 56fea26..70d6b23 100644
> > --- a/src/evaluate.c
> > +++ b/src/evaluate.c
> > @@ -1712,9 +1712,11 @@ static int expr_evaluate_meta(struct eval_ctx *ctx,
> > struct expr **exprp)
> > return expr_evaluate_primary(ctx, exprp);
> > }
> >
> > -static int expr_evaluate_socket(struct eval_ctx *ctx, struct expr **exprp)
> > +static int expr_evaluate_socket(struct eval_ctx *ctx, struct expr **expr)
> > {
> > - return expr_evaluate_primary(ctx, exprp);
> > + __expr_set_context(>ectx, (*expr)->dtype, (*expr)->byteorder,
> > + (*expr)->len, 1);
> > + return 0;
> > }
> >
> > static int expr_evaluate_variable(struct eval_ctx *ctx, struct expr
> > **exprp)
> >
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH nft] netlink: Print value sizes on Relational expression size mismatch
On Thu, May 31, 2018 at 01:42:17PM +0200, Máté Eckl wrote:
> On Thu, May 31, 2018 at 10:57:49AM +0200, Pablo Neira Ayuso wrote:
> > > I just wanted to make sure that the only accepted values are 0 and 1 and I
> > > didn't find other way to provide this check.
> >
> > You can reject this from the evaluation phase.
>
> Oh, earlier I didn't find how to do it, but now I think I did.
>
> Would you accept a new version of the patch with this?
That looks good.
Please tests if this will that work with maps too? eg.
socket transparent ip saddr map { 1.1.1.1 : 1,
2.2.2.2 : 0 }
> diff --git a/src/evaluate.c b/src/evaluate.c
> index 56fea26..70d6b23 100644
> --- a/src/evaluate.c
> +++ b/src/evaluate.c
> @@ -1712,9 +1712,11 @@ static int expr_evaluate_meta(struct eval_ctx *ctx,
> struct expr **exprp)
> return expr_evaluate_primary(ctx, exprp);
> }
>
> -static int expr_evaluate_socket(struct eval_ctx *ctx, struct expr **exprp)
> +static int expr_evaluate_socket(struct eval_ctx *ctx, struct expr **expr)
> {
> - return expr_evaluate_primary(ctx, exprp);
> + __expr_set_context(>ectx, (*expr)->dtype, (*expr)->byteorder,
> + (*expr)->len, 1);
> + return 0;
> }
>
> static int expr_evaluate_variable(struct eval_ctx *ctx, struct expr **exprp)
>
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH nft] netlink: Print value sizes on Relational expression size mismatch
On Thu, May 31, 2018 at 10:57:49AM +0200, Pablo Neira Ayuso wrote:
> > I just wanted to make sure that the only accepted values are 0 and 1 and I
> > didn't find other way to provide this check.
>
> You can reject this from the evaluation phase.
Oh, earlier I didn't find how to do it, but now I think I did.
Would you accept a new version of the patch with this?
diff --git a/src/evaluate.c b/src/evaluate.c
index 56fea26..70d6b23 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -1712,9 +1712,11 @@ static int expr_evaluate_meta(struct eval_ctx *ctx,
struct expr **exprp)
return expr_evaluate_primary(ctx, exprp);
}
-static int expr_evaluate_socket(struct eval_ctx *ctx, struct expr **exprp)
+static int expr_evaluate_socket(struct eval_ctx *ctx, struct expr **expr)
{
- return expr_evaluate_primary(ctx, exprp);
+ __expr_set_context(>ectx, (*expr)->dtype, (*expr)->byteorder,
+ (*expr)->len, 1);
+ return 0;
}
static int expr_evaluate_variable(struct eval_ctx *ctx, struct expr **exprp)
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH nft] netlink: Print value sizes on Relational expression size mismatch
On Thu, May 31, 2018 at 08:07:04AM +0200, Máté Eckl wrote: > On Wed, May 30, 2018 at 08:56:46PM +0200, Pablo Neira Ayuso wrote: > > On Wed, May 30, 2018 at 08:54:41PM +0200, Florian Westphal wrote: > > > Pablo Neira Ayuso wrote: > > > > I suspect you're missing some code in the evaluation phase? > > > > > > > > So you get left->len 1 and right->len 4? > > > > > > Yes, meta template sets len of 1, where as RHS is 8 bit. > > > > > > This can be fixed up during delinearization, in this case > > > we can know that kernel actually stores 1 byte. > > > > > > The 'template says its 1 bit' is only to force an error when > > > someone asks to match 'socket transparent 3'. > > > > > > I asked Mate to send this patch to help with future debugging, > > > so one can see what the mismatching values are. > > > > Use 8 bit field. I guess this will simplify things and it will map to > > what we have in the kernel. > > I just wanted to make sure that the only accepted values are 0 and 1 and I > didn't find other way to provide this check. You can reject this from the evaluation phase. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH nft] netlink: Print value sizes on Relational expression size mismatch
On Wed, May 30, 2018 at 08:54:41PM +0200, Florian Westphal wrote: > Pablo Neira Ayuso wrote: > > I suspect you're missing some code in the evaluation phase? > > > > So you get left->len 1 and right->len 4? > > Yes, meta template sets len of 1, where as RHS is 8 bit. > > This can be fixed up during delinearization, in this case > we can know that kernel actually stores 1 byte. > > The 'template says its 1 bit' is only to force an error when > someone asks to match 'socket transparent 3'. > > I asked Mate to send this patch to help with future debugging, > so one can see what the mismatching values are. Use 8 bit field. I guess this will simplify things and it will map to what we have in the kernel. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH nft] netlink: Print value sizes on Relational expression size mismatch
Pablo Neira Ayuso wrote: > I suspect you're missing some code in the evaluation phase? > > So you get left->len 1 and right->len 4? Yes, meta template sets len of 1, where as RHS is 8 bit. This can be fixed up during delinearization, in this case we can know that kernel actually stores 1 byte. The 'template says its 1 bit' is only to force an error when someone asks to match 'socket transparent 3'. I asked Mate to send this patch to help with future debugging, so one can see what the mismatching values are. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH nft] netlink: Print value sizes on Relational expression size mismatch
On Wed, May 30, 2018 at 04:01:12PM +0200, Máté Eckl wrote:
> Not all of the ocurances are covered as this information is not
> available in the scope where the error message is printed.
>
> Signed-off-by: Máté Eckl
> ---
> src/netlink_delinearize.c | 18 +-
> 1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
> index 52706b7..460ef7b 100644
> --- a/src/netlink_delinearize.c
> +++ b/src/netlink_delinearize.c
> @@ -272,14 +272,22 @@ static void netlink_parse_cmp(struct netlink_parse_ctx
> *ctx,
>
> if (left->len > right->len &&
> expr_basetype(left) != _type) {
> - return netlink_error(ctx, loc, "Relational expression size
> mismatch");
> + return netlink_error(ctx, loc, "Relational expression size
> mismatch."
> + "Left value size: %d, right value size:
> %d",
> + left->len, right->len);
> } else if (left->len > 0 && left->len < right->len) {
> + int llen = left->len,
> + rlen = right->len;
> left = netlink_parse_concat_expr(ctx, loc, sreg, right->len);
> - if (left == NULL)
> - return;
> - right = netlink_parse_concat_data(ctx, loc, sreg, right->len,
> right);
> - if (right == NULL)
> + if (left != NULL)
> + right = netlink_parse_concat_data(ctx, loc, sreg,
> + right->len, right);
> + if (!left || !right) {
> + netlink_error(ctx, loc,
> + "Left value size: %d, right value size:
> %d",
> + llen, rlen);
> return;
> + }
> }
I suspect you're missing some code in the evaluation phase?
So you get left->len 1 and right->len 4?
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH nft] netlink: Print value sizes on Relational expression size mismatch
Máté Eckl wrote: > Not all of the ocurances are covered as this information is not > available in the scope where the error message is printed. Looks good to me, I'll apply in a few hours in case there are no further comments. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
