Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next v2 2/3] mpls: move mpls_hdr to a common location
From: pravin shelar @ 2016-09-30 18:20 UTC (permalink / raw)
  To: Jiri Benc; +Cc: Linux Kernel Network Developers, David Ahern
In-Reply-To: <20160930195159.5b7eed00@griffin>

On Fri, Sep 30, 2016 at 10:51 AM, Jiri Benc <jbenc@redhat.com> wrote:
> On Fri, 30 Sep 2016 10:43:42 -0700, pravin shelar wrote:
>> There is already struct mpls_label defined in uapi mpls header file.
>> Is there reason for not using it here?
>
> I think this should be unified as well. But it's for a different patch
> not related to this patchset. This patch just moves the existing
> mpls_hdr and mpls_shim_hdr definition to a new place.
>
> In general, I think the mpls code deserves a round of overall cleanups.
> It should not block this patchset, though. It can be considered as a
> step towards that goal.
>

Alright, I do not oppose this patch going in as it is.

Acked-by: Pravin B Shelar <pshelar@ovn.org>

^ permalink raw reply

* Re: [PATCH net-next v2 3/3] openvswitch: use mpls_hdr
From: pravin shelar @ 2016-09-30 18:20 UTC (permalink / raw)
  To: Jiri Benc; +Cc: Linux Kernel Network Developers, David Ahern
In-Reply-To: <672d97cf2e761276a9390b72265606bb5cbb6bb6.1475255100.git.jbenc@redhat.com>

On Fri, Sep 30, 2016 at 10:08 AM, Jiri Benc <jbenc@redhat.com> wrote:
> skb_mpls_header is equivalent to mpls_hdr now. Use the existing helper
> instead.
>
> Signed-off-by: Jiri Benc <jbenc@redhat.com>
Acked-by: Pravin B Shelar <pshelar@ovn.org>

^ permalink raw reply

* Re: [PATCH 1/3] netfilter: nf_tables: avoid uninitialized variable warning
From: Pablo Neira Ayuso @ 2016-09-30 18:21 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Patrick McHardy, Jozsef Kadlecsik, David S. Miller,
	netfilter-devel, coreteam, netdev, linux-kernel
In-Reply-To: <20160930174749.GF1176@salvia>

On Fri, Sep 30, 2016 at 07:47:49PM +0200, Pablo Neira Ayuso wrote:
> On Fri, Sep 30, 2016 at 06:05:34PM +0200, Arnd Bergmann wrote:
> > The newly added nft_range_eval() function handles the two possible
> > nft range operations, but as the compiler warning points out,
> > any unexpected value would lead to the 'mismatch' variable being
> > used without being initialized:
> > 
> > net/netfilter/nft_range.c: In function 'nft_range_eval':
> > net/netfilter/nft_range.c:45:5: error: 'mismatch' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> > 
> > This can be trivially avoided by added a 'default:' clause.
> 
> Applied this patch, I took Aaron's and Pai's patches instead.

Looking at this again, I know uninitialized_var() has been discussed
as not nice since it can hide bugs behind. But if I fix the existing
code to validate priv->op from _init() (this is currently broken), we
can probably use this so save extra code in the packet path for a case
that is not going to happen.

Let me know, thanks!

^ permalink raw reply

* Re: UDP wierdness around skb_copy_and_csum_datagram_msg()
From: Christian Lamparter @ 2016-09-30 18:40 UTC (permalink / raw)
  To: Jay Smith
  Cc: Christian Lamparter, Alan Curry, Eric Dumazet, netdev, Al Viro,
	davem@davemloft.net
In-Reply-To: <87a8epp9ya.fsf@kentik.com>

On Friday, September 30, 2016 10:35:25 AM CEST Jay Smith wrote:
> Christian Lamparter writes:
> > On Wednesday, September 28, 2016 7:20:39 PM CEST Jay Smith wrote:
> >> Actually, on a little more searching of this list's archives, I think
> >> that this discussion:  https://patchwork.kernel.org/patch/9260733/ is
> >> about exactly the same issue I've found, except from the TCP side. I'm
> >> cc'ing a few of the participants from that discussion.
> >> 
> >> So is the patch proposed there (copying and restoring the entire
> >> iov_iter in skb_copy_and_csum_datagram_msg()) being considered as a
> >> fix?
> >
> > From Alan's post:
> >
> > "My ugly patch fixes this in the most obvious way: make a local copy of
> > msg->msg_iter before the call to skb_copy_and_csum_datagram(), and copy
> > it back if the checksum is bad, just before goto csum_error;"
> >
> > IMHO this meant that the patch is a proof of concept for his problem.
> 
> It's also the simplest thing that fixes all of the relevant cases (udp4,
> udp6, tcp4). 
Al Viro noted that tcp needed more work here (tp->ucopy.len and friends).
Trouble is, that this discussion (between Alan, David, me and Eric) was 
offlist at that point... And it doesn't look like anyone involved
wants to repeat what they have already said/written. 

(Al Viro, are you there?)

> [...]
> > As for fixing the issue: I'm happy to test and review patches. 
> > The trouble is that nobody seem to be able to produce them...
> 
> Sorry -- is the trouble you're talking about here that no-one's produced
> a patch, or that we don't have a reproduction of the problem?  I don't
> think either is true.
Reproduction wasn't the issue. Back in August, I posted method 
which used the network emulator (CONFIG_NET_SCH_NETEM) to reproduce
it easily and almost anywhere [0].

(i.e.: run this on the server/router)
# tc qdisc add dev eth0 root netem corrupt 0.1%
 
Alan also wrote a userspace tool that had a select/noselect switch
which would lead to different outcomes... etc. However, in the end
Alan could fix his issue by switching to CCMP(AES WLAN cipher), 
which he reported fixed his corruption issue.

So sadly yes, what I meant was that the patches are missing in action.

Regards,
Christian

[0] <https://lkml.org/lkml/2016/8/3/181>

^ permalink raw reply

* [PATCH 7/7] ethtool: fix leakage of strings resources in get_feature_defs
From: John W. Linville @ 2016-09-30 19:56 UTC (permalink / raw)
  To: netdev; +Cc: John W. Linville
In-Reply-To: <1475265381-28937-1-git-send-email-linville@tuxdriver.com>

Coverity issue: 1363125
Fixes: 6042804cf6ec ("Change -k/-K options to use ETHTOOL_{G,S}FEATURES")

Signed-off-by: John W. Linville <linville@tuxdriver.com>
---
 ethtool.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/ethtool.c b/ethtool.c
index 406cfd2f95a5..5b83a0041a95 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -1715,8 +1715,10 @@ static struct feature_defs *get_feature_defs(struct cmd_context *ctx)
 	}
 
 	defs = malloc(sizeof(*defs) + sizeof(defs->def[0]) * n_features);
-	if (!defs)
+	if (!defs) {
+		free(names);
 		return NULL;
+	}
 
 	defs->n_features = n_features;
 	memset(defs->off_flag_matched, 0, sizeof(defs->off_flag_matched));
-- 
2.7.4

^ permalink raw reply related

* [PATCH 3/7] ethtool: plug resource leaks of defs and features in do_gfeatures
From: John W. Linville @ 2016-09-30 19:56 UTC (permalink / raw)
  To: netdev; +Cc: John W. Linville
In-Reply-To: <1475265381-28937-1-git-send-email-linville@tuxdriver.com>

Coverity issues: 1363120, 1363121
Fixes: 6042804cf6ec ("Change -k/-K options to use ETHTOOL_{G,S}FEATURES")

Signed-off-by: John W. Linville <linville@tuxdriver.com>
---
 ethtool.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/ethtool.c b/ethtool.c
index 0885a61097ad..09486615a5bb 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -2233,10 +2233,13 @@ static int do_gfeatures(struct cmd_context *ctx)
 	features = get_features(ctx, defs);
 	if (!features) {
 		fprintf(stdout, "no feature info available\n");
+		free(defs);
 		return 1;
 	}
 
 	dump_features(defs, features, NULL);
+	free(features);
+	free(defs);
 	return 0;
 }
 
-- 
2.7.4

^ permalink raw reply related

* [PATCH 5/7] ethtool: fix leakage of efeatures resources in do_sfeatures
From: John W. Linville @ 2016-09-30 19:56 UTC (permalink / raw)
  To: netdev; +Cc: John W. Linville
In-Reply-To: <1475265381-28937-1-git-send-email-linville@tuxdriver.com>

Coverity issue: 1363122
Fixes: 6042804cf6ecc ("Change -k/-K options to use ETHTOOL_{G,S}FEATURES")

Signed-off-by: John W. Linville <linville@tuxdriver.com>
---
 ethtool.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/ethtool.c b/ethtool.c
index 2fd3393ca5f6..840a5da157a9 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -2430,6 +2430,8 @@ static int do_sfeatures(struct cmd_context *ctx)
 
 err:
 	free(defs);
+	if (efeatures)
+		free(efeatures);
 	return rc;
 }
 
-- 
2.7.4

^ permalink raw reply related

* [PATCH 6/7] ethtool: fix leakage of strings resources in do_sprivflags
From: John W. Linville @ 2016-09-30 19:56 UTC (permalink / raw)
  To: netdev; +Cc: John W. Linville
In-Reply-To: <1475265381-28937-1-git-send-email-linville@tuxdriver.com>

