* Re: [PATCH 3.8-rc] net: mvneta: fix driver operation in SMP context
From: Ben Hutchings @ 2013-01-07 16:53 UTC (permalink / raw)
To: Thomas Petazzoni
Cc: David S. Miller, netdev, Jason Cooper, Andrew Lunn,
Gregory Clement, linux-arm-kernel, Lior Amsalem, Maen Suleiman,
Dmitri Epshtein
In-Reply-To: <1357576074-24245-1-git-send-email-thomas.petazzoni@free-electrons.com>
On Mon, 2013-01-07 at 17:27 +0100, Thomas Petazzoni wrote:
> From: Dmitri Epshtein <dima@marvell.com>
>
> In order for the driver to behave properly in a SMP context, the same
> transmit queue should be used by the kernel in dev_queue_xmit() and in
> the driver's mvneta_tx() function. To achieve that, the driver now
> implements the ->ndo_select_txq() operation.
>
> For now, it always returns the same transmit queue, txq_def, until the
> driver is expanded to properly take advantage of the multiqueue
> capabilities of the hardware.
[...]
Well it looks like the driver already sets up multiple TX queues, and
you just need to call skb_get_queue_mapping(skb) to look up the TX queue
in mvneta_tx().
Also since the numbers of queues are variable you should be calling
alloc_etherdev_mqs(sizeof(struct mvneta_port), txq_number, rxq_number)
instead of alloc_etherdev_mq(..., 8) in mvneta_probe().
If for some reason the current hardware initialisation doesn't actually
work for more than 1 TX queue then change the number you pass to
alloc_etherdev_mqs().
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 3.8-rc] net: mvneta: fix driver operation in SMP context
From: Jason Cooper @ 2013-01-07 16:44 UTC (permalink / raw)
To: Thomas Petazzoni
Cc: David S. Miller, Lior Amsalem, Andrew Lunn, netdev, Maen Suleiman,
Gregory Clement, Dmitri Epshtein, linux-arm-kernel
In-Reply-To: <1357576074-24245-1-git-send-email-thomas.petazzoni@free-electrons.com>
On Mon, Jan 07, 2013 at 05:27:54PM +0100, Thomas Petazzoni wrote:
> From: Dmitri Epshtein <dima@marvell.com>
>
> In order for the driver to behave properly in a SMP context, the same
> transmit queue should be used by the kernel in dev_queue_xmit() and in
> the driver's mvneta_tx() function. To achieve that, the driver now
> implements the ->ndo_select_txq() operation.
>
> For now, it always returns the same transmit queue, txq_def, until the
> driver is expanded to properly take advantage of the multiqueue
> capabilities of the hardware.
>
> Without this patch, the network driver crashes the kernel almost
> immediately on Armada XP platforms, if the network load is at least a
> little bit parallel (i.e several threads).
>
> [Thomas Petazzoni: reword commit message]
> Signed-off-by: Dmitri Epshtein <dima@marvell.com>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Acked-by: Jason Cooper <jason@lakedaemon.net>
I'll assume this is going through -net, let me know if it should be
otherwise.
thx,
Jason.
^ permalink raw reply
* Re: [RFC PATCH V2] tcp: introduce raw access to experimental options
From: Eric Dumazet @ 2013-01-07 16:44 UTC (permalink / raw)
To: elelueck; +Cc: netdev, davem, ubacher, raspl, frankbla, samudrala
In-Reply-To: <1357575210-52926-2-git-send-email-elelueck@linux.vnet.ibm.com>
On Mon, 2013-01-07 at 17:13 +0100, elelueck@linux.vnet.ibm.com wrote:
> From: Einar Lueck <elelueck@linux.vnet.ibm.com>
>
> This patch adds means for raw acces to TCP expirimental options
> 253 and 254. The intention of this is to enable user space
> applications to implement communication behaviour that depends
> on experimental options. For that, new (set|get)sockopts are
> introduced:
>
> TCP_EXPOPTS (get & set): TCP experimental options to be added to
> packets
> TCP_RECV_EXPOPTS (get): experimental options received with last
> packet
> TCP_RECV_SYN_EXPOPTS (get): experimental options received with
> SYN packet
>
> Access to these options is allowed only with CAP_NET_RAW privilige.
>
> TCP experimental options 253 and 254 configured via TCP_EXPOPTS on
> any TCP socket are appended to every packet that is sent as long
> as there is enough room left. If there is not enough room left they
> are silently dropped.
>
> Listening sockets reply to SYN packets with SYN ACK packets containing
> TCP experimental options 253 and 254 as configured via TCP_EXPOPTS, too.
> If a TCP connection gets established the configured experimental options
> are the defaults for the new socket, too. Thus, a getsockopt on the
> resulting accept socket for TCP_EXPOPTS returns the same stuff configured
> on the listening socket.
>
> As mentioned above, even after the 3whs is complete, experimental options
> are sent with every packet. To enable user space applications to distinguish
> between what has been advertized via SYN and what has been received with the
> last packet the aforementioned TCP_RECV_SYN_EXPOPTS and TCP_RECV_EXPOPTS are
> introduced.
>
> Today, experimental option 253 (COOKIE) and 254 (FASTOPEN) are already
> exploited. For co-existence the following approach has been taken:
>
> General remarks:
> * Interface to COOKIE and FASTOPEN stays the same
> Sender side:
> 1. COOKIE and FASTPATH code adds own options first (if applicable)
> 2. Finally, if enough room is left, TCP_EXPOPTS experimental options are
> appended
> Receiver side:
> 1. ALL 253 and 254 experimental options are made available via
> TCP_RECV(_SYN)_EXPOPTS
> 2. COOKIE and FASTOPEN code check if there is any option relevant for them
>
> References:
> http://tools.ietf.org/html/draft-ietf-tcpm-experimental-options-02
>
> Signed-off-by: Einar Lueck <elelueck@linux.vnet.ibm.com>
> ---
> include/linux/tcp.h | 23 +++++++++
> include/net/tcp.h | 3 ++
> include/uapi/linux/tcp.h | 3 ++
> net/ipv4/tcp.c | 122 +++++++++++++++++++++++++++++++++++++++++++++++
> net/ipv4/tcp_input.c | 119 ++++++++++++++++++++++++++++++---------------
> net/ipv4/tcp_ipv4.c | 14 ++++++
> net/ipv4/tcp_minisocks.c | 17 +++++++
> net/ipv4/tcp_output.c | 47 +++++++++++-------
> 8 files changed, 293 insertions(+), 55 deletions(-)
Thats a big addition, and for example doesn't help if the SYNACK should
include an option that is depending on the content of SYN message.
For TCP fastopen for example, the cookie we send to the client is not a
constant cookie.
Also TCP coalescing of TCP collapse will merge several skbs, so storing
"the last received options" in the socket is kind of not well defined
semantic.
It looks like you need to add hooks and kernel modules to fully use
experimental options, like congestion control modules.
^ permalink raw reply
* Re: [PATCH v2] sctp: Change defaults on cookie hmac selection
From: Vlad Yasevich @ 2013-01-07 16:39 UTC (permalink / raw)
To: Neil Horman
Cc: Florian Fainelli, netdev, David Miller, Linus Torvalds,
linux-sctp
In-Reply-To: <20130107154625.GD31577@hmsreliant.think-freely.org>
On 01/07/2013 10:46 AM, Neil Horman wrote:
> On Mon, Jan 07, 2013 at 10:32:01AM -0500, Vlad Yasevich wrote:
>> On 01/07/2013 09:49 AM, Neil Horman wrote:
>>> On Mon, Jan 07, 2013 at 02:25:39PM +0100, Florian Fainelli wrote:
>>>> Hello Neil,
>>>>
>>>> Le 12/15/12 02:22, Neil Horman a écrit :
>>>>> Recently I posted commit 3c68198e75 which made selection of the cookie hmac
>>>>> algorithm selectable. This is all well and good, but Linus noted that it
>>>>> changes the default config:
>>>>> http://marc.info/?l=linux-netdev&m=135536629004808&w=2
>>>>>
>>>>> I've modified the sctp Kconfig file to reflect the recommended way of making
>>>>> this choice, using the thermal driver example specified, and brought the
>>>>> defaults back into line with the way they were prior to my origional patch
>>>>>
>>>>> Also, on Linus' suggestion, re-adding ability to select default 'none' hmac
>>>>> algorithm, so we don't needlessly bloat the kernel by forcing a non-none
>>>>> default. This also led me to note that we won't honor the default none
>>>>> condition properly because of how sctp_net_init is encoded. Fix that up as
>>>>> well.
>>>>>
>>>>> Tested by myself (allbeit fairly quickly). All configuration combinations seems
>>>>> to work soundly.
>>>>>
>>>>> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
>>>>> CC: David Miller <davem@davemloft.net>
>>>>> CC: Linus Torvalds <torvalds@linux-foundation.org>
>>>>> CC: Vlad Yasevich <vyasevich@gmail.com>
>>>>> CC: linux-sctp@vger.kernel.org
>>>>> ---
>>>>> net/sctp/Kconfig | 27 +++++++++++++++++++++++++--
>>>>> net/sctp/protocol.c | 4 ++--
>>>>> 2 files changed, 27 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/net/sctp/Kconfig b/net/sctp/Kconfig
>>>>> index a9edd2e..c262106 100644
>>>>> --- a/net/sctp/Kconfig
>>>>> +++ b/net/sctp/Kconfig
>>>>> @@ -66,12 +66,36 @@ config SCTP_DBG_OBJCNT
>>>>> 'cat /proc/net/sctp/sctp_dbg_objcnt'
>>>>>
>>>>> If unsure, say N
>>>>> +choice
>>>>> + prompt "Default SCTP cookie HMAC encoding"
>>>>> + default SCTP_COOKIE_HMAC_MD5
>>>>
>>>> Should not this be SCTP_DEFAULT_COOKIE_HMAC_MD5? I just tried to
>>>> update to 3.8-rc2, and I usually build my kernel-headers with:
>>>>
>>>> yes '' | ARCH=foo make oldconfig
>>>>
>>>> and this just kept asking me for this config symbol because none
>>>> could be provided.
>>>> --
>>>> Florian
>>>>
>>>
>>> No, the config mechanism is setup to offer the user the ability to choose a
>>> default cookie hmac, alg, then optionally select any other hmac algs you would
>>> like to be made available (in the event you want to change the default at run
>>> time). When you select the default, it eables (via the select directive), the
>>> corresponding SCTP_COOKIE_HMAC_* config option, which is used in the build, and
>>> then prompts for the remaining values.
>>>
>>
>> Neil
>>
>> Actually, I think it should be as Florian suggests. The default
>> value of the choice should be one of the values defined as part of
>> the choice (the SCTP_DEFAULT_COOKIE_*). Turning on appropriate
>> default would turn on appropriate cookie config
>> (SCTP_COOKIE_HMAC_*).
>>
> I absolutely disagree.
>
>> Would that save all the config trouble?
>>
> Yes, it would fix it as Florian has noted, but at the cost of silently modifying
> what the default hmac config vaule is. If you've expressly disabled
> SCTP_COOKIE_HMAC_MD5, and then blindly take the default choice in the
> SCTP_DEFAULT_COOKIE selection option (the default default as it were), using the
> approach your suggesting, then that will silently enable SCTP_COOKIE_HMAC_MD5
> again, which may not be expected by users. If you expressly have a config
> option disabled in an old configuration, we should leave it there.
GACK. Just reproduced this and I really don't like this infinite loop
of choice prompts. That's a horrible bug and we need to fix this.
I don't think overriding the value is that big of a deal, especially
considering that this is exactly what 'make menuconfig' and other
graphical configs will do.
If I start with:
CONFIG_IP_SCTP=m
CONFIG_NET_SCTPPROBE=m
# CONFIG_SCTP_DBG_MSG is not set
# CONFIG_SCTP_DBG_OBJCNT is not set
# CONFIG_SCTP_HMAC_NONE is not set
CONFIG_SCTP_HMAC_SHA1=y
# CONFIG_SCTP_HMAC_MD5 is not set
then run:
yes "" | make oldconfig
I get an infinite loop.
If I run "make menuconfig", I get:
CONFIG_IP_SCTP=m
CONFIG_NET_SCTPPROBE=m
# CONFIG_SCTP_DBG_MSG is not set
# CONFIG_SCTP_DBG_OBJCNT is not set
CONFIG_SCTP_DEFAULT_COOKIE_HMAC_MD5=y
# CONFIG_SCTP_DEFAULT_COOKIE_HMAC_SHA1 is not set
# CONFIG_SCTP_DEFAULT_COOKIE_HMAC_NONE is not set
CONFIG_SCTP_COOKIE_HMAC_MD5=y
# CONFIG_SCTP_COOKIE_HMAC_SHA1 is not set
Note, that SHA1 is now overridden with MD5.
If I change the value of the default choice in Kconfig, the behavior
between oldconfig and menuconfig is the same.
-vlad
>
> We're doing the right thing now, IMO. When presented with a conflictly set
> of configuration options, the config utilty is (repeatedly) prompting us to
> resolve them. That seems like a much more reasonable approach to this, than
> silently changing pre-existing options so people can do the equivalent of just
> blindly pressing enter through the config process (which is all yes "" | make
> oldconfig is).
>
> This is a momentary hiccup, corrected by taking 30 seconds to make a manual
> config change (or by taking a second to understand what the config utility is
> tell us by prompting for a default choice repeatedly). Theres nothing to fix
> here.
>
> Neil
>
>> -vlad
>>
>>> Neil
>>>
>>
>> --
>> 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] ethtool: set addr_assign_type to NET_ADDR_SET when addr is passed on create
From: Ben Hutchings @ 2013-01-07 16:36 UTC (permalink / raw)
To: Jiri Pirko; +Cc: netdev, davem, shemminger, sassmann
In-Reply-To: <1357511921-5537-1-git-send-email-jiri@resnulli.us>
On Sun, 2013-01-06 at 23:38 +0100, Jiri Pirko wrote:
> In case user passed address via netlink during create, NET_ADDR_PERM was set.
> That is not correct so fix this by setting NET_ADDR_SET.
>
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
Not sure what this has to do with ethtool...
> ---
> net/core/rtnetlink.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index 9969afb..9a419b0 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -1667,9 +1667,11 @@ struct net_device *rtnl_create_link(struct net *net,
>
> if (tb[IFLA_MTU])
> dev->mtu = nla_get_u32(tb[IFLA_MTU]);
> - if (tb[IFLA_ADDRESS])
> + if (tb[IFLA_ADDRESS]) {
> memcpy(dev->dev_addr, nla_data(tb[IFLA_ADDRESS]),
> nla_len(tb[IFLA_ADDRESS]));
> + dev->addr_assign_type = NET_ADDR_SET;
> + }
> if (tb[IFLA_BROADCAST])
> memcpy(dev->broadcast, nla_data(tb[IFLA_BROADCAST]),
> nla_len(tb[IFLA_BROADCAST]));
But if the address is not specified for this new device,
addr_assign_type remains NET_ADD_PERM, right? That sort of makes sense
in that the 'permanent address' of a software device is all-zeroes. I
would prefer to have a way for devices to deny having a permanent
address, but it's a bit late to change that now.
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
* [PATCH 3.8-rc] net: mvneta: fix driver operation in SMP context
From: Thomas Petazzoni @ 2013-01-07 16:27 UTC (permalink / raw)
To: David S. Miller
Cc: netdev, Jason Cooper, Andrew Lunn, Gregory Clement,
linux-arm-kernel, Lior Amsalem, Maen Suleiman, Dmitri Epshtein
From: Dmitri Epshtein <dima@marvell.com>
In order for the driver to behave properly in a SMP context, the same
transmit queue should be used by the kernel in dev_queue_xmit() and in
the driver's mvneta_tx() function. To achieve that, the driver now
implements the ->ndo_select_txq() operation.
For now, it always returns the same transmit queue, txq_def, until the
driver is expanded to properly take advantage of the multiqueue
capabilities of the hardware.
Without this patch, the network driver crashes the kernel almost
immediately on Armada XP platforms, if the network load is at least a
little bit parallel (i.e several threads).
[Thomas Petazzoni: reword commit message]
Signed-off-by: Dmitri Epshtein <dima@marvell.com>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
This is 3.8-rc material.
---
drivers/net/ethernet/marvell/mvneta.c | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
index b6025c3..af2c421 100644
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -1310,6 +1310,17 @@ static u32 mvneta_skb_tx_csum(struct mvneta_port *pp, struct sk_buff *skb)
return MVNETA_TX_L4_CSUM_NOT;
}
+static u16 mvneta_tx_policy(struct mvneta_port *pp, struct sk_buff *skb)
+{
+ return (u16)txq_def;
+}
+
+static u16 mvneta_select_txq(struct net_device *dev, struct sk_buff *skb)
+{
+ struct mvneta_port *pp = netdev_priv(dev);
+ return mvneta_tx_policy(pp, skb);
+}
+
/* Returns rx queue pointer (find last set bit) according to causeRxTx
* value
*/
@@ -1476,7 +1487,8 @@ error:
static int mvneta_tx(struct sk_buff *skb, struct net_device *dev)
{
struct mvneta_port *pp = netdev_priv(dev);
- struct mvneta_tx_queue *txq = &pp->txqs[txq_def];
+ u16 txq_id = mvneta_tx_policy(pp, skb);
+ struct mvneta_tx_queue *txq = &pp->txqs[txq_id];
struct mvneta_tx_desc *tx_desc;
struct netdev_queue *nq;
int frags = 0;
@@ -1486,7 +1498,7 @@ static int mvneta_tx(struct sk_buff *skb, struct net_device *dev)
goto out;
frags = skb_shinfo(skb)->nr_frags + 1;
- nq = netdev_get_tx_queue(dev, txq_def);
+ nq = netdev_get_tx_queue(dev, txq_id);
/* Get a descriptor for the first part of the packet */
tx_desc = mvneta_txq_next_desc_get(txq);
@@ -2550,6 +2562,7 @@ static const struct net_device_ops mvneta_netdev_ops = {
.ndo_change_mtu = mvneta_change_mtu,
.ndo_tx_timeout = mvneta_tx_timeout,
.ndo_get_stats64 = mvneta_get_stats64,
+ .ndo_select_queue = mvneta_select_txq,
};
const struct ethtool_ops mvneta_eth_tool_ops = {
--
1.7.9.5
^ permalink raw reply related
* Re: [patch net-next] ethtool: consolidate work with ethtool_ops
From: Richard Cochran @ 2013-01-07 16:19 UTC (permalink / raw)
To: Jiri Pirko; +Cc: netdev, davem, bhutchings, greearb, jeffrey.t.kirsher
In-Reply-To: <1357561574-14255-1-git-send-email-jiri@resnulli.us>
On Mon, Jan 07, 2013 at 01:26:14PM +0100, Jiri Pirko wrote:
> @@ -1346,13 +1347,9 @@ static int ethtool_get_ts_info(struct net_device *dev, void __user *useraddr)
> info.cmd = ETHTOOL_GET_TS_INFO;
>
> if (phydev && phydev->drv && phydev->drv->ts_info) {
> -
> err = phydev->drv->ts_info(phydev, &info);
> -
> - } else if (dev->ethtool_ops && dev->ethtool_ops->get_ts_info) {
> -
> + } else if (ops->get_ts_info) {
> err = ops->get_ts_info(dev, &info);
> -
> } else {
> info.so_timestamping =
> SOF_TIMESTAMPING_RX_SOFTWARE |
Acked-by: Richard Cochran <richardcochran@gmail.com>
^ permalink raw reply
* Re: [PATCH V2] xen/netfront: improve truesize tracking
From: Eric Dumazet @ 2013-01-07 16:19 UTC (permalink / raw)
To: Ian Campbell
Cc: netdev, Sander Eikelenboom, Konrad Rzeszutek Wilk, annie li,
xen-devel
In-Reply-To: <1357572726-7059-1-git-send-email-ian.campbell@citrix.com>
On Mon, 2013-01-07 at 15:32 +0000, Ian Campbell wrote:
> Using RX_COPY_THRESHOLD is incorrect if the SKB is actually smaller
> than that. We have already accounted for this in
> NETFRONT_SKB_CB(skb)->pull_to so use that instead.
>
> Fixes WARN_ON from skb_try_coalesce.
>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Cc: Sander Eikelenboom <linux@eikelenboom.it>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: annie li <annie.li@oracle.com>
> Cc: xen-devel@lists.xen.org
> Cc: netdev@vger.kernel.org
> Cc: stable@kernel.org # 3.7.x only
> ---
> v2:
> - acount frags as a full page as recommended by Eric.
Acked-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply
* Re: [patch net-next] ethtool: consolidate work with ethtool_ops
From: Ben Hutchings @ 2013-01-07 16:19 UTC (permalink / raw)
To: Jiri Pirko; +Cc: netdev, davem, richardcochran, greearb, jeffrey.t.kirsher
In-Reply-To: <1357561574-14255-1-git-send-email-jiri@resnulli.us>
On Mon, 2013-01-07 at 13:26 +0100, Jiri Pirko wrote:
> No need to check if ethtool_ops == NULL since it can't be.
> Use local variable "ops" in functions where it is present
> instead of dev->ethtool_ops
> Introduce local variable "ops" in functions where dev->ethtool_ops is used
> many times.
>
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
> ---
> net/core/ethtool.c | 45 +++++++++++++++++++++------------------------
> 1 file changed, 21 insertions(+), 24 deletions(-)
>
> diff --git a/net/core/ethtool.c b/net/core/ethtool.c
> index a870543..08c213e 100644
> --- a/net/core/ethtool.c
> +++ b/net/core/ethtool.c
[...]
> @@ -1082,9 +1083,10 @@ static int ethtool_phys_id(struct net_device *dev, void __user *useraddr)
> {
> struct ethtool_value id;
> static bool busy;
> + const struct ethtool_ops *ops = dev->ethtool_ops;
> int rc;
>
> - if (!dev->ethtool_ops->set_phys_id)
> + if (ops->set_phys_id)
> return -EOPNOTSUPP;
[...]
This condition is inverted.
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] xen/netfront: improve truesize tracking
From: Eric Dumazet @ 2013-01-07 16:17 UTC (permalink / raw)
To: Ian Campbell
Cc: Sander Eikelenboom, Rick Jones, netdev@vger.kernel.org,
Konrad Rzeszutek Wilk, annie li, xen-devel@lists.xensource.com
In-Reply-To: <1357566063.7989.68.camel@zakaz.uk.xensource.com>
On Mon, 2013-01-07 at 13:41 +0000, Ian Campbell wrote:
> > UDP UNIDIRECTIONAL SEND TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to 192.168.1.1 (192.168.1.1) port 0 AF_INET : +/-2.500% @ 95% conf. : demo
> > Socket Message Elapsed Messages
> > Size Size Time Okay Errors Throughput
> > bytes bytes secs # # KBytes/sec
> >
> > current 212992 65507 60.00 252586 0 269305.73
> > current 2280 60.00 229371 244553.96
> > patched 212992 65507 60.00 256209 0 273168.32
> > patched 2280 60.00 201670 215019.54
>
> The recv numbers here aren't too pleasing either.
The number of packets that can be queued into UDP socket depends on
sk->sk_rcvbuf (SO_RCVBUF) and skb truesize.
So what we notice here are packet drops (netstat -s would give us the
total counters)
To absorb a burst of incoming messages, an application would have to set
an appropriate receive buffer.
In this case, RCVBUF value was set to a very minimum, basically not
allowing more than one queued packet.
TCP stack has a 'collapse' mode, which basically converts skbs in
receive queue (or ofo queue) to better filled ones (skb->len very close
to skb->truesize) when memory limits are about to be hit.
Its very expensive, as it adds one more copy stage, but it happens only
in rare circumstances. Of course, when a driver uses one page of 4096
bytes to store one 1514 bytes ethernet frame, it can happen more often.
netstat -s | grep collap
25292 packets collapsed in receive queue due to low socket buffer
^ permalink raw reply
* [RFC PATCH V2] tcp: introduce raw access to experimental options
From: elelueck @ 2013-01-07 16:13 UTC (permalink / raw)
To: netdev; +Cc: Einar Lueck, davem, ubacher, raspl, frankbla, samudrala
In-Reply-To: <1357575210-52926-1-git-send-email-elelueck@linux.vnet.ibm.com>
From: Einar Lueck <elelueck@linux.vnet.ibm.com>
This patch adds means for raw acces to TCP expirimental options
253 and 254. The intention of this is to enable user space
applications to implement communication behaviour that depends
on experimental options. For that, new (set|get)sockopts are
introduced:
TCP_EXPOPTS (get & set): TCP experimental options to be added to
packets
TCP_RECV_EXPOPTS (get): experimental options received with last
packet
TCP_RECV_SYN_EXPOPTS (get): experimental options received with
SYN packet
Access to these options is allowed only with CAP_NET_RAW privilige.
TCP experimental options 253 and 254 configured via TCP_EXPOPTS on
any TCP socket are appended to every packet that is sent as long
as there is enough room left. If there is not enough room left they
are silently dropped.
Listening sockets reply to SYN packets with SYN ACK packets containing
TCP experimental options 253 and 254 as configured via TCP_EXPOPTS, too.
If a TCP connection gets established the configured experimental options
are the defaults for the new socket, too. Thus, a getsockopt on the
resulting accept socket for TCP_EXPOPTS returns the same stuff configured
on the listening socket.
As mentioned above, even after the 3whs is complete, experimental options
are sent with every packet. To enable user space applications to distinguish
between what has been advertized via SYN and what has been received with the
last packet the aforementioned TCP_RECV_SYN_EXPOPTS and TCP_RECV_EXPOPTS are
introduced.
Today, experimental option 253 (COOKIE) and 254 (FASTOPEN) are already
exploited. For co-existence the following approach has been taken:
General remarks:
* Interface to COOKIE and FASTOPEN stays the same
Sender side:
1. COOKIE and FASTPATH code adds own options first (if applicable)
2. Finally, if enough room is left, TCP_EXPOPTS experimental options are
appended
Receiver side:
1. ALL 253 and 254 experimental options are made available via
TCP_RECV(_SYN)_EXPOPTS
2. COOKIE and FASTOPEN code check if there is any option relevant for them
References:
http://tools.ietf.org/html/draft-ietf-tcpm-experimental-options-02
Signed-off-by: Einar Lueck <elelueck@linux.vnet.ibm.com>
---
include/linux/tcp.h | 23 +++++++++
include/net/tcp.h | 3 ++
include/uapi/linux/tcp.h | 3 ++
net/ipv4/tcp.c | 122 +++++++++++++++++++++++++++++++++++++++++++++++
net/ipv4/tcp_input.c | 119 ++++++++++++++++++++++++++++++---------------
net/ipv4/tcp_ipv4.c | 14 ++++++
net/ipv4/tcp_minisocks.c | 17 +++++++
net/ipv4/tcp_output.c | 47 +++++++++++-------
8 files changed, 293 insertions(+), 55 deletions(-)
diff --git a/include/linux/tcp.h b/include/linux/tcp.h
index 60b7aac..574c26f 100644
--- a/include/linux/tcp.h
+++ b/include/linux/tcp.h
@@ -67,6 +67,8 @@ struct tcp_sack_block {
#define TCP_FACK_ENABLED (1 << 1) /*1 = FACK is enabled locally*/
#define TCP_DSACK_SEEN (1 << 2) /*1 = DSACK was received from peer*/
+#define TCP_EXPOP_MAXLEN 40
+
struct tcp_options_received {
/* PAWS/RTTM data */
long ts_recent_stamp;/* Time we stored ts_recent (for aging) */
@@ -86,6 +88,9 @@ struct tcp_options_received {
u8 num_sacks; /* Number of SACK blocks */
u16 user_mss; /* mss requested by user in ioctl */
u16 mss_clamp; /* Maximal mss, negotiated at connection setup */
+ u8 exp_opts_len; /* length of buffer containing all exp
+ * options in format: kind length data */
+ u8 exp_opts[TCP_EXPOP_MAXLEN]; /* experimental options */
};
static inline void tcp_clear_options(struct tcp_options_received *rx_opt)
@@ -93,6 +98,7 @@ static inline void tcp_clear_options(struct tcp_options_received *rx_opt)
rx_opt->tstamp_ok = rx_opt->sack_ok = 0;
rx_opt->wscale_ok = rx_opt->snd_wscale = 0;
rx_opt->cookie_plus = 0;
+ rx_opt->exp_opts_len = 0;
}
/* This is the max number of SACKS that we'll generate and process. It's safe
@@ -118,6 +124,11 @@ struct tcp_request_sock {
* FastOpen it's the seq#
* after data-in-SYN.
*/
+ u8 syn_expopts[TCP_EXPOP_MAXLEN];
+ /* experimental options
+ * received with SYNACK */
+ u8 syn_expopts_len;
+
};
static inline struct tcp_request_sock *tcp_rsk(const struct request_sock *req)
@@ -210,6 +221,18 @@ struct tcp_sock {
u32 snd_up; /* Urgent pointer */
u8 keepalive_probes; /* num of allowed keep alive probes */
+
+/* for raw acces to experimental options */
+ struct {
+ u8 *conf; /* lazy allocation of TCP_EXPOP_MAXLEN bytes
+ * for raw access to experimental options */
+ u8 conf_len; /* bytes actually used for experimental opts */
+ u8 *syn; /* experimental options received with SYN,
+ * allocated only if received */
+ u8 syn_len; /* bytes of experimental options actually
+ * received with SYN */
+ } exp_opts;
+
/*
* Options received (usually on last packet, some only on SYN packets).
*/
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 4af45e3..8e0740c 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -180,6 +180,8 @@ extern void tcp_time_wait(struct sock *sk, int state, int timeo);
#define TCPOPT_TIMESTAMP 8 /* Better RTT estimations/PAWS */
#define TCPOPT_MD5SIG 19 /* MD5 Signature (RFC2385) */
#define TCPOPT_COOKIE 253 /* Cookie extension (experimental) */
+#define TCPOPT_EXP253 253 /* TCP experimental option 253 */
+#define TCPOPT_EXP254 254 /* TCP experimental option 254 */
#define TCPOPT_EXP 254 /* Experimental */
/* Magic number to be after the option value for sharing TCP
* experimental options. See draft-ietf-tcpm-experimental-options-00.txt
@@ -190,6 +192,7 @@ extern void tcp_time_wait(struct sock *sk, int state, int timeo);
* TCP option lengths
*/
+#define TCPOLEN_MAX_ANYEXP 40
#define TCPOLEN_MSS 4
#define TCPOLEN_WINDOW 3
#define TCPOLEN_SACK_PERM 2
diff --git a/include/uapi/linux/tcp.h b/include/uapi/linux/tcp.h
index e962faa..a6554d1 100644
--- a/include/uapi/linux/tcp.h
+++ b/include/uapi/linux/tcp.h
@@ -111,6 +111,9 @@ enum {
#define TCP_QUEUE_SEQ 21
#define TCP_REPAIR_OPTIONS 22
#define TCP_FASTOPEN 23 /* Enable FastOpen on listeners */
+#define TCP_EXPOPTS 24 /* TCP exp. options (configured) */
+#define TCP_RECV_EXPOPTS 25 /* TCP exp. options (received) */
+#define TCP_RECV_SYN_EXPOPTS 26 /* TCP exp. options (rec. with syn)) */
struct tcp_repair_opt {
__u32 opt_code;
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index e457c7a..437c22e 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -423,6 +423,12 @@ void tcp_init_sock(struct sock *sk)
sk->sk_sndbuf = sysctl_tcp_wmem[1];
sk->sk_rcvbuf = sysctl_tcp_rmem[1];
+ /* memory for raw access to experimental options is allocated lazy */
+ tp->exp_opts.conf = NULL;
+ tp->exp_opts.conf_len = 0;
+ tp->exp_opts.syn = NULL;
+ tp->exp_opts.syn_len = 0;
+
local_bh_disable();
sock_update_memcg(sk);
sk_sockets_allocated_inc(sk);
@@ -2366,6 +2372,56 @@ static int do_tcp_setsockopt(struct sock *sk, int level,
/* These are data/string values, all the others are ints */
switch (optname) {
+ case TCP_EXPOPTS: {
+ u8 conf[TCP_EXPOP_MAXLEN];
+
+ if (!capable(CAP_NET_RAW))
+ return -EACCES;
+
+ if (optlen > TCP_EXPOP_MAXLEN || (optlen < 4 && optlen > 0) ||
+ (optlen % 4 > 0))
+ return -EINVAL;
+ if (optlen > 0 && !optval)
+ return -EINVAL;
+
+ /* filter for raw access to supported options */
+ if (optlen) {
+ u8 i;
+
+ if (copy_from_user(conf, optval, optlen))
+ return -EFAULT;
+
+ i = 0;
+ while (i < optlen) {
+ if (conf[i] != TCPOPT_EXP253 &&
+ conf[i] != TCPOPT_EXP254)
+ return -EINVAL;
+
+ if (i + 1 < optlen) {
+ i += conf[i+1];
+ if (i > optlen)
+ return -EINVAL;
+ } else {
+ return -EINVAL;
+ }
+ }
+ }
+
+ lock_sock(sk);
+ if (!optlen) {
+ tp->exp_opts.conf_len = 0;
+ release_sock(sk);
+ return 0;
+ }
+ if (!tp->exp_opts.conf) {
+ tp->exp_opts.conf = kzalloc(TCP_EXPOP_MAXLEN,
+ sk->sk_allocation);
+ }
+ memcpy(tp->exp_opts.conf, conf, optlen);
+ tp->exp_opts.conf_len = optlen;
+ release_sock(sk);
+ return err;
+ }
case TCP_CONGESTION: {
char name[TCP_CA_NAME_MAX];
@@ -2947,6 +3003,72 @@ static int do_tcp_getsockopt(struct sock *sk, int level,
case TCP_USER_TIMEOUT:
val = jiffies_to_msecs(icsk->icsk_user_timeout);
break;
+ case TCP_EXPOPTS: {
+ u8 exp_opts_len;
+
+ if (!capable(CAP_NET_RAW))
+ return -EACCES;
+
+ if (get_user(len, optlen))
+ return -EFAULT;
+ if (len < 0)
+ return -EINVAL;
+
+ exp_opts_len = tp->exp_opts.conf_len;
+
+ if (exp_opts_len > len)
+ return -EINVAL;
+ if (put_user(exp_opts_len, optlen))
+ return -EFAULT;
+ if (exp_opts_len && copy_to_user(optval, tp->exp_opts.conf,
+ exp_opts_len))
+ return -EFAULT;
+ return 0;
+ }
+ case TCP_RECV_EXPOPTS:
+ if (!capable(CAP_NET_RAW))
+ return -EACCES;
+
+ if (get_user(len, optlen))
+ return -EFAULT;
+ if (len < 0)
+ return -EINVAL;
+
+ if (len < tp->rx_opt.exp_opts_len)
+ return -EINVAL;
+
+ if (put_user(tp->rx_opt.exp_opts_len, optlen))
+ return -EFAULT;
+ if (copy_to_user(optval, tp->rx_opt.exp_opts,
+ tp->rx_opt.exp_opts_len))
+ return -EFAULT;
+ return 0;
+ case TCP_RECV_SYN_EXPOPTS: {
+ u8 exp_opts_len;
+
+ if (!capable(CAP_NET_RAW))
+ return -EACCES;
+
+ if (get_user(len, optlen))
+ return -EFAULT;
+ if (len < 0)
+ return -EINVAL;
+
+ if (!tp->exp_opts.syn)
+ exp_opts_len = 0;
+ else
+ exp_opts_len = tp->exp_opts.syn_len;
+
+ if (exp_opts_len > len)
+ return -EINVAL;
+ if (put_user(exp_opts_len, optlen))
+ return -EFAULT;
+ if (exp_opts_len && copy_to_user(optval, tp->exp_opts.syn,
+ exp_opts_len)) {
+ return -EFAULT;
+ }
+ return 0;
+ }
default:
return -ENOPROTOOPT;
}
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 181fc82..ab9c88b 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -3699,11 +3699,32 @@ old_ack:
return 0;
}
+static inline void tcp_parse_fastopen_cookie(int opcode,
+ int opsize,
+ const unsigned char *ptr,
+ struct tcp_fastopen_cookie *foc,
+ const struct tcphdr *th) {
+ /* Fast Open option shares code 254 using a 16 bits magic number. It's
+ * valid only in SYN or SYN-ACK with an even size.
+ */
+ if (opsize < TCPOLEN_EXP_FASTOPEN_BASE ||
+ get_unaligned_be16(ptr) != TCPOPT_FASTOPEN_MAGIC || foc == NULL ||
+ !th->syn || (opsize & 1))
+ return;
+ foc->len = opsize - TCPOLEN_EXP_FASTOPEN_BASE;
+ if (foc->len >= TCP_FASTOPEN_COOKIE_MIN &&
+ foc->len <= TCP_FASTOPEN_COOKIE_MAX)
+ memcpy(foc->val, ptr + 2, foc->len);
+ else if (foc->len != 0)
+ foc->len = -1;
+}
+
/* Look for tcp options. Normally only called on SYN and SYNACK packets.
* But, this can also be called on packets in the established flow when
* the fast version below fails.
*/
-void tcp_parse_options(const struct sk_buff *skb, struct tcp_options_received *opt_rx,
+void tcp_parse_options(const struct sk_buff *skb,
+ struct tcp_options_received *opt_rx,
const u8 **hvpp, int estab,
struct tcp_fastopen_cookie *foc)
{
@@ -3713,6 +3734,7 @@ void tcp_parse_options(const struct sk_buff *skb, struct tcp_options_received *o
ptr = (const unsigned char *)(th + 1);
opt_rx->saw_tstamp = 0;
+ opt_rx->exp_opts_len = 0;
while (length > 0) {
int opcode = *ptr++;
@@ -3788,48 +3810,56 @@ void tcp_parse_options(const struct sk_buff *skb, struct tcp_options_received *o
*/
break;
#endif
- case TCPOPT_COOKIE:
- /* This option is variable length.
+ case TCPOPT_EXP253:
+ case TCPOPT_EXP254:
+ /* First parse options into raw access area for
+ * experimental options. Then handle
+ * potential exploitations
*/
- switch (opsize) {
- case TCPOLEN_COOKIE_BASE:
- /* not yet implemented */
- break;
- case TCPOLEN_COOKIE_PAIR:
- /* not yet implemented */
- break;
- case TCPOLEN_COOKIE_MIN+0:
- case TCPOLEN_COOKIE_MIN+2:
- case TCPOLEN_COOKIE_MIN+4:
- case TCPOLEN_COOKIE_MIN+6:
- case TCPOLEN_COOKIE_MAX:
- /* 16-bit multiple */
- opt_rx->cookie_plus = opsize;
- *hvpp = ptr;
- break;
- default:
- /* ignore option */
- break;
+ if (opsize <= TCPOLEN_MAX_ANYEXP &&
+ opsize >= 2 &&
+ (opt_rx->exp_opts_len + opsize <=
+ TCPOLEN_MAX_ANYEXP)) {
+ opt_rx->exp_opts[
+ opt_rx->exp_opts_len] = opcode;
+ opt_rx->exp_opts[
+ opt_rx->exp_opts_len + 1] =
+ opsize;
+ memcpy(opt_rx->exp_opts +
+ opt_rx->exp_opts_len + 2, ptr,
+ opsize - 2);
+ opt_rx->exp_opts_len += opsize;
}
- break;
- case TCPOPT_EXP:
- /* Fast Open option shares code 254 using a
- * 16 bits magic number. It's valid only in
- * SYN or SYN-ACK with an even size.
- */
- if (opsize < TCPOLEN_EXP_FASTOPEN_BASE ||
- get_unaligned_be16(ptr) != TCPOPT_FASTOPEN_MAGIC ||
- foc == NULL || !th->syn || (opsize & 1))
- break;
- foc->len = opsize - TCPOLEN_EXP_FASTOPEN_BASE;
- if (foc->len >= TCP_FASTOPEN_COOKIE_MIN &&
- foc->len <= TCP_FASTOPEN_COOKIE_MAX)
- memcpy(foc->val, ptr + 2, foc->len);
- else if (foc->len != 0)
- foc->len = -1;
+ /* handle potential exploitations */
+ if (opcode == TCPOPT_COOKIE) {
+ /* This option is variable length. */
+ switch (opsize) {
+ case TCPOLEN_COOKIE_BASE:
+ /* not yet implemented */
+ break;
+ case TCPOLEN_COOKIE_PAIR:
+ /* not yet implemented */
+ break;
+ case TCPOLEN_COOKIE_MIN+0:
+ case TCPOLEN_COOKIE_MIN+2:
+ case TCPOLEN_COOKIE_MIN+4:
+ case TCPOLEN_COOKIE_MIN+6:
+ case TCPOLEN_COOKIE_MAX:
+ /* 16-bit multiple */
+ opt_rx->cookie_plus = opsize;
+ *hvpp = ptr;
+ break;
+ default:
+ /* ignore option */
+ break;
+ }
+ } else {
+ tcp_parse_fastopen_cookie(opcode,
+ opsize, ptr,
+ foc, th);
+ }
break;
-
}
ptr += opsize-2;
length -= opsize;
@@ -3861,6 +3891,9 @@ static bool tcp_fast_parse_options(const struct sk_buff *skb,
const struct tcphdr *th,
struct tcp_sock *tp, const u8 **hvpp)
{
+ /* required if exp options are not used anymore by the counter part */
+ tp->rx_opt.exp_opts_len = 0;
+
/* In the spirit of fast parsing, compare doff directly to constant
* values. Because equality is used, short doff can be ignored here.
*/
@@ -5786,6 +5819,14 @@ static int tcp_rcv_synsent_state_process(struct sock *sk, struct sk_buff *skb,
}
}
+ if (unlikely(tp->rx_opt.exp_opts_len > 0)) {
+ tp->exp_opts.syn = kzalloc(tp->rx_opt.exp_opts_len,
+ sk->sk_allocation);
+ tp->exp_opts.syn_len = tp->rx_opt.exp_opts_len;
+ memcpy(tp->exp_opts.syn, &tp->rx_opt.exp_opts,
+ tp->rx_opt.exp_opts_len);
+ }
+
smp_mb();
tcp_finish_connect(sk, skb);
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 0c4a643..b7ab768 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1525,6 +1525,16 @@ int tcp_v4_conn_request(struct sock *sk, struct sk_buff *skb)
tcp_parse_options(skb, &tmp_opt, &hash_location, 0,
want_cookie ? NULL : &foc);
+ /* for raw access to experimental options in SYN packet */
+ tcp_rsk(req)->syn_expopts_len = tmp_opt.exp_opts_len;
+ if (tcp_rsk(req)->syn_expopts_len) {
+ /* transport experimental options via request socket to big
+ * socket
+ */
+ memcpy(tcp_rsk(req)->syn_expopts, tmp_opt.exp_opts,
+ tcp_rsk(req)->syn_expopts_len);
+ }
+
if (tmp_opt.cookie_plus > 0 &&
tmp_opt.saw_tstamp &&
!tp->rx_opt.cookie_out_never &&
@@ -2209,6 +2219,10 @@ void tcp_v4_destroy_sock(struct sock *sk)
}
BUG_ON(tp->fastopen_rsk != NULL);
+ /* buffers for raw access to experimental options */
+ kfree(tp->exp_opts.conf);
+ kfree(tp->exp_opts.syn);
+
/* If socket is aborted during connect operation */
tcp_free_fastopen_req(tp);
diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
index a7302d9..1ca8980 100644
--- a/net/ipv4/tcp_minisocks.c
+++ b/net/ipv4/tcp_minisocks.c
@@ -468,6 +468,23 @@ struct sock *tcp_create_openreq_child(struct sock *sk, struct request_sock *req,
newtp->urg_data = 0;
+ if (tcp_rsk(req)->syn_expopts_len) {
+ newtp->exp_opts.syn_len =
+ tcp_rsk(req)->syn_expopts_len;
+ newtp->exp_opts.syn = kzalloc(newtp->exp_opts.syn_len,
+ GFP_ATOMIC);
+ memcpy(newtp->exp_opts.syn, tcp_rsk(req)->syn_expopts,
+ newtp->exp_opts.syn_len);
+ }
+
+ if (oldtp->exp_opts.conf_len > 0) {
+ newtp->exp_opts.conf_len = oldtp->exp_opts.conf_len;
+ newtp->exp_opts.conf = kzalloc(TCP_EXPOP_MAXLEN,
+ GFP_ATOMIC);
+ memcpy(newtp->exp_opts.conf, oldtp->exp_opts.conf,
+ oldtp->exp_opts.conf_len);
+ }
+
if (sock_flag(newsk, SOCK_KEEPOPEN))
inet_csk_reset_keepalive_timer(newsk,
keepalive_time_when(newtp));
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 948ac27..a57470a 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -385,6 +385,7 @@ static inline bool tcp_urg_mode(const struct tcp_sock *tp)
#define OPTION_MD5 (1 << 2)
#define OPTION_WSCALE (1 << 3)
#define OPTION_COOKIE_EXTENSION (1 << 4)
+#define OPTION_EXP (1 << 5)
#define OPTION_FAST_OPEN_COOKIE (1 << 8)
struct tcp_out_options {
@@ -581,6 +582,12 @@ static void tcp_options_write(__be32 *ptr, struct tcp_sock *tp,
}
ptr += (foc->len + 3) >> 2;
}
+ if (unlikely(OPTION_EXP & options && tp->exp_opts.conf_len > 0)) {
+ __u8 *p = (__u8 *) ptr;
+ memcpy(ptr, tp->exp_opts.conf, tp->exp_opts.conf_len);
+ p += tp->exp_opts.conf_len;
+ ptr = (__be32 *) p;
+ }
}
/* Compute TCP options for SYN packets. This is not the final
@@ -693,6 +700,11 @@ static unsigned int tcp_syn_options(struct sock *sk, struct sk_buff *skb,
remaining -= need;
}
}
+ if (unlikely(tp->exp_opts.conf_len > 0 &&
+ tp->exp_opts.conf_len <= remaining)) {
+ opts->options |= OPTION_EXP;
+ remaining -= tp->exp_opts.conf_len;
+ }
return MAX_TCP_OPTION_SPACE - remaining;
}
@@ -748,15 +760,6 @@ static unsigned int tcp_synack_options(struct sock *sk,
if (unlikely(!ireq->tstamp_ok))
remaining -= TCPOLEN_SACKPERM_ALIGNED;
}
- if (foc != NULL) {
- u32 need = TCPOLEN_EXP_FASTOPEN_BASE + foc->len;
- need = (need + 3) & ~3U; /* Align to 32 bits */
- if (remaining >= need) {
- opts->options |= OPTION_FAST_OPEN_COOKIE;
- opts->fastopen_cookie = foc;
- remaining -= need;
- }
- }
/* Similar rationale to tcp_syn_options() applies here, too.
* If the <SYN> options fit, the same options should fit now!
*/
@@ -779,6 +782,12 @@ static unsigned int tcp_synack_options(struct sock *sk,
opts->hash_size = 0;
}
}
+
+ if (unlikely(tcp_sk(sk)->exp_opts.conf_len > 0 &&
+ tcp_sk(sk)->exp_opts.conf_len <= remaining)) {
+ opts->options |= OPTION_EXP;
+ remaining -= tcp_sk(sk)->exp_opts.conf_len;
+ }
return MAX_TCP_OPTION_SPACE - remaining;
}
@@ -791,38 +800,44 @@ static unsigned int tcp_established_options(struct sock *sk, struct sk_buff *skb
{
struct tcp_skb_cb *tcb = skb ? TCP_SKB_CB(skb) : NULL;
struct tcp_sock *tp = tcp_sk(sk);
- unsigned int size = 0;
+ unsigned remaining = MAX_TCP_OPTION_SPACE;
unsigned int eff_sacks;
#ifdef CONFIG_TCP_MD5SIG
*md5 = tp->af_specific->md5_lookup(sk, sk);
if (unlikely(*md5)) {
opts->options |= OPTION_MD5;
- size += TCPOLEN_MD5SIG_ALIGNED;
+ remaining -= TCPOLEN_MD5SIG_ALIGNED;
}
#else
*md5 = NULL;
#endif
- if (likely(tp->rx_opt.tstamp_ok)) {
+ if (likely(tp->rx_opt.tstamp_ok &&
+ remaining >= TCPOLEN_TSTAMP_ALIGNED)) {
opts->options |= OPTION_TS;
opts->tsval = tcb ? tcb->when : 0;
opts->tsecr = tp->rx_opt.ts_recent;
- size += TCPOLEN_TSTAMP_ALIGNED;
+ remaining -= TCPOLEN_TSTAMP_ALIGNED;
}
eff_sacks = tp->rx_opt.num_sacks + tp->rx_opt.dsack;
if (unlikely(eff_sacks)) {
- const unsigned int remaining = MAX_TCP_OPTION_SPACE - size;
opts->num_sack_blocks =
min_t(unsigned int, eff_sacks,
(remaining - TCPOLEN_SACK_BASE_ALIGNED) /
TCPOLEN_SACK_PERBLOCK);
- size += TCPOLEN_SACK_BASE_ALIGNED +
+ remaining -= TCPOLEN_SACK_BASE_ALIGNED +
opts->num_sack_blocks * TCPOLEN_SACK_PERBLOCK;
}
- return size;
+ if (unlikely(tp->exp_opts.conf_len > 0 &&
+ tp->exp_opts.conf_len <= remaining)) {
+ opts->options |= OPTION_EXP;
+ remaining -= tp->exp_opts.conf_len;
+ }
+
+ return MAX_TCP_OPTION_SPACE - remaining;
}
--
1.7.12.4
^ permalink raw reply related
* [RFC PATCH V2] tcp: introduce raw access to experimental options
From: elelueck @ 2013-01-07 16:13 UTC (permalink / raw)
To: netdev; +Cc: Einar Lueck, davem, ubacher, raspl, frankbla, samudrala
From: Einar Lueck <elelueck@linux.vnet.ibm.com>
V2 addresses the privilige concern. CAP_NET_RAW is now required for all of the
new socket options.
Einar Lueck (1):
tcp: introduce raw access to experimental options
include/linux/tcp.h | 23 +++++++++
include/net/tcp.h | 3 ++
include/uapi/linux/tcp.h | 3 ++
net/ipv4/tcp.c | 122 +++++++++++++++++++++++++++++++++++++++++++++++
net/ipv4/tcp_input.c | 119 ++++++++++++++++++++++++++++++---------------
net/ipv4/tcp_ipv4.c | 14 ++++++
net/ipv4/tcp_minisocks.c | 17 +++++++
net/ipv4/tcp_output.c | 47 +++++++++++-------
8 files changed, 293 insertions(+), 55 deletions(-)
--
1.7.12.4
^ permalink raw reply
* [BUG] via_rhine stops making new tcp connections.
From: Stephen Hemminger @ 2013-01-07 15:55 UTC (permalink / raw)
To: Roger Luethi; +Cc: netdev
Please look at this bug report from davidwhodgins@gmail.com
https://bugzilla.kernel.org/show_bug.cgi?id=52151
As shown in the pcap file
https://bugs.mageia.org/attachment.cgi?id=3304
after a period of time ranging from hours to weeks,
the driver starts replying to syn,ack messages with reset,
instead of ack messages.
Rebooting corrects the problem, as shown by the pcap file
https://bugs.mageia.org/attachment.cgi?id=3305
^ permalink raw reply
* Re: [PATCH v2] sctp: Change defaults on cookie hmac selection
From: Vlad Yasevich @ 2013-01-07 15:48 UTC (permalink / raw)
To: Neil Horman
Cc: Florian Fainelli, netdev, David Miller, Linus Torvalds,
linux-sctp
In-Reply-To: <20130107153812.GC31577@hmsreliant.think-freely.org>
On 01/07/2013 10:38 AM, Neil Horman wrote:
> On Mon, Jan 07, 2013 at 04:15:24PM +0100, Florian Fainelli wrote:
>> Le 01/07/13 15:49, Neil Horman a écrit :
>>> On Mon, Jan 07, 2013 at 02:25:39PM +0100, Florian Fainelli wrote:
>>>> Hello Neil,
>>>>
>>>> Le 12/15/12 02:22, Neil Horman a écrit :
>>>>> Recently I posted commit 3c68198e75 which made selection of the cookie hmac
>>>>> algorithm selectable. This is all well and good, but Linus noted that it
>>>>> changes the default config:
>>>>> http://marc.info/?l=linux-netdev&m=135536629004808&w=2
>>>>>
>>>>> I've modified the sctp Kconfig file to reflect the recommended way of making
>>>>> this choice, using the thermal driver example specified, and brought the
>>>>> defaults back into line with the way they were prior to my origional patch
>>>>>
>>>>> Also, on Linus' suggestion, re-adding ability to select default 'none' hmac
>>>>> algorithm, so we don't needlessly bloat the kernel by forcing a non-none
>>>>> default. This also led me to note that we won't honor the default none
>>>>> condition properly because of how sctp_net_init is encoded. Fix that up as
>>>>> well.
>>>>>
>>>>> Tested by myself (allbeit fairly quickly). All configuration combinations seems
>>>>> to work soundly.
>>>>>
>>>>> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
>>>>> CC: David Miller <davem@davemloft.net>
>>>>> CC: Linus Torvalds <torvalds@linux-foundation.org>
>>>>> CC: Vlad Yasevich <vyasevich@gmail.com>
>>>>> CC: linux-sctp@vger.kernel.org
>>>>> ---
>>>>> net/sctp/Kconfig | 27 +++++++++++++++++++++++++--
>>>>> net/sctp/protocol.c | 4 ++--
>>>>> 2 files changed, 27 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/net/sctp/Kconfig b/net/sctp/Kconfig
>>>>> index a9edd2e..c262106 100644
>>>>> --- a/net/sctp/Kconfig
>>>>> +++ b/net/sctp/Kconfig
>>>>> @@ -66,12 +66,36 @@ config SCTP_DBG_OBJCNT
>>>>> 'cat /proc/net/sctp/sctp_dbg_objcnt'
>>>>>
>>>>> If unsure, say N
>>>>> +choice
>>>>> + prompt "Default SCTP cookie HMAC encoding"
>>>>> + default SCTP_COOKIE_HMAC_MD5
>>>> Should not this be SCTP_DEFAULT_COOKIE_HMAC_MD5? I just tried to
>>>> update to 3.8-rc2, and I usually build my kernel-headers with:
>>>>
>>>> yes '' | ARCH=foo make oldconfig
>>>>
>>>> and this just kept asking me for this config symbol because none
>>>> could be provided.
>>>> --
>>>> Florian
>>>>
>>> No, the config mechanism is setup to offer the user the ability to choose a
>>> default cookie hmac, alg, then optionally select any other hmac algs you would
>>> like to be made available (in the event you want to change the default at run
>>> time). When you select the default, it eables (via the select directive), the
>>> corresponding SCTP_COOKIE_HMAC_* config option, which is used in the build, and
>>> then prompts for the remaining values.
>>
>> Ok for the explanation, but this still breaks an oldconfig because
>> we do not actually propose the user with a default choice:
>>
>> choice[1-3?]: Default SCTP cookie HMAC encoding
>> 1. Enable optional MD5 hmac cookie generation
>> (SCTP_DEFAULT_COOKIE_HMAC_MD5) (NEW)
>> 2. Enable optional SHA1 hmac cookie generation
>> (SCTP_DEFAULT_COOKIE_HMAC_SHA1) (NEW)
>> 3. Use no hmac alg in SCTP cookie generation
>> (SCTP_DEFAULT_COOKIE_HMAC_NONE) (NEW)
>>
>> I do not see any difference in what I am proposed if the default
>> config symbol is SCTP_DEFAULT_COOKIE_HMAC_MD5, I can still
>> optionally choose SHA1 to be supported, and I do have a valid
>> default config for this choice. While if I keep SCTP_COOKIE_HMAC_MD5
> No, thats the problem, your old config is no longer valid with this new Kconfig
> file. Your config is telling the config utility that you want your default
> Cookie hmac to be MD5, but you've explicitly told it (via your yes "" | make
> oldconfig command), that you want SCTP_COOKIE_HMAC_MD5 to be disabled, so the
> config utility is left with no choice to prompt you again for a default hmac,
> which your command answers again by saying SCTP_DEFAULT_COOKIE_HMAC_MD5 (the
> default choice of 1). Thats your loop, you keep telling the config utility that
> you both want the default hmac to be md5, and that you don't want to allow md5
> to be an available hmac alg.
>
> Thats not a bug. I'm sorry if your old configuration needs manual updating, but
> there are no guarantees that old configurations will 'just work' in perpituity.
>
Neil
Actually, I think we have a bug in the config. Look at the thermal
driver config again. It has:
choice
prompt "Default Thermal governor"
default THERMAL_DEFAULT_GOV_STEP_WISE
config THERMAL_DEFAULT_GOV_STEP_WISE
...
config THERMAL_DEFAULT_GOV_FAIR_SHARE
...
config THERMAL_DEFAULT_GOV_USER_SPACE
...
endchoice
SCTP has:
choice
prompt "Default SCTP cookie HMAC encoding"
default SCTP_COOKIE_HMAC_MD5
config SCTP_DEFAULT_COOKIE_HMAC_MD5
...
config SCTP_DEFAULT_COOKIE_HMAC_SHA1
...
config SCTP_DEFAULT_COOKIE_HMAC_NONE
...
endchoice
See the difference? The default value of the choice statement needs to
be one of the available choices.
-vlad
> Neil
>
>> as the default I have to manually enter which option I want.
>> --
>> Florian
>> --
>> 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
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" 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 v2] sctp: Change defaults on cookie hmac selection
From: Neil Horman @ 2013-01-07 15:46 UTC (permalink / raw)
To: Vlad Yasevich
Cc: Florian Fainelli, netdev, David Miller, Linus Torvalds,
linux-sctp
In-Reply-To: <50EAEA71.1060007@gmail.com>
On Mon, Jan 07, 2013 at 10:32:01AM -0500, Vlad Yasevich wrote:
> On 01/07/2013 09:49 AM, Neil Horman wrote:
> >On Mon, Jan 07, 2013 at 02:25:39PM +0100, Florian Fainelli wrote:
> >>Hello Neil,
> >>
> >>Le 12/15/12 02:22, Neil Horman a écrit :
> >>>Recently I posted commit 3c68198e75 which made selection of the cookie hmac
> >>>algorithm selectable. This is all well and good, but Linus noted that it
> >>>changes the default config:
> >>>http://marc.info/?l=linux-netdev&m=135536629004808&w=2
> >>>
> >>>I've modified the sctp Kconfig file to reflect the recommended way of making
> >>>this choice, using the thermal driver example specified, and brought the
> >>>defaults back into line with the way they were prior to my origional patch
> >>>
> >>>Also, on Linus' suggestion, re-adding ability to select default 'none' hmac
> >>>algorithm, so we don't needlessly bloat the kernel by forcing a non-none
> >>>default. This also led me to note that we won't honor the default none
> >>>condition properly because of how sctp_net_init is encoded. Fix that up as
> >>>well.
> >>>
> >>>Tested by myself (allbeit fairly quickly). All configuration combinations seems
> >>>to work soundly.
> >>>
> >>>Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> >>>CC: David Miller <davem@davemloft.net>
> >>>CC: Linus Torvalds <torvalds@linux-foundation.org>
> >>>CC: Vlad Yasevich <vyasevich@gmail.com>
> >>>CC: linux-sctp@vger.kernel.org
> >>>---
> >>> net/sctp/Kconfig | 27 +++++++++++++++++++++++++--
> >>> net/sctp/protocol.c | 4 ++--
> >>> 2 files changed, 27 insertions(+), 4 deletions(-)
> >>>
> >>>diff --git a/net/sctp/Kconfig b/net/sctp/Kconfig
> >>>index a9edd2e..c262106 100644
> >>>--- a/net/sctp/Kconfig
> >>>+++ b/net/sctp/Kconfig
> >>>@@ -66,12 +66,36 @@ config SCTP_DBG_OBJCNT
> >>> 'cat /proc/net/sctp/sctp_dbg_objcnt'
> >>>
> >>> If unsure, say N
> >>>+choice
> >>>+ prompt "Default SCTP cookie HMAC encoding"
> >>>+ default SCTP_COOKIE_HMAC_MD5
> >>
> >>Should not this be SCTP_DEFAULT_COOKIE_HMAC_MD5? I just tried to
> >>update to 3.8-rc2, and I usually build my kernel-headers with:
> >>
> >>yes '' | ARCH=foo make oldconfig
> >>
> >>and this just kept asking me for this config symbol because none
> >>could be provided.
> >>--
> >>Florian
> >>
> >
> >No, the config mechanism is setup to offer the user the ability to choose a
> >default cookie hmac, alg, then optionally select any other hmac algs you would
> >like to be made available (in the event you want to change the default at run
> >time). When you select the default, it eables (via the select directive), the
> >corresponding SCTP_COOKIE_HMAC_* config option, which is used in the build, and
> >then prompts for the remaining values.
> >
>
> Neil
>
> Actually, I think it should be as Florian suggests. The default
> value of the choice should be one of the values defined as part of
> the choice (the SCTP_DEFAULT_COOKIE_*). Turning on appropriate
> default would turn on appropriate cookie config
> (SCTP_COOKIE_HMAC_*).
>
I absolutely disagree.
> Would that save all the config trouble?
>
Yes, it would fix it as Florian has noted, but at the cost of silently modifying
what the default hmac config vaule is. If you've expressly disabled
SCTP_COOKIE_HMAC_MD5, and then blindly take the default choice in the
SCTP_DEFAULT_COOKIE selection option (the default default as it were), using the
approach your suggesting, then that will silently enable SCTP_COOKIE_HMAC_MD5
again, which may not be expected by users. If you expressly have a config
option disabled in an old configuration, we should leave it there.
We're doing the right thing now, IMO. When presented with a conflictly set
of configuration options, the config utilty is (repeatedly) prompting us to
resolve them. That seems like a much more reasonable approach to this, than
silently changing pre-existing options so people can do the equivalent of just
blindly pressing enter through the config process (which is all yes "" | make
oldconfig is).
This is a momentary hiccup, corrected by taking 30 seconds to make a manual
config change (or by taking a second to understand what the config utility is
tell us by prompting for a default choice repeatedly). Theres nothing to fix
here.
Neil
> -vlad
>
> >Neil
> >
>
> --
> 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 1/2] ipv6: avoid blackhole and prohibited entries upon prefix purge [v2]
From: Nicolas Dichtel @ 2013-01-07 15:43 UTC (permalink / raw)
To: Romain KUNTZ; +Cc: netdev, Eric Dumazet, yoshfuji, davem
In-Reply-To: <3EF640F8-5242-486E-B7A3-9DA2A88F5A4F@ipflavors.com>
Le 07/01/2013 12:30, Romain KUNTZ a écrit :
> Hello Nicolas,
>
> On Jan 7, 2013, at 11:25 , Nicolas Dichtel <nicolas.dichtel@6wind.com> wrote:
>
>> Le 05/01/2013 22:44, Romain KUNTZ a écrit :
>>> Mobile IPv6 provokes a kernel Oops since commit 64c6d08e (ipv6:
>>> del unreachable route when an addr is deleted on lo), because
>>> ip6_route_lookup() may also return blackhole and prohibited
>>> entry. However, these entries have a NULL rt6i_table argument,
>>> which provokes an Oops in __ip6_del_rt() when trying to lock
>>> rt6i_table->tb6_lock.
>>>
>>> Beside, when purging a prefix, blakhole and prohibited entries
>>> should not be selected because they are not what we are looking
>>> for.
>>>
>>> We fix this by adding two new lookup flags (RT6_LOOKUP_F_NO_BLK_HOLE
>>> and RT6_LOOKUP_F_NO_PROHIBIT) in order to ensure that such entries
>>> are skipped during lookup and that the correct entry is returned.
>>>
>>> [v2]: use 'goto out;' instead of 'goto again;' to avoid unnecessary
>>> oprations on rt (as suggested by Eric Dumazet).
>>>
>>> Signed-off-by: Romain Kuntz <r.kuntz@ipflavors.com>
>>> ---
>>> include/net/ip6_route.h | 2 ++
>>> net/ipv6/addrconf.c | 4 +++-
>>> net/ipv6/fib6_rules.c | 4 ++++
>>> 3 files changed, 9 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/include/net/ip6_route.h b/include/net/ip6_route.h
>>> index 27d8318..3c93743 100644
>>> --- a/include/net/ip6_route.h
>>> +++ b/include/net/ip6_route.h
>>> @@ -30,6 +30,8 @@ struct route_info {
>>> #define RT6_LOOKUP_F_SRCPREF_TMP 0x00000008
>>> #define RT6_LOOKUP_F_SRCPREF_PUBLIC 0x00000010
>>> #define RT6_LOOKUP_F_SRCPREF_COA 0x00000020
>>> +#define RT6_LOOKUP_F_NO_BLK_HOLE 0x00000040
>>> +#define RT6_LOOKUP_F_NO_PROHIBIT 0x00000080
>>>
>>> /*
>>> * rt6_srcprefs2flags() and rt6_flags2srcprefs() translate
>>> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
>>> index 408cac4a..1891e23 100644
>>> --- a/net/ipv6/addrconf.c
>>> +++ b/net/ipv6/addrconf.c
>>> @@ -948,7 +948,9 @@ static void ipv6_del_addr(struct inet6_ifaddr *ifp)
>>> fl6.flowi6_oif = ifp->idev->dev->ifindex;
>>> fl6.daddr = prefix;
>>> rt = (struct rt6_info *)ip6_route_lookup(net, &fl6,
>>> - RT6_LOOKUP_F_IFACE);
>>> + RT6_LOOKUP_F_IFACE |
>>> + RT6_LOOKUP_F_NO_BLK_HOLE |
>>> + RT6_LOOKUP_F_NO_PROHIBIT);
>>>
>>> if (rt != net->ipv6.ip6_null_entry &&
>> Is it not simpler to test the result here (net->ipv6.ip6_blk_hole_entry and
>> net->ipv6.ip6_prohibit_entry) like for the null_entry?
>> It will also avoid adding more flags.
>
> Your proposal would only solve part of the problem (the Oops in __ip6_del_rt()). Another problem here is that blackhole and prohibited rules should not be selected when trying to purge a prefix (correct me if I'm wrong) because they are not what we are looking for. This can prevent the targeted prefix from being purged.
In fact, I'm not sure to get the scenario. This part of the code just tries
to remove the connected prefix, added by the kernel when the address was added.
Can you describe your scenario?
^ permalink raw reply
* Re: [PATCH v2] sctp: Change defaults on cookie hmac selection
From: Neil Horman @ 2013-01-07 15:38 UTC (permalink / raw)
To: Florian Fainelli
Cc: netdev, David Miller, Linus Torvalds, Vlad Yasevich, linux-sctp
In-Reply-To: <50EAE68C.2050300@openwrt.org>
On Mon, Jan 07, 2013 at 04:15:24PM +0100, Florian Fainelli wrote:
> Le 01/07/13 15:49, Neil Horman a écrit :
> >On Mon, Jan 07, 2013 at 02:25:39PM +0100, Florian Fainelli wrote:
> >>Hello Neil,
> >>
> >>Le 12/15/12 02:22, Neil Horman a écrit :
> >>>Recently I posted commit 3c68198e75 which made selection of the cookie hmac
> >>>algorithm selectable. This is all well and good, but Linus noted that it
> >>>changes the default config:
> >>>http://marc.info/?l=linux-netdev&m=135536629004808&w=2
> >>>
> >>>I've modified the sctp Kconfig file to reflect the recommended way of making
> >>>this choice, using the thermal driver example specified, and brought the
> >>>defaults back into line with the way they were prior to my origional patch
> >>>
> >>>Also, on Linus' suggestion, re-adding ability to select default 'none' hmac
> >>>algorithm, so we don't needlessly bloat the kernel by forcing a non-none
> >>>default. This also led me to note that we won't honor the default none
> >>>condition properly because of how sctp_net_init is encoded. Fix that up as
> >>>well.
> >>>
> >>>Tested by myself (allbeit fairly quickly). All configuration combinations seems
> >>>to work soundly.
> >>>
> >>>Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> >>>CC: David Miller <davem@davemloft.net>
> >>>CC: Linus Torvalds <torvalds@linux-foundation.org>
> >>>CC: Vlad Yasevich <vyasevich@gmail.com>
> >>>CC: linux-sctp@vger.kernel.org
> >>>---
> >>> net/sctp/Kconfig | 27 +++++++++++++++++++++++++--
> >>> net/sctp/protocol.c | 4 ++--
> >>> 2 files changed, 27 insertions(+), 4 deletions(-)
> >>>
> >>>diff --git a/net/sctp/Kconfig b/net/sctp/Kconfig
> >>>index a9edd2e..c262106 100644
> >>>--- a/net/sctp/Kconfig
> >>>+++ b/net/sctp/Kconfig
> >>>@@ -66,12 +66,36 @@ config SCTP_DBG_OBJCNT
> >>> 'cat /proc/net/sctp/sctp_dbg_objcnt'
> >>>
> >>> If unsure, say N
> >>>+choice
> >>>+ prompt "Default SCTP cookie HMAC encoding"
> >>>+ default SCTP_COOKIE_HMAC_MD5
> >>Should not this be SCTP_DEFAULT_COOKIE_HMAC_MD5? I just tried to
> >>update to 3.8-rc2, and I usually build my kernel-headers with:
> >>
> >>yes '' | ARCH=foo make oldconfig
> >>
> >>and this just kept asking me for this config symbol because none
> >>could be provided.
> >>--
> >>Florian
> >>
> >No, the config mechanism is setup to offer the user the ability to choose a
> >default cookie hmac, alg, then optionally select any other hmac algs you would
> >like to be made available (in the event you want to change the default at run
> >time). When you select the default, it eables (via the select directive), the
> >corresponding SCTP_COOKIE_HMAC_* config option, which is used in the build, and
> >then prompts for the remaining values.
>
> Ok for the explanation, but this still breaks an oldconfig because
> we do not actually propose the user with a default choice:
>
> choice[1-3?]: Default SCTP cookie HMAC encoding
> 1. Enable optional MD5 hmac cookie generation
> (SCTP_DEFAULT_COOKIE_HMAC_MD5) (NEW)
> 2. Enable optional SHA1 hmac cookie generation
> (SCTP_DEFAULT_COOKIE_HMAC_SHA1) (NEW)
> 3. Use no hmac alg in SCTP cookie generation
> (SCTP_DEFAULT_COOKIE_HMAC_NONE) (NEW)
>
> I do not see any difference in what I am proposed if the default
> config symbol is SCTP_DEFAULT_COOKIE_HMAC_MD5, I can still
> optionally choose SHA1 to be supported, and I do have a valid
> default config for this choice. While if I keep SCTP_COOKIE_HMAC_MD5
No, thats the problem, your old config is no longer valid with this new Kconfig
file. Your config is telling the config utility that you want your default
Cookie hmac to be MD5, but you've explicitly told it (via your yes "" | make
oldconfig command), that you want SCTP_COOKIE_HMAC_MD5 to be disabled, so the
config utility is left with no choice to prompt you again for a default hmac,
which your command answers again by saying SCTP_DEFAULT_COOKIE_HMAC_MD5 (the
default choice of 1). Thats your loop, you keep telling the config utility that
you both want the default hmac to be md5, and that you don't want to allow md5
to be an available hmac alg.
Thats not a bug. I'm sorry if your old configuration needs manual updating, but
there are no guarantees that old configurations will 'just work' in perpituity.
Neil
> as the default I have to manually enter which option I want.
> --
> Florian
> --
> 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
* [PATCH V2] xen/netfront: improve truesize tracking
From: Ian Campbell @ 2013-01-07 15:32 UTC (permalink / raw)
To: netdev
Cc: Ian Campbell, Sander Eikelenboom, Konrad Rzeszutek Wilk, annie li,
xen-devel
Using RX_COPY_THRESHOLD is incorrect if the SKB is actually smaller
than that. We have already accounted for this in
NETFRONT_SKB_CB(skb)->pull_to so use that instead.
Fixes WARN_ON from skb_try_coalesce.
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Cc: Sander Eikelenboom <linux@eikelenboom.it>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: annie li <annie.li@oracle.com>
Cc: xen-devel@lists.xen.org
Cc: netdev@vger.kernel.org
Cc: stable@kernel.org # 3.7.x only
---
v2:
- acount frags as a full page as recommended by Eric.
---
drivers/net/xen-netfront.c | 27 ++++-----------------------
1 files changed, 4 insertions(+), 23 deletions(-)
diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index caa0110..b518538 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -959,29 +959,10 @@ err:
i = xennet_fill_frags(np, skb, &tmpq);
/*
- * Truesize approximates the size of true data plus
- * any supervisor overheads. Adding hypervisor
- * overheads has been shown to significantly reduce
- * achievable bandwidth with the default receive
- * buffer size. It is therefore not wise to account
- * for it here.
- *
- * After alloc_skb(RX_COPY_THRESHOLD), truesize is set
- * to RX_COPY_THRESHOLD + the supervisor
- * overheads. Here, we add the size of the data pulled
- * in xennet_fill_frags().
- *
- * We also adjust for any unused space in the main
- * data area by subtracting (RX_COPY_THRESHOLD -
- * len). This is especially important with drivers
- * which split incoming packets into header and data,
- * using only 66 bytes of the main data area (see the
- * e1000 driver for example.) On such systems,
- * without this last adjustement, our achievable
- * receive throughout using the standard receive
- * buffer size was cut by 25%(!!!).
- */
- skb->truesize += skb->data_len - RX_COPY_THRESHOLD;
+ * Truesize is the actual allocation size, even if the
+ * allocation is only partially used.
+ */
+ skb->truesize += PAGE_SIZE * skb_shinfo(skb)->nr_frags;
skb->len += skb->data_len;
if (rx->flags & XEN_NETRXF_csum_blank)
--
1.7.2.5
^ permalink raw reply related
* Re: [PATCH v2] sctp: Change defaults on cookie hmac selection
From: Vlad Yasevich @ 2013-01-07 15:32 UTC (permalink / raw)
To: Neil Horman
Cc: Florian Fainelli, netdev, David Miller, Linus Torvalds,
linux-sctp
In-Reply-To: <20130107144921.GA31577@hmsreliant.think-freely.org>
On 01/07/2013 09:49 AM, Neil Horman wrote:
> On Mon, Jan 07, 2013 at 02:25:39PM +0100, Florian Fainelli wrote:
>> Hello Neil,
>>
>> Le 12/15/12 02:22, Neil Horman a écrit :
>>> Recently I posted commit 3c68198e75 which made selection of the cookie hmac
>>> algorithm selectable. This is all well and good, but Linus noted that it
>>> changes the default config:
>>> http://marc.info/?l=linux-netdev&m=135536629004808&w=2
>>>
>>> I've modified the sctp Kconfig file to reflect the recommended way of making
>>> this choice, using the thermal driver example specified, and brought the
>>> defaults back into line with the way they were prior to my origional patch
>>>
>>> Also, on Linus' suggestion, re-adding ability to select default 'none' hmac
>>> algorithm, so we don't needlessly bloat the kernel by forcing a non-none
>>> default. This also led me to note that we won't honor the default none
>>> condition properly because of how sctp_net_init is encoded. Fix that up as
>>> well.
>>>
>>> Tested by myself (allbeit fairly quickly). All configuration combinations seems
>>> to work soundly.
>>>
>>> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
>>> CC: David Miller <davem@davemloft.net>
>>> CC: Linus Torvalds <torvalds@linux-foundation.org>
>>> CC: Vlad Yasevich <vyasevich@gmail.com>
>>> CC: linux-sctp@vger.kernel.org
>>> ---
>>> net/sctp/Kconfig | 27 +++++++++++++++++++++++++--
>>> net/sctp/protocol.c | 4 ++--
>>> 2 files changed, 27 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/net/sctp/Kconfig b/net/sctp/Kconfig
>>> index a9edd2e..c262106 100644
>>> --- a/net/sctp/Kconfig
>>> +++ b/net/sctp/Kconfig
>>> @@ -66,12 +66,36 @@ config SCTP_DBG_OBJCNT
>>> 'cat /proc/net/sctp/sctp_dbg_objcnt'
>>>
>>> If unsure, say N
>>> +choice
>>> + prompt "Default SCTP cookie HMAC encoding"
>>> + default SCTP_COOKIE_HMAC_MD5
>>
>> Should not this be SCTP_DEFAULT_COOKIE_HMAC_MD5? I just tried to
>> update to 3.8-rc2, and I usually build my kernel-headers with:
>>
>> yes '' | ARCH=foo make oldconfig
>>
>> and this just kept asking me for this config symbol because none
>> could be provided.
>> --
>> Florian
>>
>
> No, the config mechanism is setup to offer the user the ability to choose a
> default cookie hmac, alg, then optionally select any other hmac algs you would
> like to be made available (in the event you want to change the default at run
> time). When you select the default, it eables (via the select directive), the
> corresponding SCTP_COOKIE_HMAC_* config option, which is used in the build, and
> then prompts for the remaining values.
>
Neil
Actually, I think it should be as Florian suggests. The default value
of the choice should be one of the values defined as part of the choice
(the SCTP_DEFAULT_COOKIE_*). Turning on appropriate default would turn
on appropriate cookie config (SCTP_COOKIE_HMAC_*).
Would that save all the config trouble?
-vlad
> Neil
>
^ permalink raw reply
* Re: [PATCH] xen/netfront: improve truesize tracking
From: Eric Dumazet @ 2013-01-07 15:26 UTC (permalink / raw)
To: Ian Campbell
Cc: Sander Eikelenboom, Rick Jones, netdev@vger.kernel.org,
Konrad Rzeszutek Wilk, annie li, xen-devel@lists.xensource.com
In-Reply-To: <1357568565.7989.96.camel@zakaz.uk.xensource.com>
On Mon, 2013-01-07 at 14:22 +0000, Ian Campbell wrote:
> On Mon, 2013-01-07 at 14:11 +0000, Sander Eikelenboom wrote:
> one is buggy.
> >
> > Will see if i can run against that as well, although i thought Eric
> > said to prefer the "skb->truesize += PAGE_SIZE *
> > skb_shinfo(skb)->nr_frags;"
>
> Right, I meant to compare "PAGE_SIZE * skb_shinfo(skb)->nr_frags" vs
> "skb->data_len - NETFRONT_SKB_CB(skb)->pull_to". TBH I trust Eric so I'm
> inclined to just go with what he suggests.
This is the more precise estimation, and it appears to not bring obvious
performance regression, so I definitely would favor this one ;)
^ permalink raw reply
* Re: [PATCH 1/2] usbnet: allow status interrupt URB to always be active
From: Dan Williams @ 2013-01-07 15:25 UTC (permalink / raw)
To: Oliver Neukum; +Cc: Elina Pasheva, netdev, linux-usb, Rory Filer, Phil Sutter
In-Reply-To: <3476529.YzSzbMTfeu@linux-5eaq.site>
On Sat, 2013-01-05 at 12:01 +0100, Oliver Neukum wrote:
> On Friday 04 January 2013 19:26:33 Dan Williams wrote:
> > On Fri, 2013-01-04 at 23:16 +0100, Oliver Neukum wrote:
> > > On Friday 04 January 2013 10:48:16 Dan Williams wrote:
> > > > Some drivers (ex sierra_net) need the status interrupt URB
> > > > active even when the device is closed, because they receive
> > > > custom indications from firmware. Allow sub-drivers to set
> > > > a flag that submits the status interrupt URB on probe and
> > > > keeps the URB alive over device open/close. The URB is still
> > > > killed/re-submitted for suspend/resume, as before.
> > > >
> > > > Signed-off-by: Dan Williams <dcbw@redhat.com>
> > > > ---
> > > > Oliver: alternatively, is there a problem with *always*
> > > > submitting the interrupt URB, and then simply not calling
> > > > the subdriver's .status function when the netdev is
> > > > closed? That would be a much simpler patch.
> > >
> > > That is quite radical. We have no idea what a device
> > > does when we do not react to a status update. I would
> > > much prefer to not take the risk.
> > > Besides, we don't use bandwidth if we don't have to.
> >
> > Ok, so scratch the alternative. Thus, does the posted patch look like
> > the right course of action?
>
> In principle yes.
>
> > If I wasn't clear enough before, sierra_net needs to listen to the
> > status interrupt URB to receive the custom Restart indication as part of
> > the driver's device setup. Thus for sierra_net at least, tying the
> > status interrupt URB submission to device open/close isn't right.
>
> So, there seems to be an inevitable race before probe() is called.
Yeah, you're right, we need Sierra to send the SYNC message *after* bind
is called. Is there an existing "after bind" hook or do we need to
create one?
> Have you looked at FLAG_AVOID_UNLINK_URBS ?
Not yet, but I will.
Thanks,
Dan
> > I'd previously done a patch to handle this all in sierra_net, but the
> > problem there is suspend/resume: without directly accessing the usbnet
> > structure's ->suspend_count member (icky!) sierra_net can't correctly
> > kill/submit the URB itself. So I went with a flag to usbnet that Sierra
> > can set.
>
> That is absolutely the right way to do it.
>
> Regards
> Oliver
>
> --
> 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 1/2] usbnet: allow status interrupt URB to always be active
From: Dan Williams @ 2013-01-07 15:21 UTC (permalink / raw)
To: Bjørn Mork
Cc: Oliver Neukum, Elina Pasheva, netdev, linux-usb, Rory Filer,
Phil Sutter
In-Reply-To: <87y5g7nbej.fsf@nemi.mork.no>
On Sat, 2013-01-05 at 11:59 +0100, Bjørn Mork wrote:
> Dan Williams <dcbw@redhat.com> writes:
> > On Fri, 2013-01-04 at 23:16 +0100, Oliver Neukum wrote:
> >> On Friday 04 January 2013 10:48:16 Dan Williams wrote:
> >> > Some drivers (ex sierra_net) need the status interrupt URB
> >> > active even when the device is closed, because they receive
> >> > custom indications from firmware. Allow sub-drivers to set
> >> > a flag that submits the status interrupt URB on probe and
> >> > keeps the URB alive over device open/close. The URB is still
> >> > killed/re-submitted for suspend/resume, as before.
> >> >
> >> > Signed-off-by: Dan Williams <dcbw@redhat.com>
> >> > ---
> >> > Oliver: alternatively, is there a problem with *always*
> >> > submitting the interrupt URB, and then simply not calling
> >> > the subdriver's .status function when the netdev is
> >> > closed? That would be a much simpler patch.
> >>
> >> That is quite radical. We have no idea what a device
> >> does when we do not react to a status update. I would
> >> much prefer to not take the risk.
> >> Besides, we don't use bandwidth if we don't have to.
> >
> > Ok, so scratch the alternative. Thus, does the posted patch look like
> > the right course of action?
> >
> > If I wasn't clear enough before, sierra_net needs to listen to the
> > status interrupt URB to receive the custom Restart indication as part of
> > the driver's device setup. Thus for sierra_net at least, tying the
> > status interrupt URB submission to device open/close isn't right.
> >
> > I'd previously done a patch to handle this all in sierra_net, but the
> > problem there is suspend/resume: without directly accessing the usbnet
> > structure's ->suspend_count member (icky!) sierra_net can't correctly
> > kill/submit the URB itself. So I went with a flag to usbnet that Sierra
> > can set.
>
> Just a few random thoughts...
>
> The sierra_net driver would have been an excellent candidate for the
> cdc-wdm subdriver model if that had existed when sierra_net was written.
> I assume the current driver only implements a minimal subset of the
> DirectIP HIP protocol embedded in CDC, and exporting this to userspace
> instead would have made it possible to extend that support without
> changing the driver, in addtion to making the driver much more robust
> against firmware differences. It would have eliminated the problem you
> are facing and removed the minidriver workqueue complexity.
>
> But I guess it's too late to change this now. In theory we could
> implement the cdc-wdm hooks that were initially proposed for cdc_mbim
> and rewrite sierra_net to use it while being backwards compatible.
> Don't think anyone wants to do either, so forget about it...
>
> You can still use a trick similar to what qmi_wwan and cdc_mbim does to
> take over the status endpoint from usbnet: By not implementing .status,
> and possibly setting dev->status to NULL in .bind, you are free to
> handle the status endpoint entirely inside the minidriver. Not sure if
> that is smart though. You would have to reimplement init_status and
> intr_complete from usbnet, and kill or resubmit the interrupt urb on
> suspend/resume/disconnect yourself.
That was exactly the approach of my first patch. But that was icky
because we need to also track suspend/resume state by looking at
internal members of usbnet's structure. Yes, it's specific to Sierra,
but it's pretty much a layer violation. So I went with the flag
approach.
Dan
> The new usbnet flag is probably a better solution.
>
> FWIW, I agree with Oliver that always submitting the interrupt URB is
> both risky and will cause too much unnecessary USB activity for most
> usbnet devices.
>
>
> Bjørn
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" 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 v2] sctp: Change defaults on cookie hmac selection
From: Florian Fainelli @ 2013-01-07 15:15 UTC (permalink / raw)
To: Neil Horman
Cc: netdev, David Miller, Linus Torvalds, Vlad Yasevich, linux-sctp
In-Reply-To: <20130107144921.GA31577@hmsreliant.think-freely.org>
Le 01/07/13 15:49, Neil Horman a écrit :
> On Mon, Jan 07, 2013 at 02:25:39PM +0100, Florian Fainelli wrote:
>> Hello Neil,
>>
>> Le 12/15/12 02:22, Neil Horman a écrit :
>>> Recently I posted commit 3c68198e75 which made selection of the cookie hmac
>>> algorithm selectable. This is all well and good, but Linus noted that it
>>> changes the default config:
>>> http://marc.info/?l=linux-netdev&m=135536629004808&w=2
>>>
>>> I've modified the sctp Kconfig file to reflect the recommended way of making
>>> this choice, using the thermal driver example specified, and brought the
>>> defaults back into line with the way they were prior to my origional patch
>>>
>>> Also, on Linus' suggestion, re-adding ability to select default 'none' hmac
>>> algorithm, so we don't needlessly bloat the kernel by forcing a non-none
>>> default. This also led me to note that we won't honor the default none
>>> condition properly because of how sctp_net_init is encoded. Fix that up as
>>> well.
>>>
>>> Tested by myself (allbeit fairly quickly). All configuration combinations seems
>>> to work soundly.
>>>
>>> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
>>> CC: David Miller <davem@davemloft.net>
>>> CC: Linus Torvalds <torvalds@linux-foundation.org>
>>> CC: Vlad Yasevich <vyasevich@gmail.com>
>>> CC: linux-sctp@vger.kernel.org
>>> ---
>>> net/sctp/Kconfig | 27 +++++++++++++++++++++++++--
>>> net/sctp/protocol.c | 4 ++--
>>> 2 files changed, 27 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/net/sctp/Kconfig b/net/sctp/Kconfig
>>> index a9edd2e..c262106 100644
>>> --- a/net/sctp/Kconfig
>>> +++ b/net/sctp/Kconfig
>>> @@ -66,12 +66,36 @@ config SCTP_DBG_OBJCNT
>>> 'cat /proc/net/sctp/sctp_dbg_objcnt'
>>>
>>> If unsure, say N
>>> +choice
>>> + prompt "Default SCTP cookie HMAC encoding"
>>> + default SCTP_COOKIE_HMAC_MD5
>> Should not this be SCTP_DEFAULT_COOKIE_HMAC_MD5? I just tried to
>> update to 3.8-rc2, and I usually build my kernel-headers with:
>>
>> yes '' | ARCH=foo make oldconfig
>>
>> and this just kept asking me for this config symbol because none
>> could be provided.
>> --
>> Florian
>>
> No, the config mechanism is setup to offer the user the ability to choose a
> default cookie hmac, alg, then optionally select any other hmac algs you would
> like to be made available (in the event you want to change the default at run
> time). When you select the default, it eables (via the select directive), the
> corresponding SCTP_COOKIE_HMAC_* config option, which is used in the build, and
> then prompts for the remaining values.
Ok for the explanation, but this still breaks an oldconfig because we do
not actually propose the user with a default choice:
choice[1-3?]: Default SCTP cookie HMAC encoding
1. Enable optional MD5 hmac cookie generation
(SCTP_DEFAULT_COOKIE_HMAC_MD5) (NEW)
2. Enable optional SHA1 hmac cookie generation
(SCTP_DEFAULT_COOKIE_HMAC_SHA1) (NEW)
3. Use no hmac alg in SCTP cookie generation
(SCTP_DEFAULT_COOKIE_HMAC_NONE) (NEW)
I do not see any difference in what I am proposed if the default config
symbol is SCTP_DEFAULT_COOKIE_HMAC_MD5, I can still optionally choose
SHA1 to be supported, and I do have a valid default config for this
choice. While if I keep SCTP_COOKIE_HMAC_MD5 as the default I have to
manually enter which option I want.
--
Florian
^ permalink raw reply
* Re: [PATCH V3 2/2] vhost: handle polling errors
From: Jason Wang @ 2013-01-07 15:04 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: kvm, eric.dumazet, netdev, linux-kernel, virtualization, davem
In-Reply-To: <20130107145559.GA5901@redhat.com>
On 01/07/2013 10:55 PM, Michael S. Tsirkin wrote:
> On Mon, Jan 07, 2013 at 12:38:17PM +0800, Jason Wang wrote:
>> On 01/06/2013 09:22 PM, Michael S. Tsirkin wrote:
>>> On Sun, Jan 06, 2013 at 03:18:38PM +0800, Jason Wang wrote:
>>>> Polling errors were ignored by vhost/vhost_net, this may lead to crash when
>>>> trying to remove vhost from waitqueue when after the polling is failed. Solve
>>>> this problem by:
>>>>
>>>> - checking the poll->wqh before trying to remove from waitqueue
>>>> - report an error when poll() returns a POLLERR in vhost_start_poll()
>>>> - report an error when vhost_start_poll() fails in
>>>> vhost_vring_ioctl()/vhost_net_set_backend() which is used to notify the
>>>> failure to userspace.
>>>> - report an error in the data path in vhost_net when meet polling errors.
>>>>
>>>> After those changes, we can safely drop the tx polling state in vhost_net since
>>>> it was replaced by the checking of poll->wqh.
>>>>
>>>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>>>> ---
>>>> drivers/vhost/net.c | 74 ++++++++++++++++--------------------------------
>>>> drivers/vhost/vhost.c | 31 +++++++++++++++-----
>>>> drivers/vhost/vhost.h | 2 +-
>>>> 3 files changed, 49 insertions(+), 58 deletions(-)
>>>>
>>>> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
>>>> index d10ad6f..125c1e5 100644
>>>> --- a/drivers/vhost/net.c
>>>> +++ b/drivers/vhost/net.c
>>>> @@ -64,20 +64,10 @@ enum {
>>>> VHOST_NET_VQ_MAX = 2,
>>>> };
>>>>
>>>> -enum vhost_net_poll_state {
>>>> - VHOST_NET_POLL_DISABLED = 0,
>>>> - VHOST_NET_POLL_STARTED = 1,
>>>> - VHOST_NET_POLL_STOPPED = 2,
>>>> -};
>>>> -
>>>> struct vhost_net {
>>>> struct vhost_dev dev;
>>>> struct vhost_virtqueue vqs[VHOST_NET_VQ_MAX];
>>>> struct vhost_poll poll[VHOST_NET_VQ_MAX];
>>>> - /* Tells us whether we are polling a socket for TX.
>>>> - * We only do this when socket buffer fills up.
>>>> - * Protected by tx vq lock. */
>>>> - enum vhost_net_poll_state tx_poll_state;
>>>> /* Number of TX recently submitted.
>>>> * Protected by tx vq lock. */
>>>> unsigned tx_packets;
>>>> @@ -155,24 +145,6 @@ static void copy_iovec_hdr(const struct iovec *from, struct iovec *to,
>>>> }
>>>> }
>>>>
>>>> -/* Caller must have TX VQ lock */
>>>> -static void tx_poll_stop(struct vhost_net *net)
>>>> -{
>>>> - if (likely(net->tx_poll_state != VHOST_NET_POLL_STARTED))
>>>> - return;
>>>> - vhost_poll_stop(net->poll + VHOST_NET_VQ_TX);
>>>> - net->tx_poll_state = VHOST_NET_POLL_STOPPED;
>>>> -}
>>>> -
>>>> -/* Caller must have TX VQ lock */
>>>> -static void tx_poll_start(struct vhost_net *net, struct socket *sock)
>>>> -{
>>>> - if (unlikely(net->tx_poll_state != VHOST_NET_POLL_STOPPED))
>>>> - return;
>>>> - vhost_poll_start(net->poll + VHOST_NET_VQ_TX, sock->file);
>>>> - net->tx_poll_state = VHOST_NET_POLL_STARTED;
>>>> -}
>>>> -
>>>> /* In case of DMA done not in order in lower device driver for some reason.
>>>> * upend_idx is used to track end of used idx, done_idx is used to track head
>>>> * of used idx. Once lower device DMA done contiguously, we will signal KVM
>>>> @@ -227,6 +199,7 @@ static void vhost_zerocopy_callback(struct ubuf_info *ubuf, bool success)
>>>> static void handle_tx(struct vhost_net *net)
>>>> {
>>>> struct vhost_virtqueue *vq = &net->dev.vqs[VHOST_NET_VQ_TX];
>>>> + struct vhost_poll *poll = net->poll + VHOST_NET_VQ_TX;
>>>> unsigned out, in, s;
>>>> int head;
>>>> struct msghdr msg = {
>>>> @@ -252,7 +225,8 @@ static void handle_tx(struct vhost_net *net)
>>>> wmem = atomic_read(&sock->sk->sk_wmem_alloc);
>>>> if (wmem >= sock->sk->sk_sndbuf) {
>>>> mutex_lock(&vq->mutex);
>>>> - tx_poll_start(net, sock);
>>>> + if (vhost_poll_start(poll, sock->file))
>>>> + vq_err(vq, "Fail to start TX polling\n");
>>> s/Fail/Failed/
>>>
>>> A question though: how can this happen? Could you clarify please?
>>> Maybe we can find a way to prevent this error?
>> Two conditions I think this can happen:
>>
>> 1) a buggy userspace disable a queue through TUNSETQUEUE
>> 2) the net device were gone
>>
>> For 1, looks like we can delay the disabling until the refcnt goes to
>> zero. For 2 may needs more changes.
> I'd expect keeping a socket reference would prevent both issues.
> Doesn't it?
Doesn't work for 2 I think, the socket didn't hold a refcnt of the
device, so the device can go away at anytime. Although we can change
this, but it's the behaviour before multiqueue support.
>
>> Not sure it's worth to do this work,
>> maybe a warning is enough just like other failure.
> With other failures, you normally can correct the error then
> kick to have it restart. This is soomething thagt would not
> work here.
If userspace is wrote correctly, (e.g passing a fd with correct state)
it can also be corrected.
>
>>>> mutex_unlock(&vq->mutex);
>>>> return;
>>>> }
>>>> @@ -261,7 +235,7 @@ static void handle_tx(struct vhost_net *net)
>>>> vhost_disable_notify(&net->dev, vq);
>>>>
>>>> if (wmem < sock->sk->sk_sndbuf / 2)
>>>> - tx_poll_stop(net);
>>>> + vhost_poll_stop(poll);
>>>> hdr_size = vq->vhost_hlen;
>>>> zcopy = vq->ubufs;
>>>>
>>>> @@ -283,8 +257,10 @@ static void handle_tx(struct vhost_net *net)
>>>>
>>>> wmem = atomic_read(&sock->sk->sk_wmem_alloc);
>>>> if (wmem >= sock->sk->sk_sndbuf * 3 / 4) {
>>>> - tx_poll_start(net, sock);
>>>> - set_bit(SOCK_ASYNC_NOSPACE, &sock->flags);
>>>> + if (vhost_poll_start(poll, sock->file))
>>>> + vq_err(vq, "Fail to start TX polling\n");
>>>> + else
>>>> + set_bit(SOCK_ASYNC_NOSPACE, &sock->flags);
>>>> break;
>>>> }
>>>> /* If more outstanding DMAs, queue the work.
>>>> @@ -294,8 +270,10 @@ static void handle_tx(struct vhost_net *net)
>>>> (vq->upend_idx - vq->done_idx) :
>>>> (vq->upend_idx + UIO_MAXIOV - vq->done_idx);
>>>> if (unlikely(num_pends > VHOST_MAX_PEND)) {
>>>> - tx_poll_start(net, sock);
>>>> - set_bit(SOCK_ASYNC_NOSPACE, &sock->flags);
>>>> + if (vhost_poll_start(poll, sock->file))
>>>> + vq_err(vq, "Fail to start TX polling\n");
>>>> + else
>>>> + set_bit(SOCK_ASYNC_NOSPACE, &sock->flags);
>>>> break;
>>>> }
>>>> if (unlikely(vhost_enable_notify(&net->dev, vq))) {
>>>> @@ -360,7 +338,8 @@ static void handle_tx(struct vhost_net *net)
>>>> }
>>>> vhost_discard_vq_desc(vq, 1);
>>>> if (err == -EAGAIN || err == -ENOBUFS)
>>>> - tx_poll_start(net, sock);
>>>> + if (vhost_poll_start(poll, sock->file))
>>>> + vq_err(vq, "Fail to start TX polling\n");
>>>> break;
>>>> }
>>>> if (err != len)
>>>> @@ -623,7 +602,6 @@ static int vhost_net_open(struct inode *inode, struct file *f)
>>>>
>>>> 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);
>>>> - n->tx_poll_state = VHOST_NET_POLL_DISABLED;
>>>>
>>>> f->private_data = n;
>>>>
>>>> @@ -633,29 +611,25 @@ static int vhost_net_open(struct inode *inode, struct file *f)
>>>> static void vhost_net_disable_vq(struct vhost_net *n,
>>>> struct vhost_virtqueue *vq)
>>>> {
>>>> + struct vhost_poll *poll = n->poll + (vq - n->vqs);
>>>> +
>>>> if (!vq->private_data)
>>>> return;
>>>> - if (vq == n->vqs + VHOST_NET_VQ_TX) {
>>>> - tx_poll_stop(n);
>>>> - n->tx_poll_state = VHOST_NET_POLL_DISABLED;
>>>> - } else
>>>> - vhost_poll_stop(n->poll + VHOST_NET_VQ_RX);
>>>> + vhost_poll_stop(poll);
>>>> }
>>>>
>>>> -static void vhost_net_enable_vq(struct vhost_net *n,
>>>> +static int vhost_net_enable_vq(struct vhost_net *n,
>>>> struct vhost_virtqueue *vq)
>>>> {
>>>> struct socket *sock;
>>>> + struct vhost_poll *poll = n->poll + (vq - n->vqs);
>>>>
>>>> sock = rcu_dereference_protected(vq->private_data,
>>>> lockdep_is_held(&vq->mutex));
>>>> if (!sock)
>>>> - return;
>>>> - if (vq == n->vqs + VHOST_NET_VQ_TX) {
>>>> - n->tx_poll_state = VHOST_NET_POLL_STOPPED;
>>>> - tx_poll_start(n, sock);
>>>> - } else
>>>> - vhost_poll_start(n->poll + VHOST_NET_VQ_RX, sock->file);
>>>> + return 0;
>>>> +
>>>> + return vhost_poll_start(poll, sock->file);
>>>> }
>>>>
>>>> static struct socket *vhost_net_stop_vq(struct vhost_net *n,
>>>> @@ -833,7 +807,9 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
>>>> r = vhost_init_used(vq);
>>>> if (r)
>>>> goto err_used;
>>>> - vhost_net_enable_vq(n, vq);
>>>> + r = vhost_net_enable_vq(n, vq);
>>>> + if (r)
>>>> + goto err_used;
>>>>
>>>> oldubufs = vq->ubufs;
>>>> vq->ubufs = ubufs;
>>>> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
>>>> index 34389f7..5c7a466 100644
>>>> --- a/drivers/vhost/vhost.c
>>>> +++ b/drivers/vhost/vhost.c
>>>> @@ -77,26 +77,41 @@ void vhost_poll_init(struct vhost_poll *poll, vhost_work_fn_t fn,
>>>> init_poll_funcptr(&poll->table, vhost_poll_func);
>>>> poll->mask = mask;
>>>> poll->dev = dev;
>>>> + poll->wqh = NULL;
>>>>
>>>> vhost_work_init(&poll->work, fn);
>>>> }
>>>>
>>>> +/* Stop polling a file. After this function returns, it becomes safe to drop the
>>>> + * file reference. You must also flush afterwards. */
>>>> +void vhost_poll_stop(struct vhost_poll *poll)
>>>> +{
>>>> + if (poll->wqh) {
>>>> + remove_wait_queue(poll->wqh, &poll->wait);
>>>> + poll->wqh = NULL;
>>>> + }
>>>> +}
>>>> +
>>>> /* Start polling a file. We add ourselves to file's wait queue. The caller must
>>>> * keep a reference to a file until after vhost_poll_stop is called. */
>>>> -void vhost_poll_start(struct vhost_poll *poll, struct file *file)
>>>> +int vhost_poll_start(struct vhost_poll *poll, struct file *file)
>>>> {
>>>> unsigned long mask;
>>>> + int ret = 0;
>>>> +
>>>> + if (poll->wqh)
>>>> + return -EBUSY;
>>>>
>>> I think this should return success: we are already polling.
>>> Otherwise this would trigger a bug below I think.
>> Ok.
>>>> mask = file->f_op->poll(file, &poll->table);
>>>> if (mask)
>>>> vhost_poll_wakeup(&poll->wait, 0, 0, (void *)mask);
>>>> -}
>>>>
>>>> -/* Stop polling a file. After this function returns, it becomes safe to drop the
>>>> - * file reference. You must also flush afterwards. */
>>>> -void vhost_poll_stop(struct vhost_poll *poll)
>>>> -{
>>>> - remove_wait_queue(poll->wqh, &poll->wait);
>>>> + if (mask & POLLERR) {
>>>> + ret = -EINVAL;
>>>> + vhost_poll_stop(poll);
>>>> + }
>>>> +
>>>> + return ret;
>>>> }
>>>>
>>>> static bool vhost_work_seq_done(struct vhost_dev *dev, struct vhost_work *work,
>>>> @@ -792,7 +807,7 @@ long vhost_vring_ioctl(struct vhost_dev *d, int ioctl, void __user *argp)
>>>> fput(filep);
>>>>
>>>> if (pollstart && vq->handle_kick)
>>>> - vhost_poll_start(&vq->poll, vq->kick);
>>>> + r = vhost_poll_start(&vq->poll, vq->kick);
>>>>
>>>> mutex_unlock(&vq->mutex);
>>>>
>>>> diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
>>>> index 2639c58..17261e2 100644
>>>> --- a/drivers/vhost/vhost.h
>>>> +++ b/drivers/vhost/vhost.h
>>>> @@ -42,7 +42,7 @@ 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);
>>>> -void vhost_poll_start(struct vhost_poll *poll, struct file *file);
>>>> +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);
>>>> void vhost_poll_queue(struct vhost_poll *poll);
>>>> --
>>>> 1.7.1
>>> --
>>> 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
> --
> 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
* Re: [PATCH V3 2/2] vhost: handle polling errors
From: Michael S. Tsirkin @ 2013-01-07 14:55 UTC (permalink / raw)
To: Jason Wang; +Cc: kvm, eric.dumazet, netdev, linux-kernel, virtualization, davem
In-Reply-To: <50EA5139.7000904@redhat.com>
On Mon, Jan 07, 2013 at 12:38:17PM +0800, Jason Wang wrote:
> On 01/06/2013 09:22 PM, Michael S. Tsirkin wrote:
> > On Sun, Jan 06, 2013 at 03:18:38PM +0800, Jason Wang wrote:
> >> Polling errors were ignored by vhost/vhost_net, this may lead to crash when
> >> trying to remove vhost from waitqueue when after the polling is failed. Solve
> >> this problem by:
> >>
> >> - checking the poll->wqh before trying to remove from waitqueue
> >> - report an error when poll() returns a POLLERR in vhost_start_poll()
> >> - report an error when vhost_start_poll() fails in
> >> vhost_vring_ioctl()/vhost_net_set_backend() which is used to notify the
> >> failure to userspace.
> >> - report an error in the data path in vhost_net when meet polling errors.
> >>
> >> After those changes, we can safely drop the tx polling state in vhost_net since
> >> it was replaced by the checking of poll->wqh.
> >>
> >> Signed-off-by: Jason Wang <jasowang@redhat.com>
> >> ---
> >> drivers/vhost/net.c | 74 ++++++++++++++++--------------------------------
> >> drivers/vhost/vhost.c | 31 +++++++++++++++-----
> >> drivers/vhost/vhost.h | 2 +-
> >> 3 files changed, 49 insertions(+), 58 deletions(-)
> >>
> >> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> >> index d10ad6f..125c1e5 100644
> >> --- a/drivers/vhost/net.c
> >> +++ b/drivers/vhost/net.c
> >> @@ -64,20 +64,10 @@ enum {
> >> VHOST_NET_VQ_MAX = 2,
> >> };
> >>
> >> -enum vhost_net_poll_state {
> >> - VHOST_NET_POLL_DISABLED = 0,
> >> - VHOST_NET_POLL_STARTED = 1,
> >> - VHOST_NET_POLL_STOPPED = 2,
> >> -};
> >> -
> >> struct vhost_net {
> >> struct vhost_dev dev;
> >> struct vhost_virtqueue vqs[VHOST_NET_VQ_MAX];
> >> struct vhost_poll poll[VHOST_NET_VQ_MAX];
> >> - /* Tells us whether we are polling a socket for TX.
> >> - * We only do this when socket buffer fills up.
> >> - * Protected by tx vq lock. */
> >> - enum vhost_net_poll_state tx_poll_state;
> >> /* Number of TX recently submitted.
> >> * Protected by tx vq lock. */
> >> unsigned tx_packets;
> >> @@ -155,24 +145,6 @@ static void copy_iovec_hdr(const struct iovec *from, struct iovec *to,
> >> }
> >> }
> >>
> >> -/* Caller must have TX VQ lock */
> >> -static void tx_poll_stop(struct vhost_net *net)
> >> -{
> >> - if (likely(net->tx_poll_state != VHOST_NET_POLL_STARTED))
> >> - return;
> >> - vhost_poll_stop(net->poll + VHOST_NET_VQ_TX);
> >> - net->tx_poll_state = VHOST_NET_POLL_STOPPED;
> >> -}
> >> -
> >> -/* Caller must have TX VQ lock */
> >> -static void tx_poll_start(struct vhost_net *net, struct socket *sock)
> >> -{
> >> - if (unlikely(net->tx_poll_state != VHOST_NET_POLL_STOPPED))
> >> - return;
> >> - vhost_poll_start(net->poll + VHOST_NET_VQ_TX, sock->file);
> >> - net->tx_poll_state = VHOST_NET_POLL_STARTED;
> >> -}
> >> -
> >> /* In case of DMA done not in order in lower device driver for some reason.
> >> * upend_idx is used to track end of used idx, done_idx is used to track head
> >> * of used idx. Once lower device DMA done contiguously, we will signal KVM
> >> @@ -227,6 +199,7 @@ static void vhost_zerocopy_callback(struct ubuf_info *ubuf, bool success)
> >> static void handle_tx(struct vhost_net *net)
> >> {
> >> struct vhost_virtqueue *vq = &net->dev.vqs[VHOST_NET_VQ_TX];
> >> + struct vhost_poll *poll = net->poll + VHOST_NET_VQ_TX;
> >> unsigned out, in, s;
> >> int head;
> >> struct msghdr msg = {
> >> @@ -252,7 +225,8 @@ static void handle_tx(struct vhost_net *net)
> >> wmem = atomic_read(&sock->sk->sk_wmem_alloc);
> >> if (wmem >= sock->sk->sk_sndbuf) {
> >> mutex_lock(&vq->mutex);
> >> - tx_poll_start(net, sock);
> >> + if (vhost_poll_start(poll, sock->file))
> >> + vq_err(vq, "Fail to start TX polling\n");
> > s/Fail/Failed/
> >
> > A question though: how can this happen? Could you clarify please?
> > Maybe we can find a way to prevent this error?
>
> Two conditions I think this can happen:
>
> 1) a buggy userspace disable a queue through TUNSETQUEUE
> 2) the net device were gone
>
> For 1, looks like we can delay the disabling until the refcnt goes to
> zero. For 2 may needs more changes.
I'd expect keeping a socket reference would prevent both issues.
Doesn't it?
> Not sure it's worth to do this work,
> maybe a warning is enough just like other failure.
With other failures, you normally can correct the error then
kick to have it restart. This is soomething thagt would not
work here.
> >
> >> mutex_unlock(&vq->mutex);
> >> return;
> >> }
> >> @@ -261,7 +235,7 @@ static void handle_tx(struct vhost_net *net)
> >> vhost_disable_notify(&net->dev, vq);
> >>
> >> if (wmem < sock->sk->sk_sndbuf / 2)
> >> - tx_poll_stop(net);
> >> + vhost_poll_stop(poll);
> >> hdr_size = vq->vhost_hlen;
> >> zcopy = vq->ubufs;
> >>
> >> @@ -283,8 +257,10 @@ static void handle_tx(struct vhost_net *net)
> >>
> >> wmem = atomic_read(&sock->sk->sk_wmem_alloc);
> >> if (wmem >= sock->sk->sk_sndbuf * 3 / 4) {
> >> - tx_poll_start(net, sock);
> >> - set_bit(SOCK_ASYNC_NOSPACE, &sock->flags);
> >> + if (vhost_poll_start(poll, sock->file))
> >> + vq_err(vq, "Fail to start TX polling\n");
> >> + else
> >> + set_bit(SOCK_ASYNC_NOSPACE, &sock->flags);
> >> break;
> >> }
> >> /* If more outstanding DMAs, queue the work.
> >> @@ -294,8 +270,10 @@ static void handle_tx(struct vhost_net *net)
> >> (vq->upend_idx - vq->done_idx) :
> >> (vq->upend_idx + UIO_MAXIOV - vq->done_idx);
> >> if (unlikely(num_pends > VHOST_MAX_PEND)) {
> >> - tx_poll_start(net, sock);
> >> - set_bit(SOCK_ASYNC_NOSPACE, &sock->flags);
> >> + if (vhost_poll_start(poll, sock->file))
> >> + vq_err(vq, "Fail to start TX polling\n");
> >> + else
> >> + set_bit(SOCK_ASYNC_NOSPACE, &sock->flags);
> >> break;
> >> }
> >> if (unlikely(vhost_enable_notify(&net->dev, vq))) {
> >> @@ -360,7 +338,8 @@ static void handle_tx(struct vhost_net *net)
> >> }
> >> vhost_discard_vq_desc(vq, 1);
> >> if (err == -EAGAIN || err == -ENOBUFS)
> >> - tx_poll_start(net, sock);
> >> + if (vhost_poll_start(poll, sock->file))
> >> + vq_err(vq, "Fail to start TX polling\n");
> >> break;
> >> }
> >> if (err != len)
> >> @@ -623,7 +602,6 @@ static int vhost_net_open(struct inode *inode, struct file *f)
> >>
> >> 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);
> >> - n->tx_poll_state = VHOST_NET_POLL_DISABLED;
> >>
> >> f->private_data = n;
> >>
> >> @@ -633,29 +611,25 @@ static int vhost_net_open(struct inode *inode, struct file *f)
> >> static void vhost_net_disable_vq(struct vhost_net *n,
> >> struct vhost_virtqueue *vq)
> >> {
> >> + struct vhost_poll *poll = n->poll + (vq - n->vqs);
> >> +
> >> if (!vq->private_data)
> >> return;
> >> - if (vq == n->vqs + VHOST_NET_VQ_TX) {
> >> - tx_poll_stop(n);
> >> - n->tx_poll_state = VHOST_NET_POLL_DISABLED;
> >> - } else
> >> - vhost_poll_stop(n->poll + VHOST_NET_VQ_RX);
> >> + vhost_poll_stop(poll);
> >> }
> >>
> >> -static void vhost_net_enable_vq(struct vhost_net *n,
> >> +static int vhost_net_enable_vq(struct vhost_net *n,
> >> struct vhost_virtqueue *vq)
> >> {
> >> struct socket *sock;
> >> + struct vhost_poll *poll = n->poll + (vq - n->vqs);
> >>
> >> sock = rcu_dereference_protected(vq->private_data,
> >> lockdep_is_held(&vq->mutex));
> >> if (!sock)
> >> - return;
> >> - if (vq == n->vqs + VHOST_NET_VQ_TX) {
> >> - n->tx_poll_state = VHOST_NET_POLL_STOPPED;
> >> - tx_poll_start(n, sock);
> >> - } else
> >> - vhost_poll_start(n->poll + VHOST_NET_VQ_RX, sock->file);
> >> + return 0;
> >> +
> >> + return vhost_poll_start(poll, sock->file);
> >> }
> >>
> >> static struct socket *vhost_net_stop_vq(struct vhost_net *n,
> >> @@ -833,7 +807,9 @@ static long vhost_net_set_backend(struct vhost_net *n, unsigned index, int fd)
> >> r = vhost_init_used(vq);
> >> if (r)
> >> goto err_used;
> >> - vhost_net_enable_vq(n, vq);
> >> + r = vhost_net_enable_vq(n, vq);
> >> + if (r)
> >> + goto err_used;
> >>
> >> oldubufs = vq->ubufs;
> >> vq->ubufs = ubufs;
> >> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> >> index 34389f7..5c7a466 100644
> >> --- a/drivers/vhost/vhost.c
> >> +++ b/drivers/vhost/vhost.c
> >> @@ -77,26 +77,41 @@ void vhost_poll_init(struct vhost_poll *poll, vhost_work_fn_t fn,
> >> init_poll_funcptr(&poll->table, vhost_poll_func);
> >> poll->mask = mask;
> >> poll->dev = dev;
> >> + poll->wqh = NULL;
> >>
> >> vhost_work_init(&poll->work, fn);
> >> }
> >>
> >> +/* Stop polling a file. After this function returns, it becomes safe to drop the
> >> + * file reference. You must also flush afterwards. */
> >> +void vhost_poll_stop(struct vhost_poll *poll)
> >> +{
> >> + if (poll->wqh) {
> >> + remove_wait_queue(poll->wqh, &poll->wait);
> >> + poll->wqh = NULL;
> >> + }
> >> +}
> >> +
> >> /* Start polling a file. We add ourselves to file's wait queue. The caller must
> >> * keep a reference to a file until after vhost_poll_stop is called. */
> >> -void vhost_poll_start(struct vhost_poll *poll, struct file *file)
> >> +int vhost_poll_start(struct vhost_poll *poll, struct file *file)
> >> {
> >> unsigned long mask;
> >> + int ret = 0;
> >> +
> >> + if (poll->wqh)
> >> + return -EBUSY;
> >>
> > I think this should return success: we are already polling.
> > Otherwise this would trigger a bug below I think.
>
> Ok.
> >
> >> mask = file->f_op->poll(file, &poll->table);
> >> if (mask)
> >> vhost_poll_wakeup(&poll->wait, 0, 0, (void *)mask);
> >> -}
> >>
> >> -/* Stop polling a file. After this function returns, it becomes safe to drop the
> >> - * file reference. You must also flush afterwards. */
> >> -void vhost_poll_stop(struct vhost_poll *poll)
> >> -{
> >> - remove_wait_queue(poll->wqh, &poll->wait);
> >> + if (mask & POLLERR) {
> >> + ret = -EINVAL;
> >> + vhost_poll_stop(poll);
> >> + }
> >> +
> >> + return ret;
> >> }
> >>
> >> static bool vhost_work_seq_done(struct vhost_dev *dev, struct vhost_work *work,
> >> @@ -792,7 +807,7 @@ long vhost_vring_ioctl(struct vhost_dev *d, int ioctl, void __user *argp)
> >> fput(filep);
> >>
> >> if (pollstart && vq->handle_kick)
> >> - vhost_poll_start(&vq->poll, vq->kick);
> >> + r = vhost_poll_start(&vq->poll, vq->kick);
> >>
> >> mutex_unlock(&vq->mutex);
> >>
> >> diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> >> index 2639c58..17261e2 100644
> >> --- a/drivers/vhost/vhost.h
> >> +++ b/drivers/vhost/vhost.h
> >> @@ -42,7 +42,7 @@ 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);
> >> -void vhost_poll_start(struct vhost_poll *poll, struct file *file);
> >> +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);
> >> void vhost_poll_queue(struct vhost_poll *poll);
> >> --
> >> 1.7.1
> > --
> > 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
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox