Netdev List
 help / color / mirror / Atom feed
* Re: [RFC PATCH 2/3] usbnet: Avoid potential races in usbnet_deferred_kevent()
From: Oliver Neukum @ 2017-09-19 20:37 UTC (permalink / raw)
  To: Douglas Anderson; +Cc: groeck, grundler, linux-kernel, linux-usb, netdev
In-Reply-To: <20170919161522.995-2-dianders@chromium.org>

Am Dienstag, den 19.09.2017, 09:15 -0700 schrieb Douglas Anderson:
> In general when you've got a flag communicating that "something needs
> to be done" you want to clear that flag _before_ doing the task.  If
> you clear the flag _after_ doing the task you end up with the risk
> that this will happen:
> 
> 1. Requester sets flag saying task A needs to be done.
> 2. Worker comes and stars doing task A.
> 3. Worker finishes task A but hasn't yet cleared the flag.
> 4. Requester wants to set flag saying task A needs to be done again.
> 5. Worker clears the flag without doing anything.
> 
> Let's make the usbnet codebase consistently clear the flag _before_ it
> does the requested work.  That way if there's another request to do
> the work while the work is already in progress it won't be lost.
> 
> NOTES:
> - No known bugs are fixed by this; it's just found by code inspection.

Hi,

unfortunately the patch is wrong. The flags must be cleared only
in case the handler is successful. That is not guaranteed.

	Regards
		Oliver

NACK

^ permalink raw reply

* Re: [RFC PATCH 1/3] usbnet: Get rid of spammy usbnet "kevent X may have been dropped"
From: Oliver Neukum @ 2017-09-19 20:36 UTC (permalink / raw)
  To: Douglas Anderson; +Cc: groeck, grundler, linux-kernel, linux-usb, netdev
In-Reply-To: <20170919161522.995-1-dianders@chromium.org>

Am Dienstag, den 19.09.2017, 09:15 -0700 schrieb Douglas Anderson:
> 
> ALSO NOTE: If somehow some of the types of work need to be repeated if
> usbnet_defer_kevent() is called multiple times then that should be
> quite easy to accomplish without dropping any work on the floor.  We
> can just keep an atomic count for that type of work and add a loop
> into usbnet_deferred_kevent().

Thanks for doing this, it is overdue.

	Regards
		Oliver

> Signed-off-by: Douglas Anderson <dianders@chromium.org>
Acked-by: Oliver Neukum <oneukum@suse.com>

^ permalink raw reply

* Re: [PATCH net-next v4 1/4] bpf: add helper bpf_perf_event_read_value for perf event array map
From: Daniel Borkmann @ 2017-09-19 20:33 UTC (permalink / raw)
  To: Yonghong Song, peterz, rostedt, ast, netdev; +Cc: kernel-team
In-Reply-To: <20170919070413.3838201-2-yhs@fb.com>

On 09/19/2017 09:04 AM, Yonghong Song wrote:
[...]

Just some minor things, but a bit scattered.

> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 43ab5c4..2c68b9e 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -582,6 +582,14 @@ union bpf_attr {
>    *	@map: pointer to sockmap to update
>    *	@key: key to insert/update sock in map
>    *	@flags: same flags as map update elem
> + *
> + * int bpf_perf_event_read_value(map, flags, buf, buf_size)
> + *     read perf event counter value and perf event enabled/running time
> + *     @map: pointer to perf_event_array map
> + *     @flags: index of event in the map or bitmask flags
> + *     @buf: buf to fill
> + *     @buf_size: size of the buf
> + *     Return: 0 on success or negative error code
>    */
>   #define __BPF_FUNC_MAPPER(FN)		\
>   	FN(unspec),			\
> @@ -638,6 +646,7 @@ union bpf_attr {
>   	FN(redirect_map),		\
>   	FN(sk_redirect_map),		\
>   	FN(sock_map_update),		\
> +	FN(perf_event_read_value),		\

Nit: can you indent the '\' so it aligns with the rest

>   /* integer value in 'imm' field of BPF_CALL instruction selects which helper
>    * function eBPF program intends to call
> @@ -681,7 +690,8 @@ enum bpf_func_id {
>   #define BPF_F_ZERO_CSUM_TX		(1ULL << 1)
>   #define BPF_F_DONT_FRAGMENT		(1ULL << 2)
>
> -/* BPF_FUNC_perf_event_output and BPF_FUNC_perf_event_read flags. */
> +/* BPF_FUNC_perf_event_output, BPF_FUNC_perf_event_read and
> + * BPF_FUNC_perf_event_read_value flags. */

Nit: comment style

>   #define BPF_F_INDEX_MASK		0xffffffffULL
>   #define BPF_F_CURRENT_CPU		BPF_F_INDEX_MASK
>   /* BPF_FUNC_perf_event_output for sk_buff input context. */
> @@ -864,4 +874,10 @@ enum {
>   #define TCP_BPF_IW		1001	/* Set TCP initial congestion window */
>   #define TCP_BPF_SNDCWND_CLAMP	1002	/* Set sndcwnd_clamp */
>
> +struct bpf_perf_event_value {
> +	__u64 counter;
> +	__u64 enabled;
> +	__u64 running;
> +};
> +
[...]
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index 3e691b7..2d5bbe5 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -3684,10 +3684,12 @@ static inline u64 perf_event_count(struct perf_event *event)
>    *     will not be local and we cannot read them atomically
>    *   - must not have a pmu::count method
>    */
> -int perf_event_read_local(struct perf_event *event, u64 *value)
> +int perf_event_read_local(struct perf_event *event, u64 *value,
> +			  u64 *enabled, u64 *running)
>   {
>   	unsigned long flags;
>   	int ret = 0;
> +	u64 now;
>
>   	/*
>   	 * Disabling interrupts avoids all counter scheduling (context
> @@ -3718,14 +3720,21 @@ int perf_event_read_local(struct perf_event *event, u64 *value)
>   		goto out;
>   	}
>
> +	now = event->shadow_ctx_time + perf_clock();
> +	if (enabled)
> +		*enabled = now - event->tstamp_enabled;
>   	/*
>   	 * If the event is currently on this CPU, its either a per-task event,
>   	 * or local to this CPU. Furthermore it means its ACTIVE (otherwise
>   	 * oncpu == -1).
>   	 */
> -	if (event->oncpu == smp_processor_id())
> +	if (event->oncpu == smp_processor_id()) {
>   		event->pmu->read(event);
> -
> +		if (running)
> +			*running = now - event->tstamp_running;
> +	} else if (running) {
> +		*running = event->total_time_running;
> +	}
>   	*value = local64_read(&event->count);
>   out:
>   	local_irq_restore(flags);
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index dc498b6..39ce5d9 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -255,13 +255,13 @@ const struct bpf_func_proto *bpf_get_trace_printk_proto(void)
>   	return &bpf_trace_printk_proto;
>   }
>
> -BPF_CALL_2(bpf_perf_event_read, struct bpf_map *, map, u64, flags)
> -{
> +static __always_inline int
> +get_map_perf_counter(struct bpf_map *map, u64 flags,
> +		u64 *value, u64 *enabled, u64 *running) {

Nit: coding style

>   	struct bpf_array *array = container_of(map, struct bpf_array, map);
>   	unsigned int cpu = smp_processor_id();
>   	u64 index = flags & BPF_F_INDEX_MASK;
>   	struct bpf_event_entry *ee;
> -	u64 value = 0;
>   	int err;
>
>   	if (unlikely(flags & ~(BPF_F_INDEX_MASK)))
> @@ -275,7 +275,17 @@ BPF_CALL_2(bpf_perf_event_read, struct bpf_map *, map, u64, flags)
>   	if (!ee)
>   		return -ENOENT;
>
> -	err = perf_event_read_local(ee->event, &value);
> +	err = perf_event_read_local(ee->event, value, enabled, running);
> +	return err;

err can be removed entirely then.

   return perf_event_read_local(ee->event, value, enabled, running);

> +}
> +
> +

Nit: Two newlines slipped in.

> +BPF_CALL_2(bpf_perf_event_read, struct bpf_map *, map, u64, flags)
> +{
> +	u64 value = 0;
> +	int err;
> +
> +	err = get_map_perf_counter(map, flags, &value, NULL, NULL);
>   	/*
>   	 * this api is ugly since we miss [-22..-2] range of valid
>   	 * counter values, but that's uapi
> @@ -285,6 +295,20 @@ BPF_CALL_2(bpf_perf_event_read, struct bpf_map *, map, u64, flags)
>   	return value;
>   }

Can you also move the bpf_perf_event_read_proto definition right
underneath here as we have with all other helpers?

> +BPF_CALL_4(bpf_perf_event_read_value, struct bpf_map *, map, u64, flags,
> +	struct bpf_perf_event_value *, buf, u32, size)

Nit: indent

> +{
> +	int err;
> +
> +	if (unlikely(size != sizeof(struct bpf_perf_event_value)))
> +		return -EINVAL;
> +	err = get_map_perf_counter(map, flags, &buf->counter, &buf->enabled,
> +                            &buf->running);

^ This is only indented with spaces.

> +	if (err)
> +		return err;
> +	return 0;

Also here err can be removed entirely, make it
less convoluted:

BPF_CALL_4(bpf_perf_event_read_value, struct bpf_map *, map, u64, flags,
           struct bpf_perf_event_value *, eval, u32, size)
{
        if (unlikely(size != sizeof(struct bpf_perf_event_value)))
                return -EINVAL;

        return get_map_perf_counter(map, flags, &eval->counter, &eval->enabled,
                                    &eval->running);
}

> +}
> +
>   static const struct bpf_func_proto bpf_perf_event_read_proto = {
>   	.func		= bpf_perf_event_read,
>   	.gpl_only	= true,
> @@ -293,6 +317,16 @@ static const struct bpf_func_proto bpf_perf_event_read_proto = {
>   	.arg2_type	= ARG_ANYTHING,
>   };
>
> +static const struct bpf_func_proto bpf_perf_event_read_value_proto = {
> +	.func		= bpf_perf_event_read_value,
> +	.gpl_only	= true,
> +	.ret_type	= RET_INTEGER,
> +	.arg1_type	= ARG_CONST_MAP_PTR,
> +	.arg2_type	= ARG_ANYTHING,
> +	.arg3_type	= ARG_PTR_TO_UNINIT_MEM,

If you do that, then error paths need to memset the region, e.g.
see bpf_skb_get_tunnel_opt() and bpf_skb_get_tunnel_key() helpers
which operate similarly.

> +	.arg4_type	= ARG_CONST_SIZE,
> +};
> +
>   static DEFINE_PER_CPU(struct perf_sample_data, bpf_sd);
>
>   static __always_inline u64
> @@ -499,6 +533,8 @@ static const struct bpf_func_proto *kprobe_prog_func_proto(enum bpf_func_id func
>   		return &bpf_perf_event_output_proto;
>   	case BPF_FUNC_get_stackid:
>   		return &bpf_get_stackid_proto;
> +	case BPF_FUNC_perf_event_read_value:
> +		return &bpf_perf_event_read_value_proto;
>   	default:
>   		return tracing_func_proto(func_id);
>   	}
>

^ permalink raw reply

* [PATCH net-next] net_sched: no need to free qdisc in RCU callback
From: Cong Wang @ 2017-09-19 20:15 UTC (permalink / raw)
  To: netdev; +Cc: Cong Wang, Jamal Hadi Salim, Eric Dumazet

gen estimator has been rewritten in commit 1c0d32fde5bd
("net_sched: gen_estimator: complete rewrite of rate estimators"),
the caller no longer needs to wait for a grace period. So this
patch gets rid of it.

Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: Eric Dumazet <edumazet@google.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
 include/net/sch_generic.h |  1 -
 net/sched/sch_generic.c   | 10 ++--------
 2 files changed, 2 insertions(+), 9 deletions(-)

diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index 135f5a2dd931..684d8ed27eaa 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -93,7 +93,6 @@ struct Qdisc {
 	unsigned long		state;
 	struct Qdisc            *next_sched;
 	struct sk_buff		*skb_bad_txq;
-	struct rcu_head		rcu_head;
 	int			padded;
 	refcount_t		refcnt;
 
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index 92237e75dbbc..1fb0c754b7fd 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -688,10 +688,8 @@ void qdisc_reset(struct Qdisc *qdisc)
 }
 EXPORT_SYMBOL(qdisc_reset);
 
-static void qdisc_rcu_free(struct rcu_head *head)
+static void qdisc_free(struct Qdisc *qdisc)
 {
-	struct Qdisc *qdisc = container_of(head, struct Qdisc, rcu_head);
-
 	if (qdisc_is_percpu_stats(qdisc)) {
 		free_percpu(qdisc->cpu_bstats);
 		free_percpu(qdisc->cpu_qstats);
@@ -724,11 +722,7 @@ void qdisc_destroy(struct Qdisc *qdisc)
 
 	kfree_skb_list(qdisc->gso_skb);
 	kfree_skb(qdisc->skb_bad_txq);
-	/*
-	 * gen_estimator est_timer() might access qdisc->q.lock,
-	 * wait a RCU grace period before freeing qdisc.
-	 */
-	call_rcu(&qdisc->rcu_head, qdisc_rcu_free);
+	qdisc_free(qdisc);
 }
 EXPORT_SYMBOL(qdisc_destroy);
 
-- 
2.13.0

^ permalink raw reply related

* Re: [PATCH net-next 0/4] net: dsa: move master ethtool code
From: Florian Fainelli @ 2017-09-19 20:04 UTC (permalink / raw)
  To: Vivien Didelot, netdev; +Cc: linux-kernel, kernel, David S. Miller, Andrew Lunn
In-Reply-To: <20170919155700.14474-1-vivien.didelot@savoirfairelinux.com>

On 09/19/2017 08:56 AM, Vivien Didelot wrote:
> The DSA core overrides the master device's ethtool_ops structure so that
> it can inject statistics and such of its dedicated switch CPU port.
> 
> This ethtool code is currently called on unnecessary conditions or
> before the master interface and its switch CPU port get wired up.
> This patchset fixes this.
> 
> Similarly to slave.c where the DSA slave net_device is the entry point
> of the dsa_slave_* functions, this patchset also isolates the master's
> ethtool code in a new master.c file, where the DSA master net_device is
> the entry point of the dsa_master_* functions.
> 
> This is a first step towards better control of the master device and
> support for multiple CPU ports.

Tested-by: Florian Fainelli <f.fainelli@gmail.com>

* ethtool -S eth0 -> switch port CPU stats are still correctly overlayed
* ethtool -s gphy wol g -> both switch port and CPU port correctly
enable WoL
* ethtool -i eth0 -> driver still reports correct information

Thanks!

> 
> Vivien Didelot (4):
>   net: dsa: remove copy of master ethtool_ops
>   net: dsa: setup master ethtool unconditionally
>   net: dsa: setup master ethtool after dsa_ptr
>   net: dsa: move master ethtool code
> 
>  include/net/dsa.h  |   1 -
>  net/dsa/Makefile   |   2 +-
>  net/dsa/dsa.c      |  28 -------------
>  net/dsa/dsa2.c     |  18 ++++----
>  net/dsa/dsa_priv.h |   7 ++--
>  net/dsa/legacy.c   |  10 ++---
>  net/dsa/master.c   | 120 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  net/dsa/slave.c    |  80 -----------------------------------
>  8 files changed, 136 insertions(+), 130 deletions(-)
>  create mode 100644 net/dsa/master.c
> 


-- 
Florian

^ permalink raw reply

* Re: [PATCH 2/3] selftests: actually run the various net selftests
From: Shuah Khan @ 2017-09-19 20:00 UTC (permalink / raw)
  To: Willem de Bruijn, Josef Bacik
  Cc: Willem de Bruijn, Josef Bacik, David S. Miller, LKML,
	linux-kselftest, Network Development, Shuah Khan, Shuah Khan
In-Reply-To: <CAF=yD-JewUiQso6XbDEAjC+gP=yK=czsdujYmcu5=jjXTsweDA@mail.gmail.com>

On 09/19/2017 12:14 PM, Willem de Bruijn wrote:
> On Tue, Sep 19, 2017 at 9:34 AM, Josef Bacik <josef@toxicpanda.com> wrote:
>> On Mon, Sep 18, 2017 at 04:14:41PM -0600, Shuah Khan wrote:
>>> On 09/18/2017 11:32 AM, josef@toxicpanda.com wrote:
>>>> From: Josef Bacik <jbacik@fb.com>
>>>>
>>>> These self tests are just self contained binaries, they are not run by
>>>> any of the scripts in the directory.  This means they need to be marked
>>>> with TEST_GEN_PROGS to actually be run, not TEST_GEN_FILES.
>>>>
>>>> Signed-off-by: Josef Bacik <jbacik@fb.com>
>>>> ---
>>>>  tools/testing/selftests/net/Makefile | 4 ++--
>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
>>>> index 3df542c84610..45a4e77a47c4 100644
>>>> --- a/tools/testing/selftests/net/Makefile
>>>> +++ b/tools/testing/selftests/net/Makefile
>>>> @@ -6,8 +6,8 @@ CFLAGS += -I../../../../usr/include/
>>>>  TEST_PROGS := run_netsocktests run_afpackettests test_bpf.sh netdevice.sh rtnetlink.sh
>>>>  TEST_GEN_FILES =  socket
>>>>  TEST_GEN_FILES += psock_fanout psock_tpacket
>>>> -TEST_GEN_FILES += reuseport_bpf reuseport_bpf_cpu reuseport_bpf_numa
>>>> -TEST_GEN_FILES += reuseport_dualstack msg_zerocopy reuseaddr_conflict
>>>> +TEST_GEN_PROGS += reuseport_bpf reuseport_bpf_cpu reuseport_bpf_numa
>>>> +TEST_GEN_PROGS += reuseport_dualstack msg_zerocopy reuseaddr_conflict
>>>
>>> Hmm. I see msg_zerocopy.sh for running msg_zerocopy. msg_zerocopy should
>>> still stay in TEST_GEN_FILES and msg_zerocopy.sh needs to be added to
>>> TEST_PROGS so it runs.
>>>
>>
>> Actually the shell script requires arguments, it doesn't just run the test.
>> I'll fix this to just omit the test for now as it's not setup to run properly.
>>
>> Willem, could you follow up with a patch so that the zero copy test is run
>> properly the way you envision it running?  You need to make sure that
>>
>> make -C tools/testing/selftests TARGETS=net run_tests
>>
>> actually runs your zero copy test the way you expect it to, otherwise it's just
>> sitting there collecting dust.  Thanks,
> 
> Will do.
> 
> In its current state, this test is really only meant to be run manually.
> It demonstrates the API and outputs some information on stderr.
> 
> Zerocopy itself requires a two-host test. The feature is expressly
> disabled over loopback.

Running this manually is [perfectly fine. TEST_GEN_FILES is the right
place for it as TEST_GEN_FILES will get built and installed, but won't
be run bt lib.mk. No changes needed.

> 
> But I can make this a pass/fail tests that exercises the interface
> and notification channel and verifies that data was copied. It will
> be a bit more work than just changing the default invocation of
> msg_zerocopy.sh
> 

No need to make changes as this test is intended to be run manually.
Josef's v2 doesn't change the location of msg_zerocopy. We are all set.

Thanks for clearing this up.

-- Shuah

^ permalink raw reply

* Re: [PATCH net-next 1/3] bpf: Implement map_delete_elem for BPF_MAP_TYPE_LPM_TRIE
From: Daniel Mack @ 2017-09-19 19:48 UTC (permalink / raw)
  To: Daniel Borkmann, Craig Gallek, Alexei Starovoitov
  Cc: David S . Miller, netdev
In-Reply-To: <59C141DC.9060200@iogearbox.net>

Hi,

Thanks for working on this, Craig!

On 09/19/2017 06:12 PM, Daniel Borkmann wrote:
> On 09/19/2017 05:08 PM, Craig Gallek wrote:
>> On Mon, Sep 18, 2017 at 6:53 PM, Alexei Starovoitov <ast@fb.com> wrote:
>>> On 9/18/17 12:30 PM, Craig Gallek wrote:
> [...]
>>>> +
>>>> +               next_bit = extract_bit(key->data, node->prefixlen);
>>>> +               /* If we hit a node that has more than one child or is a
>>>> valid
>>>> +                * prefix itself, do not remove it. Reset the root of the
>>>> trim
>>>> +                * path to its descendant on our path.
>>>> +                */
>>>> +               if (!(node->flags & LPM_TREE_NODE_FLAG_IM) ||
>>>> +                   (node->child[0] && node->child[1]))
>>>> +                       trim = &node->child[next_bit];
>>>> +               node = rcu_dereference_protected(
>>>> +                       node->child[next_bit],
>>>> lockdep_is_held(&trie->lock));
>>>> +       }
>>>> +
>>>> +       if (!node || node->prefixlen != key->prefixlen ||
>>>> +           (node->flags & LPM_TREE_NODE_FLAG_IM)) {
>>>> +               ret = -ENOENT;
>>>> +               goto out;
>>>> +       }
>>>> +
>>>> +       trie->n_entries--;
>>>> +
>>>> +       /* If the node we are removing is not a leaf node, simply mark it
>>>> +        * as intermediate and we are done.
>>>> +        */
>>>> +       if (rcu_access_pointer(node->child[0]) ||
>>>> +           rcu_access_pointer(node->child[1])) {
>>>> +               node->flags |= LPM_TREE_NODE_FLAG_IM;
>>>> +               goto out;
>>>> +       }
>>>> +
>>>> +       /* trim should now point to the slot holding the start of a path
>>>> from
>>>> +        * zero or more intermediate nodes to our leaf node for deletion.
>>>> +        */
>>>> +       while ((node = rcu_dereference_protected(
>>>> +                       *trim, lockdep_is_held(&trie->lock)))) {
>>>> +               RCU_INIT_POINTER(*trim, NULL);
>>>> +               trim = rcu_access_pointer(node->child[0]) ?
>>>> +                       &node->child[0] :
>>>> +                       &node->child[1];
>>>> +               kfree_rcu(node, rcu);
>>>
>>> can it be that some of the nodes this loop walks have
>>> both child[0] and [1] ?
>> No, the loop above will push trim down the walk every time it
>> encounters a node with two children.  The only other trim assignment
>> is the initial trim = &trie->root.  But the only time we would skip
>> the assignment in the loop is if the node being removed is the root.
>> If the root had multiple children and is being removed, it would be
>> handled by the case that turns the node into an intermediate node
>> rather than walking the trim path freeing things.
> 
> Looks good to me. We should probably still merge nodes once we turn
> a real node into an im which just has a single child attached to it;
> parent can be im or real node. Thus, we don't need to traverse this
> extra one on lookup.

Right, but only if the the parent of the node allows us to do that,
because the 'next bit' in the lookup key has to match the slot index.

To illustrate, consider the following trie with no IM nodes:

                      +----------------+
                      |       (1)  (R) |
                      | 192.168.0.0/16 |
                      |    value: 1    |
                      |   [0]    [1]   |
                      +----------------+
                           |      |
            +----------------+  +------------------+
            |       (2)      |  |        (3)       |
            | 192.168.0.0/23 |  | 192.168.128.0/24 |
            |    value: 2    |  |     value: 3     |
            |   [0]    [1]   |  |    [0]    [1]    |
            +----------------+  +------------------+
                        |
                      +----------------+
                      |       (4)      |
                      | 192.168.1.0/24 |
                      |     value: 4   |
                      |   [0]    [1]   |
                      +----------------+

If you now try to delete (2), the node has to stay around because (3)
and (4) share the same value in bit 17 (1). If, however, (4) had a
prefix of 192.168.0.0/24, then (2) should be removed completely, and (4)
should be directly attached to (1) as child[0].

With this implementation, a situation in which multiple IM nodes appear
in a chain cannot emerge. And that again should make your trimming
algorithm simpler, because you only need to find an exact match, and
then handle three distinct cases:

a) the node as 0 children: simply remove it and nullify the pointer in
the parent

b) the node has 1 child: apply logic I described above

c) the node has 2 children: turn the node into an IM


Makes sense?


Thanks,
Daniel

^ permalink raw reply

* Re: [PATCH net-next 4/4] net: dsa: move master ethtool code
From: Florian Fainelli @ 2017-09-19 19:56 UTC (permalink / raw)
  To: Vivien Didelot, netdev; +Cc: linux-kernel, kernel, David S. Miller, Andrew Lunn
In-Reply-To: <20170919155700.14474-5-vivien.didelot@savoirfairelinux.com>

On 09/19/2017 08:57 AM, Vivien Didelot wrote:
> DSA overrides the master device ethtool ops, so that it can inject stats
> from its dedicated switch CPU port as well.
> 
> The related code is currently split in dsa.c and slave.c, but it only
> scopes the master net device. Move it to a new master.c DSA core file.
> 
> This file will be later extented with master net device specific code.
> 
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

^ permalink raw reply

* Re: [PATCH net-next 1/4] net: dsa: remove copy of master ethtool_ops
From: Florian Fainelli @ 2017-09-19 19:55 UTC (permalink / raw)
  To: Vivien Didelot, netdev; +Cc: linux-kernel, kernel, David S. Miller, Andrew Lunn
In-Reply-To: <20170919155700.14474-2-vivien.didelot@savoirfairelinux.com>

On 09/19/2017 08:56 AM, Vivien Didelot wrote:
> There is no need to store a copy of the master ethtool ops, storing the
> original pointer in DSA and the new one in the master netdev itself is
> enough.
> 
> In the meantime, set orig_ethtool_ops to NULL when restoring the master
> ethtool ops and check the presence of the master original ethtool ops as
> well as its needed functions before calling them.

I clearly like memcpy() too much, this looks good:

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

^ permalink raw reply

* Re: [5/5] e1000e: Avoid receiver overrun interrupt bursts
From: Benjamin Poirier @ 2017-09-19 19:41 UTC (permalink / raw)
  To: Philip Prindeville
  Cc: Jeff Kirsher, netdev, intel-wired-lan, linux-kernel,
	Lennart Sorensen
In-Reply-To: <DDD41C67-CDF2-4443-9F49-0DC5489D9243@redfish-solutions.com>

On 2017/09/19 12:38, Philip Prindeville wrote:
> Hi.
> 
> We’ve been running this patchset (all 5) for about as long as they’ve been under review… about 2 months.  And in a burn-in lab with heavy traffic.
> 
> We’ve not seen a single link-flap in hundreds of ours of saturated traffic.
> 
> Would love to see some resolution soon on this as we don’t want to ship a release with unsanctioned patches.
> 
> Is there an estimate on when that might be?

The patches have been added to Jeff Kirsher's next-queue tree. I guess
they will be submitted for v4.15 which might be released in early
2018...
http://phb-crystal-ball.org/

^ permalink raw reply

* [PATCH] isdn/i4l: check the message proto does not change across fetches
From: Meng Xu @ 2017-09-19 18:52 UTC (permalink / raw)
  To: isdn, davem, johannes.berg, netdev, linux-kernel
  Cc: meng.xu, sanidhya, taesoo, Meng Xu

In isdn_ppp_write(), the header (i.e., protobuf) of the buffer is fetched
twice from userspace. The first fetch is used to peek at the protocol
of the message and reset the huptimer if necessary; while the second
fetch copies in the whole buffer. However, given that buf resides in
userspace memory, a user process can race to change its memory content
across fetches. By doing so, we can either avoid resetting the huptimer
for any type of packets (by first setting proto to PPP_LCP and later
change to the actual type) or force resetting the huptimer for LCP packets.

This patch does a memcmp between the two fetches and abort if changes to
the protobuf is detected across fetches.

Signed-off-by: Meng Xu <mengxu.gatech@gmail.com>
---
 drivers/isdn/i4l/isdn_ppp.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/isdn/i4l/isdn_ppp.c b/drivers/isdn/i4l/isdn_ppp.c
index 6c44609..21a9ae8 100644
--- a/drivers/isdn/i4l/isdn_ppp.c
+++ b/drivers/isdn/i4l/isdn_ppp.c
@@ -857,6 +857,7 @@ isdn_ppp_write(int min, struct file *file, const char __user *buf, int count)
 		    (lp->flags & ISDN_NET_CONNECTED)) {
 			unsigned short hl;
 			struct sk_buff *skb;
+			void *skb_tail;
 			/*
 			 * we need to reserve enough space in front of
 			 * sk_buff. old call to dev_alloc_skb only reserved
@@ -869,11 +870,21 @@ isdn_ppp_write(int min, struct file *file, const char __user *buf, int count)
 				return count;
 			}
 			skb_reserve(skb, hl);
-			if (copy_from_user(skb_put(skb, count), buf, count))
+			skb_tail = skb_put(skb, count);
+			if (copy_from_user(skb_tail, buf, count))
 			{
 				kfree_skb(skb);
 				return -EFAULT;
 			}
+
+			/*
+			 * abort if the message proto is changed between the fetches
+			 */
+			if (memcmp(skb_tail, protobuf, 4)) {
+				kfree_skb(skb);
+				return -EFAULT;
+			}
+
 			if (is->debug & 0x40) {
 				printk(KERN_DEBUG "ppp xmit: len %d\n", (int) skb->len);
 				isdn_ppp_frame_log("xmit", skb->data, skb->len, 32, is->unit, lp->ppp_slot);
-- 
2.7.4

^ permalink raw reply related

* Re: [5/5] e1000e: Avoid receiver overrun interrupt bursts
From: Philip Prindeville @ 2017-09-19 18:38 UTC (permalink / raw)
  To: Jeff Kirsher
  Cc: netdev, intel-wired-lan, linux-kernel, Lennart Sorensen,
	Benjamin Poirier
In-Reply-To: <20170721183627.13373-5-bpoirier@suse.com>

Hi.

We’ve been running this patchset (all 5) for about as long as they’ve been under review… about 2 months.  And in a burn-in lab with heavy traffic.

We’ve not seen a single link-flap in hundreds of ours of saturated traffic.

Would love to see some resolution soon on this as we don’t want to ship a release with unsanctioned patches.

Is there an estimate on when that might be?

Thanks,

-Philip



> On Jul 21, 2017, at 12:36 PM, Benjamin Poirier <bpoirier@suse.com> wrote:
> 
> When e1000e_poll() is not fast enough to keep up with incoming traffic, the
> adapter (when operating in msix mode) raises the Other interrupt to signal
> Receiver Overrun.
> 
> This is a double problem because 1) at the moment e1000_msix_other()
> assumes that it is only called in case of Link Status Change and 2) if the
> condition persists, the interrupt is repeatedly raised again in quick
> succession.
> 
> Ideally we would configure the Other interrupt to not be raised in case of
> receiver overrun but this doesn't seem possible on this adapter. Instead,
> we handle the first part of the problem by reverting to the practice of
> reading ICR in the other interrupt handler, like before commit 16ecba59bc33
> ("e1000e: Do not read ICR in Other interrupt"). Thanks to commit
> 0a8047ac68e5 ("e1000e: Fix msi-x interrupt automask") which cleared IAME
> from CTRL_EXT, reading ICR doesn't interfere with RxQ0, TxQ0 interrupts
> anymore. We handle the second part of the problem by not re-enabling the
> Other interrupt right away when there is overrun. Instead, we wait until
> traffic subsides, napi polling mode is exited and interrupts are
> re-enabled.
> 
> Reported-by: Lennart Sorensen <lsorense@csclub.uwaterloo.ca>
> Fixes: 16ecba59bc33 ("e1000e: Do not read ICR in Other interrupt")
> Signed-off-by: Benjamin Poirier <bpoirier@suse.com>
> Tested-by: Aaron Brown <aaron.f.brown@intel.com>
> ---
> drivers/net/ethernet/intel/e1000e/defines.h |  1 +
> drivers/net/ethernet/intel/e1000e/netdev.c  | 33 +++++++++++++++++++++++------
> 2 files changed, 27 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/e1000e/defines.h b/drivers/net/ethernet/intel/e1000e/defines.h
> index 0641c0098738..afb7ebe20b24 100644
> --- a/drivers/net/ethernet/intel/e1000e/defines.h
> +++ b/drivers/net/ethernet/intel/e1000e/defines.h
> @@ -398,6 +398,7 @@
> #define E1000_ICR_LSC           0x00000004 /* Link Status Change */
> #define E1000_ICR_RXSEQ         0x00000008 /* Rx sequence error */
> #define E1000_ICR_RXDMT0        0x00000010 /* Rx desc min. threshold (0) */
> +#define E1000_ICR_RXO           0x00000040 /* Receiver Overrun */
> #define E1000_ICR_RXT0          0x00000080 /* Rx timer intr (ring 0) */
> #define E1000_ICR_ECCER         0x00400000 /* Uncorrectable ECC Error */
> /* If this bit asserted, the driver should claim the interrupt */
> diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
> index 5a8ab1136566..803edd1a6401 100644
> --- a/drivers/net/ethernet/intel/e1000e/netdev.c
> +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
> @@ -1910,12 +1910,30 @@ static irqreturn_t e1000_msix_other(int __always_unused irq, void *data)
> 	struct net_device *netdev = data;
> 	struct e1000_adapter *adapter = netdev_priv(netdev);
> 	struct e1000_hw *hw = &adapter->hw;
> +	u32 icr;
> +	bool enable = true;
> +
> +	icr = er32(ICR);
> +	if (icr & E1000_ICR_RXO) {
> +		ew32(ICR, E1000_ICR_RXO);
> +		enable = false;
> +		/* napi poll will re-enable Other, make sure it runs */
> +		if (napi_schedule_prep(&adapter->napi)) {
> +			adapter->total_rx_bytes = 0;
> +			adapter->total_rx_packets = 0;
> +			__napi_schedule(&adapter->napi);
> +		}
> +	}
> +	if (icr & E1000_ICR_LSC) {
> +		ew32(ICR, E1000_ICR_LSC);
> +		hw->mac.get_link_status = true;
> +		/* guard against interrupt when we're going down */
> +		if (!test_bit(__E1000_DOWN, &adapter->state)) {
> +			mod_timer(&adapter->watchdog_timer, jiffies + 1);
> +		}
> +	}
> 
> -	hw->mac.get_link_status = true;
> -
> -	/* guard against interrupt when we're going down */
> -	if (!test_bit(__E1000_DOWN, &adapter->state)) {
> -		mod_timer(&adapter->watchdog_timer, jiffies + 1);
> +	if (enable && !test_bit(__E1000_DOWN, &adapter->state)) {
> 		ew32(IMS, E1000_IMS_OTHER);
> 	}
> 
> @@ -2687,7 +2705,8 @@ static int e1000e_poll(struct napi_struct *napi, int weight)
> 		napi_complete_done(napi, work_done);
> 		if (!test_bit(__E1000_DOWN, &adapter->state)) {
> 			if (adapter->msix_entries)
> -				ew32(IMS, adapter->rx_ring->ims_val);
> +				ew32(IMS, adapter->rx_ring->ims_val |
> +				     E1000_IMS_OTHER);
> 			else
> 				e1000_irq_enable(adapter);
> 		}
> @@ -4204,7 +4223,7 @@ static void e1000e_trigger_lsc(struct e1000_adapter *adapter)
> 	struct e1000_hw *hw = &adapter->hw;
> 
> 	if (adapter->msix_entries)
> -		ew32(ICS, E1000_ICS_OTHER);
> +		ew32(ICS, E1000_ICS_LSC | E1000_ICS_OTHER);
> 	else
> 		ew32(ICS, E1000_ICS_LSC);
> }

^ permalink raw reply

* Re: [PATCH v2 net-next] net: sk_buff rbnode reorg
From: Soheil Hassas Yeganeh @ 2017-09-19 18:33 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David Miller, netdev, Soheil Hassas Yeganeh, Wei Wang,
	Willem de Bruijn
In-Reply-To: <1505823264.29839.54.camel@edumazet-glaptop3.roam.corp.google.com>

On Tue, Sep 19, 2017 at 8:14 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> skb->rbnode shares space with skb->next, skb->prev and skb->tstamp
>
> Current uses (TCP receive ofo queue and netem) need to save/restore
> tstamp, while skb->dev is either NULL (TCP) or a constant for a given
> queue (netem).
>
> Since we plan using an RB tree for TCP retransmit queue to speedup SACK
> processing with large BDP, this patch exchanges skb->dev and
> skb->tstamp.
>
> This saves some overhead in both TCP and netem.
>
> v2: removes the swtstamp field from struct tcp_skb_cb
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Soheil Hassas Yeganeh <soheil@google.com>
> Cc: Wei Wang <weiwan@google.com>
> Cc: Willem de Bruijn <willemb@google.com>

Acked-by: Soheil Hassas Yeganeh <soheil@google.com>

Very nice!

^ permalink raw reply

* [patch net-next] team: fall back to hash if table entry is empty
From: Jim Hanko @ 2017-09-19 18:33 UTC (permalink / raw)
  To: jiri, netdev, linux-kernel; +Cc: Jim Hanko

If the hash to port mapping table does not have a valid port (i.e. when
a port goes down), fall back to the simple hashing mechanism to avoid
dropping packets.

Signed-off-by: Jim Hanko <hanko@drivescale.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
 drivers/net/team/team_mode_loadbalance.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/net/team/team_mode_loadbalance.c b/drivers/net/team/team_mode_loadbalance.c
index 1468ddf..a5ef970 100644
--- a/drivers/net/team/team_mode_loadbalance.c
+++ b/drivers/net/team/team_mode_loadbalance.c
@@ -137,7 +137,13 @@ static struct team_port *lb_htpm_select_tx_port(struct team *team,
 						struct sk_buff *skb,
 						unsigned char hash)
 {
-	return rcu_dereference_bh(LB_HTPM_PORT_BY_HASH(lb_priv, hash));
+	struct team_port *port;
+
+	port = rcu_dereference_bh(LB_HTPM_PORT_BY_HASH(lb_priv, hash));
+	if (likely(port))
+		return port;
+	/* If no valid port in the table, fall back to simple hash */
+	return lb_hash_select_tx_port(team, lb_priv, skb, hash);
 }
 
 struct lb_select_tx_port {
-- 
2.7.4

^ permalink raw reply related

* Re: [REGRESSION] Warning in tcp_fastretrans_alert() of net/ipv4/tcp_input.c
From: Yuchung Cheng @ 2017-09-19 18:16 UTC (permalink / raw)
  To: Oleksandr Natalenko
  Cc: Neal Cardwell, David S. Miller, Alexey Kuznetsov,
	Hideaki YOSHIFUJI, Netdev
In-Reply-To: <13e86265b8db3a5b1f1eaf8135ff4510@natalenko.name>

On Tue, Sep 19, 2017 at 4:04 AM, Oleksandr Natalenko
<oleksandr@natalenko.name> wrote:
> Hi.
>
> 18.09.2017 23:40, Yuchung Cheng wrote:
>>
>> I assume this kernel does not have the patch that Neal proposed in his
>> first reply?
>
>
> Correct.
>
>> The main warning needs to be triggered by another peculiar SACK that
>> kicks the sender into recovery again (after undo). Please let it run
>> longer if possible to see if we can get both. But the new data does
>> indicate the we can (validly) be in CA_Open with retrans_out > 0.
>
>
> OK, here it is:
>
> ===
> » LC_TIME=C jctl -kb | grep RIP
> …
> Sep 19 12:54:03 defiant kernel: RIP: 0010:tcp_undo_cwnd_reduction+0xbd/0xd0
> Sep 19 12:54:22 defiant kernel: RIP: 0010:tcp_undo_cwnd_reduction+0xbd/0xd0
> Sep 19 12:54:25 defiant kernel: RIP: 0010:tcp_undo_cwnd_reduction+0xbd/0xd0
> Sep 19 12:56:00 defiant kernel: RIP: 0010:tcp_fastretrans_alert+0x7c8/0x990
> Sep 19 12:57:07 defiant kernel: RIP: 0010:tcp_undo_cwnd_reduction+0xbd/0xd0
> Sep 19 12:57:14 defiant kernel: RIP: 0010:tcp_undo_cwnd_reduction+0xbd/0xd0
> Sep 19 12:58:04 defiant kernel: RIP: 0010:tcp_undo_cwnd_reduction+0xbd/0xd0
> …
> ===
>
> Note timestamps — two types of warning are distant in time, so didn't happen
> at once.
>
> While still running this kernel, anything else I can check for you?
Thanks. Based on all the experiments you did I believe there's other
code path than my hypothesis that'd cause the warning:
1) Neal's proposed F-RTO fix didn't work
2) the main warning is not being triggered together with the newly-instrumented
warning in undo
3) Disabling RACK stopped the warning

