| Message ID | 20180916191112.1231-1-ffmancera@riseup.net |
|---|---|
| State | Changes Requested |
| Delegated to: | Pablo Neira |
| Headers | show |
| Series | [nft] src: osf: add ttl option support | expand |
I have not implemented tests and json support yet because I prefer to do it after the review of this patchset. Thanks! On 9/16/18 9:11 PM, Fernando Fernandez Mancera wrote: > Add support for ttl option in "osf" expression. Example: > > table ip foo { > chain bar { > type filter hook input priority filter; policy accept; > osf ttl 0 name "Linux" > } > } > > Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net> > --- > include/expression.h | 4 ++++ > include/linux/netfilter/nf_tables.h | 2 ++ > include/osf.h | 2 +- > src/netlink_delinearize.c | 5 ++++- > src/netlink_linearize.c | 1 + > src/osf.c | 13 +++++++++++-- > src/parser_bison.y | 4 ++-- > 7 files changed, 25 insertions(+), 6 deletions(-) > > diff --git a/include/expression.h b/include/expression.h > index f2c5c1a..d3cca1c 100644 > --- a/include/expression.h > +++ b/include/expression.h > @@ -337,6 +337,10 @@ struct expr { > uint32_t flags; > uint32_t result; > } fib; > + struct { > + /* EXPR_OSF */ > + uint8_t ttl; > + } osf; > }; > }; > > diff --git a/include/linux/netfilter/nf_tables.h b/include/linux/netfilter/nf_tables.h > index 143ebe2..6c3f08c 100644 > --- a/include/linux/netfilter/nf_tables.h > +++ b/include/linux/netfilter/nf_tables.h > @@ -938,10 +938,12 @@ enum nft_socket_keys { > * enum nft_osf_attributes - nf_tables osf expression netlink attributes > * > * @NFTA_OSF_DREG: destination register (NLA_U32: nft_registers) > + * @NFTA_OSF_TTL: Value of the TTL osf option (NLA_U8) > */ > enum nft_osf_attributes { > NFTA_OSF_UNSPEC, > NFTA_OSF_DREG, > + NFTA_OSF_TTL, > __NFTA_OSF_MAX > }; > #define NFT_OSF_MAX (__NFTA_OSF_MAX - 1) > diff --git a/include/osf.h b/include/osf.h > index 54cdd4a..23ea34d 100644 > --- a/include/osf.h > +++ b/include/osf.h > @@ -1,7 +1,7 @@ > #ifndef NFTABLES_OSF_H > #define NFTABLES_OSF_H > > -struct expr *osf_expr_alloc(const struct location *loc); > +struct expr *osf_expr_alloc(const struct location *loc, const uint8_t ttl); > > extern int nfnl_osf_load_fingerprints(struct netlink_ctx *ctx, int del); > > diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c > index 6c5188c..021ff7b 100644 > --- a/src/netlink_delinearize.c > +++ b/src/netlink_delinearize.c > @@ -636,8 +636,11 @@ static void netlink_parse_osf(struct netlink_parse_ctx *ctx, > { > enum nft_registers dreg; > struct expr *expr; > + uint8_t ttl; > + > + ttl = nftnl_expr_get_u8(nle, NFTNL_EXPR_OSF_TTL); > + expr = osf_expr_alloc(loc, ttl); > > - expr = osf_expr_alloc(loc); > dreg = netlink_parse_register(nle, NFTNL_EXPR_OSF_DREG); > netlink_set_register(ctx, dreg, expr); > } > diff --git a/src/netlink_linearize.c b/src/netlink_linearize.c > index 0bd946a..28f23da 100644 > --- a/src/netlink_linearize.c > +++ b/src/netlink_linearize.c > @@ -227,6 +227,7 @@ static void netlink_gen_osf(struct netlink_linearize_ctx *ctx, > > nle = alloc_nft_expr("osf"); > netlink_put_register(nle, NFTNL_EXPR_OSF_DREG, dreg); > + nftnl_expr_set_u8(nle, NFTNL_EXPR_OSF_TTL, expr->osf.ttl); > nftnl_rule_add_expr(ctx->nlr, nle); > } > > diff --git a/src/osf.c b/src/osf.c > index 85c9573..2341dfb 100644 > --- a/src/osf.c > +++ b/src/osf.c > @@ -7,11 +7,17 @@ > > static void osf_expr_print(const struct expr *expr, struct output_ctx *octx) > { > - nft_print(octx, "osf name"); > + nft_print(octx, "osf ttl %u name", expr->osf.ttl); > } > > static void osf_expr_clone(struct expr *new, const struct expr *expr) > { > + new->osf.ttl = expr->osf.ttl; > +} > + > +static bool osf_expr_cmp(const struct expr *e1, const struct expr *e2) > +{ > + return e1->osf.ttl == e2->osf.ttl; > } > > static const struct expr_ops osf_expr_ops = { > @@ -19,10 +25,11 @@ static const struct expr_ops osf_expr_ops = { > .name = "osf", > .print = osf_expr_print, > .clone = osf_expr_clone, > + .cmp = osf_expr_cmp, > .json = osf_expr_json, > }; > > -struct expr *osf_expr_alloc(const struct location *loc) > +struct expr *osf_expr_alloc(const struct location *loc, const uint8_t ttl) > { > unsigned int len = NFT_OSF_MAXGENRELEN * BITS_PER_BYTE; > const struct datatype *type = &string_type; > @@ -31,5 +38,7 @@ struct expr *osf_expr_alloc(const struct location *loc) > expr = expr_alloc(loc, &osf_expr_ops, type, > BYTEORDER_HOST_ENDIAN, len); > > + expr->osf.ttl = ttl; > + > return expr; > } > diff --git a/src/parser_bison.y b/src/parser_bison.y > index 85830d8..11f8ad5 100644 > --- a/src/parser_bison.y > +++ b/src/parser_bison.y > @@ -3087,9 +3087,9 @@ fib_tuple : fib_flag DOT fib_tuple > | fib_flag > ; > > -osf_expr : OSF NAME > +osf_expr : OSF NUM NAME > { > - $$ = osf_expr_alloc(&@$); > + $$ = osf_expr_alloc(&@$, $2); > } > ; > >
On Sun, Sep 16, 2018 at 09:11:12PM +0200, Fernando Fernandez Mancera wrote: > Add support for ttl option in "osf" expression. Example: > > table ip foo { > chain bar { > type filter hook input priority filter; policy accept; > osf ttl 0 name "Linux" Looking at nf_osf_ttl() * Currently, default behaviour is "check for exact TTL" if no ttl option is specified, which is -m osf --ttl 0, which works for local area network. Therefore: * We need an option to skip TTL checking, eg. 'ttl nocheck', which is mapping -m osf --ttl 2. * We need an option to check for globally-routable address, eg. 'ttl global', which is mapping -m osf --ttl 1. You could also add 'ttl local', but that seems to be the default behaviour anyway, so you could just document this.
On 9/18/18 1:16 AM, Pablo Neira Ayuso wrote: > On Sun, Sep 16, 2018 at 09:11:12PM +0200, Fernando Fernandez Mancera wrote: >> Add support for ttl option in "osf" expression. Example: >> >> table ip foo { >> chain bar { >> type filter hook input priority filter; policy accept; >> osf ttl 0 name "Linux" > > Looking at nf_osf_ttl() > > * Currently, default behaviour is "check for exact TTL" if no ttl option > is specified, which is -m osf --ttl 0, which works for local area > network. > > Therefore: > > * We need an option to skip TTL checking, eg. 'ttl nocheck', which is > mapping -m osf --ttl 2. > * We need an option to check for globally-routable address, eg. 'ttl > global', which is mapping -m osf --ttl 1. > > You could also add 'ttl local', but that seems to be the default > behaviour anyway, so you could just document this. > Okay, I am going to work on this. Thanks for the review.
Sorry if I have misunderstood you but right now, the function implements 'ttl nocheck' and 'ttl global' behaviours. Yes I am going to document that the default behaviour is 'ttl local'. So if I am not wrong this doesn't require changes. Did you mean something different or it is fine? Thanks! On 9/18/18 1:16 AM, Pablo Neira Ayuso wrote: > On Sun, Sep 16, 2018 at 09:11:12PM +0200, Fernando Fernandez Mancera wrote: >> Add support for ttl option in "osf" expression. Example: >> >> table ip foo { >> chain bar { >> type filter hook input priority filter; policy accept; >> osf ttl 0 name "Linux" > > Looking at nf_osf_ttl() > > * Currently, default behaviour is "check for exact TTL" if no ttl option > is specified, which is -m osf --ttl 0, which works for local area > network. > > Therefore: > > * We need an option to skip TTL checking, eg. 'ttl nocheck', which is > mapping -m osf --ttl 2. > * We need an option to check for globally-routable address, eg. 'ttl > global', which is mapping -m osf --ttl 1. > > You could also add 'ttl local', but that seems to be the default > behaviour anyway, so you could just document this. >
> On 9/18/18 1:16 AM, Pablo Neira Ayuso wrote: >> On Sun, Sep 16, 2018 at 09:11:12PM +0200, Fernando Fernandez Mancera >> wrote: >>> Add support for ttl option in "osf" expression. Example: >>> >>> table ip foo { >>> chain bar { >>> type filter hook input priority filter; policy accept; >>> osf ttl 0 name "Linux" >> >> Looking at nf_osf_ttl() >> >> * Currently, default behaviour is "check for exact TTL" if no ttl option >> is specified, which is -m osf --ttl 0, which works for local area >> network. >> >> Therefore: >> >> * We need an option to skip TTL checking, eg. 'ttl nocheck', which is >> mapping -m osf --ttl 2. >> * We need an option to check for globally-routable address, eg. 'ttl >> global', which is mapping -m osf --ttl 1. >> >> You could also add 'ttl local', but that seems to be the default >> behaviour anyway, so you could just document this. >> I am not sure how to modify the json support, so the version of tests that I have at this moment doesn't include json support. Any recommendations to start with? Thanks :-)
On Wed, Sep 26, 2018 at 08:53:08PM +0200, Fernando Fernandez Mancera wrote: > Sorry if I have misunderstood you but right now, the function implements > 'ttl nocheck' and 'ttl global' behaviours. Yes I am going to document that > the default behaviour is 'ttl local'. > > So if I am not wrong this doesn't require changes. Did you mean something > different or it is fine? Thanks! Given default is ttl local, I don't think we need an explicit option for this, so ttl nocheck and ttl global should be fine. Thanks.
diff --git a/include/expression.h b/include/expression.h index f2c5c1a..d3cca1c 100644 --- a/include/expression.h +++ b/include/expression.h @@ -337,6 +337,10 @@ struct expr { uint32_t flags; uint32_t result; } fib; + struct { + /* EXPR_OSF */ + uint8_t ttl; + } osf; }; }; diff --git a/include/linux/netfilter/nf_tables.h b/include/linux/netfilter/nf_tables.h index 143ebe2..6c3f08c 100644 --- a/include/linux/netfilter/nf_tables.h +++ b/include/linux/netfilter/nf_tables.h @@ -938,10 +938,12 @@ enum nft_socket_keys { * enum nft_osf_attributes - nf_tables osf expression netlink attributes * * @NFTA_OSF_DREG: destination register (NLA_U32: nft_registers) + * @NFTA_OSF_TTL: Value of the TTL osf option (NLA_U8) */ enum nft_osf_attributes { NFTA_OSF_UNSPEC, NFTA_OSF_DREG, + NFTA_OSF_TTL, __NFTA_OSF_MAX }; #define NFT_OSF_MAX (__NFTA_OSF_MAX - 1) diff --git a/include/osf.h b/include/osf.h index 54cdd4a..23ea34d 100644 --- a/include/osf.h +++ b/include/osf.h @@ -1,7 +1,7 @@ #ifndef NFTABLES_OSF_H #define NFTABLES_OSF_H -struct expr *osf_expr_alloc(const struct location *loc); +struct expr *osf_expr_alloc(const struct location *loc, const uint8_t ttl); extern int nfnl_osf_load_fingerprints(struct netlink_ctx *ctx, int del); diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c index 6c5188c..021ff7b 100644 --- a/src/netlink_delinearize.c +++ b/src/netlink_delinearize.c @@ -636,8 +636,11 @@ static void netlink_parse_osf(struct netlink_parse_ctx *ctx, { enum nft_registers dreg; struct expr *expr; + uint8_t ttl; + + ttl = nftnl_expr_get_u8(nle, NFTNL_EXPR_OSF_TTL); + expr = osf_expr_alloc(loc, ttl); - expr = osf_expr_alloc(loc); dreg = netlink_parse_register(nle, NFTNL_EXPR_OSF_DREG); netlink_set_register(ctx, dreg, expr); } diff --git a/src/netlink_linearize.c b/src/netlink_linearize.c index 0bd946a..28f23da 100644 --- a/src/netlink_linearize.c +++ b/src/netlink_linearize.c @@ -227,6 +227,7 @@ static void netlink_gen_osf(struct netlink_linearize_ctx *ctx, nle = alloc_nft_expr("osf"); netlink_put_register(nle, NFTNL_EXPR_OSF_DREG, dreg); + nftnl_expr_set_u8(nle, NFTNL_EXPR_OSF_TTL, expr->osf.ttl); nftnl_rule_add_expr(ctx->nlr, nle); } diff --git a/src/osf.c b/src/osf.c index 85c9573..2341dfb 100644 --- a/src/osf.c +++ b/src/osf.c @@ -7,11 +7,17 @@ static void osf_expr_print(const struct expr *expr, struct output_ctx *octx) { - nft_print(octx, "osf name"); + nft_print(octx, "osf ttl %u name", expr->osf.ttl); } static void osf_expr_clone(struct expr *new, const struct expr *expr) { + new->osf.ttl = expr->osf.ttl; +} + +static bool osf_expr_cmp(const struct expr *e1, const struct expr *e2) +{ + return e1->osf.ttl == e2->osf.ttl; } static const struct expr_ops osf_expr_ops = { @@ -19,10 +25,11 @@ static const struct expr_ops osf_expr_ops = { .name = "osf", .print = osf_expr_print, .clone = osf_expr_clone, + .cmp = osf_expr_cmp, .json = osf_expr_json, }; -struct expr *osf_expr_alloc(const struct location *loc) +struct expr *osf_expr_alloc(const struct location *loc, const uint8_t ttl) { unsigned int len = NFT_OSF_MAXGENRELEN * BITS_PER_BYTE; const struct datatype *type = &string_type; @@ -31,5 +38,7 @@ struct expr *osf_expr_alloc(const struct location *loc) expr = expr_alloc(loc, &osf_expr_ops, type, BYTEORDER_HOST_ENDIAN, len); + expr->osf.ttl = ttl; + return expr; } diff --git a/src/parser_bison.y b/src/parser_bison.y index 85830d8..11f8ad5 100644 --- a/src/parser_bison.y +++ b/src/parser_bison.y @@ -3087,9 +3087,9 @@ fib_tuple : fib_flag DOT fib_tuple | fib_flag ; -osf_expr : OSF NAME +osf_expr : OSF NUM NAME { - $$ = osf_expr_alloc(&@$); + $$ = osf_expr_alloc(&@$, $2); } ;
Add support for ttl option in "osf" expression. Example: table ip foo { chain bar { type filter hook input priority filter; policy accept; osf ttl 0 name "Linux" } } Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net> --- include/expression.h | 4 ++++ include/linux/netfilter/nf_tables.h | 2 ++ include/osf.h | 2 +- src/netlink_delinearize.c | 5 ++++- src/netlink_linearize.c | 1 + src/osf.c | 13 +++++++++++-- src/parser_bison.y | 4 ++-- 7 files changed, 25 insertions(+), 6 deletions(-)