Netdev List
 help / color / mirror / Atom feed
* Re: [RFC] net;sched: Try to find idle cpu for RPS to handle packets
From: Eric Dumazet @ 2018-09-19 15:49 UTC (permalink / raw)
  To: Kirill Tkhai
  Cc: Peter Zijlstra, David Miller, Daniel Borkmann, tom, netdev, LKML
In-Reply-To: <2fdf2bd7-1cc4-a1e1-15c2-e2badfcd4d59@virtuozzo.com>

[-- Attachment #1: Type: text/plain, Size: 4801 bytes --]

On Wed, Sep 19, 2018 at 8:41 AM Kirill Tkhai <ktkhai@virtuozzo.com> wrote:
>
> On 19.09.2018 17:55, Eric Dumazet wrote:
> > On Wed, Sep 19, 2018 at 5:29 AM Kirill Tkhai <ktkhai@virtuozzo.com> wrote:
> >>
> >> Many workloads have polling mode of work. The application
> >> checks for incomming packets from time to time, but it also
> >> has a work to do, when there is no packets. This RFC
> >> tries to develop an idea to queue RPS packets on idle
> >> CPU in the the L3 domain of the consumer, so backlog
> >> processing of the packets and the application can execute
> >> in parallel.
> >>
> >> We require this in case of network cards does not
> >> have enough RX queues to cover all online CPUs (this seems
> >> to be the most cards), and  get_rps_cpu() actually chooses
> >> remote cpu, and SMP interrupt is sent. Here we may try
> >> our best, and to find idle CPU nearly the consumer's CPU.
> >> Note, that in case of consumer works in poll mode and it
> >> does not waits for incomming packets, its CPU will be not
> >> idle, while CPU of a sleeping consumer may be idle. So,
> >> not polling consumers will still be able to have skb
> >> handled on its CPU.
> >>
> >> In case of network card has many queues, the device
> >> interrupts will come on consumer's CPU, and this patch
> >> won't try to find idle cpu for them.
> >>
> >> I've tried simple netperf test for this:
> >> netserver -p 1234
> >> netperf -L 127.0.0.1 -p 1234 -l 100
> >>
> >> Before:
> >>  87380  16384  16384    100.00   60323.56
> >>  87380  16384  16384    100.00   60388.46
> >>  87380  16384  16384    100.00   60217.68
> >>  87380  16384  16384    100.00   57995.41
> >>  87380  16384  16384    100.00   60659.00
> >>
> >> After:
> >>  87380  16384  16384    100.00   64569.09
> >>  87380  16384  16384    100.00   64569.25
> >>  87380  16384  16384    100.00   64691.63
> >>  87380  16384  16384    100.00   64930.14
> >>  87380  16384  16384    100.00   62670.15
> >>
> >> The difference between best runs is +7%,
> >> the worst runs differ +8%.
> >>
> >> What do you think about following somehow in this way?
> >
> > Hi Kirill
> >
> > In my experience, scheduler has a poor view of softirq processing
> > happening on various cpus.
> > A cpu spending 90% of its cycles processing IRQ might be considered 'idle'
>
> Yes, in case of there is softirq on top of irq_exit(), the cpu is not
> considered as busy. But after MAX_SOFTIRQ_TIME (=2ms), ksoftirqd are
> waken up to execute the work in process context, and the processor is
> considered as !idle. 2ms is 2 timer ticks in case of HZ=1000. So, we
> don't restart softirq in case of it was executed for more then 2ms.
>

That's the theory, but reality is very different unfortunately.

If RFS/RPS is setup properly, we really do not hit MAX_SOFTIRQ_TIME condition
unless in some synthetic benchmarks maybe.

> The similar way, single net_rx_action() can't be executed longer
> than 2ms.
>
> Having 90% load in softirq (called on top of irq_exit()) should be
> very unlikely situation, when there are too many interrupts with small
> amount of work, which related softirq calls are doing for each of them.
> I think it had be a problem even in plain napi case, since it would
> worked not like expected.
>
> But anyway. You worry, that during handling of next portion of skbs,
> we find that previous portion of skbs already woken ksoftirqd, and
> we don't see this cpu as idle? Yeah, then we'll try to change cpu,
> and this is not what we want. We want to continue use the cpu, where
> previous portion was handler. Hm, not so fast I'll answer, but certainly,
> this may be handled somehow in more creative way.
>
> > So please run a real workload (it is _very_ uncommon anyone set up RPS
> > on lo interface !)
> >
> > Like 400 or more concurrent netperf -t TCP_RR on a 10Gbit NIC.
>
> Yeah, it's just a simulation of a single irq nic. I'll try on something
> more real hardware.

Also my concern is that you might have results that are tied to a particular
version of process scheduling, platform, workload...

One month later, a small change in process scheduler,
and very different results.

This is why I believe this new feature must be controllable, via a new
tunable (like RPS/RFS are controllable per rx queue)

>
> How do you execute such the tests? I don't see the appropriate parameter
> of netperf. Does this mean just to start 400 copies of netperf? How is
> to aggregate their results in this case?

Yeah, there are various 'super_netperf' scripts available on the net
(almost trivial to write anyway)

( I am attaching one of them)

Thanks.
>
> > Thanks.
> >
> > PS: Idea of playing with L3 domains is interesting, I have personally
> > tried various strategies in the past but none of them
> > demonstrated a clear win.
>
> Thanks,
> Kirill

[-- Attachment #2: super_netperf --]
[-- Type: application/octet-stream, Size: 435 bytes --]

#!/bin/bash

run_netperf() {
	loops=$1
	shift
	for ((i=0; i<loops; i++)); do
		./netperf -s 2 $@ | awk '/Min/{
			if (!once) {
				print;
				once=1;
			}
		}
		{
			if (NR == 6)
				save = $NF
			else if (NR==7) {
				if (NF > 0)
					print $NF
				else
					print save
			} else if (NR==11) {
				print $0
			}
		}' &
	done
	wait
	return 0
}

run_netperf $@ | awk '{if (NF==7) {print $0; next}} {sum += $1} END {printf "%7u\n",sum}'

^ permalink raw reply

* Re: [PATCH ethtool] ethtool: support combinations of FEC modes
From: Michal Kubecek @ 2018-09-19 15:56 UTC (permalink / raw)
  To: Edward Cree
  Cc: John W. Linville, netdev, Ganesh Goudar, Jakub Kicinski,
	Dustin Byford, Dirk van der Merwe
In-Reply-To: <613d954c-d88c-fa20-f436-75625f11a2ae@solarflare.com>

On Wed, Sep 19, 2018 at 04:38:27PM +0100, Edward Cree wrote:
> On 19/09/18 15:41, Michal Kubecek wrote:
> > I'm sorry I didn't notice this before the patch was accepted but as it's
> > not in a release yet, maybe it's still not too late.
> >
> > Could I suggest to make the syntax consistent with other options?
> I didn't realise ethtool had any patterns to be consistent with ;)

Way too many, I must say. :-) That is why I wasn't happy about adding
another.

> > I mean rather than a comma separated list to use either
> >
> >   ethtool --set-fec <dev> encoding enc1 enc2 ...
> but yes this looks fine to me, as long as we're reasonably confident that
>  we won't want to add new parameters (that might require determining
>  whether enc2 is an encoding or a parameter name) in the future, because
>  while the parsing wouldn't be impossible it might get ugly.

This problem already exists for "-s ... msglvl". In the parser for the
netlink series I introduced an "end of list" marker (tentatively "--")
for this purpose, perhaps that could be a way.

> I'll rustle up an RFC patch.

Thank you.

Michal Kubecek

^ permalink raw reply

* [RFC PATCH ethtool] ethtool: better syntax for combinations of FEC modes
From: Edward Cree @ 2018-09-19 16:06 UTC (permalink / raw)
  To: John W. Linville; +Cc: Michal Kubecek, netdev
In-Reply-To: <20180919144117.GF3876@unicorn.suse.cz>

Instead of commas, just have them as separate argvs.

The parsing state machine might look heavyweight, but it makes it easy to add
 more parameters later and distinguish parameter names from encoding names.

Suggested-by: Michal Kubecek <mkubecek@suse.cz>
Signed-off-by: Edward Cree <ecree@solarflare.com>
---
 ethtool.8.in   |  6 +++---
 ethtool.c      | 63 ++++++++++++++++------------------------------------------
 test-cmdline.c | 10 +++++-----
 3 files changed, 25 insertions(+), 54 deletions(-)

diff --git a/ethtool.8.in b/ethtool.8.in
index 414eaa1..7ea2cc0 100644
--- a/ethtool.8.in
+++ b/ethtool.8.in
@@ -390,7 +390,7 @@ ethtool \- query or control network driver and hardware settings
 .B ethtool \-\-set\-fec
 .I devname
 .B encoding
-.BR auto | off | rs | baser [ , ...]
+.BR auto | off | rs | baser \ [...]
 .
 .\" Adjust lines (i.e. full justification) and hyphenate.
 .ad
@@ -1120,11 +1120,11 @@ current FEC mode, the driver or firmware must take the link down
 administratively and report the problem in the system logs for users to correct.
 .RS 4
 .TP
-.BR encoding\ auto | off | rs | baser [ , ...]
+.BR encoding\ auto | off | rs | baser \ [...]
 
 Sets the FEC encoding for the device.  Combinations of options are specified as
 e.g.
-.B auto,rs
+.B encoding auto rs
 ; the semantics of such combinations vary between drivers.
 .TS
 nokeep;
diff --git a/ethtool.c b/ethtool.c
index 9997930..2f7e96b 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -4979,39 +4979,6 @@ static int fecmode_str_to_type(const char *str)
 	return 0;
 }
 
-/* Takes a comma-separated list of FEC modes, returns the bitwise OR of their
- * corresponding ETHTOOL_FEC_* constants.
- * Accepts repetitions (e.g. 'auto,auto') and trailing comma (e.g. 'off,').
- */
-static int parse_fecmode(const char *str)
-{
-	int fecmode = 0;
-	char buf[6];
-
-	if (!str)
-		return 0;
-	while (*str) {
-		size_t next;
-		int mode;
-
-		next = strcspn(str, ",");
-		if (next >= 6) /* Bad mode, longest name is 5 chars */
-			return 0;
-		/* Copy into temp buffer and nul-terminate */
-		memcpy(buf, str, next);
-		buf[next] = 0;
-		mode = fecmode_str_to_type(buf);
-		if (!mode) /* Bad mode encountered */
-			return 0;
-		fecmode |= mode;
-		str += next;
-		/* Skip over ',' (but not nul) */
-		if (*str)
-			str++;
-	}
-	return fecmode;
-}
-
 static int do_gfec(struct cmd_context *ctx)
 {
 	struct ethtool_fecparam feccmd = { 0 };
@@ -5041,22 +5008,26 @@ static int do_gfec(struct cmd_context *ctx)
 
 static int do_sfec(struct cmd_context *ctx)
 {
-	char *fecmode_str = NULL;
+	enum { ARG_NONE, ARG_ENCODING } state = ARG_NONE;
 	struct ethtool_fecparam feccmd;
-	struct cmdline_info cmdline_fec[] = {
-		{ "encoding", CMDL_STR,  &fecmode_str,  &feccmd.fec},
-	};
-	int changed;
-	int fecmode;
-	int rv;
+	int fecmode = 0, newmode;
+	int rv, i;
 
-	parse_generic_cmdline(ctx, &changed, cmdline_fec,
-			      ARRAY_SIZE(cmdline_fec));
-
-	if (!fecmode_str)
+	for (i = 0; i < ctx->argc; i++) {
+		if (!strcmp(ctx->argp[i], "encoding")) {
+			state = ARG_ENCODING;
+			continue;
+		}
+		if (state == ARG_ENCODING) {
+			newmode = fecmode_str_to_type(ctx->argp[i]);
+			if (!newmode)
+				exit_bad_args();
+			fecmode |= newmode;
+			continue;
+		}
 		exit_bad_args();
+	}
 
-	fecmode = parse_fecmode(fecmode_str);
 	if (!fecmode)
 		exit_bad_args();
 
@@ -5265,7 +5236,7 @@ static const struct option {
 	  "		[ all ]\n"},
 	{ "--show-fec", 1, do_gfec, "Show FEC settings"},
 	{ "--set-fec", 1, do_sfec, "Set FEC settings",
-	  "		[ encoding auto|off|rs|baser ]\n"},
+	  "		[ encoding auto|off|rs|baser [...]]\n"},
 	{ "-h|--help", 0, show_usage, "Show this help" },
 	{ "--version", 0, do_version, "Show version number" },
 	{}
diff --git a/test-cmdline.c b/test-cmdline.c
index 9c51cca..84630a5 100644
--- a/test-cmdline.c
+++ b/test-cmdline.c
@@ -268,12 +268,12 @@ static struct test_case {
 	{ 1, "--set-eee devname advertise foo" },
 	{ 1, "--set-fec devname" },
 	{ 0, "--set-fec devname encoding auto" },
-	{ 0, "--set-fec devname encoding off," },
-	{ 0, "--set-fec devname encoding baser,rs" },
-	{ 0, "--set-fec devname encoding auto,auto," },
+	{ 0, "--set-fec devname encoding off" },
+	{ 0, "--set-fec devname encoding baser rs" },
+	{ 0, "--set-fec devname encoding auto auto" },
 	{ 1, "--set-fec devname encoding foo" },
-	{ 1, "--set-fec devname encoding auto,foo" },
-	{ 1, "--set-fec devname encoding auto,," },
+	{ 1, "--set-fec devname encoding auto foo" },
+	{ 1, "--set-fec devname encoding none" },
 	{ 1, "--set-fec devname auto" },
 	/* can't test --set-priv-flags yet */
 	{ 0, "-h" },

^ permalink raw reply related

* [PATCH] net: macb: Clean 64b dma addresses if they are not detected
From: Michal Simek @ 2018-09-19 16:08 UTC (permalink / raw)
  To: linux-kernel, monstr, Edgar E. Iglesias
  Cc: David S. Miller, netdev, u-boot, Joe Hershberger, Nicolas Ferre

Clear ADDR64 dma bit in DMACFG register in case that HW_DMA_CAP_64B
is not detected on 64bit system.
The issue was observed when bootloader(u-boot) does not check macb
feature at DCFG6 register (DAW64_OFFSET) and enabling 64bit dma support
by default. Then macb driver is reading DMACFG register back and only
adding 64bit dma configuration but not cleaning it out.

This is also align with other features which are also cleared if they are not
present.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

 drivers/net/ethernet/cadence/macb_main.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 16e4ef7d7185..79707dff3f13 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -2163,6 +2163,8 @@ static void macb_configure_dma(struct macb *bp)
 #ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
 		if (bp->hw_dma_cap & HW_DMA_CAP_64B)
 			dmacfg |= GEM_BIT(ADDR64);
+		else
+			dmacfg &= ~GEM_BIT(ADDR64);
 #endif
 #ifdef CONFIG_MACB_USE_HWSTAMP
 		if (bp->hw_dma_cap & HW_DMA_CAP_PTP)
-- 
1.9.1

^ permalink raw reply related

* Re: [PATCH 3/7] netlink: set extack error message in nla_validate()
From: David Ahern @ 2018-09-19 16:20 UTC (permalink / raw)
  To: Johannes Berg, linux-wireless, netdev; +Cc: Johannes Berg
In-Reply-To: <20180919120900.28708-4-johannes@sipsolutions.net>

On 9/19/18 5:08 AM, Johannes Berg wrote:
> From: Johannes Berg <johannes.berg@intel.com>
> 
> In nla_parse() we already set this, but it makes sense to
> also do it in nla_validate() which already also sets the
> bad attribute pointer.
> 
> CC: David Ahern <dsahern@gmail.com>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> ---
>  lib/nlattr.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/nlattr.c b/lib/nlattr.c
> index e2e5b38394d5..33404745bec4 100644
> --- a/lib/nlattr.c
> +++ b/lib/nlattr.c
> @@ -180,9 +180,13 @@ int nla_validate(const struct nlattr *head, int len, int maxtype,
>  	int rem;
>  
>  	nla_for_each_attr(nla, head, len, rem) {
> -		int err = validate_nla(nla, maxtype, policy, NULL);
> +		static const char _msg[] = "Attribute failed policy validation";
> +		const char *msg = _msg;
> +		int err = validate_nla(nla, maxtype, policy, &msg);
>  
>  		if (err < 0) {
> +			if (extack)
> +				extack->_msg = msg;
>  			NL_SET_BAD_ATTR(extack, nla);

msg, NL_SET_BAD_ATTR and extack handling all can be done in validate_nla
removing the need for the same message ("Attribute failed policy
validation") to be declared twice and simplifying the extack setting.

>  			return err;
>  		}
> 

^ permalink raw reply

* Re: [PATCH 2/7] netlink: make validation_data const
From: David Ahern @ 2018-09-19 16:21 UTC (permalink / raw)
  To: Johannes Berg, linux-wireless, netdev; +Cc: Johannes Berg
In-Reply-To: <20180919120900.28708-3-johannes@sipsolutions.net>

On 9/19/18 5:08 AM, Johannes Berg wrote:
> From: Johannes Berg <johannes.berg@intel.com>
> 
> The validation data is only used within the policy that
> should usually already be const, and isn't changed in any
> code that uses it. Therefore, make the validation_data
> pointer const.
> 
> While at it, remove the duplicate variable in the bitfield
> validation that I'd otherwise have to change to const.
> 
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> ---
>  include/net/netlink.h | 2 +-
>  lib/nlattr.c          | 5 ++---
>  2 files changed, 3 insertions(+), 4 deletions(-)
> 

Reviewed-by: David Ahern <dsahern@gmail.com>

^ permalink raw reply

* [PATCH 03/22] linux/net.h: use DYNAMIC_DEBUG_BRANCH in net_dbg_ratelimited
From: Rasmus Villemoes @ 2018-09-19 22:04 UTC (permalink / raw)
  To: Jason Baron, Andrew Morton; +Cc: linux-kernel, Rasmus Villemoes, netdev
In-Reply-To: <20180919220444.23190-1-linux@rasmusvillemoes.dk>

net_dbg_ratelimited tests the dynamic debug descriptor the old-fashioned
way, and doesn't utilize the static key/jump label implementation on
architectures that HAVE_JUMP_LABEL. Use the DYNAMIC_DEBUG_BRANCH which
is defined appropriately.

Cc: netdev@vger.kernel.org
Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 include/linux/net.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/net.h b/include/linux/net.h
index e0930678c8bf..651fca72286c 100644
--- a/include/linux/net.h
+++ b/include/linux/net.h
@@ -263,7 +263,7 @@ do {								\
 #define net_dbg_ratelimited(fmt, ...)					\
 do {									\
 	DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, fmt);			\
-	if (unlikely(descriptor.flags & _DPRINTK_FLAGS_PRINT) &&	\
+	if (DYNAMIC_DEBUG_BRANCH(descriptor) &&				\
 	    net_ratelimit())						\
 		__dynamic_pr_debug(&descriptor, pr_fmt(fmt),		\
 		                   ##__VA_ARGS__);			\
-- 
2.16.4

^ permalink raw reply related

* Re: [PATCH 5/7] netlink: prepare validate extack setting for recursion
From: David Ahern @ 2018-09-19 16:28 UTC (permalink / raw)
  To: Johannes Berg, linux-wireless, netdev; +Cc: Johannes Berg
In-Reply-To: <20180919120900.28708-6-johannes@sipsolutions.net>

On 9/19/18 5:08 AM, Johannes Berg wrote:
> diff --git a/lib/nlattr.c b/lib/nlattr.c
> index 966cd3dcf31b..2b015e43b725 100644
> --- a/lib/nlattr.c
> +++ b/lib/nlattr.c
> @@ -69,7 +69,7 @@ static int validate_nla_bitfield32(const struct nlattr *nla,
>  
>  static int validate_nla(const struct nlattr *nla, int maxtype,
>  			const struct nla_policy *policy,
> -			const char **error_msg)
> +			struct netlink_ext_ack *extack, bool *extack_set)

extack_set arg is not needed if you handle the "Attribute failed policy
validation" message and NL_SET_BAD_ATTR here as well.

^ permalink raw reply

* Re: [PATCH 3/7] netlink: set extack error message in nla_validate()
From: Johannes Berg @ 2018-09-19 16:31 UTC (permalink / raw)
  To: David Ahern, linux-wireless, netdev
In-Reply-To: <3e7aadff-5228-660a-537a-9b54c11fa5cc@gmail.com>

On Wed, 2018-09-19 at 09:20 -0700, David Ahern wrote:

> >  	nla_for_each_attr(nla, head, len, rem) {
> > -		int err = validate_nla(nla, maxtype, policy, NULL);
> > +		static const char _msg[] = "Attribute failed policy validation";
> > +		const char *msg = _msg;
> > +		int err = validate_nla(nla, maxtype, policy, &msg);
> >  
> >  		if (err < 0) {
> > +			if (extack)
> > +				extack->_msg = msg;
> >  			NL_SET_BAD_ATTR(extack, nla);
> 
> msg, NL_SET_BAD_ATTR and extack handling all can be done in validate_nla
> removing the need for the same message ("Attribute failed policy
> validation") to be declared twice and simplifying the extack setting.

Yeah, perhaps I should take another look at that. I didn't want to do
that originally as validate_nla() has so many exit points, but perhaps
it's better to put a goto label there.

FWIW, in the next patch I'm getting rid of the duplication again - I
wanted to have the patch that has an effect (this one, setting the
message) more clearly separated.

johannes

^ permalink raw reply

* Re: [PATCH 5/7] netlink: prepare validate extack setting for recursion
From: Johannes Berg @ 2018-09-19 16:36 UTC (permalink / raw)
  To: David Ahern, linux-wireless, netdev
In-Reply-To: <18ea5293-5a12-7aca-7373-12d1ab3a0821@gmail.com>

On Wed, 2018-09-19 at 09:28 -0700, David Ahern wrote:
> On 9/19/18 5:08 AM, Johannes Berg wrote:
> > diff --git a/lib/nlattr.c b/lib/nlattr.c
> > index 966cd3dcf31b..2b015e43b725 100644
> > --- a/lib/nlattr.c
> > +++ b/lib/nlattr.c
> > @@ -69,7 +69,7 @@ static int validate_nla_bitfield32(const struct nlattr *nla,
> >  
> >  static int validate_nla(const struct nlattr *nla, int maxtype,
> >  			const struct nla_policy *policy,
> > -			const char **error_msg)
> > +			struct netlink_ext_ack *extack, bool *extack_set)
> 
> extack_set arg is not needed if you handle the "Attribute failed policy
> validation" message and NL_SET_BAD_ATTR here as well.

I'm not sure that's true, but perhaps you have a better idea than me?

My thought would be to introduce an "error" label in validate_nla(),
that sets up the extack data.

Then we could skip over that if we have a separate message to report,
making the NLA_REJECT case easier.

However, if we do nested validation, I'm not sure it really is that much
easier? We still need to figure out if the nested validation was setting
the message (and bad attr), rather than it having been set before we
even get into this function.

So let's say we have

        case NLA_NESTED:
                /* a nested attributes is allowed to be empty; if its not,
                 * it must have a size of at least NLA_HDRLEN.
                 */
                if (attrlen == 0)
                        break;
                if (attrlen < NLA_HDRLEN)
                        return -ERANGE;
                if (pt->validation_data) {
                        int err;

                        err = nla_validate_parse(nla_data(nla), nla_len(nla),
                                                 pt->len, pt->validation_data,
                                                 extack, extack_set, NULL);
                        if (err < 0)
                                return err;
                }
                break;

right now after all the patches.

The "return -ERANGE;" would become "{ err = -ERANGE; goto error; }", but
I'm not really sure we can cleanly handle the other case?

Hmm. Maybe it works if we ensure that nla_validate_parse() has no other
return points that can fail outside of validate_nla(), or we set up the
extack data there as well, so that once we have a nested
nla_validate_parse() we know that it's been set.

Actually, we need to do that anyway so that we can move the setting into
validate_nla(), and then it should work.

Mechanics aside - I'll take a look later tonight or tomorrow - do you
think the goal/external interface of this makes sense?

johannes

^ permalink raw reply

* Re: [PATCH 1/1] macsec: reflect the master interface state
From: Radu Rendec @ 2018-09-19 16:44 UTC (permalink / raw)
  To: sd; +Cc: netdev, davem, ptalbert
In-Reply-To: <20180919152425.GA3046@bistromath.localdomain>

Hello,

On Wed, Sep 19, 2018 at 11:24 AM Sabrina Dubroca <sd@queasysnail.net> wrote:
> 2018-09-18, 20:16:12 -0400, Radu Rendec wrote:
> > This patch makes macsec interfaces reflect the state of the underlying
> > interface: if the master interface changes state to up/down, the macsec
> > interface changes its state accordingly.
>
> Yes, that's a bit unfortunate. I was wondering if we should just allow
> setting the device up, and let link state handle the rest.

Yes, that could work. It would also be consistent with the idea of
propagating only the link state.

> It's missing small parts of link state handling that I have been
> testing, mainly when creating a new device.

Can you please be more specific? I've just looked at macsec_newlink()
and I didn't notice anything related to link state handling.

> > -             list_for_each_entry_safe(m, n, &rxd->secys, secys) {
> > +             list_for_each_entry_safe(m, n, &rxd->secys, secys)
> >                       macsec_common_dellink(m->secy.netdev, &head);
> > -             }
>
> Please don't mix coding style changes with bug fixes.

Noted.

> > +     case NETDEV_DOWN: {
> > +             struct net_device *dev, *tmp;
> > +             LIST_HEAD(close_list);
> > +
> > +             list_for_each_entry(m, &rxd->secys, secys) {
> > +                     dev = m->secy.netdev;
> > +
> > +                     if (dev->flags & IFF_UP)
> > +                             list_add(&dev->close_list, &close_list);
> > +             }
> > +
> > +             dev_close_many(&close_list, false);
> > +
> > +             list_for_each_entry_safe(dev, tmp, &close_list, close_list) {
> > +                     netif_stacked_transfer_operstate(real_dev, dev);
> > +                     list_del_init(&dev->close_list);
> > +             }
> > +             list_del(&close_list);
> > +             break;
>
> My version of the patch only does netif_stacked_transfer_operstate(),
> and skips setting the device administratively down (ie, same as
> NETDEV_CHANGE).

That makes sense. But, since you mentioned you had your own patch, does
it make sense for me to continue working on mine?

> >       }
> > +     case NETDEV_UP:
> > +             list_for_each_entry(m, &rxd->secys, secys) {
> > +                     struct net_device *dev = m->secy.netdev;
> > +                     int flags = dev_get_flags(dev);
> > +
> > +                     if (!(flags & IFF_UP)) {
> > +                             dev_change_flags(dev, flags | IFF_UP);
> > +                             netif_stacked_transfer_operstate(real_dev, dev);
> > +                     }
> > +             }
> > +             break;
>
> I don't like that this completely ignores whether the device was done
> independently of the lower link. Maybe the administrator actually
> wants the macsec device down, regardless of state changes on the lower
> device.

I thought about that too and I don't like it either. Perhaps the vlan
approach of having a "loose binding" flag is worth considering. Then the
admin can decice for themselves.

However, I guess the problem disappears if only the link state is
propagated. The only caveat to that is to not propagate an "up" link
state while the macsec interface is administratively down, but
"remember" to propagate it later if the macsec interface is
administratively set to "up".

Thanks for the feedback!

Radu Rendec

^ permalink raw reply

* Re: Bridge connectivity interruptions while devices join or leave the bridge
From: Ido Schimmel @ 2018-09-19 16:45 UTC (permalink / raw)
  To: Johannes Wienke; +Cc: netdev
In-Reply-To: <cc7b7c8d-16bb-18a5-4b3a-91690acf47dc@techfak.uni-bielefeld.de>

On Wed, Sep 19, 2018 at 01:00:15PM +0200, Johannes Wienke wrote:
> This behavior of inheriting the mac address is really unexpected to us.
> Is it documented somewhere?

Not that I'm aware, but it's a well established behavior.

^ permalink raw reply

* Re: array bounds warning in xfrm_output_resume
From: David Ahern @ 2018-09-19 16:57 UTC (permalink / raw)
  To: Florian Westphal; +Cc: netdev@vger.kernel.org
In-Reply-To: <f116bb15-7fed-3b2e-4b20-a7a2c4317964@gmail.com>

On 6/18/18 11:10 AM, David Ahern wrote:
> Florian:
> 
> I am seeing this warning:
> 
> $ make O=kbuild/perf -j 24 -s
> In file included from /home/dsa/kernel-3.git/include/linux/kernel.h:10:0,
>                  from /home/dsa/kernel-3.git/include/linux/list.h:9,
>                  from /home/dsa/kernel-3.git/include/linux/module.h:9,
>                  from /home/dsa/kernel-3.git/net/xfrm/xfrm_output.c:13:
> /home/dsa/kernel-3.git/net/xfrm/xfrm_output.c: In function
> ‘xfrm_output_resume’:
> /home/dsa/kernel-3.git/include/linux/compiler.h:252:20: warning: array
> subscript is above array bounds [-Warray-bounds]
>    __read_once_size(&(x), __u.__c, sizeof(x));  \
>                     ^~~~
> /home/dsa/kernel-3.git/include/linux/compiler.h:258:22: note: in
> expansion of macro ‘__READ_ONCE’
>  #define READ_ONCE(x) __READ_ONCE(x, 1)
>                       ^~~~~~~~~~~
> /home/dsa/kernel-3.git/include/linux/rcupdate.h:350:48: note: in
> expansion of macro ‘READ_ONCE’
>   typeof(*p) *________p1 = (typeof(*p) *__force)READ_ONCE(p); \
>                                                 ^~~~~~~~~
> /home/dsa/kernel-3.git/include/linux/rcupdate.h:487:2: note: in
> expansion of macro ‘__rcu_dereference_check’
>   __rcu_dereference_check((p), (c) || rcu_read_lock_held(), __rcu)
>   ^~~~~~~~~~~~~~~~~~~~~~~
> /home/dsa/kernel-3.git/include/linux/rcupdate.h:545:28: note: in
> expansion of macro ‘rcu_dereference_check’
>  #define rcu_dereference(p) rcu_dereference_check(p, 0)
>                             ^~~~~~~~~~~~~~~~~~~~~
> /home/dsa/kernel-3.git/include/linux/netfilter.h:218:15: note: in
> expansion of macro ‘rcu_dereference’
>    hook_head = rcu_dereference(net->nf.hooks_arp[hook]);
>                ^~~~~~~~~~~~~~~
> 
> Line in question is the nf_hook in xfrm_output_resume.
> NF_INET_POST_ROUTING = 4 which is greater than NF_ARP_NUMHOOKS = 3
> 
> I believe ef57170bbfdd6 is the commit that introduced the warning
> 

Hi Florian:

I am still seeing this. I realize the compiler is not understanding the
conditions properly, but it is a distraction every time I do a kernel
build on Debian Stretch having to filter the above noise from whether I
introduce another warning.

^ permalink raw reply

* [PATCH net] bnxt_en: don't try to offload VLAN 'modify' action
From: Davide Caratti @ 2018-09-19 17:01 UTC (permalink / raw)
  To: Michael Chan, David S. Miller, netdev; +Cc: Sathya Perla

bnxt offload code currently supports only 'push' and 'pop' operation: let
.ndo_setup_tc() return -EOPNOTSUPP if VLAN 'modify' action is configured.

Fixes: 2ae7408fedfe ("bnxt_en: bnxt: add TC flower filter offload support")
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
 drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c
index 092c817f8f11..e1594c9df4c6 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_tc.c
@@ -75,17 +75,23 @@ static int bnxt_tc_parse_redir(struct bnxt *bp,
 	return 0;
 }
 
-static void bnxt_tc_parse_vlan(struct bnxt *bp,
-			       struct bnxt_tc_actions *actions,
-			       const struct tc_action *tc_act)
+static int bnxt_tc_parse_vlan(struct bnxt *bp,
+			      struct bnxt_tc_actions *actions,
+			      const struct tc_action *tc_act)
 {
-	if (tcf_vlan_action(tc_act) == TCA_VLAN_ACT_POP) {
+	switch (tcf_vlan_action(tc_act)) {
+	case TCA_VLAN_ACT_POP:
 		actions->flags |= BNXT_TC_ACTION_FLAG_POP_VLAN;
-	} else if (tcf_vlan_action(tc_act) == TCA_VLAN_ACT_PUSH) {
+		break;
+	case TCA_VLAN_ACT_PUSH:
 		actions->flags |= BNXT_TC_ACTION_FLAG_PUSH_VLAN;
 		actions->push_vlan_tci = htons(tcf_vlan_push_vid(tc_act));
 		actions->push_vlan_tpid = tcf_vlan_push_proto(tc_act);
+		break;
+	default:
+		return -EOPNOTSUPP;
 	}
+	return 0;
 }
 
 static int bnxt_tc_parse_tunnel_set(struct bnxt *bp,
@@ -134,7 +140,9 @@ static int bnxt_tc_parse_actions(struct bnxt *bp,
 
 		/* Push/pop VLAN */
 		if (is_tcf_vlan(tc_act)) {
-			bnxt_tc_parse_vlan(bp, actions, tc_act);
+			rc = bnxt_tc_parse_vlan(bp, actions, tc_act);
+			if (rc)
+				return rc;
 			continue;
 		}
 
-- 
2.17.1

^ permalink raw reply related

* Re: Bridge connectivity interruptions while devices join or leave the bridge
From: Stephen Hemminger @ 2018-09-19 17:03 UTC (permalink / raw)
  To: Ido Schimmel; +Cc: Johannes Wienke, netdev
In-Reply-To: <20180919164508.GA4975@splinter>

On Wed, 19 Sep 2018 19:45:08 +0300
Ido Schimmel <idosch@idosch.org> wrote:

> On Wed, Sep 19, 2018 at 01:00:15PM +0200, Johannes Wienke wrote:
> > This behavior of inheriting the mac address is really unexpected to us.
> > Is it documented somewhere?  
> 
> Not that I'm aware, but it's a well established behavior.

Not documented, has always been that way. It seems to be part of 802 standard maybe?
Anyway, if you set a MAC address of the bridge device it makes it sticky;
i.e it won't change if ports of bridge change.

^ permalink raw reply

* Re: [PATCH net-next] ipv6: Allow the l3mdev to be a loopback
From: David Ahern @ 2018-09-19 17:19 UTC (permalink / raw)
  To: Mike Manning, netdev; +Cc: Robert Shearman
In-Reply-To: <20180919125653.15913-1-mmanning@vyatta.att-mail.com>

On 9/19/18 5:56 AM, Mike Manning wrote:
> From: Robert Shearman <rshearma@vyatta.att-mail.com>
> 
> There is no way currently for an IPv6 client connect using a loopback
> address in a VRF, whereas for IPv4 the loopback address can be added:
> 
>     $ sudo ip addr add dev vrfred 127.0.0.1/8
>     $ sudo ip -6 addr add ::1/128 dev vrfred
>     RTNETLINK answers: Cannot assign requested address
> 
> So allow ::1 to be configured on an L3 master device. In order for
> this to be usable ip_route_output_flags needs to not consider ::1 to
> be a link scope address (since oif == l3mdev and so it would be
> dropped), and ipv6_rcv needs to consider the l3mdev to be a loopback
> device so that it doesn't drop the packets.
> 
> Signed-off-by: Robert Shearman <rshearma@vyatta.att-mail.com>
> Signed-off-by: Mike Manning <mmanning@vyatta.att-mail.com>
> ---
>  net/ipv6/addrconf.c  | 1 +
>  net/ipv6/ip6_input.c | 3 ++-
>  net/ipv6/route.c     | 3 ++-
>  3 files changed, 5 insertions(+), 2 deletions(-)
> 

Reviewed-by: David Ahern <dsahern@gmail.com>

Been on my to-do list for a while. Thanks for the patch. This resolves,
for example, a harmless error message from the 'host' command from
bind9-host-9.10.3 which probes for dscp support via the loopback
address. e.g.,

$ host www.google.com
../../../../lib/isc/unix/net.c:581: sendmsg() failed: Network is unreachable

^ permalink raw reply

* Re: [PATCH net-next v5 00/20] WireGuard: Secure Network Tunnel
From: Ard Biesheuvel @ 2018-09-19 17:21 UTC (permalink / raw)
  To: Jason A. Donenfeld
  Cc: Linux Kernel Mailing List, <netdev@vger.kernel.org>,
	open list:HARDWARE RANDOM NUMBER GENERATOR CORE, David S. Miller,
	Greg Kroah-Hartman
In-Reply-To: <20180918210120.GA29812@zx2c4.com>

On 18 September 2018 at 14:01, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> Hi Ard,
>
> On Tue, Sep 18, 2018 at 11:28:50AM -0700, Ard Biesheuvel wrote:
>> On 18 September 2018 at 09:16, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>> >   - While I initially wasn't going to do this for the initial
>> >     patchset, it was just so simple to do: now there's a nosimd
>> >     module parameter that can be used to disable simd instructions
>> >     for debugging and testing, or on weird systems.
>> >
>>
>> I was going to respond in the other thread but it is probably better
>> to move the discussion here.
>>
>> My concern about the monolithic nature of each algo module is not only
>> about SIMD, and it has nothing to do with weird systems. It has to do
>> with micro-architectural differences which are more common on ARM than
>> on other architectures *, I suppose. But generalizing from that, it
>> has to do with policy which is currently owned by userland and not by
>> the kernel. This will also be important for choosing between the time
>> variant but less safe table based scalar AES and the much slower time
>> invariant version (which is substantially slower, especially on
>> decryption) once we move AES into this library.
>>
>> So a command line option for the kernel is not the solution here. If
>> we can't have separate modules, could we at least have per-module
>> options that put the policy decisions back into userland?
>>
>> * as an example, the SHA256 NEON code I collaborated on with Andy
>> Polyakov 2 years ago is significantly faster on some cores and not on
>> others
>
> Interesting concern. There are micro-architectural quirks on x86 too
> that the current code actually already considers. Notably, we use an
> AVX-512VL path for Skylake-X but an AVX-512F path for Knights Landing
> and Coffee Lake and others, due to thermal throttling when touching the
> zmm registers on Skylake-X. So, in the code, we have it automatically
> select the right thing based on the micro-architecture.
>
> Is the same thing not possible with ARM? Do you not have access to this
> information already, such that the module can just always do the right
> thing and not require any user intervention?
>

That depends on what the right thing is. 'Fastest' does not
necessarily mean 'optimal', and I guess the thermal throttling on
Skylake-X may still result in the most power efficient implementation,
which may be the preferred one in some contexts.

The point is that this is a policy decision, and those belong in
userland not in the kernel.

> If so, that would be ideal. If not (and I'm curious to learn why not
> exactly), then indeed we could add some runtime nobs in /sys/module/
> {algo}/parameters/{nob}, or the like. This would be super easy to do,
> should we ever encounter a situation where we're unable to auto-detect
> the correct thing.
>
> Regards,
> Jason

^ permalink raw reply

* Re: [PATCH mlx5-next 02/25] net/mlx5: Set uid as part of QP commands
From: Jason Gunthorpe @ 2018-09-19 17:27 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: Doug Ledford, Leon Romanovsky, RDMA mailing list, Yishai Hadas,
	Saeed Mahameed, linux-netdev
In-Reply-To: <20180917110418.18937-3-leon@kernel.org>

On Mon, Sep 17, 2018 at 02:03:55PM +0300, Leon Romanovsky wrote:
> From: Yishai Hadas <yishaih@mellanox.com>
> 
> Set uid as part of QP commands so that the firmware can manage the
> QP object in a secured way.
> 
> That will enable using a QP that was created by verbs application to
> be used by the DEVX flow in case the uid is equal.
> 
> Signed-off-by: Yishai Hadas <yishaih@mellanox.com>
> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
>  drivers/net/ethernet/mellanox/mlx5/core/qp.c | 45 +++++++++++++++++-----------
>  include/linux/mlx5/mlx5_ifc.h                | 22 +++++++-------
>  include/linux/mlx5/qp.h                      |  1 +
>  3 files changed, 39 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/qp.c b/drivers/net/ethernet/mellanox/mlx5/core/qp.c
> index 4ca07bfb6b14..04f72a1cdbcc 100644
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/qp.c
> @@ -240,6 +240,7 @@ int mlx5_core_create_qp(struct mlx5_core_dev *dev,
>  	if (err)
>  		return err;
>  
> +	qp->uid = MLX5_GET(create_qp_in, in, uid);
>  	qp->qpn = MLX5_GET(create_qp_out, out, qpn);
>  	mlx5_core_dbg(dev, "qpn = 0x%x\n", qp->qpn);
>  
> @@ -261,6 +262,7 @@ int mlx5_core_create_qp(struct mlx5_core_dev *dev,
>  	memset(dout, 0, sizeof(dout));
>  	MLX5_SET(destroy_qp_in, din, opcode, MLX5_CMD_OP_DESTROY_QP);
>  	MLX5_SET(destroy_qp_in, din, qpn, qp->qpn);
> +	MLX5_SET(destroy_qp_in, din, uid, qp->uid);
>  	mlx5_cmd_exec(dev, din, sizeof(din), dout, sizeof(dout));
>  	return err;
>  }
> @@ -320,6 +322,7 @@ int mlx5_core_destroy_qp(struct mlx5_core_dev *dev,
>  
>  	MLX5_SET(destroy_qp_in, in, opcode, MLX5_CMD_OP_DESTROY_QP);
>  	MLX5_SET(destroy_qp_in, in, qpn, qp->qpn);
> +	MLX5_SET(destroy_qp_in, in, uid, qp->uid);
>  	err = mlx5_cmd_exec(dev, in, sizeof(in), out, sizeof(out));
>  	if (err)
>  		return err;
> @@ -373,7 +376,7 @@ static void mbox_free(struct mbox_info *mbox)
>  
>  static int modify_qp_mbox_alloc(struct mlx5_core_dev *dev, u16 opcode, int qpn,
>  				u32 opt_param_mask, void *qpc,
> -				struct mbox_info *mbox)
> +				struct mbox_info *mbox, u16 uid)
>  {
>  	mbox->out = NULL;
>  	mbox->in = NULL;
> @@ -381,26 +384,32 @@ static int modify_qp_mbox_alloc(struct mlx5_core_dev *dev, u16 opcode, int qpn,
>  #define MBOX_ALLOC(mbox, typ)  \
>  	mbox_alloc(mbox, MLX5_ST_SZ_BYTES(typ##_in), MLX5_ST_SZ_BYTES(typ##_out))
>  
> -#define MOD_QP_IN_SET(typ, in, _opcode, _qpn) \
> -	MLX5_SET(typ##_in, in, opcode, _opcode); \
> -	MLX5_SET(typ##_in, in, qpn, _qpn)
> -
> -#define MOD_QP_IN_SET_QPC(typ, in, _opcode, _qpn, _opt_p, _qpc) \
> -	MOD_QP_IN_SET(typ, in, _opcode, _qpn); \
> -	MLX5_SET(typ##_in, in, opt_param_mask, _opt_p); \
> -	memcpy(MLX5_ADDR_OF(typ##_in, in, qpc), _qpc, MLX5_ST_SZ_BYTES(qpc))
> +#define MOD_QP_IN_SET(typ, in, _opcode, _qpn, _uid) \
> +	do { \
> +		MLX5_SET(typ##_in, in, opcode, _opcode); \
> +		MLX5_SET(typ##_in, in, qpn, _qpn); \
> +		MLX5_SET(typ##_in, in, uid, _uid); \
> +	} while (0)
> +
> +#define MOD_QP_IN_SET_QPC(typ, in, _opcode, _qpn, _opt_p, _qpc, _uid) \
> +	do { \
> +		MOD_QP_IN_SET(typ, in, _opcode, _qpn, _uid); \
> +		MLX5_SET(typ##_in, in, opt_param_mask, _opt_p); \
> +		memcpy(MLX5_ADDR_OF(typ##_in, in, qpc), \
> +		       _qpc, MLX5_ST_SZ_BYTES(qpc)); \
> +	} while (0)

These should get touched by clang-format to follow the usual
formatting for macros..

Jason

^ permalink raw reply

* Re: [PATCH mlx5-next 03/25] net/mlx5: Set uid as part of RQ commands
From: Jason Gunthorpe @ 2018-09-19 17:28 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: Doug Ledford, Leon Romanovsky, RDMA mailing list, Yishai Hadas,
	Saeed Mahameed, linux-netdev
In-Reply-To: <20180917110418.18937-4-leon@kernel.org>

On Mon, Sep 17, 2018 at 02:03:56PM +0300, Leon Romanovsky wrote:
> From: Yishai Hadas <yishaih@mellanox.com>
> 
> Set uid as part of RQ commands so that the firmware can manage the
> RQ object in a secured way.
> 
> That will enable using an RQ that was created by verbs application
> to be used by the DEVX flow in case the uid is equal.
> 
> Signed-off-by: Yishai Hadas <yishaih@mellanox.com>
> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
>  drivers/net/ethernet/mellanox/mlx5/core/qp.c | 16 ++++++++++++++--
>  include/linux/mlx5/mlx5_ifc.h                |  6 +++---
>  2 files changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/qp.c b/drivers/net/ethernet/mellanox/mlx5/core/qp.c
> index 04f72a1cdbcc..0ca68ef54d93 100644
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/qp.c
> @@ -540,6 +540,17 @@ int mlx5_core_xrcd_dealloc(struct mlx5_core_dev *dev, u32 xrcdn)
>  }
>  EXPORT_SYMBOL_GPL(mlx5_core_xrcd_dealloc);
>  
> +static void destroy_rq_tracked(struct mlx5_core_dev *dev, u32 rqn, u16 uid)
> +{
> +	u32 in[MLX5_ST_SZ_DW(destroy_rq_in)]   = {0};
> +	u32 out[MLX5_ST_SZ_DW(destroy_rq_out)] = {0};

= {} is the preferred version of this, right? 

{0} explicitly initializes the first element to zero and only works if
the first element happens to be something integral..

Jason

^ permalink raw reply

* [PATCH net-next 01/12] net: hns3: Add default irq affinity
From: Salil Mehta @ 2018-09-19 17:29 UTC (permalink / raw)
  To: davem
  Cc: salil.mehta, yisen.zhuang, lipeng321, mehta.salil, netdev,
	linux-kernel, linuxarm
In-Reply-To: <20180919172958.12992-1-salil.mehta@huawei.com>

From: Peng Li <lipeng321@huawei.com>

All irq will float to cpu0 if do not set irq affinity.
This patch adds default irq affinity in hns3 driver, users can
also change the irq affinity in OS.

Signed-off-by: Peng Li <lipeng321@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
 drivers/net/ethernet/hisilicon/hns3/hns3_enet.c | 49 +++++++++++++++++++++++++
 drivers/net/ethernet/hisilicon/hns3/hns3_enet.h |  2 +
 2 files changed, 51 insertions(+)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
index 75e8ee9..052b7fb 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
@@ -66,6 +66,23 @@ static irqreturn_t hns3_irq_handle(int irq, void *vector)
 	return IRQ_HANDLED;
 }
 
+/* This callback function is used to set affinity changes to the irq affinity
+ * masks when the irq_set_affinity_notifier function is used.
+ */
+static void hns3_nic_irq_affinity_notify(struct irq_affinity_notify *notify,
+					 const cpumask_t *mask)
+{
+	struct hns3_enet_tqp_vector *tqp_vectors =
+		container_of(notify, struct hns3_enet_tqp_vector,
+			     affinity_notify);
+
+	tqp_vectors->affinity_mask = *mask;
+}
+
+static void hns3_nic_irq_affinity_release(struct kref *ref)
+{
+}
+
 static void hns3_nic_uninit_irq(struct hns3_nic_priv *priv)
 {
 	struct hns3_enet_tqp_vector *tqp_vectors;
@@ -77,6 +94,10 @@ static void hns3_nic_uninit_irq(struct hns3_nic_priv *priv)
 		if (tqp_vectors->irq_init_flag != HNS3_VECTOR_INITED)
 			continue;
 
+		/* clear the affinity notifier and affinity mask */
+		irq_set_affinity_notifier(tqp_vectors->vector_irq, NULL);
+		irq_set_affinity_hint(tqp_vectors->vector_irq, NULL);
+
 		/* release the irq resource */
 		free_irq(tqp_vectors->vector_irq, tqp_vectors);
 		tqp_vectors->irq_init_flag = HNS3_VECTOR_NOT_INITED;
@@ -127,6 +148,15 @@ static int hns3_nic_init_irq(struct hns3_nic_priv *priv)
 			return ret;
 		}
 
+		tqp_vectors->affinity_notify.notify =
+					hns3_nic_irq_affinity_notify;
+		tqp_vectors->affinity_notify.release =
+					hns3_nic_irq_affinity_release;
+		irq_set_affinity_notifier(tqp_vectors->vector_irq,
+					  &tqp_vectors->affinity_notify);
+		irq_set_affinity_hint(tqp_vectors->vector_irq,
+				      &tqp_vectors->affinity_mask);
+
 		tqp_vectors->irq_init_flag = HNS3_VECTOR_INITED;
 	}
 
@@ -2640,6 +2670,23 @@ static void hns3_add_ring_to_group(struct hns3_enet_ring_group *group,
 	group->count++;
 }
 
+static void hns3_nic_set_cpumask(struct hns3_nic_priv *priv)
+{
+	struct pci_dev *pdev = priv->ae_handle->pdev;
+	struct hns3_enet_tqp_vector *tqp_vector;
+	int num_vectors = priv->vector_num;
+	int numa_node;
+	int vector_i;
+
+	numa_node = dev_to_node(&pdev->dev);
+
+	for (vector_i = 0; vector_i < num_vectors; vector_i++) {
+		tqp_vector = &priv->tqp_vector[vector_i];
+		cpumask_set_cpu(cpumask_local_spread(vector_i, numa_node),
+				&tqp_vector->affinity_mask);
+	}
+}
+
 static int hns3_nic_init_vector_data(struct hns3_nic_priv *priv)
 {
 	struct hnae3_ring_chain_node vector_ring_chain;
@@ -2648,6 +2695,8 @@ static int hns3_nic_init_vector_data(struct hns3_nic_priv *priv)
 	int ret = 0;
 	u16 i;
 
+	hns3_nic_set_cpumask(priv);
+
 	for (i = 0; i < priv->vector_num; i++) {
 		tqp_vector = &priv->tqp_vector[i];
 		hns3_vector_gl_rl_init_hw(tqp_vector, priv);
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
index cb450d7..31fa21f 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
@@ -491,7 +491,9 @@ struct hns3_enet_tqp_vector {
 	struct hns3_enet_ring_group rx_group;
 	struct hns3_enet_ring_group tx_group;
 
+	cpumask_t affinity_mask;
 	u16 num_tqps;	/* total number of tqps in TQP vector */
+	struct irq_affinity_notify affinity_notify;
 
 	char name[HNAE3_INT_NAME_LEN];
 
-- 
2.7.4

^ permalink raw reply related

* [PATCH net-next 11/12] net: hns3: Fix client initialize state issue when roce client initialize failed
From: Salil Mehta @ 2018-09-19 17:29 UTC (permalink / raw)
  To: davem
  Cc: salil.mehta, yisen.zhuang, lipeng321, mehta.salil, netdev,
	linux-kernel, linuxarm, Jian Shen
In-Reply-To: <20180919172958.12992-1-salil.mehta@huawei.com>

From: Jian Shen <shenjian15@huawei.com>

When roce is loaded before nic, the roce client will not be initialized
until nic client is initialized, but roce init flag is set before it.
Furthermore, in this case of nic initialized success and roce failed,
the nic init flag is not set, and roce init flag is not cleared.

This patch fixes it by set init flag only after the client is initialized
successfully.

Fixes: e2cb1dec9779 ("net: hns3: Add HNS3 VF HCL(Hardware Compatibility Layer) Support")
Fixes: 46a3df9f9718 ("net: hns3: Add HNS3 Acceleration Engine & Compatibility Layer Support")
Signed-off-by: Jian Shen <shenjian15@huawei.com>
Signed-off-by: Peng Li <lipeng321@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
 drivers/net/ethernet/hisilicon/hns3/hnae3.c               | 12 +++++-------
 drivers/net/ethernet/hisilicon/hns3/hnae3.h               |  3 +++
 drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c   |  9 +++++++++
 drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c |  9 +++++++++
 4 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hnae3.c b/drivers/net/ethernet/hisilicon/hns3/hnae3.c
index fff5be8..781e5de 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hnae3.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hnae3.c
@@ -29,8 +29,8 @@ static bool hnae3_client_match(enum hnae3_client_type client_type,
 	return false;
 }
 
-static void hnae3_set_client_init_flag(struct hnae3_client *client,
-				       struct hnae3_ae_dev *ae_dev, int inited)
+void hnae3_set_client_init_flag(struct hnae3_client *client,
+				struct hnae3_ae_dev *ae_dev, int inited)
 {
 	switch (client->type) {
 	case HNAE3_CLIENT_KNIC:
@@ -46,6 +46,7 @@ static void hnae3_set_client_init_flag(struct hnae3_client *client,
 		break;
 	}
 }
+EXPORT_SYMBOL(hnae3_set_client_init_flag);
 
 static int hnae3_get_client_init_flag(struct hnae3_client *client,
 				       struct hnae3_ae_dev *ae_dev)
@@ -86,14 +87,11 @@ static int hnae3_match_n_instantiate(struct hnae3_client *client,
 	/* now, (un-)instantiate client by calling lower layer */
 	if (is_reg) {
 		ret = ae_dev->ops->init_client_instance(client, ae_dev);
-		if (ret) {
+		if (ret)
 			dev_err(&ae_dev->pdev->dev,
 				"fail to instantiate client, ret = %d\n", ret);
-			return ret;
-		}
 
-		hnae3_set_client_init_flag(client, ae_dev, 1);
-		return 0;
+		return ret;
 	}
 
 	if (hnae3_get_client_init_flag(client, ae_dev)) {
diff --git a/drivers/net/ethernet/hisilicon/hns3/hnae3.h b/drivers/net/ethernet/hisilicon/hns3/hnae3.h
index c48c187..17db631 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hnae3.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hnae3.h
@@ -523,4 +523,7 @@ void hnae3_register_ae_algo(struct hnae3_ae_algo *ae_algo);
 
 void hnae3_unregister_client(struct hnae3_client *client);
 int hnae3_register_client(struct hnae3_client *client);
+
+void hnae3_set_client_init_flag(struct hnae3_client *client,
+				struct hnae3_ae_dev *ae_dev, int inited);
 #endif
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
index 141da18..cf365d4 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
@@ -5171,6 +5171,8 @@ static int hclge_init_client_instance(struct hnae3_client *client,
 				goto clear_nic;
 			}
 
+			hnae3_set_client_init_flag(client, ae_dev, 1);
+
 			if (hdev->roce_client &&
 			    hnae3_dev_roce_supported(hdev)) {
 				struct hnae3_client *rc = hdev->roce_client;
@@ -5182,6 +5184,9 @@ static int hclge_init_client_instance(struct hnae3_client *client,
 				ret = rc->ops->init_instance(&vport->roce);
 				if (ret)
 					goto clear_roce;
+
+				hnae3_set_client_init_flag(hdev->roce_client,
+							   ae_dev, 1);
 			}
 
 			break;
@@ -5193,6 +5198,8 @@ static int hclge_init_client_instance(struct hnae3_client *client,
 			if (ret)
 				goto clear_nic;
 
+			hnae3_set_client_init_flag(client, ae_dev, 1);
+
 			break;
 		case HNAE3_CLIENT_ROCE:
 			if (hnae3_dev_roce_supported(hdev)) {
@@ -5208,6 +5215,8 @@ static int hclge_init_client_instance(struct hnae3_client *client,
 				ret = client->ops->init_instance(&vport->roce);
 				if (ret)
 					goto clear_roce;
+
+				hnae3_set_client_init_flag(client, ae_dev, 1);
 			}
 		}
 	}
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
index 88cff5f..166a2cf 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
@@ -1626,6 +1626,8 @@ static int hclgevf_init_client_instance(struct hnae3_client *client,
 		if (ret)
 			goto clear_nic;
 
+		hnae3_set_client_init_flag(client, ae_dev, 1);
+
 		if (hdev->roce_client && hnae3_dev_roce_supported(hdev)) {
 			struct hnae3_client *rc = hdev->roce_client;
 
@@ -1635,6 +1637,9 @@ static int hclgevf_init_client_instance(struct hnae3_client *client,
 			ret = rc->ops->init_instance(&hdev->roce);
 			if (ret)
 				goto clear_roce;
+
+			hnae3_set_client_init_flag(hdev->roce_client, ae_dev,
+						   1);
 		}
 		break;
 	case HNAE3_CLIENT_UNIC:
@@ -1644,6 +1649,8 @@ static int hclgevf_init_client_instance(struct hnae3_client *client,
 		ret = client->ops->init_instance(&hdev->nic);
 		if (ret)
 			goto clear_nic;
+
+		hnae3_set_client_init_flag(client, ae_dev, 1);
 		break;
 	case HNAE3_CLIENT_ROCE:
 		if (hnae3_dev_roce_supported(hdev)) {
@@ -1660,6 +1667,8 @@ static int hclgevf_init_client_instance(struct hnae3_client *client,
 			if (ret)
 				goto clear_roce;
 		}
+
+		hnae3_set_client_init_flag(client, ae_dev, 1);
 	}
 
 	return 0;
-- 
2.7.4

^ permalink raw reply related

* [PATCH net-next 10/12] net: hns3: Clear client pointer when initialize client failed or unintialize finished
From: Salil Mehta @ 2018-09-19 17:29 UTC (permalink / raw)
  To: davem
  Cc: salil.mehta, yisen.zhuang, lipeng321, mehta.salil, netdev,
	linux-kernel, linuxarm, Jian Shen
In-Reply-To: <20180919172958.12992-1-salil.mehta@huawei.com>

From: Jian Shen <shenjian15@huawei.com>

If initialize client failed or finish uninitializing client, we should
clear the client pointer. It may cause unexpected result when use
uninitialized client. Meanwhile, we also should check whether client
exist when uninitialize it.

Fixes: 46a3df9f9718 ("net: hns3: Add HNS3 Acceleration Engine & Compatibility Layer Support")
Signed-off-by: Jian Shen <shenjian15@huawei.com>
Signed-off-by: Peng Li <lipeng321@huawei.com>
Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
---
 .../ethernet/hisilicon/hns3/hns3pf/hclge_main.c    | 25 ++++++++++------
 .../ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c  | 33 ++++++++++++++++------
 2 files changed, 41 insertions(+), 17 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
index 8a50688..141da18 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
@@ -5162,13 +5162,13 @@ static int hclge_init_client_instance(struct hnae3_client *client,
 			vport->nic.client = client;
 			ret = client->ops->init_instance(&vport->nic);
 			if (ret)
-				return ret;
+				goto clear_nic;
 
 			ret = hclge_init_instance_hw(hdev);
 			if (ret) {
 			        client->ops->uninit_instance(&vport->nic,
 			                                     0);
-			        return ret;
+				goto clear_nic;
 			}
 
 			if (hdev->roce_client &&
@@ -5177,11 +5177,11 @@ static int hclge_init_client_instance(struct hnae3_client *client,
 
 				ret = hclge_init_roce_base_info(vport);
 				if (ret)
-					return ret;
+					goto clear_roce;
 
 				ret = rc->ops->init_instance(&vport->roce);
 				if (ret)
-					return ret;
+					goto clear_roce;
 			}
 
 			break;
@@ -5191,7 +5191,7 @@ static int hclge_init_client_instance(struct hnae3_client *client,
 
 			ret = client->ops->init_instance(&vport->nic);
 			if (ret)
-				return ret;
+				goto clear_nic;
 
 			break;
 		case HNAE3_CLIENT_ROCE:
@@ -5203,16 +5203,25 @@ static int hclge_init_client_instance(struct hnae3_client *client,
 			if (hdev->roce_client && hdev->nic_client) {
 				ret = hclge_init_roce_base_info(vport);
 				if (ret)
-					return ret;
+					goto clear_roce;
 
 				ret = client->ops->init_instance(&vport->roce);
 				if (ret)
-					return ret;
+					goto clear_roce;
 			}
 		}
 	}
 
 	return 0;
+
+clear_nic:
+	hdev->nic_client = NULL;
+	vport->nic.client = NULL;
+	return ret;
+clear_roce:
+	hdev->roce_client = NULL;
+	vport->roce.client = NULL;
+	return ret;
 }
 
 static void hclge_uninit_client_instance(struct hnae3_client *client,
@@ -5232,7 +5241,7 @@ static void hclge_uninit_client_instance(struct hnae3_client *client,
 		}
 		if (client->type == HNAE3_CLIENT_ROCE)
 			return;
-		if (client->ops->uninit_instance) {
+		if (hdev->nic_client && client->ops->uninit_instance) {
 			hclge_uninit_instance_hw(hdev);
 			client->ops->uninit_instance(&vport->nic, 0);
 			hdev->nic_client = NULL;
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
index 320043e..88cff5f 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
@@ -1624,17 +1624,17 @@ static int hclgevf_init_client_instance(struct hnae3_client *client,
 
 		ret = client->ops->init_instance(&hdev->nic);
 		if (ret)
-			return ret;
+			goto clear_nic;
 
 		if (hdev->roce_client && hnae3_dev_roce_supported(hdev)) {
 			struct hnae3_client *rc = hdev->roce_client;
 
 			ret = hclgevf_init_roce_base_info(hdev);
 			if (ret)
-				return ret;
+				goto clear_roce;
 			ret = rc->ops->init_instance(&hdev->roce);
 			if (ret)
-				return ret;
+				goto clear_roce;
 		}
 		break;
 	case HNAE3_CLIENT_UNIC:
@@ -1643,7 +1643,7 @@ static int hclgevf_init_client_instance(struct hnae3_client *client,
 
 		ret = client->ops->init_instance(&hdev->nic);
 		if (ret)
-			return ret;
+			goto clear_nic;
 		break;
 	case HNAE3_CLIENT_ROCE:
 		if (hnae3_dev_roce_supported(hdev)) {
@@ -1654,15 +1654,24 @@ static int hclgevf_init_client_instance(struct hnae3_client *client,
 		if (hdev->roce_client && hdev->nic_client) {
 			ret = hclgevf_init_roce_base_info(hdev);
 			if (ret)
-				return ret;
+				goto clear_roce;
 
 			ret = client->ops->init_instance(&hdev->roce);
 			if (ret)
-				return ret;
+				goto clear_roce;
 		}
 	}
 
 	return 0;
+
+clear_nic:
+	hdev->nic_client = NULL;
+	hdev->nic.client = NULL;
+	return ret;
+clear_roce:
+	hdev->roce_client = NULL;
+	hdev->roce.client = NULL;
+	return ret;
 }
 
 static void hclgevf_uninit_client_instance(struct hnae3_client *client,
@@ -1671,13 +1680,19 @@ static void hclgevf_uninit_client_instance(struct hnae3_client *client,
 	struct hclgevf_dev *hdev = ae_dev->priv;
 
 	/* un-init roce, if it exists */
-	if (hdev->roce_client)
+	if (hdev->roce_client) {
 		hdev->roce_client->ops->uninit_instance(&hdev->roce, 0);
+		hdev->roce_client = NULL;
+		hdev->roce.client = NULL;
+	}
 
 	/* un-init nic/unic, if this was not called by roce client */
-	if ((client->ops->uninit_instance) &&
-	    (client->type != HNAE3_CLIENT_ROCE))
+	if (client->ops->uninit_instance && hdev->nic_client &&
+	    client->type != HNAE3_CLIENT_ROCE) {
 		client->ops->uninit_instance(&hdev->nic, 0);
+		hdev->nic_client = NULL;
+		hdev->nic.client = NULL;
+	}
 }
 
 static int hclgevf_pci_init(struct hclgevf_dev *hdev)
-- 
2.7.4

^ permalink raw reply related

* Re: [PATCH mlx5-next 07/25] net/mlx5: Update mlx5_ifc with DEVX UID bits
From: Jason Gunthorpe @ 2018-09-19 17:31 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: Doug Ledford, Leon Romanovsky, RDMA mailing list, Yishai Hadas,
	Saeed Mahameed, linux-netdev
In-Reply-To: <20180917110418.18937-8-leon@kernel.org>

On Mon, Sep 17, 2018 at 02:04:00PM +0300, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@mellanox.com>
> 
> Add DEVX information to WQ, SRQ, CQ, TRI, TIS, QP,
> RQ, XRCD, PD, MKEY and MCG.
> 
> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> ---
>  include/linux/mlx5/mlx5_ifc.h | 67 +++++++++++++++++++++++++++----------------
>  1 file changed, 43 insertions(+), 24 deletions(-)

It is weird that we sometimes have these IFC bundle updates and
sometimes the IFC is inlined in the patch.. 

Why not just do one big IFC only patch with everything the series
needs?

Jason

^ permalink raw reply

* Re: array bounds warning in xfrm_output_resume
From: Florian Westphal @ 2018-09-19 17:45 UTC (permalink / raw)
  To: David Ahern; +Cc: Florian Westphal, netdev@vger.kernel.org
In-Reply-To: <8808f8a3-ea1c-f91b-8288-cc4952086f04@gmail.com>

David Ahern <dsahern@gmail.com> wrote:
> > I believe ef57170bbfdd6 is the commit that introduced the warning
> > 
> 
> Hi Florian:
> 
> I am still seeing this. I realize the compiler is not understanding the
> conditions properly, but it is a distraction every time I do a kernel
> build on Debian Stretch having to filter the above noise from whether I
> introduce another warning.

Sorry, I forgot about this.  I'll look into it tomorrow.

^ permalink raw reply

* Re: [PATCH] netfilter: nf_tables: add SECMARK support
From: Casey Schaufler @ 2018-09-19 23:36 UTC (permalink / raw)
  To: Christian Göttsche, pablo, kadlec, fw, davem,
	netfilter-devel, coreteam, netdev, linux-kernel, paul, sds,
	eparis, jmorris, serge, selinux, linux-security-module
In-Reply-To: <20180919231402.4482-1-cgzones@googlemail.com>

On 9/19/2018 4:14 PM, Christian Göttsche wrote:
> Add the ability to set the security context of packets within the nf_tables framework.
> Add a nft_object for holding security contexts in the kernel and manipulating packets on the wire.
> The contexts are kept as strings and are evaluated to security identifiers at runtime (packet arrival),
> so that the nft_objects do not need to be refreshed after security changes.
> The maximum security context length is set to 256.
>
> Based on v4.18.6
>
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>

I've only had a cursory look at your patch, but how is it
different from what's in xt_SECMARK.c ?

> ---
>  include/net/netfilter/nf_tables_core.h   |  4 +
>  include/uapi/linux/netfilter/nf_tables.h | 18 ++++-
>  net/netfilter/nf_tables_core.c           | 28 ++++++-
>  net/netfilter/nft_meta.c                 | 95 ++++++++++++++++++++++++
>  4 files changed, 140 insertions(+), 5 deletions(-)
>
> diff --git a/include/net/netfilter/nf_tables_core.h b/include/net/netfilter/nf_tables_core.h
> index a0513450..0d1f3b96 100644
> --- a/include/net/netfilter/nf_tables_core.h
> +++ b/include/net/netfilter/nf_tables_core.h
> @@ -16,6 +16,10 @@ extern struct nft_expr_type nft_meta_type;
>  extern struct nft_expr_type nft_rt_type;
>  extern struct nft_expr_type nft_exthdr_type;
>  
> +#ifdef CONFIG_NETWORK_SECMARK
> +extern struct nft_object_type nft_secmark_obj_type;
> +#endif
> +
>  int nf_tables_core_module_init(void);
>  void nf_tables_core_module_exit(void);
>  
> diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
> index 89438e68..f1527962 100644
> --- a/include/uapi/linux/netfilter/nf_tables.h
> +++ b/include/uapi/linux/netfilter/nf_tables.h
> @@ -1169,6 +1169,21 @@ enum nft_quota_attributes {
>  };
>  #define NFTA_QUOTA_MAX		(__NFTA_QUOTA_MAX - 1)
>  
> +/**
> + * enum nft_secmark_attributes - nf_tables secmark object netlink attributes
> + *
> + * @NFTA_SECMARK_CTX: security context (NLA_STRING)
> + */
> +enum nft_secmark_attributes {
> +	NFTA_SECMARK_UNSPEC,
> +	NFTA_SECMARK_CTX,
> +	__NFTA_SECMARK_MAX,
> +};
> +#define NFTA_SECMARK_MAX	(__NFTA_SECMARK_MAX - 1)
> +
> +/* Max security context length */
> +#define NFT_SECMARK_CTX_MAXLEN		256
> +
>  /**
>   * enum nft_reject_types - nf_tables reject expression reject types
>   *
> @@ -1398,7 +1413,8 @@ enum nft_ct_helper_attributes {
>  #define NFT_OBJECT_CT_HELPER	3
>  #define NFT_OBJECT_LIMIT	4
>  #define NFT_OBJECT_CONNLIMIT	5
> -#define __NFT_OBJECT_MAX	6
> +#define NFT_OBJECT_SECMARK	6
> +#define __NFT_OBJECT_MAX	7
>  #define NFT_OBJECT_MAX		(__NFT_OBJECT_MAX - 1)
>  
>  /**
> diff --git a/net/netfilter/nf_tables_core.c b/net/netfilter/nf_tables_core.c
> index 8de912ca..d59ebba0 100644
> --- a/net/netfilter/nf_tables_core.c
> +++ b/net/netfilter/nf_tables_core.c
> @@ -235,12 +235,24 @@ static struct nft_expr_type *nft_basic_types[] = {
>  	&nft_exthdr_type,
>  };
>  
> +static struct nft_object_type *nft_basic_objects[] = {
> +#ifdef CONFIG_NETWORK_SECMARK
> +	&nft_secmark_obj_type,
> +#endif
> +};
> +
>  int __init nf_tables_core_module_init(void)
>  {
> -	int err, i;
> +	int err, i, j = 0;
> +
> +	for (i = 0; i < ARRAY_SIZE(nft_basic_objects); i++) {
> +		err = nft_register_obj(nft_basic_objects[i]);
> +		if (err)
> +			goto err;
> +	}
>  
> -	for (i = 0; i < ARRAY_SIZE(nft_basic_types); i++) {
> -		err = nft_register_expr(nft_basic_types[i]);
> +	for (j = 0; j < ARRAY_SIZE(nft_basic_types); j++) {
> +		err = nft_register_expr(nft_basic_types[j]);
>  		if (err)
>  			goto err;
>  	}
> @@ -248,8 +260,12 @@ int __init nf_tables_core_module_init(void)
>  	return 0;
>  
>  err:
> +	while (j-- > 0)
> +		nft_unregister_expr(nft_basic_types[j]);
> +
>  	while (i-- > 0)
> -		nft_unregister_expr(nft_basic_types[i]);
> +		nft_unregister_obj(nft_basic_objects[i]);
> +
>  	return err;
>  }
>  
> @@ -260,4 +276,8 @@ void nf_tables_core_module_exit(void)
>  	i = ARRAY_SIZE(nft_basic_types);
>  	while (i-- > 0)
>  		nft_unregister_expr(nft_basic_types[i]);
> +
> +	i = ARRAY_SIZE(nft_basic_objects);
> +	while (i-- > 0)
> +		nft_unregister_obj(nft_basic_objects[i]);
>  }
> diff --git a/net/netfilter/nft_meta.c b/net/netfilter/nft_meta.c
> index 1105a23b..26b79a3c 100644
> --- a/net/netfilter/nft_meta.c
> +++ b/net/netfilter/nft_meta.c
> @@ -540,3 +540,98 @@ struct nft_expr_type nft_meta_type __read_mostly = {
>  	.maxattr	= NFTA_META_MAX,
>  	.owner		= THIS_MODULE,
>  };
> +
> +#ifdef CONFIG_NETWORK_SECMARK
> +
> +struct nft_secmark {
> +	char ctx[NFT_SECMARK_CTX_MAXLEN];
> +	int len;
> +};
> +
> +static const struct nla_policy nft_secmark_policy[NFTA_SECMARK_MAX + 1] = {
> +	[NFTA_SECMARK_CTX]     = { .type = NLA_STRING, .len = NFT_SECMARK_CTX_MAXLEN },
> +};
> +
> +
> +static void nft_secmark_obj_eval(struct nft_object *obj, struct nft_regs *regs, const struct nft_pktinfo *pkt)
> +{
> +	const struct nft_secmark *priv = nft_obj_data(obj);
> +	struct sk_buff *skb = pkt->skb;
> +	int err;
> +	u32 secid = 0;
> +
> +	/* skip if packet has already a secmark */
> +	if (skb->secmark)
> +		return;
> +
> +	err = security_secctx_to_secid(priv->ctx, priv->len, &secid);
> +	if (err) {
> +		if (err == -EINVAL)
> +			pr_notice_ratelimited("invalid security context \'%s\'\n", priv->ctx);
> +		else
> +			pr_notice_ratelimited("unable to convert security context \'%s\': %d\n", priv->ctx, -err);
> +		return;
> +	}
> +
> +	if (!secid) {
> +		pr_notice_ratelimited("unable to map security context \'%s\'\n", priv->ctx);
> +		return;
> +	}
> +
> +	err = security_secmark_relabel_packet(secid);
> +	if (err) {
> +		pr_notice_ratelimited("unable to obtain relabeling permission: %d\n", -err);
> +		return;
> +	}
> +
> +	skb->secmark = secid;
> +}
> +
> +
> +static int nft_secmark_obj_init(const struct nft_ctx *ctx, const struct nlattr * const tb[], struct nft_object *obj)
> +{
> +	struct nft_secmark *priv = nft_obj_data(obj);
> +
> +	if (tb[NFTA_SECMARK_CTX] == NULL)
> +		return -EINVAL;
> +
> +	nla_strlcpy(priv->ctx, tb[NFTA_SECMARK_CTX], NFT_SECMARK_CTX_MAXLEN);
> +	priv->len = strlen(priv->ctx);
> +
> +	security_secmark_refcount_inc();
> +
> +	return 0;
> +}
> +
> +static int nft_secmark_obj_dump(struct sk_buff *skb, struct nft_object *obj, bool reset)
> +{
> +	const struct nft_secmark *priv = nft_obj_data(obj);
> +
> +	if (nla_put_string(skb, NFTA_SECMARK_CTX, priv->ctx))
> +		return -1;
> +
> +	return 0;
> +}
> +
> +static void nft_secmark_obj_destroy(const struct nft_ctx *ctx, struct nft_object *obj)
> +{
> +	security_secmark_refcount_dec();
> +}
> +
> +static const struct nft_object_ops nft_secmark_obj_ops = {
> +	.type		= &nft_secmark_obj_type,
> +	.size		= sizeof(struct nft_secmark),
> +	.init		= nft_secmark_obj_init,
> +	.eval		= nft_secmark_obj_eval,
> +	.dump		= nft_secmark_obj_dump,
> +	.destroy	= nft_secmark_obj_destroy,
> +};
> +struct nft_object_type nft_secmark_obj_type __read_mostly = {
> +	.type		= NFT_OBJECT_SECMARK,
> +	.ops		= &nft_secmark_obj_ops,
> +	.maxattr	= NFTA_SECMARK_MAX,
> +	.policy		= nft_secmark_policy,
> +	.owner		= THIS_MODULE,
> +};
> +
> +#endif /* CONFIG_NETWORK_SECMARK */

^ permalink raw reply


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