* Re: [PATCH v2] can: m_can: Move accessing of message ram to after clocks are enabled
From: Marc Kleine-Budde @ 2018-07-23 8:07 UTC (permalink / raw)
To: Faiz Abbas, linux-kernel, netdev, linux-can, linux-omap; +Cc: wg, davem
In-Reply-To: <20180703111710.27084-1-faiz_abbas@ti.com>
[-- Attachment #1.1: Type: text/plain, Size: 571 bytes --]
On 07/03/2018 01:17 PM, Faiz Abbas wrote:
> MCAN message ram should only be accessed once clocks are enabled.
> Therefore, move the call to parse/init the message ram to after
> clocks are enabled.
>
> Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
applied to linux-can.
tnx,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH] netlink: fix memory leak of dump
From: Florian Westphal @ 2018-07-23 10:59 UTC (permalink / raw)
To: shaochun chen
Cc: Florian Westphal, Pablo Neira Ayuso, David Miller, kadlec,
johannes.berg, jason, ktkhai, lucien.xin, xiyou.wangcong, dsahern,
netfilter-devel, tom, netdev, linux-kernel
In-Reply-To: <CAKDdixBBd08eKvk-hJtwPy46u2uzihRCh_KdEMF3xG1ZJ6oNsg@mail.gmail.com>
shaochun chen <cscnull@gmail.com> wrote:
> I have a question: we will try_module_get in __netlink_dump_start(),
Thats too late, we release rcu read lock before this, so the module
implementing ->dump might have been removed already.
> but why we need to call try_module_get again in nft_netlink_dump_start ??
Its the other way around.
This is the first try_module_get; at this point we still hold rcu read
lock.
If nf_tables module is being removed, try_module_get will fail and
we can error out.
If it succeeds, its safe to drop the rcu read lock.
^ permalink raw reply
* [PATCH net-next] net/tls: Corrected enabling of zero-copy mode
From: Vakul Garg @ 2018-07-23 15:30 UTC (permalink / raw)
To: netdev; +Cc: borisp, aviadye, davejwatson, davem, Vakul Garg
Zero-copy mode was left enabled even when zerocopy_from_iter() failed.
In this case, control falls back to regular non zero-copy mode. This
caused skipping call of datagram copy function skb_copy_datagram_msg()
and required an extra iteration of record decryption loop for decrypted
record to be copied into user space provided buffer. Hence zero-copy
mode should be enabled/disabled as per the success/failure of
zerocopy_from_iter().
Fixes: c46234ebb4d1 ("tls: RX path for ktls")
Signed-off-by: Vakul Garg <vakul.garg@nxp.com>
---
The patch does not need to be applied to 'net' branch as it does not fix
any functional bug. The patch achieves better run-time efficiency and
code readability.
net/tls/tls_sw.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index 0c2d029c9d4c..9ae57bec4927 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -787,7 +787,7 @@ int tls_sw_recvmsg(struct sock *sk,
target = sock_rcvlowat(sk, flags & MSG_WAITALL, len);
timeo = sock_rcvtimeo(sk, flags & MSG_DONTWAIT);
do {
- bool zc = false;
+ bool zc;
int chunk = 0;
skb = tls_wait_data(sk, flags, timeo, &err);
@@ -824,7 +824,6 @@ int tls_sw_recvmsg(struct sock *sk,
struct scatterlist sgin[MAX_SKB_FRAGS + 1];
int pages = 0;
- zc = true;
sg_init_table(sgin, MAX_SKB_FRAGS + 1);
sg_set_buf(&sgin[0], ctx->rx_aad_plaintext,
TLS_AAD_SPACE_SIZE);
@@ -836,6 +835,7 @@ int tls_sw_recvmsg(struct sock *sk,
if (err < 0)
goto fallback_to_reg_recv;
+ zc = true;
err = decrypt_skb_update(sk, skb, sgin, &zc);
for (; pages > 0; pages--)
put_page(sg_page(&sgin[pages]));
@@ -845,6 +845,7 @@ int tls_sw_recvmsg(struct sock *sk,
}
} else {
fallback_to_reg_recv:
+ zc = false;
err = decrypt_skb_update(sk, skb, NULL, &zc);
if (err < 0) {
tls_err_abort(sk, EBADMSG);
--
2.13.6
^ permalink raw reply related
* [PATCH net-next] selftests: forwarding: gre_multipath: Drop IPv6 tests
From: Petr Machata @ 2018-07-23 10:33 UTC (permalink / raw)
To: netdev, linux-kselftest; +Cc: davem, shuah, idosch, dsahern
Support for device-only IPv6 multipath next hops was dropped in
commit 33bd5ac54dc4 ("net/ipv6: Revert attempt to simplify route replace
and append") and as of commit b5d2d75e079a ("net/ipv6: Do not allow
device only routes via the multipath API"), attempts to add a next hop
like that yield an explicit diagnostic.
Correspondingly, drop the IPv6 parts of GRE multipath test that are
supposed to test that code.
Signed-off-by: Petr Machata <petrm@mellanox.com>
---
.../selftests/net/forwarding/gre_multipath.sh | 113 ++-------------------
1 file changed, 6 insertions(+), 107 deletions(-)
diff --git a/tools/testing/selftests/net/forwarding/gre_multipath.sh b/tools/testing/selftests/net/forwarding/gre_multipath.sh
index 982cc8c23200..3b1e047f7617 100755
--- a/tools/testing/selftests/net/forwarding/gre_multipath.sh
+++ b/tools/testing/selftests/net/forwarding/gre_multipath.sh
@@ -8,14 +8,12 @@
# | H1 |
# | $h1 + |
# | 192.0.2.1/28 | |
-# | 2001:db8:1::1/64 | |
# +-------------------|-----+
# |
# +-------------------|------------------------+
# | SW1 | |
# | $ol1 + |
# | 192.0.2.2/28 |
-# | 2001:db8:1::2/64 |
# | |
# | + g1a (gre) + g1b (gre) |
# | loc=192.0.2.65 loc=192.0.2.81 |
@@ -49,22 +47,17 @@
# | |
# | $ol2 + |
# | 192.0.2.17/28 | |
-# | 2001:db8:2::1/64 | |
# +-------------------|------------------------+
# |
# +-------------------|-----+
# | H2 | |
# | $h2 + |
# | 192.0.2.18/28 |
-# | 2001:db8:2::2/64 |
# +-------------------------+
ALL_TESTS="
ping_ipv4
- ping_ipv6
multipath_ipv4
- multipath_ipv6
- multipath_ipv6_l4
"
NUM_NETIFS=6
@@ -74,19 +67,17 @@ h1_create()
{
simple_if_init $h1 192.0.2.1/28 2001:db8:1::1/64
ip route add vrf v$h1 192.0.2.16/28 via 192.0.2.2
- ip route add vrf v$h1 2001:db8:2::/64 via 2001:db8:1::2
}
h1_destroy()
{
- ip route del vrf v$h1 2001:db8:2::/64 via 2001:db8:1::2
ip route del vrf v$h1 192.0.2.16/28 via 192.0.2.2
simple_if_fini $h1 192.0.2.1/28
}
sw1_create()
{
- simple_if_init $ol1 192.0.2.2/28 2001:db8:1::2/64
+ simple_if_init $ol1 192.0.2.2/28
__simple_if_init $ul1 v$ol1
vlan_create $ul1 111 v$ol1 192.0.2.129/28
vlan_create $ul1 222 v$ol1 192.0.2.145/28
@@ -102,9 +93,6 @@ sw1_create()
ip route add vrf v$ol1 192.0.2.16/28 \
nexthop dev g1a \
nexthop dev g1b
- ip route add vrf v$ol1 2001:db8:2::/64 \
- nexthop dev g1a \
- nexthop dev g1b
tc qdisc add dev $ul1 clsact
tc filter add dev $ul1 egress pref 111 prot 802.1q \
@@ -117,7 +105,6 @@ sw1_destroy()
{
tc qdisc del dev $ul1 clsact
- ip route del vrf v$ol1 2001:db8:2::/64
ip route del vrf v$ol1 192.0.2.16/28
ip route del vrf v$ol1 192.0.2.82/32 via 192.0.2.146
@@ -131,12 +118,12 @@ sw1_destroy()
vlan_destroy $ul1 222
vlan_destroy $ul1 111
__simple_if_fini $ul1
- simple_if_fini $ol1 192.0.2.2/28 2001:db8:1::2/64
+ simple_if_fini $ol1 192.0.2.2/28
}
sw2_create()
{
- simple_if_init $ol2 192.0.2.17/28 2001:db8:2::1/64
+ simple_if_init $ol2 192.0.2.17/28
__simple_if_init $ul2 v$ol2
vlan_create $ul2 111 v$ol2 192.0.2.130/28
vlan_create $ul2 222 v$ol2 192.0.2.146/28
@@ -152,14 +139,10 @@ sw2_create()
ip route add vrf v$ol2 192.0.2.0/28 \
nexthop dev g2a \
nexthop dev g2b
- ip route add vrf v$ol2 2001:db8:1::/64 \
- nexthop dev g2a \
- nexthop dev g2b
}
sw2_destroy()
{
- ip route del vrf v$ol2 2001:db8:1::/64
ip route del vrf v$ol2 192.0.2.0/28
ip route del vrf v$ol2 192.0.2.81/32 via 192.0.2.145
@@ -173,21 +156,19 @@ sw2_destroy()
vlan_destroy $ul2 222
vlan_destroy $ul2 111
__simple_if_fini $ul2
- simple_if_fini $ol2 192.0.2.17/28 2001:db8:2::1/64
+ simple_if_fini $ol2 192.0.2.17/28
}
h2_create()
{
- simple_if_init $h2 192.0.2.18/28 2001:db8:2::2/64
+ simple_if_init $h2 192.0.2.18/28
ip route add vrf v$h2 192.0.2.0/28 via 192.0.2.17
- ip route add vrf v$h2 2001:db8:1::/64 via 2001:db8:2::1
}
h2_destroy()
{
- ip route del vrf v$h2 2001:db8:1::/64 via 2001:db8:2::1
ip route del vrf v$h2 192.0.2.0/28 via 192.0.2.17
- simple_if_fini $h2 192.0.2.18/28 2001:db8:2::2/64
+ simple_if_fini $h2 192.0.2.18/28
}
setup_prepare()
@@ -250,77 +231,11 @@ multipath4_test()
sysctl_restore net.ipv4.fib_multipath_hash_policy
}
-multipath6_l4_test()
-{
- local what=$1; shift
- local weight1=$1; shift
- local weight2=$1; shift
-
- sysctl_set net.ipv6.fib_multipath_hash_policy 1
- ip route replace vrf v$ol1 2001:db8:2::/64 \
- nexthop dev g1a weight $weight1 \
- nexthop dev g1b weight $weight2
-
- local t0_111=$(tc_rule_stats_get $ul1 111 egress)
- local t0_222=$(tc_rule_stats_get $ul1 222 egress)
-
- ip vrf exec v$h1 \
- $MZ $h1 -6 -q -p 64 -A 2001:db8:1::1 -B 2001:db8:2::2 \
- -d 1msec -t udp "sp=1024,dp=0-32768"
-
- local t1_111=$(tc_rule_stats_get $ul1 111 egress)
- local t1_222=$(tc_rule_stats_get $ul1 222 egress)
-
- local d111=$((t1_111 - t0_111))
- local d222=$((t1_222 - t0_222))
- multipath_eval "$what" $weight1 $weight2 $d111 $d222
-
- ip route replace vrf v$ol1 2001:db8:2::/64 \
- nexthop dev g1a \
- nexthop dev g1b
- sysctl_restore net.ipv6.fib_multipath_hash_policy
-}
-
-multipath6_test()
-{
- local what=$1; shift
- local weight1=$1; shift
- local weight2=$1; shift
-
- ip route replace vrf v$ol1 2001:db8:2::/64 \
- nexthop dev g1a weight $weight1 \
- nexthop dev g1b weight $weight2
-
- local t0_111=$(tc_rule_stats_get $ul1 111 egress)
- local t0_222=$(tc_rule_stats_get $ul1 222 egress)
-
- # Generate 16384 echo requests, each with a random flow label.
- for ((i=0; i < 16384; ++i)); do
- ip vrf exec v$h1 $PING6 2001:db8:2::2 -F 0 -c 1 -q &> /dev/null
- done
-
- local t1_111=$(tc_rule_stats_get $ul1 111 egress)
- local t1_222=$(tc_rule_stats_get $ul1 222 egress)
-
- local d111=$((t1_111 - t0_111))
- local d222=$((t1_222 - t0_222))
- multipath_eval "$what" $weight1 $weight2 $d111 $d222
-
- ip route replace vrf v$ol1 2001:db8:2::/64 \
- nexthop dev g1a \
- nexthop dev g1b
-}
-
ping_ipv4()
{
ping_test $h1 192.0.2.18
}
-ping_ipv6()
-{
- ping6_test $h1 2001:db8:2::2
-}
-
multipath_ipv4()
{
log_info "Running IPv4 multipath tests"
@@ -329,22 +244,6 @@ multipath_ipv4()
multipath4_test "Weighted MP 11:45" 11 45
}
-multipath_ipv6()
-{
- log_info "Running IPv6 multipath tests"
- multipath6_test "ECMP" 1 1
- multipath6_test "Weighted MP 2:1" 2 1
- multipath6_test "Weighted MP 11:45" 11 45
-}
-
-multipath_ipv6_l4()
-{
- log_info "Running IPv6 L4 hash multipath tests"
- multipath6_l4_test "ECMP" 1 1
- multipath6_l4_test "Weighted MP 2:1" 2 1
- multipath6_l4_test "Weighted MP 11:45" 11 45
-}
-
trap cleanup EXIT
setup_prepare
--
2.4.11
^ permalink raw reply related
* Re: [PATCH] can: mpc5xxx_can: check of_iomap return before use
From: Marc Kleine-Budde @ 2018-07-23 11:37 UTC (permalink / raw)
To: Fabio Estevam, Nicholas Mc Guire
Cc: Wolfram Sang, Wolfgang Grandegger, David S. Miller, linux-can,
netdev, linux-kernel
In-Reply-To: <CAOMZO5CPVx_3kj5xKaoNbYP14cgPk9aSniyu44oWF-0jbSx91g@mail.gmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 1853 bytes --]
On 07/09/2018 09:28 PM, Fabio Estevam wrote:
> On Mon, Jul 9, 2018 at 4:16 PM, Nicholas Mc Guire <hofrat@osadl.org> wrote:
>> of_iompa() can return NULL so that return needs to be checked and NULL
>
> s/of_iompa/of_iomap/
>
>> treated as failure. While at it also take care of the missing
>> of_node_put() in the error path.
>>
>> Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
>> Fixes: commit afa17a500a36 ("net/can: add driver for mscan family & mpc52xx_mscan")
>> ---
>>
>> Problem was found by an experimental coccinelle script
>>
>> Patch was compiletested with: mpc5200_defconfig + CONFIG_CAN=y,
>> CONFIG_CAN_MSCAN=y, CONFIG_CAN_MPC5XXX=y
>> (with a number of sparse warnings not related to the proposed change)
>>
>> Patch is against 4.18-rc3 (localversion-next is next-20180706)
>>
>> drivers/net/can/mscan/mpc5xxx_can.c | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/net/can/mscan/mpc5xxx_can.c b/drivers/net/can/mscan/mpc5xxx_can.c
>> index c7427bd..2949a38 100644
>> --- a/drivers/net/can/mscan/mpc5xxx_can.c
>> +++ b/drivers/net/can/mscan/mpc5xxx_can.c
>> @@ -86,6 +86,11 @@ static u32 mpc52xx_can_get_clock(struct platform_device *ofdev,
>> return 0;
>> }
>> cdm = of_iomap(np_cdm, 0);
>> + if (!cdm) {
>> + of_node_put(np_cdm);
>> + dev_err(&ofdev->dev, "can't map clock node!\n");
>> + return 0;
>
> I think you should return an error code here. -ENOMEM maybe?
Can you send an updated version?
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH] net/9p/trans_fd.c: fix double list_del() and race in access
From: Tomas Bortoli @ 2018-07-23 11:46 UTC (permalink / raw)
To: Dominique Martinet
Cc: ericvh, rminnich, lucho, jiangyiwen, davem, v9fs-developer,
netdev, linux-kernel, syzkaller
In-Reply-To: <20180723030251.GB24608@nautica>
On 07/23/2018 05:02 AM, Dominique Martinet wrote:
> Tomas Bortoli wrote on Fri, Jul 20, 2018:
>> This patch uses list_del_init() instead of list_del() to eliminate
>> "req_list". This to prevent double list_del()'s calls to the same list
>> from provoking a GPF. Furthermore, this patch fixes an access to
>> "req_list" that was made without getting the relative lock.
>
> Please see comment about locking.
>
> As for list_del to list_del_init, it feels a little wrong to me, but I
> don't have a better idea so let's go with that.
Yes, it's not the best solution.
> Do you know what happened to trigger this? one thread running
> p9_conn_cancel then the other thread doing p9_fd_cancel ?
>
I don't see how races should be prevented. The bug is triggered in
p9_fd_cancel and in this case it's due to the status of the request
being REQ_STATUS_UNSENT but list_del(&req->req_list) is used 4 times in
trans_fd.c:
- p9_read_work()
with the lock but updating the status afterwards (brings to race)
- p9_conn_cancel()
without the lock and updating the status afterwards (brings to race)
- p9_fd_cancelled()
.. ?
-p9_fd_cancel()
with lock, run on conditional status
BOOM
So, maybe we can try to see if it's the problem of syncing the status
between different threads or if it's more but idk.
>> Signed-off-by: Tomas Bortoli <tomasbortoli@gmail.com>
>> Reported-by: syzbot+735d926e9d1317c3310c@syzkaller.appspotmail.com
>> ---
>>
>> net/9p/trans_fd.c | 10 ++++++----
>> 1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/net/9p/trans_fd.c b/net/9p/trans_fd.c
>> index a64b01c56e30..131bb1f059e6 100644
>> --- a/net/9p/trans_fd.c
>> +++ b/net/9p/trans_fd.c
>> @@ -223,7 +223,9 @@ static void p9_conn_cancel(struct p9_conn *m, int err)
>>
>> list_for_each_entry_safe(req, rtmp, &cancel_list, req_list) {
>> p9_debug(P9_DEBUG_ERROR, "call back req %p\n", req);
>> - list_del(&req->req_list);
>> + spin_lock_irqsave(&m->client->lock, flags);
>> + list_del_init(&req->req_list);
>> + spin_unlock_irqrestore(&m->client->lock, flags);
>
> Just locking around one item if you're afraid it might change won't be
> enough - list_for_each_entry_safe is only "safe" from removing the
> current element from the list yourself, not from other threads messing
> with it, so you'd need to lock around the whole loop if that's what
> you're protecting against.
>
Right, I thought I had to unlock before p9_client_cb() as here:
https://github.com/torvalds/linux/blob/master/net/9p/trans_fd.c#L375
However, also locking the client mutex for the whole loop doesn't seem
to give problems. See patch below
> (Also, since I've taken the other patchs to change spin locks on
> client->lock to spin_lock instead of spin_lock_irqsave, please use that
> function for new locking of that variable - in general just basing your
> patchs off linux-next's master branch is a good idea.)
>
>> if (!req->t_err)
>> req->t_err = err;
>> p9_client_cb(m->client, req, REQ_STATUS_ERROR);
>> @@ -369,7 +371,7 @@ static void p9_read_work(struct work_struct *work)
>> spin_lock(&m->client->lock);
>> if (m->req->status != REQ_STATUS_ERROR)
>> status = REQ_STATUS_RCVD;
>> - list_del(&m->req->req_list);
>> + list_del_init(&m->req->req_list);
>> spin_unlock(&m->client->lock);
>> p9_client_cb(m->client, m->req, status);
>> m->rc.sdata = NULL;
>> @@ -684,7 +686,7 @@ static int p9_fd_cancel(struct p9_client *client, struct p9_req_t *req)
>> spin_lock(&client->lock);
>>
>> if (req->status == REQ_STATUS_UNSENT) {
>> - list_del(&req->req_list);
>> + list_del_init(&req->req_list);
>> req->status = REQ_STATUS_FLSHD;
>> ret = 0;
>> }
>> @@ -701,7 +703,7 @@ static int p9_fd_cancelled(struct p9_client *client, struct p9_req_t *req)
>> * remove it from the list.
>> */
>> spin_lock(&client->lock);
>> - list_del(&req->req_list);
>> + list_del_init(&req->req_list);
>> spin_unlock(&client->lock);
>>
>> return 0;
diff --git a/net/9p/trans_fd.c b/net/9p/trans_fd.c
index a64b01c56e30..2ae5f03d872f 100644
--- a/net/9p/trans_fd.c
+++ b/net/9p/trans_fd.c
@@ -199,15 +199,14 @@ static void p9_mux_poll_stop(struct p9_conn *m)
static void p9_conn_cancel(struct p9_conn *m, int err)
{
struct p9_req_t *req, *rtmp;
- unsigned long flags;
LIST_HEAD(cancel_list);
p9_debug(P9_DEBUG_ERROR, "mux %p err %d\n", m, err);
- spin_lock_irqsave(&m->client->lock, flags);
+ spin_lock(&m->client->lock);
if (m->err) {
- spin_unlock_irqrestore(&m->client->lock, flags);
+ spin_unlock(&m->client->lock);
return;
}
@@ -223,11 +222,12 @@ static void p9_conn_cancel(struct p9_conn *m, int err)
list_for_each_entry_safe(req, rtmp, &cancel_list, req_list) {
p9_debug(P9_DEBUG_ERROR, "call back req %p\n", req);
- list_del(&req->req_list);
+ list_del_init(&req->req_list);
if (!req->t_err)
req->t_err = err;
p9_client_cb(m->client, req, REQ_STATUS_ERROR);
}
+ spin_unlock(&m->client->lock);
}
static __poll_t
@@ -369,7 +369,7 @@ static void p9_read_work(struct work_struct *work)
spin_lock(&m->client->lock);
if (m->req->status != REQ_STATUS_ERROR)
status = REQ_STATUS_RCVD;
- list_del(&m->req->req_list);
+ list_del_init(&m->req->req_list);
spin_unlock(&m->client->lock);
p9_client_cb(m->client, m->req, status);
m->rc.sdata = NULL;
@@ -684,7 +684,7 @@ static int p9_fd_cancel(struct p9_client *client,
struct p9_req_t *req)
spin_lock(&client->lock);
if (req->status == REQ_STATUS_UNSENT) {
- list_del(&req->req_list);
+ list_del_init(&req->req_list);
req->status = REQ_STATUS_FLSHD;
ret = 0;
}
@@ -701,7 +701,7 @@ static int p9_fd_cancelled(struct p9_client *client,
struct p9_req_t *req)
* remove it from the list.
*/
spin_lock(&client->lock);
- list_del(&req->req_list);
+ list_del_init(&req->req_list);
spin_unlock(&client->lock);
return 0;
^ permalink raw reply related
* [PATCH nf] netfilter: nf_tables: move dumper state alloation into ->start
From: Florian Westphal @ 2018-07-23 10:47 UTC (permalink / raw)
To: netfilter-devel; +Cc: netdev, cscnull, Florian Westphal
Shaochun Chen points out we leak dumper filter state allocations
stored in dump_control->data in case there is an error before netlink sets
cb_running (after which ->done will be called at some point).
In order to fix this, add .start functions and do the allocations
there.
->done is going to clean up, and in case error occurs before
->start invocation no cleanups need to be done anymore.
Reported-by: shaochun chen <cscnull@gmail.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
There are other problematic spots outside of nf_tables too which
will need similar changes.
net/netfilter/nf_tables_api.c | 219 ++++++++++++++++++++++--------------------
1 file changed, 115 insertions(+), 104 deletions(-)
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index d7b9748e338e..f5745e4c6513 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -2271,6 +2271,39 @@ static int nf_tables_dump_rules(struct sk_buff *skb,
return skb->len;
}
+static int nf_tables_dump_rules_start(struct netlink_callback *cb)
+{
+ const struct nlattr * const *nla = cb->data;
+ struct nft_rule_dump_ctx *ctx = NULL;
+
+ if (nla[NFTA_RULE_TABLE] || nla[NFTA_RULE_CHAIN]) {
+ ctx = kzalloc(sizeof(*ctx), GFP_ATOMIC);
+ if (!ctx)
+ return -ENOMEM;
+
+ if (nla[NFTA_RULE_TABLE]) {
+ ctx->table = nla_strdup(nla[NFTA_RULE_TABLE],
+ GFP_ATOMIC);
+ if (!ctx->table) {
+ kfree(ctx);
+ return -ENOMEM;
+ }
+ }
+ if (nla[NFTA_RULE_CHAIN]) {
+ ctx->chain = nla_strdup(nla[NFTA_RULE_CHAIN],
+ GFP_ATOMIC);
+ if (!ctx->chain) {
+ kfree(ctx->table);
+ kfree(ctx);
+ return -ENOMEM;
+ }
+ }
+ }
+
+ cb->data = ctx;
+ return 0;
+}
+
static int nf_tables_dump_rules_done(struct netlink_callback *cb)
{
struct nft_rule_dump_ctx *ctx = cb->data;
@@ -2300,38 +2333,13 @@ static int nf_tables_getrule(struct net *net, struct sock *nlsk,
if (nlh->nlmsg_flags & NLM_F_DUMP) {
struct netlink_dump_control c = {
+ .start= nf_tables_dump_rules_start,
.dump = nf_tables_dump_rules,
.done = nf_tables_dump_rules_done,
.module = THIS_MODULE,
+ .data = (void *)nla,
};
- if (nla[NFTA_RULE_TABLE] || nla[NFTA_RULE_CHAIN]) {
- struct nft_rule_dump_ctx *ctx;
-
- ctx = kzalloc(sizeof(*ctx), GFP_ATOMIC);
- if (!ctx)
- return -ENOMEM;
-
- if (nla[NFTA_RULE_TABLE]) {
- ctx->table = nla_strdup(nla[NFTA_RULE_TABLE],
- GFP_ATOMIC);
- if (!ctx->table) {
- kfree(ctx);
- return -ENOMEM;
- }
- }
- if (nla[NFTA_RULE_CHAIN]) {
- ctx->chain = nla_strdup(nla[NFTA_RULE_CHAIN],
- GFP_ATOMIC);
- if (!ctx->chain) {
- kfree(ctx->table);
- kfree(ctx);
- return -ENOMEM;
- }
- }
- c.data = ctx;
- }
-
return nft_netlink_dump_start_rcu(nlsk, skb, nlh, &c);
}
@@ -3181,6 +3189,18 @@ static int nf_tables_dump_sets(struct sk_buff *skb, struct netlink_callback *cb)
return skb->len;
}
+static int nf_tables_dump_sets_start(struct netlink_callback *cb)
+{
+ struct nft_ctx *ctx_dump = NULL;
+
+ ctx_dump = kmemdup(cb->data, sizeof(*ctx_dump), GFP_ATOMIC);
+ if (ctx_dump == NULL)
+ return -ENOMEM;
+
+ cb->data = ctx_dump;
+ return 0;
+}
+
static int nf_tables_dump_sets_done(struct netlink_callback *cb)
{
kfree(cb->data);
@@ -3208,18 +3228,12 @@ static int nf_tables_getset(struct net *net, struct sock *nlsk,
if (nlh->nlmsg_flags & NLM_F_DUMP) {
struct netlink_dump_control c = {
+ .start = nf_tables_dump_sets_start,
.dump = nf_tables_dump_sets,
.done = nf_tables_dump_sets_done,
+ .data = &ctx,
.module = THIS_MODULE,
};
- struct nft_ctx *ctx_dump;
-
- ctx_dump = kmalloc(sizeof(*ctx_dump), GFP_ATOMIC);
- if (ctx_dump == NULL)
- return -ENOMEM;
-
- *ctx_dump = ctx;
- c.data = ctx_dump;
return nft_netlink_dump_start_rcu(nlsk, skb, nlh, &c);
}
@@ -3869,6 +3883,15 @@ static int nf_tables_dump_set(struct sk_buff *skb, struct netlink_callback *cb)
return -ENOSPC;
}
+static int nf_tables_dump_set_start(struct netlink_callback *cb)
+{
+ struct nft_set_dump_ctx *dump_ctx = cb->data;
+
+ cb->data = kmemdup(dump_ctx, sizeof(*dump_ctx), GFP_ATOMIC);
+
+ return cb->data ? 0 : -ENOMEM;
+}
+
static int nf_tables_dump_set_done(struct netlink_callback *cb)
{
kfree(cb->data);
@@ -4022,20 +4045,17 @@ static int nf_tables_getsetelem(struct net *net, struct sock *nlsk,
if (nlh->nlmsg_flags & NLM_F_DUMP) {
struct netlink_dump_control c = {
+ .start = nf_tables_dump_set_start,
.dump = nf_tables_dump_set,
.done = nf_tables_dump_set_done,
.module = THIS_MODULE,
};
- struct nft_set_dump_ctx *dump_ctx;
-
- dump_ctx = kmalloc(sizeof(*dump_ctx), GFP_ATOMIC);
- if (!dump_ctx)
- return -ENOMEM;
-
- dump_ctx->set = set;
- dump_ctx->ctx = ctx;
+ struct nft_set_dump_ctx dump_ctx = {
+ .set = set,
+ .ctx = ctx,
+ };
- c.data = dump_ctx;
+ c.data = &dump_ctx;
return nft_netlink_dump_start_rcu(nlsk, skb, nlh, &c);
}
@@ -4995,38 +5015,42 @@ static int nf_tables_dump_obj(struct sk_buff *skb, struct netlink_callback *cb)
return skb->len;
}
-static int nf_tables_dump_obj_done(struct netlink_callback *cb)
+static int nf_tables_dump_obj_start(struct netlink_callback *cb)
{
- struct nft_obj_filter *filter = cb->data;
+ const struct nlattr * const *nla = cb->data;
+ struct nft_obj_filter *filter = NULL;
- if (filter) {
- kfree(filter->table);
- kfree(filter);
+ if (nla[NFTA_OBJ_TABLE] || nla[NFTA_OBJ_TYPE]) {
+ filter = kzalloc(sizeof(*filter), GFP_ATOMIC);
+ if (!filter)
+ return -ENOMEM;
+
+ if (nla[NFTA_OBJ_TABLE]) {
+ filter->table = nla_strdup(nla[NFTA_OBJ_TABLE], GFP_ATOMIC);
+ if (!filter->table) {
+ kfree(filter);
+ return -ENOMEM;
+ }
+ }
+
+ if (nla[NFTA_OBJ_TYPE])
+ filter->type = ntohl(nla_get_be32(nla[NFTA_OBJ_TYPE]));
}
+ cb->data = filter;
return 0;
}
-static struct nft_obj_filter *
-nft_obj_filter_alloc(const struct nlattr * const nla[])
+static int nf_tables_dump_obj_done(struct netlink_callback *cb)
{
- struct nft_obj_filter *filter;
-
- filter = kzalloc(sizeof(*filter), GFP_ATOMIC);
- if (!filter)
- return ERR_PTR(-ENOMEM);
+ struct nft_obj_filter *filter = cb->data;
- if (nla[NFTA_OBJ_TABLE]) {
- filter->table = nla_strdup(nla[NFTA_OBJ_TABLE], GFP_ATOMIC);
- if (!filter->table) {
- kfree(filter);
- return ERR_PTR(-ENOMEM);
- }
+ if (filter) {
+ kfree(filter->table);
+ kfree(filter);
}
- if (nla[NFTA_OBJ_TYPE])
- filter->type = ntohl(nla_get_be32(nla[NFTA_OBJ_TYPE]));
- return filter;
+ return 0;
}
/* called with rcu_read_lock held */
@@ -5047,21 +5071,13 @@ static int nf_tables_getobj(struct net *net, struct sock *nlsk,
if (nlh->nlmsg_flags & NLM_F_DUMP) {
struct netlink_dump_control c = {
+ .start = nf_tables_dump_obj_start,
.dump = nf_tables_dump_obj,
.done = nf_tables_dump_obj_done,
.module = THIS_MODULE,
+ .data = (void *)nla,
};
- if (nla[NFTA_OBJ_TABLE] ||
- nla[NFTA_OBJ_TYPE]) {
- struct nft_obj_filter *filter;
-
- filter = nft_obj_filter_alloc(nla);
- if (IS_ERR(filter))
- return -ENOMEM;
-
- c.data = filter;
- }
return nft_netlink_dump_start_rcu(nlsk, skb, nlh, &c);
}
@@ -5667,37 +5683,39 @@ static int nf_tables_dump_flowtable(struct sk_buff *skb,
return skb->len;
}
-static int nf_tables_dump_flowtable_done(struct netlink_callback *cb)
+static int nf_tables_dump_flowtable_start(struct netlink_callback *cb)
{
- struct nft_flowtable_filter *filter = cb->data;
+ const struct nlattr * const *nla = cb->data;
+ struct nft_flowtable_filter *filter = NULL;
- if (!filter)
- return 0;
+ if (nla[NFTA_FLOWTABLE_TABLE]) {
+ filter = kzalloc(sizeof(*filter), GFP_ATOMIC);
+ if (!filter)
+ return -ENOMEM;
- kfree(filter->table);
- kfree(filter);
+ filter->table = nla_strdup(nla[NFTA_FLOWTABLE_TABLE],
+ GFP_ATOMIC);
+ if (!filter->table) {
+ kfree(filter);
+ return -ENOMEM;
+ }
+ }
+ cb->data = filter;
return 0;
}
-static struct nft_flowtable_filter *
-nft_flowtable_filter_alloc(const struct nlattr * const nla[])
+static int nf_tables_dump_flowtable_done(struct netlink_callback *cb)
{
- struct nft_flowtable_filter *filter;
+ struct nft_flowtable_filter *filter = cb->data;
- filter = kzalloc(sizeof(*filter), GFP_ATOMIC);
if (!filter)
- return ERR_PTR(-ENOMEM);
+ return 0;
- if (nla[NFTA_FLOWTABLE_TABLE]) {
- filter->table = nla_strdup(nla[NFTA_FLOWTABLE_TABLE],
- GFP_ATOMIC);
- if (!filter->table) {
- kfree(filter);
- return ERR_PTR(-ENOMEM);
- }
- }
- return filter;
+ kfree(filter->table);
+ kfree(filter);
+
+ return 0;
}
/* called with rcu_read_lock held */
@@ -5717,20 +5735,13 @@ static int nf_tables_getflowtable(struct net *net, struct sock *nlsk,
if (nlh->nlmsg_flags & NLM_F_DUMP) {
struct netlink_dump_control c = {
+ .start = nf_tables_dump_flowtable_start,
.dump = nf_tables_dump_flowtable,
.done = nf_tables_dump_flowtable_done,
.module = THIS_MODULE,
+ .data = (void *)nla,
};
- if (nla[NFTA_FLOWTABLE_TABLE]) {
- struct nft_flowtable_filter *filter;
-
- filter = nft_flowtable_filter_alloc(nla);
- if (IS_ERR(filter))
- return -ENOMEM;
-
- c.data = filter;
- }
return nft_netlink_dump_start_rcu(nlsk, skb, nlh, &c);
}
--
2.16.4
^ permalink raw reply related
* Re: [PATCH] netlink: fix memory leak of dump
From: Florian Westphal @ 2018-07-23 10:51 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: Florian Westphal, David Miller, cscnull, kadlec, johannes.berg,
Jason, ktkhai, lucien.xin, xiyou.wangcong, dsahern,
netfilter-devel, tom, netdev, linux-kernel
In-Reply-To: <20180723094742.gg6e73l6aknghal7@salvia>
Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Mon, Jul 23, 2018 at 11:42:28AM +0200, Florian Westphal wrote:
> We can also add another scratchpad area, similar to the ->cb[x] area
> that can be initialized before netlink_dump_start()? So we don't need
> the data pointer.
>
> By passing the array of attributes, we'll need to do attribute parsing
> over and over again from each netlink_dump() call.
Its still done once, parsing is only delayed/moved to ->start().
Now:
nla_parse
control->data = kmalloc() + init
netlink_dump_start()
->start() /* if it returns 0, done() will be called eventually
netlink_dump()
->done()
free(cb->data);
Proposed fix:
control->data = &on_stack;
netlink_dump_start()
->start()
nla_parse
cb->data = kmalloc() + init
netlink_dump()
->done()
free(cb->data);
I've submitted a patch for nf_tables, let me know if you have a better
idea or see a problem with it.
^ permalink raw reply
* [PATCH net-next 1/5] net/smc: provide smc mode in smc_diag.c
From: Ursula Braun @ 2018-07-23 11:53 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-s390, schwidefsky, heiko.carstens, raspl,
linux-kernel
In-Reply-To: <20180723115312.54694-1-ubraun@linux.ibm.com>
From: Karsten Graul <kgraul@linux.ibm.com>
Rename field diag_fallback into diag_mode and set the smc mode of a
connection explicitly.
Signed-off-by: Karsten Graul <kgraul@linux.ibm.com>
Signed-off-by: Ursula Braun <ubraun@linux.ibm.com>
---
include/uapi/linux/smc_diag.h | 9 ++++++++-
net/smc/smc_diag.c | 7 ++++++-
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/include/uapi/linux/smc_diag.h b/include/uapi/linux/smc_diag.h
index 92be255e534c..48ae3ee22b2d 100644
--- a/include/uapi/linux/smc_diag.h
+++ b/include/uapi/linux/smc_diag.h
@@ -20,7 +20,7 @@ struct smc_diag_req {
struct smc_diag_msg {
__u8 diag_family;
__u8 diag_state;
- __u8 diag_fallback;
+ __u8 diag_mode;
__u8 diag_shutdown;
struct inet_diag_sockid id;
@@ -28,6 +28,13 @@ struct smc_diag_msg {
__u64 diag_inode;
};
+/* Mode of a connection */
+enum {
+ SMC_DIAG_MODE_SMCR,
+ SMC_DIAG_MODE_FALLBACK_TCP,
+ SMC_DIAG_MODE_SMCD,
+};
+
/* Extensions */
enum {
diff --git a/net/smc/smc_diag.c b/net/smc/smc_diag.c
index 6d83eef1b743..d772cd10297e 100644
--- a/net/smc/smc_diag.c
+++ b/net/smc/smc_diag.c
@@ -91,7 +91,12 @@ static int __smc_diag_dump(struct sock *sk, struct sk_buff *skb,
r = nlmsg_data(nlh);
smc_diag_msg_common_fill(r, sk);
r->diag_state = sk->sk_state;
- r->diag_fallback = smc->use_fallback;
+ if (smc->use_fallback)
+ r->diag_mode = SMC_DIAG_MODE_FALLBACK_TCP;
+ else if (smc->conn.lgr && smc->conn.lgr->is_smcd)
+ r->diag_mode = SMC_DIAG_MODE_SMCD;
+ else
+ r->diag_mode = SMC_DIAG_MODE_SMCR;
user_ns = sk_user_ns(NETLINK_CB(cb->skb).sk);
if (smc_diag_msg_attrs_fill(sk, skb, r, user_ns))
goto errout;
--
2.16.4
^ permalink raw reply related
* [PATCH net-next 2/5] net/smc: eliminate cursor read and write calls
From: Ursula Braun @ 2018-07-23 11:53 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-s390, schwidefsky, heiko.carstens, raspl,
linux-kernel
In-Reply-To: <20180723115312.54694-1-ubraun@linux.ibm.com>
From: Stefan Raspl <raspl@linux.ibm.com>
The functions to read and write cursors are exclusively used to copy
cursors. Therefore switch to a respective function instead.
Signed-off-by: Stefan Raspl <raspl@linux.ibm.com>
Signed-off-by: Ursula Braun <ubraun@linux.ibm.com>
---
net/smc/af_smc.c | 8 ++------
net/smc/smc_cdc.c | 33 ++++++++++++---------------------
net/smc/smc_cdc.h | 43 +++++++++++++++----------------------------
net/smc/smc_rx.c | 15 ++++-----------
net/smc/smc_tx.c | 46 +++++++++++++---------------------------------
net/smc/smc_tx.h | 4 ++--
6 files changed, 48 insertions(+), 101 deletions(-)
diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
index 143b2220c0c8..7fc810ec31c5 100644
--- a/net/smc/af_smc.c
+++ b/net/smc/af_smc.c
@@ -1755,12 +1755,8 @@ static int smc_ioctl(struct socket *sock, unsigned int cmd,
smc->sk.sk_state == SMC_CLOSED) {
answ = 0;
} else {
- smc_curs_write(&cons,
- smc_curs_read(&conn->local_tx_ctrl.cons, conn),
- conn);
- smc_curs_write(&urg,
- smc_curs_read(&conn->urg_curs, conn),
- conn);
+ smc_curs_copy(&cons, &conn->local_tx_ctrl.cons, conn);
+ smc_curs_copy(&urg, &conn->urg_curs, conn);
answ = smc_curs_diff(conn->rmb_desc->len,
&cons, &urg) == 1;
}
diff --git a/net/smc/smc_cdc.c b/net/smc/smc_cdc.c
index 621d8cca570b..f3a1497953ee 100644
--- a/net/smc/smc_cdc.c
+++ b/net/smc/smc_cdc.c
@@ -34,14 +34,15 @@ static void smc_cdc_tx_handler(struct smc_wr_tx_pend_priv *pnd_snd,
enum ib_wc_status wc_status)
{
struct smc_cdc_tx_pend *cdcpend = (struct smc_cdc_tx_pend *)pnd_snd;
+ struct smc_connection *conn = cdcpend->conn;
struct smc_sock *smc;
int diff;
- if (!cdcpend->conn)
+ if (!conn)
/* already dismissed */
return;
- smc = container_of(cdcpend->conn, struct smc_sock, conn);
+ smc = container_of(conn, struct smc_sock, conn);
bh_lock_sock(&smc->sk);
if (!wc_status) {
diff = smc_curs_diff(cdcpend->conn->sndbuf_desc->len,
@@ -52,9 +53,7 @@ static void smc_cdc_tx_handler(struct smc_wr_tx_pend_priv *pnd_snd,
atomic_add(diff, &cdcpend->conn->sndbuf_space);
/* guarantee 0 <= sndbuf_space <= sndbuf_desc->len */
smp_mb__after_atomic();
- smc_curs_write(&cdcpend->conn->tx_curs_fin,
- smc_curs_read(&cdcpend->cursor, cdcpend->conn),
- cdcpend->conn);
+ smc_curs_copy(&conn->tx_curs_fin, &cdcpend->cursor, conn);
}
smc_tx_sndbuf_nonfull(smc);
bh_unlock_sock(&smc->sk);
@@ -110,9 +109,8 @@ int smc_cdc_msg_send(struct smc_connection *conn,
&conn->local_tx_ctrl, conn);
rc = smc_wr_tx_send(link, (struct smc_wr_tx_pend_priv *)pend);
if (!rc)
- smc_curs_write(&conn->rx_curs_confirmed,
- smc_curs_read(&conn->local_tx_ctrl.cons, conn),
- conn);
+ smc_curs_copy(&conn->rx_curs_confirmed,
+ &conn->local_tx_ctrl.cons, conn);
return rc;
}
@@ -194,8 +192,8 @@ int smcd_cdc_msg_send(struct smc_connection *conn)
rc = smcd_tx_ism_write(conn, &cdc, sizeof(cdc), 0, 1);
if (rc)
return rc;
- smc_curs_write(&conn->rx_curs_confirmed,
- smc_curs_read(&conn->local_tx_ctrl.cons, conn), conn);
+ smc_curs_copy(&conn->rx_curs_confirmed, &conn->local_tx_ctrl.cons,
+ conn);
/* Calculate transmitted data and increment free send buffer space */
diff = smc_curs_diff(conn->sndbuf_desc->len, &conn->tx_curs_fin,
&conn->tx_curs_sent);
@@ -204,8 +202,7 @@ int smcd_cdc_msg_send(struct smc_connection *conn)
atomic_add(diff, &conn->sndbuf_space);
/* guarantee 0 <= sndbuf_space <= sndbuf_desc->len */
smp_mb__after_atomic();
- smc_curs_write(&conn->tx_curs_fin,
- smc_curs_read(&conn->tx_curs_sent, conn), conn);
+ smc_curs_copy(&conn->tx_curs_fin, &conn->tx_curs_sent, conn);
smc_tx_sndbuf_nonfull(smc);
return rc;
@@ -225,9 +222,7 @@ static void smc_cdc_handle_urg_data_arrival(struct smc_sock *smc,
char *base;
/* new data included urgent business */
- smc_curs_write(&conn->urg_curs,
- smc_curs_read(&conn->local_rx_ctrl.prod, conn),
- conn);
+ smc_curs_copy(&conn->urg_curs, &conn->local_rx_ctrl.prod, conn);
conn->urg_state = SMC_URG_VALID;
if (!sock_flag(&smc->sk, SOCK_URGINLINE))
/* we'll skip the urgent byte, so don't account for it */
@@ -247,12 +242,8 @@ static void smc_cdc_msg_recv_action(struct smc_sock *smc,
struct smc_connection *conn = &smc->conn;
int diff_cons, diff_prod;
- smc_curs_write(&prod_old,
- smc_curs_read(&conn->local_rx_ctrl.prod, conn),
- conn);
- smc_curs_write(&cons_old,
- smc_curs_read(&conn->local_rx_ctrl.cons, conn),
- conn);
+ smc_curs_copy(&prod_old, &conn->local_rx_ctrl.prod, conn);
+ smc_curs_copy(&cons_old, &conn->local_rx_ctrl.cons, conn);
smc_cdc_msg_to_host(&conn->local_rx_ctrl, cdc, conn);
diff_cons = smc_curs_diff(conn->peer_rmbe_size, &cons_old,
diff --git a/net/smc/smc_cdc.h b/net/smc/smc_cdc.h
index 8fbce4fee3e4..934df4473a7c 100644
--- a/net/smc/smc_cdc.h
+++ b/net/smc/smc_cdc.h
@@ -104,47 +104,34 @@ static inline u64 smc_curs_read(union smc_host_cursor *curs,
#endif
}
-static inline u64 smc_curs_read_net(union smc_cdc_cursor *curs,
- struct smc_connection *conn)
-{
-#ifndef KERNEL_HAS_ATOMIC64
- unsigned long flags;
- u64 ret;
-
- spin_lock_irqsave(&conn->acurs_lock, flags);
- ret = curs->acurs;
- spin_unlock_irqrestore(&conn->acurs_lock, flags);
- return ret;
-#else
- return atomic64_read(&curs->acurs);
-#endif
-}
-
-static inline void smc_curs_write(union smc_host_cursor *curs, u64 val,
- struct smc_connection *conn)
+/* Copy cursor src into tgt */
+static inline void smc_curs_copy(union smc_host_cursor *tgt,
+ union smc_host_cursor *src,
+ struct smc_connection *conn)
{
#ifndef KERNEL_HAS_ATOMIC64
unsigned long flags;
spin_lock_irqsave(&conn->acurs_lock, flags);
- curs->acurs = val;
+ tgt->acurs = src->acurs;
spin_unlock_irqrestore(&conn->acurs_lock, flags);
#else
- atomic64_set(&curs->acurs, val);
+ atomic64_set(&tgt->acurs, atomic64_read(&src->acurs));
#endif
}
-static inline void smc_curs_write_net(union smc_cdc_cursor *curs, u64 val,
- struct smc_connection *conn)
+static inline void smc_curs_copy_net(union smc_cdc_cursor *tgt,
+ union smc_cdc_cursor *src,
+ struct smc_connection *conn)
{
#ifndef KERNEL_HAS_ATOMIC64
unsigned long flags;
spin_lock_irqsave(&conn->acurs_lock, flags);
- curs->acurs = val;
+ tgt->acurs = src->acurs;
spin_unlock_irqrestore(&conn->acurs_lock, flags);
#else
- atomic64_set(&curs->acurs, val);
+ atomic64_set(&tgt->acurs, atomic64_read(&src->acurs));
#endif
}
@@ -179,7 +166,7 @@ static inline void smc_host_cursor_to_cdc(union smc_cdc_cursor *peer,
{
union smc_host_cursor temp;
- smc_curs_write(&temp, smc_curs_read(local, conn), conn);
+ smc_curs_copy(&temp, local, conn);
peer->count = htonl(temp.count);
peer->wrap = htons(temp.wrap);
/* peer->reserved = htons(0); must be ensured by caller */
@@ -206,8 +193,8 @@ static inline void smc_cdc_cursor_to_host(union smc_host_cursor *local,
union smc_host_cursor temp, old;
union smc_cdc_cursor net;
- smc_curs_write(&old, smc_curs_read(local, conn), conn);
- smc_curs_write_net(&net, smc_curs_read_net(peer, conn), conn);
+ smc_curs_copy(&old, local, conn);
+ smc_curs_copy_net(&net, peer, conn);
temp.count = ntohl(net.count);
temp.wrap = ntohs(net.wrap);
if ((old.wrap > temp.wrap) && temp.wrap)
@@ -215,7 +202,7 @@ static inline void smc_cdc_cursor_to_host(union smc_host_cursor *local,
if ((old.wrap == temp.wrap) &&
(old.count > temp.count))
return;
- smc_curs_write(local, smc_curs_read(&temp, conn), conn);
+ smc_curs_copy(local, &temp, conn);
}
static inline void smcr_cdc_msg_to_host(struct smc_host_cdc_msg *local,
diff --git a/net/smc/smc_rx.c b/net/smc/smc_rx.c
index b329803c8339..c99c987097b1 100644
--- a/net/smc/smc_rx.c
+++ b/net/smc/smc_rx.c
@@ -82,8 +82,7 @@ static int smc_rx_update_consumer(struct smc_sock *smc,
}
}
- smc_curs_write(&conn->local_tx_ctrl.cons, smc_curs_read(&cons, conn),
- conn);
+ smc_curs_copy(&conn->local_tx_ctrl.cons, &cons, conn);
/* send consumer cursor update if required */
/* similar to advertising new TCP rcv_wnd if required */
@@ -97,8 +96,7 @@ static void smc_rx_update_cons(struct smc_sock *smc, size_t len)
struct smc_connection *conn = &smc->conn;
union smc_host_cursor cons;
- smc_curs_write(&cons, smc_curs_read(&conn->local_tx_ctrl.cons, conn),
- conn);
+ smc_curs_copy(&cons, &conn->local_tx_ctrl.cons, conn);
smc_rx_update_consumer(smc, cons, len);
}
@@ -245,10 +243,7 @@ static int smc_rx_recv_urg(struct smc_sock *smc, struct msghdr *msg, int len,
if (!(flags & MSG_TRUNC))
rc = memcpy_to_msg(msg, &conn->urg_rx_byte, 1);
len = 1;
- smc_curs_write(&cons,
- smc_curs_read(&conn->local_tx_ctrl.cons,
- conn),
- conn);
+ smc_curs_copy(&cons, &conn->local_tx_ctrl.cons, conn);
if (smc_curs_diff(conn->rmb_desc->len, &cons,
&conn->urg_curs) > 1)
conn->urg_rx_skip_pend = true;
@@ -370,9 +365,7 @@ int smc_rx_recvmsg(struct smc_sock *smc, struct msghdr *msg,
continue;
}
- smc_curs_write(&cons,
- smc_curs_read(&conn->local_tx_ctrl.cons, conn),
- conn);
+ smc_curs_copy(&cons, &conn->local_tx_ctrl.cons, conn);
/* subsequent splice() calls pick up where previous left */
if (splbytes)
smc_curs_add(conn->rmb_desc->len, &cons, splbytes);
diff --git a/net/smc/smc_tx.c b/net/smc/smc_tx.c
index 142bcb134dd6..2f5e324e54b9 100644
--- a/net/smc/smc_tx.c
+++ b/net/smc/smc_tx.c
@@ -181,9 +181,7 @@ int smc_tx_sendmsg(struct smc_sock *smc, struct msghdr *msg, size_t len)
copylen = min_t(size_t, send_remaining, writespace);
/* determine start of sndbuf */
sndbuf_base = conn->sndbuf_desc->cpu_addr;
- smc_curs_write(&prep,
- smc_curs_read(&conn->tx_curs_prep, conn),
- conn);
+ smc_curs_copy(&prep, &conn->tx_curs_prep, conn);
tx_cnt_prep = prep.count;
/* determine chunks where to write into sndbuf */
/* either unwrapped case, or 1st chunk of wrapped case */
@@ -214,9 +212,7 @@ int smc_tx_sendmsg(struct smc_sock *smc, struct msghdr *msg, size_t len)
smc_sndbuf_sync_sg_for_device(conn);
/* update cursors */
smc_curs_add(conn->sndbuf_desc->len, &prep, copylen);
- smc_curs_write(&conn->tx_curs_prep,
- smc_curs_read(&prep, conn),
- conn);
+ smc_curs_copy(&conn->tx_curs_prep, &prep, conn);
/* increased in send tasklet smc_cdc_tx_handler() */
smp_mb__before_atomic();
atomic_sub(copylen, &conn->sndbuf_space);
@@ -417,8 +413,8 @@ static int smc_tx_rdma_writes(struct smc_connection *conn)
int rc;
/* source: sndbuf */
- smc_curs_write(&sent, smc_curs_read(&conn->tx_curs_sent, conn), conn);
- smc_curs_write(&prep, smc_curs_read(&conn->tx_curs_prep, conn), conn);
+ smc_curs_copy(&sent, &conn->tx_curs_sent, conn);
+ smc_curs_copy(&prep, &conn->tx_curs_prep, conn);
/* cf. wmem_alloc - (snd_max - snd_una) */
to_send = smc_curs_diff(conn->sndbuf_desc->len, &sent, &prep);
if (to_send <= 0)
@@ -429,12 +425,8 @@ static int smc_tx_rdma_writes(struct smc_connection *conn)
rmbespace = atomic_read(&conn->peer_rmbe_space);
if (rmbespace <= 0)
return 0;
- smc_curs_write(&prod,
- smc_curs_read(&conn->local_tx_ctrl.prod, conn),
- conn);
- smc_curs_write(&cons,
- smc_curs_read(&conn->local_rx_ctrl.cons, conn),
- conn);
+ smc_curs_copy(&prod, &conn->local_tx_ctrl.prod, conn);
+ smc_curs_copy(&cons, &conn->local_rx_ctrl.cons, conn);
/* if usable snd_wnd closes ask peer to advertise once it opens again */
pflags = &conn->local_tx_ctrl.prod_flags;
@@ -481,14 +473,9 @@ static int smc_tx_rdma_writes(struct smc_connection *conn)
pflags->urg_data_present = 1;
smc_tx_advance_cursors(conn, &prod, &sent, len);
/* update connection's cursors with advanced local cursors */
- smc_curs_write(&conn->local_tx_ctrl.prod,
- smc_curs_read(&prod, conn),
- conn);
+ smc_curs_copy(&conn->local_tx_ctrl.prod, &prod, conn);
/* dst: peer RMBE */
- smc_curs_write(&conn->tx_curs_sent,
- smc_curs_read(&sent, conn),
- conn);
- /* src: local sndbuf */
+ smc_curs_copy(&conn->tx_curs_sent, &sent, conn);/* src: local sndbuf */
return 0;
}
@@ -606,17 +593,11 @@ void smc_tx_consumer_update(struct smc_connection *conn, bool force)
int sender_free = conn->rmb_desc->len;
int to_confirm;
- smc_curs_write(&cons,
- smc_curs_read(&conn->local_tx_ctrl.cons, conn),
- conn);
- smc_curs_write(&cfed,
- smc_curs_read(&conn->rx_curs_confirmed, conn),
- conn);
+ smc_curs_copy(&cons, &conn->local_tx_ctrl.cons, conn);
+ smc_curs_copy(&cfed, &conn->rx_curs_confirmed, conn);
to_confirm = smc_curs_diff(conn->rmb_desc->len, &cfed, &cons);
if (to_confirm > conn->rmbe_update_limit) {
- smc_curs_write(&prod,
- smc_curs_read(&conn->local_rx_ctrl.prod, conn),
- conn);
+ smc_curs_copy(&prod, &conn->local_rx_ctrl.prod, conn);
sender_free = conn->rmb_desc->len -
smc_curs_diff(conn->rmb_desc->len, &prod, &cfed);
}
@@ -632,9 +613,8 @@ void smc_tx_consumer_update(struct smc_connection *conn, bool force)
SMC_TX_WORK_DELAY);
return;
}
- smc_curs_write(&conn->rx_curs_confirmed,
- smc_curs_read(&conn->local_tx_ctrl.cons, conn),
- conn);
+ smc_curs_copy(&conn->rx_curs_confirmed,
+ &conn->local_tx_ctrl.cons, conn);
conn->local_rx_ctrl.prod_flags.cons_curs_upd_req = 0;
}
if (conn->local_rx_ctrl.prod_flags.write_blocked &&
diff --git a/net/smc/smc_tx.h b/net/smc/smc_tx.h
index b22bdc5694c4..07e6ad76224a 100644
--- a/net/smc/smc_tx.h
+++ b/net/smc/smc_tx.h
@@ -22,8 +22,8 @@ static inline int smc_tx_prepared_sends(struct smc_connection *conn)
{
union smc_host_cursor sent, prep;
- smc_curs_write(&sent, smc_curs_read(&conn->tx_curs_sent, conn), conn);
- smc_curs_write(&prep, smc_curs_read(&conn->tx_curs_prep, conn), conn);
+ smc_curs_copy(&sent, &conn->tx_curs_sent, conn);
+ smc_curs_copy(&prep, &conn->tx_curs_prep, conn);
return smc_curs_diff(conn->sndbuf_desc->len, &sent, &prep);
}
--
2.16.4
^ permalink raw reply related
* [PATCH net-next 3/5] net/smc: add function to get link group from link
From: Ursula Braun @ 2018-07-23 11:53 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-s390, schwidefsky, heiko.carstens, raspl,
linux-kernel
In-Reply-To: <20180723115312.54694-1-ubraun@linux.ibm.com>
From: Stefan Raspl <raspl@linux.ibm.com>
Replace a frequently used construct with a more readable variant,
reducing the code. Also might come handy when we start to support
more than a single per link group.
Signed-off-by: Stefan Raspl <raspl@linux.ibm.com>
Signed-off-by: Ursula Braun <ubraun@linux.ibm.com>
---
net/smc/smc_cdc.c | 2 +-
net/smc/smc_core.h | 5 +++++
net/smc/smc_ib.c | 3 +--
net/smc/smc_llc.c | 30 ++++++++----------------------
net/smc/smc_wr.c | 27 +++++----------------------
5 files changed, 20 insertions(+), 47 deletions(-)
diff --git a/net/smc/smc_cdc.c b/net/smc/smc_cdc.c
index f3a1497953ee..a7af2289cdff 100644
--- a/net/smc/smc_cdc.c
+++ b/net/smc/smc_cdc.c
@@ -365,7 +365,7 @@ static void smc_cdc_rx_handler(struct ib_wc *wc, void *buf)
return; /* invalid message */
/* lookup connection */
- lgr = container_of(link, struct smc_link_group, lnk[SMC_SINGLE_LINK]);
+ lgr = smc_get_lgr(link);
read_lock_bh(&lgr->conns_lock);
conn = smc_lgr_find_conn(ntohl(cdc->token), lgr);
read_unlock_bh(&lgr->conns_lock);
diff --git a/net/smc/smc_core.h b/net/smc/smc_core.h
index 8b47e0168fc3..8807865483bb 100644
--- a/net/smc/smc_core.h
+++ b/net/smc/smc_core.h
@@ -266,4 +266,9 @@ int smc_conn_create(struct smc_sock *smc, bool is_smcd, int srv_first_contact,
u64 peer_gid);
void smcd_conn_free(struct smc_connection *conn);
void smc_core_exit(void);
+
+static inline struct smc_link_group *smc_get_lgr(struct smc_link *link)
+{
+ return container_of(link, struct smc_link_group, lnk[SMC_SINGLE_LINK]);
+}
#endif
diff --git a/net/smc/smc_ib.c b/net/smc/smc_ib.c
index 36de2fd76170..4706ab7092a9 100644
--- a/net/smc/smc_ib.c
+++ b/net/smc/smc_ib.c
@@ -112,8 +112,7 @@ int smc_ib_modify_qp_reset(struct smc_link *lnk)
int smc_ib_ready_link(struct smc_link *lnk)
{
- struct smc_link_group *lgr =
- container_of(lnk, struct smc_link_group, lnk[0]);
+ struct smc_link_group *lgr = smc_get_lgr(lnk);
int rc = 0;
rc = smc_ib_modify_qp_init(lnk);
diff --git a/net/smc/smc_llc.c b/net/smc/smc_llc.c
index 5800a6b43d83..b7944aa1ffc3 100644
--- a/net/smc/smc_llc.c
+++ b/net/smc/smc_llc.c
@@ -186,8 +186,7 @@ int smc_llc_send_confirm_link(struct smc_link *link, u8 mac[],
union ib_gid *gid,
enum smc_llc_reqresp reqresp)
{
- struct smc_link_group *lgr = container_of(link, struct smc_link_group,
- lnk[SMC_SINGLE_LINK]);
+ struct smc_link_group *lgr = smc_get_lgr(link);
struct smc_llc_msg_confirm_link *confllc;
struct smc_wr_tx_pend_priv *pend;
struct smc_wr_buf *wr_buf;
@@ -381,11 +380,9 @@ static int smc_llc_send_message(struct smc_link *link, void *llcbuf, int llclen)
static void smc_llc_rx_confirm_link(struct smc_link *link,
struct smc_llc_msg_confirm_link *llc)
{
- struct smc_link_group *lgr;
+ struct smc_link_group *lgr = smc_get_lgr(link);
int conf_rc;
- lgr = container_of(link, struct smc_link_group, lnk[SMC_SINGLE_LINK]);
-
/* RMBE eyecatchers are not supported */
if (llc->hd.flags & SMC_LLC_FLAG_NO_RMBE_EYEC)
conf_rc = 0;
@@ -411,8 +408,7 @@ static void smc_llc_rx_confirm_link(struct smc_link *link,
static void smc_llc_rx_add_link(struct smc_link *link,
struct smc_llc_msg_add_link *llc)
{
- struct smc_link_group *lgr = container_of(link, struct smc_link_group,
- lnk[SMC_SINGLE_LINK]);
+ struct smc_link_group *lgr = smc_get_lgr(link);
if (llc->hd.flags & SMC_LLC_FLAG_RESP) {
if (link->state == SMC_LNK_ACTIVATING)
@@ -442,8 +438,7 @@ static void smc_llc_rx_add_link(struct smc_link *link,
static void smc_llc_rx_delete_link(struct smc_link *link,
struct smc_llc_msg_del_link *llc)
{
- struct smc_link_group *lgr = container_of(link, struct smc_link_group,
- lnk[SMC_SINGLE_LINK]);
+ struct smc_link_group *lgr = smc_get_lgr(link);
if (llc->hd.flags & SMC_LLC_FLAG_RESP) {
if (lgr->role == SMC_SERV)
@@ -476,17 +471,14 @@ static void smc_llc_rx_test_link(struct smc_link *link,
static void smc_llc_rx_confirm_rkey(struct smc_link *link,
struct smc_llc_msg_confirm_rkey *llc)
{
- struct smc_link_group *lgr;
int rc;
- lgr = container_of(link, struct smc_link_group, lnk[SMC_SINGLE_LINK]);
-
if (llc->hd.flags & SMC_LLC_FLAG_RESP) {
link->llc_confirm_rkey_rc = llc->hd.flags &
SMC_LLC_FLAG_RKEY_NEG;
complete(&link->llc_confirm_rkey);
} else {
- rc = smc_rtoken_add(lgr,
+ rc = smc_rtoken_add(smc_get_lgr(link),
llc->rtoken[0].rmb_vaddr,
llc->rtoken[0].rmb_key);
@@ -514,18 +506,15 @@ static void smc_llc_rx_confirm_rkey_cont(struct smc_link *link,
static void smc_llc_rx_delete_rkey(struct smc_link *link,
struct smc_llc_msg_delete_rkey *llc)
{
- struct smc_link_group *lgr;
u8 err_mask = 0;
int i, max;
- lgr = container_of(link, struct smc_link_group, lnk[SMC_SINGLE_LINK]);
-
if (llc->hd.flags & SMC_LLC_FLAG_RESP) {
/* unused as long as we don't send this type of msg */
} else {
max = min_t(u8, llc->num_rkeys, SMC_LLC_DEL_RKEY_MAX);
for (i = 0; i < max; i++) {
- if (smc_rtoken_delete(lgr, llc->rkey[i]))
+ if (smc_rtoken_delete(smc_get_lgr(link), llc->rkey[i]))
err_mask |= 1 << (SMC_LLC_DEL_RKEY_MAX - 1 - i);
}
@@ -583,12 +572,10 @@ static void smc_llc_testlink_work(struct work_struct *work)
struct smc_link *link = container_of(to_delayed_work(work),
struct smc_link, llc_testlink_wrk);
unsigned long next_interval;
- struct smc_link_group *lgr;
unsigned long expire_time;
u8 user_data[16] = { 0 };
int rc;
- lgr = container_of(link, struct smc_link_group, lnk[SMC_SINGLE_LINK]);
if (link->state != SMC_LNK_ACTIVE)
return; /* don't reschedule worker */
expire_time = link->wr_rx_tstamp + link->llc_testlink_time;
@@ -602,7 +589,7 @@ static void smc_llc_testlink_work(struct work_struct *work)
rc = wait_for_completion_interruptible_timeout(&link->llc_testlink_resp,
SMC_LLC_WAIT_TIME);
if (rc <= 0) {
- smc_lgr_terminate(lgr);
+ smc_lgr_terminate(smc_get_lgr(link));
return;
}
next_interval = link->llc_testlink_time;
@@ -613,8 +600,7 @@ static void smc_llc_testlink_work(struct work_struct *work)
int smc_llc_link_init(struct smc_link *link)
{
- struct smc_link_group *lgr = container_of(link, struct smc_link_group,
- lnk[SMC_SINGLE_LINK]);
+ struct smc_link_group *lgr = smc_get_lgr(link);
link->llc_wq = alloc_ordered_workqueue("llc_wq-%x:%x)", WQ_MEM_RECLAIM,
*((u32 *)lgr->id),
link->link_id);
diff --git a/net/smc/smc_wr.c b/net/smc/smc_wr.c
index dbd2605d1962..b6df69756bef 100644
--- a/net/smc/smc_wr.c
+++ b/net/smc/smc_wr.c
@@ -92,8 +92,6 @@ static inline void smc_wr_tx_process_cqe(struct ib_wc *wc)
if (!test_and_clear_bit(pnd_snd_idx, link->wr_tx_mask))
return;
if (wc->status) {
- struct smc_link_group *lgr;
-
for_each_set_bit(i, link->wr_tx_mask, link->wr_tx_cnt) {
/* clear full struct smc_wr_tx_pend including .priv */
memset(&link->wr_tx_pends[i], 0,
@@ -103,9 +101,7 @@ static inline void smc_wr_tx_process_cqe(struct ib_wc *wc)
clear_bit(i, link->wr_tx_mask);
}
/* terminate connections of this link group abnormally */
- lgr = container_of(link, struct smc_link_group,
- lnk[SMC_SINGLE_LINK]);
- smc_lgr_terminate(lgr);
+ smc_lgr_terminate(smc_get_lgr(link));
}
if (pnd_snd.handler)
pnd_snd.handler(&pnd_snd.priv, link, wc->status);
@@ -188,8 +184,7 @@ int smc_wr_tx_get_free_slot(struct smc_link *link,
} else {
struct smc_link_group *lgr;
- lgr = container_of(link, struct smc_link_group,
- lnk[SMC_SINGLE_LINK]);
+ lgr = smc_get_lgr(link);
rc = wait_event_timeout(
link->wr_tx_wait,
list_empty(&lgr->list) || /* lgr terminated */
@@ -250,12 +245,8 @@ int smc_wr_tx_send(struct smc_link *link, struct smc_wr_tx_pend_priv *priv)
rc = ib_post_send(link->roce_qp, &link->wr_tx_ibs[pend->idx],
&failed_wr);
if (rc) {
- struct smc_link_group *lgr =
- container_of(link, struct smc_link_group,
- lnk[SMC_SINGLE_LINK]);
-
smc_wr_tx_put_slot(link, priv);
- smc_lgr_terminate(lgr);
+ smc_lgr_terminate(smc_get_lgr(link));
}
return rc;
}
@@ -283,11 +274,7 @@ int smc_wr_reg_send(struct smc_link *link, struct ib_mr *mr)
SMC_WR_REG_MR_WAIT_TIME);
if (!rc) {
/* timeout - terminate connections */
- struct smc_link_group *lgr;
-
- lgr = container_of(link, struct smc_link_group,
- lnk[SMC_SINGLE_LINK]);
- smc_lgr_terminate(lgr);
+ smc_lgr_terminate(smc_get_lgr(link));
return -EPIPE;
}
if (rc == -ERESTARTSYS)
@@ -380,8 +367,6 @@ static inline void smc_wr_rx_process_cqes(struct ib_wc wc[], int num)
smc_wr_rx_demultiplex(&wc[i]);
smc_wr_rx_post(link); /* refill WR RX */
} else {
- struct smc_link_group *lgr;
-
/* handle status errors */
switch (wc[i].status) {
case IB_WC_RETRY_EXC_ERR:
@@ -390,9 +375,7 @@ static inline void smc_wr_rx_process_cqes(struct ib_wc wc[], int num)
/* terminate connections of this link group
* abnormally
*/
- lgr = container_of(link, struct smc_link_group,
- lnk[SMC_SINGLE_LINK]);
- smc_lgr_terminate(lgr);
+ smc_lgr_terminate(smc_get_lgr(link));
break;
default:
smc_wr_rx_post(link); /* refill WR RX */
--
2.16.4
^ permalink raw reply related
* Re: [PATCH] can: mpc5xxx_can: check of_iomap return before use
From: Nicholas Mc Guire @ 2018-07-23 11:57 UTC (permalink / raw)
To: Marc Kleine-Budde
Cc: Fabio Estevam, Nicholas Mc Guire, Wolfram Sang,
Wolfgang Grandegger, David S. Miller, linux-can, netdev,
linux-kernel
In-Reply-To: <4f81e688-3d5c-6174-5a76-f416ced63cf3@pengutronix.de>
On Mon, Jul 23, 2018 at 01:37:05PM +0200, Marc Kleine-Budde wrote:
> On 07/09/2018 09:28 PM, Fabio Estevam wrote:
> > On Mon, Jul 9, 2018 at 4:16 PM, Nicholas Mc Guire <hofrat@osadl.org> wrote:
> >> of_iompa() can return NULL so that return needs to be checked and NULL
> >
> > s/of_iompa/of_iomap/
> >
> >> treated as failure. While at it also take care of the missing
> >> of_node_put() in the error path.
> >>
> >> Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
> >> Fixes: commit afa17a500a36 ("net/can: add driver for mscan family & mpc52xx_mscan")
> >> ---
> >>
> >> Problem was found by an experimental coccinelle script
> >>
> >> Patch was compiletested with: mpc5200_defconfig + CONFIG_CAN=y,
> >> CONFIG_CAN_MSCAN=y, CONFIG_CAN_MPC5XXX=y
> >> (with a number of sparse warnings not related to the proposed change)
> >>
> >> Patch is against 4.18-rc3 (localversion-next is next-20180706)
> >>
> >> drivers/net/can/mscan/mpc5xxx_can.c | 5 +++++
> >> 1 file changed, 5 insertions(+)
> >>
> >> diff --git a/drivers/net/can/mscan/mpc5xxx_can.c b/drivers/net/can/mscan/mpc5xxx_can.c
> >> index c7427bd..2949a38 100644
> >> --- a/drivers/net/can/mscan/mpc5xxx_can.c
> >> +++ b/drivers/net/can/mscan/mpc5xxx_can.c
> >> @@ -86,6 +86,11 @@ static u32 mpc52xx_can_get_clock(struct platform_device *ofdev,
> >> return 0;
> >> }
> >> cdm = of_iomap(np_cdm, 0);
> >> + if (!cdm) {
> >> + of_node_put(np_cdm);
> >> + dev_err(&ofdev->dev, "can't map clock node!\n");
> >> + return 0;
> >
> > I think you should return an error code here. -ENOMEM maybe?
>
> Can you send an updated version?
>
The way I understand the code the return value will be used
as a frequency - the only valid error valeu thus is 0 here,
so I think the return 0 is correct.
thx!
hofrat
^ permalink raw reply
* Re: [PATCH nf] netfilter: nf_tables: move dumper state alloation into ->start
From: Pablo Neira Ayuso @ 2018-07-23 11:09 UTC (permalink / raw)
To: Florian Westphal; +Cc: netfilter-devel, netdev, cscnull
In-Reply-To: <20180723104714.24008-1-fw@strlen.de>
On Mon, Jul 23, 2018 at 12:47:14PM +0200, Florian Westphal wrote:
> Shaochun Chen points out we leak dumper filter state allocations
> stored in dump_control->data in case there is an error before netlink sets
> cb_running (after which ->done will be called at some point).
>
> In order to fix this, add .start functions and do the allocations
> there.
>
> ->done is going to clean up, and in case error occurs before
> ->start invocation no cleanups need to be done anymore.
LGTM. Thanks Florian
BTW, probably we can add cb->nla with strict typing instead, so we
don't need to use cb->data. This cb->nla would be only valid from
.start().
Then, set cb->nla to NULL after .start() is called. Just to make sure
people don't make wrong assumptions and try to access this from the
classic dump path?
Just an idea.
^ permalink raw reply
* Re: [PATCH nf] netfilter: nf_tables: move dumper state alloation into ->start
From: shaochun chen @ 2018-07-23 11:18 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: Florian Westphal, netfilter-devel, netdev
In-Reply-To: <20180723110911.guinjwo5vglcyw2z@salvia>
[-- Attachment #1: Type: text/plain, Size: 1057 bytes --]
conditions of dump isn't limit in nla, so if you replace cb->data with
cb->nla,
sometimes it's hard to implement user's dump().
2018-07-23 19:09 GMT+08:00 Pablo Neira Ayuso <pablo@netfilter.org>:
> On Mon, Jul 23, 2018 at 12:47:14PM +0200, Florian Westphal wrote:
> > Shaochun Chen points out we leak dumper filter state allocations
> > stored in dump_control->data in case there is an error before netlink
> sets
> > cb_running (after which ->done will be called at some point).
> >
> > In order to fix this, add .start functions and do the allocations
> > there.
> >
> > ->done is going to clean up, and in case error occurs before
> > ->start invocation no cleanups need to be done anymore.
>
> LGTM. Thanks Florian
>
> BTW, probably we can add cb->nla with strict typing instead, so we
> don't need to use cb->data. This cb->nla would be only valid from
> .start().
>
> Then, set cb->nla to NULL after .start() is called. Just to make sure
> people don't make wrong assumptions and try to access this from the
> classic dump path?
>
> Just an idea.
>
[-- Attachment #2: Type: text/html, Size: 1531 bytes --]
^ permalink raw reply
* [PATCHv3 net-next 0/2] route: add support and selftests for directed broadcast forwarding
From: Xin Long @ 2018-07-23 11:51 UTC (permalink / raw)
To: network dev; +Cc: davem, David Ahern, Davide Caratti, idosch
Patch 1/2 is the feature and 2/2 is the selftest. Check the changelog
on each of them to know the details.
v1->v2:
- fix a typo in changelog.
- fix an uapi break that Davide noticed.
- flush route cache when bc_forwarding is changed.
- add the selftest for this patch as Ido's suggestion.
v2->v3:
- fix an incorrect 'if check' in devinet_conf_proc as David Ahern
noticed.
- extend the selftest after one David Ahern fix for vrf.
Xin Long (2):
route: add support for directed broadcast forwarding
selftests: add a selftest for directed broadcast forwarding
include/linux/inetdevice.h | 1 +
include/uapi/linux/ip.h | 1 +
include/uapi/linux/netconf.h | 1 +
net/ipv4/devinet.c | 11 ++
net/ipv4/route.c | 6 +-
.../selftests/net/forwarding/router_broadcast.sh | 215 +++++++++++++++++++++
6 files changed, 234 insertions(+), 1 deletion(-)
create mode 100755 tools/testing/selftests/net/forwarding/router_broadcast.sh
--
2.1.0
^ permalink raw reply
* [PATCHv3 net-next 1/2] route: add support for directed broadcast forwarding
From: Xin Long @ 2018-07-23 11:51 UTC (permalink / raw)
To: network dev; +Cc: davem, David Ahern, Davide Caratti, idosch
In-Reply-To: <cover.1532346504.git.lucien.xin@gmail.com>
This patch implements the feature described in rfc1812#section-5.3.5.2
and rfc2644. It allows the router to forward directed broadcast when
sysctl bc_forwarding is enabled.
Note that this feature could be done by iptables -j TEE, but it would
cause some problems:
- target TEE's gateway param has to be set with a specific address,
and it's not flexible especially when the route wants forward all
directed broadcasts.
- this duplicates the directed broadcasts so this may cause side
effects to applications.
Besides, to keep consistent with other os router like BSD, it's also
necessary to implement it in the route rx path.
Note that route cache needs to be flushed when bc_forwarding is
changed.
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
include/linux/inetdevice.h | 1 +
include/uapi/linux/ip.h | 1 +
include/uapi/linux/netconf.h | 1 +
net/ipv4/devinet.c | 11 +++++++++++
net/ipv4/route.c | 6 +++++-
5 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/include/linux/inetdevice.h b/include/linux/inetdevice.h
index 27650f1..c759d1c 100644
--- a/include/linux/inetdevice.h
+++ b/include/linux/inetdevice.h
@@ -93,6 +93,7 @@ static inline void ipv4_devconf_setall(struct in_device *in_dev)
#define IN_DEV_FORWARD(in_dev) IN_DEV_CONF_GET((in_dev), FORWARDING)
#define IN_DEV_MFORWARD(in_dev) IN_DEV_ANDCONF((in_dev), MC_FORWARDING)
+#define IN_DEV_BFORWARD(in_dev) IN_DEV_ANDCONF((in_dev), BC_FORWARDING)
#define IN_DEV_RPFILTER(in_dev) IN_DEV_MAXCONF((in_dev), RP_FILTER)
#define IN_DEV_SRC_VMARK(in_dev) IN_DEV_ORCONF((in_dev), SRC_VMARK)
#define IN_DEV_SOURCE_ROUTE(in_dev) IN_DEV_ANDCONF((in_dev), \
diff --git a/include/uapi/linux/ip.h b/include/uapi/linux/ip.h
index b24a742..e42d13b 100644
--- a/include/uapi/linux/ip.h
+++ b/include/uapi/linux/ip.h
@@ -168,6 +168,7 @@ enum
IPV4_DEVCONF_IGNORE_ROUTES_WITH_LINKDOWN,
IPV4_DEVCONF_DROP_UNICAST_IN_L2_MULTICAST,
IPV4_DEVCONF_DROP_GRATUITOUS_ARP,
+ IPV4_DEVCONF_BC_FORWARDING,
__IPV4_DEVCONF_MAX
};
diff --git a/include/uapi/linux/netconf.h b/include/uapi/linux/netconf.h
index c84fcdf..fac4edd 100644
--- a/include/uapi/linux/netconf.h
+++ b/include/uapi/linux/netconf.h
@@ -18,6 +18,7 @@ enum {
NETCONFA_PROXY_NEIGH,
NETCONFA_IGNORE_ROUTES_WITH_LINKDOWN,
NETCONFA_INPUT,
+ NETCONFA_BC_FORWARDING,
__NETCONFA_MAX
};
#define NETCONFA_MAX (__NETCONFA_MAX - 1)
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index d7585ab..ea4bd8a 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -1827,6 +1827,8 @@ static int inet_netconf_msgsize_devconf(int type)
size += nla_total_size(4);
if (all || type == NETCONFA_MC_FORWARDING)
size += nla_total_size(4);
+ if (all || type == NETCONFA_BC_FORWARDING)
+ size += nla_total_size(4);
if (all || type == NETCONFA_PROXY_NEIGH)
size += nla_total_size(4);
if (all || type == NETCONFA_IGNORE_ROUTES_WITH_LINKDOWN)
@@ -1873,6 +1875,10 @@ static int inet_netconf_fill_devconf(struct sk_buff *skb, int ifindex,
nla_put_s32(skb, NETCONFA_MC_FORWARDING,
IPV4_DEVCONF(*devconf, MC_FORWARDING)) < 0)
goto nla_put_failure;
+ if ((all || type == NETCONFA_BC_FORWARDING) &&
+ nla_put_s32(skb, NETCONFA_BC_FORWARDING,
+ IPV4_DEVCONF(*devconf, BC_FORWARDING)) < 0)
+ goto nla_put_failure;
if ((all || type == NETCONFA_PROXY_NEIGH) &&
nla_put_s32(skb, NETCONFA_PROXY_NEIGH,
IPV4_DEVCONF(*devconf, PROXY_ARP)) < 0)
@@ -2143,6 +2149,10 @@ static int devinet_conf_proc(struct ctl_table *ctl, int write,
if ((new_value == 0) && (old_value != 0))
rt_cache_flush(net);
+ if (i == IPV4_DEVCONF_BC_FORWARDING - 1 &&
+ new_value != old_value)
+ rt_cache_flush(net);
+
if (i == IPV4_DEVCONF_RP_FILTER - 1 &&
new_value != old_value) {
ifindex = devinet_conf_ifindex(net, cnf);
@@ -2259,6 +2269,7 @@ static struct devinet_sysctl_table {
DEVINET_SYSCTL_COMPLEX_ENTRY(FORWARDING, "forwarding",
devinet_sysctl_forward),
DEVINET_SYSCTL_RO_ENTRY(MC_FORWARDING, "mc_forwarding"),
+ DEVINET_SYSCTL_RW_ENTRY(BC_FORWARDING, "bc_forwarding"),
DEVINET_SYSCTL_RW_ENTRY(ACCEPT_REDIRECTS, "accept_redirects"),
DEVINET_SYSCTL_RW_ENTRY(SECURE_REDIRECTS, "secure_redirects"),
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 1df6e97..b678466 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1996,8 +1996,11 @@ static int ip_route_input_slow(struct sk_buff *skb, __be32 daddr, __be32 saddr,
goto no_route;
}
- if (res->type == RTN_BROADCAST)
+ if (res->type == RTN_BROADCAST) {
+ if (IN_DEV_BFORWARD(in_dev))
+ goto make_route;
goto brd_input;
+ }
if (res->type == RTN_LOCAL) {
err = fib_validate_source(skb, saddr, daddr, tos,
@@ -2014,6 +2017,7 @@ static int ip_route_input_slow(struct sk_buff *skb, __be32 daddr, __be32 saddr,
if (res->type != RTN_UNICAST)
goto martian_destination;
+make_route:
err = ip_mkroute_input(skb, res, in_dev, daddr, saddr, tos, flkeys);
out: return err;
--
2.1.0
^ permalink raw reply related
* [PATCHv3 net-next 2/2] selftests: add a selftest for directed broadcast forwarding
From: Xin Long @ 2018-07-23 11:51 UTC (permalink / raw)
To: network dev; +Cc: davem, David Ahern, Davide Caratti, idosch
In-Reply-To: <cover.1532346504.git.lucien.xin@gmail.com>
As Ido's suggestion, this patch is to add a selftest for directed
broadcast forwarding with vrf. It does the assertion by checking
the src IP of the echo-reply packet in ping_test_from.
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
.../selftests/net/forwarding/router_broadcast.sh | 215 +++++++++++++++++++++
1 file changed, 215 insertions(+)
create mode 100755 tools/testing/selftests/net/forwarding/router_broadcast.sh
diff --git a/tools/testing/selftests/net/forwarding/router_broadcast.sh b/tools/testing/selftests/net/forwarding/router_broadcast.sh
new file mode 100755
index 0000000..f2a5a51
--- /dev/null
+++ b/tools/testing/selftests/net/forwarding/router_broadcast.sh
@@ -0,0 +1,215 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+
+ALL_TESTS="ping_ipv4"
+NUM_NETIFS=6
+source lib.sh
+
+h1_create()
+{
+ vrf_create "vrf-h1"
+ ip link set dev $h1 master vrf-h1
+
+ ip link set dev vrf-h1 up
+ ip link set dev $h1 up
+
+ ip address add 192.0.2.2/24 dev $h1
+
+ ip route add 198.51.100.0/24 vrf vrf-h1 nexthop via 192.0.2.1
+ ip route add 198.51.200.0/24 vrf vrf-h1 nexthop via 192.0.2.1
+}
+
+h1_destroy()
+{
+ ip route del 198.51.200.0/24 vrf vrf-h1
+ ip route del 198.51.100.0/24 vrf vrf-h1
+
+ ip address del 192.0.2.2/24 dev $h1
+
+ ip link set dev $h1 down
+ vrf_destroy "vrf-h1"
+}
+
+h2_create()
+{
+ vrf_create "vrf-h2"
+ ip link set dev $h2 master vrf-h2
+
+ ip link set dev vrf-h2 up
+ ip link set dev $h2 up
+
+ ip address add 198.51.100.2/24 dev $h2
+
+ ip route add 192.0.2.0/24 vrf vrf-h2 nexthop via 198.51.100.1
+ ip route add 198.51.200.0/24 vrf vrf-h2 nexthop via 198.51.100.1
+}
+
+h2_destroy()
+{
+ ip route del 198.51.200.0/24 vrf vrf-h2
+ ip route del 192.0.2.0/24 vrf vrf-h2
+
+ ip address del 198.51.100.2/24 dev $h2
+
+ ip link set dev $h2 down
+ vrf_destroy "vrf-h2"
+}
+
+h3_create()
+{
+ vrf_create "vrf-h3"
+ ip link set dev $h3 master vrf-h3
+
+ ip link set dev vrf-h3 up
+ ip link set dev $h3 up
+
+ ip address add 198.51.200.2/24 dev $h3
+
+ ip route add 192.0.2.0/24 vrf vrf-h3 nexthop via 198.51.200.1
+ ip route add 198.51.100.0/24 vrf vrf-h3 nexthop via 198.51.200.1
+}
+
+h3_destroy()
+{
+ ip route del 198.51.100.0/24 vrf vrf-h3
+ ip route del 192.0.2.0/24 vrf vrf-h3
+
+ ip address del 198.51.200.2/24 dev $h3
+
+ ip link set dev $h3 down
+ vrf_destroy "vrf-h3"
+}
+
+router_create()
+{
+ ip link set dev $rp1 up
+ ip link set dev $rp2 up
+ ip link set dev $rp3 up
+
+ ip address add 192.0.2.1/24 dev $rp1
+
+ ip address add 198.51.100.1/24 dev $rp2
+ ip address add 198.51.200.1/24 dev $rp3
+}
+
+router_destroy()
+{
+ ip address del 198.51.200.1/24 dev $rp3
+ ip address del 198.51.100.1/24 dev $rp2
+
+ ip address del 192.0.2.1/24 dev $rp1
+
+ ip link set dev $rp3 down
+ ip link set dev $rp2 down
+ ip link set dev $rp1 down
+}
+
+setup_prepare()
+{
+ h1=${NETIFS[p1]}
+ rp1=${NETIFS[p2]}
+
+ rp2=${NETIFS[p3]}
+ h2=${NETIFS[p4]}
+
+ rp3=${NETIFS[p5]}
+ h3=${NETIFS[p6]}
+
+ vrf_prepare
+
+ h1_create
+ h2_create
+ h3_create
+
+ router_create
+
+ forwarding_enable
+}
+
+cleanup()
+{
+ pre_cleanup
+
+ forwarding_restore
+
+ router_destroy
+
+ h3_destroy
+ h2_destroy
+ h1_destroy
+
+ vrf_cleanup
+}
+
+bc_forwarding_disable()
+{
+ sysctl_set net.ipv4.conf.all.bc_forwarding 0
+ sysctl_set net.ipv4.conf.$rp1.bc_forwarding 0
+}
+
+bc_forwarding_enable()
+{
+ sysctl_set net.ipv4.conf.all.bc_forwarding 1
+ sysctl_set net.ipv4.conf.$rp1.bc_forwarding 1
+}
+
+bc_forwarding_restore()
+{
+ sysctl_restore net.ipv4.conf.$rp1.bc_forwarding
+ sysctl_restore net.ipv4.conf.all.bc_forwarding
+}
+
+ping_test_from()
+{
+ local oif=$1
+ local dip=$2
+ local from=$3
+ local fail=${4:-0}
+
+ RET=0
+
+ ip vrf exec $(master_name_get $oif) \
+ $PING -I $oif $dip -c 10 -i 0.1 -w 2 -b 2>&1 | grep $from &> /dev/null
+ check_err_fail $fail $?
+ log_test "ping_test_from"
+}
+
+ping_ipv4()
+{
+ sysctl_set net.ipv4.icmp_echo_ignore_broadcasts 0
+
+ bc_forwarding_disable
+ ping_test_from $h1 198.51.100.255 192.0.2.1
+ ping_test_from $h1 198.51.200.255 192.0.2.1
+ ping_test_from $h1 192.0.2.255 192.0.2.1
+ ping_test_from $h1 255.255.255.255 192.0.2.1
+
+ ping_test_from $h2 192.0.2.255 198.51.100.1
+ ping_test_from $h2 198.51.200.255 198.51.100.1
+ ping_test_from $h2 198.51.100.255 198.51.100.1
+ ping_test_from $h2 255.255.255.255 198.51.100.1
+ bc_forwarding_restore
+
+ bc_forwarding_enable
+ ping_test_from $h1 198.51.100.255 198.51.100.2
+ ping_test_from $h1 198.51.200.255 198.51.200.2
+ ping_test_from $h1 192.0.2.255 192.0.2.1 1
+ ping_test_from $h1 255.255.255.255 192.0.2.1
+
+ ping_test_from $h2 192.0.2.255 192.0.2.2
+ ping_test_from $h2 198.51.200.255 198.51.200.2
+ ping_test_from $h2 198.51.100.255 198.51.100.1 1
+ ping_test_from $h2 255.255.255.255 198.51.100.1
+ bc_forwarding_restore
+
+ sysctl_restore net.ipv4.icmp_echo_ignore_broadcasts
+}
+
+trap cleanup EXIT
+
+setup_prepare
+setup_wait
+
+tests_run
+
+exit $EXIT_STATUS
--
2.1.0
^ permalink raw reply related
* [PATCH net-next 0/5] net/smc: patches 2018-07-23
From: Ursula Braun @ 2018-07-23 11:53 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-s390, schwidefsky, heiko.carstens, raspl,
linux-kernel
Dave,
here are some small patches for SMC: Just the first patch contains a
functional change. It allows to differ between the modes SMCR and SMCD
on s390 when monitoring SMC sockets. The remaining patches are cleanups
without functional changes.
Thanks, Ursula
Karsten Graul (1):
net/smc: provide smc mode in smc_diag.c
Stefan Raspl (2):
net/smc: eliminate cursor read and write calls
net/smc: add function to get link group from link
Ursula Braun (2):
net/smc: use DECLARE_BITMAP for rtokens_used_mask
net/smc: remove local variable page in smc_rx_splice()
include/uapi/linux/smc_diag.h | 9 ++++++++-
net/smc/af_smc.c | 8 ++------
net/smc/smc_cdc.c | 35 ++++++++++++--------------------
net/smc/smc_cdc.h | 43 ++++++++++++++--------------------------
net/smc/smc_core.h | 8 ++++++--
net/smc/smc_diag.c | 7 ++++++-
net/smc/smc_ib.c | 3 +--
net/smc/smc_llc.c | 30 ++++++++--------------------
net/smc/smc_rx.c | 19 +++++-------------
net/smc/smc_tx.c | 46 ++++++++++++-------------------------------
net/smc/smc_tx.h | 4 ++--
net/smc/smc_wr.c | 27 +++++--------------------
12 files changed, 84 insertions(+), 155 deletions(-)
--
2.16.4
^ permalink raw reply
* [PATCH net-next 4/5] net/smc: use DECLARE_BITMAP for rtokens_used_mask
From: Ursula Braun @ 2018-07-23 11:53 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-s390, schwidefsky, heiko.carstens, raspl,
linux-kernel
In-Reply-To: <20180723115312.54694-1-ubraun@linux.ibm.com>
From: Ursula Braun <ursula.braun@de.ibm.com>
Link group field tokens_used_mask is a bitmap. Use macro
DECLARE_BITMAP for its definition.
Signed-off-by: Ursula Braun <ubraun@linux.ibm.com>
---
net/smc/smc_core.h | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/net/smc/smc_core.h b/net/smc/smc_core.h
index 8807865483bb..1e8974c50550 100644
--- a/net/smc/smc_core.h
+++ b/net/smc/smc_core.h
@@ -192,8 +192,7 @@ struct smc_link_group {
struct smc_rtoken rtokens[SMC_RMBS_PER_LGR_MAX]
[SMC_LINKS_PER_LGR_MAX];
/* remote addr/key pairs */
- unsigned long rtokens_used_mask[BITS_TO_LONGS
- (SMC_RMBS_PER_LGR_MAX)];
+ DECLARE_BITMAP(rtokens_used_mask, SMC_RMBS_PER_LGR_MAX);
/* used rtoken elements */
};
struct { /* SMC-D */
--
2.16.4
^ permalink raw reply related
* [PATCH net-next 5/5] net/smc: remove local variable page in smc_rx_splice()
From: Ursula Braun @ 2018-07-23 11:53 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-s390, schwidefsky, heiko.carstens, raspl,
linux-kernel
In-Reply-To: <20180723115312.54694-1-ubraun@linux.ibm.com>
From: Ursula Braun <ursula.braun@de.ibm.com>
The page map address is already stored in the RMB descriptor.
There is no need to derive it from the cpu_addr value.
Signed-off-by: Ursula Braun <ubraun@linux.ibm.com>
---
net/smc/smc_rx.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/net/smc/smc_rx.c b/net/smc/smc_rx.c
index c99c987097b1..bbcf0fe4ae10 100644
--- a/net/smc/smc_rx.c
+++ b/net/smc/smc_rx.c
@@ -155,10 +155,8 @@ static int smc_rx_splice(struct pipe_inode_info *pipe, char *src, size_t len,
struct splice_pipe_desc spd;
struct partial_page partial;
struct smc_spd_priv *priv;
- struct page *page;
int bytes;
- page = virt_to_page(smc->conn.rmb_desc->cpu_addr);
priv = kzalloc(sizeof(*priv), GFP_KERNEL);
if (!priv)
return -ENOMEM;
@@ -170,7 +168,7 @@ static int smc_rx_splice(struct pipe_inode_info *pipe, char *src, size_t len,
spd.nr_pages_max = 1;
spd.nr_pages = 1;
- spd.pages = &page;
+ spd.pages = &smc->conn.rmb_desc->pages;
spd.partial = &partial;
spd.ops = &smc_pipe_ops;
spd.spd_release = smc_rx_spd_release;
--
2.16.4
^ permalink raw reply related
* Re: [PATCH] net/p9/trans_fd.c: fix double list_del()
From: Dominique Martinet @ 2018-07-23 12:57 UTC (permalink / raw)
To: Tomas Bortoli
Cc: ericvh, rminnich, lucho, jiangyiwen, davem, v9fs-developer,
netdev, linux-kernel, syzkaller
In-Reply-To: <20180723121902.20201-1-tomasbortoli@gmail.com>
Tomas Bortoli wrote on Mon, Jul 23, 2018:
> A double list_del(&req->req_list) is possible in p9_fd_cancel() as
> shown by Syzbot. To prevent it we have to ensure that we have the
> client->lock when deleting the list. Furthermore, we have to update
> the status of the request before releasing the lock, to prevent the
> race.
Nice, so no need to change the list_del to list_del_init!
I still have a nitpick on the last moved unlock, but it's mostly
aesthetic - the change looks much better to me now.
(Since that will require a v2 I'll be evil and go further than Yiwen
about the commit message: let it breathe a bit! :) I think a line break
before "furthermore" for example will make it easier to read)
>
> Signed-off-by: Tomas Bortoli <tomasbortoli@gmail.com>
> Reported-by: syzbot+735d926e9d1317c3310c@syzkaller.appspotmail.com
> ---
> net/9p/trans_fd.c | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/net/9p/trans_fd.c b/net/9p/trans_fd.c
> index a64b01c56e30..370c6c69a05c 100644
> --- a/net/9p/trans_fd.c
> +++ b/net/9p/trans_fd.c
> @@ -199,15 +199,14 @@ static void p9_mux_poll_stop(struct p9_conn *m)
> static void p9_conn_cancel(struct p9_conn *m, int err)
> {
> struct p9_req_t *req, *rtmp;
> - unsigned long flags;
> LIST_HEAD(cancel_list);
>
> p9_debug(P9_DEBUG_ERROR, "mux %p err %d\n", m, err);
>
> - spin_lock_irqsave(&m->client->lock, flags);
> + spin_lock(&m->client->lock);
>
> if (m->err) {
> - spin_unlock_irqrestore(&m->client->lock, flags);
> + spin_unlock(&m->client->lock);
> return;
> }
>
> @@ -219,7 +218,6 @@ static void p9_conn_cancel(struct p9_conn *m, int err)
> list_for_each_entry_safe(req, rtmp, &m->unsent_req_list, req_list) {
> list_move(&req->req_list, &cancel_list);
> }
> - spin_unlock_irqrestore(&m->client->lock, flags);
>
> list_for_each_entry_safe(req, rtmp, &cancel_list, req_list) {
> p9_debug(P9_DEBUG_ERROR, "call back req %p\n", req);
> @@ -228,6 +226,7 @@ static void p9_conn_cancel(struct p9_conn *m, int err)
> req->t_err = err;
> p9_client_cb(m->client, req, REQ_STATUS_ERROR);
> }
> + spin_unlock(&m->client->lock);
> }
>
> static __poll_t
> @@ -370,12 +369,12 @@ static void p9_read_work(struct work_struct *work)
> if (m->req->status != REQ_STATUS_ERROR)
> status = REQ_STATUS_RCVD;
> list_del(&m->req->req_list);
> - spin_unlock(&m->client->lock);
> p9_client_cb(m->client, m->req, status);
> m->rc.sdata = NULL;
> m->rc.offset = 0;
> m->rc.capacity = 0;
> m->req = NULL;
> + spin_unlock(&m->client->lock);
It took me a while to understand why you extended this lock despite
having just read the commit message, I'd suggest:
- moving the spin_unlock to right after p9_client_cb (afterall that's
what we want, the m->rc and m->req don't need to be protected)
- add a comment before p9_client_cb saying something like 'updates
req->status' or try to explain why it needs to be locked here but other
transports don't need such a lock (they're not dependant on req->status
like this)
--
Dominique
^ permalink raw reply
* [PATCH 06/12] can: xilinx_can: fix device dropping off bus on RX overrun
From: Marc Kleine-Budde @ 2018-07-23 12:58 UTC (permalink / raw)
To: netdev; +Cc: davem, linux-can, kernel, Anssi Hannula, stable,
Marc Kleine-Budde
In-Reply-To: <20180723125843.391-1-mkl@pengutronix.de>
From: Anssi Hannula <anssi.hannula@bitwise.fi>
The xilinx_can driver performs a software reset when an RX overrun is
detected. This causes the device to enter Configuration mode where no
messages are received or transmitted.
The documentation does not mention any need to perform a reset on an RX
overrun, and testing by inducing an RX overflow also indicated that the
device continues to work just fine without a reset.
Remove the software reset.
Tested with the integrated CAN on Zynq-7000 SoC.
Fixes: b1201e44f50b ("can: xilinx CAN controller support")
Signed-off-by: Anssi Hannula <anssi.hannula@bitwise.fi>
Cc: <stable@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
drivers/net/can/xilinx_can.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/can/xilinx_can.c b/drivers/net/can/xilinx_can.c
index 89aec07c225f..389a9603db8c 100644
--- a/drivers/net/can/xilinx_can.c
+++ b/drivers/net/can/xilinx_can.c
@@ -600,7 +600,6 @@ static void xcan_err_interrupt(struct net_device *ndev, u32 isr)
if (isr & XCAN_IXR_RXOFLW_MASK) {
stats->rx_over_errors++;
stats->rx_errors++;
- priv->write_reg(priv, XCAN_SRR_OFFSET, XCAN_SRR_RESET_MASK);
if (skb) {
cf->can_id |= CAN_ERR_CRTL;
cf->data[1] |= CAN_ERR_CRTL_RX_OVERFLOW;
--
2.18.0
^ permalink raw reply related
* [PATCH 07/12] can: xilinx_can: fix RX loop if RXNEMP is asserted without RXOK
From: Marc Kleine-Budde @ 2018-07-23 12:58 UTC (permalink / raw)
To: netdev; +Cc: davem, linux-can, kernel, Anssi Hannula, stable,
Marc Kleine-Budde
In-Reply-To: <20180723125843.391-1-mkl@pengutronix.de>
From: Anssi Hannula <anssi.hannula@bitwise.fi>
If the device gets into a state where RXNEMP (RX FIFO not empty)
interrupt is asserted without RXOK (new frame received successfully)
interrupt being asserted, xcan_rx_poll() will continue to try to clear
RXNEMP without actually reading frames from RX FIFO. If the RX FIFO is
not empty, the interrupt will not be cleared and napi_schedule() will
just be called again.
This situation can occur when:
(a) xcan_rx() returns without reading RX FIFO due to an error condition.
The code tries to clear both RXOK and RXNEMP but RXNEMP will not clear
due to a frame still being in the FIFO. The frame will never be read
from the FIFO as RXOK is no longer set.
(b) A frame is received between xcan_rx_poll() reading interrupt status
and clearing RXOK. RXOK will be cleared, but RXNEMP will again remain
set as the new message is still in the FIFO.
I'm able to trigger case (b) by flooding the bus with frames under load.
There does not seem to be any benefit in using both RXNEMP and RXOK in
the way the driver does, and the polling example in the reference manual
(UG585 v1.10 18.3.7 Read Messages from RxFIFO) also says that either
RXOK or RXNEMP can be used for detecting incoming messages.
Fix the issue and simplify the RX processing by only using RXNEMP
without RXOK.
Tested with the integrated CAN on Zynq-7000 SoC.
Fixes: b1201e44f50b ("can: xilinx CAN controller support")
Signed-off-by: Anssi Hannula <anssi.hannula@bitwise.fi>
Cc: <stable@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
drivers/net/can/xilinx_can.c | 18 +++++-------------
1 file changed, 5 insertions(+), 13 deletions(-)
diff --git a/drivers/net/can/xilinx_can.c b/drivers/net/can/xilinx_can.c
index 389a9603db8c..1bda47aa62f5 100644
--- a/drivers/net/can/xilinx_can.c
+++ b/drivers/net/can/xilinx_can.c
@@ -101,7 +101,7 @@ enum xcan_reg {
#define XCAN_INTR_ALL (XCAN_IXR_TXOK_MASK | XCAN_IXR_BSOFF_MASK |\
XCAN_IXR_WKUP_MASK | XCAN_IXR_SLP_MASK | \
XCAN_IXR_RXNEMP_MASK | XCAN_IXR_ERROR_MASK | \
- XCAN_IXR_ARBLST_MASK | XCAN_IXR_RXOK_MASK)
+ XCAN_IXR_ARBLST_MASK)
/* CAN register bit shift - XCAN_<REG>_<BIT>_SHIFT */
#define XCAN_BTR_SJW_SHIFT 7 /* Synchronous jump width */
@@ -708,15 +708,7 @@ static int xcan_rx_poll(struct napi_struct *napi, int quota)
isr = priv->read_reg(priv, XCAN_ISR_OFFSET);
while ((isr & XCAN_IXR_RXNEMP_MASK) && (work_done < quota)) {
- if (isr & XCAN_IXR_RXOK_MASK) {
- priv->write_reg(priv, XCAN_ICR_OFFSET,
- XCAN_IXR_RXOK_MASK);
- work_done += xcan_rx(ndev);
- } else {
- priv->write_reg(priv, XCAN_ICR_OFFSET,
- XCAN_IXR_RXNEMP_MASK);
- break;
- }
+ work_done += xcan_rx(ndev);
priv->write_reg(priv, XCAN_ICR_OFFSET, XCAN_IXR_RXNEMP_MASK);
isr = priv->read_reg(priv, XCAN_ISR_OFFSET);
}
@@ -727,7 +719,7 @@ static int xcan_rx_poll(struct napi_struct *napi, int quota)
if (work_done < quota) {
napi_complete_done(napi, work_done);
ier = priv->read_reg(priv, XCAN_IER_OFFSET);
- ier |= (XCAN_IXR_RXOK_MASK | XCAN_IXR_RXNEMP_MASK);
+ ier |= XCAN_IXR_RXNEMP_MASK;
priv->write_reg(priv, XCAN_IER_OFFSET, ier);
}
return work_done;
@@ -799,9 +791,9 @@ static irqreturn_t xcan_interrupt(int irq, void *dev_id)
}
/* Check for the type of receive interrupt and Processing it */
- if (isr & (XCAN_IXR_RXNEMP_MASK | XCAN_IXR_RXOK_MASK)) {
+ if (isr & XCAN_IXR_RXNEMP_MASK) {
ier = priv->read_reg(priv, XCAN_IER_OFFSET);
- ier &= ~(XCAN_IXR_RXNEMP_MASK | XCAN_IXR_RXOK_MASK);
+ ier &= ~XCAN_IXR_RXNEMP_MASK;
priv->write_reg(priv, XCAN_IER_OFFSET, ier);
napi_schedule(&priv->napi);
}
--
2.18.0
^ permalink raw reply related
* [PATCH 09/12] can: xilinx_can: keep only 1-2 frames in TX FIFO to fix TX accounting
From: Marc Kleine-Budde @ 2018-07-23 12:58 UTC (permalink / raw)
To: netdev; +Cc: davem, linux-can, kernel, Anssi Hannula, stable,
Marc Kleine-Budde
In-Reply-To: <20180723125843.391-1-mkl@pengutronix.de>
From: Anssi Hannula <anssi.hannula@bitwise.fi>
The xilinx_can driver assumes that the TXOK interrupt only clears after
it has been acknowledged as many times as there have been successfully
sent frames.
However, the documentation does not mention such behavior, instead
saying just that the interrupt is cleared when the clear bit is set.
Similarly, testing seems to also suggest that it is immediately cleared
regardless of the amount of frames having been sent. Performing some
heavy TX load and then going back to idle has the tx_head drifting
further away from tx_tail over time, steadily reducing the amount of
frames the driver keeps in the TX FIFO (but not to zero, as the TXOK
interrupt always frees up space for 1 frame from the driver's
perspective, so frames continue to be sent) and delaying the local echo
frames.
The TX FIFO tracking is also otherwise buggy as it does not account for
TX FIFO being cleared after software resets, causing
BUG!, TX FIFO full when queue awake!
messages to be output.
There does not seem to be any way to accurately track the state of the
TX FIFO for local echo support while using the full TX FIFO.
The Zynq version of the HW (but not the soft-AXI version) has watermark
programming support and with it an additional TX-FIFO-empty interrupt
bit.
Modify the driver to only put 1 frame into TX FIFO at a time on soft-AXI
and 2 frames at a time on Zynq. On Zynq the TXFEMP interrupt bit is used
to detect whether 1 or 2 frames have been sent at interrupt processing
time.
Tested with the integrated CAN on Zynq-7000 SoC. The 1-frame-FIFO mode
was also tested.
An alternative way to solve this would be to drop local echo support but
keep using the full TX FIFO.
v2: Add FIFO space check before TX queue wake with locking to
synchronize with queue stop. This avoids waking the queue when xmit()
had just filled it.
v3: Keep local echo support and reduce the amount of frames in FIFO
instead as suggested by Marc Kleine-Budde.
Fixes: b1201e44f50b ("can: xilinx CAN controller support")
Signed-off-by: Anssi Hannula <anssi.hannula@bitwise.fi>
Cc: <stable@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
drivers/net/can/xilinx_can.c | 139 +++++++++++++++++++++++++++++++----
1 file changed, 123 insertions(+), 16 deletions(-)
diff --git a/drivers/net/can/xilinx_can.c b/drivers/net/can/xilinx_can.c
index 763408a3eafb..dcbdc3cd651c 100644
--- a/drivers/net/can/xilinx_can.c
+++ b/drivers/net/can/xilinx_can.c
@@ -26,8 +26,10 @@
#include <linux/module.h>
#include <linux/netdevice.h>
#include <linux/of.h>
+#include <linux/of_device.h>
#include <linux/platform_device.h>
#include <linux/skbuff.h>
+#include <linux/spinlock.h>
#include <linux/string.h>
#include <linux/types.h>
#include <linux/can/dev.h>
@@ -119,6 +121,7 @@ enum xcan_reg {
/**
* struct xcan_priv - This definition define CAN driver instance
* @can: CAN private data structure.
+ * @tx_lock: Lock for synchronizing TX interrupt handling
* @tx_head: Tx CAN packets ready to send on the queue
* @tx_tail: Tx CAN packets successfully sended on the queue
* @tx_max: Maximum number packets the driver can send
@@ -133,6 +136,7 @@ enum xcan_reg {
*/
struct xcan_priv {
struct can_priv can;
+ spinlock_t tx_lock;
unsigned int tx_head;
unsigned int tx_tail;
unsigned int tx_max;
@@ -160,6 +164,11 @@ static const struct can_bittiming_const xcan_bittiming_const = {
.brp_inc = 1,
};
+#define XCAN_CAP_WATERMARK 0x0001
+struct xcan_devtype_data {
+ unsigned int caps;
+};
+
/**
* xcan_write_reg_le - Write a value to the device register little endian
* @priv: Driver private data structure
@@ -239,6 +248,10 @@ static int set_reset_mode(struct net_device *ndev)
usleep_range(500, 10000);
}
+ /* reset clears FIFOs */
+ priv->tx_head = 0;
+ priv->tx_tail = 0;
+
return 0;
}
@@ -393,6 +406,7 @@ static int xcan_start_xmit(struct sk_buff *skb, struct net_device *ndev)
struct net_device_stats *stats = &ndev->stats;
struct can_frame *cf = (struct can_frame *)skb->data;
u32 id, dlc, data[2] = {0, 0};
+ unsigned long flags;
if (can_dropped_invalid_skb(ndev, skb))
return NETDEV_TX_OK;
@@ -440,6 +454,9 @@ static int xcan_start_xmit(struct sk_buff *skb, struct net_device *ndev)
data[1] = be32_to_cpup((__be32 *)(cf->data + 4));
can_put_echo_skb(skb, ndev, priv->tx_head % priv->tx_max);
+
+ spin_lock_irqsave(&priv->tx_lock, flags);
+
priv->tx_head++;
/* Write the Frame to Xilinx CAN TX FIFO */
@@ -455,10 +472,16 @@ static int xcan_start_xmit(struct sk_buff *skb, struct net_device *ndev)
stats->tx_bytes += cf->can_dlc;
}
+ /* Clear TX-FIFO-empty interrupt for xcan_tx_interrupt() */
+ if (priv->tx_max > 1)
+ priv->write_reg(priv, XCAN_ICR_OFFSET, XCAN_IXR_TXFEMP_MASK);
+
/* Check if the TX buffer is full */
if ((priv->tx_head - priv->tx_tail) == priv->tx_max)
netif_stop_queue(ndev);
+ spin_unlock_irqrestore(&priv->tx_lock, flags);
+
return NETDEV_TX_OK;
}
@@ -832,19 +855,71 @@ static void xcan_tx_interrupt(struct net_device *ndev, u32 isr)
{
struct xcan_priv *priv = netdev_priv(ndev);
struct net_device_stats *stats = &ndev->stats;
+ unsigned int frames_in_fifo;
+ int frames_sent = 1; /* TXOK => at least 1 frame was sent */
+ unsigned long flags;
+ int retries = 0;
+
+ /* Synchronize with xmit as we need to know the exact number
+ * of frames in the FIFO to stay in sync due to the TXFEMP
+ * handling.
+ * This also prevents a race between netif_wake_queue() and
+ * netif_stop_queue().
+ */
+ spin_lock_irqsave(&priv->tx_lock, flags);
+
+ frames_in_fifo = priv->tx_head - priv->tx_tail;
- while ((priv->tx_head - priv->tx_tail > 0) &&
- (isr & XCAN_IXR_TXOK_MASK)) {
+ if (WARN_ON_ONCE(frames_in_fifo == 0)) {
+ /* clear TXOK anyway to avoid getting back here */
priv->write_reg(priv, XCAN_ICR_OFFSET, XCAN_IXR_TXOK_MASK);
+ spin_unlock_irqrestore(&priv->tx_lock, flags);
+ return;
+ }
+
+ /* Check if 2 frames were sent (TXOK only means that at least 1
+ * frame was sent).
+ */
+ if (frames_in_fifo > 1) {
+ WARN_ON(frames_in_fifo > priv->tx_max);
+
+ /* Synchronize TXOK and isr so that after the loop:
+ * (1) isr variable is up-to-date at least up to TXOK clear
+ * time. This avoids us clearing a TXOK of a second frame
+ * but not noticing that the FIFO is now empty and thus
+ * marking only a single frame as sent.
+ * (2) No TXOK is left. Having one could mean leaving a
+ * stray TXOK as we might process the associated frame
+ * via TXFEMP handling as we read TXFEMP *after* TXOK
+ * clear to satisfy (1).
+ */
+ while ((isr & XCAN_IXR_TXOK_MASK) && !WARN_ON(++retries == 100)) {
+ priv->write_reg(priv, XCAN_ICR_OFFSET, XCAN_IXR_TXOK_MASK);
+ isr = priv->read_reg(priv, XCAN_ISR_OFFSET);
+ }
+
+ if (isr & XCAN_IXR_TXFEMP_MASK) {
+ /* nothing in FIFO anymore */
+ frames_sent = frames_in_fifo;
+ }
+ } else {
+ /* single frame in fifo, just clear TXOK */
+ priv->write_reg(priv, XCAN_ICR_OFFSET, XCAN_IXR_TXOK_MASK);
+ }
+
+ while (frames_sent--) {
can_get_echo_skb(ndev, priv->tx_tail %
priv->tx_max);
priv->tx_tail++;
stats->tx_packets++;
- isr = priv->read_reg(priv, XCAN_ISR_OFFSET);
}
+
+ netif_wake_queue(ndev);
+
+ spin_unlock_irqrestore(&priv->tx_lock, flags);
+
can_led_event(ndev, CAN_LED_EVENT_TX);
xcan_update_error_state_after_rxtx(ndev);
- netif_wake_queue(ndev);
}
/**
@@ -1151,6 +1226,18 @@ static const struct dev_pm_ops xcan_dev_pm_ops = {
SET_RUNTIME_PM_OPS(xcan_runtime_suspend, xcan_runtime_resume, NULL)
};
+static const struct xcan_devtype_data xcan_zynq_data = {
+ .caps = XCAN_CAP_WATERMARK,
+};
+
+/* Match table for OF platform binding */
+static const struct of_device_id xcan_of_match[] = {
+ { .compatible = "xlnx,zynq-can-1.0", .data = &xcan_zynq_data },
+ { .compatible = "xlnx,axi-can-1.00.a", },
+ { /* end of list */ },
+};
+MODULE_DEVICE_TABLE(of, xcan_of_match);
+
/**
* xcan_probe - Platform registration call
* @pdev: Handle to the platform device structure
@@ -1165,8 +1252,10 @@ static int xcan_probe(struct platform_device *pdev)
struct resource *res; /* IO mem resources */
struct net_device *ndev;
struct xcan_priv *priv;
+ const struct of_device_id *of_id;
+ int caps = 0;
void __iomem *addr;
- int ret, rx_max, tx_max;
+ int ret, rx_max, tx_max, tx_fifo_depth;
/* Get the virtual base address for the device */
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -1176,7 +1265,8 @@ static int xcan_probe(struct platform_device *pdev)
goto err;
}
- ret = of_property_read_u32(pdev->dev.of_node, "tx-fifo-depth", &tx_max);
+ ret = of_property_read_u32(pdev->dev.of_node, "tx-fifo-depth",
+ &tx_fifo_depth);
if (ret < 0)
goto err;
@@ -1184,6 +1274,30 @@ static int xcan_probe(struct platform_device *pdev)
if (ret < 0)
goto err;
+ of_id = of_match_device(xcan_of_match, &pdev->dev);
+ if (of_id) {
+ const struct xcan_devtype_data *devtype_data = of_id->data;
+
+ if (devtype_data)
+ caps = devtype_data->caps;
+ }
+
+ /* There is no way to directly figure out how many frames have been
+ * sent when the TXOK interrupt is processed. If watermark programming
+ * is supported, we can have 2 frames in the FIFO and use TXFEMP
+ * to determine if 1 or 2 frames have been sent.
+ * Theoretically we should be able to use TXFWMEMP to determine up
+ * to 3 frames, but it seems that after putting a second frame in the
+ * FIFO, with watermark at 2 frames, it can happen that TXFWMEMP (less
+ * than 2 frames in FIFO) is set anyway with no TXOK (a frame was
+ * sent), which is not a sensible state - possibly TXFWMEMP is not
+ * completely synchronized with the rest of the bits?
+ */
+ if (caps & XCAN_CAP_WATERMARK)
+ tx_max = min(tx_fifo_depth, 2);
+ else
+ tx_max = 1;
+
/* Create a CAN device instance */
ndev = alloc_candev(sizeof(struct xcan_priv), tx_max);
if (!ndev)
@@ -1198,6 +1312,7 @@ static int xcan_probe(struct platform_device *pdev)
CAN_CTRLMODE_BERR_REPORTING;
priv->reg_base = addr;
priv->tx_max = tx_max;
+ spin_lock_init(&priv->tx_lock);
/* Get IRQ for the device */
ndev->irq = platform_get_irq(pdev, 0);
@@ -1262,9 +1377,9 @@ static int xcan_probe(struct platform_device *pdev)
pm_runtime_put(&pdev->dev);
- netdev_dbg(ndev, "reg_base=0x%p irq=%d clock=%d, tx fifo depth:%d\n",
+ netdev_dbg(ndev, "reg_base=0x%p irq=%d clock=%d, tx fifo depth: actual %d, using %d\n",
priv->reg_base, ndev->irq, priv->can.clock.freq,
- priv->tx_max);
+ tx_fifo_depth, priv->tx_max);
return 0;
@@ -1298,14 +1413,6 @@ static int xcan_remove(struct platform_device *pdev)
return 0;
}
-/* Match table for OF platform binding */
-static const struct of_device_id xcan_of_match[] = {
- { .compatible = "xlnx,zynq-can-1.0", },
- { .compatible = "xlnx,axi-can-1.00.a", },
- { /* end of list */ },
-};
-MODULE_DEVICE_TABLE(of, xcan_of_match);
-
static struct platform_driver xcan_driver = {
.probe = xcan_probe,
.remove = xcan_remove,
--
2.18.0
^ permalink raw reply related
* [PATCH 11/12] can: xilinx_can: fix incorrect clear of non-processed interrupts
From: Marc Kleine-Budde @ 2018-07-23 12:58 UTC (permalink / raw)
To: netdev
Cc: davem, linux-can, kernel, Anssi Hannula, Michal Simek, stable,
Marc Kleine-Budde
In-Reply-To: <20180723125843.391-1-mkl@pengutronix.de>
From: Anssi Hannula <anssi.hannula@bitwise.fi>
xcan_interrupt() clears ERROR|RXOFLV|BSOFF|ARBLST interrupts if any of
them is asserted. This does not take into account that some of them
could have been asserted between interrupt status read and interrupt
clear, therefore clearing them without handling them.
Fix the code to only clear those interrupts that it knows are asserted
and therefore going to be processed in xcan_err_interrupt().
Fixes: b1201e44f50b ("can: xilinx CAN controller support")
Signed-off-by: Anssi Hannula <anssi.hannula@bitwise.fi>
Cc: Michal Simek <michal.simek@xilinx.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
drivers/net/can/xilinx_can.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/net/can/xilinx_can.c b/drivers/net/can/xilinx_can.c
index ea9f9d1a5ba7..cb80a9aa7281 100644
--- a/drivers/net/can/xilinx_can.c
+++ b/drivers/net/can/xilinx_can.c
@@ -938,6 +938,7 @@ static irqreturn_t xcan_interrupt(int irq, void *dev_id)
struct net_device *ndev = (struct net_device *)dev_id;
struct xcan_priv *priv = netdev_priv(ndev);
u32 isr, ier;
+ u32 isr_errors;
/* Get the interrupt status from Xilinx CAN */
isr = priv->read_reg(priv, XCAN_ISR_OFFSET);
@@ -956,11 +957,10 @@ static irqreturn_t xcan_interrupt(int irq, void *dev_id)
xcan_tx_interrupt(ndev, isr);
/* Check for the type of error interrupt and Processing it */
- if (isr & (XCAN_IXR_ERROR_MASK | XCAN_IXR_RXOFLW_MASK |
- XCAN_IXR_BSOFF_MASK | XCAN_IXR_ARBLST_MASK)) {
- priv->write_reg(priv, XCAN_ICR_OFFSET, (XCAN_IXR_ERROR_MASK |
- XCAN_IXR_RXOFLW_MASK | XCAN_IXR_BSOFF_MASK |
- XCAN_IXR_ARBLST_MASK));
+ isr_errors = isr & (XCAN_IXR_ERROR_MASK | XCAN_IXR_RXOFLW_MASK |
+ XCAN_IXR_BSOFF_MASK | XCAN_IXR_ARBLST_MASK);
+ if (isr_errors) {
+ priv->write_reg(priv, XCAN_ICR_OFFSET, isr_errors);
xcan_err_interrupt(ndev, isr);
}
--
2.18.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