Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH 1/2] net: Add skb_unclone() helper function.
From: Pravin Shelar @ 2013-01-25  1:36 UTC (permalink / raw)
  To: Michał Mirosław; +Cc: netdev, jesse, eric.dumazet
In-Reply-To: <CAHXqBF+uHoGRXP3HdiL2Zm9RUhT-+7RsrRhAYZyEg-FjzH2-eQ@mail.gmail.com>

On Thu, Jan 24, 2013 at 5:21 PM, Michał Mirosław <mirqus@gmail.com> wrote:
> 2013/1/24 Pravin B Shelar <pshelar@nicira.com>:
> [...]
>> --- a/include/linux/skbuff.h
>> +++ b/include/linux/skbuff.h
>> @@ -798,6 +798,19 @@ static inline int skb_cloned(const struct sk_buff *skb)
>>  }
>>
>>  /**
>> + *     skb_unclone - creates separate copy if skb is cloned.
>> + */
>> +static inline bool skb_unclone(struct sk_buff *skb, gfp_t pri)
>> +{
>> +       might_sleep_if(pri & __GFP_WAIT);
>> +
>> +       if (skb_cloned(skb))
>> +               return pskb_expand_head(skb, 0, 0, pri);
>> +
>> +       return 0;
>> +}
>
> This should return int. pskb_expand_head() returns 0 or -ENOMEM.
>
> [...]
>> diff --git a/net/ipv4/xfrm4_mode_tunnel.c b/net/ipv4/xfrm4_mode_tunnel.c
>> index ddee0a0..c1f00ef 100644
>> --- a/net/ipv4/xfrm4_mode_tunnel.c
>> +++ b/net/ipv4/xfrm4_mode_tunnel.c
>> @@ -142,8 +142,7 @@ static int xfrm4_mode_tunnel_input(struct xfrm_state *x, struct sk_buff *skb)
>>         for_each_input_rcu(rcv_notify_handlers, handler)
>>                 handler->handler(skb);
>>
>> -       if (skb_cloned(skb) &&
>> -           (err = pskb_expand_head(skb, 0, 0, GFP_ATOMIC)))
>> +       if (skb_unclone(skb, GFP_ATOMIC))
>>                 goto out;
>
> And here return -ENOMEM is replaced with return -EINVAL because of this.
>
ok, I will change skb_unclone return to int.

Thanks.

^ permalink raw reply

* Re: [PATCH 2/2 net-next] doc: dt: fsl: fec: add napi optional properties
From: Frank Li @ 2013-01-25  1:46 UTC (permalink / raw)
  To: David Miller
  Cc: Frank.Li, shawn.guo, B38611, linux-arm-kernel, netdev, bhutchings,
	s.hauer
In-Reply-To: <20130124.181800.580308569081621879.davem@davemloft.net>

2013/1/25 David Miller <davem@davemloft.net>:
> From: Frank Li <Frank.Li@freescale.com>
> Date: Thu, 24 Jan 2013 15:06:59 +0800
>
>> Signed-off-by: Frank Li <Frank.Li@freescale.com>
>
> For the thousandth time, you should not make NAPI run time
> selectable, this is non-negotable.

Sorry, this patch send by accident.
Please forget it.

Best regards
Frank Li

^ permalink raw reply

* Re: [PATCH v3 1/1 net-next] net: fec: enable pause frame to improve rx prefomance for 1G network
From: Frank Li @ 2013-01-25  1:50 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: B38611, netdev, s.hauer, Frank Li, shawn.guo, davem,
	linux-arm-kernel
In-Reply-To: <1359046173.2736.2.camel@bwh-desktop.uk.solarflarecom.com>

