Project

General

Profile

Bug #11489

DLS stat delete and aggr kstat can deadlock

Added by Robert Mustacchi 12 months ago. Updated 4 months ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
networking
Start date:
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
Gerrit CR:

Description

A process running kstat against the link module (e.g., kstat -p link:::) can deadlock with a process performing a dladm rename-link operation (or any process calling dls_stat_delete() from inside the MAC perim). These two operations perform the kstat_hold() and MAC perim enter in the opposite order.

Thread A is a node process reading the link module kstats. It's waiting on the MAC perim so it can read one of the MAC stats:

> 0t28505::pid2proc|::walk thread| ::findstack -v
stack pointer for thread ffffd0c8c4ded140: ffffd003d575a750
[ ffffd003d575a750 _resume_from_idle+0x112() ]
  ffffd003d575a780 swtch+0x141()
  ffffd003d575a7c0 cv_wait+0x70(ffffd0c572dbfc04, ffffd0c572dbfbf0)
  ffffd003d575a800 i_mac_perim_enter+0x63(ffffd0c572dbfb18)
  ffffd003d575a830 mac_perim_enter_by_mh+0x23(ffffd0c572dbfb18, ffffd003d575a848
  )
  ffffd003d575a890 aggr_m_stat+0x35(ffffd0c5884201b8, 3e8, ffffd003d575a8a8)
  ffffd003d575a8d0 mac_stat_get+0x41(ffffd0c572dbfb18, 3e8)
  ffffd003d575a900 mac_client_ifspeed+0x24(ffffd0c8b8717660)
  ffffd003d575a920 mac_client_stat_get+0x29d(ffffd0c8b8717660, 3e8)
  ffffd003d575a960 vnic_m_stat+0x75(ffffd0db6359a2d8, 3e8, ffffd003d575a978)
  ffffd003d575a9a0 mac_stat_get+0x41(ffffd0c902e555b0, 3e8)
  ffffd003d575a9e0 dls_stat_update+0x49(ffffd35f205b2260, ffffd10e2c18d058, 0)
  ffffd003d575aa40 dls_devnet_stat_update+0xaf(ffffd35f205b2260, 0)
  ffffd003d575ac30 read_kstat_data+0xd0(ffffd003d575ae58, 97e59a8, 100001)
  ffffd003d575ac70 kstat_ioctl+0x83(200000000, 4b02, 97e59a8, 100001,
  ffffd0c5855fee18, ffffd003d575ae58)
  ffffd003d575acb0 cdev_ioctl+0x39(200000000, 4b02, 97e59a8, 100001,
  ffffd0c5855fee18, ffffd003d575ae58)
  ffffd003d575ad00 spec_ioctl+0x60(ffffd0c579709780, 4b02, 97e59a8, 100001,
  ffffd0c5855fee18, ffffd003d575ae58, 0)
  ffffd003d575ad90 fop_ioctl+0x55(ffffd0c579709780, 4b02, 97e59a8, 100001,
  ffffd0c5855fee18, ffffd003d575ae58, 0)
  ffffd003d575aeb0 ioctl+0x9b(b, 4b02, 97e59a8)
  ffffd003d575af10 _sys_sysenter_post_swapgs+0x153()

Thread A is part of the cmon-agent process:

