| Message ID | 146115978018.25287.16460508385150502285.stgit@nfdev2.cica.es |
|---|---|
| State | Not Applicable |
| Delegated to: | Pablo Neira |
| Headers | show |
On Wed, Apr 20, 2016 at 03:43:00PM +0200, Arturo Borrero Gonzalez wrote: > Currently, if we choose a set name larger than allowed, the error message is: > Error: Could not process rule: Numerical result out of range > > Let's inform the user with a better error message. > > We can discuss later if length of set names should be increased, but I think > this better error reporting is necessary right now to avoid headaches to users. /* The max length of strings including NUL: set and type identifiers */ #define IPSET_MAXNAMELEN 32 I would like that we get the same length as ipset, this should make it easier for people to migrate. This would require a bit of work though since the interface name size is limited by the register size. Not much a problem, but it would require a bit of code adjustments from the kernel. So let me postpone this userspace check. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 27 Apr 2016, Pablo Neira Ayuso wrote: > On Wed, Apr 20, 2016 at 03:43:00PM +0200, Arturo Borrero Gonzalez wrote: > > Currently, if we choose a set name larger than allowed, the error message is: > > Error: Could not process rule: Numerical result out of range > > > > Let's inform the user with a better error message. > > > > We can discuss later if length of set names should be increased, but I think > > this better error reporting is necessary right now to avoid headaches to users. > > /* The max length of strings including NUL: set and type identifiers */ > #define IPSET_MAXNAMELEN 32 > > I would like that we get the same length as ipset, this should make it > easier for people to migrate. I think it's all right if set names are longer in nftables. That won't cause incompatibilites, unless someone wants to move from nftables to ipset. Best regards, Jozsef - E-mail : kadlec@blackhole.kfki.hu, kadlecsik.jozsef@wigner.mta.hu PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences H-1525 Budapest 114, POB. 49, Hungary -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Apr 27, 2016 at 07:36:38PM +0200, Jozsef Kadlecsik wrote: > On Wed, 27 Apr 2016, Pablo Neira Ayuso wrote: > > > On Wed, Apr 20, 2016 at 03:43:00PM +0200, Arturo Borrero Gonzalez wrote: > > > Currently, if we choose a set name larger than allowed, the error message is: > > > Error: Could not process rule: Numerical result out of range > > > > > > Let's inform the user with a better error message. > > > > > > We can discuss later if length of set names should be increased, but I think > > > this better error reporting is necessary right now to avoid headaches to users. > > > > /* The max length of strings including NUL: set and type identifiers */ > > #define IPSET_MAXNAMELEN 32 > > > > I would like that we get the same length as ipset, this should make it > > easier for people to migrate. > > I think it's all right if set names are longer in nftables. That won't > cause incompatibilites, unless someone wants to move from nftables to > ipset. Currently in nftables we have 16 bytes, so we're smaller than ipset. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 27 April 2016 at 19:14, Pablo Neira Ayuso <pablo@netfilter.org> wrote: > On Wed, Apr 20, 2016 at 03:43:00PM +0200, Arturo Borrero Gonzalez wrote: >> Currently, if we choose a set name larger than allowed, the error message is: >> Error: Could not process rule: Numerical result out of range >> >> Let's inform the user with a better error message. >> >> We can discuss later if length of set names should be increased, but I think >> this better error reporting is necessary right now to avoid headaches to users. > > /* The max length of strings including NUL: set and type identifiers */ > #define IPSET_MAXNAMELEN 32 > > I would like that we get the same length as ipset, this should make it > easier for people to migrate. > > This would require a bit of work though since the interface name size > is limited by the register size. Not much a problem, but it would > require a bit of code adjustments from the kernel. > > So let me postpone this userspace check. We would need the userspace check anyway to avoid the very misleading error reporting from the kernel. Then if tomorrow we change the name length, we just need an oneliner here to update with the new size. I remember in the past we discussed using set names as completely variable size strings, but that's another discussion. If loading a ruleset with 'nft -f' with lot of nested and included files, the error is just very difficult to track down; I've hit it several times already. == t.nft == flush ruleset table t { set abcdefghijklmopqrst { type ipv4_addr } } =========== % nft -f t.nft t.nft:2:1-2: Error: Could not process rule: Numerical result out of range table t { ^^ So I don't understand the point in not including some more informative message right now.
On 28 April 2016 at 09:42, Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com> wrote: > On 27 April 2016 at 19:14, Pablo Neira Ayuso <pablo@netfilter.org> wrote: >> On Wed, Apr 20, 2016 at 03:43:00PM +0200, Arturo Borrero Gonzalez wrote: >>> Currently, if we choose a set name larger than allowed, the error message is: >>> Error: Could not process rule: Numerical result out of range >>> >>> Let's inform the user with a better error message. >>> >>> We can discuss later if length of set names should be increased, but I think >>> this better error reporting is necessary right now to avoid headaches to users. >> >> /* The max length of strings including NUL: set and type identifiers */ >> #define IPSET_MAXNAMELEN 32 >> >> I would like that we get the same length as ipset, this should make it >> easier for people to migrate. >> >> This would require a bit of work though since the interface name size >> is limited by the register size. Not much a problem, but it would >> require a bit of code adjustments from the kernel. >> >> So let me postpone this userspace check. > > We would need the userspace check anyway to avoid the very misleading > error reporting from the kernel. > Then if tomorrow we change the name length, we just need an oneliner > here to update with the new size. I remember in the past we discussed > using set names as completely variable size strings, but that's > another discussion. > > If loading a ruleset with 'nft -f' with lot of nested and included > files, the error is just very difficult to track down; I've hit it > several times already. > > == t.nft == > flush ruleset > table t { > set abcdefghijklmopqrst { > type ipv4_addr > } > } > =========== > > % nft -f t.nft > t.nft:2:1-2: Error: Could not process rule: Numerical result out of range > table t { > ^^ > > So I don't understand the point in not including some more informative > message right now. > -- ping
diff --git a/src/evaluate.c b/src/evaluate.c index 346e34f..b86e5b6 100644 --- a/src/evaluate.c +++ b/src/evaluate.c @@ -2123,6 +2123,7 @@ static int setelem_evaluate(struct eval_ctx *ctx, struct expr **expr) static int set_evaluate(struct eval_ctx *ctx, struct set *set) { + size_t namelen = IFNAMSIZ - 1; struct table *table; const char *type; @@ -2136,6 +2137,10 @@ static int set_evaluate(struct eval_ctx *ctx, struct set *set) type = set->flags & SET_F_MAP ? "map" : "set"; + if (strlen(set->handle.set) > namelen) + return cmd_error(ctx, "%s maximum allowed name length is %lu", + type, namelen); + if (set->keytype == NULL) return set_error(ctx, set, "%s definition does not specify " "key data type", type);
Currently, if we choose a set name larger than allowed, the error message is: Error: Could not process rule: Numerical result out of range Let's inform the user with a better error message. We can discuss later if length of set names should be increased, but I think this better error reporting is necessary right now to avoid headaches to users. Signed-off-by: Arturo Borrero Gonzalez <arturo.borrero.glez@gmail.com> --- src/evaluate.c | 5 +++++ 1 file changed, 5 insertions(+) -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html