Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next-2.6 1/2] net: Allow changing number of RX queues after device allocation
From: Ben Hutchings @ 2010-09-23 20:19 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, linux-net-drivers, Tom Herbert
In-Reply-To: <1285273118.7794.23.camel@achroite.uk.solarflarecom.com>

For RPS, we create a kobject for each RX queue based on the number of
queues passed to alloc_netdev_mq().  However, drivers generally do not
determine the numbers of hardware queues to use until much later, so
this usually represents the maximum number the driver may use and not
the actual number in use.

For TX queues, drivers can update the actual number using
netif_set_real_num_tx_queues().  Add a corresponding function for RX
queues, netif_set_real_num_rx_queues().

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
 include/linux/netdevice.h |   14 ++++++++++++++
 net/core/dev.c            |   43 +++++++++++++++++++++++++++++++++++++++----
 net/core/net-sysfs.c      |   34 +++++++++++++++++++---------------
 net/core/net-sysfs.h      |    4 ++++
 4 files changed, 76 insertions(+), 19 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index f7f1302..c2b3880 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -978,6 +978,9 @@ struct net_device {
 
 	/* Number of RX queues allocated at alloc_netdev_mq() time  */
 	unsigned int		num_rx_queues;
+
+	/* Number of RX queues currently active in device  */
+	unsigned int		real_num_rx_queues;
 #endif
 
 	rx_handler_func_t	*rx_handler;
@@ -1682,6 +1685,17 @@ static inline int netif_is_multiqueue(const struct net_device *dev)
 extern void netif_set_real_num_tx_queues(struct net_device *dev,
 					 unsigned int txq);
 
+#ifdef CONFIG_RPS
+extern int netif_set_real_num_rx_queues(struct net_device *dev,
+					unsigned int rxq);
+#else
+static inline int netif_set_real_num_rx_queues(struct net_device *dev,
+						unsigned int rxq)
+{
+	return 0;
+}
+#endif
+
 /* Use this variant when it is known for sure that it
  * is executing from hardware interrupt context or with hardware interrupts
  * disabled.
diff --git a/net/core/dev.c b/net/core/dev.c
index 2c7934f..1287ce1 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1567,6 +1567,38 @@ void netif_set_real_num_tx_queues(struct net_device *dev, unsigned int txq)
 }
 EXPORT_SYMBOL(netif_set_real_num_tx_queues);
 
+#ifdef CONFIG_RPS
+/**
+ *	netif_set_real_num_rx_queues - set actual number of RX queues used
+ *	@dev: Network device
+ *	@rxq: Actual number of RX queues.  Must not be greater than the
+ *		queue count specified at device allocation time.
+ *
+ * This must be called either with the rtnl_lock held or before registration
+ * of the net device.  Returns 0 on success, or a negative error code.
+ * If called before registration, it always succeeds.
+ */
+int netif_set_real_num_rx_queues(struct net_device *dev, unsigned int rxq)
+{
+	int rc;
+
+	if (rxq > dev->num_rx_queues)
+		return -EINVAL;
+
+	if (dev->reg_state == NETREG_REGISTERED) {
+		ASSERT_RTNL();
+		rc = net_rx_queue_update_kobjects(dev, dev->real_num_rx_queues,
+						  rxq);
+		if (rc)
+			return rc;
+	}
+
+	dev->real_num_rx_queues = rxq;
+	return 0;
+}
+EXPORT_SYMBOL(netif_set_real_num_rx_queues);
+#endif
+
 static inline void __netif_reschedule(struct Qdisc *q)
 {
 	struct softnet_data *sd;
@@ -2352,10 +2384,11 @@ static int get_rps_cpu(struct net_device *dev, struct sk_buff *skb,
 
 	if (skb_rx_queue_recorded(skb)) {
 		u16 index = skb_get_rx_queue(skb);
-		if (unlikely(index >= dev->num_rx_queues)) {
-			WARN_ONCE(dev->num_rx_queues > 1, "%s received packet "
-				"on queue %u, but number of RX queues is %u\n",
-				dev->name, index, dev->num_rx_queues);
+		if (unlikely(index >= dev->real_num_rx_queues)) {
+			WARN_ONCE(dev->real_num_rx_queues > 1,
+				  "%s received packet on queue %u, but number "
+				  "of RX queues is %u\n",
+				  dev->name, index, dev->real_num_rx_queues);
 			goto done;
 		}
 		rxqueue = dev->_rx + index;
@@ -5017,6 +5050,7 @@ int register_netdevice(struct net_device *dev)
 		dev->_rx->first = dev->_rx;
 		atomic_set(&dev->_rx->count, 1);
 		dev->num_rx_queues = 1;
+		dev->real_num_rx_queues = 1;
 	}
 #endif
 	/* Init, if this function is available */
@@ -5479,6 +5513,7 @@ struct net_device *alloc_netdev_mq(int sizeof_priv, const char *name,
 #ifdef CONFIG_RPS
 	dev->_rx = rx;
 	dev->num_rx_queues = queue_count;
+	dev->real_num_rx_queues = queue_count;
 #endif
 
 	dev->gso_max_size = GSO_MAX_SIZE;
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index 76485a3..4791cfc 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -742,34 +742,38 @@ static int rx_queue_add_kobject(struct net_device *net, int index)
 	return error;
 }
 
-static int rx_queue_register_kobjects(struct net_device *net)
+int
+net_rx_queue_update_kobjects(struct net_device *net, int old_num, int new_num)
 {
 	int i;
 	int error = 0;
-
-	net->queues_kset = kset_create_and_add("queues",
-	    NULL, &net->dev.kobj);
-	if (!net->queues_kset)
-		return -ENOMEM;
-	for (i = 0; i < net->num_rx_queues; i++) {
+	
+	for (i = old_num; i < new_num; i++) {
 		error = rx_queue_add_kobject(net, i);
-		if (error)
+		if (error) {
+			new_num = old_num;
 			break;
+		}
 	}
 
-	if (error)
-		while (--i >= 0)
-			kobject_put(&net->_rx[i].kobj);
+	while (--i >= new_num)
+		kobject_put(&net->_rx[i].kobj);
 
 	return error;
 }
 
-static void rx_queue_remove_kobjects(struct net_device *net)
+static int rx_queue_register_kobjects(struct net_device *net)
 {
-	int i;
+	net->queues_kset = kset_create_and_add("queues",
+	    NULL, &net->dev.kobj);
+	if (!net->queues_kset)
+		return -ENOMEM;
+	return net_rx_queue_update_kobjects(net, 0, net->real_num_rx_queues);
+}
 
-	for (i = 0; i < net->num_rx_queues; i++)
-		kobject_put(&net->_rx[i].kobj);
+static void rx_queue_remove_kobjects(struct net_device *net)
+{
+	net_rx_queue_update_kobjects(net, net->real_num_rx_queues, 0);
 	kset_unregister(net->queues_kset);
 }
 #endif /* CONFIG_RPS */
diff --git a/net/core/net-sysfs.h b/net/core/net-sysfs.h
index 805555e..778e157 100644
--- a/net/core/net-sysfs.h
+++ b/net/core/net-sysfs.h
@@ -4,4 +4,8 @@
 int netdev_kobject_init(void);
 int netdev_register_kobject(struct net_device *);
 void netdev_unregister_kobject(struct net_device *);
+#ifdef CONFIG_RPS
+int net_rx_queue_update_kobjects(struct net_device *, int old_num, int new_num);
+#endif
+
 #endif
-- 
1.7.2.1



-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
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 related

* [PATCH net-next-2.6 2/2] sfc: Use proper functions to set core RX and TX queue counts
From: Ben Hutchings @ 2010-09-23 20:20 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, linux-net-drivers
In-Reply-To: <1285273118.7794.23.camel@achroite.uk.solarflarecom.com>

Call netif_real_num_tx_queues() instead of setting
net_device::real_num_tx_queues directly.

Also call the new netif_real_num_rx_queues() function to set the
core RX queue count correctly for RPS.

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
---
 drivers/net/sfc/efx.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/net/sfc/efx.c b/drivers/net/sfc/efx.c
index 5be71f4..fa6e020 100644
--- a/drivers/net/sfc/efx.c
+++ b/drivers/net/sfc/efx.c
@@ -1315,7 +1315,8 @@ static int efx_probe_nic(struct efx_nic *efx)
 		efx->rx_indir_table[i] = i % efx->n_rx_channels;
 
 	efx_set_channels(efx);
-	efx->net_dev->real_num_tx_queues = efx->n_tx_channels;
+	netif_set_real_num_tx_queues(efx->net_dev, efx->n_tx_channels);
+	netif_set_real_num_rx_queues(efx->net_dev, efx->n_rx_channels);
 
 	/* Initialise the interrupt moderation settings */
 	efx_init_irq_moderation(efx, tx_irq_mod_usec, rx_irq_mod_usec, true);
-- 
1.7.2.1


-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
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 related

* Re: [PATCH v6 0/8] ptp: IEEE 1588 hardware clock support
From: john stultz @ 2010-09-23 20:28 UTC (permalink / raw)
  To: Christoph Lameter
  Cc: Richard Cochran, linux-kernel, devicetree-discuss, linux-api,
	linux-arm-kernel, linuxppc-dev, netdev, Arnd Bergmann,
	David Miller, Krzysztof Halasa, Peter Zijlstra, Rodolfo Giometti,
	Thomas Gleixner
In-Reply-To: <alpine.DEB.2.00.1009231402420.2962@router.home>

On Thu, 2010-09-23 at 14:15 -0500, Christoph Lameter wrote:
> On Thu, 23 Sep 2010, john stultz wrote:
> 
> > This was my initial gut reaction as well, but in the end, I agree with
> > Richard that in the case of one or multiple PTP hardware clocks, we
> > really can't abstract over the different time domains.
> 
> My (arguably still superficial) review of the source does not show
> anything that would make me reach that conclusion.
> 
> > I really don't think the PTP clock can be used as a clocksource sanely.
> >
> > First, the hardware access is much to slow for system timekeeping.
> 
> The HPET or pit timesource are also quite slow these days. You only need
> access periodically to essentially tune the TSC ratio.

If we're using the TSC, then we're not using the PTP clock as you
suggest. Further the HPET and PIT aren't used to steer the system time
when we are using the TSC as a clocksource. Its only used to calibrate
the initial constant freq used by the timekeeping code (and if its
non-constant, we throw it out).

> > Second, there is the problem that the system time is a software clock,
> > and adjustments made (like freq) are made in the layer that interprets
> > the underlying hardware cycle counter. Adjustments made in PTP (in order
> > to sync the network timestamps) are made at the hardware level.
> 
> From what I can see the PTP clocks are periodic hardware cycle counters
> like any other clock that we currently support. If its configurable enough
> then setup a hardware cycle counter that mimics nanoseconds since the
> epoch as closely as possible and use that to sync the TSC rate to. Makes
> it very easy.

I guess I'm confused by what you're suggesting.
If we're using the TSC, then that's the clocksource timekeeping uses.
The original issue seemed to be around the suggestion of using the PTP
clock as a clocksource, which I don't think is really feasible.

Again, that's because
1) The PTP access latency is slow (so is the PIT, true enough, but no
one should be using the PIT as a clocksource unless they really have no
better hardware - its really only useful for 486s and old freq scaling
laptops that have no other stable clocksource).

2) The way PTP clocks are steered to sync with network time causes their
hardware freq to actually change. Since these adjustments are done on
the hardware clock level, and not on the system time level, the
adjustments to sync the system time/freq would then be made incorrect by
PTP hardware adjustments. 

3) Further, the PTP hardware counter can be simply set to a new offset
to put it in line with the network time. This could cause trouble with
timekeeping much like unsynced TSCs do.


Now, what you seem to be suggesting is to use the TSC (or whatever
clocksource the system time is using) but to steer the system time using
the PTP clock. This is actually what is being proposed, however, the
steering is done in userland. This is due to the fact that there are two
components to the steering, 1) adjusting the PTP clock hardware to
network time and 2) adjusting the system time to the PTP hardware. By
exposing the PTP clock to userland via the posix clocks interface, we
allow this to easily be done.


> > This would cause a disconnect between the hardware freq understood by
> > the system time management code and the actual hardware freq.
> 
> We can switch underlying clocks for system time already. We can adapt to a
> different hw frequency.

Actually no. The timekeeping code requires a fixed freq counter. Dealing
with hardware freq changes is difficult, because error is introduced by
the latency between when the freq changes and when the timekeeping code
is notified of it. So the system treats the hardware counters as fixed
freq. Now, hardware does vary freq ever so slightly as thermal
conditions change, but this is addressed in userland and corrected via
adjtimex.

>  But then I do not know why adjust the freq? I
> thought the point was that the periodic clock was network synchronized and
> can be used as "the" master clock for multiple machines?

Not parsing that. What do you mean by periodic clock?

> > Richard, I'd actually strike this paragraph from the rational, as I feel
> > it has the tendency to confuse as it suggests having the PHC as a
> > clocksource is feasible when really it isn't. Or alternatively, maybe
> > express more clearly why its not feasible, so it doesn't just seem like
> > a minor design choice.
> 
> Sorry but I still feel that this is pretty much a misguided approach that
> creates unnecessary layers in the kernel.

Unnecessary layers? Where? This approach has less in-kernel layers, as
it exposes the PTP clock to userland, instead of trying to layer things
on top of it and stretching the system time abstraction to cover it.

>  The trivial easy approach was
> not done (copy a driver from drivers/clocksource, modify so that it
> programs access to a centralized periodic ptp signal and uses it for
> system sync).

I disagree.

I've argued through the approach trying to keep it all internal to the
kernel, but to do so would be anything but trivial. Further, there's the
case of master-clocks, where the PTP hardware must be synced to system
time, instead of the other way around. And then there's the case of
boundary-clocks, which may have multiple PTP hardware clocks that have
to be synced.

I think exposing this through the posix clock interface is really the
best approach. Its not a static clockid, so its not something most apps
will ever have to deal with, but it allows the few apps that really need
to have access to the PTP clock hardware can do so in a clean way.


And credits to Richard for having to slowly explain this to me (and
others) many times over, before I got it.

thanks
-john


^ permalink raw reply

* Re: [PATCH 6/8] ptp: Added a clock that uses the eTSEC found on the MPC85xx.
From: Christoph Lameter @ 2010-09-23 20:32 UTC (permalink / raw)
  To: Alan Cox
  Cc: Richard Cochran, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ,
	netdev-u79uwXL29TY76Z2rM5mHXA, Arnd Bergmann, David Miller,
	John Stultz, Krzysztof Halasa, Peter Zijlstra, Rodolfo Giometti,
	Thomas Gleixner
In-Reply-To: <20100923214359.3f287b11-qBU/x9rampVanCEyBjwyrvXRex20P6io@public.gmane.org>

On Thu, 23 Sep 2010, Alan Cox wrote:

> > Please do not introduce useless additional layers for clock sync. Load
> > these ptp clocks like the other regular clock modules and make them sync
> > system time like any other clock.
>
> I don't think you understand PTP. PTP has masters, a system can need to
> be honouring multiple conflicting masters at once.

The upshot of it all has to be some synchronized notion of time regardless
of how many other things are going on under the hood. And the spec here
suggests a hardware able to generate periodic accurate events that can be
used to sync system time.

> > Really guys: I want a PTP solution! Now! And not some idiotic additional
> > kernel layers that just pass bits around because its so much fun and
> > screws up clock accurary in due to the latency noise introduced while
> > having so much fun with the bits.
>
> There are some interesting complications in putting a PTP sync
> interface in kernel.

If the PTP logic internally has to juggle multiple clocks then that is a
complication for the driver ok. In any case the driver ultimately has to
provide *one* source of time for the system to sync to.

^ permalink raw reply

* Re: [PATCH v6 0/8] ptp: IEEE 1588 hardware clock support
From: Alan Cox @ 2010-09-23 20:36 UTC (permalink / raw)
  To: Richard Cochran
  Cc: John Stultz, Rodolfo Giometti, Arnd Bergmann, Peter Zijlstra,
	linux-api, devicetree-discuss, linux-kernel, Thomas Gleixner,
	netdev, Christoph Lameter, linuxppc-dev, David Miller,
	linux-arm-kernel, Krzysztof Halasa
In-Reply-To: <cover.1285261533.git.richard.cochran@omicron.at>

>    So as far as the POSIX standard is concerned, offering a clock id
>    to represent the PHC would be acceptable.

But completely useless as you may have more than one entirely different
time managed by PTP and in which you are not master but must work with
the timebases provided.


>     /sys/class/timesource/<name>/id
>     /sys/class/ptp/ptp_clock_X/id
> 
>     Note: I am not too sure that this is exactly what people imagined,
>           but it is my best understanding so far. I gleaned two
>           different ideas about where to offer the clock id. In order
>           to keep just one way, I will be happy to remove the less
>           popular one.

I see no fix proposed for the race condition I pointed out. This doesn't
work.


>    If the Linux system time is synchronized to the PHC via the PPS

To which PHC we can have several

>    + Intel IXP465
>      - Auxiliary Slave/Master Mode Snapshot (optional interrupt)
>      - Target Time (optional interrupt)

And about 40 already supported by char driver interface clocks and rtcs
in the kernel...


I'd say the inability to have multiple clocks and the race condition
because of the clockid stuff leaves the proposal dead in the water.

It also ignores the existing APIs we have floating around attached to
devices.

You need to make one small important change. You need to take the POSIX
crap about enumerating things out and shoot it, bury it at a crossroads
and sprinkle holy water on it.

Drop the clockid_t and swap it for a file handle like a proper Unix or
Linux interface. The rest is much the same

	fd = open /sys/class/timesource/[whatever]

	various queries you may want to do to check the name etc

	fclock_adjtime(fd, ...)
	

The posix interface is fundamentally flawed. It only works for staticly
enumerable objects. Unix avoided that forty years ago by making the
identifier a handle which immediately cures all your object lifetime
problems in one swoop.

Namespace -> file handle translations are dynamic, but once you have it
open you hold on to the same object, which means you can check what you
have.



Alan

^ permalink raw reply

* Re: [PATCH 6/8] ptp: Added a clock that uses the eTSEC found on the MPC85xx.
From: Alan Cox @ 2010-09-23 20:43 UTC (permalink / raw)
  To: Christoph Lameter
  Cc: Richard Cochran, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ,
	netdev-u79uwXL29TY76Z2rM5mHXA, Arnd Bergmann, David Miller,
	John Stultz, Krzysztof Halasa, Peter Zijlstra, Rodolfo Giometti,
	Thomas Gleixner
In-Reply-To: <alpine.DEB.2.00.1009231348150.2962-sBS69tsa9Uj/9pzu0YdTqQ@public.gmane.org>

> Please do not introduce useless additional layers for clock sync. Load
> these ptp clocks like the other regular clock modules and make them sync
> system time like any other clock.

I don't think you understand PTP. PTP has masters, a system can need to
be honouring multiple conflicting masters at once.

> Really guys: I want a PTP solution! Now! And not some idiotic additional
> kernel layers that just pass bits around because its so much fun and
> screws up clock accurary in due to the latency noise introduced while
> having so much fun with the bits.

There are some interesting complications in putting a PTP sync
interface in kernel.

^ permalink raw reply

* Re: [PATCH v6 0/8] ptp: IEEE 1588 hardware clock support
From: john stultz @ 2010-09-23 20:49 UTC (permalink / raw)
  To: Alan Cox
  Cc: Rodolfo Giometti, Arnd Bergmann, Peter Zijlstra, linux-api,
	devicetree-discuss, linux-kernel, David Miller, Thomas Gleixner,
	netdev, Christoph Lameter, linuxppc-dev, Richard Cochran,
	linux-arm-kernel, Krzysztof Halasa
In-Reply-To: <20100923213654.0c64b047@lxorguk.ukuu.org.uk>

On Thu, 2010-09-23 at 21:36 +0100, Alan Cox wrote:
> >    So as far as the POSIX standard is concerned, offering a clock id
> >    to represent the PHC would be acceptable.
> 
> But completely useless as you may have more than one entirely different
> time managed by PTP and in which you are not master but must work with
> the timebases provided.

I don't see how this is a problem, as it exposes the multiple hardware
clocks via different posix clock ids. So in the boundary clock case, you
can configure which side is the client and which side is the master in a
config file and the PTPd will appropriately steer them individually.


> 
> >     /sys/class/timesource/<name>/id
> >     /sys/class/ptp/ptp_clock_X/id
> > 
> >     Note: I am not too sure that this is exactly what people imagined,
> >           but it is my best understanding so far. I gleaned two
> >           different ideas about where to offer the clock id. In order
> >           to keep just one way, I will be happy to remove the less
> >           popular one.
> 
> I see no fix proposed for the race condition I pointed out. This doesn't
> work.

So, if I recall this was: "How do you keep the module from unloading
while its being used?" 

There may need to be proper locking for unregistering the posix clock_id
on module unload, but I don't think we need a use-count to prevent the
module from being unloaded.

My question would be: How do we handle a USB network device ($14.99 now
with PTP!) being unplugged? We can't say "Sorry! That's in use!". So we
note the hardware is gone, and return the proper error code.

Or am I missing something else?


> >    If the Linux system time is synchronized to the PHC via the PPS
> 
> To which PHC we can have several
> 
> >    + Intel IXP465
> >      - Auxiliary Slave/Master Mode Snapshot (optional interrupt)
> >      - Target Time (optional interrupt)
> 
> And about 40 already supported by char driver interface clocks and rtcs
> in the kernel...

And those char driver interfaces are all subtly different.

I actually recently submitted an RFC to expose the RTC devices via the
posix clock/timer interface, because working with the RTC hardware
device directly is terrible for managing alarm interrupts. 

For instance, you easily run into the case where your TV recording
application programs an alarm to record your favorite show at 8pm. Then
your backup script programs an alarm to wake up at 2am to do your
nightly backups. Your box suspends and the next morning, you're missing
your favorite show!


> I'd say the inability to have multiple clocks and the race condition
> because of the clockid stuff leaves the proposal dead in the water.
> 
> It also ignores the existing APIs we have floating around attached to
> devices.
> 
> You need to make one small important change. You need to take the POSIX
> crap about enumerating things out and shoot it, bury it at a crossroads
> and sprinkle holy water on it.

We agree the list-by-name stuff isn't the way to go. :)


> Drop the clockid_t and swap it for a file handle like a proper Unix or
> Linux interface. The rest is much the same
> 
> 	fd = open /sys/class/timesource/[whatever]
> 
> 	various queries you may want to do to check the name etc
> 
> 	fclock_adjtime(fd, ...)
> 	
> 
> The posix interface is fundamentally flawed. It only works for staticly
> enumerable objects. Unix avoided that forty years ago by making the
> identifier a handle which immediately cures all your object lifetime
> problems in one swoop.

So, I don't really see how that's so different from what is being
proposed. The clock_id is dynamically assigned per registered clock, and
exposed via the sysfs interface from ptp hardware entry.

The only difference is the open/close reference counting, which I don't
think is necessary here (since we can't always keep the hardware from
going away).

thanks
-john

^ permalink raw reply

* Re: [PATCH v6 0/8] ptp: IEEE 1588 hardware clock support
From: Christoph Lameter @ 2010-09-23 20:49 UTC (permalink / raw)
  To: john stultz
  Cc: Rodolfo Giometti, Arnd Bergmann, Peter Zijlstra, linux-api,
	devicetree-discuss, linux-kernel, David Miller, netdev,
	Thomas Gleixner, linuxppc-dev, Richard Cochran, linux-arm-kernel,
	Krzysztof Halasa
In-Reply-To: <1285273684.2587.92.camel@localhost.localdomain>

On Thu, 23 Sep 2010, john stultz wrote:

> > The HPET or pit timesource are also quite slow these days. You only need
> > access periodically to essentially tune the TSC ratio.
>
> If we're using the TSC, then we're not using the PTP clock as you
> suggest. Further the HPET and PIT aren't used to steer the system time
> when we are using the TSC as a clocksource. Its only used to calibrate
> the initial constant freq used by the timekeeping code (and if its
> non-constant, we throw it out).

There is no other scalable time source available for fast timer access
than the time stamp counter in the cpu. Other time source require
memory accesses which is inherently slower.

An accurate other time source is used to adjust this clock. NTP does that
via the clock interfaces from user space which has its problems with
accuracy. PTP can provide the network synced time access
that would a more accurate calibration of the time.

> 2) The way PTP clocks are steered to sync with network time causes their
> hardware freq to actually change. Since these adjustments are done on
> the hardware clock level, and not on the system time level, the
> adjustments to sync the system time/freq would then be made incorrect by
> PTP hardware adjustments.

Right. So use these as a way to fine tune the TSC clock (and thereby the
system time).

> 3) Further, the PTP hardware counter can be simply set to a new offset
> to put it in line with the network time. This could cause trouble with
> timekeeping much like unsynced TSCs do.

You can do the same for system time.

> Now, what you seem to be suggesting is to use the TSC (or whatever
> clocksource the system time is using) but to steer the system time using
> the PTP clock. This is actually what is being proposed, however, the
> steering is done in userland. This is due to the fact that there are two
> components to the steering, 1) adjusting the PTP clock hardware to
> network time and 2) adjusting the system time to the PTP hardware. By
> exposing the PTP clock to userland via the posix clocks interface, we
> allow this to easily be done.

Userland code would introduce latencies that would make sub microsecond
time sync very difficult.

> > We can switch underlying clocks for system time already. We can adapt to a
> > different hw frequency.
>
> Actually no. The timekeeping code requires a fixed freq counter. Dealing
> with hardware freq changes is difficult, because error is introduced by
> the latency between when the freq changes and when the timekeeping code
> is notified of it. So the system treats the hardware counters as fixed
> freq. Now, hardware does vary freq ever so slightly as thermal
> conditions change, but this is addressed in userland and corrected via
> adjtimex.

Acadmic hair splitting? I have repeatedly switched between different
clocks on various systems. So its difficult but we do it?

> Unnecessary layers? Where? This approach has less in-kernel layers, as
> it exposes the PTP clock to userland, instead of trying to layer things
> on top of it and stretching the system time abstraction to cover it.

You dont need the user APIs if you directly use the PTP time source to
steer the system clock. In fact I think you have to do it in kernel space
since user space latencies will degrade accuracy otherwise.

> I've argued through the approach trying to keep it all internal to the
> kernel, but to do so would be anything but trivial. Further, there's the
> case of master-clocks, where the PTP hardware must be synced to system
> time, instead of the other way around. And then there's the case of
> boundary-clocks, which may have multiple PTP hardware clocks that have
> to be synced.

Ok maybe we need some sort of control interface to manage the clock like
the others have.

> I think exposing this through the posix clock interface is really the
> best approach. Its not a static clockid, so its not something most apps
> will ever have to deal with, but it allows the few apps that really need
> to have access to the PTP clock hardware can do so in a clean way.

It implies clock tuning in userspace for a potential sub microsecond
accurate clock. The clock accuracy will be limited by user space
latencies and noise. You wont be able to discipline the system clock
accurately.

The posix clocks today assumes one notion of real "time" in the kernel.
All clocks increase in lockstep (aside from offset updates). This approach
here result in multiple notions of "time" increasing at various speeds.
And it implies that someone is user space is trying to tinker around with
extremely low latencies using system call APIs that take much longer than
these intervals to process the data.

^ permalink raw reply

* [PATCH] de2104x: disable autonegotiation on broken hardware
From: Ondrej Zary @ 2010-09-23 20:59 UTC (permalink / raw)
  To: jgarzik; +Cc: netdev, Kernel development list

At least on older 21041-AA chips (mine is rev. 11), TP duplex autonegotiation
causes the card not to work at all (link is up but no packets are transmitted).

de4x5 disables autonegotiation completely. But it seems to work on newer
(21041-PA rev. 21) so disable it only on rev<20 chips.

Signed-off-by: Ondrej Zary <linux@rainbow-software.org>

--- linux-2.6.36-rc3-orig/drivers/net/tulip/de2104x.c	2010-08-29 17:36:04.000000000 +0200
+++ linux-2.6.36-rc3/drivers/net/tulip/de2104x.c	2010-09-24 00:27:41.000000000 +0200
@@ -364,6 +364,8 @@ static u16 t21040_csr15[] = { 0, 0, 0x00
 /* 21041 transceiver register settings: TP AUTO, BNC, AUI, TP, TP FD*/
 static u16 t21041_csr13[] = { 0xEF01, 0xEF09, 0xEF09, 0xEF01, 0xEF09, };
 static u16 t21041_csr14[] = { 0xFFFF, 0xF7FD, 0xF7FD, 0x6F3F, 0x6F3D, };
+/* If on-chip autonegotiation is broken, use half-duplex (FF3F) instead */
+static u16 t21041_csr14_brk[] = { 0xFF3F, 0xF7FD, 0xF7FD, 0x6F3F, 0x6F3D, };
 static u16 t21041_csr15[] = { 0x0008, 0x0006, 0x000E, 0x0008, 0x0008, };
 
 
@@ -1911,8 +1913,14 @@ fill_defaults:
 	for (i = 0; i < DE_MAX_MEDIA; i++) {
 		if (de->media[i].csr13 == 0xffff)
 			de->media[i].csr13 = t21041_csr13[i];
-		if (de->media[i].csr14 == 0xffff)
-			de->media[i].csr14 = t21041_csr14[i];
+		if (de->media[i].csr14 == 0xffff) {
+			/* autonegotiation is broken at least on some chip
+			   revisions - rev. 0x21 works, 0x11 does not */
+			if (de->pdev->revision < 0x20)
+				de->media[i].csr14 = t21041_csr14_brk[i];
+			else
+				de->media[i].csr14 = t21041_csr14[i];
+		}
 		if (de->media[i].csr15 == 0xffff)
 			de->media[i].csr15 = t21041_csr15[i];
 	}


-- 
Ondrej Zary

^ permalink raw reply

* Re: [PATCH] de2104x: disable autonegotiation on broken hardware
From: Jeff Garzik @ 2010-09-23 21:03 UTC (permalink / raw)
  To: Ondrej Zary; +Cc: netdev, Kernel development list
In-Reply-To: <201009232259.20261.linux@rainbow-software.org>

On Thu, Sep 23, 2010 at 4:59 PM, Ondrej Zary <linux@rainbow-software.org> wrote:
> At least on older 21041-AA chips (mine is rev. 11), TP duplex autonegotiation
> causes the card not to work at all (link is up but no packets are transmitted).
>
> de4x5 disables autonegotiation completely. But it seems to work on newer
> (21041-PA rev. 21) so disable it only on rev<20 chips.
>
> Signed-off-by: Ondrej Zary <linux@rainbow-software.org>
>
> --- linux-2.6.36-rc3-orig/drivers/net/tulip/de2104x.c   2010-08-29 17:36:04.000000000 +0200
> +++ linux-2.6.36-rc3/drivers/net/tulip/de2104x.c        2010-09-24 00:27:41.000000000 +0200
> @@ -364,6 +364,8 @@ static u16 t21040_csr15[] = { 0, 0, 0x00
>  /* 21041 transceiver register settings: TP AUTO, BNC, AUI, TP, TP FD*/
>  static u16 t21041_csr13[] = { 0xEF01, 0xEF09, 0xEF09, 0xEF01, 0xEF09, };
>  static u16 t21041_csr14[] = { 0xFFFF, 0xF7FD, 0xF7FD, 0x6F3F, 0x6F3D, };
> +/* If on-chip autonegotiation is broken, use half-duplex (FF3F) instead */
> +static u16 t21041_csr14_brk[] = { 0xFF3F, 0xF7FD, 0xF7FD, 0x6F3F, 0x6F3D, };
>  static u16 t21041_csr15[] = { 0x0008, 0x0006, 0x000E, 0x0008, 0x0008, };
>
>
> @@ -1911,8 +1913,14 @@ fill_defaults:
>        for (i = 0; i < DE_MAX_MEDIA; i++) {
>                if (de->media[i].csr13 == 0xffff)
>                        de->media[i].csr13 = t21041_csr13[i];
> -               if (de->media[i].csr14 == 0xffff)
> -                       de->media[i].csr14 = t21041_csr14[i];
> +               if (de->media[i].csr14 == 0xffff) {
> +                       /* autonegotiation is broken at least on some chip
> +                          revisions - rev. 0x21 works, 0x11 does not */
> +                       if (de->pdev->revision < 0x20)
> +                               de->media[i].csr14 = t21041_csr14_brk[i];
> +                       else
> +                               de->media[i].csr14 = t21041_csr14[i];
> +               }

Interesting...  I never knew about that quirk.

Acked-by: Jeff Garzik <jgarzik@redhat.com>

^ permalink raw reply

* [PATCH] netfilter: fix kconfig unmet dependency warning
From: Randy Dunlap @ 2010-09-23 21:03 UTC (permalink / raw)
  To: netdev; +Cc: davem, netfilter-devel, Patrick McHardy

From: Randy Dunlap <randy.dunlap@oracle.com>

Fix netfilter kconfig unmet dependencies warning & spell out
"compatible" while there.

warning: (IP_NF_TARGET_TTL && NET && INET && NETFILTER && IP_NF_IPTABLES && NETFILTER_ADVANCED || IP6_NF_TARGET_HL && NET && INET && IPV6 && NETFILTER && IP6_NF_IPTABLES && NETFILTER_ADVANCED) selects NETFILTER_XT_TARGET_HL which has unmet direct dependencies ((IP_NF_MANGLE || IP6_NF_MANGLE) && NETFILTER_ADVANCED)

Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
---
 net/ipv4/netfilter/Kconfig |    4 ++--
 net/ipv6/netfilter/Kconfig |    4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

--- linux-next-20100921.orig/net/ipv4/netfilter/Kconfig
+++ linux-next-20100921/net/ipv4/netfilter/Kconfig
@@ -324,10 +324,10 @@ config IP_NF_TARGET_ECN
 
 config IP_NF_TARGET_TTL
 	tristate '"TTL" target support'
-	depends on NETFILTER_ADVANCED
+	depends on NETFILTER_ADVANCED && IP_NF_MANGLE
 	select NETFILTER_XT_TARGET_HL
 	---help---
-	This is a backwards-compat option for the user's convenience
+	This is a backwards-compatible option for the user's convenience
 	(e.g. when running oldconfig). It selects
 	CONFIG_NETFILTER_XT_TARGET_HL.
 
--- linux-next-20100921.orig/net/ipv6/netfilter/Kconfig
+++ linux-next-20100921/net/ipv6/netfilter/Kconfig
@@ -132,10 +132,10 @@ config IP6_NF_MATCH_RT
 # The targets
 config IP6_NF_TARGET_HL
 	tristate '"HL" hoplimit target support'
-	depends on NETFILTER_ADVANCED
+	depends on NETFILTER_ADVANCED && IP6_NF_MANGLE
 	select NETFILTER_XT_TARGET_HL
 	---help---
-	This is a backwards-compat option for the user's convenience
+	This is a backwards-compatible option for the user's convenience
 	(e.g. when running oldconfig). It selects
 	CONFIG_NETFILTER_XT_TARGET_HL.
 

^ permalink raw reply

* Re: [PATCH] de2104x: disable autonegotiation on broken hardware
From: Ondrej Zary @ 2010-09-23 21:18 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: netdev, Kernel development list
In-Reply-To: <AANLkTikwXyr4pH5rqkoXpiYvkvpLc7s8tq-EwmM1+r7m@mail.gmail.com>

On Thursday 23 September 2010 23:03:13 Jeff Garzik wrote:
> On Thu, Sep 23, 2010 at 4:59 PM, Ondrej Zary <linux@rainbow-software.org> 
wrote:
> > At least on older 21041-AA chips (mine is rev. 11), TP duplex
> > autonegotiation causes the card not to work at all (link is up but no
> > packets are transmitted).
> >
> > de4x5 disables autonegotiation completely. But it seems to work on newer
> > (21041-PA rev. 21) so disable it only on rev<20 chips.
> >
> > Signed-off-by: Ondrej Zary <linux@rainbow-software.org>
> >
> > --- linux-2.6.36-rc3-orig/drivers/net/tulip/de2104x.c   2010-08-29
> > 17:36:04.000000000 +0200 +++ linux-2.6.36-rc3/drivers/net/tulip/de2104x.c
> >        2010-09-24 00:27:41.000000000 +0200 @@ -364,6 +364,8 @@ static u16
> > t21040_csr15[] = { 0, 0, 0x00
> >  /* 21041 transceiver register settings: TP AUTO, BNC, AUI, TP, TP FD*/
> >  static u16 t21041_csr13[] = { 0xEF01, 0xEF09, 0xEF09, 0xEF01, 0xEF09, };
> >  static u16 t21041_csr14[] = { 0xFFFF, 0xF7FD, 0xF7FD, 0x6F3F, 0x6F3D, };
> > +/* If on-chip autonegotiation is broken, use half-duplex (FF3F) instead
> > */ +static u16 t21041_csr14_brk[] = { 0xFF3F, 0xF7FD, 0xF7FD, 0x6F3F,
> > 0x6F3D, }; static u16 t21041_csr15[] = { 0x0008, 0x0006, 0x000E, 0x0008,
> > 0x0008, };
> >
> >
> > @@ -1911,8 +1913,14 @@ fill_defaults:
> >        for (i = 0; i < DE_MAX_MEDIA; i++) {
> >                if (de->media[i].csr13 == 0xffff)
> >                        de->media[i].csr13 = t21041_csr13[i];
> > -               if (de->media[i].csr14 == 0xffff)
> > -                       de->media[i].csr14 = t21041_csr14[i];
> > +               if (de->media[i].csr14 == 0xffff) {
> > +                       /* autonegotiation is broken at least on some
> > chip +                          revisions - rev. 0x21 works, 0x11 does
> > not */ +                       if (de->pdev->revision < 0x20)
> > +                               de->media[i].csr14 = t21041_csr14_brk[i];
> > +                       else
> > +                               de->media[i].csr14 = t21041_csr14[i];
> > +               }
>
> Interesting...  I never knew about that quirk.

This errata document says that autonegotiation is somehow broken but it does 
not specify it further:
http://ftp.nluug.nl/ftp/ftp/pub/os/NetBSD/misc/dec-docs/ec-qd2ma-te.ps.gz

-- 
Ondrej Zary

^ permalink raw reply

* [PATCH] net: reset skb queue mapping when rx'ing over tunnel
From: Tom Herbert @ 2010-09-23 21:19 UTC (permalink / raw)
  To: netdev, davem; +Cc: chavey

Reset queue mapping when an skb is reentering the stack via a tunnel.
On second pass, the queue mapping from the original device is no
longer valid.

Signed-off-by: Tom Herbert <therbert@google.com>
---
diff --git a/include/net/dst.h b/include/net/dst.h
index 81d1413..0238650 100644
--- a/include/net/dst.h
+++ b/include/net/dst.h
@@ -242,6 +242,7 @@ static inline void skb_tunnel_rx(struct sk_buff *skb, struct net_device *dev)
 	dev->stats.rx_packets++;
 	dev->stats.rx_bytes += skb->len;
 	skb->rxhash = 0;
+	skb_set_queue_mapping(skb, 0);
 	skb_dst_drop(skb);
 	nf_reset(skb);
 }

^ permalink raw reply related

* Re: [PATCH 1/2 -next] r8169: allocate with GFP_KERNEL flag when able to sleep
From: Francois Romieu @ 2010-09-23 21:20 UTC (permalink / raw)
  To: Stanislaw Gruszka; +Cc: netdev
In-Reply-To: <1285243291-4520-1-git-send-email-sgruszka@redhat.com>

Stanislaw Gruszka <sgruszka@redhat.com> :
> We have fedora bug report where driver fail to initialize after
> suspend/resume because of memory allocation errors:
> https://bugzilla.redhat.com/show_bug.cgi?id=629158
> 
> To fix use GFP_KERNEL allocation where possible. 

Feel free to add a Acked-by: Francois Romieu <romieu@fr.zoreil.com>
as soon as it will have been explicitely reported to improve the
situation (it is not clear in the PR above).

-- 
Ueimor

^ permalink raw reply

* Re: [PATCH 6/8] ptp: Added a clock that uses the eTSEC found on the MPC85xx.
From: Christian Riesch @ 2010-09-23 21:26 UTC (permalink / raw)
  To: Alan Cox
  Cc: Christoph Lameter, Richard Cochran, linux-kernel,
	devicetree-discuss, linux-api, linux-arm-kernel, linuxppc-dev,
	netdev, Arnd Bergmann, David Miller, John Stultz,
	Krzysztof Halasa, Peter Zijlstra, Rodolfo Giometti,
	Thomas Gleixner
In-Reply-To: <20100923214359.3f287b11@lxorguk.ukuu.org.uk>

Alan Cox wrote:
>> Please do not introduce useless additional layers for clock sync. Load
>> these ptp clocks like the other regular clock modules and make them sync
>> system time like any other clock.
> 
> I don't think you understand PTP. PTP has masters, a system can need to
> be honouring multiple conflicting masters at once.

AFAIK the master's should not be conflicting. The Best Master Clock 
algorithm (BMC) defined in IEEE1588 selects the best master clock. This 
clock distributes its notion of time on the network while the other 
masters, that is the other clocks/nodes that are configured to 
potentially become a master, keep quiet. So usually we will only have 
one source of time (the master clock selected by the BMC) and we will 
steer our single PHC (PTP hardware clock) to follow this master (Of 
course there may be use-cases that require more than one PTP clock, 
e.g., for research purposes).

However, if the clock selected by the BMC is switched off, loses its 
network connection..., the second best clock is selected by the BMC and 
becomes master. This clock may be less accurate and thus our slave clock 
has to switch from one notion of time to another. Is that the conflict 
you mentioned?

Christian

^ permalink raw reply

* Re: [PATCH v6 0/8] ptp: IEEE 1588 hardware clock support
From: Alan Cox @ 2010-09-23 21:30 UTC (permalink / raw)
  To: john stultz
  Cc: Richard Cochran, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ,
	netdev-u79uwXL29TY76Z2rM5mHXA, Arnd Bergmann, Christoph Lameter,
	David Miller, Krzysztof Halasa, Peter Zijlstra, Rodolfo Giometti,
	Thomas Gleixner
In-Reply-To: <1285274952.2587.113.camel-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>

O> I don't see how this is a problem, as it exposes the multiple hardware
> clocks via different posix clock ids. So in the boundary clock case, you
> can configure which side is the client and which side is the master in a
> config file and the PTPd will appropriately steer them individually.

They may all be slaves - that means you can't treat them as part of
system time.
> 

> on module unload, but I don't think we need a use-count to prevent the
> module from being unloaded.
> 
> My question would be: How do we handle a USB network device ($14.99 now
> with PTP!) being unplugged? We can't say "Sorry! That's in use!". So we
> note the hardware is gone, and return the proper error code.
> 
> Or am I missing something else?

Open list
Oh number 31 appears to be the device I want
Close list

	USB unplugged
	Random other device plugged

clock_op(31, ....)

Oh bugger I've just reprogrammed the wrong time source.

We don't have stop the device being removed, instead of a disaster you get

	clock_op(fd, blah)
	-ENODEV

which btw is how just about everything else USB works when you pull the
hardware.

> > And about 40 already supported by char driver interface clocks and rtcs
> > in the kernel...
> 
> And those char driver interfaces are all subtly different.
> 
> I actually recently submitted an RFC to expose the RTC devices via the
> posix clock/timer interface, because working with the RTC hardware
> device directly is terrible for managing alarm interrupts. 

Given that driver interfaces are sane and posix clock/timer interfaces
have totally broken enumeration maybe you have it backwards. But if you
follow through to my proposal maybe there is a saner answer still
 
> For instance, you easily run into the case where your TV recording
> application programs an alarm to record your favorite show at 8pm. Then
> your backup script programs an alarm to wake up at 2am to do your
> nightly backups. Your box suspends and the next morning, you're missing
> your favorite show!

Poor resource management, and yes I'd agree you want a sensible interface.


> > Drop the clockid_t and swap it for a file handle like a proper Unix or
> > Linux interface. The rest is much the same
> > 
> > 	fd = open /sys/class/timesource/[whatever]
> > 
> > 	various queries you may want to do to check the name etc
> > 
> > 	fclock_adjtime(fd, ...)
> > 	
> > 
> > The posix interface is fundamentally flawed. It only works for staticly
> > enumerable objects. Unix avoided that forty years ago by making the
> > identifier a handle which immediately cures all your object lifetime
> > problems in one swoop.
> 
> So, I don't really see how that's so different from what is being
> proposed. The clock_id is dynamically assigned per registered clock, and
> exposed via the sysfs interface from ptp hardware entry.
> 
> The only difference is the open/close reference counting, which I don't
> think is necessary here (since we can't always keep the hardware from
> going away).

It is absolutely neccessary in order that you can be sure that two calls
actually relate to the *same* device. It's as fundamental as the
difference betweeh chmod and fchmod although with the added ugliness of
some random numeric identifier stuck in the middle.

It also btw makes it much easier to fix up the existing random collection
of /dev/rtc devices - because you can open them and issue fclock_adjtime
if we are careful how we do it and it makes sense.

Alan

^ permalink raw reply

* Re: [net-next-2.6 PATCH 1/3] e1000: use work queues
From: David Miller @ 2010-09-23 21:34 UTC (permalink / raw)
  To: jeffrey.t.kirsher; +Cc: netdev, gospo, bphilips, jesse.brandeburg
In-Reply-To: <20100923042126.11798.49675.stgit@localhost.localdomain>

From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Wed, 22 Sep 2010 21:22:17 -0700

> From: Jesse Brandeburg <jesse.brandeburg@intel.com>
> 
> E1000 is using several timers that in a follow on patch
> will need to acquire the rtnl_lock in order to be safe.
> 
> This patch moves the timer bodies into work queues which
> will allow the next patch to add rtnl_lock.
> 
> Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
> Tested-by: Jeff Pieper <jeffrey.e.pieper@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

Applied.

^ permalink raw reply

* Re: [net-next-2.6 PATCH 2/3] e1000: fix occasional panic on unload
From: David Miller @ 2010-09-23 21:34 UTC (permalink / raw)
  To: jeffrey.t.kirsher; +Cc: netdev, gospo, bphilips, jesse.brandeburg
In-Reply-To: <20100923042240.11798.65680.stgit@localhost.localdomain>

From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Wed, 22 Sep 2010 21:22:42 -0700

> From: Jesse Brandeburg <jesse.brandeburg@intel.com>
> 
> Net drivers in general have an issue where timers fired
> by mod_timer or work threads with schedule_work are running
> outside of the rtnl_lock.
> 
> With no other lock protection these routines are vulnerable
> to races with driver unload or reset paths.
> 
> The longer term solution to this might be a redesign with
> safer locks being taken in the driver to guarantee no
> reentrance, but for now a safe and effective fix is
> to take the rtnl_lock in these routines.
> 
> Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
> Tested-by: Jeff Pieper <jeffrey.e.pieper@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

Applied.

^ permalink raw reply

* Re: [PATCH v6 0/8] ptp: IEEE 1588 hardware clock support
From: Christian Riesch @ 2010-09-23 21:34 UTC (permalink / raw)
  To: Alan Cox
  Cc: Christoph Lameter, john stultz, Richard Cochran, linux-kernel,
	devicetree-discuss, linux-api, linux-arm-kernel, linuxppc-dev,
	netdev, Arnd Bergmann, David Miller, Krzysztof Halasa,
	Peter Zijlstra, Rodolfo Giometti, Thomas Gleixner
In-Reply-To: <20100923223417.4ed62e5b@lxorguk.ukuu.org.uk>

Alan Cox wrote:
>> It implies clock tuning in userspace for a potential sub microsecond
>> accurate clock. The clock accuracy will be limited by user space
>> latencies and noise. You wont be able to discipline the system clock
>> accurately.
> 
> Noise matters, latency doesn't. 

Well put! That's why we need hardware support for PTP timestamping to 
reduce the noise, but get along well with the clock servo that is 
steering the PHC in user space.

Christian

^ permalink raw reply

* Re: [net-next-2.6 PATCH 3/3] e1000: use GRO for receive
From: David Miller @ 2010-09-23 21:34 UTC (permalink / raw)
  To: jeffrey.t.kirsher; +Cc: netdev, gospo, bphilips, jesse.brandeburg
In-Reply-To: <20100923042303.11798.50335.stgit@localhost.localdomain>

From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Wed, 22 Sep 2010 21:23:05 -0700

> From: Jesse Brandeburg <jesse.brandeburg@intel.com>
> 
> E1000 can benefit from calling the GRO receive functions.
> 
> Signed-off-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
> Tested-by: Jeff Pieper <jeffrey.e.pieper@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

Applied.

^ permalink raw reply

* Re: [PATCH v6 0/8] ptp: IEEE 1588 hardware clock support
From: Alan Cox @ 2010-09-23 21:34 UTC (permalink / raw)
  To: Christoph Lameter
  Cc: john stultz, Richard Cochran, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ,
	netdev-u79uwXL29TY76Z2rM5mHXA, Arnd Bergmann, David Miller,
	Krzysztof Halasa, Peter Zijlstra, Rodolfo Giometti,
	Thomas Gleixner
In-Reply-To: <alpine.DEB.2.00.1009231533040.7522-sBS69tsa9Uj/9pzu0YdTqQ@public.gmane.org>

> There is no other scalable time source available for fast timer access
> than the time stamp counter in the cpu. Other time source require
> memory accesses which is inherently slower.

On what hardware ?

> An accurate other time source is used to adjust this clock. NTP does that
> via the clock interfaces from user space which has its problems with
> accuracy. PTP can provide the network synced time access
> that would a more accurate calibration of the time.

Accuracy does not require speed of access. Accuracy requires predictible
latency of access.

> Userland code would introduce latencies that would make sub microsecond
> time sync very difficult.

You can take a multiple micro-second I/O stall or SMI trap on a PC so you
already lost the battle on the platform you seem to be discussing.

> You dont need the user APIs if you directly use the PTP time source to
> steer the system clock. In fact I think you have to do it in kernel space
> since user space latencies will degrade accuracy otherwise.

PTP is not a 'time source' it is one or more source of time. The
distinction is rather important.

> It implies clock tuning in userspace for a potential sub microsecond
> accurate clock. The clock accuracy will be limited by user space
> latencies and noise. You wont be able to discipline the system clock
> accurately.

Noise matters, latency doesn't. And the kernel is getting more and more
real time support all the time.

^ permalink raw reply

* Re: [PATCH net-next-2.6] net: return operator cleanup
From: David Miller @ 2010-09-23 21:34 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev
In-Reply-To: <1285224237.2380.111.camel@edumazet-laptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 23 Sep 2010 08:43:57 +0200

> Change "return (EXPR);" to "return EXPR;"
> 
> return is not a function, parentheses are not required.
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>

Applied.

^ permalink raw reply

* [PATCH v3] xmit_compl_seq: information to reclaim vmsplice buffers
From: Tom Herbert @ 2010-09-23 21:35 UTC (permalink / raw)
  To: netdev, davem; +Cc: sridharr

In this patch we propose to adds some socket API to retrieve the
 "transmit completion sequence number", essentially a byte counter
for the number of bytes that have been transmitted and will not be
retransmitted.  In the case of TCP, this should correspond to snd_una.

The purpose of this API is to provide information to userspace about
which buffers can be reclaimed when sending with vmsplice() on a
socket.

There are two methods for retrieving the completed sequence number:
through a simple getsockopt (implemented here for TCP), as well as
returning the value in the ancilary data of a recvmsg.

The expected flow would be something like:
   - Connect is created
   - Initial completion seq # is retrieved through the sockopt, and is
     stored in userspace "compl_seq" variable for the connection.
   - Whenever a send is done, compl_seq += # bytes sent.
   - When doing a vmsplice the completion sequence number is saved
     for each user space buffer, buffer_compl_seq = compl_seq.
   - When recvmsg returns with a completion sequence number in
     ancillary data, any buffers cover by that sequence number
     (where buffer_compl_seq < recvmsg_compl_seq) are reclaimed
     and can be written to again.
   - If no data is receieved on a connection (recvmsg does not
     return), a timeout can be used to call the getsockopt and
     reclaim buffers as a fallback.

Using recvmsg data in this manner is sort of a cheap way to get a
"callback" for when a vmspliced buffer is consumed.  It will work
well for a client where the response causes recvmsg to return.
On the server side it works well if there are a sufficient
number of requests coming on the connection (resorting to the
timeout if necessary as described above).

Signed-off-by: Tom Herbert <therbert@google.com>
Signed-off-by: Sridhar Raman <sridharr@google.com>
---
diff --git a/arch/alpha/include/asm/socket.h b/arch/alpha/include/asm/socket.h
index 06edfef..3587082 100644
--- a/arch/alpha/include/asm/socket.h
+++ b/arch/alpha/include/asm/socket.h
@@ -69,6 +69,9 @@
 
 #define SO_RXQ_OVFL             40
 
+#define SO_XMIT_COMPL_SEQ	41
+#define SCM_XMIT_COMPL_SEQ	SO_XMIT_COMPL_SEQ
+
 /* O_NONBLOCK clashes with the bits used for socket types.  Therefore we
  * have to define SOCK_NONBLOCK to a different value here.
  */
diff --git a/arch/arm/include/asm/socket.h b/arch/arm/include/asm/socket.h
index 90ffd04..962c365 100644
--- a/arch/arm/include/asm/socket.h
+++ b/arch/arm/include/asm/socket.h
@@ -62,4 +62,6 @@
 
 #define SO_RXQ_OVFL             40
 
+#define SO_XMIT_COMPL_SEQ	41
+#define SCM_XMIT_COMPL_SEQ	SO_XMIT_COMPL_SEQ
 #endif /* _ASM_SOCKET_H */
diff --git a/arch/avr32/include/asm/socket.h b/arch/avr32/include/asm/socket.h
index c8d1fae..de59dfe 100644
--- a/arch/avr32/include/asm/socket.h
+++ b/arch/avr32/include/asm/socket.h
@@ -62,4 +62,6 @@
 
 #define SO_RXQ_OVFL             40
 
+#define SO_XMIT_COMPL_SEQ	41
+#define SCM_XMIT_COMPL_SEQ	SO_XMIT_COMPL_SEQ
 #endif /* __ASM_AVR32_SOCKET_H */
diff --git a/arch/cris/include/asm/socket.h b/arch/cris/include/asm/socket.h
index 1a4a619..fe9a1ba 100644
--- a/arch/cris/include/asm/socket.h
+++ b/arch/cris/include/asm/socket.h
@@ -64,6 +64,8 @@
 
 #define SO_RXQ_OVFL             40
 
+#define SO_XMIT_COMPL_SEQ	41
+#define SCM_XMIT_COMPL_SEQ	SO_XMIT_COMPL_SEQ
 #endif /* _ASM_SOCKET_H */
 
 
diff --git a/arch/frv/include/asm/socket.h b/arch/frv/include/asm/socket.h
index a6b2688..769f1f5 100644
--- a/arch/frv/include/asm/socket.h
+++ b/arch/frv/include/asm/socket.h
@@ -62,5 +62,7 @@
 
 #define SO_RXQ_OVFL             40
 
+#define SO_XMIT_COMPL_SEQ	41
+#define SCM_XMIT_COMPL_SEQ	SO_XMIT_COMPL_SEQ
 #endif /* _ASM_SOCKET_H */
 
diff --git a/arch/h8300/include/asm/socket.h b/arch/h8300/include/asm/socket.h
index 04c0f45..5ef4551 100644
--- a/arch/h8300/include/asm/socket.h
+++ b/arch/h8300/include/asm/socket.h
@@ -62,4 +62,6 @@
 
 #define SO_RXQ_OVFL             40
 
+#define SO_XMIT_COMPL_SEQ	41
+#define SCM_XMIT_COMPL_SEQ	SO_XMIT_COMPL_SEQ
 #endif /* _ASM_SOCKET_H */
diff --git a/arch/ia64/include/asm/socket.h b/arch/ia64/include/asm/socket.h
index 51427ea..217e4d8 100644
--- a/arch/ia64/include/asm/socket.h
+++ b/arch/ia64/include/asm/socket.h
@@ -71,4 +71,6 @@
 
 #define SO_RXQ_OVFL             40
 
+#define SO_XMIT_COMPL_SEQ	41
+#define SCM_XMIT_COMPL_SEQ	SO_XMIT_COMPL_SEQ
 #endif /* _ASM_IA64_SOCKET_H */
diff --git a/arch/m32r/include/asm/socket.h b/arch/m32r/include/asm/socket.h
index 469787c..e0830c6 100644
--- a/arch/m32r/include/asm/socket.h
+++ b/arch/m32r/include/asm/socket.h
@@ -62,4 +62,6 @@
 
 #define SO_RXQ_OVFL             40
 
+#define SO_XMIT_COMPL_SEQ	41
+#define SCM_XMIT_COMPL_SEQ	SO_XMIT_COMPL_SEQ
 #endif /* _ASM_M32R_SOCKET_H */
diff --git a/arch/m68k/include/asm/socket.h b/arch/m68k/include/asm/socket.h
index 9bf49c8..438cb8e 100644
--- a/arch/m68k/include/asm/socket.h
+++ b/arch/m68k/include/asm/socket.h
@@ -62,4 +62,6 @@
 
 #define SO_RXQ_OVFL             40
 
+#define SO_XMIT_COMPL_SEQ	41
+#define SCM_XMIT_COMPL_SEQ	SO_XMIT_COMPL_SEQ
 #endif /* _ASM_SOCKET_H */
diff --git a/arch/mips/include/asm/socket.h b/arch/mips/include/asm/socket.h
index 9de5190..cb927d7 100644
--- a/arch/mips/include/asm/socket.h
+++ b/arch/mips/include/asm/socket.h
@@ -82,6 +82,9 @@ To add: #define SO_REUSEPORT 0x0200	/* Allow local address and port reuse.  */
 
 #define SO_RXQ_OVFL             40
 
+#define SO_XMIT_COMPL_SEQ	41
+#define SCM_XMIT_COMPL_SEQ	SO_XMIT_COMPL_SEQ
+
 #ifdef __KERNEL__
 
 /** sock_type - Socket types
diff --git a/arch/mn10300/include/asm/socket.h b/arch/mn10300/include/asm/socket.h
index 4e60c42..1c09487 100644
--- a/arch/mn10300/include/asm/socket.h
+++ b/arch/mn10300/include/asm/socket.h
@@ -62,4 +62,6 @@
 
 #define SO_RXQ_OVFL             40
 
+#define SO_XMIT_COMPL_SEQ	41
+#define SCM_XMIT_COMPL_SEQ	SO_XMIT_COMPL_SEQ
 #endif /* _ASM_SOCKET_H */
diff --git a/arch/parisc/include/asm/socket.h b/arch/parisc/include/asm/socket.h
index 225b7d6..c1b0479 100644
--- a/arch/parisc/include/asm/socket.h
+++ b/arch/parisc/include/asm/socket.h
@@ -61,6 +61,9 @@
 
 #define SO_RXQ_OVFL             0x4021
 
+#define SO_XMIT_COMPL_SEQ	0x4022
+#define SCM_XMIT_COMPL_SEQ	SO_XMIT_COMPL_SEQ
+
 /* O_NONBLOCK clashes with the bits used for socket types.  Therefore we
  * have to define SOCK_NONBLOCK to a different value here.
  */
diff --git a/arch/powerpc/include/asm/socket.h b/arch/powerpc/include/asm/socket.h
index 866f760..9452547 100644
--- a/arch/powerpc/include/asm/socket.h
+++ b/arch/powerpc/include/asm/socket.h
@@ -69,4 +69,6 @@
 
 #define SO_RXQ_OVFL             40
 
+#define SO_XMIT_COMPL_SEQ	41
+#define SCM_XMIT_COMPL_SEQ	SO_XMIT_COMPL_SEQ
 #endif	/* _ASM_POWERPC_SOCKET_H */
diff --git a/arch/s390/include/asm/socket.h b/arch/s390/include/asm/socket.h
index fdff1e9..3e3c17b 100644
--- a/arch/s390/include/asm/socket.h
+++ b/arch/s390/include/asm/socket.h
@@ -70,4 +70,6 @@
 
 #define SO_RXQ_OVFL             40
 
+#define SO_XMIT_COMPL_SEQ	41
+#define SCM_XMIT_COMPL_SEQ	SO_XMIT_COMPL_SEQ
 #endif /* _ASM_SOCKET_H */
diff --git a/arch/sparc/include/asm/socket.h b/arch/sparc/include/asm/socket.h
index 9d3fefc..0675f54 100644
--- a/arch/sparc/include/asm/socket.h
+++ b/arch/sparc/include/asm/socket.h
@@ -58,6 +58,9 @@
 
 #define SO_RXQ_OVFL             0x0024
 
+#define SO_XMIT_COMPL_SEQ	0x0025
+#define SCM_XMIT_COMPL_SEQ	SO_XMIT_COMPL_SEQ
+
 /* Security levels - as per NRL IPv6 - don't actually do anything */
 #define SO_SECURITY_AUTHENTICATION		0x5001
 #define SO_SECURITY_ENCRYPTION_TRANSPORT	0x5002
diff --git a/arch/xtensa/include/asm/socket.h b/arch/xtensa/include/asm/socket.h
index cbdf2ff..3c5afbf 100644
--- a/arch/xtensa/include/asm/socket.h
+++ b/arch/xtensa/include/asm/socket.h
@@ -73,4 +73,6 @@
 
 #define SO_RXQ_OVFL             40
 
+#define SO_XMIT_COMPL_SEQ	41
+#define SCM_XMIT_COMPL_SEQ	SO_XMIT_COMPL_SEQ
 #endif	/* _XTENSA_SOCKET_H */
diff --git a/include/asm-generic/socket.h b/include/asm-generic/socket.h
index 9a6115e..6dc1ed8 100644
--- a/include/asm-generic/socket.h
+++ b/include/asm-generic/socket.h
@@ -64,4 +64,7 @@
 #define SO_DOMAIN		39
 
 #define SO_RXQ_OVFL             40
+
+#define SO_XMIT_COMPL_SEQ	41
+#define SCM_XMIT_COMPL_SEQ	SO_XMIT_COMPL_SEQ
 #endif /* __ASM_GENERIC_SOCKET_H */
diff --git a/include/linux/tcp.h b/include/linux/tcp.h
index e64f4c6..f044aff 100644
--- a/include/linux/tcp.h
+++ b/include/linux/tcp.h
@@ -106,6 +106,7 @@ enum {
 #define TCP_THIN_LINEAR_TIMEOUTS 16      /* Use linear timeouts for thin streams*/
 #define TCP_THIN_DUPACK         17      /* Fast retrans. after 1 dupack */
 #define TCP_USER_TIMEOUT	18	/* How long for loss retry before timeout */
+#define TCP_XMIT_COMPL_SEQ	19	/* Return current snd_una */
 
 /* for TCP_INFO socket option */
 #define TCPI_OPT_TIMESTAMPS	1
diff --git a/include/net/sock.h b/include/net/sock.h
index 8ae97c4..e820e2b 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -543,6 +543,7 @@ enum sock_flags {
 	SOCK_TIMESTAMPING_SYS_HARDWARE, /* %SOF_TIMESTAMPING_SYS_HARDWARE */
 	SOCK_FASYNC, /* fasync() active */
 	SOCK_RXQ_OVFL,
+	SOCK_XMIT_COMPL_SEQ, /* SO_XMIT_COMPL_SEQ setting */
 };
 
 static inline void sock_copy_flags(struct sock *nsk, struct sock *osk)
diff --git a/net/core/sock.c b/net/core/sock.c
index f3a06c4..7a10215 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -740,6 +740,12 @@ set_rcvbuf:
 		else
 			sock_reset_flag(sk, SOCK_RXQ_OVFL);
 		break;
+	case SO_XMIT_COMPL_SEQ:
+		if (valbool)
+			sock_set_flag(sk, SOCK_XMIT_COMPL_SEQ);
+		else
+			sock_reset_flag(sk, SOCK_XMIT_COMPL_SEQ);
+		break;
 	default:
 		ret = -ENOPROTOOPT;
 		break;
@@ -961,6 +967,10 @@ int sock_getsockopt(struct socket *sock, int level, int optname,
 		v.val = !!sock_flag(sk, SOCK_RXQ_OVFL);
 		break;
 
+	case SO_XMIT_COMPL_SEQ:
+		v.val = !!sock_flag(sk, SOCK_XMIT_COMPL_SEQ);
+		break;
+
 	default:
 		return -ENOPROTOOPT;
 	}
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 3e8a4db..3d8d33f 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -1768,6 +1768,12 @@ skip_copy:
 
 	TCP_CHECK_TIMER(sk);
 	release_sock(sk);
+
+	/* Copy the first unacked seq into the receive msg control part. */
+	if (sock_flag(sk, SOCK_XMIT_COMPL_SEQ))
+		put_cmsg(msg, SOL_SOCKET, SCM_XMIT_COMPL_SEQ,
+		    sizeof(tp->snd_una), &tp->snd_una);
+
 	return copied;
 
 out:
@@ -2617,6 +2623,9 @@ static int do_tcp_getsockopt(struct sock *sk, int level,
 	case TCP_USER_TIMEOUT:
 		val = jiffies_to_msecs(icsk->icsk_user_timeout);
 		break;
+	case TCP_XMIT_COMPL_SEQ:
+		val = tp->snd_una;
+		break;
 	default:
 		return -ENOPROTOOPT;
 	}

^ permalink raw reply related

* Re: [PATCH v6 0/8] ptp: IEEE 1588 hardware clock support
From: john stultz @ 2010-09-23 21:42 UTC (permalink / raw)
  To: Christoph Lameter
  Cc: Richard Cochran, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ,
	netdev-u79uwXL29TY76Z2rM5mHXA, Arnd Bergmann, David Miller,
	Krzysztof Halasa, Peter Zijlstra, Rodolfo Giometti,
	Thomas Gleixner
In-Reply-To: <alpine.DEB.2.00.1009231533040.7522-sBS69tsa9Uj/9pzu0YdTqQ@public.gmane.org>

On Thu, 2010-09-23 at 15:49 -0500, Christoph Lameter wrote:
> On Thu, 23 Sep 2010, john stultz wrote:
> 
> > > The HPET or pit timesource are also quite slow these days. You only need
> > > access periodically to essentially tune the TSC ratio.
> >
> > If we're using the TSC, then we're not using the PTP clock as you
> > suggest. Further the HPET and PIT aren't used to steer the system time
> > when we are using the TSC as a clocksource. Its only used to calibrate
> > the initial constant freq used by the timekeeping code (and if its
> > non-constant, we throw it out).
> 
> There is no other scalable time source available for fast timer access
> than the time stamp counter in the cpu. Other time source require
> memory accesses which is inherently slower.

Right, but no one likes the HPET or ACPI PM for a clocksource, its just
the TSC isn't usable in some cases, so they have to be used.

We don't want to force folks to decide between closely sycned time and
fast time reads. So that is part of the reason why PTP as a clocksource
isn't a good idea.

> An accurate other time source is used to adjust this clock. NTP does that
> via the clock interfaces from user space which has its problems with
> accuracy. PTP can provide the network synced time access
> that would a more accurate calibration of the time.

Calibration isn't whats needed here (it is an issue, but a separate one
- and I've got some patches if you're interested!) as its a one-time
source of error and can be corrected by ntp today without trouble.
Adjustments to the system time is something that has to be done
continuously to handle for variable thermal drift over time.

> > 2) The way PTP clocks are steered to sync with network time causes their
> > hardware freq to actually change. Since these adjustments are done on
> > the hardware clock level, and not on the system time level, the
> > adjustments to sync the system time/freq would then be made incorrect by
> > PTP hardware adjustments.
> 
> Right. So use these as a way to fine tune the TSC clock (and thereby the
> system time).

So you're then not suggesting to "use the PTP as a clocksource".

Using the PTP hardware to adjust the system time freq is exactly whats
being proposed.

> > 3) Further, the PTP hardware counter can be simply set to a new offset
> > to put it in line with the network time. This could cause trouble with
> > timekeeping much like unsynced TSCs do.
> 
> You can do the same for system time.

Settimeofday does allow CLOCK_REALTIME to jump, but the CLOCK_MONOTONIC
time cannot jump around. Having a clocksource that is non-monotonic
would break this.

> > Now, what you seem to be suggesting is to use the TSC (or whatever
> > clocksource the system time is using) but to steer the system time using
> > the PTP clock. This is actually what is being proposed, however, the
> > steering is done in userland. This is due to the fact that there are two
> > components to the steering, 1) adjusting the PTP clock hardware to
> > network time and 2) adjusting the system time to the PTP hardware. By
> > exposing the PTP clock to userland via the posix clocks interface, we
> > allow this to easily be done.
> 
> Userland code would introduce latencies that would make sub microsecond
> time sync very difficult.

The design actually avoids most userland induced latency.

1) On the PTP hardware syncing point, the reference packet gets
timestamped with the PTP hardware time on arrival. This allows the
offset calculation to be done in userland without introducing latency.

2) On the system syncing side, the proposal for the PPS interrupt allows
the PTP hardware to trigger an interrupt on the second boundary that
would take a timestamp of the system time. Then the pps interface allows
for the timestamp to be read from userland allowing the offset to be
calculated without introducing additional latency.


> > > We can switch underlying clocks for system time already. We can adapt to a
> > > different hw frequency.
> >
> > Actually no. The timekeeping code requires a fixed freq counter. Dealing
> > with hardware freq changes is difficult, because error is introduced by
> > the latency between when the freq changes and when the timekeeping code
> > is notified of it. So the system treats the hardware counters as fixed
> > freq. Now, hardware does vary freq ever so slightly as thermal
> > conditions change, but this is addressed in userland and corrected via
> > adjtimex.
> 
> Acadmic hair splitting? I have repeatedly switched between different
> clocks on various systems. So its difficult but we do it?

Sure, we handle the fairly-rare case of switching clocksources. And that
introduces a bit of error each time. But one doesn't expect to be
switching clock-sources every second and still keep synced time.


> > Unnecessary layers? Where? This approach has less in-kernel layers, as
> > it exposes the PTP clock to userland, instead of trying to layer things
> > on top of it and stretching the system time abstraction to cover it.
> 
> You dont need the user APIs if you directly use the PTP time source to
> steer the system clock. In fact I think you have to do it in kernel space
> since user space latencies will degrade accuracy otherwise.