We couldn't figure out exactly what. So we'll do a bit code auditing
first to find more suspects

^ permalink raw reply

* Re: [PATCH 2/3] selftests: actually run the various net selftests
From: Willem de Bruijn @ 2017-09-19 18:14 UTC (permalink / raw)
  To: Josef Bacik
  Cc: Willem de Bruijn, Josef Bacik, David S. Miller, LKML,
	linux-kselftest, Network Development, Shuah Khan, Shuah Khan
In-Reply-To: <20170919133424.mzshgf67al7mv3fg@destiny>

On Tue, Sep 19, 2017 at 9:34 AM, Josef Bacik <josef@toxicpanda.com> wrote:
> On Mon, Sep 18, 2017 at 04:14:41PM -0600, Shuah Khan wrote:
>> On 09/18/2017 11:32 AM, josef@toxicpanda.com wrote:
>> > From: Josef Bacik <jbacik@fb.com>
>> >
>> > These self tests are just self contained binaries, they are not run by
>> > any of the scripts in the directory.  This means they need to be marked
>> > with TEST_GEN_PROGS to actually be run, not TEST_GEN_FILES.
>> >
>> > Signed-off-by: Josef Bacik <jbacik@fb.com>
>> > ---
>> >  tools/testing/selftests/net/Makefile | 4 ++--
>> >  1 file changed, 2 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
>> > index 3df542c84610..45a4e77a47c4 100644
>> > --- a/tools/testing/selftests/net/Makefile
>> > +++ b/tools/testing/selftests/net/Makefile
>> > @@ -6,8 +6,8 @@ CFLAGS += -I../../../../usr/include/
>> >  TEST_PROGS := run_netsocktests run_afpackettests test_bpf.sh netdevice.sh rtnetlink.sh
>> >  TEST_GEN_FILES =  socket
>> >  TEST_GEN_FILES += psock_fanout psock_tpacket
>> > -TEST_GEN_FILES += reuseport_bpf reuseport_bpf_cpu reuseport_bpf_numa
>> > -TEST_GEN_FILES += reuseport_dualstack msg_zerocopy reuseaddr_conflict
>> > +TEST_GEN_PROGS += reuseport_bpf reuseport_bpf_cpu reuseport_bpf_numa
>> > +TEST_GEN_PROGS += reuseport_dualstack msg_zerocopy reuseaddr_conflict
>>
>> Hmm. I see msg_zerocopy.sh for running msg_zerocopy. msg_zerocopy should
>> still stay in TEST_GEN_FILES and msg_zerocopy.sh needs to be added to
>> TEST_PROGS so it runs.
>>
>
> Actually the shell script requires arguments, it doesn't just run the test.
> I'll fix this to just omit the test for now as it's not setup to run properly.
>
> Willem, could you follow up with a patch so that the zero copy test is run
> properly the way you envision it running?  You need to make sure that
>
> make -C tools/testing/selftests TARGETS=net run_tests
>
> actually runs your zero copy test the way you expect it to, otherwise it's just
> sitting there collecting dust.  Thanks,

Will do.

In its current state, this test is really only meant to be run manually.
It demonstrates the API and outputs some information on stderr.

Zerocopy itself requires a two-host test. The feature is expressly
disabled over loopback.

But I can make this a pass/fail tests that exercises the interface
and notification channel and verifies that data was copied. It will
be a bit more work than just changing the default invocation of
msg_zerocopy.sh

^ permalink raw reply

* Re: [PATCH net-next 05/14] gtp: Remove special mtu handling
From: Tom Herbert @ 2017-09-19 18:12 UTC (permalink / raw)
  To: Harald Welte
  Cc: Tom Herbert, David S. Miller, Linux Kernel Network Developers,
	Pablo Neira Ayuso, Rohit Seth
In-Reply-To: <20170919114205.z2txj6rwo2row7u4@nataraja>

On Tue, Sep 19, 2017 at 4:42 AM, Harald Welte <laforge@gnumonks.org> wrote:
> Hi Tom,
>
> On Mon, Sep 18, 2017 at 05:38:55PM -0700, Tom Herbert wrote:
>> Removes MTU handling in gtp_build_skb_ip4. This is non standard relative
>> to how other tunneling protocols handle MTU. The model espoused is that
>> the inner interface should set it's MTU to be less than the expected
>> path MTU on the overlay network. Path MTU discovery is not typically
>> used for modifying tunnel MTUs.
>
> The point of the kernel GTP module is to interoperate with existing
> other GTP implementations and the practises established by cellular
> operators when operating GTP in their networks.
>
> While what you describe (chose interface MTU to be less than the
> expected path MTU) is generally best practise in the Linux IP/networking
> world, this is not generally reflected in the cellular
> universe. You see quite a bit of GTP fragmentation due to the fact
> that the transport network simply has to deal with the MTU that has
> been established via the control plane between SGSN and MS/UE, without
> the GGSN even being part of that negotiation.
>
> Also, you may very well have one "gtp0" tunnel device at the GGSN,
> but you are establishing individual GTP tunnels to dozesn to hundreds of
> different SGSNs at operators all over the world.  You cannot reliably
> set the "gtp0" interface MTU to "the path MTU of the overlay network",
> as the overlay network is in fact different for each of the SGSNs you're
> talking to - and each may have a different path MTU.
>
> So unless I'm missing something, I would currently vote for staying with
> the current code, which uses the path MTU to the specific destination IP
> address (the SGSN).
>
Okay, I'll modify tnl_update_pmtu so we can call it from GTP and not
have to replicate that function. I suspect VXLAN might also what this
at some point.

Tom

> Regards,
>         Harald
>
> --
> - Harald Welte <laforge@gnumonks.org>           http://laforge.gnumonks.org/
> ============================================================================
> "Privacy in residential applications is a desirable marketing option."
>                                                   (ETSI EN 300 175-7 Ch. A6)

^ permalink raw reply

* Re: pull-request: mac80211 2017-11-19
From: David Miller @ 2017-09-19 17:51 UTC (permalink / raw)
  To: johannes; +Cc: netdev, linux-wireless
In-Reply-To: <20170919072048.8484-1-johannes@sipsolutions.net>

From: Johannes Berg <johannes@sipsolutions.net>
Date: Tue, 19 Sep 2017 09:20:47 +0200

> Here's a new set of two small changes to prevent null pointer
> dereferences on malformed netlink messages.
> 
> Please pull and let me know if there's any problem.

Pulled, thank you.

^ permalink raw reply

* Re: [PATCH net-next 3/4] net: dsa: setup master ethtool after dsa_ptr
From: Florian Fainelli @ 2017-09-19 17:49 UTC (permalink / raw)
  To: Vivien Didelot, netdev; +Cc: linux-kernel, kernel, David S. Miller, Andrew Lunn
In-Reply-To: <20170919155700.14474-4-vivien.didelot@savoirfairelinux.com>

