On Thu, Nov 29, 2018 at 5:56 AM Sabrina Dubroca <s...@queasysnail.net> wrote: > > 2018-11-28, 14:27:57 -0800, Roopa Prabhu wrote: > > From: Roopa Prabhu <ro...@cumulusnetworks.com> > > > > We started very conservative when supporting changelink > > especially because not all attribute changes could be > > tested. This patch opens up a few more attributes for > > changelink. The reason for choosing this set of attributes > > is based on code references for these attributes. I have > > tested TTL changes and did some changelink api testing > > to sanity test the others. > > > > Signed-off-by: Roopa Prabhu <ro...@cumulusnetworks.com> > > --- > > drivers/net/vxlan.c | 36 ++++-------------------------------- > > 1 file changed, 4 insertions(+), 32 deletions(-) > > > > diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c > > index 9110662..73caa65 100644 > > --- a/drivers/net/vxlan.c > > +++ b/drivers/net/vxlan.c > > @@ -3438,11 +3438,8 @@ 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]) { > > - if (changelink) > > - return -EOPNOTSUPP; > > + if (data[IFLA_VXLAN_TTL_INHERIT]) > > conf->flags |= VXLAN_F_TTL_INHERIT; > > - } > > This doesn't give us an option to disable TTL_INHERIT after it was > enabled once. Same thing with GBP, GPE, REMCSUM_NOPARTIAL.
that is provided by patch3, with the changelink patch. I can squash them, if thats easier. > > > - if (data[IFLA_VXLAN_GBP]) { > > - if (changelink) > > - return -EOPNOTSUPP; > > + if (data[IFLA_VXLAN_GBP]) > > conf->flags |= VXLAN_F_GBP; > > - } > > > > - if (data[IFLA_VXLAN_GPE]) { > > - if (changelink) > > - return -EOPNOTSUPP; > > + if (data[IFLA_VXLAN_GPE]) > > conf->flags |= VXLAN_F_GPE; > > - } > > GPE implies running a different setup function (vxlan_raw_setup() vs > vxlan_ether_setup()), that vxlan_config_apply() only calls for > !changelink. I think this is incomplete. > > I think we'd also end up with mixed tunnel types (GPE/!GPE) on the > same socket, I'm not sure how that would work. Normally, they each try > to create a separate socket, and pass the GPE flag on to the > associated vxlan_sock. I suspect that's also a problem with rx > offload. that is good to know. I will drop the change to GPE and also the rx offload flag and let somebody else using it open it up for changelink. thanks for the review > > - if (data[IFLA_VXLAN_REMCSUM_NOPARTIAL]) { > > - if (changelink) > > - return -EOPNOTSUPP; > > + if (data[IFLA_VXLAN_REMCSUM_NOPARTIAL]) > > conf->flags |= VXLAN_F_REMCSUM_NOPARTIAL; > > - } > > > > if (tb[IFLA_MTU]) { > > if (changelink) > > --