* [net-next PATCH 0/4] Clean up intel driver page reuse Rx code
From: Alexander Duyck @ 2014-11-13 16:18 UTC (permalink / raw)
To: netdev; +Cc: jeffrey.t.kirsher
This patch series cleans up the page reuse and Rx path so that all of the
fixes that have been applied to any one driver are now in place for igb,
fm10k, and ixgbe. It occured to me that fm10k was missing the pfmemalloc
bits, and ixgbe was missing some logic that reduced the number of writes
needed as well as the pfmemalloc fix.
In addition ixgbe was carrying around some mostly-dead code that was
wrapping a call to writel. I removed it since it masked the fact that
ixgbe was missing the mmiowb it was supposed to have between the tail write
and the Tx queue lock release to prevent the MMIO access from racing
between CPUs.
Also one change made to all 3 drivers is that we now only overwrite 3 of
the 4 DWORDs in the Rx descriptor after allocation. The last DWORD
contains the upper 32 bits of the header address on fetch, and the length
and vlan on writeback. Since we are no longer using header split we don't
need to clear it prior to descriptor fetch as it is unused so we can save
ourselves a cycle on 32b systems by just leaving it untouched.
---
Alexander Duyck (4):
igb: Clean-up page reuse code
fm10k: Clean-up page reuse code
ixgbe: Clean-up page reuse code
ixgbe: Remove tail write abstraction and add missing barrier
drivers/net/ethernet/intel/fm10k/fm10k_main.c | 34 ++++---
drivers/net/ethernet/intel/igb/igb_main.c | 35 +++----
drivers/net/ethernet/intel/ixgbe/ixgbe.h | 5 -
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 118 ++++++++++++-------------
4 files changed, 89 insertions(+), 103 deletions(-)
--
^ permalink raw reply
* [net-next PATCH 2/4] fm10k: Clean-up page reuse code
From: Alexander Duyck @ 2014-11-13 16:18 UTC (permalink / raw)
To: netdev; +Cc: Matthew Vick, jeffrey.t.kirsher
In-Reply-To: <20141113161148.2790.22082.stgit@ahduyck-vm-fedora20>
This patch cleans up the page reuse code getting it into a state where all
the workarounds needed are in place as well as cleaning up a few minor
oversights such as using __free_pages instead of put_page to drop a locally
allocated page.
It also cleans up how we clear the descriptor status bits. Previously they
were zeroed as a part of clearing the hdr_addr. However the hdr_addr is a
64 bit field and 64 bit writes can be a bit more expensive on on 32 bit
systems. Since we are no longer using the header split feature the upper
32 bits of the address no longer need to be cleared. As a result we can
just clear the status bits and leave the length and VLAN fields as-is which
should provide more information in debugging.
Cc: Matthew Vick <matthew.vick@intel.com>
Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com>
---
drivers/net/ethernet/intel/fm10k/fm10k_main.c | 34 +++++++++++++------------
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_main.c b/drivers/net/ethernet/intel/fm10k/fm10k_main.c
index 73457ed..8c8deec 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_main.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_main.c
@@ -97,7 +97,6 @@ static bool fm10k_alloc_mapped_page(struct fm10k_ring *rx_ring,
*/
if (dma_mapping_error(rx_ring->dev, dma)) {
__free_page(page);
- bi->page = NULL;
rx_ring->rx_stats.alloc_failed++;
return false;
@@ -147,8 +146,8 @@ void fm10k_alloc_rx_buffers(struct fm10k_ring *rx_ring, u16 cleaned_count)
i -= rx_ring->count;
}
- /* clear the hdr_addr for the next_to_use descriptor */
- rx_desc->q.hdr_addr = 0;
+ /* clear the status bits for the next_to_use descriptor */
+ rx_desc->d.staterr = 0;
cleaned_count--;
} while (cleaned_count);
@@ -194,7 +193,7 @@ static void fm10k_reuse_rx_page(struct fm10k_ring *rx_ring,
rx_ring->next_to_alloc = (nta < rx_ring->count) ? nta : 0;
/* transfer page from old buffer to new buffer */
- memcpy(new_buff, old_buff, sizeof(struct fm10k_rx_buffer));
+ *new_buff = *old_buff;
/* sync the buffer for use by the device */
dma_sync_single_range_for_device(rx_ring->dev, old_buff->dma,
@@ -203,12 +202,17 @@ static void fm10k_reuse_rx_page(struct fm10k_ring *rx_ring,
DMA_FROM_DEVICE);
}
+static inline bool fm10k_page_is_reserved(struct page *page)
+{
+ return (page_to_nid(page) != numa_mem_id()) || page->pfmemalloc;
+}
+
static bool fm10k_can_reuse_rx_page(struct fm10k_rx_buffer *rx_buffer,
struct page *page,
unsigned int truesize)
{
/* avoid re-using remote pages */
- if (unlikely(page_to_nid(page) != numa_mem_id()))
+ if (unlikely(fm10k_page_is_reserved(page)))
return false;
#if (PAGE_SIZE < 8192)
@@ -218,22 +222,19 @@ static bool fm10k_can_reuse_rx_page(struct fm10k_rx_buffer *rx_buffer,
/* flip page offset to other buffer */
rx_buffer->page_offset ^= FM10K_RX_BUFSZ;
-
- /* Even if we own the page, we are not allowed to use atomic_set()
- * This would break get_page_unless_zero() users.
- */
- atomic_inc(&page->_count);
#else
/* move offset up to the next cache line */
rx_buffer->page_offset += truesize;
if (rx_buffer->page_offset > (PAGE_SIZE - FM10K_RX_BUFSZ))
return false;
-
- /* bump ref count on page before it is given to the stack */
- get_page(page);
#endif
+ /* Even if we own the page, we are not allowed to use atomic_set()
+ * This would break get_page_unless_zero() users.
+ */
+ atomic_inc(&page->_count);
+
return true;
}
@@ -270,12 +271,12 @@ static bool fm10k_add_rx_frag(struct fm10k_ring *rx_ring,
memcpy(__skb_put(skb, size), va, ALIGN(size, sizeof(long)));
- /* we can reuse buffer as-is, just make sure it is local */
- if (likely(page_to_nid(page) == numa_mem_id()))
+ /* page is not reserved, we can reuse buffer as-is */
+ if (likely(!fm10k_page_is_reserved(page)))
return true;
/* this page cannot be reused so discard it */
- put_page(page);
+ __free_page(page);
return false;
}
@@ -293,7 +294,6 @@ static struct sk_buff *fm10k_fetch_rx_buffer(struct fm10k_ring *rx_ring,
struct page *page;
rx_buffer = &rx_ring->rx_buffer[rx_ring->next_to_clean];
-
page = rx_buffer->page;
prefetchw(page);
^ permalink raw reply related
* [net-next PATCH 1/4] igb: Clean-up page reuse code
From: Alexander Duyck @ 2014-11-13 16:18 UTC (permalink / raw)
To: netdev; +Cc: jeffrey.t.kirsher
In-Reply-To: <20141113161148.2790.22082.stgit@ahduyck-vm-fedora20>
This patch cleans up the page reuse code getting it into a state where all
the workarounds needed are in place as well as cleaning up a few minor
oversights such as using __free_pages instead of put_page to drop a locally
allocated page.
It also cleans up how we clear the descriptor status bits. Previously they
were zeroed as a part of clearing the hdr_addr. However the hdr_addr is a
64 bit field and 64 bit writes can be a bit more expensive on on 32 bit
systems. Since we are no longer using the header split feature the upper
32 bits of the address no longer need to be cleared. As a result we can
just clear the status bits and leave the length and VLAN fields as-is which
should provide more information in debugging.
Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com>
---
drivers/net/ethernet/intel/igb/igb_main.c | 35 +++++++++++++----------------
1 file changed, 16 insertions(+), 19 deletions(-)
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 1e35fae..ade207d 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -6529,15 +6529,17 @@ static void igb_reuse_rx_page(struct igb_ring *rx_ring,
DMA_FROM_DEVICE);
}
+static inline bool igb_page_is_reserved(struct page *page)
+{
+ return (page_to_nid(page) != numa_mem_id()) || page->pfmemalloc;
+}
+
static bool igb_can_reuse_rx_page(struct igb_rx_buffer *rx_buffer,
struct page *page,
unsigned int truesize)
{
/* avoid re-using remote pages */
- if (unlikely(page_to_nid(page) != numa_node_id()))
- return false;
-
- if (unlikely(page->pfmemalloc))
+ if (unlikely(igb_page_is_reserved(page)))
return false;
#if (PAGE_SIZE < 8192)
@@ -6547,22 +6549,19 @@ static bool igb_can_reuse_rx_page(struct igb_rx_buffer *rx_buffer,
/* flip page offset to other buffer */
rx_buffer->page_offset ^= IGB_RX_BUFSZ;
-
- /* Even if we own the page, we are not allowed to use atomic_set()
- * This would break get_page_unless_zero() users.
- */
- atomic_inc(&page->_count);
#else
/* move offset up to the next cache line */
rx_buffer->page_offset += truesize;
if (rx_buffer->page_offset > (PAGE_SIZE - IGB_RX_BUFSZ))
return false;
-
- /* bump ref count on page before it is given to the stack */
- get_page(page);
#endif
+ /* Even if we own the page, we are not allowed to use atomic_set()
+ * This would break get_page_unless_zero() users.
+ */
+ atomic_inc(&page->_count);
+
return true;
}
@@ -6605,13 +6604,12 @@ static bool igb_add_rx_frag(struct igb_ring *rx_ring,
memcpy(__skb_put(skb, size), va, ALIGN(size, sizeof(long)));
- /* we can reuse buffer as-is, just make sure it is local */
- if (likely((page_to_nid(page) == numa_node_id()) &&
- !page->pfmemalloc))
+ /* 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 */
- put_page(page);
+ __free_page(page);
return false;
}
@@ -6629,7 +6627,6 @@ static struct sk_buff *igb_fetch_rx_buffer(struct igb_ring *rx_ring,
struct page *page;
rx_buffer = &rx_ring->rx_buffer_info[rx_ring->next_to_clean];
-
page = rx_buffer->page;
prefetchw(page);
@@ -7050,8 +7047,8 @@ void igb_alloc_rx_buffers(struct igb_ring *rx_ring, u16 cleaned_count)
i -= rx_ring->count;
}
- /* clear the hdr_addr for the next_to_use descriptor */
- rx_desc->read.hdr_addr = 0;
+ /* clear the status bits for the next_to_use descriptor */
+ rx_desc->wb.upper.status_error = 0;
cleaned_count--;
} while (cleaned_count);
^ permalink raw reply related
* [net-next PATCH 3/4] ixgbe: Clean-up page reuse code
From: Alexander Duyck @ 2014-11-13 16:18 UTC (permalink / raw)
To: netdev; +Cc: Don Skidmore, jeffrey.t.kirsher
In-Reply-To: <20141113161148.2790.22082.stgit@ahduyck-vm-fedora20>
This patch cleans up the page reuse code getting it into a state where all
the workarounds needed are in place as well as cleaning up a few minor
oversights such as using __free_pages instead of put_page to drop a locally
allocated page.
It also cleans up how we clear the descriptor status bits. Previously they
were zeroed as a part of clearing the hdr_addr. However the hdr_addr is a
64 bit field and 64 bit writes can be a bit more expensive on on 32 bit
systems. Since we are no longer using the header split feature the upper
32 bits of the address no longer need to be cleared. As a result we can
just clear the status bits and leave the length and VLAN fields as-is which
should provide more information in debugging.
Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Cc: Don Skidmore <donald.c.skidmore@intel.com>
Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 78 ++++++++++++-------------
1 file changed, 36 insertions(+), 42 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index f5fcba4..a72c852 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -1436,20 +1436,17 @@ static bool ixgbe_alloc_mapped_page(struct ixgbe_ring *rx_ring,
struct ixgbe_rx_buffer *bi)
{
struct page *page = bi->page;
- dma_addr_t dma = bi->dma;
+ dma_addr_t dma;
/* since we are recycling buffers we should seldom need to alloc */
- if (likely(dma))
+ if (likely(page))
return true;
/* alloc new page for storage */
- if (likely(!page)) {
- page = dev_alloc_pages(ixgbe_rx_pg_order(rx_ring));
- if (unlikely(!page)) {
- rx_ring->rx_stats.alloc_rx_page_failed++;
- return false;
- }
- bi->page = page;
+ page = dev_alloc_pages(ixgbe_rx_pg_order(rx_ring));
+ if (unlikely(!page)) {
+ rx_ring->rx_stats.alloc_rx_page_failed++;
+ return false;
}
/* map page for use */
@@ -1462,13 +1459,13 @@ static bool ixgbe_alloc_mapped_page(struct ixgbe_ring *rx_ring,
*/
if (dma_mapping_error(rx_ring->dev, dma)) {
__free_pages(page, ixgbe_rx_pg_order(rx_ring));
- bi->page = NULL;
rx_ring->rx_stats.alloc_rx_page_failed++;
return false;
}
bi->dma = dma;
+ bi->page = page;
bi->page_offset = 0;
return true;
@@ -1512,8 +1509,8 @@ void ixgbe_alloc_rx_buffers(struct ixgbe_ring *rx_ring, u16 cleaned_count)
i -= rx_ring->count;
}
- /* clear the hdr_addr for the next_to_use descriptor */
- rx_desc->read.hdr_addr = 0;
+ /* clear the status bits for the next_to_use descriptor */
+ rx_desc->wb.upper.status_error = 0;
cleaned_count--;
} while (cleaned_count);
@@ -1798,9 +1795,7 @@ static void ixgbe_reuse_rx_page(struct ixgbe_ring *rx_ring,
rx_ring->next_to_alloc = (nta < rx_ring->count) ? nta : 0;
/* transfer page from old buffer to new buffer */
- new_buff->page = old_buff->page;
- new_buff->dma = old_buff->dma;
- new_buff->page_offset = old_buff->page_offset;
+ *new_buff = *old_buff;
/* sync the buffer for use by the device */
dma_sync_single_range_for_device(rx_ring->dev, new_buff->dma,
@@ -1809,6 +1804,11 @@ static void ixgbe_reuse_rx_page(struct ixgbe_ring *rx_ring,
DMA_FROM_DEVICE);
}
+static inline bool ixgbe_page_is_reserved(struct page *page)
+{
+ return (page_to_nid(page) != numa_mem_id()) || page->pfmemalloc;
+}
+
/**
* ixgbe_add_rx_frag - Add contents of Rx buffer to sk_buff
* @rx_ring: rx descriptor ring to transact packets on
@@ -1844,12 +1844,12 @@ static bool ixgbe_add_rx_frag(struct ixgbe_ring *rx_ring,
memcpy(__skb_put(skb, size), va, ALIGN(size, sizeof(long)));
- /* we can reuse buffer as-is, just make sure it is local */
- if (likely(page_to_nid(page) == numa_node_id()))
+ /* page is not reserved, we can reuse buffer as-is */
+ if (likely(!ixgbe_page_is_reserved(page)))
return true;
/* this page cannot be reused so discard it */
- put_page(page);
+ __free_pages(page, ixgbe_rx_pg_order(rx_ring));
return false;
}
@@ -1857,7 +1857,7 @@ static bool ixgbe_add_rx_frag(struct ixgbe_ring *rx_ring,
rx_buffer->page_offset, size, truesize);
/* avoid re-using remote pages */
- if (unlikely(page_to_nid(page) != numa_node_id()))
+ if (unlikely(ixgbe_page_is_reserved(page)))
return false;
#if (PAGE_SIZE < 8192)
@@ -1867,22 +1867,19 @@ static bool ixgbe_add_rx_frag(struct ixgbe_ring *rx_ring,
/* flip page offset to other buffer */
rx_buffer->page_offset ^= truesize;
-
- /* Even if we own the page, we are not allowed to use atomic_set()
- * This would break get_page_unless_zero() users.
- */
- atomic_inc(&page->_count);
#else
/* move offset up to the next cache line */
rx_buffer->page_offset += truesize;
if (rx_buffer->page_offset > last_offset)
return false;
-
- /* bump ref count on page before it is given to the stack */
- get_page(page);
#endif
+ /* Even if we own the page, we are not allowed to use atomic_set()
+ * This would break get_page_unless_zero() users.
+ */
+ atomic_inc(&page->_count);
+
return true;
}
@@ -1945,6 +1942,8 @@ dma_sync:
rx_buffer->page_offset,
ixgbe_rx_bufsz(rx_ring),
DMA_FROM_DEVICE);
+
+ rx_buffer->skb = NULL;
}
/* pull page into skb */
@@ -1962,8 +1961,6 @@ dma_sync:
}
/* clear contents of buffer_info */
- rx_buffer->skb = NULL;
- rx_buffer->dma = 0;
rx_buffer->page = NULL;
return skb;
@@ -4345,29 +4342,26 @@ static void ixgbe_clean_rx_ring(struct ixgbe_ring *rx_ring)
/* Free all the Rx ring sk_buffs */
for (i = 0; i < rx_ring->count; i++) {
- struct ixgbe_rx_buffer *rx_buffer;
+ struct ixgbe_rx_buffer *rx_buffer = &rx_ring->rx_buffer_info[i];
- rx_buffer = &rx_ring->rx_buffer_info[i];
if (rx_buffer->skb) {
struct sk_buff *skb = rx_buffer->skb;
- if (IXGBE_CB(skb)->page_released) {
+ if (IXGBE_CB(skb)->page_released)
dma_unmap_page(dev,
IXGBE_CB(skb)->dma,
ixgbe_rx_bufsz(rx_ring),
DMA_FROM_DEVICE);
- IXGBE_CB(skb)->page_released = false;
- }
dev_kfree_skb(skb);
rx_buffer->skb = NULL;
}
- if (rx_buffer->dma)
- dma_unmap_page(dev, rx_buffer->dma,
- ixgbe_rx_pg_size(rx_ring),
- DMA_FROM_DEVICE);
- rx_buffer->dma = 0;
- if (rx_buffer->page)
- __free_pages(rx_buffer->page,
- ixgbe_rx_pg_order(rx_ring));
+
+ if (!rx_buffer->page)
+ continue;
+
+ dma_unmap_page(dev, rx_buffer->dma,
+ ixgbe_rx_pg_size(rx_ring), DMA_FROM_DEVICE);
+ __free_pages(rx_buffer->page, ixgbe_rx_pg_order(rx_ring));
+
rx_buffer->page = NULL;
}
^ permalink raw reply related
* [net-next PATCH 4/4] ixgbe: Remove tail write abstraction and add missing barrier
From: Alexander Duyck @ 2014-11-13 16:18 UTC (permalink / raw)
To: netdev; +Cc: Don Skidmore, jeffrey.t.kirsher
In-Reply-To: <20141113161148.2790.22082.stgit@ahduyck-vm-fedora20>
This change cleans up the tail writes for the ixgbe descriptor queues. The
current implementation had me confused as I wasn't sure if it was still
making use of the suprise remove logic or not.
It also adds the mmiowb which is needed on ia64, mips, and a couple other
architectures in order to synchronize the MMIO writes with the Tx queue
_xmit_lock spinlock.
Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Cc: Don Skidmore <donald.c.skidmore@intel.com>
Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe.h | 5 ---
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 40 +++++++++++++------------
2 files changed, 20 insertions(+), 25 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
index 5032a60..86fa607 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h
@@ -553,11 +553,6 @@ static inline u16 ixgbe_desc_unused(struct ixgbe_ring *ring)
return ((ntc > ntu) ? 0 : ring->count) + ntc - ntu - 1;
}
-static inline void ixgbe_write_tail(struct ixgbe_ring *ring, u32 value)
-{
- writel(value, ring->tail);
-}
-
#define IXGBE_RX_DESC(R, i) \
(&(((union ixgbe_adv_rx_desc *)((R)->desc))[i]))
#define IXGBE_TX_DESC(R, i) \
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index a72c852..6b29dfe 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -1416,22 +1416,6 @@ static inline void ixgbe_rx_checksum(struct ixgbe_ring *ring,
skb->ip_summed = CHECKSUM_UNNECESSARY;
}
-static inline void ixgbe_release_rx_desc(struct ixgbe_ring *rx_ring, u32 val)
-{
- rx_ring->next_to_use = val;
-
- /* update next to alloc since we have filled the ring */
- rx_ring->next_to_alloc = val;
- /*
- * Force memory writes to complete before letting h/w
- * know there are new descriptors to fetch. (Only
- * applicable for weak-ordered memory model archs,
- * such as IA-64).
- */
- wmb();
- ixgbe_write_tail(rx_ring, val);
-}
-
static bool ixgbe_alloc_mapped_page(struct ixgbe_ring *rx_ring,
struct ixgbe_rx_buffer *bi)
{
@@ -1517,8 +1501,20 @@ void ixgbe_alloc_rx_buffers(struct ixgbe_ring *rx_ring, u16 cleaned_count)
i += rx_ring->count;
- if (rx_ring->next_to_use != i)
- ixgbe_release_rx_desc(rx_ring, i);
+ if (rx_ring->next_to_use != i) {
+ rx_ring->next_to_use = i;
+
+ /* update next to alloc since we have filled the ring */
+ rx_ring->next_to_alloc = i;
+
+ /* Force memory writes to complete before letting h/w
+ * know there are new descriptors to fetch. (Only
+ * applicable for weak-ordered memory model archs,
+ * such as IA-64).
+ */
+ wmb();
+ writel(i, rx_ring->tail);
+ }
}
static void ixgbe_set_rsc_gso_size(struct ixgbe_ring *ring,
@@ -6955,8 +6951,12 @@ static void ixgbe_tx_map(struct ixgbe_ring *tx_ring,
ixgbe_maybe_stop_tx(tx_ring, DESC_NEEDED);
if (netif_xmit_stopped(txring_txq(tx_ring)) || !skb->xmit_more) {
- /* notify HW of packet */
- ixgbe_write_tail(tx_ring, i);
+ writel(i, tx_ring->tail);
+
+ /* we need this if more than one processor can write to our tail
+ * at a time, it synchronizes IO on IA64/Altix systems
+ */
+ mmiowb();
}
return;
^ permalink raw reply related
* CitiBank Project
From: Jin.Zhang @ 2014-11-13 16:23 UTC (permalink / raw)
To: netdev
Dear Attorney,
This is to inquire if your law firm handles Purchase and Sale Agreements in your area in the United States.
Regards,
Anthony Miyamoto
President & CEO
MIYAMOTO Industrial Co. Ltd.
6-4-10
NANKO-NAKA, SUMINOE-KU,
OSAKA, JAPAN 559-0033
^ permalink raw reply
* CitiBank Project
From: Jin.Zhang @ 2014-11-13 16:34 UTC (permalink / raw)
To: netdev
Dear Attorney,
This is to inquire if your law firm handles Purchase and Sale Agreements in your area in the United States.
Regards,
Anthony Miyamoto
President & CEO
MIYAMOTO Industrial Co. Ltd.
6-4-10
NANKO-NAKA, SUMINOE-KU,
OSAKA, JAPAN 559-0033
^ permalink raw reply
* [PATCH net v3] vxlan: Do not reuse sockets for a different address family
From: Marcelo Ricardo Leitner @ 2014-11-13 16:43 UTC (permalink / raw)
To: netdev; +Cc: stephen, sergei.shtylyov
Currently, we only match against local port number in order to reuse
socket. But if this new vxlan wants an IPv6 socket and a IPv4 one bound
to that port, vxlan will reuse an IPv4 socket as IPv6 and a panic will
follow. The following steps reproduce it:
# ip link add vxlan6 type vxlan id 42 group 229.10.10.10 \
srcport 5000 6000 dev eth0
# ip link add vxlan7 type vxlan id 43 group ff0e::110 \
srcport 5000 6000 dev eth0
# ip link set vxlan6 up
# ip link set vxlan7 up
<panic>
[ 4.187481] BUG: unable to handle kernel NULL pointer dereference at 0000000000000058
...
[ 4.188076] Call Trace:
[ 4.188085] [<ffffffff81667c4a>] ? ipv6_sock_mc_join+0x3a/0x630
[ 4.188098] [<ffffffffa05a6ad6>] vxlan_igmp_join+0x66/0xd0 [vxlan]
[ 4.188113] [<ffffffff810a3430>] process_one_work+0x220/0x710
[ 4.188125] [<ffffffff810a33c4>] ? process_one_work+0x1b4/0x710
[ 4.188138] [<ffffffff810a3a3b>] worker_thread+0x11b/0x3a0
[ 4.188149] [<ffffffff810a3920>] ? process_one_work+0x710/0x710
So address family must also match in order to reuse a socket.
Reported-by: Jean-Tsung Hsiao <jhsiao@redhat.com>
Signed-off-by: Marcelo Ricardo Leitner <mleitner@redhat.com>
---
drivers/net/vxlan.c | 29 +++++++++++++++++++----------
1 file changed, 19 insertions(+), 10 deletions(-)
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 0ab411461d2ec2a0f57930a663f2e89842632a92..7ef7b0ead33c464bf5314c048d32127fe54335e1 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -275,13 +275,15 @@ static inline struct vxlan_rdst *first_remote_rtnl(struct vxlan_fdb *fdb)
return list_first_entry(&fdb->remotes, struct vxlan_rdst, list);
}
-/* Find VXLAN socket based on network namespace and UDP port */
-static struct vxlan_sock *vxlan_find_sock(struct net *net, __be16 port)
+/* Find VXLAN socket based on network namespace, address family and UDP port */
+static struct vxlan_sock *vxlan_find_sock(struct net *net,
+ sa_family_t family, __be16 port)
{
struct vxlan_sock *vs;
hlist_for_each_entry_rcu(vs, vs_head(net, port), hlist) {
- if (inet_sk(vs->sock->sk)->inet_sport == port)
+ if (inet_sk(vs->sock->sk)->inet_sport == port &&
+ inet_sk(vs->sock->sk)->sk.sk_family == family)
return vs;
}
return NULL;
@@ -300,11 +302,12 @@ static struct vxlan_dev *vxlan_vs_find_vni(struct vxlan_sock *vs, u32 id)
}
/* Look up VNI in a per net namespace table */
-static struct vxlan_dev *vxlan_find_vni(struct net *net, u32 id, __be16 port)
+static struct vxlan_dev *vxlan_find_vni(struct net *net, u32 id,
+ sa_family_t family, __be16 port)
{
struct vxlan_sock *vs;
- vs = vxlan_find_sock(net, port);
+ vs = vxlan_find_sock(net, family, port);
if (!vs)
return NULL;
@@ -1771,7 +1774,8 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
struct vxlan_dev *dst_vxlan;
ip_rt_put(rt);
- dst_vxlan = vxlan_find_vni(vxlan->net, vni, dst_port);
+ dst_vxlan = vxlan_find_vni(vxlan->net, vni,
+ dst->sa.sa_family, dst_port);
if (!dst_vxlan)
goto tx_error;
vxlan_encap_bypass(skb, vxlan, dst_vxlan);
@@ -1825,7 +1829,8 @@ static void vxlan_xmit_one(struct sk_buff *skb, struct net_device *dev,
struct vxlan_dev *dst_vxlan;
dst_release(ndst);
- dst_vxlan = vxlan_find_vni(vxlan->net, vni, dst_port);
+ dst_vxlan = vxlan_find_vni(vxlan->net, vni,
+ dst->sa.sa_family, dst_port);
if (!dst_vxlan)
goto tx_error;
vxlan_encap_bypass(skb, vxlan, dst_vxlan);
@@ -1985,13 +1990,15 @@ static int vxlan_init(struct net_device *dev)
struct vxlan_dev *vxlan = netdev_priv(dev);
struct vxlan_net *vn = net_generic(vxlan->net, vxlan_net_id);
struct vxlan_sock *vs;
+ bool ipv6 = vxlan->flags & VXLAN_F_IPV6;
dev->tstats = netdev_alloc_pcpu_stats(struct pcpu_sw_netstats);
if (!dev->tstats)
return -ENOMEM;
spin_lock(&vn->sock_lock);
- vs = vxlan_find_sock(vxlan->net, vxlan->dst_port);
+ vs = vxlan_find_sock(vxlan->net, ipv6 ? AF_INET6 : AF_INET,
+ vxlan->dst_port);
if (vs) {
/* If we have a socket with same port already, reuse it */
atomic_inc(&vs->refcnt);
@@ -2385,6 +2392,7 @@ struct vxlan_sock *vxlan_sock_add(struct net *net, __be16 port,
{
struct vxlan_net *vn = net_generic(net, vxlan_net_id);
struct vxlan_sock *vs;
+ bool ipv6 = flags & VXLAN_F_IPV6;
vs = vxlan_socket_create(net, port, rcv, data, flags);
if (!IS_ERR(vs))
@@ -2394,7 +2402,7 @@ struct vxlan_sock *vxlan_sock_add(struct net *net, __be16 port,
return vs;
spin_lock(&vn->sock_lock);
- vs = vxlan_find_sock(net, port);
+ vs = vxlan_find_sock(net, ipv6 ? AF_INET6 : AF_INET, port);
if (vs) {
if (vs->rcv == rcv)
atomic_inc(&vs->refcnt);
@@ -2553,7 +2561,8 @@ static int vxlan_newlink(struct net *net, struct net_device *dev,
nla_get_u8(data[IFLA_VXLAN_UDP_ZERO_CSUM6_RX]))
vxlan->flags |= VXLAN_F_UDP_ZERO_CSUM6_RX;
- if (vxlan_find_vni(net, vni, vxlan->dst_port)) {
+ if (vxlan_find_vni(net, vni, use_ipv6 ? AF_INET6 : AF_INET,
+ vxlan->dst_port)) {
pr_info("duplicate VNI %u\n", vni);
return -EEXIST;
}
--
1.9.3
^ permalink raw reply related
* Re: [net-next PATCH 0/4] Clean up intel driver page reuse Rx code
From: Jeff Kirsher @ 2014-11-13 16:49 UTC (permalink / raw)
To: Alexander Duyck; +Cc: netdev
In-Reply-To: <20141113161148.2790.22082.stgit@ahduyck-vm-fedora20>
[-- Attachment #1: Type: text/plain, Size: 1801 bytes --]
On Thu, 2014-11-13 at 08:18 -0800, Alexander Duyck wrote:
> This patch series cleans up the page reuse and Rx path so that all of the
> fixes that have been applied to any one driver are now in place for igb,
> fm10k, and ixgbe. It occured to me that fm10k was missing the pfmemalloc
> bits, and ixgbe was missing some logic that reduced the number of writes
> needed as well as the pfmemalloc fix.
>
> In addition ixgbe was carrying around some mostly-dead code that was
> wrapping a call to writel. I removed it since it masked the fact that
> ixgbe was missing the mmiowb it was supposed to have between the tail write
> and the Tx queue lock release to prevent the MMIO access from racing
> between CPUs.
>
> Also one change made to all 3 drivers is that we now only overwrite 3 of
> the 4 DWORDs in the Rx descriptor after allocation. The last DWORD
> contains the upper 32 bits of the header address on fetch, and the length
> and vlan on writeback. Since we are no longer using header split we don't
> need to clear it prior to descriptor fetch as it is unused so we can save
> ourselves a cycle on 32b systems by just leaving it untouched.
>
> ---
>
> Alexander Duyck (4):
> igb: Clean-up page reuse code
> fm10k: Clean-up page reuse code
> ixgbe: Clean-up page reuse code
> ixgbe: Remove tail write abstraction and add missing barrier
>
>
> drivers/net/ethernet/intel/fm10k/fm10k_main.c | 34 ++++---
> drivers/net/ethernet/intel/igb/igb_main.c | 35 +++----
> drivers/net/ethernet/intel/ixgbe/ixgbe.h | 5 -
> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 118 ++++++++++++-------------
> 4 files changed, 89 insertions(+), 103 deletions(-)
>
> --
Thanks Alex, I will add your series to my queue.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH net-next 1/1] ipvlan: Initial check-in of the IPVLAN driver.
From: Mahesh Bandewar @ 2014-11-13 16:50 UTC (permalink / raw)
To: Pavel Emelyanov
Cc: netdev, Eric Dumazet, Maciej Zenczykowski, Laurent Chavey,
Tim Hockin, David Miller, Brandon Philips
In-Reply-To: <546490E6.2050509@parallels.com>
On Thu, Nov 13, 2014 at 3:07 AM, Pavel Emelyanov <xemul@parallels.com> wrote:
>>>> +static int ipvlan_link_new(struct net *src_net, struct net_device *dev,
>>>> + struct nlattr *tb[], struct nlattr *data[])
>>>> +{
>>>> + struct ipvl_dev *ipvlan = netdev_priv(dev);
>>>> + struct ipvl_port *port;
>>>> + struct net_device *phy_dev;
>>>> + int err;
>>>> +
>>>> + ipvlan_dbg(3, "%s[%d]: Entering...\n", __func__, __LINE__);
>>>> + if (!tb[IFLA_LINK]) {
>>>> + ipvlan_dbg(3, "%s[%d]: Returning -EINVAL...\n",
>>>> + __func__, __LINE__);
>>>> + return -EINVAL;
>>>> + }
>>>> +
>>>> + phy_dev = __dev_get_by_index(src_net, nla_get_u32(tb[IFLA_LINK]));
>>>> + if (phy_dev == NULL) {
>>>> + ipvlan_dbg(3, "%s[%d]: Returning -ENODEV...\n",
>>>> + __func__, __LINE__);
>>>> + return -ENODEV;
>>>> + }
>>>> +
>>>> + /* TODO will someone try creating ipvlan-dev on an ipvlan-virtual dev?*/
>>>> + if (!ipvlan_dev_master(phy_dev)) {
>>>> + err = ipvlan_port_create(phy_dev);
>>>> + if (err < 0) {
>>>> + ipvlan_dbg(3, "%s[%d]: Returning error (%d)...\n",
>>>> + __func__, __LINE__, err);
>>>> + return err;
>>>> + }
>>>> + }
>>>> +
>>>> + port = ipvlan_port_get_rtnl(phy_dev);
>>>> + /* Get the mode if specified. */
>>>> + if (data && data[IFLA_IPVLAN_MODE])
>>>> + port->mode = nla_get_u16(data[IFLA_IPVLAN_MODE]);
>>>
>>> Should the invalid value be checked here? There are places
>>> where we BUG() in mode being "unknown".
>>>
>> Assuming the calls come over netlink, the ".validate" will be called
>> before ".newlink", so that would be unnecessary, isn't it?
>
> Yes, you're right. I've missed the validate callback.
>
>>>> + break;
>>>> +
>>>
>>>> +static int ipvlan_addr4_event(struct notifier_block *unused,
>>>> + unsigned long event, void *ptr)
>>>> +{
>>>> + struct in_ifaddr *if4 = (struct in_ifaddr *)ptr;
>>>> + struct net_device *dev = (struct net_device *)if4->ifa_dev->dev;
>>>> + struct ipvl_dev *ipvlan = netdev_priv(dev);
>>>> + struct in_addr ip4_addr;
>>>> +
>>>> + ipvlan_dbg(3, "%s[%d]: Entering...\n", __func__, __LINE__);
>>>> + if (!ipvlan_dev_slave(dev))
>>>> + return NOTIFY_DONE;
>>>> +
>>>> + if (!ipvlan || !ipvlan->port)
>>>> + return NOTIFY_DONE;
>>>> +
>>>> + switch (event) {
>>>> + case NETDEV_UP:
>>>
>>> Can it be (in the future) somehow restricted so that net-namespace wouldn't
>>> be able to assign arbitrary IP address here? One of the reasons for using
>>> such devices is to enforce the container to use the IP address given from
>>> the host.
>>>
>> Probably this could be a config (sysfs?) entry which would lockup the
>> config coming from ns when set. So code could look like -
>> case NETDEV_UP:
>> if (!restrict_ns_config) {
>> ...
>> }
>> break;
>
> Maybe introduce some "lock" call for ipvlan device after which no new IP addresses
> can be assigned? And the configuration would look like
>
> 1. create ipvlan
> 2. move to proper net namespace
> 3. add addresses
> 4. lock
>
> ?
Yes. Exporting this "locked" property on the master device so that it
can be controlled from masters' net-ns. Only thing we have to ensure
is that both possibilities are allowed i.e. trusted ns where config do
not need to be locked as well as untrusted/hostile ns where one can
lock it down. However this is a future enhancement and if your
implementation idea is different than this concept; we can discuss it
at the time of implementation.
>
> Thanks,
> Pavel
>
^ permalink raw reply
* Re: [PATCH V4 2/3] can: m_can: update to support CAN FD features
From: Oliver Hartkopp @ 2014-11-13 16:56 UTC (permalink / raw)
To: Marc Kleine-Budde, Dong Aisheng, linux-can
Cc: wg, varkabhadram, netdev, linux-arm-kernel
In-Reply-To: <54648382.9080105@pengutronix.de>
On 11/13/2014 11:10 AM, Marc Kleine-Budde wrote:
> On 11/07/2014 09:45 AM, Dong Aisheng wrote:
>>
>> - if (id & RX_BUF_RTR) {
>> + if (id & RX_BUF_ESI) {
>> + cf->flags |= CANFD_ESI;
>> + netdev_dbg(dev, "ESI Error\n");
>> + }
>> +
>> + if (!(dlc & RX_BUF_EDL) && (id & RX_BUF_RTR)) {
>> cf->can_id |= CAN_RTR_FLAG;
>
> I just noticed, that you don't set the cf->dlc (or cf->len) in the RTR
> case. Please create a separate patch that fixes this problem.
>
>> } else {
>> id = m_can_fifo_read(priv, fgi, M_CAN_FIFO_DLC);
>> - cf->can_dlc = get_can_dlc((id >> 16) & 0x0F);
>> - *(u32 *)(cf->data + 0) = m_can_fifo_read(priv, fgi,
>> - M_CAN_FIFO_DATA(0));
>> - *(u32 *)(cf->data + 4) = m_can_fifo_read(priv, fgi,
>> - M_CAN_FIFO_DATA(1));
>> + if (dlc & RX_BUF_EDL)
>> + cf->len = can_dlc2len((id >> 16) & 0x0F);
>> + else
>> + cf->len = get_can_dlc((id >> 16) & 0x0F);
>> +
Grr. I missed that one too :-(
Thanks for catching it.
As you committed patch 1 & 3 you expect a new single patch containing the
(fixed) content of this patch 2, right?
Regards,
Oliver
^ permalink raw reply
* Re: [PATCH net-next 1/1] ipvlan: Initial check-in of the IPVLAN driver.
From: Pavel Emelyanov @ 2014-11-13 15:57 UTC (permalink / raw)
To: Mahesh Bandewar
Cc: netdev, Eric Dumazet, Maciej Zenczykowski, Laurent Chavey,
Tim Hockin, David Miller, Brandon Philips
In-Reply-To: <CAF2d9jjmFymJdZmnfDCCUTRSSm-63RPaEFdB3TeYOq9S3jcPOA@mail.gmail.com>
>> Maybe introduce some "lock" call for ipvlan device after which no new IP addresses
>> can be assigned? And the configuration would look like
>>
>> 1. create ipvlan
>> 2. move to proper net namespace
>> 3. add addresses
>> 4. lock
>>
>> ?
> Yes. Exporting this "locked" property on the master device so that it
> can be controlled from masters' net-ns. Only thing we have to ensure
> is that both possibilities are allowed i.e. trusted ns where config do
> not need to be locked as well as untrusted/hostile ns where one can
> lock it down. However this is a future enhancement and if your
> implementation idea is different than this concept; we can discuss it
> at the time of implementation.
Sure. It's not a wish for the very first version of the set, it can
be added later.
Thanks,
Pavel
^ permalink raw reply
* Re: [patch net-next 2/2] sched: introduce vlan action
From: Cong Wang @ 2014-11-13 17:06 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: Jiri Pirko, netdev, David Miller, Pravin B Shelar, Tom Herbert,
Eric Dumazet, Willem de Bruijn, Daniel Borkmann, mst,
Florian Westphal, Paul.Durrant, Thomas Graf
In-Reply-To: <5463522C.1030104@mojatatu.com>
On Wed, Nov 12, 2014 at 4:27 AM, Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>
> Cong,
> I think it is better to have all these "tunneling" activities
> as a separate action each. It is cleaner from a usability perspective.
> [e.g. it is not hard to express nat action with pedit action or take
> checksum out of nat since we have a csum action), but makes sense to
> have it separate)].
>
Sounds good.
^ permalink raw reply
* Re: [patch net-next v2 4/4] sched: introduce vlan action
From: Cong Wang @ 2014-11-13 17:07 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, David Miller, Jamal Hadi Salim, Pravin B Shelar,
Tom Herbert, Eric Dumazet, Willem de Bruijn, Daniel Borkmann, mst,
Florian Westphal, Paul.Durrant, Thomas Graf
In-Reply-To: <1415803950-9838-4-git-send-email-jiri@resnulli.us>
On Wed, Nov 12, 2014 at 6:52 AM, Jiri Pirko <jiri@resnulli.us> wrote:
> This tc action allows to work with vlan tagged skbs. Two supported
> sub-actions are header pop and header push.
>
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
Reviewed-by: Cong Wang <cwang@twopensource.com>
^ permalink raw reply
* Re: [E1000-devel] [PATCH] ixgbe: make VLAN filter conditional in SR-IOV case
From: Jeff Kirsher @ 2014-11-13 17:08 UTC (permalink / raw)
To: Hiroshi Shimamoto
Cc: e1000-devel@lists.sourceforge.net, netdev@vger.kernel.org,
Choi, Sy Jong, Hayato Momma, linux-kernel@vger.kernel.org
In-Reply-To: <7F861DC0615E0C47A872E6F3C5FCDDBD05D9D336@BPXM14GP.gisp.nec.co.jp>
[-- Attachment #1: Type: text/plain, Size: 1271 bytes --]
On Thu, 2014-11-13 at 08:28 +0000, Hiroshi Shimamoto wrote:
> From: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
>
> Disable hardware VLAN filtering if netdev->features VLAN flag is
> dropped.
>
> In SR-IOV case, there is a use case which needs to disable VLAN
> filter.
> For example, we need to make a network function with VF in virtualized
> environment. That network function may be a software switch, a router
> or etc. It means that that network function will be an end point which
> terminates many VLANs.
>
> In the current implementation, VLAN filtering always be turned on and
> VF can receive only 63 VLANs. It means that only 63 VLANs can be used
> and it's not enough at all for building a virtual router.
>
> With this patch, if the user turns VLAN filtering off on the host, VF
> can receive every VLAN packet.
> The behavior is changed only if VLAN filtering is turned off by
> ethtool.
>
> Signed-off-by: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
> CC: Choi, Sy Jong <sy.jong.choi@intel.com>
> ---
> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 10 ++++++++++
> drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 4 ++++
> 2 files changed, 14 insertions(+)
Thanks Hiroshi, I will add your patch to my queue.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* [PATCH net-next] tcp: limit GSO packets to half cwnd
From: Eric Dumazet @ 2014-11-13 17:45 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Neal Cardwell, Yuchung Cheng, Nandita Dukkipati
From: Eric Dumazet <edumazet@google.com>
In DC world, GSO packets initially cooked by tcp_sendmsg() are usually
big, as sk_pacing_rate is high.
When network is congested, cwnd can be smaller than the GSO packets
found in socket write queue. tcp_write_xmit() splits GSO packets
using the available cwnd, and we end up sending a single GSO packet,
consuming all available cwnd.
With GRO aggregation on the receiver, we might handle a single GRO
packet, sending back a single ACK.
1) This single ACK might be lost
TLP or RTO are forced to attempt a retransmit.
2) This ACK releases a full cwnd, sender sends another big GSO packet,
in a ping pong mode.
This behavior does not fill the pipes in the best way, because of
scheduling artifacts.
Make sure we always have at least two GSO packets in flight.
This allows us to safely increase GRO efficiency without risking
spurious retransmits.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/ipv4/tcp_output.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 0b88158dd4a70d5007e79f0d8251fee2f7c6c7f8..eb73a1dccf56b823a45c0ca034e40dc50fc48068 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -1562,7 +1562,7 @@ static unsigned int tcp_mss_split_point(const struct sock *sk,
static inline unsigned int tcp_cwnd_test(const struct tcp_sock *tp,
const struct sk_buff *skb)
{
- u32 in_flight, cwnd;
+ u32 in_flight, cwnd, halfcwnd;
/* Don't be strict about the congestion window for the final FIN. */
if ((TCP_SKB_CB(skb)->tcp_flags & TCPHDR_FIN) &&
@@ -1571,10 +1571,14 @@ static inline unsigned int tcp_cwnd_test(const struct tcp_sock *tp,
in_flight = tcp_packets_in_flight(tp);
cwnd = tp->snd_cwnd;
- if (in_flight < cwnd)
- return (cwnd - in_flight);
+ if (in_flight >= cwnd)
+ return 0;
- return 0;
+ /* For better scheduling, ensure we have at least
+ * 2 GSO packets in flight.
+ */
+ halfcwnd = max(cwnd >> 1, 1U);
+ return min(halfcwnd, cwnd - in_flight);
}
/* Initialize TSO state of a skb.
^ permalink raw reply related
* Re: [PATCH net-next] tcp: limit GSO packets to half cwnd
From: Dave Taht @ 2014-11-13 18:16 UTC (permalink / raw)
To: Eric Dumazet
Cc: David Miller, netdev, Neal Cardwell, Yuchung Cheng,
Nandita Dukkipati
In-Reply-To: <1415900722.17262.22.camel@edumazet-glaptop2.roam.corp.google.com>
On Thu, Nov 13, 2014 at 9:45 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> In DC world, GSO packets initially cooked by tcp_sendmsg() are usually
> big, as sk_pacing_rate is high.
>
> When network is congested, cwnd can be smaller than the GSO packets
> found in socket write queue. tcp_write_xmit() splits GSO packets
> using the available cwnd, and we end up sending a single GSO packet,
> consuming all available cwnd.
My take on this is that this will also help reduce inter-flow latency
on devices that are running GSO at 100Mbit or lower speeds. (?)
I have really liked many of the patches entering netdev in this cycle!
(could this be a tunable instead to split on 1/4th for example?)
That does not prevent me from wishing, abstractly, to strand all the
tcp development orgs of the world on a tropic island with a 10mbit
uplink, or on a gbus, endlessly circling san francisco... until more
low bandwidth with high latency problems are resolved.
:)
> With GRO aggregation on the receiver, we might handle a single GRO
> packet, sending back a single ACK.
>
> 1) This single ACK might be lost
> TLP or RTO are forced to attempt a retransmit.
> 2) This ACK releases a full cwnd, sender sends another big GSO packet,
> in a ping pong mode.
>
> This behavior does not fill the pipes in the best way, because of
> scheduling artifacts.
>
> Make sure we always have at least two GSO packets in flight.
>
> This allows us to safely increase GRO efficiency without risking
> spurious retransmits.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> ---
> net/ipv4/tcp_output.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> index 0b88158dd4a70d5007e79f0d8251fee2f7c6c7f8..eb73a1dccf56b823a45c0ca034e40dc50fc48068 100644
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
> @@ -1562,7 +1562,7 @@ static unsigned int tcp_mss_split_point(const struct sock *sk,
> static inline unsigned int tcp_cwnd_test(const struct tcp_sock *tp,
> const struct sk_buff *skb)
> {
> - u32 in_flight, cwnd;
> + u32 in_flight, cwnd, halfcwnd;
>
> /* Don't be strict about the congestion window for the final FIN. */
> if ((TCP_SKB_CB(skb)->tcp_flags & TCPHDR_FIN) &&
> @@ -1571,10 +1571,14 @@ static inline unsigned int tcp_cwnd_test(const struct tcp_sock *tp,
>
> in_flight = tcp_packets_in_flight(tp);
> cwnd = tp->snd_cwnd;
> - if (in_flight < cwnd)
> - return (cwnd - in_flight);
> + if (in_flight >= cwnd)
> + return 0;
>
> - return 0;
> + /* For better scheduling, ensure we have at least
> + * 2 GSO packets in flight.
> + */
> + halfcwnd = max(cwnd >> 1, 1U);
> + return min(halfcwnd, cwnd - in_flight);
> }
>
> /* Initialize TSO state of a skb.
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Dave Täht
http://www.bufferbloat.net/projects/bloat/wiki/Upcoming_Talks
^ permalink raw reply
* Re: [PATCH net-next] tcp: limit GSO packets to half cwnd
From: Neal Cardwell @ 2014-11-13 18:24 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev, Yuchung Cheng, Nandita Dukkipati
In-Reply-To: <1415900722.17262.22.camel@edumazet-glaptop2.roam.corp.google.com>
On Thu, Nov 13, 2014 at 12:45 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> In DC world, GSO packets initially cooked by tcp_sendmsg() are usually
> big, as sk_pacing_rate is high.
>
> When network is congested, cwnd can be smaller than the GSO packets
> found in socket write queue. tcp_write_xmit() splits GSO packets
> using the available cwnd, and we end up sending a single GSO packet,
> consuming all available cwnd.
>
> With GRO aggregation on the receiver, we might handle a single GRO
> packet, sending back a single ACK.
>
> 1) This single ACK might be lost
> TLP or RTO are forced to attempt a retransmit.
> 2) This ACK releases a full cwnd, sender sends another big GSO packet,
> in a ping pong mode.
>
> This behavior does not fill the pipes in the best way, because of
> scheduling artifacts.
>
> Make sure we always have at least two GSO packets in flight.
>
> This allows us to safely increase GRO efficiency without risking
> spurious retransmits.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> ---
> net/ipv4/tcp_output.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
Acked-by: Neal Cardwell <ncardwell@google.com>
Thanks, Eric!
neal
^ permalink raw reply
* Re: [PATCH] net: skb_fclone_busy() needs to detect orphaned skb
From: Luis Henriques @ 2014-11-13 19:15 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev, Neal Cardwell, Joseph Salisbury
In-Reply-To: <1414690354.9028.9.camel@edumazet-glaptop2.roam.corp.google.com>
Hi Eric,
On Thu, Oct 30, 2014 at 10:32:34AM -0700, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> Some drivers are unable to perform TX completions in a bound time.
> They instead call skb_orphan()
>
> Problem is skb_fclone_busy() has to detect this case, otherwise
> we block TCP retransmits and can freeze unlucky tcp sessions on
> mostly idle hosts.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Fixes: 1f3279ae0c13 ("tcp: avoid retransmits of TCP packets hanging in host queues")
> ---
> This is a stable candidate.
> This problem is known to hurt users of linux-3.16 kernels used by guests kernels.
> David, I can provide backports if you want.
> Thanks !
>
We got a bug report[0] where a backport for 3.16 was provided. Since
I couldn't find the original backport post, I'm not sure who's the
actual author. Could you please confirm if this backport is correct?
(I'm copying the patch below).
[0] https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1390604
Cheers,
--
Luís
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 4e4932b5079b..a8794367cd20 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -2082,7 +2082,8 @@ static bool skb_still_in_host_queue(const struct sock *sk,
const struct sk_buff *fclone = skb + 1;
if (unlikely(skb->fclone == SKB_FCLONE_ORIG &&
- fclone->fclone == SKB_FCLONE_CLONE)) {
+ fclone->fclone == SKB_FCLONE_CLONE &&
+ fclone->sk == sk)) {
NET_INC_STATS_BH(sock_net(sk),
LINUX_MIB_TCPSPURIOUS_RTX_HOSTQUEUES);
return true;
> include/linux/skbuff.h | 8 ++++++--
> net/ipv4/tcp_output.c | 2 +-
> net/xfrm/xfrm_policy.c | 2 +-
> 3 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 5884f95ff0e9..6c8b6f604e76 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -799,15 +799,19 @@ struct sk_buff_fclones {
> * @skb: buffer
> *
> * Returns true is skb is a fast clone, and its clone is not freed.
> + * Some drivers call skb_orphan() in their ndo_start_xmit(),
> + * so we also check that this didnt happen.
> */
> -static inline bool skb_fclone_busy(const struct sk_buff *skb)
> +static inline bool skb_fclone_busy(const struct sock *sk,
> + const struct sk_buff *skb)
> {
> const struct sk_buff_fclones *fclones;
>
> fclones = container_of(skb, struct sk_buff_fclones, skb1);
>
> return skb->fclone == SKB_FCLONE_ORIG &&
> - fclones->skb2.fclone == SKB_FCLONE_CLONE;
> + fclones->skb2.fclone == SKB_FCLONE_CLONE &&
> + fclones->skb2.sk == sk;
> }
>
> static inline struct sk_buff *alloc_skb_fclone(unsigned int size,
> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> index 3af21296d967..a3d453b94747 100644
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
> @@ -2126,7 +2126,7 @@ bool tcp_schedule_loss_probe(struct sock *sk)
> static bool skb_still_in_host_queue(const struct sock *sk,
> const struct sk_buff *skb)
> {
> - if (unlikely(skb_fclone_busy(skb))) {
> + if (unlikely(skb_fclone_busy(sk, skb))) {
> NET_INC_STATS_BH(sock_net(sk),
> LINUX_MIB_TCPSPURIOUS_RTX_HOSTQUEUES);
> return true;
> diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
> index 4c4e457e7888..88bf289abdc9 100644
> --- a/net/xfrm/xfrm_policy.c
> +++ b/net/xfrm/xfrm_policy.c
> @@ -1962,7 +1962,7 @@ static int xdst_queue_output(struct sock *sk, struct sk_buff *skb)
> struct xfrm_policy *pol = xdst->pols[0];
> struct xfrm_policy_queue *pq = &pol->polq;
>
> - if (unlikely(skb_fclone_busy(skb))) {
> + if (unlikely(skb_fclone_busy(sk, skb))) {
> kfree_skb(skb);
> return 0;
> }
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH 15/16] netfilter: Replace smp_read_barrier_depends() with lockless_dereference()
From: Pranith Kumar @ 2014-11-13 19:24 UTC (permalink / raw)
To: Pablo Neira Ayuso, Patrick McHardy, Jozsef Kadlecsik,
David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, netfilter-devel, coreteam,
open list:NETWORKING [IPv4/..., open list
Cc: paulmck
In-Reply-To: <1415906662-4576-1-git-send-email-bobby.prani@gmail.com>
Recently lockless_dereference() was added which can be used in place of
hard-coding smp_read_barrier_depends(). The following PATCH makes the change.
Signed-off-by: Pranith Kumar <bobby.prani@gmail.com>
---
net/ipv4/netfilter/arp_tables.c | 3 +--
net/ipv4/netfilter/ip_tables.c | 3 +--
net/ipv6/netfilter/ip6_tables.c | 3 +--
3 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/net/ipv4/netfilter/arp_tables.c b/net/ipv4/netfilter/arp_tables.c
index f95b6f9..fc7533d 100644
--- a/net/ipv4/netfilter/arp_tables.c
+++ b/net/ipv4/netfilter/arp_tables.c
@@ -270,12 +270,11 @@ unsigned int arpt_do_table(struct sk_buff *skb,
local_bh_disable();
addend = xt_write_recseq_begin();
- private = table->private;
/*
* Ensure we load private-> members after we've fetched the base
* pointer.
*/
- smp_read_barrier_depends();
+ private = lockless_dereference(table->private);
table_base = private->entries[smp_processor_id()];
e = get_entry(table_base, private->hook_entry[hook]);
diff --git a/net/ipv4/netfilter/ip_tables.c b/net/ipv4/netfilter/ip_tables.c
index 99e810f..e0fd044 100644
--- a/net/ipv4/netfilter/ip_tables.c
+++ b/net/ipv4/netfilter/ip_tables.c
@@ -325,13 +325,12 @@ ipt_do_table(struct sk_buff *skb,
IP_NF_ASSERT(table->valid_hooks & (1 << hook));
local_bh_disable();
addend = xt_write_recseq_begin();
- private = table->private;
cpu = smp_processor_id();
/*
* Ensure we load private-> members after we've fetched the base
* pointer.
*/
- smp_read_barrier_depends();
+ private = lockless_dereference(table->private);
table_base = private->entries[cpu];
jumpstack = (struct ipt_entry **)private->jumpstack[cpu];
stackptr = per_cpu_ptr(private->stackptr, cpu);
diff --git a/net/ipv6/netfilter/ip6_tables.c b/net/ipv6/netfilter/ip6_tables.c
index e080fbb..0459d6a 100644
--- a/net/ipv6/netfilter/ip6_tables.c
+++ b/net/ipv6/netfilter/ip6_tables.c
@@ -348,12 +348,11 @@ ip6t_do_table(struct sk_buff *skb,
local_bh_disable();
addend = xt_write_recseq_begin();
- private = table->private;
/*
* Ensure we load private-> members after we've fetched the base
* pointer.
*/
- smp_read_barrier_depends();
+ private = lockless_dereference(table->private);
cpu = smp_processor_id();
table_base = private->entries[cpu];
jumpstack = (struct ip6t_entry **)private->jumpstack[cpu];
--
1.9.1
^ permalink raw reply related
* [PATCH 16/16] rxrpc: Replace smp_read_barrier_depends() with lockless_dereference()
From: Pranith Kumar @ 2014-11-13 19:24 UTC (permalink / raw)
To: David S. Miller, David Howells, Dan Carpenter,
open list:NETWORKING [GENERAL], open list
Cc: paulmck
In-Reply-To: <1415906662-4576-1-git-send-email-bobby.prani@gmail.com>
Recently lockless_dereference() was added which can be used in place of
hard-coding smp_read_barrier_depends(). The following PATCH makes the change.
Signed-off-by: Pranith Kumar <bobby.prani@gmail.com>
---
net/rxrpc/ar-ack.c | 22 +++++++++-------------
1 file changed, 9 insertions(+), 13 deletions(-)
diff --git a/net/rxrpc/ar-ack.c b/net/rxrpc/ar-ack.c
index c6be17a..9237448 100644
--- a/net/rxrpc/ar-ack.c
+++ b/net/rxrpc/ar-ack.c
@@ -234,8 +234,7 @@ static void rxrpc_resend(struct rxrpc_call *call)
loop != call->acks_head || stop;
loop = (loop + 1) & (call->acks_winsz - 1)
) {
- p_txb = call->acks_window + loop;
- smp_read_barrier_depends();
+ p_txb = lockless_dereference(call)->acks_window + loop;
if (*p_txb & 1)
continue;
@@ -303,8 +302,7 @@ static void rxrpc_resend_timer(struct rxrpc_call *call)
loop != call->acks_head;
loop = (loop + 1) & (call->acks_winsz - 1)
) {
- p_txb = call->acks_window + loop;
- smp_read_barrier_depends();
+ p_txb = lockless_dereference(call)->acks_window + loop;
txb = (struct sk_buff *) (*p_txb & ~1);
sp = rxrpc_skb(txb);
@@ -354,9 +352,10 @@ static int rxrpc_process_soft_ACKs(struct rxrpc_call *call,
resend = 0;
resend_at = 0;
for (loop = 0; loop < ack->nAcks; loop++) {
- p_txb = call->acks_window;
- p_txb += (call->acks_tail + loop) & (call->acks_winsz - 1);
- smp_read_barrier_depends();
+ struct rxrpc_call *callp = lockless_dereference(call);
+
+ p_txb = callp->acks_window;
+ p_txb += (callp->acks_tail + loop) & (callp->acks_winsz - 1);
txb = (struct sk_buff *) (*p_txb & ~1);
sp = rxrpc_skb(txb);
@@ -385,8 +384,7 @@ static int rxrpc_process_soft_ACKs(struct rxrpc_call *call,
loop != call->acks_head;
loop = (loop + 1) & (call->acks_winsz - 1)
) {
- p_txb = call->acks_window + loop;
- smp_read_barrier_depends();
+ p_txb = lockless_dereference(call)->acks_window + loop;
txb = (struct sk_buff *) (*p_txb & ~1);
sp = rxrpc_skb(txb);
@@ -432,8 +430,7 @@ static void rxrpc_rotate_tx_window(struct rxrpc_call *call, u32 hard)
ASSERTCMP(hard - call->acks_hard, <=, win);
while (call->acks_hard < hard) {
- smp_read_barrier_depends();
- _skb = call->acks_window[tail] & ~1;
+ _skb = lockless_dereference(call)->acks_window[tail] & ~1;
rxrpc_free_skb((struct sk_buff *) _skb);
old_tail = tail;
tail = (tail + 1) & (call->acks_winsz - 1);
@@ -577,8 +574,7 @@ static void rxrpc_zap_tx_window(struct rxrpc_call *call)
call->acks_window = NULL;
while (CIRC_CNT(call->acks_head, call->acks_tail, winsz) > 0) {
- tail = call->acks_tail;
- smp_read_barrier_depends();
+ tail = lockless_dereference(call)->acks_tail;
_skb = acks_window[tail] & ~1;
smp_mb();
call->acks_tail = (call->acks_tail + 1) & (winsz - 1);
--
1.9.1
^ permalink raw reply related
* [RFC PATCH 00/16] Replace smp_read_barrier_depends() with lockless_derefrence()
From: Pranith Kumar @ 2014-11-13 19:24 UTC (permalink / raw)
To: Herbert Xu, David S. Miller, Cristian Stoica, Horia Geanta,
Ruchika Gupta, Michael Neuling, Wolfram Sang,
open list:CRYPTO API, open list, Vinod Koul, Dan Williams,
Bartlomiej Zolnierkiewicz, Kyungmin Park, Manuel Schölling,
Dave Jiang, Rashika, open list:DMA GENERIC OFFLO...,
K. Y. Srinivasan, Haiyang Zhang
Cc: paulmck
Recently lockless_dereference() was added which can be used in place of
hard-coding smp_read_barrier_depends().
http://lkml.iu.edu/hypermail/linux/kernel/1410.3/04561.html
The following series tries to do this.
There are still some hard-coded locations which I was not sure how to replace
with. I will send in separate patches/questions regarding them.
Pranith Kumar (16):
crypto: caam - Remove unnecessary smp_read_barrier_depends()
doc: memory-barriers.txt: Document use of lockless_dereference()
drivers: dma: Replace smp_read_barrier_depends() with
lockless_dereference()
dcache: Replace smp_read_barrier_depends() with lockless_dereference()
overlayfs: Replace smp_read_barrier_depends() with
lockless_dereference()
assoc_array: Replace smp_read_barrier_depends() with
lockless_dereference()
hyperv: Replace smp_read_barrier_depends() with lockless_dereference()
rcupdate: Replace smp_read_barrier_depends() with
lockless_dereference()
percpu: Replace smp_read_barrier_depends() with lockless_dereference()
perf: Replace smp_read_barrier_depends() with lockless_dereference()
seccomp: Replace smp_read_barrier_depends() with
lockless_dereference()
task_work: Replace smp_read_barrier_depends() with
lockless_dereference()
ksm: Replace smp_read_barrier_depends() with lockless_dereference()
slab: Replace smp_read_barrier_depends() with lockless_dereference()
netfilter: Replace smp_read_barrier_depends() with
lockless_dereference()
rxrpc: Replace smp_read_barrier_depends() with lockless_dereference()
Documentation/memory-barriers.txt | 2 +-
drivers/crypto/caam/jr.c | 3 ---
drivers/dma/ioat/dma_v2.c | 3 +--
drivers/dma/ioat/dma_v3.c | 3 +--
fs/dcache.c | 7 ++-----
fs/overlayfs/super.c | 4 +---
include/linux/assoc_array_priv.h | 11 +++++++----
include/linux/hyperv.h | 9 ++++-----
include/linux/percpu-refcount.h | 4 +---
include/linux/rcupdate.h | 10 +++++-----
kernel/events/core.c | 3 +--
kernel/events/uprobes.c | 8 ++++----
kernel/seccomp.c | 7 +++----
kernel/task_work.c | 3 +--
lib/assoc_array.c | 7 -------
mm/ksm.c | 7 +++----
mm/slab.h | 6 +++---
net/ipv4/netfilter/arp_tables.c | 3 +--
net/ipv4/netfilter/ip_tables.c | 3 +--
net/ipv6/netfilter/ip6_tables.c | 3 +--
net/rxrpc/ar-ack.c | 22 +++++++++-------------
security/keys/keyring.c | 6 ------
22 files changed, 50 insertions(+), 84 deletions(-)
--
1.9.1
^ permalink raw reply
* tcp_fastretrans_alert warning in Linux kernel 3.10.
From: Vinson Lee @ 2014-11-13 19:26 UTC (permalink / raw)
To: netdev
Hi.
We saw this networking tcp_fastretrans_alert warning in Linux kernel 3.10.
------------[ cut here ]------------
WARNING: at net/ipv4/tcp_input.c:2788 tcp_fastretrans_alert+0x7a1/0x7be()
Modules linked in: netconsole configfs xt_DSCP iptable_mangle
cpufreq_ondemand ipv6 ppdev parport_pc lp parport tcp_diag inet_diag
ipmi_si ipmi_devintf ipmi
_wdt iTCO_vendor_support acpi_cpufreq freq_table mperf coretemp
kvm_intel kvm crc32c_intel ghash_clmulni_intel microcode serio_raw
i2c_i801 lpc_ich mfd_core
t i2c_core ptp pps_core ioatdma dca wmi
CPU: 13 PID: 0 Comm: swapper/13 Not tainted 3.10.50 #1
0000000000000000 ffff88085fce39b8 ffffffff814a4094 ffff88085fce39f0
ffffffff8103cbf9 0000000000000000 000000000000470e 0000000000000000
0000000000000000 ffff880824fe5e80 ffff88085fce3a00 ffffffff8103ccbf
Call Trace:
<IRQ> [<ffffffff814a4094>] dump_stack+0x19/0x1b
[<ffffffff8103cbf9>] warn_slowpath_common+0x65/0x7d
[<ffffffff8103ccbf>] warn_slowpath_null+0x1a/0x1c
[<ffffffff81438c56>] tcp_fastretrans_alert+0x7a1/0x7be
[<ffffffff81439644>] tcp_ack+0x95e/0xb47
[<ffffffff81439d46>] tcp_rcv_established+0x23e/0x442
[<ffffffff81442cab>] tcp_v4_do_rcv+0x1e6/0x3e8
[<ffffffff8144371e>] tcp_v4_rcv+0x293/0x52c
[<ffffffff81425e6a>] ? NF_HOOK_THRESH.constprop.11+0x53/0x53
[<ffffffff8142c668>] ? do_ip_setsockopt.isra.7+0xa95/0xb3f
[<ffffffff81425f4a>] ip_local_deliver_finish+0xe0/0x155
[<ffffffff81425e6a>] ? NF_HOOK_THRESH.constprop.11+0x53/0x53
[<ffffffff81425e48>] NF_HOOK_THRESH.constprop.11+0x31/0x53
[<ffffffff814260ef>] ip_local_deliver+0x40/0x52
[<ffffffff81425d50>] ip_rcv_finish+0x274/0x2b6
[<ffffffff81425adc>] ? inet_del_offload+0x3d/0x3d
[<ffffffff81425e48>] NF_HOOK_THRESH.constprop.11+0x31/0x53
[<ffffffff81426324>] ip_rcv+0x223/0x267
[<ffffffff813f76b4>] __netif_receive_skb_core+0x435/0x4a7
[<ffffffff8107bab8>] ? timekeeping_get_ns.constprop.10+0x11/0x36
[<ffffffff813f773e>] __netif_receive_skb+0x18/0x5a
[<ffffffff813f8813>] netif_receive_skb+0x40/0x75
[<ffffffff813f8f94>] napi_gro_receive+0x3e/0x80
[<ffffffffa004a097>] igb_clean_rx_irq+0x67e/0x69e [igb]
[<ffffffffa004a425>] igb_poll+0x36e/0x5b0 [igb]
[<ffffffff8107bab8>] ? timekeeping_get_ns.constprop.10+0x11/0x36
[<ffffffff8107bf32>] ? ktime_get+0x68/0x76
[<ffffffff813f8a61>] net_rx_action+0xcf/0x196
[<ffffffff8106870c>] ? sched_clock_cpu+0x42/0xc7
[<ffffffff8104371d>] __do_softirq+0xd5/0x1f4
[<ffffffff814b01bc>] call_softirq+0x1c/0x30
[<ffffffff81003ecd>] do_softirq+0x33/0x6e
[<ffffffff81043931>] irq_exit+0x51/0x93
[<ffffffff814b086e>] do_IRQ+0x8e/0xa5
[<ffffffff814a85aa>] common_interrupt+0x6a/0x6a
<EOI> [<ffffffff813c6807>] ? cpuidle_enter_state+0x52/0xa3
[<ffffffff813c6800>] ? cpuidle_enter_state+0x4b/0xa3
[<ffffffff813c693d>] cpuidle_idle_call+0xe5/0x13f
[<ffffffff81009408>] arch_cpu_idle+0xe/0x1d
[<ffffffff8107a872>] cpu_startup_entry+0x128/0x17a
[<ffffffff814956be>] start_secondary+0x246/0x248
---[ end trace 804e605f7757900d ]---
net/ipv4/tcp_input.c
2775 static void tcp_fastretrans_alert(struct sock *sk, int pkts_acked,
2776 int prior_sacked, int prior_packets,
2777 bool is_dupack, int flag)
2778 {
2779 struct inet_connection_sock *icsk = inet_csk(sk);
2780 struct tcp_sock *tp = tcp_sk(sk);
2781 int do_lost = is_dupack || ((flag & FLAG_DATA_SACKED) &&
2782 (tcp_fackets_out(tp) >
tp->reordering));
2783 int newly_acked_sacked = 0;
2784 int fast_rexmit = 0;
2785
2786 if (WARN_ON(!tp->packets_out && tp->sacked_out))
2787 tp->sacked_out = 0;
2788 if (WARN_ON(!tp->sacked_out && tp->fackets_out))
2789 tp->fackets_out = 0;
Cheers,
Vinson
^ permalink raw reply
* [PATCH 0/3] Introduce load_acquire() and store_release()
From: Alexander Duyck @ 2014-11-13 19:27 UTC (permalink / raw)
To: linux-arch, netdev, linux-kernel
Cc: mikey, tony.luck, mathieu.desnoyers, donald.c.skidmore, peterz,
benh, heiko.carstens, oleg, will.deacon, davem, michael,
matthew.vick, nic_swsd, geert, jeffrey.t.kirsher, fweisbec,
schwidefsky, linux, paulmck, torvalds, mingo
These patches introduce uniprocessor or CPU<->device equivalents for
smp_load_acquire() and smp_store_release(). These two new primitives are:
load_acquire()
store_release()
The first patch adds the primitives for the applicable architectures and
asm-generic.
The second patch adds the primitives to r8169 which turns out to be a good
example of where the new primitives might be useful as they have memory
barriers ordering accesses to the descriptors and the DescOwn bit within the
descriptors which follow acquire/release style semantics.
The third patch adds support for load_acquire() to the Intel fm10k, igb,
and ixgbe drivers. Testing with the ixgbe driver has shown a processing
time reduction of at least 7ns per 64B frame on a Core i7-4930K.
This patch series is essentially the v2 for:
arch: Introduce read_acquire()
The key changes in this patch series versus that patch are:
- Renamed read_acquire() to be consistent with smp_load_acquire()
- Changed barrier used to be consistent with smp_load_acquire()
- Updated PowerPC code to use __lwsync based on IBM article
- Added store_release() as this is a viable use case for drivers
- Added r8169 patch which is able to fully use primitives
- Added fm10k/igb/ixgbe patch which is able to test performance
---
Alexander Duyck (3):
arch: Introduce load_acquire() and store_release()
r8169: Use load_acquire() and store_release() to reduce memory barrier overhead
fm10k/igb/ixgbe: Use load_acquire on Rx descriptor
arch/arm/include/asm/barrier.h | 15 ++++++
arch/arm64/include/asm/barrier.h | 59 +++++++++++++------------
arch/ia64/include/asm/barrier.h | 7 ++-
arch/metag/include/asm/barrier.h | 15 ++++++
arch/mips/include/asm/barrier.h | 15 ++++++
arch/powerpc/include/asm/barrier.h | 24 ++++++++--
arch/s390/include/asm/barrier.h | 7 ++-
arch/sparc/include/asm/barrier_64.h | 6 ++-
arch/x86/include/asm/barrier.h | 22 ++++++++-
drivers/net/ethernet/intel/fm10k/fm10k_main.c | 8 +--
drivers/net/ethernet/intel/igb/igb_main.c | 8 +--
drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 11 ++---
drivers/net/ethernet/realtek/r8169.c | 23 ++++------
include/asm-generic/barrier.h | 15 ++++++
14 files changed, 163 insertions(+), 72 deletions(-)
--
^ permalink raw reply
* [PATCH 1/3] arch: Introduce load_acquire() and store_release()
From: Alexander Duyck @ 2014-11-13 19:27 UTC (permalink / raw)
To: linux-arch, netdev, linux-kernel
Cc: mikey, tony.luck, mathieu.desnoyers, donald.c.skidmore, peterz,
benh, heiko.carstens, oleg, will.deacon, davem, michael,
matthew.vick, nic_swsd, geert, jeffrey.t.kirsher, fweisbec,
schwidefsky, linux, paulmck, torvalds, mingo
In-Reply-To: <20141113191250.12579.19694.stgit@ahduyck-server>
It is common for device drivers to make use of acquire/release semantics
when dealing with descriptors stored in device memory. On reviewing the
documentation and code for smp_load_acquire() and smp_store_release() as
well as reviewing an IBM website that goes over the use of PowerPC barriers
at http://www.ibm.com/developerworks/systems/articles/powerpc.html it
occurred to me that the same code could likely be applied to device drivers.
As a result this patch introduces load_acquire() and store_release(). The
load_acquire() function can be used in the place of situations where a test
for ownership must be followed by a memory barrier. The below example is
from ixgbe:
if (!rx_desc->wb.upper.status_error)
break;
/* This memory barrier is needed to keep us from reading
* any other fields out of the rx_desc until we know the
* descriptor has been written back
*/
rmb();
With load_acquire() this can be changed to:
if (!load_acquire(&rx_desc->wb.upper.status_error))
break;
A similar change can be made in the release path of many drivers. For
example in the Realtek r8169 driver there are a number of flows that
consist of something like the following:
wmb();
status = opts[0] | len | (RingEnd * !((entry + 1) % NUM_TX_DESC));
txd->opts1 = cpu_to_le32(status);
tp->cur_tx += frags + 1;
wmb();
With store_release() this can be changed to the following:
status = opts[0] | len | (RingEnd * !((entry + 1) % NUM_TX_DESC));
store_release(&txd->opts1, cpu_to_le32(status));
tp->cur_tx += frags + 1;
wmb();
The resulting assembler code generated as a result can be significantly
less expensive on architectures such as x86 and s390 that support strong
ordering. On architectures that are able to use different primitives than
their rmb/wmb() such as powerpc, ia64, and arm64 we should see gains as we
are able to use less expensive barriers, and for other architectures we end
up using a mb() which may come at the same amount of overhead or more than
a rmb/wmb() as we must ensure Load/Store ordering.
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
Cc: Michael Ellerman <michael@ellerman.id.au>
Cc: Michael Neuling <mikey@neuling.org>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: David Miller <davem@davemloft.net>
Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com>
---
arch/arm/include/asm/barrier.h | 15 +++++++++
arch/arm64/include/asm/barrier.h | 59 ++++++++++++++++++-----------------
arch/ia64/include/asm/barrier.h | 7 +++-
arch/metag/include/asm/barrier.h | 15 +++++++++
arch/mips/include/asm/barrier.h | 15 +++++++++
arch/powerpc/include/asm/barrier.h | 24 +++++++++++---
arch/s390/include/asm/barrier.h | 7 +++-
arch/sparc/include/asm/barrier_64.h | 6 ++--
arch/x86/include/asm/barrier.h | 22 ++++++++++++-
include/asm-generic/barrier.h | 15 +++++++++
10 files changed, 144 insertions(+), 41 deletions(-)
diff --git a/arch/arm/include/asm/barrier.h b/arch/arm/include/asm/barrier.h
index c6a3e73..bbdcd34 100644
--- a/arch/arm/include/asm/barrier.h
+++ b/arch/arm/include/asm/barrier.h
@@ -59,6 +59,21 @@
#define smp_wmb() dmb(ishst)
#endif
+#define store_release(p, v) \
+do { \
+ compiletime_assert_atomic_type(*p); \
+ mb(); \
+ ACCESS_ONCE(*p) = (v); \
+} while (0)
+
+#define load_acquire(p) \
+({ \
+ typeof(*p) ___p1 = ACCESS_ONCE(*p); \
+ compiletime_assert_atomic_type(*p); \
+ mb(); \
+ ___p1; \
+})
+
#define smp_store_release(p, v) \
do { \
compiletime_assert_atomic_type(*p); \
diff --git a/arch/arm64/include/asm/barrier.h b/arch/arm64/include/asm/barrier.h
index 6389d60..c91571c 100644
--- a/arch/arm64/include/asm/barrier.h
+++ b/arch/arm64/include/asm/barrier.h
@@ -32,33 +32,7 @@
#define rmb() dsb(ld)
#define wmb() dsb(st)
-#ifndef CONFIG_SMP
-#define smp_mb() barrier()
-#define smp_rmb() barrier()
-#define smp_wmb() barrier()
-
-#define smp_store_release(p, v) \
-do { \
- compiletime_assert_atomic_type(*p); \
- barrier(); \
- ACCESS_ONCE(*p) = (v); \
-} while (0)
-
-#define smp_load_acquire(p) \
-({ \
- typeof(*p) ___p1 = ACCESS_ONCE(*p); \
- compiletime_assert_atomic_type(*p); \
- barrier(); \
- ___p1; \
-})
-
-#else
-
-#define smp_mb() dmb(ish)
-#define smp_rmb() dmb(ishld)
-#define smp_wmb() dmb(ishst)
-
-#define smp_store_release(p, v) \
+#define store_release(p, v) \
do { \
compiletime_assert_atomic_type(*p); \
switch (sizeof(*p)) { \
@@ -73,7 +47,7 @@ do { \
} \
} while (0)
-#define smp_load_acquire(p) \
+#define load_acquire(p) \
({ \
typeof(*p) ___p1; \
compiletime_assert_atomic_type(*p); \
@@ -90,6 +64,35 @@ do { \
___p1; \
})
+#ifndef CONFIG_SMP
+#define smp_mb() barrier()
+#define smp_rmb() barrier()
+#define smp_wmb() barrier()
+
+#define smp_store_release(p, v) \
+do { \
+ compiletime_assert_atomic_type(*p); \
+ barrier(); \
+ ACCESS_ONCE(*p) = (v); \
+} while (0)
+
+#define smp_load_acquire(p) \
+({ \
+ typeof(*p) ___p1 = ACCESS_ONCE(*p); \
+ compiletime_assert_atomic_type(*p); \
+ barrier(); \
+ ___p1; \
+})
+
+#else
+
+#define smp_mb() dmb(ish)
+#define smp_rmb() dmb(ishld)
+#define smp_wmb() dmb(ishst)
+
+#define smp_store_release(p, v) store_release(p, v)
+#define smp_load_acquire(p) load_acquire(p)
+
#endif
#define read_barrier_depends() do { } while(0)
diff --git a/arch/ia64/include/asm/barrier.h b/arch/ia64/include/asm/barrier.h
index a48957c..d7fe208 100644
--- a/arch/ia64/include/asm/barrier.h
+++ b/arch/ia64/include/asm/barrier.h
@@ -63,14 +63,14 @@
* need for asm trickery!
*/
-#define smp_store_release(p, v) \
+#define store_release(p, v) \
do { \
compiletime_assert_atomic_type(*p); \
barrier(); \
ACCESS_ONCE(*p) = (v); \
} while (0)
-#define smp_load_acquire(p) \
+#define load_acquire(p) \
({ \
typeof(*p) ___p1 = ACCESS_ONCE(*p); \
compiletime_assert_atomic_type(*p); \
@@ -78,6 +78,9 @@ do { \
___p1; \
})
+#define smp_store_release(p, v) store_release(p, v)
+#define smp_load_acquire(p) load_acquire(p)
+
/*
* XXX check on this ---I suspect what Linus really wants here is
* acquire vs release semantics but we can't discuss this stuff with
diff --git a/arch/metag/include/asm/barrier.h b/arch/metag/include/asm/barrier.h
index c7591e8..9beb687 100644
--- a/arch/metag/include/asm/barrier.h
+++ b/arch/metag/include/asm/barrier.h
@@ -85,6 +85,21 @@ static inline void fence(void)
#define smp_read_barrier_depends() do { } while (0)
#define set_mb(var, value) do { var = value; smp_mb(); } while (0)
+#define store_release(p, v) \
+do { \
+ compiletime_assert_atomic_type(*p); \
+ mb(); \
+ ACCESS_ONCE(*p) = (v); \
+} while (0)
+
+#define load_acquire(p) \
+({ \
+ typeof(*p) ___p1 = ACCESS_ONCE(*p); \
+ compiletime_assert_atomic_type(*p); \
+ mb(); \
+ ___p1; \
+})
+
#define smp_store_release(p, v) \
do { \
compiletime_assert_atomic_type(*p); \
diff --git a/arch/mips/include/asm/barrier.h b/arch/mips/include/asm/barrier.h
index d0101dd..fc7323c 100644
--- a/arch/mips/include/asm/barrier.h
+++ b/arch/mips/include/asm/barrier.h
@@ -180,6 +180,21 @@
#define nudge_writes() mb()
#endif
+#define store_release(p, v) \
+do { \
+ compiletime_assert_atomic_type(*p); \
+ mb(); \
+ ACCESS_ONCE(*p) = (v); \
+} while (0)
+
+#define load_acquire(p) \
+({ \
+ typeof(*p) ___p1 = ACCESS_ONCE(*p); \
+ compiletime_assert_atomic_type(*p); \
+ mb(); \
+ ___p1; \
+})
+
#define smp_store_release(p, v) \
do { \
compiletime_assert_atomic_type(*p); \
diff --git a/arch/powerpc/include/asm/barrier.h b/arch/powerpc/include/asm/barrier.h
index bab79a1..f2a0d73 100644
--- a/arch/powerpc/include/asm/barrier.h
+++ b/arch/powerpc/include/asm/barrier.h
@@ -37,6 +37,23 @@
#define set_mb(var, value) do { var = value; mb(); } while (0)
+#define __lwsync() __asm__ __volatile__ (stringify_in_c(LWSYNC) : : :"memory")
+
+#define store_release(p, v) \
+do { \
+ compiletime_assert_atomic_type(*p); \
+ __lwsync(); \
+ ACCESS_ONCE(*p) = (v); \
+} while (0)
+
+#define load_acquire(p) \
+({ \
+ typeof(*p) ___p1 = ACCESS_ONCE(*p); \
+ compiletime_assert_atomic_type(*p); \
+ __lwsync(); \
+ ___p1; \
+})
+
#ifdef CONFIG_SMP
#ifdef __SUBARCH_HAS_LWSYNC
@@ -45,15 +62,12 @@
# define SMPWMB eieio
#endif
-#define __lwsync() __asm__ __volatile__ (stringify_in_c(LWSYNC) : : :"memory")
#define smp_mb() mb()
#define smp_rmb() __lwsync()
#define smp_wmb() __asm__ __volatile__ (stringify_in_c(SMPWMB) : : :"memory")
#define smp_read_barrier_depends() read_barrier_depends()
#else
-#define __lwsync() barrier()
-
#define smp_mb() barrier()
#define smp_rmb() barrier()
#define smp_wmb() barrier()
@@ -72,7 +86,7 @@
#define smp_store_release(p, v) \
do { \
compiletime_assert_atomic_type(*p); \
- __lwsync(); \
+ smp_rmb(); \
ACCESS_ONCE(*p) = (v); \
} while (0)
@@ -80,7 +94,7 @@ do { \
({ \
typeof(*p) ___p1 = ACCESS_ONCE(*p); \
compiletime_assert_atomic_type(*p); \
- __lwsync(); \
+ smp_rmb(); \
___p1; \
})
diff --git a/arch/s390/include/asm/barrier.h b/arch/s390/include/asm/barrier.h
index b5dce65..637d7a9 100644
--- a/arch/s390/include/asm/barrier.h
+++ b/arch/s390/include/asm/barrier.h
@@ -35,14 +35,14 @@
#define set_mb(var, value) do { var = value; mb(); } while (0)
-#define smp_store_release(p, v) \
+#define store_release(p, v) \
do { \
compiletime_assert_atomic_type(*p); \
barrier(); \
ACCESS_ONCE(*p) = (v); \
} while (0)
-#define smp_load_acquire(p) \
+#define load_acquire(p) \
({ \
typeof(*p) ___p1 = ACCESS_ONCE(*p); \
compiletime_assert_atomic_type(*p); \
@@ -50,4 +50,7 @@ do { \
___p1; \
})
+#define smp_store_release(p, v) store_release(p, v)
+#define smp_load_acquire(p) load_acquire(p)
+
#endif /* __ASM_BARRIER_H */
diff --git a/arch/sparc/include/asm/barrier_64.h b/arch/sparc/include/asm/barrier_64.h
index 305dcc3..7de3c69 100644
--- a/arch/sparc/include/asm/barrier_64.h
+++ b/arch/sparc/include/asm/barrier_64.h
@@ -53,14 +53,14 @@ do { __asm__ __volatile__("ba,pt %%xcc, 1f\n\t" \
#define smp_read_barrier_depends() do { } while(0)
-#define smp_store_release(p, v) \
+#define store_release(p, v) \
do { \
compiletime_assert_atomic_type(*p); \
barrier(); \
ACCESS_ONCE(*p) = (v); \
} while (0)
-#define smp_load_acquire(p) \
+#define load_acquire(p) \
({ \
typeof(*p) ___p1 = ACCESS_ONCE(*p); \
compiletime_assert_atomic_type(*p); \
@@ -68,6 +68,8 @@ do { \
___p1; \
})
+#define smp_store_release(p, v) store_release(p, v)
+#define smp_load_acquire(p) load_acquire(p)
#define smp_mb__before_atomic() barrier()
#define smp_mb__after_atomic() barrier()
diff --git a/arch/x86/include/asm/barrier.h b/arch/x86/include/asm/barrier.h
index 0f4460b..3d2aa18 100644
--- a/arch/x86/include/asm/barrier.h
+++ b/arch/x86/include/asm/barrier.h
@@ -103,6 +103,21 @@
* model and we should fall back to full barriers.
*/
+#define store_release(p, v) \
+do { \
+ compiletime_assert_atomic_type(*p); \
+ mb(); \
+ ACCESS_ONCE(*p) = (v); \
+} while (0)
+
+#define load_acquire(p) \
+({ \
+ typeof(*p) ___p1 = ACCESS_ONCE(*p); \
+ compiletime_assert_atomic_type(*p); \
+ mb(); \
+ ___p1; \
+})
+
#define smp_store_release(p, v) \
do { \
compiletime_assert_atomic_type(*p); \
@@ -120,14 +135,14 @@ do { \
#else /* regular x86 TSO memory ordering */
-#define smp_store_release(p, v) \
+#define store_release(p, v) \
do { \
compiletime_assert_atomic_type(*p); \
barrier(); \
ACCESS_ONCE(*p) = (v); \
} while (0)
-#define smp_load_acquire(p) \
+#define load_acquire(p) \
({ \
typeof(*p) ___p1 = ACCESS_ONCE(*p); \
compiletime_assert_atomic_type(*p); \
@@ -135,6 +150,9 @@ do { \
___p1; \
})
+#define smp_store_release(p, v) store_release(p, v)
+#define smp_load_acquire(p) load_acquire(p)
+
#endif
/* Atomic operations are already serializing on x86 */
diff --git a/include/asm-generic/barrier.h b/include/asm-generic/barrier.h
index 1402fa8..c6e4b99 100644
--- a/include/asm-generic/barrier.h
+++ b/include/asm-generic/barrier.h
@@ -70,6 +70,21 @@
#define smp_mb__after_atomic() smp_mb()
#endif
+#define store_release(p, v) \
+do { \
+ compiletime_assert_atomic_type(*p); \
+ mb(); \
+ ACCESS_ONCE(*p) = (v); \
+} while (0)
+
+#define load_acquire(p) \
+({ \
+ typeof(*p) ___p1 = ACCESS_ONCE(*p); \
+ compiletime_assert_atomic_type(*p); \
+ mb(); \
+ ___p1; \
+})
+
#define smp_store_release(p, v) \
do { \
compiletime_assert_atomic_type(*p); \
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox