[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAMZdPi-6Fpft80Vis-NR4grfcyfdH9DTs9BHfE-J+-6_c+2dJw@mail.gmail.com>
Date: Tue, 22 Jun 2021 16:04:04 +0200
From: Loic Poulain <loic.poulain@...aro.org>
To: Sergey Ryazanov <ryazanov.s.a@...il.com>
Cc: Johannes Berg <johannes@...solutions.net>,
"David S. Miller" <davem@...emloft.net>,
Jakub Kicinski <kuba@...nel.org>,
Network Development <netdev@...r.kernel.org>,
M Chetan Kumar <m.chetan.kumar@...el.com>,
Intel Corporation <linuxwwan@...el.com>
Subject: Re: [PATCH net-next v2 08/10] wwan: core: support default netdev creation
Hi Sergey,
On Tue, 22 Jun 2021 at 00:51, Sergey Ryazanov <ryazanov.s.a@...il.com> wrote:
>
> Most, if not each WWAN device driver will create a netdev for the
> default data channel. Therefore, add an option for the WWAN netdev ops
> registration function to create a default netdev for the WWAN device.
>
> A WWAN device driver should pass a default data channel link id to the
> ops registering function to request the creation of a default netdev, or
> a special value WWAN_NO_DEFAULT_LINK to inform the WWAN core that the
> default netdev should not be created.
>
> For now, only wwan_hwsim utilize the default link creation option. Other
> drivers will be reworked next.
>
> Signed-off-by: Sergey Ryazanov <ryazanov.s.a@...il.com>
> CC: M Chetan Kumar <m.chetan.kumar@...el.com>
> CC: Intel Corporation <linuxwwan@...el.com>
> ---
>
> v1 -> v2:
> * fix a comment style '/**' -> '/*' to avoid confusion with kernel-doc
>
> drivers/net/mhi/net.c | 3 +-
> drivers/net/wwan/iosm/iosm_ipc_wwan.c | 3 +-
> drivers/net/wwan/wwan_core.c | 75 ++++++++++++++++++++++++++-
> drivers/net/wwan/wwan_hwsim.c | 2 +-
> include/linux/wwan.h | 8 ++-
> 5 files changed, 86 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/mhi/net.c b/drivers/net/mhi/net.c
> index ffd1c01b3f35..f36ca5c0dfe9 100644
> --- a/drivers/net/mhi/net.c
> +++ b/drivers/net/mhi/net.c
> @@ -397,7 +397,8 @@ static int mhi_net_probe(struct mhi_device *mhi_dev,
> struct net_device *ndev;
> int err;
>
> - err = wwan_register_ops(&cntrl->mhi_dev->dev, &mhi_wwan_ops, mhi_dev);
> + err = wwan_register_ops(&cntrl->mhi_dev->dev, &mhi_wwan_ops, mhi_dev,
> + WWAN_NO_DEFAULT_LINK);
> if (err)
> return err;
>
> diff --git a/drivers/net/wwan/iosm/iosm_ipc_wwan.c b/drivers/net/wwan/iosm/iosm_ipc_wwan.c
> index bee9b278223d..adb2bd40a404 100644
> --- a/drivers/net/wwan/iosm/iosm_ipc_wwan.c
> +++ b/drivers/net/wwan/iosm/iosm_ipc_wwan.c
> @@ -317,7 +317,8 @@ struct iosm_wwan *ipc_wwan_init(struct iosm_imem *ipc_imem, struct device *dev)
> ipc_wwan->dev = dev;
> ipc_wwan->ipc_imem = ipc_imem;
>
> - if (wwan_register_ops(ipc_wwan->dev, &iosm_wwan_ops, ipc_wwan)) {
> + if (wwan_register_ops(ipc_wwan->dev, &iosm_wwan_ops, ipc_wwan,
> + WWAN_NO_DEFAULT_LINK)) {
> kfree(ipc_wwan);
> return NULL;
> }
> diff --git a/drivers/net/wwan/wwan_core.c b/drivers/net/wwan/wwan_core.c
> index b634a0ba1196..ef6ec641d877 100644
> --- a/drivers/net/wwan/wwan_core.c
> +++ b/drivers/net/wwan/wwan_core.c
> @@ -903,17 +903,81 @@ static struct rtnl_link_ops wwan_rtnl_link_ops __read_mostly = {
> .policy = wwan_rtnl_policy,
> };
>
> +static void wwan_create_default_link(struct wwan_device *wwandev,
> + u32 def_link_id)
> +{
> + struct nlattr *tb[IFLA_MAX + 1], *linkinfo[IFLA_INFO_MAX + 1];
> + struct nlattr *data[IFLA_WWAN_MAX + 1];
> + struct net_device *dev;
> + struct nlmsghdr *nlh;
> + struct sk_buff *msg;
> +
> + /* Forge attributes required to create a WWAN netdev. We first
> + * build a netlink message and then parse it. This looks
> + * odd, but such approach is less error prone.
> + */
> + msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
> + if (WARN_ON(!msg))
> + return;
> + nlh = nlmsg_put(msg, 0, 0, RTM_NEWLINK, 0, 0);
> + if (WARN_ON(!nlh))
> + goto free_attrs;
> +
> + if (nla_put_string(msg, IFLA_PARENT_DEV_NAME, dev_name(&wwandev->dev)))
> + goto free_attrs;
> + tb[IFLA_LINKINFO] = nla_nest_start(msg, IFLA_LINKINFO);
> + if (!tb[IFLA_LINKINFO])
> + goto free_attrs;
> + linkinfo[IFLA_INFO_DATA] = nla_nest_start(msg, IFLA_INFO_DATA);
> + if (!linkinfo[IFLA_INFO_DATA])
> + goto free_attrs;
> + if (nla_put_u32(msg, IFLA_WWAN_LINK_ID, def_link_id))
> + goto free_attrs;
> + nla_nest_end(msg, linkinfo[IFLA_INFO_DATA]);
> + nla_nest_end(msg, tb[IFLA_LINKINFO]);
> +
> + nlmsg_end(msg, nlh);
> +
> + /* The next three parsing calls can not fail */
> + nlmsg_parse_deprecated(nlh, 0, tb, IFLA_MAX, NULL, NULL);
> + nla_parse_nested_deprecated(linkinfo, IFLA_INFO_MAX, tb[IFLA_LINKINFO],
> + NULL, NULL);
> + nla_parse_nested_deprecated(data, IFLA_WWAN_MAX,
> + linkinfo[IFLA_INFO_DATA], NULL, NULL);
> +
> + rtnl_lock();
> +
> + dev = rtnl_create_link(&init_net, "wwan%d", NET_NAME_ENUM,
> + &wwan_rtnl_link_ops, tb, NULL);
That can be a bit confusing since the same naming convention as for
the WWAN devices is used, so you can end with something like a wwan0
netdev being a link of wwan1 dev. Maybe something like ('%s.%d',
dev_name(&wwandev->dev), link_id) to have e.g. wwan1.0 for link 0 of
the wwan1 device. Or alternatively, the specific wwan driver to
specify the default name to use.
Regards,
Loic
Powered by blists - more mailing lists