Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH 1/1] iwmc3200: add more SDIO device ids
From: Marcel Holtmann @ 2009-08-24 23:21 UTC (permalink / raw)
  To: Tomas Winkler
  Cc: Luis R. Rodriguez, davem-fT/PcQaiUtIeIZ0/mPfg9Q,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA, Greg KH
In-Reply-To: <1ba2fa240908241520q5be2060co145ec0745954dcf0-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

Hi Tomas,

> >> add WiFi BGN SKU and WiMAX 2.4GHz SKU device ids
> >>
> >> Cc:inaky.perez-gonzalez-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org
> >> Cc:cindy.h.kao-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org
> >> Cc:yi.zhu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org
> >> Signed-off-by: Tomas Winkler <tomas.winkler-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> >
> > Since it seems we are preferring to not do this for PCI for cases
> > where the ID will only be used for 1 device perhaps its best to remove
> > all those ids and stuff them directly into the intel driver itself
> > that will use it.
> >
> I don't have strong opinion about it but this is the current habit
> IIRC there was a mail thread that explicitly asked to move SDIO IDs
> there,  of course I cannot locate it right now :(

first thing is that IDs should only be added if there are users for it.
I am missing the patches that are actually using them. Send them all
together.

Second, we have to establish what is the preferred method of IDs for the
SDIO subsystem. In general I would prefer we have a global policy for
this, but in reality it is up to the subsystem maintainers.

My personal vote is for keeping all IDs inside the drivers. And I also
prefer to keep the plain hex values and just put a comment above them
which device this is. Something like this:

static struct usb_device_id btusb_table[] = {
	/* Generic Bluetooth USB device */
	{ USB_DEVICE_INFO(0xe0, 0x01, 0x01) },

	/* AVM BlueFRITZ! USB v2.0 */
	{ USB_DEVICE(0x057c, 0x3800) },

	{ }	/* Terminating entry */
};

Regards

Marcel


--
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: UDP multicast packet loss not reported if TX ring overrun?
From: Eric Dumazet @ 2009-08-24 23:14 UTC (permalink / raw)
  To: Christoph Lameter; +Cc: Sridhar Samudrala, Nivedita Singhvi, netdev
In-Reply-To: <alpine.DEB.1.10.0908171848370.15956@gentwo.org>

Christoph Lameter a écrit :
> On Mon, 17 Aug 2009, Sridhar Samudrala wrote:
> 
>> On Mon, 2009-08-17 at 18:13 -0400, Christoph Lameter wrote:
>>> On Mon, 17 Aug 2009, Sridhar Samudrala wrote:
>>>
>>>> What about ethtool -S ? Does it report any errors?
>>> Neither. This is is a broadcom bnx2 NIC.
>> Are you sure the packets are dropped at the sender?
> 
> Yes I am sending 400k messages from the app and the receiver only gets
> 341k @300 byte (which is the line rate). There is no way that the 400k get
> over the line. Also if I reduce SO_SNDBUF then both receiver and
> sender get down to 341k.
> 
> I added the output of ethtool -S at the end.
> 
> The mcast tool can be had from http://gentwo.org/ll or from my directory
> on www.kernel.org.
> 

# gcc -v
Using built-in specs.
Target: i686-pc-linux-gnu
Configured with: ../gcc-4.4.1/configure --enable-languages=c,c++ --prefix=/usr
Thread model: posix
gcc version 4.4.1 (GCC)
# pwd
/opt/src/lldiag-0.14
# make
gcc -Wall -omcast mcast.c -lrt -lm
mcast.c: In function ‘set_ip’:
mcast.c:121: warning: implicit declaration of function ‘htons’
mcast.c: In function ‘build_pattern_array’:
mcast.c:168: warning: implicit declaration of function ‘htonl’
/tmp/cc4sYCDr.o: In function `lock':
mcast.c:(.text+0xcad): undefined reference to `__sync_fetch_and_add_4'
collect2: ld returned 1 exit status
make: *** [mcast] Error 1


I have no idea where is defined sync_fetch_and_add

Thanks

^ permalink raw reply
* Re: [PATCH 1/1] iwmc3200: add more SDIO device ids
From: Bob Copeland @ 2009-08-24 22:37 UTC (permalink / raw)
  To: Tomas Winkler
  Cc: Luis R. Rodriguez, davem-fT/PcQaiUtIeIZ0/mPfg9Q,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA, Greg KH
In-Reply-To: <1ba2fa240908241520q5be2060co145ec0745954dcf0-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Mon, Aug 24, 2009 at 6:20 PM, Tomas Winkler<tomasw-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> I don't have strong opinion about it but this is the current habit
> IIRC there was a mail thread that explicitly asked to move SDIO IDs
> there,  of course I cannot locate it right now :(

This one:
http://marc.info/?l=linux-wireless&m=124489211911222&w=2

Of course, Pierre is no longer maintainer and there are only a few
IDs there, so I don't think anyone will shout either way.

-- 
Bob Copeland %% www.bobcopeland.com
--
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: UDP multicast packet loss not reported if TX ring overrun?
From: Sridhar Samudrala @ 2009-08-24 22:36 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Christoph Lameter, Nivedita Singhvi, netdev, David S. Miller
In-Reply-To: <4A930DEF.5000008@gmail.com>

On Tue, 2009-08-25 at 00:02 +0200, Eric Dumazet wrote:
> Christoph Lameter a écrit :
> > On Mon, 17 Aug 2009, Sridhar Samudrala wrote:
> > 
> >> So it is possible that there is some other place in the stack where the packets
> >> are gettting dropped but not counted.
> > 
> > Such a deed occurs in ip_push_pending_frames():
> > 
> >         /* Netfilter gets whole the not fragmented skb. */
> >         err = ip_local_out(skb);
> >         if (err) {
> >                 if (err > 0)
> >                         err = inet->recverr ? net_xmit_errno(err) : 0;
> > 			^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> >                 if (err)
> >                         goto error;
> >         }
> > 
> > out:
> >         ip_cork_release(inet);
> >         return err;
> > 
> > error:
> >         IP_INC_STATS(net, IPSTATS_MIB_OUTDISCARDS);
> >         goto out;
> > 
> > 
> > So if ip_local_out returns NET_XMIT_DROP then its simply going to be
> > replaced by 0. Then we check err again and there is no error!!!!

Christoph,

So are you hitting this case with your workload and does this account for all the
packet losses you are seeing? 

If we are dropping the packet and returing NET_XMIT_DROP, should
we also increment qdisc drop stats (sch->qstats.drops)?

In dev_queue_xmit(), we have

        if (q->enqueue) {
                spinlock_t *root_lock = qdisc_lock(q);

                spin_lock(root_lock);

                if (unlikely(test_bit(__QDISC_STATE_DEACTIVATED, &q->state))) {
                        kfree_skb(skb);
                        rc = NET_XMIT_DROP;
                } else {
                        rc = qdisc_enqueue_root(skb, q);
                        qdisc_run(q);
                }
                spin_unlock(root_lock);

                goto out;
        }

Here, if QDISC_STATE_DEACTIVATED is true, the skb is dropped and NET_XMIT_DROP
is returned, but not accounted in qdisc drop stats.
However it is incremented when NET_XMIT_DROP is returned via qdisc_drop().

If we count these drops as qdisc drops, should we also count them as IP OUTDISCARDS?

Thanks
Sridhar

> > 
> NET_XMIT_CN strikes again :)
> 
> Well, if ip_local_out() returns a negative error (say -EPERM for example),
>  your patch disables OUTDISCARDS increments.
> 
> Maybe a simpler patch like this one ?
> 
> [PATCH] net: correctly updates OUTDISCARDS in ip_push_pending_frames()
> 
> ip_push_pending_frames() can fail to send a frame because of a congestioned
> device. In this case, we increment SNMP OUTDISCARDS only if user set
> IP_RECVERR, which is not RFC conformant.
> 
> Only case where we should not update OUTDISCARDS is when
> ip_local_output() return value is NET_XMIT_CN (meaning
> skb was xmitted but future frames might be dropped)
> 
> Signed-off-by: Christoph Lameter <cl@linux-foundation.org>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> ---
> diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
> index 7d08210..27a5b79 100644
> --- a/net/ipv4/ip_output.c
> +++ b/net/ipv4/ip_output.c
> @@ -1301,19 +1301,15 @@ int ip_push_pending_frames(struct sock *sk)
>  	/* Netfilter gets whole the not fragmented skb. */
>  	err = ip_local_out(skb);
>  	if (err) {
> +		if (err != NET_XMIT_CN)
> +			IP_INC_STATS(net, IPSTATS_MIB_OUTDISCARDS);
>  		if (err > 0)
>  			err = inet->recverr ? net_xmit_errno(err) : 0;
> -		if (err)
> -			goto error;
>  	}
> 
>  out:
>  	ip_cork_release(inet);
>  	return err;
> -
> -error:
> -	IP_INC_STATS(net, IPSTATS_MIB_OUTDISCARDS);
> -	goto out;
>  }
> 
>  /*



^ permalink raw reply
* Re: [PATCH 1/1] iwmc3200: add more SDIO device ids
From: Luis R. Rodriguez @ 2009-08-24 22:29 UTC (permalink / raw)
  To: Tomas Winkler
  Cc: davem-fT/PcQaiUtIeIZ0/mPfg9Q, netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA, Greg KH
In-Reply-To: <1ba2fa240908241520q5be2060co145ec0745954dcf0-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Mon, Aug 24, 2009 at 3:20 PM, Tomas Winkler<tomasw-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Tue, Aug 25, 2009 at 12:36 AM, Luis R. Rodriguez<mcgrof-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> On Mon, Aug 24, 2009 at 2:34 PM, Tomas Winkler<tomas.winkler@intel.com> wrote:
>>> add WiFi BGN SKU and WiMAX 2.4GHz SKU device ids
>>>
>>> Cc:inaky.perez-gonzalez-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org
>>> Cc:cindy.h.kao-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org
>>> Cc:yi.zhu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org
>>> Signed-off-by: Tomas Winkler <tomas.winkler-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>>
>> Since it seems we are preferring to not do this for PCI for cases
>> where the ID will only be used for 1 device perhaps its best to remove
>> all those ids and stuff them directly into the intel driver itself
>> that will use it.
>>
> I don't have strong opinion about it but this is the current habit
> IIRC there was a mail thread that explicitly asked to move SDIO IDs
> there,  of course I cannot locate it right now :(

Sure, understood, whoever maintains this should decide.

  Luis
--
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: [PATCH 1/1] iwmc3200: add more SDIO device ids
From: Tomas Winkler @ 2009-08-24 22:20 UTC (permalink / raw)
  To: Luis R. Rodriguez; +Cc: davem, netdev, linux-wireless, Greg KH
In-Reply-To: <43e72e890908241436o5f30cfe6xa7bdda8b25ac082@mail.gmail.com>

On Tue, Aug 25, 2009 at 12:36 AM, Luis R. Rodriguez<mcgrof@gmail.com> wrote:
> On Mon, Aug 24, 2009 at 2:34 PM, Tomas Winkler<tomas.winkler@intel.com> wrote:
>> add WiFi BGN SKU and WiMAX 2.4GHz SKU device ids
>>
>> Cc:inaky.perez-gonzalez@intel.com
>> Cc:cindy.h.kao@intel.com
>> Cc:yi.zhu@intel.com
>> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
>
> Since it seems we are preferring to not do this for PCI for cases
> where the ID will only be used for 1 device perhaps its best to remove
> all those ids and stuff them directly into the intel driver itself
> that will use it.
>
I don't have strong opinion about it but this is the current habit
IIRC there was a mail thread that explicitly asked to move SDIO IDs
there,  of course I cannot locate it right now :(

Thanks
Tomas

^ permalink raw reply
* Re: UDP multicast packet loss not reported if TX ring overrun?
From: Eric Dumazet @ 2009-08-24 22:02 UTC (permalink / raw)
  To: Christoph Lameter
  Cc: Sridhar Samudrala, Nivedita Singhvi, netdev, David S. Miller
In-Reply-To: <alpine.DEB.1.10.0908241334230.30487@gentwo.org>

Christoph Lameter a écrit :
> On Mon, 17 Aug 2009, Sridhar Samudrala wrote:
> 
>> So it is possible that there is some other place in the stack where the packets
>> are gettting dropped but not counted.
> 
> Such a deed occurs in ip_push_pending_frames():
> 
>         /* Netfilter gets whole the not fragmented skb. */
>         err = ip_local_out(skb);
>         if (err) {
>                 if (err > 0)
>                         err = inet->recverr ? net_xmit_errno(err) : 0;
> 			^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>                 if (err)
>                         goto error;
>         }
> 
> out:
>         ip_cork_release(inet);
>         return err;
> 
> error:
>         IP_INC_STATS(net, IPSTATS_MIB_OUTDISCARDS);
>         goto out;
> 
> 
> So if ip_local_out returns NET_XMIT_DROP then its simply going to be
> replaced by 0. Then we check err again and there is no error!!!!
> 
> The statistics are only generated if IP_RECVERR is set.
> 
> Could we move the increment of IPSTATS_MIB_OUTDISCARDS up so that it
> is incremented regardless of the setting of IP_RECVERR?
> 
> F.e?
> 
> 
> Subject: Report TX drops
> 
> Incrementing of TX drop counters currently does not work if errors from the
> network stack are suppressed (IP_RECVERR off). Increment the statistics
> independently of the setting of IP_RECVERR.
> 
> Signed-off-by: Christoph Lameter <cl@linux-foundation.org>
> 
> ---
>  net/ipv4/ip_output.c |   19 ++++++++++---------
>  1 file changed, 10 insertions(+), 9 deletions(-)
> 
> Index: linux-2.6/net/ipv4/ip_output.c
> ===================================================================
> --- linux-2.6.orig/net/ipv4/ip_output.c	2009-08-24 17:04:27.000000000 +0000
> +++ linux-2.6/net/ipv4/ip_output.c	2009-08-24 17:32:05.000000000 +0000
> @@ -1300,20 +1300,21 @@ int ip_push_pending_frames(struct sock *
> 
>  	/* Netfilter gets whole the not fragmented skb. */
>  	err = ip_local_out(skb);
> -	if (err) {
> -		if (err > 0)
> -			err = inet->recverr ? net_xmit_errno(err) : 0;
> -		if (err)
> -			goto error;
> +	if (err > 0) {
> +		/* The packet was dropped by the network subsystem */
> +		IP_INC_STATS(net, IPSTATS_MIB_OUTDISCARDS);
> +
> +		/*
> +		 * Errors are not passed on if the socket
> +		 * does not process errors (see IP_RECVERR).
> +		 * net_xmit_errno filters NET_XMIT_CN.
> +		 */
> +		err = inet->recverr ? net_xmit_errno(err) : 0;
>  	}
> 
>  out:
>  	ip_cork_release(inet);
>  	return err;
> -
> -error:
> -	IP_INC_STATS(net, IPSTATS_MIB_OUTDISCARDS);
> -	goto out;
>  }
> 
>  /*
> 
> 
> 
> 

NET_XMIT_CN strikes again :)

Well, if ip_local_out() returns a negative error (say -EPERM for example),
 your patch disables OUTDISCARDS increments.

Maybe a simpler patch like this one ?

[PATCH] net: correctly updates OUTDISCARDS in ip_push_pending_frames()

ip_push_pending_frames() can fail to send a frame because of a congestioned
device. In this case, we increment SNMP OUTDISCARDS only if user set
IP_RECVERR, which is not RFC conformant.

Only case where we should not update OUTDISCARDS is when
ip_local_output() return value is NET_XMIT_CN (meaning
skb was xmitted but future frames might be dropped)

Signed-off-by: Christoph Lameter <cl@linux-foundation.org>
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index 7d08210..27a5b79 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -1301,19 +1301,15 @@ int ip_push_pending_frames(struct sock *sk)
 	/* Netfilter gets whole the not fragmented skb. */
 	err = ip_local_out(skb);
 	if (err) {
+		if (err != NET_XMIT_CN)
+			IP_INC_STATS(net, IPSTATS_MIB_OUTDISCARDS);
 		if (err > 0)
 			err = inet->recverr ? net_xmit_errno(err) : 0;
-		if (err)
-			goto error;
 	}
 
 out:
 	ip_cork_release(inet);
 	return err;
-
-error:
-	IP_INC_STATS(net, IPSTATS_MIB_OUTDISCARDS);
-	goto out;
 }
 
 /*

^ permalink raw reply related
* TSecr != 0 check in inet_lro.c
From: Octavian Purdila @ 2009-08-24 21:54 UTC (permalink / raw)
  To: Jan-Bernd Themann; +Cc: netdev, Christoph Raisch


Hi,

We are seeing a performance issue with TSO/LRO which we tracked down to the 
TSecr !=0 check in lro_tcp_ip_check.

It happens when the LRO side's TSval wraps around and gets to 0. That triggers 
the TSO side to send packets with TSecr set to 0, which means that such 
packets won't be aggregated - and that will put a lot of burden on the stack 
which will result in lots of drops.

I'm failing to understand the purpose of this check. Any hints? :)

Thanks,
tavi

^ permalink raw reply
* [PATCH -next] bt: fix l2cap build error
From: Randy Dunlap @ 2009-08-24 21:41 UTC (permalink / raw)
  To: linux-next, netdev, David Miller, Marcel Holtmann, akpm

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

Fix net/bluetooth/l2cap.c build errors:

l2cap.c:(.text+0x126035): undefined reference to `crc16'
l2cap.c:(.text+0x126323): undefined reference to `crc16'
l2cap.c:(.text+0x12668e): undefined reference to `crc16'
l2cap.c:(.text+0x12683b): undefined reference to `crc16'
l2cap.c:(.text+0x126956): undefined reference to `crc16'
net/built-in.o:l2cap.c:(.text+0x129041): more undefined references to `crc16' follow

Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
---
 net/bluetooth/Kconfig |    1 +
 1 file changed, 1 insertion(+)

--- linux-next-20090824.orig/net/bluetooth/Kconfig
+++ linux-next-20090824/net/bluetooth/Kconfig
@@ -34,6 +34,7 @@ menuconfig BT
 config BT_L2CAP
 	tristate "L2CAP protocol support"
 	depends on BT
+	select CRC16
 	help
 	  L2CAP (Logical Link Control and Adaptation Protocol) provides
 	  connection oriented and connection-less data transport.  L2CAP

^ permalink raw reply
* Re: [PATCH 1/1] iwmc3200: add more SDIO device ids
From: Luis R. Rodriguez @ 2009-08-24 21:36 UTC (permalink / raw)
  To: Tomas Winkler
  Cc: davem-fT/PcQaiUtIeIZ0/mPfg9Q, netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA, Greg KH
In-Reply-To: <1251149659-32212-1-git-send-email-tomas.winkler-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

On Mon, Aug 24, 2009 at 2:34 PM, Tomas Winkler<tomas.winkler-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
> add WiFi BGN SKU and WiMAX 2.4GHz SKU device ids
>
> Cc:inaky.perez-gonzalez-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org
> Cc:cindy.h.kao-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org
> Cc:yi.zhu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org
> Signed-off-by: Tomas Winkler <tomas.winkler-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

Since it seems we are preferring to not do this for PCI for cases
where the ID will only be used for 1 device perhaps its best to remove
all those ids and stuff them directly into the intel driver itself
that will use it.

  Luis
--
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
* [PATCH 1/1] iwmc3200: add more SDIO device ids
From: Tomas Winkler @ 2009-08-24 21:34 UTC (permalink / raw)
  To: davem-fT/PcQaiUtIeIZ0/mPfg9Q
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA, Tomas Winkler

add WiFi BGN SKU and WiMAX 2.4GHz SKU device ids

Cc:inaky.perez-gonzalez-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org
Cc:cindy.h.kao-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org
Cc:yi.zhu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org
Signed-off-by: Tomas Winkler <tomas.winkler-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 include/linux/mmc/sdio_ids.h |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/include/linux/mmc/sdio_ids.h b/include/linux/mmc/sdio_ids.h
index 2dbfb5a..addd456 100644
--- a/include/linux/mmc/sdio_ids.h
+++ b/include/linux/mmc/sdio_ids.h
@@ -28,6 +28,8 @@
 #define SDIO_DEVICE_ID_INTEL_IWMC3200TOP	0x1404
 #define SDIO_DEVICE_ID_INTEL_IWMC3200GPS	0x1405
 #define SDIO_DEVICE_ID_INTEL_IWMC3200BT		0x1406
+#define SDIO_DEVICE_ID_INTEL_IWMC3200WIMAX_24	0x1407
+#define SDIO_DEVICE_ID_INTEL_IWMC3200WIFI_BGN	0x1408
 
 #define SDIO_VENDOR_ID_MARVELL			0x02df
 #define SDIO_DEVICE_ID_MARVELL_LIBERTAS		0x9103
-- 
1.6.0.6

---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

--
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 related
* Re: [PATCH] tcp: fix premature termination of FIN_WAIT2 time-wait sockets
From: Octavian Purdila @ 2009-08-24 20:47 UTC (permalink / raw)
  To: netdev
In-Reply-To: <200908150339.12730.opurdila@ixiacom.com>

[-- Attachment #1: Type: Text/Plain, Size: 556 bytes --]

On Saturday 15 August 2009 03:39:12 Octavian Purdila wrote:

> NOTE: this issue has been found, fixed and tested on an ancient 2.6.7
> kernel. This patch is a blind port of that fix, since unfortunately there
> is no easy way for me to reproduce the original issue with a newer kernel.
> But the issue still seems to be there.

Update: I was able to reproduce the issue on a 2.6.30 debian kernel with the 
attached test. It took me about 10 runs of 2-5 mins each to reproduce it 
(multiple runs to keep the capture file reasonable in terms of size).

tavi

[-- Attachment #2: finwait2.c --]
[-- Type: text/x-csrc, Size: 3384 bytes --]

#define _GNU_SOURCE
#include <sys/syscall.h>
#include <sched.h>
#include <stdlib.h>
#include <string.h>
#include <error.h>
#include <argp.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <arpa/inet.h>
#include <sys/epoll.h>
#include <sys/fcntl.h>
#include <sys/wait.h>
#include <stdio.h>
#include <errno.h>
#include <stdlib.h>
#include <unistd.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <netdb.h>
#include <net/if.h>
#include <signal.h>
#include <sys/socket.h>
#include <netpacket/packet.h>
#include <assert.h>
#include <netinet/tcp.h>
#include <linux/unistd.h>

static char doc[] = "";
static char args_doc[] = "";
static struct argp_option options[] = {
	{"server", 's', "host", 0, "server address"},
	{"port", 'p', "int", 0, "port to connect"},
	{"connections", 'c', "int", 0, "how many connections to open"},
	{0},
};

struct cl_args {
	unsigned short port, connections;
	uint32_t server;
};

struct cl_args cla = {
	.port=5555, 
	.connections=2500,
};


static error_t parse_opt(int key, char *arg, struct argp_state *state)
{
	struct cl_args *cla = state->input;
	switch (key) {
	case 's':
	{
		struct hostent *hostinfo =gethostbyname(arg);
		if (!hostinfo) {
			fprintf(stderr, "unknown host %s\n", arg);
			return -1;
		}
		cla->server=ntohl(((struct in_addr*)hostinfo->h_addr)->s_addr);
		break;
	}
	case 'p':
	{
		int port = atoi(arg);
		if (port <=0 || port > 65535) {
			fprintf(stderr, "invalid port: %d\n", cla->port);
			return -1;
		}
		cla->port=port;
		break;
	}
	case 'c':
	{
		cla->connections=atoi(arg);
		break;
	}
	case ARGP_KEY_ARG:
		break;
	default:
		return ARGP_ERR_UNKNOWN;
	}
	return 0;
}
static struct argp argp = { options, parse_opt, args_doc, doc };

static inline int is_client(void)
{
	if (cla.server != INADDR_ANY)
		return 1;
	return 0;
}

#define FAIL(x) \
	do {								\
		if (x) {						\
			fprintf(stderr, "%s:%d: error: %s\n", __func__, __LINE__, \
				strerror(errno));			\
			exit(1);					\
		}							\
	} while (0)

static void run_client(void)
{
	int i, socks[cla.connections];
	struct sockaddr_in name = {
		.sin_port = htons (cla.port),
		.sin_addr = { htonl(cla.server) },
		.sin_family = AF_INET
	};

	for(i=0; i < cla.connections; i++) {
		FAIL((socks[i] = socket(PF_INET, SOCK_STREAM, 0)) < 0);
		FAIL(connect(socks[i], (struct sockaddr*)&name, sizeof(name)) < 0);
	}

	/* wait server to close */
	sleep(1);

	for(i=0; i < cla.connections; i++)
		close(socks[i]);
}