On 09/19/2017 08:56 AM, Vivien Didelot wrote:
> DSA overrides the master's ethtool ops so that we can inject its CPU
> port's statistics. Because of that, we need to setup the ethtool ops
> after the master's dsa_ptr pointer has been assigned, not before.

Yes, good point, technically this is a bugfix, but since we have changed
this quite often and the race is tiny, I am not positive we could a)
trigger this in real life, and b) provide a proper Fixes tag.

> 
> This patch setups the ethtool ops after dsa_ptr is assigned, and
> restores them before it gets cleared.
> 
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

^ permalink raw reply

* Re: [PATCH net-next 2/4] net: dsa: setup master ethtool unconditionally
From: Florian Fainelli @ 2017-09-19 17:48 UTC (permalink / raw)
  To: Vivien Didelot, netdev; +Cc: linux-kernel, kernel, David S. Miller, Andrew Lunn
In-Reply-To: <20170919155700.14474-3-vivien.didelot@savoirfairelinux.com>

On 09/19/2017 08:56 AM, Vivien Didelot wrote:
> When a DSA switch tree is meant to be applied, it already has a CPU
> port. Thus remove the condition of dst->cpu_dp.
> 
> Moreover, the next lines access dst->cpu_dp unconditionally.
> 
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
-- 
Florian

^ permalink raw reply

* [PATCH net-next v3 12/12] net: dsa: bcm_sf2: Utilize b53_{enable,disable}_port
From: Florian Fainelli @ 2017-09-19 17:46 UTC (permalink / raw)
  To: netdev; +Cc: davem, andrew, vivien.didelot, Florian Fainelli
In-Reply-To: <20170919174654.2122-1-f.fainelli@gmail.com>

Export b53_{enable,disable}_port and use these two functions in
bcm_sf2_port_setup and bcm_sf2_port_disable. The generic functions
cannot be used without wrapping because we need to manage additional
switch integration details (PHY, Broadcom tag etc.).

Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/dsa/b53/b53_common.c |  8 ++++----
 drivers/net/dsa/b53/b53_priv.h   |  2 ++
 drivers/net/dsa/bcm_sf2.c        | 26 ++------------------------
 3 files changed, 8 insertions(+), 28 deletions(-)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index c3f1cd2c33ea..a9f2a5b55a5e 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -502,8 +502,7 @@ void b53_imp_vlan_setup(struct dsa_switch *ds, int cpu_port)
 }
 EXPORT_SYMBOL(b53_imp_vlan_setup);
 
