diff mbox series

[iproute2-rc] rdma: Fix statistics bind/unbing argument handling

Message ID 20210214083335.19558-1-leon@kernel.org (mailing list archive)
State Accepted
Delegated to: Stephen Hemminger
Headers show
Series [iproute2-rc] rdma: Fix statistics bind/unbing argument handling | expand

Checks

Context Check Description
netdev/tree_selection success Not a local patch

Commit Message

Leon Romanovsky Feb. 14, 2021, 8:33 a.m. UTC 
From: Ido Kalir <idok@nvidia.com>

The dump isn't supported for the statistics bind/unbind commands
because they operate on specific QP counters. This is different
from query commands that can operate on many objects at the same
time.

Let's check the user input and ensure that arguments are valid.

Fixes: a6d0773ebecc ("rdma: Add stat manual mode support")
Signed-off-by: Ido Kalir <idok@nvidia.com>
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
 rdma/rdma.h  |  1 +
 rdma/stat.c  | 21 +++++++++++++++++++++
 rdma/utils.c |  7 +++++++
 3 files changed, 29 insertions(+)

--
2.29.2

Comments

David Ahern Feb. 15, 2021, 3:26 a.m. UTC | #1 
what does iproute2-rc mean?
Leon Romanovsky Feb. 15, 2021, 5:40 a.m. UTC | #2 
On Sun, Feb 14, 2021 at 08:26:16PM -0700, David Ahern wrote:
> what does iproute2-rc mean?

Patch target is iproute2.git:
https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/
vs -next repo:
https://git.kernel.org/pub/scm/network/iproute2/iproute2-next.git/

How do you want me to mark the patches?

https://git.kernel.org/pub/scm/network/iproute2/

Thanks
David Ahern Feb. 16, 2021, 1:56 a.m. UTC | #3 
On 2/14/21 10:40 PM, Leon Romanovsky wrote:
> On Sun, Feb 14, 2021 at 08:26:16PM -0700, David Ahern wrote:
>> what does iproute2-rc mean?
> 
> Patch target is iproute2.git:
> https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/

so you are asking them to be committed for the 5.11 release?


> vs -next repo:
> https://git.kernel.org/pub/scm/network/iproute2/iproute2-next.git/
> 
> How do you want me to mark the patches?
> 
> https://git.kernel.org/pub/scm/network/iproute2/
> 
> Thanks
>
Leon Romanovsky Feb. 16, 2021, 6:16 a.m. UTC | #4 
On Mon, Feb 15, 2021 at 06:56:26PM -0700, David Ahern wrote:
> On 2/14/21 10:40 PM, Leon Romanovsky wrote:
> > On Sun, Feb 14, 2021 at 08:26:16PM -0700, David Ahern wrote:
> >> what does iproute2-rc mean?
> >
> > Patch target is iproute2.git:
> > https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/
>
> so you are asking them to be committed for the 5.11 release?

This is a Fix to an existing issue (not theoretical one), so I was under
impression that it should go to -rc repo and not to -next.

Personally, I don't care to which repo will this fix be applied as long
as it is applied to one of the two iproute2 official repos.

Do you have clear guidance when should I send patches to iproute2-rc/iproute2-next?

Thanks
David Ahern Feb. 16, 2021, 3:48 p.m. UTC | #5 
On 2/15/21 11:16 PM, Leon Romanovsky wrote:
> On Mon, Feb 15, 2021 at 06:56:26PM -0700, David Ahern wrote:
>> On 2/14/21 10:40 PM, Leon Romanovsky wrote:
>>> On Sun, Feb 14, 2021 at 08:26:16PM -0700, David Ahern wrote:
>>>> what does iproute2-rc mean?
>>>
>>> Patch target is iproute2.git:
>>> https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/
>>
>> so you are asking them to be committed for the 5.11 release?
> 
> This is a Fix to an existing issue (not theoretical one), so I was under
> impression that it should go to -rc repo and not to -next.

It is assigned to Stephen for iproute2.

> 
> Personally, I don't care to which repo will this fix be applied as long
> as it is applied to one of the two iproute2 official repos.
> 
> Do you have clear guidance when should I send patches to iproute2-rc/iproute2-next?
> 

It's the rc label that needs to be dropped: iproute2 or iproute2-next.
Just like there is net and net-next.
Leon Romanovsky Feb. 18, 2021, 8:44 a.m. UTC | #6 
On Tue, Feb 16, 2021 at 08:48:24AM -0700, David Ahern wrote:
> On 2/15/21 11:16 PM, Leon Romanovsky wrote:
> > On Mon, Feb 15, 2021 at 06:56:26PM -0700, David Ahern wrote:
> >> On 2/14/21 10:40 PM, Leon Romanovsky wrote:
> >>> On Sun, Feb 14, 2021 at 08:26:16PM -0700, David Ahern wrote:
> >>>> what does iproute2-rc mean?
> >>>
> >>> Patch target is iproute2.git:
> >>> https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/
> >>
> >> so you are asking them to be committed for the 5.11 release?
> >
> > This is a Fix to an existing issue (not theoretical one), so I was under
> > impression that it should go to -rc repo and not to -next.
>
> It is assigned to Stephen for iproute2.
>
> >
> > Personally, I don't care to which repo will this fix be applied as long
> > as it is applied to one of the two iproute2 official repos.
> >
> > Do you have clear guidance when should I send patches to iproute2-rc/iproute2-next?
> >
>
> It's the rc label that needs to be dropped: iproute2 or iproute2-next.

Sure, no problem.

Thanks
Stephen Hemminger Feb. 22, 2021, 7:18 p.m. UTC | #7 
On Thu, 18 Feb 2021 10:44:16 +0200
Leon Romanovsky <leon@kernel.org> wrote:

> On Tue, Feb 16, 2021 at 08:48:24AM -0700, David Ahern wrote:
> > On 2/15/21 11:16 PM, Leon Romanovsky wrote:  
> > > On Mon, Feb 15, 2021 at 06:56:26PM -0700, David Ahern wrote:  
> > >> On 2/14/21 10:40 PM, Leon Romanovsky wrote:  
> > >>> On Sun, Feb 14, 2021 at 08:26:16PM -0700, David Ahern wrote:  
> > >>>> what does iproute2-rc mean?  
> > >>>
> > >>> Patch target is iproute2.git:
> > >>> https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/  
> > >>
> > >> so you are asking them to be committed for the 5.11 release?  
> > >
> > > This is a Fix to an existing issue (not theoretical one), so I was under
> > > impression that it should go to -rc repo and not to -next.  
> >
> > It is assigned to Stephen for iproute2.
> >  
> > >
> > > Personally, I don't care to which repo will this fix be applied as long
> > > as it is applied to one of the two iproute2 official repos.
> > >
> > > Do you have clear guidance when should I send patches to iproute2-rc/iproute2-next?
> > >  
> >
> > It's the rc label that needs to be dropped: iproute2 or iproute2-next.  
> 
> Sure, no problem.
> 
> Thanks

Applied, and fixed a minor whitespace issue reported by checkpatch
diff mbox series

Patch

diff --git a/rdma/rdma.h b/rdma/rdma.h
index 735b1bf7..7f96c051 100644
--- a/rdma/rdma.h
+++ b/rdma/rdma.h
@@ -83,6 +83,7 @@  struct rd_cmd {
  * Parser interface
  */
 bool rd_no_arg(struct rd *rd);
+bool rd_is_multiarg(struct rd *rd);
 void rd_arg_inc(struct rd *rd);

 char *rd_argv(struct rd *rd);
diff --git a/rdma/stat.c b/rdma/stat.c
index 8d4b7a11..a6b6dfbf 100644
--- a/rdma/stat.c
+++ b/rdma/stat.c
@@ -455,6 +455,12 @@  static int stat_get_arg(struct rd *rd, const char *arg)
 		return -EINVAL;

 	rd_arg_inc(rd);
+
+	if (rd_is_multiarg(rd)){
+		pr_err("The parameter %s shouldn't include range\n", arg);
+		return -EINVAL;
+	}
+
 	value = strtol(rd_argv(rd), &endp, 10);
 	rd_arg_inc(rd);

@@ -476,6 +482,8 @@  static int stat_one_qp_bind(struct rd *rd)
 		return ret;

 	lqpn = stat_get_arg(rd, "lqpn");
+	if (lqpn < 0)
+		return lqpn;

 	rd_prepare_msg(rd, RDMA_NLDEV_CMD_STAT_SET,
 		       &seq, (NLM_F_REQUEST | NLM_F_ACK));
@@ -490,6 +498,9 @@  static int stat_one_qp_bind(struct rd *rd)

 	if (rd_argc(rd)) {
 		cntn = stat_get_arg(rd, "cntn");
+		if (cntn < 0)
+			return cntn;
+
 		mnl_attr_put_u32(rd->nlh, RDMA_NLDEV_ATTR_STAT_COUNTER_ID,
 				 cntn);
 	}
@@ -560,13 +571,23 @@  static int stat_one_qp_unbind(struct rd *rd)
 	unsigned int portid;
 	uint32_t seq;

+	if (rd_no_arg(rd)) {
+		stat_help(rd);
+		return -EINVAL;
+	}
+
 	ret = rd_build_filter(rd, stat_valid_filters);
 	if (ret)
 		return ret;

 	cntn = stat_get_arg(rd, "cntn");
+	if (cntn < 0)
+		return cntn;
+
 	if (rd_argc(rd)) {
 		lqpn = stat_get_arg(rd, "lqpn");
+		if (lqpn < 0)
+			return lqpn;
 		return do_stat_qp_unbind_lqpn(rd, cntn, lqpn);
 	}

diff --git a/rdma/utils.c b/rdma/utils.c
index e25c3adf..bbfa23ba 100644
--- a/rdma/utils.c
+++ b/rdma/utils.c
@@ -47,6 +47,13 @@  bool rd_no_arg(struct rd *rd)
 	return rd_argc(rd) == 0;
 }

+bool rd_is_multiarg(struct rd *rd)
+{
+	if (!rd_argc(rd))
+		return false;
+	return strpbrk(rd_argv(rd), ",-") != NULL;
+}
+
 /*
  * Possible input:output
  * dev/port    | first port | is_dump_all