Netdev List
 help / color / mirror / Atom feed
* [net-next V2 PATCH 1/5] bpf: introduce new bpf cpu map type BPF_MAP_TYPE_CPUMAP
From: Jesper Dangaard Brouer @ 2017-09-29 16:34 UTC (permalink / raw)
  To: netdev
  Cc: jakub.kicinski, Michael S. Tsirkin, Jason Wang, mchan,
	John Fastabend, peter.waskiewicz.jr, Jesper Dangaard Brouer,
	Daniel Borkmann, Alexei Starovoitov, Andy Gospodarek
In-Reply-To: <150670281423.23765.8984643281418950330.stgit@firesoul>

The 'cpumap' is primary used as a backend map for XDP BPF helper
call bpf_redirect_map() and XDP_REDIRECT action, like 'devmap'.

This patch implement the main part of the map.  It is not connected to
the XDP redirect system yet, and no SKB allocation are done yet.

The main concern in this patch is to ensure the datapath can run
without any locking.  This adds complexity to the setup and tear-down
procedure, which assumptions are extra carefully documented in the
code comments.

V2: make sure array isn't larger than num possible CPUs

Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
 include/linux/bpf_types.h      |    1 
 include/uapi/linux/bpf.h       |    1 
 kernel/bpf/Makefile            |    1 
 kernel/bpf/cpumap.c            |  555 ++++++++++++++++++++++++++++++++++++++++
 kernel/bpf/syscall.c           |    8 +
 tools/include/uapi/linux/bpf.h |    1 
 6 files changed, 566 insertions(+), 1 deletion(-)
 create mode 100644 kernel/bpf/cpumap.c

diff --git a/include/linux/bpf_types.h b/include/linux/bpf_types.h
index 6f1a567667b8..814c1081a4a9 100644
--- a/include/linux/bpf_types.h
+++ b/include/linux/bpf_types.h
@@ -41,4 +41,5 @@ BPF_MAP_TYPE(BPF_MAP_TYPE_DEVMAP, dev_map_ops)
 #ifdef CONFIG_STREAM_PARSER
 BPF_MAP_TYPE(BPF_MAP_TYPE_SOCKMAP, sock_map_ops)
 #endif
+BPF_MAP_TYPE(BPF_MAP_TYPE_CPUMAP, cpu_map_ops)
 #endif
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index e43491ac4823..f14e15702533 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -111,6 +111,7 @@ enum bpf_map_type {
 	BPF_MAP_TYPE_HASH_OF_MAPS,
 	BPF_MAP_TYPE_DEVMAP,
 	BPF_MAP_TYPE_SOCKMAP,
+	BPF_MAP_TYPE_CPUMAP,
 };
 
 enum bpf_prog_type {
diff --git a/kernel/bpf/Makefile b/kernel/bpf/Makefile
index 897daa005b23..dba0bd33a43c 100644
--- a/kernel/bpf/Makefile
+++ b/kernel/bpf/Makefile
@@ -4,6 +4,7 @@ obj-$(CONFIG_BPF_SYSCALL) += syscall.o verifier.o inode.o helpers.o tnum.o
 obj-$(CONFIG_BPF_SYSCALL) += hashtab.o arraymap.o percpu_freelist.o bpf_lru_list.o lpm_trie.o map_in_map.o
 ifeq ($(CONFIG_NET),y)
 obj-$(CONFIG_BPF_SYSCALL) += devmap.o
+obj-$(CONFIG_BPF_SYSCALL) += cpumap.o
 ifeq ($(CONFIG_STREAM_PARSER),y)
 obj-$(CONFIG_BPF_SYSCALL) += sockmap.o
 endif
diff --git a/kernel/bpf/cpumap.c b/kernel/bpf/cpumap.c
new file mode 100644
index 000000000000..94fe2047e264
--- /dev/null
+++ b/kernel/bpf/cpumap.c
@@ -0,0 +1,555 @@
+/* bpf/cpumap.c
+ *
+ * Copyright (c) 2017 Jesper Dangaard Brouer, Red Hat Inc.
+ * Released under terms in GPL version 2.  See COPYING.
+ */
+
+/* The 'cpumap' is primary used as a backend map for XDP BPF helper
+ * call bpf_redirect_map() and XDP_REDIRECT action, like 'devmap'.
+ *
+ * Unlike devmap which redirect XDP frames out another NIC device,
+ * this map type redirect raw XDP frames to another CPU.  The remote
+ * CPU will do SKB-allocation and call the normal network stack.
+ *
+ * This is a scalability and isolation mechanism, that allow
+ * separating the early driver network XDP layer, from the rest of the
+ * netstack, and assigning dedicated CPUs for this stage.  This
+ * basically allows for 10G wirespeed pre-filtering via bpf.
+ */
+#include <linux/bpf.h>
+#include <linux/filter.h>
+#include <linux/ptr_ring.h>
+
+#include <linux/sched.h>
+#include <linux/workqueue.h>
+#include <linux/kthread.h>
+
+/*
+ * General idea: XDP packets getting XDP redirected to another CPU,
+ * will maximum be stored/queued for one driver ->poll() call.  It is
+ * guaranteed that setting flush bit and flush operation happen on
+ * same CPU.  Thus, cpu_map_flush operation can deduct via this_cpu_ptr()
+ * which queue in bpf_cpu_map_entry contains packets.
+ */
+
+#define CPU_MAP_BULK_SIZE 8  /* 8 == one cacheline on 64-bit archs */
+struct xdp_bulk_queue {
+	void *q[CPU_MAP_BULK_SIZE];
+	unsigned int count;
+};
+
+/* Struct for every remote "destination" CPU in map */
+struct bpf_cpu_map_entry {
+	u32 cpu;    /* kthread CPU and map index */
+	int map_id; /* Back reference to map */
+	u32 qsize;  /* Redundant queue size for map lookup */
+
+	/* XDP can run multiple RX-ring queues, need __percpu enqueue store */
+	struct xdp_bulk_queue __percpu *bulkq;
+
+	/* Queue with potential multi-producers, and single-consumer kthread */
+	struct ptr_ring *queue;
+	struct task_struct *kthread;
+	struct work_struct kthread_stop_wq;
+
+	atomic_t refcnt; /* Control when this struct can be free'ed */
+	struct rcu_head rcu;
+};
+
+struct bpf_cpu_map {
+	struct bpf_map map;
+	/* Below members specific for map type */
+	struct bpf_cpu_map_entry **cpu_map;
+	unsigned long __percpu *flush_needed;
+};
+
+static int bq_flush_to_queue(struct bpf_cpu_map_entry *rcpu,
+			     struct xdp_bulk_queue *bq);
+
+static u64 cpu_map_bitmap_size(const union bpf_attr *attr)
+{
+	return BITS_TO_LONGS(attr->max_entries) * sizeof(unsigned long);
+}
+
+static struct bpf_map *cpu_map_alloc(union bpf_attr *attr)
+{
+	struct bpf_cpu_map *cmap;
+	u64 cost;
+	int err;
+
+	/* check sanity of attributes */
+	if (attr->max_entries == 0 || attr->key_size != 4 ||
+	    attr->value_size != 4 || attr->map_flags & ~BPF_F_NUMA_NODE)
+		return ERR_PTR(-EINVAL);
+
+	cmap = kzalloc(sizeof(*cmap), GFP_USER);
+	if (!cmap)
+		return ERR_PTR(-ENOMEM);
+
+	/* mandatory map attributes */
+	cmap->map.map_type = attr->map_type;
+	cmap->map.key_size = attr->key_size;
+	cmap->map.value_size = attr->value_size;
+	cmap->map.max_entries = attr->max_entries;
+	cmap->map.map_flags = attr->map_flags;
+	cmap->map.numa_node = bpf_map_attr_numa_node(attr);
+
+	/* limit array to resonable size based on NR_CPUS, not final CPU check*/
+	if (cmap->map.max_entries > NR_CPUS)
+		return ERR_PTR(-E2BIG);
+
+	/* make sure page count doesn't overflow */
+	cost = (u64) cmap->map.max_entries * sizeof(struct bpf_cpu_map_entry *);
+	cost += cpu_map_bitmap_size(attr) * num_possible_cpus();
+	if (cost >= U32_MAX - PAGE_SIZE)
+		goto free_cmap;
+	cmap->map.pages = round_up(cost, PAGE_SIZE) >> PAGE_SHIFT;
+
+	/* if map size is larger than memlock limit, reject it early */
+	err = bpf_map_precharge_memlock(cmap->map.pages);
+	if (err)
+		goto free_cmap;
+
+	/* A per cpu bitfield with a bit per possible CPU in map  */
+	cmap->flush_needed = __alloc_percpu(cpu_map_bitmap_size(attr),
+					    __alignof__(unsigned long));
+	if (!cmap->flush_needed)
+		goto free_cmap;
+
+	/* Alloc array for possible remote "destination" CPUs */
+	cmap->cpu_map = bpf_map_area_alloc(cmap->map.max_entries *
+					   sizeof(struct bpf_cpu_map_entry *),
+					   cmap->map.numa_node);
+	if (!cmap->cpu_map)
+		goto free_cmap;
+
+	return &cmap->map;
+free_cmap:
+	free_percpu(cmap->flush_needed);
+	kfree(cmap);
+	return ERR_PTR(-ENOMEM);
+}
+
+void __cpu_map_queue_destructor(void *ptr)
+{
+	/* For now, just catch this as an error */
+	if (!ptr)
+		return;
+	pr_err("ERROR: %s() cpu_map queue was not empty\n", __func__);
+	page_frag_free(ptr);
+}
+
+static void put_cpu_map_entry(struct bpf_cpu_map_entry *rcpu)
+{
+	if (atomic_dec_and_test(&rcpu->refcnt)) {
+		/* The queue should be empty at this point */
+		ptr_ring_cleanup(rcpu->queue, __cpu_map_queue_destructor);
+		kfree(rcpu->queue);
+		kfree(rcpu);
+	}
+}
+
+static void get_cpu_map_entry(struct bpf_cpu_map_entry *rcpu)
+{
+	atomic_inc(&rcpu->refcnt);
+}
+
+/* called from workqueue, to workaround syscall using preempt_disable */
+static void cpu_map_kthread_stop(struct work_struct *work)
+{
+	struct bpf_cpu_map_entry *rcpu;
+
+	rcpu = container_of(work, struct bpf_cpu_map_entry, kthread_stop_wq);
+	synchronize_rcu(); /* wait for flush in __cpu_map_entry_free() */
+	kthread_stop(rcpu->kthread); /* calls put_cpu_map_entry */
+}
+
+static int cpu_map_kthread_run(void *data)
+{
+	struct bpf_cpu_map_entry *rcpu = data;
+
+	set_current_state(TASK_INTERRUPTIBLE);
+	while (!kthread_should_stop()) {
+		struct xdp_pkt *xdp_pkt;
+
+		schedule();
+		/* Do work */
+		while ((xdp_pkt = ptr_ring_consume(rcpu->queue))) {
+			/* For now just "refcnt-free" */
+			page_frag_free(xdp_pkt);
+		}
+		__set_current_state(TASK_INTERRUPTIBLE);
+	}
+	put_cpu_map_entry(rcpu);
+
+	__set_current_state(TASK_RUNNING);
+	return 0;
+}
+
+struct bpf_cpu_map_entry *__cpu_map_entry_alloc(u32 qsize, u32 cpu, int map_id)
+{
+	gfp_t gfp = GFP_ATOMIC|__GFP_NOWARN;
+	struct bpf_cpu_map_entry *rcpu;
+	int numa, err;
+
+	/* Have map->numa_node, but choose node of redirect target CPU */
+	numa = cpu_to_node(cpu);
+
+	rcpu = kzalloc_node(sizeof(*rcpu), gfp, numa);
+	if (!rcpu)
+		return NULL;
+
+	/* Alloc percpu bulkq */
+	rcpu->bulkq = __alloc_percpu_gfp(sizeof(*rcpu->bulkq),
+					 sizeof(void *), gfp);
+	if (!rcpu->bulkq)
+		goto fail;
+
+	/* Alloc queue */
+	rcpu->queue = kzalloc_node(sizeof(*rcpu->queue), gfp, numa);
+	if (!rcpu->queue)
+		goto fail;
+
+	err = ptr_ring_init(rcpu->queue, qsize, gfp);
+	if (err)
+		goto fail;
+	rcpu->qsize = qsize;
+
+	/* Setup kthread */
+	rcpu->kthread = kthread_create_on_node(cpu_map_kthread_run, rcpu, numa,
+					       "cpumap/%d/map:%d", cpu, map_id);
+	if (IS_ERR(rcpu->kthread))
+		goto fail;
+
+	/* Make sure kthread runs on a single CPU */
+	kthread_bind(rcpu->kthread, cpu);
+	wake_up_process(rcpu->kthread);
+
+	get_cpu_map_entry(rcpu); /* 1-refcnt for being in cmap->cpu_map[] */
+	get_cpu_map_entry(rcpu); /* 1-refcnt for kthread */
+
+	return rcpu;
+
+fail:   /* Hint: free API detect NULL values */
+	free_percpu(rcpu->bulkq);
+	kfree(rcpu->queue);
+	kfree(rcpu);
+	return NULL;
+}
+
+void __cpu_map_entry_free(struct rcu_head *rcu)
+{
+	struct bpf_cpu_map_entry *rcpu;
+	int cpu;
+
+	/* This cpu_map_entry have been disconnected from map and one
+	 * RCU graze-period have elapsed.  Thus, XDP cannot queue any
+	 * new packets and cannot change/set flush_needed that can
+	 * find this entry.
+	 */
+	rcpu = container_of(rcu, struct bpf_cpu_map_entry, rcu);
+
+	/* Flush remaining packets in percpu bulkq */
+	for_each_online_cpu(cpu) {
+		struct xdp_bulk_queue *bq = per_cpu_ptr(rcpu->bulkq, cpu);
+
+		/* No concurrent bq_enqueue can run at this point */
+		bq_flush_to_queue(rcpu, bq);
+	}
+	free_percpu(rcpu->bulkq);
+	/* Cannot kthread_stop() here, last put free rcpu resources */
+	put_cpu_map_entry(rcpu);
+}
+
+/*
+ * After xchg pointer to bpf_cpu_map_entry, use the call_rcu() to
+ * ensure any driver rcu critical sections have completed, but this
+ * does not guarantee a flush has happened yet. Because driver side
+ * rcu_read_lock/unlock only protects the running XDP program.  The
+ * atomic xchg and NULL-ptr check in __cpu_map_flush() makes sure a
+ * pending flush op doesn't fail.
+ *
+ * The bpf_cpu_map_entry is still used by the kthread, and there can
+ * still be pending packets (in queue and percpu bulkq).  A refcnt
+ * makes sure to last user (kthread_stop vs. call_rcu) free memory
+ * resources.
+ *
+ * The rcu callback __cpu_map_entry_free flush remaining packets in
+ * percpu bulkq to queue.  Due to caller map_delete_elem() disable
+ * preemption, cannot call kthread_stop() to make sure queue is empty.
+ * Instead a work_queue is started for stopping kthread,
+ * cpu_map_kthread_stop, which waits for an RCU graze period before
+ * stopping kthread, emptying the queue.
+ */
+void __cpu_map_entry_replace(struct bpf_cpu_map *cmap,
+			     u32 key_cpu, struct bpf_cpu_map_entry *rcpu)
+{
+	struct bpf_cpu_map_entry *old_rcpu;
+
+	old_rcpu = xchg(&cmap->cpu_map[key_cpu], rcpu);
+	if (old_rcpu) {
+		call_rcu(&old_rcpu->rcu, __cpu_map_entry_free);
+		INIT_WORK(&old_rcpu->kthread_stop_wq, cpu_map_kthread_stop);
+		schedule_work(&old_rcpu->kthread_stop_wq);
+	}
+}
+
+int cpu_map_delete_elem(struct bpf_map *map, void *key)
+{
+	struct bpf_cpu_map *cmap = container_of(map, struct bpf_cpu_map, map);
+	u32 key_cpu = *(u32 *)key;
+
+	if (key_cpu >= map->max_entries)
+		return -EINVAL;
+
+	/* notice caller map_delete_elem() use preempt_disable() */
+	__cpu_map_entry_replace(cmap, key_cpu, NULL);
+	return 0;
+}
+
+int cpu_map_update_elem(struct bpf_map *map, void *key, void *value,
+				u64 map_flags)
+{
+	struct bpf_cpu_map *cmap = container_of(map, struct bpf_cpu_map, map);
+	struct bpf_cpu_map_entry *rcpu;
+
+	/* Array index key correspond to CPU number */
+	u32 key_cpu = *(u32 *)key;
+	/* Value is the queue size */
+	u32 qsize = *(u32 *)value;
+
+	/* make sure CPU is a valid possible cpu */
+	if (key_cpu > num_possible_cpus())
+		return -ENODEV;
+
+	if (unlikely(map_flags > BPF_EXIST))
+		return -EINVAL;
+	if (unlikely(key_cpu >= cmap->map.max_entries))
+		return -E2BIG;
+	if (unlikely(map_flags == BPF_NOEXIST))
+		return -EEXIST;
+	if (unlikely(qsize > 16384)) /* sanity limit on qsize */
+		return -EOVERFLOW;
+
+	if (qsize == 0) {
+		rcpu = NULL; /* Same as deleting */
+	} else {
+		/* Updating qsize cause re-allocation of bpf_cpu_map_entry */
+		rcpu = __cpu_map_entry_alloc(qsize, key_cpu, map->id);
+		if (!rcpu)
+			return -ENOMEM;
+	}
+	rcu_read_lock();
+	__cpu_map_entry_replace(cmap, key_cpu, rcpu);
+	rcu_read_unlock();
+	return 0;
+}
+
+void cpu_map_free(struct bpf_map *map)
+{
+	struct bpf_cpu_map *cmap = container_of(map, struct bpf_cpu_map, map);
+	int cpu;
+	u32 i;
+
+	/* At this point bpf_prog->aux->refcnt == 0 and this map->refcnt == 0,
+	 * so the bpf programs (can be more than one that used this map) were
+	 * disconnected from events. Wait for outstanding critical sections in
+	 * these programs to complete. The rcu critical section only guarantees
+	 * no further "XDP/bpf-side" reads against bpf_cpu_map->cpu_map.
+	 * It does __not__ ensure pending flush operations (if any) are
+	 * complete.
+	 */
+	synchronize_rcu();
+
+	/* To ensure all pending flush operations have completed wait for flush
+	 * bitmap to indicate all flush_needed bits to be zero on _all_ cpus.
+	 * Because the above synchronize_rcu() ensures the map is disconnected
+	 * from the program we can assume no new bits will be set.
+	 */
+	for_each_online_cpu(cpu) {
+		unsigned long *bitmap = per_cpu_ptr(cmap->flush_needed, cpu);
+
+		while (!bitmap_empty(bitmap, cmap->map.max_entries))
+			cond_resched();
+	}
+
+	/* For cpu_map the remote CPUs can still be using the entries
+	 * (struct bpf_cpu_map_entry).
+	 */
+	for (i = 0; i < cmap->map.max_entries; i++) {
+		struct bpf_cpu_map_entry *rcpu;
+
+		rcpu = READ_ONCE(cmap->cpu_map[i]);
+		if (!rcpu)
+			continue;
+
+		/* bq flush and cleanup happens after RCU graze-period */
+		__cpu_map_entry_replace(cmap, i, NULL); /* call_rcu */
+	}
+	free_percpu(cmap->flush_needed);
+	bpf_map_area_free(cmap->cpu_map);
+	kfree(cmap);
+}
+
+struct bpf_cpu_map_entry *__cpu_map_lookup_elem(struct bpf_map *map, u32 key)
+{
+	struct bpf_cpu_map *cmap = container_of(map, struct bpf_cpu_map, map);
+	struct bpf_cpu_map_entry *rcpu;
+
+	if (key >= map->max_entries)
+		return NULL;
+
+	rcpu = READ_ONCE(cmap->cpu_map[key]);
+	return rcpu;
+}
+
+static void *cpu_map_lookup_elem(struct bpf_map *map, void *key)
+{
+	struct bpf_cpu_map_entry *rcpu =
+		__cpu_map_lookup_elem(map, *(u32 *)key);
+
+	return rcpu ? &rcpu->qsize : NULL;
+}
+
+static int cpu_map_get_next_key(struct bpf_map *map, void *key, void *next_key)
+{
+	struct bpf_cpu_map *cmap = container_of(map, struct bpf_cpu_map, map);
+	u32 index = key ? *(u32 *)key : U32_MAX;
+	u32 *next = next_key;
+
+	if (index >= cmap->map.max_entries) {
+		*next = 0;
+		return 0;
+	}
+
+	if (index == cmap->map.max_entries - 1)
+		return -ENOENT;
+	*next = index + 1;
+	return 0;
+}
+
+const struct bpf_map_ops cpu_map_ops = {
+	.map_alloc		= cpu_map_alloc,
+	.map_free		= cpu_map_free,
+	.map_delete_elem	= cpu_map_delete_elem,
+	.map_update_elem	= cpu_map_update_elem,
+	.map_lookup_elem	= cpu_map_lookup_elem,
+	.map_get_next_key	= cpu_map_get_next_key,
+};
+
+
+static int bq_flush_to_queue(struct bpf_cpu_map_entry *rcpu,
+			     struct xdp_bulk_queue *bq)
+{
+	struct ptr_ring *q;
+	int i;
+
+	if (unlikely(!bq->count))
+		return 0;
+
+	q = rcpu->queue;
+	spin_lock(&q->producer_lock);
+
+	for (i = 0; i < bq->count; i++) {
+		void *xdp_pkt = bq->q[i];
+		int err;
+
+		err = __ptr_ring_produce(q, xdp_pkt);
+		if (err) {
+			/* Free xdp_pkt */
+			page_frag_free(xdp_pkt);
+		}
+	}
+	bq->count = 0;
+	spin_unlock(&q->producer_lock);
+
+	return 0;
+}
+
+/* Notice: Will change in later patch */
+struct xdp_pkt {
+	void *data;
+	u16 len;
+	u16 headroom;
+};
+
+/* Runs under RCU-read-side, plus in softirq under NAPI protection.
+ * Thus, safe percpu variable access.
+ */
+static int bq_enqueue(struct bpf_cpu_map_entry *rcpu, struct xdp_pkt *xdp_pkt)
+{
+	struct xdp_bulk_queue *bq = this_cpu_ptr(rcpu->bulkq);
+
+	if (unlikely(bq->count == CPU_MAP_BULK_SIZE)) {
+		bq_flush_to_queue(rcpu, bq);
+	}
+	/* Notice, xdp_buff/page MUST be queued here, long enough for
+	 * driver to code invoking us to finished, due to driver
+	 * (e.g. ixgbe) recycle tricks based on page-refcnt.
+	 *
+	 * Thus, incoming xdp_pkt is always queued here (else we race
+	 * with another CPU on page-refcnt and remaining driver code).
+	 * Queue time is very short, as driver will invoke flush
+	 * operation, when completing napi->poll call.
+	 */
+	bq->q[bq->count++] = xdp_pkt;
+	return 0;
+}
+
+int cpu_map_enqueue(struct bpf_cpu_map_entry *rcpu, struct xdp_buff *xdp)
+{
+	struct xdp_pkt *xdp_pkt;
+	int headroom;
+
+	/* Convert xdp_buff to xdp_pkt */
+	headroom = xdp->data - xdp->data_hard_start;
+	if (headroom < sizeof(*xdp_pkt))
+		return -EOVERFLOW;
+	xdp_pkt = xdp->data_hard_start;
+	xdp_pkt->data = xdp->data;
+	xdp_pkt->len  = xdp->data_end - xdp->data;
+	xdp_pkt->headroom = headroom;
+	/* For now this is just used as a void pointer to data_hard_start */
+
+	bq_enqueue(rcpu, xdp_pkt);
+	return 0;
+}
+
+void __cpu_map_insert_ctx(struct bpf_map *map, u32 bit)
+{
+	struct bpf_cpu_map *cmap = container_of(map, struct bpf_cpu_map, map);
+	unsigned long *bitmap = this_cpu_ptr(cmap->flush_needed);
+
+	__set_bit(bit, bitmap);
+}
+
+void __cpu_map_flush(struct bpf_map *map)
+{
+	struct bpf_cpu_map *cmap = container_of(map, struct bpf_cpu_map, map);
+	unsigned long *bitmap = this_cpu_ptr(cmap->flush_needed);
+	u32 bit;
+
+	/* The napi->poll softirq makes sure __cpu_map_insert_ctx()
+	 * and __cpu_map_flush() happen on same CPU. Thus, the percpu
+	 * bitmap indicate which percpu bulkq have packets.
+	 */
+	for_each_set_bit(bit, bitmap, map->max_entries) {
+		struct bpf_cpu_map_entry *rcpu = READ_ONCE(cmap->cpu_map[bit]);
+		struct xdp_bulk_queue *bq;
+
+		/* This is possible if entry is removed by user space
+		 * between xdp redirect and flush op.
+		 */
+		if (unlikely(!rcpu))
+			continue;
+
+		__clear_bit(bit, bitmap);
+
+		/* Flush all frames in bulkq to real queue */
+		bq = this_cpu_ptr(rcpu->bulkq);
+		bq_flush_to_queue(rcpu, bq);
+
+		/* If already running, costs spin_lock_irqsave + smb_mb */
+		wake_up_process(rcpu->kthread);
+	}
+}
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 25d074920a00..68fe3f51e1a0 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -562,6 +562,12 @@ static int map_update_elem(union bpf_attr *attr)
 	if (copy_from_user(value, uvalue, value_size) != 0)
 		goto free_value;
 
+	/* Need to create a kthread, thus must support schedule */
+	if (map->map_type == BPF_MAP_TYPE_CPUMAP) {
+		err = map->ops->map_update_elem(map, key, value, attr->flags);
+		goto out;
+	}
+
 	/* must increment bpf_prog_active to avoid kprobe+bpf triggering from
 	 * inside bpf map update or delete otherwise deadlocks are possible
 	 */
@@ -592,7 +598,7 @@ static int map_update_elem(union bpf_attr *attr)
 	}
 	__this_cpu_dec(bpf_prog_active);
 	preempt_enable();
-
+out:
 	if (!err)
 		trace_bpf_map_update_elem(map, ufd, key, value);
 free_value:
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index e43491ac4823..f14e15702533 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -111,6 +111,7 @@ enum bpf_map_type {
 	BPF_MAP_TYPE_HASH_OF_MAPS,
 	BPF_MAP_TYPE_DEVMAP,
 	BPF_MAP_TYPE_SOCKMAP,
+	BPF_MAP_TYPE_CPUMAP,
 };
 
 enum bpf_prog_type {

^ permalink raw reply related

* [net-next V2 PATCH 0/5] New bpf cpumap type for XDP_REDIRECT
From: Jesper Dangaard Brouer @ 2017-09-29 16:34 UTC (permalink / raw)
  To: netdev
  Cc: jakub.kicinski, Michael S. Tsirkin, Jason Wang, mchan,
	John Fastabend, peter.waskiewicz.jr, Jesper Dangaard Brouer,
	Daniel Borkmann, Alexei Starovoitov, Andy Gospodarek

Introducing a new way to redirect XDP frames.  Notice how no driver
changes are necessary given the design of XDP_REDIRECT.

This redirect map type is called 'cpumap', as it allows redirection
XDP frames to remote CPUs.  The remote CPU will do the SKB allocation
and start the network stack invocation on that CPU.

This is a scalability and isolation mechanism, that allow separating
the early driver network XDP layer, from the rest of the netstack, and
assigning dedicated CPUs for this stage.  The sysadm control/configure
the RX-CPU to NIC-RX queue (as usual) via procfs smp_affinity and how
many queues are configured via ethtool --set-channels.  Benchmarks
show that a single CPU can handle approx 11Mpps.  Thus, only assigning
two NIC RX-queues (and two CPUs) is sufficient for handling 10Gbit/s
wirespeed smallest packet 14.88Mpps.  Reducing the number of queues
have the advantage that more packets being "bulk" available per hard
interrupt[1].

[1] https://www.netdevconf.org/2.1/papers/BusyPollingNextGen.pdf

Use-cases:

1. End-host based pre-filtering for DDoS mitigation.  This is fast
   enough to allow software to see and filter all packets wirespeed.
   Thus, no packets getting silently dropped by hardware.

2. Given NIC HW unevenly distributes packets across RX queue, this
   mechanism can be used for redistribution load across CPUs.  This
   usually happens when HW is unaware of a new protocol.  This
   resembles RPS (Receive Packet Steering), just faster, but with more
   responsibility placed on the BPF program for correct steering.

3. Auto-scaling or power saving via only activating the appropriate
   number of remote CPUs for handling the current load.  The cpumap
   tracepoints can function as a feedback loop for this purpose.

Patchset based on net-next at:
 commit 14a0d032f4ec ("Merge branch 'mlxsw-pass-gact'")

---

Jesper Dangaard Brouer (5):
      bpf: introduce new bpf cpu map type BPF_MAP_TYPE_CPUMAP
      bpf: XDP_REDIRECT enable use of cpumap
      bpf: cpumap xdp_buff to skb conversion and allocation
      bpf: cpumap add tracepoints
      samples/bpf: add cpumap sample program xdp_redirect_cpu


 include/linux/bpf.h                 |    7 
 include/linux/bpf_types.h           |    1 
 include/trace/events/xdp.h          |   80 ++++
 include/uapi/linux/bpf.h            |    1 
 kernel/bpf/Makefile                 |    1 
 kernel/bpf/cpumap.c                 |  686 +++++++++++++++++++++++++++++++++++
 kernel/bpf/syscall.c                |    8 
 kernel/bpf/verifier.c               |    3 
 net/core/filter.c                   |   65 +++
 samples/bpf/Makefile                |    4 
 samples/bpf/xdp_redirect_cpu_kern.c |  640 +++++++++++++++++++++++++++++++++
 samples/bpf/xdp_redirect_cpu_user.c |  639 +++++++++++++++++++++++++++++++++
 tools/include/uapi/linux/bpf.h      |    1 
 13 files changed, 2124 insertions(+), 12 deletions(-)
 create mode 100644 kernel/bpf/cpumap.c
 create mode 100644 samples/bpf/xdp_redirect_cpu_kern.c
 create mode 100644 samples/bpf/xdp_redirect_cpu_user.c

^ permalink raw reply

* RE: [RESEND PATCH 2/6] staging: fsl-dpaa2/ethsw: Add Freescale DPAA2 Ethernet Switch driver
From: Florian Fainelli @ 2017-09-29 16:11 UTC (permalink / raw)
  To: Razvan Stefanescu, Bogdan Purcareata, gregkh@linuxfoundation.org
  Cc: devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org,
	netdev@vger.kernel.org, agraf@suse.de, arnd@arndb.de,
	Alexandru Marginean, Ruxandra Ioana Radulescu, Laurentiu Tudor,
	stuyoder@gmail.com
In-Reply-To: <AM3PR04MB0743F62497ED0CB445F51524E67E0@AM3PR04MB0743.eurprd04.prod.outlook.com>

On September 29, 2017 6:59:18 AM PDT, Razvan Stefanescu <razvan.stefanescu@nxp.com> wrote:
>
>
>> -----Original Message-----
>> From: Bogdan Purcareata
>> Sent: Friday, September 29, 2017 16:36
>> To: Razvan Stefanescu <razvan.stefanescu@nxp.com>;
>> gregkh@linuxfoundation.org
>> Cc: devel@driverdev.osuosl.org; linux-kernel@vger.kernel.org;
>> netdev@vger.kernel.org; agraf@suse.de; arnd@arndb.de; Alexandru
>Marginean
>> <alexandru.marginean@nxp.com>; Ruxandra Ioana Radulescu
>> <ruxandra.radulescu@nxp.com>; Laurentiu Tudor
><laurentiu.tudor@nxp.com>;
>> stuyoder@gmail.com
>> Subject: RE: [RESEND PATCH 2/6] staging: fsl-dpaa2/ethsw: Add
>Freescale DPAA2
>> Ethernet Switch driver
>> 
>> > Introduce the DPAA2 Ethernet Switch driver, which manages Datapath
>Switch
>> > (DPSW) objects discovered on the MC bus.
>> >
>> > Suggested-by: Alexandru Marginean <alexandru.marginean@nxp.com>
>> > Signed-off-by: Razvan Stefanescu <razvan.stefanescu@nxp.com>

This looks pretty good for a new switchdev driver, is there a reason you can't target drivers/net/ethernet instead of staging? Is it because the MC bus code is still in staging (AFAICT)?

-- 
Florian

^ permalink raw reply

* Re: [PATCH v4 net-next 0/8] flow_dissector: Protocol specific flow dissector offload
From: Tom Herbert @ 2017-09-29 15:48 UTC (permalink / raw)
  To: Hannes Frederic Sowa
  Cc: Tom Herbert, David S. Miller, Linux Kernel Network Developers,
	Rohit Seth
In-Reply-To: <87y3oyylzv.fsf@stressinduktion.org>

On Fri, Sep 29, 2017 at 12:58 AM, Hannes Frederic Sowa
<hannes@stressinduktion.org> wrote:
> Tom Herbert <tom@quantonium.net> writes:
>
>> This patch set adds a new offload type to perform flow dissection for
>> specific protocols (either by EtherType or by IP protocol). This is
>> primary useful to crack open UDP encapsulations (like VXLAN, GUE) for
>> the purposes of parsing the encapsulated packet.
>>
>> Items in this patch set:
>> - Create new protocol case in __skb_dissect for ETH_P_TEB. This is based
>>   on the code in the GRE dissect function and the special handling in
>>   GRE can now be removed (it sets protocol to ETH_P_TEB and returns so
>>   goto proto_again is done)
>> - Add infrastructure for protocol specific flow dissection offload
>> - Add infrastructure to perform UDP flow dissection. Uses same model of
>>   GRO where a flow_dissect callback can be associated with a UDP
>>   socket
>> - Use the infrastructure to support flow dissection of VXLAN and GUE
>>
>> Tested:
>>
>> Forced RPS to call flow dissection for VXLAN, FOU, and GUE. Observed
>> that inner packet was being properly dissected.
>
> I have the feeling that this patch series changes the behavior of flower
> and thus causes uAPI problems.
>
The flow_dissector interface is not a uAPI. And in this case we are
not changing behavior, we are extending the functionality which is a
routine occurrence in this facility. Semantically UDP encapsulations
are no different than other encapsulations, it's just that the details
are different. This patch set brings us closer consistency across
various encapsulation protocols. For instance, if some were to upgrade
use of GRE to GRE/UDP the user would likely expect that the UDP is
mostly transparent and that accelerations and parsing (like
flow_disseector) of GRE are the same.

> flower seems to use the flow dissector results for parsing the inner
> packets. In case of vxlan in vxlan encapsulation, which seems to become
> more common (sigh!) you let part of the flow specification match on the
> most inner header, while the flower ingress filter might want to match
> inside the first encapsulation only.

I don't see why this would be any different than if flower wanted to
match on the outer headers of a GRE packet versus the inner headers.
In any case, there are already FLOW_DISSECTOR_F_STOP_AT_L3,
FLOW_DISSECTOR_F_STOP_AT_FLOW_LABEL, and
FLOW_DISSECTOR_F_STOP_AT_ENCAP-- those should be sufficient to control
the depth of parsing for flower or other use cases.

Tom

^ permalink raw reply

* RE: [PATCH net-next RFC 0/9] net: dsa: PTP timestamping for mv88e6xxx
From: Brandon Streiff @ 2017-09-29 15:34 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	David S. Miller, Florian Fainelli, Vivien Didelot,
	Richard Cochran, Erik Hons
In-Reply-To: <20170928173629.GD14940@lunn.ch>

> From: Andrew Lunn [mailto:andrew@lunn.ch]
> Sent: Thursday, September 28, 2017 12:36 PM
>
> I assume ptp already has the core code to use pinctrl and Linux
> standard GPIOs? What does the device tree binding look like? How do
> you specify the GPIOs to use?
> 
> What we want to avoid is defining an ABI now, otherwise it is going to
> be hard to swap to pinctrl later.

A ptp_clock_info has an array of struct ptp_pin_desc which defines "pins" with a name ("Hardware specific human readable pin name"), an index, and a bitmask of valid functions. The ptp_pin_desc structure is shared with usermode for the PTP_PIN_GETFUNC and PTP_PIN_SETFUNC ioctls. The pins are also exposed in sysfs (see Documentation/ABI/testing/sysfs-ptp). The underlying implementation for configuring the hardware is left up to the PHC driver. I don't see any drivers today that use the PHC pin API as a layer over pinctrl/gpiochip, but there's no reason that that couldn't be the case.

For mv88e6xxx, we name the pins using the pattern "mv88e6xxx_gpio%d"; this appears to be in line with current practice (igb_ptp.c uses "SDP%d", mlx5 driver uses "mlx5_pps%d"). Usermode code appears to be expected to determine which pin it needs to use. (Our current userspace code, for instance, knows that it needs to find "mv88e6xxx_gpio8".)

For mv88e6xxx, Device Tree does feel like a better option here for declaring names, functions, and pin usages. Not all platforms that use the PTP API use Device Tree though.

-- brandon

^ permalink raw reply

* RE: [PATCH net-next RFC 6/9] net: dsa: forward timestamping callbacks to switch drivers
From: Brandon Streiff @ 2017-09-29 15:30 UTC (permalink / raw)
  To: Florian Fainelli, netdev@vger.kernel.org
  Cc: linux-kernel@vger.kernel.org, David S. Miller, Andrew Lunn,
	Vivien Didelot, Richard Cochran, Erik Hons
In-Reply-To: <c4b98f32-c495-623a-53b0-56c1cdf9806a@gmail.com>

> From: Florian Fainelli [mailto:f.fainelli@gmail.com]
> Sent: Thursday, September 28, 2017 12:40 PM
>
> Can we also have a fast-path bypass in case time stamping is not
> supported by the switch so we don't have to even try to classify this
> packet only to realize we don't have a port_rxtsamp() operation later?
> You can either gate this with a compile-time option, or use e.g: a
> static key or something like an early test?

I was trying to follow the existing pattern for skb_defer_rx_timestamp, but that function be turned into a stub by not configuring NETWORK_PHY_TIMESTAMPING. Maybe a similar compile-time token is appropriate.

> >
> > -   nskb = dst->rcv(skb, dev, pt);
> > +   nskb = dst->rcv(skb, dev, pt, &ds, &source_port);
>
> I don't think this is necessary, what dst->rcv() does is actually
> properly assign skb->dev to the correct dsa slave network device, which
> has the information about the port number already in its private context.

Yes, looking in that private context seems like it'd be a better approach (and avoids having to touch all the taggers). I'll look into that further.

> > +   type = ptp_classify_raw(skb);
> > +   if (type == PTP_CLASS_NONE)
> > +           return;
>
> If we don't have a port_txtstamp option, is there even value in
> classifying this packet?

There isn't. This could also use a bypass just like the RX case.

-- brandon

^ permalink raw reply

* RE: [PATCH net-next RFC 3/9] net: dsa: mv88e6xxx: add support for GPIO configuration
From: Brandon Streiff @ 2017-09-29 15:30 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli
  Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	David S. Miller, Vivien Didelot, Richard Cochran, Erik Hons
In-Reply-To: <20170928180111.GF14940@lunn.ch>

> From: Andrew Lunn [mailto:andrew@lunn.ch]
> Sent: Thursday, September 28, 2017 1:01 PM
>
> > With the write and read acquiring and then releasing the lock
> > immediately, is no there room for this sequence to be interrupted in the
> > middle and end-up returning inconsistent reads?
> 
> The general pattern in this code is that the lock chip->reg_lock is
> taken at a higher level. That protects against other threads. The
> driver tends to do that at the highest levels, at the entry points
> into the driver. I've not yet checked this code follows the pattern
> yet. However, we have a check in the low level to ensure the lock has
> been taken. So it seems likely the lock is held.

Yes, the expectation here is that an upper layer takes the reg_lock. All the functions in ptp.c that call this function do that. If they did not, then assert_reg_lock() gets very angry. :)

Perhaps using __must_hold() and similar annotations would also help document the requirements, but we don't seem to use those in this driver today.
 
-- brandon

^ permalink raw reply

* RE: [PATCH net-next RFC 2/9] net: dsa: mv88e6xxx: expose switch time as a PTP hardware clock
From: Brandon Streiff @ 2017-09-29 15:28 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	David S. Miller, Florian Fainelli, Vivien Didelot,
	Richard Cochran, Erik Hons
In-Reply-To: <20170928165643.GB14940@lunn.ch>

> From: Andrew Lunn [mailto:andrew@lunn.ch]
> Sent: Thursday, September 28, 2017 11:57 AM
> 
> It is the MAC which is doing the time stamping, not they PHY?
> So why NETWORK_PHY_TIMESTAMPING?

NETWORK_PHY_TIMESTAMPING implies NET_PTP_CLASSIFY (which I do use) and net/core/timestamping.c (which I didn't). It probably makes more sense to just depend on NET_PTP_CLASSIFY directly.

-- brandon

^ permalink raw reply

* Re: netlink backwards compatibility in userspace tools
From: Stephen Hemminger @ 2017-09-29 15:21 UTC (permalink / raw)
  To: Jason A. Donenfeld; +Cc: Netdev, LKML, Daniel Kahn Gillmor
In-Reply-To: <CAHmME9oixZtPVdH24KJQ9NaTuf_ECAOoHwQhuA+Fy-BX+F_3dw@mail.gmail.com>

On Fri, 29 Sep 2017 12:22:42 +0200
"Jason A. Donenfeld" <Jason@zx2c4.com> wrote:

> Hi guys,
> 
> One handy aspect of Netlink is that it's backwards compatible. This
> means that you can run old userspace utilities on new kernels, even if
> the new kernel supports new features and netlink attributes. The wire
> format is stable enough that the data marshaled can be extended
> without breaking compat. Neat.
> 
> I was wondering, though, what you think the best stance is toward
> these old userspace utilities. What should they do if the kernel sends
> it netlink attributes that it does not recognize? At the moment, I'm
> doing something like this:
> 
> static void warn_unrecognized(void)
> {
>     static bool once = false;
>     if (once)
>         return;
>     once = true;
>     fprintf(stderr,
>         "Warning: this program received from your kernel one or more\n"
>         "attributes that it did not recognize. It is possible that\n"
>         "this version of wg(8) is older than your kernel. You may\n"
>         "want to update this program.\n");
> }
> 
> This seems like a somewhat sensible warning, but then I wonder about
> distributions like Debian, which has a long stable life cycle, so it
> frequently has very old tools (ancient iproute2 for example). Then,
> VPS providers have these Debian images run on top of newer kernels.
> People in this situation would undoubtedly see the above warning a lot
> and not be able to do anything about it. Not horrible, but a bit
> annoying. Is this an okay annoyance? Or is it advised to just have no
> warning at all? One idea would be to put it behind an environment
> variable flag, but I don't like too many nobs.
> 
> I'm generally wondering about attitudes toward this kind of userspace
> program behavior in response to newer kernels.
> 
> Thanks,
> Jason

I can not see a reason that such a warning is required.
Old utilities should just work fine, they just won't show or allow
setting attributes they don't understand.

Any netlink attributes that the tools do not recognize should just
be ignored.

^ permalink raw reply

* RE: [PATCH net-next RFC 2/9] net: dsa: mv88e6xxx: expose switch time as a PTP hardware clock
From: Brandon Streiff @ 2017-09-29 15:17 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	David S. Miller, Florian Fainelli, Vivien Didelot,
	Richard Cochran, Erik Hons
In-Reply-To: <20170928170329.GC14940@lunn.ch>

> From: Andrew Lunn [mailto:andrew@lunn.ch]
> Sent: Thursday, September 28, 2017 12:03 PM
> 
> > +	bool timeout = time_is_before_jiffies(chip->last_overflow_check +
> > +					      MV88E6XXX_TAI_OVERFLOW_PERIOD);
> > +
> > +	if (timeout) {
> 
> Why do you need this timeout? Do you think the kernel will call this
> more often than required?
> 
> Also, if it did call this function early, you skip the read, and
> reschedule. There is then a danger the next read is after the
> wraparound.....

That was, conceptually, a copy-paste from ixgbe_ptp.c as I was looking for how to implement the overflow accounting; that driver has a similar time_is_before_jiffies check in ixgbe_ptp_overflow_check.
 
Although now that I'm looking it over again, I'm also not certain of the need. Even if we're called more frequently than we expect, that doesn't seem to be harmful with regard to timekeeping. Hmm.

-- brandon

^ permalink raw reply

* Re: [PATCH net-next] net: bridge: add per-port group_fwd_mask with less restrictions
From: Stephen Hemminger @ 2017-09-29 15:14 UTC (permalink / raw)
  To: Nikolay Aleksandrov; +Cc: netdev, roopa, bridge
In-Reply-To: <1506517964-17479-1-git-send-email-nikolay@cumulusnetworks.com>

On Wed, 27 Sep 2017 16:12:44 +0300
Nikolay Aleksandrov <nikolay@cumulusnetworks.com> wrote:

> We need to be able to transparently forward most link-local frames via
> tunnels (e.g. vxlan, qinq). Currently the bridge's group_fwd_mask has a
> mask which restricts the forwarding of STP and LACP, but we need to be able
> to forward these over tunnels and control that forwarding on a per-port
> basis thus add a new per-port group_fwd_mask option which only disallows
> mac pause frames to be forwarded (they're always dropped anyway).
> The patch does not change the current default situation - all of the others
> are still restricted unless configured for forwarding.
> We have successfully tested this patch with LACP and STP forwarding over
> VxLAN and qinq tunnels.
> 
> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>

LACP is fine, but STP must not be forwarded if STP in user or kernel
mode is enabled.

Please update this patch or revert it.

^ permalink raw reply

* Re: [Intel-wired-lan] [PATCH net v2] i40e: Fix limit imprecise of the number of MAC/VLAN that can be added for VFs
From: Alexander Duyck @ 2017-09-29 15:04 UTC (permalink / raw)
  To: wangyunjian
  Cc: David Miller, Jeff Kirsher, Sergei Shtylyov, Netdev, caihe,
	intel-wired-lan
In-Reply-To: <34EFBCA9F01B0748BEB6B629CE643AE60C64BFF6@dggemm513-mbx.china.huawei.com>

On Fri, Sep 29, 2017 at 2:13 AM, wangyunjian <wangyunjian@huawei.com> wrote:
>
>
>> -----Original Message-----
>> From: Alexander Duyck [mailto:alexander.duyck@gmail.com]
>> Sent: Thursday, September 28, 2017 11:44 PM
>> To: wangyunjian <wangyunjian@huawei.com>
>> Cc: David Miller <davem@davemloft.net>; Jeff Kirsher
>> <jeffrey.t.kirsher@intel.com>; Sergei Shtylyov
>> <sergei.shtylyov@cogentembedded.com>; Netdev
>> <netdev@vger.kernel.org>; caihe <caihe@huawei.com>; intel-wired-lan
>> <intel-wired-lan@lists.osuosl.org>
>> Subject: Re: [Intel-wired-lan] [PATCH net v2] i40e: Fix limit imprecise of the
>> number of MAC/VLAN that can be added for VFs
>>
>> On Wed, Sep 27, 2017 at 7:01 PM, w00273186 <wangyunjian@huawei.com>
>> wrote:
>> > From: Yunjian Wang <wangyunjian@huawei.com>
>> >
>> > Now it doesn't limit the number of MAC/VLAN strictly. When there is more
>> > elements in the virtchnl MAC/VLAN list, it can still add successfully.
>>
>> You could still add but should you. I'm not clear from this patch
>> description what this is supposed to be addressing. If you enable the
>> "trust" flag for a VF via the "ip link set dev <iface> vf <vfnum>
>> trust on" it can make use of any resources on the device, but without
>> that there is an upper limit that is supposed to be enforced to
>> prevent the VF from making use of an excessive amount of resources.
>> That is what is being enforced by the code you are moving out of the
>> way below.
>
> I don't enable the "trust" flag for a VF. But this script can successfully add
> MACs more than I40E_VC_MAX_MAC_ADDR_PER_VF(12) in VM. It has
> same problem with VLAN.
>
> Test script:
>
> for((i=10;i<50;i++))
> do
>     ipmaddr add 01:00:5e:01:02:$i  dev eth0
> done
>
> for ((i=1;i<40;i++))
> do
>     ip link add link eth0 name eth0.$i type vlan id $i
> done
>

Okay, thanks for the info. I can see if we can address the issue in a
way that prevents us from adding the filters to the hardware before we
return the result indicating if we can support it or not.

- Alex

^ permalink raw reply

* [PATCH][net-next] net_sched: remove redundant assignment to ret
From: Colin King @ 2017-09-29 14:01 UTC (permalink / raw)
  To: Jamal Hadi Salim, Cong Wang, Jiri Pirko, David S . Miller, netdev
  Cc: kernel-janitors, linux-kernel

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

The assignment of -EINVAL to variable ret is redundant as it
is being overwritten on the following error exit paths or
to the return value from the following call to basic_set_parms.
Fix this up by removing it. Cleans up clang warning message:

net/sched/cls_basic.c:185:2: warning: Value stored to 'err' is never read

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 net/sched/cls_basic.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/net/sched/cls_basic.c b/net/sched/cls_basic.c
index cfeb6f158566..700b345b07f9 100644
--- a/net/sched/cls_basic.c
+++ b/net/sched/cls_basic.c
@@ -182,7 +182,6 @@ static int basic_change(struct net *net, struct sk_buff *in_skb,
 	if (err < 0)
 		goto errout;
 
-	err = -EINVAL;
 	if (handle) {
 		fnew->handle = handle;
 		if (!fold) {
-- 
2.14.1

^ permalink raw reply related

* RE: [RESEND PATCH 2/6] staging: fsl-dpaa2/ethsw: Add Freescale DPAA2 Ethernet Switch driver
From: Razvan Stefanescu @ 2017-09-29 13:59 UTC (permalink / raw)
  To: Bogdan Purcareata, gregkh@linuxfoundation.org
  Cc: devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org,
	netdev@vger.kernel.org, agraf@suse.de, arnd@arndb.de,
	Alexandru Marginean, Ruxandra Ioana Radulescu, Laurentiu Tudor,
	stuyoder@gmail.com
In-Reply-To: <DB5PR04MB12400025DF190790B4792B50EA7E0@DB5PR04MB1240.eurprd04.prod.outlook.com>



> -----Original Message-----
> From: Bogdan Purcareata
> Sent: Friday, September 29, 2017 16:36
> To: Razvan Stefanescu <razvan.stefanescu@nxp.com>;
> gregkh@linuxfoundation.org
> Cc: devel@driverdev.osuosl.org; linux-kernel@vger.kernel.org;
> netdev@vger.kernel.org; agraf@suse.de; arnd@arndb.de; Alexandru Marginean
> <alexandru.marginean@nxp.com>; Ruxandra Ioana Radulescu
> <ruxandra.radulescu@nxp.com>; Laurentiu Tudor <laurentiu.tudor@nxp.com>;
> stuyoder@gmail.com
> Subject: RE: [RESEND PATCH 2/6] staging: fsl-dpaa2/ethsw: Add Freescale DPAA2
> Ethernet Switch driver
> 
> > Introduce the DPAA2 Ethernet Switch driver, which manages Datapath Switch
> > (DPSW) objects discovered on the MC bus.
> >
> > Suggested-by: Alexandru Marginean <alexandru.marginean@nxp.com>
> > Signed-off-by: Razvan Stefanescu <razvan.stefanescu@nxp.com>
> > ---
> >  drivers/staging/fsl-dpaa2/ethsw/Makefile |    2 +-
> >  drivers/staging/fsl-dpaa2/ethsw/ethsw.c  | 1523
> ++++++++++++++++++++++++++++++
> >  drivers/staging/fsl-dpaa2/ethsw/ethsw.h  |   88 ++
> >  3 files changed, 1612 insertions(+), 1 deletion(-)
> >  create mode 100644 drivers/staging/fsl-dpaa2/ethsw/ethsw.c
> >  create mode 100644 drivers/staging/fsl-dpaa2/ethsw/ethsw.h
> >
> > diff --git a/drivers/staging/fsl-dpaa2/ethsw/Makefile b/drivers/staging/fsl-
> > dpaa2/ethsw/Makefile
> > index db137f7..a6d72d1 100644
> > --- a/drivers/staging/fsl-dpaa2/ethsw/Makefile
> > +++ b/drivers/staging/fsl-dpaa2/ethsw/Makefile
> > @@ -4,4 +4,4 @@
> >
> >  obj-$(CONFIG_FSL_DPAA2_ETHSW) += dpaa2-ethsw.o
> >
> > -dpaa2-ethsw-objs := dpsw.o
> > +dpaa2-ethsw-objs := ethsw.o dpsw.o
> > diff --git a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c b/drivers/staging/fsl-
> > dpaa2/ethsw/ethsw.c
> > new file mode 100644
> > index 0000000..ae86078
> > --- /dev/null
> > +++ b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
> > @@ -0,0 +1,1523 @@
> > +/* Copyright 2014-2016 Freescale Semiconductor Inc.
> > + * Copyright 2017 NXP
> > + *
> > + * Redistribution and use in source and binary forms, with or without
> > + * modification, are permitted provided that the following conditions are met:
> > + *     * Redistributions of source code must retain the above copyright
> > + *	 notice, this list of conditions and the following disclaimer.
> > + *     * Redistributions in binary form must reproduce the above copyright
> > + *	 notice, this list of conditions and the following disclaimer in the
> > + *	 documentation and/or other materials provided with the distribution.
> > + *     * Neither the name of the above-listed copyright holders nor the
> > + *	 names of any contributors may be used to endorse or promote products
> > + *	 derived from this software without specific prior written permission.
> > + *
> > + *
> > + * ALTERNATIVELY, this software may be distributed under the terms of the
> > + * GNU General Public License ("GPL") as published by the Free Software
> > + * Foundation, either version 2 of that License or (at your option) any
> > + * later version.
> > + *
> > + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
> CONTRIBUTORS "AS IS"
> > + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> LIMITED TO, THE
> > + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
> PARTICULAR PURPOSE
> > + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDERS OR
> CONTRIBUTORS BE
> > + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
> > + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
> PROCUREMENT OF
> > + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
> BUSINESS
> > + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY,
> WHETHER IN
> > + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR
> OTHERWISE)
> > + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
> ADVISED OF THE
> > + * POSSIBILITY OF SUCH DAMAGE.
> > + */
> > +
> > +#include <linux/module.h>
> > +
> > +#include <linux/interrupt.h>
> > +#include <linux/msi.h>
> > +#include <linux/kthread.h>
> > +#include <linux/workqueue.h>
> > +
> > +#include "../../fsl-mc/include/mc.h"
> > +
> > +#include "ethsw.h"
> > +
> > +static struct workqueue_struct *ethsw_owq;
> > +
> > +/* Minimal supported DPSW version */
> > +#define DPSW_MIN_VER_MAJOR		8
> > +#define DPSW_MIN_VER_MINOR		0
> > +
> > +#define DEFAULT_VLAN_ID			1
> > +
> > +static int ethsw_add_vlan(struct ethsw_core *ethsw, u16 vid)
> > +{
> > +	int err;
> > +
> > +	struct dpsw_vlan_cfg	vcfg = {
> > +		.fdb_id = 0,
> > +	};
> > +
> > +	if (ethsw->vlans[vid]) {
> > +		dev_err(ethsw->dev, "VLAN already configured\n");
> > +		return -EEXIST;
> > +	}
> > +
> > +	err = dpsw_vlan_add(ethsw->mc_io, 0,
> > +			    ethsw->dpsw_handle, vid, &vcfg);
> > +	if (err) {
> > +		dev_err(ethsw->dev, "dpsw_vlan_add err %d\n", err);
> > +		return err;
> > +	}
> > +	ethsw->vlans[vid] = ETHSW_VLAN_MEMBER;/
> > +
> > +	return 0;
> > +}
> > +
> > +static int ethsw_port_add_vlan(struct ethsw_port_priv *port_priv,
> > +			       u16 vid, u16 flags)
> > +{
> > +	struct ethsw_core *ethsw = port_priv->ethsw_data;
> > +	struct net_device *netdev = port_priv->netdev;
> > +	struct dpsw_vlan_if_cfg vcfg;
> > +	bool is_oper;
> > +	int err, err2;
> 
> Mild suggestion - s/err2/ret/, just because it sounds better, at least to me (same
> for similar situations in the rest of the file).
> 
Thank you for your suggestion. I will make the change and send v2.
> > +
> > +	if (port_priv->vlans[vid]) {
> > +		netdev_warn(netdev, "VLAN %d already configured\n", vid);
> > +		return -EEXIST;
> > +	}
> > +
> > +	vcfg.num_ifs = 1;
> > +	vcfg.if_id[0] = port_priv->idx;
> > +	err = dpsw_vlan_add_if(ethsw->mc_io, 0, ethsw->dpsw_handle, vid,
> &vcfg);
> > +	if (err) {
> > +		netdev_err(netdev, "dpsw_vlan_add_if err %d\n", err);
> > +		return err;
> > +	}
> > +
> > +	port_priv->vlans[vid] = ETHSW_VLAN_MEMBER;
> > +
> > +	if (flags & BRIDGE_VLAN_INFO_UNTAGGED) {
> > +		err = dpsw_vlan_add_if_untagged(ethsw->mc_io, 0,
> > +						ethsw->dpsw_handle,
> > +						vid, &vcfg);
> > +		if (err) {
> > +			netdev_err(netdev,
> > +				   "dpsw_vlan_add_if_untagged err %d\n", err);
> > +			return err;
> > +		}
> > +		port_priv->vlans[vid] |= ETHSW_VLAN_UNTAGGED;
> > +	}
> > +
> > +	if (flags & BRIDGE_VLAN_INFO_PVID) {
> > +		struct dpsw_tci_cfg tci_cfg = {
> > +			.pcp = 0,
> > +			.dei = 0,
> > +			.vlan_id = vid,
> > +		};
> > +
> > +		/* Interface needs to be down to change PVID */
> > +		is_oper = netif_oper_up(netdev);
> > +		if (is_oper) {
> > +			err = dpsw_if_disable(ethsw->mc_io, 0,
> > +					      ethsw->dpsw_handle,
> > +					      port_priv->idx);
> > +			if (err) {
> > +				netdev_err(netdev,
> > +					   "dpsw_if_disable err %d\n", err);
> > +				return err;
> > +			}
> > +		}
> > +
> > +		err = dpsw_if_set_tci(ethsw->mc_io, 0, ethsw->dpsw_handle,
> > +				      port_priv->idx, &tci_cfg);
> > +		if (!err) {
> > +			/* Delete previous PVID info and mark the new one */
> > +			if (port_priv->pvid)
> > +				port_priv->vlans[port_priv->pvid]
> > +						 ^= ETHSW_VLAN_PVID;
> 
> Can it be " &= ~ETHSW_VLAN_PVID" ? Are there other implications?
> 
You are right. Flag must be un-set. Will update in v2, along with the other observations.
Thank you,
Razvan S.
> > +
> > +			port_priv->vlans[vid] |= ETHSW_VLAN_PVID;
> > +			port_priv->pvid = vid;
> > +		} else {
> > +			netdev_err(netdev, "dpsw_if_set_tci err %d\n", err);
> > +		}
> > +
> > +		if (is_oper) {
> > +			err2 = dpsw_if_enable(ethsw->mc_io, 0,
> > +					      ethsw->dpsw_handle,
> > +					      port_priv->idx);
> > +			if (err2) {
> > +				netdev_err(netdev,
> > +					   "dpsw_if_enable err %d\n", err2);
> > +				return err2;
> > +			}
> > +		}
> > +	}
> > +
> > +	return err;
> > +}
> > +
> > +static int ethsw_set_learning(struct ethsw_core *ethsw, u8 flag)
> > +{
> > +	enum dpsw_fdb_learning_mode learn_mode;
> > +	int err;
> > +
> > +	if (flag)
> > +		learn_mode = DPSW_FDB_LEARNING_MODE_HW;
> > +	else
> > +		learn_mode = DPSW_FDB_LEARNING_MODE_DIS;
> > +
> > +	err = dpsw_fdb_set_learning_mode(ethsw->mc_io, 0, ethsw-
> >dpsw_handle, 0,
> > +					 learn_mode);
> > +	if (err) {
> > +		dev_err(ethsw->dev, "dpsw_fdb_set_learning_mode err %d\n",
> err);
> > +		return err;
> > +	}
> > +	ethsw->learning = !!flag;
> > +
> > +	return 0;
> > +}
> > +
> > +static int ethsw_port_set_flood(struct ethsw_port_priv *port_priv, u8 flag)
> > +{
> > +	int err;
> > +
> > +	err = dpsw_if_set_flooding(port_priv->ethsw_data->mc_io, 0,
> > +				   port_priv->ethsw_data->dpsw_handle,
> > +				   port_priv->idx, (int)flag);
> 
> Why is this cast necessary? Can't the API be reworked to use u8 (or, better yet,
> bool) instead of int?
> 
> > +	if (err) {
> > +		netdev_err(port_priv->netdev,
> > +			   "dpsw_fdb_set_learning_mode err %d\n", err);
> > +		return err;
> > +	}
> > +	port_priv->flood = !!flag;
> > +
> > +	return 0;
> > +}
> > +
> > +static int ethsw_port_set_stp_state(struct ethsw_port_priv *port_priv, u8
> > state)
> > +{
> > +	struct dpsw_stp_cfg stp_cfg = {
> > +		.vlan_id = DEFAULT_VLAN_ID,
> > +		.state = state,
> > +	};
> > +	int err;
> > +
> > +	if (!netif_oper_up(port_priv->netdev) || state == port_priv->stp_state)
> > +		return 0;	/* Nothing to do */
> > +
> > +	err = dpsw_if_set_stp(port_priv->ethsw_data->mc_io, 0,
> > +			      port_priv->ethsw_data->dpsw_handle,
> > +			      port_priv->idx, &stp_cfg);
> > +	if (err) {
> > +		netdev_err(port_priv->netdev,
> > +			   "dpsw_if_set_stp err %d\n", err);
> > +		return err;
> > +	}
> > +
> > +	port_priv->stp_state = state;
> > +
> > +	return 0;
> > +}
> > +
> > +static int ethsw_dellink_switch(struct ethsw_core *ethsw, u16 vid)
> > +{
> > +	struct ethsw_port_priv *ppriv_local = NULL;
> > +	int i, err;
> > +
> > +	if (!ethsw->vlans[vid])
> > +		return -ENOENT;
> > +
> > +	err = dpsw_vlan_remove(ethsw->mc_io, 0, ethsw->dpsw_handle, vid);
> > +	if (err) {
> > +		dev_err(ethsw->dev, "dpsw_vlan_remove err %d\n", err);
> > +		return err;
> > +	}
> > +	ethsw->vlans[vid] = 0;
> > +
> > +	for (i = 0; i < ethsw->sw_attr.num_ifs; i++) {
> > +		ppriv_local = ethsw->ports[i];
> > +		ppriv_local->vlans[vid] = 0;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int ethsw_port_fdb_add_uc(struct ethsw_port_priv *port_priv,
> > +				 const unsigned char *addr)
> > +{
> > +	struct dpsw_fdb_unicast_cfg entry = {0};
> > +	int err;
> > +
> > +	entry.if_egress = port_priv->idx;
> > +	entry.type = DPSW_FDB_ENTRY_STATIC;
> > +	ether_addr_copy(entry.mac_addr, addr);
> > +
> > +	err = dpsw_fdb_add_unicast(port_priv->ethsw_data->mc_io, 0,
> > +				   port_priv->ethsw_data->dpsw_handle,
> > +				   0, &entry);
> > +	if (err)
> > +		netdev_err(port_priv->netdev,
> > +			   "dpsw_fdb_add_unicast err %d\n", err);
> > +	return err;
> > +}
> > +
> > +static int ethsw_port_fdb_del_uc(struct ethsw_port_priv *port_priv,
> > +				 const unsigned char *addr)
> > +{
> > +	struct dpsw_fdb_unicast_cfg entry = {0};
> > +	int err;
> > +
> > +	entry.if_egress = port_priv->idx;
> > +	entry.type = DPSW_FDB_ENTRY_STATIC;
> > +	ether_addr_copy(entry.mac_addr, addr);
> > +
> > +	err = dpsw_fdb_remove_unicast(port_priv->ethsw_data->mc_io, 0,
> > +				      port_priv->ethsw_data->dpsw_handle,
> > +				      0, &entry);
> > +	if (err)
> > +		netdev_err(port_priv->netdev,
> > +			   "dpsw_fdb_remove_unicast err %d\n", err);
> > +	return err;
> > +}
> > +
> > +static int ethsw_port_fdb_add_mc(struct ethsw_port_priv *port_priv,
> > +				 const unsigned char *addr)
> > +{
> > +	struct dpsw_fdb_multicast_cfg entry = {0};
> > +	int err;
> > +
> > +	ether_addr_copy(entry.mac_addr, addr);
> > +	entry.type = DPSW_FDB_ENTRY_STATIC;
> > +	entry.num_ifs = 1;
> > +	entry.if_id[0] = port_priv->idx;
> > +
> > +	err = dpsw_fdb_add_multicast(port_priv->ethsw_data->mc_io, 0,
> > +				     port_priv->ethsw_data->dpsw_handle,
> > +				     0, &entry);
> > +	if (err)
> > +		netdev_err(port_priv->netdev, "dpsw_fdb_add_multicast err
> %d\n",
> > +			   err);
> > +	return err;
> > +}
> > +
> > +static int ethsw_port_fdb_del_mc(struct ethsw_port_priv *port_priv,
> > +				 const unsigned char *addr)
> > +{
> > +	struct dpsw_fdb_multicast_cfg entry = {0};
> > +	int err;
> > +
> > +	ether_addr_copy(entry.mac_addr, addr);
> > +	entry.type = DPSW_FDB_ENTRY_STATIC;
> > +	entry.num_ifs = 1;
> > +	entry.if_id[0] = port_priv->idx;
> > +
> > +	err = dpsw_fdb_remove_multicast(port_priv->ethsw_data->mc_io, 0,
> > +					port_priv->ethsw_data->dpsw_handle,
> > +					0, &entry);
> > +	if (err)
> > +		netdev_err(port_priv->netdev,
> > +			   "dpsw_fdb_remove_multicast err %d\n", err);
> > +	return err;
> > +}
> > +
> > +static void port_get_stats(struct net_device *netdev,
> > +			   struct rtnl_link_stats64 *stats)
> > +{
> > +	struct ethsw_port_priv *port_priv = netdev_priv(netdev);
> > +	u64 tmp;
> > +	int err;
> > +
> > +	err = dpsw_if_get_counter(port_priv->ethsw_data->mc_io, 0,
> > +				  port_priv->ethsw_data->dpsw_handle,
> > +				  port_priv->idx,
> > +				  DPSW_CNT_ING_FRAME, &stats->rx_packets);
> > +	if (err)
> > +		goto error;
> > +
> > +	err = dpsw_if_get_counter(port_priv->ethsw_data->mc_io, 0,
> > +				  port_priv->ethsw_data->dpsw_handle,
> > +				  port_priv->idx,
> > +				  DPSW_CNT_EGR_FRAME, &stats->tx_packets);
> > +	if (err)
> > +		goto error;
> > +
> > +	err = dpsw_if_get_counter(port_priv->ethsw_data->mc_io, 0,
> > +				  port_priv->ethsw_data->dpsw_handle,
> > +				  port_priv->idx,
> > +				  DPSW_CNT_ING_BYTE, &stats->rx_bytes);
> > +	if (err)
> > +		goto error;
> > +
> > +	err = dpsw_if_get_counter(port_priv->ethsw_data->mc_io, 0,
> > +				  port_priv->ethsw_data->dpsw_handle,
> > +				  port_priv->idx,
> > +				  DPSW_CNT_EGR_BYTE, &stats->tx_bytes);
> > +	if (err)
> > +		goto error;
> > +
> > +	err = dpsw_if_get_counter(port_priv->ethsw_data->mc_io, 0,
> > +				  port_priv->ethsw_data->dpsw_handle,
> > +				  port_priv->idx,
> > +				  DPSW_CNT_ING_FRAME_DISCARD,
> > +				  &stats->rx_dropped);
> > +	if (err)
> > +		goto error;
> > +
> > +	err = dpsw_if_get_counter(port_priv->ethsw_data->mc_io, 0,
> > +				  port_priv->ethsw_data->dpsw_handle,
> > +				  port_priv->idx,
> > +				  DPSW_CNT_ING_FLTR_FRAME,
> > +				  &tmp);
> > +	if (err)
> > +		goto error;
> > +	stats->rx_dropped += tmp;
> > +
> > +	err = dpsw_if_get_counter(port_priv->ethsw_data->mc_io, 0,
> > +				  port_priv->ethsw_data->dpsw_handle,
> > +				  port_priv->idx,
> > +				  DPSW_CNT_EGR_FRAME_DISCARD,
> > +				  &stats->tx_dropped);
> > +	if (err)
> > +		goto error;
> > +
> > +	return;
> > +
> > +error:
> > +	netdev_err(netdev, "dpsw_if_get_counter err %d\n", err);
> > +}
> > +
> > +static bool port_has_offload_stats(const struct net_device *netdev,
> > +				   int attr_id)
> > +{
> > +	return (attr_id == IFLA_OFFLOAD_XSTATS_CPU_HIT);
> > +}
> > +
> > +static int port_get_offload_stats(int attr_id,
> > +				  const struct net_device *netdev,
> > +				  void *sp)
> > +{
> > +	switch (attr_id) {
> > +	case IFLA_OFFLOAD_XSTATS_CPU_HIT:
> > +		port_get_stats((struct net_device *)netdev, sp);
> > +		return 0;
> > +	}
> > +
> > +	return -EINVAL;
> > +}
> > +
> > +static int port_change_mtu(struct net_device *netdev, int mtu)
> > +{
> > +	struct ethsw_port_priv *port_priv = netdev_priv(netdev);
> > +	int err;
> > +
> > +	err = dpsw_if_set_max_frame_length(port_priv->ethsw_data->mc_io,
> > +					   0,
> > +					   port_priv->ethsw_data->dpsw_handle,
> > +					   port_priv->idx,
> > +					   (u16)ETHSW_L2_MAX_FRM(mtu));
> > +	if (err) {
> > +		netdev_err(netdev,
> > +			   "dpsw_if_set_max_frame_length() err %d\n", err);
> > +		return err;
> > +	}
> > +
> > +	netdev->mtu = mtu;
> > +	return 0;
> > +}
> > +
> > +static int port_carrier_state_sync(struct net_device *netdev)
> > +{
> > +	struct ethsw_port_priv *port_priv = netdev_priv(netdev);
> > +	struct dpsw_link_state state;
> > +	int err;
> > +
> > +	err = dpsw_if_get_link_state(port_priv->ethsw_data->mc_io, 0,
> > +				     port_priv->ethsw_data->dpsw_handle,
> > +				     port_priv->idx, &state);
> > +	if (err) {
> > +		netdev_err(netdev, "dpsw_if_get_link_state() err %d\n", err);
> > +		return err;
> > +	}
> > +
> > +	WARN_ONCE(state.up > 1, "Garbage read into link_state");
> > +
> > +	if (state.up != port_priv->link_state) {
> > +		if (state.up)
> > +			netif_carrier_on(netdev);
> > +		else
> > +			netif_carrier_off(netdev);
> > +		port_priv->link_state = state.up;
> > +	}
> > +	return 0;
> > +}
> > +
> > +static int port_open(struct net_device *netdev)
> > +{
> > +	struct ethsw_port_priv *port_priv = netdev_priv(netdev);
> > +	int err;
> > +
> > +	/* No need to allow Tx as control interface is disabled */
> > +	netif_tx_stop_all_queues(netdev);
> > +
> > +	err = dpsw_if_enable(port_priv->ethsw_data->mc_io, 0,
> > +			     port_priv->ethsw_data->dpsw_handle,
> > +			     port_priv->idx);
> > +	if (err) {
> > +		netdev_err(netdev, "dpsw_if_enable err %d\n", err);
> > +		return err;
> > +	}
> > +
> > +	/* sync carrier state */
> > +	err = port_carrier_state_sync(netdev);
> > +	if (err) {
> > +		netdev_err(netdev,
> > +			   "port_carrier_state_sync err %d\n", err);
> > +		goto err_carrier_sync;
> > +	}
> > +
> > +	return 0;
> > +
> > +err_carrier_sync:
> > +	dpsw_if_disable(port_priv->ethsw_data->mc_io, 0,
> > +			port_priv->ethsw_data->dpsw_handle,
> > +			port_priv->idx);
> > +	return err;
> > +}
> > +
> > +static int port_stop(struct net_device *netdev)
> > +{
> > +	struct ethsw_port_priv *port_priv = netdev_priv(netdev);
> > +	int err;
> > +
> > +	err = dpsw_if_disable(port_priv->ethsw_data->mc_io, 0,
> > +			      port_priv->ethsw_data->dpsw_handle,
> > +			      port_priv->idx);
> > +	if (err) {
> > +		netdev_err(netdev, "dpsw_if_disable err %d\n", err);
> > +		return err;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static netdev_tx_t port_dropframe(struct sk_buff *skb,
> > +				  struct net_device *netdev)
> > +{
> > +	/* we don't support I/O for now, drop the frame */
> > +	dev_kfree_skb_any(skb);
> > +
> > +	return NETDEV_TX_OK;
> > +}
> > +
> > +static const struct net_device_ops ethsw_port_ops = {
> > +	.ndo_open		= port_open,
> > +	.ndo_stop		= port_stop,
> > +
> > +	.ndo_set_mac_address	= eth_mac_addr,
> > +	.ndo_change_mtu		= port_change_mtu,
> > +	.ndo_has_offload_stats	= port_has_offload_stats,
> > +	.ndo_get_offload_stats	= port_get_offload_stats,
> > +
> > +	.ndo_start_xmit		= port_dropframe,
> > +};
> > +
> > +static void ethsw_links_state_update(struct ethsw_core *ethsw)
> > +{
> > +	int i;
> > +
> > +	for (i = 0; i < ethsw->sw_attr.num_ifs; i++)
> > +		port_carrier_state_sync(ethsw->ports[i]->netdev);
> > +}
> > +
> > +static irqreturn_t ethsw_irq0_handler(int irq_num, void *arg)
> > +{
> > +	return IRQ_WAKE_THREAD;
> > +}
> > +
> > +static irqreturn_t ethsw_irq0_handler_thread(int irq_num, void *arg)
> > +{
> > +	struct device *dev = (struct device *)arg;
> > +	struct ethsw_core *ethsw = dev_get_drvdata(dev);
> > +
> > +	/* Mask the events and the if_id reserved bits to be cleared on read */
> > +	u32 status = DPSW_IRQ_EVENT_LINK_CHANGED | 0xFFFF0000;
> > +	int err;
> > +
> > +	err = dpsw_get_irq_status(ethsw->mc_io, 0, ethsw->dpsw_handle,
> > +				  DPSW_IRQ_INDEX_IF, &status);
> > +	if (err) {
> > +		dev_err(dev, "Can't get irq status (err %d)", err);
> > +
> > +		err = dpsw_clear_irq_status(ethsw->mc_io, 0, ethsw-
> >dpsw_handle,
> > +					    DPSW_IRQ_INDEX_IF, 0xFFFFFFFF);
> > +		if (err)
> > +			dev_err(dev, "Can't clear irq status (err %d)", err);
> > +		goto out;
> > +	}
> > +
> > +	if (status & DPSW_IRQ_EVENT_LINK_CHANGED)
> > +		ethsw_links_state_update(ethsw);
> > +
> > +out:
> > +	return IRQ_HANDLED;
> > +}
> > +
> > +static int ethsw_setup_irqs(struct fsl_mc_device *sw_dev)
> > +{
> > +	struct device *dev = &sw_dev->dev;
> > +	struct ethsw_core *ethsw = dev_get_drvdata(dev);
> > +	u32 mask = DPSW_IRQ_EVENT_LINK_CHANGED;
> > +	struct fsl_mc_device_irq *irq;
> > +	int err;
> > +
> > +	err = fsl_mc_allocate_irqs(sw_dev);
> > +	if (err) {
> > +		dev_err(dev, "MC irqs allocation failed\n");
> > +		return err;
> > +	}
> > +
> > +	if (WARN_ON(sw_dev->obj_desc.irq_count != DPSW_IRQ_NUM)) {
> > +		err = -EINVAL;
> > +		goto free_irq;
> > +	}
> > +
> > +	err = dpsw_set_irq_enable(ethsw->mc_io, 0, ethsw->dpsw_handle,
> > +				  DPSW_IRQ_INDEX_IF, 0);
> > +	if (err) {
> > +		dev_err(dev, "dpsw_set_irq_enable err %d\n", err);
> > +		goto free_irq;
> > +	}
> > +
> > +	irq = sw_dev->irqs[DPSW_IRQ_INDEX_IF];
> > +
> > +	err = devm_request_threaded_irq(dev, irq->msi_desc->irq,
> > +					ethsw_irq0_handler,
> > +					ethsw_irq0_handler_thread,
> > +					IRQF_NO_SUSPEND | IRQF_ONESHOT,
> > +					dev_name(dev), dev);
> > +	if (err) {
> > +		dev_err(dev, "devm_request_threaded_irq(): %d", err);
> > +		goto free_irq;
> > +	}
> > +
> > +	err = dpsw_set_irq_mask(ethsw->mc_io, 0, ethsw->dpsw_handle,
> > +				DPSW_IRQ_INDEX_IF, mask);
> > +	if (err) {
> > +		dev_err(dev, "dpsw_set_irq_mask(): %d", err);
> > +		goto free_devm_irq;
> > +	}
> > +
> > +	err = dpsw_set_irq_enable(ethsw->mc_io, 0, ethsw->dpsw_handle,
> > +				  DPSW_IRQ_INDEX_IF, 1);
> > +	if (err) {
> > +		dev_err(dev, "dpsw_set_irq_enable(): %d", err);
> > +		goto free_devm_irq;
> > +	}
> > +
> > +	return 0;
> > +
> > +free_devm_irq:
> > +	devm_free_irq(dev, irq->msi_desc->irq, dev);
> > +free_irq:
> > +	fsl_mc_free_irqs(sw_dev);
> > +	return err;
> > +}
> > +
> > +static void ethsw_teardown_irqs(struct fsl_mc_device *sw_dev)
> > +{
> > +	struct device *dev = &sw_dev->dev;
> > +	struct ethsw_core *ethsw = dev_get_drvdata(dev);
> > +	struct fsl_mc_device_irq *irq;
> > +
> > +	irq = sw_dev->irqs[DPSW_IRQ_INDEX_IF];
> > +	dpsw_set_irq_enable(ethsw->mc_io, 0, ethsw->dpsw_handle,
> > +			    DPSW_IRQ_INDEX_IF, 0);
> 
> You can still print an error message here, in case something goes wrong.
> 
> > +	fsl_mc_free_irqs(sw_dev);
> > +}
> > +
> > +static int swdev_port_attr_get(struct net_device *netdev,
> > +			       struct switchdev_attr *attr)
> > +{
> > +	struct ethsw_port_priv *port_priv = netdev_priv(netdev);
> > +
> > +	switch (attr->id) {
> > +	case SWITCHDEV_ATTR_ID_PORT_PARENT_ID:
> > +		attr->u.ppid.id_len = 1;
> > +		attr->u.ppid.id[0] = port_priv->ethsw_data->dev_id;
> > +		break;
> > +	case SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS:
> > +		attr->u.brport_flags =
> > +			(port_priv->ethsw_data->learning ? BR_LEARNING : 0) |
> > +			(port_priv->flood ? BR_FLOOD : 0);
> > +		break;
> > +	case SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS_SUPPORT:
> > +		attr->u.brport_flags_support = BR_LEARNING | BR_FLOOD;
> > +		break;
> > +	default:
> > +		return -EOPNOTSUPP;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int port_attr_stp_state_set(struct net_device *netdev,
> > +				   struct switchdev_trans *trans,
> > +				   u8 state)
> > +{
> > +	struct ethsw_port_priv *port_priv = netdev_priv(netdev);
> > +
> > +	if (switchdev_trans_ph_prepare(trans))
> > +		return 0;
> > +
> > +	return ethsw_port_set_stp_state(port_priv, state);
> > +}
> > +
> > +static int port_attr_br_flags_set(struct net_device *netdev,
> > +				  struct switchdev_trans *trans,
> > +				  unsigned long flags)
> > +{
> > +	struct ethsw_port_priv *port_priv = netdev_priv(netdev);
> > +	int err = 0;
> > +
> > +	if (switchdev_trans_ph_prepare(trans))
> > +		return 0;
> > +
> > +	/* Learning is enabled per switch */
> > +	err = ethsw_set_learning(port_priv->ethsw_data, flags &
> BR_LEARNING);
> > +	if (err)
> > +		goto exit;
> > +
> > +	err = ethsw_port_set_flood(port_priv, flags & BR_FLOOD);
> > +
> > +exit:
> > +	return err;
> > +}
> > +
> > +static int swdev_port_attr_set(struct net_device *netdev,
> > +			       const struct switchdev_attr *attr,
> > +			       struct switchdev_trans *trans)
> > +{
> > +	int err = 0;
> > +
> > +	switch (attr->id) {
> > +	case SWITCHDEV_ATTR_ID_PORT_STP_STATE:
> > +		err = port_attr_stp_state_set(netdev, trans,
> > +					      attr->u.stp_state);
> > +		break;
> > +	case SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS:
> > +		err = port_attr_br_flags_set(netdev, trans,
> > +					     attr->u.brport_flags);
> > +		break;
> > +	case SWITCHDEV_ATTR_ID_BRIDGE_VLAN_FILTERING:
> > +		/* VLANs are supported by default  */
> > +		break;
> > +	default:
> > +		err = -EOPNOTSUPP;
> > +		break;
> > +	}
> > +
> > +	return err;
> > +}
> > +
> > +static int port_vlans_add(struct net_device *netdev,
> > +			  const struct switchdev_obj_port_vlan *vlan,
> > +			  struct switchdev_trans *trans)
> > +{
> > +	struct ethsw_port_priv *port_priv = netdev_priv(netdev);
> > +	int vid, err;
> > +
> > +	if (switchdev_trans_ph_prepare(trans))
> > +		return 0;
> > +
> > +	for (vid = vlan->vid_begin; vid <= vlan->vid_end; vid++) {
> > +		if (!port_priv->ethsw_data->vlans[vid]) {
> > +			/* this is a new VLAN */
> > +			err = ethsw_add_vlan(port_priv->ethsw_data, vid);
> > +			if (err)
> > +				return err;
> > +
> > +			port_priv->ethsw_data->vlans[vid] |=
> ETHSW_VLAN_GLOBAL;
> > +		}
> > +		err = ethsw_port_add_vlan(port_priv, vid, vlan->flags);
> > +		if (err)
> > +			break;
> > +	}
> > +
> > +	return err;
> > +}
> > +
> > +static int port_lookup_address(struct net_device *netdev, int is_uc,
> > +			       const unsigned char *addr)
> > +{
> > +	struct netdev_hw_addr_list *list = (is_uc) ? &netdev->uc : &netdev->mc;
> > +	struct netdev_hw_addr *ha;
> > +
> > +	netif_addr_lock_bh(netdev);
> > +	list_for_each_entry(ha, &list->list, list) {
> > +		if (ether_addr_equal(ha->addr, addr)) {
> > +			netif_addr_unlock_bh(netdev);
> > +			return 1;
> > +		}
> > +	}
> > +	netif_addr_unlock_bh(netdev);
> > +	return 0;
> > +}
> > +
> > +static int port_mdb_add(struct net_device *netdev,
> > +			const struct switchdev_obj_port_mdb *mdb,
> > +			struct switchdev_trans *trans)
> > +{
> > +	struct ethsw_port_priv *port_priv = netdev_priv(netdev);
> > +	int err;
> > +
> > +	if (switchdev_trans_ph_prepare(trans))
> > +		return 0;
> > +
> > +	/* Check if address is already set on this port */
> > +	if (port_lookup_address(netdev, 0, mdb->addr))
> > +		return -EEXIST;
> > +
> > +	err = ethsw_port_fdb_add_mc(port_priv, mdb->addr);
> > +	if (err)
> > +		return err;
> > +
> > +	err = dev_mc_add(netdev, mdb->addr);
> > +	if (err)
> > +		netdev_err(netdev, "dev_mc_add err %d\n", err);
> 
> In the error case, shouldn't there be a "ethsw_port_fdb_del_mc" ?
> 
> > +
> > +	return err;
> > +}
> > +
> > +static int swdev_port_obj_add(struct net_device *netdev,
> > +			      const struct switchdev_obj *obj,
> > +			      struct switchdev_trans *trans)
> > +{
> > +	int err;
> > +
> > +	switch (obj->id) {
> > +	case SWITCHDEV_OBJ_ID_PORT_VLAN:
> > +		err = port_vlans_add(netdev,
> > +				     SWITCHDEV_OBJ_PORT_VLAN(obj),
> > +				     trans);
> > +		break;
> > +	case SWITCHDEV_OBJ_ID_PORT_MDB:
> > +		err = port_mdb_add(netdev,
> > +				   SWITCHDEV_OBJ_PORT_MDB(obj),
> > +				   trans);
> > +		break;
> > +	default:
> > +		err = -EOPNOTSUPP;
> > +		break;
> > +	}
> > +
> > +	return err;
> > +}
> > +
> > +static int ethsw_port_del_vlan(struct ethsw_port_priv *port_priv, u16 vid)
> > +{
> > +	struct ethsw_core *ethsw = port_priv->ethsw_data;
> > +	struct net_device *netdev = port_priv->netdev;
> > +	struct dpsw_vlan_if_cfg vcfg;
> > +	int i, err, err2;
> > +	bool is_oper;
> > +
> > +	if (!port_priv->vlans[vid])
> > +		return -ENOENT;
> > +
> > +	if (port_priv->vlans[vid] & ETHSW_VLAN_PVID) {
> > +		struct dpsw_tci_cfg tci_cfg = { 0 };
> > +		/* Interface needs to be down to change PVID */
> > +		is_oper = netif_oper_up(netdev);
> > +
> > +		if (is_oper) {
> > +			err = dpsw_if_disable(ethsw->mc_io, 0,
> > +					      ethsw->dpsw_handle,
> > +					      port_priv->idx);
> > +			if (err) {
> > +				netdev_err(netdev, "dpsw_if_disable err %d\n",
> > +					   err);
> > +				goto exit_err;
> > +			}
> > +		}
> > +
> > +		err = dpsw_if_set_tci(ethsw->mc_io, 0,
> > +				      ethsw->dpsw_handle,
> > +				      port_priv->idx, &tci_cfg);
> > +		if (!err) {
> > +			port_priv->vlans[vid] &= ~ETHSW_VLAN_PVID;
> > +			port_priv->pvid = 0;
> > +		} else {
> > +			netdev_err(netdev, "dpsw_if_set_tci err %d\n", err);
> > +		}
> > +
> > +		if (is_oper) {
> > +			err2 = dpsw_if_enable(ethsw->mc_io, 0,
> > +					      ethsw->dpsw_handle,
> > +					      port_priv->idx);
> > +			if (err2) {
> > +				netdev_err(netdev, "dpsw_if_enable err %d\n",
> > +					   err2);
> > +				return err2;
> > +			}
> > +		}
> > +
> > +		if (err)
> > +			goto exit_err;
> > +	}
> > +
> > +	vcfg.num_ifs = 1;
> > +	vcfg.if_id[0] = port_priv->idx;
> > +	if (port_priv->vlans[vid] & ETHSW_VLAN_UNTAGGED) {
> > +		err = dpsw_vlan_remove_if_untagged(ethsw->mc_io, 0,
> > +						   ethsw->dpsw_handle,
> > +						   vid, &vcfg);
> > +		if (err) {
> > +			netdev_err(netdev,
> > +				   "dpsw_vlan_remove_if_untagged err %d\n",
> > +				   err);
> > +		}
> > +		port_priv->vlans[vid] &= ~ETHSW_VLAN_UNTAGGED;
> > +	}
> > +
> > +	if (port_priv->vlans[vid] & ETHSW_VLAN_MEMBER) {
> > +		err = dpsw_vlan_remove_if(ethsw->mc_io, 0, ethsw-
> >dpsw_handle,
> > +					  vid, &vcfg);
> > +		if (err) {
> > +			netdev_err(netdev,
> > +				   "dpsw_vlan_remove_if err %d\n", err);
> > +			return err;
> > +		}
> > +		port_priv->vlans[vid] &= ~ETHSW_VLAN_MEMBER;
> > +
> > +		/* Delete VLAN from switch if it is no longer configured on
> > +		 * any port
> > +		 */
> > +		for (i = 0; i < ethsw->sw_attr.num_ifs; i++)
> > +			if (ethsw->ports[i]->vlans[vid] &
> ETHSW_VLAN_MEMBER)
> > +				return 0; /* Found a port member in VID */
> > +
> > +		ethsw->vlans[vid] &= ~ETHSW_VLAN_GLOBAL;
> > +
> > +		err = ethsw_dellink_switch(ethsw, vid);
> > +		if (err)
> > +			goto exit_err;
> > +	}
> > +
> > +	return 0;
> > +exit_err:
> > +	return err;
> > +}
> > +
> > +static int port_vlans_del(struct net_device *netdev,
> > +			  const struct switchdev_obj_port_vlan *vlan)
> > +{
> > +	struct ethsw_port_priv *port_priv = netdev_priv(netdev);
> > +	int vid, err;
> > +
> > +	for (vid = vlan->vid_begin; vid <= vlan->vid_end; vid++) {
> > +		err = ethsw_port_del_vlan(port_priv, vid);
> > +		if (err)
> > +			break;
> > +	}
> > +
> > +	return err;
> > +}
> > +
> > +static int port_mdb_del(struct net_device *netdev,
> > +			const struct switchdev_obj_port_mdb *mdb)
> > +{
> > +	struct ethsw_port_priv *port_priv = netdev_priv(netdev);
> > +	int err;
> > +
> > +	if (!port_lookup_address(netdev, 0, mdb->addr))
> > +		return -ENOENT;
> > +
> > +	err = ethsw_port_fdb_del_mc(port_priv, mdb->addr);
> > +	if (err)
> > +		return err;
> > +
> > +	err = dev_mc_del(netdev, mdb->addr);
> > +	if (err) {
> > +		netdev_err(netdev, "dev_mc_del err %d\n", err);
> > +		return err;
> > +	}
> > +
> > +	return err;
> > +}
> > +
> > +static int swdev_port_obj_del(struct net_device *netdev,
> > +			      const struct switchdev_obj *obj)
> > +{
> > +	int err;
> > +
> > +	switch (obj->id) {
> > +	case SWITCHDEV_OBJ_ID_PORT_VLAN:
> > +		err = port_vlans_del(netdev,
> SWITCHDEV_OBJ_PORT_VLAN(obj));
> > +		break;
> > +	case SWITCHDEV_OBJ_ID_PORT_MDB:
> > +		err = port_mdb_del(netdev, SWITCHDEV_OBJ_PORT_MDB(obj));
> > +		break;
> > +	default:
> > +		err = -EOPNOTSUPP;
> > +		break;
> > +	}
> > +	return err;
> > +}
> > +
> > +static const struct switchdev_ops ethsw_port_switchdev_ops = {
> > +	.switchdev_port_attr_get	= swdev_port_attr_get,
> > +	.switchdev_port_attr_set	= swdev_port_attr_set,
> > +	.switchdev_port_obj_add		= swdev_port_obj_add,
> > +	.switchdev_port_obj_del		= swdev_port_obj_del,
> > +};
> > +
> > +/* For the moment, only flood setting needs to be updated */
> > +static int port_bridge_join(struct net_device *netdev)
> > +{
> > +	struct ethsw_port_priv *port_priv = netdev_priv(netdev);
> > +
> > +	/* Enable flooding */
> > +	return ethsw_port_set_flood(port_priv, 1);
> > +}
> > +
> > +static int port_bridge_leave(struct net_device *netdev)
> > +{
> > +	struct ethsw_port_priv *port_priv = netdev_priv(netdev);
> > +
> > +	/* Disable flooding */
> > +	return ethsw_port_set_flood(port_priv, 0);
> > +}
> > +
> > +static int port_netdevice_event(struct notifier_block *unused,
> > +				unsigned long event, void *ptr)
> > +{
> > +	struct net_device *netdev = netdev_notifier_info_to_dev(ptr);
> > +	struct netdev_notifier_changeupper_info *info = ptr;
> > +	struct net_device *upper_dev;
> > +	int err = 0;
> > +
> > +	if (netdev->netdev_ops != &ethsw_port_ops)
> > +		return NOTIFY_DONE;
> > +
> > +	/* Handle just upper dev link/unlink for the moment */
> > +	if (event == NETDEV_CHANGEUPPER) {
> > +		upper_dev = info->upper_dev;
> > +		if (netif_is_bridge_master(upper_dev)) {
> > +			if (info->linking)
> > +				err = port_bridge_join(netdev);
> > +			else
> > +				err = port_bridge_leave(netdev);
> > +		}
> > +	}
> > +
> > +	return notifier_from_errno(err);
> > +}
> > +
> > +static struct notifier_block port_nb __read_mostly = {
> > +	.notifier_call = port_netdevice_event,
> > +};
> > +
> > +struct ethsw_switchdev_event_work {
> > +	struct work_struct work;
> > +	struct switchdev_notifier_fdb_info fdb_info;
> > +	struct net_device *dev;
> > +	unsigned long event;
> > +};
> > +
> > +static void ethsw_switchdev_event_work(struct work_struct *work)
> > +{
> > +	struct ethsw_switchdev_event_work *switchdev_work =
> > +		container_of(work, struct ethsw_switchdev_event_work, work);
> > +	struct net_device *dev = switchdev_work->dev;
> > +	struct switchdev_notifier_fdb_info *fdb_info;
> > +	struct ethsw_port_priv *port_priv;
> > +
> > +	rtnl_lock();
> > +	port_priv = netdev_priv(dev);
> > +	fdb_info = &switchdev_work->fdb_info;
> > +
> > +	switch (switchdev_work->event) {
> > +	case SWITCHDEV_FDB_ADD_TO_DEVICE:
> > +		ethsw_port_fdb_add_uc(netdev_priv(dev), fdb_info->addr);
> > +		break;
> > +	case SWITCHDEV_FDB_DEL_TO_DEVICE:
> > +		ethsw_port_fdb_del_uc(netdev_priv(dev), fdb_info->addr);
> > +		break;
> > +	}
> > +
> > +	rtnl_unlock();
> > +	kfree(switchdev_work->fdb_info.addr);
> > +	kfree(switchdev_work);
> > +	dev_put(dev);
> > +}
> > +
> > +/* Called under rcu_read_lock() */
> > +static int port_switchdev_event(struct notifier_block *unused,
> > +				unsigned long event, void *ptr)
> > +{
> > +	struct net_device *dev = switchdev_notifier_info_to_dev(ptr);
> > +	struct ethsw_switchdev_event_work *switchdev_work;
> > +	struct switchdev_notifier_fdb_info *fdb_info = ptr;
> > +
> > +	switchdev_work = kzalloc(sizeof(*switchdev_work), GFP_ATOMIC);
> > +	if (!switchdev_work)
> > +		return NOTIFY_BAD;
> > +
> > +	INIT_WORK(&switchdev_work->work, ethsw_switchdev_event_work);
> > +	switchdev_work->dev = dev;
> > +	switchdev_work->event = event;
> > +
> > +	switch (event) {
> > +	case SWITCHDEV_FDB_ADD_TO_DEVICE:
> > +	case SWITCHDEV_FDB_DEL_TO_DEVICE:
> > +		memcpy(&switchdev_work->fdb_info, ptr,
> > +		       sizeof(switchdev_work->fdb_info));
> > +		switchdev_work->fdb_info.addr = kzalloc(ETH_ALEN,
> GFP_ATOMIC);
> > +		if (!switchdev_work->fdb_info.addr)
> > +			goto err_addr_alloc;
> > +
> > +		ether_addr_copy((u8 *)switchdev_work->fdb_info.addr,
> > +				fdb_info->addr);
> > +
> > +		/* Take a reference on the device to avoid being freed. */
> > +		dev_hold(dev);
> > +		break;
> > +	default:
> > +		return NOTIFY_DONE;
> > +	}
> > +
> > +	queue_work(ethsw_owq, &switchdev_work->work);
> > +
> > +	return NOTIFY_DONE;
> > +
> > +err_addr_alloc:
> > +	kfree(switchdev_work);
> > +	return NOTIFY_BAD;
> > +}
> > +
> > +static struct notifier_block port_switchdev_nb = {
> > +	.notifier_call = port_switchdev_event,
> > +};
> > +
> > +static int ethsw_register_notifier(struct device *dev)
> > +{
> > +	int err;
> > +
> > +	err = register_netdevice_notifier(&port_nb);
> > +	if (err) {
> > +		dev_err(dev, "Failed to register netdev notifier\n");
> > +		return err;
> > +	}
> > +
> > +	err = register_switchdev_notifier(&port_switchdev_nb);
> > +	if (err) {
> > +		dev_err(dev, "Failed to register switchdev notifier\n");
> > +		goto err_switchdev_nb;
> > +	}
> > +
> > +	return 0;
> > +
> > +err_switchdev_nb:
> > +	unregister_netdevice_notifier(&port_nb);
> > +	return err;
> > +}
> > +
> > +static int ethsw_open(struct ethsw_core	*ethsw)
> 
> Minor formatting error, tab in function signature - see following function as well.
> 
> > +{
> > +	struct ethsw_port_priv *port_priv = NULL;
> > +	int i, err;
> > +
> > +	err = dpsw_enable(ethsw->mc_io, 0, ethsw->dpsw_handle);
> > +	if (err) {
> > +		dev_err(ethsw->dev, "dpsw_enable err %d\n", err);
> > +		return err;
> > +	}
> > +
> > +	for (i = 0; i < ethsw->sw_attr.num_ifs; i++) {
> > +		port_priv = ethsw->ports[i];
> > +		err = dev_open(port_priv->netdev);
> > +		if (err) {
> > +			netdev_err(port_priv->netdev, "dev_open err %d\n",
> err);
> > +			return err;
> > +		}
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int ethsw_stop(struct ethsw_core	*ethsw)
> > +{
> > +	struct ethsw_port_priv *port_priv = NULL;
> > +	int i, err;
> > +
> > +	destroy_workqueue(ethsw_owq);
> 
> If workqueue is destroyed here, shouldn't it be alloc'd in ethsw_open?
> 
> > +
> > +	for (i = 0; i < ethsw->sw_attr.num_ifs; i++) {
> > +		port_priv = ethsw->ports[i];
> > +		dev_close(port_priv->netdev);
> > +	}
> > +
> > +	err = dpsw_disable(ethsw->mc_io, 0, ethsw->dpsw_handle);
> > +	if (err) {
> > +		dev_err(ethsw->dev, "dpsw_disable err %d\n", err);
> > +		return err;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int ethsw_init(struct fsl_mc_device *sw_dev)
> > +{
> > +	struct device *dev = &sw_dev->dev;
> > +	struct ethsw_core *ethsw = dev_get_drvdata(dev);
> > +	u16 version_major, version_minor, i;
> > +	struct dpsw_stp_cfg stp_cfg;
> > +	int err;
> > +
> > +	ethsw->dev_id = sw_dev->obj_desc.id;
> > +
> > +	err = dpsw_open(ethsw->mc_io, 0, ethsw->dev_id, &ethsw-
> >dpsw_handle);
> > +	if (err) {
> > +		dev_err(dev, "dpsw_open err %d\n", err);
> > +		return err;
> > +	}
> > +
> > +	err = dpsw_get_attributes(ethsw->mc_io, 0, ethsw->dpsw_handle,
> > +				  &ethsw->sw_attr);
> > +	if (err) {
> > +		dev_err(dev, "dpsw_get_attributes err %d\n", err);
> > +		goto err_close;
> > +	}
> > +
> > +	err = dpsw_get_api_version(ethsw->mc_io, 0,
> > +				   &version_major,
> > +				   &version_minor);
> > +	if (err) {
> > +		dev_err(dev, "dpsw_get_api_version err %d\n", err);
> > +		goto err_close;
> > +	}
> > +
> > +	/* Minimum supported DPSW version check */
> > +	if (version_major < DPSW_MIN_VER_MAJOR ||
> > +	    (version_major == DPSW_MIN_VER_MAJOR &&
> > +	     version_minor < DPSW_MIN_VER_MINOR)) {
> > +		dev_err(dev, "DPSW version %d:%d not supported. Use %d.%d
> or
> > greater.\n",
> > +			version_major,
> > +			version_minor,
> > +			DPSW_MIN_VER_MAJOR, DPSW_MIN_VER_MINOR);
> > +		err = -ENOTSUPP;
> > +		goto err_close;
> > +	}
> > +
> > +	err = dpsw_reset(ethsw->mc_io, 0, ethsw->dpsw_handle);
> > +	if (err) {
> > +		dev_err(dev, "dpsw_reset err %d\n", err);
> > +		goto err_close;
> > +	}
> > +
> > +	err = dpsw_fdb_set_learning_mode(ethsw->mc_io, 0, ethsw-
> >dpsw_handle, 0,
> > +					 DPSW_FDB_LEARNING_MODE_HW);
> > +	if (err) {
> > +		dev_err(dev, "dpsw_fdb_set_learning_mode err %d\n", err);
> > +		goto err_close;
> > +	}
> > +
> > +	stp_cfg.vlan_id = DEFAULT_VLAN_ID;
> > +	stp_cfg.state = DPSW_STP_STATE_FORWARDING;
> > +
> > +	for (i = 0; i < ethsw->sw_attr.num_ifs; i++) {
> > +		err = dpsw_if_set_stp(ethsw->mc_io, 0, ethsw->dpsw_handle, i,
> > +				      &stp_cfg);
> > +		if (err) {
> > +			dev_err(dev, "dpsw_if_set_stp err %d for port %d\n",
> > +				err, i);
> > +			goto err_close;
> > +		}
> > +
> > +		err = dpsw_if_set_broadcast(ethsw->mc_io, 0,
> > +					    ethsw->dpsw_handle, i, 1);
> > +		if (err) {
> > +			dev_err(dev,
> > +				"dpsw_if_set_broadcast err %d for port %d\n",
> > +				err, i);
> > +			goto err_close;
> > +		}
> > +	}
> > +
> > +	ethsw_owq = alloc_ordered_workqueue("%s_ordered",
> WQ_MEM_RECLAIM,
> > +					    "ethsw");
> > +	if (!ethsw_owq) {
> > +		err = -ENOMEM;
> > +		goto err_close;
> > +	}
> > +
> > +	err = ethsw_register_notifier(dev);
> > +	if (err)
> > +		goto err_destroy_ordered_workqueue;
> > +
> > +	return 0;
> > +
> > +err_destroy_ordered_workqueue:
> > +	destroy_workqueue(ethsw_owq);
> > +
> > +err_close:
> > +	dpsw_close(ethsw->mc_io, 0, ethsw->dpsw_handle);
> > +	return err;
> > +}
> > +
> > +static int ethsw_port_init(struct ethsw_port_priv *port_priv, u16 port)
> > +{
> > +	const char def_mcast[ETH_ALEN] = {0x01, 0x00, 0x5e, 0x00, 0x00, 0x01};
> > +	struct net_device *netdev = port_priv->netdev;
> > +	struct ethsw_core *ethsw = port_priv->ethsw_data;
> > +	struct dpsw_tci_cfg tci_cfg = {0};
> > +	struct dpsw_vlan_if_cfg vcfg;
> > +	int err;
> > +
> > +	/* Switch starts with all ports configured to VLAN 1. Need to
> > +	 * remove this setting to allow configuration at bridge join
> > +	 */
> > +	vcfg.num_ifs = 1;
> > +	vcfg.if_id[0] = port_priv->idx;
> > +
> > +	err = dpsw_vlan_remove_if_untagged(ethsw->mc_io, 0, ethsw-
> >dpsw_handle,
> > +					   DEFAULT_VLAN_ID, &vcfg);
> > +	if (err) {
> > +		netdev_err(netdev, "dpsw_vlan_remove_if_untagged err
> %d\n",
> > +			   err);
> > +		return err;
> > +	}
> > +
> > +	err = dpsw_if_set_tci(ethsw->mc_io, 0, ethsw->dpsw_handle,
> > +			      port_priv->idx, &tci_cfg);
> > +	if (err) {
> > +		netdev_err(netdev, "dpsw_if_set_tci err %d\n", err);
> > +		return err;
> > +	}
> > +
> > +	err = dpsw_vlan_remove_if(ethsw->mc_io, 0, ethsw->dpsw_handle,
> > +				  DEFAULT_VLAN_ID, &vcfg);
> > +	if (err) {
> > +		netdev_err(netdev, "dpsw_vlan_remove_if err %d\n", err);
> > +		return err;
> > +	}
> > +
> > +	err = ethsw_port_fdb_add_mc(port_priv, def_mcast);
> > +
> > +	return err;
> > +}
> > +
> > +static void ethsw_takedown(struct fsl_mc_device *sw_dev)
> > +{
> > +	struct device *dev = &sw_dev->dev;
> > +	struct ethsw_core *ethsw = dev_get_drvdata(dev);
> > +	int err;
> > +
> > +	err = unregister_switchdev_notifier(&port_switchdev_nb);
> > +	if (err)
> > +		dev_err(dev,
> > +			"Failed to unregister switchdev notifier (%d)\n", err);
> > +
> > +	err = unregister_netdevice_notifier(&port_nb);
> > +	if (err)
> > +		dev_err(dev,
> > +			"Failed to unregister netdev notifier (%d)\n", err);
> 
> Above 2 can be grouped into ethsw_unregister_notifier.
> 
> > +
> > +	err = dpsw_close(ethsw->mc_io, 0, ethsw->dpsw_handle);
> > +	if (err)
> > +		dev_warn(dev, "dpsw_close err %d\n", err);
> > +}
> > +
> > +static int ethsw_remove(struct fsl_mc_device *sw_dev)
> > +{
> > +	struct ethsw_port_priv *port_priv;
> > +	struct ethsw_core *ethsw;
> > +	struct device *dev;
> > +	int i;
> > +
> > +	dev = &sw_dev->dev;
> > +	ethsw = dev_get_drvdata(dev);
> > +
> > +	ethsw_teardown_irqs(sw_dev);
> > +
> > +	rtnl_lock();
> > +	ethsw_stop(ethsw);
> > +	rtnl_unlock();
> > +
> > +	for (i = 0; i < ethsw->sw_attr.num_ifs; i++) {
> > +		port_priv = ethsw->ports[i];
> > +		unregister_netdev(port_priv->netdev);
> > +		free_netdev(port_priv->netdev);
> > +	}
> > +	kfree(ethsw->ports);
> > +
> > +	ethsw_takedown(sw_dev);
> > +	fsl_mc_portal_free(ethsw->mc_io);
> > +
> > +	kfree(ethsw);
> > +
> > +	dev_set_drvdata(dev, NULL);
> > +
> > +	return 0;
> > +}
> > +
> > +static int ethsw_probe_port(struct ethsw_core *ethsw, u16 port_idx)
> > +{
> > +	struct ethsw_port_priv *port_priv;
> > +	struct device *dev = ethsw->dev;
> > +	struct net_device *port_netdev;
> > +	int err;
> > +
> > +	port_netdev = alloc_etherdev(sizeof(struct ethsw_port_priv));
> > +	if (!port_netdev) {
> > +		dev_err(dev, "alloc_etherdev error\n");
> > +		return -ENOMEM;
> > +	}
> > +
> > +	port_priv = netdev_priv(port_netdev);
> > +	port_priv->netdev = port_netdev;
> > +	port_priv->ethsw_data = ethsw;
> > +
> > +	port_priv->idx = port_idx;
> > +	port_priv->stp_state = BR_STATE_FORWARDING;
> > +
> > +	/* Flooding is implicitly enabled */
> > +	port_priv->flood = true;
> > +
> > +	SET_NETDEV_DEV(port_netdev, dev);
> > +	port_netdev->netdev_ops = &ethsw_port_ops;
> > +	port_netdev->switchdev_ops = &ethsw_port_switchdev_ops;
> > +
> > +	/* Set MTU limits */
> > +	port_netdev->min_mtu = ETH_MIN_MTU;
> > +	port_netdev->max_mtu = ETHSW_MAX_FRAME_LENGTH;
> > +
> > +	err = register_netdev(port_netdev);
> > +	if (err < 0) {
> > +		dev_err(dev, "register_netdev error %d\n", err);
> > +			free_netdev(port_netdev);
> > +			return err;
> > +		}
> > +
> > +	ethsw->ports[port_idx] = port_priv;
> > +
> > +	return ethsw_port_init(port_priv, port_idx);
> > +}
> > +
> > +static int ethsw_probe(struct fsl_mc_device *sw_dev)
> > +{
> > +	struct device *dev = &sw_dev->dev;
> > +	struct ethsw_core *ethsw;
> > +	int err;
> > +	u16 i, j;
> > +
> > +	/* Allocate switch core*/
> > +	ethsw = kzalloc(sizeof(*ethsw), GFP_KERNEL);
> > +
> > +	if (!ethsw)
> > +		return -ENOMEM;
> > +
> > +	ethsw->dev = dev;
> > +	dev_set_drvdata(dev, ethsw);
> > +
> > +	err = fsl_mc_portal_allocate(sw_dev, 0, &ethsw->mc_io);
> > +	if (err) {
> > +		dev_err(dev, "fsl_mc_portal_allocate err %d\n", err);
> > +		goto err_free_drvdata;
> > +	}
> > +
> > +	err = ethsw_init(sw_dev);
> > +	if (err)
> > +		goto err_free_cmdport;
> > +
> > +	/* DEFAULT_VLAN_ID is implicitly configured on the switch */
> > +	ethsw->vlans[DEFAULT_VLAN_ID] = ETHSW_VLAN_MEMBER;
> > +
> > +	/* Learning is implicitly enabled */
> > +	ethsw->learning = true;
> > +
> > +	ethsw->ports = kcalloc(ethsw->sw_attr.num_ifs, sizeof(*ethsw->ports),
> > +			       GFP_KERNEL);
> > +	if (!(ethsw->ports)) {
> > +		err = -ENOMEM;
> > +		goto err_takedown;
> > +	}
> > +
> > +	for (i = 0; i < ethsw->sw_attr.num_ifs; i++) {
> > +		err = ethsw_probe_port(ethsw, i);
> > +		if (err) {
> > +			/* Cleanup previous ports only */
> > +			for (j = 0; j < i; j++) {
> 
> I think you can go with
> for (i--; i >= 0; i--)
> 
> or better yet:
> goto err_free_ports
> and refactor err_free_ports to look like:
> for (i--; i >= 0; i--) {
> ...
> }
> 
> Best regards,
> Bogdan P.
> 
> > +				unregister_netdev(ethsw->ports[j]->netdev);
> > +				free_netdev(ethsw->ports[j]->netdev);
> > +			}
> > +			goto err_takedown;
> > +		}
> > +	}
> > +
> > +	/* Switch starts up enabled */
> > +	rtnl_lock();
> > +	err = ethsw_open(ethsw);
> > +	rtnl_unlock();
> > +	if (err)
> > +		goto err_free_ports;
> > +
> > +	/* Setup IRQs */
> > +	err = ethsw_setup_irqs(sw_dev);
> > +	if (err)
> > +		goto err_stop;
> > +
> > +	dev_info(dev, "probed %d port switch\n", ethsw->sw_attr.num_ifs);
> > +	return 0;
> > +
> > +err_stop:
> > +	rtnl_lock();
> > +	ethsw_stop(ethsw);
> > +	rtnl_unlock();
> > +
> > +err_free_ports:
> > +	for (i = 0; i < ethsw->sw_attr.num_ifs; i++) {
> > +		unregister_netdev(ethsw->ports[i]->netdev);
> > +		free_netdev(ethsw->ports[i]->netdev);
> > +	}
> > +	kfree(ethsw->ports);
> > +
> > +err_takedown:
> > +	ethsw_takedown(sw_dev);
> > +
> > +err_free_cmdport:
> > +	fsl_mc_portal_free(ethsw->mc_io);
> > +
> > +err_free_drvdata:
> > +	kfree(ethsw);
> > +	dev_set_drvdata(dev, NULL);
> > +
> > +	return err;
> > +}
> > +
> > +static const struct fsl_mc_device_id ethsw_match_id_table[] = {
> > +	{
> > +		.vendor = FSL_MC_VENDOR_FREESCALE,
> > +		.obj_type = "dpsw",
> > +	},
> > +	{ .vendor = 0x0 }
> > +};
> > +MODULE_DEVICE_TABLE(fslmc, ethsw_match_id_table);
> > +
> > +static struct fsl_mc_driver eth_sw_drv = {
> > +	.driver = {
> > +		.name = KBUILD_MODNAME,
> > +		.owner = THIS_MODULE,
> > +	},
> > +	.probe = ethsw_probe,
> > +	.remove = ethsw_remove,
> > +	.match_id_table = ethsw_match_id_table
> > +};
> > +
> > +module_fsl_mc_driver(eth_sw_drv);
> > +
> > +MODULE_LICENSE("Dual BSD/GPL");
> > +MODULE_DESCRIPTION("DPAA2 Ethernet Switch Driver");
> > diff --git a/drivers/staging/fsl-dpaa2/ethsw/ethsw.h b/drivers/staging/fsl-
> > dpaa2/ethsw/ethsw.h
> > new file mode 100644
> > index 0000000..8c1d645
> > --- /dev/null
> > +++ b/drivers/staging/fsl-dpaa2/ethsw/ethsw.h
> > @@ -0,0 +1,88 @@
> > +/* Copyright 2014-2017 Freescale Semiconductor Inc.
> > + * Copyright 2017 NXP
> > + *
> > + * Redistribution and use in source and binary forms, with or without
> > + * modification, are permitted provided that the following conditions are met:
> > + *     * Redistributions of source code must retain the above copyright
> > + *	 notice, this list of conditions and the following disclaimer.
> > + *     * Redistributions in binary form must reproduce the above copyright
> > + *	 notice, this list of conditions and the following disclaimer in the
> > + *	 documentation and/or other materials provided with the distribution.
> > + *     * Neither the name of the above-listed copyright holders nor the
> > + *	 names of any contributors may be used to endorse or promote products
> > + *	 derived from this software without specific prior written permission.
> > + *
> > + *
> > + * ALTERNATIVELY, this software may be distributed under the terms of the
> > + * GNU General Public License ("GPL") as published by the Free Software
> > + * Foundation, either version 2 of that License or (at your option) any
> > + * later version.
> > + *
> > + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
> CONTRIBUTORS "AS IS"
> > + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> LIMITED TO, THE
> > + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
> PARTICULAR PURPOSE
> > + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDERS OR
> CONTRIBUTORS BE
> > + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
> > + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
> PROCUREMENT OF
> > + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
> BUSINESS
> > + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY,
> WHETHER IN
> > + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR
> OTHERWISE)
> > + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
> ADVISED OF THE
> > + * POSSIBILITY OF SUCH DAMAGE.
> > + */
> > +
> > +#ifndef __ETHSW_H
> > +#define __ETHSW_H
> > +
> > +#include <linux/netdevice.h>
> > +#include <linux/etherdevice.h>
> > +#include <linux/rtnetlink.h>
> > +#include <linux/if_vlan.h>
> > +#include <uapi/linux/if_bridge.h>
> > +#include <net/switchdev.h>
> > +#include <linux/if_bridge.h>
> > +
> > +#include "dpsw.h"
> > +
> > +/* Number of IRQs supported */
> > +#define DPSW_IRQ_NUM	2
> > +
> > +#define ETHSW_VLAN_MEMBER	1
> > +#define ETHSW_VLAN_UNTAGGED	2
> > +#define ETHSW_VLAN_PVID		4
> > +#define ETHSW_VLAN_GLOBAL	8
> > +
> > +/* Maximum Frame Length supported by HW (currently 10k) */
> > +#define DPAA2_MFL		(10 * 1024)
> > +#define ETHSW_MAX_FRAME_LENGTH	(DPAA2_MFL - VLAN_ETH_HLEN -
> ETH_FCS_LEN)
> > +#define ETHSW_L2_MAX_FRM(mtu)	((mtu) + VLAN_ETH_HLEN +
> ETH_FCS_LEN)
> > +
> > +struct ethsw_core;
> > +
> > +/* Per port private data */
> > +struct ethsw_port_priv {
> > +	struct net_device	*netdev;
> > +	u16			idx;
> > +	struct ethsw_core	*ethsw_data;
> > +	u8			link_state;
> > +	u8			stp_state;
> > +	bool			flood;
> > +
> > +	u8			vlans[VLAN_VID_MASK + 1];
> > +	u16			pvid;
> > +};
> > +
> > +/* Switch data */
> > +struct ethsw_core {
> > +	struct device			*dev;
> > +	struct fsl_mc_io		*mc_io;
> > +	u16				dpsw_handle;
> > +	struct dpsw_attr		sw_attr;
> > +	int				dev_id;
> > +	struct ethsw_port_priv		**ports;
> > +
> > +	u8				vlans[VLAN_VID_MASK + 1];
> > +	bool				learning;
> > +};
> > +
> > +#endif	/* __ETHSW_H */
> > --
> > 1.9.1

^ permalink raw reply

* (unknown), 
From: marketing @ 2017-09-29 13:49 UTC (permalink / raw)
  To: netdev

[-- Attachment #1: 761981.zip --]
[-- Type: application/zip, Size: 7219 bytes --]

^ permalink raw reply

* RE: [RESEND PATCH 2/6] staging: fsl-dpaa2/ethsw: Add Freescale DPAA2 Ethernet Switch driver
From: Bogdan Purcareata @ 2017-09-29 13:36 UTC (permalink / raw)
  To: Razvan Stefanescu, gregkh@linuxfoundation.org
  Cc: devel@driverdev.osuosl.org, arnd@arndb.de, netdev@vger.kernel.org,
	Alexandru Marginean, linux-kernel@vger.kernel.org, agraf@suse.de,
	stuyoder@gmail.com, Laurentiu Tudor
In-Reply-To: <1505825158-8192-3-git-send-email-razvan.stefanescu@nxp.com>

> Introduce the DPAA2 Ethernet Switch driver, which manages Datapath Switch
> (DPSW) objects discovered on the MC bus.
> 
> Suggested-by: Alexandru Marginean <alexandru.marginean@nxp.com>
> Signed-off-by: Razvan Stefanescu <razvan.stefanescu@nxp.com>
> ---
>  drivers/staging/fsl-dpaa2/ethsw/Makefile |    2 +-
>  drivers/staging/fsl-dpaa2/ethsw/ethsw.c  | 1523 ++++++++++++++++++++++++++++++
>  drivers/staging/fsl-dpaa2/ethsw/ethsw.h  |   88 ++
>  3 files changed, 1612 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/staging/fsl-dpaa2/ethsw/ethsw.c
>  create mode 100644 drivers/staging/fsl-dpaa2/ethsw/ethsw.h
> 
> diff --git a/drivers/staging/fsl-dpaa2/ethsw/Makefile b/drivers/staging/fsl-
> dpaa2/ethsw/Makefile
> index db137f7..a6d72d1 100644
> --- a/drivers/staging/fsl-dpaa2/ethsw/Makefile
> +++ b/drivers/staging/fsl-dpaa2/ethsw/Makefile
> @@ -4,4 +4,4 @@
> 
>  obj-$(CONFIG_FSL_DPAA2_ETHSW) += dpaa2-ethsw.o
> 
> -dpaa2-ethsw-objs := dpsw.o
> +dpaa2-ethsw-objs := ethsw.o dpsw.o
> diff --git a/drivers/staging/fsl-dpaa2/ethsw/ethsw.c b/drivers/staging/fsl-
> dpaa2/ethsw/ethsw.c
> new file mode 100644
> index 0000000..ae86078
> --- /dev/null
> +++ b/drivers/staging/fsl-dpaa2/ethsw/ethsw.c
> @@ -0,0 +1,1523 @@
> +/* Copyright 2014-2016 Freescale Semiconductor Inc.
> + * Copyright 2017 NXP
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions are met:
> + *     * Redistributions of source code must retain the above copyright
> + *	 notice, this list of conditions and the following disclaimer.
> + *     * Redistributions in binary form must reproduce the above copyright
> + *	 notice, this list of conditions and the following disclaimer in the
> + *	 documentation and/or other materials provided with the distribution.
> + *     * Neither the name of the above-listed copyright holders nor the
> + *	 names of any contributors may be used to endorse or promote products
> + *	 derived from this software without specific prior written permission.
> + *
> + *
> + * ALTERNATIVELY, this software may be distributed under the terms of the
> + * GNU General Public License ("GPL") as published by the Free Software
> + * Foundation, either version 2 of that License or (at your option) any
> + * later version.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
> + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDERS OR CONTRIBUTORS BE
> + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
> + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
> + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
> + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
> + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
> + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
> + * POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +#include <linux/module.h>
> +
> +#include <linux/interrupt.h>
> +#include <linux/msi.h>
> +#include <linux/kthread.h>
> +#include <linux/workqueue.h>
> +
> +#include "../../fsl-mc/include/mc.h"
> +
> +#include "ethsw.h"
> +
> +static struct workqueue_struct *ethsw_owq;
> +
> +/* Minimal supported DPSW version */
> +#define DPSW_MIN_VER_MAJOR		8
> +#define DPSW_MIN_VER_MINOR		0
> +
> +#define DEFAULT_VLAN_ID			1
> +
> +static int ethsw_add_vlan(struct ethsw_core *ethsw, u16 vid)
> +{
> +	int err;
> +
> +	struct dpsw_vlan_cfg	vcfg = {
> +		.fdb_id = 0,
> +	};
> +
> +	if (ethsw->vlans[vid]) {
> +		dev_err(ethsw->dev, "VLAN already configured\n");
> +		return -EEXIST;
> +	}
> +
> +	err = dpsw_vlan_add(ethsw->mc_io, 0,
> +			    ethsw->dpsw_handle, vid, &vcfg);
> +	if (err) {
> +		dev_err(ethsw->dev, "dpsw_vlan_add err %d\n", err);
> +		return err;
> +	}
> +	ethsw->vlans[vid] = ETHSW_VLAN_MEMBER;/
> +
> +	return 0;
> +}
> +
> +static int ethsw_port_add_vlan(struct ethsw_port_priv *port_priv,
> +			       u16 vid, u16 flags)
> +{
> +	struct ethsw_core *ethsw = port_priv->ethsw_data;
> +	struct net_device *netdev = port_priv->netdev;
> +	struct dpsw_vlan_if_cfg vcfg;
> +	bool is_oper;
> +	int err, err2;

Mild suggestion - s/err2/ret/, just because it sounds better, at least to me (same for similar situations in the rest of the file).

> +
> +	if (port_priv->vlans[vid]) {
> +		netdev_warn(netdev, "VLAN %d already configured\n", vid);
> +		return -EEXIST;
> +	}
> +
> +	vcfg.num_ifs = 1;
> +	vcfg.if_id[0] = port_priv->idx;
> +	err = dpsw_vlan_add_if(ethsw->mc_io, 0, ethsw->dpsw_handle, vid, &vcfg);
> +	if (err) {
> +		netdev_err(netdev, "dpsw_vlan_add_if err %d\n", err);
> +		return err;
> +	}
> +
> +	port_priv->vlans[vid] = ETHSW_VLAN_MEMBER;
> +
> +	if (flags & BRIDGE_VLAN_INFO_UNTAGGED) {
> +		err = dpsw_vlan_add_if_untagged(ethsw->mc_io, 0,
> +						ethsw->dpsw_handle,
> +						vid, &vcfg);
> +		if (err) {
> +			netdev_err(netdev,
> +				   "dpsw_vlan_add_if_untagged err %d\n", err);
> +			return err;
> +		}
> +		port_priv->vlans[vid] |= ETHSW_VLAN_UNTAGGED;
> +	}
> +
> +	if (flags & BRIDGE_VLAN_INFO_PVID) {
> +		struct dpsw_tci_cfg tci_cfg = {
> +			.pcp = 0,
> +			.dei = 0,
> +			.vlan_id = vid,
> +		};
> +
> +		/* Interface needs to be down to change PVID */
> +		is_oper = netif_oper_up(netdev);
> +		if (is_oper) {
> +			err = dpsw_if_disable(ethsw->mc_io, 0,
> +					      ethsw->dpsw_handle,
> +					      port_priv->idx);
> +			if (err) {
> +				netdev_err(netdev,
> +					   "dpsw_if_disable err %d\n", err);
> +				return err;
> +			}
> +		}
> +
> +		err = dpsw_if_set_tci(ethsw->mc_io, 0, ethsw->dpsw_handle,
> +				      port_priv->idx, &tci_cfg);
> +		if (!err) {
> +			/* Delete previous PVID info and mark the new one */
> +			if (port_priv->pvid)
> +				port_priv->vlans[port_priv->pvid]
> +						 ^= ETHSW_VLAN_PVID;

Can it be " &= ~ETHSW_VLAN_PVID" ? Are there other implications?

> +
> +			port_priv->vlans[vid] |= ETHSW_VLAN_PVID;
> +			port_priv->pvid = vid;
> +		} else {
> +			netdev_err(netdev, "dpsw_if_set_tci err %d\n", err);
> +		}
> +
> +		if (is_oper) {
> +			err2 = dpsw_if_enable(ethsw->mc_io, 0,
> +					      ethsw->dpsw_handle,
> +					      port_priv->idx);
> +			if (err2) {
> +				netdev_err(netdev,
> +					   "dpsw_if_enable err %d\n", err2);
> +				return err2;
> +			}
> +		}
> +	}
> +
> +	return err;
> +}
> +
> +static int ethsw_set_learning(struct ethsw_core *ethsw, u8 flag)
> +{
> +	enum dpsw_fdb_learning_mode learn_mode;
> +	int err;
> +
> +	if (flag)
> +		learn_mode = DPSW_FDB_LEARNING_MODE_HW;
> +	else
> +		learn_mode = DPSW_FDB_LEARNING_MODE_DIS;
> +
> +	err = dpsw_fdb_set_learning_mode(ethsw->mc_io, 0, ethsw->dpsw_handle, 0,
> +					 learn_mode);
> +	if (err) {
> +		dev_err(ethsw->dev, "dpsw_fdb_set_learning_mode err %d\n", err);
> +		return err;
> +	}
> +	ethsw->learning = !!flag;
> +
> +	return 0;
> +}
> +
> +static int ethsw_port_set_flood(struct ethsw_port_priv *port_priv, u8 flag)
> +{
> +	int err;
> +
> +	err = dpsw_if_set_flooding(port_priv->ethsw_data->mc_io, 0,
> +				   port_priv->ethsw_data->dpsw_handle,
> +				   port_priv->idx, (int)flag);

Why is this cast necessary? Can't the API be reworked to use u8 (or, better yet, bool) instead of int?

> +	if (err) {
> +		netdev_err(port_priv->netdev,
> +			   "dpsw_fdb_set_learning_mode err %d\n", err);
> +		return err;
> +	}
> +	port_priv->flood = !!flag;
> +
> +	return 0;
> +}
> +
> +static int ethsw_port_set_stp_state(struct ethsw_port_priv *port_priv, u8
> state)
> +{
> +	struct dpsw_stp_cfg stp_cfg = {
> +		.vlan_id = DEFAULT_VLAN_ID,
> +		.state = state,
> +	};
> +	int err;
> +
> +	if (!netif_oper_up(port_priv->netdev) || state == port_priv->stp_state)
> +		return 0;	/* Nothing to do */
> +
> +	err = dpsw_if_set_stp(port_priv->ethsw_data->mc_io, 0,
> +			      port_priv->ethsw_data->dpsw_handle,
> +			      port_priv->idx, &stp_cfg);
> +	if (err) {
> +		netdev_err(port_priv->netdev,
> +			   "dpsw_if_set_stp err %d\n", err);
> +		return err;
> +	}
> +
> +	port_priv->stp_state = state;
> +
> +	return 0;
> +}
> +
> +static int ethsw_dellink_switch(struct ethsw_core *ethsw, u16 vid)
> +{
> +	struct ethsw_port_priv *ppriv_local = NULL;
> +	int i, err;
> +
> +	if (!ethsw->vlans[vid])
> +		return -ENOENT;
> +
> +	err = dpsw_vlan_remove(ethsw->mc_io, 0, ethsw->dpsw_handle, vid);
> +	if (err) {
> +		dev_err(ethsw->dev, "dpsw_vlan_remove err %d\n", err);
> +		return err;
> +	}
> +	ethsw->vlans[vid] = 0;
> +
> +	for (i = 0; i < ethsw->sw_attr.num_ifs; i++) {
> +		ppriv_local = ethsw->ports[i];
> +		ppriv_local->vlans[vid] = 0;
> +	}
> +
> +	return 0;
> +}
> +
> +static int ethsw_port_fdb_add_uc(struct ethsw_port_priv *port_priv,
> +				 const unsigned char *addr)
> +{
> +	struct dpsw_fdb_unicast_cfg entry = {0};
> +	int err;
> +
> +	entry.if_egress = port_priv->idx;
> +	entry.type = DPSW_FDB_ENTRY_STATIC;
> +	ether_addr_copy(entry.mac_addr, addr);
> +
> +	err = dpsw_fdb_add_unicast(port_priv->ethsw_data->mc_io, 0,
> +				   port_priv->ethsw_data->dpsw_handle,
> +				   0, &entry);
> +	if (err)
> +		netdev_err(port_priv->netdev,
> +			   "dpsw_fdb_add_unicast err %d\n", err);
> +	return err;
> +}
> +
> +static int ethsw_port_fdb_del_uc(struct ethsw_port_priv *port_priv,
> +				 const unsigned char *addr)
> +{
> +	struct dpsw_fdb_unicast_cfg entry = {0};
> +	int err;
> +
> +	entry.if_egress = port_priv->idx;
> +	entry.type = DPSW_FDB_ENTRY_STATIC;
> +	ether_addr_copy(entry.mac_addr, addr);
> +
> +	err = dpsw_fdb_remove_unicast(port_priv->ethsw_data->mc_io, 0,
> +				      port_priv->ethsw_data->dpsw_handle,
> +				      0, &entry);
> +	if (err)
> +		netdev_err(port_priv->netdev,
> +			   "dpsw_fdb_remove_unicast err %d\n", err);
> +	return err;
> +}
> +
> +static int ethsw_port_fdb_add_mc(struct ethsw_port_priv *port_priv,
> +				 const unsigned char *addr)
> +{
> +	struct dpsw_fdb_multicast_cfg entry = {0};
> +	int err;
> +
> +	ether_addr_copy(entry.mac_addr, addr);
> +	entry.type = DPSW_FDB_ENTRY_STATIC;
> +	entry.num_ifs = 1;
> +	entry.if_id[0] = port_priv->idx;
> +
> +	err = dpsw_fdb_add_multicast(port_priv->ethsw_data->mc_io, 0,
> +				     port_priv->ethsw_data->dpsw_handle,
> +				     0, &entry);
> +	if (err)
> +		netdev_err(port_priv->netdev, "dpsw_fdb_add_multicast err %d\n",
> +			   err);
> +	return err;
> +}
> +
> +static int ethsw_port_fdb_del_mc(struct ethsw_port_priv *port_priv,
> +				 const unsigned char *addr)
> +{
> +	struct dpsw_fdb_multicast_cfg entry = {0};
> +	int err;
> +
> +	ether_addr_copy(entry.mac_addr, addr);
> +	entry.type = DPSW_FDB_ENTRY_STATIC;
> +	entry.num_ifs = 1;
> +	entry.if_id[0] = port_priv->idx;
> +
> +	err = dpsw_fdb_remove_multicast(port_priv->ethsw_data->mc_io, 0,
> +					port_priv->ethsw_data->dpsw_handle,
> +					0, &entry);
> +	if (err)
> +		netdev_err(port_priv->netdev,
> +			   "dpsw_fdb_remove_multicast err %d\n", err);
> +	return err;
> +}
> +
> +static void port_get_stats(struct net_device *netdev,
> +			   struct rtnl_link_stats64 *stats)
> +{
> +	struct ethsw_port_priv *port_priv = netdev_priv(netdev);
> +	u64 tmp;
> +	int err;
> +
> +	err = dpsw_if_get_counter(port_priv->ethsw_data->mc_io, 0,
> +				  port_priv->ethsw_data->dpsw_handle,
> +				  port_priv->idx,
> +				  DPSW_CNT_ING_FRAME, &stats->rx_packets);
> +	if (err)
> +		goto error;
> +
> +	err = dpsw_if_get_counter(port_priv->ethsw_data->mc_io, 0,
> +				  port_priv->ethsw_data->dpsw_handle,
> +				  port_priv->idx,
> +				  DPSW_CNT_EGR_FRAME, &stats->tx_packets);
> +	if (err)
> +		goto error;
> +
> +	err = dpsw_if_get_counter(port_priv->ethsw_data->mc_io, 0,
> +				  port_priv->ethsw_data->dpsw_handle,
> +				  port_priv->idx,
> +				  DPSW_CNT_ING_BYTE, &stats->rx_bytes);
> +	if (err)
> +		goto error;
> +
> +	err = dpsw_if_get_counter(port_priv->ethsw_data->mc_io, 0,
> +				  port_priv->ethsw_data->dpsw_handle,
> +				  port_priv->idx,
> +				  DPSW_CNT_EGR_BYTE, &stats->tx_bytes);
> +	if (err)
> +		goto error;
> +
> +	err = dpsw_if_get_counter(port_priv->ethsw_data->mc_io, 0,
> +				  port_priv->ethsw_data->dpsw_handle,
> +				  port_priv->idx,
> +				  DPSW_CNT_ING_FRAME_DISCARD,
> +				  &stats->rx_dropped);
> +	if (err)
> +		goto error;
> +
> +	err = dpsw_if_get_counter(port_priv->ethsw_data->mc_io, 0,
> +				  port_priv->ethsw_data->dpsw_handle,
> +				  port_priv->idx,
> +				  DPSW_CNT_ING_FLTR_FRAME,
> +				  &tmp);
> +	if (err)
> +		goto error;
> +	stats->rx_dropped += tmp;
> +
> +	err = dpsw_if_get_counter(port_priv->ethsw_data->mc_io, 0,
> +				  port_priv->ethsw_data->dpsw_handle,
> +				  port_priv->idx,
> +				  DPSW_CNT_EGR_FRAME_DISCARD,
> +				  &stats->tx_dropped);
> +	if (err)
> +		goto error;
> +
> +	return;
> +
> +error:
> +	netdev_err(netdev, "dpsw_if_get_counter err %d\n", err);
> +}
> +
> +static bool port_has_offload_stats(const struct net_device *netdev,
> +				   int attr_id)
> +{
> +	return (attr_id == IFLA_OFFLOAD_XSTATS_CPU_HIT);
> +}
> +
> +static int port_get_offload_stats(int attr_id,
> +				  const struct net_device *netdev,
> +				  void *sp)
> +{
> +	switch (attr_id) {
> +	case IFLA_OFFLOAD_XSTATS_CPU_HIT:
> +		port_get_stats((struct net_device *)netdev, sp);
> +		return 0;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static int port_change_mtu(struct net_device *netdev, int mtu)
> +{
> +	struct ethsw_port_priv *port_priv = netdev_priv(netdev);
> +	int err;
> +
> +	err = dpsw_if_set_max_frame_length(port_priv->ethsw_data->mc_io,
> +					   0,
> +					   port_priv->ethsw_data->dpsw_handle,
> +					   port_priv->idx,
> +					   (u16)ETHSW_L2_MAX_FRM(mtu));
> +	if (err) {
> +		netdev_err(netdev,
> +			   "dpsw_if_set_max_frame_length() err %d\n", err);
> +		return err;
> +	}
> +
> +	netdev->mtu = mtu;
> +	return 0;
> +}
> +
> +static int port_carrier_state_sync(struct net_device *netdev)
> +{
> +	struct ethsw_port_priv *port_priv = netdev_priv(netdev);
> +	struct dpsw_link_state state;
> +	int err;
> +
> +	err = dpsw_if_get_link_state(port_priv->ethsw_data->mc_io, 0,
> +				     port_priv->ethsw_data->dpsw_handle,
> +				     port_priv->idx, &state);
> +	if (err) {
> +		netdev_err(netdev, "dpsw_if_get_link_state() err %d\n", err);
> +		return err;
> +	}
> +
> +	WARN_ONCE(state.up > 1, "Garbage read into link_state");
> +
> +	if (state.up != port_priv->link_state) {
> +		if (state.up)
> +			netif_carrier_on(netdev);
> +		else
> +			netif_carrier_off(netdev);
> +		port_priv->link_state = state.up;
> +	}
> +	return 0;
> +}
> +
> +static int port_open(struct net_device *netdev)
> +{
> +	struct ethsw_port_priv *port_priv = netdev_priv(netdev);
> +	int err;
> +
> +	/* No need to allow Tx as control interface is disabled */
> +	netif_tx_stop_all_queues(netdev);
> +
> +	err = dpsw_if_enable(port_priv->ethsw_data->mc_io, 0,
> +			     port_priv->ethsw_data->dpsw_handle,
> +			     port_priv->idx);
> +	if (err) {
> +		netdev_err(netdev, "dpsw_if_enable err %d\n", err);
> +		return err;
> +	}
> +
> +	/* sync carrier state */
> +	err = port_carrier_state_sync(netdev);
> +	if (err) {
> +		netdev_err(netdev,
> +			   "port_carrier_state_sync err %d\n", err);
> +		goto err_carrier_sync;
> +	}
> +
> +	return 0;
> +
> +err_carrier_sync:
> +	dpsw_if_disable(port_priv->ethsw_data->mc_io, 0,
> +			port_priv->ethsw_data->dpsw_handle,
> +			port_priv->idx);
> +	return err;
> +}
> +
> +static int port_stop(struct net_device *netdev)
> +{
> +	struct ethsw_port_priv *port_priv = netdev_priv(netdev);
> +	int err;
> +
> +	err = dpsw_if_disable(port_priv->ethsw_data->mc_io, 0,
> +			      port_priv->ethsw_data->dpsw_handle,
> +			      port_priv->idx);
> +	if (err) {
> +		netdev_err(netdev, "dpsw_if_disable err %d\n", err);
> +		return err;
> +	}
> +
> +	return 0;
> +}
> +
> +static netdev_tx_t port_dropframe(struct sk_buff *skb,
> +				  struct net_device *netdev)
> +{
> +	/* we don't support I/O for now, drop the frame */
> +	dev_kfree_skb_any(skb);
> +
> +	return NETDEV_TX_OK;
> +}
> +
> +static const struct net_device_ops ethsw_port_ops = {
> +	.ndo_open		= port_open,
> +	.ndo_stop		= port_stop,
> +
> +	.ndo_set_mac_address	= eth_mac_addr,
> +	.ndo_change_mtu		= port_change_mtu,
> +	.ndo_has_offload_stats	= port_has_offload_stats,
> +	.ndo_get_offload_stats	= port_get_offload_stats,
> +
> +	.ndo_start_xmit		= port_dropframe,
> +};
> +
> +static void ethsw_links_state_update(struct ethsw_core *ethsw)
> +{
> +	int i;
> +
> +	for (i = 0; i < ethsw->sw_attr.num_ifs; i++)
> +		port_carrier_state_sync(ethsw->ports[i]->netdev);
> +}
> +
> +static irqreturn_t ethsw_irq0_handler(int irq_num, void *arg)
> +{
> +	return IRQ_WAKE_THREAD;
> +}
> +
> +static irqreturn_t ethsw_irq0_handler_thread(int irq_num, void *arg)
> +{
> +	struct device *dev = (struct device *)arg;
> +	struct ethsw_core *ethsw = dev_get_drvdata(dev);
> +
> +	/* Mask the events and the if_id reserved bits to be cleared on read */
> +	u32 status = DPSW_IRQ_EVENT_LINK_CHANGED | 0xFFFF0000;
> +	int err;
> +
> +	err = dpsw_get_irq_status(ethsw->mc_io, 0, ethsw->dpsw_handle,
> +				  DPSW_IRQ_INDEX_IF, &status);
> +	if (err) {
> +		dev_err(dev, "Can't get irq status (err %d)", err);
> +
> +		err = dpsw_clear_irq_status(ethsw->mc_io, 0, ethsw->dpsw_handle,
> +					    DPSW_IRQ_INDEX_IF, 0xFFFFFFFF);
> +		if (err)
> +			dev_err(dev, "Can't clear irq status (err %d)", err);
> +		goto out;
> +	}
> +
> +	if (status & DPSW_IRQ_EVENT_LINK_CHANGED)
> +		ethsw_links_state_update(ethsw);
> +
> +out:
> +	return IRQ_HANDLED;
> +}
> +
> +static int ethsw_setup_irqs(struct fsl_mc_device *sw_dev)
> +{
> +	struct device *dev = &sw_dev->dev;
> +	struct ethsw_core *ethsw = dev_get_drvdata(dev);
> +	u32 mask = DPSW_IRQ_EVENT_LINK_CHANGED;
> +	struct fsl_mc_device_irq *irq;
> +	int err;
> +
> +	err = fsl_mc_allocate_irqs(sw_dev);
> +	if (err) {
> +		dev_err(dev, "MC irqs allocation failed\n");
> +		return err;
> +	}
> +
> +	if (WARN_ON(sw_dev->obj_desc.irq_count != DPSW_IRQ_NUM)) {
> +		err = -EINVAL;
> +		goto free_irq;
> +	}
> +
> +	err = dpsw_set_irq_enable(ethsw->mc_io, 0, ethsw->dpsw_handle,
> +				  DPSW_IRQ_INDEX_IF, 0);
> +	if (err) {
> +		dev_err(dev, "dpsw_set_irq_enable err %d\n", err);
> +		goto free_irq;
> +	}
> +
> +	irq = sw_dev->irqs[DPSW_IRQ_INDEX_IF];
> +
> +	err = devm_request_threaded_irq(dev, irq->msi_desc->irq,
> +					ethsw_irq0_handler,
> +					ethsw_irq0_handler_thread,
> +					IRQF_NO_SUSPEND | IRQF_ONESHOT,
> +					dev_name(dev), dev);
> +	if (err) {
> +		dev_err(dev, "devm_request_threaded_irq(): %d", err);
> +		goto free_irq;
> +	}
> +
> +	err = dpsw_set_irq_mask(ethsw->mc_io, 0, ethsw->dpsw_handle,
> +				DPSW_IRQ_INDEX_IF, mask);
> +	if (err) {
> +		dev_err(dev, "dpsw_set_irq_mask(): %d", err);
> +		goto free_devm_irq;
> +	}
> +
> +	err = dpsw_set_irq_enable(ethsw->mc_io, 0, ethsw->dpsw_handle,
> +				  DPSW_IRQ_INDEX_IF, 1);
> +	if (err) {
> +		dev_err(dev, "dpsw_set_irq_enable(): %d", err);
> +		goto free_devm_irq;
> +	}
> +
> +	return 0;
> +
> +free_devm_irq:
> +	devm_free_irq(dev, irq->msi_desc->irq, dev);
> +free_irq:
> +	fsl_mc_free_irqs(sw_dev);
> +	return err;
> +}
> +
> +static void ethsw_teardown_irqs(struct fsl_mc_device *sw_dev)
> +{
> +	struct device *dev = &sw_dev->dev;
> +	struct ethsw_core *ethsw = dev_get_drvdata(dev);
> +	struct fsl_mc_device_irq *irq;
> +
> +	irq = sw_dev->irqs[DPSW_IRQ_INDEX_IF];
> +	dpsw_set_irq_enable(ethsw->mc_io, 0, ethsw->dpsw_handle,
> +			    DPSW_IRQ_INDEX_IF, 0);

You can still print an error message here, in case something goes wrong.

> +	fsl_mc_free_irqs(sw_dev);
> +}
> +
> +static int swdev_port_attr_get(struct net_device *netdev,
> +			       struct switchdev_attr *attr)
> +{
> +	struct ethsw_port_priv *port_priv = netdev_priv(netdev);
> +
> +	switch (attr->id) {
> +	case SWITCHDEV_ATTR_ID_PORT_PARENT_ID:
> +		attr->u.ppid.id_len = 1;
> +		attr->u.ppid.id[0] = port_priv->ethsw_data->dev_id;
> +		break;
> +	case SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS:
> +		attr->u.brport_flags =
> +			(port_priv->ethsw_data->learning ? BR_LEARNING : 0) |
> +			(port_priv->flood ? BR_FLOOD : 0);
> +		break;
> +	case SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS_SUPPORT:
> +		attr->u.brport_flags_support = BR_LEARNING | BR_FLOOD;
> +		break;
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +
> +	return 0;
> +}
> +
> +static int port_attr_stp_state_set(struct net_device *netdev,
> +				   struct switchdev_trans *trans,
> +				   u8 state)
> +{
> +	struct ethsw_port_priv *port_priv = netdev_priv(netdev);
> +
> +	if (switchdev_trans_ph_prepare(trans))
> +		return 0;
> +
> +	return ethsw_port_set_stp_state(port_priv, state);
> +}
> +
> +static int port_attr_br_flags_set(struct net_device *netdev,
> +				  struct switchdev_trans *trans,
> +				  unsigned long flags)
> +{
> +	struct ethsw_port_priv *port_priv = netdev_priv(netdev);
> +	int err = 0;
> +
> +	if (switchdev_trans_ph_prepare(trans))
> +		return 0;
> +
> +	/* Learning is enabled per switch */
> +	err = ethsw_set_learning(port_priv->ethsw_data, flags & BR_LEARNING);
> +	if (err)
> +		goto exit;
> +
> +	err = ethsw_port_set_flood(port_priv, flags & BR_FLOOD);
> +
> +exit:
> +	return err;
> +}
> +
> +static int swdev_port_attr_set(struct net_device *netdev,
> +			       const struct switchdev_attr *attr,
> +			       struct switchdev_trans *trans)
> +{
> +	int err = 0;
> +
> +	switch (attr->id) {
> +	case SWITCHDEV_ATTR_ID_PORT_STP_STATE:
> +		err = port_attr_stp_state_set(netdev, trans,
> +					      attr->u.stp_state);
> +		break;
> +	case SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS:
> +		err = port_attr_br_flags_set(netdev, trans,
> +					     attr->u.brport_flags);
> +		break;
> +	case SWITCHDEV_ATTR_ID_BRIDGE_VLAN_FILTERING:
> +		/* VLANs are supported by default  */
> +		break;
> +	default:
> +		err = -EOPNOTSUPP;
> +		break;
> +	}
> +
> +	return err;
> +}
> +
> +static int port_vlans_add(struct net_device *netdev,
> +			  const struct switchdev_obj_port_vlan *vlan,
> +			  struct switchdev_trans *trans)
> +{
> +	struct ethsw_port_priv *port_priv = netdev_priv(netdev);
> +	int vid, err;
> +
> +	if (switchdev_trans_ph_prepare(trans))
> +		return 0;
> +
> +	for (vid = vlan->vid_begin; vid <= vlan->vid_end; vid++) {
> +		if (!port_priv->ethsw_data->vlans[vid]) {
> +			/* this is a new VLAN */
> +			err = ethsw_add_vlan(port_priv->ethsw_data, vid);
> +			if (err)
> +				return err;
> +
> +			port_priv->ethsw_data->vlans[vid] |= ETHSW_VLAN_GLOBAL;
> +		}
> +		err = ethsw_port_add_vlan(port_priv, vid, vlan->flags);
> +		if (err)
> +			break;
> +	}
> +
> +	return err;
> +}
> +
> +static int port_lookup_address(struct net_device *netdev, int is_uc,
> +			       const unsigned char *addr)
> +{
> +	struct netdev_hw_addr_list *list = (is_uc) ? &netdev->uc : &netdev->mc;
> +	struct netdev_hw_addr *ha;
> +
> +	netif_addr_lock_bh(netdev);
> +	list_for_each_entry(ha, &list->list, list) {
> +		if (ether_addr_equal(ha->addr, addr)) {
> +			netif_addr_unlock_bh(netdev);
> +			return 1;
> +		}
> +	}
> +	netif_addr_unlock_bh(netdev);
> +	return 0;
> +}
> +
> +static int port_mdb_add(struct net_device *netdev,
> +			const struct switchdev_obj_port_mdb *mdb,
> +			struct switchdev_trans *trans)
> +{
> +	struct ethsw_port_priv *port_priv = netdev_priv(netdev);
> +	int err;
> +
> +	if (switchdev_trans_ph_prepare(trans))
> +		return 0;
> +
> +	/* Check if address is already set on this port */
> +	if (port_lookup_address(netdev, 0, mdb->addr))
> +		return -EEXIST;
> +
> +	err = ethsw_port_fdb_add_mc(port_priv, mdb->addr);
> +	if (err)
> +		return err;
> +
> +	err = dev_mc_add(netdev, mdb->addr);
> +	if (err)
> +		netdev_err(netdev, "dev_mc_add err %d\n", err);

In the error case, shouldn't there be a "ethsw_port_fdb_del_mc" ?

> +
> +	return err;
> +}
> +
> +static int swdev_port_obj_add(struct net_device *netdev,
> +			      const struct switchdev_obj *obj,
> +			      struct switchdev_trans *trans)
> +{
> +	int err;
> +
> +	switch (obj->id) {
> +	case SWITCHDEV_OBJ_ID_PORT_VLAN:
> +		err = port_vlans_add(netdev,
> +				     SWITCHDEV_OBJ_PORT_VLAN(obj),
> +				     trans);
> +		break;
> +	case SWITCHDEV_OBJ_ID_PORT_MDB:
> +		err = port_mdb_add(netdev,
> +				   SWITCHDEV_OBJ_PORT_MDB(obj),
> +				   trans);
> +		break;
> +	default:
> +		err = -EOPNOTSUPP;
> +		break;
> +	}
> +
> +	return err;
> +}
> +
> +static int ethsw_port_del_vlan(struct ethsw_port_priv *port_priv, u16 vid)
> +{
> +	struct ethsw_core *ethsw = port_priv->ethsw_data;
> +	struct net_device *netdev = port_priv->netdev;
> +	struct dpsw_vlan_if_cfg vcfg;
> +	int i, err, err2;
> +	bool is_oper;
> +
> +	if (!port_priv->vlans[vid])
> +		return -ENOENT;
> +
> +	if (port_priv->vlans[vid] & ETHSW_VLAN_PVID) {
> +		struct dpsw_tci_cfg tci_cfg = { 0 };
> +		/* Interface needs to be down to change PVID */
> +		is_oper = netif_oper_up(netdev);
> +
> +		if (is_oper) {
> +			err = dpsw_if_disable(ethsw->mc_io, 0,
> +					      ethsw->dpsw_handle,
> +					      port_priv->idx);
> +			if (err) {
> +				netdev_err(netdev, "dpsw_if_disable err %d\n",
> +					   err);
> +				goto exit_err;
> +			}
> +		}
> +
> +		err = dpsw_if_set_tci(ethsw->mc_io, 0,
> +				      ethsw->dpsw_handle,
> +				      port_priv->idx, &tci_cfg);
> +		if (!err) {
> +			port_priv->vlans[vid] &= ~ETHSW_VLAN_PVID;
> +			port_priv->pvid = 0;
> +		} else {
> +			netdev_err(netdev, "dpsw_if_set_tci err %d\n", err);
> +		}
> +
> +		if (is_oper) {
> +			err2 = dpsw_if_enable(ethsw->mc_io, 0,
> +					      ethsw->dpsw_handle,
> +					      port_priv->idx);
> +			if (err2) {
> +				netdev_err(netdev, "dpsw_if_enable err %d\n",
> +					   err2);
> +				return err2;
> +			}
> +		}
> +
> +		if (err)
> +			goto exit_err;
> +	}
> +
> +	vcfg.num_ifs = 1;
> +	vcfg.if_id[0] = port_priv->idx;
> +	if (port_priv->vlans[vid] & ETHSW_VLAN_UNTAGGED) {
> +		err = dpsw_vlan_remove_if_untagged(ethsw->mc_io, 0,
> +						   ethsw->dpsw_handle,
> +						   vid, &vcfg);
> +		if (err) {
> +			netdev_err(netdev,
> +				   "dpsw_vlan_remove_if_untagged err %d\n",
> +				   err);
> +		}
> +		port_priv->vlans[vid] &= ~ETHSW_VLAN_UNTAGGED;
> +	}
> +
> +	if (port_priv->vlans[vid] & ETHSW_VLAN_MEMBER) {
> +		err = dpsw_vlan_remove_if(ethsw->mc_io, 0, ethsw->dpsw_handle,
> +					  vid, &vcfg);
> +		if (err) {
> +			netdev_err(netdev,
> +				   "dpsw_vlan_remove_if err %d\n", err);
> +			return err;
> +		}
> +		port_priv->vlans[vid] &= ~ETHSW_VLAN_MEMBER;
> +
> +		/* Delete VLAN from switch if it is no longer configured on
> +		 * any port
> +		 */
> +		for (i = 0; i < ethsw->sw_attr.num_ifs; i++)
> +			if (ethsw->ports[i]->vlans[vid] & ETHSW_VLAN_MEMBER)
> +				return 0; /* Found a port member in VID */
> +
> +		ethsw->vlans[vid] &= ~ETHSW_VLAN_GLOBAL;
> +
> +		err = ethsw_dellink_switch(ethsw, vid);
> +		if (err)
> +			goto exit_err;
> +	}
> +
> +	return 0;
> +exit_err:
> +	return err;
> +}
> +
> +static int port_vlans_del(struct net_device *netdev,
> +			  const struct switchdev_obj_port_vlan *vlan)
> +{
> +	struct ethsw_port_priv *port_priv = netdev_priv(netdev);
> +	int vid, err;
> +
> +	for (vid = vlan->vid_begin; vid <= vlan->vid_end; vid++) {
> +		err = ethsw_port_del_vlan(port_priv, vid);
> +		if (err)
> +			break;
> +	}
> +
> +	return err;
> +}
> +
> +static int port_mdb_del(struct net_device *netdev,
> +			const struct switchdev_obj_port_mdb *mdb)
> +{
> +	struct ethsw_port_priv *port_priv = netdev_priv(netdev);
> +	int err;
> +
> +	if (!port_lookup_address(netdev, 0, mdb->addr))
> +		return -ENOENT;
> +
> +	err = ethsw_port_fdb_del_mc(port_priv, mdb->addr);
> +	if (err)
> +		return err;
> +
> +	err = dev_mc_del(netdev, mdb->addr);
> +	if (err) {
> +		netdev_err(netdev, "dev_mc_del err %d\n", err);
> +		return err;
> +	}
> +
> +	return err;
> +}
> +
> +static int swdev_port_obj_del(struct net_device *netdev,
> +			      const struct switchdev_obj *obj)
> +{
> +	int err;
> +
> +	switch (obj->id) {
> +	case SWITCHDEV_OBJ_ID_PORT_VLAN:
> +		err = port_vlans_del(netdev, SWITCHDEV_OBJ_PORT_VLAN(obj));
> +		break;
> +	case SWITCHDEV_OBJ_ID_PORT_MDB:
> +		err = port_mdb_del(netdev, SWITCHDEV_OBJ_PORT_MDB(obj));
> +		break;
> +	default:
> +		err = -EOPNOTSUPP;
> +		break;
> +	}
> +	return err;
> +}
> +
> +static const struct switchdev_ops ethsw_port_switchdev_ops = {
> +	.switchdev_port_attr_get	= swdev_port_attr_get,
> +	.switchdev_port_attr_set	= swdev_port_attr_set,
> +	.switchdev_port_obj_add		= swdev_port_obj_add,
> +	.switchdev_port_obj_del		= swdev_port_obj_del,
> +};
> +
> +/* For the moment, only flood setting needs to be updated */
> +static int port_bridge_join(struct net_device *netdev)
> +{
> +	struct ethsw_port_priv *port_priv = netdev_priv(netdev);
> +
> +	/* Enable flooding */
> +	return ethsw_port_set_flood(port_priv, 1);
> +}
> +
> +static int port_bridge_leave(struct net_device *netdev)
> +{
> +	struct ethsw_port_priv *port_priv = netdev_priv(netdev);
> +
> +	/* Disable flooding */
> +	return ethsw_port_set_flood(port_priv, 0);
> +}
> +
> +static int port_netdevice_event(struct notifier_block *unused,
> +				unsigned long event, void *ptr)
> +{
> +	struct net_device *netdev = netdev_notifier_info_to_dev(ptr);
> +	struct netdev_notifier_changeupper_info *info = ptr;
> +	struct net_device *upper_dev;
> +	int err = 0;
> +
> +	if (netdev->netdev_ops != &ethsw_port_ops)
> +		return NOTIFY_DONE;
> +
> +	/* Handle just upper dev link/unlink for the moment */
> +	if (event == NETDEV_CHANGEUPPER) {
> +		upper_dev = info->upper_dev;
> +		if (netif_is_bridge_master(upper_dev)) {
> +			if (info->linking)
> +				err = port_bridge_join(netdev);
> +			else
> +				err = port_bridge_leave(netdev);
> +		}
> +	}
> +
> +	return notifier_from_errno(err);
> +}
> +
> +static struct notifier_block port_nb __read_mostly = {
> +	.notifier_call = port_netdevice_event,
> +};
> +
> +struct ethsw_switchdev_event_work {
> +	struct work_struct work;
> +	struct switchdev_notifier_fdb_info fdb_info;
> +	struct net_device *dev;
> +	unsigned long event;
> +};
> +
> +static void ethsw_switchdev_event_work(struct work_struct *work)
> +{
> +	struct ethsw_switchdev_event_work *switchdev_work =
> +		container_of(work, struct ethsw_switchdev_event_work, work);
> +	struct net_device *dev = switchdev_work->dev;
> +	struct switchdev_notifier_fdb_info *fdb_info;
> +	struct ethsw_port_priv *port_priv;
> +
> +	rtnl_lock();
> +	port_priv = netdev_priv(dev);
> +	fdb_info = &switchdev_work->fdb_info;
> +
> +	switch (switchdev_work->event) {
> +	case SWITCHDEV_FDB_ADD_TO_DEVICE:
> +		ethsw_port_fdb_add_uc(netdev_priv(dev), fdb_info->addr);
> +		break;
> +	case SWITCHDEV_FDB_DEL_TO_DEVICE:
> +		ethsw_port_fdb_del_uc(netdev_priv(dev), fdb_info->addr);
> +		break;
> +	}
> +
> +	rtnl_unlock();
> +	kfree(switchdev_work->fdb_info.addr);
> +	kfree(switchdev_work);
> +	dev_put(dev);
> +}
> +
> +/* Called under rcu_read_lock() */
> +static int port_switchdev_event(struct notifier_block *unused,
> +				unsigned long event, void *ptr)
> +{
> +	struct net_device *dev = switchdev_notifier_info_to_dev(ptr);
> +	struct ethsw_switchdev_event_work *switchdev_work;
> +	struct switchdev_notifier_fdb_info *fdb_info = ptr;
> +
> +	switchdev_work = kzalloc(sizeof(*switchdev_work), GFP_ATOMIC);
> +	if (!switchdev_work)
> +		return NOTIFY_BAD;
> +
> +	INIT_WORK(&switchdev_work->work, ethsw_switchdev_event_work);
> +	switchdev_work->dev = dev;
> +	switchdev_work->event = event;
> +
> +	switch (event) {
> +	case SWITCHDEV_FDB_ADD_TO_DEVICE:
> +	case SWITCHDEV_FDB_DEL_TO_DEVICE:
> +		memcpy(&switchdev_work->fdb_info, ptr,
> +		       sizeof(switchdev_work->fdb_info));
> +		switchdev_work->fdb_info.addr = kzalloc(ETH_ALEN, GFP_ATOMIC);
> +		if (!switchdev_work->fdb_info.addr)
> +			goto err_addr_alloc;
> +
> +		ether_addr_copy((u8 *)switchdev_work->fdb_info.addr,
> +				fdb_info->addr);
> +
> +		/* Take a reference on the device to avoid being freed. */
> +		dev_hold(dev);
> +		break;
> +	default:
> +		return NOTIFY_DONE;
> +	}
> +
> +	queue_work(ethsw_owq, &switchdev_work->work);
> +
> +	return NOTIFY_DONE;
> +
> +err_addr_alloc:
> +	kfree(switchdev_work);
> +	return NOTIFY_BAD;
> +}
> +
> +static struct notifier_block port_switchdev_nb = {
> +	.notifier_call = port_switchdev_event,
> +};
> +
> +static int ethsw_register_notifier(struct device *dev)
> +{
> +	int err;
> +
> +	err = register_netdevice_notifier(&port_nb);
> +	if (err) {
> +		dev_err(dev, "Failed to register netdev notifier\n");
> +		return err;
> +	}
> +
> +	err = register_switchdev_notifier(&port_switchdev_nb);
> +	if (err) {
> +		dev_err(dev, "Failed to register switchdev notifier\n");
> +		goto err_switchdev_nb;
> +	}
> +
> +	return 0;
> +
> +err_switchdev_nb:
> +	unregister_netdevice_notifier(&port_nb);
> +	return err;
> +}
> +
> +static int ethsw_open(struct ethsw_core	*ethsw)

Minor formatting error, tab in function signature - see following function as well.

> +{
> +	struct ethsw_port_priv *port_priv = NULL;
> +	int i, err;
> +
> +	err = dpsw_enable(ethsw->mc_io, 0, ethsw->dpsw_handle);
> +	if (err) {
> +		dev_err(ethsw->dev, "dpsw_enable err %d\n", err);
> +		return err;
> +	}
> +
> +	for (i = 0; i < ethsw->sw_attr.num_ifs; i++) {
> +		port_priv = ethsw->ports[i];
> +		err = dev_open(port_priv->netdev);
> +		if (err) {
> +			netdev_err(port_priv->netdev, "dev_open err %d\n", err);
> +			return err;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static int ethsw_stop(struct ethsw_core	*ethsw)
> +{
> +	struct ethsw_port_priv *port_priv = NULL;
> +	int i, err;
> +
> +	destroy_workqueue(ethsw_owq);

If workqueue is destroyed here, shouldn't it be alloc'd in ethsw_open?

> +
> +	for (i = 0; i < ethsw->sw_attr.num_ifs; i++) {
> +		port_priv = ethsw->ports[i];
> +		dev_close(port_priv->netdev);
> +	}
> +
> +	err = dpsw_disable(ethsw->mc_io, 0, ethsw->dpsw_handle);
> +	if (err) {
> +		dev_err(ethsw->dev, "dpsw_disable err %d\n", err);
> +		return err;
> +	}
> +
> +	return 0;
> +}
> +
> +static int ethsw_init(struct fsl_mc_device *sw_dev)
> +{
> +	struct device *dev = &sw_dev->dev;
> +	struct ethsw_core *ethsw = dev_get_drvdata(dev);
> +	u16 version_major, version_minor, i;
> +	struct dpsw_stp_cfg stp_cfg;
> +	int err;
> +
> +	ethsw->dev_id = sw_dev->obj_desc.id;
> +
> +	err = dpsw_open(ethsw->mc_io, 0, ethsw->dev_id, &ethsw->dpsw_handle);
> +	if (err) {
> +		dev_err(dev, "dpsw_open err %d\n", err);
> +		return err;
> +	}
> +
> +	err = dpsw_get_attributes(ethsw->mc_io, 0, ethsw->dpsw_handle,
> +				  &ethsw->sw_attr);
> +	if (err) {
> +		dev_err(dev, "dpsw_get_attributes err %d\n", err);
> +		goto err_close;
> +	}
> +
> +	err = dpsw_get_api_version(ethsw->mc_io, 0,
> +				   &version_major,
> +				   &version_minor);
> +	if (err) {
> +		dev_err(dev, "dpsw_get_api_version err %d\n", err);
> +		goto err_close;
> +	}
> +
> +	/* Minimum supported DPSW version check */
> +	if (version_major < DPSW_MIN_VER_MAJOR ||
> +	    (version_major == DPSW_MIN_VER_MAJOR &&
> +	     version_minor < DPSW_MIN_VER_MINOR)) {
> +		dev_err(dev, "DPSW version %d:%d not supported. Use %d.%d or
> greater.\n",
> +			version_major,
> +			version_minor,
> +			DPSW_MIN_VER_MAJOR, DPSW_MIN_VER_MINOR);
> +		err = -ENOTSUPP;
> +		goto err_close;
> +	}
> +
> +	err = dpsw_reset(ethsw->mc_io, 0, ethsw->dpsw_handle);
> +	if (err) {
> +		dev_err(dev, "dpsw_reset err %d\n", err);
> +		goto err_close;
> +	}
> +
> +	err = dpsw_fdb_set_learning_mode(ethsw->mc_io, 0, ethsw->dpsw_handle, 0,
> +					 DPSW_FDB_LEARNING_MODE_HW);
> +	if (err) {
> +		dev_err(dev, "dpsw_fdb_set_learning_mode err %d\n", err);
> +		goto err_close;
> +	}
> +
> +	stp_cfg.vlan_id = DEFAULT_VLAN_ID;
> +	stp_cfg.state = DPSW_STP_STATE_FORWARDING;
> +
> +	for (i = 0; i < ethsw->sw_attr.num_ifs; i++) {
> +		err = dpsw_if_set_stp(ethsw->mc_io, 0, ethsw->dpsw_handle, i,
> +				      &stp_cfg);
> +		if (err) {
> +			dev_err(dev, "dpsw_if_set_stp err %d for port %d\n",
> +				err, i);
> +			goto err_close;
> +		}
> +
> +		err = dpsw_if_set_broadcast(ethsw->mc_io, 0,
> +					    ethsw->dpsw_handle, i, 1);
> +		if (err) {
> +			dev_err(dev,
> +				"dpsw_if_set_broadcast err %d for port %d\n",
> +				err, i);
> +			goto err_close;
> +		}
> +	}
> +
> +	ethsw_owq = alloc_ordered_workqueue("%s_ordered", WQ_MEM_RECLAIM,
> +					    "ethsw");
> +	if (!ethsw_owq) {
> +		err = -ENOMEM;
> +		goto err_close;
> +	}
> +
> +	err = ethsw_register_notifier(dev);
> +	if (err)
> +		goto err_destroy_ordered_workqueue;
> +
> +	return 0;
> +
> +err_destroy_ordered_workqueue:
> +	destroy_workqueue(ethsw_owq);
> +
> +err_close:
> +	dpsw_close(ethsw->mc_io, 0, ethsw->dpsw_handle);
> +	return err;
> +}
> +
> +static int ethsw_port_init(struct ethsw_port_priv *port_priv, u16 port)
> +{
> +	const char def_mcast[ETH_ALEN] = {0x01, 0x00, 0x5e, 0x00, 0x00, 0x01};
> +	struct net_device *netdev = port_priv->netdev;
> +	struct ethsw_core *ethsw = port_priv->ethsw_data;
> +	struct dpsw_tci_cfg tci_cfg = {0};
> +	struct dpsw_vlan_if_cfg vcfg;
> +	int err;
> +
> +	/* Switch starts with all ports configured to VLAN 1. Need to
> +	 * remove this setting to allow configuration at bridge join
> +	 */
> +	vcfg.num_ifs = 1;
> +	vcfg.if_id[0] = port_priv->idx;
> +
> +	err = dpsw_vlan_remove_if_untagged(ethsw->mc_io, 0, ethsw->dpsw_handle,
> +					   DEFAULT_VLAN_ID, &vcfg);
> +	if (err) {
> +		netdev_err(netdev, "dpsw_vlan_remove_if_untagged err %d\n",
> +			   err);
> +		return err;
> +	}
> +
> +	err = dpsw_if_set_tci(ethsw->mc_io, 0, ethsw->dpsw_handle,
> +			      port_priv->idx, &tci_cfg);
> +	if (err) {
> +		netdev_err(netdev, "dpsw_if_set_tci err %d\n", err);
> +		return err;
> +	}
> +
> +	err = dpsw_vlan_remove_if(ethsw->mc_io, 0, ethsw->dpsw_handle,
> +				  DEFAULT_VLAN_ID, &vcfg);
> +	if (err) {
> +		netdev_err(netdev, "dpsw_vlan_remove_if err %d\n", err);
> +		return err;
> +	}
> +
> +	err = ethsw_port_fdb_add_mc(port_priv, def_mcast);
> +
> +	return err;
> +}
> +
> +static void ethsw_takedown(struct fsl_mc_device *sw_dev)
> +{
> +	struct device *dev = &sw_dev->dev;
> +	struct ethsw_core *ethsw = dev_get_drvdata(dev);
> +	int err;
> +
> +	err = unregister_switchdev_notifier(&port_switchdev_nb);
> +	if (err)
> +		dev_err(dev,
> +			"Failed to unregister switchdev notifier (%d)\n", err);
> +
> +	err = unregister_netdevice_notifier(&port_nb);
> +	if (err)
> +		dev_err(dev,
> +			"Failed to unregister netdev notifier (%d)\n", err);

Above 2 can be grouped into ethsw_unregister_notifier.

> +
> +	err = dpsw_close(ethsw->mc_io, 0, ethsw->dpsw_handle);
> +	if (err)
> +		dev_warn(dev, "dpsw_close err %d\n", err);
> +}
> +
> +static int ethsw_remove(struct fsl_mc_device *sw_dev)
> +{
> +	struct ethsw_port_priv *port_priv;
> +	struct ethsw_core *ethsw;
> +	struct device *dev;
> +	int i;
> +
> +	dev = &sw_dev->dev;
> +	ethsw = dev_get_drvdata(dev);
> +
> +	ethsw_teardown_irqs(sw_dev);
> +
> +	rtnl_lock();
> +	ethsw_stop(ethsw);
> +	rtnl_unlock();
> +
> +	for (i = 0; i < ethsw->sw_attr.num_ifs; i++) {
> +		port_priv = ethsw->ports[i];
> +		unregister_netdev(port_priv->netdev);
> +		free_netdev(port_priv->netdev);
> +	}
> +	kfree(ethsw->ports);
> +
> +	ethsw_takedown(sw_dev);
> +	fsl_mc_portal_free(ethsw->mc_io);
> +
> +	kfree(ethsw);
> +
> +	dev_set_drvdata(dev, NULL);
> +
> +	return 0;
> +}
> +
> +static int ethsw_probe_port(struct ethsw_core *ethsw, u16 port_idx)
> +{
> +	struct ethsw_port_priv *port_priv;
> +	struct device *dev = ethsw->dev;
> +	struct net_device *port_netdev;
> +	int err;
> +
> +	port_netdev = alloc_etherdev(sizeof(struct ethsw_port_priv));
> +	if (!port_netdev) {
> +		dev_err(dev, "alloc_etherdev error\n");
> +		return -ENOMEM;
> +	}
> +
> +	port_priv = netdev_priv(port_netdev);
> +	port_priv->netdev = port_netdev;
> +	port_priv->ethsw_data = ethsw;
> +
> +	port_priv->idx = port_idx;
> +	port_priv->stp_state = BR_STATE_FORWARDING;
> +
> +	/* Flooding is implicitly enabled */
> +	port_priv->flood = true;
> +
> +	SET_NETDEV_DEV(port_netdev, dev);
> +	port_netdev->netdev_ops = &ethsw_port_ops;
> +	port_netdev->switchdev_ops = &ethsw_port_switchdev_ops;
> +
> +	/* Set MTU limits */
> +	port_netdev->min_mtu = ETH_MIN_MTU;
> +	port_netdev->max_mtu = ETHSW_MAX_FRAME_LENGTH;
> +
> +	err = register_netdev(port_netdev);
> +	if (err < 0) {
> +		dev_err(dev, "register_netdev error %d\n", err);
> +			free_netdev(port_netdev);
> +			return err;
> +		}
> +
> +	ethsw->ports[port_idx] = port_priv;
> +
> +	return ethsw_port_init(port_priv, port_idx);
> +}
> +
> +static int ethsw_probe(struct fsl_mc_device *sw_dev)
> +{
> +	struct device *dev = &sw_dev->dev;
> +	struct ethsw_core *ethsw;
> +	int err;
> +	u16 i, j;
> +
> +	/* Allocate switch core*/
> +	ethsw = kzalloc(sizeof(*ethsw), GFP_KERNEL);
> +
> +	if (!ethsw)
> +		return -ENOMEM;
> +
> +	ethsw->dev = dev;
> +	dev_set_drvdata(dev, ethsw);
> +
> +	err = fsl_mc_portal_allocate(sw_dev, 0, &ethsw->mc_io);
> +	if (err) {
> +		dev_err(dev, "fsl_mc_portal_allocate err %d\n", err);
> +		goto err_free_drvdata;
> +	}
> +
> +	err = ethsw_init(sw_dev);
> +	if (err)
> +		goto err_free_cmdport;
> +
> +	/* DEFAULT_VLAN_ID is implicitly configured on the switch */
> +	ethsw->vlans[DEFAULT_VLAN_ID] = ETHSW_VLAN_MEMBER;
> +
> +	/* Learning is implicitly enabled */
> +	ethsw->learning = true;
> +
> +	ethsw->ports = kcalloc(ethsw->sw_attr.num_ifs, sizeof(*ethsw->ports),
> +			       GFP_KERNEL);
> +	if (!(ethsw->ports)) {
> +		err = -ENOMEM;
> +		goto err_takedown;
> +	}
> +
> +	for (i = 0; i < ethsw->sw_attr.num_ifs; i++) {
> +		err = ethsw_probe_port(ethsw, i);
> +		if (err) {
> +			/* Cleanup previous ports only */
> +			for (j = 0; j < i; j++) {

I think you can go with
for (i--; i >= 0; i--)

or better yet:
goto err_free_ports
and refactor err_free_ports to look like:
for (i--; i >= 0; i--) {
...
}

Best regards,
Bogdan P.

> +				unregister_netdev(ethsw->ports[j]->netdev);
> +				free_netdev(ethsw->ports[j]->netdev);
> +			}
> +			goto err_takedown;
> +		}
> +	}
> +
> +	/* Switch starts up enabled */
> +	rtnl_lock();
> +	err = ethsw_open(ethsw);
> +	rtnl_unlock();
> +	if (err)
> +		goto err_free_ports;
> +
> +	/* Setup IRQs */
> +	err = ethsw_setup_irqs(sw_dev);
> +	if (err)
> +		goto err_stop;
> +
> +	dev_info(dev, "probed %d port switch\n", ethsw->sw_attr.num_ifs);
> +	return 0;
> +
> +err_stop:
> +	rtnl_lock();
> +	ethsw_stop(ethsw);
> +	rtnl_unlock();
> +
> +err_free_ports:
> +	for (i = 0; i < ethsw->sw_attr.num_ifs; i++) {
> +		unregister_netdev(ethsw->ports[i]->netdev);
> +		free_netdev(ethsw->ports[i]->netdev);
> +	}
> +	kfree(ethsw->ports);
> +
> +err_takedown:
> +	ethsw_takedown(sw_dev);
> +
> +err_free_cmdport:
> +	fsl_mc_portal_free(ethsw->mc_io);
> +
> +err_free_drvdata:
> +	kfree(ethsw);
> +	dev_set_drvdata(dev, NULL);
> +
> +	return err;
> +}
> +
> +static const struct fsl_mc_device_id ethsw_match_id_table[] = {
> +	{
> +		.vendor = FSL_MC_VENDOR_FREESCALE,
> +		.obj_type = "dpsw",
> +	},
> +	{ .vendor = 0x0 }
> +};
> +MODULE_DEVICE_TABLE(fslmc, ethsw_match_id_table);
> +
> +static struct fsl_mc_driver eth_sw_drv = {
> +	.driver = {
> +		.name = KBUILD_MODNAME,
> +		.owner = THIS_MODULE,
> +	},
> +	.probe = ethsw_probe,
> +	.remove = ethsw_remove,
> +	.match_id_table = ethsw_match_id_table
> +};
> +
> +module_fsl_mc_driver(eth_sw_drv);
> +
> +MODULE_LICENSE("Dual BSD/GPL");
> +MODULE_DESCRIPTION("DPAA2 Ethernet Switch Driver");
> diff --git a/drivers/staging/fsl-dpaa2/ethsw/ethsw.h b/drivers/staging/fsl-
> dpaa2/ethsw/ethsw.h
> new file mode 100644
> index 0000000..8c1d645
> --- /dev/null
> +++ b/drivers/staging/fsl-dpaa2/ethsw/ethsw.h
> @@ -0,0 +1,88 @@
> +/* Copyright 2014-2017 Freescale Semiconductor Inc.
> + * Copyright 2017 NXP
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions are met:
> + *     * Redistributions of source code must retain the above copyright
> + *	 notice, this list of conditions and the following disclaimer.
> + *     * Redistributions in binary form must reproduce the above copyright
> + *	 notice, this list of conditions and the following disclaimer in the
> + *	 documentation and/or other materials provided with the distribution.
> + *     * Neither the name of the above-listed copyright holders nor the
> + *	 names of any contributors may be used to endorse or promote products
> + *	 derived from this software without specific prior written permission.
> + *
> + *
> + * ALTERNATIVELY, this software may be distributed under the terms of the
> + * GNU General Public License ("GPL") as published by the Free Software
> + * Foundation, either version 2 of that License or (at your option) any
> + * later version.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
> + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDERS OR CONTRIBUTORS BE
> + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
> + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
> + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
> + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
> + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
> + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
> + * POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +#ifndef __ETHSW_H
> +#define __ETHSW_H
> +
> +#include <linux/netdevice.h>
> +#include <linux/etherdevice.h>
> +#include <linux/rtnetlink.h>
> +#include <linux/if_vlan.h>
> +#include <uapi/linux/if_bridge.h>
> +#include <net/switchdev.h>
> +#include <linux/if_bridge.h>
> +
> +#include "dpsw.h"
> +
> +/* Number of IRQs supported */
> +#define DPSW_IRQ_NUM	2
> +
> +#define ETHSW_VLAN_MEMBER	1
> +#define ETHSW_VLAN_UNTAGGED	2
> +#define ETHSW_VLAN_PVID		4
> +#define ETHSW_VLAN_GLOBAL	8
> +
> +/* Maximum Frame Length supported by HW (currently 10k) */
> +#define DPAA2_MFL		(10 * 1024)
> +#define ETHSW_MAX_FRAME_LENGTH	(DPAA2_MFL - VLAN_ETH_HLEN - ETH_FCS_LEN)
> +#define ETHSW_L2_MAX_FRM(mtu)	((mtu) + VLAN_ETH_HLEN + ETH_FCS_LEN)
> +
> +struct ethsw_core;
> +
> +/* Per port private data */
> +struct ethsw_port_priv {
> +	struct net_device	*netdev;
> +	u16			idx;
> +	struct ethsw_core	*ethsw_data;
> +	u8			link_state;
> +	u8			stp_state;
> +	bool			flood;
> +
> +	u8			vlans[VLAN_VID_MASK + 1];
> +	u16			pvid;
> +};
> +
> +/* Switch data */
> +struct ethsw_core {
> +	struct device			*dev;
> +	struct fsl_mc_io		*mc_io;
> +	u16				dpsw_handle;
> +	struct dpsw_attr		sw_attr;
> +	int				dev_id;
> +	struct ethsw_port_priv		**ports;
> +
> +	u8				vlans[VLAN_VID_MASK + 1];
> +	bool				learning;
> +};
> +
> +#endif	/* __ETHSW_H */
> --
> 1.9.1

^ permalink raw reply

* [PATCH][net-next] net: ipmr: make function ipmr_notifier_init static
From: Colin King @ 2017-09-29 13:34 UTC (permalink / raw)
  To: David S . Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, netdev,
	linux-kernel
  Cc: kernel-janitors

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

The function ipmr_notifier_init is local to the source and does
not need to be in global scope, so make it static.

Cleans up sparse warning:
warning: symbol 'ipmr_notifier_init' was not declared. Should it be static?

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 net/ipv4/ipmr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index 292a8e80bdfa..a844738b38bd 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -3224,7 +3224,7 @@ static const struct fib_notifier_ops ipmr_notifier_ops_template = {
 	.owner		= THIS_MODULE,
 };
 
-int __net_init ipmr_notifier_init(struct net *net)
+static int __net_init ipmr_notifier_init(struct net *net)
 {
 	struct fib_notifier_ops *ops;
 
-- 
2.14.1

^ permalink raw reply related

* Re: [PATCHv4 iproute2 2/2] lib/libnetlink: update rtnl_talk to support malloc buff at run time
From: Michal Kubecek @ 2017-09-29 13:06 UTC (permalink / raw)
  To: Hangbin Liu; +Cc: netdev, Stephen Hemminger, Phil Sutter, Hangbin Liu
In-Reply-To: <1506605626-1744-3-git-send-email-haliu@redhat.com>

On Thu, Sep 28, 2017 at 09:33:46PM +0800, Hangbin Liu wrote:
> From: Hangbin Liu <liuhangbin@gmail.com>
> 
> This is an update for 460c03f3f3cc ("iplink: double the buffer size also in
> iplink_get()"). After update, we will not need to double the buffer size
> every time when VFs number increased.
> 
> With call like rtnl_talk(&rth, &req.n, NULL, 0), we can simply remove the
> length parameter.
> 
> With call like rtnl_talk(&rth, nlh, nlh, sizeof(req), I add a new variable
> answer to avoid overwrite data in nlh, because it may has more info after
> nlh. also this will avoid nlh buffer not enough issue.
> 
> We need to free answer after using.
> 
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> Signed-off-by: Phil Sutter <phil@nwl.cc>
> ---

Reviewed-by: Michal Kubecek <mkubecek@suse.cz>

> diff --git a/ip/link_gre.c b/ip/link_gre.c
> index 9ea2970..35782ca 100644
> --- a/ip/link_gre.c
> +++ b/ip/link_gre.c
> @@ -68,7 +68,6 @@ static int gre_parse_opt(struct link_util *lu, int argc, char **argv,
>  	struct {
>  		struct nlmsghdr n;
>  		struct ifinfomsg i;
> -		char buf[16384];
>  	} req = {
>  		.n.nlmsg_len = NLMSG_LENGTH(sizeof(*ifi)),
>  		.n.nlmsg_flags = NLM_F_REQUEST,
> @@ -76,6 +75,7 @@ static int gre_parse_opt(struct link_util *lu, int argc, char **argv,
>  		.i.ifi_family = preferred_family,
>  		.i.ifi_index = ifi->ifi_index,
>  	};
> +	struct nlmsghdr *answer = NULL;
>  	struct rtattr *tb[IFLA_MAX + 1];
>  	struct rtattr *linkinfo[IFLA_INFO_MAX+1];
>  	struct rtattr *greinfo[IFLA_GRE_MAX + 1];
> @@ -100,19 +100,20 @@ static int gre_parse_opt(struct link_util *lu, int argc, char **argv,
>  	__u32 erspan_idx = 0;
>  
>  	if (!(n->nlmsg_flags & NLM_F_CREATE)) {
> -		if (rtnl_talk(&rth, &req.n, &req.n, sizeof(req)) < 0) {
> +		if (rtnl_talk(&rth, &req.n, &answer) < 0) {
>  get_failed:
>  			fprintf(stderr,
>  				"Failed to get existing tunnel info.\n");
> +			free(answer);
>  			return -1;
>  		}

Took me a moment to realize answer is still NULL if we get here via
failed rtnl_talk() but non-NULL if we get here via "goto get_failed"
later. Nice trick. :-)

Michal Kubecek

^ permalink raw reply

* Re: [net-next PATCH 3/5] bpf: cpumap xdp_buff to skb conversion and allocation
From: Jesper Dangaard Brouer @ 2017-09-29 13:05 UTC (permalink / raw)
  To: Jason Wang
  Cc: netdev, jakub.kicinski, Michael S. Tsirkin, mchan, John Fastabend,
	peter.waskiewicz.jr, Daniel Borkmann, Alexei Starovoitov,
	Andy Gospodarek, brouer
In-Reply-To: <1c37f945-0e2f-1eec-fe88-a740815026d3@redhat.com>

On Fri, 29 Sep 2017 17:49:23 +0800
Jason Wang <jasowang@redhat.com> wrote:

> On 2017年09月28日 20:57, Jesper Dangaard Brouer wrote:
> > +};
> > +
> > +/* Convert xdp_buff to xdp_pkt */
> > +static struct xdp_pkt *convert_to_xdp_pkt(struct xdp_buff *xdp)
> > +{
> > +	struct xdp_pkt *xdp_pkt;
> > +	int headroom;
> > +
> > +	/* Assure headroom is available for storing info */
> > +	headroom = xdp->data - xdp->data_hard_start;
> > +	if (headroom < sizeof(*xdp_pkt))
> > +		return NULL;  
> 
> Hi Jesper:
> 
> Do you consider this as a trick or a long term solution? Is it better to 
> store XDP in a circular buffer? (I'm asking since I meet similar issue 
> when doing xdp_xmit for tun).

(The way you ask the question is slightly ambiguous, but I hope I understand.)

IMHO the best solution to allow queueing of XDP packets is to create a
meta-data structure, with the needed info.  For performance reasons, we
don't want to allocate a new memory area for this.  Thus, we simply use
the available headroom in the page that the packet is stored into.
Notice that DPDK also use the first cache-line of the packet data, for
its packet meta-data structure. (This is not a performance problem.
I've done several PoC benchmarks, before choosing to do this)

For now, this "trick" is local to the cpumap, and thus not exposed as
any API.  Thus we can evolve and change the contents easily.  But I
would in time, like to see this generalized. When/if more places need
to queue XDP packets, this header meta-data format should be
standardized.


Pipe-dreaming: Taking this to the extreme... if I could get away with
it, I would actually like to store the (232 bytes) SKB meta-data header
inside headroom too.  That would eliminate any real SKB memory alloc.


> > +
> > +	/* Store info in top of packet */
> > +	xdp_pkt = xdp->data_hard_start;
> > +
> > +	xdp_pkt->data = xdp->data;
> > +	xdp_pkt->len  = xdp->data_end - xdp->data;
> > +	xdp_pkt->headroom = headroom - sizeof(*xdp_pkt);
> > +  
> 
> Is wmb() needed here?

No. This xdp_pkt is queued into a into a ptr_ring, which have a
spin_lock on enqueue, and any atomic operation works as a full memory
barrirer mb().

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

^ permalink raw reply

* Re: [PATCH net-next 03/10] sctp: factor out stream->in allocation
From: 'Marcelo Ricardo Leitner' @ 2017-09-29 13:05 UTC (permalink / raw)
  To: David Laight
  Cc: netdev@vger.kernel.org, linux-sctp@vger.kernel.org, Neil Horman,
	Vlad Yasevich, Xin Long
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DD0084F8E@AcuExch.aculab.com>

On Fri, Sep 29, 2017 at 10:04:15AM +0000, David Laight wrote:
> From: Marcelo Ricardo Leitner
> > Sent: 28 September 2017 21:25
> > Same idea as previous patch.
> 
> That needs a proper description.

Alright.

  Marcelo

^ permalink raw reply

* Re: [PATCHv4 iproute2 1/2] lib/libnetlink: re malloc buff if size is not enough
From: Michal Kubecek @ 2017-09-29 12:55 UTC (permalink / raw)
  To: Hangbin Liu; +Cc: netdev, Stephen Hemminger, Phil Sutter, Hangbin Liu
In-Reply-To: <1506605626-1744-2-git-send-email-haliu@redhat.com>

On Thu, Sep 28, 2017 at 09:33:45PM +0800, Hangbin Liu wrote:
> From: Hangbin Liu <liuhangbin@gmail.com>
> 
> With commit 72b365e8e0fd ("libnetlink: Double the dump buffer size")
> we doubled the buffer size to support more VFs. But the VFs number is
> increasing all the time. Some customers even use more than 200 VFs now.
> 
> We could not double it everytime when the buffer is not enough. Let's just
> not hard code the buffer size and malloc the correct number when running.
> 
> Introduce function rtnl_recvmsg() to always return a newly allocated buffer.
> The caller need to free it after using.
> 
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> Signed-off-by: Phil Sutter <phil@nwl.cc>
> ---
>  lib/libnetlink.c | 114 ++++++++++++++++++++++++++++++++++++++-----------------
>  1 file changed, 80 insertions(+), 34 deletions(-)
> 

Reviewed-by: Michal Kubecek <mkubecek@suse.cz>

^ permalink raw reply

* [PATCH v2 net] net: mvpp2: Fix clock resource by adding an optional bus clock
From: Gregory CLEMENT @ 2017-09-29 12:27 UTC (permalink / raw)
  To: David S. Miller, linux-kernel, netdev
  Cc: Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Gregory CLEMENT,
	Thomas Petazzoni, linux-arm-kernel, Antoine Tenart,
	Miquèl Raynal, Nadav Haklai, Shadi Ammouri, Yehuda Yitschak,
	Omri Itach, Hanna Hawa, Igal Liberman, Marcin Wojtas

On Armada 7K/8K we need to explicitly enable the bus clock. The bus clock
is optional because not all the SoCs need them but at least for Armada
7K/8K it is actually mandatory.

The binding documentation is updating accordingly.

Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
Changelog:
v1 -> v2:
 - manage the -EPROBE_DEFER case
 - fix typos in documentation
 - remove useless test before clk_disable_unprepare()

 Documentation/devicetree/bindings/net/marvell-pp2.txt | 10 ++++++----
 drivers/net/ethernet/marvell/mvpp2.c                  | 15 +++++++++++++++
 2 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/marvell-pp2.txt b/Documentation/devicetree/bindings/net/marvell-pp2.txt
index 7e2dad08a12e..1814fa13f6ab 100644
--- a/Documentation/devicetree/bindings/net/marvell-pp2.txt
+++ b/Documentation/devicetree/bindings/net/marvell-pp2.txt
@@ -21,8 +21,9 @@ Required properties:
 	- main controller clock (for both armada-375-pp2 and armada-7k-pp2)
 	- GOP clock (for both armada-375-pp2 and armada-7k-pp2)
 	- MG clock (only for armada-7k-pp2)
-- clock-names: names of used clocks, must be "pp_clk", "gop_clk" and
-  "mg_clk" (the latter only for armada-7k-pp2).
+	- AXI clock (only for armada-7k-pp2)
+- clock-names: names of used clocks, must be "pp_clk", "gop_clk", "mg_clk"
+  and "axi_clk" (the 2 latter only for armada-7k-pp2).
 
 The ethernet ports are represented by subnodes. At least one port is
 required.
@@ -78,8 +79,9 @@ Example for marvell,armada-7k-pp2:
 cpm_ethernet: ethernet@0 {
 	compatible = "marvell,armada-7k-pp22";
 	reg = <0x0 0x100000>, <0x129000 0xb000>;
-	clocks = <&cpm_syscon0 1 3>, <&cpm_syscon0 1 9>, <&cpm_syscon0 1 5>;
-	clock-names = "pp_clk", "gop_clk", "gp_clk";
+	clocks = <&cpm_syscon0 1 3>, <&cpm_syscon0 1 9>,
+		 <&cpm_syscon0 1 5>, <&cpm_syscon0 1 18>;
+	clock-names = "pp_clk", "gop_clk", "gp_clk", "axi_clk";
 
 	eth0: eth0 {
 		interrupts = <ICU_GRP_NSR 39 IRQ_TYPE_LEVEL_HIGH>,
diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
index dd0ee2691c86..f2656112986b 100644
--- a/drivers/net/ethernet/marvell/mvpp2.c
+++ b/drivers/net/ethernet/marvell/mvpp2.c
@@ -792,6 +792,7 @@ struct mvpp2 {
 	struct clk *pp_clk;
 	struct clk *gop_clk;
 	struct clk *mg_clk;
+	struct clk *axi_clk;
 
 	/* List of pointers to port structures */
 	struct mvpp2_port **port_list;
@@ -7963,6 +7964,18 @@ static int mvpp2_probe(struct platform_device *pdev)
 		err = clk_prepare_enable(priv->mg_clk);
 		if (err < 0)
 			goto err_gop_clk;
+
+		priv->axi_clk = devm_clk_get(&pdev->dev, "axi_clk");
+		if (IS_ERR(priv->axi_clk)) {
+			err = PTR_ERR(priv->axi_clk);
+			if (err == -EPROBE_DEFER)
+				goto err_gop_clk;
+			priv->axi_clk = NULL;
+		} else {
+			err = clk_prepare_enable(priv->axi_clk);
+			if (err < 0)
+				goto err_gop_clk;
+		}
 	}
 
 	/* Get system's tclk rate */
@@ -8015,6 +8028,7 @@ static int mvpp2_probe(struct platform_device *pdev)
 	return 0;
 
 err_mg_clk:
+	clk_disable_unprepare(priv->axi_clk);
 	if (priv->hw_version == MVPP22)
 		clk_disable_unprepare(priv->mg_clk);
 err_gop_clk:
@@ -8052,6 +8066,7 @@ static int mvpp2_remove(struct platform_device *pdev)
 				  aggr_txq->descs_dma);
 	}
 
+	clk_disable_unprepare(priv->axi_clk);
 	clk_disable_unprepare(priv->mg_clk);
 	clk_disable_unprepare(priv->pp_clk);
 	clk_disable_unprepare(priv->gop_clk);
-- 
2.14.1

^ permalink raw reply related

* Re: [PATCH RFC 3/5] Add KSZ8795 switch driver
From: Andrew Lunn @ 2017-09-29 12:12 UTC (permalink / raw)
  To: David Laight
  Cc: Tristram.Ha@microchip.com, muvarov@gmail.com, pavel@ucw.cz,
	nathan.leigh.conrad@gmail.com,
	vivien.didelot@savoirfairelinux.com, f.fainelli@gmail.com,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	Woojung.Huh@microchip.com
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DD0084EDC@AcuExch.aculab.com>

On Fri, Sep 29, 2017 at 09:14:26AM +0000, David Laight wrote:
> From: Andrew Lunn
> > Sent: 28 September 2017 20:34
> ...
> > > There are 34 counters.  In normal case using generic bus I/O or PCI to read them
> > > is very quick, but the switch is mostly accessed using SPI, or even I2C.  As the SPI
> > > access is very slow.
> > 
> > How slow is it? The Marvell switches all use MDIO. It is probably a
> > bit faster than I2C, but it is a lot slower than MMIO or PCI.
> > 
> > ethtool -S lan0 takes about 25ms.
> 
> Is the SPI access software bit-banged?

That will depend on the board design. I've used mdio bit banging, and
that was painfully slow for stats.

But we should primarily think about average hardware. It is going to
have hardware SPI or I2C. If statistics reading with hardware I2C is
reasonable, i would avoid caching, and just ensure other accesses are
permitted between individual statistic reads.

It also requires Microchip also post new code. They have been very
silent for quite a while....

	  Andrew

^ permalink raw reply

* Fwd: [PATCH] net: ethernet: sfc: There is a typo, modify curent to current.
From: Hao Zhang @ 2017-09-29 11:53 UTC (permalink / raw)
  To: netdev, davem
In-Reply-To: <20170929101937.GA8963@arx-kt>

Resend to maintainer.



Regards :)


---------- Forwarded message ----------
From: Hao Zhang <hao5781286@gmail.com>
Date: 2017-09-29 18:19 GMT+08:00
Subject: [PATCH] net: ethernet: sfc: There is a typo, modify curent to current.
To: linux-net-drivers@solarflare.com, ecree@solarflare.com,
bkenward@solarflare.com
抄送: linux-kernel@vger.kernel.org, hao5781286@gmail.com


There is a typo, fix it by modify curent to current.

Signed-off-by: Hao Zhang <hao5781286@gmail.com>
---
 drivers/net/ethernet/sfc/mcdi_pcol.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/sfc/mcdi_pcol.h
b/drivers/net/ethernet/sfc/mcdi_pcol.h
index 91fb54f..8bfc8d4 100644
--- a/drivers/net/ethernet/sfc/mcdi_pcol.h
+++ b/drivers/net/ethernet/sfc/mcdi_pcol.h
@@ -11505,7 +11505,7 @@

 /***********************************/
 /* MC_CMD_GET_SNAPSHOT_LENGTH
- * Obtain the curent range of allowable values for the SNAPSHOT_LENGTH
+ * Obtain the current range of allowable values for the SNAPSHOT_LENGTH
  * parameter to MC_CMD_INIT_RXQ.
  */
 #define MC_CMD_GET_SNAPSHOT_LENGTH 0x101

^ permalink raw reply related

* Re: [PATCH] net: ethernet: sfc: There is a typo, modify curent to current.
From: Hao Zhang @ 2017-09-29 11:51 UTC (permalink / raw)
  To: davem, bkenward, ecree, netdev, linux-net-drivers
In-Reply-To: <20170929101937.GA8963@arx-kt>

Resend to maintainer.



Regards :)

2017-09-29 18:19 GMT+08:00 Hao Zhang <hao5781286@gmail.com>:
> There is a typo, fix it by modify curent to current.
>
> Signed-off-by: Hao Zhang <hao5781286@gmail.com>
> ---
>  drivers/net/ethernet/sfc/mcdi_pcol.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/sfc/mcdi_pcol.h b/drivers/net/ethernet/sfc/mcdi_pcol.h
> index 91fb54f..8bfc8d4 100644
> --- a/drivers/net/ethernet/sfc/mcdi_pcol.h
> +++ b/drivers/net/ethernet/sfc/mcdi_pcol.h
> @@ -11505,7 +11505,7 @@
>
>  /***********************************/
>  /* MC_CMD_GET_SNAPSHOT_LENGTH
> - * Obtain the curent range of allowable values for the SNAPSHOT_LENGTH
> + * Obtain the current range of allowable values for the SNAPSHOT_LENGTH
>   * parameter to MC_CMD_INIT_RXQ.
>   */
>  #define MC_CMD_GET_SNAPSHOT_LENGTH 0x101
> --
> 2.7.4
>

^ permalink raw reply


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