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