Hi Phil,
This series looks to good to me. Only one comment below.
On Tue, Jul 18, 2017 at 05:40:27PM +0200, Phil Sutter wrote:
> This flag is required by userspace only, so can live within userdata.
> It's sole purpose is for 'nft monitor' to detect half-open ranges (which
> are comprised of a single element only).
>
> Signed-off-by: Phil Sutter <phil@xxxxxx>
> ---
> include/expression.h | 1 +
> include/rule.h | 7 ++++++
> src/netlink.c | 66 ++++++++++++++++++++++++++++++++++++++--------------
> src/segtree.c | 5 ++++
> 4 files changed, 62 insertions(+), 17 deletions(-)
>
> diff --git a/include/expression.h b/include/expression.h
> index 68a36e8af792a..202eb4c140eda 100644
> --- a/include/expression.h
> +++ b/include/expression.h
> @@ -180,6 +180,7 @@ enum expr_flags {
> EXPR_F_PROTOCOL = 0x4,
> EXPR_F_INTERVAL_END = 0x8,
> EXPR_F_BOOLEAN = 0x10,
> + EXPR_F_INTERVAL_OPEN = 0x20,
Could you place this here?
diff --git a/include/expression.h b/include/expression.h
index 7ddcc3226267..12434b335b71 100644
--- a/include/expression.h
+++ b/include/expression.h
@@ -256,6 +256,7 @@ struct expr {
uint64_t expiration;
const char *comment;
struct stmt *stmt;
+ unsigned int elem_flags;
};
struct {
/* EXPR_UNARY */
We can probably move EXPR_F_INTERVAL_END there too in a follow up
patch.
> };
>
> #include <payload.h>
> diff --git a/include/rule.h b/include/rule.h
> index 7424b21c6e019..592c93ddd0e2f 100644
> --- a/include/rule.h
> +++ b/include/rule.h
> @@ -503,4 +503,11 @@ enum udata_set_type {
> };
> #define UDATA_SET_MAX (__UDATA_SET_MAX - 1)
>
> +enum udata_set_elem_type {
> + UDATA_SET_ELEM_COMMENT,
> + UDATA_SET_ELEM_FLAGS,
> + __UDATA_SET_ELEM_MAX,
> +};
> +#define UDATA_SET_ELEM_MAX (__UDATA_SET_ELEM_MAX - 1)
> +
> #endif /* NFTABLES_RULE_H */
> diff --git a/src/netlink.c b/src/netlink.c
> index 2e30622de4bb1..be26188e097aa 100644
> --- a/src/netlink.c
> +++ b/src/netlink.c
> @@ -211,7 +211,7 @@ static struct nftnl_set_elem *alloc_nftnl_setelem(const struct expr *set,
> const struct expr *elem, *key, *data;
> struct nftnl_set_elem *nlse;
> struct nft_data_linearize nld;
> - struct nftnl_udata_buf *udbuf;
> + struct nftnl_udata_buf *udbuf = NULL;
>
> nlse = nftnl_set_elem_alloc();
> if (nlse == NULL)
> @@ -232,13 +232,22 @@ static struct nftnl_set_elem *alloc_nftnl_setelem(const struct expr *set,
> if (elem->timeout)
> nftnl_set_elem_set_u64(nlse, NFTNL_SET_ELEM_TIMEOUT,
> elem->timeout);
> - if (elem->comment) {
> + if (elem->comment || expr->flags & EXPR_F_INTERVAL_OPEN) {
> udbuf = nftnl_udata_buf_alloc(NFT_USERDATA_MAXLEN);
> if (!udbuf)
> memory_allocation_error();
> - if (!nftnl_udata_put_strz(udbuf, UDATA_TYPE_COMMENT,
> + }
> + if (elem->comment) {
> + if (!nftnl_udata_put_strz(udbuf, UDATA_SET_ELEM_COMMENT,
> elem->comment))
> memory_allocation_error();
> + }
> + if (expr->flags & EXPR_F_INTERVAL_OPEN) {
> + if (!nftnl_udata_put_u32(udbuf, UDATA_SET_ELEM_FLAGS,
> + EXPR_F_INTERVAL_OPEN))
Better make a new definition for this? Instead of using
EXPR_F_INTERVAL_OPEN which is 0x20.
Once we expose this value, we cannot change it otherwise we may break
nft between software updates, we should avoid 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
