From: Jakub Kicinski <kuba@kernel.org>
To: Koichiro Den <den@valinux.co.jp>
Cc: Jon Mason <jdmason@kudzu.us>, Dave Jiang <dave.jiang@intel.com>,
Allen Hubbe <allenbh@gmail.com>,
Andrew Lunn <andrew+netdev@lunn.ch>,
"David S. Miller" <davem@davemloft.net>,
Eric Dumazet <edumazet@google.com>,
Paolo Abeni <pabeni@redhat.com>,
ntb@lists.linux.dev, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 1/4] net: ntb_netdev: Introduce per-queue context
Date: Wed, 4 Mar 2026 17:13:37 -0800 [thread overview]
Message-ID: <20260304171337.5bc44eef@kernel.org> (raw)
In-Reply-To: <20260228145538.3955864-2-den@valinux.co.jp>
On Sat, 28 Feb 2026 23:55:35 +0900 Koichiro Den wrote:
> @@ -99,7 +114,9 @@ static void ntb_netdev_event_handler(void *data, int link_is_up)
> static void ntb_netdev_rx_handler(struct ntb_transport_qp *qp, void *qp_data,
> void *data, int len)
> {
> - struct net_device *ndev = qp_data;
> + struct ntb_netdev_queue *q = qp_data;
> + struct ntb_netdev *dev = q->ntdev;
> + struct net_device *ndev = dev->ndev;
> struct sk_buff *skb;
> int rc;
>
> @@ -118,6 +135,7 @@ static void ntb_netdev_rx_handler(struct ntb_transport_qp *qp, void *qp_data,
> skb_put(skb, len);
> skb->protocol = eth_type_trans(skb, ndev);
> skb->ip_summed = CHECKSUM_NONE;
> + skb_record_rx_queue(skb, q->qid);
>
> if (netif_rx(skb) == NET_RX_DROP) {
> ndev->stats.rx_errors++;
> @@ -135,7 +153,8 @@ static void ntb_netdev_rx_handler(struct ntb_transport_qp *qp, void *qp_data,
> }
>
> enqueue_again:
> - rc = ntb_transport_rx_enqueue(qp, skb, skb->data, ndev->mtu + ETH_HLEN);
> + rc = ntb_transport_rx_enqueue(q->qp, skb, skb->data,
nit: I think in this case you don't have to replace qp qith q->qp ?
qp is an argument of ntb_netdev_rx_handler()
> @@ -208,16 +227,26 @@ static netdev_tx_t ntb_netdev_start_xmit(struct sk_buff *skb,
> struct net_device *ndev)
> {
> struct ntb_netdev *dev = netdev_priv(ndev);
> + u16 qid = skb_get_queue_mapping(skb);
> + struct ntb_netdev_queue *q;
> int rc;
>
> - ntb_netdev_maybe_stop_tx(ndev, dev->qp, tx_stop);
> + if (unlikely(!dev->num_queues))
please avoid such defensive checks, this should never happen
> + goto err;
>
> - rc = ntb_transport_tx_enqueue(dev->qp, skb, skb->data, skb->len);
> + if (unlikely(qid >= dev->num_queues))
same, the stack should not sent patches for disabled queues
> + qid = 0;
> +
> static void ntb_netdev_tx_timer(struct timer_list *t)
> {
> - struct ntb_netdev *dev = timer_container_of(dev, t, tx_timer);
> + struct ntb_netdev_queue *q = timer_container_of(q, t, tx_timer);
> + struct ntb_netdev *dev = q->ntdev;
> struct net_device *ndev = dev->ndev;
nit: if you can't maintain the longest to shortest order because of
dependencies the init should be moved out-of-line
> static int ntb_netdev_open(struct net_device *ndev)
> {
> struct ntb_netdev *dev = netdev_priv(ndev);
> + struct ntb_netdev_queue *queue;
> struct sk_buff *skb;
> - int rc, i, len;
> + unsigned int q;
> + int rc = 0, i, len;
nit: sort variable declaration lines longest to shortest (there's at
least one more of such issues in this patch)
> @@ -420,6 +485,16 @@ static int ntb_netdev_probe(struct device *client_dev)
> dev = netdev_priv(ndev);
> dev->ndev = ndev;
> dev->pdev = pdev;
> + dev->client_dev = client_dev;
> + dev->num_queues = 0;
> +
> + dev->queues = kcalloc(NTB_NETDEV_MAX_QUEUES, sizeof(*dev->queues),
> + GFP_KERNEL);
I think we may be expected to use kzalloc_objs() now
> @@ -464,9 +560,14 @@ static void ntb_netdev_remove(struct device *client_dev)
> {
> struct net_device *ndev = dev_get_drvdata(client_dev);
> struct ntb_netdev *dev = netdev_priv(ndev);
> + unsigned int q;
> +
>
nit double new line
--
pw-bot: cr
next prev parent reply other threads:[~2026-03-05 1:13 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-02-28 14:55 [PATCH v2 0/4] net: ntb_netdev: Add Multi-queue support Koichiro Den
2026-02-28 14:55 ` [PATCH v2 1/4] net: ntb_netdev: Introduce per-queue context Koichiro Den
2026-03-05 1:13 ` Jakub Kicinski [this message]
2026-03-05 13:56 ` Koichiro Den
2026-02-28 14:55 ` [PATCH v2 2/4] net: ntb_netdev: Gate subqueue stop/wake by transport link Koichiro Den
2026-02-28 14:55 ` [PATCH v2 3/4] net: ntb_netdev: Factor out multi-queue helpers Koichiro Den
2026-02-28 14:55 ` [PATCH v2 4/4] net: ntb_netdev: Support ethtool channels for multi-queue Koichiro Den
2026-03-05 1:19 ` Jakub Kicinski
2026-03-05 13:47 ` Koichiro Den
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260304171337.5bc44eef@kernel.org \
--to=kuba@kernel.org \
--cc=allenbh@gmail.com \
--cc=andrew+netdev@lunn.ch \
--cc=dave.jiang@intel.com \
--cc=davem@davemloft.net \
--cc=den@valinux.co.jp \
--cc=edumazet@google.com \
--cc=jdmason@kudzu.us \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=ntb@lists.linux.dev \
--cc=pabeni@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox