Re: [PATCH nft] Add tproxy support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Máté Eckl <ecklm94@xxxxxxxxx> wrote:
> This patch is built on the commit not applied yet with the title:
> 	evaluate: Detect address family in inet context

You can add this ...

> Example ruleset:
> 	table inet x {
> 		chain y {
> 			type filter hook prerouting priority -150; policy accept;
> 			socket transparent 1 mark set 0x00000001 accept
> 			tproxy mark set 0x00000001 counter packets 611 bytes 46181
> 			meta l4proto tcp tproxy to :50080 mark set 0x00000001 counter packets 202 bytes 13600
> 		}
> 	}
> 
> Signed-off-by: Máté Eckl <ecklm94@xxxxxxxxx>
> ---

... here, as its removed automatically when using 'git am'.

The ruleset above seems a bit strange because it uses two 'tproxy'
ruleset.
Also, this patch should include a man page entry to document
socket/tproxy.

A patch to add test cases would also be nice (can be as followup
after patch has been applied of course).

> +	if (stmt->tproxy.addr) {
> +		addr_reg = get_register(ctx, NULL);
> +		registers++;
> +
> +		if (stmt->tproxy.addr->ops->type != EXPR_RANGE) {
> +			netlink_gen_expr(ctx, stmt->tproxy.addr, addr_reg);
> +			netlink_put_register(nle, NFTNL_EXPR_TPROXY_REG_ADDR,
> +					     addr_reg);
> +		}

This looks weird.  Why this != EXPR_RANGE check?
It should never happen, because in evaluate.c you do this for
ports, so why not also reject the unwanted expression type there?

> if (stmt->tproxy.port != NULL) {
>       if (stmt->tproxy.port->ops->type == EXPR_RANGE)
>            return -EOPNOTSUPP;

Might be a good idea to use stmt_error() though to indicate that this
doesn't work.

> +	if (stmt->tproxy.port) {
> +		port_reg = get_register(ctx, NULL);
> +		registers++;
> +
> +		if (stmt->tproxy.port->ops->type != EXPR_RANGE) {
> +			netlink_gen_expr(ctx, stmt->tproxy.port, port_reg);
> +			netlink_put_register(nle, nftnl_reg_port, port_reg);
> +		}

Likewise, this is always true, right?

> +tproxy_stmt		:	TPROXY
> +			{
> +				$$ = tproxy_stmt_alloc(&@$);
> +			}

I wonder what the use case for plain TPROXY is.
Do we need to support that?  How is it useful?

We should probably enforce at least the port number, no?

If not, it would be good to illustrate this with an example
in the nft man page at least.
--
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



[Index of Archives]     [Netfitler Users]     [LARTC]     [Bugtraq]     [Yosemite Forum]

  Powered by Linux