Re: [PATCHv2 net-next iproute2] gre: add support for ERSPAN tunnel

2017-08-17 Thread William Tu 
>> + if (greinfo[IFLA_GRE_ERSPAN_INDEX])
>> + erspan_idx = 
>> rta_getattr_u32(greinfo[IFLA_GRE_ERSPAN_INDEX]);
>>   }
>
> Are you missing a ntohl() here?

>
> It is encoded as htonl() when sending to kernel.
>
> addattr32(n, 1024, IFLA_GRE_FWMARK, fwmark);
> +   addattr32(n, 1024, IFLA_GRE_ERSPAN_INDEX, htonl(erspan_idx));
Thanks for the feedbacks, and sorry for my late response.

Now I plan to not translate to network-byte order, but do it inside
the kernel. So kernel expects host order, ip route only sends
host-byte order.

>
> Is erspan_idx == 0 a reserved value?
ERSPAN spec does not say erspan_idx = 0 should be reserved or not. But
I assume at lease people will put some value, so I assume it is
reserved. And only when it is non-zero, then send to kernel.

> You should only send erspan_idx to kernel if it is given on command line.
Yes, I will resubmit next patch.

Regards,
William

Re: [PATCHv2 net-next iproute2] gre: add support for ERSPAN tunnel

2017-08-10 Thread Stephen Hemminger 
On Thu, 10 Aug 2017 13:14:27 -0700
William Tu  wrote:

> +
> + if (greinfo[IFLA_GRE_ERSPAN_INDEX])
> + erspan_idx = 
> rta_getattr_u32(greinfo[IFLA_GRE_ERSPAN_INDEX]);
>   }

Are you missing a ntohl() here?

It is encoded as htonl() when sending to kernel.

addattr32(n, 1024, IFLA_GRE_FWMARK, fwmark);
+   addattr32(n, 1024, IFLA_GRE_ERSPAN_INDEX, htonl(erspan_idx));

Is erspan_idx == 0 a reserved value?
You should only send erspan_idx to kernel if it is given on command line.