* Re: Use after free in __dst_destroy_metrics_generic
From: Julian Anastasov @ 2017-09-15 22:38 UTC (permalink / raw)
To: Eric Dumazet
Cc: Cong Wang, Subash Abhinov Kasiviswanathan,
Linux Kernel Network Developers, Lorenzo Colitti
In-Reply-To: <1505509219.16139.6.camel@edumazet-glaptop3.roam.corp.google.com>
Hello,
On Fri, 15 Sep 2017, Eric Dumazet wrote:
> On Fri, 2017-09-08 at 09:10 -0700, Cong Wang wrote:
> > On Thu, Sep 7, 2017 at 5:52 PM, Subash Abhinov Kasiviswanathan
> > <subashab@codeaurora.org> wrote:
> > > We are seeing a possible use after free in ip6_dst_destroy.
> > >
> > > It appears as if memory of the __DST_METRICS_PTR(old) was freed in some path
> > > and allocated
> > > to ion driver. ion driver has also freed it. Finally the memory is freed by
> > > the
> > > fib gc and crashes since it is already deallocated.
> >
> > Does the attach (compile-only) patch help anything?
> >
> > From my _quick_ glance, it seems we miss the refcnt'ing
> > right in __dst_destroy_metrics_generic().
> >
> > Thanks!
>
>
> Hi Cong
>
> I believe your patch makes a lot of sense, please submit it formally ?
Cong's patch is wrong for few reasons:
- it will stop to kfree non-refcounted metrics
- report was for IPV6 and we set DST_METRICS_REFCOUNTED only
for IPv4, for DST_METRICS_READ_ONLY metrics
- __dst_destroy_metrics_generic is called for val without
DST_METRICS_READ_ONLY flag and such metrics are not with
DST_METRICS_REFCOUNTED flag
- ->cow_metrics and dst_cow_metrics_generic are called with
DST_METRICS_READ_ONLY flag set, there is no chance to write
new value twice, especially to write DST_METRICS_REFCOUNTED flag
and later to see this flag in __dst_destroy_metrics_generic
So, I'm not sure where exactly is the bug with the
metrics.
May be I'm missing some posting but I don't see if
the patch was tested successfully.
Regards
^ permalink raw reply
* Re: selftests/bpf doesn't compile
From: Shuah Khan @ 2017-09-15 22:32 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Shuah Khan, Daniel Borkmann, Thomas Meyer, linux-kernel,
linux-kselftest, Networking, Shuah Khan
In-Reply-To: <20170915181440.adkgyqbt4yxrnbw7@ast-mbp>
On 09/15/2017 12:14 PM, Alexei Starovoitov wrote:
> On Fri, Sep 15, 2017 at 11:00:31AM -0600, Shuah Khan wrote:
>>>> I could add a special target for bpf TARGET_BPF perhaps and exclude it from
>>>> the run_test>
>>> I'm not sure what was the motivation to exclude hotplug from default testing,
>>
>> These are considered a bit more disruptive and were excluded a while
>> back. These take cpus and memory on and off-line. Also require
>> root access. So even if they are included in the regular run, these
>> won't run.
>
> most of bpf tests require root access as well.
>
>> The first failure due to clang dependency is not a problem. The second one
>> in the case of "make kselftest" is the one that requires some work when bpf
>> make is run from the main Makefile. A lots of users run tests using the
>> kselftest target from the mail Makefile. hence I would like to get this
>> working, so it would be easier to run this test on test servers.
>
> 'make kselftest' doesn't work for me at all, since I suspect it
> assumes in-source build. I always use KBUILD_OUTPUT,
> since I build multiple archs with different configs out of the same
> source tree, so there is a bigger problem here.
Right. I think you probably use
make KBUILD_OUTPUT=dir -C tools/testing/selftests/bpf
>
> $ make kselftest
> make[1]: Entering directory `/data/users/ast/net-next/bld_x64'
> make: Entering an unknown directory
> make: *** tools/testing/selftests: No such file or directory. Stop.
> make: Leaving an unknown directory
> make[1]: *** [kselftest] Error 2
> make[1]: Leaving directory `/data/users/ast/net-next/bld_x64'
> make: *** [sub-make] Error 2
>
Right. There is patch set out to fix this problem which I sent out couple
of days ago. It is probably the case that bpf Makefile might need changes
to build when it is built from the sleftests/Makefile or the main Makefile.
thanks,
-- Shuah
^ permalink raw reply
* [PATCH] bgmac: Remove all offloading features, including GRO.
From: Rosen Penev @ 2017-09-15 22:22 UTC (permalink / raw)
To: netdev, zajec5, nbd; +Cc: Rosen Penev
On a linksys E1200v1 (actually a crossflashed E1000v2), the offloading features give no measurable benefit to speed or latency. Furthermore, disabling GRO actually improves iperf performance by a whoppimg 3mbps. Results:
Currently:
[ 4] local 192.168.1.1 port 5001 connected with 192.168.1.100 port 52410
[ ID] Interval Transfer Bandwidth Reads Dist(bin=16.0K)
[ 4] 0.00-10.02 sec 52.4 MBytes 43.8 Mbits/sec 641 75:181:12:1:1:0:0:371
[ 5] local 192.168.1.1 port 5001 connected with 192.168.1.100 port 52412
[ 5] 0.00-10.02 sec 52.4 MBytes 43.8 Mbits/sec 629 51:194:13:1:0:1:0:369
[ 4] local 192.168.1.1 port 5001 connected with 192.168.1.100 port 52414
[ 4] 0.00-10.02 sec 51.9 MBytes 43.4 Mbits/sec 695 126:203:1:0:0:0:2:363
[ 5] local 192.168.1.1 port 5001 connected with 192.168.1.100 port 52416
[ 5] 0.00-10.01 sec 52.4 MBytes 43.9 Mbits/sec 626 57:186:10:0:0:0:0:373
[ 4] local 192.168.1.1 port 5001 connected with 192.168.1.100 port 52420
[ 4] 0.00-10.02 sec 52.4 MBytes 43.8 Mbits/sec 605 36:179:16:1:0:1:0:372
[ 5] local 192.168.1.1 port 5001 connected with 192.168.1.100 port 52422
After disabling everything - including GRO:
[ 4] local 192.168.1.1 port 5001 connected with 192.168.1.100 port 52440
[ ID] Interval Transfer Bandwidth Reads Dist(bin=16.0K)
[ 4] 0.00-10.01 sec 55.1 MBytes 46.2 Mbits/sec 672 180:82:0:0:1:0:0:409
[ 5] local 192.168.1.1 port 5001 connected with 192.168.1.100 port 52442
[ 5] 0.00-10.01 sec 56.0 MBytes 46.9 Mbits/sec 636 117:96:0:0:1:0:0:422
[ 4] local 192.168.1.1 port 5001 connected with 192.168.1.100 port 52444
[ 4] 0.00-10.01 sec 55.4 MBytes 46.4 Mbits/sec 675 172:92:0:0:1:0:0:410
[ 5] local 192.168.1.1 port 5001 connected with 192.168.1.100 port 52446
[ 5] 0.00-10.01 sec 56.0 MBytes 46.9 Mbits/sec 633 119:90:0:1:1:0:0:422
[ 4] local 192.168.1.1 port 5001 connected with 192.168.1.100 port 52448
[ 4] 0.00-10.01 sec 55.2 MBytes 46.3 Mbits/sec 688 157:123:0:0:2:0:0:406
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
drivers/net/ethernet/broadcom/bgmac.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c
index 48d672b204a4..c2db5673b073 100644
--- a/drivers/net/ethernet/broadcom/bgmac.c
+++ b/drivers/net/ethernet/broadcom/bgmac.c
@@ -1534,7 +1534,7 @@ int bgmac_enet_probe(struct bgmac *bgmac)
goto err_dma_free;
}
- net_dev->features = NETIF_F_SG | NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM;
+ net_dev->features &= NETIF_F_GRO;
net_dev->hw_features = net_dev->features;
net_dev->vlan_features = net_dev->features;
--
2.13.5
^ permalink raw reply related
* Re: [PATCH net] bpf/verifier: reject BPF_ALU64|BPF_END
From: David Miller @ 2017-09-15 22:02 UTC (permalink / raw)
To: ecree; +Cc: netdev, daniel
In-Reply-To: <8c3d5513-6171-3e68-56df-1efd0e87f071@solarflare.com>
From: Edward Cree <ecree@solarflare.com>
Date: Fri, 15 Sep 2017 14:37:38 +0100
> Neither ___bpf_prog_run nor the JITs accept it.
> Also adds a new test case.
>
> Fixes: 17a5267067f3 ("bpf: verifier (add verifier core)")
> Signed-off-by: Edward Cree <ecree@solarflare.com>
Applied and queued up for -stable.
^ permalink raw reply
* Re: [PATCH net] net: systemport: Fix 64-bit stats deadlock
From: David Miller @ 2017-09-15 21:53 UTC (permalink / raw)
To: f.fainelli; +Cc: netdev, edumazet, jqiaoulk
In-Reply-To: <C414D433-28B3-4BA4-938B-B5C868918532@gmail.com>
From: Florian Fainelli <f.fainelli@gmail.com>
Date: Fri, 15 Sep 2017 14:28:40 -0700
> FYI, there is another patch needed to ensure consistency between
> ethtool reported stats and netdevice stats, will submit that after
> some more testing. Thanks!
Yes, I saw that, th anks.
^ permalink raw reply
* Re: [PATCH net] sctp: do not mark sk dumped when inet_sctp_diag_fill returns err
From: David Miller @ 2017-09-15 21:51 UTC (permalink / raw)
To: lucien.xin; +Cc: netdev, linux-sctp, marcelo.leitner, nhorman
In-Reply-To: <c5398945d4e80ed8b8497675dd0449763fec4eec.1505444568.git.lucien.xin@gmail.com>
From: Xin Long <lucien.xin@gmail.com>
Date: Fri, 15 Sep 2017 11:02:48 +0800
> sctp_diag would not actually dump out sk/asoc if inet_sctp_diag_fill
> returns err, in which case it shouldn't mark sk dumped by setting
> cb->args[3] as 1 in sctp_sock_dump().
>
> Otherwise, it could cause some asocs to have no parent's sk dumped
> in 'ss --sctp'.
>
> So this patch is to not set cb->args[3] when inet_sctp_diag_fill()
> returns err in sctp_sock_dump().
>
> Fixes: 8f840e47f190 ("sctp: add the sctp_diag.c file")
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
Applied.
^ permalink raw reply
* Re: [Outreachy kernel] Re: [PATCH 2/2] Staging: irda: Remove parentheses on the right of assignment
From: Julia Lawall @ 2017-09-15 21:50 UTC (permalink / raw)
To: Joe Perches
Cc: Srishti Sharma, samuel, gregkh, netdev, devel, linux-kernel,
outreachy-kernel
In-Reply-To: <1505511108.27581.16.camel@perches.com>
On Fri, 15 Sep 2017, Joe Perches wrote:
> On Sat, 2017-09-16 at 02:36 +0530, Srishti Sharma wrote:
> > Parentheses are not needed on the right hand side of assignment
> > statement in most cases. Done using the following semantic
> > patch by coccinelle.
> []
> > @@
> > identifier E,F,G,f;
> > expression e,r;
> > @@
> >
> > (
> > E = (G == F);
> > >
> >
> > E = (e == r);
> > >
> >
> > E =
> > -(
> > ...
> > -)
> > ;
> > )
> []
> > diff --git a/drivers/staging/irda/drivers/mcs7780.c b/drivers/staging/irda/drivers/mcs7780.c
> []
> > @@ -605,7 +605,7 @@ static int mcs_speed_change(struct mcs_cb *mcs)
> > if (mcs->new_speed <= 115200) {
> > rval &= ~MCS_FIR;
> >
> > - rst = (mcs->speed > 115200);
> > + rst = mcs->speed > 115200;
> > if (rst)
> > mcs_set_reg(mcs, MCS_MINRXPW_REG, 0);
>
> Coccinelle is a good tool, but its output is limited to
> the correctness
> and completeness of its input script.
>
> Please look at the suggested modifications of the script
> and examine the code for other similar uses.
>
> The else if block immediately below this is:
>
> } else if (mcs->new_speed <= 1152000) {
> rval &= ~MCS_FIR;
>
> if ((rst = !(mcs->speed == 576000 || mcs->speed == 11520
> 00)))
> mcs_set_reg(mcs, MCS_MINRXPW_REG, 5);
>
> which should also be corrected by this patch.
You're concerned about the assignment in the if header? Because that was
in 1/2. One could also push the ! under the ||, but I'm not sure that
would be much of an improvement.
julia
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/1505511108.27581.16.camel%40perches.com.
> For more options, visit https://groups.google.com/d/optout.
>
^ permalink raw reply
* Re: [PATCH net] sctp: fix an use-after-free issue in sctp_sock_dump
From: David Miller @ 2017-09-15 21:48 UTC (permalink / raw)
To: lucien.xin; +Cc: netdev, linux-sctp, marcelo.leitner, nhorman, pabeni
In-Reply-To: <9de7bef8f4f8c0f369361118f6d2daee5d188467.1505444541.git.lucien.xin@gmail.com>
From: Xin Long <lucien.xin@gmail.com>
Date: Fri, 15 Sep 2017 11:02:21 +0800
> Commit 86fdb3448cc1 ("sctp: ensure ep is not destroyed before doing the
> dump") tried to fix an use-after-free issue by checking !sctp_sk(sk)->ep
> with holding sock and sock lock.
>
> But Paolo noticed that endpoint could be destroyed in sctp_rcv without
> sock lock protection. It means the use-after-free issue still could be
> triggered when sctp_rcv put and destroy ep after sctp_sock_dump checks
> !ep, although it's pretty hard to reproduce.
>
> I could reproduce it by mdelay in sctp_rcv while msleep in sctp_close
> and sctp_sock_dump long time.
>
> This patch is to add another param cb_done to sctp_for_each_transport
> and dump ep->assocs with holding tsp after jumping out of transport's
> traversal in it to avoid this issue.
>
> It can also improve sctp diag dump to make it run faster, as no need
> to save sk into cb->args[5] and keep calling sctp_for_each_transport
> any more.
>
> This patch is also to use int * instead of int for the pos argument
> in sctp_for_each_transport, which could make postion increment only
> in sctp_for_each_transport and no need to keep changing cb->args[2]
> in sctp_sock_filter and sctp_sock_dump any more.
>
> Fixes: 86fdb3448cc1 ("sctp: ensure ep is not destroyed before doing the dump")
> Reported-by: Paolo Abeni <pabeni@redhat.com>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH net] netvsc: increase default receive buffer size
From: David Miller @ 2017-09-15 21:48 UTC (permalink / raw)
To: stephen; +Cc: netdev, haiyangz, sthemmin, devel
In-Reply-To: <20170914163107.8404-1-sthemmin@microsoft.com>
From: Stephen Hemminger <stephen@networkplumber.org>
Date: Thu, 14 Sep 2017 09:31:07 -0700
> The default receive buffer size was reduced by recent change
> to a value which was appropriate for 10G and Windows Server 2016.
> But the value is too small for full performance with 40G on Azure.
> Increase the default back to maximum supported by host.
>
> Fixes: 8b5327975ae1 ("netvsc: allow controlling send/recv buffer size")
> Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
Applied.
^ permalink raw reply
* Re: [PATCH net] tcp: update skb->skb_mstamp more carefully
From: David Miller @ 2017-09-15 21:37 UTC (permalink / raw)
To: eric.dumazet
Cc: liujian56, edumazet, ycheng, hkchu, netdev, weiyongjun1,
wangkefeng.wang
In-Reply-To: <1505359839.15310.209.camel@edumazet-glaptop3.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 13 Sep 2017 20:30:39 -0700
> From: Eric Dumazet <edumazet@googl.com>
>
> liujian reported a problem in TCP_USER_TIMEOUT processing with a patch
> in tcp_probe_timer() :
> https://www.spinics.net/lists/netdev/msg454496.html
>
> After investigations, the root cause of the problem is that we update
> skb->skb_mstamp of skbs in write queue, even if the attempt to send a
> clone or copy of it failed. One reason being a routing problem.
>
> This patch prevents this, solving liujian issue.
>
> It also removes a potential RTT miscalculation, since
> __tcp_retransmit_skb() is not OR-ing TCP_SKB_CB(skb)->sacked with
> TCPCB_EVER_RETRANS if a failure happens, but skb->skb_mstamp has
> been changed.
>
> A future ACK would then lead to a very small RTT sample and min_rtt
> would then be lowered to this too small value.
>
> Tested:
...
> Signed-off-by: Eric Dumazet <edumazet@googl.com>
> Reported-by: liujian <liujian56@huawei.com>
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: [PATCH 2/2] Staging: irda: Remove parentheses on the right of assignment
From: Joe Perches @ 2017-09-15 21:31 UTC (permalink / raw)
To: Srishti Sharma, samuel
Cc: gregkh, netdev, devel, linux-kernel, outreachy-kernel
In-Reply-To: <9380189edb2722544981c868699ade29ab5c297d.1505508423.git.srishtishar@gmail.com>
On Sat, 2017-09-16 at 02:36 +0530, Srishti Sharma wrote:
> Parentheses are not needed on the right hand side of assignment
> statement in most cases. Done using the following semantic
> patch by coccinelle.
[]
> @@
> identifier E,F,G,f;
> expression e,r;
> @@
>
> (
> E = (G == F);
> >
>
> E = (e == r);
> >
>
> E =
> -(
> ...
> -)
> ;
> )
[]
> diff --git a/drivers/staging/irda/drivers/mcs7780.c b/drivers/staging/irda/drivers/mcs7780.c
[]
> @@ -605,7 +605,7 @@ static int mcs_speed_change(struct mcs_cb *mcs)
> if (mcs->new_speed <= 115200) {
> rval &= ~MCS_FIR;
>
> - rst = (mcs->speed > 115200);
> + rst = mcs->speed > 115200;
> if (rst)
> mcs_set_reg(mcs, MCS_MINRXPW_REG, 0);
Coccinelle is a good tool, but its output is limited to
the correctness
and completeness of its input script.
Please look at the suggested modifications of the script
and examine the code for other similar uses.
The else if block immediately below this is:
} else if (mcs->new_speed <= 1152000) {
rval &= ~MCS_FIR;
if ((rst = !(mcs->speed == 576000 || mcs->speed == 11520
00)))
mcs_set_reg(mcs, MCS_MINRXPW_REG, 5);
which should also be corrected by this patch.
^ permalink raw reply
* Re: [PATCH net] net: systemport: Fix 64-bit stats deadlock
From: Florian Fainelli @ 2017-09-15 21:28 UTC (permalink / raw)
To: David Miller; +Cc: netdev, edumazet, jqiaoulk
In-Reply-To: <20170915.142511.221689318302073484.davem@davemloft.net>
On September 15, 2017 2:25:11 PM PDT, David Miller <davem@davemloft.net> wrote:
>From: Florian Fainelli <f.fainelli@gmail.com>
>Date: Tue, 12 Sep 2017 13:14:26 -0700
>
>> We can enter a deadlock situation because there is no sufficient
>protection
>> when ndo_get_stats64() runs in process context to guard against RX or
>TX NAPI
>> contexts running in softirq, this can lead to the following lockdep
>splat and
>> actual deadlock was experienced as well with an iperf session in the
>background
>> and a while loop doing ifconfig + ethtool.
> ...
>> So just remove the u64_stats_update_begin()/end() pair in
>ndo_get_stats64()
>> since it does not appear to be useful for anything. No inconsistency
>was
>> observed with either ifconfig or ethtool, global TX counts equal the
>sum of
>> per-queue TX counts on a 32-bit architecture.
>>
>> Fixes: 10377ba7673d ("net: systemport: Support 64bit statistics")
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>
>Applied.
FYI, there is another patch needed to ensure consistency between ethtool reported stats and netdevice stats, will submit that after some more testing. Thanks!
--
Florian
^ permalink raw reply
* Re: [PATCH] net: ipv4: fix l3slave check for index returned in IP_PKTINFO
From: David Miller @ 2017-09-15 21:28 UTC (permalink / raw)
To: dsahern; +Cc: netdev
In-Reply-To: <1505347897-27422-1-git-send-email-dsahern@gmail.com>
From: David Ahern <dsahern@gmail.com>
Date: Wed, 13 Sep 2017 17:11:37 -0700
> rt_iif is only set to the actual egress device for the output path. The
> recent change to consider the l3slave flag when returning IP_PKTINFO
> works for local traffic (the correct device index is returned), but it
> broke the more typical use case of packets received from a remote host
> always returning the VRF index rather than the original ingress device.
> Update the fixup to consider l3slave and rt_iif actually getting set.
>
> Fixes: 1dfa76390bf05 ("net: ipv4: add check for l3slave for index returned in IP_PKTINFO")
> Signed-off-by: David Ahern <dsahern@gmail.com>
Applied, thanks David.
^ permalink raw reply
* Re: [PATCH v2] net: smsc911x: Quieten netif during suspend
From: David Miller @ 2017-09-15 21:26 UTC (permalink / raw)
To: geert+renesas
Cc: steve.glendinning, andrew, f.fainelli, netdev, linux-pm,
linux-renesas-soc, linux-kernel
In-Reply-To: <1505324525-9998-1-git-send-email-geert+renesas@glider.be>
From: Geert Uytterhoeven <geert+renesas@glider.be>
Date: Wed, 13 Sep 2017 19:42:05 +0200
> If the network interface is kept running during suspend, the net core
> may call net_device_ops.ndo_start_xmit() while the Ethernet device is
> still suspended, which may lead to a system crash.
>
> E.g. on sh73a0/kzm9g and r8a73a4/ape6evm, the external Ethernet chip is
> driven by a PM controlled clock. If the Ethernet registers are accessed
> while the clock is not running, the system will crash with an imprecise
> external abort.
>
> As this is a race condition with a small time window, it is not so easy
> to trigger at will. Using pm_test may increase your chances:
>
> # echo 0 > /sys/module/printk/parameters/console_suspend
> # echo platform > /sys/power/pm_test
> # echo mem > /sys/power/state
>
> To fix this, make sure the network interface is quietened during
> suspend.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Applied.
^ permalink raw reply
* Re: [PATCH net] net: systemport: Fix 64-bit stats deadlock
From: David Miller @ 2017-09-15 21:25 UTC (permalink / raw)
To: f.fainelli; +Cc: netdev, edumazet, jqiaoulk
In-Reply-To: <1505247266-42195-1-git-send-email-f.fainelli@gmail.com>
From: Florian Fainelli <f.fainelli@gmail.com>
Date: Tue, 12 Sep 2017 13:14:26 -0700
> We can enter a deadlock situation because there is no sufficient protection
> when ndo_get_stats64() runs in process context to guard against RX or TX NAPI
> contexts running in softirq, this can lead to the following lockdep splat and
> actual deadlock was experienced as well with an iperf session in the background
> and a while loop doing ifconfig + ethtool.
...
> So just remove the u64_stats_update_begin()/end() pair in ndo_get_stats64()
> since it does not appear to be useful for anything. No inconsistency was
> observed with either ifconfig or ethtool, global TX counts equal the sum of
> per-queue TX counts on a 32-bit architecture.
>
> Fixes: 10377ba7673d ("net: systemport: Support 64bit statistics")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH] net: vrf: avoid gcc-4.6 warning
From: David Miller @ 2017-09-15 21:22 UTC (permalink / raw)
To: arnd; +Cc: dsa, shm, ja, mschiffer, weiwan, netdev, linux-kernel
In-Reply-To: <20170912201100.3680915-1-arnd@arndb.de>
From: Arnd Bergmann <arnd@arndb.de>
Date: Tue, 12 Sep 2017 22:10:53 +0200
> When building an allmodconfig kernel with gcc-4.6, we get a rather
> odd warning:
>
> drivers/net/vrf.c: In function ‘vrf_ip6_input_dst’:
> drivers/net/vrf.c:964:3: error: initialized field with side-effects overwritten [-Werror]
> drivers/net/vrf.c:964:3: error: (near initialization for ‘fl6’) [-Werror]
>
> I have no idea what this warning is even trying to say, but it does
> seem like a false positive. Reordering the initialization in to match
> the structure definition gets rid of the warning, and might also avoid
> whatever gcc thinks is wrong here.
>
> Fixes: 9ff74384600a ("net: vrf: Handle ipv6 multicast and link-local addresses")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
That's extremely weird, I can't explain it either.
Oh well, applied, thanks.
^ permalink raw reply
* Re: [PATCH net-next v1] bridge: also trigger RTM_NEWLINK when interface is released from bridge
From: Vincent Bernat @ 2017-09-15 21:08 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David S. Miller, bridge, netdev
In-Reply-To: <20170915193808.22338-1-vincent@bernat.im>
❦ 15 septembre 2017 21:38 +0200, Vincent Bernat <vincent@bernat.im> :
> Currently, when an interface is released from a bridge, we get a
> RTM_DELLINK event through netlink:
>
> Deleted 2: dummy0: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
> link/ether 6e:23:c2:54:3a:b3
It should be noted this only happens when using the ioctl API. When
using the netlink API, rtnetlink.c will send a RTM_NEWLINK because of
the modification.
--
When in doubt, tell the truth.
-- Mark Twain
^ permalink raw reply
* Re: [PATCH] VSOCK: fix uapi/linux/vm_sockets.h incomplete types
From: David Miller @ 2017-09-15 21:14 UTC (permalink / raw)
To: stefanha; +Cc: netdev, jhansen
In-Reply-To: <20170912163435.4049-1-stefanha@redhat.com>
From: Stefan Hajnoczi <stefanha@redhat.com>
Date: Tue, 12 Sep 2017 17:34:35 +0100
> This patch fixes the following compiler errors when userspace
> applications use the vm_sockets.h header:
>
> include/uapi/linux/vm_sockets.h:148:32: error: invalid application of ‘sizeof’ to incomplete type ‘struct sockaddr’
> unsigned char svm_zero[sizeof(struct sockaddr) -
> ^~~~~~
> include/uapi/linux/vm_sockets.h:149:18: error: ‘sa_family_t’ undeclared here (not in a function)
> sizeof(sa_family_t) -
> ^~~~~~~~~~~
>
> Two issues:
> 1. In the kernel struct sockaddr comes in via <linux/socket.h> but in
> userspace <sys/socket.h> is required.
> 2. struct sockaddr_vm has a __kernel_sa_family_t field so let's be
> consistent and use the same type for the sizeof(sa_family_t)
> calculation.
>
> Currently userspace applications work around this broken header by first
> including <sys/socket.h>. In the kernel there is no compiler error
> because <linux/socket.h> provides everything. It's worth fixing the
> header file though.
>
> Cc: Jorgen Hansen <jhansen@vmware.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> include/uapi/linux/vm_sockets.h | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/include/uapi/linux/vm_sockets.h b/include/uapi/linux/vm_sockets.h
> index b4ed5d895699..4ae5c625ac56 100644
> --- a/include/uapi/linux/vm_sockets.h
> +++ b/include/uapi/linux/vm_sockets.h
> @@ -18,6 +18,10 @@
>
> #include <linux/socket.h>
>
> +#ifndef __KERNEL__
> +#include <sys/socket.h> /* struct sockaddr */
> +#endif
> +
There is no precedence whatsoever to include sys/socket.h in _any_ UAPI
header file provided by the kernel.
__kernel_sa_family_t is what should be used in UAPI headers, only
non-UAPI headers can use plain sa_family_t.
So that is the correct fix for this problem.
Thank you.
^ permalink raw reply
* Re: [Outreachy kernel] [PATCH v2] Staging: irda: Don't use assignment inside if statement
From: Srishti Sharma @ 2017-09-15 21:14 UTC (permalink / raw)
To: Julia Lawall
Cc: devel, samuel, Greg KH, Linux kernel mailing list,
outreachy-kernel, netdev
In-Reply-To: <alpine.DEB.2.20.1709152308300.2533@hadrien>
On Sat, Sep 16, 2017 at 2:39 AM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>
>
> On Sat, 16 Sep 2017, Srishti Sharma wrote:
>
>> On Sat, Sep 16, 2017 at 1:30 AM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>> >
>> >
>> > On Sat, 16 Sep 2017, Srishti Sharma wrote:
>> >
>> >> Write assignment statement outside the if statement. Done using
>> >> the following semantic patch by coccinelle.
>> >>
>> >> @@
>> >> identifier E;
>> >> expression F;
>> >> statement S;
>> >> @@
>> >>
>> >> -if((E = F))
>> >> +E = F;
>> >> +if(E)
>> >> S
>> >>
>> >> Signed-off-by: Srishti Sharma <srishtishar@gmail.com>
>> >
>> > Acked-by: Julia Lawall <julia.lawall@lip6.fr>
>>
>> Have sent a patchset for this instead .
>
> Thanks. Actually, you should have copied the ack.
Oh.. Sorry didn't realise I could do that :)
Regards,
Srishti
>
> julia
>
>>
>> Regards,
>> Srishti
>> >
>> >> ---
>> >> Changes in v2:
>> >> -Semicolon was missing in one of the statements of the
>> >> semantic patch
>> >>
>> >> drivers/staging/irda/drivers/irda-usb.c | 4 ++--
>> >> drivers/staging/irda/drivers/mcs7780.c | 9 ++++++---
>> >> drivers/staging/irda/net/irqueue.c | 3 ++-
>> >> 3 files changed, 10 insertions(+), 6 deletions(-)
>> >>
>> >> diff --git a/drivers/staging/irda/drivers/irda-usb.c b/drivers/staging/irda/drivers/irda-usb.c
>> >> index 723e49b..82bfc05 100644
>> >> --- a/drivers/staging/irda/drivers/irda-usb.c
>> >> +++ b/drivers/staging/irda/drivers/irda-usb.c
>> >> @@ -334,9 +334,9 @@ static void irda_usb_change_speed_xbofs(struct irda_usb_cb *self)
>> >> urb->transfer_flags = 0;
>> >>
>> >> /* Irq disabled -> GFP_ATOMIC */
>> >> - if ((ret = usb_submit_urb(urb, GFP_ATOMIC))) {
>> >> + ret = usb_submit_urb(urb, GFP_ATOMIC);
>> >> + if (ret)
>> >> net_warn_ratelimited("%s(), failed Speed URB\n", __func__);
>> >> - }
>> >> }
>> >>
>> >> /*------------------------------------------------------------------*/
>> >> diff --git a/drivers/staging/irda/drivers/mcs7780.c b/drivers/staging/irda/drivers/mcs7780.c
>> >> index c3f0b25..2b674d5 100644
>> >> --- a/drivers/staging/irda/drivers/mcs7780.c
>> >> +++ b/drivers/staging/irda/drivers/mcs7780.c
>> >> @@ -605,19 +605,22 @@ static int mcs_speed_change(struct mcs_cb *mcs)
>> >> if (mcs->new_speed <= 115200) {
>> >> rval &= ~MCS_FIR;
>> >>
>> >> - if ((rst = (mcs->speed > 115200)))
>> >> + rst = (mcs->speed > 115200);
>> >> + if (rst)
>> >> mcs_set_reg(mcs, MCS_MINRXPW_REG, 0);
>> >>
>> >> } else if (mcs->new_speed <= 1152000) {
>> >> rval &= ~MCS_FIR;
>> >>
>> >> - if ((rst = !(mcs->speed == 576000 || mcs->speed == 1152000)))
>> >> + rst = !(mcs->speed == 576000 || mcs->speed == 1152000);
>> >> + if (rst)
>> >> mcs_set_reg(mcs, MCS_MINRXPW_REG, 5);
>> >>
>> >> } else {
>> >> rval |= MCS_FIR;
>> >>
>> >> - if ((rst = (mcs->speed != 4000000)))
>> >> + rst = (mcs->speed != 4000000);
>> >> + if (rst)
>> >> mcs_set_reg(mcs, MCS_MINRXPW_REG, 5);
>> >>
>> >> }
>> >> diff --git a/drivers/staging/irda/net/irqueue.c b/drivers/staging/irda/net/irqueue.c
>> >> index 160dc89..5aab072 100644
>> >> --- a/drivers/staging/irda/net/irqueue.c
>> >> +++ b/drivers/staging/irda/net/irqueue.c
>> >> @@ -217,7 +217,8 @@ static __u32 hash( const char* name)
>> >>
>> >> while(*name) {
>> >> h = (h<<4) + *name++;
>> >> - if ((g = (h & 0xf0000000)))
>> >> + g = (h & 0xf0000000);
>> >> + if (g)
>> >> h ^=g>>24;
>> >> h &=~g;
>> >> }
>> >> --
>> >> 2.7.4
>> >>
>> >> --
>> >> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
>> >> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
>> >> To post to this group, send email to outreachy-kernel@googlegroups.com.
>> >> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/1505504680-22167-1-git-send-email-srishtishar%40gmail.com.
>> >> For more options, visit https://groups.google.com/d/optout.
>> >>
>>
>> --
>> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
>> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
>> To post to this group, send email to outreachy-kernel@googlegroups.com.
>> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/CAB3L5oxr5K5aMo06ROCVV9ejY6a_g1%2BuUkjU%3DZ5bKTJtzUs7sA%40mail.gmail.com.
>> For more options, visit https://groups.google.com/d/optout.
>>
^ permalink raw reply
* Re: [PATCH net] ipv6: fix net.ipv6.conf.all interface DAD handlers
From: David Miller @ 2017-09-15 21:12 UTC (permalink / raw)
To: mcroce; +Cc: netdev, linux-doc, ek
In-Reply-To: <20170912154637.12996-1-mcroce@redhat.com>
From: Matteo Croce <mcroce@redhat.com>
Date: Tue, 12 Sep 2017 17:46:37 +0200
> Currently, writing into
> net.ipv6.conf.all.{accept_dad,use_optimistic,optimistic_dad} has no effect.
> Fix handling of these flags by:
>
> - using the maximum of global and per-interface values for the
> accept_dad flag. That is, if at least one of the two values is
> non-zero, enable DAD on the interface. If at least one value is
> set to 2, enable DAD and disable IPv6 operation on the interface if
> MAC-based link-local address was found
>
> - using the logical OR of global and per-interface values for the
> optimistic_dad flag. If at least one of them is set to one, optimistic
> duplicate address detection (RFC 4429) is enabled on the interface
>
> - using the logical OR of global and per-interface values for the
> use_optimistic flag. If at least one of them is set to one,
> optimistic addresses won't be marked as deprecated during source address
> selection on the interface.
>
> While at it, as we're modifying the prototype for ipv6_use_optimistic_addr(),
> drop inline, and let the compiler decide.
>
> Fixes: 7fd2561e4ebd ("net: ipv6: Add a sysctl to make optimistic addresses useful candidates")
> Signed-off-by: Matteo Croce <mcroce@redhat.com>
Erik, please review.
^ permalink raw reply
* Re: [Outreachy kernel] [PATCH v2] Staging: irda: Don't use assignment inside if statement
From: Julia Lawall @ 2017-09-15 21:09 UTC (permalink / raw)
To: Srishti Sharma
Cc: samuel, Greg KH, netdev, devel, Linux kernel mailing list,
outreachy-kernel
In-Reply-To: <CAB3L5oxr5K5aMo06ROCVV9ejY6a_g1+uUkjU=Z5bKTJtzUs7sA@mail.gmail.com>
On Sat, 16 Sep 2017, Srishti Sharma wrote:
> On Sat, Sep 16, 2017 at 1:30 AM, Julia Lawall <julia.lawall@lip6.fr> wrote:
> >
> >
> > On Sat, 16 Sep 2017, Srishti Sharma wrote:
> >
> >> Write assignment statement outside the if statement. Done using
> >> the following semantic patch by coccinelle.
> >>
> >> @@
> >> identifier E;
> >> expression F;
> >> statement S;
> >> @@
> >>
> >> -if((E = F))
> >> +E = F;
> >> +if(E)
> >> S
> >>
> >> Signed-off-by: Srishti Sharma <srishtishar@gmail.com>
> >
> > Acked-by: Julia Lawall <julia.lawall@lip6.fr>
>
> Have sent a patchset for this instead .
Thanks. Actually, you should have copied the ack.
julia
>
> Regards,
> Srishti
> >
> >> ---
> >> Changes in v2:
> >> -Semicolon was missing in one of the statements of the
> >> semantic patch
> >>
> >> drivers/staging/irda/drivers/irda-usb.c | 4 ++--
> >> drivers/staging/irda/drivers/mcs7780.c | 9 ++++++---
> >> drivers/staging/irda/net/irqueue.c | 3 ++-
> >> 3 files changed, 10 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/drivers/staging/irda/drivers/irda-usb.c b/drivers/staging/irda/drivers/irda-usb.c
> >> index 723e49b..82bfc05 100644
> >> --- a/drivers/staging/irda/drivers/irda-usb.c
> >> +++ b/drivers/staging/irda/drivers/irda-usb.c
> >> @@ -334,9 +334,9 @@ static void irda_usb_change_speed_xbofs(struct irda_usb_cb *self)
> >> urb->transfer_flags = 0;
> >>
> >> /* Irq disabled -> GFP_ATOMIC */
> >> - if ((ret = usb_submit_urb(urb, GFP_ATOMIC))) {
> >> + ret = usb_submit_urb(urb, GFP_ATOMIC);
> >> + if (ret)
> >> net_warn_ratelimited("%s(), failed Speed URB\n", __func__);
> >> - }
> >> }
> >>
> >> /*------------------------------------------------------------------*/
> >> diff --git a/drivers/staging/irda/drivers/mcs7780.c b/drivers/staging/irda/drivers/mcs7780.c
> >> index c3f0b25..2b674d5 100644
> >> --- a/drivers/staging/irda/drivers/mcs7780.c
> >> +++ b/drivers/staging/irda/drivers/mcs7780.c
> >> @@ -605,19 +605,22 @@ static int mcs_speed_change(struct mcs_cb *mcs)
> >> if (mcs->new_speed <= 115200) {
> >> rval &= ~MCS_FIR;
> >>
> >> - if ((rst = (mcs->speed > 115200)))
> >> + rst = (mcs->speed > 115200);
> >> + if (rst)
> >> mcs_set_reg(mcs, MCS_MINRXPW_REG, 0);
> >>
> >> } else if (mcs->new_speed <= 1152000) {
> >> rval &= ~MCS_FIR;
> >>
> >> - if ((rst = !(mcs->speed == 576000 || mcs->speed == 1152000)))
> >> + rst = !(mcs->speed == 576000 || mcs->speed == 1152000);
> >> + if (rst)
> >> mcs_set_reg(mcs, MCS_MINRXPW_REG, 5);
> >>
> >> } else {
> >> rval |= MCS_FIR;
> >>
> >> - if ((rst = (mcs->speed != 4000000)))
> >> + rst = (mcs->speed != 4000000);
> >> + if (rst)
> >> mcs_set_reg(mcs, MCS_MINRXPW_REG, 5);
> >>
> >> }
> >> diff --git a/drivers/staging/irda/net/irqueue.c b/drivers/staging/irda/net/irqueue.c
> >> index 160dc89..5aab072 100644
> >> --- a/drivers/staging/irda/net/irqueue.c
> >> +++ b/drivers/staging/irda/net/irqueue.c
> >> @@ -217,7 +217,8 @@ static __u32 hash( const char* name)
> >>
> >> while(*name) {
> >> h = (h<<4) + *name++;
> >> - if ((g = (h & 0xf0000000)))
> >> + g = (h & 0xf0000000);
> >> + if (g)
> >> h ^=g>>24;
> >> h &=~g;
> >> }
> >> --
> >> 2.7.4
> >>
> >> --
> >> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> >> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> >> To post to this group, send email to outreachy-kernel@googlegroups.com.
> >> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/1505504680-22167-1-git-send-email-srishtishar%40gmail.com.
> >> For more options, visit https://groups.google.com/d/optout.
> >>
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/CAB3L5oxr5K5aMo06ROCVV9ejY6a_g1%2BuUkjU%3DZ5bKTJtzUs7sA%40mail.gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>
^ permalink raw reply
* Re: [Outreachy kernel] [PATCH v2] Staging: irda: Don't use assignment inside if statement
From: Srishti Sharma @ 2017-09-15 21:07 UTC (permalink / raw)
To: Julia Lawall
Cc: samuel, Greg KH, netdev, devel, Linux kernel mailing list,
outreachy-kernel
In-Reply-To: <alpine.DEB.2.20.1709152200030.2533@hadrien>
On Sat, Sep 16, 2017 at 1:30 AM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>
>
> On Sat, 16 Sep 2017, Srishti Sharma wrote:
>
>> Write assignment statement outside the if statement. Done using
>> the following semantic patch by coccinelle.
>>
>> @@
>> identifier E;
>> expression F;
>> statement S;
>> @@
>>
>> -if((E = F))
>> +E = F;
>> +if(E)
>> S
>>
>> Signed-off-by: Srishti Sharma <srishtishar@gmail.com>
>
> Acked-by: Julia Lawall <julia.lawall@lip6.fr>
Have sent a patchset for this instead .
Regards,
Srishti
>
>> ---
>> Changes in v2:
>> -Semicolon was missing in one of the statements of the
>> semantic patch
>>
>> drivers/staging/irda/drivers/irda-usb.c | 4 ++--
>> drivers/staging/irda/drivers/mcs7780.c | 9 ++++++---
>> drivers/staging/irda/net/irqueue.c | 3 ++-
>> 3 files changed, 10 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/staging/irda/drivers/irda-usb.c b/drivers/staging/irda/drivers/irda-usb.c
>> index 723e49b..82bfc05 100644
>> --- a/drivers/staging/irda/drivers/irda-usb.c
>> +++ b/drivers/staging/irda/drivers/irda-usb.c
>> @@ -334,9 +334,9 @@ static void irda_usb_change_speed_xbofs(struct irda_usb_cb *self)
>> urb->transfer_flags = 0;
>>
>> /* Irq disabled -> GFP_ATOMIC */
>> - if ((ret = usb_submit_urb(urb, GFP_ATOMIC))) {
>> + ret = usb_submit_urb(urb, GFP_ATOMIC);
>> + if (ret)
>> net_warn_ratelimited("%s(), failed Speed URB\n", __func__);
>> - }
>> }
>>
>> /*------------------------------------------------------------------*/
>> diff --git a/drivers/staging/irda/drivers/mcs7780.c b/drivers/staging/irda/drivers/mcs7780.c
>> index c3f0b25..2b674d5 100644
>> --- a/drivers/staging/irda/drivers/mcs7780.c
>> +++ b/drivers/staging/irda/drivers/mcs7780.c
>> @@ -605,19 +605,22 @@ static int mcs_speed_change(struct mcs_cb *mcs)
>> if (mcs->new_speed <= 115200) {
>> rval &= ~MCS_FIR;
>>
>> - if ((rst = (mcs->speed > 115200)))
>> + rst = (mcs->speed > 115200);
>> + if (rst)
>> mcs_set_reg(mcs, MCS_MINRXPW_REG, 0);
>>
>> } else if (mcs->new_speed <= 1152000) {
>> rval &= ~MCS_FIR;
>>
>> - if ((rst = !(mcs->speed == 576000 || mcs->speed == 1152000)))
>> + rst = !(mcs->speed == 576000 || mcs->speed == 1152000);
>> + if (rst)
>> mcs_set_reg(mcs, MCS_MINRXPW_REG, 5);
>>
>> } else {
>> rval |= MCS_FIR;
>>
>> - if ((rst = (mcs->speed != 4000000)))
>> + rst = (mcs->speed != 4000000);
>> + if (rst)
>> mcs_set_reg(mcs, MCS_MINRXPW_REG, 5);
>>
>> }
>> diff --git a/drivers/staging/irda/net/irqueue.c b/drivers/staging/irda/net/irqueue.c
>> index 160dc89..5aab072 100644
>> --- a/drivers/staging/irda/net/irqueue.c
>> +++ b/drivers/staging/irda/net/irqueue.c
>> @@ -217,7 +217,8 @@ static __u32 hash( const char* name)
>>
>> while(*name) {
>> h = (h<<4) + *name++;
>> - if ((g = (h & 0xf0000000)))
>> + g = (h & 0xf0000000);
>> + if (g)
>> h ^=g>>24;
>> h &=~g;
>> }
>> --
>> 2.7.4
>>
>> --
>> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
>> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
>> To post to this group, send email to outreachy-kernel@googlegroups.com.
>> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/1505504680-22167-1-git-send-email-srishtishar%40gmail.com.
>> For more options, visit https://groups.google.com/d/optout.
>>
^ permalink raw reply
* [PATCH 2/2] Staging: irda: Remove parentheses on the right of assignment
From: Srishti Sharma @ 2017-09-15 21:06 UTC (permalink / raw)
To: samuel
Cc: devel, gregkh, linux-kernel, outreachy-kernel, Srishti Sharma,
netdev
In-Reply-To: <cover.1505508423.git.srishtishar@gmail.com>
Parentheses are not needed on the right hand side of assignment
statement in most cases. Done using the following semantic
patch by coccinelle.
@@
identifier E,F,G,f;
expression e,r;
@@
(
E = (G == F);
|
E = (e == r);
|
E =
-(
...
-)
;
)
Signed-off-by: Srishti Sharma <srishtishar@gmail.com>
---
drivers/staging/irda/drivers/mcs7780.c | 4 ++--
drivers/staging/irda/net/irqueue.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/irda/drivers/mcs7780.c b/drivers/staging/irda/drivers/mcs7780.c
index 2b674d5..d52e9f4 100644
--- a/drivers/staging/irda/drivers/mcs7780.c
+++ b/drivers/staging/irda/drivers/mcs7780.c
@@ -605,7 +605,7 @@ static int mcs_speed_change(struct mcs_cb *mcs)
if (mcs->new_speed <= 115200) {
rval &= ~MCS_FIR;
- rst = (mcs->speed > 115200);
+ rst = mcs->speed > 115200;
if (rst)
mcs_set_reg(mcs, MCS_MINRXPW_REG, 0);
@@ -619,7 +619,7 @@ static int mcs_speed_change(struct mcs_cb *mcs)
} else {
rval |= MCS_FIR;
- rst = (mcs->speed != 4000000);
+ rst = mcs->speed != 4000000;
if (rst)
mcs_set_reg(mcs, MCS_MINRXPW_REG, 5);
diff --git a/drivers/staging/irda/net/irqueue.c b/drivers/staging/irda/net/irqueue.c
index 5aab072..14291cb 100644
--- a/drivers/staging/irda/net/irqueue.c
+++ b/drivers/staging/irda/net/irqueue.c
@@ -217,7 +217,7 @@ static __u32 hash( const char* name)
while(*name) {
h = (h<<4) + *name++;
- g = (h & 0xf0000000);
+ g = h & 0xf0000000;
if (g)
h ^=g>>24;
h &=~g;
--
2.7.4
^ permalink raw reply related
* [PATCH 1/2] Staging: irda: Don't use assignment inside if statement
From: Srishti Sharma @ 2017-09-15 21:05 UTC (permalink / raw)
To: samuel
Cc: gregkh, netdev, devel, linux-kernel, outreachy-kernel,
Srishti Sharma
In-Reply-To: <cover.1505508423.git.srishtishar@gmail.com>
Write assignment statement outside of the if statement. Done
using the following semantic patch by coccinelle.
@@
identifier E;
expression F;
statement S;
@@
-if((E = F))
+E = F;
+if(E)
S
Signed-off-by: Srishti Sharma <srishtishar@gmail.com>
---
drivers/staging/irda/drivers/irda-usb.c | 4 ++--
drivers/staging/irda/drivers/mcs7780.c | 9 ++++++---
drivers/staging/irda/net/irqueue.c | 3 ++-
3 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/irda/drivers/irda-usb.c b/drivers/staging/irda/drivers/irda-usb.c
index 723e49b..82bfc05 100644
--- a/drivers/staging/irda/drivers/irda-usb.c
+++ b/drivers/staging/irda/drivers/irda-usb.c
@@ -334,9 +334,9 @@ static void irda_usb_change_speed_xbofs(struct irda_usb_cb *self)
urb->transfer_flags = 0;
/* Irq disabled -> GFP_ATOMIC */
- if ((ret = usb_submit_urb(urb, GFP_ATOMIC))) {
+ ret = usb_submit_urb(urb, GFP_ATOMIC);
+ if (ret)
net_warn_ratelimited("%s(), failed Speed URB\n", __func__);
- }
}
/*------------------------------------------------------------------*/
diff --git a/drivers/staging/irda/drivers/mcs7780.c b/drivers/staging/irda/drivers/mcs7780.c
index c3f0b25..2b674d5 100644
--- a/drivers/staging/irda/drivers/mcs7780.c
+++ b/drivers/staging/irda/drivers/mcs7780.c
@@ -605,19 +605,22 @@ static int mcs_speed_change(struct mcs_cb *mcs)
if (mcs->new_speed <= 115200) {
rval &= ~MCS_FIR;
- if ((rst = (mcs->speed > 115200)))
+ rst = (mcs->speed > 115200);
+ if (rst)
mcs_set_reg(mcs, MCS_MINRXPW_REG, 0);
} else if (mcs->new_speed <= 1152000) {
rval &= ~MCS_FIR;
- if ((rst = !(mcs->speed == 576000 || mcs->speed == 1152000)))
+ rst = !(mcs->speed == 576000 || mcs->speed == 1152000);
+ if (rst)
mcs_set_reg(mcs, MCS_MINRXPW_REG, 5);
} else {
rval |= MCS_FIR;
- if ((rst = (mcs->speed != 4000000)))
+ rst = (mcs->speed != 4000000);
+ if (rst)
mcs_set_reg(mcs, MCS_MINRXPW_REG, 5);
}
diff --git a/drivers/staging/irda/net/irqueue.c b/drivers/staging/irda/net/irqueue.c
index 160dc89..5aab072 100644
--- a/drivers/staging/irda/net/irqueue.c
+++ b/drivers/staging/irda/net/irqueue.c
@@ -217,7 +217,8 @@ static __u32 hash( const char* name)
while(*name) {
h = (h<<4) + *name++;
- if ((g = (h & 0xf0000000)))
+ g = (h & 0xf0000000);
+ if (g)
h ^=g>>24;
h &=~g;
}
--
2.7.4
^ permalink raw reply related
* [PATCH 0/2] Assign outside if
From: Srishti Sharma @ 2017-09-15 21:05 UTC (permalink / raw)
To: samuel
Cc: gregkh, netdev, devel, linux-kernel, outreachy-kernel,
Srishti Sharma
This patch series intends to remove the assignment statements
inside the if statement, and eliminates the cases of parentheses
around the right hand side of assignment generated as a result of
the same.
Srishti Sharma (2):
Staging: irda: Don't use assignment inside if statement
Staging: irda: Remove parentheses on the right of assignment
drivers/staging/irda/drivers/irda-usb.c | 4 ++--
drivers/staging/irda/drivers/mcs7780.c | 9 ++++++---
drivers/staging/irda/net/irqueue.c | 3 ++-
3 files changed, 10 insertions(+), 6 deletions(-)
--
2.7.4
^ permalink raw reply
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