[nftables PATCH] src: proto: fix byteorder for ip headers.

[nftables PATCH] src: proto: fix byteorder for ip headers.

[Yuxuan Shui]

Add a new datatype for ip headers which specify the byteorder, this
fixes bug 918.

Signed-off-by: Yuxuan Shui <yshuiv7@gmail.com>
---
 src/proto.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/proto.c b/src/proto.c
index 0a37a65..f1556db 100644
--- a/src/proto.c
+++ b/src/proto.c
@@ -184,13 +184,21 @@ void proto_ctx_update(struct proto_ctx *ctx, enum proto_bases base,
 	proto_ctx_debug(ctx, base);
 }
 
+const struct datatype net_integer = {
+	.type = TYPE_INTEGER,
+	.byteorder = BYTEORDER_BIG_ENDIAN,
+	.name = "net_integer",
+	.desc = "network integer",
+	.basetype = &integer_type,
+};
+
 #define HDR_TEMPLATE(__name, __dtype, __type, __member)			\
 	PROTO_HDR_TEMPLATE(__name, __dtype,				\
 			   offsetof(__type, __member) * 8,		\
 			   field_sizeof(__type, __member) * 8)
 
 #define HDR_FIELD(__name, __struct, __member)				\
-	HDR_TEMPLATE(__name, &integer_type, __struct, __member)
+	HDR_TEMPLATE(__name, &net_integer, __struct, __member)
 #define HDR_BITFIELD(__name, __dtype,  __offset, __len)			\
 	PROTO_HDR_TEMPLATE(__name, __dtype, __offset, __len)
 #define HDR_TYPE(__name, __dtype, __struct, __member)			\
-- 
2.0.0

Re: [nftables PATCH] src: proto: fix byteorder for ip headers.

[Patrick McHardy]

On Wed, Jun 04, 2014 at 08:46:29PM +0800, Yuxuan Shui wrote:
> Add a new datatype for ip headers which specify the byteorder, this
> fixes bug 918.
> 
> Signed-off-by: Yuxuan Shui <yshuiv7@gmail.com>
> ---
>  src/proto.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/src/proto.c b/src/proto.c
> index 0a37a65..f1556db 100644
> --- a/src/proto.c
> +++ b/src/proto.c
> @@ -184,13 +184,21 @@ void proto_ctx_update(struct proto_ctx *ctx, enum proto_bases base,
>  	proto_ctx_debug(ctx, base);
>  }
>  
> +const struct datatype net_integer = {
> +	.type = TYPE_INTEGER,
> +	.byteorder = BYTEORDER_BIG_ENDIAN,
> +	.name = "net_integer",
> +	.desc = "network integer",
> +	.basetype = &integer_type,
> +};
> +
>  #define HDR_TEMPLATE(__name, __dtype, __type, __member)			\
>  	PROTO_HDR_TEMPLATE(__name, __dtype,				\
>  			   offsetof(__type, __member) * 8,		\
>  			   field_sizeof(__type, __member) * 8)
>  
>  #define HDR_FIELD(__name, __struct, __member)				\
> -	HDR_TEMPLATE(__name, &integer_type, __struct, __member)
> +	HDR_TEMPLATE(__name, &net_integer, __struct, __member)
>  #define HDR_BITFIELD(__name, __dtype,  __offset, __len)			\
>  	PROTO_HDR_TEMPLATE(__name, __dtype, __offset, __len)
>  #define HDR_TYPE(__name, __dtype, __struct, __member)			\

While this does make some sense, I would rather change
byteorder_conversion_op() to return immediately for expressions of size <= 8.
There is obviously no byteorder in this case, so this seems more like
working around incorrect behaviour of that function.

Re: [nftables PATCH] src: proto: fix byteorder for ip headers.

[Pablo Neira Ayuso]

Hi Patrick,

On Wed, Jun 04, 2014 at 06:44:13PM +0200, Patrick McHardy wrote:
> On Wed, Jun 04, 2014 at 08:46:29PM +0800, Yuxuan Shui wrote:
> > Add a new datatype for ip headers which specify the byteorder, this
> > fixes bug 918.
> > 
> > Signed-off-by: Yuxuan Shui <yshuiv7@gmail.com>
> > ---
> >  src/proto.c | 10 +++++++++-
> >  1 file changed, 9 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/proto.c b/src/proto.c
> > index 0a37a65..f1556db 100644
> > --- a/src/proto.c
> > +++ b/src/proto.c
> > @@ -184,13 +184,21 @@ void proto_ctx_update(struct proto_ctx *ctx, enum proto_bases base,
> >  	proto_ctx_debug(ctx, base);
> >  }
> >  
> > +const struct datatype net_integer = {
> > +	.type = TYPE_INTEGER,
> > +	.byteorder = BYTEORDER_BIG_ENDIAN,
> > +	.name = "net_integer",
> > +	.desc = "network integer",
> > +	.basetype = &integer_type,
> > +};
> > +
> >  #define HDR_TEMPLATE(__name, __dtype, __type, __member)			\
> >  	PROTO_HDR_TEMPLATE(__name, __dtype,				\
> >  			   offsetof(__type, __member) * 8,		\
> >  			   field_sizeof(__type, __member) * 8)
> >  
> >  #define HDR_FIELD(__name, __struct, __member)				\
> > -	HDR_TEMPLATE(__name, &integer_type, __struct, __member)
> > +	HDR_TEMPLATE(__name, &net_integer, __struct, __member)
> >  #define HDR_BITFIELD(__name, __dtype,  __offset, __len)			\
> >  	PROTO_HDR_TEMPLATE(__name, __dtype, __offset, __len)
> >  #define HDR_TYPE(__name, __dtype, __struct, __member)			\
> 
> While this does make some sense, I would rather change
> byteorder_conversion_op() to return immediately for expressions of size <= 8.
> There is obviously no byteorder in this case, so this seems more like
> working around incorrect behaviour of that function.

Indeed, that should calm down that assertion.

But I think this patch also addresses another problem that we have.
The integer_type_parse function uses hostbyte order type for values in
the network header fields. I think this patch also addresses wrong
byteorder when the header field is larger than 8 bits.