* Re: FEC on i.MX 7 transmit queue timeout
From: Stefan Agner @ 2017-05-04 21:36 UTC (permalink / raw)
To: Andy Duan; +Cc: festevam, netdev, netdev-owner
In-Reply-To: <AM4PR0401MB2260F011313883C522EA7684FFEA0@AM4PR0401MB2260.eurprd04.prod.outlook.com>
On 2017-05-03 20:08, Andy Duan wrote:
> From: Stefan Agner <stefan@agner.ch> Sent: Thursday, May 04, 2017 9:22 AM
>>To: Andy Duan <fugang.duan@nxp.com>
>>Cc: fugang.duan@freescale.com; festevam@gmail.com;
>>netdev@vger.kernel.org; netdev-owner@vger.kernel.org
>>Subject: Re: FEC on i.MX 7 transmit queue timeout
>>
>>Hi Andy,
>>
>>On 2017-04-20 19:48, Andy Duan wrote:
>>> On 2017年04月20日 07:15, Stefan Agner wrote:
>>>> I tested again with imx6sx-fec compatible string. I could reproduce
>>>> it on a Colibri with i.MX 7Dual. But not always: It really depends
>>>> whether queue 2 is counting up or not. Just after boot, I check
>>>> /proc/interrupts twice, if queue 2 is counting it will happen!
>>>>
>>>> But if only queue 0 is mostly in use, then it seems to work just fine.
>>> If your case is only running best effort like tcp/udp, you can re-set
>>> the "fsl,num-tx-queues" and "fsl,num-rx-queues" to 1 in board dts file.
>>> Other two queues are for AVB audio/video queues, they have high
>>> priority than queue 0. If running iperf tcp test on the three queues,
>>> then the tcp segment may be out-of-order that cause net watchdog
>>timeout.
>>>>
>>>> I also tried i.MX 7Dual SabreSD here, and the same thing. I had to
>>>> reboot 3 times, then queue 2 was counting:
>>>> 57: 8 GIC-0 150 Level 30be0000.ethernet
>>>> 58: 20137 GIC-0 151 Level 30be0000.ethernet
>>>> 59: 9269 GIC-0 152 Level 30be0000.ethernet
>>>>
>>>> It took me about 40 minutes on Sabre until it happened, and I had to
>>>> force it using iperf, but then I got the ring dumps:
>>> My board had ran more than 47 hours with nfs rootfs in 4.11.0-rc6, but
>>> not running iperf.
>>> I am testing with iperf.
>>
>>Any update on this issue?
>>
>>When using iperf (server) on the board with Linux 4.11 the issue appears
>>within a few iperf iterations on a Sabre (TO 1.2, Board Rev C, if that matters)...
>>
> I don’t know whether you received my last mail. (maybe failed due to I
> received some rejection mails)
I think I did not... The last email I received was Fri, 21 Apr 2017
02:48:23 UTC.
> If your case is only running best effort like tcp/udp, you can re-set
> the "fsl,num-tx-queues" and "fsl,num-rx-queues" to 1 in board dts
> file.
I did test that, and it seems to work fine with those properties set to
1.
> Other two queues are for AVB audio/video queues, they have high
> priority than queue 0. If running iperf tcp test on the three queues,
> then the tcp segment may be out-of-order that cause net watchdog
> timeout.
Okay. A single event would be understandable, but it seems to enter some
kind of loop after that (continuously printing "fec 30be0000.ethernet
eth0: TX ring dump ...").
In a quick test I commented out the fec_dump call, with that it seems to
print only once and continues working afterwards (although, speed starts
to decrease, so something is not good at that point).
> In fsl kernel tree, there have one patch that only select the queue0
> for best effort like tcp/udp. Pls test again in your board, if no
> problem I will upstream the patch.
That sounds like a reasonable fix.
IP, no matter whether TCP/UDP, is the most common use case, so IMHO this
should "just work" by default.
^ permalink raw reply
* Re: [PATCH 1/2] PCI: Add new PCIe Fabric End Node flag, PCI_DEV_FLAGS_NO_RELAXED_ORDERING
From: Casey Leedom @ 2017-05-04 21:01 UTC (permalink / raw)
To: Alexander Duyck
Cc: Raj, Ashok, Bjorn Helgaas, Michael Werner, Ganesh GR, Arjun V.,
Asit K Mallick, Patrick J Cramer, Suravee Suthikulpanit, Bob Shaw,
h, Ding Tianhong, Mark Rutland, Amir Ancel, Gabriele Paoloni,
Catalin Marinas, Will Deacon, LinuxArm, David Laight,
Jeff Kirsher, Netdev
In-Reply-To: <CAKgT0UdJ17DGAphuP5Y9co6ky-GBFdq2s-1nqctY_Tz_iz5__g@mail.gmail.com>
| From: Alexander Duyck <alexander.duyck@gmail.com>
| Sent: Wednesday, May 3, 2017 9:02 AM
| ...
| It sounds like we are more or less in agreement. My only concern is
| really what we default this to. On x86 I would say we could probably
| default this to disabled for existing platforms since my understanding
| is that relaxed ordering doesn't provide much benefit on what is out
| there right now when performing DMA through the root complex. As far
| as peer-to-peer I would say we should probably look at enabling the
| ability to have Relaxed Ordering enabled for some channels but not
| others. In those cases the hardware needs to be smart enough to allow
| for you to indicate you want it disabled by default for most of your
| DMA channels, and then enabled for the select channels that are
| handling the peer-to-peer traffic.
Yes, I think that we are mostly in agreement. I had just wanted to make
sure that whatever scheme was developed would allow for simultaneously
supporting non-Relaxed Ordering for some PCIe End Points and Relaxed
Ordering for others within the same system. I.e. not simply
enabling/disabling/etc. based solely on System Platform Architecture.
By the way, I've started our QA folks off looking at what things look like
in Linux Virtual Machines under different Hypervisors to see what
information they may provide to the VM in the way of what Root Complex Port
is being used, etc. So far they've got Windows HyperV done and there
there's no PCIe Fabric exposed in any way: just the attached device. I'll
have to see what pci_find_pcie_root_port() returns in that environment.
Maybe NULL?
With your reservations (which I also share), I think that it probably
makes sense to have a per-architecture definition of the "Can I Use Relaxed
Ordering With TLPs Directed At This End Point" predicate, with the default
being "No" for any architecture which doesn't implement the predicate. And
if the specified (struct pci_dev *) End Node is NULL, it ought to return
False for that as well. I can't see any reason to pass in the Source End
Node but I may be missing something.
At this point, this is pretty far outside my level of expertise. I'm
happy to give it a go, but I'd be even happier if someone with a lot more
experience in the PCIe Infrastructure were to want to carry the ball
forward. I'm not super familiar with the Linux Kernel "Rules Of
Engagement", so let me know what my next step should be. Thanks.
Casey
^ permalink raw reply
* Re: [RFC iproute2 0/8] RDMA tool
From: Doug Ledford @ 2017-05-04 20:45 UTC (permalink / raw)
To: Dennis Dalessandro, Leon Romanovsky, Bart Van Assche
Cc: jiri-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
ram.amrani-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org,
sagi-NQWnxTmZq1alnMjI0IkVqw@public.gmane.org,
ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org,
hch-jcswGhMUV9g@public.gmane.org,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org,
stephen-OTpzqLSitTUnbdJkjeBofR2eb7JE58TQ@public.gmane.org,
ariela-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org
In-Reply-To: <2dee6cde-0406-b101-0fe6-c1f6de7c1b1a-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
On Thu, 2017-05-04 at 15:26 -0400, Dennis Dalessandro wrote:
> On 05/04/2017 02:45 PM, Leon Romanovsky wrote:
> >
> > On Thu, May 04, 2017 at 06:30:27PM +0000, Bart Van Assche wrote:
> > >
> > > On Thu, 2017-05-04 at 21:25 +0300, Leon Romanovsky wrote:
> > > >
> > > > On Thu, May 04, 2017 at 06:10:54PM +0000, Bart Van Assche
> > > > wrote:
> > > > >
> > > > > On Thu, 2017-05-04 at 21:02 +0300, Leon Romanovsky wrote:
> > > > > >
> > > > > > Following our discussion both in mailing list [1] and at
> > > > > > the LPC 2016 [2],
> > > > > > we would like to propose this RDMA tool to be part of
> > > > > > iproute2 package
> > > > > > and finally improve this situation.
> > > > >
> > > > > Hello Leon,
> > > > >
> > > > > Although I really appreciate your work: can you clarify why
> > > > > you would like to
> > > > > add *RDMA* functionality to an *IP routing* tool? I haven't
> > > > > found any motivation
> > > > > for adding RDMA functionality to iproute2 in [1].
> > > >
> > > > We are planning to reuse the same infrastructure provided by
> > > > iproute2,
> > > > like netlink parsing, access to distributions, same CLI and
> > > > same standards.
> > > >
> > > > Right now, RDMA is already tightened to netdev: iWARP, RoCE,
> > > > IPoIB, HFI-VNIC.
> > > > Many drivers (mlx, qed, i40, cxgb) are sharing code between net
> > > > and
> > > > RDMA.
> > > >
> > > > I do expect that iproute2 will be installed on every machine
> > > > with any
> > > > type of connection, including IB and OPA.
> > > >
> > > > So I think that it is enough to be part of that suite and don't
> > > > invent
> > > > our own for one specific tool.
> > >
> > > Hello Leon,
> > >
> > > Sorry but to me that sounds like a weak argument for including
> > > RDMA functionality
> > > in iproute2. There is already a library for communication over
> > > netlink sockets,
> > > namely libnl. Is there functionality that is in iproute2 but not
> > > in libnl and
> > > that is needed for the new tool? If so, have you considered to
> > > create a new
> > > library for that functionality?
> >
> > It is not hard to create new tool, the hardest part is to ensure
> > that it is
> > part of the distributions. Did you count how many months we are
> > trying to
> > add rdma-core to debian?
>
> I do agree that it is a strange pairing and am not really a fan.
> However
> at the end of the day it's just a name for a repo/package. If the
> iproute folks are fine to include rdma in their repo/package, great
> we
> can leverage their code for CLI and other common stuff.
If you look into the iproute2 package, it becomes clear that the name
iproute2 is historical and not really accurate any more. It contains
things like the bridge control software, tc for controlling send
queues, and many things network related but not routing related. The
rdma tool is a perfectly fine fit in the sense that it is an additional
network management tool IMO.
For reference, here's the list of stuff already in iproute on my Fedora
24 box:
/usr/sbin/arpd
/usr/sbin/bridge
/usr/sbin/cbq
/usr/sbin/ctstat
/usr/sbin/genl
/usr/sbin/ifcfg
/usr/sbin/ifstat
/usr/sbin/ip
/usr/sbin/lnstat
/usr/sbin/nstat
/usr/sbin/routef
/usr/sbin/routel
/usr/sbin/rtacct
/usr/sbin/rtmon
/usr/sbin/rtpr
/usr/sbin/rtstat
/usr/sbin/ss
/usr/sbin/tc
/usr/sbin/tipc
And in fact, if you check, tipc is almost similar to RDMA ;-) So, I
suggest people not get hung up on the name iproute2, the fit is fine
when you look deeper into the nature of the package.
--
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57 2FDD
--
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
^ permalink raw reply
* Re: [RFC] iproute: Add support for extended ack to rtnl_talk
From: Phil Sutter @ 2017-05-04 20:43 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David Miller, dsahern, daniel, netdev
In-Reply-To: <20170504094356.66590a9a@xeon-e3>
Hi,
On Thu, May 04, 2017 at 09:43:56AM -0700, Stephen Hemminger wrote:
> On Thu, 04 May 2017 10:41:03 -0400 (EDT)
> David Miller <davem@davemloft.net> wrote:
>
> > From: David Ahern <dsahern@gmail.com>
> > Date: Thu, 4 May 2017 08:27:35 -0600
> >
> > > On 5/4/17 3:36 AM, Daniel Borkmann wrote:
> > >> What is the clear benefit/rationale of outsourcing this to
> > >> libmnl? I always was the impression we should strive for as little
> > >> dependencies as possible?
> > >
> > > +1
> >
> > Agreed, all else being equal iproute2 should be as self contained
> > as possible since it is such a fundamental tool.
>
> Sorry, the old netlink code is more difficult to understand than libmnl.
> Having dependency on a library is not a problem. There already is
> an alternative implementation of ip commands in busybox for those
> people trying to work in small environments.
I second that. If you can't afford the extra ~24KB of libmnl on your
system, you much rather can't afford the 20 times bigger ip binary,
either.
Regarding conversion to libmnl, which I investigated and started working
on once: My gut feeling back then was that it's not quite worth the
effor since iproute2 requires an intermediate layer of functions anyway.
Another detail which I didn't like that much was libmnl's idiom of
creating netlink messages on base of just a plain buffer and using
mnl_nlmsg_put_header() et al. to populate it with data. I'm probably a
bit biased since I did the conversion to c99-style initializers for the
various struct req data types, but I didn't like the added run-time
overhead to achieve just the same.
So in summary, given that very little change happens to iproute2's
internal libnetlink, I don't see much urge to make it use libmnl as
backend. In my opinion it just adds another potential source of errors.
Eventually this should be a maintainer level decision, though. :)
Cheers, Phil
^ permalink raw reply
* Re: [PATCH] aquantia: Fix "ethtool -S" crash when adapter down.
From: David Arcari @ 2017-05-04 20:39 UTC (permalink / raw)
To: Pavel Belous, David S . Miller; +Cc: netdev, Lino Sanfilippo, Simon Edelhaus
In-Reply-To: <9ea5a2ba21b4f8b22a63acf39135a41e2f1da23c.1493928470.git.pavel.belous@aquantia.com>
On 05/04/2017 04:10 PM, Pavel Belous wrote:
> From: Pavel Belous <pavel.belous@aquantia.com>
>
> This patch fixes the crash that happens when driver tries to collect statistics
> from already released "aq_vec" object.
> If adapter is in "down" state we still allow user to see statistics from HW.
>
> V2: fixed braces around "aq_vec_free".
>
> Fixes: 97bde5c4f909 ("net: ethernet: aquantia: Support for NIC-specific code")
> Signed-off-by: Pavel Belous <pavel.belous@aquantia.com>
> ---
> drivers/net/ethernet/aquantia/atlantic/aq_nic.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
> index cdb0299..9ee1c50 100644
> --- a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
> +++ b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
> @@ -755,7 +755,7 @@ void aq_nic_get_stats(struct aq_nic_s *self, u64 *data)
> count = 0U;
>
> for (i = 0U, aq_vec = self->aq_vec[0];
> - self->aq_vecs > i; ++i, aq_vec = self->aq_vec[i]) {
> + aq_vec && self->aq_vecs > i; ++i, aq_vec = self->aq_vec[i]) {
> data += count;
> aq_vec_get_sw_stats(aq_vec, data, &count);
> }
> @@ -959,8 +959,10 @@ void aq_nic_free_hot_resources(struct aq_nic_s *self)
> goto err_exit;
>
> for (i = AQ_DIMOF(self->aq_vec); i--;) {
> - if (self->aq_vec[i])
> + if (self->aq_vec[i]) {
> aq_vec_free(self->aq_vec[i]);
> + self->aq_vec[i] = NULL;
> + }
> }
>
> err_exit:;
>
Resolves the ethtool crash.
Tested-by: David Arcari <darcari@redhat.com>
^ permalink raw reply
* [PATCH 6/6] cxgb4: Combine substrings for two messages
From: SF Markus Elfring @ 2017-05-04 20:36 UTC (permalink / raw)
To: netdev, Casey Leedom, Ganesh Goudar; +Cc: LKML, kernel-janitors
In-Reply-To: <a3216042-2461-b397-dfa1-1de178d58513@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 4 May 2017 22:16:57 +0200
The script "checkpatch.pl" pointed information out like the following.
WARNING: quoted string split across lines
Thus fix two source code places.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c
index 32add8dfc253..f9384d8b680d 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c
@@ -1353,8 +1353,9 @@ static int mps_trc_show(struct seq_file *seq, void *v)
if (tp.port < 8) {
i = adap->chan_map[tp.port & 3];
if (i >= MAX_NPORTS) {
- dev_err(adap->pdev_dev, "tracer %u is assigned "
- "to non-existing port\n", trcidx);
+ dev_err(adap->pdev_dev,
+ "tracer %u is assigned to non-existing port\n",
+ trcidx);
return -EINVAL;
}
seq_printf(seq, "tracer is capturing %s %s, ",
@@ -1798,11 +1799,11 @@ static int mps_tcam_show(struct seq_file *seq, void *v)
FW_LDST_CMD_IDX_V(idx));
ret = t4_wr_mbox(adap, adap->mbox, &ldst_cmd,
sizeof(ldst_cmd), &ldst_cmd);
- if (ret)
- dev_warn(adap->pdev_dev, "Can't read MPS "
- "replication map for idx %d: %d\n",
+ if (ret) {
+ dev_warn(adap->pdev_dev,
+ "Can't read MPS replication map for idx %d: %d\n",
idx, -ret);
- else {
+ } else {
mps_rplc = ldst_cmd.u.mps.rplc;
rplc[0] = ntohl(mps_rplc.rplc31_0);
rplc[1] = ntohl(mps_rplc.rplc63_32);
--
2.12.2
^ permalink raw reply related
* [PATCH 5/6] cxgb4: Use seq_puts() in cim_qcfg_show()
From: SF Markus Elfring @ 2017-05-04 20:35 UTC (permalink / raw)
To: netdev, Casey Leedom, Ganesh Goudar; +Cc: LKML, kernel-janitors
In-Reply-To: <a3216042-2461-b397-dfa1-1de178d58513@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 4 May 2017 21:52:32 +0200
A string which did not contain a data format specification should be put
into a sequence. Thus use the corresponding function "seq_puts".
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c
index 2bc40d89f874..32add8dfc253 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c
@@ -357,9 +357,8 @@ static int cim_qcfg_show(struct seq_file *seq, void *v)
return i;
t4_read_cimq_cfg(adap, base, size, thres);
-
- seq_printf(seq,
- " Queue Base Size Thres RdPtr WrPtr SOP EOP Avail\n");
+ seq_puts(seq,
+ " Queue Base Size Thres RdPtr WrPtr SOP EOP Avail\n");
for (i = 0; i < CIM_NUM_IBQ; i++, p += 4)
seq_printf(seq, "%7s %5x %5u %5u %6x %4x %4u %4u %5u\n",
qname[i], base[i], size[i], thres[i],
--
2.12.2
^ permalink raw reply related
* [PATCH 4/6] cxgb4: Replace seven seq_puts() calls by seq_putc()
From: SF Markus Elfring @ 2017-05-04 20:34 UTC (permalink / raw)
To: netdev, Casey Leedom, Ganesh Goudar; +Cc: LKML, kernel-janitors
In-Reply-To: <a3216042-2461-b397-dfa1-1de178d58513@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 4 May 2017 21:40:54 +0200
Seven single characters (line breaks) should be put into a sequence.
Thus use the corresponding function "seq_putc".
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c | 24 +++++++---------------
1 file changed, 7 insertions(+), 17 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c
index 1fa34b009891..2bc40d89f874 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c
@@ -278,7 +278,7 @@ static int cim_ma_la_show(struct seq_file *seq, void *v, int idx)
const u32 *p = v;
if (v == SEQ_START_TOKEN) {
- seq_puts(seq, "\n");
+ seq_putc(seq, '\n');
} else if (idx < CIM_MALA_SIZE) {
seq_printf(seq, "%02x%08x%08x%08x%08x\n",
p[4], p[3], p[2], p[1], p[0]);
@@ -1196,7 +1196,7 @@ static int mboxlog_show(struct seq_file *seq, void *v)
seq_printf(seq, " %08x %08x", hi, lo);
}
- seq_puts(seq, "\n");
+ seq_putc(seq, '\n');
return 0;
}
@@ -2112,9 +2112,7 @@ static int rss_config_show(struct seq_file *seq, void *v)
HASHTOEPLITZ_F));
seq_printf(seq, " Udp4En: %3s\n", yesno(rssconf & UDPENABLE_F));
seq_printf(seq, " Disable: %3s\n", yesno(rssconf & DISABLE_F));
-
- seq_puts(seq, "\n");
-
+ seq_putc(seq, '\n');
rssconf = t4_read_reg(adapter, TP_RSS_CONFIG_TNL_A);
seq_printf(seq, "TP_RSS_CONFIG_TNL: %#x\n", rssconf);
seq_printf(seq, " MaskSize: %3d\n", MASKSIZE_G(rssconf));
@@ -2126,25 +2124,19 @@ static int rss_config_show(struct seq_file *seq, void *v)
yesno(rssconf & HASHETH_F));
}
seq_printf(seq, " UseWireCh: %3s\n", yesno(rssconf & USEWIRECH_F));
-
- seq_puts(seq, "\n");
-
+ seq_putc(seq, '\n');
rssconf = t4_read_reg(adapter, TP_RSS_CONFIG_OFD_A);
seq_printf(seq, "TP_RSS_CONFIG_OFD: %#x\n", rssconf);
seq_printf(seq, " MaskSize: %3d\n", MASKSIZE_G(rssconf));
seq_printf(seq, " RRCplMapEn: %3s\n", yesno(rssconf &
RRCPLMAPEN_F));
seq_printf(seq, " RRCplQueWidth: %3d\n", RRCPLQUEWIDTH_G(rssconf));
-
- seq_puts(seq, "\n");
-
+ seq_putc(seq, '\n');
rssconf = t4_read_reg(adapter, TP_RSS_CONFIG_SYN_A);
seq_printf(seq, "TP_RSS_CONFIG_SYN: %#x\n", rssconf);
seq_printf(seq, " MaskSize: %3d\n", MASKSIZE_G(rssconf));
seq_printf(seq, " UseWireCh: %3s\n", yesno(rssconf & USEWIRECH_F));
-
- seq_puts(seq, "\n");
-
+ seq_putc(seq, '\n');
rssconf = t4_read_reg(adapter, TP_RSS_CONFIG_VRT_A);
seq_printf(seq, "TP_RSS_CONFIG_VRT: %#x\n", rssconf);
if (CHELSIO_CHIP_VERSION(adapter->params.chip) > CHELSIO_T5) {
@@ -2170,9 +2162,7 @@ static int rss_config_show(struct seq_file *seq, void *v)
seq_printf(seq, " VfWrEn: %3s\n", yesno(rssconf & VFWREN_F));
seq_printf(seq, " KeyWrEn: %3s\n", yesno(rssconf & KEYWREN_F));
seq_printf(seq, " KeyWrAddr: %3d\n", KEYWRADDR_G(rssconf));
-
- seq_puts(seq, "\n");
-
+ seq_putc(seq, '\n');
rssconf = t4_read_reg(adapter, TP_RSS_CONFIG_CNG_A);
seq_printf(seq, "TP_RSS_CONFIG_CNG: %#x\n", rssconf);
seq_printf(seq, " ChnCount3: %3s\n", yesno(rssconf & CHNCOUNT3_F));
--
2.12.2
^ permalink raw reply related
* [PATCH 3/6] cxgb4vf: Adjust five checks for null pointers
From: SF Markus Elfring @ 2017-05-04 20:33 UTC (permalink / raw)
To: netdev, Casey Leedom, Ganesh Goudar; +Cc: LKML, kernel-janitors
In-Reply-To: <a3216042-2461-b397-dfa1-1de178d58513@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 4 May 2017 21:20:25 +0200
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
The script “checkpatch.pl” pointed information out like the following.
Comparison to NULL could be written !…
Thus fix the affected source code places.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c b/drivers/net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c
index 9c2690aeb32b..682e844c5a7d 100644
--- a/drivers/net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c
@@ -491,7 +491,7 @@ static int fwevtq_handler(struct sge_rspq *rspq, const __be64 *rsp,
break;
}
tq = s->egr_map[eq_idx];
- if (unlikely(tq == NULL)) {
+ if (unlikely(!tq)) {
dev_err(adapter->pdev_dev,
"Egress Update QID %d TXQ=NULL\n", qid);
break;
@@ -2939,7 +2939,7 @@ static int cxgb4vf_pci_probe(struct pci_dev *pdev,
*/
netdev = alloc_etherdev_mq(sizeof(struct port_info),
MAX_PORT_QSETS);
- if (netdev == NULL) {
+ if (!netdev) {
t4vf_free_vi(adapter, viid);
err = -ENOMEM;
goto err_free_dev;
@@ -3053,7 +3053,7 @@ static int cxgb4vf_pci_probe(struct pci_dev *pdev,
for_each_port(adapter, pidx) {
struct port_info *pi = netdev_priv(adapter->port[pidx]);
netdev = adapter->port[pidx];
- if (netdev == NULL)
+ if (!netdev)
continue;
netif_set_real_num_tx_queues(netdev, pi->nqsets);
@@ -3120,7 +3120,7 @@ static int cxgb4vf_pci_probe(struct pci_dev *pdev,
err_free_dev:
for_each_port(adapter, pidx) {
netdev = adapter->port[pidx];
- if (netdev == NULL)
+ if (!netdev)
continue;
pi = netdev_priv(netdev);
t4vf_free_vi(adapter, pi->viid);
@@ -3197,7 +3197,7 @@ static void cxgb4vf_pci_remove(struct pci_dev *pdev)
struct net_device *netdev = adapter->port[pidx];
struct port_info *pi;
- if (netdev == NULL)
+ if (!netdev)
continue;
pi = netdev_priv(netdev);
--
2.12.2
^ permalink raw reply related
* [PATCH 2/6] cxgb4vf: Combine substrings for 24 messages
From: SF Markus Elfring @ 2017-05-04 20:32 UTC (permalink / raw)
To: netdev, Casey Leedom, Ganesh Goudar; +Cc: LKML, kernel-janitors
In-Reply-To: <a3216042-2461-b397-dfa1-1de178d58513@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 4 May 2017 21:00:20 +0200
The script "checkpatch.pl" pointed information out like the following.
WARNING: quoted string split across lines
Thus fix the affected source code places.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
.../net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c | 113 ++++++++++++---------
1 file changed, 64 insertions(+), 49 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c b/drivers/net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c
index 4ac9316f3081..9c2690aeb32b 100644
--- a/drivers/net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c
@@ -226,17 +226,20 @@ void t4vf_os_portmod_changed(struct adapter *adapter, int pidx)
dev_info(adapter->pdev_dev, "%s: %s port module inserted\n",
dev->name, mod_str[pi->mod_type]);
else if (pi->mod_type == FW_PORT_MOD_TYPE_NOTSUPPORTED)
- dev_info(adapter->pdev_dev, "%s: unsupported optical port "
- "module inserted\n", dev->name);
+ dev_info(adapter->pdev_dev,
+ "%s: unsupported optical port module inserted\n",
+ dev->name);
else if (pi->mod_type == FW_PORT_MOD_TYPE_UNKNOWN)
- dev_info(adapter->pdev_dev, "%s: unknown port module inserted,"
- "forcing TWINAX\n", dev->name);
+ dev_info(adapter->pdev_dev,
+ "%s: unknown port module inserted,forcing TWINAX\n",
+ dev->name);
else if (pi->mod_type == FW_PORT_MOD_TYPE_ERROR)
dev_info(adapter->pdev_dev, "%s: transceiver module error\n",
dev->name);
else
- dev_info(adapter->pdev_dev, "%s: unknown module type %d "
- "inserted\n", dev->name, pi->mod_type);
+ dev_info(adapter->pdev_dev,
+ "%s: unknown module type %d inserted\n",
+ dev->name, pi->mod_type);
}
/*
@@ -2357,8 +2360,9 @@ static void size_nports_qsets(struct adapter *adapter)
*/
adapter->params.nports = vfres->nvi;
if (adapter->params.nports > MAX_NPORTS) {
- dev_warn(adapter->pdev_dev, "only using %d of %d maximum"
- " allowed virtual interfaces\n", MAX_NPORTS,
+ dev_warn(adapter->pdev_dev,
+ "only using %d of %d maximum allowed virtual interfaces\n",
+ MAX_NPORTS,
adapter->params.nports);
adapter->params.nports = MAX_NPORTS;
}
@@ -2370,9 +2374,9 @@ static void size_nports_qsets(struct adapter *adapter)
*/
pmask_nports = hweight32(adapter->params.vfres.pmask);
if (pmask_nports < adapter->params.nports) {
- dev_warn(adapter->pdev_dev, "only using %d of %d provisioned"
- " virtual interfaces; limited by Port Access Rights"
- " mask %#x\n", pmask_nports, adapter->params.nports,
+ dev_warn(adapter->pdev_dev,
+ "only using %d of %d provisioned virtual interfaces; limited by Port Access Rights mask %#x\n",
+ pmask_nports, adapter->params.nports,
adapter->params.vfres.pmask);
adapter->params.nports = pmask_nports;
}
@@ -2403,8 +2407,8 @@ static void size_nports_qsets(struct adapter *adapter)
adapter->sge.max_ethqsets = ethqsets;
if (adapter->sge.max_ethqsets < adapter->params.nports) {
- dev_warn(adapter->pdev_dev, "only using %d of %d available"
- " virtual interfaces (too few Queue Sets)\n",
+ dev_warn(adapter->pdev_dev,
+ "only using %d of %d available virtual interfaces (too few Queue Sets)\n",
adapter->sge.max_ethqsets, adapter->params.nports);
adapter->params.nports = adapter->sge.max_ethqsets;
}
@@ -2448,38 +2452,44 @@ static int adap_init0(struct adapter *adapter)
*/
err = t4vf_get_dev_params(adapter);
if (err) {
- dev_err(adapter->pdev_dev, "unable to retrieve adapter"
- " device parameters: err=%d\n", err);
+ dev_err(adapter->pdev_dev,
+ "unable to retrieve adapter device parameters: err=%d\n",
+ err);
return err;
}
err = t4vf_get_vpd_params(adapter);
if (err) {
- dev_err(adapter->pdev_dev, "unable to retrieve adapter"
- " VPD parameters: err=%d\n", err);
+ dev_err(adapter->pdev_dev,
+ "unable to retrieve adapter VPD parameters: err=%d\n",
+ err);
return err;
}
err = t4vf_get_sge_params(adapter);
if (err) {
- dev_err(adapter->pdev_dev, "unable to retrieve adapter"
- " SGE parameters: err=%d\n", err);
+ dev_err(adapter->pdev_dev,
+ "unable to retrieve adapter SGE parameters: err=%d\n",
+ err);
return err;
}
err = t4vf_get_rss_glb_config(adapter);
if (err) {
- dev_err(adapter->pdev_dev, "unable to retrieve adapter"
- " RSS parameters: err=%d\n", err);
+ dev_err(adapter->pdev_dev,
+ "unable to retrieve adapter RSS parameters: err=%d\n",
+ err);
return err;
}
if (adapter->params.rss.mode !=
FW_RSS_GLB_CONFIG_CMD_MODE_BASICVIRTUAL) {
- dev_err(adapter->pdev_dev, "unable to operate with global RSS"
- " mode %d\n", adapter->params.rss.mode);
+ dev_err(adapter->pdev_dev,
+ "unable to operate with global RSS mode %d\n",
+ adapter->params.rss.mode);
return -EINVAL;
}
err = t4vf_sge_init(adapter);
if (err) {
- dev_err(adapter->pdev_dev, "unable to use adapter parameters:"
- " err=%d\n", err);
+ dev_err(adapter->pdev_dev,
+ "unable to use adapter parameters: err=%d\n",
+ err);
return err;
}
@@ -2522,20 +2532,21 @@ static int adap_init0(struct adapter *adapter)
*/
err = t4vf_get_vfres(adapter);
if (err) {
- dev_err(adapter->pdev_dev, "unable to get virtual interface"
- " resources: err=%d\n", err);
+ dev_err(adapter->pdev_dev,
+ "unable to get virtual interface resources: err=%d\n",
+ err);
return err;
}
/* Check for various parameter sanity issues */
if (adapter->params.vfres.pmask == 0) {
- dev_err(adapter->pdev_dev, "no port access configured\n"
- "usable!\n");
+ dev_err(adapter->pdev_dev,
+ "no port access configured/usable!\n");
return -EINVAL;
}
if (adapter->params.vfres.nvi == 0) {
- dev_err(adapter->pdev_dev, "no virtual interfaces configured/"
- "usable!\n");
+ dev_err(adapter->pdev_dev,
+ "no virtual interfaces configured/usable!\n");
return -EINVAL;
}
@@ -2726,8 +2737,9 @@ static int enable_msix(struct adapter *adapter)
nqsets = want - MSIX_EXTRAS;
if (nqsets < s->max_ethqsets) {
- dev_warn(adapter->pdev_dev, "only enough MSI-X vectors"
- " for %d Queue Sets\n", nqsets);
+ dev_warn(adapter->pdev_dev,
+ "only enough MSI-X vectors for %d Queue Sets\n",
+ nqsets);
s->max_ethqsets = nqsets;
if (nqsets < s->ethqsets)
reduce_ethqs(adapter, nqsets);
@@ -2804,8 +2816,8 @@ static int cxgb4vf_pci_probe(struct pci_dev *pdev,
if (err == 0) {
err = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(64));
if (err) {
- dev_err(&pdev->dev, "unable to obtain 64-bit DMA for"
- " coherent allocations\n");
+ dev_err(&pdev->dev,
+ "unable to obtain 64-bit DMA for coherent allocations\n");
goto err_release_regions;
}
pci_using_dac = 1;
@@ -2866,8 +2878,9 @@ static int cxgb4vf_pci_probe(struct pci_dev *pdev,
*/
err = t4vf_prep_adapter(adapter);
if (err) {
- dev_err(adapter->pdev_dev, "device didn't become ready:"
- " err=%d\n", err);
+ dev_err(adapter->pdev_dev,
+ "device didn't become ready: err=%d\n",
+ err);
goto err_unmap_bar0;
}
@@ -2914,8 +2927,9 @@ static int cxgb4vf_pci_probe(struct pci_dev *pdev,
pmask &= ~(1 << port_id);
viid = t4vf_alloc_vi(adapter, port_id);
if (viid < 0) {
- dev_err(&pdev->dev, "cannot allocate VI for port %d:"
- " err=%d\n", port_id, viid);
+ dev_err(&pdev->dev,
+ "cannot allocate VI for port %d: err=%d\n",
+ port_id, viid);
err = viid;
goto err_free_dev;
}
@@ -2978,8 +2992,8 @@ static int cxgb4vf_pci_probe(struct pci_dev *pdev,
err = t4vf_get_vf_mac_acl(adapter, pf, &naddr, mac);
if (err) {
dev_err(&pdev->dev,
- "unable to determine MAC ACL address, "
- "continuing anyway.. (status %d)\n", err);
+ "unable to determine MAC ACL address, continuing anyway.. (status %d)\n",
+ err);
} else if (naddr && adapter->params.vfres.nvi == 1) {
struct sockaddr addr;
@@ -3006,8 +3020,7 @@ static int cxgb4vf_pci_probe(struct pci_dev *pdev,
else {
if (msi == MSI_MSIX) {
dev_info(adapter->pdev_dev,
- "Unable to use MSI-X Interrupts; falling "
- "back to MSI Interrupts\n");
+ "Unable to use MSI-X Interrupts; falling back to MSI Interrupts\n");
/* We're going to need a Forwarded Interrupt Queue so
* that may cut into how many Queue Sets we can
@@ -3018,8 +3031,9 @@ static int cxgb4vf_pci_probe(struct pci_dev *pdev,
}
err = pci_enable_msi(pdev);
if (err) {
- dev_err(&pdev->dev, "Unable to allocate MSI Interrupts;"
- " err=%d\n", err);
+ dev_err(&pdev->dev,
+ "Unable to allocate MSI Interrupts; err=%d\n",
+ err);
goto err_free_dev;
}
adapter->flags |= USING_MSI;
@@ -3047,8 +3061,9 @@ static int cxgb4vf_pci_probe(struct pci_dev *pdev,
err = register_netdev(netdev);
if (err) {
- dev_warn(&pdev->dev, "cannot register net device %s,"
- " skipping\n", netdev->name);
+ dev_warn(&pdev->dev,
+ "cannot register net device %s, skipping\n",
+ netdev->name);
continue;
}
@@ -3067,8 +3082,8 @@ static int cxgb4vf_pci_probe(struct pci_dev *pdev,
debugfs_create_dir(pci_name(pdev),
cxgb4vf_debugfs_root);
if (IS_ERR_OR_NULL(adapter->debugfs_root))
- dev_warn(&pdev->dev, "could not create debugfs"
- " directory");
+ dev_warn(&pdev->dev,
+ "could not create debugfs directory");
else
setup_debugfs(adapter);
}
--
2.12.2
^ permalink raw reply related
* [PATCH 1/6] cxgb4vf: Use seq_putc() in mboxlog_show()
From: SF Markus Elfring @ 2017-05-04 20:31 UTC (permalink / raw)
To: netdev, Casey Leedom, Ganesh Goudar; +Cc: LKML, kernel-janitors
In-Reply-To: <a3216042-2461-b397-dfa1-1de178d58513@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 4 May 2017 20:02:04 +0200
A single character (line break) should be put into a sequence.
Thus use the corresponding function "seq_putc".
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c b/drivers/net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c
index ac7a150c54e9..4ac9316f3081 100644
--- a/drivers/net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c
@@ -1818,7 +1818,7 @@ static int mboxlog_show(struct seq_file *seq, void *v)
seq_printf(seq, " %08x %08x", hi, lo);
}
- seq_puts(seq, "\n");
+ seq_putc(seq, '\n');
return 0;
}
--
2.12.2
^ permalink raw reply related
* [PATCH 0/6] cxgb4: Fine-tuning for some function implementations
From: SF Markus Elfring @ 2017-05-04 20:30 UTC (permalink / raw)
To: netdev, Casey Leedom, Ganesh Goudar; +Cc: LKML, kernel-janitors
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 4 May 2017 22:23:45 +0200
A few update suggestions were taken into account
from static source code analysis.
Markus Elfring (6):
Use seq_putc() in mboxlog_show()
Combine substrings for 24 messages
Adjust five checks for null pointers
Replace seven seq_puts() calls by seq_putc()
Use seq_puts() in cim_qcfg_show()
Combine substrings for two messages
drivers/net/ethernet/chelsio/cxgb4/cxgb4_debugfs.c | 42 +++----
.../net/ethernet/chelsio/cxgb4vf/cxgb4vf_main.c | 125 ++++++++++++---------
2 files changed, 86 insertions(+), 81 deletions(-)
--
2.12.2
^ permalink raw reply
* Re: [PATCH] aquantia: Fix "ethtool -S" crash when adapter down.
From: Joe Perches @ 2017-05-04 20:30 UTC (permalink / raw)
To: Pavel Belous, David S . Miller
Cc: netdev, David Arcari, Lino Sanfilippo, Simon Edelhaus
In-Reply-To: <9ea5a2ba21b4f8b22a63acf39135a41e2f1da23c.1493928470.git.pavel.belous@aquantia.com>
On Thu, 2017-05-04 at 23:10 +0300, Pavel Belous wrote:
> diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
[]
> @@ -959,8 +959,10 @@ void aq_nic_free_hot_resources(struct aq_nic_s *self)
> goto err_exit;
>
> for (i = AQ_DIMOF(self->aq_vec); i--;) {
> - if (self->aq_vec[i])
> + if (self->aq_vec[i]) {
> aq_vec_free(self->aq_vec[i]);
> + self->aq_vec[i] = NULL;
> + }
> }
>
> err_exit:;
unrelated style trivia:
err_exit:;
the error label then :; is pretty odd.
Also casting the return value to void is really odd.
A simple return instead of goto would be more common.
drivers/net/ethernet/aquantia/atlantic/aq_ring.c:311:err_exit:;
drivers/net/ethernet/aquantia/atlantic/aq_ring.c:326:err_exit:;
drivers/net/ethernet/aquantia/atlantic/aq_main.c:161:err_exit:;
drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.c:277:err_exit:;
drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.c:313:err_exit:;
drivers/net/ethernet/aquantia/atlantic/aq_nic.c:304:err_exit:;
drivers/net/ethernet/aquantia/atlantic/aq_nic.c:763:err_exit:;
drivers/net/ethernet/aquantia/atlantic/aq_nic.c:951:err_exit:;
drivers/net/ethernet/aquantia/atlantic/aq_nic.c:966:err_exit:;
drivers/net/ethernet/aquantia/atlantic/aq_vec.c:281:err_exit:;
drivers/net/ethernet/aquantia/atlantic/aq_vec.c:302:err_exit:;
drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c:251:err_exit:;
drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c:270:err_exit:;
^ permalink raw reply
* [PATCH] aquantia: Fix "ethtool -S" crash when adapter down.
From: Pavel Belous @ 2017-05-04 20:10 UTC (permalink / raw)
To: David S . Miller; +Cc: netdev, David Arcari, Lino Sanfilippo, Simon Edelhaus
From: Pavel Belous <pavel.belous@aquantia.com>
This patch fixes the crash that happens when driver tries to collect statistics
from already released "aq_vec" object.
If adapter is in "down" state we still allow user to see statistics from HW.
V2: fixed braces around "aq_vec_free".
Fixes: 97bde5c4f909 ("net: ethernet: aquantia: Support for NIC-specific code")
Signed-off-by: Pavel Belous <pavel.belous@aquantia.com>
---
drivers/net/ethernet/aquantia/atlantic/aq_nic.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
index cdb0299..9ee1c50 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
@@ -755,7 +755,7 @@ void aq_nic_get_stats(struct aq_nic_s *self, u64 *data)
count = 0U;
for (i = 0U, aq_vec = self->aq_vec[0];
- self->aq_vecs > i; ++i, aq_vec = self->aq_vec[i]) {
+ aq_vec && self->aq_vecs > i; ++i, aq_vec = self->aq_vec[i]) {
data += count;
aq_vec_get_sw_stats(aq_vec, data, &count);
}
@@ -959,8 +959,10 @@ void aq_nic_free_hot_resources(struct aq_nic_s *self)
goto err_exit;
for (i = AQ_DIMOF(self->aq_vec); i--;) {
- if (self->aq_vec[i])
+ if (self->aq_vec[i]) {
aq_vec_free(self->aq_vec[i]);
+ self->aq_vec[i] = NULL;
+ }
}
err_exit:;
--
2.7.4
^ permalink raw reply related
* Re: [PATCH] iproute2: hide devices starting with period by default
From: David Ahern @ 2017-05-04 19:47 UTC (permalink / raw)
To: Florian Fainelli, nicolas.dichtel; +Cc: David Miller, stephen, netdev
In-Reply-To: <26442035-25fd-ff9a-2804-ed21ec6198cf@gmail.com>
On 5/4/17 1:10 PM, Florian Fainelli wrote:
> On 05/04/2017 09:37 AM, David Ahern wrote:
>> On 5/4/17 9:15 AM, Nicolas Dichtel wrote:
>>> Le 24/02/2017 à 16:52, David Ahern a écrit :
>>>> On 2/23/17 8:12 PM, David Miller wrote:
>>>>> This really need to be a fundamental facility, so that it transparently
>>>>> works for NetworkManager, router daemons, everything. Not just iproute2
>>>>> and "ls".
>>>>
>>>> I'll rebase my patch and send out as RFC.
>>>>
>>> David, did you finally send those patches?
>>>
>>
>> No, but for a few reasons.
>>
>> It is easy to hide devices in a dump:
>>
>> https://github.com/dsahern/linux/commit/48a80a00eac284e58bae04af10a5a932dd7aee00
>>
>>
>> But I think those devices should also not exist in sysfs or procfs which
>> overlaps what I would like to see for lightweight netdevices:
>>
>> https://github.com/dsahern/linux/commit/70574be699cf252e77f71e3df11192438689f976
>
> Interesting that does indeed solve the same problems as the L2 only
> patch set intended. I am not exactly sure if hiding the devices from
> procfs/sysfs would be appropriate in my case (dumb L2 only switch that
> only does 802.1q for instance), but why not.
>
>
>>
>>
>> and to be complete, hidden devices should not be allowed to have a
>> network address or transmit packets which is the L2 only intent from
>> Florian:
>> https://www.spinics.net/lists/netdev/msg340808.html
>>
>
> Do you plan on submitting the LWT patch set at some point?
Definitely. Maybe I can find some time this weekend.
^ permalink raw reply
* [PATCH] net: ipv4: add code comment for clarification
From: Gustavo A. R. Silva @ 2017-05-04 19:44 UTC (permalink / raw)
To: David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy
Cc: netdev, linux-kernel, Gustavo A. R. Silva, Joe Perches
In-Reply-To: <20170504142432.Horde.g9y26Ryxbtg1EIl_cnsdbbw@gator4166.hostgator.com>
Add code comment to make it clear that the position of the arguments
req->id.idiag_dport and req->id.idiag_sport is a locked in behavior
and it should not be changed.
Addresses-Coverity-ID: 1357474
Cc: David Miller <davem@davemloft.net>
Cc: Joe Perches <joe@perches.com>
Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
---
net/ipv4/inet_diag.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/net/ipv4/inet_diag.c b/net/ipv4/inet_diag.c
index 3828b3a..841800b 100644
--- a/net/ipv4/inet_diag.c
+++ b/net/ipv4/inet_diag.c
@@ -389,6 +389,12 @@ static int sk_diag_fill(struct sock *sk, struct sk_buff *skb,
nlmsg_flags, unlh, net_admin);
}
+/*
+ * Ignore the position of the arguments req->id.idiag_dport and
+ * req->id.idiag_sport in both calls to inet_lookup() and inet6_lookup()
+ * functions, once this is a locked in behavior exposed to user space.
+ * Changing this will break things for people.
+ */
struct sock *inet_diag_find_one_icsk(struct net *net,
struct inet_hashinfo *hashinfo,
const struct inet_diag_req_v2 *req)
--
2.5.0
^ permalink raw reply related
* Patch "netlink: Allow direct reclaim for fallback allocation" has been added to the 4.4-stable tree
From: gregkh @ 2017-05-04 19:43 UTC (permalink / raw)
To: ross.lagerwall, davem, edumazet, gregkh, linux-kernel, netdev,
stable
Cc: stable, stable-commits
In-Reply-To: <1493801059-2828-1-git-send-email-ross.lagerwall@citrix.com>
This is a note to let you know that I've just added the patch titled
netlink: Allow direct reclaim for fallback allocation
to the 4.4-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
netlink-allow-direct-reclaim-for-fallback-allocation.patch
and it can be found in the queue-4.4 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@vger.kernel.org> know about it.
>From ross.lagerwall@citrix.com Thu May 4 12:37:51 2017
From: Ross Lagerwall <ross.lagerwall@citrix.com>
Date: Wed, 3 May 2017 09:44:19 +0100
Subject: netlink: Allow direct reclaim for fallback allocation
To: <stable@vger.kernel.org>
Cc: Ross Lagerwall <ross.lagerwall@citrix.com>, "David S. Miller" <davem@davemloft.net>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Eric Dumazet <edumazet@google.com>, <netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Message-ID: <1493801059-2828-1-git-send-email-ross.lagerwall@citrix.com>
From: Ross Lagerwall <ross.lagerwall@citrix.com>
The backport of d35c99ff77ec ("netlink: do not enter direct reclaim from
netlink_dump()") to the 4.4 branch (first in 4.4.32) mistakenly removed
direct claim from the initial large allocation _and_ the fallback
allocation which means that allocations can spuriously fail.
Fix the issue by adding back the direct reclaim flag to the fallback
allocation.
Fixes: 6d123f1d396b ("netlink: do not enter direct reclaim from netlink_dump()")
Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
Note that this is only for the 4.4 branch as the regression is only in
this branch. Consequently, there is no corresponding upstream commit.
I'm resending this to the linux-stable list since I now understand the
netdev maintainer only handles backports for the last couple of versions
of Linux.
net/netlink/af_netlink.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -2107,7 +2107,7 @@ static int netlink_dump(struct sock *sk)
if (!skb) {
alloc_size = alloc_min_size;
skb = netlink_alloc_skb(sk, alloc_size, nlk->portid,
- (GFP_KERNEL & ~__GFP_DIRECT_RECLAIM));
+ GFP_KERNEL);
}
if (!skb)
goto errout_skb;
Patches currently in stable-queue which might be from ross.lagerwall@citrix.com are
queue-4.4/netlink-allow-direct-reclaim-for-fallback-allocation.patch
^ permalink raw reply
* Re: [RFC iproute2 0/8] RDMA tool
From: Leon Romanovsky @ 2017-05-04 19:42 UTC (permalink / raw)
To: Dennis Dalessandro
Cc: Bart Van Assche, jiri-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
ram.amrani-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org,
sagi-NQWnxTmZq1alnMjI0IkVqw@public.gmane.org,
ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org,
hch-jcswGhMUV9g@public.gmane.org,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org,
stephen-OTpzqLSitTUnbdJkjeBofR2eb7JE58TQ@public.gmane.org,
dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
ariela-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org
In-Reply-To: <2dee6cde-0406-b101-0fe6-c1f6de7c1b1a-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 3598 bytes --]
On Thu, May 04, 2017 at 03:26:13PM -0400, Dennis Dalessandro wrote:
> On 05/04/2017 02:45 PM, Leon Romanovsky wrote:
> > On Thu, May 04, 2017 at 06:30:27PM +0000, Bart Van Assche wrote:
> > > On Thu, 2017-05-04 at 21:25 +0300, Leon Romanovsky wrote:
> > > > On Thu, May 04, 2017 at 06:10:54PM +0000, Bart Van Assche wrote:
> > > > > On Thu, 2017-05-04 at 21:02 +0300, Leon Romanovsky wrote:
> > > > > > Following our discussion both in mailing list [1] and at the LPC 2016 [2],
> > > > > > we would like to propose this RDMA tool to be part of iproute2 package
> > > > > > and finally improve this situation.
> > > > >
> > > > > Hello Leon,
> > > > >
> > > > > Although I really appreciate your work: can you clarify why you would like to
> > > > > add *RDMA* functionality to an *IP routing* tool? I haven't found any motivation
> > > > > for adding RDMA functionality to iproute2 in [1].
> > > >
> > > > We are planning to reuse the same infrastructure provided by iproute2,
> > > > like netlink parsing, access to distributions, same CLI and same standards.
> > > >
> > > > Right now, RDMA is already tightened to netdev: iWARP, RoCE, IPoIB, HFI-VNIC.
> > > > Many drivers (mlx, qed, i40, cxgb) are sharing code between net and
> > > > RDMA.
> > > >
> > > > I do expect that iproute2 will be installed on every machine with any
> > > > type of connection, including IB and OPA.
> > > >
> > > > So I think that it is enough to be part of that suite and don't invent
> > > > our own for one specific tool.
> > >
> > > Hello Leon,
> > >
> > > Sorry but to me that sounds like a weak argument for including RDMA functionality
> > > in iproute2. There is already a library for communication over netlink sockets,
> > > namely libnl. Is there functionality that is in iproute2 but not in libnl and
> > > that is needed for the new tool? If so, have you considered to create a new
> > > library for that functionality?
> >
> > It is not hard to create new tool, the hardest part is to ensure that it is
> > part of the distributions. Did you count how many months we are trying to
> > add rdma-core to debian?
>
> I do agree that it is a strange pairing and am not really a fan. However at
> the end of the day it's just a name for a repo/package. If the iproute folks
> are fine to include rdma in their repo/package, great we can leverage their
> code for CLI and other common stuff.
>
> Now if the interface was something like "ip -FlagForRdma ..." I would object
> to that, but the interface is "rdma ... " so from users perspective it's
> different tools. They don't need to care that it was sourced from a common
> git repo.
>
> Just as an aside this already works a bit with OPA:
>
> $ ./rdma link
> 1/1: hfi1_0/1: ifname NONE cap_mask 0x00410022 lid 0x1 lid_mask_count 0
> link_layer InfiniBand
> phys_state 5: LinkUp rate 100 Gb/sec (4X EDR) sm_lid 0x1 sm_sl 0
> state 4: ACTIVE
>
> Leon I'll get you more feedback and testing, I've just been really bogged
> down this week, sorry.
Thanks Denny,
Before you are starting to test it, can you please provide your feedback
on my initial questions? Usability and need of sysfs.
----
This is initial phase to understand if user experience for this tool fits
RDMA and netdev communities exepectations. Also I would like to get feedback
if it is really worth to provide legacy sysfs for old kernels, or maybe I should
implement netlink from the beginning and abandon sysfs completely.
-----
P.S. I believe this will give you wrong output, because it parses IB port cap_mask.
$./rdma link show hfi1_0/1 cap_mask
Thanks
>
> -Denny
>
>
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [Patch net v2] ipv6: reorder ip6_route_dev_notifier after ipv6_dev_notf
From: David Ahern @ 2017-05-04 19:41 UTC (permalink / raw)
To: Cong Wang, netdev; +Cc: andreyknvl
In-Reply-To: <1493919363-19989-1-git-send-email-xiyou.wangcong@gmail.com>
On 5/4/17 11:36 AM, Cong Wang wrote:
> For each netns (except init_net), we initialize its null entry
> in 3 places:
>
> 1) The template itself, as we use kmemdup()
> 2) Code around dst_init_metrics() in ip6_route_net_init()
> 3) ip6_route_dev_notify(), which is supposed to initialize it after
> loopback registers
>
> Unfortunately the last one still happens in a wrong order because
> we expect to initialize net->ipv6.ip6_null_entry->rt6i_idev to
> net->loopback_dev's idev, so we have to do that after we add
> idev to it. However, this notifier has priority == 0 same as
> ipv6_dev_notf, and ipv6_dev_notf is registered after
> ip6_route_dev_notifier so it is called actually after
> ip6_route_dev_notifier.
>
> Fix it by picking a smaller priority for ip6_route_dev_notifier.
> Also, we have to release the refcnt accordingly when unregistering
> loopback_dev because device exit functions are called before subsys
> exit functions.
>
> Cc: David Ahern <dsahern@gmail.com>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
> ---
Commit message needs a tie in to the problem that Andrey reported. It
solves the same problem for namespaces other than init_net.
Acked-by: David Ahern <dsahern@gmail.com>
Tested-by: David Ahern <dsahern@gmail.com>
^ permalink raw reply
* Re: [PATCH RESEND 4.4-only] netlink: Allow direct reclaim for fallback allocation
From: Greg Kroah-Hartman @ 2017-05-04 19:38 UTC (permalink / raw)
To: Ross Lagerwall
Cc: stable, David S. Miller, Eric Dumazet, netdev, linux-kernel
In-Reply-To: <1493801059-2828-1-git-send-email-ross.lagerwall@citrix.com>
On Wed, May 03, 2017 at 09:44:19AM +0100, Ross Lagerwall wrote:
> The backport of d35c99ff77ec ("netlink: do not enter direct reclaim from
> netlink_dump()") to the 4.4 branch (first in 4.4.32) mistakenly removed
> direct claim from the initial large allocation _and_ the fallback
> allocation which means that allocations can spuriously fail.
> Fix the issue by adding back the direct reclaim flag to the fallback
> allocation.
>
> Fixes: 6d123f1d396b ("netlink: do not enter direct reclaim from netlink_dump()")
> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
> ---
>
> Note that this is only for the 4.4 branch as the regression is only in
> this branch. Consequently, there is no corresponding upstream commit.
>
> I'm resending this to the linux-stable list since I now understand the
> netdev maintainer only handles backports for the last couple of versions
> of Linux.
>
Many thanks for this fix, now queued up.
greg k-h
^ permalink raw reply
* Re: [RFC iproute2 0/8] RDMA tool
From: Dennis Dalessandro @ 2017-05-04 19:26 UTC (permalink / raw)
To: Leon Romanovsky, Bart Van Assche
Cc: jiri@mellanox.com, linux-rdma@vger.kernel.org,
ram.amrani@cavium.com, sagi@grimberg.me, ogerlitz@mellanox.com,
hch@lst.de, netdev@vger.kernel.org,
jgunthorpe@obsidianresearch.com, stephen@networkplumber.org,
dledford@redhat.com, ariela@mellanox.com
In-Reply-To: <20170504184531.GE22833@mtr-leonro.local>
On 05/04/2017 02:45 PM, Leon Romanovsky wrote:
> On Thu, May 04, 2017 at 06:30:27PM +0000, Bart Van Assche wrote:
>> On Thu, 2017-05-04 at 21:25 +0300, Leon Romanovsky wrote:
>>> On Thu, May 04, 2017 at 06:10:54PM +0000, Bart Van Assche wrote:
>>>> On Thu, 2017-05-04 at 21:02 +0300, Leon Romanovsky wrote:
>>>>> Following our discussion both in mailing list [1] and at the LPC 2016 [2],
>>>>> we would like to propose this RDMA tool to be part of iproute2 package
>>>>> and finally improve this situation.
>>>>
>>>> Hello Leon,
>>>>
>>>> Although I really appreciate your work: can you clarify why you would like to
>>>> add *RDMA* functionality to an *IP routing* tool? I haven't found any motivation
>>>> for adding RDMA functionality to iproute2 in [1].
>>>
>>> We are planning to reuse the same infrastructure provided by iproute2,
>>> like netlink parsing, access to distributions, same CLI and same standards.
>>>
>>> Right now, RDMA is already tightened to netdev: iWARP, RoCE, IPoIB, HFI-VNIC.
>>> Many drivers (mlx, qed, i40, cxgb) are sharing code between net and
>>> RDMA.
>>>
>>> I do expect that iproute2 will be installed on every machine with any
>>> type of connection, including IB and OPA.
>>>
>>> So I think that it is enough to be part of that suite and don't invent
>>> our own for one specific tool.
>>
>> Hello Leon,
>>
>> Sorry but to me that sounds like a weak argument for including RDMA functionality
>> in iproute2. There is already a library for communication over netlink sockets,
>> namely libnl. Is there functionality that is in iproute2 but not in libnl and
>> that is needed for the new tool? If so, have you considered to create a new
>> library for that functionality?
>
> It is not hard to create new tool, the hardest part is to ensure that it is
> part of the distributions. Did you count how many months we are trying to
> add rdma-core to debian?
I do agree that it is a strange pairing and am not really a fan. However
at the end of the day it's just a name for a repo/package. If the
iproute folks are fine to include rdma in their repo/package, great we
can leverage their code for CLI and other common stuff.
Now if the interface was something like "ip -FlagForRdma ..." I would
object to that, but the interface is "rdma ... " so from users
perspective it's different tools. They don't need to care that it was
sourced from a common git repo.
Just as an aside this already works a bit with OPA:
$ ./rdma link
1/1: hfi1_0/1: ifname NONE cap_mask 0x00410022 lid 0x1 lid_mask_count 0
link_layer InfiniBand
phys_state 5: LinkUp rate 100 Gb/sec (4X EDR) sm_lid 0x1 sm_sl
0 state 4: ACTIVE
Leon I'll get you more feedback and testing, I've just been really
bogged down this week, sorry.
-Denny
^ permalink raw reply
* Re: [net-ipv4] question about arguments position
From: Gustavo A. R. Silva @ 2017-05-04 19:24 UTC (permalink / raw)
To: Joe Perches
Cc: David Miller, kuznet, jmorris, yoshfuji, kaber, netdev,
linux-kernel
In-Reply-To: <1493925433.31950.0.camel@perches.com>
Quoting Joe Perches <joe@perches.com>:
[]
>> > > +/*
>> > > + * Ignore the position of the arguments req->id.idiag_dport and
>> > > + * req->id.idiag_sport in both calls to inet_lookup() and
>> inet6_lookup()
>> > > + * functions, once this is a locked in behavior exposed to user space.
>> > > + * Changing this will break things for people.
>> > > + */
>> > > struct sock *inet_diag_find_one_icsk(struct net *net,
>> > > struct inet_hashinfo *hashinfo,
>> > > const struct
>> inet_diag_req_v2 *req)
>> > >
>> >
>> > Seems sensible. Thanks.
>>
>> Should I resend it in a full and proper format or it can taken from here?
>
> If you want it applied, it should be resent as a full patch
> with your sign-off.
I'll send it shortly.
Thanks for clarifying
--
Gustavo A. R. Silva
^ permalink raw reply
* Re: [net-ipv4] question about arguments position
From: Gustavo A. R. Silva @ 2017-05-04 19:00 UTC (permalink / raw)
To: Joe Perches
Cc: David Miller, kuznet, jmorris, yoshfuji, kaber, netdev,
linux-kernel
In-Reply-To: <1493920071.22125.37.camel@perches.com>
Hi Joe,
Quoting Joe Perches <joe@perches.com>:
> On Thu, 2017-05-04 at 12:46 -0400, David Miller wrote:
>> From: "Gustavo A. R. Silva" <garsilva@embeddedor.com>
>> Date: Thu, 04 May 2017 11:07:54 -0500
>>
>> > While looking into Coverity ID 1357474 I ran into the following piece
>> > of code at net/ipv4/inet_diag.c:392:
>>
>> Because it's been this way since at least 2005, it doesn't matter if
>> the order is correct or not. What's there is the locked in behavior
>> exposed to userspace and changing it will break things for people.
>
> Adding a few comments around the code about why
> it is this way will help avoid future questions.
In the case of Coverity, I already triaged and documented this issue.
So people can ignore it in the future.
Regarding the code comments, what about the following patch:
diff --git a/net/ipv4/inet_diag.c b/net/ipv4/inet_diag.c
index 3828b3a..7a56641 100644
--- a/net/ipv4/inet_diag.c
+++ b/net/ipv4/inet_diag.c
@@ -389,6 +389,12 @@ static int sk_diag_fill(struct sock *sk, struct
sk_buff *skb,
nlmsg_flags, unlh, net_admin);
}
+/*
+ * Ignore the position of the arguments req->id.idiag_dport and
+ * req->id.idiag_sport in both calls to inet_lookup() and inet6_lookup()
+ * functions, once this is a locked in behavior exposed to user space.
+ * Changing this will break things for people.
+ */
struct sock *inet_diag_find_one_icsk(struct net *net,
struct inet_hashinfo *hashinfo,
const struct inet_diag_req_v2 *req)
Thanks
^ permalink raw reply related
* Re: [net-ipv4] question about arguments position
From: Joe Perches @ 2017-05-04 19:17 UTC (permalink / raw)
To: Gustavo A. R. Silva
Cc: David Miller, kuznet, jmorris, yoshfuji, kaber, netdev,
linux-kernel
In-Reply-To: <20170504141548.Horde.VjUEZJdTZiLesC8PBMAO8QM@gator4166.hostgator.com>
On Thu, 2017-05-04 at 14:15 -0500, Gustavo A. R. Silva wrote:
> Quoting Joe Perches <joe@perches.com>:
>
> > On Thu, 2017-05-04 at 14:00 -0500, Gustavo A. R. Silva wrote:
> > > Regarding the code comments, what about the following patch:
> >
> > []
> > > diff --git a/net/ipv4/inet_diag.c b/net/ipv4/inet_diag.c
> >
> > []
> > > @@ -389,6 +389,12 @@ static int sk_diag_fill(struct sock *sk, struct
> > > sk_buff *skb,
> > > nlmsg_flags, unlh, net_admin);
> > > }
> > >
> > > +/*
> > > + * Ignore the position of the arguments req->id.idiag_dport and
> > > + * req->id.idiag_sport in both calls to inet_lookup() and inet6_lookup()
> > > + * functions, once this is a locked in behavior exposed to user space.
> > > + * Changing this will break things for people.
> > > + */
> > > struct sock *inet_diag_find_one_icsk(struct net *net,
> > > struct inet_hashinfo *hashinfo,
> > > const struct inet_diag_req_v2 *req)
> > >
> >
> > Seems sensible. Thanks.
>
> Should I resend it in a full and proper format or it can taken from here?
If you want it applied, it should be resent as a full patch
with your sign-off.
^ permalink raw reply
* Re: [net-ipv4] question about arguments position
From: Gustavo A. R. Silva @ 2017-05-04 19:15 UTC (permalink / raw)
To: Joe Perches
Cc: David Miller, kuznet, jmorris, yoshfuji, kaber, netdev,
linux-kernel
In-Reply-To: <1493924538.22125.43.camel@perches.com>
Quoting Joe Perches <joe@perches.com>:
> On Thu, 2017-05-04 at 14:00 -0500, Gustavo A. R. Silva wrote:
>> Regarding the code comments, what about the following patch:
> []
>> diff --git a/net/ipv4/inet_diag.c b/net/ipv4/inet_diag.c
> []
>> @@ -389,6 +389,12 @@ static int sk_diag_fill(struct sock *sk, struct
>> sk_buff *skb,
>> nlmsg_flags, unlh, net_admin);
>> }
>>
>> +/*
>> + * Ignore the position of the arguments req->id.idiag_dport and
>> + * req->id.idiag_sport in both calls to inet_lookup() and inet6_lookup()
>> + * functions, once this is a locked in behavior exposed to user space.
>> + * Changing this will break things for people.
>> + */
>> struct sock *inet_diag_find_one_icsk(struct net *net,
>> struct inet_hashinfo *hashinfo,
>> const struct inet_diag_req_v2 *req)
>>
>
> Seems sensible. Thanks.
Should I resend it in a full and proper format or it can taken from here?
Thanks
--
Gustavo A. R. Silva
^ 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