* [PATCH] qed: fix spelling mistake: "taskelt" -> "tasklet"
From: Colin King @ 2018-05-10 14:03 UTC (permalink / raw)
To: Ariel Elior, everest-linux-l2, David S . Miller, netdev
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
Trivial fix to spelling mistake in DP_VERBOSE message text
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/net/ethernet/qlogic/qed/qed_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/qlogic/qed/qed_main.c b/drivers/net/ethernet/qlogic/qed/qed_main.c
index 70bc5634675c..9feed3b79cd4 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_main.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_main.c
@@ -680,7 +680,7 @@ static int qed_nic_stop(struct qed_dev *cdev)
tasklet_disable(p_hwfn->sp_dpc);
p_hwfn->b_sp_dpc_enabled = false;
DP_VERBOSE(cdev, NETIF_MSG_IFDOWN,
- "Disabled sp taskelt [hwfn %d] at %p\n",
+ "Disabled sp tasklet [hwfn %d] at %p\n",
i, p_hwfn->sp_dpc);
}
}
--
2.17.0
^ permalink raw reply related
* Re: i.MX6S/DL and QCA8334 switch using DSA driver - CPU port not working
From: Michal Vokáč @ 2018-05-10 13:49 UTC (permalink / raw)
To: Andrew Lunn; +Cc: netdev, Vivien Didelot, Florian Fainelli
In-Reply-To: <20180504133014.GB9666@lunn.ch>
On 4.5.2018 15:30, Andrew Lunn wrote:
>> I am out of ideas how to further debug this.
>> Any additional adivce will be much appreciated.
>
> I would suggest looking at the statistics counters. ethtool -S. Try
> it for both the slave interfaces, and the master interface. The master
> interfaces statistics should have both the host counters, and the
> switches counters. Do you see packets being sent but not received? Are
> there errors reported?
Thanks Andrew, now I am getting closer!
I briefly looked at the statistics some time ago but could not interpret
the numbers correctly. Now I went through the relevant source code and it
shed some light to it.
This is what I get from the numbers:
Slaves - switch counters: some packets received, none transmitted. No errors.
Slaves - DSA counters: some packets transmitted, none received. No error cntrs.
Master - switch counters: some packets transmitted, none received. No errors.
Master - host/fec counters: some packets transmitted, none received. No errors,
but IEEE_rx_drop counter shows frames are being dropped!
So this is most probably the issue. From the i.MX6 RM:
"Counter increments if a frame with invalid or missing SFD character is
detected and has been dropped. None of the other counters increments if
this counter increments."
At first I thought that this could be caused by the atheros header.
But the atheros header is inserted in the frame by the switch after the SA.
So it can not be the issue as the frame is discarded even before the DA and SA
is processed.
So both the CPU and the switch are trying to talk to each other but their
frames are not delivered. This still looks like RGMII configuration problem.
As it works with my old PHY driver I suspect the problem is at the switches
side somewhere in qca8k_set_pad_ctrl or qca8k_setup in the qca8k driver.
Any other ideas?
BR, Michal
^ permalink raw reply
* Re: [PATCH] ipv6: remove min MTU check for tunnel destinations
From: Ashwanth Goli @ 2018-05-10 13:47 UTC (permalink / raw)
To: David Miller; +Cc: netdev, pabeni, dsahern, netdev-owner
In-Reply-To: <20180501.122223.1661453536072731491.davem@davemloft.net>
On 2018-05-01 21:52, David Miller wrote:
> From: Ashwanth Goli <ashwanth@codeaurora.org>
> Date: Mon, 30 Apr 2018 00:36:47 +0530
>
>> With 749439bfac "fix udpv6 sendmsg crash caused by too small MTU"
>> tunnel dst's that report a MTU less than IPV6_MIN_MTU are broken
>> even for packets that are smaller than IPV6_MIN_MTU.
>>
>> According to rfc2473#section-7.1
>>
>> if the original IPv6 packet is equal or smaller than the
>> IPv6 minimum link MTU, the tunnel entry-point node
>> encapsulates the original packet, and subsequently
>> fragments the resulting IPv6 tunnel packet into IPv6
>> fragments that do not exceed the Path MTU to the tunnel
>> exit-point.
>>
>> This patch drops the MTU check for tunnel destinations.
>>
>> Signed-off-by: Ashwanth Goli <ashwanth@codeaurora.org>
>
> RFC 2473 is generally about ipv6 tunnels....
>
>> - if (mtu < IPV6_MIN_MTU)
>> + if (!(rt->dst.flags & DST_XFRM_TUNNEL) && mtu < IPV6_MIN_MTU)
>> return -EINVAL;
>
> But the check you are adding is specifically checking only IPSEC
> tunnels.
>
> If what you say is true in your commit message, this test must
> more generally trigger for all ipv6 tunnel types, not just IPSEC
> ones.
>
> If IPSEC tunnels are being targetting in this patch intentionally,
> that needs to be explained in the commit message.
My intention is to fix the issue for IPSEC tunnels. Will resend by
changing the commit text.
^ permalink raw reply
* Re: [PATCH] net/mlx4_en: Fix an error handling path in 'mlx4_en_init_netdev()'
From: Yuval Shaia @ 2018-05-10 13:38 UTC (permalink / raw)
To: Christophe JAILLET
Cc: davem, tariqt, netdev, linux-rdma, linux-kernel, kernel-janitors
In-Reply-To: <20180510070226.19575-1-christophe.jaillet@wanadoo.fr>
On Thu, May 10, 2018 at 09:02:26AM +0200, Christophe JAILLET wrote:
> If an error occurs, 'mlx4_en_destroy_netdev()' is called.
> It then calls 'mlx4_en_free_resources()' which does the needed resources
> cleanup.
>
> So, doing some explicit kfree in the error handling path would lead to
> some double kfree.
Patch make sense but what's bothering me is that mlx4_en_free_resources
loops on the entire array, assuming !priv->tx_ring[t] means entry is
allocated but the existing code does not assume that, see [1]. So i looked
to see where tx_ring array is zeroed and didn't find it.
Am i missing something here.
>
> Simplify code to avoid such a case.
>
> Fixes: 67f8b1dcb9ee ("net/mlx4_en: Refactor the XDP forwarding rings scheme")
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
> drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 8 +-------
> 1 file changed, 1 insertion(+), 7 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> index e0adac4a9a19..9670b33fc9b1 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> @@ -3324,12 +3324,11 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int port,
> MAX_TX_RINGS, GFP_KERNEL);
> if (!priv->tx_ring[t]) {
> err = -ENOMEM;
> - goto err_free_tx;
> + goto out;
> }
> priv->tx_cq[t] = kzalloc(sizeof(struct mlx4_en_cq *) *
> MAX_TX_RINGS, GFP_KERNEL);
> if (!priv->tx_cq[t]) {
> - kfree(priv->tx_ring[t]);
> err = -ENOMEM;
> goto out;
> }
> @@ -3582,11 +3581,6 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int port,
>
> return 0;
>
> -err_free_tx:
> - while (t--) {
[1]
> - kfree(priv->tx_ring[t]);
> - kfree(priv->tx_cq[t]);
> - }
> out:
> mlx4_en_destroy_netdev(dev);
> return err;
> --
> 2.17.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH net-next] rocker: Postpone filtering of !added_by_user FDB
From: Petr Machata @ 2018-05-10 13:29 UTC (permalink / raw)
To: netdev; +Cc: jiri, davem, vivien.didelot
Breaking out of the switch in rocker_switchdev_event() still ends up
scheduling work, except an ill-defined one. This leads to an OOPS cited
below. Fix by postponing the check until rocker_switchdev_event_work().
[ 23.148476] BUG: unable to handle kernel NULL pointer dereference at 0000000000000000
[ 23.148810] PGD 0 P4D 0
[ 23.148982] Oops: 0000 [#1] PREEMPT SMP PTI
[ 23.149190] Modules linked in: bridge stp llc iptable_nat nf_nat_ipv4 nf_nat e1000 rocker
[ 23.149768] CPU: 0 PID: 239 Comm: kworker/u2:4 Not tainted 4.17.0-rc3-net_next_queue-custom #6
[ 23.150298] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-2.fc27 04/01/2014
[ 23.150868] Workqueue: rocker rocker_switchdev_event_work [rocker]
[ 23.151258] RIP: 0010:ofdpa_port_fdb+0x7b/0x230 [rocker]
[ 23.151597] RSP: 0018:ffffc900004b3e18 EFLAGS: 00010246
[ 23.151952] RAX: 00000000fffbc68c RBX: 0000000000000000 RCX: 0000000000000000
[ 23.152363] RDX: 0000000000000010 RSI: ffff88003b4471e0 RDI: 00000000ffffffff
[ 23.152768] RBP: ffff88003b4471c0 R08: ffff88003b4471e0 R09: ffff88003b4471c0
[ 23.153141] R10: 0000000000000000 R11: 0000000000000000 R12: ffff880036caf000
[ 23.153515] R13: 0000000000000000 R14: 0000000000000000 R15: ffff88003bc00000
[ 23.153919] FS: 0000000000000000(0000) GS:ffff88003fc00000(0000) knlGS:0000000000000000
[ 23.154444] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 23.154806] CR2: 0000000000000000 CR3: 0000000036eb6000 CR4: 00000000000006f0
[ 23.155194] Call Trace:
[ 23.155472] rocker_switchdev_event_work+0x9b/0xd0 [rocker]
[ 23.155850] ? __schedule+0x231/0x700
[ 23.156175] process_one_work+0x1cf/0x3e0
[ 23.156490] worker_thread+0x26/0x3d0
[ 23.156795] ? trace_event_raw_event_workqueue_execute_start+0x80/0x80
[ 23.157181] kthread+0x10e/0x130
[ 23.157485] ? kthread_create_worker_on_cpu+0x40/0x40
[ 23.157824] ret_from_fork+0x35/0x40
[ 23.158174] Code: 00 00 c1 e8 02 4c 8d 45 20 bf ff ff ff ff 83 e0 01 4c 89 65 20 88 45 14 48 8b 05 21 da 1f e2 4c 89 c6 4c 89 44 24 10 48 89 45 18 <41> 8b 45 00 89 45 28 41 0f b7 45 04 66 89 45 2c 0f b7 44 24 04
[ 23.159346] RIP: ofdpa_port_fdb+0x7b/0x230 [rocker] RSP: ffffc900004b3e18
[ 23.159742] CR2: 0000000000000000
[ 23.160088] ---[ end trace f9b16d4cb6df0629 ]---
Fixes: 816a3bed9549 ("switchdev: Add fdb.added_by_user to switchdev notifications")
Suggested-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Signed-off-by: Petr Machata <petrm@mellanox.com>
---
drivers/net/ethernet/rocker/rocker_main.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/rocker/rocker_main.c b/drivers/net/ethernet/rocker/rocker_main.c
index 152d694..e73e4fe 100644
--- a/drivers/net/ethernet/rocker/rocker_main.c
+++ b/drivers/net/ethernet/rocker/rocker_main.c
@@ -2738,6 +2738,8 @@ static void rocker_switchdev_event_work(struct work_struct *work)
switch (switchdev_work->event) {
case SWITCHDEV_FDB_ADD_TO_DEVICE:
fdb_info = &switchdev_work->fdb_info;
+ if (!fdb_info->added_by_user)
+ break;
err = rocker_world_port_fdb_add(rocker_port, fdb_info);
if (err) {
netdev_dbg(rocker_port->dev, "fdb add failed err=%d\n", err);
@@ -2747,6 +2749,8 @@ static void rocker_switchdev_event_work(struct work_struct *work)
break;
case SWITCHDEV_FDB_DEL_TO_DEVICE:
fdb_info = &switchdev_work->fdb_info;
+ if (!fdb_info->added_by_user)
+ break;
err = rocker_world_port_fdb_del(rocker_port, fdb_info);
if (err)
netdev_dbg(rocker_port->dev, "fdb add failed err=%d\n", err);
@@ -2783,8 +2787,6 @@ static int rocker_switchdev_event(struct notifier_block *unused,
switch (event) {
case SWITCHDEV_FDB_ADD_TO_DEVICE: /* fall through */
case SWITCHDEV_FDB_DEL_TO_DEVICE:
- if (!fdb_info->added_by_user)
- break;
memcpy(&switchdev_work->fdb_info, ptr,
sizeof(switchdev_work->fdb_info));
switchdev_work->fdb_info.addr = kzalloc(ETH_ALEN, GFP_ATOMIC);
--
2.4.11
^ permalink raw reply related
* [PATCH net-next] tls: Fix tls_device initialization
From: Boris Pismenny @ 2018-05-10 13:27 UTC (permalink / raw)
To: davem; +Cc: netdev, borisp, davejwatson
Add sg table initialization to fix a BUG_ON encountered when enabling
CONFIG_DEBUG_SG.
Signed-off-by: Boris Pismenny <borisp@mellanox.com>
---
net/tls/tls_device.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/tls/tls_device.c b/net/tls/tls_device.c
index ac45d62..a7a8f8e 100644
--- a/net/tls/tls_device.c
+++ b/net/tls/tls_device.c
@@ -604,6 +604,8 @@ int tls_set_device_offload(struct sock *sk, struct tls_context *ctx)
INIT_LIST_HEAD(&offload_ctx->records_list);
list_add_tail(&start_marker_record->list, &offload_ctx->records_list);
spin_lock_init(&offload_ctx->lock);
+ sg_init_table(offload_ctx->sg_tx_data,
+ ARRAY_SIZE(offload_ctx->sg_tx_data));
clean_acked_data_enable(inet_csk(sk), &tls_icsk_clean_acked);
ctx->push_pending_record = tls_device_push_pending_record;
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH net-next v2] tcp: Add mark for TIMEWAIT sockets
From: Eric Dumazet @ 2018-05-10 13:27 UTC (permalink / raw)
To: Jon Maxwell, davem; +Cc: kuznet, yoshfuji, netdev, linux-kernel, jmaxwell
In-Reply-To: <20180510065351.22535-1-jmaxwell37@gmail.com>
On 05/09/2018 11:53 PM, Jon Maxwell wrote:
> This version has some suggestions by Eric Dumazet:
>
> - Use a local variable for the mark in IPv6 instead of ctl_sk to avoid SMP
> races.
> - Use the more elegant "IP4_REPLY_MARK(net, skb->mark) ?: sk->sk_mark"
> statement.
> - Factorize code as sk_fullsock() check is not necessary.
>
> Aidan McGurn from Openwave Mobility systems reported the following bug:
>
> "Marked routing is broken on customer deployment. Its effects are large
> increase in Uplink retransmissions caused by the client never receiving
> the final ACK to their FINACK - this ACK misses the mark and routes out
> of the incorrect route."
>
> Currently marks are added to sk_buffs for replies when the "fwmark_reflect"
> sysctl is enabled. But not for TW sockets that had sk->sk_mark set via
> setsockopt(SO_MARK..).
>
> Fix this in IPv4/v6 by adding tw->tw_mark for TIME_WAIT sockets. Copy the the
> original sk->sk_mark in __inet_twsk_hashdance() to the new tw->tw_mark location.
> Then progate this so that the skb gets sent with the correct mark. Do the same
> for resets. Give the "fwmark_reflect" sysctl precedence over sk->sk_mark so that
> netfilter rules are still honored.
>
> Signed-off-by: Jon Maxwell <jmaxwell37@gmail.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Thanks Jon.
^ permalink raw reply
* Re: [PATCH] mlxsw: core: Fix an error handling path in \'mlxsw_core_bus_device_register()\'
From: Dan Carpenter @ 2018-05-10 13:17 UTC (permalink / raw)
To: Arkadi Sharshevsky; +Cc: netdev@vger.kernel.org
In-Reply-To: <2c5caff2-5ece-d07b-4df9-47a9ff24a86f@mellanox.com>
On Thu, May 10, 2018 at 04:08:22PM +0300, Arkadi Sharshevsky wrote:
> Hi Dan,
>
> I will fix the error path. Regarding the goto label this is
> the convention in the driver.
There is no rule against learning from the past.
Or there might be... I don't know all the rules at Mellanox.
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH] mlxsw: core: Fix an error handling path in \'mlxsw_core_bus_device_register()\'
From: Arkadi Sharshevsky @ 2018-05-10 13:08 UTC (permalink / raw)
To: dan.carpenter; +Cc: netdev@vger.kernel.org
In-Reply-To: <20180510124138.6yerbsfcl3abqtmq@mwanda>
Hi Dan,
I will fix the error path. Regarding the goto label this is
the convention in the driver.
Thanks,
Arkadi
^ permalink raw reply
* Re: [PATCH net] net: Correct wrong skb_flow_limit check when enable RPS
From: Eric Dumazet @ 2018-05-10 13:02 UTC (permalink / raw)
To: gfree.wind, davem, daniel, jakub.kicinski, dsahern, netdev
In-Reply-To: <1525940884-21067-1-git-send-email-gfree.wind@vip.163.com>
On 05/10/2018 01:28 AM, gfree.wind@vip.163.com wrote:
> From: Gao Feng <gfree.wind@vip.163.com>
>
> The skb flow limit is implemented for each CPU independently. In the
> current codes, the function skb_flow_limit gets the softnet_data by
> this_cpu_ptr. But the target cpu of enqueue_to_backlog would be not
> the current cpu when enable RPS. As the result, the skb_flow_limit checks
> the stats of current CPU, while the skb is going to append the queue of
> another CPU. It isn't the expected behavior.
>
> Now pass the softnet_data as a param to softnet_data to make consistent.
>
Please add a correct Fixes: tag
By doing so, you will likely add a CC: tag to make sure the author of the code
will receive your email and give feed back.
Thanks !
^ permalink raw reply
* [PATCH][next] net: aquantia: fix unsigned numvecs comparison with less than zero
From: Colin King @ 2018-05-10 12:52 UTC (permalink / raw)
To: David S . Miller, Igor Russkikh, Pavel Belous, Wei Yongjun,
netdev
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
The comparison of numvecs < 0 is always false because numvecs is a u32
and hence the error return from a failed call to pci_alloc_irq_vectores
is never detected. Fix this by using the signed int ret to handle the
error return and assign numvecs to err.
Detected by CoverityScan, CID#1468650 ("Unsigned compared against 0")
Fixes: a09bd81b5413 ("net: aquantia: Limit number of vectors to actually allocated irqs")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c b/drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c
index a50e08bb4748..750007513f9d 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c
@@ -267,14 +267,13 @@ static int aq_pci_probe(struct pci_dev *pdev,
numvecs = min(numvecs, num_online_cpus());
/*enable interrupts */
#if !AQ_CFG_FORCE_LEGACY_INT
- numvecs = pci_alloc_irq_vectors(self->pdev, 1, numvecs,
- PCI_IRQ_MSIX | PCI_IRQ_MSI |
- PCI_IRQ_LEGACY);
+ err = pci_alloc_irq_vectors(self->pdev, 1, numvecs,
+ PCI_IRQ_MSIX | PCI_IRQ_MSI |
+ PCI_IRQ_LEGACY);
- if (numvecs < 0) {
- err = numvecs;
+ if (err < 0)
goto err_hwinit;
- }
+ numvecs = err;
#endif
self->irqvecs = numvecs;
--
2.17.0
^ permalink raw reply related
* Re: [PATCH v2] net/mlx4_en: Fix an error handling path in 'mlx4_en_init_netdev()'
From: Tariq Toukan @ 2018-05-10 12:42 UTC (permalink / raw)
To: Christophe JAILLET, davem, tariqt
Cc: netdev, linux-rdma, linux-kernel, kernel-janitors
In-Reply-To: <20180510070604.19635-1-christophe.jaillet@wanadoo.fr>
On 10/05/2018 10:06 AM, Christophe JAILLET wrote:
> If an error occurs, 'mlx4_en_destroy_netdev()' is called.
> It then calls 'mlx4_en_free_resources()' which does the needed resources
> cleanup.
>
> So, doing some explicit kfree in the error handling path would lead to
> some double kfree.
>
> Simplify code to avoid such a case.
>
> Fixes: 67f8b1dcb9ee ("net/mlx4_en: Refactor the XDP forwarding rings scheme")
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
> v1 -> v2 : rewrite the fix as explained by Tariq Toukan
> (this 2nd version may have been posted twice, once without the
> v2 tag. PLease ignore the first one)
> ---
>
> drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 8 +-------
> 1 file changed, 1 insertion(+), 7 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> index e0adac4a9a19..9670b33fc9b1 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
> @@ -3324,12 +3324,11 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int port,
> MAX_TX_RINGS, GFP_KERNEL);
> if (!priv->tx_ring[t]) {
> err = -ENOMEM;
> - goto err_free_tx;
> + goto out;
> }
> priv->tx_cq[t] = kzalloc(sizeof(struct mlx4_en_cq *) *
> MAX_TX_RINGS, GFP_KERNEL);
> if (!priv->tx_cq[t]) {
> - kfree(priv->tx_ring[t]);
> err = -ENOMEM;
> goto out;
> }
> @@ -3582,11 +3581,6 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int port,
>
> return 0;
>
> -err_free_tx:
> - while (t--) {
> - kfree(priv->tx_ring[t]);
> - kfree(priv->tx_cq[t]);
> - }
> out:
> mlx4_en_destroy_netdev(dev);
> return err;
>
Reviewed-by: Tariq Toukan <tariqt@mellanox.com>
Thanks Christophe.
^ permalink raw reply
* Re: [PATCH] mlxsw: core: Fix an error handling path in 'mlxsw_core_bus_device_register()'
From: Dan Carpenter @ 2018-05-10 12:41 UTC (permalink / raw)
To: Christophe JAILLET
Cc: jiri, idosch, davem, netdev, linux-kernel, kernel-janitors
In-Reply-To: <20180510112616.20508-1-christophe.jaillet@wanadoo.fr>
On Thu, May 10, 2018 at 01:26:16PM +0200, Christophe JAILLET wrote:
> Resources are not freed in the reverse order of the allocation.
> Labels are also mixed-up.
>
> Fix it and reorder code and labels in the error handling path of
> 'mlxsw_core_bus_device_register()'
>
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
> Please review carefully. This patch is proposed because it triggers one of
> my coccinelle scripts. I'm not 100% sure if correct.
>
Looks correct.
The err = mlxsw_driver->resources_register(mlxsw_core); pointer is a
pointer to mlxsw_sp_resources_register(). That function doesn't clean
up after itself on failure. Ideally, you'd want a matching
mlxsw_driver->resources_unregister as well pointer instead of hard
coding devlink_resources_unregister().
The error handling would be easier to review if the gotos told you what
the did. Right now they're written in "come from" style so the tell you
what happened on the line before.
if (!foo)
goto allocating_foo_failed;
Hopefully if someone fixes mlxsw_sp_resources_register() they'll choose
better label names.
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH 2/2] alx: add disable_wol paramenter
From: Andrew Lunn @ 2018-05-10 12:34 UTC (permalink / raw)
To: AceLan Kao
Cc: David Miller, James Cliburn, Chris Snook, rakesh, netdev,
Linux-Kernel@Vger. Kernel. Org, Emily Chien
In-Reply-To: <CAFv23QkETCHYqpsp_1JR30A1F=x+xCCQQv2RM9VOg24qhQRMOQ@mail.gmail.com>
On Thu, May 10, 2018 at 01:58:24PM +0800, AceLan Kao wrote:
> Hi Andrew,
>
> We have some machines using Qualcomm Atheros Killer E2400 Gigabit
> Ethernet Controller,
> but none of them has the unintentional wake up issue.
> We're willing to fix it if we encountered the issue, but before we can
> do it, we need this feature is supported by the driver.
>
> Taking the feature has been removed for 5 years into account, I doubt
> if we still can reproduce this issue,
> but again, to verify this issue we need to add back this feature first.
> Set WoL disabled by default won't introduce any regression but give
> users and developers a chance to fix it.
The main problem here is the module parameter. That is not going to be
accepted.
Can you argue the cure is worse than the disease? Is WoL not working
considered by a lot of people as being a bug? Double wake up is also a
bug, but not many people care, it does not cause any data corruption,
etc. So can you argue overall we have a less buggy system, but still
buggy, if WoL is enabled?
If you can write a convincing Change Message arguing the case, a patch
simply re-enabling WoL might be accepted.
But you also need to take on the responsibility to help debug the
failed shutdowns in order to get to the bottom of this problem.
Andrew
^ permalink raw reply
* Re: [PATCH net] tipc: eliminate KMSAN uninit-value in strcmp complaint
From: David Miller @ 2018-05-10 12:25 UTC (permalink / raw)
To: ying.xue; +Cc: jon.maloy, netdev, syzkaller-bugs, tipc-discussion
In-Reply-To: <1525787046-5661-1-git-send-email-ying.xue@windriver.com>
From: Ying Xue <ying.xue@windriver.com>
Date: Tue, 8 May 2018 21:44:06 +0800
> When we get link properties through netlink interface with
> tipc_nl_node_get_link(), we don't validate TIPC_NLA_LINK_NAME
> attribute at all, instead we directly use it. As a consequence,
> KMSAN detected the TIPC_NLA_LINK_NAME attribute was an uninitialized
> value, and then posted the following complaint:
...
> To quiet the complaint, TIPC_NLA_LINK_NAME attribute has been
> validated in tipc_nl_node_get_link() before it's used.
>
> Reported-by: syzbot+df0257c92ffd4fcc58cd@syzkaller.appspotmail.com
> Signed-off-by: Ying Xue <ying.xue@windriver.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH] net/9p: correct some comment errors in 9p file system code
From: David Miller @ 2018-05-10 12:22 UTC (permalink / raw)
To: sunlw.fnst; +Cc: viro, rdunlap, netdev
In-Reply-To: <20180508014938.28134-1-sunlw.fnst@cn.fujitsu.com>
From: Sun Lianwen <sunlw.fnst@cn.fujitsu.com>
Date: Tue, 8 May 2018 09:49:38 +0800
> There are follow comment errors:
> 1 The function name is wrong in p9_release_pages() comment.
> 2 The function name and variable name is wrong in p9_poll_workfn() comment.
> 3 There is no variable dm_mr and lkey in struct p9_trans_rdma.
> 4 The function name is wrong in rdma_create_trans() comment.
> 5 There is no variable initialized in struct virtio_chan.
> 6 The variable name is wrong in p9_virtio_zc_request() comment.
>
> Signed-off-by: Sun Lianwen <sunlw.fnst@cn.fujitsu.com>
> Reviewed-by: Randy Dunlap <rdunlap@infradead.org>
Applied.
^ permalink raw reply
* Re: [pull request][for-next 0/6] Mellanox, mlx5 updates 2018-05-07
From: David Miller @ 2018-05-10 12:18 UTC (permalink / raw)
To: saeedm; +Cc: dledford, netdev, linux-rdma, leonro, jgg
In-Reply-To: <20180507235304.25085-1-saeedm@mellanox.com>
From: Saeed Mahameed <saeedm@mellanox.com>
Date: Mon, 7 May 2018 16:52:58 -0700
> This pull request includes misc updates and cleanups for mlx5 core
> driver for both net and rdma next branches, for more information please
> see tag log below.
>
> Please pull and let me know if there's any problem.
Looks good, pulled into net-next.
Thanks.
^ permalink raw reply
* Re: [PATCH net-next 1/4] net: bridge: Allow bridge master in br_vlan_get_info()
From: Nikolay Aleksandrov @ 2018-05-10 12:06 UTC (permalink / raw)
To: Ido Schimmel, netdev, bridge; +Cc: davem, jiri, petrm, stephen, mlxsw
In-Reply-To: <20180510101306.4891-2-idosch@mellanox.com>
On 10/05/18 13:13, Ido Schimmel wrote:
> From: Petr Machata <petrm@mellanox.com>
>
> Mirroring offload in mlxsw needs to check that a given VLAN is allowed
> to ingress the bridge device. br_vlan_get_info() is the function that is
> used for this, however currently it only supports bridge port devices.
> Extend it to support bridge masters as well.
>
> Signed-off-by: Petr Machata <petrm@mellanox.com>
> Signed-off-by: Ido Schimmel <idosch@mellanox.com>
> ---
> net/bridge/br_vlan.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
> index df37a5137c25..dc832c0934c6 100644
> --- a/net/bridge/br_vlan.c
> +++ b/net/bridge/br_vlan.c
> @@ -1176,6 +1176,8 @@ int br_vlan_get_info(const struct net_device *dev, u16 vid,
> p = br_port_get_check_rtnl(dev);
> if (p)
> vg = nbp_vlan_group(p);
> + else if (netif_is_bridge_master(dev))
> + vg = br_vlan_group(netdev_priv(dev));
> else
> return -EINVAL;
>
>
Acked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
^ permalink raw reply
* Re: [PATCH net] sctp: remove sctp_chunk_put from fail_mark err path in sctp_ulpevent_make_rcvmsg
From: Neil Horman @ 2018-05-10 11:58 UTC (permalink / raw)
To: Xin Long; +Cc: network dev, linux-sctp, davem, Marcelo Ricardo Leitner,
syzkaller
In-Reply-To: <e4d7cf118c028ed05c9005951e30babc8bb300eb.1525944853.git.lucien.xin@gmail.com>
On Thu, May 10, 2018 at 05:34:13PM +0800, Xin Long wrote:
> In Commit 1f45f78f8e51 ("sctp: allow GSO frags to access the chunk too"),
> it held the chunk in sctp_ulpevent_make_rcvmsg to access it safely later
> in recvmsg. However, it also added sctp_chunk_put in fail_mark err path,
> which is only triggered before holding the chunk.
>
> syzbot reported a use-after-free crash happened on this err path, where
> it shouldn't call sctp_chunk_put.
>
> This patch simply removes this call.
>
> Fixes: 1f45f78f8e51 ("sctp: allow GSO frags to access the chunk too")
> Reported-by: syzbot+141d898c5f24489db4aa@syzkaller.appspotmail.com
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> ---
> net/sctp/ulpevent.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/net/sctp/ulpevent.c b/net/sctp/ulpevent.c
> index 84207ad..8cb7d98 100644
> --- a/net/sctp/ulpevent.c
> +++ b/net/sctp/ulpevent.c
> @@ -715,7 +715,6 @@ struct sctp_ulpevent *sctp_ulpevent_make_rcvmsg(struct sctp_association *asoc,
> return event;
>
> fail_mark:
> - sctp_chunk_put(chunk);
> kfree_skb(skb);
> fail:
> return NULL;
> --
> 2.1.0
>
>
Acked-by: Neil Horman <nhorman@tuxdriver.com>
^ permalink raw reply
* Re: [PATCH] mlxsw: core: Fix an error handling path in 'mlxsw_core_bus_device_register()'
From: Ido Schimmel @ 2018-05-10 11:58 UTC (permalink / raw)
To: Christophe JAILLET
Cc: jiri, idosch, davem, netdev, linux-kernel, kernel-janitors
In-Reply-To: <20180510112616.20508-1-christophe.jaillet@wanadoo.fr>
On Thu, May 10, 2018 at 01:26:16PM +0200, Christophe JAILLET wrote:
> Resources are not freed in the reverse order of the allocation.
> Labels are also mixed-up.
>
> Fix it and reorder code and labels in the error handling path of
> 'mlxsw_core_bus_device_register()'
>
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
For net:
Fixes: ef3116e5403e ("mlxsw: spectrum: Register KVD resources with devlink")
Reviewed-by: Ido Schimmel <idosch@mellanox.com>
Next time, please indicate the tree you're targeting as explained here:
https://www.kernel.org/doc/Documentation/networking/netdev-FAQ.txt
And include a Fixes line as explained here:
https://www.kernel.org/doc/Documentation/process/submitting-patches.rst
Thanks!
^ permalink raw reply
* [PATCH] mlxsw: core: Fix an error handling path in 'mlxsw_core_bus_device_register()'
From: Christophe JAILLET @ 2018-05-10 11:26 UTC (permalink / raw)
To: jiri, idosch, davem
Cc: netdev, linux-kernel, kernel-janitors, Christophe JAILLET
Resources are not freed in the reverse order of the allocation.
Labels are also mixed-up.
Fix it and reorder code and labels in the error handling path of
'mlxsw_core_bus_device_register()'
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
Please review carefully. This patch is proposed because it triggers one of
my coccinelle scripts. I'm not 100% sure if correct.
The script tries to spot wrongly ordered error handling path. It is:
@@
identifier l1, l2;
@@
if (...) {
...
* goto l1;
}
...
if (...) {
...
* goto l2;
}
...
*l1:
...
*l2:
...
---
drivers/net/ethernet/mellanox/mlxsw/core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.c b/drivers/net/ethernet/mellanox/mlxsw/core.c
index 93ea56620a24..e13ac3b8dff7 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.c
@@ -1100,11 +1100,11 @@ int mlxsw_core_bus_device_register(const struct mlxsw_bus_info *mlxsw_bus_info,
err_alloc_lag_mapping:
mlxsw_ports_fini(mlxsw_core);
err_ports_init:
- mlxsw_bus->fini(bus_priv);
-err_bus_init:
if (!reload)
devlink_resources_unregister(devlink, NULL);
err_register_resources:
+ mlxsw_bus->fini(bus_priv);
+err_bus_init:
if (!reload)
devlink_free(devlink);
err_devlink_alloc:
--
2.17.0
^ permalink raw reply related
* Re: [RFC v3 3/5] virtio_ring: add packed ring support
From: Tiwei Bie @ 2018-05-10 10:50 UTC (permalink / raw)
To: Jason Wang; +Cc: netdev, wexu, virtualization, linux-kernel, mst
In-Reply-To: <2fc35cd5-9dbd-7743-497f-b6637d92f528@redhat.com>
On Thu, May 10, 2018 at 05:49:20PM +0800, Jason Wang wrote:
> On 2018年05月10日 16:56, Tiwei Bie wrote:
> > On Thu, May 10, 2018 at 03:34:50PM +0800, Jason Wang wrote:
> > > On 2018年05月10日 15:32, Jason Wang wrote:
> > > > On 2018年04月25日 13:15, Tiwei Bie wrote:
> > > > > + /* We're using some buffers from the free list. */
> > > > > + vq->vq.num_free -= descs_used;
> > > > > +
> > > > > + /* Update free pointer */
> > > > > + if (indirect) {
> > > > > + n = head + 1;
> > > > > + if (n >= vq->vring_packed.num) {
> > > > > + n = 0;
> > > > > + vq->wrap_counter ^= 1;
> > > > > + }
> > > > > + vq->next_avail_idx = n;
> > > > > + } else
> > > > > + vq->next_avail_idx = i;
> > > > During testing zerocopy (out of order completion), I found driver may
> > > > submit two identical buffer id to vhost. So the above code may not work
> > > > well.
> > > >
> > > > Consider the case that driver adds 3 buffer and virtqueue size is 8.
> > > >
> > > > a) id = 0,count = 2,next_avail = 2
> > > >
> > > > b) id = 2,count = 4,next_avail = 2
> > > next_avail should be 6 here.
> > >
> > > > c) id = 4,count = 2,next_avail = 0
> > > >
> > > id should be 6 here.
> > >
> > > Thanks
> > >
> > > > if packet b is done before packet a, driver may think buffer id 0 is
> > > > available and try to use it if even if the real buffer 0 was not done.
> > > >
> > > > Thanks
> > Nice catch! Thanks a lot!
> > I'll implement an ID allocator.
> >
> > Best regards,
> > Tiwei Bie
>
> Sounds good.
>
> Another similar issue is detac_buf_packed(). It did:
>
> for (j = 0; j < vq->desc_state[head].num; j++) {
> desc = &vq->vring_packed.desc[i];
> vring_unmap_one_packed(vq, desc);
> i++;
> if (i >= vq->vring_packed.num)
> i = 0;
> }
>
> This probably won't work for out of order too and according to the spec:
>
> """
> Driver needs to keep track of the size of the list corresponding to each
> buffer ID, to be able to skip to where the next used descriptor is written
> by the device.
> """
>
> Looks like we should not depend on the descriptor ring.
Yeah, the previous ID allocation is too simple..
Let me fix it in the next version.
Thanks!
>
> Thanks
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH bpf v3] x86/cpufeature: bpf hack for clang not supporting asm goto
From: Borislav Petkov @ 2018-05-10 10:49 UTC (permalink / raw)
To: Yonghong Song
Cc: peterz, mingo, torvalds, ast, daniel, linux-kernel, x86, netdev,
kernel-team
In-Reply-To: <20180504033119.2130788-1-yhs@fb.com>
On Thu, May 03, 2018 at 08:31:19PM -0700, Yonghong Song wrote:
> Commit d0266046ad54 ("x86: Remove FAST_FEATURE_TESTS")
> removed X86_FAST_FEATURE_TESTS and make macro static_cpu_has() always
> use __always_inline function _static_cpu_has() funciton.
> The static_cpu_has() uses gcc feature asm goto construct,
> which is not supported by clang.
>
> Issues
> ======
>
> Currently, for BPF programs written in C, the only widely-supported
> compiler is clang. Because of clang lacking support
> of asm goto construct, if you try to compile
> bpf programs under samples/bpf/,
> $ make -j20 && make headers_install && make samples/bpf/
> you will see a lot of failures like below:
>
> clang -nostdinc -isystem /usr/lib/gcc/x86_64-redhat-linux/7/include \
> -I/home/yhs/work/bpf-next/arch/x86/include \
> -I./arch/x86/include/generated -I/home/yhs/work/bpf-next/include \
> -I./include -I/home/yhs/work/bpf-next/arch/x86/include/uapi \
> -I./arch/x86/include/generated/uapi \
> -I/home/yhs/work/bpf-next/include/uapi -I./include/generated/uapi \
> -include /home/yhs/work/bpf-next/include/linux/kconfig.h
> -Isamples/bpf \
> -I/home/yhs/work/bpf-next/tools/testing/selftests/bpf/ \
> -D__KERNEL__ -Wno-unused-value -Wno-pointer-sign \
> -D__TARGET_ARCH_x86 -Wno-compare-distinct-pointer-types \
> -Wno-gnu-variable-sized-type-not-at-end \
> -Wno-address-of-packed-member -Wno-tautological-compare \
> -Wno-unknown-warning-option \
> -O2 -emit-llvm -c /home/yhs/work/bpf-next/samples/bpf/xdp_redirect_cpu_kern.c \
> -o -| llc -march=bpf -filetype=obj -o samples/bpf/xdp_redirect_cpu_kern.o
> In file included from /home/yhs/work/bpf-next/samples/bpf/xdp_redirect_cpu_kern.c:10:
This is not the kernel's problem.
Fix the samples to build successfully like the rest of the things in
tools/ use kernel machinery and undefine/redefine stuff so that it
builds. For example, tools/include/asm/alternative-asm.h redefines two
macros. So do something along those lines.
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
^ permalink raw reply
* Re: [RFC PATCH 5/5] net: macb: Add WOL support with ARP
From: Harini Katakam @ 2018-05-10 10:37 UTC (permalink / raw)
To: Claudiu Beznea
Cc: Nicolas Ferre, David Miller, netdev, linux-kernel, michals,
appanad
In-Reply-To: <51e1b2f3-4dd7-3513-2148-649372775130@microchip.com>
Hi Claudiu,
On Fri, May 4, 2018 at 5:47 PM, Claudiu Beznea
<Claudiu.Beznea@microchip.com> wrote:
>
>
> On 22.03.2018 15:51, harinikatakamlinux@gmail.com wrote:
>> From: Harini Katakam <harinik@xilinx.com>
>>
>> This patch enables ARP wake event support in GEM through the following:
>>
>> -> WOL capability can be selected based on the SoC/GEM IP version rather
>> than a devictree property alone. Hence add a new capability property and
>> set device as "wakeup capable" in probe in this case.
>> -> Wake source selection can be done via ethtool or by enabling wakeup
>> in /sys/devices/platform/..../ethx/power/
>> This patch adds default wake source as ARP and the existing selection of
>> WOL using magic packet remains unchanged.
>> -> When GEM is the wake device with ARP as the wake event, the current
>> IP address to match is written to WOL register along with other
>> necessary confuguration required for MAC to recognize an ARP event.
>> -> While RX needs to remain enabled, there is no need to process the
>> actual wake packet - hence tie off all RX queues to avoid unnecessary
>> processing by DMA in the background.
>
> Why is this different for magic packet vs ARP packet?
This should ideally be the same whether it is a magic packet or ARP on
the version of the IP we use (more details in my comment below).
I simply did not alter the magic packet code for now to avoid breaking
others' flow.
<snip>
>> +#define MACB_CAPS_WOL 0x00000080
>
> I think would be better to have this as part of bp->wol and use it properly
> in suspend/resume hooks.
I think a capability flag as part of config structure is better
because this is clearly an SoC related feature and there is no need
to have a devicetree property.
<snip>
> Wouldn't it work if you will change it in something like this:
>
> u32 wolmask, arpipmask = 0;
>
> if (bp->wol & MACB_WOL_ENABLED) {
> macb_writel(bp, IER, MACB_BIT(WOL));
>
> if (bp->wol & MACB_WOL_HAS_ARP_PACKET) {
> /* Enable broadcast. */
> gem_writel(bp, NCFGR, gem_readl(bp, NCFGR) & ~MACB_BIT(NBC));
> arpipmask = cpu_to_be32p(&bp->dev->ip_ptr->ifa_list->ifa_local) & 0xFFFF;
> wolmask = arpipmask | MACB_BIT(ARP);
> } else {
> wolmask = MACB_BIT(MAG);
> }
>
> macb_writel(bp, WOL, wolmask);
> enable_irq_wake(bp->queues[0].irq);
The above would work. But I'd still have to add the RX BD changes
and then stop the phy, disable interrupt etc., for most optimal power
down state - the idea is to keep only is essential to detect a wake event.
> netif_device_detach(netdev);
> }
>
> I cannot find anything particular for ARP WOL events in datasheet. Also,
> I cannot find something related to DMA activity while WOL is active
Can you please let me know which version you are referring to?
ZynqMP uses the IP version r1p06 or 07.
There is a clear set of rules for ARP wake event to be recognized.
About the DMA activity, it is not explicitly mentioned but we have
observed that the DMA will continue to process incoming packets
and try to write them to the memory and Cadence has confirmed
the same. Later versions of the IP may have some provision to
stop DMA activity on RX channel but unfortunately in this version,
using a dummy RX buffer descriptor is the only way.
I'm looking into your other suggestions.
Thanks, will get back to you.
Regards,
Harini
^ permalink raw reply
* [PATCH net-next] cxgb4: fix the wrong conversion of Mbps to Kbps
From: Ganesh Goudar @ 2018-05-10 10:37 UTC (permalink / raw)
To: netdev, davem; +Cc: nirranjan, indranil, venkatesh, Ganesh Goudar
fix the wrong conversion where 1 Mbps was converted to
1024 Kbps.
Signed-off-by: Ganesh Goudar <ganeshgr@chelsio.com>
---
drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
index 32cad0a..5e33780 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
@@ -2886,13 +2886,13 @@ static int cxgb_set_tx_maxrate(struct net_device *dev, int index, u32 rate)
}
/* Convert from Mbps to Kbps */
- req_rate = rate << 10;
+ req_rate = rate * 1000;
/* Max rate is 100 Gbps */
- if (req_rate >= SCHED_MAX_RATE_KBPS) {
+ if (req_rate > SCHED_MAX_RATE_KBPS) {
dev_err(adap->pdev_dev,
"Invalid rate %u Mbps, Max rate is %u Mbps\n",
- rate, SCHED_MAX_RATE_KBPS >> 10);
+ rate, SCHED_MAX_RATE_KBPS / 1000);
return -ERANGE;
}
--
2.1.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox