Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next 2/3] net: phy: add __phy_speed_down and phy_resolve_min_speed
From: Heiner Kallweit @ 2019-08-12 21:20 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, David Miller; +Cc: netdev@vger.kernel.org
In-Reply-To: <0799ec1f-307c-25ab-0259-b8239e4e04ba@gmail.com>

__phy_speed_down provides most of the functionality for phy_speed_down.
It makes use of new helper phy_resolve_min_speed that is based on the
sorting of the settings[] array.
In certain cases it may be helpful to be able to exclude legacy half
duplex modes, therefore prepare phy_resolve_min_speed() for it.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/phy/phy-core.c | 29 +++++++++++++++++++++++++++++
 include/linux/phy.h        |  1 +
 2 files changed, 30 insertions(+)

diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c
index de085f255..d7331875e 100644
--- a/drivers/net/phy/phy-core.c
+++ b/drivers/net/phy/phy-core.c
@@ -316,6 +316,35 @@ void phy_resolve_aneg_linkmode(struct phy_device *phydev)
 }
 EXPORT_SYMBOL_GPL(phy_resolve_aneg_linkmode);
 
+static int phy_resolve_min_speed(struct phy_device *phydev, bool fdx_only)
+{
+	__ETHTOOL_DECLARE_LINK_MODE_MASK(common);
+	int i = ARRAY_SIZE(settings);
+
+	linkmode_and(common, phydev->lp_advertising, phydev->advertising);
+
+	while (i--) {
+		if (test_bit(settings[i].bit, common)) {
+			if (fdx_only && settings[i].duplex != DUPLEX_FULL)
+				continue;
+			return settings[i].speed;
+		}
+	}
+
+	return SPEED_UNKNOWN;
+}
+
+int __phy_speed_down(struct phy_device *phydev)
+{
+	int min_common_speed = phy_resolve_min_speed(phydev, true);
+
+	if (min_common_speed == SPEED_UNKNOWN)
+		return -EINVAL;
+
+	return __set_linkmode_max_speed(phydev, min_common_speed,
+					phydev->advertising);
+}
+
 static void mmd_phy_indirect(struct mii_bus *bus, int phy_addr, int devad,
 			     u16 regnum)
 {
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 781f4810c..4be6d3b47 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -665,6 +665,7 @@ size_t phy_speeds(unsigned int *speeds, size_t size,
 		  unsigned long *mask);
 void of_set_phy_supported(struct phy_device *phydev);
 void of_set_phy_eee_broken(struct phy_device *phydev);
+int __phy_speed_down(struct phy_device *phydev);
 
 /**
  * phy_is_started - Convenience function to check whether PHY is started
-- 
2.22.0



^ permalink raw reply related

* [PATCH net-next 1/3] net: phy: add __set_linkmode_max_speed
From: Heiner Kallweit @ 2019-08-12 21:19 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, David Miller; +Cc: netdev@vger.kernel.org
In-Reply-To: <0799ec1f-307c-25ab-0259-b8239e4e04ba@gmail.com>

We will need the functionality of __set_linkmode_max_speed also for
linkmode bitmaps other than phydev->supported. Therefore split it.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/phy/phy-core.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c
index 9ae3abb2d..de085f255 100644
--- a/drivers/net/phy/phy-core.c
+++ b/drivers/net/phy/phy-core.c
@@ -207,14 +207,15 @@ size_t phy_speeds(unsigned int *speeds, size_t size,
 	return count;
 }
 
-static int __set_phy_supported(struct phy_device *phydev, u32 max_speed)
+static int __set_linkmode_max_speed(struct phy_device *phydev, u32 max_speed,
+				    unsigned long *addr)
 {
 	const struct phy_setting *p;
 	int i;
 
 	for (i = 0, p = settings; i < ARRAY_SIZE(settings); i++, p++) {
 		if (p->speed > max_speed)
-			linkmode_clear_bit(p->bit, phydev->supported);
+			linkmode_clear_bit(p->bit, addr);
 		else
 			break;
 	}
@@ -222,6 +223,11 @@ static int __set_phy_supported(struct phy_device *phydev, u32 max_speed)
 	return 0;
 }
 
+static int __set_phy_supported(struct phy_device *phydev, u32 max_speed)
+{
+	return __set_linkmode_max_speed(phydev, max_speed, phydev->supported);
+}
+
 int phy_set_max_speed(struct phy_device *phydev, u32 max_speed)
 {
 	int err;
-- 
2.22.0



^ permalink raw reply related

* [PATCH net] netfilter: ebtables: Fix argument order to ADD_COUNTER
From: Todd Seidelmann @ 2019-08-12 21:20 UTC (permalink / raw)
  To: netdev

The ordering of arguments to the x_tables ADD_COUNTER macro
appears to be wrong in ebtables (cf. ip_tables.c, ip6_tables.c,
and arp_tables.c).

This causes data corruption in the ebtables userspace tools
because they get incorrect packet & byte counts from the kernel.
---
  net/bridge/netfilter/ebtables.c | 8 ++++----
  1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/bridge/netfilter/ebtables.c 
b/net/bridge/netfilter/ebtables.c
index c8177a8..4096d8a 100644
--- a/net/bridge/netfilter/ebtables.c
+++ b/net/bridge/netfilter/ebtables.c
@@ -221,7 +221,7 @@ unsigned int ebt_do_table(struct sk_buff *skb,
              return NF_DROP;
          }

-        ADD_COUNTER(*(counter_base + i), 1, skb->len);
+        ADD_COUNTER(*(counter_base + i), skb->len, 1);

          /* these should only watch: not modify, nor tell us
           * what to do with the packet
@@ -959,8 +959,8 @@ static void get_counters(const struct ebt_counter 
*oldcounters,
              continue;
          counter_base = COUNTER_BASE(oldcounters, nentries, cpu);
          for (i = 0; i < nentries; i++)
-            ADD_COUNTER(counters[i], counter_base[i].pcnt,
-                    counter_base[i].bcnt);
+            ADD_COUNTER(counters[i], counter_base[i].bcnt,
+                    counter_base[i].pcnt);
      }
  }

@@ -1280,7 +1280,7 @@ static int do_update_counters(struct net *net, 
const char *name,

      /* we add to the counters of the first cpu */
      for (i = 0; i < num_counters; i++)
-        ADD_COUNTER(t->private->counters[i], tmp[i].pcnt, tmp[i].bcnt);
+        ADD_COUNTER(t->private->counters[i], tmp[i].bcnt, tmp[i].pcnt);

      write_unlock_bh(&t->lock);
      ret = 0;
--
1.8.3.1


^ permalink raw reply related

* Re: [PATCH v2 bpf-next] mm: mmap: increase sockets maximum memory size pgoff for 32bits
From: Andrew Morton @ 2019-08-12 21:19 UTC (permalink / raw)
  To: Ivan Khoronzhuk
  Cc: bjorn.topel, linux-mm, xdp-newbies, netdev, bpf, linux-kernel,
	ast, magnus.karlsson
In-Reply-To: <20190812124326.32146-1-ivan.khoronzhuk@linaro.org>

On Mon, 12 Aug 2019 15:43:26 +0300 Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote:

> The AF_XDP sockets umem mapping interface uses XDP_UMEM_PGOFF_FILL_RING
> and XDP_UMEM_PGOFF_COMPLETION_RING offsets. The offsets seems like are
> established already and are part of configuration interface.
> 
> But for 32-bit systems, while AF_XDP socket configuration, the values
> are to large to pass maximum allowed file size verification.
> The offsets can be tuned ofc, but instead of changing existent
> interface - extend max allowed file size for sockets.


What are the implications of this?  That all code in the kernel which
handles mapped sockets needs to be audited (and tested) for correctly
handling mappings larger than 4G on 32-bit machines?  Has that been
done?  Are we confident that we aren't introducing user-visible buggy
behaviour into unsuspecting legacy code?

Also...  what are the user-visible runtime effects of this change? 
Please send along a paragraph which explains this, for the changelog. 
Does this patch fix some user-visible problem?  If so, should be code
be backported into -stable kernels?


^ permalink raw reply

* [PATCH net-next 0/3] net: phy: let phy_speed_down/up support speeds >1Gbps
From: Heiner Kallweit @ 2019-08-12 21:18 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, David Miller; +Cc: netdev@vger.kernel.org

So far phy_speed_down/up can be used up to 1Gbps only. Remove this
restriction and add needed helpers to phy-core.c

Heiner Kallweit (3):
  net: phy: add __set_linkmode_max_speed
  net: phy: add __phy_speed_down and phy_resolve_min_speed
  net: phy: let phy_speed_down/up support speeds >1Gbps

 drivers/net/phy/phy-core.c | 39 +++++++++++++++++++++++--
 drivers/net/phy/phy.c      | 60 ++++++++++----------------------------
 include/linux/phy.h        |  3 ++
 3 files changed, 56 insertions(+), 46 deletions(-)

-- 
2.22.0


^ permalink raw reply

* Re: [PATCH v2] Documentation: virt: Fix broken reference to virt tree's index
From: Jonathan Corbet @ 2019-08-12 21:16 UTC (permalink / raw)
  To: Sheriff Esseson
  Cc: skhan, linux-kernel-mentees, Paul Walmsley, Palmer Dabbelt,
	Albert Ou, Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau,
	Song Liu, Yonghong Song, open list:DOCUMENTATION, open list,
	open list:RISC-V ARCHITECTURE,
	open list:BPF (Safe dynamic programs and tools),
	open list:BPF (Safe dynamic programs and tools)
In-Reply-To: <20190809132349.GA15460@localhost>

On Fri, 9 Aug 2019 14:23:49 +0100
Sheriff Esseson <sheriffesseson@gmail.com> wrote:

> Fix broken reference to virt/index.rst.
> 
> Fixes: 2f5947dfcaec ("Documentation: move Documentation/virtual to
> Documentation/virt")
> 
> Signed-off-by: Sheriff Esseson <sheriffesseson@gmail.com>

Note that you should keep the "Fixes:" tag on a single line, and not put a
blank line between it and the other tags.  I've fixed that up and applied
the patch, thanks.

jon

^ permalink raw reply

* [PATCH net v2] ibmveth: Convert multicast list size for little-endian system
From: Thomas Falcon @ 2019-08-12 21:13 UTC (permalink / raw)
  To: netdev; +Cc: liuhangbin, davem, joe, Thomas Falcon

The ibm,mac-address-filters property defines the maximum number of
addresses the hypervisor's multicast filter list can support. It is
encoded as a big-endian integer in the OF device tree, but the virtual
ethernet driver does not convert it for use by little-endian systems.
As a result, the driver is not behaving as it should on affected systems
when a large number of multicast addresses are assigned to the device.

Reported-by: Hangbin Liu <liuhangbin@gmail.com>
Signed-off-by: Thomas Falcon <tlfalcon@linux.ibm.com>
---
v2: define filter size pointer as __be32 instead of unsigned int,
    suggested by Joe Perches
---
 drivers/net/ethernet/ibm/ibmveth.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmveth.c b/drivers/net/ethernet/ibm/ibmveth.c
index 77af9c2c0571..5641c00d34f2 100644
--- a/drivers/net/ethernet/ibm/ibmveth.c
+++ b/drivers/net/ethernet/ibm/ibmveth.c
@@ -1643,7 +1643,7 @@ static int ibmveth_probe(struct vio_dev *dev, const struct vio_device_id *id)
 	struct net_device *netdev;
 	struct ibmveth_adapter *adapter;
 	unsigned char *mac_addr_p;
-	unsigned int *mcastFilterSize_p;
+	__be32 *mcastFilterSize_p;
 	long ret;
 	unsigned long ret_attr;
 
@@ -1665,8 +1665,9 @@ static int ibmveth_probe(struct vio_dev *dev, const struct vio_device_id *id)
 		return -EINVAL;
 	}
 
-	mcastFilterSize_p = (unsigned int *)vio_get_attribute(dev,
-						VETH_MCAST_FILTER_SIZE, NULL);
+	mcastFilterSize_p = (__be32 *)vio_get_attribute(dev,
+							VETH_MCAST_FILTER_SIZE,
+							NULL);
 	if (!mcastFilterSize_p) {
 		dev_err(&dev->dev, "Can't find VETH_MCAST_FILTER_SIZE "
 			"attribute\n");
@@ -1683,7 +1684,7 @@ static int ibmveth_probe(struct vio_dev *dev, const struct vio_device_id *id)
 
 	adapter->vdev = dev;
 	adapter->netdev = netdev;
-	adapter->mcastFilterSize = *mcastFilterSize_p;
+	adapter->mcastFilterSize = be32_to_cpu(*mcastFilterSize_p);
 	adapter->pool_config = 0;
 	ibmveth_init_link_settings(adapter);
 
-- 
2.16.4


^ permalink raw reply related

* Re: [PATCH net-next v2 04/12] net: stmmac: Add Split Header support and enable it in XGMAC cores
From: David Miller @ 2019-08-12 21:06 UTC (permalink / raw)
  To: Jose.Abreu
  Cc: netdev, Joao.Pinto, peppe.cavallaro, alexandre.torgue,
	mcoquelin.stm32, linux-stm32, linux-arm-kernel, linux-kernel
In-Reply-To: <6279e35421e17256ac023227ec8cd5c8498d34d0.1565602974.git.joabreu@synopsys.com>

From: Jose Abreu <Jose.Abreu@synopsys.com>
Date: Mon, 12 Aug 2019 11:44:03 +0200

> 	- Add performance info (David)

Ummm...

Whilst cpu utilization is interesting, I might be mainly interested in
how this effects "networking" performance.  I find it very surprising
that it isn't obvious that this is what I wanted.

Do you not do performance testing on the networking level when you
make fundamental changes to how packets are processed by the
hardware/driver?

^ permalink raw reply

* Re: [PATCH v2] net: phy: at803x: stop switching phy delay config needlessly
From: David Miller @ 2019-08-12 21:02 UTC (permalink / raw)
  To: git; +Cc: linux-kernel, andrew, f.fainelli, hkallweit1, netdev
In-Reply-To: <20190809112025.27482-1-git@andred.net>

From: André Draszik <git@andred.net>
Date: Fri,  9 Aug 2019 12:20:25 +0100

> This driver does a funny dance disabling and re-enabling
> RX and/or TX delays. In any of the RGMII-ID modes, it first
> disables the delays, just to re-enable them again right
> away. This looks like a needless exercise.
> 
> Just enable the respective delays when in any of the
> relevant 'id' modes, and disable them otherwise.
> 
> Also, remove comments which don't add anything that can't be
> seen by looking at the code.
> 
> Signed-off-by: André Draszik <git@andred.net>

Applied.

^ permalink raw reply

* Re: [PATCH 0/7] Add definition for the number of standard PCI BARs
From: Denis Efremov @ 2019-08-12 21:00 UTC (permalink / raw)
  To: Andrew Murray
  Cc: Bjorn Helgaas, Sebastian Ott, Gerald Schaefer, H. Peter Anvin,
	Giuseppe Cavallaro, Alexandre Torgue, Matt Porter,
	Alexandre Bounine, Peter Jones, Bartlomiej Zolnierkiewicz,
	Cornelia Huck, Alex Williamson, kvm, linux-fbdev, netdev, x86,
	linux-s390, linux-pci, linux-kernel
In-Reply-To: <20190812090639.GX56241@e119886-lin.cambridge.arm.com>

On 12.08.2019 12:06, Andrew Murray wrote:
> 
> Hi Denis,

Hi!

> 
> You could also fix up a few cases where the number of BARs is hard coded in
> loops, e.g.
> 
> drivers/pci/controller/pci-hyperv.c - look for uses of probed_bar in loops
> drivers/pci/pci.c - pci_release_selected_regions and __pci_request_selected_regions
> drivers/pci/quirks.c - quirk_alder_ioapic
> 

Thanks for pointing me on that. I will take this into account in v2.

Denis

^ permalink raw reply

* Re: [PATCH] vhost: do not reference a file that does not exist
From: Enrico Granata @ 2019-08-12 20:55 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Enrico Granata, Linux Kernel Mailing List, mst, jasowang, kvm,
	virtualization, netdev, trivial
In-Reply-To: <20190810081540.GA30426@infradead.org>

Fair enough, yeah.

I think what I found confusing was that the file had a precise
(directly actionable in a file browser, if you will) path. If it was
just listed as a filename, or a project name, it might have been more
obvious that one shouldn't expect to find it within the kernel tree
and just go look it up in your favorite search engine.

The right incantation to get your hands on that file is a web search,
not a local file navigation, and to my perception a full and seemingly
valid path pointed in the direction of doing the wrong thing.

It's not a huge deal, obviously, and it may be that I was the only one
confused by that. If so, feel free to disregard the patch.

Thanks,
- Enrico

Thanks,
- Enrico


On Sat, Aug 10, 2019 at 1:15 AM Christoph Hellwig <hch@infradead.org> wrote:
>
> On Wed, Aug 07, 2019 at 05:52:55PM -0700, egranata@chromium.org wrote:
> > From: Enrico Granata <egranata@google.com>
> >
> > lguest was removed from the mainline kernel in late 2017.
> >
> > Signed-off-by: Enrico Granata <egranata@google.com>
>
> But this particular file even has an override in the script looking
> for dead references, which together with the content of the overal
> contents makes me thing the dangling reference is somewhat intentional.

^ permalink raw reply

* Re: Error when loading BPF_CGROUP_INET_EGRESS program with bpftool
From: Fejes Ferenc @ 2019-08-12 20:48 UTC (permalink / raw)
  To: Andrii Nakryiko; +Cc: netdev@vger.kernel.org
In-Reply-To: <CAEf4BzZ27SnYkQ=psqxeWadLhnspojiJGQrGB0JRuPkP+GTiNQ@mail.gmail.com>

Thanks for the answer, I really appreciate it. I tried omitting
"cgroup/skb" to let libbpf guess the attach type, but I got the same
error. Really interesting, because I got the error
> libbpf: failed to load program 'cgroup_skb/egress'
wich is weird because it shows that libbpf guess the program type
correctly. So something definitely on my side - thank you for verifyng
that - I try to investigate it!

Ferenc
Andrii Nakryiko <andrii.nakryiko@gmail.com> ezt írta (időpont: 2019.
aug. 12., H, 20:27):
>
> On Mon, Aug 12, 2019 at 1:59 AM Fejes Ferenc <fejes@inf.elte.hu> wrote:
> >
> > Greetings!
> >
> > I found a strange error when I tried to load a BPF_CGROUP_INET_EGRESS
> > prog with bpftool. Loading the same program from C code with
> > bpf_prog_load_xattr works without problem.
> >
> > The error message I got:
> > bpftool prog loadall hbm_kern.o /sys/fs/bpf/hbm type cgroup/skb
>
> You need "cgroup_skb/egress" instead of "cgroup/skb" (or try just
> dropping it, bpftool will try to guess the type from program's section
> name, which would be correct in this case).
>
> > libbpf: load bpf program failed: Invalid argument
> > libbpf: -- BEGIN DUMP LOG ---
> > libbpf:
> > ; return ALLOW_PKT | REDUCE_CW;
> > 0: (b7) r0 = 3
> > 1: (95) exit
> > At program exit the register R0 has value (0x3; 0x0) should have been
> > in (0x0; 0x1)
> > processed 2 insns (limit 1000000) max_states_per_insn 0 total_states 0
> > peak_states 0 mark_read 0
> >
> > libbpf: -- END LOG --
> > libbpf: failed to load program 'cgroup_skb/egress'
> > libbpf: failed to load object 'hbm_kern.o'
> > Error: failed to load object file
> >
> >
> > My environment: 5.3-rc3 / net-next master (both producing the error).
> > Libbpf and bpftool installed from the kernel source (cleaned and
> > reinstalled when I tried a new kernel). I compiled the program with
> > Clang 8, on Ubuntu 19.10 server image, the source:
> >
> > #include <linux/bpf.h>
> > #include "bpf_helpers.h"
> >
> > #define DROP_PKT        0
> > #define ALLOW_PKT       1
> > #define REDUCE_CW       2
> >
> > SEC("cgroup_skb/egress")
> > int hbm(struct __sk_buff *skb)
> > {
> >         return ALLOW_PKT | REDUCE_CW;
> > }
> > char _license[] SEC("license") = "GPL";
> >
> >
> > I also tried to trace down the bug with gdb. It seems like the
> > section_names array in libbpf.c filled with garbage, especially the
>
> I did the same, section_names appears to be correct, not sure what was
> going on in your case. The problem is that "cgroup/skb", which you
> provided on command line, overrides this section name and forces
> bpftool to guess program type as BPF_CGROUP_INET_INGRESS, which
> restricts return codes to just 0 or 1, while for
> BPF_CGROUP_INET_EGRESS i is [0, 3].
>
> > expected_attach_type fields (in my case, this contains
> > BPF_CGROUP_INET_INGRESS instead of BPF_CGROUP_INET_EGRESS).
> >
> > Thanks!

^ permalink raw reply

* Re: [PATCH net] net: phy: consider AN_RESTART status when reading link status
From: Andrew Lunn @ 2019-08-12 20:24 UTC (permalink / raw)
  To: Heiner Kallweit
  Cc: Florian Fainelli, David Miller, netdev@vger.kernel.org,
	Yonglong Liu
In-Reply-To: <46efcf9f-0938-e017-706c-fb5a400f6fbb@gmail.com>

On Mon, Aug 12, 2019 at 09:20:02PM +0200, Heiner Kallweit wrote:
> After configuring and restarting aneg we immediately try to read the
> link status. On some systems the PHY may not yet have cleared the
> "aneg complete" and "link up" bits, resulting in a false link-up
> signal. See [0] for a report.
> Clause 22 and 45 both require the PHY to keep the AN_RESTART
> bit set until the PHY actually starts auto-negotiation.
> Let's consider this in the generic functions for reading link status.
> The commit marked as fixed is the first one where the patch applies
> cleanly.
> 
> [0] https://marc.info/?t=156518400300003&r=1&w=2
> 
> Fixes: c1164bb1a631 ("net: phy: check PMAPMD link status only in genphy_c45_read_link")
> Tested-by: Yonglong Liu <liuyonglong@huawei.com>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

^ permalink raw reply

* Re: 5.3-rc3-ish VM crash: RIP: 0010:tcp_trim_head+0x20/0xe0
From: Sander Eikelenboom @ 2019-08-12 20:17 UTC (permalink / raw)
  To: Eric Dumazet, netdev, linux-kernel
In-Reply-To: <4d803565-b716-42ab-1db8-3dcade91e939@gmail.com>

On 12/08/2019 19:56, Eric Dumazet wrote:
> 
> 
> On 8/12/19 2:50 PM, Sander Eikelenboom wrote:
>> L.S.,
>>
>> While testing a somewhere-after-5.3-rc3 kernel (which included the latest net merge (33920f1ec5bf47c5c0a1d2113989bdd9dfb3fae9),
>> one of my Xen VM's (which gets quite some network load) crashed.
>> See below for the stacktrace.
>>
>> Unfortunately I haven't got a clear trigger, so bisection doesn't seem to be an option at the moment. 
>> I haven't encountered this on 5.2, so it seems to be an regression against 5.2.
>>
>> Any ideas ?
>>
>> --
>> Sander
>>
>>
>> [16930.653595] general protection fault: 0000 [#1] SMP NOPTI
>> [16930.653624] CPU: 0 PID: 3275 Comm: rsync Not tainted 5.3.0-rc3-20190809-doflr+ #1
>> [16930.653657] RIP: 0010:tcp_trim_head+0x20/0xe0
>> [16930.653677] Code: 2e 0f 1f 84 00 00 00 00 00 90 41 54 41 89 d4 55 48 89 fd 53 48 89 f3 f6 46 7e 01 74 2f 8b 86 bc 00 00 00 48 03 86 c0 00 00 00 <8b> 40 20 66 83 f8 01 74 19 31 d2 31 f6 b9 20 0a 00 00 48 89 df e8
>> [16930.653741] RSP: 0000:ffffc90000003ad8 EFLAGS: 00010286
>> [16930.653762] RAX: fffe888005bf62c0 RBX: ffff8880115fb800 RCX: 000000008010000b
> 
> crash in " mov    0x20(%rax),%eax"   and RAX=fffe888005bf62c0 (not a valid kernel address)
> 
> Look like one bit corruption maybe.
> 
> Nothing comes to mind really between 5.2 and 53 that could explain this.

Hi Eric,

Hmm could be it's a rare coincidence, sp that it just never occurred on pre 5.3 by chance.
Let's wait and see if it reoccurs, will report back if it does.

Thanks for your explanation.

--
Sander


>> [16930.653791] RDX: 00000000000005a0 RSI: ffff8880115fb800 RDI: ffff888016b00880
>> [16930.653819] RBP: ffff888016b00880 R08: 0000000000000001 R09: 0000000000000000
>> [16930.653848] R10: ffff88800ae00800 R11: 00000000bfe632e6 R12: 00000000000005a0
>> [16930.653875] R13: 0000000000000001 R14: 00000000bfe62d46 R15: 0000000000000004
>> [16930.653913] FS:  00007fe71fe2cb80(0000) GS:ffff88801f200000(0000) knlGS:0000000000000000
>> [16930.653943] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> [16930.653965] CR2: 000055de0f3e7000 CR3: 0000000011f32000 CR4: 00000000000006f0
>> [16930.653993] Call Trace:
>> [16930.654005]  <IRQ>
>> [16930.654018]  tcp_ack+0xbb0/0x1230
>> [16930.654033]  tcp_rcv_established+0x2e8/0x630
>> [16930.654053]  tcp_v4_do_rcv+0x129/0x1d0
>> [16930.654070]  tcp_v4_rcv+0xac9/0xcb0
>> [16930.654088]  ip_protocol_deliver_rcu+0x27/0x1b0
>> [16930.654109]  ip_local_deliver_finish+0x3f/0x50
>> [16930.654128]  ip_local_deliver+0x4d/0xe0
>> [16930.654145]  ? ip_protocol_deliver_rcu+0x1b0/0x1b0
>> [16930.654163]  ip_rcv+0x4c/0xd0
>> [16930.654179]  __netif_receive_skb_one_core+0x79/0x90
>> [16930.654200]  netif_receive_skb_internal+0x2a/0xa0
>> [16930.654219]  napi_gro_receive+0xe7/0x140
>> [16930.654237]  xennet_poll+0x9be/0xae0
>> [16930.654254]  net_rx_action+0x136/0x340
>> [16930.654271]  __do_softirq+0xdd/0x2cf
>> [16930.654287]  irq_exit+0x7a/0xa0
>> [16930.654304]  xen_evtchn_do_upcall+0x27/0x40
>> [16930.654320]  xen_hvm_callback_vector+0xf/0x20
>> [16930.654339]  </IRQ>
>> [16930.654349] RIP: 0033:0x55de0d87db99
>> [16930.654364] Code: 00 00 48 89 7c 24 f8 45 39 fe 45 0f 42 fe 44 89 7c 24 f4 eb 09 0f 1f 40 00 83 e9 01 74 3e 89 f2 48 63 f8 4c 01 d2 44 38 1c 3a <75> 25 44 38 6c 3a ff 75 1e 41 0f b6 3c 24 40 38 3a 75 14 41 0f b6
>> [16930.654432] RSP: 002b:00007ffd5531eec8 EFLAGS: 00000a87 ORIG_RAX: ffffffffffffff0c
>> [16930.655004] RAX: 0000000000000002 RBX: 000055de0f3e8e50 RCX: 000000000000007f
>> [16930.655034] RDX: 000055de0f3dc2d2 RSI: 0000000000003492 RDI: 0000000000000002
>> [16930.655062] RBP: 0000000000007fff R08: 00000000000080ea R09: 00000000000001f0
>> [16930.655089] R10: 000055de0f3d8e40 R11: 0000000000000094 R12: 000055de0f3e0f2a
>> [16930.655116] R13: 0000000000000010 R14: 0000000000007f16 R15: 0000000000000080
>> [16930.655144] Modules linked in:
>> [16930.655200] ---[ end trace 533367c95501b645 ]---
>> [16930.655223] RIP: 0010:tcp_trim_head+0x20/0xe0
>> [16930.655243] Code: 2e 0f 1f 84 00 00 00 00 00 90 41 54 41 89 d4 55 48 89 fd 53 48 89 f3 f6 46 7e 01 74 2f 8b 86 bc 00 00 00 48 03 86 c0 00 00 00 <8b> 40 20 66 83 f8 01 74 19 31 d2 31 f6 b9 20 0a 00 00 48 89 df e8
>> [16930.655312] RSP: 0000:ffffc90000003ad8 EFLAGS: 00010286
>> [16930.655331] RAX: fffe888005bf62c0 RBX: ffff8880115fb800 RCX: 000000008010000b
>> [16930.655360] RDX: 00000000000005a0 RSI: ffff8880115fb800 RDI: ffff888016b00880
>> [16930.655387] RBP: ffff888016b00880 R08: 0000000000000001 R09: 0000000000000000
>> [16930.655414] R10: ffff88800ae00800 R11: 00000000bfe632e6 R12: 00000000000005a0
>> [16930.655441] R13: 0000000000000001 R14: 00000000bfe62d46 R15: 0000000000000004
>> [16930.655475] FS:  00007fe71fe2cb80(0000) GS:ffff88801f200000(0000) knlGS:0000000000000000
>> [16930.655502] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> [16930.655525] CR2: 000055de0f3e7000 CR3: 0000000011f32000 CR4: 00000000000006f0
>> [16930.655553] Kernel panic - not syncing: Fatal exception in interrupt
>> [16930.655789] Kernel Offset: disabled
>>


^ permalink raw reply

* Re: [PATCH 0/7] Add definition for the number of standard PCI BARs
From: Thomas Gleixner @ 2019-08-12 20:11 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Denis Efremov, Sebastian Ott, Gerald Schaefer, H. Peter Anvin,
	Giuseppe Cavallaro, Alexandre Torgue, Matt Porter,
	Alexandre Bounine, Peter Jones, Bartlomiej Zolnierkiewicz,
	Cornelia Huck, Alex Williamson, kvm, linux-fbdev, netdev, x86,
	linux-s390, linux-pci, linux-kernel
In-Reply-To: <20190812200134.GB11785@google.com>

On Mon, 12 Aug 2019, Bjorn Helgaas wrote:

> On Sun, Aug 11, 2019 at 06:07:55PM +0300, Denis Efremov wrote:
> > Code that iterates over all standard PCI BARs typically uses
> > PCI_STD_RESOURCE_END, but this is error-prone because it requires
> > "i <= PCI_STD_RESOURCE_END" rather than something like
> > "i < PCI_STD_NUM_BARS". We could add such a definition and use it the same
> > way PCI_SRIOV_NUM_BARS is used. There is already the definition
> > PCI_BAR_COUNT for s390 only. Thus, this patchset introduces it globally.
> > 
> > The patch is splitted into 7 parts for different drivers/subsystems for
> > easy readability.
> 
> This looks good.  I can take all these together, since they all depend
> on the first patch.  I have a few comments on the individual patches.
> 
> > Denis Efremov (7):
> >   PCI: Add define for the number of standard PCI BARs
> >   s390/pci: Replace PCI_BAR_COUNT with PCI_STD_NUM_BARS
> >   x86/PCI: Use PCI_STD_NUM_BARS in loops instead of PCI_STD_RESOURCE_END

Fine with me for the x86 part. That's your turf anyway :)

Thanks,

	tglx

^ permalink raw reply

* [PATCH net] netlink: Fix nlmsg_parse as a wrapper for strict message parsing
From: David Ahern @ 2019-08-12 20:07 UTC (permalink / raw)
  To: davem; +Cc: netdev, johannes.berg, edumazet, David Ahern

From: David Ahern <dsahern@gmail.com>

Eric reported a syzbot warning:

BUG: KMSAN: uninit-value in nh_valid_get_del_req+0x6f1/0x8c0 net/ipv4/nexthop.c:1510
CPU: 0 PID: 11812 Comm: syz-executor444 Not tainted 5.3.0-rc3+ #17
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Call Trace:
 __dump_stack lib/dump_stack.c:77 [inline]
 dump_stack+0x191/0x1f0 lib/dump_stack.c:113
 kmsan_report+0x162/0x2d0 mm/kmsan/kmsan_report.c:109
 __msan_warning+0x75/0xe0 mm/kmsan/kmsan_instr.c:294
 nh_valid_get_del_req+0x6f1/0x8c0 net/ipv4/nexthop.c:1510
 rtm_del_nexthop+0x1b1/0x610 net/ipv4/nexthop.c:1543
 rtnetlink_rcv_msg+0x115a/0x1580 net/core/rtnetlink.c:5223
 netlink_rcv_skb+0x431/0x620 net/netlink/af_netlink.c:2477
 rtnetlink_rcv+0x50/0x60 net/core/rtnetlink.c:5241
 netlink_unicast_kernel net/netlink/af_netlink.c:1302 [inline]
 netlink_unicast+0xf6c/0x1050 net/netlink/af_netlink.c:1328
 netlink_sendmsg+0x110f/0x1330 net/netlink/af_netlink.c:1917
 sock_sendmsg_nosec net/socket.c:637 [inline]
 sock_sendmsg net/socket.c:657 [inline]
 ___sys_sendmsg+0x14ff/0x1590 net/socket.c:2311
 __sys_sendmmsg+0x53a/0xae0 net/socket.c:2413
 __do_sys_sendmmsg net/socket.c:2442 [inline]
 __se_sys_sendmmsg+0xbd/0xe0 net/socket.c:2439
 __x64_sys_sendmmsg+0x56/0x70 net/socket.c:2439
 do_syscall_64+0xbc/0xf0 arch/x86/entry/common.c:297
 entry_SYSCALL_64_after_hwframe+0x63/0xe7

The root cause is nlmsg_parse calling __nla_parse which means the
header struct size is not checked.

nlmsg_parse should be a wrapper around __nlmsg_parse with
NL_VALIDATE_STRICT for the validate argument very much like
nlmsg_parse_deprecated is for NL_VALIDATE_LIBERAL.

Fixes: 3de6440354465 ("netlink: re-add parse/validate functions in strict mode")
Reported-by: Eric Dumazet <edumazet@google.com>
Reported-by: syzbot <syzkaller@googlegroups.com>
Signed-off-by: David Ahern <dsahern@gmail.com>
---
 include/net/netlink.h | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/include/net/netlink.h b/include/net/netlink.h
index e4650e5b64a1..b140c8f1be22 100644
--- a/include/net/netlink.h
+++ b/include/net/netlink.h
@@ -684,9 +684,8 @@ static inline int nlmsg_parse(const struct nlmsghdr *nlh, int hdrlen,
 			      const struct nla_policy *policy,
 			      struct netlink_ext_ack *extack)
 {
-	return __nla_parse(tb, maxtype, nlmsg_attrdata(nlh, hdrlen),
-			   nlmsg_attrlen(nlh, hdrlen), policy,
-			   NL_VALIDATE_STRICT, extack);
+	return __nlmsg_parse(nlh, hdrlen, tb, maxtype, policy,
+			     NL_VALIDATE_STRICT, extack);
 }
 
 /**
-- 
2.11.0


^ permalink raw reply related

* Re: [PATCH 4/7] PCI/net: Use PCI_STD_NUM_BARS in loops instead of PCI_STD_RESOURCE_END
From: Bjorn Helgaas @ 2019-08-12 20:02 UTC (permalink / raw)
  To: Denis Efremov
  Cc: Giuseppe Cavallaro, Alexandre Torgue, netdev, linux-pci,
	linux-kernel
In-Reply-To: <20190811150802.2418-5-efremov@linux.com>

The subject can be simply:

  <prefix>: Loop using PCI_STD_NUM_BARS

to keep them a little shorter so "git log --online" doesn't wrap.

On Sun, Aug 11, 2019 at 06:08:00PM +0300, Denis Efremov wrote:
> This patch refactors the loop condition scheme from
> 'i <= PCI_STD_RESOURCE_END' to 'i < PCI_STD_NUM_BARS'.

  Refactor loops to use 'i < PCI_STD_NUM_BARS' instead of 'i <=
  PCI_STD_RESOURCE_END'.

See https://chris.beams.io/posts/git-commit/

> Signed-off-by: Denis Efremov <efremov@linux.com>
> ---
>  drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 4 ++--
>  drivers/net/ethernet/synopsys/dwc-xlgmac-pci.c   | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)

This patch touches two unrelated drivers and should be split up.
When you do that, pay attention to the convention for commit log
prefixes, e.g.,

  $ git log --oneline drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
  37e9c087c814 stmmac: pci: Fix typo in IOT2000 comment
  d4a62ea411f9 stmmac: pci: Use pci_dev_id() helper
  e0c1d14a1a32 stmmac: pci: Adjust IOT2000 matching

  $ git log --oneline drivers/net/ethernet/synopsys/dwc-xlgmac-pci.c
  ea8c1c642ea5 net: dwc-xlgmac: declaration of dual license in headers
  65e0ace2c5cd net: dwc-xlgmac: Initial driver for DesignWare Enterprise Ethernet

> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
> index 86f9c07a38cf..cfe496cdd78b 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
> @@ -258,7 +258,7 @@ static int stmmac_pci_probe(struct pci_dev *pdev,
>  	}
>  
>  	/* Get the base address of device */
> -	for (i = 0; i <= PCI_STD_RESOURCE_END; i++) {
> +	for (i = 0; i < PCI_STD_NUM_BARS; i++) {
>  		if (pci_resource_len(pdev, i) == 0)
>  			continue;
>  		ret = pcim_iomap_regions(pdev, BIT(i), pci_name(pdev));
> @@ -296,7 +296,7 @@ static void stmmac_pci_remove(struct pci_dev *pdev)
>  
>  	stmmac_dvr_remove(&pdev->dev);
>  
> -	for (i = 0; i <= PCI_STD_RESOURCE_END; i++) {
> +	for (i = 0; i < PCI_STD_NUM_BARS; i++) {
>  		if (pci_resource_len(pdev, i) == 0)
>  			continue;
>  		pcim_iounmap_regions(pdev, BIT(i));
> diff --git a/drivers/net/ethernet/synopsys/dwc-xlgmac-pci.c b/drivers/net/ethernet/synopsys/dwc-xlgmac-pci.c
> index 386bafe74c3f..fa8604d7b797 100644
> --- a/drivers/net/ethernet/synopsys/dwc-xlgmac-pci.c
> +++ b/drivers/net/ethernet/synopsys/dwc-xlgmac-pci.c
> @@ -34,7 +34,7 @@ static int xlgmac_probe(struct pci_dev *pcidev, const struct pci_device_id *id)
>  		return ret;
>  	}
>  
> -	for (i = 0; i <= PCI_STD_RESOURCE_END; i++) {
> +	for (i = 0; i < PCI_STD_NUM_BARS; i++) {
>  		if (pci_resource_len(pcidev, i) == 0)
>  			continue;
>  		ret = pcim_iomap_regions(pcidev, BIT(i), XLGMAC_DRV_NAME);
> -- 
> 2.21.0
> 

^ permalink raw reply

* Re: [PATCH 0/7] Add definition for the number of standard PCI BARs
From: Bjorn Helgaas @ 2019-08-12 20:01 UTC (permalink / raw)
  To: Denis Efremov
  Cc: Sebastian Ott, Gerald Schaefer, H. Peter Anvin,
	Giuseppe Cavallaro, Alexandre Torgue, Matt Porter,
	Alexandre Bounine, Peter Jones, Bartlomiej Zolnierkiewicz,
	Cornelia Huck, Alex Williamson, kvm, linux-fbdev, netdev, x86,
	linux-s390, linux-pci, linux-kernel
In-Reply-To: <20190811150802.2418-1-efremov@linux.com>

On Sun, Aug 11, 2019 at 06:07:55PM +0300, Denis Efremov wrote:
> Code that iterates over all standard PCI BARs typically uses
> PCI_STD_RESOURCE_END, but this is error-prone because it requires
> "i <= PCI_STD_RESOURCE_END" rather than something like
> "i < PCI_STD_NUM_BARS". We could add such a definition and use it the same
> way PCI_SRIOV_NUM_BARS is used. There is already the definition
> PCI_BAR_COUNT for s390 only. Thus, this patchset introduces it globally.
> 
> The patch is splitted into 7 parts for different drivers/subsystems for
> easy readability.

This looks good.  I can take all these together, since they all depend
on the first patch.  I have a few comments on the individual patches.

> Denis Efremov (7):
>   PCI: Add define for the number of standard PCI BARs
>   s390/pci: Replace PCI_BAR_COUNT with PCI_STD_NUM_BARS
>   x86/PCI: Use PCI_STD_NUM_BARS in loops instead of PCI_STD_RESOURCE_END
>   PCI/net: Use PCI_STD_NUM_BARS in loops instead of PCI_STD_RESOURCE_END
>   rapidio/tsi721: use PCI_STD_NUM_BARS in loops instead of
>     PCI_STD_RESOURCE_END
>   efifb: Use PCI_STD_NUM_BARS in loops instead of PCI_STD_RESOURCE_END
>   vfio_pci: Use PCI_STD_NUM_BARS in loops instead of
>     PCI_STD_RESOURCE_END
> 
>  arch/s390/include/asm/pci.h                      |  5 +----
>  arch/s390/include/asm/pci_clp.h                  |  6 +++---
>  arch/s390/pci/pci.c                              | 16 ++++++++--------
>  arch/s390/pci/pci_clp.c                          |  6 +++---
>  arch/x86/pci/common.c                            |  2 +-
>  drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c |  4 ++--
>  drivers/net/ethernet/synopsys/dwc-xlgmac-pci.c   |  2 +-
>  drivers/pci/quirks.c                             |  2 +-
>  drivers/rapidio/devices/tsi721.c                 |  2 +-
>  drivers/vfio/pci/vfio_pci.c                      |  4 ++--
>  drivers/vfio/pci/vfio_pci_config.c               |  2 +-
>  drivers/vfio/pci/vfio_pci_private.h              |  4 ++--
>  drivers/video/fbdev/efifb.c                      |  2 +-
>  include/linux/pci.h                              |  2 +-
>  include/uapi/linux/pci_regs.h                    |  1 +
>  15 files changed, 29 insertions(+), 31 deletions(-)
> 
> -- 
> 2.21.0
> 

^ permalink raw reply

* Re: [PATCH v4 7/9] mfd: ioc3: Add driver for SGI IOC3 chip
From: Jakub Kicinski @ 2019-08-12 19:52 UTC (permalink / raw)
  To: Thomas Bogendoerfer
  Cc: Ralf Baechle, Paul Burton, James Hogan, Dmitry Torokhov,
	Lee Jones, David S. Miller, Srinivas Kandagatla, Alessandro Zummo,
	Alexandre Belloni, Greg Kroah-Hartman, Jiri Slaby,
	Evgeniy Polyakov, linux-mips, linux-kernel, linux-input, netdev,
	linux-rtc, linux-serial
In-Reply-To: <20190811093212.88635fb1a6c796a073ec71ff@suse.de>

On Sun, 11 Aug 2019 09:32:12 +0200, Thomas Bogendoerfer wrote:
> > Also please don't use stdint types in the kernel, please try checkpatch
> > to catch coding style issues.  
> 
> my patch already reduces them and checkpatch only warns about usage of printk
> for the network part. Changing that to dev_warn/dev_err in the mfd patch didn't
> seem the right thing to do. As I'm splitting the conversion patch into a few
> steps I could also replace the printks.

Thanks for looking into it. I was referring to the use of uint32_t
instead of u32. Perhaps checkpatch has to be motivated with the --strict
option to point those out?

^ permalink raw reply

* Re: [PATCH v3 13/17] mvpp2: no need to check return value of debugfs_create functions
From: Greg Kroah-Hartman @ 2019-08-12 19:51 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: netdev, David S. Miller, Maxime Chevallier, Nathan Huckleberry
In-Reply-To: <CAKwvOdkWzr5fu3v0KR2XXj0dqCZki=JOoMft9SMjs+XmZ8HpUg@mail.gmail.com>

On Mon, Aug 12, 2019 at 12:44:36PM -0700, Nick Desaulniers wrote:
> On Mon, Aug 12, 2019 at 12:01 PM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Mon, Aug 12, 2019 at 10:55:51AM -0700, Nick Desaulniers wrote:
> > > On Sat, Aug 10, 2019 at 3:17 AM Greg Kroah-Hartman
> > > <gregkh@linuxfoundation.org> wrote:
> > > >
> > > > When calling debugfs functions, there is no need to ever check the
> > > > return value.  The function can work or not, but the code logic should
> > > > never do something different based on this.
> > >
> > > Maybe adding this recommendation to the comment block above the
> > > definition of debugfs_create_dir() in fs/debugfs/inode.c would help
> > > prevent this issue in the future?  What failure means, and how to
> > > proceed can be tricky; more documentation can only help in this
> > > regard.
> >
> > If it was there, would you have read it?  :)
> 
> Absolutely; I went looking for it, which is why I haven't added my
> reviewed by tag, because it's not clear from the existing comment
> block how callers should handle the return value, particularly as you
> describe in this commit's commit message.

Ok, fair enough, I'll update the documentation soon, thanks.

greg k-h

^ permalink raw reply

* Re: WARNING: ODEBUG bug in netdev_freemem (2)
From: Thomas Gleixner @ 2019-08-12 19:45 UTC (permalink / raw)
  To: syzbot
  Cc: alexander.h.duyck, amritha.nambiar, andriy.shevchenko, avagin,
	davem, dmitry.torokhov, dvyukov, eric.dumazet, f.fainelli, gregkh,
	idosch, jiri, kimbrownkd, linux-kernel, netdev, syzkaller-bugs,
	tyhicks, wanghai26, yuehaibing
In-Reply-To: <000000000000ea2c30058f901624@google.com>

On Wed, 7 Aug 2019, syzbot wrote:

> syzbot has found a reproducer for the following crash on:
> 
> HEAD commit:    13dfb3fa Merge git://git.kernel.org/pub/scm/linux/kernel/g..
> git tree:       net-next
> console output: https://syzkaller.appspot.com/x/log.txt?x=1671e69a600000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=d4cf1ffb87d590d7
> dashboard link: https://syzkaller.appspot.com/bug?extid=c4521ac872a4ccc3afec
> compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=170542c2600000

I can't reproduce that here. Can you please apply the patch from:

  https://lore.kernel.org/lkml/alpine.DEB.2.21.1906241920540.32342@nanos.tec.linutronix.de

and try to reproduce with that applied? That should give us more
information about the actual delayed work.

Thanks,

	tglx

^ permalink raw reply

* Re: [PATCH v3 13/17] mvpp2: no need to check return value of debugfs_create functions
From: Nick Desaulniers @ 2019-08-12 19:44 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: netdev, David S. Miller, Maxime Chevallier, Nathan Huckleberry
In-Reply-To: <20190812190128.GB14905@kroah.com>

On Mon, Aug 12, 2019 at 12:01 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Mon, Aug 12, 2019 at 10:55:51AM -0700, Nick Desaulniers wrote:
> > On Sat, Aug 10, 2019 at 3:17 AM Greg Kroah-Hartman
> > <gregkh@linuxfoundation.org> wrote:
> > >
> > > When calling debugfs functions, there is no need to ever check the
> > > return value.  The function can work or not, but the code logic should
> > > never do something different based on this.
> >
> > Maybe adding this recommendation to the comment block above the
> > definition of debugfs_create_dir() in fs/debugfs/inode.c would help
> > prevent this issue in the future?  What failure means, and how to
> > proceed can be tricky; more documentation can only help in this
> > regard.
>
> If it was there, would you have read it?  :)

Absolutely; I went looking for it, which is why I haven't added my
reviewed by tag, because it's not clear from the existing comment
block how callers should handle the return value, particularly as you
describe in this commit's commit message.

>
> I'll add it to the list for when I revamp the debugfs documentation that
> is already in the kernel, that very few people actually read...
>
> thanks,
>
> greg k-h



-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply

* [PATCH net] net: phy: consider AN_RESTART status when reading link status
From: Heiner Kallweit @ 2019-08-12 19:20 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, David Miller
  Cc: netdev@vger.kernel.org, Yonglong Liu

After configuring and restarting aneg we immediately try to read the
link status. On some systems the PHY may not yet have cleared the
"aneg complete" and "link up" bits, resulting in a false link-up
signal. See [0] for a report.
Clause 22 and 45 both require the PHY to keep the AN_RESTART
bit set until the PHY actually starts auto-negotiation.
Let's consider this in the generic functions for reading link status.
The commit marked as fixed is the first one where the patch applies
cleanly.

[0] https://marc.info/?t=156518400300003&r=1&w=2

Fixes: c1164bb1a631 ("net: phy: check PMAPMD link status only in genphy_c45_read_link")
Tested-by: Yonglong Liu <liuyonglong@huawei.com>
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/phy/phy-c45.c    | 14 ++++++++++++++
 drivers/net/phy/phy_device.c | 12 +++++++++++-
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c
index b9d414578..58bb25e4a 100644
--- a/drivers/net/phy/phy-c45.c
+++ b/drivers/net/phy/phy-c45.c
@@ -219,6 +219,20 @@ int genphy_c45_read_link(struct phy_device *phydev)
 	int val, devad;
 	bool link = true;
 
+	if (phydev->c45_ids.devices_in_package & MDIO_DEVS_AN) {
+		val = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_CTRL1);
+		if (val < 0)
+			return val;
+
+		/* Autoneg is being started, therefore disregard current
+		 * link status and report link as down.
+		 */
+		if (val & MDIO_AN_CTRL1_RESTART) {
+			phydev->link = 0;
+			return 0;
+		}
+	}
+
 	while (mmd_mask && link) {
 		devad = __ffs(mmd_mask);
 		mmd_mask &= ~BIT(devad);
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index b039632de..163295dbc 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1741,7 +1741,17 @@ EXPORT_SYMBOL(genphy_aneg_done);
  */
 int genphy_update_link(struct phy_device *phydev)
 {
-	int status;
+	int status = 0, bmcr;
+
+	bmcr = phy_read(phydev, MII_BMCR);
+	if (bmcr < 0)
+		return bmcr;
+
+	/* Autoneg is being started, therefore disregard BMSR value and
+	 * report link as down.
+	 */
+	if (bmcr & BMCR_ANRESTART)
+		goto done;
 
 	/* The link state is latched low so that momentary link
 	 * drops can be detected. Do not double-read the status
-- 
2.22.0


^ permalink raw reply related

* [PATCH v2] net/mlx4_en: fix a memory leak bug
From: Wenwen Wang @ 2019-08-12 19:11 UTC (permalink / raw)
  To: Wenwen Wang
  Cc: Tariq Toukan, David S. Miller,
	open list:MELLANOX ETHERNET DRIVER (mlx4_en),
	open list:MELLANOX MLX4 core VPI driver, open list

In mlx4_en_config_rss_steer(), 'rss_map->indir_qp' is allocated through
kzalloc(). After that, mlx4_qp_alloc() is invoked to configure RSS
indirection. However, if mlx4_qp_alloc() fails, the allocated
'rss_map->indir_qp' is not deallocated, leading to a memory leak bug.

To fix the above issue, add the 'qp_alloc_err' label to free
'rss_map->indir_qp'.

Fixes: 4931c6ef04b4 ("net/mlx4_en: Optimized single ring steering")

Signed-off-by: Wenwen Wang <wenwen@cs.uga.edu>
---
 drivers/net/ethernet/mellanox/mlx4/en_rx.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_rx.c b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
index 6c01314..db3552f 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_rx.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
@@ -1187,7 +1187,7 @@ int mlx4_en_config_rss_steer(struct mlx4_en_priv *priv)
 	err = mlx4_qp_alloc(mdev->dev, priv->base_qpn, rss_map->indir_qp);
 	if (err) {
 		en_err(priv, "Failed to allocate RSS indirection QP\n");
-		goto rss_err;
+		goto qp_alloc_err;
 	}
 
 	rss_map->indir_qp->event = mlx4_en_sqp_event;
@@ -1241,6 +1241,7 @@ int mlx4_en_config_rss_steer(struct mlx4_en_priv *priv)
 		       MLX4_QP_STATE_RST, NULL, 0, 0, rss_map->indir_qp);
 	mlx4_qp_remove(mdev->dev, rss_map->indir_qp);
 	mlx4_qp_free(mdev->dev, rss_map->indir_qp);
+qp_alloc_err:
 	kfree(rss_map->indir_qp);
 	rss_map->indir_qp = NULL;
 rss_err:
-- 
2.7.4


^ permalink raw reply related

* libbpf distro packaging
From: Julia Kartseva @ 2019-08-12 19:04 UTC (permalink / raw)
  To: labbott@redhat.com, acme@kernel.org,
	debian-kernel@lists.debian.org, netdev@vger.kernel.org
  Cc: Andrii Nakryiko, Andrey Ignatov, Alexei Starovoitov,
	Yonghong Song, jolsa@kernel.org

I would like to bring up libbpf publishing discussion started at [1].
The present state of things is that libbpf is built from kernel tree, e.g. [2]
For Debian and [3] for Fedora whereas the better way would be having a
package built from github mirror. The advantages of the latter:
- Consistent, ABI matching versioning across distros
- The mirror has integration tests
- No need in kernel tree to build a package
- Changes can be merged directly to github w/o waiting them to be merged
through bpf-next -> net-next -> main
There is a PR introducing a libbpf.spec which can be used as a starting point: [4]
Any comments regarding the spec itself can be posted there.
In the future it may be used as a source of truth.
Please consider switching libbpf packaging to the github mirror instead
of the kernel tree.
Thanks

[1] https://lists.iovisor.org/g/iovisor-dev/message/1521
[2] https://packages.debian.org/sid/libbpf4.19
[3] http://rpmfind.net/linux/RPM/fedora/devel/rawhide/x86_64/l/libbpf-5.3.0-0.rc2.git0.1.fc31.x86_64.html
[4] https://github.com/libbpf/libbpf/pull/64



^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox