Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH v3 iproute2 1/1] tc: fix conversion types when printing actions unsigned values
From: Stephen Hemminger @ 2018-03-27 16:02 UTC (permalink / raw)
  To: Roman Mashak; +Cc: netdev, jhs, xiyou.wangcong, jiri
In-Reply-To: <1521493541-31340-1-git-send-email-mrv@mojatatu.com>

On Mon, 19 Mar 2018 17:05:41 -0400
Roman Mashak <mrv@mojatatu.com> wrote:

> diff --git a/tc/m_gact.c b/tc/m_gact.c
> index 16c4413f4217..52022415db48 100644
> --- a/tc/m_gact.c
> +++ b/tc/m_gact.c
> @@ -194,7 +194,7 @@ print_gact(struct action_util *au, FILE *f, struct rtattr *arg)
>  	print_string(PRINT_ANY, "random_type", "\n\t random type %s",
>  		     prob_n2a(pp->ptype));
>  	print_action_control(f, " ", pp->paction, " ");
> -	print_int(PRINT_ANY, "val", "val %d", pp->pval);
> +	print_int(PRINT_ANY, "val", "val %u", pp->pval);
>  	close_json_object();

This needs to be print_uint in order to work correctly with json output.

Also, please consider doing json on all the match types in a later patch
for net-next.

^ permalink raw reply

* Re: [PATCH net 0/2] mlx4 misc fixes for 4.16
From: David Miller @ 2018-03-27 16:02 UTC (permalink / raw)
  To: tariqt; +Cc: netdev, eranbe
In-Reply-To: <1522150879-16501-1-git-send-email-tariqt@mellanox.com>

From: Tariq Toukan <tariqt@mellanox.com>
Date: Tue, 27 Mar 2018 14:41:17 +0300

> This patchset contains misc bug fixes from the team
> to the mlx4 Core and Eth drivers.
> 
> Patch 1 by Eran fixes a control mix of PFC and Global pauses, please queue it
> to -stable for >= v4.8.
> Patch 2 by Moshe fixes a resource leak in slave's delete flow, please queue it
> to -stable for >= v4.5.
> 
> Series generated against net commit:
> 3c82b372a9f4 net: dsa: mt7530: fix module autoloading for OF platform drivers

Series applied and queued up for -stable, thank you.

^ permalink raw reply

* Re: [PATCH iproute2 1/1] tc: print index, refcnt & bindcnt for nat action
From: Stephen Hemminger @ 2018-03-27 16:05 UTC (permalink / raw)
  To: Roman Mashak; +Cc: netdev, jhs, xiyou.wangcong, jiri
In-Reply-To: <1521570107-30024-1-git-send-email-mrv@mojatatu.com>

On Tue, 20 Mar 2018 14:21:47 -0400
Roman Mashak <mrv@mojatatu.com> wrote:

> Signed-off-by: Roman Mashak <mrv@mojatatu.com>
> ---
>  tc/m_nat.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/tc/m_nat.c b/tc/m_nat.c
> index 1e4ff51fe75a..f6e373957c1b 100644
> --- a/tc/m_nat.c
> +++ b/tc/m_nat.c
> @@ -169,6 +169,9 @@ print_nat(struct action_util *au, FILE * f, struct rtattr *arg)
>  		format_host_r(AF_INET, 4, &sel->new_addr, buf2, sizeof(buf2)));
>  	print_action_control(f, " ", sel->action, "");
>  
> +	fprintf(f, "\n\t index %u ref %d bind %d",
> +		sel->index, sel->refcnt, sel->bindcnt);
> +
>  	if (show_stats) {
>  		if (tb[TCA_NAT_TM]) {
>  			struct tcf_t *tm = RTA_DATA(tb[TCA_NAT_TM]);
> @@ -177,6 +180,8 @@ print_nat(struct action_util *au, FILE * f, struct rtattr *arg)
>  		}
>  	}
>  
> +	fprintf(f, "\n");
> +
>  	return 0;
>  }

Rather than printing newline all the time, you need to use _SL_ to keep the optional
oneline output format.

I.e
	fprintf(f, "%s\t index %u ref %d bind %d",
		_SL_, sel->index, sel->refcnt, sel->bindcnt);

^ permalink raw reply

* Re: [PATCH v6 bpf-next 07/11] tracepoint: introduce kernel_tracepoint_find_by_name
From: Mathieu Desnoyers @ 2018-03-27 16:09 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: rostedt, David S. Miller, Daniel Borkmann, Linus Torvalds,
	Peter Zijlstra, netdev, kernel-team, linux-api
In-Reply-To: <8600dbae-389d-dd01-f003-44f1af7dcf0a@fb.com>

----- On Mar 27, 2018, at 11:53 AM, Alexei Starovoitov ast@fb.com wrote:

> On 3/27/18 7:42 AM, Steven Rostedt wrote:
>> On Tue, 27 Mar 2018 10:18:24 -0400 (EDT)
>> Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote:
>>
>>> ----- On Mar 27, 2018, at 10:07 AM, rostedt rostedt@goodmis.org wrote:
>>>
>>>> On Mon, 26 Mar 2018 19:47:02 -0700
>>>> Alexei Starovoitov <ast@fb.com> wrote:
>>>>
>>>>> From: Alexei Starovoitov <ast@kernel.org>
>>>>>
>>>>> introduce kernel_tracepoint_find_by_name() helper to let bpf core
>>>>> find tracepoint by name and later attach bpf probe to a tracepoint
>>>>>
>>>>> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
>>>>
>>>> Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
>>>
>>> Steven showed preference for tracepoint_kernel_find_by_name() at some
>>> point (starting with a tracepoint_ prefix). I'm find with either of
>>> the names.
>>
>> Yeah, I do prefer tracepoint_kernel_find_by_name() to stay consistent
>> with the other tracepoint functions. But we have
>> "for_each_kernel_tracepoint()" and not "for_each_tracepoint_kernel()",
>> thus we need to pick being consistent with one or the other. One answer
>> is to use tracpoint_kernel_find_by_name() and rename the for_each to
>> for_each_tracpoint_kernel().
> 
> yep. that's exactly the reason I picked kernel_tracepoint_find_by_name()
> to match for_each_kernel_tracepoint() naming.
> 
> I can certainly send a follow up patch to rename both to
> *tracepoint_kernel* and then you can nack it because it breaks lttng :)

If Steven prefers changing the name of for_each_kernel_tracepoint() to
for_each_tracepoint_kernel(), I'll adapt LTTng accordingly. I don't
mind either way, as long as the change is justified.

Thanks,

Mathieu


> but let's do it in a separate thread.
> 
> Daniel,
> do you mind adding { } as Steven requested while applying or
> you want me to resubmit the whole thing?
> 
> Thanks!

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

^ permalink raw reply

* RE: [PATCH v2 iproute2-next 3/6] rdma: Add CM_ID resource tracking information
From: Steve Wise @ 2018-03-27 16:20 UTC (permalink / raw)
  To: 'Leon Romanovsky'
  Cc: 'Jason Gunthorpe', 'David Ahern', stephen, netdev,
	linux-rdma
In-Reply-To: <20180327160150.GB1877@mtr-leonro.local>

> On Tue, Mar 27, 2018 at 10:45:30AM -0500, Steve Wise wrote:
> >
> > >
> > > On Tue, Mar 27, 2018 at 06:15:44PM +0300, Leon Romanovsky wrote:
> > > > On Tue, Mar 27, 2018 at 08:44:55AM -0600, Jason Gunthorpe wrote:
> > > > > On Tue, Mar 27, 2018 at 06:21:41AM +0300, Leon Romanovsky
> wrote:
> > > > > > On Mon, Mar 26, 2018 at 04:30:33PM -0600, Jason Gunthorpe
> wrote:
> > > > > > > On Mon, Mar 26, 2018 at 04:34:44PM -0500, Steve Wise wrote:
> > > > > > > >
> > > > > > > > On 3/26/2018 4:15 PM, Jason Gunthorpe wrote:
> > > > > > > > > On Mon, Mar 26, 2018 at 09:30:41AM -0500, Steve Wise
> wrote:
> > > > > > > > >>
> > > > > > > > >> On 3/26/2018 9:17 AM, David Ahern wrote:
> > > > > > > > >>> On 2/27/18 9:07 AM, Steve Wise wrote:
> > > > > > > > >>>> diff --git a/rdma/rdma.h b/rdma/rdma.h
> > > > > > > > >>>> index 5809f70..e55205b 100644
> > > > > > > > >>>> +++ b/rdma/rdma.h
> > > > > > > > >>>> @@ -18,10 +18,12 @@
> > > > > > > > >>>>  #include <libmnl/libmnl.h>
> > > > > > > > >>>>  #include <rdma/rdma_netlink.h>
> > > > > > > > >>>>  #include <time.h>
> > > > > > > > >>>> +#include <net/if_arp.h>
> > > > > > > > >>>>
> > > > > > > > >>>>  #include "list.h"
> > > > > > > > >>>>  #include "utils.h"
> > > > > > > > >>>>  #include "json_writer.h"
> > > > > > > > >>>> +#include <rdma/rdma_cma.h>
> > > > > > > > >>>>
> > > > > > > > >>> did you forget to add rdma_cma.h? I don't see that file
in
> my
> > > repo.
> > > > > > > > >> It is provided by the rdma-core package, upon which rdma
> tool
> > > now
> > > > > > > > >> depends for the rdma_port_space enum.
> > > > > > > > > It is a kernel bug that enum is not in an
include/uapi/rdma
> > > header
> > > > > > > > >
> > > > > > > > > Fix it there and don't try to use rdma-core headers to get
> kernel
> > > ABI.
> > > > > > > > >
> > > > > > > > > Jason
> > > > > > > >
> > > > > > > > I wish you'd commented on this just a little sooner.  I just
> resent
> > > v3
> > > > > > > > of this series... with rdma_cma.h included. :)
> > > > > > > >
> > > > > > > > How about the restrack/nldev code just translates the port
> space
> > > from
> > > > > > > > enum rdma_port_space to a new ABI enum, say
> > > nldev_rdma_port_space, that
> > > > > > > > i add to rdma_netlink.h?  I'd hate to open the can of worms
of
> > > trying to
> > > > > > > > split rdma_cma.h into uabi and no uabi headers. :(
> > > > > > >
> > > > > > > If port space is already part of the ABI there isn't much
reason to
> > > > > > > translate it.
> > > > > > >
> > > > > > > You just need to pick the right header to put it in, since it
is a
> verbs
> > > > > > > define it doesn't belong in the netlink header.
> > > > > >
> > > > > > I completely understand Steve's concerns.
> > > > > >
> > > > > > I tried to do such thing (expose kernel headers) in first
incarnation
> of
> > > > > > rdmatool with attempt to clean IB/core as well to ensure that we
> > > won't expose
> > > > > > anything that is not implemented. It didn't go well.
> > > > >
> > > > > rdma-core is now using the kernel uapi/ headers natively, seems to
> be
> > > > > going OK. What problem did you face?
> > > >
> > > > I didn't agree to move to UAPI defines which are not implemented and
> > > not
> > > > used in the kernel, so I sent small number of patches similar to
those
> [1,
> > > 2].
> > > >
> > > > Those patches were rejected.
> > > >
> > > > So please don't mix Steve's need to use 3 defines with very large
and
> > > > painful task to expose proper UAPIs.
> > >
> > > Steve can just move the 3 defines he needs to the uapi, we are doing
> > > this incrementally..
> > >
> > > rdma_core does not define kernel ABI and it is totally wrong to use
> > > random constants from rdma_cma.h as kernel ABI.
> > >
> >
> > Proposal:
> >
> > Since the cm_id port space is part of the rdma_ucm_create_id struct in
> include/uapi/rdma/rdma_user_cm.h, I'll move the rdma_port_space enum
> there.  And then my iproute2 series will have to add a copy of
> rdma_user_cm.h locally into rdma/include/uapi/rdma, right?
> >
> > Will that work for everyone?
> 
> You need to remove _PS from that structure and from the kernel with
> justification that it is safe to do.
> 
> Thanks

I'm pretty sure port space is needed.  That struct is used to create a user
mode cm_id...

^ permalink raw reply

* [RFC PATCH iproute2] Drop capabilities if not running ip exec vrf with libcap
From: Luca Boccassi @ 2018-03-27 16:24 UTC (permalink / raw)
  To: netdev; +Cc: dsahern, luto, stephen, Luca Boccassi

ip vrf exec requires root or CAP_NET_ADMIN, CAP_SYS_ADMIN and
CAP_DAC_OVERRIDE. It is not possible to run unprivileged commands like
ping as non-root or non-cap-enabled due to this requirement.
To allow users and administrators to safely add the required
capabilities to the binary, drop all capabilities on start if not
invoked with "vrf exec".
Update the manpage with the requirements.

Signed-off-by: Luca Boccassi <bluca@debian.org>
---

I'd like to be able to run ip vrf exec as a normal user, does this approach
sound sensible? Any concerns? Are there any other alternatives?
It would be up to each user/admin/whatever to make sure the program is
built with libcap and to add the capabilities manually, so nothing will be
there by default.

 configure         | 17 +++++++++++++++++
 ip/ip.c           | 34 ++++++++++++++++++++++++++++++++++
 man/man8/ip-vrf.8 |  8 ++++++++
 3 files changed, 59 insertions(+)

diff --git a/configure b/configure
index f7c2d7a7..5ef5cd4c 100755
--- a/configure
+++ b/configure
@@ -336,6 +336,20 @@ EOF
     rm -f $TMPDIR/strtest.c $TMPDIR/strtest
 }
 
+check_cap()
+{
+	if ${PKG_CONFIG} libcap --exists
+	then
+		echo "HAVE_CAP:=y" >>$CONFIG
+		echo "yes"
+
+		echo 'CFLAGS += -DHAVE_LIBCAP' `${PKG_CONFIG} libcap --cflags` >>$CONFIG
+		echo 'LDLIBS +=' `${PKG_CONFIG} libcap --libs` >> $CONFIG
+	else
+		echo "no"
+	fi
+}
+
 quiet_config()
 {
 	cat <<EOF
@@ -410,6 +424,9 @@ check_berkeley_db
 echo -n "need for strlcpy: "
 check_strlcpy
 
+echo -n "libcap support: "
+check_cap
+
 echo >> $CONFIG
 echo "%.o: %.c" >> $CONFIG
 echo '	$(QUIET_CC)$(CC) $(CFLAGS) $(EXTRA_CFLAGS) -c -o $@ $<' >> $CONFIG
diff --git a/ip/ip.c b/ip/ip.c
index e0cd96cb..49739571 100644
--- a/ip/ip.c
+++ b/ip/ip.c
@@ -17,6 +17,10 @@
 #include <netinet/in.h>
 #include <string.h>
 #include <errno.h>
+#include <sys/types.h>
+#ifdef HAVE_LIBCAP
+#include <sys/capability.h>
+#endif
 
 #include "SNAPSHOT.h"
 #include "utils.h"
@@ -68,6 +72,24 @@ static int do_help(int argc, char **argv)
 	return 0;
 }
 
+static void drop_cap(void)
+{
+#ifdef HAVE_LIBCAP
+	/* don't harmstring root/sudo */
+	if (getuid() != 0 && geteuid() != 0) {
+		cap_t capabilities;
+		capabilities = cap_get_proc();
+		if (!capabilities)
+			exit(EXIT_FAILURE);
+		if (cap_clear(capabilities) != 0)
+			exit(EXIT_FAILURE);
+		if (cap_set_proc(capabilities) != 0)
+			exit(EXIT_FAILURE);
+		cap_free(capabilities);
+	}
+#endif
+}
+
 static const struct cmd {
 	const char *cmd;
 	int (*func)(int argc, char **argv);
@@ -174,6 +196,18 @@ int main(int argc, char **argv)
 	char *batch_file = NULL;
 	int color = 0;
 
+	/* to run vrf exec without root, capabilities might be set, drop them
+	 * if not needed as the first thing.
+	 * execv will drop them for the child command.
+	 * vrf exec requires:
+	 * - cap_dac_override to create the cgroup subdir in /sys
+	 * - cap_sys_admin to load the BPF program
+	 * - cap_net_admin to set the socket into the cgroup
+	 */
+	if (argc < 3 || strcmp(argv[1], "vrf") != 0 ||
+			strcmp(argv[2], "exec") != 0)
+		drop_cap();
+
 	basename = strrchr(argv[0], '/');
 	if (basename == NULL)
 		basename = argv[0];
diff --git a/man/man8/ip-vrf.8 b/man/man8/ip-vrf.8
index 18789339..1a42cebe 100644
--- a/man/man8/ip-vrf.8
+++ b/man/man8/ip-vrf.8
@@ -63,6 +63,14 @@ a VRF other than the default VRF (main table). A command can be run against
 the default VRF by passing the "default" as the VRF name. This is useful if
 the current shell is associated with another VRF (e.g, Management VRF).
 
+This command requires the system to be booted with cgroup v2 (e.g. with systemd,
+add systemd.unified_cgroup_hierarchy=1 to the kernel command line).
+
+This command also requires to be ran as root or with the CAP_SYS_ADMIN,
+CAP_NET_ADMIN and CAP_DAC_OVERRIDE capabilities. If built with libcap and if
+capabilities are added to the ip binary program via setcap, the program will
+drop them as the first thing when invoked, unless the command is vrf exec.
+
 .TP
 .B ip vrf identify [PID] - Report VRF association for process
 .sp
-- 
2.14.2

^ permalink raw reply related

* Re: [PATCH v2 iproute2-next 3/6] rdma: Add CM_ID resource tracking information
From: Leon Romanovsky @ 2018-03-27 16:30 UTC (permalink / raw)
  To: Steve Wise
  Cc: 'Jason Gunthorpe', 'David Ahern', stephen, netdev,
	linux-rdma
In-Reply-To: <026701d3c5e7$838c65d0$8aa53170$@opengridcomputing.com>

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

On Tue, Mar 27, 2018 at 11:20:25AM -0500, Steve Wise wrote:
> > On Tue, Mar 27, 2018 at 10:45:30AM -0500, Steve Wise wrote:
> > >
> > > >
> > > > On Tue, Mar 27, 2018 at 06:15:44PM +0300, Leon Romanovsky wrote:
> > > > > On Tue, Mar 27, 2018 at 08:44:55AM -0600, Jason Gunthorpe wrote:
> > > > > > On Tue, Mar 27, 2018 at 06:21:41AM +0300, Leon Romanovsky
> > wrote:
> > > > > > > On Mon, Mar 26, 2018 at 04:30:33PM -0600, Jason Gunthorpe
> > wrote:
> > > > > > > > On Mon, Mar 26, 2018 at 04:34:44PM -0500, Steve Wise wrote:
> > > > > > > > >
> > > > > > > > > On 3/26/2018 4:15 PM, Jason Gunthorpe wrote:
> > > > > > > > > > On Mon, Mar 26, 2018 at 09:30:41AM -0500, Steve Wise
> > wrote:
> > > > > > > > > >>
> > > > > > > > > >> On 3/26/2018 9:17 AM, David Ahern wrote:
> > > > > > > > > >>> On 2/27/18 9:07 AM, Steve Wise wrote:
> > > > > > > > > >>>> diff --git a/rdma/rdma.h b/rdma/rdma.h
> > > > > > > > > >>>> index 5809f70..e55205b 100644
> > > > > > > > > >>>> +++ b/rdma/rdma.h
> > > > > > > > > >>>> @@ -18,10 +18,12 @@
> > > > > > > > > >>>>  #include <libmnl/libmnl.h>
> > > > > > > > > >>>>  #include <rdma/rdma_netlink.h>
> > > > > > > > > >>>>  #include <time.h>
> > > > > > > > > >>>> +#include <net/if_arp.h>
> > > > > > > > > >>>>
> > > > > > > > > >>>>  #include "list.h"
> > > > > > > > > >>>>  #include "utils.h"
> > > > > > > > > >>>>  #include "json_writer.h"
> > > > > > > > > >>>> +#include <rdma/rdma_cma.h>
> > > > > > > > > >>>>
> > > > > > > > > >>> did you forget to add rdma_cma.h? I don't see that file
> in
> > my
> > > > repo.
> > > > > > > > > >> It is provided by the rdma-core package, upon which rdma
> > tool
> > > > now
> > > > > > > > > >> depends for the rdma_port_space enum.
> > > > > > > > > > It is a kernel bug that enum is not in an
> include/uapi/rdma
> > > > header
> > > > > > > > > >
> > > > > > > > > > Fix it there and don't try to use rdma-core headers to get
> > kernel
> > > > ABI.
> > > > > > > > > >
> > > > > > > > > > Jason
> > > > > > > > >
> > > > > > > > > I wish you'd commented on this just a little sooner.  I just
> > resent
> > > > v3
> > > > > > > > > of this series... with rdma_cma.h included. :)
> > > > > > > > >
> > > > > > > > > How about the restrack/nldev code just translates the port
> > space
> > > > from
> > > > > > > > > enum rdma_port_space to a new ABI enum, say
> > > > nldev_rdma_port_space, that
> > > > > > > > > i add to rdma_netlink.h?  I'd hate to open the can of worms
> of
> > > > trying to
> > > > > > > > > split rdma_cma.h into uabi and no uabi headers. :(
> > > > > > > >
> > > > > > > > If port space is already part of the ABI there isn't much
> reason to
> > > > > > > > translate it.
> > > > > > > >
> > > > > > > > You just need to pick the right header to put it in, since it
> is a
> > verbs
> > > > > > > > define it doesn't belong in the netlink header.
> > > > > > >
> > > > > > > I completely understand Steve's concerns.
> > > > > > >
> > > > > > > I tried to do such thing (expose kernel headers) in first
> incarnation
> > of
> > > > > > > rdmatool with attempt to clean IB/core as well to ensure that we
> > > > won't expose
> > > > > > > anything that is not implemented. It didn't go well.
> > > > > >
> > > > > > rdma-core is now using the kernel uapi/ headers natively, seems to
> > be
> > > > > > going OK. What problem did you face?
> > > > >
> > > > > I didn't agree to move to UAPI defines which are not implemented and
> > > > not
> > > > > used in the kernel, so I sent small number of patches similar to
> those
> > [1,
> > > > 2].
> > > > >
> > > > > Those patches were rejected.
> > > > >
> > > > > So please don't mix Steve's need to use 3 defines with very large
> and
> > > > > painful task to expose proper UAPIs.
> > > >
> > > > Steve can just move the 3 defines he needs to the uapi, we are doing
> > > > this incrementally..
> > > >
> > > > rdma_core does not define kernel ABI and it is totally wrong to use
> > > > random constants from rdma_cma.h as kernel ABI.
> > > >
> > >
> > > Proposal:
> > >
> > > Since the cm_id port space is part of the rdma_ucm_create_id struct in
> > include/uapi/rdma/rdma_user_cm.h, I'll move the rdma_port_space enum
> > there.  And then my iproute2 series will have to add a copy of
> > rdma_user_cm.h locally into rdma/include/uapi/rdma, right?
> > >
> > > Will that work for everyone?
> >
> > You need to remove _PS from that structure and from the kernel with
> > justification that it is safe to do.
> >
> > Thanks
>
> I'm pretty sure port space is needed.  That struct is used to create a user
> mode cm_id...

Sorry, it is RDMA_PS_SDP.

Thanks

>
>
>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* [net-next PATCH v2 00/10] Add support for netcp driver on K2G SoC
From: Murali Karicheri @ 2018-03-27 16:31 UTC (permalink / raw)
  To: robh+dt, mark.rutland, ssantosh, malat, w-kwok2, devicetree,
	linux-kernel, linux-arm-kernel, davem, netdev

K2G SoC is another variant of Keystone family of SoCs. This patch
series add support for NetCP driver on this SoC. The QMSS found on
K2G SoC is a cut down version of the QMSS found on other keystone
devices with less number of queues, internal link ram etc. The patch
series has 2 patch sets that goes into the drivers/soc and the
rest has to be applied to net sub system. Please review and merge
if this looks good.

Thanks

The boot logs on K2G ICE board (tftp boot over Ethernet)
is at https://pastebin.ubuntu.com/p/VQTv3c2XBS/

The boot logs on K2G GP board (tftp boot over Ethernet)
is at https://pastebin.ubuntu.com/p/6Vh55DW8vT/

This series applies to net-next master branch.

Change history:
  v2 - Addressed following comments on initial version
       - split patch 3/5 to multiple patches from Andrew Lunn

Murali Karicheri (10):
  soc: ti: K2G: enhancement to support QMSS in NSS
  soc: ti: K2G: provide APIs to support driver probe deferral
  net: netcp: ethss: make call to gbe_sgmii_config() conditional
  net: netcp: ethss: add support for handling sgmii link interface
  net: netcp: ethss: use rgmii link status for 2u cpsw hardware
  net: netcp: ethss: map vlan priorities to zero flow
  net: netcp: ethss: re-use stats handling code for 2u hardware
  net: netcp: ethss: use of_get_phy_mode() to support different RGMII
    modes
  Revert "net: netcp: remove dead code from the driver"
  net: netcp: support probe deferral

 .../bindings/soc/ti/keystone-navigator-qmss.txt    |  7 ++
 drivers/net/ethernet/ti/netcp.h                    |  2 +
 drivers/net/ethernet/ti/netcp_core.c               | 13 +++
 drivers/net/ethernet/ti/netcp_ethss.c              | 75 ++++++++++++++---
 drivers/soc/ti/knav_dma.c                          |  8 ++
 drivers/soc/ti/knav_qmss.h                         |  6 ++
 drivers/soc/ti/knav_qmss_queue.c                   | 98 +++++++++++++++++-----
 include/linux/soc/ti/knav_dma.h                    | 12 +++
 include/linux/soc/ti/knav_qmss.h                   |  1 +
 9 files changed, 189 insertions(+), 33 deletions(-)

-- 
1.9.1

^ permalink raw reply

* [net-next PATCH v2 01/10] soc: ti: K2G: enhancement to support QMSS in NSS
From: Murali Karicheri @ 2018-03-27 16:31 UTC (permalink / raw)
  To: robh+dt, mark.rutland, ssantosh, malat, w-kwok2, devicetree,
	linux-kernel, linux-arm-kernel, davem, netdev
In-Reply-To: <1522168309-12338-1-git-send-email-m-karicheri2@ti.com>

Navigator Subsystem (NSS) available on K2G SoC has a cut down
version of QMSS with less number of queues, internal linking ram
with lesser number of buffers etc.  It doesn't have status and
explicit push register space as in QMSS available on other K2 SoCs.
So define reg indices specific to QMSS on K2G. This patch introduces
"keystone-navigator-qmss-l" compatibility to identify QMSS on
K2G NSS (QMSS Lite) and to customize the dts handling code. Per
Device manual, descriptors with index less than or equal to
regions0_size is in region 0 in the case of QMSS where as for
QMSS Lite, descriptors with index less than regions0_size is in
region 0. So update the size accordingly in the regions0_size bits
of the linking ram size 0 register.

Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
Signed-off-by: WingMan Kwok <w-kwok2@ti.com>
---
 .../bindings/soc/ti/keystone-navigator-qmss.txt    |  7 ++
 drivers/soc/ti/knav_qmss.h                         |  6 ++
 drivers/soc/ti/knav_qmss_queue.c                   | 90 ++++++++++++++++------
 3 files changed, 81 insertions(+), 22 deletions(-)

diff --git a/Documentation/devicetree/bindings/soc/ti/keystone-navigator-qmss.txt b/Documentation/devicetree/bindings/soc/ti/keystone-navigator-qmss.txt
index 77cd42c..1b0878a 100644
--- a/Documentation/devicetree/bindings/soc/ti/keystone-navigator-qmss.txt
+++ b/Documentation/devicetree/bindings/soc/ti/keystone-navigator-qmss.txt
@@ -18,6 +18,7 @@ pool management.
 
 Required properties:
 - compatible	: Must be "ti,keystone-navigator-qmss";
+		: Must be "ti,keystone-navigator-qmss-l" for NSS Lite
 - clocks	: phandle to the reference clock for this device.
 - queue-range	: <start number> total range of queue numbers for the device.
 - linkram0	: <address size> for internal link ram, where size is the total
@@ -39,6 +40,12 @@ Required properties:
 			  - Descriptor memory setup region.
 			  - Queue Management/Queue Proxy region for queue Push.
 			  - Queue Management/Queue Proxy region for queue Pop.
+
+For NSS lite, following QMSS reg indexes are used in that order
+			  - Queue Peek region.
+			  - Queue configuration region.
+			  - Queue Management/Queue Proxy region for queue Push/Pop.
+
 - queue-pools	: child node classifying the queue ranges into pools.
 		  Queue ranges are grouped into 3 type of pools:
 		  - qpend	    : pool of qpend(interruptible) queues
diff --git a/drivers/soc/ti/knav_qmss.h b/drivers/soc/ti/knav_qmss.h
index 905b974..5fa1ce6 100644
--- a/drivers/soc/ti/knav_qmss.h
+++ b/drivers/soc/ti/knav_qmss.h
@@ -292,6 +292,11 @@ struct knav_queue {
 	struct list_head		list;
 };
 
+enum qmss_version {
+	QMSS,
+	QMSS_LITE,
+};
+
 struct knav_device {
 	struct device				*dev;
 	unsigned				base_id;
@@ -305,6 +310,7 @@ struct knav_device {
 	struct list_head			pools;
 	struct list_head			pdsps;
 	struct list_head			qmgrs;
+	enum qmss_version			version;
 };
 
 struct knav_range_ops {
diff --git a/drivers/soc/ti/knav_qmss_queue.c b/drivers/soc/ti/knav_qmss_queue.c
index 77d6b5c..5b2c4e9 100644
--- a/drivers/soc/ti/knav_qmss_queue.c
+++ b/drivers/soc/ti/knav_qmss_queue.c
@@ -42,6 +42,15 @@
 #define KNAV_QUEUE_PUSH_REG_INDEX	4
 #define KNAV_QUEUE_POP_REG_INDEX	5
 
+/* Queue manager register indices in DTS for QMSS in NSS lite.
+ * There are no status and vbusm push registers on this version
+ * QMSS. Push registers are same as pop, So all indices above 1
+ * are to be re-defined
+ */
+#define KNAV_L_QUEUE_CONFIG_REG_INDEX	1
+#define KNAV_L_QUEUE_REGION_REG_INDEX	2
+#define KNAV_L_QUEUE_PUSH_REG_INDEX	3
+
 /* PDSP register indices in DTS */
 #define KNAV_QUEUE_PDSP_IRAM_REG_INDEX	0
 #define KNAV_QUEUE_PDSP_REGS_REG_INDEX	1
@@ -1169,8 +1178,12 @@ static int knav_queue_setup_link_ram(struct knav_device *kdev)
 		dev_dbg(kdev->dev, "linkram0: dma:%pad, virt:%p, size:%x\n",
 			&block->dma, block->virt, block->size);
 		writel_relaxed((u32)block->dma, &qmgr->reg_config->link_ram_base0);
-		writel_relaxed(block->size, &qmgr->reg_config->link_ram_size0);
-
+		if (kdev->version == QMSS_LITE)
+			writel_relaxed(block->size,
+				       &qmgr->reg_config->link_ram_size0);
+		else
+			writel_relaxed(block->size - 1,
+				       &qmgr->reg_config->link_ram_size0);
 		block++;
 		if (!block->size)
 			continue;
@@ -1387,42 +1400,64 @@ static int knav_queue_init_qmgrs(struct knav_device *kdev,
 		qmgr->reg_peek =
 			knav_queue_map_reg(kdev, child,
 					   KNAV_QUEUE_PEEK_REG_INDEX);
-		qmgr->reg_status =
-			knav_queue_map_reg(kdev, child,
-					   KNAV_QUEUE_STATUS_REG_INDEX);
+
+		if (kdev->version == QMSS) {
+			qmgr->reg_status =
+				knav_queue_map_reg(kdev, child,
+						   KNAV_QUEUE_STATUS_REG_INDEX);
+		}
+
 		qmgr->reg_config =
 			knav_queue_map_reg(kdev, child,
+					   (kdev->version == QMSS_LITE) ?
+					   KNAV_L_QUEUE_CONFIG_REG_INDEX :
 					   KNAV_QUEUE_CONFIG_REG_INDEX);
 		qmgr->reg_region =
 			knav_queue_map_reg(kdev, child,
+					   (kdev->version == QMSS_LITE) ?
+					   KNAV_L_QUEUE_REGION_REG_INDEX :
 					   KNAV_QUEUE_REGION_REG_INDEX);
+
 		qmgr->reg_push =
 			knav_queue_map_reg(kdev, child,
-					   KNAV_QUEUE_PUSH_REG_INDEX);
-		qmgr->reg_pop =
-			knav_queue_map_reg(kdev, child,
-					   KNAV_QUEUE_POP_REG_INDEX);
+					   (kdev->version == QMSS_LITE) ?
+					    KNAV_L_QUEUE_PUSH_REG_INDEX :
+					    KNAV_QUEUE_PUSH_REG_INDEX);
+
+		if (kdev->version == QMSS) {
+			qmgr->reg_pop =
+				knav_queue_map_reg(kdev, child,
+						   KNAV_QUEUE_POP_REG_INDEX);
+		}
 
-		if (IS_ERR(qmgr->reg_peek) || IS_ERR(qmgr->reg_status) ||
+		if (IS_ERR(qmgr->reg_peek) ||
+		    ((kdev->version == QMSS) &&
+		    (IS_ERR(qmgr->reg_status) || IS_ERR(qmgr->reg_pop))) ||
 		    IS_ERR(qmgr->reg_config) || IS_ERR(qmgr->reg_region) ||
-		    IS_ERR(qmgr->reg_push) || IS_ERR(qmgr->reg_pop)) {
+		    IS_ERR(qmgr->reg_push)) {
 			dev_err(dev, "failed to map qmgr regs\n");
+			if (kdev->version == QMSS) {
+				if (!IS_ERR(qmgr->reg_status))
+					devm_iounmap(dev, qmgr->reg_status);
+				if (!IS_ERR(qmgr->reg_pop))
+					devm_iounmap(dev, qmgr->reg_pop);
+			}
 			if (!IS_ERR(qmgr->reg_peek))
 				devm_iounmap(dev, qmgr->reg_peek);
-			if (!IS_ERR(qmgr->reg_status))
-				devm_iounmap(dev, qmgr->reg_status);
 			if (!IS_ERR(qmgr->reg_config))
 				devm_iounmap(dev, qmgr->reg_config);
 			if (!IS_ERR(qmgr->reg_region))
 				devm_iounmap(dev, qmgr->reg_region);
 			if (!IS_ERR(qmgr->reg_push))
 				devm_iounmap(dev, qmgr->reg_push);
-			if (!IS_ERR(qmgr->reg_pop))
-				devm_iounmap(dev, qmgr->reg_pop);
 			devm_kfree(dev, qmgr);
 			continue;
 		}
 
+		/* Use same push register for pop as well */
+		if (kdev->version == QMSS_LITE)
+			qmgr->reg_pop = qmgr->reg_push;
+
 		list_add_tail(&qmgr->list, &kdev->qmgrs);
 		dev_info(dev, "added qmgr start queue %d, num of queues %d, reg_peek %p, reg_status %p, reg_config %p, reg_region %p, reg_push %p, reg_pop %p\n",
 			 qmgr->start_queue, qmgr->num_queues,
@@ -1681,10 +1716,24 @@ static int knav_queue_init_queues(struct knav_device *kdev)
 	return 0;
 }
 
+/* Match table for of_platform binding */
+static const struct of_device_id keystone_qmss_of_match[] = {
+	{
+		.compatible = "ti,keystone-navigator-qmss",
+	},
+	{
+		.compatible = "ti,keystone-navigator-qmss-l",
+		.data	= (void *)QMSS_LITE,
+	},
+	{},
+};
+MODULE_DEVICE_TABLE(of, keystone_qmss_of_match);
+
 static int knav_queue_probe(struct platform_device *pdev)
 {
 	struct device_node *node = pdev->dev.of_node;
 	struct device_node *qmgrs, *queue_pools, *regions, *pdsps;
+	const struct of_device_id *match;
 	struct device *dev = &pdev->dev;
 	u32 temp[2];
 	int ret;
@@ -1700,6 +1749,10 @@ static int knav_queue_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
+	match = of_match_device(of_match_ptr(keystone_qmss_of_match), dev);
+	if (match && match->data)
+		kdev->version = QMSS_LITE;
+
 	platform_set_drvdata(pdev, kdev);
 	kdev->dev = dev;
 	INIT_LIST_HEAD(&kdev->queue_ranges);
@@ -1815,13 +1868,6 @@ static int knav_queue_remove(struct platform_device *pdev)
 	return 0;
 }
 
-/* Match table for of_platform binding */
-static struct of_device_id keystone_qmss_of_match[] = {
-	{ .compatible = "ti,keystone-navigator-qmss", },
-	{},
-};
-MODULE_DEVICE_TABLE(of, keystone_qmss_of_match);
-
 static struct platform_driver keystone_qmss_driver = {
 	.probe		= knav_queue_probe,
 	.remove		= knav_queue_remove,
-- 
1.9.1

^ permalink raw reply related

* [net-next PATCH v2 02/10] soc: ti: K2G: provide APIs to support driver probe deferral
From: Murali Karicheri @ 2018-03-27 16:31 UTC (permalink / raw)
  To: robh+dt, mark.rutland, ssantosh, malat, w-kwok2, devicetree,
	linux-kernel, linux-arm-kernel, davem, netdev
In-Reply-To: <1522168309-12338-1-git-send-email-m-karicheri2@ti.com>

This patch provide APIs to allow client drivers to support
probe deferral. On K2G SoC, devices can be probed only
after the ti_sci_pm_domains driver is probed and ready.
As drivers may get probed at different order, any driver
that depends on knav dma and qmss drivers, for example
netcp network driver, needs to defer probe until
knav devices are probed and ready to service. To do this,
add an API to query the device ready status from the knav
dma and qmss devices.

Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
---
 drivers/soc/ti/knav_dma.c        |  8 ++++++++
 drivers/soc/ti/knav_qmss_queue.c |  8 ++++++++
 include/linux/soc/ti/knav_dma.h  | 12 ++++++++++++
 include/linux/soc/ti/knav_qmss.h |  1 +
 4 files changed, 29 insertions(+)

diff --git a/drivers/soc/ti/knav_dma.c b/drivers/soc/ti/knav_dma.c
index 026182d..224d7dd 100644
--- a/drivers/soc/ti/knav_dma.c
+++ b/drivers/soc/ti/knav_dma.c
@@ -134,6 +134,13 @@ struct knav_dma_chan {
 
 static struct knav_dma_pool_device *kdev;
 
+static bool device_ready;
+bool knav_dma_device_ready(void)
+{
+	return device_ready;
+}
+EXPORT_SYMBOL_GPL(knav_dma_device_ready);
+
 static bool check_config(struct knav_dma_chan *chan, struct knav_dma_cfg *cfg)
 {
 	if (!memcmp(&chan->cfg, cfg, sizeof(*cfg)))
@@ -773,6 +780,7 @@ static int knav_dma_probe(struct platform_device *pdev)
 	debugfs_create_file("knav_dma", S_IFREG | S_IRUGO, NULL, NULL,
 			    &knav_dma_debug_ops);
 
+	device_ready = true;
 	return ret;
 }
 
diff --git a/drivers/soc/ti/knav_qmss_queue.c b/drivers/soc/ti/knav_qmss_queue.c
index 5b2c4e9..f0e67aa 100644
--- a/drivers/soc/ti/knav_qmss_queue.c
+++ b/drivers/soc/ti/knav_qmss_queue.c
@@ -74,6 +74,13 @@
  */
 const char *knav_acc_firmwares[] = {"ks2_qmss_pdsp_acc48.bin"};
 
+static bool device_ready;
+bool knav_qmss_device_ready(void)
+{
+	return device_ready;
+}
+EXPORT_SYMBOL_GPL(knav_qmss_device_ready);
+
 /**
  * knav_queue_notify: qmss queue notfier call
  *
@@ -1849,6 +1856,7 @@ static int knav_queue_probe(struct platform_device *pdev)
 
 	debugfs_create_file("qmss", S_IFREG | S_IRUGO, NULL, NULL,
 			    &knav_queue_debug_ops);
+	device_ready = true;
 	return 0;
 
 err:
diff --git a/include/linux/soc/ti/knav_dma.h b/include/linux/soc/ti/knav_dma.h
index 66693bc..7127ec3 100644
--- a/include/linux/soc/ti/knav_dma.h
+++ b/include/linux/soc/ti/knav_dma.h
@@ -167,6 +167,8 @@ struct knav_dma_desc {
 void *knav_dma_open_channel(struct device *dev, const char *name,
 				struct knav_dma_cfg *config);
 void knav_dma_close_channel(void *channel);
+int knav_dma_get_flow(void *channel);
+bool knav_dma_device_ready(void);
 #else
 static inline void *knav_dma_open_channel(struct device *dev, const char *name,
 				struct knav_dma_cfg *config)
@@ -176,6 +178,16 @@ static inline void *knav_dma_open_channel(struct device *dev, const char *name,
 static inline void knav_dma_close_channel(void *channel)
 {}
 
+static inline int knav_dma_get_flow(void *channel)
+{
+	return -EINVAL;
+}
+
+static inline bool knav_dma_device_ready(void)
+{
+	return false;
+}
+
 #endif
 
 #endif /* __SOC_TI_KEYSTONE_NAVIGATOR_DMA_H__ */
diff --git a/include/linux/soc/ti/knav_qmss.h b/include/linux/soc/ti/knav_qmss.h
index 9f0ebb3b..9745df6 100644
--- a/include/linux/soc/ti/knav_qmss.h
+++ b/include/linux/soc/ti/knav_qmss.h
@@ -86,5 +86,6 @@ int knav_pool_desc_map(void *ph, void *desc, unsigned size,
 void *knav_pool_desc_unmap(void *ph, dma_addr_t dma, unsigned dma_sz);
 dma_addr_t knav_pool_desc_virt_to_dma(void *ph, void *virt);
 void *knav_pool_desc_dma_to_virt(void *ph, dma_addr_t dma);
+bool knav_qmss_device_ready(void);
 
 #endif /* __SOC_TI_KNAV_QMSS_H__ */
-- 
1.9.1

^ permalink raw reply related

* [net-next PATCH v2 03/10] net: netcp: ethss: make call to gbe_sgmii_config() conditional
From: Murali Karicheri @ 2018-03-27 16:31 UTC (permalink / raw)
  To: robh+dt, mark.rutland, ssantosh, malat, w-kwok2, devicetree,
	linux-kernel, linux-arm-kernel, davem, netdev
In-Reply-To: <1522168309-12338-1-git-send-email-m-karicheri2@ti.com>

As a preparatory patch to add support for 2u cpsw hardware found on
K2G SoC, make call to gbe_sgmii_config() conditional. This is required
since 2u uses RGMII interface instead of SGMII and to allow for driver
re-use.

Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
---
 drivers/net/ethernet/ti/netcp_ethss.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/ti/netcp_ethss.c b/drivers/net/ethernet/ti/netcp_ethss.c
index 56dbc0b..1dea891 100644
--- a/drivers/net/ethernet/ti/netcp_ethss.c
+++ b/drivers/net/ethernet/ti/netcp_ethss.c
@@ -2271,7 +2271,8 @@ static int gbe_slave_open(struct gbe_intf *gbe_intf)
 
 	void (*hndlr)(struct net_device *) = gbe_adjust_link;
 
-	gbe_sgmii_config(priv, slave);
+	if ((priv->ss_version == GBE_SS_VERSION_14) || IS_SS_ID_NU(priv))
+		gbe_sgmii_config(priv, slave);
 	gbe_port_reset(slave);
 	gbe_sgmii_rtreset(priv, slave, false);
 	gbe_port_config(priv, slave, priv->rx_packet_max);
@@ -3039,7 +3040,9 @@ static void init_secondary_ports(struct gbe_priv *gbe_dev,
 			continue;
 		}
 
-		gbe_sgmii_config(gbe_dev, slave);
+		if ((gbe_dev->ss_version == GBE_SS_VERSION_14) ||
+		    IS_SS_ID_NU(gbe_dev))
+			gbe_sgmii_config(gbe_dev, slave);
 		gbe_port_reset(slave);
 		gbe_port_config(gbe_dev, slave, gbe_dev->rx_packet_max);
 		list_add_tail(&slave->slave_list, &gbe_dev->secondary_slaves);
-- 
1.9.1

^ permalink raw reply related

* [net-next PATCH v2 04/10] net: netcp: ethss: add support for handling sgmii link interface
From: Murali Karicheri @ 2018-03-27 16:31 UTC (permalink / raw)
  To: robh+dt, mark.rutland, ssantosh, malat, w-kwok2, devicetree,
	linux-kernel, linux-arm-kernel, davem, netdev
In-Reply-To: <1522168309-12338-1-git-send-email-m-karicheri2@ti.com>

2u cpsw hardware on K2G uses sgmii link to interface with Phy. So add
support for this interface in the code so that drover can be re-used
for 2u hardware.

Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
---
 drivers/net/ethernet/ti/netcp.h       |  2 ++
 drivers/net/ethernet/ti/netcp_ethss.c | 18 ++++++++++++++----
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/ti/netcp.h b/drivers/net/ethernet/ti/netcp.h
index 8900a6f..416f732 100644
--- a/drivers/net/ethernet/ti/netcp.h
+++ b/drivers/net/ethernet/ti/netcp.h
@@ -33,6 +33,8 @@
 #define SGMII_LINK_MAC_MAC_FORCED	2
 #define SGMII_LINK_MAC_FIBER		3
 #define SGMII_LINK_MAC_PHY_NO_MDIO	4
+#define RGMII_LINK_MAC_PHY		5
+#define RGMII_LINK_MAC_PHY_NO_MDIO	7
 #define XGMII_LINK_MAC_PHY		10
 #define XGMII_LINK_MAC_MAC_FORCED	11
 
diff --git a/drivers/net/ethernet/ti/netcp_ethss.c b/drivers/net/ethernet/ti/netcp_ethss.c
index 1dea891..ab9d369 100644
--- a/drivers/net/ethernet/ti/netcp_ethss.c
+++ b/drivers/net/ethernet/ti/netcp_ethss.c
@@ -166,6 +166,9 @@
 #define	GBE_RXHOOK_ORDER			0
 #define GBE_DEFAULT_ALE_AGEOUT			30
 #define SLAVE_LINK_IS_XGMII(s) ((s)->link_interface >= XGMII_LINK_MAC_PHY)
+#define SLAVE_LINK_IS_RGMII(s) \
+	(((s)->link_interface >= RGMII_LINK_MAC_PHY) && \
+	 ((s)->link_interface <= RGMII_LINK_MAC_PHY_NO_MDIO))
 #define NETCP_LINK_STATE_INVALID		-1
 
 #define GBE_SET_REG_OFS(p, rb, rn) p->rb##_ofs.rn = \
@@ -2091,8 +2094,9 @@ static void netcp_ethss_link_state_action(struct gbe_priv *gbe_dev,
 				     ALE_PORT_STATE_FORWARD);
 
 		if (ndev && slave->open &&
-		    slave->link_interface != SGMII_LINK_MAC_PHY &&
-		    slave->link_interface != XGMII_LINK_MAC_PHY)
+		    ((slave->link_interface != SGMII_LINK_MAC_PHY) &&
+		    (slave->link_interface != RGMII_LINK_MAC_PHY) &&
+		    (slave->link_interface != XGMII_LINK_MAC_PHY)))
 			netif_carrier_on(ndev);
 	} else {
 		writel(mac_control, GBE_REG_ADDR(slave, emac_regs,
@@ -2101,8 +2105,9 @@ static void netcp_ethss_link_state_action(struct gbe_priv *gbe_dev,
 				     ALE_PORT_STATE,
 				     ALE_PORT_STATE_DISABLE);
 		if (ndev &&
-		    slave->link_interface != SGMII_LINK_MAC_PHY &&
-		    slave->link_interface != XGMII_LINK_MAC_PHY)
+		    ((slave->link_interface != SGMII_LINK_MAC_PHY) &&
+		    (slave->link_interface != RGMII_LINK_MAC_PHY) &&
+		    (slave->link_interface != XGMII_LINK_MAC_PHY)))
 			netif_carrier_off(ndev);
 	}
 
@@ -2914,6 +2919,7 @@ static int init_slave(struct gbe_priv *gbe_dev, struct gbe_slave *slave,
 
 	slave->open = false;
 	if ((slave->link_interface == SGMII_LINK_MAC_PHY) ||
+	    (slave->link_interface == RGMII_LINK_MAC_PHY) ||
 	    (slave->link_interface == XGMII_LINK_MAC_PHY))
 		slave->phy_node = of_parse_phandle(node, "phy-handle", 0);
 	slave->port_num = gbe_get_slave_port(gbe_dev, slave->slave_num);
@@ -3076,6 +3082,9 @@ static void init_secondary_ports(struct gbe_priv *gbe_dev,
 	if (slave->link_interface == SGMII_LINK_MAC_PHY) {
 		phy_mode = PHY_INTERFACE_MODE_SGMII;
 		slave->phy_port_t = PORT_MII;
+	} else if (slave->link_interface == RGMII_LINK_MAC_PHY) {
+		phy_mode = PHY_INTERFACE_MODE_RGMII;
+		slave->phy_port_t = PORT_MII;
 	} else {
 		phy_mode = PHY_INTERFACE_MODE_NA;
 		slave->phy_port_t = PORT_FIBRE;
@@ -3083,6 +3092,7 @@ static void init_secondary_ports(struct gbe_priv *gbe_dev,
 
 	for_each_sec_slave(slave, gbe_dev) {
 		if ((slave->link_interface != SGMII_LINK_MAC_PHY) &&
+		    (slave->link_interface != RGMII_LINK_MAC_PHY) &&
 		    (slave->link_interface != XGMII_LINK_MAC_PHY))
 			continue;
 		slave->phy =
-- 
1.9.1

^ permalink raw reply related

* [net-next PATCH v2 05/10] net: netcp: ethss: use rgmii link status for 2u cpsw hardware
From: Murali Karicheri @ 2018-03-27 16:31 UTC (permalink / raw)
  To: robh+dt, mark.rutland, ssantosh, malat, w-kwok2, devicetree,
	linux-kernel, linux-arm-kernel, davem, netdev
In-Reply-To: <1522168309-12338-1-git-send-email-m-karicheri2@ti.com>

Introduce rgmii link status to handle link state events for 2u
cpsw hardware on K2G.

Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
---
 drivers/net/ethernet/ti/netcp_ethss.c | 27 +++++++++++++++++++++++----
 1 file changed, 23 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/ti/netcp_ethss.c b/drivers/net/ethernet/ti/netcp_ethss.c
index ab9d369..078a1b8 100644
--- a/drivers/net/ethernet/ti/netcp_ethss.c
+++ b/drivers/net/ethernet/ti/netcp_ethss.c
@@ -552,6 +552,7 @@ struct gbe_ss_regs {
 struct gbe_ss_regs_ofs {
 	u16	id_ver;
 	u16	control;
+	u16	rgmii_status; /* 2U */
 };
 
 struct gbe_switch_regs {
@@ -2120,23 +2121,39 @@ static bool gbe_phy_link_status(struct gbe_slave *slave)
 	 return !slave->phy || slave->phy->link;
 }
 
+#define RGMII_REG_STATUS_LINK	BIT(0)
+
+static void netcp_2u_rgmii_get_port_link(struct gbe_priv *gbe_dev, bool *status)
+{
+	u32 val = 0;
+
+	val = readl(GBE_REG_ADDR(gbe_dev, ss_regs, rgmii_status));
+	*status = false;
+	if ((val & RGMII_REG_STATUS_LINK) != 0)
+		*status = true;
+}
+
 static void netcp_ethss_update_link_state(struct gbe_priv *gbe_dev,
 					  struct gbe_slave *slave,
 					  struct net_device *ndev)
 {
-	int sp = slave->slave_num;
-	int phy_link_state, sgmii_link_state = 1, link_state;
+	bool sw_link_state = true, phy_link_state;
+	int sp = slave->slave_num, link_state;
 
 	if (!slave->open)
 		return;
 
 	if (!SLAVE_LINK_IS_XGMII(slave)) {
-		sgmii_link_state =
+		if (SLAVE_LINK_IS_RGMII(slave))
+			netcp_2u_rgmii_get_port_link(gbe_dev,
+						     &sw_link_state);
+		else
+			sw_link_state =
 			netcp_sgmii_get_port_link(SGMII_BASE(gbe_dev, sp), sp);
 	}
 
 	phy_link_state = gbe_phy_link_status(slave);
-	link_state = phy_link_state & sgmii_link_state;
+	link_state = phy_link_state & sw_link_state;
 
 	if (atomic_xchg(&slave->link_state, link_state) != link_state)
 		netcp_ethss_link_state_action(gbe_dev, ndev, slave,
@@ -3432,6 +3449,8 @@ static int set_gbenu_ethss_priv(struct gbe_priv *gbe_dev,
 
 	/* Subsystem registers */
 	GBENU_SET_REG_OFS(gbe_dev, ss_regs, id_ver);
+	/* ok to set for MU, but used by 2U only */
+	GBENU_SET_REG_OFS(gbe_dev, ss_regs, rgmii_status);
 
 	/* Switch module registers */
 	GBENU_SET_REG_OFS(gbe_dev, switch_regs, id_ver);
-- 
1.9.1

^ permalink raw reply related

* [net-next PATCH v2 06/10] net: netcp: ethss: map vlan priorities to zero flow
From: Murali Karicheri @ 2018-03-27 16:31 UTC (permalink / raw)
  To: robh+dt, mark.rutland, ssantosh, malat, w-kwok2, devicetree,
	linux-kernel, linux-arm-kernel, davem, netdev
In-Reply-To: <1522168309-12338-1-git-send-email-m-karicheri2@ti.com>

The driver currently support only vlan priority zero. So map the
vlan priorities to zero flow in hardware.

Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
---
 drivers/net/ethernet/ti/netcp_ethss.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/net/ethernet/ti/netcp_ethss.c b/drivers/net/ethernet/ti/netcp_ethss.c
index 078a1b8..828243d 100644
--- a/drivers/net/ethernet/ti/netcp_ethss.c
+++ b/drivers/net/ethernet/ti/netcp_ethss.c
@@ -595,6 +595,7 @@ struct gbe_port_regs {
 struct gbe_port_regs_ofs {
 	u16	port_vlan;
 	u16	tx_pri_map;
+	u16     rx_pri_map;
 	u16	sa_lo;
 	u16	sa_hi;
 	u16	ts_ctl;
@@ -2299,6 +2300,13 @@ static int gbe_slave_open(struct gbe_intf *gbe_intf)
 	gbe_sgmii_rtreset(priv, slave, false);
 	gbe_port_config(priv, slave, priv->rx_packet_max);
 	gbe_set_slave_mac(slave, gbe_intf);
+	/* For NU & 2U switch, map the vlan priorities to zero
+	 * as we only configure to use priority 0
+	 */
+	if (IS_SS_ID_MU(priv))
+		writel(HOST_TX_PRI_MAP_DEFAULT,
+		       GBE_REG_ADDR(slave, port_regs, rx_pri_map));
+
 	/* enable forwarding */
 	cpsw_ale_control_set(priv->ale, slave->port_num,
 			     ALE_PORT_STATE, ALE_PORT_STATE_FORWARD);
@@ -3000,6 +3008,7 @@ static int init_slave(struct gbe_priv *gbe_dev, struct gbe_slave *slave,
 		/* Initialize  slave port register offsets */
 		GBENU_SET_REG_OFS(slave, port_regs, port_vlan);
 		GBENU_SET_REG_OFS(slave, port_regs, tx_pri_map);
+		GBENU_SET_REG_OFS(slave, port_regs, rx_pri_map);
 		GBENU_SET_REG_OFS(slave, port_regs, sa_lo);
 		GBENU_SET_REG_OFS(slave, port_regs, sa_hi);
 		GBENU_SET_REG_OFS(slave, port_regs, ts_ctl);
-- 
1.9.1

^ permalink raw reply related

* [net-next PATCH v2 07/10] net: netcp: ethss: re-use stats handling code for 2u hardware
From: Murali Karicheri @ 2018-03-27 16:31 UTC (permalink / raw)
  To: robh+dt, mark.rutland, ssantosh, malat, w-kwok2, devicetree,
	linux-kernel, linux-arm-kernel, davem, netdev
In-Reply-To: <1522168309-12338-1-git-send-email-m-karicheri2@ti.com>

The stats block in 2u cpsw hardware is similar to the one on nu
and hence handle it in a similar way by using a macro that includes
2u hardware as well.

Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
---
 drivers/net/ethernet/ti/netcp_ethss.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/ti/netcp_ethss.c b/drivers/net/ethernet/ti/netcp_ethss.c
index 828243d..26ccddb 100644
--- a/drivers/net/ethernet/ti/netcp_ethss.c
+++ b/drivers/net/ethernet/ti/netcp_ethss.c
@@ -3394,7 +3394,7 @@ static int set_gbenu_ethss_priv(struct gbe_priv *gbe_dev,
 	gbe_dev->num_stats_mods = gbe_dev->max_num_ports;
 	gbe_dev->et_stats = gbenu_et_stats;
 
-	if (IS_SS_ID_NU(gbe_dev))
+	if (IS_SS_ID_MU(gbe_dev))
 		gbe_dev->num_et_stats = GBENU_ET_STATS_HOST_SIZE +
 			(gbe_dev->max_num_slaves * GBENU_ET_STATS_PORT_SIZE);
 	else
-- 
1.9.1

^ permalink raw reply related

* [net-next PATCH v2 08/10] net: netcp: ethss: use of_get_phy_mode() to support different RGMII modes
From: Murali Karicheri @ 2018-03-27 16:31 UTC (permalink / raw)
  To: robh+dt, mark.rutland, ssantosh, malat, w-kwok2, devicetree,
	linux-kernel, linux-arm-kernel, davem, netdev
In-Reply-To: <1522168309-12338-1-git-send-email-m-karicheri2@ti.com>

The phy used for K2G allows for internal delays to be added optionally
to the clock circuitry based on board desing. To add this support,
enhance the driver to use of_get_phy_mode() to read the phy-mode from
the phy device and pass the same to phy through of_phy_connect().

Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
---
 drivers/net/ethernet/ti/netcp_ethss.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/net/ethernet/ti/netcp_ethss.c b/drivers/net/ethernet/ti/netcp_ethss.c
index 26ccddb..df163ec 100644
--- a/drivers/net/ethernet/ti/netcp_ethss.c
+++ b/drivers/net/ethernet/ti/netcp_ethss.c
@@ -21,6 +21,7 @@
 #include <linux/io.h>
 #include <linux/module.h>
 #include <linux/of_mdio.h>
+#include <linux/of_net.h>
 #include <linux/of_address.h>
 #include <linux/if_vlan.h>
 #include <linux/ptp_classify.h>
@@ -700,6 +701,7 @@ struct gbe_slave {
 	u32				link_interface;
 	u32				mac_control;
 	u8				phy_port_t;
+	struct device_node		*node;
 	struct device_node		*phy_node;
 	struct ts_ctl                   ts_ctl;
 	struct list_head		slave_list;
@@ -2317,6 +2319,15 @@ static int gbe_slave_open(struct gbe_intf *gbe_intf)
 		has_phy = true;
 		phy_mode = PHY_INTERFACE_MODE_SGMII;
 		slave->phy_port_t = PORT_MII;
+	} else if (slave->link_interface == RGMII_LINK_MAC_PHY) {
+		has_phy = true;
+		phy_mode = of_get_phy_mode(slave->node);
+		/* if phy-mode is not present, default to
+		 * PHY_INTERFACE_MODE_RGMII
+		 */
+		if (phy_mode < 0)
+			phy_mode = PHY_INTERFACE_MODE_RGMII;
+		slave->phy_port_t = PORT_MII;
 	} else if (slave->link_interface == XGMII_LINK_MAC_PHY) {
 		has_phy = true;
 		phy_mode = PHY_INTERFACE_MODE_NA;
@@ -2942,6 +2953,7 @@ static int init_slave(struct gbe_priv *gbe_dev, struct gbe_slave *slave,
 		slave->link_interface = SGMII_LINK_MAC_PHY;
 	}
 
+	slave->node = node;
 	slave->open = false;
 	if ((slave->link_interface == SGMII_LINK_MAC_PHY) ||
 	    (slave->link_interface == RGMII_LINK_MAC_PHY) ||
-- 
1.9.1

^ permalink raw reply related

* [net-next PATCH v2 09/10] Revert "net: netcp: remove dead code from the driver"
From: Murali Karicheri @ 2018-03-27 16:31 UTC (permalink / raw)
  To: robh+dt, mark.rutland, ssantosh, malat, w-kwok2, devicetree,
	linux-kernel, linux-arm-kernel, davem, netdev
In-Reply-To: <1522168309-12338-1-git-send-email-m-karicheri2@ti.com>

As the probe sequence is not guaranteed contrary to the assumption
of the commit 2d8e276a9030, same has to be reverted.

commit 2d8e276a9030 ("net: netcp: remove dead code from the driver")

Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
---
 drivers/net/ethernet/ti/netcp_core.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/net/ethernet/ti/netcp_core.c b/drivers/net/ethernet/ti/netcp_core.c
index f5a7eb2..9c51b25 100644
--- a/drivers/net/ethernet/ti/netcp_core.c
+++ b/drivers/net/ethernet/ti/netcp_core.c
@@ -2155,6 +2155,7 @@ static int netcp_probe(struct platform_device *pdev)
 	struct device_node *child, *interfaces;
 	struct netcp_device *netcp_device;
 	struct device *dev = &pdev->dev;
+	struct netcp_module *module;
 	int ret;
 
 	if (!node) {
@@ -2203,6 +2204,14 @@ static int netcp_probe(struct platform_device *pdev)
 	/* Add the device instance to the list */
 	list_add_tail(&netcp_device->device_list, &netcp_devices);
 
+	/* Probe & attach any modules already registered */
+	mutex_lock(&netcp_modules_lock);
+	for_each_netcp_module(module) {
+		ret = netcp_module_probe(netcp_device, module);
+		if (ret < 0)
+			dev_err(dev, "module(%s) probe failed\n", module->name);
+	}
+	mutex_unlock(&netcp_modules_lock);
 	return 0;
 
 probe_quit_interface:
-- 
1.9.1

^ permalink raw reply related

* [net-next PATCH v2 10/10] net: netcp: support probe deferral
From: Murali Karicheri @ 2018-03-27 16:31 UTC (permalink / raw)
  To: robh+dt, mark.rutland, ssantosh, malat, w-kwok2, devicetree,
	linux-kernel, linux-arm-kernel, davem, netdev
In-Reply-To: <1522168309-12338-1-git-send-email-m-karicheri2@ti.com>

The netcp driver shouldn't proceed until the knav qmss and dma
devices are ready. So return -EPROBE_DEFER if these devices are not
ready.

Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
---
 drivers/net/ethernet/ti/netcp_core.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/ti/netcp_core.c b/drivers/net/ethernet/ti/netcp_core.c
index 9c51b25..736f6f7 100644
--- a/drivers/net/ethernet/ti/netcp_core.c
+++ b/drivers/net/ethernet/ti/netcp_core.c
@@ -2158,6 +2158,10 @@ static int netcp_probe(struct platform_device *pdev)
 	struct netcp_module *module;
 	int ret;
 
+	if (!knav_dma_device_ready() ||
+	    !knav_qmss_device_ready())
+		return -EPROBE_DEFER;
+
 	if (!node) {
 		dev_err(dev, "could not find device info\n");
 		return -ENODEV;
-- 
1.9.1

^ permalink raw reply related

* Re: [PATCH v6 bpf-next 07/11] tracepoint: introduce kernel_tracepoint_find_by_name
From: Daniel Borkmann @ 2018-03-27 16:36 UTC (permalink / raw)
  To: Alexei Starovoitov, Steven Rostedt, Mathieu Desnoyers
  Cc: David S. Miller, Linus Torvalds, Peter Zijlstra, netdev,
	kernel-team, linux-api
In-Reply-To: <8600dbae-389d-dd01-f003-44f1af7dcf0a@fb.com>

On 03/27/2018 05:53 PM, Alexei Starovoitov wrote:
> On 3/27/18 7:42 AM, Steven Rostedt wrote:
>> On Tue, 27 Mar 2018 10:18:24 -0400 (EDT)
>> Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote:
>>> ----- On Mar 27, 2018, at 10:07 AM, rostedt rostedt@goodmis.org wrote:
>>>> On Mon, 26 Mar 2018 19:47:02 -0700
>>>> Alexei Starovoitov <ast@fb.com> wrote:
>>>>> From: Alexei Starovoitov <ast@kernel.org>
>>>>>
>>>>> introduce kernel_tracepoint_find_by_name() helper to let bpf core
>>>>> find tracepoint by name and later attach bpf probe to a tracepoint
>>>>>
>>>>> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
>>>>
>>>> Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
>>>
>>> Steven showed preference for tracepoint_kernel_find_by_name() at some
>>> point (starting with a tracepoint_ prefix). I'm find with either of
>>> the names.
>>
>> Yeah, I do prefer tracepoint_kernel_find_by_name() to stay consistent
>> with the other tracepoint functions. But we have
>> "for_each_kernel_tracepoint()" and not "for_each_tracepoint_kernel()",
>> thus we need to pick being consistent with one or the other. One answer
>> is to use tracpoint_kernel_find_by_name() and rename the for_each to
>> for_each_tracpoint_kernel().
> 
> yep. that's exactly the reason I picked kernel_tracepoint_find_by_name()
> to match for_each_kernel_tracepoint() naming.
> 
> I can certainly send a follow up patch to rename both to
> *tracepoint_kernel* and then you can nack it because it breaks lttng :)
> but let's do it in a separate thread.
> 
> Daniel,
> do you mind adding { } as Steven requested while applying or
> you want me to resubmit the whole thing?

Yeah, I can fix it up while applying.

^ permalink raw reply

* Re: [PATCH iproute2] Revert "iproute: "list/flush/save default" selected all of the routes"
From: Alexander Zubkov @ 2018-03-27 16:29 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Serhey Popovych, Luca Boccassi, netdev@vger.kernel.org
In-Reply-To: <20180327090135.7d60b796@xeon-e3>

Hi Stephen,

Looks like the new patch was applied after the revert of original patch and fix patch for 4.15 branch. Which is not correct and I did not test it. This is how patches were designed:
1) your revert patch - rolls back 4.15 branch to old behaviour by reverting the original patch
2) my patch for 4.15 - fixes problem is 4.15 branch, it does not require revert patch, it is an alternative solution for the problem, it is designed solely for version 4.15
3) my patch for master - fixes problem, it requires neither revert patch nor my patch for 4.15, it is standalone patch designed to do things right in master branch

27.03.2018, 18:01, "Stephen Hemminger" <stephen@networkplumber.org>:
> On Wed, 14 Mar 2018 21:26:40 +0100
> Alexander Zubkov <green@msu.ru> wrote:
>
>>  Hello,
>>
>>  For example, it can be fixed in such way (patch is below):
>>  - split handling of default and all/any
>>  - set needed attributes in get_addr: PREFIXLEN_SPECIFIED flag for default
>>  - and AF_UNSPEC for all/any
>>  In this case "ip route show default" shows only default route and "ip
>>  route show all" shows all routes. And both also work when family (-4 or
>>  -6) is specified.
>>  Serhey, does it goes in line with what you wanted to achieve? Because I
>>  do not know - may be there are reasons why all/any should be provided
>>  with specific family. If you think this solution is suitable, I'll do
>>  some additional tests and package the patch in a proper way for this
>>  mailing list.
>>  And I'm unsure if check for AF_DECnet and AF_MPLS should be kept in both
>>  branches. May be someone have some additional thoughts on that?
>
> I applied this to master.
>
> We can work on the other cases after that.

^ permalink raw reply

* RE: [PATCH v2 iproute2-next 3/6] rdma: Add CM_ID resource tracking information
From: Steve Wise @ 2018-03-27 16:38 UTC (permalink / raw)
  To: 'Leon Romanovsky'
  Cc: 'Jason Gunthorpe', 'David Ahern', stephen, netdev,
	linux-rdma
In-Reply-To: <20180327163023.GC1877@mtr-leonro.local>

> 
> On Tue, Mar 27, 2018 at 11:20:25AM -0500, Steve Wise wrote:
> > > On Tue, Mar 27, 2018 at 10:45:30AM -0500, Steve Wise wrote:
> > > >
> > > > >
> > > > > On Tue, Mar 27, 2018 at 06:15:44PM +0300, Leon Romanovsky wrote:
> > > > > > On Tue, Mar 27, 2018 at 08:44:55AM -0600, Jason Gunthorpe
> wrote:
> > > > > > > On Tue, Mar 27, 2018 at 06:21:41AM +0300, Leon Romanovsky
> > > wrote:
> > > > > > > > On Mon, Mar 26, 2018 at 04:30:33PM -0600, Jason Gunthorpe
> > > wrote:
> > > > > > > > > On Mon, Mar 26, 2018 at 04:34:44PM -0500, Steve Wise
> wrote:
> > > > > > > > > >
> > > > > > > > > > On 3/26/2018 4:15 PM, Jason Gunthorpe wrote:
> > > > > > > > > > > On Mon, Mar 26, 2018 at 09:30:41AM -0500, Steve Wise
> > > wrote:
> > > > > > > > > > >>
> > > > > > > > > > >> On 3/26/2018 9:17 AM, David Ahern wrote:
> > > > > > > > > > >>> On 2/27/18 9:07 AM, Steve Wise wrote:
> > > > > > > > > > >>>> diff --git a/rdma/rdma.h b/rdma/rdma.h
> > > > > > > > > > >>>> index 5809f70..e55205b 100644
> > > > > > > > > > >>>> +++ b/rdma/rdma.h
> > > > > > > > > > >>>> @@ -18,10 +18,12 @@
> > > > > > > > > > >>>>  #include <libmnl/libmnl.h>
> > > > > > > > > > >>>>  #include <rdma/rdma_netlink.h>
> > > > > > > > > > >>>>  #include <time.h>
> > > > > > > > > > >>>> +#include <net/if_arp.h>
> > > > > > > > > > >>>>
> > > > > > > > > > >>>>  #include "list.h"
> > > > > > > > > > >>>>  #include "utils.h"
> > > > > > > > > > >>>>  #include "json_writer.h"
> > > > > > > > > > >>>> +#include <rdma/rdma_cma.h>
> > > > > > > > > > >>>>
> > > > > > > > > > >>> did you forget to add rdma_cma.h? I don't see that
file
> > in
> > > my
> > > > > repo.
> > > > > > > > > > >> It is provided by the rdma-core package, upon which
> rdma
> > > tool
> > > > > now
> > > > > > > > > > >> depends for the rdma_port_space enum.
> > > > > > > > > > > It is a kernel bug that enum is not in an
> > include/uapi/rdma
> > > > > header
> > > > > > > > > > >
> > > > > > > > > > > Fix it there and don't try to use rdma-core headers to
get
> > > kernel
> > > > > ABI.
> > > > > > > > > > >
> > > > > > > > > > > Jason
> > > > > > > > > >
> > > > > > > > > > I wish you'd commented on this just a little sooner.  I
just
> > > resent
> > > > > v3
> > > > > > > > > > of this series... with rdma_cma.h included. :)
> > > > > > > > > >
> > > > > > > > > > How about the restrack/nldev code just translates the
port
> > > space
> > > > > from
> > > > > > > > > > enum rdma_port_space to a new ABI enum, say
> > > > > nldev_rdma_port_space, that
> > > > > > > > > > i add to rdma_netlink.h?  I'd hate to open the can of
worms
> > of
> > > > > trying to
> > > > > > > > > > split rdma_cma.h into uabi and no uabi headers. :(
> > > > > > > > >
> > > > > > > > > If port space is already part of the ABI there isn't much
> > reason to
> > > > > > > > > translate it.
> > > > > > > > >
> > > > > > > > > You just need to pick the right header to put it in, since
it
> > is a
> > > verbs
> > > > > > > > > define it doesn't belong in the netlink header.
> > > > > > > >
> > > > > > > > I completely understand Steve's concerns.
> > > > > > > >
> > > > > > > > I tried to do such thing (expose kernel headers) in first
> > incarnation
> > > of
> > > > > > > > rdmatool with attempt to clean IB/core as well to ensure
that
> we
> > > > > won't expose
> > > > > > > > anything that is not implemented. It didn't go well.
> > > > > > >
> > > > > > > rdma-core is now using the kernel uapi/ headers natively,
seems
> to
> > > be
> > > > > > > going OK. What problem did you face?
> > > > > >
> > > > > > I didn't agree to move to UAPI defines which are not implemented
> and
> > > > > not
> > > > > > used in the kernel, so I sent small number of patches similar to
> > those
> > > [1,
> > > > > 2].
> > > > > >
> > > > > > Those patches were rejected.
> > > > > >
> > > > > > So please don't mix Steve's need to use 3 defines with very
large
> > and
> > > > > > painful task to expose proper UAPIs.
> > > > >
> > > > > Steve can just move the 3 defines he needs to the uapi, we are
doing
> > > > > this incrementally..
> > > > >
> > > > > rdma_core does not define kernel ABI and it is totally wrong to
use
> > > > > random constants from rdma_cma.h as kernel ABI.
> > > > >
> > > >
> > > > Proposal:
> > > >
> > > > Since the cm_id port space is part of the rdma_ucm_create_id struct
> in
> > > include/uapi/rdma/rdma_user_cm.h, I'll move the rdma_port_space
> enum
> > > there.  And then my iproute2 series will have to add a copy of
> > > rdma_user_cm.h locally into rdma/include/uapi/rdma, right?
> > > >
> > > > Will that work for everyone?
> > >
> > > You need to remove _PS from that structure and from the kernel with
> > > justification that it is safe to do.
> > >
> > > Thanks
> >
> > I'm pretty sure port space is needed.  That struct is used to create a
user
> > mode cm_id...
> 
> Sorry, it is RDMA_PS_SDP.
> 
> Thanks

Right.  Will do.

^ permalink raw reply

* Re: [RFC PATCH iproute2] Drop capabilities if not running ip exec vrf with libcap
From: David Ahern @ 2018-03-27 16:40 UTC (permalink / raw)
  To: Luca Boccassi, netdev; +Cc: luto, stephen
In-Reply-To: <20180327162419.8962-1-bluca@debian.org>

On 3/27/18 10:24 AM, Luca Boccassi wrote:
> ip vrf exec requires root or CAP_NET_ADMIN, CAP_SYS_ADMIN and
> CAP_DAC_OVERRIDE. It is not possible to run unprivileged commands like
> ping as non-root or non-cap-enabled due to this requirement.
> To allow users and administrators to safely add the required
> capabilities to the binary, drop all capabilities on start if not
> invoked with "vrf exec".
> Update the manpage with the requirements.
> 
> Signed-off-by: Luca Boccassi <bluca@debian.org>
> ---
> 
> I'd like to be able to run ip vrf exec as a normal user, does this approach
> sound sensible? Any concerns? Are there any other alternatives?
> It would be up to each user/admin/whatever to make sure the program is
> built with libcap and to add the capabilities manually, so nothing will be
> there by default.

This is very similar to a change I recently made for our distribution. I
created a separate 'runvrf' command so as to not contaminate 'ip' (the
runvrf command has the limitation it can not configure the VRF cgroup so
that needs to be done before runvrf).

> 
>  configure         | 17 +++++++++++++++++
>  ip/ip.c           | 34 ++++++++++++++++++++++++++++++++++
>  man/man8/ip-vrf.8 |  8 ++++++++
>  3 files changed, 59 insertions(+)
> 
> diff --git a/configure b/configure
> index f7c2d7a7..5ef5cd4c 100755
> --- a/configure
> +++ b/configure
> @@ -336,6 +336,20 @@ EOF
>      rm -f $TMPDIR/strtest.c $TMPDIR/strtest
>  }
>  
> +check_cap()
> +{
> +	if ${PKG_CONFIG} libcap --exists
> +	then
> +		echo "HAVE_CAP:=y" >>$CONFIG
> +		echo "yes"
> +
> +		echo 'CFLAGS += -DHAVE_LIBCAP' `${PKG_CONFIG} libcap --cflags` >>$CONFIG
> +		echo 'LDLIBS +=' `${PKG_CONFIG} libcap --libs` >> $CONFIG
> +	else
> +		echo "no"
> +	fi
> +}
> +
>  quiet_config()
>  {
>  	cat <<EOF
> @@ -410,6 +424,9 @@ check_berkeley_db
>  echo -n "need for strlcpy: "
>  check_strlcpy
>  
> +echo -n "libcap support: "
> +check_cap
> +
>  echo >> $CONFIG
>  echo "%.o: %.c" >> $CONFIG
>  echo '	$(QUIET_CC)$(CC) $(CFLAGS) $(EXTRA_CFLAGS) -c -o $@ $<' >> $CONFIG
> diff --git a/ip/ip.c b/ip/ip.c
> index e0cd96cb..49739571 100644
> --- a/ip/ip.c
> +++ b/ip/ip.c
> @@ -17,6 +17,10 @@
>  #include <netinet/in.h>
>  #include <string.h>
>  #include <errno.h>
> +#include <sys/types.h>
> +#ifdef HAVE_LIBCAP
> +#include <sys/capability.h>
> +#endif
>  
>  #include "SNAPSHOT.h"
>  #include "utils.h"
> @@ -68,6 +72,24 @@ static int do_help(int argc, char **argv)
>  	return 0;
>  }
>  
> +static void drop_cap(void)
> +{
> +#ifdef HAVE_LIBCAP
> +	/* don't harmstring root/sudo */
> +	if (getuid() != 0 && geteuid() != 0) {
> +		cap_t capabilities;
> +		capabilities = cap_get_proc();
> +		if (!capabilities)
> +			exit(EXIT_FAILURE);
> +		if (cap_clear(capabilities) != 0)
> +			exit(EXIT_FAILURE);
> +		if (cap_set_proc(capabilities) != 0)
> +			exit(EXIT_FAILURE);
> +		cap_free(capabilities);
> +	}
> +#endif
> +}

You don't need the capabilities after the cgroup has been changed, so
you can add a call to drop_cap at the end of vrf_switch.

> +
>  static const struct cmd {
>  	const char *cmd;
>  	int (*func)(int argc, char **argv);
> @@ -174,6 +196,18 @@ int main(int argc, char **argv)
>  	char *batch_file = NULL;
>  	int color = 0;
>  
> +	/* to run vrf exec without root, capabilities might be set, drop them
> +	 * if not needed as the first thing.
> +	 * execv will drop them for the child command.
> +	 * vrf exec requires:
> +	 * - cap_dac_override to create the cgroup subdir in /sys
> +	 * - cap_sys_admin to load the BPF program
> +	 * - cap_net_admin to set the socket into the cgroup
> +	 */
> +	if (argc < 3 || strcmp(argv[1], "vrf") != 0 ||
> +			strcmp(argv[2], "exec") != 0)
> +		drop_cap();
> +
>  	basename = strrchr(argv[0], '/');
>  	if (basename == NULL)
>  		basename = argv[0];
> diff --git a/man/man8/ip-vrf.8 b/man/man8/ip-vrf.8
> index 18789339..1a42cebe 100644
> --- a/man/man8/ip-vrf.8
> +++ b/man/man8/ip-vrf.8
> @@ -63,6 +63,14 @@ a VRF other than the default VRF (main table). A command can be run against
>  the default VRF by passing the "default" as the VRF name. This is useful if
>  the current shell is associated with another VRF (e.g, Management VRF).
>  
> +This command requires the system to be booted with cgroup v2 (e.g. with systemd,
> +add systemd.unified_cgroup_hierarchy=1 to the kernel command line).
> +
> +This command also requires to be ran as root or with the CAP_SYS_ADMIN,
> +CAP_NET_ADMIN and CAP_DAC_OVERRIDE capabilities. If built with libcap and if
> +capabilities are added to the ip binary program via setcap, the program will
> +drop them as the first thing when invoked, unless the command is vrf exec.
> +
>  .TP
>  .B ip vrf identify [PID] - Report VRF association for process
>  .sp
> 

^ permalink raw reply

* [PATCH net-next 0/6] sfc: rework locking around filter management
From: Edward Cree @ 2018-03-27 16:40 UTC (permalink / raw)
  To: linux-net-drivers, David Miller; +Cc: netdev

The use of a spinlock to protect filter state combined with the need for a
 sleeping operation (MCDI) to apply that state to the NIC (on EF10) led to
 unfixable race conditions, around the handling of filter restoration after
 an MC reboot.
So, this patch series removes the requirement to be able to modify the SW
 filter table from atomic context, by using a workqueue to request
 asynchronous filter operations (which are needed for ARFS).  Then, the
 filter table locks are changed to mutexes, replacing the dance of spinlocks
 and 'busy' flags.  Also, a mutex is added to protect the RSS context state,
 since otherwise a similar race is possible around restoring that after an
 MC reboot.  While we're at it, fix a couple of other related bugs.

Edward Cree (6):
  sfc: replace asynchronous filter operations
  sfc: give ef10 its own rwsem in the filter table instead of
    filter_lock
  sfc: use a semaphore to lock farch filters too
  sfc: return a better error if filter insertion collides with MC reboot
  sfc: protect list of RSS contexts under a mutex
  sfc: fix flow type handling for RSS filters

 drivers/net/ethernet/sfc/ef10.c       | 545 ++++++++++++----------------------
 drivers/net/ethernet/sfc/efx.c        |  30 +-
 drivers/net/ethernet/sfc/efx.h        |  11 +-
 drivers/net/ethernet/sfc/ethtool.c    |  77 +++--
 drivers/net/ethernet/sfc/farch.c      |  80 +++--
 drivers/net/ethernet/sfc/net_driver.h |  16 +-
 drivers/net/ethernet/sfc/nic.h        |   5 +-
 drivers/net/ethernet/sfc/rx.c         | 119 +++++---
 drivers/net/ethernet/sfc/siena.c      |   1 -
 9 files changed, 400 insertions(+), 484 deletions(-)

^ permalink raw reply

* [PATCH net-next 1/6] sfc: replace asynchronous filter operations
From: Edward Cree @ 2018-03-27 16:41 UTC (permalink / raw)
  To: linux-net-drivers, David Miller; +Cc: netdev
In-Reply-To: <7a932be7-0335-a7f3-4fb4-408ccb191737@solarflare.com>

Instead of having an efx->type->filter_rfs_insert() method, just use
 workitems with a worker function that calls efx->type->filter_insert().
The only user of this is efx_filter_rfs(), which now queues a call to
 efx_filter_rfs_work().
Similarly, efx_filter_rfs_expire() is now a worker function called on a
 new channel->filter_work work_struct, so the method
 efx->type->filter_rfs_expire_one() is no longer called in atomic context.
 We also add a new mutex efx->rps_mutex to protect the RPS state (efx->
 rps_expire_channel, efx->rps_expire_index, and channel->rps_flow_id) so
 that the taking of efx->filter_lock can be moved to
 efx->type->filter_rfs_expire_one().
Thus, all filter table functions are now called in a sleepable context,
 allowing them to use sleeping locks in a future patch.

Signed-off-by: Edward Cree <ecree@solarflare.com>
---
 drivers/net/ethernet/sfc/ef10.c       | 161 +++-------------------------------
 drivers/net/ethernet/sfc/efx.c        |  15 +++-
 drivers/net/ethernet/sfc/efx.h        |   7 +-
 drivers/net/ethernet/sfc/farch.c      |  17 ++--
 drivers/net/ethernet/sfc/net_driver.h |  10 +--
 drivers/net/ethernet/sfc/nic.h        |   2 -
 drivers/net/ethernet/sfc/rx.c         | 119 +++++++++++++++++--------
 drivers/net/ethernet/sfc/siena.c      |   1 -
 8 files changed, 124 insertions(+), 208 deletions(-)

diff --git a/drivers/net/ethernet/sfc/ef10.c b/drivers/net/ethernet/sfc/ef10.c
index e100273b623d..bcbaba330fb5 100644
--- a/drivers/net/ethernet/sfc/ef10.c
+++ b/drivers/net/ethernet/sfc/ef10.c
@@ -4758,143 +4758,6 @@ static s32 efx_ef10_filter_get_rx_ids(struct efx_nic *efx,
 
 #ifdef CONFIG_RFS_ACCEL
 
-static efx_mcdi_async_completer efx_ef10_filter_rfs_insert_complete;
-
-static s32 efx_ef10_filter_rfs_insert(struct efx_nic *efx,
-				      struct efx_filter_spec *spec)
-{
-	struct efx_ef10_filter_table *table = efx->filter_state;
-	MCDI_DECLARE_BUF(inbuf, MC_CMD_FILTER_OP_EXT_IN_LEN);
-	struct efx_filter_spec *saved_spec;
-	unsigned int hash, i, depth = 1;
-	bool replacing = false;
-	int ins_index = -1;
-	u64 cookie;
-	s32 rc;
-
-	/* Must be an RX filter without RSS and not for a multicast
-	 * destination address (RFS only works for connected sockets).
-	 * These restrictions allow us to pass only a tiny amount of
-	 * data through to the completion function.
-	 */
-	EFX_WARN_ON_PARANOID(spec->flags !=
-			     (EFX_FILTER_FLAG_RX | EFX_FILTER_FLAG_RX_SCATTER));
-	EFX_WARN_ON_PARANOID(spec->priority != EFX_FILTER_PRI_HINT);
-	EFX_WARN_ON_PARANOID(efx_filter_is_mc_recipient(spec));
-
-	hash = efx_ef10_filter_hash(spec);
-
-	spin_lock_bh(&efx->filter_lock);
-
-	/* Find any existing filter with the same match tuple or else
-	 * a free slot to insert at.  If an existing filter is busy,
-	 * we have to give up.
-	 */
-	for (;;) {
-		i = (hash + depth) & (HUNT_FILTER_TBL_ROWS - 1);
-		saved_spec = efx_ef10_filter_entry_spec(table, i);
-
-		if (!saved_spec) {
-			if (ins_index < 0)
-				ins_index = i;
-		} else if (efx_ef10_filter_equal(spec, saved_spec)) {
-			if (table->entry[i].spec & EFX_EF10_FILTER_FLAG_BUSY) {
-				rc = -EBUSY;
-				goto fail_unlock;
-			}
-			if (spec->priority < saved_spec->priority) {
-				rc = -EPERM;
-				goto fail_unlock;
-			}
-			ins_index = i;
-			break;
-		}
-
-		/* Once we reach the maximum search depth, use the
-		 * first suitable slot or return -EBUSY if there was
-		 * none
-		 */
-		if (depth == EFX_EF10_FILTER_SEARCH_LIMIT) {
-			if (ins_index < 0) {
-				rc = -EBUSY;
-				goto fail_unlock;
-			}
-			break;
-		}
-
-		++depth;
-	}
-
-	/* Create a software table entry if necessary, and mark it
-	 * busy.  We might yet fail to insert, but any attempt to
-	 * insert a conflicting filter while we're waiting for the
-	 * firmware must find the busy entry.
-	 */
-	saved_spec = efx_ef10_filter_entry_spec(table, ins_index);
-	if (saved_spec) {
-		replacing = true;
-	} else {
-		saved_spec = kmalloc(sizeof(*spec), GFP_ATOMIC);
-		if (!saved_spec) {
-			rc = -ENOMEM;
-			goto fail_unlock;
-		}
-		*saved_spec = *spec;
-	}
-	efx_ef10_filter_set_entry(table, ins_index, saved_spec,
-				  EFX_EF10_FILTER_FLAG_BUSY);
-
-	spin_unlock_bh(&efx->filter_lock);
-
-	/* Pack up the variables needed on completion */
-	cookie = replacing << 31 | ins_index << 16 | spec->dmaq_id;
-
-	efx_ef10_filter_push_prep(efx, spec, inbuf,
-				  table->entry[ins_index].handle, NULL,
-				  replacing);
-	efx_mcdi_rpc_async(efx, MC_CMD_FILTER_OP, inbuf, sizeof(inbuf),
-			   MC_CMD_FILTER_OP_OUT_LEN,
-			   efx_ef10_filter_rfs_insert_complete, cookie);
-
-	return ins_index;
-
-fail_unlock:
-	spin_unlock_bh(&efx->filter_lock);
-	return rc;
-}
-
-static void
-efx_ef10_filter_rfs_insert_complete(struct efx_nic *efx, unsigned long cookie,
-				    int rc, efx_dword_t *outbuf,
-				    size_t outlen_actual)
-{
-	struct efx_ef10_filter_table *table = efx->filter_state;
-	unsigned int ins_index, dmaq_id;
-	struct efx_filter_spec *spec;
-	bool replacing;
-
-	/* Unpack the cookie */
-	replacing = cookie >> 31;
-	ins_index = (cookie >> 16) & (HUNT_FILTER_TBL_ROWS - 1);
-	dmaq_id = cookie & 0xffff;
-
-	spin_lock_bh(&efx->filter_lock);
-	spec = efx_ef10_filter_entry_spec(table, ins_index);
-	if (rc == 0) {
-		table->entry[ins_index].handle =
-			MCDI_QWORD(outbuf, FILTER_OP_OUT_HANDLE);
-		if (replacing)
-			spec->dmaq_id = dmaq_id;
-	} else if (!replacing) {
-		kfree(spec);
-		spec = NULL;
-	}
-	efx_ef10_filter_set_entry(table, ins_index, spec, 0);
-	spin_unlock_bh(&efx->filter_lock);
-
-	wake_up_all(&table->waitq);
-}
-
 static void
 efx_ef10_filter_rfs_expire_complete(struct efx_nic *efx,
 				    unsigned long filter_idx,
@@ -4905,18 +4768,22 @@ static bool efx_ef10_filter_rfs_expire_one(struct efx_nic *efx, u32 flow_id,
 					   unsigned int filter_idx)
 {
 	struct efx_ef10_filter_table *table = efx->filter_state;
-	struct efx_filter_spec *spec =
-		efx_ef10_filter_entry_spec(table, filter_idx);
+	struct efx_filter_spec *spec;
 	MCDI_DECLARE_BUF(inbuf,
 			 MC_CMD_FILTER_OP_IN_HANDLE_OFST +
 			 MC_CMD_FILTER_OP_IN_HANDLE_LEN);
+	bool ret = true;
 
+	spin_lock_bh(&efx->filter_lock);
+	spec = efx_ef10_filter_entry_spec(table, filter_idx);
 	if (!spec ||
 	    (table->entry[filter_idx].spec & EFX_EF10_FILTER_FLAG_BUSY) ||
 	    spec->priority != EFX_FILTER_PRI_HINT ||
 	    !rps_may_expire_flow(efx->net_dev, spec->dmaq_id,
-				 flow_id, filter_idx))
-		return false;
+				 flow_id, filter_idx)) {
+		ret = false;
+		goto out_unlock;
+	}
 
 	MCDI_SET_DWORD(inbuf, FILTER_OP_IN_OP,
 		       MC_CMD_FILTER_OP_IN_OP_REMOVE);
@@ -4924,10 +4791,12 @@ static bool efx_ef10_filter_rfs_expire_one(struct efx_nic *efx, u32 flow_id,
 		       table->entry[filter_idx].handle);
 	if (efx_mcdi_rpc_async(efx, MC_CMD_FILTER_OP, inbuf, sizeof(inbuf), 0,
 			       efx_ef10_filter_rfs_expire_complete, filter_idx))
-		return false;
-
-	table->entry[filter_idx].spec |= EFX_EF10_FILTER_FLAG_BUSY;
-	return true;
+		ret = false;
+	else
+		table->entry[filter_idx].spec |= EFX_EF10_FILTER_FLAG_BUSY;
+out_unlock:
+	spin_unlock_bh(&efx->filter_lock);
+	return ret;
 }
 
 static void
@@ -6784,7 +6653,6 @@ const struct efx_nic_type efx_hunt_a0_vf_nic_type = {
 	.filter_get_rx_id_limit = efx_ef10_filter_get_rx_id_limit,
 	.filter_get_rx_ids = efx_ef10_filter_get_rx_ids,
 #ifdef CONFIG_RFS_ACCEL
-	.filter_rfs_insert = efx_ef10_filter_rfs_insert,
 	.filter_rfs_expire_one = efx_ef10_filter_rfs_expire_one,
 #endif
 #ifdef CONFIG_SFC_MTD
@@ -6897,7 +6765,6 @@ const struct efx_nic_type efx_hunt_a0_nic_type = {
 	.filter_get_rx_id_limit = efx_ef10_filter_get_rx_id_limit,
 	.filter_get_rx_ids = efx_ef10_filter_get_rx_ids,
 #ifdef CONFIG_RFS_ACCEL
-	.filter_rfs_insert = efx_ef10_filter_rfs_insert,
 	.filter_rfs_expire_one = efx_ef10_filter_rfs_expire_one,
 #endif
 #ifdef CONFIG_SFC_MTD
diff --git a/drivers/net/ethernet/sfc/efx.c b/drivers/net/ethernet/sfc/efx.c
index 7321a4cf6f4d..0be93abdb2ad 100644
--- a/drivers/net/ethernet/sfc/efx.c
+++ b/drivers/net/ethernet/sfc/efx.c
@@ -340,7 +340,10 @@ static int efx_poll(struct napi_struct *napi, int budget)
 			efx_update_irq_mod(efx, channel);
 		}
 
-		efx_filter_rfs_expire(channel);
+#ifdef CONFIG_RFS_ACCEL
+		/* Perhaps expire some ARFS filters */
+		schedule_work(&channel->filter_work);
+#endif
 
 		/* There is no race here; although napi_disable() will
 		 * only wait for napi_complete(), this isn't a problem
@@ -470,6 +473,10 @@ efx_alloc_channel(struct efx_nic *efx, int i, struct efx_channel *old_channel)
 		tx_queue->channel = channel;
 	}
 
+#ifdef CONFIG_RFS_ACCEL
+	INIT_WORK(&channel->filter_work, efx_filter_rfs_expire);
+#endif
+
 	rx_queue = &channel->rx_queue;
 	rx_queue->efx = efx;
 	timer_setup(&rx_queue->slow_fill, efx_rx_slow_fill, 0);
@@ -512,6 +519,9 @@ efx_copy_channel(const struct efx_channel *old_channel)
 	rx_queue->buffer = NULL;
 	memset(&rx_queue->rxd, 0, sizeof(rx_queue->rxd));
 	timer_setup(&rx_queue->slow_fill, efx_rx_slow_fill, 0);
+#ifdef CONFIG_RFS_ACCEL
+	INIT_WORK(&channel->filter_work, efx_filter_rfs_expire);
+#endif
 
 	return channel;
 }
@@ -3012,6 +3022,9 @@ static int efx_init_struct(struct efx_nic *efx,
 	efx->num_mac_stats = MC_CMD_MAC_NSTATS;
 	BUILD_BUG_ON(MC_CMD_MAC_NSTATS - 1 != MC_CMD_MAC_GENERATION_END);
 	mutex_init(&efx->mac_lock);
+#ifdef CONFIG_RFS_ACCEL
+	mutex_init(&efx->rps_mutex);
+#endif
 	efx->phy_op = &efx_dummy_phy_operations;
 	efx->mdio.dev = net_dev;
 	INIT_WORK(&efx->mac_work, efx_mac_work);
diff --git a/drivers/net/ethernet/sfc/efx.h b/drivers/net/ethernet/sfc/efx.h
index 3429ae3f3b08..545c2ea1622e 100644
--- a/drivers/net/ethernet/sfc/efx.h
+++ b/drivers/net/ethernet/sfc/efx.h
@@ -170,15 +170,18 @@ static inline s32 efx_filter_get_rx_ids(struct efx_nic *efx,
 int efx_filter_rfs(struct net_device *net_dev, const struct sk_buff *skb,
 		   u16 rxq_index, u32 flow_id);
 bool __efx_filter_rfs_expire(struct efx_nic *efx, unsigned quota);
-static inline void efx_filter_rfs_expire(struct efx_channel *channel)
+static inline void efx_filter_rfs_expire(struct work_struct *data)
 {
+	struct efx_channel *channel = container_of(data, struct efx_channel,
+						   filter_work);
+
 	if (channel->rfs_filters_added >= 60 &&
 	    __efx_filter_rfs_expire(channel->efx, 100))
 		channel->rfs_filters_added -= 60;
 }
 #define efx_filter_rfs_enabled() 1
 #else
-static inline void efx_filter_rfs_expire(struct efx_channel *channel) {}
+static inline void efx_filter_rfs_expire(struct work_struct *data) {}
 #define efx_filter_rfs_enabled() 0
 #endif
 bool efx_filter_is_mc_recipient(const struct efx_filter_spec *spec);
diff --git a/drivers/net/ethernet/sfc/farch.c b/drivers/net/ethernet/sfc/farch.c
index ad001e77d554..6b5ca569db5e 100644
--- a/drivers/net/ethernet/sfc/farch.c
+++ b/drivers/net/ethernet/sfc/farch.c
@@ -2901,28 +2901,25 @@ void efx_farch_filter_update_rx_scatter(struct efx_nic *efx)
 
 #ifdef CONFIG_RFS_ACCEL
 
-s32 efx_farch_filter_rfs_insert(struct efx_nic *efx,
-				struct efx_filter_spec *gen_spec)
-{
-	return efx_farch_filter_insert(efx, gen_spec, true);
-}
-
 bool efx_farch_filter_rfs_expire_one(struct efx_nic *efx, u32 flow_id,
 				     unsigned int index)
 {
 	struct efx_farch_filter_state *state = efx->filter_state;
-	struct efx_farch_filter_table *table =
-		&state->table[EFX_FARCH_FILTER_TABLE_RX_IP];
+	struct efx_farch_filter_table *table;
+	bool ret = false;
 
+	spin_lock_bh(&efx->filter_lock);
+	table = &state->table[EFX_FARCH_FILTER_TABLE_RX_IP];
 	if (test_bit(index, table->used_bitmap) &&
 	    table->spec[index].priority == EFX_FILTER_PRI_HINT &&
 	    rps_may_expire_flow(efx->net_dev, table->spec[index].dmaq_id,
 				flow_id, index)) {
 		efx_farch_filter_table_clear_entry(efx, table, index);
-		return true;
+		ret = true;
 	}
 
-	return false;
+	spin_unlock_bh(&efx->filter_lock);
+	return ret;
 }
 
 #endif /* CONFIG_RFS_ACCEL */
diff --git a/drivers/net/ethernet/sfc/net_driver.h b/drivers/net/ethernet/sfc/net_driver.h
index 203d64c88de5..fe0c5322454d 100644
--- a/drivers/net/ethernet/sfc/net_driver.h
+++ b/drivers/net/ethernet/sfc/net_driver.h
@@ -430,6 +430,7 @@ enum efx_sync_events_state {
  * @event_test_cpu: Last CPU to handle interrupt or test event for this channel
  * @irq_count: Number of IRQs since last adaptive moderation decision
  * @irq_mod_score: IRQ moderation score
+ * @filter_work: Work item for efx_filter_rfs_expire()
  * @rps_flow_id: Flow IDs of filters allocated for accelerated RFS,
  *      indexed by filter ID
  * @n_rx_tobe_disc: Count of RX_TOBE_DISC errors
@@ -475,6 +476,7 @@ struct efx_channel {
 	unsigned int irq_mod_score;
 #ifdef CONFIG_RFS_ACCEL
 	unsigned int rfs_filters_added;
+	struct work_struct filter_work;
 #define RPS_FLOW_ID_INVALID 0xFFFFFFFF
 	u32 *rps_flow_id;
 #endif
@@ -837,6 +839,7 @@ struct efx_rss_context {
  * @filter_sem: Filter table rw_semaphore, for freeing the table
  * @filter_lock: Filter table lock, for mere content changes
  * @filter_state: Architecture-dependent filter table state
+ * @rps_mutex: Protects RPS state of all channels
  * @rps_expire_channel: Next channel to check for expiry
  * @rps_expire_index: Next index to check for expiry in
  *	@rps_expire_channel's @rps_flow_id
@@ -990,6 +993,7 @@ struct efx_nic {
 	spinlock_t filter_lock;
 	void *filter_state;
 #ifdef CONFIG_RFS_ACCEL
+	struct mutex rps_mutex;
 	unsigned int rps_expire_channel;
 	unsigned int rps_expire_index;
 #endif
@@ -1144,10 +1148,6 @@ struct efx_udp_tunnel {
  * @filter_count_rx_used: Get the number of filters in use at a given priority
  * @filter_get_rx_id_limit: Get maximum value of a filter id, plus 1
  * @filter_get_rx_ids: Get list of RX filters at a given priority
- * @filter_rfs_insert: Add or replace a filter for RFS.  This must be
- *	atomic.  The hardware change may be asynchronous but should
- *	not be delayed for long.  It may fail if this can't be done
- *	atomically.
  * @filter_rfs_expire_one: Consider expiring a filter inserted for RFS.
  *	This must check whether the specified table entry is used by RFS
  *	and that rps_may_expire_flow() returns true for it.
@@ -1298,8 +1298,6 @@ struct efx_nic_type {
 				 enum efx_filter_priority priority,
 				 u32 *buf, u32 size);
 #ifdef CONFIG_RFS_ACCEL
-	s32 (*filter_rfs_insert)(struct efx_nic *efx,
-				 struct efx_filter_spec *spec);
 	bool (*filter_rfs_expire_one)(struct efx_nic *efx, u32 flow_id,
 				      unsigned int index);
 #endif
diff --git a/drivers/net/ethernet/sfc/nic.h b/drivers/net/ethernet/sfc/nic.h
index d080a414e8f2..ac59afad6e5c 100644
--- a/drivers/net/ethernet/sfc/nic.h
+++ b/drivers/net/ethernet/sfc/nic.h
@@ -601,8 +601,6 @@ s32 efx_farch_filter_get_rx_ids(struct efx_nic *efx,
 				enum efx_filter_priority priority, u32 *buf,
 				u32 size);
 #ifdef CONFIG_RFS_ACCEL
-s32 efx_farch_filter_rfs_insert(struct efx_nic *efx,
-				struct efx_filter_spec *spec);
 bool efx_farch_filter_rfs_expire_one(struct efx_nic *efx, u32 flow_id,
 				     unsigned int index);
 #endif
diff --git a/drivers/net/ethernet/sfc/rx.c b/drivers/net/ethernet/sfc/rx.c
index cfe76aad79ee..95682831484e 100644
--- a/drivers/net/ethernet/sfc/rx.c
+++ b/drivers/net/ethernet/sfc/rx.c
@@ -827,14 +827,67 @@ MODULE_PARM_DESC(rx_refill_threshold,
 
 #ifdef CONFIG_RFS_ACCEL
 
+/**
+ * struct efx_async_filter_insertion - Request to asynchronously insert a filter
+ * @net_dev: Reference to the netdevice
+ * @spec: The filter to insert
+ * @work: Workitem for this request
+ * @rxq_index: Identifies the channel for which this request was made
+ * @flow_id: Identifies the kernel-side flow for which this request was made
+ */
+struct efx_async_filter_insertion {
+	struct net_device *net_dev;
+	struct efx_filter_spec spec;
+	struct work_struct work;
+	u16 rxq_index;
+	u32 flow_id;
+};
+
+static void efx_filter_rfs_work(struct work_struct *data)
+{
+	struct efx_async_filter_insertion *req = container_of(data, struct efx_async_filter_insertion,
+							      work);
+	struct efx_nic *efx = netdev_priv(req->net_dev);
+	struct efx_channel *channel = efx_get_channel(efx, req->rxq_index);
+	int rc;
+
+	rc = efx->type->filter_insert(efx, &req->spec, false);
+	if (rc >= 0) {
+		/* Remember this so we can check whether to expire the filter
+		 * later.
+		 */
+		mutex_lock(&efx->rps_mutex);
+		channel->rps_flow_id[rc] = req->flow_id;
+		++channel->rfs_filters_added;
+		mutex_unlock(&efx->rps_mutex);
+
+		if (req->spec.ether_type == htons(ETH_P_IP))
+			netif_info(efx, rx_status, efx->net_dev,
+				   "steering %s %pI4:%u:%pI4:%u to queue %u [flow %u filter %d]\n",
+				   (req->spec.ip_proto == IPPROTO_TCP) ? "TCP" : "UDP",
+				   req->spec.rem_host, ntohs(req->spec.rem_port),
+				   req->spec.loc_host, ntohs(req->spec.loc_port),
+				   req->rxq_index, req->flow_id, rc);
+		else
+			netif_info(efx, rx_status, efx->net_dev,
+				   "steering %s [%pI6]:%u:[%pI6]:%u to queue %u [flow %u filter %d]\n",
+				   (req->spec.ip_proto == IPPROTO_TCP) ? "TCP" : "UDP",
+				   req->spec.rem_host, ntohs(req->spec.rem_port),
+				   req->spec.loc_host, ntohs(req->spec.loc_port),
+				   req->rxq_index, req->flow_id, rc);
+	}
+
+	/* Release references */
+	dev_put(req->net_dev);
+	kfree(req);
+}
+
 int efx_filter_rfs(struct net_device *net_dev, const struct sk_buff *skb,
 		   u16 rxq_index, u32 flow_id)
 {
 	struct efx_nic *efx = netdev_priv(net_dev);
-	struct efx_channel *channel;
-	struct efx_filter_spec spec;
+	struct efx_async_filter_insertion *req;
 	struct flow_keys fk;
-	int rc;
 
 	if (flow_id == RPS_FLOW_ID_INVALID)
 		return -EINVAL;
@@ -847,50 +900,39 @@ int efx_filter_rfs(struct net_device *net_dev, const struct sk_buff *skb,
 	if (fk.control.flags & FLOW_DIS_IS_FRAGMENT)
 		return -EPROTONOSUPPORT;
 
-	efx_filter_init_rx(&spec, EFX_FILTER_PRI_HINT,
+	req = kmalloc(sizeof(*req), GFP_ATOMIC);
+	if (!req)
+		return -ENOMEM;
+
+	efx_filter_init_rx(&req->spec, EFX_FILTER_PRI_HINT,
 			   efx->rx_scatter ? EFX_FILTER_FLAG_RX_SCATTER : 0,
 			   rxq_index);
-	spec.match_flags =
+	req->spec.match_flags =
 		EFX_FILTER_MATCH_ETHER_TYPE | EFX_FILTER_MATCH_IP_PROTO |
 		EFX_FILTER_MATCH_LOC_HOST | EFX_FILTER_MATCH_LOC_PORT |
 		EFX_FILTER_MATCH_REM_HOST | EFX_FILTER_MATCH_REM_PORT;
-	spec.ether_type = fk.basic.n_proto;
-	spec.ip_proto = fk.basic.ip_proto;
+	req->spec.ether_type = fk.basic.n_proto;
+	req->spec.ip_proto = fk.basic.ip_proto;
 
 	if (fk.basic.n_proto == htons(ETH_P_IP)) {
-		spec.rem_host[0] = fk.addrs.v4addrs.src;
-		spec.loc_host[0] = fk.addrs.v4addrs.dst;
+		req->spec.rem_host[0] = fk.addrs.v4addrs.src;
+		req->spec.loc_host[0] = fk.addrs.v4addrs.dst;
 	} else {
-		memcpy(spec.rem_host, &fk.addrs.v6addrs.src, sizeof(struct in6_addr));
-		memcpy(spec.loc_host, &fk.addrs.v6addrs.dst, sizeof(struct in6_addr));
+		memcpy(req->spec.rem_host, &fk.addrs.v6addrs.src,
+		       sizeof(struct in6_addr));
+		memcpy(req->spec.loc_host, &fk.addrs.v6addrs.dst,
+		       sizeof(struct in6_addr));
 	}
 
-	spec.rem_port = fk.ports.src;
-	spec.loc_port = fk.ports.dst;
-
-	rc = efx->type->filter_rfs_insert(efx, &spec);
-	if (rc < 0)
-		return rc;
+	req->spec.rem_port = fk.ports.src;
+	req->spec.loc_port = fk.ports.dst;
 
-	/* Remember this so we can check whether to expire the filter later */
-	channel = efx_get_channel(efx, rxq_index);
-	channel->rps_flow_id[rc] = flow_id;
-	++channel->rfs_filters_added;
-
-	if (spec.ether_type == htons(ETH_P_IP))
-		netif_info(efx, rx_status, efx->net_dev,
-			   "steering %s %pI4:%u:%pI4:%u to queue %u [flow %u filter %d]\n",
-			   (spec.ip_proto == IPPROTO_TCP) ? "TCP" : "UDP",
-			   spec.rem_host, ntohs(spec.rem_port), spec.loc_host,
-			   ntohs(spec.loc_port), rxq_index, flow_id, rc);
-	else
-		netif_info(efx, rx_status, efx->net_dev,
-			   "steering %s [%pI6]:%u:[%pI6]:%u to queue %u [flow %u filter %d]\n",
-			   (spec.ip_proto == IPPROTO_TCP) ? "TCP" : "UDP",
-			   spec.rem_host, ntohs(spec.rem_port), spec.loc_host,
-			   ntohs(spec.loc_port), rxq_index, flow_id, rc);
-
-	return rc;
+	dev_hold(req->net_dev = net_dev);
+	INIT_WORK(&req->work, efx_filter_rfs_work);
+	req->rxq_index = rxq_index;
+	req->flow_id = flow_id;
+	schedule_work(&req->work);
+	return 0;
 }
 
 bool __efx_filter_rfs_expire(struct efx_nic *efx, unsigned int quota)
@@ -899,9 +941,8 @@ bool __efx_filter_rfs_expire(struct efx_nic *efx, unsigned int quota)
 	unsigned int channel_idx, index, size;
 	u32 flow_id;
 
-	if (!spin_trylock_bh(&efx->filter_lock))
+	if (!mutex_trylock(&efx->rps_mutex))
 		return false;
-
 	expire_one = efx->type->filter_rfs_expire_one;
 	channel_idx = efx->rps_expire_channel;
 	index = efx->rps_expire_index;
@@ -926,7 +967,7 @@ bool __efx_filter_rfs_expire(struct efx_nic *efx, unsigned int quota)
 	efx->rps_expire_channel = channel_idx;
 	efx->rps_expire_index = index;
 
-	spin_unlock_bh(&efx->filter_lock);
+	mutex_unlock(&efx->rps_mutex);
 	return true;
 }
 
diff --git a/drivers/net/ethernet/sfc/siena.c b/drivers/net/ethernet/sfc/siena.c
index 18aab25234ba..65161f68265a 100644
--- a/drivers/net/ethernet/sfc/siena.c
+++ b/drivers/net/ethernet/sfc/siena.c
@@ -1035,7 +1035,6 @@ const struct efx_nic_type siena_a0_nic_type = {
 	.filter_get_rx_id_limit = efx_farch_filter_get_rx_id_limit,
 	.filter_get_rx_ids = efx_farch_filter_get_rx_ids,
 #ifdef CONFIG_RFS_ACCEL
-	.filter_rfs_insert = efx_farch_filter_rfs_insert,
 	.filter_rfs_expire_one = efx_farch_filter_rfs_expire_one,
 #endif
 #ifdef CONFIG_SFC_MTD

^ permalink raw reply related

* [PATCH net-next 2/6] sfc: give ef10 its own rwsem in the filter table instead of filter_lock
From: Edward Cree @ 2018-03-27 16:42 UTC (permalink / raw)
  To: linux-net-drivers, David Miller; +Cc: netdev
In-Reply-To: <7a932be7-0335-a7f3-4fb4-408ccb191737@solarflare.com>

efx->filter_lock remains in place for use on farch, but EF10 now ignores it.
EFX_EF10_FILTER_FLAG_BUSY is no longer needed, hence it is removed.

Signed-off-by: Edward Cree <ecree@solarflare.com>
---
 drivers/net/ethernet/sfc/ef10.c       | 361 ++++++++++++++--------------------
 drivers/net/ethernet/sfc/efx.c        |   1 -
 drivers/net/ethernet/sfc/net_driver.h |   2 +-
 3 files changed, 151 insertions(+), 213 deletions(-)

diff --git a/drivers/net/ethernet/sfc/ef10.c b/drivers/net/ethernet/sfc/ef10.c
index bcbaba330fb5..9db1b9144e70 100644
--- a/drivers/net/ethernet/sfc/ef10.c
+++ b/drivers/net/ethernet/sfc/ef10.c
@@ -96,17 +96,15 @@ struct efx_ef10_filter_table {
 		MC_CMD_GET_PARSER_DISP_INFO_OUT_SUPPORTED_MATCHES_MAXNUM * 2];
 	unsigned int rx_match_count;
 
+	struct rw_semaphore lock; /* Protects entries */
 	struct {
 		unsigned long spec;	/* pointer to spec plus flag bits */
-/* BUSY flag indicates that an update is in progress.  AUTO_OLD is
- * used to mark and sweep MAC filters for the device address lists.
- */
-#define EFX_EF10_FILTER_FLAG_BUSY	1UL
+/* AUTO_OLD is used to mark and sweep MAC filters for the device address lists. */
+/* unused flag	1UL */
 #define EFX_EF10_FILTER_FLAG_AUTO_OLD	2UL
 #define EFX_EF10_FILTER_FLAGS		3UL
 		u64 handle;		/* firmware handle */
 	} *entry;
-	wait_queue_head_t waitq;
 /* Shadow of net_device address lists, guarded by mac_lock */
 	struct efx_ef10_dev_addr dev_uc_list[EFX_EF10_FILTER_DEV_UC_MAX];
 	struct efx_ef10_dev_addr dev_mc_list[EFX_EF10_FILTER_DEV_MC_MAX];
@@ -4302,26 +4300,33 @@ static s32 efx_ef10_filter_insert(struct efx_nic *efx,
 				  struct efx_filter_spec *spec,
 				  bool replace_equal)
 {
-	struct efx_ef10_filter_table *table = efx->filter_state;
 	DECLARE_BITMAP(mc_rem_map, EFX_EF10_FILTER_SEARCH_LIMIT);
+	struct efx_ef10_filter_table *table;
 	struct efx_filter_spec *saved_spec;
 	struct efx_rss_context *ctx = NULL;
 	unsigned int match_pri, hash;
 	unsigned int priv_flags;
 	bool replacing = false;
+	unsigned int depth, i;
 	int ins_index = -1;
 	DEFINE_WAIT(wait);
 	bool is_mc_recip;
 	s32 rc;
 
+	down_read(&efx->filter_sem);
+	table = efx->filter_state;
+	down_write(&table->lock);
+
 	/* For now, only support RX filters */
 	if ((spec->flags & (EFX_FILTER_FLAG_RX | EFX_FILTER_FLAG_TX)) !=
-	    EFX_FILTER_FLAG_RX)
-		return -EINVAL;
+	    EFX_FILTER_FLAG_RX) {
+		rc = -EINVAL;
+		goto out_unlock;
+	}
 
 	rc = efx_ef10_filter_pri(table, spec);
 	if (rc < 0)
-		return rc;
+		goto out_unlock;
 	match_pri = rc;
 
 	hash = efx_ef10_filter_hash(spec);
@@ -4335,86 +4340,64 @@ static s32 efx_ef10_filter_insert(struct efx_nic *efx,
 							 &efx->rss_context.list);
 		else
 			ctx = &efx->rss_context;
-		if (!ctx)
-			return -ENOENT;
-		if (ctx->context_id == EFX_EF10_RSS_CONTEXT_INVALID)
-			return -EOPNOTSUPP;
+		if (!ctx) {
+			rc = -ENOENT;
+			goto out_unlock;
+		}
+		if (ctx->context_id == EFX_EF10_RSS_CONTEXT_INVALID) {
+			rc = -EOPNOTSUPP;
+			goto out_unlock;
+		}
 	}
 
 	/* Find any existing filters with the same match tuple or
-	 * else a free slot to insert at.  If any of them are busy,
-	 * we have to wait and retry.
+	 * else a free slot to insert at.
 	 */
-	for (;;) {
-		unsigned int depth = 1;
-		unsigned int i;
-
-		spin_lock_bh(&efx->filter_lock);
+	for (depth = 1; depth < EFX_EF10_FILTER_SEARCH_LIMIT; depth++) {
+		i = (hash + depth) & (HUNT_FILTER_TBL_ROWS - 1);
+		saved_spec = efx_ef10_filter_entry_spec(table, i);
 
-		for (;;) {
-			i = (hash + depth) & (HUNT_FILTER_TBL_ROWS - 1);
-			saved_spec = efx_ef10_filter_entry_spec(table, i);
-
-			if (!saved_spec) {
-				if (ins_index < 0)
-					ins_index = i;
-			} else if (efx_ef10_filter_equal(spec, saved_spec)) {
-				if (table->entry[i].spec &
-				    EFX_EF10_FILTER_FLAG_BUSY)
-					break;
-				if (spec->priority < saved_spec->priority &&
-				    spec->priority != EFX_FILTER_PRI_AUTO) {
-					rc = -EPERM;
-					goto out_unlock;
-				}
-				if (!is_mc_recip) {
-					/* This is the only one */
-					if (spec->priority ==
-					    saved_spec->priority &&
-					    !replace_equal) {
-						rc = -EEXIST;
-						goto out_unlock;
-					}
-					ins_index = i;
-					goto found;
-				} else if (spec->priority >
-					   saved_spec->priority ||
-					   (spec->priority ==
-					    saved_spec->priority &&
-					    replace_equal)) {
-					if (ins_index < 0)
-						ins_index = i;
-					else
-						__set_bit(depth, mc_rem_map);
-				}
+		if (!saved_spec) {
+			if (ins_index < 0)
+				ins_index = i;
+		} else if (efx_ef10_filter_equal(spec, saved_spec)) {
+			if (spec->priority < saved_spec->priority &&
+			    spec->priority != EFX_FILTER_PRI_AUTO) {
+				rc = -EPERM;
+				goto out_unlock;
 			}
-
-			/* Once we reach the maximum search depth, use
-			 * the first suitable slot or return -EBUSY if
-			 * there was none
-			 */
-			if (depth == EFX_EF10_FILTER_SEARCH_LIMIT) {
-				if (ins_index < 0) {
-					rc = -EBUSY;
+			if (!is_mc_recip) {
+				/* This is the only one */
+				if (spec->priority ==
+				    saved_spec->priority &&
+				    !replace_equal) {
+					rc = -EEXIST;
 					goto out_unlock;
 				}
-				goto found;
+				ins_index = i;
+				break;
+			} else if (spec->priority >
+				   saved_spec->priority ||
+				   (spec->priority ==
+				    saved_spec->priority &&
+				    replace_equal)) {
+				if (ins_index < 0)
+					ins_index = i;
+				else
+					__set_bit(depth, mc_rem_map);
 			}
-
-			++depth;
 		}
-
-		prepare_to_wait(&table->waitq, &wait, TASK_UNINTERRUPTIBLE);
-		spin_unlock_bh(&efx->filter_lock);
-		schedule();
 	}
 
-found:
-	/* Create a software table entry if necessary, and mark it
-	 * busy.  We might yet fail to insert, but any attempt to
-	 * insert a conflicting filter while we're waiting for the
-	 * firmware must find the busy entry.
+	/* Once we reach the maximum search depth, use the first suitable
+	 * slot, or return -EBUSY if there was none
 	 */
+	if (ins_index < 0) {
+		rc = -EBUSY;
+		goto out_unlock;
+	}
+
+	/* Create a software table entry if necessary. */
 	saved_spec = efx_ef10_filter_entry_spec(table, ins_index);
 	if (saved_spec) {
 		if (spec->priority == EFX_FILTER_PRI_AUTO &&
@@ -4438,28 +4421,13 @@ static s32 efx_ef10_filter_insert(struct efx_nic *efx,
 		*saved_spec = *spec;
 		priv_flags = 0;
 	}
-	efx_ef10_filter_set_entry(table, ins_index, saved_spec,
-				  priv_flags | EFX_EF10_FILTER_FLAG_BUSY);
-
-	/* Mark lower-priority multicast recipients busy prior to removal */
-	if (is_mc_recip) {
-		unsigned int depth, i;
-
-		for (depth = 0; depth < EFX_EF10_FILTER_SEARCH_LIMIT; depth++) {
-			i = (hash + depth) & (HUNT_FILTER_TBL_ROWS - 1);
-			if (test_bit(depth, mc_rem_map))
-				table->entry[i].spec |=
-					EFX_EF10_FILTER_FLAG_BUSY;
-		}
-	}
-
-	spin_unlock_bh(&efx->filter_lock);
+	efx_ef10_filter_set_entry(table, ins_index, saved_spec, priv_flags);
 
+	/* Actually insert the filter on the HW */
 	rc = efx_ef10_filter_push(efx, spec, &table->entry[ins_index].handle,
 				  ctx, replacing);
 
 	/* Finalise the software table entry */
-	spin_lock_bh(&efx->filter_lock);
 	if (rc == 0) {
 		if (replacing) {
 			/* Update the fields that may differ */
@@ -4475,6 +4443,12 @@ static s32 efx_ef10_filter_insert(struct efx_nic *efx,
 	} else if (!replacing) {
 		kfree(saved_spec);
 		saved_spec = NULL;
+	} else {
+		/* We failed to replace, so the old filter is still present.
+		 * Roll back the software table to reflect this.  In fact the
+		 * efx_ef10_filter_set_entry() call below will do the right
+		 * thing, so nothing extra is needed here.
+		 */
 	}
 	efx_ef10_filter_set_entry(table, ins_index, saved_spec, priv_flags);
 
@@ -4496,7 +4470,6 @@ static s32 efx_ef10_filter_insert(struct efx_nic *efx,
 			priv_flags = efx_ef10_filter_entry_flags(table, i);
 
 			if (rc == 0) {
-				spin_unlock_bh(&efx->filter_lock);
 				MCDI_SET_DWORD(inbuf, FILTER_OP_IN_OP,
 					       MC_CMD_FILTER_OP_IN_OP_UNSUBSCRIBE);
 				MCDI_SET_QWORD(inbuf, FILTER_OP_IN_HANDLE,
@@ -4504,15 +4477,12 @@ static s32 efx_ef10_filter_insert(struct efx_nic *efx,
 				rc = efx_mcdi_rpc(efx, MC_CMD_FILTER_OP,
 						  inbuf, sizeof(inbuf),
 						  NULL, 0, NULL);
-				spin_lock_bh(&efx->filter_lock);
 			}
 
 			if (rc == 0) {
 				kfree(saved_spec);
 				saved_spec = NULL;
 				priv_flags = 0;
-			} else {
-				priv_flags &= ~EFX_EF10_FILTER_FLAG_BUSY;
 			}
 			efx_ef10_filter_set_entry(table, i, saved_spec,
 						  priv_flags);
@@ -4523,10 +4493,9 @@ static s32 efx_ef10_filter_insert(struct efx_nic *efx,
 	if (rc == 0)
 		rc = efx_ef10_make_filter_id(match_pri, ins_index);
 
-	wake_up_all(&table->waitq);
 out_unlock:
-	spin_unlock_bh(&efx->filter_lock);
-	finish_wait(&table->waitq, &wait);
+	up_write(&table->lock);
+	up_read(&efx->filter_sem);
 	return rc;
 }
 
@@ -4539,6 +4508,8 @@ static void efx_ef10_filter_update_rx_scatter(struct efx_nic *efx)
  * If !by_index, remove by ID
  * If by_index, remove by index
  * Filter ID may come from userland and must be range-checked.
+ * Caller must hold efx->filter_sem for read, and efx->filter_state->lock
+ * for write.
  */
 static int efx_ef10_filter_remove_internal(struct efx_nic *efx,
 					   unsigned int priority_mask,
@@ -4553,45 +4524,23 @@ static int efx_ef10_filter_remove_internal(struct efx_nic *efx,
 	DEFINE_WAIT(wait);
 	int rc;
 
-	/* Find the software table entry and mark it busy.  Don't
-	 * remove it yet; any attempt to update while we're waiting
-	 * for the firmware must find the busy entry.
-	 */
-	for (;;) {
-		spin_lock_bh(&efx->filter_lock);
-		if (!(table->entry[filter_idx].spec &
-		      EFX_EF10_FILTER_FLAG_BUSY))
-			break;
-		prepare_to_wait(&table->waitq, &wait, TASK_UNINTERRUPTIBLE);
-		spin_unlock_bh(&efx->filter_lock);
-		schedule();
-	}
-
 	spec = efx_ef10_filter_entry_spec(table, filter_idx);
 	if (!spec ||
 	    (!by_index &&
 	     efx_ef10_filter_pri(table, spec) !=
-	     efx_ef10_filter_get_unsafe_pri(filter_id))) {
-		rc = -ENOENT;
-		goto out_unlock;
-	}
+	     efx_ef10_filter_get_unsafe_pri(filter_id)))
+		return -ENOENT;
 
 	if (spec->flags & EFX_FILTER_FLAG_RX_OVER_AUTO &&
 	    priority_mask == (1U << EFX_FILTER_PRI_AUTO)) {
 		/* Just remove flags */
 		spec->flags &= ~EFX_FILTER_FLAG_RX_OVER_AUTO;
 		table->entry[filter_idx].spec &= ~EFX_EF10_FILTER_FLAG_AUTO_OLD;
-		rc = 0;
-		goto out_unlock;
-	}
-
-	if (!(priority_mask & (1U << spec->priority))) {
-		rc = -ENOENT;
-		goto out_unlock;
+		return 0;
 	}
 
-	table->entry[filter_idx].spec |= EFX_EF10_FILTER_FLAG_BUSY;
-	spin_unlock_bh(&efx->filter_lock);
+	if (!(priority_mask & (1U << spec->priority)))
+		return -ENOENT;
 
 	if (spec->flags & EFX_FILTER_FLAG_RX_OVER_AUTO) {
 		/* Reset to an automatic filter */
@@ -4609,7 +4558,6 @@ static int efx_ef10_filter_remove_internal(struct efx_nic *efx,
 					  &efx->rss_context,
 					  true);
 
-		spin_lock_bh(&efx->filter_lock);
 		if (rc == 0)
 			*spec = new_spec;
 	} else {
@@ -4624,7 +4572,6 @@ static int efx_ef10_filter_remove_internal(struct efx_nic *efx,
 		rc = efx_mcdi_rpc_quiet(efx, MC_CMD_FILTER_OP,
 					inbuf, sizeof(inbuf), NULL, 0, NULL);
 
-		spin_lock_bh(&efx->filter_lock);
 		if ((rc == 0) || (rc == -ENOENT)) {
 			/* Filter removed OK or didn't actually exist */
 			kfree(spec);
@@ -4636,11 +4583,6 @@ static int efx_ef10_filter_remove_internal(struct efx_nic *efx,
 		}
 	}
 
-	table->entry[filter_idx].spec &= ~EFX_EF10_FILTER_FLAG_BUSY;
-	wake_up_all(&table->waitq);
-out_unlock:
-	spin_unlock_bh(&efx->filter_lock);
-	finish_wait(&table->waitq, &wait);
 	return rc;
 }
 
@@ -4648,17 +4590,33 @@ static int efx_ef10_filter_remove_safe(struct efx_nic *efx,
 				       enum efx_filter_priority priority,
 				       u32 filter_id)
 {
-	return efx_ef10_filter_remove_internal(efx, 1U << priority,
-					       filter_id, false);
+	struct efx_ef10_filter_table *table;
+	int rc;
+
+	down_read(&efx->filter_sem);
+	table = efx->filter_state;
+	down_write(&table->lock);
+	rc = efx_ef10_filter_remove_internal(efx, 1U << priority, filter_id,
+					     false);
+	up_write(&table->lock);
+	up_read(&efx->filter_sem);
+	return rc;
 }
 
+/* Caller must hold efx->filter_sem for read */
 static void efx_ef10_filter_remove_unsafe(struct efx_nic *efx,
 					  enum efx_filter_priority priority,
 					  u32 filter_id)
 {
+	struct efx_ef10_filter_table *table = efx->filter_state;
+
 	if (filter_id == EFX_EF10_FILTER_ID_INVALID)
 		return;
-	efx_ef10_filter_remove_internal(efx, 1U << priority, filter_id, true);
+
+	down_write(&table->lock);
+	efx_ef10_filter_remove_internal(efx, 1U << priority, filter_id,
+					true);
+	up_write(&table->lock);
 }
 
 static int efx_ef10_filter_get_safe(struct efx_nic *efx,
@@ -4666,11 +4624,13 @@ static int efx_ef10_filter_get_safe(struct efx_nic *efx,
 				    u32 filter_id, struct efx_filter_spec *spec)
 {
 	unsigned int filter_idx = efx_ef10_filter_get_unsafe_id(filter_id);
-	struct efx_ef10_filter_table *table = efx->filter_state;
 	const struct efx_filter_spec *saved_spec;
+	struct efx_ef10_filter_table *table;
 	int rc;
 
-	spin_lock_bh(&efx->filter_lock);
+	down_read(&efx->filter_sem);
+	table = efx->filter_state;
+	down_read(&table->lock);
 	saved_spec = efx_ef10_filter_entry_spec(table, filter_idx);
 	if (saved_spec && saved_spec->priority == priority &&
 	    efx_ef10_filter_pri(table, saved_spec) ==
@@ -4680,13 +4640,15 @@ static int efx_ef10_filter_get_safe(struct efx_nic *efx,
 	} else {
 		rc = -ENOENT;
 	}
-	spin_unlock_bh(&efx->filter_lock);
+	up_read(&table->lock);
+	up_read(&efx->filter_sem);
 	return rc;
 }
 
 static int efx_ef10_filter_clear_rx(struct efx_nic *efx,
-				     enum efx_filter_priority priority)
+				    enum efx_filter_priority priority)
 {
+	struct efx_ef10_filter_table *table;
 	unsigned int priority_mask;
 	unsigned int i;
 	int rc;
@@ -4694,31 +4656,40 @@ static int efx_ef10_filter_clear_rx(struct efx_nic *efx,
 	priority_mask = (((1U << (priority + 1)) - 1) &
 			 ~(1U << EFX_FILTER_PRI_AUTO));
 
+	down_read(&efx->filter_sem);
+	table = efx->filter_state;
+	down_write(&table->lock);
 	for (i = 0; i < HUNT_FILTER_TBL_ROWS; i++) {
 		rc = efx_ef10_filter_remove_internal(efx, priority_mask,
 						     i, true);
 		if (rc && rc != -ENOENT)
-			return rc;
+			break;
+		rc = 0;
 	}
 
-	return 0;
+	up_write(&table->lock);
+	up_read(&efx->filter_sem);
+	return rc;
 }
 
 static u32 efx_ef10_filter_count_rx_used(struct efx_nic *efx,
 					 enum efx_filter_priority priority)
 {
-	struct efx_ef10_filter_table *table = efx->filter_state;
+	struct efx_ef10_filter_table *table;
 	unsigned int filter_idx;
 	s32 count = 0;
 
-	spin_lock_bh(&efx->filter_lock);
+	down_read(&efx->filter_sem);
+	table = efx->filter_state;
+	down_read(&table->lock);
 	for (filter_idx = 0; filter_idx < HUNT_FILTER_TBL_ROWS; filter_idx++) {
 		if (table->entry[filter_idx].spec &&
 		    efx_ef10_filter_entry_spec(table, filter_idx)->priority ==
 		    priority)
 			++count;
 	}
-	spin_unlock_bh(&efx->filter_lock);
+	up_read(&table->lock);
+	up_read(&efx->filter_sem);
 	return count;
 }
 
@@ -4733,12 +4704,15 @@ static s32 efx_ef10_filter_get_rx_ids(struct efx_nic *efx,
 				      enum efx_filter_priority priority,
 				      u32 *buf, u32 size)
 {
-	struct efx_ef10_filter_table *table = efx->filter_state;
+	struct efx_ef10_filter_table *table;
 	struct efx_filter_spec *spec;
 	unsigned int filter_idx;
 	s32 count = 0;
 
-	spin_lock_bh(&efx->filter_lock);
+	down_read(&efx->filter_sem);
+	table = efx->filter_state;
+	down_read(&table->lock);
+
 	for (filter_idx = 0; filter_idx < HUNT_FILTER_TBL_ROWS; filter_idx++) {
 		spec = efx_ef10_filter_entry_spec(table, filter_idx);
 		if (spec && spec->priority == priority) {
@@ -4752,73 +4726,44 @@ static s32 efx_ef10_filter_get_rx_ids(struct efx_nic *efx,
 					filter_idx);
 		}
 	}
-	spin_unlock_bh(&efx->filter_lock);
+	up_read(&table->lock);
+	up_read(&efx->filter_sem);
 	return count;
 }
 
 #ifdef CONFIG_RFS_ACCEL
 
-static void
-efx_ef10_filter_rfs_expire_complete(struct efx_nic *efx,
-				    unsigned long filter_idx,
-				    int rc, efx_dword_t *outbuf,
-				    size_t outlen_actual);
-
 static bool efx_ef10_filter_rfs_expire_one(struct efx_nic *efx, u32 flow_id,
 					   unsigned int filter_idx)
 {
-	struct efx_ef10_filter_table *table = efx->filter_state;
+	struct efx_ef10_filter_table *table;
 	struct efx_filter_spec *spec;
-	MCDI_DECLARE_BUF(inbuf,
-			 MC_CMD_FILTER_OP_IN_HANDLE_OFST +
-			 MC_CMD_FILTER_OP_IN_HANDLE_LEN);
-	bool ret = true;
+	bool ret;
 
-	spin_lock_bh(&efx->filter_lock);
+	down_read(&efx->filter_sem);
+	table = efx->filter_state;
+	down_write(&table->lock);
 	spec = efx_ef10_filter_entry_spec(table, filter_idx);
-	if (!spec ||
-	    (table->entry[filter_idx].spec & EFX_EF10_FILTER_FLAG_BUSY) ||
-	    spec->priority != EFX_FILTER_PRI_HINT ||
-	    !rps_may_expire_flow(efx->net_dev, spec->dmaq_id,
+
+	if (!spec || spec->priority != EFX_FILTER_PRI_HINT) {
+		ret = true;
+		goto out_unlock;
+	}
+
+	if (!rps_may_expire_flow(efx->net_dev, spec->dmaq_id,
 				 flow_id, filter_idx)) {
 		ret = false;
 		goto out_unlock;
 	}
 
-	MCDI_SET_DWORD(inbuf, FILTER_OP_IN_OP,
-		       MC_CMD_FILTER_OP_IN_OP_REMOVE);
-	MCDI_SET_QWORD(inbuf, FILTER_OP_IN_HANDLE,
-		       table->entry[filter_idx].handle);
-	if (efx_mcdi_rpc_async(efx, MC_CMD_FILTER_OP, inbuf, sizeof(inbuf), 0,
-			       efx_ef10_filter_rfs_expire_complete, filter_idx))
-		ret = false;
-	else
-		table->entry[filter_idx].spec |= EFX_EF10_FILTER_FLAG_BUSY;
+	ret = efx_ef10_filter_remove_internal(efx, 1U << spec->priority,
+					      filter_idx, true) == 0;
 out_unlock:
-	spin_unlock_bh(&efx->filter_lock);
+	up_write(&table->lock);
+	up_read(&efx->filter_sem);
 	return ret;
 }
 
-static void
-efx_ef10_filter_rfs_expire_complete(struct efx_nic *efx,
-				    unsigned long filter_idx,
-				    int rc, efx_dword_t *outbuf,
-				    size_t outlen_actual)
-{
-	struct efx_ef10_filter_table *table = efx->filter_state;
-	struct efx_filter_spec *spec =
-		efx_ef10_filter_entry_spec(table, filter_idx);
-
-	spin_lock_bh(&efx->filter_lock);
-	if (rc == 0) {
-		kfree(spec);
-		efx_ef10_filter_set_entry(table, filter_idx, NULL, 0);
-	}
-	table->entry[filter_idx].spec &= ~EFX_EF10_FILTER_FLAG_BUSY;
-	wake_up_all(&table->waitq);
-	spin_unlock_bh(&efx->filter_lock);
-}
-
 #endif /* CONFIG_RFS_ACCEL */
 
 static int efx_ef10_filter_match_flags_from_mcdi(bool encap, u32 mcdi_flags)
@@ -5011,9 +4956,9 @@ static int efx_ef10_filter_table_probe(struct efx_nic *efx)
 	table->vlan_filter =
 		!!(efx->net_dev->features & NETIF_F_HW_VLAN_CTAG_FILTER);
 	INIT_LIST_HEAD(&table->vlan_list);
+	init_rwsem(&table->lock);
 
 	efx->filter_state = table;
-	init_waitqueue_head(&table->waitq);
 
 	list_for_each_entry(vlan, &nic_data->vlan_list, list) {
 		rc = efx_ef10_filter_add_vlan(efx, vlan->vid);
@@ -5055,7 +5000,7 @@ static void efx_ef10_filter_table_restore(struct efx_nic *efx)
 	if (!table)
 		return;
 
-	spin_lock_bh(&efx->filter_lock);
+	down_write(&table->lock);
 
 	for (filter_idx = 0; filter_idx < HUNT_FILTER_TBL_ROWS; filter_idx++) {
 		spec = efx_ef10_filter_entry_spec(table, filter_idx);
@@ -5093,15 +5038,11 @@ static void efx_ef10_filter_table_restore(struct efx_nic *efx)
 			}
 		}
 
-		table->entry[filter_idx].spec |= EFX_EF10_FILTER_FLAG_BUSY;
-		spin_unlock_bh(&efx->filter_lock);
-
 		rc = efx_ef10_filter_push(efx, spec,
 					  &table->entry[filter_idx].handle,
 					  ctx, false);
 		if (rc)
 			failed++;
-		spin_lock_bh(&efx->filter_lock);
 
 		if (rc) {
 not_restored:
@@ -5113,13 +5054,10 @@ static void efx_ef10_filter_table_restore(struct efx_nic *efx)
 
 			kfree(spec);
 			efx_ef10_filter_set_entry(table, filter_idx, NULL, 0);
-		} else {
-			table->entry[filter_idx].spec &=
-				~EFX_EF10_FILTER_FLAG_BUSY;
 		}
 	}
 
-	spin_unlock_bh(&efx->filter_lock);
+	up_write(&table->lock);
 
 	/* This can happen validly if the MC's capabilities have changed, so
 	 * is not an error.
@@ -5187,6 +5125,8 @@ static void efx_ef10_filter_mark_one_old(struct efx_nic *efx, uint16_t *id)
 	struct efx_ef10_filter_table *table = efx->filter_state;
 	unsigned int filter_idx;
 
+	efx_rwsem_assert_write_locked(&table->lock);
+
 	if (*id != EFX_EF10_FILTER_ID_INVALID) {
 		filter_idx = efx_ef10_filter_get_unsafe_id(*id);
 		if (!table->entry[filter_idx].spec)
@@ -5222,10 +5162,10 @@ static void efx_ef10_filter_mark_old(struct efx_nic *efx)
 	struct efx_ef10_filter_table *table = efx->filter_state;
 	struct efx_ef10_filter_vlan *vlan;
 
-	spin_lock_bh(&efx->filter_lock);
+	down_write(&table->lock);
 	list_for_each_entry(vlan, &table->vlan_list, list)
 		_efx_ef10_filter_vlan_mark_old(efx, vlan);
-	spin_unlock_bh(&efx->filter_lock);
+	up_write(&table->lock);
 }
 
 static void efx_ef10_filter_uc_addr_list(struct efx_nic *efx)
@@ -5502,10 +5442,7 @@ static int efx_ef10_filter_insert_def(struct efx_nic *efx,
 	return rc;
 }
 
-/* Remove filters that weren't renewed.  Since nothing else changes the AUTO_OLD
- * flag or removes these filters, we don't need to hold the filter_lock while
- * scanning for these filters.
- */
+/* Remove filters that weren't renewed. */
 static void efx_ef10_filter_remove_old(struct efx_nic *efx)
 {
 	struct efx_ef10_filter_table *table = efx->filter_state;
@@ -5514,6 +5451,7 @@ static void efx_ef10_filter_remove_old(struct efx_nic *efx)
 	int rc;
 	int i;
 
+	down_write(&table->lock);
 	for (i = 0; i < HUNT_FILTER_TBL_ROWS; i++) {
 		if (READ_ONCE(table->entry[i].spec) &
 		    EFX_EF10_FILTER_FLAG_AUTO_OLD) {
@@ -5525,6 +5463,7 @@ static void efx_ef10_filter_remove_old(struct efx_nic *efx)
 				remove_failed++;
 		}
 	}
+	up_write(&table->lock);
 
 	if (remove_failed)
 		netif_info(efx, drv, efx->net_dev,
diff --git a/drivers/net/ethernet/sfc/efx.c b/drivers/net/ethernet/sfc/efx.c
index 0be93abdb2ad..cb9273016916 100644
--- a/drivers/net/ethernet/sfc/efx.c
+++ b/drivers/net/ethernet/sfc/efx.c
@@ -1783,7 +1783,6 @@ static int efx_probe_filters(struct efx_nic *efx)
 {
 	int rc;
 
-	spin_lock_init(&efx->filter_lock);
 	init_rwsem(&efx->filter_sem);
 	mutex_lock(&efx->mac_lock);
 	down_write(&efx->filter_sem);
diff --git a/drivers/net/ethernet/sfc/net_driver.h b/drivers/net/ethernet/sfc/net_driver.h
index fe0c5322454d..6f37a4dbe268 100644
--- a/drivers/net/ethernet/sfc/net_driver.h
+++ b/drivers/net/ethernet/sfc/net_driver.h
@@ -836,7 +836,7 @@ struct efx_rss_context {
  * @loopback_mode: Loopback status
  * @loopback_modes: Supported loopback mode bitmask
  * @loopback_selftest: Offline self-test private state
- * @filter_sem: Filter table rw_semaphore, for freeing the table
+ * @filter_sem: Filter table rw_semaphore, protects existence of @filter_state
  * @filter_lock: Filter table lock, for mere content changes
  * @filter_state: Architecture-dependent filter table state
  * @rps_mutex: Protects RPS state of all channels

^ 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