[PATCH nft 1/3] parser: fix parsing of ethernet protocol types

[PATCH nft 1/3] parser: fix parsing of ethernet protocol types

[Pablo Neira Ayuso]

This allows us to use the protocol type keyword, eg.

nft add rule ip filter output meta protocol ip6 counte
                                            ^^^

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/parser.y |   29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/src/parser.y b/src/parser.y
index 038282e..23662f7 100644
--- a/src/parser.y
+++ b/src/parser.y
@@ -23,6 +23,7 @@
 #include <expression.h>
 #include <utils.h>
 #include <parser.h>
+#include <if_ether.h>
 #include <erec.h>
 
 #include "parser.h"
@@ -1418,6 +1419,13 @@ vlan_hdr_expr		:	VLAN	vlan_hdr_field
 			{
 				$$ = payload_expr_alloc(&@$, &payload_vlan, $2);
 			}
+			|	VLAN
+			{
+				uint16_t data = ETH_P_8021Q;
+				$$ = constant_expr_alloc(&@$, &ethertype_type,
+							 BYTEORDER_HOST_ENDIAN,
+							 sizeof(data) * BITS_PER_BYTE, &data);
+			}
 			;
 
 vlan_hdr_field		:	ID		{ $$ = VLANHDR_VID; }
@@ -1430,6 +1438,13 @@ arp_hdr_expr		:	ARP	arp_hdr_field
 			{
 				$$ = payload_expr_alloc(&@$, &payload_arp, $2);
 			}
+			|	ARP
+			{
+				uint16_t data = ETH_P_ARP;
+				$$ = constant_expr_alloc(&@$, &ethertype_type,
+							 BYTEORDER_HOST_ENDIAN,
+							 sizeof(data) * BITS_PER_BYTE, &data);
+			}
 			;
 
 arp_hdr_field		:	HTYPE		{ $$ = ARPHDR_HRD; }
@@ -1443,6 +1458,13 @@ ip_hdr_expr		:	IP	ip_hdr_field
 			{
 				$$ = payload_expr_alloc(&@$, &payload_ip, $2);
 			}
+			|	IP
+			{
+				uint16_t data = ETH_P_IP;
+				$$ = constant_expr_alloc(&@$, &ethertype_type,
+							 BYTEORDER_HOST_ENDIAN,
+							 sizeof(data) * BITS_PER_BYTE, &data);
+			}
 			;
 
 ip_hdr_field		:	VERSION		{ $$ = IPHDR_VERSION; }
@@ -1484,6 +1506,13 @@ ip6_hdr_expr		:	IP6	ip6_hdr_field
 			{
 				$$ = payload_expr_alloc(&@$, &payload_ip6, $2);
 			}
+			|	IP6
+			{
+				uint16_t data = ETH_P_IPV6;
+				$$ = constant_expr_alloc(&@$, &ethertype_type,
+							 BYTEORDER_HOST_ENDIAN,
+							 sizeof(data) * BITS_PER_BYTE, &data);
+			}
 			;
 
 ip6_hdr_field		:	VERSION		{ $$ = IP6HDR_VERSION; }
-- 
1.7.10.4

[PATCH nft 2/3] payload: fix crash when wrong ethernet protocol type is used

[Pablo Neira Ayuso]

nft add rule ip filter output meta protocol xyz counter
                                            ^^^

This fix is similar to 4097ad7 ("meta: fix crash when parsing
unresolvable mark values").

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/payload.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/src/payload.c b/src/payload.c
index 86d75fa..decfcd6 100644
--- a/src/payload.c
+++ b/src/payload.c
@@ -993,6 +993,7 @@ static struct error_record *ethertype_parse(const struct expr *sym,
 {
 	struct error_record *erec;
 
+	*res = NULL;
 	erec = sym->dtype->basetype->parse(sym, res);
 	if (erec != NULL)
 		return erec;
-- 
1.7.10.4

Re: [PATCH nft 2/3] payload: fix crash when wrong ethernet protocol type is used

[Patrick McHardy]

On Wed, Jan 15, 2014 at 09:30:22PM +0100, Pablo Neira Ayuso wrote:
> nft add rule ip filter output meta protocol xyz counter
>                                             ^^^
> 
> This fix is similar to 4097ad7 ("meta: fix crash when parsing
> unresolvable mark values").

I already fixed that for mark_type, but didn't realize we have more
instances of this. I double checked and we should be good now.

> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> ---
>  src/payload.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/src/payload.c b/src/payload.c
> index 86d75fa..decfcd6 100644
> --- a/src/payload.c
> +++ b/src/payload.c
> @@ -993,6 +993,7 @@ static struct error_record *ethertype_parse(const struct expr *sym,
>  {
>  	struct error_record *erec;
>  
> +	*res = NULL;
>  	erec = sym->dtype->basetype->parse(sym, res);
>  	if (erec != NULL)
>  		return erec;
> -- 
> 1.7.10.4
> 
> --
> 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

[PATCH nft 3/3] payload: fix inconsistency in ethertype output

[Pablo Neira Ayuso]

Use ip6 instead of ipv6.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/payload.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/payload.c b/src/payload.c
index decfcd6..bfd8ba4 100644
--- a/src/payload.c
+++ b/src/payload.c
@@ -982,7 +982,7 @@ static const struct symbol_table ethertype_tbl = {
 	.symbols	= {
 		SYMBOL("ip",		ETH_P_IP),
 		SYMBOL("arp",		ETH_P_ARP),
-		SYMBOL("ipv6",		ETH_P_IPV6),
+		SYMBOL("ip6",		ETH_P_IPV6),
 		SYMBOL("vlan",		ETH_P_8021Q),
 		SYMBOL_LIST_END
 	},
-- 
1.7.10.4

Re: [PATCH nft 3/3] payload: fix inconsistency in ethertype output

[Patrick McHardy]

On Wed, Jan 15, 2014 at 09:30:23PM +0100, Pablo Neira Ayuso wrote:
> Use ip6 instead of ipv6.

Sorry, didn't see this before. That leaves just the first problem for patch
1/3.

> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> ---
>  src/payload.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/payload.c b/src/payload.c
> index decfcd6..bfd8ba4 100644
> --- a/src/payload.c
> +++ b/src/payload.c
> @@ -982,7 +982,7 @@ static const struct symbol_table ethertype_tbl = {
>  	.symbols	= {
>  		SYMBOL("ip",		ETH_P_IP),
>  		SYMBOL("arp",		ETH_P_ARP),
> -		SYMBOL("ipv6",		ETH_P_IPV6),
> +		SYMBOL("ip6",		ETH_P_IPV6),
>  		SYMBOL("vlan",		ETH_P_8021Q),
>  		SYMBOL_LIST_END
>  	},
> -- 
> 1.7.10.4

Re: [PATCH nft 1/3] parser: fix parsing of ethernet protocol types

[Patrick McHardy]

On Wed, Jan 15, 2014 at 09:30:21PM +0100, Pablo Neira Ayuso wrote:
> This allows us to use the protocol type keyword, eg.
> 
> nft add rule ip filter output meta protocol ip6 counte
>                                             ^^^

I see two problems with this patch:

- the mapping to ETH_P_* is fixed. In case of f.i. meta nfproto relational
  expression it would have to map to NFPROTO_* values. So I think we should
  use symbolic expressions instead of constants and leave parsing to the
  evaluation phase-

- we're still using a mix of ip6 and ipv6. Lets also fix that, ideally
  as a patch before this one.

I can take care of this if you like.


> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> ---
>  src/parser.y |   29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/src/parser.y b/src/parser.y
> index 038282e..23662f7 100644
> --- a/src/parser.y
> +++ b/src/parser.y
> @@ -23,6 +23,7 @@
>  #include <expression.h>
>  #include <utils.h>
>  #include <parser.h>
> +#include <if_ether.h>
>  #include <erec.h>
>  
>  #include "parser.h"
> @@ -1418,6 +1419,13 @@ vlan_hdr_expr		:	VLAN	vlan_hdr_field
>  			{
>  				$$ = payload_expr_alloc(&@$, &payload_vlan, $2);
>  			}
> +			|	VLAN
> +			{
> +				uint16_t data = ETH_P_8021Q;
> +				$$ = constant_expr_alloc(&@$, &ethertype_type,
> +							 BYTEORDER_HOST_ENDIAN,
> +							 sizeof(data) * BITS_PER_BYTE, &data);
> +			}
>  			;
>  
>  vlan_hdr_field		:	ID		{ $$ = VLANHDR_VID; }
> @@ -1430,6 +1438,13 @@ arp_hdr_expr		:	ARP	arp_hdr_field
>  			{
>  				$$ = payload_expr_alloc(&@$, &payload_arp, $2);
>  			}
> +			|	ARP
> +			{
> +				uint16_t data = ETH_P_ARP;
> +				$$ = constant_expr_alloc(&@$, &ethertype_type,
> +							 BYTEORDER_HOST_ENDIAN,
> +							 sizeof(data) * BITS_PER_BYTE, &data);
> +			}
>  			;
>  
>  arp_hdr_field		:	HTYPE		{ $$ = ARPHDR_HRD; }
> @@ -1443,6 +1458,13 @@ ip_hdr_expr		:	IP	ip_hdr_field
>  			{
>  				$$ = payload_expr_alloc(&@$, &payload_ip, $2);
>  			}
> +			|	IP
> +			{
> +				uint16_t data = ETH_P_IP;
> +				$$ = constant_expr_alloc(&@$, &ethertype_type,
> +							 BYTEORDER_HOST_ENDIAN,
> +							 sizeof(data) * BITS_PER_BYTE, &data);
> +			}
>  			;
>  
>  ip_hdr_field		:	VERSION		{ $$ = IPHDR_VERSION; }
> @@ -1484,6 +1506,13 @@ ip6_hdr_expr		:	IP6	ip6_hdr_field
>  			{
>  				$$ = payload_expr_alloc(&@$, &payload_ip6, $2);
>  			}
> +			|	IP6
> +			{
> +				uint16_t data = ETH_P_IPV6;
> +				$$ = constant_expr_alloc(&@$, &ethertype_type,
> +							 BYTEORDER_HOST_ENDIAN,
> +							 sizeof(data) * BITS_PER_BYTE, &data);
> +			}
>  			;
>  
>  ip6_hdr_field		:	VERSION		{ $$ = IP6HDR_VERSION; }
> -- 
> 1.7.10.4
> 
> --
> 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

Re: [PATCH nft 1/3] parser: fix parsing of ethernet protocol types

[Pablo Neira Ayuso]

On Thu, Jan 16, 2014 at 04:28:16PM +0000, Patrick McHardy wrote:
> On Wed, Jan 15, 2014 at 09:30:21PM +0100, Pablo Neira Ayuso wrote:
> > This allows us to use the protocol type keyword, eg.
> > 
> > nft add rule ip filter output meta protocol ip6 counte
> >                                             ^^^
> 
> I see two problems with this patch:
> 
> - the mapping to ETH_P_* is fixed. In case of f.i. meta nfproto relational
>   expression it would have to map to NFPROTO_* values. So I think we should
>   use symbolic expressions instead of constants and leave parsing to the
>   evaluation phase-

Yes, that change needs to be done in next-3.14 to get it working with
your new inet table. I was focusing to fix this in master for the
upcoming release.

> - we're still using a mix of ip6 and ipv6. Lets also fix that, ideally
>   as a patch before this one.
> 
> I can take care of this if you like.

Please, go ahead, I'm looking at more pending stuff I want to provide
feedback on.

Re: [PATCH nft 1/3] parser: fix parsing of ethernet protocol types

[Patrick McHardy]

On Thu, Jan 16, 2014 at 05:49:37PM +0100, Pablo Neira Ayuso wrote:
> On Thu, Jan 16, 2014 at 04:28:16PM +0000, Patrick McHardy wrote:
> > On Wed, Jan 15, 2014 at 09:30:21PM +0100, Pablo Neira Ayuso wrote:
> > > This allows us to use the protocol type keyword, eg.
> > > 
> > > nft add rule ip filter output meta protocol ip6 counte
> > >                                             ^^^
> > 
> > I see two problems with this patch:
> > 
> > - the mapping to ETH_P_* is fixed. In case of f.i. meta nfproto relational
> >   expression it would have to map to NFPROTO_* values. So I think we should
> >   use symbolic expressions instead of constants and leave parsing to the
> >   evaluation phase-
> 
> Yes, that change needs to be done in next-3.14 to get it working with
> your new inet table. I was focusing to fix this in master for the
> upcoming release.
> 
> > - we're still using a mix of ip6 and ipv6. Lets also fix that, ideally
> >   as a patch before this one.
> > 
> > I can take care of this if you like.
> 
> Please, go ahead, I'm looking at more pending stuff I want to provide
> feedback on.

Already done :) I'm simply going to take your entire patchset and integrate
it with my change since they kind of depend on each other.