From: Jakub Kicinski <kuba@kernel.org>
To: Tang Longjun <lange_tang@163.com>
Cc: xuanzhuo@linux.alibaba.com, jasowang@redhat.com, mst@redhat.com,
davem@davemloft.net, edumazet@google.com, netdev@vger.kernel.org,
virtualization@lists.linux.dev,
Tang Longjun <tanglongjun@kylinos.cn>
Subject: Re: [PATCH] virtio_net: Fix duplicated return values in virtnet_get_hw_stats
Date: Thu, 15 May 2025 07:10:42 -0700 [thread overview]
Message-ID: <20250515071042.5f668345@kernel.org> (raw)
In-Reply-To: <20250514054433.29709-1-lange_tang@163.com>
On Wed, 14 May 2025 13:44:33 +0800 Tang Longjun wrote:
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index e53ba600605a..c9a86f325619 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -4897,7 +4897,7 @@ static int __virtnet_get_hw_stats(struct virtnet_info *vi,
> &sgs_out, &sgs_in);
>
> if (!ok)
> - return ok;
> + return 1;
This makes sense, looks like a typo in the original code.
But we are now returning the reverse polarity of "ok", so we should
probably rename the variable in virtnet_get_hw_stats() ok -> failed
Or invert the polarity here and in virtnet_get_hw_stats()
> for (p = reply; p - reply < res_size; p += le16_to_cpu(hdr->size)) {
> hdr = p;
> @@ -4937,7 +4937,7 @@ static int virtnet_get_hw_stats(struct virtnet_info *vi,
> int ok;
>
> if (!virtio_has_feature(vi->vdev, VIRTIO_NET_F_DEVICE_STATS))
> - return 0;
> + return -EOPNOTSUPP;
IDK about this part. We should not spam the logs if the device does not
support a feature. We should instead skip reporting the relevant stats.
User can tell that those stats are not supported from the fact they are
not present.
--
pw-bot: cr
next prev parent reply other threads:[~2025-05-15 14:10 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-05-14 5:44 [PATCH] virtio_net: Fix duplicated return values in virtnet_get_hw_stats Tang Longjun
2025-05-15 14:10 ` Jakub Kicinski [this message]
2025-05-16 7:36 ` Lange Tang
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=20250515071042.5f668345@kernel.org \
--to=kuba@kernel.org \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=jasowang@redhat.com \
--cc=lange_tang@163.com \
--cc=mst@redhat.com \
--cc=netdev@vger.kernel.org \
--cc=tanglongjun@kylinos.cn \
--cc=virtualization@lists.linux.dev \
--cc=xuanzhuo@linux.alibaba.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.