netdev - Re: [PATCH net-next v2 3/3] vxlan: move flag sets to use a helper func

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20181129141936.GB20478@bistromath.localdomain>
Date:   Thu, 29 Nov 2018 15:19:36 +0100
From:   Sabrina Dubroca <sd@...asysnail.net>
To:     Roopa Prabhu <roopa@...ulusnetworks.com>
Cc:     davem@...emloft.net, netdev@...r.kernel.org
Subject: Re: [PATCH net-next v2 3/3] vxlan: move flag sets to use a helper
 func

2018-11-28, 14:27:59 -0800, Roopa Prabhu wrote:
> +/* Set/clear flags based on attribute */
> +static void vxlan_nl2flag(struct vxlan_config *conf, struct nlattr *tb[],
> +			  int attrtype, unsigned long mask)
> +{
> +	unsigned long flags;
> +
> +	if (!tb[attrtype])
> +		return;
> +
> +	if (nla_get_u8(tb[attrtype]))
> +		flags = conf->flags | mask;
> +	else
> +		flags = conf->flags & ~mask;
> +
> +	conf->flags = flags;
> +}
> +
>  static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[],
>  			 struct net_device *dev, struct vxlan_config *conf,
>  			 bool changelink, struct netlink_ext_ack *extack)
> @@ -3458,45 +3475,27 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[],
>  	if (data[IFLA_VXLAN_TTL])
>  		conf->ttl = nla_get_u8(data[IFLA_VXLAN_TTL]);
>  
> -	if (data[IFLA_VXLAN_TTL_INHERIT])
> -		conf->flags |= VXLAN_F_TTL_INHERIT;
> +	vxlan_nl2flag(conf, data, IFLA_VXLAN_TTL_INHERIT,
> +		      VXLAN_F_TTL_INHERIT);

IFLA_VXLAN_TTL_INHERIT is a NLA_FLAG. It doesn't have any u8 data to
get. Same for:

	[IFLA_VXLAN_GBP]	= { .type = NLA_FLAG, },
	[IFLA_VXLAN_GPE]	= { .type = NLA_FLAG, },
	[IFLA_VXLAN_REMCSUM_NOPARTIAL]	= { .type = NLA_FLAG },
	[IFLA_VXLAN_TTL_INHERIT]	= { .type = NLA_FLAG },



nit: This patch would have been easier to review if it came first in
the series. Converting:

    if (nla_get_u8(data[IFLA_VXLAN_FOO]))
        conf->flags |= VXLAN_F_FOO;

to this new helper, which in effect does

    if (nla_get_u8(data[IFLA_VXLAN_FOO]))
        conf->flags |= VXLAN_F_FOO;
    else
        conf->flags &= ~VXLAN_F_FOO;

seems to change behavior, but since you're (unless I missed one) only
applying it to flags that couldn't be changed before patch 1, it looks
fine.

-- 
Sabrina