* Pull request: sfc-next 2013-08-30
From: Ben Hutchings @ 2013-08-30 3:36 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-net-drivers
[-- Attachment #1: Type: text/plain, Size: 4369 bytes --]
The following changes since commit d4fbdcfe93928fbcb7374ea490e41f7b69d95380:
sfc: Use extended MC_CMD_SENSOR_INFO and MC_CMD_READ_SENSORS (2013-08-27 22:29:56 +0100)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/bwh/sfc-next.git for-davem
for you to fetch changes up to f7a6d2c4427790cc8695401576dc594fcce8fc80:
sfc: Update copyright banners (2013-08-29 23:34:51 +0100)
1. A little more refactoring.
2. Remove the unnecessary use of atomic_t that you pointed out.
3. Add support for starting or queueing firmware requests from atomic
context.
4. Add hwmon support for additional sensors found on some new boards.
5. Add support for the EF10 controller architecture, the SFC9100 family
and specifically the SFC9120 controller.
Ben.
----------------------------------------------------------------
Alexandre Rames (1):
sfc: Use a global count of active queues instead of pending drains
Ben Hutchings (12):
sfc: Add support for new board sensors
sfc: Refactor efx_mcdi_rpc_start() and efx_mcdi_copyin()
sfc: Remove unnecessary use of atomic_t
sfc: Implement asynchronous MCDI requests
sfc: Document conditions for multicast replication vs filter replacement
sfc: Allow efx_nic_type::dimension_resources to fail
sfc: Initialise IRQ moderation for all NIC types from efx_init_eventq()
sfc: Extend struct efx_tx_buffer to allow pushing option descriptors
sfc: Add EF10 register and structure definitions
sfc: Make efx_mcdi_{init,fini}() call efx_mcdi_drv_attach()
sfc: Add support for Solarflare SFC9100 family
sfc: Update copyright banners
Jon Cooper (2):
sfc: Allow event queue initialisation to fail
sfc: Prepare for RX scatter on EF10
Matthew Slattery (1):
sfc: Allocate NVRAM partition ID range for PHY images
drivers/net/ethernet/sfc/Kconfig | 9 +-
drivers/net/ethernet/sfc/Makefile | 4 +-
drivers/net/ethernet/sfc/bitfield.h | 8 +-
drivers/net/ethernet/sfc/ef10.c | 3043 ++++++++++++++++++++++++++++++
drivers/net/ethernet/sfc/ef10_regs.h | 415 ++++
drivers/net/ethernet/sfc/efx.c | 175 +-
drivers/net/ethernet/sfc/efx.h | 26 +-
drivers/net/ethernet/sfc/enum.h | 4 +-
drivers/net/ethernet/sfc/ethtool.c | 6 +-
drivers/net/ethernet/sfc/falcon.c | 7 +-
drivers/net/ethernet/sfc/falcon_boards.c | 4 +-
drivers/net/ethernet/sfc/farch.c | 28 +-
drivers/net/ethernet/sfc/farch_regs.h | 4 +-
drivers/net/ethernet/sfc/filter.h | 4 +-
drivers/net/ethernet/sfc/io.h | 24 +-
drivers/net/ethernet/sfc/mcdi.c | 461 ++++-
drivers/net/ethernet/sfc/mcdi.h | 45 +-
drivers/net/ethernet/sfc/mcdi_mon.c | 125 +-
drivers/net/ethernet/sfc/mcdi_pcol.h | 4 +
drivers/net/ethernet/sfc/mcdi_port.c | 25 +-
drivers/net/ethernet/sfc/mdio_10g.c | 2 +-
drivers/net/ethernet/sfc/mdio_10g.h | 2 +-
drivers/net/ethernet/sfc/mtd.c | 4 +-
drivers/net/ethernet/sfc/net_driver.h | 33 +-
drivers/net/ethernet/sfc/nic.c | 4 +-
drivers/net/ethernet/sfc/nic.h | 89 +-
drivers/net/ethernet/sfc/phy.h | 2 +-
drivers/net/ethernet/sfc/ptp.c | 4 +-
drivers/net/ethernet/sfc/qt202x_phy.c | 4 +-
drivers/net/ethernet/sfc/rx.c | 46 +-
drivers/net/ethernet/sfc/selftest.c | 4 +-
drivers/net/ethernet/sfc/selftest.h | 4 +-
drivers/net/ethernet/sfc/siena.c | 28 +-
drivers/net/ethernet/sfc/siena_sriov.c | 4 +-
drivers/net/ethernet/sfc/tenxpress.c | 2 +-
drivers/net/ethernet/sfc/tx.c | 8 +-
drivers/net/ethernet/sfc/txc43128_phy.c | 2 +-
drivers/net/ethernet/sfc/vfdi.h | 2 +-
drivers/net/ethernet/sfc/workarounds.h | 10 +-
39 files changed, 4404 insertions(+), 271 deletions(-)
create mode 100644 drivers/net/ethernet/sfc/ef10.c
create mode 100644 drivers/net/ethernet/sfc/ef10_regs.h
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 482 bytes --]
^ permalink raw reply
* [PATCH] drivers: net: ethernet: 8390: Kconfig: add H8300H_AKI3068NET and H8300H_H8MAX dependancy for NE_H8300
From: Chen Gang @ 2013-08-30 3:29 UTC (permalink / raw)
To: Paul Gortmaker, Yoshinori Sato, Kees Cook; +Cc: David Miller, netdev
Only H8300H_AKI3068NET and H8300H_H8MAX can support NE_H8300 (or it
will no related irq and base address).
The release error:
CC [M] drivers/net/hamradio/bpqether.o
drivers/net/ethernet/8390/ne-h8300.c: In function 'init_dev':
drivers/net/ethernet/8390/ne-h8300.c:117:23: error: 'h8300_ne_base' undeclared (first use in this function)
drivers/net/ethernet/8390/ne-h8300.c:117:23: note: each undeclared identifier is reported only once for each function it appears in
drivers/net/ethernet/8390/ne-h8300.c:117:23: error: bit-field '<anonymous>' width not an integer constant
drivers/net/ethernet/8390/ne-h8300.c:119:20: error: 'h8300_ne_irq' undeclared (first use in this function)
drivers/net/ethernet/8390/ne-h8300.c: In function 'init_module':
drivers/net/ethernet/8390/ne-h8300.c:647:21: error: 'h8300_ne_base' undeclared (first use in this function)
drivers/net/ethernet/8390/ne-h8300.c:648:15: error: 'h8300_ne_irq' undeclared (first use in this function)
drivers/net/ethernet/8390/ne-h8300.c:661:4: warning: format '%x' expects argument of type 'unsigned int', but argument 2 has type 'long unsigned int' [-Wformat]
Signed-off-by: Chen Gang <gang.chen@asianux.com>
---
drivers/net/ethernet/8390/Kconfig | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ethernet/8390/Kconfig b/drivers/net/ethernet/8390/Kconfig
index a5f91e1..becef25 100644
--- a/drivers/net/ethernet/8390/Kconfig
+++ b/drivers/net/ethernet/8390/Kconfig
@@ -148,7 +148,7 @@ config PCMCIA_PCNET
config NE_H8300
tristate "NE2000 compatible support for H8/300"
- depends on H8300
+ depends on H8300H_AKI3068NET || H8300H_H8MAX
---help---
Say Y here if you want to use the NE2000 compatible
controller on the Renesas H8/300 processor.
--
1.7.7.6
^ permalink raw reply related
* Re: [PATCH 3/5] bonding: add rtnl lock for bonding_store_xmit_hash
From: Ding Tianhong @ 2013-08-30 3:26 UTC (permalink / raw)
To: Nikolay Aleksandrov
Cc: Jay Vosburgh, Andy Gospodarek, David S. Miller, Veaceslav Falico,
Netdev
In-Reply-To: <521F59E6.9050804@redhat.com>
On 2013/8/29 22:25, Nikolay Aleksandrov wrote:
> On 08/28/2013 06:21 AM, Ding Tianhong wrote:
>> The bonding_store_xmit_hash() call bond_set_mode_ops() to set bond xmit_policy,
>> the xmit_policy is used in xmit path for xor mode, so any changes may occur
>> problem without protection, add rtnl lock is fit here.
>>
>> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
>> ---
> I don't think we need any locking there, nothing bad can happen.
>
>
>
But I think bond->xmit_hash_policy need protect if write it, as the bond_3ad_xmit may use it,
or did I miss something? :)
^ permalink raw reply
* Re: [PATCH 6/6] vhost_net: remove the max pending check
From: Jason Wang @ 2013-08-30 3:23 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: netdev, linux-kernel, kvm, virtualization
In-Reply-To: <20130825115344.GB1829@redhat.com>
On 08/25/2013 07:53 PM, Michael S. Tsirkin wrote:
> On Fri, Aug 23, 2013 at 04:55:49PM +0800, Jason Wang wrote:
>> On 08/20/2013 10:48 AM, Jason Wang wrote:
>>> On 08/16/2013 06:02 PM, Michael S. Tsirkin wrote:
>>>>> On Fri, Aug 16, 2013 at 01:16:30PM +0800, Jason Wang wrote:
>>>>>>> We used to limit the max pending DMAs to prevent guest from pinning too many
>>>>>>> pages. But this could be removed since:
>>>>>>>
>>>>>>> - We have the sk_wmem_alloc check in both tun/macvtap to do the same work
>>>>>>> - This max pending check were almost useless since it was one done when there's
>>>>>>> no new buffers coming from guest. Guest can easily exceeds the limitation.
>>>>>>> - We've already check upend_idx != done_idx and switch to non zerocopy then. So
>>>>>>> even if all vq->heads were used, we can still does the packet transmission.
>>>>> We can but performance will suffer.
>>> The check were in fact only done when no new buffers submitted from
>>> guest. So if guest keep sending, the check won't be done.
>>>
>>> If we really want to do this, we should do it unconditionally. Anyway, I
>>> will do test to see the result.
>> There's a bug in PATCH 5/6, the check:
>>
>> nvq->upend_idx != nvq->done_idx
>>
>> makes the zerocopy always been disabled since we initialize both
>> upend_idx and done_idx to zero. So I change it to:
>>
>> (nvq->upend_idx + 1) % UIO_MAXIOV != nvq->done_idx.
> But what I would really like to try is limit ubuf_info to VHOST_MAX_PEND.
> I think this has a chance to improve performance since
> we'll be using less cache.
> Of course this means we must fix the code to really never submit
> more than VHOST_MAX_PEND requests.
>
> Want to try?
The result is, I see about 5%-10% improvement for per cpu throughput on
guest tx. But about 5% degradation on per cpu transaction rate on TCP_RR.
>> With this change on top, I didn't see performance difference w/ and w/o
>> this patch.
> Did you try small message sizes btw (like 1K)? Or just netperf
> default of 64K?
>
5%-10% improvement on for per cpu throughput on guest rx, but some
regressions (5%) on guest tx. So we'd better keep and make it doing
properly.
Will post V2 for your reviewing.
^ permalink raw reply
* Re: [PATCH] xgmac: use bufsz not dma_buf_sz in rx_refill
From: Rob Herring @ 2013-08-30 3:17 UTC (permalink / raw)
To: David Miller; +Cc: kmcmarti, netdev
In-Reply-To: <20130829.163835.1514973445858189297.davem@davemloft.net>
On 08/29/2013 03:38 PM, David Miller wrote:
> From: Kyle McMartin <kmcmarti@redhat.com>
> Date: Thu, 29 Aug 2013 16:35:23 -0400
>
>> On Thu, Aug 29, 2013 at 04:34:37PM -0400, David Miller wrote:
>>> From: Kyle McMartin <kyle@redhat.com>
>>> Date: Thu, 29 Aug 2013 16:11:51 -0400
>>>
>>>> Noticed while debugging another issue that DMA debug turned up, looks
>>>> like commit ef468d23 missed a case when switching to bufsz instead of
>>>> priv->dma_buf_sz? (tbf, the whole handling of DMA buffers seems slightly
>>>> suspect since we're not tracking the size, but trusting the hardware
>>>> between map and unmap...)
>>>>
>>>> Signed-off-by: Kyle McMartin <kyle@redhat.com>
>>>
>>> Applied and queued up for -stable, thanks.
>>
>> Heh, too fast David, Rob nak'd it. :)
>>
>> Sorry about that.
>
> If Rob had resubmitted his series in a timely manner this wouldn't
> have happened.
I was giving them more time hoping to have some feedback from Lennert.
Anyways, I've just resubmitted v2 which addresses Ben's comments.
Rob
^ permalink raw reply
* [PATCH v2 11/11] net: calxedaxgmac: fix xgmac_xmit DMA mapping error handling
From: Rob Herring @ 2013-08-30 3:13 UTC (permalink / raw)
To: netdev; +Cc: Lennert Buytenhek, bhutchings, Rob Herring
In-Reply-To: <1377832428-18117-1-git-send-email-robherring2@gmail.com>
From: Rob Herring <rob.herring@calxeda.com>
On a DMA mapping error in xgmac_xmit, we should simply free the skb and
return NETDEV_TX_OK rather than -EIO. In the case of errors in mapping
frags, we need to undo everything that has been setup.
Reported-by: Andreas Herrmann <andreas.herrmann@calxeda.com>
Signed-off-by: Rob Herring <rob.herring@calxeda.com>
---
v2: No change
drivers/net/ethernet/calxeda/xgmac.c | 31 ++++++++++++++++++++++++++-----
1 file changed, 26 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/calxeda/xgmac.c b/drivers/net/ethernet/calxeda/xgmac.c
index 1af20ce..d18b7ac 100644
--- a/drivers/net/ethernet/calxeda/xgmac.c
+++ b/drivers/net/ethernet/calxeda/xgmac.c
@@ -466,6 +466,13 @@ static inline void desc_set_tx_owner(struct xgmac_dma_desc *p, u32 flags)
p->flags = cpu_to_le32(tmpflags);
}
+static inline void desc_clear_tx_owner(struct xgmac_dma_desc *p)
+{
+ u32 tmpflags = le32_to_cpu(p->flags);
+ tmpflags &= TXDESC_END_RING;
+ p->flags = cpu_to_le32(tmpflags);
+}
+
static inline int desc_get_tx_ls(struct xgmac_dma_desc *p)
{
return le32_to_cpu(p->flags) & TXDESC_LAST_SEG;
@@ -1100,7 +1107,7 @@ static netdev_tx_t xgmac_xmit(struct sk_buff *skb, struct net_device *dev)
paddr = dma_map_single(priv->device, skb->data, len, DMA_TO_DEVICE);
if (dma_mapping_error(priv->device, paddr)) {
dev_kfree_skb(skb);
- return -EIO;
+ return NETDEV_TX_OK;
}
priv->tx_skbuff[entry] = skb;
desc_set_buf_addr_and_size(desc, paddr, len);
@@ -1112,10 +1119,8 @@ static netdev_tx_t xgmac_xmit(struct sk_buff *skb, struct net_device *dev)
paddr = skb_frag_dma_map(priv->device, frag, 0, len,
DMA_TO_DEVICE);
- if (dma_mapping_error(priv->device, paddr)) {
- dev_kfree_skb(skb);
- return -EIO;
- }
+ if (dma_mapping_error(priv->device, paddr))
+ goto dma_err;
entry = dma_ring_incr(entry, DMA_TX_RING_SZ);
desc = priv->dma_tx + entry;
@@ -1151,6 +1156,22 @@ static netdev_tx_t xgmac_xmit(struct sk_buff *skb, struct net_device *dev)
netif_start_queue(dev);
}
return NETDEV_TX_OK;
+
+dma_err:
+ entry = priv->tx_head;
+ for ( ; i > 0; i--) {
+ entry = dma_ring_incr(entry, DMA_TX_RING_SZ);
+ desc = priv->dma_tx + entry;
+ priv->tx_skbuff[entry] = NULL;
+ dma_unmap_page(priv->device, desc_get_buf_addr(desc),
+ desc_get_buf_len(desc), DMA_TO_DEVICE);
+ desc_clear_tx_owner(desc);
+ }
+ desc = first;
+ dma_unmap_single(priv->device, desc_get_buf_addr(desc),
+ desc_get_buf_len(desc), DMA_TO_DEVICE);
+ dev_kfree_skb(skb);
+ return NETDEV_TX_OK;
}
static int xgmac_rx(struct xgmac_priv *priv, int limit)
--
1.8.1.2
^ permalink raw reply related
* [PATCH v2 10/11] net: calxedaxgmac: fix rx DMA mapping API size mismatches
From: Rob Herring @ 2013-08-30 3:13 UTC (permalink / raw)
To: netdev; +Cc: Lennert Buytenhek, bhutchings, Rob Herring
In-Reply-To: <1377832428-18117-1-git-send-email-robherring2@gmail.com>
From: Rob Herring <rob.herring@calxeda.com>
Fix the mismatch in the DMA mapping and unmapping sizes for receive. The
unmap size must be equal to the map size and should not be the actual
received frame length. The map size should also be adjusted by the
NET_IP_ALIGN size since the h/w buffer size (dma_buf_sz) includes this
offset.
Also, add a missing dma_mapping_error check in xgmac_rx_refill.
Reported-by: Lennert Buytenhek <buytenh@wantstofly.org>
Signed-off-by: Rob Herring <rob.herring@calxeda.com>
---
v2: No change
drivers/net/ethernet/calxeda/xgmac.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/calxeda/xgmac.c b/drivers/net/ethernet/calxeda/xgmac.c
index 4ecf937..1af20ce 100644
--- a/drivers/net/ethernet/calxeda/xgmac.c
+++ b/drivers/net/ethernet/calxeda/xgmac.c
@@ -695,9 +695,14 @@ static void xgmac_rx_refill(struct xgmac_priv *priv)
if (unlikely(skb == NULL))
break;
- priv->rx_skbuff[entry] = skb;
paddr = dma_map_single(priv->device, skb->data,
- bufsz, DMA_FROM_DEVICE);
+ priv->dma_buf_sz - NET_IP_ALIGN,
+ DMA_FROM_DEVICE);
+ if (dma_mapping_error(priv->device, paddr)) {
+ dev_kfree_skb_any(skb);
+ break;
+ }
+ priv->rx_skbuff[entry] = skb;
desc_set_buf_addr(p, paddr, priv->dma_buf_sz);
}
@@ -794,13 +799,14 @@ static void xgmac_free_rx_skbufs(struct xgmac_priv *priv)
return;
for (i = 0; i < DMA_RX_RING_SZ; i++) {
- if (priv->rx_skbuff[i] == NULL)
+ struct sk_buff *skb = priv->rx_skbuff[i];
+ if (skb == NULL)
continue;
p = priv->dma_rx + i;
dma_unmap_single(priv->device, desc_get_buf_addr(p),
- priv->dma_buf_sz, DMA_FROM_DEVICE);
- dev_kfree_skb_any(priv->rx_skbuff[i]);
+ priv->dma_buf_sz - NET_IP_ALIGN, DMA_FROM_DEVICE);
+ dev_kfree_skb_any(skb);
priv->rx_skbuff[i] = NULL;
}
}
@@ -1187,7 +1193,7 @@ static int xgmac_rx(struct xgmac_priv *priv, int limit)
skb_put(skb, frame_len);
dma_unmap_single(priv->device, desc_get_buf_addr(p),
- frame_len, DMA_FROM_DEVICE);
+ priv->dma_buf_sz - NET_IP_ALIGN, DMA_FROM_DEVICE);
skb->protocol = eth_type_trans(skb, priv->dev);
skb->ip_summed = ip_checksum;
--
1.8.1.2
^ permalink raw reply related
* [PATCH v2 08/11] net: calxedaxgmac: fix various errors in xgmac_set_rx_mode
From: Rob Herring @ 2013-08-30 3:13 UTC (permalink / raw)
To: netdev; +Cc: Lennert Buytenhek, bhutchings, Rob Herring
In-Reply-To: <1377832428-18117-1-git-send-email-robherring2@gmail.com>
From: Rob Herring <rob.herring@calxeda.com>
Fix xgmac_set_rx_mode to handle several conditions that were not handled
correctly as Lennert Buytenhek describes:
If we have, say, 100 unicast addresses, and 5 multicast addresses, the
unicast address count check will evaluate to true, and set use_hash to
true. The multicast address check will however evaluate to false, and
use_hash won't be set to true again, and XGMAC_FRAME_FILTER_HMC won't
be OR'd into XGMAC_FRAME_FILTER, but since use_hash was still true
from the unicast check, netdev_for_each_mc_addr() will program the
multicast addresses into the hash table instead of using the MAC
address registers, but since the HMC bit wasn't set, the hash table
won't be checked for multicast addresses on receive, and we'll stop
receiving multicast packets entirely.
Also, there is no code that zeroes out MAC address registers reg..31
at the end of this function, meaning that under the right conditions,
unicast/multicast addresses that were previously in the filter but
were then deleted won't be cleared out.
Reported-by: Lennert Buytenhek <buytenh@wantstofly.org>
Signed-off-by: Rob Herring <rob.herring@calxeda.com>
---
v2: No change
drivers/net/ethernet/calxeda/xgmac.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/calxeda/xgmac.c b/drivers/net/ethernet/calxeda/xgmac.c
index 530b594..a7cb18d 100644
--- a/drivers/net/ethernet/calxeda/xgmac.c
+++ b/drivers/net/ethernet/calxeda/xgmac.c
@@ -618,10 +618,15 @@ static void xgmac_set_mac_addr(void __iomem *ioaddr, unsigned char *addr,
{
u32 data;
- data = (addr[5] << 8) | addr[4] | (num ? XGMAC_ADDR_AE : 0);
- writel(data, ioaddr + XGMAC_ADDR_HIGH(num));
- data = (addr[3] << 24) | (addr[2] << 16) | (addr[1] << 8) | addr[0];
- writel(data, ioaddr + XGMAC_ADDR_LOW(num));
+ if (addr) {
+ data = (addr[5] << 8) | addr[4] | (num ? XGMAC_ADDR_AE : 0);
+ writel(data, ioaddr + XGMAC_ADDR_HIGH(num));
+ data = (addr[3] << 24) | (addr[2] << 16) | (addr[1] << 8) | addr[0];
+ writel(data, ioaddr + XGMAC_ADDR_LOW(num));
+ } else {
+ writel(0, ioaddr + XGMAC_ADDR_HIGH(num));
+ writel(0, ioaddr + XGMAC_ADDR_LOW(num));
+ }
}
static void xgmac_get_mac_addr(void __iomem *ioaddr, unsigned char *addr,
@@ -1294,6 +1299,8 @@ static void xgmac_set_rx_mode(struct net_device *dev)
if ((netdev_mc_count(dev) + reg - 1) > XGMAC_MAX_FILTER_ADDR) {
use_hash = true;
value |= XGMAC_FRAME_FILTER_HMC | XGMAC_FRAME_FILTER_HPF;
+ } else {
+ use_hash = false;
}
netdev_for_each_mc_addr(ha, dev) {
if (use_hash) {
@@ -1310,6 +1317,8 @@ static void xgmac_set_rx_mode(struct net_device *dev)
}
out:
+ for (i = reg; i < XGMAC_MAX_FILTER_ADDR; i++)
+ xgmac_set_mac_addr(ioaddr, NULL, reg);
for (i = 0; i < XGMAC_NUM_HASH; i++)
writel(hash_filter[i], ioaddr + XGMAC_HASH(i));
--
1.8.1.2
^ permalink raw reply related
* [PATCH v2 09/11] net: calxedaxgmac: remove some unused statistic counters
From: Rob Herring @ 2013-08-30 3:13 UTC (permalink / raw)
To: netdev; +Cc: Lennert Buytenhek, bhutchings, Rob Herring
In-Reply-To: <1377832428-18117-1-git-send-email-robherring2@gmail.com>
From: Rob Herring <rob.herring@calxeda.com>
rx_sa_filter_fail and tx_undeflow events are unused and impossible
to occur based on how the h/w is used. We never filter on source MAC
address and TX store and forward mode prevents underflow events.
Reported-by: Lennert Buytenhek <buytenh@wantstofly.org>
Signed-off-by: Rob Herring <rob.herring@calxeda.com>
---
v2: No change
drivers/net/ethernet/calxeda/xgmac.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/net/ethernet/calxeda/xgmac.c b/drivers/net/ethernet/calxeda/xgmac.c
index a7cb18d..4ecf937 100644
--- a/drivers/net/ethernet/calxeda/xgmac.c
+++ b/drivers/net/ethernet/calxeda/xgmac.c
@@ -353,11 +353,9 @@ struct xgmac_extra_stats {
/* Receive errors */
unsigned long rx_watchdog;
unsigned long rx_da_filter_fail;
- unsigned long rx_sa_filter_fail;
unsigned long rx_payload_error;
unsigned long rx_ip_header_error;
/* Tx/Rx IRQ errors */
- unsigned long tx_undeflow;
unsigned long tx_process_stopped;
unsigned long rx_buf_unav;
unsigned long rx_process_stopped;
@@ -1581,7 +1579,6 @@ static const struct xgmac_stats xgmac_gstrings_stats[] = {
XGMAC_STAT(rx_payload_error),
XGMAC_STAT(rx_ip_header_error),
XGMAC_STAT(rx_da_filter_fail),
- XGMAC_STAT(rx_sa_filter_fail),
XGMAC_STAT(fatal_bus_error),
XGMAC_HW_STAT(rx_watchdog, XGMAC_MMC_RXWATCHDOG),
XGMAC_HW_STAT(tx_vlan, XGMAC_MMC_TXVLANFRAME),
--
1.8.1.2
^ permalink raw reply related
* [PATCH v2 07/11] net: calxedaxgmac: enable interrupts after napi_enable
From: Rob Herring @ 2013-08-30 3:13 UTC (permalink / raw)
To: netdev; +Cc: Lennert Buytenhek, bhutchings, Rob Herring
In-Reply-To: <1377832428-18117-1-git-send-email-robherring2@gmail.com>
From: Rob Herring <rob.herring@calxeda.com>
Fix a race condition where the interrupt handler may have called
napi_schedule before napi_enable is called. This would disable interrupts
and never actually schedule napi to run.
Reported-by: Lennert Buytenhek <buytenh@wantstofly.org>
Signed-off-by: Rob Herring <rob.herring@calxeda.com>
---
v2: No change
drivers/net/ethernet/calxeda/xgmac.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/calxeda/xgmac.c b/drivers/net/ethernet/calxeda/xgmac.c
index 54be55b..530b594 100644
--- a/drivers/net/ethernet/calxeda/xgmac.c
+++ b/drivers/net/ethernet/calxeda/xgmac.c
@@ -959,9 +959,7 @@ static int xgmac_hw_init(struct net_device *dev)
DMA_BUS_MODE_FB | DMA_BUS_MODE_ATDS | DMA_BUS_MODE_AAL;
writel(value, ioaddr + XGMAC_DMA_BUS_MODE);
- /* Enable interrupts */
- writel(DMA_INTR_DEFAULT_MASK, ioaddr + XGMAC_DMA_STATUS);
- writel(DMA_INTR_DEFAULT_MASK, ioaddr + XGMAC_DMA_INTR_ENA);
+ writel(0, ioaddr + XGMAC_DMA_INTR_ENA);
/* Mask power mgt interrupt */
writel(XGMAC_INT_STAT_PMTIM, ioaddr + XGMAC_INT_STAT);
@@ -1029,6 +1027,10 @@ static int xgmac_open(struct net_device *dev)
napi_enable(&priv->napi);
netif_start_queue(dev);
+ /* Enable interrupts */
+ writel(DMA_INTR_DEFAULT_MASK, ioaddr + XGMAC_DMA_STATUS);
+ writel(DMA_INTR_DEFAULT_MASK, ioaddr + XGMAC_DMA_INTR_ENA);
+
return 0;
}
--
1.8.1.2
^ permalink raw reply related
* [PATCH v2 06/11] net: calxedaxgmac: fix race with tx queue stop/wake
From: Rob Herring @ 2013-08-30 3:13 UTC (permalink / raw)
To: netdev; +Cc: Lennert Buytenhek, bhutchings, Rob Herring
In-Reply-To: <1377832428-18117-1-git-send-email-robherring2@gmail.com>
From: Rob Herring <rob.herring@calxeda.com>
Since the xgmac transmit start and completion work locklessly, it is
possible for xgmac_xmit to stop the tx queue after the xgmac_tx_complete
has run resulting in the tx queue never being woken up. Fix this by
ensuring that ring buffer index updates are visible and recheck the ring
space after stopping the queue.
The implementation used here was copied from
drivers/net/ethernet/broadcom/tg3.c.
Signed-off-by: Rob Herring <rob.herring@calxeda.com>
---
v2:
- drop netif_tx_lock
- use netif_start_queue instead of netif_wake_queue
drivers/net/ethernet/calxeda/xgmac.c | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/calxeda/xgmac.c b/drivers/net/ethernet/calxeda/xgmac.c
index f630855..54be55b 100644
--- a/drivers/net/ethernet/calxeda/xgmac.c
+++ b/drivers/net/ethernet/calxeda/xgmac.c
@@ -410,6 +410,9 @@ struct xgmac_priv {
#define dma_ring_space(h, t, s) CIRC_SPACE(h, t, s)
#define dma_ring_cnt(h, t, s) CIRC_CNT(h, t, s)
+#define tx_dma_ring_space(p) \
+ dma_ring_space((p)->tx_head, (p)->tx_tail, DMA_TX_RING_SZ)
+
/* XGMAC Descriptor Access Helpers */
static inline void desc_set_buf_len(struct xgmac_dma_desc *p, u32 buf_sz)
{
@@ -886,8 +889,10 @@ static void xgmac_tx_complete(struct xgmac_priv *priv)
priv->tx_tail = dma_ring_incr(entry, DMA_TX_RING_SZ);
}
- if (dma_ring_space(priv->tx_head, priv->tx_tail, DMA_TX_RING_SZ) >
- MAX_SKB_FRAGS)
+ /* Ensure tx_tail is visible to xgmac_xmit */
+ smp_mb();
+ if (unlikely(netif_queue_stopped(priv->dev) &&
+ (tx_dma_ring_space(priv) > MAX_SKB_FRAGS)))
netif_wake_queue(priv->dev);
}
@@ -1125,10 +1130,15 @@ static netdev_tx_t xgmac_xmit(struct sk_buff *skb, struct net_device *dev)
priv->tx_head = dma_ring_incr(entry, DMA_TX_RING_SZ);
- if (dma_ring_space(priv->tx_head, priv->tx_tail, DMA_TX_RING_SZ) <
- MAX_SKB_FRAGS)
+ /* Ensure tx_head update is visible to tx completion */
+ smp_mb();
+ if (unlikely(tx_dma_ring_space(priv) < MAX_SKB_FRAGS)) {
netif_stop_queue(dev);
-
+ /* Ensure netif_stop_queue is visible to tx completion */
+ smp_mb();
+ if (tx_dma_ring_space(priv) > MAX_SKB_FRAGS)
+ netif_start_queue(dev);
+ }
return NETDEV_TX_OK;
}
--
1.8.1.2
^ permalink raw reply related
* [PATCH v2 05/11] net: calxedaxgmac: update ring buffer tx_head after barriers
From: Rob Herring @ 2013-08-30 3:13 UTC (permalink / raw)
To: netdev; +Cc: Lennert Buytenhek, bhutchings, Rob Herring
In-Reply-To: <1377832428-18117-1-git-send-email-robherring2@gmail.com>
From: Rob Herring <rob.herring@calxeda.com>
Ensure that the descriptor writes are visible before the ring buffer head
is updated. Since writel is a barrier, we can simply update the head after
the writel.
Reported-by: Lennert Buytenhek <buytenh@wantstofly.org>
Signed-off-by: Rob Herring <rob.herring@calxeda.com>
---
v2: No change
drivers/net/ethernet/calxeda/xgmac.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/calxeda/xgmac.c b/drivers/net/ethernet/calxeda/xgmac.c
index 64854ad..f630855 100644
--- a/drivers/net/ethernet/calxeda/xgmac.c
+++ b/drivers/net/ethernet/calxeda/xgmac.c
@@ -1121,9 +1121,10 @@ static netdev_tx_t xgmac_xmit(struct sk_buff *skb, struct net_device *dev)
wmb();
desc_set_tx_owner(first, desc_flags | TXDESC_FIRST_SEG);
+ writel(1, priv->base + XGMAC_DMA_TX_POLL);
+
priv->tx_head = dma_ring_incr(entry, DMA_TX_RING_SZ);
- writel(1, priv->base + XGMAC_DMA_TX_POLL);
if (dma_ring_space(priv->tx_head, priv->tx_tail, DMA_TX_RING_SZ) <
MAX_SKB_FRAGS)
netif_stop_queue(dev);
--
1.8.1.2
^ permalink raw reply related
* [PATCH v2 04/11] net: calxedaxgmac: fix possible skb free before tx complete
From: Rob Herring @ 2013-08-30 3:13 UTC (permalink / raw)
To: netdev; +Cc: Lennert Buytenhek, bhutchings, Rob Herring
In-Reply-To: <1377832428-18117-1-git-send-email-robherring2@gmail.com>
From: Rob Herring <rob.herring@calxeda.com>
The TX completion code may have freed an skb before the entire sg list
was transmitted. The DMA unmap calls for the fragments could also get
skipped. Now set the skb pointer on every entry in the ring, not just
the head of the sg list. We then use the FS (first segment) bit in the
descriptors to determine skb head vs. fragment.
This also fixes similar bug in xgmac_free_tx_skbufs where clean-up of
a sg list that wraps at the end of the ring buffer would not get
unmapped.
Signed-off-by: Rob Herring <rob.herring@calxeda.com>
---
v2: No change
drivers/net/ethernet/calxeda/xgmac.c | 55 ++++++++++++++++--------------------
1 file changed, 24 insertions(+), 31 deletions(-)
diff --git a/drivers/net/ethernet/calxeda/xgmac.c b/drivers/net/ethernet/calxeda/xgmac.c
index df7e3e2..64854ad 100644
--- a/drivers/net/ethernet/calxeda/xgmac.c
+++ b/drivers/net/ethernet/calxeda/xgmac.c
@@ -470,6 +470,11 @@ static inline int desc_get_tx_ls(struct xgmac_dma_desc *p)
return le32_to_cpu(p->flags) & TXDESC_LAST_SEG;
}
+static inline int desc_get_tx_fs(struct xgmac_dma_desc *p)
+{
+ return le32_to_cpu(p->flags) & TXDESC_FIRST_SEG;
+}
+
static inline u32 desc_get_buf_addr(struct xgmac_dma_desc *p)
{
return le32_to_cpu(p->buf1_addr);
@@ -796,7 +801,7 @@ static void xgmac_free_rx_skbufs(struct xgmac_priv *priv)
static void xgmac_free_tx_skbufs(struct xgmac_priv *priv)
{
- int i, f;
+ int i;
struct xgmac_dma_desc *p;
if (!priv->tx_skbuff)
@@ -807,16 +812,15 @@ static void xgmac_free_tx_skbufs(struct xgmac_priv *priv)
continue;
p = priv->dma_tx + i;
- dma_unmap_single(priv->device, desc_get_buf_addr(p),
- desc_get_buf_len(p), DMA_TO_DEVICE);
-
- for (f = 0; f < skb_shinfo(priv->tx_skbuff[i])->nr_frags; f++) {
- p = priv->dma_tx + i++;
+ if (desc_get_tx_fs(p))
+ dma_unmap_single(priv->device, desc_get_buf_addr(p),
+ desc_get_buf_len(p), DMA_TO_DEVICE);
+ else
dma_unmap_page(priv->device, desc_get_buf_addr(p),
desc_get_buf_len(p), DMA_TO_DEVICE);
- }
- dev_kfree_skb_any(priv->tx_skbuff[i]);
+ if (desc_get_tx_ls(p))
+ dev_kfree_skb_any(priv->tx_skbuff[i]);
priv->tx_skbuff[i] = NULL;
}
}
@@ -853,8 +857,6 @@ static void xgmac_free_dma_desc_rings(struct xgmac_priv *priv)
*/
static void xgmac_tx_complete(struct xgmac_priv *priv)
{
- int i;
-
while (dma_ring_cnt(priv->tx_head, priv->tx_tail, DMA_TX_RING_SZ)) {
unsigned int entry = priv->tx_tail;
struct sk_buff *skb = priv->tx_skbuff[entry];
@@ -864,33 +866,24 @@ static void xgmac_tx_complete(struct xgmac_priv *priv)
if (desc_get_owner(p))
break;
- /* Verify tx error by looking at the last segment */
- if (desc_get_tx_ls(p))
- desc_get_tx_status(priv, p);
-
netdev_dbg(priv->dev, "tx ring: curr %d, dirty %d\n",
priv->tx_head, priv->tx_tail);
- dma_unmap_single(priv->device, desc_get_buf_addr(p),
- desc_get_buf_len(p), DMA_TO_DEVICE);
-
- priv->tx_skbuff[entry] = NULL;
- priv->tx_tail = dma_ring_incr(entry, DMA_TX_RING_SZ);
-
- if (!skb) {
- continue;
- }
-
- for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
- entry = priv->tx_tail = dma_ring_incr(priv->tx_tail,
- DMA_TX_RING_SZ);
- p = priv->dma_tx + priv->tx_tail;
-
+ if (desc_get_tx_fs(p))
+ dma_unmap_single(priv->device, desc_get_buf_addr(p),
+ desc_get_buf_len(p), DMA_TO_DEVICE);
+ else
dma_unmap_page(priv->device, desc_get_buf_addr(p),
desc_get_buf_len(p), DMA_TO_DEVICE);
+
+ /* Check tx error on the last segment */
+ if (desc_get_tx_ls(p)) {
+ desc_get_tx_status(priv, p);
+ dev_kfree_skb(skb);
}
- dev_kfree_skb(skb);
+ priv->tx_skbuff[entry] = NULL;
+ priv->tx_tail = dma_ring_incr(entry, DMA_TX_RING_SZ);
}
if (dma_ring_space(priv->tx_head, priv->tx_tail, DMA_TX_RING_SZ) >
@@ -1110,7 +1103,7 @@ static netdev_tx_t xgmac_xmit(struct sk_buff *skb, struct net_device *dev)
entry = dma_ring_incr(entry, DMA_TX_RING_SZ);
desc = priv->dma_tx + entry;
- priv->tx_skbuff[entry] = NULL;
+ priv->tx_skbuff[entry] = skb;
desc_set_buf_addr_and_size(desc, paddr, len);
if (i < (nfrags - 1))
--
1.8.1.2
^ permalink raw reply related
* [PATCH v2 02/11] net: calxedaxgmac: read correct field in xgmac_desc_get_buf_len
From: Rob Herring @ 2013-08-30 3:13 UTC (permalink / raw)
To: netdev; +Cc: Lennert Buytenhek, bhutchings, Rob Herring
In-Reply-To: <1377832428-18117-1-git-send-email-robherring2@gmail.com>
From: Rob Herring <rob.herring@calxeda.com>
xgmac_desc_get_buf_len appears to have a copy/paste error. flags is the
wrong field to read. We should be reading buf_size field. cpu_to_le32
should also be le32_to_cpu. This never really mattered as this function
is only used for DMA mapping calls which happen to be nops with coherent
DMA.
Signed-off-by: Rob Herring <rob.herring@calxeda.com>
---
v2: No change
drivers/net/ethernet/calxeda/xgmac.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/calxeda/xgmac.c b/drivers/net/ethernet/calxeda/xgmac.c
index 71f6720..d41af68 100644
--- a/drivers/net/ethernet/calxeda/xgmac.c
+++ b/drivers/net/ethernet/calxeda/xgmac.c
@@ -421,7 +421,7 @@ static inline void desc_set_buf_len(struct xgmac_dma_desc *p, u32 buf_sz)
static inline int desc_get_buf_len(struct xgmac_dma_desc *p)
{
- u32 len = cpu_to_le32(p->flags);
+ u32 len = le32_to_cpu(p->buf_size);
return (len & DESC_BUFFER1_SZ_MASK) +
((len & DESC_BUFFER2_SZ_MASK) >> DESC_BUFFER2_SZ_OFFSET);
}
--
1.8.1.2
^ permalink raw reply related
* [PATCH v2 03/11] net: calxedaxgmac: fix race between xgmac_tx_complete and xgmac_tx_err
From: Rob Herring @ 2013-08-30 3:13 UTC (permalink / raw)
To: netdev; +Cc: Lennert Buytenhek, bhutchings, Rob Herring
In-Reply-To: <1377832428-18117-1-git-send-email-robherring2@gmail.com>
From: Rob Herring <rob.herring@calxeda.com>
It is possible for the xgmac_tx_complete to run concurrently with
xgmac_tx_err since there are no locks. Fix this by moving the tx
error handling to a workqueue so we can disable napi while we reset
the transmitter.
Reported-by: Andreas Herrmann <andreas.herrmann@calxeda.com>
Signed-off-by: Rob Herring <rob.herring@calxeda.com>
---
v2: No change
drivers/net/ethernet/calxeda/xgmac.c | 38 +++++++++++++++++-------------------
1 file changed, 18 insertions(+), 20 deletions(-)
diff --git a/drivers/net/ethernet/calxeda/xgmac.c b/drivers/net/ethernet/calxeda/xgmac.c
index d41af68..df7e3e2 100644
--- a/drivers/net/ethernet/calxeda/xgmac.c
+++ b/drivers/net/ethernet/calxeda/xgmac.c
@@ -393,6 +393,7 @@ struct xgmac_priv {
char rx_pause;
char tx_pause;
int wolopts;
+ struct work_struct tx_timeout_work;
};
/* XGMAC Configuration Settings */
@@ -897,21 +898,18 @@ static void xgmac_tx_complete(struct xgmac_priv *priv)
netif_wake_queue(priv->dev);
}
-/**
- * xgmac_tx_err:
- * @priv: pointer to the private device structure
- * Description: it cleans the descriptors and restarts the transmission
- * in case of errors.
- */
-static void xgmac_tx_err(struct xgmac_priv *priv)
+static void xgmac_tx_timeout_work(struct work_struct *work)
{
- u32 reg, value, inten;
+ u32 reg, value;
+ struct xgmac_priv *priv =
+ container_of(work, struct xgmac_priv, tx_timeout_work);
- netif_stop_queue(priv->dev);
+ napi_disable(&priv->napi);
- inten = readl(priv->base + XGMAC_DMA_INTR_ENA);
writel(0, priv->base + XGMAC_DMA_INTR_ENA);
+ netif_tx_lock(priv->dev);
+
reg = readl(priv->base + XGMAC_DMA_CONTROL);
writel(reg & ~DMA_CONTROL_ST, priv->base + XGMAC_DMA_CONTROL);
do {
@@ -927,9 +925,15 @@ static void xgmac_tx_err(struct xgmac_priv *priv)
writel(DMA_STATUS_TU | DMA_STATUS_TPS | DMA_STATUS_NIS | DMA_STATUS_AIS,
priv->base + XGMAC_DMA_STATUS);
- writel(inten, priv->base + XGMAC_DMA_INTR_ENA);
+ netif_tx_unlock(priv->dev);
netif_wake_queue(priv->dev);
+
+ napi_enable(&priv->napi);
+
+ /* Enable interrupts */
+ writel(DMA_INTR_DEFAULT_MASK, priv->base + XGMAC_DMA_STATUS);
+ writel(DMA_INTR_DEFAULT_MASK, priv->base + XGMAC_DMA_INTR_ENA);
}
static int xgmac_hw_init(struct net_device *dev)
@@ -1225,9 +1229,7 @@ static int xgmac_poll(struct napi_struct *napi, int budget)
static void xgmac_tx_timeout(struct net_device *dev)
{
struct xgmac_priv *priv = netdev_priv(dev);
-
- /* Clear Tx resources and restart transmitting again */
- xgmac_tx_err(priv);
+ schedule_work(&priv->tx_timeout_work);
}
/**
@@ -1366,7 +1368,6 @@ static irqreturn_t xgmac_pmt_interrupt(int irq, void *dev_id)
static irqreturn_t xgmac_interrupt(int irq, void *dev_id)
{
u32 intr_status;
- bool tx_err = false;
struct net_device *dev = (struct net_device *)dev_id;
struct xgmac_priv *priv = netdev_priv(dev);
struct xgmac_extra_stats *x = &priv->xstats;
@@ -1396,16 +1397,12 @@ static irqreturn_t xgmac_interrupt(int irq, void *dev_id)
if (intr_status & DMA_STATUS_TPS) {
netdev_err(priv->dev, "transmit process stopped\n");
x->tx_process_stopped++;
- tx_err = true;
+ schedule_work(&priv->tx_timeout_work);
}
if (intr_status & DMA_STATUS_FBI) {
netdev_err(priv->dev, "fatal bus error\n");
x->fatal_bus_error++;
- tx_err = true;
}
-
- if (tx_err)
- xgmac_tx_err(priv);
}
/* TX/RX NORMAL interrupts */
@@ -1708,6 +1705,7 @@ static int xgmac_probe(struct platform_device *pdev)
ndev->netdev_ops = &xgmac_netdev_ops;
SET_ETHTOOL_OPS(ndev, &xgmac_ethtool_ops);
spin_lock_init(&priv->stats_lock);
+ INIT_WORK(&priv->tx_timeout_work, xgmac_tx_timeout_work);
priv->device = &pdev->dev;
priv->dev = ndev;
--
1.8.1.2
^ permalink raw reply related
* [PATCH v2 01/11] net: calxedaxgmac: remove NETIF_F_FRAGLIST setting
From: Rob Herring @ 2013-08-30 3:13 UTC (permalink / raw)
To: netdev; +Cc: Lennert Buytenhek, bhutchings, Rob Herring
From: Rob Herring <rob.herring@calxeda.com>
The xgmac does not actually handle frag lists, so this option should not
be set. It does not appear to have had any impact though.
Reported-by: Lennert Buytenhek <buytenh@wantstofly.org>
Signed-off-by: Rob Herring <rob.herring@calxeda.com>
---
v2: No change
drivers/net/ethernet/calxeda/xgmac.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/calxeda/xgmac.c b/drivers/net/ethernet/calxeda/xgmac.c
index 7cb148c..71f6720 100644
--- a/drivers/net/ethernet/calxeda/xgmac.c
+++ b/drivers/net/ethernet/calxeda/xgmac.c
@@ -1759,7 +1759,7 @@ static int xgmac_probe(struct platform_device *pdev)
if (device_can_wakeup(priv->device))
priv->wolopts = WAKE_MAGIC; /* Magic Frame as default */
- ndev->hw_features = NETIF_F_SG | NETIF_F_FRAGLIST | NETIF_F_HIGHDMA;
+ ndev->hw_features = NETIF_F_SG | NETIF_F_HIGHDMA;
if (readl(priv->base + XGMAC_DMA_HW_FEATURE) & DMA_HW_FEAT_TXCOESEL)
ndev->hw_features |= NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
NETIF_F_RXCSUM;
--
1.8.1.2
^ permalink raw reply related
* Re: [PATCH v3 net-next] tcp: TSO packets automatic sizing
From: Jason Wang @ 2013-08-30 3:02 UTC (permalink / raw)
To: Eric Dumazet
Cc: David Miller, netdev, Neal Cardwell, Yuchung Cheng, Van Jacobson,
Tom Herbert, Michael S. Tsirkin
In-Reply-To: <1377686084.8828.175.camel@edumazet-glaptop>
On 08/28/2013 06:34 PM, Eric Dumazet wrote:
> On Wed, 2013-08-28 at 15:37 +0800, Jason Wang wrote:
>
>>> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
>>> index 884efff..e63ae4c 100644
>>> --- a/net/ipv4/tcp_output.c
>>> +++ b/net/ipv4/tcp_output.c
>>> @@ -1631,7 +1631,7 @@ static bool tcp_tso_should_defer(struct sock *sk, struct sk_buff *skb)
>>>
>>> /* If a full-sized TSO skb can be sent, do it. */
>>> if (limit >= min_t(unsigned int, sk->sk_gso_max_size,
>>> - sk->sk_gso_max_segs * tp->mss_cache))
>>> + tp->xmit_size_goal_segs * tp->mss_cache))
>>> goto send_now;
>> A question is: Does this really guarantee the minimal TSO segments
>> excluding the case of small available window? The skb->len may be much
>> smaller and can still be sent here. Maybe we should check skb->len also?
> tcp_tso_should_defer() is all about hoping the application will
> 'complete' the last skb in write queue with more payload in the near
> future.
>
> skb->len might therefore change because sendmsg()/sendpage() will add
> new stuff in the skb.
Ture, but sometimes the application may be slow to fill the bytes into
skb. Especially the application run in virt guest with multiqueue. In
the case, the application in guest tends to be slower than the
nic(virtio-net) which does the transmission through a host thread
(vhost). Looks like current defer algorithm could not do this very well
and if we want to force the batching of 64K packet, tcp_min_tso_segs
could not works well also.
> We try hard to not remove tcp_tso_should_defer() and take the best of
> it. We have not yet decided to add a real timer instead of relying on
> upcoming ACKS.
>
> Neal has an idea/patch to avoid a defer depending on
> the expected time of following ACKS.
>
> By making the TSO sizes smaller for low rates, we avoid these stalls
> from tcp_tso_should_defer(), because an incoming ACK has normally freed
> enough window to send the next packet in write queue without the need to
> split it into two parts.
>
> These changes are fundamental to use delay based congestion modules like
> Vegas/Westwood and experimental new ones, without having to disable TSO.
>
>
>
^ permalink raw reply
* RE: [PATCH] net: fec: fix build warning of used uninitialized variable
From: Duan Fugang-B38611 @ 2013-08-30 2:47 UTC (permalink / raw)
To: davem@davemloft.net
Cc: netdev@vger.kernel.org, shawn.guo@linaro.org,
bhutchings@solarflare.com, Estevam Fabio-R49496,
stephen@networkplumber.org, Li Frank-B20596
In-Reply-To: <1F990F8245A4214A8CC4BFFBD9F790F908B16AA6@039-SN1MPN1-002.039d.mgd.msft.net>
Ping...
> -----Original Message-----
> From: Li Frank-B20596
> Sent: Thursday, August 22, 2013 10:16 PM
> To: Duan Fugang-B38611; Zhou Luwei-B45643; davem@davemloft.net
> Cc: netdev@vger.kernel.org; shawn.guo@linaro.org;
> bhutchings@solarflare.com; Estevam Fabio-R49496;
> stephen@networkplumber.org
> Subject: RE: [PATCH] net: fec: fix build warning of used uninitialized
> variable
>
> > diff --git a/drivers/net/ethernet/freescale/fec_main.c
> > b/drivers/net/ethernet/freescale/fec_main.c
> > index 77ea0db..4ea1555 100644
> > --- a/drivers/net/ethernet/freescale/fec_main.c
> > +++ b/drivers/net/ethernet/freescale/fec_main.c
> > @@ -835,7 +835,7 @@ fec_enet_rx(struct net_device *ndev, int budget)
> > int pkt_received = 0;
> > struct bufdesc_ex *ebdp = NULL;
> > bool vlan_packet_rcvd = false;
> > - u16 vlan_tag;
> > + u16 vlan_tag = 0;
> >
> > #ifdef CONFIG_M532x
> > flush_cache_all();
>
> Acked
>
> > --
> > 1.7.1
^ permalink raw reply
* [PATCH iproute2] pkt_sched: fq: Fair Queue packet scheduler
From: Eric Dumazet @ 2013-08-30 2:30 UTC (permalink / raw)
To: David Miller, Stephen Hemminger; +Cc: netdev, ycheng, ncardwell
In-Reply-To: <20130829.214730.2158572637993114547.davem@davemloft.net>
From: Eric Dumazet <edumazet@google.com>
Support for FQ packet scheduler
$ tc qd add dev eth0 root fq help
Usage: ... fq [ limit PACKETS ] [ flow_limit PACKETS ]
[ quantum BYTES ] [ initial_quantum BYTES ]
[ maxrate RATE ] [ buckets NUMBER ]
[ [no]pacing ]
$ tc -s -d qd
qdisc fq 8002: dev eth0 root refcnt 32 limit 10000p flow_limit 100p
buckets 256 quantum 3028 initial_quantum 15140
Sent 216532416 bytes 148395 pkt (dropped 0, overlimits 0 requeues 14)
backlog 0b 0p requeues 14
511 flows (511 inactive, 0 throttled)
110 gc, 0 highprio, 0 retrans, 1143 throttled, 0 flows_plimit
limit : max number of packets on whole Qdisc (default 10000)
flow_limit : max number of packets per flow (default 100)
quantum : the max deficit per RR round (default is 2 MTU)
initial_quantum : initial credit for new flows (default is 10 MTU)
maxrate : max per flow rate (default : unlimited)
buckets : number of RB trees (default : 1024) in hash table.
(consumes 8 bytes per bucket)
[no]pacing : disable/enable pacing (default is enable)
Usage :
tc qdisc add dev $ETH root fq
tc qdisc del dev $ETH root 2>/dev/null
tc qdisc add dev $ETH root handle 1: mq
for i in `seq 1 4`
do
tc qdisc add dev $ETH parent 1:$i est 1sec 4sec fq
done
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/linux/pkt_sched.h | 51 ++++++
tc/Makefile | 1
tc/q_fq.c | 279 ++++++++++++++++++++++++++++++++++++
3 files changed, 330 insertions(+), 1 deletion(-)
diff --git a/include/linux/pkt_sched.h b/include/linux/pkt_sched.h
index dbd71b0..9b82913 100644
--- a/include/linux/pkt_sched.h
+++ b/include/linux/pkt_sched.h
@@ -73,9 +73,17 @@ struct tc_estimator {
#define TC_H_ROOT (0xFFFFFFFFU)
#define TC_H_INGRESS (0xFFFFFFF1U)
+/* Need to corrospond to iproute2 tc/tc_core.h "enum link_layer" */
+enum tc_link_layer {
+ TC_LINKLAYER_UNAWARE, /* Indicate unaware old iproute2 util */
+ TC_LINKLAYER_ETHERNET,
+ TC_LINKLAYER_ATM,
+};
+#define TC_LINKLAYER_MASK 0x0F /* limit use to lower 4 bits */
+
struct tc_ratespec {
unsigned char cell_log;
- unsigned char __reserved;
+ __u8 linklayer; /* lower 4 bits */
unsigned short overhead;
short cell_align;
unsigned short mpu;
@@ -736,4 +744,45 @@ struct tc_fq_codel_xstats {
};
};
+/* FQ */
+
+enum {
+ TCA_FQ_UNSPEC,
+
+ TCA_FQ_PLIMIT, /* limit of total number of packets in queue */
+
+ TCA_FQ_FLOW_PLIMIT, /* limit of packets per flow */
+
+ TCA_FQ_QUANTUM, /* RR quantum */
+
+ TCA_FQ_INITIAL_QUANTUM, /* RR quantum for new flow */
+
+ TCA_FQ_RATE_ENABLE, /* enable/disable rate limiting */
+
+ TCA_FQ_FLOW_DEFAULT_RATE,/* for sockets with unspecified sk_rate,
+ * use the following rate
+ */
+
+ TCA_FQ_FLOW_MAX_RATE, /* per flow max rate */
+
+ TCA_FQ_BUCKETS_LOG, /* log2(number of buckets) */
+ __TCA_FQ_MAX
+};
+
+#define TCA_FQ_MAX (__TCA_FQ_MAX - 1)
+
+struct tc_fq_qd_stats {
+ __u64 gc_flows;
+ __u64 highprio_packets;
+ __u64 tcp_retrans;
+ __u64 throttled;
+ __u64 flows_plimit;
+ __u64 pkts_too_long;
+ __u64 allocation_errors;
+ __s64 time_next_delayed_flow;
+ __u32 flows;
+ __u32 inactive_flows;
+ __u32 throttled_flows;
+ __u32 pad;
+};
#endif
diff --git a/tc/Makefile b/tc/Makefile
index f26e764..1eeabd8 100644
--- a/tc/Makefile
+++ b/tc/Makefile
@@ -50,6 +50,7 @@ TCMODULES += em_meta.o
TCMODULES += q_mqprio.o
TCMODULES += q_codel.o
TCMODULES += q_fq_codel.o
+TCMODULES += q_fq.o
ifeq ($(TC_CONFIG_IPSET), y)
ifeq ($(TC_CONFIG_XT), y)
diff --git a/tc/q_fq.c b/tc/q_fq.c
new file mode 100644
index 0000000..c0bcdb9
--- /dev/null
+++ b/tc/q_fq.c
@@ -0,0 +1,279 @@
+/*
+ * Fair Queue
+ *
+ * Copyright (C) 2013 Eric Dumazet <edumazet@google.com>
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions, and the following disclaimer,
+ * without modification.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ * 3. The names of the authors may not be used to endorse or promote products
+ * derived from this software without specific prior written permission.
+ *
+ * Alternatively, provided that this notice is retained in full, this
+ * software may be distributed under the terms of the GNU General
+ * Public License ("GPL") version 2, in which case the provisions of the
+ * GPL apply INSTEAD OF those given above.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
+ * DAMAGE.
+ *
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <syslog.h>
+#include <fcntl.h>
+#include <sys/socket.h>
+#include <netinet/in.h>
+#include <arpa/inet.h>
+#include <string.h>
+
+#include "utils.h"
+#include "tc_util.h"
+
+static void explain(void)
+{
+ fprintf(stderr, "Usage: ... fq [ limit PACKETS ] [ flow_limit PACKETS ]\n");
+ fprintf(stderr, " [ quantum BYTES ] [ initial_quantum BYTES ]\n");
+ fprintf(stderr, " [ maxrate RATE ] [ buckets NUMBER ]\n");
+ fprintf(stderr, " [ [no]pacing ]\n");
+}
+
+static unsigned int ilog2(unsigned int val)
+{
+ unsigned int res = 0;
+
+ val--;
+ while (val) {
+ res++;
+ val >>= 1;
+ }
+ return res;
+}
+
+static int fq_parse_opt(struct qdisc_util *qu, int argc, char **argv,
+ struct nlmsghdr *n)
+{
+ unsigned int plimit = ~0U;
+ unsigned int flow_plimit = ~0U;
+ unsigned int quantum = ~0U;
+ unsigned int initial_quantum = ~0U;
+ unsigned int buckets = 0;
+ unsigned int maxrate = ~0U;
+ unsigned int defrate = ~0U;
+ int pacing = -1;
+ struct rtattr *tail;
+
+ while (argc > 0) {
+ if (strcmp(*argv, "limit") == 0) {
+ NEXT_ARG();
+ if (get_unsigned(&plimit, *argv, 0)) {
+ fprintf(stderr, "Illegal \"limit\"\n");
+ return -1;
+ }
+ } else if (strcmp(*argv, "flow_limit") == 0) {
+ NEXT_ARG();
+ if (get_unsigned(&flow_plimit, *argv, 0)) {
+ fprintf(stderr, "Illegal \"flow_limit\"\n");
+ return -1;
+ }
+ } else if (strcmp(*argv, "buckets") == 0) {
+ NEXT_ARG();
+ if (get_unsigned(&buckets, *argv, 0)) {
+ fprintf(stderr, "Illegal \"buckets\"\n");
+ return -1;
+ }
+ } else if (strcmp(*argv, "maxrate") == 0) {
+ NEXT_ARG();
+ if (get_rate(&maxrate, *argv)) {
+ fprintf(stderr, "Illegal \"maxrate\"\n");
+ return -1;
+ }
+ } else if (strcmp(*argv, "defrate") == 0) {
+ NEXT_ARG();
+ if (get_rate(&defrate, *argv)) {
+ fprintf(stderr, "Illegal \"defrate\"\n");
+ return -1;
+ }
+ } else if (strcmp(*argv, "quantum") == 0) {
+ NEXT_ARG();
+ if (get_unsigned(&quantum, *argv, 0)) {
+ fprintf(stderr, "Illegal \"quantum\"\n");
+ return -1;
+ }
+ } else if (strcmp(*argv, "initial_quantum") == 0) {
+ NEXT_ARG();
+ if (get_unsigned(&initial_quantum, *argv, 0)) {
+ fprintf(stderr, "Illegal \"initial_quantum\"\n");
+ return -1;
+ }
+ } else if (strcmp(*argv, "pacing") == 0) {
+ pacing = 1;
+ } else if (strcmp(*argv, "nopacing") == 0) {
+ pacing = 0;
+ } else if (strcmp(*argv, "help") == 0) {
+ explain();
+ return -1;
+ } else {
+ fprintf(stderr, "What is \"%s\"?\n", *argv);
+ explain();
+ return -1;
+ }
+ argc--; argv++;
+ }
+
+ tail = NLMSG_TAIL(n);
+ addattr_l(n, 1024, TCA_OPTIONS, NULL, 0);
+ if (buckets) {
+ unsigned int log = ilog2(buckets);
+
+ addattr_l(n, 1024, TCA_FQ_BUCKETS_LOG,
+ &log, sizeof(log));
+ }
+ if (plimit != ~0U)
+ addattr_l(n, 1024, TCA_FQ_PLIMIT,
+ &plimit, sizeof(plimit));
+ if (flow_plimit != ~0U)
+ addattr_l(n, 1024, TCA_FQ_FLOW_PLIMIT,
+ &flow_plimit, sizeof(flow_plimit));
+ if (quantum != ~0U)
+ addattr_l(n, 1024, TCA_FQ_QUANTUM, &quantum, sizeof(quantum));
+ if (initial_quantum != ~0U)
+ addattr_l(n, 1024, TCA_FQ_INITIAL_QUANTUM,
+ &initial_quantum, sizeof(initial_quantum));
+ if (pacing != -1)
+ addattr_l(n, 1024, TCA_FQ_RATE_ENABLE,
+ &pacing, sizeof(pacing));
+ if (maxrate != ~0U)
+ addattr_l(n, 1024, TCA_FQ_FLOW_MAX_RATE,
+ &maxrate, sizeof(maxrate));
+ if (defrate != ~0U)
+ addattr_l(n, 1024, TCA_FQ_FLOW_DEFAULT_RATE,
+ &defrate, sizeof(defrate));
+ tail->rta_len = (void *) NLMSG_TAIL(n) - (void *) tail;
+ return 0;
+}
+
+static int fq_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt)
+{
+ struct rtattr *tb[TCA_FQ_MAX + 1];
+ unsigned int plimit, flow_plimit;
+ unsigned int buckets_log;
+ int pacing;
+ unsigned int rate, quantum;
+ SPRINT_BUF(b1);
+
+ if (opt == NULL)
+ return 0;
+
+ parse_rtattr_nested(tb, TCA_FQ_MAX, opt);
+
+ if (tb[TCA_FQ_PLIMIT] &&
+ RTA_PAYLOAD(tb[TCA_FQ_PLIMIT]) >= sizeof(__u32)) {
+ plimit = rta_getattr_u32(tb[TCA_FQ_PLIMIT]);
+ fprintf(f, "limit %up ", plimit);
+ }
+ if (tb[TCA_FQ_FLOW_PLIMIT] &&
+ RTA_PAYLOAD(tb[TCA_FQ_FLOW_PLIMIT]) >= sizeof(__u32)) {
+ flow_plimit = rta_getattr_u32(tb[TCA_FQ_FLOW_PLIMIT]);
+ fprintf(f, "flow_limit %up ", flow_plimit);
+ }
+ if (tb[TCA_FQ_BUCKETS_LOG] &&
+ RTA_PAYLOAD(tb[TCA_FQ_BUCKETS_LOG]) >= sizeof(__u32)) {
+ buckets_log = rta_getattr_u32(tb[TCA_FQ_BUCKETS_LOG]);
+ fprintf(f, "buckets %u ", 1U << buckets_log);
+ }
+ if (tb[TCA_FQ_RATE_ENABLE] &&
+ RTA_PAYLOAD(tb[TCA_FQ_RATE_ENABLE]) >= sizeof(int)) {
+ pacing = rta_getattr_u32(tb[TCA_FQ_RATE_ENABLE]);
+ if (pacing == 0)
+ fprintf(f, "nopacing ");
+ }
+ if (tb[TCA_FQ_QUANTUM] &&
+ RTA_PAYLOAD(tb[TCA_FQ_QUANTUM]) >= sizeof(__u32)) {
+ quantum = rta_getattr_u32(tb[TCA_FQ_QUANTUM]);
+ fprintf(f, "quantum %u ", quantum);
+ }
+ if (tb[TCA_FQ_INITIAL_QUANTUM] &&
+ RTA_PAYLOAD(tb[TCA_FQ_INITIAL_QUANTUM]) >= sizeof(__u32)) {
+ quantum = rta_getattr_u32(tb[TCA_FQ_INITIAL_QUANTUM]);
+ fprintf(f, "initial_quantum %u ", quantum);
+ }
+ if (tb[TCA_FQ_FLOW_MAX_RATE] &&
+ RTA_PAYLOAD(tb[TCA_FQ_FLOW_MAX_RATE]) >= sizeof(__u32)) {
+ rate = rta_getattr_u32(tb[TCA_FQ_FLOW_MAX_RATE]);
+
+ if (rate != ~0U)
+ fprintf(f, "maxrate %s ", sprint_rate(rate, b1));
+ }
+ if (tb[TCA_FQ_FLOW_DEFAULT_RATE] &&
+ RTA_PAYLOAD(tb[TCA_FQ_FLOW_DEFAULT_RATE]) >= sizeof(__u32)) {
+ rate = rta_getattr_u32(tb[TCA_FQ_FLOW_DEFAULT_RATE]);
+
+ if (rate != 0)
+ fprintf(f, "defrate %s ", sprint_rate(rate, b1));
+ }
+
+ return 0;
+}
+
+static int fq_print_xstats(struct qdisc_util *qu, FILE *f,
+ struct rtattr *xstats)
+{
+ struct tc_fq_qd_stats *st;
+
+ if (xstats == NULL)
+ return 0;
+
+ if (RTA_PAYLOAD(xstats) < sizeof(*st))
+ return -1;
+
+ st = RTA_DATA(xstats);
+
+ fprintf(f, " %u flows (%u inactive, %u throttled)",
+ st->flows, st->inactive_flows, st->throttled_flows);
+
+ if (st->time_next_delayed_flow > 0)
+ fprintf(f, ", next packet delay %llu ns", st->time_next_delayed_flow);
+
+ fprintf(f, "\n %llu gc, %llu highprio",
+ st->gc_flows, st->highprio_packets);
+
+ if (st->tcp_retrans)
+ fprintf(f, ", %llu retrans", st->tcp_retrans);
+
+ fprintf(f, ", %llu throttled", st->throttled);
+
+ if (st->flows_plimit)
+ fprintf(f, ", %llu flows_plimit", st->flows_plimit);
+
+ if (st->pkts_too_long || st->allocation_errors)
+ fprintf(f, "\n %llu too long pkts, %llu alloc errors\n",
+ st->pkts_too_long, st->allocation_errors);
+
+ return 0;
+}
+
+struct qdisc_util fq_qdisc_util = {
+ .id = "fq",
+ .parse_qopt = fq_parse_opt,
+ .print_qopt = fq_print_opt,
+ .print_xstats = fq_print_xstats,
+};
^ permalink raw reply related
* Re: [PATCH net-next] qdisc: allow setting default queuing discipline
From: David Miller @ 2013-08-30 2:25 UTC (permalink / raw)
To: greearb; +Cc: stephen, netdev
In-Reply-To: <521D34CC.205@candelatech.com>
From: Ben Greear <greearb@candelatech.com>
Date: Tue, 27 Aug 2013 16:22:52 -0700
> On 08/27/2013 04:19 PM, Stephen Hemminger wrote:
>> +CoDel (codel) or fair queue CoDel (fq_codel). Dont' use queuing
>> disciplines
>
> nit: Don't has typo above....
I fixed this when I commited Stephen's patch, thanks.
^ permalink raw reply
* Re: [PATCH net-next] qdisc: allow setting default queuing discipline
From: David Miller @ 2013-08-30 2:23 UTC (permalink / raw)
To: stephen; +Cc: netdev
In-Reply-To: <20130827161908.26062336@nehalam.linuxnetplumber.net>
From: Stephen Hemminger <stephen@networkplumber.org>
Date: Tue, 27 Aug 2013 16:19:08 -0700
> By default, the pfifo_fast queue discipline has been used by default
> for all devices. But we have better choices now.
>
> This patch allow setting the default queueing discipline with sysctl.
> This allows easy use of better queueing disciplines on all devices
> without having to use tc qdisc scripts. It is intended to allow
> an easy path for distributions to make fq_codel or sfq the default
> qdisc.
>
> This patch also makes pfifo_fast more of a first class qdisc, since
> it is now possible to manually override the default and explicitly
> use pfifo_fast. The behavior for systems who do not use the sysctl
> is unchanged, they still get pfifo_fast
>
> Also removes leftover random # in sysctl net core.
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Applied, thanks Stpehen.
^ permalink raw reply
* Re: [PATCH net-next] drivers:net: Convert dma_alloc_coherent(...__GFP_ZERO) to dma_zalloc_coherent
From: David Miller @ 2013-08-30 2:09 UTC (permalink / raw)
To: joe; +Cc: netdev, linux-kernel, e1000-devel, linux-wireless, b43-dev, users
In-Reply-To: <1377582323.2658.10.camel@joe-AO722>
From: Joe Perches <joe@perches.com>
Date: Mon, 26 Aug 2013 22:45:23 -0700
> __GFP_ZERO is an uncommon flag and perhaps is better
> not used. static inline dma_zalloc_coherent exists
> so convert the uses of dma_alloc_coherent with __GFP_ZERO
> to the more common kernel style with zalloc.
>
> Remove memset from the static inline dma_zalloc_coherent
> and add just one use of __GFP_ZERO instead.
>
> Trivially reduces the size of the existing uses of
> dma_zalloc_coherent.
>
> Realign arguments as appropriate.
>
> Signed-off-by: Joe Perches <joe@perches.com>
Applied, thanks a lot Joe.
^ permalink raw reply
* RE: [patch v2] net/fec: cleanup types in fec_get_mac()
From: Duan Fugang-B38611 @ 2013-08-30 2:02 UTC (permalink / raw)
To: Ben Hutchings, Dan Carpenter
Cc: Grant Likely, Rob Herring, David S. Miller, Estevam Fabio-R49496,
Li Frank-B20596, Jim Baxter, netdev@vger.kernel.org,
devicetree@vger.kernel.org, kernel-janitors@vger.kernel.org
In-Reply-To: <1377802138.5372.12.camel@bwh-desktop.uk.level5networks.com>
From: Ben Hutchings [mailto:bhutchings@solarflare.com]
Data: Friday, August 30, 2013 2:49 AM
> To: Dan Carpenter
> Cc: Grant Likely; Rob Herring; David S. Miller; Estevam Fabio-R49496; Li
> Frank-B20596; Jim Baxter; Duan Fugang-B38611; netdev@vger.kernel.org;
> devicetree@vger.kernel.org; kernel-janitors@vger.kernel.org
> Subject: Re: [patch v2] net/fec: cleanup types in fec_get_mac()
>
> On Thu, 2013-08-29 at 11:25 +0300, Dan Carpenter wrote:
> > My static checker complains that on some arches unsigned longs can be
> > 8 characters which is larger than the buffer is only 6 chars.
> > Additionally, Ben Hutchings points out that the buffer actually holds
> > big endian data and the buffer we are reading from is CPU endian.
>
> It's not really as clear-cut as that. :-) But I think it's slightly more
> logical this way.
>
Yes, it is not clear, pls remove somebody's name from the commit log.
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> Reviewed-by: Ben Hutchings <bhutchings@solarflare.com>
>
> > ---
> > v2: fix endian annotations and reverse the beXX_to_cpu() calls so that
> > they say cpu_to_beXX().
> >
> > diff --git a/drivers/net/ethernet/freescale/fec_main.c
> > b/drivers/net/ethernet/freescale/fec_main.c
> > index fdf9307..0b12866 100644
> > --- a/drivers/net/ethernet/freescale/fec_main.c
> > +++ b/drivers/net/ethernet/freescale/fec_main.c
> > @@ -1100,10 +1100,10 @@ static void fec_get_mac(struct net_device *ndev)
> > * 4) FEC mac registers set by bootloader
> > */
> > if (!is_valid_ether_addr(iap)) {
> > - *((unsigned long *) &tmpaddr[0]) =
> > - be32_to_cpu(readl(fep->hwp + FEC_ADDR_LOW));
> > - *((unsigned short *) &tmpaddr[4]) =
> > - be16_to_cpu(readl(fep->hwp + FEC_ADDR_HIGH) >> 16);
> > + *((__be32 *) &tmpaddr[0]) =
> > + cpu_to_be32(readl(fep->hwp + FEC_ADDR_LOW));
> > + *((__be16 *) &tmpaddr[4]) =
> > + cpu_to_be16(readl(fep->hwp + FEC_ADDR_HIGH) >> 16);
> > iap = &tmpaddr[0];
> > }
> >
>
> --
> Ben Hutchings, Staff Engineer, Solarflare Not speaking for my employer;
> that's the marketing department's job.
> They asked us to note that Solarflare product names are trademarked.
>
^ permalink raw reply
* RE: [PATCH] net: fec: add the initial to set the physical mac address
From: Duan Fugang-B38611 @ 2013-08-30 1:59 UTC (permalink / raw)
To: Lucas Stach, davem@davemloft.net
Cc: davem@davemloft.net, netdev@vger.kernel.org,
bhutchings@solarflare.com, Estevam Fabio-R49496,
stephen@networkplumber.org
In-Reply-To: <9848F2DB572E5649BA045B288BE08FBE01653FEA@039-SN2MPN1-021.039d.mgd.msft.net>
Ping...
> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]
> On Behalf Of Duan Fugang-B38611
> Sent: Wednesday, August 28, 2013 10:11 AM
> To: Lucas Stach
> Cc: Li Frank-B20596; Zhou Luwei-B45643; davem@davemloft.net;
> netdev@vger.kernel.org; shawn.guo@linaro.org; bhutchings@solarflare.com;
> Estevam Fabio-R49496; stephen@networkplumber.org
> Subject: RE: [PATCH] net: fec: add the initial to set the physical mac
> address
>
> From: Lucas Stach [mailto:l.stach@pengutronix.de]
> Data: Tuesday, August 27, 2013 10:19 PM
>
> > To: Duan Fugang-B38611
> > Cc: Li Frank-B20596; Zhou Luwei-B45643; davem@davemloft.net;
> > netdev@vger.kernel.org; shawn.guo@linaro.org;
> > bhutchings@solarflare.com; Estevam Fabio-R49496;
> > stephen@networkplumber.org
> > Subject: Re: [PATCH] net: fec: add the initial to set the physical mac
> > address
> >
> > Am Donnerstag, den 22.08.2013, 19:17 +0800 schrieb Fugang Duan:
> > > For imx6slx evk platform, the fec driver cannot work since there
> > > have no valid mac address set in physical mac registers.
> > >
> > > For imx serial platform, fec/enet IPs both need the physical MAC
> > > address initial, otherwise it cannot work.
> > >
> > > After acquiring the valid MAC address from devices tree/pfuse/
> > > kernel command line/random mac address, and then set it to the
> > > physical MAC address registers.
> > >
> > Yeah, we have also seen that we need to set this explicitly. The
> > strange thing is that I recall this used to work without calling
> > fec_set_mac_address() explicitly, so either the netdev core was
> > calling it at some point, or our userspace was. I didn't got around to
> > investigate this further.
> Netdev core don't call the function willingly, if only user config MAC
> address such as call ioctl(sockfd, SIOCSIFHWADDR, ifreq).
>
> > So anyway:
> > Reviewed-by: Lucas Stach <l.stach@pengutronix.de>
> >
> > > Signed-off-by: Fugang Duan <B38611@freescale.com>
> > > ---
> > > drivers/net/ethernet/freescale/fec_main.c | 9 ++++++---
> > > 1 files changed, 6 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/net/ethernet/freescale/fec_main.c
> > > b/drivers/net/ethernet/freescale/fec_main.c
> > > index 4349a9e..9b5e08c 100644
> > > --- a/drivers/net/ethernet/freescale/fec_main.c
> > > +++ b/drivers/net/ethernet/freescale/fec_main.c
> > > @@ -1867,10 +1867,12 @@ fec_set_mac_address(struct net_device *ndev,
> > void *p)
> > > struct fec_enet_private *fep = netdev_priv(ndev);
> > > struct sockaddr *addr = p;
> > >
> > > - if (!is_valid_ether_addr(addr->sa_data))
> > > - return -EADDRNOTAVAIL;
> > > + if (p) {
> > > + if (!is_valid_ether_addr(addr->sa_data))
> > > + return -EADDRNOTAVAIL;
> > >
> > > - memcpy(ndev->dev_addr, addr->sa_data, ndev->addr_len);
> > > + memcpy(ndev->dev_addr, addr->sa_data, ndev->addr_len);
> > > + }
> > >
> > > writel(ndev->dev_addr[3] | (ndev->dev_addr[2] << 8) |
> > > (ndev->dev_addr[1] << 16) | (ndev->dev_addr[0] << 24), @@ -
> > 1969,6
> > > +1971,7 @@ static int fec_enet_init(struct net_device *ndev)
> > >
> > > /* Get the Ethernet address */
> > > fec_get_mac(ndev);
> > > + fec_set_mac_address(ndev, NULL);
> > >
> > > /* Set receive and transmit descriptor base. */
> > > fep->rx_bd_base = cbd_base;
> >
> > --
> > Pengutronix e.K. | Lucas Stach
> |
> > Industrial Linux Solutions | http://www.pengutronix.de/
> |
> > Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-5076
> |
> > Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555
> |
> >
>
^ permalink raw reply
* Re: [PATCH 1/5] bonding: simplify and use RCU protection for 3ad xmit path
From: Ding Tianhong @ 2013-08-30 1:57 UTC (permalink / raw)
To: Nikolay Aleksandrov
Cc: Jay Vosburgh, Andy Gospodarek, David S. Miller, Veaceslav Falico,
Netdev
In-Reply-To: <521F59E2.8030707@redhat.com>
On 2013/8/29 22:25, Nikolay Aleksandrov wrote:
> On 08/28/2013 06:20 AM, Ding Tianhong wrote:
>> The commit 278b20837511776dc9d5f6ee1c7fabd5479838bb
>> (bonding: initial RCU conversion) has convert the roundrobin, active-backup,
>> broadcast and xor xmit path to rcu protection, the performance will be better
>> for these mode, so this time, convert xmit path for 3ad mode.
>>
>> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
>> Signed-off-by: Wang Yufen <wangyufen@huawei.com>
>> Cc: Nikolay Aleksandrov <nikolay@redhat.com>
>> Cc: Veaceslav Falico <vfalico@redhat.com>
>> ---
>> drivers/net/bonding/bond_3ad.c | 31 +++++++++++++------------------
>> drivers/net/bonding/bonding.h | 22 ++++++++++++++++++++++
>> 2 files changed, 35 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
>> index 9010265..52d76a4 100644
>> --- a/drivers/net/bonding/bond_3ad.c
>> +++ b/drivers/net/bonding/bond_3ad.c
>> @@ -143,7 +143,7 @@ static inline struct bonding *__get_bond_by_port(struct port *port)
>> */
>> static inline struct port *__get_first_port(struct bonding *bond)
>> {
>> - struct slave *first_slave = bond_first_slave(bond);
>> + struct slave *first_slave = bond_first_slave_rcu(bond);
>>
>> return first_slave ? &(SLAVE_AD_INFO(first_slave).port) : NULL;
>> }
>> @@ -163,7 +163,7 @@ static inline struct port *__get_next_port(struct port *port)
>> // If there's no bond for this port, or this is the last slave
>> if (bond == NULL)
>> return NULL;
>> - slave_next = bond_next_slave(bond, slave);
>> + slave_next = bond_next_slave_rcu(bond, slave);
>> if (!slave_next || bond_is_first_slave(bond, slave_next))
>> return NULL;
>>
>> @@ -2417,16 +2417,14 @@ int bond_3ad_get_active_agg_info(struct bonding *bond, struct ad_info *ad_info)
>>
>> int bond_3ad_xmit_xor(struct sk_buff *skb, struct net_device *dev)
>> {
>> - struct slave *slave, *start_at;
>> + struct slave *slave;
> Please re-arrange longest -> shortest where possible.
>
something wrong with my vim, so fix it, thanks.
>> struct bonding *bond = netdev_priv(dev);
>> int slave_agg_no;
>> int slaves_in_agg;
>> int agg_id;
>> - int i;
>> struct ad_info ad_info;
>> int res = 1;
>>
>> - read_lock(&bond->lock);
>> if (__bond_3ad_get_active_agg_info(bond, &ad_info)) {
>> pr_debug("%s: Error: __bond_3ad_get_active_agg_info failed\n",
>> dev->name);
>> @@ -2444,13 +2442,16 @@ int bond_3ad_xmit_xor(struct sk_buff *skb, struct net_device *dev)
>>
>> slave_agg_no = bond->xmit_hash_policy(skb, slaves_in_agg);
>>
>> - bond_for_each_slave(bond, slave) {
>> + bond_for_each_slave_rcu(bond, slave) {
>> struct aggregator *agg = SLAVE_AD_INFO(slave).port.aggregator;
>>
>> if (agg && (agg->aggregator_identifier == agg_id)) {
>> - slave_agg_no--;
>> - if (slave_agg_no < 0)
>> - break;
>> + if (--slave_agg_no < 0) {
>> + if (SLAVE_IS_OK(slave)) {
>> + res = bond_dev_queue_xmit(bond, skb, slave->dev);
>> + goto out;
>> + }
>> + }
>> }
>> }
>>
>> @@ -2460,23 +2461,17 @@ int bond_3ad_xmit_xor(struct sk_buff *skb, struct net_device *dev)
>> goto out;
>> }
>>
>> - start_at = slave;
>> -
>> - bond_for_each_slave_from(bond, slave, i, start_at) {
>> - int slave_agg_id = 0;
>> + bond_for_each_slave_rcu(bond, slave) {
>> struct aggregator *agg = SLAVE_AD_INFO(slave).port.aggregator;
>>
>> - if (agg)
>> - slave_agg_id = agg->aggregator_identifier;
>> -
>> - if (SLAVE_IS_OK(slave) && agg && (slave_agg_id == agg_id)) {
>> + if (SLAVE_IS_OK(slave) && agg &&
>> + (agg->aggregator_identifier == agg_id)) {
> Please drop the ( ) from (agg->aggregator_identifier == agg_id), also align it
> properly.
>
my mistake, thanks.
>> res = bond_dev_queue_xmit(bond, skb, slave->dev);
>> break;
>> }
>> }
>>
>> out:
>> - read_unlock(&bond->lock);
>> if (res) {
>> /* no suitable interface, frame not sent */
>> kfree_skb(skb);
>> diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
>> index 4bf52d5..ecb5d1d 100644
>> --- a/drivers/net/bonding/bonding.h
>> +++ b/drivers/net/bonding/bonding.h
>> @@ -74,6 +74,9 @@
>> /* slave list primitives */
>> #define bond_to_slave(ptr) list_entry(ptr, struct slave, list)
>>
>> +/* slave list primitives, Caller must hold rcu_read_lock */
>> +#define bond_to_slave_rcu(ptr) list_entry_rcu(ptr, struct slave, list)
>> +
>> /* IMPORTANT: bond_first/last_slave can return NULL in case of an empty list */
>> #define bond_first_slave(bond) \
>> list_first_entry_or_null(&(bond)->slave_list, struct slave, list)
>> @@ -81,6 +84,16 @@
>> (list_empty(&(bond)->slave_list) ? NULL : \
>> bond_to_slave((bond)->slave_list.prev))
>>
>> +/**
>> + * IMPORTANT: bond_first/last_slave_rcu can return NULL in case of an empty list
>> + * Caller must hold rcu_read_lock
>> + */
> Styling nit: the comments in net/ should be /* text...
miss the net-next. add in resend.
>
>> +#define bond_first_slave_rcu(bond) \
>> + list_first_or_null_rcu(&(bond)->slave_list, struct slave, list)
>> +#define bond_last_slave_rcu(bond) \
>> + (list_emptry(&(bond)->slave_list) ? NULL : \
> ^^^^^^^^^^^^^^^^^^^^^
> Did you compile this ?
oh, I compiled, but the func did not used, so did not show any problem, it's a big error, I will review
and check all above.
>
>> + bond_to_slave_rcu((bond)->slave_list.prev))
>> +
>> #define bond_is_first_slave(bond, pos) ((pos)->list.prev == &(bond)->slave_list)
>> #define bond_is_last_slave(bond, pos) ((pos)->list.next == &(bond)->slave_list)
>>
>> @@ -93,6 +106,15 @@
>> (bond_is_first_slave(bond, pos) ? bond_last_slave(bond) : \
>> bond_to_slave((pos)->list.prev))
>>
>> +/* Since bond_first/last_slave_rcu can return NULL, these can return NULL too */
>> +#define bond_next_slave_rcu(bond, pos) \
>> + (bond_is_last_slave(bond, pos) ? bond_first_slave_rcu(bond) : \
>> + bond_to_slave_rcu((pos)->list.next))
>> +
>> +#define bond_prev_slave_rcu(bond, pos) \
>> + (bond_is_first_slave(bond, pos) ? bond_last_slave_rcu(bond) : \
>> + bond_to_slave_rcu((pos)->list.prev))
>> +
>> /**
>> * bond_for_each_slave_from - iterate the slaves list from a starting point
>> * @bond: the bond holding this list.
>>
>
>
> .
>
^ 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