Re: [nft PATCH 2/4 v3] src: fix byteorder conversions in simple values

[Pablo Neira Ayuso <pablo@netfilter.org>]

On Thu, Jul 31, 2014 at 07:01:31PM +0200, Patrick McHardy wrote:
> On Wed, Jul 30, 2014 at 01:14:14PM +0200, Pablo Neira Ayuso wrote:
> > Some comments below.
> > 
> > On Tue, Jul 29, 2014 at 07:09:55PM +0200, Alvaro Neira Ayuso wrote:
> > > --- a/src/datatype.c
> > > +++ b/src/datatype.c
> > > @@ -255,8 +255,11 @@ static struct error_record *integer_type_parse(const struct expr *sym,
> > >  			     sym->dtype->desc);
> > >  	}
> > >  
> > > +	/* If we use integer type, we must to convert it to big endian for
> > > +	 * using it in internet format (big endian).
> > > +	 */
> > >  	*res = constant_expr_alloc(&sym->location, sym->dtype,
> > > -				   BYTEORDER_HOST_ENDIAN, 1, NULL);
> > > +				   BYTEORDER_BIG_ENDIAN, 1, NULL);
> > 
> > I guess sym->dtype->byteorder still indicates invalid byteorder when
> > you call datatype_parse, right?
> 
> This all looks quite strange to me. When parsing the integer it is
> of course always in host byteorder. We simply need to add the conversion
> in the correct spot, this should be quite minimal.

Right. IIRC he is trying to fix payload matching of rare header fields
for which the integer_type is used, which are currently sent in host
byte order to the kernel. I guess the right spot for this is the
evaluation step.

> Please do not apply any of these patches until I can get a good look at
> this. Should get a new notebook any day now.

No problem, thanks Patrick.