netdev - Re: [PATCH net v3 09/11] net: core: add ignore flag to netdev

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sun, 22 Sep 2019 21:54:37 +0900
From:   Taehee Yoo <ap420073@...il.com>
To:     Jakub Kicinski <jakub.kicinski@...ronome.com>
Cc:     David Miller <davem@...emloft.net>,
        Netdev <netdev@...r.kernel.org>, j.vosburgh@...il.com,
        vfalico@...il.com, Andy Gospodarek <andy@...yhouse.net>,
        Jiří Pírko <jiri@...nulli.us>,
        sd@...asysnail.net, Roopa Prabhu <roopa@...ulusnetworks.com>,
        saeedm@...lanox.com, manishc@...vell.com, rahulv@...vell.com,
        kys@...rosoft.com, haiyangz@...rosoft.com,
        Stephen Hemminger <stephen@...workplumber.org>,
        sashal@...nel.org, hare@...e.de, varun@...lsio.com,
        ubraun@...ux.ibm.com, kgraul@...ux.ibm.com,
        Jay Vosburgh <jay.vosburgh@...onical.com>
Subject: Re: [PATCH net v3 09/11] net: core: add ignore flag to
 netdev_adjacent structure

On Sat, 21 Sep 2019 at 08:55, Jakub Kicinski
<jakub.kicinski@...ronome.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 <ap420073@...il.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!