* Re: [PATCH 3/3] net: allocate tx queues in register_netdevice
From: Ben Hutchings @ 2010-10-18 21:41 UTC (permalink / raw)
To: Tom Herbert; +Cc: davem, netdev, eric.dumazet, John Fastabend
In-Reply-To: <alpine.DEB.1.00.1010181057580.16304@pokey.mtv.corp.google.com>
On Mon, 2010-10-18 at 11:02 -0700, Tom Herbert wrote:
> This patch introduces netif_alloc_netdev_queues which is called from
> register_device instead of alloc_netdev_mq. This makes TX queue
> allocation symmetric with RX allocation similarly allows drivers to
> change dev->num_tx_queues after allocating netdev and before
> registering it.
Changing num_tx_queues is probably *not* desirable, same as for
num_rx_queues.
[...]
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -1553,18 +1553,24 @@ static void dev_queue_xmit_nit(struct sk_buff *skb, struct net_device *dev)
> * Routine to help set real_num_tx_queues. To avoid skbs mapped to queues
> * greater then real_num_tx_queues stale skbs on the qdisc must be flushed.
> */
> -void netif_set_real_num_tx_queues(struct net_device *dev, unsigned int txq)
> +int netif_set_real_num_tx_queues(struct net_device *dev, unsigned int txq)
> {
> - unsigned int real_num = dev->real_num_tx_queues;
> + if (txq < 1)
> + return -EINVAL;
>
> - if (unlikely(txq > dev->num_tx_queues))
> - ;
> - else if (txq > real_num)
> - dev->real_num_tx_queues = txq;
> - else if (txq < real_num) {
> - dev->real_num_tx_queues = txq;
> - qdisc_reset_all_tx_gt(dev, txq);
> - }
> + if (dev->reg_state == NETREG_REGISTERED) {
> + ASSERT_RTNL();
> +
> + if (txq > dev->num_tx_queues)
> + return -EINVAL;
> +
> + if (txq < dev->real_num_tx_queues)
> + qdisc_reset_all_tx_gt(dev, txq);
> + } else
> + dev->num_tx_queues = txq;
[...]
The kernel-doc comment should be updated to reflect the locking
requirement and the possibility of failure when called after
registration.
Ben.
--
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
* Re: [PATCH net-next 3/5] tipc: Optimizations to bearer enabling logic
From: Paul Gortmaker @ 2010-10-18 21:43 UTC (permalink / raw)
To: Neil Horman; +Cc: davem, netdev, allan.stephens
In-Reply-To: <20101018105017.GA16326@hmsreliant.think-freely.org>
[Re: [PATCH net-next 3/5] tipc: Optimizations to bearer enabling logic] On 18/10/2010 (Mon 06:50) Neil Horman wrote:
> On Fri, Oct 15, 2010 at 05:31:16PM -0400, Paul Gortmaker wrote:
> > On 10-10-15 07:00 AM, Neil Horman wrote:
> >
> > [...]
> >
> > > This definately looks more concise, but I don't see why its necessecary to drop
> > > the tipc_net_lock around the call to enable_bearer. The only caler of
> > > tipc_register_media sets the enable_bearer pointer to the enable_bearer
> > > function, and I' don't see any path through that function which would
> > > potentially retake that lock. In fact it seems dropping it might be dangerous,
> > > if other paths (like from cfg_named_msg_event), tried to enable a bearer in
> > > parallel with a user space directive from the netlink socket). With out the
> > > protection of tipc_net_lock, you could use the same eth_bearer twice, and
> > > corrupt that array.
> >
> > I think it would be protected by config_lock. but in the end if it is
> > just an academic optimization that really isn't in a hot code path
> > anyway, and if it just adds more confusion than real world value, then
> > I'm fine with just dropping this on the floor (and deleting the extra
> > memset in a cleanup patch) -- it won't be the 1st time doing this with
> > these carry forward patches, and I'm sure it won't be the last.
> >
>
> Copy that.
And here is all that is left once I drop all the above. Not much. :)
Thanks again,
Paul.
>From 35b078621c4ca6e6f5a5aed80c34594e00f08c8e Mon Sep 17 00:00:00 2001
From: Allan Stephens <allan.stephens@windriver.com>
Date: Thu, 14 Oct 2010 16:09:23 -0400
Subject: [PATCH] tipc: delete needless memset from bearer enabling.
Eliminates zeroing out of the new bearer structure at the start of
activation, since it is already in that state.
Signed-off-by: Allan Stephens <allan.stephens@windriver.com>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
net/tipc/bearer.c | 2 --
1 files changed, 0 insertions(+), 2 deletions(-)
diff --git a/net/tipc/bearer.c b/net/tipc/bearer.c
index fd9c06c..9927d1d 100644
--- a/net/tipc/bearer.c
+++ b/net/tipc/bearer.c
@@ -556,8 +556,6 @@ restart:
}
b_ptr = &tipc_bearers[bearer_id];
- memset(b_ptr, 0, sizeof(struct bearer));
-
strcpy(b_ptr->publ.name, name);
res = m_ptr->enable_bearer(&b_ptr->publ);
if (res) {
--
1.7.2.1
^ permalink raw reply related
* [patch 1/1] phy/marvell: fix 88e1121 support
From: Arnaud Patard @ 2010-10-18 21:44 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller, Cyril Chemparathy
In-Reply-To: <20101018214401.053486146@rtp-net.org>
[-- Attachment #1: phy_marvell_fix_modes.patch --]
[-- Type: text/plain, Size: 1278 bytes --]
Commit c477d0447db08068a497e7beb892b2b2a7bff64b added support for RGMII
rx/tx delays except that it ends up clearing rx/tx delays bit for modes
differents that RGMII*ID. Due to this, ethernet is not working anymore
on my guruplug server +. This patch is fixing that.
Signed-off-by: Arnaud Patard <arnaud.patard@rtp-net.org>
Index: linux-2.6/drivers/net/phy/marvell.c
===================================================================
--- linux-2.6.orig/drivers/net/phy/marvell.c 2010-10-18 22:46:09.000000000 +0200
+++ linux-2.6/drivers/net/phy/marvell.c 2010-10-18 23:19:14.000000000 +0200
@@ -199,13 +199,13 @@
mscr = phy_read(phydev, MII_88E1121_PHY_MSCR_REG) &
MII_88E1121_PHY_MSCR_DELAY_MASK;
- if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID)
- mscr |= (MII_88E1121_PHY_MSCR_RX_DELAY |
- MII_88E1121_PHY_MSCR_TX_DELAY);
- else if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID)
+ if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID)
mscr |= MII_88E1121_PHY_MSCR_RX_DELAY;
else if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID)
mscr |= MII_88E1121_PHY_MSCR_TX_DELAY;
+ else
+ mscr |= (MII_88E1121_PHY_MSCR_RX_DELAY |
+ MII_88E1121_PHY_MSCR_TX_DELAY);
err = phy_write(phydev, MII_88E1121_PHY_MSCR_REG, mscr);
if (err < 0)
^ permalink raw reply
* Re: [PATCH 1/3] net: fail alloc_netdev_mq if queue count < 1
From: Tom Herbert @ 2010-10-18 21:46 UTC (permalink / raw)
To: Ben Hutchings; +Cc: davem, netdev, eric.dumazet
In-Reply-To: <1287437621.2252.766.camel@achroite.uk.solarflarecom.com>
> Off by one?
>
Yes, thanks for catching that.
> Ben.
>
>> + return NULL;
>> + }
>> +
>> alloc_size = sizeof(struct net_device);
>> if (sizeof_priv) {
>> /* ensure 32-byte alignment of private area */
>
> --
> 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
* Re: Bug#595265: linux-image-2.6.32-5-686: Nerwork card fails to come up again after suspend
From: Francois Romieu @ 2010-10-18 21:45 UTC (permalink / raw)
To: Ben Hutchings; +Cc: Arnout Boelens, 595265, netdev
In-Reply-To: <1287361431.20865.133.camel@localhost>
Ben Hutchings <ben@decadent.org.uk> :
[...]
> Arnout Boelens reported that his RTL8111/8168B fails to link-up after
> suspend and resume, both under Debian's kernel based on 2.6.32 and under
> 2.6.36-rc6. Full details are at <http://bugs.debian.org/595265>, though
> the log isn't very informative:
>
> [31837.396594] PM: Finishing wakeup.
> [31837.396597] Restarting tasks ... done.
> [31840.267267] r8169: eth0: link down
>
> Can you suggest how to investigate this further?
Something like the patch made by Stanislaw at :
https://bugzilla.redhat.com/show_bug.cgi?id=502974
--
Ueimor
^ permalink raw reply
* Re: [patch 1/1] phy/marvell: fix 88e1121 support
From: Cyril Chemparathy @ 2010-10-18 22:02 UTC (permalink / raw)
To: Arnaud Patard; +Cc: netdev@vger.kernel.org, David S. Miller
In-Reply-To: <20101018214414.375368957@rtp-net.org>
Hi,
On 10/18/2010 05:44 PM, Arnaud Patard wrote:
> Commit c477d0447db08068a497e7beb892b2b2a7bff64b added support for RGMII
> rx/tx delays except that it ends up clearing rx/tx delays bit for modes
> differents that RGMII*ID. Due to this, ethernet is not working anymore
> on my guruplug server +. This patch is fixing that.
>
> Signed-off-by: Arnaud Patard <arnaud.patard@rtp-net.org>
> Index: linux-2.6/drivers/net/phy/marvell.c
> ===================================================================
> --- linux-2.6.orig/drivers/net/phy/marvell.c 2010-10-18 22:46:09.000000000 +0200
> +++ linux-2.6/drivers/net/phy/marvell.c 2010-10-18 23:19:14.000000000 +0200
> @@ -199,13 +199,13 @@
> mscr = phy_read(phydev, MII_88E1121_PHY_MSCR_REG) &
> MII_88E1121_PHY_MSCR_DELAY_MASK;
>
> - if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID)
> - mscr |= (MII_88E1121_PHY_MSCR_RX_DELAY |
> - MII_88E1121_PHY_MSCR_TX_DELAY);
> - else if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID)
> + if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID)
> mscr |= MII_88E1121_PHY_MSCR_RX_DELAY;
> else if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID)
> mscr |= MII_88E1121_PHY_MSCR_TX_DELAY;
> + else
> + mscr |= (MII_88E1121_PHY_MSCR_RX_DELAY |
> + MII_88E1121_PHY_MSCR_TX_DELAY);
>
> err = phy_write(phydev, MII_88E1121_PHY_MSCR_REG, mscr);
> if (err < 0)
This doesn't look right. This change makes it impossible to configure
the phy without interface delays, i.e., PHY_INTERFACE_MODE_RGMII ends up
behaving the same as PHY_INTERFACE_MODE_RGMII_ID.
I think the correct fix would be to modify guruplug's interface mode to
PHY_INTERFACE_MODE_RGMII_ID if both TX and RX side delays are desired.
Regards
Cyril.
^ permalink raw reply
* [PATCH] pktgen: clean up handling of local/transient counter vars
From: Paul Gortmaker @ 2010-10-18 22:14 UTC (permalink / raw)
To: davem; +Cc: netdev
The temporary variable "i" is needlessly initialized to zero
in two distinct cases in this file:
1) where it is set to zero and then used as an argument in an addition
before being assigned a non-zero value.
2) where it is only used in a standard/typical loop counter
For (1), simply delete assignment to zero and usages while still
zero; for (2) simply make the loop start at zero as per standard
practice as seen everywhere else in the same file.
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
net/core/pktgen.c | 30 +++++++++++++++---------------
1 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 2c0df0f..679b797 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -771,10 +771,10 @@ done:
static unsigned long num_arg(const char __user * user_buffer,
unsigned long maxlen, unsigned long *num)
{
- int i = 0;
+ int i;
*num = 0;
- for (; i < maxlen; i++) {
+ for (i = 0; i < maxlen; i++) {
char c;
if (get_user(c, &user_buffer[i]))
return -EFAULT;
@@ -789,9 +789,9 @@ static unsigned long num_arg(const char __user * user_buffer,
static int strn_len(const char __user * user_buffer, unsigned int maxlen)
{
- int i = 0;
+ int i;
- for (; i < maxlen; i++) {
+ for (i = 0; i < maxlen; i++) {
char c;
if (get_user(c, &user_buffer[i]))
return -EFAULT;
@@ -846,7 +846,7 @@ static ssize_t pktgen_if_write(struct file *file,
{
struct seq_file *seq = file->private_data;
struct pktgen_dev *pkt_dev = seq->private;
- int i = 0, max, len;
+ int i, max, len;
char name[16], valstr[32];
unsigned long value = 0;
char *pg_result = NULL;
@@ -860,13 +860,13 @@ static ssize_t pktgen_if_write(struct file *file,
return -EINVAL;
}
- max = count - i;
- tmp = count_trail_chars(&user_buffer[i], max);
+ max = count;
+ tmp = count_trail_chars(user_buffer, max);
if (tmp < 0) {
pr_warning("illegal format\n");
return tmp;
}
- i += tmp;
+ i = tmp;
/* Read variable name */
@@ -1764,7 +1764,7 @@ static ssize_t pktgen_thread_write(struct file *file,
{
struct seq_file *seq = file->private_data;
struct pktgen_thread *t = seq->private;
- int i = 0, max, len, ret;
+ int i, max, len, ret;
char name[40];
char *pg_result;
@@ -1773,12 +1773,12 @@ static ssize_t pktgen_thread_write(struct file *file,
return -EINVAL;
}
- max = count - i;
- len = count_trail_chars(&user_buffer[i], max);
+ max = count;
+ len = count_trail_chars(user_buffer, max);
if (len < 0)
return len;
- i += len;
+ i = len;
/* Read variable name */
@@ -1975,7 +1975,7 @@ static struct net_device *pktgen_dev_get_by_name(struct pktgen_dev *pkt_dev,
const char *ifname)
{
char b[IFNAMSIZ+5];
- int i = 0;
+ int i;
for (i = 0; ifname[i] != '@'; i++) {
if (i == IFNAMSIZ)
@@ -2519,8 +2519,8 @@ static void free_SAs(struct pktgen_dev *pkt_dev)
{
if (pkt_dev->cflows) {
/* let go of the SAs if we have them */
- int i = 0;
- for (; i < pkt_dev->cflows; i++) {
+ int i;
+ for (i = 0; i < pkt_dev->cflows; i++) {
struct xfrm_state *x = pkt_dev->flows[i].x;
if (x) {
xfrm_state_put(x);
--
1.7.2.1
^ permalink raw reply related
* Re: Linux 2.6.35/TIPC 2.0 ABI breaking changes
From: David Miller @ 2010-10-18 22:17 UTC (permalink / raw)
To: paul.gortmaker; +Cc: luca, jon.maloy, tipc-discussion, linux-kernel, netdev
In-Reply-To: <AANLkTimKbOreV4Fuyw3TbCrk9aa5yBZQSuRbo3Vdx=Q1@mail.gmail.com>
From: Paul Gortmaker <paul.gortmaker@windriver.com>
Date: Mon, 18 Oct 2010 16:42:33 -0400
> If you have access to the user space code in question, you can just
> switch behaviour semantics based on the results of a uname call, knowing
> that this change was included in versions since approx last Feb. There
> is also /proc/version which can be parsed manually if you prefer.
Requiring userspace to check kernel versioning information in order
to user an exported userspace API correctly is _ALWAYS_ _WRONG_.
You cannot and must not make backwards incompatible changes to
userspace interfaces.
Really, you can't.
^ permalink raw reply
* [patch 0/1] Re: [patch 1/1] phy/marvell: fix 88e1121 support
From: Arnaud Patard @ 2010-10-18 22:29 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller, Cyril Chemparathy
In-Reply-To: <4CBCC3FC.2030506@ti.com>
>> @@ -199,13 +199,13 @@
>> mscr = phy_read(phydev, MII_88E1121_PHY_MSCR_REG) &
>> MII_88E1121_PHY_MSCR_DELAY_MASK;
>>
>> - if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID)
>> - mscr |= (MII_88E1121_PHY_MSCR_RX_DELAY |
>> - MII_88E1121_PHY_MSCR_TX_DELAY);
>> - else if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID)
>> + if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID)
>> mscr |= MII_88E1121_PHY_MSCR_RX_DELAY;
>> else if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID)
>> mscr |= MII_88E1121_PHY_MSCR_TX_DELAY;
>> + else
>> + mscr |= (MII_88E1121_PHY_MSCR_RX_DELAY |
>> + MII_88E1121_PHY_MSCR_TX_DELAY);
>>
>> err = phy_write(phydev, MII_88E1121_PHY_MSCR_REG, mscr);
>> if (err < 0)
>
> This doesn't look right. This change makes it impossible to configure
> the phy without interface delays, i.e., PHY_INTERFACE_MODE_RGMII ends up
> behaving the same as PHY_INTERFACE_MODE_RGMII_ID.
>
ok. The other possibility is to modify the MII_88E1121_PHY_MSCR_REG regs
only in RGMII_* case like for the 88e1111 case imho.
I'm sending a patch doing that right now.
Regards,
Arnaud
^ permalink raw reply
* [patch 1/1] phy/marvell: fix 88e1121 support
From: Arnaud Patard @ 2010-10-18 22:29 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller, Cyril Chemparathy, Arnaud Patard
In-Reply-To: <20101018222947.418028323@rtp-net.org>
[-- Attachment #1: phy_marvell_fix_modes.patch --]
[-- Type: text/plain, Size: 2052 bytes --]
Commit c477d0447db08068a497e7beb892b2b2a7bff64b added support for RGMII
rx/tx delays except that it ends up clearing rx/tx delays bit for modes
differents that RGMII*ID. Due to this, ethernet is not working anymore
on my guruplug server +. This patch is fixing that.
Signed-off-by: Arnaud Patard <arnaud.patard@rtp-net.org>
Index: linux-2.6/drivers/net/phy/marvell.c
===================================================================
--- linux-2.6.orig/drivers/net/phy/marvell.c 2010-10-18 22:46:09.000000000 +0200
+++ linux-2.6/drivers/net/phy/marvell.c 2010-10-19 00:20:22.000000000 +0200
@@ -196,20 +196,27 @@
MII_88E1121_PHY_MSCR_PAGE);
if (err < 0)
return err;
- mscr = phy_read(phydev, MII_88E1121_PHY_MSCR_REG) &
- MII_88E1121_PHY_MSCR_DELAY_MASK;
- if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID)
- mscr |= (MII_88E1121_PHY_MSCR_RX_DELAY |
- MII_88E1121_PHY_MSCR_TX_DELAY);
- else if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID)
- mscr |= MII_88E1121_PHY_MSCR_RX_DELAY;
- else if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID)
- mscr |= MII_88E1121_PHY_MSCR_TX_DELAY;
+ if ((phydev->interface == PHY_INTERFACE_MODE_RGMII) ||
+ (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) ||
+ (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID) ||
+ (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID)) {
- err = phy_write(phydev, MII_88E1121_PHY_MSCR_REG, mscr);
- if (err < 0)
- return err;
+ mscr = phy_read(phydev, MII_88E1121_PHY_MSCR_REG) &
+ MII_88E1121_PHY_MSCR_DELAY_MASK;
+
+ if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID)
+ mscr |= (MII_88E1121_PHY_MSCR_RX_DELAY |
+ MII_88E1121_PHY_MSCR_TX_DELAY);
+ else if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID)
+ mscr |= MII_88E1121_PHY_MSCR_RX_DELAY;
+ else if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID)
+ mscr |= MII_88E1121_PHY_MSCR_TX_DELAY;
+
+ err = phy_write(phydev, MII_88E1121_PHY_MSCR_REG, mscr);
+ if (err < 0)
+ return err;
+ }
phy_write(phydev, MII_88E1121_PHY_PAGE, oldpage);
^ permalink raw reply
* Re: Bug#595265: linux-image-2.6.32-5-686: Nerwork card fails to come up again after suspend
From: Ben Hutchings @ 2010-10-18 22:48 UTC (permalink / raw)
To: Arnout Boelens, Francois Romieu; +Cc: 595265, netdev
In-Reply-To: <20101018214536.GA14344@electric-eye.fr.zoreil.com>
[-- Attachment #1: Type: text/plain, Size: 1000 bytes --]
On Mon, 2010-10-18 at 23:45 +0200, Francois Romieu wrote:
> Ben Hutchings <ben@decadent.org.uk> :
> [...]
> > Arnout Boelens reported that his RTL8111/8168B fails to link-up after
> > suspend and resume, both under Debian's kernel based on 2.6.32 and under
> > 2.6.36-rc6. Full details are at <http://bugs.debian.org/595265>, though
> > the log isn't very informative:
> >
> > [31837.396594] PM: Finishing wakeup.
> > [31837.396597] Restarting tasks ... done.
> > [31840.267267] r8169: eth0: link down
> >
> > Can you suggest how to investigate this further?
>
> Something like the patch made by Stanislaw at :
> https://bugzilla.redhat.com/show_bug.cgi?id=502974
Thanks.
Arnout, please test the patch
<https://bugzilla.redhat.com/attachment.cgi?id=454034>, following the
instructions at
<http://kernel-handbook.alioth.debian.org/ch-common-tasks.html#s-common-official>.
Ben.
--
Ben Hutchings
Once a job is fouled up, anything done to improve it makes it worse.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]
^ permalink raw reply
* Re: [PATCH] secmark: do not return early if there was no error
From: James Morris @ 2010-10-18 22:48 UTC (permalink / raw)
To: Patrick McHardy
Cc: Eric Paris, netfilter-devel, netfilter, coreteam, netdev, davem,
jengelh, paul.moore
In-Reply-To: <4CB86E6A.1040807@trash.net>
On Fri, 15 Oct 2010, Patrick McHardy wrote:
> > ...
> > RIP [<ffffffffa022117d>] secmark_tg+0x17/0x2e [xt_SECMARK]
> > RSP <ffff880003e03a40>
> > ---[ end trace 9aa5d06a71143e74 ]---
> >
> > Signed-off-by: Eric Paris <eparis@redhat.com>
> > Acked-by: Paul Moore <paul.moore@hp.com>
> > Acked-by: James Morris <jmorris@namei.org>
>
> Acked-by: Patrick McHardy <kaber@trash.net>
>
> I'll leave it up to Dave whether this can still go into 2.6.36.
FYI, I have a copy now in my #next branch, as it's a pre-requisite for
further patches.
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Re: Bug#595265: linux-image-2.6.32-5-686: Nerwork card fails to come up again after suspend
From: Eric Dumazet @ 2010-10-18 22:50 UTC (permalink / raw)
To: Francois Romieu; +Cc: Ben Hutchings, Arnout Boelens, 595265, netdev
In-Reply-To: <20101018214536.GA14344@electric-eye.fr.zoreil.com>
Le lundi 18 octobre 2010 à 23:45 +0200, Francois Romieu a écrit :
> Ben Hutchings <ben@decadent.org.uk> :
> [...]
> > Arnout Boelens reported that his RTL8111/8168B fails to link-up after
> > suspend and resume, both under Debian's kernel based on 2.6.32 and under
> > 2.6.36-rc6. Full details are at <http://bugs.debian.org/595265>, though
> > the log isn't very informative:
> >
> > [31837.396594] PM: Finishing wakeup.
> > [31837.396597] Restarting tasks ... done.
> > [31840.267267] r8169: eth0: link down
> >
> > Can you suggest how to investigate this further?
>
> Something like the patch made by Stanislaw at :
> https://bugzilla.redhat.com/show_bug.cgi?id=502974
>
Seems to be down at this moment, patch is also here (and included in
2.6.36-rc8 ) :
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=aeb19f6052b5e5c8a24aa444fbff73b84341beac
^ permalink raw reply
* Re: [Ksummit-2010-discuss] [v2] Remaining BKL users, what to do
From: Dave Airlie @ 2010-10-18 23:00 UTC (permalink / raw)
To: Greg KH
Cc: Arnd Bergmann, codalist, ksummit-2010-discuss, autofs, Jan Harkes,
Samuel Ortiz, Jan Kara, Arnaldo Carvalho de Melo, netdev,
Anders Larsen, linux-kernel, dri-devel, Bryan Schumaker,
Christoph Hellwig, Petr Vandrovec, Mikulas Patocka, linux-fsdevel,
Evgeniy Dushistov, Ingo Molnar, Andrew Hendry, linux-media
In-Reply-To: <20101018184346.GD27089@kroah.com>
On Tue, Oct 19, 2010 at 4:43 AM, Greg KH <greg@kroah.com> wrote:
> On Mon, Oct 18, 2010 at 05:42:06PM +0200, Arnd Bergmann wrote:
>>
>> Out of the remaining modules, I guess i810/i830, adfs, hpfs and ufs might end
>> up not getting fixed at all, we can either mark them non-SMP or move them
>> to drivers/staging once all the others are done.
>
> I recommend moving them to staging, and then retire them from there if
> no one steps up to maintain them.
I think this sets a bad precedent, these drivers work fine. Removing
BKL from them is hard, and involves finding and booting hw that
developers don't have much time/interest in at the moment. Anyone who
has access to the i810 hw and has time to work out the locking has
more important things to be doing with modern hw, however it doesn't
mean we should just drop support for old drivers because they don't
have active maintainers. Removing the BKL from the kernel is a great
goal, but breaking userspace ABI by removing drivers isn't.
Dave.
^ permalink raw reply
* Re: Linux 2.6.35/TIPC 2.0 ABI breaking changes
From: Paul Gortmaker @ 2010-10-18 23:11 UTC (permalink / raw)
To: David Miller
Cc: luca, jon.maloy, tipc-discussion, linux-kernel, netdev,
Neil Horman
In-Reply-To: <20101018.151708.193712688.davem@davemloft.net>
On 10-10-18 06:17 PM, David Miller wrote:
> From: Paul Gortmaker<paul.gortmaker@windriver.com>
> Date: Mon, 18 Oct 2010 16:42:33 -0400
>
>> If you have access to the user space code in question, you can just
>> switch behaviour semantics based on the results of a uname call, knowing
>> that this change was included in versions since approx last Feb. There
>> is also /proc/version which can be parsed manually if you prefer.
>
> Requiring userspace to check kernel versioning information in order
> to user an exported userspace API correctly is _ALWAYS_ _WRONG_.
>
> You cannot and must not make backwards incompatible changes to
> userspace interfaces.
What I think has happened here (and I'll double check this
tomorrow, since it is before I started assisting with tipc)
is that a backwards incompatible change *did* inadvertently
creep in via these two (related) commits:
--------------
commit d88dca79d3852a3623f606f781e013d61486828a
Author: Neil Horman <nhorman@tuxdriver.com>
Date: Mon Mar 8 12:20:58 2010 -0800
tipc: fix endianness on tipc subscriber messages
--------------
and
---------------
commit c6537d6742985da1fbf12ae26cde6a096fd35b5c
Author: Jon Paul Maloy <jon.maloy@ericsson.com>
Date: Tue Apr 6 11:40:52 2010 +0000
TIPC: Updated topology subscription protocol according to latest spec
---------------
Based on Leandro's info, I think it comes down to userspace
not knowing exactly where to find these bits anymore:
#define TIPC_SUB_SERVICE 0x00 /* Filter for service availability */
#define TIPC_SUB_PORTS 0x01 /* Filter for port availability */
#define TIPC_SUB_CANCEL 0x04 /* Cancel a subscription */
...because it doesn't know if there is the old auto endian
swap thing being done or not being done.
Assuming it is possible to do so in some non-kludgy way,
it sounds like we want to be looking into an in-kernel change
that ensures the older user space binaries get their
functionality restored then?
Thanks,
Paul.
^ permalink raw reply
* Re: [PATCH 2/3] cxgb4: function namespace cleanup (v3)
From: Dimitris Michailidis @ 2010-10-18 23:16 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David Miller, swise, divy, leedom, netdev
In-Reply-To: <20101018083918.3777a278@nehalam>
Stephen Hemminger wrote:
> Make functions only used in one file local.
> Remove lots of dead code, relating to unsupported functions
> in mainline driver like RSS, IPv6, and TCP offload.
>
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
Acked-by: Dimitris Michailidis <dm@chelsio.com>
Thanks much.
^ permalink raw reply
* [PATCH] smsc95xx: generate random MAC address once, not every ifup
From: Bernard Blackham @ 2010-10-18 23:16 UTC (permalink / raw)
To: Steve Glendinning, netdev; +Cc: linux-omap
The smsc95xx driver currently generates a new random MAC address
every time the interface is brought up. This makes it impossible to
override using the standard `ifconfig hw ether` approach.
Past patches tried to make the MAC address a module parameter or
base it off the die ID, but it seems to me much simpler (and
hopefully less controversial) to stick with the current random
generation scheme, but allow the user to change the address.
This patch does exactly that - it moves the random address
generation from smsc95xx_reset() into smsc95xx_bind(), so that it is
done once on module load, not on every ifup. The user can then
override this using the standard mechanisms.
Applies against 2.6.35 and linux-2.6 head.
Signed-off-by: Bernard Blackham <b-omap@largestprime.net>
---
--- a/drivers/net/usb/smsc95xx.c 2010-10-19 00:15:35.915612223 +1100
+++ b/drivers/net/usb/smsc95xx.c 2010-10-19 00:15:55.408550929 +1100
@@ -805,8 +805,6 @@ static int smsc95xx_reset(struct usbnet
return ret;
}
- smsc95xx_init_mac_address(dev);
-
ret = smsc95xx_set_mac_address(dev);
if (ret < 0)
return ret;
@@ -1047,6 +1045,8 @@ static int smsc95xx_bind(struct usbnet *
pdata->use_tx_csum = DEFAULT_TX_CSUM_ENABLE;
pdata->use_rx_csum = DEFAULT_RX_CSUM_ENABLE;
+ smsc95xx_init_mac_address(dev);
+
/* Init all registers */
ret = smsc95xx_reset(dev);
^ permalink raw reply
* Re: Linux 2.6.35/TIPC 2.0 ABI breaking changes
From: Leandro Lucarella @ 2010-10-18 23:38 UTC (permalink / raw)
To: Paul Gortmaker
Cc: jon.maloy, Neil Horman, netdev, linux-kernel, tipc-discussion,
David Miller
In-Reply-To: <4CBCD429.8090306@windriver.com>
Paul Gortmaker, el 18 de octubre a las 19:11 me escribiste:
> On 10-10-18 06:17 PM, David Miller wrote:
> > From: Paul Gortmaker<paul.gortmaker@windriver.com>
> > Date: Mon, 18 Oct 2010 16:42:33 -0400
> >
> >> If you have access to the user space code in question, you can just
> >> switch behaviour semantics based on the results of a uname call, knowing
> >> that this change was included in versions since approx last Feb. There
> >> is also /proc/version which can be parsed manually if you prefer.
> >
> > Requiring userspace to check kernel versioning information in order
> > to user an exported userspace API correctly is _ALWAYS_ _WRONG_.
> >
> > You cannot and must not make backwards incompatible changes to
> > userspace interfaces.
>
> What I think has happened here (and I'll double check this
> tomorrow, since it is before I started assisting with tipc)
> is that a backwards incompatible change *did* inadvertently
> creep in via these two (related) commits:
>
> --------------
> commit d88dca79d3852a3623f606f781e013d61486828a
> Author: Neil Horman <nhorman@tuxdriver.com>
> Date: Mon Mar 8 12:20:58 2010 -0800
>
> tipc: fix endianness on tipc subscriber messages
> --------------
>
> and
>
> ---------------
> commit c6537d6742985da1fbf12ae26cde6a096fd35b5c
> Author: Jon Paul Maloy <jon.maloy@ericsson.com>
> Date: Tue Apr 6 11:40:52 2010 +0000
>
> TIPC: Updated topology subscription protocol according to latest spec
> ---------------
Exactly. The damage is already done, the thing now is how to proceed.
What I'm doing right now is exactly that, use uname(2) to see what
kernel version is running and act according. Act according means, check
how with which tipc.h header the program was compiled (pre or post
2.6.35), compare against the current running kernel version, fix the BO
of subscriptions and fix the filter flags using my own constants
TIPC_SUB_SERVICE_V1 and TIPC_SUB_SERVICE_V2. Is not trivial, is not
nice.
And worse, as I said, for other languages bindings, specially those that
can't directly include a .h, there is no easy solution at all.
> Based on Leandro's info, I think it comes down to userspace
> not knowing exactly where to find these bits anymore:
>
> #define TIPC_SUB_SERVICE 0x00 /* Filter for service availability */
> #define TIPC_SUB_PORTS 0x01 /* Filter for port availability */
> #define TIPC_SUB_CANCEL 0x04 /* Cancel a subscription */
>
> ...because it doesn't know if there is the old auto endian
> swap thing being done or not being done.
There are, at least, 2 problems here. One is the auto endian swap (which
I knew it existed just because of this issue). But the auto endian swap
is not fully backward compatible either AFAIK, at least I tried Gentoo's
kernel 2.6.22 r5 and the topology services doesn't work with NBO (I
don't know if is a bug introduced by Gentoo, or a bug in the kernel or
what else). So just changing the BO and claiming backward compatibility
(even when is only at source code level) is not entirely true (unless is
a Gentoo issue of course, which I should probably check).
The other problem is TIPC_SUB_SERVICE changed it value from 2 to 0. In
TIPC 1.x it was a flag set in the filter member of a subscription. In
TIPC 2.0 is the absence of TIPC_SUB_PORTS. The change seems reasonable,
as TIPC_SUB_SERVICE and TIPC_SUB_PORTS were mutually exclusive and you
had to use one always, but the result is an API and *ABI* change. You
can't use an old TIPC application without changing the code and
recompiling using a tipc.h header from 2.6.35 or newer. And then, if you
need to reboot with an older kernel, the new compiled application won't
work with the old kernel.
A third problem, much less critical but which I find annoying, is that
an inconsistency was introduced. In kernels older than 2.6.35 (TIPC 1.6)
you used HBO for all the TIPC data structures, addresses, port names,
port sequences, port ids, subscriptions, events. Now, subscriptions and
events go in NBO (which contains port sequences and port ids), but when
binding a port name/sequence, you have to use HBO. Is true that
a subscription is supposed to be a network message and the port name you
bind to is not, but is at least inconsistent with AF_INET, where all you
do uses NBO.
> Assuming it is possible to do so in some non-kludgy way,
> it sounds like we want to be looking into an in-kernel change
> that ensures the older user space binaries get their
> functionality restored then?
I think the most reasonable way to do this was to use a different port
name for the topology service for TIPC 2.0, and keep the old TIPC 1.x
topology service at the same port name it was, and using the same
interface as it did. Adding new constants TIPC_TOP_SRV2 and
TIPC_SUB_SERVICE2 (for example) would have been enough to keep backward
compatibility and allow a new clean interface. The old topology service
could be deprecated and completely removed some time in a distant
future. You could even add some #ifdef TIPC_1_COMPATIBILITY to make sure
people using the old interface is aware of that and update to the new
one.
--
Leandro Lucarella (AKA luca) http://llucax.com.ar/
----------------------------------------------------------------------
GPG Key: 5F5A8D05 (F8CD F9A7 BF00 5431 4145 104C 949E BFB6 5F5A 8D05)
----------------------------------------------------------------------
If you do not change your beliefs
Your life will always be like this
------------------------------------------------------------------------------
Download new Adobe(R) Flash(R) Builder(TM) 4
The new Adobe(R) Flex(R) 4 and Flash(R) Builder(TM) 4 (formerly
Flex(R) Builder(TM)) enable the development of rich applications that run
across multiple browsers and platforms. Download your free trials today!
http://p.sf.net/sfu/adobe-dev2dev
^ permalink raw reply
* Re: Linux 2.6.35/TIPC 2.0 ABI breaking changes
From: Neil Horman @ 2010-10-18 23:45 UTC (permalink / raw)
To: Paul Gortmaker
Cc: jon.maloy, netdev, linux-kernel, luca, tipc-discussion,
David Miller
In-Reply-To: <4CBCD429.8090306@windriver.com>
On Mon, Oct 18, 2010 at 07:11:37PM -0400, Paul Gortmaker wrote:
> On 10-10-18 06:17 PM, David Miller wrote:
> > From: Paul Gortmaker<paul.gortmaker@windriver.com>
> > Date: Mon, 18 Oct 2010 16:42:33 -0400
> >
> >> If you have access to the user space code in question, you can just
> >> switch behaviour semantics based on the results of a uname call, knowing
> >> that this change was included in versions since approx last Feb. There
> >> is also /proc/version which can be parsed manually if you prefer.
> >
> > Requiring userspace to check kernel versioning information in order
> > to user an exported userspace API correctly is _ALWAYS_ _WRONG_.
> >
> > You cannot and must not make backwards incompatible changes to
> > userspace interfaces.
>
> What I think has happened here (and I'll double check this
> tomorrow, since it is before I started assisting with tipc)
> is that a backwards incompatible change *did* inadvertently
> creep in via these two (related) commits:
>
> --------------
> commit d88dca79d3852a3623f606f781e013d61486828a
> Author: Neil Horman <nhorman@tuxdriver.com>
> Date: Mon Mar 8 12:20:58 2010 -0800
>
> tipc: fix endianness on tipc subscriber messages
> --------------
>
> and
>
> ---------------
> commit c6537d6742985da1fbf12ae26cde6a096fd35b5c
> Author: Jon Paul Maloy <jon.maloy@ericsson.com>
> Date: Tue Apr 6 11:40:52 2010 +0000
>
> TIPC: Updated topology subscription protocol according to latest spec
> ---------------
>
> Based on Leandro's info, I think it comes down to userspace
> not knowing exactly where to find these bits anymore:
>
> #define TIPC_SUB_SERVICE 0x00 /* Filter for service availability */
> #define TIPC_SUB_PORTS 0x01 /* Filter for port availability */
> #define TIPC_SUB_CANCEL 0x04 /* Cancel a subscription */
>
That shouldn't be the case. Prior to the above changes the tipc implementation
tracked the endianess of the hosts to which it was connected and swapped data
that it sent to those hosts accordingly. With these changes the kernel client
simply swaps the data to network byte order on send and swaps it back to local
order on receive universally. That second commit added a bit from the reserved
pool of one of the connection establishment messages to indicate that a peer was
using this new protocol. If some non-local byte order information is making it
into user space, thats a bug that needs fixing.
What may be happening is some old client that doesn't know about the new bit
might be communicating with an new client that does. IIRC the spec called for
clients that set bits in the reserved field to drop frames from that client, so
that condition shouldn't occur, but TIPC may just be ignoring reserved bits. I
wouldn't be suprised.
Its also possible that the payload data between applications using tipc follow
the same broken byte swapping method that the protocol itself did, but if that
were the case I would expect the application to continue running normally,
unless user space had direct access to the protocol header in its entirety, and
read it directly, in which case I think I would just cry.
> ...because it doesn't know if there is the old auto endian
> swap thing being done or not being done.
>
> Assuming it is possible to do so in some non-kludgy way,
> it sounds like we want to be looking into an in-kernel change
> that ensures the older user space binaries get their
> functionality restored then?
>
Lets try figure out exactly what data is getting mis-read first. Maybe we can
fix it without having to go back to making a sending host figure out a receiving
hosts byte order. That would be nice. Can you describe the problem in more
detail?
Neil
> Thanks,
> Paul.
>
------------------------------------------------------------------------------
Download new Adobe(R) Flash(R) Builder(TM) 4
The new Adobe(R) Flex(R) 4 and Flash(R) Builder(TM) 4 (formerly
Flex(R) Builder(TM)) enable the development of rich applications that run
across multiple browsers and platforms. Download your free trials today!
http://p.sf.net/sfu/adobe-dev2dev
^ permalink raw reply
* Re: Linux 2.6.35/TIPC 2.0 ABI breaking changes
From: Alan Cox @ 2010-10-18 23:58 UTC (permalink / raw)
To: David Miller
Cc: paul.gortmaker, luca, jon.maloy, tipc-discussion, linux-kernel,
netdev
In-Reply-To: <20101018.151708.193712688.davem@davemloft.net>
On Mon, 18 Oct 2010 15:17:08 -0700 (PDT)
David Miller <davem@davemloft.net> wrote:
> From: Paul Gortmaker <paul.gortmaker@windriver.com>
> Date: Mon, 18 Oct 2010 16:42:33 -0400
>
> > If you have access to the user space code in question, you can just
> > switch behaviour semantics based on the results of a uname call, knowing
> > that this change was included in versions since approx last Feb. There
> > is also /proc/version which can be parsed manually if you prefer.
>
> Requiring userspace to check kernel versioning information in order
> to user an exported userspace API correctly is _ALWAYS_ _WRONG_.
>
> You cannot and must not make backwards incompatible changes to
> userspace interfaces.
>
> Really, you can't.
In which case given that most distros are shipping older stuff and care
about ABI stability presumably the bug should get fixed for 2.6.36 ?
^ permalink raw reply
* Re: [PATCH net-next 3/5] tipc: Optimizations to bearer enabling logic
From: Neil Horman @ 2010-10-18 23:59 UTC (permalink / raw)
To: Paul Gortmaker; +Cc: davem, netdev, allan.stephens
In-Reply-To: <20101018214356.GA27204@windriver.com>
On Mon, Oct 18, 2010 at 05:43:56PM -0400, Paul Gortmaker wrote:
> [Re: [PATCH net-next 3/5] tipc: Optimizations to bearer enabling logic] On 18/10/2010 (Mon 06:50) Neil Horman wrote:
>
> > On Fri, Oct 15, 2010 at 05:31:16PM -0400, Paul Gortmaker wrote:
> > > On 10-10-15 07:00 AM, Neil Horman wrote:
> > >
> > > [...]
> > >
> > > > This definately looks more concise, but I don't see why its necessecary to drop
> > > > the tipc_net_lock around the call to enable_bearer. The only caler of
> > > > tipc_register_media sets the enable_bearer pointer to the enable_bearer
> > > > function, and I' don't see any path through that function which would
> > > > potentially retake that lock. In fact it seems dropping it might be dangerous,
> > > > if other paths (like from cfg_named_msg_event), tried to enable a bearer in
> > > > parallel with a user space directive from the netlink socket). With out the
> > > > protection of tipc_net_lock, you could use the same eth_bearer twice, and
> > > > corrupt that array.
> > >
> > > I think it would be protected by config_lock. but in the end if it is
> > > just an academic optimization that really isn't in a hot code path
> > > anyway, and if it just adds more confusion than real world value, then
> > > I'm fine with just dropping this on the floor (and deleting the extra
> > > memset in a cleanup patch) -- it won't be the 1st time doing this with
> > > these carry forward patches, and I'm sure it won't be the last.
> > >
> >
> > Copy that.
>
> And here is all that is left once I drop all the above. Not much. :)
>
> Thanks again,
> Paul.
>
> From 35b078621c4ca6e6f5a5aed80c34594e00f08c8e Mon Sep 17 00:00:00 2001
> From: Allan Stephens <allan.stephens@windriver.com>
> Date: Thu, 14 Oct 2010 16:09:23 -0400
> Subject: [PATCH] tipc: delete needless memset from bearer enabling.
>
> Eliminates zeroing out of the new bearer structure at the start of
> activation, since it is already in that state.
>
> Signed-off-by: Allan Stephens <allan.stephens@windriver.com>
> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
> ---
> net/tipc/bearer.c | 2 --
> 1 files changed, 0 insertions(+), 2 deletions(-)
>
> diff --git a/net/tipc/bearer.c b/net/tipc/bearer.c
> index fd9c06c..9927d1d 100644
> --- a/net/tipc/bearer.c
> +++ b/net/tipc/bearer.c
> @@ -556,8 +556,6 @@ restart:
> }
>
> b_ptr = &tipc_bearers[bearer_id];
> - memset(b_ptr, 0, sizeof(struct bearer));
> -
> strcpy(b_ptr->publ.name, name);
> res = m_ptr->enable_bearer(&b_ptr->publ);
> if (res) {
> --
> 1.7.2.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
Acked-by: Neil Horman <nhorman@tuxdriver.com>
^ permalink raw reply
* [PATCH] bridge: make br_parse_ip_options static
From: Stephen Hemminger @ 2010-10-19 0:03 UTC (permalink / raw)
To: David Miller; +Cc: netdev
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
--- a/net/bridge/br_netfilter.c 2010-10-18 17:01:36.903364885 -0700
+++ b/net/bridge/br_netfilter.c 2010-10-18 17:01:48.106569141 -0700
@@ -213,7 +213,7 @@ static inline void nf_bridge_update_prot
* expected format
*/
-int br_parse_ip_options(struct sk_buff *skb)
+static int br_parse_ip_options(struct sk_buff *skb)
{
struct ip_options *opt;
struct iphdr *iph;
^ permalink raw reply
* Re: [MeeGo-Dev] can: How to set bitrate
From: Masayuki Ohtake @ 2010-10-19 0:09 UTC (permalink / raw)
To: Wolfgang Grandegger
Cc: andrew.chih.howe.khor-ral2JQCrhuEAvxtiuMwx3w, Samuel Ortiz,
Foster, Margie, ML netdev, ML linux-kernel,
socketcan-core-0fE9KPoRgkgATYTw5x5z8w, Wang, Yong Y,
Ewe, Kok Howg, Intel OTC, Tomoya MORINAGA, David S. Miller,
Christian Pellegrin, Wang, Qi
In-Reply-To: <4CBC29E6.3070601@grandegger.com>
On Monday, October 18, 2010 8:05 PM, Wolfgang Grandegger wrote:
> ~$ rpm -qf /usr/include/db_185.h
> db4-devel-4.7.25-13.fc12.i686
>
> Which means you need db4 devel to fully build iproute2.
Thank you for your information.
Using the following command,
"yum install db4*"
I could confirm building iproute2 is success and
set bitrate.
Thanks, Ohtake(OKISemi)
^ permalink raw reply
* Re: [PATCH 3/3] net: allocate tx queues in register_netdevice
From: John Fastabend @ 2010-10-19 0:12 UTC (permalink / raw)
To: Ben Hutchings
Cc: Tom Herbert, davem@davemloft.net, netdev@vger.kernel.org,
eric.dumazet@gmail.com
In-Reply-To: <1287438080.2252.792.camel@achroite.uk.solarflarecom.com>
On 10/18/2010 2:41 PM, Ben Hutchings wrote:
> On Mon, 2010-10-18 at 11:02 -0700, Tom Herbert wrote:
>> This patch introduces netif_alloc_netdev_queues which is called from
>> register_device instead of alloc_netdev_mq. This makes TX queue
>> allocation symmetric with RX allocation similarly allows drivers to
>> change dev->num_tx_queues after allocating netdev and before
>> registering it.
>
> Changing num_tx_queues is probably *not* desirable, same as for
> num_rx_queues.
>
Right, this will break ixgbe and other drivers that may call netif_set_real_num_{rx|tx}_queues() to increase the number of
queues. Returning an error code seems like a good idea though.
John.
> [...]
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -1553,18 +1553,24 @@ static void dev_queue_xmit_nit(struct sk_buff *skb, struct net_device *dev)
>> * Routine to help set real_num_tx_queues. To avoid skbs mapped to queues
>> * greater then real_num_tx_queues stale skbs on the qdisc must be flushed.
>> */
>> -void netif_set_real_num_tx_queues(struct net_device *dev, unsigned int txq)
>> +int netif_set_real_num_tx_queues(struct net_device *dev, unsigned int txq)
>> {
>> - unsigned int real_num = dev->real_num_tx_queues;
>> + if (txq < 1)
>> + return -EINVAL;
>>
>> - if (unlikely(txq > dev->num_tx_queues))
>> - ;
>> - else if (txq > real_num)
>> - dev->real_num_tx_queues = txq;
>> - else if (txq < real_num) {
>> - dev->real_num_tx_queues = txq;
>> - qdisc_reset_all_tx_gt(dev, txq);
>> - }
>> + if (dev->reg_state == NETREG_REGISTERED) {
>> + ASSERT_RTNL();
>> +
>> + if (txq > dev->num_tx_queues)
>> + return -EINVAL;
>> +
>> + if (txq < dev->real_num_tx_queues)
>> + qdisc_reset_all_tx_gt(dev, txq);
>> + } else
>> + dev->num_tx_queues = txq;
> [...]
>
> The kernel-doc comment should be updated to reflect the locking
> requirement and the possibility of failure when called after
> registration.
>
> Ben.
>
^ permalink raw reply
* Re: [PATCH 3/3] net: allocate tx queues in register_netdevice
From: Tom Herbert @ 2010-10-19 0:17 UTC (permalink / raw)
To: John Fastabend
Cc: Ben Hutchings, davem@davemloft.net, netdev@vger.kernel.org,
eric.dumazet@gmail.com
In-Reply-To: <4CBCE26E.4020405@intel.com>
On Mon, Oct 18, 2010 at 5:12 PM, John Fastabend
<john.r.fastabend@intel.com> wrote:
> On 10/18/2010 2:41 PM, Ben Hutchings wrote:
>> On Mon, 2010-10-18 at 11:02 -0700, Tom Herbert wrote:
>>> This patch introduces netif_alloc_netdev_queues which is called from
>>> register_device instead of alloc_netdev_mq. This makes TX queue
>>> allocation symmetric with RX allocation similarly allows drivers to
>>> change dev->num_tx_queues after allocating netdev and before
>>> registering it.
>>
>> Changing num_tx_queues is probably *not* desirable, same as for
>> num_rx_queues.
>>
Okay, so both netif_set_real_num_rx_queues and
netif_set_real_num_tx_queues should return -EINVAL if queue count >
num_[tr]x_queues?
>
> Right, this will break ixgbe and other drivers that may call netif_set_real_num_{rx|tx}_queues() to increase the number of
> queues. Returning an error code seems like a good idea though.
>
> John.
>
>> [...]
>>> --- a/net/core/dev.c
>>> +++ b/net/core/dev.c
>>> @@ -1553,18 +1553,24 @@ static void dev_queue_xmit_nit(struct sk_buff *skb, struct net_device *dev)
>>> * Routine to help set real_num_tx_queues. To avoid skbs mapped to queues
>>> * greater then real_num_tx_queues stale skbs on the qdisc must be flushed.
>>> */
>>> -void netif_set_real_num_tx_queues(struct net_device *dev, unsigned int txq)
>>> +int netif_set_real_num_tx_queues(struct net_device *dev, unsigned int txq)
>>> {
>>> - unsigned int real_num = dev->real_num_tx_queues;
>>> + if (txq < 1)
>>> + return -EINVAL;
>>>
>>> - if (unlikely(txq > dev->num_tx_queues))
>>> - ;
>>> - else if (txq > real_num)
>>> - dev->real_num_tx_queues = txq;
>>> - else if (txq < real_num) {
>>> - dev->real_num_tx_queues = txq;
>>> - qdisc_reset_all_tx_gt(dev, txq);
>>> - }
>>> + if (dev->reg_state == NETREG_REGISTERED) {
>>> + ASSERT_RTNL();
>>> +
>>> + if (txq > dev->num_tx_queues)
>>> + return -EINVAL;
>>> +
>>> + if (txq < dev->real_num_tx_queues)
>>> + qdisc_reset_all_tx_gt(dev, txq);
>>> + } else
>>> + dev->num_tx_queues = txq;
>> [...]
>>
>> The kernel-doc comment should be updated to reflect the locking
>> requirement and the possibility of failure when called after
>> registration.
>>
>> Ben.
>>
>
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox