Re: [PATCH net-next v7] vxlan: add support for flowlabel inherit

[Jakub Kicinski <kuba@kernel.org>]

On Tue, 24 Oct 2023 11:50:28 -0500 Alce Lafranque wrote:
> +	if (data[IFLA_VXLAN_LABEL_POLICY])
> +		conf->label_policy = nla_get_u8(data[IFLA_VXLAN_LABEL_POLICY]);
>  
>  	if (data[IFLA_VXLAN_LEARNING]) {
>  		err = vxlan_nl2flag(conf, data, IFLA_VXLAN_LEARNING,
> @@ -4398,6 +4417,7 @@ static size_t vxlan_get_size(const struct net_device *dev)
>  		nla_total_size(sizeof(__u8)) +	/* IFLA_VXLAN_TOS */
>  		nla_total_size(sizeof(__u8)) +	/* IFLA_VXLAN_DF */
>  		nla_total_size(sizeof(__be32)) + /* IFLA_VXLAN_LABEL */
> +		nla_total_size(sizeof(__u8)) +  /* IFLA_VXLAN_LABEL_POLICY */
>  		nla_total_size(sizeof(__u8)) +	/* IFLA_VXLAN_LEARNING */
>  		nla_total_size(sizeof(__u8)) +	/* IFLA_VXLAN_PROXY */
>  		nla_total_size(sizeof(__u8)) +	/* IFLA_VXLAN_RSC */
> @@ -4470,6 +4490,7 @@ static int vxlan_fill_info(struct sk_buff *skb, const struct net_device *dev)
>  	    nla_put_u8(skb, IFLA_VXLAN_TOS, vxlan->cfg.tos) ||
>  	    nla_put_u8(skb, IFLA_VXLAN_DF, vxlan->cfg.df) ||
>  	    nla_put_be32(skb, IFLA_VXLAN_LABEL, vxlan->cfg.label) ||
> +	    nla_put_u8(skb, IFLA_VXLAN_LABEL_POLICY, vxlan->cfg.label_policy) ||
>  	    nla_put_u8(skb, IFLA_VXLAN_LEARNING,
>  		       !!(vxlan->cfg.flags & VXLAN_F_LEARN)) ||
>  	    nla_put_u8(skb, IFLA_VXLAN_PROXY,

nit: please don't use u8 in netlink, unless it's a fixed-size protocol
field. Attr len is rounded up to 4, see NLA_ALIGN(). You can as well
make it u32.    

> diff --git a/include/net/vxlan.h b/include/net/vxlan.h
> index 6a9f8a5f387c..33ba6fc151cf 100644
> --- a/include/net/vxlan.h
> +++ b/include/net/vxlan.h
> @@ -210,22 +210,23 @@ struct vxlan_rdst {
>  };
>  
>  struct vxlan_config {
> -	union vxlan_addr	remote_ip;
> -	union vxlan_addr	saddr;
> -	__be32			vni;
> -	int			remote_ifindex;
> -	int			mtu;
> -	__be16			dst_port;
> -	u16			port_min;
> -	u16			port_max;
> -	u8			tos;
> -	u8			ttl;
> -	__be32			label;
> -	u32			flags;
> -	unsigned long		age_interval;
> -	unsigned int		addrmax;
> -	bool			no_share;
> -	enum ifla_vxlan_df	df;
> +	union vxlan_addr		remote_ip;
> +	union vxlan_addr		saddr;
> +	__be32				vni;
> +	int				remote_ifindex;
> +	int				mtu;
> +	__be16				dst_port;
> +	u16				port_min;
> +	u16				port_max;
> +	u8				tos;
> +	u8				ttl;
> +	__be32				label;
> +	enum ifla_vxlan_label_policy	label_policy;

Here, OTOH, you could save some space, by making it a u8.

> +	u32				flags;
> +	unsigned long			age_interval;
> +	unsigned int			addrmax;
> +	bool				no_share;
> +	enum ifla_vxlan_df		df;
>  };