Netdev List
 help / color / mirror / Atom feed
* Re: Linux, tcpdump and vlan
From: Stephen Hemminger @ 2007-07-19 13:41 UTC (permalink / raw)
  To: Krzysztof Halasa
  Cc: Patrick McHardy, andrei radulescu-banu, linux-kernel,
	Linux Netdev List
In-Reply-To: <m3zm1sbjzl.fsf@maximus.localdomain>

On Thu, 19 Jul 2007 15:28:46 +0200
Krzysztof Halasa <khc@pm.waw.pl> wrote:

> Patrick McHardy <kaber@trash.net> writes:
> 
> > Your suggestion of disabling VLAN acceleration in promiscous
> > mode sounds like a reasonable solution until then ..
> 
> From a user perspective:
> 
> I'm not sure promiscous mode is related to the problem.
> Tcpdump without promiscous mode makes perfect sense.
> 
> I don't know very well VLAN code internals, but I think
> the VLAN # is used for looking up the interface, so
> presenting the "original" packet on the trunk device
> would IMHO involve some skb cloning, and perhaps some
> ethtool option could probably control that.
> 
> Not sure about untagged frames vs. tagged frames with
> the default VLAN id - can the hardware at all differentiate
> between them?
> 
> 
> Or, perhaps it should be left (almost) as is - with "software"
> VLANs the traffic always goes through the master interface,
> but with "accelerated" mode it only goes through logical
> interfaces and doesn't show up on master? Probably with
> exception of invalid VLANs, which could be injected back to
> master (because no logical device exists)?


I don't claim to be a VLAN expert but there are really three cases
for handling tagged frames

1) non-accelerated device 
    * all frames show in promiscious mode
    * tag is part of the frame that shows up
       in tcpdump, and then gets stripped by the 8021q module.
2) rx tag stripping device
     * all frames show in promiscious mode
     * tag is in skb but NOT passed to tcpdump
3) rx vlan acceleration
     * only frames that for vlan's that are registered show up
        in promisicous mode
     * tag is in skb but NOT passed to tcpdump

Unfortunately, the tag is lost as part of the VLAN acceleration process
so it is not a simple matter of changing code in AF_PACKET receive
to restore the tag.

^ permalink raw reply

* Re: [PATCH 1/4] ibmveth: Enable TCP checksum offload
From: Brian King @ 2007-07-19 13:35 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: linuxppc-dev, rcjenn, santil, netdev
In-Reply-To: <469E9510.7060703@garzik.org>

Jeff Garzik wrote:
> Brian King wrote:
>> This patchset enables TCP checksum offload support for IPV4
>> on ibmveth. This completely eliminates the generation and checking of
>> the checksum for packets that are completely virtual and never
>> touch a physical network. A simple TCP_STREAM netperf run on
>> a virtual network with maximum mtu set yielded a ~30% increase
>> in throughput. This feature is enabled by default on systems that
>> support it, but can be disabled with a module option.
>>
>> Signed-off-by: Brian King <brking@linux.vnet.ibm.com>
> 
> this should be controllable purely via ethtool...

I'll remove the module parameter and resend.

>>  struct ibmveth_rx_q_entry {
>>      u16 toggle : 1;
>>      u16 valid : 1;
>> -    u16 reserved : 14;
>> +    u16 reserved : 4;
>> +    u16 no_csum : 1;
>> +    u16 csum_good : 1;
>> +    u16 reserved2 : 8;
>>      u16 offset;
> 
> As a general rule, it would be nice to start moving away from bitfields 
> in this driver

I'll take a look at that for a future cleanup patch.

Thanks,

Brian

-- 
Brian King
Linux on Power Virtualization
IBM Linux Technology Center

^ permalink raw reply

* Re: tg3 issues
From: patric @ 2007-07-19 13:24 UTC (permalink / raw)
  To: Neil Horman; +Cc: netdev
In-Reply-To: <20070719113755.GA1934@hmsreliant.homelinux.net>

Neil Horman wrote:

> On Thu, Jul 19, 2007 at 04:49:13PM +0530, pradeep singh wrote:
>   
>> CCing: netdev
>>
>> On 7/19/07, patric <pakar@imperialnet.org> wrote:
>>     
>>> Hi,
>>>
>>>
>>> To start with, i'm not sure if this should go to the dev or user list,
>>> but i'll start here..
>>>
>>>
>>> I'm currently running a nfsroot via a Broadcom NetXtreme 1000-SX card
>>> (BCM5701) and i have a big problem with the tg3 drivers autonegotiation.
>>>
>>> The issue seems to be that when the kernel comes so far as it's trying
>>> to mount the boot the autonegotiation has not yet completed and then
>>> causes a panic since it cannot mount the nfsroot.
>>>
>>>
>>> From some debugging i have done here the issues seems to be related to
>>> the flowcontrol configuration, and just to make it a bit more fun it
>>> does work some of the time.. (around once every 5-10 attempts.)
>>>
>>>
>>> On the console it looks something like this when failing. (written from
>>> memory since i don't have netconsole enabled)
>>>
>>> tg3: eth0: Link is up at 1000 Mbps, full duplex.
>>> tg3: eth0: Flow control is off for TX and off for RX.
>>> IP-Config: Complete:
>>>      device=eth0, addr=192.168.1.10, mask=255.255.255.0,
>>> gw=255.255.255.255,
>>>     host=amd, domain=, nis-domain=(none),
>>>     bootserver=255.255.255.255, rootserver=192.168.1.1, rootpath=
>>> Root-NFS: unknown option: nolocks
>>> Looking up port of RPC 100003/3 on 192.168.1.1
>>> rpcbind: server 192.168.1.1 not responding, timed out
>>>
>>> Root-NFS: Unable to get nfsd port number from server, using default
>>>
>>> Looking up port of RPC 100003/3 on 192.168.1.1
>>> rpcbind: server 192.168.1.1 not responding, timed out
>>>
>>> Root-NFS: Unable to get nfsd port number from server, using default
>>>
>>>
>>> and so on until it panics...
>>>
>>>       
>
> IIRC, there are two main problems in this typ of situation
>
> 1) Spanning tree convergence
> 2) Firmware initalization latency
>
> If you are running spanning tree on your network, it can take up to 2 minutes
> before your port will forward frames properly.  if you have the options
> available, disable spanning tree on the switch port connected to your host
> system, or at least enable portfast if it is an option.  That should fix any
> spanning tree issues you have
>
> If the tg3 card is just taking a long time to initalize, there is not too much
> you can do about it.  If your goal is to use nfs root, I would, instead of
> enabling nfs-root as a kernel config option, I would create an initramfs that:
> A) Brings up your NIC
> B) Mounts your nfs partition
> C) executes a switch_root or pivot_root operation
>
> That way you can calibrate a delay between steps (A) and (B) in your initramfs
> init script
>
> Regards
> Neil
>
>   
Hi Neil and thanks for your quick reply, and thanks Pradeep for 
forwarding the question to the correct mailinglist.

Well, not using any switches and just a crossed cable between the 
machines. Did notice that it seems to get a 'good link' more often when 
cold-booting the client.
Have been thinking about using a initrd to get around the issue, but the 
problem is that you never know how long the init will be so there will 
always have to be a quite big delay before the system can boot. But 
don't really think the issue is that the card takes a long time to 
initialize since it does sometime work without delay during a warm-boot 
and the cards do report that they are up but they then are reporting 
different states of flow-control. Maybe set the flowcontrol static in 
the driver for a test, if i now can figure out how this driver works. :)

