Netdev List
 help / color / mirror / Atom feed
* Re: phy address in the device tree, vs auto probing
From: Mitch Bradley @ 2010-02-10 18:35 UTC (permalink / raw)
  To: Grant Likely; +Cc: Scott Wood, netdev, devicetree-discuss, Andy Fleming
In-Reply-To: <4B72FB38.7080909-D5eQfiDGL7eakBO8gow8eQ@public.gmane.org>

>
>>
>> On Wed, Feb 10, 2010 at 9:52 AM, John Linn <John.Linn-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org> wrote:
>>  
>>>> >> -----Original Message-----
>>>> >> From: glikely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org [mailto:glikely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org] On 
>>>> Behalf Of Grant Likely
>>>> >> Sent: Wednesday, February 10, 2010 9:44 AM
>>>> >> To: John Linn; devicetree-discuss; netdev
>>>> >> Subject: Re: phy address in the device tree, vs auto probing
>>>> >>
>>>> >> (cc'ing devicetree-discuss and netdev mailing lists)
>>>> >>
>>>> >> On Tue, Feb 9, 2010 at 4:23 PM, John Linn <John.Linn-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org> 
>>>> wrote:
>>>>      
>>>>> >> > Hi Grant,
>>>>> >> >
>>>>> >> > I notice that the OF driver for the mdio bus is not doing 
>>>>> auto probing.
>>>>> >> >
>>>>> >> > As we start putting in the phy layer in the emac drivers, the 
>>>>> device
>>>>> >> > trees tend to have the phy address in them, but we're not 
>>>>> sure we really
>>>>> >> > like that.
>>>>> >> >
>>>>> >> > We really think that being able to let the kernel find the 
>>>>> phy address
>>>>> >> > is a big benefit, otherwise this is one other piece of info 
>>>>> the user has
>>>>> >> > to know and get right.
>>>>> >> >
>>>>> >> > Am I missing something here?
>>>>>         
>>>> >>
>>>> >> No, you're not really missing something, but there is an inherent
>>>> >> complexity in what you're wanting to do.  Like i2c, MDIO is one of
>>>> >> those busses that is hard to probe reliable.  Some PHYs respond on
>>>> >> more than one address, and there is no way to determine which MAC a
>>>> >> PHY is wired up to.  Many PHYs can live on a single MDIO bus.  MACs
>>>> >> with their own MDIO busses may still get wired to a PHY on a 
>>>> different
>>>> >> bus.
>>>> >>
>>>> >> In the simple case where there is a one:one:one relationship 
>>>> between
>>>> >> MAC, MDIO bus and PHY, then it should be okay to probe the PHY,
>>>> >> correct?  The question then must be asked; how does the kernel
>>>> >> determine that it can use the simple case?  Nobody has yet 
>>>> defined a
>>>> >> way to describe that in the device tree; mostly because nobody has
>>>> >> needed to yet.
>>>> >>
>>>> >> So, it is possible to do what you want, but you need a way to
>>>> >> *explicitly* ask for that behaviour.  ie, some way to indicate in a
>>>> >> MAC node which MDIO bus the phy is on, and that the phy needs to be
>>>> >> probed for.  I think this should only be an option when the MDIO 
>>>> bus
>>>> >> has only one PHY.  Come up with a proposal and post it to the
>>>> >> devicetree-discuss mailing list.
>>>>       
>>> >
>>> > Here's a couple ideas. See what everyone thinks as I'm not stuck 
>>> on either.
>>> >
>>> > Thanks,
>>> > John
>>> >
>>> > 1. What if we just don't specific a phy address with a reg 
>>> property which would specify to auto probe it and find the phy as 
>>> illustrated below?
>>> >
>>> >
>>> >                Ethernet_MAC: ethernet@81000000 {
>>> >                        #address-cells = <1>;
>>> >                        #size-cells = <1>;
>>> >                        phy-handle = <&phy0>;
>>> >                        mdio {
>>> >                                #address-cells = <1>;
>>> >                                #size-cells = <0>;
>>> >                                phy0: phy@7 {
>>> >                                } ;
>>> >                        } ;
>>> >
>>> > 2. Or a special value (-1 or something not 0 - 31) in the phy 
>>> address that specifies to auto probe as illustrated below.
>>> >                                phy0: phy@7 {
>>> >                                        reg = <-1>;
>>> >                                } ;
>>>     
>>
>> I don't like abusing the reg property in this way.  I wonder if a new
>> empty property would be a better way to indicate this.  Maybe
>> "phy-probe-address;"?  It would also be important to specify in the
>> binding that only one phy node is allowed when phy-probe-address is
>> used.
>>
>> Also, without a known reg the 'phy@7' name is inaccurate.  Drop the @7.
>>
>> Scott, Andy: any thoughts?
>>   
>
> This case is somewhat similar to "wildcard nodes" on unprobed SCSI 
> buses, going all the way back to pre-1275 Open Boot.  Since full 
> probing of a SCSI bus could take a really long time (spin-up delays 
> etc), Open Boot would usually create a bus node for the host 
> controller and populate it with a disk node and a tape node, neither 
> of which had a reg property.  That meant that there was a good chance 
> that you might find such devices on that bus, but their specific SCSI 
> bus addresses had not yet been determined.  In addition to those 
> wildcard nodes, similar nodes with extant reg properties could also 
> appear, asserting the presence of a known device at the given 
> address.  The node matching algorithm first looks for an exact match 
> with a reg property, and failing that, looks for a wildcard match.
>

FYI, wildcard matching is defined in section 4.3.3 clause (b) and 
section 4.3.5 of IEEE 1275-1994.

^ permalink raw reply

* Corrupt /proc/interrupts with e1000e
From: Kelvin Ku @ 2010-02-10 18:31 UTC (permalink / raw)
  To: e1000-devel; +Cc: netdev, users

I see the following in /proc/interrupts

 55:          0          0          0     338331   PCI-MSI-edge      lan0-rx-0
 56:          0          0          0       2664   PCI-MSI-edge      lan0-tx-0
 57:      47230          0          0          0   PCI-MSI-edge      lan0
 58:        427          0          0          0   PCI-MSI-edge      �
 59:          0        277          0          0   PCI-MSI-edge      �����������
 60:          0          4          0          0   PCI-MSI-edge      lan1

Notice the corrupt entries for IRQs 58 and 59. Sometimes 55 and 56 are corrupt as well. Any idea why this is happening? I am running Fedora Core 12.

$ sudo ethtool -i lan0
driver: e1000e
version: 1.0.2-k2
firmware-version: 1.8-0
bus-info: 0000:06:00.0

$ uname -r
2.6.31.12-174.2.3.fc12.x86_64

This is the NIC:

$ lspci

06:00.0 Ethernet controller: Intel Corporation 82574L Gigabit Network Connection
07:00.0 Ethernet controller: Intel Corporation 82574L Gigabit Network Connection

- Kelvin
-- 
users mailing list
users@lists.fedoraproject.org
To unsubscribe or change subscription options:
https://admin.fedoraproject.org/mailman/listinfo/users
Guidelines: http://fedoraproject.org/wiki/Communicate/MailingListGuidelines

^ permalink raw reply

* Re: phy address in the device tree, vs auto probing
From: Scott Wood @ 2010-02-10 18:28 UTC (permalink / raw)
  To: Grant Likely; +Cc: netdev, devicetree-discuss, Andy Fleming
In-Reply-To: <fa686aa41002101014s43682e3cra55854b82a40bb5f-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

Grant Likely wrote:
>> 1. What if we just don't specific a phy address with a reg property which would specify to auto probe it and find the phy as illustrated below?
>>
>>
>>                Ethernet_MAC: ethernet@81000000 {
>>                        #address-cells = <1>;
>>                        #size-cells = <1>;
>>                        phy-handle = <&phy0>;
>>                        mdio {
>>                                #address-cells = <1>;
>>                                #size-cells = <0>;
>>                                phy0: phy@7 {
>>                                } ;
>>                        } ;
>>
>> 2. Or a special value (-1 or something not 0 - 31) in the phy address that specifies to auto probe as illustrated below.
>>                                phy0: phy@7 {
>>                                        reg = <-1>;
>>                                } ;
> 
> I don't like abusing the reg property in this way.  I wonder if a new
> empty property would be a better way to indicate this.  Maybe
> "phy-probe-address;"?  It would also be important to specify in the
> binding that only one phy node is allowed when phy-probe-address is
> used.
> 
> Also, without a known reg the 'phy@7' name is inaccurate.  Drop the @7.
> 
> Scott, Andy: any thoughts?

I'm not fond of the -1.  I'd prefer the explicit phy-probe-address property, 
though I don't mind too much using the absence of reg.

-Scott

^ permalink raw reply

* RX performance degradation with e1000e in Linux 2.6.31 / F12
From: Kelvin Ku @ 2010-02-10 18:24 UTC (permalink / raw)
  To: e1000-devel; +Cc: netdev, users

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

After upgrading from Linux 2.6.30 (Fedora Core 11) to 2.6.31 (F12), I am
experiencing significant packet loss on an Intel 82574L NIC running on the
e1000e driver. I was not experiencing this with kernel 2.6.30. I notice 2.6.30
uses e1000e version 0.3.3.4-k4 whereas 2.6.31 uses version 1.0.2-k2.

I have tried setting IntMode to 0, 1, and 2 and InterruptThrottleRate to 0, 1,
3 (the default), 1000, 5000, 10000, and 100000. I've also tried booting with
the "noapic" kernel parameter.

I am testing with ttcp, sending 100000 1450 byte UDP packets at about 910 Mbps.
With InterruptThrottleRate at 1, 3, 5000, or 10000, I see the following
behaviour on the receiver side:

        ttcp -u -4 -l 1450 -s -fm -r
ttcp-r: buflen=1450, nbuf=2048, align=16384/0, port=5001  udp
ttcp-r: socket
ttcp-r: 98486900 bytes in 1.22 real seconds = 617.22 Mbit/sec +++
ttcp-r: 67924 I/O calls, msec/call = 0.02, calls/sec = 55794.64
ttcp-r: 0.0user 0.0sys 0:01real 0% 0i+0d 0maxrss 0+0pf 4963+3csw

So in total (145000000 - 98486900)/1450 = 32078 out of 100000 packets were
dropped, or about 32%.

This is the difference between /proc/interrupts (the change in each counter)
before and after the test. lan0 is the interface being tested. Notice that
there are a significant number of interrupts on the "sequence error" interrupt;
I'm guessing that's 57:

 55:          0          0          0       8603 PCI-MSI-edge 
 56:          0          0          0         25 PCI-MSI-edge Q�����V 
 57:       4868          0          0          0 PCI-MSI-edge lan0 
 67:          0          0          2          0 PCI-MSI-edge ��������@�
 68:          0          0          0          0 PCI-MSI-edge 
 69:          0          0          0          0 PCI-MSI-edge lan1 

This is the difference between the output from 'ethtool -S lan0' before and
after the test; only fields which changed are shown:

: rx_broadcast:  585730 - 581046 = 4684
: rx_bytes:  931068459 - 822452567 = 108615892
: rx_csum_offload_good:  650149 - 577551 = 72598
: rx_long_byte_count:  931068459 - 822452567 = 108615892
: rx_missed_errors:  31003 - 6 = 30997
: rx_packets:  655692 - 583072 = 72620
: rx_smbus:  5784 - 5763 = 21
: tx_broadcast:  972 - 969 = 3
: tx_bytes:  388453 - 385439 = 3014
: tx_packets:  3025 - 3012 = 13

Notice the large rx_missed_errors count which indicates NIC FIFO or PCI bus
exhaustion.

If I disable interrupt throttling or set the limit very high, e.g., 100000, the
same test generates about 65,000 data interrupts and 93,000 error interrupts
and rx_missed_errors increases by 34,000. This suggests to me that the NIC is
attempting to raise an interrupt for every packet received.

An Intel 82576 NIC in the same system, running on the igb driver, is performing
OK under 2.6.31 (0 to 0.1% packet loss). For comparison, the same UDP test
generates about 6000 interrupts on the 82576.

dmesg, dmidecode, ethtool, lspci, 'netstat -s', and /proc/interrupts output is
attached.

N.B. I tried removing the 82576 NIC from the system before testing as well; no
change.

- Kelvin

[-- Attachment #2: testhost.dmesg.gz --]
[-- Type: application/x-gzip, Size: 12360 bytes --]

[-- Attachment #3: testhost.dmidecode.gz --]
[-- Type: application/x-gzip, Size: 4371 bytes --]

[-- Attachment #4: testhost.ethtool.gz --]
[-- Type: application/x-gzip, Size: 1075 bytes --]

[-- Attachment #5: testhost.lspci.gz --]
[-- Type: application/x-gzip, Size: 5162 bytes --]

[-- Attachment #6: testhost.netstat.gz --]
[-- Type: application/x-gzip, Size: 679 bytes --]

[-- Attachment #7: testhost.proc-interrupts.gz --]
[-- Type: application/x-gzip, Size: 660 bytes --]

[-- Attachment #8: Type: text/plain, Size: 231 bytes --]

-- 
users mailing list
users@lists.fedoraproject.org
To unsubscribe or change subscription options:
https://admin.fedoraproject.org/mailman/listinfo/users
Guidelines: http://fedoraproject.org/wiki/Communicate/MailingListGuidelines

^ permalink raw reply

* Re: Note: Atheros ethernet part of compat-wireless too
From: Luis R. Rodriguez @ 2010-02-10 18:16 UTC (permalink / raw)
  To: Tony Espy; +Cc: Ubuntu Kernel Team, netdev
In-Reply-To: <4B686056.7090405@canonical.com>

On Tue, Feb 2, 2010 at 9:26 AM, Tony Espy <espy@canonical.com> wrote:
> Luis R. Rodriguez wrote:
>>
>> I wanted to let you know I threw in the Atheros ethernet drivers as
>> part of compat-wireless.
>
> Luis --
>
> Does this include support for the Atheros AR8152 PCI-E fast ethernet
> adapter?  Also, what's the lead time for this to land in a "stable" release?

Figured I'd give you the heads up that the AR8152 is not supported
upstream yet. I just got a tarball of the driver that supports this
and AR8151. Here's our current upstream support status table:

Driver Model-name             vendor:device    Type
atl1c  AR8131                 1969:1063        Gigabit Ethernet
atl1c  AR8132                 1969:1062        Fast Ethernet
atl1   L1                     1969:1048        Gigabit Ethernet
atl2   L2                     1969:2048        Fast Ethernet
atl1e  AR8121/AR8113/AR8114   1969:1026        Gigabit/Fast Ethernet(driver
                                               check one internal register to
                                               see if support gigabit)
atl1c  AR8151(v1.0)           1969:1073        Gigabit Ethernet(not support
                                               in kernel yet)
atl1c  AR8152(v1.1)           1969:2060        Fast Ethernet(not support
                                               in kernel yet)
atl1c  AR8152(v2.0)           1969:2062        Fast Ethernet(not support
                                               in kernel yet)

I just updated the PCI ID database accordingly. The driver patches for
atl1c might be easy, will take a quick look. FWIW the tarball I will
be looking at in a few moments is

http://kernel.org/pub/linux/kernel/people/mcgrof/ethernet/AR81Family-linux-v1.0.1.6.tar.bz2

  Luis

^ permalink raw reply

* Re: phy address in the device tree, vs auto probing
From: Grant Likely @ 2010-02-10 18:14 UTC (permalink / raw)
  To: John Linn; +Cc: devicetree-discuss, netdev, Andy Fleming, Scott Wood
In-Reply-To: <7d35ae9a-9ac0-46e6-8817-15315e0dcc07@SG2EHSMHS004.ehs.local>

On Wed, Feb 10, 2010 at 9:52 AM, John Linn <John.Linn@xilinx.com> wrote:
>> -----Original Message-----
>> From: glikely@secretlab.ca [mailto:glikely@secretlab.ca] On Behalf Of Grant Likely
>> Sent: Wednesday, February 10, 2010 9:44 AM
>> To: John Linn; devicetree-discuss; netdev
>> Subject: Re: phy address in the device tree, vs auto probing
>>
>> (cc'ing devicetree-discuss and netdev mailing lists)
>>
>> On Tue, Feb 9, 2010 at 4:23 PM, John Linn <John.Linn@xilinx.com> wrote:
>> > Hi Grant,
>> >
>> > I notice that the OF driver for the mdio bus is not doing auto probing.
>> >
>> > As we start putting in the phy layer in the emac drivers, the device
>> > trees tend to have the phy address in them, but we're not sure we really
>> > like that.
>> >
>> > We really think that being able to let the kernel find the phy address
>> > is a big benefit, otherwise this is one other piece of info the user has
>> > to know and get right.
>> >
>> > Am I missing something here?
>>
>> No, you're not really missing something, but there is an inherent
>> complexity in what you're wanting to do.  Like i2c, MDIO is one of
>> those busses that is hard to probe reliable.  Some PHYs respond on
>> more than one address, and there is no way to determine which MAC a
>> PHY is wired up to.  Many PHYs can live on a single MDIO bus.  MACs
>> with their own MDIO busses may still get wired to a PHY on a different
>> bus.
>>
>> In the simple case where there is a one:one:one relationship between
>> MAC, MDIO bus and PHY, then it should be okay to probe the PHY,
>> correct?  The question then must be asked; how does the kernel
>> determine that it can use the simple case?  Nobody has yet defined a
>> way to describe that in the device tree; mostly because nobody has
>> needed to yet.
>>
>> So, it is possible to do what you want, but you need a way to
>> *explicitly* ask for that behaviour.  ie, some way to indicate in a
>> MAC node which MDIO bus the phy is on, and that the phy needs to be
>> probed for.  I think this should only be an option when the MDIO bus
>> has only one PHY.  Come up with a proposal and post it to the
>> devicetree-discuss mailing list.
>
> Here's a couple ideas. See what everyone thinks as I'm not stuck on either.
>
> Thanks,
> John
>
> 1. What if we just don't specific a phy address with a reg property which would specify to auto probe it and find the phy as illustrated below?
>
>
>                Ethernet_MAC: ethernet@81000000 {
>                        #address-cells = <1>;
>                        #size-cells = <1>;
>                        phy-handle = <&phy0>;
>                        mdio {
>                                #address-cells = <1>;
>                                #size-cells = <0>;
>                                phy0: phy@7 {
>                                } ;
>                        } ;
>
> 2. Or a special value (-1 or something not 0 - 31) in the phy address that specifies to auto probe as illustrated below.
>                                phy0: phy@7 {
>                                        reg = <-1>;
>                                } ;

I don't like abusing the reg property in this way.  I wonder if a new
empty property would be a better way to indicate this.  Maybe
"phy-probe-address;"?  It would also be important to specify in the
binding that only one phy node is allowed when phy-probe-address is
used.

Also, without a known reg the 'phy@7' name is inaccurate.  Drop the @7.

Scott, Andy: any thoughts?

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply

* Re: [PATCH 0/3 v4] macvtap driver
From: Patrick McHardy @ 2010-02-10 18:10 UTC (permalink / raw)
  To: Sridhar Samudrala; +Cc: Arnd Bergmann, Ed Swierk, netdev
In-Reply-To: <1265825143.30414.81.camel@w-sridhar.beaverton.ibm.com>

Sridhar Samudrala wrote:
> On Wed, 2010-02-10 at 15:48 +0100, Arnd Bergmann wrote:
>> On Monday 08 February 2010, Sridhar Samudrala wrote:
>>> I am also seeing this issue with net-next-2.6.
>>> Basically macvtap_put_user() and macvtap_get_user() call copy_to/from_user
>>> from within a RCU read-side critical section.
>>>
>>> The following patch fixes this issue by releasing the RCU read lock before
>>> calling these routines, but instead hold a reference to q->sk.
>>>
>>> Signed-off-by: Sridhar Samudrala <sri@us.ibm.com>
>> Yes, we need something like this, but we also need to protect the
>> device from going away. The concept right now is to use file_get_queue
>> to protect both the macvtap_queue and the macvlan_dev from going
>> away. The sock_hold will keep the macvtap_queue around, but
>> as far as I can tell, a user could still destroy the macvlan_dev
>> using netlink at the same time, which still breaks.
> 
> may be we should do a dev_hold() in macvtap_set_queue() and dev_put()
> in macvtap_del_queue() so that the underlying device cannot go away as
> long the macvtap fd is open.

You either need some kind of loose binding (f.i. using the ifindex)
or need to handle the case that the device goes away asynchronously
by indicating an error to the socket and unbinding it.

But you can't make the lifetime of the device dependant on the socket.

^ permalink raw reply

* Re: [PATCH 0/3 v4] macvtap driver
From: Sridhar Samudrala @ 2010-02-10 18:05 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: Ed Swierk, netdev
In-Reply-To: <201002101548.47634.arnd@arndb.de>

On Wed, 2010-02-10 at 15:48 +0100, Arnd Bergmann wrote:
> On Monday 08 February 2010, Sridhar Samudrala wrote:
> > I am also seeing this issue with net-next-2.6.
> > Basically macvtap_put_user() and macvtap_get_user() call copy_to/from_user
> > from within a RCU read-side critical section.
> > 
> > The following patch fixes this issue by releasing the RCU read lock before
> > calling these routines, but instead hold a reference to q->sk.
> > 
> > Signed-off-by: Sridhar Samudrala <sri@us.ibm.com>
> 
> Yes, we need something like this, but we also need to protect the
> device from going away. The concept right now is to use file_get_queue
> to protect both the macvtap_queue and the macvlan_dev from going
> away. The sock_hold will keep the macvtap_queue around, but
> as far as I can tell, a user could still destroy the macvlan_dev
> using netlink at the same time, which still breaks.

may be we should do a dev_hold() in macvtap_set_queue() and dev_put()
in macvtap_del_queue() so that the underlying device cannot go away as
long the macvtap fd is open.

Thanks
Sridhar


^ permalink raw reply

* Re: [PATCH 3/4] Add checkpoint support for veth devices
From: Serge E. Hallyn @ 2010-02-10 17:57 UTC (permalink / raw)
  To: Dan Smith; +Cc: containers, netdev
In-Reply-To: <1265750713-15749-4-git-send-email-danms@us.ibm.com>

Quoting Dan Smith (danms@us.ibm.com):
> Adds an ndo_checkpoint() handler for veth devices to checkpoint themselves.
> Writes out the pairing information, addresses, and initiates a checkpoint
> on the peer if the peer won't be reached from another netns.  Throws an
> error of our peer's netns isn't already in the hash (i.e., a tree leak).
> 
> Signed-off-by: Dan Smith <danms@us.ibm.com>

What is here looks good to me, both patches 3 and 4  (but of course i
can't ack as is :)

> ---
>  drivers/net/veth.c |   74 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 74 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/net/veth.c b/drivers/net/veth.c
> index 3a15de5..ad0f561 100644
> --- a/drivers/net/veth.c
> +++ b/drivers/net/veth.c
> @@ -16,6 +16,9 @@
>  #include <net/xfrm.h>
>  #include <linux/veth.h>
> 
> +#include <linux/checkpoint.h>
> +#include <linux/checkpoint_hdr.h>
> +
>  #define DRV_NAME	"veth"
>  #define DRV_VERSION	"1.0"
> 
> @@ -284,6 +287,74 @@ static void veth_dev_free(struct net_device *dev)
>  	free_netdev(dev);
>  }
> 
> +#ifdef CONFIG_CHECKPOINT
> +static int veth_checkpoint(struct ckpt_ctx *ctx, struct net_device *dev)
> +{
> +	struct ckpt_hdr_netdev *h;
> +	struct veth_priv *priv = netdev_priv(dev);
> +	struct net_device *peer = priv->peer;
> +	struct ckpt_netdev_addr *addrs;
> +	int ret;
> +	int n;
> +
> +	if (!peer) {
> +		ckpt_err(ctx, -EINVAL, "veth device has no peer!\n");
> +		return -EINVAL;
> +	}
> +
> +	h = ckpt_netdev_base(ctx, dev, &addrs);
> +	if (IS_ERR(h))
> +		return PTR_ERR(h);
> +
> +	h->type = CKPT_NETDEV_VETH;
> +
> +	ret = h->this_ref = ckpt_obj_lookup_add(ctx, dev, CKPT_OBJ_NETDEV, &n);
> +	if (ret < 0)
> +		goto out;
> +
> +	ret = h->peer_ref = ckpt_obj_lookup_add(ctx, peer, CKPT_OBJ_NETDEV, &n);
> +	if (ret < 0)
> +		goto out;
> +
> +	ret = ckpt_write_obj(ctx, (struct ckpt_hdr *)h);
> +	if (ret < 0)
> +		goto out;
> +
> +	ret = ckpt_write_buffer(ctx, dev->name, IFNAMSIZ);
> +	if (ret < 0)
> +		goto out;
> +
> +	ret = ckpt_write_buffer(ctx, peer->name, IFNAMSIZ);
> +	if (ret < 0)
> +		goto out;
> +
> +	if (h->inet_addrs > 0) {
> +		int len = (sizeof(struct ckpt_netdev_addr) * h->inet_addrs);
> +		ret = ckpt_write_buffer(ctx, addrs, len);
> +		if (ret)
> +			goto out;
> +	}
> +
> +	/* Only checkpoint peer if we're not going to arrive at it
> +	 * via another task's netns.  Fail if the pipe exits
> +	 * our container to a netns not already in the hash
> +	 */
> +	if (ckpt_netdev_in_init_netns(ctx, peer))
> +		ret = checkpoint_obj(ctx, peer, CKPT_OBJ_NETDEV);
> +	else if (!ckpt_obj_lookup(ctx, peer->nd_net, CKPT_OBJ_NET_NS)) {
> +		ret = -EINVAL;
> +		ckpt_err(ctx, ret,
> +			 "Peer %s of %s not in checkpointed namespaces\n",
> +			 peer->name, dev->name);
> +	}
> + out:
> +	ckpt_hdr_put(ctx, h);
> +	kfree(addrs);
> +
> +	return ret;
> +}
> +#endif
> +
>  static const struct net_device_ops veth_netdev_ops = {
>  	.ndo_init            = veth_dev_init,
>  	.ndo_open            = veth_open,
> @@ -292,6 +363,9 @@ static const struct net_device_ops veth_netdev_ops = {
>  	.ndo_change_mtu      = veth_change_mtu,
>  	.ndo_get_stats       = veth_get_stats,
>  	.ndo_set_mac_address = eth_mac_addr,
> +#ifdef CONFIG_CHECKPOINT
> +	.ndo_checkpoint      = veth_checkpoint,
> +#endif
>  };
> 
>  static void veth_setup(struct net_device *dev)
> -- 
> 1.6.2.5
> 
> _______________________________________________
> Containers mailing list
> Containers@lists.linux-foundation.org
> https://lists.linux-foundation.org/mailman/listinfo/containers

^ permalink raw reply

* Re: [PATCH 2/4] C/R: Basic support for network namespaces and devices (v3)
From: Dan Smith @ 2010-02-10 17:55 UTC (permalink / raw)
  To: containers; +Cc: netdev
In-Reply-To: <1265750713-15749-3-git-send-email-danms@us.ibm.com>

Guilt dropped the new checkpoint_dev.c file when I switched to the
newer branch.  Sorry about that.  Updated patch included below.

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

C/R: Basic support for network namespaces and devices (v3)

When checkpointing a task tree with network namespaces, we hook into
do_checkpoint_ns() along with the others.  Any devices in a given namespace
are checkpointed (including their peer, in the case of veth) sequentially.
Each network device stores a list of protocol addresses, as well as other
information, such as hardware address.

This patch supports veth pairs, as well as the loopback adapter.  The
loopback support is there to make sure that any additional addresses and
state (such as up/down) is copied to the loopback adapter that we are
given in the new network namespace.

On restart, we instantiate new network namespaces and veth pairs as
necessary.  Any device we encounter that isn't in a network namespace
that was checkpointed as part of a task is left in the namespace of the
restarting process.  This will be the case for a veth half that exists
in the init netns to provide network access to a container.

Still to do are:

  1. Routes
  2. Netfilter rules
  3. IPv6 addresses
  4. Other virtual device types (e.g. bridges)
  5. Multicast
  6. Device config info (ipv4_devconf)
  7. Additional ipv4 address attributes

Changes in v3:
 - Use dev->checkpoint() for per-device checkpoint operation
 - Use RTNL for veth pair creation on restart
 - Export some of the functions that will be needed by dev->ndo_checkpoint()

Changes in v2:
 - Add CONFIG_CHECKPOINT_NETNS that is dependent on NET, NET_NS, and
   CHECKPOINT.  Conditionally compile the checkpoint_dev code based on it.
 - Updated comment on should_checkpoint_netdev()
 - Updated checkpoint_netdev() to explicitly check for "veth" in name
 - Changed checkpoint_netns() to use BUG() for impossible condition
 - Fixed a bug on restart with all devices in the init netns
 - Lock the dev_base_lock while traversing interface addresses
 - Collect all addresses for an interface before writing out in one
   single pass

Signed-off-by: Dan Smith <danms@us.ibm.com>
Cc: netdev@vger.kernel.org

diff --git a/checkpoint/checkpoint.c b/checkpoint/checkpoint.c
index b4e0021..0b3da7c 100644
--- a/checkpoint/checkpoint.c
+++ b/checkpoint/checkpoint.c
@@ -180,16 +180,23 @@ static int checkpoint_write_header(struct ckpt_ctx *ctx)
 static int checkpoint_container(struct ckpt_ctx *ctx)
 {
 	struct ckpt_hdr_container *h;
+	int new;
 	int ret;
 
 	h = ckpt_hdr_get_type(ctx, sizeof(*h), CKPT_HDR_CONTAINER);
 	if (!h)
 		return -ENOMEM;
-	ret = ckpt_write_obj(ctx, &h->h);
-	ckpt_hdr_put(ctx, h);
 
+	ret = ckpt_obj_lookup_add(ctx, current->nsproxy->net_ns,
+				  CKPT_OBJ_NET_NS, &new);
 	if (ret < 0)
-		return ret;
+		goto out;
+
+	ctx->init_netns_ref = h->init_netns_ref = ret;
+
+	ret = ckpt_write_obj(ctx, &h->h);
+	if (ret < 0)
+		goto out;
 
 	memset(ctx->lsm_name, 0, CHECKPOINT_LSM_NAME_MAX + 1);
 	strlcpy(ctx->lsm_name, security_get_lsm_name(),
@@ -197,9 +204,13 @@ static int checkpoint_container(struct ckpt_ctx *ctx)
 	ret = ckpt_write_buffer(ctx, ctx->lsm_name,
 				CHECKPOINT_LSM_NAME_MAX + 1);
 	if (ret < 0)
-		return ret;
+		goto out;
 
-	return security_checkpoint_header(ctx);
+	ret = security_checkpoint_header(ctx);
+ out:
+	ckpt_hdr_put(ctx, h);
+
+	return ret;
 }
 
 /* write the checkpoint trailer */
diff --git a/checkpoint/objhash.c b/checkpoint/objhash.c
index 4ca7799..729fbe5 100644
--- a/checkpoint/objhash.c
+++ b/checkpoint/objhash.c
@@ -348,6 +348,36 @@ static void lsm_string_drop(void *ptr, int lastref)
 	kref_put(&s->kref, lsm_string_free);
 }
 
+static int netns_grab(void *ptr)
+{
+	struct net *net = ptr;
+
+	get_net(net);
+	return 0;
+}
+
+static void netns_drop(void *ptr, int lastref)
+{
+	struct net *net = ptr;
+
+	put_net(net);
+}
+
+static int netdev_grab(void *ptr)
+{
+	struct net_device *dev = ptr;
+
+	dev_hold(dev);
+	return 0;
+}
+
+static void netdev_drop(void *ptr, int lastref)
+{
+	struct net_device *dev = ptr;
+
+	dev_put(dev);
+}
+
 /* security context strings */
 static int checkpoint_lsm_string(struct ckpt_ctx *ctx, void *ptr);
 static struct ckpt_lsm_string *restore_lsm_string(struct ckpt_ctx *ctx);
@@ -550,6 +580,24 @@ static struct ckpt_obj_ops ckpt_obj_ops[] = {
 		.checkpoint = checkpoint_lsm_string,
 		.restore = restore_lsm_string_wrap,
 	},
+	/* Network Namespace Object */
+	{
+		.obj_name = "NET_NS",
+		.obj_type = CKPT_OBJ_NET_NS,
+		.ref_grab = netns_grab,
+		.ref_drop = netns_drop,
+		.checkpoint = checkpoint_netns,
+		.restore = restore_netns,
+	},
+	/* Network Device Object */
+	{
+		.obj_name = "NET_DEV",
+		.obj_type = CKPT_OBJ_NETDEV,
+		.ref_grab = netdev_grab,
+		.ref_drop = netdev_drop,
+		.checkpoint = checkpoint_netdev,
+		.restore = restore_netdev,
+	},
 };
 
 
diff --git a/checkpoint/restart.c b/checkpoint/restart.c
index fcd07fa..9375e62 100644
--- a/checkpoint/restart.c
+++ b/checkpoint/restart.c
@@ -690,6 +690,10 @@ static int restore_container(struct ckpt_ctx *ctx)
 		return PTR_ERR(h);
 	ckpt_hdr_put(ctx, h);
 
+	/* Store the ref of the init netns so we know to leave its
+	 * devices where they fall */
+	ctx->init_netns_ref = h->init_netns_ref;
+
 	/* read the LSM name and info which follow ("are a part of")
 	 * the ckpt_hdr_container */
 	ret = restore_lsm(ctx);
diff --git a/include/linux/checkpoint.h b/include/linux/checkpoint.h
index 7101d6f..f6e144f 100644
--- a/include/linux/checkpoint.h
+++ b/include/linux/checkpoint.h
@@ -35,6 +35,7 @@
 #include <linux/checkpoint_types.h>
 #include <linux/checkpoint_hdr.h>
 #include <linux/err.h>
+#include <linux/inetdevice.h>
 #include <net/sock.h>
 
 /* sycall helpers */
@@ -119,6 +120,26 @@ extern int ckpt_sock_getnames(struct ckpt_ctx *ctx,
 extern struct sk_buff *sock_restore_skb(struct ckpt_ctx *ctx, struct sock *sk);
 extern void sock_listening_list_free(struct list_head *head);
 
+#ifdef CONFIG_CHECKPOINT_NETNS
+int checkpoint_netns(struct ckpt_ctx *ctx, void *ptr);
+void *restore_netns(struct ckpt_ctx *ctx);
+int checkpoint_netdev(struct ckpt_ctx *ctx, void *ptr);
+void *restore_netdev(struct ckpt_ctx *ctx);
+
+int ckpt_netdev_in_init_netns(struct ckpt_ctx *ctx, struct net_device *dev);
+int ckpt_netdev_inet_addrs(struct in_device *indev,
+			   struct ckpt_netdev_addr *list[]);
+int ckpt_netdev_hwaddr(struct net_device *dev, struct ckpt_hdr_netdev *h);
+struct ckpt_hdr_netdev *ckpt_netdev_base(struct ckpt_ctx *ctx,
+					 struct net_device *dev,
+					 struct ckpt_netdev_addr *addrs[]);
+#else
+# define checkpoint_netns NULL
+# define restore_netns NULL
+# define checkpoint_netdev NULL
+# define restore_netdev NULL
+#endif
+
 /* ckpt kflags */
 #define ckpt_set_ctx_kflag(__ctx, __kflag)  \
 	set_bit(__kflag##_BIT, &(__ctx)->kflags)
diff --git a/include/linux/checkpoint_hdr.h b/include/linux/checkpoint_hdr.h
index e591fd1..d78bd6f 100644
--- a/include/linux/checkpoint_hdr.h
+++ b/include/linux/checkpoint_hdr.h
@@ -181,6 +181,12 @@ enum {
 #define CKPT_HDR_SOCKET_UNIX CKPT_HDR_SOCKET_UNIX
 	CKPT_HDR_SOCKET_INET,
 #define CKPT_HDR_SOCKET_INET CKPT_HDR_SOCKET_INET
+	CKPT_HDR_NET_NS,
+#define CKPT_HDR_NET_NS CKPT_HDR_NET_NS
+	CKPT_HDR_NETDEV,
+#define CKPT_HDR_NETDEV CKPT_HDR_NETDEV
+	CKPT_HDR_NETDEV_ADDR,
+#define CKPT_HDR_NETDEV_ADDR CKPT_HDR_NETDEV_ADDR
 
 	CKPT_HDR_TAIL = 9001,
 #define CKPT_HDR_TAIL CKPT_HDR_TAIL
@@ -253,6 +259,10 @@ enum obj_type {
 #define CKPT_OBJ_SECURITY_PTR CKPT_OBJ_SECURITY_PTR
 	CKPT_OBJ_SECURITY,
 #define CKPT_OBJ_SECURITY CKPT_OBJ_SECURITY
+	CKPT_OBJ_NET_NS,
+#define CKPT_OBJ_NET_NS CKPT_OBJ_NET_NS
+	CKPT_OBJ_NETDEV,
+#define CKPT_OBJ_NETDEV CKPT_OBJ_NETDEV
 	CKPT_OBJ_MAX
 #define CKPT_OBJ_MAX CKPT_OBJ_MAX
 };
@@ -313,6 +323,7 @@ struct ckpt_hdr_tail {
 /* container configuration section header */
 struct ckpt_hdr_container {
 	struct ckpt_hdr h;
+	__s32 init_netns_ref;
 	/*
 	 * the header is followed by the string:
 	 *   char lsm_name[SECURITY_NAME_MAX + 1]
@@ -434,6 +445,7 @@ struct ckpt_hdr_ns {
 	struct ckpt_hdr h;
 	__s32 uts_objref;
 	__s32 ipc_objref;
+	__s32 net_objref;
 } __attribute__((aligned(8)));
 
 /* cannot include <linux/tty.h> from userspace, so define: */
@@ -758,6 +770,43 @@ struct ckpt_hdr_file_socket {
 	__s32 sock_objref;
 } __attribute__((aligned(8)));
 
+struct ckpt_hdr_netns {
+	struct ckpt_hdr h;
+	__s32 this_ref;
+} __attribute__((aligned(8)));
+
+enum ckpt_netdev_types {
+	CKPT_NETDEV_LO,
+	CKPT_NETDEV_VETH,
+};
+
+struct ckpt_hdr_netdev {
+	struct ckpt_hdr h;
+ 	__s32 netns_ref;
+	__s32 this_ref;     /* veth only */
+	__s32 peer_ref;     /* veth only */
+	__u32 inet_addrs;
+	__u16 type;
+	__u16 flags;
+	__u8 hwaddr[6];
+} __attribute__((aligned(8)));
+
+enum ckpt_netdev_addr_types {
+	CKPT_NETDEV_ADDR_IPV4,
+};
+
+struct ckpt_netdev_addr {
+	__u16 type;
+	union {
+		struct {
+			__u32 inet4_local;
+			__u32 inet4_address;
+			__u32 inet4_mask;
+			__u32 inet4_broadcast;
+		};
+	};
+} __attribute__((aligned(8)));
+
 struct ckpt_hdr_eventpoll_items {
 	struct ckpt_hdr h;
 	__s32  epfile_objref;
diff --git a/include/linux/checkpoint_types.h b/include/linux/checkpoint_types.h
index 51efd5a..e646ec6 100644
--- a/include/linux/checkpoint_types.h
+++ b/include/linux/checkpoint_types.h
@@ -86,6 +86,7 @@ struct ckpt_ctx {
 	wait_queue_head_t ghostq;	/* waitqueue for ghost tasks */
 	struct cred *realcred, *ecred;	/* tmp storage for cred at restart */
 	struct list_head listen_sockets;/* listening parent sockets */
+	int init_netns_ref;             /* Objref of root net namespace */
 
 	struct ckpt_stats stats;	/* statistics */
 
diff --git a/kernel/nsproxy.c b/kernel/nsproxy.c
index b0e71f2..78f5615 100644
--- a/kernel/nsproxy.c
+++ b/kernel/nsproxy.c
@@ -248,6 +248,11 @@ int ckpt_collect_ns(struct ckpt_ctx *ctx, struct task_struct *t)
 	ret = ckpt_obj_collect(ctx, nsproxy->uts_ns, CKPT_OBJ_UTS_NS);
 	if (ret < 0)
 		goto out;
+#ifdef CONFIG_CHECKPOINT_NETNS
+	ret = ckpt_obj_collect(ctx, nsproxy->net_ns, CKPT_OBJ_NET_NS);
+	if (ret < 0)
+		goto out;
+#endif
 	ret = ckpt_obj_collect(ctx, nsproxy->ipc_ns, CKPT_OBJ_IPC_NS);
 	if (ret < 0)
 		goto out;
@@ -288,6 +293,12 @@ static int do_checkpoint_ns(struct ckpt_ctx *ctx, struct nsproxy *nsproxy)
 	if (ret < 0)
 		goto out;
 	h->ipc_objref = ret;
+#ifdef CONFIG_CHECKPOINT_NETNS
+	ret = checkpoint_obj(ctx, nsproxy->net_ns, CKPT_OBJ_NET_NS);
+	if (ret < 0)
+		goto out;
+	h->net_objref = ret;
+#endif
 
 	/* FIXME: for now, only marked visited to pacify leaks */
 	ret = ckpt_obj_visit(ctx, nsproxy->mnt_ns, CKPT_OBJ_MNT_NS);
@@ -306,6 +317,34 @@ int checkpoint_ns(struct ckpt_ctx *ctx, void *ptr)
 	return do_checkpoint_ns(ctx, (struct nsproxy *) ptr);
 }
 
+static int do_restore_netns(struct ckpt_ctx *ctx,
+			    struct ckpt_hdr_ns *h,
+			    struct nsproxy *nsproxy)
+{
+#ifdef CONFIG_CHECKPOINT_NETNS
+	struct net *net_ns;
+
+	if (h->net_objref < 0)
+		return -EINVAL;
+	else if (h->net_objref == 0)
+		return 0;
+
+	net_ns = ckpt_obj_fetch(ctx, h->net_objref, CKPT_OBJ_NET_NS);
+	if (IS_ERR(net_ns))
+		return PTR_ERR(net_ns);
+
+	get_net(net_ns);
+	nsproxy->net_ns = net_ns;
+#else
+	if (h->net_objref > 0)
+		return -EINVAL;
+	get_net(current->nsproxy->net_ns);
+	nsproxy->net_ns = current->nsproxy->net_ns;
+#endif
+
+	return 0;
+}
+
 static struct nsproxy *do_restore_ns(struct ckpt_ctx *ctx)
 {
 	struct ckpt_hdr_ns *h;
@@ -349,8 +388,6 @@ static struct nsproxy *do_restore_ns(struct ckpt_ctx *ctx)
 	nsproxy->pid_ns = current->nsproxy->pid_ns;
 	get_mnt_ns(current->nsproxy->mnt_ns);
 	nsproxy->mnt_ns = current->nsproxy->mnt_ns;
-	get_net(current->nsproxy->net_ns);
-	nsproxy->net_ns = current->nsproxy->net_ns;
 #else
 	nsproxy = current->nsproxy;
 	get_nsproxy(nsproxy);
@@ -359,6 +396,10 @@ static struct nsproxy *do_restore_ns(struct ckpt_ctx *ctx)
 	BUG_ON(nsproxy->ipc_ns != ipc_ns);
 #endif
 
+	ret = do_restore_netns(ctx, h, nsproxy);
+	if (ret < 0)
+		goto out;
+
 	/* TODO: add more namespaces here */
 	ret = 0;
  out:
diff --git a/net/Kconfig b/net/Kconfig
index 041c35e..64dd3cd 100644
--- a/net/Kconfig
+++ b/net/Kconfig
@@ -276,4 +276,8 @@ source "net/wimax/Kconfig"
 source "net/rfkill/Kconfig"
 source "net/9p/Kconfig"
 
+config CHECKPOINT_NETNS
+       bool
+       default y if NET && NET_NS && CHECKPOINT
+
 endif   # if NET
diff --git a/net/Makefile b/net/Makefile
index 74b038f..570ee98 100644
--- a/net/Makefile
+++ b/net/Makefile
@@ -67,3 +67,4 @@ endif
 obj-$(CONFIG_WIMAX)		+= wimax/
 
 obj-$(CONFIG_CHECKPOINT)	+= checkpoint.o
+obj-$(CONFIG_CHECKPOINT_NETNS)	+= checkpoint_dev.o
diff --git a/net/checkpoint_dev.c b/net/checkpoint_dev.c
new file mode 100644
index 0000000..0dddd15
--- /dev/null
+++ b/net/checkpoint_dev.c
@@ -0,0 +1,673 @@
+/*
+ *  Copyright 2010 IBM Corporation
+ *
+ *  Author(s): Dan Smith <danms@us.ibm.com>
+ *
+ *  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, version 2 of the
+ *  License.
+ */
+
+#include <linux/sched.h>
+#include <linux/if.h>
+#include <linux/if_arp.h>
+#include <linux/inetdevice.h>
+#include <linux/veth.h>
+#include <linux/checkpoint.h>
+#include <linux/checkpoint_hdr.h>
+#include <linux/deferqueue.h>
+
+#include <net/net_namespace.h>
+#include <net/sch_generic.h>
+
+struct dq_netdev {
+	struct net_device *dev;
+	struct ckpt_ctx *ctx;
+};
+
+static int __kern_devinet_ioctl(struct net *net, unsigned int cmd, void *arg)
+{
+	mm_segment_t fs;
+	int ret;
+
+	fs = get_fs();
+	set_fs(KERNEL_DS);
+	ret = devinet_ioctl(net, cmd, arg);
+	set_fs(fs);
+
+	return ret;
+}
+
+static int __kern_dev_ioctl(struct net *net, unsigned int cmd, void *arg)
+{
+	mm_segment_t fs;
+	int ret;
+
+	fs = get_fs();
+	set_fs(KERNEL_DS);
+	ret = dev_ioctl(net, cmd, arg);
+	set_fs(fs);
+
+	return ret;
+}
+
+static struct socket *rtnl_open(void)
+{
+	struct socket *sock;
+	int ret;
+
+	ret = sock_create(AF_NETLINK, SOCK_DGRAM, NETLINK_ROUTE, &sock);
+	if (ret < 0)
+		return ERR_PTR(ret);
+
+	return sock;
+}
+
+static int rtnl_close(struct socket *rtnl)
+{
+	return kernel_sock_shutdown(rtnl, SHUT_RDWR);
+}
+
+static struct nlmsghdr *rtnl_get_response(struct socket *rtnl,
+					  struct sk_buff **skb)
+{
+	int ret;
+	long timeo = MAX_SCHEDULE_TIMEOUT;
+	struct nlmsghdr *nlh;
+
+	ret = sk_wait_data(rtnl->sk, &timeo);
+	if (!ret)
+		return ERR_PTR(-EPIPE);
+
+	*skb = skb_dequeue(&rtnl->sk->sk_receive_queue);
+	if (!*skb)
+		return ERR_PTR(-EPIPE);
+
+	ret = -EINVAL;
+	nlh = nlmsg_hdr(*skb);
+	if (!nlh)
+		goto err;
+
+	if (nlh->nlmsg_type == NLMSG_ERROR) {
+		struct nlmsgerr *errmsg = nlmsg_data(nlh);
+		ret = errmsg->error;
+		goto err;
+	}
+
+	return nlh;
+ err:
+	kfree_skb(*skb);
+	*skb = NULL;
+
+	return ERR_PTR(ret);
+}
+
+int ckpt_netdev_in_init_netns(struct ckpt_ctx *ctx, struct net_device *dev)
+{
+	struct net *net = dev->nd_net;
+	int ref;
+
+	ref = ckpt_obj_lookup(ctx, net, CKPT_OBJ_NET_NS);
+	return ref == ctx->init_netns_ref;
+}
+
+int ckpt_netdev_hwaddr(struct net_device *dev, struct ckpt_hdr_netdev *h)
+{
+	struct net *net = dev->nd_net;
+	struct ifreq req;
+	int ret;
+
+	memcpy(req.ifr_name, dev->name, IFNAMSIZ);
+	ret = __kern_dev_ioctl(net, SIOCGIFFLAGS, &req);
+	h->flags = req.ifr_flags;
+	if (ret < 0)
+		return ret;
+
+	ret = __kern_dev_ioctl(net, SIOCGIFHWADDR, &req);
+	if (ret < 0)
+		return ret;
+
+	memcpy(h->hwaddr, req.ifr_hwaddr.sa_data, sizeof(h->hwaddr));
+
+	return 0;
+}
+
+int ckpt_netdev_inet_addrs(struct in_device *indev,
+			   struct ckpt_netdev_addr *_abuf[])
+{
+	struct ckpt_netdev_addr *abuf = NULL;
+	struct in_ifaddr *addr = indev->ifa_list;
+	int pages = 0;
+	int addrs = 0;
+	int max;
+
+	read_lock(&dev_base_lock);
+ retry:
+	if (++pages > 4) {
+		addrs = -ENOMEM;
+		goto out;
+	}
+
+	*_abuf = krealloc(abuf, PAGE_SIZE * pages, GFP_KERNEL);
+	if (*_abuf == NULL) {
+		addrs = -ENOMEM;
+		goto out;
+	}
+	abuf = *_abuf;
+
+	max = (pages * PAGE_SIZE) / sizeof(*abuf);
+	while (addr) {
+		abuf[addrs].type = CKPT_NETDEV_ADDR_IPV4; /* Only IPv4 now */
+		abuf[addrs].inet4_local = addr->ifa_local;
+		abuf[addrs].inet4_address = addr->ifa_address;
+		abuf[addrs].inet4_mask = addr->ifa_mask;
+		abuf[addrs].inet4_broadcast = addr->ifa_broadcast;
+
+		addr = addr->ifa_next;
+		if (++addrs >= max)
+			goto retry;
+	}
+
+ out:
+	read_unlock(&dev_base_lock);
+
+	if (addrs < 0) {
+		kfree(abuf);
+		*_abuf = NULL;
+	}
+
+	return addrs;
+}
+
+struct ckpt_hdr_netdev *ckpt_netdev_base(struct ckpt_ctx *ctx,
+					 struct net_device *dev,
+					 struct ckpt_netdev_addr *addrs[])
+{
+	struct ckpt_hdr_netdev *h;
+	int ret;
+
+	h = ckpt_hdr_get_type(ctx, sizeof(*h), CKPT_HDR_NETDEV);
+	if (!h)
+		return ERR_PTR(-ENOMEM);
+
+	ret = ckpt_netdev_hwaddr(dev, h);
+	if (ret < 0)
+		goto out;
+
+	*addrs = NULL;
+	ret = h->inet_addrs = ckpt_netdev_inet_addrs(dev->ip_ptr, addrs);
+	if (ret < 0)
+		goto out;
+
+	ret = h->netns_ref = checkpoint_obj(ctx, dev->nd_net, CKPT_OBJ_NET_NS);
+ out:
+	if (ret < 0) {
+		ckpt_hdr_put(ctx, h);
+		h = ERR_PTR(ret);
+		if (*addrs)
+			kfree(*addrs);
+	}
+
+	return h;
+}
+
+int checkpoint_netdev(struct ckpt_ctx *ctx, void *ptr)
+{
+	struct net_device *dev = (struct net_device *)ptr;
+
+	if (!dev->netdev_ops->ndo_checkpoint)
+		return -EINVAL;
+
+	ckpt_debug("checkpointing netdev %s\n", dev->name);
+
+	return dev->netdev_ops->ndo_checkpoint(ctx, dev);
+}
+
+int checkpoint_netns(struct ckpt_ctx *ctx, void *ptr)
+{
+	struct net *net = ptr;
+	struct net_device *dev;
+	struct ckpt_hdr_netns *h;
+	int ret;
+
+	h = ckpt_hdr_get_type(ctx, sizeof(*h), CKPT_HDR_NET_NS);
+	if (!h)
+		return -ENOMEM;
+
+	h->this_ref = ckpt_obj_lookup(ctx, net, CKPT_OBJ_NET_NS);
+	BUG_ON(h->this_ref == 0);
+
+	ret = ckpt_write_obj(ctx, (struct ckpt_hdr *) h);
+	if (ret < 0)
+		goto out;
+
+	for_each_netdev(net, dev) {
+		if (!dev->netdev_ops->ndo_checkpoint)
+			continue;
+		ret = checkpoint_obj(ctx, dev, CKPT_OBJ_NETDEV);
+		if (ret < 0)
+			break;
+	}
+ out:
+	ckpt_hdr_put(ctx, h);
+
+	return ret;
+}
+
+static int restore_in_addrs(struct ckpt_ctx *ctx,
+			    __u32 naddrs,
+			    struct net *net,
+			    struct net_device *dev)
+{
+	__u32 i;
+	int ret = 0;
+	int len = naddrs * sizeof(struct ckpt_netdev_addr);
+	struct ckpt_netdev_addr *addrs = NULL;
+
+	addrs = kmalloc(len, GFP_KERNEL);
+	if (!addrs)
+		return -ENOMEM;
+
+	ret = _ckpt_read_buffer(ctx, addrs, len);
+	if (ret < 0)
+		goto out;
+
+	for (i = 0; i < naddrs; i++) {
+		struct ckpt_netdev_addr *addr = &addrs[i];
+		struct ifreq req;
+		struct sockaddr_in *inaddr;
+
+		if (addr->type != CKPT_NETDEV_ADDR_IPV4) {
+			ret = -EINVAL;
+			ckpt_err(ctx, ret, "Unsupported netdev addr type %i\n",
+				 addr->type);
+			break;
+		}
+
+		ckpt_debug("restoring %s: %x/%x/%x\n", dev->name,
+			   addr->inet4_address,
+			   addr->inet4_mask,
+			   addr->inet4_broadcast);
+
+		memcpy(req.ifr_name, dev->name, IFNAMSIZ);
+
+		inaddr = (struct sockaddr_in *)&req.ifr_addr;
+		inaddr->sin_addr.s_addr = addr->inet4_address;
+		inaddr->sin_family = AF_INET;
+		ret = __kern_devinet_ioctl(net, SIOCSIFADDR, &req);
+		if (ret < 0) {
+			ckpt_err(ctx, ret, "Failed to set address\n");
+			break;
+		}
+
+		inaddr = (struct sockaddr_in *)&req.ifr_addr;
+		inaddr->sin_addr.s_addr = addr->inet4_mask;
+		inaddr->sin_family = AF_INET;
+		ret = __kern_devinet_ioctl(net, SIOCSIFNETMASK, &req);
+		if (ret < 0) {
+			ckpt_err(ctx, ret, "Failed to set netmask\n");
+			break;
+		}
+
+		inaddr = (struct sockaddr_in *)&req.ifr_addr;
+		inaddr->sin_addr.s_addr = addr->inet4_broadcast;
+		inaddr->sin_family = AF_INET;
+		ret = __kern_devinet_ioctl(net, SIOCSIFBRDADDR, &req);
+		if (ret < 0) {
+			ckpt_err(ctx, ret, "Failed to set broadcast\n");
+			break;
+		}
+	}
+
+ out:
+	kfree(addrs);
+
+	return ret;
+}
+
+static int veth_peer_data(struct sk_buff *skb, char *peer_name)
+{
+	struct nlattr *linkdata;
+	struct ifinfomsg ifm;
+
+	linkdata = nla_nest_start(skb, IFLA_INFO_DATA);
+	if (!linkdata)
+		return -ENOMEM;
+
+	nla_put(skb, VETH_INFO_PEER, sizeof(ifm), &ifm);
+	nla_put_string(skb, IFLA_IFNAME, peer_name);
+
+	nla_nest_end(skb, linkdata);
+
+	return 0;
+}
+
+static struct sk_buff *new_link_message(char *this_name, char *peer_name)
+{
+	int ret = -ENOMEM;
+	int flags = NLM_F_REQUEST | NLM_F_CREATE | NLM_F_ACK;
+	struct nlmsghdr *nlh;
+	struct sk_buff *skb;
+	struct ifinfomsg *ifm;
+	struct nlattr *linkinfo;
+
+	skb = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
+	if (!skb)
+		goto out;
+
+	nlh = nlmsg_put(skb, 0, 0, RTM_NEWLINK, sizeof(*ifm), flags);
+	if (!nlh)
+		goto out;
+
+	ifm = nlmsg_data(nlh);
+	memset(ifm, 0, sizeof(*ifm));
+
+	ret = nla_put_string(skb, IFLA_IFNAME, this_name);
+	if (ret)
+		goto out;
+
+	ret = -ENOMEM;
+
+	linkinfo = nla_nest_start(skb, IFLA_LINKINFO);
+	if (!linkinfo)
+		goto out;
+
+	if (nla_put_string(skb, IFLA_INFO_KIND, "veth") < 0)
+		goto out;
+
+	ret = veth_peer_data(skb, peer_name);
+	if (ret < 0)
+		goto out;
+
+	nla_nest_end(skb, linkinfo);
+	nlmsg_end(skb, nlh);
+
+ out:
+	if (ret < 0) {
+		kfree(skb);
+		skb = ERR_PTR(ret);
+	}
+
+	return skb;
+}
+
+static struct net_device *new_veth_pair(char *this_name, char *peer_name)
+{
+	int ret = -ENOMEM;
+	struct socket *rtnl;
+	struct sk_buff *skb = NULL;
+	struct nlmsghdr *nlh;
+	struct msghdr msg;
+	struct kvec kvec;
+
+	skb = new_link_message(this_name, peer_name);
+	if (IS_ERR(skb)) {
+		ret = PTR_ERR(skb);
+		ckpt_debug("failed to create new link message: %i\n", ret);
+		skb = NULL;
+		goto out;
+	}
+
+	memset(&msg, 0, sizeof(msg));
+	kvec.iov_len = skb->len;
+	kvec.iov_base = skb->head;
+
+	rtnl = rtnl_open();
+	if (IS_ERR(rtnl)) {
+		ret = PTR_ERR(rtnl);
+		ckpt_debug("Unable to open rtnetlink socket: %i\n", ret);
+		goto out_noclose;
+	}
+
+	ret = kernel_sendmsg(rtnl, &msg, &kvec, 1, kvec.iov_len);
+	if (ret < 0)
+		goto out;
+	else if (ret != skb->len) {
+		ret = -EIO;
+		goto out;
+	}
+
+	/* Free the send skb to make room for the receive skb */
+	kfree(skb);
+
+	nlh = rtnl_get_response(rtnl, &skb);
+	if (IS_ERR(nlh)) {
+		ret = PTR_ERR(nlh);
+		ckpt_debug("RTNETLINK said: %i\n", ret);
+	}
+ out:
+	rtnl_close(rtnl);
+ out_noclose:
+	kfree(skb);
+
+	if (ret < 0)
+		return ERR_PTR(ret);
+	else
+		return dev_get_by_name(current->nsproxy->net_ns, this_name);
+}
+
+static int netdev_noop(void *data)
+{
+	return 0;
+}
+
+static int netdev_cleanup(void *data)
+{
+	struct dq_netdev *dq = data;
+
+	dev_put(dq->dev);
+
+	if (dq->ctx->errno) {
+		ckpt_debug("Unregistering netdev %s\n", dq->dev->name);
+		unregister_netdev(dq->dev);
+	}
+
+	return 0;
+}
+
+static struct net_device *restore_veth(struct ckpt_ctx *ctx,
+				       struct ckpt_hdr_netdev *h,
+				       struct net *net)
+{
+	int ret;
+	char this_name[IFNAMSIZ];
+	char peer_name[IFNAMSIZ];
+	struct net_device *dev;
+	struct net_device *peer;
+	int didreg = 0;
+
+	struct dq_netdev dq;
+
+	dq.ctx = ctx;
+
+	ret = _ckpt_read_buffer(ctx, this_name, IFNAMSIZ);
+	if (ret < 0)
+		return ERR_PTR(ret);
+
+	ret = _ckpt_read_buffer(ctx, peer_name, IFNAMSIZ);
+	if (ret < 0)
+		return ERR_PTR(ret);
+
+	ckpt_debug("restored veth netdev %s:%s\n", this_name, peer_name);
+
+	peer = ckpt_obj_try_fetch(ctx, h->peer_ref, CKPT_OBJ_NETDEV);
+	if (IS_ERR(peer)) {
+		/* We're first: allocate the veth pair */
+		didreg = 1;
+		dev = new_veth_pair(this_name, peer_name);
+		if (IS_ERR(dev))
+			return dev;
+
+		peer = dev_get_by_name(current->nsproxy->net_ns, peer_name);
+		if (!peer) {
+			ret = -EINVAL;
+			goto err_dev;
+		}
+
+		dq.dev = peer;
+		ret = deferqueue_add(ctx->deferqueue, &dq, sizeof(dq),
+				     netdev_noop, netdev_cleanup);
+		if (ret)
+			goto err_peer;
+
+		ret = ckpt_obj_insert(ctx, peer, h->peer_ref, CKPT_OBJ_NETDEV);
+		if (ret < 0)
+			/* Can't recall peer dq, so let it cleanup peer */
+			goto err_dev;
+
+		dq.dev = dev;
+		ret = deferqueue_add(ctx->deferqueue, &dq, sizeof(dq),
+				     netdev_noop, netdev_cleanup);
+		if (ret)
+			/* Can't recall peer dq, so let it cleanup peer */
+			goto err_dev;
+
+	} else {
+		/* We're second: get our dev from the hash */
+		dev = ckpt_obj_fetch(ctx, h->this_ref, CKPT_OBJ_NETDEV);
+		if (IS_ERR(dev))
+			return dev;
+	}
+
+	/* Move to our new netns */
+	rtnl_lock();
+	ret = dev_change_net_namespace(dev, net, dev->name);
+	rtnl_unlock();
+
+	if (ret)
+		dev = ERR_PTR(ret);
+
+	return dev;
+
+ err_peer:
+	dev_put(peer);
+	unregister_netdev(peer);
+ err_dev:
+	dev_put(dev);
+	unregister_netdev(dev);
+
+	return ERR_PTR(ret);
+}
+
+static struct net_device *restore_lo(struct ckpt_ctx *ctx,
+				     struct ckpt_hdr_netdev *h,
+				     struct net *net)
+{
+	struct net_device *dev;
+	char name[IFNAMSIZ+1];
+	int ret;
+
+	dev = dev_get_by_name(net, "lo");
+	if (!dev)
+		return ERR_PTR(-EINVAL);
+
+	ret = _ckpt_read_buffer(ctx, name, IFNAMSIZ);
+	if (ret < 0)
+		goto err;
+
+	if (strncmp(dev->name, name, IFNAMSIZ) != 0) {
+		ret = dev_change_name(dev, name);
+		if (ret < 0)
+			goto err;
+	}
+
+	return dev;
+ err:
+	dev_put(dev);
+
+	return ERR_PTR(ret);
+}
+
+void *restore_netdev(struct ckpt_ctx *ctx)
+{
+	struct ckpt_hdr_netdev *h;
+	struct net_device *dev = NULL;
+	struct ifreq req;
+	struct net *net;
+	int ret;
+
+	h = ckpt_read_obj_type(ctx, sizeof(*h), CKPT_HDR_NETDEV);
+	if (IS_ERR(h)) {
+		ckpt_err(ctx, PTR_ERR(h), "failed to read netdev\n");
+		return h;
+	}
+
+	if (h->netns_ref != ctx->init_netns_ref) {
+		net = ckpt_obj_try_fetch(ctx, h->netns_ref, CKPT_OBJ_NET_NS);
+		if (IS_ERR(net)) {
+			ckpt_debug("failed to get net for %i\n", h->netns_ref);
+			net = current->nsproxy->net_ns;
+			ret = PTR_ERR(net);
+			goto out;
+		}
+	} else
+		net = current->nsproxy->net_ns;
+
+	if (h->type == CKPT_NETDEV_VETH)
+		dev = restore_veth(ctx, h, net);
+	else if (h->type == CKPT_NETDEV_LO)
+		dev = restore_lo(ctx, h, net);
+	else
+		dev = ERR_PTR(-EINVAL);
+
+	if (IS_ERR(dev)) {
+		ret = PTR_ERR(dev);
+		ckpt_err(ctx, ret, "Netdev type %i not supported\n", h->type);
+		goto out;
+	}
+
+	memcpy(req.ifr_name, dev->name, IFNAMSIZ);
+
+	if (h->type != CKPT_NETDEV_LO) {
+		/* Restore MAC address */
+		memcpy(req.ifr_hwaddr.sa_data, h->hwaddr, sizeof(h->hwaddr));
+		req.ifr_hwaddr.sa_family = ARPHRD_ETHER;
+		ret = __kern_dev_ioctl(net, SIOCSIFHWADDR, &req);
+		if (ret < 0)
+			goto out;
+	}
+
+	/* Restore flags (which will likely bring the interface up) */
+	req.ifr_flags = h->flags;
+	ret = __kern_dev_ioctl(net, SIOCSIFFLAGS, &req);
+	if (ret < 0)
+		goto out;
+
+	if (h->inet_addrs > 0)
+		ret = restore_in_addrs(ctx, h->inet_addrs, net, dev);
+ out:
+	if (ret) {
+		ckpt_err(ctx, ret, "Failed to restore netdevice\n");
+		if ((h->type == CKPT_NETDEV_VETH) && !IS_ERR(dev)) {
+			dev_put(dev);
+		}
+		dev = ERR_PTR(ret);
+	}
+	ckpt_hdr_put(ctx, h);
+
+	return dev;
+}
+
+void *restore_netns(struct ckpt_ctx *ctx)
+{
+	struct ckpt_hdr_netns *h;
+	struct net *net;
+
+	h = ckpt_read_obj_type(ctx, sizeof(*h), CKPT_HDR_NET_NS);
+	if (IS_ERR(h)) {
+		ckpt_err(ctx, PTR_ERR(h), "failed to read netns\n");
+		return h;
+	}
+
+	if (h->this_ref != ctx->init_netns_ref) {
+		net = copy_net_ns(CLONE_NEWNET, current->nsproxy->net_ns);
+		if (IS_ERR(net))
+			goto out;
+	} else
+		net = current->nsproxy->net_ns;
+ out:
+	ckpt_hdr_put(ctx, h);
+
+	return net;
+}

^ permalink raw reply related

* Attn:Account Owner
From: Customer Service @ 2010-02-10 17:43 UTC (permalink / raw)



Attn:Account Owner,

Account Upgrade/Maintenance Webmail  Accounts

We regret to announce to you that we will be making
some vital maintenance on our web mail account. During 
this
process you might have login problems in signing into your
account,but to prevent this you have to confirm your
account immediately after you receive this
notification.

To confirm and to keep your webmail account active
during and after this process, please reply to this
message with the below account information's.Failure to do
this might cause a permanent deactivation of your webmail
account from our database to enable us create more spaces
for digital webmai account.

Confirm Your Account webmail Account
by clinck this link

http://grasup.vistahosting.cn/feedback/feedback.html

Your account shall remain active after you have
successfully confirmed your account details.

We thank you for your prompt attention to this matter.
Please understand that this is a security measure intended
to help protect your account a we apologize for any
inconvenience.

Its Technical Support
Customer Service				

Copyright 2010. All rights Reserved.

Judith Cha Rautenbach

^ permalink raw reply

* Re: [PATCH 3/3] via-velocity: Fix races on shared interrupts
From: Laurent Chavey @ 2010-02-10 17:41 UTC (permalink / raw)
  To: Simon Kagstrom; +Cc: netdev, davem, davej, ben
In-Reply-To: <20100205165545.28832c53@marrow.netinsight.se>

On Fri, Feb 5, 2010 at 7:55 AM, Simon Kagstrom
<simon.kagstrom@netinsight.net> wrote:
> This patch fixes two potential races in the velocity driver:
>
> * Move the ACK and error handler to the interrupt handler. This fixes a
>  potential race with shared interrupts when the other device interrupts
>  before the NAPI poll handler has finished. As the velocity driver hasn't
>  acked it's own interrupt, it will then steal the interrupt from the
>  other device.
>
> * Use spin_trylock in the interrupt handler. To avoid having the
>  interrupt off for long periods of time, velocity_poll uses non-irqsave
>  spinlocks. In the current code, the interrupt handler will deadlock if
>  e.g., the NAPI poll handler is executing when an interrupt (for another
>  device) comes in since it tries to take the already held lock.
>
> Signed-off-by: Simon Kagstrom <simon.kagstrom@netinsight.net>
> Signed-off-by: Anders Grafstrom <anders.grafstrom@netinsight.net>
> ---
>  drivers/net/via-velocity.c |   26 +++++++++++++++++---------
>  1 files changed, 17 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/via-velocity.c b/drivers/net/via-velocity.c
> index 5e213f7..6882e7c 100644
> --- a/drivers/net/via-velocity.c
> +++ b/drivers/net/via-velocity.c
> @@ -2148,16 +2148,8 @@ static int velocity_poll(struct napi_struct *napi, int budget)
>        struct velocity_info *vptr = container_of(napi,
>                        struct velocity_info, napi);
>        unsigned int rx_done;
> -       u32 isr_status;
>
>        spin_lock(&vptr->lock);
> -       isr_status = mac_read_isr(vptr->mac_regs);
> -
> -       /* Ack the interrupt */
> -       mac_write_isr(vptr->mac_regs, isr_status);
> -       if (isr_status & (~(ISR_PRXI | ISR_PPRXI | ISR_PTXI | ISR_PPTXI)))
> -               velocity_error(vptr, isr_status);
> -
>        /*
>         * Do rx and tx twice for performance (taken from the VIA
>         * out-of-tree driver).
> @@ -2194,7 +2186,16 @@ static irqreturn_t velocity_intr(int irq, void *dev_instance)
>        struct velocity_info *vptr = netdev_priv(dev);
>        u32 isr_status;
>
> -       spin_lock(&vptr->lock);
> +       /* Check if the lock is taken, and if so ignore the interrupt. This
> +        * can happen with shared interrupts, where the other device can
> +        * interrupt during velocity_poll (where the lock is held).
> +        *
> +        * With spinlock debugging active on a uniprocessor, this will give
> +        * a warning which can safely be ignored.
> +        */
> +       if (!spin_trylock(&vptr->lock))
> +               return IRQ_NONE;

does the thread handling the interrupts check that an new
interrupts was received while it was servicing a previous one ?
wondering if there is a potential for an event that generates the interrupt
to be missed.


> +
>        isr_status = mac_read_isr(vptr->mac_regs);
>
>        /* Not us ? */
> @@ -2203,10 +2204,17 @@ static irqreturn_t velocity_intr(int irq, void *dev_instance)
>                return IRQ_NONE;
>        }
>
> +       /* Ack the interrupt */
> +       mac_write_isr(vptr->mac_regs, isr_status);
> +
>        if (likely(napi_schedule_prep(&vptr->napi))) {
>                mac_disable_int(vptr->mac_regs);
>                __napi_schedule(&vptr->napi);
>        }
> +
> +       if (isr_status & (~(ISR_PRXI | ISR_PPRXI | ISR_PTXI | ISR_PPTXI)))
> +               velocity_error(vptr, isr_status);
> +
>        spin_unlock(&vptr->lock);
>
>        return IRQ_HANDLED;
> --
> 1.6.0.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

^ permalink raw reply

* Re: [PATCH 2/4] C/R: Basic support for network namespaces and devices (v3)
From: Dan Smith @ 2010-02-10 17:38 UTC (permalink / raw)
  To: Serge E. Hallyn; +Cc: containers, netdev
In-Reply-To: <20100210172405.GB12251@us.ibm.com>

SH> What exactly is this == 0 case?  Does it mean 'use inherited
SH> netns'?

It means it wasn't in the checkpoint image to begin with, so yeah,
kinda.

SH> Don't you then still need to 

SH> 	get_net(current->nsproxy->net_ns);
SH>     nsproxy-> net_ns = current->nsproxy->net_ns;

Yes, I'll unify that path a bit.  Thanks.

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

^ permalink raw reply

* Re: [PATCH 5/5] sky2: Avoid down and up during sky2_reset
From: Stephen Hemminger @ 2010-02-10 17:34 UTC (permalink / raw)
  To: Mike McCormack; +Cc: netdev
In-Reply-To: <4B72BEC9.4000004@ring3k.org>

On Wed, 10 Feb 2010 23:12:25 +0900
Mike McCormack <mikem@ring3k.org> wrote:

> +	if (!interrupts_disabled) {
> +		/* Disable port IRQ */
> +		u32 imask = sky2_read32(hw, B0_IMSK);
> +		imask &= ~portirq_msk[port];
> +		sky2_write32(hw, B0_IMSK, imask);
> +		sky2_read32(hw, B0_IMSK);
>  

I don't like conditional context like this. It makes code
hard to debug, etc. Could you change sky2_down to do the irq
cleanup before calling hw_down?

-- 

^ permalink raw reply

* Re: [net-next PATCH v2 2/3] net: TCP thin linear timeouts
From: Damian Lukowski @ 2010-02-10 17:33 UTC (permalink / raw)
  To: Andreas Petlund
  Cc: netdev@vger.kernel.org, Ilpo Järvinen, Eric Dumazet,
	Arnd Hannemann, LKML, shemminger, David Miller,
	william.allen.simpson
In-Reply-To: <4B718FF4.6040906@simula.no>

Am 09.02.2010, 17:40 Uhr, schrieb Andreas Petlund <apetlund@simula.no>:

> On 02/08/2010 09:50 PM, Damian Lukowski wrote:
>>>  out_reset_timer:
>>> -	icsk->icsk_rto = min(icsk->icsk_rto << 1, TCP_RTO_MAX);
>>> +	/* If stream is thin, use linear timeouts. Since 'icsk_backoff' is
>>> +	 * used to reset timer, set to 0. Recalculate 'icsk_rto' as this
>>> +	 * might be increased if the stream oscillates between thin and  
>>> thick,
>>> +	 * thus the old value might already be too high compared to the value
>>> +	 * set by 'tcp_set_rto' in tcp_input.c which resets the rto without
>>> +	 * backoff. Limit to TCP_THIN_LT_RETRIES before initiating  
>>> exponential
>>> +	 * backoff behaviour to avoid continue hammering linear-timeout
>>> +	 * retransmissions into a black hole*/
>>> +	if ((tp->thin_lt || sysctl_tcp_force_thin_linear_timeouts) &&
>>> +	    tcp_stream_is_thin(sk) && sk->sk_state == TCP_ESTABLISHED &&
>>> +	    icsk->icsk_retransmits <= TCP_THIN_LT_RETRIES) {
>>> +		icsk->icsk_backoff = 0;
>>
>> Hi,
>> I think, this value should be at least 1, as icsk_backoff
>> might be decreased to -1 and used for bit-shifting in tcp_v4_err().
>> A lower boundary check might be even better.
>
> Hi
>
> Thanks for the feedback.
>
> As far as I can see, the check a couple of lines above the decrementation
> stops the icsk->icsk_backoff from being decremented if already zero.
> Beyond that I cannot find any more places where this situation may arise.
> Please correct me if I'm wrong and a boundary check is indeed warranted.

Oops, you are right, of course ...
I just had in mind, that a thin stream might also be a candidate for
backoff reversion when connectivity breaks down, that's why I said
"at least 1". And I really have forgotten the already existing check,  
sorry.
So, setting icsk_backoff = 0 will prevent a backoff reversion, but that's
ok, as the RTO is not doubled in the first place.

It might have been an issue, if you had not used __tcp_set_rto() but left
the value unchanged *and* a non-thin stream became thin at some point in
the RTO retransmission phase (if that is even possible).

Damian

^ permalink raw reply

* Re: [PATCH 2/4] C/R: Basic support for network namespaces and devices (v3)
From: Serge E. Hallyn @ 2010-02-10 17:24 UTC (permalink / raw)
  To: Dan Smith
  Cc: containers-qjLDD68F18O7TbgM5vRIOg, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1265750713-15749-3-git-send-email-danms-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>

Quoting Dan Smith (danms-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org):
> +struct ckpt_netdev_addr {
> +	__u16 type;

Pretty sure this will have to come after the union to get the
same sized struct on 32- and 64-bit.

> +	union {
> +		struct {
> +			__u32 inet4_local;
> +			__u32 inet4_address;
> +			__u32 inet4_mask;
> +			__u32 inet4_broadcast;
> +		};
> +	};
> +} __attribute__((aligned(8)));
> +
>  struct ckpt_hdr_eventpoll_items {
>  	struct ckpt_hdr h;
>  	__s32  epfile_objref;
> diff --git a/include/linux/checkpoint_types.h b/include/linux/checkpoint_types.h
> index 51efd5a..e646ec6 100644
> --- a/include/linux/checkpoint_types.h
> +++ b/include/linux/checkpoint_types.h
> @@ -86,6 +86,7 @@ struct ckpt_ctx {
>  	wait_queue_head_t ghostq;	/* waitqueue for ghost tasks */
>  	struct cred *realcred, *ecred;	/* tmp storage for cred at restart */
>  	struct list_head listen_sockets;/* listening parent sockets */
> +	int init_netns_ref;             /* Objref of root net namespace */
> 
>  	struct ckpt_stats stats;	/* statistics */
> 
> diff --git a/kernel/nsproxy.c b/kernel/nsproxy.c
> index b0e71f2..78f5615 100644
> --- a/kernel/nsproxy.c
> +++ b/kernel/nsproxy.c
> @@ -248,6 +248,11 @@ int ckpt_collect_ns(struct ckpt_ctx *ctx, struct task_struct *t)
>  	ret = ckpt_obj_collect(ctx, nsproxy->uts_ns, CKPT_OBJ_UTS_NS);
>  	if (ret < 0)
>  		goto out;
> +#ifdef CONFIG_CHECKPOINT_NETNS
> +	ret = ckpt_obj_collect(ctx, nsproxy->net_ns, CKPT_OBJ_NET_NS);
> +	if (ret < 0)
> +		goto out;
> +#endif
>  	ret = ckpt_obj_collect(ctx, nsproxy->ipc_ns, CKPT_OBJ_IPC_NS);
>  	if (ret < 0)
>  		goto out;
> @@ -288,6 +293,12 @@ static int do_checkpoint_ns(struct ckpt_ctx *ctx, struct nsproxy *nsproxy)
>  	if (ret < 0)
>  		goto out;
>  	h->ipc_objref = ret;
> +#ifdef CONFIG_CHECKPOINT_NETNS
> +	ret = checkpoint_obj(ctx, nsproxy->net_ns, CKPT_OBJ_NET_NS);
> +	if (ret < 0)
> +		goto out;
> +	h->net_objref = ret;
> +#endif
> 
>  	/* FIXME: for now, only marked visited to pacify leaks */
>  	ret = ckpt_obj_visit(ctx, nsproxy->mnt_ns, CKPT_OBJ_MNT_NS);
> @@ -306,6 +317,34 @@ int checkpoint_ns(struct ckpt_ctx *ctx, void *ptr)
>  	return do_checkpoint_ns(ctx, (struct nsproxy *) ptr);
>  }
> 
> +static int do_restore_netns(struct ckpt_ctx *ctx,
> +			    struct ckpt_hdr_ns *h,
> +			    struct nsproxy *nsproxy)
> +{
> +#ifdef CONFIG_CHECKPOINT_NETNS
> +	struct net *net_ns;
> +
> +	if (h->net_objref < 0)
> +		return -EINVAL;
> +	else if (h->net_objref == 0)
> +		return 0;

What exactly is this == 0 case?  Does it mean 'use inherited netns'?

Don't you then still need to 

	get_net(current->nsproxy->net_ns);
	nsproxy->net_ns = current->nsproxy->net_ns;

as below?

> +	net_ns = ckpt_obj_fetch(ctx, h->net_objref, CKPT_OBJ_NET_NS);
> +	if (IS_ERR(net_ns))
> +		return PTR_ERR(net_ns);
> +
> +	get_net(net_ns);
> +	nsproxy->net_ns = net_ns;
> +#else
> +	if (h->net_objref > 0)
> +		return -EINVAL;
> +	get_net(current->nsproxy->net_ns);
> +	nsproxy->net_ns = current->nsproxy->net_ns;

> +#endif
> +
> +	return 0;
> +}
> +
>  static struct nsproxy *do_restore_ns(struct ckpt_ctx *ctx)
>  {
>  	struct ckpt_hdr_ns *h;
> @@ -349,8 +388,6 @@ static struct nsproxy *do_restore_ns(struct ckpt_ctx *ctx)
>  	nsproxy->pid_ns = current->nsproxy->pid_ns;
>  	get_mnt_ns(current->nsproxy->mnt_ns);
>  	nsproxy->mnt_ns = current->nsproxy->mnt_ns;
> -	get_net(current->nsproxy->net_ns);
> -	nsproxy->net_ns = current->nsproxy->net_ns;
>  #else
>  	nsproxy = current->nsproxy;
>  	get_nsproxy(nsproxy);
> @@ -359,6 +396,10 @@ static struct nsproxy *do_restore_ns(struct ckpt_ctx *ctx)
>  	BUG_ON(nsproxy->ipc_ns != ipc_ns);
>  #endif
> 
> +	ret = do_restore_netns(ctx, h, nsproxy);
> +	if (ret < 0)
> +		goto out;
> +
>  	/* TODO: add more namespaces here */
>  	ret = 0;
>   out:

Otherwise, looks good.

thanks,
-serge

^ permalink raw reply

* Re: Re: [PATCH 18/23 v3] mlx4_core: Managing common port filters by
From: Yevgeny Petrilin @ 2010-02-10 17:22 UTC (permalink / raw)
  To: or.gerlitz; +Cc: yevgenyp, Roland Dreier, netdev, tziporet, liranl

On Sunday -10,January,-28163 09:59 PM, Or Gerlitz [or.gerlitz@gmail.com] wrote:
> Yevgeny Petrilin <yevgenyp@mellanox.co.il> wrote:
>> The Multicast filter configuration is done by the master, that manages 
>> the filter which is common for all the functions.
>> The VLAN filter is a bitwise OR of all the VLAN filters for all 
>> functions, the result is a false-positive filter.
> 
> Reading through patches "18/23 v3 mlx4_core: Managing common port filters..." &&
> "16/23 v3 mlx4_core: Multi-Function MCG support" I have difficulties to follow on the vlan and multicast filtering related changes and proposed architecture.
> 
> Can you explain them in a bit more detailed fashion? For example, in the multicast case I believe that currently a bloom filter is applied  at the port level and each packet which pass the filter and has no registered QP is consumed by the EN driver.
> At some point
> my understanding was that everyone (all PFs, all VFs, etc) move to exact match, is it correct?
> 
> As for the false-positive vlan filter, I understand that this means further filtering has to be applied by some driver. Where you thinking on the 802q driver? what about the case where a VF driver is assigned a vlan, does the mlx4 VF code does such filtering?
> 
> 
> Or.
The multicast bloom filter is a false-positive filter per port.
All multicast packets go through the Multicast tables mechanism where every PF/VF is registered for every multicast address it listens to.
We will add support for per-Function multicast promiscuous mode.

The deal with the VLAN filter is similar, it includes all VIDs that some PF/VF requested.
We plan to add a MAC+VLAN filter implementation, that would cancel the need in this filter.

Yevgeny


^ permalink raw reply

* Re: Subject: [PATCH 1/6] bna: Brocade 10Gb Ethernet device driver
From: Stephen Hemminger @ 2010-02-10 17:15 UTC (permalink / raw)
  To: Rasesh Mody; +Cc: netdev, adapter_linux_open_src_team
In-Reply-To: <201002100629.o1A6TFUk026363@blc-10-10.brocade.com>

On Tue, 9 Feb 2010 22:29:15 -0800
Rasesh Mody <rmody@brocade.com> wrote:

> +static struct pci_device_id bnad_pci_id_table[] = {
> +	{
> +	 .vendor = PCI_VENDOR_ID_BROCADE,
> +	 .device = PCI_DEVICE_ID_BROCADE_CATAPULT,
> +	 .subvendor = PCI_ANY_ID,
> +	 .subdevice = PCI_ANY_ID,
> +	 .class = PCI_CLASS_NETWORK_ETHERNET << 8,
> +	 .class_mask = 0xffff00},
> +	{0, 0}
> +};
> +
> +MODULE_DEVICE_TABLE(pci, bnad_pci_id_table

Use:

static DEFINE_PCI_DEVICE_TABLE(bnad_pci_id_table) = {
     
not sure if you require class/class_mask or can use
	PCI_VDEVICE(BROCADE, PCI_DEVICE_ID_BROCADE_CATAPLUT)

-- 

^ permalink raw reply

* Re: Subject: [PATCH 1/6] bna: Brocade 10Gb Ethernet device driver
From: Stephen Hemminger @ 2010-02-10 17:10 UTC (permalink / raw)
  To: Rasesh Mody; +Cc: netdev, adapter_linux_open_src_team
In-Reply-To: <201002100629.o1A6TFUk026363@blc-10-10.brocade.com>

On Tue, 9 Feb 2010 22:29:15 -0800
Rasesh Mody <rmody@brocade.com> wrote:

> +	netdev = alloc_etherdev(sizeof(struct bnad));
> +	if (!netdev) {
> +		dev_err(&pdev->dev, "alloc_etherdev failed\n");
> +		err = -ENOMEM;
> +		goto release_regions;
> +	}
> +	SET_NETDEV_DEV(netdev, &pdev->dev);
> +	pci_set_drvdata(pdev, netdev);
> +
> +	bnad = netdev_priv(netdev);
> +
> +	memset(bnad, 0, sizeof(struct bnad));

The private area provided by alloc_etherdev is already 
guaranteed to be zerod.

-- 

^ permalink raw reply

* Re: Subject: [PATCH 1/6] bna: Brocade 10Gb Ethernet device driver
From: Stephen Hemminger @ 2010-02-10 17:09 UTC (permalink / raw)
  To: Rasesh Mody; +Cc: netdev, adapter_linux_open_src_team
In-Reply-To: <201002100629.o1A6TFUk026363@blc-10-10.brocade.com>

On Tue, 9 Feb 2010 22:29:15 -0800
Rasesh Mody <rmody@brocade.com> wrote:

> +#define bnad_conf_lock()	down(&bnad->conf_sem)
> +#define bnad_conf_unlock()	up(&bnad->conf_sem)

Don't wrap locking in macros.
Don't use semaphores as locks, use mutex.

-- 

^ permalink raw reply

* Re: Subject: [PATCH 1/6] bna: Brocade 10Gb Ethernet device driver
From: Stephen Hemminger @ 2010-02-10 17:08 UTC (permalink / raw)
  To: Rasesh Mody; +Cc: netdev, adapter_linux_open_src_team
In-Reply-To: <201002100629.o1A6TFUk026363@blc-10-10.brocade.com>

On Tue, 9 Feb 2010 22:29:15 -0800
Rasesh Mody <rmody@brocade.com> wrote:

> +		BUG_ON(!(skb));

This driver seems to have lots of extra BUG_ON assertions
which implies either paranoia or still in development.

Putting in a check for NULL in places like this is not really
helpful. The check requires overhead, and will not provide any additional
help to the user. If the BUG_ON() is there the user will see
a backtrace and that cpu will be stuck ... if the BUG_ON was omitted,
and the skb was NULL, user would see a backtrace and cpu would
be stuck.

-- 

^ permalink raw reply

* Re: [PATCH net-next-2.6] macb: straighten out macb_mii_probe function
From: Haavard Skinnemoen @ 2010-02-10  9:47 UTC (permalink / raw)
  To: Jiri Pirko; +Cc: netdev, hskinnemoen, davem
In-Reply-To: <20100208151207.GE2601@psychotron.lab.eng.brq.redhat.com>

Jiri Pirko <jpirko@redhat.com> wrote:
> Use "phy_find_first" instead of manual lookup. Also use "phy_connect_direct"
> instead of "phy_connect" since we already have phy_device pointer here.
> 
> Signed-off-by: Jiri Pirko <jpirko@redhat.com>

Looks reasonable as far as I can see.

Acked-by: Haavard Skinnemoen <haavard.skinnemoen@atmel.com>

^ permalink raw reply

* Re: [PATCH 1/1 net-next] virtio_net: remove send queue
From: Shirley Ma @ 2010-02-10 16:54 UTC (permalink / raw)
  To: davem; +Cc: netdev, kvm, rusty, mst, linux-kernel
In-Reply-To: <1265674482.4484.10.camel@localhost.localdomain>

netperf results showed about 20% transmission gain with this patch for
virtio_net.

Thanks
Shirley


^ permalink raw reply

* RE: phy address in the device tree, vs auto probing
From: John Linn @ 2010-02-10 16:52 UTC (permalink / raw)
  To: Grant Likely, devicetree-discuss, netdev
In-Reply-To: <fa686aa41002100843o7a1d4160q8fb01768ad48dcf4@mail.gmail.com>

> -----Original Message-----
> From: glikely@secretlab.ca [mailto:glikely@secretlab.ca] On Behalf Of Grant Likely
> Sent: Wednesday, February 10, 2010 9:44 AM
> To: John Linn; devicetree-discuss; netdev
> Subject: Re: phy address in the device tree, vs auto probing
> 
> (cc'ing devicetree-discuss and netdev mailing lists)
> 
> On Tue, Feb 9, 2010 at 4:23 PM, John Linn <John.Linn@xilinx.com> wrote:
> > Hi Grant,
> >
> > I notice that the OF driver for the mdio bus is not doing auto probing.
> >
> > As we start putting in the phy layer in the emac drivers, the device
> > trees tend to have the phy address in them, but we're not sure we really
> > like that.
> >
> > We really think that being able to let the kernel find the phy address
> > is a big benefit, otherwise this is one other piece of info the user has
> > to know and get right.
> >
> > Am I missing something here?
> 
> No, you're not really missing something, but there is an inherent
> complexity in what you're wanting to do.  Like i2c, MDIO is one of
> those busses that is hard to probe reliable.  Some PHYs respond on
> more than one address, and there is no way to determine which MAC a
> PHY is wired up to.  Many PHYs can live on a single MDIO bus.  MACs
> with their own MDIO busses may still get wired to a PHY on a different
> bus.
> 
> In the simple case where there is a one:one:one relationship between
> MAC, MDIO bus and PHY, then it should be okay to probe the PHY,
> correct?  The question then must be asked; how does the kernel
> determine that it can use the simple case?  Nobody has yet defined a
> way to describe that in the device tree; mostly because nobody has
> needed to yet.
> 
> So, it is possible to do what you want, but you need a way to
> *explicitly* ask for that behaviour.  ie, some way to indicate in a
> MAC node which MDIO bus the phy is on, and that the phy needs to be
> probed for.  I think this should only be an option when the MDIO bus
> has only one PHY.  Come up with a proposal and post it to the
> devicetree-discuss mailing list.

Here's a couple ideas. See what everyone thinks as I'm not stuck on either.

Thanks,
John

1. What if we just don't specific a phy address with a reg property which would specify to auto probe it and find the phy as illustrated below?


		Ethernet_MAC: ethernet@81000000 {
			#address-cells = <1>;
			#size-cells = <1>;
			phy-handle = <&phy0>; 
			mdio {
				#address-cells = <1>;
				#size-cells = <0>;
				phy0: phy@7 {
				} ; 
			} ;

2. Or a special value (-1 or something not 0 - 31) in the phy address that specifies to auto probe as illustrated below.

		Ethernet_MAC: ethernet@81000000 {
			#address-cells = <1>;
			#size-cells = <1>;
			phy-handle = <&phy0>; 
			mdio {
				#address-cells = <1>;
				#size-cells = <0>;
				phy0: phy@7 {
					reg = <-1>;
				} ; 
			} ;

> 
> g.
> 
> >
> > Thanks,
> > John
> >
> >
> > int of_mdiobus_register(struct mii_bus *mdio, struct device_node *np)
> > {
> >        struct phy_device *phy;
> >        struct device_node *child;
> >        int rc, i;
> >
> >        /* Mask out all PHYs from auto probing.  Instead the PHYs listed
> > in
> >         * the device tree are populated after the bus has been
> > registered */
> >        mdio->phy_mask = ~0;
> >
> > This email and any attachments are intended for the sole use of the named recipient(s) and
> contain(s) confidential information that may be proprietary, privileged or copyrighted under
> applicable law. If you are not the intended recipient, do not read, copy, or forward this email
> message or any attachments. Delete this email message and any attachments immediately.
> >
> >
> >
> 
> 
> 
> --
> Grant Likely, B.Sc., P.Eng.
> Secret Lab Technologies Ltd.


This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.



^ permalink raw reply

* Re: Problem with VLANs and via-velocity driver
From: Patrick McHardy @ 2010-02-10 16:46 UTC (permalink / raw)
  To: Tiago Pierezan Camargo; +Cc: netdev
In-Reply-To: <loom.20100210T171704-457@post.gmane.org>

Tiago Pierezan Camargo wrote:
>> Kevin Shanahan <kmshanah <at> disenchant.net> writes:
> 
> Hi
> Sorry for resurrecting an old thread, but I'm having the same problem here.
> 
>> On Fri, Nov 13, 2009 at 07:40:39AM +0100, Patrick McHardy wrote:
>>> Kevin Shanahan wrote:
>>>> Hi,
>>>>
>>>> I've had some problems with getting a fairly simple (I thought) VLAN
>>>> configuration working with the on board Via NICs on my Via M700
>>>> board. Looks like as soon as a tagged VLAN interface is added, the
>>>> underlying "raw" (untagged) interface stops responding.
> 
> My setup is a bit different: I want to sniff all network activity, including
> possible tagged traffic. No tagged interfaces are created.
> 
>>>> A bit of searching found a few references to similar problems going
>>>> back a few years (2005, 2007). Sounded like there were some driver
>>>> issues, but it wasn't clear from the messages I found whether they
>>>> were believed to be fixed or not. I tried the same test using a
>>>> differnt NIC with the tg3 driver and there were no problems, so it
>>>> looks to me like it's still a via-velocity issue. Unfortunately I
>>>> don't have room to add NICs to this machine and need to use the on
>>>> board Via hardware.
>>> There's some special-casing for VID 0 in velocity_init_cam_filter().
>>> Does "ip link add link eth0 type vlan id 0" make any difference?
>> Thanks Patrick, this command got the untagged interface working again
>> (eth1 in my case). I can use this as a work around.
> 
> Unfortunately, Patrick's workaround doesn't work for me. To see any tagged
> packet I need to create a vlan that matches the packet vlan (same id).
> 
> Régis' promiscous mode patch (http://lists.openwall.net/netdev/2008/10/08/106)
> does not work either. I tried to completely disable hw vlan filtering without
> success. It seems my board uses a different register/offset to disable filtering.
> 
> ...
> 
> Any suggestions?

This should be fixed in the driver as discussed previously.

^ permalink raw reply


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