Via the PPS interface, this can be done easily without large latency.

Additionally, even just in userland, it would be easy to bracket two
reads of the system time around one read of the PTP clock to bound any
userland latency fairly well. It may not be as good as the PPS interface
(although that depends on the interrupt latency), but if the accesses
are all local, it probably could get fairly close.

> > I've argued through the approach trying to keep it all internal to the
> > kernel, but to do so would be anything but trivial. Further, there's the
> > case of master-clocks, where the PTP hardware must be synced to system
> > time, instead of the other way around. And then there's the case of
> > boundary-clocks, which may have multiple PTP hardware clocks that have
> > to be synced.
> 
> Ok maybe we need some sort of control interface to manage the clock like
> the others have.

That's what the clock_adjtime call provides.


> > I think exposing this through the posix clock interface is really the
> > best approach. Its not a static clockid, so its not something most apps
> > will ever have to deal with, but it allows the few apps that really need
> > to have access to the PTP clock hardware can do so in a clean way.
> 
> It implies clock tuning in userspace for a potential sub microsecond
> accurate clock. The clock accuracy will be limited by user space
> latencies and noise. You wont be able to discipline the system clock
> accurately.

I think you'll find that the userspace latency issue is fairly well
addressed by the existing design.


> The posix clocks today assumes one notion of real "time" in the kernel.
> All clocks increase in lockstep (aside from offset updates).

Not true. The cputime clockids do not increment at the same rate (as the
apps don't always run). Further CLOCK_MONOTONIC_RAW provides a non-freq
corrected view of CLOCK_MONOTONIC, so it increments at a slightly
different rate.

>  This approach
> here result in multiple notions of "time" increasing at various speeds.

Yes. I found this initially to be distasteful as well. But the more I
realized its just a reality of the hardware, and that as long as its not
advertised along side of CLOCK_REALTIME and CLOCK_MONOTONIC (the ptp
clock ids are dynamic and somewhat hidden in the sysfs tree), so
userland developers don't get confused, its not so bad. 

Especially since driver writers will just expose the same stuff via a
chardev ioctl in a less standard way and its likely no one will notice.
Re-using the fairly nice (Alan of course disagrees :) posix interface
seems at least a little better for application developers who actually
have to use the hardware.

thanks
-john

^ permalink raw reply

* [PATCH ethtool 0/7] Update RX n-tuple filtering
From: Ben Hutchings @ 2010-09-23 21:47 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: netdev, linux-net-drivers

This patch series brings ethtool up to date with my recent changes to RX
n-tuple filtering in the kernel.

Ben.

Ben Hutchings (7):
  ethtool-copy.h: sync with net-next
  ethtool: Generalise cmdline_info::unwanted_val to a "seen" flag or
    bitmask
  ethtool: Fix RX n-tuple masks and documentation
  ethtool: Add MAC parameter type based on the parse_sopass() function
  ethtool: Add Ethernet-level RX n-tuple filtering and 'clear' action
  ethtool: Update sfc register dump
  ethtool: Add my authorship and Solarflare copyright notice

 AUTHORS        |    1 +
 ethtool-copy.h |  228 +++++++++++++++++++++++++++++++++---------------
 ethtool.8      |   79 ++++++++++++-----
 ethtool.c      |  265 +++++++++++++++++++++++++++++++++++++++++---------------
 sfc.c          |   25 +++---
 5 files changed, 426 insertions(+), 172 deletions(-)

-- 
1.7.2.1


-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
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


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox