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
