MIME-Version: 1.0
References: <20211208040414.151960-1-xiayu.zhang@mediatek.com>
 <CAHNKnsRZpYsiWORgAejYwQqP5P=PSt-V7_i73G1yfh-UR2zFjw@mail.gmail.com>
 <6f4ae1d8b1b53cf998eaa14260d93fd3f4c8d5ad.camel@mediatek.com>
 <CAHNKnsQ6qLcUTiTiPEAp+rmoVtrGOjoY98nQFsrwSWUu-v7wYQ@mail.gmail.com> <76bc0c0174edc3a0c89bb880a237c844d44ac46b.camel@mediatek.com>
In-Reply-To: <76bc0c0174edc3a0c89bb880a237c844d44ac46b.camel@mediatek.com>
From:   Loic Poulain <loic.poulain@linaro.org>
Date:   Wed, 15 Dec 2021 10:29:48 +0100
Message-ID: <CAMZdPi_bpCdbxsEfh-=-LzGywsmXqoqujg-8H8y1bp2VM5uayw@mail.gmail.com>
Subject: Re: [PATCH] Add Multiple TX/RX Queues Support for WWAN Network Device
To:     Xiayu Zhang <xiayu.zhang@mediatek.com>
Cc:     Sergey Ryazanov <ryazanov.s.a@gmail.com>,
        David Miller <davem@davemloft.net>,
        Jakub Kicinski <kuba@kernel.org>,
        Johannes Berg <johannes@sipsolutions.net>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        open list <linux-kernel@vger.kernel.org>,
        =?UTF-8?B?SGFpanVuIExpdSAo5YiY5rW35YabKQ==?= 
        <haijun.liu@mediatek.com>,
        =?UTF-8?B?Wmhhb3BpbmcgU2h1ICjoiJLlj6zlubMp?= 
        <Zhaoping.Shu@mediatek.com>,
        =?UTF-8?B?SFcgSGUgKOS9leS8nyk=?= <HW.He@mediatek.com>,
        srv_heupstream <srv_heupstream@mediatek.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Xiayu, Sergey,

> > > > As for the default queues number selection it seems better to
> > > > implement the RTNL .get_num_rx_queues callback in the WWAN core
> > > > and
> > > > call optional driver specific callback through it. Something like
> > > > this:
> > > >
> > > > static unsigned int wwan_rtnl_get_num_tx_queues(struct nlattr
> > > > *tb[])
> > > > {
> > > >     const char *devname = nla_data(tb[IFLA_PARENT_DEV_NAME]);
> > > >     struct wwan_device *wwandev = wwan_dev_get_by_name(devname);
> > > >
> > > >     return wwandev && wwandev->ops && wwandev->ops-
> > > > >get_num_tx_queues
> > > > ?
> > > >               wwandev->ops->get_num_tx_queues() : 1;
> > > > }
> > > >
> > > > static struct rtnl_link_ops wwan_rtnl_link_ops __read_mostly = {
> > > >     ...
> > > >     .get_num_tx_queues = wwan_rtnl_get_num_tx_queues,
> > > > };
> > > >
> > > > This way the default queues number selection will be implemented
> > > > in a
> > > > less surprising way.
> > > >
> > > > But to be able to implement this we need to modify the RTNL ops
> > > > .get_num_tx_queues/.get_num_rx_queues callback definitions to
> > > > make
> > > > them able to accept the RTM_NEWLINK message attributes. This is
> > > > not
> > > > difficult since the callbacks are implemented only by a few
> > > > virtual
> > > > devices, but can be assumed too intrusive to implement a one
> > > > feature
> > > > for a single subsystem.

I agree with this solution. The wwan core can forward
get_num_tx_queues to per wwan driver callbacks or simply rely on a
'default_tx_queues' field in ops struct. As said previously, you need
a user for this change in the same series.

Regards,
Loic