Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next 4/4] r8152: support firmware files
From: Daniele Forsi @ 2014-08-20 13:32 UTC (permalink / raw)
  To: Hayes Wang
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, nic_swsd,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, USB list
In-Reply-To: <0835B3720019904CB8F7AA43166CEEB251585A-JIZ+AM9kKNzuvTFwvkocLypo8c9IxeqyAjHCUHv49ws@public.gmane.org>

2014-08-20 14:35 GMT+02:00 Hayes Wang:

> Is this necessary? Besides the check of the "start",
> there are checks of the "size" and rtl_fw_data_ok().
> I think they cover the situations which you indicate.

it's not necessary and it's better checked later as you did!

-- 
Daniele Forsi
--
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

* [PATCH] net: ethernet: broadcom: bnx2x: Remove redundant #ifdef
From: Rasmus Villemoes @ 2014-08-20 13:14 UTC (permalink / raw)
  To: Ariel Elior; +Cc: netdev, linux-kernel, Rasmus Villemoes

Nothing defines _ASM_GENERIC_INT_L64_H, it is a weird way to check for
64 bit longs, and u64 should be printed using %llx anyway.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
index 4e6c82e..4ccc806 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_cmn.c
@@ -483,11 +483,7 @@ static void bnx2x_tpa_start(struct bnx2x_fastpath *fp, u16 queue,
 
 #ifdef BNX2X_STOP_ON_ERROR
 	fp->tpa_queue_used |= (1 << queue);
-#ifdef _ASM_GENERIC_INT_L64_H
-	DP(NETIF_MSG_RX_STATUS, "fp->tpa_queue_used = 0x%lx\n",
-#else
 	DP(NETIF_MSG_RX_STATUS, "fp->tpa_queue_used = 0x%llx\n",
-#endif
 	   fp->tpa_queue_used);
 #endif
 }
-- 
1.9.2

^ permalink raw reply related

* Re: [patch net-next] bonding: create netlink event when bonding option is changed
From: Jiri Pirko @ 2014-08-20 12:49 UTC (permalink / raw)
  To: Neil Horman
  Cc: netdev, davem, j.vosburgh, vfalico, andy, nikolay, dingtianhong,
	sfeldma
In-Reply-To: <20140820121828.GA31336@hmsreliant.think-freely.org>

Wed, Aug 20, 2014 at 02:18:28PM CEST, nhorman@tuxdriver.com wrote:
>On Tue, Aug 19, 2014 at 04:02:12PM +0200, Jiri Pirko wrote:
>> Userspace needs to be notified if one changes some option.
>> 
>> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
>> ---
>>  drivers/net/bonding/bond_options.c | 2 ++
>>  include/linux/netdevice.h          | 1 +
>>  2 files changed, 3 insertions(+)
>> 
>> diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
>> index dc73463..d8dc17f 100644
>> --- a/drivers/net/bonding/bond_options.c
>> +++ b/drivers/net/bonding/bond_options.c
>> @@ -625,6 +625,8 @@ int __bond_opt_set(struct bonding *bond,
>>  out:
>>  	if (ret)
>>  		bond_opt_error_interpret(bond, opt, ret, val);
>> +	else
>> +		call_netdevice_notifiers(NETDEV_CHANGEINFODATA, bond->dev);
>>  
>>  	return ret;
>>  }
>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>> index 967ddcc..44bc4bd 100644
>> --- a/include/linux/netdevice.h
>> +++ b/include/linux/netdevice.h
>> @@ -2017,6 +2017,7 @@ struct pcpu_sw_netstats {
>>  #define NETDEV_CHANGEUPPER	0x0015
>>  #define NETDEV_RESEND_IGMP	0x0016
>>  #define NETDEV_PRECHANGEMTU	0x0017 /* notify before mtu change happened */
>> +#define NETDEV_CHANGEINFODATA	0x0018
>>  
>Do you need to create a new netdev event here?  Seems like it might be more
>concise to reuse NETDEV_CHANGE and expand the netdev_notifier_change_info
>struct.

No, I would not like to do that. There are many notifiers processing
NETDEV_CHANGE now. Plus there is no limitation in adding event number.
Plus since this it not part of any api, it can be easily changed. I
prefer to add another event number.

>
>Neil
>
>>  int register_netdevice_notifier(struct notifier_block *nb);
>>  int unregister_netdevice_notifier(struct notifier_block *nb);
>> -- 
>> 1.9.3
>> 
>> --
>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> 

^ permalink raw reply

* RE: [PATCH net-next 4/4] r8152: support firmware files
From: Hayes Wang @ 2014-08-20 12:35 UTC (permalink / raw)
  To: Daniele Forsi
  Cc: netdev@vger.kernel.org, nic_swsd, linux-kernel@vger.kernel.org,
	USB list
In-Reply-To: <CAN_we7P2LDvR8miAsoSeL6+iEOK+ucgjRDC8jtPbc4_-2nn7MQ@mail.gmail.com>

 Daniele Forsi [mailto:dforsi@gmail.com] 
> Sent: Wednesday, August 20, 2014 8:01 PM
> To: Hayes Wang
> Cc: netdev@vger.kernel.org; nic_swsd; 
> linux-kernel@vger.kernel.org; USB list
> Subject: Re: [PATCH net-next 4/4] r8152: support firmware files
[...]
> > +       start = le32_to_cpu(fw_header->fw_start);
> > +       if (start > fw->size)
> > +               goto out;
> 
> since "start" is an offset in an array of size "fw->size" this should
> check for ">=" and if a command is at least cmd(2 bytes) + length(2
> bytes), shouldn't this check for "start >= fw->size - 4"?

Is this necessary? Besides the check of the "start",
there are checks of the "size" and rtl_fw_data_ok().
I think they cover the situations which you indicate.
 
Best Regards,
Hayes

^ permalink raw reply

* Re: [patch net-next] bonding: create netlink event when bonding option is changed
From: Neil Horman @ 2014-08-20 12:18 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: netdev, davem, j.vosburgh, vfalico, andy, nikolay, dingtianhong,
	sfeldma
In-Reply-To: <1408456932-10318-1-git-send-email-jiri@resnulli.us>

On Tue, Aug 19, 2014 at 04:02:12PM +0200, Jiri Pirko wrote:
> Userspace needs to be notified if one changes some option.
> 
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
> ---
>  drivers/net/bonding/bond_options.c | 2 ++
>  include/linux/netdevice.h          | 1 +
>  2 files changed, 3 insertions(+)
> 
> diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c
> index dc73463..d8dc17f 100644
> --- a/drivers/net/bonding/bond_options.c
> +++ b/drivers/net/bonding/bond_options.c
> @@ -625,6 +625,8 @@ int __bond_opt_set(struct bonding *bond,
>  out:
>  	if (ret)
>  		bond_opt_error_interpret(bond, opt, ret, val);
> +	else
> +		call_netdevice_notifiers(NETDEV_CHANGEINFODATA, bond->dev);
>  
>  	return ret;
>  }
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 967ddcc..44bc4bd 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -2017,6 +2017,7 @@ struct pcpu_sw_netstats {
>  #define NETDEV_CHANGEUPPER	0x0015
>  #define NETDEV_RESEND_IGMP	0x0016
>  #define NETDEV_PRECHANGEMTU	0x0017 /* notify before mtu change happened */
> +#define NETDEV_CHANGEINFODATA	0x0018
>  
Do you need to create a new netdev event here?  Seems like it might be more
concise to reuse NETDEV_CHANGE and expand the netdev_notifier_change_info
struct.

Neil

>  int register_netdevice_notifier(struct notifier_block *nb);
>  int unregister_netdevice_notifier(struct notifier_block *nb);
> -- 
> 1.9.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply

* Re: [PATCH net-next 4/4] r8152: support firmware files
From: Daniele Forsi @ 2014-08-20 12:01 UTC (permalink / raw)
  To: Hayes Wang; +Cc: netdev, nic_swsd, linux-kernel, USB list
In-Reply-To: <1394712342-15778-20-Taiwan-albertk@realtek.com>

2014-08-20 10:58 GMT+02:00 Hayes Wang:

> The firmware file is composed of the fw header and the commands. Each
> command has the following type.
>
>         cmd(2 bytes) + length(2 bytes) + data(variable bytes)

> +static bool rtl_fw_format_ok(struct rtl_fw *rtl_fw)

> +       start = le32_to_cpu(fw_header->fw_start);
> +       if (start > fw->size)
> +               goto out;

since "start" is an offset in an array of size "fw->size" this should
check for ">=" and if a command is at least cmd(2 bytes) + length(2
bytes), shouldn't this check for "start >= fw->size - 4"?

-- 
Daniele Forsi

^ permalink raw reply

* Re: [PATCH net-next 3/4] r8152: remove clear_bp function
From: Sergei Shtylyov @ 2014-08-20 12:00 UTC (permalink / raw)
  To: Hayes Wang, netdev; +Cc: nic_swsd, linux-kernel, linux-usb
In-Reply-To: <1394712342-15778-19-Taiwan-albertk@realtek.com>

Hello.

On 8/20/2014 12:58 PM, Hayes Wang wrote:

> The functions are used to update the firmware. Move the actions into
> the firmware files.

> Signed-off-by: Hayes Wang <hayeswang@realtek.com>
> ---
>   drivers/net/usb/r8152.c | 24 ------------------------
>   1 file changed, 24 deletions(-)

> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index 33dcc97..937d132 100644
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c
[...]
> @@ -2248,7 +2226,6 @@ static void r8152b_hw_phy_cfg(struct r8152 *tp)
>
>   	r8152b_disable_aldps(tp);
>
> -	rtl_clear_bp(tp);
>

    Why leave 2 empty lines? One is enough.

>   	r8152b_enable_aldps(tp);
>   	set_bit(PHY_RESET, &tp->flags);
> @@ -2404,7 +2381,6 @@ static void r8153_hw_phy_cfg(struct r8152 *tp)
>   		r8152_mdio_write(tp, MII_BMCR, data);
>   	}
>
> -	r8153_clear_bp(tp);
>

    Ditto.

>   	if (tp->version == RTL_VER_03) {
>   		data = ocp_reg_read(tp, OCP_EEE_CFG);
>

WBR, Sergei

^ permalink raw reply

* Re: [RFC 2/4] tuntap: Publish tuntap maximum number of queues as module_param
From: Jiri Pirko @ 2014-08-20 11:53 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Pankaj Gupta, linux-kernel, netdev, davem, jasowang, dgibson,
	vfalico, edumazet, vyasevic, hkchu, wuzhy, xemul, therbert,
	bhutchings, xii, stephen
In-Reply-To: <20140820114907.GA17943@redhat.com>

Wed, Aug 20, 2014 at 01:49:07PM CEST, mst@redhat.com wrote:
>On Wed, Aug 20, 2014 at 01:46:20PM +0200, Jiri Pirko wrote:
>> Wed, Aug 20, 2014 at 01:17:24PM CEST, mst@redhat.com wrote:
>> >On Wed, Aug 20, 2014 at 12:58:17PM +0200, Jiri Pirko wrote:
>> >> Mon, Aug 18, 2014 at 03:37:18PM CEST, pagupta@redhat.com wrote:
>> >> > This patch publishes maximum number of tun/tap queues allocated as a
>> >> > read_only module parameter which a user space application like libvirt
>> >> > can make use of to limit maximum number of queues. Value of read_only
>> >> > module parameter can be writable only at module load time. If no value is set
>> >> > at module load time a default value 256 is used which is equal to maximum number
>> >> > of vCPUS allowed by KVM.
>> >> >
>> >> > Administrator can specify maximum number of queues only at the driver
>> >> > module load time.
>> >> >
>> >> >Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
>> >> >---
>> >> > drivers/net/tun.c |   13 +++++++++++--
>> >> > 1 files changed, 11 insertions(+), 2 deletions(-)
>> >> >
>> >> >diff --git a/drivers/net/tun.c b/drivers/net/tun.c
>> >> >index acaaf67..1f518e2 100644
>> >> >--- a/drivers/net/tun.c
>> >> >+++ b/drivers/net/tun.c
>> >> >@@ -119,6 +119,9 @@ struct tap_filter {
>> >> > 
>> >> > #define TUN_FLOW_EXPIRE (3 * HZ)
>> >> > 
>> >> >+static int max_tap_queues = MAX_TAP_QUEUES;
>> >> >+module_param(max_tap_queues, int, S_IRUGO);
>> >> 
>> >> Please do not introduce new module paramaters. Please other ways to
>> >> interchange values with userspace.
>> >
>> >I suggested this initially, but thinking more about it, I agree.
>> >
>> >It's a global limit (necessary to limit memory utilization by
>> >userspace), but it should be possible to change it
>> >after module load.
>> >
>> >Additionally, userspace that has the FD should be able to
>> >retrieve the value without guessing that the FD is
>> >for the tun device (and not e.g. macvtap).
>> >To retrieve the value, an ioctl is probably the
>> >cleanest approach.
>> >
>> >To set it, how about a sysctl? I think the limit can also apply to
>> >all devices, not just tun.
>> 
>> Or netlink?
>
>Are there examples of netlink being used to set global defaults
>as opposed to per-device parameters?

That is so far not possible. But I believe that it can be implemented.
I'm just thinking out loud.

>
>> >
>> >-- 
>> >MST

^ permalink raw reply

* Re: [RFC 2/4] tuntap: Publish tuntap maximum number of queues as module_param
From: Michael S. Tsirkin @ 2014-08-20 11:49 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: Pankaj Gupta, linux-kernel, netdev, davem, jasowang, dgibson,
	vfalico, edumazet, vyasevic, hkchu, wuzhy, xemul, therbert,
	bhutchings, xii, stephen
In-Reply-To: <20140820114620.GC1916@nanopsycho.orion>

On Wed, Aug 20, 2014 at 01:46:20PM +0200, Jiri Pirko wrote:
> Wed, Aug 20, 2014 at 01:17:24PM CEST, mst@redhat.com wrote:
> >On Wed, Aug 20, 2014 at 12:58:17PM +0200, Jiri Pirko wrote:
> >> Mon, Aug 18, 2014 at 03:37:18PM CEST, pagupta@redhat.com wrote:
> >> > This patch publishes maximum number of tun/tap queues allocated as a
> >> > read_only module parameter which a user space application like libvirt
> >> > can make use of to limit maximum number of queues. Value of read_only
> >> > module parameter can be writable only at module load time. If no value is set
> >> > at module load time a default value 256 is used which is equal to maximum number
> >> > of vCPUS allowed by KVM.
> >> >
> >> > Administrator can specify maximum number of queues only at the driver
> >> > module load time.
> >> >
> >> >Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
> >> >---
> >> > drivers/net/tun.c |   13 +++++++++++--
> >> > 1 files changed, 11 insertions(+), 2 deletions(-)
> >> >
> >> >diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> >> >index acaaf67..1f518e2 100644
> >> >--- a/drivers/net/tun.c
> >> >+++ b/drivers/net/tun.c
> >> >@@ -119,6 +119,9 @@ struct tap_filter {
> >> > 
> >> > #define TUN_FLOW_EXPIRE (3 * HZ)
> >> > 
> >> >+static int max_tap_queues = MAX_TAP_QUEUES;
> >> >+module_param(max_tap_queues, int, S_IRUGO);
> >> 
> >> Please do not introduce new module paramaters. Please other ways to
> >> interchange values with userspace.
> >
> >I suggested this initially, but thinking more about it, I agree.
> >
> >It's a global limit (necessary to limit memory utilization by
> >userspace), but it should be possible to change it
> >after module load.
> >
> >Additionally, userspace that has the FD should be able to
> >retrieve the value without guessing that the FD is
> >for the tun device (and not e.g. macvtap).
> >To retrieve the value, an ioctl is probably the
> >cleanest approach.
> >
> >To set it, how about a sysctl? I think the limit can also apply to
> >all devices, not just tun.
> 
> Or netlink?

Are there examples of netlink being used to set global defaults
as opposed to per-device parameters?

> >
> >-- 
> >MST

^ permalink raw reply

* Re: [RFC 2/4] tuntap: Publish tuntap maximum number of queues as module_param
From: Jiri Pirko @ 2014-08-20 11:46 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Pankaj Gupta, linux-kernel, netdev, davem, jasowang, dgibson,
	vfalico, edumazet, vyasevic, hkchu, wuzhy, xemul, therbert,
	bhutchings, xii, stephen
In-Reply-To: <20140820111724.GE17371@redhat.com>

Wed, Aug 20, 2014 at 01:17:24PM CEST, mst@redhat.com wrote:
>On Wed, Aug 20, 2014 at 12:58:17PM +0200, Jiri Pirko wrote:
>> Mon, Aug 18, 2014 at 03:37:18PM CEST, pagupta@redhat.com wrote:
>> > This patch publishes maximum number of tun/tap queues allocated as a
>> > read_only module parameter which a user space application like libvirt
>> > can make use of to limit maximum number of queues. Value of read_only
>> > module parameter can be writable only at module load time. If no value is set
>> > at module load time a default value 256 is used which is equal to maximum number
>> > of vCPUS allowed by KVM.
>> >
>> > Administrator can specify maximum number of queues only at the driver
>> > module load time.
>> >
>> >Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
>> >---
>> > drivers/net/tun.c |   13 +++++++++++--
>> > 1 files changed, 11 insertions(+), 2 deletions(-)
>> >
>> >diff --git a/drivers/net/tun.c b/drivers/net/tun.c
>> >index acaaf67..1f518e2 100644
>> >--- a/drivers/net/tun.c
>> >+++ b/drivers/net/tun.c
>> >@@ -119,6 +119,9 @@ struct tap_filter {
>> > 
>> > #define TUN_FLOW_EXPIRE (3 * HZ)
>> > 
>> >+static int max_tap_queues = MAX_TAP_QUEUES;
>> >+module_param(max_tap_queues, int, S_IRUGO);
>> 
>> Please do not introduce new module paramaters. Please other ways to
>> interchange values with userspace.
>
>I suggested this initially, but thinking more about it, I agree.
>
>It's a global limit (necessary to limit memory utilization by
>userspace), but it should be possible to change it
>after module load.
>
>Additionally, userspace that has the FD should be able to
>retrieve the value without guessing that the FD is
>for the tun device (and not e.g. macvtap).
>To retrieve the value, an ioctl is probably the
>cleanest approach.
>
>To set it, how about a sysctl? I think the limit can also apply to
>all devices, not just tun.

Or netlink?

>
>-- 
>MST

^ permalink raw reply

* Re: [RFC 2/4] tuntap: Publish tuntap maximum number of queues as module_param
From: Michael S. Tsirkin @ 2014-08-20 11:17 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: Pankaj Gupta, linux-kernel, netdev, davem, jasowang, dgibson,
	vfalico, edumazet, vyasevic, hkchu, wuzhy, xemul, therbert,
	bhutchings, xii, stephen
In-Reply-To: <20140820105817.GB1916@nanopsycho.orion>

On Wed, Aug 20, 2014 at 12:58:17PM +0200, Jiri Pirko wrote:
> Mon, Aug 18, 2014 at 03:37:18PM CEST, pagupta@redhat.com wrote:
> > This patch publishes maximum number of tun/tap queues allocated as a
> > read_only module parameter which a user space application like libvirt
> > can make use of to limit maximum number of queues. Value of read_only
> > module parameter can be writable only at module load time. If no value is set
> > at module load time a default value 256 is used which is equal to maximum number
> > of vCPUS allowed by KVM.
> >
> > Administrator can specify maximum number of queues only at the driver
> > module load time.
> >
> >Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
> >---
> > drivers/net/tun.c |   13 +++++++++++--
> > 1 files changed, 11 insertions(+), 2 deletions(-)
> >
> >diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> >index acaaf67..1f518e2 100644
> >--- a/drivers/net/tun.c
> >+++ b/drivers/net/tun.c
> >@@ -119,6 +119,9 @@ struct tap_filter {
> > 
> > #define TUN_FLOW_EXPIRE (3 * HZ)
> > 
> >+static int max_tap_queues = MAX_TAP_QUEUES;
> >+module_param(max_tap_queues, int, S_IRUGO);
> 
> Please do not introduce new module paramaters. Please other ways to
> interchange values with userspace.

I suggested this initially, but thinking more about it, I agree.

It's a global limit (necessary to limit memory utilization by
userspace), but it should be possible to change it
after module load.

Additionally, userspace that has the FD should be able to
retrieve the value without guessing that the FD is
for the tun device (and not e.g. macvtap).
To retrieve the value, an ioctl is probably the
cleanest approach.

To set it, how about a sysctl? I think the limit can also apply to
all devices, not just tun.

-- 
MST

^ permalink raw reply

* Re: [patch net-next] bonding: create netlink event when bonding option is changed
From: Veaceslav Falico @ 2014-08-20 11:11 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: Andy Gospodarek, netdev, davem, j.vosburgh, andy, nikolay,
	dingtianhong, sfeldma
In-Reply-To: <20140820064458.GA1916@nanopsycho.orion>

On Wed, Aug 20, 2014 at 08:44:58AM +0200, Jiri Pirko wrote:
>Tue, Aug 19, 2014 at 10:46:34PM CEST, gospo@cumulusnetworks.com wrote:
>>On Tue, Aug 19, 2014 at 10:37:48PM +0200, Jiri Pirko wrote:
>>> Tue, Aug 19, 2014 at 10:34:01PM CEST, gospo@cumulusnetworks.com wrote:
>>> >On Tue, Aug 19, 2014 at 04:02:12PM +0200, Jiri Pirko wrote:
...snip...
>>> >>  	if (ret)
>>> >>  		bond_opt_error_interpret(bond, opt, ret, val);
>>> >> +	else
>>> >> +		call_netdevice_notifiers(NETDEV_CHANGEINFODATA, bond->dev);
...snip...
>>My question was:  Is there a need for 2 netlink messages to indicate
>>that a bonding configuration change was needed when the config came over
>>netlink.  I did not see the need for 2 netlink messages in this case and
>>was asking for clarification from you.
>
>Imagine 2 applications. One is just monitoring netlink events ("ip
>mon"), the second is setting up bond via netlink. Now, the second sends
>a message to kernel, that is unicast to kernel. The first application
>does not see that message. Therefore there is need to generate the event
>message in kernel and send it back to userspace via multicast.
>That message the first application will see. And that is exactly what
>my patch is doing.

Seems fair, as there's no other way to catch those modifications, even
though they're known to the "changing" application.

Acked-by: Veaceslav Falico <vfalico@gmail.com>

^ permalink raw reply

* Re: [PATCH] vhost: Add polling mode
From: Michael S. Tsirkin @ 2014-08-20 11:05 UTC (permalink / raw)
  To: Razya Ladelsky
  Cc: Eran Raichstein, kvm-owner, kvm, linux-kernel, abel.gordon,
	Alex Glikson, Yossi Kuperman1, Joel Nider, netdev, virtualization,
	David Miller
In-Reply-To: <OFB79DEF3C.1ADC0F14-ONC2257D39.002F0F0A-C2257D39.002F3CBC@il.ibm.com>

On Tue, Aug 19, 2014 at 11:36:31AM +0300, Razya Ladelsky wrote:
> > That was just one example. There many other possibilities.  Either
> > actually make the systems load all host CPUs equally, or divide
> > throughput by host CPU.
> > 
> 
> The polling patch adds this capability to vhost, reducing costly exit 
> overhead when the vm is loaded.
> 
> In order to load the vm I ran netperf  with msg size of 256:
> 
> Without polling:  2480 Mbits/sec,  utilization: vm - 100%   vhost - 64% 
> With Polling: 4160 Mbits/sec,  utilization: vm - 100%   vhost - 100% 
> 
> Therefore, throughput/cpu without polling is 15.1, and 20.8 with polling.
> 

Can you please present results in a form that makes
it possible to see the effect on various configurations
and workloads?

Here's one example where this was done:
https://lkml.org/lkml/2014/8/14/495

You really should also provide data about your host
configuration (missing in the above link).

> My intention was to load vhost as close as possible to 100% utilization 
> without polling, in order to compare it to the polling utilization case 
> (where vhost is always 100%). 
> The best use case, of course, would be when the shared vhost thread work 
> (TBD) is integrated and then vhost will actually be using its polling 
> cycles to handle requests of multiple devices (even from multiple vms).
> 
> Thanks,
> Razya


-- 
MST

^ permalink raw reply

* Re: [RFC 2/4] tuntap: Publish tuntap maximum number of queues as module_param
From: Jiri Pirko @ 2014-08-20 10:58 UTC (permalink / raw)
  To: Pankaj Gupta
  Cc: linux-kernel, netdev, davem, jasowang, mst, dgibson, vfalico,
	edumazet, vyasevic, hkchu, wuzhy, xemul, therbert, bhutchings,
	xii, stephen
In-Reply-To: <1408369040-1216-3-git-send-email-pagupta@redhat.com>

Mon, Aug 18, 2014 at 03:37:18PM CEST, pagupta@redhat.com wrote:
> This patch publishes maximum number of tun/tap queues allocated as a
> read_only module parameter which a user space application like libvirt
> can make use of to limit maximum number of queues. Value of read_only
> module parameter can be writable only at module load time. If no value is set
> at module load time a default value 256 is used which is equal to maximum number
> of vCPUS allowed by KVM.
>
> Administrator can specify maximum number of queues only at the driver
> module load time.
>
>Signed-off-by: Pankaj Gupta <pagupta@redhat.com>
>---
> drivers/net/tun.c |   13 +++++++++++--
> 1 files changed, 11 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/net/tun.c b/drivers/net/tun.c
>index acaaf67..1f518e2 100644
>--- a/drivers/net/tun.c
>+++ b/drivers/net/tun.c
>@@ -119,6 +119,9 @@ struct tap_filter {
> 
> #define TUN_FLOW_EXPIRE (3 * HZ)
> 
>+static int max_tap_queues = MAX_TAP_QUEUES;
>+module_param(max_tap_queues, int, S_IRUGO);

Please do not introduce new module paramaters. Please other ways to
interchange values with userspace.

^ permalink raw reply

* Re: [PATCH] vhost: Add polling mode
From: Michael S. Tsirkin @ 2014-08-20 10:57 UTC (permalink / raw)
  To: Razya Ladelsky
  Cc: ERANRA, kvm, linux-kernel, GLIKSON, abel.gordon, YOSSIKU, JOELN,
	netdev, virtualization
In-Reply-To: <20140810083035.0CF58380729@moren.haifa.ibm.com>

On Sun, Aug 10, 2014 at 11:30:35AM +0300, Razya Ladelsky wrote:
> From: Razya Ladelsky <razya@il.ibm.com>
> Date: Thu, 31 Jul 2014 09:47:20 +0300
> Subject: [PATCH] vhost: Add polling mode
> 
> When vhost is waiting for buffers from the guest driver (e.g., more packets to
> send in vhost-net's transmit queue), it normally goes to sleep and waits for the
> guest to "kick" it. This kick involves a PIO in the guest, and therefore an exit
> (and possibly userspace involvement in translating this PIO exit into a file
> descriptor event), all of which hurts performance.
> 
> If the system is under-utilized (has cpu time to spare), vhost can continuously
> poll the virtqueues for new buffers, and avoid asking the guest to kick us.
> This patch adds an optional polling mode to vhost, that can be enabled via a
> kernel module parameter, "poll_start_rate".
> 
> When polling is active for a virtqueue, the guest is asked to disable
> notification (kicks), and the worker thread continuously checks for new buffers.
> When it does discover new buffers, it simulates a "kick" by invoking the
> underlying backend driver (such as vhost-net), which thinks it got a real kick
> from the guest, and acts accordingly. If the underlying driver asks not to be
> kicked, we disable polling on this virtqueue.
> 
> We start polling on a virtqueue when we notice it has work to do. Polling on
> this virtqueue is later disabled after 3 seconds of polling turning up no new
> work, as in this case we are better off returning to the exit-based notification
> mechanism. The default timeout of 3 seconds can be changed with the
> "poll_stop_idle" kernel module parameter.
> 
> This polling approach makes lot of sense for new HW with posted-interrupts for
> which we have exitless host-to-guest notifications. But even with support for
> posted interrupts, guest-to-host communication still causes exits. Polling adds
> the missing part.
> 
> When systems are overloaded, there won't be enough cpu time for the various
> vhost threads to poll their guests' devices. For these scenarios, we plan to add
> support for vhost threads that can be shared by multiple devices, even of
> multiple vms.
> Our ultimate goal is to implement the I/O acceleration features described in:
> KVM Forum 2013: Efficient and Scalable Virtio (by Abel Gordon)
> https://www.youtube.com/watch?v=9EyweibHfEs
> and
> https://www.mail-archive.com/kvm@vger.kernel.org/msg98179.html
> 
> I ran some experiments with TCP stream netperf and filebench (having 2 threads
> performing random reads) benchmarks on an IBM System x3650 M4.
> I have two machines, A and B. A hosts the vms, B runs the netserver.
> The vms (on A) run netperf, its destination server is running on B.
> All runs loaded the guests in a way that they were (cpu) saturated. For example,
> I ran netperf with 64B messages, which is heavily loading the vm (which is why
> its throughput is low).
> The idea was to get it 100% loaded, so we can see that the polling is getting it
> to produce higher throughput.
> 
> The system had two cores per guest, as to allow for both the vcpu and the vhost
> thread to run concurrently for maximum throughput (but I didn't pin the threads
> to specific cores).
> My experiments were fair in a sense that for both cases, with or without
> polling, I run both threads, vcpu and vhost, on 2 cores (set their affinity that
> way). The only difference was whether polling was enabled/disabled.
> 
> Results:
> 
> Netperf, 1 vm:
> The polling patch improved throughput by ~33% (1516 MB/sec -> 2046 MB/sec).
> Number of exits/sec decreased 6x.
> The same improvement was shown when I tested with 3 vms running netperf
> (4086 MB/sec -> 5545 MB/sec).
> 
> filebench, 1 vm:
> ops/sec improved by 13% with the polling patch. Number of exits was reduced by
> 31%.
> The same experiment with 3 vms running filebench showed similar numbers.
> 
> Signed-off-by: Razya Ladelsky <razya@il.ibm.com>

This really needs more thourough benchmarking report, including
system data.  One good example for a related patch:
http://lwn.net/Articles/551179/
though for virtualization, we need data about host as well, and if you
want to look at streaming benchmarks, you need to test different message
sizes and measure packet size.

For now, commenting on the patches assuming that will be forthcoming.

> ---
>  drivers/vhost/net.c   |    6 +-
>  drivers/vhost/scsi.c  |    6 +-
>  drivers/vhost/vhost.c |  245 +++++++++++++++++++++++++++++++++++++++++++++++--
>  drivers/vhost/vhost.h |   38 +++++++-
>  4 files changed, 277 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> index 971a760..558aecb 100644
> --- a/drivers/vhost/net.c
> +++ b/drivers/vhost/net.c
> @@ -742,8 +742,10 @@ static int vhost_net_open(struct inode *inode, struct file *f)
>  	}
>  	vhost_dev_init(dev, vqs, VHOST_NET_VQ_MAX);
>  
> -	vhost_poll_init(n->poll + VHOST_NET_VQ_TX, handle_tx_net, POLLOUT, dev);
> -	vhost_poll_init(n->poll + VHOST_NET_VQ_RX, handle_rx_net, POLLIN, dev);
> +	vhost_poll_init(n->poll + VHOST_NET_VQ_TX, handle_tx_net, POLLOUT,
> +			vqs[VHOST_NET_VQ_TX]);
> +	vhost_poll_init(n->poll + VHOST_NET_VQ_RX, handle_rx_net, POLLIN,
> +			vqs[VHOST_NET_VQ_RX]);
>  
>  	f->private_data = n;
>  
> diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
> index 4f4ffa4..665eeeb 100644
> --- a/drivers/vhost/scsi.c
> +++ b/drivers/vhost/scsi.c
> @@ -1528,9 +1528,9 @@ static int vhost_scsi_open(struct inode *inode, struct file *f)
>  	if (!vqs)
>  		goto err_vqs;
>  
> -	vhost_work_init(&vs->vs_completion_work, vhost_scsi_complete_cmd_work);
> -	vhost_work_init(&vs->vs_event_work, tcm_vhost_evt_work);
> -
> +	vhost_work_init(&vs->vs_completion_work, NULL,
> +						vhost_scsi_complete_cmd_work);
> +	vhost_work_init(&vs->vs_event_work, NULL, tcm_vhost_evt_work);
>  	vs->vs_events_nr = 0;
>  	vs->vs_events_missed = false;
>  
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index c90f437..fbe8174 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -24,9 +24,17 @@
>  #include <linux/slab.h>
>  #include <linux/kthread.h>
>  #include <linux/cgroup.h>
> +#include <linux/jiffies.h>
>  #include <linux/module.h>
>  
>  #include "vhost.h"
> +static int poll_start_rate = 0;
> +module_param(poll_start_rate, int, S_IRUGO|S_IWUSR);
> +MODULE_PARM_DESC(poll_start_rate, "Start continuous polling of virtqueue when rate of events is at least this number per jiffy. If 0, never start polling.");
> +
> +static int poll_stop_idle = 3*HZ; /* 3 seconds */
> +module_param(poll_stop_idle, int, S_IRUGO|S_IWUSR);
> +MODULE_PARM_DESC(poll_stop_idle, "Stop continuous polling of virtqueue after this many jiffies of no work.");
>  
>  enum {
>  	VHOST_MEMORY_MAX_NREGIONS = 64,

So how does one know whether the heuristic works?
We need some kind of counter here.
E.g.  sk_busy_loop uses
                        NET_ADD_STATS_BH(sock_net(sk),
                                         LINUX_MIB_BUSYPOLLRXPACKETS, rc);


> @@ -58,27 +66,28 @@ static int vhost_poll_wakeup(wait_queue_t *wait, unsigned mode, int sync,
>  	return 0;
>  }
>  
> -void vhost_work_init(struct vhost_work *work, vhost_work_fn_t fn)
> +void vhost_work_init(struct vhost_work *work, struct vhost_virtqueue *vq,
> +							vhost_work_fn_t fn)
>  {
>  	INIT_LIST_HEAD(&work->node);
>  	work->fn = fn;
>  	init_waitqueue_head(&work->done);
>  	work->flushing = 0;
>  	work->queue_seq = work->done_seq = 0;
> +	work->vq = vq;
>  }
>  EXPORT_SYMBOL_GPL(vhost_work_init);
>  
>  /* Init poll structure */
>  void vhost_poll_init(struct vhost_poll *poll, vhost_work_fn_t fn,
> -		     unsigned long mask, struct vhost_dev *dev)
> +		     unsigned long mask, struct vhost_virtqueue *vq)
>  {
>  	init_waitqueue_func_entry(&poll->wait, vhost_poll_wakeup);
>  	init_poll_funcptr(&poll->table, vhost_poll_func);
>  	poll->mask = mask;
> -	poll->dev = dev;
> +	poll->dev = vq->dev;
>  	poll->wqh = NULL;
> -
> -	vhost_work_init(&poll->work, fn);
> +	vhost_work_init(&poll->work, vq, fn);
>  }
>  EXPORT_SYMBOL_GPL(vhost_poll_init);
>  
> @@ -174,6 +183,86 @@ void vhost_poll_queue(struct vhost_poll *poll)
>  }
>  EXPORT_SYMBOL_GPL(vhost_poll_queue);
>  
> +/* Enable or disable virtqueue polling (vqpoll.enabled) for a virtqueue.
> + *
> + * Enabling this mode it tells the guest not to notify ("kick") us when its
> + * has made more work available on this virtqueue; Rather, we will continuously
> + * poll this virtqueue in the worker thread. If multiple virtqueues are polled,
> + * the worker thread polls them all, e.g., in a round-robin fashion.
> + * Note that vqpoll.enabled doesn't always mean that this virtqueue is
> + * actually being polled: The backend (e.g., net.c) may temporarily disable it
> + * using vhost_disable/enable_notify(), while vqpoll.enabled is unchanged.
> + *
> + * It is assumed that these functions are called relatively rarely, when vhost
> + * notices that this virtqueue's usage pattern significantly changed in a way
> + * that makes polling more efficient than notification, or vice versa.
> + * Also, we assume that vhost_vq_disable_vqpoll() is always called on vq
> + * cleanup, so any allocations done by vhost_vq_enable_vqpoll() can be
> + * reclaimed.
> + */
> +static void vhost_vq_enable_vqpoll(struct vhost_virtqueue *vq)
> +{
> +	if (vq->vqpoll.enabled)
> +		return; /* already enabled, nothing to do */
> +	if (!vq->handle_kick)
> +		return; /* polling will be a waste of time if no callback! */
> +	if (!(vq->used_flags & VRING_USED_F_NO_NOTIFY)) {
> +		/* vq has guest notifications enabled. Disable them,
> +		   and instead add vq to the polling list */

Pls fix up multiline comment to match coding style.

> +		vhost_disable_notify(vq->dev, vq);
> +		list_add_tail(&vq->vqpoll.link, &vq->dev->vqpoll_list);
> +	}
> +	vq->vqpoll.jiffies_last_kick = jiffies;
> +	__get_user(vq->avail_idx, &vq->avail->idx);
> +	vq->vqpoll.enabled = true;
> +
> +	/* Map userspace's vq->avail to the kernel's memory space. */
> +	if (get_user_pages_fast((unsigned long)vq->avail, 1, 0,
> +		&vq->vqpoll.avail_page) != 1) {
> +		/* TODO: can this happen, as we check access
> +		to vq->avail in advance? */

It can since you don't have the mm lock, so userspace can
unmap the page in this window. And especially since you didn't
check __get_user return code, so you don't even know that
it succeeded in the first place.

> +		BUG();
> +	}
> +	vq->vqpoll.avail_mapped = (struct vring_avail *) (
> +		(unsigned long)kmap(vq->vqpoll.avail_page) |
> +		((unsigned long)vq->avail & ~PAGE_MASK));
> +}
> +
> +/*
> + * This function doesn't always succeed in changing the mode. Sometimes
> + * a temporary race condition prevents turning on guest notifications, so
> + * vq should be polled next time again.
> + */
> +static void vhost_vq_disable_vqpoll(struct vhost_virtqueue *vq)
> +{
> +	if (!vq->vqpoll.enabled)
> +		return; /* already disabled, nothing to do */
> +
> +	vq->vqpoll.enabled = false;
> +
> +	if (!list_empty(&vq->vqpoll.link)) {
> +		/* vq is on the polling list, remove it from this list and
> +		 * instead enable guest notifications. */
> +		list_del_init(&vq->vqpoll.link);
> +		if (unlikely(vhost_enable_notify(vq->dev, vq))
> +			&& !vq->vqpoll.shutdown) {
> +			/* Race condition: guest wrote before we enabled
> +			 * notification, so we'll never get a notification for
> +			 * this work - so continue polling mode for a while. */
> +			vhost_disable_notify(vq->dev, vq);
> +			vq->vqpoll.enabled = true;
> +			vhost_enable_notify(vq->dev, vq);
> +			return;
> +		}
> +	}
> +
> +	if (vq->vqpoll.avail_mapped) {
> +		kunmap(vq->vqpoll.avail_page);
> +		put_page(vq->vqpoll.avail_page);
> +		vq->vqpoll.avail_mapped = 0;
> +	}
> +}
> +
>  static void vhost_vq_reset(struct vhost_dev *dev,
>  			   struct vhost_virtqueue *vq)
>  {
> @@ -199,6 +288,48 @@ static void vhost_vq_reset(struct vhost_dev *dev,
>  	vq->call = NULL;
>  	vq->log_ctx = NULL;
>  	vq->memory = NULL;
> +	INIT_LIST_HEAD(&vq->vqpoll.link);
> +	vq->vqpoll.enabled = false;
> +	vq->vqpoll.shutdown = false;
> +	vq->vqpoll.avail_mapped = NULL;
> +}
> +
> +/* roundrobin_poll() takes worker->vqpoll_list, and returns one of the
> + * virtqueues which the caller should kick, or NULL in case none should be
> + * kicked. roundrobin_poll() also disables polling on a virtqueue which has
> + * been polled for too long without success.
> + *
> + * This current implementation (the "round-robin" implementation) only
> + * polls the first vq in the list, returning it or NULL as appropriate, and
> + * moves this vq to the end of the list, so next time a different one is
> + * polled.
> + */
> +static struct vhost_virtqueue *roundrobin_poll(struct list_head *list)
> +{
> +	struct vhost_virtqueue *vq;
> +	u16 avail_idx;
> +
> +	if (list_empty(list))
> +		return NULL;
> +
> +	vq = list_first_entry(list, struct vhost_virtqueue, vqpoll.link);
> +	WARN_ON(!vq->vqpoll.enabled);
> +	list_move_tail(&vq->vqpoll.link, list);
> +
> +	/* See if there is any new work available from the guest. */
> +	/* TODO: can check the optional idx feature, and if we haven't
> +	* reached that idx yet, don't kick... */
> +	avail_idx = vq->vqpoll.avail_mapped->idx;
> +	if (avail_idx != vq->last_avail_idx)
> +		return vq;
> +
> +	if (jiffies > vq->vqpoll.jiffies_last_kick + poll_stop_idle) {
> +		/* We've been polling this virtqueue for a long time with no
> +		* results, so switch back to guest notification
> +		*/
> +		vhost_vq_disable_vqpoll(vq);
> +	}
> +	return NULL;
>  }
>  
>  static int vhost_worker(void *data)
> @@ -237,12 +368,62 @@ static int vhost_worker(void *data)
>  		spin_unlock_irq(&dev->work_lock);
>  
>  		if (work) {
> +			struct vhost_virtqueue *vq = work->vq;
>  			__set_current_state(TASK_RUNNING);
>  			work->fn(work);
> +			/* Keep track of the work rate, for deciding when to
> +			 * enable polling */
> +			if (vq) {
> +				if (vq->vqpoll.jiffies_last_work != jiffies) {
> +					vq->vqpoll.jiffies_last_work = jiffies;
> +					vq->vqpoll.work_this_jiffy = 0;
> +				}
> +				vq->vqpoll.work_this_jiffy++;
> +			}
> +			/* If vq is in the round-robin list of virtqueues being
> +			 * constantly checked by this thread, move vq the end
> +			 * of the queue, because it had its fair chance now.
> +			 */
> +			if (vq && !list_empty(&vq->vqpoll.link)) {
> +				list_move_tail(&vq->vqpoll.link,
> +					&dev->vqpoll_list);
> +			}
> +			/* Otherwise, if this vq is looking for notifications
> +			 * but vq polling is not enabled for it, do it now.
> +			 */
> +			else if (poll_start_rate && vq && vq->handle_kick &&
> +				!vq->vqpoll.enabled &&
> +				!vq->vqpoll.shutdown &&
> +				!(vq->used_flags & VRING_USED_F_NO_NOTIFY) &&
> +				vq->vqpoll.work_this_jiffy >=
> +					poll_start_rate) {
> +				vhost_vq_enable_vqpoll(vq);
> +			}
> +		}
> +		/* Check one virtqueue from the round-robin list */
> +		if (!list_empty(&dev->vqpoll_list)) {
> +			struct vhost_virtqueue *vq;
> +
> +			vq = roundrobin_poll(&dev->vqpoll_list);
> +
> +			if (vq) {
> +				vq->handle_kick(&vq->poll.work);
> +				vq->vqpoll.jiffies_last_kick = jiffies;
> +			}
> +
> +			/* If our polling list isn't empty, ask to continue
> +			 * running this thread, don't yield.
> +			 */

This isn't friendly to other processes running on the same CPU,
or if we are trying to kill the process.
See sk_can_busy_loop.

I think you also want to insert cpu_relax somewhere.

> +			__set_current_state(TASK_RUNNING);
>  			if (need_resched())
>  				schedule();

> -		} else
> -			schedule();
> +		} else {
> +			if (work) {
> +				if (need_resched())
> +					schedule();
> +			} else
> +				schedule();
> +		}
>  
>  	}
>  	unuse_mm(dev->mm);
> @@ -306,6 +487,7 @@ void vhost_dev_init(struct vhost_dev *dev,
>  	dev->mm = NULL;
>  	spin_lock_init(&dev->work_lock);
>  	INIT_LIST_HEAD(&dev->work_list);
> +	INIT_LIST_HEAD(&dev->vqpoll_list);
>  	dev->worker = NULL;
>  
>  	for (i = 0; i < dev->nvqs; ++i) {
> @@ -318,7 +500,7 @@ void vhost_dev_init(struct vhost_dev *dev,
>  		vhost_vq_reset(dev, vq);
>  		if (vq->handle_kick)
>  			vhost_poll_init(&vq->poll, vq->handle_kick,
> -					POLLIN, dev);
> +					POLLIN, vq);
>  	}
>  }
>  EXPORT_SYMBOL_GPL(vhost_dev_init);
> @@ -350,7 +532,7 @@ static int vhost_attach_cgroups(struct vhost_dev *dev)
>  	struct vhost_attach_cgroups_struct attach;
>  
>  	attach.owner = current;
> -	vhost_work_init(&attach.work, vhost_attach_cgroups_work);
> +	vhost_work_init(&attach.work, NULL, vhost_attach_cgroups_work);
>  	vhost_work_queue(dev, &attach.work);
>  	vhost_work_flush(dev, &attach.work);
>  	return attach.ret;
> @@ -444,6 +626,26 @@ void vhost_dev_stop(struct vhost_dev *dev)
>  }
>  EXPORT_SYMBOL_GPL(vhost_dev_stop);
>  
> +/* shutdown_vqpoll() asks the worker thread to shut down virtqueue polling
> + * mode for a given virtqueue which is itself being shut down. We ask the
> + * worker thread to do this rather than doing it directly, so that we don't
> + * race with the worker thread's use of the queue.
> + */
> +static void shutdown_vqpoll_work(struct vhost_work *work)
> +{
> +	work->vq->vqpoll.shutdown = true;
> +	vhost_vq_disable_vqpoll(work->vq);
> +	WARN_ON(work->vq->vqpoll.avail_mapped);
> +}
> +
> +static void shutdown_vqpoll(struct vhost_virtqueue *vq)
> +{
> +	struct vhost_work work;
> +
> +	vhost_work_init(&work, vq, shutdown_vqpoll_work);
> +	vhost_work_queue(vq->dev, &work);
> +	vhost_work_flush(vq->dev, &work);
> +}
>  /* Caller should have device mutex if and only if locked is set */
>  void vhost_dev_cleanup(struct vhost_dev *dev, bool locked)
>  {
> @@ -460,6 +662,7 @@ void vhost_dev_cleanup(struct vhost_dev *dev, bool locked)
>  			eventfd_ctx_put(dev->vqs[i]->call_ctx);
>  		if (dev->vqs[i]->call)
>  			fput(dev->vqs[i]->call);
> +		shutdown_vqpoll(dev->vqs[i]);
>  		vhost_vq_reset(dev, dev->vqs[i]);
>  	}
>  	vhost_dev_free_iovecs(dev);
> @@ -1491,6 +1694,19 @@ bool vhost_enable_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq)
>  	u16 avail_idx;
>  	int r;
>  
> +	/* In polling mode, when the backend (e.g., net.c) asks to enable
> +	 * notifications, we don't enable guest notifications. Instead, start
> +	 * polling on this vq by adding it to the round-robin list.
> +	 */
> +	if (vq->vqpoll.enabled) {
> +		if (list_empty(&vq->vqpoll.link)) {
> +			list_add_tail(&vq->vqpoll.link,
> +				&vq->dev->vqpoll_list);
> +			vq->vqpoll.jiffies_last_kick = jiffies;
> +		}
> +		return false;
> +	}
> +
>  	if (!(vq->used_flags & VRING_USED_F_NO_NOTIFY))
>  		return false;
>  	vq->used_flags &= ~VRING_USED_F_NO_NOTIFY;
> @@ -1528,6 +1744,17 @@ void vhost_disable_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq)
>  {
>  	int r;
>  
> +	/* If this virtqueue is vqpoll.enabled, and on the polling list, it
> +	 * will generate notifications even if the guest is asked not to send
> +	 * them. So we must remove it from the round-robin polling list.
> +	 * Note that vqpoll.enabled remains set.
> +	 */
> +	if (vq->vqpoll.enabled) {
> +		if (!list_empty(&vq->vqpoll.link))
> +			list_del_init(&vq->vqpoll.link);
> +		return;
> +	}
> +
>  	if (vq->used_flags & VRING_USED_F_NO_NOTIFY)
>  		return;
>  	vq->used_flags |= VRING_USED_F_NO_NOTIFY;
> diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> index 3eda654..11aaaf4 100644
> --- a/drivers/vhost/vhost.h
> +++ b/drivers/vhost/vhost.h
> @@ -24,6 +24,7 @@ struct vhost_work {
>  	int			  flushing;
>  	unsigned		  queue_seq;
>  	unsigned		  done_seq;
> +	struct vhost_virtqueue    *vq;
>  };
>  
>  /* Poll a file (eventfd or socket) */
> @@ -37,11 +38,12 @@ struct vhost_poll {
>  	struct vhost_dev	 *dev;
>  };
>  
> -void vhost_work_init(struct vhost_work *work, vhost_work_fn_t fn);
> +void vhost_work_init(struct vhost_work *work, struct vhost_virtqueue *vq,
> +							vhost_work_fn_t fn);
>  void vhost_work_queue(struct vhost_dev *dev, struct vhost_work *work);
>  
>  void vhost_poll_init(struct vhost_poll *poll, vhost_work_fn_t fn,
> -		     unsigned long mask, struct vhost_dev *dev);
> +		     unsigned long mask, struct vhost_virtqueue  *vq);
>  int vhost_poll_start(struct vhost_poll *poll, struct file *file);
>  void vhost_poll_stop(struct vhost_poll *poll);
>  void vhost_poll_flush(struct vhost_poll *poll);
> @@ -54,8 +56,6 @@ struct vhost_log {
>  	u64 len;
>  };
>  
> -struct vhost_virtqueue;
> -
>  /* The virtqueue structure describes a queue attached to a device. */
>  struct vhost_virtqueue {
>  	struct vhost_dev *dev;
> @@ -110,6 +110,35 @@ struct vhost_virtqueue {
>  	/* Log write descriptors */
>  	void __user *log_base;
>  	struct vhost_log *log;
> +	struct {
> +      /* When a virtqueue is in vqpoll.enabled mode, it declares
> +       * that instead of using guest notifications (kicks) to
> +       * discover new work, we prefer to continuously poll this
> +       * virtqueue in the worker thread.
> +       * If !enabled, the rest of the fields below are undefined.
> +       */
> +		bool enabled;
> +      /* vqpoll.enabled doesn't always mean that this virtqueue is
> +       * actually being polled: The backend (e.g., net.c) may
> +       * temporarily disable it using vhost_disable/enable_notify().
> +       * vqpoll.link is used to maintain the thread's round-robin
> +       * list of virtqueues that actually need to be polled.
> +       * Note list_empty(link) means this virtqueue isn't polled.
> +       */
> +		struct list_head link;
> +      /* If this flag is true, the virtqueue is being shut down,
> +       * so vqpoll should not be re-enabled.
> +       */
> +		bool shutdown;
> +      /* Various counters used to decide when to enter polling mode
> +       * or leave it and return to notification mode.
> +       */

Please align comments with fields.

> +		unsigned long jiffies_last_kick;
> +		unsigned long jiffies_last_work;
> +		int work_this_jiffy;
> +		struct page *avail_page;
> +		volatile struct vring_avail *avail_mapped;
> +	} vqpoll;
>  };
>  
>  struct vhost_dev {
> @@ -123,6 +152,7 @@ struct vhost_dev {
>  	spinlock_t work_lock;
>  	struct list_head work_list;
>  	struct task_struct *worker;
> +	struct list_head vqpoll_list;
>  };
>  
>  void vhost_dev_init(struct vhost_dev *, struct vhost_virtqueue **vqs, int nvqs);
> -- 
> 1.7.9.5

^ permalink raw reply

* Re: [PATCH] vhost: Add polling mode
From: Michael S. Tsirkin @ 2014-08-20 10:32 UTC (permalink / raw)
  To: Christian Borntraeger
  Cc: ERANRA, kvm, linux-kernel, Razya Ladelsky, GLIKSON, YOSSIKU,
	abel.gordon, JOELN, netdev, virtualization
In-Reply-To: <53F45F3C.5000207@de.ibm.com>

On Wed, Aug 20, 2014 at 10:41:32AM +0200, Christian Borntraeger wrote:
> On 10/08/14 10:30, Razya Ladelsky wrote:
> > From: Razya Ladelsky <razya@il.ibm.com>
> > Date: Thu, 31 Jul 2014 09:47:20 +0300
> > Subject: [PATCH] vhost: Add polling mode
> > 
> > When vhost is waiting for buffers from the guest driver (e.g., more packets to
> > send in vhost-net's transmit queue), it normally goes to sleep and waits for the
> > guest to "kick" it. This kick involves a PIO in the guest, and therefore an exit
> > (and possibly userspace involvement in translating this PIO exit into a file
> > descriptor event), all of which hurts performance.
> > 
> > If the system is under-utilized (has cpu time to spare), vhost can continuously
> > poll the virtqueues for new buffers, and avoid asking the guest to kick us.
> > This patch adds an optional polling mode to vhost, that can be enabled via a
> > kernel module parameter, "poll_start_rate".
> > 
> > When polling is active for a virtqueue, the guest is asked to disable
> > notification (kicks), and the worker thread continuously checks for new buffers.
> > When it does discover new buffers, it simulates a "kick" by invoking the
> > underlying backend driver (such as vhost-net), which thinks it got a real kick
> > from the guest, and acts accordingly. If the underlying driver asks not to be
> > kicked, we disable polling on this virtqueue.
> > 
> > We start polling on a virtqueue when we notice it has work to do. Polling on
> > this virtqueue is later disabled after 3 seconds of polling turning up no new
> > work, as in this case we are better off returning to the exit-based notification
> > mechanism. The default timeout of 3 seconds can be changed with the
> > "poll_stop_idle" kernel module parameter.
> > 
> > This polling approach makes lot of sense for new HW with posted-interrupts for
> > which we have exitless host-to-guest notifications. But even with support for
> > posted interrupts, guest-to-host communication still causes exits. Polling adds
> > the missing part.
> > 
> > When systems are overloaded, there won't be enough cpu time for the various
> > vhost threads to poll their guests' devices. For these scenarios, we plan to add
> > support for vhost threads that can be shared by multiple devices, even of
> > multiple vms.
> > Our ultimate goal is to implement the I/O acceleration features described in:
> > KVM Forum 2013: Efficient and Scalable Virtio (by Abel Gordon)
> > https://www.youtube.com/watch?v=9EyweibHfEs
> > and
> > https://www.mail-archive.com/kvm@vger.kernel.org/msg98179.html
> > 
> > I ran some experiments with TCP stream netperf and filebench (having 2 threads
> > performing random reads) benchmarks on an IBM System x3650 M4.
> > I have two machines, A and B. A hosts the vms, B runs the netserver.
> > The vms (on A) run netperf, its destination server is running on B.
> > All runs loaded the guests in a way that they were (cpu) saturated. For example,
> > I ran netperf with 64B messages, which is heavily loading the vm (which is why
> > its throughput is low).
> > The idea was to get it 100% loaded, so we can see that the polling is getting it
> > to produce higher throughput.
> > 
> > The system had two cores per guest, as to allow for both the vcpu and the vhost
> > thread to run concurrently for maximum throughput (but I didn't pin the threads
> > to specific cores).
> > My experiments were fair in a sense that for both cases, with or without
> > polling, I run both threads, vcpu and vhost, on 2 cores (set their affinity that
> > way). The only difference was whether polling was enabled/disabled.
> > 
> > Results:
> > 
> > Netperf, 1 vm:
> > The polling patch improved throughput by ~33% (1516 MB/sec -> 2046 MB/sec).
> > Number of exits/sec decreased 6x.
> > The same improvement was shown when I tested with 3 vms running netperf
> > (4086 MB/sec -> 5545 MB/sec).
> > 
> > filebench, 1 vm:
> > ops/sec improved by 13% with the polling patch. Number of exits was reduced by
> > 31%.
> > The same experiment with 3 vms running filebench showed similar numbers.
> > 
> > Signed-off-by: Razya Ladelsky <razya@il.ibm.com>
> 
> Gave it a quick try on s390/kvm. As expected it makes no difference for big streaming workload like iperf.
> uperf with a 1-1 round robin got indeed faster by about 30%.
> The high CPU consumption is something that bothers me though, as virtualized systems tend to be full.
> 
> 
> > +static int poll_start_rate = 0;
> > +module_param(poll_start_rate, int, S_IRUGO|S_IWUSR);
> > +MODULE_PARM_DESC(poll_start_rate, "Start continuous polling of virtqueue when rate of events is at least this number per jiffy. If 0, never start polling.");
> > +
> > +static int poll_stop_idle = 3*HZ; /* 3 seconds */
> > +module_param(poll_stop_idle, int, S_IRUGO|S_IWUSR);
> > +MODULE_PARM_DESC(poll_stop_idle, "Stop continuous polling of virtqueue after this many jiffies of no work.");
> 
> This seems ridicoudly high. Even one jiffie is an eternity, so setting it to 1 as a default would reduce the CPU overhead for most cases.
> If we dont have a packet in one millisecond, we can surely go back to the kick approach, I think.
> 
> Christian


Seconded.
Could you publish data with different poll_stop_idle values?
Additionally, time in jiffies is not a reasonable userspace
API. Please switch to some reasonable unit, like microseconds.

Thinking more about it, isn't this almost exactly what net.core.busy_poll does?
That one suggests 50usec timeout.

The only difference I see is in poll_start_rate heuristic,
net.core does not have anything like this.
Do you have data to show that it's helpful - as opposed to just
starting polling whenever an event arrives?
If yes, might it be useful for net core as well?

Only setting timeout globally isn't friendly either.
Should be a tun ioctl similar to SO_BUSY_POLL.

-- 
MST

^ permalink raw reply

* Re: 3.17-rc1 oops during network interface configuration
From: Or Gerlitz @ 2014-08-20 10:31 UTC (permalink / raw)
  To: Bart Van Assche
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA, linux-rdma, Saeed Mahameed,
	Tal Alon, Yevgeny Petrilin
In-Reply-To: <53F1EF18.7010909-HInyCGIudOg@public.gmane.org>

On 18/08/2014 15:18, Bart Van Assche wrote:
> Has anyone else already tried to boot kernel 3.17-rc1 on an IB system ? The
> following call trace is triggered during boot on a system on which kernel
> 3.16 runs fine:

Yep, I see it on my systems too.

I narrowed this down a bit to happen only when the port link type (these 
nodes have ConnectX) is IB and IPoIB gets to load.

I reverted (below) all the IPoIB changes since 3.16 (except for the 
trivial commit c835a67) and the crash still exists.

I guess this needs to go through systematic bisection.

Or.

> net.git]# git log --oneline --no-merges v3.16.. drivers/infiniband/ulp/ipoib/
> 8a118a4 Revert "IB/ipoib: Use P_Key change event instead of P_Key polling mechanism"
> 90e6f39 Revert "IB/ipoib: Avoid flushing the workqueue from worker context"
> 030ade7 Revert "IB/ipoib: Avoid multicast join attempts with invalid P_key"
> 97ba2ff Revert "IPoIB: Remove unnecessary test for NULL before debugfs_remove()"
> e42fa20 IPoIB: Remove unnecessary test for NULL before debugfs_remove()
> dd57c93 IB/ipoib: Avoid multicast join attempts with invalid P_key
> 4eae374 IB/ipoib: Avoid flushing the workqueue from worker context
> db84f88 IB/ipoib: Use P_Key change event instead of P_Key polling mechanism
> c835a67 net: set name_assign_type in alloc_netdev()


> BUG: unable to handle kernel paging request at ffff88090000007e
> IP: __dev_queue_xmit+0x519
> Call Trace:
> ? __dev_queue_xmit+0x49
> dev_queue_xmit+0x10
> neigh_connected_output
> ? ip_finish_output
> ip_finish_output
> ? ip_finish_output
> ? netif_rx_ni
> ip_mc_output
> ip_local_out_sk
> ip_send_skb
> udp_send_skb
> udp_sendmsg
> ? ip_reply_glue_bits
> ? __lock_is_held
> inet_sendmsg
> ? inet_sendmsg
> sock_sendmsg
> ? might_fault
> ? might_fault
> ? move_addr_to_kernel.part.38
> SYSC_sendto
> ? sysret_check
> ? trace_hardirqs_on_caller
> ? trace_hardirqs_on_thunk
> SyS_sendto
> system_call_fastpath
>
> Kernel panic - not syncing: Fatal exception in interrupt
> Kernel Offset: 0x0 from 0xffffffff81000000 (relocation range: 0xffffffff80000000-0xffffffff9fffffff)
> drm_kms_helper: panic occurred, switching back to text console
>
> A screenshot of this kernel oops can be found here:
> https://drive.google.com/file/d/0B1YQOreL3_FxVDB5UTNwekF6LVU/
>
> gdb translates the crash address into the following (not sure this makes sense
> since offset 0x519 is past the end of __dev_queue_xmit()):
>
> (gdb) list *(__dev_queue_xmit+0x519)
> 0xffffffff8136bc89 is in netdev_adjacent_rename_links (net/core/dev.c:5167).
> 5162    void netdev_adjacent_rename_links(struct net_device *dev, char *oldname)
> 5163    {
> 5164            struct netdev_adjacent *iter;
> 5165
> 5166            list_for_each_entry(iter, &dev->adj_list.upper, list) {
> 5167                    netdev_adjacent_sysfs_del(iter->dev, oldname,
> 5168                                              &iter->dev->adj_list.lower);
> 5169                    netdev_adjacent_sysfs_add(iter->dev, dev,
> 5170                                              &iter->dev->adj_list.lower);
> 5171            }
>
> And the address __dev_queue_xmit+0x49 is translated by gdb into:
>
> (gdb) list *(__dev_queue_xmit+0x49)
> 0xffffffff8136b7b9 is in __dev_queue_xmit (./arch/x86/include/asm/preempt.h:75).
> 70       * The various preempt_count add/sub methods
> 71       */
> 72
> 73      static __always_inline void __preempt_count_add(int val)
> 74      {
> 75              raw_cpu_add_4(__preempt_count, val);
> 76      }
> 77
> 78      static __always_inline void __preempt_count_sub(int val)
> 79      {

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" 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] TCP: add option for silent port knocking with integrity protection
From: Alexander Holler @ 2014-08-20 10:20 UTC (permalink / raw)
  To: Hagen Paul Pfeifer
  Cc: Eric Dumazet, Christian Grothoff, Jacob Appelbaum, Andi Kleen,
	Stephen Hemminger, David Miller, netdev, linux-kernel, knock
In-Reply-To: <53F46EA3.60408@ahsoftware.de>

Am 20.08.2014 11:47, schrieb Alexander Holler:
> Am 20.08.2014 11:28, schrieb Hagen Paul Pfeifer:
>> On 20 August 2014 11:07, Alexander Holler <holler@ahsoftware.de> wrote:
>>
>>> For sure it could be better, but I'm already happy with the current
>>> imperfect solution which I can use now and not some perfect solution
>>> which
>>> might be available in some years.
>>
>> Alexander, to make it clear: we cannot include mechanisms which
>> probably open other (security) issues. This is not how things work
>> out. TCP had so many issues in the past - regarding security,
>> implementation f*ups, etc. pp. It is utterly important that there is
>> no problem with an extension. Please join the discussion ob tcpm if
>> you will drive things forward. That's all what I can say - sorry!
>
> Maybe I first should send a million syn-packets to a box where I've
> enabled that feature. ;)
>
> Anyway, I still think there should be some room for experimental
> features in the kernel. It makes them more visible to possible
> contributors and helps to drive further development.
>
> Not necessarily in my case (as most people, I can't and don't want to
> participate in all parties), but ...

And as I've just read the archives of tcpm, I don't think it would help. 
Jacob Appelbaum already expressed everything I like to use this feature 
for, so most of the time I just had to send a +1 to Jacobs comments, 
which would be somewhat annoying. ;)

Regards,

Alexander Holler

^ permalink raw reply

* Re: [Questions]
From: Michael S. Tsirkin @ 2014-08-20 10:05 UTC (permalink / raw)
  To: Zhangjie (HZ); +Cc: Jason Wang, netdev, davem, kvm, liuyongan, qinchuanyu
In-Reply-To: <53F46C3D.1030106@huawei.com>

On Wed, Aug 20, 2014 at 05:37:01PM +0800, Zhangjie (HZ) wrote:
> Hi MST,
> 
> I see "reduce networking latency"
> from Networking Todo, the idea is to "allow handling short packets from softirq or VCPU context".
> If from softirq context, how could softirq copy skb to guest memory? If the method is to use mmstruct of Qemu,
> would it be expensive?

I have some very rough patches to explain this part of the idea.
Will dig them out for you.

> If from VCPU context, maybe the internal operation of the virtual machine will have a significant delay.

We'd have to find a good heuristic here.
Maybe for a small number of very short packets the delay won't be significant.

> Thanks!
> -- 
> Best Wishes!
> Zhang Jie

^ permalink raw reply

* Re: [PATCH] TCP: add option for silent port knocking with integrity protection
From: Alexander Holler @ 2014-08-20  9:47 UTC (permalink / raw)
  To: Hagen Paul Pfeifer
  Cc: Eric Dumazet, Christian Grothoff, Jacob Appelbaum, Andi Kleen,
	Stephen Hemminger, David Miller, netdev, linux-kernel, knock
In-Reply-To: <CAPh34mc8eGqLyGrq1AH6AP5yBGnJgePkK03Ys-Zhw9cz9h3Y2g@mail.gmail.com>

Am 20.08.2014 11:28, schrieb Hagen Paul Pfeifer:
> On 20 August 2014 11:07, Alexander Holler <holler@ahsoftware.de> wrote:
>
>> For sure it could be better, but I'm already happy with the current
>> imperfect solution which I can use now and not some perfect solution which
>> might be available in some years.
>
> Alexander, to make it clear: we cannot include mechanisms which
> probably open other (security) issues. This is not how things work
> out. TCP had so many issues in the past - regarding security,
> implementation f*ups, etc. pp. It is utterly important that there is
> no problem with an extension. Please join the discussion ob tcpm if
> you will drive things forward. That's all what I can say - sorry!

Maybe I first should send a million syn-packets to a box where I've 
enabled that feature. ;)

Anyway, I still think there should be some room for experimental 
features in the kernel. It makes them more visible to possible 
contributors and helps to drive further development.

Not necessarily in my case (as most people, I can't and don't want to 
participate in all parties), but ...

Regards,

Alexander Holler

^ permalink raw reply

* [Questions]
From: Zhangjie (HZ) @ 2014-08-20  9:37 UTC (permalink / raw)
  To: Michael S. Tsirkin, Jason Wang, netdev, davem, kvm
  Cc: liuyongan, qinchuanyu, zhangjie14

Hi MST,

I see "reduce networking latency"
from Networking Todo, the idea is to "allow handling short packets from softirq or VCPU context".
If from softirq context, how could softirq copy skb to guest memory? If the method is to use mmstruct of Qemu,
would it be expensive?
If from VCPU context, maybe the internal operation of the virtual machine will have a significant delay.

Thanks!
-- 
Best Wishes!
Zhang Jie

^ permalink raw reply

* [PATCH V2] sctp: not send SCTP_PEER_ADDR_CHANGE notifications with failed probe
From: Zhu Yanjun @ 2014-08-20  9:31 UTC (permalink / raw)
  To: dborkman, linux-kernel, netdev, vyasevich, tuexen,
	khandelwal.deepak.1987, Yue.Tao, alexandre.dietsch, davem,
	zyjzyj2000
  Cc: Zhu Yanjun

Hi, Vlad && Deepak && Michael && David && Daniel

V2: Following the advice from Daniel Borkmann, I modified the comments and short log.

>From Michael && Deepak
"
 lxr SCTP implementation, doesn't transit the path state to INACTIVE, if it was never confirmed. this leads to SCTP_PEER_ADDRESS_CHANGE notification after each failed probe from this time.
 Is there any specific reason to have same notification to SCTP User with each probe in RTO time period ?
 806 case SCTP_TRANSPORT_DOWN:
 807 /* If the transport was never confirmed, do not transition it
 808 * to inactive state. Also, release the cached route since
 809 * there may be a better route next time.
 810 */
 811 if (transport->state != SCTP_UNCONFIRMED)

 812 transport->state = SCTP_INACTIVE;

 http://lxr.free-electrons.com/source/net/sctp/associola.c#L806

 ......

 I would also only expect a  SCTP_PEER_ADDRESS_CHANGE notification when a path state changes, not on every  try.

"

I made a patch to disable sending SCTP_PEER_ADDRESS_CHANGE notification every try. Now the patch is in the attachment. Please check it.

Zhu Yanjun


Zhu Yanjun (1):
  sctp: not send SCTP_PEER_ADDR_CHANGE notifications with failed probe

 net/sctp/associola.c | 1 +
 1 file changed, 1 insertion(+)

-- 
1.9.1

^ permalink raw reply

* [PATCH 1/1] sctp: not send SCTP_PEER_ADDR_CHANGE notifications with failed probe
From: Zhu Yanjun @ 2014-08-20  9:31 UTC (permalink / raw)
  To: dborkman, linux-kernel, netdev, vyasevich, tuexen,
	khandelwal.deepak.1987, Yue.Tao, alexandre.dietsch, davem,
	zyjzyj2000
  Cc: Zhu Yanjun
In-Reply-To: <1408527103-22772-1-git-send-email-Yanjun.Zhu@windriver.com>

Since the transport has always been in state SCTP_UNCONFIRMED, it
therefore wasn't active before and hasn't been used before, and it
always has been, so it is unnecessary to bug the user with a 
notification.

Reported-by: Deepak Khandelwal <khandelwal.deepak.1987@gmail.com>  
Suggested-by: Vlad Yasevich <vyasevich@gmail.com> 
Suggested-by: Michael Tuexen <tuexen@fh-muenster.de>
Suggested-by: Daniel Borkmann <dborkman@redhat.com>
Signed-off-by: Zhu Yanjun <Yanjun.Zhu@windriver.com>
---
 net/sctp/associola.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index 9de23a2..2e23f6b 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -813,6 +813,7 @@ void sctp_assoc_control_transport(struct sctp_association *asoc,
 		else {
 			dst_release(transport->dst);
 			transport->dst = NULL;
+			ulp_notify = false;
 		}
 
 		spc_state = SCTP_ADDR_UNREACHABLE;
-- 
1.9.1

^ permalink raw reply related

* RE: Lost ping responses
From: David Laight @ 2014-08-20  9:29 UTC (permalink / raw)
  To: 'Cong Wang'; +Cc: Jamal Hadi Salim, Linux Netdev List
In-Reply-To: <CAHA+R7Pv9HaeNFnOAEr9BHqBRsWKnbeo7R4beOUQe9zyzmKfBA@mail.gmail.com>

From: Cong Wang
> On Tue, Aug 19, 2014 at 4:35 AM, David Laight <David.Laight@aculab.com> wrote:
> > From: Jamal Hadi Salim
> >
> >> > Any ideas how to find out where the discards are happening?
> >> > It might just be somewhere that would affect other traffic flows.
> >>
> >> netstat -s
> >
> > Thanks - 'Ip: outgoing packets dropped' increments for every lost response.
> >
> > Next step is to work out what that means...
> >
> 
> Try dropwatch, it should give you better hints.

That involves finding/compiling a version that will run on ubuntu 13.10 server.
And working out which extra ubuntu packages it needs to work.
On a system I've built the sources on (after fixing LDLIBS) I get the
error 'Unable to find NET_DM family' - whatever that means.
but that isn't the system dropping packets.

	David


^ permalink raw reply

* Re: [PATCH] TCP: add option for silent port knocking with integrity protection
From: Hagen Paul Pfeifer @ 2014-08-20  9:28 UTC (permalink / raw)
  To: Alexander Holler
  Cc: Eric Dumazet, Christian Grothoff, Jacob Appelbaum, Andi Kleen,
	Stephen Hemminger, David Miller, netdev, linux-kernel, knock
In-Reply-To: <53F4654C.10101@ahsoftware.de>

On 20 August 2014 11:07, Alexander Holler <holler@ahsoftware.de> wrote:

> For sure it could be better, but I'm already happy with the current
> imperfect solution which I can use now and not some perfect solution which
> might be available in some years.

Alexander, to make it clear: we cannot include mechanisms which
probably open other (security) issues. This is not how things work
out. TCP had so many issues in the past - regarding security,
implementation f*ups, etc. pp. It is utterly important that there is
no problem with an extension. Please join the discussion ob tcpm if
you will drive things forward. That's all what I can say - sorry!

^ 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