[PATCH] introduce ipip tunnel support

Susant Sahani susant at redhat.com
Mon May 5 09:00:26 PDT 2014


On 05/05/2014 06:47 PM, Thomas Haller wrote:
> Hi Susant,
Hi Thomas,
>
> On Thu, 2014-04-24 at 23:16 +0530, Susant Sahani wrote:
>> This patch introduces ipip tunnel support. This
>> works with kernel module ipip.
>>
>> Signed-off-by: Susant Sahani <susant at redhat.com>
>> ---
>> +
>> +static int ipip_put_attrs(struct nl_msg *msg, struct rtnl_link *link)
>> +{
>> +        struct ipip_info *ipip = link->l_info;
>> +        struct nlattr *data;
>> +
>> +        data = nla_nest_start(msg, IFLA_INFO_DATA);
>> +        if (!data)
>> +                return -NLE_MSGSIZE;
>> +
>> +        if (ipip->ipip_mask & IPIP_ATTR_LINK)
>> +                NLA_PUT_U32(msg, IFLA_IPTUN_LINK, ipip->link);
>> +
>> +        if (ipip->ipip_mask & IPIP_ATTR_LOCAL)
>> +                NLA_PUT_U32(msg, IFLA_IPTUN_LOCAL, ipip->local);
>> +
>> +        if (ipip->ipip_mask & IPIP_ATTR_REMOTE)
>> +                NLA_PUT_U32(msg, IFLA_IPTUN_REMOTE, ipip->remote);
>> +
>> +        if (ipip->ipip_mask & IPIP_ATTR_TTL)
>> +                NLA_PUT_U8(msg, IFLA_IPTUN_TTL, ipip->ttl);
>> +
>> +        if (ipip->ipip_mask & IPIP_ATTR_TOS)
>> +                NLA_PUT_U8(msg, IFLA_IPTUN_TOS, ipip->tos);
>> +
>> +        if (ipip->ipip_mask |= IPIP_ATTR_PMTUDISC)
>> +                NLA_PUT_U8(msg, IFLA_IPTUN_PMTUDISC, ipip->pmtudisc);
>
> "|=" ? Do you really want to modify the input argument @link? Seems to
> me, it should be either
   Ahh .. My bad It should

ipip->ipip_mask & IPIP_ATTR_PMTUDISC

>
>         if (ipip->ipip_mask & IPIP_ATTR_PMTUDISC)
>                  NLA_PUT_U8(msg, IFLA_IPTUN_PMTUDISC, ipip->pmtudisc);
>
> or just unconditionally:
>
>         NLA_PUT_U8(msg, IFLA_IPTUN_PMTUDISC, ipip->pmtudisc);
>
>
>
>
>
>
> Apart from that:
>    - the patch indents with whitespace. Can we change that to TAB?
>    - the test program references wrong include paths
>    - the code comment for the getters say:
>         "@return XXX value on success or a negative error code"
>      which is not correct -- especially in case of the uintX_t types.
>
> I attach a patch for these minor issues.
> If you agree, you can just squash it and resent the patch.
> Actually, if you really agree, you don't have to resent a patch for
> these trivial changes. Just tell me, I can do that myself before merging
> to master.
>
Yes pretty much agree thanks . Please make the changes .
>
> Thanks for your effort
> Thomas
>
Thanks,
Susant



More information about the libnl mailing list