static void run_server(int lsock)
{
	int i, socks[cla.connections];

	for(i = 0; i < cla.connections; i++)
		FAIL((socks[i] = accept(lsock, NULL, NULL)) < 0);

	for(i = 0; i < cla.connections; i++)
		close(socks[i]);

	sleep(1);
}

int main(int argc, char **argv)
{
	int lsock;

	if (argp_parse(&argp, argc, argv, 0, 0, &cla) < 0)
		return -1;

	FAIL(system("echo 1 > /proc/sys/net/ipv4/tcp_tw_reuse") != 0);

	setpriority(PRIO_PROCESS, getpid(), -20);

	while (1) {
		static int count;
		if (is_client())
			run_client();
		else {
			if (!count) {
				struct sockaddr_in name = {
					.sin_port = htons (cla.port),
					.sin_family = AF_INET
				};
				
				FAIL((lsock = socket(PF_INET, SOCK_STREAM, 0)) < 0);
				FAIL(bind(lsock, (struct sockaddr *) &name, sizeof (name)) < 0);
				FAIL(listen(lsock, cla.connections) < 0);
			}
			run_server(lsock);
		}
		printf("iteration %d\n", ++count);
	}
	
	return 0;
}

^ permalink raw reply
* £987,983GBP
From: NL @ 2009-08-24 20:34 UTC (permalink / raw)


£987,983GBP you have won, send your name,address,telephone
to(ukwins@ymail.com)

^ permalink raw reply
* Re: E100 RX ring buffers continued...
From: David Miller @ 2009-08-24 20:10 UTC (permalink / raw)
  To: khc
  Cc: walt, netdev, jeffrey.t.kirsher, jesse.brandeburg, bruce.w.allan,
	peter.p.waskiewicz.jr, john.ronciak
In-Reply-To: <m3bpm5v65e.fsf@intrepid.localdomain>

From: Krzysztof Halasa <khc@pm.waw.pl>
Date: Mon, 24 Aug 2009 12:49:01 +0200

> David Miller <davem@davemloft.net> writes:
> 
>>> Probably to "stable" series, too.
>>
>> That would depend upon applying your original patch to -stable,
>> which I had no intentions of doing.
> 
> Still you sent it for both 2.6.27 and .30 anyway :-)

Did I?  Aha, then I indeed have to send your fix :-)

^ permalink raw reply
* (no subject)
From: MRS SANDRA WHITE @ 2009-08-24 19:35 UTC (permalink / raw)



I am Mrs Sandra White, I am currently sending you this mail from my sick
bed in the hospital,I would want you to contact my lawyer;Email
(Martins009@9.cn)

^ permalink raw reply
* Re: [Bridge] VLANs and bridge
From: Joakim Tjernlund @ 2009-08-24 18:51 UTC (permalink / raw)
  To: Simon Barber; +Cc: bridge, netdev, kaber
