[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAMZdPi-_1y7nxovrNVgBH45QAtr_t4pozmUGQpDhymKNETq3Bg@mail.gmail.com>
Date: Fri, 11 Jun 2021 18:12:05 +0200
From: Loic Poulain <loic.poulain@...aro.org>
To: Parav Pandit <parav@...dia.com>
Cc: "kuba@...nel.org" <kuba@...nel.org>,
"davem@...emloft.net" <davem@...emloft.net>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"johannes.berg@...el.com" <johannes.berg@...el.com>,
"leon@...nel.org" <leon@...nel.org>,
"m.chetan.kumar@...el.com" <m.chetan.kumar@...el.com>,
Sergey Ryazanov <ryazanov.s.a@...il.com>
Subject: Re: [PATCH net-next v2 2/3] rtnetlink: add IFLA_PARENT_[DEV|DEV_BUS]_NAME
On Fri, 11 Jun 2021 at 18:00, Parav Pandit <parav@...dia.com> wrote:
>
>
>
> > From: Loic Poulain <loic.poulain@...aro.org>
> > Sent: Friday, June 11, 2021 9:16 PM
> >
> > Hi Parav,
> >
> > On Fri, 11 Jun 2021 at 15:01, Parav Pandit <parav@...dia.com> wrote:
> > >
> > >
> > >
> > > > From: Loic Poulain <loic.poulain@...aro.org>
> > > > Sent: Thursday, June 10, 2021 11:15 PM
> > > >
> > > > From: Johannes Berg <johannes.berg@...el.com>
> > > >
> > > > In some cases, for example in the upcoming WWAN framework changes,
> > > > there's no natural "parent netdev", so sometimes dummy netdevs are
> > > > created or similar. IFLA_PARENT_DEV_NAME is a new attribute intended
> > > > to contain a device (sysfs, struct device) name that can be used
> > > > instead when creating a new netdev, if the rtnetlink family implements it.
> > > >
> > > > As suggested by Parav Pandit, we also introduce
> > > > IFLA_PARENT_DEV_BUS_NAME attribute in order to uniquely identify a
> > > > device on the system (with bus/name pair).
> >
> > [...]
> >
> > > > diff --git a/include/uapi/linux/if_link.h
> > > > b/include/uapi/linux/if_link.h index
> > > > a5a7f0e..4882e81 100644
> > > > --- a/include/uapi/linux/if_link.h
> > > > +++ b/include/uapi/linux/if_link.h
> > > > @@ -341,6 +341,13 @@ enum {
> > > > IFLA_ALT_IFNAME, /* Alternative ifname */
> > > > IFLA_PERM_ADDRESS,
> > > > IFLA_PROTO_DOWN_REASON,
> > > > +
> > > > + /* device (sysfs) name as parent, used instead
> > > > + * of IFLA_LINK where there's no parent netdev
> > > > + */
> > > > + IFLA_PARENT_DEV_NAME,
> > > > + IFLA_PARENT_DEV_BUS_NAME,
> > > > +
> > > > __IFLA_MAX
> > > > };
> > > >
> > > > diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c index
> > > > 92c3e43..32599f3
> > > > 100644
> > > > --- a/net/core/rtnetlink.c
> > > > +++ b/net/core/rtnetlink.c
> > > > @@ -1821,6 +1821,16 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb,
> > > > if (rtnl_fill_prop_list(skb, dev))
> > > > goto nla_put_failure;
> > > >
> > > > + if (dev->dev.parent &&
> > > > + nla_put_string(skb, IFLA_PARENT_DEV_NAME,
> > > > + dev_name(dev->dev.parent)))
> > > > + goto nla_put_failure;
> > > > +
> > > > + if (dev->dev.parent && dev->dev.parent->bus &&
> > > > + nla_put_string(skb, IFLA_PARENT_DEV_BUS_NAME,
> > > > + dev->dev.parent->bus->name))
> > > > + goto nla_put_failure;
> > > > +
> > > > nlmsg_end(skb, nlh);
> > > > return 0;
> > > >
> > > > @@ -1880,6 +1890,8 @@ static const struct nla_policy
> > > > ifla_policy[IFLA_MAX+1] = {
> > > > [IFLA_PERM_ADDRESS] = { .type = NLA_REJECT },
> > > > [IFLA_PROTO_DOWN_REASON] = { .type = NLA_NESTED },
> > > > [IFLA_NEW_IFINDEX] = NLA_POLICY_MIN(NLA_S32, 1),
> > > > + [IFLA_PARENT_DEV_NAME] = { .type = NLA_NUL_STRING },
> > > > + [IFLA_PARENT_DEV_BUS_NAME] = { .type = NLA_NUL_STRING },
> > > > };
> > > >
> > > This hunk should go in the patch that enables users to use these fields to
> > specify it for new link creation.
> >
> > Don't get it, the previous changes I see in the tree change both if_link.h and
> > rtnetlink.c for new atributes (e.g. f74877a5457d). Can you elaborate on what
> > you expect here?
> >
> Commit f74877a5457d did in same patch because one of the objective of commit f74877a5457d is to also block PERM_ADDR using policy NLA_REJECT.
> This patch-2 is not enabling user to pass these params via new_link command.
> It is done in a later patch. So ifla_policy doesn’t need to have these fields in this patch.
Understood, thanks.
Loic
Powered by blists - more mailing lists