Just a hypothetical question. If the 2 network cards starts the 
autonegotiation would it be possible that they get into a loop where 
they are chasing each others state?  Maybe a fix could be to add a sleep 
of a random length that would enable them to catch up? Maybe you know if 
any of the fiber-cards so support running without flowcontrol too since 
the cards don't seem to be able to get a link with flowcontrol turned 
off at least in this setup.


Regards,
Patric




^ permalink raw reply

* Re: Linux, tcpdump and vlan
From: Krzysztof Halasa @ 2007-07-19 13:28 UTC (permalink / raw)
  To: Patrick McHardy; +Cc: andrei radulescu-banu, linux-kernel, Linux Netdev List
In-Reply-To: <469E9AC8.3090603@trash.net>

Patrick McHardy <kaber@trash.net> writes:

> Your suggestion of disabling VLAN acceleration in promiscous
> mode sounds like a reasonable solution until then ..

>From a user perspective:

I'm not sure promiscous mode is related to the problem.
Tcpdump without promiscous mode makes perfect sense.

I don't know very well VLAN code internals, but I think
the VLAN # is used for looking up the interface, so
presenting the "original" packet on the trunk device
would IMHO involve some skb cloning, and perhaps some
ethtool option could probably control that.

Not sure about untagged frames vs. tagged frames with
the default VLAN id - can the hardware at all differentiate
between them?


Or, perhaps it should be left (almost) as is - with "software"
VLANs the traffic always goes through the master interface,
but with "accelerated" mode it only goes through logical
interfaces and doesn't show up on master? Probably with
exception of invalid VLANs, which could be injected back to
master (because no logical device exists)?
-- 
Krzysztof Halasa

^ permalink raw reply

* Re: updated tcatm patches for kernel/iproute 2.6.22
From: Patrick McHardy @ 2007-07-19 13:22 UTC (permalink / raw)
  To: safari-kernel; +Cc: Linux Networking Mailing List
In-Reply-To: <20070719113559.4y2z4bj4vwtglupw@m.safari.iki.fi>

Sami Farin wrote:
> On Thu, Jul 19, 2007 at 00:58:44 +0200, Patrick McHardy wrote:
>   
>> Let me repeat again before we get in an endless discussion
>> again - I will NACK these patches until we have a solution
>> that works for all qdiscs.
>>     
>
> I googled for
> "Accurate packet scheduling for ATM" mchardy
> and found some threads from 2006.
>   
> http://www.mail-archive.com/netdev@vger.kernel.org/msg15706.html
> http://www.mail-archive.com/netdev@vger.kernel.org/msg21395.html
>
> So, Patrick:

Please keep me as receipient or CC when talking to me.

> 1) what is this "solution"?  do you have code?
>    or a brief explanation...

Simply calculcate the length including additional encapsulation
overhead at runtime. This allows all qdiscs to use the wire-length
in their calculations.

> 2) are you still working on this? is somebody else?
>   

Not to my knowledge.

> 3) do you have patches for iproute/kernel 2.6.22?

No.

> 4) if you have some patches, but they are not yet
>    complete, what else needs to be done?
>   

Not sure, its been a while since I posted some example patches.
Its not much work, probably quicker to start over.


^ permalink raw reply

* Re: ~3 hours old git tree: Virtual device lo asks to queue packet!
From: Patrick McHardy @ 2007-07-19 13:13 UTC (permalink / raw)
  To: Arkadiusz Miskiewicz; +Cc: Peter P Waskiewicz Jr, netdev, David Miller
In-Reply-To: <200707191352.52707.arekm@maven.pl>

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

Arkadiusz Miskiewicz wrote:
> On Thursday 19 of July 2007, Patrick McHardy wrote:
>   
>> OK I see what the problem is. The loopback device is statically
>> allocated, so it doesn't have any room for the subqueues reserved.
>>
>> The easiest fix would be to use egress_subqueue[1] in struct
>> net_device, but I think that may cause warnings with newer gccs
>> when using a constant index that is > 0. OTOH using constant
>> indices doesn't seem to make much sense for the subqueue array.
>>
>> Arkadiusz, does this patch fix the problem?
>>     
>
> It fixes the problem for me.
>   

Thanks for testing. Dave, please apply.



[-- Attachment #2: x --]
[-- Type: text/plain, Size: 1607 bytes --]

[NET]: Fix loopback multiqueue bug

The loopback device is not allocated through netdev_alloc_mq and thus
has no room for the subqueue states reserved. Change the net_device
subqueue array to always include at least one element.

Signed-off-by: Patrick McHardy <kaber@trash.net>
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index da7a13c..bf9399c 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -575,7 +575,7 @@ struct net_device
 
 	/* The TX queue control structures */
 	unsigned int			egress_subqueue_count;
-	struct net_device_subqueue	egress_subqueue[0];
+	struct net_device_subqueue	egress_subqueue[1];
 };
 #define to_net_dev(d) container_of(d, struct net_device, dev)
 
diff --git a/net/core/dev.c b/net/core/dev.c
index 13a0d9f..4af0207 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3619,7 +3619,7 @@ struct net_device *alloc_netdev_mq(int sizeof_priv, const char *name,
 
 	/* ensure 32-byte alignment of both the device and private area */
 	alloc_size = (sizeof(*dev) + NETDEV_ALIGN_CONST +
-		     (sizeof(struct net_device_subqueue) * queue_count)) &
+		     (sizeof(struct net_device_subqueue) * (queue_count - 1))) &
 		     ~NETDEV_ALIGN_CONST;
 	alloc_size += sizeof_priv + NETDEV_ALIGN_CONST;
 
@@ -3637,7 +3637,7 @@ struct net_device *alloc_netdev_mq(int sizeof_priv, const char *name,
 		dev->priv = ((char *)dev +
 			     ((sizeof(struct net_device) +
 			       (sizeof(struct net_device_subqueue) *
-				queue_count) + NETDEV_ALIGN_CONST)
+				(queue_count - 1)) + NETDEV_ALIGN_CONST)
 			      & ~NETDEV_ALIGN_CONST));
 	}
 

^ permalink raw reply related

* Re: net/ipv4/inetpeer.c stack warnings
From: Gabriel C @ 2007-07-19 13:10 UTC (permalink / raw)
  To: Patrick McHardy; +Cc: Linux Kernel Mailing List, netdev
In-Reply-To: <469F5DBB.3080106@trash.net>

Patrick McHardy wrote:
> Gabriel C wrote:
>   
>> Hello ,
>>
>> I noticed on current git this warning in net/ipv4/inetpeer.c
>>     
>
> Yeah, I have no idea why the gcc people thought that this was
> something worth warning about. Especially since explicitly
> checking for != NULL silences the warning again.
>
>
>   

Thx , the patch fixes that for me.

Gabriel

^ permalink raw reply

* Re: net/ipv4/inetpeer.c stack warnings
From: Patrick McHardy @ 2007-07-19 12:48 UTC (permalink / raw)
  To: Gabriel C; +Cc: Linux Kernel Mailing List, netdev
In-Reply-To: <469F5941.7090900@googlemail.com>

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

Gabriel C wrote:
> Hello ,
>
> I noticed on current git this warning in net/ipv4/inetpeer.c

