* Re: [PATCH 0/7] Series short description
From: Shmulik Ravid @ 2011-06-19 17:58 UTC (permalink / raw)
To: John Fastabend; +Cc: davem@davemloft.net, netdev@vger.kernel.org
In-Reply-To: <20110617211027.26578.11317.stgit@jf-dev1-dcblab>
On Fri, 2011-06-17 at 14:16 -0700, John Fastabend wrote:
> The following series implements improvements to the DCB netlink
> interface. This work was done to improve Application handling
> and support firmware based LLDP agents.
>
> This adds a multicast group for DCB currently user space has
> trouble keeping in sync across multiple applications and with
> firmware agents making DCB attribute changes in the driver.
>
I was working on something along the same lines and would like to add on
top of these changes a notification for the CEE flavor.
^ permalink raw reply
* Re: [PATCH 2/7] net: dcbnl, add multicast group for DCB
From: Shmulik Ravid @ 2011-06-19 18:14 UTC (permalink / raw)
To: John Fastabend; +Cc: davem@davemloft.net, netdev@vger.kernel.org
In-Reply-To: <20110617211618.26578.34100.stgit@jf-dev1-dcblab>
On Fri, 2011-06-17 at 14:16 -0700, John Fastabend wrote:
> Now that dcbnl is being used in many cases by more
> than a single agent it is beneficial to be notified
> when some entity either driver or user space has
> changed the DCB attributes.
>
> Today applications either end up polling the interface
> or relying on a user space database to maintain the DCB
> state and post events. Polling is a poor solution for
> obvious reasons. And relying on a user space database
> has its own downside. Namely it has created strange
> boot dependencies requiring the database be populated
> before any applications dependent on DCB attributes
> starts or the application goes into a polling loop.
> Populating the database requires negotiating link
> setting with the peer and can take anywhere from less
> than a second up to a few seconds depending on the switch
> implementation.
>
> Perhaps more importantly if another application or an
> embedded agent sets a DCB link attribute the database
> has no way of knowing other than polling the kernel.
> This prevents applications from responding quickly to
> changes in link events which at least in the FCoE case
> and probably any other protocols expecting a lossless
> link may result in IO errors.
>
> By adding a multicast group for DCB we have clean way
> to disseminate kernel DCB link attributes up to user
> space. Avoiding the need for user space to maintain
> a coherant database and disperse events that potentially
> do not reflect the current link state.
>
> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
> ---
>
> include/linux/rtnetlink.h | 2
> net/dcb/dcbnl.c | 228 +++++++++++++++++++++++++++++----------------
> 2 files changed, 147 insertions(+), 83 deletions(-)
>
[...]
> +static int dcbnl_notify(struct net_device *dev, int event, int cmd,
> + u32 seq, u32 pid)
[...]
A driver supporting an embedded dcbx stack would want to directly invoke
the notification routine, therefore it should be exported. I'd like also
to add support for a CEE notification. Also when the driver invokes the
notification shouldn't it use a new event type something like
RTM_NEWDCB?
^ permalink raw reply
* Re: Linux TCP's Robustness to Multipath Packet Reordering
From: Dominik Kaspar @ 2011-06-19 16:25 UTC (permalink / raw)
To: Alexander Zimmermann
Cc: netdev, Yuchung Cheng, Carsten Wolff, John Heffner, Eric Dumazet,
Lennart Schulte, Arnd Hannemann
In-Reply-To: <C040825C-480A-46C6-AF32-76D90717BA06@comsys.rwth-aachen.de>
Hi Alexander,
Ah... the receiver DSACKs the "original" packet. However, the sender
already received an ACK for its retransmission and advances SND.UNA.
When the DSACK finally arrives, it is actually outside of the SND.UNA
- SND.NXT range, which causes the DSACK to trigger "SACK reneging".
Did I get that right? :-)
Cheers,
Dominik
On Sun, Jun 19, 2011 at 5:38 PM, Alexander Zimmermann
<alexander.zimmermann@comsys.rwth-aachen.de> wrote:
> Hi,
>
> Am 19.06.2011 um 17:22 schrieb Dominik Kaspar:
>
>> Hello again,
>>
>> I have another question to Linux TCP and packet reordering. What
>> exactly happens, when a packet is so much delayed (but not causing a
>> timeout), that it gets overtaken by a retransmitted version of itself?
>> It seems to me that this results in "SACK reneging", but I don't
>> really understand why...
>
> in theory, you can detect this case with a combination of DSACK
> and timestamps. However, in practice a reordering delay greater than
> RTT will likely case an RTO (see RFC4653). IMO, if you have an packet
> reordering with an delay greater that the RTT, you have much more problems
> that SACK reneging
>
>>
>> The simplified situation goes this:
>> - Segment A gets sent and very much delayed (but not causing RTO)
>> - Segments B, C, D cause dupACKs
>> - Segment A_ret is retransmitted and ACKed (sent over new path)
>> - Some more segments E, F, ... are sent and ACKed
>> - Segment A (the delayed one) arrives at the receiver.
>> - Now what exactly happens next...?
>
> Receiver sends a DSACK
>
>>
>> I use default Linux TCP (with sack=1, dsack=1, fack=1, timestamps=1,
>> ...) and the above described series of events is cause why
>> transparently forwarding IP packets over multiple paths with RTTs of
>> 10 and 100 milliseconds.
>>
>> I'd appreciate your help - best regards,
>> Dominik
>
> //
> // Dipl.-Inform. Alexander Zimmermann
> // Department of Computer Science, Informatik 4
> // RWTH Aachen University
> // Ahornstr. 55, 52056 Aachen, Germany
> // phone: (49-241) 80-21422, fax: (49-241) 80-22222
> // email: zimmermann@cs.rwth-aachen.de
> // web: http://www.umic-mesh.net
> //
>
>
^ permalink raw reply
* [PATCH V2 00/11] net: expand time stamping, batch #2
From: Richard Cochran @ 2011-06-19 17:56 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Eric Dumazet
This patch series represents a continuation of the effort to get
better coverage of the SO_TIMESTAMPING socket API in the Ethernet
drivers. Adding time stamping support to a given driver solves two
separate issues, namely software transmit time stamping and hardware
time stamping in PHY devices.
This second batch adds the hooks into all remaining (?) arm and
powerpc MAC drivers using phylib. The first patch exports the receive
hook for use by non-NAPI drivers when compiled as modules. Patch #5
has been tested on real hardware, but the rest have only been compile
tested.
Richard Cochran (11):
net: export the receive time stamping hook for non-NAPI drivers
lib8390: enable transmit and receive time stamping.
emaclite: enable transmit and receive time stamping.
ll_temac: enable transmit and receive time stamping.
fec_mpc52xx: enable transmit and receive time stamping.
macb: enable transmit time stamping.
fs_enet: enable transmit time stamping.
smsc911x: enable transmit time stamping.
pxa168_eth: enable transmit time stamping.
mv643xx_eth: enable transmit time stamping.
ucc_geth: enable transmit time stamping.
drivers/net/fec_mpc52xx.c | 4 +++-
drivers/net/fs_enet/fs_enet-main.c | 2 ++
drivers/net/lib8390.c | 5 +++--
drivers/net/ll_temac_main.c | 5 ++++-
drivers/net/macb.c | 2 ++
drivers/net/mv643xx_eth.c | 2 ++
drivers/net/pxa168_eth.c | 3 +++
drivers/net/smsc911x.c | 1 +
drivers/net/ucc_geth.c | 2 ++
drivers/net/xilinx_emaclite.c | 9 +++++++--
net/core/timestamping.c | 1 +
11 files changed, 30 insertions(+), 6 deletions(-)
^ permalink raw reply
* [PATCH V2 01/11] net: export the receive time stamping hook for non-NAPI drivers
From: Richard Cochran @ 2011-06-19 17:56 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Eric Dumazet
In-Reply-To: <cover.1308499701.git.richard.cochran@omicron.at>
Ethernet MAC drivers based on phylib (but not using NAPI) can
enable hardware time stamping in phy devices by calling netif_rx()
conditionally based on a call to skb_defer_rx_timestamp().
This commit exports that function so that drivers calling it may
be compiled as modules.
Signed-off-by: Richard Cochran <richard.cochran@omicron.at>
---
net/core/timestamping.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/net/core/timestamping.c b/net/core/timestamping.c
index 3b00a6b..98a5264 100644
--- a/net/core/timestamping.c
+++ b/net/core/timestamping.c
@@ -122,6 +122,7 @@ bool skb_defer_rx_timestamp(struct sk_buff *skb)
return false;
}
+EXPORT_SYMBOL_GPL(skb_defer_rx_timestamp);
void __init skb_timestamping_init(void)
{
--
1.7.0.4
^ permalink raw reply related
* [PATCH V2 02/11] lib8390: enable transmit and receive time stamping.
From: Richard Cochran @ 2011-06-19 17:56 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Eric Dumazet
In-Reply-To: <cover.1308499701.git.richard.cochran@omicron.at>
This patch enables software (and phy device) time stamping. This file is
included by drivers/net/ax88796.c, which is based on phylib. So, this
patch makes hardware time stamping in the PHY possible.
Compile tested only.
Signed-off-by: Richard Cochran <richard.cochran@omicron.at>
---
drivers/net/lib8390.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/net/lib8390.c b/drivers/net/lib8390.c
index 17b75e5..70eb207 100644
--- a/drivers/net/lib8390.c
+++ b/drivers/net/lib8390.c
@@ -410,7 +410,7 @@ static netdev_tx_t __ei_start_xmit(struct sk_buff *skb,
spin_unlock(&ei_local->page_lock);
enable_irq_lockdep_irqrestore(dev->irq, &flags);
-
+ skb_tx_timestamp(skb);
dev_kfree_skb (skb);
dev->stats.tx_bytes += send_length;
@@ -758,7 +758,8 @@ static void ei_receive(struct net_device *dev)
skb_put(skb, pkt_len); /* Make room */
ei_block_input(dev, pkt_len, skb, current_offset + sizeof(rx_frame));
skb->protocol=eth_type_trans(skb,dev);
- netif_rx(skb);
+ if (!skb_defer_rx_timestamp(skb))
+ netif_rx(skb);
dev->stats.rx_packets++;
dev->stats.rx_bytes += pkt_len;
if (pkt_stat & ENRSR_PHY)
--
1.7.0.4
^ permalink raw reply related
* [PATCH V2 04/11] ll_temac: enable transmit and receive time stamping.
From: Richard Cochran @ 2011-06-19 17:56 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Eric Dumazet, Grant Likely
In-Reply-To: <cover.1308499701.git.richard.cochran@omicron.at>
This patch enables software (and phy device) time stamping. Since this MAC
is based on phylib, adding the hooks makes hardware time stamping in the
phy possible.
Compile tested only.
Cc: Grant Likely <grant.likely@secretlab.ca>
Signed-off-by: Richard Cochran <richard.cochran@omicron.at>
---
drivers/net/ll_temac_main.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ll_temac_main.c b/drivers/net/ll_temac_main.c
index b7948cc..ce9a607 100644
--- a/drivers/net/ll_temac_main.c
+++ b/drivers/net/ll_temac_main.c
@@ -727,6 +727,8 @@ static int temac_start_xmit(struct sk_buff *skb, struct net_device *ndev)
if (lp->tx_bd_tail >= TX_BD_NUM)
lp->tx_bd_tail = 0;
+ skb_tx_timestamp(skb);
+
/* Kick off the transfer */
lp->dma_out(lp, TX_TAILDESC_PTR, tail_p); /* DMA start */
@@ -772,7 +774,8 @@ static void ll_temac_recv(struct net_device *ndev)
skb->ip_summed = CHECKSUM_COMPLETE;
}
- netif_rx(skb);
+ if (!skb_defer_rx_timestamp(skb))
+ netif_rx(skb);
ndev->stats.rx_packets++;
ndev->stats.rx_bytes += length;
--
1.7.0.4
^ permalink raw reply related
* [PATCH V2 05/11] fec_mpc52xx: enable transmit and receive time stamping.
From: Richard Cochran @ 2011-06-19 17:56 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Eric Dumazet, Grant Likely
In-Reply-To: <cover.1308499701.git.richard.cochran@omicron.at>
This patch enables software (and phy device) time stamping. Software
time stamping using the SO_TIMESTAMPING API was tested and found to be
working on the LITE5200B board.
Cc: Grant Likely <grant.likely@secretlab.ca>
Signed-off-by: Richard Cochran <richard.cochran@omicron.at>
---
drivers/net/fec_mpc52xx.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/drivers/net/fec_mpc52xx.c b/drivers/net/fec_mpc52xx.c
index 9f81b1a..a4dd6a4 100644
--- a/drivers/net/fec_mpc52xx.c
+++ b/drivers/net/fec_mpc52xx.c
@@ -335,6 +335,7 @@ static int mpc52xx_fec_start_xmit(struct sk_buff *skb, struct net_device *dev)
bd->skb_pa = dma_map_single(dev->dev.parent, skb->data, skb->len,
DMA_TO_DEVICE);
+ skb_tx_timestamp(skb);
bcom_submit_next_buffer(priv->tx_dmatsk, skb);
spin_unlock_irqrestore(&priv->lock, flags);
@@ -434,7 +435,8 @@ static irqreturn_t mpc52xx_fec_rx_interrupt(int irq, void *dev_id)
length = status & BCOM_FEC_RX_BD_LEN_MASK;
skb_put(rskb, length - 4); /* length without CRC32 */
rskb->protocol = eth_type_trans(rskb, dev);
- netif_rx(rskb);
+ if (!skb_defer_rx_timestamp(skb))
+ netif_rx(rskb);
spin_lock(&priv->lock);
}
--
1.7.0.4
^ permalink raw reply related
* [PATCH V2 06/11] macb: enable transmit time stamping.
From: Richard Cochran @ 2011-06-19 17:56 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Eric Dumazet, Nicolas Ferre
In-Reply-To: <cover.1308499701.git.richard.cochran@omicron.at>
This patch enables software (and phy device) transmit time stamping
Compile tested only.
Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
Signed-off-by: Richard Cochran <richard.cochran@omicron.at>
---
drivers/net/macb.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/net/macb.c b/drivers/net/macb.c
index 6c6a028..fab63bf 100644
--- a/drivers/net/macb.c
+++ b/drivers/net/macb.c
@@ -669,6 +669,8 @@ static int macb_start_xmit(struct sk_buff *skb, struct net_device *dev)
entry = NEXT_TX(entry);
bp->tx_head = entry;
+ skb_tx_timestamp(skb);
+
macb_writel(bp, NCR, macb_readl(bp, NCR) | MACB_BIT(TSTART));
if (TX_BUFFS_AVAIL(bp) < 1)
--
1.7.0.4
^ permalink raw reply related
* [PATCH V2 07/11] fs_enet: enable transmit time stamping.
From: Richard Cochran @ 2011-06-19 17:57 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Eric Dumazet, Pantelis Antoniou, Vitaly Bordug
In-Reply-To: <cover.1308499701.git.richard.cochran@omicron.at>
This patch enables software (and phy device) transmit time stamping.
Compile tested only.
Cc: Pantelis Antoniou <pantelis.antoniou@gmail.com>
Cc: Vitaly Bordug <vbordug@ru.mvista.com>
Signed-off-by: Richard Cochran <richard.cochran@omicron.at>
---
drivers/net/fs_enet/fs_enet-main.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/net/fs_enet/fs_enet-main.c b/drivers/net/fs_enet/fs_enet-main.c
index 21abb5c..329ef23 100644
--- a/drivers/net/fs_enet/fs_enet-main.c
+++ b/drivers/net/fs_enet/fs_enet-main.c
@@ -697,6 +697,8 @@ static int fs_enet_start_xmit(struct sk_buff *skb, struct net_device *dev)
sc |= BD_ENET_TX_PAD;
CBDS_SC(bdp, sc);
+ skb_tx_timestamp(skb);
+
(*fep->ops->tx_kickstart)(dev);
spin_unlock_irqrestore(&fep->tx_lock, flags);
--
1.7.0.4
^ permalink raw reply related
* [PATCH V2 08/11] smsc911x: enable transmit time stamping.
From: Richard Cochran @ 2011-06-19 17:57 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Eric Dumazet, Steve Glendinning
In-Reply-To: <cover.1308499701.git.richard.cochran@omicron.at>
This patch enables software (and phy device) transmit time stamping.
Compile tested only.
Cc: Steve Glendinning <steve.glendinning@smsc.com>
Signed-off-by: Richard Cochran <richard.cochran@omicron.at>
---
drivers/net/smsc911x.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/drivers/net/smsc911x.c b/drivers/net/smsc911x.c
index c6d47d1..6d08b0f 100644
--- a/drivers/net/smsc911x.c
+++ b/drivers/net/smsc911x.c
@@ -1473,6 +1473,7 @@ static int smsc911x_hard_start_xmit(struct sk_buff *skb, struct net_device *dev)
pdata->ops->tx_writefifo(pdata, (unsigned int *)bufp, wrsz);
freespace -= (skb->len + 32);
+ skb_tx_timestamp(skb);
dev_kfree_skb(skb);
if (unlikely(smsc911x_tx_get_txstatcount(pdata) >= 30))
--
1.7.0.4
^ permalink raw reply related
* [PATCH V2 09/11] pxa168_eth: enable transmit time stamping.
From: Richard Cochran @ 2011-06-19 17:57 UTC (permalink / raw)
To: netdev
Cc: David Miller, Eric Dumazet, Sachin Sanap, Zhangfei Gao,
Philip Rakity
In-Reply-To: <cover.1308499701.git.richard.cochran@omicron.at>
This patch enables software (and phy device) transmit time stamping
Compile tested only.
Cc: Sachin Sanap <ssanap@marvell.com>
Cc: Zhangfei Gao <zgao6@marvell.com>
Cc: Philip Rakity <prakity@marvell.com>
Signed-off-by: Richard Cochran <richard.cochran@omicron.at>
---
drivers/net/pxa168_eth.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/drivers/net/pxa168_eth.c b/drivers/net/pxa168_eth.c
index 89f7540..e224740 100644
--- a/drivers/net/pxa168_eth.c
+++ b/drivers/net/pxa168_eth.c
@@ -1267,6 +1267,9 @@ static int pxa168_eth_start_xmit(struct sk_buff *skb, struct net_device *dev)
pep->tx_skb[tx_index] = skb;
desc->byte_cnt = length;
desc->buf_ptr = dma_map_single(NULL, skb->data, length, DMA_TO_DEVICE);
+
+ skb_tx_timestamp(skb);
+
wmb();
desc->cmd_sts = BUF_OWNED_BY_DMA | TX_GEN_CRC | TX_FIRST_DESC |
TX_ZERO_PADDING | TX_LAST_DESC | TX_EN_INT;
--
1.7.0.4
^ permalink raw reply related
* [PATCH V2 10/11] mv643xx_eth: enable transmit time stamping.
From: Richard Cochran @ 2011-06-19 17:57 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Eric Dumazet, Lennert Buytenhek
In-Reply-To: <cover.1308499701.git.richard.cochran@omicron.at>
This patch enables software (and phy device) transmit time stamping.
Compile tested only.
Cc: Lennert Buytenhek <buytenh@wantstofly.org>
Signed-off-by: Richard Cochran <richard.cochran@omicron.at>
---
drivers/net/mv643xx_eth.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/net/mv643xx_eth.c b/drivers/net/mv643xx_eth.c
index a5d9b1c..b63a074 100644
--- a/drivers/net/mv643xx_eth.c
+++ b/drivers/net/mv643xx_eth.c
@@ -847,6 +847,8 @@ no_csum:
/* clear TX_END status */
mp->work_tx_end &= ~(1 << txq->index);
+ skb_tx_timestamp(skb);
+
/* ensure all descriptors are written before poking hardware */
wmb();
txq_enable(txq);
--
1.7.0.4
^ permalink raw reply related
* [PATCH V2 11/11] ucc_geth: enable transmit time stamping.
From: Richard Cochran @ 2011-06-19 17:57 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Eric Dumazet, Shlomi Gridish, Li Yang
In-Reply-To: <cover.1308499701.git.richard.cochran@omicron.at>
This patch enables software (and phy device) transmit time stamping.
Compile tested only.
Cc: Shlomi Gridish <gridish@freescale.com>
Cc: Li Yang <leoli@freescale.com>
Signed-off-by: Richard Cochran <richard.cochran@omicron.at>
---
drivers/net/ucc_geth.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/net/ucc_geth.c b/drivers/net/ucc_geth.c
index ef04105..3127700 100644
--- a/drivers/net/ucc_geth.c
+++ b/drivers/net/ucc_geth.c
@@ -3165,6 +3165,8 @@ static int ucc_geth_start_xmit(struct sk_buff *skb, struct net_device *dev)
ugeth->txBd[txQ] = bd;
+ skb_tx_timestamp(skb);
+
if (ugeth->p_scheduler) {
ugeth->cpucount[txQ]++;
/* Indicate to QE that there are more Tx bds ready for
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCH 07/11] fs_enet: enable transmit time stamping.
From: Richard Cochran @ 2011-06-19 18:12 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, David Miller, Pantelis Antoniou, Vitaly Bordug
In-Reply-To: <1308484684.3539.76.camel@edumazet-laptop>
On Sun, Jun 19, 2011 at 01:58:04PM +0200, Eric Dumazet wrote:
>
> Well, I'll stop my review here, there is the same problem I guess in all
> your patches.
Thanks for your review. I have posted a fix for the first batch (since
they are already in next) and reposted this series.
But, considering your point, it looks like pxa168_eth and mv643xx_eth
(see patches 9 and 10 of this series) already access skb->len unsafely.
Would you care to comment on those spots, too?
Thanks,
Richard
^ permalink raw reply
* Re: [PATCH 09/11] pxa168_eth: enable transmit time stamping.
From: Richard Cochran @ 2011-06-19 18:15 UTC (permalink / raw)
To: netdev
Cc: David Miller, Sachin Sanap, Zhangfei Gao, Philip Rakity,
Eric Dumazet
In-Reply-To: <bda59e6bdd024ed1c3249b1708900d0fa0baa994.1308481492.git.richard.cochran@omicron.at>
On Sun, Jun 19, 2011 at 01:20:05PM +0200, Richard Cochran wrote:
> This patch enables software (and phy device) transmit time stamping
> Compile tested only.
>
> Cc: Sachin Sanap <ssanap@marvell.com>
> Cc: Zhangfei Gao <zgao6@marvell.com>
> Cc: Philip Rakity <prakity@marvell.com>
> Cc: Mark Brown <markb@marvell.com>
> Signed-off-by: Richard Cochran <richard.cochran@omicron.at>
> ---
> drivers/net/pxa168_eth.c | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/pxa168_eth.c b/drivers/net/pxa168_eth.c
> index 89f7540..cd2e471 100644
> --- a/drivers/net/pxa168_eth.c
> +++ b/drivers/net/pxa168_eth.c
> @@ -1273,6 +1273,7 @@ static int pxa168_eth_start_xmit(struct sk_buff *skb, struct net_device *dev)
> wmb();
> wrl(pep, SDMA_CMD, SDMA_CMD_TXDH | SDMA_CMD_ERD);
>
> + skb_tx_timestamp(skb);
> stats->tx_bytes += skb->len;
And the line above is unsafe, too.
> stats->tx_packets++;
> dev->trans_start = jiffies;
> --
> 1.7.0.4
>
^ permalink raw reply
* Re: [PATCH 10/11] mv643xx_eth: enable transmit time stamping.
From: Richard Cochran @ 2011-06-19 18:17 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Lennert Buytenhek, Eric Dumazet
In-Reply-To: <1b5632e5cd90ed390245f3d1264e42fdd760dd7e.1308481492.git.richard.cochran@omicron.at>
On Sun, Jun 19, 2011 at 01:20:06PM +0200, Richard Cochran wrote:
> This patch enables software (and phy device) transmit time stamping.
> Compile tested only.
>
> Cc: Lennert Buytenhek <buytenh@wantstofly.org>
> Signed-off-by: Richard Cochran <richard.cochran@omicron.at>
> ---
> drivers/net/mv643xx_eth.c | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/mv643xx_eth.c b/drivers/net/mv643xx_eth.c
> index a5d9b1c..c7a8f10 100644
> --- a/drivers/net/mv643xx_eth.c
> +++ b/drivers/net/mv643xx_eth.c
> @@ -884,6 +884,8 @@ static netdev_tx_t mv643xx_eth_xmit(struct sk_buff *skb, struct net_device *dev)
> if (!txq_submit_skb(txq, skb)) {
> int entries_left;
>
> + skb_tx_timestamp(skb);
> +
> txq->tx_bytes += skb->len;
And the line above is unsafe, as well.
> txq->tx_packets++;
>
> --
> 1.7.0.4
>
^ permalink raw reply
* Re: [PATCH 07/11] fs_enet: enable transmit time stamping.
From: Eric Dumazet @ 2011-06-19 18:30 UTC (permalink / raw)
To: Richard Cochran; +Cc: netdev, David Miller, Pantelis Antoniou, Vitaly Bordug
In-Reply-To: <20110619181212.GA3594@riccoc20.at.omicron.at>
Le dimanche 19 juin 2011 à 20:12 +0200, Richard Cochran a écrit :
> Thanks for your review. I have posted a fix for the first batch (since
> they are already in next) and reposted this series.
>
> But, considering your point, it looks like pxa168_eth and mv643xx_eth
> (see patches 9 and 10 of this series) already access skb->len unsafely.
>
> Would you care to comment on those spots, too?
They certainly are buggy, at a first glance.
Not only skb->len is unsafe, but netif_tx_stop_queue() calls are unsafe
too.
Not sure anyone still use these drivers...
^ permalink raw reply
* Re: [PATCH 10/11] mv643xx_eth: enable transmit time stamping.
From: Eric Dumazet @ 2011-06-19 18:33 UTC (permalink / raw)
To: Richard Cochran; +Cc: netdev, David Miller, Lennert Buytenhek
In-Reply-To: <20110619181740.GC3594@riccoc20.at.omicron.at>
Le dimanche 19 juin 2011 à 20:17 +0200, Richard Cochran a écrit :
> On Sun, Jun 19, 2011 at 01:20:06PM +0200, Richard Cochran wrote:
> > This patch enables software (and phy device) transmit time stamping.
> > Compile tested only.
> >
> > Cc: Lennert Buytenhek <buytenh@wantstofly.org>
> > Signed-off-by: Richard Cochran <richard.cochran@omicron.at>
> > ---
> > drivers/net/mv643xx_eth.c | 2 ++
> > 1 files changed, 2 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/net/mv643xx_eth.c b/drivers/net/mv643xx_eth.c
> > index a5d9b1c..c7a8f10 100644
> > --- a/drivers/net/mv643xx_eth.c
> > +++ b/drivers/net/mv643xx_eth.c
> > @@ -884,6 +884,8 @@ static netdev_tx_t mv643xx_eth_xmit(struct sk_buff *skb, struct net_device *dev)
> > if (!txq_submit_skb(txq, skb)) {
> > int entries_left;
> >
> > + skb_tx_timestamp(skb);
> > +
> > txq->tx_bytes += skb->len;
>
> And the line above is unsafe, as well.
>
Yes, for sure, please submit patches to fix this (before adding time
stamping patches), as this should go to stable.
^ permalink raw reply
* Re: [PATCH 2/3] net/fec: add device tree support
From: Grant Likely @ 2011-06-19 18:43 UTC (permalink / raw)
To: Rob Herring
Cc: Arnd Bergmann, Shawn Guo, linux-arm-kernel, Greg Ungerer, patches,
netdev, devicetree-discuss, Jason Liu, linux-kernel,
David S. Miller
In-Reply-To: <4DFE111B.8010001@gmail.com>
On Sun, Jun 19, 2011 at 9:09 AM, Rob Herring <robherring2@gmail.com> wrote:
> On 06/19/2011 07:11 AM, Arnd Bergmann wrote:
>> On Sunday 19 June 2011 13:39:32 Greg Ungerer wrote:
>>>> +#ifdef CONFIG_OF
>>>> +static const struct of_device_id fec_dt_ids[] = {
>>>> + { .compatible = "fsl,fec", .data =&fec_devtype[0], },
>>>> + {},
>>>> +};
>>>> +
>>>> +static const struct of_device_id *
>>>> +fec_get_of_device_id(struct platform_device *pdev)
>>>> +{
>>>> + return of_match_device(fec_dt_ids,&pdev->dev);
>>>> +}
>>>> +#else
>>>> +#define fec_dt_ids NULL
>>>> +static inline struct of_device_id *
>>>> +fec_get_of_device_id(struct platform_device *pdev)
>>>> +{
>>>> + return NULL;
>>>> +}
>>>> +#endif
>>>> +
>>>> static unsigned char macaddr[ETH_ALEN];
>>>> module_param_array(macaddr, byte, NULL, 0);
>>>> MODULE_PARM_DESC(macaddr, "FEC Ethernet MAC address");
>>>> @@ -1363,6 +1385,11 @@ fec_probe(struct platform_device *pdev)
>>>> struct net_device *ndev;
>>>> int i, irq, ret = 0;
>>>> struct resource *r;
>>>> + const struct of_device_id *of_id;
>>>> +
>>>> + of_id = fec_get_of_device_id(pdev);
>>>
>>> fec_get_of_device_id() is defined inside of "#ifdef CONFIG_OF". This
>>> use of it will break compilation when this is not defined.
>>>
>>
>>
>> Why? Note the #else path defining an empty function.
>>
>
> None of this is necessary in the first place. Just call of_match_device
> directly. There is already an empty function to return NULL when
> CONFIG_OF is not defined.
Heh, right you are. I should have caught on to that. :-)
g.
^ permalink raw reply
* Re: [PATCH 1/3] serial/imx: add device tree support
From: Grant Likely @ 2011-06-19 18:44 UTC (permalink / raw)
To: Rob Herring
Cc: Shawn Guo, patches, netdev, devicetree-discuss, Jason Liu,
linux-kernel, Jeremy Kerr, Sascha Hauer, linux-arm-kernel
In-Reply-To: <4DFE129E.1010800@gmail.com>
On Sun, Jun 19, 2011 at 9:15 AM, Rob Herring <robherring2@gmail.com> wrote:
> On 06/19/2011 10:05 AM, Grant Likely wrote:
>> On Sun, Jun 19, 2011 at 1:30 AM, Shawn Guo <shawn.guo@freescale.com> wrote:
>>> On Sat, Jun 18, 2011 at 10:19:34AM -0600, Grant Likely wrote:
>>>> On Sat, Jun 18, 2011 at 11:19:12PM +0800, Shawn Guo wrote:
>>>>> It adds device tree data parsing support for imx tty/serial driver.
>>>>>
>>>>> Signed-off-by: Jeremy Kerr <jeremy.kerr@canonical.com>
>>>>> Signed-off-by: Jason Liu <jason.hui@linaro.org>
>>>>> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>
>>>>> Cc: Sascha Hauer <s.hauer@pengutronix.de>
>>>>> ---
>>>>> .../bindings/tty/serial/fsl-imx-uart.txt | 21 +++++
>>>>> drivers/tty/serial/imx.c | 81 +++++++++++++++++---
>>>>> 2 files changed, 92 insertions(+), 10 deletions(-)
>>>>> create mode 100644 Documentation/devicetree/bindings/tty/serial/fsl-imx-uart.txt
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/tty/serial/fsl-imx-uart.txt b/Documentation/devicetree/bindings/tty/serial/fsl-imx-uart.txt
>>>>> new file mode 100644
>>>>> index 0000000..7648e17
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/tty/serial/fsl-imx-uart.txt
>>>>> @@ -0,0 +1,21 @@
>>>>> +* Freescale i.MX Universal Asynchronous Receiver/Transmitter (UART)
>>>>> +
>>>>> +Required properties:
>>>>> +- compatible : should be "fsl,<soc>-uart", "fsl,imx-uart"
>>>>
>>>> I'd make this "fsl,<soc>-uart", "fsl,imx51-uart"
>>>>
>>>> It's better to anchor these things on real silicon, or a real ip block
>>>> specification rather than something pseudo-generic. Subsequent chips,
>>>> like the imx53, should simply claim compatibility with the older
>>>> fsl,imx51-uart.
>>>
>>> It is a real IP block on all imx silicons (except imx23 and imx28
>>> which are known as former stmp).
>>>
>>>>
>>>> (in essence, "fsl,imx51-uart" becomes the generic string without the
>>>> downside of having no obvious recourse when new silicon shows up that
>>>> is an imx part, but isn't compatible with the imx51 uart.
>>>>
>>> In this case, should imx1 the ancestor of imx family than imx51
>>> becomes part of that generic string? Claiming uart of imx1, imx21
>>> and imx31 (senior than imx51) compatible with the imx51 uart seems
>>> odd to me.
>>>
>>> That said, IMO, "fsl,imx-uart" stands a real IP block specification
>>> here and can be a perfect generic compatibility string to tell the
>>> recourse of any imx silicon using this IP.
>>
>> Yes, but which /version/ of the IP block? Hardware designers are
>> notorious for changing hardware designs for newer silicon, sometimes
>> to add features, sometimes to fix bugs. While I understand the
>> temptation to boil a compatible value down to a nice clean generic
>> string, doing so only works in a perfect world. In the real world,
>> you still need to have some information about the specific
>> implementation. I prefer this specifying it to the SoC name, but I've
>> been known to be convinced that specifying it to the ip-block name &
>> version in certain circumstances, like for IP blocks in an FPGA, or
>> some of the Freescale powerpc pXXXX SoCs which actually had an IP
>> block swapped out midway through the life of the chip.
>>
>
> There are definitely uart changes along the way with each generation.
>
>> Besides, encoding an soc or ip block version into the 'generic'
>> compatible values is not just good practice, it has *zero downside*.
>> That's the beauty of the compatible property semantics. Any node can
>> claim compatibility with an existing device. If no existing device
>> fits correctly, then the node simple does not claim compatibility.
>> Drivers can bind to any number of compatible strings, so it would be
>> just fine for the of_match_table list to include both "fsl,imx-21" and
>> "fsl,imx-51" (assuming that is the appropriate solution in this case).
>>
>
> Don't you need uart or serial in here somewhere.
you are of course correct. The examples should be "fsl,imx21-uart" &
"fsl,imx51-uart". I was just writing too quickly.
g.
^ permalink raw reply
* [PATCH V2 03/11] emaclite: enable transmit and receive time stamping.
From: Richard Cochran @ 2011-06-19 17:56 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Eric Dumazet, John Linn
In-Reply-To: <cover.1308499701.git.richard.cochran@omicron.at>
This patch enables software (and phy device) time stamping. Since this
MAC uses phylib, adding the hooks make hardware time stamping in the phy
possible.
Compile tested only.
Cc: John Linn <john.linn@xilinx.com>
Signed-off-by: Richard Cochran <richard.cochran@omicron.at>
---
drivers/net/xilinx_emaclite.c | 9 +++++++--
1 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/net/xilinx_emaclite.c b/drivers/net/xilinx_emaclite.c
index 372572c..8cae4f6 100644
--- a/drivers/net/xilinx_emaclite.c
+++ b/drivers/net/xilinx_emaclite.c
@@ -647,7 +647,8 @@ static void xemaclite_rx_handler(struct net_device *dev)
dev->stats.rx_packets++;
dev->stats.rx_bytes += len;
- netif_rx(skb); /* Send the packet upstream */
+ if (!skb_defer_rx_timestamp(skb))
+ netif_rx(skb); /* Send the packet upstream */
}
/**
@@ -1029,15 +1030,19 @@ static int xemaclite_send(struct sk_buff *orig_skb, struct net_device *dev)
spin_lock_irqsave(&lp->reset_lock, flags);
if (xemaclite_send_data(lp, (u8 *) new_skb->data, len) != 0) {
/* If the Emaclite Tx buffer is busy, stop the Tx queue and
- * defer the skb for transmission at a later point when the
+ * defer the skb for transmission during the ISR, after the
* current transmission is complete */
netif_stop_queue(dev);
lp->deferred_skb = new_skb;
+ /* Take the time stamp now, since we can't do this in an ISR. */
+ skb_tx_timestamp(new_skb);
spin_unlock_irqrestore(&lp->reset_lock, flags);
return 0;
}
spin_unlock_irqrestore(&lp->reset_lock, flags);
+ skb_tx_timestamp(new_skb);
+
dev->stats.tx_bytes += len;
dev_kfree_skb(new_skb);
--
1.7.0.4
^ permalink raw reply related
* Re: [PATCH 10/11] mv643xx_eth: enable transmit time stamping.
From: David Miller @ 2011-06-19 19:05 UTC (permalink / raw)
To: eric.dumazet; +Cc: richardcochran, netdev, buytenh
In-Reply-To: <1308508398.3539.80.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Sun, 19 Jun 2011 20:33:18 +0200
> Yes, for sure, please submit patches to fix this (before adding time
> stamping patches), as this should go to stable.
Agreed.
^ permalink raw reply
* Re: [PATCH v2 1/5] net: Support ethtool ops for rx of errored frames.
From: Ben Greear @ 2011-06-19 20:11 UTC (permalink / raw)
To: Francois Romieu; +Cc: netdev
In-Reply-To: <20110618213400.GA2741@electric-eye.fr.zoreil.com>
On 06/18/2011 02:34 PM, Francois Romieu wrote:
> greearb@candelatech.com<greearb@candelatech.com> :
> [...]
>> This can be useful when sniffing dodgy networks.
>
> Do you plan to add something similar - i.e. not per packet - for the Tx path ?
Ability to tx errored frames? I posted a patch to enable sending
frames with custom (ie, invalid) Ethernet FCS, but you have to
enable it per-socket, and it will only work with AF_PACKET sockets.
Can you offer more details on what you are asking for?
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: Ethernet low-level frame debugging support
From: Ben Greear @ 2011-06-19 20:19 UTC (permalink / raw)
To: Mark Smith; +Cc: netdev
In-Reply-To: <20110619104431.23a22fe9@opy.nosense.org>
On 06/18/2011 06:14 PM, Mark Smith wrote:
> Hi,
>
> Firstly, I think this is a potentially quite useful feature for
> networking people and that I hope it makes it into the kernel proper.
>
> One thing I've thought is that perhaps it might be made and named a bit
> more generally, as NICs will also drop frames for other reasons other
> than FCs failures e.g. runt frames. So perhaps something like "true
> promiscuous" or "full promiscuous" might be a more general name, and if
> it is enabled, then all NIC error checking that can be switched off is
> switched off. Looking at the chipset data sheets for a few NICs that I
> have / have had (netgear FA312 (natsemi ns83815), smc epic100, ne2000),
> they all seem to have registers which allow switching off many if not
> all of the NIC error checking settings.
I called it 'save-rxerr' in ethtool...I think that is general enough?
The early patch that saves the FCS just passes the 4-byte FCS up the stack.
It doesn't change the ability to receive bad frames or not..that is in the
later patches.
>
> The other thing I've thought could be useful would be to be able to
> send runts by not padding the frames when they're less then 64 bytes.
> I've been able to test if this is possible with the netgear FA312, as
> the chipset does the padding. I connected it back to back with an
> e1000e I have, switched off the chipset automatic padding on the FA312,
> sent small traffic, and then saw that the e1000e's internal
> rx_short_length_errors counter correspondingly increased. Of course I
> can't see them with tcpdump on the e1000e because it is dropping them.
Maybe the SO_NOFCS option could change to SO_DRVOPTS and take a bit-field
instead of just be on/off. NOFCS could be one flag, NOPAD another, etc.
That would give ability to send non-padded frames if the driver has
support.
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ 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