Skip to content

L
linux
Switch branch/tag
  1. 18 Mar, 2020 9 commits
    • Arthur Kiyanovski's avatar
      net: ena: fix request of incorrect number of IRQ vectors · e02ae6ed
      Arthur Kiyanovski authored 
      Bug:
In short the main issue is caused by the fact that the number of queues
is changed using ethtool after ena_probe() has been called and before
ena_up() was executed. Here is the full scenario in detail:

* ena_probe() is called when the driver is loaded, the driver is not up
  yet at the end of ena_probe().
* The number of queues is changed -> io_queue_count is changed as well -
  ena_up() is not called since the "dev_was_up" boolean in
  ena_update_queue_count() is false.
* ena_up() is called by the kernel (it's called asynchronously some
  time after ena_probe()). ena_setup_io_intr() is called by ena_up() and
  it uses io_queue_count to get the suitable irq lines for each msix
  vector. The function ena_request_io_irq() is called right after that
  and it uses msix_vecs - This value only changes during ena_probe() and
  ena_restore() - to request the irq vectors. This results in "Failed to
  request I/O IRQ" error for i > io_queue_count.

Numeric example:
* After ena_probe() io_queue_count = 8, msix_vecs = 9.
* The number of queues changes to 4 -> io_queue_count = 4, msix_vecs = 9.
* ena_up() is executed for the first time:
  ** ena_setup_io_intr() inits the vectors only up to io_queue_count.
  ** ena_request_io_irq() calls request_irq() and fails for i = 5.

How to reproduce:
simply run the following commands:
    sudo rmmod ena && sudo insmod ena.ko;
    sudo ethtool -L eth1 combined 3;

Fix:
Use ENA_MAX_MSIX_VEC(adapter->num_io_queues + adapter->xdp_num_queues)
instead of adapter->msix_vecs. We need to take XDP queues into
consideration as they need to have msix vectors assigned to them as well.
Note that the XDP cannot be attached before the driver is up and running
but in XDP mode the issue might occur when the number of queues changes
right after a reset trigger.
The ENA_MAX_MSIX_VEC simply adds one to the argument since the first msix
vector is reserved for management queue.

Fixes: 1738cd3e ("net: ena: Add a driver for Amazon Elastic Network Adapters (ENA)")
Signed-off-by: default avatarSameeh Jubran <sameehj@amazon.com>
Signed-off-by: default avatarArthur Kiyanovski <akiyano@amazon.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      e02ae6ed
    • Arthur Kiyanovski's avatar
      net: ena: fix incorrect setting of the number of msix vectors · ce1f3521
      Arthur Kiyanovski authored 
      Overview:
We don't frequently change the msix vectors throughout the life cycle of
the driver. We do so in two functions: ena_probe() and ena_restore().
ena_probe() is only called when the driver is loaded. ena_restore() on the
other hand is called during device reset / resume operations.

We use num_io_queues for calculating and allocating the number of msix
vectors. At ena_probe() this value is equal to max_num_io_queues and thus
this is not an issue, however ena_restore() might be called after the
number of io queues has changed.

A possible bug scenario is as follows:

* Change number of queues from 8 to 4.
  (num_io_queues = 4, max_num_io_queues = 8, msix_vecs = 9,)
* Trigger reset occurs -> ena_restore is called.
  (num_io_queues = 4, max_num_io_queues =8 , msix_vecs = 5)
* Change number of queues from 4 to 6.
  (num_io_queues = 6, max_num_io_queues = 8, msix_vecs = 5)
* The driver will reset due to failure of check_for_rx_interrupt_queue()

Fix:
This can be easily fixed by always using max_num_io_queues to init the
msix_vecs, since this number won't change as opposed to num_io_queues.

Fixes: 4d192660 ("net: ena: multiple queue creation related cleanups")
Signed-off-by: default avatarSameeh Jubran <sameehj@amazon.com>
Signed-off-by: default avatarArthur Kiyanovski <akiyano@amazon.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      ce1f3521
    • Rayagonda Kokatanur's avatar
      net: phy: mdio-mux-bcm-iproc: check clk_prepare_enable() return value · 872307ab
      Rayagonda Kokatanur authored 
      Check clk_prepare_enable() return value.

