Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next v11 1/4] net netlink: Add new type NLA_BITFIELD_32
From: Jiri Pirko @ 2017-07-28 14:55 UTC (permalink / raw)
  To: David Ahern
  Cc: Jamal Hadi Salim, davem, netdev, xiyou.wangcong, eric.dumazet,
	mrv, simon.horman, alex.aring
In-Reply-To: <886e9fe5-d523-4841-1a81-e5671447933a@gmail.com>

Fri, Jul 28, 2017 at 04:19:06PM CEST, dsahern@gmail.com wrote:
>On 7/28/17 7:51 AM, Jamal Hadi Salim wrote:
>> On 17-07-25 10:41 AM, David Ahern wrote:
>>> On 7/23/17 7:35 PM, Jamal Hadi Salim wrote:
>>>> In the most basic form, the user specifies the attribute policy as:
>>>> [ATTR_GOO] = { .type = NLA_BITFIELD_32, .validation_data =
>>>> &myvalidflags },
>>>>
>>>> where myvalidflags is the bit mask of the flags the kernel understands.
>>>>
>>>> If the user _does not_ provide myvalidflags then the attribute will
>>>> also be rejected.
>>>
>>> No other netlink attribute has this requirement. 
>> 
>> This is the first one where we have to inspect content. We add things
>> when we need them - as in this case.
>
>Sure, the validation is required. My argument is that the validation
>should be done where other attributes are validated -- inline with its
>use. Nothing about this new bitfield says it must have a generic
>validation code.
>
>> 
>>> Users of the attributes
>>> are the only ones that know if a value is valid or not (e.g, attribute
>>> passing a device index) and those are always checked in line.
>> 
>> It doesnt make sense that every user of the API has to repeat that
>> validation code. Same principle as someone specifying that a type is
>> u32 and have the nla validation check it. At some point we never had
>> the u32 validation code. Then it was factored out because everyone
>> repeats the same boilerplate code.
>
>Every user of an attribute that uses a device index must verify the
>device index is valid. The same code is repeated over and over.

This is something different. You don't have NLA_IFINDEX. If you'd have it,
might make sense to do validation on Netlink level. Ofc this is highly
hypothetical. But in Jamal's case, there is indeed NLA_BITFIELD32 and
this attribute type itself assumes some format. Therefore the validation
on Netlink level makes sense here. At least that is how I feel it.


>
>Now you are suggesting to have 1 attribute whose content is validated by
>generic infra and the rest are validated inline by the code using it. I
>believe it is wrong and going to lead to problems.

^ permalink raw reply

* Re: mwifiex: fix spelling mistake: "Insuffient" -> "Insufficient"
From: Kalle Valo @ 2017-07-28 14:54 UTC (permalink / raw)
  To: Colin Ian King
  Cc: Amitkumar Karwar, Nishant Sarmukadam, Ganapathi Bhat, Xinming Hu,
	linux-wireless, netdev, linux-kernel
In-Reply-To: <20170727220622.11957-1-colin.king@canonical.com>

Colin Ian King <colin.king@canonical.com> wrote:

> From: Colin Ian King <colin.king@canonical.com>
> 
> Trivial fix to spelling mistake in mwifiex_dbg debug message
> 
> Signed-off-by: Colin Ian King <colin.king@canonical.com>

Patch applied to wireless-drivers-next.git, thanks.

17830147c40a mwifiex: fix spelling mistake: "Insuffient" -> "Insufficient"

-- 
https://patchwork.kernel.org/patch/9867575/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

^ permalink raw reply

* [PATCH] hysdn: fix to a race condition in put_log_buffer
From: Anton Volkov @ 2017-07-28 14:53 UTC (permalink / raw)
  To: isdn; +Cc: netdev, linux-kernel, ldv-project, khoroshilov
In-Reply-To: <2f35ddaf-88d9-e848-f83b-3001b36b2883@linux-pingi.de>

The synchronization type that was used earlier to guard the loop that
deletes unused log buffers may have lead to a situation that prevents 
any thread from going through the loop.

The patch deletes previously used synchronization mechanism and moves
the loop under the spin_lock so the similar cases won't be feasible in
the future.

Found by by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Anton Volkov <avolkov@ispras.ru>
---
  drivers/isdn/hysdn/hysdn_proclog.c | 27 ++++++++++++---------------
  1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/drivers/isdn/hysdn/hysdn_proclog.c 
b/drivers/isdn/hysdn/hysdn_proclog.c
index 7b5fd8fb1761..b152c6ca444b 100644
--- a/drivers/isdn/hysdn/hysdn_proclog.c
+++ b/drivers/isdn/hysdn/hysdn_proclog.c
@@ -44,7 +44,6 @@ struct procdata {
  	char log_name[15];	/* log filename */
  	struct log_data *log_head, *log_tail;	/* head and tail for queue */
  	int if_used;		/* open count for interface */
-	int volatile del_lock;	/* lock for delete operations */
  	unsigned char logtmp[LOG_MAX_LINELEN];
  	wait_queue_head_t rd_queue;
  };
@@ -102,7 +101,6 @@ put_log_buffer(hysdn_card *card, char *cp)
  {
  	struct log_data *ib;
  	struct procdata *pd = card->proclog;
-	int i;
  	unsigned long flags;

  	if (!pd)
@@ -126,21 +124,20 @@ put_log_buffer(hysdn_card *card, char *cp)
  	else
  		pd->log_tail->next = ib;	/* follows existing messages */
  	pd->log_tail = ib;	/* new tail */
-	i = pd->del_lock++;	/* get lock state */
-	spin_unlock_irqrestore(&card->hysdn_lock, flags);

  	/* delete old entrys */
-	if (!i)
-		while (pd->log_head->next) {
-			if ((pd->log_head->usage_cnt <= 0) &&
-			    (pd->log_head->next->usage_cnt <= 0)) {
-				ib = pd->log_head;
-				pd->log_head = pd->log_head->next;
-				kfree(ib);
-			} else
-				break;
-		}		/* pd->log_head->next */
-	pd->del_lock--;		/* release lock level */
+	while (pd->log_head->next) {
+		if ((pd->log_head->usage_cnt <= 0) &&
+		    (pd->log_head->next->usage_cnt <= 0)) {
+			ib = pd->log_head;
+			pd->log_head = pd->log_head->next;
+			kfree(ib);
+		} else
+			break;
+	}		/* pd->log_head->next */
+
+	spin_unlock_irqrestore(&card->hysdn_lock, flags);
+
  	wake_up_interruptible(&(pd->rd_queue));		/* announce new entry */
  }				/* put_log_buffer */

-- 
2.11.0

^ permalink raw reply related

* Re: [PATCH net-next v2 0/3] ethtool: support for forward error correction mode setting on a link
From: Roopa Prabhu @ 2017-07-28 14:53 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: davem@davemloft.net, John W. Linville, netdev@vger.kernel.org,
	Vidya Sagar Ravipati, Dustin Byford, Dave Olson, Casey Leedom,
	Gal Pressman, Andrew Lunn, Manoj Malviya, Santosh Rastapur,
	yuval.mintz, odedw, Ariel Almog, Jeff Kirsher
In-Reply-To: <20170727193320.308e86c3@cakuba.netronome.com>

On Thu, Jul 27, 2017 at 7:33 PM, Jakub Kicinski <kubakici@wp.pl> wrote:
> On Thu, 27 Jul 2017 16:47:25 -0700, Roopa Prabhu wrote:
>> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>
>> Forward Error Correction (FEC) modes i.e Base-R
>> and Reed-Solomon modes are introduced in 25G/40G/100G standards
>> for providing good BER at high speeds. Various networking devices
>> which support 25G/40G/100G provides ability to manage supported FEC
>> modes and the lack of FEC encoding control and reporting today is a
>> source for interoperability issues for many vendors.
>> FEC capability as well as specific FEC mode i.e. Base-R
>> or RS modes can be requested or advertised through bits D44:47 of base link
>> codeword.
>>
>> This patch set intends to provide option under ethtool to manage and
>> report FEC encoding settings for networking devices as per IEEE 802.3
>> bj, bm and by specs.
>>
>> v2 :
>>         - minor patch format fixes and typos pointed out by Andrew
>>         - there was a pending discussion on the use of 'auto' vs
>>           'automatic' for fec settings. I have left it as 'auto'
>>           because in most cases today auto is used in place of
>>           automatic to represent automatically generated values.
>>           We use it in other networking config too. I would prefer
>>           leaving it as auto.
>
> On the subject of resetting the values when module is replugged I
> assume what was previously described remains:
>  - we always allow users to set the FEC regardless of the module type;
>  - if user set an incorrect FEC for the module type (or module gets
>    swapped) the link will be administratively taken down by either
>    the driver or FW.
>
> Is that correct?  Am I misremembering?

yes, correct. And possible future sfp hotplug events can give user-space
more info to react to module type changes etc.

^ permalink raw reply

* Re: mwifiex: usb: fix spelling mistake: "aggreataon"-> "aggregation"
From: Kalle Valo @ 2017-07-28 14:52 UTC (permalink / raw)
  To: Colin Ian King
  Cc: Amitkumar Karwar, Nishant Sarmukadam, Ganapathi Bhat, Xinming Hu,
	linux-wireless, netdev, linux-kernel
In-Reply-To: <20170724222623.16652-1-colin.king@canonical.com>

Colin Ian King <colin.king@canonical.com> wrote:

> From: Colin Ian King <colin.king@canonical.com>
> 
> Trivial fix to spelling mistake in aggr_ctrl module parameter
> message text.
> 
> Signed-off-by: Colin Ian King <colin.king@canonical.com>

Patch applied to wireless-drivers-next.git, thanks.

c55971726c40 mwifiex: usb: fix spelling mistake: "aggreataon"-> "aggregation"

-- 
https://patchwork.kernel.org/patch/9860753/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

^ permalink raw reply

* Re: [PATCH net-next v11 3/4] net sched actions: dump more than TCA_ACT_MAX_PRIO actions per batch
From: Jamal Hadi Salim @ 2017-07-28 14:52 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: davem, netdev, xiyou.wangcong, dsahern, eric.dumazet, mrv,
	simon.horman, alex.aring
In-Reply-To: <20170728141234.GD1857@nanopsycho>

On 17-07-28 10:12 AM, Jiri Pirko wrote:
> Fri, Jul 28, 2017 at 03:41:44PM CEST, jhs@mojatatu.com wrote:
[..]
> 
> Looks like a big mess to be honest. Mixing up u32* u32 void*. I don't
> understand ****. Would be probably good to first apply my review comment
> on the function itselt, then to add the checks :)
> 

I havent even compiled/test that Jiri.
Just ignore the void * and assume it is a u32 *.

I am trying to avoid doing unlucky number 13 patch.
So feedback on this is good. Just look at what it is disallowing
first.

back later.

cheers,
jamal

