On Sat, 21 Sep 2019 at 08:55, Jakub Kicinski
[off-list ref] wrote:

On Mon, 16 Sep 2019 22:48:00 +0900, Taehee Yoo wrote:

quoted

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 <ap420073@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.

quoted

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!