* Re: [PATCH net] netvsc: increase default receive buffer size
From: Stephen Hemminger @ 2017-09-14 17:49 UTC (permalink / raw)
To: David Miller; +Cc: netdev, haiyangz, sthemmin, devel
In-Reply-To: <20170914.100203.1656028222884493229.davem@davemloft.net>
On Thu, 14 Sep 2017 10:02:03 -0700 (PDT)
David Miller <davem@davemloft.net> wrote:
> 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>
>
> What other side effects are there to making this buffer so large?
>
> Just curious...
It increase latency and exercises bufferbloat avoidance on TCP.
The problem was the smaller buffer caused regressions in UDP
benchmarks on 40G Azure. One could argue that this is not a reasonable
benchmark but people run it. Apparently, Windows already went
the same thing and uses an even bigger buffer.
Longer term there will be more internal discussion with different
teams about what the receive latency and buffering needs to be.
Also, the issue goes away when doing accelerated networking (SR-IOV)
is more widely used.
^ permalink raw reply
* Re: RFC: Audit Kernel Container IDs
From: Eric W. Biederman @ 2017-09-14 17:33 UTC (permalink / raw)
To: Richard Guy Briggs
Cc: cgroups, Linux Containers, Linux API, Linux Audit, Linux FS Devel,
Linux Kernel, Linux Network Development, Aristeu Rozanski,
David Howells, Eric Paris, jlayton, Andy Lutomirski, mszeredi,
Paul Moore, Serge E. Hallyn, Steve Grubb, trondmy, Al Viro
In-Reply-To: <20170913171328.GP3405@madcap2.tricolour.ca>
Richard Guy Briggs <rgb@redhat.com> writes:
> The trigger is a pseudo filesystem (proc, since PID tree already exists)
> write of a u64 representing the container ID to a file representing a
> process that will become the first process in a new container.
> This might place restrictions on mount namespaces required to define a
> container, or at least careful checking of namespaces in the kernel to
> verify permissions of the orchestrator so it can't change its own
> container ID.
Why a u64?
Why a proc filesystem write and not a magic audit message?
I don't like the fact that the proc filesystem entry is likely going to
be readable and abusable by non-audit contexts?
Why the ability to change the containerid? What is the use case you are
thinking of there?
Eric
^ permalink raw reply
* Re: [PATCH net] netvsc: increase default receive buffer size
From: David Miller @ 2017-09-14 17:02 UTC (permalink / raw)
To: stephen; +Cc: kys, haiyangz, netdev, devel, sthemmin
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>
What other side effects are there to making this buffer so large?
Just curious...
^ permalink raw reply
* Re: [PATCH] tg3: clean up redundant initialization of tnapi
From: David Miller @ 2017-09-14 17:00 UTC (permalink / raw)
To: colin.king
Cc: siva.kallam, prashant, mchan, netdev, kernel-janitors,
linux-kernel
In-Reply-To: <20170914160125.11007-1-colin.king@canonical.com>
From: Colin King <colin.king@canonical.com>
Date: Thu, 14 Sep 2017 17:01:25 +0100
> From: Colin Ian King <colin.king@canonical.com>
>
> tnapi is being initialized and then immediately updated and
> hence the initialiation is redundant. Clean up the warning
> by moving the declaration and initialization to the inside
> of the for-loop.
>
> Cleans up clang scan-build warning:
> warning: Value stored to 'tnapi' during its initialization is never read
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Applied.
^ permalink raw reply
* Re: [PATCH] tls: make tls_sw_free_resources static
From: David Miller @ 2017-09-14 16:56 UTC (permalink / raw)
To: tklauser; +Cc: ilyal, aviadye, davejwatson, netdev
In-Reply-To: <20170914112225.4296-1-tklauser@distanz.ch>
From: Tobias Klauser <tklauser@distanz.ch>
Date: Thu, 14 Sep 2017 13:22:25 +0200
> Make the needlessly global function tls_sw_free_resources static to fix
> a gcc/sparse warning.
>
> Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
Applied.
^ permalink raw reply
* Page allocator bottleneck
From: Tariq Toukan @ 2017-09-14 16:49 UTC (permalink / raw)
To: David Miller, Jesper Dangaard Brouer, Mel Gorman, Eric Dumazet,
Alexei Starovoitov, Saeed Mahameed, Eran Ben Elisha,
Linux Kernel Network Developers, Andrew Morton, Michal Hocko,
linux-mm
Hi all,
As part of the efforts to support increasing next-generation NIC speeds,
I am investigating SW bottlenecks in network stack receive flow.
Here I share some numbers I got for a simple experiment, in which I
simulate the page allocation rate needed in 200Gpbs NICs.
I ran the test below over 3 different (modified) mlx5 driver versions,
loaded on server side (RX):
1) RX page cache disabled, 2 packets per page.
2) RX page cache disabled, one packet per page.
3) Huge RX page cache, one packet per page.
All page allocations are of order 0.
NIC: Connectx-5 100 Gbps.
CPU: Intel(R) Xeon(R) CPU E5-2680 v3 @ 2.50GHz
Test:
128 TCP streams (using super_netperf).
Changing num of RX queues.
HW LRO OFF, GRO ON, MTU 1500.
Observe: BW as a function of num RX queues.
Results:
Driver #1:
#rings BW (Mbps)
1 23,813
2 44,086
3 62,128
4 78,058
6 94,210 (linerate)
8 94,205 (linerate)
12 94,202 (linerate)
16 94,191 (linerate)
Driver #2:
#rings BW (Mbps)
1 18,835
2 36,716
3 50,521
4 61,746
6 63,637
8 60,299
12 51,048
16 43,337
Driver #3:
#rings BW (Mbps)
1 19,316
2 44,850
3 69,549
4 87,434
6 94,342 (linerate)
8 94,350 (linerate)
12 94,327 (linerate)
16 94,327 (linerate)
Insights:
Major degradation between #1 and #2, not getting any close to linerate!
Degradation is fixed between #2 and #3.
This is because page allocator cannot stand the higher allocation rate.
In #2, we also see that the addition of rings (cores) reduces BW (!!),
as result of increasing congestion over shared resources.
Congestion in this case is very clear.
When monitored in perf top:
85.58% [kernel] [k] queued_spin_lock_slowpath
I think that page allocator issues should be discussed separately:
1) Rate: Increase the allocation rate on a single core.
2) Scalability: Reduce congestion and sync overhead between cores.
This is clearly the current bottleneck in the network stack receive flow.
I know about some efforts that were made in the past two years.
For example the ones from Jesper et al.:
- Page-pool (not accepted AFAIK).
- Page-allocation bulking.
- Optimize order-0 allocations in Per-Cpu-Pages.
I am not an mm expert, but wanted to raise the issue again, to combine
the efforts and hear from you guys about status and possible directions.
Best regards,
Tariq Toukan
^ permalink raw reply
* Re: [LPC] 2nd RDMA Mini-Summit Schedule
From: Leon Romanovsky @ 2017-09-14 16:48 UTC (permalink / raw)
To: RDMA mailing list
Cc: linux-netdev, NVME mailing list, Jason Gunthorpe, Bart Van Assche,
Knut Omang, Christoph Lameter, Don Dutile, Liran Liss, Tzahi Oved,
Matan Barak, Dennis Dalessandro, Yuval Shaia, Marcel Apfelbaum,
lwn-T1hC0tSOHrs
In-Reply-To: <20170913130216.GT3405-U/DQcQFIOTAAJjI8aNfphQ@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 3327 bytes --]
On Wed, Sep 13, 2017 at 04:02:16PM +0300, Leon Romanovsky wrote:
> On Thu, Sep 07, 2017 at 08:18:05AM +0300, Leon Romanovsky wrote:
> > Hi,
> >
> > We're happy to announce schedule of the 2nd RDMA mini-summit, which will be
> > held as part of coming Linux Plumbers Conference 2017.
> >
> > During the conference, we will have two sessions: main track session and
> > round table session.
> >
> > First session will be held on Thursday, September 14, 2017 from 9:30am – 12:30pm
> > with the topics relevant for the wider audience.
> >
> > In the second session, we will focus on more face to face discussions in round
> > table format and it will be scheduled a little bit later once we will get meeting
> > room.
>
> UPDATE:
> The round table will take place at 2:00pm on Thursday, September 14,
> 2017 in one of the speakers's suites (3rd floor, near Georgia I/II).
>
> Thanks
UPDATE:
RDMA round table will be held at Platinum Ballroom J. from 2:00pm till 6:30pm.
Thanks
>
> >
> > ---------------------------------------------------------------------------------------
> > FIRST SESSION:
> > ---------------------------------------------------------------------------------------
> > * Backporting issues with multi-subsystem device (RDMA + netdev + more) by Don Dutile
> > - More and more devices in RDMA are dependent on netdev as well as other
> > subsystems -- target, NFS, scsi, block, cgroups, SELinux.... Can we make
> > backporting task for distro people easier?
> > * kABI Update by Matan Barak
> > - Going forward with a flexible and secure RDMA kABI
> > * System Boot and RDMA by Jason Gunthrope
> > - Discuss boot-time issues with the RDMA subsystem
> > * Paravirtual RDMA device by Marcel Apfelbaum and Yuval Shaia
> > - QEMU's limited RDMA support leaves it behind other modern hypervisors.
> > Marcel and Yuval will present the implementation of an emulated RDMA device,
> > analyze its performance and usability, and finally talk about future plans
> > for a possible virtio-rdma device.
> > * TX Flow Steering for IPsec by Liran Liss (pending time slot)
> > - The IPsec protocol provides both encryption and authentication for IP
> > protocol packets. Users of the raw ethernet QP that send/receive IPsec packets
> > could benefit from the IPsec crypto offloads capabilities that are
> > available in recent NICs.
> > * OpenFabrics Alliance - Status and New Directions by Jason Gunthrope
> > * BOF - Open Discussion
> > - Open discussion with the community lead by Leon Romanovsky, Jason Gunthorpe,
> > Christoph Lameter and Dennis Dalessandro
> >
> > ---------------------------------------------------------------------------------------
> > ROUND TABLE SESSION:
> > ---------------------------------------------------------------------------------------
> > Possible list, bring your own topics:
> > * RDMA and ULPs
> > * Verbs testing suite
> > * RDMA netlink and RDMAtool future plans
> > * RDMA and the Linux IP stack
> > * Infrastructure issues with Travis CI
> > * Infrastructure gaps with rdma-core
> > * Packaging as part of CI for rdma-core.
> > * ....
> >
> > See you next week at Linux Plumber Conference 2017.
> >
> > Thanks
>
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH net] tcp: update skb->skb_mstamp more carefully
From: Soheil Hassas Yeganeh @ 2017-09-14 16:41 UTC (permalink / raw)
To: Yuchung Cheng
Cc: Neal Cardwell, Eric Dumazet, liujian, David Miller, Eric Dumazet,
Jerry Chu, Netdev, weiyongjun (A), wangkefeng 00227729
In-Reply-To: <CAK6E8=dzYTaXnrXGeANX+wOtE+jfzX7M1ZokToe=ukm+tRK+Jw@mail.gmail.com>
On Thu, Sep 14, 2017 at 12:32 PM, Yuchung Cheng <ycheng@google.com> wrote:
> On Thu, Sep 14, 2017 at 6:57 AM, Neal Cardwell <ncardwell@google.com> wrote:
>> On Wed, Sep 13, 2017 at 11:30 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>>>
>>> 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.
>> ...
>>>
>>> Signed-off-by: Eric Dumazet <edumazet@googl.com>
>>> Reported-by: liujian <liujian56@huawei.com>
>>> ---
>>> net/ipv4/tcp_output.c | 19 ++++++++++++-------
>>> 1 file changed, 12 insertions(+), 7 deletions(-)
>>
>> Acked-by: Neal Cardwell <ncardwell@google.com>
> Acked-by: Yuchung Cheng <ycheng@google.com>
Acked-by: Soheil Hassas Yeganeh <soheil@google.com>
Very nice! Thank you, Eric!
^ permalink raw reply
* Re: [PATCH net] tcp: update skb->skb_mstamp more carefully
From: Yuchung Cheng @ 2017-09-14 16:32 UTC (permalink / raw)
To: Neal Cardwell
Cc: Eric Dumazet, liujian, David Miller, Eric Dumazet, Jerry Chu,
Netdev, weiyongjun (A), wangkefeng 00227729
In-Reply-To: <CADVnQy=OeD1C4xg6pdYY98djsGCdnKLv6PD5vC7j6-GSkfr8Yw@mail.gmail.com>
On Thu, Sep 14, 2017 at 6:57 AM, Neal Cardwell <ncardwell@google.com> wrote:
> On Wed, Sep 13, 2017 at 11:30 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>>
>> 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.
> ...
>>
>> Signed-off-by: Eric Dumazet <edumazet@googl.com>
>> Reported-by: liujian <liujian56@huawei.com>
>> ---
>> net/ipv4/tcp_output.c | 19 ++++++++++++-------
>> 1 file changed, 12 insertions(+), 7 deletions(-)
>
> Acked-by: Neal Cardwell <ncardwell@google.com>
Acked-by: Yuchung Cheng <ycheng@google.com>
nicely done!
>
> Thanks, Eric!
>
> neal
^ permalink raw reply
* [PATCH net] netvsc: increase default receive buffer size
From: Stephen Hemminger @ 2017-09-14 16:31 UTC (permalink / raw)
To: kys, haiyangz, davem; +Cc: netdev, Stephen Hemminger, devel
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>
---
drivers/net/hyperv/netvsc_drv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index c538a4f15f3b..d4902ee5f260 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -49,7 +49,7 @@
#define NETVSC_MIN_TX_SECTIONS 10
#define NETVSC_DEFAULT_TX 192 /* ~1M */
#define NETVSC_MIN_RX_SECTIONS 10 /* ~64K */
-#define NETVSC_DEFAULT_RX 2048 /* ~4M */
+#define NETVSC_DEFAULT_RX 10485 /* Max ~16M */
#define LINKCHANGE_INT (2 * HZ)
#define VF_TAKEOVER_INT (HZ / 10)
--
2.11.0
^ permalink raw reply related
* Re: [PATCH v2] vxlan: only reduce known arp broadcast request to support virtual IP
From: Jiri Benc @ 2017-09-14 16:14 UTC (permalink / raw)
To: Chen Haiquan; +Cc: davem, netdev
In-Reply-To: <20170914151440.GA30675@samsung>
On Thu, 14 Sep 2017 23:14:40 +0800, Chen Haiquan wrote:
> +static bool arp_reduce_ignore_unknown_ip;
> +module_param(arp_reduce_ignore_unknown_ip, bool, 0644);
> +MODULE_PARM_DESC(arp_reduce_ignore_unknown_ip,
> + "Only reduce known arp broaddcast request to support virtual IP");
Oh, no. This is not the way. It needs to be a runtime configuration
option (a netlink attribute). Generally, every time you find yourself
adding a module parameter, you're doing something wrong.
Also, net-next is currently closed. You should not submit new features
during this period, it's bugfixes only now.
http://vger.kernel.org/~davem/net-next.html
Jiri
^ permalink raw reply
* [PATCH] tg3: clean up redundant initialization of tnapi
From: Colin King @ 2017-09-14 16:01 UTC (permalink / raw)
To: Siva Reddy Kallam, Prashant Sreedharan, Michael Chan, netdev
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
tnapi is being initialized and then immediately updated and
hence the initialiation is redundant. Clean up the warning
by moving the declaration and initialization to the inside
of the for-loop.
Cleans up clang scan-build warning:
warning: Value stored to 'tnapi' during its initialization is never read
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/net/ethernet/broadcom/tg3.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index af33dc15c55f..54588809b867 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -11536,11 +11536,11 @@ static int tg3_start(struct tg3 *tp, bool reset_phy, bool test_irq,
tg3_napi_enable(tp);
for (i = 0; i < tp->irq_cnt; i++) {
- struct tg3_napi *tnapi = &tp->napi[i];
err = tg3_request_irq(tp, i);
if (err) {
for (i--; i >= 0; i--) {
- tnapi = &tp->napi[i];
+ struct tg3_napi *tnapi = &tp->napi[i];
+
free_irq(tnapi->irq_vec, tnapi);
}
goto out_napi_fini;
--
2.14.1
^ permalink raw reply related
* selftests/bpf doesn't compile
From: Shuah Khan @ 2017-09-14 15:33 UTC (permalink / raw)
To: Daniel Borkmann, Alexei Starovoitov, Thomas Meyer
Cc: linux-kernel, linux-kselftest, Shuah Khan, Networking
In-Reply-To: <ed20875b-7a01-fbc8-e8d1-3929ba35c8ac@kernel.org>
Hi Alexei and Daniel,
bpf test depends on clang and fails to compile when
------------------------------------------------------
make -C tools/testing/selftests/bpf run_tests
make: clang: Command not found
Makefile:39: recipe for target '.linux-kselftest/tools/testing/selftests/bpf/test_pkt_access.o' failed
make: *** [./linux-kselftest/tools/testing/selftests/bpf/test_pkt_access.o] Error 127
make: Leaving directory '.linux-kselftest/tools/testing/selftests/bpf'
With "make TARGETS=bpf kselftest" it fails earlier:
make[3]: Entering directory './linux-kselftest/tools/lib/bpf'
Makefile:40: tools/scripts/Makefile.arch: No such file or directory
Makefile:84: tools/build/Makefile.feature: No such file or directory
Makefile:143: tools/build/Makefile.include: No such file or directory
make[3]: *** No rule to make target 'tools/build/Makefile.include'. Stop.
make[3]: Leaving directory '.linux-kselftest/tools/lib/bpf'
Makefile:34: recipe for target './linux-kselftest/tools/testing/selftests/bpf/libbpf.a' failed
make[2]: *** [./linux-kselftest/tools/testing/selftests/bpf/libbpf.a] Error 2
make[2]: Leaving directory './linux-kselftest/tools/testing/selftests/bpf'
Makefile:69: recipe for target 'all' failed
make[1]: *** [all] Error 2
Makefile:1190: recipe for target 'kselftest' failed
make: *** [kselftest] Error 2
--------------------------------------------------------------
Is bpf test intended to be run in kselftest run? The clang dependency might
not be met on majority of the systems. Is this a hard dependency??
Would it make sense to create a special target for bpf test? We do have a few
tests that do that now.
TARGETS_HOTPLUG = cpu-hotplug
TARGETS_HOTPLUG += memory-hotplug
I could add a special target for bpf TARGET_BPF perhaps and exclude it from
the run_tests.
Please let me know your thoughts on this.
thanks,
-- Shuah
^ permalink raw reply
* [PATCH v2] vxlan: only reduce known arp broadcast request to support virtual IP
From: Chen Haiquan @ 2017-09-14 15:14 UTC (permalink / raw)
To: jbenc; +Cc: davem, netdev, oc
The purpose of vxlan arp reduce feature is to reply the broadcast
arp request in vtep instead of sending it out to save traffic.
The current implementation drops arp packet, if the ip cannot be
found in neigh table. In the case of virtual IP address, user
defines IP address without management from SDN controller. The IP
address does not exist in neigh table, so the arp broadcast request
from a client can not be sent to the server who owns the virtual IP
address.
This patch allow the arp request to be sent out if:
1. not arp broadcast request
2. cannot be found in neigh table
3. arp record status is not NUD_CONNECTED
The user defined of virtual IP address works while arp reduce still
suppress the arp broadcast for IP address managed by SDN controller
with this patch.
Signed-off-by: Chen Haiquan <oc@yunify.com>
---
Changes in v2:
- Add config option, arp_reduce_ignore_unknown_ip, to enable
the behavior.
---
drivers/net/vxlan.c | 42 +++++++++++++++++++++++++++++++++++-------
1 file changed, 35 insertions(+), 7 deletions(-)
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index d7c49cf1d5e9..cc9ee28f3481 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -53,6 +53,11 @@ static bool log_ecn_error = true;
module_param(log_ecn_error, bool, 0644);
MODULE_PARM_DESC(log_ecn_error, "Log packets received with corrupted ECN");
+static bool arp_reduce_ignore_unknown_ip;
+module_param(arp_reduce_ignore_unknown_ip, bool, 0644);
+MODULE_PARM_DESC(arp_reduce_ignore_unknown_ip,
+ "Only reduce known arp broaddcast request to support virtual IP");
+
static unsigned int vxlan_net_id;
static struct rtnl_link_ops vxlan_link_ops;
@@ -1473,7 +1478,7 @@ static int arp_reduce(struct net_device *dev, struct sk_buff *skb, __be32 vni)
parp->ar_op != htons(ARPOP_REQUEST) ||
parp->ar_hln != dev->addr_len ||
parp->ar_pln != 4)
- goto out;
+ goto ignore;
arpptr = (u8 *)parp + sizeof(struct arphdr);
sha = arpptr;
arpptr += dev->addr_len; /* sha */
@@ -1494,7 +1499,7 @@ static int arp_reduce(struct net_device *dev, struct sk_buff *skb, __be32 vni)
if (!(n->nud_state & NUD_CONNECTED)) {
neigh_release(n);
- goto out;
+ goto ignore;
}
f = vxlan_find_mac(vxlan, n->ha, vni);
@@ -1526,10 +1531,20 @@ static int arp_reduce(struct net_device *dev, struct sk_buff *skb, __be32 vni)
};
vxlan_ip_miss(dev, &ipa);
+ goto ignore;
+ } else {
+ /* broadcast unknown arp */
+ goto ignore;
}
+
out:
consume_skb(skb);
return NETDEV_TX_OK;
+
+ignore:
+ if (arp_reduce_ignore_unknown_ip)
+ return 1;
+ goto out;
}
#if IS_ENABLED(CONFIG_IPV6)
@@ -1642,7 +1657,7 @@ static int neigh_reduce(struct net_device *dev, struct sk_buff *skb, __be32 vni)
msg = (struct nd_msg *)(iphdr + 1);
if (msg->icmph.icmp6_code != 0 ||
msg->icmph.icmp6_type != NDISC_NEIGHBOUR_SOLICITATION)
- goto out;
+ goto ignore;
if (ipv6_addr_loopback(daddr) ||
ipv6_addr_is_multicast(&msg->target))
@@ -1656,7 +1671,7 @@ static int neigh_reduce(struct net_device *dev, struct sk_buff *skb, __be32 vni)
if (!(n->nud_state & NUD_CONNECTED)) {
neigh_release(n);
- goto out;
+ goto ignore;
}
f = vxlan_find_mac(vxlan, n->ha, vni);
@@ -1684,11 +1699,20 @@ static int neigh_reduce(struct net_device *dev, struct sk_buff *skb, __be32 vni)
};
vxlan_ip_miss(dev, &ipa);
+ goto ignore;
+ } else {
+ /* broadcast unknown neigh */
+ goto ignore;
}
out:
consume_skb(skb);
return NETDEV_TX_OK;
+
+ignore:
+ if (arp_reduce_ignore_unknown_ip)
+ return 1;
+ goto out;
}
#endif
@@ -2266,8 +2290,10 @@ static netdev_tx_t vxlan_xmit(struct sk_buff *skb, struct net_device *dev)
if (vxlan->cfg.flags & VXLAN_F_PROXY) {
eth = eth_hdr(skb);
- if (ntohs(eth->h_proto) == ETH_P_ARP)
- return arp_reduce(dev, skb, vni);
+ if (ntohs(eth->h_proto) == ETH_P_ARP) {
+ if (arp_reduce(dev, skb, vni) == NETDEV_TX_OK)
+ return NETDEV_TX_OK;
+ }
#if IS_ENABLED(CONFIG_IPV6)
else if (ntohs(eth->h_proto) == ETH_P_IPV6) {
struct ipv6hdr *hdr, _hdr;
@@ -2275,7 +2301,9 @@ static netdev_tx_t vxlan_xmit(struct sk_buff *skb, struct net_device *dev)
skb_network_offset(skb),
sizeof(_hdr), &_hdr)) &&
hdr->nexthdr == IPPROTO_ICMPV6)
- return neigh_reduce(dev, skb, vni);
+ if (neigh_reduce(dev,
+ skb, vni) == NETDEV_TX_OK)
+ return NETDEV_TX_OK;
}
#endif
}
--
2.7.4
^ permalink raw reply related
* [PATCH iproute2] tc: fix typo in tc-tcindex man page
From: Davide Caratti @ 2017-09-14 15:00 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
fix mis-typed 'pass_on' keyword.
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
man/man8/tc-tcindex.8 | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/man/man8/tc-tcindex.8 b/man/man8/tc-tcindex.8
index 7fcf8254..9a4e5ffc 100644
--- a/man/man8/tc-tcindex.8
+++ b/man/man8/tc-tcindex.8
@@ -11,7 +11,7 @@ tcindex \- traffic control index filter
.IR MASK " ] [ "
.B shift
.IR SHIFT " ] [ "
-.BR pas_on " | " fall_through " ] [ " classid
+.BR pass_on " | " fall_through " ] [ " classid
.IR CLASSID " ] [ "
.B action
.BR ACTION_SPEC " ]"
--
2.13.5
^ permalink raw reply related
* Re: [Intel-wired-lan] [PATCH] igb: check memory allocation failure
From: Waskiewicz Jr, Peter @ 2017-09-14 14:47 UTC (permalink / raw)
To: Brown, Aaron F, Christophe JAILLET, Kirsher, Jeffrey T
Cc: netdev@vger.kernel.org, kernel-janitors@vger.kernel.org,
intel-wired-lan@lists.osuosl.org, linux-kernel@vger.kernel.org
In-Reply-To: <309B89C4C689E141A5FF6A0C5FB2118B8C6A028B@ORSMSX101.amr.corp.intel.com>
On 9/13/17 7:24 PM, Brown, Aaron F wrote:
>> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@osuosl.org] On Behalf
>> Of Christophe JAILLET
>> Sent: Monday, August 28, 2017 10:13 AM
>> To: Waskiewicz Jr, Peter <peter.waskiewicz.jr@intel.com>; Kirsher, Jeffrey T
>> <jeffrey.t.kirsher@intel.com>
>> Cc: netdev@vger.kernel.org; kernel-janitors@vger.kernel.org; intel-wired-
>> lan@lists.osuosl.org; linux-kernel@vger.kernel.org
>> Subject: Re: [Intel-wired-lan] [PATCH] igb: check memory allocation failure
>>
>> Le 28/08/2017 à 01:09, Waskiewicz Jr, Peter a écrit :
>>> On 8/27/17 2:42 AM, Christophe JAILLET wrote:
>>>> Check memory allocation failures and return -ENOMEM in such cases, as
>>>> already done for other memory allocations in this function.
>>>>
>>>> This avoids NULL pointers dereference.
>>>>
>>>> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
>>>> ---
>>>> drivers/net/ethernet/intel/igb/igb_main.c | 2 ++
>>>> 1 file changed, 2 insertions(+)
>>>>
>
> This seems to be fine from a "it does not break in testing" perspective, so...
>
> Tested-by: Aaron Brown <aaron.f.brown@intel.com
>
>>> -PJ
>>>
>> Hi,
>>
>> in fact, there is no leak because the only caller of 'igb_sw_init()'
>> (i.e. 'igb_probe()'), already frees these resources in case of error,
>> see [1]
>>
>> These resources are also freed in 'igb_remove()'.
>>
>> Best reagrds,
>> CJ
>>
>> [1]:
>> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-
>> next.git/tree/drivers/net/ethernet/intel/igb/igb_main.c#n2775
>
> But is PJ's comment saying that it is not really necessary? If so I tend to lean towards the don't touch it if it's not broken perspective.
I guess I didn't respond after Christophe replied, sorry about that.
The patch is good to me. It's definitely catching an issue where we're
not checking for a memory failure, then just follows the same
de-allocation path on unwind.
If you want it:
Acked-by: PJ Waskiewicz <peter.waskiewicz.jr@intel.com>
^ permalink raw reply
* Re: [PATCH] net/packet: fix race condition between fanout_add and __unregister_prot_hook
From: Willem de Bruijn @ 2017-09-14 14:35 UTC (permalink / raw)
To: nixiaoming
Cc: David Miller, Eric Dumazet, waltje, gw4pts, Andrey Konovalov,
Tobias Klauser, Philip Pettersson, Alexander Potapenko,
Network Development, LKML, dede.wu
In-Reply-To: <20170914140722.44292-1-nixiaoming@huawei.com>
On Thu, Sep 14, 2017 at 10:07 AM, nixiaoming <nixiaoming@huawei.com> wrote:
> From: l00219569 <lisimin@huawei.com>
>
> If fanout_add is preempted after running po-> fanout = match
> and before running __fanout_link,
> it will cause BUG_ON when __unregister_prot_hook call __fanout_unlink
>
> so, we need add mutex_lock(&fanout_mutex) to __unregister_prot_hook
The packet socket code has no shortage of locks, so there are many
ways to avoid the race condition between fanout_add and packet_set_ring.
Another option would be to lock the socket when calling fanout_add:
- return fanout_add(sk, val & 0xffff, val >> 16);
+ lock_sock(sk);
+ ret = fanout_add(sk, val & 0xffff, val >> 16);
+ release_sock(sk);
+ return ret;
But, for consistency, and to be able to continue to make sense of the
locking policy, we should use the most appropriate lock. This
is po->bind_lock, as it ensures atomicity between testing whether
a protocol hook is active through po->running and the actual existence
of that hook on the protocol hook list.
fanout_mutex protects the fanout object's list. Taking that on
__unregister_prot_hook even in the case where fanout is not
used (and __dev_remove_pack is called) complicates locking
in this already complicated code.
> or add spin_lock(&po->bind_lock) before po-> fanout = match
>
> this is a patch for add po->bind_lock in fanout_add
>
> test on linux 4.1.12:
> ./trinity -c setsockopt -C 2 -X &
Thanks for testing!
>
> BUG: failure at net/packet/af_packet.c:1414/__fanout_unlink()!
> Kernel panic - not syncing: BUG!
> CPU: 2 PID: 2271 Comm: trinity-c0 Tainted: G W O 4.1.12 #1
> Hardware name: Hisilicon PhosphorHi1382 FPGA (DT)
> Call trace:
> [<ffffffc000209414>] dump_backtrace+0x0/0xf8
> [<ffffffc00020952c>] show_stack+0x20/0x28
> [<ffffffc000635574>] dump_stack+0xac/0xe4
> [<ffffffc000633fb8>] panic+0xf8/0x268
> [<ffffffc0005fa778>] __unregister_prot_hook+0xa0/0x144
> [<ffffffc0005fba48>] packet_set_ring+0x280/0x5b4
> [<ffffffc0005fc33c>] packet_setsockopt+0x320/0x950
> [<ffffffc000554a04>] SyS_setsockopt+0xa4/0xd4
>
> Signed-off-by: nixiaoming <nixiaoming@huawei.com>
> Tested-by: wudesheng <dede.wu@huawei.com>
> ---
> net/packet/af_packet.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> index 54a18a8..7a52a3b 100644
> --- a/net/packet/af_packet.c
> +++ b/net/packet/af_packet.c
> @@ -1446,12 +1446,16 @@ static int fanout_add(struct sock *sk, u16 id, u16 type_flags)
> default:
> return -EINVAL;
> }
> -
> - if (!po->running)
> + spin_lock(&po->bind_lock);
> + if (!po->running) {
> + spin_unlock(&po->bind_lock);
> return -EINVAL;
> + }
>
> - if (po->fanout)
> + if (po->fanout) {
> + spin_unlock(&po->bind_lock);
> return -EALREADY;
> + }
>
> mutex_lock(&fanout_mutex);
> match = NULL;
> @@ -1501,6 +1505,7 @@ static int fanout_add(struct sock *sk, u16 id, u16 type_flags)
> }
> out:
> mutex_unlock(&fanout_mutex);
> + spin_unlock(&po->bind_lock);
This function can call kzalloc with GFP_KERNEL, which may sleep. It is
not correct to sleep while holding a spinlock. Which is why I take the lock
later and test po->running again.
I will clean up that patch and send it for review.
^ permalink raw reply
* Re: [PATCH] net: phy: Fix mask value write on gmii2rgmii converter speed register.
From: Andrew Lunn @ 2017-09-14 14:34 UTC (permalink / raw)
To: Fahad Kunnathadi
Cc: f.fainelli, netdev, michal.simek, linux-kernel, soren.brinkmann,
linux-arm-kernel
In-Reply-To: <1505373391-18697-1-git-send-email-fahad.kunnathadi@dexceldesigns.com>
On Thu, Sep 14, 2017 at 12:46:31PM +0530, Fahad Kunnathadi wrote:
> To clear Speed Selection in MDIO control register(0x10),
> ie, clear bits 6 and 13 to zero while keeping other bits same.
> Before AND operation,The Mask value has to be perform with bitwise NOT
> operation (ie, ~ operator)
>
> This patch clears current speed selection before writing the
> new speed settings to gmii2rgmii converter
Hi Fahad
I expect you will find other issues with this driver. I pointed some
out at the time it is submitted, but the developers went quiet as soon
as it was accepted.
Anyway, please ensure David Miller <davem@davemloft.net> gets a copy.
The subject line should be:
[PATCH net] net: phy: Fix mask value write on gmii2rgmii converter speed register.
and include a fixes tag:
Fixes: f411a6160bd4 ("net: phy: Add gmiitorgmii converter support")
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH] net/ethernet/freescale: fix warning for ucc_geth
From: Longchamp, Valentin @ 2017-09-14 14:17 UTC (permalink / raw)
To: christophe.leroy@c-s.fr, netdev@vger.kernel.org,
leoli@freescale.com
Cc: linuxppc-dev@lists.ozlabs.org
In-Reply-To: <f5c5c526-99d4-ff3d-1e2a-23d71bcafa4b@c-s.fr>
Hi Christophe,
On Thu, 2017-09-14 at 15:24 +0200, Christophe LEROY wrote:
> Hi,
>
> Le 14/09/2017 à 14:05, Valentin Longchamp a écrit :
> > Simple printk format warning for the the ucc registers address.
>
> Did you test your patch with mpc83xx_defconfig ?
No I only tested on a 85xx where I had another (similar, because the
physical addresses are u64 and not u32) warning.
My quick fix indeed did not take the different typedefs for
phys_addr_t.
I try to come with a v2 that covers this.
Thanks for the feedback.
Valentin
>
> I get a new warning with your patch:
>
> CC drivers/net/ethernet/freescale/ucc_geth.o
> In file included from ./include/linux/printk.h:6:0,
> from ./include/linux/kernel.h:13,
> from drivers/net/ethernet/freescale/ucc_geth.c:18:
> drivers/net/ethernet/freescale/ucc_geth.c: In function
> ‘ucc_geth_probe’:
> ./include/linux/kern_levels.h:4:18: warning: format ‘%llx’ expects
> argument of type ‘long long unsigned int’, but argument 3 has type
> ‘resource_size_t {aka unsigned int}’ [-Wformat=]
> #define KERN_SOH "\001" /* ASCII Start Of Header */
> ^
> ./include/linux/kern_levels.h:13:19: note: in expansion of macro
> ‘KERN_SOH’
> #define KERN_INFO KERN_SOH "6" /* informational */
> ^
> ./include/linux/printk.h:308:9: note: in expansion of macro
> ‘KERN_INFO’
> printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
> ^
> drivers/net/ethernet/freescale/ucc_geth.c:3860:3: note: in expansion
> of
> macro ‘pr_info’
> pr_info("UCC%1d at 0x%8llx (irq = %d)\n",
> ^
>
> Christophe
>
> >
> > Signed-off-by: Valentin Longchamp <valentin.longchamp@keymile.com>
> > ---
> > drivers/net/ethernet/freescale/ucc_geth.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/freescale/ucc_geth.c
> > b/drivers/net/ethernet/freescale/ucc_geth.c
> > index f77ba9fa257b..56b8fdb35c3b 100644
> > --- a/drivers/net/ethernet/freescale/ucc_geth.c
> > +++ b/drivers/net/ethernet/freescale/ucc_geth.c
> > @@ -3857,7 +3857,7 @@ static int ucc_geth_probe(struct
> > platform_device* ofdev)
> > }
> >
> > if (netif_msg_probe(&debug))
> > - pr_info("UCC%1d at 0x%8x (irq = %d)\n",
> > + pr_info("UCC%1d at 0x%8llx (irq = %d)\n",
> > ug_info->uf_info.ucc_num + 1, ug_info-
> > >uf_info.regs,
> > ug_info->uf_info.irq);
> >
> >
^ permalink raw reply
* [PATCH] net/packet: fix race condition between fanout_add and __unregister_prot_hook
From: nixiaoming @ 2017-09-14 14:07 UTC (permalink / raw)
To: davem, edumazet, waltje, gw4pts, andreyknvl, tklauser,
philip.pettersson, glider, willemdebruijn.kernel
Cc: netdev, linux-kernel, nixiaoming, dede.wu
From: l00219569 <lisimin@huawei.com>
If fanout_add is preempted after running po-> fanout = match
and before running __fanout_link,
it will cause BUG_ON when __unregister_prot_hook call __fanout_unlink
so, we need add mutex_lock(&fanout_mutex) to __unregister_prot_hook
or add spin_lock(&po->bind_lock) before po-> fanout = match
this is a patch for add po->bind_lock in fanout_add
test on linux 4.1.12:
./trinity -c setsockopt -C 2 -X &
BUG: failure at net/packet/af_packet.c:1414/__fanout_unlink()!
Kernel panic - not syncing: BUG!
CPU: 2 PID: 2271 Comm: trinity-c0 Tainted: G W O 4.1.12 #1
Hardware name: Hisilicon PhosphorHi1382 FPGA (DT)
Call trace:
[<ffffffc000209414>] dump_backtrace+0x0/0xf8
[<ffffffc00020952c>] show_stack+0x20/0x28
[<ffffffc000635574>] dump_stack+0xac/0xe4
[<ffffffc000633fb8>] panic+0xf8/0x268
[<ffffffc0005fa778>] __unregister_prot_hook+0xa0/0x144
[<ffffffc0005fba48>] packet_set_ring+0x280/0x5b4
[<ffffffc0005fc33c>] packet_setsockopt+0x320/0x950
[<ffffffc000554a04>] SyS_setsockopt+0xa4/0xd4
Signed-off-by: nixiaoming <nixiaoming@huawei.com>
Tested-by: wudesheng <dede.wu@huawei.com>
---
net/packet/af_packet.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 54a18a8..7a52a3b 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -1446,12 +1446,16 @@ static int fanout_add(struct sock *sk, u16 id, u16 type_flags)
default:
return -EINVAL;
}
-
- if (!po->running)
+ spin_lock(&po->bind_lock);
+ if (!po->running) {
+ spin_unlock(&po->bind_lock);
return -EINVAL;
+ }
- if (po->fanout)
+ if (po->fanout) {
+ spin_unlock(&po->bind_lock);
return -EALREADY;
+ }
mutex_lock(&fanout_mutex);
match = NULL;
@@ -1501,6 +1505,7 @@ static int fanout_add(struct sock *sk, u16 id, u16 type_flags)
}
out:
mutex_unlock(&fanout_mutex);
+ spin_unlock(&po->bind_lock);
return err;
}
--
2.10.1
^ permalink raw reply related
* Re: [PATCH net] tcp: update skb->skb_mstamp more carefully
From: Neal Cardwell @ 2017-09-14 13:57 UTC (permalink / raw)
To: Eric Dumazet
Cc: liujian, David Miller, Eric Dumazet, Yuchung Cheng, Jerry Chu,
Netdev, weiyongjun (A), wangkefeng 00227729
In-Reply-To: <1505359839.15310.209.camel@edumazet-glaptop3.roam.corp.google.com>
On Wed, Sep 13, 2017 at 11:30 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
> 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.
...
>
> Signed-off-by: Eric Dumazet <edumazet@googl.com>
> Reported-by: liujian <liujian56@huawei.com>
> ---
> net/ipv4/tcp_output.c | 19 ++++++++++++-------
> 1 file changed, 12 insertions(+), 7 deletions(-)
Acked-by: Neal Cardwell <ncardwell@google.com>
Thanks, Eric!
neal
^ permalink raw reply
* Re: [PATCH] net/ethernet/freescale: fix warning for ucc_geth
From: Christophe LEROY @ 2017-09-14 13:24 UTC (permalink / raw)
To: Valentin Longchamp, leoli, netdev; +Cc: linuxppc-dev
In-Reply-To: <20170914120514.31845-1-valentin.longchamp@keymile.com>
Hi,
Le 14/09/2017 à 14:05, Valentin Longchamp a écrit :
> Simple printk format warning for the the ucc registers address.
Did you test your patch with mpc83xx_defconfig ?
I get a new warning with your patch:
CC drivers/net/ethernet/freescale/ucc_geth.o
In file included from ./include/linux/printk.h:6:0,
from ./include/linux/kernel.h:13,
from drivers/net/ethernet/freescale/ucc_geth.c:18:
drivers/net/ethernet/freescale/ucc_geth.c: In function ‘ucc_geth_probe’:
./include/linux/kern_levels.h:4:18: warning: format ‘%llx’ expects
argument of type ‘long long unsigned int’, but argument 3 has type
‘resource_size_t {aka unsigned int}’ [-Wformat=]
#define KERN_SOH "\001" /* ASCII Start Of Header */
^
./include/linux/kern_levels.h:13:19: note: in expansion of macro ‘KERN_SOH’
#define KERN_INFO KERN_SOH "6" /* informational */
^
./include/linux/printk.h:308:9: note: in expansion of macro ‘KERN_INFO’
printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__)
^
drivers/net/ethernet/freescale/ucc_geth.c:3860:3: note: in expansion of
macro ‘pr_info’
pr_info("UCC%1d at 0x%8llx (irq = %d)\n",
^
Christophe
>
> Signed-off-by: Valentin Longchamp <valentin.longchamp@keymile.com>
> ---
> drivers/net/ethernet/freescale/ucc_geth.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/freescale/ucc_geth.c b/drivers/net/ethernet/freescale/ucc_geth.c
> index f77ba9fa257b..56b8fdb35c3b 100644
> --- a/drivers/net/ethernet/freescale/ucc_geth.c
> +++ b/drivers/net/ethernet/freescale/ucc_geth.c
> @@ -3857,7 +3857,7 @@ static int ucc_geth_probe(struct platform_device* ofdev)
> }
>
> if (netif_msg_probe(&debug))
> - pr_info("UCC%1d at 0x%8x (irq = %d)\n",
> + pr_info("UCC%1d at 0x%8llx (irq = %d)\n",
> ug_info->uf_info.ucc_num + 1, ug_info->uf_info.regs,
> ug_info->uf_info.irq);
>
>
^ permalink raw reply
* Re: [PATCH] net/mlx5: fpga: avoid uninitialized return codes
From: Leon Romanovsky @ 2017-09-14 12:54 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Ilan Tayari, Saeed Mahameed, Matan Barak, Boris Pismenny,
netdev-u79uwXL29TY76Z2rM5mHXA, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170914110628.3590833-1-arnd-r2nGTMty4D4@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 2965 bytes --]
On Thu, Sep 14, 2017 at 01:06:18PM +0200, Arnd Bergmann wrote:
> calling mlx5_fpga_mem_{read,write}_i2c() with a zero length on
> older compiler version such as gcc-4.6 results in a warning that
> the return code is not initialized:
>
> drivers/net/ethernet/mellanox/mlx5/core/fpga/sdk.c:147:6: error: ‘err’ may be used uninitialized in this function [-Werror=uninitialized]
> drivers/net/ethernet/mellanox/mlx5/core/fpga/sdk.c:126:6: error: ‘err’ may be used uninitialized in this function [-Werror=uninitialized]
>
> On newer compilers, the 'err' variable is optimized away in this
> code path and assumed to be zero when the loop completes, so we
> don't get this warning.
>
> I'm changing the function here to instead return -EINVAL for the
> case, under the assumption that it was never meant to be called
> with a zero length argument.
I agree with you that size can't be zero and this patch will fix the
warning, but if it is possible, I will prefer to have this check is
written explicitly and not implicitly.
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fpga/sdk.c b/drivers/net/ethernet/mellanox/mlx5/core/fpga/sdk.c
index 3c11d6e2160a..0e85bebe1dad 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/fpga/sdk.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/fpga/sdk.c
@@ -69,6 +69,9 @@ static int mlx5_fpga_mem_read_i2c(struct mlx5_fpga_device *fdev, size_t size,
if (!fdev->mdev)
return -ENOTCONN;
+ if (!size)
+ return -EINVAL;
+
while (bytes_done < size) {
actual_size = min(max_size, (size - bytes_done));
Thanks
>
> Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82203
> Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
> ---
> drivers/net/ethernet/mellanox/mlx5/core/fpga/sdk.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fpga/sdk.c b/drivers/net/ethernet/mellanox/mlx5/core/fpga/sdk.c
> index 3c11d6e2160a..914fb9d77a1a 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/fpga/sdk.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/fpga/sdk.c
> @@ -64,7 +64,7 @@ static int mlx5_fpga_mem_read_i2c(struct mlx5_fpga_device *fdev, size_t size,
> size_t max_size = MLX5_FPGA_ACCESS_REG_SIZE_MAX;
> size_t bytes_done = 0;
> u8 actual_size;
> - int err;
> + int err = -EINVAL;
>
> if (!fdev->mdev)
> return -ENOTCONN;
> @@ -93,7 +93,7 @@ static int mlx5_fpga_mem_write_i2c(struct mlx5_fpga_device *fdev, size_t size,
> size_t max_size = MLX5_FPGA_ACCESS_REG_SIZE_MAX;
> size_t bytes_done = 0;
> u8 actual_size;
> - int err;
> + int err = -EINVAL;
>
> if (!fdev->mdev)
> return -ENOTCONN;
> --
> 2.9.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply related
* [PATCH] net/ethernet/freescale: fix warning for ucc_geth
From: Valentin Longchamp @ 2017-09-14 12:05 UTC (permalink / raw)
To: leoli, netdev; +Cc: linuxppc-dev, Valentin Longchamp
Simple printk format warning for the the ucc registers address.
Signed-off-by: Valentin Longchamp <valentin.longchamp@keymile.com>
---
drivers/net/ethernet/freescale/ucc_geth.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/freescale/ucc_geth.c b/drivers/net/ethernet/freescale/ucc_geth.c
index f77ba9fa257b..56b8fdb35c3b 100644
--- a/drivers/net/ethernet/freescale/ucc_geth.c
+++ b/drivers/net/ethernet/freescale/ucc_geth.c
@@ -3857,7 +3857,7 @@ static int ucc_geth_probe(struct platform_device* ofdev)
}
if (netif_msg_probe(&debug))
- pr_info("UCC%1d at 0x%8x (irq = %d)\n",
+ pr_info("UCC%1d at 0x%8llx (irq = %d)\n",
ug_info->uf_info.ucc_num + 1, ug_info->uf_info.regs,
ug_info->uf_info.irq);
--
2.13.5
^ permalink raw reply related
* [PATCH] tls: make tls_sw_free_resources static
From: Tobias Klauser @ 2017-09-14 11:22 UTC (permalink / raw)
To: Ilya Lesokhin, Aviad Yehezkel, Dave Watson; +Cc: David S. Miller, netdev
Make the needlessly global function tls_sw_free_resources static to fix
a gcc/sparse warning.
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
---
net/tls/tls_sw.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index fa596fa71ba7..7d80040a37b6 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -639,7 +639,7 @@ int tls_sw_sendpage(struct sock *sk, struct page *page,
return ret;
}
-void tls_sw_free_resources(struct sock *sk)
+static void tls_sw_free_resources(struct sock *sk)
{
struct tls_context *tls_ctx = tls_get_ctx(sk);
struct tls_sw_context *ctx = tls_sw_ctx(tls_ctx);
--
2.13.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