diff mbox

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

Message ID 1389817823-7251-1-git-send-email-pablo@netfilter.org
State Accepted
Headers show

Commit Message

Pablo Neira Ayuso Jan. 15, 2014, 8:30 p.m. UTC 
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(+)

Comments

Patrick McHardy Jan. 16, 2014, 4:28 p.m. UTC | #1 
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
--
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. 16, 2014, 4:49 p.m. UTC | #2 
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.
--
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
Patrick McHardy Jan. 16, 2014, 4:51 p.m. UTC | #3 
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.
--
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/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; }