-static int b53_enable_port(struct dsa_switch *ds, int port,
-			   struct phy_device *phy)
+int b53_enable_port(struct dsa_switch *ds, int port, struct phy_device *phy)
 {
 	struct b53_device *dev = ds->priv;
 	unsigned int cpu_port = dev->cpu_port;
@@ -530,9 +529,9 @@ static int b53_enable_port(struct dsa_switch *ds, int port,
 
 	return 0;
 }
+EXPORT_SYMBOL(b53_enable_port);
 
-static void b53_disable_port(struct dsa_switch *ds, int port,
-			     struct phy_device *phy)
+void b53_disable_port(struct dsa_switch *ds, int port, struct phy_device *phy)
 {
 	struct b53_device *dev = ds->priv;
 	u8 reg;
@@ -542,6 +541,7 @@ static void b53_disable_port(struct dsa_switch *ds, int port,
 	reg |= PORT_CTRL_RX_DISABLE | PORT_CTRL_TX_DISABLE;
 	b53_write8(dev, B53_CTRL_PAGE, B53_PORT_CTRL(port), reg);
 }
+EXPORT_SYMBOL(b53_disable_port);
 
 void b53_brcm_hdr_setup(struct dsa_switch *ds, int port)
 {
diff --git a/drivers/net/dsa/b53/b53_priv.h b/drivers/net/dsa/b53/b53_priv.h
index 8f4f83e2e4bd..603c66d240d8 100644
--- a/drivers/net/dsa/b53/b53_priv.h
+++ b/drivers/net/dsa/b53/b53_priv.h
@@ -311,6 +311,8 @@ int b53_mirror_add(struct dsa_switch *ds, int port,
 		   struct dsa_mall_mirror_tc_entry *mirror, bool ingress);
 void b53_mirror_del(struct dsa_switch *ds, int port,
 		    struct dsa_mall_mirror_tc_entry *mirror);
+int b53_enable_port(struct dsa_switch *ds, int port, struct phy_device *phy);
+void b53_disable_port(struct dsa_switch *ds, int port, struct phy_device *phy);
 void b53_brcm_hdr_setup(struct dsa_switch *ds, int port);
 void b53_eee_enable_set(struct dsa_switch *ds, int port, bool enable);
 int b53_eee_init(struct dsa_switch *ds, int port, struct phy_device *phy);
diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
index 08639674947a..0072a959db5b 100644
--- a/drivers/net/dsa/bcm_sf2.c
+++ b/drivers/net/dsa/bcm_sf2.c
@@ -163,7 +163,6 @@ static int bcm_sf2_port_setup(struct dsa_switch *ds, int port,
 			      struct phy_device *phy)
 {
 	struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds);
-	s8 cpu_port = ds->dst->cpu_dp->index;
 	unsigned int i;
 	u32 reg;
 
@@ -184,9 +183,6 @@ static int bcm_sf2_port_setup(struct dsa_switch *ds, int port,
 		reg |= i << (PRT_TO_QID_SHIFT * i);
 	core_writel(priv, reg, CORE_PORT_TC2_QOS_MAP_PORT(port));
 
-	/* Clear the Rx and Tx disable bits and set to no spanning tree */
-	core_writel(priv, 0, CORE_G_PCTL_PORT(port));
-
 	/* Re-enable the GPHY and re-apply workarounds */
 	if (priv->int_phy_mask & 1 << port && priv->hw_params.num_gphy == 1) {
 		bcm_sf2_gphy_enable_set(ds, true);
@@ -209,23 +205,7 @@ static int bcm_sf2_port_setup(struct dsa_switch *ds, int port,
 	if (port == priv->moca_port)
 		bcm_sf2_port_intr_enable(priv, port);
 
-	/* Set this port, and only this one to be in the default VLAN,
-	 * if member of a bridge, restore its membership prior to
-	 * bringing down this port.
-	 */
-	reg = core_readl(priv, CORE_PORT_VLAN_CTL_PORT(port));
-	reg &= ~PORT_VLAN_CTRL_MASK;
-	reg |= (1 << port);
-	reg |= priv->dev->ports[port].vlan_ctl_mask;
-	core_writel(priv, reg, CORE_PORT_VLAN_CTL_PORT(port));
-
-	b53_imp_vlan_setup(ds, cpu_port);
-
-	/* If EEE was enabled, restore it */
-	if (priv->dev->ports[port].eee.eee_enabled)
-		b53_eee_enable_set(ds, port, true);
-
-	return 0;
+	return b53_enable_port(ds, port, phy);
 }
 
 static void bcm_sf2_port_disable(struct dsa_switch *ds, int port,
@@ -248,9 +228,7 @@ static void bcm_sf2_port_disable(struct dsa_switch *ds, int port,
 	else
 		off = CORE_G_PCTL_PORT(port);
 
-	reg = core_readl(priv, off);
-	reg |= RX_DIS | TX_DIS;
-	core_writel(priv, reg, off);
+	b53_disable_port(ds, port, phy);
 
 	/* Power down the port memory */
 	reg = core_readl(priv, CORE_MEM_PSM_VDD_CTRL);
-- 
2.9.3

^ permalink raw reply related

* [PATCH net-next v3 11/12] net: dsa: bcm_sf2: Use SF2_NUM_EGRESS_QUEUES for CFP
From: Florian Fainelli @ 2017-09-19 17:46 UTC (permalink / raw)
  To: netdev; +Cc: davem, andrew, vivien.didelot, Florian Fainelli
In-Reply-To: <20170919174654.2122-1-f.fainelli@gmail.com>

The magic number 8 in 3 locations in bcm_sf2_cfp.c actually designates
the number of switch port egress queues, so use that define instead of
open-coding it.

Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/dsa/bcm_sf2_cfp.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/dsa/bcm_sf2_cfp.c b/drivers/net/dsa/bcm_sf2_cfp.c
index 8a1da7e67707..94649e1481ec 100644
--- a/drivers/net/dsa/bcm_sf2_cfp.c
+++ b/drivers/net/dsa/bcm_sf2_cfp.c
@@ -144,7 +144,7 @@ static int bcm_sf2_cfp_rule_set(struct dsa_switch *ds, int port,
 	 * destination port is enabled and that we are within the
 	 * number of ports supported by the switch
 	 */
-	port_num = fs->ring_cookie / 8;
+	port_num = fs->ring_cookie / SF2_NUM_EGRESS_QUEUES;
 
 	if (fs->ring_cookie == RX_CLS_FLOW_DISC ||
 	    !(BIT(port_num) & ds->enabled_port_mask) ||
@@ -280,7 +280,7 @@ static int bcm_sf2_cfp_rule_set(struct dsa_switch *ds, int port,
 	 * We have a small oddity where Port 6 just does not have a
 	 * valid bit here (so we subtract by one).
 	 */
-	queue_num = fs->ring_cookie % 8;
+	queue_num = fs->ring_cookie % SF2_NUM_EGRESS_QUEUES;
 	if (port_num >= 7)
 		port_num -= 1;
 
@@ -401,7 +401,7 @@ static int bcm_sf2_cfp_rule_get(struct bcm_sf2_priv *priv, int port,
 	/* There is no Port 6, so we compensate for that here */
 	if (nfc->fs.ring_cookie >= 6)
 		nfc->fs.ring_cookie++;
-	nfc->fs.ring_cookie *= 8;
+	nfc->fs.ring_cookie *= SF2_NUM_EGRESS_QUEUES;
 
 	/* Extract the destination queue */
 	queue_num = (reg >> NEW_TC_SHIFT) & NEW_TC_MASK;
-- 
2.9.3

^ permalink raw reply related

* [PATCH net-next v3 10/12] net: dsa: b53: Export b53_imp_vlan_setup()
From: Florian Fainelli @ 2017-09-19 17:46 UTC (permalink / raw)
  To: netdev; +Cc: davem, andrew, vivien.didelot, Florian Fainelli
In-Reply-To: <20170919174654.2122-1-f.fainelli@gmail.com>

bcm_sf2 and b53 do exactly the same thing, so share that piece.

Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/dsa/b53/b53_common.c |  3 ++-
 drivers/net/dsa/b53/b53_priv.h   |  1 +
 drivers/net/dsa/bcm_sf2.c        | 23 +----------------------
 3 files changed, 4 insertions(+), 23 deletions(-)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 4e37ec27e496..c3f1cd2c33ea 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -484,7 +484,7 @@ static int b53_fast_age_vlan(struct b53_device *dev, u16 vid)
 	return b53_flush_arl(dev, FAST_AGE_VLAN);
 }
 
-static void b53_imp_vlan_setup(struct dsa_switch *ds, int cpu_port)
+void b53_imp_vlan_setup(struct dsa_switch *ds, int cpu_port)
 {
 	struct b53_device *dev = ds->priv;
 	unsigned int i;
@@ -500,6 +500,7 @@ static void b53_imp_vlan_setup(struct dsa_switch *ds, int cpu_port)
 		b53_write16(dev, B53_PVLAN_PAGE, B53_PVLAN_PORT_MASK(i), pvlan);
 	}
 }
+EXPORT_SYMBOL(b53_imp_vlan_setup);
 
 static int b53_enable_port(struct dsa_switch *ds, int port,
 			   struct phy_device *phy)
diff --git a/drivers/net/dsa/b53/b53_priv.h b/drivers/net/dsa/b53/b53_priv.h
index aabe80eab25d..8f4f83e2e4bd 100644
--- a/drivers/net/dsa/b53/b53_priv.h
+++ b/drivers/net/dsa/b53/b53_priv.h
@@ -284,6 +284,7 @@ static inline int b53_switch_get_reset_gpio(struct b53_device *dev)
 #endif
 
 /* Exported functions towards other drivers */
+void b53_imp_vlan_setup(struct dsa_switch *ds, int cpu_port);
 void b53_get_strings(struct dsa_switch *ds, int port, uint8_t *data);
 void b53_get_ethtool_stats(struct dsa_switch *ds, int port, uint64_t *data);
 int b53_get_sset_count(struct dsa_switch *ds);
diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
index 4e8ef4c07eab..08639674947a 100644
--- a/drivers/net/dsa/bcm_sf2.c
+++ b/drivers/net/dsa/bcm_sf2.c
@@ -40,27 +40,6 @@ static enum dsa_tag_protocol bcm_sf2_sw_get_tag_protocol(struct dsa_switch *ds)
 	return DSA_TAG_PROTO_BRCM;
 }
 
-static void bcm_sf2_imp_vlan_setup(struct dsa_switch *ds, int cpu_port)
-{
-	struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds);
-	unsigned int i;
-	u32 reg;
-
-	/* Enable the IMP Port to be in the same VLAN as the other ports
-	 * on a per-port basis such that we only have Port i and IMP in
-	 * the same VLAN.
-	 */
-	for (i = 0; i < priv->hw_params.num_ports; i++) {
-		if (!((1 << i) & ds->enabled_port_mask))
-			continue;
-
-		reg = core_readl(priv, CORE_PORT_VLAN_CTL_PORT(i));
-		reg |= (1 << cpu_port);
-		core_writel(priv, reg, CORE_PORT_VLAN_CTL_PORT(i));
-	}
-}
-
-
 static void bcm_sf2_imp_setup(struct dsa_switch *ds, int port)
 {
 	struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds);
@@ -240,7 +219,7 @@ static int bcm_sf2_port_setup(struct dsa_switch *ds, int port,
 	reg |= priv->dev->ports[port].vlan_ctl_mask;
 	core_writel(priv, reg, CORE_PORT_VLAN_CTL_PORT(port));
 
-	bcm_sf2_imp_vlan_setup(ds, cpu_port);
+	b53_imp_vlan_setup(ds, cpu_port);
 
 	/* If EEE was enabled, restore it */
 	if (priv->dev->ports[port].eee.eee_enabled)
-- 
2.9.3

^ permalink raw reply related

* [PATCH net-next v3 09/12] net: dsa: b53: Wire-up EEE
From: Florian Fainelli @ 2017-09-19 17:46 UTC (permalink / raw)
  To: netdev; +Cc: davem, andrew, vivien.didelot, Florian Fainelli
In-Reply-To: <20170919174654.2122-1-f.fainelli@gmail.com>

Add support for enabling and disabling EEE, as well as re-negotiating it in
.adjust_link() and in .port_enable().

Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/dsa/b53/b53_common.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index 491e4ffa8a0e..4e37ec27e496 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -523,6 +523,10 @@ static int b53_enable_port(struct dsa_switch *ds, int port,
 
 	b53_imp_vlan_setup(ds, cpu_port);
 
+	/* If EEE was enabled, restore it */
+	if (dev->ports[port].eee.eee_enabled)
+		b53_eee_enable_set(ds, port, true);
+
 	return 0;
 }
 
@@ -879,6 +883,7 @@ static void b53_adjust_link(struct dsa_switch *ds, int port,
 			    struct phy_device *phydev)
 {
 	struct b53_device *dev = ds->priv;
+	struct ethtool_eee *p = &dev->ports[port].eee;
 	u8 rgmii_ctrl = 0, reg = 0, off;
 
 	if (!phy_is_pseudo_fixed_link(phydev))
@@ -1000,6 +1005,9 @@ static void b53_adjust_link(struct dsa_switch *ds, int port,
 			b53_write8(dev, B53_CTRL_PAGE, po_reg, gmii_po);
 		}
 	}
+
+	/* Re-negotiate EEE if it was enabled already */
+	p->eee_enabled = b53_eee_init(ds, port, phydev);
 }
 
 int b53_vlan_filtering(struct dsa_switch *ds, int port, bool vlan_filtering)
@@ -1605,6 +1613,8 @@ static const struct dsa_switch_ops b53_switch_ops = {
 	.adjust_link		= b53_adjust_link,
 	.port_enable		= b53_enable_port,
 	.port_disable		= b53_disable_port,
+	.get_mac_eee		= b53_get_mac_eee,
+	.set_mac_eee		= b53_set_mac_eee,
 	.port_bridge_join	= b53_br_join,
 	.port_bridge_leave	= b53_br_leave,
 	.port_stp_state_set	= b53_br_set_stp_state,
-- 
2.9.3

^ permalink raw reply related

* [PATCH net-next v3 08/12] net: dsa: b53: Move EEE functions to b53
From: Florian Fainelli @ 2017-09-19 17:46 UTC (permalink / raw)
  To: netdev; +Cc: davem, andrew, vivien.didelot, Florian Fainelli
In-Reply-To: <20170919174654.2122-1-f.fainelli@gmail.com>

Move the bcm_sf2 EEE-related functions to the b53 driver because this is shared
code amongst Gigabit capable switch, only 5325 and 5365 are too old to support
that.

Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/dsa/b53/b53_common.c | 63 ++++++++++++++++++++++++++++++++++++++
 drivers/net/dsa/b53/b53_priv.h   |  5 +++
 drivers/net/dsa/bcm_sf2.c        | 66 ++++------------------------------------
 drivers/net/dsa/bcm_sf2.h        |  2 --
 drivers/net/dsa/bcm_sf2_regs.h   |  3 --
 5 files changed, 74 insertions(+), 65 deletions(-)

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index aa2187c71ea5..491e4ffa8a0e 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -1531,6 +1531,69 @@ void b53_mirror_del(struct dsa_switch *ds, int port,
 }
 EXPORT_SYMBOL(b53_mirror_del);
 
+void b53_eee_enable_set(struct dsa_switch *ds, int port, bool enable)
+{
+	struct b53_device *dev = ds->priv;
+	u16 reg;
+
+	b53_read16(dev, B53_EEE_PAGE, B53_EEE_EN_CTRL, &reg);
+	if (enable)
+		reg |= BIT(port);
+	else
+		reg &= ~BIT(port);
+	b53_write16(dev, B53_EEE_PAGE, B53_EEE_EN_CTRL, reg);
+}
+EXPORT_SYMBOL(b53_eee_enable_set);
+
+
+/* Returns 0 if EEE was not enabled, or 1 otherwise
+ */
+int b53_eee_init(struct dsa_switch *ds, int port, struct phy_device *phy)
+{
+	int ret;
+
+	ret = phy_init_eee(phy, 0);
+	if (ret)
+		return 0;
+
+	b53_eee_enable_set(ds, port, true);
+
+	return 1;
+}
+EXPORT_SYMBOL(b53_eee_init);
+
+int b53_get_mac_eee(struct dsa_switch *ds, int port, struct ethtool_eee *e)
+{
+	struct b53_device *dev = ds->priv;
+	struct ethtool_eee *p = &dev->ports[port].eee;
+	u16 reg;
+
+	if (is5325(dev) || is5365(dev))
+		return -EOPNOTSUPP;
+
+	b53_read16(dev, B53_EEE_PAGE, B53_EEE_LPI_INDICATE, &reg);
+	e->eee_enabled = p->eee_enabled;
+	e->eee_active = !!(reg & BIT(port));
+
+	return 0;
+}
+EXPORT_SYMBOL(b53_get_mac_eee);
+
+int b53_set_mac_eee(struct dsa_switch *ds, int port, struct ethtool_eee *e)
+{
+	struct b53_device *dev = ds->priv;
+	struct ethtool_eee *p = &dev->ports[port].eee;
+
+	if (is5325(dev) || is5365(dev))
+		return -EOPNOTSUPP;
+
+	p->eee_enabled = e->eee_enabled;
+	b53_eee_enable_set(ds, port, e->eee_enabled);
+
+	return 0;
+}
+EXPORT_SYMBOL(b53_set_mac_eee);
+
 static const struct dsa_switch_ops b53_switch_ops = {
 	.get_tag_protocol	= b53_get_tag_protocol,
 	.setup			= b53_setup,
diff --git a/drivers/net/dsa/b53/b53_priv.h b/drivers/net/dsa/b53/b53_priv.h
index 77102f685da0..aabe80eab25d 100644
--- a/drivers/net/dsa/b53/b53_priv.h
+++ b/drivers/net/dsa/b53/b53_priv.h
@@ -70,6 +70,7 @@ enum {
 
 struct b53_port {
 	u16		vlan_ctl_mask;
+	struct ethtool_eee eee;
 };
 
 struct b53_vlan {
@@ -310,5 +311,9 @@ int b53_mirror_add(struct dsa_switch *ds, int port,
 void b53_mirror_del(struct dsa_switch *ds, int port,
 		    struct dsa_mall_mirror_tc_entry *mirror);
 void b53_brcm_hdr_setup(struct dsa_switch *ds, int port);
+void b53_eee_enable_set(struct dsa_switch *ds, int port, bool enable);
+int b53_eee_init(struct dsa_switch *ds, int port, struct phy_device *phy);
+int b53_get_mac_eee(struct dsa_switch *ds, int port, struct ethtool_eee *e);
+int b53_set_mac_eee(struct dsa_switch *ds, int port, struct ethtool_eee *e);
 
 #endif
diff --git a/drivers/net/dsa/bcm_sf2.c b/drivers/net/dsa/bcm_sf2.c
index 49cb51223f70..4e8ef4c07eab 100644
--- a/drivers/net/dsa/bcm_sf2.c
+++ b/drivers/net/dsa/bcm_sf2.c
@@ -107,19 +107,6 @@ static void bcm_sf2_imp_setup(struct dsa_switch *ds, int port)
 	core_writel(priv, reg, offset);
 }
 
-static void bcm_sf2_eee_enable_set(struct dsa_switch *ds, int port, bool enable)
-{
-	struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds);
-	u32 reg;
-
-	reg = core_readl(priv, CORE_EEE_EN_CTRL);
-	if (enable)
-		reg |= 1 << port;
-	else
-		reg &= ~(1 << port);
-	core_writel(priv, reg, CORE_EEE_EN_CTRL);
-}
-
 static void bcm_sf2_gphy_enable_set(struct dsa_switch *ds, bool enable)
 {
 	struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds);
@@ -256,8 +243,8 @@ static int bcm_sf2_port_setup(struct dsa_switch *ds, int port,
 	bcm_sf2_imp_vlan_setup(ds, cpu_port);
 
 	/* If EEE was enabled, restore it */
-	if (priv->port_sts[port].eee.eee_enabled)
-		bcm_sf2_eee_enable_set(ds, port, true);
+	if (priv->dev->ports[port].eee.eee_enabled)
+		b53_eee_enable_set(ds, port, true);
 
 	return 0;
 }
@@ -292,47 +279,6 @@ static void bcm_sf2_port_disable(struct dsa_switch *ds, int port,
 	core_writel(priv, reg, CORE_MEM_PSM_VDD_CTRL);
 }
 
-/* Returns 0 if EEE was not enabled, or 1 otherwise
- */
-static int bcm_sf2_eee_init(struct dsa_switch *ds, int port,
-			    struct phy_device *phy)
-{
-	int ret;
-
-	ret = phy_init_eee(phy, 0);
-	if (ret)
-		return 0;
-
-	bcm_sf2_eee_enable_set(ds, port, true);
-
-	return 1;
-}
-
-static int bcm_sf2_sw_get_mac_eee(struct dsa_switch *ds, int port,
-				  struct ethtool_eee *e)
-{
-	struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds);
-	struct ethtool_eee *p = &priv->port_sts[port].eee;
-	u32 reg;
-
-	reg = core_readl(priv, CORE_EEE_LPI_INDICATE);
-	e->eee_enabled = p->eee_enabled;
-	e->eee_active = !!(reg & (1 << port));
-
-	return 0;
-}
-
-static int bcm_sf2_sw_set_mac_eee(struct dsa_switch *ds, int port,
-				  struct ethtool_eee *e)
-{
-	struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds);
-	struct ethtool_eee *p = &priv->port_sts[port].eee;
-
-	p->eee_enabled = e->eee_enabled;
-	bcm_sf2_eee_enable_set(ds, port, e->eee_enabled);
-
-	return 0;
-}
 
 static int bcm_sf2_sw_indir_rw(struct bcm_sf2_priv *priv, int op, int addr,
 			       int regnum, u16 val)
@@ -567,7 +513,7 @@ static void bcm_sf2_sw_adjust_link(struct dsa_switch *ds, int port,
 				   struct phy_device *phydev)
 {
 	struct bcm_sf2_priv *priv = bcm_sf2_to_priv(ds);
-	struct ethtool_eee *p = &priv->port_sts[port].eee;
+	struct ethtool_eee *p = &priv->dev->ports[port].eee;
 	u32 id_mode_dis = 0, port_mode;
 	const char *str = NULL;
 	u32 reg, offset;
@@ -649,7 +595,7 @@ static void bcm_sf2_sw_adjust_link(struct dsa_switch *ds, int port,
 	core_writel(priv, reg, offset);
 
 	if (!phydev->is_pseudo_fixed_link)
-		p->eee_enabled = bcm_sf2_eee_init(ds, port, phydev);
+		p->eee_enabled = b53_eee_init(ds, port, phydev);
 }
 
 static void bcm_sf2_sw_fixed_link_update(struct dsa_switch *ds, int port,
@@ -978,8 +924,8 @@ static const struct dsa_switch_ops bcm_sf2_ops = {
 	.set_wol		= bcm_sf2_sw_set_wol,
 	.port_enable		= bcm_sf2_port_setup,
 	.port_disable		= bcm_sf2_port_disable,
-	.get_mac_eee		= bcm_sf2_sw_get_mac_eee,
-	.set_mac_eee		= bcm_sf2_sw_set_mac_eee,
+	.get_mac_eee		= b53_get_mac_eee,
+	.set_mac_eee		= b53_set_mac_eee,
 	.port_bridge_join	= b53_br_join,
 	.port_bridge_leave	= b53_br_leave,
 	.port_stp_state_set	= b53_br_set_stp_state,
diff --git a/drivers/net/dsa/bcm_sf2.h b/drivers/net/dsa/bcm_sf2.h
index 02c499f9c56b..1922e027ff59 100644
--- a/drivers/net/dsa/bcm_sf2.h
+++ b/drivers/net/dsa/bcm_sf2.h
@@ -48,8 +48,6 @@ struct bcm_sf2_hw_params {
 
 struct bcm_sf2_port_status {
 	unsigned int link;
-
-	struct ethtool_eee eee;
 };
 
 struct bcm_sf2_cfp_priv {
diff --git a/drivers/net/dsa/bcm_sf2_regs.h b/drivers/net/dsa/bcm_sf2_regs.h
index 788361ad68a0..d8b8074a47b9 100644
--- a/drivers/net/dsa/bcm_sf2_regs.h
+++ b/drivers/net/dsa/bcm_sf2_regs.h
@@ -244,9 +244,6 @@ enum bcm_sf2_reg_offs {
 
 #define CORE_JOIN_ALL_VLAN_EN		0xd140
 
-#define CORE_EEE_EN_CTRL		0x24800
-#define CORE_EEE_LPI_INDICATE		0x24810
-
 #define CORE_CFP_ACC			0x28000
 #define  OP_STR_DONE			(1 << 0)
 #define  OP_SEL_SHIFT			1
-- 
2.9.3

^ permalink raw reply related


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