Yeah, I have no idea why the gcc people thought that this was
something worth warning about. Especially since explicitly
checking for != NULL silences the warning again.



[-- Attachment #2: x --]
[-- Type: text/plain, Size: 1267 bytes --]

[IPV4]: Fix inetpeer gcc-4.2 warnings

  CC      net/ipv4/inetpeer.o
net/ipv4/inetpeer.c: In function 'unlink_from_pool':
net/ipv4/inetpeer.c:297: warning: the address of 'stack' will always evaluate as 'true'
net/ipv4/inetpeer.c:297: warning: the address of 'stack' will always evaluate as 'true'
net/ipv4/inetpeer.c: In function 'inet_getpeer':
net/ipv4/inetpeer.c:409: warning: the address of 'stack' will always evaluate as 'true'
net/ipv4/inetpeer.c:409: warning: the address of 'stack' will always evaluate as 'true'

"Fix" by checking for != NULL.

Signed-off-by: Patrick McHardy <kaber@trash.net>
diff --git a/net/ipv4/inetpeer.c b/net/ipv4/inetpeer.c
index 2f44e61..f725f9f 100644
--- a/net/ipv4/inetpeer.c
+++ b/net/ipv4/inetpeer.c
@@ -158,7 +158,7 @@ static void unlink_from_unused(struct inet_peer *p)
 #define lookup(_daddr,_stack) 					\
 ({								\
 	struct inet_peer *u, **v;				\
-	if (_stack) {						\
+	if (_stack != NULL) {					\
 		stackptr = _stack;				\
 		*stackptr++ = &peer_root;			\
 	}							\
@@ -169,7 +169,7 @@ static void unlink_from_unused(struct inet_peer *p)
 			v = &u->avl_left;			\
 		else						\
 			v = &u->avl_right;			\
-		if (_stack)					\
+		if (_stack != NULL)				\
 			*stackptr++ = v;			\
 		u = *v;						\
 	}							\

^ permalink raw reply related

* Re: updated tcatm patches for kernel/iproute 2.6.22
From: Sami Farin @ 2007-07-19 11:35 UTC (permalink / raw)
  To: Linux Networking Mailing List
In-Reply-To: <469E9B24.2080002@trash.net>

On Thu, Jul 19, 2007 at 00:58:44 +0200, Patrick McHardy wrote:
> Sami Farin wrote:
> > I had used tcatm patch with 2.6.16 kernel and I was
> > happy with it.
> > Now I patched Linux kernel 2.6.22 and iproute-2.6.22
> > for tcatm.  Seems to work (TM).  Only HTB tested.
> 
> 
> Let me repeat again before we get in an endless discussion
> again - I will NACK these patches until we have a solution
> that works for all qdiscs.

I googled for
"Accurate packet scheduling for ATM" mchardy
and found some threads from 2006.

http://www.mail-archive.com/netdev@vger.kernel.org/msg15706.html
http://www.mail-archive.com/netdev@vger.kernel.org/msg21395.html

So, Patrick:
1) what is this "solution"?  do you have code?
   or a brief explanation...
2) are you still working on this?
   is somebody else?
3) do you have patches for iproute/kernel 2.6.22?
4) if you have some patches, but they are not yet
   complete, what else needs to be done?

-- 
Do what you love because life is too short for anything else.


^ permalink raw reply

* net/ipv4/inetpeer.c stack warnings
From: Gabriel C @ 2007-07-19 12:29 UTC (permalink / raw)
  To: Linux Kernel Mailing List; +Cc: netdev

Hello ,

I noticed on current git this warning in net/ipv4/inetpeer.c

...

  CC      net/ipv4/inetpeer.o
net/ipv4/inetpeer.c: In function 'unlink_from_pool':
net/ipv4/inetpeer.c:297: warning: the address of 'stack' will always
evaluate as 'true'
net/ipv4/inetpeer.c:297: warning: the address of 'stack' will always
evaluate as 'true'
net/ipv4/inetpeer.c: In function 'inet_getpeer':
net/ipv4/inetpeer.c:409: warning: the address of 'stack' will always
evaluate as 'true'
net/ipv4/inetpeer.c:409: warning: the address of 'stack' will always
evaluate as 'true'


...

Regards,

Gabriel C

^ permalink raw reply

* Re: ~3 hours old git tree: Virtual device lo asks to queue packet!
From: Arkadiusz Miskiewicz @ 2007-07-19 11:52 UTC (permalink / raw)
  To: Patrick McHardy; +Cc: Peter P Waskiewicz Jr, netdev
In-Reply-To: <469F38BE.4030401@trash.net>

On Thursday 19 of July 2007, Patrick McHardy wrote:
> Patrick McHardy wrote:
> > Arkadiusz Miskiewicz wrote:
> >>>>CONFIG_NETDEVICES_MULTIQUEUE=y
> >>>
> >>>Does it go away if you disable this option?
> >>
> >>Yes, it goes away after disabling this.
> >
> > I don't see a bug in the code itself, maybe the queue_mapping points
> > to an invalid subqueue. Could you please try this patch and post the
> > output?
>
> OK I see what the problem is. The loopback device is statically
> allocated, so it doesn't have any room for the subqueues reserved.
>
> The easiest fix would be to use egress_subqueue[1] in struct
> net_device, but I think that may cause warnings with newer gccs
> when using a constant index that is > 0. OTOH using constant
> indices doesn't seem to make much sense for the subqueue array.
>
> Arkadiusz, does this patch fix the problem?

It fixes the problem for me.

-- 
Arkadiusz Miśkiewicz        PLD/Linux Team
arekm / maven.pl            http://ftp.pld-linux.org/

^ permalink raw reply

* Re: tg3 issues
From: Neil Horman @ 2007-07-19 11:37 UTC (permalink / raw)
  To: pradeep singh; +Cc: patric, linux-net, netdev
In-Reply-To: <366312910707190419se33ddd2o3870b5684e077276@mail.gmail.com>

On Thu, Jul 19, 2007 at 04:49:13PM +0530, pradeep singh wrote:
> CCing: netdev
> 
> On 7/19/07, patric <pakar@imperialnet.org> wrote:
> >Hi,
> >
> >
> >To start with, i'm not sure if this should go to the dev or user list,
> >but i'll start here..
> >
> >
> >I'm currently running a nfsroot via a Broadcom NetXtreme 1000-SX card
> >(BCM5701) and i have a big problem with the tg3 drivers autonegotiation.
> >
> >The issue seems to be that when the kernel comes so far as it's trying
> >to mount the boot the autonegotiation has not yet completed and then
> >causes a panic since it cannot mount the nfsroot.
> >
> >
> > From some debugging i have done here the issues seems to be related to
> >the flowcontrol configuration, and just to make it a bit more fun it
> >does work some of the time.. (around once every 5-10 attempts.)
> >
> >
> >On the console it looks something like this when failing. (written from
> >memory since i don't have netconsole enabled)
> >
> >tg3: eth0: Link is up at 1000 Mbps, full duplex.
> >tg3: eth0: Flow control is off for TX and off for RX.
> >IP-Config: Complete:
> >      device=eth0, addr=192.168.1.10, mask=255.255.255.0,
> >gw=255.255.255.255,
> >     host=amd, domain=, nis-domain=(none),
> >     bootserver=255.255.255.255, rootserver=192.168.1.1, rootpath=
> >Root-NFS: unknown option: nolocks
> >Looking up port of RPC 100003/3 on 192.168.1.1
> >rpcbind: server 192.168.1.1 not responding, timed out
> >
> >Root-NFS: Unable to get nfsd port number from server, using default
> >
> >Looking up port of RPC 100003/3 on 192.168.1.1
> >rpcbind: server 192.168.1.1 not responding, timed out
> >
> >Root-NFS: Unable to get nfsd port number from server, using default
> >
> >
> >and so on until it panics...
> >

IIRC, there are two main problems in this typ of situation

1) Spanning tree convergence
2) Firmware initalization latency

