* [net-next PATCH 2/5] igb: remove IGB_DESC_UNUSED since it is better handled by a function call
2009-03-20 10:16 [net-next PATCH 1/5] igb: allow tx of pre-formatted vlan tagged packets Jeff Kirsher
@ 2009-03-20 10:16 ` Jeff Kirsher
2009-03-21 23:57 ` David Miller
2009-03-20 10:17 ` [net-next PATCH 3/5] igb: update driver to use setup_timer function Jeff Kirsher
` (4 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Jeff Kirsher @ 2009-03-20 10:16 UTC (permalink / raw)
To: davem; +Cc: netdev, gospo, Alexander Duyck, Jeff Kirsher
From: Alexander Duyck <alexander.h.duyck@intel.com>
This patch removes IGB_DESC_UNUSED and replaces it with a function call
instead in order to cleanup some of the ugliness introduced by the macro.
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/igb/igb.h | 4 ----
drivers/net/igb/igb_main.c | 25 ++++++++++++++++++-------
2 files changed, 18 insertions(+), 11 deletions(-)
diff --git a/drivers/net/igb/igb.h b/drivers/net/igb/igb.h
index e18ac1b..4e8464b 100644
--- a/drivers/net/igb/igb.h
+++ b/drivers/net/igb/igb.h
@@ -182,10 +182,6 @@ struct igb_ring {
char name[IFNAMSIZ + 5];
};
-#define IGB_DESC_UNUSED(R) \
- ((((R)->next_to_clean > (R)->next_to_use) ? 0 : (R)->count) + \
- (R)->next_to_clean - (R)->next_to_use - 1)
-
#define E1000_RX_DESC_ADV(R, i) \
(&(((union e1000_adv_rx_desc *)((R).desc))[i]))
#define E1000_TX_DESC_ADV(R, i) \
diff --git a/drivers/net/igb/igb_main.c b/drivers/net/igb/igb_main.c
index c14d569..05bb7a7 100644
--- a/drivers/net/igb/igb_main.c
+++ b/drivers/net/igb/igb_main.c
@@ -278,6 +278,17 @@ static char *igb_get_time_str(struct igb_adapter *adapter,
#endif
/**
+ * igb_desc_unused - calculate if we have unused descriptors
+ **/
+static int igb_desc_unused(struct igb_ring *ring)
+{
+ if (ring->next_to_clean > ring->next_to_use)
+ return ring->next_to_clean - ring->next_to_use - 1;
+
+ return ring->count + ring->next_to_clean - ring->next_to_use - 1;
+}
+
+/**
* igb_init_module - Driver Registration Routine
*
* igb_init_module is the first routine called when the driver is
@@ -873,12 +884,12 @@ static void igb_configure(struct igb_adapter *adapter)
igb_rx_fifo_flush_82575(&adapter->hw);
- /* call IGB_DESC_UNUSED which always leaves
+ /* call igb_desc_unused which always leaves
* at least 1 descriptor unused to make sure
* next_to_use != next_to_clean */
for (i = 0; i < adapter->num_rx_queues; i++) {
struct igb_ring *ring = &adapter->rx_ring[i];
- igb_alloc_rx_buffers_adv(ring, IGB_DESC_UNUSED(ring));
+ igb_alloc_rx_buffers_adv(ring, igb_desc_unused(ring));
}
@@ -2661,7 +2672,7 @@ link_up:
igb_update_adaptive(&adapter->hw);
if (!netif_carrier_ok(netdev)) {
- if (IGB_DESC_UNUSED(tx_ring) + 1 < tx_ring->count) {
+ if (igb_desc_unused(tx_ring) + 1 < tx_ring->count) {
/* We've lost link, so the controller stops DMA,
* but we've got queued Tx work that's never going
* to get done, so reset controller to flush Tx.
@@ -3199,7 +3210,7 @@ static int __igb_maybe_stop_tx(struct net_device *netdev,
/* We need to check again in a case another CPU has just
* made room available. */
- if (IGB_DESC_UNUSED(tx_ring) < size)
+ if (igb_desc_unused(tx_ring) < size)
return -EBUSY;
/* A reprieve! */
@@ -3211,7 +3222,7 @@ static int __igb_maybe_stop_tx(struct net_device *netdev,
static int igb_maybe_stop_tx(struct net_device *netdev,
struct igb_ring *tx_ring, int size)
{
- if (IGB_DESC_UNUSED(tx_ring) >= size)
+ if (igb_desc_unused(tx_ring) >= size)
return 0;
return __igb_maybe_stop_tx(netdev, tx_ring, size);
}
@@ -4310,7 +4321,7 @@ static bool igb_clean_tx_irq(struct igb_ring *tx_ring)
if (unlikely(count &&
netif_carrier_ok(netdev) &&
- IGB_DESC_UNUSED(tx_ring) >= IGB_TX_QUEUE_WAKE)) {
+ igb_desc_unused(tx_ring) >= IGB_TX_QUEUE_WAKE)) {
/* Make sure that anybody stopping the queue after this
* sees the new next_to_clean.
*/
@@ -4587,7 +4598,7 @@ next_desc:
}
rx_ring->next_to_clean = i;
- cleaned_count = IGB_DESC_UNUSED(rx_ring);
+ cleaned_count = igb_desc_unused(rx_ring);
if (cleaned_count)
igb_alloc_rx_buffers_adv(rx_ring, cleaned_count);
^ permalink raw reply related [flat|nested] 12+ messages in thread* [net-next PATCH 3/5] igb: update driver to use setup_timer function
2009-03-20 10:16 [net-next PATCH 1/5] igb: allow tx of pre-formatted vlan tagged packets Jeff Kirsher
2009-03-20 10:16 ` [net-next PATCH 2/5] igb: remove IGB_DESC_UNUSED since it is better handled by a function call Jeff Kirsher
@ 2009-03-20 10:17 ` Jeff Kirsher
2009-03-21 23:57 ` David Miller
2009-03-20 10:17 ` [net-next PATCH 4/5] igb: rework igb_set_multi so that vfs are properly updated Jeff Kirsher
` (3 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Jeff Kirsher @ 2009-03-20 10:17 UTC (permalink / raw)
To: davem; +Cc: netdev, gospo, Alexander Duyck, Jeff Kirsher
From: Alexander Duyck <alexander.h.duyck@intel.com>
igb was previously setting up all of the timer members itself. It is
easier to just call setup_timer and reduce the calls to one line.
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/igb/igb_main.c | 11 ++++-------
1 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/net/igb/igb_main.c b/drivers/net/igb/igb_main.c
index 05bb7a7..3dd3a5a 100644
--- a/drivers/net/igb/igb_main.c
+++ b/drivers/net/igb/igb_main.c
@@ -1312,13 +1312,10 @@ static int __devinit igb_probe(struct pci_dev *pdev,
goto err_eeprom;
}
- init_timer(&adapter->watchdog_timer);
- adapter->watchdog_timer.function = &igb_watchdog;
- adapter->watchdog_timer.data = (unsigned long) adapter;
-
- init_timer(&adapter->phy_info_timer);
- adapter->phy_info_timer.function = &igb_update_phy_info;
- adapter->phy_info_timer.data = (unsigned long) adapter;
+ setup_timer(&adapter->watchdog_timer, &igb_watchdog,
+ (unsigned long) adapter);
+ setup_timer(&adapter->phy_info_timer, &igb_update_phy_info,
+ (unsigned long) adapter);
INIT_WORK(&adapter->reset_task, igb_reset_task);
INIT_WORK(&adapter->watchdog_task, igb_watchdog_task);
^ permalink raw reply related [flat|nested] 12+ messages in thread* [net-next PATCH 4/5] igb: rework igb_set_multi so that vfs are properly updated
2009-03-20 10:16 [net-next PATCH 1/5] igb: allow tx of pre-formatted vlan tagged packets Jeff Kirsher
2009-03-20 10:16 ` [net-next PATCH 2/5] igb: remove IGB_DESC_UNUSED since it is better handled by a function call Jeff Kirsher
2009-03-20 10:17 ` [net-next PATCH 3/5] igb: update driver to use setup_timer function Jeff Kirsher
@ 2009-03-20 10:17 ` Jeff Kirsher
2009-03-21 23:57 ` David Miller
2009-03-20 10:17 ` [net-next PATCH 5/5] igb: cleanup tx dma so map & unmap use matching calls Jeff Kirsher
` (2 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Jeff Kirsher @ 2009-03-20 10:17 UTC (permalink / raw)
To: davem; +Cc: netdev, gospo, Alexander Duyck, Jeff Kirsher
From: Alexander Duyck <alexander.h.duyck@intel.com>
Currently if there are no multicast addresses programmed into the PF then
the VFs cannot have their multicast filters reset. This change makes it so
the code path that updates vf multicast is always called along with the pf
updates.
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/igb/igb_main.c | 18 ++++++++----------
1 files changed, 8 insertions(+), 10 deletions(-)
diff --git a/drivers/net/igb/igb_main.c b/drivers/net/igb/igb_main.c
index 3dd3a5a..a977bd7 100644
--- a/drivers/net/igb/igb_main.c
+++ b/drivers/net/igb/igb_main.c
@@ -2464,7 +2464,7 @@ static void igb_set_multi(struct net_device *netdev)
struct e1000_hw *hw = &adapter->hw;
struct e1000_mac_info *mac = &hw->mac;
struct dev_mc_list *mc_ptr;
- u8 *mta_list;
+ u8 *mta_list = NULL;
u32 rctl;
int i;
@@ -2485,17 +2485,15 @@ static void igb_set_multi(struct net_device *netdev)
}
wr32(E1000_RCTL, rctl);
- if (!netdev->mc_count) {
- /* nothing to program, so clear mc list */
- igb_update_mc_addr_list(hw, NULL, 0, 1,
- mac->rar_entry_count);
- return;
+ if (netdev->mc_count) {
+ mta_list = kzalloc(netdev->mc_count * 6, GFP_ATOMIC);
+ if (!mta_list) {
+ dev_err(&adapter->pdev->dev,
+ "failed to allocate multicast filter list\n");
+ return;
+ }
}
- mta_list = kzalloc(netdev->mc_count * 6, GFP_ATOMIC);
- if (!mta_list)
- return;
-
/* The shared function expects a packed array of only addresses. */
mc_ptr = netdev->mc_list;
^ permalink raw reply related [flat|nested] 12+ messages in thread* [net-next PATCH 5/5] igb: cleanup tx dma so map & unmap use matching calls
2009-03-20 10:16 [net-next PATCH 1/5] igb: allow tx of pre-formatted vlan tagged packets Jeff Kirsher
` (2 preceding siblings ...)
2009-03-20 10:17 ` [net-next PATCH 4/5] igb: rework igb_set_multi so that vfs are properly updated Jeff Kirsher
@ 2009-03-20 10:17 ` Jeff Kirsher
2009-03-21 23:57 ` David Miller
2009-03-20 15:27 ` [net-next PATCH 1/5] igb: allow tx of pre-formatted vlan tagged packets Arthur Jones
2009-03-21 23:55 ` David Miller
5 siblings, 1 reply; 12+ messages in thread
From: Jeff Kirsher @ 2009-03-20 10:17 UTC (permalink / raw)
To: davem; +Cc: netdev, gospo, Alexander Duyck, Jeff Kirsher
From: Alexander Duyck <alexander.h.duyck@intel.com>
The igb driver was using map_single to map the skbs and then unmap_page to
unmap them. This update changes that so instead uses skb_dma_map and
skb_dma_unmap.
In addition the next_to_watch member of the buffer_info struct was being
set uneccesarily. I removed the spots where it was set without being needed.
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
drivers/net/igb/igb_main.c | 66 +++++++++++++++++++++++---------------------
1 files changed, 35 insertions(+), 31 deletions(-)
diff --git a/drivers/net/igb/igb_main.c b/drivers/net/igb/igb_main.c
index a977bd7..d6f7af8 100644
--- a/drivers/net/igb/igb_main.c
+++ b/drivers/net/igb/igb_main.c
@@ -2257,19 +2257,14 @@ static void igb_free_all_tx_resources(struct igb_adapter *adapter)
static void igb_unmap_and_free_tx_resource(struct igb_adapter *adapter,
struct igb_buffer *buffer_info)
{
- if (buffer_info->dma) {
- pci_unmap_page(adapter->pdev,
- buffer_info->dma,
- buffer_info->length,
- PCI_DMA_TODEVICE);
- buffer_info->dma = 0;
- }
+ buffer_info->dma = 0;
if (buffer_info->skb) {
+ skb_dma_unmap(&adapter->pdev->dev, buffer_info->skb,
+ DMA_TO_DEVICE);
dev_kfree_skb_any(buffer_info->skb);
buffer_info->skb = NULL;
}
buffer_info->time_stamp = 0;
- buffer_info->next_to_watch = 0;
/* buffer_info must be completely set up in the transmit path */
}
@@ -3078,25 +3073,33 @@ static inline int igb_tx_map_adv(struct igb_adapter *adapter,
unsigned int len = skb_headlen(skb);
unsigned int count = 0, i;
unsigned int f;
+ dma_addr_t *map;
i = tx_ring->next_to_use;
+ if (skb_dma_map(&adapter->pdev->dev, skb, DMA_TO_DEVICE)) {
+ dev_err(&adapter->pdev->dev, "TX DMA map failed\n");
+ return 0;
+ }
+
+ map = skb_shinfo(skb)->dma_maps;
+
buffer_info = &tx_ring->buffer_info[i];
BUG_ON(len >= IGB_MAX_DATA_PER_TXD);
buffer_info->length = len;
/* set time_stamp *before* dma to help avoid a possible race */
buffer_info->time_stamp = jiffies;
buffer_info->next_to_watch = i;
- buffer_info->dma = pci_map_single(adapter->pdev, skb->data, len,
- PCI_DMA_TODEVICE);
+ buffer_info->dma = map[count];
count++;
- i++;
- if (i == tx_ring->count)
- i = 0;
for (f = 0; f < skb_shinfo(skb)->nr_frags; f++) {
struct skb_frag_struct *frag;
+ i++;
+ if (i == tx_ring->count)
+ i = 0;
+
frag = &skb_shinfo(skb)->frags[f];
len = frag->size;
@@ -3105,19 +3108,10 @@ static inline int igb_tx_map_adv(struct igb_adapter *adapter,
buffer_info->length = len;
buffer_info->time_stamp = jiffies;
buffer_info->next_to_watch = i;
- buffer_info->dma = pci_map_page(adapter->pdev,
- frag->page,
- frag->page_offset,
- len,
- PCI_DMA_TODEVICE);
-
+ buffer_info->dma = map[count];
count++;
- i++;
- if (i == tx_ring->count)
- i = 0;
}
- i = ((i == 0) ? tx_ring->count - 1 : i - 1);
tx_ring->buffer_info[i].skb = skb;
tx_ring->buffer_info[first].next_to_watch = i;
@@ -3230,6 +3224,7 @@ static int igb_xmit_frame_ring_adv(struct sk_buff *skb,
unsigned int first;
unsigned int tx_flags = 0;
u8 hdr_len = 0;
+ int count = 0;
int tso = 0;
union skb_shared_tx *shtx;
@@ -3291,14 +3286,23 @@ static int igb_xmit_frame_ring_adv(struct sk_buff *skb,
(skb->ip_summed == CHECKSUM_PARTIAL))
tx_flags |= IGB_TX_FLAGS_CSUM;
- igb_tx_queue_adv(adapter, tx_ring, tx_flags,
- igb_tx_map_adv(adapter, tx_ring, skb, first),
- skb->len, hdr_len);
-
- netdev->trans_start = jiffies;
-
- /* Make sure there is space in the ring for the next send. */
- igb_maybe_stop_tx(netdev, tx_ring, MAX_SKB_FRAGS + 4);
+ /*
+ * count reflects descriptors mapped, if 0 then mapping error
+ * has occured and we need to rewind the descriptor queue
+ */
+ count = igb_tx_map_adv(adapter, tx_ring, skb, first);
+
+ if (count) {
+ igb_tx_queue_adv(adapter, tx_ring, tx_flags, count,
+ skb->len, hdr_len);
+ netdev->trans_start = jiffies;
+ /* Make sure there is space in the ring for the next send. */
+ igb_maybe_stop_tx(netdev, tx_ring, MAX_SKB_FRAGS + 4);
+ } else {
+ dev_kfree_skb_any(skb);
+ tx_ring->buffer_info[first].time_stamp = 0;
+ tx_ring->next_to_use = first;
+ }
return NETDEV_TX_OK;
}
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [net-next PATCH 5/5] igb: cleanup tx dma so map & unmap use matching calls
2009-03-20 10:17 ` [net-next PATCH 5/5] igb: cleanup tx dma so map & unmap use matching calls Jeff Kirsher
@ 2009-03-21 23:57 ` David Miller
0 siblings, 0 replies; 12+ messages in thread
From: David Miller @ 2009-03-21 23:57 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: netdev, gospo, alexander.h.duyck
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Fri, 20 Mar 2009 03:17:43 -0700
> The igb driver was using map_single to map the skbs and then unmap_page to
> unmap them. This update changes that so instead uses skb_dma_map and
> skb_dma_unmap.
>
> In addition the next_to_watch member of the buffer_info struct was being
> set uneccesarily. I removed the spots where it was set without being needed.
>
> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Applied.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [net-next PATCH 1/5] igb: allow tx of pre-formatted vlan tagged packets
2009-03-20 10:16 [net-next PATCH 1/5] igb: allow tx of pre-formatted vlan tagged packets Jeff Kirsher
` (3 preceding siblings ...)
2009-03-20 10:17 ` [net-next PATCH 5/5] igb: cleanup tx dma so map & unmap use matching calls Jeff Kirsher
@ 2009-03-20 15:27 ` Arthur Jones
2009-03-20 21:28 ` Jeff Kirsher
2009-03-21 23:55 ` David Miller
5 siblings, 1 reply; 12+ messages in thread
From: Arthur Jones @ 2009-03-20 15:27 UTC (permalink / raw)
To: Jeff Kirsher
Cc: davem@davemloft.net, netdev@vger.kernel.org, gospo@redhat.com,
Arthur Jones
Hi Jeff, ...
On Fri, Mar 20, 2009 at 03:16:31AM -0700, Jeff Kirsher wrote:
> [...]
>
> drivers/net/igb/igb_main.c | 11 +++++++++++
> 1 files changed, 11 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/igb/igb_main.c b/drivers/net/igb/igb_main.c
> index 7c4481b..c14d569 100644
> --- a/drivers/net/igb/igb_main.c
> +++ b/drivers/net/igb/igb_main.c
> @@ -3008,6 +3008,17 @@ static inline bool igb_tx_csum_adv(struct igb_adapter *adapter,
> tu_cmd |= (E1000_TXD_CMD_DEXT | E1000_ADVTXD_DTYP_CTXT);
>
> if (skb->ip_summed == CHECKSUM_PARTIAL) {
> + __be16 protocol;
> +
> + if (skb->protocol == cpu_to_be16(ETH_P_8021Q)) {
> + const struct vlan_ethhdr *vhdr =
> + (const struct vlan_ethhdr*)skb->data;
> +
> + protocol = vhdr->h_vlan_encapsulated_proto;
> + } else {
> + protocol = skb->protocol;
> + }
> +
> switch (skb->protocol) {
^^^^^^^^^^^^^
This has the same merge error as the e1000e version,
it should be just protocol here, not skb->protocol...
Arthur
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [net-next PATCH 1/5] igb: allow tx of pre-formatted vlan tagged packets
2009-03-20 15:27 ` [net-next PATCH 1/5] igb: allow tx of pre-formatted vlan tagged packets Arthur Jones
@ 2009-03-20 21:28 ` Jeff Kirsher
0 siblings, 0 replies; 12+ messages in thread
From: Jeff Kirsher @ 2009-03-20 21:28 UTC (permalink / raw)
To: Arthur Jones
Cc: davem@davemloft.net, netdev@vger.kernel.org, gospo@redhat.com,
Arthur Jones
On Fri, Mar 20, 2009 at 8:27 AM, Arthur Jones <ajones@riverbed.com> wrote:
> Hi Jeff, ...
>
> On Fri, Mar 20, 2009 at 03:16:31AM -0700, Jeff Kirsher wrote:
>> [...]
>>
>> drivers/net/igb/igb_main.c | 11 +++++++++++
>> 1 files changed, 11 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/net/igb/igb_main.c b/drivers/net/igb/igb_main.c
>> index 7c4481b..c14d569 100644
>> --- a/drivers/net/igb/igb_main.c
>> +++ b/drivers/net/igb/igb_main.c
>> @@ -3008,6 +3008,17 @@ static inline bool igb_tx_csum_adv(struct igb_adapter *adapter,
>> tu_cmd |= (E1000_TXD_CMD_DEXT | E1000_ADVTXD_DTYP_CTXT);
>>
>> if (skb->ip_summed == CHECKSUM_PARTIAL) {
>> + __be16 protocol;
>> +
>> + if (skb->protocol == cpu_to_be16(ETH_P_8021Q)) {
>> + const struct vlan_ethhdr *vhdr =
>> + (const struct vlan_ethhdr*)skb->data;
>> +
>> + protocol = vhdr->h_vlan_encapsulated_proto;
>> + } else {
>> + protocol = skb->protocol;
>> + }
>> +
>> switch (skb->protocol) {
> ^^^^^^^^^^^^^
> This has the same merge error as the e1000e version,
> it should be just protocol here, not skb->protocol...
>
> Arthur
> --
Sorry, when I merged your patch into my tree, that is when the merge
error occurred.
--
Cheers,
Jeff
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [net-next PATCH 1/5] igb: allow tx of pre-formatted vlan tagged packets
2009-03-20 10:16 [net-next PATCH 1/5] igb: allow tx of pre-formatted vlan tagged packets Jeff Kirsher
` (4 preceding siblings ...)
2009-03-20 15:27 ` [net-next PATCH 1/5] igb: allow tx of pre-formatted vlan tagged packets Arthur Jones
@ 2009-03-21 23:55 ` David Miller
5 siblings, 0 replies; 12+ messages in thread
From: David Miller @ 2009-03-21 23:55 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: netdev, gospo, ajones
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Fri, 20 Mar 2009 03:16:31 -0700
> When the 82575 is fed 802.1q packets, it chokes with
> an error of the form:
>
> igb 0000:08:00.1 partial checksum but proto=81!
...
> Signed-off-by: Arthur Jones <ajones@riverbed.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> +
> switch (skb->protocol) {
> case cpu_to_be16(ETH_P_IP):
> tu_cmd |= E1000_ADVTXD_TUCMD_IPV4;
Applied with skb->protocol fixed to be just plain 'protocol'.
^ permalink raw reply [flat|nested] 12+ messages in thread