* [PATCH net-next 0/2] vxlan: create and changelink extack support
@ 2019-02-26 6:03 Roopa Prabhu
2019-02-26 6:03 ` [PATCH net-next 1/2] vxlan: add extack support for create and changelink Roopa Prabhu
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Roopa Prabhu @ 2019-02-26 6:03 UTC (permalink / raw)
To: davem; +Cc: netdev, dsa, sd, johannes
From: Roopa Prabhu <roopa@cumulusnetworks.com>
This series adds extack support to changelink paths.
In the process re-factors flag sets to a separate helper.
Also adds some changelink testcases to rtnetlink.sh
(This series was initially part of another series that
tried to support changelink for more attributes.
But after some feedback from sabrina, i have dropped the
'support changelink for more attributes' part because some
of them cannot be supported today or may require additional
use-case handling code. These can be done separately
as and when we see the need for it.)
Roopa Prabhu (2):
vxlan: add extack support for create and changelink
tools: selftests: rtnetlink: add testcases for vxlan flag sets
drivers/net/vxlan.c | 208 +++++++++++++++++++++----------
include/net/vxlan.h | 31 +++++
tools/testing/selftests/net/rtnetlink.sh | 52 ++++++++
3 files changed, 224 insertions(+), 67 deletions(-)
--
2.1.4
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH net-next 1/2] vxlan: add extack support for create and changelink 2019-02-26 6:03 [PATCH net-next 0/2] vxlan: create and changelink extack support Roopa Prabhu @ 2019-02-26 6:03 ` Roopa Prabhu 2019-02-26 13:51 ` Petr Machata 2019-02-26 6:03 ` [PATCH net-next 2/2] tools: selftests: rtnetlink: add testcases for vxlan flag sets Roopa Prabhu 2019-02-26 17:01 ` [PATCH net-next 0/2] vxlan: create and changelink extack support David Miller 2 siblings, 1 reply; 8+ messages in thread From: Roopa Prabhu @ 2019-02-26 6:03 UTC (permalink / raw) To: davem; +Cc: netdev, dsa, sd, johannes From: Roopa Prabhu <roopa@cumulusnetworks.com> This patch adds extack coverage in vxlan link create and changelink paths. Introduces a new helper vxlan_nl2flags to consolidate flag attribute validation. thanks to Johannes Berg for some tips to construct the generic vxlan flag extack strings. Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com> --- drivers/net/vxlan.c | 208 +++++++++++++++++++++++++++++++++++----------------- include/net/vxlan.h | 31 ++++++++ 2 files changed, 172 insertions(+), 67 deletions(-) diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c index 577201c..a3c46d7 100644 --- a/drivers/net/vxlan.c +++ b/drivers/net/vxlan.c @@ -3583,11 +3583,40 @@ static int __vxlan_dev_create(struct net *net, struct net_device *dev, return err; } +/* Set/clear flags based on attribute */ +static int vxlan_nl2flag(struct vxlan_config *conf, struct nlattr *tb[], + int attrtype, unsigned long mask, bool changelink, + bool changelink_supported, + struct netlink_ext_ack *extack) +{ + unsigned long flags; + + if (!tb[attrtype]) + return 0; + + if (changelink && !changelink_supported) { + vxlan_flag_attr_error(attrtype, extack); + return -EOPNOTSUPP; + } + + if (vxlan_policy[attrtype].type == NLA_FLAG) + flags = conf->flags | mask; + else if (nla_get_u8(tb[attrtype])) + flags = conf->flags | mask; + else + flags = conf->flags & ~mask; + + conf->flags = flags; + + return 0; +} + static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[], struct net_device *dev, struct vxlan_config *conf, - bool changelink) + bool changelink, struct netlink_ext_ack *extack) { struct vxlan_dev *vxlan = netdev_priv(dev); + int err = 0; memset(conf, 0, sizeof(*conf)); @@ -3598,40 +3627,54 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[], if (data[IFLA_VXLAN_ID]) { __be32 vni = cpu_to_be32(nla_get_u32(data[IFLA_VXLAN_ID])); - if (changelink && (vni != conf->vni)) + if (changelink && (vni != conf->vni)) { + NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_ID], "Cannot change VNI"); return -EOPNOTSUPP; + } conf->vni = cpu_to_be32(nla_get_u32(data[IFLA_VXLAN_ID])); } if (data[IFLA_VXLAN_GROUP]) { - if (changelink && (conf->remote_ip.sa.sa_family != AF_INET)) + if (changelink && (conf->remote_ip.sa.sa_family != AF_INET)) { + NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_GROUP], "New group address family does not match old group"); return -EOPNOTSUPP; + } conf->remote_ip.sin.sin_addr.s_addr = nla_get_in_addr(data[IFLA_VXLAN_GROUP]); conf->remote_ip.sa.sa_family = AF_INET; } else if (data[IFLA_VXLAN_GROUP6]) { - if (!IS_ENABLED(CONFIG_IPV6)) + if (!IS_ENABLED(CONFIG_IPV6)) { + NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_GROUP6], "IPv6 support not enabled in the kernel"); return -EPFNOSUPPORT; + } - if (changelink && (conf->remote_ip.sa.sa_family != AF_INET6)) + if (changelink && (conf->remote_ip.sa.sa_family != AF_INET6)) { + NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_GROUP6], "New group address family does not match old group"); return -EOPNOTSUPP; + } conf->remote_ip.sin6.sin6_addr = nla_get_in6_addr(data[IFLA_VXLAN_GROUP6]); conf->remote_ip.sa.sa_family = AF_INET6; } if (data[IFLA_VXLAN_LOCAL]) { - if (changelink && (conf->saddr.sa.sa_family != AF_INET)) + if (changelink && (conf->saddr.sa.sa_family != AF_INET)) { + NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_LOCAL], "New local address family does not match old"); return -EOPNOTSUPP; + } conf->saddr.sin.sin_addr.s_addr = nla_get_in_addr(data[IFLA_VXLAN_LOCAL]); conf->saddr.sa.sa_family = AF_INET; } else if (data[IFLA_VXLAN_LOCAL6]) { - if (!IS_ENABLED(CONFIG_IPV6)) + if (!IS_ENABLED(CONFIG_IPV6)) { + NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_LOCAL6], "IPv6 support not enabled in the kernel"); return -EPFNOSUPPORT; + } - if (changelink && (conf->saddr.sa.sa_family != AF_INET6)) + if (changelink && (conf->saddr.sa.sa_family != AF_INET6)) { + NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_LOCAL6], "New local address family does not match old"); return -EOPNOTSUPP; + } /* TODO: respect scope id */ conf->saddr.sin6.sin6_addr = nla_get_in6_addr(data[IFLA_VXLAN_LOCAL6]); @@ -3648,9 +3691,12 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[], conf->ttl = nla_get_u8(data[IFLA_VXLAN_TTL]); if (data[IFLA_VXLAN_TTL_INHERIT]) { - if (changelink) - return -EOPNOTSUPP; - conf->flags |= VXLAN_F_TTL_INHERIT; + err = vxlan_nl2flag(conf, data, IFLA_VXLAN_TTL_INHERIT, + VXLAN_F_TTL_INHERIT, changelink, false, + extack); + if (err) + return err; + } if (data[IFLA_VXLAN_LABEL]) @@ -3658,10 +3704,11 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[], IPV6_FLOWLABEL_MASK; if (data[IFLA_VXLAN_LEARNING]) { - if (nla_get_u8(data[IFLA_VXLAN_LEARNING])) - conf->flags |= VXLAN_F_LEARN; - else - conf->flags &= ~VXLAN_F_LEARN; + err = vxlan_nl2flag(conf, data, IFLA_VXLAN_LEARNING, + VXLAN_F_LEARN, changelink, true, + extack); + if (err) + return err; } else if (!changelink) { /* default to learn on a new device */ conf->flags |= VXLAN_F_LEARN; @@ -3671,44 +3718,52 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[], conf->age_interval = nla_get_u32(data[IFLA_VXLAN_AGEING]); if (data[IFLA_VXLAN_PROXY]) { - if (changelink) - return -EOPNOTSUPP; - if (nla_get_u8(data[IFLA_VXLAN_PROXY])) - conf->flags |= VXLAN_F_PROXY; + err = vxlan_nl2flag(conf, data, IFLA_VXLAN_PROXY, + VXLAN_F_PROXY, changelink, false, + extack); + if (err) + return err; } if (data[IFLA_VXLAN_RSC]) { - if (changelink) - return -EOPNOTSUPP; - if (nla_get_u8(data[IFLA_VXLAN_RSC])) - conf->flags |= VXLAN_F_RSC; + err = vxlan_nl2flag(conf, data, IFLA_VXLAN_RSC, + VXLAN_F_RSC, changelink, false, + extack); + if (err) + return err; } if (data[IFLA_VXLAN_L2MISS]) { - if (changelink) - return -EOPNOTSUPP; - if (nla_get_u8(data[IFLA_VXLAN_L2MISS])) - conf->flags |= VXLAN_F_L2MISS; + err = vxlan_nl2flag(conf, data, IFLA_VXLAN_L2MISS, + VXLAN_F_L2MISS, changelink, false, + extack); + if (err) + return err; } if (data[IFLA_VXLAN_L3MISS]) { - if (changelink) - return -EOPNOTSUPP; - if (nla_get_u8(data[IFLA_VXLAN_L3MISS])) - conf->flags |= VXLAN_F_L3MISS; + err = vxlan_nl2flag(conf, data, IFLA_VXLAN_L3MISS, + VXLAN_F_L3MISS, changelink, false, + extack); + if (err) + return err; } if (data[IFLA_VXLAN_LIMIT]) { - if (changelink) + if (changelink) { + NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_LIMIT], + "Cannot change limit"); return -EOPNOTSUPP; + } conf->addrmax = nla_get_u32(data[IFLA_VXLAN_LIMIT]); } if (data[IFLA_VXLAN_COLLECT_METADATA]) { - if (changelink) - return -EOPNOTSUPP; - if (nla_get_u8(data[IFLA_VXLAN_COLLECT_METADATA])) - conf->flags |= VXLAN_F_COLLECT_METADATA; + err = vxlan_nl2flag(conf, data, IFLA_VXLAN_COLLECT_METADATA, + VXLAN_F_COLLECT_METADATA, changelink, false, + extack); + if (err) + return err; } if (data[IFLA_VXLAN_PORT_RANGE]) { @@ -3718,72 +3773,92 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[], conf->port_min = ntohs(p->low); conf->port_max = ntohs(p->high); } else { + NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_PORT_RANGE], + "Cannot change port range"); return -EOPNOTSUPP; } } if (data[IFLA_VXLAN_PORT]) { - if (changelink) + if (changelink) { + NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_PORT], + "Cannot change port"); return -EOPNOTSUPP; + } conf->dst_port = nla_get_be16(data[IFLA_VXLAN_PORT]); } if (data[IFLA_VXLAN_UDP_CSUM]) { - if (changelink) + if (changelink) { + NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_UDP_CSUM], + "Cannot change UDP_CSUM flag"); return -EOPNOTSUPP; + } if (!nla_get_u8(data[IFLA_VXLAN_UDP_CSUM])) conf->flags |= VXLAN_F_UDP_ZERO_CSUM_TX; } if (data[IFLA_VXLAN_UDP_ZERO_CSUM6_TX]) { - if (changelink) - return -EOPNOTSUPP; - if (nla_get_u8(data[IFLA_VXLAN_UDP_ZERO_CSUM6_TX])) - conf->flags |= VXLAN_F_UDP_ZERO_CSUM6_TX; + err = vxlan_nl2flag(conf, data, IFLA_VXLAN_UDP_ZERO_CSUM6_TX, + VXLAN_F_UDP_ZERO_CSUM6_TX, changelink, + false, extack); + if (err) + return err; } if (data[IFLA_VXLAN_UDP_ZERO_CSUM6_RX]) { - if (changelink) - return -EOPNOTSUPP; - if (nla_get_u8(data[IFLA_VXLAN_UDP_ZERO_CSUM6_RX])) - conf->flags |= VXLAN_F_UDP_ZERO_CSUM6_RX; + err = vxlan_nl2flag(conf, data, IFLA_VXLAN_UDP_ZERO_CSUM6_RX, + VXLAN_F_UDP_ZERO_CSUM6_RX, changelink, + false, extack); + if (err) + return err; } if (data[IFLA_VXLAN_REMCSUM_TX]) { - if (changelink) - return -EOPNOTSUPP; - if (nla_get_u8(data[IFLA_VXLAN_REMCSUM_TX])) - conf->flags |= VXLAN_F_REMCSUM_TX; + err = vxlan_nl2flag(conf, data, IFLA_VXLAN_REMCSUM_TX, + VXLAN_F_REMCSUM_TX, changelink, false, + extack); + if (err) + return err; } if (data[IFLA_VXLAN_REMCSUM_RX]) { - if (changelink) - return -EOPNOTSUPP; - if (nla_get_u8(data[IFLA_VXLAN_REMCSUM_RX])) - conf->flags |= VXLAN_F_REMCSUM_RX; + err = vxlan_nl2flag(conf, data, IFLA_VXLAN_REMCSUM_RX, + VXLAN_F_REMCSUM_RX, changelink, false, + extack); + if (err) + return err; } if (data[IFLA_VXLAN_GBP]) { - if (changelink) - return -EOPNOTSUPP; - conf->flags |= VXLAN_F_GBP; + err = vxlan_nl2flag(conf, data, IFLA_VXLAN_GBP, + VXLAN_F_GBP, changelink, false, extack); + if (err) + return err; } if (data[IFLA_VXLAN_GPE]) { - if (changelink) - return -EOPNOTSUPP; - conf->flags |= VXLAN_F_GPE; + err = vxlan_nl2flag(conf, data, IFLA_VXLAN_GPE, + VXLAN_F_GPE, changelink, false, + extack); + if (err) + return err; } if (data[IFLA_VXLAN_REMCSUM_NOPARTIAL]) { - if (changelink) - return -EOPNOTSUPP; - conf->flags |= VXLAN_F_REMCSUM_NOPARTIAL; + err = vxlan_nl2flag(conf, data, IFLA_VXLAN_REMCSUM_NOPARTIAL, + VXLAN_F_REMCSUM_NOPARTIAL, changelink, + false, extack); + if (err) + return err; } if (tb[IFLA_MTU]) { - if (changelink) + if (changelink) { + NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_MTU], + "Cannot change mtu"); return -EOPNOTSUPP; + } conf->mtu = nla_get_u32(tb[IFLA_MTU]); } @@ -3800,7 +3875,7 @@ static int vxlan_newlink(struct net *src_net, struct net_device *dev, struct vxlan_config conf; int err; - err = vxlan_nl2conf(tb, data, dev, &conf, false); + err = vxlan_nl2conf(tb, data, dev, &conf, false, extack); if (err) return err; @@ -3817,8 +3892,7 @@ static int vxlan_changelink(struct net_device *dev, struct nlattr *tb[], struct vxlan_config conf; int err; - err = vxlan_nl2conf(tb, data, - dev, &conf, true); + err = vxlan_nl2conf(tb, data, dev, &conf, true, extack); if (err) return err; diff --git a/include/net/vxlan.h b/include/net/vxlan.h index 0976781..00254a5 100644 --- a/include/net/vxlan.h +++ b/include/net/vxlan.h @@ -453,4 +453,35 @@ vxlan_fdb_clear_offload(const struct net_device *dev, __be32 vni) } #endif +static inline void vxlan_flag_attr_error(int attrtype, + struct netlink_ext_ack *extack) +{ +#define VXLAN_FLAG(flg) \ + case IFLA_VXLAN_##flg: \ + NL_SET_ERR_MSG_MOD(extack, \ + "cannot change " #flg " flag"); \ + break + switch (attrtype) { + VXLAN_FLAG(TTL_INHERIT); + VXLAN_FLAG(LEARNING); + VXLAN_FLAG(PROXY); + VXLAN_FLAG(RSC); + VXLAN_FLAG(L2MISS); + VXLAN_FLAG(L3MISS); + VXLAN_FLAG(COLLECT_METADATA); + VXLAN_FLAG(UDP_ZERO_CSUM6_TX); + VXLAN_FLAG(UDP_ZERO_CSUM6_RX); + VXLAN_FLAG(REMCSUM_TX); + VXLAN_FLAG(REMCSUM_RX); + VXLAN_FLAG(GBP); + VXLAN_FLAG(GPE); + VXLAN_FLAG(REMCSUM_NOPARTIAL); + default: + NL_SET_ERR_MSG_MOD(extack, \ + "cannot change flag"); + break; + } +#undef VXLAN_FLAG +} + #endif -- 2.1.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH net-next 1/2] vxlan: add extack support for create and changelink 2019-02-26 6:03 ` [PATCH net-next 1/2] vxlan: add extack support for create and changelink Roopa Prabhu @ 2019-02-26 13:51 ` Petr Machata 2019-02-26 14:06 ` Roopa Prabhu 0 siblings, 1 reply; 8+ messages in thread From: Petr Machata @ 2019-02-26 13:51 UTC (permalink / raw) To: Roopa Prabhu Cc: davem@davemloft.net, netdev@vger.kernel.org, dsa@cumulusnetworks.com, sd@queasysnail.net, johannes@sipsolutions.net Roopa Prabhu <roopa@cumulusnetworks.com> writes: > From: Roopa Prabhu <roopa@cumulusnetworks.com> > > This patch adds extack coverage in vxlan link > create and changelink paths. Introduces a new helper > vxlan_nl2flags to consolidate flag attribute validation. > > thanks to Johannes Berg for some tips to construct the > generic vxlan flag extack strings. > > Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com> > --- > drivers/net/vxlan.c | 208 +++++++++++++++++++++++++++++++++++----------------- > include/net/vxlan.h | 31 ++++++++ > 2 files changed, 172 insertions(+), 67 deletions(-) > > diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c > index 577201c..a3c46d7 100644 > --- a/drivers/net/vxlan.c > +++ b/drivers/net/vxlan.c > @@ -3583,11 +3583,40 @@ static int __vxlan_dev_create(struct net *net, struct net_device *dev, > return err; > } > > +/* Set/clear flags based on attribute */ > +static int vxlan_nl2flag(struct vxlan_config *conf, struct nlattr *tb[], > + int attrtype, unsigned long mask, bool changelink, > + bool changelink_supported, > + struct netlink_ext_ack *extack) > +{ > + unsigned long flags; > + > + if (!tb[attrtype]) > + return 0; > + > + if (changelink && !changelink_supported) { > + vxlan_flag_attr_error(attrtype, extack); > + return -EOPNOTSUPP; > + } > + > + if (vxlan_policy[attrtype].type == NLA_FLAG) > + flags = conf->flags | mask; > + else if (nla_get_u8(tb[attrtype])) > + flags = conf->flags | mask; > + else > + flags = conf->flags & ~mask; Many of the flags for which you call this don't actually have the else branch. However I suspect there are no good reasons not to allow resetting a flag. Reviewed-by: Petr Machata <petrm@mellanox.com> > + > + conf->flags = flags; > + > + return 0; > +} > + > static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[], > struct net_device *dev, struct vxlan_config *conf, > - bool changelink) > + bool changelink, struct netlink_ext_ack *extack) > { > struct vxlan_dev *vxlan = netdev_priv(dev); > + int err = 0; > > memset(conf, 0, sizeof(*conf)); > > @@ -3598,40 +3627,54 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[], > if (data[IFLA_VXLAN_ID]) { > __be32 vni = cpu_to_be32(nla_get_u32(data[IFLA_VXLAN_ID])); > > - if (changelink && (vni != conf->vni)) > + if (changelink && (vni != conf->vni)) { > + NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_ID], "Cannot change VNI"); > return -EOPNOTSUPP; > + } > conf->vni = cpu_to_be32(nla_get_u32(data[IFLA_VXLAN_ID])); > } > > if (data[IFLA_VXLAN_GROUP]) { > - if (changelink && (conf->remote_ip.sa.sa_family != AF_INET)) > + if (changelink && (conf->remote_ip.sa.sa_family != AF_INET)) { > + NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_GROUP], "New group address family does not match old group"); > return -EOPNOTSUPP; > + } > > conf->remote_ip.sin.sin_addr.s_addr = nla_get_in_addr(data[IFLA_VXLAN_GROUP]); > conf->remote_ip.sa.sa_family = AF_INET; > } else if (data[IFLA_VXLAN_GROUP6]) { > - if (!IS_ENABLED(CONFIG_IPV6)) > + if (!IS_ENABLED(CONFIG_IPV6)) { > + NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_GROUP6], "IPv6 support not enabled in the kernel"); > return -EPFNOSUPPORT; > + } > > - if (changelink && (conf->remote_ip.sa.sa_family != AF_INET6)) > + if (changelink && (conf->remote_ip.sa.sa_family != AF_INET6)) { > + NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_GROUP6], "New group address family does not match old group"); > return -EOPNOTSUPP; > + } > > conf->remote_ip.sin6.sin6_addr = nla_get_in6_addr(data[IFLA_VXLAN_GROUP6]); > conf->remote_ip.sa.sa_family = AF_INET6; > } > > if (data[IFLA_VXLAN_LOCAL]) { > - if (changelink && (conf->saddr.sa.sa_family != AF_INET)) > + if (changelink && (conf->saddr.sa.sa_family != AF_INET)) { > + NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_LOCAL], "New local address family does not match old"); > return -EOPNOTSUPP; > + } > > conf->saddr.sin.sin_addr.s_addr = nla_get_in_addr(data[IFLA_VXLAN_LOCAL]); > conf->saddr.sa.sa_family = AF_INET; > } else if (data[IFLA_VXLAN_LOCAL6]) { > - if (!IS_ENABLED(CONFIG_IPV6)) > + if (!IS_ENABLED(CONFIG_IPV6)) { > + NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_LOCAL6], "IPv6 support not enabled in the kernel"); > return -EPFNOSUPPORT; > + } > > - if (changelink && (conf->saddr.sa.sa_family != AF_INET6)) > + if (changelink && (conf->saddr.sa.sa_family != AF_INET6)) { > + NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_LOCAL6], "New local address family does not match old"); > return -EOPNOTSUPP; > + } > > /* TODO: respect scope id */ > conf->saddr.sin6.sin6_addr = nla_get_in6_addr(data[IFLA_VXLAN_LOCAL6]); > @@ -3648,9 +3691,12 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[], > conf->ttl = nla_get_u8(data[IFLA_VXLAN_TTL]); > > if (data[IFLA_VXLAN_TTL_INHERIT]) { > - if (changelink) > - return -EOPNOTSUPP; > - conf->flags |= VXLAN_F_TTL_INHERIT; > + err = vxlan_nl2flag(conf, data, IFLA_VXLAN_TTL_INHERIT, > + VXLAN_F_TTL_INHERIT, changelink, false, > + extack); > + if (err) > + return err; > + > } > > if (data[IFLA_VXLAN_LABEL]) > @@ -3658,10 +3704,11 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[], > IPV6_FLOWLABEL_MASK; > > if (data[IFLA_VXLAN_LEARNING]) { > - if (nla_get_u8(data[IFLA_VXLAN_LEARNING])) > - conf->flags |= VXLAN_F_LEARN; > - else > - conf->flags &= ~VXLAN_F_LEARN; > + err = vxlan_nl2flag(conf, data, IFLA_VXLAN_LEARNING, > + VXLAN_F_LEARN, changelink, true, > + extack); > + if (err) > + return err; > } else if (!changelink) { > /* default to learn on a new device */ > conf->flags |= VXLAN_F_LEARN; > @@ -3671,44 +3718,52 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[], > conf->age_interval = nla_get_u32(data[IFLA_VXLAN_AGEING]); > > if (data[IFLA_VXLAN_PROXY]) { > - if (changelink) > - return -EOPNOTSUPP; > - if (nla_get_u8(data[IFLA_VXLAN_PROXY])) > - conf->flags |= VXLAN_F_PROXY; > + err = vxlan_nl2flag(conf, data, IFLA_VXLAN_PROXY, > + VXLAN_F_PROXY, changelink, false, > + extack); > + if (err) > + return err; > } > > if (data[IFLA_VXLAN_RSC]) { > - if (changelink) > - return -EOPNOTSUPP; > - if (nla_get_u8(data[IFLA_VXLAN_RSC])) > - conf->flags |= VXLAN_F_RSC; > + err = vxlan_nl2flag(conf, data, IFLA_VXLAN_RSC, > + VXLAN_F_RSC, changelink, false, > + extack); > + if (err) > + return err; > } > > if (data[IFLA_VXLAN_L2MISS]) { > - if (changelink) > - return -EOPNOTSUPP; > - if (nla_get_u8(data[IFLA_VXLAN_L2MISS])) > - conf->flags |= VXLAN_F_L2MISS; > + err = vxlan_nl2flag(conf, data, IFLA_VXLAN_L2MISS, > + VXLAN_F_L2MISS, changelink, false, > + extack); > + if (err) > + return err; > } > > if (data[IFLA_VXLAN_L3MISS]) { > - if (changelink) > - return -EOPNOTSUPP; > - if (nla_get_u8(data[IFLA_VXLAN_L3MISS])) > - conf->flags |= VXLAN_F_L3MISS; > + err = vxlan_nl2flag(conf, data, IFLA_VXLAN_L3MISS, > + VXLAN_F_L3MISS, changelink, false, > + extack); > + if (err) > + return err; > } > > if (data[IFLA_VXLAN_LIMIT]) { > - if (changelink) > + if (changelink) { > + NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_LIMIT], > + "Cannot change limit"); > return -EOPNOTSUPP; > + } > conf->addrmax = nla_get_u32(data[IFLA_VXLAN_LIMIT]); > } > > if (data[IFLA_VXLAN_COLLECT_METADATA]) { > - if (changelink) > - return -EOPNOTSUPP; > - if (nla_get_u8(data[IFLA_VXLAN_COLLECT_METADATA])) > - conf->flags |= VXLAN_F_COLLECT_METADATA; > + err = vxlan_nl2flag(conf, data, IFLA_VXLAN_COLLECT_METADATA, > + VXLAN_F_COLLECT_METADATA, changelink, false, > + extack); > + if (err) > + return err; > } > > if (data[IFLA_VXLAN_PORT_RANGE]) { > @@ -3718,72 +3773,92 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[], > conf->port_min = ntohs(p->low); > conf->port_max = ntohs(p->high); > } else { > + NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_PORT_RANGE], > + "Cannot change port range"); > return -EOPNOTSUPP; > } > } > > if (data[IFLA_VXLAN_PORT]) { > - if (changelink) > + if (changelink) { > + NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_PORT], > + "Cannot change port"); > return -EOPNOTSUPP; > + } > conf->dst_port = nla_get_be16(data[IFLA_VXLAN_PORT]); > } > > if (data[IFLA_VXLAN_UDP_CSUM]) { > - if (changelink) > + if (changelink) { > + NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_UDP_CSUM], > + "Cannot change UDP_CSUM flag"); > return -EOPNOTSUPP; > + } > if (!nla_get_u8(data[IFLA_VXLAN_UDP_CSUM])) > conf->flags |= VXLAN_F_UDP_ZERO_CSUM_TX; > } > > if (data[IFLA_VXLAN_UDP_ZERO_CSUM6_TX]) { > - if (changelink) > - return -EOPNOTSUPP; > - if (nla_get_u8(data[IFLA_VXLAN_UDP_ZERO_CSUM6_TX])) > - conf->flags |= VXLAN_F_UDP_ZERO_CSUM6_TX; > + err = vxlan_nl2flag(conf, data, IFLA_VXLAN_UDP_ZERO_CSUM6_TX, > + VXLAN_F_UDP_ZERO_CSUM6_TX, changelink, > + false, extack); > + if (err) > + return err; > } > > if (data[IFLA_VXLAN_UDP_ZERO_CSUM6_RX]) { > - if (changelink) > - return -EOPNOTSUPP; > - if (nla_get_u8(data[IFLA_VXLAN_UDP_ZERO_CSUM6_RX])) > - conf->flags |= VXLAN_F_UDP_ZERO_CSUM6_RX; > + err = vxlan_nl2flag(conf, data, IFLA_VXLAN_UDP_ZERO_CSUM6_RX, > + VXLAN_F_UDP_ZERO_CSUM6_RX, changelink, > + false, extack); > + if (err) > + return err; > } > > if (data[IFLA_VXLAN_REMCSUM_TX]) { > - if (changelink) > - return -EOPNOTSUPP; > - if (nla_get_u8(data[IFLA_VXLAN_REMCSUM_TX])) > - conf->flags |= VXLAN_F_REMCSUM_TX; > + err = vxlan_nl2flag(conf, data, IFLA_VXLAN_REMCSUM_TX, > + VXLAN_F_REMCSUM_TX, changelink, false, > + extack); > + if (err) > + return err; > } > > if (data[IFLA_VXLAN_REMCSUM_RX]) { > - if (changelink) > - return -EOPNOTSUPP; > - if (nla_get_u8(data[IFLA_VXLAN_REMCSUM_RX])) > - conf->flags |= VXLAN_F_REMCSUM_RX; > + err = vxlan_nl2flag(conf, data, IFLA_VXLAN_REMCSUM_RX, > + VXLAN_F_REMCSUM_RX, changelink, false, > + extack); > + if (err) > + return err; > } > > if (data[IFLA_VXLAN_GBP]) { > - if (changelink) > - return -EOPNOTSUPP; > - conf->flags |= VXLAN_F_GBP; > + err = vxlan_nl2flag(conf, data, IFLA_VXLAN_GBP, > + VXLAN_F_GBP, changelink, false, extack); > + if (err) > + return err; > } > > if (data[IFLA_VXLAN_GPE]) { > - if (changelink) > - return -EOPNOTSUPP; > - conf->flags |= VXLAN_F_GPE; > + err = vxlan_nl2flag(conf, data, IFLA_VXLAN_GPE, > + VXLAN_F_GPE, changelink, false, > + extack); > + if (err) > + return err; > } > > if (data[IFLA_VXLAN_REMCSUM_NOPARTIAL]) { > - if (changelink) > - return -EOPNOTSUPP; > - conf->flags |= VXLAN_F_REMCSUM_NOPARTIAL; > + err = vxlan_nl2flag(conf, data, IFLA_VXLAN_REMCSUM_NOPARTIAL, > + VXLAN_F_REMCSUM_NOPARTIAL, changelink, > + false, extack); > + if (err) > + return err; > } > > if (tb[IFLA_MTU]) { > - if (changelink) > + if (changelink) { > + NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_MTU], > + "Cannot change mtu"); > return -EOPNOTSUPP; > + } > conf->mtu = nla_get_u32(tb[IFLA_MTU]); > } > > @@ -3800,7 +3875,7 @@ static int vxlan_newlink(struct net *src_net, struct net_device *dev, > struct vxlan_config conf; > int err; > > - err = vxlan_nl2conf(tb, data, dev, &conf, false); > + err = vxlan_nl2conf(tb, data, dev, &conf, false, extack); > if (err) > return err; > > @@ -3817,8 +3892,7 @@ static int vxlan_changelink(struct net_device *dev, struct nlattr *tb[], > struct vxlan_config conf; > int err; > > - err = vxlan_nl2conf(tb, data, > - dev, &conf, true); > + err = vxlan_nl2conf(tb, data, dev, &conf, true, extack); > if (err) > return err; > > diff --git a/include/net/vxlan.h b/include/net/vxlan.h > index 0976781..00254a5 100644 > --- a/include/net/vxlan.h > +++ b/include/net/vxlan.h > @@ -453,4 +453,35 @@ vxlan_fdb_clear_offload(const struct net_device *dev, __be32 vni) > } > #endif > > +static inline void vxlan_flag_attr_error(int attrtype, > + struct netlink_ext_ack *extack) > +{ > +#define VXLAN_FLAG(flg) \ > + case IFLA_VXLAN_##flg: \ > + NL_SET_ERR_MSG_MOD(extack, \ > + "cannot change " #flg " flag"); \ > + break > + switch (attrtype) { > + VXLAN_FLAG(TTL_INHERIT); > + VXLAN_FLAG(LEARNING); > + VXLAN_FLAG(PROXY); > + VXLAN_FLAG(RSC); > + VXLAN_FLAG(L2MISS); > + VXLAN_FLAG(L3MISS); > + VXLAN_FLAG(COLLECT_METADATA); > + VXLAN_FLAG(UDP_ZERO_CSUM6_TX); > + VXLAN_FLAG(UDP_ZERO_CSUM6_RX); > + VXLAN_FLAG(REMCSUM_TX); > + VXLAN_FLAG(REMCSUM_RX); > + VXLAN_FLAG(GBP); > + VXLAN_FLAG(GPE); > + VXLAN_FLAG(REMCSUM_NOPARTIAL); > + default: > + NL_SET_ERR_MSG_MOD(extack, \ > + "cannot change flag"); > + break; > + } > +#undef VXLAN_FLAG > +} > + > #endif ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next 1/2] vxlan: add extack support for create and changelink 2019-02-26 13:51 ` Petr Machata @ 2019-02-26 14:06 ` Roopa Prabhu 2019-02-26 17:45 ` Petr Machata 0 siblings, 1 reply; 8+ messages in thread From: Roopa Prabhu @ 2019-02-26 14:06 UTC (permalink / raw) To: Petr Machata Cc: davem@davemloft.net, netdev@vger.kernel.org, dsa@cumulusnetworks.com, sd@queasysnail.net, johannes@sipsolutions.net On Tue, Feb 26, 2019 at 5:51 AM Petr Machata <petrm@mellanox.com> wrote: > > > Roopa Prabhu <roopa@cumulusnetworks.com> writes: > > > From: Roopa Prabhu <roopa@cumulusnetworks.com> > > > > This patch adds extack coverage in vxlan link > > create and changelink paths. Introduces a new helper > > vxlan_nl2flags to consolidate flag attribute validation. > > > > thanks to Johannes Berg for some tips to construct the > > generic vxlan flag extack strings. > > > > Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com> > > --- > > drivers/net/vxlan.c | 208 +++++++++++++++++++++++++++++++++++----------------- > > include/net/vxlan.h | 31 ++++++++ > > 2 files changed, 172 insertions(+), 67 deletions(-) > > > > diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c > > index 577201c..a3c46d7 100644 > > --- a/drivers/net/vxlan.c > > +++ b/drivers/net/vxlan.c > > @@ -3583,11 +3583,40 @@ static int __vxlan_dev_create(struct net *net, struct net_device *dev, > > return err; > > } > > > > +/* Set/clear flags based on attribute */ > > +static int vxlan_nl2flag(struct vxlan_config *conf, struct nlattr *tb[], > > + int attrtype, unsigned long mask, bool changelink, > > + bool changelink_supported, > > + struct netlink_ext_ack *extack) > > +{ > > + unsigned long flags; > > + > > + if (!tb[attrtype]) > > + return 0; > > + > > + if (changelink && !changelink_supported) { > > + vxlan_flag_attr_error(attrtype, extack); > > + return -EOPNOTSUPP; > > + } > > + > > + if (vxlan_policy[attrtype].type == NLA_FLAG) > > + flags = conf->flags | mask; > > + else if (nla_get_u8(tb[attrtype])) > > + flags = conf->flags | mask; > > + else > > + flags = conf->flags & ~mask; > > Many of the flags for which you call this don't actually have the else > branch. However I suspect there are no good reasons not to allow > resetting a flag. > > Reviewed-by: Petr Machata <petrm@mellanox.com> yes, correct, that was intentional. also, I am not sure what is the best way to support reseting of a NLA_FLAG. Absence of the flag attribute in the request cannot be the reason for resetting or clearing the flag. None of the NLA_FLAG attributes support changing the flag today. This patch does not change that. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next 1/2] vxlan: add extack support for create and changelink 2019-02-26 14:06 ` Roopa Prabhu @ 2019-02-26 17:45 ` Petr Machata 2019-02-27 1:05 ` Roopa Prabhu 0 siblings, 1 reply; 8+ messages in thread From: Petr Machata @ 2019-02-26 17:45 UTC (permalink / raw) To: Roopa Prabhu Cc: davem@davemloft.net, netdev@vger.kernel.org, dsa@cumulusnetworks.com, sd@queasysnail.net, johannes@sipsolutions.net Roopa Prabhu <roopa@cumulusnetworks.com> writes: > On Tue, Feb 26, 2019 at 5:51 AM Petr Machata <petrm@mellanox.com> wrote: >> >> Many of the flags for which you call this don't actually have the else >> branch. However I suspect there are no good reasons not to allow >> resetting a flag. >> >> Reviewed-by: Petr Machata <petrm@mellanox.com> > > yes, correct, that was intentional. > also, I am not sure what is the best way to support reseting of a NLA_FLAG. > Absence of the flag attribute in the request cannot be the reason for > resetting or clearing the flag. Yeah, I was thinking about that. It looks like this would need a new set of u8 attributes :-( > None of the NLA_FLAG attributes support changing the flag today. This > patch does not change that. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net-next 1/2] vxlan: add extack support for create and changelink 2019-02-26 17:45 ` Petr Machata @ 2019-02-27 1:05 ` Roopa Prabhu 0 siblings, 0 replies; 8+ messages in thread From: Roopa Prabhu @ 2019-02-27 1:05 UTC (permalink / raw) To: Petr Machata Cc: davem@davemloft.net, netdev@vger.kernel.org, dsa@cumulusnetworks.com, sd@queasysnail.net, johannes@sipsolutions.net On Tue, Feb 26, 2019 at 9:45 AM Petr Machata <petrm@mellanox.com> wrote: > > > Roopa Prabhu <roopa@cumulusnetworks.com> writes: > > > On Tue, Feb 26, 2019 at 5:51 AM Petr Machata <petrm@mellanox.com> wrote: > >> > >> Many of the flags for which you call this don't actually have the else > >> branch. However I suspect there are no good reasons not to allow > >> resetting a flag. > >> > >> Reviewed-by: Petr Machata <petrm@mellanox.com> > > > > yes, correct, that was intentional. > > also, I am not sure what is the best way to support reseting of a NLA_FLAG. > > Absence of the flag attribute in the request cannot be the reason for > > resetting or clearing the flag. > > Yeah, I was thinking about that. It looks like this would need a new set > of u8 attributes :-( yes, and thats unfortunate. We should keep an eye on future NLA_FLAG attributes during review . ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH net-next 2/2] tools: selftests: rtnetlink: add testcases for vxlan flag sets 2019-02-26 6:03 [PATCH net-next 0/2] vxlan: create and changelink extack support Roopa Prabhu 2019-02-26 6:03 ` [PATCH net-next 1/2] vxlan: add extack support for create and changelink Roopa Prabhu @ 2019-02-26 6:03 ` Roopa Prabhu 2019-02-26 17:01 ` [PATCH net-next 0/2] vxlan: create and changelink extack support David Miller 2 siblings, 0 replies; 8+ messages in thread From: Roopa Prabhu @ 2019-02-26 6:03 UTC (permalink / raw) To: davem; +Cc: netdev, dsa, sd, johannes From: Roopa Prabhu <roopa@cumulusnetworks.com> This patch extends rtnetlink.sh to cover some vxlan flag netlink attribute sets. Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com> --- tools/testing/selftests/net/rtnetlink.sh | 52 ++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/tools/testing/selftests/net/rtnetlink.sh b/tools/testing/selftests/net/rtnetlink.sh index 78fc593..5baf35a 100755 --- a/tools/testing/selftests/net/rtnetlink.sh +++ b/tools/testing/selftests/net/rtnetlink.sh @@ -408,6 +408,58 @@ kci_test_encap_vxlan() ip netns exec "$testns" ip link add link "$vxlan" name "$vlan" type vlan id 1 check_err $? + # changelink testcases + ip netns exec "$testns" ip link set dev "$vxlan" type vxlan vni 43 2>/dev/null + check_fail $? + + ip netns exec "$testns" ip link set dev "$vxlan" type vxlan group ffe5::5 dev "$devdummy" 2>/dev/null + check_fail $? + + ip netns exec "$testns" ip link set dev "$vxlan" type vxlan ttl inherit 2>/dev/null + check_fail $? + + ip netns exec "$testns" ip link set dev "$vxlan" type vxlan ttl 64 + check_err $? + + ip netns exec "$testns" ip link set dev "$vxlan" type vxlan nolearning + check_err $? + + ip netns exec "$testns" ip link set dev "$vxlan" type vxlan proxy 2>/dev/null + check_fail $? + + ip netns exec "$testns" ip link set dev "$vxlan" type vxlan norsc 2>/dev/null + check_fail $? + + ip netns exec "$testns" ip link set dev "$vxlan" type vxlan l2miss 2>/dev/null + check_fail $? + + ip netns exec "$testns" ip link set dev "$vxlan" type vxlan l3miss 2>/dev/null + check_fail $? + + ip netns exec "$testns" ip link set dev "$vxlan" type vxlan external 2>/dev/null + check_fail $? + + ip netns exec "$testns" ip link set dev "$vxlan" type vxlan udpcsum 2>/dev/null + check_fail $? + + ip netns exec "$testns" ip link set dev "$vxlan" type vxlan udp6zerocsumtx 2>/dev/null + check_fail $? + + ip netns exec "$testns" ip link set dev "$vxlan" type vxlan udp6zerocsumrx 2>/dev/null + check_fail $? + + ip netns exec "$testns" ip link set dev "$vxlan" type vxlan remcsumtx 2>/dev/null + check_fail $? + + ip netns exec "$testns" ip link set dev "$vxlan" type vxlan remcsumrx 2>/dev/null + check_fail $? + + ip netns exec "$testns" ip link set dev "$vxlan" type vxlan gbp 2>/dev/null + check_fail $? + + ip netns exec "$testns" ip link set dev "$vxlan" type vxlan gpe 2>/dev/null + check_fail $? + ip netns exec "$testns" ip link del "$vxlan" check_err $? -- 2.1.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH net-next 0/2] vxlan: create and changelink extack support 2019-02-26 6:03 [PATCH net-next 0/2] vxlan: create and changelink extack support Roopa Prabhu 2019-02-26 6:03 ` [PATCH net-next 1/2] vxlan: add extack support for create and changelink Roopa Prabhu 2019-02-26 6:03 ` [PATCH net-next 2/2] tools: selftests: rtnetlink: add testcases for vxlan flag sets Roopa Prabhu @ 2019-02-26 17:01 ` David Miller 2 siblings, 0 replies; 8+ messages in thread From: David Miller @ 2019-02-26 17:01 UTC (permalink / raw) To: roopa; +Cc: netdev, dsa, sd, johannes From: Roopa Prabhu <roopa@cumulusnetworks.com> Date: Mon, 25 Feb 2019 22:03:00 -0800 > From: Roopa Prabhu <roopa@cumulusnetworks.com> > > This series adds extack support to changelink paths. > In the process re-factors flag sets to a separate helper. > Also adds some changelink testcases to rtnetlink.sh > > (This series was initially part of another series that > tried to support changelink for more attributes. > But after some feedback from sabrina, i have dropped the > 'support changelink for more attributes' part because some > of them cannot be supported today or may require additional > use-case handling code. These can be done separately > as and when we see the need for it.) Series applied, thanks Roopa. ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-02-27 1:05 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-02-26 6:03 [PATCH net-next 0/2] vxlan: create and changelink extack support Roopa Prabhu 2019-02-26 6:03 ` [PATCH net-next 1/2] vxlan: add extack support for create and changelink Roopa Prabhu 2019-02-26 13:51 ` Petr Machata 2019-02-26 14:06 ` Roopa Prabhu 2019-02-26 17:45 ` Petr Machata 2019-02-27 1:05 ` Roopa Prabhu 2019-02-26 6:03 ` [PATCH net-next 2/2] tools: selftests: rtnetlink: add testcases for vxlan flag sets Roopa Prabhu 2019-02-26 17:01 ` [PATCH net-next 0/2] vxlan: create and changelink extack support David Miller
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).