If you are running spanning tree on your network, it can take up to 2 minutes
before your port will forward frames properly.  if you have the options
available, disable spanning tree on the switch port connected to your host
system, or at least enable portfast if it is an option.  That should fix any
spanning tree issues you have

If the tg3 card is just taking a long time to initalize, there is not too much
you can do about it.  If your goal is to use nfs root, I would, instead of
enabling nfs-root as a kernel config option, I would create an initramfs that:
A) Brings up your NIC
B) Mounts your nfs partition
C) executes a switch_root or pivot_root operation

That way you can calibrate a delay between steps (A) and (B) in your initramfs
init script

Regards
Neil

-- 
/***************************************************
 *Neil Horman
 *Software Engineer
 *Red Hat, Inc.
 *nhorman@tuxdriver.com
 *gpg keyid: 1024D / 0x92A74FA1
 *http://pgp.mit.edu
 ***************************************************/

^ permalink raw reply

* Re: tg3 issues
From: pradeep singh @ 2007-07-19 11:19 UTC (permalink / raw)
  To: patric; +Cc: linux-net, netdev
In-Reply-To: <469E78A2.80904@imperialnet.org>

CCing: netdev

On 7/19/07, patric <pakar@imperialnet.org> wrote:
> Hi,
>
>
> To start with, i'm not sure if this should go to the dev or user list,
> but i'll start here..
>
>
> I'm currently running a nfsroot via a Broadcom NetXtreme 1000-SX card
> (BCM5701) and i have a big problem with the tg3 drivers autonegotiation.
>
> The issue seems to be that when the kernel comes so far as it's trying
> to mount the boot the autonegotiation has not yet completed and then
> causes a panic since it cannot mount the nfsroot.
>
>
>  From some debugging i have done here the issues seems to be related to
> the flowcontrol configuration, and just to make it a bit more fun it
> does work some of the time.. (around once every 5-10 attempts.)
>
>
> On the console it looks something like this when failing. (written from
> memory since i don't have netconsole enabled)
>
> tg3: eth0: Link is up at 1000 Mbps, full duplex.
> tg3: eth0: Flow control is off for TX and off for RX.
> IP-Config: Complete:
>       device=eth0, addr=192.168.1.10, mask=255.255.255.0,
> gw=255.255.255.255,
>      host=amd, domain=, nis-domain=(none),
>      bootserver=255.255.255.255, rootserver=192.168.1.1, rootpath=
> Root-NFS: unknown option: nolocks
> Looking up port of RPC 100003/3 on 192.168.1.1
> rpcbind: server 192.168.1.1 not responding, timed out
>
> Root-NFS: Unable to get nfsd port number from server, using default
>
> Looking up port of RPC 100003/3 on 192.168.1.1
> rpcbind: server 192.168.1.1 not responding, timed out
>
> Root-NFS: Unable to get nfsd port number from server, using default
>
>
> and so on until it panics...
>
>
> And for a successful attempt:
>
> tg3: eth0: Link is up at 1000 Mbps, full duplex.
> tg3: eth0: Flow control is off for TX and off for RX.
> IP-Config: Complete:
>       device=eth0, addr=192.168.1.10, mask=255.255.255.0,
> gw=255.255.255.255,
>      host=amd, domain=, nis-domain=(none),
>      bootserver=255.255.255.255, rootserver=192.168.1.1, rootpath=
> Root-NFS: unknown option: nolocks
> Looking up port of RPC 100003/3 on 192.168.1.1
> tg3: eth0: Link is up at 1000 Mbps, full duplex.
> tg3: eth0: Flow control is on for TX and on for RX.
> rpcbind: server 192.168.1.1 not responding, timed out
> Root-NFS: Unable to get nfsd port number from server, using default
> Looking up port of RPC 100005/3 on 192.168.1.1
> VFS: Mounted root (nfs filesystem).
>
>
> Also, if  i get the "flowcontrol is on" before it tries to mount the
> root it does not report any issues. And this is also not unique to NFS
> booting, but anytime i bring the link up (when not using it for booting).
>
>
> Currently i'm running 2.6.22-git9 (for testing) and i experience the
> same issues in plain 2.6.22 and before.
>
>
> Regards,
>
> Patric
>
>
>
>
>
>
> -
> To unsubscribe from this list: send the line "unsubscribe linux-net" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>


-- 
play the game

^ permalink raw reply

* Re: [RFC 2/3] TCP: cubic - eliminate use of receive time stamp
From: Ilpo Järvinen @ 2007-07-19 10:47 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: David S. Miller, Sangtae Ha, Luca De Cicco, Gavin McCullagh,
	Netdev
In-Reply-To: <20070719074906.634042117@linux-foundation.org>

Hi all,

It's definately better to do rtt measurements in the proper callback
like you're suggesting since the call to cong_avoid depends on too many
factors (ECN, FRTO) that shouldn't inhibit rtt sampling though they,
for a period, determine how cwnd must be set...

-- 
 i.

On Thu, 19 Jul 2007, Stephen Hemminger wrote:

> Remove use of received timestamp option value from RTT calculation in Cubic.
> A hostile receiver may be returning a larger timestamp option than the original
> value. This would cause the sender to believe the malevolent receiver had
> a larger RTT and because Cubic tries to provide some RTT friendliness, the
> sender would then favor the liar.
> 
> Instead, use the jiffie resolutionRTT value already computed and
> passed back after ack.
> 
> Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>
> 
> --- a/net/ipv4/tcp_cubic.c	2007-07-19 08:21:42.000000000 +0100
> +++ b/net/ipv4/tcp_cubic.c	2007-07-19 08:24:58.000000000 +0100
> @@ -246,38 +246,12 @@ static inline void bictcp_update(struct 
>  		ca->cnt = 1;
>  }
>  
> -
> -/* Keep track of minimum rtt */
> -static inline void measure_delay(struct sock *sk)
> -{
> -	const struct tcp_sock *tp = tcp_sk(sk);
> -	struct bictcp *ca = inet_csk_ca(sk);
> -	u32 delay;
> -
> -	/* No time stamp */
> -	if (!(tp->rx_opt.saw_tstamp && tp->rx_opt.rcv_tsecr) ||
> -	     /* Discard delay samples right after fast recovery */
> -	    (s32)(tcp_time_stamp - ca->epoch_start) < HZ)
> -		return;
> -
> -	delay = (tcp_time_stamp - tp->rx_opt.rcv_tsecr)<<3;
> -	if (delay == 0)
> -		delay = 1;
> -
> -	/* first time call or link delay decreases */
> -	if (ca->delay_min == 0 || ca->delay_min > delay)
> -		ca->delay_min = delay;
> -}
> -
>  static void bictcp_cong_avoid(struct sock *sk, u32 ack,
>  			      u32 seq_rtt, u32 in_flight, int data_acked)
>  {
>  	struct tcp_sock *tp = tcp_sk(sk);
>  	struct bictcp *ca = inet_csk_ca(sk);
>  
> -	if (data_acked)
> -		measure_delay(sk);
> -
>  	if (!tcp_is_cwnd_limited(sk, in_flight))
>  		return;
>  
> @@ -337,14 +311,30 @@ static void bictcp_state(struct sock *sk
>  static void bictcp_acked(struct sock *sk, u32 cnt, s32 rtt_us)
>  {
>  	const struct inet_connection_sock *icsk = inet_csk(sk);
> +	struct bictcp *ca = inet_csk_ca(sk);
> +	u32 delay;
>  
>  	if (cnt > 0 && icsk->icsk_ca_state == TCP_CA_Open) {
> -		struct bictcp *ca = inet_csk_ca(sk);
>  		cnt -= ca->delayed_ack >> ACK_RATIO_SHIFT;
>  		ca->delayed_ack += cnt;
>  	}
> -}
>  
> +	/* Some calls are for duplicates without timetamps */
> +	if (rtt_us < 0)
> +		return;
> +
> +	/* Discard delay samples right after fast recovery */
> +	if ((s32)(tcp_time_stamp - ca->epoch_start) < HZ)
> +		return;
> +
> +	delay = usecs_to_jiffies(rtt_us) << 3;
> +	if (delay == 0)
> +		delay = 1;
> +
> +	/* first time call or link delay decreases */
> +	if (ca->delay_min == 0 || ca->delay_min > delay)
> +		ca->delay_min = delay;
> +}
>  
>  static struct tcp_congestion_ops cubictcp = {
>  	.init		= bictcp_init,
> 
> 



^ permalink raw reply

* Re: [patch 0/3] dynamic generic netlink multicast
From: Johannes Berg @ 2007-07-19 10:39 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Patrick McHardy, Zhang Rui, jamal
In-Reply-To: <20070717122706.899784000@sipsolutions.net>

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

Oh, this patchset triggers a current bug in slub, see:

http://article.gmane.org/gmane.linux.kernel/556887

I forgot mentioning that yesterday, I hope it's not causing any trouble.
Also, I had expected it to be fixed soonish, but can't see that it is.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 190 bytes --]

^ permalink raw reply

* Re: ~3 hours old git tree: Virtual device lo asks to queue packet!
From: Patrick McHardy @ 2007-07-19 10:11 UTC (permalink / raw)
  To: Peter P Waskiewicz Jr; +Cc: Arkadiusz Miskiewicz, netdev
In-Reply-To: <469F2E14.9050200@trash.net>

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

Patrick McHardy wrote:
> Arkadiusz Miskiewicz wrote:
> 
>>>>CONFIG_NETDEVICES_MULTIQUEUE=y
>>>
>>>Does it go away if you disable this option?
>>
>>
>>Yes, it goes away after disabling this.
> 
> 
> I don't see a bug in the code itself, maybe the queue_mapping points
> to an invalid subqueue. Could you please try this patch and post the
> output?


OK I see what the problem is. The loopback device is statically
allocated, so it doesn't have any room for the subqueues reserved.

The easiest fix would be to use egress_subqueue[1] in struct
net_device, but I think that may cause warnings with newer gccs
when using a constant index that is > 0. OTOH using constant
indices doesn't seem to make much sense for the subqueue array.

Arkadiusz, does this patch fix the problem?

[-- Attachment #2: x --]
[-- Type: text/plain, Size: 1328 bytes --]

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index da7a13c..bf9399c 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -575,7 +575,7 @@ struct net_device
 
 	/* The TX queue control structures */
 	unsigned int			egress_subqueue_count;
-	struct net_device_subqueue	egress_subqueue[0];
+	struct net_device_subqueue	egress_subqueue[1];
 };
 #define to_net_dev(d) container_of(d, struct net_device, dev)
 
diff --git a/net/core/dev.c b/net/core/dev.c
index 13a0d9f..4af0207 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3619,7 +3619,7 @@ struct net_device *alloc_netdev_mq(int sizeof_priv, const char *name,
 
 	/* ensure 32-byte alignment of both the device and private area */
 	alloc_size = (sizeof(*dev) + NETDEV_ALIGN_CONST +
-		     (sizeof(struct net_device_subqueue) * queue_count)) &
+		     (sizeof(struct net_device_subqueue) * (queue_count - 1))) &
 		     ~NETDEV_ALIGN_CONST;
 	alloc_size += sizeof_priv + NETDEV_ALIGN_CONST;
 
@@ -3637,7 +3637,7 @@ struct net_device *alloc_netdev_mq(int sizeof_priv, const char *name,
 		dev->priv = ((char *)dev +
 			     ((sizeof(struct net_device) +
 			       (sizeof(struct net_device_subqueue) *
-				queue_count) + NETDEV_ALIGN_CONST)
+				(queue_count - 1)) + NETDEV_ALIGN_CONST)
 			      & ~NETDEV_ALIGN_CONST));
 	}
 

^ permalink raw reply related

* Re: [PATCH v2 1/3] netlink: allocate group bitmaps dynamically
From: Johannes Berg @ 2007-07-19 10:08 UTC (permalink / raw)
  To: David Miller; +Cc: kaber, netdev, rui.zhang, hadi
In-Reply-To: <20070718.153404.00455892.davem@davemloft.net>

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

On Wed, 2007-07-18 at 15:34 -0700, David Miller wrote:

> > All three patches look good to me.
> > 
> > Acked-by: Patrick McHardy <kaber@trash.net>
> > 
> > and for Jamal again ..
> > 
> > Acked-by: Jamal Hadi Salim <hadi@cyberus.ca>
> 
> Aha, Johannes did resubmit all 3 patches, they arrived out
> of order here which confused me :-)
> 
> I'll apply these, thanks.

