All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jacob Keller <jacob.e.keller@intel.com>
To: netdev@vger.kernel.org
Cc: jiri@resnulli.us, valex@mellanox.com, parav@mellanox.com,
	Jacob Keller <jacob.e.keller@intel.com>
Subject: [net] devlink: report 0 after hitting end in region read
Date: Tue,  4 Feb 2020 15:59:50 -0800	[thread overview]
Message-ID: <20200204235950.2209828-1-jacob.e.keller@intel.com> (raw)

commit fdd41ec21e15 ("devlink: Return right error code in case of errors
for region read") modified the region read code to report errors
properly in unexpected cases.

In the case where the start_offset and ret_offset match, it unilaterally
converted this into an error. This causes an issue for the "dump"
version of the command. In this case, the devlink region dump will
always report an invalid argument:

000000000000ffd0 ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
000000000000ffe0 ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
devlink answers: Invalid argument
000000000000fff0 ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff

This occurs because the expected flow for the dump is to return 0 after
there is no further data.

The simplest fix would be to stop converting the error code to -EINVAL
if start_offset == ret_offset. However, avoid unnecessary work by
checking for when start_offset is larger than the region size and
returning 0 upfront.

Fixes: fdd41ec21e15 ("devlink: Return right error code in case of errors for region read")
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
---
I noticed this while working on adding the new region reading support at

https://lore.kernel.org/netdev/20200130225913.1671982-1-jacob.e.keller@intel.com/https://lore.kernel.org/netdev/20200130225913.1671982-1-jacob.e.keller@intel.com/

This is fairly cosmetic at least for the devlink userspace tools in iproute2
suite, but it is annoying. It appears to only happen with the dump-based
logic that does not specify a range or initial offset.

 net/core/devlink.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/net/core/devlink.c b/net/core/devlink.c
index ca1df0ec3c97..549ee56b7a21 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -3986,6 +3986,12 @@ static int devlink_nl_cmd_region_read_dumpit(struct sk_buff *skb,
 		goto out_unlock;
 	}
 
+	/* return 0 if there is no further data to read */
+	if (start_offset >= region->size) {
+		err = 0;
+		goto out_unlock;
+	}
+
 	hdr = genlmsg_put(skb, NETLINK_CB(cb->skb).portid, cb->nlh->nlmsg_seq,
 			  &devlink_nl_family, NLM_F_ACK | NLM_F_MULTI,
 			  DEVLINK_CMD_REGION_READ);
-- 
2.25.0.rc1


             reply	other threads:[~2020-02-04 23:59 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-04 23:59 Jacob Keller [this message]
2020-02-05  6:18 ` [net] devlink: report 0 after hitting end in region read Jiri Pirko
2020-02-05 13:25 ` David Miller

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=20200204235950.2209828-1-jacob.e.keller@intel.com \
    --to=jacob.e.keller@intel.com \
    --cc=jiri@resnulli.us \
    --cc=netdev@vger.kernel.org \
    --cc=parav@mellanox.com \
    --cc=valex@mellanox.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.