Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH] ftgmac100: Disable HW checksum generation on AST2500
From: Florian Fainelli @ 2019-09-10 22:05 UTC (permalink / raw)
  To: Vijay Khemka, David S. Miller, YueHaibing, Andrew Lunn,
	Kate Stewart, Mauro Carvalho Chehab, Luis Chamberlain,
	Thomas Gleixner, netdev, linux-kernel
  Cc: openbmc @ lists . ozlabs . org, joel, linux-aspeed, sdasari
In-Reply-To: <20190910213734.3112330-1-vijaykhemka@fb.com>

On 9/10/19 2:37 PM, Vijay Khemka wrote:
> HW checksum generation is not working for AST2500, specially with IPV6
> over NCSI. All TCP packets with IPv6 get dropped. By disabling this
> it works perfectly fine with IPV6.
> 
> Verified with IPV6 enabled and can do ssh.

How about IPv4, do these packets have problem? If not, can you continue
advertising NETIF_F_IP_CSUM but take out NETIF_F_IPV6_CSUM?

> 
> Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
> ---
>  drivers/net/ethernet/faraday/ftgmac100.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
> index 030fed65393e..591c9725002b 100644
> --- a/drivers/net/ethernet/faraday/ftgmac100.c
> +++ b/drivers/net/ethernet/faraday/ftgmac100.c
> @@ -1839,8 +1839,9 @@ static int ftgmac100_probe(struct platform_device *pdev)
>  	if (priv->use_ncsi)
>  		netdev->hw_features |= NETIF_F_HW_VLAN_CTAG_FILTER;
>  
> -	/* AST2400  doesn't have working HW checksum generation */
> -	if (np && (of_device_is_compatible(np, "aspeed,ast2400-mac")))
> +	/* AST2400  and AST2500 doesn't have working HW checksum generation */
> +	if (np && (of_device_is_compatible(np, "aspeed,ast2400-mac") ||
> +		   of_device_is_compatible(np, "aspeed,ast2500-mac")))
>  		netdev->hw_features &= ~NETIF_F_HW_CSUM;
>  	if (np && of_get_property(np, "no-hw-checksum", NULL))
>  		netdev->hw_features &= ~(NETIF_F_HW_CSUM | NETIF_F_RXCSUM);
> 


-- 
Florian

^ permalink raw reply

* Re: [PATCH net-next] tcp: force a PSH flag on TSO packets
From: Soheil Hassas Yeganeh @ 2019-09-10 21:52 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David S . Miller, netdev, Eric Dumazet, Neal Cardwell,
	Yuchung Cheng, Daniel Borkmann, Tariq Toukan
In-Reply-To: <20190910214928.220727-1-edumazet@google.com>

On Tue, Sep 10, 2019 at 5:49 PM Eric Dumazet <edumazet@google.com> wrote:
>
> When tcp sends a TSO packet, adding a PSH flag on it
> reduces the sojourn time of GRO packet in GRO receivers.
>
> This is particularly the case under pressure, since RX queues
> receive packets for many concurrent flows.
>
> A sender can give a hint to GRO engines when it is
> appropriate to flush a super-packet, especially when pacing
> is in the picture, since next packet is probably delayed by
> one ms.
>
> Having less packets in GRO engine reduces chance
> of LRU eviction or inflated RTT, and reduces GRO cost.
>
> We found recently that we must not set the PSH flag on
> individual full-size MSS segments [1] :
>
>  Under pressure (CWR state), we better let the packet sit
>  for a small delay (depending on NAPI logic) so that the
>  ACK packet is delayed, and thus next packet we send is
>  also delayed a bit. Eventually the bottleneck queue can
>  be drained. DCTCP flows with CWND=1 have demonstrated
>  the issue.
>
> This patch allows to slowdown the aggregate traffic without
> involving high resolution timers on senders and/or
> receivers.
>
> It has been used at Google for about four years,
> and has been discussed at various networking conferences.
>
> [1] segments smaller than MSS already have PSH flag set
>     by tcp_sendmsg() / tcp_mark_push(), unless MSG_MORE
>     has been requested by the user.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Soheil Hassas Yeganeh <soheil@google.com>
> Cc: Neal Cardwell <ncardwell@google.com>
> Cc: Yuchung Cheng <ycheng@google.com>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> Cc: Tariq Toukan <tariqt@mellanox.com>


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

Eric, thank you for the patch and the very nice commit message!

> ---
>  net/ipv4/tcp_output.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> index 42abc9bd687a5fea627cbc7cfa750d022f393d84..fec6d67bfd146dc78f0f25173fd71b8b8cc752fe 100644
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
> @@ -1050,11 +1050,22 @@ static int __tcp_transmit_skb(struct sock *sk, struct sk_buff *skb,
>         tcb = TCP_SKB_CB(skb);
>         memset(&opts, 0, sizeof(opts));
>
> -       if (unlikely(tcb->tcp_flags & TCPHDR_SYN))
> +       if (unlikely(tcb->tcp_flags & TCPHDR_SYN)) {
>                 tcp_options_size = tcp_syn_options(sk, skb, &opts, &md5);
> -       else
> +       } else {
>                 tcp_options_size = tcp_established_options(sk, skb, &opts,
>                                                            &md5);
> +               /* Force a PSH flag on all (GSO) packets to expedite GRO flush
> +                * at receiver : This slightly improve GRO performance.
> +                * Note that we do not force the PSH flag for non GSO packets,
> +                * because they might be sent under high congestion events,
> +                * and in this case it is better to delay the delivery of 1-MSS
> +                * packets and thus the corresponding ACK packet that would
> +                * release the following packet.
> +                */
> +               if (tcp_skb_pcount(skb) > 1)
> +                       tcb->tcp_flags |= TCPHDR_PSH;
> +       }
>         tcp_header_size = tcp_options_size + sizeof(struct tcphdr);
>
>         /* if no packet is in qdisc/device queue, then allow XPS to select
> --
> 2.23.0.162.g0b9fbb3734-goog
>

^ permalink raw reply

* [PATCH net-next] tcp: force a PSH flag on TSO packets
From: Eric Dumazet @ 2019-09-10 21:49 UTC (permalink / raw)
  To: David S . Miller
  Cc: netdev, Eric Dumazet, Eric Dumazet, Soheil Hassas Yeganeh,
	Neal Cardwell, Yuchung Cheng, Daniel Borkmann, Tariq Toukan

When tcp sends a TSO packet, adding a PSH flag on it
reduces the sojourn time of GRO packet in GRO receivers.

This is particularly the case under pressure, since RX queues
receive packets for many concurrent flows.

A sender can give a hint to GRO engines when it is
appropriate to flush a super-packet, especially when pacing
is in the picture, since next packet is probably delayed by
one ms.

Having less packets in GRO engine reduces chance
of LRU eviction or inflated RTT, and reduces GRO cost.

We found recently that we must not set the PSH flag on
individual full-size MSS segments [1] :

 Under pressure (CWR state), we better let the packet sit
 for a small delay (depending on NAPI logic) so that the
 ACK packet is delayed, and thus next packet we send is
 also delayed a bit. Eventually the bottleneck queue can
 be drained. DCTCP flows with CWND=1 have demonstrated
 the issue.

This patch allows to slowdown the aggregate traffic without
involving high resolution timers on senders and/or
receivers.

It has been used at Google for about four years,
and has been discussed at various networking conferences.

[1] segments smaller than MSS already have PSH flag set
    by tcp_sendmsg() / tcp_mark_push(), unless MSG_MORE
    has been requested by the user.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Soheil Hassas Yeganeh <soheil@google.com>
Cc: Neal Cardwell <ncardwell@google.com>
Cc: Yuchung Cheng <ycheng@google.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Tariq Toukan <tariqt@mellanox.com>
---
 net/ipv4/tcp_output.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 42abc9bd687a5fea627cbc7cfa750d022f393d84..fec6d67bfd146dc78f0f25173fd71b8b8cc752fe 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -1050,11 +1050,22 @@ static int __tcp_transmit_skb(struct sock *sk, struct sk_buff *skb,
 	tcb = TCP_SKB_CB(skb);
 	memset(&opts, 0, sizeof(opts));
 
-	if (unlikely(tcb->tcp_flags & TCPHDR_SYN))
+	if (unlikely(tcb->tcp_flags & TCPHDR_SYN)) {
 		tcp_options_size = tcp_syn_options(sk, skb, &opts, &md5);
-	else
+	} else {
 		tcp_options_size = tcp_established_options(sk, skb, &opts,
 							   &md5);
+		/* Force a PSH flag on all (GSO) packets to expedite GRO flush
+		 * at receiver : This slightly improve GRO performance.
+		 * Note that we do not force the PSH flag for non GSO packets,
+		 * because they might be sent under high congestion events,
+		 * and in this case it is better to delay the delivery of 1-MSS
+		 * packets and thus the corresponding ACK packet that would
+		 * release the following packet.
+		 */
+		if (tcp_skb_pcount(skb) > 1)
+			tcb->tcp_flags |= TCPHDR_PSH;
+	}
 	tcp_header_size = tcp_options_size + sizeof(struct tcphdr);
 
 	/* if no packet is in qdisc/device queue, then allow XPS to select
-- 
2.23.0.162.g0b9fbb3734-goog


^ permalink raw reply related

* [net-next 3/3] net/mlx5: FWTrace, Reduce stack usage
From: Saeed Mahameed @ 2019-09-10 21:46 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev@vger.kernel.org, Saeed Mahameed, Arnd Bergmann
In-Reply-To: <20190910214542.8433-1-saeedm@mellanox.com>

Mark mlx5_tracer_print_trace as noinline as the function only uses 512
bytes on the stack to avoid the following build warning:

drivers/net/ethernet/mellanox/mlx5/core/diag/fw_tracer.c:660:13: error: stack frame size of 1032 bytes in function 'mlx5_fw_tracer_handle_traces' [-Werror,-Wframe-larger-than=]

Fixes: 70dd6fdb8987 ("net/mlx5: FW tracer, parse traces and kernel tracing support")
Reported-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/diag/fw_tracer.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/diag/fw_tracer.c b/drivers/net/ethernet/mellanox/mlx5/core/diag/fw_tracer.c
index 2011eaf15cc5..94d7b69a95c7 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/diag/fw_tracer.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/diag/fw_tracer.c
@@ -553,9 +553,10 @@ static void mlx5_fw_tracer_save_trace(struct mlx5_fw_tracer *tracer,
 	mutex_unlock(&tracer->st_arr.lock);
 }
 
-static void mlx5_tracer_print_trace(struct tracer_string_format *str_frmt,
-				    struct mlx5_core_dev *dev,
-				    u64 trace_timestamp)
+static noinline
+void mlx5_tracer_print_trace(struct tracer_string_format *str_frmt,
+			     struct mlx5_core_dev *dev,
+			     u64 trace_timestamp)
 {
 	char	tmp[512];
 
-- 
2.21.0


^ permalink raw reply related

* [net-next 2/3] net/mlx5: Fix addr's type in mlx5dr_icm_dm
From: Saeed Mahameed @ 2019-09-10 21:46 UTC (permalink / raw)
  To: David S. Miller
  Cc: netdev@vger.kernel.org, Nathan Chancellor, Arnd Bergmann,
	Saeed Mahameed
In-Reply-To: <20190910214542.8433-1-saeedm@mellanox.com>

From: Nathan Chancellor <natechancellor@gmail.com>

clang errors when CONFIG_PHYS_ADDR_T_64BIT is not set:

drivers/net/ethernet/mellanox/mlx5/core/steering/dr_icm_pool.c:121:8:
error: incompatible pointer types passing 'u64 *' (aka 'unsigned long
long *') to parameter of type 'phys_addr_t *' (aka 'unsigned int *')
[-Werror,-Wincompatible-pointer-types]
                                   &icm_mr->dm.addr, &icm_mr->dm.obj_id);
                                   ^~~~~~~~~~~~~~~~
include/linux/mlx5/driver.h:1092:39: note: passing argument to parameter
'addr' here
                         u64 length, u16 uid, phys_addr_t *addr, u32 *obj_id);
                                                           ^
1 error generated.

Use phys_addr_t for addr's type in mlx5dr_icm_dm, which won't change
anything with 64-bit builds because phys_addr_t is u64 when
CONFIG_PHYS_ADDR_T_64BIT is set, which is always when CONFIG_64BIT is
set.

Fixes: 29cf8febd185 ("net/mlx5: DR, ICM pool memory allocator")
Link: https://github.com/ClangBuiltLinux/linux/issues/653
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
Reported-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/steering/dr_icm_pool.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_icm_pool.c b/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_icm_pool.c
index e76f61e7555e..913f1e5aaaf2 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_icm_pool.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_icm_pool.c
@@ -53,7 +53,7 @@ struct mlx5dr_icm_pool {
 struct mlx5dr_icm_dm {
 	u32 obj_id;
 	enum mlx5_sw_icm_type type;
-	u64 addr;
+	phys_addr_t addr;
 	size_t length;
 };
 
-- 
2.21.0


^ permalink raw reply related

* [net-next 1/3] net/mlx5: Fix rt's type in dr_action_create_reformat_action
From: Saeed Mahameed @ 2019-09-10 21:45 UTC (permalink / raw)
  To: David S. Miller
  Cc: netdev@vger.kernel.org, Nathan Chancellor, Austin Kim,
	Arnd Bergmann, Saeed Mahameed
In-Reply-To: <20190910214542.8433-1-saeedm@mellanox.com>

From: Nathan Chancellor <natechancellor@gmail.com>

clang warns:

drivers/net/ethernet/mellanox/mlx5/core/steering/dr_action.c:1080:9:
warning: implicit conversion from enumeration type 'enum
mlx5_reformat_ctx_type' to different enumeration type 'enum
mlx5dr_action_type' [-Wenum-conversion]
                        rt = MLX5_REFORMAT_TYPE_L2_TO_L2_TUNNEL;
                           ~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/ethernet/mellanox/mlx5/core/steering/dr_action.c:1082:9:
warning: implicit conversion from enumeration type 'enum
mlx5_reformat_ctx_type' to different enumeration type 'enum
mlx5dr_action_type' [-Wenum-conversion]
                        rt = MLX5_REFORMAT_TYPE_L2_TO_L3_TUNNEL;
                           ~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/ethernet/mellanox/mlx5/core/steering/dr_action.c:1084:51:
warning: implicit conversion from enumeration type 'enum
mlx5dr_action_type' to different enumeration type 'enum
mlx5_reformat_ctx_type' [-Wenum-conversion]
                ret = mlx5dr_cmd_create_reformat_ctx(dmn->mdev, rt, data_sz, data,
                      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~            ^~
3 warnings generated.

Use the right type for rt, which is mlx5_reformat_ctx_type so there are
no warnings about mismatched types.

Fixes: 9db810ed2d37 ("net/mlx5: DR, Expose steering action functionality")
Link: https://github.com/ClangBuiltLinux/linux/issues/652
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
Reported-by: Austin Kim <austindh.kim@gmail.com>
Reported-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/steering/dr_action.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_action.c b/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_action.c
index a02f87f85c17..7d81a7735de5 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_action.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/steering/dr_action.c
@@ -1074,7 +1074,7 @@ dr_action_create_reformat_action(struct mlx5dr_domain *dmn,
 	case DR_ACTION_TYP_L2_TO_TNL_L2:
 	case DR_ACTION_TYP_L2_TO_TNL_L3:
 	{
-		enum mlx5dr_action_type rt;
+		enum mlx5_reformat_ctx_type rt;
 
 		if (action->action_type == DR_ACTION_TYP_L2_TO_TNL_L2)
 			rt = MLX5_REFORMAT_TYPE_L2_TO_L2_TUNNEL;
-- 
2.21.0


^ permalink raw reply related

* Re: ixgbe: driver drops packets routed from an IPSec interface with a "bad sa_idx" error
From: Shannon Nelson @ 2019-09-10 21:43 UTC (permalink / raw)
  To: Michael Marley; +Cc: netdev, Jeff Kirsher, steffen.klassert
In-Reply-To: <fb63dec226170199e9b0fd1b356d2314@michaelmarley.com>

On 9/9/19 11:45 AM, Michael Marley wrote:
> On 2019-09-09 14:21, Shannon Nelson wrote:
>> On 9/6/19 11:13 AM, Michael Marley wrote:
>>> (This is also reported at 
>>> https://bugzilla.kernel.org/show_bug.cgi?id=204551, but it was 
>>> recommended that I send it to this list as well.)
>>>
>>> I have a put together a router that routes traffic from several 
>>> local subnets from a switch attached to an i82599ES card through an 
>>> IPSec VPN interface set up with StrongSwan. (The VPN is running on 
>>> an unrelated second interface with a different driver.)  Traffic 
>>> from the local interfaces to the VPN works as it should and 
>>> eventually makes it through the VPN server and out to the Internet.  
>>> The return traffic makes it back to the router and tcpdump shows it 
>>> leaving by the i82599, but the traffic never actually makes it onto 
>>> the wire and I instead get one of
>>>
>>> enp1s0: ixgbe_ipsec_tx: bad sa_idx=64512 handle=0
>>>
>>> for each packet that should be transmitted.  (The sa_idx and handle 
>>> values are always the same.)
>>>
>>> I realized this was probably related to ixgbe's IPSec offloading 
>>> feature, so I tried with the motherboard's integrated e1000e device 
>>> and didn't have the problem.  I tried using ethtool to disable all 
>>> the IPSec-related offloads (tx-esp-segmentation, esp-hw-offload, 
>>> esp-tx-csum-hw-offload), but the problem persisted.  I then tried 
>>> recompiling the kernel with CONFIG_IXGBE_IPSEC=n and that worked 
>>> around the problem.
>>>
>>> I was also able to find another instance of the same problem 
>>> reported in Debian at 
>>> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=930443. That 
>>> person seems to be having exactly the same issue as me, down to the 
>>> sa_idx and handle values being the same.
>>>
>>> If there are any more details I can provide to make this easier to 
>>> track down, please let me know.
>>>
>>> Thanks,
>>>
>>> Michael Marley
>>
>> Hi Michael,
>>
>> Thanks for pointing this out.  The issue this error message is
>> complaining about is that the handle given to the driver is a bad
>> value.  The handle is what helps the driver find the right encryption
>> information, and in this case is an index into an array, one array for
>> Rx and one for Tx, each of which have up to 1024 entries.  In order to
>> encode them into a single value, 1024 is added to the Tx values to
>> make the handle, and 1024 is subtracted to use the handle later.  Note
>> that the bad sa_idx is 64512, which happens to also be -1024; if the
>> Tx handle given to ixgbe for xmit is 0, we subtract 1024 from that and
>> get this bad sa_idx value.
>>
>> That handle is supposed to be an opaque value only used by the
>> driver.  It looks to me like either (a) the driver is not setting up
>> the handle correctly when the SA is first set up, or (b) something in
>> the upper levels of the ipsec code is clearing the handle value. We
>> would need to know more about all the bits in your SA set up to have a
>> better idea what parts of the ipsec code are being exercised when this
>> problem happens.
>>
>> I currently don't have access to a good ixgbe setup on which to
>> test/debug this, and I haven't been paying much attention lately to
>> what's happening in the upper ipsec layers, so my help will be
>> somewhat limited.  I'm hoping the the Intel folks can add a little
>> help, so I've copied Jeff Kirsher on this (they'll probably point back
>> to me since I wrote this chunk :-) ).  I've also copied Stephen
>> Klassert for his ipsec thoughts.
>>
>> In the meantime, can you give more details on the exact ipsec rules
>> that are used here, and are there any error messages coming from ixgbe
>> regarding the ipsec rule setup that might help us identify what's
>> happening?
>>
>> Thanks,
>> sln
>
> Hi Shannon,
>
> Thanks for your response!  I apologize, I am a bit of a newbie to 
> IPSec myself, so I'm not 100% sure what is the best way to provide the 
> information you need, but here is the (slightly-redacted) output of 
> swanctl --list-sas first from the server and then from the client:
>
> <servername>: #24, ESTABLISHED, IKEv2, 3cb75c180ee5dc68_i 
> cc7dae551b603bb7_r*
>   local  '<serverip>' @ <serverip>[4500]
>   remote '<clientip>' @ <clientip>[4500]
>   AES_GCM_16-256/PRF_HMAC_SHA2_512/ECP_384
>   established 174180s ago
>   <servername>: #110, reqid 12, INSTALLED, TUNNEL-in-UDP, 
> ESP:AES_GCM_16-256/ECP_384
>     installed 469s ago
>     in  c51a0f11 (-|0x00000064), 1548864 bytes, 19575 packets, 6s ago
>     out c3bd9741 (-|0x00000064), 23618807 bytes, 22865 packets,     7s 
> ago
>     local  0.0.0.0/0 ::/0
>     remote 0.0.0.0/0 ::/0
>
> <clientname>: #1, ESTABLISHED, IKEv2, 3cb75c180ee5dc68_i* 
> cc7dae551b603bb7_r
>   local  '<clientip>' @ <clientip>[4500]
>   remote '<serverip>' @ <serverip>[4500]
>   AES_GCM_16-256/PRF_HMAC_SHA2_512/ECP_384
>   established 174013s ago
>   <clientname>: #54, reqid 1, INSTALLED, TUNNEL-in-UDP, 
> ESP:AES_GCM_16-256/ECP_384
>     installed 303s ago, rekeying in 2979s, expires in 3657s
>     in  c3bd9741 (-|0x00000064), 23178523 bytes, 20725 packets,     0s 
> ago
>     out c51a0f11 (-|0x00000064), 1429124 bytes, 17719 packets, 0s ago
>     local  0.0.0.0/0 ::/0
>     remote 0.0.0.0/0 ::/0
>
> It might also be worth mentioning that I am using an xfrm interface to 
> do "regular" routing rather than the policy-based routing that 
> StrongSwan/IPSec normally uses. If there is anything else that would 
> help more, I would be happy to provide it.
>
> Just to be clear though, I'm not trying to run IPSec on the ixgbe 
> interface at all.  The ixgbe adapter is being used to connect the 
> router to the switch on the LAN side of the network.  IPSec is running 
> on the WAN interface without any hardware acceleration (besides 
> AES-NI).  The problem occurs when a computer on the LAN tries to 
> access the WAN.  The outgoing packets work as expected and the 
> incoming packets are routed back out through the ixgbe device toward 
> the LAN client, but the driver drops the packets with the sa_idx error.
>
> I hope this helps.
>
> Thanks,
>
> Michael

I'm not familiar with StrongSwan and its configurations, but I'm 
guessing that if you didn't expressly enable it, perhaps StrongSwan 
enabled the ipsec offload capability.  I would suggest turning it off to 
at least get you passed the immediate issue.  If there isn't an obvious 
configuration knob in StrongSwan, perhaps you can at least use ethtool 
to disable the offload, which should be off be default anyway.

You can check it with "ethtool -k ethX | grep esp-hw-offload" and see if 
it is set.  You can disable it with "ethtool -K ethX esp-hw-offload off"

Meanwhile, can you please send the output of the following commands:
uname -a
ip xfrm s
ip xfrm p
dmesg | grep ixgbe

And any other /var/log/syslog or /var/log/messages that look suspicious 
and might give any more insight to what's happening.

Thanks,
sln


^ permalink raw reply

* [pull request][net-next 0/3] Mellanox, mlx5 build cleanup 2019-09-10
From: Saeed Mahameed @ 2019-09-10 21:45 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev@vger.kernel.org, Saeed Mahameed

Hi Dave,

This series provides three build warnings cleanup patches for mlx5,
Originally i wanted to wait a bit more and attach more patches to this
series, but apparently this can't wait since already 3 different patches
for the same fix were submitted this week :).

For more information please see tag log below.

Please pull and let me know if there is any problem.

Thanks,
Saeed.

---
The following changes since commit 074be7fd99a29ff36dcb2c036b3b31a6b670b3cf:

  Merge branch 'nfp-implement-firmware-loading-policy' (2019-09-10 17:29:27 +0100)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/saeed/linux.git tags/mlx5-updates-2019-09-10

for you to fetch changes up to fa355bb1b0373e7fe087cfc830b1b0b9b6130388:

  net/mlx5: FWTrace, Reduce stack usage (2019-09-10 13:43:27 -0700)

----------------------------------------------------------------
mlx5-updates-2019-09-10

Misc build warnings cleanup for mlx5:

1) Reduce stack usage in FW trace
2) Fix addr's type in mlx5dr_icm_dm
3) Fix rt's type in dr_action_create_reformat_action

----------------------------------------------------------------
Nathan Chancellor (2):
      net/mlx5: Fix rt's type in dr_action_create_reformat_action
      net/mlx5: Fix addr's type in mlx5dr_icm_dm

Saeed Mahameed (1):
      net/mlx5: FWTrace, Reduce stack usage

 drivers/net/ethernet/mellanox/mlx5/core/diag/fw_tracer.c       | 7 ++++---
 drivers/net/ethernet/mellanox/mlx5/core/steering/dr_action.c   | 2 +-
 drivers/net/ethernet/mellanox/mlx5/core/steering/dr_icm_pool.c | 2 +-
 3 files changed, 6 insertions(+), 5 deletions(-)

^ permalink raw reply

* [PATCH] cxgb4: Fix spelling typos
From: Arkadiusz Drabczyk @ 2019-09-10 20:49 UTC (permalink / raw)
  To: vishal; +Cc: davem, netdev, linux-kernel

Fix several spelling typos in comments in t4_hw.c.

Signed-off-by: Arkadiusz Drabczyk <arkadiusz@drabczyk.org>
---
 drivers/net/ethernet/chelsio/cxgb4/t4_hw.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c b/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c
index f7fc553..f2a7824 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c
@@ -329,7 +329,7 @@ int t4_wr_mbox_meat_timeout(struct adapter *adap, int mbox, const void *cmd,
 	for (i = 0; ; i += ms) {
 		/* If we've waited too long, return a busy indication.  This
 		 * really ought to be based on our initial position in the
-		 * mailbox access list but this is a start.  We very rearely
+		 * mailbox access list but this is a start.  We very rarely
 		 * contend on access to the mailbox ...
 		 */
 		pcie_fw = t4_read_reg(adap, PCIE_FW_A);
@@ -606,7 +606,7 @@ void t4_memory_rw_residual(struct adapter *adap, u32 off, u32 addr, u8 *buf,
  *
  *	Reads/writes an [almost] arbitrary memory region in the firmware: the
  *	firmware memory address and host buffer must be aligned on 32-bit
- *	boudaries; the length may be arbitrary.  The memory is transferred as
+ *	boundaries; the length may be arbitrary.  The memory is transferred as
  *	a raw byte sequence from/to the firmware's memory.  If this memory
  *	contains data structures which contain multi-byte integers, it's the
  *	caller's responsibility to perform appropriate byte order conversions.
@@ -3774,7 +3774,7 @@ int t4_phy_fw_ver(struct adapter *adap, int *phy_fw_ver)
  *	A negative error number will be returned if an error occurs.  If
  *	version number support is available and there's no need to upgrade
  *	the firmware, 0 will be returned.  If firmware is successfully
- *	transferred to the adapter, 1 will be retured.
+ *	transferred to the adapter, 1 will be returned.
  *
  *	NOTE: some adapters only have local RAM to store the PHY firmware.  As
  *	a result, a RESET of the adapter would cause that RAM to lose its
@@ -3808,7 +3808,7 @@ int t4_load_phy_fw(struct adapter *adap,
 	}
 
 	/* Ask the firmware where it wants us to copy the PHY firmware image.
-	 * The size of the file requires a special version of the READ coommand
+	 * The size of the file requires a special version of the READ command
 	 * which will pass the file size via the values field in PARAMS_CMD and
 	 * retrieve the return value from firmware and place it in the same
 	 * buffer values
@@ -4082,7 +4082,7 @@ static inline fw_port_cap32_t cc_to_fwcap_pause(enum cc_pause cc_pause)
 		fw_pause |= FW_PORT_CAP32_FORCE_PAUSE;
 
 	/* Translate orthogonal Pause controls into IEEE 802.3 Pause,
-	 * Asymetrical Pause for use in reporting to upper layer OS code, etc.
+	 * Asymmetrical Pause for use in reporting to upper layer OS code, etc.
 	 * Note that these bits are ignored in L1 Configure commands.
 	 */
 	if (cc_pause & PAUSE_RX) {
@@ -4151,7 +4151,7 @@ fw_port_cap32_t t4_link_acaps(struct adapter *adapter, unsigned int port,
 	/* Convert Common Code Forward Error Control settings into the
 	 * Firmware's API.  If the current Requested FEC has "Automatic"
 	 * (IEEE 802.3) specified, then we use whatever the Firmware
-	 * sent us as part of it's IEEE 802.3-based interpratation of
+	 * sent us as part of its IEEE 802.3-based interpretation of
 	 * the Transceiver Module EPROM FEC parameters.  Otherwise we
 	 * use whatever is in the current Requested FEC settings.
 	 */
@@ -4248,7 +4248,7 @@ int t4_link_l1cfg_core(struct adapter *adapter, unsigned int mbox,
 
 	/* Unfortunately, even if the Requested Port Capabilities "fit" within
 	 * the Physical Port Capabilities, some combinations of features may
-	 * still not be leagal.  For example, 40Gb/s and Reed-Solomon Forward
+	 * still not be legal.  For example, 40Gb/s and Reed-Solomon Forward
 	 * Error Correction.  So if the Firmware rejects the L1 Configure
 	 * request, flag that here.
 	 */
@@ -6797,7 +6797,7 @@ int t4_sge_ctxt_flush(struct adapter *adap, unsigned int mbox, int ctxt_type)
 }
 
 /**
- *	t4_read_sge_dbqtimers - reag SGE Doorbell Queue Timer values
+ *	t4_read_sge_dbqtimers - read SGE Doorbell Queue Timer values
  *	@adap - the adapter
  *	@ndbqtimers: size of the provided SGE Doorbell Queue Timer table
  *	@dbqtimers: SGE Doorbell Queue Timer table
@@ -6925,8 +6925,8 @@ int t4_fw_hello(struct adapter *adap, unsigned int mbox, unsigned int evt_mbox,
 			waiting -= 50;
 
 			/*
-			 * If neither Error nor Initialialized are indicated
-			 * by the firmware keep waiting till we exaust our
+			 * If neither Error nor Initialized are indicated
+			 * by the firmware keep waiting till we exhaust our
 			 * timeout ... and then retry if we haven't exhausted
 			 * our retries ...
 			 */
@@ -7238,7 +7238,7 @@ int t4_fl_pkt_align(struct adapter *adap)
 	 * separately.  The actual Ingress Packet Data alignment boundary
 	 * within Packed Buffer Mode is the maximum of these two
 	 * specifications.  (Note that it makes no real practical sense to
-	 * have the Pading Boudary be larger than the Packing Boundary but you
+	 * have the Padding Boundary be larger than the Packing Boundary but you
 	 * could set the chip up that way and, in fact, legacy T4 code would
 	 * end doing this because it would initialize the Padding Boundary and
 	 * leave the Packing Boundary initialized to 0 (16 bytes).)
@@ -8973,10 +8973,10 @@ static int t4_get_flash_params(struct adapter *adap)
 			goto found;
 		}
 
-	/* Decode Flash part size.  The code below looks repetative with
+	/* Decode Flash part size.  The code below looks repetitive with
 	 * common encodings, but that's not guaranteed in the JEDEC
-	 * specification for the Read JADEC ID command.  The only thing that
-	 * we're guaranteed by the JADEC specification is where the
+	 * specification for the Read JEDEC ID command.  The only thing that
+	 * we're guaranteed by the JEDEC specification is where the
 	 * Manufacturer ID is in the returned result.  After that each
 	 * Manufacturer ~could~ encode things completely differently.
 	 * Note, all Flash parts must have 64KB sectors.
@@ -9317,7 +9317,7 @@ int t4_init_devlog_params(struct adapter *adap)
 	struct fw_devlog_cmd devlog_cmd;
 	int ret;
 
-	/* If we're dealing with newer firmware, the Device Log Paramerters
+	/* If we're dealing with newer firmware, the Device Log Parameters
 	 * are stored in a designated register which allows us to access the
 	 * Device Log even if we can't talk to the firmware.
 	 */
-- 
2.9.0


^ permalink raw reply related

* Re: [PATCH 3/7] dt-bindings: mv88e6xxx: add ability to set default queue priorities per port
From: Vladimir Oltean @ 2019-09-10 20:46 UTC (permalink / raw)
  To: Vivien Didelot
  Cc: Florian Fainelli, Robert Beckett, netdev, Andrew Lunn,
	David S. Miller, Rob Herring, Mark Rutland, devicetree,
	Jiri Pirko, Ido Schimmel
In-Reply-To: <20190910124952.GG32337@t480s.localdomain>

Hi guys,

On 10/09/2019, Vivien Didelot <vivien.didelot@gmail.com> wrote:
> Hi Robert,
>
> On Tue, 10 Sep 2019 09:42:24 -0700, Florian Fainelli <f.fainelli@gmail.com>
> wrote:
>> This is a vendor specific driver/property,
>> marvell,default-queue-priority (which be cheapskate on words) would be
>> more readable. But still, I have some more fundamental issues with the
>> general approach, see my response in the cover letter.
>
> As Florian said, the DT is unlikely to welcome vendor specific nodes for
> configuration which may be generic through standard network userspace
> tools.
>
>
> Thanks,
>
> 	Vivien
>

While I do agree that the DT bindings are a big no-no for QoS
settings, the topic is interesting.
What is the user space knob for configuring port-default priority (say
RX queue)?
Something like this maybe? (a very forced "matchall" with rxnfc)
ethtool --config-nfc eth0 flow-type ether src 00:00:00:00:00:00 m
00:00:00:00:00:00 action 5

Regards,
-Vladimir

^ permalink raw reply

* Re: [PATCH v2] tcp: Add TCP_INFO counter for packets received out-of-order
From: Eric Dumazet @ 2019-09-10 20:38 UTC (permalink / raw)
  To: Thomas Higdon; +Cc: netdev, Jonathan Lemon, Dave Jones
In-Reply-To: <20190910201128.3967163-1-tph@fb.com>

On Tue, Sep 10, 2019 at 10:11 PM Thomas Higdon <tph@fb.com> wrote:
>
>
...
> Because an additional 32-bit member in struct tcp_info would cause
> a hole on 64-bit systems, we reserve a struct member '_reserved'.
...
> diff --git a/include/uapi/linux/tcp.h b/include/uapi/linux/tcp.h
> index b3564f85a762..990a5bae3ac1 100644
> --- a/include/uapi/linux/tcp.h
> +++ b/include/uapi/linux/tcp.h
> @@ -270,6 +270,9 @@ struct tcp_info {
>         __u64   tcpi_bytes_retrans;  /* RFC4898 tcpEStatsPerfOctetsRetrans */
>         __u32   tcpi_dsack_dups;     /* RFC4898 tcpEStatsStackDSACKDups */
>         __u32   tcpi_reord_seen;     /* reordering events seen */
> +
> +       __u32   _reserved;           /* Reserved for future 32-bit member. */
> +       __u32   tcpi_rcv_ooopack;    /* Out-of-order packets received */
>  };
>

Unfortunately we won't be able to use this hole, because the way the
TCP_INFO works,

The kernel will report the same size after the reserved field is
renamed to something else.

User space code is able to detect which fields are there or not based
on what the kernel
returns for the size of the structure.

^ permalink raw reply

* Re: [PATCH net-next 0/6] net: stmmac: Improvements for -next
From: Jesse Brandeburg @ 2019-09-10 20:36 UTC (permalink / raw)
  To: Jose Abreu
  Cc: netdev, Joao Pinto, Giuseppe Cavallaro, Alexandre Torgue,
	David S. Miller, Maxime Coquelin, linux-stm32, linux-arm-kernel,
	linux-kernel, jesse.brandeburg
In-Reply-To: <cover.1568126224.git.joabreu@synopsys.com>

On Tue, 10 Sep 2019 16:41:21 +0200 Jose wrote:
> Misc patches for -next. It includes:
>  - Two fixes for features in -next only
>  - New features support for GMAC cores (which includes GMAC4 and GMAC5)
> 
> ---
> Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
> Cc: Alexandre Torgue <alexandre.torgue@st.com>
> Cc: Jose Abreu <joabreu@synopsys.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
> Cc: netdev@vger.kernel.org
> Cc: linux-stm32@st-md-mailman.stormreply.com
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> ---
> 
> Jose Abreu (6):
>   net: stmmac: Prevent divide-by-zero
>   net: stmmac: Add VLAN HASH filtering support in GMAC4+
>   net: stmmac: xgmac: Reinitialize correctly a variable
>   net: stmmac: Add support for SA Insertion/Replacement in GMAC4+
>   net: stmmac: Add support for VLAN Insertion Offload in GMAC4+
>   net: stmmac: ARP Offload for GMAC4+ Cores

For the series, looks good to me.

Reviewed-by: Jesse Brandeburg <jesse.brandeburg@intel.com>

^ permalink raw reply

* [PATCH net] net: Fix null de-reference of device refcount
From: Subash Abhinov Kasiviswanathan @ 2019-09-10 20:02 UTC (permalink / raw)
  To: dlezcano, eric.dumazet, davem, netdev
  Cc: Subash Abhinov Kasiviswanathan, Sean Tranchetti

In event of failure during register_netdevice, free_netdev is
invoked immediately. free_netdev assumes that all the netdevice
refcounts have been dropped prior to it being called and as a
result frees and clears out the refcount pointer.

However, this is not necessarily true as some of the operations
in the NETDEV_UNREGISTER notifier handlers queue RCU callbacks for
invocation after a grace period. The IPv4 callback in_dev_rcu_put
tries to access the refcount after free_netdev is called which
leads to a null de-reference-

44837.761523:   <6> Unable to handle kernel paging request at
                    virtual address 0000004a88287000
44837.761651:   <2> pc : in_dev_finish_destroy+0x4c/0xc8
44837.761654:   <2> lr : in_dev_finish_destroy+0x2c/0xc8
44837.762393:   <2> Call trace:
44837.762398:   <2>  in_dev_finish_destroy+0x4c/0xc8
44837.762404:   <2>  in_dev_rcu_put+0x24/0x30
44837.762412:   <2>  rcu_nocb_kthread+0x43c/0x468
44837.762418:   <2>  kthread+0x118/0x128
44837.762424:   <2>  ret_from_fork+0x10/0x1c

Fix this by waiting for the completion of the call_rcu() in
case of register_netdevice errors.

Fixes: 93ee31f14f6f ("[NET]: Fix free_netdev on register_netdev failure.")
Cc: Sean Tranchetti <stranche@codeaurora.org>
Signed-off-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
---
 net/core/dev.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/core/dev.c b/net/core/dev.c
index 49589ed..c7463c9 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -8796,6 +8796,8 @@ int register_netdevice(struct net_device *dev)
 	ret = notifier_to_errno(ret);
 	if (ret) {
 		rollback_registered(dev);
+		rcu_barrier();
+
 		dev->reg_state = NETREG_UNREGISTERED;
 	}
 	/*
-- 
1.9.1


^ permalink raw reply related

* Re: [PATCH] tcp: Add TCP_INFO counter for packets received out-of-order
From: Thomas Higdon @ 2019-09-10 19:59 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev, Jonathan Lemon, Dave Jones
In-Reply-To: <CANn89iJ5wANqhpR28y5AYf6GTBgzTau+u0N0ogG690C71LbxaA@mail.gmail.com>

On Mon, Sep 09, 2019 at 05:01:46PM +0200, Eric Dumazet wrote:
> On Mon, Sep 9, 2019 at 4:30 PM Thomas Higdon <tph@fb.com> wrote:
> > diff --git a/include/uapi/linux/tcp.h b/include/uapi/linux/tcp.h
> > index b3564f85a762..20237987ccc8 100644
> > --- a/include/uapi/linux/tcp.h
> > +++ b/include/uapi/linux/tcp.h
> > @@ -270,6 +270,8 @@ struct tcp_info {
> >         __u64   tcpi_bytes_retrans;  /* RFC4898 tcpEStatsPerfOctetsRetrans */
> >         __u32   tcpi_dsack_dups;     /* RFC4898 tcpEStatsStackDSACKDups */
> >         __u32   tcpi_reord_seen;     /* reordering events seen */
> > +
> > +       __u32   tcpi_rcv_ooopack;    /* Out-of-order packets received */
> 
> This is problematic : you create a 32bit hole in this structure that
> we will never be able to fill.
> 
> We need to add another metric here so that the whole 64bit space is used.

I don't have another metric to add currently. Perhaps I could first place
a '__u32 _reserved' member so that someone else may replace it with
a 32-bit member in the future. Unless there is a canonical way to do
this? I couldn't find any prior examples.

> > diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> > index 706cbb3b2986..2774680c5d05 100644
> > --- a/net/ipv4/tcp_input.c
> > +++ b/net/ipv4/tcp_input.c
> > @@ -4555,6 +4555,7 @@ static void tcp_data_queue_ofo(struct sock *sk, struct sk_buff *skb)
> >         tp->pred_flags = 0;
> >         inet_csk_schedule_ack(sk);
> >
> > +       tp->rcv_ooopack++;
> 
> We count skbs or we count segments ?
> 
> (GRO might have aggregated multiple segments)

Let's count segments -- I will copy the technique of tcp_segs_in(), which
checks the maximum of 1 and gso_segs from the lower layer. Interestingly,
on my development machine, which uses the virtio-net driver, when LRO is
enabled, gso_segs is always zero, even when an aggregated segment is
passed up the stack. I guess this may be a problem with virtio-net? It
still seems that using gso_segs here is the correct solution.

^ permalink raw reply

* Re: [PATCH net-next 2/2] mlx5: fix type mismatch
From: Arnd Bergmann @ 2019-09-10 19:51 UTC (permalink / raw)
  To: Saeed Mahameed
  Cc: dledford@redhat.com, davem@davemloft.net, jgg@ziepe.ca,
	leon@kernel.org, linux-rdma@vger.kernel.org, Yishai Hadas,
	Mark Bloch, linux-kernel@vger.kernel.org, Erez Shitrit,
	netdev@vger.kernel.org, Alex Vesker, Ariel Levkovich,
	Nathan Chancellor
In-Reply-To: <8311cb643690d3e80dddd5d4f2f6a7d923b9fbbc.camel@mellanox.com>

On Tue, Sep 10, 2019 at 7:56 PM Saeed Mahameed <saeedm@mellanox.com> wrote:
>
> On Mon, 2019-09-09 at 21:50 +0200, Arnd Bergmann wrote:
> > In mlx5, pointers to 'phys_addr_t' and 'u64' are mixed since the
> > addition
> > of the pool memory allocator, leading to incorrect behavior on 32-bit
> > architectures and this compiler warning:
> >
> > drivers/net/ethernet/mellanox/mlx5/core/steering/dr_icm_pool.c:121:8:
> > error: incompatible pointer types passing 'u64 *' (aka 'unsigned long
> > long *') to parameter of type 'phys_addr_t *' (aka 'unsigned int *')
> > [-Werror,-Wincompatible-pointer-types]
> >                                    &icm_mr->dm.addr, &icm_mr-
> > >dm.obj_id);
> >                                    ^~~~~~~~~~~~~~~~
> > include/linux/mlx5/driver.h:1092:39: note: passing argument to
> > parameter 'addr' here
> >                          u64 length, u16 uid, phys_addr_t *addr, u32
> > *obj_id);
> >
> > Change the code to use 'u64' consistently in place of 'phys_addr_t'
> > to
> > fix this. The alternative of using phys_addr_t more would require a
> > larger
> > rework.
> >
> > Fixes: 29cf8febd185 ("net/mlx5: DR, ICM pool memory allocator")
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>
> Hi Arnd,
>
> Nathan Chancellor Already submitted a patch to fix this and it is more
> minimal:
> https://patchwork.ozlabs.org/patch/1158177/
>
> I would like to use that patch if it is ok with you..

Yes, please do. I think I had tried something like that
initially and concluded it wasn't quite right before I went
into a different direction with my patch.

Looking at the two versions now, I also prefer Nathan's,
and I just confirmed that it fixes all the randconfig failures
I ran into.

       Arnd

^ permalink raw reply

* Re: [PATCH net 1/2] sctp: remove redundant assignment when call sctp_get_port_local
From: Dan Carpenter @ 2019-09-10 19:22 UTC (permalink / raw)
  To: Mao Wenan
  Cc: vyasevich, nhorman, marcelo.leitner, davem, linux-sctp, netdev,
	linux-kernel, kernel-janitors
In-Reply-To: <20190910185710.GF15977@kadam>

On Tue, Sep 10, 2019 at 09:57:10PM +0300, Dan Carpenter wrote:
> On Tue, Sep 10, 2019 at 03:13:42PM +0800, Mao Wenan wrote:
> > There are more parentheses in if clause when call sctp_get_port_local
> > in sctp_do_bind, and redundant assignment to 'ret'. This patch is to
> > do cleanup.
> > 
> > Signed-off-by: Mao Wenan <maowenan@huawei.com>
> > ---
> >  net/sctp/socket.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> > 
> > diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> > index 9d1f83b10c0a..766b68b55ebe 100644
> > --- a/net/sctp/socket.c
> > +++ b/net/sctp/socket.c
> > @@ -399,9 +399,8 @@ static int sctp_do_bind(struct sock *sk, union sctp_addr *addr, int len)
> >  	 * detection.
> >  	 */
> >  	addr->v4.sin_port = htons(snum);
> > -	if ((ret = sctp_get_port_local(sk, addr))) {
> > +	if (sctp_get_port_local(sk, addr))
> >  		return -EADDRINUSE;
> 
> sctp_get_port_local() returns a long which is either 0,1 or a pointer
> casted to long.  It's not documented what it means and neither of the
> callers use the return since commit 62208f12451f ("net: sctp: simplify
> sctp_get_port").

Actually it was commit 4e54064e0a13 ("sctp: Allow only 1 listening
socket with SO_REUSEADDR") from 11 years ago.  That patch fixed a bug,
because before the code assumed that a pointer casted to an int was the
same as a pointer casted to a long.

regards,
dan carpenter


^ permalink raw reply

* Re: [PATCH net-next 03/11] net: aquantia: add basic ptp_clock callbacks
From: Andrew Lunn @ 2019-09-10 19:20 UTC (permalink / raw)
  To: Igor Russkikh
  Cc: netdev@vger.kernel.org, richardcochran@gmail.com,
	davem@davemloft.net, Egor Pomozov, Sergey Samoilenko,
	Dmitry Bezrukov
In-Reply-To: <8868449ec5508f498131ee141399149bf801ea94.1568034880.git.igor.russkikh@aquantia.com>

> +static s64 ptp_clk_offset;

> +static void hw_atl_b0_get_ptp_ts(struct aq_hw_s *self, u64 *stamp)
> +{
> +	u64 ns;
> +
> +	hw_atl_pcs_ptp_clock_read_enable(self, 1);
> +	hw_atl_pcs_ptp_clock_read_enable(self, 0);
> +	ns = (get_ptp_ts_val_u64(self, 0) +
> +	      (get_ptp_ts_val_u64(self, 1) << 16)) * 1000000000llu +
> +	     (get_ptp_ts_val_u64(self, 3) +
> +	      (get_ptp_ts_val_u64(self, 4) << 16));
> +
> +	*stamp = ns + ptp_clk_offset;
> +}
> +
> +static void hw_atl_b0_adj_params_get(u64 freq, s64 adj, u32 *ns, u32 *fns)
> +{
> +	/* For accuracy, the digit is extended */
> +	s64 divisor = 0, base_ns = ((adj + 1000000000ll) * 1000000000ll) / freq;
> +	u32 nsi_frac = 0, nsi = base_ns / 1000000000ll;
> +
> +	if (base_ns != nsi * 1000000000ll) {
> +		divisor = 1000000000000000000ll /
> +			  (base_ns - nsi * 1000000000ll);
> +		nsi_frac = 0x100000000ll * 1000000000ll / divisor;
> +	}
> +
> +	*ns = nsi;
> +	*fns = nsi_frac;
> +}
> +

> +static int hw_atl_b0_adj_sys_clock(struct aq_hw_s *self, s64 delta)
> +{
> +	ptp_clk_offset += delta;
> +
> +	return 0;
> +}

Does this work when i have a box with 42 NICs in it? Does not each NIC
need its own clock offset? Just seeing code like this causes alarm
bells. So if it is correct, i would expect some sort of comment to
prevent those alarm bells.

      Andrew






> +
> +static int hw_atl_b0_set_sys_clock(struct aq_hw_s *self, u64 time, u64 ts)
> +{
> +	s64 delta = time - (ptp_clk_offset + ts);
> +
> +	return hw_atl_b0_adj_sys_clock(self, delta);
> +}
> +
> +static int hw_atl_b0_adj_clock_freq(struct aq_hw_s *self, s32 ppb)
> +{
> +	struct hw_fw_request_iface fwreq;
> +	size_t size;
> +
> +	memset(&fwreq, 0, sizeof(fwreq));
> +
> +	fwreq.msg_id = HW_AQ_FW_REQUEST_PTP_ADJ_FREQ;
> +	hw_atl_b0_adj_params_get(AQ_HW_MAC_COUNTER_HZ, ppb,
> +				 &fwreq.ptp_adj_freq.ns_mac,
> +				 &fwreq.ptp_adj_freq.fns_mac);
> +	hw_atl_b0_adj_params_get(AQ_HW_PHY_COUNTER_HZ, ppb,
> +				 &fwreq.ptp_adj_freq.ns_phy,
> +				 &fwreq.ptp_adj_freq.fns_phy);
> +	hw_atl_b0_mac_adj_param_calc(&fwreq.ptp_adj_freq,
> +				     AQ_HW_PHY_COUNTER_HZ,
> +				     AQ_HW_MAC_COUNTER_HZ);
> +
> +	size = sizeof(fwreq.msg_id) + sizeof(fwreq.ptp_adj_freq);
> +	return self->aq_fw_ops->send_fw_request(self, &fwreq, size);
> +}
> +
>  static int hw_atl_b0_hw_fl3l4_clear(struct aq_hw_s *self,
>  				    struct aq_rx_filter_l3l4 *data)
>  {
> @@ -1164,6 +1256,12 @@ const struct aq_hw_ops hw_atl_ops_b0 = {
>  	.hw_get_regs                 = hw_atl_utils_hw_get_regs,
>  	.hw_get_hw_stats             = hw_atl_utils_get_hw_stats,
>  	.hw_get_fw_version           = hw_atl_utils_get_fw_version,
> -	.hw_set_offload              = hw_atl_b0_hw_offload_set,
> +
> +	.hw_get_ptp_ts           = hw_atl_b0_get_ptp_ts,
> +	.hw_adj_sys_clock        = hw_atl_b0_adj_sys_clock,
> +	.hw_set_sys_clock        = hw_atl_b0_set_sys_clock,
> +	.hw_adj_clock_freq       = hw_atl_b0_adj_clock_freq,
> +
> +	.hw_set_offload          = hw_atl_b0_hw_offload_set,
>  	.hw_set_fc                   = hw_atl_b0_set_fc,
>  };
> diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_llh.c b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_llh.c
> index 1149812ae463..25e7261f6a44 100644
> --- a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_llh.c
> +++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_llh.c
> @@ -1,7 +1,7 @@
>  // SPDX-License-Identifier: GPL-2.0-only
>  /*
>   * aQuantia Corporation Network Driver
> - * Copyright (C) 2014-2017 aQuantia Corporation. All rights reserved
> + * Copyright (C) 2014-2019 aQuantia Corporation. All rights reserved
>   */
>  
>  /* File hw_atl_llh.c: Definitions of bitfield and register access functions for
> @@ -1513,6 +1513,20 @@ void hw_atl_reg_glb_cpu_scratch_scp_set(struct aq_hw_s *aq_hw,
>  			glb_cpu_scratch_scp);
>  }
>  
> +void hw_atl_pcs_ptp_clock_read_enable(struct aq_hw_s *aq_hw,
> +				      u32 ptp_clock_read_enable)
> +{
> +	aq_hw_write_reg_bit(aq_hw, HW_ATL_PCS_PTP_CLOCK_READ_ENABLE_ADR,
> +			    HW_ATL_PCS_PTP_CLOCK_READ_ENABLE_MSK,
> +			    HW_ATL_PCS_PTP_CLOCK_READ_ENABLE_SHIFT,
> +			    ptp_clock_read_enable);
> +}
> +
> +u32 hw_atl_pcs_ptp_clock_get(struct aq_hw_s *aq_hw, u32 index)
> +{
> +	return aq_hw_read_reg(aq_hw, HW_ATL_PCS_PTP_TS_VAL_ADDR(index));
> +}
> +
>  void hw_atl_mcp_up_force_intr_set(struct aq_hw_s *aq_hw, u32 up_force_intr)
>  {
>  	aq_hw_write_reg_bit(aq_hw, HW_ATL_MCP_UP_FORCE_INTERRUPT_ADR,
> diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_llh.h b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_llh.h
> index 0c37abbabca5..a62693e51a6b 100644
> --- a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_llh.h
> +++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_llh.h
> @@ -1,7 +1,7 @@
>  /* SPDX-License-Identifier: GPL-2.0-only */
>  /*
>   * aQuantia Corporation Network Driver
> - * Copyright (C) 2014-2017 aQuantia Corporation. All rights reserved
> + * Copyright (C) 2014-2019 aQuantia Corporation. All rights reserved
>   */
>  
>  /* File hw_atl_llh.h: Declarations of bitfield and register access functions for
> @@ -712,6 +712,12 @@ void hw_atl_msm_reg_wr_strobe_set(struct aq_hw_s *aq_hw, u32 reg_wr_strobe);
>  /* set pci register reset disable */
>  void hw_atl_pci_pci_reg_res_dis_set(struct aq_hw_s *aq_hw, u32 pci_reg_res_dis);
>  
> +/* pcs */
> +void hw_atl_pcs_ptp_clock_read_enable(struct aq_hw_s *aq_hw,
> +				      u32 ptp_clock_read_enable);
> +
> +u32 hw_atl_pcs_ptp_clock_get(struct aq_hw_s *aq_hw, u32 index);
> +
>  /* set uP Force Interrupt */
>  void hw_atl_mcp_up_force_intr_set(struct aq_hw_s *aq_hw, u32 up_force_intr);
>  
> diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_llh_internal.h b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_llh_internal.h
> index c3febcdfa92e..7716e0fc22b5 100644
> --- a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_llh_internal.h
> +++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_llh_internal.h
> @@ -1,7 +1,7 @@
>  /* SPDX-License-Identifier: GPL-2.0-only */
>  /*
>   * aQuantia Corporation Network Driver
> - * Copyright (C) 2014-2017 aQuantia Corporation. All rights reserved
> + * Copyright (C) 2014-2019 aQuantia Corporation. All rights reserved
>   */
>  
>  /* File hw_atl_llh_internal.h: Preprocessor definitions
> @@ -2421,6 +2421,22 @@
>  /* default value of bitfield register write strobe */
>  #define HW_ATL_MSM_REG_WR_STROBE_DEFAULT 0x0
>  
> +/* register address for bitfield PTP Digital Clock Read Enable */
> +#define HW_ATL_PCS_PTP_CLOCK_READ_ENABLE_ADR 0x00004628
> +/* bitmask for bitfield PTP Digital Clock Read Enable */
> +#define HW_ATL_PCS_PTP_CLOCK_READ_ENABLE_MSK 0x00000010
> +/* inverted bitmask for bitfield PTP Digital Clock Read Enable */
> +#define HW_ATL_PCS_PTP_CLOCK_READ_ENABLE_MSKN 0xFFFFFFEF
> +/* lower bit position of bitfield PTP Digital Clock Read Enable */
> +#define HW_ATL_PCS_PTP_CLOCK_READ_ENABLE_SHIFT 4
> +/* width of bitfield PTP Digital Clock Read Enable */
> +#define HW_ATL_PCS_PTP_CLOCK_READ_ENABLE_WIDTH 1
> +/* default value of bitfield PTP Digital Clock Read Enable */
> +#define HW_ATL_PCS_PTP_CLOCK_READ_ENABLE_DEFAULT 0x0
> +
> +/* register address for ptp counter reading */
> +#define HW_ATL_PCS_PTP_TS_VAL_ADDR(index) (0x00004900 + (index) * 0x4)
> +
>  /* mif soft reset bitfield definitions
>   * preprocessor definitions for the bitfield "soft reset".
>   * port="pif_glb_res_i"
> diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.c b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.c
> index 32512539ae86..6fc5640065bd 100644
> --- a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.c
> +++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.c
> @@ -327,8 +327,7 @@ int hw_atl_utils_fw_downld_dwords(struct aq_hw_s *self, u32 a,
>  	return err;
>  }
>  
> -static int hw_atl_utils_fw_upload_dwords(struct aq_hw_s *self, u32 a, u32 *p,
> -					 u32 cnt)
> +int hw_atl_utils_fw_upload_dwords(struct aq_hw_s *self, u32 a, u32 *p, u32 cnt)
>  {
>  	u32 val;
>  	int err = 0;
> @@ -964,4 +963,6 @@ const struct aq_fw_ops aq_fw_1x_ops = {
>  	.set_eee_rate = NULL,
>  	.get_eee_rate = NULL,
>  	.set_flow_control = NULL,
> +	.send_fw_request = NULL,
> +	.enable_ptp = NULL,
>  };
> diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.h b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.h
> index 766e02c7fd4e..f2eb94f298e2 100644
> --- a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.h
> +++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.h
> @@ -279,6 +279,34 @@ struct __packed offload_info {
>  	u8 buf[0];
>  };
>  
> +/* Mailbox FW Request interface */
> +struct __packed hw_fw_request_ptp_adj_freq {
> +	u32 ns_mac;
> +	u32 fns_mac;
> +	u32 ns_phy;
> +	u32 fns_phy;
> +	u32 mac_ns_adj;
> +	u32 mac_fns_adj;
> +};
> +
> +struct __packed hw_fw_request_ptp_adj_clock {
> +	u32 ns;
> +	u32 sec;
> +	int sign;
> +};
> +
> +#define HW_AQ_FW_REQUEST_PTP_ADJ_FREQ	         0x12
> +#define HW_AQ_FW_REQUEST_PTP_ADJ_CLOCK	         0x13
> +
> +struct __packed hw_fw_request_iface {
> +	u32 msg_id;
> +	union {
> +		/* PTP FW Request */
> +		struct hw_fw_request_ptp_adj_freq ptp_adj_freq;
> +		struct hw_fw_request_ptp_adj_clock ptp_adj_clock;
> +	};
> +};
> +
>  enum hw_atl_rx_action_with_traffic {
>  	HW_ATL_RX_DISCARD,
>  	HW_ATL_RX_HOST,
> @@ -561,6 +589,8 @@ struct aq_stats_s *hw_atl_utils_get_hw_stats(struct aq_hw_s *self);
>  int hw_atl_utils_fw_downld_dwords(struct aq_hw_s *self, u32 a,
>  				  u32 *p, u32 cnt);
>  
> +int hw_atl_utils_fw_upload_dwords(struct aq_hw_s *self, u32 a, u32 *p, u32 cnt);
> +
>  int hw_atl_utils_fw_set_wol(struct aq_hw_s *self, bool wol_enabled, u8 *mac);
>  
>  int hw_atl_utils_fw_rpc_call(struct aq_hw_s *self, unsigned int rpc_size);
> diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils_fw2x.c b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils_fw2x.c
> index da726489e3c8..8b9824b1dc5e 100644
> --- a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils_fw2x.c
> +++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils_fw2x.c
> @@ -1,7 +1,7 @@
>  // SPDX-License-Identifier: GPL-2.0-only
>  /*
>   * aQuantia Corporation Network Driver
> - * Copyright (C) 2014-2017 aQuantia Corporation. All rights reserved
> + * Copyright (C) 2014-2019 aQuantia Corporation. All rights reserved
>   */
>  
>  /* File hw_atl_utils_fw2x.c: Definition of firmware 2.x functions for
> @@ -17,14 +17,17 @@
>  #include "hw_atl_utils.h"
>  #include "hw_atl_llh.h"
>  
> -#define HW_ATL_FW2X_MPI_RPC_ADDR        0x334
> +#define HW_ATL_FW2X_MPI_RPC_ADDR         0x334
>  
> -#define HW_ATL_FW2X_MPI_MBOX_ADDR       0x360
> -#define HW_ATL_FW2X_MPI_EFUSE_ADDR	0x364
> -#define HW_ATL_FW2X_MPI_CONTROL_ADDR	0x368
> -#define HW_ATL_FW2X_MPI_CONTROL2_ADDR	0x36C
> -#define HW_ATL_FW2X_MPI_STATE_ADDR	0x370
> -#define HW_ATL_FW2X_MPI_STATE2_ADDR     0x374
> +#define HW_ATL_FW2X_MPI_MBOX_ADDR        0x360
> +#define HW_ATL_FW2X_MPI_EFUSE_ADDR       0x364
> +#define HW_ATL_FW2X_MPI_CONTROL_ADDR     0x368
> +#define HW_ATL_FW2X_MPI_CONTROL2_ADDR    0x36C
> +#define HW_ATL_FW2X_MPI_STATE_ADDR       0x370
> +#define HW_ATL_FW2X_MPI_STATE2_ADDR      0x374
> +
> +#define HW_ATL_FW3X_EXT_CONTROL_ADDR     0x378
> +#define HW_ATL_FW3X_EXT_STATE_ADDR       0x37c
>  
>  #define HW_ATL_FW2X_CAP_PAUSE            BIT(CAPS_HI_PAUSE)
>  #define HW_ATL_FW2X_CAP_ASYM_PAUSE       BIT(CAPS_HI_ASYMMETRIC_PAUSE)
> @@ -444,6 +447,54 @@ static int aq_fw2x_set_power(struct aq_hw_s *self, unsigned int power_state,
>  	return err;
>  }
>  
> +static int aq_fw2x_send_fw_request(struct aq_hw_s *self,
> +				   const struct hw_fw_request_iface *fw_req,
> +				   size_t size)
> +{
> +	u32 ctrl2, orig_ctrl2;
> +	u32 dword_cnt;
> +	int err = 0;
> +	u32 val;
> +
> +	/* Write data to drvIface Mailbox */
> +	dword_cnt = size / sizeof(u32);
> +	if (size % sizeof(u32))
> +		dword_cnt++;
> +	err = hw_atl_utils_fw_upload_dwords(self, aq_fw2x_rpc_get(self),
> +					    (void *)fw_req, dword_cnt);
> +	if (err < 0)
> +		goto err_exit;
> +
> +	/* Toggle statistics bit for FW to update */
> +	ctrl2 = aq_hw_read_reg(self, HW_ATL_FW2X_MPI_CONTROL2_ADDR);
> +	orig_ctrl2 = ctrl2 & BIT(CAPS_HI_FW_REQUEST);
> +	ctrl2 = ctrl2 ^ BIT(CAPS_HI_FW_REQUEST);
> +	aq_hw_write_reg(self, HW_ATL_FW2X_MPI_CONTROL2_ADDR, ctrl2);
> +
> +	/* Wait FW to report back */
> +	err = readx_poll_timeout_atomic(aq_fw2x_state2_get, self, val,
> +					orig_ctrl2 != (val &
> +						       BIT(CAPS_HI_FW_REQUEST)),
> +					1U, 10000U);
> +
> +err_exit:
> +	return err;
> +}
> +
> +static void aq_fw3x_enable_ptp(struct aq_hw_s *self, int enable)
> +{
> +	u32 ptp_opts = aq_hw_read_reg(self, HW_ATL_FW3X_EXT_STATE_ADDR);
> +	u32 all_ptp_features = BIT(CAPS_EX_PHY_PTP_EN) |
> +						   BIT(CAPS_EX_PTP_GPIO_EN);
> +
> +	if (enable)
> +		ptp_opts |= all_ptp_features;
> +	else
> +		ptp_opts &= ~all_ptp_features;
> +
> +	aq_hw_write_reg(self, HW_ATL_FW3X_EXT_CONTROL_ADDR, ptp_opts);
> +}
> +
>  static int aq_fw2x_set_eee_rate(struct aq_hw_s *self, u32 speed)
>  {
>  	u32 mpi_opts = aq_hw_read_reg(self, HW_ATL_FW2X_MPI_CONTROL2_ADDR);
> @@ -534,19 +585,21 @@ static u32 aq_fw2x_state2_get(struct aq_hw_s *self)
>  }
>  
>  const struct aq_fw_ops aq_fw_2x_ops = {
> -	.init = aq_fw2x_init,
> -	.deinit = aq_fw2x_deinit,
> -	.reset = NULL,
> -	.renegotiate = aq_fw2x_renegotiate,
> -	.get_mac_permanent = aq_fw2x_get_mac_permanent,
> -	.set_link_speed = aq_fw2x_set_link_speed,
> -	.set_state = aq_fw2x_set_state,
> +	.init               = aq_fw2x_init,
> +	.deinit             = aq_fw2x_deinit,
> +	.reset              = NULL,
> +	.renegotiate        = aq_fw2x_renegotiate,
> +	.get_mac_permanent  = aq_fw2x_get_mac_permanent,
> +	.set_link_speed     = aq_fw2x_set_link_speed,
> +	.set_state          = aq_fw2x_set_state,
>  	.update_link_status = aq_fw2x_update_link_status,
> -	.update_stats = aq_fw2x_update_stats,
> -	.get_phy_temp = aq_fw2x_get_phy_temp,
> -	.set_power = aq_fw2x_set_power,
> -	.set_eee_rate = aq_fw2x_set_eee_rate,
> -	.get_eee_rate = aq_fw2x_get_eee_rate,
> -	.set_flow_control = aq_fw2x_set_flow_control,
> -	.get_flow_control = aq_fw2x_get_flow_control
> +	.update_stats       = aq_fw2x_update_stats,
> +	.get_phy_temp       = aq_fw2x_get_phy_temp,
> +	.set_power          = aq_fw2x_set_power,
> +	.set_eee_rate       = aq_fw2x_set_eee_rate,
> +	.get_eee_rate       = aq_fw2x_get_eee_rate,
> +	.set_flow_control   = aq_fw2x_set_flow_control,
> +	.get_flow_control   = aq_fw2x_get_flow_control,
> +	.send_fw_request    = aq_fw2x_send_fw_request,
> +	.enable_ptp         = aq_fw3x_enable_ptp,
>  };
> -- 
> 2.17.1
> 

^ permalink raw reply

* Re: INFO: rcu detected stall in br_handle_frame
From: Cong Wang @ 2019-09-10 19:20 UTC (permalink / raw)
  To: syzbot
  Cc: David Miller, Jamal Hadi Salim, Jiri Pirko, LKML,
	Linux Kernel Network Developers, syzkaller-bugs
In-Reply-To: <000000000000bcb83d059237c30a@google.com>

#syz fix: sch_hhf: ensure quantum and hhf_non_hh_weight are non-zero

^ permalink raw reply

* Re: INFO: rcu detected stall in addrconf_dad_work
From: Cong Wang @ 2019-09-10 19:20 UTC (permalink / raw)
  To: syzbot
  Cc: David Miller, Jamal Hadi Salim, Jiri Pirko, LKML,
	Linux Kernel Network Developers, syzkaller-bugs
In-Reply-To: <000000000000c1391b059237c3a6@google.com>

#syz fix: sch_hhf: ensure quantum and hhf_non_hh_weight are non-zero

^ permalink raw reply

* INFO: rcu detected stall in br_handle_frame
From: syzbot @ 2019-09-10 19:18 UTC (permalink / raw)
  To: davem, jhs, jiri, linux-kernel, netdev, syzkaller-bugs,
	xiyou.wangcong

Hello,

syzbot found the following crash on:

HEAD commit:    3b47fd5c Merge tag 'nfs-for-5.3-4' of git://git.linux-nfs...
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=12983001600000
kernel config:  https://syzkaller.appspot.com/x/.config?x=b89bb446a3faaba4
dashboard link: https://syzkaller.appspot.com/bug?extid=2addc1f058bd021b0a40
compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=17d8afae600000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=1253d1fa600000

Bisection is inconclusive: the bug happens on the oldest tested release.

bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=109dcfc6600000
console output: https://syzkaller.appspot.com/x/log.txt?x=149dcfc6600000

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+2addc1f058bd021b0a40@syzkaller.appspotmail.com

rcu: INFO: rcu_preempt self-detected stall on CPU
rcu: 	0-...!: (10500 ticks this GP) idle=c4e/1/0x4000000000000002  
softirq=10445/10445 fqs=0
	(t=10500 jiffies g=10669 q=248)
rcu: rcu_preempt kthread starved for 10500 jiffies! g10669 f0x0  
RCU_GP_WAIT_FQS(5) ->state=0x402 ->cpu=0
rcu: RCU grace-period kthread stack dump:
rcu_preempt     I29392    10      2 0x80004000
Call Trace:
  context_switch kernel/sched/core.c:3254 [inline]
  __schedule+0x755/0x1580 kernel/sched/core.c:3880
  schedule+0xd9/0x260 kernel/sched/core.c:3947
  schedule_timeout+0x486/0xc50 kernel/time/timer.c:1807
  rcu_gp_fqs_loop kernel/rcu/tree.c:1611 [inline]
  rcu_gp_kthread+0x9b2/0x18c0 kernel/rcu/tree.c:1768
  kthread+0x361/0x430 kernel/kthread.c:255
  ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352
NMI backtrace for cpu 0
CPU: 0 PID: 9 Comm: ksoftirqd/0 Not tainted 5.3.0-rc7+ #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
Call Trace:
  <IRQ>
  __dump_stack lib/dump_stack.c:77 [inline]
  dump_stack+0x172/0x1f0 lib/dump_stack.c:113
  nmi_cpu_backtrace.cold+0x70/0xb2 lib/nmi_backtrace.c:101
  nmi_trigger_cpumask_backtrace+0x23b/0x28b lib/nmi_backtrace.c:62
  arch_trigger_cpumask_backtrace+0x14/0x20 arch/x86/kernel/apic/hw_nmi.c:38
  trigger_single_cpu_backtrace include/linux/nmi.h:164 [inline]
  rcu_dump_cpu_stacks+0x183/0x1cf kernel/rcu/tree_stall.h:254
  print_cpu_stall kernel/rcu/tree_stall.h:455 [inline]
  check_cpu_stall kernel/rcu/tree_stall.h:529 [inline]
  rcu_pending kernel/rcu/tree.c:2736 [inline]
  rcu_sched_clock_irq.cold+0x4dd/0xc13 kernel/rcu/tree.c:2183
  update_process_times+0x32/0x80 kernel/time/timer.c:1639
  tick_sched_handle+0xa2/0x190 kernel/time/tick-sched.c:167
  tick_sched_timer+0x53/0x140 kernel/time/tick-sched.c:1296
  __run_hrtimer kernel/time/hrtimer.c:1389 [inline]
  __hrtimer_run_queues+0x364/0xe40 kernel/time/hrtimer.c:1451
  hrtimer_interrupt+0x314/0x770 kernel/time/hrtimer.c:1509
  local_apic_timer_interrupt arch/x86/kernel/apic/apic.c:1106 [inline]
  smp_apic_timer_interrupt+0x160/0x610 arch/x86/kernel/apic/apic.c:1131
  apic_timer_interrupt+0xf/0x20 arch/x86/entry/entry_64.S:830
  </IRQ>
RIP: 0010:write_comp_data+0x68/0x70 kernel/kcov.c:147
Code: 00 00 4e 8d 14 dd 28 00 00 00 4d 39 d0 72 1b 49 83 c1 01 4a 89 7c 10  
e0 4a 89 74 10 e8 4a 89 54 10 f0 4a 89 4c d8 20 4c 89 08 <c3> 0f 1f 80 00  
00 00 00 55 40 0f b6 d6 40 0f b6 f7 31 ff 48 89 e5
RSP: 0018:ffff8880a98be990 EFLAGS: 00000206 ORIG_RAX: ffffffffffffff13
RAX: 0000000000000301 RBX: ffff88808c6eb0b8 RCX: ffffffff85c64b39
RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000005
RBP: ffff8880a98be998 R08: ffff8880a98b2240 R09: 0000000000000000
R10: fffffbfff134af8f R11: ffff8880a98b2240 R12: dffffc0000000000
R13: ffff88808c6eadc0 R14: ffff88808c6eb150 R15: 0000000000000000
  hhf_dequeue+0xb9/0xa20 net/sched/sch_hhf.c:434
  dequeue_skb net/sched/sch_generic.c:258 [inline]
  qdisc_restart net/sched/sch_generic.c:361 [inline]
  __qdisc_run+0x1e7/0x19d0 net/sched/sch_generic.c:379
  __dev_xmit_skb net/core/dev.c:3533 [inline]
  __dev_queue_xmit+0x16f1/0x3650 net/core/dev.c:3838
  dev_queue_xmit+0x18/0x20 net/core/dev.c:3902
  br_dev_queue_push_xmit+0x3f3/0x5c0 net/bridge/br_forward.c:52
  br_nf_dev_queue_xmit+0x34e/0x1470 net/bridge/br_netfilter_hooks.c:796
  NF_HOOK include/linux/netfilter.h:305 [inline]
  NF_HOOK include/linux/netfilter.h:299 [inline]
  br_nf_post_routing+0x1502/0x1d30 net/bridge/br_netfilter_hooks.c:844
  nf_hook_entry_hookfn include/linux/netfilter.h:135 [inline]
  nf_hook_slow+0xbc/0x1e0 net/netfilter/core.c:512
  nf_hook include/linux/netfilter.h:260 [inline]
  NF_HOOK include/linux/netfilter.h:303 [inline]
  br_forward_finish+0x215/0x400 net/bridge/br_forward.c:65
  br_nf_hook_thresh+0x2e9/0x370 net/bridge/br_netfilter_hooks.c:1015
  br_nf_forward_finish+0x66c/0xa90 net/bridge/br_netfilter_hooks.c:560
  NF_HOOK include/linux/netfilter.h:305 [inline]
  NF_HOOK include/linux/netfilter.h:299 [inline]
  br_nf_forward_ip net/bridge/br_netfilter_hooks.c:630 [inline]
  br_nf_forward_ip+0xc74/0x21e0 net/bridge/br_netfilter_hooks.c:571
  nf_hook_entry_hookfn include/linux/netfilter.h:135 [inline]
  nf_hook_slow+0xbc/0x1e0 net/netfilter/core.c:512
  nf_hook include/linux/netfilter.h:260 [inline]
  NF_HOOK include/linux/netfilter.h:303 [inline]
  __br_forward+0x393/0xb00 net/bridge/br_forward.c:109
  deliver_clone+0x61/0xc0 net/bridge/br_forward.c:125
  br_flood+0x325/0x3d0 net/bridge/br_forward.c:232
  br_handle_frame_finish+0xb46/0x1670 net/bridge/br_input.c:162
  br_nf_hook_thresh+0x2e9/0x370 net/bridge/br_netfilter_hooks.c:1015
  br_nf_pre_routing_finish_ipv6+0x6fb/0xd80  
net/bridge/br_netfilter_ipv6.c:206
  NF_HOOK include/linux/netfilter.h:305 [inline]
  br_nf_pre_routing_ipv6+0x456/0x832 net/bridge/br_netfilter_ipv6.c:236
  br_nf_pre_routing+0x1743/0x2355 net/bridge/br_netfilter_hooks.c:501
  nf_hook_entry_hookfn include/linux/netfilter.h:135 [inline]
  nf_hook_bridge_pre net/bridge/br_input.c:223 [inline]
  br_handle_frame+0x806/0x133e net/bridge/br_input.c:348
  __netif_receive_skb_core+0xfc1/0x3060 net/core/dev.c:4905
  __netif_receive_skb_one_core+0xa8/0x1a0 net/core/dev.c:5002
  __netif_receive_skb+0x2c/0x1d0 net/core/dev.c:5118
  process_backlog+0x206/0x750 net/core/dev.c:5929
  napi_poll net/core/dev.c:6352 [inline]
  net_rx_action+0x4d6/0x1030 net/core/dev.c:6418
  __do_softirq+0x262/0x98c kernel/softirq.c:292
  run_ksoftirqd kernel/softirq.c:603 [inline]
  run_ksoftirqd+0x8e/0x110 kernel/softirq.c:595
  smpboot_thread_fn+0x6a3/0xa40 kernel/smpboot.c:165
  kthread+0x361/0x430 kernel/kthread.c:255
  ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352


---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
For information about bisection process see: https://goo.gl/tpsmEJ#bisection
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches

^ permalink raw reply

* INFO: rcu detected stall in addrconf_dad_work
From: syzbot @ 2019-09-10 19:18 UTC (permalink / raw)
  To: davem, jhs, jiri, linux-kernel, netdev, syzkaller-bugs,
	xiyou.wangcong

Hello,

syzbot found the following crash on:

HEAD commit:    3b47fd5c Merge tag 'nfs-for-5.3-4' of git://git.linux-nfs...
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=126c0f51600000
kernel config:  https://syzkaller.appspot.com/x/.config?x=b89bb446a3faaba4
dashboard link: https://syzkaller.appspot.com/bug?extid=0055e43d6f67fb9b8ba1
compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=1500264e600000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=16d7a46e600000

Bisection is inconclusive: the bug happens on the oldest tested release.

bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=160261e1600000
console output: https://syzkaller.appspot.com/x/log.txt?x=110261e1600000

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+0055e43d6f67fb9b8ba1@syzkaller.appspotmail.com

rcu: INFO: rcu_preempt self-detected stall on CPU
rcu: 	1-...!: (10499 ticks this GP) idle=e02/1/0x4000000000000002  
softirq=9705/9705 fqs=4
	(t=10500 jiffies g=9309 q=84)
rcu: rcu_preempt kthread starved for 10492 jiffies! g9309 f0x0  
RCU_GP_WAIT_FQS(5) ->state=0x0 ->cpu=0
rcu: RCU grace-period kthread stack dump:
rcu_preempt     R  running task    29112    10      2 0x80004000
Call Trace:
  context_switch kernel/sched/core.c:3254 [inline]
  __schedule+0x755/0x1580 kernel/sched/core.c:3880
  schedule+0xd9/0x260 kernel/sched/core.c:3947
  schedule_timeout+0x486/0xc50 kernel/time/timer.c:1807
  rcu_gp_fqs_loop kernel/rcu/tree.c:1611 [inline]
  rcu_gp_kthread+0x9b2/0x18c0 kernel/rcu/tree.c:1768
  kthread+0x361/0x430 kernel/kthread.c:255
  ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352
NMI backtrace for cpu 1
CPU: 1 PID: 3027 Comm: kworker/1:2 Not tainted 5.3.0-rc7+ #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
Workqueue: ipv6_addrconf addrconf_dad_work
Call Trace:
  <IRQ>
  __dump_stack lib/dump_stack.c:77 [inline]
  dump_stack+0x172/0x1f0 lib/dump_stack.c:113
  nmi_cpu_backtrace.cold+0x70/0xb2 lib/nmi_backtrace.c:101
  nmi_trigger_cpumask_backtrace+0x23b/0x28b lib/nmi_backtrace.c:62
  arch_trigger_cpumask_backtrace+0x14/0x20 arch/x86/kernel/apic/hw_nmi.c:38
  trigger_single_cpu_backtrace include/linux/nmi.h:164 [inline]
  rcu_dump_cpu_stacks+0x183/0x1cf kernel/rcu/tree_stall.h:254
  print_cpu_stall kernel/rcu/tree_stall.h:455 [inline]
  check_cpu_stall kernel/rcu/tree_stall.h:529 [inline]
  rcu_pending kernel/rcu/tree.c:2736 [inline]
  rcu_sched_clock_irq.cold+0x4dd/0xc13 kernel/rcu/tree.c:2183
  update_process_times+0x32/0x80 kernel/time/timer.c:1639
  tick_sched_handle+0xa2/0x190 kernel/time/tick-sched.c:167
  tick_sched_timer+0x53/0x140 kernel/time/tick-sched.c:1296
  __run_hrtimer kernel/time/hrtimer.c:1389 [inline]
  __hrtimer_run_queues+0x364/0xe40 kernel/time/hrtimer.c:1451
  hrtimer_interrupt+0x314/0x770 kernel/time/hrtimer.c:1509
  local_apic_timer_interrupt arch/x86/kernel/apic/apic.c:1106 [inline]
  smp_apic_timer_interrupt+0x160/0x610 arch/x86/kernel/apic/apic.c:1131
  apic_timer_interrupt+0xf/0x20 arch/x86/entry/entry_64.S:830
  </IRQ>
RIP: 0010:__list_del_entry_valid+0x4b/0xf5 lib/list_debug.c:45
Code: 54 80 3c 02 00 0f 85 a1 00 00 00 4c 89 f2 4d 8b 66 08 48 b8 00 00 00  
00 00 fc ff df 48 c1 ea 03 80 3c 02 00 0f 85 9d 00 00 00 <48> b8 00 01 00  
00 00 00 ad de 4d 8b 2e 49 39 c5 0f 84 e1 00 00 00
RSP: 0018:ffff88809ff26d30 EFLAGS: 00000246 ORIG_RAX: ffffffffffffff13
RAX: dffffc0000000000 RBX: ffff88808c560778 RCX: ffffffff85c64b39
RDX: 1ffff110118ac0ef RSI: ffffffff85c65006 RDI: ffff88808c560780
RBP: ffff88809ff26d48 R08: ffff88809ff08300 R09: 0000000000000000
R10: fffffbfff134af8f R11: ffff88809ff08300 R12: ffff88808c560810
R13: ffff88808c560480 R14: ffff88808c560778 R15: 0000000000000000
  __list_del_entry include/linux/list.h:131 [inline]
  list_move_tail include/linux/list.h:213 [inline]
  hhf_dequeue+0x5c5/0xa20 net/sched/sch_hhf.c:439
  dequeue_skb net/sched/sch_generic.c:258 [inline]
  qdisc_restart net/sched/sch_generic.c:361 [inline]
  __qdisc_run+0x1e7/0x19d0 net/sched/sch_generic.c:379
  __dev_xmit_skb net/core/dev.c:3533 [inline]
  __dev_queue_xmit+0x16f1/0x3650 net/core/dev.c:3838
  dev_queue_xmit+0x18/0x20 net/core/dev.c:3902
  br_dev_queue_push_xmit+0x3f3/0x5c0 net/bridge/br_forward.c:52
  NF_HOOK include/linux/netfilter.h:305 [inline]
  NF_HOOK include/linux/netfilter.h:299 [inline]
  br_forward_finish+0xfa/0x400 net/bridge/br_forward.c:65
  NF_HOOK include/linux/netfilter.h:305 [inline]
  NF_HOOK include/linux/netfilter.h:299 [inline]
  __br_forward+0x641/0xb00 net/bridge/br_forward.c:109
  deliver_clone+0x61/0xc0 net/bridge/br_forward.c:125
  maybe_deliver+0x2c7/0x390 net/bridge/br_forward.c:181
  br_flood+0x13a/0x3d0 net/bridge/br_forward.c:223
  br_dev_xmit+0x98c/0x15a0 net/bridge/br_device.c:100
  __netdev_start_xmit include/linux/netdevice.h:4406 [inline]
  netdev_start_xmit include/linux/netdevice.h:4420 [inline]
  xmit_one net/core/dev.c:3280 [inline]
  dev_hard_start_xmit+0x1a3/0x9c0 net/core/dev.c:3296
  __dev_queue_xmit+0x2b15/0x3650 net/core/dev.c:3869
  dev_queue_xmit+0x18/0x20 net/core/dev.c:3902
  neigh_resolve_output net/core/neighbour.c:1490 [inline]
  neigh_resolve_output+0x5a5/0x970 net/core/neighbour.c:1470
  neigh_output include/net/neighbour.h:511 [inline]
  ip6_finish_output2+0x1034/0x2520 net/ipv6/ip6_output.c:116
  __ip6_finish_output+0x444/0xa50 net/ipv6/ip6_output.c:142
  ip6_finish_output+0x38/0x1f0 net/ipv6/ip6_output.c:152
  NF_HOOK_COND include/linux/netfilter.h:294 [inline]
  ip6_output+0x235/0x7c0 net/ipv6/ip6_output.c:175
  dst_output include/net/dst.h:436 [inline]
  NF_HOOK include/linux/netfilter.h:305 [inline]
  ndisc_send_skb+0xf29/0x1450 net/ipv6/ndisc.c:504
  ndisc_send_ns+0x3a9/0x850 net/ipv6/ndisc.c:646
  addrconf_dad_work+0xb88/0x1150 net/ipv6/addrconf.c:4120
  process_one_work+0x9af/0x1740 kernel/workqueue.c:2269
  worker_thread+0x98/0xe40 kernel/workqueue.c:2415
  kthread+0x361/0x430 kernel/kthread.c:255
  ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352


---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
For information about bisection process see: https://goo.gl/tpsmEJ#bisection
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches

^ permalink raw reply

* Re: [PATCH] net/mlx5: reduce stack usage in FW tracer
From: Saeed Mahameed @ 2019-09-10 19:13 UTC (permalink / raw)
  To: David.Laight@ACULAB.COM, arnd@arndb.de
  Cc: cai@lca.pw, linux-rdma@vger.kernel.org, davem@davemloft.net,
	Moshe Shemesh, Feras Daoud, linux-kernel@vger.kernel.org,
	Eran Ben Elisha, netdev@vger.kernel.org, leon@kernel.org,
	Erez Shitrit
In-Reply-To: <d50f78334e64476bad033862035c734c@AcuMS.aculab.com>

On Tue, 2019-09-10 at 15:38 +0000, David Laight wrote:
> From: Arnd Bergmann
> > Sent: 10 September 2019 09:15
> ...
> > > I am not sure how this would work, since the format parameters
> > > can
> > > changes depending on the FW string and the specific traces.
> > 
> > Ah, so the format string comes from the firmware? I didn't look
> > at the code in enough detail to understand why it's done like this,
> > only enough to notice that it's rather unusual.
> 
> If the format string comes from the firmware you really shouldn't
> pass it to any standard printf function.
> You must ensure that it doesn't contain any format effectors
> that might dereference parameters.
> (The code might try to do that.)
> 
> Given that 'pointer' format effectors can't be used, the firmware
> must also supply the relevant integer ones?
> This should mean that all the processing is deferrable until the
> trace record is read.
> 

Point taken, i will discuss this with the team next week, since i am
traveling this week. and we will provide a proper solution. for now,
off the top of my head, FW strings and parameters are well defined and
very simple, many of the problems here do not apply, but will verify.

> 'noinline' just papers over the cracks.
> Especially since vasprintf() is likely to use a lot of stack.

Right i also tend to agree with noinline as a temporary solution until
we address all the points provided here.

> 
> 	David
> 
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes,
> MK1 1PT, UK
> Registration No: 1397386 (Wales)

^ permalink raw reply

* syzbot report in rsi_91x_usb.c
From: Julia Lawall @ 2019-09-10 19:11 UTC (permalink / raw)
  To: amitkarwar, siva8118
  Cc: syzkaller-bugs, kvalo, davem, linux-wireless, netdev,
	linux-kernel

Hello,

I spent some time looking at:

https://syzkaller.appspot.com/bug?id=c1b6aa968706d9380dcdb98a9f2c338071cc938c

which does not yet report that it has been fixed.

The problem seems to be in rsi_probe in
drivers/net/wireless/rsi/rsi_91x_usb.c.  This ends with the following
code:

        status = rsi_rx_urb_submit(adapter, WLAN_EP);
        if (status)
                goto err1;

        if (adapter->priv->coex_mode > 1) {
                status = rsi_rx_urb_submit(adapter, BT_EP);
                if (status)
                        goto err1;
        }

        return 0;
err1:
        rsi_deinit_usb_interface(adapter);
err:
        rsi_91x_deinit(adapter);


The problem seems to be that the first call to rsi_rx_urb_submit succeeds,
submitting an urb, and then theh second one fails.  Both share adapter,
which is used in rsi_rx_done_handler, invoked later as the callback
provided with usb_submit_urb.  But adapter, and in particular its rsi_dev
field, are freed by the rsi_91x_deinit call due to the failure of the
second call to rsi_rx_urb_submit.

How should this be fixed?

thanks,
julia

^ permalink raw reply

* Re: [PATCH net-next 01/11] net: aquantia: PTP skeleton declarations and callbacks
From: Andrew Lunn @ 2019-09-10 19:10 UTC (permalink / raw)
  To: Igor Russkikh
  Cc: netdev@vger.kernel.org, richardcochran@gmail.com,
	davem@davemloft.net, Egor Pomozov, Sergey Samoilenko,
	Dmitry Bezrukov
In-Reply-To: <cf60b1d3d797d0666a4828fcf5e521e0bd73f8d4.1568034880.git.igor.russkikh@aquantia.com>

On Mon, Sep 09, 2019 at 01:38:38PM +0000, Igor Russkikh wrote:
> From: Dmitry Bezrukov <dmitry.bezrukov@aquantia.com>

Hi Igor, et al.

> @@ -331,6 +332,10 @@ int aq_nic_init(struct aq_nic_s *self)
>  		self->aq_vecs > i; ++i, aq_vec = self->aq_vec[i])
>  		aq_vec_init(aq_vec, self->aq_hw_ops, self->aq_hw);

> +int aq_ptp_init(struct aq_nic_s *aq_nic, unsigned int idx_vec)
> +{
> +	struct hw_atl_utils_mbox mbox;
> +	struct ptp_clock *clock;
> +	struct aq_ptp_s *self;

I find the use of self in this code quite confusing. It does not
appear to have a clear meaning. It can be a aq_ring_s, aq_nic_c,
aq_hw_s, aq_vec_s.

Looking at this code i always have to figure out what self is. Could
you not just use struct aq_ptp_s aq_ptp consistently in the code?

> +	int err = 0;
> +
> +	hw_atl_utils_mpi_read_stats(aq_nic->aq_hw, &mbox);
> +
> +	if (!(mbox.info.caps_ex & BIT(CAPS_EX_PHY_PTP_EN))) {
> +		aq_nic->aq_ptp = NULL;
> +		return 0;
> +	}
> +
> +	self = kzalloc(sizeof(*self), GFP_KERNEL);

Using devm_kzalloc() will make your clean up easier.

> +void aq_ptp_free(struct aq_nic_s *aq_nic)
> +{
> +	struct aq_ptp_s *self = aq_nic->aq_ptp;
> +
> +	if (!self)
> +		return;
> +
> +	kfree(self);

kfree() is happy to take a NULL pointer. But this could all go away
with devm_kzalloc().

     Andrew

^ permalink raw reply

* [PATCH 2/3] rtlwifi: rtl8192cu: replace _rtl92c_evm_db_to_percentage with generic version
From: Michael Straube @ 2019-09-10 19:04 UTC (permalink / raw)
  To: kvalo; +Cc: pkshih, davem, linux-wireless, netdev, linux-kernel,
	Michael Straube
In-Reply-To: <20190910190422.63378-1-straube.linux@gmail.com>

Function _rtl92c_evm_db_to_percentage is functionally identical
to the generic version rtl_evm_db_to_percentage, so remove
_rtl92c_evm_db_to_percentage and use the generic version instead.

Signed-off-by: Michael Straube <straube.linux@gmail.com>
---
 .../wireless/realtek/rtlwifi/rtl8192cu/mac.c   | 18 +-----------------
 1 file changed, 1 insertion(+), 17 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8192cu/mac.c b/drivers/net/wireless/realtek/rtlwifi/rtl8192cu/mac.c
index c8daad1e749f..cec19b32c7e2 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8192cu/mac.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192cu/mac.c
@@ -577,22 +577,6 @@ static u8 _rtl92c_query_rxpwrpercentage(s8 antpower)
 		return 100 + antpower;
 }
 
-static u8 _rtl92c_evm_db_to_percentage(s8 value)
-{
-	s8 ret_val;
-
-	ret_val = value;
-	if (ret_val >= 0)
-		ret_val = 0;
-	if (ret_val <= -33)
-		ret_val = -33;
-	ret_val = 0 - ret_val;
-	ret_val *= 3;
-	if (ret_val == 99)
-		ret_val = 100;
-	return ret_val;
-}
-
 static long _rtl92c_signal_scale_mapping(struct ieee80211_hw *hw,
 		long currsig)
 {
@@ -743,7 +727,7 @@ static void _rtl92c_query_rxphystatus(struct ieee80211_hw *hw,
 		else
 			max_spatial_stream = 1;
 		for (i = 0; i < max_spatial_stream; i++) {
-			evm = _rtl92c_evm_db_to_percentage(p_drvinfo->rxevm[i]);
+			evm = rtl_evm_db_to_percentage(p_drvinfo->rxevm[i]);
 			if (packet_match_bssid) {
 				if (i == 0)
 					pstats->signalquality =
-- 
2.23.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