Thanks. And sorry about the out-of-order, my mistake because I'd noticed
a small bug in one patch and didn't rewrite all three emails.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 190 bytes --]

^ permalink raw reply

* [PATCH iproute2] show multicast groups
From: Johannes Berg @ 2007-07-19  9:13 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev, Jamal

Update the included version of the genetlink.h header to the multicast
group API and make the generic netlink controller part show multicast
groups where applicable. Also fix two typos.

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>

diff --git a/genl/ctrl.c b/genl/ctrl.c
index fe010f3..72d39a0 100644
--- a/genl/ctrl.c
+++ b/genl/ctrl.c
@@ -6,7 +6,8 @@
  *		as published by the Free Software Foundation; either version
  *		2 of the License, or (at your option) any later version.
  *
- * Authors:  J Hadi Salim (hadi@cyberus.ca)
+ * Authors:	J Hadi Salim (hadi@cyberus.ca)
+ *		Johannes Berg (johannes@sipsolutions.net)
  */
 
 #include <stdio.h>
@@ -23,6 +24,8 @@
 #include "genl_utils.h"
 
 #define GENL_MAX_FAM_OPS	256
+#define GENL_MAX_FAM_GRPS	256
+
 static int usage(void)
 {
 	fprintf(stderr,"Usage: ctrl <CMD>\n" \
@@ -82,7 +85,7 @@ int genl_ctrl_resolve_family(const char *family)
 		}
 
 		if (ghdr->cmd != CTRL_CMD_NEWFAMILY) {
-			fprintf(stderr, "Unkown controller command %d\n", ghdr->cmd);
+			fprintf(stderr, "Unknown controller command %d\n", ghdr->cmd);
 			goto errout;
 		}
 
@@ -151,6 +154,26 @@ static int print_ctrl_cmds(FILE *fp, struct rtattr *arg, __u32 ctrl_ver)
 
 }
 
