Netdev List
 help / color / mirror / Atom feed
* [net: PATCH] net: mvpp2: disable link IRQ waiting for IDLE frames
From: Marcin Wojtas @ 2019-02-27 17:47 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel, netdev
  Cc: davem, linux, antoine.tenart, thomas.petazzoni, gregory.clement,
	stefanc, nadavh, mw, jaz, tn

Current version of the driver was configuring XLG MAC
in a way to wait 3 IDLE frames before allowing for the
link-up interrupt to be triggered. This resulted in an
issue, preventing to detect the link change during RX
traffic on the interface. Fix that.

Fixes: 4bb043262878 ("net: mvpp2: phylink support")
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index 16066c2..f1378f9 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -4532,8 +4532,7 @@ static void mvpp2_xlg_config(struct mvpp2_port *port, unsigned int mode,
 		ctrl0 |= MVPP22_XLG_CTRL0_RX_FLOW_CTRL_EN;
 
 	ctrl4 &= ~MVPP22_XLG_CTRL4_MACMODSELECT_GMAC;
-	ctrl4 |= MVPP22_XLG_CTRL4_FWD_FC | MVPP22_XLG_CTRL4_FWD_PFC |
-		 MVPP22_XLG_CTRL4_EN_IDLE_CHECK;
+	ctrl4 |= MVPP22_XLG_CTRL4_FWD_FC | MVPP22_XLG_CTRL4_FWD_PFC;
 
 	writel(ctrl0, port->base + MVPP22_XLG_CTRL0_REG);
 	writel(ctrl4, port->base + MVPP22_XLG_CTRL4_REG);
-- 
2.7.4


^ permalink raw reply related

* Re: [PATCH v4 perf,bpf 14/15] perf: introduce side band thread
From: Song Liu @ 2019-02-27 17:52 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Netdev, linux-kernel, ast@kernel.org, daniel@iogearbox.net,
	Kernel Team, peterz@infradead.org, acme@redhat.com,
	jolsa@kernel.org, namhyung@kernel.org
In-Reply-To: <20190227132105.GC18893@krava>



> On Feb 27, 2019, at 5:21 AM, Jiri Olsa <jolsa@redhat.com> wrote:
> 
> On Mon, Feb 25, 2019 at 04:20:18PM -0800, Song Liu wrote:
> 
> SNIP
> 
>> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
>> index 8c902276d4b4..61b87c8111e6 100644
>> --- a/tools/perf/util/evlist.c
>> +++ b/tools/perf/util/evlist.c
>> @@ -19,6 +19,7 @@
>> #include "debug.h"
>> #include "units.h"
>> #include "asm/bug.h"
>> +#include "bpf-event.h"
>> #include <signal.h>
>> #include <unistd.h>
>> 
>> @@ -1841,3 +1842,102 @@ struct perf_evsel *perf_evlist__reset_weak_group(struct perf_evlist *evsel_list,
>> 	}
>> 	return leader;
>> }
>> +
>> +static struct perf_evlist *sb_evlist;
>> +pthread_t poll_thread;
> 
> so some of the things are static and some like poll_args
> you alloced on the stack.. I dont like this interface,
> could we come up with something generic? perhaps
> encapsulated in perf_evlist, like:

I picked global sb_evlist and poll_thread because there 
should be only one sb_evlist and one thread polling it. 
There might be multiple evsel on the sb_evlist. 

I am not sure I understand your suggestion..

> struct perf_evlist {
> 	...
> 	struct {
> 		pthread_t	th;
> 		int		state;
> 	} thread;

This will not be used by the main perf_evlist, right?

> };
> 
> typedef int (perf_evlist__thread_cb_t)(perf_evlist, union perf_event *event,....)

How do we use this callback? We need a way to specify which
events to poll (mmap, bpf_event, ksymbol, etc). How do we do 
that with the callback function?

Thanks,
Song

> perf_evlist__start_thread(perf_evlist, perf_evlist__thread_cb_t cb);
> perf_evlist__stop_thread(perf_evlist);
> 
> 
> jirka


^ permalink raw reply

* Re: [PATCH v4] net: phy: Micrel KSZ8061: link failure after cable connect
From: David Miller @ 2019-02-27 18:05 UTC (permalink / raw)
  To: T.Rajasingh
  Cc: alexander.onnasch, andrew, f.fainelli, hkallweit1, netdev,
	linux-kernel
In-Reply-To: <20190227121319.4106-1-T.Rajasingh@landisgyr.com>

From: Rajasingh Thavamani <T.Rajasingh@landisgyr.com>
Date: Wed, 27 Feb 2019 17:43:19 +0530

> With Micrel KSZ8061 PHY, the link may occasionally not come up after
> Ethernet cable connect. The vendor's (Microchip, former Micrel) errata
> sheet 80000688A.pdf descripes the problem and possible workarounds in
> detail, see below.
> The batch implements workaround 1, which permanently fixes the issue.
 ...
> Fixes: 7ab59dc15e2f ("drivers/net/phy/micrel_phy: Add support for new PHYs")
> Signed-off-by: Alexander Onnasch <alexander.onnasch@landisgyr.com>
> Signed-off-by: Rajasingh Thavamani <T.Rajasingh@landisgyr.com>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>

Applied and queued up for -stable, thanks.

^ permalink raw reply

* Re: [PATCH][net-next] ethtool: Use explicit designated initializers for .cmd
From: David Miller @ 2019-02-27 18:08 UTC (permalink / raw)
  To: lirongqing; +Cc: netdev
In-Reply-To: <1551271677-14492-1-git-send-email-lirongqing@baidu.com>

From: Li RongQing <lirongqing@baidu.com>
Date: Wed, 27 Feb 2019 20:47:57 +0800

> Initialize the .cmd member by using a designated struct
> initializer. This fixes warning of missing field initializers,
> and makes code a little easier to read.
> 
> Signed-off-by: Li RongQing <lirongqing@baidu.com>

Applied.

^ permalink raw reply

* Re: [PATCH net-next] Revert "net: sched: route: don't set arg->stop in route4_walk() when empty"
From: David Miller @ 2019-02-27 18:12 UTC (permalink / raw)
  To: vladbu; +Cc: netdev, jhs, xiyou.wangcong, jiri
In-Reply-To: <20190227134859.18424-1-vladbu@mellanox.com>

From: Vlad Buslov <vladbu@mellanox.com>
Date: Wed, 27 Feb 2019 15:48:59 +0200

> This reverts commit 3027ff41f67c ("net: sched: route: don't set arg->stop
> in route4_walk() when empty")
> 
> Cls API function tcf_proto_is_empty() was changed in commit
> 6676d5e416ee ("net: sched: set dedicated tcf_walker flag when tp is empty")
> to no longer depend on arg->stop to determine that classifier instance is
> empty. Instead, it adds dedicated arg->nonempty field, which makes the fix
> in route classifier no longer necessary.
> 
> Signed-off-by: Vlad Buslov <vladbu@mellanox.com>

Looks good, applied, thanks Vlad.

^ permalink raw reply

* Re: [PATCH] appletalk: Fix use-after-free in atalk_proc_exit
From: David Miller @ 2019-02-27 18:14 UTC (permalink / raw)
  To: yuehaibing; +Cc: joe, gregkh, linux-kernel, netdev
In-Reply-To: <20190227145900.28808-1-yuehaibing@huawei.com>

From: Yue Haibing <yuehaibing@huawei.com>
Date: Wed, 27 Feb 2019 22:59:00 +0800

> @@ -1925,7 +1925,10 @@ static int __init atalk_init(void)
>  
>  	register_netdevice_notifier(&ddp_notifier);
>  	aarp_proto_init();
> -	atalk_proc_init();
> +	rc = atalk_proc_init();
> +	if (rc)
> +		goto out;
> +
>  	atalk_register_sysctl();
>  out:

This leaves the netdevice notifier registered etc.  You need to add a proper
error cleanup code path.

^ permalink raw reply

* [PATCH] netfilter: xt_IDLETIMER: fix sysfs callback function type
From: Sami Tolvanen @ 2019-02-27 18:19 UTC (permalink / raw)
  To: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal
  Cc: David S . Miller, netfilter-devel, coreteam, netdev, linux-kernel,
	Sami Tolvanen

Use struct device_attribute instead of struct idletimer_tg_attr, and
the correct callback function type to avoid indirect call mismatches
with Control Flow Integrity checking.

Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
---
 net/netfilter/xt_IDLETIMER.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/net/netfilter/xt_IDLETIMER.c b/net/netfilter/xt_IDLETIMER.c
index eb4cbd244c3d..5f9b37e12801 100644
--- a/net/netfilter/xt_IDLETIMER.c
+++ b/net/netfilter/xt_IDLETIMER.c
@@ -41,19 +41,13 @@
 #include <linux/workqueue.h>
 #include <linux/sysfs.h>
 
-struct idletimer_tg_attr {
-	struct attribute attr;
-	ssize_t	(*show)(struct kobject *kobj,
-			struct attribute *attr, char *buf);
-};
-
 struct idletimer_tg {
 	struct list_head entry;
 	struct timer_list timer;
 	struct work_struct work;
 
 	struct kobject *kobj;
-	struct idletimer_tg_attr attr;
+	struct device_attribute attr;
 
 	unsigned int refcnt;
 };
@@ -76,15 +70,15 @@ struct idletimer_tg *__idletimer_tg_find_by_label(const char *label)
 	return NULL;
 }
 