In-Reply-To: <4A9026EA.9020404@superduper.net>

Simon Barber <simon@superduper.net> wrote on 22/08/2009 19:12:10:
>
> Looking through B.1.3 it looks like the patch would need some
> enhancement. It provides a good basis - handling tagging/untagging and
> filtering, but would need a way to specify the untagged vlan separately
> for in and out.

I see. Perhaps the VLAN maintainer(CC:ed) can comment too. Especially about extending
the VLAN code to allowed several VLANs in one interface?

>
> Simon
>
>
> Joakim Tjernlund wrote:
> > Joakim Tjernlund/Transmode wrote on 22/08/2009 17:36:54:
> >
> >> Simon Barber <simon@superduper.net> wrote on 22/08/2009 16:34:11:
> >>
> >>> Hi Joakim,
> >>>
> >>> Look back a month or so in the archives - you'll find an email from me
> >>> referencing a patch to make the bridge VLAN aware. It's a little old,
> >>> and needs updating for 2.6 (it was written for 2.4) - but should not be
> >>> much work to get it done. There is a matching patch for the brctl utility.
> >>>
> >>> Simon
> >>>
> >>>
> >> Thanks Simon
> >>
> >> Do you mean the "bridge vlan integration" patch?
> >> It is not clear to me if this would support the case
> >> described in B.1.3(802.1Q-2005), does it?
> >>
> >> Seems like there is little interest to post this to 2.6 and
> >> I am not sure how welcome this would be either, perhaps
> >> the bridge maintainer could comment?
> >>
> >>   Jocke
> >>
> >
> > Thinking a bit more about this I could envision some changes to the
> > vlan code too. I image one could create one vlan interface which will listen
> > to several VLANs. One VLAN is the primary VLAN and is used in the tx path
> > by default. The bridge would be able to filter on individual VLANs received
> > from such interface and be able to chose outgoing VLAN too.
> >
> >  Jocke
> >
> >
>
>
>


^ permalink raw reply
* Re: [Bug #13328] b44: eth0: BUG! Timeout waiting for bit 00000002 of register 42c to clear.
From: Rafael J. Wysocki @ 2009-08-24 18:30 UTC (permalink / raw)
  To: Francis Moreau; +Cc: Linux Kernel Mailing List, Kernel Testers List, netdev
In-Reply-To: <38b2ab8a0908240041s7d9707eftb6e8078d7b6ad553-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Monday 24 August 2009, Francis Moreau wrote:
> On Thu, Aug 20, 2009 at 4:52 PM, Rafael J. Wysocki<rjw-KKrjLPT3xs0@public.gmane.org> wrote:
> > On Thursday 20 August 2009, Francis Moreau wrote:
> >> Hello,
> >>
> >> On Wed, Aug 19, 2009 at 10:40 PM, Rafael J. Wysocki<rjw-KKrjLPT3xs0@public.gmane.org> wrote:
> >> > This message has been generated automatically as a part of a report
> >> > of regressions introduced between 2.6.29 and 2.6.30.
> >> >
> >> > The following bug entry is on the current list of known regressions
> >> > introduced between 2.6.29 and 2.6.30.  Please verify if it still should
> >> > be listed and let me know (either way).
> >> >
> >> >
> >> > Bug-Entry       : http://bugzilla.kernel.org/show_bug.cgi?id=13328
> >> > Subject         : b44: eth0: BUG! Timeout waiting for bit 00000002 of register 42c to clear.
> >> > Submitter       : Francis Moreau <francis.moro-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> >> > Date            : 2009-05-03 16:22 (109 days old)
> >> > References      : http://marc.info/?l=linux-kernel&m=124136778012280&w=4
> >> >
> >>
> >> It hasn't triggered on 2.6.31-rc5 after a couple of days.
> >
> > Great, let's assume it's fixed, then.
> 
> Sorry but it did happen this weekend.

That's OK, I'll reopen the bug.

Thanks,
Rafael

^ permalink raw reply
* Re: [PATCH 1/3] Set socket flags on restore using sock_setsockopt() where possible (v2)
From: Dan Smith @ 2009-08-24 18:03 UTC (permalink / raw)
  To: Dave Hansen; +Cc: containers, netdev
In-Reply-To: <1251136090.22398.7523.camel@nimitz>

DH> Would it make more sense to do this programatically?

If it pleases the court, sure :)

-- 
Dan Smith
IBM Linux Technology Center
email: danms@us.ibm.com

^ permalink raw reply
* Re: [PATCH 1/2] alchemy: add au1000-eth platform device
From: Sergei Shtylyov @ 2009-08-24 18:02 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Ralf Baechle, linux-mips, Manuel Lauss, David Miller, netdev
In-Reply-To: <200908181801.41602.florian@openwrt.org>

Hello.

Florian Fainelli wrote:

>>>This patch adds the board code to register a per-board au1000-eth
>>>platform device to be used wit the au1000-eth platform driver in a
>>>subsequent patch. Note that the au1000-eth driver knows about the
>>>default driver settings such that we do not need to pass any
>>>platform_data informations in most cases except db1x00.

>>    Sigh, NAK...
>>    Please don't register the SoC device per board, do it in
>>alchemy/common/platfrom.c and find a way to pass the board specific
>>platform data from the board file there instead -- something like
>>arch/arm/mach-davinci/usb.c does.

> Ok, like I promised, this was the per-board device registration. Do you prefer something like this:

    I certainly do, but still not in this incarnation... :-)

> --
> From fd75b7c7fa3c05c21122c43e43260d2785475a79 Mon Sep 17 00:00:00 2001
> From: Florian Fainelli <florian@openwrt.org>
> Date: Tue, 18 Aug 2009 17:53:21 +0200
> Subject: [PATCH] alchemy: add au1000-eth platform device (v2)
> 
> This patch makes the board code register the au1000-eth
> platform device. The au1000-eth platform data can be
> overriden with the au1xxx_override_eth0_cfg function
> like it has to be done for the Bosporus board.
> 
> Changes from v1:
> - remove per-board platform.c file
> - add an override function to pass custom eth0 platform_data PHY settings
> 
> Signed-off-by: Florian Fainelli <florian@openwrt.org>
> ---
> diff --git a/arch/mips/alchemy/common/platform.c b/arch/mips/alchemy/common/platform.c
> index 117f99f..559294a 100644
> --- a/arch/mips/alchemy/common/platform.c
> +++ b/arch/mips/alchemy/common/platform.c
> @@ -19,6 +19,7 @@
>  #include <asm/mach-au1x00/au1xxx.h>
>  #include <asm/mach-au1x00/au1xxx_dbdma.h>
>  #include <asm/mach-au1x00/au1100_mmc.h>
> +#include <asm/mach-au1x00/au1xxx_eth.h>
>  
>  #define PORT(_base, _irq)				\
>  	{						\
> @@ -331,6 +332,76 @@ static struct platform_device pbdb_smbus_device = {
>  };
>  #endif
>  
> +/* Macro to help defining the Ethernet MAC resources */
> +#define MAC_RES(_base, _enable, _irq)			\
> +	{						\
> +		.start	= CPHYSADDR(_base),		\
> +		.end	= CPHYSADDR(_base + 0xffff),	\
> +		.flags	= IORESOURCE_MEM,		\
> +	},						\
> +	{						\
> +		.start	= CPHYSADDR(_enable),		\
> +		.end	= CPHYSADDR(_enable + 0x3),	\
> +		.flags	= IORESOURCE_MEM,		\
> +	},						\
> +	{						\
> +		.start	= _irq,				\
> +		.end	= _irq,				\
> +		.flags	= IORESOURCE_IRQ		\
> +	}
> +
> +static struct resource au1xxx_eth0_resources[] = {
> +#if defined(CONFIG_SOC_AU1000)
> +	MAC_RES(AU1000_ETH0_BASE, AU1000_MAC0_ENABLE, AU1000_MAC0_DMA_INT),
> +#elif defined(CONFIG_SOC_AU1100)
> +	MAC_RES(AU1100_ETH0_BASE, AU1100_MAC0_ENABLE, AU1100_MAC0_DMA_INT),
> +#elif defined(CONFIG_SOC_AU1550)
> +	MAC_RES(AU1550_ETH0_BASE, AU1550_MAC0_ENABLE, AU1550_MAC0_DMA_INT),
> +#elif defined(CONFIG_SOC_AU1500)
> +	MAC_RES(AU1500_ETH0_BASE, AU1500_MAC0_ENABLE, AU1500_MAC0_DMA_INT),
> +#endif
> +};
> +
> +static struct resource au1xxx_eth1_resources[] = {
> +#if defined(CONFIG_SOC_AU1000)
> +	MAC_RES(AU1000_ETH1_BASE, AU1000_MAC1_ENABLE, AU1000_MAC1_DMA_INT),
> +#elif defined(CONFIG_SOC_AU1550)
> +	MAC_RES(AU1550_ETH1_BASE, AU1550_MAC1_ENABLE, AU1550_MAC1_DMA_INT),
> +#elif defined(CONFIG_SOC_AU1500)
> +	MAC_RES(AU1500_ETH1_BASE, AU1500_MAC1_ENABLE, AU1500_MAC1_DMA_INT),
> +#endif
> +};
> +
> +static struct au1000_eth_platform_data au1xxx_eth0_platform_data = {
> +	.phy1_search_mac0 = 1,
> +};

    I'm not sure that the default platfrom data is really a great idea...

> +#ifndef CONFIG_SOC_AU1100
> +static struct platform_device au1xxx_eth1_device = {
> +	.name		= "au1000-eth",
> +	.id		= 1,
> +	.num_resources	= ARRAY_SIZE(au1xxx_eth1_resources),
> +	.resource	= au1xxx_eth1_resources,

    And where's the platfrom data for the second Ethernet?

> +};
> +#endif
> +
> +void __init au1xxx_override_eth0_cfg(struct au1000_eth_platform_data *eth_data)
> +{
> +	if (!eth_data)
> +		return;
> +
> +	memcpy(&au1xxx_eth0_platform_data, eth_data,
> +		sizeof(struct au1000_eth_platform_data));

    Why not just set the pointer in au1xxx_eth0_device. And really, why not 
make the function more generic, with a prototype like:

void __init au1xxx_override_eth_cfg(unsigned port, struct
				    au1000_eth_platform_data *eth_data);

> +}
> +
>  static struct platform_device *au1xxx_platform_devices[] __initdata = {
>  	&au1xx0_uart_device,
>  	&au1xxx_usb_ohci_device,
> @@ -351,17 +422,25 @@ static struct platform_device *au1xxx_platform_devices[] __initdata = {
>  #ifdef SMBUS_PSC_BASE
>  	&pbdb_smbus_device,
>  #endif
> +	&au1xxx_eth0_device,
>  };
>  
>  static int __init au1xxx_platform_init(void)
>  {
>  	unsigned int uartclk = get_au1x00_uart_baud_base() * 16;
> -	int i;
> +	int i, ni;
>  
>  	/* Fill up uartclk. */
>  	for (i = 0; au1x00_uart_data[i].flags; i++)
>  		au1x00_uart_data[i].uartclk = uartclk;
>  
> +	/* Register second MAC if enabled in pinfunc */
> +#ifndef CONFIG_SOC_AU1100
> +	ni = (int)((au_readl(SYS_PINFUNC) & (u32)(SYS_PF_NI2)) >> 4);
> +	if (!(ni + 1))

    Why so complex, and how can (ni + 1) ever be 0?! :-/
    Doesn't that field when 0 mean the pins configured for MAC1 and when 1 
-- for GPIO? Why not just:

	if (!(au_readl(SYS_PINFUNC) & SYS_PF_NI2))

> +		platform_device_register(&au1xxx_eth1_device);
> +#endif
> +

WBR, Sergei

^ permalink raw reply
* Re: [RFC] defer skb allocation in virtio_net -- mergable buff part
From: Shirley Ma @ 2009-08-24 17:51 UTC (permalink / raw)
  To: Avi Kivity; +Cc: netdev, kvm, linux-kernel
In-Reply-To: <4A880DFE.2040507@redhat.com>

Hello Avi,

Thanks for you review. I was on vacation last week.

On Sun, 2009-08-16 at 16:47 +0300, Avi Kivity wrote:
> Alternatives include:
> - store the link in the page itself
> - have an array of pages per list element instead of just one pointer
> - combine the two, store an array of page pointers in one of the free
> pages
> - use the struct page::lru member
> 
> The last is the most traditional and easiest so I'd recommend it
> (though 
> it still takes the cacheline hit).

I prefer the combine of the two. But I will compare the performance
differences if no much difference, we can use the easiest one.

> > +static struct page_list *get_a_free_page(struct virtnet_info *vi,
> gfp_t gfp_mask)
> > +{
> > +     struct page_list *plist;
> > +
> > +     if (list_empty(&vi->freed_pages)) {
> > +             plist = kmalloc(sizeof(struct page_list), gfp_mask);
> > +             if (!plist)
> > +                     return NULL;
> > +             list_add_tail(&plist->list,&vi->freed_pages);
> > +             plist->page = alloc_page(gfp_mask);
> >    
> 
> What if the allocation fails here?

It is handled by the caller.

Thanks
Shirley


^ permalink raw reply
* Re: UDP multicast packet loss not reported if TX ring overrun?
From: Christoph Lameter @ 2009-08-24 17:40 UTC (permalink / raw)
  To: Sridhar Samudrala; +Cc: Nivedita Singhvi, netdev, Eric Dumazet
In-Reply-To: <1250554332.25939.46.camel@w-sridhar.beaverton.ibm.com>

On Mon, 17 Aug 2009, Sridhar Samudrala wrote:

> So it is possible that there is some other place in the stack where the packets
> are gettting dropped but not counted.

Such a deed occurs in ip_push_pending_frames():

        /* Netfilter gets whole the not fragmented skb. */
        err = ip_local_out(skb);
        if (err) {
                if (err > 0)
                        err = inet->recverr ? net_xmit_errno(err) : 0;
			^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                if (err)
                        goto error;
        }

out:
        ip_cork_release(inet);
        return err;

error:
        IP_INC_STATS(net, IPSTATS_MIB_OUTDISCARDS);
        goto out;


So if ip_local_out returns NET_XMIT_DROP then its simply going to be
replaced by 0. Then we check err again and there is no error!!!!

The statistics are only generated if IP_RECVERR is set.

Could we move the increment of IPSTATS_MIB_OUTDISCARDS up so that it
is incremented regardless of the setting of IP_RECVERR?

F.e?


Subject: Report TX drops

Incrementing of TX drop counters currently does not work if errors from the
network stack are suppressed (IP_RECVERR off). Increment the statistics
independently of the setting of IP_RECVERR.

Signed-off-by: Christoph Lameter <cl@linux-foundation.org>

---
 net/ipv4/ip_output.c |   19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

Index: linux-2.6/net/ipv4/ip_output.c
===================================================================
--- linux-2.6.orig/net/ipv4/ip_output.c	2009-08-24 17:04:27.000000000 +0000
+++ linux-2.6/net/ipv4/ip_output.c	2009-08-24 17:32:05.000000000 +0000
@@ -1300,20 +1300,21 @@ int ip_push_pending_frames(struct sock *

 	/* Netfilter gets whole the not fragmented skb. */
 	err = ip_local_out(skb);
-	if (err) {
-		if (err > 0)
-			err = inet->recverr ? net_xmit_errno(err) : 0;
-		if (err)
-			goto error;
+	if (err > 0) {
+		/* The packet was dropped by the network subsystem */
+		IP_INC_STATS(net, IPSTATS_MIB_OUTDISCARDS);
+
+		/*
+		 * Errors are not passed on if the socket
+		 * does not process errors (see IP_RECVERR).
+		 * net_xmit_errno filters NET_XMIT_CN.
+		 */
+		err = inet->recverr ? net_xmit_errno(err) : 0;
 	}

 out:
 	ip_cork_release(inet);
 	return err;
-
-error:
-	IP_INC_STATS(net, IPSTATS_MIB_OUTDISCARDS);
-	goto out;
 }

 /*




^ permalink raw reply
* Re: [PATCH 1/3] Set socket flags on restore using sock_setsockopt() where possible (v2)
From: Dave Hansen @ 2009-08-24 17:48 UTC (permalink / raw)
  To: Dan Smith; +Cc: containers, netdev
In-Reply-To: <1251134884-24491-2-git-send-email-danms@us.ibm.com>

On Mon, 2009-08-24 at 10:28 -0700, Dan Smith wrote:
> 
> +static int sock_restore_flags(struct socket *sock,
> +                             struct ckpt_hdr_socket *h)
> +{
> +       int ret;
> +       int v = 1;
> +       unsigned long sk_flags = h->sock.flags;
> +       unsigned long sock_flags = h->socket.flags;
> +
> +       if (test_and_clear_bit(SOCK_URGINLINE, &sk_flags)) {
> +               ret = sock_setsockopt(sock, SOL_SOCKET, SO_OOBINLINE,
> +                                     (char *)&v, sizeof(v));
> +               if (ret)
> +                       return ret;
> +       }
> +
> +       if (test_and_clear_bit(SOCK_KEEPOPEN, &sk_flags)) {
> +               ret = sock_setsockopt(sock, SOL_SOCKET, SO_KEEPALIVE,
> +                                     (char *)&v, sizeof(v));
> +               if (ret)
> +                       return ret;
> +       }

Would it make more sense to do this programatically?

cr_sock_restore_flag(struct socket *sock, unsigned long sck_flag,
		     struct ckpt_hdr_socket *h, unsigned long sock_flag)
{
	unsigned long sk_flags = h->sock.flags;
	unsigned long sock_flags = h->socket.flags;

	
	if (!test_and_clear_bit(sk, &sk_flags))
		return 0;

	return sock_setsockopt(sock, sock_flag, SO_OOBINLINE,
 			       (char *)&v, sizeof(v));
}

Then, each call becomes:

	ret = cr_sock_restore_flag(sock, cr_sock, SOCK_URGINLINE, SO_OOBINLINE);
	if (ret)
		return ret;

	ret = cr_sock_restore_flag(sock, cr_sock, SOCK_KEEPOPEN, SO_KEEPALIVE);
	if (ret)
		return ret;

Or, you could spell the flags out in a (better named) structure:

struct sock_flagpair
{
	unsigned long sock_flag;
	unsigned long sk_flag;
}	

struct sock_flagpair sock_flagpairs[] = {
	{ SOCK_URGINLINE, SO_OOBINLINE },
	{ SOCK_KEEPOPEN, SO_KEEPALIVE},
	...
};

And just walk through the array to do the restore:

static int sock_restore_flags(struct socket *sock,
                             struct ckpt_hdr_socket *h)
{
	int i;

	for (i = 0; i < ARRAY_SIZE(sock_flagpairs); i++) {
		int ret;
		unsigned long sock_flag = sock_flagpairs[i].sock_flag;
		unsigned long sk_flag = sock_flagpairs[i].sk_flag;
		ret = cr_sock_restore_flag(sock, cr_sock, sock_flag, sk_flag);
		if (ret)
			break;
	}
	...
}

-- Dave


^ permalink raw reply
* Re: [PATCH] netfilter: bridge: refcount fix
From: Patrick McHardy @ 2009-08-24 17:38 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David S. Miller, Linux Netdev List, Bart De Schuymer
In-Reply-To: <4A92CEA4.6020604@gmail.com>

Eric Dumazet wrote:
> [PATCH] netfilter: bridge: refcount fix
> 
> commit f216f082b2b37c4943f1e7c393e2786648d48f6f
> ([NETFILTER]: bridge netfilter: deal with martians correctly)
> added a refcount leak on in_dev.
> 
> Instead of using in_dev_get(), we can use __in_dev_get_rcu(),
> as netfilter hooks are running under rcu_read_lock(), as pointed
> by Patrick.
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> Signed-off-by: Patrick McHardy <kaber@trash.net>

I've applied it to nf-next-2.6.git since its not as EARTH SHATTERING
as I understood Dave would like it to be for consideration for
net-2.6.git :)

Thanks!

^ permalink raw reply
* Re: [Bonding-devel] [PATCH net-next-2.6] bonding: introduce primary_lazy option
From: Jay Vosburgh @ 2009-08-24 17:35 UTC (permalink / raw)
  To: Jiri Pirko; +Cc: Nicolas de Pesloüan, netdev, davem, bonding-devel
In-Reply-To: <20090824152002.GD4018@psychotron.englab.brq.redhat.com>

Jiri Pirko <jpirko@redhat.com> wrote:

>Mon, Aug 24, 2009 at 05:07:18PM CEST, nicolas.2p.debian@free.fr wrote:
>> Jiri Pirko wrote:
>>> Thu, Aug 20, 2009 at 02:40:07PM CEST, nicolas.2p.debian@free.fr wrote:
>> [--cut--]
>>>> I suggest that instead of having a per bond "primary_lazy" option, we
>>>> define a per slave option, describing whether this particular slave is
>>>> "sticky when active" or not.
>>>>
>>>> The above setup would become :
>>>>
>>>> echo 1 > /sys/class/net/eth0/bonding/sticky_active
>>>> echo 1 > /sys/class/net/eth1/bonding/sticky_active
>>>> echo 0 > /sys/class/net/eth2/bonding/sticky_active
>>>> echo eth0 > /sys/class/net/bond0/bonding/primary
>>>>
>>>> Or may be better, keeping the "weight" idea in mind, a per slave option
>>>> "active_weight" that gives the weight of the slave, *when active*.
>>>>
>>>> The effective weight of a slave would become :
>>>>
>>>> effective_slave =
>>>> (is_active ? user_supplied_active_weight ? user_supplied_weight) *
>>>> natural_weight
>>>>
>>>> # Prefer eth0, then one of eth1 or eth2, then eth3.
>>>> echo 1000 > /sys/class/net/eth0/bonding/weight
>>>> echo 999 > /sys/class/net/eth1/bonding/weight
>>>> echo 999 > /sys/class/net/eth2/bonding/weight
>>>> echo 10 > /sys/class/net/eth3/bonding/weight
>>>>
>>>> # Do not switch back to primary eth0 if eth1 or eth2 is active.
>>>> echo 1000 > /sys/class/net/eth1/bonding/active_weight
>>>> echo 1000 > /sys/class/net/eth2/bonding/active_weight
>>>>
>>>> Every time one changes the user_supplied_weight, then
>>>> user_supplied_active_weight must be reset to the same value. This 
>>>> way,  if no special setup is done on active_weight, then the current 
>>>> normal
>>>> behavior is achieved.
>>>
>>> I must say I like this approach. But it would be not trivial to implement this.
>>> Therefore I would stick with your propose of extending primary lazy to 3 values
>>> until the weight option is implemented.
>>
>> It sounds good for me. Later, if I eventually implement the weight 
>> option, it shouldn't be that hard to convert internally the primary_lazy 
>> setup to active_weight, the same way we plan to convert internally 
>> primary setup to weight setup.
>>
>> primary and primary_lazy are convenient for simple - two slaves only -  
>> configurations. weight and active_weight are for more advanced 
>> configurations. Keeping both configuration interface does sound user 
>> friendly.
>
>Ok, I agree.
>
>>
>>> I'm going to implement your propose below.
>>
>> By the way, even if I'm not a native English speaker, I think that 
>> primary_lazy option should be named lazy_primary instead.
>
>Well I've intentionally put "primary" as a first word to amplify it's linked
>with primary option... But...

	I'm still unclear as to why it's better to add another special
case option to bonding instead of changing this in user space, other
than it'd be a change to user space (initscripts / sysconfig).

	The way I see it, this patch is adding a mechanism that says,
effectively, "make slave X the active slave, but do it only once."
There is already a way to do that in bonding (sysfs, as above, or
ifenslave -c); I am reluctant to add another without good reason.

	I'm not necessarily against the "weight" business in general.
For the purposes of this discussion, however, it's a big complex
solution to a pretty simple problem, and the "weight" system still has
to have special sauce added it to to handle this special case.

	Last, presuming for the moment that this goes forward as an
option to bonding, I think this should be named something along the
lines of "make_active" (or perhaps "make_active_once", but that's a bit
long).  The option has the effect of making the specified slave the
active slave one time, then the option setting is cleared.

	-J

---
	-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com

^ permalink raw reply
* Re: [PATCH] netfilter: bridge: refcount fix
From: Eric Dumazet @ 2009-08-24 17:32 UTC (permalink / raw)
  To: Patrick McHardy; +Cc: David S. Miller, Linux Netdev List, Bart De Schuymer
In-Reply-To: <4A92CC71.2000300@trash.net>

Patrick McHardy a écrit :
> Eric Dumazet wrote:
>> Hi David
>>
>> I found following by code review only, I am not sure it is critical enough for net-2.6
>>
>> This is a stable candidate, bug is more than 2 years old.
>>
>> Thanks
>>
>> commit f216f082b2b37c4943f1e7c393e2786648d48f6f
>> ([NETFILTER]: bridge netfilter: deal with martians correctly)
>> added a refcount leak on in_dev.
>>
>> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
>> ---
>> diff --git a/net/bridge/br_netfilter.c b/net/bridge/br_netfilter.c
>> index 4fde742..c62eca3 100644
>> --- a/net/bridge/br_netfilter.c
>> +++ b/net/bridge/br_netfilter.c
>> @@ -386,6 +386,7 @@ static int br_nf_pre_routing_finish(struct sk_buff *skb)
>>  				dst_release((struct dst_entry *)rt);
>>  			}
>>  free_skb:
>> +			in_dev_put(in_dev);
>>  			kfree_skb(skb);
>>  			return 0;
> 
> I guess we could simply use __in_dev_get_rcu() here since all
> netfilter hooks are running under rcu_read_lock() anyways.

Ah very good point, Thanks Patrick.

[PATCH] netfilter: bridge: refcount fix

commit f216f082b2b37c4943f1e7c393e2786648d48f6f
([NETFILTER]: bridge netfilter: deal with martians correctly)
added a refcount leak on in_dev.

Instead of using in_dev_get(), we can use __in_dev_get_rcu(),
as netfilter hooks are running under rcu_read_lock(), as pointed
by Patrick.

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: Patrick McHardy <kaber@trash.net>
---
diff --git a/net/bridge/br_netfilter.c b/net/bridge/br_netfilter.c
index 4fde742..907a82e 100644
--- a/net/bridge/br_netfilter.c
+++ b/net/bridge/br_netfilter.c
@@ -359,7 +359,7 @@ static int br_nf_pre_routing_finish(struct sk_buff *skb)
 				},
 				.proto = 0,
 			};
-			struct in_device *in_dev = in_dev_get(dev);
+			struct in_device *in_dev = __in_dev_get_rcu(dev);
 
 			/* If err equals -EHOSTUNREACH the error is due to a
 			 * martian destination or due to the fact that

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