Skip to contentSkip to conversation

pull request: sfc-next-2.6 2011-02-15

PATCH New Ben Hutchings 3 participants 10 messages Feb 15, 2011 – Feb 22, 2011

Subsystems (3)
Networking Network drivers Traffic control (tc)
lore
Share & tools
lore

Ben Hutchings

author
#0 lore raw

The following changes since commit 263fb5b1bf9265d0e4ce59ff6ea92f478b5b61ea:

Merge branch 'master' of master.kernel.org:/pub/scm/linux/kernel/git/davem/net-2.6 (2011-02-08 17:19:01 -0800)

are available in the git repository at:

git://git.kernel.org/pub/scm/linux/kernel/git/bwh/sfc-next-2.6.git for-davem

Multiqueue TX priority support for sfc, and some necessary fixes elsewhere.

Ben.

Ben Hutchings (5):
sch_mqprio: Always set num_tc to 0 in mqprio_destroy()
net: Adjust TX queue kobjects if number of queues changes during unregister
sfc: Move TX queue core queue mapping into tx.c
sfc: Distinguish queue lookup from test for queue existence
sfc: Add TX queues for high-priority traffic

 drivers/net/sfc/efx.c        |   31 ++++----------
 drivers/net/sfc/efx.h        |    2 +
 drivers/net/sfc/ethtool.c    |    6 +-
 drivers/net/sfc/net_driver.h |   64 +++++++++++++++++++++++-------
 drivers/net/sfc/nic.c        |   51 ++++++++++++++++++------
 drivers/net/sfc/regs.h       |    6 +++
 drivers/net/sfc/selftest.c   |    2 +-
 drivers/net/sfc/tx.c         |   90 ++++++++++++++++++++++++++++++++++++++++-
 net/core/dev.c               |    3 +-
 net/sched/sch_mqprio.c       |   14 +++---
 10 files changed, 204 insertions(+), 65 deletions(-)
Signature · 3 lines
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.
author
#1 lore raw

PATCH 1/5 sch_mqprio: Always set num_tc to 0 in mqprio_destroy()

All the cleanup code in mqprio_destroy() is currently conditional on priv->qdiscs being non-null, but that condition should only apply to the per-queue qdisc cleanup. We should always set the number of traffic classes back to 0 here.

Signed-off-by Ben Hutchings <[email protected]>
---
 net/sched/sch_mqprio.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)
net/sched/sch_mqprio.c +7 -7
index effd4ee..ace37f9 100644
@@ -29,18 +29,18 @@ static void mqprio_destroy(struct Qdisc *sch)
 	struct mqprio_sched *priv = qdisc_priv(sch);
 	unsigned int ntx;
 
-	if (!priv->qdiscs)
-		return;
-
-	for (ntx = 0; ntx < dev->num_tx_queues && priv->qdiscs[ntx]; ntx++)
-		qdisc_destroy(priv->qdiscs[ntx]);
+	if (priv->qdiscs) {
+		for (ntx = 0;
+		     ntx < dev->num_tx_queues && priv->qdiscs[ntx];
+		     ntx++)
+			qdisc_destroy(priv->qdiscs[ntx]);
+		kfree(priv->qdiscs);
+	}
 
 	if (priv->hw_owned && dev->netdev_ops->ndo_setup_tc)
 		dev->netdev_ops->ndo_setup_tc(dev, 0);
 	else
 		netdev_set_num_tc(dev, 0);
-
-	kfree(priv->qdiscs);
 }
 
 static int mqprio_parse_opt(struct net_device *dev, struct tc_mqprio_qopt *qopt)
Signature · 8 lines
1.7.3.4



-- 
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.
author
#2 lore raw

PATCH 2/5 net: Adjust TX queue kobjects if number of queues changes during unregister

In reply to Ben Hutchings

If the root qdisc for a net device is mqprio, and the driver's ndo_setup_tc() operation dynamically adds and remvoes TX queues, netif_set_real_num_tx_queues() will be called during device unregistration to remove the extra TX queues when the qdisc is destroyed. Currently this causes the corresponding kobjects to be leaked, and the device's reference count never drops to 0.

Signed-off-by Ben Hutchings <[email protected]>
---
 net/core/dev.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
net/core/dev.c +2 -1
index 6392ea0..30c71f9 100644
@@ -1648,7 +1648,8 @@ int netif_set_real_num_tx_queues(struct net_device *dev, unsigned int txq)
 	if (txq < 1 || txq > dev->num_tx_queues)
 		return -EINVAL;
 
-	if (dev->reg_state == NETREG_REGISTERED) {
+	if (dev->reg_state == NETREG_REGISTERED ||
+	    dev->reg_state == NETREG_UNREGISTERING) {
 		ASSERT_RTNL();
 
 		rc = netdev_queue_update_kobjects(dev, dev->real_num_tx_queues,
Signature · 8 lines
1.7.3.4



-- 
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.
author
#3 lore raw

PATCH 3/5 sfc: Move TX queue core queue mapping into tx.c

In reply to Ben Hutchings

efx_hard_start_xmit() needs to implement a mapping which is the inverse of tx_queue::core_txq. Move the initialisation of tx_queue::core_txq next to efx_hard_start_xmit() to make the connection more obvious.

Signed-off-by Ben Hutchings <[email protected]>
---
 drivers/net/sfc/efx.c |    6 ++----
 drivers/net/sfc/efx.h |    1 +
 drivers/net/sfc/tx.c  |    7 +++++++
 3 files changed, 10 insertions(+), 4 deletions(-)
drivers/net/sfc/efx.c +2 -4
index 002bac7..c559bc3 100644
@@ -1910,10 +1910,8 @@ static int efx_register_netdev(struct efx_nic *efx)
 
 	efx_for_each_channel(channel, efx) {
 		struct efx_tx_queue *tx_queue;
-		efx_for_each_channel_tx_queue(tx_queue, channel) {
-			tx_queue->core_txq = netdev_get_tx_queue(
-				efx->net_dev, tx_queue->queue / EFX_TXQ_TYPES);
-		}
+		efx_for_each_channel_tx_queue(tx_queue, channel)
+			efx_init_tx_queue_core_txq(tx_queue);
 	}
 
 	/* Always start with carrier off; PHY events will detect the link */
drivers/net/sfc/efx.h +1 -0
index d43a7e5..1162070 100644
@@ -29,6 +29,7 @@
 extern int efx_probe_tx_queue(struct efx_tx_queue *tx_queue);
 extern void efx_remove_tx_queue(struct efx_tx_queue *tx_queue);
 extern void efx_init_tx_queue(struct efx_tx_queue *tx_queue);
+extern void efx_init_tx_queue_core_txq(struct efx_tx_queue *tx_queue);
 extern void efx_fini_tx_queue(struct efx_tx_queue *tx_queue);
 extern void efx_release_tx_buffers(struct efx_tx_queue *tx_queue);
 extern netdev_tx_t
drivers/net/sfc/tx.c +7 -0
index 2f5e9da..7e463fb 100644
@@ -347,6 +347,13 @@ netdev_tx_t efx_hard_start_xmit(struct sk_buff *skb,
 	return efx_enqueue_skb(tx_queue, skb);
 }
 
+void efx_init_tx_queue_core_txq(struct efx_tx_queue *tx_queue)
+{
+	/* Must be inverse of queue lookup in efx_hard_start_xmit() */
+	tx_queue->core_txq = netdev_get_tx_queue(
+		tx_queue->efx->net_dev, tx_queue->queue / EFX_TXQ_TYPES);
+}
+
 void efx_xmit_done(struct efx_tx_queue *tx_queue, unsigned int index)
 {
 	unsigned fill_level;
Signature · 8 lines
1.7.3.4



-- 
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.
author
#4 lore raw

PATCH 4/5 sfc: Distinguish queue lookup from test for queue existence

In reply to Ben Hutchings

efx_channel_get_{rx,tx}_queue() currently return NULL if the channel isn't used for traffic in that direction. In most cases this is a bug, but some callers rely on it as an existence test.

Add existence test functions efx_channel_has_{rx_queue,tx_queues}() and use them as appropriate.

Change efx_channel_get_{rx,tx}_queue() to assert that the requested queue exists.

Remove now-redundant initialisation from efx_set_channels().

Signed-off-by Ben Hutchings <[email protected]>
---
 drivers/net/sfc/efx.c        |   17 ++---------------
 drivers/net/sfc/ethtool.c    |    6 +++---
 drivers/net/sfc/net_driver.h |   39 ++++++++++++++++++++++++++++-----------
 3 files changed, 33 insertions(+), 29 deletions(-)
drivers/net/sfc/efx.c +2 -15
index c559bc3..6189d30 100644
@@ -1271,21 +1271,8 @@ static void efx_remove_interrupts(struct efx_nic *efx)
 
 static void efx_set_channels(struct efx_nic *efx)
 {
-	struct efx_channel *channel;
-	struct efx_tx_queue *tx_queue;
-
 	efx->tx_channel_offset =
 		separate_tx_channels ? efx->n_channels - efx->n_tx_channels : 0;
-
-	/* Channel pointers were set in efx_init_struct() but we now
-	 * need to clear them for TX queues in any RX-only channels. */
-	efx_for_each_channel(channel, efx) {
-		if (channel->channel - efx->tx_channel_offset >=
-		    efx->n_tx_channels) {
-			efx_for_each_channel_tx_queue(tx_queue, channel)
-				tx_queue->channel = NULL;
-		}
-	}
 }
 
 static int efx_probe_nic(struct efx_nic *efx)

@@ -1531,9 +1518,9 @@ void efx_init_irq_moderation(struct efx_nic *efx, int tx_usecs, int rx_usecs,
 	efx->irq_rx_adaptive = rx_adaptive;
 	efx->irq_rx_moderation = rx_ticks;
 	efx_for_each_channel(channel, efx) {
-		if (efx_channel_get_rx_queue(channel))
+		if (efx_channel_has_rx_queue(channel))
 			channel->irq_moderation = rx_ticks;
-		else if (efx_channel_get_tx_queue(channel, 0))
+		else if (efx_channel_has_tx_queues(channel))
 			channel->irq_moderation = tx_ticks;
 	}
 }
drivers/net/sfc/ethtool.c +3 -3
index 713969a..272cfe7 100644
@@ -631,7 +631,7 @@ static int efx_ethtool_get_coalesce(struct net_device *net_dev,
 	/* Find lowest IRQ moderation across all used TX queues */
 	coalesce->tx_coalesce_usecs_irq = ~((u32) 0);
 	efx_for_each_channel(channel, efx) {
-		if (!efx_channel_get_tx_queue(channel, 0))
+		if (!efx_channel_has_tx_queues(channel))
 			continue;
 		if (channel->irq_moderation < coalesce->tx_coalesce_usecs_irq) {
 			if (channel->channel < efx->n_rx_channels)

@@ -676,8 +676,8 @@ static int efx_ethtool_set_coalesce(struct net_device *net_dev,
 
 	/* If the channel is shared only allow RX parameters to be set */
 	efx_for_each_channel(channel, efx) {
-		if (efx_channel_get_rx_queue(channel) &&
-		    efx_channel_get_tx_queue(channel, 0) &&
+		if (efx_channel_has_rx_queue(channel) &&
+		    efx_channel_has_tx_queues(channel) &&
 		    tx_usecs) {
 			netif_err(efx, drv, efx->net_dev, "Channel is shared. "
 				  "Only RX coalescing may be set\n");
drivers/net/sfc/net_driver.h +28 -11
index c652702..77b7ce4 100644
@@ -938,19 +938,28 @@ efx_get_tx_queue(struct efx_nic *efx, unsigned index, unsigned type)
 	return &efx->channel[efx->tx_channel_offset + index]->tx_queue[type];
 }
 
+static inline bool efx_channel_has_tx_queues(struct efx_channel *channel)
+{
+	return channel->channel - channel->efx->tx_channel_offset <
+		channel->efx->n_tx_channels;
+}
+
 static inline struct efx_tx_queue *
 efx_channel_get_tx_queue(struct efx_channel *channel, unsigned type)
 {
-	struct efx_tx_queue *tx_queue = channel->tx_queue;
-	EFX_BUG_ON_PARANOID(type >= EFX_TXQ_TYPES);
-	return tx_queue->channel ? tx_queue + type : NULL;
+	EFX_BUG_ON_PARANOID(!efx_channel_has_tx_queues(channel) ||
+			    type >= EFX_TXQ_TYPES);
+	return &channel->tx_queue[type];
 }
 
 /* Iterate over all TX queues belonging to a channel */
 #define efx_for_each_channel_tx_queue(_tx_queue, _channel)		\
-	for (_tx_queue = efx_channel_get_tx_queue(channel, 0);		\
-	     _tx_queue && _tx_queue < (_channel)->tx_queue + EFX_TXQ_TYPES; \
-	     _tx_queue++)
+	if (!efx_channel_has_tx_queues(_channel))			\
+		;							\
+	else								\
+		for (_tx_queue = (_channel)->tx_queue;			\
+		     _tx_queue < (_channel)->tx_queue + EFX_TXQ_TYPES;	\
+		     _tx_queue++)
 
 static inline struct efx_rx_queue *
 efx_get_rx_queue(struct efx_nic *efx, unsigned index)

@@ -959,18 +968,26 @@ efx_get_rx_queue(struct efx_nic *efx, unsigned index)
 	return &efx->channel[index]->rx_queue;
 }
 
+static inline bool efx_channel_has_rx_queue(struct efx_channel *channel)
+{
+	return channel->channel < channel->efx->n_rx_channels;
+}
+
 static inline struct efx_rx_queue *
 efx_channel_get_rx_queue(struct efx_channel *channel)
 {
-	return channel->channel < channel->efx->n_rx_channels ?
-		&channel->rx_queue : NULL;
+	EFX_BUG_ON_PARANOID(!efx_channel_has_rx_queue(channel));
+	return &channel->rx_queue;
 }
 
 /* Iterate over all RX queues belonging to a channel */
 #define efx_for_each_channel_rx_queue(_rx_queue, _channel)		\
-	for (_rx_queue = efx_channel_get_rx_queue(channel);		\
-	     _rx_queue;							\
-	     _rx_queue = NULL)
+	if (!efx_channel_has_rx_queue(_channel))			\
+		;							\
+	else								\
+		for (_rx_queue = &(_channel)->rx_queue;			\
+		     _rx_queue;						\
+		     _rx_queue = NULL)
 
 static inline struct efx_channel *
 efx_rx_queue_channel(struct efx_rx_queue *rx_queue)
Signature · 8 lines
1.7.3.4



-- 
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.
author
#5 lore raw

PATCH 5/5 sfc: Add TX queues for high-priority traffic

In reply to Ben Hutchings

Implement the ndo_setup_tc() operation with 2 traffic classes.

Current Solarstorm controllers do not implement TX queue priority, but they do allow queues to be 'paced' with an enforced delay between packets. Paced and unpaced queues are scheduled in round-robin within two separate hardware bins (paced queues with a large delay may be placed into a third bin temporarily, but we won't use that). If there are queues in both bins, the TX scheduler will alternate between them.

If we make high-priority queues unpaced and best-effort queues paced, and high-priority queues are mostly empty, a single high-priority queue can then instantly take 50% of the packet rate regardless of how many of the best-effort queues have descriptors outstanding.

We do not actually want an enforced delay between packets on best- effort queues, so we set the pace value to a reserved value that actually results in a delay of 0.

Signed-off-by Ben Hutchings <[email protected]>
---
 drivers/net/sfc/efx.c        |    8 ++-
 drivers/net/sfc/efx.h        |    1 +
 drivers/net/sfc/net_driver.h |   29 +++++++++++---
 drivers/net/sfc/nic.c        |   51 ++++++++++++++++++------
 drivers/net/sfc/regs.h       |    6 +++
 drivers/net/sfc/selftest.c   |    2 +-
 drivers/net/sfc/tx.c         |   87 +++++++++++++++++++++++++++++++++++++++--
 7 files changed, 156 insertions(+), 28 deletions(-)
drivers/net/sfc/efx.c +5 -3
index 6189d30..d4e0425 100644
@@ -673,7 +673,7 @@ static void efx_fini_channels(struct efx_nic *efx)
 
 		efx_for_each_channel_rx_queue(rx_queue, channel)
 			efx_fini_rx_queue(rx_queue);
-		efx_for_each_channel_tx_queue(tx_queue, channel)
+		efx_for_each_possible_channel_tx_queue(tx_queue, channel)
 			efx_fini_tx_queue(tx_queue);
 		efx_fini_eventq(channel);
 	}

@@ -689,7 +689,7 @@ static void efx_remove_channel(struct efx_channel *channel)
 
 	efx_for_each_channel_rx_queue(rx_queue, channel)
 		efx_remove_rx_queue(rx_queue);
-	efx_for_each_channel_tx_queue(tx_queue, channel)
+	efx_for_each_possible_channel_tx_queue(tx_queue, channel)
 		efx_remove_tx_queue(tx_queue);
 	efx_remove_eventq(channel);
 }

@@ -1836,6 +1836,7 @@ static const struct net_device_ops efx_netdev_ops = {
 #ifdef CONFIG_NET_POLL_CONTROLLER
 	.ndo_poll_controller = efx_netpoll,
 #endif
+	.ndo_setup_tc		= efx_setup_tc,
 };
 
 static void efx_update_name(struct efx_nic *efx)

@@ -2386,7 +2387,8 @@ static int __devinit efx_pci_probe(struct pci_dev *pci_dev,
 	int i, rc;
 
 	/* Allocate and initialise a struct net_device and struct efx_nic */
-	net_dev = alloc_etherdev_mq(sizeof(*efx), EFX_MAX_CORE_TX_QUEUES);
+	net_dev = alloc_etherdev_mqs(sizeof(*efx), EFX_MAX_CORE_TX_QUEUES,
+				     EFX_MAX_RX_QUEUES);
 	if (!net_dev)
 		return -ENOMEM;
 	net_dev->features |= (type->offload_features | NETIF_F_SG |
drivers/net/sfc/efx.h +1 -0
index 1162070..0cb198a 100644
@@ -37,6 +37,7 @@ efx_hard_start_xmit(struct sk_buff *skb, struct net_device *net_dev);
 extern netdev_tx_t
 efx_enqueue_skb(struct efx_tx_queue *tx_queue, struct sk_buff *skb);
 extern void efx_xmit_done(struct efx_tx_queue *tx_queue, unsigned int index);
+extern int efx_setup_tc(struct net_device *net_dev, u8 num_tc);
 
 /* RX */
 extern int efx_probe_rx_queue(struct efx_rx_queue *rx_queue);
drivers/net/sfc/net_driver.h +23 -6
index 77b7ce4..96e22ad 100644
@@ -63,10 +63,12 @@
 /* Checksum generation is a per-queue option in hardware, so each
  * queue visible to the networking core is backed by two hardware TX
  * queues. */
-#define EFX_MAX_CORE_TX_QUEUES	EFX_MAX_CHANNELS
-#define EFX_TXQ_TYPE_OFFLOAD	1
-#define EFX_TXQ_TYPES		2
-#define EFX_MAX_TX_QUEUES	(EFX_TXQ_TYPES * EFX_MAX_CORE_TX_QUEUES)
+#define EFX_MAX_TX_TC		2
+#define EFX_MAX_CORE_TX_QUEUES	(EFX_MAX_TX_TC * EFX_MAX_CHANNELS)
+#define EFX_TXQ_TYPE_OFFLOAD	1	/* flag */
+#define EFX_TXQ_TYPE_HIGHPRI	2	/* flag */
+#define EFX_TXQ_TYPES		4
+#define EFX_MAX_TX_QUEUES	(EFX_TXQ_TYPES * EFX_MAX_CHANNELS)
 
 /**
  * struct efx_special_buffer - An Efx special buffer

@@ -140,6 +142,7 @@ struct efx_tx_buffer {
  * @buffer: The software buffer ring
  * @txd: The hardware descriptor ring
  * @ptr_mask: The size of the ring minus 1.
+ * @initialised: Has hardware queue been initialised?
  * @flushed: Used when handling queue flushing
  * @read_count: Current read pointer.
  *	This is the number of buffers that have been removed from both rings.

@@ -182,6 +185,7 @@ struct efx_tx_queue {
 	struct efx_tx_buffer *buffer;
 	struct efx_special_buffer txd;
 	unsigned int ptr_mask;
+	bool initialised;
 	enum efx_flush_state flushed;
 
 	/* Members used mainly on the completion path */

@@ -377,7 +381,7 @@ struct efx_channel {
 	bool rx_pkt_csummed;
 
 	struct efx_rx_queue rx_queue;
-	struct efx_tx_queue tx_queue[2];
+	struct efx_tx_queue tx_queue[EFX_TXQ_TYPES];
 };
 
 enum efx_led_mode {

@@ -952,15 +956,28 @@ efx_channel_get_tx_queue(struct efx_channel *channel, unsigned type)
 	return &channel->tx_queue[type];
 }
 
+static inline bool efx_tx_queue_used(struct efx_tx_queue *tx_queue)
+{
+	return !(tx_queue->efx->net_dev->num_tc < 2 &&
+		 tx_queue->queue & EFX_TXQ_TYPE_HIGHPRI);
+}
+
 /* Iterate over all TX queues belonging to a channel */
 #define efx_for_each_channel_tx_queue(_tx_queue, _channel)		\
 	if (!efx_channel_has_tx_queues(_channel))			\
 		;							\
 	else								\
 		for (_tx_queue = (_channel)->tx_queue;			\
-		     _tx_queue < (_channel)->tx_queue + EFX_TXQ_TYPES;	\
+		     _tx_queue < (_channel)->tx_queue + EFX_TXQ_TYPES && \
+			     efx_tx_queue_used(_tx_queue);		\
 		     _tx_queue++)
 
+/* Iterate over all possible TX queues belonging to a channel */
+#define efx_for_each_possible_channel_tx_queue(_tx_queue, _channel)	\
+	for (_tx_queue = (_channel)->tx_queue;				\
+	     _tx_queue < (_channel)->tx_queue + EFX_TXQ_TYPES;		\
+	     _tx_queue++)
+
 static inline struct efx_rx_queue *
 efx_get_rx_queue(struct efx_nic *efx, unsigned index)
 {
drivers/net/sfc/nic.c +38 -13

106 lines not on this page · raw message

drivers/net/sfc/regs.h +6 -0
index 96430ed..8227de6 100644
@@ -2907,6 +2907,12 @@
 #define FRF_CZ_TMFT_SRC_MAC_HI_LBN 44
 #define FRF_CZ_TMFT_SRC_MAC_HI_WIDTH 16
 
+/* TX_PACE_TBL */
+/* Values >20 are documented as reserved, but will result in a queue going
+ * into the fast bin with a pace value of zero. */
+#define FFE_BZ_TX_PACE_OFF 0
+#define FFE_BZ_TX_PACE_RESERVED 21
+
 /* DRIVER_EV */
 /* Sub-fields of an RX flush completion event */
 #define FSF_AZ_DRIVER_EV_RX_FLUSH_FAIL_LBN 12
drivers/net/sfc/selftest.c +1 -1
index 0ebfb99..f936892 100644
@@ -644,7 +644,7 @@ static int efx_test_loopbacks(struct efx_nic *efx, struct efx_self_tests *tests,
 			goto out;
 		}
 
-		/* Test both types of TX queue */
+		/* Test all enabled types of TX queue */
 		efx_for_each_channel_tx_queue(tx_queue, channel) {
 			state->offload_csum = (tx_queue->queue &
 					       EFX_TXQ_TYPE_OFFLOAD);
drivers/net/sfc/tx.c +82 -5

125 lines not on this page · raw message

Signature · 7 lines
1.7.3.4


-- 
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.
maintainer
#6 lore raw
In reply to Ben Hutchings

From: Ben Hutchings <[email protected]>
Date: Tue, 15 Feb 2011 20:12:13 +0000

Quoted 10 lines

The following changes since commit 263fb5b1bf9265d0e4ce59ff6ea92f478b5b61ea:

Merge branch 'master' of master.kernel.org:/pub/scm/linux/kernel/git/davem/net-2.6 (2011-02-08 17:19:01 -0800)

are available in the git repository at:

git://git.kernel.org/pub/scm/linux/kernel/git/bwh/sfc-next-2.6.git for-davem

Multiqueue TX priority support for sfc, and some necessary fixes elsewhere.

Looks good, pulled, thanks Ben.

#7 lore raw

PATCH 1/1 sfc: lower stack usage in efx_ethtool_self_test

In reply to Ben Hutchings

drivers/net/sfc/ethtool.c: In function ‘efx_ethtool_self_test’:
drivers/net/sfc/ethtool.c:613: warning: the frame size of 1200 bytes is larger than 1024 bytes

Signed-off-by Eric Dumazet <[email protected]>
---
 drivers/net/sfc/ethtool.c |   22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)
drivers/net/sfc/ethtool.c +13 -9
index 272cfe7..0836b37 100644
@@ -569,9 +569,14 @@ static void efx_ethtool_self_test(struct net_device *net_dev,
 				  struct ethtool_test *test, u64 *data)
 {
 	struct efx_nic *efx = netdev_priv(net_dev);
-	struct efx_self_tests efx_tests;
+	struct efx_self_tests *efx_tests;
 	int already_up;
-	int rc;
+	int rc = -ENOMEM;
+
+	efx_tests = kzalloc(sizeof(*efx_tests), GFP_KERNEL);
+	if (!efx_tests)
+		goto fail;
+
 
 	ASSERT_RTNL();
 	if (efx->state != STATE_RUNNING) {

@@ -589,13 +594,11 @@ static void efx_ethtool_self_test(struct net_device *net_dev,
 		if (rc) {
 			netif_err(efx, drv, efx->net_dev,
 				  "failed opening device.\n");
-			goto fail2;
+			goto fail1;
 		}
 	}
 
-	memset(&efx_tests, 0, sizeof(efx_tests));
-
-	rc = efx_selftest(efx, &efx_tests, test->flags);
+	rc = efx_selftest(efx, efx_tests, test->flags);
 
 	if (!already_up)
 		dev_close(efx->net_dev);

@@ -604,10 +607,11 @@ static void efx_ethtool_self_test(struct net_device *net_dev,
 		   rc == 0 ? "passed" : "failed",
 		   (test->flags & ETH_TEST_FL_OFFLINE) ? "off" : "on");
 
- fail2:
- fail1:
+fail1:
 	/* Fill ethtool results structures */
-	efx_ethtool_fill_self_tests(efx, &efx_tests, NULL, data);
+	efx_ethtool_fill_self_tests(efx, efx_tests, NULL, data);
+	kfree(efx_tests);
+fail:
 	if (rc)
 		test->flags |= ETH_TEST_FL_FAILED;
 }
author Acked-by on v1 1/1
#8 lore raw

Re: [PATCH] sfc: lower stack usage in efx_ethtool_self_test

Quoted 3 lines · Eric Dumazet

drivers/net/sfc/ethtool.c: In function ‘efx_ethtool_self_test’:
drivers/net/sfc/ethtool.c:613: warning: the frame size of 1200 bytes is larger than 1024 bytes

This also fixes a whopping information leak in case the device is in a broken state.

Quoted 1 line
Signed-off-by Eric Dumazet <[email protected]>
Acked-by Ben Hutchings <[email protected]>

Ben.

Signature · 3 lines
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.
maintainer
#9 lore raw

Re: [PATCH] sfc: lower stack usage in efx_ethtool_self_test

From: Ben Hutchings <[email protected]>
Date: Mon, 21 Feb 2011 16:09:21 +0000

Quoted 10 lines · Eric Dumazet
Quoted 3 lines

drivers/net/sfc/ethtool.c: In function ‘efx_ethtool_self_test’:
drivers/net/sfc/ethtool.c:613: warning: the frame size of 1200 bytes is larger than 1024 bytes

This also fixes a whopping information leak in case the device is in a broken state.

Quoted 1 line
Signed-off-by Eric Dumazet <[email protected]>
Acked-by Ben Hutchings <[email protected]>

Ok, I'll apply this to net-2.6, thanks!