Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next-2.6 v3 00/12] net-caif: introducing CAIF protocol stack
From: David Miller @ 2010-02-27 10:31 UTC (permalink / raw)
  To: sjur.brandeland
  Cc: marcel, netdev, daniel.martensson, kaber, stefano.babic,
	randy.dunlap
In-Reply-To: <61D8D34BB13CFE408D154529C120E079138CB8@eseldmw101.eemea.ericsson.se>

From: Sjur Brændeland <sjur.brandeland@stericsson.com>
Date: Sat, 27 Feb 2010 11:05:14 +0100

> Unfortunately, I am traveling until Thursday this week, without
> Internet access, so if you would like me to resubmit again I will
> not be able to do that before Thursday or Friday this week.

Then you won't be able to make this merge window, that's way too
late.

Sorry.

^ permalink raw reply

* Re: pull request: wireless-next-2.6 2010-02-26
From: David Miller @ 2010-02-27 10:31 UTC (permalink / raw)
  To: linville-2XuSBdqkA4R54TAoqtyWWQ
  Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20100226230149.GA10987-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>

From: "John W. Linville" <linville-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
Date: Fri, 26 Feb 2010 18:01:50 -0500

> Here is one final round of wireless bits for the 2.6.34 merge window.
> Basically these would qualify as fixes anyway (more or less), but we
> might as well get them in now.
> 
> Please let me know if there are problems!

Pulled, thanks John.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [net-next-2.6 PATCH v3 0/3] Support for MPC512x FEC
From: David Miller @ 2010-02-27 10:34 UTC (permalink / raw)
  To: agust; +Cc: netdev, linuxppc-dev, grant.likely, dzu, wd, jcrigby
In-Reply-To: <1267221649-22754-1-git-send-email-agust@denx.de>

From: Anatolij Gustschin <agust@denx.de>
Date: Fri, 26 Feb 2010 23:00:46 +0100

> These patches attempt to provide support for the Freescale MPC512x
> FEC in the fs_enet driver. The first cleanup patch replaces printk
> by dev_xxx. The second and third attemt to support MPC5121 FEC
> in the FEC driver.

Applied, for real this time! :-)

Thanks.

^ permalink raw reply

* loan @ 2%
From: Mr Nosa @ 2010-02-27 10:25 UTC (permalink / raw)


Dear Valued Customer

 Are you a businessman or woman? Are you in any financial stress or do you need funds to start up your own business? Do you have a low credit score and  you are finding it hard to obtain a loan from local banks and other financial institutes? 

Awaiting Your Response via Contact address; nosa_ighodaro@yahoo.com
Best Regards 
Nosa

^ permalink raw reply

* Re: [net-next-2.6 PATCH] infiniband: convert to use netdev_for_each_mc_addr
From: Jiri Pirko @ 2010-02-27 10:38 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Or Gerlitz, netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA, davem-fT/PcQaiUtIeIZ0/mPfg9Q
In-Reply-To: <20100227060146.GA13231-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>

Sat, Feb 27, 2010 at 07:01:46AM CET, jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org wrote:
>On Thu, Feb 25, 2010 at 09:49:15AM +0100, Jiri Pirko wrote:
>> Thu, Feb 25, 2010 at 09:00:07AM CET, ogerlitz-smomgflXvOZWk0Htik3J/w@public.gmane.org wrote:
>> >Jiri Pirko wrote:
>> >> +++ b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
>> >> @@ -767,11 +767,8 @@ void ipoib_mcast_dev_flush(struct net_device *dev)
>> >> -static int ipoib_mcast_addr_is_valid(const u8 *addr, unsigned int addrlen,
>> >> -				     const u8 *broadcast)
>> >> +static int ipoib_mcast_addr_is_valid(const u8 *addr, const u8 *broadcast)
>> >>  {
>> >> -	if (addrlen != INFINIBAND_ALEN)
>> >> -		return 0;
>> >
>> >This check was added by commit 5e47596b "IPoIB: Check multicast address format", may I ask what is the reason for removing it now?
>> 
>> Yes, at this very moment the check is not needless but it will be in a brief
>> future. dev_mc_add will look very similar like dev_unicast_add. But ok. Here's
>> patch adding the check in dev_mc_add right now to correct this state. Thanks Or.
>
>Is this enough?
>
>The problem this statement is trying to solve had to do with bonding
>creating multicast addresess for ethernet rather than infiniband in
>some cases. This happens because bonding makes a device that switches
>from ethernet to infiniband during its lifetime. I'm not quite
>sure what kind of life cycle those addresses go through, but if they
>somehow stay on the mc_list then ipoib_mcast_addr_is_valid will still
>need to have the check.
>

Ok I see your point now. But in this case, the length check should be in all
drivers because if for example bonding device changes from infiniband back to
eth, the addresses stored would be "cut" to 6 bytes and would make no sense.

I see two solutions:
1) check the length in all drivers to ensure correctness.
2) when bonding changes it's type, flush mc addresses and start over.

Second option looks nicer to me, but I might be missing something, thouths?

Thanks

Jirka

>Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* SV: [PATCH net-next-2.6 v4 02/12] net-caif: add CAIF socket and configuration headers
From: Sjur Brændeland @ 2010-02-27 10:36 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: netdev, davem, Daniel Martensson, kaber, stefano.babic,
	randy.dunlap
In-Reply-To: <1267226115.18491.71.camel@violet>

Hi Marcel.
Thank you for your feedback.

Marcel Holtmann wrote:
>Hi Sjur,
>
>I think most issues have been resolved and this should be ready for
>merging, but I am bit worried about the userspace API. Can we start a
>bit smaller and extend it later? Especially the socket options worry me
>a bit.

>Dave, personally I would prefer if we can merge this without these
>socket options. Since I am really missing the need for it.

As mentioned in previous mail to Dave, I will be off traveling,
so I will not be able to send out anything new in the next four days.
Which means that we will miss the next pull of net-next-2.6.

>> +/**
>> + * enum caif_socket_opts - CAIF option values for getsockopt and setsockopt.
>> + *
>> + * @CAIFSO_LINK_SELECT:		Selector used if multiple CAIF Link layers are
>> + *				available. Either a high bandwidth
>> + *				link can be selected (CAIF_LINK_HIGH_BANDW) or
>> + *				or a low latency link (CAIF_LINK_LOW_LATENCY).
>> + *                              This option is of type u_int32_t.
>> + *				Alternatively SO_BINDTODEVICE can be used.
>> + *
>> + * @CAIFSO_REQ_PARAM:		Used to set the request parameters for a
>> + *				utility channel. (struct caif_param). This
>> + *				option must be set before connecting.
>> + *
>> + * @CAIFSO_RSP_PARAM:		Gets the request parameters for a utility

It is a typo in the documentation here, s/request/response/. Sorry for the confusion.

>> + *				channel. (struct caif_param). This option
>> + *				is valid after a successful connect.
>
>These two more look like a combination of setsockopt/getsockopt instead
>of two socket options. Maybe it is leftover from a ioctl interface, but
>socket options work differently.

No the REQ_PARAM and RSP_PARAM are not just reading/writing the same option.
The CAIF protocol defines "utility channels". This channels are "pipes" between
processes on the modem and host side.
The CAIF protocol defines extra request parameters (REQ_PARAM) for Utility
channels that can be sent from the client to the server in the connect request.
The server may also send response parameters (RSP_PARAM) in the connect response message. 
These socket options are used for setting the request parameters and reading the response 
parameters.

I think we need these socket options, otherwise we would not be able to support
the CAIF Utility Links.
>
>Also the caif_param struct seems pointless. Socket options contain a
>length parameter anyway. So why bother with a struct that is just a data
>field and a length field.

Yes, I see your point here, I could have skipped this type.
>
>> + * @CAIFSO_CHANNEL_ID:		Gets the channel id on a CAIF Channel.
>> + *				This option is valid after a successful connect.
>> + *				( u_int32_t)
>
>Where is this used and what is it used for? Is this something that
>shouldn't be better part of the sockaddr structure. Then you can use
>getpeername for it?

CAIF on the modem side generates unique channel IDs for each CAIF Channel.
This ID identifies the CAIF Channel both on Host and Modem side.
This socket option gives the Linux Side client a possibility to get hold
of this client id. 
This would typically be used for application logging purposes in order to be able to
correlate host side logs and modem side logs. I could move this to debugfs...

Personally I would prefer to keep it, but it could be skipped.

>> + * @CAIFSO_NEXT_PAKCET_LEN:	Gets the size of next received packet.
>> + *				Value is 0 if no packet is available.
>> + *				This option is valid after a successful connect.
>> + *				( u_int32_t)
>Typo. And why do we need this?

This would be used by a client to see the size of the next message to read, allowing
the client to allocate a buffer of the correct size.
I agree that this option is not vital.

>> + * @CAIFSO_MAX_PAKCET_LEN:	Gets the maximum packet size for this
>> + *				connection. ( u_int32_t)
>Isn't this more like SO_RCVBUF or SO_SNDBUF.

CAIF protocol on modem side has a limit on one page size (4096) on the link layer.
However different CAIF Channel types and Link Layers will result in different
maximum sizes for each CAIF Channel. This options allows client to see the 
maximum CAIF packet size can be used in sendmsg. I guess the SO_SNDBUF would have
slightly different semantic, describing the maximum number of bytes in the hosts send queue.

If we in the future change this protocol limitation, it would be nice for the host client to
have this information dynamically pr channel instead of constants in the clients source code.

BR/Sjur






^ permalink raw reply

* Re: [net-next-2.6 PATCH] infiniband: convert to use netdev_for_each_mc_addr
From: David Miller @ 2010-02-27 10:44 UTC (permalink / raw)
  To: jpirko; +Cc: jgunthorpe, ogerlitz, netdev, linux-rdma
In-Reply-To: <20100227103835.GA2862@psychotron.redhat.com>

From: Jiri Pirko <jpirko@redhat.com>
Date: Sat, 27 Feb 2010 11:38:37 +0100

> I see two solutions:
> 1) check the length in all drivers to ensure correctness.
> 2) when bonding changes it's type, flush mc addresses and start over.
> 
> Second option looks nicer to me, but I might be missing something, thouths?

If bonding is causing this problem, perhaps we should make bonding
go through a programmatic interface to do it's work so that the
check can be done generically and in a place that will get
caught during code changes and audits.

^ permalink raw reply

* [PATCH 02/49] pcmcia: remove remaining unused IRQ_FIRST_SHARED parameter
From: Dominik Brodowski @ 2010-02-27 10:41 UTC (permalink / raw)
  To: linux-pcmcia; +Cc: Dominik Brodowski, netdev, Komuro
In-Reply-To: <20100227103934.GA4430@comet.dominikbrodowski.net>

Komuro pointed out correctly that I missed one IRQ_FIRST_SHARED
parameter in smc91c92_cs.c, and that another line could be writter
more beautifully.

CC: netdev@vger.kernel.org
CC: Komuro <komurojun-mbn@nifty.com>
Tested-by: Wolfram Sang <w.sang@pengutronix.de>
Signed-off-by: Dominik Brodowski <linux@dominikbrodowski.net>
---
 drivers/net/pcmcia/smc91c92_cs.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/pcmcia/smc91c92_cs.c b/drivers/net/pcmcia/smc91c92_cs.c
index 6dd486d..aa57cfd 100644
--- a/drivers/net/pcmcia/smc91c92_cs.c
+++ b/drivers/net/pcmcia/smc91c92_cs.c
@@ -453,8 +453,7 @@ static int mhz_mfc_config(struct pcmcia_device *link)
 
     link->conf.Attributes |= CONF_ENABLE_SPKR;
     link->conf.Status = CCSR_AUDIO_ENA;
-    link->irq.Attributes =
-	IRQ_TYPE_DYNAMIC_SHARING;
+    link->irq.Attributes = IRQ_TYPE_DYNAMIC_SHARING;
     link->io.IOAddrLines = 16;
     link->io.Attributes2 = IO_DATA_PATH_WIDTH_8;
     link->io.NumPorts2 = 8;
@@ -652,8 +651,7 @@ static int osi_config(struct pcmcia_device *link)
 
     link->conf.Attributes |= CONF_ENABLE_SPKR;
     link->conf.Status = CCSR_AUDIO_ENA;
-    link->irq.Attributes =
-	IRQ_TYPE_DYNAMIC_SHARING|IRQ_FIRST_SHARED;
+    link->irq.Attributes = IRQ_TYPE_DYNAMIC_SHARING;
     link->io.NumPorts1 = 64;
     link->io.Attributes2 = IO_DATA_PATH_WIDTH_8;
     link->io.NumPorts2 = 8;
-- 
1.6.3.3


^ permalink raw reply related

* Re: net: rtnetlink: support specifying device flags on device creation
From: David Miller @ 2010-02-27 10:51 UTC (permalink / raw)
  To: kaber; +Cc: netdev
In-Reply-To: <20100226163449.11434.13013.sendpatchset@x2.localnet>

From: Patrick McHardy <kaber@trash.net>
Date: Fri, 26 Feb 2010 17:34:49 +0100 (MET)

> The following patches add support to specify the device flags (like UP) when
> creating a new device through rtnl_link. This requires to surpress netlink
> notifications until the device is fully configured in order to not confuse
> userspace when changing the flags fails and registration has to be undone.
> Once the device is configured, a single NEWLINK message with the full state
> is sent.
> 
> The individual patch changelogs describe the necessary changes in more detail.

All applied, but two things:

1) This patch set was harder to review because there were no
   default initializations of the new enumeration you added
   to struct netdev.

   I know RTNL_LINK_INITIALIZED is probably zero by C enumeration
   rules, and the zeroing out of new netdev objects gives us this,
   but I only figured that out after some time.

   It deserved at least a commit message mention in patch #2.

2) I would really appreciate you forming your patch postings
   properly.  I have to edit them every single time

   You put the whole output of "git show" or "git format-patch" into
   your email body.  That doesn't work, we don't want all of those
   commit ID etc. lines in there.  It also causes every line of your
   commit messages to be indented by 4 spaces.

   Your email body should just contain the unindented commit message
   and the signoffs, then the patch itself.

   Your Subject lines are also not setup properly.  Because the
   "net X/N:" thing isn't in [] brackets, it ends up in the
   commit message header lines when I feed your emails to
   "git am".

   All of this could be avoided if you used git send-email but
   I realize that a lot of people dislike that for one reason
   or another (myself included), but if you're going to compose
   the emails by hand you ought to make it look the same (syntax
   wise) as what git send-email would have emitted.

Thanks!

^ permalink raw reply

* Re: bonding: fix device leak on error in bond_create()
From: David Miller @ 2010-02-27 10:52 UTC (permalink / raw)
  To: kaber; +Cc: netdev, fubar
In-Reply-To: <4B87EA58.7090601@trash.net>

From: Patrick McHardy <kaber@trash.net>
Date: Fri, 26 Feb 2010 16:35:52 +0100

> commit f357a5caa0ca29b74811a0fc08efb5ae4aade959
> Author: Patrick McHardy <kaber@trash.net>
> Date:   Thu Feb 25 20:21:10 2010 +0100
> 
>     bonding: fix device leak on error in bond_create()
>     
>     When the register_netdevice() call fails, the newly allocated device is
>     not freed.
>     
>     Signed-off-by: Patrick McHardy <kaber@trash.net>

I think whether we should be checking for negative return values is
not something to gripe over, so I've applied this, thanks!

^ permalink raw reply

* TCP ACK loop with 2.6.33-rc6
From: Simon Arlott @ 2010-02-27 11:01 UTC (permalink / raw)
  To: netdev

[-- Attachment #1: Type: text/plain, Size: 3533 bytes --]

Packet capture attached. Is it possible to tell which side is at fault?
They are both responding to each other's ACK with another ACK at 400pps.

10:32:28.991111 IP6 2001:8b0:ffea:0:205:b4ff:fe12:530.35545 > 2001:500:b::1.53: . ack 766 win 58 <nop,nop,timestamp 2411331059 251057709>
10:32:28.995046 IP6 2001:500:b::1.53 > 2001:8b0:ffea:0:205:b4ff:fe12:530.35545: . ack 1 win 1559
10:32:28.995092 IP6 2001:8b0:ffea:0:205:b4ff:fe12:530.35545 > 2001:500:b::1.53: . ack 766 win 58 <nop,nop,timestamp 2411331063 251057709>
10:32:29.009052 IP6 2001:500:b::1.53 > 2001:8b0:ffea:0:205:b4ff:fe12:530.35545: . ack 1 win 1559
10:32:29.009112 IP6 2001:8b0:ffea:0:205:b4ff:fe12:530.35545 > 2001:500:b::1.53: . ack 766 win 58 <nop,nop,timestamp 2411331077 251057709>
10:32:29.010046 IP6 2001:500:b::1.53 > 2001:8b0:ffea:0:205:b4ff:fe12:530.35545: . ack 1 win 1559
10:32:29.010093 IP6 2001:8b0:ffea:0:205:b4ff:fe12:530.35545 > 2001:500:b::1.53: . ack 766 win 58 <nop,nop,timestamp 2411331078 251057709>
10:32:29.014039 IP6 2001:500:b::1.53 > 2001:8b0:ffea:0:205:b4ff:fe12:530.35545: . ack 1 win 1559
10:32:29.014087 IP6 2001:8b0:ffea:0:205:b4ff:fe12:530.35545 > 2001:500:b::1.53: . ack 766 win 58 <nop,nop,timestamp 2411331082 251057709>
10:32:29.018052 IP6 2001:500:b::1.53 > 2001:8b0:ffea:0:205:b4ff:fe12:530.35545: . ack 1 win 1559
10:32:29.018129 IP6 2001:8b0:ffea:0:205:b4ff:fe12:530.35545 > 2001:500:b::1.53: . ack 766 win 58 <nop,nop,timestamp 2411331086 251057709>
10:32:29.021053 IP6 2001:500:b::1.53 > 2001:8b0:ffea:0:205:b4ff:fe12:530.35545: . ack 1 win 1559
10:32:29.021097 IP6 2001:8b0:ffea:0:205:b4ff:fe12:530.35545 > 2001:500:b::1.53: . ack 766 win 58 <nop,nop,timestamp 2411331089 251057709>

The latency to 2001:500:b::1 is about 45ms (another ACK arrives before
the response to the previous ACK could have been received).

The trailing 0 bytes on packets going to 2001:8b0:ffea:0:205:b4ff:fe12:530
have been added by my ISP to avoid a Cisco bug.

net.ipv4.tcp_timestamps = 1
net.ipv4.tcp_window_scaling = 1
net.ipv4.tcp_sack = 1
net.ipv4.tcp_retrans_collapse = 1
net.ipv4.tcp_syn_retries = 5
net.ipv4.tcp_synack_retries = 5
net.ipv4.tcp_max_orphans = 131072
net.ipv4.tcp_max_tw_buckets = 180000
net.ipv4.tcp_keepalive_time = 7200
net.ipv4.tcp_keepalive_probes = 9
net.ipv4.tcp_keepalive_intvl = 75
net.ipv4.tcp_retries1 = 3
net.ipv4.tcp_retries2 = 15
net.ipv4.tcp_fin_timeout = 60
net.ipv4.tcp_syncookies = 1
net.ipv4.tcp_tw_recycle = 0
net.ipv4.tcp_abort_on_overflow = 0
net.ipv4.tcp_stdurg = 0
net.ipv4.tcp_rfc1337 = 0
net.ipv4.tcp_max_syn_backlog = 1024
net.ipv4.tcp_orphan_retries = 0
net.ipv4.tcp_fack = 1
net.ipv4.tcp_reordering = 50
net.ipv4.tcp_ecn = 1
net.ipv4.tcp_dsack = 1
net.ipv4.tcp_mem = 82752	110336	165504
net.ipv4.tcp_wmem = 262144	1048576	4194304
net.ipv4.tcp_rmem = 262144	1048576	4194304
net.ipv4.tcp_app_win = 31
net.ipv4.tcp_adv_win_scale = 2
net.ipv4.tcp_tw_reuse = 0
net.ipv4.tcp_frto = 2
net.ipv4.tcp_frto_response = 0
net.ipv4.tcp_low_latency = 0
net.ipv4.tcp_no_metrics_save = 0
net.ipv4.tcp_moderate_rcvbuf = 1
net.ipv4.tcp_tso_win_divisor = 3
net.ipv4.tcp_congestion_control = cubic
net.ipv4.tcp_abc = 0
net.ipv4.tcp_mtu_probing = 0
net.ipv4.tcp_base_mss = 512
net.ipv4.tcp_workaround_signed_windows = 0
net.ipv4.tcp_slow_start_after_idle = 1
net.ipv4.tcp_available_congestion_control = cubic reno bic westwood highspeed hybla htcp vegas veno scalable lp yeah illinois
net.ipv4.tcp_allowed_congestion_control = cubic reno
net.ipv4.tcp_max_ssthresh = 0
net.ipv4.tcp_cookie_size = 0

-- 
Simon Arlott

[-- Attachment #2: c0.org.afilias-nst.info_ack_loop2.pcap.bz2 --]
[-- Type: application/x-bzip, Size: 64165 bytes --]

^ permalink raw reply

* Re: [PATCH v3 0/7] pci: Add VPD parsing support
From: David Miller @ 2010-02-27 11:08 UTC (permalink / raw)
  To: mcarlson; +Cc: jbarnes, linux-pci, netdev, andy
In-Reply-To: <1267229085-21303-1-git-send-email-mcarlson@broadcom.com>

From: "Matt Carlson" <mcarlson@broadcom.com>
Date: Fri, 26 Feb 2010 16:04:38 -0800

> This patch adds code to help parse a device's VPD.  The bnx2 and tg3
> network drivers are modified as an example on how to use the API.  I
> believe the bnx2x driver will also use this API in the future.

PCI folks, ok with this?

And if so, OK if I merge this via the net-next-2.6 tree?

Thanks!

^ permalink raw reply

* Re: TCP ACK loop with 2.6.33-rc6
From: Ilpo Järvinen @ 2010-02-27 11:13 UTC (permalink / raw)
  To: Simon Arlott; +Cc: netdev
In-Reply-To: <4B88FB88.3050902@simon.arlott.org.uk>

On Sat, 27 Feb 2010, Simon Arlott wrote:

> Packet capture attached. Is it possible to tell which side is at fault?
> They are both responding to each other's ACK with another ACK at 400pps.
> 
> 10:32:28.991111 IP6 2001:8b0:ffea:0:205:b4ff:fe12:530.35545 > 2001:500:b::1.53: . ack 766 win 58 <nop,nop,timestamp 2411331059 251057709>
> 10:32:28.995046 IP6 2001:500:b::1.53 > 2001:8b0:ffea:0:205:b4ff:fe12:530.35545: . ack 1 win 1559

Why only a single ends talks with timestamps?!?!?

-- 
 i.

^ permalink raw reply

* Re: [PATCH 2/13] bridge: Allow tail-call on br_pass_frame_up
From: David Miller @ 2010-02-27 11:14 UTC (permalink / raw)
  To: herbert; +Cc: netdev, shemminger
In-Reply-To: <E1Nl2Db-0006v3-21@gondolin.me.apana.org.au>

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Fri, 26 Feb 2010 23:35:07 +0800

> @@ -20,9 +20,9 @@
>  /* Bridge group multicast address 802.1d (pg 51). */
>  const u8 br_group_address[ETH_ALEN] = { 0x01, 0x80, 0xc2, 0x00, 0x00, 0x00 };
>  
> -static void br_pass_frame_up(struct net_bridge *br, struct sk_buff *skb)
> +static int br_pass_frame_up(struct sk_buff *skb)
>  {
> -	struct net_device *indev, *brdev = br->dev;
> +	struct net_device *indev, *brdev = BR_INPUT_SKB_CB(skb)->brdev;
>  

You use this new BR_INPUT_SKB_CB() here in patch #2, but you only
start setting ->brdev it in patch #4.

This breaks things and makes your patch series non-bisectable.

Please fix, thanks.

^ permalink raw reply

* Re: [PATCH v3] skbuff: align sk_buff::cb to 64 bit and close some potential holes
From: David Miller @ 2010-02-27 11:17 UTC (permalink / raw)
  To: nbd; +Cc: ddaney, eric.dumazet, netdev, buytenh
In-Reply-To: <4B844C8F.2060908@openwrt.org>

From: Felix Fietkau <nbd@openwrt.org>
Date: Tue, 23 Feb 2010 22:45:51 +0100

> The alignment requirement for 64-bit load/store instructions on ARM is
> implementation defined. Some CPUs (such as Marvell Feroceon) do not
> generate an exception, if such an instruction is executed with an
> address that is not 64 bit aligned. In such a case, the Feroceon
> corrupts adjacent memory, which showed up in my tests as a crash in the
> rx path of ath9k that only occured with CONFIG_XFRM set.
> 
> This crash happened, because the first field of the mac80211 rx status
> info in the cb is an u64, and changing it corrupted the skb->sp field.
> 
> This patch also closes some potential pre-existing holes in the sk_buff
> struct surrounding the cb[] area.
> 
> Signed-off-by: Felix Fietkau <nbd@openwrt.org>
> Cc: stable@kernel.org

Applied, thanks for following up on this Felix.

^ permalink raw reply

* Re: [net-next-2.6 PATCH v2] net: consolidate netif_needs_gso() checks
From: David Miller @ 2010-02-27 11:27 UTC (permalink / raw)
  To: jeffrey.t.kirsher; +Cc: netdev, gospo, john.r.fastabend, herbert
In-Reply-To: <20100227001954.32474.62766.stgit@localhost.localdomain>

From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Fri, 26 Feb 2010 16:20:11 -0800

> From: John Fastabend <john.r.fastabend@intel.com>
> 
> netif_needs_gso() is checked twice in the TX path once,
> before submitting the skb to the qdisc and once after
> it is dequeued from the qdisc just before calling
> ndo_hard_start().  This opens a window for a user to
> change the gso/tso or tx checksum settings that can
> cause netif_needs_gso to be true in one check and false
> in the other.
> 
> Specifically, changing TX checksum setting may cause
> the warning in skb_gso_segment() to be triggered if
> the checksum is calculated earlier.
> 
> This consolidates the netif_needs_gso() calls so that
> the stack only checks if gso is needed in
> dev_hard_start_xmit().
> 
> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

This looks mostly fine, but I have at least one doubt.

If we have ip_summed == CHECKSUM_PARTIAL might some classifier
or packet scheduler action module require that the
transport header is setup properly before the SKB gets into
there?

Arguably, that's happening already in the GSO case but this
change is bringing the issue more to light.

Herbert, could you also take a look at this patch?

Thanks!

> diff --git a/net/core/dev.c b/net/core/dev.c
> index eb7f1a4..626124d 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -1835,12 +1835,40 @@ int dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev,
>  {
>  	const struct net_device_ops *ops = dev->netdev_ops;
>  	int rc = NETDEV_TX_OK;
> +	int need_gso = netif_needs_gso(dev, skb);
> +
> +	if (!need_gso) {
> +		if (skb_has_frags(skb) &&
> +		    !(dev->features & NETIF_F_FRAGLIST) &&
> +		    __skb_linearize(skb))
> +			goto out_kfree_skb;
> +
> +		/* Fragmented skb is linearized if device does not support SG,
> +		 * or if at least one of fragments is in highmem and device
> +		 * does not support DMA from it.
> +		 */
> +		if (skb_shinfo(skb)->nr_frags &&
> +		    (!(dev->features & NETIF_F_SG) ||
> +		      illegal_highdma(dev, skb)) &&
> +		    __skb_linearize(skb))
> +			goto out_kfree_skb;
> +		/* If packet is not checksummed and device does not support
> +		 * checksumming for this protocol, complete checksumming here.
> +		 */
> +		if (skb->ip_summed == CHECKSUM_PARTIAL) {
> +			skb_set_transport_header(skb, skb->csum_start -
> +				      skb_headroom(skb));
> +			if (!dev_can_checksum(dev, skb) &&
> +			     skb_checksum_help(skb))
> +				goto out_kfree_skb;
> +		}
> +	}
>  
>  	if (likely(!skb->next)) {
>  		if (!list_empty(&ptype_all))
>  			dev_queue_xmit_nit(skb, dev);
>  
> -		if (netif_needs_gso(dev, skb)) {
> +		if (need_gso) {
>  			if (unlikely(dev_gso_segment(skb)))
>  				goto out_kfree_skb;
>  			if (skb->next)
> @@ -2056,25 +2084,6 @@ int dev_queue_xmit(struct sk_buff *skb)
>  	struct Qdisc *q;
>  	int rc = -ENOMEM;
>  
> -	/* GSO will handle the following emulations directly. */
> -	if (netif_needs_gso(dev, skb))
> -		goto gso;
> -
> -	/* Convert a paged skb to linear, if required */
> -	if (skb_needs_linearize(skb, dev) && __skb_linearize(skb))
> -		goto out_kfree_skb;
> -
> -	/* If packet is not checksummed and device does not support
> -	 * checksumming for this protocol, complete checksumming here.
> -	 */
> -	if (skb->ip_summed == CHECKSUM_PARTIAL) {
> -		skb_set_transport_header(skb, skb->csum_start -
> -					      skb_headroom(skb));
> -		if (!dev_can_checksum(dev, skb) && skb_checksum_help(skb))
> -			goto out_kfree_skb;
> -	}
> -
> -gso:
>  	/* Disable soft irqs for various locks below. Also
>  	 * stops preemption for RCU.
>  	 */
> @@ -2133,7 +2142,6 @@ gso:
>  	rc = -ENETDOWN;
>  	rcu_read_unlock_bh();
>  
> -out_kfree_skb:
>  	kfree_skb(skb);
>  	return rc;
>  out:
> 


^ permalink raw reply

* Re: [net-next-2.6 PATCH] ixgbe: move TC_PRIO_CONTROL check into ixgbe_select_queue()
From: David Miller @ 2010-02-27 11:28 UTC (permalink / raw)
  To: jeffrey.t.kirsher; +Cc: netdev, gospo, john.r.fastabend, peter.p.waskiewicz.jr
In-Reply-To: <20100227002135.32576.23757.stgit@localhost.localdomain>

From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Fri, 26 Feb 2010 16:21:36 -0800

> From: John Fastabend <john.r.fastabend@intel.com>
> 
> Move TC_PRIO_CONTROL check and queue remapping into
> ixgbe_select_queue().  Remapping queues after the qdisc
> can result in the wrong qdisc queue being stopped with
> netif_stop_subqueue().  Even if this is resolved and the
> correct queue is stopped it can result in a queue being
> blocked by TC_PRIO_CONTROL frames uneccesarily.  Moving
> this into the select_queue routine maintains alignment
> between tx_rings and qdisc queues.
> 
> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
> Acked-by: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH 10/13] bridge: Add multicast_router sysfs entries
From: David Miller @ 2010-02-27 11:29 UTC (permalink / raw)
  To: shemminger; +Cc: herbert, netdev
In-Reply-To: <20100226164211.1a02a31b@nehalam>

From: Stephen Hemminger <shemminger@vyatta.com>
Date: Fri, 26 Feb 2010 16:42:11 -0800

> I like the functionality, but don't like users whacking on sysfs
> directly. Could you send patches to integrate a user API into
> bridge-utils; the utils are at: 
>   git://git.kernel.org/pub/scm/linux/kernel/git/shemminger/bridge-utils.git

Sounds reasonable to me.

Herbert, please do this after we've resolved the issues in your
patch set and integrated it.

Thanks!

^ permalink raw reply

* Re: [net-next PATCH v6 0/3] net: reserve ports for applications using fixed port numbers
From: David Miller @ 2010-02-27 11:32 UTC (permalink / raw)
  To: opurdila; +Cc: netdev, linux-kernel, nhorman, eric.dumazet, ebiederm, amwang
In-Reply-To: <1267233952-5856-1-git-send-email-opurdila@ixiacom.com>


Eric B., could you look over the first two patches (which touch the
sysctl core) and give some review and ACK/NACK?

Thanks!

^ permalink raw reply

* Re: [RFC PATCH] fix problems with NETIF_F_HIGHDMA in networking drivers
From: Bartlomiej Zolnierkiewicz @ 2010-02-27 11:59 UTC (permalink / raw)
  To: David Miller; +Cc: hancockrwd, linux-kernel, netdev, linux-usb
In-Reply-To: <20100227.015350.71138134.davem@davemloft.net>

On Saturday 27 February 2010 10:53:50 am David Miller wrote:
> From: Robert Hancock <hancockrwd@gmail.com>
> Date: Fri, 26 Feb 2010 21:08:04 -0600
> 
> > That seems like a reasonable approach to me. Only question is how to
> > implement the check for DMA_64BIT. Can we just check page_to_phys on
> > each of the pages in the skb to see if it's > 0xffffffff ? Are there
> > any architectures where it's more complicated than that?
> 
> On almost every platform it's "more complicated than that".

Mildly speaking, I see the real problem now and it is much higher in
the software stack than networking..

> This is the whole issue.  What matters is the final DMA address and
> since we have IOMMUs and the like, it is absolutely not tenable to
> solve this by checking physical address attributes.

What's more we may not have IOMMU in place which creates really interesting
scenarios for HIGHMEM=y and results in all kind of wonderful band-aids in
particular device drivers.

Having IOMMU (even if it is only a software one, i.e. this would mean
swiotlb for x86-32/highmem) always in place would simplify things greatly..

--
Bartlomiej Zolnierkiewicz

^ permalink raw reply

* Re: [RFC PATCH] fix problems with NETIF_F_HIGHDMA in networking drivers
From: David Miller @ 2010-02-27 12:05 UTC (permalink / raw)
  To: bzolnier; +Cc: hancockrwd, linux-kernel, netdev, linux-usb
In-Reply-To: <201002271259.31596.bzolnier@gmail.com>

From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Date: Sat, 27 Feb 2010 12:59:31 +0100

> Having IOMMU (even if it is only a software one, i.e. this would
> mean swiotlb for x86-32/highmem) always in place would simplify
> things greatly..

I agree, things would be a lot simpler.

^ permalink raw reply

* [PATCH net-next-2.6 1/2] mq: support for bonding
From: "Oleg A. Arkhangelsky" @ 2010-02-27 13:27 UTC (permalink / raw)
  To: David Miller; +Cc: netdev



Make bonding driver multiqueue aware.

Signed-off-by: Oleg A. Arkhangelsky <sysoleg@yandex.ru> 

---

 drivers/net/bonding/bond_main.c |    5 +++--
 drivers/net/bonding/bonding.h   |    1 +
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 430c022..ea4ff33 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -4928,8 +4928,9 @@ int bond_create(struct net *net, const char *name)
 
 	rtnl_lock();
 
-	bond_dev = alloc_netdev(sizeof(struct bonding), name ? name : "",
-				bond_setup);
+	bond_dev = alloc_netdev_mq(sizeof(struct bonding), name ? name : "",
+				bond_setup,
+				min_t(u32, BOND_MAX_TX_QUEUES, num_online_cpus()));
 	if (!bond_dev) {
 		pr_err("%s: eek! can't alloc netdev!\n", name);
 		res = -ENOMEM;
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index 257a7a4..4a6cfb4 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -29,6 +29,7 @@
 #define DRV_DESCRIPTION	"Ethernet Channel Bonding Driver"
 
 #define BOND_MAX_ARP_TARGETS	16
+#define BOND_MAX_TX_QUEUES	8 
 
 #define IS_UP(dev)					   \
 	      ((((dev)->flags & IFF_UP) == IFF_UP)	&& \

---

-- 
wbr, Oleg.

^ permalink raw reply related

* [PATCH net-next-2.6 2/2] mq: preserve queue mapping with bonding and VLAN devices
From: "Oleg A. Arkhangelsky" @ 2010-02-27 13:28 UTC (permalink / raw)
  To: David Miller; +Cc: netdev



Forwarded packet goes through dev_queue_xmit() more that once
when using bonding or 802.1q VLAN devices, so we've lost rx-tx
queue mapping index for real devices. This is because initial
queue index value (as it recorded by skb_record_tx_queue()) is
overwritten by skb_set_queue_mapping(). 

Signed-off-by: Oleg A. Arkhangelsky <sysoleg@yandex.ru> 

---

 drivers/net/bnx2.c              |    2 +-
 drivers/net/bnx2x_main.c        |    2 +-
 drivers/net/gianfar.c           |    6 +++---
 drivers/net/igb/igb_main.c      |    2 +-
 drivers/net/ixgbe/ixgbe_main.c  |    6 +++---
 drivers/net/mlx4/en_tx.c        |    2 +-
 drivers/net/niu.c               |    2 +-
 drivers/net/qlge/qlge_main.c    |    2 +-
 drivers/net/s2io.c              |    2 +-
 include/linux/skbuff.h          |   14 ++------------
 net/core/dev.c                  |    2 +-
 11 files changed, 16 insertions(+), 26 deletions(-)

diff --git a/drivers/net/bnx2.c b/drivers/net/bnx2.c
index d3f739a..abbbe40 100644
--- a/drivers/net/bnx2.c
+++ b/drivers/net/bnx2.c
@@ -3199,7 +3199,7 @@ bnx2_rx_int(struct bnx2 *bp, struct bnx2_napi *bnapi, int budget)
 				skb->ip_summed = CHECKSUM_UNNECESSARY;
 		}
 
-		skb_record_rx_queue(skb, bnapi - &bp->bnx2_napi[0]);
+		skb_set_queue_mapping(skb, bnapi - &bp->bnx2_napi[0]);
 
 #ifdef BCM_VLAN
 		if (hw_vlan)
diff --git a/drivers/net/bnx2x_main.c b/drivers/net/bnx2x_main.c
index 5adf2a0..6e8e327 100644
--- a/drivers/net/bnx2x_main.c
+++ b/drivers/net/bnx2x_main.c
@@ -1681,7 +1681,7 @@ reuse_rx:
 			}
 		}
 
-		skb_record_rx_queue(skb, fp->index);
+		skb_set_queue_mapping(skb, fp->index);
 
 #ifdef BCM_VLAN
 		if ((bp->vlgrp != NULL) && (bp->flags & HW_VLAN_RX_FLAG) &&
diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
index 6aa526e..d034f4e 100644
--- a/drivers/net/gianfar.c
+++ b/drivers/net/gianfar.c
@@ -1934,7 +1934,7 @@ static int gfar_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	unsigned int nr_frags, length;
 
 
-	rq = skb->queue_mapping;
+	rq = skb_get_queue_mapping(skb);
 	tx_queue = priv->tx_queue[rq];
 	txq = netdev_get_tx_queue(dev, rq);
 	base = tx_queue->tx_bd_base;
@@ -2466,7 +2466,7 @@ static int gfar_process_frame(struct net_device *dev, struct sk_buff *skb,
 	/* Remove the FCB from the skb */
 	/* Remove the padded bytes, if there are any */
 	if (amount_pull) {
-		skb_record_rx_queue(skb, fcb->rq);
+		skb_set_queue_mapping(skb, fcb->rq);
 		skb_pull(skb, amount_pull);
 	}
 
@@ -2549,7 +2549,7 @@ int gfar_clean_rx_ring(struct gfar_priv_rx_q *rx_queue, int rx_work_limit)
 				/* Remove the FCS from the packet length */
 				skb_put(skb, pkt_len);
 				rx_queue->stats.rx_bytes += pkt_len;
-				skb_record_rx_queue(skb, rx_queue->qindex);
+				skb_set_queue_mapping(skb, rx_queue->qindex);
 				gfar_process_frame(dev, skb, amount_pull);
 
 			} else {
diff --git a/drivers/net/igb/igb_main.c b/drivers/net/igb/igb_main.c
index 583a21c..def942c 100644
--- a/drivers/net/igb/igb_main.c
+++ b/drivers/net/igb/igb_main.c
@@ -3838,7 +3838,7 @@ static netdev_tx_t igb_xmit_frame_adv(struct sk_buff *skb,
 		return NETDEV_TX_OK;
 	}
 
-	r_idx = skb->queue_mapping & (IGB_ABS_MAX_TX_QUEUES - 1);
+	r_idx = skb_get_queue_mapping(skb) & (IGB_ABS_MAX_TX_QUEUES - 1);
 	tx_ring = adapter->multi_tx_table[r_idx];
 
 	/* This goes back to the question of how to logically map a tx queue
diff --git a/drivers/net/ixgbe/ixgbe_main.c b/drivers/net/ixgbe/ixgbe_main.c
index 45e3532..0d08eba 100644
--- a/drivers/net/ixgbe/ixgbe_main.c
+++ b/drivers/net/ixgbe/ixgbe_main.c
@@ -5668,17 +5668,17 @@ static netdev_tx_t ixgbe_xmit_frame(struct sk_buff *skb,
 		tx_flags |= vlan_tx_tag_get(skb);
 		if (adapter->flags & IXGBE_FLAG_DCB_ENABLED) {
 			tx_flags &= ~IXGBE_TX_FLAGS_VLAN_PRIO_MASK;
-			tx_flags |= ((skb->queue_mapping & 0x7) << 13);
+			tx_flags |= ((skb_get_queue_mapping(skb) & 0x7) << 13);
 		}
 		tx_flags <<= IXGBE_TX_FLAGS_VLAN_SHIFT;
 		tx_flags |= IXGBE_TX_FLAGS_VLAN;
 	} else if (adapter->flags & IXGBE_FLAG_DCB_ENABLED) {
-		tx_flags |= ((skb->queue_mapping & 0x7) << 13);
+		tx_flags |= ((skb_get_queue_mapping(skb) & 0x7) << 13);
 		tx_flags <<= IXGBE_TX_FLAGS_VLAN_SHIFT;
 		tx_flags |= IXGBE_TX_FLAGS_VLAN;
 	}
 
-	tx_ring = adapter->tx_ring[skb->queue_mapping];
+	tx_ring = adapter->tx_ring[skb_get_queue_mapping(skb)];
 
 	if ((adapter->flags & IXGBE_FLAG_FCOE_ENABLED) &&
 	    (skb->protocol == htons(ETH_P_FCOE))) {
diff --git a/drivers/net/mlx4/en_tx.c b/drivers/net/mlx4/en_tx.c
index 3d1396a..c1bca72 100644
--- a/drivers/net/mlx4/en_tx.c
+++ b/drivers/net/mlx4/en_tx.c
@@ -624,7 +624,7 @@ netdev_tx_t mlx4_en_xmit(struct sk_buff *skb, struct net_device *dev)
 		goto tx_drop;
 	}
 
-	tx_ind = skb->queue_mapping;
+	tx_ind = skb_get_queue_mapping(skb);
 	ring = &priv->tx_ring[tx_ind];
 	if (priv->vlgrp && vlan_tx_tag_present(skb))
 		vlan_tag = vlan_tx_tag_get(skb);
diff --git a/drivers/net/niu.c b/drivers/net/niu.c
index 0678f31..0819cb2 100644
--- a/drivers/net/niu.c
+++ b/drivers/net/niu.c
@@ -3516,7 +3516,7 @@ static int niu_process_rx_pkt(struct napi_struct *napi, struct niu *np,
 	rp->rx_bytes += skb->len;
 
 	skb->protocol = eth_type_trans(skb, np->dev);
-	skb_record_rx_queue(skb, rp->rx_channel);
+	skb_set_queue_mapping(skb, rp->rx_channel);
 	napi_gro_receive(napi, skb);
 
 	return num_rcr;
diff --git a/drivers/net/qlge/qlge_main.c b/drivers/net/qlge/qlge_main.c
index c26ec5d..f645d42 100644
--- a/drivers/net/qlge/qlge_main.c
+++ b/drivers/net/qlge/qlge_main.c
@@ -2525,7 +2525,7 @@ static netdev_tx_t qlge_send(struct sk_buff *skb, struct net_device *ndev)
 	struct ql_adapter *qdev = netdev_priv(ndev);
 	int tso;
 	struct tx_ring *tx_ring;
-	u32 tx_ring_idx = (u32) skb->queue_mapping;
+	u32 tx_ring_idx = (u32) skb_get_queue_mapping(skb);
 
 	tx_ring = &qdev->tx_ring[tx_ring_idx];
 
diff --git a/drivers/net/s2io.c b/drivers/net/s2io.c
index 43bc66a..afdab06 100644
--- a/drivers/net/s2io.c
+++ b/drivers/net/s2io.c
@@ -7549,7 +7549,7 @@ static int rx_osm_handler(struct ring_info *ring_data, struct RxD_t * rxdp)
 
 	swstats->mem_freed += skb->truesize;
 send_up:
-	skb_record_rx_queue(skb, ring_no);
+	skb_set_queue_mapping(skb, ring_no);
 	queue_rx_frame(skb, RXD_GET_VLAN_TAG(rxdp->Control_2));
 aggregate:
 	sp->mac_control.rings[ring_no].rx_bufs_left -= 1;
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index d266eee..dd7cdf4 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -2014,12 +2014,12 @@ static inline void skb_init_secmark(struct sk_buff *skb)
 
 static inline void skb_set_queue_mapping(struct sk_buff *skb, u16 queue_mapping)
 {
-	skb->queue_mapping = queue_mapping;
+	skb->queue_mapping = queue_mapping + 1;
 }
 
 static inline u16 skb_get_queue_mapping(const struct sk_buff *skb)
 {
-	return skb->queue_mapping;
+	return skb_rx_queue_recorded(skb) ? skb->queue_mapping - 1 : 0;
 }
 
 static inline void skb_copy_queue_mapping(struct sk_buff *to, const struct sk_buff *from)
@@ -2027,16 +2027,6 @@ static inline void skb_copy_queue_mapping(struct sk_buff *to, const struct sk_bu
 	to->queue_mapping = from->queue_mapping;
 }
 
-static inline void skb_record_rx_queue(struct sk_buff *skb, u16 rx_queue)
-{
-	skb->queue_mapping = rx_queue + 1;
-}
-
-static inline u16 skb_get_rx_queue(const struct sk_buff *skb)
-{
-	return skb->queue_mapping - 1;
-}
-
 static inline bool skb_rx_queue_recorded(const struct sk_buff *skb)
 {
 	return (skb->queue_mapping != 0);
diff --git a/net/core/dev.c b/net/core/dev.c
index e5972f7..61e56b7 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1938,7 +1938,7 @@ u16 skb_tx_hash(const struct net_device *dev, const struct sk_buff *skb)
 	u32 hash;
 
 	if (skb_rx_queue_recorded(skb)) {
-		hash = skb_get_rx_queue(skb);
+		hash = skb->queue_mapping - 1;
 		while (unlikely(hash >= dev->real_num_tx_queues))
 			hash -= dev->real_num_tx_queues;
 		return hash;

---

-- 
wbr, Oleg.

^ permalink raw reply related

* Re: [patch] zd1211rw: fix potential array underflow
From: Benoit PAPILLAULT @ 2010-02-27 14:20 UTC (permalink / raw)
  To: Dan Carpenter, Daniel Drake, Ulrich Kunitz, John W. Linville,
	Johannes Berg
In-Reply-To: <20100227061234.GA14323@bicker>

Dan Carpenter a écrit :
> The first chunk fixes a debugging assert to print a warning about array underflows.
> The second chunk corrects a potential array underflow.  I also removed an assert
> in the second chunk because it can no longer happen.
>
> Signed-off-by: Dan Carpenter <error27@gmail.com>
> ---
> This was found by a static check and compile tested only.  Please review carefully.
>
> diff --git a/drivers/net/wireless/zd1211rw/zd_mac.c b/drivers/net/wireless/zd1211rw/zd_mac.c
> index f14deb0..ead2f2c 100644
> --- a/drivers/net/wireless/zd1211rw/zd_mac.c
> +++ b/drivers/net/wireless/zd1211rw/zd_mac.c
> @@ -350,7 +350,7 @@ static void zd_mac_tx_status(struct ieee80211_hw *hw, struct sk_buff *skb,
>  	first_idx = info->status.rates[0].idx;
>  	ZD_ASSERT(0<=first_idx && first_idx<ARRAY_SIZE(zd_retry_rates));
>  	retries = &zd_retry_rates[first_idx];
> -	ZD_ASSERT(0<=retry && retry<=retries->count);
> +	ZD_ASSERT(1 <= retry && retry <= retries->count);
>   
Note: normal hardware always report a tx_status->retry >= 1. There are 2 
code paths to initialize retry itself : either tx_status is NULL and 
then retry=1 (so we are safe), or tx_status is not NULL and retry = 
tx_status->retry + success >=1 (so we are safe again).

However, I wonder how we should handle if it happens that the HW reports 
a tx_status->retry = 0. I think ZD_ASSERT purpose is to catch 
programming errors, not bogus hardware. Comments?
>  
>  	info->status.rates[0].idx = retries->rate[0];
>  	info->status.rates[0].count = 1; // (retry > 1 ? 2 : 1);
> @@ -360,7 +360,7 @@ static void zd_mac_tx_status(struct ieee80211_hw *hw, struct sk_buff *skb,
>  		info->status.rates[i].count = 1; // ((i==retry-1) && success ? 1:2);
>  	}
>  	for (; i<IEEE80211_TX_MAX_RATES && i<retry; i++) {
> -		info->status.rates[i].idx = retries->rate[retry-1];
> +		info->status.rates[i].idx = retries->rate[retry - 1];
>  		info->status.rates[i].count = 1; // (success ? 1:2);
>  	}
>  	if (i<IEEE80211_TX_MAX_RATES)
> @@ -424,12 +424,10 @@ void zd_mac_tx_failed(struct urb *urb)
>  		first_idx = info->status.rates[0].idx;
>  		ZD_ASSERT(0<=first_idx && first_idx<ARRAY_SIZE(zd_retry_rates));
>  		retries = &zd_retry_rates[first_idx];
> -		if (retry < 0 || retry > retries->count) {
> +		if (retry <= 0 || retry > retries->count)
>  			continue;
> -		}
>  
> -		ZD_ASSERT(0<=retry && retry<=retries->count);
> -		final_idx = retries->rate[retry-1];
> +		final_idx = retries->rate[retry - 1];
>  		final_rate = zd_rates[final_idx].hw_value;
>  
>  		if (final_rate != tx_status->rate) {
>
>   
Acked-by: Benoit Papillault <benoit.papillault@free.fr>

Regards,
Benoit


^ permalink raw reply

* Re: iproute2: adding /128 address to sit interface borks
From: Jan Engelhardt @ 2010-02-27 15:03 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Stephen Hemminger, netdev
In-Reply-To: <20090710082731.0b06c0eb@nehalam>


On Friday 2009-07-10 17:27, Stephen Hemminger wrote:
>On Fri, 10 Jul 2009 14:05:01 +0200 (CEST)
>Jan Engelhardt <jengelh@medozas.de> wrote:
>
>> it's impossible to do this, and the error message does not say why 
>> that is so. No word in dmesg either.
>> 
>> # ip a a fc00::49/128 peer fc00::7 dev sit7
>> RTNETLINK answers: Invalid argument
>
>You will need to walk through the path in the kernel.
>In worst case, it means adding kernel printk's.
>
>Then if you want to be nice, make a patch to ipaddr.c in iproute
>utilities so command line can find the error before passing to
>kernel and give a nice error :-)

The issue here was that the peer is encoded using the broadcast
field, and since IPv6 has no broadcast, it was rejected by
the kernel. The error messages really need to be improved.

This year also came an article about that,
http://lwn.net/Articles/374794/

^ permalink raw reply


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