> 
>> I can think of.
>>
>> static int validate_nla_bitfield32(const struct nlattr *nla,
>>                                     void *valid_flags_allowed)
>> {
>>         const struct nla_bitfield32 *bf = nla_data(nla);
>>         u32 *valid_flags_mask = valid_flags_allowed;
>>
>>         if (!valid_flags_allowed)
>>                 return -EINVAL;
>>         /*disallow invalid selector */
>>         if ((bf->selector & valid_flags_allowed) >*valid_flags_allowed)
>>                 return -EINVAL;
>>         /*disallow invalid bit values */
>>         if (bf->value & ~*valid_flags_mask)
>>                 return -EINVAL;
>>         /*disallow valid bit values that are not selected*/
>>         if (bf->value & ~nbf->selector)
>>                 return -EINVAL;
>>
>>         return 0;
>> }
>>
>> cheers,
>> jamal

^ permalink raw reply

* Re: [PATCH 0/3] net-next: stmmac: support future possible different internal phy mode
From: Andrew Lunn @ 2017-07-28 14:51 UTC (permalink / raw)
  To: Corentin Labbe
  Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8, wens-jdAy2FN1RRM,
	linux-I+IVW8TIWO2tmTQ+vhA3Yw, peppe.cavallaro-qxv4g6HH51o,
	alexandre.torgue-qxv4g6HH51o, devicetree-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	icenowy-h8G6r0blFSE
In-Reply-To: <20170728144424.GB7221@Red>

> It is too late when we know the PHY ID.
> We need to set a syscon for choosing external/internal PHY.
> So we can rely only on DT.

The point is, its not a property of the PHY. It is a syscon or a MAC
property. Having it as a MAC property would be more generic.

   Andrew

^ permalink raw reply

* Re: [v2] mwifiex: uninit wakeup info in the error handling
From: Kalle Valo @ 2017-07-28 14:50 UTC (permalink / raw)
  To: Jeffy Chen
  Cc: linux-wireless, akarwar, briannorris, Jeffy Chen, Xinming Hu,
	Ganapathi Bhat, Amitkumar Karwar, linux-kernel,
	Nishant Sarmukadam, netdev
In-Reply-To: <1499327728-25388-1-git-send-email-jeffy.chen@rock-chips.com>

Jeffy Chen <jeffy.chen@rock-chips.com> wrote:

> We inited wakeup info at the beginning of mwifiex_add_card, so we need
> to uninit it in the error handling.
> 
> It's much the same as what we did in:
> 36908c4 mwifiex: uninit wakeup info when removing device
> 
> Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
> Reviewed-by: Brian Norris <briannorris@chromium.org>

Patch applied to wireless-drivers-next.git, thanks.

f101d9649c42 mwifiex: uninit wakeup info in the error handling

-- 
https://patchwork.kernel.org/patch/9827589/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

^ permalink raw reply

* Re: mwifiex: fix compile warning of unused variable
From: Kalle Valo @ 2017-07-28 14:50 UTC (permalink / raw)
  To: Shawn Lin
  Cc: Amitkumar Karwar, linux-wireless, netdev, Nishant Sarmukadam,
	Xinming Hu, Shawn Lin
In-Reply-To: <1499327433-70786-1-git-send-email-shawn.lin@rock-chips.com>

Shawn Lin <shawn.lin@rock-chips.com> wrote:

> We got a compile warning shows below:
> 
> drivers/net/wireless/marvell/mwifiex/sdio.c: In function
> 'mwifiex_sdio_remove':
> drivers/net/wireless/marvell/mwifiex/sdio.c:377:6: warning: variable
> 'ret' set but not used [-Wunused-but-set-variable]
> 
> Per the code, it didn't check if mwifiex_sdio_read_fw_status
> finish successfully. We should at least check the return of
> mwifiex_sdio_read_fw_status, otherwise the following check of
> firmware_stat and adapter->mfg_mode is pointless as the device
> is probably dead.
> 
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> Reviewed-by: Brian Norris <briannorris@chromium.org>

Patch applied to wireless-drivers-next.git, thanks.

f46a5b0156b1 mwifiex: fix compile warning of unused variable

-- 
https://patchwork.kernel.org/patch/9827585/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

^ permalink raw reply

* [PATCH net] mcs7780: Silence uninitialized variable warning
From: Dan Carpenter @ 2017-07-28 14:45 UTC (permalink / raw)
  To: Samuel Ortiz; +Cc: netdev, kernel-janitors

My static checker complains that, if the allocation in mcs_get_reg()
fails, it means we use "rval" without initializing it.  Small
allocations never fail in current kernels so it's not a major concern
but it's simple enough to silence the warning.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/drivers/net/irda/mcs7780.c b/drivers/net/irda/mcs7780.c
index 765de3bedb88..64b29880e534 100644
--- a/drivers/net/irda/mcs7780.c
+++ b/drivers/net/irda/mcs7780.c
@@ -585,7 +585,7 @@ static int mcs_speed_change(struct mcs_cb *mcs)
 	int rst = 0;
 	int cnt = 0;
 	__u16 nspeed;
-	__u16 rval;
+	__u16 rval = -1;
 
 	nspeed = mcs_speed_set[(mcs->new_speed >> 8) & 0x0f];
 

^ permalink raw reply related

* Re: [PATCH 0/3] net-next: stmmac: support future possible different internal phy mode
From: Corentin Labbe @ 2017-07-28 14:44 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8, wens-jdAy2FN1RRM,
	linux-I+IVW8TIWO2tmTQ+vhA3Yw, peppe.cavallaro-qxv4g6HH51o,
	alexandre.torgue-qxv4g6HH51o, devicetree-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	icenowy-h8G6r0blFSE
In-Reply-To: <20170728143600.GB2132-g2DYL2Zd6BY@public.gmane.org>

On Fri, Jul 28, 2017 at 04:36:00PM +0200, Andrew Lunn wrote:
> > > I've probably asked this before: Does the internal PHY use a different
> > > PHY ID in registers 2 and 3?
> > > 
> > 
> > yes
> > 
> > reg2: 0x0044
> > reg3: 0X1500

Copy/paste error, its 1400

> 
> So this is not about loading the correct PHY driver. You can already
> do this based on the PHY IDs...
> 
> This is about selecting which PHY to use. Internal or External?	
> 
>      Andrew

It is too late when we know the PHY ID.
We need to set a syscon for choosing external/internal PHY.
So we can rely only on DT.

^ permalink raw reply

* [patch net-next 20/20] net: sched: cls_u32: no need to call tcf_exts_change for newly allocated struct
From: Jiri Pirko @ 2017-07-28 14:40 UTC (permalink / raw)
  To: netdev; +Cc: davem, jhs, xiyou.wangcong, daniel, mlxsw
In-Reply-To: <20170728144042.6380-1-jiri@resnulli.us>

From: Jiri Pirko <jiri@mellanox.com>

As the n struct was allocated right before u32_set_parms call,
no need to use tcf_exts_change to do atomic change, and we can just
fill-up the unused exts struct directly by tcf_exts_validate.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 net/sched/cls_u32.c | 18 +++++-------------
 1 file changed, 5 insertions(+), 13 deletions(-)

diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index 2c834f3..b8b0786 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -723,27 +723,24 @@ static int u32_set_parms(struct net *net, struct tcf_proto *tp,
 			 struct tc_u_knode *n, struct nlattr **tb,
 			 struct nlattr *est, bool ovr)
 {
-	struct tcf_exts e;
 	int err;
 
-	tcf_exts_init(&e, TCA_U32_ACT, TCA_U32_POLICE);
-	err = tcf_exts_validate(net, tp, tb, est, &e, ovr);
+	err = tcf_exts_validate(net, tp, tb, est, &n->exts, ovr);
 	if (err < 0)
-		goto errout;
+		return err;
 
-	err = -EINVAL;
 	if (tb[TCA_U32_LINK]) {
 		u32 handle = nla_get_u32(tb[TCA_U32_LINK]);
 		struct tc_u_hnode *ht_down = NULL, *ht_old;
 
 		if (TC_U32_KEY(handle))
-			goto errout;
+			return -EINVAL;
 
 		if (handle) {
 			ht_down = u32_lookup_ht(ht->tp_c, handle);
 
 			if (ht_down == NULL)
-				goto errout;
+				return -EINVAL;
 			ht_down->refcnt++;
 		}
 
@@ -763,16 +760,11 @@ static int u32_set_parms(struct net *net, struct tcf_proto *tp,
 		int ret;
 		ret = tcf_change_indev(net, tb[TCA_U32_INDEV]);
 		if (ret < 0)
-			goto errout;
+			return -EINVAL;
 		n->ifindex = ret;
 	}
 #endif
-	tcf_exts_change(&n->exts, &e);
-
 	return 0;
-errout:
-	tcf_exts_destroy(&e);
-	return err;
 }
 
 static void u32_replace_knode(struct tcf_proto *tp, struct tc_u_common *tp_c,
-- 
2.9.3

^ permalink raw reply related

* [patch net-next 19/20] net: sched: cls_route: no need to call tcf_exts_change for newly allocated struct
From: Jiri Pirko @ 2017-07-28 14:40 UTC (permalink / raw)
  To: netdev; +Cc: davem, jhs, xiyou.wangcong, daniel, mlxsw
In-Reply-To: <20170728144042.6380-1-jiri@resnulli.us>

From: Jiri Pirko <jiri@mellanox.com>

As the f struct was allocated right before route4_set_parms call,
no need to use tcf_exts_change to do atomic change, and we can just
fill-up the unused exts struct directly by tcf_exts_validate.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 net/sched/cls_route.c | 30 ++++++++++--------------------
 1 file changed, 10 insertions(+), 20 deletions(-)

diff --git a/net/sched/cls_route.c b/net/sched/cls_route.c
index 60be2c4..a3a09fc 100644
--- a/net/sched/cls_route.c
+++ b/net/sched/cls_route.c
@@ -372,35 +372,32 @@ static int route4_set_parms(struct net *net, struct tcf_proto *tp,
 	struct route4_filter *fp;
 	unsigned int h1;
 	struct route4_bucket *b;
-	struct tcf_exts e;
 	int err;
 
-	tcf_exts_init(&e, TCA_ROUTE4_ACT, TCA_ROUTE4_POLICE);
-	err = tcf_exts_validate(net, tp, tb, est, &e, ovr);
+	err = tcf_exts_validate(net, tp, tb, est, &f->exts, ovr);
 	if (err < 0)
-		goto errout;
+		return err;
 
-	err = -EINVAL;
 	if (tb[TCA_ROUTE4_TO]) {
 		if (new && handle & 0x8000)
-			goto errout;
+			return -EINVAL;
 		to = nla_get_u32(tb[TCA_ROUTE4_TO]);
 		if (to > 0xFF)
-			goto errout;
+			return -EINVAL;
 		nhandle = to;
 	}
 
 	if (tb[TCA_ROUTE4_FROM]) {
 		if (tb[TCA_ROUTE4_IIF])
-			goto errout;
+			return -EINVAL;
 		id = nla_get_u32(tb[TCA_ROUTE4_FROM]);
 		if (id > 0xFF)
-			goto errout;
+			return -EINVAL;
 		nhandle |= id << 16;
 	} else if (tb[TCA_ROUTE4_IIF]) {
 		id = nla_get_u32(tb[TCA_ROUTE4_IIF]);
 		if (id > 0x7FFF)
-			goto errout;
+			return -EINVAL;
 		nhandle |= (id | 0x8000) << 16;
 	} else
 		nhandle |= 0xFFFF << 16;
@@ -408,27 +405,25 @@ static int route4_set_parms(struct net *net, struct tcf_proto *tp,
 	if (handle && new) {
 		nhandle |= handle & 0x7F00;
 		if (nhandle != handle)
-			goto errout;
+			return -EINVAL;
 	}
 
 	h1 = to_hash(nhandle);
 	b = rtnl_dereference(head->table[h1]);
 	if (!b) {
-		err = -ENOBUFS;
 		b = kzalloc(sizeof(struct route4_bucket), GFP_KERNEL);
 		if (b == NULL)
-			goto errout;
+			return -ENOBUFS;
 
 		rcu_assign_pointer(head->table[h1], b);
 	} else {
 		unsigned int h2 = from_hash(nhandle >> 16);
 
-		err = -EEXIST;
 		for (fp = rtnl_dereference(b->ht[h2]);
 		     fp;
 		     fp = rtnl_dereference(fp->next))
 			if (fp->handle == f->handle)
-				goto errout;
+				return -EEXIST;
 	}
 
 	if (tb[TCA_ROUTE4_TO])
@@ -448,12 +443,7 @@ static int route4_set_parms(struct net *net, struct tcf_proto *tp,
 		tcf_bind_filter(tp, &f->res, base);
 	}
 
-	tcf_exts_change(&f->exts, &e);
-
 	return 0;
-errout:
-	tcf_exts_destroy(&e);
-	return err;
 }
 
 static int route4_change(struct net *net, struct sk_buff *in_skb,
-- 
2.9.3

^ permalink raw reply related

* [patch net-next 18/20] net: sched: cls_flow: no need to call tcf_exts_change for newly allocated struct
From: Jiri Pirko @ 2017-07-28 14:40 UTC (permalink / raw)
  To: netdev; +Cc: davem, jhs, xiyou.wangcong, daniel, mlxsw
In-Reply-To: <20170728144042.6380-1-jiri@resnulli.us>

From: Jiri Pirko <jiri@mellanox.com>

As the fnew struct just was allocated, so no need to use tcf_exts_change
to do atomic change, and we can just fill-up the unused exts struct
directly by tcf_exts_validate.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 net/sched/cls_flow.c | 36 ++++++++++++++----------------------
 1 file changed, 14 insertions(+), 22 deletions(-)

diff --git a/net/sched/cls_flow.c b/net/sched/cls_flow.c
index a2441c0..232f30b 100644
--- a/net/sched/cls_flow.c
+++ b/net/sched/cls_flow.c
@@ -388,7 +388,6 @@ static int flow_change(struct net *net, struct sk_buff *in_skb,
 	struct flow_filter *fold, *fnew;
 	struct nlattr *opt = tca[TCA_OPTIONS];
 	struct nlattr *tb[TCA_FLOW_MAX + 1];
-	struct tcf_exts e;
 	unsigned int nkeys = 0;
 	unsigned int perturb_period = 0;
 	u32 baseclass = 0;
@@ -424,27 +423,24 @@ static int flow_change(struct net *net, struct sk_buff *in_skb,
 			return -EOPNOTSUPP;
 	}
 
-	tcf_exts_init(&e, TCA_FLOW_ACT, TCA_FLOW_POLICE);
-	err = tcf_exts_validate(net, tp, tb, tca[TCA_RATE], &e, ovr);
-	if (err < 0)
-		goto err1;
-
-	err = -ENOBUFS;
 	fnew = kzalloc(sizeof(*fnew), GFP_KERNEL);
 	if (!fnew)
-		goto err1;
+		return -ENOBUFS;
 
 	err = tcf_em_tree_validate(tp, tb[TCA_FLOW_EMATCHES], &fnew->ematches);
 	if (err < 0)
-		goto err2;
+		goto err1;
 
 	tcf_exts_init(&fnew->exts, TCA_FLOW_ACT, TCA_FLOW_POLICE);
+	err = tcf_exts_validate(net, tp, tb, tca[TCA_RATE], &fnew->exts, ovr);
+	if (err < 0)
+		goto err2;
 
 	fold = (struct flow_filter *)*arg;
 	if (fold) {
 		err = -EINVAL;
 		if (fold->handle != handle && handle)
-			goto err3;
+			goto err2;
 
 		/* Copy fold into fnew */
 		fnew->tp = fold->tp;
@@ -464,31 +460,31 @@ static int flow_change(struct net *net, struct sk_buff *in_skb,
 		if (tb[TCA_FLOW_MODE])
 			mode = nla_get_u32(tb[TCA_FLOW_MODE]);
 		if (mode != FLOW_MODE_HASH && nkeys > 1)
-			goto err3;
+			goto err2;
 
 		if (mode == FLOW_MODE_HASH)
 			perturb_period = fold->perturb_period;
 		if (tb[TCA_FLOW_PERTURB]) {
 			if (mode != FLOW_MODE_HASH)
-				goto err3;
+				goto err2;
 			perturb_period = nla_get_u32(tb[TCA_FLOW_PERTURB]) * HZ;
 		}
 	} else {
 		err = -EINVAL;
 		if (!handle)
-			goto err3;
+			goto err2;
 		if (!tb[TCA_FLOW_KEYS])
-			goto err3;
+			goto err2;
 
 		mode = FLOW_MODE_MAP;
 		if (tb[TCA_FLOW_MODE])
 			mode = nla_get_u32(tb[TCA_FLOW_MODE]);
 		if (mode != FLOW_MODE_HASH && nkeys > 1)
-			goto err3;
+			goto err2;
 
 		if (tb[TCA_FLOW_PERTURB]) {
 			if (mode != FLOW_MODE_HASH)
-				goto err3;
+				goto err2;
 			perturb_period = nla_get_u32(tb[TCA_FLOW_PERTURB]) * HZ;
 		}
 
@@ -506,8 +502,6 @@ static int flow_change(struct net *net, struct sk_buff *in_skb,
 	setup_deferrable_timer(&fnew->perturb_timer, flow_perturbation,
 			       (unsigned long)fnew);
 
-	tcf_exts_change(&fnew->exts, &e);
-
 	netif_keep_dst(qdisc_dev(tp->q));
 
 	if (tb[TCA_FLOW_KEYS]) {
@@ -546,13 +540,11 @@ static int flow_change(struct net *net, struct sk_buff *in_skb,
 		call_rcu(&fold->rcu, flow_destroy_filter);
 	return 0;
 
-err3:
+err2:
 	tcf_exts_destroy(&fnew->exts);
 	tcf_em_tree_destroy(&fnew->ematches);
-err2:
-	kfree(fnew);
 err1:
-	tcf_exts_destroy(&e);
+	kfree(fnew);
 	return err;
 }
 
-- 
2.9.3

^ permalink raw reply related

* [PATCH net] tcp: avoid bogus gcc-7 array-bounds warning
From: Arnd Bergmann @ 2017-07-28 14:41 UTC (permalink / raw)
  To: David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI
  Cc: Arnd Bergmann, Eric Dumazet, Neal Cardwell, Soheil Hassas Yeganeh,
	Yuchung Cheng, netdev, linux-kernel

When using CONFIG_UBSAN_SANITIZE_ALL, the TCP code produces a
false-positive warning:

net/ipv4/tcp_output.c: In function 'tcp_connect':
net/ipv4/tcp_output.c:2207:40: error: array subscript is below array bounds [-Werror=array-bounds]
   tp->chrono_stat[tp->chrono_type - 1] += now - tp->chrono_start;
                                        ^~
net/ipv4/tcp_output.c:2207:40: error: array subscript is below array bounds [-Werror=array-bounds]
   tp->chrono_stat[tp->chrono_type - 1] += now - tp->chrono_start;
   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~

I have opened a gcc bug for this, but distros have already shipped
compilers with this problem, and it's not clear yet whether there is
a way for gcc to avoid the warning. As the problem is related to the
bitfield access, this introduces a temporary variable to store the old
enum value.

I did not notice this warning earlier, since UBSAN is disabled when
building with COMPILE_TEST, and that was always turned on in both
allmodconfig and randconfig tests.

Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81601
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 net/ipv4/tcp_output.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 886d874775df..bb901297a369 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -2202,9 +2202,10 @@ static bool tcp_small_queue_check(struct sock *sk, const struct sk_buff *skb,
 static void tcp_chrono_set(struct tcp_sock *tp, const enum tcp_chrono new)
 {
 	const u32 now = tcp_jiffies32;
+	enum tcp_chrono old = tp->chrono_type;
 
-	if (tp->chrono_type > TCP_CHRONO_UNSPEC)
-		tp->chrono_stat[tp->chrono_type - 1] += now - tp->chrono_start;
+	if (old > TCP_CHRONO_UNSPEC)
+		tp->chrono_stat[old - 1] += now - tp->chrono_start;
 	tp->chrono_start = now;
 	tp->chrono_type = new;
 }
-- 
2.9.0

^ permalink raw reply related

* [patch net-next 17/20] net: sched: cls_cgroup: no need to call tcf_exts_change for newly allocated struct
From: Jiri Pirko @ 2017-07-28 14:40 UTC (permalink / raw)
  To: netdev; +Cc: davem, jhs, xiyou.wangcong, daniel, mlxsw
In-Reply-To: <20170728144042.6380-1-jiri@resnulli.us>

From: Jiri Pirko <jiri@mellanox.com>

As the new struct just was allocated, so no need to use tcf_exts_change
to do atomic change, and we can just fill-up the unused exts struct
directly by tcf_exts_validate.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 net/sched/cls_cgroup.c | 14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/net/sched/cls_cgroup.c b/net/sched/cls_cgroup.c
index f6f302a..676855f 100644
--- a/net/sched/cls_cgroup.c
+++ b/net/sched/cls_cgroup.c
@@ -76,7 +76,6 @@ static int cls_cgroup_change(struct net *net, struct sk_buff *in_skb,
 	struct nlattr *tb[TCA_CGROUP_MAX + 1];
 	struct cls_cgroup_head *head = rtnl_dereference(tp->root);
 	struct cls_cgroup_head *new;
-	struct tcf_exts e;
 	int err;
 
 	if (!tca[TCA_OPTIONS])
@@ -100,20 +99,13 @@ static int cls_cgroup_change(struct net *net, struct sk_buff *in_skb,
 	if (err < 0)
 		goto errout;
 
-	tcf_exts_init(&e, TCA_CGROUP_ACT, TCA_CGROUP_POLICE);
-	err = tcf_exts_validate(net, tp, tb, tca[TCA_RATE], &e, ovr);
-	if (err < 0) {
-		tcf_exts_destroy(&e);
+	err = tcf_exts_validate(net, tp, tb, tca[TCA_RATE], &new->exts, ovr);
+	if (err < 0)
 		goto errout;
-	}
 
 	err = tcf_em_tree_validate(tp, tb[TCA_CGROUP_EMATCHES], &new->ematches);
-	if (err < 0) {
-		tcf_exts_destroy(&e);
+	if (err < 0)
 		goto errout;
-	}
-
-	tcf_exts_change(&new->exts, &e);
 
 	rcu_assign_pointer(tp->root, new);
 	if (head)
-- 
2.9.3

^ permalink raw reply related

* [patch net-next 16/20] net: sched: cls_bpf: no need to call tcf_exts_change for newly allocated struct
From: Jiri Pirko @ 2017-07-28 14:40 UTC (permalink / raw)
  To: netdev; +Cc: davem, jhs, xiyou.wangcong, daniel, mlxsw
In-Reply-To: <20170728144042.6380-1-jiri@resnulli.us>

From: Jiri Pirko <jiri@mellanox.com>

As the prog struct was allocated right before cls_bpf_set_parms call,
no need to use tcf_exts_change to do atomic change, and we can just
fill-up the unused exts struct directly by tcf_exts_validate.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 net/sched/cls_bpf.c | 25 +++++++------------------
 1 file changed, 7 insertions(+), 18 deletions(-)

diff --git a/net/sched/cls_bpf.c b/net/sched/cls_bpf.c
index 929cae9..d8bc3ec 100644
--- a/net/sched/cls_bpf.c
+++ b/net/sched/cls_bpf.c
@@ -387,7 +387,6 @@ static int cls_bpf_set_parms(struct net *net, struct tcf_proto *tp,
 			     struct nlattr **tb, struct nlattr *est, bool ovr)
 {
 	bool is_bpf, is_ebpf, have_exts = false;
-	struct tcf_exts exts;
 	u32 gen_flags = 0;
 	int ret;
 
@@ -396,28 +395,23 @@ static int cls_bpf_set_parms(struct net *net, struct tcf_proto *tp,
 	if ((!is_bpf && !is_ebpf) || (is_bpf && is_ebpf))
 		return -EINVAL;
 
-	tcf_exts_init(&exts, TCA_BPF_ACT, TCA_BPF_POLICE);
-	ret = tcf_exts_validate(net, tp, tb, est, &exts, ovr);
+	ret = tcf_exts_validate(net, tp, tb, est, &prog->exts, ovr);
 	if (ret < 0)
-		goto errout;
+		return ret;
 
 	if (tb[TCA_BPF_FLAGS]) {
 		u32 bpf_flags = nla_get_u32(tb[TCA_BPF_FLAGS]);
 
-		if (bpf_flags & ~TCA_BPF_FLAG_ACT_DIRECT) {
-			ret = -EINVAL;
-			goto errout;
-		}
+		if (bpf_flags & ~TCA_BPF_FLAG_ACT_DIRECT)
+			return -EINVAL;
 
 		have_exts = bpf_flags & TCA_BPF_FLAG_ACT_DIRECT;
 	}
 	if (tb[TCA_BPF_FLAGS_GEN]) {
 		gen_flags = nla_get_u32(tb[TCA_BPF_FLAGS_GEN]);
 		if (gen_flags & ~CLS_BPF_SUPPORTED_GEN_FLAGS ||
-		    !tc_flags_valid(gen_flags)) {
-			ret = -EINVAL;
-			goto errout;
-		}
+		    !tc_flags_valid(gen_flags))
+			return -EINVAL;
 	}
 
 	prog->exts_integrated = have_exts;
@@ -426,19 +420,14 @@ static int cls_bpf_set_parms(struct net *net, struct tcf_proto *tp,
 	ret = is_bpf ? cls_bpf_prog_from_ops(tb, prog) :
 		       cls_bpf_prog_from_efd(tb, prog, tp);
 	if (ret < 0)
-		goto errout;
+		return ret;
 
 	if (tb[TCA_BPF_CLASSID]) {
 		prog->res.classid = nla_get_u32(tb[TCA_BPF_CLASSID]);
 		tcf_bind_filter(tp, &prog->res, base);
 	}
 
-	tcf_exts_change(&prog->exts, &exts);
 	return 0;
-
-errout:
-	tcf_exts_destroy(&exts);
-	return ret;
 }
 
 static u32 cls_bpf_grab_new_handle(struct tcf_proto *tp,
-- 
2.9.3

^ permalink raw reply related

* [patch net-next 15/20] net: sched: cls_basic: no need to call tcf_exts_change for newly allocated struct
From: Jiri Pirko @ 2017-07-28 14:40 UTC (permalink / raw)
  To: netdev; +Cc: davem, jhs, xiyou.wangcong, daniel, mlxsw
In-Reply-To: <20170728144042.6380-1-jiri@resnulli.us>

From: Jiri Pirko <jiri@mellanox.com>

As the f struct was allocated right before basic_set_parms call, no need
to use tcf_exts_change to do atomic change, and we can just fill-up
the unused exts struct directly by tcf_exts_validate.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 net/sched/cls_basic.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/net/sched/cls_basic.c b/net/sched/cls_basic.c
index dc12d79..0a35cb5 100644
--- a/net/sched/cls_basic.c
+++ b/net/sched/cls_basic.c
@@ -129,29 +129,22 @@ static int basic_set_parms(struct net *net, struct tcf_proto *tp,
 			   struct nlattr *est, bool ovr)
 {
 	int err;
-	struct tcf_exts e;
 
-	tcf_exts_init(&e, TCA_BASIC_ACT, TCA_BASIC_POLICE);
-	err = tcf_exts_validate(net, tp, tb, est, &e, ovr);
+	err = tcf_exts_validate(net, tp, tb, est, &f->exts, ovr);
 	if (err < 0)
-		goto errout;
+		return err;
 
 	err = tcf_em_tree_validate(tp, tb[TCA_BASIC_EMATCHES], &f->ematches);
 	if (err < 0)
-		goto errout;
+		return err;
 
 	if (tb[TCA_BASIC_CLASSID]) {
 		f->res.classid = nla_get_u32(tb[TCA_BASIC_CLASSID]);
 		tcf_bind_filter(tp, &f->res, base);
 	}
 
-	tcf_exts_change(&f->exts, &e);
 	f->tp = tp;
-
 	return 0;
-errout:
-	tcf_exts_destroy(&e);
-	return err;
 }
 
 static int basic_change(struct net *net, struct sk_buff *in_skb,
-- 
2.9.3

^ permalink raw reply related

* [patch net-next 14/20] net: sched: cls_matchall: no need to call tcf_exts_change for newly allocated struct
From: Jiri Pirko @ 2017-07-28 14:40 UTC (permalink / raw)
  To: netdev; +Cc: davem, jhs, xiyou.wangcong, daniel, mlxsw
In-Reply-To: <20170728144042.6380-1-jiri@resnulli.us>

From: Jiri Pirko <jiri@mellanox.com>

As the head struct was allocated right before mall_set_parms call,
no need to use tcf_exts_change to do atomic change, and we can just
fill-up the unused exts struct directly by tcf_exts_validate.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 net/sched/cls_matchall.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/net/sched/cls_matchall.c b/net/sched/cls_matchall.c
index 0b77f6e..94181e5 100644
--- a/net/sched/cls_matchall.c
+++ b/net/sched/cls_matchall.c
@@ -120,25 +120,17 @@ static int mall_set_parms(struct net *net, struct tcf_proto *tp,
 			  unsigned long base, struct nlattr **tb,
 			  struct nlattr *est, bool ovr)
 {
-	struct tcf_exts e;
 	int err;
 
-	tcf_exts_init(&e, TCA_MATCHALL_ACT, 0);
-	err = tcf_exts_validate(net, tp, tb, est, &e, ovr);
+	err = tcf_exts_validate(net, tp, tb, est, &head->exts, ovr);
 	if (err < 0)
-		goto errout;
+		return err;
 
 	if (tb[TCA_MATCHALL_CLASSID]) {
 		head->res.classid = nla_get_u32(tb[TCA_MATCHALL_CLASSID]);
 		tcf_bind_filter(tp, &head->res, base);
 	}
-
-	tcf_exts_change(&head->exts, &e);
-
 	return 0;
-errout:
-	tcf_exts_destroy(&e);
-	return err;
 }
 
 static int mall_change(struct net *net, struct sk_buff *in_skb,
-- 
2.9.3

^ permalink raw reply related

* [patch net-next 13/20] net: sched: cls_fw: no need to call tcf_exts_change for newly allocated struct
From: Jiri Pirko @ 2017-07-28 14:40 UTC (permalink / raw)
  To: netdev; +Cc: davem, jhs, xiyou.wangcong, daniel, mlxsw
In-Reply-To: <20170728144042.6380-1-jiri@resnulli.us>

From: Jiri Pirko <jiri@mellanox.com>

As the f struct was allocated right before fw_set_parms call, no need
to use tcf_exts_change to do atomic change, and we can just fill-up
the unused exts struct directly by tcf_exts_validate.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 net/sched/cls_fw.c | 21 ++++++---------------
 1 file changed, 6 insertions(+), 15 deletions(-)

diff --git a/net/sched/cls_fw.c b/net/sched/cls_fw.c
index a30a098..d8c084a 100644
--- a/net/sched/cls_fw.c
+++ b/net/sched/cls_fw.c
@@ -195,14 +195,12 @@ static int fw_set_parms(struct net *net, struct tcf_proto *tp,
 			struct nlattr **tca, unsigned long base, bool ovr)
 {
 	struct fw_head *head = rtnl_dereference(tp->root);
-	struct tcf_exts e;
 	u32 mask;
 	int err;
 
-	tcf_exts_init(&e, TCA_FW_ACT, TCA_FW_POLICE);
-	err = tcf_exts_validate(net, tp, tb, tca[TCA_RATE], &e, ovr);
+	err = tcf_exts_validate(net, tp, tb, tca[TCA_RATE], &f->exts, ovr);
 	if (err < 0)
-		goto errout;
+		return err;
 
 	if (tb[TCA_FW_CLASSID]) {
 		f->res.classid = nla_get_u32(tb[TCA_FW_CLASSID]);
@@ -213,10 +211,8 @@ static int fw_set_parms(struct net *net, struct tcf_proto *tp,
 	if (tb[TCA_FW_INDEV]) {
 		int ret;
 		ret = tcf_change_indev(net, tb[TCA_FW_INDEV]);
-		if (ret < 0) {
-			err = ret;
-			goto errout;
-		}
+		if (ret < 0)
+			return ret;
 		f->ifindex = ret;
 	}
 #endif /* CONFIG_NET_CLS_IND */
@@ -225,16 +221,11 @@ static int fw_set_parms(struct net *net, struct tcf_proto *tp,
 	if (tb[TCA_FW_MASK]) {
 		mask = nla_get_u32(tb[TCA_FW_MASK]);
 		if (mask != head->mask)
-			goto errout;
+			return err;
 	} else if (head->mask != 0xFFFFFFFF)
-		goto errout;
-
-	tcf_exts_change(&f->exts, &e);
+		return err;
 
 	return 0;
-errout:
-	tcf_exts_destroy(&e);
-	return err;
 }
 
 static int fw_change(struct net *net, struct sk_buff *in_skb,
-- 
2.9.3

^ permalink raw reply related

* [patch net-next 12/20] net: sched: cls_flower: no need to call tcf_exts_change for newly allocated struct
From: Jiri Pirko @ 2017-07-28 14:40 UTC (permalink / raw)
  To: netdev; +Cc: davem, jhs, xiyou.wangcong, daniel, mlxsw
In-Reply-To: <20170728144042.6380-1-jiri@resnulli.us>

From: Jiri Pirko <jiri@mellanox.com>

As the f struct was allocated right before fl_set_parms call, no need
to use tcf_exts_change to do atomic change, and we can just fill-up
the unused exts struct directly by tcf_exts_validate.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 net/sched/cls_flower.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index 4bba357..32cbbb3 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -852,13 +852,11 @@ static int fl_set_parms(struct net *net, struct tcf_proto *tp,
 			unsigned long base, struct nlattr **tb,
 			struct nlattr *est, bool ovr)
 {
-	struct tcf_exts e;
 	int err;
 
-	tcf_exts_init(&e, TCA_FLOWER_ACT, 0);
-	err = tcf_exts_validate(net, tp, tb, est, &e, ovr);
+	err = tcf_exts_validate(net, tp, tb, est, &f->exts, ovr);
 	if (err < 0)
-		goto errout;
+		return err;
 
 	if (tb[TCA_FLOWER_CLASSID]) {
 		f->res.classid = nla_get_u32(tb[TCA_FLOWER_CLASSID]);
@@ -867,17 +865,12 @@ static int fl_set_parms(struct net *net, struct tcf_proto *tp,
 
 	err = fl_set_key(net, tb, &f->key, &mask->key);
 	if (err)
-		goto errout;
+		return err;
 
 	fl_mask_update_range(mask);
 	fl_set_masked_key(&f->mkey, &f->key, mask);
 
-	tcf_exts_change(&f->exts, &e);
-
 	return 0;
-errout:
-	tcf_exts_destroy(&e);
-	return err;
 }
 
 static u32 fl_grab_new_handle(struct tcf_proto *tp,
-- 
2.9.3

^ permalink raw reply related

* [patch net-next 11/20] net: sched: cls_fw: rename fw_change_attrs function
From: Jiri Pirko @ 2017-07-28 14:40 UTC (permalink / raw)
  To: netdev; +Cc: davem, jhs, xiyou.wangcong, daniel, mlxsw
In-Reply-To: <20170728144042.6380-1-jiri@resnulli.us>

From: Jiri Pirko <jiri@mellanox.com>

Since the function name is misleading since it is not changing
anything, name it similarly to other cls.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 net/sched/cls_fw.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/net/sched/cls_fw.c b/net/sched/cls_fw.c
index 46a0e32..a30a098 100644
--- a/net/sched/cls_fw.c
+++ b/net/sched/cls_fw.c
@@ -190,10 +190,9 @@ static const struct nla_policy fw_policy[TCA_FW_MAX + 1] = {
 	[TCA_FW_MASK]		= { .type = NLA_U32 },
 };
 
-static int
-fw_change_attrs(struct net *net, struct tcf_proto *tp, struct fw_filter *f,
-		struct nlattr **tb, struct nlattr **tca, unsigned long base,
-		bool ovr)
+static int fw_set_parms(struct net *net, struct tcf_proto *tp,
+			struct fw_filter *f, struct nlattr **tb,
+			struct nlattr **tca, unsigned long base, bool ovr)
 {
 	struct fw_head *head = rtnl_dereference(tp->root);
 	struct tcf_exts e;
@@ -276,7 +275,7 @@ static int fw_change(struct net *net, struct sk_buff *in_skb,
 
 		tcf_exts_init(&fnew->exts, TCA_FW_ACT, TCA_FW_POLICE);
 
-		err = fw_change_attrs(net, tp, fnew, tb, tca, base, ovr);
+		err = fw_set_parms(net, tp, fnew, tb, tca, base, ovr);
 		if (err < 0) {
 			tcf_exts_destroy(&fnew->exts);
 			kfree(fnew);
@@ -322,7 +321,7 @@ static int fw_change(struct net *net, struct sk_buff *in_skb,
 	f->id = handle;
 	f->tp = tp;
 
-	err = fw_change_attrs(net, tp, f, tb, tca, base, ovr);
+	err = fw_set_parms(net, tp, f, tb, tca, base, ovr);
 	if (err < 0)
 		goto errout;
 
-- 
2.9.3

^ permalink raw reply related

* [patch net-next 10/20] net: sched: cls_bpf: rename cls_bpf_modify_existing function
From: Jiri Pirko @ 2017-07-28 14:40 UTC (permalink / raw)
  To: netdev; +Cc: davem, jhs, xiyou.wangcong, daniel, mlxsw
In-Reply-To: <20170728144042.6380-1-jiri@resnulli.us>

From: Jiri Pirko <jiri@mellanox.com>

The name cls_bpf_modify_existing is highly misleading, as it indeed does
not modify anything existing. It does not modify at all.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 net/sched/cls_bpf.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/net/sched/cls_bpf.c b/net/sched/cls_bpf.c
index 49f311a..929cae9 100644
--- a/net/sched/cls_bpf.c
+++ b/net/sched/cls_bpf.c
@@ -382,10 +382,9 @@ static int cls_bpf_prog_from_efd(struct nlattr **tb, struct cls_bpf_prog *prog,
 	return 0;
 }
 
-static int cls_bpf_modify_existing(struct net *net, struct tcf_proto *tp,
-				   struct cls_bpf_prog *prog,
-				   unsigned long base, struct nlattr **tb,
-				   struct nlattr *est, bool ovr)
+static int cls_bpf_set_parms(struct net *net, struct tcf_proto *tp,
+			     struct cls_bpf_prog *prog, unsigned long base,
+			     struct nlattr **tb, struct nlattr *est, bool ovr)
 {
 	bool is_bpf, is_ebpf, have_exts = false;
 	struct tcf_exts exts;
@@ -504,8 +503,7 @@ static int cls_bpf_change(struct net *net, struct sk_buff *in_skb,
 		goto errout;
 	}
 
-	ret = cls_bpf_modify_existing(net, tp, prog, base, tb, tca[TCA_RATE],
-				      ovr);
+	ret = cls_bpf_set_parms(net, tp, prog, base, tb, tca[TCA_RATE], ovr);
 	if (ret < 0)
 		goto errout;
 
-- 
2.9.3

^ permalink raw reply related

* [patch net-next 09/20] net: sched: convert actions array into rcu list
From: Jiri Pirko @ 2017-07-28 14:40 UTC (permalink / raw)
  To: netdev; +Cc: davem, jhs, xiyou.wangcong, daniel, mlxsw
In-Reply-To: <20170728144042.6380-1-jiri@resnulli.us>

From: Jiri Pirko <jiri@mellanox.com>

Currently the actions are stored in array with array size. To traverse
this array in fastpath, tcf_tree_lock is taken to protect it. Convert
the array into a singly linked list, similar to the filter chains style
and allow traversal protected by rcu.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 include/net/act_api.h    | 11 +++++++---
 include/net/pkt_cls.h    | 46 ++++++++++++++++++-----------------------
 net/sched/act_api.c      | 16 ++++++++-------
 net/sched/cls_api.c      | 31 ++++++++++++----------------
 net/sched/cls_basic.c    | 10 +++------
 net/sched/cls_bpf.c      | 10 +++------
 net/sched/cls_cgroup.c   | 10 +++------
 net/sched/cls_flow.c     | 10 +++------
 net/sched/cls_flower.c   | 10 +++------
 net/sched/cls_fw.c       | 16 ++++-----------
 net/sched/cls_matchall.c | 11 +++-------
 net/sched/cls_route.c    | 10 +++------
 net/sched/cls_rsvp.h     | 18 +++++-----------
 net/sched/cls_tcindex.c  | 53 ++++++++++++++----------------------------------
 net/sched/cls_u32.c      | 15 ++++----------
 15 files changed, 99 insertions(+), 178 deletions(-)

diff --git a/include/net/act_api.h b/include/net/act_api.h
index 26ffd83..74e2657 100644
--- a/include/net/act_api.h
+++ b/include/net/act_api.h
@@ -21,6 +21,8 @@ struct tcf_hashinfo {
 struct tc_action_ops;
 
 struct tc_action {
+	/* Fast access part */
+	struct tc_action __rcu		*next;
 	const struct tc_action_ops	*ops;
 	__u32				type; /* for backward compat(TCA_OLD_COMPAT) */
 	__u32				order;
@@ -179,15 +181,18 @@ int tcf_register_action(struct tc_action_ops *a, struct pernet_operations *ops);
 int tcf_unregister_action(struct tc_action_ops *a,
 			  struct pernet_operations *ops);
 int tcf_action_destroy(struct list_head *actions, int bind);
-int tcf_action_exec(struct sk_buff *skb, struct tc_action **actions,
-		    int nr_actions, struct tcf_result *res);
+
+struct tcf_exts;
+
+int tcf_action_exec(struct sk_buff *skb, struct tcf_exts *exts,
+		    struct tcf_result *res);
 int tcf_action_init(struct net *net, struct tcf_proto *tp, struct nlattr *nla,
 		    struct nlattr *est, char *name, int ovr, int bind,
 		    struct list_head *actions);
 struct tc_action *tcf_action_init_1(struct net *net, struct tcf_proto *tp,
 				    struct nlattr *nla, struct nlattr *est,
 				    char *name, int ovr, int bind);
-int tcf_action_dump(struct sk_buff *skb, struct list_head *, int, int);
+int tcf_action_dump(struct sk_buff *skb, struct list_head *actions, int, int);
 int tcf_action_dump_old(struct sk_buff *skb, struct tc_action *a, int, int);
 int tcf_action_dump_1(struct sk_buff *skb, struct tc_action *a, int, int);
 int tcf_action_copy_stats(struct sk_buff *, struct tc_action *, int);
diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index b8959c9..8257a4e 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -88,8 +88,7 @@ tcf_unbind_filter(struct tcf_proto *tp, struct tcf_result *r)
 struct tcf_exts {
 #ifdef CONFIG_NET_CLS_ACT
 	__u32	type; /* for backward compat(TCA_OLD_COMPAT) */
-	int nr_actions;
-	struct tc_action **actions;
+	struct tc_action __rcu *action_list;
 #endif
 	/* Map to export classifier specific extension TLV types to the
 	 * generic extensions API. Unsupported extensions must be set to 0.
@@ -98,32 +97,32 @@ struct tcf_exts {
 	int police;
 };
 
-static inline int tcf_exts_init(struct tcf_exts *exts, int action, int police)
+#define tcf_exts_for_each(a, exts)				\
+	for (a = rtnl_dereference(exts->action_list);		\
+	     a; a = rtnl_dereference(a->next))
+
+#define tcf_exts_for_each_rcu_bh(a, exts)			\
+	for (a = rcu_dereference_bh(exts->action_list);		\
+	     a; a = rcu_dereference_bh(a->next))
+
+static inline void tcf_exts_init(struct tcf_exts *exts, int action, int police)
 {
 #ifdef CONFIG_NET_CLS_ACT
 	exts->type = 0;
-	exts->nr_actions = 0;
-	exts->actions = kcalloc(TCA_ACT_MAX_PRIO, sizeof(struct tc_action *),
-				GFP_KERNEL);
-	if (!exts->actions)
-		return -ENOMEM;
+	RCU_INIT_POINTER(exts->action_list, NULL);
 #endif
 	exts->action = action;
 	exts->police = police;
-	return 0;
 }
 
 static inline void tcf_exts_to_list(const struct tcf_exts *exts,
 				    struct list_head *actions)
 {
 #ifdef CONFIG_NET_CLS_ACT
-	int i;
-
-	for (i = 0; i < exts->nr_actions; i++) {
-		struct tc_action *a = exts->actions[i];
+	struct tc_action *a;
 
+	tcf_exts_for_each(a, exts)
 		list_add_tail(&a->list, actions);
-	}
 #endif
 }
 
@@ -132,16 +131,11 @@ tcf_exts_stats_update(const struct tcf_exts *exts,
 		      u64 bytes, u64 packets, u64 lastuse)
 {
 #ifdef CONFIG_NET_CLS_ACT
-	int i;
+	struct tc_action *a;
 
 	preempt_disable();
-
-	for (i = 0; i < exts->nr_actions; i++) {
-		struct tc_action *a = exts->actions[i];
-
+	tcf_exts_for_each(a, exts)
 		tcf_action_stats_update(a, bytes, packets, lastuse);
-	}
-
 	preempt_enable();
 #endif
 }
@@ -155,7 +149,7 @@ tcf_exts_stats_update(const struct tcf_exts *exts,
 static inline bool tcf_exts_has_actions(struct tcf_exts *exts)
 {
 #ifdef CONFIG_NET_CLS_ACT
-	return exts->nr_actions;
+	return rtnl_dereference(exts->action_list);
 #else
 	return false;
 #endif
@@ -170,7 +164,8 @@ static inline bool tcf_exts_has_actions(struct tcf_exts *exts)
 static inline bool tcf_exts_has_one_action(struct tcf_exts *exts)
 {
 #ifdef CONFIG_NET_CLS_ACT
-	return exts->nr_actions == 1;
+	return rtnl_dereference(exts->action_list) &&
+	       !rtnl_dereference(exts->action_list->next);
 #else
 	return false;
 #endif
@@ -192,7 +187,7 @@ tcf_exts_exec(struct sk_buff *skb, struct tcf_exts *exts,
 	      struct tcf_result *res)
 {
 #ifdef CONFIG_NET_CLS_ACT
-	return tcf_action_exec(skb, exts->actions, exts->nr_actions, res);
+	return tcf_action_exec(skb, exts, res);
 #endif
 	return TC_ACT_OK;
 }
@@ -201,8 +196,7 @@ int tcf_exts_validate(struct net *net, struct tcf_proto *tp,
 		      struct nlattr **tb, struct nlattr *rate_tlv,
 		      struct tcf_exts *exts, bool ovr);
 void tcf_exts_destroy(struct tcf_exts *exts);
-void tcf_exts_change(struct tcf_proto *tp, struct tcf_exts *dst,
-		     struct tcf_exts *src);
+void tcf_exts_change(struct tcf_exts *dst, struct tcf_exts *src);
 int tcf_exts_dump(struct sk_buff *skb, struct tcf_exts *exts);
 int tcf_exts_dump_stats(struct sk_buff *skb, struct tcf_exts *exts);
 int tcf_exts_get_dev(struct net_device *dev, struct tcf_exts *exts,
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index 8d2e506..177b9ab 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -457,21 +457,19 @@ static struct tc_action_ops *tc_lookup_action(struct nlattr *kind)
 
 /*TCA_ACT_MAX_PRIO is 32, there count upto 32 */
 #define TCA_ACT_MAX_PRIO_MASK 0x1FF
-int tcf_action_exec(struct sk_buff *skb, struct tc_action **actions,
-		    int nr_actions, struct tcf_result *res)
+int tcf_action_exec(struct sk_buff *skb, struct tcf_exts *exts,
+		    struct tcf_result *res)
 {
 	u32 jmp_prgcnt = 0;
 	u32 jmp_ttl = TCA_ACT_MAX_PRIO; /*matches actions per filter */
-	int i;
+	struct tc_action *a;
 	int ret = TC_ACT_OK;
 
 	if (skb_skip_tc_classify(skb))
 		return TC_ACT_OK;
 
 restart_act_graph:
-	for (i = 0; i < nr_actions; i++) {
-		const struct tc_action *a = actions[i];
-
+	tcf_exts_for_each_rcu_bh(a, exts) {
 		if (jmp_prgcnt > 0) {
 			jmp_prgcnt -= 1;
 			continue;
@@ -483,7 +481,7 @@ int tcf_action_exec(struct sk_buff *skb, struct tc_action **actions,
 
 		if (TC_ACT_EXT_CMP(ret, TC_ACT_JUMP)) {
 			jmp_prgcnt = ret & TCA_ACT_MAX_PRIO_MASK;
-			if (!jmp_prgcnt || (jmp_prgcnt > nr_actions)) {
+			if (!jmp_prgcnt) {
 				/* faulty opcode, stop pipeline */
 				return TC_ACT_OK;
 			} else {
@@ -501,6 +499,10 @@ int tcf_action_exec(struct sk_buff *skb, struct tc_action **actions,
 			break;
 	}
 
+	if (jmp_prgcnt > 0)
+		/* faulty opcode, stop pipeline */
+		return TC_ACT_OK;
+
 	return ret;
 }
 EXPORT_SYMBOL(tcf_action_exec);
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 735d556..129f314 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -836,8 +836,6 @@ void tcf_exts_destroy(struct tcf_exts *exts)
 
 	tcf_exts_to_list(exts, &actions);
 	tcf_action_destroy(&actions, TCA_ACT_UNBIND);
-	kfree(exts->actions);
-	exts->nr_actions = 0;
 #endif
 }
 EXPORT_SYMBOL(tcf_exts_destroy);
@@ -857,20 +855,22 @@ int tcf_exts_validate(struct net *net, struct tcf_proto *tp, struct nlattr **tb,
 				return PTR_ERR(act);
 
 			act->type = exts->type = TCA_OLD_COMPAT;
-			exts->actions[0] = act;
-			exts->nr_actions = 1;
+			exts->action_list = act;
 		} else if (exts->action && tb[exts->action]) {
+			struct tc_action **pprev;
 			LIST_HEAD(actions);
-			int err, i = 0;
+			int err;
 
 			err = tcf_action_init(net, tp, tb[exts->action],
 					      rate_tlv, NULL, ovr, TCA_ACT_BIND,
 					      &actions);
 			if (err)
 				return err;
-			list_for_each_entry(act, &actions, list)
-				exts->actions[i++] = act;
-			exts->nr_actions = i;
+			pprev = &exts->action_list;
+			list_for_each_entry(act, &actions, list) {
+				*pprev = act;
+				pprev = &act->next;
+			}
 		}
 	}
 #else
@@ -883,18 +883,16 @@ int tcf_exts_validate(struct net *net, struct tcf_proto *tp, struct nlattr **tb,
 }
 EXPORT_SYMBOL(tcf_exts_validate);
 
-void tcf_exts_change(struct tcf_proto *tp, struct tcf_exts *dst,
-		     struct tcf_exts *src)
+void tcf_exts_change(struct tcf_exts *dst, struct tcf_exts *src)
 {
 #ifdef CONFIG_NET_CLS_ACT
 	struct tcf_exts old = *dst;
 
-	tcf_tree_lock(tp);
-	dst->nr_actions = src->nr_actions;
-	dst->actions = src->actions;
+	rcu_assign_pointer(dst->action_list,
+			   rtnl_dereference(src->action_list));
 	dst->type = src->type;
-	tcf_tree_unlock(tp);
 
+	synchronize_rcu();
 	tcf_exts_destroy(&old);
 #endif
 }
@@ -903,10 +901,7 @@ EXPORT_SYMBOL(tcf_exts_change);
 #ifdef CONFIG_NET_CLS_ACT
 static struct tc_action *tcf_exts_first_act(struct tcf_exts *exts)
 {
-	if (exts->nr_actions == 0)
-		return NULL;
-	else
-		return exts->actions[0];
+	return exts->action_list;
 }
 #endif
 
diff --git a/net/sched/cls_basic.c b/net/sched/cls_basic.c
index 979cd26..dc12d79 100644
--- a/net/sched/cls_basic.c
+++ b/net/sched/cls_basic.c
@@ -131,9 +131,7 @@ static int basic_set_parms(struct net *net, struct tcf_proto *tp,
 	int err;
 	struct tcf_exts e;
 
-	err = tcf_exts_init(&e, TCA_BASIC_ACT, TCA_BASIC_POLICE);
-	if (err < 0)
-		return err;
+	tcf_exts_init(&e, TCA_BASIC_ACT, TCA_BASIC_POLICE);
 	err = tcf_exts_validate(net, tp, tb, est, &e, ovr);
 	if (err < 0)
 		goto errout;
@@ -147,7 +145,7 @@ static int basic_set_parms(struct net *net, struct tcf_proto *tp,
 		tcf_bind_filter(tp, &f->res, base);
 	}
 
-	tcf_exts_change(tp, &f->exts, &e);
+	tcf_exts_change(&f->exts, &e);
 	f->tp = tp;
 
 	return 0;
@@ -183,9 +181,7 @@ static int basic_change(struct net *net, struct sk_buff *in_skb,
 	if (!fnew)
 		return -ENOBUFS;
 
-	err = tcf_exts_init(&fnew->exts, TCA_BASIC_ACT, TCA_BASIC_POLICE);
-	if (err < 0)
-		goto errout;
+	tcf_exts_init(&fnew->exts, TCA_BASIC_ACT, TCA_BASIC_POLICE);
 
 	err = -EINVAL;
 	if (handle) {
diff --git a/net/sched/cls_bpf.c b/net/sched/cls_bpf.c
index f57bd53..49f311a 100644
--- a/net/sched/cls_bpf.c
+++ b/net/sched/cls_bpf.c
@@ -397,9 +397,7 @@ static int cls_bpf_modify_existing(struct net *net, struct tcf_proto *tp,
 	if ((!is_bpf && !is_ebpf) || (is_bpf && is_ebpf))
 		return -EINVAL;
 
-	ret = tcf_exts_init(&exts, TCA_BPF_ACT, TCA_BPF_POLICE);
-	if (ret < 0)
-		return ret;
+	tcf_exts_init(&exts, TCA_BPF_ACT, TCA_BPF_POLICE);
 	ret = tcf_exts_validate(net, tp, tb, est, &exts, ovr);
 	if (ret < 0)
 		goto errout;
@@ -436,7 +434,7 @@ static int cls_bpf_modify_existing(struct net *net, struct tcf_proto *tp,
 		tcf_bind_filter(tp, &prog->res, base);
 	}
 
-	tcf_exts_change(tp, &prog->exts, &exts);
+	tcf_exts_change(&prog->exts, &exts);
 	return 0;
 
 errout:
@@ -488,9 +486,7 @@ static int cls_bpf_change(struct net *net, struct sk_buff *in_skb,
 	if (!prog)
 		return -ENOBUFS;
 
-	ret = tcf_exts_init(&prog->exts, TCA_BPF_ACT, TCA_BPF_POLICE);
-	if (ret < 0)
-		goto errout;
+	tcf_exts_init(&prog->exts, TCA_BPF_ACT, TCA_BPF_POLICE);
 
 	if (oldprog) {
 		if (handle && oldprog->handle != handle) {
diff --git a/net/sched/cls_cgroup.c b/net/sched/cls_cgroup.c
index ce7d38b..f6f302a 100644
--- a/net/sched/cls_cgroup.c
+++ b/net/sched/cls_cgroup.c
@@ -92,9 +92,7 @@ static int cls_cgroup_change(struct net *net, struct sk_buff *in_skb,
 	if (!new)
 		return -ENOBUFS;
 
-	err = tcf_exts_init(&new->exts, TCA_CGROUP_ACT, TCA_CGROUP_POLICE);
-	if (err < 0)
-		goto errout;
+	tcf_exts_init(&new->exts, TCA_CGROUP_ACT, TCA_CGROUP_POLICE);
 	new->handle = handle;
 	new->tp = tp;
 	err = nla_parse_nested(tb, TCA_CGROUP_MAX, tca[TCA_OPTIONS],
@@ -102,9 +100,7 @@ static int cls_cgroup_change(struct net *net, struct sk_buff *in_skb,
 	if (err < 0)
 		goto errout;
 
-	err = tcf_exts_init(&e, TCA_CGROUP_ACT, TCA_CGROUP_POLICE);
-	if (err < 0)
-		goto errout;
+	tcf_exts_init(&e, TCA_CGROUP_ACT, TCA_CGROUP_POLICE);
 	err = tcf_exts_validate(net, tp, tb, tca[TCA_RATE], &e, ovr);
 	if (err < 0) {
 		tcf_exts_destroy(&e);
@@ -117,7 +113,7 @@ static int cls_cgroup_change(struct net *net, struct sk_buff *in_skb,
 		goto errout;
 	}
 
-	tcf_exts_change(tp, &new->exts, &e);
+	tcf_exts_change(&new->exts, &e);
 
 	rcu_assign_pointer(tp->root, new);
 	if (head)
diff --git a/net/sched/cls_flow.c b/net/sched/cls_flow.c
index 71fd1af..a2441c0 100644
--- a/net/sched/cls_flow.c
+++ b/net/sched/cls_flow.c
@@ -424,9 +424,7 @@ static int flow_change(struct net *net, struct sk_buff *in_skb,
 			return -EOPNOTSUPP;
 	}
 
-	err = tcf_exts_init(&e, TCA_FLOW_ACT, TCA_FLOW_POLICE);
-	if (err < 0)
-		goto err1;
+	tcf_exts_init(&e, TCA_FLOW_ACT, TCA_FLOW_POLICE);
 	err = tcf_exts_validate(net, tp, tb, tca[TCA_RATE], &e, ovr);
 	if (err < 0)
 		goto err1;
@@ -440,9 +438,7 @@ static int flow_change(struct net *net, struct sk_buff *in_skb,
 	if (err < 0)
 		goto err2;
 
-	err = tcf_exts_init(&fnew->exts, TCA_FLOW_ACT, TCA_FLOW_POLICE);
-	if (err < 0)
-		goto err3;
+	tcf_exts_init(&fnew->exts, TCA_FLOW_ACT, TCA_FLOW_POLICE);
 
 	fold = (struct flow_filter *)*arg;
 	if (fold) {
@@ -510,7 +506,7 @@ static int flow_change(struct net *net, struct sk_buff *in_skb,
 	setup_deferrable_timer(&fnew->perturb_timer, flow_perturbation,
 			       (unsigned long)fnew);
 
-	tcf_exts_change(tp, &fnew->exts, &e);
+	tcf_exts_change(&fnew->exts, &e);
 
 	netif_keep_dst(qdisc_dev(tp->q));
 
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index 7832eb9..4bba357 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -855,9 +855,7 @@ static int fl_set_parms(struct net *net, struct tcf_proto *tp,
 	struct tcf_exts e;
 	int err;
 
-	err = tcf_exts_init(&e, TCA_FLOWER_ACT, 0);
-	if (err < 0)
-		return err;
+	tcf_exts_init(&e, TCA_FLOWER_ACT, 0);
 	err = tcf_exts_validate(net, tp, tb, est, &e, ovr);
 	if (err < 0)
 		goto errout;
@@ -874,7 +872,7 @@ static int fl_set_parms(struct net *net, struct tcf_proto *tp,
 	fl_mask_update_range(mask);
 	fl_set_masked_key(&f->mkey, &f->key, mask);
 
-	tcf_exts_change(tp, &f->exts, &e);
+	tcf_exts_change(&f->exts, &e);
 
 	return 0;
 errout:
@@ -938,9 +936,7 @@ static int fl_change(struct net *net, struct sk_buff *in_skb,
 		goto errout_tb;
 	}
 
-	err = tcf_exts_init(&fnew->exts, TCA_FLOWER_ACT, 0);
-	if (err < 0)
-		goto errout;
+	tcf_exts_init(&fnew->exts, TCA_FLOWER_ACT, 0);
 
 	if (!handle) {
 		handle = fl_grab_new_handle(tp, head);
diff --git a/net/sched/cls_fw.c b/net/sched/cls_fw.c
index a53fa75..46a0e32 100644
--- a/net/sched/cls_fw.c
+++ b/net/sched/cls_fw.c
@@ -200,9 +200,7 @@ fw_change_attrs(struct net *net, struct tcf_proto *tp, struct fw_filter *f,
 	u32 mask;
 	int err;
 
-	err = tcf_exts_init(&e, TCA_FW_ACT, TCA_FW_POLICE);
-	if (err < 0)
-		return err;
+	tcf_exts_init(&e, TCA_FW_ACT, TCA_FW_POLICE);
 	err = tcf_exts_validate(net, tp, tb, tca[TCA_RATE], &e, ovr);
 	if (err < 0)
 		goto errout;
@@ -232,7 +230,7 @@ fw_change_attrs(struct net *net, struct tcf_proto *tp, struct fw_filter *f,
 	} else if (head->mask != 0xFFFFFFFF)
 		goto errout;
 
-	tcf_exts_change(tp, &f->exts, &e);
+	tcf_exts_change(&f->exts, &e);
 
 	return 0;
 errout:
@@ -276,11 +274,7 @@ static int fw_change(struct net *net, struct sk_buff *in_skb,
 #endif /* CONFIG_NET_CLS_IND */
 		fnew->tp = f->tp;
 
-		err = tcf_exts_init(&fnew->exts, TCA_FW_ACT, TCA_FW_POLICE);
-		if (err < 0) {
-			kfree(fnew);
-			return err;
-		}
+		tcf_exts_init(&fnew->exts, TCA_FW_ACT, TCA_FW_POLICE);
 
 		err = fw_change_attrs(net, tp, fnew, tb, tca, base, ovr);
 		if (err < 0) {
@@ -324,9 +318,7 @@ static int fw_change(struct net *net, struct sk_buff *in_skb,
 	if (f == NULL)
 		return -ENOBUFS;
 
-	err = tcf_exts_init(&f->exts, TCA_FW_ACT, TCA_FW_POLICE);
-	if (err < 0)
-		goto errout;
+	tcf_exts_init(&f->exts, TCA_FW_ACT, TCA_FW_POLICE);
 	f->id = handle;
 	f->tp = tp;
 
diff --git a/net/sched/cls_matchall.c b/net/sched/cls_matchall.c
index 9dc26c3..0b77f6e 100644
--- a/net/sched/cls_matchall.c
+++ b/net/sched/cls_matchall.c
@@ -123,9 +123,7 @@ static int mall_set_parms(struct net *net, struct tcf_proto *tp,
 	struct tcf_exts e;
 	int err;
 
-	err = tcf_exts_init(&e, TCA_MATCHALL_ACT, 0);
-	if (err)
-		return err;
+	tcf_exts_init(&e, TCA_MATCHALL_ACT, 0);
 	err = tcf_exts_validate(net, tp, tb, est, &e, ovr);
 	if (err < 0)
 		goto errout;
@@ -135,7 +133,7 @@ static int mall_set_parms(struct net *net, struct tcf_proto *tp,
 		tcf_bind_filter(tp, &head->res, base);
 	}
 
-	tcf_exts_change(tp, &head->exts, &e);
+	tcf_exts_change(&head->exts, &e);
 
 	return 0;
 errout:
@@ -176,9 +174,7 @@ static int mall_change(struct net *net, struct sk_buff *in_skb,
 	if (!new)
 		return -ENOBUFS;
 
-	err = tcf_exts_init(&new->exts, TCA_MATCHALL_ACT, 0);
-	if (err)
-		goto err_exts_init;
+	tcf_exts_init(&new->exts, TCA_MATCHALL_ACT, 0);
 
 	if (!handle)
 		handle = 1;
@@ -209,7 +205,6 @@ static int mall_change(struct net *net, struct sk_buff *in_skb,
 err_replace_hw_filter:
 err_set_parms:
 	tcf_exts_destroy(&new->exts);
-err_exts_init:
 	kfree(new);
 	return err;
 }
diff --git a/net/sched/cls_route.c b/net/sched/cls_route.c
index 26f8636..60be2c4 100644
--- a/net/sched/cls_route.c
+++ b/net/sched/cls_route.c
@@ -375,9 +375,7 @@ static int route4_set_parms(struct net *net, struct tcf_proto *tp,
 	struct tcf_exts e;
 	int err;
 
-	err = tcf_exts_init(&e, TCA_ROUTE4_ACT, TCA_ROUTE4_POLICE);
-	if (err < 0)
-		return err;
+	tcf_exts_init(&e, TCA_ROUTE4_ACT, TCA_ROUTE4_POLICE);
 	err = tcf_exts_validate(net, tp, tb, est, &e, ovr);
 	if (err < 0)
 		goto errout;
@@ -450,7 +448,7 @@ static int route4_set_parms(struct net *net, struct tcf_proto *tp,
 		tcf_bind_filter(tp, &f->res, base);
 	}
 
-	tcf_exts_change(tp, &f->exts, &e);
+	tcf_exts_change(&f->exts, &e);
 
 	return 0;
 errout:
@@ -488,9 +486,7 @@ static int route4_change(struct net *net, struct sk_buff *in_skb,
 	if (!f)
 		goto errout;
 
-	err = tcf_exts_init(&f->exts, TCA_ROUTE4_ACT, TCA_ROUTE4_POLICE);
-	if (err < 0)
-		goto errout;
+	tcf_exts_init(&f->exts, TCA_ROUTE4_ACT, TCA_ROUTE4_POLICE);
 
 	if (fold) {
 		f->id = fold->id;
diff --git a/net/sched/cls_rsvp.h b/net/sched/cls_rsvp.h
index 0d9d077..2088262 100644
--- a/net/sched/cls_rsvp.h
+++ b/net/sched/cls_rsvp.h
@@ -486,9 +486,7 @@ static int rsvp_change(struct net *net, struct sk_buff *in_skb,
 	if (err < 0)
 		return err;
 
-	err = tcf_exts_init(&e, TCA_RSVP_ACT, TCA_RSVP_POLICE);
-	if (err < 0)
-		return err;
+	tcf_exts_init(&e, TCA_RSVP_ACT, TCA_RSVP_POLICE);
 	err = tcf_exts_validate(net, tp, tb, tca[TCA_RATE], &e, ovr);
 	if (err < 0)
 		goto errout2;
@@ -507,18 +505,14 @@ static int rsvp_change(struct net *net, struct sk_buff *in_skb,
 			goto errout2;
 		}
 
-		err = tcf_exts_init(&n->exts, TCA_RSVP_ACT, TCA_RSVP_POLICE);
-		if (err < 0) {
-			kfree(n);
-			goto errout2;
-		}
+		tcf_exts_init(&n->exts, TCA_RSVP_ACT, TCA_RSVP_POLICE);
 
 		if (tb[TCA_RSVP_CLASSID]) {
 			n->res.classid = nla_get_u32(tb[TCA_RSVP_CLASSID]);
 			tcf_bind_filter(tp, &n->res, base);
 		}
 
-		tcf_exts_change(tp, &n->exts, &e);
+		tcf_exts_change(&n->exts, &e);
 		rsvp_replace(tp, n, handle);
 		return 0;
 	}
@@ -535,9 +529,7 @@ static int rsvp_change(struct net *net, struct sk_buff *in_skb,
 	if (f == NULL)
 		goto errout2;
 
-	err = tcf_exts_init(&f->exts, TCA_RSVP_ACT, TCA_RSVP_POLICE);
-	if (err < 0)
-		goto errout;
+	tcf_exts_init(&f->exts, TCA_RSVP_ACT, TCA_RSVP_POLICE);
 	h2 = 16;
 	if (tb[TCA_RSVP_SRC]) {
 		memcpy(f->src, nla_data(tb[TCA_RSVP_SRC]), sizeof(f->src));
@@ -591,7 +583,7 @@ static int rsvp_change(struct net *net, struct sk_buff *in_skb,
 			if (f->tunnelhdr == 0)
 				tcf_bind_filter(tp, &f->res, base);
 
-			tcf_exts_change(tp, &f->exts, &e);
+			tcf_exts_change(&f->exts, &e);
 
 			fp = &s->ht[h2];
 			for (nfp = rtnl_dereference(*fp); nfp;
diff --git a/net/sched/cls_tcindex.c b/net/sched/cls_tcindex.c
index 66924d1..b2d771c 100644
--- a/net/sched/cls_tcindex.c
+++ b/net/sched/cls_tcindex.c
@@ -223,10 +223,10 @@ static const struct nla_policy tcindex_policy[TCA_TCINDEX_MAX + 1] = {
 	[TCA_TCINDEX_CLASSID]		= { .type = NLA_U32 },
 };
 
-static int tcindex_filter_result_init(struct tcindex_filter_result *r)
+static void tcindex_filter_result_init(struct tcindex_filter_result *r)
 {
 	memset(r, 0, sizeof(*r));
-	return tcf_exts_init(&r->exts, TCA_TCINDEX_ACT, TCA_TCINDEX_POLICE);
+	tcf_exts_init(&r->exts, TCA_TCINDEX_ACT, TCA_TCINDEX_POLICE);
 }
 
 static void __tcindex_partial_destroy(struct rcu_head *head)
@@ -248,25 +248,18 @@ static void tcindex_free_perfect_hash(struct tcindex_data *cp)
 
 static int tcindex_alloc_perfect_hash(struct tcindex_data *cp)
 {
-	int i, err = 0;
+	int i;
 
 	cp->perfect = kcalloc(cp->hash, sizeof(struct tcindex_filter_result),
 			      GFP_KERNEL);
 	if (!cp->perfect)
 		return -ENOMEM;
 
-	for (i = 0; i < cp->hash; i++) {
-		err = tcf_exts_init(&cp->perfect[i].exts,
-				    TCA_TCINDEX_ACT, TCA_TCINDEX_POLICE);
-		if (err < 0)
-			goto errout;
-	}
+	for (i = 0; i < cp->hash; i++)
+		tcf_exts_init(&cp->perfect[i].exts,
+			      TCA_TCINDEX_ACT, TCA_TCINDEX_POLICE);
 
 	return 0;
-
-errout:
-	tcindex_free_perfect_hash(cp);
-	return err;
 }
 
 static int
@@ -282,9 +275,7 @@ tcindex_set_parms(struct net *net, struct tcf_proto *tp, unsigned long base,
 	int err, balloc = 0;
 	struct tcf_exts e;
 
-	err = tcf_exts_init(&e, TCA_TCINDEX_ACT, TCA_TCINDEX_POLICE);
-	if (err < 0)
-		return err;
+	tcf_exts_init(&e, TCA_TCINDEX_ACT, TCA_TCINDEX_POLICE);
 	err = tcf_exts_validate(net, tp, tb, est, &e, ovr);
 	if (err < 0)
 		goto errout;
@@ -316,12 +307,8 @@ tcindex_set_parms(struct net *net, struct tcf_proto *tp, unsigned long base,
 	}
 	cp->h = p->h;
 
-	err = tcindex_filter_result_init(&new_filter_result);
-	if (err < 0)
-		goto errout1;
-	err = tcindex_filter_result_init(&cr);
-	if (err < 0)
-		goto errout1;
+	tcindex_filter_result_init(&new_filter_result);
+	tcindex_filter_result_init(&cr);
 	if (old_r)
 		cr.res = r->res;
 
@@ -406,11 +393,7 @@ tcindex_set_parms(struct net *net, struct tcf_proto *tp, unsigned long base,
 			goto errout_alloc;
 		f->key = handle;
 		f->next = NULL;
-		err = tcindex_filter_result_init(&f->result);
-		if (err < 0) {
-			kfree(f);
-			goto errout_alloc;
-		}
+		tcindex_filter_result_init(&f->result);
 	}
 
 	if (tb[TCA_TCINDEX_CLASSID]) {
@@ -419,17 +402,12 @@ tcindex_set_parms(struct net *net, struct tcf_proto *tp, unsigned long base,
 	}
 
 	if (old_r)
-		tcf_exts_change(tp, &r->exts, &e);
+		tcf_exts_change(&r->exts, &e);
 	else
-		tcf_exts_change(tp, &cr.exts, &e);
+		tcf_exts_change(&cr.exts, &e);
 
-	if (old_r && old_r != r) {
-		err = tcindex_filter_result_init(old_r);
-		if (err < 0) {
-			kfree(f);
-			goto errout_alloc;
-		}
-	}
+	if (old_r && old_r != r)
+		tcindex_filter_result_init(old_r);
 
 	oldp = p;
 	r->res = cr.res;
@@ -439,7 +417,7 @@ tcindex_set_parms(struct net *net, struct tcf_proto *tp, unsigned long base,
 		struct tcindex_filter *nfp;
 		struct tcindex_filter __rcu **fp;
 
-		tcf_exts_change(tp, &f->result.exts, &r->exts);
+		tcf_exts_change(&f->result.exts, &r->exts);
 
 		fp = cp->h + (handle % cp->hash);
 		for (nfp = rtnl_dereference(*fp);
@@ -459,7 +437,6 @@ tcindex_set_parms(struct net *net, struct tcf_proto *tp, unsigned long base,
 		tcindex_free_perfect_hash(cp);
 	else if (balloc == 2)
 		kfree(cp->h);
-errout1:
 	tcf_exts_destroy(&cr.exts);
 	tcf_exts_destroy(&new_filter_result.exts);
 errout:
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index 2d01195..2c834f3 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -726,9 +726,7 @@ static int u32_set_parms(struct net *net, struct tcf_proto *tp,
 	struct tcf_exts e;
 	int err;
 
-	err = tcf_exts_init(&e, TCA_U32_ACT, TCA_U32_POLICE);
-	if (err < 0)
-		return err;
+	tcf_exts_init(&e, TCA_U32_ACT, TCA_U32_POLICE);
 	err = tcf_exts_validate(net, tp, tb, est, &e, ovr);
 	if (err < 0)
 		goto errout;
@@ -769,7 +767,7 @@ static int u32_set_parms(struct net *net, struct tcf_proto *tp,
 		n->ifindex = ret;
 	}
 #endif
-	tcf_exts_change(tp, &n->exts, &e);
+	tcf_exts_change(&n->exts, &e);
 
 	return 0;
 errout:
@@ -848,10 +846,7 @@ static struct tc_u_knode *u32_init_knode(struct tcf_proto *tp,
 	new->tp = tp;
 	memcpy(&new->sel, s, sizeof(*s) + s->nkeys*sizeof(struct tc_u32_key));
 
-	if (tcf_exts_init(&new->exts, TCA_U32_ACT, TCA_U32_POLICE)) {
-		kfree(new);
-		return NULL;
-	}
+	tcf_exts_init(&new->exts, TCA_U32_ACT, TCA_U32_POLICE);
 
 	return new;
 }
@@ -1007,9 +1002,7 @@ static int u32_change(struct net *net, struct sk_buff *in_skb,
 	n->flags = flags;
 	n->tp = tp;
 
-	err = tcf_exts_init(&n->exts, TCA_U32_ACT, TCA_U32_POLICE);
-	if (err < 0)
-		goto errout;
+	tcf_exts_init(&n->exts, TCA_U32_ACT, TCA_U32_POLICE);
 
 #ifdef CONFIG_CLS_U32_MARK
 	n->pcpu_success = alloc_percpu(u32);
-- 
2.9.3

^ permalink raw reply related

* [patch net-next 08/20] net: sched: use tcf_exts_has_actions instead of exts->nr_actions
From: Jiri Pirko @ 2017-07-28 14:40 UTC (permalink / raw)
  To: netdev; +Cc: davem, jhs, xiyou.wangcong, daniel, mlxsw
In-Reply-To: <20170728144042.6380-1-jiri@resnulli.us>

From: Jiri Pirko <jiri@mellanox.com>

For check in tcf_exts_dump use tcf_exts_has_actions helper instead
of exts->nr_actions for checking if there are any actions present.

Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
 net/sched/cls_api.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 287ae6c..735d556 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -915,7 +915,7 @@ int tcf_exts_dump(struct sk_buff *skb, struct tcf_exts *exts)
 #ifdef CONFIG_NET_CLS_ACT
 	struct nlattr *nest;
 
-	if (exts->action && exts->nr_actions) {
+	if (exts->action && tcf_exts_has_actions(exts)) {
 		/*
 		 * again for backward compatible mode - we want
 		 * to work with both old and new modes of entering
-- 
2.9.3

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox