* Re: [PATCH] bonding: reset queue mapping prior to transmission to physical device (v2)
From: Vitalii Demianets @ 2011-06-03 14:59 UTC (permalink / raw)
To: Neil Horman; +Cc: netdev, Jay Vosburgh, Andy Gospodarek, David S. Miller
In-Reply-To: <1307107607-3015-1-git-send-email-nhorman@tuxdriver.com>
On Friday 03 June 2011 16:26:47 Neil Horman wrote:
[...]
> + /*
> + *restore the origional mapping
Style and spelling nits, shouldn't it be:
* Restore the original mapping
> + */
> + skb_set_queue_mapping(skb, (u16)skb->cb[0]);
As already pointed by Ben, it should be ((u16 *)skb->cb)[0]
[...]
> + /*
> + * Save the origional txq to restore before passing to the driver
Spelling: original
> + */
> + skb->cb[0] = txq;
As already pointed by Ben, it should be ((u16 *)skb->cb)[0]
--
Vitalii Demianets
^ permalink raw reply
* Re: [PATCH net-next] bonding: make 802.3ad use update lacp_rate
From: Jiri Pirko @ 2011-06-03 15:00 UTC (permalink / raw)
To: Weiping Pan
Cc: Jay Vosburgh, Andy Gospodarek, open list:BONDING DRIVER,
open list
In-Reply-To: <1307111733-4746-1-git-send-email-panweiping3@gmail.com>
Fri, Jun 03, 2011 at 04:35:33PM CEST, panweiping3@gmail.com wrote:
>There is a bug that when you modify lacp_rate via sysfs,
>802.3ad won't use the new value of lacp_rate to transmit packets.
>That is because port->actor_oper_port_state isn't changed.
>
>And I want to use AD_STATE_LACP_TIMEOUT,
>so I move related macros from bond_3ad.c into bond_3ad.h.
>
>Signed-off-by: Weiping Pan <panweiping3@gmail.com>
>---
> drivers/net/bonding/bond_3ad.c | 48 --------------------------------------
> drivers/net/bonding/bond_3ad.h | 48 ++++++++++++++++++++++++++++++++++++++
> drivers/net/bonding/bond_sysfs.c | 15 +++++++++++-
> 3 files changed, 62 insertions(+), 49 deletions(-)
>
>diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
>index c7537abc..9083258 100644
>--- a/drivers/net/bonding/bond_3ad.c
>+++ b/drivers/net/bonding/bond_3ad.c
>@@ -34,54 +34,6 @@
> #include "bonding.h"
> #include "bond_3ad.h"
>
>-// General definitions
>-#define AD_SHORT_TIMEOUT 1
>-#define AD_LONG_TIMEOUT 0
>-#define AD_STANDBY 0x2
>-#define AD_MAX_TX_IN_SECOND 3
>-#define AD_COLLECTOR_MAX_DELAY 0
>-
>-// Timer definitions(43.4.4 in the 802.3ad standard)
>-#define AD_FAST_PERIODIC_TIME 1
>-#define AD_SLOW_PERIODIC_TIME 30
>-#define AD_SHORT_TIMEOUT_TIME (3*AD_FAST_PERIODIC_TIME)
>-#define AD_LONG_TIMEOUT_TIME (3*AD_SLOW_PERIODIC_TIME)
>-#define AD_CHURN_DETECTION_TIME 60
>-#define AD_AGGREGATE_WAIT_TIME 2
>-
>-// Port state definitions(43.4.2.2 in the 802.3ad standard)
>-#define AD_STATE_LACP_ACTIVITY 0x1
>-#define AD_STATE_LACP_TIMEOUT 0x2
>-#define AD_STATE_AGGREGATION 0x4
>-#define AD_STATE_SYNCHRONIZATION 0x8
>-#define AD_STATE_COLLECTING 0x10
>-#define AD_STATE_DISTRIBUTING 0x20
>-#define AD_STATE_DEFAULTED 0x40
>-#define AD_STATE_EXPIRED 0x80
>-
>-// Port Variables definitions used by the State Machines(43.4.7 in the 802.3ad standard)
>-#define AD_PORT_BEGIN 0x1
>-#define AD_PORT_LACP_ENABLED 0x2
>-#define AD_PORT_ACTOR_CHURN 0x4
>-#define AD_PORT_PARTNER_CHURN 0x8
>-#define AD_PORT_READY 0x10
>-#define AD_PORT_READY_N 0x20
>-#define AD_PORT_MATCHED 0x40
>-#define AD_PORT_STANDBY 0x80
>-#define AD_PORT_SELECTED 0x100
>-#define AD_PORT_MOVED 0x200
>-
>-// Port Key definitions
>-// key is determined according to the link speed, duplex and
>-// user key(which is yet not supported)
>-// ------------------------------------------------------------
>-// Port key : | User key | Speed |Duplex|
>-// ------------------------------------------------------------
>-// 16 6 1 0
>-#define AD_DUPLEX_KEY_BITS 0x1
>-#define AD_SPEED_KEY_BITS 0x3E
>-#define AD_USER_KEY_BITS 0xFFC0
>-
> //dalloun
> #define AD_LINK_SPEED_BITMASK_1MBPS 0x1
> #define AD_LINK_SPEED_BITMASK_10MBPS 0x2
>diff --git a/drivers/net/bonding/bond_3ad.h b/drivers/net/bonding/bond_3ad.h
>index 0ee3f16..6ce1f6b 100644
>--- a/drivers/net/bonding/bond_3ad.h
>+++ b/drivers/net/bonding/bond_3ad.h
>@@ -37,6 +37,54 @@
> #define AD_LACP_SLOW 0
> #define AD_LACP_FAST 1
>
>+#define AD_SHORT_TIMEOUT 1
>+#define AD_LONG_TIMEOUT 0
>+#define AD_STANDBY 0x2
>+#define AD_MAX_TX_IN_SECOND 3
>+#define AD_COLLECTOR_MAX_DELAY 0
>+
>+// Timer definitions(43.4.4 in the 802.3ad standard)
>+#define AD_FAST_PERIODIC_TIME 1
>+#define AD_SLOW_PERIODIC_TIME 30
>+#define AD_SHORT_TIMEOUT_TIME (3*AD_FAST_PERIODIC_TIME)
>+#define AD_LONG_TIMEOUT_TIME (3*AD_SLOW_PERIODIC_TIME)
>+#define AD_CHURN_DETECTION_TIME 60
>+#define AD_AGGREGATE_WAIT_TIME 2
>+
>+// Port state definitions(43.4.2.2 in the 802.3ad standard)
>+#define AD_STATE_LACP_ACTIVITY 0x1
>+#define AD_STATE_LACP_TIMEOUT 0x2
>+#define AD_STATE_AGGREGATION 0x4
>+#define AD_STATE_SYNCHRONIZATION 0x8
>+#define AD_STATE_COLLECTING 0x10
>+#define AD_STATE_DISTRIBUTING 0x20
>+#define AD_STATE_DEFAULTED 0x40
>+#define AD_STATE_EXPIRED 0x80
>+
>+// Port Variables definitions used by the State Machines(43.4.7 in the 802.3ad standard)
>+#define AD_PORT_BEGIN 0x1
>+#define AD_PORT_LACP_ENABLED 0x2
>+#define AD_PORT_ACTOR_CHURN 0x4
>+#define AD_PORT_PARTNER_CHURN 0x8
>+#define AD_PORT_READY 0x10
>+#define AD_PORT_READY_N 0x20
>+#define AD_PORT_MATCHED 0x40
>+#define AD_PORT_STANDBY 0x80
>+#define AD_PORT_SELECTED 0x100
>+#define AD_PORT_MOVED 0x200
>+
>+// Port Key definitions
>+// key is determined according to the link speed, duplex and
>+// user key(which is yet not supported)
>+// ------------------------------------------------------------
>+// Port key : | User key | Speed |Duplex|
>+// ------------------------------------------------------------
>+// 16 6 1 0
>+#define AD_DUPLEX_KEY_BITS 0x1
>+#define AD_SPEED_KEY_BITS 0x3E
>+#define AD_USER_KEY_BITS 0xFFC0
>+
>+
> typedef struct mac_addr {
> u8 mac_addr_value[ETH_ALEN];
> } __packed mac_addr_t;
>diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
>index 88fcb25..2cad514 100644
>--- a/drivers/net/bonding/bond_sysfs.c
>+++ b/drivers/net/bonding/bond_sysfs.c
>@@ -783,8 +783,13 @@ static ssize_t bonding_store_lacp(struct device *d,
> struct device_attribute *attr,
> const char *buf, size_t count)
> {
>- int new_value, ret = count;
>+ int new_value, i, ret = count;
> struct bonding *bond = to_bond(d);
>+ struct slave *slave;
>+ struct port *port = NULL;
>+
>+ if (!rtnl_trylock())
>+ return restart_syscall();
>
> if (bond->dev->flags & IFF_UP) {
> pr_err("%s: Unable to update LACP rate because interface is up.\n",
>@@ -804,6 +809,13 @@ static ssize_t bonding_store_lacp(struct device *d,
>
> if ((new_value == 1) || (new_value == 0)) {
> bond->params.lacp_fast = new_value;
>+ bond_for_each_slave(bond, slave, i) {
you should hold read_lock(&bond->lock) when you iterate over slaves
>+ port = &(slave->ad_info.port);
>+ if (new_value)
>+ port->actor_oper_port_state |= AD_STATE_LACP_TIMEOUT;
>+ else
>+ port->actor_oper_port_state &= ~AD_STATE_LACP_TIMEOUT;
>+ }
> pr_info("%s: Setting LACP rate to %s (%d).\n",
> bond->dev->name, bond_lacp_tbl[new_value].modename,
> new_value);
>@@ -813,6 +825,7 @@ static ssize_t bonding_store_lacp(struct device *d,
> ret = -EINVAL;
> }
> out:
>+ rtnl_unlock();
> return ret;
> }
> static DEVICE_ATTR(lacp_rate, S_IRUGO | S_IWUSR,
>--
>1.7.4.4
>
>--
>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
^ permalink raw reply
* Re: KVM induced panic on 2.6.38[2367] & 2.6.39
From: Bernhard Held @ 2011-06-03 15:50 UTC (permalink / raw)
To: kvm; +Cc: linux-mm, linux-kernel, netdev, linux-mm, kvm, netdev
In-Reply-To: <4DE8E3ED.7080004@fnarfbargle.com>
Am 03.06.2011 15:38, schrieb Brad Campbell:
> On 02/06/11 07:03, CaT wrote:
>> On Wed, Jun 01, 2011 at 07:52:33PM +0800, Brad Campbell wrote:
>>> Unfortunately the only interface that is mentioned by name anywhere
>>> in my firewall is $DMZ (which is ppp0 and not part of any bridge).
>>>
>>> All of the nat/dnat and other horrible hacks are based on IP addresses.
>>
>> Damn. Not referencing the bridge interfaces at all stopped our host from
>> going down in flames when we passed it a few packets. These are two
>> of the oopses we got from it. Whilst the kernel here is .35 we got the
>> same issue from a range of kernels. Seems related.
>
> Well, I tried sending an explanatory message to netdev, netfilter & cc'd to kvm,
> but it appears not to have made it to kvm or netfilter, and the cc to netdev has
> not elicited a response. My resend to netfilter seems to have dropped into the
> bit bucket also.
Just another reference 3.5 months ago:
http://www.spinics.net/lists/netfilter-devel/msg17239.html
Bernhard
^ permalink raw reply
* [PATCH] qlcnic: Avoid double free of skb in tx path
From: Anirban Chakraborty @ 2011-06-03 15:52 UTC (permalink / raw)
To: davem
Cc: netdev, Dept_NX_Linux_NIC_Driver, sucheta.chakraborty,
Anirban Chakraborty
From: Sucheta Chakraborty <sucheta.chakraborty@qlogic.com>
buffer->skb should be marked NULL to avoid double free of the skb.
Signed-off-by: Sucheta Chakraborty <sucheta.chakraborty@qlogic.com>
Signed-off-by: Anirban Chakraborty <anirban.chakraborty@qlogic.com>
---
drivers/net/qlcnic/qlcnic_main.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/drivers/net/qlcnic/qlcnic_main.c b/drivers/net/qlcnic/qlcnic_main.c
index 3ab7d2c..0f6af5c 100644
--- a/drivers/net/qlcnic/qlcnic_main.c
+++ b/drivers/net/qlcnic/qlcnic_main.c
@@ -2159,6 +2159,7 @@ qlcnic_unmap_buffers(struct pci_dev *pdev, struct sk_buff *skb,
nf = &pbuf->frag_array[0];
pci_unmap_single(pdev, nf->dma, skb_headlen(skb), PCI_DMA_TODEVICE);
+ pbuf->skb = NULL;
}
static inline void
--
1.7.4.1
^ permalink raw reply related
* Re: KVM induced panic on 2.6.38[2367] & 2.6.39
From: Brad Campbell @ 2011-06-03 16:07 UTC (permalink / raw)
To: kvm; +Cc: linux-mm, linux-kernel, netdev, netfilter-devel
In-Reply-To: <isavsg$3or$1@dough.gmane.org>
On 03/06/11 23:50, Bernhard Held wrote:
> Am 03.06.2011 15:38, schrieb Brad Campbell:
>> On 02/06/11 07:03, CaT wrote:
>>> On Wed, Jun 01, 2011 at 07:52:33PM +0800, Brad Campbell wrote:
>>>> Unfortunately the only interface that is mentioned by name anywhere
>>>> in my firewall is $DMZ (which is ppp0 and not part of any bridge).
>>>>
>>>> All of the nat/dnat and other horrible hacks are based on IP addresses.
>>>
>>> Damn. Not referencing the bridge interfaces at all stopped our host from
>>> going down in flames when we passed it a few packets. These are two
>>> of the oopses we got from it. Whilst the kernel here is .35 we got the
>>> same issue from a range of kernels. Seems related.
>>
>> Well, I tried sending an explanatory message to netdev, netfilter &
>> cc'd to kvm,
>> but it appears not to have made it to kvm or netfilter, and the cc to
>> netdev has
>> not elicited a response. My resend to netfilter seems to have dropped
>> into the
>> bit bucket also.
> Just another reference 3.5 months ago:
> http://www.spinics.net/lists/netfilter-devel/msg17239.html
<waves hands around shouting "I have a reproducible test case for this
and don't mind patching and crashing the machine to get it fixed">
Attempted to add netfilter-devel to the cc this time.
^ permalink raw reply
* [PATCH] bonding: reset queue mapping prior to transmission to physical device (v3)
From: Neil Horman @ 2011-06-03 17:32 UTC (permalink / raw)
To: netdev; +Cc: Neil Horman, Jay Vosburgh, Andy Gospodarek, David S. Miller
In-Reply-To: <1307112216.2789.16.camel@bwh-desktop>
The bonding driver is multiqueue enabled, in which each queue represents a slave
to enable optional steering of output frames to given slaves against the default
output policy. However, it needs to reset the skb->queue_mapping prior to
queuing to the physical device or the physical slave (if it is multiqueue) could
wind up transmitting on an unintended tx queue
Change Notes:
v2) Based on first pass review, updated the patch to restore the origional queue
mapping that was found in bond_select_queue, rather than simply resetting to
zero. This preserves the value of queue_mapping when it was set on receive in
the forwarding case which is desireable.
v3) Fixed spelling an casting error in skb->cb
Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
CC: Jay Vosburgh <fubar@us.ibm.com>
CC: Andy Gospodarek <andy@greyhouse.net>
CC: "David S. Miller" <davem@davemloft.net>
---
drivers/net/bonding/bond_main.c | 10 ++++++++++
1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 17b4dd9..dbb1048 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -400,6 +400,11 @@ int bond_dev_queue_xmit(struct bonding *bond, struct sk_buff *skb,
{
skb->dev = slave_dev;
skb->priority = 1;
+ /*
+ *restore the origional mapping
+ */
+ skb_set_queue_mapping(skb, ((u16 *)skb->cb)[0]);
+
if (unlikely(netpoll_tx_running(slave_dev)))
bond_netpoll_send_skb(bond_get_slave_by_dev(bond, slave_dev), skb);
else
@@ -4216,6 +4221,11 @@ static u16 bond_select_queue(struct net_device *dev, struct sk_buff *skb)
*/
u16 txq = skb_rx_queue_recorded(skb) ? skb_get_rx_queue(skb) : 0;
+ /*
+ * Save the original txq to restore before passing to the driver
+ */
+ ((u16 *)skb->cb)[0] = txq;
+
if (unlikely(txq >= dev->real_num_tx_queues)) {
do {
txq -= dev->real_num_tx_queues;
--
1.7.3.4
^ permalink raw reply related
* Re: [PATCH] bonding: reset queue mapping prior to transmission to physical device (v3)
From: Ben Hutchings @ 2011-06-03 17:59 UTC (permalink / raw)
To: Neil Horman; +Cc: netdev, Jay Vosburgh, Andy Gospodarek, David S. Miller
In-Reply-To: <1307122353-3414-1-git-send-email-nhorman@tuxdriver.com>
On Fri, 2011-06-03 at 13:32 -0400, Neil Horman wrote:
> The bonding driver is multiqueue enabled, in which each queue represents a slave
> to enable optional steering of output frames to given slaves against the default
> output policy. However, it needs to reset the skb->queue_mapping prior to
> queuing to the physical device or the physical slave (if it is multiqueue) could
> wind up transmitting on an unintended tx queue
>
> Change Notes:
> v2) Based on first pass review, updated the patch to restore the origional queue
> mapping that was found in bond_select_queue, rather than simply resetting to
> zero. This preserves the value of queue_mapping when it was set on receive in
> the forwarding case which is desireable.
>
> v3) Fixed spelling an casting error in skb->cb
>
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> CC: Jay Vosburgh <fubar@us.ibm.com>
> CC: Andy Gospodarek <andy@greyhouse.net>
> CC: "David S. Miller" <davem@davemloft.net>
> ---
> drivers/net/bonding/bond_main.c | 10 ++++++++++
> 1 files changed, 10 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 17b4dd9..dbb1048 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -400,6 +400,11 @@ int bond_dev_queue_xmit(struct bonding *bond, struct sk_buff *skb,
> {
> skb->dev = slave_dev;
> skb->priority = 1;
> + /*
> + *restore the origional mapping
> + */
> + skb_set_queue_mapping(skb, ((u16 *)skb->cb)[0]);
> +
> if (unlikely(netpoll_tx_running(slave_dev)))
> bond_netpoll_send_skb(bond_get_slave_by_dev(bond, slave_dev), skb);
> else
> @@ -4216,6 +4221,11 @@ static u16 bond_select_queue(struct net_device *dev, struct sk_buff *skb)
> */
> u16 txq = skb_rx_queue_recorded(skb) ? skb_get_rx_queue(skb) : 0;
>
> + /*
> + * Save the original txq to restore before passing to the driver
> + */
> + ((u16 *)skb->cb)[0] = txq;
> +
> if (unlikely(txq >= dev->real_num_tx_queues)) {
> do {
> txq -= dev->real_num_tx_queues;
I should have read this more thoroughly before - I'm afraid this is
still not quite right as skb_get_rx_queue() will subtract 1. You need
to save skb_get_queue_mapping() rather than txq.
But skb_{get,set}_queue_mapping() seem to be meant to deal with TX queue
numbers so maybe it's better to save and restore skb->queue_mapping
directly.
Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCH] bonding: reset queue mapping prior to transmission to physical device (v3)
From: Jay Vosburgh @ 2011-06-03 18:06 UTC (permalink / raw)
To: Neil Horman; +Cc: netdev, Andy Gospodarek, David S. Miller
In-Reply-To: <1307122353-3414-1-git-send-email-nhorman@tuxdriver.com>
Neil Horman <nhorman@tuxdriver.com> wrote:
>The bonding driver is multiqueue enabled, in which each queue represents a slave
>to enable optional steering of output frames to given slaves against the default
>output policy. However, it needs to reset the skb->queue_mapping prior to
>queuing to the physical device or the physical slave (if it is multiqueue) could
>wind up transmitting on an unintended tx queue
>
>Change Notes:
>v2) Based on first pass review, updated the patch to restore the origional queue
>mapping that was found in bond_select_queue, rather than simply resetting to
>zero. This preserves the value of queue_mapping when it was set on receive in
>the forwarding case which is desireable.
>
>v3) Fixed spelling an casting error in skb->cb
>
>Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
>CC: Jay Vosburgh <fubar@us.ibm.com>
>CC: Andy Gospodarek <andy@greyhouse.net>
>CC: "David S. Miller" <davem@davemloft.net>
>---
> drivers/net/bonding/bond_main.c | 10 ++++++++++
> 1 files changed, 10 insertions(+), 0 deletions(-)
>
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index 17b4dd9..dbb1048 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -400,6 +400,11 @@ int bond_dev_queue_xmit(struct bonding *bond, struct sk_buff *skb,
> {
> skb->dev = slave_dev;
> skb->priority = 1;
>+ /*
>+ *restore the origional mapping
>+ */
I don't think this comment (or the one below) really adds much.
If you think they're necessary, then (a) it's "original," and (b)
they'll probably fit on one line.
Or, alternately, make one of them (probably the one below) big
enough to actually explain what's going on, especially if (as Ben says)
you need to stash queue_mapping directly.
-J
>+ skb_set_queue_mapping(skb, ((u16 *)skb->cb)[0]);
>+
> if (unlikely(netpoll_tx_running(slave_dev)))
> bond_netpoll_send_skb(bond_get_slave_by_dev(bond, slave_dev), skb);
> else
>@@ -4216,6 +4221,11 @@ static u16 bond_select_queue(struct net_device *dev, struct sk_buff *skb)
> */
> u16 txq = skb_rx_queue_recorded(skb) ? skb_get_rx_queue(skb) : 0;
>
>+ /*
>+ * Save the original txq to restore before passing to the driver
>+ */
>+ ((u16 *)skb->cb)[0] = txq;
>+
> if (unlikely(txq >= dev->real_num_tx_queues)) {
> do {
> txq -= dev->real_num_tx_queues;
>--
>1.7.3.4
---
-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
^ permalink raw reply
* Re: [RFT PATCH] net: remove legacy ethtool ops
From: Ben Hutchings @ 2011-06-03 18:34 UTC (permalink / raw)
To: Michał Mirosław, Jeff Kirsher
Cc: netdev, David S. Miller, Patrick McHardy, e1000-devel
In-Reply-To: <20110507114802.A0C841389B@rere.qmqm.pl>
On Sat, 2011-05-07 at 13:48 +0200, Michał Mirosław wrote:
> As all drivers are converted, we may now remove discrete offload setting
> callback handling.
>
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> ---
>
> Note: This needs to wait for Intel guys to finish conversion of their
> LAN drivers.
[...]
It's a bit disappointing that this didn't make it into 3.0.
Jeff, has there been any progress on your drivers?
Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCH] bonding: reset queue mapping prior to transmission to physical device (v3)
From: Neil Horman @ 2011-06-03 18:36 UTC (permalink / raw)
To: Ben Hutchings; +Cc: netdev, Jay Vosburgh, Andy Gospodarek, David S. Miller
In-Reply-To: <1307123978.2789.53.camel@bwh-desktop>
On Fri, Jun 03, 2011 at 06:59:38PM +0100, Ben Hutchings wrote:
> On Fri, 2011-06-03 at 13:32 -0400, Neil Horman wrote:
> > The bonding driver is multiqueue enabled, in which each queue represents a slave
> > to enable optional steering of output frames to given slaves against the default
> > output policy. However, it needs to reset the skb->queue_mapping prior to
> > queuing to the physical device or the physical slave (if it is multiqueue) could
> > wind up transmitting on an unintended tx queue
> >
> > Change Notes:
> > v2) Based on first pass review, updated the patch to restore the origional queue
> > mapping that was found in bond_select_queue, rather than simply resetting to
> > zero. This preserves the value of queue_mapping when it was set on receive in
> > the forwarding case which is desireable.
> >
> > v3) Fixed spelling an casting error in skb->cb
> >
> > Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> > CC: Jay Vosburgh <fubar@us.ibm.com>
> > CC: Andy Gospodarek <andy@greyhouse.net>
> > CC: "David S. Miller" <davem@davemloft.net>
> > ---
> > drivers/net/bonding/bond_main.c | 10 ++++++++++
> > 1 files changed, 10 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> > index 17b4dd9..dbb1048 100644
> > --- a/drivers/net/bonding/bond_main.c
> > +++ b/drivers/net/bonding/bond_main.c
> > @@ -400,6 +400,11 @@ int bond_dev_queue_xmit(struct bonding *bond, struct sk_buff *skb,
> > {
> > skb->dev = slave_dev;
> > skb->priority = 1;
> > + /*
> > + *restore the origional mapping
> > + */
> > + skb_set_queue_mapping(skb, ((u16 *)skb->cb)[0]);
> > +
> > if (unlikely(netpoll_tx_running(slave_dev)))
> > bond_netpoll_send_skb(bond_get_slave_by_dev(bond, slave_dev), skb);
> > else
> > @@ -4216,6 +4221,11 @@ static u16 bond_select_queue(struct net_device *dev, struct sk_buff *skb)
> > */
> > u16 txq = skb_rx_queue_recorded(skb) ? skb_get_rx_queue(skb) : 0;
> >
> > + /*
> > + * Save the original txq to restore before passing to the driver
> > + */
> > + ((u16 *)skb->cb)[0] = txq;
> > +
> > if (unlikely(txq >= dev->real_num_tx_queues)) {
> > do {
> > txq -= dev->real_num_tx_queues;
>
> I should have read this more thoroughly before - I'm afraid this is
> still not quite right as skb_get_rx_queue() will subtract 1. You need
> to save skb_get_queue_mapping() rather than txq.
>
I agree that we should use skb_get_queue_mapping here, but looking at this begs
the question now, is the queue value here a 1 based or a zero based value? The
way I see it, skb->queue_mapping can be set from one of two paths:
1) from the ingress rx_path of another interface, in which case queue_mapping
will be 1 based value which we should subtract one from
2) from a local source, after having passed through a tc filter with an skbedit
action attached to it that set the queue mapping
In either case I agree, we should save and restore the raw value, rather than
the adjusted one, but it begs the question in the greater scheme, is the raw
value 1 based or zero based?
Thats probably discussion for another work item/patch, but it bears
consideration
Neil
^ permalink raw reply
* Re: [PATCH] bonding: reset queue mapping prior to transmission to physical device (v3)
From: Ben Hutchings @ 2011-06-03 19:12 UTC (permalink / raw)
To: Neil Horman; +Cc: netdev, Jay Vosburgh, Andy Gospodarek, David S. Miller
In-Reply-To: <20110603183610.GA16656@hmsreliant.think-freely.org>
On Fri, 2011-06-03 at 14:36 -0400, Neil Horman wrote:
> On Fri, Jun 03, 2011 at 06:59:38PM +0100, Ben Hutchings wrote:
> > On Fri, 2011-06-03 at 13:32 -0400, Neil Horman wrote:
> > > The bonding driver is multiqueue enabled, in which each queue represents a slave
> > > to enable optional steering of output frames to given slaves against the default
> > > output policy. However, it needs to reset the skb->queue_mapping prior to
> > > queuing to the physical device or the physical slave (if it is multiqueue) could
> > > wind up transmitting on an unintended tx queue
> > >
> > > Change Notes:
> > > v2) Based on first pass review, updated the patch to restore the origional queue
> > > mapping that was found in bond_select_queue, rather than simply resetting to
> > > zero. This preserves the value of queue_mapping when it was set on receive in
> > > the forwarding case which is desireable.
> > >
> > > v3) Fixed spelling an casting error in skb->cb
> > >
> > > Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> > > CC: Jay Vosburgh <fubar@us.ibm.com>
> > > CC: Andy Gospodarek <andy@greyhouse.net>
> > > CC: "David S. Miller" <davem@davemloft.net>
> > > ---
> > > drivers/net/bonding/bond_main.c | 10 ++++++++++
> > > 1 files changed, 10 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> > > index 17b4dd9..dbb1048 100644
> > > --- a/drivers/net/bonding/bond_main.c
> > > +++ b/drivers/net/bonding/bond_main.c
> > > @@ -400,6 +400,11 @@ int bond_dev_queue_xmit(struct bonding *bond, struct sk_buff *skb,
> > > {
> > > skb->dev = slave_dev;
> > > skb->priority = 1;
> > > + /*
> > > + *restore the origional mapping
> > > + */
> > > + skb_set_queue_mapping(skb, ((u16 *)skb->cb)[0]);
> > > +
> > > if (unlikely(netpoll_tx_running(slave_dev)))
> > > bond_netpoll_send_skb(bond_get_slave_by_dev(bond, slave_dev), skb);
> > > else
> > > @@ -4216,6 +4221,11 @@ static u16 bond_select_queue(struct net_device *dev, struct sk_buff *skb)
> > > */
> > > u16 txq = skb_rx_queue_recorded(skb) ? skb_get_rx_queue(skb) : 0;
> > >
> > > + /*
> > > + * Save the original txq to restore before passing to the driver
> > > + */
> > > + ((u16 *)skb->cb)[0] = txq;
> > > +
> > > if (unlikely(txq >= dev->real_num_tx_queues)) {
> > > do {
> > > txq -= dev->real_num_tx_queues;
> >
> > I should have read this more thoroughly before - I'm afraid this is
> > still not quite right as skb_get_rx_queue() will subtract 1. You need
> > to save skb_get_queue_mapping() rather than txq.
> >
> I agree that we should use skb_get_queue_mapping here, but looking at this begs
> the question now, is the queue value here a 1 based or a zero based value?
My understanding is that queue_mapping is supposed to be:
1. Before and including dev_pick_tx(): optionally-recorded RX queue
index (1-based)
2. After dev_pick_tx(): selected TX queue index (0-based)
But in every stacked device case 2 leads back into case 1 and
queue_mapping has to be converted at some point.
ndo_select_queue() is called within dev_pick_tx(), so case 1 applies.
> The way I see it, skb->queue_mapping can be set from one of two paths:
>
> 1) from the ingress rx_path of another interface, in which case queue_mapping
> will be 1 based value which we should subtract one from
>
> 2) from a local source, after having passed through a tc filter with an skbedit
> action attached to it that set the queue mapping
This implies that skbedit is called in case 1 and is recording a fake RX
queue index with an off-by-one error.
> In either case I agree, we should save and restore the raw value, rather than
> the adjusted one, but it begs the question in the greater scheme, is the raw
> value 1 based or zero based?
>
> Thats probably discussion for another work item/patch, but it bears
> consideration
Absolutely.
We really have to get rid of 1-based indexing, so that the only
difference is that in case 2 we know:
0 <= skb->queue_mapping < skb->dev->real_num_tx_queues
Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* bridge/netfilter: regression in 2.6.39.1
From: Alexander Holler @ 2011-06-03 19:21 UTC (permalink / raw)
To: linux-kernel; +Cc: Eric Dumazet, David Miller, Herbert Xu, netdev
Hello,
I'm getting a oops in the bridge code in br_change_mtu() with 2.6.39.1.
The patch below seems to fix that.
I'm not sure about the usage of dst_cow_metrics_generic() in
fake_dst_ops, but after having a quick look at it seems to be ok to use
that here.
Regards,
Alexander
-----
From 3c1d5951af73389798afeea672ec224e195b8e8d Mon Sep 17 00:00:00 2001
From: Alexander Holler <holler@ahsoftware.de>
Date: Fri, 3 Jun 2011 20:43:06 +0200
Subject: [PATCH] bridge: add dst_cow_metrics_generic to fake_dst_ops
Commit 42923465fb8d025a2b5153f2e7ab1e6e1058bf00 does here what it
should prevent, it introduces NULL a dereference.
The above commit uses dst_init_metrics() which sets the metrics as
read only. As result br_change_mtu() dies in dst_metric_set()
which calls dst_metrics_write_ptr() which calls
dst->ops->cow_metrics() if the metrics are read only.
---
net/bridge/br_netfilter.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/net/bridge/br_netfilter.c b/net/bridge/br_netfilter.c
index 5f9c091..de982a1 100644
--- a/net/bridge/br_netfilter.c
+++ b/net/bridge/br_netfilter.c
@@ -107,6 +107,7 @@ static void fake_update_pmtu(struct dst_entry *dst,
u32 mtu)
static struct dst_ops fake_dst_ops = {
.family = AF_INET,
.protocol = cpu_to_be16(ETH_P_IP),
+ .cow_metrics = dst_cow_metrics_generic,
.update_pmtu = fake_update_pmtu,
};
--
1.7.3.4
^ permalink raw reply related
* Re: [PATCH] bonding: reset queue mapping prior to transmission to physical device (v3)
From: Neil Horman @ 2011-06-03 19:23 UTC (permalink / raw)
To: Ben Hutchings; +Cc: netdev, Jay Vosburgh, Andy Gospodarek, David S. Miller
In-Reply-To: <1307128371.2789.72.camel@bwh-desktop>
On Fri, Jun 03, 2011 at 08:12:51PM +0100, Ben Hutchings wrote:
> On Fri, 2011-06-03 at 14:36 -0400, Neil Horman wrote:
> > On Fri, Jun 03, 2011 at 06:59:38PM +0100, Ben Hutchings wrote:
> > > On Fri, 2011-06-03 at 13:32 -0400, Neil Horman wrote:
> > > > The bonding driver is multiqueue enabled, in which each queue represents a slave
> > > > to enable optional steering of output frames to given slaves against the default
> > > > output policy. However, it needs to reset the skb->queue_mapping prior to
> > > > queuing to the physical device or the physical slave (if it is multiqueue) could
> > > > wind up transmitting on an unintended tx queue
> > > >
> > > > Change Notes:
> > > > v2) Based on first pass review, updated the patch to restore the origional queue
> > > > mapping that was found in bond_select_queue, rather than simply resetting to
> > > > zero. This preserves the value of queue_mapping when it was set on receive in
> > > > the forwarding case which is desireable.
> > > >
> > > > v3) Fixed spelling an casting error in skb->cb
> > > >
> > > > Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> > > > CC: Jay Vosburgh <fubar@us.ibm.com>
> > > > CC: Andy Gospodarek <andy@greyhouse.net>
> > > > CC: "David S. Miller" <davem@davemloft.net>
> > > > ---
> > > > drivers/net/bonding/bond_main.c | 10 ++++++++++
> > > > 1 files changed, 10 insertions(+), 0 deletions(-)
> > > >
> > > > diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> > > > index 17b4dd9..dbb1048 100644
> > > > --- a/drivers/net/bonding/bond_main.c
> > > > +++ b/drivers/net/bonding/bond_main.c
> > > > @@ -400,6 +400,11 @@ int bond_dev_queue_xmit(struct bonding *bond, struct sk_buff *skb,
> > > > {
> > > > skb->dev = slave_dev;
> > > > skb->priority = 1;
> > > > + /*
> > > > + *restore the origional mapping
> > > > + */
> > > > + skb_set_queue_mapping(skb, ((u16 *)skb->cb)[0]);
> > > > +
> > > > if (unlikely(netpoll_tx_running(slave_dev)))
> > > > bond_netpoll_send_skb(bond_get_slave_by_dev(bond, slave_dev), skb);
> > > > else
> > > > @@ -4216,6 +4221,11 @@ static u16 bond_select_queue(struct net_device *dev, struct sk_buff *skb)
> > > > */
> > > > u16 txq = skb_rx_queue_recorded(skb) ? skb_get_rx_queue(skb) : 0;
> > > >
> > > > + /*
> > > > + * Save the original txq to restore before passing to the driver
> > > > + */
> > > > + ((u16 *)skb->cb)[0] = txq;
> > > > +
> > > > if (unlikely(txq >= dev->real_num_tx_queues)) {
> > > > do {
> > > > txq -= dev->real_num_tx_queues;
> > >
> > > I should have read this more thoroughly before - I'm afraid this is
> > > still not quite right as skb_get_rx_queue() will subtract 1. You need
> > > to save skb_get_queue_mapping() rather than txq.
> > >
> > I agree that we should use skb_get_queue_mapping here, but looking at this begs
> > the question now, is the queue value here a 1 based or a zero based value?
>
> My understanding is that queue_mapping is supposed to be:
> 1. Before and including dev_pick_tx(): optionally-recorded RX queue
> index (1-based)
> 2. After dev_pick_tx(): selected TX queue index (0-based)
>
> But in every stacked device case 2 leads back into case 1 and
> queue_mapping has to be converted at some point.
>
> ndo_select_queue() is called within dev_pick_tx(), so case 1 applies.
>
> > The way I see it, skb->queue_mapping can be set from one of two paths:
> >
> > 1) from the ingress rx_path of another interface, in which case queue_mapping
> > will be 1 based value which we should subtract one from
> >
> > 2) from a local source, after having passed through a tc filter with an skbedit
> > action attached to it that set the queue mapping
>
> This implies that skbedit is called in case 1 and is recording a fake RX
> queue index with an off-by-one error.
>
Yes, it does, and that just seems wrong to me, in that it requires users of tc
to understand that queue_mapping is a one based value, but can be set to zero.
It implies that tc users have to have implicit knoweldge of how the driver model
works, that seems wrong.
> > In either case I agree, we should save and restore the raw value, rather than
> > the adjusted one, but it begs the question in the greater scheme, is the raw
> > value 1 based or zero based?
> >
> > Thats probably discussion for another work item/patch, but it bears
> > consideration
>
> Absolutely.
>
> We really have to get rid of 1-based indexing, so that the only
> difference is that in case 2 we know:
>
> 0 <= skb->queue_mapping < skb->dev->real_num_tx_queues
>
Agreed. Not quite sure of the best way to do that, but I'll put some thought
into it.
Neil
> Ben.
>
> --
> Ben Hutchings, Senior Software Engineer, Solarflare
> Not speaking for my employer; that's the marketing department's job.
> They asked us to note that Solarflare product names are trademarked.
>
>
^ permalink raw reply
* RE: [PATCH] TODO FLAG_POINTTOPOINT => FLAG_WWAN? usbnet/cdc_ncm: mark ncm devices as "mobile broadband devices" with FLAG_WWAN
From: Dan Williams @ 2011-06-03 19:24 UTC (permalink / raw)
To: Alexey ORISHKO
Cc: Valdis.Kletnieks@vt.edu, Stefan Metzmacher, Oliver Neukum,
Greg Kroah-Hartman, linux-usb@vger.kernel.org,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <2AC7D4AD8BA1C640B4C60C61C8E520153E3C13B5F2@EXDCVYMBSTM006.EQ1STM.local>
On Fri, 2011-06-03 at 11:45 +0200, Alexey ORISHKO wrote:
> > -----Original Message-----
> > From: Valdis.Kletnieks@vt.edu [mailto:Valdis.Kletnieks@vt.edu]
> > Sent: Friday, June 03, 2011 2:58 AM
> >
> > > A follow-on to this is that if you really care about specific
> > devices,
> > > your application can use udev rules to "tag" specific interfaces
> > based
> > > on USB VID/PID/GUID or other device attributes, and check for those
> > > tags in your program. Use udev (good) or netlink (good) or
> > > SIOCGIFCONF (bad) to enumerate the various network interfaces on the
> > > system and pick the
> >
> > I think Alexey's point was that the patch will hose up programs that
> > currently do the netlink or SIOCGIFCONF thing and look for
> > FLAG_POINTTOPOINT.
>
> Just to clarify, I was objecting to renaming interface name mostly because
> devices which use CDC NCM function might be something different from wwan
> devices. I would prefer to keep a generic name of interface (usbX or ethX).
>
> As an option anyone can use udev rules to set interface name they want
> for their device based on VID/PID or MAC address or something else.
> I've already provided udev rule example earlier in this thread.
>
> Dan, is it in line with your statement?
Sort of; the point of 'wwan' is to detect when we need some auxiliary
configuration to make the net interface actually do something. It
allows userspace tools (like NetworkManager) to treat ethernet
interfaces that do require aux config specially. Matching driver name
is not the solution here since as you said the driver can drive a wide
variety of hardware not all of which may be WWAN.
But there are a bunch of devices we *know* are 3G devices, and for those
we should mark them as WWAN. And the best place to do that is in the
driver where that device's USB IDs and/or workarounds may exist, just
like the existing cdc-wdm stuff for Ericsson MBM minicards. This
ensures that we keep this information in *one* place, instead of
sprinkling pieces around between userspace udev rules, apps, and the
driver.
Dan
^ permalink raw reply
* [PATCH] bonding: reset queue mapping prior to transmission to physical device (v4)
From: Neil Horman @ 2011-06-03 19:24 UTC (permalink / raw)
To: netdev; +Cc: Neil Horman, Jay Vosburgh, Andy Gospodarek, David S. Miller
In-Reply-To: <1307123978.2789.53.camel@bwh-desktop>
The bonding driver is multiqueue enabled, in which each queue represents a slave
to enable optional steering of output frames to given slaves against the default
output policy. However, it needs to reset the skb->queue_mapping prior to
queuing to the physical device or the physical slave (if it is multiqueue) could
wind up transmitting on an unintended tx queue
Change Notes:
v2) Based on first pass review, updated the patch to restore the origional queue
mapping that was found in bond_select_queue, rather than simply resetting to
zero. This preserves the value of queue_mapping when it was set on receive in
the forwarding case which is desireable.
v3) Fixed spelling an casting error in skb->cb
v4) fixed to store raw queue_mapping to avoid double decrement
Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
CC: Jay Vosburgh <fubar@us.ibm.com>
CC: Andy Gospodarek <andy@greyhouse.net>
CC: "David S. Miller" <davem@davemloft.net>
---
drivers/net/bonding/bond_main.c | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 17b4dd9..76adf27 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -400,6 +400,9 @@ int bond_dev_queue_xmit(struct bonding *bond, struct sk_buff *skb,
{
skb->dev = slave_dev;
skb->priority = 1;
+
+ skb->queue_mapping = ((u16 *)skb->cb)[0];
+
if (unlikely(netpoll_tx_running(slave_dev)))
bond_netpoll_send_skb(bond_get_slave_by_dev(bond, slave_dev), skb);
else
@@ -4216,6 +4219,11 @@ static u16 bond_select_queue(struct net_device *dev, struct sk_buff *skb)
*/
u16 txq = skb_rx_queue_recorded(skb) ? skb_get_rx_queue(skb) : 0;
+ /*
+ * Save the original txq to restore before passing to the driver
+ */
+ ((u16 *)skb->cb)[0] = skb->queue_mapping;
+
if (unlikely(txq >= dev->real_num_tx_queues)) {
do {
txq -= dev->real_num_tx_queues;
--
1.7.3.4
^ permalink raw reply related
* Re: bridge/netfilter: regression in 2.6.39.1
From: Eric Dumazet @ 2011-06-03 19:34 UTC (permalink / raw)
To: Alexander Holler; +Cc: linux-kernel, David Miller, Herbert Xu, netdev
In-Reply-To: <4DE93422.3070000@ahsoftware.de>
Le vendredi 03 juin 2011 à 21:21 +0200, Alexander Holler a écrit :
> Hello,
>
> I'm getting a oops in the bridge code in br_change_mtu() with 2.6.39.1.
> The patch below seems to fix that.
>
> I'm not sure about the usage of dst_cow_metrics_generic() in
> fake_dst_ops, but after having a quick look at it seems to be ok to use
> that here.
>
> Regards,
>
> Alexander
>
> -----
> From 3c1d5951af73389798afeea672ec224e195b8e8d Mon Sep 17 00:00:00 2001
> From: Alexander Holler <holler@ahsoftware.de>
> Date: Fri, 3 Jun 2011 20:43:06 +0200
> Subject: [PATCH] bridge: add dst_cow_metrics_generic to fake_dst_ops
>
> Commit 42923465fb8d025a2b5153f2e7ab1e6e1058bf00 does here what it
> should prevent, it introduces NULL a dereference.
>
I cant find this commit in known trees. Could you give the real commit
id and its title ?
> The above commit uses dst_init_metrics() which sets the metrics as
> read only. As result br_change_mtu() dies in dst_metric_set()
> which calls dst_metrics_write_ptr() which calls
> dst->ops->cow_metrics() if the metrics are read only.
> ---
> net/bridge/br_netfilter.c | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/net/bridge/br_netfilter.c b/net/bridge/br_netfilter.c
> index 5f9c091..de982a1 100644
> --- a/net/bridge/br_netfilter.c
> +++ b/net/bridge/br_netfilter.c
> @@ -107,6 +107,7 @@ static void fake_update_pmtu(struct dst_entry *dst,
> u32 mtu)
> static struct dst_ops fake_dst_ops = {
> .family = AF_INET,
> .protocol = cpu_to_be16(ETH_P_IP),
> + .cow_metrics = dst_cow_metrics_generic,
> .update_pmtu = fake_update_pmtu,
> };
>
Your patch is mangled (white spaces instead of tabulations)
Thanks
^ permalink raw reply
* Re: [Bugme-new] [Bug 36602] New: Bridge fails to work normally without net.ipv4.ip_forward=1
From: Andrew Morton @ 2011-06-03 19:36 UTC (permalink / raw)
To: netdev; +Cc: bugzilla-daemon, bugme-daemon, igor, Stephen Hemminger
In-Reply-To: <bug-36602-10286@https.bugzilla.kernel.org/>
(switched to email. Please respond via emailed reply-to-all, not via the
bugzilla web interface).
On Fri, 3 Jun 2011 19:21:20 GMT
bugzilla-daemon@bugzilla.kernel.org wrote:
> https://bugzilla.kernel.org/show_bug.cgi?id=36602
>
> Summary: Bridge fails to work normally without
> net.ipv4.ip_forward=1
> Product: Networking
> Version: 2.5
> Kernel Version: 2.6.38.7
> Platform: All
> OS/Version: Linux
> Tree: Mainline
> Status: NEW
> Severity: normal
> Priority: P1
> Component: Other
> AssignedTo: acme@ghostprotocols.net
> ReportedBy: igor@novg.net
> Regression: No
>
>
> Yes, this seems strange, but it's seems to be true.
>
> My network scheme is quite simple:
>
> (host1) <--- 10gbe ---> (bridge host) <--- 10gbe ---> (host2)
>
> host1 & host2 are actually VMWare ESXi hypervisors, but that's irrelevant in
> this case i think.
>
> Network adapters are Intel's 82599 10 gig cards on all hosts.
>
> At the bridge, on each interface i've created a vlan, and then bridged them:
> # vconfig add eth0 102
> # vconfig add eth1 102
> # brctl addbr br0
> # brctl addif br0 eth0.102
> # brctl addif br0 eth1.102
> # ip link set br0 mtu 9000 up
> ...etc...
>
> At this point, the bridge seems to be working, i can ping between host1 &
> host2, even with jumbo frames without fragmentation.
>
> BUT when i am trying to use iperf & friends to measure raw tcp speed between
> hosts 1/2, i'm getting something weird like 7-10 MEGABITS per second, or even
> an iperf hang until ctrl+c.
>
> If i attach an ip address to the bridge, and measure between hosts and the
> bridge, it works flawlessly, rendering 9.8Gbit/s in both directions.
>
> While trying to find a solution, when i ran out of options, i've set
> net.ipv4.ip_forward to 1, and, SURPRISE, the bridge started to work like a
> charm, at almost 10gig speed.
>
> What makes it stranger, is that in my kernel, i've turned off all routing code,
> iptables and other stuff, as this host serves primarily as iSCSI target.
>
> I have little knowledge in kernel's deep internals, but i always thought that
> bridging & routing are on different levels of operation and couldn't affect
> each other (ebtables is an exception, but i don't have it :) ).
>
> Maybe i'm interpreting the results wrong, but i've ruled out everything else.
>
> Currently, i can't use this setup as a test ground, i'll try to replicate the
> scheme in a virtual environment to see if other kernels are affected as well.
>
> Glad to hear any ideas on this.
>
^ permalink raw reply
* Re: [PATCH net-next] bonding: make 802.3ad use update lacp_rate
From: Jay Vosburgh @ 2011-06-03 19:42 UTC (permalink / raw)
To: Weiping Pan; +Cc: Andy Gospodarek, open list:BONDING DRIVER, open list
In-Reply-To: <1307111733-4746-1-git-send-email-panweiping3@gmail.com>
Weiping Pan <panweiping3@gmail.com> wrote:
>There is a bug that when you modify lacp_rate via sysfs,
>802.3ad won't use the new value of lacp_rate to transmit packets.
>That is because port->actor_oper_port_state isn't changed.
>
>And I want to use AD_STATE_LACP_TIMEOUT,
>so I move related macros from bond_3ad.c into bond_3ad.h.
>
>Signed-off-by: Weiping Pan <panweiping3@gmail.com>
>---
> drivers/net/bonding/bond_3ad.c | 48 --------------------------------------
> drivers/net/bonding/bond_3ad.h | 48 ++++++++++++++++++++++++++++++++++++++
> drivers/net/bonding/bond_sysfs.c | 15 +++++++++++-
> 3 files changed, 62 insertions(+), 49 deletions(-)
>
>diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
>index c7537abc..9083258 100644
>--- a/drivers/net/bonding/bond_3ad.c
>+++ b/drivers/net/bonding/bond_3ad.c
>@@ -34,54 +34,6 @@
> #include "bonding.h"
> #include "bond_3ad.h"
>
>-// General definitions
>-#define AD_SHORT_TIMEOUT 1
>-#define AD_LONG_TIMEOUT 0
>-#define AD_STANDBY 0x2
>-#define AD_MAX_TX_IN_SECOND 3
>-#define AD_COLLECTOR_MAX_DELAY 0
>-
>-// Timer definitions(43.4.4 in the 802.3ad standard)
>-#define AD_FAST_PERIODIC_TIME 1
>-#define AD_SLOW_PERIODIC_TIME 30
>-#define AD_SHORT_TIMEOUT_TIME (3*AD_FAST_PERIODIC_TIME)
>-#define AD_LONG_TIMEOUT_TIME (3*AD_SLOW_PERIODIC_TIME)
>-#define AD_CHURN_DETECTION_TIME 60
>-#define AD_AGGREGATE_WAIT_TIME 2
>-
>-// Port state definitions(43.4.2.2 in the 802.3ad standard)
>-#define AD_STATE_LACP_ACTIVITY 0x1
>-#define AD_STATE_LACP_TIMEOUT 0x2
>-#define AD_STATE_AGGREGATION 0x4
>-#define AD_STATE_SYNCHRONIZATION 0x8
>-#define AD_STATE_COLLECTING 0x10
>-#define AD_STATE_DISTRIBUTING 0x20
>-#define AD_STATE_DEFAULTED 0x40
>-#define AD_STATE_EXPIRED 0x80
>-
>-// Port Variables definitions used by the State Machines(43.4.7 in the 802.3ad standard)
>-#define AD_PORT_BEGIN 0x1
>-#define AD_PORT_LACP_ENABLED 0x2
>-#define AD_PORT_ACTOR_CHURN 0x4
>-#define AD_PORT_PARTNER_CHURN 0x8
>-#define AD_PORT_READY 0x10
>-#define AD_PORT_READY_N 0x20
>-#define AD_PORT_MATCHED 0x40
>-#define AD_PORT_STANDBY 0x80
>-#define AD_PORT_SELECTED 0x100
>-#define AD_PORT_MOVED 0x200
>-
>-// Port Key definitions
>-// key is determined according to the link speed, duplex and
>-// user key(which is yet not supported)
>-// ------------------------------------------------------------
>-// Port key : | User key | Speed |Duplex|
>-// ------------------------------------------------------------
>-// 16 6 1 0
>-#define AD_DUPLEX_KEY_BITS 0x1
>-#define AD_SPEED_KEY_BITS 0x3E
>-#define AD_USER_KEY_BITS 0xFFC0
>-
> //dalloun
> #define AD_LINK_SPEED_BITMASK_1MBPS 0x1
> #define AD_LINK_SPEED_BITMASK_10MBPS 0x2
>diff --git a/drivers/net/bonding/bond_3ad.h b/drivers/net/bonding/bond_3ad.h
>index 0ee3f16..6ce1f6b 100644
>--- a/drivers/net/bonding/bond_3ad.h
>+++ b/drivers/net/bonding/bond_3ad.h
>@@ -37,6 +37,54 @@
> #define AD_LACP_SLOW 0
> #define AD_LACP_FAST 1
>
>+#define AD_SHORT_TIMEOUT 1
>+#define AD_LONG_TIMEOUT 0
>+#define AD_STANDBY 0x2
>+#define AD_MAX_TX_IN_SECOND 3
>+#define AD_COLLECTOR_MAX_DELAY 0
>+
>+// Timer definitions(43.4.4 in the 802.3ad standard)
>+#define AD_FAST_PERIODIC_TIME 1
>+#define AD_SLOW_PERIODIC_TIME 30
>+#define AD_SHORT_TIMEOUT_TIME (3*AD_FAST_PERIODIC_TIME)
>+#define AD_LONG_TIMEOUT_TIME (3*AD_SLOW_PERIODIC_TIME)
>+#define AD_CHURN_DETECTION_TIME 60
>+#define AD_AGGREGATE_WAIT_TIME 2
>+
>+// Port state definitions(43.4.2.2 in the 802.3ad standard)
>+#define AD_STATE_LACP_ACTIVITY 0x1
>+#define AD_STATE_LACP_TIMEOUT 0x2
>+#define AD_STATE_AGGREGATION 0x4
>+#define AD_STATE_SYNCHRONIZATION 0x8
>+#define AD_STATE_COLLECTING 0x10
>+#define AD_STATE_DISTRIBUTING 0x20
>+#define AD_STATE_DEFAULTED 0x40
>+#define AD_STATE_EXPIRED 0x80
>+
>+// Port Variables definitions used by the State Machines(43.4.7 in the 802.3ad standard)
>+#define AD_PORT_BEGIN 0x1
>+#define AD_PORT_LACP_ENABLED 0x2
>+#define AD_PORT_ACTOR_CHURN 0x4
>+#define AD_PORT_PARTNER_CHURN 0x8
>+#define AD_PORT_READY 0x10
>+#define AD_PORT_READY_N 0x20
>+#define AD_PORT_MATCHED 0x40
>+#define AD_PORT_STANDBY 0x80
>+#define AD_PORT_SELECTED 0x100
>+#define AD_PORT_MOVED 0x200
>+
>+// Port Key definitions
>+// key is determined according to the link speed, duplex and
>+// user key(which is yet not supported)
>+// ------------------------------------------------------------
>+// Port key : | User key | Speed |Duplex|
>+// ------------------------------------------------------------
>+// 16 6 1 0
>+#define AD_DUPLEX_KEY_BITS 0x1
>+#define AD_SPEED_KEY_BITS 0x3E
>+#define AD_USER_KEY_BITS 0xFFC0
>+
>+
> typedef struct mac_addr {
> u8 mac_addr_value[ETH_ALEN];
> } __packed mac_addr_t;
>diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
>index 88fcb25..2cad514 100644
>--- a/drivers/net/bonding/bond_sysfs.c
>+++ b/drivers/net/bonding/bond_sysfs.c
>@@ -783,8 +783,13 @@ static ssize_t bonding_store_lacp(struct device *d,
> struct device_attribute *attr,
> const char *buf, size_t count)
> {
>- int new_value, ret = count;
>+ int new_value, i, ret = count;
> struct bonding *bond = to_bond(d);
>+ struct slave *slave;
>+ struct port *port = NULL;
>+
>+ if (!rtnl_trylock())
>+ return restart_syscall();
>
> if (bond->dev->flags & IFF_UP) {
> pr_err("%s: Unable to update LACP rate because interface is up.\n",
>@@ -804,6 +809,13 @@ static ssize_t bonding_store_lacp(struct device *d,
>
> if ((new_value == 1) || (new_value == 0)) {
> bond->params.lacp_fast = new_value;
>+ bond_for_each_slave(bond, slave, i) {
>+ port = &(slave->ad_info.port);
>+ if (new_value)
>+ port->actor_oper_port_state |= AD_STATE_LACP_TIMEOUT;
>+ else
>+ port->actor_oper_port_state &= ~AD_STATE_LACP_TIMEOUT;
>+ }
I think this would be cleaner if done via a helper function in
bond_3ad.c rather than inline.
Also, this should either have a comment about not needing
locking beyond RTNL, or just do the "correct" locking.
Since this requires the bond to be down, there is no real
contention for the port state (because the state machines and LACPDU
processing does not run), and holding RTNL is enough to mutex. If the
!IFF_UP restriction is ever removed, though, this would require holding
bond->lock for read and locking each port's state machine lock before
altering actor_oper_port_state.
-J
> pr_info("%s: Setting LACP rate to %s (%d).\n",
> bond->dev->name, bond_lacp_tbl[new_value].modename,
> new_value);
>@@ -813,6 +825,7 @@ static ssize_t bonding_store_lacp(struct device *d,
> ret = -EINVAL;
> }
> out:
>+ rtnl_unlock();
> return ret;
> }
> static DEVICE_ATTR(lacp_rate, S_IRUGO | S_IWUSR,
>--
>1.7.4.4
>
---
-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
^ permalink raw reply
* Re: bridge/netfilter: regression in 2.6.39.1
From: Alexander Holler @ 2011-06-03 19:42 UTC (permalink / raw)
To: Eric Dumazet; +Cc: linux-kernel, David Miller, Herbert Xu, netdev
In-Reply-To: <1307129642.2600.2.camel@edumazet-laptop>
Am 03.06.2011 21:34, schrieb Eric Dumazet:
> Le vendredi 03 juin 2011 à 21:21 +0200, Alexander Holler a écrit :
>> Hello,
>>
>> I'm getting a oops in the bridge code in br_change_mtu() with 2.6.39.1.
>> The patch below seems to fix that.
>>
>> I'm not sure about the usage of dst_cow_metrics_generic() in
>> fake_dst_ops, but after having a quick look at it seems to be ok to use
>> that here.
>>
>> Regards,
>>
>> Alexander
>>
>> -----
>> From 3c1d5951af73389798afeea672ec224e195b8e8d Mon Sep 17 00:00:00 2001
>> From: Alexander Holler<holler@ahsoftware.de>
>> Date: Fri, 3 Jun 2011 20:43:06 +0200
>> Subject: [PATCH] bridge: add dst_cow_metrics_generic to fake_dst_ops
>>
>> Commit 42923465fb8d025a2b5153f2e7ab1e6e1058bf00 does here what it
>> should prevent, it introduces NULL a dereference.
>>
>
> I cant find this commit in known trees. Could you give the real commit
> id and its title ?
>
>> The above commit uses dst_init_metrics() which sets the metrics as
>> read only. As result br_change_mtu() dies in dst_metric_set()
>> which calls dst_metrics_write_ptr() which calls
>> dst->ops->cow_metrics() if the metrics are read only.
>> ---
>> net/bridge/br_netfilter.c | 1 +
>> 1 files changed, 1 insertions(+), 0 deletions(-)
>>
>> diff --git a/net/bridge/br_netfilter.c b/net/bridge/br_netfilter.c
>> index 5f9c091..de982a1 100644
>> --- a/net/bridge/br_netfilter.c
>> +++ b/net/bridge/br_netfilter.c
>> @@ -107,6 +107,7 @@ static void fake_update_pmtu(struct dst_entry *dst,
>> u32 mtu)
>> static struct dst_ops fake_dst_ops = {
>> .family = AF_INET,
>> .protocol = cpu_to_be16(ETH_P_IP),
>> + .cow_metrics = dst_cow_metrics_generic,
>> .update_pmtu = fake_update_pmtu,
>> };
>>
>
> Your patch is mangled (white spaces instead of tabulations)
The patch had a tab, so either c&p failed or something else removed the
tab. Maybe Thunderbird, don't know. Normally I'm using git send-email.
If someone gives a feedback about the content and not the style, I'm
willing to send a nice patch which too includes the forgotten Signed-off-by.
Regards,
Alexander
^ permalink raw reply
* Re: [PATCH] bonding: reset queue mapping prior to transmission to physical device (v4)
From: Eric Dumazet @ 2011-06-03 19:48 UTC (permalink / raw)
To: Neil Horman; +Cc: netdev, Jay Vosburgh, Andy Gospodarek, David S. Miller
In-Reply-To: <1307129073-3769-1-git-send-email-nhorman@tuxdriver.com>
Le vendredi 03 juin 2011 à 15:24 -0400, Neil Horman a écrit :
> The bonding driver is multiqueue enabled, in which each queue represents a slave
> to enable optional steering of output frames to given slaves against the default
> output policy. However, it needs to reset the skb->queue_mapping prior to
> queuing to the physical device or the physical slave (if it is multiqueue) could
> wind up transmitting on an unintended tx queue
>
> Change Notes:
> v2) Based on first pass review, updated the patch to restore the origional queue
> mapping that was found in bond_select_queue, rather than simply resetting to
> zero. This preserves the value of queue_mapping when it was set on receive in
> the forwarding case which is desireable.
>
> v3) Fixed spelling an casting error in skb->cb
>
> v4) fixed to store raw queue_mapping to avoid double decrement
>
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> CC: Jay Vosburgh <fubar@us.ibm.com>
> CC: Andy Gospodarek <andy@greyhouse.net>
> CC: "David S. Miller" <davem@davemloft.net>
> ---
> drivers/net/bonding/bond_main.c | 8 ++++++++
> 1 files changed, 8 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 17b4dd9..76adf27 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -400,6 +400,9 @@ int bond_dev_queue_xmit(struct bonding *bond, struct sk_buff *skb,
> {
> skb->dev = slave_dev;
> skb->priority = 1;
> +
> + skb->queue_mapping = ((u16 *)skb->cb)[0];
Please dont do that. Use a helper.
For example :
#define bond_queue_mapping(skb) (*(unsigned int *)((skb)->cb))
skb->queue_mapping = bond_queue_mapping(skb);
> +
> if (unlikely(netpoll_tx_running(slave_dev)))
> bond_netpoll_send_skb(bond_get_slave_by_dev(bond, slave_dev), skb);
> else
> @@ -4216,6 +4219,11 @@ static u16 bond_select_queue(struct net_device *dev, struct sk_buff *skb)
> */
> u16 txq = skb_rx_queue_recorded(skb) ? skb_get_rx_queue(skb) : 0;
>
> + /*
> + * Save the original txq to restore before passing to the driver
> + */
> + ((u16 *)skb->cb)[0] = skb->queue_mapping;
bond_queue_mapping(skb) = skb->queue_mapping;
> +
> if (unlikely(txq >= dev->real_num_tx_queues)) {
> do {
> txq -= dev->real_num_tx_queues;
^ permalink raw reply
* Re: bridge/netfilter: regression in 2.6.39.1
From: Alexander Holler @ 2011-06-03 19:51 UTC (permalink / raw)
To: Eric Dumazet; +Cc: linux-kernel, David Miller, Herbert Xu, netdev
In-Reply-To: <1307129642.2600.2.camel@edumazet-laptop>
Hello,
Am 03.06.2011 21:34, schrieb Eric Dumazet:
>>
>> Commit 42923465fb8d025a2b5153f2e7ab1e6e1058bf00 does here what it
>> should prevent, it introduces NULL a dereference.
>>
>
> I cant find this commit in known trees. Could you give the real commit
> id and its title ?
Sorry, I haven't seen that question at first. This is the real commit id
as found in the stable tree for 2.6.39. The title of that commit is
"bridge: initialize fake_rtable metrics"
Regards,
Alexander
^ permalink raw reply
* pull request: wireless-2.6 2011-06-03
From: John W. Linville @ 2011-06-03 19:52 UTC (permalink / raw)
To: davem; +Cc: linux-wireless, netdev, linux-kernel
Dave,
Here is a batch of fixes intended for 3.0 -- I guess I'll need to
rename my tree eventually... :-)
Included are a libertas fix to ensure proper sequence number assignment
on transmitted frames, an ath9k fix that propogates an earlier fix to a
few more places, a fix for a cfg80211 regression caused by some mesh
code changes, a locking fix for the bluetooth l2cap code, a zd1211rw
regression fix, a collection of small fixes for wl12xx, an iwl4965
fix for a bad temperature check, an a device ID-related fix for iwlagn.
Please let me know if there are problems!
Thanks,
John
---
The following changes since commit 9a2e0fb0893ddf595d0a372e681f5b98017c6d90:
tg3: Fix tg3_skb_error_unmap() (2011-06-03 00:01:30 -0700)
are available in the git repository at:
ssh://master.kernel.org/pub/scm/linux/kernel/git/linville/wireless-2.6.git for-davem
Daniel Drake (1):
libertas: Set command sequence number later to ensure consistency
Daniel Halperin (1):
ath9k: fix two more bugs in tx power
Eliad Peller (1):
cfg80211: don't drop p2p probe responses
Johannes Berg (1):
bluetooth l2cap: fix locking in l2cap_global_chan_by_psm
John W. Linville (1):
Merge branch 'master' of git://git.kernel.org/.../linville/wireless-2.6 into for-davem
Jussi Kivilinna (1):
zd1211rw: fix to work on OHCI
Luciano Coelho (4):
wl12xx: fix passive and radar channel generation for scheduled scan
wl12xx: fix DFS channels handling in scheduled scan
wl12xx: add separate config value for DFS dwell time on sched scan
wl12xx: fix oops in sched_scan when forcing a passive scan
Stanislaw Gruszka (1):
iwl4965: correctly validate temperature value
Wey-Yi Guy (1):
iwlagn: fix incorrect PCI subsystem id for 6150 devices
drivers/net/wireless/ath/ath9k/ar9003_eeprom.c | 10 ++++-
drivers/net/wireless/ath/ath9k/eeprom_9287.c | 10 ++++-
drivers/net/wireless/iwlegacy/iwl-4965.c | 2 +-
drivers/net/wireless/iwlwifi/iwl-6000.c | 28 ++++++++----
drivers/net/wireless/iwlwifi/iwl-agn.c | 6 +-
drivers/net/wireless/iwlwifi/iwl-agn.h | 1 +
drivers/net/wireless/libertas/cmd.c | 6 +-
drivers/net/wireless/wl12xx/conf.h | 3 +
drivers/net/wireless/wl12xx/main.c | 1 +
drivers/net/wireless/wl12xx/scan.c | 49 +++++++++++++++-------
drivers/net/wireless/wl12xx/scan.h | 3 +
drivers/net/wireless/zd1211rw/zd_usb.c | 53 +++++++++++++++++++-----
include/linux/ieee80211.h | 8 +++-
net/bluetooth/l2cap_core.c | 2 +-
net/wireless/scan.c | 43 +++++++++++--------
15 files changed, 155 insertions(+), 70 deletions(-)
diff --git a/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c b/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c
index 0ca7635..ff8150e 100644
--- a/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c
+++ b/drivers/net/wireless/ath/ath9k/ar9003_eeprom.c
@@ -4645,10 +4645,16 @@ static void ar9003_hw_set_power_per_rate_table(struct ath_hw *ah,
case 1:
break;
case 2:
- scaledPower -= REDUCE_SCALED_POWER_BY_TWO_CHAIN;
+ if (scaledPower > REDUCE_SCALED_POWER_BY_TWO_CHAIN)
+ scaledPower -= REDUCE_SCALED_POWER_BY_TWO_CHAIN;
+ else
+ scaledPower = 0;
break;
case 3:
- scaledPower -= REDUCE_SCALED_POWER_BY_THREE_CHAIN;
+ if (scaledPower > REDUCE_SCALED_POWER_BY_THREE_CHAIN)
+ scaledPower -= REDUCE_SCALED_POWER_BY_THREE_CHAIN;
+ else
+ scaledPower = 0;
break;
}
diff --git a/drivers/net/wireless/ath/ath9k/eeprom_9287.c b/drivers/net/wireless/ath/ath9k/eeprom_9287.c
index 7856f0d..343fc9f 100644
--- a/drivers/net/wireless/ath/ath9k/eeprom_9287.c
+++ b/drivers/net/wireless/ath/ath9k/eeprom_9287.c
@@ -524,10 +524,16 @@ static void ath9k_hw_set_ar9287_power_per_rate_table(struct ath_hw *ah,
case 1:
break;
case 2:
- scaledPower -= REDUCE_SCALED_POWER_BY_TWO_CHAIN;
+ if (scaledPower > REDUCE_SCALED_POWER_BY_TWO_CHAIN)
+ scaledPower -= REDUCE_SCALED_POWER_BY_TWO_CHAIN;
+ else
+ scaledPower = 0;
break;
case 3:
- scaledPower -= REDUCE_SCALED_POWER_BY_THREE_CHAIN;
+ if (scaledPower > REDUCE_SCALED_POWER_BY_THREE_CHAIN)
+ scaledPower -= REDUCE_SCALED_POWER_BY_THREE_CHAIN;
+ else
+ scaledPower = 0;
break;
}
scaledPower = max((u16)0, scaledPower);
diff --git a/drivers/net/wireless/iwlegacy/iwl-4965.c b/drivers/net/wireless/iwlegacy/iwl-4965.c
index f5433c7..f9db25b 100644
--- a/drivers/net/wireless/iwlegacy/iwl-4965.c
+++ b/drivers/net/wireless/iwlegacy/iwl-4965.c
@@ -1543,7 +1543,7 @@ static void iwl4965_temperature_calib(struct iwl_priv *priv)
s32 temp;
temp = iwl4965_hw_get_temperature(priv);
- if (temp < 0)
+ if (IWL_TX_POWER_TEMPERATURE_OUT_OF_RANGE(temp))
return;
if (priv->temperature != temp) {
diff --git a/drivers/net/wireless/iwlwifi/iwl-6000.c b/drivers/net/wireless/iwlwifi/iwl-6000.c
index f8c710d..fda6fe0 100644
--- a/drivers/net/wireless/iwlwifi/iwl-6000.c
+++ b/drivers/net/wireless/iwlwifi/iwl-6000.c
@@ -603,19 +603,27 @@ struct iwl_cfg iwl6050_2abg_cfg = {
IWL_DEVICE_6050,
};
+#define IWL_DEVICE_6150 \
+ .fw_name_pre = IWL6050_FW_PRE, \
+ .ucode_api_max = IWL6050_UCODE_API_MAX, \
+ .ucode_api_min = IWL6050_UCODE_API_MIN, \
+ .ops = &iwl6150_ops, \
+ .eeprom_ver = EEPROM_6150_EEPROM_VERSION, \
+ .eeprom_calib_ver = EEPROM_6150_TX_POWER_VERSION, \
+ .base_params = &iwl6050_base_params, \
+ .need_dc_calib = true, \
+ .led_mode = IWL_LED_BLINK, \
+ .internal_wimax_coex = true
+
struct iwl_cfg iwl6150_bgn_cfg = {
.name = "Intel(R) Centrino(R) Wireless-N + WiMAX 6150 BGN",
- .fw_name_pre = IWL6050_FW_PRE,
- .ucode_api_max = IWL6050_UCODE_API_MAX,
- .ucode_api_min = IWL6050_UCODE_API_MIN,
- .eeprom_ver = EEPROM_6150_EEPROM_VERSION,
- .eeprom_calib_ver = EEPROM_6150_TX_POWER_VERSION,
- .ops = &iwl6150_ops,
- .base_params = &iwl6050_base_params,
+ IWL_DEVICE_6150,
.ht_params = &iwl6000_ht_params,
- .need_dc_calib = true,
- .led_mode = IWL_LED_RF_STATE,
- .internal_wimax_coex = true,
+};
+
+struct iwl_cfg iwl6150_bg_cfg = {
+ .name = "Intel(R) Centrino(R) Wireless-N + WiMAX 6150 BG",
+ IWL_DEVICE_6150,
};
struct iwl_cfg iwl6000_3agn_cfg = {
diff --git a/drivers/net/wireless/iwlwifi/iwl-agn.c b/drivers/net/wireless/iwlwifi/iwl-agn.c
index 11c6c11..a662adc 100644
--- a/drivers/net/wireless/iwlwifi/iwl-agn.c
+++ b/drivers/net/wireless/iwlwifi/iwl-agn.c
@@ -3831,11 +3831,11 @@ static DEFINE_PCI_DEVICE_TABLE(iwl_hw_card_ids) = {
/* 6150 WiFi/WiMax Series */
{IWL_PCI_DEVICE(0x0885, 0x1305, iwl6150_bgn_cfg)},
- {IWL_PCI_DEVICE(0x0885, 0x1306, iwl6150_bgn_cfg)},
+ {IWL_PCI_DEVICE(0x0885, 0x1307, iwl6150_bg_cfg)},
{IWL_PCI_DEVICE(0x0885, 0x1325, iwl6150_bgn_cfg)},
- {IWL_PCI_DEVICE(0x0885, 0x1326, iwl6150_bgn_cfg)},
+ {IWL_PCI_DEVICE(0x0885, 0x1327, iwl6150_bg_cfg)},
{IWL_PCI_DEVICE(0x0886, 0x1315, iwl6150_bgn_cfg)},
- {IWL_PCI_DEVICE(0x0886, 0x1316, iwl6150_bgn_cfg)},
+ {IWL_PCI_DEVICE(0x0886, 0x1317, iwl6150_bg_cfg)},
/* 1000 Series WiFi */
{IWL_PCI_DEVICE(0x0083, 0x1205, iwl1000_bgn_cfg)},
diff --git a/drivers/net/wireless/iwlwifi/iwl-agn.h b/drivers/net/wireless/iwlwifi/iwl-agn.h
index 2495fe7..d171684 100644
--- a/drivers/net/wireless/iwlwifi/iwl-agn.h
+++ b/drivers/net/wireless/iwlwifi/iwl-agn.h
@@ -89,6 +89,7 @@ extern struct iwl_cfg iwl6000_3agn_cfg;
extern struct iwl_cfg iwl6050_2agn_cfg;
extern struct iwl_cfg iwl6050_2abg_cfg;
extern struct iwl_cfg iwl6150_bgn_cfg;
+extern struct iwl_cfg iwl6150_bg_cfg;
extern struct iwl_cfg iwl1000_bgn_cfg;
extern struct iwl_cfg iwl1000_bg_cfg;
extern struct iwl_cfg iwl100_bgn_cfg;
diff --git a/drivers/net/wireless/libertas/cmd.c b/drivers/net/wireless/libertas/cmd.c
index 84566db..71c8f3f 100644
--- a/drivers/net/wireless/libertas/cmd.c
+++ b/drivers/net/wireless/libertas/cmd.c
@@ -994,6 +994,8 @@ static void lbs_submit_command(struct lbs_private *priv,
cmd = cmdnode->cmdbuf;
spin_lock_irqsave(&priv->driver_lock, flags);
+ priv->seqnum++;
+ cmd->seqnum = cpu_to_le16(priv->seqnum);
priv->cur_cmd = cmdnode;
spin_unlock_irqrestore(&priv->driver_lock, flags);
@@ -1621,11 +1623,9 @@ struct cmd_ctrl_node *__lbs_cmd_async(struct lbs_private *priv,
/* Copy the incoming command to the buffer */
memcpy(cmdnode->cmdbuf, in_cmd, in_cmd_size);
- /* Set sequence number, clean result, move to buffer */
- priv->seqnum++;
+ /* Set command, clean result, move to buffer */
cmdnode->cmdbuf->command = cpu_to_le16(command);
cmdnode->cmdbuf->size = cpu_to_le16(in_cmd_size);
- cmdnode->cmdbuf->seqnum = cpu_to_le16(priv->seqnum);
cmdnode->cmdbuf->result = 0;
lbs_deb_host("PREP_CMD: command 0x%04x\n", command);
diff --git a/drivers/net/wireless/wl12xx/conf.h b/drivers/net/wireless/wl12xx/conf.h
index 1ab6c86..c83fefb 100644
--- a/drivers/net/wireless/wl12xx/conf.h
+++ b/drivers/net/wireless/wl12xx/conf.h
@@ -1157,6 +1157,9 @@ struct conf_sched_scan_settings {
/* time to wait on the channel for passive scans (in TUs) */
u32 dwell_time_passive;
+ /* time to wait on the channel for DFS scans (in TUs) */
+ u32 dwell_time_dfs;
+
/* number of probe requests to send on each channel in active scans */
u8 num_probe_reqs;
diff --git a/drivers/net/wireless/wl12xx/main.c b/drivers/net/wireless/wl12xx/main.c
index bc00e52..e6497dc 100644
--- a/drivers/net/wireless/wl12xx/main.c
+++ b/drivers/net/wireless/wl12xx/main.c
@@ -311,6 +311,7 @@ static struct conf_drv_settings default_conf = {
.min_dwell_time_active = 8,
.max_dwell_time_active = 30,
.dwell_time_passive = 100,
+ .dwell_time_dfs = 150,
.num_probe_reqs = 2,
.rssi_threshold = -90,
.snr_threshold = 0,
diff --git a/drivers/net/wireless/wl12xx/scan.c b/drivers/net/wireless/wl12xx/scan.c
index f37e5a3..56f76ab 100644
--- a/drivers/net/wireless/wl12xx/scan.c
+++ b/drivers/net/wireless/wl12xx/scan.c
@@ -331,16 +331,22 @@ wl1271_scan_get_sched_scan_channels(struct wl1271 *wl,
struct conf_sched_scan_settings *c = &wl->conf.sched_scan;
int i, j;
u32 flags;
+ bool force_passive = !req->n_ssids;
for (i = 0, j = start;
i < req->n_channels && j < MAX_CHANNELS_ALL_BANDS;
i++) {
flags = req->channels[i]->flags;
- if (!(flags & IEEE80211_CHAN_DISABLED) &&
- ((flags & IEEE80211_CHAN_PASSIVE_SCAN) == passive) &&
- ((flags & IEEE80211_CHAN_RADAR) == radar) &&
- (req->channels[i]->band == band)) {
+ if (force_passive)
+ flags |= IEEE80211_CHAN_PASSIVE_SCAN;
+
+ if ((req->channels[i]->band == band) &&
+ !(flags & IEEE80211_CHAN_DISABLED) &&
+ (!!(flags & IEEE80211_CHAN_RADAR) == radar) &&
+ /* if radar is set, we ignore the passive flag */
+ (radar ||
+ !!(flags & IEEE80211_CHAN_PASSIVE_SCAN) == passive)) {
wl1271_debug(DEBUG_SCAN, "band %d, center_freq %d ",
req->channels[i]->band,
req->channels[i]->center_freq);
@@ -350,7 +356,12 @@ wl1271_scan_get_sched_scan_channels(struct wl1271 *wl,
wl1271_debug(DEBUG_SCAN, "max_power %d",
req->channels[i]->max_power);
- if (flags & IEEE80211_CHAN_PASSIVE_SCAN) {
+ if (flags & IEEE80211_CHAN_RADAR) {
+ channels[j].flags |= SCAN_CHANNEL_FLAGS_DFS;
+ channels[j].passive_duration =
+ cpu_to_le16(c->dwell_time_dfs);
+ }
+ else if (flags & IEEE80211_CHAN_PASSIVE_SCAN) {
channels[j].passive_duration =
cpu_to_le16(c->dwell_time_passive);
} else {
@@ -359,7 +370,7 @@ wl1271_scan_get_sched_scan_channels(struct wl1271 *wl,
channels[j].max_duration =
cpu_to_le16(c->max_dwell_time_active);
}
- channels[j].tx_power_att = req->channels[j]->max_power;
+ channels[j].tx_power_att = req->channels[i]->max_power;
channels[j].channel = req->channels[i]->hw_value;
j++;
@@ -386,7 +397,11 @@ wl1271_scan_sched_scan_channels(struct wl1271 *wl,
wl1271_scan_get_sched_scan_channels(wl, req, cfg->channels,
IEEE80211_BAND_2GHZ,
false, false, idx);
- idx += cfg->active[0];
+ /*
+ * 5GHz channels always start at position 14, not immediately
+ * after the last 2.4GHz channel
+ */
+ idx = 14;
cfg->passive[1] =
wl1271_scan_get_sched_scan_channels(wl, req, cfg->channels,
@@ -394,22 +409,23 @@ wl1271_scan_sched_scan_channels(struct wl1271 *wl,
false, true, idx);
idx += cfg->passive[1];
- cfg->active[1] =
+ cfg->dfs =
wl1271_scan_get_sched_scan_channels(wl, req, cfg->channels,
IEEE80211_BAND_5GHZ,
- false, false, 14);
- idx += cfg->active[1];
+ true, true, idx);
+ idx += cfg->dfs;
- cfg->dfs =
+ cfg->active[1] =
wl1271_scan_get_sched_scan_channels(wl, req, cfg->channels,
IEEE80211_BAND_5GHZ,
- true, false, idx);
- idx += cfg->dfs;
+ false, false, idx);
+ idx += cfg->active[1];
wl1271_debug(DEBUG_SCAN, " 2.4GHz: active %d passive %d",
cfg->active[0], cfg->passive[0]);
wl1271_debug(DEBUG_SCAN, " 5GHz: active %d passive %d",
cfg->active[1], cfg->passive[1]);
+ wl1271_debug(DEBUG_SCAN, " DFS: %d", cfg->dfs);
return idx;
}
@@ -421,6 +437,7 @@ int wl1271_scan_sched_scan_config(struct wl1271 *wl,
struct wl1271_cmd_sched_scan_config *cfg = NULL;
struct conf_sched_scan_settings *c = &wl->conf.sched_scan;
int i, total_channels, ret;
+ bool force_passive = !req->n_ssids;
wl1271_debug(DEBUG_CMD, "cmd sched_scan scan config");
@@ -444,7 +461,7 @@ int wl1271_scan_sched_scan_config(struct wl1271 *wl,
for (i = 0; i < SCAN_MAX_CYCLE_INTERVALS; i++)
cfg->intervals[i] = cpu_to_le32(req->interval);
- if (req->ssids[0].ssid_len && req->ssids[0].ssid) {
+ if (!force_passive && req->ssids[0].ssid_len && req->ssids[0].ssid) {
cfg->filter_type = SCAN_SSID_FILTER_SPECIFIC;
cfg->ssid_len = req->ssids[0].ssid_len;
memcpy(cfg->ssid, req->ssids[0].ssid,
@@ -461,7 +478,7 @@ int wl1271_scan_sched_scan_config(struct wl1271 *wl,
goto out;
}
- if (cfg->active[0]) {
+ if (!force_passive && cfg->active[0]) {
ret = wl1271_cmd_build_probe_req(wl, req->ssids[0].ssid,
req->ssids[0].ssid_len,
ies->ie[IEEE80211_BAND_2GHZ],
@@ -473,7 +490,7 @@ int wl1271_scan_sched_scan_config(struct wl1271 *wl,
}
}
- if (cfg->active[1]) {
+ if (!force_passive && cfg->active[1]) {
ret = wl1271_cmd_build_probe_req(wl, req->ssids[0].ssid,
req->ssids[0].ssid_len,
ies->ie[IEEE80211_BAND_5GHZ],
diff --git a/drivers/net/wireless/wl12xx/scan.h b/drivers/net/wireless/wl12xx/scan.h
index c833195..a0b6c5d 100644
--- a/drivers/net/wireless/wl12xx/scan.h
+++ b/drivers/net/wireless/wl12xx/scan.h
@@ -137,6 +137,9 @@ enum {
SCAN_BSS_TYPE_ANY,
};
+#define SCAN_CHANNEL_FLAGS_DFS BIT(0)
+#define SCAN_CHANNEL_FLAGS_DFS_ENABLED BIT(1)
+
struct conn_scan_ch_params {
__le16 min_duration;
__le16 max_duration;
diff --git a/drivers/net/wireless/zd1211rw/zd_usb.c b/drivers/net/wireless/zd1211rw/zd_usb.c
index 0e81994..631194d 100644
--- a/drivers/net/wireless/zd1211rw/zd_usb.c
+++ b/drivers/net/wireless/zd1211rw/zd_usb.c
@@ -1533,6 +1533,31 @@ static void __exit usb_exit(void)
module_init(usb_init);
module_exit(usb_exit);
+static int zd_ep_regs_out_msg(struct usb_device *udev, void *data, int len,
+ int *actual_length, int timeout)
+{
+ /* In USB 2.0 mode EP_REGS_OUT endpoint is interrupt type. However in
+ * USB 1.1 mode endpoint is bulk. Select correct type URB by endpoint
+ * descriptor.
+ */
+ struct usb_host_endpoint *ep;
+ unsigned int pipe;
+
+ pipe = usb_sndintpipe(udev, EP_REGS_OUT);
+ ep = usb_pipe_endpoint(udev, pipe);
+ if (!ep)
+ return -EINVAL;
+
+ if (usb_endpoint_xfer_int(&ep->desc)) {
+ return usb_interrupt_msg(udev, pipe, data, len,
+ actual_length, timeout);
+ } else {
+ pipe = usb_sndbulkpipe(udev, EP_REGS_OUT);
+ return usb_bulk_msg(udev, pipe, data, len, actual_length,
+ timeout);
+ }
+}
+
static int usb_int_regs_length(unsigned int count)
{
return sizeof(struct usb_int_regs) + count * sizeof(struct reg_data);
@@ -1648,15 +1673,14 @@ int zd_usb_ioread16v(struct zd_usb *usb, u16 *values,
udev = zd_usb_to_usbdev(usb);
prepare_read_regs_int(usb);
- r = usb_interrupt_msg(udev, usb_sndintpipe(udev, EP_REGS_OUT),
- req, req_len, &actual_req_len, 50 /* ms */);
+ r = zd_ep_regs_out_msg(udev, req, req_len, &actual_req_len, 50 /*ms*/);
if (r) {
dev_dbg_f(zd_usb_dev(usb),
- "error in usb_interrupt_msg(). Error number %d\n", r);
+ "error in zd_ep_regs_out_msg(). Error number %d\n", r);
goto error;
}
if (req_len != actual_req_len) {
- dev_dbg_f(zd_usb_dev(usb), "error in usb_interrupt_msg()\n"
+ dev_dbg_f(zd_usb_dev(usb), "error in zd_ep_regs_out_msg()\n"
" req_len %d != actual_req_len %d\n",
req_len, actual_req_len);
r = -EIO;
@@ -1818,9 +1842,17 @@ int zd_usb_iowrite16v_async(struct zd_usb *usb, const struct zd_ioreq16 *ioreqs,
rw->value = cpu_to_le16(ioreqs[i].value);
}
- usb_fill_int_urb(urb, udev, usb_sndintpipe(udev, EP_REGS_OUT),
- req, req_len, iowrite16v_urb_complete, usb,
- ep->desc.bInterval);
+ /* In USB 2.0 mode endpoint is interrupt type. However in USB 1.1 mode
+ * endpoint is bulk. Select correct type URB by endpoint descriptor.
+ */
+ if (usb_endpoint_xfer_int(&ep->desc))
+ usb_fill_int_urb(urb, udev, usb_sndintpipe(udev, EP_REGS_OUT),
+ req, req_len, iowrite16v_urb_complete, usb,
+ ep->desc.bInterval);
+ else
+ usb_fill_bulk_urb(urb, udev, usb_sndbulkpipe(udev, EP_REGS_OUT),
+ req, req_len, iowrite16v_urb_complete, usb);
+
urb->transfer_flags |= URB_FREE_BUFFER;
/* Submit previous URB */
@@ -1924,15 +1956,14 @@ int zd_usb_rfwrite(struct zd_usb *usb, u32 value, u8 bits)
}
udev = zd_usb_to_usbdev(usb);
- r = usb_interrupt_msg(udev, usb_sndintpipe(udev, EP_REGS_OUT),
- req, req_len, &actual_req_len, 50 /* ms */);
+ r = zd_ep_regs_out_msg(udev, req, req_len, &actual_req_len, 50 /*ms*/);
if (r) {
dev_dbg_f(zd_usb_dev(usb),
- "error in usb_interrupt_msg(). Error number %d\n", r);
+ "error in zd_ep_regs_out_msg(). Error number %d\n", r);
goto out;
}
if (req_len != actual_req_len) {
- dev_dbg_f(zd_usb_dev(usb), "error in usb_interrupt_msg()"
+ dev_dbg_f(zd_usb_dev(usb), "error in zd_ep_regs_out_msg()"
" req_len %d != actual_req_len %d\n",
req_len, actual_req_len);
r = -EIO;
diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
index b2eee58..bf56b6f 100644
--- a/include/linux/ieee80211.h
+++ b/include/linux/ieee80211.h
@@ -1003,8 +1003,12 @@ struct ieee80211_ht_info {
#define WLAN_CAPABILITY_ESS (1<<0)
#define WLAN_CAPABILITY_IBSS (1<<1)
-/* A mesh STA sets the ESS and IBSS capability bits to zero */
-#define WLAN_CAPABILITY_IS_MBSS(cap) \
+/*
+ * A mesh STA sets the ESS and IBSS capability bits to zero.
+ * however, this holds true for p2p probe responses (in the p2p_find
+ * phase) as well.
+ */
+#define WLAN_CAPABILITY_IS_STA_BSS(cap) \
(!((cap) & (WLAN_CAPABILITY_ESS | WLAN_CAPABILITY_IBSS)))
#define WLAN_CAPABILITY_CF_POLLABLE (1<<2)
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index a86f9ba..e64a1c2 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -906,7 +906,7 @@ static struct l2cap_chan *l2cap_global_chan_by_psm(int state, __le16 psm, bdaddr
if (c->psm == psm) {
/* Exact match. */
if (!bacmp(&bt_sk(sk)->src, src)) {
- read_unlock_bh(&chan_list_lock);
+ read_unlock(&chan_list_lock);
return c;
}
diff --git a/net/wireless/scan.c b/net/wireless/scan.c
index 73a441d..7a6c676 100644
--- a/net/wireless/scan.c
+++ b/net/wireless/scan.c
@@ -267,13 +267,35 @@ static bool is_bss(struct cfg80211_bss *a,
return memcmp(ssidie + 2, ssid, ssid_len) == 0;
}
+static bool is_mesh_bss(struct cfg80211_bss *a)
+{
+ const u8 *ie;
+
+ if (!WLAN_CAPABILITY_IS_STA_BSS(a->capability))
+ return false;
+
+ ie = cfg80211_find_ie(WLAN_EID_MESH_ID,
+ a->information_elements,
+ a->len_information_elements);
+ if (!ie)
+ return false;
+
+ ie = cfg80211_find_ie(WLAN_EID_MESH_CONFIG,
+ a->information_elements,
+ a->len_information_elements);
+ if (!ie)
+ return false;
+
+ return true;
+}
+
static bool is_mesh(struct cfg80211_bss *a,
const u8 *meshid, size_t meshidlen,
const u8 *meshcfg)
{
const u8 *ie;
- if (!WLAN_CAPABILITY_IS_MBSS(a->capability))
+ if (!WLAN_CAPABILITY_IS_STA_BSS(a->capability))
return false;
ie = cfg80211_find_ie(WLAN_EID_MESH_ID,
@@ -311,7 +333,7 @@ static int cmp_bss(struct cfg80211_bss *a,
if (a->channel != b->channel)
return b->channel->center_freq - a->channel->center_freq;
- if (WLAN_CAPABILITY_IS_MBSS(a->capability | b->capability)) {
+ if (is_mesh_bss(a) && is_mesh_bss(b)) {
r = cmp_ies(WLAN_EID_MESH_ID,
a->information_elements,
a->len_information_elements,
@@ -457,7 +479,6 @@ cfg80211_bss_update(struct cfg80211_registered_device *dev,
struct cfg80211_internal_bss *res)
{
struct cfg80211_internal_bss *found = NULL;
- const u8 *meshid, *meshcfg;
/*
* The reference to "res" is donated to this function.
@@ -470,22 +491,6 @@ cfg80211_bss_update(struct cfg80211_registered_device *dev,
res->ts = jiffies;
- if (WLAN_CAPABILITY_IS_MBSS(res->pub.capability)) {
- /* must be mesh, verify */
- meshid = cfg80211_find_ie(WLAN_EID_MESH_ID,
- res->pub.information_elements,
- res->pub.len_information_elements);
- meshcfg = cfg80211_find_ie(WLAN_EID_MESH_CONFIG,
- res->pub.information_elements,
- res->pub.len_information_elements);
- if (!meshid || !meshcfg ||
- meshcfg[1] != sizeof(struct ieee80211_meshconf_ie)) {
- /* bogus mesh */
- kref_put(&res->ref, bss_release);
- return NULL;
- }
- }
-
spin_lock_bh(&dev->bss_lock);
found = rb_find_bss(dev, res);
--
John W. Linville Someday the world will need a hero, and you
linville@tuxdriver.com might be all we have. Be ready.
^ permalink raw reply related
* Re: bridge/netfilter: regression in 2.6.39.1
From: Eric Dumazet @ 2011-06-03 19:55 UTC (permalink / raw)
To: Alexander Holler; +Cc: linux-kernel, David Miller, Herbert Xu, netdev
In-Reply-To: <4DE93B31.8040700@ahsoftware.de>
Le vendredi 03 juin 2011 à 21:51 +0200, Alexander Holler a écrit :
> Hello,
>
> Am 03.06.2011 21:34, schrieb Eric Dumazet:
> >>
> >> Commit 42923465fb8d025a2b5153f2e7ab1e6e1058bf00 does here what it
> >> should prevent, it introduces NULL a dereference.
> >>
> >
> > I cant find this commit in known trees. Could you give the real commit
> > id and its title ?
>
> Sorry, I haven't seen that question at first. This is the real commit id
> as found in the stable tree for 2.6.39. The title of that commit is
> "bridge: initialize fake_rtable metrics"
OK, its commit id is 33eb9873a283a2076f2b5628813d5365ca420ea9
Please always use linux-2.6 tree commits, this saves a lot of time.
Thanks
^ permalink raw reply
* Re: [PATCH] bonding: reset queue mapping prior to transmission to physical device (v4)
From: Neil Horman @ 2011-06-03 19:57 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, Jay Vosburgh, Andy Gospodarek, David S. Miller
In-Reply-To: <1307130531.2600.8.camel@edumazet-laptop>
On Fri, Jun 03, 2011 at 09:48:51PM +0200, Eric Dumazet wrote:
> Le vendredi 03 juin 2011 à 15:24 -0400, Neil Horman a écrit :
> > The bonding driver is multiqueue enabled, in which each queue represents a slave
> > to enable optional steering of output frames to given slaves against the default
> > output policy. However, it needs to reset the skb->queue_mapping prior to
> > queuing to the physical device or the physical slave (if it is multiqueue) could
> > wind up transmitting on an unintended tx queue
> >
> > Change Notes:
> > v2) Based on first pass review, updated the patch to restore the origional queue
> > mapping that was found in bond_select_queue, rather than simply resetting to
> > zero. This preserves the value of queue_mapping when it was set on receive in
> > the forwarding case which is desireable.
> >
> > v3) Fixed spelling an casting error in skb->cb
> >
> > v4) fixed to store raw queue_mapping to avoid double decrement
> >
> > Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> > CC: Jay Vosburgh <fubar@us.ibm.com>
> > CC: Andy Gospodarek <andy@greyhouse.net>
> > CC: "David S. Miller" <davem@davemloft.net>
> > ---
> > drivers/net/bonding/bond_main.c | 8 ++++++++
> > 1 files changed, 8 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> > index 17b4dd9..76adf27 100644
> > --- a/drivers/net/bonding/bond_main.c
> > +++ b/drivers/net/bonding/bond_main.c
> > @@ -400,6 +400,9 @@ int bond_dev_queue_xmit(struct bonding *bond, struct sk_buff *skb,
> > {
> > skb->dev = slave_dev;
> > skb->priority = 1;
> > +
> > + skb->queue_mapping = ((u16 *)skb->cb)[0];
>
> Please dont do that. Use a helper.
>
Why? It seems to be reasonably common practice for drivers to access
queue_mapping directly.
Examples can be found in:
ixgbe_xmit_frame
mlx4_en_xmit
qlge_send
igb_xmit_frame_adv
gfar_start_xmit
among others.
Not saying its correct to do so necessecarily, but it seems a helper doesn't buy
us much here, particularly a per-driver helper. If a helper really should be
used, why not just consistently use skb_get_queue_mapping?
Neil
^ permalink raw reply
* Re: [PATCH] bonding: reset queue mapping prior to transmission to physical device (v4)
From: Eric Dumazet @ 2011-06-03 20:05 UTC (permalink / raw)
To: Neil Horman; +Cc: netdev, Jay Vosburgh, Andy Gospodarek, David S. Miller
In-Reply-To: <20110603195700.GC16656@hmsreliant.think-freely.org>
Le vendredi 03 juin 2011 à 15:57 -0400, Neil Horman a écrit :
> On Fri, Jun 03, 2011 at 09:48:51PM +0200, Eric Dumazet wrote:
> > Le vendredi 03 juin 2011 à 15:24 -0400, Neil Horman a écrit :
> > > The bonding driver is multiqueue enabled, in which each queue represents a slave
> > > to enable optional steering of output frames to given slaves against the default
> > > output policy. However, it needs to reset the skb->queue_mapping prior to
> > > queuing to the physical device or the physical slave (if it is multiqueue) could
> > > wind up transmitting on an unintended tx queue
> > >
> > > Change Notes:
> > > v2) Based on first pass review, updated the patch to restore the origional queue
> > > mapping that was found in bond_select_queue, rather than simply resetting to
> > > zero. This preserves the value of queue_mapping when it was set on receive in
> > > the forwarding case which is desireable.
> > >
> > > v3) Fixed spelling an casting error in skb->cb
> > >
> > > v4) fixed to store raw queue_mapping to avoid double decrement
> > >
> > > Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> > > CC: Jay Vosburgh <fubar@us.ibm.com>
> > > CC: Andy Gospodarek <andy@greyhouse.net>
> > > CC: "David S. Miller" <davem@davemloft.net>
> > > ---
> > > drivers/net/bonding/bond_main.c | 8 ++++++++
> > > 1 files changed, 8 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> > > index 17b4dd9..76adf27 100644
> > > --- a/drivers/net/bonding/bond_main.c
> > > +++ b/drivers/net/bonding/bond_main.c
> > > @@ -400,6 +400,9 @@ int bond_dev_queue_xmit(struct bonding *bond, struct sk_buff *skb,
> > > {
> > > skb->dev = slave_dev;
> > > skb->priority = 1;
> > > +
> > > + skb->queue_mapping = ((u16 *)skb->cb)[0];
> >
> > Please dont do that. Use a helper.
> >
> Why? It seems to be reasonably common practice for drivers to access
> queue_mapping directly.
>
> Examples can be found in:
> ixgbe_xmit_frame
> mlx4_en_xmit
> qlge_send
> igb_xmit_frame_adv
> gfar_start_xmit
>
> among others.
>
> Not saying its correct to do so necessecarily, but it seems a helper doesn't buy
> us much here, particularly a per-driver helper. If a helper really should be
> used, why not just consistently use skb_get_queue_mapping?
> Neil
I was speaking of skb->cb access, of course, sorry if you missed my
point ;)
Please take a look at various files using helpers for this.
For example : net/ipv4/igmp.c
#define igmp_skb_size(skb) (*(unsigned int *)((skb)->cb))
^ 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