Netdev List
 help / color / mirror / Atom feed
* re: Ethernet driver for the WIZnet W5100 chip
From: Dan Carpenter @ 2012-04-10  8:40 UTC (permalink / raw)
  To: msink; +Cc: netdev

Hello Mike Sinkovsky,

This is a semi-automatic email about new static checker warnings.

The patch 8b1467a31343: "Ethernet driver for the WIZnet W5100 chip" 
from Apr 4, 2012, leads to the following Smatch complaint:

drivers/net/ethernet/wiznet/w5100.c:685 w5100_hw_probe()
	 error: we previously assumed 'data' could be null (see line 637)

drivers/net/ethernet/wiznet/w5100.c
   636	
   637		if (data && is_valid_ether_addr(data->mac_addr)) {
                    ^^^^
New check.

   638			memcpy(ndev->dev_addr, data->mac_addr, ETH_ALEN);
   639		} else {
   640			random_ether_addr(ndev->dev_addr);
   641			ndev->addr_assign_type |= NET_ADDR_RANDOM;
   642		}
   643	
   644		mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
   645		if (!mem)
   646			return -ENXIO;
   647		mem_size = resource_size(mem);
   648		if (!devm_request_mem_region(&pdev->dev, mem->start, mem_size, name))
   649			return -EBUSY;
   650		priv->base = devm_ioremap(&pdev->dev, mem->start, mem_size);
   651		if (!priv->base)
   652			return -EBUSY;
   653	
   654		spin_lock_init(&priv->reg_lock);
   655		priv->indirect = mem_size < W5100_BUS_DIRECT_SIZE;
   656		if (priv->indirect) {
   657			priv->read     = w5100_read_indirect;
   658			priv->write    = w5100_write_indirect;
   659			priv->read16   = w5100_read16_indirect;
   660			priv->write16  = w5100_write16_indirect;
   661			priv->readbuf  = w5100_readbuf_indirect;
   662			priv->writebuf = w5100_writebuf_indirect;
   663		} else {
   664			priv->read     = w5100_read_direct;
   665			priv->write    = w5100_write_direct;
   666			priv->read16   = w5100_read16_direct;
   667			priv->write16  = w5100_write16_direct;
   668			priv->readbuf  = w5100_readbuf_direct;
   669			priv->writebuf = w5100_writebuf_direct;
   670		}
   671	
   672		w5100_hw_reset(priv);
   673		if (w5100_read16(priv, W5100_RTR) != RTR_DEFAULT)
   674			return -ENODEV;
   675	
   676		irq = platform_get_irq(pdev, 0);
   677		if (irq < 0)
   678			return irq;
   679		ret = request_irq(irq, w5100_interrupt,
   680				  IRQ_TYPE_LEVEL_LOW, name, ndev);
   681		if (ret < 0)
   682			return ret;
   683		priv->irq = irq;
   684	
   685		priv->link_gpio = data->link_gpio;
                                  ^^^^^^^^^^^^^^^
New unchecked dereference.

   686		if (gpio_is_valid(priv->link_gpio)) {
   687			char *link_name = devm_kzalloc(&pdev->dev, 16, GFP_KERNEL);

regards,
dan carpenter

^ permalink raw reply

* Re: a F-RTO question
From: Ilpo Järvinen @ 2012-04-10  8:34 UTC (permalink / raw)
  To: Li Yu; +Cc: Chao Pei, Netdev
In-Reply-To: <4F728EC9.1050302@gmail.com>

On Wed, 28 Mar 2012, Li Yu wrote:

> Even, the reason is latter, it also means the netowrk already is
> recovered from temporarily congestion or disordered state, so we also should
> not enter loss state.

There are also other considerations in this btw, some devices are simply 
broken and go to RTO loop increasing the RTOs exponentially if we wouldn't 
force retransmit of the next segment (one of those newly sent segments) 
that isn't strictly mandatory imho. ...We used to do something more 
clever here and avoided that retransmission but learned the hard way that 
there's simply no way around it.

-- 
 i.

^ permalink raw reply

* Re: net: more accurate skb truesize - regression on Microblaze
From: Michal Simek @ 2012-04-10  8:37 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev, John Williams, David Miller
In-Reply-To: <1334046746.3126.13.camel@edumazet-glaptop>

On 04/10/2012 10:32 AM, Eric Dumazet wrote:
> On Tue, 2012-04-10 at 10:27 +0200, Eric Dumazet wrote:
>
>> BTW, some NIC drivers are known to provide fat skb in their rx path, and
>> need to be fixed as well. (Some others just lie about skb->truesize to
>> avoid the tcp slowdown, see my previous iwlwifi patch)
>>
>>
>
> What is the driver you currently use on your platform ?

Using Xilinx ll_temac(in mainline) and axi_emac.


Michal

-- 
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian

^ permalink raw reply

* Re: net: more accurate skb truesize - regression on Microblaze
From: Michal Simek @ 2012-04-10  8:36 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev, John Williams, David Miller
In-Reply-To: <1334046444.3126.12.camel@edumazet-glaptop>

On 04/10/2012 10:27 AM, Eric Dumazet wrote:
> On Tue, 2012-04-10 at 10:10 +0200, Michal Simek wrote:
>> Hi Eric,
>>
>> I have spent some time to investigate what it is causing regression on our network bechmark
>> and I have identified that your patch "net: more accurate skb truesize" is causing that.
>>
>> I see regression especially on incoming tcp (RX) connection which is 20%. On TX I see regression till 5%.
>>
>> In general microblaze systems are pretty sensitive for memory usage and working with it.
>> Increasing packet sizes has big impact on performance.
>> I was surprised that regression is so high.
>>
>>   From microblaze point of view is more important to remove that performance regression
>> than having more accurate memory accounting. 20% performance regression is simple so high
>> for that.
>> Isn't there any other way to doing better memory accounting?
>> Or is there any workaround which would be possible to use?
>>
>> Thanks,
>> Michal
>>
>
> Hi Michal
>
> We are currently working on the issue.
>
> TCP stack was a bit optimistic in the ideal skb->len / skb->truesize
> ratio that happened to 'mostly work' before my patch, but not anymore
> after it.
>
> memory accounting was wrong, we really wanted to be accurate, or risk
> OOM and crashes.
>
> Now we must fix tcp.
>
> In the meantime you could try :
>
> echo 1>/proc/sys/net/ipv4/tcp_adv_win_scale

will retest it and let you know.

Thanks,
Michal



-- 
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian

^ permalink raw reply

* Re: net: more accurate skb truesize - regression on Microblaze
From: Eric Dumazet @ 2012-04-10  8:32 UTC (permalink / raw)
  To: monstr; +Cc: netdev, John Williams, David Miller
In-Reply-To: <1334046444.3126.12.camel@edumazet-glaptop>

On Tue, 2012-04-10 at 10:27 +0200, Eric Dumazet wrote:

> BTW, some NIC drivers are known to provide fat skb in their rx path, and
> need to be fixed as well. (Some others just lie about skb->truesize to
> avoid the tcp slowdown, see my previous iwlwifi patch)
> 
> 

What is the driver you currently use on your platform ?

^ permalink raw reply

* Re: net: more accurate skb truesize - regression on Microblaze
From: Eric Dumazet @ 2012-04-10  8:27 UTC (permalink / raw)
  To: monstr; +Cc: netdev, John Williams, David Miller
In-Reply-To: <4F83EB0E.4020104@monstr.eu>

On Tue, 2012-04-10 at 10:10 +0200, Michal Simek wrote:
> Hi Eric,
> 
> I have spent some time to investigate what it is causing regression on our network bechmark
> and I have identified that your patch "net: more accurate skb truesize" is causing that.
> 
> I see regression especially on incoming tcp (RX) connection which is 20%. On TX I see regression till 5%.
> 
> In general microblaze systems are pretty sensitive for memory usage and working with it.
> Increasing packet sizes has big impact on performance.
> I was surprised that regression is so high.
> 
>  From microblaze point of view is more important to remove that performance regression
> than having more accurate memory accounting. 20% performance regression is simple so high
> for that.
> Isn't there any other way to doing better memory accounting?
> Or is there any workaround which would be possible to use?
> 
> Thanks,
> Michal
> 

Hi Michal

We are currently working on the issue.

TCP stack was a bit optimistic in the ideal skb->len / skb->truesize
ratio that happened to 'mostly work' before my patch, but not anymore
after it.

memory accounting was wrong, we really wanted to be accurate, or risk
OOM and crashes.

Now we must fix tcp.

In the meantime you could try :

echo 1 >/proc/sys/net/ipv4/tcp_adv_win_scale

BTW, some NIC drivers are known to provide fat skb in their rx path, and
need to be fixed as well. (Some others just lie about skb->truesize to
avoid the tcp slowdown, see my previous iwlwifi patch)

^ permalink raw reply

* Re: [net-next PATCH v1 7/7] macvlan: add FDB bridge ops and new macvlan mode
From: Michael S. Tsirkin @ 2012-04-10  8:14 UTC (permalink / raw)
  To: John Fastabend
  Cc: roprabhu, stephen.hemminger, davem, hadi, bhutchings,
	jeffrey.t.kirsher, netdev, gregory.v.rose, krkumar2, sri
In-Reply-To: <20120410080916.GB26540@redhat.com>

On Tue, Apr 10, 2012 at 11:09:16AM +0300, Michael S. Tsirkin wrote:
> On Mon, Apr 09, 2012 at 03:00:54PM -0700, John Fastabend wrote:
> > This adds a new macvlan mode MACVLAN_PASSTHRU_NOPROMISC
> > this mode acts the same as the original passthru mode _except_
> > it does not set promiscuous mode on the lowerdev. Because the
> > lowerdev is not put in promiscuous mode any unicast or multicast
> > addresses the device should receive must be explicitely added
> > with the FDB bridge ops. In many use cases the management stack
> > will know the mac addresses needed (maybe negotiated via EVB/VDP)
> > or may require only receiving known "good" mac addresses. This
> > mode with the FDB ops supports this usage model.
> 
> 
> Looks good to me. Some questions below:
> 
> > This patch is a result of Roopa Prabhu's work. Follow up
> > patches are needed for VEPA and VEB macvlan modes.
> 
> And bridge too?
> 
> Also, my understanding is that other modes won't need a flag
> like this since they don't put the device in promisc mode initially,
> so no assumptions are broken if we require all addresses
> to be declared, right?
> 
> A final question: I think we'll later add a macvlan mode
> that does not flood all multicasts. This would change behaviour
> in an incompatible way so we'll probably need yet another
> flag. Would it make sense to combine this functionality
> with nopromisc so we have less modes to support?

One other question I forgot:

> > CC: Roopa Prabhu <roprabhu@cisco.com>
> > CC: Michael S. Tsirkin <mst@redhat.com>
> > Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
> > ---
> > 
> >  drivers/net/macvlan.c   |   60 ++++++++++++++++++++++++++++++++++++++++++-----
> >  include/linux/if_link.h |    1 +
> >  2 files changed, 55 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
> > index b17fc90..9892d8d 100644
> > --- a/drivers/net/macvlan.c
> > +++ b/drivers/net/macvlan.c
> > @@ -181,6 +181,7 @@ static rx_handler_result_t macvlan_handle_frame(struct sk_buff **pskb)
> >  					  MACVLAN_MODE_PRIVATE |
> >  					  MACVLAN_MODE_VEPA    |
> >  					  MACVLAN_MODE_PASSTHRU|
> > +					  MACVLAN_MODE_PASSTHRU_NOPROMISC |
> >  					  MACVLAN_MODE_BRIDGE);
> >  		else if (src->mode == MACVLAN_MODE_VEPA)
> >  			/* flood to everyone except source */
> > @@ -312,7 +313,8 @@ static int macvlan_open(struct net_device *dev)
> >  	int err;
> >  
> >  	if (vlan->port->passthru) {
> > -		dev_set_promiscuity(lowerdev, 1);
> > +		if (vlan->mode == MACVLAN_MODE_PASSTHRU)
> > +			dev_set_promiscuity(lowerdev, 1);
> >  		goto hash_add;
> >  	}
> >  
> > @@ -344,12 +346,15 @@ static int macvlan_stop(struct net_device *dev)
> >  	struct macvlan_dev *vlan = netdev_priv(dev);
> >  	struct net_device *lowerdev = vlan->lowerdev;
> >  
> > +	dev_uc_unsync(lowerdev, dev);
> > +	dev_mc_unsync(lowerdev, dev);
> > +
> >  	if (vlan->port->passthru) {
> > -		dev_set_promiscuity(lowerdev, -1);
> > +		if (vlan->mode == MACVLAN_MODE_PASSTHRU)
> > +			dev_set_promiscuity(lowerdev, 1);
> >  		goto hash_del;
> >  	}
> >  
> > -	dev_mc_unsync(lowerdev, dev);
> >  	if (dev->flags & IFF_ALLMULTI)
> >  		dev_set_allmulti(lowerdev, -1);
> >  
> > @@ -399,10 +404,11 @@ static void macvlan_change_rx_flags(struct net_device *dev, int change)
> >  		dev_set_allmulti(lowerdev, dev->flags & IFF_ALLMULTI ? 1 : -1);

In the new mode, do we want to have promisc on lowerdev follow whatever
is set on the macvlan, like we do for allmulti?
I'm not sure at this point - what do others think?


> >  }
> >  
> > -static void macvlan_set_multicast_list(struct net_device *dev)
> > +static void macvlan_set_mac_lists(struct net_device *dev)
> >  {
> >  	struct macvlan_dev *vlan = netdev_priv(dev);
> >  
> > +	dev_uc_sync(vlan->lowerdev, dev);
> >  	dev_mc_sync(vlan->lowerdev, dev);
> >  }
> >  
> > @@ -542,6 +548,43 @@ static int macvlan_vlan_rx_kill_vid(struct net_device *dev,
> >  	return 0;
> >  }
> >  
> > +static int macvlan_fdb_add(struct ndmsg *ndm,
> > +			   struct net_device *dev,
> > +			   unsigned char *addr,
> > +			   u16 flags)
> > +{
> > +	struct macvlan_dev *vlan = netdev_priv(dev);
> > +	int err = -EINVAL;
> > +
> > +	if (!vlan->port->passthru)
> > +		return -EOPNOTSUPP;
> > +
> > +	if (is_unicast_ether_addr(addr))
> > +		err = dev_uc_add_excl(dev, addr);
> > +	else if (is_multicast_ether_addr(addr))
> > +		err = dev_mc_add_excl(dev, addr);
> > +
> > +	return err;
> > +}
> > +
> > +static int macvlan_fdb_del(struct ndmsg *ndm,
> > +			   struct net_device *dev,
> > +			   unsigned char *addr)
> > +{
> > +	struct macvlan_dev *vlan = netdev_priv(dev);
> > +	int err = -EINVAL;
> > +
> > +	if (!vlan->port->passthru)
> > +		return -EOPNOTSUPP;
> > +
> > +	if (is_unicast_ether_addr(addr))
> > +		err = dev_uc_del(dev, addr);
> > +	else if (is_multicast_ether_addr(addr))
> > +		err = dev_mc_del(dev, addr);
> > +
> > +	return err;
> > +}
> > +
> >  static void macvlan_ethtool_get_drvinfo(struct net_device *dev,
> >  					struct ethtool_drvinfo *drvinfo)
> >  {
> > @@ -572,11 +615,14 @@ static const struct net_device_ops macvlan_netdev_ops = {
> >  	.ndo_change_mtu		= macvlan_change_mtu,
> >  	.ndo_change_rx_flags	= macvlan_change_rx_flags,
> >  	.ndo_set_mac_address	= macvlan_set_mac_address,
> > -	.ndo_set_rx_mode	= macvlan_set_multicast_list,
> > +	.ndo_set_rx_mode	= macvlan_set_mac_lists,
> >  	.ndo_get_stats64	= macvlan_dev_get_stats64,
> >  	.ndo_validate_addr	= eth_validate_addr,
> >  	.ndo_vlan_rx_add_vid	= macvlan_vlan_rx_add_vid,
> >  	.ndo_vlan_rx_kill_vid	= macvlan_vlan_rx_kill_vid,
> > +	.ndo_fdb_add		= macvlan_fdb_add,
> > +	.ndo_fdb_del		= macvlan_fdb_del,
> > +	.ndo_fdb_dump		= ndo_dflt_fdb_dump,
> >  };
> >  
> >  void macvlan_common_setup(struct net_device *dev)
> > @@ -648,6 +694,7 @@ static int macvlan_validate(struct nlattr *tb[], struct nlattr *data[])
> >  		case MACVLAN_MODE_VEPA:
> >  		case MACVLAN_MODE_BRIDGE:
> >  		case MACVLAN_MODE_PASSTHRU:
> > +		case MACVLAN_MODE_PASSTHRU_NOPROMISC:
> >  			break;
> >  		default:
> >  			return -EINVAL;
> > @@ -711,7 +758,8 @@ int macvlan_common_newlink(struct net *src_net, struct net_device *dev,
> >  	if (data && data[IFLA_MACVLAN_MODE])
> >  		vlan->mode = nla_get_u32(data[IFLA_MACVLAN_MODE]);
> >  
> > -	if (vlan->mode == MACVLAN_MODE_PASSTHRU) {
> > +	if ((vlan->mode == MACVLAN_MODE_PASSTHRU) ||
> > +	    (vlan->mode == MACVLAN_MODE_PASSTHRU_NOPROMISC)) {
> >  		if (port->count)
> >  			return -EINVAL;
> >  		port->passthru = true;
> > diff --git a/include/linux/if_link.h b/include/linux/if_link.h
> > index 2f4fa93..db67b9d 100644
> > --- a/include/linux/if_link.h
> > +++ b/include/linux/if_link.h
> > @@ -265,6 +265,7 @@ enum macvlan_mode {
> >  	MACVLAN_MODE_VEPA    = 2, /* talk to other ports through ext bridge */
> >  	MACVLAN_MODE_BRIDGE  = 4, /* talk to bridge ports directly */
> >  	MACVLAN_MODE_PASSTHRU = 8,/* take over the underlying device */
> > +	MACVLAN_MODE_PASSTHRU_NOPROMISC = 16, /* passthru without promisc */
> >  };
> >  
> >  /* SR-IOV virtual function management section */

^ permalink raw reply

* net: more accurate skb truesize - regression on Microblaze
From: Michal Simek @ 2012-04-10  8:10 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev, John Williams, David Miller

Hi Eric,

I have spent some time to investigate what it is causing regression on our network bechmark
and I have identified that your patch "net: more accurate skb truesize" is causing that.

I see regression especially on incoming tcp (RX) connection which is 20%. On TX I see regression till 5%.

In general microblaze systems are pretty sensitive for memory usage and working with it.
Increasing packet sizes has big impact on performance.
I was surprised that regression is so high.

 From microblaze point of view is more important to remove that performance regression
than having more accurate memory accounting. 20% performance regression is simple so high
for that.
Isn't there any other way to doing better memory accounting?
Or is there any workaround which would be possible to use?

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian

^ permalink raw reply

* Re: [net-next PATCH v1 7/7] macvlan: add FDB bridge ops and new macvlan mode
From: Michael S. Tsirkin @ 2012-04-10  8:09 UTC (permalink / raw)
  To: John Fastabend
  Cc: roprabhu, stephen.hemminger, davem, hadi, bhutchings,
	jeffrey.t.kirsher, netdev, gregory.v.rose, krkumar2, sri
In-Reply-To: <20120409220053.3288.40867.stgit@jf-dev1-dcblab>

On Mon, Apr 09, 2012 at 03:00:54PM -0700, John Fastabend wrote:
> This adds a new macvlan mode MACVLAN_PASSTHRU_NOPROMISC
> this mode acts the same as the original passthru mode _except_
> it does not set promiscuous mode on the lowerdev. Because the
> lowerdev is not put in promiscuous mode any unicast or multicast
> addresses the device should receive must be explicitely added
> with the FDB bridge ops. In many use cases the management stack
> will know the mac addresses needed (maybe negotiated via EVB/VDP)
> or may require only receiving known "good" mac addresses. This
> mode with the FDB ops supports this usage model.


Looks good to me. Some questions below:

> This patch is a result of Roopa Prabhu's work. Follow up
> patches are needed for VEPA and VEB macvlan modes.

And bridge too?

Also, my understanding is that other modes won't need a flag
like this since they don't put the device in promisc mode initially,
so no assumptions are broken if we require all addresses
to be declared, right?

A final question: I think we'll later add a macvlan mode
that does not flood all multicasts. This would change behaviour
in an incompatible way so we'll probably need yet another
flag. Would it make sense to combine this functionality
with nopromisc so we have less modes to support?

> CC: Roopa Prabhu <roprabhu@cisco.com>
> CC: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
> ---
> 
>  drivers/net/macvlan.c   |   60 ++++++++++++++++++++++++++++++++++++++++++-----
>  include/linux/if_link.h |    1 +
>  2 files changed, 55 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
> index b17fc90..9892d8d 100644
> --- a/drivers/net/macvlan.c
> +++ b/drivers/net/macvlan.c
> @@ -181,6 +181,7 @@ static rx_handler_result_t macvlan_handle_frame(struct sk_buff **pskb)
>  					  MACVLAN_MODE_PRIVATE |
>  					  MACVLAN_MODE_VEPA    |
>  					  MACVLAN_MODE_PASSTHRU|
> +					  MACVLAN_MODE_PASSTHRU_NOPROMISC |
>  					  MACVLAN_MODE_BRIDGE);
>  		else if (src->mode == MACVLAN_MODE_VEPA)
>  			/* flood to everyone except source */
> @@ -312,7 +313,8 @@ static int macvlan_open(struct net_device *dev)
>  	int err;
>  
>  	if (vlan->port->passthru) {
> -		dev_set_promiscuity(lowerdev, 1);
> +		if (vlan->mode == MACVLAN_MODE_PASSTHRU)
> +			dev_set_promiscuity(lowerdev, 1);
>  		goto hash_add;
>  	}
>  
> @@ -344,12 +346,15 @@ static int macvlan_stop(struct net_device *dev)
>  	struct macvlan_dev *vlan = netdev_priv(dev);
>  	struct net_device *lowerdev = vlan->lowerdev;
>  
> +	dev_uc_unsync(lowerdev, dev);
> +	dev_mc_unsync(lowerdev, dev);
> +
>  	if (vlan->port->passthru) {
> -		dev_set_promiscuity(lowerdev, -1);
> +		if (vlan->mode == MACVLAN_MODE_PASSTHRU)
> +			dev_set_promiscuity(lowerdev, 1);
>  		goto hash_del;
>  	}
>  
> -	dev_mc_unsync(lowerdev, dev);
>  	if (dev->flags & IFF_ALLMULTI)
>  		dev_set_allmulti(lowerdev, -1);
>  
> @@ -399,10 +404,11 @@ static void macvlan_change_rx_flags(struct net_device *dev, int change)
>  		dev_set_allmulti(lowerdev, dev->flags & IFF_ALLMULTI ? 1 : -1);
>  }
>  
> -static void macvlan_set_multicast_list(struct net_device *dev)
> +static void macvlan_set_mac_lists(struct net_device *dev)
>  {
>  	struct macvlan_dev *vlan = netdev_priv(dev);
>  
> +	dev_uc_sync(vlan->lowerdev, dev);
>  	dev_mc_sync(vlan->lowerdev, dev);
>  }
>  
> @@ -542,6 +548,43 @@ static int macvlan_vlan_rx_kill_vid(struct net_device *dev,
>  	return 0;
>  }
>  
> +static int macvlan_fdb_add(struct ndmsg *ndm,
> +			   struct net_device *dev,
> +			   unsigned char *addr,
> +			   u16 flags)
> +{
> +	struct macvlan_dev *vlan = netdev_priv(dev);
> +	int err = -EINVAL;
> +
> +	if (!vlan->port->passthru)
> +		return -EOPNOTSUPP;
> +
> +	if (is_unicast_ether_addr(addr))
> +		err = dev_uc_add_excl(dev, addr);
> +	else if (is_multicast_ether_addr(addr))
> +		err = dev_mc_add_excl(dev, addr);
> +
> +	return err;
> +}
> +
> +static int macvlan_fdb_del(struct ndmsg *ndm,
> +			   struct net_device *dev,
> +			   unsigned char *addr)
> +{
> +	struct macvlan_dev *vlan = netdev_priv(dev);
> +	int err = -EINVAL;
> +
> +	if (!vlan->port->passthru)
> +		return -EOPNOTSUPP;
> +
> +	if (is_unicast_ether_addr(addr))
> +		err = dev_uc_del(dev, addr);
> +	else if (is_multicast_ether_addr(addr))
> +		err = dev_mc_del(dev, addr);
> +
> +	return err;
> +}
> +
>  static void macvlan_ethtool_get_drvinfo(struct net_device *dev,
>  					struct ethtool_drvinfo *drvinfo)
>  {
> @@ -572,11 +615,14 @@ static const struct net_device_ops macvlan_netdev_ops = {
>  	.ndo_change_mtu		= macvlan_change_mtu,
>  	.ndo_change_rx_flags	= macvlan_change_rx_flags,
>  	.ndo_set_mac_address	= macvlan_set_mac_address,
> -	.ndo_set_rx_mode	= macvlan_set_multicast_list,
> +	.ndo_set_rx_mode	= macvlan_set_mac_lists,
>  	.ndo_get_stats64	= macvlan_dev_get_stats64,
>  	.ndo_validate_addr	= eth_validate_addr,
>  	.ndo_vlan_rx_add_vid	= macvlan_vlan_rx_add_vid,
>  	.ndo_vlan_rx_kill_vid	= macvlan_vlan_rx_kill_vid,
> +	.ndo_fdb_add		= macvlan_fdb_add,
> +	.ndo_fdb_del		= macvlan_fdb_del,
> +	.ndo_fdb_dump		= ndo_dflt_fdb_dump,
>  };
>  
>  void macvlan_common_setup(struct net_device *dev)
> @@ -648,6 +694,7 @@ static int macvlan_validate(struct nlattr *tb[], struct nlattr *data[])
>  		case MACVLAN_MODE_VEPA:
>  		case MACVLAN_MODE_BRIDGE:
>  		case MACVLAN_MODE_PASSTHRU:
> +		case MACVLAN_MODE_PASSTHRU_NOPROMISC:
>  			break;
>  		default:
>  			return -EINVAL;
> @@ -711,7 +758,8 @@ int macvlan_common_newlink(struct net *src_net, struct net_device *dev,
>  	if (data && data[IFLA_MACVLAN_MODE])
>  		vlan->mode = nla_get_u32(data[IFLA_MACVLAN_MODE]);
>  
> -	if (vlan->mode == MACVLAN_MODE_PASSTHRU) {
> +	if ((vlan->mode == MACVLAN_MODE_PASSTHRU) ||
> +	    (vlan->mode == MACVLAN_MODE_PASSTHRU_NOPROMISC)) {
>  		if (port->count)
>  			return -EINVAL;
>  		port->passthru = true;
> diff --git a/include/linux/if_link.h b/include/linux/if_link.h
> index 2f4fa93..db67b9d 100644
> --- a/include/linux/if_link.h
> +++ b/include/linux/if_link.h
> @@ -265,6 +265,7 @@ enum macvlan_mode {
>  	MACVLAN_MODE_VEPA    = 2, /* talk to other ports through ext bridge */
>  	MACVLAN_MODE_BRIDGE  = 4, /* talk to bridge ports directly */
>  	MACVLAN_MODE_PASSTHRU = 8,/* take over the underlying device */
> +	MACVLAN_MODE_PASSTHRU_NOPROMISC = 16, /* passthru without promisc */
>  };
>  
>  /* SR-IOV virtual function management section */

^ permalink raw reply

* Re: [net-next PATCH v1 2/7] net: addr_list: add exclusive dev_uc_add and dev_mc_add
From: Michael S. Tsirkin @ 2012-04-10  8:03 UTC (permalink / raw)
  To: John Fastabend
  Cc: roprabhu, stephen.hemminger, davem, hadi, bhutchings,
	jeffrey.t.kirsher, netdev, gregory.v.rose, krkumar2, sri
In-Reply-To: <20120409220023.3288.59939.stgit@jf-dev1-dcblab>

On Mon, Apr 09, 2012 at 03:00:23PM -0700, John Fastabend wrote:
> This adds a dev_uc_add_excl() and dev_mc_add_excl() calls
> similar to the original dev_{uc|mc}_add() except it sets
> the global bit and returns -EEXIST for duplicat entires.
> 
> This is useful for drivers that support SR-IOV, macvlan
> devices and any other devices that need to manage the
> unicast and multicast lists.
> 
> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
> ---
> 
>  include/linux/netdevice.h |    2 +
>  net/core/dev_addr_lists.c |   97 ++++++++++++++++++++++++++++++++++++++-------
>  2 files changed, 83 insertions(+), 16 deletions(-)
> 
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 05822e5..b68a326 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -2572,6 +2572,7 @@ extern int dev_addr_init(struct net_device *dev);
>  
>  /* Functions used for unicast addresses handling */
>  extern int dev_uc_add(struct net_device *dev, unsigned char *addr);
> +extern int dev_uc_add_excl(struct net_device *dev, unsigned char *addr);
>  extern int dev_uc_del(struct net_device *dev, unsigned char *addr);
>  extern int dev_uc_sync(struct net_device *to, struct net_device *from);
>  extern void dev_uc_unsync(struct net_device *to, struct net_device *from);
> @@ -2581,6 +2582,7 @@ extern void dev_uc_init(struct net_device *dev);
>  /* Functions used for multicast addresses handling */
>  extern int dev_mc_add(struct net_device *dev, unsigned char *addr);
>  extern int dev_mc_add_global(struct net_device *dev, unsigned char *addr);
> +extern int dev_mc_add_excl(struct net_device *dev, unsigned char *addr);
>  extern int dev_mc_del(struct net_device *dev, unsigned char *addr);
>  extern int dev_mc_del_global(struct net_device *dev, unsigned char *addr);
>  extern int dev_mc_sync(struct net_device *to, struct net_device *from);
> diff --git a/net/core/dev_addr_lists.c b/net/core/dev_addr_lists.c
> index 29c07fe..5405e28 100644
> --- a/net/core/dev_addr_lists.c
> +++ b/net/core/dev_addr_lists.c
> @@ -21,12 +21,35 @@
>   * General list handling functions
>   */
>  
> +static int __hw_addr_create_ex(struct netdev_hw_addr_list *list,
> +			       unsigned char *addr, int addr_len,
> +			       unsigned char addr_type, bool global)
> +{
> +	struct netdev_hw_addr *ha;
> +	int alloc_size;
> +
> +	alloc_size = sizeof(*ha);
> +	if (alloc_size < L1_CACHE_BYTES)
> +		alloc_size = L1_CACHE_BYTES;
> +	ha = kmalloc(alloc_size, GFP_ATOMIC);
> +	if (!ha)
> +		return -ENOMEM;
> +	memcpy(ha->addr, addr, addr_len);
> +	ha->type = addr_type;
> +	ha->refcount = 1;
> +	ha->global_use = global;
> +	ha->synced = false;
> +	list_add_tail_rcu(&ha->list, &list->list);
> +	list->count++;
> +
> +	return 0;
> +}
> +
>  static int __hw_addr_add_ex(struct netdev_hw_addr_list *list,
>  			    unsigned char *addr, int addr_len,
>  			    unsigned char addr_type, bool global)
>  {
>  	struct netdev_hw_addr *ha;
> -	int alloc_size;
>  
>  	if (addr_len > MAX_ADDR_LEN)
>  		return -EINVAL;
> @@ -46,21 +69,7 @@ static int __hw_addr_add_ex(struct netdev_hw_addr_list *list,
>  		}
>  	}
>  
> -
> -	alloc_size = sizeof(*ha);
> -	if (alloc_size < L1_CACHE_BYTES)
> -		alloc_size = L1_CACHE_BYTES;
> -	ha = kmalloc(alloc_size, GFP_ATOMIC);
> -	if (!ha)
> -		return -ENOMEM;
> -	memcpy(ha->addr, addr, addr_len);
> -	ha->type = addr_type;
> -	ha->refcount = 1;
> -	ha->global_use = global;
> -	ha->synced = false;
> -	list_add_tail_rcu(&ha->list, &list->list);
> -	list->count++;
> -	return 0;
> +	return __hw_addr_create_ex(list, addr, addr_len, addr_type, global);
>  }
>  
>  static int __hw_addr_add(struct netdev_hw_addr_list *list, unsigned char *addr,
> @@ -377,6 +386,34 @@ EXPORT_SYMBOL(dev_addr_del_multiple);
>   */
>  
>  /**
> + *	dev_uc_add_excl - Add a global secondary unicast address
> + *	@dev: device
> + *	@addr: address to add
> + */
> +int dev_uc_add_excl(struct net_device *dev, unsigned char *addr)
> +{
> +	struct netdev_hw_addr *ha;
> +	int err;
> +
> +	netif_addr_lock_bh(dev);
> +	list_for_each_entry(ha, &dev->uc.list, list) {
> +		if (!memcmp(ha->addr, addr, dev->addr_len) &&
> +		    ha->type == NETDEV_HW_ADDR_T_UNICAST) {
> +			err = -EEXIST;
> +			goto out;
> +		}
> +	}
> +	err = __hw_addr_create_ex(&dev->uc, addr, dev->addr_len,
> +				  NETDEV_HW_ADDR_T_UNICAST, true);
> +	if (!err)
> +		__dev_set_rx_mode(dev);
> +out:
> +	netif_addr_unlock_bh(dev);
> +	return err;
> +}
> +EXPORT_SYMBOL(dev_uc_add_excl);
> +
> +/**
>   *	dev_uc_add - Add a secondary unicast address
>   *	@dev: device
>   *	@addr: address to add
> @@ -501,6 +538,34 @@ EXPORT_SYMBOL(dev_uc_init);
>   * Multicast list handling functions
>   */
>  
> +/**
> + *	dev_mc_add_excl - Add a global secondary multicast address
> + *	@dev: device
> + *	@addr: address to add
> + */
> +int dev_mc_add_excl(struct net_device *dev, unsigned char *addr)
> +{
> +	struct netdev_hw_addr *ha;
> +	int err;
> +
> +	netif_addr_lock_bh(dev);
> +	list_for_each_entry(ha, &dev->mc.list, list) {
> +		if (!memcmp(ha->addr, addr, dev->addr_len) &&
> +		    ha->type == NETDEV_HW_ADDR_T_UNICAST) {
> +			err = -EEXIST;
> +			goto out;
> +		}
> +	}
> +	err = __hw_addr_create_ex(&dev->mc, addr, dev->addr_len,
> +				  NETDEV_HW_ADDR_T_UNICAST, true);
> +	if (!err)
> +		__dev_set_rx_mode(dev);
> +out:
> +	netif_addr_unlock_bh(dev);
> +	return err;
> +}
> +EXPORT_SYMBOL(dev_mc_add_excl);
> +

Why is it a mistake to have duplicate multicast addresses?
For example macvlan floods all multicasts so it's fine,
and assuming it didn't, it might still be fine to have to
macvlans get the same multicast, no?

>  static int __dev_mc_add(struct net_device *dev, unsigned char *addr,
>  			bool global)
>  {

^ permalink raw reply

* Re: [PATCH] net: orphan queued skbs if device tx can stall
From: Eric Dumazet @ 2012-04-10  7:55 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: netdev, linux-kernel, David S. Miller, Jamal Hadi Salim,
	Stephen Hemminger, Jason Wang, Neil Horman, Jiri Pirko,
	Jeff Kirsher, Michał Mirosław, Ben Hutchings,
	Herbert Xu
In-Reply-To: <20120408171323.GA16012@redhat.com>

On Sun, 2012-04-08 at 20:13 +0300, Michael S. Tsirkin wrote:
> commit 0110d6f22f392f976e84ab49da1b42f85b64a3c5
> tun: orphan an skb on tx
> Fixed a configuration where skbs get queued
> at the tun device forever, blocking senders.
> 
> However this fix isn't waterproof:
> userspace can control whether the interface
> is stopped, and if it is, packets
> get queued in the qdisc, again potentially forever.
> 
> Complete the fix by setting a private flag and orphaning
> at the qdisc level.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  drivers/net/tun.c       |    3 +++
>  include/linux/if.h      |    1 +
>  net/core/dev.c          |    5 +++++
>  net/sched/sch_generic.c |    5 +++++
>  4 files changed, 14 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index bb8c72c..15c5bb8 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -535,6 +535,9 @@ static void tun_net_init(struct net_device *dev)
>  		dev->tx_queue_len = TUN_READQ_SIZE;  /* We prefer our own queue length */
>  		break;
>  	}
> +	/* Once queue becomes full, we stop tx until userspace
> +	 * dequeues some packets, that is potentially forever. */
> +	dev->priv_flags |= IFF_TX_CAN_STALL;
>  }
>  
>  /* Character device part */
> diff --git a/include/linux/if.h b/include/linux/if.h
> index f995c66..dd2c7f7 100644
> --- a/include/linux/if.h
> +++ b/include/linux/if.h
> @@ -81,6 +81,7 @@
>  #define IFF_UNICAST_FLT	0x20000		/* Supports unicast filtering	*/
>  #define IFF_TEAM_PORT	0x40000		/* device used as team port */
>  #define IFF_SUPP_NOFCS	0x80000		/* device supports sending custom FCS */
> +#define IFF_TX_CAN_STALL 0x100000	/* Device can stop tx forever */
>  
> 
>  #define IF_GET_IFACE	0x0001		/* for querying only */
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 5d59155..e812706 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -2516,6 +2516,11 @@ int dev_queue_xmit(struct sk_buff *skb)
>  	struct Qdisc *q;
>  	int rc = -ENOMEM;
>  
> +	/* Orphan the skb - required if we might hang on to it
> +	 * for indefinite time. */
> +	if (dev->priv_flags & IFF_TX_CAN_STALL)
> +		skb_orphan(skb);
> +
>  	/* Disable soft irqs for various locks below. Also
>  	 * stops preemption for RCU.
>  	 */
> diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
> index 67fc573..27883d1 100644
> --- a/net/sched/sch_generic.c
> +++ b/net/sched/sch_generic.c
> @@ -120,6 +120,11 @@ int sch_direct_xmit(struct sk_buff *skb, struct Qdisc *q,
>  	/* And release qdisc */
>  	spin_unlock(root_lock);
>  
> +	/* Orphan the skb - required if we might hang on to it
> +	 * for indefinite time. */
> +	if (dev->priv_flags & IFF_TX_CAN_STALL)
> +		skb_orphan(skb);
> +
>  	HARD_TX_LOCK(dev, txq, smp_processor_id());
>  	if (!netif_xmit_frozen_or_stopped(txq))
>  		ret = dev_hard_start_xmit(skb, dev, txq);

This slows down the core fastpath for a very specific use.

In your case I would just not use qdisc at all, like other virtual
devices.

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index bb8c72c..fd8c7f0 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -396,7 +396,7 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
 	    sk_filter(tun->socket.sk, skb))
 		goto drop;
 
-	if (skb_queue_len(&tun->socket.sk->sk_receive_queue) >= dev->tx_queue_len) {
+	if (skb_queue_len(&tun->socket.sk->sk_receive_queue) >= TUN_READQ_SIZE) {
 		if (!(tun->flags & TUN_ONE_QUEUE)) {
 			/* Normal queueing mode. */
 			/* Packet scheduler handles dropping of further packets. */
@@ -521,7 +521,7 @@ static void tun_net_init(struct net_device *dev)
 		/* Zero header length */
 		dev->type = ARPHRD_NONE;
 		dev->flags = IFF_POINTOPOINT | IFF_NOARP | IFF_MULTICAST;
-		dev->tx_queue_len = TUN_READQ_SIZE;  /* We prefer our own queue length */
+		dev->tx_queue_len = 0;
 		break;
 
 	case TUN_TAP_DEV:
@@ -532,7 +532,7 @@ static void tun_net_init(struct net_device *dev)
 
 		eth_hw_addr_random(dev);
 
-		dev->tx_queue_len = TUN_READQ_SIZE;  /* We prefer our own queue length */
+		dev->tx_queue_len = 0;
 		break;
 	}
 }

^ permalink raw reply related

* Re: [PATCH 2/2] qlcnic: Remove redundant NULL check on kfree().
From: Dan Carpenter @ 2012-04-10  7:49 UTC (permalink / raw)
  To: santosh nayak
  Cc: anirban.chakraborty, rajesh.borundia, sony.chacko, linux-driver,
	netdev, kernel-janitors
In-Reply-To: <1333951879-3230-1-git-send-email-santoshprasadnayak@gmail.com>

On Mon, Apr 09, 2012 at 11:41:19AM +0530, santosh nayak wrote:
> From: Santosh Nayak <santoshprasadnayak@gmail.com>
> 
> kfree() checks for NULL before freeing the memory.
> Remove redundant NULL check.
> This is just a clean up and also looks good from performance point of view.

I doubt it has any impact on performance.

[snip]

>  static void qlcnic_free_lb_filters_mem(struct qlcnic_adapter *adapter)
>  {
> -	if (adapter->fhash.fmax && adapter->fhash.fhead)
> -		kfree(adapter->fhash.fhead);
> -
> +	kfree(adapter->fhash.fhead);

This changes how the code works.  That's very sloppy to not notice
that.  If you did notice it, it should be justified in the
changelog.  :(

>  	adapter->fhash.fhead = NULL;
>  	adapter->fhash.fmax = 0;
>  }

regards,
dan carpenter

^ permalink raw reply

* Re: [PATCH] gianfar: add missing include
From: Michael Neuling @ 2012-04-10  6:41 UTC (permalink / raw)
  To: Richard Cochran; +Cc: David S. Miller, linuxppc-dev, netdev, linux-next
In-Reply-To: <20120410063641.GA10025@localhost.localdomain>

Richard Cochran <richardcochran@gmail.com> wrote:
> On Tue, Apr 10, 2012 at 02:18:47PM +1000, Michael Neuling wrote:
> > This is because of a missing include file from:
> >   6663628 gianfar: Support the get_ts_info ethtool method.
> > 
> > Signed-off-by: Michael Neuling <mikey@neuling.org>
> 
> I did a poor job testing the non-x86 stuff in this series.  Thanks for
> the fix.
> 
> Acked-by: Richard Cochran <richardcochran@gmail.com>

Looks like davem fixed it before my post anyway...

http://git.kernel.org/?p=linux/kernel/git/davem/net-next.git;a=commitdiff;h=65a85a8d4dcabe95587ab61bece38b8d58174043

This is in next-20120410.

Sorry for the noise. 

Mikey

^ permalink raw reply

* Re: [PATCH 2/2] phonet: Sort out initiailziation and cleanup code.
From: Rémi Denis-Courmont @ 2012-04-10  6:39 UTC (permalink / raw)
  To: ext Eric W. Biederman
  Cc: David Miller, Eric Dumazet, Eric Van Hensbergen, Dave Jones,
	linux-kernel, netdev, Sasha Levin
In-Reply-To: <m162dctjbo.fsf_-_@fess.ebiederm.org>

Le vendredi 6 avril 2012 18:35:39 ext Eric W. Biederman a écrit :
> Recently an oops was reported in phonet if there was a failure during
> network namespace creation.
> 
> [  163.733755] ------------[ cut here ]------------
> [  163.734501] kernel BUG at include/net/netns/generic.h:45!
> [  163.734501] invalid opcode: 0000 [#1] PREEMPT SMP
> [  163.734501] CPU 2
> [  163.734501] Pid: 19145, comm: trinity Tainted: G        W
> 3.4.0-rc1-next-20120405-sasha-dirty #57 [  163.734501] RIP:
> 0010:[<ffffffff824d6062>]  [<ffffffff824d6062>] phonet_pernet+0x182/0x1a0 [
>  163.734501] RSP: 0018:ffff8800674d5ca8  EFLAGS: 00010246
> [  163.734501] RAX: 000000003fffffff RBX: 0000000000000000 RCX:
> ffff8800678c88d8 [  163.734501] RDX: 00000000003f4000 RSI: ffff8800678c8910
> RDI: 0000000000000282 [  163.734501] RBP: ffff8800674d5cc8 R08:
> 0000000000000000 R09: 0000000000000000 [  163.734501] R10: 0000000000000000
> R11: 0000000000000000 R12: ffff880068bec920 [  163.734501] R13:
> ffffffff836b90c0 R14: 0000000000000000 R15: 0000000000000000 [  163.734501]
> FS:  00007f055e8de700(0000) GS:ffff88007d000000(0000)
> knlGS:0000000000000000 [  163.734501] CS:  0010 DS: 0000 ES: 0000 CR0:
> 000000008005003b
> [  163.734501] CR2: 00007f055e6bb518 CR3: 0000000070c16000 CR4:
> 00000000000406e0 [  163.734501] DR0: 0000000000000000 DR1: 0000000000000000
> DR2: 0000000000000000 [  163.734501] DR3: 0000000000000000 DR6:
> 00000000ffff0ff0 DR7: 0000000000000400 [  163.734501] Process trinity (pid:
> 19145, threadinfo ffff8800674d4000, task ffff8800678c8000) [  163.734501]
> Stack:
> [  163.734501]  ffffffff824d5f00 ffffffff810e2ec1 ffff880067ae0000
> 00000000ffffffd4 [  163.734501]  ffff8800674d5cf8 ffffffff824d667a
> ffff880067ae0000 00000000ffffffd4 [  163.734501]  ffffffff836b90c0
> 0000000000000000 ffff8800674d5d18 ffffffff824d707d [  163.734501] Call
> Trace:
> [  163.734501]  [<ffffffff824d5f00>] ? phonet_pernet+0x20/0x1a0
> [  163.734501]  [<ffffffff810e2ec1>] ? get_parent_ip+0x11/0x50
> [  163.734501]  [<ffffffff824d667a>] phonet_device_destroy+0x1a/0x100
> [  163.734501]  [<ffffffff824d707d>] phonet_device_notify+0x3d/0x50
> [  163.734501]  [<ffffffff810dd96e>] notifier_call_chain+0xee/0x130
> [  163.734501]  [<ffffffff810dd9d1>] raw_notifier_call_chain+0x11/0x20
> [  163.734501]  [<ffffffff821cce12>] call_netdevice_notifiers+0x52/0x60
> [  163.734501]  [<ffffffff821cd235>] rollback_registered_many+0x185/0x270
> [  163.734501]  [<ffffffff821cd334>] unregister_netdevice_many+0x14/0x60
> [  163.734501]  [<ffffffff823123e3>] ipip_exit_net+0x1b3/0x1d0
> [  163.734501]  [<ffffffff82312230>] ? ipip_rcv+0x420/0x420
> [  163.734501]  [<ffffffff821c8515>] ops_exit_list+0x35/0x70
> [  163.734501]  [<ffffffff821c911b>] setup_net+0xab/0xe0
> [  163.734501]  [<ffffffff821c9416>] copy_net_ns+0x76/0x100
> [  163.734501]  [<ffffffff810dc92b>] create_new_namespaces+0xfb/0x190
> [  163.734501]  [<ffffffff810dca21>] unshare_nsproxy_namespaces+0x61/0x80
> [  163.734501]  [<ffffffff810afd1f>] sys_unshare+0xff/0x290
> [  163.734501]  [<ffffffff8187622e>] ? trace_hardirqs_on_thunk+0x3a/0x3f
> [  163.734501]  [<ffffffff82665539>] system_call_fastpath+0x16/0x1b
> [  163.734501] Code: e0 c3 fe 66 0f 1f 44 00 00 48 c7 c2 40 60 4d 82 be 01
> 00 00 00 48 c7 c7 80 d1 23 83 e8 48 2a c4 fe e8 73 06 c8 fe 48 85 db 75 0e
> <0f> 0b 0f 1f 40 00 eb fe 66 0f 1f 44 00 00 48 83 c4 10 48 89 d8 [ 
> 163.734501] RIP  [<ffffffff824d6062>] phonet_pernet+0x182/0x1a0 [ 
> 163.734501]  RSP <ffff8800674d5ca8>
> [  163.861289] ---[ end trace fb5615826c548066 ]---
> 
> After investigation it turns out there were two issues.
> 1) Phonet was not implementing network devices but was using
> register_pernet_device instead of register_pernet_subsys.
> 
>    This was allowing there to be cases when phonenet was not initialized and
> the phonet net_generic was not set for a network namespace when network
> device events were being reported on the netdevice_notifier for a network
> namespace leading to the oops above.
> 
> 2) phonet_exit_net was implementing a confusing and special case of handling
> all network devices from going away that it was hard to see was correct,
> and would only occur when the phonet module was removed.
> 
>    Now that unregister_netdevice_notifier has been modified to synthesize
> unregistration events for the network devices that are extant when called
> this confusing special case in phonet_exit_net is no longer needed.
> 
> Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>

Acked-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>

> ---
>  net/phonet/pn_dev.c |   21 ++-------------------
>  1 files changed, 2 insertions(+), 19 deletions(-)
> 
> diff --git a/net/phonet/pn_dev.c b/net/phonet/pn_dev.c
> index 9b9a85e..bf5cf69 100644
> --- a/net/phonet/pn_dev.c
> +++ b/net/phonet/pn_dev.c
> @@ -331,23 +331,6 @@ static int __net_init phonet_init_net(struct net *net)
> 
>  static void __net_exit phonet_exit_net(struct net *net)
>  {
> -	struct phonet_net *pnn = phonet_pernet(net);
> -	struct net_device *dev;
> -	unsigned i;
> -
> -	rtnl_lock();
> -	for_each_netdev(net, dev)
> -		phonet_device_destroy(dev);
> -
> -	for (i = 0; i < 64; i++) {
> -		dev = pnn->routes.table[i];
> -		if (dev) {
> -			rtm_phonet_notify(RTM_DELROUTE, dev, i);
> -			dev_put(dev);
> -		}
> -	}
> -	rtnl_unlock();
> -
>  	proc_net_remove(net, "phonet");
>  }
> 
> @@ -361,7 +344,7 @@ static struct pernet_operations phonet_net_ops = {
>  /* Initialize Phonet devices list */
>  int __init phonet_device_init(void)
>  {
> -	int err = register_pernet_device(&phonet_net_ops);
> +	int err = register_pernet_subsys(&phonet_net_ops);
>  	if (err)
>  		return err;
> 
> @@ -377,7 +360,7 @@ void phonet_device_exit(void)
>  {
>  	rtnl_unregister_all(PF_PHONET);
>  	unregister_netdevice_notifier(&phonet_device_notifier);
> -	unregister_pernet_device(&phonet_net_ops);
> +	unregister_pernet_subsys(&phonet_net_ops);
>  	proc_net_remove(&init_net, "pnresource");
>  }
-- 
Rémi Denis-Courmont
http://www.remlab.net/

^ permalink raw reply

* Re: [PATCH 09/14] usb/net: rndis: merge media type definitions
From: Jussi Kivilinna @ 2012-04-10  6:37 UTC (permalink / raw)
  To: Linus Walleij
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA, linux-usb-u79uwXL29TY76Z2rM5mHXA,
	Greg Kroah-Hartman, David S. Miller, Felipe Balbi, Haiyang Zhang,
	Wei Yongjun, Ben Hutchings
In-Reply-To: <1333874900-5072-1-git-send-email-linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

Quoting Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>:

> Let's have a unified table of RNDIS media. We used to have a similar
> table with NDIS_* prefix from the gadget driver, but since we're only
> using RNDIS in the kernel (IIRC NDIS, non-remote, is for the windows-
> internal network drivers so what do we care) let's prefix everything
> with RNDIS. Some of the definitions were conflicting, in one of the
> defines 0x0B is bearer "CO WAN" and in two others "BPC". Well I took
> the majority vote. Two definition of medium 0x09 calls it "wireless
> WAN" but one vote for "wireless LAN" but in this case I am sticking
> with the minority, "Wide Area Network" does not make much sense in
> this case as far as I can tell.
>
> NOTE: latin singular and plural is so screwed up in these defines
> that it makes my eyes bleed. But I will not attempt to submit a
> patch converting all use of _MEDIA_ to _MEDIUM_ while I can probably
> tell from the semantics of the code that RNDIS_MEDIA_STATE_CONNECTED
> is most probably (erroneously) referring to a singular, unless it
> can return an array of connected media. I suspect these erroneous
> plurals are used in documentation and such so I don't want to
> mess around with things for no functional change.
>
> Signed-off-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>  drivers/net/usb/rndis_host.c |    6 ++--
>  drivers/usb/gadget/f_rndis.c |    6 ++--
>  drivers/usb/gadget/rndis.c   |    8 ++--
>  include/linux/rndis.h        |   70  
> ++++++++++++-----------------------------
>  4 files changed, 31 insertions(+), 59 deletions(-)
>
> diff --git a/drivers/net/usb/rndis_host.c b/drivers/net/usb/rndis_host.c
> index f56b9f7..2dd47ec 100644
> --- a/drivers/net/usb/rndis_host.c
> +++ b/drivers/net/usb/rndis_host.c
> @@ -400,18 +400,18 @@ generic_rndis_bind(struct usbnet *dev, struct  
> usb_interface *intf, int flags)
>  			0, (void **) &phym, &reply_len);
>  	if (retval != 0 || !phym) {
>  		/* OID is optional so don't fail here. */
> -		phym_unspec = cpu_to_le32(RNDIS_PHYSICAL_MEDIUM_UNSPECIFIED);
> +		phym_unspec = cpu_to_le32(RNDIS_MEDIUM_UNSPECIFIED);
>  		phym = &phym_unspec;
>  	}
>  	if ((flags & FLAG_RNDIS_PHYM_WIRELESS) &&
> -	    *phym != cpu_to_le32(RNDIS_PHYSICAL_MEDIUM_WIRELESS_LAN)) {
> +	    *phym != cpu_to_le32(RNDIS_MEDIUM_WIRELESS_LAN)) {

This does not work.. RNDIS_PHYSICAL_MEDIUM_WIRELESS_LAN == 0x01 and  
RNDIS_MEDIUM_WIRELESS_LAN == 0x09.

>  		netif_dbg(dev, probe, dev->net,
>  			  "driver requires wireless physical medium, but device is not\n");
>  		retval = -ENODEV;
>  		goto halt_fail_and_release;
>  	}
>  	if ((flags & FLAG_RNDIS_PHYM_NOT_WIRELESS) &&
> -	    *phym == cpu_to_le32(RNDIS_PHYSICAL_MEDIUM_WIRELESS_LAN)) {
> +	    *phym == cpu_to_le32(RNDIS_MEDIUM_WIRELESS_LAN)) {
>  		netif_dbg(dev, probe, dev->net,
>  			  "driver requires non-wireless physical medium, but device is  
> wireless.\n");
>  		retval = -ENODEV;
> diff --git a/drivers/usb/gadget/f_rndis.c b/drivers/usb/gadget/f_rndis.c
> index 7b1cf18..c25d24e 100644
> --- a/drivers/usb/gadget/f_rndis.c
> +++ b/drivers/usb/gadget/f_rndis.c
> @@ -636,7 +636,7 @@ static void rndis_open(struct gether *geth)
>
>  	DBG(cdev, "%s\n", __func__);
>
> -	rndis_set_param_medium(rndis->config, NDIS_MEDIUM_802_3,
> +	rndis_set_param_medium(rndis->config, RNDIS_MEDIUM_802_3,
>  				bitrate(cdev->gadget) / 100);
>  	rndis_signal_connect(rndis->config);
>  }
> @@ -647,7 +647,7 @@ static void rndis_close(struct gether *geth)
>
>  	DBG(geth->func.config->cdev, "%s\n", __func__);
>
> -	rndis_set_param_medium(rndis->config, NDIS_MEDIUM_802_3, 0);
> +	rndis_set_param_medium(rndis->config, RNDIS_MEDIUM_802_3, 0);
>  	rndis_signal_disconnect(rndis->config);
>  }
>
> @@ -764,7 +764,7 @@ rndis_bind(struct usb_configuration *c, struct  
> usb_function *f)
>  		goto fail;
>  	rndis->config = status;
>
> -	rndis_set_param_medium(rndis->config, NDIS_MEDIUM_802_3, 0);
> +	rndis_set_param_medium(rndis->config, RNDIS_MEDIUM_802_3, 0);
>  	rndis_set_host_mac(rndis->config, rndis->ethaddr);
>
>  #if 0
> diff --git a/drivers/usb/gadget/rndis.c b/drivers/usb/gadget/rndis.c
> index 79ed261..d9086ca 100644
> --- a/drivers/usb/gadget/rndis.c
> +++ b/drivers/usb/gadget/rndis.c
> @@ -252,7 +252,7 @@ static int gen_ndis_query_resp(int configNr, u32  
> OID, u8 *buf,
>  		if (rndis_debug > 1)
>  			pr_debug("%s: RNDIS_OID_GEN_LINK_SPEED\n", __func__);
>  		if (rndis_per_dev_params[configNr].media_state
> -				== NDIS_MEDIA_STATE_DISCONNECTED)
> +				== RNDIS_MEDIA_STATE_DISCONNECTED)
>  			*outbuf = cpu_to_le32(0);
>  		else
>  			*outbuf = cpu_to_le32(
> @@ -758,7 +758,7 @@ static int rndis_indicate_status_msg(int  
> configNr, u32 status)
>  int rndis_signal_connect(int configNr)
>  {
>  	rndis_per_dev_params[configNr].media_state
> -			= NDIS_MEDIA_STATE_CONNECTED;
> +			= RNDIS_MEDIA_STATE_CONNECTED;
>  	return rndis_indicate_status_msg(configNr,
>  					  RNDIS_STATUS_MEDIA_CONNECT);
>  }
> @@ -766,7 +766,7 @@ int rndis_signal_connect(int configNr)
>  int rndis_signal_disconnect(int configNr)
>  {
>  	rndis_per_dev_params[configNr].media_state
> -			= NDIS_MEDIA_STATE_DISCONNECTED;
> +			= RNDIS_MEDIA_STATE_DISCONNECTED;
>  	return rndis_indicate_status_msg(configNr,
>  					  RNDIS_STATUS_MEDIA_DISCONNECT);
>  }
> @@ -1173,7 +1173,7 @@ int rndis_init(void)
>  		rndis_per_dev_params[i].used = 0;
>  		rndis_per_dev_params[i].state = RNDIS_UNINITIALIZED;
>  		rndis_per_dev_params[i].media_state
> -				= NDIS_MEDIA_STATE_DISCONNECTED;
> +				= RNDIS_MEDIA_STATE_DISCONNECTED;
>  		INIT_LIST_HEAD(&(rndis_per_dev_params[i].resp_queue));
>  	}
>
> diff --git a/include/linux/rndis.h b/include/linux/rndis.h
> index 2e0b1bd..705dccc 100644
> --- a/include/linux/rndis.h
> +++ b/include/linux/rndis.h
> @@ -100,17 +100,27 @@
>
>  #define RNDIS_STATUS_TOKEN_RING_OPEN_ERROR	0xC0011000
>
> -/* codes for RNDIS_OID_GEN_PHYSICAL_MEDIUM */
> -#define	RNDIS_PHYSICAL_MEDIUM_UNSPECIFIED	0x00000000
> -#define	RNDIS_PHYSICAL_MEDIUM_WIRELESS_LAN	0x00000001
> -#define	RNDIS_PHYSICAL_MEDIUM_CABLE_MODEM	0x00000002
> -#define	RNDIS_PHYSICAL_MEDIUM_PHONE_LINE	0x00000003
> -#define	RNDIS_PHYSICAL_MEDIUM_POWER_LINE	0x00000004
> -#define	RNDIS_PHYSICAL_MEDIUM_DSL		0x00000005
> -#define	RNDIS_PHYSICAL_MEDIUM_FIBRE_CHANNEL	0x00000006
> -#define	RNDIS_PHYSICAL_MEDIUM_1394		0x00000007
> -#define	RNDIS_PHYSICAL_MEDIUM_WIRELESS_WAN	0x00000008
> -#define	RNDIS_PHYSICAL_MEDIUM_MAX		0x00000009
> +/*  Remote NDIS medium types. */
> +#define RNDIS_MEDIUM_UNSPECIFIED		0x00000000
> +#define RNDIS_MEDIUM_802_3			0x00000000
> +#define RNDIS_MEDIUM_802_5			0x00000001
> +#define RNDIS_MEDIUM_FDDI			0x00000002
> +#define RNDIS_MEDIUM_WAN			0x00000003
> +#define RNDIS_MEDIUM_LOCAL_TALK			0x00000004
> +#define RNDIS_MEDIUM_ARCNET_RAW			0x00000006
> +#define RNDIS_MEDIUM_ARCNET_878_2		0x00000007
> +#define RNDIS_MEDIUM_ATM			0x00000008
> +#define RNDIS_MEDIUM_WIRELESS_LAN		0x00000009
> +#define RNDIS_MEDIUM_IRDA			0x0000000A
> +#define RNDIS_MEDIUM_BPC			0x0000000B
> +#define RNDIS_MEDIUM_CO_WAN			0x0000000C
> +#define RNDIS_MEDIUM_1394			0x0000000D

NDIS medium type enumeration is not same as NDIS physical medium enumeration..

http://msdn.microsoft.com/en-us/library/windows/hardware/ff565910%28v=vs.85%29.aspx
http://msdn.microsoft.com/en-us/library/windows/hardware/ff569621%28v=vs.85%29.aspx


> +/* Not a real medium, defined as an upper-bound */
> +#define RNDIS_MEDIUM_MAX			0x0000000E
> +
> +/* Remote NDIS medium connection states. */
> +#define RNDIS_MEDIA_STATE_CONNECTED		0x00000000
> +#define RNDIS_MEDIA_STATE_DISCONNECTED		0x00000001
>
>  /* packet filter bits used by RNDIS_OID_GEN_CURRENT_PACKET_FILTER */
>  #define RNDIS_PACKET_TYPE_DIRECTED		0x00000001
> @@ -163,21 +173,6 @@
>  #define NDIS_MINIPORT_SUPPORTS_CANCEL_SEND_PACKETS    0x00800000
>  #define NDIS_MINIPORT_64BITS_DMA                      0x01000000
>
> -#define NDIS_MEDIUM_802_3		0x00000000
> -#define NDIS_MEDIUM_802_5		0x00000001
> -#define NDIS_MEDIUM_FDDI		0x00000002
> -#define NDIS_MEDIUM_WAN			0x00000003
> -#define NDIS_MEDIUM_LOCAL_TALK		0x00000004
> -#define NDIS_MEDIUM_DIX			0x00000005
> -#define NDIS_MEDIUM_ARCENT_RAW		0x00000006
> -#define NDIS_MEDIUM_ARCENT_878_2	0x00000007
> -#define NDIS_MEDIUM_ATM			0x00000008
> -#define NDIS_MEDIUM_WIRELESS_LAN	0x00000009
> -#define NDIS_MEDIUM_IRDA		0x0000000A
> -#define NDIS_MEDIUM_BPC			0x0000000B
> -#define NDIS_MEDIUM_CO_WAN		0x0000000C
> -#define NDIS_MEDIUM_1394		0x0000000D
> -
>  #define NDIS_PACKET_TYPE_DIRECTED	0x00000001
>  #define NDIS_PACKET_TYPE_MULTICAST	0x00000002
>  #define NDIS_PACKET_TYPE_ALL_MULTICAST	0x00000004
> @@ -191,9 +186,6 @@
>  #define NDIS_PACKET_TYPE_FUNCTIONAL	0x00000400
>  #define NDIS_PACKET_TYPE_MAC_FRAME	0x00000800
>
> -#define NDIS_MEDIA_STATE_CONNECTED	0x00000000
> -#define NDIS_MEDIA_STATE_DISCONNECTED	0x00000001
> -
>  #define NDIS_MAC_OPTION_COPY_LOOKAHEAD_DATA     0x00000001
>  #define NDIS_MAC_OPTION_RECEIVE_SERIALIZED      0x00000002
>  #define NDIS_MAC_OPTION_TRANSFERS_NOT_PEND      0x00000004
> @@ -421,23 +413,3 @@
>   * driver and remote device, if necessary.
>   */
>  #define REMOTE_NDIS_BUS_MSG			0xff000001
> -
> -/*  Remote NDIS medium types. */
> -#define RNDIS_MEDIUM_802_3			0x00000000
> -#define RNDIS_MEDIUM_802_5			0x00000001
> -#define RNDIS_MEDIUM_FDDI				0x00000002
> -#define RNDIS_MEDIUM_WAN				0x00000003
> -#define RNDIS_MEDIUM_LOCAL_TALK			0x00000004
> -#define RNDIS_MEDIUM_ARCNET_RAW			0x00000006
> -#define RNDIS_MEDIUM_ARCNET_878_2			0x00000007
> -#define RNDIS_MEDIUM_ATM				0x00000008
> -#define RNDIS_MEDIUM_WIRELESS_WAN			0x00000009
> -#define RNDIS_MEDIUM_IRDA				0x0000000a
> -#define RNDIS_MEDIUM_CO_WAN			0x0000000b
> -/* Not a real medium, defined as an upper-bound */
> -#define RNDIS_MEDIUM_MAX				0x0000000d
> -
> -
> -/* Remote NDIS medium connection states. */
> -#define RNDIS_MEDIA_STATE_CONNECTED		0x00000000
> -#define RNDIS_MEDIA_STATE_DISCONNECTED		0x00000001
> --
> 1.7.7.6
>
>
>



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

^ permalink raw reply

* Re: [PATCH] gianfar: add missing include
From: Richard Cochran @ 2012-04-10  6:36 UTC (permalink / raw)
  To: Michael Neuling; +Cc: David S. Miller, linuxppc-dev, netdev, linux-next
In-Reply-To: <25411.1334031527@neuling.org>

On Tue, Apr 10, 2012 at 02:18:47PM +1000, Michael Neuling wrote:
> This is because of a missing include file from:
>   6663628 gianfar: Support the get_ts_info ethtool method.
> 
> Signed-off-by: Michael Neuling <mikey@neuling.org>

I did a poor job testing the non-x86 stuff in this series.  Thanks for
the fix.

Acked-by: Richard Cochran <richardcochran@gmail.com>

^ permalink raw reply

* Re: 3.2.8/amd64 full interrupt hangs and deadlocks under big network copies (page allocation failure)
From: Eric Dumazet @ 2012-04-10  6:11 UTC (permalink / raw)
  To: Marc MERLIN
  Cc: David Miller, Larry.Finger, bhutchings, linux-wireless, netdev
In-Reply-To: <20120410051127.GA32048@merlins.org>

On Mon, 2012-04-09 at 22:11 -0700, Marc MERLIN wrote:
> On Tue, Apr 10, 2012 at 05:56:20AM +0200, Eric Dumazet wrote:
> > > What wireless device are we dealing with again?
> > 
> > Problem seems related to tailroom needed by mac80211
> > (IEEE80211_ENCRYPT_TAILROOM = 18 bytes)
> > 
> > So we must reallocate skb->head, thats impressive nobody cares.
> > 
> > [ 3007.249687] ieee80211_skb_resize(skb=ffff8802329846e8) cloned=1 head_need=0 tail_need=18 skb->len=1494 ksize=4096 tailroom=0 headroom=2282
> > [ 3007.249693] ieee80211_skb_resize(skb=ffff8802329846e8) cloned=0 head_need=0 tail_need=0 skb->len=1526 ksize=8192 tailroom=64 headroom=2250
> > 
> > Ouch... skb_tailroom() seems wrong ... it seems pskb_expand_head() is really suboptimal.
> > 
> > It appears tcp_sendmsg() tries to fill skb completely, with no available tailroom :
> > 
> >                         if (skb_tailroom(skb) > 0) {
> >                                 /* We have some space in skb head. Superb! */
> >                                 if (copy > skb_tailroom(skb))
> >                                         copy = skb_tailroom(skb);
> >                                 err = skb_add_data_nocache(sk, skb, from, copy);
> >                                 if (err)
> >                                         goto do_fault;
> >                         } else {
> > 
> > Shouldnt we take into account dev->needed_tailroom ?
> > 
> > I'll submit a pskb_expand_head() fix asap.
> 
> Thanks for finding this.
> 
> To answer an earlier question, I tried the non wireless case too.
> 
> The problem is harder to reproduce over e1000e though, I just got two short
> hangs where my mouse cursor was hung for 5-10 seconds, but nothing in
> syslog/dmesg this time.
> 
> I'm pretty sure this older log below did happen on e1000e with wireless disabled
> though (but it had a taint 'O'):
> 
> If that helps, my earlier message had the traces below.
> 
> I can report back when you have a patch you'd like me to try out.

Hi Marc

Please try following patch, as it solved the problem for me (no more
order-1 allocations in tx path)

Thanks !

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 3337027..70a3f8d 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -481,6 +481,7 @@ struct sk_buff {
 	union {
 		__u32		mark;
 		__u32		dropcount;
+		__u32		avail_size;
 	};
 
 	sk_buff_data_t		transport_header;
@@ -1366,6 +1367,18 @@ static inline int skb_tailroom(const struct sk_buff *skb)
 }
 
 /**
+ *	skb_availroom - bytes at buffer end
+ *	@skb: buffer to check
+ *
+ *	Return the number of bytes of free space at the tail of an sk_buff
+ *	allocated by sk_stream_alloc()
+ */
+static inline int skb_availroom(const struct sk_buff *skb)
+{
+	return skb_is_nonlinear(skb) ? 0 : skb->avail_size - skb->len;
+}
+
+/**
  *	skb_reserve - adjust headroom
  *	@skb: buffer to alter
  *	@len: bytes to move
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index baf8d28..1887454 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -952,9 +952,11 @@ int pskb_expand_head(struct sk_buff *skb, int nhead, int ntail,
 		goto adjust_others;
 	}
 
-	data = kmalloc(size + sizeof(struct skb_shared_info), gfp_mask);
+	data = kmalloc(size + SKB_DATA_ALIGN(sizeof(struct skb_shared_info)),
+		       gfp_mask);
 	if (!data)
 		goto nodata;
+	size = ksize(data) - SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
 
 	/* Copy only real data... and, alas, header. This should be
 	 * optimized for the cases when header is void.
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 5d54ed3..87f497f 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -701,11 +701,12 @@ struct sk_buff *sk_stream_alloc_skb(struct sock *sk, int size, gfp_t gfp)
 	skb = alloc_skb_fclone(size + sk->sk_prot->max_header, gfp);
 	if (skb) {
 		if (sk_wmem_schedule(sk, skb->truesize)) {
+			skb_reserve(skb, sk->sk_prot->max_header);
 			/*
 			 * Make sure that we have exactly size bytes
 			 * available to the caller, no more, no less.
 			 */
-			skb_reserve(skb, skb_tailroom(skb) - size);
+			skb->avail_size = size;		
 			return skb;
 		}
 		__kfree_skb(skb);
@@ -995,10 +996,9 @@ new_segment:
 				copy = seglen;
 
 			/* Where to copy to? */
-			if (skb_tailroom(skb) > 0) {
+			if (skb_availroom(skb) > 0) {
 				/* We have some space in skb head. Superb! */
-				if (copy > skb_tailroom(skb))
-					copy = skb_tailroom(skb);
+				copy = min_t(int, copy, skb_availroom(skb));
 				err = skb_add_data_nocache(sk, skb, from, copy);
 				if (err)
 					goto do_fault;
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 364784a..376b2cf 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -2060,7 +2060,7 @@ static void tcp_retrans_try_collapse(struct sock *sk, struct sk_buff *to,
 		/* Punt if not enough space exists in the first SKB for
 		 * the data in the second
 		 */
-		if (skb->len > skb_tailroom(to))
+		if (skb->len > skb_availroom(to))
 			break;
 
 		if (after(TCP_SKB_CB(skb)->end_seq, tcp_wnd_end(tp)))

^ permalink raw reply related

* linux-next: build failure after merge of the final tree (net-next tree related)
From: Stephen Rothwell @ 2012-04-10  5:29 UTC (permalink / raw)
  To: David Miller, netdev
  Cc: linux-next, linux-kernel, Mike Sinkovsky, Martin Schwidefsky,
	Heiko Carstens

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

Hi all,

After merging the final tree, today's linux-next build (s390 allmodconfig)
failed like this:

drivers/net/ethernet/wiznet/w5100.c: In function 'w5100_read_direct':
drivers/net/ethernet/wiznet/w5100.c:121:2: error: implicit declaration of function 'ioread8' [-Werror=implicit-function-declaration]
drivers/net/ethernet/wiznet/w5100.c: In function 'w5100_write_direct':
drivers/net/ethernet/wiznet/w5100.c:127:2: error: implicit declaration of function 'iowrite8' [-Werror=implicit-function-declaration]
drivers/net/ethernet/wiznet/w5100.c: In function 'w5100_read_indirect':
drivers/net/ethernet/wiznet/w5100.c:188:2: error: implicit declaration of function 'mmiowb' [-Werror=implicit-function-declaration]
drivers/net/ethernet/wiznet/w5100.c: In function 'w5100_hw_probe':
drivers/net/ethernet/wiznet/w5100.c:680:6: error: 'IRQ_TYPE_LEVEL_LOW' undeclared (first use in this function)

Caused by commit 8b1467a31343 ("Ethernet driver for the WIZnet W5100 chip").
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* RE: [PATCH] davinci_emac: Add cpu_freq support
From: Manjunathappa, Prakash @ 2012-04-10  5:16 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: netdev@vger.kernel.org,
	davinci-linux-open-source@linux.davincidsp.com,
	davem@davemloft.net
In-Reply-To: <4F82C830.1020100@mvista.com>

Hi Sergei,

On Mon, Apr 09, 2012 at 16:59:52, Sergei Shtylyov wrote:
> Hello.
> 
> On 09-04-2012 14:49, Manjunathappa, Prakash wrote:
> 
> > Reconfigure interrupt coalesce parameter for changed emac bus_freq
> > due to DVFS.
> 
> > Signed-off-by: Manjunathappa, Prakash<prakash.pm@ti.com>
> > ---
> >   drivers/net/ethernet/ti/davinci_emac.c |   60 ++++++++++++++++++++++++++++++++
> >   1 files changed, 60 insertions(+), 0 deletions(-)
> 
> > diff --git a/drivers/net/ethernet/ti/davinci_emac.c b/drivers/net/ethernet/ti/davinci_emac.c
> > index 174a334..11d3bd7 100644
> > --- a/drivers/net/ethernet/ti/davinci_emac.c
> > +++ b/drivers/net/ethernet/ti/davinci_emac.c
> [...]
> > @@ -1761,6 +1765,46 @@ static const struct net_device_ops emac_netdev_ops = {
> >   #endif
> >   };
> >
> > +#ifdef CONFIG_CPU_FREQ
> > +static int davinci_emac_cpufreq_transition(struct notifier_block *nb,
> > +				     unsigned long val, void *data)
> > +{
> > +	int ret = 0;
> > +	struct emac_priv *priv;
> > +
> > +	priv = container_of(nb, struct emac_priv, freq_transition);
> > +	if (priv->coal_intvl != 0) {
> > +		if (val == CPUFREQ_POSTCHANGE) {
> > +			if (emac_bus_frequency != clk_get_rate(emac_clk)) {
> 
>     These 3 *if*s could be collapsed into one, and so indentation level 
> lowered significantly.
> 

Ok I will tie them with "&&".

> > +				struct ethtool_coalesce coal;
> > +
> > +				emac_bus_frequency = clk_get_rate(emac_clk);
> > +
> > +				priv->bus_freq_mhz = (u32)(emac_bus_frequency /
> > +						1000000);
> > +				coal.rx_coalesce_usecs = (priv->coal_intvl
> > +						<<  4);
> > +				ret = emac_set_coalesce(priv->ndev,&coal);
> > +			}
> > +		}
> > +	}
> > +	return ret;
> > +}
> > +
> > +static inline int davinci_emac_cpufreq_register(struct emac_priv *priv)
> > +{
> > +	priv->freq_transition.notifier_call = davinci_emac_cpufreq_transition;
> > +	return cpufreq_register_notifier(&priv->freq_transition,
> > +					 CPUFREQ_TRANSITION_NOTIFIER);
> > +}
> > +
> > +static inline void davinci_emac_cpufreq_deregister(struct emac_priv *priv)
> > +{
> > +	cpufreq_unregister_notifier(&priv->freq_transition,
> > +				    CPUFREQ_TRANSITION_NOTIFIER);
> > +}
> > +#endif
> > +
> >   /**
> >    * davinci_emac_probe: EMAC device probe
> >    * @pdev: The DaVinci EMAC device that we are removing
> > @@ -1925,8 +1969,21 @@ static int __devinit davinci_emac_probe(struct platform_device *pdev)
> >   			   "(regs: %p, irq: %d)\n",
> >   			   (void *)priv->emac_base_phys, ndev->irq);
> >   	}
> > +
> > +#ifdef CONFIG_CPU_FREQ
> > +	rc = davinci_emac_cpufreq_register(priv);
> > +	if (rc) {
> > +		dev_err(&pdev->dev, "error in register_netdev\n");
> 
>     Really?
> 

Ahh, my bad... I will fix this.

> > +		rc = -ENODEV;
> > +		goto cpufreq_reg_err;
> > +	}
> > +#endif
> >   	return 0;
> >
> > +#ifdef CONFIG_CPU_FREQ
> > +cpufreq_reg_err:
> > +	unregister_netdev(ndev);
> > +#endif
> >   netdev_reg_err:
> >   	clk_disable(emac_clk);
> >   no_irq_res:
> > @@ -1973,6 +2030,9 @@ static int __devexit davinci_emac_remove(struct platform_device *pdev)
> >
> >   	release_mem_region(res->start, resource_size(res));
> >
> > +#ifdef CONFIG_CPU_FREQ
> > +	davinci_emac_cpufreq_deregister(priv);
> > +#endif
> 
>     This is considered a bad practice to use #ifdef in the body of function.
> Define the faunction you call here as empty inline in case CONFIG_CPU_FREQ is 
> not defined instead. The same about davinci_emac_cpufreq_register().
> 

Ok. I will correct this as you suggested.

Thanks,
Prakash

> WBR, Sergei
> 

^ permalink raw reply

* Re: 3.2.8/amd64 full interrupt hangs and deadlocks under big network copies (page allocation failure)
From: Marc MERLIN @ 2012-04-10  5:11 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David Miller, Larry.Finger, bhutchings, linux-wireless, netdev
In-Reply-To: <1334030180.13293.98.camel@edumazet-glaptop>

On Tue, Apr 10, 2012 at 05:56:20AM +0200, Eric Dumazet wrote:
> > What wireless device are we dealing with again?
> 
> Problem seems related to tailroom needed by mac80211
> (IEEE80211_ENCRYPT_TAILROOM = 18 bytes)
> 
> So we must reallocate skb->head, thats impressive nobody cares.
> 
> [ 3007.249687] ieee80211_skb_resize(skb=ffff8802329846e8) cloned=1 head_need=0 tail_need=18 skb->len=1494 ksize=4096 tailroom=0 headroom=2282
> [ 3007.249693] ieee80211_skb_resize(skb=ffff8802329846e8) cloned=0 head_need=0 tail_need=0 skb->len=1526 ksize=8192 tailroom=64 headroom=2250
> 
> Ouch... skb_tailroom() seems wrong ... it seems pskb_expand_head() is really suboptimal.
> 
> It appears tcp_sendmsg() tries to fill skb completely, with no available tailroom :
> 
>                         if (skb_tailroom(skb) > 0) {
>                                 /* We have some space in skb head. Superb! */
>                                 if (copy > skb_tailroom(skb))
>                                         copy = skb_tailroom(skb);
>                                 err = skb_add_data_nocache(sk, skb, from, copy);
>                                 if (err)
>                                         goto do_fault;
>                         } else {
> 
> Shouldnt we take into account dev->needed_tailroom ?
> 
> I'll submit a pskb_expand_head() fix asap.

Thanks for finding this.

To answer an earlier question, I tried the non wireless case too.

The problem is harder to reproduce over e1000e though, I just got two short
hangs where my mouse cursor was hung for 5-10 seconds, but nothing in
syslog/dmesg this time.

I'm pretty sure this older log below did happen on e1000e with wireless disabled
though (but it had a taint 'O'):

If that helps, my earlier message had the traces below.

I can report back when you have a patch you'd like me to try out.

Thanks again,
Marc


> [28451.191115] WorkerPool/1248 D ffff88013bc93580     0 12483   3740 0x00000080
> [28451.191115]  ffff8801189ba100 0000000000000082 0000000000000000 ffff880134f2e180
> [28451.191115]  0000000000013580 ffff88001614bfd8 ffff88001614bfd8 ffff8801189ba100
> [28451.191115]  ffffffff811b4b62 000000010164525a 0000000000000046 ffffffff8165a250
> [28451.191115] Call Trace:
> [28451.191115]  [<ffffffff811b4b62>] ? sha_transform+0x395/0x1209
> [28451.191115]  [<ffffffff8134a9b4>] ? __mutex_lock_common.isra.6+0x13d/0x219
> [28451.191115]  [<ffffffff81242714>] ? extract_buf+0x86/0xf2
> [28451.191115]  [<ffffffff8134a7e6>] ? mutex_lock+0xf/0x1f
> [28451.191115]  [<ffffffff81298979>] ? rtnetlink_rcv+0xe/0x28
> [28451.191115]  [<ffffffff812ad007>] ? netlink_unicast+0xe6/0x14e
> [28451.191115]  [<ffffffff812ad26b>] ? netlink_sendmsg+0x1fc/0x237
> [28451.191115]  [<ffffffff8127c770>] ? sock_sendmsg+0xc1/0xde
> [28451.191115]  [<ffffffff810eca23>] ? __cache_free.isra.40+0x19/0x1a7
> [28451.191115]  [<ffffffff813496be>] ? nl_pid_hash_rehash+0xc8/0xef
> [28451.191115]  [<ffffffff8103e0fa>] ? get_parent_ip+0x9/0x1b
> [28451.191115]  [<ffffffff8103e0fa>] ? get_parent_ip+0x9/0x1b
> [28451.191115]  [<ffffffff8134e1d2>] ? sub_preempt_count+0x83/0x94
> [28451.191115]  [<ffffffff810fd81e>] ? fget_light+0x85/0x8d
> [28451.191115]  [<ffffffff8127e0e3>] ? sys_sendto+0xf7/0x137
> [28451.191115]  [<ffffffff8103e0fa>] ? get_parent_ip+0x9/0x1b
> [28451.191115]  [<ffffffff8134e1d2>] ? sub_preempt_count+0x83/0x94
> [28451.191115]  [<ffffffff8134b725>] ? _raw_spin_unlock+0x24/0x30
> [28451.191115]  [<ffffffff8108d73e>] ? audit_syscall_entry+0x105/0x130
> [28451.191115]  [<ffffffff8134fd52>] ? system_call_fastpath+0x16/0x1b
> 
> 
> 
> Below are lines I got in syslog during the copy.
> Highlight is:
> [ 4437.367046] kworker/1:1: page allocation failure: order:1, mode:0x20
> and then:
> [ 8640.516177] INFO: task flush-0:37:7122 blocked for more than 120 seconds.
> and then 120,000 lines(!) of:
> [ 9654.042164] ieee80211 phy0: failed to reallocate TX buffer
> 
> unedited lines below.
> 
> So, any idea of what I can try next?
> 
> Thanks,
> Marc
> 
> 
> [ 4437.367046] kworker/1:1: page allocation failure: order:1, mode:0x20
> [ 4437.367053] Pid: 8067, comm: kworker/1:1 Tainted: G           O 3.2.8-amd64-volpreempt-noide-20120208 #1
> [ 4437.367056] Call Trace:
> [ 4437.367058]  <IRQ>  [<ffffffff810b9ec0>] ? warn_alloc_failed+0x11f/0x132
> [ 4437.367074]  [<ffffffff810bcdaa>] ? __alloc_pages_nodemask+0x6b1/0x72f
> [ 4437.367081]  [<ffffffff810ec911>] ? kmem_getpages+0x4c/0xd9
> [ 4437.367086]  [<ffffffff810ec911>] ? kmem_getpages+0x4c/0xd9
> [ 4437.367090]  [<ffffffff810edd21>] ? fallback_alloc+0x123/0x1c2
> [ 4437.367096]  [<ffffffff812846db>] ? pskb_expand_head+0xe0/0x24a
> [ 4437.367101]  [<ffffffff810ee215>] ? __kmalloc+0xb2/0x10a
> [ 4437.367105]  [<ffffffff812846db>] ? pskb_expand_head+0xe0/0x24a
> [ 4437.367139]  [<ffffffffa03e22c1>] ? ieee80211_skb_resize+0x64/0x9d [mac80211]
> [ 4437.367154]  [<ffffffffa03e4252>] ? ieee80211_subif_start_xmit+0x705/0x883 [mac80211]
> [ 4437.367175]  [<ffffffff8128e767>] ? dev_hard_start_xmit+0x40b/0x552
> [ 4437.367179]  [<ffffffff812a4adc>] ? sch_direct_xmit+0x63/0x13a
> [ 4437.367182]  [<ffffffff8128eb8e>] ? dev_queue_xmit+0x2e0/0x4b5
> [ 4437.367185]  [<ffffffff812b764d>] ? ip_finish_output2+0x1c7/0x218
> [ 4437.367188]  [<ffffffff812b86aa>] ? __ip_flush_pending_frames.isra.29+0x69/0x69
> [ 4437.367191]  [<ffffffff812b8a6a>] ? ip_queue_xmit+0x2cd/0x30d
> [ 4437.367195]  [<ffffffff81066be9>] ? getnstimeofday+0x4a/0x7b
> [ 4437.367198]  [<ffffffff812ca1d2>] ? tcp_transmit_skb+0x6d7/0x70a
> [ 4437.367201]  [<ffffffff812cac5f>] ? tcp_write_xmit+0x698/0x7a1
> [ 4437.367204]  [<ffffffff812c77bf>] ? tcp_ack+0x14e3/0x1658
> [ 4437.367207]  [<ffffffff812c89bd>] ? tcp_established_options+0x2b/0x9e
> [ 4437.367210]  [<ffffffff812cada9>] ? __tcp_push_pending_frames+0x18/0x44
> [ 4437.367213]  [<ffffffff812c4e27>] ? tcp_data_snd_check+0x2c/0xfd
> [ 4437.367216]  [<ffffffff812c86c5>] ? tcp_rcv_established+0x4f0/0x549
> [ 4437.367220]  [<ffffffff8103ec39>] ? select_task_rq_fair+0x67b/0x690
> [ 4437.367223]  [<ffffffff812ce735>] ? tcp_v4_do_rcv+0x166/0x323
> [ 4437.367226]  [<ffffffff812cfdce>] ? tcp_v4_rcv+0x404/0x65d
> [ 4437.367230]  [<ffffffff812b4d55>] ? ip_local_deliver_finish+0x148/0x1ba
> [ 4437.367233]  [<ffffffff8128cfa4>] ? __netif_receive_skb+0x3f2/0x43f
> [ 4437.367236]  [<ffffffff8128d31d>] ? netif_receive_skb+0x7e/0x84
> [ 4437.367239]  [<ffffffff8128d7dd>] ? napi_gro_receive+0x1c/0x29
> [ 4437.367241]  [<ffffffff8128d398>] ? napi_skb_finish+0x1c/0x31
> [ 4437.367253]  [<ffffffffa026bde3>] ? e1000_clean_rx_irq+0x1f3/0x290 [e1000e]
> [ 4437.367261]  [<ffffffffa026c26c>] ? e1000_clean+0x69/0x208 [e1000e]
> [ 4437.367264]  [<ffffffff8128d8fb>] ? net_rx_action+0xa4/0x1c0
> [ 4437.367268]  [<ffffffff8104c581>] ? __do_softirq+0xc0/0x188
> [ 4437.367272]  [<ffffffff81351fac>] ? call_softirq+0x1c/0x30
> [ 4437.367276]  [<ffffffff8100f98d>] ? do_softirq+0x3c/0x7b
> [ 4437.367278]  [<ffffffff8104c87c>] ? irq_exit+0x3d/0xa7
> [ 4437.367281]  [<ffffffff8100f6b4>] ? do_IRQ+0x81/0x97
> [ 4437.367285]  [<ffffffff8134ba2e>] ? common_interrupt+0x6e/0x6e
> [ 4437.367287]  <EOI>  [<ffffffffa008b32c>] ? dec128+0x434/0x80c [aes_x86_64]
> [ 4437.367307]  [<ffffffffa0085164>] ? crypt+0xae/0x101 [xts]
> [ 4437.367313]  [<ffffffffa008b712>] ? aes_decrypt+0xe/0xe [aes_x86_64]
> [ 4437.367320]  [<ffffffffa008b704>] ? dec128+0x80c/0x80c [aes_x86_64]
> [ 4437.367327]  [<ffffffffa00851f6>] ? decrypt+0x3f/0x44 [xts]
> [ 4437.367331]  [<ffffffff8118cdb3>] ? async_decrypt+0x37/0x3c
> [ 4437.367338]  [<ffffffffa0105e2a>] ? crypt_convert+0x22f/0x2c4 [dm_crypt]
> [ 4437.367342]  [<ffffffff8100d02f>] ? load_TLS+0x7/0xa
> [ 4437.367348]  [<ffffffffa01061b8>] ? kcryptd_crypt+0x56/0x342 [dm_crypt]
> [ 4437.367352]  [<ffffffff81038cd2>] ? finish_task_switch+0x86/0xb7
> [ 4437.367355]  [<ffffffff8103e0fa>] ? get_parent_ip+0x9/0x1b
> [ 4437.367358]  [<ffffffff8134e1d2>] ? sub_preempt_count+0x83/0x94
> [ 4437.367361]  [<ffffffff8103612b>] ? need_resched+0x1a/0x23
> [ 4437.367368]  [<ffffffffa0106162>] ? crypt_convert_init.isra.14+0x4f/0x4f [dm_crypt]
> [ 4437.367372]  [<ffffffff8105b867>] ? process_one_work+0x16d/0x298
> [ 4437.367375]  [<ffffffff8105c84a>] ? worker_thread+0xc2/0x145
> [ 4437.367378]  [<ffffffff8105c788>] ? manage_workers.isra.23+0x15b/0x15b
> [ 4437.367381]  [<ffffffff8105f9fe>] ? kthread+0x76/0x7e
> [ 4437.367384]  [<ffffffff81351eb4>] ? kernel_thread_helper+0x4/0x10
> [ 4437.367387]  [<ffffffff8105f988>] ? kthread_worker_fn+0x139/0x139
> [ 4437.367390]  [<ffffffff81351eb0>] ? gs_change+0x13/0x13
> [ 4437.367392] Mem-Info:
> [ 4437.367393] Node 0 DMA per-cpu:
> [ 4437.367396] CPU    0: hi:    0, btch:   1 usd:   0
> [ 4437.367397] CPU    1: hi:    0, btch:   1 usd:   0
> [ 4437.367399] Node 0 DMA32 per-cpu:
> [ 4437.367401] CPU    0: hi:  186, btch:  31 usd: 164
> [ 4437.367403] CPU    1: hi:  186, btch:  31 usd: 111
> [ 4437.367405] Node 0 Normal per-cpu:
> [ 4437.367407] CPU    0: hi:  186, btch:  31 usd: 114
> [ 4437.367409] CPU    1: hi:  186, btch:  31 usd: 158
> [ 4437.367413] active_anon:391300 inactive_anon:132951 isolated_anon:0
> [ 4437.367414]  active_file:136666 inactive_file:140710 isolated_file:31
> [ 4437.367415]  unevictable:1 dirty:3402 writeback:26688 unstable:7844
> [ 4437.367416]  free:36509 slab_reclaimable:85289 slab_unreclaimable:35524
> [ 4437.367417]  mapped:18088 shmem:35934 pagetables:9300 bounce:0
> [ 4437.367419] Node 0 DMA free:15712kB min:260kB low:324kB high:388kB active_anon:0kB inactive_anon:0kB active_file:0kB inactive_file:36kB unevictable:0kB isolated(anon):0kB isolated(file):0kB present:15684kB mlocked:0kB dirty:0kB writeback:36kB mapped:0kB shmem:0kB slab_reclaimable:0kB slab_unreclaimable:160kB kernel_stack:0kB pagetables:0kB unstable:0kB bounce:0kB writeback_tmp:0kB pages_scanned:40833 all_unreclaimable? yes
> [ 4437.367428] lowmem_reserve[]: 0 2960 3907 3907
> [ 4437.367432] Node 0 DMA32 free:110732kB min:51004kB low:63752kB high:76504kB active_anon:1380396kB inactive_anon:345140kB active_file:422008kB inactive_file:437440kB unevictable:4kB isolated(anon):0kB isolated(file):124kB present:3031688kB mlocked:4kB dirty:7148kB writeback:72004kB mapped:39424kB shmem:64836kB slab_reclaimable:212408kB slab_unreclaimable:80516kB kernel_stack:1720kB pagetables:19252kB unstable:23964kB bounce:0kB writeback_tmp:0kB pages_scanned:63 all_unreclaimable? no
> [ 4437.367442] lowmem_reserve[]: 0 0 946 946
> [ 4437.367445] Node 0 Normal free:19592kB min:16312kB low:20388kB high:24468kB active_anon:184804kB inactive_anon:186664kB active_file:124656kB inactive_file:125364kB unevictable:0kB isolated(anon):0kB isolated(file):0kB present:969600kB mlocked:0kB dirty:6460kB writeback:34712kB mapped:32928kB shmem:78900kB slab_reclaimable:128748kB slab_unreclaimable:61420kB kernel_stack:2792kB pagetables:17948kB unstable:7412kB bounce:0kB writeback_tmp:0kB pages_scanned:89 all_unreclaimable? no
> [ 4437.367455] lowmem_reserve[]: 0 0 0 0
> [ 4437.367458] Node 0 DMA: 2*4kB 1*8kB 1*16kB 0*32kB 1*64kB 2*128kB 2*256kB 1*512kB 2*1024kB 2*2048kB 2*4096kB = 15712kB
> [ 4437.367467] Node 0 DMA32: 25961*4kB 73*8kB 8*16kB 1*32kB 0*64kB 0*128kB 0*256kB 0*512kB 0*1024kB 1*2048kB 1*4096kB = 110732kB
> [ 4437.367475] Node 0 Normal: 4134*4kB 0*8kB 1*16kB 1*32kB 0*64kB 2*128kB 1*256kB 1*512kB 0*1024kB 1*2048kB 0*4096kB = 19656kB
> [ 4437.367484] 317456 total pagecache pages
> [ 4437.367485] 4042 pages in swap cache
> [ 4437.367487] Swap cache stats: add 31786, delete 27744, find 10282/11070
> [ 4437.367489] Free swap  = 4012560kB
> [ 4437.367490] Total swap = 4106248kB
> [ 4437.370978] 1032176 pages RAM
> [ 4437.370978] 42834 pages reserved
> [ 4437.370978] 390787 pages shared
> [ 4437.370978] 750687 pages non-shared
> 
> 
> [ 8640.516177] INFO: task flush-0:37:7122 blocked for more than 120 seconds.
> [ 8640.516182] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> [ 8640.516186] flush-0:37      D ffff88013bc93580     0  7122      2 0x00000080
> [ 8640.516192]  ffff880072c28810 0000000000000046 ffff880100000000 ffff880134f2e180
> [ 8640.516199]  0000000000013580 ffff88006d491fd8 ffff88006d491fd8 ffff880072c28810
> [ 8640.516205]  ffff88013bfd1c50 000000018134b58b ffff88010c3cc1b0 ffff88006d491d18
> [ 8640.516211] Call Trace:
> [ 8640.516221]  [<ffffffff8110e81a>] ? inode_owner_or_capable+0x36/0x36
> [ 8640.516226]  [<ffffffff8110e820>] ? inode_wait+0x6/0xa
> [ 8640.516232]  [<ffffffff8134a72c>] ? __wait_on_bit+0x3e/0x71
> [ 8640.516241]  [<ffffffff8103e0fa>] ? get_parent_ip+0x9/0x1b
> [ 8640.516245]  [<ffffffff81119674>] ? inode_wait_for_writeback+0xa2/0xc8
> [ 8640.516249]  [<ffffffff810600c9>] ? autoremove_wake_function+0x2a/0x2a
> [ 8640.516252]  [<ffffffff8111b4b4>] ? wb_writeback+0x226/0x255
> [ 8640.516255]  [<ffffffff8134e27d>] ? add_preempt_count+0x9a/0x9c
> [ 8640.516258]  [<ffffffff8111b8d4>] ? wb_do_writeback+0x150/0x1b2
> [ 8640.516261]  [<ffffffff8111b9c5>] ? bdi_writeback_thread+0x8f/0x204
> [ 8640.516264]  [<ffffffff8111b936>] ? wb_do_writeback+0x1b2/0x1b2
> [ 8640.516266]  [<ffffffff8105f9fe>] ? kthread+0x76/0x7e
> [ 8640.516270]  [<ffffffff81351eb4>] ? kernel_thread_helper+0x4/0x10
> [ 8640.516273]  [<ffffffff8105f988>] ? kthread_worker_fn+0x139/0x139
> [ 8640.516275]  [<ffffffff81351eb0>] ? gs_change+0x13/0x13
> [ 8640.516281] INFO: task cp:7568 blocked for more than 120 seconds.
> [ 8640.516283] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> [ 8640.516284] cp              D ffff88013bc13580     0  7568   6744 0x00000080
> [ 8640.516288]  ffff880123976750 0000000000000082 0000000000000000 ffffffff8160d020
> [ 8640.516292]  0000000000013580 ffff88001b3a9fd8 ffff88001b3a9fd8 ffff880123976750
> [ 8640.516295]  0000000000000001 0000000181066767 ffff880131463e50 ffff88013bc13e08
> [ 8640.516299] Call Trace:
> [ 8640.516303]  [<ffffffff810b5d03>] ? __lock_page+0x66/0x66
> [ 8640.516306]  [<ffffffff8134a2ec>] ? io_schedule+0x58/0x6f
> [ 8640.516308]  [<ffffffff810b5d09>] ? sleep_on_page+0x6/0xa
> [ 8640.516311]  [<ffffffff8134a72c>] ? __wait_on_bit+0x3e/0x71
> [ 8640.516313]  [<ffffffff810b5e51>] ? wait_on_page_bit+0x6e/0x73
> [ 8640.516316]  [<ffffffff810600c9>] ? autoremove_wake_function+0x2a/0x2a
> [ 8640.516319]  [<ffffffff810b5f29>] ? filemap_fdatawait_range+0x74/0x139
> [ 8640.516327]  [<ffffffff8111acab>] ? writeback_single_inode+0x155/0x2f4
> [ 8640.516330]  [<ffffffff8111ae94>] ? sync_inode+0x4a/0x6f
> [ 8640.516343]  [<ffffffffa06b9b02>] ? nfs_wb_all+0x39/0x3e [nfs]
> [ 8640.516351]  [<ffffffffa06aeed1>] ? nfs_setattr+0x8e/0xf6 [nfs]
> [ 8640.516354]  [<ffffffff811104c3>] ? notify_change+0x177/0x24f
> [ 8640.516357]  [<ffffffff8111e85c>] ? utimes_common+0x10c/0x135
> [ 8640.516361]  [<ffffffff810fd55a>] ? fget+0x50/0x57
> [ 8640.516364]  [<ffffffff8111e90f>] ? do_utimes+0x8a/0xd6
> [ 8640.516367]  [<ffffffff810fc7a2>] ? vfs_read+0x9f/0xe6
> [ 8640.516369]  [<ffffffff8111ea24>] ? sys_utimensat+0x64/0x6b
> [ 8640.516372]  [<ffffffff8134fd52>] ? system_call_fastpath+0x16/0x1b
> 
> 
> [ 9654.042164] ieee80211 phy0: failed to reallocate TX buffer
> [ 9654.042189] ieee80211 phy0: failed to reallocate TX buffer
> (120,000 lines of this)

-- 
"A mouse is a device used to point at the xterm you want to type in" - A.S.R.
Microsoft is to operating systems ....
                                      .... what McDonalds is to gourmet cooking
Home page: http://marc.merlins.org/  

^ permalink raw reply

* [PATCH] gianfar: add missing include
From: Michael Neuling @ 2012-04-10  4:18 UTC (permalink / raw)
  To: David S. Miller, Richard Cochran; +Cc: linuxppc-dev, netdev, linux-next

next-20120405 compiled with mpc85xx_defconfig gives the following:

  CC      drivers/net/ethernet/freescale/gianfar_ethtool.o
drivers/net/ethernet/freescale/gianfar_ethtool.c: In function 'gfar_get_ts_info':
drivers/net/ethernet/freescale/gianfar_ethtool.c:1751:4: error: 'SOF_TIMESTAMPING_RX_SOFTWARE' undeclared (first use in this function)
drivers/net/ethernet/freescale/gianfar_ethtool.c:1751:4: note: each undeclared identifier is reported only once for each function it appears in
drivers/net/ethernet/freescale/gianfar_ethtool.c:1752:4: error: 'SOF_TIMESTAMPING_SOFTWARE' undeclared (first use in this function)
drivers/net/ethernet/freescale/gianfar_ethtool.c:1757:3: error: 'SOF_TIMESTAMPING_TX_HARDWARE' undeclared (first use in this function)
drivers/net/ethernet/freescale/gianfar_ethtool.c:1758:3: error: 'SOF_TIMESTAMPING_RX_HARDWARE' undeclared (first use in this function)
drivers/net/ethernet/freescale/gianfar_ethtool.c:1759:3: error: 'SOF_TIMESTAMPING_RAW_HARDWARE' undeclared (first use in this function)
drivers/net/ethernet/freescale/gianfar_ethtool.c:1762:9: error: 'HWTSTAMP_TX_OFF' undeclared (first use in this function)
drivers/net/ethernet/freescale/gianfar_ethtool.c:1763:9: error: 'HWTSTAMP_TX_ON' undeclared (first use in this function)
drivers/net/ethernet/freescale/gianfar_ethtool.c:1765:9: error: 'HWTSTAMP_FILTER_NONE' undeclared (first use in this function)
drivers/net/ethernet/freescale/gianfar_ethtool.c:1766:9: error: 'HWTSTAMP_FILTER_ALL' undeclared (first use in this function)

This is because of a missing include file from:
  6663628 gianfar: Support the get_ts_info ethtool method.

Signed-off-by: Michael Neuling <mikey@neuling.org>
CC: Richard Cochran <richardcochran@gmail.com>
CC: David S. Miller <davem@davemloft.net>

diff --git a/drivers/net/ethernet/freescale/gianfar_ethtool.c b/drivers/net/ethernet/freescale/gianfar_ethtool.c
index 27f49c7..3c34b32b 100644
--- a/drivers/net/ethernet/freescale/gianfar_ethtool.c
+++ b/drivers/net/ethernet/freescale/gianfar_ethtool.c
@@ -29,6 +29,7 @@
 #include <linux/skbuff.h>
 #include <linux/spinlock.h>
 #include <linux/mm.h>
+#include <linux/net_tstamp.h>
 
 #include <asm/io.h>
 #include <asm/irq.h>

^ permalink raw reply related

* Re: [PATCH] memcg/tcp: fix warning caused b res->usage go to negative.
From: KAMEZAWA Hiroyuki @ 2012-04-10  4:15 UTC (permalink / raw)
  To: Glauber Costa; +Cc: netdev, David Miller, Andrew Morton
In-Reply-To: <4F83A29D.1060402@parallels.com>

(2012/04/10 12:01), Glauber Costa wrote:

> On 04/09/2012 11:51 PM, Glauber Costa wrote:
>> On 04/09/2012 11:37 PM, KAMEZAWA Hiroyuki wrote:
>>> Hm. What happens in following sequence ?
>>>
>>>     1. a memcg is created
>>>     2. put a task into the memcg, start tcp steam
>>>     3. set tcp memory limit
>>>
>>> The resource used between 2 and 3 will cause the problem finally.
>>
>> I don't get it. if a task is in memcg, but no limit is set,
>> that socket will be assigned null memcg, and will stay like that
>> forever. Only new sockets will have the new memcg pointer.
>>
>> And previously, we could have the memcg pointer alive, but the jump
>> labels to be disabled. With the patch I posted, this can't happen
>> anymore, since the jump labels are guaranteed to live throughout the
>> whole socket life.
>>
>>> Then, Dave's request
>>> ==
>>> You must either:
>>>
>>> 1) Integrate the socket's existing usage when the limit is set.
>>>
>>> 2) Avoid accounting completely for a socket that started before
>>>      the limit was set.
>>> ==
>>> are not satisfied. So, we need to have a state per sockets, it's accounted
>>> or not. I'll look into this problem again, today.
>>>
>>
>> Of course they are.
>>
>> Every socket created before we set the limit is not accounted.
>> This is 2) that Dave mentioned, and it was *always* this way.
>>
>> The problem here was the opposite: You could disable the jump labels
>> with sockets still in flight, because we were disabling it based on
>> the limit being set back to unlimited.
>>
>> What this patch does, is defer that until the last socket limited dies.
>>
> 
> Okay, there is an additional thing to be considered here:
> 
> Due to the nature of how jump label works, once they are enabled for one
> of the cgroups, they will be enabled for all of them. So the patch I
> sent may still break in some scenarios because of the way we record that
> the limit was set.
> 
> However, if my theory behind what is causing the problem is correct,
> this patch should fix the issue for you. 


Now, our issue is leak of accounting, regardless of warning.

> Let me know if it does, and

> I'll work on the final solution.
> 


The problem is that jump_label updating is not atomic_ops.

I'm _not_ sure the update order of the jump_label in sock_update_memcg()
and other jump instructions inserted at accounting.

For example, if the jump instruction in sock_update_memcg() is updated 1st
and others are updated later, it's unclear whether sockets which has _valid_
sock->sk_cgrp will be accounted or not because accounting jump instruction
may not be updated yet.

Hopefully, label in sock_update_memcg should be updated last...

Hm. If I do, I'll add one more key as:

atomic_t	sock_should_memcg_aware;

And update 2 keys in following order.

At enable
	static_key_slow_inc(&memcg_socket_limit_enabled)
	atomic_inc(&sock_should_memcg_aware);

At disable
	atomic_dec(&sock_should_memcg_aware);
	static_key_slow_dec(&memcg_socket_limit_enabled)

And
==
void sock_update_memcg(struct sock *sk)
{
        if (atomic_read(&sock_should_memcg_aware)) {

==


Thanks,
-Kame

^ permalink raw reply

* Re: 3.2.8/amd64 full interrupt hangs and deadlocks under big network copies (page allocation failure)
From: Eric Dumazet @ 2012-04-10  3:56 UTC (permalink / raw)
  To: David Miller; +Cc: Larry.Finger, marc, bhutchings, linux-wireless, netdev
In-Reply-To: <20120409.153452.1284163346306246866.davem@davemloft.net>

On Mon, 2012-04-09 at 15:34 -0400, David Miller wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Mon, 09 Apr 2012 21:11:12 +0200
> 
> > I think Marc posted stack traces showing problem on transmit side.
>  ...
> > I dont really understand how it can happen, with MTU=1500
> 
> Depending upon the configuration and the driver, wireless can need
> more headroom.  For encryption an extra 8 bytes are necessary, and the
> driver may request a variable amount of extra headroom via
> ->hw.extra_tx_headroom
> 
> What wireless device are we dealing with again?

Problem seems related to tailroom needed by mac80211
(IEEE80211_ENCRYPT_TAILROOM = 18 bytes)

So we must reallocate skb->head, thats impressive nobody cares.

[ 3007.249687] ieee80211_skb_resize(skb=ffff8802329846e8) cloned=1 head_need=0 tail_need=18 skb->len=1494 ksize=4096 tailroom=0 headroom=2282
[ 3007.249693] ieee80211_skb_resize(skb=ffff8802329846e8) cloned=0 head_need=0 tail_need=0 skb->len=1526 ksize=8192 tailroom=64 headroom=2250

Ouch... skb_tailroom() seems wrong ... it seems pskb_expand_head() is really suboptimal.

It appears tcp_sendmsg() tries to fill skb completely, with no available tailroom :

                        if (skb_tailroom(skb) > 0) {
                                /* We have some space in skb head. Superb! */
                                if (copy > skb_tailroom(skb))
                                        copy = skb_tailroom(skb);
                                err = skb_add_data_nocache(sk, skb, from, copy);
                                if (err)
                                        goto do_fault;
                        } else {


Shouldnt we take into account dev->needed_tailroom ?

I'll submit a pskb_expand_head() fix asap.

^ permalink raw reply

* Re: [PATCH] memcg/tcp: fix warning caused b res->usage go to negative.
From: KAMEZAWA Hiroyuki @ 2012-04-10  3:21 UTC (permalink / raw)
  To: Glauber Costa; +Cc: netdev, David Miller, Andrew Morton
In-Reply-To: <4F83A022.1000701@parallels.com>

(2012/04/10 11:51), Glauber Costa wrote:

> On 04/09/2012 11:37 PM, KAMEZAWA Hiroyuki wrote:
>> Hm. What happens in following sequence ?
>>
>>    1. a memcg is created
>>    2. put a task into the memcg, start tcp steam
>>    3. set tcp memory limit
>>
>> The resource used between 2 and 3 will cause the problem finally.
> 
> I don't get it. if a task is in memcg, but no limit is set,
> that socket will be assigned null memcg, and will stay like that
> forever. Only new sockets will have the new memcg pointer.
> 
> And previously, we could have the memcg pointer alive, but the jump
> labels to be disabled. With the patch I posted, this can't happen
> anymore, since the jump labels are guaranteed to live throughout the
> whole socket life.
> 
>> Then, Dave's request
>> ==
>> You must either:
>>
>> 1) Integrate the socket's existing usage when the limit is set.
>>
>> 2) Avoid accounting completely for a socket that started before
>>     the limit was set.
>> ==
>> are not satisfied. So, we need to have a state per sockets, it's accounted
>> or not. I'll look into this problem again, today.
>>
> 
> Of course they are.
> 
> Every socket created before we set the limit is not accounted.
> This is 2) that Dave mentioned, and it was *always* this way.
> 
> The problem here was the opposite: You could disable the jump labels
> with sockets still in flight, because we were disabling it based on
> the limit being set back to unlimited.
> 
> What this patch does, is defer that until the last socket limited dies.
> 

Thank you for explanation. Hmm, sk->cgrp check ?
Ah, yes it's updated by sock_update_memcg() under jump_label, which is
called by tcp_v4_init_sock().
Hm. and jump_label()'s atomic counter and mutex_lock will be a guard against
set/unset race. Ok.

BTW, what will happen in following case ?

Assume that the last memcg is destroyed and call jump_label_dec. And the
thread waits for jump_label_mutex for a while.

      CPU A                                           CPU B

    jump_label_dec() # mutex will be held        sock_update_memcg() is called
                                                 sk_cgrp is set.
    ...modify instructions                       some accounting is done.
    mutex_unlock()

I wonder you need some serialization somewhere OR disallow turning off accounting.                                              

Thanks,
-Kame

^ permalink raw reply

* Re: [PATCH] memcg/tcp: fix warning caused b res->usage go to negative.
From: Glauber Costa @ 2012-04-10  3:01 UTC (permalink / raw)
  To: KAMEZAWA Hiroyuki; +Cc: netdev, David Miller, Andrew Morton
In-Reply-To: <4F83A022.1000701@parallels.com>

On 04/09/2012 11:51 PM, Glauber Costa wrote:
> On 04/09/2012 11:37 PM, KAMEZAWA Hiroyuki wrote:
>> Hm. What happens in following sequence ?
>>
>>     1. a memcg is created
>>     2. put a task into the memcg, start tcp steam
>>     3. set tcp memory limit
>>
>> The resource used between 2 and 3 will cause the problem finally.
> 
> I don't get it. if a task is in memcg, but no limit is set,
> that socket will be assigned null memcg, and will stay like that
> forever. Only new sockets will have the new memcg pointer.
> 
> And previously, we could have the memcg pointer alive, but the jump
> labels to be disabled. With the patch I posted, this can't happen
> anymore, since the jump labels are guaranteed to live throughout the
> whole socket life.
> 
>> Then, Dave's request
>> ==
>> You must either:
>>
>> 1) Integrate the socket's existing usage when the limit is set.
>>
>> 2) Avoid accounting completely for a socket that started before
>>      the limit was set.
>> ==
>> are not satisfied. So, we need to have a state per sockets, it's accounted
>> or not. I'll look into this problem again, today.
>>
> 
> Of course they are.
> 
> Every socket created before we set the limit is not accounted.
> This is 2) that Dave mentioned, and it was *always* this way.
> 
> The problem here was the opposite: You could disable the jump labels
> with sockets still in flight, because we were disabling it based on
> the limit being set back to unlimited.
> 
> What this patch does, is defer that until the last socket limited dies.
> 

Okay, there is an additional thing to be considered here:

Due to the nature of how jump label works, once they are enabled for one
of the cgroups, they will be enabled for all of them. So the patch I
sent may still break in some scenarios because of the way we record that
the limit was set.

However, if my theory behind what is causing the problem is correct,
this patch should fix the issue for you. Let me know if it does, and
I'll work on the final solution.

^ 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