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

Reply via email to