* [PATCH net-next] igb: use build_skb()
From: Eric Dumazet @ 2012-08-02 15:51 UTC (permalink / raw)
To: Jeff Kirsher; +Cc: Alexander Duyck, netdev
From: Eric Dumazet <edumazet@google.com>
By using netdev_alloc_frag() & build_skb() instead of legacy
netdev_alloc_skb_ip_align() calls, we reduce number of cache misses in
RX path and size of working set.
For a given rx workload, number of 'inuse' sk_buff can be reduced to a
very minimum, especially when packets are dropped by our stack.
(Before this patch, default sk_buff allocation was 2048 sk_buffs in rx
ring buffer)
They are initialized right before being delivered to stack, so can stay
hot in cpu caches.
Ethernet header prefetching is more effective (old prefetch of skb->data
paid a stall to access skb->data pointer)
I have 15% performance increase in a RX stress test, removing SLUB slow
path in the profiles.
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Alexander Duyck <alexander.h.duyck@intel.com>
---
drivers/net/ethernet/intel/igb/igb.h | 8 ++
drivers/net/ethernet/intel/igb/igb_ethtool.c | 14 ++--
drivers/net/ethernet/intel/igb/igb_main.c | 56 ++++++++++-------
3 files changed, 49 insertions(+), 29 deletions(-)
diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h
index 9e572dd..e43b160 100644
--- a/drivers/net/ethernet/intel/igb/igb.h
+++ b/drivers/net/ethernet/intel/igb/igb.h
@@ -135,6 +135,11 @@ struct vf_data_storage {
#define IGB_RXBUFFER_16384 16384
#define IGB_RX_HDR_LEN IGB_RXBUFFER_512
+/* size of fragments allocated with netdev_alloc_frag() */
+#define IGB_FRAGSZ \
+ (SKB_DATA_ALIGN(IGB_RX_HDR_LEN + NET_SKB_PAD + NET_IP_ALIGN) + \
+ SKB_DATA_ALIGN(sizeof(struct skb_shared_info)))
+
/* How many Tx Descriptors do we need to call netif_wake_queue ? */
#define IGB_TX_QUEUE_WAKE 16
/* How many Rx Buffers do we bundle into one write to the hardware ? */
@@ -173,7 +178,7 @@ struct igb_tx_buffer {
};
struct igb_rx_buffer {
- struct sk_buff *skb;
+ void *data;
dma_addr_t dma;
struct page *page;
dma_addr_t page_dma;
@@ -242,6 +247,7 @@ struct igb_ring {
u16 next_to_clean ____cacheline_aligned_in_smp;
u16 next_to_use;
+ struct sk_buff *skb;
union {
/* TX */
struct {
diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c b/drivers/net/ethernet/intel/igb/igb_ethtool.c
index a19c84c..e6fc5cb 100644
--- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
+++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
@@ -1636,12 +1636,12 @@ static void igb_create_lbtest_frame(struct sk_buff *skb,
memset(&skb->data[frame_size + 12], 0xAF, 1);
}
-static int igb_check_lbtest_frame(struct sk_buff *skb, unsigned int frame_size)
+static int igb_check_lbtest_frame(const u8 *data, unsigned int frame_size)
{
frame_size /= 2;
- if (*(skb->data + 3) == 0xFF) {
- if ((*(skb->data + frame_size + 10) == 0xBE) &&
- (*(skb->data + frame_size + 12) == 0xAF)) {
+ if (*(data + 3) == 0xFF) {
+ if ((*(data + frame_size + 10) == 0xBE) &&
+ (*(data + frame_size + 12) == 0xAF)) {
return 0;
}
}
@@ -1675,8 +1675,10 @@ static int igb_clean_test_rings(struct igb_ring *rx_ring,
DMA_FROM_DEVICE);
rx_buffer_info->dma = 0;
- /* verify contents of skb */
- if (!igb_check_lbtest_frame(rx_buffer_info->skb, size))
+ /* verify contents of buffer */
+ if (!igb_check_lbtest_frame(rx_buffer_info->data +
+ NET_SKB_PAD + NET_IP_ALIGN,
+ size))
count++;
/* unmap buffer on tx side */
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index b7c2d50..8b732c9 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -538,14 +538,14 @@ rx_ring_summary:
"--------- %p%s\n", "RWB", i,
le64_to_cpu(u0->a),
le64_to_cpu(u0->b),
- buffer_info->skb, next_desc);
+ buffer_info->data, next_desc);
} else {
pr_info("%s[0x%03X] %016llX %016llX %016llX"
" %p%s\n", "R ", i,
le64_to_cpu(u0->a),
le64_to_cpu(u0->b),
(u64)buffer_info->dma,
- buffer_info->skb, next_desc);
+ buffer_info->data, next_desc);
if (netif_msg_pktdata(adapter)) {
print_hex_dump(KERN_INFO, "",
@@ -3379,7 +3379,7 @@ static void igb_clean_rx_ring(struct igb_ring *rx_ring)
if (!rx_ring->rx_buffer_info)
return;
- /* Free all the Rx ring sk_buffs */
+ /* Free all the Rx ring frags */
for (i = 0; i < rx_ring->count; i++) {
struct igb_rx_buffer *buffer_info = &rx_ring->rx_buffer_info[i];
if (buffer_info->dma) {
@@ -3390,9 +3390,9 @@ static void igb_clean_rx_ring(struct igb_ring *rx_ring)
buffer_info->dma = 0;
}
- if (buffer_info->skb) {
- dev_kfree_skb(buffer_info->skb);
- buffer_info->skb = NULL;
+ if (buffer_info->data) {
+ put_page(virt_to_head_page(buffer_info->data));
+ buffer_info->data = NULL;
}
if (buffer_info->page_dma) {
dma_unmap_page(rx_ring->dev,
@@ -3407,6 +3407,8 @@ static void igb_clean_rx_ring(struct igb_ring *rx_ring)
buffer_info->page_offset = 0;
}
}
+ kfree_skb(rx_ring->skb);
+ rx_ring->skb = NULL;
size = sizeof(struct igb_rx_buffer) * rx_ring->count;
memset(rx_ring->rx_buffer_info, 0, size);
@@ -6078,11 +6080,13 @@ static bool igb_clean_rx_irq(struct igb_q_vector *q_vector, int budget)
while (igb_test_staterr(rx_desc, E1000_RXD_STAT_DD)) {
struct igb_rx_buffer *buffer_info = &rx_ring->rx_buffer_info[i];
- struct sk_buff *skb = buffer_info->skb;
+ struct sk_buff *skb = rx_ring->skb;
+ void *data = buffer_info->data;
union e1000_adv_rx_desc *next_rxd;
- buffer_info->skb = NULL;
- prefetch(skb->data);
+ prefetch(data + NET_SKB_PAD);
+ buffer_info->data = NULL;
+ rx_ring->skb = NULL;
i++;
if (i == rx_ring->count)
@@ -6091,6 +6095,15 @@ static bool igb_clean_rx_irq(struct igb_q_vector *q_vector, int budget)
next_rxd = IGB_RX_DESC(rx_ring, i);
prefetch(next_rxd);
+ if (!skb) {
+ skb = build_skb(data, IGB_FRAGSZ);
+ if (unlikely(!skb)) {
+ rx_ring->rx_stats.alloc_failed++;
+ buffer_info->data = data;
+ goto next_desc;
+ }
+ skb_reserve(skb, NET_SKB_PAD + NET_IP_ALIGN);
+ }
/*
* This memory barrier is needed to keep us from reading
* any other fields out of the rx_desc until we know the
@@ -6118,6 +6131,8 @@ static bool igb_clean_rx_irq(struct igb_q_vector *q_vector, int budget)
skb->data_len += length;
skb->truesize += PAGE_SIZE / 2;
+ skb_propagate_pfmemalloc(buffer_info->page, skb);
+
if ((page_count(buffer_info->page) != 1) ||
(page_to_nid(buffer_info->page) != current_node))
buffer_info->page = NULL;
@@ -6132,9 +6147,9 @@ static bool igb_clean_rx_irq(struct igb_q_vector *q_vector, int budget)
if (!igb_test_staterr(rx_desc, E1000_RXD_STAT_EOP)) {
struct igb_rx_buffer *next_buffer;
next_buffer = &rx_ring->rx_buffer_info[i];
- buffer_info->skb = next_buffer->skb;
+ buffer_info->data = next_buffer->data;
buffer_info->dma = next_buffer->dma;
- next_buffer->skb = skb;
+ rx_ring->skb = skb;
next_buffer->dma = 0;
goto next_desc;
}
@@ -6158,6 +6173,7 @@ static bool igb_clean_rx_irq(struct igb_q_vector *q_vector, int budget)
skb->protocol = eth_type_trans(skb, rx_ring->netdev);
+ skb_record_rx_queue(skb, rx_ring->queue_index);
napi_gro_receive(&q_vector->napi, skb);
budget--;
@@ -6193,26 +6209,22 @@ next_desc:
static bool igb_alloc_mapped_skb(struct igb_ring *rx_ring,
struct igb_rx_buffer *bi)
{
- struct sk_buff *skb = bi->skb;
+ void *data = bi->data;
dma_addr_t dma = bi->dma;
if (dma)
return true;
- if (likely(!skb)) {
- skb = netdev_alloc_skb_ip_align(rx_ring->netdev,
- IGB_RX_HDR_LEN);
- bi->skb = skb;
- if (!skb) {
+ if (likely(!data)) {
+ data = netdev_alloc_frag(IGB_FRAGSZ);
+ bi->data = data;
+ if (!data) {
rx_ring->rx_stats.alloc_failed++;
return false;
}
-
- /* initialize skb for ring */
- skb_record_rx_queue(skb, rx_ring->queue_index);
}
- dma = dma_map_single(rx_ring->dev, skb->data,
+ dma = dma_map_single(rx_ring->dev, data + NET_SKB_PAD + NET_IP_ALIGN,
IGB_RX_HDR_LEN, DMA_FROM_DEVICE);
if (dma_mapping_error(rx_ring->dev, dma)) {
@@ -6235,7 +6247,7 @@ static bool igb_alloc_mapped_page(struct igb_ring *rx_ring,
return true;
if (!page) {
- page = __skb_alloc_page(GFP_ATOMIC, bi->skb);
+ page = alloc_page(GFP_ATOMIC | __GFP_COLD);
bi->page = page;
if (unlikely(!page)) {
rx_ring->rx_stats.alloc_failed++;
^ permalink raw reply related
* Re: [PATCH] igb: reduce Rx header size
From: Eric Dumazet @ 2012-08-02 15:52 UTC (permalink / raw)
To: Alexander Duyck; +Cc: Jeff Kirsher, netdev, Wyborny, Carolyn
In-Reply-To: <501AA00E.3010400@intel.com>
On Thu, 2012-08-02 at 08:43 -0700, Alexander Duyck wrote:
> Would there be any problems with us running this through Jeff's tree for
> testing prior to applying this? I'm just wanting to do some testing
> internally for any performance regressions due to this change.
Sure, I was expecting this exactly, sorry if it was not clear.
( by the way, I am more interested on the second patch I just sent )
Thanks !
^ permalink raw reply
* RE: [PATCH net-next] igb: use build_skb()
From: Wyborny, Carolyn @ 2012-08-02 16:10 UTC (permalink / raw)
To: Eric Dumazet, Kirsher, Jeffrey T; +Cc: Duyck, Alexander H, netdev
In-Reply-To: <1343922692.9299.231.camel@edumazet-glaptop>
-----Original Message-----
From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On Behalf Of Eric Dumazet
Sent: Thursday, August 02, 2012 8:52 AM
To: Kirsher, Jeffrey T
Cc: Duyck, Alexander H; netdev
Subject: [PATCH net-next] igb: use build_skb()
From: Eric Dumazet <edumazet@google.com>
By using netdev_alloc_frag() & build_skb() instead of legacy
netdev_alloc_skb_ip_align() calls, we reduce number of cache misses in RX path and size of working set.
For a given rx workload, number of 'inuse' sk_buff can be reduced to a very minimum, especially when packets are dropped by our stack.
(Before this patch, default sk_buff allocation was 2048 sk_buffs in rx ring buffer)
They are initialized right before being delivered to stack, so can stay hot in cpu caches.
Ethernet header prefetching is more effective (old prefetch of skb->data paid a stall to access skb->data pointer)
I have 15% performance increase in a RX stress test, removing SLUB slow path in the profiles.
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Alexander Duyck <alexander.h.duyck@intel.com>
---
Thanks Eric,
We've been working on some more extensive performance refactoring that will be coming in addition to this.
Jeff will pull these patches into his tree for testing.
Carolyn
Carolyn Wyborny
Linux Development
LAN Access Division
Intel Corporation
^ permalink raw reply
* Re: binding UDP port 0 with SO_REUSEADDR
From: Tobias S. Josefowitz @ 2012-08-02 16:33 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev
In-Reply-To: <CAEZM=bPqOgRUvUKfdYkrVXoWin+SMft0GK67hqsGhBD6nocryA@mail.gmail.com>
Hi again,
I need to say, after looking at "my" socket(7),
SO_REUSEADDR
Indicates that the rules used in validating addresses supplied
in a bind(2) call should allow reuse of local addresses. For
AF_INET sockets this means that a socket may bind, except when
there is an active listening socket bound to the address. When
the listening socket is bound to INADDR_ANY with a specific port
then it is not possible to bind to this port for any local
address. Argument is an integer boolean flag.
I think the surprise-factor of the bind-0-behaviour even increased for
me. Is there a specific reason for handing out used ports when binding
port 0 with REUSEADDR?
Best,
Tobi
^ permalink raw reply
* Re: binding UDP port 0 with SO_REUSEADDR
From: Eric Dumazet @ 2012-08-02 16:57 UTC (permalink / raw)
To: Tobias S. Josefowitz; +Cc: netdev
In-Reply-To: <CAEZM=bM=AGp=FOxLiDiBFgVJWGmNdPpG_FV3Ne2i5jeALiB4YA@mail.gmail.com>
On Thu, 2012-08-02 at 18:33 +0200, Tobias S. Josefowitz wrote:
> Hi again,
>
> I need to say, after looking at "my" socket(7),
>
> SO_REUSEADDR
> Indicates that the rules used in validating addresses supplied
> in a bind(2) call should allow reuse of local addresses. For
> AF_INET sockets this means that a socket may bind, except when
> there is an active listening socket bound to the address. When
> the listening socket is bound to INADDR_ANY with a specific port
> then it is not possible to bind to this port for any local
> address. Argument is an integer boolean flag.
>
> I think the surprise-factor of the bind-0-behaviour even increased for
> me. Is there a specific reason for handing out used ports when binding
> port 0 with REUSEADDR?
There is no concept of listening sockets for UDP.
This documentation applies for TCP, and makes no sense for UDP.
There is no value using 'port 0' and REUSEADDR on UDP, really.
^ permalink raw reply
* Re: [PATCH v2] dynamic_debug: Restore dev_dbg functionality, optimize stack
From: Joe Perches @ 2012-08-02 17:02 UTC (permalink / raw)
To: Andrew Morton
Cc: Greg Kroah-Hartman, David S. Miller, Jason Baron, Jim Cromie,
Kay Sievers, linux-kernel, netdev
In-Reply-To: <20120731145534.5606e840.akpm@linux-foundation.org>
On Tue, 2012-07-31 at 14:55 -0700, Andrew Morton wrote:
> On Sat, 28 Jul 2012 00:55:07 -0700
> Joe Perches <joe@perches.com> wrote:
>
> > commit c4e00daaa9 ("driver-core: extend dev_printk() to pass structured data")
> > changed __dev_printk and broke dynamic-debug's ability to control the
> > dynamic prefix of dev_dbg(dev,..).
> >
> > dynamic_emit_prefix() adds "[tid] module:func:line:" to the output and
> > those additions got lost.
> >
> > In addition, the current dynamic debug code uses up to 3 recursion
> > levels via %pV. This can consume quite a bit of stack. Directly
> > call printk_emit to reduce the recursion depth.
[]
> > Changes in v2:
> >
> > o Fix dynamic_emit_prefix to always initialize output
> > o Call create_syslog_header and emit_printk from__netdev_printk and
> > eliminate call to dev_printk to remove another recursion via %pV
>
> This one causes an oops-on-boot
> (http://ozlabs.org/~akpm/stuff/IMG_20120731_144047.jpg). v1 didn't do
> that.
>
> config: http://ozlabs.org/~akpm/stuff/config-akpm2
>
Thanks. I'll fix it when I can.
Expect a week or so delay though.
I'm a bit busy on other projects and this one seems to
require additional testing.
cheers, Joe
^ permalink raw reply
* Re: [PATCH V2 09/12] net/eipoib: Add main driver functionality
From: Eric W. Biederman @ 2012-08-02 17:15 UTC (permalink / raw)
To: Or Gerlitz; +Cc: davem, roland, netdev, ali, sean.hefty, Erez Shitrit
In-Reply-To: <1343840975-3252-10-git-send-email-ogerlitz@mellanox.com>
Or Gerlitz <ogerlitz@mellanox.com> writes:
> From: Erez Shitrit <erezsh@mellanox.co.il>
>
> The eipoib driver provides a standard Ethernet netdevice over
> the InfiniBand IPoIB interface .
>
> Some services can run only on top of Ethernet L2 interfaces, and cannot be
> bound to an IPoIB interface. With this new driver, these services can run
> seamlessly.
Do I read this code correctly that what you are doing is not tunneling
ethernet over IB but instead you are removing an ethernet header and
replacing it with an IB header?
Do I also read this code correctly if you can't find your destination
mac address in your ""neighbor table"" you do a normal IPoIB arp
for the infiniband GUID?
Do I read this right that if presented with a non-IPv4 or ARP packet
this code will do something undefined and unpredictable?
Maybe this makes some sense but just skimming it looks like you
are trying to force a square peg into a round hole resulting in
some weird code and some very weird maintainability issues.
I am honestly surprised at this approach. I would think it would be
faster and simpler to run an IB queue pair directly to the hypervisor or
possibly even the guest operating system bypassing the kernel and doing
all of this translation in userspace.
Eric
^ permalink raw reply
* Re: binding UDP port 0 with SO_REUSEADDR
From: Tobias S. Josefowitz @ 2012-08-02 19:05 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1343926645.9299.303.camel@edumazet-glaptop>
On Thu, Aug 2, 2012 at 6:57 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> There is no value using 'port 0' and REUSEADDR on UDP, really.
And nobody ever makes mistakes, so why not surprise them for even more
than they are in for.
"Looking into src/pkg/net/sock.go (func socket) I see that SO_REUSEADDR
is set unconditionally to all types of sockets." --
http://code.google.com/p/go/issues/detail?id=1692
Anyway, I'm out of this now.
Tobi
^ permalink raw reply
* discussion questions: SR-IOV, virtualization, and bonding
From: Chris Friesen @ 2012-08-02 19:21 UTC (permalink / raw)
To: e1000-devel@lists.sourceforge.net, netdev
Hi all,
I wanted to just highlight some issues that we're seeing and see what
others are doing in this area.
Our configuration is that we have a host with SR-IOV-capable NICs with
bonding enabled on the PF. Depending on the exact system it could be
active/standby or some form of active/active.
In the guests we generally have several VFs (corresponding to several
PFs) and we want to bond them for reliability.
We're seeing a number of issues:
1) If the guests use arp monitoring then broadcast arp packets from the
guests are visible on the other guests and on the host, and can cause
them to think the link is good even if we aren't receiving arp packets
from the external network. (I'm assuming carrier is up.)
2) If both the host and guest use active/backup but pick different
devices as the active, there is no traffic between host/guest over the
bond link. Packets are sent out the active and looped back internally
to arrive on the inactive, then skb_bond_should_drop() suppresses them.
3) For active/standby the default is to set the standby to the MAC
address of the bond. If the host has already set the MAC address (using
some algorithm to ensure uniqueness within the local network) then the
guest is not allowed to change it.
So far the solutions to 1 seem to be either using arp validation (which
currently doesn't exist for loadbalancing modes) or else have the
underlying ethernet driver distinguish between packets coming from the
wire vs being looped back internally and have the bonding driver only
set last_rx for external packets.
For issue 2, it would seem beneficial for the host to be able to ensure
that the guest uses the same link as the active. I don't see a tidy
solution here. One somewhat messy possibility here is to have bonding
send a message to the standby PF which then tells all its VFs to fake
loss of carrier.
For issue 3, the logical solution would seem to be some way of assigning
a list of "valid" mac addresses to a given VF--like maybe all MAC
addresses assigned to a VM or something. Anyone have any bright ideas?
I'm sure we're not the only ones running into this, so what are others
doing? Is the only current option to use active/active with miimon?
Chris
--
Chris Friesen
Software Designer
3500 Carling Avenue
Ottawa, Ontario K2H 8E9
www.genband.com
^ permalink raw reply
* Re: [PATCH 1/4] net/mlx4_en: Setting the NETIF_F_GRO flag back to dev->hw_features
From: Michał Mirosław @ 2012-08-02 19:36 UTC (permalink / raw)
To: Yevgeny Petrilin; +Cc: davem, netdev
In-Reply-To: <1343921456-19180-2-git-send-email-yevgenyp@mellanox.co.il>
2012/8/2 Yevgeny Petrilin <yevgenyp@mellanox.co.il>:
> The bug which removed it was introduced in commit c8c64cff
> which added the hw_features.
[...]
> - dev->hw_features = NETIF_F_SG | NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM;
> + dev->hw_features = NETIF_F_SG | NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
> + NETIF_F_GRO;
Just try to realize the truth: there is no bug.
register_netdevice() is setting GSO and GRO bits for everyone.
Best Regards,
Michał Mirosław
^ permalink raw reply
* [PATCH 1/2] net: davinci_mdio: enable and disable clock
From: Daniel Mack @ 2012-08-02 19:43 UTC (permalink / raw)
To: netdev
Cc: devicetree-discuss, koen, mugunthanvnm, linux-arm-kernel, paul,
Daniel Mack
Make the driver control the device clocks. Appearantly, the Davinci
platform probes this driver with the clock all powered up, but on OMAP,
this isn't the case.
Signed-off-by: Daniel Mack <zonque@gmail.com>
---
drivers/net/ethernet/ti/davinci_mdio.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/ti/davinci_mdio.c b/drivers/net/ethernet/ti/davinci_mdio.c
index cd7ee20..b4b6015 100644
--- a/drivers/net/ethernet/ti/davinci_mdio.c
+++ b/drivers/net/ethernet/ti/davinci_mdio.c
@@ -332,6 +332,8 @@ static int __devinit davinci_mdio_probe(struct platform_device *pdev)
goto bail_out;
}
+ clk_enable(data->clk);
+
dev_set_drvdata(dev, data);
data->dev = dev;
spin_lock_init(&data->lock);
@@ -379,8 +381,11 @@ bail_out:
if (data->bus)
mdiobus_free(data->bus);
- if (data->clk)
+ if (data->clk) {
+ clk_disable(data->clk);
clk_put(data->clk);
+ }
+
pm_runtime_put_sync(&pdev->dev);
pm_runtime_disable(&pdev->dev);
@@ -397,8 +402,11 @@ static int __devexit davinci_mdio_remove(struct platform_device *pdev)
if (data->bus)
mdiobus_free(data->bus);
- if (data->clk)
+ if (data->clk) {
+ clk_disable(data->clk);
clk_put(data->clk);
+ }
+
pm_runtime_put_sync(&pdev->dev);
pm_runtime_disable(&pdev->dev);
@@ -427,6 +435,8 @@ static int davinci_mdio_suspend(struct device *dev)
data->suspended = true;
spin_unlock(&data->lock);
+ clk_disable(data->clk);
+
return 0;
}
@@ -435,6 +445,8 @@ static int davinci_mdio_resume(struct device *dev)
struct davinci_mdio_data *data = dev_get_drvdata(dev);
u32 ctrl;
+ clk_enable(data->clk);
+
spin_lock(&data->lock);
pm_runtime_put_sync(data->dev);
--
1.7.11.2
^ permalink raw reply related
* [PATCH 2/2] net: davinci_mdio: add DT bindings
From: Daniel Mack @ 2012-08-02 19:43 UTC (permalink / raw)
To: netdev
Cc: devicetree-discuss, koen, mugunthanvnm, linux-arm-kernel, paul,
Daniel Mack
In-Reply-To: <1343936616-29318-1-git-send-email-zonque@gmail.com>
Signed-off-by: Daniel Mack <zonque@gmail.com>
---
.../devicetree/bindings/net/davinci_mdio.txt | 24 +++++++++++++
drivers/net/ethernet/ti/davinci_mdio.c | 39 ++++++++++++++++++++++
2 files changed, 63 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/davinci_mdio.txt
diff --git a/Documentation/devicetree/bindings/net/davinci_mdio.txt b/Documentation/devicetree/bindings/net/davinci_mdio.txt
new file mode 100644
index 0000000..03292a5
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/davinci_mdio.txt
@@ -0,0 +1,24 @@
+Davinci MDIO DT bindings
+
+Required properties:
+
+ - compatible : should be "ti,davinci-mdio"
+
+Optional properties:
+
+ - bus-freq : an integer to specify the bus speed
+
+Examples:
+
+ mdio: davinci_mdio@4a101000 {
+ compatible = "ti,davinci-mdio";
+ ti,hwmods = "davinci_mdio";
+ };
+
+ or
+
+ mdio: davinci_mdio@4a101000 {
+ compatible = "ti,davinci-mdio";
+ reg = <0x4a101000 0x100>;
+ };
+
diff --git a/drivers/net/ethernet/ti/davinci_mdio.c b/drivers/net/ethernet/ti/davinci_mdio.c
index b4b6015..2640cae 100644
--- a/drivers/net/ethernet/ti/davinci_mdio.c
+++ b/drivers/net/ethernet/ti/davinci_mdio.c
@@ -36,6 +36,8 @@
#include <linux/io.h>
#include <linux/pm_runtime.h>
#include <linux/davinci_emac.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
/*
* This timeout definition is a worst-case ultra defensive measure against
@@ -289,6 +291,37 @@ static int davinci_mdio_write(struct mii_bus *bus, int phy_id,
return 0;
}
+#ifdef CONFIG_OF
+static const struct of_device_id davinci_mdio_dt_ids[] = {
+ { .compatible = "ti,davinci-mdio" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, davinci_mdio_dt_ids);
+
+static inline int davinci_mdio_probe_dt(struct device *dev,
+ struct mdio_platform_data *pdata)
+{
+ struct device_node *np = dev->of_node;
+ const struct of_device_id *of_id =
+ of_match_device(davinci_mdio_dt_ids, dev);
+ u32 tmp;
+
+ if (!of_id)
+ return 0;
+
+ if (of_property_read_u32(np, "bus-freq", &tmp) == 0)
+ pdata->bus_freq = tmp;
+
+ return 0;
+}
+#else
+static inline int davinci_mdio_probe_dt(struct device *dev,
+ struct mdio_platform_data *pdata)
+{
+ return 0;
+}
+#endif
+
static int __devinit davinci_mdio_probe(struct platform_device *pdev)
{
struct mdio_platform_data *pdata = pdev->dev.platform_data;
@@ -306,6 +339,10 @@ static int __devinit davinci_mdio_probe(struct platform_device *pdev)
data->pdata = pdata ? (*pdata) : default_pdata;
+ ret = davinci_mdio_probe_dt(dev, pdata);
+ if (ret < 0)
+ goto free_mem;
+
data->bus = mdiobus_alloc();
if (!data->bus) {
dev_err(dev, "failed to alloc mii bus\n");
@@ -389,6 +426,7 @@ bail_out:
pm_runtime_put_sync(&pdev->dev);
pm_runtime_disable(&pdev->dev);
+free_mem:
kfree(data);
return ret;
@@ -471,6 +509,7 @@ static struct platform_driver davinci_mdio_driver = {
.name = "davinci_mdio",
.owner = THIS_MODULE,
.pm = &davinci_mdio_pm_ops,
+ .of_match_table = of_match_ptr(davinci_mdio_dt_ids),
},
.probe = davinci_mdio_probe,
.remove = __devexit_p(davinci_mdio_remove),
--
1.7.11.2
^ permalink raw reply related
* Re: [PATCH 1/2] net: davinci_mdio: enable and disable clock
From: Russell King - ARM Linux @ 2012-08-02 19:53 UTC (permalink / raw)
To: Daniel Mack
Cc: netdev, mugunthanvnm, paul, devicetree-discuss, koen,
linux-arm-kernel
In-Reply-To: <1343936616-29318-1-git-send-email-zonque@gmail.com>
On Thu, Aug 02, 2012 at 09:43:35PM +0200, Daniel Mack wrote:
> Make the driver control the device clocks. Appearantly, the Davinci
> platform probes this driver with the clock all powered up, but on OMAP,
> this isn't the case.
Hmm, this looks like it could do with improvement, especially as we're
moving everything over to a common clk API.
1. This driver could do with clk_prepare()/clk_unprepare() calls.
2. This driver should not be making the assumption that NULL means
it can avoid clk_* calls. It should instead be using
if (!IS_ERR(clk))
Thanks.
^ permalink raw reply
* Re: [PATCH 1/2] net: davinci_mdio: enable and disable clock
From: Daniel Mack @ 2012-08-02 20:17 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: netdev, mugunthanvnm, paul, devicetree-discuss, koen,
linux-arm-kernel
In-Reply-To: <20120802195308.GZ6802@n2100.arm.linux.org.uk>
[-- Attachment #1: Type: text/plain, Size: 696 bytes --]
On 02.08.2012 21:53, Russell King - ARM Linux wrote:
> On Thu, Aug 02, 2012 at 09:43:35PM +0200, Daniel Mack wrote:
>> Make the driver control the device clocks. Appearantly, the Davinci
>> platform probes this driver with the clock all powered up, but on OMAP,
>> this isn't the case.
>
> Hmm, this looks like it could do with improvement, especially as we're
> moving everything over to a common clk API.
>
> 1. This driver could do with clk_prepare()/clk_unprepare() calls.
Ok, done.
> 2. This driver should not be making the assumption that NULL means
> it can avoid clk_* calls. It should instead be using
> if (!IS_ERR(clk))
Well spotted. Amended patch below.
Thanks,
Daniel
[-- Attachment #2: 0001-net-davinci_mdio-prepare-and-unprepare-clocks.patch --]
[-- Type: text/x-patch, Size: 2045 bytes --]
>From 57670e52d19218f897d835d25223bf4b4932252f Mon Sep 17 00:00:00 2001
From: Daniel Mack <zonque@gmail.com>
Date: Thu, 2 Aug 2012 21:24:36 +0200
Subject: [PATCH] net: davinci_mdio: prepare and unprepare clocks
Make the driver control the device clocks. Appearantly, the Davinci
platform probes this driver with the clock all powered up, but on OMAP,
this isn't the case.
While at it, also check for IS_ERR(data->clk) in the bail_out: label of
.probe().
Signed-off-by: Daniel Mack <zonque@gmail.com>
---
drivers/net/ethernet/ti/davinci_mdio.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/ti/davinci_mdio.c b/drivers/net/ethernet/ti/davinci_mdio.c
index cd7ee20..462f81d 100644
--- a/drivers/net/ethernet/ti/davinci_mdio.c
+++ b/drivers/net/ethernet/ti/davinci_mdio.c
@@ -332,6 +332,8 @@ static int __devinit davinci_mdio_probe(struct platform_device *pdev)
goto bail_out;
}
+ clk_prepare(data->clk);
+
dev_set_drvdata(dev, data);
data->dev = dev;
spin_lock_init(&data->lock);
@@ -379,8 +381,11 @@ bail_out:
if (data->bus)
mdiobus_free(data->bus);
- if (data->clk)
+ if (data->clk && !IS_ERR(data->clk)) {
+ clk_unprepare(data->clk);
clk_put(data->clk);
+ }
+
pm_runtime_put_sync(&pdev->dev);
pm_runtime_disable(&pdev->dev);
@@ -397,8 +402,11 @@ static int __devexit davinci_mdio_remove(struct platform_device *pdev)
if (data->bus)
mdiobus_free(data->bus);
- if (data->clk)
+ if (data->clk) {
+ clk_unprepare(data->clk);
clk_put(data->clk);
+ }
+
pm_runtime_put_sync(&pdev->dev);
pm_runtime_disable(&pdev->dev);
@@ -427,6 +435,8 @@ static int davinci_mdio_suspend(struct device *dev)
data->suspended = true;
spin_unlock(&data->lock);
+ clk_unprepare(data->clk);
+
return 0;
}
@@ -435,6 +445,8 @@ static int davinci_mdio_resume(struct device *dev)
struct davinci_mdio_data *data = dev_get_drvdata(dev);
u32 ctrl;
+ clk_prepare(data->clk);
+
spin_lock(&data->lock);
pm_runtime_put_sync(data->dev);
--
1.7.11.2
^ permalink raw reply related
* Re: [PATCH 1/2] net: davinci_mdio: enable and disable clock
From: Paul Walmsley @ 2012-08-02 20:20 UTC (permalink / raw)
To: Daniel Mack
Cc: netdev, devicetree-discuss, koen, mugunthanvnm, linux-arm-kernel
In-Reply-To: <1343936616-29318-1-git-send-email-zonque@gmail.com>
Hi
On Thu, 2 Aug 2012, Daniel Mack wrote:
> Make the driver control the device clocks. Appearantly, the Davinci
> platform probes this driver with the clock all powered up, but on OMAP,
> this isn't the case.
>
> Signed-off-by: Daniel Mack <zonque@gmail.com>
> ---
> drivers/net/ethernet/ti/davinci_mdio.c | 16 ++++++++++++++--
> 1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/ti/davinci_mdio.c b/drivers/net/ethernet/ti/davinci_mdio.c
> index cd7ee20..b4b6015 100644
> --- a/drivers/net/ethernet/ti/davinci_mdio.c
> +++ b/drivers/net/ethernet/ti/davinci_mdio.c
> @@ -332,6 +332,8 @@ static int __devinit davinci_mdio_probe(struct platform_device *pdev)
> goto bail_out;
> }
>
> + clk_enable(data->clk);
> +
This doesn't look right. This clock should be enabled by the
pm_runtime_get_sync() call just above this. It shouldn't be necessary
to enable it again unless something isn't right with the integration
data. Likewise the pm_runtime_put_sync() calls should be superfluous.
What hwmod data/device tree file are you using with this?
- Paul
^ permalink raw reply
* Re: [PATCH] igb: reduce Rx header size
From: Jeff Kirsher @ 2012-08-02 20:28 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Alexander Duyck, netdev
In-Reply-To: <1343920538.9299.200.camel@edumazet-glaptop>
[-- Attachment #1: Type: text/plain, Size: 457 bytes --]
On Thu, 2012-08-02 at 17:15 +0200, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> Reduce skb truesize by 256 bytes.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Alexander Duyck <alexander.h.duyck@intel.com>
> ---
> Tested on my machine without any problem
>
> drivers/net/ethernet/intel/igb/igb.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
Thanks Eric, I have added it to my queue.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH 1/2] net: davinci_mdio: enable and disable clock
From: Daniel Mack @ 2012-08-02 20:28 UTC (permalink / raw)
To: Paul Walmsley
Cc: netdev, devicetree-discuss, koen, mugunthanvnm, linux-arm-kernel
In-Reply-To: <alpine.DEB.2.00.1208021416550.18867@utopia.booyaka.com>
On 02.08.2012 22:20, Paul Walmsley wrote:
> Hi
>
> On Thu, 2 Aug 2012, Daniel Mack wrote:
>
>> Make the driver control the device clocks. Appearantly, the Davinci
>> platform probes this driver with the clock all powered up, but on OMAP,
>> this isn't the case.
>>
>> Signed-off-by: Daniel Mack <zonque@gmail.com>
>
>> ---
>> drivers/net/ethernet/ti/davinci_mdio.c | 16 ++++++++++++++--
>> 1 file changed, 14 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/ti/davinci_mdio.c b/drivers/net/ethernet/ti/davinci_mdio.c
>> index cd7ee20..b4b6015 100644
>> --- a/drivers/net/ethernet/ti/davinci_mdio.c
>> +++ b/drivers/net/ethernet/ti/davinci_mdio.c
>> @@ -332,6 +332,8 @@ static int __devinit davinci_mdio_probe(struct platform_device *pdev)
>> goto bail_out;
>> }
>>
>> + clk_enable(data->clk);
>> +
>
> This doesn't look right. This clock should be enabled by the
> pm_runtime_get_sync() call just above this. It shouldn't be necessary
> to enable it again unless something isn't right with the integration
> data. Likewise the pm_runtime_put_sync() calls should be superfluous.
Aah, thanks for the heads-up. To explain, I first worked with a dirty
hack to alias the clock, and I definitely needed these extra calls then.
> What hwmod data/device tree file are you using with this?
Later, I added the hwmod to move away from these hacks, and indeed, that
lets the pm runtime code handle the clock enabling. With that in place,
the patch we're talking about here is in fact unnecessary.
The second one though (the one that adds DT bindings) should go in.
I will send a separate one later that fixes the IS_ERR(data->clk) error
that Russell spotted. But that's now unrelated.
Thanks for the review,
Daniel
^ permalink raw reply
* Re: [PATCH net-next] igb: use build_skb()
From: Jeff Kirsher @ 2012-08-02 20:29 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Alexander Duyck, netdev
In-Reply-To: <1343922692.9299.231.camel@edumazet-glaptop>
[-- Attachment #1: Type: text/plain, Size: 1383 bytes --]
On Thu, 2012-08-02 at 17:51 +0200, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> By using netdev_alloc_frag() & build_skb() instead of legacy
> netdev_alloc_skb_ip_align() calls, we reduce number of cache misses in
> RX path and size of working set.
>
> For a given rx workload, number of 'inuse' sk_buff can be reduced to a
> very minimum, especially when packets are dropped by our stack.
>
> (Before this patch, default sk_buff allocation was 2048 sk_buffs in rx
> ring buffer)
>
> They are initialized right before being delivered to stack, so can
> stay
> hot in cpu caches.
>
> Ethernet header prefetching is more effective (old prefetch of
> skb->data
> paid a stall to access skb->data pointer)
>
> I have 15% performance increase in a RX stress test, removing SLUB
> slow
> path in the profiles.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Alexander Duyck <alexander.h.duyck@intel.com>
> ---
> drivers/net/ethernet/intel/igb/igb.h | 8 ++
> drivers/net/ethernet/intel/igb/igb_ethtool.c | 14 ++--
> drivers/net/ethernet/intel/igb/igb_main.c | 56 ++++++++++-------
> 3 files changed, 49 insertions(+), 29 deletions(-)
Thanks Eric, I have added this as well to my queue.
NOTE- you had some trailing whitespace errors in igb.h which I cleaned
up before adding to my queue.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: discussion questions: SR-IOV, virtualization, and bonding
From: Jay Vosburgh @ 2012-08-02 20:30 UTC (permalink / raw)
To: Chris Friesen; +Cc: e1000-devel@lists.sourceforge.net, netdev
In-Reply-To: <501AD33E.5090308@genband.com>
Chris Friesen <chris.friesen@genband.com> wrote:
>Hi all,
>
>I wanted to just highlight some issues that we're seeing and see what
>others are doing in this area.
>
>Our configuration is that we have a host with SR-IOV-capable NICs with
>bonding enabled on the PF. Depending on the exact system it could be
>active/standby or some form of active/active.
>
>In the guests we generally have several VFs (corresponding to several
>PFs) and we want to bond them for reliability.
>
>We're seeing a number of issues:
>
>1) If the guests use arp monitoring then broadcast arp packets from the
>guests are visible on the other guests and on the host, and can cause
>them to think the link is good even if we aren't receiving arp packets
>from the external network. (I'm assuming carrier is up.)
>
>2) If both the host and guest use active/backup but pick different
>devices as the active, there is no traffic between host/guest over the
>bond link. Packets are sent out the active and looped back internally
>to arrive on the inactive, then skb_bond_should_drop() suppresses them.
Just to be sure that I'm following this correctly, you're
setting up active-backup bonds on the guest and the host. The guest
sets its active slave to be a VF from "SR-IOV Device A," but the host
sets its active slave to a PF from "SR-IOV Device B." Traffic from the
guest to the host then arrives at the host's inactive slave (it's PF for
"SR-IOV Device A") and is then dropped.
Correct?
>3) For active/standby the default is to set the standby to the MAC
>address of the bond. If the host has already set the MAC address (using
>some algorithm to ensure uniqueness within the local network) then the
>guest is not allowed to change it.
>
>
>So far the solutions to 1 seem to be either using arp validation (which
>currently doesn't exist for loadbalancing modes) or else have the
>underlying ethernet driver distinguish between packets coming from the
>wire vs being looped back internally and have the bonding driver only
>set last_rx for external packets.
As discussed previously, e.g.,:
http://marc.info/?l=linux-netdev&m=134316327912154&w=2
implementing arp_validate for load balance modes is tricky at
best, regardless of SR-IOV issues.
This is really a variation on the situation that led to the
arp_validate functionality in the first place (that multiple instances
of ARP monitor on a subnet can fool one another), except that the switch
here is within the SR-IOV device and the various hosts are guests.
The best long term solution is to have a user space API that
provides link state input to bonding on a per-slave basis, and then some
user space entity can perform whatever link monitoring method is
appropriate (e.g., LLDP) and pass the results to bonding.
>For issue 2, it would seem beneficial for the host to be able to ensure
>that the guest uses the same link as the active. I don't see a tidy
>solution here. One somewhat messy possibility here is to have bonding
>send a message to the standby PF which then tells all its VFs to fake
>loss of carrier.
There is no tidy solution here that I'm aware of; this has been
a long standing concern in bladecenter type of network environments,
wherein all blade "eth0" interfaces connect to one chassis switch, and
all blade "eth1" interfaces connect to a different chassis switch. If
those switches are not connected, then there may not be a path from
blade A:eth0 to blade B:eth1. There is no simple mechanism to force a
gang failover across multiple hosts.
That said, I've seen a slight rub on this using virtualized
network devices (pseries ehea, which is similar in principle to SR-IOV,
although implemented differently). In that case, the single ehea card
provides all "eth0" devices for all lpars (logical partitions,
"guests"). A separate card (or individual per-lpar cards) provides the
"eth1" devices.
In this configuration, the bonding primary option is used to
make eth0 the primary, and thus all lpars use eth0 preferentially, and
there is no connectivity issue. If the ehea card itself fails, all of
the bonds will fail over simultaneously to the backup devices, and
again, there is no connectivity issue. This works because the ehea is a
single point of failure for all of the partitions.
Note that the ehea can propagate link failure of its external
port (the one that connects to a "real" switch) to its internal ports
(what the lpars see), so that bonding can detect the link failure. This
is an option to ehea; by default, all internal ports are always carrier
up so that they can communicate with one another regardless of the
external port link state. To my knowledge, this is used with miimon,
not the arp monitor.
I don't know how SR-IOV operates in this regard (e.g., can VFs
fail independently from the PF?). It is somewhat different from your
case in that there is no equivalent to the PF in the ehea case. If the
PFs participate in the primary setting it will likely permit initial
connectivity, but I'm not sure if a PF plus all its VFs fail as a unit
(from bonding's point of view).
>For issue 3, the logical solution would seem to be some way of assigning
>a list of "valid" mac addresses to a given VF--like maybe all MAC
>addresses assigned to a VM or something. Anyone have any bright ideas?
There's an option to bonding, fail_over_mac, that modifies
bonding's handling of the slaves' MAC address(es). One setting,
"active" instructs bonding to make its MAC be whatever the currently
active slave's MAC is, never changing any of the slave's MAC addresses.
>I'm sure we're not the only ones running into this, so what are others
>doing? Is the only current option to use active/active with miimon?
I think you're at least close to the edge here; I've only done
some basic testing of bonding with SR-IOV, although I'm planning to do
some more early next week (and what you've found has been good input for
me, so thanks for that, at least).
I suspect that some bonding configurations are simply not going
to work at all; e.g., I'm not aware of any SR-IOV devices that implement
LACP on the internal switch, and in any event, it would have to create
aggregators that span across physical network devices to be really
useful.
-J
---
-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and
threat landscape has changed and how IT managers can respond. Discussions
will include endpoint security, mobile security and the latest in malware
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
^ permalink raw reply
* Re: [PATCH] firmware: Remove obsolete Chelsio cxgb3 firmware
From: Paul Gortmaker @ 2012-08-02 20:40 UTC (permalink / raw)
To: Tim Gardner
Cc: David Miller, linux-kernel, ben, JBottomley, dan.j.williams, divy,
netdev
In-Reply-To: <501A728A.2070905@canonical.com>
On 12-08-02 08:28 AM, Tim Gardner wrote:
> On 08/02/2012 01:20 AM, David Miller wrote:
>>
>> "git am" refuses to apply this to current 'net':
>>
>> Applying: firmware: Remove obsolete Chelsio cxgb3 firmware
>> error: removal patch leaves file contents
>> error: firmware/cxgb3/t3fw-7.10.0.bin.ihex: patch does not apply
>>
>
> Paul Gortmaker suggested I use 'git format-patch --irreversible-delete'
> to produce shorter patches, but then even I can't reapply it.
The git folks originally designed it that way on purpose, (i.e. used
for review only) but it doesn't really need to be limited like that,
so I've proposed a fix and we'll see what the git folks have to say.
http://marc.info/?l=git&m=134394003916648&w=2
Paul.
--
>
> How about a pull request against net-next instead ?
>
> The following changes since commit 1a9b4993b70fb1884716902774dc9025b457760d:
>
> Merge branch 'upstream' of
> git://git.linux-mips.org/pub/scm/ralf/upstream-linus (2012-08-01
> 16:47:15 -0700)
>
> are available in the git repository at:
>
>
> git://kernel.ubuntu.com/rtg/net-next.git master
>
> for you to fetch changes up to 044b722f36a17bc5f7f472cc3246cb15a430bb0e:
>
> firmware: Remove obsolete Chelsio cxgb3 firmware (2012-08-02 06:23:25
> -0600)
>
> ----------------------------------------------------------------
> Tim Gardner (1):
> firmware: Remove obsolete Chelsio cxgb3 firmware
>
> firmware/Makefile | 1 -
> firmware/cxgb3/t3fw-7.10.0.bin.ihex | 1935
> -----------------------------------
> 2 files changed, 1936 deletions(-)
> delete mode 100644 firmware/cxgb3/t3fw-7.10.0.bin.ihex
>
^ permalink raw reply
* [PATCH] mlx4_en: add UFO support
From: Thadeu Lima de Souza Cascardo @ 2012-08-02 20:53 UTC (permalink / raw)
To: David S. Miller
Cc: netdev, Yevgeny Petrilin, Or Gerlitz,
Thadeu Lima de Souza Cascardo
Mellanox Ethernet adapters support Large Segmentation Offload for UDP
packets. The only change needed is using the proper header size when the
packet is UDP instead of TCP.
This significantly increases performance for large UDP packets on
platforms which have an expensive dma_map call, like pseries.
On a simple test with 64000 payload size, throughput has increased from
about 6Gbps to 9.5Gbps, while CPU use dropped from about 600% to about
80% or less, on a 8-core Power7 machine.
Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@linux.vnet.ibm.com>
---
drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 2 +-
drivers/net/ethernet/mellanox/mlx4/en_tx.c | 7 ++++++-
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
index edd9cb8..59e808a 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c
@@ -1660,7 +1660,7 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int port,
*/
dev->hw_features = NETIF_F_SG | NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM;
if (mdev->LSO_support)
- dev->hw_features |= NETIF_F_TSO | NETIF_F_TSO6;
+ dev->hw_features |= NETIF_F_TSO | NETIF_F_TSO6 | NETIF_F_UFO;
dev->vlan_features = dev->hw_features;
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_tx.c b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
index 019d856..2aad5a4 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_tx.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
@@ -39,6 +39,7 @@
#include <linux/if_vlan.h>
#include <linux/vmalloc.h>
#include <linux/tcp.h>
+#include <linux/udp.h>
#include <linux/moduleparam.h>
#include "mlx4_en.h"
@@ -455,7 +456,11 @@ static int get_real_size(struct sk_buff *skb, struct net_device *dev,
int real_size;
if (skb_is_gso(skb)) {
- *lso_header_size = skb_transport_offset(skb) + tcp_hdrlen(skb);
+ *lso_header_size = skb_transport_offset(skb);
+ if (skb_shinfo(skb)->gso_type == SKB_GSO_UDP)
+ *lso_header_size += sizeof(struct udphdr);
+ else
+ *lso_header_size += tcp_hdrlen(skb);
real_size = CTRL_SIZE + skb_shinfo(skb)->nr_frags * DS_SIZE +
ALIGN(*lso_header_size + 4, DS_SIZE);
if (unlikely(*lso_header_size != skb_headlen(skb))) {
--
1.7.4.4
^ permalink raw reply related
* Re: [PATCH] sctp: Make "Invalid Stream Identifier" ERROR follows SACK when bundling
From: Vlad Yasevich @ 2012-08-02 21:17 UTC (permalink / raw)
To: xufeng zhang
Cc: Xufeng Zhang, Neil Horman, sri, davem, linux-sctp, netdev,
linux-kernel
In-Reply-To: <50178061.4010709@windriver.com>
On 07/31/2012 02:51 AM, xufeng zhang wrote:
> Sorry, please ignore the above patch, there was an paste error.
> Please check the following patch.
> ============================================
> I'm wondering if the below solution is fine to you which is based on
> your changes.
> BTW, I have verified this patch and it works ok for all the situation,
> but only one problem persists:
> there is a potential that commands will exceeds SCTP_MAX_NUM_COMMANDS
> which happens during sending lots of small error DATA chunks.
>
I started thinking along the same vein, but was thinking that maybe it
makes sense to make error list more generic. I need to check the spec on
the ordering of ERROR chunks. If they are always after other control
chunks, then maybe make an error list and queue all errors there. Then
when sending control chunks, drain the control queue first, then the
error queue, and finally the data queue.
BTW, the patch below doesn't include the code to queue the error chunk
onto the new error queue.
-vlad
> Thanks,
> Xufeng Zhang
>
> ---
> include/net/sctp/command.h | 1 +
> include/net/sctp/structs.h | 3 +++
> net/sctp/outqueue.c | 7 +++++++
> net/sctp/sm_sideeffect.c | 16 ++++++++++++++++
> net/sctp/sm_statefuns.c | 17 ++++++++++++++---
> 5 files changed, 41 insertions(+), 3 deletions(-)
>
> diff --git a/include/net/sctp/command.h b/include/net/sctp/command.h
> index 712b3be..62c34f5 100644
> --- a/include/net/sctp/command.h
> +++ b/include/net/sctp/command.h
> @@ -110,6 +110,7 @@ typedef enum {
> SCTP_CMD_SEND_NEXT_ASCONF, /* Send the next ASCONF after ACK */
> SCTP_CMD_PURGE_ASCONF_QUEUE, /* Purge all asconf queues.*/
> SCTP_CMD_SET_ASOC, /* Restore association context */
> + SCTP_CMD_GEN_BAD_STREAM, /* Invalid Stream errors happened
> command */
> SCTP_CMD_LAST
> } sctp_verb_t;
>
> diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
> index fc5e600..3d218e0 100644
> --- a/include/net/sctp/structs.h
> +++ b/include/net/sctp/structs.h
> @@ -1183,6 +1183,9 @@ struct sctp_outq {
> */
> struct list_head abandoned;
>
> + /* Put Invalid Stream error chunks on this list */
> + struct list_head bad_stream_err;
> +
> /* How many unackd bytes do we have in-flight? */
> __u32 outstanding_bytes;
>
> diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c
> index e7aa177..1e87b0b 100644
> --- a/net/sctp/outqueue.c
> +++ b/net/sctp/outqueue.c
> @@ -211,6 +211,7 @@ void sctp_outq_init(struct sctp_association *asoc,
> struct sctp_outq *q)
> INIT_LIST_HEAD(&q->retransmit);
> INIT_LIST_HEAD(&q->sacked);
> INIT_LIST_HEAD(&q->abandoned);
> + INIT_LIST_HEAD(&q->bad_stream_err);
>
> q->fast_rtx = 0;
> q->outstanding_bytes = 0;
> @@ -283,6 +284,12 @@ void sctp_outq_teardown(struct sctp_outq *q)
> list_del_init(&chunk->list);
> sctp_chunk_free(chunk);
> }
> +
> + /* Throw away any pending Invalid Stream error chunks */
> + list_for_each_entry_safe(chunk, tmp,&q->bad_stream_err, list) {
> + list_del_init(&chunk->list);
> + sctp_chunk_free(chunk);
> + }
> }
>
> /* Free the outqueue structure and any related pending chunks. */
> diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c
> index fe99628..4698593 100644
> --- a/net/sctp/sm_sideeffect.c
> +++ b/net/sctp/sm_sideeffect.c
> @@ -1060,6 +1060,18 @@ static void sctp_cmd_send_asconf(struct
> sctp_association *asoc)
> }
> }
>
> +static void sctp_cmd_make_inv_stream_err(sctp_cmd_seq_t *commands,
> + struct sctp_association *asoc)
> +{
> + struct sctp_chunk *err, *tmp;
> + struct sctp_outq *q =&asoc->outqueue;
> +
> + list_for_each_entry_safe(err, tmp,&q->bad_stream_err, list) {
> + list_del_init(&err->list);
> + sctp_add_cmd_sf(commands, SCTP_CMD_REPLY,
> + SCTP_CHUNK(err));
> + }
> +}
>
> /* These three macros allow us to pull the debugging code out of the
> * main flow of sctp_do_sm() to keep attention focused on the real
> @@ -1724,6 +1736,10 @@ static int sctp_cmd_interpreter(sctp_event_t
> event_type,
> asoc = cmd->obj.asoc;
> break;
>
> + case SCTP_CMD_GEN_BAD_STREAM:
> + sctp_cmd_make_inv_stream_err(commands, asoc);
> + break;
> +
> default:
> pr_warn("Impossible command: %u, %p\n",
> cmd->verb, cmd->obj.ptr);
> diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
> index 9fca103..1c1bcd9 100644
> --- a/net/sctp/sm_statefuns.c
> +++ b/net/sctp/sm_statefuns.c
> @@ -2967,8 +2967,14 @@ discard_force:
> return SCTP_DISPOSITION_DISCARD;
>
> discard_noforce:
> - if (chunk->end_of_packet)
> + if (chunk->end_of_packet) {
> + struct sctp_outq *q =&asoc->outqueue;
> sctp_add_cmd_sf(commands, SCTP_CMD_GEN_SACK, force);
> + /* Queue the INVALID STREAM error after the SACK if one
> is needed. */
> + if (!list_empty(&q->bad_stream_err))
> + sctp_add_cmd_sf(commands, SCTP_CMD_GEN_BAD_STREAM,
> + SCTP_NULL());
> + }
>
> return SCTP_DISPOSITION_DISCARD;
> consume:
> @@ -3037,11 +3043,16 @@ sctp_disposition_t
> sctp_sf_eat_data_fast_4_4(const struct sctp_endpoint *ep,
> * with a SACK, a SHUTDOWN chunk, and restart the T2-shutdown
> timer
> */
> if (chunk->end_of_packet) {
> + struct sctp_outq *q =&asoc->outqueue;
> /* We must delay the chunk creation since the cumulative
> * TSN has not been updated yet.
> */
> sctp_add_cmd_sf(commands, SCTP_CMD_GEN_SHUTDOWN,
> SCTP_NULL());
> sctp_add_cmd_sf(commands, SCTP_CMD_GEN_SACK,
> SCTP_FORCE());
> + /* Queue the INVALID STREAM error after the SACK if one
> is needed. */
> + if (!list_empty(&q->bad_stream_err))
> + sctp_add_cmd_sf(commands, SCTP_CMD_GEN_BAD_STREAM,
> + SCTP_NULL());
> sctp_add_cmd_sf(commands, SCTP_CMD_TIMER_RESTART,
> SCTP_TO(SCTP_EVENT_TIMEOUT_T2_SHUTDOWN));
> }
> @@ -6136,6 +6147,7 @@ static int sctp_eat_data(const struct
> sctp_association *asoc,
> */
> sid = ntohs(data_hdr->stream);
> if (sid>= asoc->c.sinit_max_instreams) {
> + struct sctp_outq *q =&asoc->outqueue;
> /* Mark tsn as received even though we drop it */
> sctp_add_cmd_sf(commands, SCTP_CMD_REPORT_TSN,
> SCTP_U32(tsn));
>
> @@ -6144,8 +6156,7 @@ static int sctp_eat_data(const struct
> sctp_association *asoc,
> sizeof(data_hdr->stream),
> sizeof(u16));
> if (err)
> - sctp_add_cmd_sf(commands, SCTP_CMD_REPLY,
> - SCTP_CHUNK(err));
> + list_add_tail(&err->list,&q->bad_stream_err);
> return SCTP_IERROR_BAD_STREAM;
> }
>
^ permalink raw reply
* Re: discussion questions: SR-IOV, virtualization, and bonding
From: Chris Friesen @ 2012-08-02 22:26 UTC (permalink / raw)
To: Jay Vosburgh; +Cc: e1000-devel@lists.sourceforge.net, netdev
In-Reply-To: <17679.1343939453@death.nxdomain>
On 08/02/2012 02:30 PM, Jay Vosburgh wrote:
>
> Chris Friesen<chris.friesen@genband.com> wrote:
>> 2) If both the host and guest use active/backup but pick different
>> devices as the active, there is no traffic between host/guest over the
>> bond link. Packets are sent out the active and looped back internally
>> to arrive on the inactive, then skb_bond_should_drop() suppresses them.
>
> Just to be sure that I'm following this correctly, you're
> setting up active-backup bonds on the guest and the host. The guest
> sets its active slave to be a VF from "SR-IOV Device A," but the host
> sets its active slave to a PF from "SR-IOV Device B." Traffic from the
> guest to the host then arrives at the host's inactive slave (it's PF for
> "SR-IOV Device A") and is then dropped.
>
> Correct?
Yes, that's correct. The issue is that the internal switch on device A
knows nothing about device B. Ideally what should happen is that the
internal switch routes the packets out onto the wire so that they come
back in on device B and get routed up to the host. However, at least
with the Intel devices the internal switch has no learning capabilities.
The alternative is to have the external switch(es) configured to do the
loopback, but that puts some extra requirements on the selection of the
external switch.
>> So far the solutions to 1 seem to be either using arp validation (which
>> currently doesn't exist for loadbalancing modes) or else have the
>> underlying ethernet driver distinguish between packets coming from the
>> wire vs being looped back internally and have the bonding driver only
>> set last_rx for external packets.
>
> As discussed previously, e.g.,:
>
> http://marc.info/?l=linux-netdev&m=134316327912154&w=2
>
> implementing arp_validate for load balance modes is tricky at
> best, regardless of SR-IOV issues.
Yes, I should have referenced that discussion. I thought I'd include it
here with the other issues to group everything together.
> This is really a variation on the situation that led to the
> arp_validate functionality in the first place (that multiple instances
> of ARP monitor on a subnet can fool one another), except that the switch
> here is within the SR-IOV device and the various hosts are guests.
>
> The best long term solution is to have a user space API that
> provides link state input to bonding on a per-slave basis, and then some
> user space entity can perform whatever link monitoring method is
> appropriate (e.g., LLDP) and pass the results to bonding.
I think this has potential. This requires a virtual communication
channel between guest/host if we want the host to be able to influence
the guest's choice of active link, but I think that's not unreasonable.
Actually, couldn't we do this now? Turn off miimon and arpmon, then
just have the userspace thing write to
/sys/class/net/bondX/bonding/active_slave
>> For issue 2, it would seem beneficial for the host to be able to ensure
>> that the guest uses the same link as the active. I don't see a tidy
>> solution here. One somewhat messy possibility here is to have bonding
>> send a message to the standby PF which then tells all its VFs to fake
>> loss of carrier.
>
> There is no tidy solution here that I'm aware of; this has been
> a long standing concern in bladecenter type of network environments,
> wherein all blade "eth0" interfaces connect to one chassis switch, and
> all blade "eth1" interfaces connect to a different chassis switch. If
> those switches are not connected, then there may not be a path from
> blade A:eth0 to blade B:eth1. There is no simple mechanism to force a
> gang failover across multiple hosts.
In our blade server environment those two switches are indeed
cross-connected, so we haven't had to do gang-failover.
> Note that the ehea can propagate link failure of its external
> port (the one that connects to a "real" switch) to its internal ports
> (what the lpars see), so that bonding can detect the link failure. This
> is an option to ehea; by default, all internal ports are always carrier
> up so that they can communicate with one another regardless of the
> external port link state. To my knowledge, this is used with miimon,
> not the arp monitor.
>
> I don't know how SR-IOV operates in this regard (e.g., can VFs
> fail independently from the PF?). It is somewhat different from your
> case in that there is no equivalent to the PF in the ehea case. If the
> PFs participate in the primary setting it will likely permit initial
> connectivity, but I'm not sure if a PF plus all its VFs fail as a unit
> (from bonding's point of view).
With current Intel drivers at least, if the PF detects link failure it
fires a message to the VFs and they detect link failure within a short
time (milliseconds).
We can recommend the use of the "primary" option, but we don't always
have total control over what the guest does, and for some reason some of
them don't want to use "primary". I'm not sure why.
>> For issue 3, the logical solution would seem to be some way of assigning
>> a list of "valid" mac addresses to a given VF--like maybe all MAC
>> addresses assigned to a VM or something. Anyone have any bright ideas?
>
> There's an option to bonding, fail_over_mac, that modifies
> bonding's handling of the slaves' MAC address(es). One setting,
> "active" instructs bonding to make its MAC be whatever the currently
> active slave's MAC is, never changing any of the slave's MAC addresses.
Yes, I'm aware of that option. It does have drawbacks though, as
described in the bonding.txt docs.
>> I'm sure we're not the only ones running into this, so what are others
>> doing? Is the only current option to use active/active with miimon?
>
> I think you're at least close to the edge here; I've only done
> some basic testing of bonding with SR-IOV, although I'm planning to do
> some more early next week (and what you've found has been good input for
> me, so thanks for that, at least).
Glad we could help. :)
Chris
------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and
threat landscape has changed and how IT managers can respond. Discussions
will include endpoint security, mobile security and the latest in malware
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
^ permalink raw reply
* Re: discussion questions: SR-IOV, virtualization, and bonding
From: Chris Friesen @ 2012-08-02 22:33 UTC (permalink / raw)
To: Jay Vosburgh; +Cc: e1000-devel@lists.sourceforge.net, netdev
In-Reply-To: <501AFEAD.10001@genband.com>
On 08/02/2012 04:26 PM, Chris Friesen wrote:
> On 08/02/2012 02:30 PM, Jay Vosburgh wrote:
>> The best long term solution is to have a user space API that
>> provides link state input to bonding on a per-slave basis, and then some
>> user space entity can perform whatever link monitoring method is
>> appropriate (e.g., LLDP) and pass the results to bonding.
>
> I think this has potential. This requires a virtual communication
> channel between guest/host if we want the host to be able to influence
> the guest's choice of active link, but I think that's not unreasonable.
>
> Actually, couldn't we do this now? Turn off miimon and arpmon, then just
> have the userspace thing write to /sys/class/net/bondX/bonding/active_slave
Hmm...looks like the bonding code requires either miimon or arpmon. I
wonder if setting miimon to INT_MAX might work, at least for some
bonding modes.
Chris
------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and
threat landscape has changed and how IT managers can respond. Discussions
will include endpoint security, mobile security and the latest in malware
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
^ permalink raw reply
* Re: CPU: 0 Not tainted (3.1.9+ #1) when ifconfig rose0 down
From: Eric W. Biederman @ 2012-08-02 22:55 UTC (permalink / raw)
To: Bernard Pidoux; +Cc: linux-hams, Linux Netdev List
In-Reply-To: <5017E786.5000102@free.fr>
Bernard Pidoux <bernard.pidoux@free.fr> writes:
> Hi,
>
> I observe systematically a kernel panic when I try to shutdown rose0 device
> using ifconfig rose0 down
>
> This is happening on two very different ROSE implementation, one is on a machine
> with x86-64 kernel 4.6.3 on an Intel core 2 duo CPU
> the other is on a RaspBerry Pi with Raspbian and 3.1.9+ wheezy kernel
> recompiled with AX.25 modules (ax25, rose, netrom, 6pack, kiss) enabled.
>
> Here is an image of the screen dump :
>
> http://f6bvp.org/photos/rose_device_event.JPG
For some reason I can not connect f6bvp.org.
> It can be noticed that PC is at rose_device_event and
> LR is at sock_def_wakeup
>
> One thing to be noticed is that when I close before all ROSE and AX.25
> applications, there are still a few populated sockets, probably for one of the
> program did not close the sockets properly.
>
> I that case, does rose module should accept to shutdown rose0 device ?
> However, I guess that it should not create a kernel panic due to a kernel NULL
> pointer.
No. The kernel should not panic.
> I don't know what to do in order to debug that issue.
I assume that rose is interesting to you and you would like for it to
work better?
In general you can read the code and figure out what it is doing ref
counting wise. It doesn't look like anyone has done much with the rose
code for years except very basic maintenance across the kernel. It
scares me to know that I was the last one to touch the rose code.
Part of what is happening at the time of the panic is
unregister_netdevice_notifier now generates synthetic removal for all of
the network devices in the system to remove the need for a special path
to handle network device removal in modules.
Unfortunately it looks like one of those modules is a problem.
You might want to simply try moving unregister_netdevice_notifier a bit
earlier in rose_exit and see if that helps. Otherwise I would recommend
instrumenting the code up with some printk so you can understand what
part of unregistration is failing.
Eric
^ permalink raw reply
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