On Sat, 21 Sep 2019 at 08:55, Jakub Kicinski <jakub.kicin...@netronome.com> wrote: > > On Mon, 16 Sep 2019 22:48:00 +0900, Taehee Yoo wrote: > > In order to link an adjacent node, netdev_upper_dev_link() is used > > and in order to unlink an adjacent node, netdev_upper_dev_unlink() is used. > > unlink operation does not fail, but link operation can fail. > > > > In order to exchange adjacent nodes, we should unlink an old adjacent > > node first. then, link a new adjacent node. > > If link operation is failed, we should link an old adjacent node again. > > But this link operation can fail too. > > It eventually breaks the adjacent link relationship. > > > > This patch adds an ignore flag into the netdev_adjacent structure. > > If this flag is set, netdev_upper_dev_link() ignores an old adjacent > > node for a moment. > > So we can skip unlink operation before link operation. > > > > Signed-off-by: Taehee Yoo <ap420...@gmail.com> > > Could this perhaps be achieved by creating prepare, commit, and abort > helpers? That would make the API look slightly more canonical. > > netdev_adjacent_change_prepare(old, new, dev) > netdev_adjacent_change_commit(old, new, dev) > netdev_adjacent_change_abort(old, new, dev) > > The current naming makes the operation a little harder to follow if one > is just reading the vxlan code. >
I fully agree with your opinion. I will provide these three functions that you mentioned. netdev_adjacent_change_prepare netdev_adjacent_change_commit netdev_adjacent_change_abort > Please let me know if I didn't read the code closely enough to > understand why that's not fitting here. > > > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h > > index 5bb5756129af..4506810c301b 100644 > > --- a/include/linux/netdevice.h > > +++ b/include/linux/netdevice.h > > @@ -4319,6 +4319,10 @@ int netdev_master_upper_dev_link(struct net_device > > *dev, > > struct netlink_ext_ack *extack); > > void netdev_upper_dev_unlink(struct net_device *dev, > > struct net_device *upper_dev); > > +void netdev_adjacent_dev_disable(struct net_device *upper_dev, > > + struct net_device *lower_dev); > > +void netdev_adjacent_dev_enable(struct net_device *upper_dev, > > + struct net_device *lower_dev); > > void netdev_adjacent_rename_links(struct net_device *dev, char *oldname); > > void *netdev_lower_dev_get_private(struct net_device *dev, > > struct net_device *lower_dev); I found that I missed static keyword. So I will fix this too. Thank you!
