[PATCH 1/2] Add ipvlan support

[Thomas Haller]

On Tue, 2015-06-09 at 21:53 -0700, Cong Wang wrote:
> Signed-off-by: Cong Wang <xiyou.wangcong at gmail.com>
> ---
>  include/Makefile.am                   |   1 +
>  include/linux-private/linux/if_link.h |  15 ++
>  include/netlink/route/link/ipvlan.h   |  37 +++++
>  lib/Makefile.am                       |   2 +-
>  lib/route/link/ipvlan.c               | 279 
> ++++++++++++++++++++++++++++++++++
>  libnl-route-3.sym                     |  10 ++
>  6 files changed, 343 insertions(+), 1 deletion(-)
>  create mode 100644 include/netlink/route/link/ipvlan.h
>  create mode 100644 lib/route/link/ipvlan.c


Hi Cong,

Thanks for the patches. Both unmodified applied as
https://github.com/thom311/libnl/commit/12e87e444dc4ffd4e5d3ca2e99b08e4
e22665a86




Two follow-up questions though...


> +int rtnl_link_ipvlan_set_mode(struct rtnl_link *link, uint16_t mode)
> +{
> +	struct ipvlan_info *ipi = link->l_info;
> +
> +	IS_IPVLAN_LINK_ASSERT(link);
> +
> +	if (mode != IPVLAN_MODE_L2 && mode != IPVLAN_MODE_L3)
> +		return -NLE_INVAL;
> +
> +	ipi->ipi_mode = mode;
> +	ipi->ipi_mask |= IPVLAN_HAS_MODE;
> +
> +	return 0;
> +}

wouldn't it be better to allow any mode? Otherwise, if a new mode gets
added, libnl needs to be patched too.
Or do you think it's highly unlikely that another mode will be added in
the future?



> +/**
> + * Get IPVLAN Mode
> + * @arg link		Link object
> + *
> + * @return IPVLAN mode, 0 if not set or a negative error code.
> + */
> +uint16_t rtnl_link_ipvlan_get_mode(struct rtnl_link *link)
> +{
> +	struct ipvlan_info *ipi = link->l_info;
> +
> +	IS_IPVLAN_LINK_ASSERT(link);
> +
> +	if (ipi->ipi_mask & IPVLAN_HAS_MODE)
> +		return ipi->ipi_mode;
> +	else
> +		return 0;
> +}

0 is equal to IPVLAN_MODE_L2, so you cannot signal an error.
Also, the function cannot ever return a negative error code.

How about changing the signature to:

int rtnl_link_ipvlan_get_mode(struct rtnl_link *link, uint16_t *out_mode);




Thomas
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.infradead.org/pipermail/libnl/attachments/20150619/26e18fdb/attachment.sig>