+static int print_ctrl_grp(FILE *fp, struct rtattr *arg, __u32 ctrl_ver)
+{
+	struct rtattr *tb[CTRL_ATTR_MCAST_GRP_MAX + 1];
+
+	if (arg == NULL)
+		return -1;
+
+	parse_rtattr_nested(tb, CTRL_ATTR_MCAST_GRP_MAX, arg);
+	if (tb[2]) {
+		__u32 *id = RTA_DATA(tb[CTRL_ATTR_MCAST_GRP_ID]);
+		fprintf(fp, " ID-0x%x ",*id);
+	}
+	if (tb[1]) {
+		char *name = RTA_DATA(tb[CTRL_ATTR_MCAST_GRP_NAME]);
+		fprintf(fp, " name: %s ", name);
+	}
+	return 0;
+
+}
+
 /*
  * The controller sends one nlmsg per family
 */
@@ -172,8 +195,10 @@ static int print_ctrl(const struct sockaddr_nl *who, struct nlmsghdr *n,
 
 	if (ghdr->cmd != CTRL_CMD_GETFAMILY &&
 	    ghdr->cmd != CTRL_CMD_DELFAMILY &&
-	    ghdr->cmd != CTRL_CMD_NEWFAMILY) {
-		fprintf(stderr, "Unkown controller command %d\n", ghdr->cmd);
+	    ghdr->cmd != CTRL_CMD_NEWFAMILY &&
+	    ghdr->cmd != CTRL_CMD_NEWMCAST_GRP &&
+	    ghdr->cmd != CTRL_CMD_DELMCAST_GRP) {
+		fprintf(stderr, "Unknown controller command %d\n", ghdr->cmd);
 		return 0;
 	}
 
@@ -229,6 +254,29 @@ static int print_ctrl(const struct sockaddr_nl *who, struct nlmsghdr *n,
 		/* end of family::cmds definitions .. */
 		fprintf(fp,"\n");
 	}
+
+	if (tb[CTRL_ATTR_MCAST_GROUPS]) {
+		struct rtattr *tb2[GENL_MAX_FAM_GRPS + 1];
+		int i;
+
+		parse_rtattr_nested(tb2, GENL_MAX_FAM_GRPS,
+				    tb[CTRL_ATTR_MCAST_GROUPS]);
+		fprintf(fp, "\tmulticast groups:\n");
+
+		for (i = 0; i < GENL_MAX_FAM_GRPS; i++) {
+			if (tb2[i]) {
+				fprintf(fp, "\t\t#%d: ", i);
+				if (0 > print_ctrl_grp(fp, tb2[i], ctrl_v))
+					fprintf(fp, "Error printing group\n");
+				/* for next group */
+				fprintf(fp,"\n");
+			}
+		}
+
+		/* end of family::groups definitions .. */
+		fprintf(fp,"\n");
+	}
+
 	fflush(fp);
 	return 0;
 }
diff --git a/include/linux/genetlink.h b/include/linux/genetlink.h
index f7a9377..7da02c9 100644
--- a/include/linux/genetlink.h
+++ b/include/linux/genetlink.h
@@ -39,6 +39,9 @@ enum {
 	CTRL_CMD_NEWOPS,
 	CTRL_CMD_DELOPS,
 	CTRL_CMD_GETOPS,
+	CTRL_CMD_NEWMCAST_GRP,
+	CTRL_CMD_DELMCAST_GRP,
+	CTRL_CMD_GETMCAST_GRP, /* unused */
 	__CTRL_CMD_MAX,
 };
 
@@ -52,6 +55,7 @@ enum {
 	CTRL_ATTR_HDRSIZE,
 	CTRL_ATTR_MAXATTR,
 	CTRL_ATTR_OPS,
+	CTRL_ATTR_MCAST_GROUPS,
 	__CTRL_ATTR_MAX,
 };
 
@@ -66,4 +70,13 @@ enum {
 
 #define CTRL_ATTR_OP_MAX (__CTRL_ATTR_OP_MAX - 1)
 
+enum {
+	CTRL_ATTR_MCAST_GRP_UNSPEC,
+	CTRL_ATTR_MCAST_GRP_NAME,
+	CTRL_ATTR_MCAST_GRP_ID,
+	__CTRL_ATTR_MCAST_GRP_MAX,
+};
+
+#define CTRL_ATTR_MCAST_GRP_MAX (__CTRL_ATTR_MCAST_GRP_MAX - 1)
+
 #endif	/* __LINUX_GENERIC_NETLINK_H */



^ permalink raw reply related

* [PATCH] enforce generic netlink multicast group API
From: Johannes Berg @ 2007-07-18 22:05 UTC (permalink / raw)
  To: David Miller; +Cc: Patrick McHardy, netdev, jamal

This patch changes genlmsg_multicast() to take a struct
genl_multicast_group * instead of the ID, that way users are forced to
register a group before being able to use genlmsg_multicast().

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>

---
Just seemed like a good idea to enforce using this API so we won't get
into any trouble with somebody using the family number as the group,
nobody noticing and then being stuck with that userspace ABI. Not sure
if it's worth it, but it doesn't really make a difference code-wise, I
guess it shouldn't even generate different code.

---
 include/net/genetlink.h |    9 +++++----
 net/netlink/genetlink.c |   10 ++++++++--
 2 files changed, 13 insertions(+), 6 deletions(-)

--- wireless-dev.orig/include/net/genetlink.h	2007-07-18 23:54:48.680937160 +0200
+++ wireless-dev/include/net/genetlink.h	2007-07-18 23:55:44.330937160 +0200
@@ -7,7 +7,7 @@
 /**
  * struct genl_multicast_group - generic netlink multicast group
  * @name: name of the multicast group, names are per-family
- * @id: multicast group ID, assigned by the core, to use with
+ * @id: multicast group ID, assigned by the core, used by
  *      genlmsg_multicast().
  * @list: list entry for linking
  * @family: pointer to family, need not be set before registering
@@ -171,13 +171,14 @@ static inline int genlmsg_cancel(struct 
  * genlmsg_multicast - multicast a netlink message
  * @skb: netlink message as socket buffer
  * @pid: own netlink pid to avoid sending to yourself
- * @group: multicast group id
+ * @group: multicast group
  * @flags: allocation flags
  */
 static inline int genlmsg_multicast(struct sk_buff *skb, u32 pid,
-				    unsigned int group, gfp_t flags)
+				    struct genl_multicast_group *group,
+				    gfp_t flags)
 {
-	return nlmsg_multicast(genl_sock, skb, pid, group, flags);
+	return nlmsg_multicast(genl_sock, skb, pid, group->id, flags);
 }
 
 /**
--- wireless-dev.orig/net/netlink/genetlink.c	2007-07-18 23:54:08.210937160 +0200
+++ wireless-dev/net/netlink/genetlink.c	2007-07-18 23:59:29.470937160 +0200
@@ -730,7 +730,7 @@ static int genl_ctrl_event(int event, vo
 		if (IS_ERR(msg))
 			return PTR_ERR(msg);
 
-		genlmsg_multicast(msg, 0, GENL_ID_CTRL, GFP_KERNEL);
+		genlmsg_multicast(msg, 0, &notify_grp, GFP_KERNEL);
 		break;
 	case CTRL_CMD_NEWMCAST_GRP:
 	case CTRL_CMD_DELMCAST_GRP:
@@ -738,7 +738,7 @@ static int genl_ctrl_event(int event, vo
 		if (IS_ERR(msg))
 			return PTR_ERR(msg);
 
-		genlmsg_multicast(msg, 0, GENL_ID_CTRL, GFP_KERNEL);
+		genlmsg_multicast(msg, 0, &notify_grp, GFP_KERNEL);
 		break;
 	}
 
@@ -754,6 +754,12 @@ static struct genl_ops genl_ctrl_ops = {
 
 static struct genl_multicast_group notify_grp = {
 	.name		= "notify",
+	/*
+	 * The ID is fixed because it is userspace ABI,
+	 * pre-assign so we already have a valid one
+	 * during registration of the controller family.
+	 */
+	.id		= GENL_ID_CTRL,
 };
 
 static int __init genl_init(void)



^ permalink raw reply

* Re: TCP stalls in current git, possibly splice related
From: Johannes Berg @ 2007-07-18 21:24 UTC (permalink / raw)
  To: Jens Axboe; +Cc: James Morris, David S. Miller, netdev
In-Reply-To: <1184756332.4658.16.camel@johannes.berg>

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

On Wed, 2007-07-18 at 12:58 +0200, Johannes Berg wrote:

> Yeah, I only need to try to remember when I last *didn't* see the
> problem, and hope that my analysis about it being on the receiver side
> is correct :)

Hm. I went back to 2.6.20 on the receiver side and still saw it. Maybe I
need to go back further?

With strace (the synergyc process) and tcpdump (on the interface) I
continually see packets coming in but the read sometimes pauses for 8
seconds... Not sure why. Very weird, but right now I don't have enough
time for investigating.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 190 bytes --]

^ permalink raw reply

* Re: kmap_atomic() oopses in current mainline
From: Evgeniy Polyakov @ 2007-07-19 10:01 UTC (permalink / raw)
  To: Andrew Morton; +Cc: netdev, linux-kernel-announce, Dan Williams
In-Reply-To: <20070719023831.400f7905.akpm@linux-foundation.org>

On Thu, Jul 19, 2007 at 02:38:31AM -0700, Andrew Morton (akpm@linux-foundation.org) wrote:
> > > is very wrong if both ASYNC_TX_KMAP_DST and ASYNC_TX_KMAP_SRC can ever be
> > > set.  We'll end up using the same kmap slot for both src add dest and we
> > > get either corrupted data or a BUG.
> > 
> > So far it can not since the only user is raid code, which only allows to
> > perform either reading from bio or writing into one, which requires only
> > one mapping.
> 
> hm, so we got lucky?

I would say it was intentionally, current code can perform only one
operation in a time. Of course changing KM_USER from 0 to 1 in second
kmap_atomic will not force oceans to run out of coasts.

Kind of:

diff --git a/crypto/async_tx/async_memcpy.c b/crypto/async_tx/async_memcpy.c
index a973f4e..a48c7f3 100644
--- a/crypto/async_tx/async_memcpy.c
+++ b/crypto/async_tx/async_memcpy.c
@@ -94,7 +94,7 @@ async_memcpy(struct page *dest, struct page *src, unsigned int dest_offset,
 			dest_buf = page_address(dest) + dest_offset;
 
 		if (flags & ASYNC_TX_KMAP_SRC)
-			src_buf = kmap_atomic(src, KM_USER0) + src_offset;
+			src_buf = kmap_atomic(src, KM_USER1) + src_offset;
 		else
 			src_buf = page_address(src) + src_offset;
 
@@ -104,7 +104,7 @@ async_memcpy(struct page *dest, struct page *src, unsigned int dest_offset,
 			kunmap_atomic(dest_buf, KM_USER0);
 
 		if (flags & ASYNC_TX_KMAP_SRC)
-			kunmap_atomic(src_buf, KM_USER0);
+			kunmap_atomic(src_buf, KM_USER1);
 
 		async_tx_sync_epilog(flags, depend_tx, cb_fn, cb_param);
 	}

> > Btw, shouldn't it always be kmap_atomic() even if flag is not set.
> > That pages are usual one returned by alloc_page().
> 
> The code would work OK if the kmap_atomic()s were unconditional, but it
> would be a bit more expensive if the page is in highmem and we don't
> actually intend to access it with the CPU.
> 
> kmap_atomic() against a non-highmem page is basically free: just an
> additional test_bit().

As far as I recall there was an intention to do async memory copy to
userspace, so likely kmapping is a good idea.

-- 
	Evgeniy Polyakov

^ permalink raw reply related

* Re: kmap_atomic() oopses in current mainline
From: Andrew Morton @ 2007-07-19  9:38 UTC (permalink / raw)
  To: Evgeniy Polyakov; +Cc: netdev, linux-kernel-announce, Dan Williams
In-Reply-To: <20070719092856.GA15839@2ka.mipt.ru>

On Thu, 19 Jul 2007 13:28:56 +0400 Evgeniy Polyakov <johnpol@2ka.mipt.ru> wrote:

> Hi.
> 
> On Thu, Jul 19, 2007 at 01:33:04AM -0700, Andrew Morton (akpm@linux-foundation.org) wrote:
> > I don't think the 2-year-old Vaio has offload engine support ;) Dan, this:
> > 
> > +		if (flags & ASYNC_TX_KMAP_DST)
> > +			dest_buf = kmap_atomic(dest, KM_USER0) + dest_offset;
> > +		else
> > +			dest_buf = page_address(dest) + dest_offset;
> > +
> > +		if (flags & ASYNC_TX_KMAP_SRC)
> > +			src_buf = kmap_atomic(src, KM_USER0) + src_offset;
> > +		else
> > +			src_buf = page_address(src) + src_offset;
> > +
> > +		memcpy(dest_buf, src_buf, len);
> > +
> > +		if (flags & ASYNC_TX_KMAP_DST)
> > +			kunmap_atomic(dest_buf, KM_USER0);
> > +
> > +		if (flags & ASYNC_TX_KMAP_SRC)
> > +			kunmap_atomic(src_buf, KM_USER0);
> > +
> > 
> > is very wrong if both ASYNC_TX_KMAP_DST and ASYNC_TX_KMAP_SRC can ever be
> > set.  We'll end up using the same kmap slot for both src add dest and we
> > get either corrupted data or a BUG.
> 
> So far it can not since the only user is raid code, which only allows to
> perform either reading from bio or writing into one, which requires only
> one mapping.

hm, so we got lucky?

> Btw, shouldn't it always be kmap_atomic() even if flag is not set.
> That pages are usual one returned by alloc_page().

The code would work OK if the kmap_atomic()s were unconditional, but it
would be a bit more expensive if the page is in highmem and we don't
actually intend to access it with the CPU.

kmap_atomic() against a non-highmem page is basically free: just an
additional test_bit().

^ permalink raw reply

* [PATCH 2/2] nbd: change a parameter's type to remove a memcpy call
From: Denis Cheng @ 2007-07-19  9:34 UTC (permalink / raw)
  To: Paul Clements
  Cc: netdev, linux-kernel, Pavel Machek, Steven Whitehouse,
	Andrew Morton, Denis Cheng
In-Reply-To: <11848376711601-git-send-email-crquan@gmail.com>

this memcpy looks so strange, in fact it's merely a pointer dereference,
so I change the parameter's type to refer it more directly,
this could make the memcpy not needed anymore.

in the function nbd_read_stat where nbd_find_request is only once called,
the parameter served should be transformed accordingly.

Signed-off-by: Denis Cheng <crquan@gmail.com>
---
 drivers/block/nbd.c |    7 ++-----
 1 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 86639c0..a4d8508 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -235,14 +235,11 @@ error_out:
 	return 1;
 }
 
-static struct request *nbd_find_request(struct nbd_device *lo, char *handle)
+static struct request *nbd_find_request(struct nbd_device *lo, struct request *xreq)
 {
 	struct request *req, *n;
-	struct request *xreq;
 	int err;
 
-	memcpy(&xreq, handle, sizeof(xreq));
-
 	err = wait_event_interruptible(lo->active_wq, lo->active_req != xreq);
 	if (unlikely(err))
 		goto out;
@@ -297,7 +294,7 @@ static struct request *nbd_read_stat(struct nbd_device *lo)
 		goto harderror;
 	}
 
-	req = nbd_find_request(lo, reply.handle);
+	req = nbd_find_request(lo, *(struct request **)reply.handle);
 	if (unlikely(IS_ERR(req))) {
 		result = PTR_ERR(req);
 		if (result != -ENOENT)
-- 
1.5.2.2


^ permalink raw reply related

* [PATCH 1/2] nbd: use list_for_each_entry_safe to make it more consolidated and readable
From: Denis Cheng @ 2007-07-19  9:34 UTC (permalink / raw)
  To: Paul Clements
  Cc: netdev, linux-kernel, Pavel Machek, Steven Whitehouse,
	Andrew Morton, Denis Cheng

Thus the traverse of the loop may delete nodes, use the safe version.

Signed-off-by: Denis Cheng <crquan@gmail.com>
---
 drivers/block/nbd.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index c129510..86639c0 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -237,8 +237,7 @@ error_out:
 
 static struct request *nbd_find_request(struct nbd_device *lo, char *handle)
 {
-	struct request *req;
-	struct list_head *tmp;
+	struct request *req, *n;
 	struct request *xreq;
 	int err;
 
@@ -249,8 +248,7 @@ static struct request *nbd_find_request(struct nbd_device *lo, char *handle)
 		goto out;
 
 	spin_lock(&lo->queue_lock);
-	list_for_each(tmp, &lo->queue_head) {
-		req = list_entry(tmp, struct request, queuelist);
+	list_for_each_entry_safe(req, n, &lo->queue_head, queuelist) {
 		if (req != xreq)
 			continue;
 		list_del_init(&req->queuelist);
-- 
1.5.2.2


^ permalink raw reply related

* [PATCH 2/2] Module for ip utility to support veth device
From: Pavel Emelyanov @ 2007-07-19  9:33 UTC (permalink / raw)
  To: David Miller, Patrick McHardy; +Cc: Linux Netdev List, devel
In-Reply-To: <469F2F30.5080602@openvz.org>

The link_veth.so itself.

Signed-off-by: Pavel Emelyanov <xemul@openvz.org>
Acked-by: Patrick McHardy <kaber@trash.net>

---

 Makefile    |    6 ++++-
 link_veth.c |   63 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 veth.h      |   12 +++++++++++
 3 files changed, 80 insertions(+), 1 deletion(-)

diff --git a/ip/Makefile b/ip/Makefile
index 9a5bfe3..b46bce3 100644
--- a/ip/Makefile
+++ b/ip/Makefile
@@ -8,8 +8,9 @@ RTMONOBJ=rtmon.o
 ALLOBJ=$(IPOBJ) $(RTMONOBJ)
 SCRIPTS=ifcfg rtpr routel routef
 TARGETS=ip rtmon
+LIBS=link_veth.so
 
-all: $(TARGETS) $(SCRIPTS)
+all: $(TARGETS) $(SCRIPTS) $(LIBS)
 
 ip: $(IPOBJ) $(LIBNETLINK) $(LIBUTIL)
 
@@ -24,3 +25,6 @@ clean:
 
 LDLIBS	+= -ldl
 LDFLAGS	+= -Wl,-export-dynamic
+
+%.so: %.c
+	$(CC) $(CFLAGS) -shared $< -o $@
diff --git a/ip/link_veth.c b/ip/link_veth.c
new file mode 100644
index 0000000..ded2cdd
--- /dev/null
+++ b/ip/link_veth.c
@@ -0,0 +1,63 @@
+/*
+ * link_veth.c	veth driver module
+ *
+ *		This program is free software; you can redistribute it and/or
+ *		modify it under the terms of the GNU General Public License
+ *		as published by the Free Software Foundation; either version
+ *		2 of the License, or (at your option) any later version.
+ *
+ * Authors:	Pavel Emelianov <xemul@openvz.org>
+ *
+ */
+
+#include <string.h>
+
+#include "utils.h"
+#include "ip_common.h"
+#include "veth.h"
+
+#define	IFNAMSIZ	16
+
+static void usage(void)
+{
+	printf("Usage: ip link add ... type veth "
+			"[peer <peer-name>] [mac <mac>] [peer_mac <mac>]\n");
+}
+
+static int veth_parse_opt(struct link_util *lu, int argc, char **argv,
+		struct nlmsghdr *hdr)
+{
+	char *name, *type, *link, *dev;
+	int err, len;
+	struct rtattr * data;
+
+	if (strcmp(argv[0], "peer") != 0) {
+		usage();
+		return -1;
+	}
+
+	data = NLMSG_TAIL(hdr);
+	addattr_l(hdr, 1024, VETH_INFO_PEER, NULL, 0);
+
+	hdr->nlmsg_len += sizeof(struct ifinfomsg);
+
+	err = iplink_parse(argc - 1, argv + 1, (struct iplink_req *)hdr,
+			&name, &type, &link, &dev);
+	if (err < 0)
+		return err;
+
+	if (name) {
+		len = strlen(name) + 1;
+		if (len > IFNAMSIZ)
+			invarg("\"name\" too long\n", *argv);
+		addattr_l(hdr, 1024, IFLA_IFNAME, name, len);
+	}
+
+	data->rta_len = (void *)NLMSG_TAIL(hdr) - (void *)data;
+	return argc - 1 - err;
+}
+
+struct link_util veth_link_util = {
+	.id = "veth",
+	.parse_opt = veth_parse_opt,
+};
diff --git a/ip/veth.h b/ip/veth.h
new file mode 100644
index 0000000..aa2e6f9
--- /dev/null
+++ b/ip/veth.h
@@ -0,0 +1,12 @@
+#ifndef __NET_VETH_H__
+#define __NET_VETH_H__
+
+enum {
+	VETH_INFO_UNSPEC,
+	VETH_INFO_PEER,
+
+	__VETH_INFO_MAX
+#define VETH_INFO_MAX	(__VETH_INFO_MAX - 1)
+};
+
+#endif

^ 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