Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next v3] net: phy: broadcom: add 1000Base-X support for BCM54616S
From: Tao Ren @ 2019-08-04  4:48 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: Andrew Lunn, Florian Fainelli, Heiner Kallweit, David S . Miller,
	Arun Parameswaran, Justin Chen, netdev, lkml,
	openbmc@lists.ozlabs.org
In-Reply-To: <CA+h21hrOEape89MTqCUyGFt=f6ba7Q-2KcOsN_Vw2Qv8iq86jw@mail.gmail.com>

Hi Vladimir,

On 8/3/19 6:49 AM, Vladimir Oltean wrote:
> Hi Tao,
> 
> On Sat, 3 Aug 2019 at 00:56, Tao Ren <taoren@fb.com> wrote:
>>
>> genphy_read_status() cannot report correct link speed when BCM54616S PHY
>> is configured in RGMII->1000Base-KX mode (for example, on Facebook CMM
>> BMC platform), and it is because speed-related fields in MII registers
>> are assigned different meanings in 1000X register set. Actually there
>> is no speed field in 1000X register set because link speed is always
>> 1000 Mb/s.
>>
>> The patch adds "probe" callback to detect PHY's operation mode based on
>> INTERF_SEL[1:0] pins and 1000X/100FX selection bit in SerDES 100-FX
>> Control register. Besides, link speed is manually set to 1000 Mb/s in
>> "read_status" callback if PHY-switch link is 1000Base-X.
>>
>> Signed-off-by: Tao Ren <taoren@fb.com>
>> ---
>>  Changes in v3:
>>   - rename bcm5482_read_status to bcm54xx_read_status so the callback can
>>     be shared by BCM5482 and BCM54616S.
>>  Changes in v2:
>>   - Auto-detect PHY operation mode instead of passing DT node.
>>   - move PHY mode auto-detect logic from config_init to probe callback.
>>   - only set speed (not including duplex) in read_status callback.
>>   - update patch description with more background to avoid confusion.
>>   - patch #1 in the series ("net: phy: broadcom: set features explicitly
>>     for BCM54616") is dropped: the fix should go to get_features callback
>>     which may potentially depend on this patch.
>>
>>  drivers/net/phy/broadcom.c | 41 +++++++++++++++++++++++++++++++++-----
>>  include/linux/brcmphy.h    | 10 ++++++++--
>>  2 files changed, 44 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/net/phy/broadcom.c b/drivers/net/phy/broadcom.c
>> index 937d0059e8ac..ecad8a201a09 100644
>> --- a/drivers/net/phy/broadcom.c
>> +++ b/drivers/net/phy/broadcom.c
>> @@ -383,9 +383,9 @@ static int bcm5482_config_init(struct phy_device *phydev)
>>                 /*
>>                  * Select 1000BASE-X register set (primary SerDes)
>>                  */
>> -               reg = bcm_phy_read_shadow(phydev, BCM5482_SHD_MODE);
>> -               bcm_phy_write_shadow(phydev, BCM5482_SHD_MODE,
>> -                                    reg | BCM5482_SHD_MODE_1000BX);
>> +               reg = bcm_phy_read_shadow(phydev, BCM54XX_SHD_MODE);
>> +               bcm_phy_write_shadow(phydev, BCM54XX_SHD_MODE,
>> +                                    reg | BCM54XX_SHD_MODE_1000BX);
>>
>>                 /*
>>                  * LED1=ACTIVITYLED, LED3=LINKSPD[2]
>> @@ -409,7 +409,7 @@ static int bcm5482_config_init(struct phy_device *phydev)
>>         return err;
>>  }
>>
>> -static int bcm5482_read_status(struct phy_device *phydev)
>> +static int bcm54xx_read_status(struct phy_device *phydev)
>>  {
>>         int err;
>>
>> @@ -464,6 +464,35 @@ static int bcm54616s_config_aneg(struct phy_device *phydev)
>>         return ret;
>>  }
>>
>> +static int bcm54616s_probe(struct phy_device *phydev)
>> +{
>> +       int val, intf_sel;
>> +
>> +       val = bcm_phy_read_shadow(phydev, BCM54XX_SHD_MODE);
>> +       if (val < 0)
>> +               return val;
>> +
>> +       /* The PHY is strapped in RGMII to fiber mode when INTERF_SEL[1:0]
>> +        * is 01b.
>> +        */
>> +       intf_sel = (val & BCM54XX_SHD_INTF_SEL_MASK) >> 1;
>> +       if (intf_sel == 1) {
>> +               val = bcm_phy_read_shadow(phydev, BCM54616S_SHD_100FX_CTRL);
>> +               if (val < 0)
>> +                       return val;
>> +
>> +               /* Bit 0 of the SerDes 100-FX Control register, when set
>> +                * to 1, sets the MII/RGMII -> 100BASE-FX configuration.
>> +                * When this bit is set to 0, it sets the GMII/RGMII ->
>> +                * 1000BASE-X configuration.
>> +                */
>> +               if (!(val & BCM54616S_100FX_MODE))
>> +                       phydev->dev_flags |= PHY_BCM_FLAGS_MODE_1000BX;
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>>  static int brcm_phy_setbits(struct phy_device *phydev, int reg, int set)
>>  {
>>         int val;
>> @@ -655,6 +684,8 @@ static struct phy_driver broadcom_drivers[] = {
>>         .config_aneg    = bcm54616s_config_aneg,
>>         .ack_interrupt  = bcm_phy_ack_intr,
>>         .config_intr    = bcm_phy_config_intr,
>> +       .read_status    = bcm54xx_read_status,
>> +       .probe          = bcm54616s_probe,
>>  }, {
>>         .phy_id         = PHY_ID_BCM5464,
>>         .phy_id_mask    = 0xfffffff0,
>> @@ -689,7 +720,7 @@ static struct phy_driver broadcom_drivers[] = {
>>         .name           = "Broadcom BCM5482",
>>         /* PHY_GBIT_FEATURES */
>>         .config_init    = bcm5482_config_init,
>> -       .read_status    = bcm5482_read_status,
>> +       .read_status    = bcm54xx_read_status,
>>         .ack_interrupt  = bcm_phy_ack_intr,
>>         .config_intr    = bcm_phy_config_intr,
>>  }, {
>> diff --git a/include/linux/brcmphy.h b/include/linux/brcmphy.h
>> index 6db2d9a6e503..b475e7f20d28 100644
>> --- a/include/linux/brcmphy.h
>> +++ b/include/linux/brcmphy.h
>> @@ -200,9 +200,15 @@
>>  #define BCM5482_SHD_SSD                0x14    /* 10100: Secondary SerDes control */
>>  #define BCM5482_SHD_SSD_LEDM   0x0008  /* SSD LED Mode enable */
>>  #define BCM5482_SHD_SSD_EN     0x0001  /* SSD enable */
>> -#define BCM5482_SHD_MODE       0x1f    /* 11111: Mode Control Register */
>> -#define BCM5482_SHD_MODE_1000BX        0x0001  /* Enable 1000BASE-X registers */
>>
>> +/* 10011: SerDes 100-FX Control Register */
>> +#define BCM54616S_SHD_100FX_CTRL       0x13
>> +#define        BCM54616S_100FX_MODE            BIT(0)  /* 100-FX SerDes Enable */
>> +
>> +/* 11111: Mode Control Register */
>> +#define BCM54XX_SHD_MODE               0x1f
>> +#define BCM54XX_SHD_INTF_SEL_MASK      GENMASK(2, 1)   /* INTERF_SEL[1:0] */
>> +#define BCM54XX_SHD_MODE_1000BX                BIT(0)  /* Enable 1000-X registers */
>>
>>  /*
>>   * EXPANSION SHADOW ACCESS REGISTERS.  (PHY REG 0x15, 0x16, and 0x17)
>> --
>> 2.17.1
>>
> 
> The patchset looks better now. But is it ok, I wonder, to keep
> PHY_BCM_FLAGS_MODE_1000BX in phydev->dev_flags, considering that
> phy_attach_direct is overwriting it?

I checked ftgmac100 driver (used on my machine) and it calls phy_connect_direct which passes phydev->dev_flags when calling phy_attach_direct: that explains why the flag is not cleared in my case.

Can you give me some suggestions since dev_flags may be override in other calling paths? For example, is it good idea to move the auto-detect logic from probe to config_init? Or are there other fields in phy_device structure that can be used to store PHY-switch link type? Or maybe I should just include the auto-detect logic in read_status callback?


Thanks,

Tao

^ permalink raw reply

* Re: [PATCH bpf-next 1/2] selftests/bpf: add loop test 4
From: Yonghong Song @ 2019-08-04  5:29 UTC (permalink / raw)
  To: Alexei Starovoitov, davem@davemloft.net
  Cc: daniel@iogearbox.net, netdev@vger.kernel.org, bpf@vger.kernel.org,
	Kernel Team
In-Reply-To: <20190802233344.863418-2-ast@kernel.org>



On 8/2/19 4:33 PM, Alexei Starovoitov wrote:
> Add a test that returns a 'random' number between [0, 2^20)
> If state pruning is not working correctly for loop body the number of
> processed insns will be 2^20 * num_of_insns_in_loop_body and the program
> will be rejected.

The maximum processed insns will be 2^20 or 2^20 * 
num_of_insns_in_loop_body? I thought the verifier will
stop processing once processed insns reach 1M?

Could you elaborate which potential issues in verifier
you try to cover with this test case? Extra tests are
always welcome. We already have scale/loop tests and some
(e.g., strobemeta tests) are more complex than this one.
Maybe you have something in mind for this particular
test? Putting in the commit message may help people understand
the concerns.

> 
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> ---
>   .../bpf/prog_tests/bpf_verif_scale.c          |  1 +
>   tools/testing/selftests/bpf/progs/loop4.c     | 23 +++++++++++++++++++
>   2 files changed, 24 insertions(+)
>   create mode 100644 tools/testing/selftests/bpf/progs/loop4.c
> 
> diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c b/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c
> index b4be96162ff4..757e39540eda 100644
> --- a/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c
> +++ b/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c
> @@ -71,6 +71,7 @@ void test_bpf_verif_scale(void)
>   
>   		{ "loop1.o", BPF_PROG_TYPE_RAW_TRACEPOINT },
>   		{ "loop2.o", BPF_PROG_TYPE_RAW_TRACEPOINT },
> +		{ "loop4.o", BPF_PROG_TYPE_RAW_TRACEPOINT },

The program is more like a BPF_PROG_TYPE_SCHED_CLS type than
a BPF_PROG_TYPE_RAW_TRACEPOINT?

>   
>   		/* partial unroll. 19k insn in a loop.
>   		 * Total program size 20.8k insn.
> diff --git a/tools/testing/selftests/bpf/progs/loop4.c b/tools/testing/selftests/bpf/progs/loop4.c
> new file mode 100644
> index 000000000000..3e7ee14fddbd
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/loop4.c
> @@ -0,0 +1,23 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) 2019 Facebook
> +#include <linux/sched.h>
> +#include <linux/ptrace.h>

Since the program is a networking type,
the above two headers are probably unneeded.

> +#include <stdint.h>
> +#include <stddef.h>
> +#include <stdbool.h>
> +#include <linux/bpf.h>
> +#include "bpf_helpers.h"
> +
> +char _license[] SEC("license") = "GPL";
> +
> +SEC("socket")
> +int combinations(volatile struct __sk_buff* skb)
> +{
> +	int ret = 0, i;
> +
> +#pragma nounroll
> +	for (i = 0; i < 20; i++)
> +		if (skb->len)
> +			ret |= 1 << i;
> +	return ret;
> +}
> 

^ permalink raw reply

* Re: [PATCH bpf-next 2/2] selftests/bpf: add loop test 5
From: Yonghong Song @ 2019-08-04  5:45 UTC (permalink / raw)
  To: Alexei Starovoitov, davem@davemloft.net
  Cc: daniel@iogearbox.net, netdev@vger.kernel.org, bpf@vger.kernel.org,
	Kernel Team
In-Reply-To: <20190802233344.863418-3-ast@kernel.org>



On 8/2/19 4:33 PM, Alexei Starovoitov wrote:
> Add a test with multiple exit conditions.
> It's not an infinite loop only when the verifier can properly track
> all math on variable 'i' through all possible ways of executing this loop.

Agreed with motivation of this test.

> 
> barrier()s are needed to disable llvm optimization that combines multiple
> branches into fewer branches.
> 
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> ---
>   .../bpf/prog_tests/bpf_verif_scale.c          |  1 +
>   tools/testing/selftests/bpf/progs/loop5.c     | 37 +++++++++++++++++++
>   2 files changed, 38 insertions(+)
>   create mode 100644 tools/testing/selftests/bpf/progs/loop5.c
> 
> diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c b/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c
> index 757e39540eda..29615a4a9362 100644
> --- a/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c
> +++ b/tools/testing/selftests/bpf/prog_tests/bpf_verif_scale.c
> @@ -72,6 +72,7 @@ void test_bpf_verif_scale(void)
>   		{ "loop1.o", BPF_PROG_TYPE_RAW_TRACEPOINT },
>   		{ "loop2.o", BPF_PROG_TYPE_RAW_TRACEPOINT },
>   		{ "loop4.o", BPF_PROG_TYPE_RAW_TRACEPOINT }, > +		{ "loop5.o", BPF_PROG_TYPE_RAW_TRACEPOINT },

More like a BPF_PROG_TYPE_SCHED_CLS type although probably it does not 
matter as we did not attach it to anywhere?

>   
>   		/* partial unroll. 19k insn in a loop.
>   		 * Total program size 20.8k insn.
> diff --git a/tools/testing/selftests/bpf/progs/loop5.c b/tools/testing/selftests/bpf/progs/loop5.c
> new file mode 100644
> index 000000000000..9d9817efe208
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/loop5.c
> @@ -0,0 +1,37 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) 2019 Facebook
> +#include <linux/sched.h>
> +#include <linux/ptrace.h>

The above headers probably not needed.

> +#include <stdint.h>
> +#include <stddef.h>
> +#include <stdbool.h>
> +#include <linux/bpf.h>
> +#include "bpf_helpers.h"
> +#define barrier() __asm__ __volatile__("": : :"memory")
> +
> +char _license[] SEC("license") = "GPL";
> +
> +SEC("socket")
> +int while_true(volatile struct __sk_buff* skb)
> +{
> +	int i = 0;
> +
> +	while (true) {
> +		if (skb->len)
> +			i += 3;
> +		else
> +			i += 7;
> +		if (i == 9)
> +			break;
> +		barrier();
> +		if (i == 10)
> +			break;
> +		barrier();
> +		if (i == 13)
> +			break;
> +		barrier();
> +		if (i == 14)
> +			break;
> +	}
> +	return i;
> +}
> 

^ permalink raw reply

* Re: [PATCH 1/1] bpf: introduce new helper udp_flow_src_port
From: Y Song @ 2019-08-04  6:52 UTC (permalink / raw)
  To: Farid Zakaria; +Cc: Alexei Starovoitov, Daniel Borkmann, netdev, bpf
In-Reply-To: <20190803044320.5530-2-farid.m.zakaria@gmail.com>

On Sat, Aug 3, 2019 at 8:29 PM Farid Zakaria <farid.m.zakaria@gmail.com> wrote:
>
> Foo over UDP uses UDP encapsulation to add additional entropy
> into the packets so that they get beter distribution across EMCP
> routes.
>
> Expose udp_flow_src_port as a bpf helper so that tunnel filters
> can benefit from the helper.
>
> Signed-off-by: Farid Zakaria <farid.m.zakaria@gmail.com>
> ---
>  include/uapi/linux/bpf.h                      | 21 +++++++--
>  net/core/filter.c                             | 20 ++++++++
>  tools/include/uapi/linux/bpf.h                | 21 +++++++--
>  tools/testing/selftests/bpf/bpf_helpers.h     |  2 +
>  .../bpf/prog_tests/udp_flow_src_port.c        | 28 +++++++++++
>  .../bpf/progs/test_udp_flow_src_port_kern.c   | 47 +++++++++++++++++++
>  6 files changed, 131 insertions(+), 8 deletions(-)
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/udp_flow_src_port.c
>  create mode 100644 tools/testing/selftests/bpf/progs/test_udp_flow_src_port_kern.c

First, for each review, backport and sync with libbpf repo, in the future,
could you break the patch to two patches?
   1. kernel changes (net/core/filter.c, include/uapi/linux/bpf.h)
   2. tools/include/uapi/linux/bpf.h
   3. tools/testing/ changes

Second, could you explain why existing __sk_buff->hash not enough?
there are corner cases where if __sk_buff->hash is 0 and the kernel did some
additional hashing, but maybe you can approximate in bpf program?
For case, min >= max, I suppose you can get min/max port values
from the user space for a particular net device and then calculate
the hash in the bpf program?
What I want to know if how much accuracy you will lose if you just
use __sk_buff->hash and do approximation in bpf program.

>
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 4393bd4b2419..90e814153dec 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -2545,9 +2545,21 @@ union bpf_attr {
>   *             *th* points to the start of the TCP header, while *th_len*
>   *             contains **sizeof**\ (**struct tcphdr**).
>   *
> - *     Return
> - *             0 if *iph* and *th* are a valid SYN cookie ACK, or a negative
> - *             error otherwise.
> + *  Return
> + *      0 if *iph* and *th* are a valid SYN cookie ACK, or a negative
> + *      error otherwise.
> + *
> + * int bpf_udp_flow_src_port(struct sk_buff *skb, int min, int max, int use_eth)
> + *  Description
> + *      It's common to implement tunnelling inside a UDP protocol to provide
> + *      additional randomness to the packet. The destination port of the UDP
> + *      header indicates the inner packet type whereas the source port is used
> + *      for additional entropy.
> + *
> + *  Return
> + *      An obfuscated hash of the packet that falls within the
> + *      min & max port range.
> + *      If min >= max, the default port range is used
>   *
>   * int bpf_sysctl_get_name(struct bpf_sysctl *ctx, char *buf, size_t buf_len, u64 flags)
>   *     Description
> @@ -2853,7 +2865,8 @@ union bpf_attr {
>         FN(sk_storage_get),             \
>         FN(sk_storage_delete),          \
>         FN(send_signal),                \
> -       FN(tcp_gen_syncookie),
> +       FN(tcp_gen_syncookie),  \
> +       FN(udp_flow_src_port),
>
>  /* integer value in 'imm' field of BPF_CALL instruction selects which helper
>   * function eBPF program intends to call
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 5a2707918629..fdf0ebb8c2c8 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -2341,6 +2341,24 @@ static const struct bpf_func_proto bpf_msg_pull_data_proto = {
>         .arg4_type      = ARG_ANYTHING,
>  };
>
> +BPF_CALL_4(bpf_udp_flow_src_port, struct sk_buff *, skb, int, min,
> +          int, max, int, use_eth)
> +{
> +       struct net *net = dev_net(skb->dev);
> +
> +       return udp_flow_src_port(net, skb, min, max, use_eth);
> +}
> +
[...]

^ permalink raw reply

* Re: [PATCH v2] net/mlx5e: always initialize frag->last_in_page
From: Tariq Toukan @ 2019-08-04  7:45 UTC (permalink / raw)
  To: Qian Cai, davem@davemloft.net
  Cc: Saeed Mahameed, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <1564667574-31542-1-git-send-email-cai@lca.pw>



On 8/1/2019 4:52 PM, Qian Cai wrote:
> The commit 069d11465a80 ("net/mlx5e: RX, Enhance legacy Receive Queue
> memory scheme") introduced an undefined behaviour below due to
> "frag->last_in_page" is only initialized in mlx5e_init_frags_partition()
> when,
> 
> if (next_frag.offset + frag_info[f].frag_stride > PAGE_SIZE)
> 
> or after bailed out the loop,
> 
> for (i = 0; i < mlx5_wq_cyc_get_size(&rq->wqe.wq); i++)
> 
> As the result, there could be some "frag" have uninitialized
> value of "last_in_page".
> 
> Later, get_frag() obtains those "frag" and check "frag->last_in_page" in
> mlx5e_put_rx_frag() and triggers the error during boot. Fix it by always
> initializing "frag->last_in_page" to "false" in
> mlx5e_init_frags_partition().
> 
> UBSAN: Undefined behaviour in
> drivers/net/ethernet/mellanox/mlx5/core/en_rx.c:325:12
> load of value 170 is not a valid value for type 'bool' (aka '_Bool')
> Call trace:
>   dump_backtrace+0x0/0x264
>   show_stack+0x20/0x2c
>   dump_stack+0xb0/0x104
>   __ubsan_handle_load_invalid_value+0x104/0x128
>   mlx5e_handle_rx_cqe+0x8e8/0x12cc [mlx5_core]
>   mlx5e_poll_rx_cq+0xca8/0x1a94 [mlx5_core]
>   mlx5e_napi_poll+0x17c/0xa30 [mlx5_core]
>   net_rx_action+0x248/0x940
>   __do_softirq+0x350/0x7b8
>   irq_exit+0x200/0x26c
>   __handle_domain_irq+0xc8/0x128
>   gic_handle_irq+0x138/0x228
>   el1_irq+0xb8/0x140
>   arch_cpu_idle+0x1a4/0x348
>   do_idle+0x114/0x1b0
>   cpu_startup_entry+0x24/0x28
>   rest_init+0x1ac/0x1dc
>   arch_call_rest_init+0x10/0x18
>   start_kernel+0x4d4/0x57c
> 
> Fixes: 069d11465a80 ("net/mlx5e: RX, Enhance legacy Receive Queue memory scheme")
> Signed-off-by: Qian Cai <cai@lca.pw>
> ---
> 
> v2: zero-init the whole struct instead per Tariq.
> 
>   drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 5 ++---
>   1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> index 47eea6b3a1c3..e1810c03a510 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> @@ -331,12 +331,11 @@ static inline u64 mlx5e_get_mpwqe_offset(struct mlx5e_rq *rq, u16 wqe_ix)
>   
>   static void mlx5e_init_frags_partition(struct mlx5e_rq *rq)
>   {
> -	struct mlx5e_wqe_frag_info next_frag, *prev;
> +	struct mlx5e_wqe_frag_info next_frag = {};
> +	struct mlx5e_wqe_frag_info *prev = NULL;
>   	int i;
>   
>   	next_frag.di = &rq->wqe.di[0];
> -	next_frag.offset = 0;
> -	prev = NULL;
>   
>   	for (i = 0; i < mlx5_wq_cyc_get_size(&rq->wqe.wq); i++) {
>   		struct mlx5e_rq_frag_info *frag_info = &rq->wqe.info.arr[0];
> 

Reviewed-by: Tariq Toukan <tariqt@mellanox.com>

Thanks.

^ permalink raw reply

* [PATCH net-next] r8169: remove access to legacy register MultiIntr
From: Heiner Kallweit @ 2019-08-04  7:42 UTC (permalink / raw)
  To: Realtek linux nic maintainers, David Miller; +Cc: netdev@vger.kernel.org

This code piece was inherited from RTL8139 code, the register at
address 0x5c however has a different meaning on RTL8169 and is unused.
So we can remove this.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/ethernet/realtek/r8169_main.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index 0be8e5c08..e38bc01eb 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -271,7 +271,6 @@ enum rtl_registers {
 	Config3		= 0x54,
 	Config4		= 0x55,
 	Config5		= 0x56,
-	MultiIntr	= 0x5c,
 	PHYAR		= 0x60,
 	PHYstatus	= 0x6c,
 	RxMaxSize	= 0xda,
@@ -5241,10 +5240,7 @@ static void rtl_hw_start(struct  rtl8169_private *tp)
 	RTL_W8(tp, ChipCmd, CmdTxEnb | CmdRxEnb);
 	rtl_init_rxcfg(tp);
 	rtl_set_tx_config_registers(tp);
-
 	rtl_set_rx_mode(tp->dev);
-	/* no early-rx interrupts */
-	RTL_W16(tp, MultiIntr, RTL_R16(tp, MultiIntr) & 0xf000);
 	rtl_irq_enable(tp);
 }
 
-- 
2.22.0


^ permalink raw reply related

* [PATCH net-next] r8169: add helper r8168_mac_ocp_modify
From: Heiner Kallweit @ 2019-08-04  7:47 UTC (permalink / raw)
  To: Realtek linux nic maintainers, David Miller; +Cc: netdev@vger.kernel.org

Add a helper for MAC OCP read-modify-write operations.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/ethernet/realtek/r8169_main.c | 65 +++++++----------------
 1 file changed, 19 insertions(+), 46 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index e38bc01eb..039a967c7 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -850,6 +850,14 @@ static u16 r8168_mac_ocp_read(struct rtl8169_private *tp, u32 reg)
 	return RTL_R32(tp, OCPDR);
 }
 
+static void r8168_mac_ocp_modify(struct rtl8169_private *tp, u32 reg, u16 mask,
+				 u16 set)
+{
+	u16 data = r8168_mac_ocp_read(tp, reg);
+
+	r8168_mac_ocp_write(tp, reg, (data & ~mask) | set);
+}
+
 #define OCP_STD_PHY_BASE	0xa400
 
 static void r8168g_mdio_write(struct rtl8169_private *tp, int reg, int value)
@@ -4809,8 +4817,6 @@ static void rtl_hw_start_8411_2(struct rtl8169_private *tp)
 
 static void rtl_hw_start_8168h_1(struct rtl8169_private *tp)
 {
-	int rg_saw_cnt;
-	u32 data;
 	static const struct ephy_info e_info_8168h_1[] = {
 		{ 0x1e, 0x0800,	0x0001 },
 		{ 0x1d, 0x0000,	0x0800 },
@@ -4819,6 +4825,7 @@ static void rtl_hw_start_8168h_1(struct rtl8169_private *tp)
 		{ 0x04, 0xffff,	0x154a },
 		{ 0x01, 0xffff,	0x068b }
 	};
+	int rg_saw_cnt;
 
 	/* disable aspm and clock request before access ephy */
 	rtl_hw_aspm_clkreq_enable(tp, false);
@@ -4863,31 +4870,13 @@ static void rtl_hw_start_8168h_1(struct rtl8169_private *tp)
 
 		sw_cnt_1ms_ini = 16000000/rg_saw_cnt;
 		sw_cnt_1ms_ini &= 0x0fff;
-		data = r8168_mac_ocp_read(tp, 0xd412);
-		data &= ~0x0fff;
-		data |= sw_cnt_1ms_ini;
-		r8168_mac_ocp_write(tp, 0xd412, data);
+		r8168_mac_ocp_modify(tp, 0xd412, 0x0fff, sw_cnt_1ms_ini);
 	}
 
-	data = r8168_mac_ocp_read(tp, 0xe056);
-	data &= ~0xf0;
-	data |= 0x70;
-	r8168_mac_ocp_write(tp, 0xe056, data);
-
-	data = r8168_mac_ocp_read(tp, 0xe052);
-	data &= ~0x6000;
-	data |= 0x8008;
-	r8168_mac_ocp_write(tp, 0xe052, data);
-
-	data = r8168_mac_ocp_read(tp, 0xe0d6);
-	data &= ~0x01ff;
-	data |= 0x017f;
-	r8168_mac_ocp_write(tp, 0xe0d6, data);
-
-	data = r8168_mac_ocp_read(tp, 0xd420);
-	data &= ~0x0fff;
-	data |= 0x047f;
-	r8168_mac_ocp_write(tp, 0xd420, data);
+	r8168_mac_ocp_modify(tp, 0xe056, 0x00f0, 0x0070);
+	r8168_mac_ocp_modify(tp, 0xe052, 0x6000, 0x8008);
+	r8168_mac_ocp_modify(tp, 0xe0d6, 0x01ff, 0x017f);
+	r8168_mac_ocp_modify(tp, 0xd420, 0x0fff, 0x047f);
 
 	r8168_mac_ocp_write(tp, 0xe63e, 0x0001);
 	r8168_mac_ocp_write(tp, 0xe63e, 0x0000);
@@ -4969,7 +4958,6 @@ static void rtl_hw_start_8168ep_2(struct rtl8169_private *tp)
 
 static void rtl_hw_start_8168ep_3(struct rtl8169_private *tp)
 {
-	u32 data;
 	static const struct ephy_info e_info_8168ep_3[] = {
 		{ 0x00, 0xffff,	0x10a3 },
 		{ 0x19, 0xffff,	0x7c00 },
@@ -4986,18 +4974,9 @@ static void rtl_hw_start_8168ep_3(struct rtl8169_private *tp)
 	RTL_W8(tp, DLLPR, RTL_R8(tp, DLLPR) & ~PFM_EN);
 	RTL_W8(tp, MISC_1, RTL_R8(tp, MISC_1) & ~PFM_D3COLD_EN);
 
-	data = r8168_mac_ocp_read(tp, 0xd3e2);
-	data &= 0xf000;
-	data |= 0x0271;
-	r8168_mac_ocp_write(tp, 0xd3e2, data);
-
-	data = r8168_mac_ocp_read(tp, 0xd3e4);
-	data &= 0xff00;
-	r8168_mac_ocp_write(tp, 0xd3e4, data);
-
-	data = r8168_mac_ocp_read(tp, 0xe860);
-	data |= 0x0080;
-	r8168_mac_ocp_write(tp, 0xe860, data);
+	r8168_mac_ocp_modify(tp, 0xd3e2, 0x0fff, 0x0271);
+	r8168_mac_ocp_modify(tp, 0xd3e4, 0x00ff, 0x0000);
+	r8168_mac_ocp_modify(tp, 0xe860, 0x0000, 0x0080);
 
 	rtl_hw_aspm_clkreq_enable(tp, true);
 }
@@ -6659,8 +6638,6 @@ static int r8169_mdio_register(struct rtl8169_private *tp)
 
 static void rtl_hw_init_8168g(struct rtl8169_private *tp)
 {
-	u32 data;
-
 	tp->ocp_base = OCP_STD_PHY_BASE;
 
 	RTL_W32(tp, MISC, RTL_R32(tp, MISC) | RXDV_GATED_EN);
@@ -6675,16 +6652,12 @@ static void rtl_hw_init_8168g(struct rtl8169_private *tp)
 	msleep(1);
 	RTL_W8(tp, MCU, RTL_R8(tp, MCU) & ~NOW_IS_OOB);
 
-	data = r8168_mac_ocp_read(tp, 0xe8de);
-	data &= ~(1 << 14);
-	r8168_mac_ocp_write(tp, 0xe8de, data);
+	r8168_mac_ocp_modify(tp, 0xe8de, BIT(14), 0);
 
 	if (!rtl_udelay_loop_wait_high(tp, &rtl_link_list_ready_cond, 100, 42))
 		return;
 
-	data = r8168_mac_ocp_read(tp, 0xe8de);
-	data |= (1 << 15);
-	r8168_mac_ocp_write(tp, 0xe8de, data);
+	r8168_mac_ocp_modify(tp, 0xe8de, 0, BIT(15));
 
 	rtl_udelay_loop_wait_high(tp, &rtl_link_list_ready_cond, 100, 42);
 }
-- 
2.22.0


^ permalink raw reply related

* [PATCH net-next] r8169: sync PCIe PHY init with vendor driver 8.047.01
From: Heiner Kallweit @ 2019-08-04  7:52 UTC (permalink / raw)
  To: Realtek linux nic maintainers, David Miller; +Cc: netdev@vger.kernel.org

Synchronize PCIe PHY initialization with vendor driver version 8.047.01.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/ethernet/realtek/r8169_main.c | 60 ++++++++++++++---------
 1 file changed, 38 insertions(+), 22 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index 039a967c7..3c7af6669 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -4415,7 +4415,7 @@ static void rtl_hw_start_8168c_2(struct rtl8169_private *tp)
 {
 	static const struct ephy_info e_info_8168c_2[] = {
 		{ 0x01, 0,	0x0001 },
-		{ 0x03, 0x0400,	0x0220 }
+		{ 0x03, 0x0400,	0x0020 }
 	};
 
 	rtl_set_def_aspm_entry_latency(tp);
@@ -4462,7 +4462,8 @@ static void rtl_hw_start_8168d_4(struct rtl8169_private *tp)
 	static const struct ephy_info e_info_8168d_4[] = {
 		{ 0x0b, 0x0000,	0x0048 },
 		{ 0x19, 0x0020,	0x0050 },
-		{ 0x0c, 0x0100,	0x0020 }
+		{ 0x0c, 0x0100,	0x0020 },
+		{ 0x10, 0x0004,	0x0000 },
 	};
 
 	rtl_set_def_aspm_entry_latency(tp);
@@ -4512,7 +4513,9 @@ static void rtl_hw_start_8168e_2(struct rtl8169_private *tp)
 {
 	static const struct ephy_info e_info_8168e_2[] = {
 		{ 0x09, 0x0000,	0x0080 },
-		{ 0x19, 0x0000,	0x0224 }
+		{ 0x19, 0x0000,	0x0224 },
+		{ 0x00, 0x0000,	0x0004 },
+		{ 0x0c, 0x3df0,	0x0200 },
 	};
 
 	rtl_set_def_aspm_entry_latency(tp);
@@ -4574,7 +4577,9 @@ static void rtl_hw_start_8168f_1(struct rtl8169_private *tp)
 		{ 0x06, 0x00c0,	0x0020 },
 		{ 0x08, 0x0001,	0x0002 },
 		{ 0x09, 0x0000,	0x0080 },
-		{ 0x19, 0x0000,	0x0224 }
+		{ 0x19, 0x0000,	0x0224 },
+		{ 0x00, 0x0000,	0x0004 },
+		{ 0x0c, 0x3df0,	0x0200 },
 	};
 
 	rtl_hw_start_8168f(tp);
@@ -4589,8 +4594,9 @@ static void rtl_hw_start_8411(struct rtl8169_private *tp)
 	static const struct ephy_info e_info_8168f_1[] = {
 		{ 0x06, 0x00c0,	0x0020 },
 		{ 0x0f, 0xffff,	0x5200 },
-		{ 0x1e, 0x0000,	0x4000 },
-		{ 0x19, 0x0000,	0x0224 }
+		{ 0x19, 0x0000,	0x0224 },
+		{ 0x00, 0x0000,	0x0004 },
+		{ 0x0c, 0x3df0,	0x0200 },
 	};
 
 	rtl_hw_start_8168f(tp);
@@ -4629,8 +4635,8 @@ static void rtl_hw_start_8168g(struct rtl8169_private *tp)
 static void rtl_hw_start_8168g_1(struct rtl8169_private *tp)
 {
 	static const struct ephy_info e_info_8168g_1[] = {
-		{ 0x00, 0x0000,	0x0008 },
-		{ 0x0c, 0x37d0,	0x0820 },
+		{ 0x00, 0x0008,	0x0000 },
+		{ 0x0c, 0x3ff0,	0x0820 },
 		{ 0x1e, 0x0000,	0x0001 },
 		{ 0x19, 0x8000,	0x0000 }
 	};
@@ -4646,10 +4652,15 @@ static void rtl_hw_start_8168g_1(struct rtl8169_private *tp)
 static void rtl_hw_start_8168g_2(struct rtl8169_private *tp)
 {
 	static const struct ephy_info e_info_8168g_2[] = {
-		{ 0x00, 0x0000,	0x0008 },
-		{ 0x0c, 0x3df0,	0x0200 },
-		{ 0x19, 0xffff,	0xfc00 },
-		{ 0x1e, 0xffff,	0x20eb }
+		{ 0x00, 0x0008,	0x0000 },
+		{ 0x0c, 0x3ff0,	0x0820 },
+		{ 0x19, 0xffff,	0x7c00 },
+		{ 0x1e, 0xffff,	0x20eb },
+		{ 0x0d, 0xffff,	0x1666 },
+		{ 0x00, 0xffff,	0x10a3 },
+		{ 0x06, 0xffff,	0xf050 },
+		{ 0x04, 0x0000,	0x0010 },
+		{ 0x1d, 0x4000,	0x0000 },
 	};
 
 	rtl_hw_start_8168g(tp);
@@ -4663,11 +4674,16 @@ static void rtl_hw_start_8168g_2(struct rtl8169_private *tp)
 static void rtl_hw_start_8411_2(struct rtl8169_private *tp)
 {
 	static const struct ephy_info e_info_8411_2[] = {
-		{ 0x00, 0x0000,	0x0008 },
-		{ 0x0c, 0x3df0,	0x0200 },
-		{ 0x0f, 0xffff,	0x5200 },
-		{ 0x19, 0x0020,	0x0000 },
-		{ 0x1e, 0x0000,	0x2000 }
+		{ 0x00, 0x0008,	0x0000 },
+		{ 0x0c, 0x37d0,	0x0820 },
+		{ 0x1e, 0x0000,	0x0001 },
+		{ 0x19, 0x8021,	0x0000 },
+		{ 0x1e, 0x0000,	0x2000 },
+		{ 0x0d, 0x0100,	0x0200 },
+		{ 0x00, 0x0000,	0x0080 },
+		{ 0x06, 0x0000,	0x0010 },
+		{ 0x04, 0x0000,	0x0010 },
+		{ 0x1d, 0x0000,	0x4000 },
 	};
 
 	rtl_hw_start_8168g(tp);
@@ -4822,7 +4838,7 @@ static void rtl_hw_start_8168h_1(struct rtl8169_private *tp)
 		{ 0x1d, 0x0000,	0x0800 },
 		{ 0x05, 0xffff,	0x2089 },
 		{ 0x06, 0xffff,	0x5881 },
-		{ 0x04, 0xffff,	0x154a },
+		{ 0x04, 0xffff,	0x854a },
 		{ 0x01, 0xffff,	0x068b }
 	};
 	int rg_saw_cnt;
@@ -4959,10 +4975,10 @@ static void rtl_hw_start_8168ep_2(struct rtl8169_private *tp)
 static void rtl_hw_start_8168ep_3(struct rtl8169_private *tp)
 {
 	static const struct ephy_info e_info_8168ep_3[] = {
-		{ 0x00, 0xffff,	0x10a3 },
-		{ 0x19, 0xffff,	0x7c00 },
-		{ 0x1e, 0xffff,	0x20eb },
-		{ 0x0d, 0xffff,	0x1666 }
+		{ 0x00, 0x0000,	0x0080 },
+		{ 0x0d, 0x0100,	0x0200 },
+		{ 0x19, 0x8021,	0x0000 },
+		{ 0x1e, 0x0000,	0x2000 },
 	};
 
 	/* disable aspm and clock request before access ephy */
-- 
2.22.0


^ permalink raw reply related

* Re: [PATCH V2 7/9] vhost: do not use RCU to synchronize MMU notifier with worker
From: Michael S. Tsirkin @ 2019-08-04  8:07 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Jason Wang, kvm, virtualization, netdev, linux-kernel, linux-mm
In-Reply-To: <20190804001400.GA25543@ziepe.ca>

On Sat, Aug 03, 2019 at 09:14:00PM -0300, Jason Gunthorpe wrote:
> On Sat, Aug 03, 2019 at 05:36:13PM -0400, Michael S. Tsirkin wrote:
> > On Fri, Aug 02, 2019 at 02:24:18PM -0300, Jason Gunthorpe wrote:
> > > On Fri, Aug 02, 2019 at 10:27:21AM -0400, Michael S. Tsirkin wrote:
> > > > On Fri, Aug 02, 2019 at 09:46:13AM -0300, Jason Gunthorpe wrote:
> > > > > On Fri, Aug 02, 2019 at 05:40:07PM +0800, Jason Wang wrote:
> > > > > > > This must be a proper barrier, like a spinlock, mutex, or
> > > > > > > synchronize_rcu.
> > > > > > 
> > > > > > 
> > > > > > I start with synchronize_rcu() but both you and Michael raise some
> > > > > > concern.
> > > > > 
> > > > > I've also idly wondered if calling synchronize_rcu() under the various
> > > > > mm locks is a deadlock situation.
> > > > > 
> > > > > > Then I try spinlock and mutex:
> > > > > > 
> > > > > > 1) spinlock: add lots of overhead on datapath, this leads 0 performance
> > > > > > improvement.
> > > > > 
> > > > > I think the topic here is correctness not performance improvement
> > > > 
> > > > The topic is whether we should revert
> > > > commit 7f466032dc9 ("vhost: access vq metadata through kernel virtual address")
> > > > 
> > > > or keep it in. The only reason to keep it is performance.
> > > 
> > > Yikes, I'm not sure you can ever win against copy_from_user using
> > > mmu_notifiers?
> > 
> > Ever since copy_from_user started playing with flags (for SMAP) and
> > added speculation barriers there's a chance we can win by accessing
> > memory through the kernel address.
> 
> You think copy_to_user will be more expensive than the minimum two
> atomics required to synchronize with another thread?

I frankly don't know. With SMAP you flip flags twice, and with spectre
you flush the pipeline. Is that cheaper or more expensive than an atomic
operation? Testing is the only way to tell.

> > > Also, why can't this just permanently GUP the pages? In fact, where
> > > does it put_page them anyhow? Worrying that 7f466 adds a get_user page
> > > but does not add a put_page??
> 
> You didn't answer this.. Why not just use GUP?
> 
> Jason

Sorry I misunderstood the question. Permanent GUP breaks lots of
functionality we need such as THP and numa balancing.

release_pages is used instead of put_page.




-- 
MST

^ permalink raw reply

* [PATCH iproute2-next] rdma: Add driver QP type string
From: Gal Pressman @ 2019-08-04  8:07 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev, linux-rdma, Leon Romanovsky, Gal Pressman

RDMA resource tracker now tracks driver QPs as well, add driver QP type
string to qp_types_to_str function.

Signed-off-by: Gal Pressman <galpress@amazon.com>
---
 rdma/res.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/rdma/res.c b/rdma/res.c
index ef863f142eca..97a7b9640185 100644
--- a/rdma/res.c
+++ b/rdma/res.c
@@ -148,9 +148,11 @@ const char *qp_types_to_str(uint8_t idx)
 						     "UC", "UD", "RAW_IPV6",
 						     "RAW_ETHERTYPE",
 						     "UNKNOWN", "RAW_PACKET",
-						     "XRC_INI", "XRC_TGT" };
+						     "XRC_INI", "XRC_TGT",
+						     [0xFF] = "DRIVER",
+	};
 
-	if (idx < ARRAY_SIZE(qp_types_str))
+	if (idx < ARRAY_SIZE(qp_types_str) && qp_types_str[idx])
 		return qp_types_str[idx];
 	return "UNKNOWN";
 }
-- 
2.22.0


^ permalink raw reply related

* Re: [PATCH v4 3/4] net: phy: realtek: Add helpers for accessing RTL8211E extension pages
From: Heiner Kallweit @ 2019-08-04  8:33 UTC (permalink / raw)
  To: Matthias Kaehlcke, David S . Miller, Rob Herring, Mark Rutland,
	Andrew Lunn, Florian Fainelli
  Cc: netdev, devicetree, linux-kernel, Douglas Anderson
In-Reply-To: <20190801190759.28201-4-mka@chromium.org>

On 01.08.2019 21:07, Matthias Kaehlcke wrote:
> The RTL8211E has extension pages, which can be accessed after
> selecting a page through a custom method. Add a function to
> modify bits in a register of an extension page and a helper for
> selecting an ext page. Use rtl8211e_modify_ext_paged() in
> rtl8211e_config_init() instead of doing things 'manually'.
> 
> Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
> ---
> Changes in v4:
> - don't add constant RTL8211E_EXT_PAGE, it's only used once,
>   use a literal instead
> - pass 'oldpage' to phy_restore_page() in rtl8211e_select_ext_page(),
>   not 'page'
> - return 'oldpage' in rtl8211e_select_ext_page()
> - use __phy_modify() in rtl8211e_modify_ext_paged() instead of
>   reimplementing __phy_modify_changed()
> - in rtl8211e_modify_ext_paged() return directly when
>   rtl8211e_select_ext_page() fails
> ---
>  drivers/net/phy/realtek.c | 48 +++++++++++++++++++++++++++------------
>  1 file changed, 34 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
> index a669945eb829..e09d3b0da2c7 100644
> --- a/drivers/net/phy/realtek.c
> +++ b/drivers/net/phy/realtek.c
> @@ -53,6 +53,36 @@ static int rtl821x_write_page(struct phy_device *phydev, int page)
>  	return __phy_write(phydev, RTL821x_PAGE_SELECT, page);
>  }
>  
> +static int rtl8211e_select_ext_page(struct phy_device *phydev, int page)

The "extended page" mechanism doesn't exist on RTL8211E only. A prefix
rtl821x like in other functions may be better therefore.

> +{
> +	int ret, oldpage;
> +
> +	oldpage = phy_select_page(phydev, 7);
> +	if (oldpage < 0)
> +		return oldpage;
> +
> +	ret = __phy_write(phydev, RTL821x_EXT_PAGE_SELECT, page);
> +	if (ret)
> +		return phy_restore_page(phydev, oldpage, ret);
> +
> +	return oldpage;
> +}
> +
> +static int rtl8211e_modify_ext_paged(struct phy_device *phydev, int page,
> +				     u32 regnum, u16 mask, u16 set)
> +{
> +	int ret = 0;
> +	int oldpage;
> +
> +	oldpage = rtl8211e_select_ext_page(phydev, page);
> +	if (oldpage < 0)
> +		return oldpage;
> +
> +	ret = __phy_modify(phydev, regnum, mask, set);
> +
> +	return phy_restore_page(phydev, oldpage, ret);
> +}
> +
>  static int rtl8201_ack_interrupt(struct phy_device *phydev)
>  {
>  	int err;
> @@ -184,7 +214,7 @@ static int rtl8211f_config_init(struct phy_device *phydev)
>  
>  static int rtl8211e_config_init(struct phy_device *phydev)
>  {
> -	int ret = 0, oldpage;
> +	int ret;
>  	u16 val;
>  
>  	/* enable TX/RX delay for rgmii-* modes, and disable them for rgmii. */
> @@ -213,19 +243,9 @@ static int rtl8211e_config_init(struct phy_device *phydev)
>  	 * 2 = RX Delay, 1 = TX Delay, 0 = SELRGV (see original PHY datasheet
>  	 * for details).
>  	 */
> -	oldpage = phy_select_page(phydev, 0x7);
> -	if (oldpage < 0)
> -		goto err_restore_page;
> -
> -	ret = __phy_write(phydev, RTL821x_EXT_PAGE_SELECT, 0xa4);
> -	if (ret)
> -		goto err_restore_page;
> -
> -	ret = __phy_modify(phydev, 0x1c, RTL8211E_TX_DELAY | RTL8211E_RX_DELAY,
> -			   val);
> -
> -err_restore_page:
> -	return phy_restore_page(phydev, oldpage, ret);
> +	return rtl8211e_modify_ext_paged(phydev, 0xa4, 0x1c,
> +					 RTL8211E_TX_DELAY | RTL8211E_RX_DELAY,
> +					 val);
>  }
>  
>  static int rtl8211b_suspend(struct phy_device *phydev)
> 


^ permalink raw reply

* Re: [PATCH net-next 1/2] net: phy: broadcom: set features explicitly for BCM54616S
From: Heiner Kallweit @ 2019-08-04  8:40 UTC (permalink / raw)
  To: Tao Ren, Andrew Lunn
  Cc: Florian Fainelli, David S . Miller, Arun Parameswaran,
	Justin Chen, Vladimir Oltean, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, Andrew Jeffery,
	openbmc@lists.ozlabs.org
In-Reply-To: <88f4d709-d9bb-943c-37a9-aeebe8ca0ebc@fb.com>

On 01.08.2019 07:20, Tao Ren wrote:
> On 7/30/19 11:00 PM, Tao Ren wrote:
>> On 7/30/19 10:53 PM, Heiner Kallweit wrote:
>>> On 31.07.2019 02:12, Tao Ren wrote:
>>>> On 7/29/19 11:00 PM, Heiner Kallweit wrote:
>>>>> On 30.07.2019 07:05, Tao Ren wrote:
>>>>>> On 7/29/19 8:35 PM, Andrew Lunn wrote:
>>>>>>> On Mon, Jul 29, 2019 at 05:25:32PM -0700, Tao Ren wrote:
>>>>>>>> BCM54616S feature "PHY_GBIT_FEATURES" was removed by commit dcdecdcfe1fc
>>>>>>>> ("net: phy: switch drivers to use dynamic feature detection"). As dynamic
>>>>>>>> feature detection doesn't work when BCM54616S is working in RGMII-Fiber
>>>>>>>> mode (different sets of MII Control/Status registers being used), let's
>>>>>>>> set "PHY_GBIT_FEATURES" for BCM54616S explicitly.
>>>>>>>
>>>>>>> Hi Tao
>>>>>>>
>>>>>>> What exactly does it get wrong?
>>>>>>>
>>>>>>>      Thanks
>>>>>>> 	Andrew
>>>>>>
>>>>>> Hi Andrew,
>>>>>>
>>>>>> BCM54616S is set to RGMII-Fiber (1000Base-X) mode on my platform, and none of the features (1000BaseT/100BaseT/10BaseT) can be detected by genphy_read_abilities(), because the PHY only reports 1000BaseX_Full|Half ability in this mode.
>>>>>>
>>>>> Are you going to use the PHY in copper or fibre mode?
>>>>> In case you use fibre mode, why do you need the copper modes set as supported?
>>>>> Or does the PHY just start in fibre mode and you want to switch it to copper mode?
>>>>
>>>> Hi Heiner,
>>>>
>>>> The phy starts in fiber mode and that's the mode I want.
>>>> My observation is: phydev->link is always 0 (Link status bit is never set in MII_BMSR) by using dynamic ability detection on my machine. I checked phydev->supported and it's set to "AutoNeg | TP | MII | Pause | Asym_Pause" by dynamic ability detection. Is it normal/expected? Or maybe the fix should go to different places? Thank you for your help.
>>>>
>>>
>>> Not sure whether you stated already which kernel version you're using.
>>> There's a brand-new extension to auto-detect 1000BaseX:
>>> f30e33bcdab9 ("net: phy: Add more 1000BaseX support detection")
>>> It's included in the 5.3-rc series.
>>
>> I'm running kernel 5.2.0. Thank you for the sharing and I didn't know the patch. Let me check it out.
> 
> I applied above patch and ca72efb6bdc7 ("net: phy: Add detection of 1000BaseX link mode support") to my 5.2.0 tree but got following warning when booting up my machine:
> 
> "PHY advertising (0,00000200,000062c0) more modes than genphy supports, some modes not advertised".
> 
It's genphy_config_advert complaining which is called from genphy_config_aneg.
genphy_config_aneg deals with the standard Base-T modes. Therefore in your case
most likely you want to provide an own config_aneg callback (in case autoneg
is applicable at all).

> The BCM54616S PHY on my machine only reports 1000-X features in RGMII->1000Base-KX mode. Is it a known problem?
> 
> Anyways let me see if I missed some dependency/follow-up patches..
> 
> 
> Cheers,
> 
> Tao
> 

Heiner

^ permalink raw reply

* Re: BPF: ETLS: RECV FLOW
From: Shridhar Venkatraman @ 2019-08-04  9:39 UTC (permalink / raw)
  To: netdev
In-Reply-To: <CADJe1ZsN8+1brBNdN2VNMp4PRdeYjCC=qaMZALQxOTvPmgJQhA@mail.gmail.com>

Hi,

The eTLS work has BPF integration which is great.
However there is one spot where access to the clear text is not available.

From kernel 4.20 - receiver BPF support added for KTLS.

a. receiver BPF is applied on encrypted message
b. after applying BPF, message is decrypted
c. BPF run logic on the decrypted plain message   - can we add this support ?
d. then copy the decrypted message back to userspace.

code flow reference: tls receive message call flow:
--------------------------------------------------------------

tls_sw_recvmsg
  __tcp_bpf_recvmsg [ bpf exec function called on encrypted message ]
  decrypt_skb_update
  decrypt_internal
  BPF_PROG_RUN on decrypted plain message - can we add this support ?
  skb_copy_datagram_msg [ decrypted message copied back to userspace ]

Thanks
ps: I sent this to the bpf list as I don't know which one it should go to

^ permalink raw reply

* Re: [PATCH iproute2-next] rdma: Add driver QP type string
From: Leon Romanovsky @ 2019-08-04  9:52 UTC (permalink / raw)
  To: Gal Pressman; +Cc: Stephen Hemminger, netdev, linux-rdma
In-Reply-To: <20190804080756.58364-1-galpress@amazon.com>

On Sun, Aug 04, 2019 at 11:07:56AM +0300, Gal Pressman wrote:
> RDMA resource tracker now tracks driver QPs as well, add driver QP type
> string to qp_types_to_str function.
>
> Signed-off-by: Gal Pressman <galpress@amazon.com>
> ---
>  rdma/res.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>

Thanks,
Reviewed-by: Leon Romanovsky <leonro@mellanox.com>

^ permalink raw reply

* [PATCH rdma-next v1 0/3] ODP support for mlx5 DC QPs
From: Leon Romanovsky @ 2019-08-04 10:00 UTC (permalink / raw)
  To: Doug Ledford, Jason Gunthorpe
  Cc: Leon Romanovsky, RDMA mailing list, Michael Guralnik, Moni Shoua,
	Saeed Mahameed, linux-netdev

From: Leon Romanovsky <leonro@mellanox.com>

Changelog
 v1:
 * Fixed alignment to u64 in mlx5-abi.h (Gal P.)
 v0:
 * https://lore.kernel.org/linux-rdma/20190801122139.25224-1-leon@kernel.org

---------------------------------------------------------------------------------
From Michael,

The series adds support for on-demand paging for DC transport.
Adding handling of DC WQE parsing upon page faults and exposing
capabilities.

As DC is mlx-only transport, the capabilities are exposed to the user
using the direct-verbs mechanism. Namely through the
mlx5dv_query_device.

Thanks

Thanks

Michael Guralnik (3):
  IB/mlx5: Query ODP capabilities for DC
  IB/mlx5: Expose ODP for DC capabilities to user
  IB/mlx5: Add page fault handler for DC initiator WQE

 drivers/infiniband/hw/mlx5/main.c             |  6 +++++
 drivers/infiniband/hw/mlx5/mlx5_ib.h          |  1 +
 drivers/infiniband/hw/mlx5/odp.c              | 27 ++++++++++++++++++-
 .../net/ethernet/mellanox/mlx5/core/main.c    |  6 +++++
 include/linux/mlx5/mlx5_ifc.h                 |  4 ++-
 include/uapi/rdma/mlx5-abi.h                  |  3 +++
 6 files changed, 45 insertions(+), 2 deletions(-)

--
2.20.1


^ permalink raw reply

* [PATCH rdma-next v1 2/3] IB/mlx5: Expose ODP for DC capabilities to user
From: Leon Romanovsky @ 2019-08-04 10:00 UTC (permalink / raw)
  To: Doug Ledford, Jason Gunthorpe
  Cc: Leon Romanovsky, RDMA mailing list, Michael Guralnik, Moni Shoua,
	Saeed Mahameed, linux-netdev
In-Reply-To: <20190804100048.32671-1-leon@kernel.org>

From: Michael Guralnik <michaelgur@mellanox.com>

Return ODP capabilities for DC to user in alloc_context.

Signed-off-by: Michael Guralnik <michaelgur@mellanox.com>
Reviewed-by: Moni Shoua <monis@mellanox.com>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 drivers/infiniband/hw/mlx5/main.c | 6 ++++++
 include/uapi/rdma/mlx5-abi.h      | 3 +++
 2 files changed, 9 insertions(+)

diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c
index 4a3d700cd783..a53e0dc7c17f 100644
--- a/drivers/infiniband/hw/mlx5/main.c
+++ b/drivers/infiniband/hw/mlx5/main.c
@@ -1954,6 +1954,12 @@ static int mlx5_ib_alloc_ucontext(struct ib_ucontext *uctx,
 		resp.response_length += sizeof(resp.dump_fill_mkey);
 	}
 
+	if (field_avail(typeof(resp), dc_odp_caps, udata->outlen)) {
+		resp.dc_odp_caps = dev->dc_odp_caps;
+		resp.comp_mask |= MLX5_IB_ALLOC_UCONTEXT_RESP_MASK_DC_ODP_CAPS;
+		resp.response_length += sizeof(resp.dc_odp_caps);
+	}
+
 	err = ib_copy_to_udata(udata, &resp, resp.response_length);
 	if (err)
 		goto out_mdev;
diff --git a/include/uapi/rdma/mlx5-abi.h b/include/uapi/rdma/mlx5-abi.h
index 624f5b53eb1f..7cab806d7fa7 100644
--- a/include/uapi/rdma/mlx5-abi.h
+++ b/include/uapi/rdma/mlx5-abi.h
@@ -98,6 +98,7 @@ struct mlx5_ib_alloc_ucontext_req_v2 {
 enum mlx5_ib_alloc_ucontext_resp_mask {
 	MLX5_IB_ALLOC_UCONTEXT_RESP_MASK_CORE_CLOCK_OFFSET = 1UL << 0,
 	MLX5_IB_ALLOC_UCONTEXT_RESP_MASK_DUMP_FILL_MKEY    = 1UL << 1,
+	MLX5_IB_ALLOC_UCONTEXT_RESP_MASK_DC_ODP_CAPS	   = 1UL << 2,
 };
 
 enum mlx5_user_cmds_supp_uhw {
@@ -147,6 +148,8 @@ struct mlx5_ib_alloc_ucontext_resp {
 	__u32	num_uars_per_page;
 	__u32	num_dyn_bfregs;
 	__u32	dump_fill_mkey;
+	__u32	dc_odp_caps;
+	__u32   reserved;
 };
 
 struct mlx5_ib_alloc_pd_resp {
-- 
2.20.1


^ permalink raw reply related

* [PATCH rdma-next v1 3/3] IB/mlx5: Add page fault handler for DC initiator WQE
From: Leon Romanovsky @ 2019-08-04 10:00 UTC (permalink / raw)
  To: Doug Ledford, Jason Gunthorpe
  Cc: Leon Romanovsky, RDMA mailing list, Michael Guralnik, Moni Shoua,
	Saeed Mahameed, linux-netdev
In-Reply-To: <20190804100048.32671-1-leon@kernel.org>

From: Michael Guralnik <michaelgur@mellanox.com>

Parsing DC initiator WQEs upon page fault requires skipping an address
vector segment, as in UD WQEs.

Signed-off-by: Michael Guralnik <michaelgur@mellanox.com>
Reviewed-by: Moni Shoua <monis@mellanox.com>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 drivers/infiniband/hw/mlx5/odp.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/hw/mlx5/odp.c b/drivers/infiniband/hw/mlx5/odp.c
index 5e87a5e25574..6f1de5edbe8e 100644
--- a/drivers/infiniband/hw/mlx5/odp.c
+++ b/drivers/infiniband/hw/mlx5/odp.c
@@ -1065,6 +1065,12 @@ static int mlx5_ib_mr_initiator_pfault_handler(
 	case IB_QPT_UD:
 		transport_caps = dev->odp_caps.per_transport_caps.ud_odp_caps;
 		break;
+	case IB_QPT_DRIVER:
+		if (qp->qp_sub_type == MLX5_IB_QPT_DCI) {
+			transport_caps = dev->dc_odp_caps;
+			break;
+		}
+		/* fall through */
 	default:
 		mlx5_ib_err(dev, "ODP fault on QP of an unsupported transport 0x%x\n",
 			    qp->ibqp.qp_type);
@@ -1078,7 +1084,8 @@ static int mlx5_ib_mr_initiator_pfault_handler(
 		return -EFAULT;
 	}
 
-	if (qp->ibqp.qp_type == IB_QPT_UD) {
+	if (qp->ibqp.qp_type == IB_QPT_UD ||
+	    qp->qp_sub_type == MLX5_IB_QPT_DCI) {
 		av = *wqe;
 		if (av->dqp_dct & cpu_to_be32(MLX5_EXTENDED_UD_AV))
 			*wqe += sizeof(struct mlx5_av);
-- 
2.20.1


^ permalink raw reply related

* [PATCH mlx5-next v1 1/3] IB/mlx5: Query ODP capabilities for DC
From: Leon Romanovsky @ 2019-08-04 10:00 UTC (permalink / raw)
  To: Doug Ledford, Jason Gunthorpe
  Cc: Leon Romanovsky, RDMA mailing list, Michael Guralnik, Moni Shoua,
	Saeed Mahameed, linux-netdev
In-Reply-To: <20190804100048.32671-1-leon@kernel.org>

From: Michael Guralnik <michaelgur@mellanox.com>

Set current capabilities of ODP for DC to max capabilities and cache
them in mlx5_ib.

Signed-off-by: Michael Guralnik <michaelgur@mellanox.com>
Reviewed-by: Moni Shoua <monis@mellanox.com>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 drivers/infiniband/hw/mlx5/mlx5_ib.h           |  1 +
 drivers/infiniband/hw/mlx5/odp.c               | 18 ++++++++++++++++++
 drivers/net/ethernet/mellanox/mlx5/core/main.c |  6 ++++++
 include/linux/mlx5/mlx5_ifc.h                  |  4 +++-
 4 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/hw/mlx5/mlx5_ib.h b/drivers/infiniband/hw/mlx5/mlx5_ib.h
index cb41a7e6255a..f99c71b3c876 100644
--- a/drivers/infiniband/hw/mlx5/mlx5_ib.h
+++ b/drivers/infiniband/hw/mlx5/mlx5_ib.h
@@ -967,6 +967,7 @@ struct mlx5_ib_dev {
 	struct mutex			slow_path_mutex;
 	int				fill_delay;
 	struct ib_odp_caps	odp_caps;
+	uint32_t		dc_odp_caps;
 	u64			odp_max_size;
 	struct mlx5_ib_pf_eq	odp_pf_eq;
 
diff --git a/drivers/infiniband/hw/mlx5/odp.c b/drivers/infiniband/hw/mlx5/odp.c
index b0c5de39d186..5e87a5e25574 100644
--- a/drivers/infiniband/hw/mlx5/odp.c
+++ b/drivers/infiniband/hw/mlx5/odp.c
@@ -353,6 +353,24 @@ void mlx5_ib_internal_fill_odp_caps(struct mlx5_ib_dev *dev)
 	if (MLX5_CAP_ODP(dev->mdev, xrc_odp_caps.srq_receive))
 		caps->per_transport_caps.xrc_odp_caps |= IB_ODP_SUPPORT_SRQ_RECV;
 
+	if (MLX5_CAP_ODP(dev->mdev, dc_odp_caps.send))
+		dev->dc_odp_caps |= IB_ODP_SUPPORT_SEND;
+
+	if (MLX5_CAP_ODP(dev->mdev, dc_odp_caps.receive))
+		dev->dc_odp_caps |= IB_ODP_SUPPORT_RECV;
+
+	if (MLX5_CAP_ODP(dev->mdev, dc_odp_caps.write))
+		dev->dc_odp_caps |= IB_ODP_SUPPORT_WRITE;
+
+	if (MLX5_CAP_ODP(dev->mdev, dc_odp_caps.read))
+		dev->dc_odp_caps |= IB_ODP_SUPPORT_READ;
+
+	if (MLX5_CAP_ODP(dev->mdev, dc_odp_caps.atomic))
+		dev->dc_odp_caps |= IB_ODP_SUPPORT_ATOMIC;
+
+	if (MLX5_CAP_ODP(dev->mdev, dc_odp_caps.srq_receive))
+		dev->dc_odp_caps |= IB_ODP_SUPPORT_SRQ_RECV;
+
 	if (MLX5_CAP_GEN(dev->mdev, fixed_buffer_size) &&
 	    MLX5_CAP_GEN(dev->mdev, null_mkey) &&
 	    MLX5_CAP_GEN(dev->mdev, umr_extended_translation_offset))
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c b/drivers/net/ethernet/mellanox/mlx5/core/main.c
index b15b27a497fc..3995fc6d4d34 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c
@@ -495,6 +495,12 @@ static int handle_hca_cap_odp(struct mlx5_core_dev *dev)
 	ODP_CAP_SET_MAX(dev, xrc_odp_caps.write);
 	ODP_CAP_SET_MAX(dev, xrc_odp_caps.read);
 	ODP_CAP_SET_MAX(dev, xrc_odp_caps.atomic);
+	ODP_CAP_SET_MAX(dev, dc_odp_caps.srq_receive);
+	ODP_CAP_SET_MAX(dev, dc_odp_caps.send);
+	ODP_CAP_SET_MAX(dev, dc_odp_caps.receive);
+	ODP_CAP_SET_MAX(dev, dc_odp_caps.write);
+	ODP_CAP_SET_MAX(dev, dc_odp_caps.read);
+	ODP_CAP_SET_MAX(dev, dc_odp_caps.atomic);
 
 	if (do_set)
 		err = set_caps(dev, set_ctx, set_sz,
diff --git a/include/linux/mlx5/mlx5_ifc.h b/include/linux/mlx5/mlx5_ifc.h
index ec571fd7fcf8..5eae8d734435 100644
--- a/include/linux/mlx5/mlx5_ifc.h
+++ b/include/linux/mlx5/mlx5_ifc.h
@@ -944,7 +944,9 @@ struct mlx5_ifc_odp_cap_bits {
 
 	struct mlx5_ifc_odp_per_transport_service_cap_bits xrc_odp_caps;
 
-	u8         reserved_at_100[0x700];
+	struct mlx5_ifc_odp_per_transport_service_cap_bits dc_odp_caps;
+
+	u8         reserved_at_100[0x6E0];
 };
 
 struct mlx5_ifc_calc_op {
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH] net/mlx5e: Allow removing representors netdev to other namespace
From: Or Gerlitz @ 2019-08-04 10:21 UTC (permalink / raw)
  To: Tonghao Zhang; +Cc: Roi Dayan, Saeed Mahameed, Linux Netdev List
In-Reply-To: <CAMDZJNWZ=s-yf7vho0zHySD01uOZzbUdcFmgu+Rk=p-nRoHN=A@mail.gmail.com>

On Thu, Aug 1, 2019 at 3:44 AM Tonghao Zhang <xiangxia.m.yue@gmail.com> wrote:
> On Wed, May 22, 2019 at 12:49 PM Or Gerlitz <gerlitz.or@gmail.com> wrote:
> > On Wed, May 22, 2019 at 4:26 AM Tonghao Zhang <xiangxia.m.yue@gmail.com> wrote:

> > > I review the reps of netronome nfp codes,  nfp does't set the
> > > NETIF_F_NETNS_LOCAL to netdev->features.
> > > And I changed the OFED codes which used for our product environment,
> > > and then send this patch to upstream.

> > The real question here is if we can provide the required separation when
> > vport rep netdevs are put into different name-spaces -- this needs deeper
> > thinking. Technically you can do that with this one liner patch but we have
> > to see if/what assumptions could be broken as of that.

> Can we add a mode parm for allowing user to switch it off/on ?

The kernel model for namespace means a completely new copy of the
networking stack
with new routing tables, new neighbour tables. everything. It also
means netdevices in
different namespaces can't communicate with each other. I tend to
think that our FW/HW
model doesn't support that and hence we can't do proper offloading of
the SW model.

I suggest you approach the current maintainers (Roi and Saeed) to see
if they have different opinion.

Or.

^ permalink raw reply

* [net-next 0/8][pull request] 100GbE Intel Wired LAN Driver Updates 2019-08-04
From: Jeff Kirsher @ 2019-08-04 11:59 UTC (permalink / raw)
  To: davem; +Cc: Jeff Kirsher, netdev, nhorman, sassmann

This series contains more updates to fm10k from Jake Keller.

Jake removes the unnecessary initialization of some variables to help
resolve static code checker warnings.  Explicitly return success during
resume, since the value of 'err' is always success.  Fixed a issue with
incrementing a void pointer, which can produce undefined behavior.  Used
the __always_unused macro for function templates that are passed as
parameters in functions, but are not used.  Simplified the code by
removing an unnecessary macro in determining the value of NON_Q_VECTORS.
Fixed an issue, using bitwise operations to prevent the low address
overwriting the high portion of the address.

The following are changes since commit 9e8fb25254f76cb483303d8e9a97ed80a65418fe:
  Merge branch 'net-l3-l4-functional-tests'
and are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/next-queue 100GbE

Jacob Keller (8):
  fm10k: remove unnecessary variable initializer
  fm10k: remove needless assignment of err local variable
  fm10k: remove needless initialization of size local variable
  fm10k: explicitly return 0 on success path in function
  fm10k: cast page_addr to u8 * when incrementing it
  fm10k: mark unused parameters with __always_unused
  fm10k: convert NON_Q_VECTORS(hw) into NON_Q_VECTORS
  fm10k: fix fm10k_get_fault_pf to read correct address

 drivers/net/ethernet/intel/fm10k/fm10k.h      | 10 +++-----
 .../net/ethernet/intel/fm10k/fm10k_ethtool.c  |  6 ++---
 drivers/net/ethernet/intel/fm10k/fm10k_main.c |  6 ++---
 drivers/net/ethernet/intel/fm10k/fm10k_mbx.c  |  5 ++--
 .../net/ethernet/intel/fm10k/fm10k_netdev.c   | 12 ++++-----
 drivers/net/ethernet/intel/fm10k/fm10k_pci.c  | 11 ++++----
 drivers/net/ethernet/intel/fm10k/fm10k_pf.c   | 12 ++++-----
 drivers/net/ethernet/intel/fm10k/fm10k_tlv.c  |  9 ++++---
 drivers/net/ethernet/intel/fm10k/fm10k_type.h |  2 +-
 drivers/net/ethernet/intel/fm10k/fm10k_vf.c   | 25 +++++++++++--------
 10 files changed, 49 insertions(+), 49 deletions(-)

-- 
2.21.0


^ permalink raw reply

* [net-next 2/8] fm10k: remove needless assignment of err local variable
From: Jeff Kirsher @ 2019-08-04 11:59 UTC (permalink / raw)
  To: davem; +Cc: Jacob Keller, netdev, nhorman, sassmann, Andrew Bowers,
	Jeff Kirsher
In-Reply-To: <20190804115926.31944-1-jeffrey.t.kirsher@intel.com>

From: Jacob Keller <jacob.e.keller@intel.com>

The local variable err in several functions in the fm10k_netdev.c file
is initialized with a value that is never used. The err value is
immediately re-assigned in all cases where it will be checked. Remove
the unnecessary initializers.

This was detected by cppcheck and resolves the following warnings
produced by that tool:

[fm10k_netdev.c:999] -> [fm10k_netdev.c:1004]: (style) Variable 'err' is
reassigned a value before the old one has been used.

[fm10k_netdev.c:1019] -> [fm10k_netdev.c:1024]: (style) Variable 'err'
is reassigned a value before the old one has been used.

[fm10k_netdev.c:64]: (style) Variable 'err' is assigned a value that is
never used.

[fm10k_netdev.c:131]: (style) Variable 'err' is assigned a value that
is never used.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/fm10k/fm10k_netdev.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
index 259da075093f..4704395c0f66 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c
@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: GPL-2.0
-/* Copyright(c) 2013 - 2018 Intel Corporation. */
+/* Copyright(c) 2013 - 2019 Intel Corporation. */
 
 #include "fm10k.h"
 #include <linux/vmalloc.h>
@@ -54,7 +54,7 @@ int fm10k_setup_tx_resources(struct fm10k_ring *tx_ring)
  **/
 static int fm10k_setup_all_tx_resources(struct fm10k_intfc *interface)
 {
-	int i, err = 0;
+	int i, err;
 
 	for (i = 0; i < interface->num_tx_queues; i++) {
 		err = fm10k_setup_tx_resources(interface->tx_ring[i]);
@@ -121,7 +121,7 @@ int fm10k_setup_rx_resources(struct fm10k_ring *rx_ring)
  **/
 static int fm10k_setup_all_rx_resources(struct fm10k_intfc *interface)
 {
-	int i, err = 0;
+	int i, err;
 
 	for (i = 0; i < interface->num_rx_queues; i++) {
 		err = fm10k_setup_rx_resources(interface->rx_ring[i]);
@@ -871,7 +871,7 @@ static int fm10k_uc_vlan_unsync(struct net_device *netdev,
 	u16 glort = interface->glort;
 	u16 vid = interface->vid;
 	bool set = !!(vid / VLAN_N_VID);
-	int err = -EHOSTDOWN;
+	int err;
 
 	/* drop any leading bits on the VLAN ID */
 	vid &= VLAN_N_VID - 1;
@@ -891,7 +891,7 @@ static int fm10k_mc_vlan_unsync(struct net_device *netdev,
 	u16 glort = interface->glort;
 	u16 vid = interface->vid;
 	bool set = !!(vid / VLAN_N_VID);
-	int err = -EHOSTDOWN;
+	int err;
 
 	/* drop any leading bits on the VLAN ID */
 	vid &= VLAN_N_VID - 1;
-- 
2.21.0


^ permalink raw reply related

* [net-next 4/8] fm10k: explicitly return 0 on success path in function
From: Jeff Kirsher @ 2019-08-04 11:59 UTC (permalink / raw)
  To: davem; +Cc: Jacob Keller, netdev, nhorman, sassmann, Andrew Bowers,
	Jeff Kirsher
In-Reply-To: <20190804115926.31944-1-jeffrey.t.kirsher@intel.com>

From: Jacob Keller <jacob.e.keller@intel.com>

In the fm10k_handle_resume function, return 0 explicitly at the end of
the function instead of returning the err value.

This was detected by cppcheck and resolves the following style warning
produced by that tool:

[fm10k_pci.c:2768] -> [fm10k_pci.c:2787]: (warning) Identical condition
'err', second condition is always false

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/fm10k/fm10k_pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
index 9522e9f8f8b8..73928dbe714f 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
@@ -2340,7 +2340,7 @@ static int fm10k_handle_resume(struct fm10k_intfc *interface)
 	/* Restart the MAC/VLAN request queue in-case of outstanding events */
 	fm10k_macvlan_schedule(interface);
 
-	return err;
+	return 0;
 }
 
 /**
-- 
2.21.0


^ permalink raw reply related

* [net-next 8/8] fm10k: fix fm10k_get_fault_pf to read correct address
From: Jeff Kirsher @ 2019-08-04 11:59 UTC (permalink / raw)
  To: davem; +Cc: Jacob Keller, netdev, nhorman, sassmann, Andrew Bowers,
	Jeff Kirsher
In-Reply-To: <20190804115926.31944-1-jeffrey.t.kirsher@intel.com>

From: Jacob Keller <jacob.e.keller@intel.com>

Fix assignment of the FM10K_FAULT_ADDR_LO register into fault->address
by using a bit-wise |= operation. Without this, the low address is
completely overwriting the high potion of the address. This caused the
fault to incorrectly return only the lower 32 bits of the fault address.

This issue was detected by cppcheck and resolves the following warnings
produced by that tool:

[fm10k_pf.c:1668] -> [fm10k_pf.c:1670]: (style) Variable
'fault->address' is reassigned a value before the old one has been used.

[fm10k_pf.c:1669] -> [fm10k_pf.c:1670]: (style) Variable
'fault->address' is reassigned a value before the old one has been used.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/fm10k/fm10k_pf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_pf.c b/drivers/net/ethernet/intel/fm10k/fm10k_pf.c
index 095c5b0e4096..be07bfdb0bb4 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_pf.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_pf.c
@@ -1565,7 +1565,7 @@ static s32 fm10k_get_fault_pf(struct fm10k_hw *hw, int type,
 	/* read remaining fields */
 	fault->address = fm10k_read_reg(hw, type + FM10K_FAULT_ADDR_HI);
 	fault->address <<= 32;
-	fault->address = fm10k_read_reg(hw, type + FM10K_FAULT_ADDR_LO);
+	fault->address |= fm10k_read_reg(hw, type + FM10K_FAULT_ADDR_LO);
 	fault->specinfo = fm10k_read_reg(hw, type + FM10K_FAULT_SPECINFO);
 
 	/* clear valid bit to allow for next error */
-- 
2.21.0


^ permalink raw reply related

* [net-next 7/8] fm10k: convert NON_Q_VECTORS(hw) into NON_Q_VECTORS
From: Jeff Kirsher @ 2019-08-04 11:59 UTC (permalink / raw)
  To: davem; +Cc: Jacob Keller, netdev, nhorman, sassmann, Andrew Bowers,
	Jeff Kirsher
In-Reply-To: <20190804115926.31944-1-jeffrey.t.kirsher@intel.com>

From: Jacob Keller <jacob.e.keller@intel.com>

The driver currently uses a macro to decide whether we should use
NON_Q_VECTORS_PF or NON_Q_VECTORS_VF.

However, we also define NON_Q_VECTORS_VF to the same value as
NON_Q_VECTORS_PF. This means that the macro NON_Q_VECTORS(hw) will
always return the same value.

Let's just remove this macro, and replace it directly with an enum value
on the enum non_q_vectors.

This was detected by cppcheck and fixes the following warnings when
building with BUILD=KERNEL

[fm10k_ethtool.c:1123]: (style) Same value in both branches of ternary
operator.

[fm10k_ethtool.c:1142]: (style) Same value in both branches of ternary
operator.

[fm10k_main.c:1826]: (style) Same value in both branches of ternary
operator.

[fm10k_main.c:1849]: (style) Same value in both branches of ternary
operator.

[fm10k_main.c:1858]: (style) Same value in both branches of ternary
operator.

[fm10k_pci.c:901]: (style) Same value in both branches of ternary
operator.

[fm10k_pci.c:1040]: (style) Same value in both branches of ternary
operator.

[fm10k_pci.c:1726]: (style) Same value in both branches of ternary
operator.

[fm10k_pci.c:1763]: (style) Same value in both branches of ternary
operator.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/fm10k/fm10k.h         | 10 +++-------
 drivers/net/ethernet/intel/fm10k/fm10k_ethtool.c |  6 ++----
 drivers/net/ethernet/intel/fm10k/fm10k_main.c    |  4 ++--
 drivers/net/ethernet/intel/fm10k/fm10k_pci.c     |  9 ++++-----
 4 files changed, 11 insertions(+), 18 deletions(-)

diff --git a/drivers/net/ethernet/intel/fm10k/fm10k.h b/drivers/net/ethernet/intel/fm10k/fm10k.h
index 7d42582ed48d..b14441944b4b 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k.h
+++ b/drivers/net/ethernet/intel/fm10k/fm10k.h
@@ -1,5 +1,5 @@
 /* SPDX-License-Identifier: GPL-2.0 */
-/* Copyright(c) 2013 - 2018 Intel Corporation. */
+/* Copyright(c) 2013 - 2019 Intel Corporation. */
 
 #ifndef _FM10K_H_
 #define _FM10K_H_
@@ -177,14 +177,10 @@ static inline struct netdev_queue *txring_txq(const struct fm10k_ring *ring)
 #define MIN_Q_VECTORS	1
 enum fm10k_non_q_vectors {
 	FM10K_MBX_VECTOR,
-#define NON_Q_VECTORS_VF NON_Q_VECTORS_PF
-	NON_Q_VECTORS_PF
+	NON_Q_VECTORS
 };
 
-#define NON_Q_VECTORS(hw)	(((hw)->mac.type == fm10k_mac_pf) ? \
-						NON_Q_VECTORS_PF : \
-						NON_Q_VECTORS_VF)
-#define MIN_MSIX_COUNT(hw)	(MIN_Q_VECTORS + NON_Q_VECTORS(hw))
+#define MIN_MSIX_COUNT(hw)	(MIN_Q_VECTORS + NON_Q_VECTORS)
 
 struct fm10k_q_vector {
 	struct fm10k_intfc *interface;
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_ethtool.c b/drivers/net/ethernet/intel/fm10k/fm10k_ethtool.c
index 1f7e4a8f4557..c681d2d28107 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_ethtool.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_ethtool.c
@@ -1114,13 +1114,12 @@ static void fm10k_get_channels(struct net_device *dev,
 			       struct ethtool_channels *ch)
 {
 	struct fm10k_intfc *interface = netdev_priv(dev);
-	struct fm10k_hw *hw = &interface->hw;
 
 	/* report maximum channels */
 	ch->max_combined = fm10k_max_channels(dev);
 
 	/* report info for other vector */
-	ch->max_other = NON_Q_VECTORS(hw);
+	ch->max_other = NON_Q_VECTORS;
 	ch->other_count = ch->max_other;
 
 	/* record RSS queues */
@@ -1132,14 +1131,13 @@ static int fm10k_set_channels(struct net_device *dev,
 {
 	struct fm10k_intfc *interface = netdev_priv(dev);
 	unsigned int count = ch->combined_count;
-	struct fm10k_hw *hw = &interface->hw;
 
 	/* verify they are not requesting separate vectors */
 	if (!count || ch->rx_count || ch->tx_count)
 		return -EINVAL;
 
 	/* verify other_count has not changed */
-	if (ch->other_count != NON_Q_VECTORS(hw))
+	if (ch->other_count != NON_Q_VECTORS)
 		return -EINVAL;
 
 	/* verify the number of channels does not exceed hardware limits */
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_main.c b/drivers/net/ethernet/intel/fm10k/fm10k_main.c
index 17a96a49174b..e0a2be534b20 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_main.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_main.c
@@ -1824,7 +1824,7 @@ static int fm10k_init_msix_capability(struct fm10k_intfc *interface)
 	v_budget = min_t(u16, v_budget, num_online_cpus());
 
 	/* account for vectors not related to queues */
-	v_budget += NON_Q_VECTORS(hw);
+	v_budget += NON_Q_VECTORS;
 
 	/* At the same time, hardware can only support a maximum of
 	 * hw.mac->max_msix_vectors vectors.  With features
@@ -1856,7 +1856,7 @@ static int fm10k_init_msix_capability(struct fm10k_intfc *interface)
 	}
 
 	/* record the number of queues available for q_vectors */
-	interface->num_q_vectors = v_budget - NON_Q_VECTORS(hw);
+	interface->num_q_vectors = v_budget - NON_Q_VECTORS;
 
 	return 0;
 }
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
index 73928dbe714f..bb236fa44048 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c
@@ -898,7 +898,7 @@ static void fm10k_configure_tx_ring(struct fm10k_intfc *interface,
 
 	/* Map interrupt */
 	if (ring->q_vector) {
-		txint = ring->q_vector->v_idx + NON_Q_VECTORS(hw);
+		txint = ring->q_vector->v_idx + NON_Q_VECTORS;
 		txint |= FM10K_INT_MAP_TIMER0;
 	}
 
@@ -1037,7 +1037,7 @@ static void fm10k_configure_rx_ring(struct fm10k_intfc *interface,
 
 	/* Map interrupt */
 	if (ring->q_vector) {
-		rxint = ring->q_vector->v_idx + NON_Q_VECTORS(hw);
+		rxint = ring->q_vector->v_idx + NON_Q_VECTORS;
 		rxint |= FM10K_INT_MAP_TIMER1;
 	}
 
@@ -1720,10 +1720,9 @@ int fm10k_mbx_request_irq(struct fm10k_intfc *interface)
 void fm10k_qv_free_irq(struct fm10k_intfc *interface)
 {
 	int vector = interface->num_q_vectors;
-	struct fm10k_hw *hw = &interface->hw;
 	struct msix_entry *entry;
 
-	entry = &interface->msix_entries[NON_Q_VECTORS(hw) + vector];
+	entry = &interface->msix_entries[NON_Q_VECTORS + vector];
 
 	while (vector) {
 		struct fm10k_q_vector *q_vector;
@@ -1760,7 +1759,7 @@ int fm10k_qv_request_irq(struct fm10k_intfc *interface)
 	unsigned int ri = 0, ti = 0;
 	int vector, err;
 
-	entry = &interface->msix_entries[NON_Q_VECTORS(hw)];
+	entry = &interface->msix_entries[NON_Q_VECTORS];
 
 	for (vector = 0; vector < interface->num_q_vectors; vector++) {
 		struct fm10k_q_vector *q_vector = interface->q_vector[vector];
-- 
2.21.0


^ permalink raw reply related

* [net-next 5/8] fm10k: cast page_addr to u8 * when incrementing it
From: Jeff Kirsher @ 2019-08-04 11:59 UTC (permalink / raw)
  To: davem; +Cc: Jacob Keller, netdev, nhorman, sassmann, Andrew Bowers,
	Jeff Kirsher
In-Reply-To: <20190804115926.31944-1-jeffrey.t.kirsher@intel.com>

From: Jacob Keller <jacob.e.keller@intel.com>

The page_addr variable is a void pointer. Incrementing it before calling
prefetch is technically undefined. Fix this by casting it to a u8*
pointer before incrementing it. This ensures that we increment the
pointer value in byte units, instead of relying on this undefined
behavior.

This was detected by cppcheck, and resolves the following warning
produced by that tool:

[fm10k_main.c:328]: (portability) 'page_addr' is of type 'void *'. When
using void pointers in calculations, the behaviour is undefined.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/fm10k/fm10k_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_main.c b/drivers/net/ethernet/intel/fm10k/fm10k_main.c
index 9e6bddff7625..17a96a49174b 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_main.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_main.c
@@ -315,7 +315,7 @@ static struct sk_buff *fm10k_fetch_rx_buffer(struct fm10k_ring *rx_ring,
 		/* prefetch first cache line of first page */
 		prefetch(page_addr);
 #if L1_CACHE_BYTES < 128
-		prefetch(page_addr + L1_CACHE_BYTES);
+		prefetch((void *)((u8 *)page_addr + L1_CACHE_BYTES));
 #endif
 
 		/* allocate a skb to store the frags */
-- 
2.21.0


^ 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