Coverity issue: 1363124
Fixes: e1ee596326ae ("Add support for querying and setting private flags")

Signed-off-by: John W. Linville <linville@tuxdriver.com>
---
 ethtool.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/ethtool.c b/ethtool.c
index 840a5da157a9..406cfd2f95a5 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -4285,7 +4285,7 @@ static int do_sprivflags(struct cmd_context *ctx)
 	struct cmdline_info *cmdline;
 	struct ethtool_value flags;
 	u32 wanted_flags = 0, seen_flags = 0;
-	int any_changed;
+	int any_changed, rc;
 	unsigned int i;
 
 	strings = get_stringset(ctx, ETH_SS_PRIV_FLAGS,
@@ -4297,7 +4297,8 @@ static int do_sprivflags(struct cmd_context *ctx)
 	}
 	if (strings->len == 0) {
 		fprintf(stderr, "No private flags defined\n");
-		return 1;
+		rc = 1;
+		goto err;
 	}
 	if (strings->len > 32) {
 		/* ETHTOOL_{G,S}PFLAGS can only cover 32 flags */
@@ -4308,7 +4309,8 @@ static int do_sprivflags(struct cmd_context *ctx)
 	cmdline = calloc(strings->len, sizeof(*cmdline));
 	if (!cmdline) {
 		perror("Cannot parse arguments");
-		return 1;
+		rc = 1;
+		goto err;
 	}
 	for (i = 0; i < strings->len; i++) {
 		cmdline[i].name = ((const char *)strings->data +
@@ -4324,17 +4326,22 @@ static int do_sprivflags(struct cmd_context *ctx)
 	flags.cmd = ETHTOOL_GPFLAGS;
 	if (send_ioctl(ctx, &flags)) {
 		perror("Cannot get private flags");
-		return 1;
+		rc = 1;
+		goto err;
 	}
 
 	flags.cmd = ETHTOOL_SPFLAGS;
 	flags.data = (flags.data & ~seen_flags) | wanted_flags;
 	if (send_ioctl(ctx, &flags)) {
 		perror("Cannot set private flags");
-		return 1;
+		rc = 1;
+		goto err;
 	}
 
-	return 0;
+	rc = 0;
+err:
+	free(strings);
+	return rc;
 }
 
 static int do_tsinfo(struct cmd_context *ctx)
-- 
2.7.4

^ permalink raw reply related

* [PATCH 2/7] ethtool: avoid resource leak of strings in do_gprivflags
From: John W. Linville @ 2016-09-30 19:56 UTC (permalink / raw)
  To: netdev; +Cc: John W. Linville
In-Reply-To: <1475265381-28937-1-git-send-email-linville@tuxdriver.com>

Coverity issue: 1363119
Fixes: e1ee596326ae ("Add support for querying and setting private flags")

Signed-off-by: John W. Linville <linville@tuxdriver.com>
---
 ethtool.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/ethtool.c b/ethtool.c
index aa3ef5ed2f75..0885a61097ad 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -4205,7 +4205,7 @@ static int do_gprivflags(struct cmd_context *ctx)
 	struct ethtool_gstrings *strings;
 	struct ethtool_value flags;
 	unsigned int i;
-	int max_len = 0, cur_len;
+	int max_len = 0, cur_len, rc;
 
 	if (ctx->argc != 0)
 		exit_bad_args();
@@ -4215,11 +4215,13 @@ static int do_gprivflags(struct cmd_context *ctx)
 				1);
 	if (!strings) {
 		perror("Cannot get private flag names");
-		return 1;
+		rc = 1;
+		goto err;
 	}
 	if (strings->len == 0) {
 		fprintf(stderr, "No private flags defined\n");
-		return 1;
+		rc = 1;
+		goto err;
 	}
 	if (strings->len > 32) {
 		/* ETHTOOL_GPFLAGS can only cover 32 flags */
@@ -4230,7 +4232,8 @@ static int do_gprivflags(struct cmd_context *ctx)
 	flags.cmd = ETHTOOL_GPFLAGS;
 	if (send_ioctl(ctx, &flags)) {
 		perror("Cannot get private flags");
-		return 1;
+		rc = 1;
+		goto err;
 	}
 
 	/* Find longest string and align all strings accordingly */
@@ -4248,7 +4251,12 @@ static int do_gprivflags(struct cmd_context *ctx)
 		       (const char *)strings->data + i * ETH_GSTRING_LEN,
 		       (flags.data & (1U << i)) ? "on" : "off");
 
-	return 0;
+	rc = 0;
+
+err:
+	if (strings)
+		free(strings);
+	return rc;
 }
 
 static int do_sprivflags(struct cmd_context *ctx)
-- 
2.7.4

^ permalink raw reply related

* [PATCH 1/7] ethtool: avoid NULL pointer dereference in do_permaddr
From: John W. Linville @ 2016-09-30 19:56 UTC (permalink / raw)
  To: netdev; +Cc: John W. Linville
In-Reply-To: <1475265381-28937-1-git-send-email-linville@tuxdriver.com>

Coverity issue: 1363118
Fixes: fef1c4a19703 ("ethtool: add get permanent address option")

Signed-off-by: John W. Linville <linville@tuxdriver.com>
---
 ethtool.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/ethtool.c b/ethtool.c
index 6287b559c699..aa3ef5ed2f75 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -3941,6 +3941,11 @@ static int do_permaddr(struct cmd_context *ctx)
 	struct ethtool_perm_addr *epaddr;
 
 	epaddr = malloc(sizeof(struct ethtool_perm_addr) + MAX_ADDR_LEN);
+	if (!epaddr) {
+		perror("Cannot allocate memory for operation");
+		return 1;
+	}
+
 	epaddr->cmd = ETHTOOL_GPERMADDR;
 	epaddr->size = MAX_ADDR_LEN;
 
-- 
2.7.4

^ permalink raw reply related

* [PATCH 0/7] ethtool: misc fixes for Coverity issues
From: John W. Linville @ 2016-09-30 19:56 UTC (permalink / raw)
  To: netdev; +Cc: John W. Linville

I requested a Coverity scan of the current ethtool sources. There
weren't many issues identified, but it was non-zero.

Most/all of the resource leak issues can be explained away as
unimportant due to the fact that ethtool invocations are short-term
processes. Nevertheless, I prefer to have those issues "fixed" both
to avoid them as distractions and just to promote better coding
practices overall.

These are "fresh off the press" -- I'll re-review them again before
applying and pushing them. I appreciate any extra eyeballs that can
review them as well.

John

^ permalink raw reply

* [PATCH 4/7] ethtool: fix leakage of defs resources in do_sfeatures
From: John W. Linville @ 2016-09-30 19:56 UTC (permalink / raw)
  To: netdev; +Cc: John W. Linville
In-Reply-To: <1475265381-28937-1-git-send-email-linville@tuxdriver.com>

Coverity issue: 1363123
Fixes: 6042804cf6ecc ("Change -k/-K options to use ETHTOOL_{G,S}FEATURES")

Signed-off-by: John W. Linville <linville@tuxdriver.com>
---
 ethtool.c | 41 ++++++++++++++++++++++++++++-------------
 1 file changed, 28 insertions(+), 13 deletions(-)

diff --git a/ethtool.c b/ethtool.c
index 09486615a5bb..2fd3393ca5f6 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -2253,7 +2253,7 @@ static int do_sfeatures(struct cmd_context *ctx)
 	struct cmdline_info *cmdline_features;
 	struct feature_state *old_state, *new_state;
 	struct ethtool_value eval;
-	int err;
+	int err, rc;
 	int i, j;
 
 	defs = get_feature_defs(ctx);
@@ -2267,7 +2267,8 @@ static int do_sfeatures(struct cmd_context *ctx)
 				   sizeof(efeatures->features[0]));
 		if (!efeatures) {
 			perror("Cannot parse arguments");
-			return 1;
+			rc = 1;
+			goto err;
 		}
 		efeatures->cmd = ETHTOOL_SFEATURES;
 		efeatures->size = FEATURE_BITS_TO_BLOCKS(defs->n_features);
@@ -2285,7 +2286,8 @@ static int do_sfeatures(struct cmd_context *ctx)
 				  sizeof(cmdline_features[0]));
 	if (!cmdline_features) {
 		perror("Cannot parse arguments");
-		return 1;
+		rc = 1;
+		goto err;
 	}
 	for (i = 0; i < ARRAY_SIZE(off_flag_def); i++)
 		flag_to_cmdline_info(off_flag_def[i].short_name,
@@ -2304,12 +2306,15 @@ static int do_sfeatures(struct cmd_context *ctx)
 
 	if (!any_changed) {
 		fprintf(stdout, "no features changed\n");
-		return 0;
+		rc = 0;
+		goto err;
 	}
 
 	old_state = get_features(ctx, defs);
-	if (!old_state)
-		return 1;
+	if (!old_state) {
+		rc = 1;
+		goto err;
+	}
 
 	if (efeatures) {
 		/* For each offload that the user specified, update any
@@ -2353,7 +2358,8 @@ static int do_sfeatures(struct cmd_context *ctx)
 		err = send_ioctl(ctx, efeatures);
 		if (err < 0) {
 			perror("Cannot set device feature settings");
-			return 1;
+			rc = 1;
+			goto err;
 		}
 	} else {
 		for (i = 0; i < ARRAY_SIZE(off_flag_def); i++) {
@@ -2368,7 +2374,8 @@ static int do_sfeatures(struct cmd_context *ctx)
 					fprintf(stderr,
 						"Cannot set device %s settings: %m\n",
 						off_flag_def[i].long_name);
-					return 1;
+					rc = 1;
+					goto err;
 				}
 			}
 		}
@@ -2382,15 +2389,18 @@ static int do_sfeatures(struct cmd_context *ctx)
 			err = send_ioctl(ctx, &eval);
 			if (err) {
 				perror("Cannot set device flag settings");
-				return 92;
+				rc = 92;
+				goto err;
 			}
 		}
 	}
 
 	/* Compare new state with requested state */
 	new_state = get_features(ctx, defs);
-	if (!new_state)
-		return 1;
+	if (!new_state) {
+		rc = 1;
+		goto err;
+	}
 	any_changed = new_state->off_flags != old_state->off_flags;
 	any_mismatch = (new_state->off_flags !=
 			((old_state->off_flags & ~off_flags_mask) |
@@ -2409,13 +2419,18 @@ static int do_sfeatures(struct cmd_context *ctx)
 		if (!any_changed) {
 			fprintf(stderr,
 				"Could not change any device features\n");
-			return 1;
+			rc = 1;
+			goto err;
 		}
 		printf("Actual changes:\n");
 		dump_features(defs, new_state, old_state);
 	}
 
-	return 0;
+	rc = 0;
+
+err:
+	free(defs);
+	return rc;
 }
 
 static struct ethtool_link_usettings *
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH] net: rtnl: avoid uninitialized data in IFLA_VF_VLAN_LIST handling
From: Or Gerlitz @ 2016-09-30 20:12 UTC (permalink / raw)
  To: Arnd Bergmann, Moshe Shemesh, Tariq Toukan
  Cc: David S. Miller, Roopa Prabhu, Nicolas Dichtel,
	Nikolay Aleksandrov, Jiri Pirko, Eric Dumazet, Brenden Blanco,
	Hannes Frederic Sowa, Nogah Frankel, Linux Netdev List,
	Linux Kernel
In-Reply-To: <20160930161416.18660-1-arnd@arndb.de>

On Fri, Sep 30, 2016 at 7:13 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> With the newly added support for IFLA_VF_VLAN_LIST netlink messages,
> we get a warning about potential uninitialized variable use in
> the parsing of the user input when enabling the -Wmaybe-uninitialized
> warning:
>
> net/core/rtnetlink.c: In function 'do_setvfinfo':
> net/core/rtnetlink.c:1756:9: error: 'ivvl$' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>
> I have not been able to prove whether it is possible to arrive in
> this code with an empty IFLA_VF_VLAN_LIST block, but if we do,
> then ndo_set_vf_vlan gets called with uninitialized arguments.
>
> This adds an explicit check for an empty list, making it obvious
> to the reader and the compiler that this cannot happen.
>
> Fixes: 79aab093a0b5 ("net: Update API for VF vlan protocol 802.1ad support")
Added the authors of the above patch

> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

^ permalink raw reply

* Re: [PATCH 0/7] ethtool: misc fixes for Coverity issues
From: Greg @ 2016-09-30 20:26 UTC (permalink / raw)
  To: John W. Linville; +Cc: netdev
In-Reply-To: <1475265381-28937-1-git-send-email-linville@tuxdriver.com>

On Fri, 2016-09-30 at 15:56 -0400, John W. Linville wrote:
> I requested a Coverity scan of the current ethtool sources. There
> weren't many issues identified, but it was non-zero.
> 
> Most/all of the resource leak issues can be explained away as
> unimportant due to the fact that ethtool invocations are short-term
> processes. Nevertheless, I prefer to have those issues "fixed" both
> to avoid them as distractions and just to promote better coding
> practices overall.

The patch series looks good to me.

Reviewed-by: Greg Rose <grose@lightfleet.com>

> 
> These are "fresh off the press" -- I'll re-review them again before
> applying and pushing them. I appreciate any extra eyeballs that can
> review them as well.
> 
> John
> --
> John W. Linville		Someday the world will need a hero, and you
> linville@tuxdriver.com			might be all we have.  Be ready.
> 

^ permalink raw reply

* [PATCH net-next 0/7] rxrpc: More fixes and adjustments
From: David Howells @ 2016-09-30 21:40 UTC (permalink / raw)
  To: netdev; +Cc: dhowells, linux-afs, linux-kernel


This set of patches contains some more fixes and adjustments:

 (1) Actually display the retransmission indication previously added to the
     tx_data trace.

 (2) Switch to Congestion Avoidance mode properly at cwnd==ssthresh rather
     than relying on detection during an overshoot and correction.

 (3) Reduce ssthresh to the peer's declared receive window.

 (4) The offset field in rxrpc_skb_priv can be dispensed with and the error
     field is no longer used.  Get rid of them.

 (5) Keep the call timeouts as ktimes rather than jiffies to make it easier
     to deal with RTT-based timeout values in future.  Rounding to jiffies
     is still necessary when the system timer is set.

 (6) Fix the call timer handling to avoid retriggering of expired timeout
     actions.

The patches can be found here also:

	http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=rxrpc-rewrite

Tagged thusly:

	git://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git
	rxrpc-rewrite-20160930

David
---
David Howells (7):
      rxrpc: Actually display the tx_data trace retransmission note
      rxrpc: Switch to Congestion Avoidance mode at cwnd==ssthresh
      rxrpc: Reduce ssthresh to peer's receive window
      rxrpc: The offset field in struct rxrpc_skb_priv is unnecessary
      rxrpc: Remove error from struct rxrpc_skb_priv as it is unused
      rxrpc: Keep the call timeouts as ktimes rather than jiffies
      rxrpc: Fix the call timer handling


 include/trace/events/rxrpc.h |   28 +++++++-------
 net/rxrpc/ar-internal.h      |   10 ++---
 net/rxrpc/call_event.c       |   87 +++++++++++++++++++++++++-----------------
 net/rxrpc/call_object.c      |   19 +++------
 net/rxrpc/conn_event.c       |    3 +
 net/rxrpc/input.c            |   34 +++++++++-------
 net/rxrpc/local_event.c      |    3 +
 net/rxrpc/misc.c             |   15 +++++--
 net/rxrpc/recvmsg.c          |    6 +--
 net/rxrpc/rxkad.c            |    9 +++-
 net/rxrpc/sendmsg.c          |    8 ++--
 net/rxrpc/sysctl.c           |    8 ++--
 12 files changed, 125 insertions(+), 105 deletions(-)

^ permalink raw reply

* [PATCH net-next 1/7] rxrpc: Actually display the tx_data trace retransmission note
From: David Howells @ 2016-09-30 21:40 UTC (permalink / raw)
  To: netdev; +Cc: dhowells, linux-afs, linux-kernel
In-Reply-To: <147527161156.28826.2148784664652062075.stgit@warthog.procyon.org.uk>

Actually display in the tx_data trace the retransmission note added in a
previous patch.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 include/trace/events/rxrpc.h |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/trace/events/rxrpc.h b/include/trace/events/rxrpc.h
index 8ba8d76e856a..67f03946ea4a 100644
--- a/include/trace/events/rxrpc.h
+++ b/include/trace/events/rxrpc.h
@@ -280,11 +280,12 @@ TRACE_EVENT(rxrpc_tx_data,
 		    __entry->lose = lose;
 			   ),
 
-	    TP_printk("c=%p DATA %08x q=%08x fl=%02x%s",
+	    TP_printk("c=%p DATA %08x q=%08x fl=%02x%s%s",
 		      __entry->call,
 		      __entry->serial,
 		      __entry->seq,
 		      __entry->flags,
+		      __entry->retrans ? " *RETRANS*" : "",
 		      __entry->lose ? " *LOSE*" : "")
 	    );
 

^ permalink raw reply related

* [PATCH net-next 2/7] rxrpc: Switch to Congestion Avoidance mode at cwnd==ssthresh
From: David Howells @ 2016-09-30 21:40 UTC (permalink / raw)
  To: netdev; +Cc: dhowells, linux-afs, linux-kernel
In-Reply-To: <147527161156.28826.2148784664652062075.stgit@warthog.procyon.org.uk>

Switch to Congestion Avoidance mode at cwnd == ssthresh rather than relying
on cwnd getting incremented beyond ssthresh and the window size, the mode
being shifted and then cwnd being corrected.

We need to make sure we switch into CA mode so that we stop marking every
packet for ACK.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 net/rxrpc/input.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/rxrpc/input.c b/net/rxrpc/input.c
index 1461d30583c9..21746f0f7ae0 100644
--- a/net/rxrpc/input.c
+++ b/net/rxrpc/input.c
@@ -57,7 +57,7 @@ static void rxrpc_congestion_management(struct rxrpc_call *call,
 		call->cong_ssthresh = max_t(unsigned int,
 					    summary->flight_size / 2, 2);
 		cwnd = 1;
-		if (cwnd > call->cong_ssthresh &&
+		if (cwnd >= call->cong_ssthresh &&
 		    call->cong_mode == RXRPC_CALL_SLOW_START) {
 			call->cong_mode = RXRPC_CALL_CONGEST_AVOIDANCE;
 			call->cong_tstamp = skb->tstamp;
@@ -82,7 +82,7 @@ static void rxrpc_congestion_management(struct rxrpc_call *call,
 			goto packet_loss_detected;
 		if (summary->cumulative_acks > 0)
 			cwnd += 1;
-		if (cwnd > call->cong_ssthresh) {
+		if (cwnd >= call->cong_ssthresh) {
 			call->cong_mode = RXRPC_CALL_CONGEST_AVOIDANCE;
 			call->cong_tstamp = skb->tstamp;
 		}
@@ -161,7 +161,7 @@ resume_normality:
 	call->cong_dup_acks = 0;
 	call->cong_extra = 0;
 	call->cong_tstamp = skb->tstamp;
-	if (cwnd <= call->cong_ssthresh)
+	if (cwnd < call->cong_ssthresh)
 		call->cong_mode = RXRPC_CALL_SLOW_START;
 	else
 		call->cong_mode = RXRPC_CALL_CONGEST_AVOIDANCE;

^ permalink raw reply related

* [PATCH net-next 3/7] rxrpc: Reduce ssthresh to peer's receive window
From: David Howells @ 2016-09-30 21:40 UTC (permalink / raw)
  To: netdev; +Cc: dhowells, linux-afs, linux-kernel
In-Reply-To: <147527161156.28826.2148784664652062075.stgit@warthog.procyon.org.uk>

When we receive an ACK from the peer that tells us what the peer's receive
window (rwind) is, we should reduce ssthresh to rwind if rwind is smaller
than ssthresh.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 net/rxrpc/input.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/rxrpc/input.c b/net/rxrpc/input.c
index 21746f0f7ae0..7993473e56bb 100644
--- a/net/rxrpc/input.c
+++ b/net/rxrpc/input.c
@@ -658,6 +658,8 @@ static void rxrpc_input_ackinfo(struct rxrpc_call *call, struct sk_buff *skb,
 	if (rwind > RXRPC_RXTX_BUFF_SIZE - 1)
 		rwind = RXRPC_RXTX_BUFF_SIZE - 1;
 	call->tx_winsize = rwind;
+	if (call->cong_ssthresh > rwind)
+		call->cong_ssthresh = rwind;
 
 	mtu = min(ntohl(ackinfo->rxMTU), ntohl(ackinfo->maxMTU));
 

^ permalink raw reply related

* [PATCH net-next 4/7] rxrpc: The offset field in struct rxrpc_skb_priv is unnecessary
From: David Howells @ 2016-09-30 21:40 UTC (permalink / raw)
  To: netdev; +Cc: dhowells, linux-afs, linux-kernel
In-Reply-To: <147527161156.28826.2148784664652062075.stgit@warthog.procyon.org.uk>

The offset field in struct rxrpc_skb_priv is unnecessary as the value can
always be calculated.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 net/rxrpc/ar-internal.h |    1 -
 net/rxrpc/conn_event.c  |    3 ++-
 net/rxrpc/input.c       |   23 ++++++++++++-----------
 net/rxrpc/local_event.c |    3 ++-
 net/rxrpc/recvmsg.c     |    6 ++----
 net/rxrpc/rxkad.c       |    9 ++++++---
 6 files changed, 24 insertions(+), 21 deletions(-)

diff --git a/net/rxrpc/ar-internal.h b/net/rxrpc/ar-internal.h
index 539db54697f9..fd64a2bd1072 100644
--- a/net/rxrpc/ar-internal.h
+++ b/net/rxrpc/ar-internal.h
@@ -144,7 +144,6 @@ struct rxrpc_skb_priv {
 		u8		nr_jumbo;	/* Number of jumbo subpackets */
 	};
 	union {
-		unsigned int	offset;		/* offset into buffer of next read */
 		int		remain;		/* amount of space remaining for next write */
 		u32		error;		/* network error code */
 	};
diff --git a/net/rxrpc/conn_event.c b/net/rxrpc/conn_event.c
index 37609ce89f52..3f9d8d7ec632 100644
--- a/net/rxrpc/conn_event.c
+++ b/net/rxrpc/conn_event.c
@@ -276,7 +276,8 @@ static int rxrpc_process_event(struct rxrpc_connection *conn,
 		return 0;
 
 	case RXRPC_PACKET_TYPE_ABORT:
-		if (skb_copy_bits(skb, sp->offset, &wtmp, sizeof(wtmp)) < 0)
+		if (skb_copy_bits(skb, sizeof(struct rxrpc_wire_header),
+				  &wtmp, sizeof(wtmp)) < 0)
 			return -EPROTO;
 		abort_code = ntohl(wtmp);
 		_proto("Rx ABORT %%%u { ac=%d }", sp->hdr.serial, abort_code);
diff --git a/net/rxrpc/input.c b/net/rxrpc/input.c
index 7993473e56bb..5ba35b4a907b 100644
--- a/net/rxrpc/input.c
+++ b/net/rxrpc/input.c
@@ -358,7 +358,7 @@ static bool rxrpc_receiving_reply(struct rxrpc_call *call)
 static bool rxrpc_validate_jumbo(struct sk_buff *skb)
 {
 	struct rxrpc_skb_priv *sp = rxrpc_skb(skb);
-	unsigned int offset = sp->offset;
+	unsigned int offset = sizeof(struct rxrpc_wire_header);
 	unsigned int len = skb->len;
 	int nr_jumbo = 1;
 	u8 flags = sp->hdr.flags;
@@ -419,7 +419,7 @@ static void rxrpc_input_data(struct rxrpc_call *call, struct sk_buff *skb,
 			     u16 skew)
 {
 	struct rxrpc_skb_priv *sp = rxrpc_skb(skb);
-	unsigned int offset = sp->offset;
+	unsigned int offset = sizeof(struct rxrpc_wire_header);
 	unsigned int ix;
 	rxrpc_serial_t serial = sp->hdr.serial, ack_serial = 0;
 	rxrpc_seq_t seq = sp->hdr.seq, hard_ack;
@@ -746,15 +746,16 @@ static void rxrpc_input_ack(struct rxrpc_call *call, struct sk_buff *skb,
 	} buf;
 	rxrpc_serial_t acked_serial;
 	rxrpc_seq_t first_soft_ack, hard_ack;
-	int nr_acks, offset;
+	int nr_acks, offset, ioffset;
 
 	_enter("");
 
-	if (skb_copy_bits(skb, sp->offset, &buf.ack, sizeof(buf.ack)) < 0) {
+	offset = sizeof(struct rxrpc_wire_header);
+	if (skb_copy_bits(skb, offset, &buf.ack, sizeof(buf.ack)) < 0) {
 		_debug("extraction failure");
 		return rxrpc_proto_abort("XAK", call, 0);
 	}
-	sp->offset += sizeof(buf.ack);
+	offset += sizeof(buf.ack);
 
 	acked_serial = ntohl(buf.ack.serial);
 	first_soft_ack = ntohl(buf.ack.firstPacket);
@@ -792,9 +793,9 @@ static void rxrpc_input_ack(struct rxrpc_call *call, struct sk_buff *skb,
 				  rxrpc_propose_ack_respond_to_ack);
 	}
 
-	offset = sp->offset + nr_acks + 3;
-	if (skb->len >= offset + sizeof(buf.info)) {
-		if (skb_copy_bits(skb, offset, &buf.info, sizeof(buf.info)) < 0)
+	ioffset = offset + nr_acks + 3;
+	if (skb->len >= ioffset + sizeof(buf.info)) {
+		if (skb_copy_bits(skb, ioffset, &buf.info, sizeof(buf.info)) < 0)
 			return rxrpc_proto_abort("XAI", call, 0);
 		rxrpc_input_ackinfo(call, skb, &buf.info);
 	}
@@ -832,7 +833,7 @@ static void rxrpc_input_ack(struct rxrpc_call *call, struct sk_buff *skb,
 		rxrpc_rotate_tx_window(call, hard_ack, &summary);
 
 	if (nr_acks > 0) {
-		if (skb_copy_bits(skb, sp->offset, buf.acks, nr_acks) < 0)
+		if (skb_copy_bits(skb, offset, buf.acks, nr_acks) < 0)
 			return rxrpc_proto_abort("XSA", call, 0);
 		rxrpc_input_soft_acks(call, buf.acks, first_soft_ack, nr_acks,
 				      &summary);
@@ -880,7 +881,8 @@ static void rxrpc_input_abort(struct rxrpc_call *call, struct sk_buff *skb)
 	_enter("");
 
 	if (skb->len >= 4 &&
-	    skb_copy_bits(skb, sp->offset, &wtmp, sizeof(wtmp)) >= 0)
+	    skb_copy_bits(skb, sizeof(struct rxrpc_wire_header),
+			  &wtmp, sizeof(wtmp)) >= 0)
 		abort_code = ntohl(wtmp);
 
 	_proto("Rx ABORT %%%u { %x }", sp->hdr.serial, abort_code);
@@ -996,7 +998,6 @@ int rxrpc_extract_header(struct rxrpc_skb_priv *sp, struct sk_buff *skb)
 	sp->hdr.securityIndex	= whdr.securityIndex;
 	sp->hdr._rsvd		= ntohs(whdr._rsvd);
 	sp->hdr.serviceId	= ntohs(whdr.serviceId);
-	sp->offset = sizeof(whdr);
 	return 0;
 }
 
diff --git a/net/rxrpc/local_event.c b/net/rxrpc/local_event.c
index 190f68bd9e27..540d3955c1bc 100644
--- a/net/rxrpc/local_event.c
+++ b/net/rxrpc/local_event.c
@@ -95,7 +95,8 @@ void rxrpc_process_local_events(struct rxrpc_local *local)
 
 		switch (sp->hdr.type) {
 		case RXRPC_PACKET_TYPE_VERSION:
-			if (skb_copy_bits(skb, sp->offset, &v, 1) < 0)
+			if (skb_copy_bits(skb, sizeof(struct rxrpc_wire_header),
+					  &v, 1) < 0)
 				return;
 			_proto("Rx VERSION { %02x }", v);
 			if (v == 0)
diff --git a/net/rxrpc/recvmsg.c b/net/rxrpc/recvmsg.c
index 038ae62ddb4d..f05ea0a88076 100644
--- a/net/rxrpc/recvmsg.c
+++ b/net/rxrpc/recvmsg.c
@@ -261,15 +261,13 @@ static int rxrpc_locate_data(struct rxrpc_call *call, struct sk_buff *skb,
 			     u8 *_annotation,
 			     unsigned int *_offset, unsigned int *_len)
 {
-	struct rxrpc_skb_priv *sp = rxrpc_skb(skb);
-	unsigned int offset = *_offset;
+	unsigned int offset = sizeof(struct rxrpc_wire_header);
 	unsigned int len = *_len;
 	int ret;
 	u8 annotation = *_annotation;
 
 	/* Locate the subpacket */
-	offset = sp->offset;
-	len = skb->len - sp->offset;
+	len = skb->len - offset;
 	if ((annotation & RXRPC_RX_ANNO_JUMBO) > 0) {
 		offset += (((annotation & RXRPC_RX_ANNO_JUMBO) - 1) *
 			   RXRPC_JUMBO_SUBPKTLEN);
diff --git a/net/rxrpc/rxkad.c b/net/rxrpc/rxkad.c
index 88d080a1a3de..627abed5f999 100644
--- a/net/rxrpc/rxkad.c
+++ b/net/rxrpc/rxkad.c
@@ -771,7 +771,8 @@ static int rxkad_respond_to_challenge(struct rxrpc_connection *conn,
 	}
 
 	abort_code = RXKADPACKETSHORT;
-	if (skb_copy_bits(skb, sp->offset, &challenge, sizeof(challenge)) < 0)
+	if (skb_copy_bits(skb, sizeof(struct rxrpc_wire_header),
+			  &challenge, sizeof(challenge)) < 0)
 		goto protocol_error;
 
 	version = ntohl(challenge.version);
@@ -1028,7 +1029,8 @@ static int rxkad_verify_response(struct rxrpc_connection *conn,
 	_enter("{%d,%x}", conn->debug_id, key_serial(conn->server_key));
 
 	abort_code = RXKADPACKETSHORT;
-	if (skb_copy_bits(skb, sp->offset, &response, sizeof(response)) < 0)
+	if (skb_copy_bits(skb, sizeof(struct rxrpc_wire_header),
+			  &response, sizeof(response)) < 0)
 		goto protocol_error;
 	if (!pskb_pull(skb, sizeof(response)))
 		BUG();
@@ -1057,7 +1059,8 @@ static int rxkad_verify_response(struct rxrpc_connection *conn,
 		return -ENOMEM;
 
 	abort_code = RXKADPACKETSHORT;
-	if (skb_copy_bits(skb, sp->offset, ticket, ticket_len) < 0)
+	if (skb_copy_bits(skb, sizeof(struct rxrpc_wire_header),
+			  ticket, ticket_len) < 0)
 		goto protocol_error_free;
 
 	ret = rxkad_decrypt_ticket(conn, ticket, ticket_len, &session_key,

^ permalink raw reply related

* [PATCH net-next 5/7] rxrpc: Remove error from struct rxrpc_skb_priv as it is unused
From: David Howells @ 2016-09-30 21:40 UTC (permalink / raw)
  To: netdev; +Cc: dhowells, linux-afs, linux-kernel
In-Reply-To: <147527161156.28826.2148784664652062075.stgit@warthog.procyon.org.uk>

Remove error from struct rxrpc_skb_priv as it is no longer used.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 net/rxrpc/ar-internal.h |    1 -
 1 file changed, 1 deletion(-)

diff --git a/net/rxrpc/ar-internal.h b/net/rxrpc/ar-internal.h
index fd64a2bd1072..141c1458e719 100644
--- a/net/rxrpc/ar-internal.h
+++ b/net/rxrpc/ar-internal.h
@@ -145,7 +145,6 @@ struct rxrpc_skb_priv {
 	};
 	union {
 		int		remain;		/* amount of space remaining for next write */
-		u32		error;		/* network error code */
 	};
 
 	struct rxrpc_host_header hdr;		/* RxRPC packet header from this packet */

^ permalink raw reply related

* [PATCH net-next 6/7] rxrpc: Keep the call timeouts as ktimes rather than jiffies
From: David Howells @ 2016-09-30 21:40 UTC (permalink / raw)
  To: netdev; +Cc: dhowells, linux-afs, linux-kernel
In-Reply-To: <147527161156.28826.2148784664652062075.stgit@warthog.procyon.org.uk>

Keep that call timeouts as ktimes rather than jiffies so that they can be
expressed as functions of RTT.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 include/trace/events/rxrpc.h |   25 +++++++-------
 net/rxrpc/ar-internal.h      |    8 ++---
 net/rxrpc/call_event.c       |   73 ++++++++++++++++++++++--------------------
 net/rxrpc/call_object.c      |   16 +++------
 net/rxrpc/input.c            |    3 +-
 net/rxrpc/misc.c             |   15 ++++++---
 net/rxrpc/sendmsg.c          |    8 ++---
 net/rxrpc/sysctl.c           |    8 ++---
 8 files changed, 82 insertions(+), 74 deletions(-)

diff --git a/include/trace/events/rxrpc.h b/include/trace/events/rxrpc.h
index 67f03946ea4a..0383e5e9a0f3 100644
--- a/include/trace/events/rxrpc.h
+++ b/include/trace/events/rxrpc.h
@@ -453,17 +453,18 @@ TRACE_EVENT(rxrpc_rtt_rx,
 
 TRACE_EVENT(rxrpc_timer,
 	    TP_PROTO(struct rxrpc_call *call, enum rxrpc_timer_trace why,
-		     unsigned long now),
+		     ktime_t now, unsigned long now_j),
 
-	    TP_ARGS(call, why, now),
+	    TP_ARGS(call, why, now, now_j),
 
 	    TP_STRUCT__entry(
 		    __field(struct rxrpc_call *,		call		)
 		    __field(enum rxrpc_timer_trace,		why		)
-		    __field(unsigned long,			now		)
-		    __field(unsigned long,			expire_at	)
-		    __field(unsigned long,			ack_at		)
-		    __field(unsigned long,			resend_at	)
+		    __field_struct(ktime_t,			now		)
+		    __field_struct(ktime_t,			expire_at	)
+		    __field_struct(ktime_t,			ack_at		)
+		    __field_struct(ktime_t,			resend_at	)
+		    __field(unsigned long,			now_j		)
 		    __field(unsigned long,			timer		)
 			     ),
 
@@ -474,17 +475,17 @@ TRACE_EVENT(rxrpc_timer,
 		    __entry->expire_at	= call->expire_at;
 		    __entry->ack_at	= call->ack_at;
 		    __entry->resend_at	= call->resend_at;
+		    __entry->now_j	= now_j;
 		    __entry->timer	= call->timer.expires;
 			   ),
 
-	    TP_printk("c=%p %s now=%lx x=%ld a=%ld r=%ld t=%ld",
+	    TP_printk("c=%p %s x=%lld a=%lld r=%lld t=%ld",
 		      __entry->call,
 		      rxrpc_timer_traces[__entry->why],
-		      __entry->now,
-		      __entry->expire_at - __entry->now,
-		      __entry->ack_at - __entry->now,
-		      __entry->resend_at - __entry->now,
-		      __entry->timer - __entry->now)
+		      ktime_to_ns(ktime_sub(__entry->expire_at, __entry->now)),
+		      ktime_to_ns(ktime_sub(__entry->ack_at, __entry->now)),
+		      ktime_to_ns(ktime_sub(__entry->resend_at, __entry->now)),
+		      __entry->timer - __entry->now_j)
 	    );
 
 TRACE_EVENT(rxrpc_rx_lose,
diff --git a/net/rxrpc/ar-internal.h b/net/rxrpc/ar-internal.h
index 141c1458e719..d38dffd78085 100644
--- a/net/rxrpc/ar-internal.h
+++ b/net/rxrpc/ar-internal.h
@@ -464,9 +464,9 @@ struct rxrpc_call {
 	struct rxrpc_connection	*conn;		/* connection carrying call */
 	struct rxrpc_peer	*peer;		/* Peer record for remote address */
 	struct rxrpc_sock __rcu	*socket;	/* socket responsible */
-	unsigned long		ack_at;		/* When deferred ACK needs to happen */
-	unsigned long		resend_at;	/* When next resend needs to happen */
-	unsigned long		expire_at;	/* When the call times out */
+	ktime_t			ack_at;		/* When deferred ACK needs to happen */
+	ktime_t			resend_at;	/* When next resend needs to happen */
+	ktime_t			expire_at;	/* When the call times out */
 	struct timer_list	timer;		/* Combined event timer */
 	struct work_struct	processor;	/* Event processor */
 	rxrpc_notify_rx_t	notify_rx;	/* kernel service Rx notification function */
@@ -805,7 +805,7 @@ int rxrpc_reject_call(struct rxrpc_sock *);
 /*
  * call_event.c
  */
-void rxrpc_set_timer(struct rxrpc_call *, enum rxrpc_timer_trace);
+void rxrpc_set_timer(struct rxrpc_call *, enum rxrpc_timer_trace, ktime_t);
 void rxrpc_propose_ACK(struct rxrpc_call *, u8, u16, u32, bool, bool,
 		       enum rxrpc_propose_ack_trace);
 void rxrpc_process_call(struct work_struct *);
diff --git a/net/rxrpc/call_event.c b/net/rxrpc/call_event.c
index 1f6c7633b964..9ff3bb3ffb41 100644
--- a/net/rxrpc/call_event.c
+++ b/net/rxrpc/call_event.c
@@ -24,28 +24,40 @@
 /*
  * Set the timer
  */
-void rxrpc_set_timer(struct rxrpc_call *call, enum rxrpc_timer_trace why)
+void rxrpc_set_timer(struct rxrpc_call *call, enum rxrpc_timer_trace why,
+		     ktime_t now)
 {
-	unsigned long t, now = jiffies;
+	unsigned long t_j, now_j = jiffies;
+	ktime_t t;
 
 	read_lock_bh(&call->state_lock);
 
 	if (call->state < RXRPC_CALL_COMPLETE) {
 		t = call->expire_at;
-		if (time_before_eq(t, now))
+		if (!ktime_after(t, now))
 			goto out;
 
-		if (time_after(call->resend_at, now) &&
-		    time_before(call->resend_at, t))
+		if (ktime_after(call->resend_at, now) &&
+		    ktime_before(call->resend_at, t))
 			t = call->resend_at;
 
-		if (time_after(call->ack_at, now) &&
-		    time_before(call->ack_at, t))
+		if (ktime_after(call->ack_at, now) &&
+		    ktime_before(call->ack_at, t))
 			t = call->ack_at;
 
-		if (call->timer.expires != t || !timer_pending(&call->timer)) {
-			mod_timer(&call->timer, t);
-			trace_rxrpc_timer(call, why, now);
+		t_j = nsecs_to_jiffies(ktime_to_ns(ktime_sub(t, now)));
+		t_j += jiffies;
+
+		/* We have to make sure that the calculated jiffies value falls
+		 * at or after the nsec value, or we may loop ceaselessly
+		 * because the timer times out, but we haven't reached the nsec
+		 * timeout yet.
+		 */
+		t_j++;
+
+		if (call->timer.expires != t_j || !timer_pending(&call->timer)) {
+			mod_timer(&call->timer, t_j);
+			trace_rxrpc_timer(call, why, now, now_j);
 		}
 	}
 
@@ -62,7 +74,8 @@ static void __rxrpc_propose_ACK(struct rxrpc_call *call, u8 ack_reason,
 				enum rxrpc_propose_ack_trace why)
 {
 	enum rxrpc_propose_ack_outcome outcome = rxrpc_propose_ack_use;
-	unsigned long now, ack_at, expiry = rxrpc_soft_ack_delay;
+	unsigned int expiry = rxrpc_soft_ack_delay;
+	ktime_t now, ack_at;
 	s8 prior = rxrpc_ack_priority[ack_reason];
 
 	/* Update DELAY, IDLE, REQUESTED and PING_RESPONSE ACK serial
@@ -111,7 +124,6 @@ static void __rxrpc_propose_ACK(struct rxrpc_call *call, u8 ack_reason,
 		break;
 	}
 
-	now = jiffies;
 	if (test_bit(RXRPC_CALL_EV_ACK, &call->events)) {
 		_debug("already scheduled");
 	} else if (immediate || expiry == 0) {
@@ -120,11 +132,11 @@ static void __rxrpc_propose_ACK(struct rxrpc_call *call, u8 ack_reason,
 		    background)
 			rxrpc_queue_call(call);
 	} else {
-		ack_at = now + expiry;
-		_debug("deferred ACK %ld < %ld", expiry, call->ack_at - now);
-		if (time_before(ack_at, call->ack_at)) {
+		now = ktime_get_real();
+		ack_at = ktime_add_ms(now, expiry);
+		if (ktime_before(ack_at, call->ack_at)) {
 			call->ack_at = ack_at;
-			rxrpc_set_timer(call, rxrpc_timer_set_for_ack);
+			rxrpc_set_timer(call, rxrpc_timer_set_for_ack, now);
 		}
 	}
 
@@ -157,12 +169,12 @@ static void rxrpc_congestion_timeout(struct rxrpc_call *call)
 /*
  * Perform retransmission of NAK'd and unack'd packets.
  */
-static void rxrpc_resend(struct rxrpc_call *call)
+static void rxrpc_resend(struct rxrpc_call *call, ktime_t now)
 {
 	struct rxrpc_skb_priv *sp;
 	struct sk_buff *skb;
 	rxrpc_seq_t cursor, seq, top;
-	ktime_t now = ktime_get_real(), max_age, oldest, resend_at, ack_ts;
+	ktime_t max_age, oldest, ack_ts;
 	int ix;
 	u8 annotation, anno_type, retrans = 0, unacked = 0;
 
@@ -212,14 +224,7 @@ static void rxrpc_resend(struct rxrpc_call *call)
 				       ktime_to_ns(ktime_sub(skb->tstamp, max_age)));
 	}
 
-	resend_at = ktime_add_ms(oldest, rxrpc_resend_timeout);
-	call->resend_at = jiffies +
-		nsecs_to_jiffies(ktime_to_ns(ktime_sub(resend_at, now))) +
-		1; /* We have to make sure that the calculated jiffies value
-		    * falls at or after the nsec value, or we shall loop
-		    * ceaselessly because the timer times out, but we haven't
-		    * reached the nsec timeout yet.
-		    */
+	call->resend_at = ktime_add_ms(oldest, rxrpc_resend_timeout);
 
 	if (unacked)
 		rxrpc_congestion_timeout(call);
@@ -229,7 +234,7 @@ static void rxrpc_resend(struct rxrpc_call *call)
 	 * retransmitting data.
 	 */
 	if (!retrans) {
-		rxrpc_set_timer(call, rxrpc_timer_set_for_resend);
+		rxrpc_set_timer(call, rxrpc_timer_set_for_resend, now);
 		spin_unlock_bh(&call->lock);
 		ack_ts = ktime_sub(now, call->acks_latest_ts);
 		if (ktime_to_ns(ack_ts) < call->peer->rtt)
@@ -301,7 +306,7 @@ void rxrpc_process_call(struct work_struct *work)
 {
 	struct rxrpc_call *call =
 		container_of(work, struct rxrpc_call, processor);
-	unsigned long now;
+	ktime_t now;
 
 	rxrpc_see_call(call);
 
@@ -320,15 +325,15 @@ recheck_state:
 		goto out_put;
 	}
 
-	now = jiffies;
-	if (time_after_eq(now, call->expire_at)) {
+	now = ktime_get_real();
+	if (ktime_before(call->expire_at, now)) {
 		rxrpc_abort_call("EXP", call, 0, RX_CALL_TIMEOUT, ETIME);
 		set_bit(RXRPC_CALL_EV_ABORT, &call->events);
 		goto recheck_state;
 	}
 
 	if (test_and_clear_bit(RXRPC_CALL_EV_ACK, &call->events) ||
-	    time_after_eq(now, call->ack_at)) {
+	    ktime_before(call->ack_at, now)) {
 		call->ack_at = call->expire_at;
 		if (call->ackr_reason) {
 			rxrpc_send_call_packet(call, RXRPC_PACKET_TYPE_ACK);
@@ -337,12 +342,12 @@ recheck_state:
 	}
 
 	if (test_and_clear_bit(RXRPC_CALL_EV_RESEND, &call->events) ||
-	    time_after_eq(now, call->resend_at)) {
-		rxrpc_resend(call);
+	    ktime_before(call->resend_at, now)) {
+		rxrpc_resend(call, now);
 		goto recheck_state;
 	}
 
-	rxrpc_set_timer(call, rxrpc_timer_set_for_resend);
+	rxrpc_set_timer(call, rxrpc_timer_set_for_resend, now);
 
 	/* other events may have been raised since we started checking */
 	if (call->events && call->state < RXRPC_CALL_COMPLETE) {
diff --git a/net/rxrpc/call_object.c b/net/rxrpc/call_object.c
index d4b3293b78fa..456ab752d473 100644
--- a/net/rxrpc/call_object.c
+++ b/net/rxrpc/call_object.c
@@ -19,11 +19,6 @@
 #include <net/af_rxrpc.h>
 #include "ar-internal.h"
 
-/*
- * Maximum lifetime of a call (in jiffies).
- */
-unsigned int rxrpc_max_call_lifetime = 60 * HZ;
-
 const char *const rxrpc_call_states[NR__RXRPC_CALL_STATES] = {
 	[RXRPC_CALL_UNINITIALISED]		= "Uninit  ",
 	[RXRPC_CALL_CLIENT_AWAIT_CONN]		= "ClWtConn",
@@ -77,7 +72,8 @@ static void rxrpc_call_timer_expired(unsigned long _call)
 	_enter("%d", call->debug_id);
 
 	if (call->state < RXRPC_CALL_COMPLETE) {
-		trace_rxrpc_timer(call, rxrpc_timer_expired, jiffies);
+		trace_rxrpc_timer(call, rxrpc_timer_expired,
+				  ktime_get_real(), jiffies);
 		rxrpc_queue_call(call);
 	}
 }
@@ -207,14 +203,14 @@ static struct rxrpc_call *rxrpc_alloc_client_call(struct sockaddr_rxrpc *srx,
  */
 static void rxrpc_start_call_timer(struct rxrpc_call *call)
 {
-	unsigned long expire_at;
+	ktime_t now = ktime_get_real(), expire_at;
 
-	expire_at = jiffies + rxrpc_max_call_lifetime;
+	expire_at = ktime_add_ms(now, rxrpc_max_call_lifetime);
 	call->expire_at = expire_at;
 	call->ack_at = expire_at;
 	call->resend_at = expire_at;
-	call->timer.expires = expire_at + 1;
-	rxrpc_set_timer(call, rxrpc_timer_begin);
+	call->timer.expires = jiffies + LONG_MAX / 2;
+	rxrpc_set_timer(call, rxrpc_timer_begin, now);
 }
 
 /*
diff --git a/net/rxrpc/input.c b/net/rxrpc/input.c
index 5ba35b4a907b..3ad9f75031e3 100644
--- a/net/rxrpc/input.c
+++ b/net/rxrpc/input.c
@@ -328,7 +328,8 @@ static bool rxrpc_receiving_reply(struct rxrpc_call *call)
 		call->resend_at = call->expire_at;
 		call->ack_at = call->expire_at;
 		spin_unlock_bh(&call->lock);
-		rxrpc_set_timer(call, rxrpc_timer_init_for_reply);
+		rxrpc_set_timer(call, rxrpc_timer_init_for_reply,
+				ktime_get_real());
 	}
 
 	if (!test_bit(RXRPC_CALL_TX_LAST, &call->flags))
diff --git a/net/rxrpc/misc.c b/net/rxrpc/misc.c
index 47dddacdbb91..9d1c721bc4e8 100644
--- a/net/rxrpc/misc.c
+++ b/net/rxrpc/misc.c
@@ -21,28 +21,33 @@
 unsigned int rxrpc_max_backlog __read_mostly = 10;
 
 /*
+ * Maximum lifetime of a call (in mx).
+ */
+unsigned int rxrpc_max_call_lifetime = 60 * 1000;
+
+/*
  * How long to wait before scheduling ACK generation after seeing a
- * packet with RXRPC_REQUEST_ACK set (in jiffies).
+ * packet with RXRPC_REQUEST_ACK set (in ms).
  */
 unsigned int rxrpc_requested_ack_delay = 1;
 
 /*
- * How long to wait before scheduling an ACK with subtype DELAY (in jiffies).
+ * How long to wait before scheduling an ACK with subtype DELAY (in ms).
  *
  * We use this when we've received new data packets.  If those packets aren't
  * all consumed within this time we will send a DELAY ACK if an ACK was not
  * requested to let the sender know it doesn't need to resend.
  */
-unsigned int rxrpc_soft_ack_delay = 1 * HZ;
+unsigned int rxrpc_soft_ack_delay = 1 * 1000;
 
 /*
- * How long to wait before scheduling an ACK with subtype IDLE (in jiffies).
+ * How long to wait before scheduling an ACK with subtype IDLE (in ms).
  *
  * We use this when we've consumed some previously soft-ACK'd packets when
  * further packets aren't immediately received to decide when to send an IDLE
  * ACK let the other end know that it can free up its Tx buffer space.
  */
-unsigned int rxrpc_idle_ack_delay = 0.5 * HZ;
+unsigned int rxrpc_idle_ack_delay = 0.5 * 1000;
 
 /*
  * Receive window size in packets.  This indicates the maximum number of
diff --git a/net/rxrpc/sendmsg.c b/net/rxrpc/sendmsg.c
index d8dfdce874d8..3322543d460a 100644
--- a/net/rxrpc/sendmsg.c
+++ b/net/rxrpc/sendmsg.c
@@ -149,13 +149,13 @@ static void rxrpc_queue_packet(struct rxrpc_call *call, struct sk_buff *skb,
 		_debug("need instant resend %d", ret);
 		rxrpc_instant_resend(call, ix);
 	} else {
-		unsigned long resend_at;
+		ktime_t now = ktime_get_real(), resend_at;
 
-		resend_at = jiffies + msecs_to_jiffies(rxrpc_resend_timeout);
+		resend_at = ktime_add_ms(now, rxrpc_resend_timeout);
 
-		if (time_before(resend_at, call->resend_at)) {
+		if (ktime_before(resend_at, call->resend_at)) {
 			call->resend_at = resend_at;
-			rxrpc_set_timer(call, rxrpc_timer_set_for_send);
+			rxrpc_set_timer(call, rxrpc_timer_set_for_send, now);
 		}
 	}
 
diff --git a/net/rxrpc/sysctl.c b/net/rxrpc/sysctl.c
index 13d1df03ebac..34c706d2f79c 100644
--- a/net/rxrpc/sysctl.c
+++ b/net/rxrpc/sysctl.c
@@ -35,7 +35,7 @@ static struct ctl_table rxrpc_sysctl_table[] = {
 		.data		= &rxrpc_requested_ack_delay,
 		.maxlen		= sizeof(unsigned int),
 		.mode		= 0644,
-		.proc_handler	= proc_dointvec_ms_jiffies,
+		.proc_handler	= proc_dointvec,
 		.extra1		= (void *)&zero,
 	},
 	{
@@ -43,7 +43,7 @@ static struct ctl_table rxrpc_sysctl_table[] = {
 		.data		= &rxrpc_soft_ack_delay,
 		.maxlen		= sizeof(unsigned int),
 		.mode		= 0644,
-		.proc_handler	= proc_dointvec_ms_jiffies,
+		.proc_handler	= proc_dointvec,
 		.extra1		= (void *)&one,
 	},
 	{
@@ -51,7 +51,7 @@ static struct ctl_table rxrpc_sysctl_table[] = {
 		.data		= &rxrpc_idle_ack_delay,
 		.maxlen		= sizeof(unsigned int),
 		.mode		= 0644,
-		.proc_handler	= proc_dointvec_ms_jiffies,
+		.proc_handler	= proc_dointvec,
 		.extra1		= (void *)&one,
 	},
 	{
@@ -85,7 +85,7 @@ static struct ctl_table rxrpc_sysctl_table[] = {
 		.data		= &rxrpc_max_call_lifetime,
 		.maxlen		= sizeof(unsigned int),
 		.mode		= 0644,
-		.proc_handler	= proc_dointvec_jiffies,
+		.proc_handler	= proc_dointvec,
 		.extra1		= (void *)&one,
 	},
 

^ permalink raw reply related

* [PATCH net-next 7/7] rxrpc: Fix the call timer handling
From: David Howells @ 2016-09-30 21:41 UTC (permalink / raw)
  To: netdev; +Cc: dhowells, linux-afs, linux-kernel
In-Reply-To: <147527161156.28826.2148784664652062075.stgit@warthog.procyon.org.uk>

The call timer's concept of a call timeout (of which there are three) that
is inactive is that it is the timeout has the same expiration time as the
call expiration timeout (the expiration timer is never inactive).  However,
I'm not resetting the timeouts when they expire, leading to repeated
processing of expired timeouts when other timeout events occur.

Fix this by:

 (1) Move the timer expiry detection into rxrpc_set_timer() inside the
     locked section.  This means that if a timeout is set that will expire
     immediately, we deal with it immediately.

 (2) If a timeout is at or before now then it has expired.  When an expiry
     is detected, an event is raised, the timeout is automatically
     inactivated and the event processor is queued.

 (3) If a timeout is at or after the expiry timeout then it is inactive.
     Inactive timeouts do not contribute to the timer setting.

 (4) The call timer callback can now just call rxrpc_set_timer() to handle
     things.

 (5) The call processor work function now checks the event flags rather
     than checking the timeouts directly.

Signed-off-by: David Howells <dhowells@redhat.com>
---

 net/rxrpc/call_event.c  |   26 ++++++++++++++++++--------
 net/rxrpc/call_object.c |    7 ++-----
 2 files changed, 20 insertions(+), 13 deletions(-)

diff --git a/net/rxrpc/call_event.c b/net/rxrpc/call_event.c
index 9ff3bb3ffb41..4f00476630b9 100644
--- a/net/rxrpc/call_event.c
+++ b/net/rxrpc/call_event.c
@@ -29,6 +29,7 @@ void rxrpc_set_timer(struct rxrpc_call *call, enum rxrpc_timer_trace why,
 {
 	unsigned long t_j, now_j = jiffies;
 	ktime_t t;
+	bool queue = false;
 
 	read_lock_bh(&call->state_lock);
 
@@ -37,13 +38,21 @@ void rxrpc_set_timer(struct rxrpc_call *call, enum rxrpc_timer_trace why,
 		if (!ktime_after(t, now))
 			goto out;
 
-		if (ktime_after(call->resend_at, now) &&
-		    ktime_before(call->resend_at, t))
+		if (!ktime_after(call->resend_at, now)) {
+			call->resend_at = call->expire_at;
+			if (!test_and_set_bit(RXRPC_CALL_EV_RESEND, &call->events))
+				queue = true;
+		} else if (ktime_before(call->resend_at, t)) {
 			t = call->resend_at;
+		}
 
-		if (ktime_after(call->ack_at, now) &&
-		    ktime_before(call->ack_at, t))
+		if (!ktime_after(call->ack_at, now)) {
+			call->ack_at = call->expire_at;
+			if (!test_and_set_bit(RXRPC_CALL_EV_ACK, &call->events))
+				queue = true;
+		} else if (ktime_before(call->ack_at, t)) {
 			t = call->ack_at;
+		}
 
 		t_j = nsecs_to_jiffies(ktime_to_ns(ktime_sub(t, now)));
 		t_j += jiffies;
@@ -59,6 +68,9 @@ void rxrpc_set_timer(struct rxrpc_call *call, enum rxrpc_timer_trace why,
 			mod_timer(&call->timer, t_j);
 			trace_rxrpc_timer(call, why, now, now_j);
 		}
+
+		if (queue)
+			rxrpc_queue_call(call);
 	}
 
 out:
@@ -332,8 +344,7 @@ recheck_state:
 		goto recheck_state;
 	}
 
-	if (test_and_clear_bit(RXRPC_CALL_EV_ACK, &call->events) ||
-	    ktime_before(call->ack_at, now)) {
+	if (test_and_clear_bit(RXRPC_CALL_EV_ACK, &call->events)) {
 		call->ack_at = call->expire_at;
 		if (call->ackr_reason) {
 			rxrpc_send_call_packet(call, RXRPC_PACKET_TYPE_ACK);
@@ -341,8 +352,7 @@ recheck_state:
 		}
 	}
 
-	if (test_and_clear_bit(RXRPC_CALL_EV_RESEND, &call->events) ||
-	    ktime_before(call->resend_at, now)) {
+	if (test_and_clear_bit(RXRPC_CALL_EV_RESEND, &call->events)) {
 		rxrpc_resend(call, now);
 		goto recheck_state;
 	}
diff --git a/net/rxrpc/call_object.c b/net/rxrpc/call_object.c
index 456ab752d473..364b42dc3dce 100644
--- a/net/rxrpc/call_object.c
+++ b/net/rxrpc/call_object.c
@@ -71,11 +71,8 @@ static void rxrpc_call_timer_expired(unsigned long _call)
 
 	_enter("%d", call->debug_id);
 
-	if (call->state < RXRPC_CALL_COMPLETE) {
-		trace_rxrpc_timer(call, rxrpc_timer_expired,
-				  ktime_get_real(), jiffies);
-		rxrpc_queue_call(call);
-	}
+	if (call->state < RXRPC_CALL_COMPLETE)
+		rxrpc_set_timer(call, rxrpc_timer_expired, ktime_get_real());
 }
 
 /*

^ permalink raw reply related

* [PATCH] net: mvmdio: do not clk_disable_unprepare() NULL clock
From: Alexey Khoroshilov @ 2016-09-30 21:56 UTC (permalink / raw)
  To: Florian Fainelli, Andrew Lunn
  Cc: Alexey Khoroshilov, David S. Miller, netdev, linux-kernel,
	ldv-project

There is no need to clk_disable_unprepare(dev->clk)
before it was initialized.

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

Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
---
 drivers/net/ethernet/marvell/mvmdio.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvmdio.c b/drivers/net/ethernet/marvell/mvmdio.c
index 8982c882af1b..a0d1b084ecec 100644
--- a/drivers/net/ethernet/marvell/mvmdio.c
+++ b/drivers/net/ethernet/marvell/mvmdio.c
@@ -211,8 +211,7 @@ static int orion_mdio_probe(struct platform_device *pdev)
 	dev->regs = devm_ioremap(&pdev->dev, r->start, resource_size(r));
 	if (!dev->regs) {
 		dev_err(&pdev->dev, "Unable to remap SMI register\n");
-		ret = -ENODEV;
-		goto out_mdio;
+		return -ENODEV;
 	}
 
 	init_waitqueue_head(&dev->smi_busy_wait);
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH/RFC 00/12] Programming Open vSwitch (-like) flows into hardware using SwitchDev
From: pravin shelar @ 2016-09-30 22:12 UTC (permalink / raw)
  To: Simon Horman
  Cc: dev-yBygre7rU0TnMu66kgdUjQ@public.gmane.org, Linux Netdev List,
	Rony Efraim, Or Gerlitz
In-Reply-To: <20160929080904.GA24113-ucRxlxcrRFEsysjaEhV7d2ey4e3TpSOZIxS8c3vjKQDk1uMJSBkQmQ@public.gmane.org>

On Thu, Sep 29, 2016 at 1:09 AM, Simon Horman
<simon.horman@netronome.com> wrote:
> Hi Or,
>
> On Wed, Sep 28, 2016 at 04:54:40PM +0300, Or Gerlitz wrote:
>> On Wed, Sep 28, 2016 at 3:42 PM, Simon Horman
>> <simon.horman@netronome.com> wrote:
>>
>> > A different approach, not implemented by this patch-set, is for user-space
>> > to program flows into hardware by some other means, for example TC, and/or
>> > the (kernel) datapath.
>>
>> Right, and we've submitted that code to the OVS community 24h ago [1].
>>
>> This was done along the feedback we've got for the last two years (since
>> the  LPC 2014 networking micro-conf). It allows offloading from
>> multiple user-space
>> applications through a single UAPI -- the TC one (currently we did
>> flower, but the OVSD
>> patch set can be extended to use whatever TC offloads are supported by
>> the port driver,
>> e.g U32, eBPF) and integration with 3rd party policy modules  running
>> in user-space.
>>
>> Lets hear people opinions and see where we go from now.
>>
>> > I believe that approach does not conflict with this one.
>> >  And there is some scope to share infrastructure in the kernel
>>
>> maybe, possibly
>>
>> We've having a talk in netdev 1.2 on offloading HW offloading of OVS
>> and similar applications,
>> I would encourage people to come and approach me and/or Rony Efraim
>> from Mellanox before/after
>> the talk to discuss that F2F, would love to get feedbacks, and also here...
>
> Thanks for putting my post in context with the work you mention.
> I am looking forward to some F2F discussions next week.
>

I will not be able to come to netdev conference, so I wanted to post
my thoughts here.
I would like to thank for sharing the code. But I see some issues with
this design for flow offloading.

1. This adds OVS specific flow based offloading piece to switchdev
rather than adding generic one. Any switchdev flow based offloads
would be available to OVS only. Any other software switch can not make
use of it.
2. This complicates OVS kernel module by adding hardware related state
in datapath. OVS datapath is suppose to be software switch.
3. Since this switchdev offload is controlled by OVS kernel module,
any new API exposed by switchdev would need some changes to OVS kernel
module and possibly OVS API.
4. There is project going on to rewrite OVS kernel module in eBPF.
Thus plugging in hardware offload in the OVS kernel module make it
obsolete once we move to new implementation. We would need to rewrite
hardware offload again for eBPF OVS.
5. OVS kernel module has much less context than userspace. So
userspace based API can use the context and can be more effective when
offloading a flow to limited offloading resources.
6. This patch has also increased struct flow size but most of users
would not be using switchdev offloads.
7. This could be a just a bug, but patch 11 changes OVS_FLOW_CMD_SET
API where flow key has changed a required parameter from optional.

Why not allow switchdev offload API for userspace similar to TC flower
offload? or we could use flower API for switchdev flow offload.

Thanks,
Pravin.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

^ permalink raw reply

* [PATCH] net: Use ns_capable_noaudit() when determining net sysctl permissions
From: Dmitry Torokhov @ 2016-09-30 22:24 UTC (permalink / raw)
  To: David S. Miller
  Cc: Tyler Hicks, Serge E. Hallyn, James Morris, Eric W. Biederman,
	netdev, linux-kernel

From: Tyler Hicks <tyhicks@canonical.com>

The capability check should not be audited since it is only being used
to determine the inode permissions. A failed check does not indicate a
violation of security policy but, when an LSM is enabled, a denial audit
message was being generated.

The denial audit message caused confusion for some application authors
because root-running Go applications always triggered the denial. To
prevent this confusion, the capability check in net_ctl_permissions() is
switched to the noaudit variant.

BugLink: https://launchpad.net/bugs/1465724

Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Serge E. Hallyn <serge.hallyn@ubuntu.com>
Signed-off-by: James Morris <james.l.morris@oracle.com>
[dtor: reapplied after e79c6a4fc923 ("net: make net namespace sysctls
belong to container's owner") accidentally reverted the change.]
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 net/sysctl_net.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/sysctl_net.c b/net/sysctl_net.c
index c5d37f4..9199813 100644
--- a/net/sysctl_net.c
+++ b/net/sysctl_net.c
@@ -44,7 +44,7 @@ static int net_ctl_permissions(struct ctl_table_header *head,
 	struct net *net = container_of(head->set, struct net, sysctls);
 
 	/* Allow network administrator to have same access as root. */
-	if (ns_capable(net->user_ns, CAP_NET_ADMIN)) {
+	if (ns_capable_noaudit(net->user_ns, CAP_NET_ADMIN)) {
 		int mode = (table->mode >> 6) & 7;
 		return (mode << 6) | (mode << 3) | mode;
 	}
-- 
2.8.0.rc3.226.g39d4020


-- 
Dmitry

^ 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