-static ssize_t idletimer_tg_show(struct kobject *kobj, struct attribute *attr,
-				 char *buf)
+static ssize_t idletimer_tg_show(struct device *dev,
+				 struct device_attribute *attr, char *buf)
 {
 	struct idletimer_tg *timer;
 	unsigned long expires = 0;
 
 	mutex_lock(&list_mutex);
 
-	timer =	__idletimer_tg_find_by_label(attr->name);
+	timer =	__idletimer_tg_find_by_label(attr->attr.name);
 	if (timer)
 		expires = timer->timer.expires;
 
-- 
2.21.0.rc2.261.ga7da99ff1b-goog


^ permalink raw reply related

* Re: [PATCH bpf-next] bpf: add missing entries to bpf_helpers.h
From: Song Liu @ 2019-02-27 18:28 UTC (permalink / raw)
  To: Willem de Bruijn
  Cc: Networking, Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau,
	Stanislav Fomichev, Willem de Bruijn
In-Reply-To: <20190227160806.195218-1-willemdebruijn.kernel@gmail.com>

On Wed, Feb 27, 2019 at 8:09 AM Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
>
> From: Willem de Bruijn <willemb@google.com>
>
> This header defines the BPF functions enumerated in uapi/linux.bpf.h
> in a callable format. Expand to include all registered functions.
>
> Signed-off-by: Willem de Bruijn <willemb@google.com>

Acked-by: Song Liu <songliubraving@fb.com>

>
> ---
>
> changes pre->v1:
>   - add brief commit message
>     (not copying Acks as they do not necessarily agree with that)
>
>   - change return type from ull to uint for
>     bpf_get_cgroup_classid, bpf_get_route_realm, bpf_get_hash_recalc
>
>   - change size arg from int to __u32 for bpf_probe_read_str
>
> ---
>  tools/testing/selftests/bpf/bpf_helpers.h | 30 +++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
> index d9999f1ed1d2a..026bea831e039 100644
> --- a/tools/testing/selftests/bpf/bpf_helpers.h
> +++ b/tools/testing/selftests/bpf/bpf_helpers.h
> @@ -232,6 +232,36 @@ static int (*bpf_skb_change_head)(void *, int len, int flags) =
>         (void *) BPF_FUNC_skb_change_head;
>  static int (*bpf_skb_pull_data)(void *, int len) =
>         (void *) BPF_FUNC_skb_pull_data;
> +static unsigned int (*bpf_get_cgroup_classid)(void *ctx) =
> +       (void *) BPF_FUNC_get_cgroup_classid;
> +static unsigned int (*bpf_get_route_realm)(void *ctx) =
> +       (void *) BPF_FUNC_get_route_realm;
> +static int (*bpf_skb_change_proto)(void *ctx, __be16 proto, __u64 flags) =
> +       (void *) BPF_FUNC_skb_change_proto;
> +static int (*bpf_skb_change_type)(void *ctx, __u32 type) =
> +       (void *) BPF_FUNC_skb_change_type;
> +static unsigned int (*bpf_get_hash_recalc)(void *ctx) =
> +       (void *) BPF_FUNC_get_hash_recalc;
> +static unsigned long long (*bpf_get_current_task)(void *ctx) =
> +       (void *) BPF_FUNC_get_current_task;
> +static int (*bpf_skb_change_tail)(void *ctx, __u32 len, __u64 flags) =
> +       (void *) BPF_FUNC_skb_change_tail;
> +static long long (*bpf_csum_update)(void *ctx, __u32 csum) =
> +       (void *) BPF_FUNC_csum_update;
> +static void (*bpf_set_hash_invalid)(void *ctx) =
> +       (void *) BPF_FUNC_set_hash_invalid;
> +static int (*bpf_get_numa_node_id)(void) =
> +       (void *) BPF_FUNC_get_numa_node_id;
> +static int (*bpf_probe_read_str)(void *ctx, __u32 size,
> +                                const void *unsafe_ptr) =
> +       (void *) BPF_FUNC_probe_read_str;
> +static unsigned int (*bpf_get_socket_uid)(void *ctx) =
> +       (void *) BPF_FUNC_get_socket_uid;
> +static unsigned int (*bpf_set_hash)(void *ctx, __u32 hash) =
> +       (void *) BPF_FUNC_set_hash;
> +static int (*bpf_skb_adjust_room)(void *ctx, __s32 len_diff, __u32 mode,
> +                                 unsigned long long flags) =
> +       (void *) BPF_FUNC_skb_adjust_room;
>
>  /* Scan the ARCH passed in from ARCH env variable (see Makefile) */
>  #if defined(__TARGET_ARCH_x86)
> --
> 2.21.0.rc2.261.ga7da99ff1b-goog
>

^ permalink raw reply

* Re: [PATCH net-next 4/8] devlink: allow subports on devlink PCI ports
From: Jakub Kicinski @ 2019-02-27 18:30 UTC (permalink / raw)
  To: Jiri Pirko; +Cc: davem, oss-drivers, netdev, parav, jgg
In-Reply-To: <20190227123753.GB2240@nanopsycho>

On Wed, 27 Feb 2019 13:37:53 +0100, Jiri Pirko wrote:
> Tue, Feb 26, 2019 at 07:24:32PM CET, jakub.kicinski@netronome.com wrote:
> >PCI endpoint corresponds to a PCI device, but such device
> >can have one more more logical device ports associated with it.
> >We need a way to distinguish those. Add a PCI subport in the
> >dumps and print the info in phys_port_name appropriately.
> >
> >This is not equivalent to port splitting, there is no split
> >group. It's just a way of representing multiple netdevs on
> >a single PCI function.
> >
> >Note that the quality of being multiport pertains only to
> >the PCI function itself. A PF having multiple netdevs does
> >not mean that its VFs will also have multiple, or that VFs
> >are associated with any particular port of a multiport VF.
> 
> We've been discussing the problem of subport (we call it "subfunction"
> or "SF") for some time internally. Turned out, this is probably harder
> task to model. Please prove me wrong.
> 
> The nature of VF makes it a logically separate entity. It has a separate
> PCI address, it should therefore have a separate devlink instance.
> You can pass it through to VM, then the same devlink instance should be
> created inside the VM and disappear from the host.

Depends what a devlink instance represents :/  On one hand you may want
to create an instance for a VF to allow it to spawn soft ports, on the
other you may want to group multiple functions together.

IOW if devlink instance is for an ASIC, there should be one per device
per host.  So if we start connecting multiple functions (PFs and/or VFs)
to one host we should probably introduce the notion of devlink aliases
or some such (so that multiple bus addresses can target the same
devlink instance).  Those less pipelined NICs can forward between
ports, but still want a function per port (otherwise user space
sometimes gets confused).  If we have multiple functions which are on
the same "switchid" they should have a single devlink instance if you
ask me.  That instance will have all the ports of the device.

You say disappear from the host - what do you mean.  Are you referring
to the VF port disappearing?  But on the switch the port is still
there, and you should show the subports on the PF side IMHO.  Devlink
ports should allow users to understand the topology of the switch.

Is spawning VMDq sub-instances the only thing we can think of that VMs
may want to do?  Are there any other uses?

> SF (or subport) feels similar to that. Basically it is exactly the same
> thing as VF, only does reside under PF PCI function.
> 
> That is why I think, for sake of consistency, it should have a separate
> devlink entity as well. The problem is correct sysfs modelling and
> devlink handle derived from that. Parav is working on a simple soft
> bus for this purpose called "subbus". There is a RFC floating around on
> Mellanox internal mailing list, looks like it is time to send it
> upstream.
> 
> Then each PF driver which have SFs would register subbus devices
> according to SFs/subports and they would be properly handled by bus
> probe, devlink and devlink port and netdev instances created.
> 
> Ccing Parav and Jason.

You guys come from the RDMA side of the world, with which I'm less
familiar, and the soft bus + spawning devices seems to be a popular
design there.  Could you describe the advantages of that model for 
the sake of the netdev-only folks? :)

Another term that gets thrown into the mix here is mediated devices,
right?  If you wanna pass the sub-spawn-soft-port to a VM.  Or run 
DPDK on some queues.

To state the obvious AF_XDP and macvlan offload were are previous
answers to some of those use cases.  What is the forwarding model
for those subports?  Are we going to allow flower rules from VMs?
Is it going to be dst MAC only?  Or is the hypervisor going to forward
as it sees appropriate (OvS + "repr"/port netdev)?

^ permalink raw reply

* Re: [PATCH -next] appletalk: use remove_proc_subtree to simplify procfs code
From: kbuild test robot @ 2019-02-27 18:32 UTC (permalink / raw)
  To: Yue Haibing
  Cc: kbuild-all, davem, joe, gregkh, linux-kernel, netdev, YueHaibing
In-Reply-To: <20190227150023.28420-1-yuehaibing@huawei.com>

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

Hi YueHaibing,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on next-20190227]

url:    https://github.com/0day-ci/linux/commits/Yue-Haibing/appletalk-use-remove_proc_subtree-to-simplify-procfs-code/20190228-014856
config: xtensa-allyesconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 8.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=8.2.0 make.cross ARCH=xtensa 

All warnings (new ones prefixed by >>):

   net//appletalk/atalk_proc.c: In function 'atalk_proc_init':
>> net//appletalk/atalk_proc.c:218:2: warning: this 'if' clause does not guard... [-Wmisleading-indentation]
     if (!proc_create_seq("atalk/interface", 0444, init_net.proc_net,
     ^~
   net//appletalk/atalk_proc.c:220:3: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'if'
      goto out;
      ^~~~
   net//appletalk/atalk_proc.c:222:2: warning: this 'if' clause does not guard... [-Wmisleading-indentation]
     if (!proc_create_seq("atalk/route", 0444, init_net.proc_net,
     ^~
   net//appletalk/atalk_proc.c:224:3: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'if'
      goto out;
      ^~~~
   net//appletalk/atalk_proc.c:226:2: warning: this 'if' clause does not guard... [-Wmisleading-indentation]
     if (!proc_create_seq("atalk/socket", 0444, init_net.proc_net,
     ^~
   net//appletalk/atalk_proc.c:228:3: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'if'
      goto out;
      ^~~~
   net//appletalk/atalk_proc.c:230:2: warning: this 'if' clause does not guard... [-Wmisleading-indentation]
     if (!proc_create_seq_private("atalk/arp", 0444, init_net.proc_net,
     ^~
   net//appletalk/atalk_proc.c:233:3: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'if'
      goto out;
      ^~~~

vim +/if +218 net//appletalk/atalk_proc.c

   212	
   213	int __init atalk_proc_init(void)
   214	{
   215		if (!proc_mkdir("atalk", init_net.proc_net))
   216			return -ENOMEM;
   217	
 > 218		if (!proc_create_seq("atalk/interface", 0444, init_net.proc_net,
   219				    &atalk_seq_interface_ops));
   220			goto out;
   221	
   222		if (!proc_create_seq("atalk/route", 0444, init_net.proc_net,
   223				    &atalk_seq_route_ops));
   224			goto out;
   225	
   226		if (!proc_create_seq("atalk/socket", 0444, init_net.proc_net,
   227				    &atalk_seq_socket_ops));
   228			goto out;
   229	
   230		if (!proc_create_seq_private("atalk/arp", 0444, init_net.proc_net,
   231					     &aarp_seq_ops,
   232					     sizeof(struct aarp_iter_state), NULL));
   233			goto out;
   234	
   235	out:
   236		remove_proc_subtree("atalk", init_net.proc_net);
   237		return -ENOMEM;
   238	}
   239	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 56809 bytes --]

^ permalink raw reply

* Re: Request for suggestion on net device re-naming/re-ordering based on DT alias
From: Stephen Hemminger @ 2019-02-27 18:40 UTC (permalink / raw)
  To: Harini Katakam
  Cc: netdev, linux-kernel, Nicolas Ferre, David Miller, Michal Simek,
	Harini Katakam, Harini Katakam
In-Reply-To: <CAFcVEC+8P25MKWhizUUEuamikaZYUOjpppV=59=W3POTEfQqXw@mail.gmail.com>

On Wed, 27 Feb 2019 17:24:03 +0530
Harini Katakam <harinik@xilinx.com> wrote:

> Hi,
> 
> We've had some users requesting control over net device name order
> when multiple ethernet devices are present on a system. I've tried a
> few solutions to this and looked it up on forums. But I apologize if
> I have missed something.
> 
> I know that the current system allocates eth<n> as per probe order
> but that is obviously not stably controlled by user (tried DT
> re-ordering and defer probe). One solution is to use DT alias names
> to write to (net_device)->name as follows:
> Devicetree:
> aliases {
>    ethernet0 = &mac1;
>    ethernet1 = &mac0;
> };
> Driver probe:
> +       /* Read ethernet DT alias id and assign to right device name*/
> +       id = of_alias_get_id(np, "ethernet");
> +       if (id < 0) {
> +               dev_warn(&pdev->dev, "failed to get alias id (%u)\n", id);
> +               return id;
> +       }
> +       snprintf(dev->name, sizeof(dev->name), "eth%d", id);
> +
> 
> These three drivers seem to have something similar for mdio/phy bus IDs:
> drivers/net/ethernet/broadcom/genet/bcmmii.c:409: id =
> of_alias_get_id(dn, "eth");
> drivers/net/ethernet/samsung/sxgbe/sxgbe_platform.c:43: plat->bus_id =
> of_alias_get_id(np, "ethernet");
> drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c:404:
> plat->bus_id = of_alias_get_id(np, "ethernet");
> 
> Drawback: This approach will break if alias is not provided for one
> of the interfaces on board. Not to mention, there could be systems
> with multiple ethernet makes (for ex. Cadence macb and Xilinx axienet)
> If one of the drivers does not have an alias read mechanism, it is
> possible to have clashing ID assignments. Is there any way this
> solution can be changed to be stable/acceptable?
> 
> One other alternative I've tried is netdev kernel bootargs but this
> device name was not being picked by the kernel:
> https://www.kernel.org/doc/html/v4.19/admin-guide/kernel-parameters.html
> netdev= <mac1’s interrupt id>, <mac1’s base address>, eth0
> netdev=<mac0’s interrupt id>, <mac0’s base address>, eth1
> 
> Could you please suggest any alternatives?
> Thanks!
> 
> Regards,
> Harini

Device naming is a hard problem, and there is no perfect solution.

Device tree should be providing hints to userspace policy for naming, not
trying to do it in the kernel. 
See: https://www.freedesktop.org/wiki/Software/systemd/PredictableNetworkInterfaceNames/

^ permalink raw reply

* Re: Request for suggestion on net device re-naming/re-ordering based on DT alias
From: Florian Fainelli @ 2019-02-27 18:45 UTC (permalink / raw)
  To: Stephen Hemminger, Harini Katakam
  Cc: netdev, linux-kernel, Nicolas Ferre, David Miller, Michal Simek,
	Harini Katakam, Harini Katakam
In-Reply-To: <20190227104028.707a75e2@shemminger-XPS-13-9360>

On 2/27/19 10:40 AM, Stephen Hemminger wrote:
> On Wed, 27 Feb 2019 17:24:03 +0530
> Harini Katakam <harinik@xilinx.com> wrote:
> 
>> Hi,
>>
>> We've had some users requesting control over net device name order
>> when multiple ethernet devices are present on a system. I've tried a
>> few solutions to this and looked it up on forums. But I apologize if
>> I have missed something.
>>
>> I know that the current system allocates eth<n> as per probe order
>> but that is obviously not stably controlled by user (tried DT
>> re-ordering and defer probe). One solution is to use DT alias names
>> to write to (net_device)->name as follows:
>> Devicetree:
>> aliases {
>>    ethernet0 = &mac1;
>>    ethernet1 = &mac0;
>> };
>> Driver probe:
>> +       /* Read ethernet DT alias id and assign to right device name*/
>> +       id = of_alias_get_id(np, "ethernet");
>> +       if (id < 0) {
>> +               dev_warn(&pdev->dev, "failed to get alias id (%u)\n", id);
>> +               return id;
>> +       }
>> +       snprintf(dev->name, sizeof(dev->name), "eth%d", id);
>> +
>>
>> These three drivers seem to have something similar for mdio/phy bus IDs:
>> drivers/net/ethernet/broadcom/genet/bcmmii.c:409: id =
>> of_alias_get_id(dn, "eth");
>> drivers/net/ethernet/samsung/sxgbe/sxgbe_platform.c:43: plat->bus_id =
>> of_alias_get_id(np, "ethernet");
>> drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c:404:
>> plat->bus_id = of_alias_get_id(np, "ethernet");
>>
>> Drawback: This approach will break if alias is not provided for one
>> of the interfaces on board. Not to mention, there could be systems
>> with multiple ethernet makes (for ex. Cadence macb and Xilinx axienet)
>> If one of the drivers does not have an alias read mechanism, it is
>> possible to have clashing ID assignments. Is there any way this
>> solution can be changed to be stable/acceptable?
>>
>> One other alternative I've tried is netdev kernel bootargs but this
>> device name was not being picked by the kernel:
>> https://www.kernel.org/doc/html/v4.19/admin-guide/kernel-parameters.html
>> netdev= <mac1’s interrupt id>, <mac1’s base address>, eth0
>> netdev=<mac0’s interrupt id>, <mac0’s base address>, eth1
>>
>> Could you please suggest any alternatives?
>> Thanks!
>>
>> Regards,
>> Harini
> 
> Device naming is a hard problem, and there is no perfect solution.
> 
> Device tree should be providing hints to userspace policy for naming, not
> trying to do it in the kernel. 

And Device Tree does already, if you look at the uevent attributes that
the kernel sends, there should be ample information to uniquely
determine which physical network device the network device maps to, e
for instance:

cat /sys/class/net/eth*/device/uevent
DRIVER=brcm-systemport
OF_NAME=ethernet
OF_FULLNAME=/rdb/ethernet@9300000
OF_TYPE=network
OF_COMPATIBLE_0=brcm,systemportlite-v1.00
OF_COMPATIBLE_1=brcm,systemport
OF_COMPATIBLE_N=2
OF_ALIAS_0=eth0
MODALIAS=of:NethernetTnetworkCbrcm,systemportlite-v1.00Cbrcm,systemport
-- 
Florian

^ permalink raw reply

* Re: [PATCH net-next 6/8] devlink: introduce port's peer netdevs
From: Jakub Kicinski @ 2019-02-27 18:47 UTC (permalink / raw)
  To: Jiri Pirko; +Cc: davem, oss-drivers, netdev
In-Reply-To: <20190227130829.GC2240@nanopsycho>

On Wed, 27 Feb 2019 14:08:29 +0100, Jiri Pirko wrote:
> Tue, Feb 26, 2019 at 07:24:34PM CET, jakub.kicinski@netronome.com wrote:
> >Devlink ports represent ports of a switch device (or SR-IOV
> >NIC which has an embedded switch). In case of SR-IOV when
> >PCIe PFs are exposed the PFs which are directly connected
> >to the local machine may also spawn PF netdev (much like
> >VFs have a port/"repr" and an actual VF netdev).
> >
> >Allow devlink to expose such linking. There is currently no
> >way to find out which netdev corresponds to which PF.
> >
> >Example:
> >
> >$ devlink port
> >pci/0000:82:00.0/0: type eth netdev p4p1 flavour physical
> >pci/0000:82:00.0/10000: type eth netdev eth1 flavour pci_pf pf 0 peer_netdev enp130s0
> >pci/0000:82:00.0/10001: type eth netdev eth0 flavour pci_vf pf 0 vf 0
> >pci/0000:82:00.0/10002: type eth netdev eth2 flavour pci_vf pf 0 vf 1  
> 
> Peer as the other side of a "virtual cable". For PF, that is probably
> sufficient. But I think what a "peer of devlink port" should be "a
> devlink port".

Maybe I'm not clear on what devlink port is - to me its a port of the
ASIC.  The notion of devlink port connected to devlink port seems
to counter such definition :S  

I do not think that every netdev should have a devlink port associated.

> Not sure about VF.
> 
> Consider a simple problem of setting up a VF mac address. In legacy, you
> do it like this:
> $ ip link set eth2 vf 1 mac 00:52:44:11:22:33
> However, in new model, you so far cannot do that.

Why?

$ devlink port set pci/0000:82:00.0/10001 peer_eth_addr 00:52:44:11:22:33

It's more of a neighbour info situation than a local port situation.

> What I was thinking about was some "dummy peer" which would be on the
> host. Not sure if only as a "dummy peer devlink port" or even as some
> sort of "dummy netdev".
> 
> One way or another, it would provide the user some info about which VF
> representor is connected to which VF in VM (mac mapping).

Ack, but isn't the MAC setting is the only thing we're missing from
"switchdev SR-IOV"?  Would the "dummy netdev" be used for anything
else?  I would rather not introduce new netdev just to do that 
(that'd be a third for that port.)

^ permalink raw reply

* Re: stmmac / meson8b-dwmac
From: Simon Huelck @ 2019-02-27 19:02 UTC (permalink / raw)
  To: Jose Abreu, Sebastian Gottschall, Jerome Brunet,
	Martin Blumenstingl
  Cc: linux-amlogic, Gpeppe.cavallaro, alexandre.torgue,
	Emiliano Ingrassia, netdev
In-Reply-To: <c21c30e9-e53e-02a5-c367-25898c4614e9@synopsys.com>

Hi,


the thing is , that im not a stmmac developer. Yes , maybe i can bissect
it and yes you are lucky since im a C-developer since a long time for
embedded systems.

The problem is that i dont understand the structure of stmmac and im not
aware of any documentation about the driver structure nor the underlying
ethernet hardware ( even though im used to ethernet hardware in embedded
environment ). So how shall i recognize the relevant change between
4.14.29 and 5.0rc8 ?


Is it in the DTS/DTB, wrong hardware description ? Is it in the code ?
how is the duplex hardware working on this piece ?

I can try to support you the best i can, but i have little chances to
analyze it myself. At which measurements / counters is it possible to
see that duplex is fully working ?  Why did even the non-duplex
bandwidth regress from 900MBits to 650 ? Why is that 650 MBits dividing
up to TX and RX in summary when doing duplex ? Why is TX not starving in
duplex but RX ?

From my point of view should be the following things given:
- the non duplex bandwidth should be somewhere around 900MBits , the HW
is capable of that
- TX should not influence RX or vice versa in duplex
- the duplex bandwidth should be 900MBits in both directions ( maybe a
bit asymetric when buffers in both dirs are not same )

I guess we need some profiling on stmmac and ( at least i need ) more
knowledge of the hardware and stmmac itself. Can someone point me to the
driver documentation, describing the functions in the code and the
structure ? How can i profile stmmac ( usually im using hardware / JTAG
debuggers at work, but here @home i got nothing like that )

So how do we continue ?

regards,
Simon



Am 27.02.2019 um 12:09 schrieb Jose Abreu:
> Hi Simon,
>
> On 2/24/2019 8:34 PM, Simon Huelck wrote:
>> the topic is about ODROID C2 / Amlogic S905X since the start. we have a
>> performance regression since 4.14.
> As we are not advancing in this topic I suggest you try bisecting
> the offending commit.
>
> Thanks,
> Jose Miguel Abreu



^ permalink raw reply

* Re: [RESEND PATCH 0/7] Add FOLL_LONGTERM to GUP fast and use it
From: Ira Weiny @ 2019-02-27 19:14 UTC (permalink / raw)
  To: John Hubbard, Andrew Morton, Michal Hocko, Kirill A. Shutemov,
	Peter Zijlstra, Jason Gunthorpe, Benjamin Herrenschmidt,
	Paul Mackerras, David S. Miller, Martin Schwidefsky,
	Heiko Carstens, Rich Felker, Yoshinori Sato, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, Ralf Baechle, Paul Burton,
	James Hogan
  Cc: linux-kernel, linux-mm, linux-mips, linuxppc-dev, linux-s390,
	linux-sh, sparclinux, kvm-ppc, kvm, linux-fpga, dri-devel,
	linux-rdma, linux-media, linux-scsi, devel, virtualization,
	netdev, linux-fbdev, xen-devel, devel, ceph-devel, rds-devel
In-Reply-To: <20190220053040.10831-1-ira.weiny@intel.com>

On Tue, Feb 19, 2019 at 09:30:33PM -0800, 'Ira Weiny' wrote:
> From: Ira Weiny <ira.weiny@intel.com>
> 
> Resending these as I had only 1 minor comment which I believe we have covered
> in this series.  I was anticipating these going through the mm tree as they
> depend on a cleanup patch there and the IB changes are very minor.  But they
> could just as well go through the IB tree.
> 
> NOTE: This series depends on my clean up patch to remove the write parameter
> from gup_fast_permitted()[1]
> 
> HFI1, qib, and mthca, use get_user_pages_fast() due to it performance
> advantages.  These pages can be held for a significant time.  But
> get_user_pages_fast() does not protect against mapping of FS DAX pages.
> 
> Introduce FOLL_LONGTERM and use this flag in get_user_pages_fast() which
> retains the performance while also adding the FS DAX checks.  XDP has also
> shown interest in using this functionality.[2]
> 
> In addition we change get_user_pages() to use the new FOLL_LONGTERM flag and
> remove the specialized get_user_pages_longterm call.
> 
> [1] https://lkml.org/lkml/2019/2/11/237
> [2] https://lkml.org/lkml/2019/2/11/1789

Is there anything I need to do on this series or does anyone have any
objections to it going into 5.1?  And if so who's tree is it going to go
through?

Thanks,
Ira

> 
> Ira Weiny (7):
>   mm/gup: Replace get_user_pages_longterm() with FOLL_LONGTERM
>   mm/gup: Change write parameter to flags in fast walk
>   mm/gup: Change GUP fast to use flags rather than a write 'bool'
>   mm/gup: Add FOLL_LONGTERM capability to GUP fast
>   IB/hfi1: Use the new FOLL_LONGTERM flag to get_user_pages_fast()
>   IB/qib: Use the new FOLL_LONGTERM flag to get_user_pages_fast()
>   IB/mthca: Use the new FOLL_LONGTERM flag to get_user_pages_fast()
> 
>  arch/mips/mm/gup.c                          |  11 +-
>  arch/powerpc/kvm/book3s_64_mmu_hv.c         |   4 +-
>  arch/powerpc/kvm/e500_mmu.c                 |   2 +-
>  arch/powerpc/mm/mmu_context_iommu.c         |   4 +-
>  arch/s390/kvm/interrupt.c                   |   2 +-
>  arch/s390/mm/gup.c                          |  12 +-
>  arch/sh/mm/gup.c                            |  11 +-
>  arch/sparc/mm/gup.c                         |   9 +-
>  arch/x86/kvm/paging_tmpl.h                  |   2 +-
>  arch/x86/kvm/svm.c                          |   2 +-
>  drivers/fpga/dfl-afu-dma-region.c           |   2 +-
>  drivers/gpu/drm/via/via_dmablit.c           |   3 +-
>  drivers/infiniband/core/umem.c              |   5 +-
>  drivers/infiniband/hw/hfi1/user_pages.c     |   5 +-
>  drivers/infiniband/hw/mthca/mthca_memfree.c |   3 +-
>  drivers/infiniband/hw/qib/qib_user_pages.c  |   8 +-
>  drivers/infiniband/hw/qib/qib_user_sdma.c   |   2 +-
>  drivers/infiniband/hw/usnic/usnic_uiom.c    |   9 +-
>  drivers/media/v4l2-core/videobuf-dma-sg.c   |   6 +-
>  drivers/misc/genwqe/card_utils.c            |   2 +-
>  drivers/misc/vmw_vmci/vmci_host.c           |   2 +-
>  drivers/misc/vmw_vmci/vmci_queue_pair.c     |   6 +-
>  drivers/platform/goldfish/goldfish_pipe.c   |   3 +-
>  drivers/rapidio/devices/rio_mport_cdev.c    |   4 +-
>  drivers/sbus/char/oradax.c                  |   2 +-
>  drivers/scsi/st.c                           |   3 +-
>  drivers/staging/gasket/gasket_page_table.c  |   4 +-
>  drivers/tee/tee_shm.c                       |   2 +-
>  drivers/vfio/vfio_iommu_spapr_tce.c         |   3 +-
>  drivers/vfio/vfio_iommu_type1.c             |   3 +-
>  drivers/vhost/vhost.c                       |   2 +-
>  drivers/video/fbdev/pvr2fb.c                |   2 +-
>  drivers/virt/fsl_hypervisor.c               |   2 +-
>  drivers/xen/gntdev.c                        |   2 +-
>  fs/orangefs/orangefs-bufmap.c               |   2 +-
>  include/linux/mm.h                          |  17 +-
>  kernel/futex.c                              |   2 +-
>  lib/iov_iter.c                              |   7 +-
>  mm/gup.c                                    | 220 ++++++++++++--------
>  mm/gup_benchmark.c                          |   5 +-
>  mm/util.c                                   |   8 +-
>  net/ceph/pagevec.c                          |   2 +-
>  net/rds/info.c                              |   2 +-
>  net/rds/rdma.c                              |   3 +-
>  44 files changed, 232 insertions(+), 180 deletions(-)
> 
> -- 
> 2.20.1
> 

^ permalink raw reply

* [PATCH bpf-next] selftests/bpf: use __bpf_constant_htons in test_prog.c for flow dissector
From: Stanislav Fomichev @ 2019-02-27 19:15 UTC (permalink / raw)
  To: netdev, bpf; +Cc: davem, ast, daniel, Stanislav Fomichev

Older GCC (<4.8) isn't smart enough to optimize !__builtin_constant_p()
branch in bpf_htons.

I recently fixed it for pkt_v4 and pkt_v6 in commit a0517a0f7ef23
("selftests/bpf: use __bpf_constant_htons in test_prog.c"), but later
added another bunch of bpf_htons in commit bf0f0fd939451 ("selftests/bpf:
add simple BPF_PROG_TEST_RUN examples for flow dissector").

Fixes: bf0f0fd939451 ("selftests/bpf: add simple BPF_PROG_TEST_RUN examples for flow dissector")

Signed-off-by: Stanislav Fomichev <sdf@google.com>
---
 tools/testing/selftests/bpf/test_progs.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index c59d2e015d16..87cde42559f7 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -1954,7 +1954,7 @@ static struct bpf_flow_keys pkt_v4_flow_keys = {
 	.thoff = sizeof(struct iphdr),
 	.addr_proto = ETH_P_IP,
 	.ip_proto = IPPROTO_TCP,
-	.n_proto = bpf_htons(ETH_P_IP),
+	.n_proto = __bpf_constant_htons(ETH_P_IP),
 };
 
 static struct bpf_flow_keys pkt_v6_flow_keys = {
@@ -1962,7 +1962,7 @@ static struct bpf_flow_keys pkt_v6_flow_keys = {
 	.thoff = sizeof(struct ipv6hdr),
 	.addr_proto = ETH_P_IPV6,
 	.ip_proto = IPPROTO_TCP,
-	.n_proto = bpf_htons(ETH_P_IPV6),
+	.n_proto = __bpf_constant_htons(ETH_P_IPV6),
 };
 
 static void test_flow_dissector(void)
-- 
2.21.0.rc2.261.ga7da99ff1b-goog


^ permalink raw reply related

* RE: AF_XDP design flaws
From: Maxim Mikityanskiy @ 2019-02-27 19:17 UTC (permalink / raw)
  To: Björn Töpel, John Fastabend
  Cc: netdev@vger.kernel.org, Björn Töpel, Magnus Karlsson,
	David S. Miller, Tariq Toukan, Saeed Mahameed, Eran Ben Elisha
In-Reply-To: <CAJ+HfNjaaHTVdM4L6a8Pfto+oj0pPigvrYYTwoEga+FadtSeNw@mail.gmail.com>

> -----Original Message-----
> From: Björn Töpel <bjorn.topel@gmail.com>
> Sent: 27 February, 2019 10:09
> To: John Fastabend <john.fastabend@gmail.com>
> Cc: Maxim Mikityanskiy <maximmi@mellanox.com>; netdev@vger.kernel.org; Björn
> Töpel <bjorn.topel@intel.com>; Magnus Karlsson <magnus.karlsson@intel.com>;
> David S. Miller <davem@davemloft.net>; Tariq Toukan <tariqt@mellanox.com>;
> Saeed Mahameed <saeedm@mellanox.com>; Eran Ben Elisha <eranbe@mellanox.com>
> Subject: Re: AF_XDP design flaws
> 
> On 2019-02-26 17:41, John Fastabend wrote:
> > On 2/26/19 6:49 AM, Maxim Mikityanskiy wrote:
> >> Hi everyone,
> >>
> 
> Hi! It's exciting to see more vendors looking into AF_XDP. ARM was
> involved early on, and now Mellanox. :-) It's really important that
> the AF_XDP model is a good fit for all drivers/vendors! Thanks for
> looking into this.
> 
> >> I would like to discuss some design flaws of AF_XDP socket (XSK)
> implementation
> >> in kernel. At the moment I don't see a way to work around them without
> changing
> >> the API, so I would like to make sure that I'm not missing anything and
> to
> >> suggest and discuss some possible improvements that can be made.
> >>
> >> The issues I describe below are caused by the fact that the driver
> depends on
> >> the application doing some things, and if the application is
> >> slow/buggy/malicious, the driver is forced to busy poll because of the
> lack of a
> >> notification mechanism from the application side. I will refer to the
> i40e
> >> driver implementation a lot, as it is the first implementation of AF_XDP,
> but
> >> the issues are general and affect any driver. I already considered trying
> to fix
> >> it on driver level, but it doesn't seem possible, so it looks like the
> behavior
> >> and implementation of AF_XDP in the kernel has to be changed.
> >>
> >> RX side busy polling
> >> ====================
> >>
> >> On the RX side, the driver expects the application to put some
> descriptors in
> >> the Fill Ring. There is no way for the application to notify the driver
> that
> >> there are more Fill Ring descriptors to take, so the driver is forced to
> busy
> >> poll the Fill Ring if it gets empty. E.g., the i40e driver does it in
> NAPI poll:
> >>
> >> int i40e_clean_rx_irq_zc(struct i40e_ring *rx_ring, int budget)
> >> {
> >> ...
> >>                          failure = failure ||
> >>
> !i40e_alloc_rx_buffers_fast_zc(rx_ring,
> >>
> cleaned_count);
> >> ...
> >>          return failure ? budget : (int)total_rx_packets;
> >> }
> >>
> >> Basically, it means that if there are no descriptors in the Fill Ring,
> NAPI will
> >> never stop, draining CPU.
> >>
> >> Possible cases when it happens
> >> ------------------------------
> >>
> >> 1. The application is slow, it received some frames in the RX Ring, and
> it is
> >> still handling the data, so it has no free frames to put to the Fill
> Ring.
> >>
> >> 2. The application is malicious, it opens an XSK and puts no frames to
> the Fill
> >> Ring. It can be used as a local DoS attack.
> >>
> >> 3. The application is buggy and stops filling the Fill Ring for whatever
> reason
> >> (deadlock, waiting for another blocking operation, other bugs).
> >>
> >> Although loading an XDP program requires root access, the DoS attack can
> be
> >> targeted to setups that already use XDP, i.e. an XDP program is already
> loaded.
> >> Even under root, userspace applications should not be able to disrupt
> system
> >> stability by just calling normal APIs without an intention to destroy the
> >> system, and here it happens in case 1.
> > I believe this is by design. If packets per second is your top priority
> > (at the expense of power, cpu, etc.) this is the behavior you might
> > want. To resolve your points if you don't trust the application it
> > should be isolated to a queue pair and cores so the impact is known and
> > managed.
> >
> 
> Correct, and I agree with John here. AF_XDP is a raw interface, and
> needs to be managed.

OK, right, if you want high pps sacrificing system stability and
kernel-userspace isolation, XSK is at your service. It may be a valid
point. However, there is still the issue of unintended use.

If you use XSK, you can take measures to prevent the disruption, e.g.
run only trusted and properly tested applications or isolate them.
However, there is a case where XSKs are not really used on a given
machine, but are still available (CONFIG_XDP_SOCKETS=y), opening a huge
hole. If you don't even know that XSK exists, you are still vulnerable.

> > That said having a flag to back-off seems like a good idea. But should
> > be doable as a feature without breaking current API.

A flag can be added, e.g., to the sxdp_flags field of struct
sockaddr_xdp, to switch to the behavior without busy polling on empty
Fill Ring.

However, I don't think the vulnerability can be eliminated while keeping
the API intact. To protect the systems that don't use XSK, we need one
of these things:

- Disable XSK support by default, until explicitly enabled by root.

- Disable the old behavior by default, until explicitly enabled by root.

Both of those things will require additional setup on machines that use
XSK, after upgrading the kernel.

> >> Possible way to solve the issue
> >> -------------------------------
> >>
> >> When the driver can't take new Fill Ring frames, it shouldn't busy poll.
> >> Instead, it signals the failure to the application (e.g., with POLLERR),
> and
> >> after that it's up to the application to restart polling (e.g., by
> calling
> >> sendto()) after refilling the Fill Ring. The issue with this approach is
> that it
> >> changes the API, so we either have to deal with it or to introduce some
> API
> >> version field.
> > See above. I like the idea here though.
> >
> 
> The uapi doesn't mandate *how* the driver should behave. The rational
> for the aggressive i40e fill ring polling (when the ring is empty) is
> to minimize the latency. The "fill ring is empty" behavior might be
> different on different drivers.

If the behavior is different with different drivers, it will be
difficult to create applications that are portable across NICs. The
point of creating APIs is to provide a single interface to different
implementations, thus hiding the differences from a higher abstraction
level. What the driver does internally is up to the driver, but the
result visible in the userspace should be the same, so we can't just
make some drivers stop and wait when the Fill Ring is empty, while the
others busy poll. It should be controlled by some flags if we want to
preserve both options.

> I see a number of different alternatives to the current i40e way of
> busy-polling, all ranging from watchdog timers to actual HW
> implementations where writing to the tail pointer of the fill ring
> kicks a door bell.
> 
> That being said, I agree with previous speakers; Being able to set a
> policy/driver behavior from userland is a good idea. This can be added
> without breaking the existing model.
> 
> Max, any input to what this interface should look like from your
> perspective?

As I said above, we can add a flag (flags?) to sxdp_flags, that will be
considered only if !XDP_SHARED_UMEM. It uses the existing field, so the
backwards compatibility can be preserved. The only thing I'm concerned
of is that if the current busy-polling behavior stays the default, some
random machines that don't even use XSK will stay vulnerable to a local
DoS attack.

> 
> >> TX side getting stuck
> >> =====================
> >>
> >> On the TX side, there is the Completion Ring that the application has to
> clean.
> >> If it doesn't, the i40e driver stops taking descriptors from the TX Ring.
> If the
> >> application finally completes something, the driver can go on
> transmitting.
> >> However, it would require busy polling the Completion Ring (just like
> with the
> >> Fill Ring on the RX side). i40e doesn't do it, instead, it relies on the
> >> application to kick the TX by calling sendto(). The issue is that poll()
> doesn't
> >> return POLLOUT in this case, because the TX Ring is full, so the
> application
> >> will never call sendto(), and the ring is stuck forever (or at least
> until
> >> something triggers NAPI).
> >>
> >> Possible way to solve the issue
> >> -------------------------------
> >>
> >> When the driver can't reserve a descriptor in the Completion Ring, it
> should
> >> signal the failure to the application (e.g., with POLLERR). The
> application
> >> shouldn't call sendto() every time it sees that the number of not
> completed
> >> frames is greater than zero (like xdpsock sample does). Instead, the
> application
> >> should kick the TX only when it wants to flush the ring, and, in
> addition, after
> >> resolving the cause for POLLERR, i.e. after handling Completion Ring
> entries.
> >> The API will also have to change with this approach.
> >>
> > +1 seems to me this can be done without breaking existing API though.
> >
> 
> Keep in mind that the xdpsock application is sample/toy, and should
> not be seen as the API documentation.

Unfortunately, Documentation/networking/af_xdp.rst misses a lot of
things that become clear by reading xdpsock, kernel and i40e sources.
So, currently, xdpsock is the best reference code available (unless you
can point me to something different).

> I don't see that the API need to be changed. AF_XDP requires that the
> fill ring has entries, in order to receive data. Analogous, the
> completion ring needs to managed by the application to send frames.

The difference is that on the RX side, the driver busy polls until it
gets enough Fill Ring entries, while on the TX side it stops when the
Completion Ring is full. The application doesn't have a convenient
mechanism to determine it. Of course, when it takes entries from the
Completion Ring, it can check every time, whether it was full, and kick
the TX in that cases, but there are two points here:

- Why not signal it from poll() to keep kicking TX in one place and
simplify the application?

- This behavior isn't documented anywhere. The documentation says that I
need to call sendmsg() to start the tranfer. However, looking at the
source code, I found that actually I need to call sendto() each time I
see there are outstanding packets, because i40e can ignore my sendto()
if the Completion Ring is full. And the thing with the TX getting stuck
is not covered even by the sample code.

> Are you suggesting that using poll() et al should be mandatory? Again,
> take me through your send scenario, step by step.

Not necessarily; as both the kernel and the application share the same
rings, it doesn't matter who of them will look at the producer/consumer
indices and decide whether it can proceed TXing. The conditions used in
xsk_poll can be reproduced in the application itself if it wants to
eliminate poll().

My send algorithm:

1. Wait until poll() returns POLLOUT.

2. Put the packets into the UMEM, and the descriptors into the TX Ring.

3. Call sendto() (the difference is that sendto() is called only after
actual sending, not every time the application sees outstanding packets,
that can already be queued in the hardware).

4. If the driver can't reserve a descriptor in the Completion Ring, the
next poll() will return POLLERR. In this case it's up to the application
to call sendto() again after freeing some space in the Completion Ring.

Pros:

1. Still doesn't force the application to use poll() - it just has to
monitor rings itself.

2. Eliminates useless sendto()s. Although xdpsock is only a simple
example, with the current XSK implementation in the kernel, I currently
see no way to avoid calling sendto() every time the application sees
some not completed packets, because the application doesn't know whether
the kernel took that frame or not. With my approach, sendto() can be
called only when needed.

3. sendto() and poll() can be placed in a single loop. When the
Completion Ring overflows, poll() returns (with POLLERR), which doesn't
happen currently.

4. Ability to batch multiple packets in a single sendto(). If we try to
batch send packets now, we don't know how many of them were consumed by
kernel, so we'll be forced to repeat sendto() until all of them are
transmitted.

> To me, the userland application has all the knobs to determine whether
> the Tx ring can be flushed or not.

Regarding this, consider the points above.

> 
> >> Triggering NAPI on a different CPU core
> >> =======================================
> >>
> >> .ndo_xsk_async_xmit runs on a random CPU core, so, to preserve CPU
> affinity,
> >> i40e triggers an interrupt to schedule NAPI, instead of calling
> napi_schedule
> >> directly. Scheduling NAPI on the correct CPU is what would every driver
> do, I
> >> guess, but currently it has to be implemented differently in every
> driver, and
> >> it relies on hardware features (the ability to trigger an IRQ).
> > Ideally the application would be pinned to a core and the traffic
> > steered to that core using ethtool/tc. Yes it requires a bit of mgmt on
> > the platform but I think this is needed for best pps numbers.

Yes, you are right. However, the situation where .ndo_xsk_async_xmit is
called on a wrong CPU is possible, and it will happen unless some
additional setup is done, so the driver has to handle it.

> >> I suggest introducing a kernel API that would allow triggering NAPI on a
> given
> >> CPU. A brief look shows that something like
> smp_call_function_single_async can
> >> be used. Advantages:
> > Assuming you want to avoid pinning cores/traffic for some reason. Could
> > this be done with some combination of cpumap + af_xdp? I haven't thought
> > too much about it though. Maybe Bjorn has some ideas.

Not that I want to avoid pinning CPUs... Intel has a hardware feature
that allows them to trigger an IRQ on the given CPU to call NAPI on the
right CPU. (I'm talking only about the TX side here.) The data to be
transmitted can be written to the UMEM from the right CPU, but sendto()
can be called from any, and it will work for i40e. Mellanox hardware
doesn't have this feature, although I can do the same using some hacks.
So I'm seeking for a portable solution.

> 
> For the Intel drivers, the NAPI is associated to a certain
> interrupt.

The same for Mellanox.

> This does not change with AF_XDP, so ndo_xsk_async_xmit
> simply kicks the NAPI to run on the CPU bound defined by the irq
> affinity.

This is more complicated for Mellanox hardware. If there was a "trigger
NAPI on CPU X" function (that could use smp_call_function_single_async
internally), it would make this part completely driver- and
hardware-agnostic, it could even be implemented as a common code in the
kernel.

> I'm all open for additionally kernel APIs, and I'm happy to hack the
> i40e internals as well. Again, it must be really simple to add
> XDP/AF_XDP-ZC support for other vendor.
> 
> Ideally, what I would like is a generic way of associating netdev
> queues (think ethtools-channels) with NAPI contexts. E.g. "queues
> 1,2,3 on NAPI foo, and I,II,III on NAPI bar, and the NAPIs should run
> on on the baz cgroup". This is a much bigger task and outside the
> AF_XDP scope. It ties in to the whole "threaded NAPIs and netdev
> queues as a first class kernel object" discussion. ;-)

Yes, it's much more intrusive and big, let's deal with AF_XDP first :)

> Magnus is working on proper busy-poll() (i.e. that the poll() syscall
> executes the NAPI context). With that solution the user application
> can select which core it wants to execute the poll() on -- and the
> NAPI will be executed from the poll().

Wow, that's good. But the current i40e_napi_poll() checks the CPU
affinity and reschedules if it's wrong - will this check be removed? Or
will there be a separate NAPI instance for XSK TX?

> 
> >> 1. It lifts the hardware requirement to be able to raise an interrupt on
> demand.
> >>
> >> 2. It would allow to move common code to the kernel
> (.ndo_xsk_async_xmit).
> >>
> >> 3. It is also useful in the situation where CPU affinity changes while
> being in
> >> NAPI poll. Currently, i40e and mlx5e try to stop NAPI polling by
> returning
> >> a value less than budget if CPU affinity changes. However, there are
> cases
> >> (e.g., NAPIF_STATE_MISSED) when NAPI will be rescheduled on a wrong CPU.
> It's a
> >> race between the interrupt, which will move NAPI to the correct CPU, and
> >> __napi_schedule from a wrong CPU. Having an API to schedule NAPI on a
> given CPU
> >> will benefit both mlx5e and i40e, because when this situation happens, it
> kills
> >> the performance.
> > How would we know what core to trigger NAPI on?

We can check the cpumask for the necessary channel.

> >> I would be happy to hear your thoughts about these issues.
> > At least the first two observations seem incrementally solvable to me
> > and would be nice improvements. I assume the last case can be resolved
> > by pinning cores/traffic but for the general case perhaps it is useful.
> >
> 
> Good summary, John. Really good input Max, and addressable -- but API
> changes as far as I see it is not required.

OK, I'll try to sum up the points:

1. RX busy polling is good for latency, and issues caused by userspace
misbehavior can be sacrificed for the performance, but if AF_XDP sockets
are available in the system, even if unused, they open a security hole
(local DoS).

2. TX stall can be solved in the userspace with the current API,
however, this issue is not documented, and my approach can give some
benefits. We can add a flag to turn it on explicitly. No driver changes,
kernel-only feature.

3. NAPI issue can be worked around by calling sendto() from the right
CPU (and it will most likely happen in production), but the current i40e
implementation supports sendto() from any CPU, and it should be made
driver-agnostic and put in the kernel.

Thank you John and Björn for your input!

> (I'm out-of-office this week, skiing. So, I'll be away from the lists
> until Monday!)
> 
> Cheers,
> Björn
> 
> >> Thanks,
> >> Max
> >>

^ permalink raw reply

* [PATCH net-next RESEND] devlink: fix kdoc
From: Jakub Kicinski @ 2019-02-27 19:36 UTC (permalink / raw)
  To: davem; +Cc: netdev, oss-drivers, jiri, Jakub Kicinski

devlink suffers from a few kdoc warnings:

net/core/devlink.c:5292: warning: Function parameter or member 'dev' not described in 'devlink_register'
net/core/devlink.c:5351: warning: Function parameter or member 'port_index' not described in 'devlink_port_register'
net/core/devlink.c:5753: warning: Function parameter or member 'parent_resource_id' not described in 'devlink_resource_register'
net/core/devlink.c:5753: warning: Function parameter or member 'size_params' not described in 'devlink_resource_register'
net/core/devlink.c:5753: warning: Excess function parameter 'top_hierarchy' description in 'devlink_resource_register'
net/core/devlink.c:5753: warning: Excess function parameter 'reload_required' description in 'devlink_resource_register'
net/core/devlink.c:5753: warning: Excess function parameter 'parent_reosurce_id' description in 'devlink_resource_register'
net/core/devlink.c:6451: warning: Function parameter or member 'region' not described in 'devlink_region_snapshot_create'
net/core/devlink.c:6451: warning: Excess function parameter 'devlink_region' description in 'devlink_region_snapshot_create'

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
Looks like the discussion about the port flavours may take a while,
posting this clean up separately :)
---

 net/core/devlink.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/net/core/devlink.c b/net/core/devlink.c
index a49dee67e66f..6515fbec0dcd 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -5249,6 +5249,7 @@ EXPORT_SYMBOL_GPL(devlink_alloc);
  *	devlink_register - Register devlink instance
  *
  *	@devlink: devlink
+ *	@dev: parent device
  */
 int devlink_register(struct devlink *devlink, struct device *dev)
 {
@@ -5299,7 +5300,7 @@ EXPORT_SYMBOL_GPL(devlink_free);
  *
  *	@devlink: devlink
  *	@devlink_port: devlink port
- *	@port_index
+ *	@port_index: driver-specific numerical identifier of the port
  *
  *	Register devlink port with provided port index. User can use
  *	any indexing, even hw-related one. devlink_port structure
@@ -5629,13 +5630,10 @@ EXPORT_SYMBOL_GPL(devlink_dpipe_table_unregister);
  *
  *	@devlink: devlink
  *	@resource_name: resource's name
- *	@top_hierarchy: top hierarchy
- *	@reload_required: reload is required for new configuration to
- *			  apply
  *	@resource_size: resource's size
  *	@resource_id: resource's id
- *	@parent_reosurce_id: resource's parent id
- *	@size params: size parameters
+ *	@parent_resource_id: resource's parent id
+ *	@size_params: size parameters
  */
 int devlink_resource_register(struct devlink *devlink,
 			      const char *resource_name,
@@ -6332,7 +6330,7 @@ EXPORT_SYMBOL_GPL(devlink_region_shapshot_id_get);
  *	Multiple snapshots can be created on a region.
  *	The @snapshot_id should be obtained using the getter function.
  *
- *	@devlink_region: devlink region of the snapshot
+ *	@region: devlink region of the snapshot
  *	@data_len: size of snapshot data
  *	@data: snapshot data
  *	@snapshot_id: snapshot id to be created
-- 
2.19.2


^ permalink raw reply related

* [PATCH net-next v3 6/8] staging: fsl-dpaa2: ethsw: Handle SWITCHDEV_PORT_ATTR_SET
From: Florian Fainelli @ 2019-02-27 19:44 UTC (permalink / raw)
  To: netdev
  Cc: Florian Fainelli, David S. Miller, Ido Schimmel, open list,
	open list:STAGING SUBSYSTEM, moderated list:ETHERNET BRIDGE, jiri,
	andrew, vivien.didelot
In-Reply-To: <20190227194432.725-1-f.fainelli@gmail.com>

Following patches will change the way we communicate setting a port's
attribute and use a blocking notifier to perform those tasks.

Prepare ethsw to support receiving notifier events targeting
SWITCHDEV_PORT_ATTR_SET and simply translate that into the existing
swdev_port_attr_set() call.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/staging/fsl-dpaa2/ethsw/ethsw.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
index 018399ee8731..b0d2d9bf2532 100644
--- a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
+++ b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
@@ -929,6 +929,19 @@ static const struct switchdev_ops ethsw_port_switchdev_ops = {
 	.switchdev_port_attr_set	= swdev_port_attr_set,
 };
 
+static int
+ethsw_switchdev_port_attr_set_event(struct net_device *netdev,
+		struct switchdev_notifier_port_attr_info *port_attr_info)
+{
+	int err;
+
+	err = swdev_port_attr_set(netdev, port_attr_info->attr,
+				  port_attr_info->trans);
+
+	port_attr_info->handled = true;
+	return notifier_from_errno(err);
+}
+
 /* For the moment, only flood setting needs to be updated */
 static int port_bridge_join(struct net_device *netdev,
 			    struct net_device *upper_dev)
@@ -1047,6 +1060,12 @@ static int port_switchdev_event(struct notifier_block *unused,
 	struct ethsw_switchdev_event_work *switchdev_work;
 	struct switchdev_notifier_fdb_info *fdb_info = ptr;
 
+	if (!ethsw_port_dev_check(dev))
+		return NOTIFY_DONE;
+
+	if (event == SWITCHDEV_PORT_ATTR_SET)
+		return ethsw_switchdev_port_attr_set_event(dev, ptr);
+
 	switchdev_work = kzalloc(sizeof(*switchdev_work), GFP_ATOMIC);
 	if (!switchdev_work)
 		return NOTIFY_BAD;
@@ -1115,6 +1134,8 @@ static int port_switchdev_blocking_event(struct notifier_block *unused,
 	case SWITCHDEV_PORT_OBJ_ADD: /* fall through */
 	case SWITCHDEV_PORT_OBJ_DEL:
 		return ethsw_switchdev_port_obj_event(event, dev, ptr);
+	case SWITCHDEV_PORT_ATTR_SET:
+		return ethsw_switchdev_port_attr_set_event(dev, ptr);
 	}
 
 	return NOTIFY_DONE;
-- 
2.17.1


^ permalink raw reply related

* [PATCH net-next v3 7/8] net: switchdev: Replace port attr set SDO with a notification
From: Florian Fainelli @ 2019-02-27 19:44 UTC (permalink / raw)
  To: netdev
  Cc: Florian Fainelli, David S. Miller, Ido Schimmel, open list,
	open list:STAGING SUBSYSTEM, moderated list:ETHERNET BRIDGE, jiri,
	andrew, vivien.didelot
In-Reply-To: <20190227194432.725-1-f.fainelli@gmail.com>

Drop switchdev_ops.switchdev_port_attr_set. Drop the uses of this field
from all clients, which were migrated to use switchdev notification in
the previous patches.

Add a new function switchdev_port_attr_notify() that sends the switchdev
notifications SWITCHDEV_PORT_ATTR_SET and calls the blocking (process)
notifier chain.

We have one odd case within net/bridge/br_switchdev.c with the
SWITCHDEV_ATTR_ID_PORT_PRE_BRIDGE_FLAGS attribute identifier that
requires executing from atomic context, we deal with that one
specifically.

Drop __switchdev_port_attr_set() and update switchdev_port_attr_set()
likewise.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 net/bridge/br_switchdev.c |  8 +++++-
 net/switchdev/switchdev.c | 53 ++++++++++++++++++---------------------
 2 files changed, 31 insertions(+), 30 deletions(-)

diff --git a/net/bridge/br_switchdev.c b/net/bridge/br_switchdev.c
index af57c4a2b78a..921310d3cbae 100644
--- a/net/bridge/br_switchdev.c
+++ b/net/bridge/br_switchdev.c
@@ -67,12 +67,18 @@ int br_switchdev_set_port_flag(struct net_bridge_port *p,
 		.id = SWITCHDEV_ATTR_ID_PORT_PRE_BRIDGE_FLAGS,
 		.u.brport_flags = mask,
 	};
+	struct switchdev_notifier_port_attr_info info = {
+		.attr = &attr,
+	};
 	int err;
 
 	if (mask & ~BR_PORT_FLAGS_HW_OFFLOAD)
 		return 0;
 
-	err = switchdev_port_attr_set(p->dev, &attr);
+	/* We run from atomic context here */
+	err = call_switchdev_notifiers(SWITCHDEV_PORT_ATTR_SET, p->dev,
+				       &info.info, NULL);
+	err = notifier_to_errno(err);
 	if (err == -EOPNOTSUPP)
 		return 0;
 
diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
index 3560c19aa7e2..d81cfcee9ad9 100644
--- a/net/switchdev/switchdev.c
+++ b/net/switchdev/switchdev.c
@@ -174,39 +174,32 @@ static int switchdev_deferred_enqueue(struct net_device *dev,
 	return 0;
 }
 
-static int __switchdev_port_attr_set(struct net_device *dev,
-				     const struct switchdev_attr *attr,
-				     struct switchdev_trans *trans)
+static int switchdev_port_attr_notify(enum switchdev_notifier_type nt,
+				      struct net_device *dev,
+				      const struct switchdev_attr *attr,
+				      struct switchdev_trans *trans)
 {
-	const struct switchdev_ops *ops = dev->switchdev_ops;
-	struct net_device *lower_dev;
-	struct list_head *iter;
-	int err = -EOPNOTSUPP;
-
-	if (ops && ops->switchdev_port_attr_set) {
-		err = ops->switchdev_port_attr_set(dev, attr, trans);
-		goto done;
-	}
-
-	if (attr->flags & SWITCHDEV_F_NO_RECURSE)
-		goto done;
+	int err;
+	int rc;
 
-	/* Switch device port(s) may be stacked under
-	 * bond/team/vlan dev, so recurse down to set attr on
-	 * each port.
-	 */
+	struct switchdev_notifier_port_attr_info attr_info = {
+		.attr = attr,
+		.trans = trans,
+		.handled = false,
+	};
 
-	netdev_for_each_lower_dev(dev, lower_dev, iter) {
-		err = __switchdev_port_attr_set(lower_dev, attr, trans);
-		if (err)
-			break;
+	rc = call_switchdev_blocking_notifiers(nt, dev,
+					       &attr_info.info, NULL);
+	err = notifier_to_errno(rc);
+	if (err) {
+		WARN_ON(!attr_info.handled);
+		return err;
 	}
 
-done:
-	if (err == -EOPNOTSUPP && attr->flags & SWITCHDEV_F_SKIP_EOPNOTSUPP)
-		err = 0;
+	if (!attr_info.handled)
+		return -EOPNOTSUPP;
 
-	return err;
+	return 0;
 }
 
 static int switchdev_port_attr_set_now(struct net_device *dev,
@@ -225,7 +218,8 @@ static int switchdev_port_attr_set_now(struct net_device *dev,
 	 */
 
 	trans.ph_prepare = true;
-	err = __switchdev_port_attr_set(dev, attr, &trans);
+	err = switchdev_port_attr_notify(SWITCHDEV_PORT_ATTR_SET, dev, attr,
+					 &trans);
 	if (err) {
 		/* Prepare phase failed: abort the transaction.  Any
 		 * resources reserved in the prepare phase are
@@ -244,7 +238,8 @@ static int switchdev_port_attr_set_now(struct net_device *dev,
 	 */
 
 	trans.ph_prepare = false;
-	err = __switchdev_port_attr_set(dev, attr, &trans);
+	err = switchdev_port_attr_notify(SWITCHDEV_PORT_ATTR_SET, dev, attr,
+					 &trans);
 	WARN(err, "%s: Commit of attribute (id=%d) failed.\n",
 	     dev->name, attr->id);
 	switchdev_trans_items_warn_destroy(dev, &trans);
-- 
2.17.1


^ permalink raw reply related

* [PATCH net-next v3 8/8] net: Remove switchdev_ops
From: Florian Fainelli @ 2019-02-27 19:44 UTC (permalink / raw)
  To: netdev
  Cc: Florian Fainelli, David S. Miller, Ido Schimmel, open list,
	open list:STAGING SUBSYSTEM, moderated list:ETHERNET BRIDGE, jiri,
	andrew, vivien.didelot
In-Reply-To: <20190227194432.725-1-f.fainelli@gmail.com>

Now that we have converted all possible callers to using a switchdev
notifier for attributes we do not have a need for implementing
switchdev_ops anymore, and this can be removed from all drivers the
net_device structure.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/ethernet/mellanox/mlxsw/spectrum.c  |  3 ---
 drivers/net/ethernet/mellanox/mlxsw/spectrum.h  |  2 --
 .../mellanox/mlxsw/spectrum_switchdev.c         | 12 ------------
 drivers/net/ethernet/mscc/ocelot.c              |  5 -----
 drivers/net/ethernet/rocker/rocker_main.c       |  5 -----
 drivers/staging/fsl-dpaa2/ethsw/ethsw.c         |  5 -----
 include/linux/netdevice.h                       |  3 ---
 include/net/switchdev.h                         | 17 -----------------
 net/dsa/slave.c                                 |  5 -----
 9 files changed, 57 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index b00f6f74f91a..6c797e322be8 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -3660,7 +3660,6 @@ static int mlxsw_sp_port_create(struct mlxsw_sp *mlxsw_sp, u8 local_port,
 	}
 	mlxsw_sp_port->default_vlan = mlxsw_sp_port_vlan;
 
-	mlxsw_sp_port_switchdev_init(mlxsw_sp_port);
 	mlxsw_sp->ports[local_port] = mlxsw_sp_port;
 	err = register_netdev(dev);
 	if (err) {
@@ -3677,7 +3676,6 @@ static int mlxsw_sp_port_create(struct mlxsw_sp *mlxsw_sp, u8 local_port,
 
 err_register_netdev:
 	mlxsw_sp->ports[local_port] = NULL;
-	mlxsw_sp_port_switchdev_fini(mlxsw_sp_port);
 	mlxsw_sp_port_vlan_destroy(mlxsw_sp_port_vlan);
 err_port_vlan_create:
 err_port_pvid_set:
@@ -3720,7 +3718,6 @@ static void mlxsw_sp_port_remove(struct mlxsw_sp *mlxsw_sp, u8 local_port)
 	mlxsw_core_port_clear(mlxsw_sp->core, local_port, mlxsw_sp);
 	unregister_netdev(mlxsw_sp_port->dev); /* This calls ndo_stop */
 	mlxsw_sp->ports[local_port] = NULL;
-	mlxsw_sp_port_switchdev_fini(mlxsw_sp_port);
 	mlxsw_sp_port_vlan_flush(mlxsw_sp_port, true);
 	mlxsw_sp_port_nve_fini(mlxsw_sp_port);
 	mlxsw_sp_tc_qdisc_fini(mlxsw_sp_port);
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
index a61c1130d9e3..da6278b0caa4 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
@@ -407,8 +407,6 @@ extern const struct mlxsw_sp_sb_vals mlxsw_sp2_sb_vals;
 /* spectrum_switchdev.c */
 int mlxsw_sp_switchdev_init(struct mlxsw_sp *mlxsw_sp);
 void mlxsw_sp_switchdev_fini(struct mlxsw_sp *mlxsw_sp);
-void mlxsw_sp_port_switchdev_init(struct mlxsw_sp_port *mlxsw_sp_port);
-void mlxsw_sp_port_switchdev_fini(struct mlxsw_sp_port *mlxsw_sp_port);
 int mlxsw_sp_rif_fdb_op(struct mlxsw_sp *mlxsw_sp, const char *mac, u16 fid,
 			bool adding);
 void
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
index c1aedfea3a31..f6ce386c3036 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
@@ -1938,10 +1938,6 @@ static struct mlxsw_sp_port *mlxsw_sp_lag_rep_port(struct mlxsw_sp *mlxsw_sp,
 	return NULL;
 }
 
-static const struct switchdev_ops mlxsw_sp_port_switchdev_ops = {
-	.switchdev_port_attr_set	= mlxsw_sp_port_attr_set,
-};
-
 static int
 mlxsw_sp_bridge_8021q_port_join(struct mlxsw_sp_bridge_device *bridge_device,
 				struct mlxsw_sp_bridge_port *bridge_port,
@@ -3545,11 +3541,3 @@ void mlxsw_sp_switchdev_fini(struct mlxsw_sp *mlxsw_sp)
 	kfree(mlxsw_sp->bridge);
 }
 
-void mlxsw_sp_port_switchdev_init(struct mlxsw_sp_port *mlxsw_sp_port)
-{
-	mlxsw_sp_port->dev->switchdev_ops = &mlxsw_sp_port_switchdev_ops;
-}
-
-void mlxsw_sp_port_switchdev_fini(struct mlxsw_sp_port *mlxsw_sp_port)
-{
-}
diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c
index 83a678b11757..a1d0d6e42533 100644
--- a/drivers/net/ethernet/mscc/ocelot.c
+++ b/drivers/net/ethernet/mscc/ocelot.c
@@ -1324,10 +1324,6 @@ static int ocelot_port_obj_del(struct net_device *dev,
 	return ret;
 }
 
-static const struct switchdev_ops ocelot_port_switchdev_ops = {
-	.switchdev_port_attr_set	= ocelot_port_attr_set,
-};
-
 static int ocelot_port_bridge_join(struct ocelot_port *ocelot_port,
 				   struct net_device *bridge)
 {
@@ -1660,7 +1656,6 @@ int ocelot_probe_port(struct ocelot *ocelot, u8 port,
 
 	dev->netdev_ops = &ocelot_port_netdev_ops;
 	dev->ethtool_ops = &ocelot_ethtool_ops;
-	dev->switchdev_ops = &ocelot_port_switchdev_ops;
 
 	dev->hw_features |= NETIF_F_HW_VLAN_CTAG_FILTER | NETIF_F_RXFCS;
 	dev->features |= NETIF_F_HW_VLAN_CTAG_FILTER;
diff --git a/drivers/net/ethernet/rocker/rocker_main.c b/drivers/net/ethernet/rocker/rocker_main.c
index fc772cf079cc..c883aa89b7ca 100644
--- a/drivers/net/ethernet/rocker/rocker_main.c
+++ b/drivers/net/ethernet/rocker/rocker_main.c
@@ -2142,10 +2142,6 @@ static int rocker_port_obj_del(struct net_device *dev,
 	return err;
 }
 
-static const struct switchdev_ops rocker_port_switchdev_ops = {
-	.switchdev_port_attr_set	= rocker_port_attr_set,
-};
-
 struct rocker_fib_event_work {
 	struct work_struct work;
 	union {
@@ -2599,7 +2595,6 @@ static int rocker_probe_port(struct rocker *rocker, unsigned int port_number)
 	rocker_port_dev_addr_init(rocker_port);
 	dev->netdev_ops = &rocker_port_netdev_ops;
 	dev->ethtool_ops = &rocker_port_ethtool_ops;
-	dev->switchdev_ops = &rocker_port_switchdev_ops;
 	netif_tx_napi_add(dev, &rocker_port->napi_tx, rocker_port_poll_tx,
 			  NAPI_POLL_WEIGHT);
 	netif_napi_add(dev, &rocker_port->napi_rx, rocker_port_poll_rx,
diff --git a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
index b0d2d9bf2532..ad577beeb052 100644
--- a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
+++ b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
@@ -925,10 +925,6 @@ static int swdev_port_obj_del(struct net_device *netdev,
 	return err;
 }
 
-static const struct switchdev_ops ethsw_port_switchdev_ops = {
-	.switchdev_port_attr_set	= swdev_port_attr_set,
-};
-
 static int
 ethsw_switchdev_port_attr_set_event(struct net_device *netdev,
 		struct switchdev_notifier_port_attr_info *port_attr_info)
@@ -1455,7 +1451,6 @@ static int ethsw_probe_port(struct ethsw_core *ethsw, u16 port_idx)
 	SET_NETDEV_DEV(port_netdev, dev);
 	port_netdev->netdev_ops = &ethsw_port_ops;
 	port_netdev->ethtool_ops = &ethsw_port_ethtool_ops;
-	port_netdev->switchdev_ops = &ethsw_port_switchdev_ops;
 
 	/* Set MTU limits */
 	port_netdev->min_mtu = ETH_MIN_MTU;
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 58e83bd7a861..c10b60297d28 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1843,9 +1843,6 @@ struct net_device {
 #endif
 	const struct net_device_ops *netdev_ops;
 	const struct ethtool_ops *ethtool_ops;
-#ifdef CONFIG_NET_SWITCHDEV
-	const struct switchdev_ops *switchdev_ops;
-#endif
 #ifdef CONFIG_NET_L3_MASTER_DEV
 	const struct l3mdev_ops	*l3mdev_ops;
 #endif
diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index 5087c06ceb4b..e4f751e19ecf 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -112,17 +112,6 @@ void *switchdev_trans_item_dequeue(struct switchdev_trans *trans);
 
 typedef int switchdev_obj_dump_cb_t(struct switchdev_obj *obj);
 
-/**
- * struct switchdev_ops - switchdev operations
- *
- * @switchdev_port_attr_set: Set a port attribute (see switchdev_attr).
- */
-struct switchdev_ops {
-	int	(*switchdev_port_attr_set)(struct net_device *dev,
-					   const struct switchdev_attr *attr,
-					   struct switchdev_trans *trans);
-};
-
 enum switchdev_notifier_type {
 	SWITCHDEV_FDB_ADD_TO_BRIDGE = 1,
 	SWITCHDEV_FDB_DEL_TO_BRIDGE,
@@ -226,9 +215,6 @@ int switchdev_handle_port_attr_set(struct net_device *dev,
 			int (*set_cb)(struct net_device *dev,
 				      const struct switchdev_attr *attr,
 				      struct switchdev_trans *trans));
-
-#define SWITCHDEV_SET_OPS(netdev, ops) ((netdev)->switchdev_ops = (ops))
-
 #else
 
 static inline void switchdev_deferred_process(void)
@@ -325,9 +311,6 @@ switchdev_handle_port_attr_set(struct net_device *dev,
 {
 	return 0;
 }
-
-#define SWITCHDEV_SET_OPS(netdev, ops) do {} while (0)
-
 #endif
 
 #endif /* _LINUX_SWITCHDEV_H_ */
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index b089b43120e1..1808a2cd6872 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -1118,10 +1118,6 @@ static const struct net_device_ops dsa_slave_netdev_ops = {
 	.ndo_vlan_rx_kill_vid	= dsa_slave_vlan_rx_kill_vid,
 };
 
-static const struct switchdev_ops dsa_slave_switchdev_ops = {
-	.switchdev_port_attr_set	= dsa_slave_port_attr_set,
-};
-
 static struct device_type dsa_type = {
 	.name	= "dsa",
 };
@@ -1382,7 +1378,6 @@ int dsa_slave_create(struct dsa_port *port)
 	eth_hw_addr_inherit(slave_dev, master);
 	slave_dev->priv_flags |= IFF_NO_QUEUE;
 	slave_dev->netdev_ops = &dsa_slave_netdev_ops;
-	slave_dev->switchdev_ops = &dsa_slave_switchdev_ops;
 	slave_dev->min_mtu = 0;
 	slave_dev->max_mtu = ETH_MAX_MTU;
 	SET_NETDEV_DEVTYPE(slave_dev, &dsa_type);
-- 
2.17.1


^ permalink raw reply related

* [PATCH net-next v3 4/8] mlxsw: spectrum_switchdev: Handle SWITCHDEV_PORT_ATTR_SET
From: Florian Fainelli @ 2019-02-27 19:44 UTC (permalink / raw)
  To: netdev
  Cc: Florian Fainelli, David S. Miller, Ido Schimmel, open list,
	open list:STAGING SUBSYSTEM, moderated list:ETHERNET BRIDGE, jiri,
	andrew, vivien.didelot
In-Reply-To: <20190227194432.725-1-f.fainelli@gmail.com>

Following patches will change the way we communicate setting a port's
attribute and use a notifier to perform those tasks.

Prepare mlxsw to support receiving notifier events targeting
SWITCHDEV_PORT_ATTR_SET and utilize the switchdev_handle_port_attr_set()
to handle stacking of devices.

Reviewed-by: Ido Schimmel <idosch@mellanox.com>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 .../net/ethernet/mellanox/mlxsw/spectrum_switchdev.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
index 766f5b5f1cf5..c1aedfea3a31 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_switchdev.c
@@ -3123,6 +3123,13 @@ static int mlxsw_sp_switchdev_event(struct notifier_block *unused,
 	struct net_device *br_dev;
 	int err;
 
+	if (event == SWITCHDEV_PORT_ATTR_SET) {
+		err = switchdev_handle_port_attr_set(dev, ptr,
+						     mlxsw_sp_port_dev_check,
+						     mlxsw_sp_port_attr_set);
+		return notifier_from_errno(err);
+	}
+
 	/* Tunnel devices are not our uppers, so check their master instead */
 	br_dev = netdev_master_upper_dev_get_rcu(dev);
 	if (!br_dev)
@@ -3446,6 +3453,11 @@ static int mlxsw_sp_switchdev_blocking_event(struct notifier_block *unused,
 							mlxsw_sp_port_dev_check,
 							mlxsw_sp_port_obj_del);
 		return notifier_from_errno(err);
+	case SWITCHDEV_PORT_ATTR_SET:
+		err = switchdev_handle_port_attr_set(dev, ptr,
+						     mlxsw_sp_port_dev_check,
+						     mlxsw_sp_port_attr_set);
+		return notifier_from_errno(err);
 	}
 
 	return NOTIFY_DONE;
-- 
2.17.1


^ permalink raw reply related

* [PATCH net-next v3 5/8] net: mscc: ocelot: Handle SWITCHDEV_PORT_ATTR_SET
From: Florian Fainelli @ 2019-02-27 19:44 UTC (permalink / raw)
  To: netdev
  Cc: Florian Fainelli, David S. Miller, Ido Schimmel, open list,
	open list:STAGING SUBSYSTEM, moderated list:ETHERNET BRIDGE, jiri,
	andrew, vivien.didelot
In-Reply-To: <20190227194432.725-1-f.fainelli@gmail.com>

Following patches will change the way we communicate setting a port's
attribute and use notifiers to perform those tasks.

Ocelot does not currently have an atomic notifier registered for
switchdev events, so we need to register one in order to deal with
atomic context SWITCHDEV_PORT_ATTR_SET events.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/ethernet/mscc/ocelot.c       | 27 ++++++++++++++++++++++++
 drivers/net/ethernet/mscc/ocelot.h       |  1 +
 drivers/net/ethernet/mscc/ocelot_board.c |  2 ++
 3 files changed, 30 insertions(+)

diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c
index 195306d05bcd..83a678b11757 100644
--- a/drivers/net/ethernet/mscc/ocelot.c
+++ b/drivers/net/ethernet/mscc/ocelot.c
@@ -1582,6 +1582,28 @@ struct notifier_block ocelot_netdevice_nb __read_mostly = {
 };
 EXPORT_SYMBOL(ocelot_netdevice_nb);
 
+static int ocelot_switchdev_event(struct notifier_block *unused,
+				  unsigned long event, void *ptr)
+{
+	struct net_device *dev = switchdev_notifier_info_to_dev(ptr);
+	int err;
+
+	switch (event) {
+	case SWITCHDEV_PORT_ATTR_SET:
+		err = switchdev_handle_port_attr_set(dev, ptr,
+						     ocelot_netdevice_dev_check,
+						     ocelot_port_attr_set);
+		return notifier_from_errno(err);
+	}
+
+	return NOTIFY_DONE;
+}
+
+struct notifier_block ocelot_switchdev_nb __read_mostly = {
+	.notifier_call = ocelot_switchdev_event,
+};
+EXPORT_SYMBOL(ocelot_switchdev_nb);
+
 static int ocelot_switchdev_blocking_event(struct notifier_block *unused,
 					   unsigned long event, void *ptr)
 {
@@ -1600,6 +1622,11 @@ static int ocelot_switchdev_blocking_event(struct notifier_block *unused,
 						    ocelot_netdevice_dev_check,
 						    ocelot_port_obj_del);
 		return notifier_from_errno(err);
+	case SWITCHDEV_PORT_ATTR_SET:
+		err = switchdev_handle_port_attr_set(dev, ptr,
+						     ocelot_netdevice_dev_check,
+						     ocelot_port_attr_set);
+		return notifier_from_errno(err);
 	}
 
 	return NOTIFY_DONE;
diff --git a/drivers/net/ethernet/mscc/ocelot.h b/drivers/net/ethernet/mscc/ocelot.h
index 086775f7b52f..ba3b3380b4d0 100644
--- a/drivers/net/ethernet/mscc/ocelot.h
+++ b/drivers/net/ethernet/mscc/ocelot.h
@@ -499,6 +499,7 @@ int ocelot_probe_port(struct ocelot *ocelot, u8 port,
 		      struct phy_device *phy);
 
 extern struct notifier_block ocelot_netdevice_nb;
+extern struct notifier_block ocelot_switchdev_nb;
 extern struct notifier_block ocelot_switchdev_blocking_nb;
 
 #endif
diff --git a/drivers/net/ethernet/mscc/ocelot_board.c b/drivers/net/ethernet/mscc/ocelot_board.c
index ca3ea2fbfcd0..2c1121d86edf 100644
--- a/drivers/net/ethernet/mscc/ocelot_board.c
+++ b/drivers/net/ethernet/mscc/ocelot_board.c
@@ -329,6 +329,7 @@ static int mscc_ocelot_probe(struct platform_device *pdev)
 	}
 
 	register_netdevice_notifier(&ocelot_netdevice_nb);
+	register_switchdev_notifier(&ocelot_switchdev_nb);
 	register_switchdev_blocking_notifier(&ocelot_switchdev_blocking_nb);
 
 	dev_info(&pdev->dev, "Ocelot switch probed\n");
@@ -345,6 +346,7 @@ static int mscc_ocelot_remove(struct platform_device *pdev)
 
 	ocelot_deinit(ocelot);
 	unregister_switchdev_blocking_notifier(&ocelot_switchdev_blocking_nb);
+	unregister_switchdev_notifier(&ocelot_switchdev_nb);
 	unregister_netdevice_notifier(&ocelot_netdevice_nb);
 
 	return 0;
-- 
2.17.1


^ permalink raw reply related

* [PATCH net-next v3 3/8] net: dsa: Handle SWITCHDEV_PORT_ATTR_SET
From: Florian Fainelli @ 2019-02-27 19:44 UTC (permalink / raw)
  To: netdev
  Cc: Florian Fainelli, David S. Miller, Ido Schimmel, open list,
	open list:STAGING SUBSYSTEM, moderated list:ETHERNET BRIDGE, jiri,
	andrew, vivien.didelot
In-Reply-To: <20190227194432.725-1-f.fainelli@gmail.com>

Following patches will change the way we communicate setting a port's
attribute and use notifiers towards that goal.

Prepare DSA to support receiving notifier events targeting
SWITCHDEV_PORT_ATTR_SET from both atomic and process context and use a
small helper to translate the event notifier into something that
dsa_slave_port_attr_set() can process.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 net/dsa/slave.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 7274499293c9..b089b43120e1 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -1524,6 +1524,19 @@ static int dsa_slave_netdevice_event(struct notifier_block *nb,
 	return NOTIFY_DONE;
 }
 
+static int
+dsa_slave_switchdev_port_attr_set_event(struct net_device *netdev,
+		struct switchdev_notifier_port_attr_info *port_attr_info)
+{
+	int err;
+
+	err = dsa_slave_port_attr_set(netdev, port_attr_info->attr,
+				      port_attr_info->trans);
+
+	port_attr_info->handled = true;
+	return notifier_from_errno(err);
+}
+
 struct dsa_switchdev_event_work {
 	struct work_struct work;
 	struct switchdev_notifier_fdb_info fdb_info;
@@ -1602,6 +1615,9 @@ static int dsa_slave_switchdev_event(struct notifier_block *unused,
 	if (!dsa_slave_dev_check(dev))
 		return NOTIFY_DONE;
 
+	if (event == SWITCHDEV_PORT_ATTR_SET)
+		return dsa_slave_switchdev_port_attr_set_event(dev, ptr);
+
 	switchdev_work = kzalloc(sizeof(*switchdev_work), GFP_ATOMIC);
 	if (!switchdev_work)
 		return NOTIFY_BAD;
@@ -1664,6 +1680,8 @@ static int dsa_slave_switchdev_blocking_event(struct notifier_block *unused,
 	case SWITCHDEV_PORT_OBJ_ADD: /* fall through */
 	case SWITCHDEV_PORT_OBJ_DEL:
 		return dsa_slave_switchdev_port_obj_event(event, dev, ptr);
+	case SWITCHDEV_PORT_ATTR_SET:
+		return dsa_slave_switchdev_port_attr_set_event(dev, ptr);
 	}
 
 	return NOTIFY_DONE;
-- 
2.17.1


^ permalink raw reply related

* [PATCH net-next v3 1/8] switchdev: Add SWITCHDEV_PORT_ATTR_SET
From: Florian Fainelli @ 2019-02-27 19:44 UTC (permalink / raw)
  To: netdev
  Cc: Florian Fainelli, David S. Miller, Ido Schimmel, open list,
	open list:STAGING SUBSYSTEM, moderated list:ETHERNET BRIDGE, jiri,
	andrew, vivien.didelot
In-Reply-To: <20190227194432.725-1-f.fainelli@gmail.com>

In preparation for allowing switchdev enabled drivers to veto specific
attribute settings from within the context of the caller, introduce a
new switchdev notifier type for port attributes.

Suggested-by: Ido Schimmel <idosch@mellanox.com>
Reviewed-by: Ido Schimmel <idosch@mellanox.com>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 include/net/switchdev.h   | 27 +++++++++++++++++++++
 net/switchdev/switchdev.c | 51 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 78 insertions(+)

diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index be4b13e66668..5087c06ceb4b 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -132,6 +132,7 @@ enum switchdev_notifier_type {
 
 	SWITCHDEV_PORT_OBJ_ADD, /* Blocking. */
 	SWITCHDEV_PORT_OBJ_DEL, /* Blocking. */
+	SWITCHDEV_PORT_ATTR_SET, /* May be blocking . */
 
 	SWITCHDEV_VXLAN_FDB_ADD_TO_BRIDGE,
 	SWITCHDEV_VXLAN_FDB_DEL_TO_BRIDGE,
@@ -160,6 +161,13 @@ struct switchdev_notifier_port_obj_info {
 	bool handled;
 };
 
+struct switchdev_notifier_port_attr_info {
+	struct switchdev_notifier_info info; /* must be first */
+	const struct switchdev_attr *attr;
+	struct switchdev_trans *trans;
+	bool handled;
+};
+
 static inline struct net_device *
 switchdev_notifier_info_to_dev(const struct switchdev_notifier_info *info)
 {
@@ -212,7 +220,15 @@ int switchdev_handle_port_obj_del(struct net_device *dev,
 			int (*del_cb)(struct net_device *dev,
 				      const struct switchdev_obj *obj));
 
+int switchdev_handle_port_attr_set(struct net_device *dev,
+			struct switchdev_notifier_port_attr_info *port_attr_info,
+			bool (*check_cb)(const struct net_device *dev),
+			int (*set_cb)(struct net_device *dev,
+				      const struct switchdev_attr *attr,
+				      struct switchdev_trans *trans));
+
 #define SWITCHDEV_SET_OPS(netdev, ops) ((netdev)->switchdev_ops = (ops))
+
 #else
 
 static inline void switchdev_deferred_process(void)
@@ -299,6 +315,17 @@ switchdev_handle_port_obj_del(struct net_device *dev,
 	return 0;
 }
 
+static inline int
+switchdev_handle_port_attr_set(struct net_device *dev,
+			struct switchdev_notifier_port_attr_info *port_attr_info,
+			bool (*check_cb)(const struct net_device *dev),
+			int (*set_cb)(struct net_device *dev,
+				      const struct switchdev_attr *attr,
+				      struct switchdev_trans *trans))
+{
+	return 0;
+}
+
 #define SWITCHDEV_SET_OPS(netdev, ops) do {} while (0)
 
 #endif
diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
index 362413c9b389..3560c19aa7e2 100644
--- a/net/switchdev/switchdev.c
+++ b/net/switchdev/switchdev.c
@@ -655,3 +655,54 @@ int switchdev_handle_port_obj_del(struct net_device *dev,
 	return err;
 }
 EXPORT_SYMBOL_GPL(switchdev_handle_port_obj_del);
+
+static int __switchdev_handle_port_attr_set(struct net_device *dev,
+			struct switchdev_notifier_port_attr_info *port_attr_info,
+			bool (*check_cb)(const struct net_device *dev),
+			int (*set_cb)(struct net_device *dev,
+				      const struct switchdev_attr *attr,
+				      struct switchdev_trans *trans))
+{
+	struct net_device *lower_dev;
+	struct list_head *iter;
+	int err = -EOPNOTSUPP;
+
+	if (check_cb(dev)) {
+		port_attr_info->handled = true;
+		return set_cb(dev, port_attr_info->attr,
+			      port_attr_info->trans);
+	}
+
+	/* Switch ports might be stacked under e.g. a LAG. Ignore the
+	 * unsupported devices, another driver might be able to handle them. But
+	 * propagate to the callers any hard errors.
+	 *
+	 * If the driver does its own bookkeeping of stacked ports, it's not
+	 * necessary to go through this helper.
+	 */
+	netdev_for_each_lower_dev(dev, lower_dev, iter) {
+		err = __switchdev_handle_port_attr_set(lower_dev, port_attr_info,
+						       check_cb, set_cb);
+		if (err && err != -EOPNOTSUPP)
+			return err;
+	}
+
+	return err;
+}
+
+int switchdev_handle_port_attr_set(struct net_device *dev,
+			struct switchdev_notifier_port_attr_info *port_attr_info,
+			bool (*check_cb)(const struct net_device *dev),
+			int (*set_cb)(struct net_device *dev,
+				      const struct switchdev_attr *attr,
+				      struct switchdev_trans *trans))
+{
+	int err;
+
+	err = __switchdev_handle_port_attr_set(dev, port_attr_info, check_cb,
+					       set_cb);
+	if (err == -EOPNOTSUPP)
+		err = 0;
+	return err;
+}
+EXPORT_SYMBOL_GPL(switchdev_handle_port_attr_set);
-- 
2.17.1


^ 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