Fixes: 2c723044 ("net: phy: Add pm support to Broadcom iProc mdio mux driver")
Signed-off-by: default avatarRayagonda Kokatanur <rayagonda.kokatanur@broadcom.com>
Reviewed-by: default avatarAndrew Lunn <andrew@lunn.ch>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      872307ab
    • David S. Miller's avatar
      Merge branch 'net-bcmgenet-revisit-MAC-reset' · af4e6671
      David S. Miller authored 
      Doug Berger says:

====================
net: bcmgenet: revisit MAC reset

Commit 3a55402c ("net: bcmgenet: use RGMII loopback for MAC
reset") was intended to resolve issues with reseting the UniMAC
core within the GENET block by providing better control over the
clocks used by the UniMAC core. Unfortunately, it is not
compatible with all of the supported system configurations so an
alternative method must be applied.

This commit set provides such an alternative. The first commit
reverts the previous change and the second commit provides the
alternative reset sequence that addresses the concerns observed
with the previous implementation.

This replacement implementation should be applied to the stable
branches wherever commit 3a55402c ("net: bcmgenet: use RGMII
loopback for MAC reset") has been applied.

Unfortunately, reverting that commit may conflict with some
restructuring changes introduced by commit 4f8d81b7 ("net:
bcmgenet: Refactor register access in bcmgenet_mii_config").
The first commit in this set has been manually edited to
resolve the conflict on net/master. I would be happy to help
stable maintainers with resolving any such conflicts if they
occur. However, I do not expect that commit to have been
backported to stable branch so hopefully the revert can be
applied cleanly.
====================
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      af4e6671
    • Doug Berger's avatar
      net: bcmgenet: keep MAC in reset until PHY is up · 88f6c8bf
      Doug Berger authored 
      As noted in commit 28c2d1a7 ("net: bcmgenet: enable loopback
during UniMAC sw_reset") the UniMAC must be clocked at least 5
cycles while the sw_reset is asserted to ensure a clean reset.

That commit enabled local loopback to provide an Rx clock from the
GENET sourced Tx clk. However, when connected in MII mode the Tx
clk is sourced by the PHY so if an EPHY is not supplying clocks
(e.g. when the link is down) the UniMAC does not receive the
necessary clocks.

This commit extends the sw_reset window until the PHY reports that
the link is up thereby ensuring that the clocks are being provided
to the MAC to produce a clean reset.

One consequence is that if the system attempts to enter a Wake on
LAN suspend state when the PHY link has not been active the MAC
may not have had a chance to initialize cleanly. In this case, we
remove the sw_reset and enable the WoL reception path as normal
with the hope that the PHY will provide the necessary clocks to
drive the WoL blocks if the link becomes active after the system
has entered suspend.

Fixes: 1c1008c7 ("net: bcmgenet: add main driver file")
Signed-off-by: default avatarDoug Berger <opendmb@gmail.com>
Acked-by: default avatarFlorian Fainelli <f.fainelli@gmail.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      88f6c8bf
    • Doug Berger's avatar
      Revert "net: bcmgenet: use RGMII loopback for MAC reset" · 612eb1c3
      Doug Berger authored 
      This reverts commit 3a55402c.

This is not a good solution when connecting to an external switch
that may not support the isolation of the TXC signal resulting in
output driver contention on the pin.

A different solution is necessary.
Signed-off-by: default avatarDoug Berger <opendmb@gmail.com>
Acked-by: default avatarFlorian Fainelli <f.fainelli@gmail.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      612eb1c3
    • David S. Miller's avatar
      Merge branch 'net-mvmdio-avoid-error-message-for-optional-IRQ' · d36963b8
      David S. Miller authored 
      Chris Packham says:

====================
net: mvmdio: avoid error message for optional IRQ

I've gone ahead an sent a revert. This is the same as the original v1 except
I've added Andrew's review to the commit message.
====================
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      d36963b8
    • Chris Packham's avatar
      net: mvmdio: avoid error message for optional IRQ · fa2632f7
      Chris Packham authored 
      Per the dt-binding the interrupt is optional so use
platform_get_irq_optional() instead of platform_get_irq(). Since
commit 7723f4c5 ("driver core: platform: Add an error message to
platform_get_irq*()") platform_get_irq() produces an error message

  orion-mdio f1072004.mdio: IRQ index 0 not found

which is perfectly normal if one hasn't specified the optional property
in the device tree.
Signed-off-by: default avatarChris Packham <chris.packham@alliedtelesis.co.nz>
Reviewed-by: default avatarAndrew Lunn <andrew@lunn.ch>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      fa2632f7
    • Chris Packham's avatar
      Revert "net: mvmdio: avoid error message for optional IRQ" · 028fd76b
      Chris Packham authored 
      This reverts commit e1f550dc.
platform_get_irq_optional() will still return -ENXIO when no interrupt
is provided so the additional error handling caused the driver prone to
fail when no interrupt was specified. Revert the change so we can apply
the correct minimal fix.
Signed-off-by: default avatarChris Packham <chris.packham@alliedtelesis.co.nz>
Reviewed-by: default avatarAndrew Lunn <andrew@lunn.ch>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      028fd76b
  3. 17 Mar, 2020 1 commit
  5. 16 Mar, 2020 23 commits
  7. 15 Mar, 2020 7 commits
    • Florian Westphal's avatar
      geneve: move debug check after netdev unregister · 0fda7600
      Florian Westphal authored 
      The debug check must be done after unregister_netdevice_many() call --
the list_del() for this is done inside .ndo_stop.

Fixes: 2843a253 ("geneve: speedup geneve tunnels dismantle")
Reported-and-tested-by: <syzbot+68a8ed58e3d17c700de5@syzkaller.appspotmail.com>
Cc: Haishuang Yan <yanhaishuang@cmss.chinamobile.com>
Signed-off-by: default avatarFlorian Westphal <fw@strlen.de>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      0fda7600
    • Willem de Bruijn's avatar
      net/packet: tpacket_rcv: avoid a producer race condition · 61fad681
      Willem de Bruijn authored 
      PACKET_RX_RING can cause multiple writers to access the same slot if a
fast writer wraps the ring while a slow writer is still copying. This
is particularly likely with few, large, slots (e.g., GSO packets).

Synchronize kernel thread ownership of rx ring slots with a bitmap.

Writers acquire a slot race-free by testing tp_status TP_STATUS_KERNEL
while holding the sk receive queue lock. They release this lock before
copying and set tp_status to TP_STATUS_USER to release to userspace
when done. During copying, another writer may take the lock, also see
TP_STATUS_KERNEL, and start writing to the same slot.

Introduce a new rx_owner_map bitmap with a bit per slot. To acquire a
slot, test and set with the lock held. To release race-free, update
tp_status and owner bit as a transaction, so take the lock again.

This is the one of a variety of discussed options (see Link below):

* instead of a shadow ring, embed the data in the slot itself, such as
in tp_padding. But any test for this field may match a value left by
userspace, causing deadlock.

* avoid the lock on release. This leaves a small race if releasing the
shadow slot before setting TP_STATUS_USER. The below reproducer showed
that this race is not academic. If releasing the slot after tp_status,
the race is more subtle. See the first link for details.

* add a new tp_status TP_KERNEL_OWNED to avoid the transactional store
of two fields. But, legacy applications may interpret all non-zero
tp_status as owned by the user. As libpcap does. So this is possible
only opt-in by newer processes. It can be added as an optional mode.

* embed the struct at the tail of pg_vec to avoid extra allocation.
The implementation proved no less complex than a separate field.

The additional locking cost on release adds contention, no different
than scaling on multicore or multiqueue h/w. In practice, below
reproducer nor small packet tcpdump showed a noticeable change in
perf report in cycles spent in spinlock. Where contention is
problematic, packet sockets support mitigation through PACKET_FANOUT.
And we can consider adding opt-in state TP_KERNEL_OWNED.

Easy to reproduce by running multiple netperf or similar TCP_STREAM
flows concurrently with `tcpdump -B 129 -n greater 60000`.

Based on an earlier patchset by Jon Rosen. See links below.

I believe this issue goes back to the introduction of tpacket_rcv,
which predates git history.

Link: https://www.mail-archive.com/netdev@vger.kernel.org/msg237222.htmlSuggested-by: default avatarJon Rosen <jrosen@cisco.com>
Signed-off-by: default avatarWillem de Bruijn <willemb@google.com>
Signed-off-by: default avatarJon Rosen <jrosen@cisco.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      61fad681
    • Petr Machata's avatar
      net: ip_gre: Separate ERSPAN newlink / changelink callbacks · e1f8f78f
      Petr Machata authored 
      ERSPAN shares most of the code path with GRE and gretap code. While that
helps keep the code compact, it is also error prone. Currently a broken
userspace can turn a gretap tunnel into a de facto ERSPAN one by passing
IFLA_GRE_ERSPAN_VER. There has been a similar issue in ip6gretap in the
past.

To prevent these problems in future, split the newlink and changelink code
paths. Split the ERSPAN code out of ipgre_netlink_parms() into a new
function erspan_netlink_parms(). Extract a piece of common logic from
ipgre_newlink() and ipgre_changelink() into ipgre_newlink_encap_setup().
Add erspan_newlink() and erspan_changelink().

Fixes: 84e54fe0 ("gre: introduce native tunnel support for ERSPAN")
Signed-off-by: default avatarPetr Machata <petrm@mellanox.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      e1f8f78f
    • Shahjada Abul Husain's avatar
      cxgb4: fix delete filter entry fail in unload path · 46ea929b
      Shahjada Abul Husain authored 
      Currently, the hardware TID index is assumed to start from index 0.
However, with the following changeset,

commit c2193999 ("cxgb4: add support for high priority filters")

hardware TID index can start after the high priority region, which
has introduced a regression resulting in remove filters entry
failure for cxgb4 unload path. This patch fix that.

Fixes: c2193999 ("cxgb4: add support for high priority filters")
Signed-off-by: default avatarShahjada Abul Husain <shahjada@chelsio.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      46ea929b
    • Markus Fuchs's avatar
      net: stmmac: platform: Fix misleading interrupt error msg · fc191af1
      Markus Fuchs authored 
      Not every stmmac based platform makes use of the eth_wake_irq or eth_lpi
interrupts. Use the platform_get_irq_byname_optional variant for these
interrupts, so no error message is displayed, if they can't be found.
Rather print an information to hint something might be wrong to assist
debugging on platforms which use these interrupts.
Signed-off-by: default avatarMarkus Fuchs <mklntf@gmail.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      fc191af1
    • Bruno Meneguele's avatar
      net/bpfilter: fix dprintf usage for /dev/kmsg · 13d0f7b8
      Bruno Meneguele authored 
      The bpfilter UMH code was recently changed to log its informative messages to
/dev/kmsg, however this interface doesn't support SEEK_CUR yet, used by
dprintf(). As result dprintf() returns -EINVAL and doesn't log anything.

However there already had some discussions about supporting SEEK_CUR into
/dev/kmsg interface in the past it wasn't concluded. Since the only user of
that from userspace perspective inside the kernel is the bpfilter UMH
(userspace) module it's better to correct it here instead waiting a conclusion
on the interface.

Fixes: 36c4357c ("net: bpfilter: print umh messages to /dev/kmsg")
Signed-off-by: default avatarBruno Meneguele <bmeneg@redhat.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      13d0f7b8
    • Cong Wang's avatar
      net_sched: keep alloc_hash updated after hash allocation · 0d1c3530
      Cong Wang authored 
      In commit 599be01e ("net_sched: fix an OOB access in cls_tcindex")
I moved cp->hash calculation before the first
tcindex_alloc_perfect_hash(), but cp->alloc_hash is left untouched.
This difference could lead to another out of bound access.

cp->alloc_hash should always be the size allocated, we should
update it after this tcindex_alloc_perfect_hash().

Reported-and-tested-by: syzbot+dcc34d54d68ef7d2d53d@syzkaller.appspotmail.com
Reported-and-tested-by: syzbot+c72da7b9ed57cde6fca2@syzkaller.appspotmail.com
Fixes: 599be01e ("net_sched: fix an OOB access in cls_tcindex")
Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: Jiri Pirko <jiri@resnulli.us>
Signed-off-by: default avatarCong Wang <xiyou.wangcong@gmail.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      0d1c3530
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7