illumos gate

Thanks for that Chris. That helps point us in the right direction. What's happening is that we're failing this bit of the bge_addmac implementation, I think:

...
1483         if ((err = bge_unicst_set(bgep, mac_addr, slot)) != 0)
1484                 goto fail;
1485 
1486         /* A rule is already here. Deny this.  */
1487         if (rrp->mac_addr_rule != NULL) {
1488                 err = ether_cmp(mac_addr, rrp->mac_addr_val) ? EEXIST : EBUSY;
1489                 goto fail;
1490         }
...

As we've gotten an EEXIST. I'm not that familiar with this part of the bge driver, so I have some general DTrace bits I've written below to try and get more information about what the configuration of this ring is:

fbt::bge_addmac:entry
{
        self->t = 1;
        stack();
        self->ring = (recv_ring_t *)arg0;
        self->bge = self->ring->bgep;
        printf("attempting to add mac:\n");
        tracemem(arg1, 6);
        print(*self->ring);

        printf("slot 0\n");
        print(self->bge->curr_addr[0]);
        print(self->bge->curr_addr[1]);
        print(self->bge->curr_addr[2]);
        print(self->bge->curr_addr[3]);
}

fbt::bge_addmac:entry
/self->ring != NULL && self->ring->mac_addr_rule != NULL/
{
        printf("ring rule\n");
        print(*self->ring->mac_addr_rule);
}

fbt::bge_unicst_find:return
/self->t/
{
        printf("picked slot %d\n", args[1]);
        print(self->bge->curr_addr[args[1]]);
}

fbt::bge_addmac:return
{
        self->t = 0;
        self->ring = NULL;
        self->bge = NULL;
}

# dladm show-phys -m
LINK         SLOT     ADDRESS            INUSE CLIENT
bge0         primary  64:51:6:d8:7:f8    yes  bge0
bge1         primary  64:51:6:d8:7:f9    yes  bge1

Dtrace output from running vmadm start <my swift zone's uuid> :

CPU     ID                    FUNCTION:NAME
  0  82168                 bge_addmac:entry 
              mac`mac_group_addmac+0x7f
              mac`mac_add_macaddr_vlan+0x14a
              mac`mac_datapath_setup+0x6cb
              mac`mac_client_datapath_setup+0x298
              mac`i_mac_unicast_add+0x55e
              mac`mac_unicast_add+0x6e
              vnic`vnic_unicast_add+0x206
              vnic`vnic_dev_create+0x3d9
              vnic`vnic_ioc_create+0xea
              dld`drv_ioctl+0x1ef
              genunix`cdev_ioctl+0x2b
              specfs`spec_ioctl+0x45
              genunix`fop_ioctl+0x5b
              genunix`ioctl+0x153
              unix`_sys_sysenter_post_swapgs+0x159
attempting to add mac:

             0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f  0123456789abcdef
         0: 52 10 62 36 01 6f                                R.b6.o
recv_ring_t {
    dma_area_t desc = {
        ddi_acc_handle_t acc_hdl = 0xfffffe16dd309b80
        void *mem_va = 0xfffffe16dd492000
        uint32_t nslots = 0x200
        uint32_t size = 0x20
        size_t alength = 0x4000
        ddi_dma_handle_t dma_hdl = 0xfffffe16dd466a80
        offset_t offset = 0
        ddi_dma_cookie_t cookie = {
            union _dmu = {
                uint64_t _dmac_ll = 0x20999000
                uint32_t [2] _dmac_la = [ 0x20999000, 0 ]
            }
            size_t dmac_size = 0x4000
            uint_t dmac_type = 0
        }
        uint32_t ncookies = 0x1
        uint32_t token = 0xbcd573cb
    }
    bge_rcb_t hw_rcb = {
        uint64_t host_ring_addr = 0x20999000
        uint32_t nic_ring_addr = 0
        uint16_t flags = 0
        uint16_t max_len = 0x200
    }
    struct bge *bgep = 0xfffffe16dd122000
    ddi_softintr_t rx_softint = 0
    volatile uint16_t *prod_index_p = 0xfffffe16dd49c814
    bge_regno_t chip_mbx_reg = 0x280
    kmutex_t [1] rx_lock = [ 
        kmutex_t {
            void *[1] _opaque = [ 0 ]
        }
    ]
    uint64_t rx_next = 0x4
    mac_ring_handle_t ring_handle = 0xfffffe16ddc30e60
    mac_group_handle_t ring_group_handle = 0xfffffe16dd309680
    uint64_t ring_gen_num = 0
    bge_rule_info_t *mac_addr_rule = 0xfffffe171fa1d638
    uint8_t [6] mac_addr_val = [ 0x64, 0x51, 0x6, 0xd8, 0x7, 0xf8 ]
    int poll_flag = 0
    uint64_t rx_pkts = 0x1204
    uint64_t rx_bytes = 0xb6047
}slot 0
bge_mac_addr_t {
    ether_addr_t addr = [ 0x64, 0x51, 0x6, 0xd8, 0x7, 0xf8 ]
    uint8_t spare = 0
    boolean_t set = B_TRUE
}bge_mac_addr_t {
    ether_addr_t addr = [ 0, 0, 0, 0, 0, 0 ]
    uint8_t spare = 0
    boolean_t set = B_FALSE
}bge_mac_addr_t {
    ether_addr_t addr = [ 0, 0, 0, 0, 0, 0 ]
    uint8_t spare = 0
    boolean_t set = B_FALSE
}bge_mac_addr_t {
    ether_addr_t addr = [ 0, 0, 0, 0, 0, 0 ]
    uint8_t spare = 0
    boolean_t set = B_FALSE
}
  0  82168                 bge_addmac:entry ring rule
bge_rule_info_t {
    int start = 0
    int count = 0x2
}
  0  82403           bge_unicst_find:return picked slot -1
bge_mac_addr_t {
    ether_addr_t addr = [ 0, 0, 0, 0, 0, 0 ]
    uint8_t spare = 0
    boolean_t set = B_FALSE
}

Thanks for providing this everyone. With this, I think I understand everything that's happening here. Let me recap a bit.

The hardware here has up to four mac address registers, as in places that we can actually program a packet. It also has up to 8 mac address receive rules. Basically these are rules that can be used to direct a packet to a given ring. In general, it takes two of these to map to a single MAC address to a ring.

A side effect of this is that the way the driver is designed, it wants to use up to one ring for each group and each of those groups gets a single MAC address, which requires one of the four mac address registers and 2 of the 8 receive rules. The way the driver is written today, if it finds that the ring already has a rule, it'll fail. Now, the way it does this is a bit unfortunate for a few reasons. Here's the code in questions:

1486         /* A rule is already here. Deny this.  */
1487         if (rrp->mac_addr_rule != NULL) {
1488                 err = ether_cmp(mac_addr, rrp->mac_addr_val) ? EEXIST : EBUSY;
1489                 goto fail;
1490         }

If we have an address we're going to fail. It tries to see if this address matches the existing one to return something more menaingful. But there are two problems here:

1) If the device has used up its slots it should return ENOSPC, not EBUSY. This will allow MAC to fall back into promiscuous mode.

2) ether_cmp indicates that thing are the same by returning 0. Which means that when things don't match and they return 1, like in this case, we return EEXIST.

So, this should all be changed to do what mac expects. Because of how the mc_unicst() entry point was being used previously, this didn't end up mattering very much.

Now, this doesn't answer the question of why we are even seeing this on the 5720 and the other parts. While hardware could support more than one ring, it turns out that all of the devices have been told that they only support a single ring. This is why we're coming back on group 0 and ring 0 on the first vnic being created. It turns out that all the chips have decided to limit themselves to one by default and we haven't increased this.

I think the fix here is to change this around such that we properly return ENOSPC so that it'll put us into promisc mode. We can look at adding more rings and testing that once we have the rest of this working and folks aren't experiencing regressions. I have a prototype for this I'm building and will post for folks to test once I have this ready.

I've put together a potential prototype at https://code.illumos.org/c/illumos-gate/+/653. If folks would be able to test that out, that'd be appreciated. If I can help put together materials to help with that, please let me know.

Robert Mustacchi wrote:

I've put together a potential prototype at https://code.illumos.org/c/illumos-gate/+/653. If folks would be able to test that out, that'd be appreciated. If I can help put together materials to help with that, please let me know.

I've tested Jason's updated debug build at https://us-east.manta.joyent.com/jbk/public/tmp/bge/platform-20200510T182623Z.usb.gz and vmadm can now successfully start my zones!

Ultimately we've been able to conclude that the earlier analysis was correct. This was introduced as a side effect of the attempts I made to fix the panics and probable corruption in 12496. My apologies for the breakage. Chris, Olivia, thank you very much for your help here.

Hi guys, I can confirm that VNIC creation also works on NetXtreme BCM57781 with same PI (20200510T182623Z). :)