2013/1/25 Ben Hutchings <bhutchings@solarflare.com>:
> On Thu, 2013-01-24 at 10:16 +0800, Frank Li wrote:
>> 2013/1/24 Ben Hutchings <bhutchings@solarflare.com>:
>> > On Thu, 2013-01-17 at 10:55 +0800, Frank Li wrote:
>> >> The limition of imx6 internal bus cause fec can't achieve 1G perfomance.
>> >> There will be many packages lost because FIFO over run.
>> >>
>> >> This patch enable pause frame flow control.
>> > [...]
>> >> --- a/drivers/net/ethernet/freescale/fec.c
>> >> +++ b/drivers/net/ethernet/freescale/fec.c
>> > [...]
>> >> +static int fec_enet_set_pauseparam(struct net_device *ndev,
>> >> +                                struct ethtool_pauseparam *pause)
>> >> +{
>> >> +     struct fec_enet_private *fep = netdev_priv(ndev);
>> >> +
>> >> +     if (pause->tx_pause != pause->rx_pause) {
>> >> +             netdev_info(ndev,
>> >> +                     "hardware only support enable/disable both tx and rx");
>> >> +             return -EINVAL;
>> >> +     }
>> >> +
>> >> +     fep->pause_flag = 0;
>> >> +
>> >> +     /* tx pause must be same as rx pause */
>> >> +     fep->pause_flag |= pause->rx_pause ? FEC_PAUSE_FLAG_ENABLE : 0;
>> >> +     fep->pause_flag |= pause->autoneg ? FEC_PAUSE_FLAG_AUTONEG : 0;
>> >> +
>> >> +     if (pause->rx_pause || pause->autoneg) {
>> >> +             fep->phy_dev->supported |= ADVERTISED_Pause;
>> >> +             fep->phy_dev->advertising |= ADVERTISED_Pause;
>> >> +     } else {
>> >> +             fep->phy_dev->supported &= ~ADVERTISED_Pause;
>> >> +             fep->phy_dev->advertising &= ~ADVERTISED_Pause;
>> >> +     }
>> > [...]
>> >
>> > Why is this changing the supported flags, i.e. device capabilities?  You
>> > need to leave those flags alone and reject an attempt to enable pause
>> > frames on a device that doesn't support them.
>>
>> I go through phylib, I have not found good place set ADVERTISED_Pause
>> capabilities.
>> genphy_config_init never check Pause capabilities.
>
> I agree that phylib can't initialise pause capabilities because those
> depend on the MAC.  But look at which function I'm quoting: this is the
> ethtool operation, which shouldn't change capabilities.

Where is good place do you think? in probe function?

>
> Ben.
>
> --
> Ben Hutchings, Staff Engineer, Solarflare
> Not speaking for my employer; that's the marketing department's job.
> They asked us to note that Solarflare product names are trademarked.
>

^ permalink raw reply

* Re: [PATCH v2 1/1 net-next] net: fec: add napi support to improve proformance
From: Frank Li @ 2013-01-25  2:02 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Frank Li, shawn.guo, B38611, davem, linux-arm-kernel, netdev,
	s.hauer
In-Reply-To: <1359035768.12374.2083.camel@edumazet-glaptop>

2013/1/24 Eric Dumazet <eric.dumazet@gmail.com>:
> On Thu, 2013-01-24 at 15:58 +0800, Frank Li wrote:
>>
>>  static irqreturn_t
>> @@ -805,6 +823,7 @@ fec_enet_interrupt(int irq, void *dev_id)
>>       struct net_device *ndev = dev_id;
>>       struct fec_enet_private *fep = netdev_priv(ndev);
>>       uint int_events;
>> +     ulong flags;
>>       irqreturn_t ret = IRQ_NONE;
>>
>>       do {
>> @@ -813,7 +832,14 @@ fec_enet_interrupt(int irq, void *dev_id)
>>
>>               if (int_events & FEC_ENET_RXF) {
>>                       ret = IRQ_HANDLED;
>> -                     fec_enet_rx(ndev);
>> +
>> +                     spin_lock_irqsave(&fep->hw_lock, flags);
>
> You already are in a irq safe context, spin_lock() should be ok

Okay, I will fix it.

>
>> +                     /* Disable the RX interrupt */
>> +                     if (napi_schedule_prep(&fep->napi)) {
>> +                             fec_enet_rx_int_is_enabled(ndev, false);
>> +                             __napi_schedule(&fep->napi);
>> +                     }
>> +                     spin_unlock_irqrestore(&fep->hw_lock, flags);
>
>        spin_unlock();
>
>>               }
>>
>
>
> Same remark for the spin_lock_irqsave(&fep->tmreg_lock, flags) in
> fec_enet_tx()
>

Okay, but it is not related with NAPI support.
Need new patch for that.

>
>

^ permalink raw reply

* Re: [PATCH] vhost-net: fall back to vmalloc if high-order allocation fails
From: Cong Wang @ 2013-01-25  3:03 UTC (permalink / raw)
  To: kvm; +Cc: netdev, linux-kernel
In-Reply-To: <87k3r31vbc.fsf@silenus.orebokech.com>

["Followup-To:" header set to gmane.linux.network.]
On Wed, 23 Jan 2013 at 20:46 GMT, Romain Francoise <romain@orebokech.com> wrote:
> Creating a vhost-net device allocates an object large enough (34320 bytes
> on x86-64) to trigger an order-4 allocation, which may fail if memory if
> fragmented:
>
>  libvirtd: page allocation failure: order:4, mode:0x2000d0
>  ...
>  SLAB: Unable to allocate memory on node 0 (gfp=0xd0)
>    cache: size-65536, object size: 65536, order: 4
>    node 0: slabs: 8/8, objs: 8/8, free: 0
>
> In that situation, rather than forcing the caller to use regular
> virtio-net, try to allocate the descriptor with vmalloc().
>


The real problem is vhost_net struct is really big, it
should be reduced rather than workarounded like this.


> +static void vhost_net_kvfree(void *addr)
> +{
> +	if (is_vmalloc_addr(addr))
> +		vfree(addr);
> +	else
> +		kfree(addr);
> +}
> +

This kind of stuff should really go to mm, not netdev.


^ permalink raw reply

* Re: [PATCH V6 1/3] virtio-net: fix the set affinity bug when CPU IDs are not consecutive
From: Jason Wang @ 2013-01-25  3:26 UTC (permalink / raw)
  To: Wanlong Gao
  Cc: Michael S. Tsirkin, netdev, linux-kernel, virtualization,
	Eric Dumazet
In-Reply-To: <1358767524-17934-1-git-send-email-gaowanlong@cn.fujitsu.com>

On 01/21/2013 07:25 PM, Wanlong Gao wrote:
> As Michael mentioned, set affinity and select queue will not work very
> well when CPU IDs are not consecutive, this can happen with hot unplug.
> Fix this bug by traversal the online CPUs, and create a per cpu variable
> to find the mapping from CPU to the preferable virtual-queue.
>
> Cc: Rusty Russell <rusty@rustcorp.com.au>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Jason Wang <jasowang@redhat.com>
> Cc: Eric Dumazet <erdnetdev@gmail.com>
> Cc: virtualization@lists.linux-foundation.org
> Cc: netdev@vger.kernel.org
> Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
> ---
> V5->V6:
> 	remove {get|put}_online_cpus from virtnet_del_vqs (Jason)
> V4->V5:
> 	Add get/put_online_cpus to avoid CPUs go up and down during operations (Rusty)
>
> V3->V4:
> 	move vq_index into virtnet_info	(Jason)
> 	change the mapping value when not setting affinity (Jason)
> 	address the comments about select_queue (Rusty)
>
>  drivers/net/virtio_net.c | 58 +++++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 47 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index a6fcf15..70cd957 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -123,6 +123,9 @@ struct virtnet_info {
>  
>  	/* Does the affinity hint is set for virtqueues? */
>  	bool affinity_hint_set;
> +
> +	/* Per-cpu variable to show the mapping from CPU to virtqueue */
> +	int __percpu *vq_index;
>  };
>  
>  struct skb_vnet_hdr {
> @@ -1016,6 +1019,7 @@ static int virtnet_vlan_rx_kill_vid(struct net_device *dev, u16 vid)
>  static void virtnet_set_affinity(struct virtnet_info *vi, bool set)
>  {
>  	int i;
> +	int cpu;
>  
>  	/* In multiqueue mode, when the number of cpu is equal to the number of
>  	 * queue pairs, we let the queue pairs to be private to one cpu by
> @@ -1029,16 +1033,29 @@ static void virtnet_set_affinity(struct virtnet_info *vi, bool set)
>  			return;
>  	}
>  
> -	for (i = 0; i < vi->max_queue_pairs; i++) {
> -		int cpu = set ? i : -1;
> -		virtqueue_set_affinity(vi->rq[i].vq, cpu);
> -		virtqueue_set_affinity(vi->sq[i].vq, cpu);
> -	}
> +	if (set) {
> +		i = 0;
> +		for_each_online_cpu(cpu) {
> +			virtqueue_set_affinity(vi->rq[i].vq, cpu);
> +			virtqueue_set_affinity(vi->sq[i].vq, cpu);
> +			*per_cpu_ptr(vi->vq_index, cpu) = i;
> +			i++;
> +		}
>  
> -	if (set)
>  		vi->affinity_hint_set = true;
> -	else
> +	} else {
> +		for(i = 0; i < vi->max_queue_pairs; i++) {
> +			virtqueue_set_affinity(vi->rq[i].vq, -1);
> +			virtqueue_set_affinity(vi->sq[i].vq, -1);
> +		}
> +
> +		i = 0;
> +		for_each_online_cpu(cpu)
> +			*per_cpu_ptr(vi->vq_index, cpu) =
> +				++i % vi->curr_queue_pairs;
> +
>  		vi->affinity_hint_set = false;
> +	}
>  }

This looks wrong, since you always choose txq based on the per-cpu, I
think the per-cpu index should be set unconditionally even if cpus !=
queues. Consider you may boot a guest with 4 vcpus and 2 queues, you
need initialize it also in this case. Otherwise, you may always get txq
0 to be selected.
>  
>  static void virtnet_get_ringparam(struct net_device *dev,
> @@ -1087,7 +1104,9 @@ static int virtnet_set_channels(struct net_device *dev,
>  		netif_set_real_num_tx_queues(dev, queue_pairs);
>  		netif_set_real_num_rx_queues(dev, queue_pairs);
>  
> +		get_online_cpus();
>  		virtnet_set_affinity(vi, true);
> +		put_online_cpus();
>  	}
>  
>  	return err;
> @@ -1127,12 +1146,19 @@ static int virtnet_change_mtu(struct net_device *dev, int new_mtu)
>  
>  /* To avoid contending a lock hold by a vcpu who would exit to host, select the
>   * txq based on the processor id.
> - * TODO: handle cpu hotplug.
>   */
>  static u16 virtnet_select_queue(struct net_device *dev, struct sk_buff *skb)
>  {
> -	int txq = skb_rx_queue_recorded(skb) ? skb_get_rx_queue(skb) :
> -		  smp_processor_id();
> +	int txq;
> +	struct virtnet_info *vi = netdev_priv(dev);
> +
> +	if (skb_rx_queue_recorded(skb)) {
> +		txq = skb_get_rx_queue(skb);
> +	} else {
> +		txq = *__this_cpu_ptr(vi->vq_index);
> +		if (txq == -1)
> +			txq = 0;
> +	}
>  
>  	while (unlikely(txq >= dev->real_num_tx_queues))
>  		txq -= dev->real_num_tx_queues;
> @@ -1371,7 +1397,10 @@ static int init_vqs(struct virtnet_info *vi)
>  	if (ret)
>  		goto err_free;
>  
> +	get_online_cpus();
>  	virtnet_set_affinity(vi, true);
> +	put_online_cpus();
> +
>  	return 0;
>  
>  err_free:
> @@ -1453,6 +1482,10 @@ static int virtnet_probe(struct virtio_device *vdev)
>  	if (vi->stats == NULL)
>  		goto free;
>  
> +	vi->vq_index = alloc_percpu(int);
> +	if (vi->vq_index == NULL)
> +		goto free_stats;
> +
>  	mutex_init(&vi->config_lock);
>  	vi->config_enable = true;
>  	INIT_WORK(&vi->config_work, virtnet_config_changed_work);
> @@ -1476,7 +1509,7 @@ static int virtnet_probe(struct virtio_device *vdev)
>  	/* Allocate/initialize the rx/tx queues, and invoke find_vqs */
>  	err = init_vqs(vi);
>  	if (err)
> -		goto free_stats;
> +		goto free_index;
>  
>  	netif_set_real_num_tx_queues(dev, 1);
>  	netif_set_real_num_rx_queues(dev, 1);
> @@ -1520,6 +1553,8 @@ free_recv_bufs:
>  free_vqs:
>  	cancel_delayed_work_sync(&vi->refill);
>  	virtnet_del_vqs(vi);
> +free_index:
> +	free_percpu(vi->vq_index);
>  free_stats:
>  	free_percpu(vi->stats);
>  free:
> @@ -1554,6 +1589,7 @@ static void virtnet_remove(struct virtio_device *vdev)
>  
>  	flush_work(&vi->config_work);
>  
> +	free_percpu(vi->vq_index);
>  	free_percpu(vi->stats);
>  	free_netdev(vi->dev);
>  }

^ permalink raw reply

* Re: [PATCH V6 2/3] virtio-net: split out clean affinity function
From: Jason Wang @ 2013-01-25  3:28 UTC (permalink / raw)
  To: Wanlong Gao
  Cc: Michael S. Tsirkin, netdev, linux-kernel, virtualization,
	Eric Dumazet
In-Reply-To: <1358767524-17934-2-git-send-email-gaowanlong@cn.fujitsu.com>

On 01/21/2013 07:25 PM, Wanlong Gao wrote:
> Split out the clean affinity function to virtnet_clean_affinity().
>
> Cc: Rusty Russell <rusty@rustcorp.com.au>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Jason Wang <jasowang@redhat.com>
> Cc: Eric Dumazet <erdnetdev@gmail.com>
> Cc: virtualization@lists.linux-foundation.org
> Cc: netdev@vger.kernel.org
> Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
> ---
> V5->V6: NEW
>
>  drivers/net/virtio_net.c | 67 +++++++++++++++++++++++++++---------------------
>  1 file changed, 38 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 70cd957..1a35a8c 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -1016,48 +1016,57 @@ static int virtnet_vlan_rx_kill_vid(struct net_device *dev, u16 vid)
>  	return 0;
>  }
>  
> -static void virtnet_set_affinity(struct virtnet_info *vi, bool set)
> +static void virtnet_clean_affinity(struct virtnet_info *vi, long hcpu)
>  {
>  	int i;
>  	int cpu;
>  
> -	/* In multiqueue mode, when the number of cpu is equal to the number of
> -	 * queue pairs, we let the queue pairs to be private to one cpu by
> -	 * setting the affinity hint to eliminate the contention.
> -	 */
> -	if ((vi->curr_queue_pairs == 1 ||
> -	     vi->max_queue_pairs != num_online_cpus()) && set) {
> -		if (vi->affinity_hint_set)
> -			set = false;
> -		else
> -			return;
> -	}
> -
> -	if (set) {
> -		i = 0;
> -		for_each_online_cpu(cpu) {
> -			virtqueue_/set_affinity(vi->rq[i].vq, cpu);
> -			virtqueue_set_affinity(vi->sq[i].vq, cpu);
> -			*per_cpu_ptr(vi->vq_index, cpu) = i;
> -			i++;
> -		}
> -
> -		vi->affinity_hint_set = true;
> -	} else {
> -		for(i = 0; i < vi->max_queue_pairs; i++) {
> +	if (vi->affinity_hint_set) {
> +		for (i = 0; i < vi->max_queue_pairs; i++) {
>  			virtqueue_set_affinity(vi->rq[i].vq, -1);
>  			virtqueue_set_affinity(vi->sq[i].vq, -1);
>  		}
>  
>  		i = 0;
> -		for_each_online_cpu(cpu)
> +		for_each_online_cpu(cpu) {
> +			if (cpu == hcpu)
> +				continue;
>  			*per_cpu_ptr(vi->vq_index, cpu) =
>  				++i % vi->curr_queue_pairs;
> +		}
>  

Some questions here:

- Did we need reset the affinity of the queue here like the this?

virtqueue_set_affinity(vi->sq[*per_cpu_ptr(vi->vq_index, hcpu)], -1);
virtqueue_set_affinity(vi->rq[*per_cpu_ptr(vi->vq_index, hcpu)], -1);

- Looks like we need also reset the percpu index when
vi->affinity_hint_set is false.
- Does this really need this reset? Consider we're going to reset the
percpu in CPU_DEAD?

Thanks
>  		vi->affinity_hint_set = false;
>  	}
>  }
>  
> +static void virtnet_set_affinity(struct virtnet_info *vi)
> +{
> +	int i;
> +	int cpu;
> +
> +	/* In multiqueue mode, when the number of cpu is equal to the number of
> +	 * queue pairs, we let the queue pairs to be private to one cpu by
> +	 * setting the affinity hint to eliminate the contention.
> +	 */
> +	if (vi->curr_queue_pairs == 1 ||
> +	    vi->max_queue_pairs != num_online_cpus()) {
> +		if (vi->affinity_hint_set)
> +			virtnet_clean_affinity(vi, -1);
> +		else
> +			return;
> +	}
> +
> +	i = 0;
> +	for_each_online_cpu(cpu) {
> +		virtqueue_set_affinity(vi->rq[i].vq, cpu);
> +		virtqueue_set_affinity(vi->sq[i].vq, cpu);
> +		*per_cpu_ptr(vi->vq_index, cpu) = i;
> +		i++;
> +	}
> +
> +	vi->affinity_hint_set = true;
> +}
> +
>  static void virtnet_get_ringparam(struct net_device *dev,
>  				struct ethtool_ringparam *ring)
>  {
> @@ -1105,7 +1114,7 @@ static int virtnet_set_channels(struct net_device *dev,
>  		netif_set_real_num_rx_queues(dev, queue_pairs);
>  
>  		get_online_cpus();
> -		virtnet_set_affinity(vi, true);
> +		virtnet_set_affinity(vi);
>  		put_online_cpus();
>  	}
>  
> @@ -1274,7 +1283,7 @@ static void virtnet_del_vqs(struct virtnet_info *vi)
>  {
>  	struct virtio_device *vdev = vi->vdev;
>  
> -	virtnet_set_affinity(vi, false);
> +	virtnet_clean_affinity(vi, -1);
>  
>  	vdev->config->del_vqs(vdev);
>  
> @@ -1398,7 +1407,7 @@ static int init_vqs(struct virtnet_info *vi)
>  		goto err_free;
>  
>  	get_online_cpus();
> -	virtnet_set_affinity(vi, true);
> +	virtnet_set_affinity(vi);
>  	put_online_cpus();
>  
>  	return 0;

^ permalink raw reply

* Re: [PATCH 2/2] v2 GRE: Add segmentation offload for GRE
From: Pravin Shelar @ 2013-01-25  3:38 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: netdev, jesse
In-Reply-To: <1359077664.12374.2887.camel@edumazet-glaptop>

On Thu, Jan 24, 2013 at 5:34 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Thu, 2013-01-24 at 16:14 -0800, Pravin Shelar wrote:
>
>> I think this patch does fix csum issue without causing any performance
>> regression. So this patch shld be enough to solve GRE-GSO issue. Once
>> you have fix, this code can be optimized even more.
>
> It adds the extra copy, since you assume no SG capability so
> skb_segment() _does_ a copy.
>
OK,I wil use device features.

> As the checksum is needed, its true the copy is almost not noticed,
> but one day NIC will be able to perform the checksum for us.
> (Maybe its already the case for some of them)
>
> I would first fix the checksum issue in a generic way, then
> apply this patch on top of the fix, so that we can use SG and avoid
> the extra copy for the typical tcp_sendmsg()

I thought you were working on the fix. If not I will post patch.
>
> It seems you focus on the TAP use case only, seeing you removed
> code that doesn't work for TAP but do work for regular locally
> terminated flows.
>
I have tested patch with GRE TAP and non TAP devices.

> You did a lot of implementation choices and none of them
> are described in a changelog, making future work a bit hard.
>

ok. I will update changelog.

^ permalink raw reply

* Re: [PATCH V6 2/3] virtio-net: split out clean affinity function
From: Wanlong Gao @ 2013-01-25  4:20 UTC (permalink / raw)
  To: Jason Wang
  Cc: Michael S. Tsirkin, netdev, linux-kernel, virtualization,
	Eric Dumazet
In-Reply-To: <5101FBE0.7040505@redhat.com>

On 01/25/2013 11:28 AM, Jason Wang wrote:
> On 01/21/2013 07:25 PM, Wanlong Gao wrote:
>> Split out the clean affinity function to virtnet_clean_affinity().
>>
>> Cc: Rusty Russell <rusty@rustcorp.com.au>
>> Cc: "Michael S. Tsirkin" <mst@redhat.com>
>> Cc: Jason Wang <jasowang@redhat.com>
>> Cc: Eric Dumazet <erdnetdev@gmail.com>
>> Cc: virtualization@lists.linux-foundation.org
>> Cc: netdev@vger.kernel.org
>> Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
>> ---
>> V5->V6: NEW
>>
>>  drivers/net/virtio_net.c | 67 +++++++++++++++++++++++++++---------------------
>>  1 file changed, 38 insertions(+), 29 deletions(-)
>>
>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>> index 70cd957..1a35a8c 100644
>> --- a/drivers/net/virtio_net.c
>> +++ b/drivers/net/virtio_net.c
>> @@ -1016,48 +1016,57 @@ static int virtnet_vlan_rx_kill_vid(struct net_device *dev, u16 vid)
>>  	return 0;
>>  }
>>  
>> -static void virtnet_set_affinity(struct virtnet_info *vi, bool set)
>> +static void virtnet_clean_affinity(struct virtnet_info *vi, long hcpu)
>>  {
>>  	int i;
>>  	int cpu;
>>  
>> -	/* In multiqueue mode, when the number of cpu is equal to the number of
>> -	 * queue pairs, we let the queue pairs to be private to one cpu by
>> -	 * setting the affinity hint to eliminate the contention.
>> -	 */
>> -	if ((vi->curr_queue_pairs == 1 ||
>> -	     vi->max_queue_pairs != num_online_cpus()) && set) {
>> -		if (vi->affinity_hint_set)
>> -			set = false;
>> -		else
>> -			return;
>> -	}
>> -
>> -	if (set) {
>> -		i = 0;
>> -		for_each_online_cpu(cpu) {
>> -			virtqueue_/set_affinity(vi->rq[i].vq, cpu);
>> -			virtqueue_set_affinity(vi->sq[i].vq, cpu);
>> -			*per_cpu_ptr(vi->vq_index, cpu) = i;
>> -			i++;
>> -		}
>> -
>> -		vi->affinity_hint_set = true;
>> -	} else {
>> -		for(i = 0; i < vi->max_queue_pairs; i++) {
>> +	if (vi->affinity_hint_set) {
>> +		for (i = 0; i < vi->max_queue_pairs; i++) {
>>  			virtqueue_set_affinity(vi->rq[i].vq, -1);
>>  			virtqueue_set_affinity(vi->sq[i].vq, -1);
>>  		}
>>  
>>  		i = 0;
>> -		for_each_online_cpu(cpu)
>> +		for_each_online_cpu(cpu) {
>> +			if (cpu == hcpu)
>> +				continue;
>>  			*per_cpu_ptr(vi->vq_index, cpu) =
>>  				++i % vi->curr_queue_pairs;
>> +		}
>>  
> 
> Some questions here:
> 
> - Did we need reset the affinity of the queue here like the this?
> 
> virtqueue_set_affinity(vi->sq[*per_cpu_ptr(vi->vq_index, hcpu)], -1);
> virtqueue_set_affinity(vi->rq[*per_cpu_ptr(vi->vq_index, hcpu)], -1);

I think no, we are going to unset the affinity of all the set queues,
include hcpu.

> 
> - Looks like we need also reset the percpu index when
> vi->affinity_hint_set is false.

Yes, follow this and the comment on [1/3].

> - Does this really need this reset? Consider we're going to reset the
> percpu in CPU_DEAD?

I think resetting when CPU_DOWN_PREPARE can avoid selecting the wrong queue
on the dying CPU.

Thanks,
Wanlong Gao

> 
> Thanks
>>  		vi->affinity_hint_set = false;
>>  	}
>>  }
>>  
>> +static void virtnet_set_affinity(struct virtnet_info *vi)
>> +{
>> +	int i;
>> +	int cpu;
>> +
>> +	/* In multiqueue mode, when the number of cpu is equal to the number of
>> +	 * queue pairs, we let the queue pairs to be private to one cpu by
>> +	 * setting the affinity hint to eliminate the contention.
>> +	 */
>> +	if (vi->curr_queue_pairs == 1 ||
>> +	    vi->max_queue_pairs != num_online_cpus()) {
>> +		if (vi->affinity_hint_set)
>> +			virtnet_clean_affinity(vi, -1);
>> +		else
>> +			return;
>> +	}
>> +
>> +	i = 0;
>> +	for_each_online_cpu(cpu) {
>> +		virtqueue_set_affinity(vi->rq[i].vq, cpu);
>> +		virtqueue_set_affinity(vi->sq[i].vq, cpu);
>> +		*per_cpu_ptr(vi->vq_index, cpu) = i;
>> +		i++;
>> +	}
>> +
>> +	vi->affinity_hint_set = true;
>> +}
>> +
>>  static void virtnet_get_ringparam(struct net_device *dev,
>>  				struct ethtool_ringparam *ring)
>>  {
>> @@ -1105,7 +1114,7 @@ static int virtnet_set_channels(struct net_device *dev,
>>  		netif_set_real_num_rx_queues(dev, queue_pairs);
>>  
>>  		get_online_cpus();
>> -		virtnet_set_affinity(vi, true);
>> +		virtnet_set_affinity(vi);
>>  		put_online_cpus();
>>  	}
>>  
>> @@ -1274,7 +1283,7 @@ static void virtnet_del_vqs(struct virtnet_info *vi)
>>  {
>>  	struct virtio_device *vdev = vi->vdev;
>>  
>> -	virtnet_set_affinity(vi, false);
>> +	virtnet_clean_affinity(vi, -1);
>>  
>>  	vdev->config->del_vqs(vdev);
>>  
>> @@ -1398,7 +1407,7 @@ static int init_vqs(struct virtnet_info *vi)
>>  		goto err_free;
>>  
>>  	get_online_cpus();
>> -	virtnet_set_affinity(vi, true);
>> +	virtnet_set_affinity(vi);
>>  	put_online_cpus();
>>  
>>  	return 0;
> 
> 

^ permalink raw reply

* Re: [PATCH v3 1/1 net-next] net: fec: add napi support to improve proformance
From: Waskiewicz Jr, Peter P @ 2013-01-25  4:33 UTC (permalink / raw)
  To: Frank Li
  Cc: lznuaa, shawn.guo, B38611, davem, linux-arm-kernel, netdev,
	s.hauer
In-Reply-To: <1359077343-8271-1-git-send-email-Frank.Li@freescale.com>

On Fri, Jan 25, 2013 at 09:29:03AM +0800, Frank Li wrote:

[...]

>  static void
> +fec_enet_rx_int_enable(struct net_device *ndev, bool enabled)
> +{
> +	struct fec_enet_private *fep = netdev_priv(ndev);
> +	uint    int_events;
> +
> +	int_events = readl(fep->hwp + FEC_IMASK);
> +	if (enabled)
> +		int_events |= FEC_ENET_RXF;
> +	else
> +		int_events &= ~FEC_ENET_RXF;
> +	writel(int_events, fep->hwp + FEC_IMASK);
> +}

You hold your hw_lock when this is called to disable the interrupt, but
not when you re-enable it.  See below.

[...]

> @@ -813,7 +831,14 @@ fec_enet_interrupt(int irq, void *dev_id)
>  
>  		if (int_events & FEC_ENET_RXF) {
>  			ret = IRQ_HANDLED;
> -			fec_enet_rx(ndev);
> +
> +			spin_lock(&fep->hw_lock);
> +			/* Disable the RX interrupt */
> +			if (napi_schedule_prep(&fep->napi)) {
> +				fec_enet_rx_int_enable(ndev, false);
> +				__napi_schedule(&fep->napi);
> +			}
> +			spin_unlock(&fep->hw_lock);
>  		}
>  
>  		/* Transmit OK, or non-fatal error. Update the buffer
> @@ -834,7 +859,16 @@ fec_enet_interrupt(int irq, void *dev_id)
>  	return ret;
>  }
>  
> -
> +static int fec_enet_rx_napi(struct napi_struct *napi, int budget)
> +{
> +	struct net_device *ndev = napi->dev;
> +	int pkgs = fec_enet_rx(ndev, budget);
> +	if (pkgs < budget) {
> +		napi_complete(napi);
> +		fec_enet_rx_int_enable(ndev, true);

Here you're not locking when re-enabling, but you do in the disable path.

Also, when you're disabling interrupts above, you're doing that in your
HW interrupt handler, you should be using spin_lock_irqsave()/irq_restore().

Cheers,
-PJ

^ permalink raw reply

* Re: [PATCH v3 1/1 net-next] net: fec: add napi support to improve proformance
From: Frank Li @ 2013-01-25  4:51 UTC (permalink / raw)
  To: Waskiewicz Jr, Peter P
  Cc: Frank Li, shawn.guo, B38611, davem, linux-arm-kernel, netdev,
	s.hauer
In-Reply-To: <20130125043311.GA4141@ppwaskie-mobl2.amr.corp.intel.com>

2013/1/25 Waskiewicz Jr, Peter P <peter.p.waskiewicz.jr@intel.com>:
> On Fri, Jan 25, 2013 at 09:29:03AM +0800, Frank Li wrote:
>
> [...]
>
>>  static void
>> +fec_enet_rx_int_enable(struct net_device *ndev, bool enabled)
>> +{
>> +     struct fec_enet_private *fep = netdev_priv(ndev);
>> +     uint    int_events;
>> +
>> +     int_events = readl(fep->hwp + FEC_IMASK);
>> +     if (enabled)
>> +             int_events |= FEC_ENET_RXF;
>> +     else
>> +             int_events &= ~FEC_ENET_RXF;
>> +     writel(int_events, fep->hwp + FEC_IMASK);
>> +}
>
> You hold your hw_lock when this is called to disable the interrupt, but
> not when you re-enable it.  See below.
>
> [...]
>
>> @@ -813,7 +831,14 @@ fec_enet_interrupt(int irq, void *dev_id)
>>
>>               if (int_events & FEC_ENET_RXF) {
>>                       ret = IRQ_HANDLED;
>> -                     fec_enet_rx(ndev);
>> +
>> +                     spin_lock(&fep->hw_lock);
>> +                     /* Disable the RX interrupt */
>> +                     if (napi_schedule_prep(&fep->napi)) {
>> +                             fec_enet_rx_int_enable(ndev, false);
>> +                             __napi_schedule(&fep->napi);
>> +                     }
>> +                     spin_unlock(&fep->hw_lock);
>>               }
>>
>>               /* Transmit OK, or non-fatal error. Update the buffer
>> @@ -834,7 +859,16 @@ fec_enet_interrupt(int irq, void *dev_id)
>>       return ret;
>>  }
>>
>> -
>> +static int fec_enet_rx_napi(struct napi_struct *napi, int budget)
>> +{
>> +     struct net_device *ndev = napi->dev;
>> +     int pkgs = fec_enet_rx(ndev, budget);
>> +     if (pkgs < budget) {
>> +             napi_complete(napi);
>> +             fec_enet_rx_int_enable(ndev, true);
>
> Here you're not locking when re-enabling, but you do in the disable path.
>
> Also, when you're disabling interrupts above, you're doing that in your
> HW interrupt handler, you should be using spin_lock_irqsave()/irq_restore().

Good catch. Thanks!

>
> Cheers,
> -PJ

^ permalink raw reply

* Re: [PATCH V6 2/3] virtio-net: split out clean affinity function
From: Jason Wang @ 2013-01-25  5:13 UTC (permalink / raw)
  To: gaowanlong
  Cc: Michael S. Tsirkin, netdev, linux-kernel, virtualization,
	Eric Dumazet
In-Reply-To: <51020800.4080900@cn.fujitsu.com>

On 01/25/2013 12:20 PM, Wanlong Gao wrote:
> On 01/25/2013 11:28 AM, Jason Wang wrote:
>> On 01/21/2013 07:25 PM, Wanlong Gao wrote:
>>> Split out the clean affinity function to virtnet_clean_affinity().
>>>
>>> Cc: Rusty Russell <rusty@rustcorp.com.au>
>>> Cc: "Michael S. Tsirkin" <mst@redhat.com>
>>> Cc: Jason Wang <jasowang@redhat.com>
>>> Cc: Eric Dumazet <erdnetdev@gmail.com>
>>> Cc: virtualization@lists.linux-foundation.org
>>> Cc: netdev@vger.kernel.org
>>> Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
>>> ---
>>> V5->V6: NEW
>>>
>>>  drivers/net/virtio_net.c | 67 +++++++++++++++++++++++++++---------------------
>>>  1 file changed, 38 insertions(+), 29 deletions(-)
>>>
>>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>>> index 70cd957..1a35a8c 100644
>>> --- a/drivers/net/virtio_net.c
>>> +++ b/drivers/net/virtio_net.c
>>> @@ -1016,48 +1016,57 @@ static int virtnet_vlan_rx_kill_vid(struct net_device *dev, u16 vid)
>>>  	return 0;
>>>  }
>>>  
>>> -static void virtnet_set_affinity(struct virtnet_info *vi, bool set)
>>> +static void virtnet_clean_affinity(struct virtnet_info *vi, long hcpu)
>>>  {
>>>  	int i;
>>>  	int cpu;
>>>  
>>> -	/* In multiqueue mode, when the number of cpu is equal to the number of
>>> -	 * queue pairs, we let the queue pairs to be private to one cpu by
>>> -	 * setting the affinity hint to eliminate the contention.
>>> -	 */
>>> -	if ((vi->curr_queue_pairs == 1 ||
>>> -	     vi->max_queue_pairs != num_online_cpus()) && set) {
>>> -		if (vi->affinity_hint_set)
>>> -			set = false;
>>> -		else
>>> -			return;
>>> -	}
>>> -
>>> -	if (set) {
>>> -		i = 0;
>>> -		for_each_online_cpu(cpu) {
>>> -			virtqueue_/set_affinity(vi->rq[i].vq, cpu);
>>> -			virtqueue_set_affinity(vi->sq[i].vq, cpu);
>>> -			*per_cpu_ptr(vi->vq_index, cpu) = i;
>>> -			i++;
>>> -		}
>>> -
>>> -		vi->affinity_hint_set = true;
>>> -	} else {
>>> -		for(i = 0; i < vi->max_queue_pairs; i++) {
>>> +	if (vi->affinity_hint_set) {
>>> +		for (i = 0; i < vi->max_queue_pairs; i++) {
>>>  			virtqueue_set_affinity(vi->rq[i].vq, -1);
>>>  			virtqueue_set_affinity(vi->sq[i].vq, -1);
>>>  		}
>>>  
>>>  		i = 0;
>>> -		for_each_online_cpu(cpu)
>>> +		for_each_online_cpu(cpu) {
>>> +			if (cpu == hcpu)
>>> +				continue;
>>>  			*per_cpu_ptr(vi->vq_index, cpu) =
>>>  				++i % vi->curr_queue_pairs;
>>> +		}
>>>  
>> Some questions here:
>>
>> - Did we need reset the affinity of the queue here like the this?
>>
>> virtqueue_set_affinity(vi->sq[*per_cpu_ptr(vi->vq_index, hcpu)], -1);
>> virtqueue_set_affinity(vi->rq[*per_cpu_ptr(vi->vq_index, hcpu)], -1);
> I think no, we are going to unset the affinity of all the set queues,
> include hcpu.
>
>> - Looks like we need also reset the percpu index when
>> vi->affinity_hint_set is false.
> Yes, follow this and the comment on [1/3].
>
>> - Does this really need this reset? Consider we're going to reset the
>> percpu in CPU_DEAD?
> I think resetting when CPU_DOWN_PREPARE can avoid selecting the wrong queue
> on the dying CPU.

Didn't understand this. What does 'wrong queue' here mean? Looks like
you didn't change the preferable queue of the dying CPU and just change
all others.
>
> Thanks,
> Wanlong Gao
>
>> Thanks
>>>  		vi->affinity_hint_set = false;
>>>  	}
>>>  }
>>>  
>>> +static void virtnet_set_affinity(struct virtnet_info *vi)
>>> +{
>>> +	int i;
>>> +	int cpu;
>>> +
>>> +	/* In multiqueue mode, when the number of cpu is equal to the number of
>>> +	 * queue pairs, we let the queue pairs to be private to one cpu by
>>> +	 * setting the affinity hint to eliminate the contention.
>>> +	 */
>>> +	if (vi->curr_queue_pairs == 1 ||
>>> +	    vi->max_queue_pairs != num_online_cpus()) {
>>> +		if (vi->affinity_hint_set)
>>> +			virtnet_clean_affinity(vi, -1);
>>> +		else
>>> +			return;
>>> +	}
>>> +
>>> +	i = 0;
>>> +	for_each_online_cpu(cpu) {
>>> +		virtqueue_set_affinity(vi->rq[i].vq, cpu);
>>> +		virtqueue_set_affinity(vi->sq[i].vq, cpu);
>>> +		*per_cpu_ptr(vi->vq_index, cpu) = i;
>>> +		i++;
>>> +	}
>>> +
>>> +	vi->affinity_hint_set = true;
>>> +}
>>> +
>>>  static void virtnet_get_ringparam(struct net_device *dev,
>>>  				struct ethtool_ringparam *ring)
>>>  {
>>> @@ -1105,7 +1114,7 @@ static int virtnet_set_channels(struct net_device *dev,
>>>  		netif_set_real_num_rx_queues(dev, queue_pairs);
>>>  
>>>  		get_online_cpus();
>>> -		virtnet_set_affinity(vi, true);
>>> +		virtnet_set_affinity(vi);
>>>  		put_online_cpus();
>>>  	}
>>>  
>>> @@ -1274,7 +1283,7 @@ static void virtnet_del_vqs(struct virtnet_info *vi)
>>>  {
>>>  	struct virtio_device *vdev = vi->vdev;
>>>  
>>> -	virtnet_set_affinity(vi, false);
>>> +	virtnet_clean_affinity(vi, -1);
>>>  
>>>  	vdev->config->del_vqs(vdev);
>>>  
>>> @@ -1398,7 +1407,7 @@ static int init_vqs(struct virtnet_info *vi)
>>>  		goto err_free;
>>>  
>>>  	get_online_cpus();
>>> -	virtnet_set_affinity(vi, true);
>>> +	virtnet_set_affinity(vi);
>>>  	put_online_cpus();
>>>  
>>>  	return 0;
>>

^ permalink raw reply

* Re: [PATCH] vhost-net: fall back to vmalloc if high-order allocation fails
From: Jason Wang @ 2013-01-25  5:15 UTC (permalink / raw)
  To: Cong Wang; +Cc: kvm, netdev, linux-kernel
In-Reply-To: <kdsslg$ddn$1@ger.gmane.org>

On Friday, January 25, 2013 03:03:13 AM Cong Wang wrote:
> ["Followup-To:" header set to gmane.linux.network.]
> 
> On Wed, 23 Jan 2013 at 20:46 GMT, Romain Francoise <romain@orebokech.com> 
wrote:
> > Creating a vhost-net device allocates an object large enough (34320 bytes
> > on x86-64) to trigger an order-4 allocation, which may fail if memory if
> > 
> > fragmented:
> >  libvirtd: page allocation failure: order:4, mode:0x2000d0
> >  ...
> >  SLAB: Unable to allocate memory on node 0 (gfp=0xd0)
> >  
> >    cache: size-65536, object size: 65536, order: 4
> >    node 0: slabs: 8/8, objs: 8/8, free: 0
> > 
> > In that situation, rather than forcing the caller to use regular
> > virtio-net, try to allocate the descriptor with vmalloc().
> 
> The real problem is vhost_net struct is really big, it
> should be reduced rather than workarounded like this.
> 

Looks like iov in vhost_virtqueue is a little big:

    struct iovec iov[UIO_MAXIOV];

Maybe we can use pointer and allocate it like indirect in 
vhost_dev_alloc_iovecs().

> > +static void vhost_net_kvfree(void *addr)
> > +{
> > +	if (is_vmalloc_addr(addr))
> > +		vfree(addr);
> > +	else
> > +		kfree(addr);
> > +}
> > +
> 
> This kind of stuff should really go to mm, not netdev.
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH [iputils] 3/6] start gitignore files
From: Mike Frysinger @ 2013-01-25  5:24 UTC (permalink / raw)
  To: YOSHIFUJI Hideaki; +Cc: netdev
In-Reply-To: <1359091455-25380-1-git-send-email-vapier@gentoo.org>

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 .gitignore     | 22 ++++++++++++++++++++++
 doc/.gitignore |  2 ++
 2 files changed, 24 insertions(+)
 create mode 100644 .gitignore
 create mode 100644 doc/.gitignore

diff --git a/.gitignore b/.gitignore
new file mode 100644
index 0000000..30ed00c
--- /dev/null
+++ b/.gitignore
@@ -0,0 +1,22 @@
+*~
+*.o
+
+*.diff
+*.orig
+*.patch
+*.rej
+
+core
+.gdb_history
+.gdbinit
+
+/arping
+/clockdiff
+/ping
+/ping6
+/rarpd
+/rdisc
+/tftpd
+/tracepath
+/tracepath6
+/traceroute6
diff --git a/doc/.gitignore b/doc/.gitignore
new file mode 100644
index 0000000..085639f
--- /dev/null
+++ b/doc/.gitignore
@@ -0,0 +1,2 @@
+*.html
+*.8
-- 
1.8.0.2

^ permalink raw reply related

* [PATCH [iputils] 2/6] ping: fix building on older systems
From: Mike Frysinger @ 2013-01-25  5:24 UTC (permalink / raw)
  To: YOSHIFUJI Hideaki; +Cc: netdev
In-Reply-To: <1359091455-25380-1-git-send-email-vapier@gentoo.org>

The SO_MARK define is somewhat recent (linux-2.6.25), so check for its
existence before we try to use it.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 ping_common.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/ping_common.c b/ping_common.c
index 7f82851..8d6b145 100644
--- a/ping_common.c
+++ b/ping_common.c
@@ -630,6 +630,7 @@ void setup(int icmp_sock)
 			fprintf(stderr, "Warning: no SO_TIMESTAMP support, falling back to SIOCGSTAMP\n");
 	}
 #endif
+#ifdef SO_MARK
 	if (options & F_MARK) {
 		int ret;
 
@@ -644,6 +645,7 @@ void setup(int icmp_sock)
 			fprintf(stderr, "Warning: Failed to set mark %d\n", mark);
 		}
 	}
+#endif
 
 	/* Set some SNDTIMEO to prevent blocking forever
 	 * on sends, when device is too slow or stalls. Just put limit
-- 
1.8.0.2

^ permalink raw reply related

* [PATCH [iputils] 5/6] ping6: allow disabling of openssl support
From: Mike Frysinger @ 2013-01-25  5:24 UTC (permalink / raw)
  To: YOSHIFUJI Hideaki; +Cc: netdev
In-Reply-To: <1359091455-25380-1-git-send-email-vapier@gentoo.org>

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 Makefile |  5 ++++-
 ping6.c  | 14 +++++++++++++-
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 89249f5..2c49940 100644
--- a/Makefile
+++ b/Makefile
@@ -36,7 +36,7 @@ ARPING_DEFAULT_DEVICE=
 
 # GNU TLS library for ping6 [yes|no|static]
 USE_GNUTLS=yes
-# Crypto library for ping6 [shared|static]
+# Crypto library for ping6 [shared|static|no]
 USE_CRYPTO=shared
 # Resolv library for ping6 [yes|static]
 USE_RESOLV=yes
@@ -63,7 +63,10 @@ ifneq ($(USE_GNUTLS),no)
 	LIB_CRYPTO = $(call FUNC_LIB,$(USE_GNUTLS),$(LDFLAG_GNUTLS))
 	DEF_CRYPTO = -DUSE_GNUTLS
 else
+ifneq ($(USE_CRYPTO),no)
 	LIB_CRYPTO = $(call FUNC_LIB,$(USE_CRYPTO),$(LDFLAG_CRYPTO))
+	DEF_CRYPTO = -DUSE_OPENSSL
+endif
 endif
 
 # USE_RESOLV: LIB_RESOLV
diff --git a/ping6.c b/ping6.c
index c39864d..f2f90af 100644
--- a/ping6.c
+++ b/ping6.c
@@ -168,8 +168,10 @@ static int icmp_sock;
 
 #ifdef USE_GNUTLS
 # include <gnutls/openssl.h>
-#else
+# define USE_CRYPTO
+#elif defined USE_OPENSSL
 # include <openssl/md5.h>
+# define USE_CRYPTO
 #endif
 
 /* Node Information query */
@@ -326,6 +328,7 @@ static void niquery_init_nonce(void)
 #if !PING6_NONCE_MEMORY
 static int niquery_nonce(__u8 *nonce, int fill)
 {
+# ifdef USE_CRYPTO
 	static __u8 digest[MD5_DIGEST_LENGTH];
 	static int seq = -1;
 
@@ -348,6 +351,10 @@ static int niquery_nonce(__u8 *nonce, int fill)
 			return -1;
 		return ntohsp((__u16 *)nonce);
 	}
+# else
+	fprintf(stderr, "ping6: function not available; crypto disabled\n");
+	exit(3);
+# endif
 }
 #endif
 
@@ -502,6 +509,7 @@ static int niquery_option_subject_addr_handler(int index, const char *arg)
 
 static int niquery_option_subject_name_handler(int index, const char *arg)
 {
+#ifdef USE_CRYPTO
 	static char nigroup_buf[INET6_ADDRSTRLEN + 1 + IFNAMSIZ];
 	unsigned char *dnptrs[2], **dpp, **lastdnptr;
 	int n;
@@ -627,6 +635,10 @@ errexit:
 	free(idn);
 	free(name);
 	exit(1);
+#else
+	fprintf(stderr, "ping6: function not available; crypto disabled\n");
+	exit(3);
+#endif
 }
 
 int niquery_option_help_handler(int index, const char *arg)
-- 
1.8.0.2

^ permalink raw reply related

* [PATCH [iputils] 4/6] doc: fix parallel build of html/man pages
From: Mike Frysinger @ 2013-01-25  5:24 UTC (permalink / raw)
  To: YOSHIFUJI Hideaki; +Cc: netdev
In-Reply-To: <1359091455-25380-1-git-send-email-vapier@gentoo.org>

The use of the same tempdir prevents building of these files in parallel.
So build all of them in unique tempdirs so we can do them in parallel.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 doc/Makefile | 37 ++++++++++++++++++++++++-------------
 1 file changed, 24 insertions(+), 13 deletions(-)

diff --git a/doc/Makefile b/doc/Makefile
index 7ec4f1c..a9c303e 100644
--- a/doc/Makefile
+++ b/doc/Makefile
@@ -12,29 +12,40 @@ man: $(MANFILES)
 # lots of some strange temporary junk directories and files.
 # So, scope it to a temporary dir and clean all after each run.
 
-$(HTMLFILES): index.db
-	@-rm -rf tmp.db2html
-	@mkdir tmp.db2html
-	@set -e; cd tmp.db2html; docbook2html ../$< ; mv *.html ..
-	@-rm -rf tmp.db2html
+SETUP_TMPDIR = \
+	t="tmp.db2html.$@"; \
+	rm -rf $$t; \
+	mkdir $$t; \
+	pushd $$t >/dev/null
+CLEAN_TMPDIR = \
+	popd >/dev/null; \
+	rm -rf $$t
+
+MAKE_HTML = \
+	@set -e; \
+	$(SETUP_TMPDIR); \
+	docbook2html ../$<; \
+	mv *.html ..; \
+	$(CLEAN_TMPDIR)
 
+$(HTMLFILES): index.db
+	$(MAKE_HTML)
 iputils.html: iputils.db
-	@-rm -rf tmp.db2html
-	@mkdir tmp.db2html
-	@set -e; cd tmp.db2html; docbook2html -u -o html ../$< ; mv html/$@ ..
-	@-rm -rf tmp.db2html
+	$(MAKE_HTML)
 
 # docbook2man produces utterly ugly output and I did not find
 # any way to customize this but hacking backend perl script a little.
 # Well, hence...
 
 $(MANFILES): index.db
-	@-mkdir tmp.db2man
-	@set -e; cd tmp.db2man; nsgmls ../$< | sgmlspl ../docbook2man-spec.pl ;	mv $@ ..
-	@-rm -rf tmp.db2man
+	@set -e; \
+	$(SETUP_TMPDIR); \
+	nsgmls ../$< | sgmlspl ../docbook2man-spec.pl; \
+	mv $@ ..; \
+	$(CLEAN_TMPDIR)
 
 clean:
-	@rm -rf $(MANFILES) $(HTMLFILES) iputils.html tmp.db2html tmp.db2man
+	@rm -rf $(MANFILES) $(HTMLFILES) iputils.html tmp.db2html* tmp.db2man*
 
 snapshot:
 	@date "+%y%m%d" > snapshot.db
-- 
1.8.0.2

^ permalink raw reply related

* [PATCH [iputils] 6/6] fix handling of CFLAGS
From: Mike Frysinger @ 2013-01-25  5:24 UTC (permalink / raw)
  To: YOSHIFUJI Hideaki; +Cc: netdev
In-Reply-To: <1359091455-25380-1-git-send-email-vapier@gentoo.org>

This defaults CFLAGS to -O3 without clobbering settings people have set
up in the environment already.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 Makefile | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/Makefile b/Makefile
index 2c49940..dc317b1 100644
--- a/Makefile
+++ b/Makefile
@@ -48,11 +48,10 @@ ENABLE_RDISC_SERVER=no
 
 # -------------------------------------
 # What a pity, all new gccs are buggy and -Werror does not work. Sigh.
-# CCOPT=-fno-strict-aliasing -Wstrict-prototypes -Wall -Werror -g
-CCOPT=-fno-strict-aliasing -Wstrict-prototypes -Wall -g
-CCOPTOPT=-O3
-GLIBCFIX=-D_GNU_SOURCE
-DEFINES=
+# CFLAGS+=-fno-strict-aliasing -Wstrict-prototypes -Wall -Werror -g
+CFLAGS?=-O3 -g
+CFLAGS+=-fno-strict-aliasing -Wstrict-prototypes -Wall
+CPPFLAGS+=-D_GNU_SOURCE
 LDLIB=
 
 FUNC_LIB = $(if $(filter static,$(1)),$(LDFLAG_STATIC) $(2) $(LDFLAG_DYNAMIC),$(2))
@@ -113,7 +112,6 @@ IPV4_TARGETS=tracepath ping clockdiff rdisc arping tftpd rarpd
 IPV6_TARGETS=tracepath6 traceroute6 ping6
 TARGETS=$(IPV4_TARGETS) $(IPV6_TARGETS)
 
-CFLAGS=$(CCOPTOPT) $(CCOPT) $(GLIBCFIX) $(DEFINES)
 LDLIBS=$(LDLIB) $(ADDLIB)
 
 UNAME_N:=$(shell uname -n)
-- 
1.8.0.2

^ permalink raw reply related

* [PATCH [iputils] 1/6] tracepath: re-use printf return in print_host
From: Mike Frysinger @ 2013-01-25  5:24 UTC (permalink / raw)
  To: YOSHIFUJI Hideaki; +Cc: netdev

Since the printf funcs already return the length of chars displayed,
use that value instead of re-calculating the length with strlen.

This also fixes the handling of the strlen return -- it's a size_t,
not an int.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 tracepath.c  | 11 ++++-------
 tracepath6.c | 11 ++++-------
 2 files changed, 8 insertions(+), 14 deletions(-)

diff --git a/tracepath.c b/tracepath.c
index 8a08f1d..f155816 100644
--- a/tracepath.c
+++ b/tracepath.c
@@ -73,13 +73,10 @@ void data_wait(int fd)
 
 void print_host(const char *a, const char *b, int both)
 {
-	int plen = 0;
-	printf("%s", a);
-	plen = strlen(a);
-	if (both) {
-		printf(" (%s)", b);
-		plen += strlen(b) + 3;
-	}
+	int plen;
+	plen = printf("%s", a);
+	if (both)
+		plen += printf(" (%s)", b);
 	if (plen >= HOST_COLUMN_SIZE)
 		plen = HOST_COLUMN_SIZE - 1;
 	printf("%*s", HOST_COLUMN_SIZE - plen, "");
diff --git a/tracepath6.c b/tracepath6.c
index 126fadf..bee95c3 100644
--- a/tracepath6.c
+++ b/tracepath6.c
@@ -86,13 +86,10 @@ void data_wait(int fd)
 
 void print_host(const char *a, const char *b, int both)
 {
-	int plen = 0;
-	printf("%s", a);
-	plen = strlen(a);
-	if (both) {
-		printf(" (%s)", b);
-		plen += strlen(b) + 3;
-	}
+	int plen;
+	plen = printf("%s", a);
+	if (both)
+		plen += printf(" (%s)", b);
 	if (plen >= HOST_COLUMN_SIZE)
 		plen = HOST_COLUMN_SIZE - 1;
 	printf("%*s", HOST_COLUMN_SIZE - plen, "");
-- 
1.8.0.2

^ permalink raw reply related

* Re: [PATCH V6 2/3] virtio-net: split out clean affinity function
From: Wanlong Gao @ 2013-01-25  5:40 UTC (permalink / raw)
  To: Jason Wang
  Cc: linux-kernel, Rusty Russell, Michael S. Tsirkin, Eric Dumazet,
	virtualization, netdev, Wanlong Gao
In-Reply-To: <5102145D.7030607@redhat.com>

On 01/25/2013 01:13 PM, Jason Wang wrote:
> On 01/25/2013 12:20 PM, Wanlong Gao wrote:
>> On 01/25/2013 11:28 AM, Jason Wang wrote:
>>> On 01/21/2013 07:25 PM, Wanlong Gao wrote:
>>>> Split out the clean affinity function to virtnet_clean_affinity().
>>>>
>>>> Cc: Rusty Russell <rusty@rustcorp.com.au>
>>>> Cc: "Michael S. Tsirkin" <mst@redhat.com>
>>>> Cc: Jason Wang <jasowang@redhat.com>
>>>> Cc: Eric Dumazet <erdnetdev@gmail.com>
>>>> Cc: virtualization@lists.linux-foundation.org
>>>> Cc: netdev@vger.kernel.org
>>>> Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
>>>> ---
>>>> V5->V6: NEW
>>>>
>>>>  drivers/net/virtio_net.c | 67 +++++++++++++++++++++++++++---------------------
>>>>  1 file changed, 38 insertions(+), 29 deletions(-)
>>>>
>>>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>>>> index 70cd957..1a35a8c 100644
>>>> --- a/drivers/net/virtio_net.c
>>>> +++ b/drivers/net/virtio_net.c
>>>> @@ -1016,48 +1016,57 @@ static int virtnet_vlan_rx_kill_vid(struct net_device *dev, u16 vid)
>>>>  	return 0;
>>>>  }
>>>>  
>>>> -static void virtnet_set_affinity(struct virtnet_info *vi, bool set)
>>>> +static void virtnet_clean_affinity(struct virtnet_info *vi, long hcpu)
>>>>  {
>>>>  	int i;
>>>>  	int cpu;
>>>>  
>>>> -	/* In multiqueue mode, when the number of cpu is equal to the number of
>>>> -	 * queue pairs, we let the queue pairs to be private to one cpu by
>>>> -	 * setting the affinity hint to eliminate the contention.
>>>> -	 */
>>>> -	if ((vi->curr_queue_pairs == 1 ||
>>>> -	     vi->max_queue_pairs != num_online_cpus()) && set) {
>>>> -		if (vi->affinity_hint_set)
>>>> -			set = false;
>>>> -		else
>>>> -			return;
>>>> -	}
>>>> -
>>>> -	if (set) {
>>>> -		i = 0;
>>>> -		for_each_online_cpu(cpu) {
>>>> -			virtqueue_/set_affinity(vi->rq[i].vq, cpu);
>>>> -			virtqueue_set_affinity(vi->sq[i].vq, cpu);
>>>> -			*per_cpu_ptr(vi->vq_index, cpu) = i;
>>>> -			i++;
>>>> -		}
>>>> -
>>>> -		vi->affinity_hint_set = true;
>>>> -	} else {
>>>> -		for(i = 0; i < vi->max_queue_pairs; i++) {
>>>> +	if (vi->affinity_hint_set) {
>>>> +		for (i = 0; i < vi->max_queue_pairs; i++) {
>>>>  			virtqueue_set_affinity(vi->rq[i].vq, -1);
>>>>  			virtqueue_set_affinity(vi->sq[i].vq, -1);
>>>>  		}
>>>>  
>>>>  		i = 0;
>>>> -		for_each_online_cpu(cpu)
>>>> +		for_each_online_cpu(cpu) {
>>>> +			if (cpu == hcpu)
>>>> +				continue;
>>>>  			*per_cpu_ptr(vi->vq_index, cpu) =
>>>>  				++i % vi->curr_queue_pairs;
>>>> +		}
>>>>  
>>> Some questions here:
>>>
>>> - Did we need reset the affinity of the queue here like the this?
>>>
>>> virtqueue_set_affinity(vi->sq[*per_cpu_ptr(vi->vq_index, hcpu)], -1);
>>> virtqueue_set_affinity(vi->rq[*per_cpu_ptr(vi->vq_index, hcpu)], -1);
>> I think no, we are going to unset the affinity of all the set queues,
>> include hcpu.
>>
>>> - Looks like we need also reset the percpu index when
>>> vi->affinity_hint_set is false.
>> Yes, follow this and the comment on [1/3].
>>
>>> - Does this really need this reset? Consider we're going to reset the
>>> percpu in CPU_DEAD?
>> I think resetting when CPU_DOWN_PREPARE can avoid selecting the wrong queue
>> on the dying CPU.
> 
> Didn't understand this. What does 'wrong queue' here mean? Looks like
> you didn't change the preferable queue of the dying CPU and just change
> all others.

How about setting the vq index to -1 on hcpu when doing DOWN_PREPARE?
So that let it select txq to 0 when the CPU is dying.

Thanks,
Wanlong Gao

>>
>> Thanks,
>> Wanlong Gao
>>
>>> Thanks
>>>>  		vi->affinity_hint_set = false;
>>>>  	}
>>>>  }
>>>>  
>>>> +static void virtnet_set_affinity(struct virtnet_info *vi)
>>>> +{
>>>> +	int i;
>>>> +	int cpu;
>>>> +
>>>> +	/* In multiqueue mode, when the number of cpu is equal to the number of
>>>> +	 * queue pairs, we let the queue pairs to be private to one cpu by
>>>> +	 * setting the affinity hint to eliminate the contention.
>>>> +	 */
>>>> +	if (vi->curr_queue_pairs == 1 ||
>>>> +	    vi->max_queue_pairs != num_online_cpus()) {
>>>> +		if (vi->affinity_hint_set)
>>>> +			virtnet_clean_affinity(vi, -1);
>>>> +		else
>>>> +			return;
>>>> +	}
>>>> +
>>>> +	i = 0;
>>>> +	for_each_online_cpu(cpu) {
>>>> +		virtqueue_set_affinity(vi->rq[i].vq, cpu);
>>>> +		virtqueue_set_affinity(vi->sq[i].vq, cpu);
>>>> +		*per_cpu_ptr(vi->vq_index, cpu) = i;
>>>> +		i++;
>>>> +	}
>>>> +
>>>> +	vi->affinity_hint_set = true;
>>>> +}
>>>> +
>>>>  static void virtnet_get_ringparam(struct net_device *dev,
>>>>  				struct ethtool_ringparam *ring)
>>>>  {
>>>> @@ -1105,7 +1114,7 @@ static int virtnet_set_channels(struct net_device *dev,
>>>>  		netif_set_real_num_rx_queues(dev, queue_pairs);
>>>>  
>>>>  		get_online_cpus();
>>>> -		virtnet_set_affinity(vi, true);
>>>> +		virtnet_set_affinity(vi);
>>>>  		put_online_cpus();
>>>>  	}
>>>>  
>>>> @@ -1274,7 +1283,7 @@ static void virtnet_del_vqs(struct virtnet_info *vi)
>>>>  {
>>>>  	struct virtio_device *vdev = vi->vdev;
>>>>  
>>>> -	virtnet_set_affinity(vi, false);
>>>> +	virtnet_clean_affinity(vi, -1);
>>>>  
>>>>  	vdev->config->del_vqs(vdev);
>>>>  
>>>> @@ -1398,7 +1407,7 @@ static int init_vqs(struct virtnet_info *vi)
>>>>  		goto err_free;
>>>>  
>>>>  	get_online_cpus();
>>>> -	virtnet_set_affinity(vi, true);
>>>> +	virtnet_set_affinity(vi);
>>>>  	put_online_cpus();
>>>>  
>>>>  	return 0;
>>>
> 
> 

^ permalink raw reply

* Re: [PATCH 0/5]: soreuseport: Bind multiple sockets to the same port
From: Nick Jones @ 2013-01-25  5:06 UTC (permalink / raw)
  To: Tom Herbert; +Cc: netdev, davem, netdev, eric.dumazet
In-Reply-To: <alpine.DEB.2.00.1301221142300.26333@pokey.mtv.corp.google.com>

On Wednesday, January 23, 2013 03:49 AM, Tom Herbert wrote:
...
>
> The motivating case for so_resuseport in TCP would be something like
> a web server binding to port 80 running with multiple threads, where
> each thread might have it's own listener socket.  This could be done
> as an alternative to other models: 1) have one listener thread which
> dispatches completed connections to workers. 2) accept on a single
> listener socket from multiple threads.  In case #1 the listener thread
> can easily become the bottleneck with high connection turn-over rate.
> In case #2, the proportion of connections accepted per thread tends
> to be uneven under high connection load (assuming simple event loop:
> while (1) { accept(); process() }, wakeup does not promote fairness
> among the sockets.  We have seen the  disproportion to be as high
> as 3:1 ratio between thread accepting most connections and the one
> accepting the fewest.  With so_reusport the distribution is
> uniform.
>

There is another model for accepting connections in a multi threaded 
application that I experimented with: dup the listener fd one time for 
each thread, then each thread register the fd in its own epoll set, then 
listen and accept independently.

Has anyone had experience with this strategy?  I'm sure that the 
SO_REUSEPORT feature will lead to much better performance, I'm just 
asking from the point of view of one who doesn't have that feature 
available.  I wonder if this strategy is like a poor mans SO_REUSEPORT?

The advantages of this approach were not fully proved in practise, I 
didn't produce any hard figures, but in theory it was appealing:
- no bottleneck of using a single thread for accepting then distributing 
connections (in addition to latency of waiting for the job handling 
thread to receive the event and start its work)
- connections were used in the thread in which they were accepted, thus 
locality was maintained (am I exaggerating this benefit?)
- when a new connection was received, all threads woke up to activity on 
their respective fd copies, and started accepting, I assume from the 
same connection queue.  This was a disadvantage at low load levels as 
threads would often wake and find nothing to do, but as loads got 
higher, strace showed that less threads would awake to disappointment.
- this approach was able to handle stress tests from a hardware packet 
generator, which showed no dropped or unhandled connections.

One disadvantage I imagined was that the single socket and all of its 
duplicates may find themselves attached to one cpu core or hardware 
queue on the network adapter, I don't know enough about the core net 
internals to say for sure, but as a precaution the dup was done in the 
context of the thread that would use the copy.

Just sharing and seeking comments.

^ permalink raw reply

* Re: [PATCH V6 2/3] virtio-net: split out clean affinity function
From: Jason Wang @ 2013-01-25  6:12 UTC (permalink / raw)
  To: gaowanlong
  Cc: Michael S. Tsirkin, netdev, linux-kernel, virtualization,
	Eric Dumazet
In-Reply-To: <51021ABB.8050508@cn.fujitsu.com>

On 01/25/2013 01:40 PM, Wanlong Gao wrote:
> On 01/25/2013 01:13 PM, Jason Wang wrote:
>> On 01/25/2013 12:20 PM, Wanlong Gao wrote:
>>> On 01/25/2013 11:28 AM, Jason Wang wrote:
>>>> On 01/21/2013 07:25 PM, Wanlong Gao wrote:
>>>>> Split out the clean affinity function to virtnet_clean_affinity().
>>>>>
>>>>> Cc: Rusty Russell <rusty@rustcorp.com.au>
>>>>> Cc: "Michael S. Tsirkin" <mst@redhat.com>
>>>>> Cc: Jason Wang <jasowang@redhat.com>
>>>>> Cc: Eric Dumazet <erdnetdev@gmail.com>
>>>>> Cc: virtualization@lists.linux-foundation.org
>>>>> Cc: netdev@vger.kernel.org
>>>>> Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
>>>>> ---
>>>>> V5->V6: NEW
>>>>>
>>>>>  drivers/net/virtio_net.c | 67 +++++++++++++++++++++++++++---------------------
>>>>>  1 file changed, 38 insertions(+), 29 deletions(-)
>>>>>
>>>>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>>>>> index 70cd957..1a35a8c 100644
>>>>> --- a/drivers/net/virtio_net.c
>>>>> +++ b/drivers/net/virtio_net.c
>>>>> @@ -1016,48 +1016,57 @@ static int virtnet_vlan_rx_kill_vid(struct net_device *dev, u16 vid)
>>>>>  	return 0;
>>>>>  }
>>>>>  
>>>>> -static void virtnet_set_affinity(struct virtnet_info *vi, bool set)
>>>>> +static void virtnet_clean_affinity(struct virtnet_info *vi, long hcpu)
>>>>>  {
>>>>>  	int i;
>>>>>  	int cpu;
>>>>>  
>>>>> -	/* In multiqueue mode, when the number of cpu is equal to the number of
>>>>> -	 * queue pairs, we let the queue pairs to be private to one cpu by
>>>>> -	 * setting the affinity hint to eliminate the contention.
>>>>> -	 */
>>>>> -	if ((vi->curr_queue_pairs == 1 ||
>>>>> -	     vi->max_queue_pairs != num_online_cpus()) && set) {
>>>>> -		if (vi->affinity_hint_set)
>>>>> -			set = false;
>>>>> -		else
>>>>> -			return;
>>>>> -	}
>>>>> -
>>>>> -	if (set) {
>>>>> -		i = 0;
>>>>> -		for_each_online_cpu(cpu) {
>>>>> -			virtqueue_/set_affinity(vi->rq[i].vq, cpu);
>>>>> -			virtqueue_set_affinity(vi->sq[i].vq, cpu);
>>>>> -			*per_cpu_ptr(vi->vq_index, cpu) = i;
>>>>> -			i++;
>>>>> -		}
>>>>> -
>>>>> -		vi->affinity_hint_set = true;
>>>>> -	} else {
>>>>> -		for(i = 0; i < vi->max_queue_pairs; i++) {
>>>>> +	if (vi->affinity_hint_set) {
>>>>> +		for (i = 0; i < vi->max_queue_pairs; i++) {
>>>>>  			virtqueue_set_affinity(vi->rq[i].vq, -1);
>>>>>  			virtqueue_set_affinity(vi->sq[i].vq, -1);
>>>>>  		}
>>>>>  
>>>>>  		i = 0;
>>>>> -		for_each_online_cpu(cpu)
>>>>> +		for_each_online_cpu(cpu) {
>>>>> +			if (cpu == hcpu)
>>>>> +				continue;
>>>>>  			*per_cpu_ptr(vi->vq_index, cpu) =
>>>>>  				++i % vi->curr_queue_pairs;
>>>>> +		}
>>>>>  
>>>> Some questions here:
>>>>
>>>> - Did we need reset the affinity of the queue here like the this?
>>>>
>>>> virtqueue_set_affinity(vi->sq[*per_cpu_ptr(vi->vq_index, hcpu)], -1);
>>>> virtqueue_set_affinity(vi->rq[*per_cpu_ptr(vi->vq_index, hcpu)], -1);
>>> I think no, we are going to unset the affinity of all the set queues,
>>> include hcpu.
>>>
>>>> - Looks like we need also reset the percpu index when
>>>> vi->affinity_hint_set is false.
>>> Yes, follow this and the comment on [1/3].
>>>
>>>> - Does this really need this reset? Consider we're going to reset the
>>>> percpu in CPU_DEAD?
>>> I think resetting when CPU_DOWN_PREPARE can avoid selecting the wrong queue
>>> on the dying CPU.
>> Didn't understand this. What does 'wrong queue' here mean? Looks like
>> you didn't change the preferable queue of the dying CPU and just change
>> all others.
> How about setting the vq index to -1 on hcpu when doing DOWN_PREPARE?
> So that let it select txq to 0 when the CPU is dying.

Looks safe, so look like what you're going to solve here is the the race
between cpu hotplug and virtnet_set_channels(). A possible better
solution is to serialize them by protecting virtnet_set_queues() by
get_online_cpus() also. After this, we can make sure the number of
channels were not changed during cpu hotplug, and looks like there's no
need to reset the preferable queues in DOWN_PREPARE.

What's your opinion?

Thanks
>
> Thanks,
> Wanlong Gao
>
>>> Thanks,
>>> Wanlong Gao
>>>
>>>> Thanks
>>>>>  		vi->affinity_hint_set = false;
>>>>>  	}
>>>>>  }
>>>>>  
>>>>> +static void virtnet_set_affinity(struct virtnet_info *vi)
>>>>> +{
>>>>> +	int i;
>>>>> +	int cpu;
>>>>> +
>>>>> +	/* In multiqueue mode, when the number of cpu is equal to the number of
>>>>> +	 * queue pairs, we let the queue pairs to be private to one cpu by
>>>>> +	 * setting the affinity hint to eliminate the contention.
>>>>> +	 */
>>>>> +	if (vi->curr_queue_pairs == 1 ||
>>>>> +	    vi->max_queue_pairs != num_online_cpus()) {
>>>>> +		if (vi->affinity_hint_set)
>>>>> +			virtnet_clean_affinity(vi, -1);
>>>>> +		else
>>>>> +			return;
>>>>> +	}
>>>>> +
>>>>> +	i = 0;
>>>>> +	for_each_online_cpu(cpu) {
>>>>> +		virtqueue_set_affinity(vi->rq[i].vq, cpu);
>>>>> +		virtqueue_set_affinity(vi->sq[i].vq, cpu);
>>>>> +		*per_cpu_ptr(vi->vq_index, cpu) = i;
>>>>> +		i++;
>>>>> +	}
>>>>> +
>>>>> +	vi->affinity_hint_set = true;
>>>>> +}
>>>>> +
>>>>>  static void virtnet_get_ringparam(struct net_device *dev,
>>>>>  				struct ethtool_ringparam *ring)
>>>>>  {
>>>>> @@ -1105,7 +1114,7 @@ static int virtnet_set_channels(struct net_device *dev,
>>>>>  		netif_set_real_num_rx_queues(dev, queue_pairs);
>>>>>  
>>>>>  		get_online_cpus();
>>>>> -		virtnet_set_affinity(vi, true);
>>>>> +		virtnet_set_affinity(vi);
>>>>>  		put_online_cpus();
>>>>>  	}
>>>>>  
>>>>> @@ -1274,7 +1283,7 @@ static void virtnet_del_vqs(struct virtnet_info *vi)
>>>>>  {
>>>>>  	struct virtio_device *vdev = vi->vdev;
>>>>>  
>>>>> -	virtnet_set_affinity(vi, false);
>>>>> +	virtnet_clean_affinity(vi, -1);
>>>>>  
>>>>>  	vdev->config->del_vqs(vdev);
>>>>>  
>>>>> @@ -1398,7 +1407,7 @@ static int init_vqs(struct virtnet_info *vi)
>>>>>  		goto err_free;
>>>>>  
>>>>>  	get_online_cpus();
>>>>> -	virtnet_set_affinity(vi, true);
>>>>> +	virtnet_set_affinity(vi);
>>>>>  	put_online_cpus();
>>>>>  
>>>>>  	return 0;
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply

* Re: Doubts about listen backlog and tcp_max_syn_backlog
From: Nivedita SInghvi @ 2013-01-25  6:12 UTC (permalink / raw)
  To: Leandro Lucarella; +Cc: Rick Jones, Eric Dumazet, netdev, linux-kernel
In-Reply-To: <20130124192125.GD4608@sociomantic.com>

On 01/24/2013 11:21 AM, Leandro Lucarella wrote:
> On Thu, Jan 24, 2013 at 10:44:32AM -0800, Rick Jones wrote:
>> On 01/24/2013 04:22 AM, Leandro Lucarella wrote:
>>> On Wed, Jan 23, 2013 at 11:28:08AM -0800, Rick Jones wrote:
>>>>> Then if syncookies are enabled, the time spent in connect() shouldn't be
>>>>> bigger than 3 seconds even if SYNs are being "dropped" by listen, right?
>>>>
>>>> Do you mean if "ESTABLISHED" connections are dropped because the
>>>> listen queue is full?  I don't think I would put that as "SYNs being
>>>> dropped by listen" - too easy to confuse that with an actual
>>>> dropping of a SYN segment.
>>>
>>> I was just kind of quoting the name given by netstat: "SYNs to LISTEN
>>> sockets dropped" (for kernel 3.0, I noticed newer kernels don't have
>>> this stat anymore, or the name was changed). I still don't know if we
>>> are talking about the same thing.
>>
[snip]
>> I will sometimes be tripped-up by netstat's not showing a statistic
>> with a zero value...

Leandro, you should be able to do an nstat -z, it will print all counters even if zero. You should see something like so:

ipv4]> nstat -z
#kernel
IpInReceives                    2135               0.0
IpInHdrErrors                   0                  0.0
IpInAddrErrors                  202                0.0
...

You might want to take a look at those (your pkts may not even be making it to tcp) and these in particular:

TcpExtSyncookiesSent            0                  0.0
TcpExtSyncookiesRecv            0                  0.0
TcpExtSyncookiesFailed          0                  0.0
TcpExtListenOverflows           0                  0.0
TcpExtListenDrops               0                  0.0
TcpExtTCPBacklogDrop            0                  0.0
TcpExtTCPMinTTLDrop             0                  0.0
TcpExtTCPDeferAcceptDrop        0                  0.0

If you don't have nstat on that version for some reason, download the latest iproute pkg. Looking at the counter names is a lot more helpful and precise than the netstat converstion to human consumption. 


> Yes, I already did captures and we are definitely loosing packets
> (including SYNs), but it looks like the amount of SYNs I'm loosing is
> lower than the amount of long connect() times I observe. This is not
> confirmed yet, I'm still investigating.

Where did you narrow down the drop to? There are quite a few places in the networking stack we silently drop packets (such as the one pointed out earlier in this thread), although they should almost all be extremely low probability/NEVER type events. Do you want a patch to gap the most likely scenario? (I'll post that to netdev separately). 

thanks,
Nivedita

^ permalink raw reply

* Re: [PATCH 1/2] ARM: davinci: da850: add DT node for eth0.
From: Sekhar Nori @ 2013-01-25  6:23 UTC (permalink / raw)
  To: Prabhakar Lad
  Cc: LAK, DLOS, LKML, Lad, Prabhakar, netdev, devicetree-discuss,
	Heiko Schocher
In-Reply-To: <1358749759-20517-2-git-send-email-prabhakar.lad@ti.com>

Prabhakar,

On 1/21/2013 11:59 AM, Prabhakar Lad wrote:
> From: Lad, Prabhakar <prabhakar.lad@ti.com>
> 
> Add eth0 device tree node information to da850 by
> providing interrupt details and local mac address of eth0.
> 
> Signed-off-by: Lad, Prabhakar <prabhakar.lad@ti.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> Cc: davinci-linux-open-source@linux.davincidsp.com
> Cc: netdev@vger.kernel.org
> Cc: devicetree-discuss@lists.ozlabs.org
> Cc: Sekhar Nori <nsekhar@ti.com>
> Cc: Heiko Schocher <hs@denx.de>
> ---
>  arch/arm/boot/dts/da850-evm.dts |    3 +++
>  arch/arm/boot/dts/da850.dtsi    |   15 +++++++++++++++

Can you rebase this to apply to v3.9/dt branch of my tree?

>  2 files changed, 18 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/da850-evm.dts b/arch/arm/boot/dts/da850-evm.dts
> index 37dc5a3..a1d6e3e 100644
> --- a/arch/arm/boot/dts/da850-evm.dts
> +++ b/arch/arm/boot/dts/da850-evm.dts
> @@ -24,5 +24,8 @@
>  		serial2: serial@1d0d000 {
>  			status = "okay";
>  		};
> +		eth0: emac@1e20000 {
> +			status = "okay";
> +		};
>  	};
>  };
> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
> index 640ab75..309cc99 100644
> --- a/arch/arm/boot/dts/da850.dtsi
> +++ b/arch/arm/boot/dts/da850.dtsi
> @@ -56,5 +56,20 @@
>  			interrupt-parent = <&intc>;
>  			status = "disabled";
>  		};
> +		eth0: emac@1e20000 {
> +			compatible = "ti,davinci-dm6467-emac";
> +			reg = <0x220000 0x4000>;
> +			ti,davinci-ctrl-reg-offset = <0x3000>;
> +			ti,davinci-ctrl-mod-reg-offset = <0x2000>;
> +			ti,davinci-ctrl-ram-offset = <0>;
> +			ti,davinci-ctrl-ram-size = <0x2000>;
> +			local-mac-address = [ 00 00 00 00 00 00 ];
> +			interrupts = <33
> +					34
> +					35
> +					36
> +					>;
> +			interrupt-parent = <&intc>;

instead of setting interrupt-parent for each node, this can be done once
for soc and child nodes should inherit this property. This needs to be
fixed for existing serial nodes too.

Also, can you take care of pinmux too? we should not be relying on
u-boot to setup the pinmux for us.

Thanks,
Sekhar

^ permalink raw reply

* Re: [PATCH 2/2] ARM: davinci: da850: add OF_DEV_AUXDATA entry for eth0.
From: Sekhar Nori @ 2013-01-25  6:25 UTC (permalink / raw)
  To: Prabhakar Lad
  Cc: LAK, DLOS, LKML, Lad, Prabhakar, netdev, devicetree-discuss,
	Heiko Schocher
In-Reply-To: <1358749759-20517-3-git-send-email-prabhakar.lad@ti.com>



On 1/21/2013 11:59 AM, Prabhakar Lad wrote:
> From: Lad, Prabhakar <prabhakar.lad@ti.com>
> 
> Add OF_DEV_AUXDATA for eth0  driver in da850 board dt
> file to use emac clock.
> 
> Signed-off-by: Lad, Prabhakar <prabhakar.lad@ti.com>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> Cc: davinci-linux-open-source@linux.davincidsp.com
> Cc: netdev@vger.kernel.org
> Cc: devicetree-discuss@lists.ozlabs.org
> Cc: Sekhar Nori <nsekhar@ti.com>
> Cc: Heiko Schocher <hs@denx.de>
> ---
>  arch/arm/mach-davinci/da8xx-dt.c |    9 ++++++++-
>  1 files changed, 8 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/mach-davinci/da8xx-dt.c b/arch/arm/mach-davinci/da8xx-dt.c
> index 37c27af..d548a38 100644
> --- a/arch/arm/mach-davinci/da8xx-dt.c
> +++ b/arch/arm/mach-davinci/da8xx-dt.c
> @@ -37,11 +37,18 @@ static void __init da8xx_init_irq(void)
>  	of_irq_init(da8xx_irq_match);
>  }
>  
> +struct of_dev_auxdata da850_evm_auxdata_lookup[] __initdata = {

This is specific to da850 and not da850_evm so it should be named so.
You can call it da8xx_auxdata keeping with the file naming.

Thanks,
Sekhar

^ 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