From 1a8782e59ff9bb0ed4b75d6a346a2ed212bd2031 Mon Sep 17 00:00:00 2001 From: Alexander Duyck Date: Wed, 22 Apr 2015 21:49:25 -0700 Subject: [PATCH 01/17] fm10k: fold fm10k_pull_tail into fm10k_add_rx_frag This change folds the fm10k_pull_tail call into fm10k_add_rx_frag. The advantage to doing this is that the fragment doesn't have to be modified after it is added to the skb. Signed-off-by: Alexander Duyck Tested-by: Krishneil Singh Signed-off-by: Jeff Kirsher --- drivers/net/ethernet/intel/fm10k/fm10k_main.c | 66 ++++++------------- 1 file changed, 20 insertions(+), 46 deletions(-) diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_main.c b/drivers/net/ethernet/intel/fm10k/fm10k_main.c index c754b2027281f..982fdcdc795b4 100644 --- a/drivers/net/ethernet/intel/fm10k/fm10k_main.c +++ b/drivers/net/ethernet/intel/fm10k/fm10k_main.c @@ -269,16 +269,19 @@ static bool fm10k_add_rx_frag(struct fm10k_rx_buffer *rx_buffer, struct sk_buff *skb) { struct page *page = rx_buffer->page; + unsigned char *va = page_address(page) + rx_buffer->page_offset; unsigned int size = le16_to_cpu(rx_desc->w.length); #if (PAGE_SIZE < 8192) unsigned int truesize = FM10K_RX_BUFSZ; #else - unsigned int truesize = ALIGN(size, L1_CACHE_BYTES); + unsigned int truesize = SKB_DATA_ALIGN(size); #endif + unsigned int pull_len; - if ((size <= FM10K_RX_HDR_LEN) && !skb_is_nonlinear(skb)) { - unsigned char *va = page_address(page) + rx_buffer->page_offset; + if (unlikely(skb_is_nonlinear(skb))) + goto add_tail_frag; + if (likely(size <= FM10K_RX_HDR_LEN)) { memcpy(__skb_put(skb, size), va, ALIGN(size, sizeof(long))); /* page is not reserved, we can reuse buffer as-is */ @@ -290,8 +293,21 @@ static bool fm10k_add_rx_frag(struct fm10k_rx_buffer *rx_buffer, return false; } + /* we need the header to contain the greater of either ETH_HLEN or + * 60 bytes if the skb->len is less than 60 for skb_pad. + */ + pull_len = eth_get_headlen(va, FM10K_RX_HDR_LEN); + + /* align pull length to size of long to optimize memcpy performance */ + memcpy(__skb_put(skb, pull_len), va, ALIGN(pull_len, sizeof(long))); + + /* update all of the pointers */ + va += pull_len; + size -= pull_len; + +add_tail_frag: skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags, page, - rx_buffer->page_offset, size, truesize); + (unsigned long)va & ~PAGE_MASK, size, truesize); return fm10k_can_reuse_rx_page(rx_buffer, page, truesize); } @@ -517,44 +533,6 @@ static bool fm10k_is_non_eop(struct fm10k_ring *rx_ring, return true; } -/** - * fm10k_pull_tail - fm10k specific version of skb_pull_tail - * @skb: pointer to current skb being adjusted - * - * This function is an fm10k specific version of __pskb_pull_tail. The - * main difference between this version and the original function is that - * this function can make several assumptions about the state of things - * that allow for significant optimizations versus the standard function. - * As a result we can do things like drop a frag and maintain an accurate - * truesize for the skb. - */ -static void fm10k_pull_tail(struct sk_buff *skb) -{ - struct skb_frag_struct *frag = &skb_shinfo(skb)->frags[0]; - unsigned char *va; - unsigned int pull_len; - - /* it is valid to use page_address instead of kmap since we are - * working with pages allocated out of the lomem pool per - * alloc_page(GFP_ATOMIC) - */ - va = skb_frag_address(frag); - - /* we need the header to contain the greater of either ETH_HLEN or - * 60 bytes if the skb->len is less than 60 for skb_pad. - */ - pull_len = eth_get_headlen(va, FM10K_RX_HDR_LEN); - - /* align pull length to size of long to optimize memcpy performance */ - skb_copy_to_linear_data(skb, va, ALIGN(pull_len, sizeof(long))); - - /* update all of the pointers */ - skb_frag_size_sub(frag, pull_len); - frag->page_offset += pull_len; - skb->data_len -= pull_len; - skb->tail += pull_len; -} - /** * fm10k_cleanup_headers - Correct corrupted or empty headers * @rx_ring: rx descriptor ring packet is being transacted on @@ -580,10 +558,6 @@ static bool fm10k_cleanup_headers(struct fm10k_ring *rx_ring, return true; } - /* place header in linear portion of buffer */ - if (skb_is_nonlinear(skb)) - fm10k_pull_tail(skb); - /* if eth_skb_pad returns an error the skb was freed */ if (eth_skb_pad(skb)) return true; From 745136a8b72d638f3ee53a2b6a63f11c64a59937 Mon Sep 17 00:00:00 2001 From: Jacob Keller Date: Wed, 3 Jun 2015 16:30:58 -0700 Subject: [PATCH 02/17] fm10k: ignore invalid multicast address entries This change fixes an issue with adding an invalid multicast address using the iproute2 tool (ip maddr add dev ). The iproute2 tool and the kernel do not validate or filter the multicast addresses when adding them to the multicast list. Thus, when synchronizing this list with an invalid entry, the action will be aborted with an error since the fm10k driver currently validates the list. Consequently, multicast entries beyond the invalid one will not be processed and communicated with the switch via the mailbox. This change makes it so that invalid addresses will simply be skipped and allows synchronizing the full list to proceed. Signed-off-by: Ngai-Mint Kwan Signed-off-by: Jacob Keller Tested-by: Krishneil Singh Signed-off-by: Jeff Kirsher --- drivers/net/ethernet/intel/fm10k/fm10k_netdev.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c index 2f4f41b7eae7b..4c6b511115662 100644 --- a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c +++ b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c @@ -923,18 +923,12 @@ static int __fm10k_mc_sync(struct net_device *dev, struct fm10k_intfc *interface = netdev_priv(dev); struct fm10k_hw *hw = &interface->hw; u16 vid, glort = interface->glort; - s32 err; - - if (!is_multicast_ether_addr(addr)) - return -EADDRNOTAVAIL; /* update table with current entries */ for (vid = hw->mac.default_vid ? fm10k_find_next_vlan(interface, 0) : 0; vid < VLAN_N_VID; vid = fm10k_find_next_vlan(interface, vid)) { - err = hw->mac.ops.update_mc_addr(hw, glort, addr, vid, sync); - if (err) - return err; + hw->mac.ops.update_mc_addr(hw, glort, addr, vid, sync); } return 0; From 608bb146ff48588c43bf6332912ea8796768fb39 Mon Sep 17 00:00:00 2001 From: Jacob Keller Date: Wed, 3 Jun 2015 16:30:59 -0700 Subject: [PATCH 03/17] fm10k: use correct ethernet driver Tx timestamp function skb_complete_tx_timestamp is intended for use by PHY drivers which implement a different method of returning timestamps. This method is intended to be used after a PHY driver accepts a cloned packet via its phy_driver.txtstamp function. It is not correct to use in the standard ethernet driver such as fm10k. This patch fixes the following possible kernel panic. [ 2744.552896] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W OE 3.19.3-200.fc21.x86_64 #1 [ 2744.552899] Hardware name: Intel Corporation S2600CO/S2600CO, BIOS SE5C600.86B.02.03.8x23.060520140825 06/05/2014 [ 2744.552901] 0000000000000000 2f4c8b10ea3f9848 ffff88081ee03a38 ffffffff8176e215 [ 2744.552906] 0000000000000000 0000000000000000 ffff88081ee03a78 ffffffff8109bc1a [ 2744.552910] ffff88081ee03c50 ffff88080e55fc00 ffff88080e55fc00 ffffffff81647c50 [ 2744.552914] Call Trace: [ 2744.552917] [] dump_stack+0x45/0x57 [ 2744.552931] [] warn_slowpath_common+0x8a/0xc0 [ 2744.552936] [] ? skb_queue_purge+0x20/0x40 [ 2744.552941] [] warn_slowpath_null+0x1a/0x20 [ 2744.552946] [] skb_release_head_state+0xe1/0xf0 [ 2744.552950] [] skb_release_all+0x16/0x30 [ 2744.552954] [] kfree_skb+0x36/0x90 [ 2744.552958] [] skb_queue_purge+0x20/0x40 [ 2744.552964] [] packet_sock_destruct+0x1d/0x90 [ 2744.552968] [] __sk_free+0x23/0x140 [ 2744.552973] [] sk_free+0x19/0x20 [ 2744.552977] [] skb_complete_tx_timestamp+0x50/0x60 [ 2744.552988] [] fm10k_ts_tx_hwtstamp+0xd0/0x100 [fm10k] [ 2744.552994] [] fm10k_1588_msg_pf+0x12e/0x140 [fm10k] [ 2744.553002] [] fm10k_tlv_msg_parse+0x8d/0xc0 [fm10k] [ 2744.553010] [] fm10k_mbx_dequeue_rx+0x60/0xb0 [fm10k] [ 2744.553016] [] fm10k_sm_mbx_process+0x178/0x3c0 [fm10k] [ 2744.553022] [] fm10k_msix_mbx_pf+0xfa/0x360 [fm10k] [ 2744.553030] [] ? get_next_timer_interrupt+0x1f7/0x270 [ 2744.553036] [] handle_irq_event_percpu+0x77/0x1a0 [ 2744.553041] [] handle_irq_event+0x3b/0x60 [ 2744.553045] [] handle_edge_irq+0x6e/0x120 [ 2744.553054] [] handle_irq+0x74/0x140 [ 2744.553061] [] ? atomic_notifier_call_chain+0x1a/0x20 [ 2744.553066] [] do_IRQ+0x4f/0xf0 [ 2744.553072] [] common_interrupt+0x6d/0x6d [ 2744.553074] [] ? cpuidle_enter_state+0x66/0x160 [ 2744.553084] [] ? cpuidle_enter_state+0x51/0x160 [ 2744.553087] [] cpuidle_enter+0x17/0x20 [ 2744.553092] [] cpu_startup_entry+0x321/0x3c0 [ 2744.553098] [] rest_init+0x77/0x80 [ 2744.553103] [] start_kernel+0x4a4/0x4c5 [ 2744.553107] [] ? early_idt_handlers+0x120/0x120 [ 2744.553110] [] x86_64_start_reservations+0x2a/0x2c [ 2744.553114] [] x86_64_start_kernel+0x152/0x175 Signed-off-by: Jacob Keller Tested-by: Krishneil Singh Signed-off-by: Jeff Kirsher --- drivers/net/ethernet/intel/fm10k/fm10k_ptp.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_ptp.c b/drivers/net/ethernet/intel/fm10k/fm10k_ptp.c index 9043633c3e503..95f1d62b0690b 100644 --- a/drivers/net/ethernet/intel/fm10k/fm10k_ptp.c +++ b/drivers/net/ethernet/intel/fm10k/fm10k_ptp.c @@ -103,9 +103,10 @@ void fm10k_ts_tx_hwtstamp(struct fm10k_intfc *interface, __le16 dglort, if (!skb) return; - /* timestamp the sk_buff and return it to the socket */ + /* timestamp the sk_buff and free out copy */ fm10k_systime_to_hwtstamp(interface, &shhwtstamps, systime); - skb_complete_tx_timestamp(skb, &shhwtstamps); + skb_tstamp_tx(skb, &shhwtstamps); + dev_kfree_skb_any(skb); } void fm10k_ts_tx_subtask(struct fm10k_intfc *interface) From e075996ebd14dcd3d1a45172316141fe26d891fa Mon Sep 17 00:00:00 2001 From: Jacob Keller Date: Wed, 3 Jun 2015 16:31:00 -0700 Subject: [PATCH 04/17] fm10k: move setting shinfo inside ts_tx_enqueue This patch simplifies the code flow for setting the IN_PROGRESS bit of the shinfo for an skb we will be timestamping. Reported-by: Eric Dumazet Signed-off-by: Jacob Keller Tested-by: Krishneil Singh Signed-off-by: Jeff Kirsher --- drivers/net/ethernet/intel/fm10k/fm10k_ptp.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_ptp.c b/drivers/net/ethernet/intel/fm10k/fm10k_ptp.c index 95f1d62b0690b..39b832889b75f 100644 --- a/drivers/net/ethernet/intel/fm10k/fm10k_ptp.c +++ b/drivers/net/ethernet/intel/fm10k/fm10k_ptp.c @@ -70,16 +70,16 @@ void fm10k_ts_tx_enqueue(struct fm10k_intfc *interface, struct sk_buff *skb) * if none are present then insert skb in tail of list */ skb = fm10k_ts_tx_skb(interface, FM10K_CB(clone)->fi.w.dglort); - if (!skb) + if (!skb) { + skb_shinfo(clone)->tx_flags |= SKBTX_IN_PROGRESS; __skb_queue_tail(list, clone); + } spin_unlock_irqrestore(&list->lock, flags); /* if list is already has one then we just free the clone */ if (skb) kfree_skb(skb); - else - skb_shinfo(clone)->tx_flags |= SKBTX_IN_PROGRESS; } void fm10k_ts_tx_hwtstamp(struct fm10k_intfc *interface, __le16 dglort, From c23544b196e72716244108fb173f2965e9eafd1a Mon Sep 17 00:00:00 2001 From: Jacob Keller Date: Wed, 3 Jun 2015 16:31:01 -0700 Subject: [PATCH 05/17] fm10k: fix incorrect free on skb in ts_tx_enqueue This patch resolves a bug in the ts_tx_enqueue code responsible for a NULL pointer dereference and invalid access of the skb list. We incorrectly freed the actual skb we found instead of our copy. Thus the skb queue is essentially invalidated. Resolve this by freeing our clone in the cases where we did not add it to the queue. This also avoids the skb memory leak caused by failure to free the clone. [ 589.719320] BUG: unable to handle kernel NULL pointer dereference at (null) [ 589.722344] IP: [] fm10k_ts_tx_subtask+0xb0/0x160 [fm10k] [ 589.723796] PGD 0 [ 589.725228] Oops: 0000 [#1] SMP Signed-off-by: Jacob Keller Tested-by: Krishneil Singh Signed-off-by: Jeff Kirsher --- drivers/net/ethernet/intel/fm10k/fm10k_ptp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_ptp.c b/drivers/net/ethernet/intel/fm10k/fm10k_ptp.c index 39b832889b75f..b4945e8abe033 100644 --- a/drivers/net/ethernet/intel/fm10k/fm10k_ptp.c +++ b/drivers/net/ethernet/intel/fm10k/fm10k_ptp.c @@ -79,7 +79,7 @@ void fm10k_ts_tx_enqueue(struct fm10k_intfc *interface, struct sk_buff *skb) /* if list is already has one then we just free the clone */ if (skb) - kfree_skb(skb); + dev_kfree_skb(clone); } void fm10k_ts_tx_hwtstamp(struct fm10k_intfc *interface, __le16 dglort, From ec6acb801e7b2908c24a60c8aabf47c3e40508a4 Mon Sep 17 00:00:00 2001 From: Jacob Keller Date: Wed, 3 Jun 2015 16:31:02 -0700 Subject: [PATCH 06/17] fm10k: add call to fm10k_clean_all_rx_rings in fm10k_down This prevents a memory leak in fm10k_set_ringparams. The leak occurs because we go down, change ring parameters, and then come up. However, fm10k_down on its own is not clearing the Rx rings. Since fm10k_up assumes the rings are clean we basically drop the buffers and leak a bunch of memory. Eventually we hit dirty page faults and reboot the system. This issue does not occur elsewhere because other flows that involve fm10k_down go through fm10k_close which immediately called fm10k_free_all_rx_resources which properly cleans the rings. Signed-off-by: Jacob Keller Tested-by: Krishneil Singh Signed-off-by: Jeff Kirsher --- drivers/net/ethernet/intel/fm10k/fm10k_pci.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c index df9fda38bdd11..445014a49de71 100644 --- a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c +++ b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c @@ -1559,6 +1559,7 @@ void fm10k_down(struct fm10k_intfc *interface) /* free any buffers still on the rings */ fm10k_clean_all_tx_rings(interface); + fm10k_clean_all_rx_rings(interface); } /** From c0e58e93d722a53aa26b18110389c8131af9ddd9 Mon Sep 17 00:00:00 2001 From: Jacob Keller Date: Tue, 16 Jun 2015 13:39:11 -0700 Subject: [PATCH 07/17] fm10k: use an unsigned int for i in ethtool_get_strings The value will never be negative, and we use the %u print format. Thus, use unsigned int for the loop counter. Issue found using cppcheck. Signed-off-by: Jacob Keller Signed-off-by: Jeff Kirsher --- drivers/net/ethernet/intel/fm10k/fm10k_ethtool.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_ethtool.c b/drivers/net/ethernet/intel/fm10k/fm10k_ethtool.c index 4b9d9f88af706..06f0b08d9af57 100644 --- a/drivers/net/ethernet/intel/fm10k/fm10k_ethtool.c +++ b/drivers/net/ethernet/intel/fm10k/fm10k_ethtool.c @@ -124,7 +124,7 @@ static void fm10k_get_strings(struct net_device *dev, u32 stringset, u8 *data) { struct fm10k_intfc *interface = netdev_priv(dev); char *p = (char *)data; - int i; + unsigned int i; switch (stringset) { case ETH_SS_TEST: From f1f3322eb411ce875b8b9e2de59ee8d55fb3a33b Mon Sep 17 00:00:00 2001 From: Jacob Keller Date: Wed, 3 Jun 2015 16:31:04 -0700 Subject: [PATCH 08/17] fm10k: remove extraneous NULL check on l2_accel l2_accel was checked for NULL at the top of fm10k_dfwd_del_station, and we return if it is not defined. Due to this, we already know it can't be null here so a separate check is meaningless. Discovered via cppcheck. Signed-off-by: Jacob Keller Tested-by: Krishneil Singh Signed-off-by: Jeff Kirsher --- drivers/net/ethernet/intel/fm10k/fm10k_netdev.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c index 4c6b511115662..99228bf46c120 100644 --- a/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c +++ b/drivers/net/ethernet/intel/fm10k/fm10k_netdev.c @@ -1333,8 +1333,7 @@ static void fm10k_dfwd_del_station(struct net_device *dev, void *priv) dglort.rss_l = fls(interface->ring_feature[RING_F_RSS].mask); dglort.pc_l = fls(interface->ring_feature[RING_F_QOS].mask); dglort.glort = interface->glort; - if (l2_accel) - dglort.shared_l = fls(l2_accel->size); + dglort.shared_l = fls(l2_accel->size); hw->mac.ops.configure_dglort_map(hw, &dglort); /* If table is empty remove it */ From 0197cde62a0a39fdee606557b8cef6573a83e866 Mon Sep 17 00:00:00 2001 From: Jacob Keller Date: Tue, 16 Jun 2015 13:40:32 -0700 Subject: [PATCH 09/17] fm10k: trivial fixup message style to include a colon Also use %d for error values, since printing in hexadecimal is probably not helpful. Signed-off-by: Jacob Keller Signed-off-by: Jeff Kirsher --- drivers/net/ethernet/intel/fm10k/fm10k_pci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c index 445014a49de71..fe54c781d0bf0 100644 --- a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c +++ b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c @@ -1773,7 +1773,7 @@ static int fm10k_probe(struct pci_dev *pdev, fm10k_driver_name); if (err) { dev_err(&pdev->dev, - "pci_request_selected_regions failed 0x%x\n", err); + "pci_request_selected_regions failed: %d\n", err); goto err_pci_reg; } From c04ae58e2b6d836325914e7b4aa25f43da1be3df Mon Sep 17 00:00:00 2001 From: Jacob Keller Date: Tue, 16 Jun 2015 13:41:43 -0700 Subject: [PATCH 10/17] fm10k: use dma_set_mask_and_coherent in fm10k_probe This patch cleans up the use of dma_get_required_mask and uses the simpler dma_set_mask_and_coherent function instead of doing these as separate steps. I removed the dma_get_required_mask call because based on some minimal testing it appears that either (a) we're not doing the right thing with the call or (b) we don't need it anyways. If the value returned is <48bits, we'll end up trying with 48 bits anyways. If it's over 48bits, fm10k can't support that anyways, and we should try 48bits. If 48bits fails, we'll fallback to 32bits. This cleans up some very funky code. Signed-off-by: Jacob Keller Signed-off-by: Jeff Kirsher --- drivers/net/ethernet/intel/fm10k/fm10k_pci.c | 24 +++++--------------- 1 file changed, 6 insertions(+), 18 deletions(-) diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c index fe54c781d0bf0..ce53ff25f88d2 100644 --- a/drivers/net/ethernet/intel/fm10k/fm10k_pci.c +++ b/drivers/net/ethernet/intel/fm10k/fm10k_pci.c @@ -1741,30 +1741,18 @@ static int fm10k_probe(struct pci_dev *pdev, struct fm10k_intfc *interface; struct fm10k_hw *hw; int err; - u64 dma_mask; err = pci_enable_device_mem(pdev); if (err) return err; - /* By default fm10k only supports a 48 bit DMA mask */ - dma_mask = DMA_BIT_MASK(48) | dma_get_required_mask(&pdev->dev); - - if ((dma_mask <= DMA_BIT_MASK(32)) || - dma_set_mask_and_coherent(&pdev->dev, dma_mask)) { - dma_mask &= DMA_BIT_MASK(32); - + err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(48)); + if (err) err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32)); - err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(32)); - if (err) { - err = dma_set_coherent_mask(&pdev->dev, - DMA_BIT_MASK(32)); - if (err) { - dev_err(&pdev->dev, - "No usable DMA configuration, aborting\n"); - goto err_dma; - } - } + if (err) { + dev_err(&pdev->dev, + "DMA configuration failed: %d\n", err); + goto err_dma; } err = pci_request_selected_regions(pdev, From a38488f54004330071f4ec90c3cf52dc7646468e Mon Sep 17 00:00:00 2001 From: Jacob Keller Date: Wed, 3 Jun 2015 16:31:07 -0700 Subject: [PATCH 11/17] fm10k: force LPORT delete when updating VLAN or MAC address Currently, we don't notify the switch at all when the PF administratively sets a new VLAN or MAC address. This causes the old addresses to remain valid on the switch table. Since the PF is overriding any configuration done directly by the VF, we choose to simply re-create the LPORT for the VF. This does mean that all rules for the VF will be dropped when we set something directly via the PF, but it prevents some weird issues where the MAC/VLAN table retains some stale configuration. Signed-off-by: Jacob Keller Tested-by: Krishneil Singh Signed-off-by: Jeff Kirsher --- drivers/net/ethernet/intel/fm10k/fm10k_iov.c | 38 ++++++++++++-------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_iov.c b/drivers/net/ethernet/intel/fm10k/fm10k_iov.c index 5b08e6284a3ce..94571e6e790c5 100644 --- a/drivers/net/ethernet/intel/fm10k/fm10k_iov.c +++ b/drivers/net/ethernet/intel/fm10k/fm10k_iov.c @@ -400,11 +400,31 @@ int fm10k_iov_configure(struct pci_dev *pdev, int num_vfs) return num_vfs; } +static inline void fm10k_reset_vf_info(struct fm10k_intfc *interface, + struct fm10k_vf_info *vf_info) +{ + struct fm10k_hw *hw = &interface->hw; + + /* assigning the MAC address will send a mailbox message */ + fm10k_mbx_lock(interface); + + /* disable LPORT for this VF which clears switch rules */ + hw->iov.ops.reset_lport(hw, vf_info); + + /* assign new MAC+VLAN for this VF */ + hw->iov.ops.assign_default_mac_vlan(hw, vf_info); + + /* re-enable the LPORT for this VF */ + hw->iov.ops.set_lport(hw, vf_info, vf_info->vf_idx, + FM10K_VF_FLAG_MULTI_CAPABLE); + + fm10k_mbx_unlock(interface); +} + int fm10k_ndo_set_vf_mac(struct net_device *netdev, int vf_idx, u8 *mac) { struct fm10k_intfc *interface = netdev_priv(netdev); struct fm10k_iov_data *iov_data = interface->iov_data; - struct fm10k_hw *hw = &interface->hw; struct fm10k_vf_info *vf_info; /* verify SR-IOV is active and that vf idx is valid */ @@ -419,13 +439,7 @@ int fm10k_ndo_set_vf_mac(struct net_device *netdev, int vf_idx, u8 *mac) vf_info = &iov_data->vf_info[vf_idx]; ether_addr_copy(vf_info->mac, mac); - /* assigning the MAC will send a mailbox message so lock is needed */ - fm10k_mbx_lock(interface); - - /* assign MAC address to VF */ - hw->iov.ops.assign_default_mac_vlan(hw, vf_info); - - fm10k_mbx_unlock(interface); + fm10k_reset_vf_info(interface, vf_info); return 0; } @@ -455,16 +469,10 @@ int fm10k_ndo_set_vf_vlan(struct net_device *netdev, int vf_idx, u16 vid, /* record default VLAN ID for VF */ vf_info->pf_vid = vid; - /* assigning the VLAN will send a mailbox message so lock is needed */ - fm10k_mbx_lock(interface); - /* Clear the VLAN table for the VF */ hw->mac.ops.update_vlan(hw, FM10K_VLAN_ALL, vf_info->vsi, false); - /* Update VF assignment and trigger reset */ - hw->iov.ops.assign_default_mac_vlan(hw, vf_info); - - fm10k_mbx_unlock(interface); + fm10k_reset_vf_info(interface, vf_info); return 0; } From fba341d5cab38db68eed061cf20e161d967795c6 Mon Sep 17 00:00:00 2001 From: Jacob Keller Date: Wed, 3 Jun 2015 16:31:08 -0700 Subject: [PATCH 12/17] fm10k: re-map all possible VF queues after a VFLR During initialization, the VF counts its rings by walking the TQDLOC registers. This works only if the TQMAP/RQMAP registers are set to map all of the out-of-bound rings back to the first one. This allows the VF to cleanly detect when it has run out of queues. Update the PF code so that it resets the empty TQMAP/RQMAP registers post-VFLR to prevent innocent VF drivers from triggering malicious driver events. Signed-off-by: Matthew Vick Signed-off-by: Jacob Keller Tested-by: Krishneil Singh Signed-off-by: Jeff Kirsher --- drivers/net/ethernet/intel/fm10k/fm10k_pf.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_pf.c b/drivers/net/ethernet/intel/fm10k/fm10k_pf.c index 891e21874b2a1..49c8ad6476804 100644 --- a/drivers/net/ethernet/intel/fm10k/fm10k_pf.c +++ b/drivers/net/ethernet/intel/fm10k/fm10k_pf.c @@ -1046,6 +1046,12 @@ static s32 fm10k_iov_reset_resources_pf(struct fm10k_hw *hw, fm10k_write_reg(hw, FM10K_RQMAP(qmap_idx + i), vf_q_idx + i); } + /* repeat the first ring for all the remaining VF rings */ + for (i = queues_per_pool; i < qmap_stride; i++) { + fm10k_write_reg(hw, FM10K_TQMAP(qmap_idx + i), vf_q_idx); + fm10k_write_reg(hw, FM10K_RQMAP(qmap_idx + i), vf_q_idx); + } + return 0; } From 7fef39322ce7a0c1bbb5b48bef61b6c1aef73b96 Mon Sep 17 00:00:00 2001 From: Jacob Keller Date: Wed, 3 Jun 2015 16:31:09 -0700 Subject: [PATCH 13/17] fm10k: pack TLV overlay structures This patch adds the __attribute__((packed)) indicator to some structures which are overlayed onto a TLV message. These structures must be packed as small as possible in order to correctly align when copied into the mailbox buffer. Without doing so, the receiving mailbox code incorrectly parses the values and we get invalid message responses from the switch manager software. Signed-off-by: Jacob Keller Tested-by: Krishneil Singh Signed-off-by: Jeff Kirsher --- drivers/net/ethernet/intel/fm10k/fm10k_pf.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_pf.h b/drivers/net/ethernet/intel/fm10k/fm10k_pf.h index 7ab1db4fff320..40a0dbc62a04a 100644 --- a/drivers/net/ethernet/intel/fm10k/fm10k_pf.h +++ b/drivers/net/ethernet/intel/fm10k/fm10k_pf.h @@ -81,26 +81,26 @@ struct fm10k_mac_update { __le16 glort; u8 flags; u8 action; -}; +} __packed; struct fm10k_global_table_data { __le32 used; __le32 avail; -}; +} __packed; struct fm10k_swapi_error { __le32 status; struct fm10k_global_table_data mac; struct fm10k_global_table_data nexthop; struct fm10k_global_table_data ffu; -}; +} __packed; struct fm10k_swapi_1588_timestamp { __le64 egress; __le64 ingress; __le16 dglort; __le16 sglort; -}; +} __packed; s32 fm10k_msg_lport_map_pf(struct fm10k_hw *, u32 **, struct fm10k_mbx_info *); extern const struct fm10k_tlv_attr fm10k_lport_map_msg_attr[]; From 646725a7c9cbdefd8df4ae7b3217a992ad64a4cd Mon Sep 17 00:00:00 2001 From: Jacob Keller Date: Wed, 3 Jun 2015 16:31:10 -0700 Subject: [PATCH 14/17] fm10k: fix incorrect DIR_NEVATIVE bit in 1588 code The SYSTIME_CFG.Adjust Direction bit is actually supposed to indicate that the adjustment is positive. Fix the code to align correctly with hardware and documentation. Signed-off-by: Jacob Keller Tested-by: Krishneil Singh Signed-off-by: Jeff Kirsher --- drivers/net/ethernet/intel/fm10k/fm10k_pf.c | 4 ++-- drivers/net/ethernet/intel/fm10k/fm10k_type.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_pf.c b/drivers/net/ethernet/intel/fm10k/fm10k_pf.c index 49c8ad6476804..85162b7d331a3 100644 --- a/drivers/net/ethernet/intel/fm10k/fm10k_pf.c +++ b/drivers/net/ethernet/intel/fm10k/fm10k_pf.c @@ -1792,8 +1792,8 @@ static s32 fm10k_adjust_systime_pf(struct fm10k_hw *hw, s32 ppb) if (systime_adjust > FM10K_SW_SYSTIME_ADJUST_MASK) return FM10K_ERR_PARAM; - if (ppb < 0) - systime_adjust |= FM10K_SW_SYSTIME_ADJUST_DIR_NEGATIVE; + if (ppb > 0) + systime_adjust |= FM10K_SW_SYSTIME_ADJUST_DIR_POSITIVE; fm10k_write_sw_reg(hw, FM10K_SW_SYSTIME_ADJUST, (u32)systime_adjust); diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_type.h b/drivers/net/ethernet/intel/fm10k/fm10k_type.h index 4af96686c5840..2a17d82fa37d4 100644 --- a/drivers/net/ethernet/intel/fm10k/fm10k_type.h +++ b/drivers/net/ethernet/intel/fm10k/fm10k_type.h @@ -369,7 +369,7 @@ struct fm10k_hw; /* Registers contained in BAR 4 for Switch management */ #define FM10K_SW_SYSTIME_ADJUST 0x0224D #define FM10K_SW_SYSTIME_ADJUST_MASK 0x3FFFFFFF -#define FM10K_SW_SYSTIME_ADJUST_DIR_NEGATIVE 0x80000000 +#define FM10K_SW_SYSTIME_ADJUST_DIR_POSITIVE 0x80000000 #define FM10K_SW_SYSTIME_PULSE(_n) ((_n) + 0x02252) enum fm10k_int_source { From d18c438884137609bf838a703972126862db97f4 Mon Sep 17 00:00:00 2001 From: Jacob Keller Date: Wed, 3 Jun 2015 16:31:11 -0700 Subject: [PATCH 15/17] fm10k: remove err_no reference in fm10k_mbx.c The reference to err_no was left around after a previous code refactor. We never use the value, and it doesn't seem to be used in side a hidden macro reference. Discovered via cppcheck. Signed-off-by: Jacob Keller Tested-by: Krishneil Singh Signed-off-by: Jeff Kirsher --- drivers/net/ethernet/intel/fm10k/fm10k_mbx.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_mbx.c b/drivers/net/ethernet/intel/fm10k/fm10k_mbx.c index 1b27383805182..1a4b52637de91 100644 --- a/drivers/net/ethernet/intel/fm10k/fm10k_mbx.c +++ b/drivers/net/ethernet/intel/fm10k/fm10k_mbx.c @@ -1259,16 +1259,11 @@ static s32 fm10k_mbx_process_error(struct fm10k_hw *hw, struct fm10k_mbx_info *mbx) { const u32 *hdr = &mbx->mbx_hdr; - s32 err_no; u16 head; /* we will need to pull all of the fields for verification */ head = FM10K_MSG_HDR_FIELD_GET(*hdr, HEAD); - /* we only have lower 10 bits of error number so add upper bits */ - err_no = FM10K_MSG_HDR_FIELD_GET(*hdr, ERR_NO); - err_no |= ~FM10K_MSG_HDR_MASK(ERR_NO); - switch (mbx->state) { case FM10K_STATE_OPEN: case FM10K_STATE_DISCONNECT: From ee4373e7d74696821e47faf1b70f779697ddf77b Mon Sep 17 00:00:00 2001 From: Jacob Keller Date: Wed, 3 Jun 2015 16:31:12 -0700 Subject: [PATCH 16/17] fm10k: fix iov_msg_lport_state_pf issue When a VF issues an LPORT_STATE request to enable a port that is already enabled, the PF will first disable the VF LPORT. Then it should re-enable the VF again with the new requested settings. This ensures that any switch rules are cleared by deleting the LPORT on the switch. However, the flow is bugged because we actually check if the VF is enabled at the end, and thus don't re-enable it. Fix the flow so that we actually clear the enabled flags as part of our removal of the LPORT. Signed-off-by: Jacob Keller Tested-by: Krishneil Singh Signed-off-by: Jeff Kirsher --- drivers/net/ethernet/intel/fm10k/fm10k_pf.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_pf.c b/drivers/net/ethernet/intel/fm10k/fm10k_pf.c index 85162b7d331a3..3ca0233b3ea23 100644 --- a/drivers/net/ethernet/intel/fm10k/fm10k_pf.c +++ b/drivers/net/ethernet/intel/fm10k/fm10k_pf.c @@ -1351,6 +1351,14 @@ s32 fm10k_iov_msg_lport_state_pf(struct fm10k_hw *hw, u32 **results, err = fm10k_update_lport_state_pf(hw, vf_info->glort, 1, false); + /* we need to clear VF_FLAG_ENABLED flags in order to ensure + * that we actually re-enable the LPORT state below. Note that + * this has no impact if the VF is already disabled, as the + * flags are already cleared. + */ + if (!err) + vf_info->vf_flags = FM10K_VF_FLAG_CAPABLE(vf_info); + /* when enabling the port we should reset the rate limiters */ hw->iov.ops.configure_tc(hw, vf_info->vf_idx, vf_info->rate); From 986eec4341729549778f374dfc97e69a991302df Mon Sep 17 00:00:00 2001 From: Alexander Duyck Date: Wed, 3 Jun 2015 18:53:17 -0700 Subject: [PATCH 17/17] fm10k: Fix missing braces after if statement While reviewing the code I noticed that one of the commits added an if statement followed by a for loop, but the if statement was missing the braces around the loop. This change corrects the coding style error. Signed-off-by: Alexander Duyck Acked-by: Jacob Keller Tested-by: Krishneil Singh Signed-off-by: Jeff Kirsher --- drivers/net/ethernet/intel/fm10k/fm10k_ethtool.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_ethtool.c b/drivers/net/ethernet/intel/fm10k/fm10k_ethtool.c index 06f0b08d9af57..c6dc9683429e4 100644 --- a/drivers/net/ethernet/intel/fm10k/fm10k_ethtool.c +++ b/drivers/net/ethernet/intel/fm10k/fm10k_ethtool.c @@ -143,12 +143,13 @@ static void fm10k_get_strings(struct net_device *dev, u32 stringset, u8 *data) p += ETH_GSTRING_LEN; } - if (interface->hw.mac.type != fm10k_mac_vf) + if (interface->hw.mac.type != fm10k_mac_vf) { for (i = 0; i < FM10K_PF_STATS_LEN; i++) { memcpy(p, fm10k_gstrings_pf_stats[i].stat_string, ETH_GSTRING_LEN); p += ETH_GSTRING_LEN; } + } for (i = 0; i < interface->hw.mac.max_queues; i++) { sprintf(p, "tx_queue_%u_packets", i);