diff mbox

[RFC,nft] syntax: replace '=>' with '=:'

Message ID 20140112212454.GA15677@macbook.localnet
State Superseded
Headers show

Commit Message

Patrick McHardy Jan. 12, 2014, 9:24 p.m. UTC
On Sun, Jan 12, 2014 at 09:28:21PM +0100, Arturo Borrero Gonzalez wrote:
> On 12 January 2014 21:17, Patrick McHardy <kaber@trash.net> wrote:
> > On Sun, Jan 12, 2014 at 08:41:33PM +0100, Arturo Borrero Gonzalez wrote:
> >> Almost all shell uses the '>' character as a key for redirecting
> >> stdout/stderr to a file.
> >> So, using it in the syntax means that the administrator is forced to scape the
> >> character, or look for other workaround.
> >>
> >> With this patch, '=>' is replaced with '=:', thus avoiding such situation.
> >
> > I'm not opposed to this, but I like (despite the shell problematic) the =>
> > syntax better, so I'd suggest to just add an alternative syntax.
> >
> > As further simplification, why not simply use ':'?
> 
> Well, I also like using just ':'
> 
> But maybe we clash with IPv6 addresses in some cases:
> 
> nft add rule ip6 filter input ip6 saddr vmap { ::1 : accept , ::2 : drop }
> nft add rule ip6 filter input ip6 saddr vmap { ::1:accept , ::2:drop }
> 
> nft add rule ip6 filter input meta dnat set tcp dport map { 80 : ::1,
> 8888 : ::2 }
> nft add rule ip6 filter input meta dnat set tcp dport map { 80:::1, 8888:::2 }
> 
> what do you think?

IPv6 addresses are recognized by the parser, so it should be fine. I just
compile tested this patch, at least bison doesn't report any errors.


--
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

Comments

Arturo Borrero Jan. 12, 2014, 9:47 p.m. UTC | #1
On 12 January 2014 22:24, Patrick McHardy <kaber@trash.net> wrote:
>
> IPv6 addresses are recognized by the parser, so it should be fine. I just
> compile tested this patch, at least bison doesn't report any errors.
>

'=:' works also as a visual separator.

If using ':', in the case of IPv6, we can end with:
2a00:9ac0:c1ca:27::150:123

Bison may detect it wisely, but I don't know where the address
start/ends, don't you?

Or we can force/require a space in the syntax:
2a00:9ac0:c1ca:27::150 : 123

If we decide to use ':', this last case is better, IMHO.

If you don't like '=:', what about:
'--'
'-:'
'---'
'--:'
or
':--'
Patrick McHardy Jan. 12, 2014, 9:55 p.m. UTC | #2
On Sun, Jan 12, 2014 at 10:47:01PM +0100, Arturo Borrero Gonzalez wrote:
> On 12 January 2014 22:24, Patrick McHardy <kaber@trash.net> wrote:
> >
> > IPv6 addresses are recognized by the parser, so it should be fine. I just
> > compile tested this patch, at least bison doesn't report any errors.
> >
> 
> '=:' works also as a visual separator.
> 
> If using ':', in the case of IPv6, we can end with:
> 2a00:9ac0:c1ca:27::150:123
> 
> Bison may detect it wisely, but I don't know where the address
> start/ends, don't you?
> 
> Or we can force/require a space in the syntax:
> 2a00:9ac0:c1ca:27::150 : 123
> 
> If we decide to use ':', this last case is better, IMHO.

We don't need to enforce this IMO. Any reasonable user will most likely
add the space himself. If not, no problem, as long as there is no ambiguity.

> If you don't like '=:', what about:
> '--'
> '-:'
> '---'
> '--:'
> or
> ':--'

Neither of. Too long, and for a mapping ':' or '=>' seem a reasonable
choice. ':=' is more like an assignment, which doesn't fit too well.

I'd say go for ':', if the user doesn't insert spaces and can't read
his own rules anymore, his fault.
--
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
Pablo Neira Ayuso Jan. 13, 2014, 10:44 a.m. UTC | #3
On Sun, Jan 12, 2014 at 09:55:12PM +0000, Patrick McHardy wrote:
> > If you don't like '=:', what about:
> > '--'
> > '-:'
> > '---'
> > '--:'
> > or
> > ':--'
> 
> Neither of. Too long, and for a mapping ':' or '=>' seem a reasonable
> choice. ':=' is more like an assignment, which doesn't fit too well.
>
> I'd say go for ':', if the user doesn't insert spaces and can't read
> his own rules anymore, his fault.

I like ':' is used in python dictinaries too. Erlang uses ',' as
separator and ruby was using the '=>' that causes some troubles with
bash if not escaped. I think there is not chance for ambiguity, but
I'm going to make more tests and get back to you.
--
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
diff mbox

Patch

diff --git a/src/netlink.c b/src/netlink.c
index 59bd8e4..0f8275c 100644
--- a/src/netlink.c
+++ b/src/netlink.c
@@ -332,7 +332,7 @@  int netlink_add_rule_batch(struct netlink_ctx *ctx,
 					     ctx->seqnum);
 		if (err < 0)
 			netlink_io_error(ctx, &rule->location,
-					 "Could not add rule to batch: %s",
+					 "Could not add rule: %s",
 					 strerror(errno));
 	}
 	nft_rule_free(nlr);
diff --git a/src/parser.y b/src/parser.y
index 7c18875..f91746a 100644
--- a/src/parser.y
+++ b/src/parser.y
@@ -149,7 +149,6 @@  static void location_update(struct location *loc, struct location *rhs, int n)
 %token ASTERISK			"*"
 %token DASH			"-"
 %token AT			"@"
-%token ARROW			"=>"
 %token VMAP			"vmap"
 
 %token INCLUDE			"include"
@@ -764,7 +763,7 @@  map_block		:	/* empty */	{ $$ = $<set>-1; }
 			|	map_block	common_block
 			|	map_block	stmt_seperator
 			|	map_block	TYPE
-						identifier	ARROW	identifier
+						identifier	COLON	identifier
 						stmt_seperator
 			{
 				$1->keytype = datatype_lookup_byname($3);
@@ -1309,11 +1308,11 @@  set_list_member_expr	:	opt_newline	expr	opt_newline
 			{
 				$$ = $2;
 			}
-			|	opt_newline	map_lhs_expr	ARROW	concat_expr	opt_newline
+			|	opt_newline	map_lhs_expr	COLON	concat_expr	opt_newline
 			{
 				$$ = mapping_expr_alloc(&@$, $2, $4);
 			}
-			|	opt_newline	map_lhs_expr	ARROW	verdict_expr	opt_newline
+			|	opt_newline	map_lhs_expr	COLON	verdict_expr	opt_newline
 			{
 				$$ = mapping_expr_alloc(&@$, $2, $4);
 			}
diff --git a/src/scanner.l b/src/scanner.l
index 0b8abac..9fa5471 100644
--- a/src/scanner.l
+++ b/src/scanner.l
@@ -209,7 +209,6 @@  addrstring	({macaddr}|{ip4addr}|{ip6addr})
 "@"			{ return AT; }
 "$"			{ return '$'; }
 "="			{ return '='; }
-"=>"			{ return ARROW; }
 "vmap"			{ return VMAP; }
 
 "include"		{ return INCLUDE; }