> 0xffffd0c8c4ded140::print kthread_t t_procp | ::print proc_t p_user.u_psargs
p_user.u_psargs = [ " 
/opt/smartdc/agents/lib/node_modules/cmon-agent/node/bin/node --abort_on_uncaug" ]

The owner of the MAC perim, Thread B, is waiting for a kstat hold:

> ffffd0c572dbfb18::print mac_impl_t mi_perim_owner | ::findstack -v
stack pointer for thread ffffd0c8ba899000: ffffd003d5a7b810
[ ffffd003d5a7b810 _resume_from_idle+0x112() ]
  ffffd003d5a7b840 swtch+0x141()
  ffffd003d5a7b880 cv_wait+0x70(ffffd35f205b2328, fffffffffbc1bfa0)
  ffffd003d5a7b8c0 kstat_hold+0x4d(fffffffffbc2bfe0, ffffd003d5a7b8d0)
  ffffd003d5a7b9f0 kstat_hold_bykid+0x37(1da254, 0)
  ffffd003d5a7ba30 kstat_delete+0x4e(ffffd35f205b2260)
  ffffd003d5a7ba60 dls_stat_delete+0x1e(ffffd35f205b2260)
  ffffd003d5a7ba90 dls_devnet_stat_rename+0x2f(ffffd0c8a3e88d58, 1)
  ffffd003d5a7bb60 dls_devnet_rename+0x364(f1e, 0, ffffd105c40ee2c0, 1)
  ffffd003d5a7bbd0 drv_ioc_rename+0xbb(ffffd105c40ee2b8, 8047784, 100003,
  ffffd0c8bb8ebaf0, ffffd003d5a7be58)
  ffffd003d5a7bc70 drv_ioctl+0x1e4(1200000000, d1d0011, 8047784, 100003,
  ffffd0c8bb8ebaf0, ffffd003d5a7be58)
  ffffd003d5a7bcb0 cdev_ioctl+0x39(1200000000, d1d0011, 8047784, 100003,
  ffffd0c8bb8ebaf0, ffffd003d5a7be58)
  ffffd003d5a7bd00 spec_ioctl+0x60(ffffd0c50ea2d700, d1d0011, 8047784, 100003,
  ffffd0c8bb8ebaf0, ffffd003d5a7be58, 0)
  ffffd003d5a7bd90 fop_ioctl+0x55(ffffd0c50ea2d700, d1d0011, 8047784, 100003,
  ffffd0c8bb8ebaf0, ffffd003d5a7be58, 0)
  ffffd003d5a7beb0 ioctl+0x9b(4, d1d0011, 8047784)
  ffffd003d5a7bf10 _sys_sysenter_post_swapgs+0x153()

Thread B is part of dladm rename-link process. This is performed as part of zone startup on SmartOS.

> ffffd0c572dbfb18::print mac_impl_t mi_perim_owner | ::print kthread_t t_procp | ::print struct proc p_user.u_psargs
p_user.u_psargs = [ " 
dladm rename-link -z d3c62c61-83ad-45fb-b13a-1d673c5d752b tmp937960 net0" ]

Thread B is waiting to get a hold on this kstat:

> ffffd35f205b2328 - c8::print ekstat_t
{
    e_ks = {
        ks_crtime = 0x260baba253d0fc
        ks_next = 0
        ks_kid = 0x1da254
        ks_module = [ "link" ]
        ks_resv = 0
        ks_instance = 0
        ks_name = [ "tmp937960" ]
        ks_type = 0x1
        ks_class = [ "net" ]
        ks_flags = 0x41
        ks_data = 0xffffd0c8ae0d5280
        ks_ndata = 0x16
        ks_data_size = 0x427
        ks_snaptime = 0x260baba253d0fc
        ks_update = dls_devnet_stat_update
        ks_private = 0xffffd0c8a3e88d58
        ks_snapshot = default_kstat_snapshot
        ks_lock = 0
    }
    e_size = 0x110
    e_owner = 0xffffd0c8c4ded140
    e_cv = {
        _opaque = 0x1
    }
    e_avl_bykid = {
        avl_child = [ 0, 0 ]
        avl_pcb = 0xffffd3f4e523e0d5
    }
    e_avl_byname = {
        avl_child = [ 0, 0 ]
        avl_pcb = 0xffffd0d541f42d09
    }
    e_zone = {
        zoneid = 0
        next = 0
    }
}

But the e_owner of this kstat is Thread A – which is the same thread waiting for the MAC perim. And the MAC perim is owned by Thread B which is waiting on the kstat. Boom, we have a deadlock.

The kstat library first gets a hold on the kstat and then performs a module callback: dls_devnet_stat_update(). This callback will eventually call into MAC to read the stats: thus it has kstat hold -> MAC perim ordering.

The dls_devnet_rename() function does the opposite. It calls dls_devnet_stat_rename() from inside the MAC perim which eventually calls dls_stat_delete() which tries to grab a hold on the kstat. So it performs MAC perim -> kstat hold ordering.

History

#1

Updated by Electric Monk 4 months ago

  • Status changed from New to Closed

git commit 84de666edc7f7d835057ae4807a387447c086bcf

commit  84de666edc7f7d835057ae4807a387447c086bcf
Author: Ryan Zezeski <ryan@zinascii.com>
Date:   2020-03-02T07:24:09.000Z

    11490 SRS ring polling disabled for VLANs
    11491 Want DLS bypass for VLAN traffic
    11492 add VLVF bypass to ixgbe core
    2869 duplicate packets with vnics over aggrs
    11489 DLS stat delete and aggr kstat can deadlock
    Portions contributed by: Theo Schlossnagle <jesus@omniti.com>
    Reviewed by: Patrick Mooney <patrick.mooney@joyent.com>
    Reviewed by: Robert Mustacchi <rm@joyent.com>
    Reviewed by: Dan McDonald <danmcd@joyent.com>
    Reviewed by: Paul Winder <paul@winders.demon.co.uk>
    Approved by: Gordon Ross <gordon.w.ross@gmail.com>

Also available in: Atom PDF