igb: Add support for writable pages and build_skb
Subsystems (2)
cover igb: Add support for writable pages and build_skb
This patch set enables support for using the recent changes that allow for unmapping pages without invalidating their contents via DMA_ATTR_SKIP_CPU_SYNC. With this change DMA pages should be writable and as a result we should be able to make use of build_skb which can be used to drop the skb->head memory allocation, header parsing, and memcpy from the receive path which can greatly help to improve performance.
My main concern at this point is that there might be an architecture where I didn't get DMA_ATTR_SKIP_CPU_SYNC implemented that might still need it. For that reason I have also added a ethtool private flag called out as "legacy-rx". If a platform encounters an issue where the Rx can possibly corrupt data it can be enbled by running:
ethtool --set-priv-flags DEVNAME legacy-rx on
The testing matrix for all of these patches is going to be pretty extensive. Basically we want to test these patches on as many platforms and architectures as possible with as many features being toggled as possible including SR-IOV and Jumbo Frames all while receiving traffic.
Within the patches there is also some intialization changes. Specifically I have updated the code paths to defer clearing the rings until we are about to initialize them and give them to hardware. By doing this we are able to avoid having to dirty memory we don't need to which should help to improve suspend/resume times for when we start looking at possibly using the suspend/resume approach for migration of interface in VMs.
Subject: [next PATCH 0/9] Series short description
---
Alexander Duyck (9):
igb: Add support for DMA_ATTR_WEAK_ORDERING
igb: Use length to determine if descriptor is done
igb: Clear Rx buffer_info in configure instead of clean
igb: Don't bother clearing Tx buffer_info in igb_clean_tx_ring
igb: Limit maximum frame Rx based on MTU
igb: Add support for padding packet
igb: Add support for ethtool private flag to allow use of legacy Rx
igb: Break out Rx buffer page management
igb: Revert "igb: Revert support for build_skb in igb"
drivers/net/ethernet/intel/igb/igb.h | 56 +++ drivers/net/ethernet/intel/igb/igb_ethtool.c | 63 +++ drivers/net/ethernet/intel/igb/igb_main.c | 510 ++++++++++++++++---------- 3 files changed, 431 insertions(+), 198 deletions(-)
--
PATCH 1/9 igb: Add support for DMA_ATTR_WEAK_ORDERING
From: Alexander Duyck <[email protected]>
Since we are already using DMA attributes in igb for Rx there is no reason why we can't also apply DMA_ATTR_WEAK_ORDERING which is needed on some platforms to improve performance.
--- drivers/net/ethernet/intel/igb/igb.h | 3 +++ drivers/net/ethernet/intel/igb/igb_main.c | 6 +++--- 2 files changed, 6 insertions(+), 3 deletions(-)
drivers/net/ethernet/intel/igb/igb.h +3 -0
@@ -148,6 +148,9 @@ struct vf_data_storage {/* How many Rx Buffers do we bundle into one write to the hardware ? */ #define IGB_RX_BUFFER_WRITE 16/* Must be power of 2 */ +#define IGB_RX_DMA_ATTR \ + (DMA_ATTR_SKIP_CPU_SYNC | DMA_ATTR_WEAK_ORDERING) +#define AUTO_ALL_MODES 0#define IGB_EEPROM_APME 0x0400
drivers/net/ethernet/intel/igb/igb_main.c +3 -3
@@ -3963,7 +3963,7 @@ static void igb_clean_rx_ring(struct igb_ring *rx_ring) buffer_info->dma,PAGE_SIZE ,DMA_FROM_DEVICE , - DMA_ATTR_SKIP_CPU_SYNC); + IGB_RX_DMA_ATTR);__page_frag_cache_drain (buffer_info->page, buffer_info->pagecnt_bias);
@@ -6992,7 +6992,7 @@ static struct sk_buff *igb_fetch_rx_buffer(struct igb_ring *rx_ring,*/ dma_unmap_page_attrs (rx_ring->dev, rx_buffer->dma,PAGE_SIZE ,DMA_FROM_DEVICE , -DMA_ATTR_SKIP_CPU_SYNC ); +IGB_RX_DMA_ATTR );__page_frag_cache_drain (page, rx_buffer->pagecnt_bias); }
@@ -7252,7 +7252,7 @@ static bool igb_alloc_mapped_page(struct igb_ring *rx_ring,/* map page for use */ dma =dma_map_page_attrs (rx_ring->dev, page,0 ,PAGE_SIZE , -DMA_FROM_DEVICE ,DMA_ATTR_SKIP_CPU_SYNC ); +DMA_FROM_DEVICE ,IGB_RX_DMA_ATTR );/* if mapping failed free memory back to system since * there isn't much point in holding memory we can't use
PATCH 2/9 igb: Use length to determine if descriptor is done
In reply to Alexander DuyckFrom: Alexander Duyck <[email protected]>
This change makes it so that we use the length of the packet instead of the DD status bit to determine if a new descriptor is ready to be processed. The obvious advantage is that it cuts down on reads as we don't really even need the DD bit if going from a 0 to a non-zero value on size is enough to inform us that the packet has been completed.
In addition I have updated the code so that we only reset the Rx descriptor length for descriptor zero when resetting a ring instead of having to do a memset with 0 over the entire ring. By doing this we can save some time on initialization.
--- drivers/net/ethernet/intel/igb/igb_main.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)
drivers/net/ethernet/intel/igb/igb_main.c +8 -6
@@ -3720,6 +3720,7 @@ void igb_configure_rx_ring(struct igb_adapter *adapter, struct igb_ring *ring) {struct e1000_hw *hw = &adapter->hw; +union e1000_adv_rx_desc *rx_desc;u64 rdba = ring->dma;int reg_idx = ring->reg_idx;u32 srrctl =0 , rxdctl =0 ;
@@ -3758,6 +3759,10 @@ void igb_configure_rx_ring(struct igb_adapter *adapter, rxdctl |=IGB_RX_HTHRESH <<8 ; rxdctl |=IGB_RX_WTHRESH <<16 ; +/* initialize Rx descriptor 0 */ + rx_desc =IGB_RX_DESC (ring,0 ); + rx_desc->wb.upper.length =0 ; +/* enable receive descriptor fetching */ rxdctl |=E1000_RXDCTL_QUEUE_ENABLE ;wr32 (E1000_RXDCTL (reg_idx), rxdctl);
@@ -3973,9 +3978,6 @@ static void igb_clean_rx_ring(struct igb_ring *rx_ring) size =sizeof (struct igb_rx_buffer ) * rx_ring->count;memset (rx_ring->rx_buffer_info,0 , size); -/* Zero out the descriptor ring */ -memset (rx_ring->desc,0 , rx_ring->size); - rx_ring->next_to_alloc =0 ; rx_ring->next_to_clean =0 ; rx_ring->next_to_use =0 ;
@@ -7174,7 +7176,7 @@ static int igb_clean_rx_irq(struct igb_q_vector *q_vector, const int budget) rx_desc =IGB_RX_DESC (rx_ring, rx_ring->next_to_clean); -if (!rx_desc->wb.upper.status_error) +if (!rx_desc->wb.upper.length)break ;/* This memory barrier is needed to keep us from reading
@@ -7314,8 +7316,8 @@ void igb_alloc_rx_buffers(struct igb_ring *rx_ring, u16 cleaned_count) i -= rx_ring->count; } -/* clear the status bits for the next_to_use descriptor */ - rx_desc->wb.upper.status_error =0 ; +/* clear the length for the next_to_use descriptor */ + rx_desc->wb.upper.length =0 ; cleaned_count--; }while (cleaned_count);
Brown
PATCH 2/9 igb: Use length to determine if descriptor is done
Quoted 21 lines
From: Intel-wired-lan [mailto:intel-wired-lan-bounces at lists.osuosl.org] On Behalf Of Alexander Duyck
Sent: Friday, January 6, 2017 8:11 AM
To: intel-wired-lan at lists.osuosl.org; Kirsher, Jeffrey T <[email protected]>
Subject: [Intel-wired-lan] [next PATCH 2/9] igb: Use length to determine if descriptor is doneFrom: Alexander Duyck <[email protected]>
This change makes it so that we use the length of the packet instead of the DD status bit to determine if a new descriptor is ready to be processed. The obvious advantage is that it cuts down on reads as we don't really even need the DD bit if going from a 0 to a non-zero value on size is enough to inform us that the packet has been completed.
In addition I have updated the code so that we only reset the Rx descriptor length for descriptor zero when resetting a ring instead of having to do a memset with 0 over the entire ring. By doing this we can save some time on initialization.
Quoted 5 lines
Signed-off-by Alexander Duyck <[email protected]>--- drivers/net/ethernet/intel/igb/igb_main.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)
This seems to be causing ethtool diagnostics to fail on some adapters.
Hangs with a trace on complete set of patches on an ethtool diags (default ethtool -t) with our without the interface configured. On one system the console recovered after the trace and I was able to pull the following trace from dmesg. On that same system with just 1/9 and 2/9 of this series applied the same system simply hung on an ethtool -t (unresponsive system on any consoles) and never came back. Could be coincidence, other systems hung and never came back with the whole series applied as well.
With just 1/9 applied the system passes ethtool diagnostics without problems.
Here is dmesg output for driver load followed by an ethtool -t ethX. This is from an i350, but I am seeing similar results on pretty much all adapters I've tried.
--------------------------------------------------------------------------
igb: Copyright (c) 2007-2014 Intel Corporation.
igb 0000:08:00.0: added PHC on eth0
igb 0000:08:00.0: Intel(R) Gigabit Ethernet Network Connection
igb 0000:08:00.0: eth0: (PCIe:2.5Gb/s:Width x4) a0:36:9f:00:12:68
igb 0000:08:00.0: eth0: PBA No: G15137-006
igb 0000:08:00.0: Using MSI-X interrupts. 4 rx queue(s), 4 tx queue(s)
igb 0000:08:00.1: added PHC on eth1
igb 0000:08:00.1: Intel(R) Gigabit Ethernet Network Connection
igb 0000:08:00.1: eth1: (PCIe:2.5Gb/s:Width x4) a0:36:9f:00:12:69
igb 0000:08:00.1: eth1: PBA No: G15137-006
igb 0000:08:00.1: Using MSI-X interrupts. 4 rx queue(s), 4 tx queue(s)
igb 0000:09:00.0: added PHC on eth2
igb 0000:09:00.0: Intel(R) Gigabit Ethernet Network Connection
igb 0000:09:00.0: eth2: (PCIe:2.5Gb/s:Width x4) 00:1b:21:2c:65:6c
igb 0000:09:00.0: eth2: PBA No: Unknown
igb 0000:09:00.0: Using MSI-X interrupts. 4 rx queue(s), 4 tx queue(s)
igb 0000:09:00.1: added PHC on eth3
igb 0000:09:00.1: Intel(R) Gigabit Ethernet Network Connection
igb 0000:09:00.1: eth3: (PCIe:2.5Gb/s:Width x4) 00:1b:21:2c:65:6d
igb 0000:09:00.1: eth3: PBA No: Unknown
igb 0000:09:00.1: Using MSI-X interrupts. 4 rx queue(s), 4 tx queue(s)
igb 0000:08:00.0: offline testing starting
igb 0000:08:00.0: testing shared interrupt
Slab corruption (Not tainted): skbuff_head_cache start=ffff88013adff000, len=200
0c0: 6b 6b 6b 6b 3e a3 3f 9f kkkk>.?.
Next obj: start=ffff88013adff100, len=200
000: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
010: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkkswap_free: Bad swap offset entry 3ff4f6c5c0000
BUG: Bad page map in process crond pte:ffd3db1700000000 pmd:13adff067 addr:00007f5e42218000 vm_flags:00000070 anon_vma: (null) mapping:ffff88013b02b7d8 index:191 file:pam_localuser.so fault:ext4_filemap_fault mmap:ext4_file_mmap readpage:ext4_readpage CPU: 3 PID: 5108 Comm: crond Not tainted 4.10.0-rc2_next-queue_dev-queue_46091fe #11 Hardware name: Supermicro X7DB8/X7DB8, BIOS 6.00 08/11/2006 Call Trace: dump_stack+0x53/0x74 ? ext4_bh_unmapped+0x13/0x13 ? ext4_file_open+0x1f3/0x1f3 print_bad_pte+0x217/0x235 unmap_page_range+0x4f6/0x74a unmap_single_vma+0xba/0xc5 unmap_vmas+0x2d/0x44 unmap_region+0xc5/0x114 ? __vma_rb_erase+0x231/0x236 ? handle_mm_fault+0x114/0x16a do_munmap+0x262/0x2e7 SyS_munmap+0x51/0x68 entry_SYSCALL_64_fastpath+0x13/0x94 RIP: 0033:0x7f5e499f8e87 RSP: 002b:00007ffc348c0fb8 EFLAGS: 00000206 ORIG_RAX: 000000000000000b RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f5e499f8e87 RDX: 000000000000000a RSI: 0000000000202060 RDI: 00007f5e42087000 RBP: 00007ffc348c0fc0 R08: 0000000000000000 R09: 0000000000000000 R10: 0000000000000000 R11: 0000000000000206 R12: 00007f5e42288de0 R13: 000000000000000b R14: 00007ffc348c10cb R15: 00007ffc348c1038 Disabling lock debugging due to kernel taint BUG: Bad rss-counter state mm:ffff88013a263000 idx:2 val:-1 ------------[ cut here ]------------ kernel BUG at mm/slab.c:2796! invalid opcode: 0000 [#1] PREEMPT SMP Modules linked in: igb dca i2c_algo_bit i2c_core ptp pps_core hwmon nfsd lockd grace sunrpc CPU: 0 PID: 5107 Comm: ethtool Tainted: G B 4.10.0-rc2_next-queue_dev-queue_46091fe #11 Hardware name: Supermicro X7DB8/X7DB8, BIOS 6.00 08/11/2006 task: ffff880139c74a80 task.stack: ffffc900053e4000 RIP: 0010:___cache_free+0x94/0x409 RSP: 0018:ffffc900053e79c8 EFLAGS: 00010007 RAX: ffffea00044e0fc8 RBX: ffff88013f087500 RCX: ffffea00044b579f RDX: ffffea00044b5700 RSI: ffffffff81700654 RDI: ffffffff8170ea10 RBP: ffffc900053e7a78 R08: 0000000000000001 R09: ffffc900053e7ae8 R10: 0000160000000000 R11: ffffffff00001be4 R12: 000000000001d120 R13: ffff88013adff000 R14: ffffc9000531b9a0 R15: ffff88013ab995c0 FS: 00007f4518989700(0000) GS:ffff88013fc00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: ffffffffff600400 CR3: 000000013a1ad000 CR4: 00000000000006f0 Call Trace: ? __kfree_skb+0x5e/0x65 ? __schedule+0x3d6/0x414 kmem_cache_free+0x6d/0x76 ? skb_release_all+0x11/0x28 __kfree_skb+0x5e/0x65 consume_skb+0x39/0x3b __dev_kfree_skb_any+0x25/0x27 igb_diag_test+0xdce/0x1115 [igb] ? __kmalloc+0xed/0x119 ethtool_self_test+0xc9/0x141 dev_ethtool+0xa10/0x14d5 ? netdev_run_todo+0x61/0x32b ? debug_smp_processor_id+0x17/0x1c ? __might_sleep+0x76/0x7f dev_ioctl+0x4ca/0x64d sock_ioctl+0x24b/0x258 vfs_ioctl+0x13/0x23 do_vfs_ioctl+0x649/0x682 ? __do_page_fault+0x3e9/0x4e4 SyS_ioctl+0x43/0x62 entry_SYSCALL_64_fastpath+0x13/0x94 RIP: 0033:0x3ab1edd847 RSP: 002b:00007ffd93127128 EFLAGS: 00000246 ORIG_RAX: 0000000000000010 RAX: ffffffffffffffda RBX: 0000003ab219ce80 RCX: 0000003ab1edd847 RDX: 00007ffd93127190 RSI: 0000000000008946 RDI: 0000000000000003 RBP: 00000000000000c0 R08: 0000000000000028 R09: 0101010101010101 R10: 0000000000000038 R11: 0000000000000246 R12: 0000003ab219ced8 R13: 0000000000000000 R14: 0000000000000000 R15: 0000003ab219bd80 Code: 00 00 00 00 ea ff ff 48 c1 e8 0c 48 6b c0 38 48 01 d0 48 8b 50 20 48 89 55 80 48 8d 4a ff 80 e2 01 48 0f 45 c1 48 39 58 30 74 04 <0f> 0b eb fe 48 63 83 e0 00 00 00 49 29 c5 4c 89 ef e8 e1 fd ff RIP: ___cache_free+0x94/0x409 RSP: ffffc900053e79c8 ---[ end trace 9bcc0d1ee8a9c30e ]--- BUG: sleeping function called from invalid context at ./include/linux/sched.h:3153 in_atomic(): 0, irqs_disabled(): 1, pid: 5107, name: ethtool CPU: 0 PID: 5107 Comm: ethtool Tainted: G B D 4.10.0-rc2_next-queue_dev-queue_46091fe #11 Hardware name: Supermicro X7DB8/X7DB8, BIOS 6.00 08/11/2006 Call Trace: dump_stack+0x53/0x74 ___might_sleep+0x15b/0x170 __might_sleep+0x76/0x7f exit_signals+0x21/0xfd ? blocking_notifier_call_chain+0xf/0x11 do_exit+0x13a/0x523 ? do_vfs_ioctl+0x649/0x682 ? __do_page_fault+0x3e9/0x4e4 ? SyS_ioctl+0x43/0x62 rewind_stack_do_exit+0x17/0x19 RIP: 0033:0x3ab1edd847 RSP: 002b:00007ffd93127128 EFLAGS: 00000246 ORIG_RAX: 0000000000000010 RAX: ffffffffffffffda RBX: 0000003ab219ce80 RCX: 0000003ab1edd847 RDX: 00007ffd93127190 RSI: 0000000000008946 RDI: 0000000000000003 RBP: 00000000000000c0 R08: 0000000000000028 R09: 0101010101010101 R10: 0000000000000038 R11: 0000000000000246 R12: 0000003ab219ced8 R13: 0000000000000000 R14: 0000000000000000 R15: 0000003ab219bd80 ------------[ cut here ]------------ WARNING: CPU: 0 PID: 5107 at kernel/softirq.c:161 __local_bh_enable_ip+0x41/0x85 Modules linked in: igb dca i2c_algo_bit i2c_core ptp pps_core hwmon nfsd lockd grace sunrpc CPU: 0 PID: 5107 Comm: ethtool Tainted: G B D W 4.10.0-rc2_next-queue_dev-queue_46091fe #11 Hardware name: Supermicro X7DB8/X7DB8, BIOS 6.00 08/11/2006 Call Trace: dump_stack+0x53/0x74 __warn+0xd7/0xf5 warn_slowpath_null+0x18/0x1a __local_bh_enable_ip+0x41/0x85 _raw_spin_unlock_bh+0x17/0x19 udp_destroy_sock+0x39/0x67 sk_common_release+0x1d/0xb1 udp_lib_close+0x9/0xb inet_release+0x4f/0x56 sock_release+0x1a/0x70 sock_close+0xd/0x11 __fput+0x100/0x1ba ____fput+0x9/0xb task_work_run+0x76/0xa2 ? switch_task_namespaces+0x49/0x68 do_exit+0x456/0x523 ? do_vfs_ioctl+0x649/0x682 ? __do_page_fault+0x3e9/0x4e4 ? SyS_ioctl+0x43/0x62 rewind_stack_do_exit+0x17/0x19 RIP: 0033:0x3ab1edd847 RSP: 002b:00007ffd93127128 EFLAGS: 00000246 ORIG_RAX: 0000000000000010 RAX: ffffffffffffffda RBX: 0000003ab219ce80 RCX: 0000003ab1edd847 RDX: 00007ffd93127190 RSI: 0000000000008946 RDI: 0000000000000003 RBP: 00000000000000c0 R08: 0000000000000028 R09: 0101010101010101 R10: 0000000000000038 R11: 0000000000000246 R12: 0000003ab219ced8 R13: 0000000000000000 R14: 0000000000000000 R15: 0000003ab219bd80 ---[ end trace 9bcc0d1ee8a9c30f ]---
PATCH 3/9 igb: Clear Rx buffer_info in configure instead of clean
In reply to Alexander DuyckFrom: Alexander Duyck <[email protected]>
This change makes it so that instead of going through the entire ring on Rx cleanup we only go through the region that was designated to be cleaned up and stop when we reach the region where new allocations should start.
In addition we can avoid having to perform a memset on the Rx buffer_info structures until we are about to start using the ring again. By deferring this we can avoid dirtying the cache any more than we have to which can help to improve the time needed to bring the interface down and then back up again in a reset or suspend/resume cycle.
--- drivers/net/ethernet/intel/igb/igb_main.c | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-)
drivers/net/ethernet/intel/igb/igb_main.c +10 -14
@@ -3435,7 +3435,7 @@ int igb_setup_rx_resources(struct igb_ring *rx_ring) size =sizeof (struct igb_rx_buffer ) * rx_ring->count; - rx_ring->rx_buffer_info =vzalloc (size); + rx_ring->rx_buffer_info =vmalloc (size);if (!rx_ring->rx_buffer_info) goto err;
@@ -3759,6 +3759,10 @@ void igb_configure_rx_ring(struct igb_adapter *adapter, rxdctl |=IGB_RX_HTHRESH <<8 ; rxdctl |=IGB_RX_WTHRESH <<16 ; +/* initialize rx_buffer_info */ +memset (ring->rx_buffer_info,0 , +sizeof (struct igb_rx_buffer ) * ring->count); +/* initialize Rx descriptor 0 */ rx_desc =IGB_RX_DESC (ring,0 ); rx_desc->wb.upper.length =0 ;
@@ -3937,23 +3941,16 @@ static void igb_free_all_rx_resources(struct igb_adapter *adapter)**/ static void igb_clean_rx_ring (struct igb_ring *rx_ring) { -unsigned long size; -u16 i; +u16 i = rx_ring->next_to_clean;if (rx_ring->skb)dev_kfree_skb (rx_ring->skb); rx_ring->skb =NULL ; -if (!rx_ring->rx_buffer_info) -return ; -/* Free all the Rx ring sk_buffs */ -for (i =0 ; i < rx_ring->count; i++) { +while (i != rx_ring->next_to_alloc) {struct igb_rx_buffer *buffer_info = &rx_ring->rx_buffer_info[i]; -if (!buffer_info->page) -continue ; -/* Invalidate cache lines that may have been written to by * device so that we avoid corrupting memory. */
@@ -3972,12 +3969,11 @@ static void igb_clean_rx_ring(struct igb_ring *rx_ring)__page_frag_cache_drain (buffer_info->page, buffer_info->pagecnt_bias); - buffer_info->page =NULL ; + i++; +if (i == rx_ring->count) + i =0 ; } - size =sizeof (struct igb_rx_buffer ) * rx_ring->count; -memset (rx_ring->rx_buffer_info,0 , size); - rx_ring->next_to_alloc =0 ; rx_ring->next_to_clean =0 ; rx_ring->next_to_use =0 ;
PATCH 4/9 igb: Don't bother clearing Tx buffer_info in igb_clean_tx_ring
In reply to Alexander DuyckFrom: Alexander Duyck <[email protected]>
In the case of the Tx rings we need to only clear the Tx buffer_info when we are resetting the rings. Ideally we do this when we configure the ring to bring it back up instead of when we are taking it down in order to avoid dirtying pages we don't need to.
In addition we don't need to clear the Tx descriptor ring since we will fully repopulate it when we begin transmitting frames and next_to_watch can be cleared to prevent the ring from being cleaned beyond that point instead of needing to touch anything in the Tx descriptor ring.
Finally with these changes we can avoid having to reset the skb member of the Tx buffer_info structure in the cleanup path since the skb will always be associated with the first buffer which has next_to_watch set.
--- drivers/net/ethernet/intel/igb/igb.h | 1 drivers/net/ethernet/intel/igb/igb_ethtool.c | 11 ++ drivers/net/ethernet/intel/igb/igb_main.c | 119 ++++++++++++++++---------- 3 files changed, 82 insertions(+), 49 deletions(-)
drivers/net/ethernet/intel/igb/igb.h +0 -1
@@ -594,7 +594,6 @@ enum igb_boards {void igb_setup_tctl (struct igb_adapter *);void igb_setup_rctl (struct igb_adapter *);netdev_tx_t igb_xmit_frame_ring (struct sk_buff *,struct igb_ring *); -void igb_unmap_and_free_tx_resource (struct igb_ring *,struct igb_tx_buffer *);void igb_alloc_rx_buffers (struct igb_ring *,u16 );void igb_update_stats (struct igb_adapter *,struct rtnl_link_stats64 *);bool igb_has_link (struct igb_adapter *adapter);
drivers/net/ethernet/intel/igb/igb_ethtool.c +10 -1
@@ -1833,7 +1833,16 @@ static int igb_clean_test_rings(struct igb_ring *rx_ring,/* unmap buffer on Tx side */ tx_buffer_info = &tx_ring->tx_buffer_info[tx_ntc]; -igb_unmap_and_free_tx_resource (tx_ring, tx_buffer_info); + +/* Free all the Tx ring sk_buffs */ +dev_kfree_skb_any (tx_buffer_info->skb); + +/* unmap skb header data */ +dma_unmap_single (tx_ring->dev, +dma_unmap_addr (tx_buffer_info, dma), +dma_unmap_len (tx_buffer_info, len), +DMA_TO_DEVICE ); +dma_unmap_len_set (tx_buffer_info, len,0 );/* increment Rx/Tx next to clean counters */ rx_ntc++;
drivers/net/ethernet/intel/igb/igb_main.c +72 -47
170 lines not on this page · raw message
PATCH 5/9 igb: Limit maximum frame Rx based on MTU
In reply to Alexander DuyckFrom: Alexander Duyck <[email protected]>
In order to support the use of build_skb going forward it will be necessary to place a maximum limit on the amount of data we can receive when jumbo frames is not enabled. In order to do this I am adding a new upper limit for receive based on the size of a 2K buffer minus padding.
--- drivers/net/ethernet/intel/igb/igb.h | 10 +++++++++- drivers/net/ethernet/intel/igb/igb_main.c | 16 ++++++++++++---- 2 files changed, 21 insertions(+), 5 deletions(-)
drivers/net/ethernet/intel/igb/igb.h +9 -1
@@ -143,8 +143,17 @@ struct vf_data_storage {#define IGB_RXBUFFER_256 256#define IGB_RXBUFFER_2048 2048#define IGB_RX_HDR_LEN IGB_RXBUFFER_256 +#define IGB_TS_HDR_LEN 16#define IGB_RX_BUFSZ IGB_RXBUFFER_2048 +#define IGB_SKB_PAD (NET_SKB_PAD + NET_IP_ALIGN) +#if (PAGE_SIZE <8192 ) +#define IGB_MAX_FRAME_BUILD_SKB \ + (SKB_WITH_OVERHEAD(IGB_RXBUFFER_2048) - IGB_SKB_PAD - IGB_TS_HDR_LEN) +#else +#define IGB_MAX_FRAME_BUILD_SKB (IGB_RXBUFFER_2048 - IGB_TS_HDR_LEN) +#endif +/* How many Rx Buffers do we bundle into one write to the hardware ? */ #define IGB_RX_BUFFER_WRITE 16/* Must be power of 2 */
@@ -561,7 +570,6 @@ struct igb_adapter {#define IGB_DMCTLX_DCFLUSH_DIS 0x80000000/* Disable DMA Coal Flush */ #define IGB_82576_TSYNC_SHIFT 19 -#define IGB_TS_HDR_LEN 16enum e1000_state_t { __IGB_TESTING, __IGB_RESETTING,
drivers/net/ethernet/intel/igb/igb_main.c +12 -4
@@ -4250,7 +4250,7 @@ static void igb_set_rx_mode(struct net_device *netdev)struct igb_adapter *adapter =netdev_priv (netdev);struct e1000_hw *hw = &adapter->hw;unsigned int vfn = adapter->vfs_allocated_count; -u32 rctl =0 , vmolr =0 ; +u32 rctl =0 , vmolr =0 , rlpml =MAX_JUMBO_FRAME_SIZE ;int count;/* Check for Promiscuous and All Multicast modes */
@@ -4322,12 +4322,20 @@ static void igb_set_rx_mode(struct net_device *netdev) vmolr |=rd32 (E1000_VMOLR (vfn)) & ~(E1000_VMOLR_ROPE |E1000_VMOLR_MPME |E1000_VMOLR_ROMPE ); -/* enable Rx jumbo frames, no need for restriction */ +/* enable Rx jumbo frames, restrict as needed to support build_skb */ vmolr &= ~E1000_VMOLR_RLPML_MASK ; - vmolr |=MAX_JUMBO_FRAME_SIZE |E1000_VMOLR_LPE ; +#if (PAGE_SIZE <8192 ) +if (adapter->max_frame_size <=IGB_MAX_FRAME_BUILD_SKB ) { +if (!adapter->vfs_allocated_count) + rlpml =IGB_MAX_FRAME_BUILD_SKB ; + vmolr |=IGB_MAX_FRAME_BUILD_SKB ; + } else +#endif + vmolr |=MAX_JUMBO_FRAME_SIZE ; + vmolr |=E1000_VMOLR_LPE ;wr32 (E1000_VMOLR (vfn), vmolr); -wr32 (E1000_RLPML ,MAX_JUMBO_FRAME_SIZE ); +wr32 (E1000_RLPML , rlpml);igb_restore_vf_multicasts (adapter); }
PATCH 6/9 igb: Add support for padding packet
In reply to Alexander DuyckFrom: Alexander Duyck <[email protected]>
With the size of the frame limited we can now write to an offset within the buffer instead of having to write at the very start of the buffer. The advantage to this is that it allows us to leave padding room for things like supporting XDP in the future.
One side effect of this patch is that we can end up using a larger buffer if jumbo frames is enabled. The impact shouldn't be too great, but it could hurt small packet performance for UDP workloads if jumbo frames is enabled as the truesize of frames will be larger.
--- drivers/net/ethernet/intel/igb/igb.h | 42 ++++++++++++ drivers/net/ethernet/intel/igb/igb_ethtool.c | 4 + drivers/net/ethernet/intel/igb/igb_main.c | 89 +++++++++++++++++++------- 3 files changed, 107 insertions(+), 28 deletions(-)
drivers/net/ethernet/intel/igb/igb.h +41 -1
@@ -142,9 +142,9 @@ struct vf_data_storage {/* Supported Rx Buffer Sizes */ #define IGB_RXBUFFER_256 256#define IGB_RXBUFFER_2048 2048 +#define IGB_RXBUFFER_3072 3072#define IGB_RX_HDR_LEN IGB_RXBUFFER_256#define IGB_TS_HDR_LEN 16 -#define IGB_RX_BUFSZ IGB_RXBUFFER_2048#define IGB_SKB_PAD (NET_SKB_PAD + NET_IP_ALIGN)#if (PAGE_SIZE <8192 )
@@ -313,12 +313,51 @@ struct igb_q_vector { };enum e1000_ring_flags_t { +IGB_RING_FLAG_RX_3K_BUFFER , +IGB_RING_FLAG_RX_BUILD_SKB_ENABLED ,IGB_RING_FLAG_RX_SCTP_CSUM ,IGB_RING_FLAG_RX_LB_VLAN_BSWAP ,IGB_RING_FLAG_TX_CTX_IDX ,IGB_RING_FLAG_TX_DETECT_HANG }; +#define ring_uses_build_skb (ring) \ + test_bit(IGB_RING_FLAG_RX_BUILD_SKB_ENABLED, &(ring)->flags) +#define set_ring_build_skb_enabled (ring) \ + set_bit(IGB_RING_FLAG_RX_BUILD_SKB_ENABLED, &(ring)->flags) +#define clear_ring_build_skb_enabled (ring) \ + clear_bit(IGB_RING_FLAG_RX_BUILD_SKB_ENABLED, &(ring)->flags) + +#define ring_uses_large_buffer (ring) \ + test_bit(IGB_RING_FLAG_RX_3K_BUFFER, &(ring)->flags) +#define set_ring_uses_large_buffer (ring) \ + set_bit(IGB_RING_FLAG_RX_3K_BUFFER, &(ring)->flags) +#define clear_ring_uses_large_buffer (ring) \ + clear_bit(IGB_RING_FLAG_RX_3K_BUFFER, &(ring)->flags) + +static inline unsigned int igb_rx_bufsz (struct igb_ring *ring) +{ +#if (PAGE_SIZE <8192 ) +if (ring_uses_large_buffer (ring)) +return IGB_RXBUFFER_3072 ; + +if (ring_uses_build_skb (ring)) +return IGB_MAX_FRAME_BUILD_SKB +IGB_TS_HDR_LEN ; +#endif +return IGB_RXBUFFER_2048 ; +} + +static inline unsigned int igb_rx_pg_order (struct igb_ring *ring) +{ +#if (PAGE_SIZE <8192 ) +if (ring_uses_large_buffer (ring)) +return 1 ; +#endif +return 0 ; +} + +#define igb_rx_pg_size (_ring) (PAGE_SIZE << igb_rx_pg_order(_ring)) +#define IGB_TXD_DCMD (E1000_ADVTXD_DCMD_EOP | E1000_ADVTXD_DCMD_RS)#define IGB_RX_DESC (R , i) \
@@ -557,6 +596,7 @@ struct igb_adapter {#define IGB_FLAG_HAS_MSIX BIT(13)#define IGB_FLAG_EEE BIT(14)#define IGB_FLAG_VLAN_PROMISC BIT(15) +#define IGB_FLAG_RX_LEGACY BIT(16)/* Media Auto Sense */ #define IGB_MAS_ENABLE_0 0X0001
drivers/net/ethernet/intel/igb/igb_ethtool.c +2 -2
@@ -1818,7 +1818,7 @@ static int igb_clean_test_rings(struct igb_ring *rx_ring,/* sync Rx buffer for CPU read */ dma_sync_single_for_cpu (rx_ring->dev, rx_buffer_info->dma, -IGB_RX_BUFSZ , + size,DMA_FROM_DEVICE );/* verify contents of skb */
@@ -1828,7 +1828,7 @@ static int igb_clean_test_rings(struct igb_ring *rx_ring,/* sync Rx buffer for device write */ dma_sync_single_for_device (rx_ring->dev, rx_buffer_info->dma, -IGB_RX_BUFSZ , + size,DMA_FROM_DEVICE );/* unmap buffer on Tx side */
drivers/net/ethernet/intel/igb/igb_main.c +64 -25
217 lines not on this page · raw message
PATCH 7/9 igb: Add support for ethtool private flag to allow use of legacy Rx
In reply to Alexander DuyckFrom: Alexander Duyck <[email protected]>
Since there are potential drawbacks to the new Rx allocation approach I thought it best to add a "chicken bit" so that we can turn the feature off if in the event that a problem is found.
It also provides a means of validating the legacy Rx path in the event that we are forced to fall back. At some point in the future when we are convinced we don't need it anymore we might be able to drop the legacy-rx flag.
--- drivers/net/ethernet/intel/igb/igb_ethtool.c | 48 ++++++++++++++++++++++++++ 1 file changed, 48 insertions(+)
drivers/net/ethernet/intel/igb/igb_ethtool.c +48 -0
@@ -144,6 +144,13 @@ enum igb_diagnostics_results { };#define IGB_TEST_LEN (sizeof(igb_gstrings_test) / ETH_GSTRING_LEN) +static const char igb_priv_flags_strings[][ETH_GSTRING_LEN ] = { +#define IGB_PRIV_FLAGS_LEGACY_RX BIT(0) +"legacy-rx" , +}; + +#define IGB_PRIV_FLAGS_STR_LEN ARRAY_SIZE(igb_priv_flags_strings) +static int igb_get_settings (struct net_device *netdev,struct ethtool_cmd *ecmd) {struct igb_adapter *adapter =netdev_priv (netdev);
@@ -852,6 +859,8 @@ static void igb_get_drvinfo(struct net_device *netdev, sizeof(drvinfo->fw_version));strlcpy (drvinfo->bus_info,pci_name (adapter->pdev),sizeof (drvinfo->bus_info)); + + drvinfo->n_priv_flags =IGB_PRIV_FLAGS_STR_LEN ; } static void igb_get_ringparam(struct net_device *netdev,
@@ -2280,6 +2289,8 @@ static int igb_get_sset_count(struct net_device *netdev, int sset)return IGB_STATS_LEN ;case ETH_SS_TEST :return IGB_TEST_LEN ; +case ETH_SS_PRIV_FLAGS : +return IGB_PRIV_FLAGS_STR_LEN ;default :return -ENOTSUPP ; }
@@ -2385,6 +2396,10 @@ static void igb_get_strings(struct net_device *netdev, u32 stringset, u8 *data) }/* BUG_ON(p - data != IGB_STATS_LEN * ETH_GSTRING_LEN); */ break ; +case ETH_SS_PRIV_FLAGS : +memcpy (data, igb_priv_flags_strings, +IGB_PRIV_FLAGS_STR_LEN *ETH_GSTRING_LEN ); +break ; } }
@@ -3397,6 +3412,37 @@ static int igb_set_channels(struct net_device *netdev,return 0 ; } +static u32 igb_get_priv_flags (struct net_device *netdev) +{ +struct igb_adapter *adapter =netdev_priv (netdev); +u32 priv_flags =0 ; + +if (adapter->flags &IGB_FLAG_RX_LEGACY ) + priv_flags |=IGB_PRIV_FLAGS_LEGACY_RX ; + +return priv_flags; +} + +static int igb_set_priv_flags (struct net_device *netdev,u32 priv_flags) +{ +struct igb_adapter *adapter =netdev_priv (netdev); +unsigned int flags = adapter->flags; + + flags &= ~IGB_FLAG_RX_LEGACY ; +if (priv_flags &IGB_PRIV_FLAGS_LEGACY_RX ) + flags |=IGB_FLAG_RX_LEGACY ; + +if (flags != adapter->flags) { + adapter->flags = flags; + +/* reset interface to repopulate queues */ +if (netif_running (netdev)) +igb_reinit_locked (adapter); + } + +return 0 ; +} +static const struct ethtool_ops igb_ethtool_ops = { .get_settings = igb_get_settings, .set_settings = igb_set_settings,
@@ -3435,6 +3481,8 @@ static int igb_set_channels(struct net_device *netdev, .set_rxfh = igb_set_rxfh, .get_channels = igb_get_channels, .set_channels = igb_set_channels, + .get_priv_flags = igb_get_priv_flags, + .set_priv_flags = igb_set_priv_flags, .begin = igb_ethtool_begin, .complete = igb_ethtool_complete, };
PATCH 8/9 igb: Break out Rx buffer page management
In reply to Alexander DuyckFrom: Alexander Duyck <[email protected]>
At this point we have 2 to 3 paths that can be taken depending on what Rx modes are enabled. In order to better support that and improve the maintainability I am breaking out the common bits from those paths and making them into their own functions.
--- drivers/net/ethernet/intel/igb/igb_main.c | 237 +++++++++++++++-------------- 1 file changed, 123 insertions(+), 114 deletions(-)
drivers/net/ethernet/intel/igb/igb_main.c +123 -114
@@ -6882,8 +6882,14 @@ static void igb_reuse_rx_page(struct igb_ring *rx_ring, nta++; rx_ring->next_to_alloc = (nta < rx_ring->count) ? nta :0 ; -/* transfer page from old buffer to new buffer */ - *new_buff = *old_buff; +/* Transfer page from old buffer to new buffer. +* Move each member individually to avoid possible store +* forwarding stalls. +*/ + new_buff->dma = old_buff->dma; + new_buff->page = old_buff->page; + new_buff->page_offset = old_buff->page_offset; + new_buff->pagecnt_bias = old_buff->pagecnt_bias; }static inline bool igb_page_is_reserved (struct page *page)
@@ -6891,11 +6897,10 @@ static inline bool igb_page_is_reserved(struct page *page)return (page_to_nid (page) !=numa_mem_id ()) ||page_is_pfmemalloc (page); } -static bool igb_can_reuse_rx_page (struct igb_rx_buffer *rx_buffer, -struct page *page, -const unsigned int truesize) +static bool igb_can_reuse_rx_page (struct igb_rx_buffer *rx_buffer) { -unsigned int pagecnt_bias = rx_buffer->pagecnt_bias--; +unsigned int pagecnt_bias = rx_buffer->pagecnt_bias; +struct page *page = rx_buffer->page;/* avoid re-using remote pages */ if (unlikely (igb_page_is_reserved (page)))
@@ -6903,14 +6908,9 @@ static bool igb_can_reuse_rx_page(struct igb_rx_buffer *rx_buffer,#if (PAGE_SIZE <8192 )/* if we are only owner of page we can reuse it */ -if (unlikely (page_ref_count (page) != pagecnt_bias)) +if (unlikely ((page_ref_count (page) - pagecnt_bias) >1 ))return false; - -/* flip page offset to other buffer */ - rx_buffer->page_offset ^= truesize;#else -/* move offset up to the next cache line */ - rx_buffer->page_offset += truesize;#define IGB_LAST_OFFSET \ (SKB_WITH_OVERHEAD(PAGE_SIZE) - IGB_RXBUFFER_2048)
@@ -6922,7 +6922,7 @@ static bool igb_can_reuse_rx_page(struct igb_rx_buffer *rx_buffer,* the pagecnt_bias and page count so that we fully restock the * number of references the driver holds. */ -if (unlikely (pagecnt_bias ==1 )) { +if (unlikely (!pagecnt_bias)) {page_ref_add (page,USHRT_MAX ); rx_buffer->pagecnt_bias =USHRT_MAX ; }
@@ -6934,26 +6934,16 @@ static bool igb_can_reuse_rx_page(struct igb_rx_buffer *rx_buffer,* igb_add_rx_frag - Add contents of Rx buffer to sk_buff * @rx_ring: rx descriptor ring to transact packets on * @rx_buffer: buffer containing page to add -* @rx_desc: descriptor containing length of buffer written by hardware * @skb: sk_buff to place the data into +* @size: size of buffer to be added * * This function will add the data contained in rx_buffer->page to the skb. -* This is done either through a direct copy if the data in the buffer is -* less than the skb header size, otherwise it will just attach the page as -* a frag to the skb. -* -* The function will then update the page offset if necessary and return -* true if the buffer can be reused by the adapter. **/ -static bool igb_add_rx_frag (struct igb_ring *rx_ring, +static void igb_add_rx_frag (struct igb_ring *rx_ring,struct igb_rx_buffer *rx_buffer, -unsigned int size, -union e1000_adv_rx_desc *rx_desc, -struct sk_buff *skb) +struct sk_buff *skb, +unsigned int size) { -struct page *page = rx_buffer->page; -unsigned char *va =page_address (page) + rx_buffer->page_offset; -unsigned int pull_len;#if (PAGE_SIZE <8192 )unsigned int truesize =igb_rx_pg_size (rx_ring) /2 ;#else
@@ -6961,9 +6951,39 @@ static bool igb_add_rx_frag(struct igb_ring *rx_ring, SKB_DATA_ALIGN(IGB_SKB_PAD + size) :SKB_DATA_ALIGN (size);#endif +skb_add_rx_frag (skb,skb_shinfo (skb)->nr_frags, rx_buffer->page, + rx_buffer->page_offset, size, truesize); +#if (PAGE_SIZE <8192 ) + rx_buffer->page_offset ^= truesize; +#else + rx_buffer->page_offset += truesize; +#endif +} + +static struct sk_buff *igb_construct_skb (struct igb_ring *rx_ring, +struct igb_rx_buffer *rx_buffer, +union e1000_adv_rx_desc *rx_desc, +unsigned int size) +{ +void *va =page_address (rx_buffer->page) + rx_buffer->page_offset; +#if (PAGE_SIZE <8192 ) +unsigned int truesize =igb_rx_pg_size (rx_ring) /2 ; +#else +unsigned int truesize =SKB_DATA_ALIGN (size); +#endif +unsigned int headlen; +struct sk_buff *skb; -if (unlikely (skb_is_nonlinear (skb))) - goto add_tail_frag; +/* prefetch first cache line of first page */ +prefetch (va); +#if L1_CACHE_BYTES <128 +prefetch (va +L1_CACHE_BYTES ); +#endif + +/* allocate a skb to store the frags */ + skb =napi_alloc_skb (&rx_ring->q_vector->napi,IGB_RX_HDR_LEN ); +if (unlikely (!skb)) +return NULL ;if (unlikely (igb_test_staterr (rx_desc,E1000_RXDADV_STAT_TSIP ))) {igb_ptp_rx_pktstamp (rx_ring->q_vector, va, skb);
@@ -6971,95 +6991,31 @@ static bool igb_add_rx_frag(struct igb_ring *rx_ring, size -=IGB_TS_HDR_LEN ; } -if (likely (size <=IGB_RX_HDR_LEN )) { -memcpy (__skb_put (skb, size), va,ALIGN (size,sizeof (long ))); - -/* page is not reserved, we can reuse buffer as-is */ -if (likely (!igb_page_is_reserved (page))) -return true; - -/* this page cannot be reused so discard it */ -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,IGB_RX_HDR_LEN ); +/* Determine available headroom for copy */ + headlen = size; +if (headlen >IGB_RX_HDR_LEN ) + headlen =eth_get_headlen (va,IGB_RX_HDR_LEN ); +else + headlen = size;/* align pull length to size of long to optimize memcpy performance */ -memcpy (__skb_put (skb, pull_len), va,ALIGN (pull_len,sizeof (long ))); +memcpy (__skb_put (skb, headlen), va,ALIGN (headlen,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, - va -page_address (page), size, truesize); - -return igb_can_reuse_rx_page (rx_buffer, page, truesize); -} - -static struct sk_buff *igb_fetch_rx_buffer (struct igb_ring *rx_ring, -union e1000_adv_rx_desc *rx_desc, -struct sk_buff *skb) -{ -unsigned int size =le16_to_cpu (rx_desc->wb.upper.length); -struct igb_rx_buffer *rx_buffer; -struct page *page; - - rx_buffer = &rx_ring->rx_buffer_info[rx_ring->next_to_clean]; - page = rx_buffer->page; -prefetchw (page); - -/* we are reusing so sync this buffer for CPU use */ -dma_sync_single_range_for_cpu (rx_ring->dev, - rx_buffer->dma, - rx_buffer->page_offset, - size, -DMA_FROM_DEVICE ); - -if (likely (!skb)) { -void *va =page_address (page) + rx_buffer->page_offset; - -/* prefetch first cache line of first page */ -prefetch (va); -#if L1_CACHE_BYTES <128 -prefetch (va +L1_CACHE_BYTES ); + size -= headlen; +if (size) { +skb_add_rx_frag (skb,0 , rx_buffer->page, + (va + headlen) -page_address (rx_buffer->page), + size, truesize); +#if (PAGE_SIZE <8192 ) + rx_buffer->page_offset ^= truesize; +#else + rx_buffer->page_offset += truesize;#endif - -/* allocate a skb to store the frags */ - skb =napi_alloc_skb (&rx_ring->q_vector->napi,IGB_RX_HDR_LEN ); -if (unlikely (!skb)) { - rx_ring->rx_stats.alloc_failed++; -return NULL ; - } - -/* we will be copying header into skb->data in -* pskb_may_pull so it is in our interest to prefetch -* it now to avoid a possible cache miss -*/ -prefetchw (skb->data); - } - -/* pull page into skb */ -if (igb_add_rx_frag (rx_ring, rx_buffer, size, rx_desc, skb)) { -/* hand second half of page back to the ring */ -igb_reuse_rx_page (rx_ring, rx_buffer); }else { -/* We are not reusing the buffer so unmap it and free -* any references we are holding to it -*/ -dma_unmap_page_attrs (rx_ring->dev, rx_buffer->dma, -igb_rx_pg_size (rx_ring),DMA_FROM_DEVICE , -IGB_RX_DMA_ATTR ); -__page_frag_cache_drain (page, rx_buffer->pagecnt_bias); + rx_buffer->pagecnt_bias++; } -/* clear contents of rx_buffer */ - rx_buffer->page =NULL ; -return skb; }
@@ -7217,6 +7173,47 @@ static void igb_process_skb_fields(struct igb_ring *rx_ring, skb->protocol =eth_type_trans (skb, rx_ring->netdev); } +static struct igb_rx_buffer *igb_get_rx_buffer (struct igb_ring *rx_ring, +const unsigned int size) +{ +struct igb_rx_buffer *rx_buffer; + + rx_buffer = &rx_ring->rx_buffer_info[rx_ring->next_to_clean]; +prefetchw (rx_buffer->page); + +/* we are reusing so sync this buffer for CPU use */ +dma_sync_single_range_for_cpu (rx_ring->dev, + rx_buffer->dma, + rx_buffer->page_offset, + size, +DMA_FROM_DEVICE ); + + rx_buffer->pagecnt_bias--; + +return rx_buffer; +} + +static void igb_put_rx_buffer (struct igb_ring *rx_ring, +struct igb_rx_buffer *rx_buffer) +{ +if (igb_can_reuse_rx_page (rx_buffer)) { +/* hand second half of page back to the ring */ +igb_reuse_rx_page (rx_ring, rx_buffer); + }else { +/* We are not reusing the buffer so unmap it and free +* any references we are holding to it +*/ +dma_unmap_page_attrs (rx_ring->dev, rx_buffer->dma, +igb_rx_pg_size (rx_ring),DMA_FROM_DEVICE , +IGB_RX_DMA_ATTR ); +__page_frag_cache_drain (rx_buffer->page, + rx_buffer->pagecnt_bias); + } + +/* clear contents of rx_buffer */ + rx_buffer->page =NULL ; +} +static int igb_clean_rx_irq (struct igb_q_vector *q_vector,const int budget) {struct igb_ring *rx_ring = q_vector->rx.ring;
@@ -7226,6 +7223,8 @@ static int igb_clean_rx_irq(struct igb_q_vector *q_vector, const int budget)while (likely (total_packets < budget)) {union e1000_adv_rx_desc *rx_desc; +struct igb_rx_buffer *rx_buffer; +unsigned int size;/* return some buffers to hardware, one@a time is too slow */ if (cleaned_count >=IGB_RX_BUFFER_WRITE ) {
@@ -7234,8 +7233,8 @@ static int igb_clean_rx_irq(struct igb_q_vector *q_vector, const int budget) } rx_desc =IGB_RX_DESC (rx_ring, rx_ring->next_to_clean); - -if (!rx_desc->wb.upper.length) + size =le16_to_cpu (rx_desc->wb.upper.length); +if (!size)break ;/* This memory barrier is needed to keep us from reading
@@ -7244,13 +7243,23 @@ static int igb_clean_rx_irq(struct igb_q_vector *q_vector, const int budget)*/ dma_rmb (); + rx_buffer =igb_get_rx_buffer (rx_ring, size); +/* retrieve a buffer from the ring */ - skb =igb_fetch_rx_buffer (rx_ring, rx_desc, skb); +if (skb) +igb_add_rx_frag (rx_ring, rx_buffer, skb, size); +else + skb =igb_construct_skb (rx_ring, rx_buffer, + rx_desc, size);/* exit if we failed to retrieve a buffer */ -if (!skb) +if (!skb) { + rx_ring->rx_stats.alloc_failed++; + rx_buffer->pagecnt_bias++;break ; + } +igb_put_rx_buffer (rx_ring, rx_buffer); cleaned_count++;/* fetch next buffer in frame if non-eop */
PATCH 9/9 igb: Revert "igb: Revert support for build_skb in igb"
In reply to Alexander DuyckFrom: Alexander Duyck <[email protected]>
This reverts commit f9d40f6a9921 ("igb: Revert support for build_skb in igb") and adds a few changes to update it to work with the latest version of igb. We are now able to revert the removal of this due to the fact that with the recent changes to the page count and the use of DMA_ATTR_SKIP_CPU_SYNC we can make the pages writable so we should not be invalidating the additional data added when we call build_skb.
--- drivers/net/ethernet/intel/igb/igb_main.c | 47 +++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+)
drivers/net/ethernet/intel/igb/igb_main.c +47 -0
@@ -7019,6 +7019,51 @@ static struct sk_buff *igb_construct_skb(struct igb_ring *rx_ring,return skb; } +static struct sk_buff *igb_build_skb (struct igb_ring *rx_ring, +struct igb_rx_buffer *rx_buffer, +union e1000_adv_rx_desc *rx_desc, +unsigned int size) +{ +void *va =page_address (rx_buffer->page) + rx_buffer->page_offset; +#if (PAGE_SIZE <8192 ) +unsigned int truesize =igb_rx_pg_size (rx_ring) /2 ; +#else +unsigned int truesize =SKB_DATA_ALIGN (sizeof (struct skb_shared_info )) + +SKB_DATA_ALIGN (IGB_SKB_PAD + size); +#endif +struct sk_buff *skb; + +/* prefetch first cache line of first page */ +prefetch (va); +#if L1_CACHE_BYTES <128 +prefetch (va +L1_CACHE_BYTES ); +#endif + +/* build an skb to around the page buffer */ + skb =build_skb (va -IGB_SKB_PAD , truesize); +if (unlikely (!skb)) +return NULL ; + +/* update pointers within the skb to store the data */ +skb_reserve (skb,IGB_SKB_PAD ); +__skb_put (skb, size); + +/* pull timestamp out of packet data */ +if (igb_test_staterr (rx_desc,E1000_RXDADV_STAT_TSIP )) { +igb_ptp_rx_pktstamp (rx_ring->q_vector, skb->data, skb); +__skb_pull (skb,IGB_TS_HDR_LEN ); + } + +/* update buffer offset */ +#if (PAGE_SIZE <8192 ) + rx_buffer->page_offset ^= truesize; +#else + rx_buffer->page_offset += truesize; +#endif + +return skb; +} +static inline void igb_rx_checksum (struct igb_ring *ring,union e1000_adv_rx_desc *rx_desc,struct sk_buff *skb)
@@ -7248,6 +7293,8 @@ static int igb_clean_rx_irq(struct igb_q_vector *q_vector, const int budget)/* retrieve a buffer from the ring */ if (skb)igb_add_rx_frag (rx_ring, rx_buffer, skb, size); +else if (ring_uses_build_skb (rx_ring)) + skb =igb_build_skb (rx_ring, rx_buffer, rx_desc, size);else skb =igb_construct_skb (rx_ring, rx_buffer, rx_desc, size);
PATCH 9/9 igb: Revert "igb: Revert support for build_skb in igb"
Quoted 9 lines · Alexander Duyck
From: Alexander Duyck <[email protected]>
This reverts commit
f9d40f6a9921("igb: Revert support forbuild_skbin igb") and adds a few changes to update it to work with the latest version of igb. We are now able to revert the removal of this due to the fact that with the recent changes to the page count and the use ofDMA_ATTR_SKIP_CPU_SYNCwe can make the pages writable so we should not be invalidating the additional data added when we callbuild_skb.
Quoted 5 lines
Signed-off-by Alexander Duyck <[email protected]>---
?drivers/net/ethernet/intel/igb/igb_main.c |?? 47
+++++++++++++++++++++++++++++
?1 file changed, 47 insertions(+)
Weird, this patch is not showing up in patchworks. I will get this patch
added as well to my tree and work on getting it to show up in patchworks. -------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.osuosl.org/pipermail/intel-wired-lan/attachments/20170106/a0e96a3b/attachment.asc>
Duyck
PATCH 9/9 igb: Revert "igb: Revert support for build_skb in igb"
Quoted 11 lines · Jeff Kirsher
Quoted 10 lines · Alexander Duyck
From: Alexander Duyck <[email protected]>
This reverts commit
f9d40f6a9921("igb: Revert support forbuild_skbin igb") and adds a few changes to update it to work with the latest version of igb. We are now able to revert the removal of this due to the fact that with the recent changes to the page count and the use ofDMA_ATTR_SKIP_CPU_SYNCwe can make the pages writable so we should not be invalidating the additional data added when we callbuild_skb.
Quoted 9 lines
Quoted 5 lines
Signed-off-by Alexander Duyck <[email protected]>---
?drivers/net/ethernet/intel/igb/igb_main.c |?? 47
+++++++++++++++++++++++++++++
?1 file changed, 47 insertions(+)Weird, this patch is not showing up in patchworks. I will get this patch added as well to my tree and work on getting it to show up in patchworks.
Maybe it is the quotes in the patch description? ?If so feel free to replace them with some other form of punctuation if needed.
- Alex