[PATCH] introduce ipip tunnel support

Thomas Haller thaller at redhat.com
Mon May 5 06:17:04 PDT 2014


Hi Susant,


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

       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.



Thanks for your effort
Thomas

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-fixup-introduce-ipip-tunnel-support.patch
Type: text/x-patch
Size: 24634 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/libnl/attachments/20140505/e1c315d3/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-fixup-introduce-ipip-tunnel-support.patch
Type: text/x-patch
Size: 3437 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/libnl/attachments/20140505/e1c315d3/attachment-0003.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/libnl/attachments/20140505/e1c315d3/attachment-0001.sig>


More information about the libnl mailing list