On Thu, Nov 29, 2018 at 7:55 AM Sabrina Dubroca <[email protected]> wrote: > > 2018-11-29, 07:27:17 -0800, Roopa Prabhu wrote: > > On Thu, Nov 29, 2018 at 6:19 AM Sabrina Dubroca <[email protected]> > > wrote: > > > 2018-11-28, 14:27:59 -0800, Roopa Prabhu wrote: > > > nit: This patch would have been easier to review if it came first in > > > the series. Converting: > > > > I considered that. But the first patch really removes some checks > > making it easier for the follow-on patches...and that mostly dictated > > the order. > > ok. > > > > 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. > > > > It does not change behavior...the earlier code only supported the > > flags during create time and did not support changing of the flag with > > changelink. > > this patch adds changelink support. But if you do see any change in > > behavior, pls report. thanks. > > Yeah, looks correct. I just had to go back to patch 1 and check. > A note "only flags [list] are changed, and they could not be modified > after creation of the device prior to patch 1" in the cover and/or in > this patch's commit message would have spared reviewers a bit of a > scare :)
ack, sounds good. I am also going to reduce the scope of the series a bit as i am only interested in support for TTL changelink right now.
